On Thu, Mar 24, 2016 at 03:12:02PM +0000, Dave Gordon wrote:
> On 24/03/16 14:31, Chris Wilson wrote:
> >__force_wake_get() only acquire a temporary wakeref on forcewake that is
> >automatically releases when a timer expires. When reading the code
> >again, I confused __intel_uncore_forcewake_get for __force_wake_get and
> >to my shame thought I found a bug in an unbalanced wake_count handling.
> >
> >I claim that if the function had been called __force_wake_auto instead I
> >would not have embarrassed myself.
> >
> >Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> >Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> >Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>
> >---
> > drivers/gpu/drm/i915/intel_uncore.c | 18 +++++++++---------
> > 1 file changed, 9 insertions(+), 9 deletions(-)
>
> But how does this fit with arming the timer from the put()
> elsewhere? For consistency, should we not also arm it during the
> put() stage of these combined get-access-put functions? In other
> words, put it into the GEN6_{READ,WRITE}_FOOTER macros? And could
> they not be structured to use the same underlying set of functions,
> i.e. get -> inc ref, write register if previously zero, put->if ref
> == 1, arm timer, else dec ref?
We need to acquire the fw in the HEADER for obvious reasons. So the
question is whether we want to do it in the one step or two and add a
corresponding put to the FOOTER. Is the complexity of having a combined
function *and* the individual get/put worth saving a for_each_fw_domain()
loop per register access?
Yes, imo.
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
