On 12/05/2022 01:07, Rob Clark wrote:
On Wed, May 11, 2022 at 2:49 PM Dmitry Baryshkov dmitry.baryshkov@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@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@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.co...
[2] https://lore.kernel.org/all/CAPY8ntBrhYAmsraDqJGuTrSL6VjGXBAMVoN7xweV7E4qZv+...
-- With best wishes Dmitry