[5/8] drm/i915/icl: Add reset control register changes

Commit Message

Mika Kuoppala March 16, 2018, 12:14 p.m. UTC

From: Michel Thierry <[e-mail]>

The bits used to reset the different engines/domains have changed in
GEN11, this patch maps the reset engine mask bits with the new bits
in the reset control register.

v2: Use shift-left instead of BIT macro to match the file style (Paulo).
v3: Reuse gen8_reset_engines (Daniele).
v4: Do not call intel_uncore_forcewake_reset after reset, we may be
using the forcewake to read protected registers elsewhere and those
results may be clobbered by the concurrent dropping of forcewake.

bspec: 19212
Cc: Oscar Mateo <[e-mail]>
Cc: Antonio Argenziano <[e-mail]>
Cc: Paulo Zanoni <[e-mail]>
Cc: Daniele Ceraolo Spurio <[e-mail]>
Acked-by: Daniele Ceraolo Spurio <[e-mail]>
Signed-off-by: Michel Thierry <[e-mail]>
---
 drivers/gpu/drm/i915/i915_reg.h     | 11 ++++++++
 drivers/gpu/drm/i915/intel_uncore.c | 53 +++++++++++++++++++++++++++++++++++--
 2 files changed, 62 insertions(+), 2 deletions(-)

Comments

Mika Kuoppala March 16, 2018, 12:22 p.m. UTC | #1

Mika Kuoppala <[e-mail]> writes:

> From: Michel Thierry <[e-mail]>
>
> The bits used to reset the different engines/domains have changed in
> GEN11, this patch maps the reset engine mask bits with the new bits
> in the reset control register.
>
> v2: Use shift-left instead of BIT macro to match the file style (Paulo).
> v3: Reuse gen8_reset_engines (Daniele).
> v4: Do not call intel_uncore_forcewake_reset after reset, we may be
> using the forcewake to read protected registers elsewhere and those
> results may be clobbered by the concurrent dropping of forcewake.
>
> bspec: 19212
> Cc: Oscar Mateo <[e-mail]>
> Cc: Antonio Argenziano <[e-mail]>
> Cc: Paulo Zanoni <[e-mail]>
> Cc: Daniele Ceraolo Spurio <[e-mail]>
> Acked-by: Daniele Ceraolo Spurio <[e-mail]>
> Signed-off-by: Michel Thierry <[e-mail]>
> ---
>  drivers/gpu/drm/i915/i915_reg.h     | 11 ++++++++
>  drivers/gpu/drm/i915/intel_uncore.c | 53 +++++++++++++++++++++++++++++++++++--
>  2 files changed, 62 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9eaaa96287ec..f3cc77690124 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -301,6 +301,17 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define  GEN6_GRDOM_VECS		(1 << 4)
>  #define  GEN9_GRDOM_GUC			(1 << 5)
>  #define  GEN8_GRDOM_MEDIA2		(1 << 7)
> +/* GEN11 changed all bit defs except for FULL & RENDER */
> +#define  GEN11_GRDOM_FULL		GEN6_GRDOM_FULL
> +#define  GEN11_GRDOM_RENDER		GEN6_GRDOM_RENDER
> +#define  GEN11_GRDOM_BLT		(1 << 2)
> +#define  GEN11_GRDOM_GUC		(1 << 3)
> +#define  GEN11_GRDOM_MEDIA		(1 << 5)
> +#define  GEN11_GRDOM_MEDIA2		(1 << 6)
> +#define  GEN11_GRDOM_MEDIA3		(1 << 7)
> +#define  GEN11_GRDOM_MEDIA4		(1 << 8)

I would like these to be named like they are in bspec.
First being MEDIA0.

> +#define  GEN11_GRDOM_VECS		(1 << 13)
> +#define  GEN11_GRDOM_VECS2		(1 << 14)

And same to these, VECS0 and VECS1.

-Mika

