[Intel-gfx] [PATCH] drm/i915/icl: Fix context slice count configuration

[Lionel Landwerlin]

On 22/08/2018 17:18, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>
> Bitfield width for configuring the active slice count has grown in Gen11
> so we need to program the GEN8_R_PWR_CLK_STATE accordingly.
>
> Current code was always requesting eight times the number of slices (due
> writting to a bitfield starting three bits higher than it should). These
> requests were luckily a) capped by the hardware to the available number of
> slices, and b) we haven't yet exported the code to ask for reduced slice
> configurations.
>
> Due both of the above there was no impact from this incorrect programming
> but we should still fix it.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Bspec: 12247
> Reported-by: tony.ye at intel.com
> Suggested-by: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin at intel.com>
> Cc: tony.ye at intel.com
> ---
>   drivers/gpu/drm/i915/i915_reg.h  |  2 ++
>   drivers/gpu/drm/i915/intel_lrc.c | 12 ++++++++----
>   2 files changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 59d06d0055bb..640f7b774a26 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -344,6 +344,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>   #define   GEN8_RPCS_S_CNT_ENABLE	(1 << 18)
>   #define   GEN8_RPCS_S_CNT_SHIFT		15
>   #define   GEN8_RPCS_S_CNT_MASK		(0x7 << GEN8_RPCS_S_CNT_SHIFT)
> +#define   GEN11_RPCS_S_CNT_SHIFT	12
> +#define   GEN11_RPCS_S_CNT_MASK		(0x3f << GEN11_RPCS_S_CNT_SHIFT)
>   #define   GEN8_RPCS_SS_CNT_ENABLE	(1 << 11)
>   #define   GEN8_RPCS_SS_CNT_SHIFT	8
>   #define   GEN8_RPCS_SS_CNT_MASK		(0x7 << GEN8_RPCS_SS_CNT_SHIFT)
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 36050f085071..43b8b0675ba0 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -2501,10 +2501,14 @@ make_rpcs(struct drm_i915_private *dev_priv)
>   	 * enablement.
>   	*/
>   	if (INTEL_INFO(dev_priv)->sseu.has_slice_pg) {
> -		rpcs |= GEN8_RPCS_S_CNT_ENABLE;
> -		rpcs |= hweight8(INTEL_INFO(dev_priv)->sseu.slice_mask) <<
> -			GEN8_RPCS_S_CNT_SHIFT;
> -		rpcs |= GEN8_RPCS_ENABLE;
> +		rpcs = hweight8(INTEL_INFO(dev_priv)->sseu.slice_mask);
> +
> +		if (INTEL_GEN(dev_priv) >= 11)
> +			rpcs <<= GEN11_RPCS_S_CNT_SHIFT;
> +		else
> +			rpcs <<= GEN8_RPCS_S_CNT_SHIFT;
> +
> +		rpcs |= GEN8_RPCS_ENABLE | GEN8_RPCS_S_CNT_ENABLE;


I don't know if you saw that wording in the documentation :

  "

Note: In ICL, software programs this register as if GT consists of 2 
slices with 4 subslices in each slice. Hardware maps this to the 1 
slice/8-subslice physical layout.

"


My understanding is that it would make this function a bit more 
complicated ;)

It also implies that on ICL you cannot select 3 subslices, which is 
unfortunately what Tony was trying to do.

Maybe some opens need to be raised as to what's possible on ICL.


>   	}
>   
>   	if (INTEL_INFO(dev_priv)->sseu.has_subslice_pg) {