On Thu, Jul 11, 2013 at 11:56 AM, David Herrmann <dh.herrm...@gmail.com> wrote: > drm_gem_object_init() and drm_gem_private_object_init() do exactly the > same (except for shmem alloc) so make the first use the latter to reduce > code duplication. > > Also drop the return code from drm_gem_private_object_init(). It seems > unlikely that we will extend it any time soon so no reason to keep it > around. This simplifies code paths in drivers, too. > > Last but not least, fix gma500 to call drm_gem_object_release() before > freeing objects that were allocated via drm_gem_private_object_init(). > That isn't actually necessary for now, but might be in the future. > > Cc: Patrik Jakobsson <patrik.r.jakobs...@gmail.com> > Signed-off-by: David Herrmann <dh.herrm...@gmail.com> > --- > drivers/gpu/drm/drm_gem.c | 20 ++++++++------------ > drivers/gpu/drm/gma500/framebuffer.c | 6 ++---- > drivers/gpu/drm/gma500/gem.c | 7 ++++--- > drivers/gpu/drm/i915/i915_gem_dmabuf.c | 7 +------ > drivers/gpu/drm/i915/i915_gem_stolen.c | 4 +--- > drivers/gpu/drm/omapdrm/omap_gem.c | 3 ++- > include/drm/drmP.h | 4 ++-- > 7 files changed, 20 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index 603f256..1ad9e7e 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -132,16 +132,14 @@ drm_gem_destroy(struct drm_device *dev) > int drm_gem_object_init(struct drm_device *dev, > struct drm_gem_object *obj, size_t size) > { > - BUG_ON((size & (PAGE_SIZE - 1)) != 0); > + struct file *filp; > > - obj->dev = dev; > - obj->filp = shmem_file_setup("drm mm object", size, VM_NORESERVE); > - if (IS_ERR(obj->filp)) > - return PTR_ERR(obj->filp); > + filp = shmem_file_setup("drm mm object", size, VM_NORESERVE); > + if (IS_ERR(filp)) > + return PTR_ERR(filp); > > - kref_init(&obj->refcount); > - atomic_set(&obj->handle_count, 0); > - obj->size = size; > + drm_gem_private_object_init(dev, obj, size); > + obj->filp = filp; > > return 0; > } > @@ -152,8 +150,8 @@ EXPORT_SYMBOL(drm_gem_object_init); > * no GEM provided backing store. Instead the caller is responsible for > * backing the object and handling it. > */ > -int drm_gem_private_object_init(struct drm_device *dev, > - struct drm_gem_object *obj, size_t size) > +void drm_gem_private_object_init(struct drm_device *dev, > + struct drm_gem_object *obj, size_t size) > { > BUG_ON((size & (PAGE_SIZE - 1)) != 0); > > @@ -163,8 +161,6 @@ int drm_gem_private_object_init(struct drm_device *dev, > kref_init(&obj->refcount); > atomic_set(&obj->handle_count, 0); > obj->size = size; > - > - return 0; > } > EXPORT_SYMBOL(drm_gem_private_object_init); > > diff --git a/drivers/gpu/drm/gma500/framebuffer.c > b/drivers/gpu/drm/gma500/framebuffer.c > index 8b1b6d9..362dd2a 100644 > --- a/drivers/gpu/drm/gma500/framebuffer.c > +++ b/drivers/gpu/drm/gma500/framebuffer.c > @@ -321,10 +321,8 @@ static struct gtt_range *psbfb_alloc(struct drm_device > *dev, int aligned_size) > /* Begin by trying to use stolen memory backing */ > backing = psb_gtt_alloc_range(dev, aligned_size, "fb", 1); > if (backing) { > - if (drm_gem_private_object_init(dev, > - &backing->gem, aligned_size) == 0) > - return backing; > - psb_gtt_free_range(dev, backing); > + drm_gem_private_object_init(dev, &backing->gem, aligned_size); > + return backing; > } > return NULL; > } > diff --git a/drivers/gpu/drm/gma500/gem.c b/drivers/gpu/drm/gma500/gem.c > index eefd6cc..fe1d332 100644 > --- a/drivers/gpu/drm/gma500/gem.c > +++ b/drivers/gpu/drm/gma500/gem.c > @@ -261,11 +261,12 @@ static int psb_gem_create_stolen(struct drm_file *file, > struct drm_device *dev, > struct gtt_range *gtt = psb_gtt_alloc_range(dev, size, "gem", 1); > if (gtt == NULL) > return -ENOMEM; > - if (drm_gem_private_object_init(dev, >t->gem, size) != 0) > - goto free_gtt; > + > + drm_gem_private_object_init(dev, >t->gem, size); > if (drm_gem_handle_create(file, >t->gem, handle) == 0) > return 0; > -free_gtt: > + > + drm_gem_object_release(>t->gem); > psb_gtt_free_range(dev, gtt); > return -ENOMEM; > } > diff --git a/drivers/gpu/drm/i915/i915_gem_dmabuf.c > b/drivers/gpu/drm/i915/i915_gem_dmabuf.c > index dc53a52..f2e185c 100644 > --- a/drivers/gpu/drm/i915/i915_gem_dmabuf.c > +++ b/drivers/gpu/drm/i915/i915_gem_dmabuf.c > @@ -289,12 +289,7 @@ struct drm_gem_object *i915_gem_prime_import(struct > drm_device *dev, > goto fail_detach; > } > > - ret = drm_gem_private_object_init(dev, &obj->base, dma_buf->size); > - if (ret) { > - i915_gem_object_free(obj); > - goto fail_detach; > - } > - > + drm_gem_private_object_init(dev, &obj->base, dma_buf->size); > i915_gem_object_init(obj, &i915_gem_object_dmabuf_ops); > obj->base.import_attach = attach; > > diff --git a/drivers/gpu/drm/i915/i915_gem_stolen.c > b/drivers/gpu/drm/i915/i915_gem_stolen.c > index 982d473..ff27968 100644 > --- a/drivers/gpu/drm/i915/i915_gem_stolen.c > +++ b/drivers/gpu/drm/i915/i915_gem_stolen.c > @@ -271,9 +271,7 @@ _i915_gem_object_create_stolen(struct drm_device *dev, > if (obj == NULL) > return NULL; > > - if (drm_gem_private_object_init(dev, &obj->base, stolen->size)) > - goto cleanup; > - > + drm_gem_private_object_init(dev, &obj->base, stolen->size); > i915_gem_object_init(obj, &i915_gem_object_stolen_ops); > > obj->pages = i915_pages_create_for_stolen(dev, > diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c > b/drivers/gpu/drm/omapdrm/omap_gem.c > index ebbdf41..cbcd71e 100644 > --- a/drivers/gpu/drm/omapdrm/omap_gem.c > +++ b/drivers/gpu/drm/omapdrm/omap_gem.c > @@ -1427,8 +1427,9 @@ struct drm_gem_object *omap_gem_new(struct drm_device > *dev, > omap_obj->height = gsize.tiled.height; > } > > + ret = 0; > if (flags & (OMAP_BO_DMA|OMAP_BO_EXT_MEM)) > - ret = drm_gem_private_object_init(dev, obj, size); > + drm_gem_private_object_init(dev, obj, size); > else > ret = drm_gem_object_init(dev, obj, size); > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h > index 12083dc..ee2f049 100644 > --- a/include/drm/drmP.h > +++ b/include/drm/drmP.h > @@ -1613,8 +1613,8 @@ struct drm_gem_object *drm_gem_object_alloc(struct > drm_device *dev, > size_t size); > int drm_gem_object_init(struct drm_device *dev, > struct drm_gem_object *obj, size_t size); > -int drm_gem_private_object_init(struct drm_device *dev, > - struct drm_gem_object *obj, size_t size); > +void drm_gem_private_object_init(struct drm_device *dev, > + struct drm_gem_object *obj, size_t size); > void drm_gem_object_handle_free(struct drm_gem_object *obj); > void drm_gem_vm_open(struct vm_area_struct *vma); > void drm_gem_vm_close(struct vm_area_struct *vma); > -- > 1.8.3.2 >
Looks good to me. Thanks. Reviewed-by: Patrik Jakobsson <patrik.r.jakobs...@gmail.com> _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel