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

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

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)

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel