Re: [RFC 08/14] drm/i915: Store backpointer to intel_gt in the engine

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

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

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx