Re: [Intel-gfx] [PATCH v5 6/7] drm/i915/pmu: Prepare for multi-tile non-engine counters

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Tue, 16 May 2023 16:35:33 -0700, Umesh Nerlige Ramappa wrote:
>

Hi Umesh,

> +static u64 frequency_enabled_mask(void)

u32

> +{
> +	unsigned int i;
> +	u64 mask = 0;

u32

> +
> +	for (i = 0; i < I915_PMU_MAX_GTS; i++)
> +		mask |= config_mask(__I915_PMU_ACTUAL_FREQUENCY(i)) |
> +			config_mask(__I915_PMU_REQUESTED_FREQUENCY(i));
> +
> +	return mask;
> +}
> +
>  static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
>  {
>	struct drm_i915_private *i915 = container_of(pmu, typeof(*i915), pmu);
> -	u32 enable;
> +	u64 enable;

u32

>
>	/*
>	 * Only some counters need the sampling timer.
> @@ -131,9 +155,7 @@ static bool pmu_needs_timer(struct i915_pmu *pmu, bool gpu_active)
>	 * Mask out all the ones which do not need the timer, or in
>	 * other words keep all the ones that could need the timer.
>	 */
> -	enable &= config_mask(I915_PMU_ACTUAL_FREQUENCY) |
> -		  config_mask(I915_PMU_REQUESTED_FREQUENCY) |
> -		  ENGINE_SAMPLE_MASK;
> +	enable &= frequency_enabled_mask() | ENGINE_SAMPLE_MASK;
>
>	/*
>	 * When the GPU is idle per-engine counters do not need to be

/snip/

> diff --git a/drivers/gpu/drm/i915/i915_pmu.h b/drivers/gpu/drm/i915/i915_pmu.h
> index 3a811266ac6a..f88de9ae1ebb 100644
> --- a/drivers/gpu/drm/i915/i915_pmu.h
> +++ b/drivers/gpu/drm/i915/i915_pmu.h
> @@ -38,13 +38,16 @@ enum {
>	__I915_NUM_PMU_SAMPLERS
>  };
>
> +#define I915_PMU_MAX_GTS 2
> +
>  /*
>   * How many different events we track in the global PMU mask.
>   *
>   * It is also used to know to needed number of event reference counters.
>   */
>  #define I915_PMU_MASK_BITS \
> -	(I915_ENGINE_SAMPLE_COUNT + __I915_PMU_TRACKED_EVENT_COUNT)
> +	(I915_ENGINE_SAMPLE_COUNT + \
> +	 I915_PMU_MAX_GTS * __I915_PMU_TRACKED_EVENT_COUNT)
>
>  #define I915_ENGINE_SAMPLE_COUNT (I915_SAMPLE_SEMA + 1)
>
> @@ -95,7 +98,7 @@ struct i915_pmu {
>	 *
>	 * Low bits are engine samplers and other events continue from there.
>	 */
> -	u32 enable;
> +	u64 enable;

u32

Thanks.
--
Ashutosh