[PATCH] drm/i915/icl: Fix context slice count configuration

[PATCH] drm/i915/icl: Fix context slice count configuration

[Tvrtko Ursulin]

From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

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@intel.com>
Bspec: 12247
Reported-by: tony.ye@intel.com
Suggested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Cc: tony.ye@intel.com
---
 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;
 	}
 
 	if (INTEL_INFO(dev_priv)->sseu.has_subslice_pg) {
-- 
2.17.1

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

Re: [PATCH] drm/i915/icl: Fix context slice count configuration

[Lionel Landwerlin]

On 22/08/2018 17:18, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>
> 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@intel.com>
> Bspec: 12247
> Reported-by: tony.ye@intel.com
> Suggested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Cc: tony.ye@intel.com
> ---
>   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.


>   	}
>   
>   	if (INTEL_INFO(dev_priv)->sseu.has_subslice_pg) {


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

✗ Fi.CI.CHECKPATCH: warning for drm/i915/icl: Fix context slice count configuration

[Patchwork]

== Series Details ==

Series: drm/i915/icl: Fix context slice count configuration
URL   : https://patchwork.freedesktop.org/series/48570/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
5cf33a6a09d9 drm/i915/icl: Fix context slice count configuration
-:10: WARNING:TYPO_SPELLING: 'writting' may be misspelled - perhaps 'writing'?
#10: 
writting to a bitfield starting three bits higher than it should). These

total: 0 errors, 1 warnings, 0 checks, 26 lines checked

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

✓ Fi.CI.BAT: success for drm/i915/icl: Fix context slice count configuration

[Patchwork]

== Series Details ==

