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

[Dmitry Baryshkov]

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]).

> [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