Quoting Tvrtko Ursulin (2017-11-22 12:05:18)
> From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
>
> 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.
>
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
> Suggested-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
> Cc: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_pmu.c | 9 ++-------
> drivers/gpu/drm/i915/intel_engine_cs.c | 4 ++--
> drivers/gpu/drm/i915/intel_ringbuffer.h | 2 ++
> 3 files changed, 6 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_pmu.c b/drivers/gpu/drm/i915/i915_pmu.c
> index 1071935bfa67..112243720ff3 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.c
> +++ b/drivers/gpu/drm/i915/i915_pmu.c
> @@ -90,11 +90,6 @@ static unsigned int event_enabled_bit(struct perf_event *event)
> return config_enabled_bit(event->attr.config);
> }
>
> -static bool supports_busy_stats(struct drm_i915_private *i915)
> -{
> - return INTEL_GEN(i915) >= 8;
> -}
> -
> static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active)
> {
> u64 enable;
> @@ -124,7 +119,7 @@ static bool pmu_needs_timer(struct drm_i915_private *i915, bool gpu_active)
> * Also there is software busyness tracking available we do not
> * need the timer for I915_SAMPLE_BUSY counter.
> */
> - else if (supports_busy_stats(i915))
> + else if (intel_supports_engine_stats(i915))
> enable &= ~BIT(I915_SAMPLE_BUSY);
>
> /*
> @@ -463,7 +458,7 @@ static void i915_pmu_event_read(struct perf_event *event)
>
> static bool engine_needs_busy_stats(struct intel_engine_cs *engine)
> {
> - return supports_busy_stats(engine->i915) &&
> + return intel_supports_engine_stats(engine->i915) &&
> (engine->pmu.enable & BIT(I915_SAMPLE_BUSY));
> }
>
> diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> index fede62daf3e1..d53680c08cb0 100644
> --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> @@ -1863,7 +1863,7 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine)
> {
> unsigned long flags;
>
> - if (INTEL_GEN(engine->i915) < 8)
> + if (!intel_supports_engine_stats(engine->i915))
> return -ENODEV;
>
> spin_lock_irqsave(&engine->stats.lock, flags);
> @@ -1924,7 +1924,7 @@ void intel_disable_engine_stats(struct intel_engine_cs *engine)
> {
> unsigned long flags;
>
> - if (INTEL_GEN(engine->i915) < 8)
> + if (!intel_supports_engine_stats(engine->i915))
> return;
>
> spin_lock_irqsave(&engine->stats.lock, flags);
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index 3bd30d011866..37a389ff031e 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -1054,6 +1054,8 @@ static inline void intel_engine_context_out(struct intel_engine_cs *engine)
> spin_unlock_irqrestore(&engine->stats.lock, flags);
> }
>
> +#define intel_supports_engine_stats(i915) (INTEL_GEN(i915) >= 8)
Hmm, do we have an engine->flags for caps? Just thinking that would be
more accurate than a gen test stuffed away in a header. So at present we
have no engine->flags, but we do already have one bool needs_cmd_parser
that could be pulled into an engine->flags. (Not sure if engine->caps is
a good name if we are adding HW caps at a later point.)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
