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?
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@xxxxxxxxx>
> > 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@xxxxxxxxx>
> > > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> > > > ---
> > > > 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@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
