Re: [PATCH 1/3] drm/i915: Restrict PSMI context load w/a to Haswell GT1

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> After we found a workaround for a hang on context load, Ben Widawsky
> found confirmation that it was for an issue with waking from rc6 and
> loading a context image.
>
> The workaround from on high suggests that we should
>
> 	I915_WRITE(RING_WAIT_FOR_RC6_EXIT(engine->mmio_base),
> 		   _MASKED_FIELD(RING_RC6_SEL_WRITE_ADDR_MASK,
> 				 RING_RC6_SEL_WRITE_ADDR_UPPER_LEFT));
>
> in our rc6 setup for Haswell GT1, but on applying that we find instead
> that the machine encounters a GT forcewake error and locks up.
>
> As we are removing HW semaphore usage in the next patch, and the
> suggested workaround is no improvement, we need to
> decouple the PSMI workaround from HAS_SEMAPHORES to IS_HSW_GT1.
>
> References: 2c550183476d ("drm/i915: Disable PSMI sleep messages on all rings around context switches")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         | 2 ++
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 5 +----
>  2 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d44255a8655e..936ec09c9490 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2278,6 +2278,8 @@ intel_info(const struct drm_i915_private *dev_priv)
>  				 (INTEL_DEVID(dev_priv) & 0xFF00) == 0x0A00)
>  #define IS_HSW_GT3(dev_priv)	(IS_HASWELL(dev_priv) && \
>  				 (dev_priv)->info.gt == 3)
> +#define IS_HSW_GT1(dev_priv)	(IS_HASWELL(dev_priv) && \
> +				 (dev_priv)->info.gt == 1)
>  /* ULX machines are also considered ULT. */
>  #define IS_HSW_ULX(dev_priv)	(INTEL_DEVID(dev_priv) == 0x0A0E || \
>  				 INTEL_DEVID(dev_priv) == 0x0A1E)
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 65fd92eb071d..1102c2e98222 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1604,10 +1604,7 @@ static inline int mi_set_context(struct i915_request *rq, u32 flags)
>  	struct intel_engine_cs *engine = rq->engine;
>  	enum intel_engine_id id;
>  	const int num_rings =
> -		/* Use an extended w/a on gen7 if signalling from other rings */
> -		(HAS_LEGACY_SEMAPHORES(i915) && IS_GEN(i915, 7)) ?
> -		INTEL_INFO(i915)->num_rings - 1 :
> -		0;
> +		IS_HSW_GT1(i915) ? INTEL_INFO(i915)->num_rings - 1 : 0;

You are an optimist :) Well, if the gen7 survives, we can shave
some off from context switches.

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

>  	bool force_restore = false;
>  	int len;
>  	u32 *cs;
> -- 
> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx