[Intel-gfx] [PATCH 03/15] drm/i915/gt: Use caller provided forcewake for intel_mocs_init_engine

[Chris Wilson]

Quoting Tvrtko Ursulin (2019-07-03 12:23:52)
> 
> On 03/07/2019 10:17, Chris Wilson wrote:
> > During post-reset resume, we call intel_mocs_init_engine to reinitialise
> > the MOCS registers. Suprisingly, especially when enhanced by lockdep,
> > the acquisition of the forcewake lock around each register write takes a
> > substantial portion of the reset time. We don't need to use the
> > individual forcewake here as we can assume that the caller is holding a
> > blanket forcewake for the reset&resume and the resume is serialised.
> > 
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > ---
> >   drivers/gpu/drm/i915/gt/intel_mocs.c | 15 +++++++++------
> >   1 file changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/gt/intel_mocs.c b/drivers/gpu/drm/i915/gt/intel_mocs.c
> > index ae6cbf0d517c..290a5e9b90b9 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_mocs.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_mocs.c
> > @@ -346,6 +346,9 @@ void intel_mocs_init_engine(struct intel_engine_cs *engine)
> >       unsigned int index;
> >       u32 unused_value;
> >   
> > +     /* Called under a blanket forcewake */
> > +     assert_forcewakes_active(uncore, FORCEWAKE_ALL);
> > +
> 
> Assert is strictly speaking a bit weak since forcewake status can 
> theoretically change until the actual access below. But in our current 
> code we indeed hold it for the whole reset.

You want to distinguish between an explicit hold by the caller and the
auto.
 
> I don't have any actual ideas on how to fundamentally improve the 
> assert. Thought to have it after the writes is the only thing which came 
> to mind. Thoughts?

I definitely prefer having it upfront to document the function
preconditions, and so would prefer if the assert actually did assert an
explicit forcewake as it is meant to do :)
-Chris