[Intel-gfx] [PATCH] drm/i915/gt: Make the slice:unslice ratio request explicit for RPS

[Intel-gfx] [PATCH] drm/i915/gt: Make the slice:unslice ratio request explicit for RPS

[Chris Wilson]

In RPS, we have the option to only specify the unslice [ring] clock
ratio and for the pcu to derive the slice [gpu] clock ratio from its
magic table. We also have the option to tell the pcu to use our
requested gpu clock ratio, and for it to try and throttle the unslice
and slice ratios separately.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/gt/intel_rps.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
index 4dcfae16a7ce..07321e1b22f6 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -662,14 +662,17 @@ static int gen6_rps_set(struct intel_rps *rps, u8 val)
 	struct drm_i915_private *i915 = rps_to_i915(rps);
 	u32 swreq;
 
-	if (INTEL_GEN(i915) >= 9)
-		swreq = GEN9_FREQUENCY(val);
-	else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
+	if (INTEL_GEN(i915) >= 9) {
+		swreq = 0x2; /* only throttle slice, not unslice */
+		swreq |= val << 14; /* slice [gpu] ratio */
+		swreq |= val << 23; /* unslice [ring] ratio */
+	} else if (IS_HASWELL(i915) || IS_BROADWELL(i915)) {
 		swreq = HSW_FREQUENCY(val);
-	else
+	} else {
 		swreq = (GEN6_FREQUENCY(val) |
 			 GEN6_OFFSET(0) |
 			 GEN6_AGGRESSIVE_TURBO);
+	}
 	set(uncore, GEN6_RPNSWREQ, swreq);
 
 	return 0;
-- 
2.20.1

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

Re: [Intel-gfx] [PATCH] drm/i915/gt: Make the slice:unslice ratio request explicit for RPS

[Chris Wilson]

