drm/rockchip: Don't continue trying to enable crtc on failure
diff mbox

Message ID 1471036181-14777-1-git-send-email-seanpaul@chromium.org
State New
Headers show

Commit Message

Sean Paul Aug. 12, 2016, 9:09 p.m. UTC
If vop_enable fails, don't continue on, it causes system hangs.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---

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".

Sean


 drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 19 ++++++++++++++-----
 1 file changed, 14 insertions(+), 5 deletions(-)

Comments

Daniel Vetter Aug. 14, 2016, 9:42 a.m. UTC | #1
On Fri, Aug 12, 2016 at 05:09:41PM -0400, Sean Paul wrote:
> If vop_enable fails, don't continue on, it causes system hangs.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>

Hm, I thought there's clk_prepare which then makes sure that clk_enable
actually works? In that case I'd go with a WARN_ON or similar bad levels
of noise.

If this can indeed happen then ugh, someone needs to make the clk
framework more atomic ;-)
-Daniel

> ---
> 
> 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".
> 
> Sean
> 
> 
>  drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 19 ++++++++++++++-----
>  1 file changed, 14 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
> index ec8ad00..d0ffe59 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,13 +436,13 @@ 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;
> +		goto err_put_pm_runtime;
>  	}
>  
>  	ret = clk_enable(vop->dclk);
> @@ -485,7 +485,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 +493,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 +915,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.
> -- 
> 2.8.0.rc3.226.g39d4020
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Patch
diff mbox

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c
index ec8ad00..d0ffe59 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,13 +436,13 @@  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;
+		goto err_put_pm_runtime;
 	}
 
 	ret = clk_enable(vop->dclk);
@@ -485,7 +485,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 +493,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 +915,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.