[PATCH 0/5] some ringbuffer cleanups

[PATCH 0/5] some ringbuffer cleanups

[Ben Widawsky]

This is prep work for some yet to be named stuff I'm trying out. I think
it ended up standing well on its own, so I decided to submit it for
consideration on it's own. Basically it just cleans up the special
allocation of the pipe control, as I intend to do a bit in that area.

Daniel, if you still want to wait for me to unleash the other patches,
that's fine - but maybe you can give some feedback on these meanwhile.

I've only done the most basic smoke tests on this.

Ben Widawsky (5):
  drm/i915: missing error case in init status page
  drm/i915: add cpu_map to i915 object
  drm/i915: refactor ring object allocation
  drm/i915: kill struct pipe_control
  drm/i915: move ring init to intel_ringbuffer.c

 drivers/gpu/drm/i915/i915_drv.h         |   1 +
 drivers/gpu/drm/i915/i915_gem.c         |  39 +------
 drivers/gpu/drm/i915/intel_ringbuffer.c | 182 ++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_ringbuffer.h |   6 +-
 4 files changed, 112 insertions(+), 116 deletions(-)

-- 
1.7.11.1

[PATCH 1/5] drm/i915: missing error case in init status page

[Ben Widawsky]

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index ddc4859..bf0195a 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -972,6 +972,7 @@ static int init_status_page(struct intel_ring_buffer *ring)
 	ring->status_page.gfx_addr = obj->gtt_offset;
 	ring->status_page.page_addr = kmap(obj->pages[0]);
 	if (ring->status_page.page_addr == NULL) {
+		ret = -ENOMEM;
 		goto err_unpin;
 	}
 	ring->status_page.obj = obj;
-- 
1.7.11.1

[PATCH 2/5] drm/i915: add cpu_map to i915 object

[Ben Widawsky]

mostly for convenience, this will help us clear up a bit of the code in
intel_ringbuffer.c

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_drv.h | 1 +
 drivers/gpu/drm/i915/i915_gem.c | 3 +++
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 627fe35..ff0323c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -988,6 +988,7 @@ struct drm_i915_gem_object {
 	 * This is the same as gtt_space->start
 	 */
 	uint32_t gtt_offset;
+	void *cpu_map; /* optional kernel mapping */
 
 	struct intel_ring_buffer *ring;
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 53c3946..c09df96 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3542,6 +3542,9 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
 	if (gem_obj->import_attach)
 		drm_prime_gem_destroy(gem_obj, obj->sg_table);
 
+	if (WARN_ON(obj->cpu_map))
+		kunmap(obj->pages[0]);
+
 	if (obj->phys_obj)
 		i915_gem_detach_phys_object(dev, obj);
 
-- 
1.7.11.1

[PATCH 3/5] drm/i915: refactor ring object allocation

[Ben Widawsky]

There is duplication here. Even more will be coming.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 94 +++++++++++++++++++--------------
 1 file changed, 53 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index bf0195a..33d87ad 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -351,6 +351,43 @@ out:
 	return ret;
 }
 
