Hi Ulrich,

On Wednesday, 18 October 2017 18:57:14 EEST Laurent Pinchart wrote:

On Wednesday, 18 October 2017 17:53:54 EEST Ulrich Hecht wrote:

quoted

On Tue, Oct 17, 2017 at 6:05 PM, Sean Paul [off-list ref] wrote:

quoted

On Tue, Oct 17, 2017 at 12:33:09PM +0200, Matthias Brugger wrote:

quoted

From what I understand you
rebased the patches from the chromium kernel to mainline. So you should
keep the signed-off-by from the original author and just add you
signed-off-by below that.
Also it would be nice to CC these persons so that they are aware of
your
effort.>

Additionally, make sure the author and commit messages are preserved
when
you send patches. You can add your own message in addition to your SoB,
but please try to preserve history.

Fair enough, but I'm not sure what exactly to include. The chromium
commits include, among other things:

- bugtracker numbers and test cases
- "Change-Id" and "Commit-Ready" (no idea what those are)
- "Reviewed-on", containing links to chromium-review.googlesource.com

With all changes to the PS8640 driver concatenated and non-standard
tags removed, the commit message would look like this:

=============================

    CHROMIUM: drm/bridge: Add I2C based driver for ps8640 bridge
    
    This patch adds drm_bridge driver for parade DSI to eDP bridge chip.
    
    Signed-off-by: Jitao Shi [off-list ref]
    (am from https://patchwork.kernel.org/patch/8357851/)
    Signed-off-by: Daniel Kurtz [off-list ref]
    Tested-by: cawa cheng [off-list ref]
    Tested-by: Daniel Kurtz [off-list ref]
    Reviewed-by: Nicolas Boichat [off-list ref]
    
    
    FIXUP: FROMLIST: drm/bridge: Add I2C based driver for ps8640 bridge
    
    chromeos-3.18 currently has FROMLIST v15 of the PS8640 driver.
    This version is incompatible with UPSTREAM version of the Mediatek
    DRM driver.
    
    To quote Philipp Zabel:
    "The main DRM driver mtk_drm_drv now calls
    drm_connector_register_all() after drm_dev_register() in the
    mtk_drm_bind() function. That function should iterate over all
    connectors and call drm_connector_register() for each of them.
    The call to drm_connector_init() from mtk_hdmi_bridge_attach()
    should be enough to make this happen.
    
    The drm_connector_(un)register calls also have to be removed
    from the ps8640 driver."
    
    Signed-off-by: Jitao Shi [off-list ref]
    Signed-off-by: Daniel Kurtz [off-list ref]
    Tested-by: PC Liao [off-list ref]
    Tested-by: Daniel Kurtz [off-list ref]
    Tested-by: Nicolas Boichat [off-list ref]
    Reviewed-by: Daniel Kurtz [off-list ref]
    
    
    CHROMIUM: drm/bridge: ps8640: Add a 3 ms delay before unmuting output
    
    For current ps8640 firmware, the PS_GPIO9 signal only indicates the

bridge has seen the attached panel's HPD signal and read its EDID.
Unfortunately, the bridge may not yet be ready to properly handle DSI/eDP
traffic yet.

    For now, Paradetech has recommended adding a 3 ms delay after PS_GPIO9
    before starting video signal transmission.
    
    Signed-off-by: Jitao Shi [off-list ref]
    Signed-off-by: Daniel Kurtz [off-list ref]
    
    Tested-by: cawa cheng [off-list ref]
    Tested-by: Daniel Kurtz [off-list ref]
    Tested-by: jitao shi [off-list ref]
    Reviewed-by: Daniel Kurtz [off-list ref]
    Reviewed-by: jitao shi [off-list ref]
    
    
    CHROMIUM: drm/bridge: ps8640: disable MIPI MCS
    
    Disable PS8640 MIPI MCS commands to workaround an issue where normal
    MIPI DSI signals are sometimes recognized as an MSC command that can,
    for example, disable the bridge output.
    
    Signed-off-by: Jitao Shi [off-list ref]
    Tested-by: Daniel Kurtz [off-list ref]
    Reviewed-by: Daniel Kurtz [off-list ref]
    
    
    CHROMIUM: drm/bridge: ps8640: Use individual regulators instead of
    bulk
    
    According to the latest information from Parade, the PS8640 1.2 V
    supply must be enabled before 3.3 V.
    So, split the bulk regulator into separate individual regulators that
    can be enabled independently.
    
    Signed-off-by: Jitao Shi [off-list ref]
    
    Tested-by: Daniel Kurtz [off-list ref]
    Reviewed-by: Daniel Kurtz [off-list ref]
    
    
    CHROMIUM: drm/bridge: ps8640: add 5ms delay between v12 and v33
    
    According to Parade, the PS8640 1.2V must be enabled 5 ms before its
    3.3V. Otherwise, the PS8650 MCU may hang, and its settings cannot be
    written correctly, leading to a black screen.
    
    Signed-off-by: Jitao Shi [off-list ref]
    Tested-by: Daniel Kurtz [off-list ref]
    Reviewed-by: Daniel Kurtz [off-list ref]

=============================

That keeps chronology and attribution intact, but is somewhat
redundant. Any suggestions on how to digest it? Or should I just add
it as is?

I would keep all the SoB lines, but apart from that I don't think there's a
need to copy all commit messages. You can extract relevant information for a
digest, but there's no point in copying the whole history.

My comment obviously referred only to patches that add a new driver. For 
fixes, you should indeed not squash them all but submit them as individual 
patches. Some of them can be squashed as needed, for instance a change that is 
later reverted can be omitted completely.

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel