Re: [Intel-gfx] [PATCH] drm/i915/guc/slpc: Restore efficient freq earlier

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Jul 21, 2023 at 11:03:49AM -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.

that's strange. so guc is returning the rpe when we request the min freq
during the soft config?

we could alternatively change the soft config to actually get the min
and not be tricked by this.

But also the patch below doesn't hurt.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
(Although I'm still curious and want to understand exactly why
the soft min gets messed up when we don't tell guc to ignore the
efficient freq beforehand. Please help me to understand.)


> 
> Link: https://gitlab.freedesktop.org/drm/intel/-/issues/8736
> Fixes: 55f9720dbf23 ("drm/i915/guc/slpc: Provide sysfs for efficient freq")
> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 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..f16dff7c3185 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c
> @@ -743,6 +743,9 @@ int intel_guc_slpc_enable(struct intel_guc_slpc *slpc)
>  
>  	intel_guc_pm_intrmsk_enable(slpc_to_gt(slpc));
>  
> +	/* Set cached value of ignore efficient freq */
> +	intel_guc_slpc_set_ignore_eff_freq(slpc, slpc->ignore_eff_freq);
> +
>  	slpc_get_rp_values(slpc);
>  
>  	/* Handle the case where min=max=RPmax */
> @@ -765,9 +768,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
>