[PATCH] drm/i915/guc/slpc: Allow SLPC to use efficient frequency
Belgaumkar, Vinay
vinay.belgaumkar at intel.com
Mon Aug 15 23:16:26 UTC 2022
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 <rodrigo.vivi at intel.com>
> First of all sorry for looking to the old patch first... I was delayed in my inbox flow.
>
>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
>> ---
>> 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(&i915->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, &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,
>> - 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 +689,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)) {
>> diff --git a/drivers/gpu/drm/i915/intel_mchbar_regs.h b/drivers/gpu/drm/i915/intel_mchbar_regs.h
>> index 2aad2f0cc8db..ffc702b79579 100644
>> --- a/drivers/gpu/drm/i915/intel_mchbar_regs.h
>> +++ b/drivers/gpu/drm/i915/intel_mchbar_regs.h
>> @@ -196,6 +196,9 @@
>> #define RP1_CAP_MASK REG_GENMASK(15, 8)
>> #define RPN_CAP_MASK REG_GENMASK(23, 16)
>>
>> +#define GEN10_FREQ_INFO_REC _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5ef0)
>> +#define RPE_MASK REG_GENMASK(15, 8)
>> +
>> /* snb MCH registers for priority tuning */
>> #define MCH_SSKPD _MMIO(MCHBAR_MIRROR_BASE_SNB + 0x5d10)
>> #define SSKPD_NEW_WM0_MASK_HSW REG_GENMASK64(63, 56)
>> --
>> 2.35.1
>>
More information about the dri-devel
mailing list