[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