On 11/05/2022 23:06, Doug Anderson wrote:
Hi,
On Tue, Dec 7, 2021 at 2:29 PM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> 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@xxxxxxxxxx>
---
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@xxxxxxxxxxxxxxx
[2]
https://lore.kernel.org/all/CAPY8ntBrhYAmsraDqJGuTrSL6VjGXBAMVoN7xweV7E4qZv+v3Q@xxxxxxxxxxxxxx/
--
With best wishes
Dmitry
