Re: [RFC v2 0/8] Acer Chromebook R13 support

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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:
> > On Tue, Oct 17, 2017 at 6:05 PM, Sean Paul <seanpaul@chromium.org> wrote:
> > > On Tue, Oct 17, 2017 at 12:33:09PM +0200, Matthias Brugger wrote:
> > >> 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 <jitao.shi@mediatek.com>
> >     (am from https://patchwork.kernel.org/patch/8357851/)
> >     Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> >     Tested-by: cawa cheng <cawa.cheng@mediatek.com>
> >     Tested-by: Daniel Kurtz <djkurtz@chromium.org>
> >     Reviewed-by: Nicolas Boichat <drinkcat@chromium.org>
> >     
> >     
> >     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 <jitao.shi@mediatek.com>
> >     Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> >     Tested-by: PC Liao <pc.liao@mediatek.com>
> >     Tested-by: Daniel Kurtz <djkurtz@chromium.org>
> >     Tested-by: Nicolas Boichat <drinkcat@chromium.org>
> >     Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
> >     
> >     
> >     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 <jitao.shi@mediatek.com>
> >     Signed-off-by: Daniel Kurtz <djkurtz@chromium.org>
> >     
> >     Tested-by: cawa cheng <cawa.cheng@mediatek.com>
> >     Tested-by: Daniel Kurtz <djkurtz@chromium.org>
> >     Tested-by: jitao shi <jitao.shi@mediatek.com>
> >     Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
> >     Reviewed-by: jitao shi <jitao.shi@mediatek.com>
> >     
> >     
> >     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 <jitao.shi@mediatek.com>
> >     Tested-by: Daniel Kurtz <djkurtz@chromium.org>
> >     Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
> >     
> >     
> >     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 <jitao.shi@mediatek.com>
> >     
> >     Tested-by: Daniel Kurtz <djkurtz@chromium.org>
> >     Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
> >     
> >     
> >     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 <jitao.shi@mediatek.com>
> >     Tested-by: Daniel Kurtz <djkurtz@chromium.org>
> >     Reviewed-by: Daniel Kurtz <djkurtz@chromium.org>
> > 
> > =============================
> > 
> > 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