[Intel-gfx] [RFC] drm/i915/rc6: Access RC6 regs with forcewake

[Matt Roper]

On Fri, Apr 22, 2022 at 07:07:52PM +0530, Badal Nilawar wrote:
> To access RC6 related MMIO regs use intel_uncore_write(), which grabs
> forcewake if reg requires a forcewake.
> 
> Signed-off-by: Badal Nilawar <badal.nilawar at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_rc6.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
> index b4770690e794..9edaad3f19b5 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> @@ -55,7 +55,7 @@ static struct drm_i915_private *rc6_to_i915(struct intel_rc6 *rc)
>  
>  static void set(struct intel_uncore *uncore, i915_reg_t reg, u32 val)
>  {
> -	intel_uncore_write_fw(uncore, reg, val);
> +	intel_uncore_write(uncore, reg, val);

The set() function is primarily used within the *_rc6_enable() functions.
Those are all called via intel_rc6_enable() which has already
grabbed explicit forcewake before calling them:

        intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);

so there's no need to grab an additional reference on every register
access.  Likewise, the call in vlv_residency_raw() doesn't need
forcewake because its own caller (intel_rc6_residency_ns) has already
acquired it.

I think the call in intel_rc6_unpark() is the only one that might be
questionable from a very quick skim of the code.  So rather than
needlessly grabbing forcewake in all the other spots, it's probably
better to just replace that single call with a direct call to
intel_uncore_write() if it actually is problematic.

Even better, we might just want to drop the set() wrapper completely and
replace all instances with the appropriate intel_uncore_write[_fw]
calls; I don't really see the slightly shorter lines the wrapper gives
us as being worth the deviation from the rest of the driver's code
(especially if it's causing us confusion about what the intendended
forcewake semantics are).


Matt

>  }
>  
>  static void gen11_rc6_enable(struct intel_rc6 *rc6)
> -- 
> 2.25.1
> 

-- 
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
(916) 356-2795