Re: [Intel-gfx] [PATCH] i915/guc/slpc: Provide sysfs for efficient freq

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

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