Re: [PATCH] drm/i915/cnl: Fix SSEU Device Status.

[Lionel Landwerlin <lionel.g.landwerlin@intel.com>]

I don't know whether anyone noticed that sseu_status appears to be 
broken on BXT :

cat /sys/kernel/debug/dri/0/i915_sseu_status
SSEU Device Info
   Available Slice Mask: 0001
   Available Slice Total: 1
   Available Subslice Total: 2
   Available Slice0 Subslice Mask: 0006
   Available EU Total: 12
   Available EU Per Subslice: 6
   Has Pooled EU: no
   Has Slice Power Gating: no
   Has Subslice Power Gating: yes
   Has EU Power Gating: yes
SSEU Device Status
   Enabled Slice Mask: 0000
   Enabled Slice Total: 0
   Enabled Subslice Total: 0
   Enabled EU Total: 0
   Enabled EU Per Subslice: 0

GEN9_SLICE_PGCTL_ACK(0 -> 3) appears to be all set to 0

On 26/10/17 19:36, Rodrigo Vivi wrote:
> On Thu, Oct 26, 2017 at 02:31:16PM +0000, Lionel Landwerlin wrote:
>> Since I've been looking at EU_DISABLE* in intel_device_info.c, your patch
>> caught my eye :)
>> Reading the documentation I couldn't find anything wrong.
>>
>> Reviewed-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Thanks. Merged to dinq.
>
>> On 26/10/17 01:15, Rodrigo Vivi wrote:
>>> CNL adds an extra register for slice/subslice information.
>>> Although no SKU is planed with an extra slice let's already
>>> handle this extra piece of information so we don't have the
>>> risk in future of getting a part that might have chosen this
>>> part of the die instead of other slices or anything like that.
>>>
>>> Also if subslice is disabled the information of eu ack for that
>>> is garbage, so let's skip checks for eu if subslice is disabled
>>> as we skip the subslice if slice is disabled.
>>>
>>> The rest is pretty much like gen9.
>>>
>>> v2: Remove IS_CANNONLAKE from gen9 status function.
>>>
>>> v3: Consider s_max = 6 and ss_max=4 to run over all possible
>>>       slices and subslices possible by spec. Although no real
>>>       hardware will have that many slices/subslices.
>>>       To match with sseu info init.
>>> v4: Fix offset calculation for slices 4 and 5.
>>>       Removed Oscar's rv-b since this change also needs review.
>>> v5: Let's consider only valid bits for SLICE*_PGCTL_ACK.
>>>       This looks like wrong in Spec, but seems to be enough
>>>       for now. Whenever Spec gets updated and fixed we come
>>>       back and properly update the masks. Also add a FIXME,
>>>       so we can revisit this later when we find some strange
>>>       info on debugfs or when we noitce spec got updated.
>>>
>>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/i915_debugfs.c | 61 +++++++++++++++++++++++++++++++++++--
>>>    drivers/gpu/drm/i915/i915_reg.h     |  7 +++++
>>>    2 files changed, 66 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index c65e381b85f3..61c466ff87e0 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -4448,6 +4448,61 @@ static void cherryview_sseu_device_status(struct drm_i915_private *dev_priv,
>>>    	}
>>>    }
>>> +static void gen10_sseu_device_status(struct drm_i915_private *dev_priv,
>>> +				     struct sseu_dev_info *sseu)
>>> +{
>>> +	const struct intel_device_info *info = INTEL_INFO(dev_priv);
>>> +	int s_max = 6, ss_max = 4;
>>> +	int s, ss;
>>> +	u32 s_reg[s_max], eu_reg[2 * s_max], eu_mask[2];
>>> +
>>> +	for (s = 0; s < s_max; s++) {
>>> +		/*
>>> +		 * FIXME: Valid SS Mask respects the spec and read
>>> +		 * only valid bits for those registers, excluding reserverd
>>> +		 * although this seems wrong becuase it would leave many
>>> +		 * subslices without ACK.
>>> +		 */
>>> +		s_reg[s] = I915_READ(GEN10_SLICE_PGCTL_ACK(s)) &
>>> +			GEN10_PGCTL_VALID_SS_MASK(s);
>>> +		eu_reg[2 * s] = I915_READ(GEN10_SS01_EU_PGCTL_ACK(s));
>>> +		eu_reg[2 * s + 1] = I915_READ(GEN10_SS23_EU_PGCTL_ACK(s));
>>> +	}
>>> +
>>> +	eu_mask[0] = GEN9_PGCTL_SSA_EU08_ACK |
>>> +		     GEN9_PGCTL_SSA_EU19_ACK |
>>> +		     GEN9_PGCTL_SSA_EU210_ACK |
>>> +		     GEN9_PGCTL_SSA_EU311_ACK;
>>> +	eu_mask[1] = GEN9_PGCTL_SSB_EU08_ACK |
>>> +		     GEN9_PGCTL_SSB_EU19_ACK |
>>> +		     GEN9_PGCTL_SSB_EU210_ACK |
>>> +		     GEN9_PGCTL_SSB_EU311_ACK;
>>> +
>>> +	for (s = 0; s < s_max; s++) {
>>> +		if ((s_reg[s] & GEN9_PGCTL_SLICE_ACK) == 0)
>>> +			/* skip disabled slice */
>>> +			continue;
>>> +
>>> +		sseu->slice_mask |= BIT(s);
>>> +		sseu->subslice_mask = info->sseu.subslice_mask;
>>> +
>>> +		for (ss = 0; ss < ss_max; ss++) {
>>> +			unsigned int eu_cnt;
>>> +
>>> +			if (!(s_reg[s] & (GEN9_PGCTL_SS_ACK(ss))))
>>> +				/* skip disabled subslice */
>>> +				continue;
>>> +
>>> +			eu_cnt = 2 * hweight32(eu_reg[2 * s + ss / 2] &
>>> +					       eu_mask[ss % 2]);
>>> +			sseu->eu_total += eu_cnt;
>>> +			sseu->eu_per_subslice = max_t(unsigned int,
>>> +						      sseu->eu_per_subslice,
>>> +						      eu_cnt);
>>> +		}
>>> +	}
>>> +}
>>> +
>>>    static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
>>>    				    struct sseu_dev_info *sseu)
>>>    {
>>> @@ -4483,7 +4538,7 @@ static void gen9_sseu_device_status(struct drm_i915_private *dev_priv,
>>>    		sseu->slice_mask |= BIT(s);
>>> -		if (IS_GEN9_BC(dev_priv) || IS_CANNONLAKE(dev_priv))
>>> +		if (IS_GEN9_BC(dev_priv))
>>>    			sseu->subslice_mask =
>>>    				INTEL_INFO(dev_priv)->sseu.subslice_mask;
>>> @@ -4589,8 +4644,10 @@ static int i915_sseu_status(struct seq_file *m, void *unused)
>>>    		cherryview_sseu_device_status(dev_priv, &sseu);
>>>    	} else if (IS_BROADWELL(dev_priv)) {
>>>    		broadwell_sseu_device_status(dev_priv, &sseu);
>>> -	} else if (INTEL_GEN(dev_priv) >= 9) {
>>> +	} else if (IS_GEN9(dev_priv)) {
>>>    		gen9_sseu_device_status(dev_priv, &sseu);
>>> +	} else if (INTEL_GEN(dev_priv) >= 10) {
>>> +		gen10_sseu_device_status(dev_priv, &sseu);
>>>    	}
>>>    	intel_runtime_pm_put(dev_priv);
>>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>> index f138eae82bf0..8c775e96b4e4 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -8037,11 +8037,18 @@ enum {
>>>    #define   CHV_EU311_PG_ENABLE		(1<<1)
>>>    #define GEN9_SLICE_PGCTL_ACK(slice)	_MMIO(0x804c + (slice)*0x4)
>>> +#define GEN10_SLICE_PGCTL_ACK(slice)	_MMIO(0x804c + ((slice) / 3) * 0x34 + \
>>> +					      ((slice) % 3) * 0x4)
>>>    #define   GEN9_PGCTL_SLICE_ACK		(1 << 0)
>>>    #define   GEN9_PGCTL_SS_ACK(subslice)	(1 << (2 + (subslice)*2))
>>> +#define   GEN10_PGCTL_VALID_SS_MASK(slice) ((slice) == 0 ? 0x7F : 0x1F)
>>>    #define GEN9_SS01_EU_PGCTL_ACK(slice)	_MMIO(0x805c + (slice)*0x8)
>>> +#define GEN10_SS01_EU_PGCTL_ACK(slice)	_MMIO(0x805c + ((slice) / 3) * 0x30 + \
>>> +					      ((slice) % 3) * 0x8)
>>>    #define GEN9_SS23_EU_PGCTL_ACK(slice)	_MMIO(0x8060 + (slice)*0x8)
>>> +#define GEN10_SS23_EU_PGCTL_ACK(slice)	_MMIO(0x8060 + ((slice) / 3) * 0x30 + \
>>> +					      ((slice) % 3) * 0x8)
>>>    #define   GEN9_PGCTL_SSA_EU08_ACK	(1 << 0)
>>>    #define   GEN9_PGCTL_SSA_EU19_ACK	(1 << 2)
>>>    #define   GEN9_PGCTL_SSA_EU210_ACK	(1 << 4)
>>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx