Re: [PATCH v3 8/9] drm: Remove lock interfaces from GEM VRAM helpers

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Jun 13, 2019 at 09:30:40AM +0200, Thomas Zimmermann wrote:
> The lock functions and the locked-pin/unpin functions of GEM VRAM are not
> requried any longer. Remove them.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>

btw a neat thing we could do for these helpers would be to integrate them
more tighly with the reservation/fence helpers in drm_gem.h. All those
require is that gem_obj->resv is set correctly, which duplicates ttm's own
ttm_buffer_object->resv.

It hink we could also remove ttm_buffer_object->ttm_resv and require that
you always pass one in when calling ttm_bo_init, and gem drivers would
just use the gem_object->resv one.

One complication is that for dma-buf expect we need to make sure we pick
the right one. There's kinda no neat way to solve that, except by just
making ttm_buffer_object a full subclass of drm_gem_object, which probably
wouldn't go down too well I think.

Just an aside, I'm digging around in this area in radeon/amdgpu/nouveau
code right now.
-Daniel

> ---
>  drivers/gpu/drm/drm_gem_vram_helper.c | 109 --------------------------
>  include/drm/drm_gem_vram_helper.h     |   5 --
>  2 files changed, 114 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index f3e5803affb0..4cae52054e61 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -151,36 +151,6 @@ void drm_gem_vram_put(struct drm_gem_vram_object *gbo)
>  }
>  EXPORT_SYMBOL(drm_gem_vram_put);
>  
> -/**
> - * drm_gem_vram_lock() - Locks a VRAM-backed GEM object
> - * @gbo:	the GEM VRAM object
> - * @no_wait:	don't wait for buffer object to become available
> - *
> - * See ttm_bo_reserve() for more information.
> - *
> - * Returns:
> - * 0 on success, or
> - * a negative error code otherwise
> - */
> -int drm_gem_vram_lock(struct drm_gem_vram_object *gbo, bool no_wait)
> -{
> -	return ttm_bo_reserve(&gbo->bo, true, no_wait, NULL);
> -}
> -EXPORT_SYMBOL(drm_gem_vram_lock);
> -
> -/**
> - * drm_gem_vram_unlock() - \
> -	Release a reservation acquired by drm_gem_vram_lock()
> - * @gbo:	the GEM VRAM object
> - *
> - * See ttm_bo_unreserve() for more information.
> - */
> -void drm_gem_vram_unlock(struct drm_gem_vram_object *gbo)
> -{
> -	ttm_bo_unreserve(&gbo->bo);
> -}
> -EXPORT_SYMBOL(drm_gem_vram_unlock);
> -
>  /**
>   * drm_gem_vram_mmap_offset() - Returns a GEM VRAM object's mmap offset
>   * @gbo:	the GEM VRAM object
> @@ -266,49 +236,6 @@ int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag)
>  }
>  EXPORT_SYMBOL(drm_gem_vram_pin);
>  
> -/**
> - * drm_gem_vram_pin_locked() - Pins a GEM VRAM object in a region.
> - * @gbo:	the GEM VRAM object
> - * @pl_flag:	a bitmask of possible memory regions
> - *
> - * Pinning a buffer object ensures that it is not evicted from
> - * a memory region. A pinned buffer object has to be unpinned before
> - * it can be pinned to another region.
> - *
> - * This function pins a GEM VRAM object that has already been
> - * locked. Use drm_gem_vram_pin() if possible.
> - *
> - * Returns:
> - * 0 on success, or
> - * a negative error code otherwise.
> - */
> -int drm_gem_vram_pin_locked(struct drm_gem_vram_object *gbo,
> -			    unsigned long pl_flag)
> -{
> -	int i, ret;
> -	struct ttm_operation_ctx ctx = { false, false };
> -
> -	lockdep_assert_held(&gbo->bo.resv->lock.base);
> -
> -	if (gbo->pin_count) {
> -		++gbo->pin_count;
> -		return 0;
> -	}
> -
> -	drm_gem_vram_placement(gbo, pl_flag);
> -	for (i = 0; i < gbo->placement.num_placement; ++i)
> -		gbo->placements[i].flags |= TTM_PL_FLAG_NO_EVICT;
> -
> -	ret = ttm_bo_validate(&gbo->bo, &gbo->placement, &ctx);
> -	if (ret < 0)
> -		return ret;
> -
> -	gbo->pin_count = 1;
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(drm_gem_vram_pin_locked);
> -
>  /**
>   * drm_gem_vram_unpin() - Unpins a GEM VRAM object
>   * @gbo:	the GEM VRAM object
> @@ -351,42 +278,6 @@ int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo)
>  }
>  EXPORT_SYMBOL(drm_gem_vram_unpin);
>  
> -/**
> - * drm_gem_vram_unpin_locked() - Unpins a GEM VRAM object
> - * @gbo:	the GEM VRAM object
> - *
> - * This function unpins a GEM VRAM object that has already been
> - * locked. Use drm_gem_vram_unpin() if possible.
> - *
> - * Returns:
> - * 0 on success, or
> - * a negative error code otherwise.
> - */
> -int drm_gem_vram_unpin_locked(struct drm_gem_vram_object *gbo)
> -{
> -	int i, ret;
> -	struct ttm_operation_ctx ctx = { false, false };
> -
> -	lockdep_assert_held(&gbo->bo.resv->lock.base);
> -
> -	if (WARN_ON_ONCE(!gbo->pin_count))
> -		return 0;
> -
> -	--gbo->pin_count;
> -	if (gbo->pin_count)
> -		return 0;
> -
> -	for (i = 0; i < gbo->placement.num_placement ; ++i)
> -		gbo->placements[i].flags &= ~TTM_PL_FLAG_NO_EVICT;
> -
> -	ret = ttm_bo_validate(&gbo->bo, &gbo->placement, &ctx);
> -	if (ret < 0)
> -		return ret;
> -
> -	return 0;
> -}
> -EXPORT_SYMBOL(drm_gem_vram_unpin_locked);
> -
>  /**
>   * drm_gem_vram_kmap_at() - Maps a GEM VRAM object into kernel address space
>   * @gbo:	the GEM VRAM object
> diff --git a/include/drm/drm_gem_vram_helper.h b/include/drm/drm_gem_vram_helper.h
> index 4d1d2c1bf32b..1d4aa87f8dfa 100644
> --- a/include/drm/drm_gem_vram_helper.h
> +++ b/include/drm/drm_gem_vram_helper.h
> @@ -77,15 +77,10 @@ struct drm_gem_vram_object *drm_gem_vram_create(struct drm_device *dev,
>  						unsigned long pg_align,
>  						bool interruptible);
>  void drm_gem_vram_put(struct drm_gem_vram_object *gbo);
> -int drm_gem_vram_lock(struct drm_gem_vram_object *gbo, bool no_wait);
> -void drm_gem_vram_unlock(struct drm_gem_vram_object *gbo);
>  u64 drm_gem_vram_mmap_offset(struct drm_gem_vram_object *gbo);
>  s64 drm_gem_vram_offset(struct drm_gem_vram_object *gbo);
>  int drm_gem_vram_pin(struct drm_gem_vram_object *gbo, unsigned long pl_flag);
> -int drm_gem_vram_pin_locked(struct drm_gem_vram_object *gbo,
> -			      unsigned long pl_flag);
>  int drm_gem_vram_unpin(struct drm_gem_vram_object *gbo);
> -int drm_gem_vram_unpin_locked(struct drm_gem_vram_object *gbo);
>  void *drm_gem_vram_kmap_at(struct drm_gem_vram_object *gbo, bool map,
>  			   bool *is_iomem, struct ttm_bo_kmap_obj *kmap);
>  void *drm_gem_vram_kmap(struct drm_gem_vram_object *gbo, bool map,
> -- 
> 2.21.0
> 

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