diff mbox series

[1/3] drm/ttm: replace last move_notify with delete_mem_notify

Message ID 20201021044031.1752624-2-airlied@gmail.com (mailing list archive)
State New, archived
Headers show
Series ttm: change move notify + revised multi hop patch | expand

Commit Message

Dave Airlie Oct. 21, 2020, 4:40 a.m. UTC
From: Dave Airlie <airlied@redhat.com>

The move notify callback is only used in one place, this should
be removed in the future, but for now just rename it to the use
case which is to notify the driver that the GPU memory is to be
deleted.

Drivers can be cleaned up after this separately.

Signed-off-by: Dave Airlie <airlied@redhat.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 +++++++-
 drivers/gpu/drm/drm_gem_vram_helper.c      |  8 +++-----
 drivers/gpu/drm/nouveau/nouveau_bo.c       |  8 +++++++-
 drivers/gpu/drm/qxl/qxl_ttm.c              |  7 ++++++-
 drivers/gpu/drm/radeon/radeon_ttm.c        |  8 +++++++-
 drivers/gpu/drm/ttm/ttm_bo.c               |  4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  8 +++++++-
 include/drm/ttm/ttm_bo_driver.h            | 10 ++--------
 8 files changed, 41 insertions(+), 20 deletions(-)

Comments

Christian König Oct. 21, 2020, 9:58 a.m. UTC | #1
Am 21.10.20 um 06:40 schrieb Dave Airlie:
> From: Dave Airlie <airlied@redhat.com>
>
> The move notify callback is only used in one place, this should
> be removed in the future, but for now just rename it to the use
> case which is to notify the driver that the GPU memory is to be
> deleted.

Probably the right thing to do is to call the move callback with 
move(from, NULL) in this case as well.

And then driver can call the necessary function to throw away the 
backing store pipelined.

>
> Drivers can be cleaned up after this separately.
>
> Signed-off-by: Dave Airlie <airlied@redhat.com>

Reviewed-by: Christian König <christian.koenig@amd.com>

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 +++++++-
>   drivers/gpu/drm/drm_gem_vram_helper.c      |  8 +++-----
>   drivers/gpu/drm/nouveau/nouveau_bo.c       |  8 +++++++-
>   drivers/gpu/drm/qxl/qxl_ttm.c              |  7 ++++++-
>   drivers/gpu/drm/radeon/radeon_ttm.c        |  8 +++++++-
>   drivers/gpu/drm/ttm/ttm_bo.c               |  4 ++--
>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  8 +++++++-
>   include/drm/ttm/ttm_bo_driver.h            | 10 ++--------
>   8 files changed, 41 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> index 87e10a212b8a..62f9194b1dd1 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> @@ -1730,6 +1730,12 @@ static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
>   	return ret;
>   }
>   
> +static void
> +amdgpu_bo_delete_mem_notify(struct ttm_buffer_object *bo)
> +{
> +	amdgpu_bo_move_notify(bo, false, NULL);
> +}
> +
>   static struct ttm_bo_driver amdgpu_bo_driver = {
>   	.ttm_tt_create = &amdgpu_ttm_tt_create,
>   	.ttm_tt_populate = &amdgpu_ttm_tt_populate,
> @@ -1739,7 +1745,7 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
>   	.evict_flags = &amdgpu_evict_flags,
>   	.move = &amdgpu_bo_move,
>   	.verify_access = &amdgpu_verify_access,
> -	.move_notify = &amdgpu_bo_move_notify,
> +	.delete_mem_notify = &amdgpu_bo_delete_mem_notify,
>   	.release_notify = &amdgpu_bo_release_notify,
>   	.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
>   	.io_mem_pfn = amdgpu_ttm_io_mem_pfn,
> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> index 19087b22bdbb..9da823eb0edd 100644
> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> @@ -949,9 +949,7 @@ static void bo_driver_evict_flags(struct ttm_buffer_object *bo,
>   	drm_gem_vram_bo_driver_evict_flags(gbo, placement);
>   }
>   
> -static void bo_driver_move_notify(struct ttm_buffer_object *bo,
> -				  bool evict,
> -				  struct ttm_resource *new_mem)
> +static void bo_driver_delete_mem_notify(struct ttm_buffer_object *bo)
>   {
>   	struct drm_gem_vram_object *gbo;
>   
> @@ -961,7 +959,7 @@ static void bo_driver_move_notify(struct ttm_buffer_object *bo,
>   
>   	gbo = drm_gem_vram_of_bo(bo);
>   
> -	drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
> +	drm_gem_vram_bo_driver_move_notify(gbo, false, NULL);
>   }
>   
>   static int bo_driver_move(struct ttm_buffer_object *bo,
> @@ -1002,7 +1000,7 @@ static struct ttm_bo_driver bo_driver = {
>   	.eviction_valuable = ttm_bo_eviction_valuable,
>   	.evict_flags = bo_driver_evict_flags,
>   	.move = bo_driver_move,
> -	.move_notify = bo_driver_move_notify,
> +	.delete_mem_notify = bo_driver_delete_mem_notify,
>   	.io_mem_reserve = bo_driver_io_mem_reserve,
>   };
>   
> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> index 70b6f3b1ae85..acff82afe260 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> @@ -1401,6 +1401,12 @@ nouveau_bo_fence(struct nouveau_bo *nvbo, struct nouveau_fence *fence, bool excl
>   		dma_resv_add_shared_fence(resv, &fence->base);
>   }
>   
> +static void
> +nouveau_bo_delete_mem_notify(struct ttm_buffer_object *bo)
> +{
> +	nouveau_bo_move_ntfy(bo, false, NULL);
> +}
> +
>   struct ttm_bo_driver nouveau_bo_driver = {
>   	.ttm_tt_create = &nouveau_ttm_tt_create,
>   	.ttm_tt_populate = &nouveau_ttm_tt_populate,
> @@ -1408,7 +1414,7 @@ struct ttm_bo_driver nouveau_bo_driver = {
>   	.ttm_tt_destroy = &nouveau_ttm_tt_destroy,
>   	.eviction_valuable = ttm_bo_eviction_valuable,
>   	.evict_flags = nouveau_bo_evict_flags,
> -	.move_notify = nouveau_bo_move_ntfy,
> +	.delete_mem_notify = nouveau_bo_delete_mem_notify,
>   	.move = nouveau_bo_move,
>   	.verify_access = nouveau_bo_verify_access,
>   	.io_mem_reserve = &nouveau_ttm_io_mem_reserve,
> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> index 1cc3c14bc684..b52a4563b47b 100644
> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> @@ -166,6 +166,11 @@ static int qxl_bo_move(struct ttm_buffer_object *bo, bool evict,
>   	return ret;
>   }
>   
> +static void qxl_bo_delete_mem_notify(struct ttm_buffer_object *bo)
> +{
> +	qxl_bo_move_notify(bo, false, NULL);
> +}
> +
>   static struct ttm_bo_driver qxl_bo_driver = {
>   	.ttm_tt_create = &qxl_ttm_tt_create,
>   	.ttm_tt_destroy = &qxl_ttm_backend_destroy,
> @@ -173,7 +178,7 @@ static struct ttm_bo_driver qxl_bo_driver = {
>   	.evict_flags = &qxl_evict_flags,
>   	.move = &qxl_bo_move,
>   	.io_mem_reserve = &qxl_ttm_io_mem_reserve,
> -	.move_notify = &qxl_bo_move_notify,
> +	.delete_mem_notify = &qxl_bo_delete_mem_notify,
>   };
>   
>   static int qxl_ttm_init_mem_type(struct qxl_device *qdev,
> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> index cd454e5c802f..321c09d20c6c 100644
> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> @@ -824,6 +824,12 @@ bool radeon_ttm_tt_is_readonly(struct radeon_device *rdev,
>   	return !!(gtt->userflags & RADEON_GEM_USERPTR_READONLY);
>   }
>   
> +static void
> +radeon_bo_delete_mem_notify(struct ttm_buffer_object *bo)
> +{
> +	radeon_bo_move_notify(bo, false, NULL);
> +}
> +
>   static struct ttm_bo_driver radeon_bo_driver = {
>   	.ttm_tt_create = &radeon_ttm_tt_create,
>   	.ttm_tt_populate = &radeon_ttm_tt_populate,
> @@ -833,7 +839,7 @@ static struct ttm_bo_driver radeon_bo_driver = {
>   	.evict_flags = &radeon_evict_flags,
>   	.move = &radeon_bo_move,
>   	.verify_access = &radeon_verify_access,
> -	.move_notify = &radeon_bo_move_notify,
> +	.delete_mem_notify = &radeon_bo_delete_mem_notify,
>   	.io_mem_reserve = &radeon_ttm_io_mem_reserve,
>   };
>   
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index 2b578012cdef..e2afab3d97ee 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -284,8 +284,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>   
>   static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
>   {
> -	if (bo->bdev->driver->move_notify)
> -		bo->bdev->driver->move_notify(bo, false, NULL);
> +	if (bo->bdev->driver->delete_mem_notify)
> +		bo->bdev->driver->delete_mem_notify(bo);
>   
>   	ttm_bo_tt_destroy(bo);
>   	ttm_resource_free(bo, &bo->mem);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> index de25cf016be2..88be48ad0344 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> @@ -771,6 +771,12 @@ static int vmw_move(struct ttm_buffer_object *bo,
>   	return ret;
>   }
>   
> +static void
> +vmw_delete_mem_notify(struct ttm_buffer_object *bo)
> +{
> +	vmw_move_notify(bo, false, NULL);
> +}
> +
>   struct ttm_bo_driver vmw_bo_driver = {
>   	.ttm_tt_create = &vmw_ttm_tt_create,
>   	.ttm_tt_populate = &vmw_ttm_populate,
> @@ -780,7 +786,7 @@ struct ttm_bo_driver vmw_bo_driver = {
>   	.evict_flags = vmw_evict_flags,
>   	.move = vmw_move,
>   	.verify_access = vmw_verify_access,
> -	.move_notify = vmw_move_notify,
> +	.delete_mem_notify = vmw_delete_mem_notify,
>   	.swap_notify = vmw_swap_notify,
>   	.io_mem_reserve = &vmw_ttm_io_mem_reserve,
>   };
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 72f106b335e9..29f6a1d1c853 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -156,15 +156,9 @@ struct ttm_bo_driver {
>   			     struct file *filp);
>   
>   	/**
> -	 * Hook to notify driver about a driver move so it
> -	 * can do tiling things and book-keeping.
> -	 *
> -	 * @evict: whether this move is evicting the buffer from the graphics
> -	 * address space
> +	 * Hook to notify driver about a resource delete.
>   	 */
> -	void (*move_notify)(struct ttm_buffer_object *bo,
> -			    bool evict,
> -			    struct ttm_resource *new_mem);
> +	void (*delete_mem_notify)(struct ttm_buffer_object *bo);
>   
>   	/**
>   	 * notify the driver that we're about to swap out this bo
Daniel Vetter Oct. 28, 2020, 9:16 a.m. UTC | #2
On Wed, Oct 21, 2020 at 11:58:45AM +0200, Christian König wrote:
> Am 21.10.20 um 06:40 schrieb Dave Airlie:
> > From: Dave Airlie <airlied@redhat.com>
> > 
> > The move notify callback is only used in one place, this should
> > be removed in the future, but for now just rename it to the use
> > case which is to notify the driver that the GPU memory is to be
> > deleted.
> 
> Probably the right thing to do is to call the move callback with move(from,
> NULL) in this case as well.
> 
> And then driver can call the necessary function to throw away the backing
> store pipelined.
> 
> > 
> > Drivers can be cleaned up after this separately.
> > 
> > Signed-off-by: Dave Airlie <airlied@redhat.com>
> 
> Reviewed-by: Christian König <christian.koenig@amd.com>

Not sure where to best ask this question, but while reading code I
stumbled over the bo->pin_count check in ttm_bo_release(). And I'm
confused.

Allowing a bo to be pinned without holding a full reference to it feels
like a pretty serious bug. Where&why is that needed? I'm kinda tempted to
wrap this in a WARN_ON, just to make sure there's no surprises in usage
(and maybe warn in unpin if we drop the pin count with the refcount at 0
already).
-Daniel

> > ---
> >   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 +++++++-
> >   drivers/gpu/drm/drm_gem_vram_helper.c      |  8 +++-----
> >   drivers/gpu/drm/nouveau/nouveau_bo.c       |  8 +++++++-
> >   drivers/gpu/drm/qxl/qxl_ttm.c              |  7 ++++++-
> >   drivers/gpu/drm/radeon/radeon_ttm.c        |  8 +++++++-
> >   drivers/gpu/drm/ttm/ttm_bo.c               |  4 ++--
> >   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  8 +++++++-
> >   include/drm/ttm/ttm_bo_driver.h            | 10 ++--------
> >   8 files changed, 41 insertions(+), 20 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > index 87e10a212b8a..62f9194b1dd1 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> > @@ -1730,6 +1730,12 @@ static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
> >   	return ret;
> >   }
> > +static void
> > +amdgpu_bo_delete_mem_notify(struct ttm_buffer_object *bo)
> > +{
> > +	amdgpu_bo_move_notify(bo, false, NULL);
> > +}
> > +
> >   static struct ttm_bo_driver amdgpu_bo_driver = {
> >   	.ttm_tt_create = &amdgpu_ttm_tt_create,
> >   	.ttm_tt_populate = &amdgpu_ttm_tt_populate,
> > @@ -1739,7 +1745,7 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
> >   	.evict_flags = &amdgpu_evict_flags,
> >   	.move = &amdgpu_bo_move,
> >   	.verify_access = &amdgpu_verify_access,
> > -	.move_notify = &amdgpu_bo_move_notify,
> > +	.delete_mem_notify = &amdgpu_bo_delete_mem_notify,
> >   	.release_notify = &amdgpu_bo_release_notify,
> >   	.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
> >   	.io_mem_pfn = amdgpu_ttm_io_mem_pfn,
> > diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> > index 19087b22bdbb..9da823eb0edd 100644
> > --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> > +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> > @@ -949,9 +949,7 @@ static void bo_driver_evict_flags(struct ttm_buffer_object *bo,
> >   	drm_gem_vram_bo_driver_evict_flags(gbo, placement);
> >   }
> > -static void bo_driver_move_notify(struct ttm_buffer_object *bo,
> > -				  bool evict,
> > -				  struct ttm_resource *new_mem)
> > +static void bo_driver_delete_mem_notify(struct ttm_buffer_object *bo)
> >   {
> >   	struct drm_gem_vram_object *gbo;
> > @@ -961,7 +959,7 @@ static void bo_driver_move_notify(struct ttm_buffer_object *bo,
> >   	gbo = drm_gem_vram_of_bo(bo);
> > -	drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
> > +	drm_gem_vram_bo_driver_move_notify(gbo, false, NULL);
> >   }
> >   static int bo_driver_move(struct ttm_buffer_object *bo,
> > @@ -1002,7 +1000,7 @@ static struct ttm_bo_driver bo_driver = {
> >   	.eviction_valuable = ttm_bo_eviction_valuable,
> >   	.evict_flags = bo_driver_evict_flags,
> >   	.move = bo_driver_move,
> > -	.move_notify = bo_driver_move_notify,
> > +	.delete_mem_notify = bo_driver_delete_mem_notify,
> >   	.io_mem_reserve = bo_driver_io_mem_reserve,
> >   };
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > index 70b6f3b1ae85..acff82afe260 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> > @@ -1401,6 +1401,12 @@ nouveau_bo_fence(struct nouveau_bo *nvbo, struct nouveau_fence *fence, bool excl
> >   		dma_resv_add_shared_fence(resv, &fence->base);
> >   }
> > +static void
> > +nouveau_bo_delete_mem_notify(struct ttm_buffer_object *bo)
> > +{
> > +	nouveau_bo_move_ntfy(bo, false, NULL);
> > +}
> > +
> >   struct ttm_bo_driver nouveau_bo_driver = {
> >   	.ttm_tt_create = &nouveau_ttm_tt_create,
> >   	.ttm_tt_populate = &nouveau_ttm_tt_populate,
> > @@ -1408,7 +1414,7 @@ struct ttm_bo_driver nouveau_bo_driver = {
> >   	.ttm_tt_destroy = &nouveau_ttm_tt_destroy,
> >   	.eviction_valuable = ttm_bo_eviction_valuable,
> >   	.evict_flags = nouveau_bo_evict_flags,
> > -	.move_notify = nouveau_bo_move_ntfy,
> > +	.delete_mem_notify = nouveau_bo_delete_mem_notify,
> >   	.move = nouveau_bo_move,
> >   	.verify_access = nouveau_bo_verify_access,
> >   	.io_mem_reserve = &nouveau_ttm_io_mem_reserve,
> > diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> > index 1cc3c14bc684..b52a4563b47b 100644
> > --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> > +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> > @@ -166,6 +166,11 @@ static int qxl_bo_move(struct ttm_buffer_object *bo, bool evict,
> >   	return ret;
> >   }
> > +static void qxl_bo_delete_mem_notify(struct ttm_buffer_object *bo)
> > +{
> > +	qxl_bo_move_notify(bo, false, NULL);
> > +}
> > +
> >   static struct ttm_bo_driver qxl_bo_driver = {
> >   	.ttm_tt_create = &qxl_ttm_tt_create,
> >   	.ttm_tt_destroy = &qxl_ttm_backend_destroy,
> > @@ -173,7 +178,7 @@ static struct ttm_bo_driver qxl_bo_driver = {
> >   	.evict_flags = &qxl_evict_flags,
> >   	.move = &qxl_bo_move,
> >   	.io_mem_reserve = &qxl_ttm_io_mem_reserve,
> > -	.move_notify = &qxl_bo_move_notify,
> > +	.delete_mem_notify = &qxl_bo_delete_mem_notify,
> >   };
> >   static int qxl_ttm_init_mem_type(struct qxl_device *qdev,
> > diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> > index cd454e5c802f..321c09d20c6c 100644
> > --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> > +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> > @@ -824,6 +824,12 @@ bool radeon_ttm_tt_is_readonly(struct radeon_device *rdev,
> >   	return !!(gtt->userflags & RADEON_GEM_USERPTR_READONLY);
> >   }
> > +static void
> > +radeon_bo_delete_mem_notify(struct ttm_buffer_object *bo)
> > +{
> > +	radeon_bo_move_notify(bo, false, NULL);
> > +}
> > +
> >   static struct ttm_bo_driver radeon_bo_driver = {
> >   	.ttm_tt_create = &radeon_ttm_tt_create,
> >   	.ttm_tt_populate = &radeon_ttm_tt_populate,
> > @@ -833,7 +839,7 @@ static struct ttm_bo_driver radeon_bo_driver = {
> >   	.evict_flags = &radeon_evict_flags,
> >   	.move = &radeon_bo_move,
> >   	.verify_access = &radeon_verify_access,
> > -	.move_notify = &radeon_bo_move_notify,
> > +	.delete_mem_notify = &radeon_bo_delete_mem_notify,
> >   	.io_mem_reserve = &radeon_ttm_io_mem_reserve,
> >   };
> > diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> > index 2b578012cdef..e2afab3d97ee 100644
> > --- a/drivers/gpu/drm/ttm/ttm_bo.c
> > +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> > @@ -284,8 +284,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
> >   static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
> >   {
> > -	if (bo->bdev->driver->move_notify)
> > -		bo->bdev->driver->move_notify(bo, false, NULL);
> > +	if (bo->bdev->driver->delete_mem_notify)
> > +		bo->bdev->driver->delete_mem_notify(bo);
> >   	ttm_bo_tt_destroy(bo);
> >   	ttm_resource_free(bo, &bo->mem);
> > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> > index de25cf016be2..88be48ad0344 100644
> > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> > @@ -771,6 +771,12 @@ static int vmw_move(struct ttm_buffer_object *bo,
> >   	return ret;
> >   }
> > +static void
> > +vmw_delete_mem_notify(struct ttm_buffer_object *bo)
> > +{
> > +	vmw_move_notify(bo, false, NULL);
> > +}
> > +
> >   struct ttm_bo_driver vmw_bo_driver = {
> >   	.ttm_tt_create = &vmw_ttm_tt_create,
> >   	.ttm_tt_populate = &vmw_ttm_populate,
> > @@ -780,7 +786,7 @@ struct ttm_bo_driver vmw_bo_driver = {
> >   	.evict_flags = vmw_evict_flags,
> >   	.move = vmw_move,
> >   	.verify_access = vmw_verify_access,
> > -	.move_notify = vmw_move_notify,
> > +	.delete_mem_notify = vmw_delete_mem_notify,
> >   	.swap_notify = vmw_swap_notify,
> >   	.io_mem_reserve = &vmw_ttm_io_mem_reserve,
> >   };
> > diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> > index 72f106b335e9..29f6a1d1c853 100644
> > --- a/include/drm/ttm/ttm_bo_driver.h
> > +++ b/include/drm/ttm/ttm_bo_driver.h
> > @@ -156,15 +156,9 @@ struct ttm_bo_driver {
> >   			     struct file *filp);
> >   	/**
> > -	 * Hook to notify driver about a driver move so it
> > -	 * can do tiling things and book-keeping.
> > -	 *
> > -	 * @evict: whether this move is evicting the buffer from the graphics
> > -	 * address space
> > +	 * Hook to notify driver about a resource delete.
> >   	 */
> > -	void (*move_notify)(struct ttm_buffer_object *bo,
> > -			    bool evict,
> > -			    struct ttm_resource *new_mem);
> > +	void (*delete_mem_notify)(struct ttm_buffer_object *bo);
> >   	/**
> >   	 * notify the driver that we're about to swap out this bo
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
Christian König Oct. 28, 2020, 9:53 a.m. UTC | #3
Am 28.10.20 um 10:16 schrieb Daniel Vetter:
> On Wed, Oct 21, 2020 at 11:58:45AM +0200, Christian König wrote:
>> Am 21.10.20 um 06:40 schrieb Dave Airlie:
>>> From: Dave Airlie <airlied@redhat.com>
>>>
>>> The move notify callback is only used in one place, this should
>>> be removed in the future, but for now just rename it to the use
>>> case which is to notify the driver that the GPU memory is to be
>>> deleted.
>> Probably the right thing to do is to call the move callback with move(from,
>> NULL) in this case as well.
>>
>> And then driver can call the necessary function to throw away the backing
>> store pipelined.
>>
>>> Drivers can be cleaned up after this separately.
>>>
>>> Signed-off-by: Dave Airlie <airlied@redhat.com>
>> Reviewed-by: Christian König <christian.koenig@amd.com>
> Not sure where to best ask this question, but while reading code I
> stumbled over the bo->pin_count check in ttm_bo_release(). And I'm
> confused.
>
> Allowing a bo to be pinned without holding a full reference to it feels
> like a pretty serious bug. Where&why is that needed? I'm kinda tempted to
> wrap this in a WARN_ON, just to make sure there's no surprises in usage
> (and maybe warn in unpin if we drop the pin count with the refcount at 0
> already).

Yeah, I was wondering about that as well.

In general I don't see harm from the TTM perspective to drop the last 
reference while a BO is still pinned.

Only from the driver side it sounds like a bug to me, so I decided to 
not going to enforce this in TTM.

Christian.

> -Daniel
>
>>> ---
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 +++++++-
>>>    drivers/gpu/drm/drm_gem_vram_helper.c      |  8 +++-----
>>>    drivers/gpu/drm/nouveau/nouveau_bo.c       |  8 +++++++-
>>>    drivers/gpu/drm/qxl/qxl_ttm.c              |  7 ++++++-
>>>    drivers/gpu/drm/radeon/radeon_ttm.c        |  8 +++++++-
>>>    drivers/gpu/drm/ttm/ttm_bo.c               |  4 ++--
>>>    drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  8 +++++++-
>>>    include/drm/ttm/ttm_bo_driver.h            | 10 ++--------
>>>    8 files changed, 41 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> index 87e10a212b8a..62f9194b1dd1 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
>>> @@ -1730,6 +1730,12 @@ static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
>>>    	return ret;
>>>    }
>>> +static void
>>> +amdgpu_bo_delete_mem_notify(struct ttm_buffer_object *bo)
>>> +{
>>> +	amdgpu_bo_move_notify(bo, false, NULL);
>>> +}
>>> +
>>>    static struct ttm_bo_driver amdgpu_bo_driver = {
>>>    	.ttm_tt_create = &amdgpu_ttm_tt_create,
>>>    	.ttm_tt_populate = &amdgpu_ttm_tt_populate,
>>> @@ -1739,7 +1745,7 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
>>>    	.evict_flags = &amdgpu_evict_flags,
>>>    	.move = &amdgpu_bo_move,
>>>    	.verify_access = &amdgpu_verify_access,
>>> -	.move_notify = &amdgpu_bo_move_notify,
>>> +	.delete_mem_notify = &amdgpu_bo_delete_mem_notify,
>>>    	.release_notify = &amdgpu_bo_release_notify,
>>>    	.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
>>>    	.io_mem_pfn = amdgpu_ttm_io_mem_pfn,
>>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
>>> index 19087b22bdbb..9da823eb0edd 100644
>>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
>>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
>>> @@ -949,9 +949,7 @@ static void bo_driver_evict_flags(struct ttm_buffer_object *bo,
>>>    	drm_gem_vram_bo_driver_evict_flags(gbo, placement);
>>>    }
>>> -static void bo_driver_move_notify(struct ttm_buffer_object *bo,
>>> -				  bool evict,
>>> -				  struct ttm_resource *new_mem)
>>> +static void bo_driver_delete_mem_notify(struct ttm_buffer_object *bo)
>>>    {
>>>    	struct drm_gem_vram_object *gbo;
>>> @@ -961,7 +959,7 @@ static void bo_driver_move_notify(struct ttm_buffer_object *bo,
>>>    	gbo = drm_gem_vram_of_bo(bo);
>>> -	drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
>>> +	drm_gem_vram_bo_driver_move_notify(gbo, false, NULL);
>>>    }
>>>    static int bo_driver_move(struct ttm_buffer_object *bo,
>>> @@ -1002,7 +1000,7 @@ static struct ttm_bo_driver bo_driver = {
>>>    	.eviction_valuable = ttm_bo_eviction_valuable,
>>>    	.evict_flags = bo_driver_evict_flags,
>>>    	.move = bo_driver_move,
>>> -	.move_notify = bo_driver_move_notify,
>>> +	.delete_mem_notify = bo_driver_delete_mem_notify,
>>>    	.io_mem_reserve = bo_driver_io_mem_reserve,
>>>    };
>>> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
>>> index 70b6f3b1ae85..acff82afe260 100644
>>> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
>>> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
>>> @@ -1401,6 +1401,12 @@ nouveau_bo_fence(struct nouveau_bo *nvbo, struct nouveau_fence *fence, bool excl
>>>    		dma_resv_add_shared_fence(resv, &fence->base);
>>>    }
>>> +static void
>>> +nouveau_bo_delete_mem_notify(struct ttm_buffer_object *bo)
>>> +{
>>> +	nouveau_bo_move_ntfy(bo, false, NULL);
>>> +}
>>> +
>>>    struct ttm_bo_driver nouveau_bo_driver = {
>>>    	.ttm_tt_create = &nouveau_ttm_tt_create,
>>>    	.ttm_tt_populate = &nouveau_ttm_tt_populate,
>>> @@ -1408,7 +1414,7 @@ struct ttm_bo_driver nouveau_bo_driver = {
>>>    	.ttm_tt_destroy = &nouveau_ttm_tt_destroy,
>>>    	.eviction_valuable = ttm_bo_eviction_valuable,
>>>    	.evict_flags = nouveau_bo_evict_flags,
>>> -	.move_notify = nouveau_bo_move_ntfy,
>>> +	.delete_mem_notify = nouveau_bo_delete_mem_notify,
>>>    	.move = nouveau_bo_move,
>>>    	.verify_access = nouveau_bo_verify_access,
>>>    	.io_mem_reserve = &nouveau_ttm_io_mem_reserve,
>>> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
>>> index 1cc3c14bc684..b52a4563b47b 100644
>>> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
>>> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
>>> @@ -166,6 +166,11 @@ static int qxl_bo_move(struct ttm_buffer_object *bo, bool evict,
>>>    	return ret;
>>>    }
>>> +static void qxl_bo_delete_mem_notify(struct ttm_buffer_object *bo)
>>> +{
>>> +	qxl_bo_move_notify(bo, false, NULL);
>>> +}
>>> +
>>>    static struct ttm_bo_driver qxl_bo_driver = {
>>>    	.ttm_tt_create = &qxl_ttm_tt_create,
>>>    	.ttm_tt_destroy = &qxl_ttm_backend_destroy,
>>> @@ -173,7 +178,7 @@ static struct ttm_bo_driver qxl_bo_driver = {
>>>    	.evict_flags = &qxl_evict_flags,
>>>    	.move = &qxl_bo_move,
>>>    	.io_mem_reserve = &qxl_ttm_io_mem_reserve,
>>> -	.move_notify = &qxl_bo_move_notify,
>>> +	.delete_mem_notify = &qxl_bo_delete_mem_notify,
>>>    };
>>>    static int qxl_ttm_init_mem_type(struct qxl_device *qdev,
>>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
>>> index cd454e5c802f..321c09d20c6c 100644
>>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
>>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
>>> @@ -824,6 +824,12 @@ bool radeon_ttm_tt_is_readonly(struct radeon_device *rdev,
>>>    	return !!(gtt->userflags & RADEON_GEM_USERPTR_READONLY);
>>>    }
>>> +static void
>>> +radeon_bo_delete_mem_notify(struct ttm_buffer_object *bo)
>>> +{
>>> +	radeon_bo_move_notify(bo, false, NULL);
>>> +}
>>> +
>>>    static struct ttm_bo_driver radeon_bo_driver = {
>>>    	.ttm_tt_create = &radeon_ttm_tt_create,
>>>    	.ttm_tt_populate = &radeon_ttm_tt_populate,
>>> @@ -833,7 +839,7 @@ static struct ttm_bo_driver radeon_bo_driver = {
>>>    	.evict_flags = &radeon_evict_flags,
>>>    	.move = &radeon_bo_move,
>>>    	.verify_access = &radeon_verify_access,
>>> -	.move_notify = &radeon_bo_move_notify,
>>> +	.delete_mem_notify = &radeon_bo_delete_mem_notify,
>>>    	.io_mem_reserve = &radeon_ttm_io_mem_reserve,
>>>    };
>>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
>>> index 2b578012cdef..e2afab3d97ee 100644
>>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
>>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
>>> @@ -284,8 +284,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
>>>    static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
>>>    {
>>> -	if (bo->bdev->driver->move_notify)
>>> -		bo->bdev->driver->move_notify(bo, false, NULL);
>>> +	if (bo->bdev->driver->delete_mem_notify)
>>> +		bo->bdev->driver->delete_mem_notify(bo);
>>>    	ttm_bo_tt_destroy(bo);
>>>    	ttm_resource_free(bo, &bo->mem);
>>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>>> index de25cf016be2..88be48ad0344 100644
>>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
>>> @@ -771,6 +771,12 @@ static int vmw_move(struct ttm_buffer_object *bo,
>>>    	return ret;
>>>    }
>>> +static void
>>> +vmw_delete_mem_notify(struct ttm_buffer_object *bo)
>>> +{
>>> +	vmw_move_notify(bo, false, NULL);
>>> +}
>>> +
>>>    struct ttm_bo_driver vmw_bo_driver = {
>>>    	.ttm_tt_create = &vmw_ttm_tt_create,
>>>    	.ttm_tt_populate = &vmw_ttm_populate,
>>> @@ -780,7 +786,7 @@ struct ttm_bo_driver vmw_bo_driver = {
>>>    	.evict_flags = vmw_evict_flags,
>>>    	.move = vmw_move,
>>>    	.verify_access = vmw_verify_access,
>>> -	.move_notify = vmw_move_notify,
>>> +	.delete_mem_notify = vmw_delete_mem_notify,
>>>    	.swap_notify = vmw_swap_notify,
>>>    	.io_mem_reserve = &vmw_ttm_io_mem_reserve,
>>>    };
>>> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
>>> index 72f106b335e9..29f6a1d1c853 100644
>>> --- a/include/drm/ttm/ttm_bo_driver.h
>>> +++ b/include/drm/ttm/ttm_bo_driver.h
>>> @@ -156,15 +156,9 @@ struct ttm_bo_driver {
>>>    			     struct file *filp);
>>>    	/**
>>> -	 * Hook to notify driver about a driver move so it
>>> -	 * can do tiling things and book-keeping.
>>> -	 *
>>> -	 * @evict: whether this move is evicting the buffer from the graphics
>>> -	 * address space
>>> +	 * Hook to notify driver about a resource delete.
>>>    	 */
>>> -	void (*move_notify)(struct ttm_buffer_object *bo,
>>> -			    bool evict,
>>> -			    struct ttm_resource *new_mem);
>>> +	void (*delete_mem_notify)(struct ttm_buffer_object *bo);
>>>    	/**
>>>    	 * notify the driver that we're about to swap out this bo
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7Ce14a4829f0fd472cc9fd08d87b222578%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637394733878075272%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=R7%2FqQXVHnwsD%2Fj21FS%2FYXMsfFAA7%2BI84O54b6jP4bHs%3D&amp;reserved=0
Daniel Vetter Oct. 28, 2020, 10:19 a.m. UTC | #4
On Wed, Oct 28, 2020 at 10:53 AM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 28.10.20 um 10:16 schrieb Daniel Vetter:
> > On Wed, Oct 21, 2020 at 11:58:45AM +0200, Christian König wrote:
> >> Am 21.10.20 um 06:40 schrieb Dave Airlie:
> >>> From: Dave Airlie <airlied@redhat.com>
> >>>
> >>> The move notify callback is only used in one place, this should
> >>> be removed in the future, but for now just rename it to the use
> >>> case which is to notify the driver that the GPU memory is to be
> >>> deleted.
> >> Probably the right thing to do is to call the move callback with move(from,
> >> NULL) in this case as well.
> >>
> >> And then driver can call the necessary function to throw away the backing
> >> store pipelined.
> >>
> >>> Drivers can be cleaned up after this separately.
> >>>
> >>> Signed-off-by: Dave Airlie <airlied@redhat.com>
> >> Reviewed-by: Christian König <christian.koenig@amd.com>
> > Not sure where to best ask this question, but while reading code I
> > stumbled over the bo->pin_count check in ttm_bo_release(). And I'm
> > confused.
> >
> > Allowing a bo to be pinned without holding a full reference to it feels
> > like a pretty serious bug. Where&why is that needed? I'm kinda tempted to
> > wrap this in a WARN_ON, just to make sure there's no surprises in usage
> > (and maybe warn in unpin if we drop the pin count with the refcount at 0
> > already).
>
> Yeah, I was wondering about that as well.
>
> In general I don't see harm from the TTM perspective to drop the last
> reference while a BO is still pinned.
>
> Only from the driver side it sounds like a bug to me, so I decided to
> not going to enforce this in TTM.

Yeah implementing it doesn't sound tricky, but it also doesn't sound
like something anyone would ever want to do intentionally. I think
I'll stitch some WARN_ONs together.
-Daniel

>
> Christian.
>
> > -Daniel
> >
> >>> ---
> >>>    drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c    |  8 +++++++-
> >>>    drivers/gpu/drm/drm_gem_vram_helper.c      |  8 +++-----
> >>>    drivers/gpu/drm/nouveau/nouveau_bo.c       |  8 +++++++-
> >>>    drivers/gpu/drm/qxl/qxl_ttm.c              |  7 ++++++-
> >>>    drivers/gpu/drm/radeon/radeon_ttm.c        |  8 +++++++-
> >>>    drivers/gpu/drm/ttm/ttm_bo.c               |  4 ++--
> >>>    drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c |  8 +++++++-
> >>>    include/drm/ttm/ttm_bo_driver.h            | 10 ++--------
> >>>    8 files changed, 41 insertions(+), 20 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >>> index 87e10a212b8a..62f9194b1dd1 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
> >>> @@ -1730,6 +1730,12 @@ static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
> >>>     return ret;
> >>>    }
> >>> +static void
> >>> +amdgpu_bo_delete_mem_notify(struct ttm_buffer_object *bo)
> >>> +{
> >>> +   amdgpu_bo_move_notify(bo, false, NULL);
> >>> +}
> >>> +
> >>>    static struct ttm_bo_driver amdgpu_bo_driver = {
> >>>     .ttm_tt_create = &amdgpu_ttm_tt_create,
> >>>     .ttm_tt_populate = &amdgpu_ttm_tt_populate,
> >>> @@ -1739,7 +1745,7 @@ static struct ttm_bo_driver amdgpu_bo_driver = {
> >>>     .evict_flags = &amdgpu_evict_flags,
> >>>     .move = &amdgpu_bo_move,
> >>>     .verify_access = &amdgpu_verify_access,
> >>> -   .move_notify = &amdgpu_bo_move_notify,
> >>> +   .delete_mem_notify = &amdgpu_bo_delete_mem_notify,
> >>>     .release_notify = &amdgpu_bo_release_notify,
> >>>     .io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
> >>>     .io_mem_pfn = amdgpu_ttm_io_mem_pfn,
> >>> diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
> >>> index 19087b22bdbb..9da823eb0edd 100644
> >>> --- a/drivers/gpu/drm/drm_gem_vram_helper.c
> >>> +++ b/drivers/gpu/drm/drm_gem_vram_helper.c
> >>> @@ -949,9 +949,7 @@ static void bo_driver_evict_flags(struct ttm_buffer_object *bo,
> >>>     drm_gem_vram_bo_driver_evict_flags(gbo, placement);
> >>>    }
> >>> -static void bo_driver_move_notify(struct ttm_buffer_object *bo,
> >>> -                             bool evict,
> >>> -                             struct ttm_resource *new_mem)
> >>> +static void bo_driver_delete_mem_notify(struct ttm_buffer_object *bo)
> >>>    {
> >>>     struct drm_gem_vram_object *gbo;
> >>> @@ -961,7 +959,7 @@ static void bo_driver_move_notify(struct ttm_buffer_object *bo,
> >>>     gbo = drm_gem_vram_of_bo(bo);
> >>> -   drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
> >>> +   drm_gem_vram_bo_driver_move_notify(gbo, false, NULL);
> >>>    }
> >>>    static int bo_driver_move(struct ttm_buffer_object *bo,
> >>> @@ -1002,7 +1000,7 @@ static struct ttm_bo_driver bo_driver = {
> >>>     .eviction_valuable = ttm_bo_eviction_valuable,
> >>>     .evict_flags = bo_driver_evict_flags,
> >>>     .move = bo_driver_move,
> >>> -   .move_notify = bo_driver_move_notify,
> >>> +   .delete_mem_notify = bo_driver_delete_mem_notify,
> >>>     .io_mem_reserve = bo_driver_io_mem_reserve,
> >>>    };
> >>> diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
> >>> index 70b6f3b1ae85..acff82afe260 100644
> >>> --- a/drivers/gpu/drm/nouveau/nouveau_bo.c
> >>> +++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
> >>> @@ -1401,6 +1401,12 @@ nouveau_bo_fence(struct nouveau_bo *nvbo, struct nouveau_fence *fence, bool excl
> >>>             dma_resv_add_shared_fence(resv, &fence->base);
> >>>    }
> >>> +static void
> >>> +nouveau_bo_delete_mem_notify(struct ttm_buffer_object *bo)
> >>> +{
> >>> +   nouveau_bo_move_ntfy(bo, false, NULL);
> >>> +}
> >>> +
> >>>    struct ttm_bo_driver nouveau_bo_driver = {
> >>>     .ttm_tt_create = &nouveau_ttm_tt_create,
> >>>     .ttm_tt_populate = &nouveau_ttm_tt_populate,
> >>> @@ -1408,7 +1414,7 @@ struct ttm_bo_driver nouveau_bo_driver = {
> >>>     .ttm_tt_destroy = &nouveau_ttm_tt_destroy,
> >>>     .eviction_valuable = ttm_bo_eviction_valuable,
> >>>     .evict_flags = nouveau_bo_evict_flags,
> >>> -   .move_notify = nouveau_bo_move_ntfy,
> >>> +   .delete_mem_notify = nouveau_bo_delete_mem_notify,
> >>>     .move = nouveau_bo_move,
> >>>     .verify_access = nouveau_bo_verify_access,
> >>>     .io_mem_reserve = &nouveau_ttm_io_mem_reserve,
> >>> diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
> >>> index 1cc3c14bc684..b52a4563b47b 100644
> >>> --- a/drivers/gpu/drm/qxl/qxl_ttm.c
> >>> +++ b/drivers/gpu/drm/qxl/qxl_ttm.c
> >>> @@ -166,6 +166,11 @@ static int qxl_bo_move(struct ttm_buffer_object *bo, bool evict,
> >>>     return ret;
> >>>    }
> >>> +static void qxl_bo_delete_mem_notify(struct ttm_buffer_object *bo)
> >>> +{
> >>> +   qxl_bo_move_notify(bo, false, NULL);
> >>> +}
> >>> +
> >>>    static struct ttm_bo_driver qxl_bo_driver = {
> >>>     .ttm_tt_create = &qxl_ttm_tt_create,
> >>>     .ttm_tt_destroy = &qxl_ttm_backend_destroy,
> >>> @@ -173,7 +178,7 @@ static struct ttm_bo_driver qxl_bo_driver = {
> >>>     .evict_flags = &qxl_evict_flags,
> >>>     .move = &qxl_bo_move,
> >>>     .io_mem_reserve = &qxl_ttm_io_mem_reserve,
> >>> -   .move_notify = &qxl_bo_move_notify,
> >>> +   .delete_mem_notify = &qxl_bo_delete_mem_notify,
> >>>    };
> >>>    static int qxl_ttm_init_mem_type(struct qxl_device *qdev,
> >>> diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
> >>> index cd454e5c802f..321c09d20c6c 100644
> >>> --- a/drivers/gpu/drm/radeon/radeon_ttm.c
> >>> +++ b/drivers/gpu/drm/radeon/radeon_ttm.c
> >>> @@ -824,6 +824,12 @@ bool radeon_ttm_tt_is_readonly(struct radeon_device *rdev,
> >>>     return !!(gtt->userflags & RADEON_GEM_USERPTR_READONLY);
> >>>    }
> >>> +static void
> >>> +radeon_bo_delete_mem_notify(struct ttm_buffer_object *bo)
> >>> +{
> >>> +   radeon_bo_move_notify(bo, false, NULL);
> >>> +}
> >>> +
> >>>    static struct ttm_bo_driver radeon_bo_driver = {
> >>>     .ttm_tt_create = &radeon_ttm_tt_create,
> >>>     .ttm_tt_populate = &radeon_ttm_tt_populate,
> >>> @@ -833,7 +839,7 @@ static struct ttm_bo_driver radeon_bo_driver = {
> >>>     .evict_flags = &radeon_evict_flags,
> >>>     .move = &radeon_bo_move,
> >>>     .verify_access = &radeon_verify_access,
> >>> -   .move_notify = &radeon_bo_move_notify,
> >>> +   .delete_mem_notify = &radeon_bo_delete_mem_notify,
> >>>     .io_mem_reserve = &radeon_ttm_io_mem_reserve,
> >>>    };
> >>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> >>> index 2b578012cdef..e2afab3d97ee 100644
> >>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> >>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> >>> @@ -284,8 +284,8 @@ static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
> >>>    static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
> >>>    {
> >>> -   if (bo->bdev->driver->move_notify)
> >>> -           bo->bdev->driver->move_notify(bo, false, NULL);
> >>> +   if (bo->bdev->driver->delete_mem_notify)
> >>> +           bo->bdev->driver->delete_mem_notify(bo);
> >>>     ttm_bo_tt_destroy(bo);
> >>>     ttm_resource_free(bo, &bo->mem);
> >>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> >>> index de25cf016be2..88be48ad0344 100644
> >>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> >>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
> >>> @@ -771,6 +771,12 @@ static int vmw_move(struct ttm_buffer_object *bo,
> >>>     return ret;
> >>>    }
> >>> +static void
> >>> +vmw_delete_mem_notify(struct ttm_buffer_object *bo)
> >>> +{
> >>> +   vmw_move_notify(bo, false, NULL);
> >>> +}
> >>> +
> >>>    struct ttm_bo_driver vmw_bo_driver = {
> >>>     .ttm_tt_create = &vmw_ttm_tt_create,
> >>>     .ttm_tt_populate = &vmw_ttm_populate,
> >>> @@ -780,7 +786,7 @@ struct ttm_bo_driver vmw_bo_driver = {
> >>>     .evict_flags = vmw_evict_flags,
> >>>     .move = vmw_move,
> >>>     .verify_access = vmw_verify_access,
> >>> -   .move_notify = vmw_move_notify,
> >>> +   .delete_mem_notify = vmw_delete_mem_notify,
> >>>     .swap_notify = vmw_swap_notify,
> >>>     .io_mem_reserve = &vmw_ttm_io_mem_reserve,
> >>>    };
> >>> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> >>> index 72f106b335e9..29f6a1d1c853 100644
> >>> --- a/include/drm/ttm/ttm_bo_driver.h
> >>> +++ b/include/drm/ttm/ttm_bo_driver.h
> >>> @@ -156,15 +156,9 @@ struct ttm_bo_driver {
> >>>                          struct file *filp);
> >>>     /**
> >>> -    * Hook to notify driver about a driver move so it
> >>> -    * can do tiling things and book-keeping.
> >>> -    *
> >>> -    * @evict: whether this move is evicting the buffer from the graphics
> >>> -    * address space
> >>> +    * Hook to notify driver about a resource delete.
> >>>      */
> >>> -   void (*move_notify)(struct ttm_buffer_object *bo,
> >>> -                       bool evict,
> >>> -                       struct ttm_resource *new_mem);
> >>> +   void (*delete_mem_notify)(struct ttm_buffer_object *bo);
> >>>     /**
> >>>      * notify the driver that we're about to swap out this bo
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Fdri-devel&amp;data=04%7C01%7Cchristian.koenig%40amd.com%7Ce14a4829f0fd472cc9fd08d87b222578%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637394733878075272%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=R7%2FqQXVHnwsD%2Fj21FS%2FYXMsfFAA7%2BI84O54b6jP4bHs%3D&amp;reserved=0
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index 87e10a212b8a..62f9194b1dd1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -1730,6 +1730,12 @@  static int amdgpu_ttm_access_memory(struct ttm_buffer_object *bo,
 	return ret;
 }
 
+static void
+amdgpu_bo_delete_mem_notify(struct ttm_buffer_object *bo)
+{
+	amdgpu_bo_move_notify(bo, false, NULL);
+}
+
 static struct ttm_bo_driver amdgpu_bo_driver = {
 	.ttm_tt_create = &amdgpu_ttm_tt_create,
 	.ttm_tt_populate = &amdgpu_ttm_tt_populate,
@@ -1739,7 +1745,7 @@  static struct ttm_bo_driver amdgpu_bo_driver = {
 	.evict_flags = &amdgpu_evict_flags,
 	.move = &amdgpu_bo_move,
 	.verify_access = &amdgpu_verify_access,
-	.move_notify = &amdgpu_bo_move_notify,
+	.delete_mem_notify = &amdgpu_bo_delete_mem_notify,
 	.release_notify = &amdgpu_bo_release_notify,
 	.io_mem_reserve = &amdgpu_ttm_io_mem_reserve,
 	.io_mem_pfn = amdgpu_ttm_io_mem_pfn,
diff --git a/drivers/gpu/drm/drm_gem_vram_helper.c b/drivers/gpu/drm/drm_gem_vram_helper.c
index 19087b22bdbb..9da823eb0edd 100644
--- a/drivers/gpu/drm/drm_gem_vram_helper.c
+++ b/drivers/gpu/drm/drm_gem_vram_helper.c
@@ -949,9 +949,7 @@  static void bo_driver_evict_flags(struct ttm_buffer_object *bo,
 	drm_gem_vram_bo_driver_evict_flags(gbo, placement);
 }
 
-static void bo_driver_move_notify(struct ttm_buffer_object *bo,
-				  bool evict,
-				  struct ttm_resource *new_mem)
+static void bo_driver_delete_mem_notify(struct ttm_buffer_object *bo)
 {
 	struct drm_gem_vram_object *gbo;
 
@@ -961,7 +959,7 @@  static void bo_driver_move_notify(struct ttm_buffer_object *bo,
 
 	gbo = drm_gem_vram_of_bo(bo);
 
-	drm_gem_vram_bo_driver_move_notify(gbo, evict, new_mem);
+	drm_gem_vram_bo_driver_move_notify(gbo, false, NULL);
 }
 
 static int bo_driver_move(struct ttm_buffer_object *bo,
@@ -1002,7 +1000,7 @@  static struct ttm_bo_driver bo_driver = {
 	.eviction_valuable = ttm_bo_eviction_valuable,
 	.evict_flags = bo_driver_evict_flags,
 	.move = bo_driver_move,
-	.move_notify = bo_driver_move_notify,
+	.delete_mem_notify = bo_driver_delete_mem_notify,
 	.io_mem_reserve = bo_driver_io_mem_reserve,
 };
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_bo.c b/drivers/gpu/drm/nouveau/nouveau_bo.c
index 70b6f3b1ae85..acff82afe260 100644
--- a/drivers/gpu/drm/nouveau/nouveau_bo.c
+++ b/drivers/gpu/drm/nouveau/nouveau_bo.c
@@ -1401,6 +1401,12 @@  nouveau_bo_fence(struct nouveau_bo *nvbo, struct nouveau_fence *fence, bool excl
 		dma_resv_add_shared_fence(resv, &fence->base);
 }
 
+static void
+nouveau_bo_delete_mem_notify(struct ttm_buffer_object *bo)
+{
+	nouveau_bo_move_ntfy(bo, false, NULL);
+}
+
 struct ttm_bo_driver nouveau_bo_driver = {
 	.ttm_tt_create = &nouveau_ttm_tt_create,
 	.ttm_tt_populate = &nouveau_ttm_tt_populate,
@@ -1408,7 +1414,7 @@  struct ttm_bo_driver nouveau_bo_driver = {
 	.ttm_tt_destroy = &nouveau_ttm_tt_destroy,
 	.eviction_valuable = ttm_bo_eviction_valuable,
 	.evict_flags = nouveau_bo_evict_flags,
-	.move_notify = nouveau_bo_move_ntfy,
+	.delete_mem_notify = nouveau_bo_delete_mem_notify,
 	.move = nouveau_bo_move,
 	.verify_access = nouveau_bo_verify_access,
 	.io_mem_reserve = &nouveau_ttm_io_mem_reserve,
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 1cc3c14bc684..b52a4563b47b 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -166,6 +166,11 @@  static int qxl_bo_move(struct ttm_buffer_object *bo, bool evict,
 	return ret;
 }
 
+static void qxl_bo_delete_mem_notify(struct ttm_buffer_object *bo)
+{
+	qxl_bo_move_notify(bo, false, NULL);
+}
+
 static struct ttm_bo_driver qxl_bo_driver = {
 	.ttm_tt_create = &qxl_ttm_tt_create,
 	.ttm_tt_destroy = &qxl_ttm_backend_destroy,
@@ -173,7 +178,7 @@  static struct ttm_bo_driver qxl_bo_driver = {
 	.evict_flags = &qxl_evict_flags,
 	.move = &qxl_bo_move,
 	.io_mem_reserve = &qxl_ttm_io_mem_reserve,
-	.move_notify = &qxl_bo_move_notify,
+	.delete_mem_notify = &qxl_bo_delete_mem_notify,
 };
 
 static int qxl_ttm_init_mem_type(struct qxl_device *qdev,
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index cd454e5c802f..321c09d20c6c 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -824,6 +824,12 @@  bool radeon_ttm_tt_is_readonly(struct radeon_device *rdev,
 	return !!(gtt->userflags & RADEON_GEM_USERPTR_READONLY);
 }
 
+static void
+radeon_bo_delete_mem_notify(struct ttm_buffer_object *bo)
+{
+	radeon_bo_move_notify(bo, false, NULL);
+}
+
 static struct ttm_bo_driver radeon_bo_driver = {
 	.ttm_tt_create = &radeon_ttm_tt_create,
 	.ttm_tt_populate = &radeon_ttm_tt_populate,
@@ -833,7 +839,7 @@  static struct ttm_bo_driver radeon_bo_driver = {
 	.evict_flags = &radeon_evict_flags,
 	.move = &radeon_bo_move,
 	.verify_access = &radeon_verify_access,
-	.move_notify = &radeon_bo_move_notify,
+	.delete_mem_notify = &radeon_bo_delete_mem_notify,
 	.io_mem_reserve = &radeon_ttm_io_mem_reserve,
 };
 
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 2b578012cdef..e2afab3d97ee 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -284,8 +284,8 @@  static int ttm_bo_handle_move_mem(struct ttm_buffer_object *bo,
 
 static void ttm_bo_cleanup_memtype_use(struct ttm_buffer_object *bo)
 {
-	if (bo->bdev->driver->move_notify)
-		bo->bdev->driver->move_notify(bo, false, NULL);
+	if (bo->bdev->driver->delete_mem_notify)
+		bo->bdev->driver->delete_mem_notify(bo);
 
 	ttm_bo_tt_destroy(bo);
 	ttm_resource_free(bo, &bo->mem);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
index de25cf016be2..88be48ad0344 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_buffer.c
@@ -771,6 +771,12 @@  static int vmw_move(struct ttm_buffer_object *bo,
 	return ret;
 }
 
+static void
+vmw_delete_mem_notify(struct ttm_buffer_object *bo)
+{
+	vmw_move_notify(bo, false, NULL);
+}
+
 struct ttm_bo_driver vmw_bo_driver = {
 	.ttm_tt_create = &vmw_ttm_tt_create,
 	.ttm_tt_populate = &vmw_ttm_populate,
@@ -780,7 +786,7 @@  struct ttm_bo_driver vmw_bo_driver = {
 	.evict_flags = vmw_evict_flags,
 	.move = vmw_move,
 	.verify_access = vmw_verify_access,
-	.move_notify = vmw_move_notify,
+	.delete_mem_notify = vmw_delete_mem_notify,
 	.swap_notify = vmw_swap_notify,
 	.io_mem_reserve = &vmw_ttm_io_mem_reserve,
 };
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 72f106b335e9..29f6a1d1c853 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -156,15 +156,9 @@  struct ttm_bo_driver {
 			     struct file *filp);
 
 	/**
-	 * Hook to notify driver about a driver move so it
-	 * can do tiling things and book-keeping.
-	 *
-	 * @evict: whether this move is evicting the buffer from the graphics
-	 * address space
+	 * Hook to notify driver about a resource delete.
 	 */
-	void (*move_notify)(struct ttm_buffer_object *bo,
-			    bool evict,
-			    struct ttm_resource *new_mem);
+	void (*delete_mem_notify)(struct ttm_buffer_object *bo);
 
 	/**
 	 * notify the driver that we're about to swap out this bo