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

[Chris Wilson]

Quoting Tvrtko Ursulin (2017-11-22 12:05:18)
> 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.
> 
> 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>
> ---
>  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