Re: [Freedreno] [DPU PATCH v2 1/2] drm/msm/dsi: check video mode engine status before waiting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 2018-04-13 14:10, abhinavk@xxxxxxxxxxxxxx wrote:
Hi Sean

Thanks for the review.

Reply inline.

On 2018-04-13 13:26, Sean Paul wrote:
On Tue, Apr 10, 2018 at 06:54:06PM -0700, Abhinav Kumar wrote:
Make sure the video mode engine is on before waiting
for the video done interrupt.

Otherwise it leads to silent timeouts increasing display
turn ON time.

Changes in v2:
- Replace pr_err with dev_err
- Changed error message

Signed-off-by: Abhinav Kumar <abhinavk@xxxxxxxxxxxxxx>
---
 drivers/gpu/drm/msm/dsi/dsi_host.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/msm/dsi/dsi_host.c b/drivers/gpu/drm/msm/dsi/dsi_host.c
index 7a03a94..5b7b290 100644
--- a/drivers/gpu/drm/msm/dsi/dsi_host.c
+++ b/drivers/gpu/drm/msm/dsi/dsi_host.c
@@ -173,6 +173,7 @@ struct msm_dsi_host {

 	bool registered;
 	bool power_on;
+	bool enabled;
 	int irq;
 };

@@ -986,13 +987,19 @@ static void dsi_set_tx_power_mode(int mode, struct msm_dsi_host *msm_host)

 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);

-	wait_for_completion_timeout(&msm_host->video_comp,
+	ret = wait_for_completion_timeout(&msm_host->video_comp,
 			msecs_to_jiffies(70));

+	if (ret <= 0)
+		dev_err(dev, "wait for video done timed out\n");
+
 	dsi_intr_ctrl(msm_host, DSI_IRQ_MASK_VIDEO_DONE, 0);
 }

@@ -1001,7 +1008,7 @@ static void dsi_wait4video_eng_busy(struct msm_dsi_host *msm_host)
 	if (!(msm_host->mode_flags & MIPI_DSI_MODE_VIDEO))
 		return;

-	if (msm_host->power_on) {
+	if (msm_host->power_on && msm_host->enabled) {
 		dsi_wait4video_done(msm_host);
 		/* delay 4 ms to skip BLLP */
 		usleep_range(2000, 4000);
@@ -2203,7 +2210,7 @@ int msm_dsi_host_enable(struct mipi_dsi_host *host)
 	 *	pm_runtime_put_autosuspend(&msm_host->pdev->dev);
 	 * }
 	 */
-
+	msm_host->enabled = true;
 	return 0;
 }

@@ -2219,7 +2226,7 @@ int msm_dsi_host_disable(struct mipi_dsi_host *host)
 	 * Reset to disable video engine so that we can send off cmd.
 	 */
 	dsi_sw_reset(msm_host);
-
+	msm_host->enabled = false;

This should go at the start of the function. Also, it's unclear from this patch,
but I assume this is protected by a lock?

Sean
[Abhinav] Yes, will move this to the start.
No, there is no lock here but at this point doesnt need one.
The reason is that, this variable will be written to and read by the
same process
(suspend thread OR resume thread which sends the panel ON/OFF commands).
If we decide to expose other interfaces to send commands like debugfs
or sysfs and
introduce more threads, we will add the locking.
[Abhinav] Correction to my prev comment, we do have the msm_host->cmd_mutex which will
ensure this entire process is protected. That should suffice.


 	return 0;
 }

--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project

_______________________________________________
Freedreno mailing list
Freedreno@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/freedreno
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux