Quoting Mika Kuoppala (2017-10-09 11:05:40)
> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
>
> > Add assert_forcewakes_active() (the complementary function to
> > assert_forcewakes_inactive) that documents the requirement of a
> > function for its callers to be holding the forcewake ref (i.e. the
> > function is part of a sequence over which RC6 must be prevented).
> >
> > One such example is during ringbuffer reset, where RC6 must be held
> > across the whole reinitialisation sequence.
> >
> > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/intel_ringbuffer.c | 11 ++++++++++-
> > drivers/gpu/drm/i915/intel_uncore.c | 12 ++++++++++++
> > drivers/gpu/drm/i915/intel_uncore.h | 2 ++
> > 3 files changed, 24 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 05c08b0bc172..4285f09ff8b8 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -579,7 +579,16 @@ static int init_ring_common(struct intel_engine_cs *engine)
> > static void reset_ring_common(struct intel_engine_cs *engine,
> > struct drm_i915_gem_request *request)
> > {
> > - /* Try to restore the logical GPU state to match the continuation
> > + /*
> > + * RC6 must be prevented until the reset is complete and the engine
> > + * reinitialised. If it occurs in the middle of this sequence, the
> > + * state written to/loaded from the power context is ill-defined (e.g.
> > + * the PP_BASE_DIR may be lost).
> > + */
> > + assert_forcewakes_active(engine->i915, FORCEWAKE_ALL);
> > +
> > + /*
> > + * Try to restore the logical GPU state to match the continuation
> > * of the request queue. If we skip the context/PD restore, then
> > * the next request may try to execute assuming that its context
> > * is valid and loaded on the GPU and so may try to access invalid
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index b3c3f94fc7e4..3d41667919dc 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -629,6 +629,18 @@ void assert_forcewakes_inactive(struct drm_i915_private *dev_priv)
> > WARN_ON(dev_priv->uncore.fw_domains_active);
> > }
> >
> > +void assert_forcewakes_active(struct drm_i915_private *dev_priv,
> > + enum forcewake_domains fw_domains)
> > +{
> > + if (!dev_priv->uncore.funcs.force_wake_get)
> > + return;
> > +
> > + assert_rpm_wakelock_held(dev_priv);
> > +
> > + fw_domains &= dev_priv->uncore.fw_domains;
> > + WARN_ON(fw_domains & ~dev_priv->uncore.fw_domains_active);
> > +}
> > +
>
> Adding a debug telling about current domains seems not
> to add any value...atleast yet.
But will if it ever fires, so yes I had better add it.
s/WARN_ON/WARN(".../
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
