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

[Rodrigo Vivi]

On Fri, Jul 21, 2023 at 01:44:34PM -0700, Belgaumkar, Vinay wrote:
> 
> On 7/21/2023 1:41 PM, Rodrigo Vivi wrote:
> > 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 at 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.)
> 
> The soft min does not get messed up, but GuC keeps requesting RPe even after
> disabling efficient freq. (unless we manually set min freq to RPn AFTER
> disabling efficient).

so it looks to me that the right solution would be to ensure that everytime
that we disable the efficient freq we make sure to also set the mim freq to RPn,
no?!

> 
> Thanks,
> 
> Vinay.
> 
> > 
> > 
> > > 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 at 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
> > >