>
>  #define RING_PP_DIR_BASE(engine)	_MMIO((engine)->mmio_base+0x228)
>  #define RING_PP_DIR_BASE_READ(engine)	_MMIO((engine)->mmio_base+0x518)
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 4c616d074a97..cabbf0e682e7 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1909,6 +1909,50 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv,
>  	return gen6_hw_domain_reset(dev_priv, hw_mask);
>  }
>  
> +/**
> + * gen11_reset_engines - reset individual engines
> + * @dev_priv: i915 device
> + * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for full reset
> + *
> + * This function will reset the individual engines that are set in engine_mask.
> + * If you provide ALL_ENGINES as mask, full global domain reset will be issued.
> + *
> + * Note: It is responsibility of the caller to handle the difference between
> + * asking full domain reset versus reset for all available individual engines.
> + *
> + * Returns 0 on success, nonzero on error.
> + */
> +static int gen11_reset_engines(struct drm_i915_private *dev_priv,
> +			       unsigned engine_mask)
> +{
> +	struct intel_engine_cs *engine;
> +	const u32 hw_engine_mask[I915_NUM_ENGINES] = {
> +		[RCS] = GEN11_GRDOM_RENDER,
> +		[BCS] = GEN11_GRDOM_BLT,
> +		[VCS] = GEN11_GRDOM_MEDIA,
> +		[VCS2] = GEN11_GRDOM_MEDIA2,
> +		[VCS3] = GEN11_GRDOM_MEDIA3,
> +		[VCS4] = GEN11_GRDOM_MEDIA4,
> +		[VECS] = GEN11_GRDOM_VECS,
> +		[VECS2] = GEN11_GRDOM_VECS2,
> +	};
> +	u32 hw_mask;
> +
> +	BUILD_BUG_ON(VECS2 + 1 != I915_NUM_ENGINES);
> +
> +	if (engine_mask == ALL_ENGINES) {
> +		hw_mask = GEN11_GRDOM_FULL;
> +	} else {
> +		unsigned int tmp;
> +
> +		hw_mask = 0;
> +		for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
> +			hw_mask |= hw_engine_mask[engine->id];
> +	}
> +
> +	return gen6_hw_domain_reset(dev_priv, hw_mask);
> +}
> +
>  /**
>   * __intel_wait_for_register_fw - wait until register matches expected state
>   * @dev_priv: the i915 device
> @@ -2056,7 +2100,10 @@ static int gen8_reset_engines(struct drm_i915_private *dev_priv,
>  		if (gen8_reset_engine_start(engine))
>  			goto not_ready;
>  
> -	return gen6_reset_engines(dev_priv, engine_mask);
> +	if (INTEL_GEN(dev_priv) >= 11)
> +		return gen11_reset_engines(dev_priv, engine_mask);
> +	else
> +		return gen6_reset_engines(dev_priv, engine_mask);
>  
>  not_ready:
>  	for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
> @@ -2141,12 +2188,14 @@ bool intel_has_reset_engine(struct drm_i915_private *dev_priv)
>  
>  int intel_reset_guc(struct drm_i915_private *dev_priv)
>  {
> +	u32 guc_domain = INTEL_GEN(dev_priv) >= 11 ? GEN11_GRDOM_GUC :
> +						     GEN9_GRDOM_GUC;
>  	int ret;
>  
>  	GEM_BUG_ON(!HAS_GUC(dev_priv));
>  
>  	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> -	ret = gen6_hw_domain_reset(dev_priv, GEN9_GRDOM_GUC);
> +	ret = gen6_hw_domain_reset(dev_priv, guc_domain);
>  	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>  
>  	return ret;
> -- 
> 2.14.1
>
> _______________________________________________
> Intel-gfx mailing list
> [e-mail]
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Chris Wilson March 16, 2018, 12:45 p.m. UTC | #2

Quoting Mika Kuoppala (2018-03-16 12:14:53)
> +static int gen11_reset_engines(struct drm_i915_private *dev_priv,
> +                              unsigned engine_mask)
> +{
> +       struct intel_engine_cs *engine;
> +       const u32 hw_engine_mask[I915_NUM_ENGINES] = {
> +               [RCS] = GEN11_GRDOM_RENDER,
> +               [BCS] = GEN11_GRDOM_BLT,
> +               [VCS] = GEN11_GRDOM_MEDIA,
> +               [VCS2] = GEN11_GRDOM_MEDIA2,
> +               [VCS3] = GEN11_GRDOM_MEDIA3,
> +               [VCS4] = GEN11_GRDOM_MEDIA4,
> +               [VECS] = GEN11_GRDOM_VECS,
> +               [VECS2] = GEN11_GRDOM_VECS2,
> +       };

No gratuitously decorating Christmas trees.

> +       u32 hw_mask;
> +
> +       BUILD_BUG_ON(VECS2 + 1 != I915_NUM_ENGINES);
> +
> +       if (engine_mask == ALL_ENGINES) {
> +               hw_mask = GEN11_GRDOM_FULL;
> +       } else {

Plonk struct intel_engine_cs *engine; here instead.

> +               unsigned int tmp;
> +
> +               hw_mask = 0;
> +               for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
> +                       hw_mask |= hw_engine_mask[engine->id];
> +       }

Daniele Ceraolo Spurio March 16, 2018, 8:28 p.m. UTC | #3

On 16/03/18 05:14, Mika Kuoppala wrote:
> From: Michel Thierry <[e-mail]>
> 
> The bits used to reset the different engines/domains have changed in
> GEN11, this patch maps the reset engine mask bits with the new bits
> in the reset control register.
> 
> v2: Use shift-left instead of BIT macro to match the file style (Paulo).
> v3: Reuse gen8_reset_engines (Daniele).
> v4: Do not call intel_uncore_forcewake_reset after reset, we may be
> using the forcewake to read protected registers elsewhere and those
> results may be clobbered by the concurrent dropping of forcewake.
> 
> bspec: 19212
> Cc: Oscar Mateo <[e-mail]>
> Cc: Antonio Argenziano <[e-mail]>
> Cc: Paulo Zanoni <[e-mail]>
> Cc: Daniele Ceraolo Spurio <[e-mail]>
> Acked-by: Daniele Ceraolo Spurio <[e-mail]>
> Signed-off-by: Michel Thierry <[e-mail]>
> ---
>   drivers/gpu/drm/i915/i915_reg.h     | 11 ++++++++
>   drivers/gpu/drm/i915/intel_uncore.c | 53 +++++++++++++++++++++++++++++++++++--
>   2 files changed, 62 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9eaaa96287ec..f3cc77690124 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -301,6 +301,17 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>   #define  GEN6_GRDOM_VECS		(1 << 4)
>   #define  GEN9_GRDOM_GUC			(1 << 5)
>   #define  GEN8_GRDOM_MEDIA2		(1 << 7)
> +/* GEN11 changed all bit defs except for FULL & RENDER */
> +#define  GEN11_GRDOM_FULL		GEN6_GRDOM_FULL
> +#define  GEN11_GRDOM_RENDER		GEN6_GRDOM_RENDER
> +#define  GEN11_GRDOM_BLT		(1 << 2)
> +#define  GEN11_GRDOM_GUC		(1 << 3)
> +#define  GEN11_GRDOM_MEDIA		(1 << 5)
> +#define  GEN11_GRDOM_MEDIA2		(1 << 6)
> +#define  GEN11_GRDOM_MEDIA3		(1 << 7)
> +#define  GEN11_GRDOM_MEDIA4		(1 << 8)
> +#define  GEN11_GRDOM_VECS		(1 << 13)
> +#define  GEN11_GRDOM_VECS2		(1 << 14)
>   
>   #define RING_PP_DIR_BASE(engine)	_MMIO((engine)->mmio_base+0x228)
>   #define RING_PP_DIR_BASE_READ(engine)	_MMIO((engine)->mmio_base+0x518)
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index 4c616d074a97..cabbf0e682e7 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1909,6 +1909,50 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv,
>   	return gen6_hw_domain_reset(dev_priv, hw_mask);
>   }
>   
> +/**
> + * gen11_reset_engines - reset individual engines
> + * @dev_priv: i915 device
> + * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for full reset
> + *
> + * This function will reset the individual engines that are set in engine_mask.
> + * If you provide ALL_ENGINES as mask, full global domain reset will be issued.
> + *
> + * Note: It is responsibility of the caller to handle the difference between
> + * asking full domain reset versus reset for all available individual engines.
> + *
> + * Returns 0 on success, nonzero on error.
> + */
> +static int gen11_reset_engines(struct drm_i915_private *dev_priv,
> +			       unsigned engine_mask)
> +{
> +	struct intel_engine_cs *engine;
> +	const u32 hw_engine_mask[I915_NUM_ENGINES] = {
> +		[RCS] = GEN11_GRDOM_RENDER,
> +		[BCS] = GEN11_GRDOM_BLT,
> +		[VCS] = GEN11_GRDOM_MEDIA,
> +		[VCS2] = GEN11_GRDOM_MEDIA2,
> +		[VCS3] = GEN11_GRDOM_MEDIA3,
> +		[VCS4] = GEN11_GRDOM_MEDIA4,
> +		[VECS] = GEN11_GRDOM_VECS,
> +		[VECS2] = GEN11_GRDOM_VECS2,
> +	};

Just a thought, but since this function is a copy of gen6_reset_engines 
with the only difference being the array (GEN11_GRDOM_FULL is also the 
same as GEN6_GRDOM_FULL), would it make sense to just add the array to 
the gen6 function? e.g.:

	const u32 gen6_hw_engine_mask[] = {
	....
	}
	const u32 gen11_hw_engine_mask[] = {
	....
	}

	const u32 *hw_engine_mask = INTEL_GEN(dev_priv) >= 11 ?
		gen11_hw_engine_mask : gen6_hw_engine_mask;


My Ack still stands regardless and I also agree with renaming the 
defines to be closer to the specs.

Daniele

> +	u32 hw_mask;
> +
> +	BUILD_BUG_ON(VECS2 + 1 != I915_NUM_ENGINES);
> +
> +	if (engine_mask == ALL_ENGINES) {
> +		hw_mask = GEN11_GRDOM_FULL;
> +	} else {
> +		unsigned int tmp;
> +
> +		hw_mask = 0;
> +		for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
> +			hw_mask |= hw_engine_mask[engine->id];
> +	}
> +
> +	return gen6_hw_domain_reset(dev_priv, hw_mask);
> +}
> +
>   /**
>    * __intel_wait_for_register_fw - wait until register matches expected state
>    * @dev_priv: the i915 device
> @@ -2056,7 +2100,10 @@ static int gen8_reset_engines(struct drm_i915_private *dev_priv,
>   		if (gen8_reset_engine_start(engine))
>   			goto not_ready;
>   
> -	return gen6_reset_engines(dev_priv, engine_mask);
> +	if (INTEL_GEN(dev_priv) >= 11)
> +		return gen11_reset_engines(dev_priv, engine_mask);
> +	else
> +		return gen6_reset_engines(dev_priv, engine_mask);
>   
>   not_ready:
>   	for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
> @@ -2141,12 +2188,14 @@ bool intel_has_reset_engine(struct drm_i915_private *dev_priv)
>   
>   int intel_reset_guc(struct drm_i915_private *dev_priv)
>   {
> +	u32 guc_domain = INTEL_GEN(dev_priv) >= 11 ? GEN11_GRDOM_GUC :
> +						     GEN9_GRDOM_GUC;
>   	int ret;
>   
>   	GEM_BUG_ON(!HAS_GUC(dev_priv));
>   
>   	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
> -	ret = gen6_hw_domain_reset(dev_priv, GEN9_GRDOM_GUC);
> +	ret = gen6_hw_domain_reset(dev_priv, guc_domain);
>   	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>   
>   	return ret;
>

Chris Wilson March 16, 2018, 9:55 p.m. UTC | #4

Quoting Daniele Ceraolo Spurio (2018-03-16 20:28:25)
> 
> 
> On 16/03/18 05:14, Mika Kuoppala wrote:
> > From: Michel Thierry <[e-mail]>
> > 
> > The bits used to reset the different engines/domains have changed in
> > GEN11, this patch maps the reset engine mask bits with the new bits
> > in the reset control register.
> > 
> > v2: Use shift-left instead of BIT macro to match the file style (Paulo).
> > v3: Reuse gen8_reset_engines (Daniele).
> > v4: Do not call intel_uncore_forcewake_reset after reset, we may be
> > using the forcewake to read protected registers elsewhere and those
> > results may be clobbered by the concurrent dropping of forcewake.
> > 
> > bspec: 19212
> > Cc: Oscar Mateo <[e-mail]>
> > Cc: Antonio Argenziano <[e-mail]>
> > Cc: Paulo Zanoni <[e-mail]>
> > Cc: Daniele Ceraolo Spurio <[e-mail]>
> > Acked-by: Daniele Ceraolo Spurio <[e-mail]>
> > Signed-off-by: Michel Thierry <[e-mail]>
> > ---
> >   drivers/gpu/drm/i915/i915_reg.h     | 11 ++++++++
> >   drivers/gpu/drm/i915/intel_uncore.c | 53 +++++++++++++++++++++++++++++++++++--
> >   2 files changed, 62 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 9eaaa96287ec..f3cc77690124 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -301,6 +301,17 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> >   #define  GEN6_GRDOM_VECS            (1 << 4)
> >   #define  GEN9_GRDOM_GUC                     (1 << 5)
> >   #define  GEN8_GRDOM_MEDIA2          (1 << 7)
> > +/* GEN11 changed all bit defs except for FULL & RENDER */
> > +#define  GEN11_GRDOM_FULL            GEN6_GRDOM_FULL
> > +#define  GEN11_GRDOM_RENDER          GEN6_GRDOM_RENDER
> > +#define  GEN11_GRDOM_BLT             (1 << 2)
> > +#define  GEN11_GRDOM_GUC             (1 << 3)
> > +#define  GEN11_GRDOM_MEDIA           (1 << 5)
> > +#define  GEN11_GRDOM_MEDIA2          (1 << 6)
> > +#define  GEN11_GRDOM_MEDIA3          (1 << 7)
> > +#define  GEN11_GRDOM_MEDIA4          (1 << 8)
> > +#define  GEN11_GRDOM_VECS            (1 << 13)
> > +#define  GEN11_GRDOM_VECS2           (1 << 14)
> >   
> >   #define RING_PP_DIR_BASE(engine)    _MMIO((engine)->mmio_base+0x228)
> >   #define RING_PP_DIR_BASE_READ(engine)       _MMIO((engine)->mmio_base+0x518)
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index 4c616d074a97..cabbf0e682e7 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -1909,6 +1909,50 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv,
> >       return gen6_hw_domain_reset(dev_priv, hw_mask);
> >   }
> >   
> > +/**
> > + * gen11_reset_engines - reset individual engines
> > + * @dev_priv: i915 device
> > + * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for full reset
> > + *
> > + * This function will reset the individual engines that are set in engine_mask.
> > + * If you provide ALL_ENGINES as mask, full global domain reset will be issued.
> > + *
> > + * Note: It is responsibility of the caller to handle the difference between
> > + * asking full domain reset versus reset for all available individual engines.
> > + *
> > + * Returns 0 on success, nonzero on error.
> > + */
> > +static int gen11_reset_engines(struct drm_i915_private *dev_priv,
> > +                            unsigned engine_mask)
> > +{
> > +     struct intel_engine_cs *engine;
> > +     const u32 hw_engine_mask[I915_NUM_ENGINES] = {
> > +             [RCS] = GEN11_GRDOM_RENDER,
> > +             [BCS] = GEN11_GRDOM_BLT,
> > +             [VCS] = GEN11_GRDOM_MEDIA,
> > +             [VCS2] = GEN11_GRDOM_MEDIA2,
> > +             [VCS3] = GEN11_GRDOM_MEDIA3,
> > +             [VCS4] = GEN11_GRDOM_MEDIA4,
> > +             [VECS] = GEN11_GRDOM_VECS,
> > +             [VECS2] = GEN11_GRDOM_VECS2,
> > +     };
> 
> Just a thought, but since this function is a copy of gen6_reset_engines 
> with the only difference being the array (GEN11_GRDOM_FULL is also the 
> same as GEN6_GRDOM_FULL), would it make sense to just add the array to 
> the gen6 function? e.g.:
> 
>         const u32 gen6_hw_engine_mask[] = {
>         ....
>         }
>         const u32 gen11_hw_engine_mask[] = {
>         ....
>         }
> 
>         const u32 *hw_engine_mask = INTEL_GEN(dev_priv) >= 11 ?
>                 gen11_hw_engine_mask : gen6_hw_engine_mask;
> 

Oh, and we are definitely in the territory where static const should
result in a smaller binary (.text + .data).
-Chris

Michel Thierry March 27, 2018, 4:26 p.m. UTC | #5

On 3/16/2018 1:28 PM, Daniele Ceraolo Spurio wrote:
> 
> 
> On 16/03/18 05:14, Mika Kuoppala wrote:
>> From: Michel Thierry <[e-mail]>
>>
>> The bits used to reset the different engines/domains have changed in
>> GEN11, this patch maps the reset engine mask bits with the new bits
>> in the reset control register.
>>
>> v2: Use shift-left instead of BIT macro to match the file style (Paulo).
>> v3: Reuse gen8_reset_engines (Daniele).
>> v4: Do not call intel_uncore_forcewake_reset after reset, we may be
>> using the forcewake to read protected registers elsewhere and those
>> results may be clobbered by the concurrent dropping of forcewake.
>>
>> bspec: 19212
>> Cc: Oscar Mateo <[e-mail]>
>> Cc: Antonio Argenziano <[e-mail]>
>> Cc: Paulo Zanoni <[e-mail]>
>> Cc: Daniele Ceraolo Spurio <[e-mail]>
>> Acked-by: Daniele Ceraolo Spurio <[e-mail]>
>> Signed-off-by: Michel Thierry <[e-mail]>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h     | 11 ++++++++
>>   drivers/gpu/drm/i915/intel_uncore.c | 53 
>> +++++++++++++++++++++++++++++++++++--
>>   2 files changed, 62 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h 
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 9eaaa96287ec..f3cc77690124 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -301,6 +301,17 @@ static inline bool i915_mmio_reg_valid(i915_reg_t 
>> reg)
>>   #define  GEN6_GRDOM_VECS        (1 << 4)
>>   #define  GEN9_GRDOM_GUC            (1 << 5)
>>   #define  GEN8_GRDOM_MEDIA2        (1 << 7)
>> +/* GEN11 changed all bit defs except for FULL & RENDER */
>> +#define  GEN11_GRDOM_FULL        GEN6_GRDOM_FULL
>> +#define  GEN11_GRDOM_RENDER        GEN6_GRDOM_RENDER
>> +#define  GEN11_GRDOM_BLT        (1 << 2)
>> +#define  GEN11_GRDOM_GUC        (1 << 3)
>> +#define  GEN11_GRDOM_MEDIA        (1 << 5)
>> +#define  GEN11_GRDOM_MEDIA2        (1 << 6)
>> +#define  GEN11_GRDOM_MEDIA3        (1 << 7)
>> +#define  GEN11_GRDOM_MEDIA4        (1 << 8)
>> +#define  GEN11_GRDOM_VECS        (1 << 13)
>> +#define  GEN11_GRDOM_VECS2        (1 << 14)
>>   #define RING_PP_DIR_BASE(engine)    _MMIO((engine)->mmio_base+0x228)
>>   #define RING_PP_DIR_BASE_READ(engine)    
>> _MMIO((engine)->mmio_base+0x518)
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c 
>> b/drivers/gpu/drm/i915/intel_uncore.c
>> index 4c616d074a97..cabbf0e682e7 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -1909,6 +1909,50 @@ static int gen6_reset_engines(struct 
>> drm_i915_private *dev_priv,
>>       return gen6_hw_domain_reset(dev_priv, hw_mask);
>>   }
>> +/**
>> + * gen11_reset_engines - reset individual engines
>> + * @dev_priv: i915 device
>> + * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for 
>> full reset
>> + *
>> + * This function will reset the individual engines that are set in 
>> engine_mask.
>> + * If you provide ALL_ENGINES as mask, full global domain reset will 
>> be issued.
>> + *
>> + * Note: It is responsibility of the caller to handle the difference 
>> between
>> + * asking full domain reset versus reset for all available individual 
>> engines.
>> + *
>> + * Returns 0 on success, nonzero on error.
>> + */
>> +static int gen11_reset_engines(struct drm_i915_private *dev_priv,
>> +                   unsigned engine_mask)
>> +{
>> +    struct intel_engine_cs *engine;
>> +    const u32 hw_engine_mask[I915_NUM_ENGINES] = {
>> +        [RCS] = GEN11_GRDOM_RENDER,
>> +        [BCS] = GEN11_GRDOM_BLT,
>> +        [VCS] = GEN11_GRDOM_MEDIA,
>> +        [VCS2] = GEN11_GRDOM_MEDIA2,
>> +        [VCS3] = GEN11_GRDOM_MEDIA3,
>> +        [VCS4] = GEN11_GRDOM_MEDIA4,
>> +        [VECS] = GEN11_GRDOM_VECS,
>> +        [VECS2] = GEN11_GRDOM_VECS2,
>> +    };
> 
> Just a thought, but since this function is a copy of gen6_reset_engines 
> with the only difference being the array (GEN11_GRDOM_FULL is also the 
> same as GEN6_GRDOM_FULL), would it make sense to just add the array to 
> the gen6 function? e.g.:

There are more changes for gen11 coming (locking/unlocking the shared 
SFC units), so I don't think it's a good idea to combine them.

> 
>      const u32 gen6_hw_engine_mask[] = {
>      ....
>      }
>      const u32 gen11_hw_engine_mask[] = {
>      ....
>      }
> 
>      const u32 *hw_engine_mask = INTEL_GEN(dev_priv) >= 11 ?
>          gen11_hw_engine_mask : gen6_hw_engine_mask;
> 
> 
> My Ack still stands regardless and I also agree with renaming the 
> defines to be closer to the specs.
> 
> Daniele
> 
>> +    u32 hw_mask;
>> +
>> +    BUILD_BUG_ON(VECS2 + 1 != I915_NUM_ENGINES);
>> +
>> +    if (engine_mask == ALL_ENGINES) {
>> +        hw_mask = GEN11_GRDOM_FULL;
>> +    } else {
>> +        unsigned int tmp;
>> +
>> +        hw_mask = 0;
>> +        for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
>> +            hw_mask |= hw_engine_mask[engine->id];
>> +    }
>> +
>> +    return gen6_hw_domain_reset(dev_priv, hw_mask);
>> +}
>> +
>>   /**
>>    * __intel_wait_for_register_fw - wait until register matches 
>> expected state
>>    * @dev_priv: the i915 device
>> @@ -2056,7 +2100,10 @@ static int gen8_reset_engines(struct 
>> drm_i915_private *dev_priv,
>>           if (gen8_reset_engine_start(engine))
>>               goto not_ready;
>> -    return gen6_reset_engines(dev_priv, engine_mask);
>> +    if (INTEL_GEN(dev_priv) >= 11)
>> +        return gen11_reset_engines(dev_priv, engine_mask);
>> +    else
>> +        return gen6_reset_engines(dev_priv, engine_mask);
>>   not_ready:
>>       for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
>> @@ -2141,12 +2188,14 @@ bool intel_has_reset_engine(struct 
>> drm_i915_private *dev_priv)
>>   int intel_reset_guc(struct drm_i915_private *dev_priv)
>>   {
>> +    u32 guc_domain = INTEL_GEN(dev_priv) >= 11 ? GEN11_GRDOM_GUC :
>> +                             GEN9_GRDOM_GUC;
>>       int ret;
>>       GEM_BUG_ON(!HAS_GUC(dev_priv));
>>       intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>> -    ret = gen6_hw_domain_reset(dev_priv, GEN9_GRDOM_GUC);
>> +    ret = gen6_hw_domain_reset(dev_priv, guc_domain);
>>       intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
>>       return ret;
>>

Patch

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9eaaa96287ec..f3cc77690124 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -301,6 +301,17 @@  static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define  GEN6_GRDOM_VECS		(1 << 4)
 #define  GEN9_GRDOM_GUC			(1 << 5)
 #define  GEN8_GRDOM_MEDIA2		(1 << 7)
+/* GEN11 changed all bit defs except for FULL & RENDER */
+#define  GEN11_GRDOM_FULL		GEN6_GRDOM_FULL
+#define  GEN11_GRDOM_RENDER		GEN6_GRDOM_RENDER
+#define  GEN11_GRDOM_BLT		(1 << 2)
+#define  GEN11_GRDOM_GUC		(1 << 3)
+#define  GEN11_GRDOM_MEDIA		(1 << 5)
+#define  GEN11_GRDOM_MEDIA2		(1 << 6)
+#define  GEN11_GRDOM_MEDIA3		(1 << 7)
+#define  GEN11_GRDOM_MEDIA4		(1 << 8)
+#define  GEN11_GRDOM_VECS		(1 << 13)
+#define  GEN11_GRDOM_VECS2		(1 << 14)
 
 #define RING_PP_DIR_BASE(engine)	_MMIO((engine)->mmio_base+0x228)
 #define RING_PP_DIR_BASE_READ(engine)	_MMIO((engine)->mmio_base+0x518)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 4c616d074a97..cabbf0e682e7 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1909,6 +1909,50 @@  static int gen6_reset_engines(struct drm_i915_private *dev_priv,
 	return gen6_hw_domain_reset(dev_priv, hw_mask);
 }
 
