[PATCH 12/33] drm/omap: use dma_mapping_error in omap_gem_dma_sync

[Laurent Pinchart]

Hi Tomi,

On Thursday 25 February 2016 17:45:48 Tomi Valkeinen wrote:
> On 24/02/16 00:14, Laurent Pinchart wrote:
> > On Friday 19 February 2016 11:47:47 Tomi Valkeinen wrote:
> >> omap_gem_dma_sync() calls dma_map_page() but does not check the possible
> >> error with dma_mapping_error(). If DMA-API debugging is enabled, the
> >> debug layer will give a warning if dma_mapping_error() has not been
> >> used.
> >> 
> >> This patch adds proper error handling to omap_gem_dma_sync().
> >> 
> >> Signed-off-by: Tomi Valkeinen <tomi.valkeinen at ti.com>
> >> ---
> >> 
> >>  drivers/gpu/drm/omapdrm/omap_gem.c | 11 ++++++++++-
> >>  1 file changed, 10 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c
> >> b/drivers/gpu/drm/omapdrm/omap_gem.c index 7098190815f1..a60c30a59f7e
> >> 100644
> >> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> >> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> >> @@ -775,9 +775,18 @@ void omap_gem_dma_sync(struct drm_gem_object *obj,
> >> 
> >>  		for (i = 0; i < npages; i++) {
> >>  			if (!omap_obj->addrs[i]) {
> >> -				omap_obj->addrs[i] = dma_map_page(dev->dev, pages[i], 0,
> >> +				dma_addr_t addr;
> >> +
> >> +				addr = dma_map_page(dev->dev, pages[i], 0,
> >>  						PAGE_SIZE, DMA_BIDIRECTIONAL);
> >> +
> >> +				if (dma_mapping_error(dev->dev, addr)) {
> >> +					dev_warn(dev->dev, "omap_gem_dma_sync: dma_map_page
> > failed\n");
> > 
> > Same comment as for the previous patch.
> > 
> > No need to unmap ?
> 
> I don't know... Maybe, maybe not.
> 
> The function doesn't return any error, and the mapped pages are stored
> in omap_obj->addrs[]. So, if the failure was temporary, next time we
> have already mapped the pages that did succeed. If the failure was not
> temporary, well, we don't pass any error anyway, so the pages have to be
> unmapped somewhere in any case.
> 
> So possibly we could unmap, but I don't see us leaking anything even if
> the dma_map_page fails.

OK. It's not a new issue anyway, so I'm fine with the patch (after fixing the 
dev_warn()).

-- 
Regards,

Laurent Pinchart