Message-ID: <20230802191239.GA1142@pendragon.ideasonboard.com>
Date:   Wed, 2 Aug 2023 22:12:39 +0300
From:   Laurent Pinchart <laurent.pinchart@...asonboard.com>
To:     Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc:     Marek Vasut <marex@...x.de>, Amit Pundir <amit.pundir@...aro.org>,
        Neil Armstrong <neil.armstrong@...aro.org>,
        Robert Foss <rfoss@...nel.org>,
        Jonas Karlman <jonas@...boo.se>,
        dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
        Jernej Skrabec <jernej.skrabec@...il.com>,
        Abhinav Kumar <quic_abhinavk@...cinc.com>,
        Andrzej Hajda <andrzej.hajda@...el.com>,
        Jagan Teki <jagan@...rulasolutions.com>
Subject: Re: [PATCH] Revert "drm/bridge: lt9611: Do not generate HFP/HBP/HSA
 and EOT packet"

On Wed, Aug 02, 2023 at 09:49:42PM +0300, Dmitry Baryshkov wrote:
> On 02/08/2023 21:45, Marek Vasut wrote:
> > On 8/2/23 20:16, Dmitry Baryshkov wrote:
> >> On Wed, 2 Aug 2023 at 20:34, Marek Vasut wrote:
> >>> On 8/2/23 15:38, Dmitry Baryshkov wrote:
> >>>> On 02/08/2023 11:52, Neil Armstrong wrote:
> >>>>> This reverts commit [1] to fix display regression on the 
> >>>>> Dragonboard 845c
> >>>>> (SDM845) devboard.
> >>>>>
> >>>>> There's a mismatch on the real action of the following flags:
> >>>>> - MIPI_DSI_MODE_VIDEO_NO_HSA
> >>>>> - MIPI_DSI_MODE_VIDEO_NO_HFP
> >>>>> - MIPI_DSI_MODE_VIDEO_NO_HBP
> >>>>> which leads to a non-working display on qcom platforms.
> >>>>>
> >>>>> [1] 8ddce13ae696 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA and
> >>>>> EOT packet")
> >>>>>
> >>>>> Cc: Marek Vasut <marex@...x.de>
> >>>>> Cc: Robert Foss <rfoss@...nel.org>
> >>>>> Cc: Jagan Teki <jagan@...rulasolutions.com>
> >>>>> Cc: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
> >>>>> Cc: Abhinav Kumar <quic_abhinavk@...cinc.com>
> >>>>> Fixes: 8ddce13ae69 ("drm/bridge: lt9611: Do not generate HFP/HBP/HSA
> >>>>> and EOT packet")
> >>>>> Reported-by: Amit Pundir <amit.pundir@...aro.org>
> >>>>> Link:
> >>>>> https://lore.kernel.org/r/CAMi1Hd0TD=2z_=bcDrht3H_wiLvAFcv8Z-U_r_KUOoeMc6UMjw@mail.gmail.com/
> >>>>> Signed-off-by: Neil Armstrong <neil.armstrong@...aro.org>
> >>>>> ---
> >>>>>    drivers/gpu/drm/bridge/lontium-lt9611.c | 4 +---
> >>>>>    1 file changed, 1 insertion(+), 3 deletions(-)
> >>>>>
> >>>> Acked-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org> #fix db845c
> >>>>
> >>>> The boards broken by [1] are used in production by different parties
> >>>> since 5.10, breaking them doesn't seem more acceptable than breaking the
> >>>> new out-of-tree iMX8m hardware.
> >>>
> >>> The MX8M is also in-tree, so this does not apply.
> >>
> >> v6.5-rc4:
> >>
> >> $ git grep lontium,lt9611 | grep -v 9611uxc
> >> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:$id:
> >> http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
> >> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
> >>      - lontium,lt9611
> >> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
> >>        compatible = "lontium,lt9611";
> >> arch/arm64/boot/dts/qcom/sdm845-db845c.dts: compatible = "lontium,lt9611";
> >> drivers/gpu/drm/bridge/lontium-lt9611.c: { "lontium,lt9611", 0 },
> >> drivers/gpu/drm/bridge/lontium-lt9611.c: { .compatible = "lontium,lt9611" },
> >>
> >> next-20230802:
> >>
> >> $ git grep lontium,lt9611 | grep -v 9611uxc
> >> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:$id:
> >> http://devicetree.org/schemas/display/bridge/lontium,lt9611.yaml#
> >> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
> >>      - lontium,lt9611
> >> Documentation/devicetree/bindings/display/bridge/lontium,lt9611.yaml:
> >>        compatible = "lontium,lt9611";
> >> arch/arm64/boot/dts/qcom/sdm845-db845c.dts: compatible = "lontium,lt9611";
> >> drivers/gpu/drm/bridge/lontium-lt9611.c: { "lontium,lt9611", 0 },
> >> drivers/gpu/drm/bridge/lontium-lt9611.c: { .compatible = "lontium,lt9611" },
> >>
> >> Your device is not in the tree. Your commit broke existing users.
> > 
> > These devices are in tree:
> > arch/arm64/boot/dts/freescale/imx8mm-data-modul-edm-sbc.dts
> > arch/arm64/boot/dts/freescale/imx8mp-data-modul-edm-sbc.dts
> > 
> > The LT9211 and LT9611 are both expansion modules handled by DTOs and 
> > bound to the DSIM (which is also in tree).
> 
> And they DT for them is not in the tree, that was my point. You have 
> broken the existing user for the DTBO that is not present even in 
> linux-next.
> 
> >> Can we please end the argument, land the fix (this revert) for 6.5 and
> >> work on the solution for 6.6 or 6.7?
> > 
> > I would much prefer a solution which does not break my existing use 
> > case. It is still not even clear whether the problem really is on MX8M 
> > side at all, or whether it is QCOM misinterpreting flags. I cannot debug 
> > the later, since I have no access to that platform, nor its documentation.
> 
> You can get the RB1 for $199 and check the DSI behaviour on that 
> platform. It has newer bridge, but the DSI controller is (mostly) the same.

Could everybody please get away from the keyboard for a few hours, take
a deep breath, and resume the discussion in a less aggressive and more
constructive way ?

Without judging the technical merits of the arguments, and which
platform gets it wrong, the commit being reverted landed in v6.5-rc1,
and breaks in-tree users. Reverting and retrying thus seems the usual
practice to me, as we are getting too close to the v6.5 release to
ensure a correct fix can be developed and merged in time. This will not
cause a regression on i.MX8M, as the commit has never appeared in a
release kernel.

This is however an unfortunate event. It is not a nice feeling to work
on enabling features for a platform and see the work being reverted at
the last minute. Neil, Dmitry, could you please help Marek figuring out
a good solution for v6.6 ? I don't think it's reasonable to ask him to
buy an RB1 and investigate the MSM side, when Linaro has access to
hardware and support.

-- 
Regards,

Laurent Pinchart

Powered by blists - more mailing lists