| Message ID | 20190201005057.13648-2-robh@kernel.org |
|---|---|
| State | New |
| Headers | show |
| Series | Add reservation_object to drm_gem_object | expand |
On Thu, Jan 31, 2019 at 06:50:53PM -0600, Rob Herring wrote: > Many users of drm_gem_object embed a struct reservation_object into > their subclassed struct, so let's add one to struct drm_gem_object. > This will allow removing the reservation object from the subclasses > and removing the ->gem_prime_res_obj callback. > > With the addition, add a drm_gem_reservation_object_wait() helper > function for drivers to use in wait ioctls. > > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> > Cc: Maxime Ripard <maxime.ripard@bootlin.com> > Cc: Sean Paul <sean@poorly.run> > Cc: David Airlie <airlied@linux.ie> > Cc: Daniel Vetter <daniel@ffwll.ch> > Signed-off-by: Rob Herring <robh@kernel.org> > --- > Documentation/gpu/todo.rst | 9 --------- > drivers/gpu/drm/drm_gem.c | 39 +++++++++++++++++++++++++++++++++++++ > drivers/gpu/drm/drm_prime.c | 1 + > include/drm/drm_gem.h | 7 +++++++ > 4 files changed, 47 insertions(+), 9 deletions(-) > > diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst > index 14191b64446d..6e0a37d0bf6d 100644 > --- a/Documentation/gpu/todo.rst > +++ b/Documentation/gpu/todo.rst > @@ -209,15 +209,6 @@ Would be great to refactor this all into a set of small common helpers. > > Contact: Daniel Vetter > > -Put a reservation_object into drm_gem_object > --------------------------------------------- > - > -This would remove the need for the ->gem_prime_res_obj callback. It would also > -allow us to implement generic helpers for waiting for a bo, allowing for quite a > -bit of refactoring in the various wait ioctl implementations. > - > -Contact: Daniel Vetter > - > idr_init_base() > --------------- > > diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c > index 8b55ece97967..91dd06c1b3a8 100644 > --- a/drivers/gpu/drm/drm_gem.c > +++ b/drivers/gpu/drm/drm_gem.c > @@ -170,6 +170,10 @@ void drm_gem_private_object_init(struct drm_device *dev, > kref_init(&obj->refcount); > obj->handle_count = 0; > obj->size = size; > + if (!obj->resv) { > + obj->resv = &obj->_resv; > + reservation_object_init(obj->resv); You _fini unconditionally, but don't _init unconditionally. I think simplest to just always init (and only assign the pointer if nothing's been assigned to it yet). > + } > drm_vma_node_reset(&obj->vma_node); > } > EXPORT_SYMBOL(drm_gem_private_object_init); > @@ -657,6 +661,40 @@ drm_gem_object_lookup(struct drm_file *filp, u32 handle) > } > EXPORT_SYMBOL(drm_gem_object_lookup); > > +/** > + * drm_gem_object_lookup - Wait on GEM object's reservation's objects > + * shared and/or exclusive fences. > + * @filp: DRM file private date > + * @handle: userspace handle > + * @wait_all: if true, wait on all fences, else wait on just exclusive fence > + * @timeout: timeout value in jiffies or zero to return immediately > + * > + * Returns: > + * > + * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or > + * greater than 0 on success. > + */ > +long drm_gem_reservation_object_wait(struct drm_file *filep, u32 handle, > + bool wait_all, unsigned long timeout) > +{ > + long ret; > + struct drm_gem_object *obj; > + > + obj = drm_gem_object_lookup(filep, handle); > + if (!obj) { > + DRM_DEBUG("Failed to look up GEM BO %d\n", handle); > + return -EINVAL; > + } > + > + ret = reservation_object_wait_timeout_rcu(obj->resv, wait_all, > + true, timeout); > + > + drm_gem_object_put_unlocked(obj); > + > + return ret; > +} > +EXPORT_SYMBOL(drm_gem_reservation_object_wait); > + > /** > * drm_gem_close_ioctl - implementation of the GEM_CLOSE ioctl > * @dev: drm_device > @@ -821,6 +859,7 @@ drm_gem_object_release(struct drm_gem_object *obj) > if (obj->filp) > fput(obj->filp); > > + reservation_object_fini(&obj->_resv); > drm_gem_free_mmap_offset(obj); > } > EXPORT_SYMBOL(drm_gem_object_release); > diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c > index 231e3f6d5f41..dc079efb3b0f 100644 > --- a/drivers/gpu/drm/drm_prime.c > +++ b/drivers/gpu/drm/drm_prime.c > @@ -504,6 +504,7 @@ struct dma_buf *drm_gem_prime_export(struct drm_device *dev, > .size = obj->size, > .flags = flags, > .priv = obj, > + .resv = obj->resv, > }; > > if (dev->driver->gem_prime_res_obj) > diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h > index c95727425284..f450a5b6038e 100644 > --- a/include/drm/drm_gem.h > +++ b/include/drm/drm_gem.h > @@ -35,6 +35,7 @@ > */ > > #include <linux/kref.h> > +#include <linux/reservation.h> > > #include <drm/drm_vma_manager.h> > > @@ -262,6 +263,10 @@ struct drm_gem_object { > */ > struct dma_buf_attachment *import_attach; > > + /* normally (resv == &_resv) except for imported bo's */ > + struct reservation_object *resv; > + struct reservation_object _resv; Some kerneldoc here would be neat too. Otherwise looks good to me.. -Daniel > + > /** > * @funcs: > * > @@ -363,6 +368,8 @@ void drm_gem_put_pages(struct drm_gem_object *obj, struct page **pages, > bool dirty, bool accessed); > > struct drm_gem_object *drm_gem_object_lookup(struct drm_file *filp, u32 handle); > +long drm_gem_reservation_object_wait(struct drm_file *filep, u32 handle, > + bool wait_all, unsigned long timeout); > int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, > u32 handle, u64 *offset); > int drm_gem_dumb_destroy(struct drm_file *file, > -- > 2.19.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst index 14191b64446d..6e0a37d0bf6d 100644 --- a/Documentation/gpu/todo.rst +++ b/Documentation/gpu/todo.rst @@ -209,15 +209,6 @@ Would be great to refactor this all into a set of small common helpers. Contact: Daniel Vetter -Put a reservation_object into drm_gem_object --------------------------------------------- - -This would remove the need for the ->gem_prime_res_obj callback. It would also -allow us to implement generic helpers for waiting for a bo, allowing for quite a -bit of refactoring in the various wait ioctl implementations. - -Contact: Daniel Vetter - idr_init_base() --------------- diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c index 8b55ece97967..91dd06c1b3a8 100644 --- a/drivers/gpu/drm/drm_gem.c +++ b/drivers/gpu/drm/drm_gem.c @@ -170,6 +170,10 @@ void drm_gem_private_object_init(struct drm_device *dev, kref_init(&obj->refcount); obj->handle_count = 0; obj->size = size; + if (!obj->resv) { + obj->resv = &obj->_resv; + reservation_object_init(obj->resv); + } drm_vma_node_reset(&obj->vma_node); } EXPORT_SYMBOL(drm_gem_private_object_init); @@ -657,6 +661,40 @@ drm_gem_object_lookup(struct drm_file *filp, u32 handle) } EXPORT_SYMBOL(drm_gem_object_lookup); +/** + * drm_gem_object_lookup - Wait on GEM object's reservation's objects + * shared and/or exclusive fences. + * @filp: DRM file private date + * @handle: userspace handle + * @wait_all: if true, wait on all fences, else wait on just exclusive fence + * @timeout: timeout value in jiffies or zero to return immediately + * + * Returns: + * + * Returns -ERESTARTSYS if interrupted, 0 if the wait timed out, or + * greater than 0 on success. + */ +long drm_gem_reservation_object_wait(struct drm_file *filep, u32 handle, + bool wait_all, unsigned long timeout) +{ + long ret; + struct drm_gem_object *obj; + + obj = drm_gem_object_lookup(filep, handle); + if (!obj) { + DRM_DEBUG("Failed to look up GEM BO %d\n", handle); + return -EINVAL; + } + + ret = reservation_object_wait_timeout_rcu(obj->resv, wait_all, + true, timeout); + + drm_gem_object_put_unlocked(obj); + + return ret; +} +EXPORT_SYMBOL(drm_gem_reservation_object_wait); + /** * drm_gem_close_ioctl - implementation of the GEM_CLOSE ioctl * @dev: drm_device @@ -821,6 +859,7 @@ drm_gem_object_release(struct drm_gem_object *obj) if (obj->filp) fput(obj->filp); + reservation_object_fini(&obj->_resv); drm_gem_free_mmap_offset(obj); } EXPORT_SYMBOL(drm_gem_object_release); diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c index 231e3f6d5f41..dc079efb3b0f 100644 --- a/drivers/gpu/drm/drm_prime.c +++ b/drivers/gpu/drm/drm_prime.c @@ -504,6 +504,7 @@ struct dma_buf *drm_gem_prime_export(struct drm_device *dev, .size = obj->size, .flags = flags, .priv = obj, + .resv = obj->resv, }; if (dev->driver->gem_prime_res_obj) diff --git a/include/drm/drm_gem.h b/include/drm/drm_gem.h index c95727425284..f450a5b6038e 100644 --- a/include/drm/drm_gem.h +++ b/include/drm/drm_gem.h @@ -35,6 +35,7 @@ */ #include <linux/kref.h> +#include <linux/reservation.h> #include <drm/drm_vma_manager.h> @@ -262,6 +263,10 @@ struct drm_gem_object { */ struct dma_buf_attachment *import_attach; + /* normally (resv == &_resv) except for imported bo's */ + struct reservation_object *resv; + struct reservation_object _resv; + /** * @funcs: * @@ -363,6 +368,8 @@ void drm_gem_put_pages(struct drm_gem_object *obj, struct page **pages, bool dirty, bool accessed); struct drm_gem_object *drm_gem_object_lookup(struct drm_file *filp, u32 handle); +long drm_gem_reservation_object_wait(struct drm_file *filep, u32 handle, + bool wait_all, unsigned long timeout); int drm_gem_dumb_map_offset(struct drm_file *file, struct drm_device *dev, u32 handle, u64 *offset); int drm_gem_dumb_destroy(struct drm_file *file,
Many users of drm_gem_object embed a struct reservation_object into their subclassed struct, so let's add one to struct drm_gem_object. This will allow removing the reservation object from the subclasses and removing the ->gem_prime_res_obj callback. With the addition, add a drm_gem_reservation_object_wait() helper function for drivers to use in wait ioctls. Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com> Cc: Maxime Ripard <maxime.ripard@bootlin.com> Cc: Sean Paul <sean@poorly.run> Cc: David Airlie <airlied@linux.ie> Cc: Daniel Vetter <daniel@ffwll.ch> Signed-off-by: Rob Herring <robh@kernel.org> --- Documentation/gpu/todo.rst | 9 --------- drivers/gpu/drm/drm_gem.c | 39 +++++++++++++++++++++++++++++++++++++ drivers/gpu/drm/drm_prime.c | 1 + include/drm/drm_gem.h | 7 +++++++ 4 files changed, 47 insertions(+), 9 deletions(-) -- 2.19.1