[v2] drm/i915/tgl: Disable preemption while being debugged
diff mbox series

Message ID 20190912132313.12751-1-chris@chris-wilson.co.uk
State New
Headers show
Series
  • [v2] drm/i915/tgl: Disable preemption while being debugged
Related show

Commit Message

Chris Wilson Sept. 12, 2019, 1:23 p.m. UTC
We see failures where the context continues executing past a
preemption event, eventually leading to situations where a request has
executed before we have event submitted it to HW! It seems like tgl is
ignoring our RING_TAIL updates, but more likely is that there is a
missing update required for our semaphore waits around preemption.

v2: And disable internal semaphore usage

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 3 +++
 drivers/gpu/drm/i915/i915_pci.c     | 1 +
 2 files changed, 4 insertions(+)

Comments

Mika Kuoppala Sept. 12, 2019, 1:59 p.m. UTC | #1
Chris Wilson <chris@chris-wilson.co.uk> writes:

> We see failures where the context continues executing past a
> preemption event, eventually leading to situations where a request has
> executed before we have event submitted it to HW! It seems like tgl is
> ignoring our RING_TAIL updates, but more likely is that there is a
> missing update required for our semaphore waits around preemption.
>
> v2: And disable internal semaphore usage
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>

Acked-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/gt/intel_lrc.c | 3 +++
>  drivers/gpu/drm/i915/i915_pci.c     | 1 +
>  2 files changed, 4 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 47d766ccea71..a3f0e4999744 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -2939,6 +2939,9 @@ void intel_execlists_set_default_submission(struct intel_engine_cs *engine)
>  			engine->flags |= I915_ENGINE_HAS_PREEMPTION;
>  	}
>  
> +	if (INTEL_GEN(engine->i915) >= 12) /* XXX disabled for debugging */
> +		engine->flags &= ~I915_ENGINE_HAS_SEMAPHORES;
> +
>  	if (engine->class != COPY_ENGINE_CLASS && INTEL_GEN(engine->i915) >= 12)
>  		engine->flags |= I915_ENGINE_HAS_RELATIVE_MMIO;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index b3cc8560696b..2ca34a5cf7d3 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -798,6 +798,7 @@ static const struct intel_device_info intel_tigerlake_12_info = {
>  	.engine_mask =
>  		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
>  	.has_rc6 = false, /* XXX disabled for debugging */
> +	.has_logical_ring_preemption = false, /* XXX disabled for debugging */
>  };
>  
>  #undef GEN
> -- 
> 2.23.0
Daniele Ceraolo Spurio Sept. 12, 2019, 6:45 p.m. UTC | #2
On 9/12/19 6:23 AM, Chris Wilson wrote:
> We see failures where the context continues executing past a
> preemption event, eventually leading to situations where a request has
> executed before we have event submitted it to HW! It seems like tgl is

AFAIK on TGL the CS can detect tail updates in the image of a running 
context even if the lrc hasn't been resubmitted via the elsp. Could that 
be related to the early execution of requests? I haven't looked at the 
failure logs, so forgive me if I'm completely off-mark :)

Daniele

> ignoring our RING_TAIL updates, but more likely is that there is a
> missing update required for our semaphore waits around preemption.
> 
> v2: And disable internal semaphore usage
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_lrc.c | 3 +++
>   drivers/gpu/drm/i915/i915_pci.c     | 1 +
>   2 files changed, 4 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 47d766ccea71..a3f0e4999744 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -2939,6 +2939,9 @@ void intel_execlists_set_default_submission(struct intel_engine_cs *engine)
>   			engine->flags |= I915_ENGINE_HAS_PREEMPTION;
>   	}
>   
> +	if (INTEL_GEN(engine->i915) >= 12) /* XXX disabled for debugging */
> +		engine->flags &= ~I915_ENGINE_HAS_SEMAPHORES;
> +
>   	if (engine->class != COPY_ENGINE_CLASS && INTEL_GEN(engine->i915) >= 12)
>   		engine->flags |= I915_ENGINE_HAS_RELATIVE_MMIO;
>   }
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index b3cc8560696b..2ca34a5cf7d3 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -798,6 +798,7 @@ static const struct intel_device_info intel_tigerlake_12_info = {
>   	.engine_mask =
>   		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
>   	.has_rc6 = false, /* XXX disabled for debugging */
> +	.has_logical_ring_preemption = false, /* XXX disabled for debugging */
>   };
>   
>   #undef GEN
>
Chris Wilson Sept. 12, 2019, 7:43 p.m. UTC | #3
Quoting Daniele Ceraolo Spurio (2019-09-12 19:45:27)
> 
> 
> On 9/12/19 6:23 AM, Chris Wilson wrote:
> > We see failures where the context continues executing past a
> > preemption event, eventually leading to situations where a request has
> > executed before we have event submitted it to HW! It seems like tgl is
> 
> AFAIK on TGL the CS can detect tail updates in the image of a running 
> context even if the lrc hasn't been resubmitted via the elsp. Could that 
> be related to the early execution of requests? I haven't looked at the 
> failure logs, so forgive me if I'm completely off-mark :)

We only update the CTX_RING_TAIL just prior to [re]submitting the
context. But fun and games ensue around preemption where we often have
to rewind the RING_TAIL. Still we should never push it behind
RING_HEAD (so long as our semaphore is solid), so it should never be
able to go past any of the previous RING_TAIL updates we made.

At the moment, I'm just focusing on trimming back the features until we
have a solid platform and then we will be better place to be able to look
at what is going wrong.
-Chris

Patch
diff mbox series

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 47d766ccea71..a3f0e4999744 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -2939,6 +2939,9 @@  void intel_execlists_set_default_submission(struct intel_engine_cs *engine)
 			engine->flags |= I915_ENGINE_HAS_PREEMPTION;
 	}
 
+	if (INTEL_GEN(engine->i915) >= 12) /* XXX disabled for debugging */
+		engine->flags &= ~I915_ENGINE_HAS_SEMAPHORES;
+
 	if (engine->class != COPY_ENGINE_CLASS && INTEL_GEN(engine->i915) >= 12)
 		engine->flags |= I915_ENGINE_HAS_RELATIVE_MMIO;
 }
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index b3cc8560696b..2ca34a5cf7d3 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -798,6 +798,7 @@  static const struct intel_device_info intel_tigerlake_12_info = {
 	.engine_mask =
 		BIT(RCS0) | BIT(BCS0) | BIT(VECS0) | BIT(VCS0) | BIT(VCS2),
 	.has_rc6 = false, /* XXX disabled for debugging */
+	.has_logical_ring_preemption = false, /* XXX disabled for debugging */
 };
 
 #undef GEN