[Intel-gfx] [PATCH 2/2] drm/i915: Consolidate checks for engine stats availability

Chris Wilson chris at chris-wilson.co.uk
Fri Nov 24 11:44:28 UTC 2017


Quoting Tvrtko Ursulin (2017-11-24 11:21:21)
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> Sagar noticed the check can be consolidated between the engine stats
> implementation and the PMU.
> 
> My first choice was a static inline helper but that got into include
> ordering mess quickly fast so I went with a macro instead. At some point
> we should perhaps looking into taking out the non-ringubffer bits from
> intel_ringbuffer.h into a new intel_engine.h or something.
> 
> v2: Use engine->flags. (Chris Wilson)
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Suggested-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> ---
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index fede62daf3e1..1ee8c8dcc2c7 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -420,6 +420,9 @@ static void intel_engine_init_execlist(struct intel_engine_cs *engine)
>  
>         execlists->queue = RB_ROOT;
>         execlists->first = NULL;
> +
> +       if (INTEL_GEN(engine->i915) >=8)
> +               engine->flags |= I915_ENGINE_SUPPORTS_STATS;

Oh, I was thinking of sticking it over in logical_ring_setup() (or
_init()), so it was closer to where we implement the alternatve mode.
(At least that's the graph I have in my head, it's bit more confusing
because intel_engine_context_in() is elsewhere.)

Hmm, I am really not sure who depends on who. Setting the flag here is
immaterial if we don't couple it in from execlists. Similarly the stats
from execlists are meaningless unless they have been enabled.

Overall though, I think it is still a mechanism that is opted into by
execlists, and that's where we should then declare the flag.

Other than that (and a small tear for having to use
i915->engine[RCS]->flags),

Reviewed-by: Chris Wilson <chris at chris-wilson.co.uk>

for both patches. Joonas is going to be thrilled by the first ;)
-Chris


More information about the Intel-gfx mailing list