Chris Wilson <[email protected]> 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 <[email protected]> > Cc: Tvrtko Ursulin <[email protected]> > Cc: Mika Kuoppala <[email protected]> > --- > 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. I remember yearning after this in the past. Reviewed-by: Mika Kuoppala <[email protected]> > /* We give fast paths for the really cool registers */ > #define NEEDS_FORCE_WAKE(reg) ((reg) < 0x40000) > > diff --git a/drivers/gpu/drm/i915/intel_uncore.h > b/drivers/gpu/drm/i915/intel_uncore.h > index 66eae2ce2f29..582771251b57 100644 > --- a/drivers/gpu/drm/i915/intel_uncore.h > +++ b/drivers/gpu/drm/i915/intel_uncore.h > @@ -137,6 +137,8 @@ void intel_uncore_resume_early(struct drm_i915_private > *dev_priv); > > u64 intel_uncore_edram_size(struct drm_i915_private *dev_priv); > void assert_forcewakes_inactive(struct drm_i915_private *dev_priv); > +void assert_forcewakes_active(struct drm_i915_private *dev_priv, > + enum forcewake_domains fw_domains); > const char *intel_uncore_forcewake_domain_to_str(const enum > forcewake_domain_id id); > > enum forcewake_domains > -- > 2.14.2 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
