On Tue, Jul 25, 2023 at 06:00:44PM -0700, Vinay Belgaumkar wrote: > This should be done before the soft min/max frequencies are restored. > When we disable the "Ignore efficient frequency" flag, GuC does not > actually bring the requested freq down to RPn. > > Specifically, this scenario- > > - ignore efficient freq set to true > - reduce min to RPn (from efficient) > - suspend > - resume (includes GuC load, restore soft min/max, restore efficient freq) > - validate min freq has been resored to RPn > > This will fail if we didn't first restore(disable, in this case) efficient > freq flag before setting the soft min frequency. > > v2: Bring the min freq down to RPn when we disable efficient freq (Rodrigo) > Also made the change to set the min softlimit to RPn at init. Otherwise, we > were storing RPe there. Thanks Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > Link: https://gitlab.freedesktop.org/drm/intel/-/issues/8736 > Fixes: 55f9720dbf23 ("drm/i915/guc/slpc: Provide sysfs for efficient freq") > Fixes: 95ccf312a1e4 ("drm/i915/guc/slpc: Allow SLPC to use efficient frequency") > Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@xxxxxxxxx> > --- > drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 22 +++++++++++++-------- > 1 file changed, 14 insertions(+), 8 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 ee9f83af7cf6..477df260ae3a 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c > @@ -470,12 +470,19 @@ int intel_guc_slpc_set_ignore_eff_freq(struct intel_guc_slpc *slpc, bool val) > ret = slpc_set_param(slpc, > SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY, > val); > - if (ret) > + if (ret) { > guc_probe_error(slpc_to_guc(slpc), "Failed to set efficient freq(%d): %pe\n", > val, ERR_PTR(ret)); > - else > + } else { > slpc->ignore_eff_freq = val; > > + /* Set min to RPn when we disable efficient freq */ > + if (val) > + ret = slpc_set_param(slpc, > + SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ, > + slpc->min_freq); > + } > + > intel_runtime_pm_put(&i915->runtime_pm, wakeref); > mutex_unlock(&slpc->lock); > return ret; > @@ -602,9 +609,8 @@ static int slpc_set_softlimits(struct intel_guc_slpc *slpc) > return ret; > > if (!slpc->min_freq_softlimit) { > - ret = intel_guc_slpc_get_min_freq(slpc, &slpc->min_freq_softlimit); > - if (unlikely(ret)) > - return ret; > + /* Min softlimit is initialized to RPn */ > + slpc->min_freq_softlimit = slpc->min_freq; > slpc_to_gt(slpc)->defaults.min_freq = slpc->min_freq_softlimit; > } else { > return intel_guc_slpc_set_min_freq(slpc, > @@ -755,6 +761,9 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc) > return ret; > } > > + /* Set cached value of ignore efficient freq */ > + intel_guc_slpc_set_ignore_eff_freq(slpc, slpc->ignore_eff_freq); > + > /* Revert SLPC min/max to softlimits if necessary */ > ret = slpc_set_softlimits(slpc); > if (unlikely(ret)) { > @@ -765,9 +774,6 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc) > /* Set cached media freq ratio mode */ > intel_guc_slpc_set_media_ratio_mode(slpc, slpc->media_ratio_mode); > > - /* Set cached value of ignore efficient freq */ > - intel_guc_slpc_set_ignore_eff_freq(slpc, slpc->ignore_eff_freq); > - > return 0; > } > > -- > 2.38.1 >
