[PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[Michel Thierry]

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

Re: [PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[Daniel Vetter]

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

Re: [PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[Chris Wilson]

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

Re: [PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[Michel Thierry]

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

Re: [PATCH] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[shuang.he]

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

[PATCH v2] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[Michel Thierry]

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

Re: [PATCH v2] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[Chris Wilson]

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

Re: [PATCH v2] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[shuang.he]

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

[PATCH v3] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[Michel Thierry]

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

Re: [PATCH v3] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[Chris Wilson]

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

Re: [PATCH v3] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[Michel Thierry]

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

Re: [PATCH v3] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[shuang.he]

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

[PATCH v4] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[Michel Thierry]

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

Re: [PATCH v4] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[Chris Wilson]

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

Re: [PATCH v4] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[Jani Nikula]

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

Re: [PATCH v4] drm/i915: Workaround to avoid lite restore with HEAD==TAIL

[shuang.he]

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