On Fri, Oct 2, 2020 at 11:48 AM Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> wrote:
>
> Am 02.10.20 um 11:16 schrieb Daniel Vetter:
> > On Fri, Oct 2, 2020 at 11:07 AM Daniel Vetter <daniel@xxxxxxxx> wrote:
> >> On Fri, Oct 2, 2020 at 10:47 AM Christian König
> >> <ckoenig.leichtzumerken@xxxxxxxxx> wrote:
> >>> This function also works with whole device and not individual BOs.
> >>>
> >>> Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> >> This doesn't operate on the device, but on the resource_manager. It's
> >> just stuck using the old api of (bdev, man_type) pairs instead of the
> >> pointer. I think switching to the pointer, renaming it to
> >> ttm_resource_manager_evict and moving it to ttm_resource.[hc] is a lot
> >> cleaner. Or did I again get confused somewhere :-)
> > On second thought, we have that already, but with a slightly tedious
> > name of _force_list_clean. Can't we just replace this function here
> > with that other one?
>
> I was thinking the same, but than thought that we probably don't want to
> have the error checking duplicated into all drivers.
>
> On the other hand the error checking is probably complete overkill.
>
> I mean we shouldn't a driver if it wants to shot in his own foot,
> instead get some popcorn and watch the show :)
Yeah the only driver which does fancy games with disabling/enabling
ttm_res_man at runtime is vmwgfx. And for that I think making that
more explicit should be a good thing. Everyone else is probably better
off with an Oops when they get this wrong while hacking around,
instead of silently doing nothing (since most callers don't bother
checking error value of this thing anyway).
-Daniel
> Christian.
>
> > -Daniel
> >
> >>> ---
> >>> drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 2 +-
> >>> drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 2 +-
> >>> drivers/gpu/drm/nouveau/nouveau_drm.c | 2 +-
> >>> drivers/gpu/drm/qxl/qxl_object.c | 4 ++--
> >>> drivers/gpu/drm/radeon/radeon_object.c | 2 +-
> >>> drivers/gpu/drm/ttm/ttm_bo.c | 18 ------------------
> >>> drivers/gpu/drm/ttm/ttm_device.c | 18 ++++++++++++++++++
> >>> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 6 +++---
> >>> include/drm/ttm/ttm_bo_api.h | 20 --------------------
> >>> include/drm/ttm/ttm_device.h | 20 ++++++++++++++++++++
> >>> 10 files changed, 47 insertions(+), 47 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> >>> index abe0c2729e1c..bb6ab41aada0 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> >>> @@ -1327,7 +1327,7 @@ static int amdgpu_debugfs_evict_gtt(struct seq_file *m, void *data)
> >>> return r;
> >>> }
> >>>
> >>> - seq_printf(m, "(%d)\n", ttm_bo_evict_mm(&adev->mman.bdev, TTM_PL_TT));
> >>> + seq_printf(m, "(%d)\n", ttm_device_evict_mm(&adev->mman.bdev, TTM_PL_TT));
> >>>
> >>> pm_runtime_mark_last_busy(dev->dev);
> >>> pm_runtime_put_autosuspend(dev->dev);
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >>> index 964f9512dd6e..7fc5fc12dd6e 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
> >>> @@ -1026,7 +1026,7 @@ int amdgpu_bo_evict_vram(struct amdgpu_device *adev)
> >>> return 0;
> >>> }
> >>> #endif
> >>> - return ttm_bo_evict_mm(&adev->mman.bdev, TTM_PL_VRAM);
> >>> + return ttm_device_evict_mm(&adev->mman.bdev, TTM_PL_VRAM);
> >>> }
> >>>
> >>> static const char *amdgpu_vram_names[] = {
> >>> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> >>> index 72640bca1617..351eff5d038b 100644
> >>> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> >>> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> >>> @@ -836,7 +836,7 @@ nouveau_do_suspend(struct drm_device *dev, bool runtime)
> >>> }
> >>>
> >>> NV_DEBUG(drm, "evicting buffers...\n");
> >>> - ttm_bo_evict_mm(&drm->ttm.bdev, TTM_PL_VRAM);
> >>> + ttm_device_evict_mm(&drm->ttm.bdev, TTM_PL_VRAM);
> >>>
> >>> NV_DEBUG(drm, "waiting for kernel channels to go idle...\n");
> >>> if (drm->cechan) {
> >>> diff --git a/drivers/gpu/drm/qxl/qxl_object.c b/drivers/gpu/drm/qxl/qxl_object.c
> >>> index 5ba8aac00f5c..0ede91617506 100644
> >>> --- a/drivers/gpu/drm/qxl/qxl_object.c
> >>> +++ b/drivers/gpu/drm/qxl/qxl_object.c
> >>> @@ -351,10 +351,10 @@ int qxl_bo_check_id(struct qxl_device *qdev, struct qxl_bo *bo)
> >>>
> >>> int qxl_surf_evict(struct qxl_device *qdev)
> >>> {
> >>> - return ttm_bo_evict_mm(&qdev->mman.bdev, TTM_PL_PRIV);
> >>> + return ttm_device_evict_mm(&qdev->mman.bdev, TTM_PL_PRIV);
> >>> }
> >>>
> >>> int qxl_vram_evict(struct qxl_device *qdev)
> >>> {
> >>> - return ttm_bo_evict_mm(&qdev->mman.bdev, TTM_PL_VRAM);
> >>> + return ttm_device_evict_mm(&qdev->mman.bdev, TTM_PL_VRAM);
> >>> }
> >>> diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
> >>> index a0b2c4541a2a..9577ec4a8a56 100644
> >>> --- a/drivers/gpu/drm/radeon/radeon_object.c
> >>> +++ b/drivers/gpu/drm/radeon/radeon_object.c
> >>> @@ -382,7 +382,7 @@ int radeon_bo_evict_vram(struct radeon_device *rdev)
> >>> return 0;
> >>> }
> >>> #endif
> >>> - return ttm_bo_evict_mm(&rdev->mman.bdev, TTM_PL_VRAM);
> >>> + return ttm_device_evict_mm(&rdev->mman.bdev, TTM_PL_VRAM);
> >>> }
> >>>
> >>> void radeon_bo_force_delete(struct radeon_device *rdev)
> >>> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> >>> index 40d1ac91c9bf..61311f69f9f8 100644
> >>> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> >>> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> >>> @@ -1173,24 +1173,6 @@ size_t ttm_bo_dma_acc_size(struct ttm_device *bdev,
> >>> }
> >>> EXPORT_SYMBOL(ttm_bo_dma_acc_size);
> >>>
> >>> -int ttm_bo_evict_mm(struct ttm_device *bdev, unsigned mem_type)
> >>> -{
> >>> - struct ttm_resource_manager *man = ttm_manager_type(bdev, mem_type);
> >>> -
> >>> - if (mem_type == 0 || mem_type >= TTM_NUM_MEM_TYPES) {
> >>> - pr_err("Illegal memory manager memory type %u\n", mem_type);
> >>> - return -EINVAL;
> >>> - }
> >>> -
> >>> - if (!man) {
> >>> - pr_err("Memory type %u has not been initialized\n", mem_type);
> >>> - return 0;
> >>> - }
> >>> -
> >>> - return ttm_resource_manager_force_list_clean(bdev, man);
> >>> -}
> >>> -EXPORT_SYMBOL(ttm_bo_evict_mm);
> >>> -
> >>> /*
> >>> * buffer object vm functions.
> >>> */
> >>> diff --git a/drivers/gpu/drm/ttm/ttm_device.c b/drivers/gpu/drm/ttm/ttm_device.c
> >>> index 65aa6e4f5c6c..fe2e4052afcb 100644
> >>> --- a/drivers/gpu/drm/ttm/ttm_device.c
> >>> +++ b/drivers/gpu/drm/ttm/ttm_device.c
> >>> @@ -220,3 +220,21 @@ int ttm_device_release(struct ttm_device *bdev)
> >>> return ret;
> >>> }
> >>> EXPORT_SYMBOL(ttm_device_release);
> >>> +
> >>> +int ttm_device_evict_mm(struct ttm_device *bdev, unsigned mem_type)
> >>> +{
> >>> + struct ttm_resource_manager *man = ttm_manager_type(bdev, mem_type);
> >>> +
> >>> + if (mem_type == 0 || mem_type >= TTM_NUM_MEM_TYPES) {
> >>> + pr_err("Illegal memory manager memory type %u\n", mem_type);
> >>> + return -EINVAL;
> >>> + }
> >>> +
> >>> + if (!man) {
> >>> + pr_err("Memory type %u has not been initialized\n", mem_type);
> >>> + return 0;
> >>> + }
> >>> +
> >>> + return ttm_resource_manager_force_list_clean(bdev, man);
> >>> +}
> >>> +EXPORT_SYMBOL(ttm_device_evict_mm);
> >>> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> >>> index fcda3b76e0df..2f99079e122b 100644
> >>> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> >>> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> >>> @@ -468,7 +468,7 @@ static int vmw_request_device(struct vmw_private *dev_priv)
> >>> if (dev_priv->cman)
> >>> vmw_cmdbuf_remove_pool(dev_priv->cman);
> >>> if (dev_priv->has_mob) {
> >>> - (void) ttm_bo_evict_mm(&dev_priv->bdev, VMW_PL_MOB);
> >>> + (void) ttm_device_evict_mm(&dev_priv->bdev, VMW_PL_MOB);
> >>> vmw_otables_takedown(dev_priv);
> >>> }
> >>> if (dev_priv->cman)
> >>> @@ -501,7 +501,7 @@ static void vmw_release_device_early(struct vmw_private *dev_priv)
> >>> vmw_cmdbuf_remove_pool(dev_priv->cman);
> >>>
> >>> if (dev_priv->has_mob) {
> >>> - ttm_bo_evict_mm(&dev_priv->bdev, VMW_PL_MOB);
> >>> + ttm_device_evict_mm(&dev_priv->bdev, VMW_PL_MOB);
> >>> vmw_otables_takedown(dev_priv);
> >>> }
> >>> }
> >>> @@ -1257,7 +1257,7 @@ void vmw_svga_disable(struct vmw_private *dev_priv)
> >>> if (ttm_resource_manager_used(man)) {
> >>> ttm_resource_manager_set_used(man, false);
> >>> spin_unlock(&dev_priv->svga_lock);
> >>> - if (ttm_bo_evict_mm(&dev_priv->bdev, TTM_PL_VRAM))
> >>> + if (ttm_device_evict_mm(&dev_priv->bdev, TTM_PL_VRAM))
> >>> DRM_ERROR("Failed evicting VRAM buffers.\n");
> >>> vmw_write(dev_priv, SVGA_REG_ENABLE,
> >>> SVGA_REG_ENABLE_HIDE |
> >>> diff --git a/include/drm/ttm/ttm_bo_api.h b/include/drm/ttm/ttm_bo_api.h
> >>> index a02935e0d121..f4014f45bd8b 100644
> >>> --- a/include/drm/ttm/ttm_bo_api.h
> >>> +++ b/include/drm/ttm/ttm_bo_api.h
> >>> @@ -448,26 +448,6 @@ int ttm_bo_init(struct ttm_device *bdev, struct ttm_buffer_object *bo,
> >>> struct sg_table *sg, struct dma_resv *resv,
> >>> void (*destroy) (struct ttm_buffer_object *));
> >>>
> >>> -/**
> >>> - * ttm_bo_evict_mm
> >>> - *
> >>> - * @bdev: Pointer to a ttm_device struct.
> >>> - * @mem_type: The memory type.
> >>> - *
> >>> - * Evicts all buffers on the lru list of the memory type.
> >>> - * This is normally part of a VT switch or an
> >>> - * out-of-memory-space-due-to-fragmentation handler.
> >>> - * The caller must make sure that there are no other processes
> >>> - * currently validating buffers, and can do that by taking the
> >>> - * struct ttm_device::ttm_lock in write mode.
> >>> - *
> >>> - * Returns:
> >>> - * -EINVAL: Invalid or uninitialized memory type.
> >>> - * -ERESTARTSYS: The call was interrupted by a signal while waiting to
> >>> - * evict a buffer.
> >>> - */
> >>> -int ttm_bo_evict_mm(struct ttm_device *bdev, unsigned mem_type);
> >>> -
> >>> /**
> >>> * ttm_kmap_obj_virtual
> >>> *
> >>> diff --git a/include/drm/ttm/ttm_device.h b/include/drm/ttm/ttm_device.h
> >>> index 75fedd378e8f..13840697d3ba 100644
> >>> --- a/include/drm/ttm/ttm_device.h
> >>> +++ b/include/drm/ttm/ttm_device.h
> >>> @@ -363,4 +363,24 @@ int ttm_device_init(struct ttm_device *bdev, struct ttm_device_funcs *funcs,
> >>> bool need_dma32);
> >>> int ttm_device_release(struct ttm_device *bdev);
> >>>
> >>> +/**
> >>> + * ttm_device_evict_mm
> >>> + *
> >>> + * @bdev: Pointer to a ttm_device struct.
> >>> + * @mem_type: The memory type.
> >>> + *
> >>> + * Evicts all buffers on the lru list of the memory type.
> >>> + * This is normally part of a VT switch or an
> >>> + * out-of-memory-space-due-to-fragmentation handler.
> >>> + * The caller must make sure that there are no other processes
> >>> + * currently validating buffers, and can do that by taking the
> >>> + * struct ttm_device::ttm_lock in write mode.
> >>> + *
> >>> + * Returns:
> >>> + * -EINVAL: Invalid or uninitialized memory type.
> >>> + * -ERESTARTSYS: The call was interrupted by a signal while waiting to
> >>> + * evict a buffer.
> >>> + */
> >>> +int ttm_device_evict_mm(struct ttm_device *bdev, unsigned mem_type);
> >>> +
> >>> #endif
> >>> --
> >>> 2.17.1
> >>>
> >>
> >> --
> >> Daniel Vetter
> >> Software Engineer, Intel Corporation
> >> http://blog.ffwll.ch
> >
> >
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
