| Message ID | 55891648.2030402@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@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. > > Thanks, it's cleaner that way: > > commit 70a7c80ae9f787031fa3eebc70df024deadde09f (HEAD) > Author: Tomi Valkeinen <tomi.valkeinen@ti.com> > 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@ti.com> Acked-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > 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) {
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) {