Re: [PATCH 1/5] drm/omap: return error if dma_alloc_writecombine fails

From: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
To: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: dri-devel@lists.freedesktop.org
Subject: Re: [PATCH 1/5] drm/omap: return error if dma_alloc_writecombine fails
Date: Thu, 18 Jun 2015 17:27:59 +0300	[thread overview]
Message-ID: <4966397.anhbQhtP9U@avalon> (raw)
In-Reply-To: <1434622239-15629-2-git-send-email-tomi.valkeinen@ti.com>

Hi Tomi,

Thank you for the patch.

On Thursday 18 June 2015 13:10:35 Tomi Valkeinen wrote:
> On a platform with no TILER (e.g. omap3, am43xx), when the user wants to
> allocate buffer with flag OMAP_BO_SCANOUT, the buffer needs to be
> allocated with dma_alloc_writecombine. For some reason the driver does
> not return an error if that alloc fails, instead it continues without
> backing memory. This leads to errors later when the user tries to use
> the buffer.
> 
> This patch makes the driver return an error if dma_alloc_writecombine
> fails.
> 
> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/omap_gem.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c
> b/drivers/gpu/drm/omapdrm/omap_gem.c index 2ab77801cf5f..51c5635aff62
> 100644
> --- a/drivers/gpu/drm/omapdrm/omap_gem.c
> +++ b/drivers/gpu/drm/omapdrm/omap_gem.c
> @@ -1392,9 +1392,17 @@ struct drm_gem_object *omap_gem_new(struct drm_device
> *dev, */
>  		omap_obj->vaddr =  dma_alloc_writecombine(dev->dev, size,
>  				&omap_obj->paddr, GFP_KERNEL);
> -		if (omap_obj->vaddr)
> -			flags |= OMAP_BO_DMA;
> +		if (!omap_obj->vaddr) {
> +			spin_lock(&priv->list_lock);
> +			list_del(&omap_obj->mm_list);
> +			spin_unlock(&priv->list_lock);

Wouldn't it be simpler to move the list_add after the "if ((flags & 
OMAP_BO_SCANOUT) && !priv->has_dmm)" block ? You could then avoid the 
list_del.

> +			kfree(omap_obj);
> +
> +			return NULL;
> +		}
> +
> +		flags |= OMAP_BO_DMA;
>  	}
> 
>  	omap_obj->flags = flags;

-- 
Regards,

Laurent Pinchart

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