| Message ID | 1434622239-15629-6-git-send-email-tomi.valkeinen@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tomi, Thank you for the patch. On Thursday 18 June 2015 13:10:39 Tomi Valkeinen wrote: > If tiler_unpin() call in omap_gem_put_paddr() fails, > omap_gem_put_paddr() will immediately stop processing and return an > error. > > This patch remoes that error checking, and also removes > omap_gem_put_paddr()'s return value, because: > > * The caller of omap_gem_put_paddr() can do nothing if an error > happens, so it's pointless to return an error value > > * If tiler_unpin() fails, the GEM object will possibly be left in an > undefined state, where the DMM mapping may have been removed, but the > GEM object still thinks everything is as it should be, leading to > crashes later. > > * There's no point in returning an error from a "free" call, as the > caller can do nothing about it. So it's better to clean up as much as > possible. > > 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_gem.c | 8 +++----- > 2 files changed, 4 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h > b/drivers/gpu/drm/omapdrm/omap_drv.h index 2ef89c0c3006..1ae8477e4289 > 100644 > --- a/drivers/gpu/drm/omapdrm/omap_drv.h > +++ b/drivers/gpu/drm/omapdrm/omap_drv.h > @@ -211,7 +211,7 @@ void omap_gem_dma_sync(struct drm_gem_object *obj, > enum dma_data_direction dir); > int omap_gem_get_paddr(struct drm_gem_object *obj, > dma_addr_t *paddr, bool remap); > -int omap_gem_put_paddr(struct drm_gem_object *obj); > +void omap_gem_put_paddr(struct drm_gem_object *obj); > int omap_gem_get_pages(struct drm_gem_object *obj, struct page ***pages, > bool remap); > int omap_gem_put_pages(struct drm_gem_object *obj); > diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c > b/drivers/gpu/drm/omapdrm/omap_gem.c index 51c5635aff62..d01d40245298 > 100644 > --- a/drivers/gpu/drm/omapdrm/omap_gem.c > +++ b/drivers/gpu/drm/omapdrm/omap_gem.c > @@ -808,10 +808,10 @@ fail: > /* Release physical address, when DMA is no longer being performed.. this > * could potentially unpin and unmap buffers from TILER > */ > -int omap_gem_put_paddr(struct drm_gem_object *obj) > +void omap_gem_put_paddr(struct drm_gem_object *obj) > { > struct omap_gem_object *omap_obj = to_omap_bo(obj); > - int ret = 0; > + int ret; > > mutex_lock(&obj->dev->struct_mutex); > if (omap_obj->paddr_cnt > 0) { > @@ -821,7 +821,6 @@ int omap_gem_put_paddr(struct drm_gem_object *obj) > if (ret) { > dev_err(obj->dev->dev, > "could not unpin pages: %d\n", ret); > - goto fail; > } > ret = tiler_release(omap_obj->block); > if (ret) { > @@ -832,9 +831,8 @@ int omap_gem_put_paddr(struct drm_gem_object *obj) > omap_obj->block = NULL; > } > } > -fail: > + > mutex_unlock(&obj->dev->struct_mutex); > - return ret; > } > > /* Get rotated scanout address (only valid if already pinned), at the
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h index 2ef89c0c3006..1ae8477e4289 100644 --- a/drivers/gpu/drm/omapdrm/omap_drv.h +++ b/drivers/gpu/drm/omapdrm/omap_drv.h @@ -211,7 +211,7 @@ void omap_gem_dma_sync(struct drm_gem_object *obj, enum dma_data_direction dir); int omap_gem_get_paddr(struct drm_gem_object *obj, dma_addr_t *paddr, bool remap); -int omap_gem_put_paddr(struct drm_gem_object *obj); +void omap_gem_put_paddr(struct drm_gem_object *obj); int omap_gem_get_pages(struct drm_gem_object *obj, struct page ***pages, bool remap); int omap_gem_put_pages(struct drm_gem_object *obj); diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c index 51c5635aff62..d01d40245298 100644 --- a/drivers/gpu/drm/omapdrm/omap_gem.c +++ b/drivers/gpu/drm/omapdrm/omap_gem.c @@ -808,10 +808,10 @@ fail: /* Release physical address, when DMA is no longer being performed.. this * could potentially unpin and unmap buffers from TILER */ -int omap_gem_put_paddr(struct drm_gem_object *obj) +void omap_gem_put_paddr(struct drm_gem_object *obj) { struct omap_gem_object *omap_obj = to_omap_bo(obj); - int ret = 0; + int ret; mutex_lock(&obj->dev->struct_mutex); if (omap_obj->paddr_cnt > 0) { @@ -821,7 +821,6 @@ int omap_gem_put_paddr(struct drm_gem_object *obj) if (ret) { dev_err(obj->dev->dev, "could not unpin pages: %d\n", ret); - goto fail; } ret = tiler_release(omap_obj->block); if (ret) { @@ -832,9 +831,8 @@ int omap_gem_put_paddr(struct drm_gem_object *obj) omap_obj->block = NULL; } } -fail: + mutex_unlock(&obj->dev->struct_mutex); - return ret; } /* Get rotated scanout address (only valid if already pinned), at the
If tiler_unpin() call in omap_gem_put_paddr() fails, omap_gem_put_paddr() will immediately stop processing and return an error. This patch remoes that error checking, and also removes omap_gem_put_paddr()'s return value, because: * The caller of omap_gem_put_paddr() can do nothing if an error happens, so it's pointless to return an error value * If tiler_unpin() fails, the GEM object will possibly be left in an undefined state, where the DMM mapping may have been removed, but the GEM object still thinks everything is as it should be, leading to crashes later. * There's no point in returning an error from a "free" call, as the caller can do nothing about it. So it's better to clean up as much as possible. Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com> --- drivers/gpu/drm/omapdrm/omap_drv.h | 2 +- drivers/gpu/drm/omapdrm/omap_gem.c | 8 +++----- 2 files changed, 4 insertions(+), 6 deletions(-)