Series: drm/i915/icl: Fix context slice count configuration
URL   : https://patchwork.freedesktop.org/series/48570/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4696 -> Patchwork_9992 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/48570/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9992 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_pipe_crc_basic@hang-read-crc-pipe-b:
      {fi-byt-clapper}:   PASS -> FAIL (fdo#107362, fdo#103191)

    
    ==== Possible fixes ====

    igt@drv_selftest@live_hangcheck:
      {fi-skl-iommu}:     DMESG-FAIL (fdo#106560, fdo#107174) -> PASS
      fi-kbl-7567u:       DMESG-FAIL (fdo#106560, fdo#106947) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      {fi-byt-clapper}:   FAIL (fdo#107362, fdo#103191) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      {fi-cfl-8109u}:     INCOMPLETE (fdo#106070) -> PASS

    
    ==== Warnings ====

    {igt@pm_rpm@module-reload}:
      fi-cnl-psr:         FAIL -> WARN (fdo#107602)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#106070 https://bugs.freedesktop.org/show_bug.cgi?id=106070
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107602 https://bugs.freedesktop.org/show_bug.cgi?id=107602


== Participating hosts (54 -> 46) ==

  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-glk-j4005 fi-gdg-551 


== Build changes ==

    * Linux: CI_DRM_4696 -> Patchwork_9992

  CI_DRM_4696: ced152c46fc90f7c1ac8963850d64c9f1ce652d6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4608: 94ebd21177feedf03e8f6dd1e73dca1a6ec7a0ac @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9992: 5cf33a6a09d933f2cc8def1e8f43aa1812615aab @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

5cf33a6a09d9 drm/i915/icl: Fix context slice count configuration

== Logs ==

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

Re: [PATCH] drm/i915/icl: Fix context slice count configuration

[Tvrtko Ursulin]

On 22/08/2018 17:33, Lionel Landwerlin wrote:
> On 22/08/2018 17:18, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> 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@intel.com>
>> Bspec: 12247
>> Reported-by: tony.ye@intel.com
>> Suggested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> Cc: tony.ye@intel.com
>> ---
>>   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.

I interpreted the note in my head as "In ICL, _if_ _the_ software 
programs.." so did not see a problem. Thought that would be just some 
hidden/under the covers remapping hw would do. But I see now that was 
wrong, and you are most likely right. I'll try to do some digging to 
understand this better.

But for the second part of it, I don't see why 1x3 configuration would 
be illegal. If software must assume hw is 2x4, even if in reality it is 
1x8, then 1x3 would still be legal, no?

I thought the cause of the hang on ICL was that when Tony was asking for 
1x3, the code was actually programming a request for 8x3 - which is 
illegal (as in slice count must be 1 to enable subslice pg) and so would 
fail to actually turn of the unwanted subslices.

But then I also though on ICL we deal with masks and not counts when 
programming the hardware. Since apparently it is counts both for slices 
and subslices, I am mightily confused as to how media-driver would even 
theoretically be able to turn off a _specific_ (sub)slice?!

Regards,

Tvrtko

> 
>>       }
>>       if (INTEL_INFO(dev_priv)->sseu.has_subslice_pg) {
> 
> 
> _______________________________________________
> 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

Re: [PATCH] drm/i915/icl: Fix context slice count configuration

[Lionel Landwerlin]

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

On 22/08/2018 18:07, 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@intel.com>
>>>
>>> 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@intel.com>
>>> Bspec: 12247
>>> Reported-by: tony.ye@intel.com
>>> Suggested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>> Cc: tony.ye@intel.com
>>> ---
>>>   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.
>
> I interpreted the note in my head as "In ICL, _if_ _the_ software 
> programs.." so did not see a problem. Thought that would be just some 
> hidden/under the covers remapping hw would do. But I see now that was 
> wrong, and you are most likely right. I'll try to do some digging to 
> understand this better.
>
> But for the second part of it, I don't see why 1x3 configuration would 
> be illegal. If software must assume hw is 2x4, even if in reality it 
> is 1x8, then 1x3 would still be legal, no?


We still seem to put a subslice number in the register for ICL (values 
being 0b001, 0b010, 0b011 & 0b100).

My understanding is that the hardware will just multiply that value by 2 
to map to the 1x8 underlying topology.

So if that's the case, you can't really do odd numbers... ¯\_(ツ)_/¯


>
> I thought the cause of the hang on ICL was that when Tony was asking 
> for 1x3, the code was actually programming a request for 8x3 - which 
> is illegal (as in slice count must be 1 to enable subslice pg) and so 
> would fail to actually turn of the unwanted subslices.
>
> But then I also though on ICL we deal with masks and not counts when 
> programming the hardware. Since apparently it is counts both for 
> slices and subslices, I am mightily confused as to how media-driver 
> would even theoretically be able to turn off a _specific_ (sub)slice?!


The fact that the feature needed isn't implemented at by the thread 
dispatcher is really strange to me too.

It sounds like we're forced to use a bigger hammer than what we really need.

As to how that maps to the right subslices is also unknown to me.

The only explanation I have is that subslices with no VME samplers get 
turn off first in the list of subslices to turn off.


Cheers,


-

Lionel


>
> Regards,
>
> Tvrtko
>
>>
>>>       }
>>>       if (INTEL_INFO(dev_priv)->sseu.has_subslice_pg) {
>>
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>


[-- Attachment #1.2: Type: text/html, Size: 9959 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

✓ Fi.CI.IGT: success for drm/i915/icl: Fix context slice count configuration

[Patchwork]

== Series Details ==

Series: drm/i915/icl: Fix context slice count configuration
URL   : https://patchwork.freedesktop.org/series/48570/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4696_full -> Patchwork_9992_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9992_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9992_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9992_full:

  === IGT changes ===

    ==== Warnings ====

    igt@kms_cursor_legacy@cursorb-vs-flipa-toggle:
      shard-hsw:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_9992_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ctx_isolation@vcs1-s3:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    igt@kms_flip@modeset-vs-vblank-race:
      shard-apl:          PASS -> FAIL (fdo#103060)

    
    ==== Possible fixes ====

    igt@drv_suspend@shrink:
      shard-glk:          FAIL (fdo#106886) -> PASS

    igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#105363) -> PASS +1

    igt@kms_flip@flip-vs-expired-vblank-interruptible:
      shard-glk:          FAIL (fdo#102887, fdo#105363) -> PASS

    igt@kms_setmode@basic:
      shard-apl:          FAIL (fdo#99912) -> PASS
      shard-glk:          FAIL (fdo#99912) -> PASS

    igt@perf@polling:
      shard-hsw:          FAIL (fdo#102252) -> PASS

    igt@pm_rpm@system-suspend:
      shard-kbl:          INCOMPLETE (fdo#107556, fdo#103665) -> PASS

    
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
  fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107556 https://bugs.freedesktop.org/show_bug.cgi?id=107556
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4696 -> Patchwork_9992

  CI_DRM_4696: ced152c46fc90f7c1ac8963850d64c9f1ce652d6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4608: 94ebd21177feedf03e8f6dd1e73dca1a6ec7a0ac @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9992: 5cf33a6a09d933f2cc8def1e8f43aa1812615aab @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH] drm/i915/icl: Fix context slice count configuration

[Tvrtko Ursulin]

On 22/08/2018 17:33, Lionel Landwerlin wrote:
> On 22/08/2018 17:18, Tvrtko Ursulin wrote:
>> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
>>
>> 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@intel.com>
>> Bspec: 12247
>> Reported-by: tony.ye@intel.com
>> Suggested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> Cc: tony.ye@intel.com
>> ---
>>   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

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

Re: [PATCH] drm/i915/icl: Fix context slice count configuration

[Lionel Landwerlin]

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@intel.com>
>>>
>>> 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@intel.com>
>>> Bspec: 12247
>>> Reported-by: tony.ye@intel.com
>>> Suggested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>> Cc: tony.ye@intel.com
>>> ---
>>>   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.

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

Re: [PATCH] drm/i915/icl: Fix context slice count configuration

[Tvrtko Ursulin]

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@intel.com>
>>>>
>>>> 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@intel.com>
>>>> Bspec: 12247
>>>> Reported-by: tony.ye@intel.com
>>>> Suggested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>> Cc: tony.ye@intel.com
>>>> ---
>>>>   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?

Regards,

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

Re: [PATCH] drm/i915/icl: Fix context slice count configuration

[Tvrtko Ursulin]

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@intel.com>
>>>>>
>>>>> 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@intel.com>
>>>>> Bspec: 12247
>>>>> Reported-by: tony.ye@intel.com
>>>>> Suggested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>>> Cc: tony.ye@intel.com
>>>>> ---
>>>>>   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)
		subslice_count /= 2;
	else
		subslice_en = 0;
}

I think..

Regards,

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

Re: [PATCH] drm/i915/icl: Fix context slice count configuration

[Tvrtko Ursulin]

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@intel.com>
>>>>>>
>>>>>> 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@intel.com>
>>>>>> Bspec: 12247
>>>>>> Reported-by: tony.ye@intel.com
>>>>>> Suggested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>>>> Cc: tony.ye@intel.com
>>>>>> ---
>>>>>>   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.

>          subslice_count /= 2;
>      else
>          subslice_en = 0;
> }
> 
> I think..
> 
> Regards,
> 
> Tvrtko
> _______________________________________________
> 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

Re: [PATCH] drm/i915/icl: Fix context slice count configuration

[Tvrtko Ursulin]

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@intel.com>
>>>>>>>
>>>>>>> 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@intel.com>
>>>>>>> Bspec: 12247
>>>>>>> Reported-by: tony.ye@intel.com
>>>>>>> Suggested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>>>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>>>>> Cc: tony.ye@intel.com
>>>>>>> ---
>>>>>>>   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.

Tvrtko

>>          subslice_count /= 2;
>>      else
>>          subslice_en = 0;
>> }
>>
>> I think..
>>
>> Regards,
>>
>> Tvrtko
>> _______________________________________________
>> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915/icl: Fix context slice count configuration

[Lionel Landwerlin]

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@intel.com>
>>>>>>>>
>>>>>>>> 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@intel.com>
>>>>>>>> Bspec: 12247
>>>>>>>> Reported-by: tony.ye@intel.com
>>>>>>>> Suggested-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>>>>>> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>>>>>>>> Cc: tony.ye@intel.com
>>>>>>>> ---
>>>>>>>>   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@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
>

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