On Fri, 31 Mar 2023 19:00:49 -0700, Vinay Belgaumkar wrote: > Hi Vinay, > @@ -478,20 +507,15 @@ int intel_guc_slpc_set_min_freq(struct intel_guc_slpc *slpc, u32 val) > val > slpc->max_freq_softlimit) > return -EINVAL; > > + /* Ignore efficient freq if lower min freq is requested */ > + ret = intel_guc_slpc_set_ignore_eff_freq(slpc, val < slpc->rp1_freq); > + if (ret) > + goto out; > + I don't agree with this. If we are now providing an interface explicitly to ignore RPe, that should be /only/ way to ignore RPe. There should be no other "under the hood" ignoring of RPe. In other words, ignoring RPe should be minimized unless explicitly requested. I don't clearly understand why this was done previously but it makes even less sense to me now after this patch. Thanks. -- Ashutosh > /* Need a lock now since waitboost can be modifying min as well */ > mutex_lock(&slpc->lock); > wakeref = intel_runtime_pm_get(&i915->runtime_pm); > > - /* Ignore efficient freq if lower min freq is requested */ > - ret = slpc_set_param(slpc, > - SLPC_PARAM_IGNORE_EFFICIENT_FREQUENCY, > - val < slpc->rp1_freq); > - if (ret) { > - guc_probe_error(slpc_to_guc(slpc), "Failed to toggle efficient freq: %pe\n", > - ERR_PTR(ret)); > - goto out; > - } > - > ret = slpc_set_param(slpc, > SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ, > val);