Message-ID: <5bb0459a-ec3a-487f-a9b5-28ee643a1215@ideasonboard.com>
Date: Wed, 28 Aug 2024 16:32:47 +0300
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Jan Kiszka <jan.kiszka@...mens.com>,
 Dmitry Baryshkov <dmitry.baryshkov@...aro.org>,
 Alexander Stein <alexander.stein@...tq-group.com>
Cc: 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 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.

  Tomi

Powered by blists - more mailing lists