Message ID | 20200601072446.19548-13-chris@chris-wilson.co.uk (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Series | [01/36] drm/i915: Handle very early engine initialisation failure | expand |
Chris Wilson <chris@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@chris-wilson.co.uk> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> I am not a fan of having explicit put relying on timer, Reviewed-by: Mika Kuoppala <mika.kuoppala@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
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); } }
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@chris-wilson.co.uk> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> --- drivers/gpu/drm/i915/intel_uncore.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)