diff mbox series

[3/5] drm/exec: provide trylock interface for eviction

Message ID 20240703132602.4756-4-christian.koenig@amd.com (mailing list archive)
State New, archived
Headers show
Series [1/5] dma-buf/dma-resv: Introduce dma_resv_trylock_ctx() | expand

Commit Message

Christian König July 3, 2024, 1:26 p.m. UTC 
The TTM eviction path has some additional requirements which make it
necessary to trylock an object and then eventually keep or drop the lock
again.

Signed-off-by: Christian König <christian.koenig@amd.com>
---
 drivers/gpu/drm/drm_exec.c | 77 ++++++++++++++++++++++++++++++++++++++
 include/drm/drm_exec.h     |  5 +++
 2 files changed, 82 insertions(+)

Comments

Thomas Hellström July 3, 2024, 4:13 p.m. UTC | #1 
On Wed, 2024-07-03 at 15:26 +0200, Christian König wrote:
> The TTM eviction path has some additional requirements which make it
> necessary to trylock an object and then eventually keep or drop the
> lock
> again.
> 
> Signed-off-by: Christian König <christian.koenig@amd.com>
> ---
>  drivers/gpu/drm/drm_exec.c | 77
> ++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_exec.h     |  5 +++
>  2 files changed, 82 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c
> index 220df336fbd9..b81bf5a92d97 100644
> --- a/drivers/gpu/drm/drm_exec.c
> +++ b/drivers/gpu/drm/drm_exec.c
> @@ -336,5 +336,82 @@ int drm_exec_prepare_array(struct drm_exec
> *exec,
>  }
>  EXPORT_SYMBOL(drm_exec_prepare_array);
>  
> +/**
> + * drm_exec_trylock_obj - try to lock a GEM object
> + * @exec: the drm_exec object with the state
> + * @obj: the GEM object to trylock
> + *
> + * Try to lock a GEM object but don't grab a reference yet.
> + *
> + * Since we can't handle contention here it's illegal to trylock the
> first
> + * object.
> + *
> + * This function is suposed to be used from atomic context and we
> don't
> + * know if the GEM object will actually be used or not. So we don't
> grab a
> + * reference yet.

With the pending LRU walker the *need* for atomic context here is gone.

> + *
> + * Returns: True if the object could be locked, false otherwise.
> + */
> +bool drm_exec_trylock_obj(struct drm_exec *exec, struct
> drm_gem_object *obj)
> +{
> +	if (WARN_ON(!exec->num_objects))
> +		return false;

I think we were in the middle of the discussion here about how to
handle this. IIRC the last suggestion was to 

if (exec->contended)
     return false;

and provide a 

drm_exec_sanitize_for_trylock() function that could be used to pre-lock
the contended lock (and perhaps provide any needed memory to register
so that a lock in atomic context could be made)

The use-case I'm worried about moving forward is, again, bo creation
where I think a push out of the validation will make the conversion of
drm_exec buffer object creation in drivers much more complicated than
it already is. Or perhaps I'm misunderstanding how that was supposed to
work.

/Thomas


> +
> +	if (exec->prelocked == obj)
> +		return true;
> +
> +	return dma_resv_trylock_ctx(obj->resv, &exec->ticket);
> +}
> +EXPORT_SYMBOL(drm_exec_trylock_obj);
> +
> +/**
> + * drm_exec_keep_trylocked_obj - keep the trylocked obj
> + * @exec: the drm_exec object with the state
> + * @obj: the GEM object to trylock
> + *
> + * Keep a trylocked object in the drm_exec state object. Grabs a
> reference to
> + * the object and adds it to the container of locked objects.
> + */

So these could be dropped.


