Hi Tomi, On Tuesday 23 June 2015 11:18:16 Tomi Valkeinen wrote: > On 18/06/15 17:27, Laurent Pinchart wrote: > > 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@xxxxxx> > >> --- > >> > >> 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. > > Thanks, it's cleaner that way: > > commit 70a7c80ae9f787031fa3eebc70df024deadde09f (HEAD) > Author: Tomi Valkeinen <tomi.valkeinen@xxxxxx> > Date: Tue Mar 17 15:31:11 2015 +0200 > > drm/omap: return error if dma_alloc_writecombine fails > > 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@xxxxxx> Acked-by: Laurent Pinchart <laurent.pinchart@xxxxxxxxxxxxxxxx> > diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c > b/drivers/gpu/drm/omapdrm/omap_gem.c index 2ab77801cf5f..874eb6dcf94b > 100644 > --- a/drivers/gpu/drm/omapdrm/omap_gem.c > +++ b/drivers/gpu/drm/omapdrm/omap_gem.c > @@ -1378,11 +1378,7 @@ struct drm_gem_object *omap_gem_new(struct drm_device > *dev, > > omap_obj = kzalloc(sizeof(*omap_obj), GFP_KERNEL); > if (!omap_obj) > - goto fail; > - > - spin_lock(&priv->list_lock); > - list_add(&omap_obj->mm_list, &priv->obj_list); > - spin_unlock(&priv->list_lock); > + return NULL; > > obj = &omap_obj->base; > > @@ -1392,11 +1388,19 @@ 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) { > + kfree(omap_obj); > > + return NULL; > + } > + > + flags |= OMAP_BO_DMA; > } > > + spin_lock(&priv->list_lock); > + list_add(&omap_obj->mm_list, &priv->obj_list); > + spin_unlock(&priv->list_lock); > + > omap_obj->flags = flags; > > if (flags & OMAP_BO_TILED) { -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel