[PATCH 0/3] embed drm_gem_object into radeon_bo

[PATCH 0/3] embed drm_gem_object into radeon_bo

[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(-)

[PATCH 1/3] drm/radeon: embed struct drm_gem_object

[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

[PATCH 2/3] drm/radeon: introduce gem_to_radeon_bo helper

[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

[PATCH 3/3] drm/radeon: kill radeon_bo->gobj pointer

[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

Re: [PATCH 0/3] embed drm_gem_object into radeon_bo

[Thomas Hellstrom]

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
>    

Re: [PATCH 0/3] embed drm_gem_object into radeon_bo

[Daniel Vetter]

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

Re: [PATCH 0/3] embed drm_gem_object into radeon_bo

[Thomas Hellstrom]

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
>    

[PATCH 0/3] embed drm_gem_object into radeon_bo

[Sedat Dilek]

Hi Daniel,

I have tested this upgraded patchset again with linux-next
(next-20101126), they work fine.
Can you next time label the complete series as "v2": [PATCH 0/3 v2]
embed drm_gem_object into radeon_bo (don't ask me if git can create
this automatically for you).

Feel free to add:

      Tested-by: Sedat Dilek <sedat.dilek@gmail.com>

Unfortunately, with
drm-radeon-kms-improve-pflip-precision-on-r1xx-r4xx.patch from [1] in
addition and running Eric Anholt's OpenArena benchmark... it is really
smoother with pflip now but after benchmark I cannot return to
desktop.
That is a problem not of your patchset, Daniel. I already reported on
IRC to Alex.

Hope to see also your embed-gtt-space patchset soonish in drm-next (I
have still the patches from [2] in my kernel patch-series).


Kind Regards,
- Sedat -

[1] https://patchwork.kernel.org/patch/348981/
[2] http://cgit.freedesktop.org/~danvet/drm/log/?h=for-sedat-dilek

$ cd src/linux-2.6/linux-2.6.37-rc3/debian/build/source_i386_none/

$ cat .pc/applied-patches
danvet-embed-drm_gem_object-into-radeon_bo/1-3-drm-radeon-embed-struct-drm_gem_object.patch
danvet-embed-drm_gem_object-into-radeon_bo/2-3-drm-radeon-introduce-gem_to_radeon_bo-helper.patch
danvet-embed-drm_gem_object-into-radeon_bo/3-3-drm-radeon-kill-radeon_bo--gobj-pointer.patch

$ ls -l patches/danvet-embed-drm_gem_object-into-radeon_bo/insgesamt 32
-rw-r--r-- 1 sd sd 12520 28. Nov 00:51
1-3-drm-radeon-embed-struct-drm_gem_object.patch
-rw-r--r-- 1 sd sd 11560 28. Nov 00:53
2-3-drm-radeon-introduce-gem_to_radeon_bo-helper.patch
-rw-r--r-- 1 sd sd  2556 28. Nov 00:53
3-3-drm-radeon-kill-radeon_bo--gobj-pointer.patch

[PATCH 0/3] embed drm_gem_object into radeon_bo

[Daniel Vetter]

Hi Dave,

As promised rebased on top of latest drm-next to resolve a conflict the
pageflipping code.

Tested on my agp rv570.

Please review and consider merging for -next.

Thanks, 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_display.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      |    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 +-
 19 files changed, 63 insertions(+), 70 deletions(-)

Re: [PATCH 0/3] embed drm_gem_object into radeon_bo

[Sedat Dilek]

[-- Attachment #1: Type: text/plain, Size: 2532 bytes --]

On Tue, Nov 16, 2010 at 8:37 PM, Sedat Dilek <sedat.dilek@googlemail.com> wrote:
> On Tue, Nov 16, 2010 at 6:30 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
>> On Tue, Nov 16, 2010 at 06:05:25PM +0100, Sedat Dilek wrote:
>>> I have tested both patchsets from Daniel (see [1] and [2]) again on a
>>> Radeon RV250 in a none-BKL-config and it looks like
>>>
>>>        agd5f-pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support.patch
>>>
>>> is the culprit in combination with the below listed drm patches.
>> Likely a gem_bo->driver_private access. My patches set this to NULL to
>> catch conversion bugs.
>> -Daniel
>> --
>> Daniel Vetter
>> Mail: daniel@ffwll.ch
>> Mobile: +41 (0)79 365 57 48
>>
>
> [ CCing Alex Deucher ]
>
> With the attached diff to the original patch from [1], OpenArena works
> with pageflip-support for radeon-KMS.
> Unfortunately, there is a drop in fps from 18.5 down to 13.5.
>
> - Sedat -
>
> [1] http://people.freedesktop.org/~agd5f/pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support.patch
>
> $ cd ~/src/linux-2.6/linux-2.6.37-rc2/debian/build/source_i386_none/
>
> $ cat .pc/applied-patches
> danvet-drm-for-sedat-dilek/0001-drm-nouveau-don-t-munge-in-drm_mm-internals.patch
> danvet-drm-for-sedat-dilek/0002-drm_mm-add-support-for-range-restricted-fair-lru-sca.patch
> danvet-drm-for-sedat-dilek/0003-drm-mm-track-free-areas-implicitly.patch
> danvet-drm-for-sedat-dilek/0004-drm-mm-extract-node-insert-helper-functions.patch
> danvet-drm-for-sedat-dilek/0005-drm-mm-add-api-for-embedding-struct-drm_mm_node.patch
> danvet-drm-for-sedat-dilek/0006-drm-mm-add-helper-to-unwind-scan-state.patch
> danvet-embed-drm_gem_object-into-radeon_bo/1-3-drm-radeon-embed-struct-drm_gem_object.patch
> danvet-embed-drm_gem_object-into-radeon_bo/2-3-drm-radeon-introduce-gem_to_radeon_bo-helper.patch
> danvet-embed-drm_gem_object-into-radeon_bo/3-3-drm-radeon-kill-radeon_bo--gobj-pointer.patch
> drm-vblank-timestamping/0001-drm-vblank-Add-support-for-precise-vblank-timestampi.patch
> drm-vblank-timestamping/0002-drm-radeon-Add-support-for-precise-vblank-timestampi.patch
> for-drm-radeon-testing/drm-radeon-kms-enable-writeback-on-radeon-AGP-boards.patch
> agd5f-pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support-for-danvet-v2.patch
>
> $ cd ~/src/mesa/
>
> $ ./scripts/run_openarena-benchmark.sh
> 840 frames 62.6 seconds 13.4 fps 16.0/74.5/224.0/19.7 ms
>

Attached are all the patches I used with quilt-series file and kernel-config.

- Sedat -

[-- Attachment #2: for-danvet_patches.tar.xz --]
[-- Type: application/octet-stream, Size: 39512 bytes --]

[-- Attachment #3: for-danvet_patches.tar.xz.sha256sum --]
[-- Type: application/octet-stream, Size: 92 bytes --]

626533723a3828b5d54303216fe81e5e28bb9fa8a5b30dd9cd1fb63d37df04ed  for-danvet_patches.tar.xz

[-- Attachment #4: bkl-config.tar.xz --]
[-- Type: application/octet-stream, Size: 40432 bytes --]

[-- Attachment #5: bkl-config.tar.xz.sha256sum --]
[-- Type: application/octet-stream, Size: 84 bytes --]

e8ccf66b3eda0a33c87bf025491493a7600a3234c7ce0db77a8856faeb818157  bkl-config.tar.xz

[-- Attachment #6: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] embed drm_gem_object into radeon_bo

[Sedat Dilek]

[-- Attachment #1: Type: text/plain, Size: 2280 bytes --]

On Tue, Nov 16, 2010 at 6:30 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Tue, Nov 16, 2010 at 06:05:25PM +0100, Sedat Dilek wrote:
>> I have tested both patchsets from Daniel (see [1] and [2]) again on a
>> Radeon RV250 in a none-BKL-config and it looks like
>>
>>        agd5f-pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support.patch
>>
>> is the culprit in combination with the below listed drm patches.
> Likely a gem_bo->driver_private access. My patches set this to NULL to
> catch conversion bugs.
> -Daniel
> --
> Daniel Vetter
> Mail: daniel@ffwll.ch
> Mobile: +41 (0)79 365 57 48
>

[ CCing Alex Deucher ]

With the attached diff to the original patch from [1], OpenArena works
with pageflip-support for radeon-KMS.
Unfortunately, there is a drop in fps from 18.5 down to 13.5.

- Sedat -

[1] http://people.freedesktop.org/~agd5f/pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support.patch

$ cd ~/src/linux-2.6/linux-2.6.37-rc2/debian/build/source_i386_none/

$ cat .pc/applied-patches
danvet-drm-for-sedat-dilek/0001-drm-nouveau-don-t-munge-in-drm_mm-internals.patch
danvet-drm-for-sedat-dilek/0002-drm_mm-add-support-for-range-restricted-fair-lru-sca.patch
danvet-drm-for-sedat-dilek/0003-drm-mm-track-free-areas-implicitly.patch
danvet-drm-for-sedat-dilek/0004-drm-mm-extract-node-insert-helper-functions.patch
danvet-drm-for-sedat-dilek/0005-drm-mm-add-api-for-embedding-struct-drm_mm_node.patch
danvet-drm-for-sedat-dilek/0006-drm-mm-add-helper-to-unwind-scan-state.patch
danvet-embed-drm_gem_object-into-radeon_bo/1-3-drm-radeon-embed-struct-drm_gem_object.patch
danvet-embed-drm_gem_object-into-radeon_bo/2-3-drm-radeon-introduce-gem_to_radeon_bo-helper.patch
danvet-embed-drm_gem_object-into-radeon_bo/3-3-drm-radeon-kill-radeon_bo--gobj-pointer.patch
drm-vblank-timestamping/0001-drm-vblank-Add-support-for-precise-vblank-timestampi.patch
drm-vblank-timestamping/0002-drm-radeon-Add-support-for-precise-vblank-timestampi.patch
for-drm-radeon-testing/drm-radeon-kms-enable-writeback-on-radeon-AGP-boards.patch
agd5f-pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support-for-danvet-v2.patch

$ cd ~/src/mesa/

$ ./scripts/run_openarena-benchmark.sh
840 frames 62.6 seconds 13.4 fps 16.0/74.5/224.0/19.7 ms

[-- Attachment #2: agd5f-pflip.diff --]
[-- Type: text/x-diff, Size: 841 bytes --]

diff -Naur 0002-drm-radeon-kms-add-pageflip-ioctl-support.patch 0002-drm-radeon-kms-add-pageflip-ioctl-support-for-danvet-v2.patch
--- 0002-drm-radeon-kms-add-pageflip-ioctl-support.patch	2010-10-31 18:53:38.000000000 +0100
+++ 0002-drm-radeon-kms-add-pageflip-ioctl-support-for-danvet-v2.patch	2010-11-16 20:07:12.380393000 +0100
@@ -1319,7 +1319,7 @@
 +	new_radeon_fb = to_radeon_framebuffer(fb);
 +	/* schedule unpin of the old buffer */
 +	obj = old_radeon_fb->obj;
-+	rbo = obj->driver_private;
++	rbo = gem_to_radeon_bo(obj);
 +	work->old_rbo = rbo;
 +	INIT_WORK(&work->work, radeon_unpin_work_func);
 +
@@ -1338,7 +1338,7 @@
 +
 +	/* pin the new buffer */
 +	obj = new_radeon_fb->obj;
-+	rbo = obj->driver_private;
++	rbo = gem_to_radeon_bo(obj);
 +	r = radeon_bo_reserve(rbo, false);
 +	if (unlikely(r != 0))
 +		goto pflip_cleanup;

[-- Attachment #3: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/3] embed drm_gem_object into radeon_bo

[Daniel Vetter]

On Tue, Nov 16, 2010 at 06:05:25PM +0100, Sedat Dilek wrote:
> I have tested both patchsets from Daniel (see [1] and [2]) again on a
> Radeon RV250 in a none-BKL-config and it looks like
> 
>        agd5f-pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support.patch
> 
> is the culprit in combination with the below listed drm patches.
Likely a gem_bo->driver_private access. My patches set this to NULL to
catch conversion bugs.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 0/3] embed drm_gem_object into radeon_bo

[Sedat Dilek]

[ CCing Arnd Bergmann ]

Hi,

I have tested both patchsets from Daniel (see [1] and [2]) again on a
Radeon RV250 in a none-BKL-config and it looks like

       agd5f-pflip/0002-drm-radeon-kms-add-pageflip-ioctl-support.patch

is the culprit in combination with the below listed drm patches.

I have switched to a "normal" DDX (w/o pageflip-support).

The OpenArena benchmarks is now between 17.5 - 18.6 [fps] @1024x768 resolution.
( w/o below patchset: 840 frames 50.6 seconds 16.6 fps 12.0/60.2/267.0/21.3 ms )


Kind Regards,
- Sedat -

[1] http://lists.freedesktop.org/archives/dri-devel/2010-November/005420.html
[2] http://lists.freedesktop.org/archives/dri-devel/2010-November/005441.html


$ cd $HOME/src/linux-2.6/linux-2.6.37-rc2/debian/build/source_i386_none/

$ cat .pc/applied-patches
danvet-drm-for-sedat-dilek/0001-drm-nouveau-don-t-munge-in-drm_mm-internals.patch
danvet-drm-for-sedat-dilek/0002-drm_mm-add-support-for-range-restricted-fair-lru-sca.patch
danvet-drm-for-sedat-dilek/0003-drm-mm-track-free-areas-implicitly.patch
danvet-drm-for-sedat-dilek/0004-drm-mm-extract-node-insert-helper-functions.patch
danvet-drm-for-sedat-dilek/0005-drm-mm-add-api-for-embedding-struct-drm_mm_node.patch
danvet-drm-for-sedat-dilek/0006-drm-mm-add-helper-to-unwind-scan-state.patch
danvet-embed-drm_gem_object-into-radeon_bo/1-3-drm-radeon-embed-struct-drm_gem_object.patch
danvet-embed-drm_gem_object-into-radeon_bo/2-3-drm-radeon-introduce-gem_to_radeon_bo-helper.patch
danvet-embed-drm_gem_object-into-radeon_bo/3-3-drm-radeon-kill-radeon_bo--gobj-pointer.patch
drm-vblank-timestamping/0001-drm-vblank-Add-support-for-precise-vblank-timestampi.patch
drm-vblank-timestamping/0002-drm-radeon-Add-support-for-precise-vblank-timestampi.patch
for-drm-radeon-testing/drm-radeon-kms-enable-writeback-on-radeon-AGP-boards.patch

$ cd $HOME/src/mesa/
$ ./scripts/run_openarena-benchmark.sh
840 frames 46.5 seconds 18.1 fps 10.0/55.3/147.0/18.9 ms

$ grep OK LATEST_linux-2.6/logs/setup_linux-2.6_git0.sd.1.log
  (+) OK   bkl-config/0002-drm-i810-remove-the-BKL.patch
  (+) OK   bkl-config/0003-staging-stradis-mark-as-depends-on-BKL.patch
  (+) OK   bkl-config/0004-BKL-remove-extraneous-include-smp_lock.h.patch
  (+) OK   bkl-config/0005-BKL-remove-references-to-lock_kernel-from-comments.patch
  (+) OK   bkl-config/0006-BKL-disable-by-default.patch
  (+) OK   bkl-config/0007-BKL-mark-lock_kernel-as-deprecated.patch
  (+) OK   bkl-config/0008-BKL-move-CONFIG_BKL-to-staging.patch
  (+) OK   debian/version.patch
  (+) OK   debian/kernelvariables-2.6.37.patch
  (+) OK   debian/doc-build-parallel.patch
  (+) OK   bugfix/ia64/hardcode-arch-script-output.patch
  (+) OK   bugfix/mips/disable-advansys.patch
  (+) OK   bugfix/arm/disable-scsi_acard.patch
  (+) OK   debian/mips-disable-werror.patch
  (+) OK   bugfix/powerpc/lpar-console.patch
  (+) OK   features/all/i915-autoload-without-CONFIG_DRM_I915_KMS.patch
  (+) OK   debian/arch-sh4-fix-uimage-build.patch
  (+) OK   bugfix/mips/mips-ide-flush-dcache.patch
  (+) OK   bugfix/all/qla4xxx-Fix-build-on-some-architectures-lacking-64-bit-I-O.patch
  (+) OK   bugfix/x86/Skip-looking-for-ioapic-overrides-when-ioapics-are-not-present.patch

( NOTE: bkl-config/0001-preempt-fix-kernel-build-with-CONFIG_BKL.patch
is already in Linux 2.6.37-rc2 )

Re: [PATCH 0/3] embed drm_gem_object into radeon_bo

[Sedat Dilek]

Hi Daniel,

I have tried this patchset on a RV250 with linux-next (next-20101115)
in combination w/ patchset from "[PATCH 0/9] make struct drm_mm_node
embeddable" [1].

glxgears works nice.
2nd test-case is Eric Anholt's OpenArena benchmark.
The screen gets blank and system is unusable (cold start, poweroff-button).

Mesa is from master GIT branch:

$ cd ~/src/mesa
$ grep -A4 "git log" mesa.log
+ git log --pretty=short -1
commit 9cf25b3d1cd2910ae33e1faafa04629638bff0fe
Author: Marek Olšák <maraeo@gmail.com>

    r300g: return shader caps from Draw for SWTCL vertex shaders

Daniel requested me to bisect:

bad: 3-3-drm-radeon-kill-radeon_bo--gobj-pointer.patch +
2-3-drm-radeon-introduce-gem_to_radeon_bo-helper.patch
good: 1-3-drm-radeon-embed-struct-drm_gem_object.patch

$cd ~/src/linux-2.6/linux-2.6.37-rc1/debian/build/source_i386_none
$ cat .pc/applied-patches
danvet-embed-drm_gem_object-into-radeon_bo/1-3-drm-radeon-embed-struct-drm_gem_object.patch

Looks like patch #2 is culprit according to Daniel.

Kind Regards,
- Sedat -


[1] http://lists.freedesktop.org/archives/dri-devel/2010-November/005420.html
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel