* [PATCH 0/3] a few small cleanups in intel_ringbuffer.c
@ 2011-04-27 20:46 Daniel Vetter
2011-04-27 20:46 ` [PATCH 1/3] drm/i915/ringbuffer: kill snb blt workaround Daniel Vetter
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Daniel Vetter @ 2011-04-27 20:46 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
Hi all,
While mucking around with gfdt to check whether the llc api feels good (it
does!) I've once again noticed a few things in intel_ringbuffer.c
These patches weed them out. Please review and consider merging for -next.
Thanks, Daniel
Daniel Vetter (3):
drm/i915/ringbuffer: kill snb blt workaround
drm/i915/ringbuffer: kill pipe_control->gtt_offset
drm/i915/ringbuffer: embed pipe_control into struct ring_buffer
drivers/gpu/drm/i915/intel_ringbuffer.c | 146 +++++--------------------------
drivers/gpu/drm/i915/intel_ringbuffer.h | 9 ++-
2 files changed, 27 insertions(+), 128 deletions(-)
--
1.7.4.2
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] drm/i915/ringbuffer: kill snb blt workaround
2011-04-27 20:46 [PATCH 0/3] a few small cleanups in intel_ringbuffer.c Daniel Vetter
@ 2011-04-27 20:46 ` Daniel Vetter
2011-04-28 8:42 ` Daniel Vetter
2011-04-27 20:46 ` [PATCH 2/3] drm/i915/ringbuffer: kill pipe_control->gtt_offset Daniel Vetter
2011-04-27 20:46 ` [PATCH 3/3] drm/i915/ringbuffer: embed pipe_control into struct ring_buffer Daniel Vetter
2 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2011-04-27 20:46 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
This was just to facilitate product enablement with pre-production hw.
Allows us to kill quite a bit of cruft.
Cc: Eric Anholt <eric@anholt.net>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 83 +------------------------------
1 files changed, 2 insertions(+), 81 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index f15d80f..afb2a34 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -887,9 +887,6 @@ void intel_cleanup_ring_buffer(struct intel_ring_buffer *ring)
drm_gem_object_unreference(&ring->obj->base);
ring->obj = NULL;
- if (ring->cleanup)
- ring->cleanup(ring);
-
cleanup_status_page(ring);
}
@@ -1153,78 +1150,13 @@ blt_ring_put_irq(struct intel_ring_buffer *ring)
}
-/* Workaround for some stepping of SNB,
- * each time when BLT engine ring tail moved,
- * the first command in the ring to be parsed
- * should be MI_BATCH_BUFFER_START
- */
-#define NEED_BLT_WORKAROUND(dev) \
- (IS_GEN6(dev) && (dev->pdev->revision < 8))
-
-static inline struct drm_i915_gem_object *
-to_blt_workaround(struct intel_ring_buffer *ring)
-{
- return ring->private;
-}
-
-static int blt_ring_init(struct intel_ring_buffer *ring)
-{
- if (NEED_BLT_WORKAROUND(ring->dev)) {
- struct drm_i915_gem_object *obj;
- u32 *ptr;
- int ret;
-
- obj = i915_gem_alloc_object(ring->dev, 4096);
- if (obj == NULL)
- return -ENOMEM;
-
- ret = i915_gem_object_pin(obj, 4096, true);
- if (ret) {
- drm_gem_object_unreference(&obj->base);
- return ret;
- }
-
- ptr = kmap(obj->pages[0]);
- *ptr++ = MI_BATCH_BUFFER_END;
- *ptr++ = MI_NOOP;
- kunmap(obj->pages[0]);
-
- ret = i915_gem_object_set_to_gtt_domain(obj, false);
- if (ret) {
- i915_gem_object_unpin(obj);
- drm_gem_object_unreference(&obj->base);
- return ret;
- }
-
- ring->private = obj;
- }
-
- return init_ring_common(ring);
-}
-
-static int blt_ring_begin(struct intel_ring_buffer *ring,
- int num_dwords)
-{
- if (ring->private) {
- int ret = intel_ring_begin(ring, num_dwords+2);
- if (ret)
- return ret;
-
- intel_ring_emit(ring, MI_BATCH_BUFFER_START);
- intel_ring_emit(ring, to_blt_workaround(ring)->gtt_offset);
-
- return 0;
- } else
- return intel_ring_begin(ring, 4);
-}
-
static int blt_ring_flush(struct intel_ring_buffer *ring,
u32 invalidate, u32 flush)
{
uint32_t cmd;
int ret;
- ret = blt_ring_begin(ring, 4);
+ ret = intel_ring_begin(ring, 4);
if (ret)
return ret;
@@ -1239,22 +1171,12 @@ static int blt_ring_flush(struct intel_ring_buffer *ring,
return 0;
}
-static void blt_ring_cleanup(struct intel_ring_buffer *ring)
-{
- if (!ring->private)
- return;
-
- i915_gem_object_unpin(ring->private);
- drm_gem_object_unreference(ring->private);
- ring->private = NULL;
-}
-
static const struct intel_ring_buffer gen6_blt_ring = {
.name = "blt ring",
.id = RING_BLT,
.mmio_base = BLT_RING_BASE,
.size = 32 * PAGE_SIZE,
- .init = blt_ring_init,
+ .init = init_ring_common,
.write_tail = ring_write_tail,
.flush = blt_ring_flush,
.add_request = gen6_add_request,
@@ -1262,7 +1184,6 @@ static const struct intel_ring_buffer gen6_blt_ring = {
.irq_get = blt_ring_get_irq,
.irq_put = blt_ring_put_irq,
.dispatch_execbuffer = gen6_ring_dispatch_execbuffer,
- .cleanup = blt_ring_cleanup,
};
int intel_init_render_ring_buffer(struct drm_device *dev)
--
1.7.4.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] drm/i915/ringbuffer: kill pipe_control->gtt_offset
2011-04-27 20:46 [PATCH 0/3] a few small cleanups in intel_ringbuffer.c Daniel Vetter
2011-04-27 20:46 ` [PATCH 1/3] drm/i915/ringbuffer: kill snb blt workaround Daniel Vetter
@ 2011-04-27 20:46 ` Daniel Vetter
2011-04-27 20:46 ` [PATCH 3/3] drm/i915/ringbuffer: embed pipe_control into struct ring_buffer Daniel Vetter
2 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2011-04-27 20:46 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
With the snb blt workaround gone, gen5 pipe_control is the only user of
ring->private. It's already tiny, but make it even smaller to prepare for
embedding. The added indirection will be killed in the next patch.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 8 +++-----
1 files changed, 3 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index afb2a34..c73410f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -213,7 +213,6 @@ static int init_ring_common(struct intel_ring_buffer *ring)
struct pipe_control {
struct drm_i915_gem_object *obj;
volatile u32 *cpu_page;
- u32 gtt_offset;
};
static int
@@ -243,7 +242,6 @@ init_pipe_control(struct intel_ring_buffer *ring)
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;
@@ -400,7 +398,7 @@ pc_render_add_request(struct intel_ring_buffer *ring,
struct drm_device *dev = ring->dev;
u32 seqno = i915_gem_get_seqno(dev);
struct pipe_control *pc = ring->private;
- u32 scratch_addr = pc->gtt_offset + 128;
+ u32 scratch_addr = pc->obj->gtt_offset + 128;
int ret;
/* For Ironlake, MI_USER_INTERRUPT was deprecated and apparently
@@ -417,7 +415,7 @@ pc_render_add_request(struct intel_ring_buffer *ring,
intel_ring_emit(ring, GFX_OP_PIPE_CONTROL | PIPE_CONTROL_QW_WRITE |
PIPE_CONTROL_WC_FLUSH | PIPE_CONTROL_TC_FLUSH);
- intel_ring_emit(ring, pc->gtt_offset | PIPE_CONTROL_GLOBAL_GTT);
+ intel_ring_emit(ring, pc->obj->gtt_offset | PIPE_CONTROL_GLOBAL_GTT);
intel_ring_emit(ring, seqno);
intel_ring_emit(ring, 0);
PIPE_CONTROL_FLUSH(ring, scratch_addr);
@@ -434,7 +432,7 @@ pc_render_add_request(struct intel_ring_buffer *ring,
intel_ring_emit(ring, GFX_OP_PIPE_CONTROL | PIPE_CONTROL_QW_WRITE |
PIPE_CONTROL_WC_FLUSH | PIPE_CONTROL_TC_FLUSH |
PIPE_CONTROL_NOTIFY);
- intel_ring_emit(ring, pc->gtt_offset | PIPE_CONTROL_GLOBAL_GTT);
+ intel_ring_emit(ring, pc->obj->gtt_offset | PIPE_CONTROL_GLOBAL_GTT);
intel_ring_emit(ring, seqno);
intel_ring_emit(ring, 0);
intel_ring_advance(ring);
--
1.7.4.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] drm/i915/ringbuffer: embed pipe_control into struct ring_buffer
2011-04-27 20:46 [PATCH 0/3] a few small cleanups in intel_ringbuffer.c Daniel Vetter
2011-04-27 20:46 ` [PATCH 1/3] drm/i915/ringbuffer: kill snb blt workaround Daniel Vetter
2011-04-27 20:46 ` [PATCH 2/3] drm/i915/ringbuffer: kill pipe_control->gtt_offset Daniel Vetter
@ 2011-04-27 20:46 ` Daniel Vetter
2011-04-27 23:24 ` Eric Anholt
2 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2011-04-27 20:46 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
Pipecontrol is also required to implement gfdt flushing on gen6+.
And if we ever switch to pipecontrol based cache management in the
kernel, also required on gen4. I don't see the point in saving that
little bit of storge.
In the process make cleanup_pipe_control more robust and call it
unconditionally.
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
drivers/gpu/drm/i915/intel_ringbuffer.c | 55 +++++++++----------------------
drivers/gpu/drm/i915/intel_ringbuffer.h | 9 +++--
2 files changed, 22 insertions(+), 42 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index c73410f..0160508 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -210,24 +210,17 @@ static int init_ring_common(struct intel_ring_buffer *ring)
* 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;
-};
-
static int
init_pipe_control(struct intel_ring_buffer *ring)
{
- struct pipe_control *pc;
+ struct intel_pipe_control *pc;
struct drm_i915_gem_object *obj;
int ret;
- if (ring->private)
- return 0;
+ pc = &ring->pipe_control;
- pc = kmalloc(sizeof(*pc), GFP_KERNEL);
- if (!pc)
- return -ENOMEM;
+ if (pc->cpu_page)
+ return 0;
obj = i915_gem_alloc_object(ring->dev, 4096);
if (obj == NULL) {
@@ -240,41 +233,34 @@ init_pipe_control(struct intel_ring_buffer *ring)
ret = i915_gem_object_pin(obj, 4096, true);
if (ret)
- goto err_unref;
+ goto err;
pc->cpu_page = kmap(obj->pages[0]);
if (pc->cpu_page == NULL)
goto err_unpin;
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);
+ drm_gem_object_unreference(&obj->base);
return ret;
}
static void
cleanup_pipe_control(struct intel_ring_buffer *ring)
{
- struct pipe_control *pc = ring->private;
+ struct intel_pipe_control *pc = &ring->pipe_control;
struct drm_i915_gem_object *obj;
- if (!ring->private)
- return;
-
- obj = pc->obj;
- kunmap(obj->pages[0]);
- i915_gem_object_unpin(obj);
- drm_gem_object_unreference(&obj->base);
-
- kfree(pc);
- ring->private = NULL;
+ if (pc->cpu_page)
+ kunmap(obj->pages[0]);
+ if (pc->obj) {
+ i915_gem_object_unpin(pc->obj);
+ drm_gem_object_unreference(&pc->obj->base);
+ }
}
static int init_render_ring(struct intel_ring_buffer *ring)
@@ -300,14 +286,6 @@ static int init_render_ring(struct intel_ring_buffer *ring)
return ret;
}
-static void render_ring_cleanup(struct intel_ring_buffer *ring)
-{
- if (!ring->private)
- return;
-
- cleanup_pipe_control(ring);
-}
-
static void
update_semaphore(struct intel_ring_buffer *ring, int i, u32 seqno)
{
@@ -397,7 +375,7 @@ pc_render_add_request(struct intel_ring_buffer *ring,
{
struct drm_device *dev = ring->dev;
u32 seqno = i915_gem_get_seqno(dev);
- struct pipe_control *pc = ring->private;
+ struct intel_pipe_control *pc = &ring->pipe_control;
u32 scratch_addr = pc->obj->gtt_offset + 128;
int ret;
@@ -472,8 +450,7 @@ 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];
+ return ring->pipe_control.cpu_page[0];
}
static void
@@ -886,6 +863,7 @@ void intel_cleanup_ring_buffer(struct intel_ring_buffer *ring)
ring->obj = NULL;
cleanup_status_page(ring);
+ cleanup_pipe_control(ring);
}
static int intel_wrap_ring_buffer(struct intel_ring_buffer *ring)
@@ -999,7 +977,6 @@ static const struct intel_ring_buffer render_ring = {
.irq_get = render_ring_get_irq,
.irq_put = render_ring_put_irq,
.dispatch_execbuffer = render_ring_dispatch_execbuffer,
- .cleanup = render_ring_cleanup,
};
/* ring buffer for bit-stream decoder */
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 16cb125..6cddcfa 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -14,6 +14,11 @@ struct intel_hw_status_page {
struct drm_i915_gem_object *obj;
};
+struct intel_pipe_control {
+ struct drm_i915_gem_object *obj;
+ volatile u32 *cpu_page;
+};
+
#define I915_RING_READ(reg) i915_gt_read(dev_priv, reg)
#define I915_RING_WRITE(reg, val) i915_gt_write(dev_priv, reg, val)
@@ -54,6 +59,7 @@ struct intel_ring_buffer {
int size;
int effective_size;
struct intel_hw_status_page status_page;
+ struct intel_pipe_control pipe_control;
spinlock_t irq_lock;
u32 irq_refcount;
@@ -77,7 +83,6 @@ struct intel_ring_buffer {
u32 (*get_seqno)(struct intel_ring_buffer *ring);
int (*dispatch_execbuffer)(struct intel_ring_buffer *ring,
u32 offset, u32 length);
- void (*cleanup)(struct intel_ring_buffer *ring);
/**
* List of objects currently involved in rendering from the
@@ -113,8 +118,6 @@ struct intel_ring_buffer {
wait_queue_head_t irq_queue;
drm_local_map_t map;
-
- void *private;
};
static inline u32
--
1.7.4.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] drm/i915/ringbuffer: embed pipe_control into struct ring_buffer
2011-04-27 20:46 ` [PATCH 3/3] drm/i915/ringbuffer: embed pipe_control into struct ring_buffer Daniel Vetter
@ 2011-04-27 23:24 ` Eric Anholt
0 siblings, 0 replies; 6+ messages in thread
From: Eric Anholt @ 2011-04-27 23:24 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
[-- Attachment #1.1: Type: text/plain, Size: 674 bytes --]
On Wed, 27 Apr 2011 22:46:06 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Pipecontrol is also required to implement gfdt flushing on gen6+.
> And if we ever switch to pipecontrol based cache management in the
> kernel, also required on gen4. I don't see the point in saving that
> little bit of storge.
>
> In the process make cleanup_pipe_control more robust and call it
> unconditionally.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Nice. I thought about this when reviewing the first, then decided I
didn't care enough. But if you've already cleaned it up, great!
these two are:
Reviewed-by: Eric Anholt <eric@anholt.net>
[-- Attachment #1.2: Type: application/pgp-signature, Size: 197 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 1/3] drm/i915/ringbuffer: kill snb blt workaround
2011-04-27 20:46 ` [PATCH 1/3] drm/i915/ringbuffer: kill snb blt workaround Daniel Vetter
@ 2011-04-28 8:42 ` Daniel Vetter
0 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2011-04-28 8:42 UTC (permalink / raw)
To: intel-gfx; +Cc: Daniel Vetter
Bleh, I've fat-fingered the fdo address on the first submission, to
which Eric replied.
Pasting his response below:
On Wed, 27 Apr 2011 22:33:35 +0200, Daniel Vetter
<daniel.vetter@ffwll.ch> wrote:
> This was just to facilitate product enablement with pre-production hw.
> Allows us to kill quite a bit of cruft.
>
> Cc: Eric Anholt <eric@anholt.net>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Eric Anholt <eric@anholt.net>
--
Daniel Vetter
daniel.vetter@ffwll.ch - +41 (0) 79 364 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2011-04-28 8:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-27 20:46 [PATCH 0/3] a few small cleanups in intel_ringbuffer.c Daniel Vetter
2011-04-27 20:46 ` [PATCH 1/3] drm/i915/ringbuffer: kill snb blt workaround Daniel Vetter
2011-04-28 8:42 ` Daniel Vetter
2011-04-27 20:46 ` [PATCH 2/3] drm/i915/ringbuffer: kill pipe_control->gtt_offset Daniel Vetter
2011-04-27 20:46 ` [PATCH 3/3] drm/i915/ringbuffer: embed pipe_control into struct ring_buffer Daniel Vetter
2011-04-27 23:24 ` Eric Anholt
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).