[Intel-gfx] [PATCH 13/36] drm/i915: Relinquish forcewake immediately after manual grouping

[Mika Kuoppala]

Chris Wilson <chris at chris-wilson.co.uk> writes:

> Our forcewake utilisation is split into categories: automatic and
> manual. Around bare register reads, we look up the right forcewake
> domain and automatically acquire and release [upon a timer] the
> forcewake domain. For other access, where we know we require the
> forcewake across a group of register reads, we manually acquire the
> forcewake domain and release it at the end. Again, this currently arms
> the domain timer for a later release.
>
> However, looking at some energy utilisation profiles, we have tried to
> avoid using forcewake [and rely on the natural wake up to post register
> updates] due to that even keep the fw active for a brief period
> contributes to a significant power draw [i.e. when the gpu is sleeping
> with rc6 at high clocks]. But as it turns out, not posting the writes
> immediately also has unintended consequences, such as not reducing the
> clocks and so conserving power while busy.
>
> As a compromise, let us only arm the domain timer for automatic
> forcewake usage around bare register access, but immediately release the
> forcewake when manually acquired by intel_uncore_forcewake_get/_put.
>
> The corollary to this is that we may instead have to take forcewake more
> often, and so incur a latency penalty in doing so. For Sandybridge this
> was significant, and even on the latest machines, taking forcewake at
> interrupt frequency is a huge impact. [So we don't do that anymore!
> Hopefully, this will spare us from still needing the mitigation of the
> timer for steady state execution.]
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Mika Kuoppala <mika.kuoppala at linux.intel.com>

I am not a fan of having explicit put relying on timer,
Reviewed-by: Mika Kuoppala <mika.kuoppala at linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_uncore.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index a61cb8ca4d50..7d6b9ae7403c 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -709,7 +709,7 @@ static void __intel_uncore_forcewake_put(struct intel_uncore *uncore,
>  			continue;
>  		}
>  
> -		fw_domain_arm_timer(domain);
> +		uncore->funcs.force_wake_put(uncore, domain->mask);
>  	}
>  }
>  
> -- 
> 2.20.1