+static void ring_destroy_object(struct drm_i915_gem_object *obj)
+{
+	kunmap(obj->pages[0]);
+	i915_gem_object_unpin(obj);
+	drm_gem_object_unreference(&obj->base);
+}
+
+static struct drm_i915_gem_object *ring_alloc_object(struct drm_device *dev)
+{
+	struct drm_i915_gem_object *obj;
+	int ret;
+
+	obj = i915_gem_alloc_object(dev, 4096);
+	if (obj == NULL)
+		return ERR_PTR(-ENOMEM);
+
+	i915_gem_object_set_cache_level(obj, I915_CACHE_LLC);
+
+	ret = i915_gem_object_pin(obj, 4096, true);
+	if (ret != 0) {
+		goto err_unref;
+	}
+
+	obj->cpu_map = kmap(obj->pages[0]);
+	if (obj->cpu_map == NULL) {
+		ret = -ENOMEM;
+		goto err_unpin;
+	}
+	return obj;
+
+err_unpin:
+	i915_gem_object_unpin(obj);
+err_unref:
+	drm_gem_object_unreference(&obj->base);
+	return ERR_PTR(ret);
+}
+
 static int
 init_pipe_control(struct intel_ring_buffer *ring)
 {
@@ -365,32 +402,25 @@ init_pipe_control(struct intel_ring_buffer *ring)
 	if (!pc)
 		return -ENOMEM;
 
-	obj = i915_gem_alloc_object(ring->dev, 4096);
-	if (obj == NULL) {
-		DRM_ERROR("Failed to allocate seqno page\n");
-		ret = -ENOMEM;
+	obj = ring_alloc_object(ring->dev);
+	if (IS_ERR(obj)) {
+		DRM_ERROR("Failed to allocate pipe control page\n");
+		ret = PTR_ERR(obj);
 		goto err;
 	}
 
-	i915_gem_object_set_cache_level(obj, I915_CACHE_LLC);
-
-	ret = i915_gem_object_pin(obj, 4096, true);
-	if (ret)
-		goto err_unref;
 
 	pc->gtt_offset = obj->gtt_offset;
-	pc->cpu_page =  kmap(obj->pages[0]);
-	if (pc->cpu_page == NULL)
-		goto err_unpin;
+	pc->cpu_page =  obj->cpu_map;
+	if (pc->cpu_page == NULL) {
+		ret = -ENOMEM;
+		goto err;
+	}
 
 	pc->obj = obj;
 	ring->private = pc;
 	return 0;
 
-err_unpin:
-	i915_gem_object_unpin(obj);
-err_unref:
-	drm_gem_object_unreference(&obj->base);
 err:
 	kfree(pc);
 	return ret;
@@ -406,9 +436,7 @@ cleanup_pipe_control(struct intel_ring_buffer *ring)
 		return;
 
 	obj = pc->obj;
-	kunmap(obj->pages[0]);
-	i915_gem_object_unpin(obj);
-	drm_gem_object_unreference(&obj->base);
+	ring_destroy_object(obj);
 
 	kfree(pc);
 	ring->private = NULL;
@@ -943,9 +971,7 @@ static void cleanup_status_page(struct intel_ring_buffer *ring)
 	if (obj == NULL)
 		return;
 
-	kunmap(obj->pages[0]);
-	i915_gem_object_unpin(obj);
-	drm_gem_object_unreference(&obj->base);
+	ring_destroy_object(obj);
 	ring->status_page.obj = NULL;
 }
 
@@ -955,26 +981,16 @@ static int init_status_page(struct intel_ring_buffer *ring)
 	struct drm_i915_gem_object *obj;
 	int ret;
 
-	obj = i915_gem_alloc_object(dev, 4096);
-	if (obj == NULL) {
+	obj = ring_alloc_object(dev);
+	if (IS_ERR(obj)) {
 		DRM_ERROR("Failed to allocate status page\n");
-		ret = -ENOMEM;
+		ret = PTR_ERR(obj);
 		goto err;
 	}
 
-	i915_gem_object_set_cache_level(obj, I915_CACHE_LLC);
-
-	ret = i915_gem_object_pin(obj, 4096, true);
-	if (ret != 0) {
-		goto err_unref;
-	}
-
 	ring->status_page.gfx_addr = obj->gtt_offset;
-	ring->status_page.page_addr = kmap(obj->pages[0]);
-	if (ring->status_page.page_addr == NULL) {
-		ret = -ENOMEM;
-		goto err_unpin;
-	}
+	ring->status_page.page_addr = obj->cpu_map;
+
 	ring->status_page.obj = obj;
 	memset(ring->status_page.page_addr, 0, PAGE_SIZE);
 
@@ -984,10 +1000,6 @@ static int init_status_page(struct intel_ring_buffer *ring)
 
 	return 0;
 
-err_unpin:
-	i915_gem_object_unpin(obj);
-err_unref:
-	drm_gem_object_unreference(&obj->base);
 err:
 	return ret;
 }
-- 
1.7.11.1

[PATCH 4/5] drm/i915: kill struct pipe_control

[Ben Widawsky]

It's all redundant with the object now. Simply store that.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 58 ++++++++-------------------------
 1 file changed, 13 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 33d87ad..130a5a7 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -34,16 +34,6 @@
 #include "i915_trace.h"
 #include "intel_drv.h"
 
-/*
- * 965+ support PIPE_CONTROL commands, which provide finer grained control
- * over cache flushing.
- */
-struct pipe_control {
-	struct drm_i915_gem_object *obj;
-	volatile u32 *cpu_page;
-	u32 gtt_offset;
-};
-
 static inline int ring_space(struct intel_ring_buffer *ring)
 {
 	int space = (ring->head & HEAD_ADDR) - (ring->tail + 8);
@@ -176,8 +166,8 @@ gen4_render_ring_flush(struct intel_ring_buffer *ring,
 static int
 intel_emit_post_sync_nonzero_flush(struct intel_ring_buffer *ring)
 {
-	struct pipe_control *pc = ring->private;
-	u32 scratch_addr = pc->gtt_offset + 128;
+	struct drm_i915_gem_object *obj = ring->private;
+	u32 scratch_addr = obj->gtt_offset + 128;
 	int ret;
 
 
@@ -214,8 +204,8 @@ gen6_render_ring_flush(struct intel_ring_buffer *ring,
                          u32 invalidate_domains, u32 flush_domains)
 {
 	u32 flags = 0;
-	struct pipe_control *pc = ring->private;
-	u32 scratch_addr = pc->gtt_offset + 128;
+	struct drm_i915_gem_object *obj = ring->private;
+	u32 scratch_addr = obj->gtt_offset + 128;
 	int ret;
 
 	/* Force SNB workarounds for PIPE_CONTROL flushes */
@@ -391,54 +381,32 @@ err_unref:
 static int
 init_pipe_control(struct intel_ring_buffer *ring)
 {
-	struct pipe_control *pc;
 	struct drm_i915_gem_object *obj;
-	int ret;
 
 	if (ring->private)
 		return 0;
 
-	pc = kmalloc(sizeof(*pc), GFP_KERNEL);
-	if (!pc)
-		return -ENOMEM;
-
 	obj = ring_alloc_object(ring->dev);
 	if (IS_ERR(obj)) {
 		DRM_ERROR("Failed to allocate pipe control page\n");
-		ret = PTR_ERR(obj);
-		goto err;
+		return PTR_ERR(obj);
 	}
 
-
-	pc->gtt_offset = obj->gtt_offset;
-	pc->cpu_page =  obj->cpu_map;
-	if (pc->cpu_page == NULL) {
-		ret = -ENOMEM;
-		goto err;
-	}
-
-	pc->obj = obj;
-	ring->private = pc;
+	ring->private = obj;
 	return 0;
-
-err:
-	kfree(pc);
-	return ret;
 }
 
 static void
 cleanup_pipe_control(struct intel_ring_buffer *ring)
 {
-	struct pipe_control *pc = ring->private;
 	struct drm_i915_gem_object *obj;
 
 	if (!ring->private)
 		return;
 
-	obj = pc->obj;
+	obj = ring->private;
 	ring_destroy_object(obj);
 
-	kfree(pc);
 	ring->private = NULL;
 }
 
@@ -599,9 +567,9 @@ static int
 pc_render_add_request(struct intel_ring_buffer *ring,
 		      u32 *result)
 {
+	struct drm_i915_gem_object *obj = ring->private;
 	u32 seqno = i915_gem_next_request_seqno(ring);
-	struct pipe_control *pc = ring->private;
-	u32 scratch_addr = pc->gtt_offset + 128;
+	u32 scratch_addr = obj->gtt_offset + 128;
 	int ret;
 
 	/* For Ironlake, MI_USER_INTERRUPT was deprecated and apparently
@@ -619,7 +587,7 @@ pc_render_add_request(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(4) | PIPE_CONTROL_QW_WRITE |
 			PIPE_CONTROL_WRITE_FLUSH |
 			PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE);
-	intel_ring_emit(ring, pc->gtt_offset | PIPE_CONTROL_GLOBAL_GTT);
+	intel_ring_emit(ring, obj->gtt_offset | PIPE_CONTROL_GLOBAL_GTT);
 	intel_ring_emit(ring, seqno);
 	intel_ring_emit(ring, 0);
 	PIPE_CONTROL_FLUSH(ring, scratch_addr);
@@ -638,7 +606,7 @@ pc_render_add_request(struct intel_ring_buffer *ring,
 			PIPE_CONTROL_WRITE_FLUSH |
 			PIPE_CONTROL_TEXTURE_CACHE_INVALIDATE |
 			PIPE_CONTROL_NOTIFY);
-	intel_ring_emit(ring, pc->gtt_offset | PIPE_CONTROL_GLOBAL_GTT);
+	intel_ring_emit(ring, obj->gtt_offset | PIPE_CONTROL_GLOBAL_GTT);
 	intel_ring_emit(ring, seqno);
 	intel_ring_emit(ring, 0);
 	intel_ring_advance(ring);
@@ -669,8 +637,8 @@ ring_get_seqno(struct intel_ring_buffer *ring)
 static u32
 pc_render_get_seqno(struct intel_ring_buffer *ring)
 {
-	struct pipe_control *pc = ring->private;
-	return pc->cpu_page[0];
+	struct drm_i915_gem_object *obj = ring->private;
+	return ((volatile u32 *)obj->cpu_map)[0];
 }
 
 static bool
-- 
1.7.11.1

[PATCH 5/5] drm/i915: move ring init to intel_ringbuffer.c

[Ben Widawsky]

This is precursor to some work I'm doing, but I think it stands nicely
as its own refactor.

v2: skip teardown returning on success (BUG)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_gem.c         | 36 +---------------------
 drivers/gpu/drm/i915/intel_ringbuffer.c | 53 +++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_ringbuffer.h |  6 ++--
 3 files changed, 53 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index c09df96..fb74172 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -3729,22 +3729,6 @@ void i915_gem_init_ppgtt(struct drm_device *dev)
 	}
 }
 
-static bool
-intel_enable_blt(struct drm_device *dev)
-{
-	if (!HAS_BLT(dev))
-		return false;
-
-	/* The blitter was dysfunctional on early prototypes */
-	if (IS_GEN6(dev) && dev->pdev->revision < 8) {
-		DRM_INFO("BLT not supported on this pre-production hardware;"
-			 " graphics performance will be degraded.\n");
-		return false;
-	}
-
-	return true;
-}
-
 int
 i915_gem_init_hw(struct drm_device *dev)
 {
@@ -3758,22 +3742,10 @@ i915_gem_init_hw(struct drm_device *dev)
 
 	i915_gem_init_swizzling(dev);
 
-	ret = intel_init_render_ring_buffer(dev);
+	ret = intel_init_ring_buffers(dev);
 	if (ret)
 		return ret;
 
-	if (HAS_BSD(dev)) {
-		ret = intel_init_bsd_ring_buffer(dev);
-		if (ret)
-			goto cleanup_render_ring;
-	}
-
-	if (intel_enable_blt(dev)) {
-		ret = intel_init_blt_ring_buffer(dev);
-		if (ret)
-			goto cleanup_bsd_ring;
-	}
-
 	dev_priv->next_seqno = 1;
 
 	/*
@@ -3784,12 +3756,6 @@ i915_gem_init_hw(struct drm_device *dev)
 	i915_gem_init_ppgtt(dev);
 
 	return 0;
-
-cleanup_bsd_ring:
-	intel_cleanup_ring_buffer(&dev_priv->ring[VCS]);
-cleanup_render_ring:
-	intel_cleanup_ring_buffer(&dev_priv->ring[RCS]);
-	return ret;
 }
 
 static bool
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 130a5a7..d9a9d88 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1340,7 +1340,7 @@ static int blt_ring_flush(struct intel_ring_buffer *ring,
 	return 0;
 }
 
-int intel_init_render_ring_buffer(struct drm_device *dev)
+static int intel_init_render_ring_buffer(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct intel_ring_buffer *ring = &dev_priv->ring[RCS];
@@ -1470,7 +1470,7 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size)
 	return 0;
 }
 
-int intel_init_bsd_ring_buffer(struct drm_device *dev)
+static int intel_init_bsd_ring_buffer(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct intel_ring_buffer *ring = &dev_priv->ring[VCS];
@@ -1519,7 +1519,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev)
 	return intel_init_ring_buffer(dev, ring);
 }
 
-int intel_init_blt_ring_buffer(struct drm_device *dev)
+static int intel_init_blt_ring_buffer(struct drm_device *dev)
 {
 	drm_i915_private_t *dev_priv = dev->dev_private;
 	struct intel_ring_buffer *ring = &dev_priv->ring[BCS];
@@ -1546,3 +1546,50 @@ int intel_init_blt_ring_buffer(struct drm_device *dev)
 
 	return intel_init_ring_buffer(dev, ring);
 }
+
+static bool
+intel_enable_blt(struct drm_device *dev)
+{
+	if (!HAS_BLT(dev))
+		return false;
+
+	/* The blitter was dysfunctional on early prototypes */
+	if (IS_GEN6(dev) && dev->pdev->revision < 8) {
+		DRM_INFO("BLT not supported on this pre-production hardware;"
+			 " graphics performance will be degraded.\n");
+		return false;
+	}
+
+	return true;
+}
+
+int intel_init_ring_buffers(struct drm_device *dev)
+{
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret;
+
+	ret = intel_init_render_ring_buffer(dev);
+	if (ret)
+		return ret;
+
+	if (HAS_BSD(dev)) {
+		ret = intel_init_bsd_ring_buffer(dev);
+		if (ret)
+			goto cleanup_render_ring;
+	}
+
+	if (intel_enable_blt(dev)) {
+		ret = intel_init_blt_ring_buffer(dev);
+		if (ret)
+			goto cleanup_bsd_ring;
+	}
+
+	return 0;
+
+cleanup_bsd_ring:
+	intel_cleanup_ring_buffer(&dev_priv->ring[VCS]);
+cleanup_render_ring:
+	intel_cleanup_ring_buffer(&dev_priv->ring[RCS]);
+
+	return ret;
+}
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 1d3c81f..ce0dd43 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -185,6 +185,8 @@ intel_read_status_page(struct intel_ring_buffer *ring,
 #define I915_GEM_HWS_INDEX		0x20
 
 void intel_cleanup_ring_buffer(struct intel_ring_buffer *ring);
+int intel_init_ring_buffers(struct drm_device *dev);
+
 
 int __must_check intel_wait_ring_buffer(struct intel_ring_buffer *ring, int n);
 static inline int intel_wait_ring_idle(struct intel_ring_buffer *ring)
@@ -205,10 +207,6 @@ void intel_ring_advance(struct intel_ring_buffer *ring);
 
 u32 intel_ring_get_seqno(struct intel_ring_buffer *ring);
 
-int intel_init_render_ring_buffer(struct drm_device *dev);
-int intel_init_bsd_ring_buffer(struct drm_device *dev);
-int intel_init_blt_ring_buffer(struct drm_device *dev);
-
 u32 intel_ring_get_active_head(struct intel_ring_buffer *ring);
 void intel_ring_setup_status_page(struct intel_ring_buffer *ring);
 
-- 
1.7.11.1

Re: [PATCH 2/5] drm/i915: add cpu_map to i915 object

[Chris Wilson]

On Thu, 12 Jul 2012 23:16:13 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> mostly for convenience, this will help us clear up a bit of the code in
> intel_ringbuffer.c

I don't think your couple of use-cases is a strong enough argument to
justify an extra pointer on thousands of objects.

If you wanted, you could make the ilk pc w/a use the status page
instead...
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 2/5] drm/i915: add cpu_map to i915 object

[Ben Widawsky]

On Fri, 13 Jul 2012 08:47:15 +0100
Chris Wilson <chris@chris-wilson.co.uk> wrote:

> On Thu, 12 Jul 2012 23:16:13 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > mostly for convenience, this will help us clear up a bit of the code in
> > intel_ringbuffer.c
> 
> I don't think your couple of use-cases is a strong enough argument to
> justify an extra pointer on thousands of objects.
> 
> If you wanted, you could make the ilk pc w/a use the status page
> instead...
> -Chris
> 

Actually, my original code went a step further than this. It combined
all the ring data into 1 object, and then used a mini allocator for the
pipe control, ring status, and the few dwords I need. I was _sure_ you
would hate that, so I went to this.

Can you swallow the one object for everything + allocator idea?

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 2/5] drm/i915: add cpu_map to i915 object

[Chris Wilson]

On Fri, 13 Jul 2012 06:56:44 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> On Fri, 13 Jul 2012 08:47:15 +0100
> Chris Wilson <chris@chris-wilson.co.uk> wrote:
> 
> > On Thu, 12 Jul 2012 23:16:13 -0700, Ben Widawsky <ben@bwidawsk.net> wrote:
> > > mostly for convenience, this will help us clear up a bit of the code in
> > > intel_ringbuffer.c
> > 
> > I don't think your couple of use-cases is a strong enough argument to
> > justify an extra pointer on thousands of objects.
> > 
> > If you wanted, you could make the ilk pc w/a use the status page
> > instead...
> > -Chris
> > 
> 
> Actually, my original code went a step further than this. It combined
> all the ring data into 1 object, and then used a mini allocator for the
> pipe control, ring status, and the few dwords I need. I was _sure_ you
> would hate that, so I went to this.
> 
> Can you swallow the one object for everything + allocator idea?

Yes. I'll happily pay a little one-off extra complexity to reduce the
number of pages we have allocated for a smattering of dwords.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/5] drm/i915: missing error case in init status page

[Daniel Vetter]

On Thu, Jul 12, 2012 at 11:16:12PM -0700, Ben Widawsky wrote:
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
Queued for -next, thanks for the patch.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48