From 67e6456485c7c04838a5dad720886726ae5590e0 Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Mon, 9 Oct 2017 12:03:01 +0100 Subject: [PATCH] drm/i915: Provide an assert for when we expect forcewake to be held Git-commit: 67e6456485c7c04838a5dad720886726ae5590e0 Patch-mainline: v4.15-rc1 References: FATE#322643 bsc#1055900 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. V2: Include debug information in the WARN so we know which fw domain is missing. Signed-off-by: Chris Wilson Cc: Tvrtko Ursulin Cc: Mika Kuoppala Reviewed-by: Mika Kuoppala #v1 Link: https://patchwork.freedesktop.org/patch/msgid/20171009110301.21705-5-chris@chris-wilson.co.uk Acked-by: Takashi Iwai --- drivers/gpu/drm/i915/intel_ringbuffer.c | 11 ++++++++++- drivers/gpu/drm/i915/intel_uncore.c | 18 +++++++++++++++++- drivers/gpu/drm/i915/intel_uncore.h | 2 ++ 3 files changed, 29 insertions(+), 2 deletions(-) --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -579,7 +579,16 @@ out: 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 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -626,7 +626,23 @@ void assert_forcewakes_inactive(struct d if (!dev_priv->uncore.funcs.force_wake_get) return; - WARN_ON(dev_priv->uncore.fw_domains_active); + WARN(dev_priv->uncore.fw_domains_active, + "Expected all fw_domains to be inactive, but %08x are still on\n", + 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(fw_domains & ~dev_priv->uncore.fw_domains_active, + "Expected %08x fw_domains to be active, but %08x are off\n", + fw_domains, fw_domains & ~dev_priv->uncore.fw_domains_active); } /* We give fast paths for the really cool registers */ --- 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 dr 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