[Freedreno] [PATCH 1/2] drm/msm/dsi: move DSI host powerup to modeset time

[Dmitry Baryshkov]

On 12/05/2022 01:07, Rob Clark wrote:
> On Wed, May 11, 2022 at 2:49 PM Dmitry Baryshkov
> <dmitry.baryshkov at linaro.org> wrote:
>>
>> On 11/05/2022 23:06, Doug Anderson wrote:
>>> Hi,
>>>
>>> On Tue, Dec 7, 2021 at 2:29 PM Dmitry Baryshkov
>>> <dmitry.baryshkov at linaro.org> wrote:
>>>>
>>>> The DSI subsystem does not fully fall into the pre-enable/enable system
>>>> of callbacks, since typically DSI device bridge drivers expect to be
>>>> able to communicate with DSI devices at the pre-enable() callback. The
>>>> reason is that for some DSI hosts enabling the video stream would
>>>> prevent other drivers from sending DSI commands. For example see the
>>>> panel-bridge driver, which does drm_panel_prepare() from the
>>>> pre_enable() callback (which would be called before our pre_enable()
>>>> callback, resulting in panel preparation failures as the link is not yet
>>>> ready).
>>>>
>>>> Therewere several attempts to solve this issue, but currently the best
>>>> approach is to power up the DSI link from the mode_set() callback,
>>>> allowing next bridge/panel to use DSI transfers in the pre_enable()
>>>> time. Follow this approach.
>>>>
>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
>>>> ---
>>>>    drivers/gpu/drm/msm/dsi/dsi_manager.c | 43 +++++++++++++++++++--------
>>>>    1 file changed, 31 insertions(+), 12 deletions(-)
>>>
>>> I happened to be testing today on one of the sc7180-trogdor variants
>>> that has a parade-ps8640 bridge chip in it and ran into problems. A
>>> bisect pointed to this patch and, sure enough, reverting it fixes me
>>> again.
>>>
>>> The Chromebook in question is able to power the screen on at bootup
>>> but things don't work so well in other cases. Specifically, if I leave
>>> the Chromebook idle then it will turn the screen off (but in this
>>> case, not enter S3). Hitting a key should wake the screen up, but it
>>> doesn't.
>>>
>>> None of the error prints in dsi_mgr_bridge_power_on() are hitting when
>>> it fails and I even added extra error prints. It's not hitting any of
>>> the early returns.
>>>
>>> I did a little bit more debugging and it appears that nothing funny is
>>> going on. It's just the ordering difference that matters. With the
>>> patch reverted, I see this and it all works:
>>>
>>> boot:
>>> [    9.653801] DOUG: dsi_mgr_bridge_mode_set
>>> [    9.658687] DOUG: ps8640_pre_enable
>>> [    9.664194] DOUG: dsi_mgr_bridge_pre_enable
>>>
>>> screen turns off:
>>> [   82.130038] DOUG: dsi_mgr_bridge_post_disable
>>> [   82.166817] DOUG: ps8640_post_disable
>>>
>>> screen turns on:
>>> [   92.611846] DOUG: dsi_mgr_bridge_mode_set
>>> [   92.617875] DOUG: ps8640_pre_enable
>>> [   92.920237] DOUG: dsi_mgr_bridge_pre_enable
>>>
>>> Without the patch reverted, I see this and it fails:
>>>
>>> boot:
>>> [   10.817810] DOUG: dsi_mgr_bridge_mode_set
>>> [   10.822128] DOUG: dsi_mgr_bridge_power_on
>>> [   10.852131] DOUG: ps8640_pre_enable
>>> [   10.857942] DOUG: dsi_mgr_bridge_pre_enable
>>>
>>> screen turns off:
>>> [   34.819953] DOUG: dsi_mgr_bridge_post_disable
>>> [   34.883777] DOUG: ps8640_post_disable
>>>
>>> screen should turn on, but doesn't:
>>> [   46.193589] DOUG: dsi_mgr_bridge_mode_set
>>> [   46.197951] DOUG: dsi_mgr_bridge_power_on
>>> [   46.248438] DOUG: ps8640_pre_enable
>>> [   46.541700] DOUG: dsi_mgr_bridge_pre_enable
>>>
>>> Unfortunately, ps8640 is a pretty big black box. The Linux kernel
>>> driver does almost nothing at all and the parade bridge chip has a
>>> bunch of magic firmware on it. Though I don't know for sure, I assume
>>> that this magic firmware simply can't handle the fact that the MIPI
>>> side is already setup / running when the bridge is powered on.
>>>
>>> Rather than this patch, maybe you can jump on Dave Stevenson's patch
>>> series [1] which I believe would solve this problem in a more dynamic
>>> way? Would you accept a revert of ${SUBJECT} patch to fix my problem?
>>
>> I'm fine with the revert, but it will depend on [1]. Otherwise other
>> MIPI DSI bridges are broken (see the discussion at [2]).
> 
> heh, well the problem is that MIPI DSI bridges, or at least one of
> them, is broken *without* the revert ;-)
> 
> [1] looks like a bit much for -fixes, so I'd be inclined to revert
> this patch and at least go back to the broken/working state from
> before for the time being..

So... We either get the ps8640 or tc358762 & the rest broken. I'd 
suggest a modparam.

> 
> BR,
> -R
> 
>>> [1] https://lore.kernel.org/r/cover.1646406653.git.dave.stevenson@raspberrypi.com
>>
>> [2]
>> https://lore.kernel.org/all/CAPY8ntBrhYAmsraDqJGuTrSL6VjGXBAMVoN7xweV7E4qZv+v3Q@mail.gmail.com/
>>
>>
>> --
>> With best wishes
>> Dmitry


-- 
With best wishes
Dmitry