Quoting Tvrtko Ursulin (2019-06-11 13:05:58) > > On 11/06/2019 09:52, Chris Wilson wrote: > > 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. > > Yep agreed. > > Any comments on the intel_gt.c the series added? Good. It's the direction I think we need. > And the end result in i915_gem_init(_hw)? Definitely not the end yet, but passable for now :) > >>> 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. > > Okay but that doesn't explicitly answer whether you prefer I just drop > all the XXX_to_YYY wrappers in favour of using direct pointer dereferences. I'm in favour of uncore->i915 over uncore_to_i915(uncore) and drop the embedding knowledge. > You are also in favour of replacing engine->i915 with engine->gt > straight away? I can live with an extra back pointer. I think it will ultimately be redundant, but expect a incremental evolution will be easier. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx