On 27/03/2014 18:00, oscar.ma...@intel.com wrote:
From: Thomas Daniel <thomas.dan...@intel.com>
Handle all context status events in the context status buffer on every
context switch interrupt. We only remove work from the execlist queue
after a context status buffer reports that it has completed and we only
attempt to schedule new contexts on interrupt when a previously submitted
context completes (unless no contexts are queued, which means the GPU is
free).
Signed-off-by: Thomas Daniel <thomas.dan...@intel.com>
v2: Unreferencing the context when we are freeing the request might free
the backing bo, which requires the struct_mutex to be grabbed, so defer
unreferencing and freeing to a bottom half.
Signed-off-by: Oscar Mateo <oscar.ma...@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 3 +
drivers/gpu/drm/i915/i915_irq.c | 28 ++++++---
drivers/gpu/drm/i915/i915_lrc.c | 101 +++++++++++++++++++++++++++++++-
drivers/gpu/drm/i915/intel_ringbuffer.c | 1 +
drivers/gpu/drm/i915/intel_ringbuffer.h | 1 +
5 files changed, 123 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2607664..4c8cf52 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1679,6 +1679,8 @@ struct drm_i915_gem_request {
/** execlist queue entry for this request */
struct list_head execlist_link;
+ /** Struct to handle this request in the bottom half of an interrupt */
+ struct work_struct work;
};
struct drm_i915_file_private {
@@ -2344,6 +2346,7 @@ void gen8_gem_context_free(struct i915_hw_context *ctx);
int gen8_switch_context_queue(struct intel_engine *ring,
struct i915_hw_context *to,
u32 tail);
+void gen8_handle_context_events(struct intel_engine *ring);
/* i915_gem_evict.c */
int __must_check i915_gem_evict_something(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 56657b5..6e0f456 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -1334,6 +1334,7 @@ static irqreturn_t gen8_gt_irq_handler(struct drm_device
*dev,
struct drm_i915_private *dev_priv,
u32 master_ctl)
{
+ struct intel_engine *ring;
u32 rcs, bcs, vcs, vecs;
uint32_t tmp = 0;
irqreturn_t ret = IRQ_NONE;
@@ -1342,14 +1343,21 @@ static irqreturn_t gen8_gt_irq_handler(struct
drm_device *dev,
tmp = I915_READ(GEN8_GT_IIR(0));
if (tmp) {
ret = IRQ_HANDLED;
+
rcs = tmp >> GEN8_RCS_IRQ_SHIFT;
- bcs = tmp >> GEN8_BCS_IRQ_SHIFT;
+ ring = &dev_priv->ring[RCS];
if (rcs & GT_RENDER_USER_INTERRUPT)
- notify_ring(dev, &dev_priv->ring[RCS]);
+ notify_ring(dev, ring);
+ if (rcs & GEN8_GT_CONTEXT_SWITCH_INTERRUPT)
+ gen8_handle_context_events(ring);
Handling the context events here can generate a new execlist submission,
which if a small enough workload, can finish and generate a new context
event interrupt before we ack this interrupt.
When we ack this interrupt, we clear the new one too, loosing an interrupt.
Moving the
I915_WRITE(GEN8_GT_IIR(0), tmp);
to just inside the if (tmp) { conditional (or anywhere before this call)
fixes this issue. There is no harm in acking the interrupt immediately
as we have the read stored in tmp.
+
+ bcs = tmp >> GEN8_BCS_IRQ_SHIFT;
+ ring = &dev_priv->ring[BCS];
if (bcs & GT_RENDER_USER_INTERRUPT)
- notify_ring(dev, &dev_priv->ring[BCS]);
- if ((rcs | bcs) & GEN8_GT_CONTEXT_SWITCH_INTERRUPT)
- DRM_DEBUG_DRIVER("TODO: Context switch\n");
+ notify_ring(dev, ring);
+ if (bcs & GEN8_GT_CONTEXT_SWITCH_INTERRUPT)
+ gen8_handle_context_events(ring);
+
I915_WRITE(GEN8_GT_IIR(0), tmp);
} else
DRM_ERROR("The master control interrupt lied (GT0)!\n");
@@ -1360,10 +1368,11 @@ static irqreturn_t gen8_gt_irq_handler(struct
drm_device *dev,
if (tmp) {
ret = IRQ_HANDLED;
vcs = tmp >> GEN8_VCS1_IRQ_SHIFT;
+ ring = &dev_priv->ring[VCS];
if (vcs & GT_RENDER_USER_INTERRUPT)
- notify_ring(dev, &dev_priv->ring[VCS]);
+ notify_ring(dev, ring);
if (vcs & GEN8_GT_CONTEXT_SWITCH_INTERRUPT)
- DRM_DEBUG_DRIVER("TODO: Context switch\n");
+ gen8_handle_context_events(ring);
I915_WRITE(GEN8_GT_IIR(1), tmp);
} else
DRM_ERROR("The master control interrupt lied (GT1)!\n");
@@ -1374,10 +1383,11 @@ static irqreturn_t gen8_gt_irq_handler(struct
drm_device *dev,
if (tmp) {
ret = IRQ_HANDLED;
vecs = tmp >> GEN8_VECS_IRQ_SHIFT;
+ ring = &dev_priv->ring[VECS];
if (vecs & GT_RENDER_USER_INTERRUPT)
- notify_ring(dev, &dev_priv->ring[VECS]);
+ notify_ring(dev, ring);
if (vecs & GEN8_GT_CONTEXT_SWITCH_INTERRUPT)
- DRM_DEBUG_DRIVER("TODO: Context switch\n");
+ gen8_handle_context_events(ring);
I915_WRITE(GEN8_GT_IIR(3), tmp);
} else
DRM_ERROR("The master control interrupt lied (GT3)!\n");
diff --git a/drivers/gpu/drm/i915/i915_lrc.c b/drivers/gpu/drm/i915/i915_lrc.c
index 4cacabb..440da11 100644
--- a/drivers/gpu/drm/i915/i915_lrc.c
+++ b/drivers/gpu/drm/i915/i915_lrc.c
@@ -46,7 +46,24 @@
#define GEN8_LR_CONTEXT_SIZE (21 * PAGE_SIZE)
#define RING_ELSP(ring) ((ring)->mmio_base+0x230)
+#define RING_EXECLIST_STATUS(ring) ((ring)->mmio_base+0x234)
#define RING_CONTEXT_CONTROL(ring) ((ring)->mmio_base+0x244)
+#define RING_CONTEXT_STATUS_BUF(ring) ((ring)->mmio_base+0x370)
+#define RING_CONTEXT_STATUS_PTR(ring) ((ring)->mmio_base+0x3a0)
+
+#define RING_EXECLIST_QFULL (1 << 0x2)
+#define RING_EXECLIST1_VALID (1 << 0x3)
+#define RING_EXECLIST0_VALID (1 << 0x4)
+#define RING_EXECLIST_ACTIVE_STATUS (3 << 0xE)
+#define RING_EXECLIST1_ACTIVE (1 << 0x11)
+#define RING_EXECLIST0_ACTIVE (1 << 0x12)
+
+#define GEN8_CTX_STATUS_IDLE_ACTIVE (1 << 0)
+#define GEN8_CTX_STATUS_PREEMPTED (1 << 1)
+#define GEN8_CTX_STATUS_ELEMENT_SWITCH (1 << 2)
+#define GEN8_CTX_STATUS_ACTIVE_IDLE (1 << 3)
+#define GEN8_CTX_STATUS_COMPLETE (1 << 4)
+#define GEN8_CTX_STATUS_LITE_RESTORE (1 << 15)
#define CTX_LRI_HEADER_0 0x01
#define CTX_CONTEXT_CONTROL 0x02
@@ -237,6 +254,9 @@ static void gen8_switch_context_unqueue(struct intel_engine
*ring)
{
struct drm_i915_gem_request *req0 = NULL, *req1 = NULL;
struct drm_i915_gem_request *cursor = NULL, *tmp = NULL;
+ struct drm_i915_private *dev_priv = ring->dev->dev_private;
+
+ assert_spin_locked(&ring->execlist_lock);
if (list_empty(&ring->execlist_queue))
return;
@@ -249,8 +269,7 @@ static void gen8_switch_context_unqueue(struct intel_engine
*ring)
/* Same ID: ignore first request, as second request
* will update tail past first request's workload */
list_del(&req0->execlist_link);
- i915_gem_context_unreference(req0->ctx);
- kfree(req0);
+ queue_work(dev_priv->wq, &req0->work);
req0 = cursor;
} else {
req1 = cursor;
@@ -262,6 +281,83 @@ static void gen8_switch_context_unqueue(struct
intel_engine *ring)
req1? req1->ctx : NULL, req1? req1->tail : 0));
}
+static bool check_remove_request(struct intel_engine *ring, u32 request_id)
+{
+ struct drm_i915_private *dev_priv = ring->dev->dev_private;
+ struct drm_i915_gem_request *head_req;
+
+ assert_spin_locked(&ring->execlist_lock);
+
+ head_req = list_first_entry_or_null(&ring->execlist_queue,
+ struct drm_i915_gem_request, execlist_link);
+ if (head_req != NULL) {
+ if (get_submission_id(head_req->ctx) == request_id) {
+ list_del(&head_req->execlist_link);
+ queue_work(dev_priv->wq, &head_req->work);
+ return true;
+ }
+ }
+
+ return false;
+}
+
+void gen8_handle_context_events(struct intel_engine *ring)
+{
+ struct drm_i915_private *dev_priv = ring->dev->dev_private;
+ u32 status_pointer;
+ u8 read_pointer;
+ u8 write_pointer;
+ u32 status;
+ u32 status_id;
+ u32 submit_contexts = 0;
+
+ status_pointer = I915_READ(RING_CONTEXT_STATUS_PTR(ring));
+
+ read_pointer = ring->next_context_status_buffer;
+ write_pointer = status_pointer & 0x07;
+ if (read_pointer > write_pointer)
+ write_pointer += 6;
+
+ spin_lock(&ring->execlist_lock);
+
+ while (read_pointer < write_pointer) {
+ read_pointer++;
+ status = I915_READ(RING_CONTEXT_STATUS_BUF(ring) +
+ (read_pointer % 6) * 8);
+ status_id = I915_READ(RING_CONTEXT_STATUS_BUF(ring) +
+ (read_pointer % 6) * 8 + 4);
+
+ if (status & GEN8_CTX_STATUS_ELEMENT_SWITCH) {
+ if (check_remove_request(ring, status_id))
+ submit_contexts++;
+ } else if (status & GEN8_CTX_STATUS_COMPLETE) {
+ if (check_remove_request(ring, status_id))
+ submit_contexts++;
+ }
+ }
+
+ if (submit_contexts != 0)
+ gen8_switch_context_unqueue(ring);
+
+ spin_unlock(&ring->execlist_lock);
+
+ WARN(submit_contexts > 2, "More than two context complete events?\n");
+ ring->next_context_status_buffer = write_pointer % 6;
+}
+
+static void free_request_task(struct work_struct *work)
+{
+ struct drm_i915_gem_request *req =
+ container_of(work, struct drm_i915_gem_request, work);
+ struct drm_device *dev = req->ring->dev;
+
+ mutex_lock(&dev->struct_mutex);
+ i915_gem_context_unreference(req->ctx);
+ mutex_unlock(&dev->struct_mutex);
+
+ kfree(req);
+}
+
int gen8_switch_context_queue(struct intel_engine *ring,
struct i915_hw_context *to,
u32 tail)
@@ -276,6 +372,7 @@ int gen8_switch_context_queue(struct intel_engine *ring,
req->ctx = to;
i915_gem_context_reference(req->ctx);
req->tail = tail;
+ INIT_WORK(&req->work, free_request_task);
spin_lock_irqsave(&ring->execlist_lock, flags);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c
b/drivers/gpu/drm/i915/intel_ringbuffer.c
index a92bede..ee5a220 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1464,6 +1464,7 @@ static int intel_init_ring(struct drm_device *dev,
if (ring->status_page.page_addr == NULL)
return -ENOMEM;
ring->status_page.obj = obj;
+ ring->next_context_status_buffer = 0;
} else if (I915_NEED_GFX_HWS(dev)) {
ret = init_status_page(ring);
if (ret)
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
b/drivers/gpu/drm/i915/intel_ringbuffer.h
index 5f4fd3c..daca04e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.h
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
@@ -173,6 +173,7 @@ struct intel_engine {
struct i915_hw_context *default_context;
struct i915_hw_context *last_context;
+ u8 next_context_status_buffer;
struct intel_ring_hangcheck hangcheck;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
