| Message ID | 20171111004448.12360-1-michel.thierry@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Quoting Patchwork (2017-11-11 01:03:20) > == Series Details == > > Series: series starting with [1/2] drm/i915: Clear per-engine fault register as early as possible > URL : https://patchwork.freedesktop.org/series/33649/ > State : success BAT results arrived before patches, yay! Patch 1 looks ok for the minimal fix. I think we want to juggle around the suspend/resume i915_check_and_clear_faults() some more, but for now that looks fine. R-b. Patch 2 still has a lack of statics on the non-exported functions. But the names are indeed clearer. I would make the if-else-chain: if (GEN >= 8) gen8_check_and_clear_faults(); else if (GEN >= 6) gen6_check_and_clear_faults(); else return; You also want to move the POSTING_READ since it is still chasing the wrong register on gen8, no? -Chris
On 11/10/2017 5:15 PM, Chris Wilson wrote: > Quoting Patchwork (2017-11-11 01:03:20) >> == Series Details == >> >> Series: series starting with [1/2] drm/i915: Clear per-engine fault register as early as possible >> URL : https://patchwork.freedesktop.org/series/33649/ >> State : success > > BAT results arrived before patches, yay! > > Patch 1 looks ok for the minimal fix. I think we want to juggle around > the suspend/resume i915_check_and_clear_faults() some more, but for now > that looks fine. R-b. > > Patch 2 still has a lack of statics on the non-exported functions. But > the names are indeed clearer. I would make the if-else-chain: > > if (GEN >= 8) > gen8_check_and_clear_faults(); > else if (GEN >= 6) > gen6_check_and_clear_faults(); > else > return; > > You also want to move the POSTING_READ since it is still chasing the > wrong register on gen8, no? I left the POSTING_READ because it's the RCS reg, which is same as gen8, but then the comment above it is no longer valid anyway (the first patch is addressing that)... v3 soon.
Quoting Patchwork (2017-11-13 18:13:01) > == Series Details == > > Series: series starting with [1/2] drm/i915: Clear per-engine fault register as early as possible (rev2) > URL : https://patchwork.freedesktop.org/series/33649/ > State : success More MTA fun, so I'll plonk my Reviewed-by here and pick these up from pw. Thanks for the patches, -Chris
diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c index a0f9d0eb4bce..70bbe8ef8f54 100644 --- a/drivers/gpu/drm/i915/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/intel_engine_cs.c @@ -289,6 +289,8 @@ int intel_engines_init_mmio(struct drm_i915_private *dev_priv) device_info->num_rings = hweight32(mask); + i915_check_and_clear_faults(dev_priv); + return 0; cleanup: diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 211acee7c31d..a78ceafcc825 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -1420,8 +1420,6 @@ void intel_uncore_init(struct drm_i915_private *dev_priv) iosf_mbi_register_pmic_bus_access_notifier( &dev_priv->uncore.pmic_bus_access_nb); - - i915_check_and_clear_faults(dev_priv); } void intel_uncore_fini(struct drm_i915_private *dev_priv)
From gen6, the hardware tracks address lookup failures and we should clear those registers upon startup to prevent false positives. However, this was happening before we have the engines defined (intel_uncore_init()) and the for_each_engine loop was just a nop. The earliest we can call this is inside intel_engines_init_mmio(). Suggested-by: Chris Wilson <chris@chris-wilson.co.uk> Signed-off-by: Michel Thierry <michel.thierry@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/intel_engine_cs.c | 2 ++ drivers/gpu/drm/i915/intel_uncore.c | 2 -- 2 files changed, 2 insertions(+), 2 deletions(-)