On 29/08/2018 13:29, Tvrtko Ursulin wrote:
On 29/08/2018 13:11, Tvrtko Ursulin wrote:
On 29/08/2018 13:09, Tvrtko Ursulin wrote:
On 29/08/2018 13:02, Tvrtko Ursulin wrote:
On 29/08/2018 12:07, Lionel Landwerlin wrote:
On 29/08/2018 11:54, Tvrtko Ursulin wrote:
On 22/08/2018 17:33, Lionel Landwerlin wrote:
On 22/08/2018 17:18, Tvrtko Ursulin wrote:
From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
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@xxxxxxxxx>
Bspec: 12247
Reported-by: tony.ye@xxxxxxxxx
Suggested-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
Cc: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
Cc: tony.ye@xxxxxxxxx
---
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.
Happy to r-b this one since we clarified it is fine?
Regards,
Tvrtko
I think there is still an issue here with regard to the subslice
programming.
You can only program values in [1, 4].
But because we expose up to 8 subslices, you need to take that
mask and alter the slice mask based on its value.
Hmm true, thanks. I think we need to somehow express this
restriction on the uAPI level rather than silently mask it. Shall
we reject attempts to configure subslice mask if hweight(mask) >
max_subslices / 2 for ICL-LP?
No wait, I was in the dynamic sseu world, but this also applies to
the current code base..
When we program SScount in drm-tip with the hweight of 8 we overflow
the bitfield already and splat over SScountEn. Luckily with no ill
effect since we set it anyway.
So the fix needs to be in make_rpcs today..
if (GEN11_LP) {
if (hweight(slice_mask) == 1)
&& hweight(subslice_mask) < max_subslices
So to leave the default full config alone.
Which means that drm-tip configures the 1x6x8 part as SScount=0b110,
which is another undocumented value according to BSpec. Furthermore
the allowed configuration table suggests only three subslices can be
enabled when SScountEn is set, so like the situation on BXT/GLK, now I
need to figure out if we are inadvertently only enabling half of the
subslices on ICL-LP.
Or maybe make the upper 4 bits of the subslice map to "big subslices"
and lower bottom 4 to "small subslices".
Then have the restrictions checked in ioctl() context_set_param.
-
Lionel
Tvrtko
subslice_count /= 2;
else
subslice_en = 0;
}
I think..
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