+/**
+ * gen11_reset_engines - reset individual engines
+ * @dev_priv: i915 device
+ * @engine_mask: mask of intel_ring_flag() engines or ALL_ENGINES for full reset
+ *
+ * This function will reset the individual engines that are set in engine_mask.
+ * If you provide ALL_ENGINES as mask, full global domain reset will be issued.
+ *
+ * Note: It is responsibility of the caller to handle the difference between
+ * asking full domain reset versus reset for all available individual engines.
+ *
+ * Returns 0 on success, nonzero on error.
+ */
+static int gen11_reset_engines(struct drm_i915_private *dev_priv,
+			       unsigned engine_mask)
+{
+	struct intel_engine_cs *engine;
+	const u32 hw_engine_mask[I915_NUM_ENGINES] = {
+		[RCS] = GEN11_GRDOM_RENDER,
+		[BCS] = GEN11_GRDOM_BLT,
+		[VCS] = GEN11_GRDOM_MEDIA,
+		[VCS2] = GEN11_GRDOM_MEDIA2,
+		[VCS3] = GEN11_GRDOM_MEDIA3,
+		[VCS4] = GEN11_GRDOM_MEDIA4,
+		[VECS] = GEN11_GRDOM_VECS,
+		[VECS2] = GEN11_GRDOM_VECS2,
+	};
+	u32 hw_mask;
+
+	BUILD_BUG_ON(VECS2 + 1 != I915_NUM_ENGINES);
+
+	if (engine_mask == ALL_ENGINES) {
+		hw_mask = GEN11_GRDOM_FULL;
+	} else {
+		unsigned int tmp;
+
+		hw_mask = 0;
+		for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
+			hw_mask |= hw_engine_mask[engine->id];
+	}
+
+	return gen6_hw_domain_reset(dev_priv, hw_mask);
+}
+
 /**
  * __intel_wait_for_register_fw - wait until register matches expected state
  * @dev_priv: the i915 device
@@ -2056,7 +2100,10 @@  static int gen8_reset_engines(struct drm_i915_private *dev_priv,
 		if (gen8_reset_engine_start(engine))
 			goto not_ready;
 
-	return gen6_reset_engines(dev_priv, engine_mask);
+	if (INTEL_GEN(dev_priv) >= 11)
+		return gen11_reset_engines(dev_priv, engine_mask);
+	else
+		return gen6_reset_engines(dev_priv, engine_mask);
 
 not_ready:
 	for_each_engine_masked(engine, dev_priv, engine_mask, tmp)
@@ -2141,12 +2188,14 @@  bool intel_has_reset_engine(struct drm_i915_private *dev_priv)
 
 int intel_reset_guc(struct drm_i915_private *dev_priv)
 {
+	u32 guc_domain = INTEL_GEN(dev_priv) >= 11 ? GEN11_GRDOM_GUC :
+						     GEN9_GRDOM_GUC;
 	int ret;
 
 	GEM_BUG_ON(!HAS_GUC(dev_priv));
 
 	intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
-	ret = gen6_hw_domain_reset(dev_priv, GEN9_GRDOM_GUC);
+	ret = gen6_hw_domain_reset(dev_priv, guc_domain);
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
 
 	return ret;