[PATCH 4/5] drm/ttm: initialize globals during device init
Zhang, Jerry(Junwei)
Jerry.Zhang at amd.com
Mon Oct 22 06:51:11 UTC 2018
A question for ttm_bo.c
On 10/20/2018 12:41 AM, Christian König wrote:
> Make sure that the global BO state is always correctly initialized.
>
> This allows removing all the device code to initialize it.
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c | 59 +------------------------
> drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.h | 1 -
> drivers/gpu/drm/ast/ast_drv.h | 1 -
> drivers/gpu/drm/ast/ast_ttm.c | 36 ---------------
> drivers/gpu/drm/bochs/bochs.h | 1 -
> drivers/gpu/drm/bochs/bochs_mm.c | 35 ---------------
> drivers/gpu/drm/cirrus/cirrus_drv.h | 1 -
> drivers/gpu/drm/cirrus/cirrus_ttm.c | 36 ---------------
> drivers/gpu/drm/hisilicon/hibmc/hibmc_drm_drv.h | 1 -
> drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 34 --------------
> drivers/gpu/drm/mgag200/mgag200_drv.h | 1 -
> drivers/gpu/drm/mgag200/mgag200_ttm.c | 36 ---------------
> drivers/gpu/drm/nouveau/nouveau_drv.h | 1 -
> drivers/gpu/drm/nouveau/nouveau_ttm.c | 39 ----------------
> drivers/gpu/drm/qxl/qxl_drv.h | 2 -
> drivers/gpu/drm/qxl/qxl_ttm.c | 33 --------------
> drivers/gpu/drm/radeon/radeon.h | 2 -
> drivers/gpu/drm/radeon/radeon_ttm.c | 39 ----------------
> drivers/gpu/drm/ttm/ttm_bo.c | 19 +++++---
> drivers/gpu/drm/virtio/virtgpu_drv.h | 2 -
> drivers/gpu/drm/virtio/virtgpu_ttm.c | 35 ---------------
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 11 +----
> drivers/gpu/drm/vmwgfx/vmwgfx_drv.h | 3 --
> drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c | 27 -----------
> drivers/staging/vboxvideo/vbox_ttm.c | 36 ---------------
> include/drm/ttm/ttm_bo_driver.h | 41 +----------------
> 26 files changed, 16 insertions(+), 516 deletions(-)
>
> [... skip above ...]
> diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
> index d89183f95570..df028805b7e2 100644
> --- a/drivers/gpu/drm/ttm/ttm_bo.c
> +++ b/drivers/gpu/drm/ttm/ttm_bo.c
> @@ -1530,7 +1530,7 @@ static void ttm_bo_global_kobj_release(struct kobject *kobj)
> kfree(glob);
> }
>
> -void ttm_bo_global_release(void)
> +static void ttm_bo_global_release(void)
> {
> struct ttm_bo_global *glob = &ttm_bo_glob;
>
> @@ -1544,9 +1544,8 @@ void ttm_bo_global_release(void)
> out:
> mutex_unlock(&ttm_global_mutex);
> }
> -EXPORT_SYMBOL(ttm_bo_global_release);
>
> -int ttm_bo_global_init(void)
> +static int ttm_bo_global_init(void)
> {
> struct ttm_bo_global *glob = &ttm_bo_glob;
> int ret = 0;
> @@ -1583,8 +1582,6 @@ int ttm_bo_global_init(void)
> mutex_unlock(&ttm_global_mutex);
> return ret;
> }
> -EXPORT_SYMBOL(ttm_bo_global_init);
> -
>
> int ttm_bo_device_release(struct ttm_bo_device *bdev)
> {
> @@ -1623,18 +1620,25 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev)
>
> drm_vma_offset_manager_destroy(&bdev->vma_manager);
>
> + if (!ret)
> + ttm_bo_global_release();
> +
If ttm_bo_clean_mm() fails, it will skip ttm_bo_global_release()
When it will be called?
Shall we add it to a delayed work or we may call it directly here.
Regards
Jerry
> return ret;
> }
> EXPORT_SYMBOL(ttm_bo_device_release);
>
> int ttm_bo_device_init(struct ttm_bo_device *bdev,
> - struct ttm_bo_global *glob,
> struct ttm_bo_driver *driver,
> struct address_space *mapping,
> uint64_t file_page_offset,
> bool need_dma32)
> {
> - int ret = -EINVAL;
> + struct ttm_bo_global *glob = &ttm_bo_glob;
> + int ret;
> +
> + ret = ttm_bo_global_init();
> + if (ret)
> + return ret;
>
> bdev->driver = driver;
>
> @@ -1661,6 +1665,7 @@ int ttm_bo_device_init(struct ttm_bo_device *bdev,
>
> return 0;
> out_no_sys:
> + ttm_bo_global_release();
> return ret;
> }
> EXPORT_SYMBOL(ttm_bo_device_init);
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
> index dec42d421e00..30caa20d9fcf 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.h
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
> @@ -132,8 +132,6 @@ struct virtio_gpu_framebuffer {
> container_of(x, struct virtio_gpu_framebuffer, base)
>
> struct virtio_gpu_mman {
> - struct ttm_bo_global_ref bo_global_ref;
> - bool mem_global_referenced;
> struct ttm_bo_device bdev;
> };
>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_ttm.c b/drivers/gpu/drm/virtio/virtgpu_ttm.c
> index b99ecc6d97d3..c1a56d640121 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_ttm.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_ttm.c
> @@ -50,35 +50,6 @@ virtio_gpu_device *virtio_gpu_get_vgdev(struct ttm_bo_device *bdev)
> return vgdev;
> }
>
> -static int virtio_gpu_ttm_global_init(struct virtio_gpu_device *vgdev)
> -{
> - struct drm_global_reference *global_ref;
> - int r;
> -
> - vgdev->mman.mem_global_referenced = false;
> - global_ref = &vgdev->mman.bo_global_ref.ref;
> - global_ref->global_type = DRM_GLOBAL_TTM_BO;
> - global_ref->size = sizeof(struct ttm_bo_global);
> - global_ref->init = &ttm_bo_global_ref_init;
> - global_ref->release = &ttm_bo_global_ref_release;
> - r = drm_global_item_ref(global_ref);
> - if (r != 0) {
> - DRM_ERROR("Failed setting up TTM BO subsystem.\n");
> - return r;
> - }
> -
> - vgdev->mman.mem_global_referenced = true;
> - return 0;
> -}
> -
> -static void virtio_gpu_ttm_global_fini(struct virtio_gpu_device *vgdev)
> -{
> - if (vgdev->mman.mem_global_referenced) {
> - drm_global_item_unref(&vgdev->mman.bo_global_ref.ref);
> - vgdev->mman.mem_global_referenced = false;
> - }
> -}
> -
> #if 0
> /*
> * Hmm, seems to not do anything useful. Leftover debug hack?
> @@ -391,12 +362,8 @@ int virtio_gpu_ttm_init(struct virtio_gpu_device *vgdev)
> {
> int r;
>
> - r = virtio_gpu_ttm_global_init(vgdev);
> - if (r)
> - return r;
> /* No others user of address space so set it to 0 */
> r = ttm_bo_device_init(&vgdev->mman.bdev,
> - vgdev->mman.bo_global_ref.ref.object,
> &virtio_gpu_bo_driver,
> vgdev->ddev->anon_inode->i_mapping,
> DRM_FILE_PAGE_OFFSET, 0);
> @@ -415,13 +382,11 @@ int virtio_gpu_ttm_init(struct virtio_gpu_device *vgdev)
> err_mm_init:
> ttm_bo_device_release(&vgdev->mman.bdev);
> err_dev_init:
> - virtio_gpu_ttm_global_fini(vgdev);
> return r;
> }
>
> void virtio_gpu_ttm_fini(struct virtio_gpu_device *vgdev)
> {
> ttm_bo_device_release(&vgdev->mman.bdev);
> - virtio_gpu_ttm_global_fini(vgdev);
> DRM_INFO("virtio_gpu: ttm finalized\n");
> }
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 57df776d987c..29da63a0a0f2 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -801,11 +801,6 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
> DRM_INFO("MMIO at 0x%08x size is %u kiB\n",
> dev_priv->mmio_start, dev_priv->mmio_size / 1024);
>
> - ret = vmw_ttm_global_init(dev_priv);
> - if (unlikely(ret != 0))
> - goto out_err0;
> -
> -
> vmw_master_init(&dev_priv->fbdev_master);
> ttm_lock_set_kill(&dev_priv->fbdev_master.lock, false, SIGTERM);
> dev_priv->active_master = &dev_priv->fbdev_master;
> @@ -816,7 +811,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
> if (unlikely(dev_priv->mmio_virt == NULL)) {
> ret = -ENOMEM;
> DRM_ERROR("Failed mapping MMIO.\n");
> - goto out_err3;
> + goto out_err0;
> }
>
> /* Need mmio memory to check for fifo pitchlock cap. */
> @@ -870,7 +865,6 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
> }
>
> ret = ttm_bo_device_init(&dev_priv->bdev,
> - dev_priv->bo_global_ref.ref.object,
> &vmw_bo_driver,
> dev->anon_inode->i_mapping,
> VMWGFX_FILE_PAGE_OFFSET,
> @@ -992,8 +986,6 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
> ttm_object_device_release(&dev_priv->tdev);
> out_err4:
> memunmap(dev_priv->mmio_virt);
> -out_err3:
> - vmw_ttm_global_release(dev_priv);
> out_err0:
> for (i = vmw_res_context; i < vmw_res_max; ++i)
> idr_destroy(&dev_priv->res_idr[i]);
> @@ -1045,7 +1037,6 @@ static void vmw_driver_unload(struct drm_device *dev)
> memunmap(dev_priv->mmio_virt);
> if (dev_priv->ctx.staged_bindings)
> vmw_binding_state_free(dev_priv->ctx.staged_bindings);
> - vmw_ttm_global_release(dev_priv);
>
> for (i = vmw_res_context; i < vmw_res_max; ++i)
> idr_destroy(&dev_priv->res_idr[i]);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index df15a745efc3..a23f1c1fd4c9 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -365,7 +365,6 @@ enum {
>
> struct vmw_private {
> struct ttm_bo_device bdev;
> - struct ttm_bo_global_ref bo_global_ref;
>
> struct vmw_fifo_state fifo;
>
> @@ -762,8 +761,6 @@ extern int vmw_fifo_flush(struct vmw_private *dev_priv,
> * TTM glue - vmwgfx_ttm_glue.c
> */
>
> -extern int vmw_ttm_global_init(struct vmw_private *dev_priv);
> -extern void vmw_ttm_global_release(struct vmw_private *dev_priv);
> extern int vmw_mmap(struct file *filp, struct vm_area_struct *vma);
>
> /**
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> index 0ac473cd5136..154eb09aa91e 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
> @@ -42,30 +42,3 @@ int vmw_mmap(struct file *filp, struct vm_area_struct *vma)
> dev_priv = vmw_priv(file_priv->minor->dev);
> return ttm_bo_mmap(filp, vma, &dev_priv->bdev);
> }
> -
> -int vmw_ttm_global_init(struct vmw_private *dev_priv)
> -{
> - struct drm_global_reference *global_ref;
> - int ret;
> -
> - global_ref = &dev_priv->bo_global_ref.ref;
> - global_ref->global_type = DRM_GLOBAL_TTM_BO;
> - global_ref->size = sizeof(struct ttm_bo_global);
> - global_ref->init = &ttm_bo_global_ref_init;
> - global_ref->release = &ttm_bo_global_ref_release;
> - ret = drm_global_item_ref(global_ref);
> -
> - if (unlikely(ret != 0)) {
> - DRM_ERROR("Failed setting up TTM buffer objects.\n");
> - goto out_no_bo;
> - }
> -
> - return 0;
> -out_no_bo:
> - return ret;
> -}
> -
> -void vmw_ttm_global_release(struct vmw_private *dev_priv)
> -{
> - drm_global_item_unref(&dev_priv->bo_global_ref.ref);
> -}
> diff --git a/drivers/staging/vboxvideo/vbox_ttm.c b/drivers/staging/vboxvideo/vbox_ttm.c
> index 88cdacf2b0f0..6c8ba8625ad9 100644
> --- a/drivers/staging/vboxvideo/vbox_ttm.c
> +++ b/drivers/staging/vboxvideo/vbox_ttm.c
> @@ -35,37 +35,6 @@ static inline struct vbox_private *vbox_bdev(struct ttm_bo_device *bd)
> return container_of(bd, struct vbox_private, ttm.bdev);
> }
>
> -/**
> - * Adds the vbox memory manager object/structures to the global memory manager.
> - */
> -static int vbox_ttm_global_init(struct vbox_private *vbox)
> -{
> - struct drm_global_reference *global_ref;
> - int ret;
> -
> - global_ref = &vbox->ttm.bo_global_ref.ref;
> - global_ref->global_type = DRM_GLOBAL_TTM_BO;
> - global_ref->size = sizeof(struct ttm_bo_global);
> - global_ref->init = &ttm_bo_global_ref_init;
> - global_ref->release = &ttm_bo_global_ref_release;
> -
> - ret = drm_global_item_ref(global_ref);
> - if (ret) {
> - DRM_ERROR("Failed setting up TTM BO subsystem.\n");
> - return ret;
> - }
> -
> - return 0;
> -}
> -
> -/**
> - * Removes the vbox memory manager object from the global memory manager.
> - */
> -static void vbox_ttm_global_release(struct vbox_private *vbox)
> -{
> - drm_global_item_unref(&vbox->ttm.bo_global_ref.ref);
> -}
> -
> static void vbox_bo_ttm_destroy(struct ttm_buffer_object *tbo)
> {
> struct vbox_bo *bo;
> @@ -203,12 +172,7 @@ int vbox_mm_init(struct vbox_private *vbox)
> struct drm_device *dev = vbox->dev;
> struct ttm_bo_device *bdev = &vbox->ttm.bdev;
>
> - ret = vbox_ttm_global_init(vbox);
> - if (ret)
> - return ret;
> -
> ret = ttm_bo_device_init(&vbox->ttm.bdev,
> - vbox->ttm.bo_global_ref.ref.object,
> &vbox_bo_driver,
> dev->anon_inode->i_mapping,
> DRM_FILE_PAGE_OFFSET, true);
> diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
> index 26be74939f10..6fb589f64633 100644
> --- a/include/drm/ttm/ttm_bo_driver.h
> +++ b/include/drm/ttm/ttm_bo_driver.h
> @@ -569,9 +569,6 @@ void ttm_bo_mem_put(struct ttm_buffer_object *bo, struct ttm_mem_reg *mem);
> void ttm_bo_mem_put_locked(struct ttm_buffer_object *bo,
> struct ttm_mem_reg *mem);
>
> -void ttm_bo_global_release(void);
> -int ttm_bo_global_init(void);
> -
> int ttm_bo_device_release(struct ttm_bo_device *bdev);
>
> /**
> @@ -589,7 +586,7 @@ int ttm_bo_device_release(struct ttm_bo_device *bdev);
> * Returns:
> * !0: Failure.
> */
> -int ttm_bo_device_init(struct ttm_bo_device *bdev, struct ttm_bo_global *glob,
> +int ttm_bo_device_init(struct ttm_bo_device *bdev,
> struct ttm_bo_driver *driver,
> struct address_space *mapping,
> uint64_t file_page_offset, bool need_dma32);
> @@ -888,40 +885,4 @@ pgprot_t ttm_io_prot(uint32_t caching_flags, pgprot_t tmp);
>
> extern const struct ttm_mem_type_manager_func ttm_bo_manager_func;
>
> -/**
> - * struct ttm_bo_global_ref - Argument to initialize a struct ttm_bo_global.
> - */
> -
> -struct ttm_bo_global_ref {
> - struct drm_global_reference ref;
> -};
> -
> -/**
> - * ttm_bo_global_ref_init
> - *
> - * @ref: DRM global reference
> - *
> - * Helper function that initializes a struct ttm_bo_global. This function
> - * is used as init call-back function for DRM global references of type
> - * DRM_GLOBAL_TTM_BO_REF.
> - */
> -static inline int ttm_bo_global_ref_init(struct drm_global_reference *ref)
> -{
> - return ttm_bo_global_init();
> -}
> -
> -/**
> - * ttm_bo_global_ref_release
> - *
> - * @ref: DRM global reference
> - *
> - * Helper function that releases a struct ttm_bo_global. This function
> - * is used as release call-back function for DRM global references of type
> - * DRM_GLOBAL_TTM_BO_REF.
> - */
> -static inline void ttm_bo_global_ref_release(struct drm_global_reference *ref)
> -{
> - ttm_bo_global_release();
> -}
> -
> #endif
More information about the amd-gfx
mailing list