Quoting Tvrtko Ursulin (2019-06-11 09:35:07)
>
> On 10/06/2019 17:26, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-06-10 16:54:06)
> >> 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/gt/intel_reset.c | 28 ++++++++++++-----------
> >> drivers/gpu/drm/i915/gt/intel_reset.h | 2 +-
> >> drivers/gpu/drm/i915/i915_drv.c | 2 +-
> >> drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++--
> >> 5 files changed, 20 insertions(+), 18 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> >> index c0d986db5a75..a046e8dccc96 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);
> >
> > This name is still setting off red flags for me, but I have to confess
> > that staring at it, passing uncore does make sense.
>
> Rename to intel_uncore_check_and_clear_faults?
>
> Or move later in the series as intel_gt_check_and_clear_faults?
I think I prefer the latter option, intel_gt_check_and_clear_faults.
> > I just wish we have per-engines faults everywhere and this could be
> > reduced to passing engine.
> >
> > Hmm, this I guess we will just have to revisit in the near future as we
> > may get the opportunity to put these regs under more scrutiny.
> >
> >>
> >> intel_setup_engine_capabilities(i915);
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> >> index 60d24110af80..13471916559b 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> >> @@ -1166,10 +1166,10 @@ static void gen8_clear_engine_error_register(struct intel_engine_cs *engine)
> >> GEN6_RING_FAULT_REG_POSTING_READ(engine);
> >> }
> >>
> >> -static void clear_error_registers(struct drm_i915_private *i915,
> >> +static void clear_error_registers(struct intel_uncore *uncore,
> >> intel_engine_mask_t engine_mask)
> >> {
> >> - struct intel_uncore *uncore = &i915->uncore;
> >> + struct drm_i915_private *i915 = uncore_to_i915(uncore);
> >
> > Grr, I should have objected to uncore_to_i915() loudly from the
> > beginning
> >
> > What's done is done,
>
> Is it too late already? Shouldn't be. My thinking was the implementation
> can easily be changed if/when backpointer is added (instead of
> container_of). But if you would prefer we start without a helper, but
> with a direct access to backpointer straight away that is fine by me.
I'm optimistic that we can land a split display/gt intel_uncore early
and so the churn is in the not too distant future.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
