Re: [PATCH 2/2] drm/bridge: ti-sn65dsi83: Fix problem with premature PLL locking

[Frieder Schrempf <frieder.schrempf@kontron.de>]

On 16.07.26 11:32, Alexander Stein wrote:
> Hi,
> 
> Am Mittwoch, 15. Juli 2026, 16:14:42 CEST schrieb Frieder Schrempf:
>> On 15.07.26 15:52, Gary Bisson wrote:
>>> Hi Esben and Luca,
>>>
>>> On Wed, Jul 15, 2026 at 10:52:18AM +0200, Luca Ceresoli wrote:
>>>> On Sat, 11 Jul 2026 13:51:15 +0200, Esben Haabendal <esben@geanix.com> wrote:
>>>>
>>>> Hi Esben,
>>>>
>>>> +Cc Gary
>>>>
>>>>> [...]
>>>>>
>>>>> This is the same issue as addressed in the patch by Gary Bisson [1],
>>>>> but changing the ti-sn65dsi83 driver instead, so we don't have to change
>>>>> all other drivers that could potentially be used with this chip.
>>>>>
>>>>> [1] https://lore.kernel.org/all/20260120-mtkdsi-v1-1-b0f4094f3ac3@gmail.com/
>>>>
>>>> AFAICU your patch would replace Gary's one. Also Gary's patch has been
>>>> reported to introduce regressions but it hasn't been reverted yet. Can you
>>>> reply to that thread mentioning your patch, so everybody in the discussion
>>>> is aware of your alternative proposal?
>>>
>>> Thanks for including me. I just tested this change and reverted my other
>>> one (mtk_dsi) and confirm that it works on my Tungsten510 + SN65DSI83 +
>>> tm070jdhg30 panel.
>>>
>>> Tested-by: Gary Bisson <bisson.gary@gmail.com>
>>>
>>> Note that the sn65dsi83 driver wasn't changed as I thought the PLL lock
>>> in pre-enable was on purpose. It was introduced by Frieder with this
>>> commit. Adding him to the thread to weigh in.
>>> dd9e329af723 drm/bridge: ti-sn65dsi83: Fix enable/disable flow to meet spec
>>
>> Thanks for the mention. This was introduced to keep the init order
>> according to the datasheet. The DSI host first needs to put the DSI
>> lanes into the correct state in its pre_enable(). Then the bridge needs
>> to be enabled (including the PLL) in the pre_enable() of the bridge
>> driver. Only after that the DSI host is allowed to stream data.
>>
>> Moving the PLL init from pre_enable() to enable() probably violates this
>> order. In the past this lead to sporadic issues with some hardware
>> setups (depending on the display and the DSI host). Some of this is also
>> described in the docs: [1]
>>
>> So from the first glance, I would assume this issue needs to be fixed in
>> the DSI host driver.
>>
>> [1]
>> https://docs.kernel.org/gpu/drm-kms-helpers.html#mipi-dsi-bridge-operation
> 
> This link says
>> Ordinarily the downstream bridge DSI peripheral pre_enable will have been
>> called before the DSI host. If the DSI peripheral requires LP-11 and/or the
>> clock lane to be in HS mode prior to pre_enable, then it can set the
>> pre_enable_prev_first flag to request the pre_enable (and post_disable)
>> order to be altered to enable the DSI host first.
> 
> So IIRC if pre_enable() already requires LP-11 on data and HS on clock,
> pre_enable_prev_first should be true then.
> 
Yes, and it is true:
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/bridge/ti-sn65dsi83.c?h=v7.2-rc3#n1051