On 6/11/2019 2:36 AM, Chris Wilson wrote:
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
There was also a mention a while back of splitting gt and display gens
(https://patchwork.freedesktop.org/series/51860/), if we ever decide
that that makes sense the gt gen will just naturally move and we'll save
most of the jumps to i915.
Daniele
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
