thr3ads.net - Nouveau - [Nouveau] [PATCH 0/2] Provide init/release functions for struct ttm_bo

If this information is useful, please help other people find it:
Share via:

Thomas Zimmermann

2018-Aug-13 10:24 UTC

[Nouveau] [PATCH 0/2] Provide init/release functions for struct ttm_bo_global

TTM uses global memory and BO for backing graphics buffers. These are
represented by struct ttm_mem_global and struct ttm_bo_global.

Currently, struct ttm_bo_global can only be initialized and released through
struct ttm_bo_global_ref. This is a workaround for passing an instance of
ttm_mem_global to the BO global initialization code.

The use of struct ttm_bo_global_ref makes driver code unnecessary hard to
understand. At the same time drivers can use any combination of memory and
BO for initializing the global instances. This can result in subtle bugs
when the order of initializing and releasing drivers changes.

As a first step for resolving these problems, the provided patch set
separates initialization and release of struct ttm_bo_global from
struct ttm_bo_global_ref.

The first patch only renames ttm_bo_global_{init/release}. Hopefully this
change can be applied at once for all drivers.

Future directions: All TTM-based drivers follow the same pattern for setting
up the TTM. In a follow-up patch, this code can be moved into a single place
and shared among drivers.

Thomas Zimmermann (2):
  drm/ttm: Rename ttm_bo_global_{init,release}() to
    ttm_bo_global_ref_*()
  drm/ttm: Provide ttm_bo_global_{init/release}() for struct
    ttm_bo_global

 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  4 +-
 drivers/gpu/drm/ast/ast_ttm.c               |  4 +-
 drivers/gpu/drm/bochs/bochs_mm.c            |  4 +-
 drivers/gpu/drm/cirrus/cirrus_ttm.c         |  4 +-
 drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c |  4 +-
 drivers/gpu/drm/mgag200/mgag200_ttm.c       |  4 +-
 drivers/gpu/drm/nouveau/nouveau_ttm.c       |  4 +-
 drivers/gpu/drm/qxl/qxl_ttm.c               |  4 +-
 drivers/gpu/drm/radeon/radeon_ttm.c         |  4 +-
 drivers/gpu/drm/ttm/ttm_bo.c                | 12 ++---
 drivers/gpu/drm/virtio/virtgpu_ttm.c        |  4 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c    |  4 +-
 drivers/staging/vboxvideo/vbox_ttm.c        |  4 +-
 include/drm/ttm/ttm_bo_driver.h             | 53 ++++++++++++++++-----
 14 files changed, 70 insertions(+), 43 deletions(-)

--
2.18.0

Thomas Zimmermann

2018-Aug-13 10:24 UTC

head link

[Nouveau] [PATCH 1/2] drm/ttm: Rename ttm_bo_global_{init, release}() to ttm_bo_global_ref_*()

The functions ttm_bo_global_init() and ttm_bo_global_release() do not
receive an argument of type struct ttm_bo_global. Both take a struct
drm_global_reference that contains points to a struct ttm_bo_global_ref.
Renaming them reflects this.

Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     | 4 ++--
 drivers/gpu/drm/ast/ast_ttm.c               | 4 ++--
 drivers/gpu/drm/bochs/bochs_mm.c            | 4 ++--
 drivers/gpu/drm/cirrus/cirrus_ttm.c         | 4 ++--
 drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c | 4 ++--
 drivers/gpu/drm/mgag200/mgag200_ttm.c       | 4 ++--
 drivers/gpu/drm/nouveau/nouveau_ttm.c       | 4 ++--
 drivers/gpu/drm/qxl/qxl_ttm.c               | 4 ++--
 drivers/gpu/drm/radeon/radeon_ttm.c         | 4 ++--
 drivers/gpu/drm/ttm/ttm_bo.c                | 8 ++++----
 drivers/gpu/drm/virtio/virtgpu_ttm.c        | 4 ++--
 drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c    | 4 ++--
 drivers/staging/vboxvideo/vbox_ttm.c        | 4 ++--
 include/drm/ttm/ttm_bo_driver.h             | 4 ++--
 14 files changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
