Re: [PATCH] drm/mediatek: mtk_dsi: enable hs clock during pre-enable

On 7/27/26 10:54, AngeloGioacchino Del Regno wrote:
> On 7/27/26 16:23, Thorsten Leemhuis wrote:
>> On 7/27/26 14:48, Gary Bisson wrote:
>>> On Mon, Jul 27, 2026 at 02:28:56PM +0200, Thorsten Leemhuis wrote:
>>>> [top-posting to facilitate]
>>>>
>>>> Gary, Angelo, what's the status here? It looks like this fell through
>>>> the cracks -- or was this issue fixed in between somehow? If yes: 
>>>> great!
>>>> If not: Would be good to finally resolve this, as we are long past "fix
>>>> within a week" rule of thumb from Linus:
>>>> https://www.kernel.org/doc/html/latest/process/handling- 
>>>> regressions.html#on-how-quickly-regressions-should-be-fixed
>>>
>>> Not much happened I'm afraid. The status as I see is:
>>> - this patch is necessary to have TI SN65DSI83 working
>>> - this patch also follows the DRM guidelines saying that HS clock must
>>>    be enabled during pre_enable (see previous answer / [1])
>>> - this patch has been successfully tested against MIPI-DSI panels (by
>>>    Angelo) and LVDS panels via TI bridge (by myself)
>>> - Adam reported an issue with another bridge (PS8640) where resume is
>>>    broken
>>> - Esben offered a patch to the TI bridge that would fix the issue we
>>>    were seeing in the first place.
>>>
>>> But for the last two points, I'm not sure this calls for a revert yet:
>>> - enabling HS clock in pre_enable still is what should be done [1]
>>> - maybe the issue Adam is facing is due to the bridge driver instead as
>>>    it could not be reproduced with another setup
>>> - Esben patch would break the SN65DSI83 init sequence, suggesting that
>>>    the culprit really is the MIPI bridge for not following the HS clock
>>>    requirement in pre_enable instead [2]
>> It's tricky, yes, but I suspect the right time for a revert was weeks
>> ago. What Linus afaics basically wants in situations like this (espe.
>> with two reporters) boils down to "something broke recently, no fix was
>> found within a week or maybe two, so we go back to the previous state to
>> restart from there -- and this is nothing bad, that's just done to buy
>> us time."
>>
>> If something is right by some hardware or DRM specs often doesn't matter
>> much. What makes this tricky is the fact that the revert might cause a
>> regression for those that relied on the functionally the change brought.
>> But I suspect even then Linus prefers a revert, as it's a recent change
>> that only went into 7.1. The quoted from Linus on this page cover this 
>> iirc:
>> https://www.kernel.org/doc/html/latest/process/handling- 
>> regressions.html#quotes-from-linus-about-regression
>>
>> If we can't agree on this, we maybe should ask Simona or Airlied for 
>> advice.
>>
>> Ciao, Thorsten
> 
> Thorsten,
> I agree with you, and it's fine for me to get this commit reverted, for 
> the sake
> of following the rule (which is something I agree about: as a user, the 
> last thing
> that should happen is to be afraid of what breaks if I update my 
> system!!!!),
> even though reverting this actually breaks anything that is not PS8640.
> 
> ......though......
> 
> Everyone, please, look at how fundamentally broken PS8640 is... Qualcomm 
> people had
> the same issue as what is currently happening here.
> 
> They had to fix their DSI driver, as it was kind-of broken in a vagualy 
> similar way
> compared to MediaTek - and then they found out that they needed (and 
> upstreamed) a
> *hack* that had to be done specifically for the PS8640 Bridge [1]: their 
> issue was
> never really resolved, as then the same bridge started casually working 
> for unknown
> reasons (is it still working?), so they removed the hack [2] and called 
> it a day.
> 
> [1]: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux- 
> next.git/commit/drivers/gpu/drm/msm/dsi/dsi_manager.c? 
> h=next-20260726&id=ec7981e6c614254937b37ce0af9eac09901c05c5
> [2]: https://git.kernel.org/pub/scm/linux/kernel/git/next/linux- 
> next.git/commit/drivers/gpu/drm/msm/dsi/dsi_manager.c? 
> h=next-20260726&id=9e15123eca7942caa8a3e1f58ec0df7d088df149
> 
> Especially in [2], the assmption from Dmitry sadly isn't correct, 
> because this is
> happening now on MediaTek, and here Runtime PM makes sure that the 
> PS8640 is also
> being enabled "a wee bit" earlier than DSI... but that doesn't seem to 
> be enough.
> 
> 
> Now, here I'm kind of shotgunning at the driver, but I wonder if anyone 
> can try
> the following patch before reverting this commit in mtk_dsi? Adam?
> 
> ---
>   drivers/gpu/drm/bridge/parade-ps8640.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/bridge/parade-ps8640.c b/drivers/gpu/drm/ 
> bridge/parade-ps8640.c
> index 96332721cb69..54ebf2fc8d1c 100644
> --- a/drivers/gpu/drm/bridge/parade-ps8640.c
> +++ b/drivers/gpu/drm/bridge/parade-ps8640.c
> @@ -665,6 +665,7 @@ static int ps8640_probe(struct i2c_client *client)
> 
>       ps_bridge->bridge.of_node = dev->of_node;
>       ps_bridge->bridge.type = DRM_MODE_CONNECTOR_eDP;
> +    ps_bridge->bridge.pre_enable_prev_first = true;
> 
>       /*
>        * Get MIPI DSI resources early. These can return -EPROBE_DEFER so
This does not fix the issue. I built 7.1.5 with this patch applied, and 
without the revert.