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.lankho...@linux.intel.com>
> Cc: Maxime Ripard <maxime.rip...@bootlin.com>
> Cc: Sean Paul <s...@poorly.run>
> Cc: David Airlie <airl...@linux.ie>
> Cc: Daniel Vetter <dan...@ffwll.ch>
> Signed-off-by: Rob Herring <r...@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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
