* [PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
@ 2015-04-10 9:32 Michel Thierry
2015-04-10 10:01 ` Daniel Vetter
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Michel Thierry @ 2015-04-10 9:32 UTC (permalink / raw)
To: intel-gfx
WaIdleLiteRestore is an execlists-only workaround, and requires the driver
to ensure that any context always has HEAD!=TAIL when attempting lite
restore.
Add two extra MI_NOOP instructions at the end of each request, but keep
the requests tail pointing before the MI_NOOPs.
If we submit a context to the ELSP which has previously been submitted,
move the tail pointer past the MI_NOOPs. This ensures HEAD!=TAIL.
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 11 +++++++++++
drivers/gpu/drm/i915/intel_lrc.c | 29 +++++++++++++++++++++++++++++
2 files changed, 40 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index ca522c9..a44070a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2382,6 +2382,17 @@ int __i915_add_request(struct intel_engine_cs *ring,
request->head = request_start;
request->tail = intel_ring_get_tail(ringbuf);
+ if (i915.enable_execlists &&
+ (IS_GEN8(ring->dev) || IS_GEN9(ring->dev))) {
+ /*
+ * Here we add two extra NOOPs as padding to avoid
+ * lite restore of a context with HEAD==TAIL.
+ */
+ intel_logical_ring_emit(ringbuf, MI_NOOP);
+ intel_logical_ring_emit(ringbuf, MI_NOOP);
+ intel_logical_ring_advance(ringbuf);
+ }
+
/* Whilst this request exists, batch_obj will be on the
* active_list, and so will hold the active reference. Only when this
* request is retired will the the batch_obj be moved onto the
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f18f96c..8acedca 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -413,6 +413,26 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
}
}
+ if (IS_GEN8(ring->dev) || IS_GEN9(ring->dev)) {
+ /*
+ * WaIdleLiteRestore: make sure we never cause a lite
+ * restore with HEAD==TAIL
+ */
+ if (req0 && req0->elsp_submitted == 1) {
+ /*
+ * Consume the buffer NOOPs to ensure HEAD != TAIL when
+ * submitting. elsp_submitted can only be >1 after
+ * reset, in which case we don't need the workaround as
+ * a lite restore will not occur.
+ */
+ struct intel_ringbuffer *ringbuf;
+
+ ringbuf = req0->ctx->engine[ring->id].ringbuf;
+ req0->tail += 8;
+ req0->tail &= ringbuf->size - 1;
+ }
+ }
+
WARN_ON(req1 && req1->elsp_submitted);
execlists_submit_contexts(ring, req0->ctx, req0->tail,
@@ -829,6 +849,15 @@ static int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf,
struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
+ if (IS_GEN8(dev) || IS_GEN9(dev)) {
+ /*
+ * Reserve space for 2 NOOPs at the end of each request to be
+ * used as a workaround for not being allowed to do lite
+ * restore with HEAD==TAIL.
+ */
+ num_dwords += 2;
+ }
+
ret = i915_gem_check_wedge(&dev_priv->gpu_error,
dev_priv->mm.interruptible);
if (ret)
--
2.1.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
2015-04-10 9:32 [PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL Michel Thierry
@ 2015-04-10 10:01 ` Daniel Vetter
2015-04-10 10:11 ` Chris Wilson
2015-04-10 10:14 ` Michel Thierry
2015-04-14 6:08 ` shuang.he
` (3 subsequent siblings)
4 siblings, 2 replies; 16+ messages in thread
From: Daniel Vetter @ 2015-04-10 10:01 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx
On Fri, Apr 10, 2015 at 10:32:33AM +0100, Michel Thierry wrote:
> WaIdleLiteRestore is an execlists-only workaround, and requires the driver
> to ensure that any context always has HEAD!=TAIL when attempting lite
> restore.
>
> Add two extra MI_NOOP instructions at the end of each request, but keep
> the requests tail pointing before the MI_NOOPs.
>
> If we submit a context to the ELSP which has previously been submitted,
> move the tail pointer past the MI_NOOPs. This ensures HEAD!=TAIL.
>
> Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Is there an igt to provoke this? I'm thinking of submitting tiny noop
batches on 2 alternating contexts. That should be able to make this go
boom pretty reliable, we only need a slight delay in processing the
execlist completion interrupt.
-Daniel
> ---
> drivers/gpu/drm/i915/i915_gem.c | 11 +++++++++++
> drivers/gpu/drm/i915/intel_lrc.c | 29 +++++++++++++++++++++++++++++
> 2 files changed, 40 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index ca522c9..a44070a 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2382,6 +2382,17 @@ int __i915_add_request(struct intel_engine_cs *ring,
> request->head = request_start;
> request->tail = intel_ring_get_tail(ringbuf);
>
> + if (i915.enable_execlists &&
> + (IS_GEN8(ring->dev) || IS_GEN9(ring->dev))) {
> + /*
> + * Here we add two extra NOOPs as padding to avoid
> + * lite restore of a context with HEAD==TAIL.
> + */
> + intel_logical_ring_emit(ringbuf, MI_NOOP);
> + intel_logical_ring_emit(ringbuf, MI_NOOP);
> + intel_logical_ring_advance(ringbuf);
> + }
> +
> /* Whilst this request exists, batch_obj will be on the
> * active_list, and so will hold the active reference. Only when this
> * request is retired will the the batch_obj be moved onto the
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index f18f96c..8acedca 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -413,6 +413,26 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
> }
> }
>
> + if (IS_GEN8(ring->dev) || IS_GEN9(ring->dev)) {
> + /*
> + * WaIdleLiteRestore: make sure we never cause a lite
> + * restore with HEAD==TAIL
> + */
> + if (req0 && req0->elsp_submitted == 1) {
> + /*
> + * Consume the buffer NOOPs to ensure HEAD != TAIL when
> + * submitting. elsp_submitted can only be >1 after
> + * reset, in which case we don't need the workaround as
> + * a lite restore will not occur.
> + */
> + struct intel_ringbuffer *ringbuf;
> +
> + ringbuf = req0->ctx->engine[ring->id].ringbuf;
> + req0->tail += 8;
> + req0->tail &= ringbuf->size - 1;
> + }
> + }
> +
> WARN_ON(req1 && req1->elsp_submitted);
>
> execlists_submit_contexts(ring, req0->ctx, req0->tail,
> @@ -829,6 +849,15 @@ static int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf,
> struct drm_i915_private *dev_priv = dev->dev_private;
> int ret;
>
> + if (IS_GEN8(dev) || IS_GEN9(dev)) {
> + /*
> + * Reserve space for 2 NOOPs at the end of each request to be
> + * used as a workaround for not being allowed to do lite
> + * restore with HEAD==TAIL.
> + */
> + num_dwords += 2;
> + }
> +
> ret = i915_gem_check_wedge(&dev_priv->gpu_error,
> dev_priv->mm.interruptible);
> if (ret)
> --
> 2.1.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
2015-04-10 10:01 ` Daniel Vetter
@ 2015-04-10 10:11 ` Chris Wilson
2015-04-10 10:14 ` Michel Thierry
1 sibling, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2015-04-10 10:11 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On Fri, Apr 10, 2015 at 12:01:08PM +0200, Daniel Vetter wrote:
> On Fri, Apr 10, 2015 at 10:32:33AM +0100, Michel Thierry wrote:
> > ---
> > drivers/gpu/drm/i915/i915_gem.c | 11 +++++++++++
> > drivers/gpu/drm/i915/intel_lrc.c | 29 +++++++++++++++++++++++++++++
> > 2 files changed, 40 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> > index ca522c9..a44070a 100644
> > --- a/drivers/gpu/drm/i915/i915_gem.c
> > +++ b/drivers/gpu/drm/i915/i915_gem.c
> > @@ -2382,6 +2382,17 @@ int __i915_add_request(struct intel_engine_cs *ring,
> > request->head = request_start;
> > request->tail = intel_ring_get_tail(ringbuf);
> >
> > + if (i915.enable_execlists &&
> > + (IS_GEN8(ring->dev) || IS_GEN9(ring->dev))) {
> > + /*
> > + * Here we add two extra NOOPs as padding to avoid
> > + * lite restore of a context with HEAD==TAIL.
> > + */
> > + intel_logical_ring_emit(ringbuf, MI_NOOP);
> > + intel_logical_ring_emit(ringbuf, MI_NOOP);
> > + intel_logical_ring_advance(ringbuf);
> > + }
Move this to gen8_emit_request() and remove the permanent overallocation
in ring_begin. The tail pointer adjustment can then also be localised.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
2015-04-10 10:01 ` Daniel Vetter
2015-04-10 10:11 ` Chris Wilson
@ 2015-04-10 10:14 ` Michel Thierry
1 sibling, 0 replies; 16+ messages in thread
From: Michel Thierry @ 2015-04-10 10:14 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
On 4/10/2015 11:01 AM, Daniel Vetter wrote:
> On Fri, Apr 10, 2015 at 10:32:33AM +0100, Michel Thierry wrote:
>> WaIdleLiteRestore is an execlists-only workaround, and requires the driver
>> to ensure that any context always has HEAD!=TAIL when attempting lite
>> restore.
>>
>> Add two extra MI_NOOP instructions at the end of each request, but keep
>> the requests tail pointing before the MI_NOOPs.
>>
>> If we submit a context to the ELSP which has previously been submitted,
>> move the tail pointer past the MI_NOOPs. This ensures HEAD!=TAIL.
>>
>> Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
>> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> Is there an igt to provoke this? I'm thinking of submitting tiny noop
> batches on 2 alternating contexts. That should be able to make this go
> boom pretty reliable, we only need a slight delay in processing the
> execlist completion interrupt.
> -Daniel
I've seen it when running multiple contexts in parallel or when the
wa-batchbuffer was enabled.
I'll write an igt for it, we only need several small batches queued to
trigger the light restore.
--Michel
>> ---
>> drivers/gpu/drm/i915/i915_gem.c | 11 +++++++++++
>> drivers/gpu/drm/i915/intel_lrc.c | 29 +++++++++++++++++++++++++++++
>> 2 files changed, 40 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index ca522c9..a44070a 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -2382,6 +2382,17 @@ int __i915_add_request(struct intel_engine_cs *ring,
>> request->head = request_start;
>> request->tail = intel_ring_get_tail(ringbuf);
>>
>> + if (i915.enable_execlists &&
>> + (IS_GEN8(ring->dev) || IS_GEN9(ring->dev))) {
>> + /*
>> + * Here we add two extra NOOPs as padding to avoid
>> + * lite restore of a context with HEAD==TAIL.
>> + */
>> + intel_logical_ring_emit(ringbuf, MI_NOOP);
>> + intel_logical_ring_emit(ringbuf, MI_NOOP);
>> + intel_logical_ring_advance(ringbuf);
>> + }
>> +
>> /* Whilst this request exists, batch_obj will be on the
>> * active_list, and so will hold the active reference. Only when this
>> * request is retired will the the batch_obj be moved onto the
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> index f18f96c..8acedca 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -413,6 +413,26 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
>> }
>> }
>>
>> + if (IS_GEN8(ring->dev) || IS_GEN9(ring->dev)) {
>> + /*
>> + * WaIdleLiteRestore: make sure we never cause a lite
>> + * restore with HEAD==TAIL
>> + */
>> + if (req0 && req0->elsp_submitted == 1) {
>> + /*
>> + * Consume the buffer NOOPs to ensure HEAD != TAIL when
>> + * submitting. elsp_submitted can only be >1 after
>> + * reset, in which case we don't need the workaround as
>> + * a lite restore will not occur.
>> + */
>> + struct intel_ringbuffer *ringbuf;
>> +
>> + ringbuf = req0->ctx->engine[ring->id].ringbuf;
>> + req0->tail += 8;
>> + req0->tail &= ringbuf->size - 1;
>> + }
>> + }
>> +
>> WARN_ON(req1 && req1->elsp_submitted);
>>
>> execlists_submit_contexts(ring, req0->ctx, req0->tail,
>> @@ -829,6 +849,15 @@ static int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf,
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> int ret;
>>
>> + if (IS_GEN8(dev) || IS_GEN9(dev)) {
>> + /*
>> + * Reserve space for 2 NOOPs at the end of each request to be
>> + * used as a workaround for not being allowed to do lite
>> + * restore with HEAD==TAIL.
>> + */
>> + num_dwords += 2;
>> + }
>> +
>> ret = i915_gem_check_wedge(&dev_priv->gpu_error,
>> dev_priv->mm.interruptible);
>> if (ret)
>> --
>> 2.1.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
2015-04-10 9:32 [PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL Michel Thierry
2015-04-10 10:01 ` Daniel Vetter
@ 2015-04-14 6:08 ` shuang.he
2015-04-14 15:41 ` [PATCH v2] " Michel Thierry
` (2 subsequent siblings)
4 siblings, 0 replies; 16+ messages in thread
From: shuang.he @ 2015-04-14 6:08 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, michel.thierry
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6171
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV -3 270/270 267/270
ILK 303/303 303/303
SNB -21 304/304 283/304
IVB 337/337 337/337
BYT -1 287/287 286/287
HSW 361/361 361/361
BDW 309/309 309/309
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*PNV igt@gem_fence_thrash@bo-write-verify-none PASS(2) FAIL(1)PASS(1)
*PNV igt@gem_fence_thrash@bo-write-verify-threaded-none PASS(3) CRASH(1)PASS(1)
PNV igt@gem_userptr_blits@coherency-sync CRASH(5)PASS(3) CRASH(1)PASS(1)
SNB igt@kms_cursor_crc@cursor-size-change NSPT(3)PASS(1) NSPT(2)
SNB igt@kms_mmio_vs_cs_flip@setcrtc_vs_cs_flip NSPT(3)PASS(1) NSPT(2)
SNB igt@kms_mmio_vs_cs_flip@setplane_vs_cs_flip NSPT(3)PASS(1) NSPT(2)
SNB igt@kms_rotation_crc@primary-rotation NSPT(3)PASS(1) NSPT(2)
SNB igt@kms_rotation_crc@sprite-rotation NSPT(3)PASS(3) NSPT(2)
SNB igt@pm_rpm@cursor NSPT(3)PASS(1) NSPT(2)
SNB igt@pm_rpm@cursor-dpms NSPT(3)PASS(1) NSPT(2)
SNB igt@pm_rpm@dpms-mode-unset-non-lpsp NSPT(3)PASS(1) NSPT(2)
SNB igt@pm_rpm@dpms-non-lpsp NSPT(3)PASS(1) NSPT(2)
SNB igt@pm_rpm@drm-resources-equal NSPT(3)PASS(1) NSPT(2)
SNB igt@pm_rpm@fences NSPT(3)PASS(1) NSPT(2)
SNB igt@pm_rpm@fences-dpms NSPT(3)PASS(1) NSPT(2)
SNB igt@pm_rpm@gem-execbuf NSPT(3)PASS(1) NSPT(2)
SNB igt@pm_rpm@gem-mmap-cpu NSPT(3)PASS(1) NSPT(2)
SNB igt@pm_rpm@gem-mmap-gtt NSPT(3)PASS(1) NSPT(2)
SNB igt@pm_rpm@gem-pread NSPT(3)PASS(1) NSPT(2)
SNB igt@pm_rpm@i2c NSPT(3)PASS(1) NSPT(2)
SNB igt@pm_rpm@modeset-non-lpsp NSPT(3)PASS(1) NSPT(2)
SNB igt@pm_rpm@modeset-non-lpsp-stress-no-wait NSPT(3)PASS(1) NSPT(2)
SNB igt@pm_rpm@pci-d3-state NSPT(3)PASS(1) NSPT(2)
SNB igt@pm_rpm@rte NSPT(3)PASS(1) NSPT(2)
*BYT igt@gem_exec_bad_domains@conflicting-write-domain PASS(3) FAIL(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
2015-04-10 9:32 [PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL Michel Thierry
2015-04-10 10:01 ` Daniel Vetter
2015-04-14 6:08 ` shuang.he
@ 2015-04-14 15:41 ` Michel Thierry
2015-04-15 6:49 ` Chris Wilson
2015-04-15 19:04 ` shuang.he
2015-04-15 16:17 ` [PATCH v3] " Michel Thierry
2015-04-15 17:11 ` [PATCH v4] " Michel Thierry
4 siblings, 2 replies; 16+ messages in thread
From: Michel Thierry @ 2015-04-14 15:41 UTC (permalink / raw)
To: intel-gfx
WaIdleLiteRestore is an execlists-only workaround, and requires the driver
to ensure that any context always has HEAD!=TAIL when attempting lite
restore.
Add two extra MI_NOOP instructions at the end of each request, but keep
the requests tail pointing before the MI_NOOPs. We may not need to
executed them, and this is why request->tail must be sampled before adding
these extra instructions.
If we submit a context to the ELSP which has previously been submitted,
move the tail pointer past the MI_NOOPs. This ensures HEAD!=TAIL.
v2: Move overallocation to gen8_emit_request, and added note about
sampling request->tail in commit message (Chris).
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 15 ++++++++++++++-
drivers/gpu/drm/i915/intel_lrc.c | 27 ++++++++++++++++++++++++++-
2 files changed, 40 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 6dd0d57..dc94984 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2364,14 +2364,27 @@ int __i915_add_request(struct intel_engine_cs *ring,
ret = ring->emit_request(ringbuf, request);
if (ret)
return ret;
+
+ request->tail = intel_ring_get_tail(ringbuf);
+
+ if (IS_GEN8(ring->dev) || IS_GEN9(ring->dev)) {
+ /*
+ * Here we add two extra NOOPs as padding to avoid
+ * lite restore of a context with HEAD==TAIL.
+ */
+ intel_logical_ring_emit(ringbuf, MI_NOOP);
+ intel_logical_ring_emit(ringbuf, MI_NOOP);
+ intel_logical_ring_advance(ringbuf);
+ }
} else {
ret = ring->add_request(ring);
if (ret)
return ret;
+
+ request->tail = intel_ring_get_tail(ringbuf);
}
request->head = request_start;
- request->tail = intel_ring_get_tail(ringbuf);
/* Whilst this request exists, batch_obj will be on the
* active_list, and so will hold the active reference. Only when this
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 2747b02..1614425 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -427,6 +427,26 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
}
}
+ if (IS_GEN8(ring->dev) || IS_GEN9(ring->dev)) {
+ /*
+ * WaIdleLiteRestore: make sure we never cause a lite
+ * restore with HEAD==TAIL
+ */
+ if (req0 && req0->elsp_submitted == 1) {
+ /*
+ * Consume the buffer NOOPs to ensure HEAD != TAIL when
+ * submitting. elsp_submitted can only be >1 after
+ * reset, in which case we don't need the workaround as
+ * a lite restore will not occur.
+ */
+ struct intel_ringbuffer *ringbuf;
+
+ ringbuf = req0->ctx->engine[ring->id].ringbuf;
+ req0->tail += 8;
+ req0->tail &= ringbuf->size - 1;
+ }
+ }
+
WARN_ON(req1 && req1->elsp_submitted);
execlists_submit_contexts(ring, req0->ctx, req0->tail,
@@ -1272,7 +1292,12 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf,
u32 cmd;
int ret;
- ret = intel_logical_ring_begin(ringbuf, request->ctx, 6);
+ /*
+ * Reserve space for 2 NOOPs at the end of each request to be
+ * used as a workaround for not being allowed to do lite
+ * restore with HEAD==TAIL.
+ */
+ ret = intel_logical_ring_begin(ringbuf, request->ctx, 8);
if (ret)
return ret;
--
2.1.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
2015-04-14 15:41 ` [PATCH v2] " Michel Thierry
@ 2015-04-15 6:49 ` Chris Wilson
2015-04-15 19:04 ` shuang.he
1 sibling, 0 replies; 16+ messages in thread
From: Chris Wilson @ 2015-04-15 6:49 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx
On Tue, Apr 14, 2015 at 04:41:24PM +0100, Michel Thierry wrote:
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 6dd0d57..dc94984 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2364,14 +2364,27 @@ int __i915_add_request(struct intel_engine_cs *ring,
> ret = ring->emit_request(ringbuf, request);
> if (ret)
> return ret;
> +
> + request->tail = intel_ring_get_tail(ringbuf);
> +
> + if (IS_GEN8(ring->dev) || IS_GEN9(ring->dev)) {
> + /*
> + * Here we add two extra NOOPs as padding to avoid
> + * lite restore of a context with HEAD==TAIL.
> + */
> + intel_logical_ring_emit(ringbuf, MI_NOOP);
> + intel_logical_ring_emit(ringbuf, MI_NOOP);
> + intel_logical_ring_advance(ringbuf);
> + }
But you are still doing the dance here inside gem, not as a quirk of the
backend. This can be moved to advance_and_submit - needs to since
otherwise the logic is now split between two "independent" units.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
2015-04-10 9:32 [PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL Michel Thierry
` (2 preceding siblings ...)
2015-04-14 15:41 ` [PATCH v2] " Michel Thierry
@ 2015-04-15 16:17 ` Michel Thierry
2015-04-15 16:40 ` Chris Wilson
2015-04-16 9:29 ` shuang.he
2015-04-15 17:11 ` [PATCH v4] " Michel Thierry
4 siblings, 2 replies; 16+ messages in thread
From: Michel Thierry @ 2015-04-15 16:17 UTC (permalink / raw)
To: intel-gfx
WaIdleLiteRestore is an execlists-only workaround, and requires the driver
to ensure that any context always has HEAD!=TAIL when attempting lite
restore.
Add two extra MI_NOOP instructions at the end of each request, but keep
the requests tail pointing before the MI_NOOPs. We may not need to
executed them, and this is why request->tail must be sampled before adding
these extra instructions.
If we submit a context to the ELSP which has previously been submitted,
move the tail pointer past the MI_NOOPs. This ensures HEAD!=TAIL.
v2: Move overallocation to gen8_emit_request, and added note about
sampling request->tail in commit message (Chris).
v3: Remove redundant request->tail assignment in __i915_add_request, in
lrc mode this is already set in execlists_context_queue.
Do not add wa implementation details inside gem (Chris).
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 3 ++-
drivers/gpu/drm/i915/intel_lrc.c | 35 ++++++++++++++++++++++++++++++++++-
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3d5a5a8..980e17c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2400,10 +2400,11 @@ int __i915_add_request(struct intel_engine_cs *ring,
ret = ring->add_request(ring);
if (ret)
return ret;
+
+ request->tail = intel_ring_get_tail(ringbuf);
}
request->head = request_start;
- request->tail = intel_ring_get_tail(ringbuf);
/* Whilst this request exists, batch_obj will be on the
* active_list, and so will hold the active reference. Only when this
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f4a5ef9..0296350 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -427,6 +427,26 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
}
}
+ if (IS_GEN8(ring->dev) || IS_GEN9(ring->dev)) {
+ /*
+ * WaIdleLiteRestore: make sure we never cause a lite
+ * restore with HEAD==TAIL
+ */
+ if (req0 && req0->elsp_submitted == 1) {
+ /*
+ * Consume the buffer NOOPs to ensure HEAD != TAIL when
+ * submitting. elsp_submitted can only be >1 after
+ * reset, in which case we don't need the workaround as
+ * a lite restore will not occur.
+ */
+ struct intel_ringbuffer *ringbuf;
+
+ ringbuf = req0->ctx->engine[ring->id].ringbuf;
+ req0->tail += 8;
+ req0->tail &= ringbuf->size - 1;
+ }
+ }
+
WARN_ON(req1 && req1->elsp_submitted);
execlists_submit_contexts(ring, req0->ctx, req0->tail,
@@ -1289,7 +1309,12 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf,
u32 cmd;
int ret;
- ret = intel_logical_ring_begin(ringbuf, request->ctx, 6);
+ /*
+ * Reserve space for 2 NOOPs at the end of each request to be
+ * used as a workaround for not being allowed to do lite
+ * restore with HEAD==TAIL (WaIdleLiteRestore).
+ */
+ ret = intel_logical_ring_begin(ringbuf, request->ctx, 8);
if (ret)
return ret;
@@ -1307,6 +1332,14 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf,
intel_logical_ring_emit(ringbuf, MI_NOOP);
intel_logical_ring_advance_and_submit(ringbuf, request->ctx, request);
+ /*
+ * Here we add two extra NOOPs as padding to avoid
+ * lite restore of a context with HEAD==TAIL.
+ */
+ intel_logical_ring_emit(ringbuf, MI_NOOP);
+ intel_logical_ring_emit(ringbuf, MI_NOOP);
+ intel_logical_ring_advance(ringbuf);
+
return 0;
}
--
2.1.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v3] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
2015-04-15 16:17 ` [PATCH v3] " Michel Thierry
@ 2015-04-15 16:40 ` Chris Wilson
2015-04-15 17:08 ` Michel Thierry
2015-04-16 9:29 ` shuang.he
1 sibling, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2015-04-15 16:40 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx
On Wed, Apr 15, 2015 at 05:17:13PM +0100, Michel Thierry wrote:
> WaIdleLiteRestore is an execlists-only workaround, and requires the driver
> to ensure that any context always has HEAD!=TAIL when attempting lite
> restore.
>
> Add two extra MI_NOOP instructions at the end of each request, but keep
> the requests tail pointing before the MI_NOOPs. We may not need to
> executed them, and this is why request->tail must be sampled before adding
> these extra instructions.
>
> If we submit a context to the ELSP which has previously been submitted,
> move the tail pointer past the MI_NOOPs. This ensures HEAD!=TAIL.
>
> v2: Move overallocation to gen8_emit_request, and added note about
> sampling request->tail in commit message (Chris).
>
> v3: Remove redundant request->tail assignment in __i915_add_request, in
> lrc mode this is already set in execlists_context_queue.
> Do not add wa implementation details inside gem (Chris).
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 3 ++-
> drivers/gpu/drm/i915/intel_lrc.c | 35 ++++++++++++++++++++++++++++++++++-
> 2 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 3d5a5a8..980e17c 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2400,10 +2400,11 @@ int __i915_add_request(struct intel_engine_cs *ring,
> ret = ring->add_request(ring);
> if (ret)
> return ret;
> +
> + request->tail = intel_ring_get_tail(ringbuf);
> }
>
> request->head = request_start;
> - request->tail = intel_ring_get_tail(ringbuf);
>
> /* Whilst this request exists, batch_obj will be on the
> * active_list, and so will hold the active reference. Only when this
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index f4a5ef9..0296350 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -427,6 +427,26 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
> }
> }
>
> + if (IS_GEN8(ring->dev) || IS_GEN9(ring->dev)) {
> + /*
> + * WaIdleLiteRestore: make sure we never cause a lite
> + * restore with HEAD==TAIL
> + */
> + if (req0 && req0->elsp_submitted == 1) {
> + /*
> + * Consume the buffer NOOPs to ensure HEAD != TAIL when
> + * submitting. elsp_submitted can only be >1 after
> + * reset, in which case we don't need the workaround as
> + * a lite restore will not occur.
I actually think you can remove the == 1 and hence remove comment since
the wa is safe to apply in that case as well.
/* Apply the wa NOOPS to prevent ring:HEAD == rq:TAIL as we
* resubmit the request. See gen8_emit_request() for where we
* prepare the padding after the end of the request.
*/
> + */
> + struct intel_ringbuffer *ringbuf;
> +
> + ringbuf = req0->ctx->engine[ring->id].ringbuf;
> + req0->tail += 8;
> + req0->tail &= ringbuf->size - 1;
> + }
> + }
> +
> WARN_ON(req1 && req1->elsp_submitted);
>
> execlists_submit_contexts(ring, req0->ctx, req0->tail,
> @@ -1289,7 +1309,12 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf,
> u32 cmd;
> int ret;
>
> - ret = intel_logical_ring_begin(ringbuf, request->ctx, 6);
> + /*
> + * Reserve space for 2 NOOPs at the end of each request to be
> + * used as a workaround for not being allowed to do lite
> + * restore with HEAD==TAIL (WaIdleLiteRestore).
> + */
> + ret = intel_logical_ring_begin(ringbuf, request->ctx, 8);
> if (ret)
> return ret;
>
> @@ -1307,6 +1332,14 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf,
> intel_logical_ring_emit(ringbuf, MI_NOOP);
> intel_logical_ring_advance_and_submit(ringbuf, request->ctx, request);
>
> + /*
> + * Here we add two extra NOOPs as padding to avoid
> + * lite restore of a context with HEAD==TAIL.
> + */
> + intel_logical_ring_emit(ringbuf, MI_NOOP);
> + intel_logical_ring_emit(ringbuf, MI_NOOP);
> + intel_logical_ring_advance(ringbuf);
> +
Ok, looks better.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
2015-04-15 16:40 ` Chris Wilson
@ 2015-04-15 17:08 ` Michel Thierry
0 siblings, 0 replies; 16+ messages in thread
From: Michel Thierry @ 2015-04-15 17:08 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
On 4/15/2015 5:40 PM, Chris Wilson wrote:
> On Wed, Apr 15, 2015 at 05:17:13PM +0100, Michel Thierry wrote:
>> WaIdleLiteRestore is an execlists-only workaround, and requires the driver
>> to ensure that any context always has HEAD!=TAIL when attempting lite
>> restore.
>>
>> Add two extra MI_NOOP instructions at the end of each request, but keep
>> the requests tail pointing before the MI_NOOPs. We may not need to
>> executed them, and this is why request->tail must be sampled before adding
>> these extra instructions.
>>
>> If we submit a context to the ELSP which has previously been submitted,
>> move the tail pointer past the MI_NOOPs. This ensures HEAD!=TAIL.
>>
>> v2: Move overallocation to gen8_emit_request, and added note about
>> sampling request->tail in commit message (Chris).
>>
>> v3: Remove redundant request->tail assignment in __i915_add_request, in
>> lrc mode this is already set in execlists_context_queue.
>> Do not add wa implementation details inside gem (Chris).
>>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
>> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_gem.c | 3 ++-
>> drivers/gpu/drm/i915/intel_lrc.c | 35 ++++++++++++++++++++++++++++++++++-
>> 2 files changed, 36 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 3d5a5a8..980e17c 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -2400,10 +2400,11 @@ int __i915_add_request(struct intel_engine_cs *ring,
>> ret = ring->add_request(ring);
>> if (ret)
>> return ret;
>> +
>> + request->tail = intel_ring_get_tail(ringbuf);
>> }
>>
>> request->head = request_start;
>> - request->tail = intel_ring_get_tail(ringbuf);
>>
>> /* Whilst this request exists, batch_obj will be on the
>> * active_list, and so will hold the active reference. Only when this
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> index f4a5ef9..0296350 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -427,6 +427,26 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
>> }
>> }
>>
>> + if (IS_GEN8(ring->dev) || IS_GEN9(ring->dev)) {
>> + /*
>> + * WaIdleLiteRestore: make sure we never cause a lite
>> + * restore with HEAD==TAIL
>> + */
>> + if (req0 && req0->elsp_submitted == 1) {
>> + /*
>> + * Consume the buffer NOOPs to ensure HEAD != TAIL when
>> + * submitting. elsp_submitted can only be >1 after
>> + * reset, in which case we don't need the workaround as
>> + * a lite restore will not occur.
> I actually think you can remove the == 1 and hence remove comment since
> the wa is safe to apply in that case as well.
>
> /* Apply the wa NOOPS to prevent ring:HEAD == rq:TAIL as we
> * resubmit the request. See gen8_emit_request() for where we
> * prepare the padding after the end of the request.
> */
Yes, it's safe to apply it after the request has been submitted multiple
times.
I'll change that and update the comment.
Thanks,
-Michel
>> + */
>> + struct intel_ringbuffer *ringbuf;
>> +
>> + ringbuf = req0->ctx->engine[ring->id].ringbuf;
>> + req0->tail += 8;
>> + req0->tail &= ringbuf->size - 1;
>> + }
>> + }
>> +
>> WARN_ON(req1 && req1->elsp_submitted);
>>
>> execlists_submit_contexts(ring, req0->ctx, req0->tail,
>> @@ -1289,7 +1309,12 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf,
>> u32 cmd;
>> int ret;
>>
>> - ret = intel_logical_ring_begin(ringbuf, request->ctx, 6);
>> + /*
>> + * Reserve space for 2 NOOPs at the end of each request to be
>> + * used as a workaround for not being allowed to do lite
>> + * restore with HEAD==TAIL (WaIdleLiteRestore).
>> + */
>> + ret = intel_logical_ring_begin(ringbuf, request->ctx, 8);
>> if (ret)
>> return ret;
>>
>> @@ -1307,6 +1332,14 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf,
>> intel_logical_ring_emit(ringbuf, MI_NOOP);
>> intel_logical_ring_advance_and_submit(ringbuf, request->ctx, request);
>>
>> + /*
>> + * Here we add two extra NOOPs as padding to avoid
>> + * lite restore of a context with HEAD==TAIL.
>> + */
>> + intel_logical_ring_emit(ringbuf, MI_NOOP);
>> + intel_logical_ring_emit(ringbuf, MI_NOOP);
>> + intel_logical_ring_advance(ringbuf);
>> +
> Ok, looks better.
> -Chris
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v4] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
2015-04-10 9:32 [PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL Michel Thierry
` (3 preceding siblings ...)
2015-04-15 16:17 ` [PATCH v3] " Michel Thierry
@ 2015-04-15 17:11 ` Michel Thierry
2015-04-15 17:19 ` Chris Wilson
2015-04-16 14:07 ` shuang.he
4 siblings, 2 replies; 16+ messages in thread
From: Michel Thierry @ 2015-04-15 17:11 UTC (permalink / raw)
To: intel-gfx
WaIdleLiteRestore is an execlists-only workaround, and requires the driver
to ensure that any context always has HEAD!=TAIL when attempting lite
restore.
Add two extra MI_NOOP instructions at the end of each request, but keep
the requests tail pointing before the MI_NOOPs. We may not need to
executed them, and this is why request->tail is sampled before adding
these extra instructions.
If we submit a context to the ELSP which has previously been submitted,
move the tail pointer past the MI_NOOPs. This ensures HEAD!=TAIL.
v2: Move overallocation to gen8_emit_request, and added note about
sampling request->tail in commit message (Chris).
v3: Remove redundant request->tail assignment in __i915_add_request, in
lrc mode this is already set in execlists_context_queue.
Do not add wa implementation details inside gem (Chris).
v4: Apply the wa whenever the req has been resubmitted and update
comment (Chris).
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
Signed-off-by: Michel Thierry <michel.thierry@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 3 ++-
drivers/gpu/drm/i915/intel_lrc.c | 35 ++++++++++++++++++++++++++++++++++-
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 3d5a5a8..980e17c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2400,10 +2400,11 @@ int __i915_add_request(struct intel_engine_cs *ring,
ret = ring->add_request(ring);
if (ret)
return ret;
+
+ request->tail = intel_ring_get_tail(ringbuf);
}
request->head = request_start;
- request->tail = intel_ring_get_tail(ringbuf);
/* Whilst this request exists, batch_obj will be on the
* active_list, and so will hold the active reference. Only when this
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index f4a5ef9..50ed977 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -427,6 +427,26 @@ static void execlists_context_unqueue(struct intel_engine_cs *ring)
}
}
+ if (IS_GEN8(ring->dev) || IS_GEN9(ring->dev)) {
+ /*
+ * WaIdleLiteRestore: make sure we never cause a lite
+ * restore with HEAD==TAIL
+ */
+ if (req0 && req0->elsp_submitted) {
+ /*
+ * Apply the wa NOOPS to prevent ring:HEAD == req:TAIL
+ * as we resubmit the request. See gen8_emit_request()
+ * for where we prepare the padding after the end of the
+ * request.
+ */
+ struct intel_ringbuffer *ringbuf;
+
+ ringbuf = req0->ctx->engine[ring->id].ringbuf;
+ req0->tail += 8;
+ req0->tail &= ringbuf->size - 1;
+ }
+ }
+
WARN_ON(req1 && req1->elsp_submitted);
execlists_submit_contexts(ring, req0->ctx, req0->tail,
@@ -1289,7 +1309,12 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf,
u32 cmd;
int ret;
- ret = intel_logical_ring_begin(ringbuf, request->ctx, 6);
+ /*
+ * Reserve space for 2 NOOPs at the end of each request to be
+ * used as a workaround for not being allowed to do lite
+ * restore with HEAD==TAIL (WaIdleLiteRestore).
+ */
+ ret = intel_logical_ring_begin(ringbuf, request->ctx, 8);
if (ret)
return ret;
@@ -1307,6 +1332,14 @@ static int gen8_emit_request(struct intel_ringbuffer *ringbuf,
intel_logical_ring_emit(ringbuf, MI_NOOP);
intel_logical_ring_advance_and_submit(ringbuf, request->ctx, request);
+ /*
+ * Here we add two extra NOOPs as padding to avoid
+ * lite restore of a context with HEAD==TAIL.
+ */
+ intel_logical_ring_emit(ringbuf, MI_NOOP);
+ intel_logical_ring_emit(ringbuf, MI_NOOP);
+ intel_logical_ring_advance(ringbuf);
+
return 0;
}
--
2.1.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v4] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
2015-04-15 17:11 ` [PATCH v4] " Michel Thierry
@ 2015-04-15 17:19 ` Chris Wilson
2015-04-23 21:20 ` Jani Nikula
2015-04-16 14:07 ` shuang.he
1 sibling, 1 reply; 16+ messages in thread
From: Chris Wilson @ 2015-04-15 17:19 UTC (permalink / raw)
To: Michel Thierry; +Cc: intel-gfx
On Wed, Apr 15, 2015 at 06:11:33PM +0100, Michel Thierry wrote:
> WaIdleLiteRestore is an execlists-only workaround, and requires the driver
> to ensure that any context always has HEAD!=TAIL when attempting lite
> restore.
>
> Add two extra MI_NOOP instructions at the end of each request, but keep
> the requests tail pointing before the MI_NOOPs. We may not need to
> executed them, and this is why request->tail is sampled before adding
> these extra instructions.
>
> If we submit a context to the ELSP which has previously been submitted,
> move the tail pointer past the MI_NOOPs. This ensures HEAD!=TAIL.
>
> v2: Move overallocation to gen8_emit_request, and added note about
> sampling request->tail in commit message (Chris).
>
> v3: Remove redundant request->tail assignment in __i915_add_request, in
> lrc mode this is already set in execlists_context_queue.
> Do not add wa implementation details inside gem (Chris).
>
> v4: Apply the wa whenever the req has been resubmitted and update
> comment (Chris).
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
2015-04-14 15:41 ` [PATCH v2] " Michel Thierry
2015-04-15 6:49 ` Chris Wilson
@ 2015-04-15 19:04 ` shuang.he
1 sibling, 0 replies; 16+ messages in thread
From: shuang.he @ 2015-04-15 19:04 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, michel.thierry
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6192
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK 301/301 301/301
SNB 316/316 316/316
IVB -1 328/328 327/328
BYT 285/285 285/285
HSW 394/394 394/394
BDW 321/321 321/321
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
IVB igt@gem_pwrite_pread@uncached-copy-performance DMESG_WARN(3)PASS(8) DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:i915_hangcheck_elapsed[i915]]*ERROR*Hangcheck_timer_elapsed...blitter_ring_idle@Hangcheck timer elapsed... blitter ring idle
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v3] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
2015-04-15 16:17 ` [PATCH v3] " Michel Thierry
2015-04-15 16:40 ` Chris Wilson
@ 2015-04-16 9:29 ` shuang.he
1 sibling, 0 replies; 16+ messages in thread
From: shuang.he @ 2015-04-16 9:29 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, michel.thierry
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6205
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK -2 302/302 300/302
SNB 318/318 318/318
IVB 341/341 341/341
BYT 287/287 287/287
HSW 395/395 395/395
BDW 318/318 318/318
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*ILK igt@gem_fenced_exec_thrash@no-spare-fences-busy PASS(3) DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:i915_hangcheck_elapsed[i915]]*ERROR*Hangcheck_timer_elapsed...bsd_ring_idle@Hangcheck timer elapsed... bsd ring idle
ILK igt@gem_unfence_active_buffers DMESG_WARN(1)PASS(1) DMESG_WARN(1)PASS(1)
(dmesg patch applied)drm:i915_hangcheck_elapsed[i915]]*ERROR*Hangcheck_timer_elapsed...bsd_ring_idle@Hangcheck timer elapsed... bsd ring idle
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
2015-04-15 17:11 ` [PATCH v4] " Michel Thierry
2015-04-15 17:19 ` Chris Wilson
@ 2015-04-16 14:07 ` shuang.he
1 sibling, 0 replies; 16+ messages in thread
From: shuang.he @ 2015-04-16 14:07 UTC (permalink / raw)
To: shuang.he, ethan.gao, intel-gfx, michel.thierry
Tested-By: Intel Graphics QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Task id: 6207
-------------------------------------Summary-------------------------------------
Platform Delta drm-intel-nightly Series Applied
PNV 276/276 276/276
ILK 302/302 302/302
SNB 318/318 318/318
IVB 341/341 341/341
BYT 287/287 287/287
HSW -1 395/395 394/395
BDW 318/318 318/318
-------------------------------------Detailed-------------------------------------
Platform Test drm-intel-nightly Series Applied
*HSW igt@pm_rps@blocking PASS(3) FAIL(1)PASS(1)
Note: You need to pay more attention to line start with '*'
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4] drm/i915: Workaround to avoid lite restore with HEAD==TAIL
2015-04-15 17:19 ` Chris Wilson
@ 2015-04-23 21:20 ` Jani Nikula
0 siblings, 0 replies; 16+ messages in thread
From: Jani Nikula @ 2015-04-23 21:20 UTC (permalink / raw)
To: Chris Wilson, Michel Thierry; +Cc: intel-gfx
On Wed, 15 Apr 2015, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Wed, Apr 15, 2015 at 06:11:33PM +0100, Michel Thierry wrote:
>> WaIdleLiteRestore is an execlists-only workaround, and requires the driver
>> to ensure that any context always has HEAD!=TAIL when attempting lite
>> restore.
>>
>> Add two extra MI_NOOP instructions at the end of each request, but keep
>> the requests tail pointing before the MI_NOOPs. We may not need to
>> executed them, and this is why request->tail is sampled before adding
>> these extra instructions.
>>
>> If we submit a context to the ELSP which has previously been submitted,
>> move the tail pointer past the MI_NOOPs. This ensures HEAD!=TAIL.
>>
>> v2: Move overallocation to gen8_emit_request, and added note about
>> sampling request->tail in commit message (Chris).
>>
>> v3: Remove redundant request->tail assignment in __i915_add_request, in
>> lrc mode this is already set in execlists_context_queue.
>> Do not add wa implementation details inside gem (Chris).
>>
>> v4: Apply the wa whenever the req has been resubmitted and update
>> comment (Chris).
>>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Signed-off-by: Thomas Daniel <thomas.daniel@intel.com>
>> Signed-off-by: Michel Thierry <michel.thierry@intel.com>
>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Pushed to drm-intel-next-fixes, thanks for the patch and review.
BR,
Jani.
> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2015-04-23 21:18 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-10 9:32 [PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL Michel Thierry
2015-04-10 10:01 ` Daniel Vetter
2015-04-10 10:11 ` Chris Wilson
2015-04-10 10:14 ` Michel Thierry
2015-04-14 6:08 ` shuang.he
2015-04-14 15:41 ` [PATCH v2] " Michel Thierry
2015-04-15 6:49 ` Chris Wilson
2015-04-15 19:04 ` shuang.he
2015-04-15 16:17 ` [PATCH v3] " Michel Thierry
2015-04-15 16:40 ` Chris Wilson
2015-04-15 17:08 ` Michel Thierry
2015-04-16 9:29 ` shuang.he
2015-04-15 17:11 ` [PATCH v4] " Michel Thierry
2015-04-15 17:19 ` Chris Wilson
2015-04-23 21:20 ` Jani Nikula
2015-04-16 14:07 ` shuang.he
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.