Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Allow SLPC to use efficient frequency
On Sun, 14 Aug 2022 16:46:54 -0700, Vinay Belgaumkar wrote: > > Host Turbo operates at efficient frequency when GT is not idle unless > the user or workload has forced it to a higher level. Replicate the same > behavior in SLPC by allowing the algorithm to use efficient frequency. > We had disabled it during boot due to concerns that it might break > kernel ABI for min frequency. However, this is not the case since > SLPC will still abide by the (min,max) range limits. This change seems to have broken the i915 kernel ABI for min frequency for DG2. Tvrtko pointed this out here: https://patchwork.freedesktop.org/patch/512274/?series=110574=3 These bugs are the result of that ABI break: Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/6806 Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/6786 On DG2 when we set min == max freq, we see the GPU running not at the set min == max freq but at efficient freq (different from the set freq). We are still trying to see if the ABI can be salvaged but something is definitely wrong at present. Thanks. -- Ashutosh
Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Allow SLPC to use efficient frequency
On Sun, Aug 14, 2022 at 04:46:54PM -0700, Vinay Belgaumkar wrote:
> Host Turbo operates at efficient frequency when GT is not idle unless
> the user or workload has forced it to a higher level. Replicate the same
> behavior in SLPC by allowing the algorithm to use efficient frequency.
> We had disabled it during boot due to concerns that it might break
> kernel ABI for min frequency. However, this is not the case since
> SLPC will still abide by the (min,max) range limits.
>
> With this change, min freq will be at efficient frequency level at init
> instead of fused min (RPn). If user chooses to reduce min freq below the
> efficient freq, we will turn off usage of efficient frequency and honor
> the user request. When a higher value is written, it will get toggled
> back again.
>
> The patch also corrects the register which needs to be read for obtaining
> the correct efficient frequency for Gen9+.
>
> We see much better perf numbers with benchmarks like glmark2 with
> efficient frequency usage enabled as expected.
>
> BugLink: https://gitlab.freedesktop.org/drm/intel/-/issues/5468
>
> Cc: Rodrigo Vivi
First of all sorry for looking to the old patch first... I was delayed in my
inbox flow.
> Signed-off-by: Vinay Belgaumkar
> ---
> drivers/gpu/drm/i915/gt/intel_rps.c | 3 +
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 66 +++--
> drivers/gpu/drm/i915/intel_mchbar_regs.h| 3 +
> 3 files changed, 40 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
> b/drivers/gpu/drm/i915/gt/intel_rps.c
> index c7d381ad90cf..281a086fc265 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -1108,6 +1108,9 @@ void gen6_rps_get_freq_caps(struct intel_rps *rps,
> struct intel_rps_freq_caps *c
> } else {
> caps->rp0_freq = (rp_state_cap >> 0) & 0xff;
> caps->rp1_freq = (rp_state_cap >> 8) & 0xff;
> + caps->rp1_freq = REG_FIELD_GET(RPE_MASK,
> +
> intel_uncore_read(to_gt(i915)->uncore,
> +GEN10_FREQ_INFO_REC));
This register is only gen10+ while the func is gen6+.
either we handle the platform properly or we create a new rpe_freq tracker
somewhere
and if that's available we use this rpe, otherwise we use the hw fused rp1
which is a good
enough, but it is not the actual one resolved by pcode, like this new RPe one.
> caps->min_freq = (rp_state_cap >> 16) & 0xff;
> }
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> index e1fa1f32f29e..70a2af5f518d 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -465,6 +465,29 @@ int intel_guc_slpc_get_max_freq(struct intel_guc_slpc
> *slpc, u32 *val)
> return ret;
> }
>
> +static int slpc_ignore_eff_freq(struct intel_guc_slpc *slpc, bool ignore)
I know this code was already there, but I do have some questions around this
and maybe we can simplify now that are touching this function.
> +{
> + int ret = 0;
> +
> + if (ignore) {
> + ret = slpc_set_param(slpc,
> + SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
> + ignore);
> + if (!ret)
> + return slpc_set_param(slpc,
> +
> SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
> + slpc->min_freq);
why do we need to touch this min request here?
> + } else {
> + ret = slpc_unset_param(slpc,
> +SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY);
do we really need the unset param?
for me using set_param(SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY, freq < rpe_freq)
was enough...
> + if (!ret)
> + return slpc_unset_param(slpc,
> +
> SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ);
> + }
> +
> + return ret;
> +}
> +
> /**
> * intel_guc_slpc_set_min_freq() - Set min frequency limit for SLPC.
> * @slpc: pointer to intel_guc_slpc.
> @@ -491,6 +514,14 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc
> *slpc, u32 val)
>
> with_intel_runtime_pm(>runtime_pm, wakeref) {
>
> + /* Ignore efficient freq if lower min freq is requested */
> + ret = slpc_ignore_eff_freq(slpc, val < slpc->rp1_freq);
> + if (unlikely(ret)) {
> + i915_probe_error(i915, "Failed to toggle efficient freq
> (%pe)\n",
> + ERR_PTR(ret));
> + return ret;
> + }
> +
> ret = slpc_set_param(slpc,
>SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
>val);
>
Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Allow SLPC to use efficient frequency
On Tue, Aug 09, 2022 at 05:03:06PM -0700, Vinay Belgaumkar wrote:
> Host Turbo operates at efficient frequency when GT is not idle unless
> the user or workload has forced it to a higher level. Replicate the same
> behavior in SLPC by allowing the algorithm to use efficient frequency.
> We had disabled it during boot due to concerns that it might break
> kernel ABI for min frequency. However, this is not the case, since
> SLPC will still abide by the (min,max) range limits and pcode forces
> frequency to 0 anyways when GT is in C6.
>
> We also see much better perf numbers with benchmarks like glmark2 with
> efficient frequency usage enabled.
>
> Fixes: 025cb07bebfa ("drm/i915/guc/slpc: Cache platform frequency limits")
>
> Signed-off-by: Vinay Belgaumkar
I'm honestly surprised that our CI passed cleanly. What happens when user
request both min and max < rpe?
I'm sure that in this case GuC SLPC will put us to rpe ignoring our requests.
Or is this good enough for the users expectation because of the soft limits
showing the requested freq and we not asking to guc what it currently has as
minimal?
I just want to be sure that we are not causing any confusion for end users
out there in the case they request some min/max below RPe and start seeing
mismatches on the expectation because GuC is forcing the real min request
to RPe.
My suggestion is to ignore the RPe whenever we have a min request below it.
So GuC respects our (and users) chosen min. And restore whenever min request
is abobe rpe.
Thanks,
Rodrigo.
> ---
> drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 52 -
> 1 file changed, 52 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> index e1fa1f32f29e..4b824da3048a 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -137,17 +137,6 @@ static int guc_action_slpc_set_param(struct intel_guc
> *guc, u8 id, u32 value)
> return ret > 0 ? -EPROTO : ret;
> }
>
> -static int guc_action_slpc_unset_param(struct intel_guc *guc, u8 id)
> -{
> - u32 request[] = {
> - GUC_ACTION_HOST2GUC_PC_SLPC_REQUEST,
> - SLPC_EVENT(SLPC_EVENT_PARAMETER_UNSET, 1),
> - id,
> - };
> -
> - return intel_guc_send(guc, request, ARRAY_SIZE(request));
> -}
> -
> static bool slpc_is_running(struct intel_guc_slpc *slpc)
> {
> return slpc_get_state(slpc) == SLPC_GLOBAL_STATE_RUNNING;
> @@ -201,16 +190,6 @@ static int slpc_set_param(struct intel_guc_slpc *slpc,
> u8 id, u32 value)
> return ret;
> }
>
> -static int slpc_unset_param(struct intel_guc_slpc *slpc,
> - u8 id)
> -{
> - struct intel_guc *guc = slpc_to_guc(slpc);
> -
> - GEM_BUG_ON(id >= SLPC_MAX_PARAM);
> -
> - return guc_action_slpc_unset_param(guc, id);
> -}
> -
> static int slpc_force_min_freq(struct intel_guc_slpc *slpc, u32 freq)
> {
> struct drm_i915_private *i915 = slpc_to_i915(slpc);
> @@ -597,29 +576,6 @@ static int slpc_set_softlimits(struct intel_guc_slpc
> *slpc)
> return 0;
> }
>
> -static int slpc_ignore_eff_freq(struct intel_guc_slpc *slpc, bool ignore)
> -{
> - int ret = 0;
> -
> - if (ignore) {
> - ret = slpc_set_param(slpc,
> - SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
> - ignore);
> - if (!ret)
> - return slpc_set_param(slpc,
> -
> SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
> - slpc->min_freq);
> - } else {
> - ret = slpc_unset_param(slpc,
> -SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY);
> - if (!ret)
> - return slpc_unset_param(slpc,
> -
> SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ);
> - }
> -
> - return ret;
> -}
> -
> static int slpc_use_fused_rp0(struct intel_guc_slpc *slpc)
> {
> /* Force SLPC to used platform rp0 */
> @@ -679,14 +635,6 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
>
> slpc_get_rp_values(slpc);
>
> - /* Ignore efficient freq and set min to platform min */
> - ret = slpc_ignore_eff_freq(slpc, true);
> - if (unlikely(ret)) {
> - i915_probe_error(i915, "Failed to set SLPC min to RPn (%pe)\n",
> - ERR_PTR(ret));
> - return ret;
> - }
> -
> /* Set SLPC max limit to RP0 */
> ret = slpc_use_fused_rp0(slpc);
> if (unlikely(ret)) {
> --
> 2.35.1
>
[Intel-gfx] [PATCH] drm/i915/guc/slpc: Allow SLPC to use efficient frequency
Host Turbo operates at efficient frequency when GT is not idle unless
the user or workload has forced it to a higher level. Replicate the same
behavior in SLPC by allowing the algorithm to use efficient frequency.
We had disabled it during boot due to concerns that it might break
kernel ABI for min frequency. However, this is not the case since
SLPC will still abide by the (min,max) range limits.
With this change, min freq will be at efficient frequency level at init
instead of fused min (RPn). If user chooses to reduce min freq below the
efficient freq, we will turn off usage of efficient frequency and honor
the user request. When a higher value is written, it will get toggled
back again.
The patch also corrects the register which needs to be read for obtaining
the correct efficient frequency for Gen9+.
We see much better perf numbers with benchmarks like glmark2 with
efficient frequency usage enabled as expected.
BugLink: https://gitlab.freedesktop.org/drm/intel/-/issues/5468
Cc: Rodrigo Vivi
Signed-off-by: Vinay Belgaumkar
---
drivers/gpu/drm/i915/gt/intel_rps.c | 3 +
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 66 +++--
drivers/gpu/drm/i915/intel_mchbar_regs.h| 3 +
3 files changed, 40 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
b/drivers/gpu/drm/i915/gt/intel_rps.c
index c7d381ad90cf..281a086fc265 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -1108,6 +1108,9 @@ void gen6_rps_get_freq_caps(struct intel_rps *rps, struct
intel_rps_freq_caps *c
} else {
caps->rp0_freq = (rp_state_cap >> 0) & 0xff;
caps->rp1_freq = (rp_state_cap >> 8) & 0xff;
+ caps->rp1_freq = REG_FIELD_GET(RPE_MASK,
+
intel_uncore_read(to_gt(i915)->uncore,
+ GEN10_FREQ_INFO_REC));
caps->min_freq = (rp_state_cap >> 16) & 0xff;
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index e1fa1f32f29e..70a2af5f518d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -465,6 +465,29 @@ int intel_guc_slpc_get_max_freq(struct intel_guc_slpc
*slpc, u32 *val)
return ret;
}
+static int slpc_ignore_eff_freq(struct intel_guc_slpc *slpc, bool ignore)
+{
+ int ret = 0;
+
+ if (ignore) {
+ ret = slpc_set_param(slpc,
+SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
+ignore);
+ if (!ret)
+ return slpc_set_param(slpc,
+
SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
+ slpc->min_freq);
+ } else {
+ ret = slpc_unset_param(slpc,
+ SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY);
+ if (!ret)
+ return slpc_unset_param(slpc,
+
SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ);
+ }
+
+ return ret;
+}
+
/**
* intel_guc_slpc_set_min_freq() - Set min frequency limit for SLPC.
* @slpc: pointer to intel_guc_slpc.
@@ -491,6 +514,14 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc
*slpc, u32 val)
with_intel_runtime_pm(>runtime_pm, wakeref) {
+ /* Ignore efficient freq if lower min freq is requested */
+ ret = slpc_ignore_eff_freq(slpc, val < slpc->rp1_freq);
+ if (unlikely(ret)) {
+ i915_probe_error(i915, "Failed to toggle efficient freq
(%pe)\n",
+ERR_PTR(ret));
+ return ret;
+ }
+
ret = slpc_set_param(slpc,
SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
val);
@@ -587,7 +618,9 @@ static int slpc_set_softlimits(struct intel_guc_slpc *slpc)
return ret;
if (!slpc->min_freq_softlimit) {
- slpc->min_freq_softlimit = slpc->min_freq;
+ ret = intel_guc_slpc_get_min_freq(slpc,
>min_freq_softlimit);
+ if (unlikely(ret))
+ return ret;
slpc_to_gt(slpc)->defaults.min_freq = slpc->min_freq_softlimit;
} else if (slpc->min_freq_softlimit != slpc->min_freq) {
return intel_guc_slpc_set_min_freq(slpc,
@@ -597,29 +630,6 @@ static int slpc_set_softlimits(struct intel_guc_slpc *slpc)
return 0;
}
-static int slpc_ignore_eff_freq(struct intel_guc_slpc *slpc, bool ignore)
-{
- int ret = 0;
-
- if (ignore) {
- ret = slpc_set_param(slpc,
-SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
-
Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Allow SLPC to use efficient frequency
On 8/15/2022 10:32 AM, Rodrigo Vivi wrote:
On Sun, Aug 14, 2022 at 04:46:54PM -0700, Vinay Belgaumkar wrote:
Host Turbo operates at efficient frequency when GT is not idle unless
the user or workload has forced it to a higher level. Replicate the same
behavior in SLPC by allowing the algorithm to use efficient frequency.
We had disabled it during boot due to concerns that it might break
kernel ABI for min frequency. However, this is not the case since
SLPC will still abide by the (min,max) range limits.
With this change, min freq will be at efficient frequency level at init
instead of fused min (RPn). If user chooses to reduce min freq below the
efficient freq, we will turn off usage of efficient frequency and honor
the user request. When a higher value is written, it will get toggled
back again.
The patch also corrects the register which needs to be read for obtaining
the correct efficient frequency for Gen9+.
We see much better perf numbers with benchmarks like glmark2 with
efficient frequency usage enabled as expected.
BugLink: https://gitlab.freedesktop.org/drm/intel/-/issues/5468
Cc: Rodrigo Vivi
First of all sorry for looking to the old patch first... I was delayed in my
inbox flow.
Signed-off-by: Vinay Belgaumkar
---
drivers/gpu/drm/i915/gt/intel_rps.c | 3 +
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 66 +++--
drivers/gpu/drm/i915/intel_mchbar_regs.h| 3 +
3 files changed, 40 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
b/drivers/gpu/drm/i915/gt/intel_rps.c
index c7d381ad90cf..281a086fc265 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -1108,6 +1108,9 @@ void gen6_rps_get_freq_caps(struct intel_rps *rps, struct
intel_rps_freq_caps *c
} else {
caps->rp0_freq = (rp_state_cap >> 0) & 0xff;
caps->rp1_freq = (rp_state_cap >> 8) & 0xff;
+ caps->rp1_freq = REG_FIELD_GET(RPE_MASK,
+
intel_uncore_read(to_gt(i915)->uncore,
+ GEN10_FREQ_INFO_REC));
This register is only gen10+ while the func is gen6+.
either we handle the platform properly or we create a new rpe_freq tracker
somewhere
and if that's available we use this rpe, otherwise we use the hw fused rp1
which is a good
enough, but it is not the actual one resolved by pcode, like this new RPe one.
sure.
caps->min_freq = (rp_state_cap >> 16) & 0xff;
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index e1fa1f32f29e..70a2af5f518d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -465,6 +465,29 @@ int intel_guc_slpc_get_max_freq(struct intel_guc_slpc
*slpc, u32 *val)
return ret;
}
+static int slpc_ignore_eff_freq(struct intel_guc_slpc *slpc, bool ignore)
I know this code was already there, but I do have some questions around this
and maybe we can simplify now that are touching this function.
+{
+ int ret = 0;
+
+ if (ignore) {
+ ret = slpc_set_param(slpc,
+SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
+ignore);
+ if (!ret)
+ return slpc_set_param(slpc,
+
SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
+ slpc->min_freq);
why do we need to touch this min request here?
true, not needed anymore.
+ } else {
+ ret = slpc_unset_param(slpc,
+ SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY);
do we really need the unset param?
for me using set_param(SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY, freq < rpe_freq)
was enough...
Yup, removed this helper function as discussed.
Thanks,
Vinay.
+ if (!ret)
+ return slpc_unset_param(slpc,
+
SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ);
+ }
+
+ return ret;
+}
+
/**
* intel_guc_slpc_set_min_freq() - Set min frequency limit for SLPC.
* @slpc: pointer to intel_guc_slpc.
@@ -491,6 +514,14 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc
*slpc, u32 val)
with_intel_runtime_pm(>runtime_pm, wakeref) {
+ /* Ignore efficient freq if lower min freq is requested */
+ ret = slpc_ignore_eff_freq(slpc, val < slpc->rp1_freq);
+ if (unlikely(ret)) {
+ i915_probe_error(i915, "Failed to toggle efficient freq
(%pe)\n",
+ERR_PTR(ret));
+ return ret;
+ }
+
ret = slpc_set_param(slpc,
SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Allow SLPC to use efficient frequency
On 8/15/2022 9:51 AM, Rodrigo Vivi wrote:
On Tue, Aug 09, 2022 at 05:03:06PM -0700, Vinay Belgaumkar wrote:
Host Turbo operates at efficient frequency when GT is not idle unless
the user or workload has forced it to a higher level. Replicate the same
behavior in SLPC by allowing the algorithm to use efficient frequency.
We had disabled it during boot due to concerns that it might break
kernel ABI for min frequency. However, this is not the case, since
SLPC will still abide by the (min,max) range limits and pcode forces
frequency to 0 anyways when GT is in C6.
We also see much better perf numbers with benchmarks like glmark2 with
efficient frequency usage enabled.
Fixes: 025cb07bebfa ("drm/i915/guc/slpc: Cache platform frequency limits")
Signed-off-by: Vinay Belgaumkar
I'm honestly surprised that our CI passed cleanly. What happens when user
request both min and max < rpe?
I'm sure that in this case GuC SLPC will put us to rpe ignoring our requests.
Or is this good enough for the users expectation because of the soft limits
showing the requested freq and we not asking to guc what it currently has as
minimal?
I just want to be sure that we are not causing any confusion for end users
out there in the case they request some min/max below RPe and start seeing
mismatches on the expectation because GuC is forcing the real min request
to RPe.
My suggestion is to ignore the RPe whenever we have a min request below it.
So GuC respects our (and users) chosen min. And restore whenever min request
is abobe rpe.
Yup, I have already sent a patch yesterday with that change, doesn't
look like CI has run on it yet. This was the old version.
Thanks,
Vinay.
Thanks,
Rodrigo.
---
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 52 -
1 file changed, 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index e1fa1f32f29e..4b824da3048a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -137,17 +137,6 @@ static int guc_action_slpc_set_param(struct intel_guc
*guc, u8 id, u32 value)
return ret > 0 ? -EPROTO : ret;
}
-static int guc_action_slpc_unset_param(struct intel_guc *guc, u8 id)
-{
- u32 request[] = {
- GUC_ACTION_HOST2GUC_PC_SLPC_REQUEST,
- SLPC_EVENT(SLPC_EVENT_PARAMETER_UNSET, 1),
- id,
- };
-
- return intel_guc_send(guc, request, ARRAY_SIZE(request));
-}
-
static bool slpc_is_running(struct intel_guc_slpc *slpc)
{
return slpc_get_state(slpc) == SLPC_GLOBAL_STATE_RUNNING;
@@ -201,16 +190,6 @@ static int slpc_set_param(struct intel_guc_slpc *slpc, u8
id, u32 value)
return ret;
}
-static int slpc_unset_param(struct intel_guc_slpc *slpc,
- u8 id)
-{
- struct intel_guc *guc = slpc_to_guc(slpc);
-
- GEM_BUG_ON(id >= SLPC_MAX_PARAM);
-
- return guc_action_slpc_unset_param(guc, id);
-}
-
static int slpc_force_min_freq(struct intel_guc_slpc *slpc, u32 freq)
{
struct drm_i915_private *i915 = slpc_to_i915(slpc);
@@ -597,29 +576,6 @@ static int slpc_set_softlimits(struct intel_guc_slpc *slpc)
return 0;
}
-static int slpc_ignore_eff_freq(struct intel_guc_slpc *slpc, bool ignore)
-{
- int ret = 0;
-
- if (ignore) {
- ret = slpc_set_param(slpc,
-SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
-ignore);
- if (!ret)
- return slpc_set_param(slpc,
-
SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
- slpc->min_freq);
- } else {
- ret = slpc_unset_param(slpc,
- SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY);
- if (!ret)
- return slpc_unset_param(slpc,
-
SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ);
- }
-
- return ret;
-}
-
static int slpc_use_fused_rp0(struct intel_guc_slpc *slpc)
{
/* Force SLPC to used platform rp0 */
@@ -679,14 +635,6 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
slpc_get_rp_values(slpc);
- /* Ignore efficient freq and set min to platform min */
- ret = slpc_ignore_eff_freq(slpc, true);
- if (unlikely(ret)) {
- i915_probe_error(i915, "Failed to set SLPC min to RPn (%pe)\n",
-ERR_PTR(ret));
- return ret;
- }
-
/* Set SLPC max limit to RP0 */
ret = slpc_use_fused_rp0(slpc);
if (unlikely(ret)) {
--
2.35.1
Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Allow SLPC to use efficient frequency
On 8/14/2022 4:46 PM, Vinay Belgaumkar wrote:
Host Turbo operates at efficient frequency when GT is not idle unless
the user or workload has forced it to a higher level. Replicate the same
behavior in SLPC by allowing the algorithm to use efficient frequency.
We had disabled it during boot due to concerns that it might break
kernel ABI for min frequency. However, this is not the case since
SLPC will still abide by the (min,max) range limits.
With this change, min freq will be at efficient frequency level at init
instead of fused min (RPn). If user chooses to reduce min freq below the
efficient freq, we will turn off usage of efficient frequency and honor
the user request. When a higher value is written, it will get toggled
back again.
The patch also corrects the register which needs to be read for obtaining
the correct efficient frequency for Gen9+.
We see much better perf numbers with benchmarks like glmark2 with
efficient frequency usage enabled as expected.
BugLink: https://gitlab.freedesktop.org/drm/intel/-/issues/5468
Cc: Rodrigo Vivi
Signed-off-by: Vinay Belgaumkar
---
drivers/gpu/drm/i915/gt/intel_rps.c | 3 +
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 66 +++--
drivers/gpu/drm/i915/intel_mchbar_regs.h| 3 +
3 files changed, 40 insertions(+), 32 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c
b/drivers/gpu/drm/i915/gt/intel_rps.c
index c7d381ad90cf..281a086fc265 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -1108,6 +1108,9 @@ void gen6_rps_get_freq_caps(struct intel_rps *rps, struct
intel_rps_freq_caps *c
} else {
caps->rp0_freq = (rp_state_cap >> 0) & 0xff;
caps->rp1_freq = (rp_state_cap >> 8) & 0xff;
Forgot to remove old code here. Will do so for the next revision as it
does not affect the patch.
Thanks,
Vinay.
+ caps->rp1_freq = REG_FIELD_GET(RPE_MASK,
+
intel_uncore_read(to_gt(i915)->uncore,
+ GEN10_FREQ_INFO_REC));
caps->min_freq = (rp_state_cap >> 16) & 0xff;
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index e1fa1f32f29e..70a2af5f518d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -465,6 +465,29 @@ int intel_guc_slpc_get_max_freq(struct intel_guc_slpc
*slpc, u32 *val)
return ret;
}
+static int slpc_ignore_eff_freq(struct intel_guc_slpc *slpc, bool ignore)
+{
+ int ret = 0;
+
+ if (ignore) {
+ ret = slpc_set_param(slpc,
+SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
+ignore);
+ if (!ret)
+ return slpc_set_param(slpc,
+
SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
+ slpc->min_freq);
+ } else {
+ ret = slpc_unset_param(slpc,
+ SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY);
+ if (!ret)
+ return slpc_unset_param(slpc,
+
SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ);
+ }
+
+ return ret;
+}
+
/**
* intel_guc_slpc_set_min_freq() - Set min frequency limit for SLPC.
* @slpc: pointer to intel_guc_slpc.
@@ -491,6 +514,14 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc
*slpc, u32 val)
with_intel_runtime_pm(>runtime_pm, wakeref) {
+ /* Ignore efficient freq if lower min freq is requested */
+ ret = slpc_ignore_eff_freq(slpc, val < slpc->rp1_freq);
+ if (unlikely(ret)) {
+ i915_probe_error(i915, "Failed to toggle efficient freq
(%pe)\n",
+ERR_PTR(ret));
+ return ret;
+ }
+
ret = slpc_set_param(slpc,
SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
val);
@@ -587,7 +618,9 @@ static int slpc_set_softlimits(struct intel_guc_slpc *slpc)
return ret;
if (!slpc->min_freq_softlimit) {
- slpc->min_freq_softlimit = slpc->min_freq;
+ ret = intel_guc_slpc_get_min_freq(slpc,
>min_freq_softlimit);
+ if (unlikely(ret))
+ return ret;
slpc_to_gt(slpc)->defaults.min_freq = slpc->min_freq_softlimit;
} else if (slpc->min_freq_softlimit != slpc->min_freq) {
return intel_guc_slpc_set_min_freq(slpc,
@@ -597,29 +630,6 @@ static int slpc_set_softlimits(struct intel_guc_slpc *slpc)
return 0;
}
-static int slpc_ignore_eff_freq(struct intel_guc_slpc *slpc, bool ignore)
-{
- int
[Intel-gfx] [PATCH] drm/i915/guc/slpc: Allow SLPC to use efficient frequency
Host Turbo operates at efficient frequency when GT is not idle unless
the user or workload has forced it to a higher level. Replicate the same
behavior in SLPC by allowing the algorithm to use efficient frequency.
We had disabled it during boot due to concerns that it might break
kernel ABI for min frequency. However, this is not the case, since
SLPC will still abide by the (min,max) range limits and pcode forces
frequency to 0 anyways when GT is in C6.
We also see much better perf numbers with benchmarks like glmark2 with
efficient frequency usage enabled.
Fixes: 025cb07bebfa ("drm/i915/guc/slpc: Cache platform frequency limits")
Signed-off-by: Vinay Belgaumkar
---
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 52 -
1 file changed, 52 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
index e1fa1f32f29e..4b824da3048a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
@@ -137,17 +137,6 @@ static int guc_action_slpc_set_param(struct intel_guc
*guc, u8 id, u32 value)
return ret > 0 ? -EPROTO : ret;
}
-static int guc_action_slpc_unset_param(struct intel_guc *guc, u8 id)
-{
- u32 request[] = {
- GUC_ACTION_HOST2GUC_PC_SLPC_REQUEST,
- SLPC_EVENT(SLPC_EVENT_PARAMETER_UNSET, 1),
- id,
- };
-
- return intel_guc_send(guc, request, ARRAY_SIZE(request));
-}
-
static bool slpc_is_running(struct intel_guc_slpc *slpc)
{
return slpc_get_state(slpc) == SLPC_GLOBAL_STATE_RUNNING;
@@ -201,16 +190,6 @@ static int slpc_set_param(struct intel_guc_slpc *slpc, u8
id, u32 value)
return ret;
}
-static int slpc_unset_param(struct intel_guc_slpc *slpc,
- u8 id)
-{
- struct intel_guc *guc = slpc_to_guc(slpc);
-
- GEM_BUG_ON(id >= SLPC_MAX_PARAM);
-
- return guc_action_slpc_unset_param(guc, id);
-}
-
static int slpc_force_min_freq(struct intel_guc_slpc *slpc, u32 freq)
{
struct drm_i915_private *i915 = slpc_to_i915(slpc);
@@ -597,29 +576,6 @@ static int slpc_set_softlimits(struct intel_guc_slpc *slpc)
return 0;
}
-static int slpc_ignore_eff_freq(struct intel_guc_slpc *slpc, bool ignore)
-{
- int ret = 0;
-
- if (ignore) {
- ret = slpc_set_param(slpc,
-SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY,
-ignore);
- if (!ret)
- return slpc_set_param(slpc,
-
SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ,
- slpc->min_freq);
- } else {
- ret = slpc_unset_param(slpc,
- SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY);
- if (!ret)
- return slpc_unset_param(slpc,
-
SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ);
- }
-
- return ret;
-}
-
static int slpc_use_fused_rp0(struct intel_guc_slpc *slpc)
{
/* Force SLPC to used platform rp0 */
@@ -679,14 +635,6 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
slpc_get_rp_values(slpc);
- /* Ignore efficient freq and set min to platform min */
- ret = slpc_ignore_eff_freq(slpc, true);
- if (unlikely(ret)) {
- i915_probe_error(i915, "Failed to set SLPC min to RPn (%pe)\n",
-ERR_PTR(ret));
- return ret;
- }
-
/* Set SLPC max limit to RP0 */
ret = slpc_use_fused_rp0(slpc);
if (unlikely(ret)) {
--
2.35.1
