All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/cnl: Fix SSEU Device Status.
@ 2017-10-26  0:15 Rodrigo Vivi
  2017-10-26  0:35 ` ✓ Fi.CI.BAT: success for " Patchwork
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Rodrigo Vivi @ 2017-10-26  0:15 UTC (permalink / raw)
  To: intel-gfx; +Cc: Rodrigo Vivi

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)
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/cnl: Fix SSEU Device Status.
  2017-10-26  0:15 [PATCH] drm/i915/cnl: Fix SSEU Device Status Rodrigo Vivi
@ 2017-10-26  0:35 ` Patchwork
  2017-10-26  1:22 ` ✓ Fi.CI.IGT: " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2017-10-26  0:35 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/cnl: Fix SSEU Device Status.
URL   : https://patchwork.freedesktop.org/series/32665/
State : success

== Summary ==

Series 32665v1 drm/i915/cnl: Fix SSEU Device Status.
https://patchwork.freedesktop.org/api/1.0/series/32665/revisions/1/mbox/

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:451s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:457s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:371s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:530s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:264s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:500s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:498s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:497s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:482s
fi-cfl-s         total:289  pass:253  dwarn:4   dfail:0   fail:0   skip:32  time:549s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:616s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:409s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:246s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:577s
fi-glk-dsi       total:289  pass:258  dwarn:0   dfail:0   fail:1   skip:30  time:486s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:429s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:429s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:430s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:497s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:457s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:493s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:571s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:477s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:579s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:543s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:449s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:586s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:643s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:515s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:499s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:451s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:561s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:417s

2ea0b3d47030274c97624258e09fc7d1ffd0e0f2 drm-tip: 2017y-10m-25d-18h-42m-20s UTC integration manifest
7f4c25c0d2c0 drm/i915/cnl: Fix SSEU Device Status.

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6196/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915/cnl: Fix SSEU Device Status.
  2017-10-26  0:15 [PATCH] drm/i915/cnl: Fix SSEU Device Status Rodrigo Vivi
  2017-10-26  0:35 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-10-26  1:22 ` Patchwork
  2017-10-26 14:31 ` [PATCH] " Lionel Landwerlin
  2017-10-27 22:05 ` Oscar Mateo
  3 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2017-10-26  1:22 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/cnl: Fix SSEU Device Status.
URL   : https://patchwork.freedesktop.org/series/32665/
State : success

== Summary ==

Test kms_cursor_legacy:
        Subgroup flip-vs-cursor-legacy:
                fail       -> PASS       (shard-hsw) fdo#102670
Test kms_busy:
        Subgroup extended-modeset-hang-oldfb-with-reset-render-B:
                dmesg-warn -> PASS       (shard-hsw) fdo#102249
Test kms_flip:
        Subgroup flip-vs-absolute-wf_vblank:
                pass       -> FAIL       (shard-hsw) fdo#100368 +1

fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368

shard-hsw        total:2539 pass:1431 dwarn:1   dfail:1   fail:9   skip:1097 time:9268s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6196/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/cnl: Fix SSEU Device Status.
  2017-10-26  0:15 [PATCH] drm/i915/cnl: Fix SSEU Device Status Rodrigo Vivi
  2017-10-26  0:35 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-10-26  1:22 ` ✓ Fi.CI.IGT: " Patchwork
@ 2017-10-26 14:31 ` Lionel Landwerlin
  2017-10-26 18:36   ` Rodrigo Vivi
  2017-10-27 22:05 ` Oscar Mateo
  3 siblings, 1 reply; 10+ messages in thread
From: Lionel Landwerlin @ 2017-10-26 14:31 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx

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>

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/cnl: Fix SSEU Device Status.
  2017-10-26 14:31 ` [PATCH] " Lionel Landwerlin
@ 2017-10-26 18:36   ` Rodrigo Vivi
  2017-10-27 14:47     ` Lionel Landwerlin
  0 siblings, 1 reply; 10+ messages in thread
From: Rodrigo Vivi @ 2017-10-26 18:36 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/cnl: Fix SSEU Device Status.
  2017-10-26 18:36   ` Rodrigo Vivi
@ 2017-10-27 14:47     ` Lionel Landwerlin
  2017-10-27 17:10       ` Rodrigo Vivi
  0 siblings, 1 reply; 10+ messages in thread
From: Lionel Landwerlin @ 2017-10-27 14:47 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/cnl: Fix SSEU Device Status.
  2017-10-27 14:47     ` Lionel Landwerlin
@ 2017-10-27 17:10       ` Rodrigo Vivi
  2017-10-27 18:22         ` Lionel Landwerlin
  0 siblings, 1 reply; 10+ messages in thread
From: Rodrigo Vivi @ 2017-10-27 17:10 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

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@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/cnl: Fix SSEU Device Status.
  2017-10-27 17:10       ` Rodrigo Vivi
@ 2017-10-27 18:22         ` Lionel Landwerlin
  0 siblings, 0 replies; 10+ messages in thread
From: Lionel Landwerlin @ 2017-10-27 18:22 UTC (permalink / raw)
  To: Rodrigo Vivi; +Cc: intel-gfx

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@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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/cnl: Fix SSEU Device Status.
  2017-10-26  0:15 [PATCH] drm/i915/cnl: Fix SSEU Device Status Rodrigo Vivi
                   ` (2 preceding siblings ...)
  2017-10-26 14:31 ` [PATCH] " Lionel Landwerlin
@ 2017-10-27 22:05 ` Oscar Mateo
  2017-10-30 13:11   ` David Weinehall
  3 siblings, 1 reply; 10+ messages in thread
From: Oscar Mateo @ 2017-10-27 22:05 UTC (permalink / raw)
  To: Rodrigo Vivi, intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 5974 bytes --]



On 10/25/2017 05:15 PM, 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

s/becuase/because

Seems like a look compromise (a corrected BSpec would be better, but 
¯\_(ツ)_/¯)

Reviewed-by: Oscar Mateo <oscar.mateo@intel.com>

> +		 * 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)


[-- Attachment #1.2: Type: text/html, Size: 6676 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] drm/i915/cnl: Fix SSEU Device Status.
  2017-10-27 22:05 ` Oscar Mateo
@ 2017-10-30 13:11   ` David Weinehall
  0 siblings, 0 replies; 10+ messages in thread
From: David Weinehall @ 2017-10-30 13:11 UTC (permalink / raw)
  To: Oscar Mateo; +Cc: intel-gfx, Rodrigo Vivi

On Fri, Oct 27, 2017 at 03:05:58PM -0700, Oscar Mateo wrote:
> 
> 
> On 10/25/2017 05:15 PM, 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
> 
> s/becuase/because

s/reserverd/reserved,/

> Seems like a look compromise (a corrected BSpec would be better, but
> ¯\_(ツ)_/¯)
> 
> Reviewed-by: Oscar Mateo <oscar.mateo@intel.com>
> 
> > +		 * 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

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2017-10-30 13:12 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-26  0:15 [PATCH] drm/i915/cnl: Fix SSEU Device Status Rodrigo Vivi
2017-10-26  0:35 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-10-26  1:22 ` ✓ Fi.CI.IGT: " Patchwork
2017-10-26 14:31 ` [PATCH] " Lionel Landwerlin
2017-10-26 18:36   ` Rodrigo Vivi
2017-10-27 14:47     ` Lionel Landwerlin
2017-10-27 17:10       ` Rodrigo Vivi
2017-10-27 18:22         ` Lionel Landwerlin
2017-10-27 22:05 ` Oscar Mateo
2017-10-30 13:11   ` David Weinehall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.