Quoting Tvrtko Ursulin (2019-06-11 09:41:02)
>
> On 10/06/2019 19:17, Daniele Ceraolo Spurio wrote:
> > On 6/10/19 9:16 AM, Chris Wilson wrote:
> >> Quoting Tvrtko Ursulin (2019-06-10 16:54:13)
> >>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> >>> b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> >>> index 01223864237a..343c4459e8a3 100644
> >>> --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h
> >>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h
> >>> @@ -34,6 +34,7 @@ struct drm_i915_reg_table;
> >>> struct i915_gem_context;
> >>> struct i915_request;
> >>> struct i915_sched_attr;
> >>> +struct intel_gt;
> >>> struct intel_uncore;
> >>> typedef u8 intel_engine_mask_t;
> >>> @@ -266,6 +267,7 @@ struct intel_engine_execlists {
> >>> struct intel_engine_cs {
> >>> struct drm_i915_private *i915;
> >>> + struct intel_gt *gt;
> >>
> >> I'd push for gt as being the backpointer, and i915 its distant grand
> >> parent. Not sure how much pain that would bring just for the elimination
> >> of one more drm_i915_private, but that's how I picture the
> >> encapsulation.
>
> It depends on overall direction. Are we going to go with helpers
> (XXX_to_i915) or not. Well for removing engine->i915 there would be
> churn already. But same churn regardless of whether we pick
> engine_to_i915 or engine->gt->i915.
>
> But I don't see a problem with having both i915 and gt pointers in the
> engine. It's a short cut to avoid pointer chasing and verbosity. Our
> code is fundamentally still very dependent on runtime checks against
> INTEL_GEN and INTEL_INFO, so i915 is pretty much in need all over the place.
>
> > Would it be worth moving some of the flags in the device_info structure
> > in a gt substructure, like we did for display, and get a pointer to that
> > in intel_gt? We could save some jumps back that way and be more coherent
> > in where we store the info.
>
> So even with this we maybe reduce the need to chase all the way to i915
> a bit, but not fully. Unless we decide to duplicate gen in intel_gt as
> well. Well.. now I am scared we will just decide to do that. :D
Kind off, we are already reducing the runtime checks into feature flags
or vfuncs for hot paths. I do hope the only time we need to go back to
i915 is during init. This should be reasonably true for engine; looking
at intel_lrc.c the common access is for i915->scratch, which we need to
move under intel_gt. And I expect that we will see similar natural
transitions for engine->i915.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
