[PATCH] drm/i915/rc6: Disable RPG during workload execution

[Andi Shyti]

Hi Badal,

On Tue, Oct 22, 2024 at 06:52:26PM +0530, Badal Nilawar wrote:
> Encountering forcewake errors related to render power gating;

Can you please expand your explanation here?

> therefore, disable it during workload execution.

... and here.

> Cc: Chris Wilson <chris.p.wilson at linux.intel.com>
> Signed-off-by: Badal Nilawar <badal.nilawar at intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_rc6.c       | 18 +++++++++++++++++-
>  drivers/gpu/drm/i915/gt/intel_rc6_types.h |  1 +
>  2 files changed, 18 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_rc6.c b/drivers/gpu/drm/i915/gt/intel_rc6.c
> index c864d101faf9..459394ab5258 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rc6.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rc6.c
> @@ -140,6 +140,7 @@ static void gen11_rc6_enable(struct intel_rc6 *rc6)
>  					      VDN_MFX_POWERGATE_ENABLE(i));
>  	}
>  
> +	rc6->pg_enable = pg_enable;

this looks borderline racy, it's fine only because this function
is called during resume which normally runs in atomic context.

>  	intel_uncore_write_fw(uncore, GEN9_PG_ENABLE, pg_enable);
>  }
>  
> @@ -572,8 +573,11 @@ static void __intel_rc6_disable(struct intel_rc6 *rc6)
>  	intel_guc_rc_disable(gt_to_guc(gt));
>  
>  	intel_uncore_forcewake_get(uncore, FORCEWAKE_ALL);
> -	if (GRAPHICS_VER(i915) >= 9)
> +	if (GRAPHICS_VER(i915) >= 9) {
> +		rc6->pg_enable = 0;
>  		intel_uncore_write_fw(uncore, GEN9_PG_ENABLE, 0);
> +	}
> +
>  	intel_uncore_write_fw(uncore, GEN6_RC_CONTROL, 0);
>  	intel_uncore_write_fw(uncore, GEN6_RC_STATE, 0);
>  	intel_uncore_forcewake_put(uncore, FORCEWAKE_ALL);
> @@ -687,6 +691,15 @@ void intel_rc6_unpark(struct intel_rc6 *rc6)
>  
>  	/* Restore HW timers for automatic RC6 entry while busy */
>  	intel_uncore_write_fw(uncore, GEN6_RC_CONTROL, rc6->ctl_enable);
> +
> +	/*
> +	 * Seeing render forcewake timeouts during active submissions so disable render PG
> +	 * while workloads are under execution.

Can you please improve this sentence? If I never new about the
issue I would be a bit confused.

> +	 * FIXME Remove this change once real cause of render force wake timeout is fixed
> +	 */
> +	if (rc6->pg_enable == GEN9_RENDER_PG_ENABLE)

is this supposed to be "pg_enable == GEN9_RENDER_PG_ENABLE" or
"pg_enable & GEN9_RENDER_PG_ENABLE" ?

Andi