> +int drm_exec_keep_trylocked_obj(struct drm_exec *exec,
> +				struct drm_gem_object *obj)
> +{
> +	int ret;
> +
> +	ret = drm_exec_obj_locked(exec, obj);
> +	if (ret) {
> +		dma_resv_unlock(obj->resv);
> +		return ret;
> +	}
> +
> +	if (exec->prelocked == obj) {
> +		drm_gem_object_put(exec->prelocked);
> +		exec->prelocked = NULL;
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_exec_keep_trylocked_obj);
> +
> +/**
> + * drm_exec_drop_trylocked_obj - drop the trylocked obj
> + * @exec: the drm_exec object with the state
> + * @obj: the GEM object to trylock
> + *
> + * Used to drop a trylocked object in the drm_exec state object,
> drop the
> + * reservation lock again and cleanup all references.
> + */
> +void drm_exec_drop_trylocked_obj(struct drm_exec *exec,
> +				 struct drm_gem_object *obj)
> +{
> +	/*
> +	 * We can't drop the reference of prelocked objects since we
> might still
> +	 * be in atomic context. Additionally it makes sense to keep
> the
> +	 * prelocked object around since we might need it again
> later on.
> +	 */
> +	if (exec->prelocked != obj)
> +		dma_resv_unlock(obj->resv);
> +}
> +EXPORT_SYMBOL(drm_exec_drop_trylocked_obj);
> +
>  MODULE_DESCRIPTION("DRM execution context");
>  MODULE_LICENSE("Dual MIT/GPL");
> diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h
> index aa786b828a0a..a3943057a3e8 100644
> --- a/include/drm/drm_exec.h
> +++ b/include/drm/drm_exec.h
> @@ -146,5 +146,10 @@ int drm_exec_prepare_array(struct drm_exec
> *exec,
>  			   struct drm_gem_object **objects,
>  			   unsigned int num_objects,
>  			   unsigned int num_fences);
> +bool drm_exec_trylock_obj(struct drm_exec *exec, struct
> drm_gem_object *obj);
> +int drm_exec_keep_trylocked_obj(struct drm_exec *exec,
> +				struct drm_gem_object *obj);
> +void drm_exec_drop_trylocked_obj(struct drm_exec *exec,
> +				struct drm_gem_object *obj);
>  
>  #endif
diff mbox series

Patch

diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c
index 220df336fbd9..b81bf5a92d97 100644
--- a/drivers/gpu/drm/drm_exec.c
+++ b/drivers/gpu/drm/drm_exec.c
@@ -336,5 +336,82 @@  int drm_exec_prepare_array(struct drm_exec *exec,
 }
 EXPORT_SYMBOL(drm_exec_prepare_array);
 
+/**
+ * drm_exec_trylock_obj - try to lock a GEM object
+ * @exec: the drm_exec object with the state
+ * @obj: the GEM object to trylock
+ *
+ * Try to lock a GEM object but don't grab a reference yet.
+ *
+ * Since we can't handle contention here it's illegal to trylock the first
+ * object.
+ *
+ * This function is suposed to be used from atomic context and we don't
+ * know if the GEM object will actually be used or not. So we don't grab a
+ * reference yet.
+ *
+ * Returns: True if the object could be locked, false otherwise.
+ */
+bool drm_exec_trylock_obj(struct drm_exec *exec, struct drm_gem_object *obj)
+{
+	if (WARN_ON(!exec->num_objects))
+		return false;
+
+	if (exec->prelocked == obj)
+		return true;
+
+	return dma_resv_trylock_ctx(obj->resv, &exec->ticket);
+}
+EXPORT_SYMBOL(drm_exec_trylock_obj);
+
+/**
+ * drm_exec_keep_trylocked_obj - keep the trylocked obj
+ * @exec: the drm_exec object with the state
+ * @obj: the GEM object to trylock
+ *
+ * Keep a trylocked object in the drm_exec state object. Grabs a reference to
+ * the object and adds it to the container of locked objects.
+ */
+int drm_exec_keep_trylocked_obj(struct drm_exec *exec,
+				struct drm_gem_object *obj)
+{
+	int ret;
+
+	ret = drm_exec_obj_locked(exec, obj);
+	if (ret) {
+		dma_resv_unlock(obj->resv);
+		return ret;
+	}
+
+	if (exec->prelocked == obj) {
+		drm_gem_object_put(exec->prelocked);
+		exec->prelocked = NULL;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_exec_keep_trylocked_obj);
+
+/**
+ * drm_exec_drop_trylocked_obj - drop the trylocked obj
+ * @exec: the drm_exec object with the state
+ * @obj: the GEM object to trylock
+ *
+ * Used to drop a trylocked object in the drm_exec state object, drop the
+ * reservation lock again and cleanup all references.
+ */
+void drm_exec_drop_trylocked_obj(struct drm_exec *exec,
+				 struct drm_gem_object *obj)
+{
+	/*
+	 * We can't drop the reference of prelocked objects since we might still
+	 * be in atomic context. Additionally it makes sense to keep the
+	 * prelocked object around since we might need it again later on.
+	 */
+	if (exec->prelocked != obj)
+		dma_resv_unlock(obj->resv);
+}
+EXPORT_SYMBOL(drm_exec_drop_trylocked_obj);
+
 MODULE_DESCRIPTION("DRM execution context");
 MODULE_LICENSE("Dual MIT/GPL");
diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h
index aa786b828a0a..a3943057a3e8 100644
--- a/include/drm/drm_exec.h
+++ b/include/drm/drm_exec.h
@@ -146,5 +146,10 @@  int drm_exec_prepare_array(struct drm_exec *exec,
 			   struct drm_gem_object **objects,
 			   unsigned int num_objects,
 			   unsigned int num_fences);
+bool drm_exec_trylock_obj(struct drm_exec *exec, struct drm_gem_object *obj);
+int drm_exec_keep_trylocked_obj(struct drm_exec *exec,
+				struct drm_gem_object *obj);
+void drm_exec_drop_trylocked_obj(struct drm_exec *exec,
+				struct drm_gem_object *obj);
 
 #endif