[Intel-gfx] [PATCH] drm/i915/cnl: Fix SSEU Device Status.
Lionel Landwerlin
lionel.g.landwerlin at intel.com
Fri Oct 27 18:22:33 UTC 2017
On 27/10/17 18:10, Rodrigo Vivi wrote:
> On Fri, Oct 27, 2017 at 02:47:24PM +0000, Lionel Landwerlin wrote:
>> 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
> I have not looked to BXT, but I also see all 0 like this on CNL
> when RC6 enters apparently the status is live.
>
> So, could you please trigger some workload and see if
> you see some difference?
>
> Did you see something different on spec for BXT compared to SKL?
I opened this bug : https://bugs.freedesktop.org/show_bug.cgi?id=103484
Chris has been kind enough to look into it.
>
> Thanks,
> Rodrigo.
>
>> 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 at 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 at intel.com>
>>>>> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at 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)
More information about the Intel-gfx
mailing list