dri-devel.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] embed drm_gem_object into radeon_bo
@ 2010-11-13 20:57 Daniel Vetter
  2010-11-13 20:57 ` [PATCH 1/3] drm/radeon: embed struct drm_gem_object Daniel Vetter
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Daniel Vetter @ 2010-11-13 20:57 UTC (permalink / raw)
  To: dri-devel; +Cc: Daniel Vetter

Hi all,

This patch series embeds drm_gem_object into struct radeon_bo and adjusts
any references. I've decided against embedding the gem stuff into struct
ttm_bo because
- vmwgfx doesn't use gem and
- ttm is used for implementing the memory management, whereas gem provides
  the userspace interface (I know, there's some overlap there, but that's
  not really a new problem).

Please review and consider merging for -next.

Yours, Daniel

Daniel Vetter (3):
  drm/radeon: embed struct drm_gem_object
  drm/radeon: introduce gem_to_radeon_bo helper
  drm/radeon: kill radeon_bo->gobj pointer

 drivers/gpu/drm/radeon/atombios_crtc.c      |    8 ++--
 drivers/gpu/drm/radeon/evergreen_blit_kms.c |    2 +-
 drivers/gpu/drm/radeon/r600.c               |    2 +-
 drivers/gpu/drm/radeon/r600_blit_kms.c      |    2 +-
 drivers/gpu/drm/radeon/radeon.h             |    3 +-
 drivers/gpu/drm/radeon/radeon_benchmark.c   |    4 +-
 drivers/gpu/drm/radeon/radeon_cs.c          |    2 +-
 drivers/gpu/drm/radeon/radeon_device.c      |    4 +-
 drivers/gpu/drm/radeon/radeon_fb.c          |   10 +++---
 drivers/gpu/drm/radeon/radeon_gart.c        |    2 +-
 drivers/gpu/drm/radeon/radeon_gem.c         |   43 ++++++++++++---------------
 drivers/gpu/drm/radeon/radeon_legacy_crtc.c |    4 +-
 drivers/gpu/drm/radeon/radeon_object.c      |   24 +++++++--------
 drivers/gpu/drm/radeon/radeon_object.h      |    2 +-
 drivers/gpu/drm/radeon/radeon_ring.c        |    4 +-
 drivers/gpu/drm/radeon/radeon_test.c        |    4 +-
 drivers/gpu/drm/radeon/radeon_ttm.c         |    2 +-
 drivers/gpu/drm/radeon/rv770.c              |    2 +-
 18 files changed, 59 insertions(+), 65 deletions(-)

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] drm/radeon: embed struct drm_gem_object
  2010-11-13 20:57 [PATCH 0/3] embed drm_gem_object into radeon_bo Daniel Vetter
@ 2010-11-13 20:57 ` Daniel Vetter
  2010-11-13 20:57 ` [PATCH 2/3] drm/radeon: introduce gem_to_radeon_bo helper Daniel Vetter
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2010-11-13 20:57 UTC (permalink / raw)
  To: dri-devel; +Cc: Daniel Vetter

Unconditionally initialize the drm gem object - it's not
worth the trouble not to for the few kernel objects.

This patch only changes the place of the drm gem object,
access is still done via pointers.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/radeon/evergreen_blit_kms.c |    2 +-
 drivers/gpu/drm/radeon/r600.c               |    2 +-
 drivers/gpu/drm/radeon/r600_blit_kms.c      |    2 +-
 drivers/gpu/drm/radeon/radeon.h             |    1 +
 drivers/gpu/drm/radeon/radeon_benchmark.c   |    4 ++--
 drivers/gpu/drm/radeon/radeon_device.c      |    2 +-
 drivers/gpu/drm/radeon/radeon_gart.c        |    2 +-
 drivers/gpu/drm/radeon/radeon_gem.c         |   22 +++++++++-------------
 drivers/gpu/drm/radeon/radeon_object.c      |   16 +++++++++-------
 drivers/gpu/drm/radeon/radeon_object.h      |    2 +-
 drivers/gpu/drm/radeon/radeon_ring.c        |    4 ++--
 drivers/gpu/drm/radeon/radeon_test.c        |    4 ++--
 drivers/gpu/drm/radeon/radeon_ttm.c         |    2 +-
 drivers/gpu/drm/radeon/rv770.c              |    2 +-
 14 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen_blit_kms.c b/drivers/gpu/drm/radeon/evergreen_blit_kms.c
index ac3b6dd..f8b4b1b 100644
--- a/drivers/gpu/drm/radeon/evergreen_blit_kms.c
+++ b/drivers/gpu/drm/radeon/evergreen_blit_kms.c
@@ -459,7 +459,7 @@ int evergreen_blit_init(struct radeon_device *rdev)
 	obj_size += evergreen_ps_size * 4;
 	obj_size = ALIGN(obj_size, 256);
 
-	r = radeon_bo_create(rdev, NULL, obj_size, true, RADEON_GEM_DOMAIN_VRAM,
+	r = radeon_bo_create(rdev, obj_size, true, RADEON_GEM_DOMAIN_VRAM,
 				&rdev->r600_blit.shader_obj);
 	if (r) {
 		DRM_ERROR("evergreen failed to allocate shader\n");
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 33952a1..82376b2 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -2723,7 +2723,7 @@ static int r600_ih_ring_alloc(struct radeon_device *rdev)
 
 	/* Allocate ring buffer */
 	if (rdev->ih.ring_obj == NULL) {
-		r = radeon_bo_create(rdev, NULL, rdev->ih.ring_size,
+		r = radeon_bo_create(rdev, rdev->ih.ring_size,
 				     true,
 				     RADEON_GEM_DOMAIN_GTT,
 				     &rdev->ih.ring_obj);
diff --git a/drivers/gpu/drm/radeon/r600_blit_kms.c b/drivers/gpu/drm/radeon/r600_blit_kms.c
index 8362974..c9cb553 100644
--- a/drivers/gpu/drm/radeon/r600_blit_kms.c
+++ b/drivers/gpu/drm/radeon/r600_blit_kms.c
@@ -501,7 +501,7 @@ int r600_blit_init(struct radeon_device *rdev)
 	obj_size += r6xx_ps_size * 4;
 	obj_size = ALIGN(obj_size, 256);
 
-	r = radeon_bo_create(rdev, NULL, obj_size, true, RADEON_GEM_DOMAIN_VRAM,
+	r = radeon_bo_create(rdev, obj_size, true, RADEON_GEM_DOMAIN_VRAM,
 				&rdev->r600_blit.shader_obj);
 	if (r) {
 		DRM_ERROR("r600 failed to allocate shader\n");
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 73f600d..e2409b9 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -256,6 +256,7 @@ struct radeon_bo {
 	/* Constant after initialization */
 	struct radeon_device		*rdev;
 	struct drm_gem_object		*gobj;
+	struct drm_gem_object		gem_base;
 };
 
 struct radeon_bo_list {
diff --git a/drivers/gpu/drm/radeon/radeon_benchmark.c b/drivers/gpu/drm/radeon/radeon_benchmark.c
index 7932dc4..1bdf3cc 100644
--- a/drivers/gpu/drm/radeon/radeon_benchmark.c
+++ b/drivers/gpu/drm/radeon/radeon_benchmark.c
@@ -41,7 +41,7 @@ void radeon_benchmark_move(struct radeon_device *rdev, unsigned bsize,
 
 	size = bsize;
 	n = 1024;
-	r = radeon_bo_create(rdev, NULL, size, true, sdomain, &sobj);
+	r = radeon_bo_create(rdev, size, true, sdomain, &sobj);
 	if (r) {
 		goto out_cleanup;
 	}
@@ -53,7 +53,7 @@ void radeon_benchmark_move(struct radeon_device *rdev, unsigned bsize,
 	if (r) {
 		goto out_cleanup;
 	}
-	r = radeon_bo_create(rdev, NULL, size, true, ddomain, &dobj);
+	r = radeon_bo_create(rdev, size, true, ddomain, &dobj);
 	if (r) {
 		goto out_cleanup;
 	}
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 8adfedf..6ac28ee 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -180,7 +180,7 @@ int radeon_wb_init(struct radeon_device *rdev)
 	int r;
 
 	if (rdev->wb.wb_obj == NULL) {
-		r = radeon_bo_create(rdev, NULL, RADEON_GPU_PAGE_SIZE, true,
+		r = radeon_bo_create(rdev, RADEON_GPU_PAGE_SIZE, true,
 				RADEON_GEM_DOMAIN_GTT, &rdev->wb.wb_obj);
 		if (r) {
 			dev_warn(rdev->dev, "(%d) create WB bo failed\n", r);
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
index e65b903..344fe82 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -78,7 +78,7 @@ int radeon_gart_table_vram_alloc(struct radeon_device *rdev)
 	int r;
 
 	if (rdev->gart.table.vram.robj == NULL) {
-		r = radeon_bo_create(rdev, NULL, rdev->gart.table_size,
+		r = radeon_bo_create(rdev, rdev->gart.table_size,
 					true, RADEON_GEM_DOMAIN_VRAM,
 					&rdev->gart.table.vram.robj);
 		if (r) {
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index d1e595d..9873a96 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -32,7 +32,8 @@
 
 int radeon_gem_object_init(struct drm_gem_object *obj)
 {
-	/* we do nothings here */
+	BUG();
+
 	return 0;
 }
 
@@ -44,9 +45,6 @@ void radeon_gem_object_free(struct drm_gem_object *gobj)
 	if (robj) {
 		radeon_bo_unref(&robj);
 	}
-
-	drm_gem_object_release(gobj);
-	kfree(gobj);
 }
 
 int radeon_gem_object_create(struct radeon_device *rdev, int size,
@@ -54,29 +52,27 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size,
 				bool discardable, bool kernel,
 				struct drm_gem_object **obj)
 {
-	struct drm_gem_object *gobj;
 	struct radeon_bo *robj;
 	int r;
 
 	*obj = NULL;
-	gobj = drm_gem_object_alloc(rdev->ddev, size);
-	if (!gobj) {
-		return -ENOMEM;
-	}
 	/* At least align on page size */
 	if (alignment < PAGE_SIZE) {
 		alignment = PAGE_SIZE;
 	}
-	r = radeon_bo_create(rdev, gobj, size, kernel, initial_domain, &robj);
+	r = radeon_bo_create(rdev, size, kernel, initial_domain, &robj);
 	if (r) {
 		if (r != -ERESTARTSYS)
 			DRM_ERROR("Failed to allocate GEM object (%d, %d, %u, %d)\n",
 				  size, initial_domain, alignment, r);
-		drm_gem_object_unreference_unlocked(gobj);
 		return r;
 	}
-	gobj->driver_private = robj;
-	*obj = gobj;
+	*obj = &robj->gem_base;
+
+	mutex_lock(&rdev->gem.mutex);
+	list_add_tail(&robj->list, &rdev->gem.objects);
+	mutex_unlock(&rdev->gem.mutex);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index d7ab914..30a2d54 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -54,6 +54,7 @@ static void radeon_ttm_bo_destroy(struct ttm_buffer_object *tbo)
 	list_del_init(&bo->list);
 	mutex_unlock(&bo->rdev->gem.mutex);
 	radeon_bo_clear_surface_reg(bo);
+	drm_gem_object_release(&bo->gem_base);
 	kfree(bo);
 }
 
@@ -85,7 +86,7 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
 	rbo->placement.num_busy_placement = c;
 }
 
-int radeon_bo_create(struct radeon_device *rdev, struct drm_gem_object *gobj,
+int radeon_bo_create(struct radeon_device *rdev,
 			unsigned long size, bool kernel, u32 domain,
 			struct radeon_bo **bo_ptr)
 {
@@ -105,8 +106,14 @@ int radeon_bo_create(struct radeon_device *rdev, struct drm_gem_object *gobj,
 	bo = kzalloc(sizeof(struct radeon_bo), GFP_KERNEL);
 	if (bo == NULL)
 		return -ENOMEM;
+	r = drm_gem_object_init(rdev->ddev, &bo->gem_base, size);
+	if (unlikely(r)) {
+		kfree(bo);
+		return r;
+	}
 	bo->rdev = rdev;
-	bo->gobj = gobj;
+	bo->gobj = &bo->gem_base;
+	bo->gem_base.driver_private = bo;
 	bo->surface_reg = -1;
 	INIT_LIST_HEAD(&bo->list);
 
@@ -131,11 +138,6 @@ retry:
 		return r;
 	}
 	*bo_ptr = bo;
-	if (gobj) {
-		mutex_lock(&bo->rdev->gem.mutex);
-		list_add_tail(&bo->list, &rdev->gem.objects);
-		mutex_unlock(&bo->rdev->gem.mutex);
-	}
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index 3481bc7..c3dd268 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -137,7 +137,7 @@ static inline int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type,
 }
 
 extern int radeon_bo_create(struct radeon_device *rdev,
-				struct drm_gem_object *gobj, unsigned long size,
+				unsigned long size,
 				bool kernel, u32 domain,
 				struct radeon_bo **bo_ptr);
 extern int radeon_bo_kmap(struct radeon_bo *bo, void **ptr);
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 6ea798c..750e965 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -175,7 +175,7 @@ int radeon_ib_pool_init(struct radeon_device *rdev)
 		return 0;
 	INIT_LIST_HEAD(&rdev->ib_pool.bogus_ib);
 	/* Allocate 1M object buffer */
-	r = radeon_bo_create(rdev, NULL,  RADEON_IB_POOL_SIZE*64*1024,
+	r = radeon_bo_create(rdev, RADEON_IB_POOL_SIZE*64*1024,
 				true, RADEON_GEM_DOMAIN_GTT,
 				&rdev->ib_pool.robj);
 	if (r) {
@@ -332,7 +332,7 @@ int radeon_ring_init(struct radeon_device *rdev, unsigned ring_size)
 	rdev->cp.ring_size = ring_size;
 	/* Allocate ring buffer */
 	if (rdev->cp.ring_obj == NULL) {
-		r = radeon_bo_create(rdev, NULL, rdev->cp.ring_size, true,
+		r = radeon_bo_create(rdev, rdev->cp.ring_size, true,
 					RADEON_GEM_DOMAIN_GTT,
 					&rdev->cp.ring_obj);
 		if (r) {
diff --git a/drivers/gpu/drm/radeon/radeon_test.c b/drivers/gpu/drm/radeon/radeon_test.c
index 313c96b..8d7bdd6 100644
--- a/drivers/gpu/drm/radeon/radeon_test.c
+++ b/drivers/gpu/drm/radeon/radeon_test.c
@@ -52,7 +52,7 @@ void radeon_test_moves(struct radeon_device *rdev)
 		goto out_cleanup;
 	}
 
-	r = radeon_bo_create(rdev, NULL, size, true, RADEON_GEM_DOMAIN_VRAM,
+	r = radeon_bo_create(rdev, size, true, RADEON_GEM_DOMAIN_VRAM,
 				&vram_obj);
 	if (r) {
 		DRM_ERROR("Failed to create VRAM object\n");
@@ -71,7 +71,7 @@ void radeon_test_moves(struct radeon_device *rdev)
 		void **gtt_start, **gtt_end;
 		void **vram_start, **vram_end;
 
-		r = radeon_bo_create(rdev, NULL, size, true,
+		r = radeon_bo_create(rdev, size, true,
 					 RADEON_GEM_DOMAIN_GTT, gtt_obj + i);
 		if (r) {
 			DRM_ERROR("Failed to create GTT object %d\n", i);
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index fe95bb3..0c19928 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -529,7 +529,7 @@ int radeon_ttm_init(struct radeon_device *rdev)
 		DRM_ERROR("Failed initializing VRAM heap.\n");
 		return r;
 	}
-	r = radeon_bo_create(rdev, NULL, 256 * 1024, true,
+	r = radeon_bo_create(rdev, 256 * 1024, true,
 				RADEON_GEM_DOMAIN_VRAM,
 				&rdev->stollen_vga_memory);
 	if (r) {
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
index 245374e..0eb74f6 100644
--- a/drivers/gpu/drm/radeon/rv770.c
+++ b/drivers/gpu/drm/radeon/rv770.c
@@ -914,7 +914,7 @@ static int rv770_vram_scratch_init(struct radeon_device *rdev)
 	u64 gpu_addr;
 
 	if (rdev->vram_scratch.robj == NULL) {
-		r = radeon_bo_create(rdev, NULL, RADEON_GPU_PAGE_SIZE,
+		r = radeon_bo_create(rdev, RADEON_GPU_PAGE_SIZE,
 					true, RADEON_GEM_DOMAIN_VRAM,
 					&rdev->vram_scratch.robj);
 		if (r) {
-- 
1.7.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 2/3] drm/radeon: introduce gem_to_radeon_bo helper
  2010-11-13 20:57 [PATCH 0/3] embed drm_gem_object into radeon_bo Daniel Vetter
  2010-11-13 20:57 ` [PATCH 1/3] drm/radeon: embed struct drm_gem_object Daniel Vetter
@ 2010-11-13 20:57 ` Daniel Vetter
  2010-11-13 20:57 ` [PATCH 3/3] drm/radeon: kill radeon_bo->gobj pointer Daniel Vetter
  2010-11-15  7:25 ` [PATCH 0/3] embed drm_gem_object into radeon_bo Thomas Hellstrom
  3 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2010-11-13 20:57 UTC (permalink / raw)
  To: dri-devel; +Cc: Daniel Vetter

... and switch it to container_of upcasting.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/radeon/atombios_crtc.c      |    8 ++++----
 drivers/gpu/drm/radeon/radeon.h             |    1 +
 drivers/gpu/drm/radeon/radeon_cs.c          |    2 +-
 drivers/gpu/drm/radeon/radeon_device.c      |    2 +-
 drivers/gpu/drm/radeon/radeon_fb.c          |   10 +++++-----
 drivers/gpu/drm/radeon/radeon_gem.c         |   21 ++++++++++-----------
 drivers/gpu/drm/radeon/radeon_legacy_crtc.c |    4 ++--
 drivers/gpu/drm/radeon/radeon_object.c      |    3 +--
 8 files changed, 25 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/radeon/atombios_crtc.c b/drivers/gpu/drm/radeon/atombios_crtc.c
index df2b6f2..7ecb154 100644
--- a/drivers/gpu/drm/radeon/atombios_crtc.c
+++ b/drivers/gpu/drm/radeon/atombios_crtc.c
@@ -978,7 +978,7 @@ static int evergreen_crtc_do_set_base(struct drm_crtc *crtc,
 	 * just update base pointers
 	 */
 	obj = radeon_fb->obj;
-	rbo = obj->driver_private;
+	rbo = gem_to_radeon_bo(obj);
 	r = radeon_bo_reserve(rbo, false);
 	if (unlikely(r != 0))
 		return r;
@@ -1086,7 +1086,7 @@ static int evergreen_crtc_do_set_base(struct drm_crtc *crtc,
 
 	if (!atomic && fb && fb != crtc->fb) {
 		radeon_fb = to_radeon_framebuffer(fb);
-		rbo = radeon_fb->obj->driver_private;
+		rbo = gem_to_radeon_bo(radeon_fb->obj);
 		r = radeon_bo_reserve(rbo, false);
 		if (unlikely(r != 0))
 			return r;
@@ -1131,7 +1131,7 @@ static int avivo_crtc_do_set_base(struct drm_crtc *crtc,
 	}
 
 	obj = radeon_fb->obj;
-	rbo = obj->driver_private;
+	rbo = gem_to_radeon_bo(obj);
 	r = radeon_bo_reserve(rbo, false);
 	if (unlikely(r != 0))
 		return r;
@@ -1240,7 +1240,7 @@ static int avivo_crtc_do_set_base(struct drm_crtc *crtc,
 
 	if (!atomic && fb && fb != crtc->fb) {
 		radeon_fb = to_radeon_framebuffer(fb);
-		rbo = radeon_fb->obj->driver_private;
+		rbo = gem_to_radeon_bo(radeon_fb->obj);
 		r = radeon_bo_reserve(rbo, false);
 		if (unlikely(r != 0))
 			return r;
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index e2409b9..239eba7 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -258,6 +258,7 @@ struct radeon_bo {
 	struct drm_gem_object		*gobj;
 	struct drm_gem_object		gem_base;
 };
+#define gem_to_radeon_bo(gobj) container_of((gobj), struct radeon_bo, gem_base)
 
 struct radeon_bo_list {
 	struct list_head	list;
diff --git a/drivers/gpu/drm/radeon/radeon_cs.c b/drivers/gpu/drm/radeon/radeon_cs.c
index 6d64a27..64096f3 100644
--- a/drivers/gpu/drm/radeon/radeon_cs.c
+++ b/drivers/gpu/drm/radeon/radeon_cs.c
@@ -75,7 +75,7 @@ int radeon_cs_parser_relocs(struct radeon_cs_parser *p)
 				return -ENOENT;
 			}
 			p->relocs_ptr[i] = &p->relocs[i];
-			p->relocs[i].robj = p->relocs[i].gobj->driver_private;
+			p->relocs[i].robj = gem_to_radeon_bo(p->relocs[i].gobj);
 			p->relocs[i].lobj.bo = p->relocs[i].robj;
 			p->relocs[i].lobj.rdomain = r->read_domains;
 			p->relocs[i].lobj.wdomain = r->write_domain;
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 6ac28ee..3f42760 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -851,7 +851,7 @@ int radeon_suspend_kms(struct drm_device *dev, pm_message_t state)
 		if (rfb == NULL || rfb->obj == NULL) {
 			continue;
 		}
-		robj = rfb->obj->driver_private;
+		robj = gem_to_radeon_bo(rfb->obj);
 		/* don't unpin kernel fb objects */
 		if (!radeon_fbdev_robj_is_fb(rdev, robj)) {
 			r = radeon_bo_reserve(robj, false);
diff --git a/drivers/gpu/drm/radeon/radeon_fb.c b/drivers/gpu/drm/radeon/radeon_fb.c
index efa2118..99712b3 100644
--- a/drivers/gpu/drm/radeon/radeon_fb.c
+++ b/drivers/gpu/drm/radeon/radeon_fb.c
@@ -90,7 +90,7 @@ static int radeon_align_pitch(struct radeon_device *rdev, int width, int bpp, bo
 
 static void radeonfb_destroy_pinned_object(struct drm_gem_object *gobj)
 {
-	struct radeon_bo *rbo = gobj->driver_private;
+	struct radeon_bo *rbo = gem_to_radeon_bo(gobj);
 	int ret;
 
 	ret = radeon_bo_reserve(rbo, false);
@@ -128,7 +128,7 @@ static int radeonfb_create_pinned_object(struct radeon_fbdev *rfbdev,
 		       aligned_size);
 		return -ENOMEM;
 	}
-	rbo = gobj->driver_private;
+	rbo = gem_to_radeon_bo(gobj);
 
 	if (fb_tiled)
 		tiling_flags = RADEON_TILING_MACRO;
@@ -202,7 +202,7 @@ static int radeonfb_create(struct radeon_fbdev *rfbdev,
 	mode_cmd.depth = sizes->surface_depth;
 
 	ret = radeonfb_create_pinned_object(rfbdev, &mode_cmd, &gobj);
-	rbo = gobj->driver_private;
+	rbo = gem_to_radeon_bo(gobj);
 
 	/* okay we have an object now allocate the framebuffer */
 	info = framebuffer_alloc(0, device);
@@ -405,14 +405,14 @@ int radeon_fbdev_total_size(struct radeon_device *rdev)
 	struct radeon_bo *robj;
 	int size = 0;
 
-	robj = rdev->mode_info.rfbdev->rfb.obj->driver_private;
+	robj = gem_to_radeon_bo(rdev->mode_info.rfbdev->rfb.obj);
 	size += radeon_bo_size(robj);
 	return size;
 }
 
 bool radeon_fbdev_robj_is_fb(struct radeon_device *rdev, struct radeon_bo *robj)
 {
-	if (robj == rdev->mode_info.rfbdev->rfb.obj->driver_private)
+	if (robj == gem_to_radeon_bo(rdev->mode_info.rfbdev->rfb.obj))
 		return true;
 	return false;
 }
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index 9873a96..072d21c 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -39,9 +39,8 @@ int radeon_gem_object_init(struct drm_gem_object *obj)
 
 void radeon_gem_object_free(struct drm_gem_object *gobj)
 {
-	struct radeon_bo *robj = gobj->driver_private;
+	struct radeon_bo *robj = gem_to_radeon_bo(gobj);
 
-	gobj->driver_private = NULL;
 	if (robj) {
 		radeon_bo_unref(&robj);
 	}
@@ -79,7 +78,7 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size,
 int radeon_gem_object_pin(struct drm_gem_object *obj, uint32_t pin_domain,
 			  uint64_t *gpu_addr)
 {
-	struct radeon_bo *robj = obj->driver_private;
+	struct radeon_bo *robj = gem_to_radeon_bo(obj);
 	int r;
 
 	r = radeon_bo_reserve(robj, false);
@@ -92,7 +91,7 @@ int radeon_gem_object_pin(struct drm_gem_object *obj, uint32_t pin_domain,
 
 void radeon_gem_object_unpin(struct drm_gem_object *obj)
 {
-	struct radeon_bo *robj = obj->driver_private;
+	struct radeon_bo *robj = gem_to_radeon_bo(obj);
 	int r;
 
 	r = radeon_bo_reserve(robj, false);
@@ -110,7 +109,7 @@ int radeon_gem_set_domain(struct drm_gem_object *gobj,
 	int r;
 
 	/* FIXME: reeimplement */
-	robj = gobj->driver_private;
+	robj = gem_to_radeon_bo(gobj);
 	/* work out where to validate the buffer to */
 	domain = wdomain;
 	if (!domain) {
@@ -224,7 +223,7 @@ int radeon_gem_set_domain_ioctl(struct drm_device *dev, void *data,
 	if (gobj == NULL) {
 		return -ENOENT;
 	}
-	robj = gobj->driver_private;
+	robj = gem_to_radeon_bo(gobj);
 
 	r = radeon_gem_set_domain(gobj, args->read_domains, args->write_domain);
 
@@ -243,7 +242,7 @@ int radeon_gem_mmap_ioctl(struct drm_device *dev, void *data,
 	if (gobj == NULL) {
 		return -ENOENT;
 	}
-	robj = gobj->driver_private;
+	robj = gem_to_radeon_bo(gobj);
 	args->addr_ptr = radeon_bo_mmap_offset(robj);
 	drm_gem_object_unreference_unlocked(gobj);
 	return 0;
@@ -262,7 +261,7 @@ int radeon_gem_busy_ioctl(struct drm_device *dev, void *data,
 	if (gobj == NULL) {
 		return -ENOENT;
 	}
-	robj = gobj->driver_private;
+	robj = gem_to_radeon_bo(gobj);
 	r = radeon_bo_wait(robj, &cur_placement, true);
 	switch (cur_placement) {
 	case TTM_PL_VRAM:
@@ -292,7 +291,7 @@ int radeon_gem_wait_idle_ioctl(struct drm_device *dev, void *data,
 	if (gobj == NULL) {
 		return -ENOENT;
 	}
-	robj = gobj->driver_private;
+	robj = gem_to_radeon_bo(gobj);
 	r = radeon_bo_wait(robj, NULL, false);
 	/* callback hw specific functions if any */
 	if (robj->rdev->asic->ioctl_wait_idle)
@@ -313,7 +312,7 @@ int radeon_gem_set_tiling_ioctl(struct drm_device *dev, void *data,
 	gobj = drm_gem_object_lookup(dev, filp, args->handle);
 	if (gobj == NULL)
 		return -ENOENT;
-	robj = gobj->driver_private;
+	robj = gem_to_radeon_bo(gobj);
 	r = radeon_bo_set_tiling_flags(robj, args->tiling_flags, args->pitch);
 	drm_gem_object_unreference_unlocked(gobj);
 	return r;
@@ -331,7 +330,7 @@ int radeon_gem_get_tiling_ioctl(struct drm_device *dev, void *data,
 	gobj = drm_gem_object_lookup(dev, filp, args->handle);
 	if (gobj == NULL)
 		return -ENOENT;
-	rbo = gobj->driver_private;
+	rbo = gem_to_radeon_bo(gobj);
 	r = radeon_bo_reserve(rbo, false);
 	if (unlikely(r != 0))
 		goto out;
diff --git a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
index ace2e63..ee76d29 100644
--- a/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
+++ b/drivers/gpu/drm/radeon/radeon_legacy_crtc.c
@@ -415,7 +415,7 @@ int radeon_crtc_do_set_base(struct drm_crtc *crtc,
 
 	/* Pin framebuffer & get tilling informations */
 	obj = radeon_fb->obj;
-	rbo = obj->driver_private;
+	rbo = gem_to_radeon_bo(obj);
 	r = radeon_bo_reserve(rbo, false);
 	if (unlikely(r != 0))
 		return r;
@@ -520,7 +520,7 @@ int radeon_crtc_do_set_base(struct drm_crtc *crtc,
 
 	if (!atomic && fb && fb != crtc->fb) {
 		radeon_fb = to_radeon_framebuffer(fb);
-		rbo = radeon_fb->obj->driver_private;
+		rbo = gem_to_radeon_bo(radeon_fb->obj);
 		r = radeon_bo_reserve(rbo, false);
 		if (unlikely(r != 0))
 			return r;
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 30a2d54..99efe4a 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -113,7 +113,7 @@ int radeon_bo_create(struct radeon_device *rdev,
 	}
 	bo->rdev = rdev;
 	bo->gobj = &bo->gem_base;
-	bo->gem_base.driver_private = bo;
+	bo->gem_base.driver_private = NULL;
 	bo->surface_reg = -1;
 	INIT_LIST_HEAD(&bo->list);
 
@@ -266,7 +266,6 @@ void radeon_bo_force_delete(struct radeon_device *rdev)
 		list_del_init(&bo->list);
 		mutex_unlock(&bo->rdev->gem.mutex);
 		radeon_bo_unref(&bo);
-		gobj->driver_private = NULL;
 		drm_gem_object_unreference(gobj);
 		mutex_unlock(&rdev->ddev->struct_mutex);
 	}
-- 
1.7.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 3/3] drm/radeon: kill radeon_bo->gobj pointer
  2010-11-13 20:57 [PATCH 0/3] embed drm_gem_object into radeon_bo Daniel Vetter
  2010-11-13 20:57 ` [PATCH 1/3] drm/radeon: embed struct drm_gem_object Daniel Vetter
  2010-11-13 20:57 ` [PATCH 2/3] drm/radeon: introduce gem_to_radeon_bo helper Daniel Vetter
@ 2010-11-13 20:57 ` Daniel Vetter
  2010-11-15  7:25 ` [PATCH 0/3] embed drm_gem_object into radeon_bo Thomas Hellstrom
  3 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2010-11-13 20:57 UTC (permalink / raw)
  To: dri-devel; +Cc: Daniel Vetter

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/radeon/radeon.h        |    1 -
 drivers/gpu/drm/radeon/radeon_object.c |    9 +++------
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 239eba7..9464100 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -255,7 +255,6 @@ struct radeon_bo {
 	int				surface_reg;
 	/* Constant after initialization */
 	struct radeon_device		*rdev;
-	struct drm_gem_object		*gobj;
 	struct drm_gem_object		gem_base;
 };
 #define gem_to_radeon_bo(gobj) container_of((gobj), struct radeon_bo, gem_base)
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index 99efe4a..18db45f 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -112,7 +112,6 @@ int radeon_bo_create(struct radeon_device *rdev,
 		return r;
 	}
 	bo->rdev = rdev;
-	bo->gobj = &bo->gem_base;
 	bo->gem_base.driver_private = NULL;
 	bo->surface_reg = -1;
 	INIT_LIST_HEAD(&bo->list);
@@ -250,7 +249,6 @@ int radeon_bo_evict_vram(struct radeon_device *rdev)
 void radeon_bo_force_delete(struct radeon_device *rdev)
 {
 	struct radeon_bo *bo, *n;
-	struct drm_gem_object *gobj;
 
 	if (list_empty(&rdev->gem.objects)) {
 		return;
@@ -258,15 +256,14 @@ void radeon_bo_force_delete(struct radeon_device *rdev)
 	dev_err(rdev->dev, "Userspace still has active objects !\n");
 	list_for_each_entry_safe(bo, n, &rdev->gem.objects, list) {
 		mutex_lock(&rdev->ddev->struct_mutex);
-		gobj = bo->gobj;
 		dev_err(rdev->dev, "%p %p %lu %lu force free\n",
-			gobj, bo, (unsigned long)gobj->size,
-			*((unsigned long *)&gobj->refcount));
+			&bo->gem_base, bo, (unsigned long)bo->gem_base.size,
+			*((unsigned long *)&bo->gem_base.refcount));
 		mutex_lock(&bo->rdev->gem.mutex);
 		list_del_init(&bo->list);
 		mutex_unlock(&bo->rdev->gem.mutex);
 		radeon_bo_unref(&bo);
-		drm_gem_object_unreference(gobj);
+		drm_gem_object_unreference(&bo->gem_base);
 		mutex_unlock(&rdev->ddev->struct_mutex);
 	}
 }
-- 
1.7.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] embed drm_gem_object into radeon_bo
  2010-11-13 20:57 [PATCH 0/3] embed drm_gem_object into radeon_bo Daniel Vetter
                   ` (2 preceding siblings ...)
  2010-11-13 20:57 ` [PATCH 3/3] drm/radeon: kill radeon_bo->gobj pointer Daniel Vetter
@ 2010-11-15  7:25 ` Thomas Hellstrom
  2010-11-15 18:45   ` Daniel Vetter
  3 siblings, 1 reply; 8+ messages in thread
From: Thomas Hellstrom @ 2010-11-15  7:25 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: dri-devel

Hi, Daniel,

My main concerns previously for embedding GEM objects as user-space 
references for TTM has been twofold and implementation specific.

1) The locking has been using global mutexes where local spin- or RCU 
locks have been more appropriate. It looks like this has finally been / 
is finally going to be addressed.

2) The gem object is too specialized for general purpose use:
a) The shmem member has no natural use with TTM except possibly as a 
persistent swap storage, but in an ideal world, TTM would talk to the 
swap cache directly so in the longer term there is no need for the shmem 
object at all.
b) Implementations may want to use other user-space visible objects than 
buffer objects:
For example fence objects or CPU synchronization objects. The common 
traits of / operations on these are user-space visibility, inter-process 
visibility, refcounting and destruction when the relevant file is closed.

Hence a class that provides only the user-space handles, naming (flink), 
refcounting and registering with a file handle would be the best choice 
of a "base" class. Traditional Gem objects could derive from those and 
provide any extra *generic* members needed for buffer objects.

This doesn't really affect your work though. Just some comments on why 
vmwgfx don't use GEM objects by default and how they could be made 
optimal for TTM-aware drivers.

Thanks,
/Thomas


On 11/13/2010 09:57 PM, Daniel Vetter wrote:
> Hi all,
>
> This patch series embeds drm_gem_object into struct radeon_bo and adjusts
> any references. I've decided against embedding the gem stuff into struct
> ttm_bo because
> - vmwgfx doesn't use gem and
> - ttm is used for implementing the memory management, whereas gem provides
>    the userspace interface (I know, there's some overlap there, but that's
>    not really a new problem).
>
> Please review and consider merging for -next.
>
> Yours, Daniel
>
> Daniel Vetter (3):
>    drm/radeon: embed struct drm_gem_object
>    drm/radeon: introduce gem_to_radeon_bo helper
>    drm/radeon: kill radeon_bo->gobj pointer
>
>   drivers/gpu/drm/radeon/atombios_crtc.c      |    8 ++--
>   drivers/gpu/drm/radeon/evergreen_blit_kms.c |    2 +-
>   drivers/gpu/drm/radeon/r600.c               |    2 +-
>   drivers/gpu/drm/radeon/r600_blit_kms.c      |    2 +-
>   drivers/gpu/drm/radeon/radeon.h             |    3 +-
>   drivers/gpu/drm/radeon/radeon_benchmark.c   |    4 +-
>   drivers/gpu/drm/radeon/radeon_cs.c          |    2 +-
>   drivers/gpu/drm/radeon/radeon_device.c      |    4 +-
>   drivers/gpu/drm/radeon/radeon_fb.c          |   10 +++---
>   drivers/gpu/drm/radeon/radeon_gart.c        |    2 +-
>   drivers/gpu/drm/radeon/radeon_gem.c         |   43 ++++++++++++---------------
>   drivers/gpu/drm/radeon/radeon_legacy_crtc.c |    4 +-
>   drivers/gpu/drm/radeon/radeon_object.c      |   24 +++++++--------
>   drivers/gpu/drm/radeon/radeon_object.h      |    2 +-
>   drivers/gpu/drm/radeon/radeon_ring.c        |    4 +-
>   drivers/gpu/drm/radeon/radeon_test.c        |    4 +-
>   drivers/gpu/drm/radeon/radeon_ttm.c         |    2 +-
>   drivers/gpu/drm/radeon/rv770.c              |    2 +-
>   18 files changed, 59 insertions(+), 65 deletions(-)
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
>    

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] embed drm_gem_object into radeon_bo
  2010-11-15  7:25 ` [PATCH 0/3] embed drm_gem_object into radeon_bo Thomas Hellstrom
@ 2010-11-15 18:45   ` Daniel Vetter
  2010-11-15 20:48     ` Thomas Hellstrom
  0 siblings, 1 reply; 8+ messages in thread
From: Daniel Vetter @ 2010-11-15 18:45 UTC (permalink / raw)
  To: Thomas Hellstrom; +Cc: Daniel Vetter, dri-devel

Hi Thomas,

Thanks for your comments about ttm and vmwgfx. Some of my own ideas about
where this might all be heading below.

On Mon, Nov 15, 2010 at 08:25:16AM +0100, Thomas Hellstrom wrote:
> Hi, Daniel,
> 
> My main concerns previously for embedding GEM objects as user-space
> references for TTM has been twofold and implementation specific.
> 
> 1) The locking has been using global mutexes where local spin- or
> RCU locks have been more appropriate. It looks like this has finally
> been / is finally going to be addressed.

gem object lookup / insert has always (iirc) been protected by a spinlock.
drm/i915 is just a bit inefficient with lookups, hence this spinlock
showing up in profiling.
 
> 2) The gem object is too specialized for general purpose use:
> a) The shmem member has no natural use with TTM except possibly as a
> persistent swap storage, but in an ideal world, TTM would talk to
> the swap cache directly so in the longer term there is no need for
> the shmem object at all.

Chris Wilson is working on a gem_vm for i915 that employs a address_space
per bo and directly manages swap entries and has its own page allocator
(actualy cflushed page cache). I haven't yet looked into this closely
(especially the ttm part), but this might (at least in parts) be shareable
between i915 and ttm.  At least it gets rid of the shmfs inode in struct
drm_gem_object!

> b) Implementations may want to use other user-space visible objects
> than buffer objects:
> For example fence objects or CPU synchronization objects. The common
> traits of / operations on these are user-space visibility,
> inter-process visibility, refcounting and destruction when the
> relevant file is closed.
> 
> Hence a class that provides only the user-space handles, naming
> (flink), refcounting and registering with a file handle would be the
> best choice of a "base" class. Traditional Gem objects could derive
> from those and provide any extra *generic* members needed for buffer
> objects.
> 
> This doesn't really affect your work though. Just some comments on
> why vmwgfx don't use GEM objects by default and how they could be
> made optimal for TTM-aware drivers.

Yeah, I've noticed that vmwgfx is the (sometimes) sole user of many of the
more fancy stuff in ttm. And I also don't like the duplication between gem
and ttm. My plan (i.e. wishful thinking) is to have a common set of helper
functions that can be shared between i915, radeon and nouveau and any
other driver with a gem-like interface (i.e. buffer-objects + execbuffer,
gpu<->cpu sync abstracted away by the kernel). Leaving the cracy stuff for
drivers that need it (vmwgfx). Nothing concrete though (besides a new
testing rig to start hacking on nouveau), I'll intend to simply plow
along, hunting for common patterns.

> Thanks,
> /Thomas

Yours, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 0/3] embed drm_gem_object into radeon_bo
  2010-11-15 18:45   ` Daniel Vetter
@ 2010-11-15 20:48     ` Thomas Hellstrom
  0 siblings, 0 replies; 8+ messages in thread
From: Thomas Hellstrom @ 2010-11-15 20:48 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Daniel Vetter, dri-devel

On 11/15/2010 07:45 PM, Daniel Vetter wrote:
> Hi Thomas,
>
> Thanks for your comments about ttm and vmwgfx. Some of my own ideas about
> where this might all be heading below.
>
> On Mon, Nov 15, 2010 at 08:25:16AM +0100, Thomas Hellstrom wrote:
>    
>> Hi, Daniel,
>>
>> My main concerns previously for embedding GEM objects as user-space
>> references for TTM has been twofold and implementation specific.
>>
>> 1) The locking has been using global mutexes where local spin- or
>> RCU locks have been more appropriate. It looks like this has finally
>> been / is finally going to be addressed.
>>      
> gem object lookup / insert has always (iirc) been protected by a spinlock.
> drm/i915 is just a bit inefficient with lookups, hence this spinlock
> showing up in profiling.
>
>    
>> 2) The gem object is too specialized for general purpose use:
>> a) The shmem member has no natural use with TTM except possibly as a
>> persistent swap storage, but in an ideal world, TTM would talk to
>> the swap cache directly so in the longer term there is no need for
>> the shmem object at all.
>>      
> Chris Wilson is working on a gem_vm for i915 that employs a address_space
> per bo and directly manages swap entries and has its own page allocator
> (actualy cflushed page cache). I haven't yet looked into this closely
> (especially the ttm part), but this might (at least in parts) be shareable
> between i915 and ttm.  At least it gets rid of the shmfs inode in struct
> drm_gem_object!
>
>    
>> b) Implementations may want to use other user-space visible objects
>> than buffer objects:
>> For example fence objects or CPU synchronization objects. The common
>> traits of / operations on these are user-space visibility,
>> inter-process visibility, refcounting and destruction when the
>> relevant file is closed.
>>
>> Hence a class that provides only the user-space handles, naming
>> (flink), refcounting and registering with a file handle would be the
>> best choice of a "base" class. Traditional Gem objects could derive
>> from those and provide any extra *generic* members needed for buffer
>> objects.
>>
>> This doesn't really affect your work though. Just some comments on
>> why vmwgfx don't use GEM objects by default and how they could be
>> made optimal for TTM-aware drivers.
>>      
> Yeah, I've noticed that vmwgfx is the (sometimes) sole user of many of the
> more fancy stuff in ttm. And I also don't like the duplication between gem
> and ttm. My plan (i.e. wishful thinking) is to have a common set of helper
> functions that can be shared between i915, radeon and nouveau and any
> other driver with a gem-like interface (i.e. buffer-objects + execbuffer,
> gpu<->cpu sync abstracted away by the kernel). Leaving the cracy stuff for
> drivers that need it (vmwgfx). Nothing concrete though (besides a new
> testing rig to start hacking on nouveau), I'll intend to simply plow
> along, hunting for common patterns.
>    

Sounds good. I actually think the only thing which is currently 
vmwgfx-specific are the ttm_objects and the ttm_locks. TTM objects could 
really be replaced by gem objects with the bo-specific part stripped, as 
discussed above. And then there is the ttm lock to allow concurrent 
execbufs running.

/Thomas






>    
>> Thanks,
>> /Thomas
>>      
> Yours, Daniel
>    

^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/3] drm/radeon: embed struct drm_gem_object
  2010-11-27  9:47 Daniel Vetter
@ 2010-11-27  9:47 ` Daniel Vetter
  0 siblings, 0 replies; 8+ messages in thread
From: Daniel Vetter @ 2010-11-27  9:47 UTC (permalink / raw)
  To: airlied; +Cc: Daniel Vetter, dri-devel

Unconditionally initialize the drm gem object - it's not
worth the trouble not to for the few kernel objects.

This patch only changes the place of the drm gem object,
access is still done via pointers.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/radeon/evergreen_blit_kms.c |    2 +-
 drivers/gpu/drm/radeon/r600.c               |    2 +-
 drivers/gpu/drm/radeon/r600_blit_kms.c      |    2 +-
 drivers/gpu/drm/radeon/radeon.h             |    1 +
 drivers/gpu/drm/radeon/radeon_benchmark.c   |    4 ++--
 drivers/gpu/drm/radeon/radeon_device.c      |    2 +-
 drivers/gpu/drm/radeon/radeon_gart.c        |    2 +-
 drivers/gpu/drm/radeon/radeon_gem.c         |   22 +++++++++-------------
 drivers/gpu/drm/radeon/radeon_object.c      |   16 +++++++++-------
 drivers/gpu/drm/radeon/radeon_object.h      |    7 +++----
 drivers/gpu/drm/radeon/radeon_ring.c        |    4 ++--
 drivers/gpu/drm/radeon/radeon_test.c        |    4 ++--
 drivers/gpu/drm/radeon/radeon_ttm.c         |    2 +-
 drivers/gpu/drm/radeon/rv770.c              |    2 +-
 14 files changed, 35 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/radeon/evergreen_blit_kms.c b/drivers/gpu/drm/radeon/evergreen_blit_kms.c
index 2ccd1f0..0370dab 100644
--- a/drivers/gpu/drm/radeon/evergreen_blit_kms.c
+++ b/drivers/gpu/drm/radeon/evergreen_blit_kms.c
@@ -482,7 +482,7 @@ int evergreen_blit_init(struct radeon_device *rdev)
 	obj_size += evergreen_ps_size * 4;
 	obj_size = ALIGN(obj_size, 256);
 
-	r = radeon_bo_create(rdev, NULL, obj_size, PAGE_SIZE, true, RADEON_GEM_DOMAIN_VRAM,
+	r = radeon_bo_create(rdev, obj_size, PAGE_SIZE, true, RADEON_GEM_DOMAIN_VRAM,
 				&rdev->r600_blit.shader_obj);
 	if (r) {
 		DRM_ERROR("evergreen failed to allocate shader\n");
diff --git a/drivers/gpu/drm/radeon/r600.c b/drivers/gpu/drm/radeon/r600.c
index 53bfe3a..b55ee28 100644
--- a/drivers/gpu/drm/radeon/r600.c
+++ b/drivers/gpu/drm/radeon/r600.c
@@ -2724,7 +2724,7 @@ static int r600_ih_ring_alloc(struct radeon_device *rdev)
 
 	/* Allocate ring buffer */
 	if (rdev->ih.ring_obj == NULL) {
-		r = radeon_bo_create(rdev, NULL, rdev->ih.ring_size,
+		r = radeon_bo_create(rdev, rdev->ih.ring_size,
 				     PAGE_SIZE, true,
 				     RADEON_GEM_DOMAIN_GTT,
 				     &rdev->ih.ring_obj);
diff --git a/drivers/gpu/drm/radeon/r600_blit_kms.c b/drivers/gpu/drm/radeon/r600_blit_kms.c
index 86e5aa0..16e211a 100644
--- a/drivers/gpu/drm/radeon/r600_blit_kms.c
+++ b/drivers/gpu/drm/radeon/r600_blit_kms.c
@@ -501,7 +501,7 @@ int r600_blit_init(struct radeon_device *rdev)
 	obj_size += r6xx_ps_size * 4;
 	obj_size = ALIGN(obj_size, 256);
 
-	r = radeon_bo_create(rdev, NULL, obj_size, PAGE_SIZE, true, RADEON_GEM_DOMAIN_VRAM,
+	r = radeon_bo_create(rdev, obj_size, PAGE_SIZE, true, RADEON_GEM_DOMAIN_VRAM,
 				&rdev->r600_blit.shader_obj);
 	if (r) {
 		DRM_ERROR("r600 failed to allocate shader\n");
diff --git a/drivers/gpu/drm/radeon/radeon.h b/drivers/gpu/drm/radeon/radeon.h
index 431d418..69e6198 100644
--- a/drivers/gpu/drm/radeon/radeon.h
+++ b/drivers/gpu/drm/radeon/radeon.h
@@ -258,6 +258,7 @@ struct radeon_bo {
 	/* Constant after initialization */
 	struct radeon_device		*rdev;
 	struct drm_gem_object		*gobj;
+	struct drm_gem_object		gem_base;
 };
 
 struct radeon_bo_list {
diff --git a/drivers/gpu/drm/radeon/radeon_benchmark.c b/drivers/gpu/drm/radeon/radeon_benchmark.c
index c558685..10191d9 100644
--- a/drivers/gpu/drm/radeon/radeon_benchmark.c
+++ b/drivers/gpu/drm/radeon/radeon_benchmark.c
@@ -41,7 +41,7 @@ void radeon_benchmark_move(struct radeon_device *rdev, unsigned bsize,
 
 	size = bsize;
 	n = 1024;
-	r = radeon_bo_create(rdev, NULL, size, PAGE_SIZE, true, sdomain, &sobj);
+	r = radeon_bo_create(rdev, size, PAGE_SIZE, true, sdomain, &sobj);
 	if (r) {
 		goto out_cleanup;
 	}
@@ -53,7 +53,7 @@ void radeon_benchmark_move(struct radeon_device *rdev, unsigned bsize,
 	if (r) {
 		goto out_cleanup;
 	}
-	r = radeon_bo_create(rdev, NULL, size, PAGE_SIZE, true, ddomain, &dobj);
+	r = radeon_bo_create(rdev, size, PAGE_SIZE, true, ddomain, &dobj);
 	if (r) {
 		goto out_cleanup;
 	}
diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index dd93c9c..1732b56 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -180,7 +180,7 @@ int radeon_wb_init(struct radeon_device *rdev)
 	int r;
 
 	if (rdev->wb.wb_obj == NULL) {
-		r = radeon_bo_create(rdev, NULL, RADEON_GPU_PAGE_SIZE, PAGE_SIZE, true,
+		r = radeon_bo_create(rdev, RADEON_GPU_PAGE_SIZE, PAGE_SIZE, true,
 				RADEON_GEM_DOMAIN_GTT, &rdev->wb.wb_obj);
 		if (r) {
 			dev_warn(rdev->dev, "(%d) create WB bo failed\n", r);
diff --git a/drivers/gpu/drm/radeon/radeon_gart.c b/drivers/gpu/drm/radeon/radeon_gart.c
index 6501611..9ac1bf0 100644
--- a/drivers/gpu/drm/radeon/radeon_gart.c
+++ b/drivers/gpu/drm/radeon/radeon_gart.c
@@ -78,7 +78,7 @@ int radeon_gart_table_vram_alloc(struct radeon_device *rdev)
 	int r;
 
 	if (rdev->gart.table.vram.robj == NULL) {
-		r = radeon_bo_create(rdev, NULL, rdev->gart.table_size,
+		r = radeon_bo_create(rdev, rdev->gart.table_size,
 				     PAGE_SIZE, true, RADEON_GEM_DOMAIN_VRAM,
 				     &rdev->gart.table.vram.robj);
 		if (r) {
diff --git a/drivers/gpu/drm/radeon/radeon_gem.c b/drivers/gpu/drm/radeon/radeon_gem.c
index df95eb8..175cd2c 100644
--- a/drivers/gpu/drm/radeon/radeon_gem.c
+++ b/drivers/gpu/drm/radeon/radeon_gem.c
@@ -32,7 +32,8 @@
 
 int radeon_gem_object_init(struct drm_gem_object *obj)
 {
-	/* we do nothings here */
+	BUG();
+
 	return 0;
 }
 
@@ -44,9 +45,6 @@ void radeon_gem_object_free(struct drm_gem_object *gobj)
 	if (robj) {
 		radeon_bo_unref(&robj);
 	}
-
-	drm_gem_object_release(gobj);
-	kfree(gobj);
 }
 
 int radeon_gem_object_create(struct radeon_device *rdev, int size,
@@ -54,29 +52,27 @@ int radeon_gem_object_create(struct radeon_device *rdev, int size,
 				bool discardable, bool kernel,
 				struct drm_gem_object **obj)
 {
-	struct drm_gem_object *gobj;
 	struct radeon_bo *robj;
 	int r;
 
 	*obj = NULL;
-	gobj = drm_gem_object_alloc(rdev->ddev, size);
-	if (!gobj) {
-		return -ENOMEM;
-	}
 	/* At least align on page size */
 	if (alignment < PAGE_SIZE) {
 		alignment = PAGE_SIZE;
 	}
-	r = radeon_bo_create(rdev, gobj, size, alignment, kernel, initial_domain, &robj);
+	r = radeon_bo_create(rdev, size, alignment, kernel, initial_domain, &robj);
 	if (r) {
 		if (r != -ERESTARTSYS)
 			DRM_ERROR("Failed to allocate GEM object (%d, %d, %u, %d)\n",
 				  size, initial_domain, alignment, r);
-		drm_gem_object_unreference_unlocked(gobj);
 		return r;
 	}
-	gobj->driver_private = robj;
-	*obj = gobj;
+	*obj = &robj->gem_base;
+
+	mutex_lock(&rdev->gem.mutex);
+	list_add_tail(&robj->list, &rdev->gem.objects);
+	mutex_unlock(&rdev->gem.mutex);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_object.c b/drivers/gpu/drm/radeon/radeon_object.c
index a8594d2..8af4c85 100644
--- a/drivers/gpu/drm/radeon/radeon_object.c
+++ b/drivers/gpu/drm/radeon/radeon_object.c
@@ -54,6 +54,7 @@ static void radeon_ttm_bo_destroy(struct ttm_buffer_object *tbo)
 	list_del_init(&bo->list);
 	mutex_unlock(&bo->rdev->gem.mutex);
 	radeon_bo_clear_surface_reg(bo);
+	drm_gem_object_release(&bo->gem_base);
 	kfree(bo);
 }
 
@@ -85,7 +86,7 @@ void radeon_ttm_placement_from_domain(struct radeon_bo *rbo, u32 domain)
 	rbo->placement.num_busy_placement = c;
 }
 
-int radeon_bo_create(struct radeon_device *rdev, struct drm_gem_object *gobj,
+int radeon_bo_create(struct radeon_device *rdev,
 		     unsigned long size, int byte_align, bool kernel, u32 domain,
 		     struct radeon_bo **bo_ptr)
 {
@@ -108,8 +109,14 @@ retry:
 	bo = kzalloc(sizeof(struct radeon_bo), GFP_KERNEL);
 	if (bo == NULL)
 		return -ENOMEM;
+	r = drm_gem_object_init(rdev->ddev, &bo->gem_base, size);
+	if (unlikely(r)) {
+		kfree(bo);
+		return r;
+	}
 	bo->rdev = rdev;
-	bo->gobj = gobj;
+	bo->gobj = &bo->gem_base;
+	bo->gem_base.driver_private = bo;
 	bo->surface_reg = -1;
 	INIT_LIST_HEAD(&bo->list);
 	radeon_ttm_placement_from_domain(bo, domain);
@@ -132,11 +139,6 @@ retry:
 		return r;
 	}
 	*bo_ptr = bo;
-	if (gobj) {
-		mutex_lock(&bo->rdev->gem.mutex);
-		list_add_tail(&bo->list, &rdev->gem.objects);
-		mutex_unlock(&bo->rdev->gem.mutex);
-	}
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/radeon/radeon_object.h b/drivers/gpu/drm/radeon/radeon_object.h
index 22d4c23..7f8e778 100644
--- a/drivers/gpu/drm/radeon/radeon_object.h
+++ b/drivers/gpu/drm/radeon/radeon_object.h
@@ -137,10 +137,9 @@ static inline int radeon_bo_wait(struct radeon_bo *bo, u32 *mem_type,
 }
 
 extern int radeon_bo_create(struct radeon_device *rdev,
-			    struct drm_gem_object *gobj, unsigned long size,
-			    int byte_align,
-			    bool kernel, u32 domain,
-			    struct radeon_bo **bo_ptr);
+				unsigned long size, int byte_align,
+				bool kernel, u32 domain,
+				struct radeon_bo **bo_ptr);
 extern int radeon_bo_kmap(struct radeon_bo *bo, void **ptr);
 extern void radeon_bo_kunmap(struct radeon_bo *bo);
 extern void radeon_bo_unref(struct radeon_bo **bo);
diff --git a/drivers/gpu/drm/radeon/radeon_ring.c b/drivers/gpu/drm/radeon/radeon_ring.c
index 06e7982..992d99d 100644
--- a/drivers/gpu/drm/radeon/radeon_ring.c
+++ b/drivers/gpu/drm/radeon/radeon_ring.c
@@ -175,7 +175,7 @@ int radeon_ib_pool_init(struct radeon_device *rdev)
 		return 0;
 	INIT_LIST_HEAD(&rdev->ib_pool.bogus_ib);
 	/* Allocate 1M object buffer */
-	r = radeon_bo_create(rdev, NULL,  RADEON_IB_POOL_SIZE*64*1024,
+	r = radeon_bo_create(rdev, RADEON_IB_POOL_SIZE*64*1024,
 			     PAGE_SIZE, true, RADEON_GEM_DOMAIN_GTT,
 			     &rdev->ib_pool.robj);
 	if (r) {
@@ -332,7 +332,7 @@ int radeon_ring_init(struct radeon_device *rdev, unsigned ring_size)
 	rdev->cp.ring_size = ring_size;
 	/* Allocate ring buffer */
 	if (rdev->cp.ring_obj == NULL) {
-		r = radeon_bo_create(rdev, NULL, rdev->cp.ring_size, PAGE_SIZE, true,
+		r = radeon_bo_create(rdev, rdev->cp.ring_size, PAGE_SIZE, true,
 					RADEON_GEM_DOMAIN_GTT,
 					&rdev->cp.ring_obj);
 		if (r) {
diff --git a/drivers/gpu/drm/radeon/radeon_test.c b/drivers/gpu/drm/radeon/radeon_test.c
index 5b44f65..dee4a0c 100644
--- a/drivers/gpu/drm/radeon/radeon_test.c
+++ b/drivers/gpu/drm/radeon/radeon_test.c
@@ -52,7 +52,7 @@ void radeon_test_moves(struct radeon_device *rdev)
 		goto out_cleanup;
 	}
 
-	r = radeon_bo_create(rdev, NULL, size, PAGE_SIZE, true, RADEON_GEM_DOMAIN_VRAM,
+	r = radeon_bo_create(rdev, size, PAGE_SIZE, true, RADEON_GEM_DOMAIN_VRAM,
 				&vram_obj);
 	if (r) {
 		DRM_ERROR("Failed to create VRAM object\n");
@@ -71,7 +71,7 @@ void radeon_test_moves(struct radeon_device *rdev)
 		void **gtt_start, **gtt_end;
 		void **vram_start, **vram_end;
 
-		r = radeon_bo_create(rdev, NULL, size, PAGE_SIZE, true,
+		r = radeon_bo_create(rdev, size, PAGE_SIZE, true,
 					 RADEON_GEM_DOMAIN_GTT, gtt_obj + i);
 		if (r) {
 			DRM_ERROR("Failed to create GTT object %d\n", i);
diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c
index 1272e4b..e17e595 100644
--- a/drivers/gpu/drm/radeon/radeon_ttm.c
+++ b/drivers/gpu/drm/radeon/radeon_ttm.c
@@ -529,7 +529,7 @@ int radeon_ttm_init(struct radeon_device *rdev)
 		DRM_ERROR("Failed initializing VRAM heap.\n");
 		return r;
 	}
-	r = radeon_bo_create(rdev, NULL, 256 * 1024, PAGE_SIZE, true,
+	r = radeon_bo_create(rdev, 256 * 1024, PAGE_SIZE, true,
 				RADEON_GEM_DOMAIN_VRAM,
 				&rdev->stollen_vga_memory);
 	if (r) {
diff --git a/drivers/gpu/drm/radeon/rv770.c b/drivers/gpu/drm/radeon/rv770.c
index 7c2e0b1..2bb49a0 100644
--- a/drivers/gpu/drm/radeon/rv770.c
+++ b/drivers/gpu/drm/radeon/rv770.c
@@ -999,7 +999,7 @@ static int rv770_vram_scratch_init(struct radeon_device *rdev)
 	u64 gpu_addr;
 
 	if (rdev->vram_scratch.robj == NULL) {
-		r = radeon_bo_create(rdev, NULL, RADEON_GPU_PAGE_SIZE,
+		r = radeon_bo_create(rdev, RADEON_GPU_PAGE_SIZE,
 				     PAGE_SIZE, true, RADEON_GEM_DOMAIN_VRAM,
 				     &rdev->vram_scratch.robj);
 		if (r) {
-- 
1.7.1

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2010-11-27 10:04 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-11-13 20:57 [PATCH 0/3] embed drm_gem_object into radeon_bo Daniel Vetter
2010-11-13 20:57 ` [PATCH 1/3] drm/radeon: embed struct drm_gem_object Daniel Vetter
2010-11-13 20:57 ` [PATCH 2/3] drm/radeon: introduce gem_to_radeon_bo helper Daniel Vetter
2010-11-13 20:57 ` [PATCH 3/3] drm/radeon: kill radeon_bo->gobj pointer Daniel Vetter
2010-11-15  7:25 ` [PATCH 0/3] embed drm_gem_object into radeon_bo Thomas Hellstrom
2010-11-15 18:45   ` Daniel Vetter
2010-11-15 20:48     ` Thomas Hellstrom
2010-11-27  9:47 Daniel Vetter
2010-11-27  9:47 ` [PATCH 1/3] drm/radeon: embed struct drm_gem_object Daniel Vetter

This is a public inbox, see mirroring instructions
on how to clone and mirror all data and code used for this inbox