Re: [PATCH 10/12] drm/exec: add drm_exec_lock_resv function

[Thomas Hellström]

On Fri, 2026-07-10 at 20:52 +0200, Christian König wrote:
> Restructure the drm_exec object to work with dma_resv references
> instead
> of GEM object references.
> 
> Add the new function dma_exec_lock_resv() to lock individual dma_resv
> objects and so allow higher level implementations to handle
> contention
> purely on dma_resv objects.
> 
> WIP! Don't commit like that!
> 
> Signed-off-by: Christian König <christian.koenig@xxxxxxx>

Here, IMO we should move a dma-resv based implementation to dma-buf to
facilitate passing it also through dma_buf_map(),

And if wanting to avoid rewriting all users of drm_exec, Make drm_exec
a thin wrapper on top.

Thanks,
Thomas



> ---
>  drivers/gpu/drm/drm_exec.c | 75 ++++++++++++++++++++++--------------
> --
>  drivers/gpu/drm/drm_gem.c  |  2 +
>  include/drm/drm_exec.h     |  9 +++--
>  3 files changed, 50 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_exec.c b/drivers/gpu/drm/drm_exec.c
> index fa923852fae45..382bf7bcd5ff3 100644
> --- a/drivers/gpu/drm/drm_exec.c
> +++ b/drivers/gpu/drm/drm_exec.c
> @@ -58,8 +58,11 @@ static void drm_exec_unlock_all(struct drm_exec
> *exec)
>  		drm_gem_object_put(obj);
>  	}
>  
> -	drm_gem_object_put(exec->prelocked);
> -	exec->prelocked = NULL;
> +	if (exec->prelocked) {
> +		dma_resv_unlock(exec->prelocked);
> +		dma_resv_put(exec->prelocked);
> +		exec->prelocked = NULL;
> +	}
>  }
>  
>  /**
> @@ -101,7 +104,7 @@ void drm_exec_fini(struct drm_exec *exec)
>  	drm_exec_unlock_all(exec);
>  	kvfree(exec->objects);
>  	if (exec->contended != DRM_EXEC_DUMMY) {
> -		drm_gem_object_put(exec->contended);
> +		dma_resv_put(exec->contended);
>  		ww_acquire_fini(&exec->ticket);
>  	}
>  }
> @@ -158,50 +161,41 @@ static int drm_exec_obj_locked(struct drm_exec
> *exec,
>  /* Make sure the contended object is locked first */
>  static int drm_exec_lock_contended(struct drm_exec *exec)
>  {
> -	struct drm_gem_object *obj = exec->contended;
> +	struct dma_resv *resv = exec->contended;
>  	int ret;
>  
> -	if (likely(!obj))
> +	if (likely(!resv))
>  		return 0;
>  
>  	/* Always cleanup the contention so that error handling can
> kick in */
>  	exec->contended = NULL;
>  	if (exec->flags & DRM_EXEC_INTERRUPTIBLE_WAIT) {
> -		ret = dma_resv_lock_slow_interruptible(obj->resv,
> -						       &exec-
> >ticket);
> +		ret = dma_resv_lock_slow_interruptible(resv, &exec-
> >ticket);
>  		if (unlikely(ret))
>  			goto error_dropref;
>  	} else {
> -		dma_resv_lock_slow(obj->resv, &exec->ticket);
> +		dma_resv_lock_slow(resv, &exec->ticket);
>  	}
>  
> -	ret = drm_exec_obj_locked(exec, obj);
> -	if (unlikely(ret))
> -		goto error_unlock;
> -
> -	exec->prelocked = obj;
> +	exec->prelocked = resv;
>  	return 0;
>  
> -error_unlock:
> -	dma_resv_unlock(obj->resv);
> -
>  error_dropref:
> -	drm_gem_object_put(obj);
> +	dma_resv_put(resv);
>  	return ret;
>  }
>  
>  /**
> - * drm_exec_lock_obj - lock a GEM object for use
> + * drm_exec_lock_resv - lock a dma_resv object
>   * @exec: the drm_exec object with the state
> - * @obj: the GEM object to lock
> + * @resv: the dma_resv object to lock
>   *
> - * Lock a GEM object for use and grab a reference to it.
> + * Lock a dma_resv object for use or grab a reference to it on
> contention.
>   *
>   * Returns: -EDEADLK if a contention is detected, -EALREADY when
> object is
> - * already locked (can be suppressed by setting the
> DRM_EXEC_IGNORE_DUPLICATES
> - * flag), -ENOMEM when memory allocation failed and zero for
> success.
> + * already locked, -ENOMEM when memory allocation failed and zero
> for success.
>   */
> -int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object
> *obj)
> +int drm_exec_lock_resv(struct drm_exec *exec, struct dma_resv *resv)
>  {
>  	int ret;
>  
> @@ -209,22 +203,39 @@ int drm_exec_lock_obj(struct drm_exec *exec,
> struct drm_gem_object *obj)
>  	if (unlikely(ret))
>  		return ret;
>  
> -	if (exec->prelocked == obj) {
> -		drm_gem_object_put(exec->prelocked);
> +	if (exec->prelocked == resv) {
> +		dma_resv_put(exec->prelocked);
>  		exec->prelocked = NULL;
>  		return 0;
>  	}
>  
>  	if (exec->flags & DRM_EXEC_INTERRUPTIBLE_WAIT)
> -		ret = dma_resv_lock_interruptible(obj->resv, &exec-
> >ticket);
> +		ret = dma_resv_lock_interruptible(resv, &exec-
> >ticket);
>  	else
> -		ret = dma_resv_lock(obj->resv, &exec->ticket);
> +		ret = dma_resv_lock(resv, &exec->ticket);
>  
> -	if (unlikely(ret == -EDEADLK)) {
> -		drm_gem_object_get(obj);
> -		exec->contended = obj;
> -		return -EDEADLK;
> -	}
> +	if (unlikely(ret == -EDEADLK))
> +		exec->contended = dma_resv_get(resv);
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_exec_lock_resv);
> +
> +/**
> + * drm_exec_lock_obj - lock a GEM object for use
> + * @exec: the drm_exec object with the state
> + * @obj: the GEM object to lock
> + *
> + * Lock a GEM object for use and grab a reference to it.
> + *
> + * Returns: -EDEADLK if a contention is detected, -EALREADY when
> object is
> + * already locked (can be suppressed by setting the
> DRM_EXEC_IGNORE_DUPLICATES
> + * flag), -ENOMEM when memory allocation failed and zero for
> success.
> + */
> +int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object
> *obj)
> +{
> +	int ret;
> +
> +	ret = drm_exec_lock_resv(exec, obj->resv);
>  
>  	if (unlikely(ret == -EALREADY) &&
>  	    exec->flags & DRM_EXEC_IGNORE_DUPLICATES)
> diff --git a/drivers/gpu/drm/drm_gem.c b/drivers/gpu/drm/drm_gem.c
> index bbcbd25f014f0..f5cf9ad596a67 100644
> --- a/drivers/gpu/drm/drm_gem.c
> +++ b/drivers/gpu/drm/drm_gem.c
> @@ -229,6 +229,8 @@ void drm_gem_private_object_init(struct
> drm_device *dev,
>  	obj->size = size;
>  	mutex_init(&obj->gpuva.lock);
>  	dma_resv_init(&obj->_resv);
> +
> +	/* TODO: This needs to go away for drm_exec to work
> correctly!!! */
>  	if (!obj->resv)
>  		obj->resv = dma_resv_get(&obj->_resv);
>  
> diff --git a/include/drm/drm_exec.h b/include/drm/drm_exec.h
> index 8725ba92ff916..9daedb676d7b1 100644
> --- a/include/drm/drm_exec.h
> +++ b/include/drm/drm_exec.h
> @@ -47,14 +47,14 @@ struct drm_exec {
>  	struct drm_gem_object	**objects;
>  
>  	/**
> -	 * @contended: contended GEM object we backed off for
> +	 * @contended: contended dma_resv object we backed off for
>  	 */
> -	struct drm_gem_object	*contended;
> +	struct dma_resv		*contended;
>  
>  	/**
> -	 * @prelocked: already locked GEM object due to contention
> +	 * @prelocked: already locked dma_resv object due to
> contention
>  	 */
> -	struct drm_gem_object *prelocked;
> +	struct dma_resv		*prelocked;
>  };
>  
>  /**
> @@ -175,6 +175,7 @@ static inline struct ww_acquire_ctx
> *drm_exec_ticket(struct drm_exec *exec)
>  void drm_exec_init(struct drm_exec *exec, u32 flags, unsigned nr);
>  void drm_exec_fini(struct drm_exec *exec);
>  bool drm_exec_cleanup(struct drm_exec *exec);
> +int drm_exec_lock_resv(struct drm_exec *exec, struct dma_resv
> *resv);
>  int drm_exec_lock_obj(struct drm_exec *exec, struct drm_gem_object
> *obj);
>  void drm_exec_unlock_obj(struct drm_exec *exec, struct
> drm_gem_object *obj);
>  int drm_exec_prepare_obj(struct drm_exec *exec, struct
> drm_gem_object *obj,