index fcf421263fd9..7756973d084a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c
@@ -124,8 +124,8 @@ static int amdgpu_ttm_global_init(struct amdgpu_device
*adev)
 	global_ref = &adev->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_init;
-	global_ref->release = &ttm_bo_global_release;
+	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) {
 		DRM_ERROR("Failed setting up TTM BO subsystem.\n");
diff --git a/drivers/gpu/drm/ast/ast_ttm.c b/drivers/gpu/drm/ast/ast_ttm.c
index fe354ebf374d..d21fbd26785a 100644
--- a/drivers/gpu/drm/ast/ast_ttm.c
+++ b/drivers/gpu/drm/ast/ast_ttm.c
@@ -70,8 +70,8 @@ static int ast_ttm_global_init(struct ast_private *ast)
 	global_ref = &ast->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_init;
-	global_ref->release = &ttm_bo_global_release;
+	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");
diff --git a/drivers/gpu/drm/bochs/bochs_mm.c b/drivers/gpu/drm/bochs/bochs_mm.c
index 39cd08416773..e794986e6b7c 100644
--- a/drivers/gpu/drm/bochs/bochs_mm.c
+++ b/drivers/gpu/drm/bochs/bochs_mm.c
@@ -48,8 +48,8 @@ static int bochs_ttm_global_init(struct bochs_device *bochs)
 	global_ref = &bochs->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_init;
-	global_ref->release = &ttm_bo_global_release;
+	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");
diff --git a/drivers/gpu/drm/cirrus/cirrus_ttm.c
b/drivers/gpu/drm/cirrus/cirrus_ttm.c
index f21953243790..2e2141f26c5b 100644
--- a/drivers/gpu/drm/cirrus/cirrus_ttm.c
+++ b/drivers/gpu/drm/cirrus/cirrus_ttm.c
@@ -70,8 +70,8 @@ static int cirrus_ttm_global_init(struct cirrus_device
*cirrus)
 	global_ref = &cirrus->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_init;
-	global_ref->release = &ttm_bo_global_release;
+	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");
diff --git a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
index 4871025f7573..786269f0c0f1 100644
--- a/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
+++ b/drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c
@@ -59,8 +59,8 @@ static int hibmc_ttm_global_init(struct hibmc_drm_private
*hibmc)
 		hibmc->mem_global_ref.object;
 	hibmc->bo_global_ref.ref.global_type = DRM_GLOBAL_TTM_BO;
 	hibmc->bo_global_ref.ref.size = sizeof(struct ttm_bo_global);
-	hibmc->bo_global_ref.ref.init = &ttm_bo_global_init;
-	hibmc->bo_global_ref.ref.release = &ttm_bo_global_release;
+	hibmc->bo_global_ref.ref.init = &ttm_bo_global_ref_init;
+	hibmc->bo_global_ref.ref.release = &ttm_bo_global_ref_release;
 	ret = drm_global_item_ref(&hibmc->bo_global_ref.ref);
 	if (ret) {
 		DRM_ERROR("failed setting up TTM BO subsystem: %d\n", ret);
diff --git a/drivers/gpu/drm/mgag200/mgag200_ttm.c
b/drivers/gpu/drm/mgag200/mgag200_ttm.c
index 05570f0de4d7..3444b539e7f4 100644
--- a/drivers/gpu/drm/mgag200/mgag200_ttm.c
+++ b/drivers/gpu/drm/mgag200/mgag200_ttm.c
@@ -70,8 +70,8 @@ static int mgag200_ttm_global_init(struct mga_device *ast)
 	global_ref = &ast->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_init;
-	global_ref->release = &ttm_bo_global_release;
+	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");
diff --git a/drivers/gpu/drm/nouveau/nouveau_ttm.c
b/drivers/gpu/drm/nouveau/nouveau_ttm.c
index 8edb9f2a4269..a293383c8654 100644
--- a/drivers/gpu/drm/nouveau/nouveau_ttm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_ttm.c
@@ -209,8 +209,8 @@ nouveau_ttm_global_init(struct nouveau_drm *drm)
 	global_ref = &drm->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_init;
-	global_ref->release = &ttm_bo_global_release;
+	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)) {
diff --git a/drivers/gpu/drm/qxl/qxl_ttm.c b/drivers/gpu/drm/qxl/qxl_ttm.c
index 86a1fb32f6db..db2a0036e9c4 100644
--- a/drivers/gpu/drm/qxl/qxl_ttm.c
+++ b/drivers/gpu/drm/qxl/qxl_ttm.c
@@ -80,8 +80,8 @@ static int qxl_ttm_global_init(struct qxl_device *qdev)
 	global_ref = &qdev->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_init;
-	global_ref->release = &ttm_bo_global_release;
+	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");
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c
b/drivers/gpu/drm/radeon/radeon_ttm.c
index cbb67e9ffb3a..dac4ec5a120b 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -97,8 +97,8 @@ static int radeon_ttm_global_init(struct radeon_device *rdev)
 	global_ref = &rdev->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_init;
-	global_ref->release = &ttm_bo_global_release;
+	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");
diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 7c484729f9b2..93bf995a0370 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1448,16 +1448,16 @@ static void ttm_bo_global_kobj_release(struct kobject
*kobj)
 	kfree(glob);
 }
 
-void ttm_bo_global_release(struct drm_global_reference *ref)
+void ttm_bo_global_ref_release(struct drm_global_reference *ref)
 {
 	struct ttm_bo_global *glob = ref->object;
 
 	kobject_del(&glob->kobj);
 	kobject_put(&glob->kobj);
 }
-EXPORT_SYMBOL(ttm_bo_global_release);
+EXPORT_SYMBOL(ttm_bo_global_ref_release);
 
-int ttm_bo_global_init(struct drm_global_reference *ref)
+int ttm_bo_global_ref_init(struct drm_global_reference *ref)
 {
 	struct ttm_bo_global_ref *bo_ref  		container_of(ref, struct
ttm_bo_global_ref, ref);
@@ -1490,7 +1490,7 @@ int ttm_bo_global_init(struct drm_global_reference *ref)
 	kfree(glob);
 	return ret;
 }
-EXPORT_SYMBOL(ttm_bo_global_init);
+EXPORT_SYMBOL(ttm_bo_global_ref_init);
 
 
 int ttm_bo_device_release(struct ttm_bo_device *bdev)
diff --git a/drivers/gpu/drm/virtio/virtgpu_ttm.c
b/drivers/gpu/drm/virtio/virtgpu_ttm.c
index b6f021c0da5c..9f3e71f85a92 100644
--- a/drivers/gpu/drm/virtio/virtgpu_ttm.c
+++ b/drivers/gpu/drm/virtio/virtgpu_ttm.c
@@ -84,8 +84,8 @@ static int virtio_gpu_ttm_global_init(struct virtio_gpu_device
*vgdev)
 	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_init;
-	global_ref->release = &ttm_bo_global_release;
+	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");
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
index 7b1e5a5cbd2c..f3ce43c41978 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c
@@ -76,8 +76,8 @@ int vmw_ttm_global_init(struct vmw_private *dev_priv)
 	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_init;
-	global_ref->release = &ttm_bo_global_release;
+	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)) {
diff --git a/drivers/staging/vboxvideo/vbox_ttm.c
b/drivers/staging/vboxvideo/vbox_ttm.c
index 548edb7c494b..2329a55d4636 100644
--- a/drivers/staging/vboxvideo/vbox_ttm.c
+++ b/drivers/staging/vboxvideo/vbox_ttm.c
@@ -68,8 +68,8 @@ static int vbox_ttm_global_init(struct vbox_private *vbox)
 	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_init;
-	global_ref->release = &ttm_bo_global_release;
+	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) {
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 3234cc322e70..2d4021fb5382 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -550,8 +550,8 @@ 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(struct drm_global_reference *ref);
-int ttm_bo_global_init(struct drm_global_reference *ref);
+void ttm_bo_global_ref_release(struct drm_global_reference *ref);
+int ttm_bo_global_ref_init(struct drm_global_reference *ref);
 
 int ttm_bo_device_release(struct ttm_bo_device *bdev);
 
-- 
2.18.0

Thomas Zimmermann

2018-Aug-13 10:24 UTC

head link

[Nouveau] [PATCH 2/2] drm/ttm: Provide ttm_bo_global_{init/release}() for struct ttm_bo_global

So far, struct ttm_bo_global_ref was the only way of initializing a struct
ttm_bo_global. Providing separate initializer and release functions for
struct ttm_bo_global gives drivers the option of implementing their own
init and release callbacks for drm_global_references of type
DRM_GLOBAL_TTM_BO.

The original functions for initializing and releasing via struct
ttm_bo_global_ref are wrappers around the new interfaces.

Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
---
 drivers/gpu/drm/ttm/ttm_bo.c    | 16 ++++------
 include/drm/ttm/ttm_bo_driver.h | 53 ++++++++++++++++++++++++++-------
 2 files changed, 48 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/ttm/ttm_bo.c b/drivers/gpu/drm/ttm/ttm_bo.c
index 93bf995a0370..f9439698a3c4 100644
--- a/drivers/gpu/drm/ttm/ttm_bo.c
+++ b/drivers/gpu/drm/ttm/ttm_bo.c
@@ -1448,26 +1448,22 @@ static void ttm_bo_global_kobj_release(struct kobject
*kobj)
 	kfree(glob);
 }
 
-void ttm_bo_global_ref_release(struct drm_global_reference *ref)
+void ttm_bo_global_release(struct ttm_bo_global *glob)
 {
-	struct ttm_bo_global *glob = ref->object;
-
 	kobject_del(&glob->kobj);
 	kobject_put(&glob->kobj);
 }
-EXPORT_SYMBOL(ttm_bo_global_ref_release);
+EXPORT_SYMBOL(ttm_bo_global_release);
 
-int ttm_bo_global_ref_init(struct drm_global_reference *ref)
+int ttm_bo_global_init(struct ttm_bo_global *glob,
+		       struct ttm_mem_global *mem_glob)
 {
-	struct ttm_bo_global_ref *bo_ref -		container_of(ref, struct
ttm_bo_global_ref, ref);
-	struct ttm_bo_global *glob = ref->object;
 	int ret;
 	unsigned i;
 
 	mutex_init(&glob->device_list_mutex);
 	spin_lock_init(&glob->lru_lock);
-	glob->mem_glob = bo_ref->mem_glob;
+	glob->mem_glob = mem_glob;
 	glob->mem_glob->bo_glob = glob;
 	glob->dummy_read_page = alloc_page(__GFP_ZERO | GFP_DMA32);
 
@@ -1490,7 +1486,7 @@ int ttm_bo_global_ref_init(struct drm_global_reference
*ref)
 	kfree(glob);
 	return ret;
 }
-EXPORT_SYMBOL(ttm_bo_global_ref_init);
+EXPORT_SYMBOL(ttm_bo_global_init);
 
 
 int ttm_bo_device_release(struct ttm_bo_device *bdev)
diff --git a/include/drm/ttm/ttm_bo_driver.h b/include/drm/ttm/ttm_bo_driver.h
index 2d4021fb5382..daaafb96eb11 100644
--- a/include/drm/ttm/ttm_bo_driver.h
+++ b/include/drm/ttm/ttm_bo_driver.h
@@ -384,15 +384,6 @@ struct ttm_bo_driver {
 			     void *buf, int len, int write);
 };
 
-/**
- * struct ttm_bo_global_ref - Argument to initialize a struct ttm_bo_global.
- */
-
-struct ttm_bo_global_ref {
-	struct drm_global_reference ref;
-	struct ttm_mem_global *mem_glob;
-};
-
 /**
  * struct ttm_bo_global - Buffer object driver global data.
  *
@@ -550,8 +541,9 @@ 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_ref_release(struct drm_global_reference *ref);
-int ttm_bo_global_ref_init(struct drm_global_reference *ref);
+void ttm_bo_global_release(struct ttm_bo_global *glob);
+int ttm_bo_global_init(struct ttm_bo_global *glob,
+		       struct ttm_mem_global *mem_glob);
 
 int ttm_bo_device_release(struct ttm_bo_device *bdev);
 
@@ -869,4 +861,43 @@ 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;
+	struct ttm_mem_global *mem_glob;
+};
+
+/**
+ * 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)
+{
+	struct ttm_bo_global_ref *bo_ref +		container_of(ref, struct
ttm_bo_global_ref, ref);
+	return ttm_bo_global_init(ref->object, bo_ref->mem_glob);
+}
+
+/**
+ * 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(ref->object);
+}
+
 #endif
-- 
2.18.0

Christian König

2018-Aug-13 10:33 UTC

head link

[PATCH 0/2] Provide init/release functions for struct ttm_bo_global

Yes, please! I had it on my TODO list to clean that up for an eternity.

Actually I never understood why that should be driver work to setup TTM?

I mean can't we just have a module_init/module_exit for TTM?

Thanks,
Christian.

Am 13.08.2018 um 12:24 schrieb Thomas Zimmermann:> TTM uses global memory and BO for backing graphics buffers. These are
> represented by struct ttm_mem_global and struct ttm_bo_global.
>
> Currently, struct ttm_bo_global can only be initialized and released
through
> struct ttm_bo_global_ref. This is a workaround for passing an instance of
> ttm_mem_global to the BO global initialization code.
>
> The use of struct ttm_bo_global_ref makes driver code unnecessary hard to
> understand. At the same time drivers can use any combination of memory and
> BO for initializing the global instances. This can result in subtle bugs
> when the order of initializing and releasing drivers changes.
>
> As a first step for resolving these problems, the provided patch set
> separates initialization and release of struct ttm_bo_global from
> struct ttm_bo_global_ref.
>
> The first patch only renames ttm_bo_global_{init/release}. Hopefully this
> change can be applied at once for all drivers.
>
> Future directions: All TTM-based drivers follow the same pattern for
setting
> up the TTM. In a follow-up patch, this code can be moved into a single
place
> and shared among drivers.
>
> Thomas Zimmermann (2):
>    drm/ttm: Rename ttm_bo_global_{init,release}() to
>      ttm_bo_global_ref_*()
>    drm/ttm: Provide ttm_bo_global_{init/release}() for struct
>      ttm_bo_global
>
>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  4 +-
>   drivers/gpu/drm/ast/ast_ttm.c               |  4 +-
>   drivers/gpu/drm/bochs/bochs_mm.c            |  4 +-
>   drivers/gpu/drm/cirrus/cirrus_ttm.c         |  4 +-
>   drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c |  4 +-
>   drivers/gpu/drm/mgag200/mgag200_ttm.c       |  4 +-
>   drivers/gpu/drm/nouveau/nouveau_ttm.c       |  4 +-
>   drivers/gpu/drm/qxl/qxl_ttm.c               |  4 +-
>   drivers/gpu/drm/radeon/radeon_ttm.c         |  4 +-
>   drivers/gpu/drm/ttm/ttm_bo.c                | 12 ++---
>   drivers/gpu/drm/virtio/virtgpu_ttm.c        |  4 +-
>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c    |  4 +-
>   drivers/staging/vboxvideo/vbox_ttm.c        |  4 +-
>   include/drm/ttm/ttm_bo_driver.h             | 53 ++++++++++++++++-----
>   14 files changed, 70 insertions(+), 43 deletions(-)
>
> --
> 2.18.0
>

Thomas Zimmermann

2018-Aug-13 12:28 UTC

head link

[Nouveau] [PATCH 0/2] Provide init/release functions for struct ttm_bo_global

Hi

Am 13.08.2018 um 12:33 schrieb Christian König:> Yes, please! I had it on my TODO list to clean that up for an eternity.
On top of these patches, I have a patch set that provides a single
init/release interface for TTM global data. I'll post it when the
current patches got some feed back.

I'd really like to move the code from drm_global.c back into TTM. It's
TTM-specific and not useful elsewhere. However, the first commit message
from 2010 [1] says that some unnamed, external driver uses this code for
something. Do you know if this still applies?

Best regards
Thomas

[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=ba4420c224c2808f2661cf8428f43ceef7a73a4a
> 
> Actually I never understood why that should be driver work to setup TTM?
> 
> I mean can't we just have a module_init/module_exit for TTM?
> 
> Thanks,
> Christian.
> 
> Am 13.08.2018 um 12:24 schrieb Thomas Zimmermann:
>> TTM uses global memory and BO for backing graphics buffers. These are
>> represented by struct ttm_mem_global and struct ttm_bo_global.
>>
>> Currently, struct ttm_bo_global can only be initialized and released
>> through
>> struct ttm_bo_global_ref. This is a workaround for passing an instance
of
>> ttm_mem_global to the BO global initialization code.
>>
>> The use of struct ttm_bo_global_ref makes driver code unnecessary hard
to
>> understand. At the same time drivers can use any combination of memory
>> and
>> BO for initializing the global instances. This can result in subtle
bugs
>> when the order of initializing and releasing drivers changes.
>>
>> As a first step for resolving these problems, the provided patch set
>> separates initialization and release of struct ttm_bo_global from
>> struct ttm_bo_global_ref.
>>
>> The first patch only renames ttm_bo_global_{init/release}. Hopefully
this
>> change can be applied at once for all drivers.
>>
>> Future directions: All TTM-based drivers follow the same pattern for
>> setting
>> up the TTM. In a follow-up patch, this code can be moved into a single
>> place
>> and shared among drivers.
>>
>> Thomas Zimmermann (2):
>>    drm/ttm: Rename ttm_bo_global_{init,release}() to
>>      ttm_bo_global_ref_*()
>>    drm/ttm: Provide ttm_bo_global_{init/release}() for struct
>>      ttm_bo_global
>>
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  4 +-
>>   drivers/gpu/drm/ast/ast_ttm.c               |  4 +-
>>   drivers/gpu/drm/bochs/bochs_mm.c            |  4 +-
>>   drivers/gpu/drm/cirrus/cirrus_ttm.c         |  4 +-
>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c |  4 +-
>>   drivers/gpu/drm/mgag200/mgag200_ttm.c       |  4 +-
>>   drivers/gpu/drm/nouveau/nouveau_ttm.c       |  4 +-
>>   drivers/gpu/drm/qxl/qxl_ttm.c               |  4 +-
>>   drivers/gpu/drm/radeon/radeon_ttm.c         |  4 +-
>>   drivers/gpu/drm/ttm/ttm_bo.c                | 12 ++---
>>   drivers/gpu/drm/virtio/virtgpu_ttm.c        |  4 +-
>>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c    |  4 +-
>>   drivers/staging/vboxvideo/vbox_ttm.c        |  4 +-
>>   include/drm/ttm/ttm_bo_driver.h             | 53
++++++++++++++++-----
>>   14 files changed, 70 insertions(+), 43 deletions(-)
>>
>> -- 
>> 2.18.0
>>
> 
-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstr. 5, D-90409 Nürnberg
Tel: +49-911-74053-0; Fax: +49-911-7417755;  https://www.suse.com/
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard,
Graham Norton, HRB 21284 (AG Nürnberg)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20180813/1da709d0/attachment-0001.sig>

Thomas Zimmermann

2018-Aug-30 06:34 UTC

head link

[Nouveau] [PATCH 0/2] Provide init/release functions for struct ttm_bo_global

Hi Christian,

I just wanted to ask if there's something else required to get this
patch set reviewed and landed.

On top of these two patches, I have a patch set that replaces the
driver-specific global-bo-and-mem combos with a single struct ttm_global
structure. It further merges struct drm_global into struct ttm_global
and implements it such that drivers can either share the global memory
or create their private instance. It's also a step towards drm device
hotplug, which someone just asked.

Best regards
Thomas


Am 13.08.2018 um 12:33 schrieb Christian König:> Yes, please! I had it on my TODO list to clean that up for an eternity.
> 
> Actually I never understood why that should be driver work to setup TTM?
> 
> I mean can't we just have a module_init/module_exit for TTM?
> 
> Thanks,
> Christian.
> 
> Am 13.08.2018 um 12:24 schrieb Thomas Zimmermann:
>> TTM uses global memory and BO for backing graphics buffers. These are
>> represented by struct ttm_mem_global and struct ttm_bo_global.
>>
>> Currently, struct ttm_bo_global can only be initialized and released
>> through
>> struct ttm_bo_global_ref. This is a workaround for passing an instance
of
>> ttm_mem_global to the BO global initialization code.
>>
>> The use of struct ttm_bo_global_ref makes driver code unnecessary hard
to
>> understand. At the same time drivers can use any combination of memory
>> and
>> BO for initializing the global instances. This can result in subtle
bugs
>> when the order of initializing and releasing drivers changes.
>>
>> As a first step for resolving these problems, the provided patch set
>> separates initialization and release of struct ttm_bo_global from
>> struct ttm_bo_global_ref.
>>
>> The first patch only renames ttm_bo_global_{init/release}. Hopefully
this
>> change can be applied at once for all drivers.
>>
>> Future directions: All TTM-based drivers follow the same pattern for
>> setting
>> up the TTM. In a follow-up patch, this code can be moved into a single
>> place
>> and shared among drivers.
>>
>> Thomas Zimmermann (2):
>>    drm/ttm: Rename ttm_bo_global_{init,release}() to
>>      ttm_bo_global_ref_*()
>>    drm/ttm: Provide ttm_bo_global_{init/release}() for struct
>>      ttm_bo_global
>>
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_ttm.c     |  4 +-
>>   drivers/gpu/drm/ast/ast_ttm.c               |  4 +-
>>   drivers/gpu/drm/bochs/bochs_mm.c            |  4 +-
>>   drivers/gpu/drm/cirrus/cirrus_ttm.c         |  4 +-
>>   drivers/gpu/drm/hisilicon/hibmc/hibmc_ttm.c |  4 +-
>>   drivers/gpu/drm/mgag200/mgag200_ttm.c       |  4 +-
>>   drivers/gpu/drm/nouveau/nouveau_ttm.c       |  4 +-
>>   drivers/gpu/drm/qxl/qxl_ttm.c               |  4 +-
>>   drivers/gpu/drm/radeon/radeon_ttm.c         |  4 +-
>>   drivers/gpu/drm/ttm/ttm_bo.c                | 12 ++---
>>   drivers/gpu/drm/virtio/virtgpu_ttm.c        |  4 +-
>>   drivers/gpu/drm/vmwgfx/vmwgfx_ttm_glue.c    |  4 +-
>>   drivers/staging/vboxvideo/vbox_ttm.c        |  4 +-
>>   include/drm/ttm/ttm_bo_driver.h             | 53
++++++++++++++++-----
>>   14 files changed, 70 insertions(+), 43 deletions(-)
>>
>> -- 
>> 2.18.0
>>
> 
-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Linux GmbH, Maxfeldstr. 5, D-90409 Nürnberg
Tel: +49-911-74053-0; Fax: +49-911-7417755;  https://www.suse.com/
SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard,
Graham Norton, HRB 21284 (AG Nürnberg)

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: OpenPGP digital signature
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20180830/79a2028c/attachment-0001.sig>

Possibly Parallel Threads

Search for more seemingly similar threads

Nouveau - Aug 2018 - [PATCH 0/2] Provide init/release functions for struct ttm_bo_global

[Nouveau] [PATCH 0/2] Provide init/release functions for struct ttm_bo_global

[Nouveau] [PATCH 1/2] drm/ttm: Rename ttm_bo_global_{init, release}() to ttm_bo_global_ref_*()

[Nouveau] [PATCH 2/2] drm/ttm: Provide ttm_bo_global_{init/release}() for struct ttm_bo_global

[PATCH 0/2] Provide init/release functions for struct ttm_bo_global

[Nouveau] [PATCH 0/2] Provide init/release functions for struct ttm_bo_global

[Nouveau] [PATCH 0/2] Provide init/release functions for struct ttm_bo_global

Possibly Parallel Threads