diff mbox

[4/5] drm/omap: fix omap_framebuffer_unpin() error handling

Message ID 1434622239-15629-5-git-send-email-tomi.valkeinen@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Tomi Valkeinen June 18, 2015, 10:10 a.m. UTC
omap_framebuffer_unpin() check the return value of omap_gem_put_paddr()
and return immediately if omap_gem_put_paddr() fails.

This patch removes the check for the return value, and also removes the
return value of omap_framebuffer_unpin(), because:

 * Nothing checks the return value of omap_framebuffer_unpin(), and even
   something did check it, there's nothing the caller can do to handle
   the error.

 * If a omap_gem_put_paddr() fails, the framebuffer's other planes will
   be left unreleased. So it's better to call omap_gem_put_paddr() for
   all the planes, even if one would fail.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.h |  2 +-
 drivers/gpu/drm/omapdrm/omap_fb.c  | 16 ++++------------
 2 files changed, 5 insertions(+), 13 deletions(-)

Comments

Laurent Pinchart June 18, 2015, 3:13 p.m. UTC | #1
Hi Tomi,

Thank you for the patch.

On Thursday 18 June 2015 13:10:38 Tomi Valkeinen wrote:
> omap_framebuffer_unpin() check the return value of omap_gem_put_paddr()
> and return immediately if omap_gem_put_paddr() fails.
> 
> This patch removes the check for the return value, and also removes the
> return value of omap_framebuffer_unpin(), because:
> 
>  * Nothing checks the return value of omap_framebuffer_unpin(), and even
>    something did check it, there's nothing the caller can do to handle
>    the error.
> 
>  * If a omap_gem_put_paddr() fails, the framebuffer's other planes will
>    be left unreleased. So it's better to call omap_gem_put_paddr() for
>    all the planes, even if one would fail.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> ---
>  drivers/gpu/drm/omapdrm/omap_drv.h |  2 +-
>  drivers/gpu/drm/omapdrm/omap_fb.c  | 16 ++++------------
>  2 files changed, 5 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h
> b/drivers/gpu/drm/omapdrm/omap_drv.h index ae2df41f216f..2ef89c0c3006
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_drv.h
> +++ b/drivers/gpu/drm/omapdrm/omap_drv.h
> @@ -177,7 +177,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct
> drm_device *dev, struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object
> **bos); struct drm_gem_object *omap_framebuffer_bo(struct drm_framebuffer
> *fb, int p); int omap_framebuffer_pin(struct drm_framebuffer *fb);
> -int omap_framebuffer_unpin(struct drm_framebuffer *fb);
> +void omap_framebuffer_unpin(struct drm_framebuffer *fb);
>  void omap_framebuffer_update_scanout(struct drm_framebuffer *fb,
>  		struct omap_drm_window *win, struct omap_overlay_info *info);
>  struct drm_connector *omap_framebuffer_get_next_connector(
> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c
> b/drivers/gpu/drm/omapdrm/omap_fb.c index 0b967e76df1a..51b1219af87f 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fb.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
> @@ -287,10 +287,10 @@ fail:
>  }
> 
>  /* unpin, no longer being scanned out: */
> -int omap_framebuffer_unpin(struct drm_framebuffer *fb)
> +void omap_framebuffer_unpin(struct drm_framebuffer *fb)
>  {
>  	struct omap_framebuffer *omap_fb = to_omap_framebuffer(fb);
> -	int ret, i, n = drm_format_num_planes(fb->pixel_format);
> +	int i, n = drm_format_num_planes(fb->pixel_format);
> 
>  	mutex_lock(&omap_fb->lock);
> 
> @@ -298,24 +298,16 @@ int omap_framebuffer_unpin(struct drm_framebuffer *fb)
> 
>  	if (omap_fb->pin_count > 0) {
>  		mutex_unlock(&omap_fb->lock);
> -		return 0;
> +		return;
>  	}
> 
>  	for (i = 0; i < n; i++) {
>  		struct plane *plane = &omap_fb->planes[i];
> -		ret = omap_gem_put_paddr(plane->bo);
> -		if (ret)
> -			goto fail;
> +		omap_gem_put_paddr(plane->bo);
>  		plane->paddr = 0;
>  	}
> 
>  	mutex_unlock(&omap_fb->lock);
> -
> -	return 0;
> -
> -fail:
> -	mutex_unlock(&omap_fb->lock);
> -	return ret;
>  }
> 
>  struct drm_gem_object *omap_framebuffer_bo(struct drm_framebuffer *fb, int
> p)
diff mbox

Patch

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index ae2df41f216f..2ef89c0c3006 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -177,7 +177,7 @@  struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
 		struct drm_mode_fb_cmd2 *mode_cmd, struct drm_gem_object **bos);
 struct drm_gem_object *omap_framebuffer_bo(struct drm_framebuffer *fb, int p);
 int omap_framebuffer_pin(struct drm_framebuffer *fb);
-int omap_framebuffer_unpin(struct drm_framebuffer *fb);
+void omap_framebuffer_unpin(struct drm_framebuffer *fb);
 void omap_framebuffer_update_scanout(struct drm_framebuffer *fb,
 		struct omap_drm_window *win, struct omap_overlay_info *info);
 struct drm_connector *omap_framebuffer_get_next_connector(
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
index 0b967e76df1a..51b1219af87f 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -287,10 +287,10 @@  fail:
 }
 
 /* unpin, no longer being scanned out: */
-int omap_framebuffer_unpin(struct drm_framebuffer *fb)
+void omap_framebuffer_unpin(struct drm_framebuffer *fb)
 {
 	struct omap_framebuffer *omap_fb = to_omap_framebuffer(fb);
-	int ret, i, n = drm_format_num_planes(fb->pixel_format);
+	int i, n = drm_format_num_planes(fb->pixel_format);
 
 	mutex_lock(&omap_fb->lock);
 
@@ -298,24 +298,16 @@  int omap_framebuffer_unpin(struct drm_framebuffer *fb)
 
 	if (omap_fb->pin_count > 0) {
 		mutex_unlock(&omap_fb->lock);
-		return 0;
+		return;
 	}
 
 	for (i = 0; i < n; i++) {
 		struct plane *plane = &omap_fb->planes[i];
-		ret = omap_gem_put_paddr(plane->bo);
-		if (ret)
-			goto fail;
+		omap_gem_put_paddr(plane->bo);
 		plane->paddr = 0;
 	}
 
 	mutex_unlock(&omap_fb->lock);
-
-	return 0;
-
-fail:
-	mutex_unlock(&omap_fb->lock);
-	return ret;
 }
 
 struct drm_gem_object *omap_framebuffer_bo(struct drm_framebuffer *fb, int p)