[1/5] drm: Add reservation_object to drm_gem_object

Message ID 20190201005057.13648-2-robh@kernel.org
State New
Headers show
Series
  • Add reservation_object to drm_gem_object
Related show

Commit Message

Rob Herring Feb. 1, 2019, 12:50 a.m.
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

Comments

Daniel Vetter Feb. 1, 2019, 5:21 p.m. | #1
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

Patch

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,