Quoting Chris Wilson (2020-04-21 14:45:12)
> In RPS, we have the option to only specify the unslice [ring] clock
> ratio and for the pcu to derive the slice [gpu] clock ratio from its
> magic table. We also have the option to tell the pcu to use our
> requested gpu clock ratio, and for it to try and throttle the unslice
> and slice ratios separately.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_rps.c | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 4dcfae16a7ce..07321e1b22f6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -662,14 +662,17 @@ static int gen6_rps_set(struct intel_rps *rps, u8 val)
>         struct drm_i915_private *i915 = rps_to_i915(rps);
>         u32 swreq;
>  
> -       if (INTEL_GEN(i915) >= 9)
> -               swreq = GEN9_FREQUENCY(val);
> -       else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
> +       if (INTEL_GEN(i915) >= 9) {
> +               swreq = 0x2; /* only throttle slice, not unslice */

0x0 == use implicit slice ratio
0x1 == use explicit slice ratio
0x2 == use separate throttling

Not sure if 0x2 actually was implemented in the end.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/gt: Make the slice:unslice ratio request explicit for RPS

[Chris Wilson]

Quoting Chris Wilson (2020-04-21 14:50:51)
> Quoting Chris Wilson (2020-04-21 14:45:12)
> > In RPS, we have the option to only specify the unslice [ring] clock
> > ratio and for the pcu to derive the slice [gpu] clock ratio from its
> > magic table. We also have the option to tell the pcu to use our
> > requested gpu clock ratio, and for it to try and throttle the unslice
> > and slice ratios separately.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_rps.c | 11 +++++++----
> >  1 file changed, 7 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> > index 4dcfae16a7ce..07321e1b22f6 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> > @@ -662,14 +662,17 @@ static int gen6_rps_set(struct intel_rps *rps, u8 val)
> >         struct drm_i915_private *i915 = rps_to_i915(rps);
> >         u32 swreq;
> >  
> > -       if (INTEL_GEN(i915) >= 9)
> > -               swreq = GEN9_FREQUENCY(val);
> > -       else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
> > +       if (INTEL_GEN(i915) >= 9) {
> > +               swreq = 0x2; /* only throttle slice, not unslice */
> 
> 0x0 == use implicit slice ratio
> 0x1 == use explicit slice ratio
> 0x2 == use separate throttling
> 
> Not sure if 0x2 actually was implemented in the end.

That being said, 0x2 seems to be doing better.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/gt: Make the slice:unslice ratio request explicit for RPS

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Chris Wilson (2020-04-21 14:45:12)
>> In RPS, we have the option to only specify the unslice [ring] clock
>> ratio and for the pcu to derive the slice [gpu] clock ratio from its
>> magic table. We also have the option to tell the pcu to use our
>> requested gpu clock ratio, and for it to try and throttle the unslice
>> and slice ratios separately.
>> 
>> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/gt/intel_rps.c | 11 +++++++----
>>  1 file changed, 7 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
>> index 4dcfae16a7ce..07321e1b22f6 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
>> @@ -662,14 +662,17 @@ static int gen6_rps_set(struct intel_rps *rps, u8 val)
>>         struct drm_i915_private *i915 = rps_to_i915(rps);
>>         u32 swreq;
>>  
>> -       if (INTEL_GEN(i915) >= 9)
>> -               swreq = GEN9_FREQUENCY(val);
>> -       else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
>> +       if (INTEL_GEN(i915) >= 9) {
>> +               swreq = 0x2; /* only throttle slice, not unslice */
>
> 0x0 == use implicit slice ratio
> 0x1 == use explicit slice ratio
> 0x2 == use separate throttling

Care to enum/define these and add as parameter to GEN9_FREQUENCY?

Also if there is any bspec link, add a reference.

Thanks,
-Mika

>
> Not sure if 0x2 actually was implemented in the end.
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/gt: Make the slice:unslice ratio request explicit for RPS

[Chris Wilson]

Quoting Mika Kuoppala (2020-04-21 14:56:46)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Chris Wilson (2020-04-21 14:45:12)
> >> In RPS, we have the option to only specify the unslice [ring] clock
> >> ratio and for the pcu to derive the slice [gpu] clock ratio from its
> >> magic table. We also have the option to tell the pcu to use our
> >> requested gpu clock ratio, and for it to try and throttle the unslice
> >> and slice ratios separately.
> >> 
> >> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/gt/intel_rps.c | 11 +++++++----
> >>  1 file changed, 7 insertions(+), 4 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> >> index 4dcfae16a7ce..07321e1b22f6 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> >> @@ -662,14 +662,17 @@ static int gen6_rps_set(struct intel_rps *rps, u8 val)
> >>         struct drm_i915_private *i915 = rps_to_i915(rps);
> >>         u32 swreq;
> >>  
> >> -       if (INTEL_GEN(i915) >= 9)
> >> -               swreq = GEN9_FREQUENCY(val);
> >> -       else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
> >> +       if (INTEL_GEN(i915) >= 9) {
> >> +               swreq = 0x2; /* only throttle slice, not unslice */
> >
> > 0x0 == use implicit slice ratio
> > 0x1 == use explicit slice ratio
> > 0x2 == use separate throttling
> 
> Care to enum/define these and add as parameter to GEN9_FREQUENCY?

It would not be a parameter to GEN9_FREQUENCY as that gets used
elsewhere. You know my opinion on single use magic macros, only useful
for obfuscating code.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gt: Make the slice:unslice ratio request explicit for RPS

[Patchwork]

== Series Details ==

Series: drm/i915/gt: Make the slice:unslice ratio request explicit for RPS
URL   : https://patchwork.freedesktop.org/series/76269/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_8343 -> Patchwork_17406
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17406/index.html

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@requests:
    - fi-byt-j1900:       [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8343/fi-byt-j1900/igt@i915_selftest@live@requests.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17406/fi-byt-j1900/igt@i915_selftest@live@requests.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_selftest@live@gt_timelines:
    - {fi-tgl-u}:         [PASS][3] -> [INCOMPLETE][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8343/fi-tgl-u/igt@i915_selftest@live@gt_timelines.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17406/fi-tgl-u/igt@i915_selftest@live@gt_timelines.html

  
Known issues
------------

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

### IGT changes ###

#### Possible fixes ####

  * igt@i915_selftest@live@gt_pm:
    - fi-cfl-8109u:       [DMESG-FAIL][5] ([i915#1751]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8343/fi-cfl-8109u/igt@i915_selftest@live@gt_pm.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17406/fi-cfl-8109u/igt@i915_selftest@live@gt_pm.html
    - fi-apl-guc:         [DMESG-FAIL][7] ([i915#1751]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8343/fi-apl-guc/igt@i915_selftest@live@gt_pm.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17406/fi-apl-guc/igt@i915_selftest@live@gt_pm.html

  * igt@kms_chamelium@dp-edid-read:
    - fi-kbl-7500u:       [FAIL][9] ([i915#976]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8343/fi-kbl-7500u/igt@kms_chamelium@dp-edid-read.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17406/fi-kbl-7500u/igt@kms_chamelium@dp-edid-read.html

  
#### Warnings ####

  * igt@i915_pm_rpm@module-reload:
    - fi-kbl-x1275:       [FAIL][11] ([i915#62]) -> [SKIP][12] ([fdo#109271])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8343/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17406/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live@gt_pm:
    - fi-icl-u2:          [DMESG-FAIL][13] -> [DMESG-FAIL][14] ([i915#1754])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8343/fi-icl-u2/igt@i915_selftest@live@gt_pm.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_17406/fi-icl-u2/igt@i915_selftest@live@gt_pm.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1751]: https://gitlab.freedesktop.org/drm/intel/issues/1751
  [i915#1754]: https://gitlab.freedesktop.org/drm/intel/issues/1754
  [i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
  [i915#976]: https://gitlab.freedesktop.org/drm/intel/issues/976


Participating hosts (48 -> 42)
------------------------------

  Additional (1): fi-bwr-2160 
  Missing    (7): fi-cml-u2 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_8343 -> Patchwork_17406

  CI-20190529: 20190529
  CI_DRM_8343: a5f7098d36b9370b08717c04d894d01c7cb4320b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5602: a8fcccd15dcc2dd409edd23785a2d6f6e85fb682 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_17406: 976b9b93bd0bc34f76f17bd471445674332f4d27 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

976b9b93bd0b drm/i915/gt: Make the slice:unslice ratio request explicit for RPS

== Logs ==

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

Re: [Intel-gfx] [PATCH] drm/i915/gt: Make the slice:unslice ratio request explicit for RPS

[Chris Wilson]

Quoting Chris Wilson (2020-04-21 14:53:54)
> Quoting Chris Wilson (2020-04-21 14:50:51)
> > Quoting Chris Wilson (2020-04-21 14:45:12)
> > > In RPS, we have the option to only specify the unslice [ring] clock
> > > ratio and for the pcu to derive the slice [gpu] clock ratio from its
> > > magic table. We also have the option to tell the pcu to use our
> > > requested gpu clock ratio, and for it to try and throttle the unslice
> > > and slice ratios separately.
> > > 
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gt/intel_rps.c | 11 +++++++----
> > >  1 file changed, 7 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> > > index 4dcfae16a7ce..07321e1b22f6 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> > > @@ -662,14 +662,17 @@ static int gen6_rps_set(struct intel_rps *rps, u8 val)
> > >         struct drm_i915_private *i915 = rps_to_i915(rps);
> > >         u32 swreq;
> > >  
> > > -       if (INTEL_GEN(i915) >= 9)
> > > -               swreq = GEN9_FREQUENCY(val);
> > > -       else if (IS_HASWELL(i915) || IS_BROADWELL(i915))
> > > +       if (INTEL_GEN(i915) >= 9) {
> > > +               swreq = 0x2; /* only throttle slice, not unslice */
> > 
> > 0x0 == use implicit slice ratio
> > 0x1 == use explicit slice ratio
> > 0x2 == use separate throttling
> > 
> > Not sure if 0x2 actually was implemented in the end.
> 
> That being said, 0x2 seems to be doing better.

But only on tgl. icl/kbl have been meh.

So far this looks like confirmation bias.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx