On 06/06/2019 10:57, Chris Wilson wrote: > Quoting Tvrtko Ursulin (2019-06-06 10:36:27) >> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> >> >> Continuing the conversion and elimination of implicit dev_priv. >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> >> Suggested-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- >> drivers/gpu/drm/i915/i915_drv.c | 2 +- >> drivers/gpu/drm/i915/i915_gem_gtt.c | 34 ++++++++++++----------- >> drivers/gpu/drm/i915/i915_gem_gtt.h | 2 +- >> 4 files changed, 21 insertions(+), 19 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c >> index 0e9b74f52503..3554d0dd7b1a 100644 >> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c >> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c >> @@ -453,7 +453,7 @@ int intel_engines_init_mmio(struct drm_i915_private *i915) >> >> RUNTIME_INFO(i915)->num_engines = hweight32(mask); >> >> - i915_check_and_clear_faults(i915); >> + i915_check_and_clear_faults(&i915->uncore); > > I am not sold on that. Especially as it is then unwrapped back to i915. It isn't really, not on the logical level. This is the body: void i915_check_and_clear_faults(struct intel_uncore *uncore) { struct drm_i915_private *i915 = uncore_to_i915(uncore); /* From GEN8 onwards we only have one 'All Engine Fault Register' */ if (INTEL_GEN(i915) >= 8) gen8_check_faults(uncore); else if (INTEL_GEN(i915) >= 6) gen6_check_faults(uncore); else return; uncore_clear_error_registers(uncore, ALL_ENGINES); } So the idea being i915 is used only for "what gen am I checkes", while the actual functionality operates on uncore. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx