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@xxxxxxx>
> ---
> 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
