Thread (5 messages) 5 messages, 3 authors, 2016-08-23

Re: [PATCH v2] drm/rockchip: Don't continue trying to enable crtc on failure

From: Yakir Yang <hidden>
Date: 2016-08-17 07:28:45
Also in: dri-devel

Sean,

On 08/16/2016 07:12 AM, Sean Paul wrote:
If vop_enable fails, don't continue on, it causes system hangs.

Signed-off-by: Sean Paul <redacted>
Also meet this problem on my Rk3399 Kevin board. VOP just failed to get 
the pm_runtime at resume time, but driver still just continue without 
anything enable rightly, oops, and then system crashed :(

So this patch looks good to me, and also fix my problem, thanks:

Reviewed-by: Yakir Yang [off-list ref]
Tested-by: Yakir Yang <redacted>

Thanks,
- Yakir
quoted hunk ↗ jump to hunk
---

This patch uses the new DRM_DEV_ERROR logging, so it should be applied on
top of "[PATCH 2/2] drm/rockchip: Use DRM_DEV_ERROR in vop".

Changes in v2:
	- Escalate dev_err to WARN_ON for clk_enable failures (Daniel Vetter)

Sean


  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 31 ++++++++++++++++-------------
  1 file changed, 17 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index ec8ad00..a176d03 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
@@ -428,7 +428,7 @@ static void vop_dsp_hold_valid_irq_disable(struct vop *vop)
  	spin_unlock_irqrestore(&vop->irq_lock, flags);
  }
  
-static void vop_enable(struct drm_crtc *crtc)
+static int vop_enable(struct drm_crtc *crtc)
  {
  	struct vop *vop = to_vop(crtc);
  	int ret;
@@ -436,26 +436,20 @@ static void vop_enable(struct drm_crtc *crtc)
  	ret = pm_runtime_get_sync(vop->dev);
  	if (ret < 0) {
  		dev_err(vop->dev, "failed to get pm runtime: %d\n", ret);
-		return;
+		goto err_put_pm_runtime;
  	}
  
  	ret = clk_enable(vop->hclk);
-	if (ret < 0) {
-		dev_err(vop->dev, "failed to enable hclk - %d\n", ret);
-		return;
-	}
+	if (WARN_ON(ret < 0))
+		goto err_put_pm_runtime;
  
  	ret = clk_enable(vop->dclk);
-	if (ret < 0) {
-		dev_err(vop->dev, "failed to enable dclk - %d\n", ret);
+	if (WARN_ON(ret < 0))
  		goto err_disable_hclk;
-	}
  
  	ret = clk_enable(vop->aclk);
-	if (ret < 0) {
-		dev_err(vop->dev, "failed to enable aclk - %d\n", ret);
+	if (WARN_ON(ret < 0))
  		goto err_disable_dclk;
-	}
  
  	/*
  	 * Slave iommu shares power, irq and clock with vop.  It was associated
@@ -485,7 +479,7 @@ static void vop_enable(struct drm_crtc *crtc)
  
  	drm_crtc_vblank_on(crtc);
  
-	return;
+	return 0;
  
  err_disable_aclk:
  	clk_disable(vop->aclk);
@@ -493,6 +487,9 @@ err_disable_dclk:
  	clk_disable(vop->dclk);
  err_disable_hclk:
  	clk_disable(vop->hclk);
+err_put_pm_runtime:
+	pm_runtime_put_sync(vop->dev);
+	return ret;
  }
  
  static void vop_crtc_disable(struct drm_crtc *crtc)
@@ -912,10 +909,16 @@ static void vop_crtc_enable(struct drm_crtc *crtc)
  	u16 vact_st = adjusted_mode->vtotal - adjusted_mode->vsync_start;
  	u16 vact_end = vact_st + vdisplay;
  	uint32_t val;
+	int ret;
  
  	WARN_ON(vop->event);
  
-	vop_enable(crtc);
+	ret = vop_enable(crtc);
+	if (ret) {
+		DRM_DEV_ERROR(vop->dev, "Failed to enable vop (%d)\n", ret);
+		return;
+	}
+
  	/*
  	 * If dclk rate is zero, mean that scanout is stop,
  	 * we don't need wait any more.

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help