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);
