Message-ID: <lxcxo3y7z6emrpvisibj6ccr6qx5nozchuyy5aizfvkvvlp44m@jecj6k5y6z2t>
Date: Mon, 23 Sep 2024 15:25:59 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
To: Jan Kiszka <jan.kiszka@...mens.com>
Cc: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, 
	Alexander Stein <alexander.stein@...tq-group.com>, Aradhya Bhatia <a-bhatia1@...com>, 
	dri-devel@...ts.freedesktop.org, Marek Vasut <marex@...x.de>, 
	Andrzej Hajda <andrzej.hajda@...el.com>, Neil Armstrong <neil.armstrong@...aro.org>, 
	Robert Foss <rfoss@...nel.org>, Laurent Pinchart <Laurent.pinchart@...asonboard.com>, 
	Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>, 
	Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, Maxime Ripard <mripard@...nel.org>, 
	Thomas Zimmermann <tzimmermann@...e.de>, David Airlie <airlied@...il.com>, 
	Daniel Vetter <daniel@...ll.ch>, Sam Ravnborg <sam@...nborg.org>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 0/2] drm/bridge: tc358767: Fix
 DRM_BRIDGE_ATTACH_NO_CONNECTOR case

On Mon, Sep 23, 2024 at 09:42:27AM GMT, Jan Kiszka wrote:
> On 28.08.24 15:32, Tomi Valkeinen wrote:
> > On 26/08/2024 22:35, Jan Kiszka wrote:
> >> On 24.06.24 12:17, Dmitry Baryshkov wrote:
> >>> On Mon, 24 Jun 2024 at 13:07, Alexander Stein
> >>> <alexander.stein@...tq-group.com> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> Am Montag, 24. Juni 2024, 11:49:04 CEST schrieb Dmitry Baryshkov:
> >>>>> On Mon, Jun 24, 2024 at 03:07:10PM GMT, Aradhya Bhatia wrote:
> >>>>>>
> >>>>>>
> >>>>>> On 22/06/24 17:49, Dmitry Baryshkov wrote:
> >>>>>>> On Sat, Jun 22, 2024 at 05:16:58PM GMT, Aradhya Bhatia wrote:
> >>>>>>>>
> >>>>>>>>
> >>>>>>>> On 17-Jun-24 13:41, Dmitry Baryshkov wrote:
> >>>>>>>>> On Mon, Jun 17, 2024 at 07:40:32AM GMT, Jan Kiszka wrote:
> >>>>>>>>>> On 16.02.24 15:57, Marek Vasut wrote:
> >>>>>>>>>>> On 2/16/24 10:10, Tomi Valkeinen wrote:
> >>>>>>>>>>>> Ok. Does anyone have a worry that these patches make the
> >>>>>>>>>>>> situation
> >>>>>>>>>>>> worse for the DSI case than it was before? Afaics, if the
> >>>>>>>>>>>> DSI lanes
> >>>>>>>>>>>> are not set up early enough by the DSI host, the driver
> >>>>>>>>>>>> would break
> >>>>>>>>>>>> with and without these patches.
> >>>>>>>>>>>>
> >>>>>>>>>>>> These do fix the driver for DRM_BRIDGE_ATTACH_NO_CONNECTOR
> >>>>>>>>>>>> and DPI, so
> >>>>>>>>>>>> I'd like to merge these unless these cause a regression with
> >>>>>>>>>>>> the DSI
> >>>>>>>>>>>> case.
> >>>>>>>>>>>
> >>>>>>>>>>> 1/2 looks good to me, go ahead and apply .
> >>>>>>>>
> >>>>>>>> Isn't there any way for the second patch to move forward as well
> >>>>>>>> though?
> >>>>>>>> The bridge device (under DPI to (e)DP mode) cannot really work
> >>>>>>>> without
> >>>>>>>> it, and the patches have been pending idle for a long time. =)
> >>>>>>>>
> >>>>>>>>>>
> >>>>>>>>>> My local patches still apply on top of 6.10-rc4, so I don't
> >>>>>>>>>> think this
> >>>>>>>>>> ever happened. What's still holding up this long-pending fix
> >>>>>>>>>> (at least
> >>>>>>>>>> for our devices)?
> >>>>>>>>>
> >>>>>>>>> Neither of the patches contains Fixes tags. If the first patch
> >>>>>>>>> fixes an
> >>>>>>>>> issue in previous kernels, please consider following the stable
> >>>>>>>>> process.
> >>>>>>>>>
> >>>>>>>>> If we are unsure about the second patch, please send the first
> >>>>>>>>> patch
> >>>>>>>>> separately, adding proper tags.
> >>>>>>>>>
> >>>>>>>>
> >>>>>>>> Thanks Dmitry! I can send the patches again with the required fixes
> >>>>>>>> tags (or just patch-1 if we cannot do anything about patch-2).
> >>>>>>>
> >>>>>>> The problem with the second patch is that it get mixed reviews. I
> >>>>>>> can
> >>>>>>> ack the first patch, but for the second one I'd need a
> >>>>>>> confirmation from
> >>>>>>> somebody else. I'll go on and apply the first patch later today.
> >>>>>>>
> >>>>>>
> >>>>>> Thanks Dmitry!
> >>>>>>
> >>>>>> However, would it be okay if I instead add another patch that makes 2
> >>>>>> versions of the "tc_edp_bridge_funcs", say
> >>>>>> "tc_dpi_edp_bridge_funcs" and
> >>>>>> "tc_dsi_edp_bridge_funcs", that have all the same function hooks
> >>>>>> except
> >>>>>> for the .edid_read()?
> >>>>>>
> >>>>>> The dsi edid_read() will remain the same, and Tomi's patch - patch
> >>>>>> 2/2 -
> >>>>>> will only fix the dpi version of the edid_read()?
> >>>>>>
> >>>>>> The bridge already has the capability to distinguish a DSI input
> >>>>>> from a
> >>>>>> DPI input. This can be leveraged to decide which set of functions
> >>>>>> need
> >>>>>> to be used without any major changes.
> >>>>>
> >>>>> I'd prefer if somebody with the DSI setup can ack / test the second
> >>>>> patch.
> >>>>>
> >>>>>
> >>>>
> >>>> Now that my DSI-DP setup works apparently without issue I could test
> >>>> patch 2.
> >>>> Since my setup does not use DRM_BRIDGE_ATTACH_NO_CONNECTOR (running on
> >>>> drivers/gpu/drm/mxsfb/lcdif_drv.c), I can only say
> >>>> there is no regression.
> >>>
> >>> Let me send a (non-tested) patch, switching to drm_bridge_connector,
> >>> then you can probably test both of them
> >>>
> >>
> >> I suppose [1] was that follow-up, just not leading to success, right?
> >>
> >> Now, what's next? I'd love to see the regression we have for the IOT2050
> >> devices finally fixed, even if it now only requires a short downstream
> >> patch.
> >>
> >> Jan
> >>
> >> [1]
> >> https://lore.kernel.org/dri-devel/20240624-mxc-lcdif-bridge-attach-v1-1-37e8c5d5d934@linaro.org/
> > 
> > I have to say I don't remember the details for this anymore, but half a
> > year ago I said:
> > 
> >> Afaics, if the DSI lanes are not set up early enough by the DSI host,
> >> the driver would break with and without these patches.
> > 
> > I'm not sure if that is correct or not. But if it is, then, afaiu, this
> > (the second patch):
> > 
> > - Fixes the issue for the DPI-DP use case
> > 
> > - Doesn't cause issues for the DSI-DP use case without
> > DRM_BRIDGE_ATTACH_NO_CONNECTOR (as per Alexander's test)
> > 
> > - Shouldn't cause (new) issues for the DSI-DP use case with
> > DRM_BRIDGE_ATTACH_NO_CONNECTOR (as per my code review and guessing...)
> > 
> > The third point is somewhat concerning, of course, but do we have any
> > setup with DSI-DP and DRM_BRIDGE_ATTACH_NO_CONNECTOR that works now? If
> > not, maybe we can just ignore the possible issues, as this fixes
> > problems on a setup we do have.
> > 
> 
> As Dmitry asked me during Plumbers to revalidate if our setup still
> needs patch 2, I just did that over 6.11.0-next-20240923 (where patch 1
> is now included). No surprise, it is still needed for our iot2050 device
> series, otherwise the display remains black.

Granted that nobody with the DRM_BRIDGE_ATTACH_NO_CONNECTOR + DSI-DP
spoke in the last several months, I think we'd better merge the patch as
it is now. If noone objects (last call), I'll do that in one or two
days.

-- 
With best wishes
Dmitry

Powered by blists - more mailing lists