Panel drivers unable to send DSI commands in .unprepare callback

[minlexx]

When you turn display off, you see this in dmesg:

[  154.587399] panel-tianma-nt36672a c994000.dsi.0: failed to send DCS off cmds: -110

or similar:

[  121.295290] panel-jdi-fhd-r63452 994000.dsi.0: transmit data failed: -22
[  121.295368] panel-jdi-fhd-r63452 994000.dsi.0: Failed to un-initialize panel: -22

So panel's .unprepare callback fails, panel is still marked as perpared.

Then during following turning display on, you see:

[  159.940071] panel-tianma-nt36672a c994000.dsi.0: Skipping prepare of already prepared panel

which breaks e.g. touchscreen in panel follower mode.

---

🌀 There exists some solution in sdm845-mainline kernel:

• drm/panel: move some dsi commands from unprepare to disable

But this is not very acceptable upstream, Dmitry didn't like it: https://lore.kernel.org/all/kjlcqkfipct4d54hrtmwdsrifxvuq3qocv5bcmwsufrbxtvvq2@52grw5ijqx4k/

I don't think this is correct. If the driver sends enable commands in
prepare, it should be able to send commands during unprepare too.
... this flag ctx->panel.prepare_prev_first = true;
The flag should also invert the order of post_disable chain. It well
might be that the drm/msm/dsi driver shuts down the DSI link too soon.
Please consider fixing the MSM DSI driver instead.

---

🌀 The solution in msm8916-mainline kernel is 2 commits:

• drm/msm/dsi: split bridge pre_enable/post_disable
• HACK: drm/msm: Run mdp5 encoder enable()/disable() after bridge hooks

comment from minecrell:

I think my first commit is fine / „correct“, so you can first try only that one
And see if you get the errors mentioned in the second one. The second one needs a cleaner solution
I think the first commit does that (fixes the msm dsi driver).
If I hadn’t run into the mess with the second commit, I would have submitted it :/

Need to figure out which solution is "better" / upstreamable 🤔

[minlexx]

🌀 3rd solution from Vladimir Lypak

• drm/msm/dsi: improve power management

[minlexx]

I have tested the solution by @stephan-gh from msm8916-mainline/linux kernel, using only single commit:

• drm/msm/dsi: split bridge pre_enable/post_disable

made it possible to get rid of errors from panel, at least using dpu.

mdp5 still shows

[drm:mdp5_irq_error_handler [msm]] *ERROR* errors: 04000000

when powering panel on, but those are "normal" for mdp5, and there is nothing like spam of dsi_err_worker: described in 2nd HACK commit, so only 1st commit works well with mdp5 also

[minlexx]

I've seen few errors like dsi_err_worker: on clover few times, randomly on system startup, they disappear completely after you power the screen off/on at least once

Maybe I'll pick 2nd commit too 🤔

[minlexx]

Errors looks like

[   39.580109] [drm:dpu_encoder_phys_vid_wait_for_commit_done:537] [dpu error]vblank timeout: 40020041
[   39.580153] [drm:dpu_kms_wait_for_commit_done:524] [dpu error]wait for commit done returned -110
[   39.652092] [drm:dpu_encoder_phys_vid_wait_for_commit_done:537] [dpu error]vblank timeout: 40020041
[   39.652114] [drm:dpu_kms_wait_for_commit_done:524] [dpu error]wait for commit done returned -110
[   39.672033] [drm:dpu_encoder_frame_done_timeout:2708] [dpu error]enc32 frame done timeout

and it only happens with DPU, not with mdp5 so far

[minlexx]

2nd commit doesn't help, even after adding the same

+.late_enable = true,

into dpu_encoder too.
( 8ad7081#diff-e49105c9b9d60ac57747380b5c9eb26f364f6dcb45db1cb9d920a0775b8902b9R2722 )

Still same issues when using DPU on whyred and clover.
Still no issues with mdp5..

[minlexx]

🌀 another workaround from msm8953 kernel? msm8953-mainline@9c5562c

[barni2000]

2nd commit doesn't help, even after adding the same

+.late_enable = true,

into dpu_encoder too. ( 8ad7081#diff-e49105c9b9d60ac57747380b5c9eb26f364f6dcb45db1cb9d920a0775b8902b9R2722 )

Still same issues when using DPU on whyred and clover. Still no issues with mdp5..

Strange, 2 commits together was worked on 8953.

[barni2000]

Maybe close to a proper fix, dumped from msm8953 chat made by vladly:

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 4d75529c0e85..c6bd81542ed5 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -1048,7 +1048,7 @@ static void dsi_sw_reset(struct msm_dsi_host *msm_host)
 
 	/* dsi controller can only be reset while clocks are running */
 	dsi_write(msm_host, REG_DSI_RESET, 1);
-	msleep(DSI_RESET_TOGGLE_DELAY_MS); /* make sure reset happen */
+	udelay(5); /* make sure reset happen */
 	dsi_write(msm_host, REG_DSI_RESET, 0);
 	wmb(); /* controller out of reset */
 
@@ -1102,17 +1102,19 @@ static void dsi_wait4video_done(struct msm_dsi_host *msm_host)
 	u32 ret = 0;
 	struct device *dev = &msm_host->pdev->dev;
 
-	dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 1);
-
 	reinit_completion(&msm_host->video_comp);
 
-	ret = wait_for_completion_timeout(&msm_host->video_comp,
-			msecs_to_jiffies(70));
+	dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 1);
 
-	if (ret == 0)
-		DRM_DEV_ERROR(dev, "wait for video done timed out\n");
+	ret = wait_for_completion_timeout(&msm_host->video_comp,
+			msecs_to_jiffies(20));
 
 	dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 0);
+
+	if (ret == 0) {
+		DRM_DEV_DEBUG(dev, "wait for video done timed out, resetting dsi\n");
+		dsi_sw_reset(msm_host);
+	}
 }
 
 static void dsi_wait4video_eng_busy(struct msm_dsi_host *msm_host)

[minlexx]

This, I guess? msm8953-mainline@2c1b02a

The one with proper commit message