Re: [Intel-gfx] [PATCH 1/3] i915/perf: Store a mask of valid OA formats for a platform

[Lionel Landwerlin <lionel.g.landwerlin@intel.com>]

On 02/02/2021 09:54, Umesh Nerlige Ramappa wrote:
> Validity of an OA format is checked by using a sparse array of formats
> per gen. Instead maintain a mask of supported formats for a platform in
> the perf object.
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>


Nice cleanup : Reviewed-by: Lionel Landwerlin 
<lionel.g.landwerlin@intel.com>


Thanks!


> ---
>   drivers/gpu/drm/i915/i915_perf.c       | 64 +++++++++++++++++++++++++-
>   drivers/gpu/drm/i915/i915_perf_types.h | 16 +++++++
>   2 files changed, 79 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 112ba5f2ce90..973577fcad58 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -3524,6 +3524,19 @@ static u64 oa_exponent_to_ns(struct i915_perf *perf, int exponent)
>   					     2ULL << exponent);
>   }
>   
> +static __always_inline bool
> +oa_format_valid(struct i915_perf *perf, enum drm_i915_oa_format format)
> +{
> +	return !!(perf->format_mask[__format_index(format)] &
> +		  __format_bit(format));
> +}
> +
> +static __always_inline void
> +oa_format_add(struct i915_perf *perf, enum drm_i915_oa_format format)
> +{
> +	perf->format_mask[__format_index(format)] |= __format_bit(format);
> +}
> +
>   /**
>    * read_properties_unlocked - validate + copy userspace stream open properties
>    * @perf: i915 perf instance
> @@ -3615,7 +3628,7 @@ static int read_properties_unlocked(struct i915_perf *perf,
>   					  value);
>   				return -EINVAL;
>   			}
> -			if (!perf->oa_formats[value].size) {
> +			if (!oa_format_valid(perf, value)) {
>   				DRM_DEBUG("Unsupported OA report format %llu\n",
>   					  value);
>   				return -EINVAL;
> @@ -4259,6 +4272,53 @@ static struct ctl_table dev_root[] = {
>   	{}
>   };
>   
> +static void oa_init_supported_formats(struct i915_perf *perf)
> +{
> +	struct drm_i915_private *i915 = perf->i915;
> +	enum intel_platform platform = INTEL_INFO(i915)->platform;
> +
> +	switch (platform) {
> +	case INTEL_HASWELL:
> +		oa_format_add(perf, I915_OA_FORMAT_A13);
> +		oa_format_add(perf, I915_OA_FORMAT_A13);
> +		oa_format_add(perf, I915_OA_FORMAT_A29);
> +		oa_format_add(perf, I915_OA_FORMAT_A13_B8_C8);
> +		oa_format_add(perf, I915_OA_FORMAT_B4_C8);
> +		oa_format_add(perf, I915_OA_FORMAT_A45_B8_C8);
> +		oa_format_add(perf, I915_OA_FORMAT_B4_C8_A16);
> +		oa_format_add(perf, I915_OA_FORMAT_C4_B8);
> +		break;
> +
> +	case INTEL_BROADWELL:
> +	case INTEL_CHERRYVIEW:
> +	case INTEL_SKYLAKE:
> +	case INTEL_BROXTON:
> +	case INTEL_KABYLAKE:
> +	case INTEL_GEMINILAKE:
> +	case INTEL_COFFEELAKE:
> +	case INTEL_COMETLAKE:
> +	case INTEL_CANNONLAKE:
> +	case INTEL_ICELAKE:
> +	case INTEL_ELKHARTLAKE:
> +	case INTEL_JASPERLAKE:
> +		oa_format_add(perf, I915_OA_FORMAT_A12);
> +		oa_format_add(perf, I915_OA_FORMAT_A12_B8_C8);
> +		oa_format_add(perf, I915_OA_FORMAT_A32u40_A4u32_B8_C8);
> +		oa_format_add(perf, I915_OA_FORMAT_C4_B8);
> +		break;
> +
> +	case INTEL_TIGERLAKE:
> +	case INTEL_ROCKETLAKE:
> +	case INTEL_DG1:
> +	case INTEL_ALDERLAKE_S:
> +		oa_format_add(perf, I915_OA_FORMAT_A32u40_A4u32_B8_C8);
> +		break;
> +
> +	default:
> +		MISSING_CASE(platform);
> +	}
> +}
> +
>   /**
>    * i915_perf_init - initialize i915-perf state on module bind
>    * @i915: i915 device instance
> @@ -4408,6 +4468,8 @@ void i915_perf_init(struct drm_i915_private *i915)
>   			     500 * 1000 /* 500us */);
>   
>   		perf->i915 = i915;
> +
> +		oa_init_supported_formats(perf);
>   	}
>   }
>   
> diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
> index a36a455ae336..f81bcb533723 100644
> --- a/drivers/gpu/drm/i915/i915_perf_types.h
> +++ b/drivers/gpu/drm/i915/i915_perf_types.h
> @@ -15,6 +15,7 @@
>   #include <linux/types.h>
>   #include <linux/uuid.h>
>   #include <linux/wait.h>
> +#include <uapi/drm/i915_drm.h>
>   
>   #include "gt/intel_sseu.h"
>   #include "i915_reg.h"
> @@ -441,6 +442,21 @@ struct i915_perf {
>   	struct i915_oa_ops ops;
>   	const struct i915_oa_format *oa_formats;
>   
> +	/**
> +	 * Use a format mask to store the supported formats
> +	 * for a platform.
> +	 */
> +#define __fbits (BITS_PER_TYPE(u32))
> +#define __format_bit(__f) \
> +	BIT((__f) & (__fbits - 1))
> +
> +#define __format_index_shift (5)
> +#define __format_index(__f) \
> +	(((__f) & ~(__fbits - 1)) >> __format_index_shift)
> +
> +#define FORMAT_MASK_SIZE (((I915_OA_FORMAT_MAX - 1) / __fbits) + 1)
> +	u32 format_mask[FORMAT_MASK_SIZE];
> +
>   	atomic64_t noa_programming_delay;
>   };
>   


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx