diff mbox series

[1/3] i915/perf: Store a mask of valid OA formats for a platform

Message ID 20210202075417.28230-1-umesh.nerlige.ramappa@intel.com (mailing list archive)
State New, archived
Headers show
Series [1/3] i915/perf: Store a mask of valid OA formats for a platform | expand

Commit Message

Umesh Nerlige Ramappa Feb. 2, 2021, 7:54 a.m. UTC
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>
---
 drivers/gpu/drm/i915/i915_perf.c       | 64 +++++++++++++++++++++++++-
 drivers/gpu/drm/i915/i915_perf_types.h | 16 +++++++
 2 files changed, 79 insertions(+), 1 deletion(-)

Comments

Chris Wilson Feb. 2, 2021, 8:24 a.m. UTC | #1
Quoting Umesh Nerlige Ramappa (2021-02-02 07:54:15)
> 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>
> ---
>  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));

!! is already provided by the implicit cast to (bool)

> +}
> +
> +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;

Ok, this looks as compact and readable as writing it as a bunch of
tables. I presume there's a reason you didn't just use generation rather
than platform.

switch (gen) {
case 7:
	haswell();
	break;
case 8 .. 11:
	broadwell();
	break;
case 12:
	tigerlake();
	break;
}
if you wanted to stick with a switch rather than an if-else tree for the
ranges.

Note you could equally do 
	case INTEL_BROADWELL .. INTEL_JASPERLAKE:
but I expect that to cause confusion for the reader.

> +       /**
> +        * 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];

This is just open-coding set_bit/test_bit

#define FORMAT_MASK_SIZE DIV_ROUND_UP(I915_OA_FORMAT_MAX - 1, BITS_PER_LONG)
unsigned long format_mask[FORMAT_MASK_SIZE];

static __always_inline bool
oa_format_valid(struct i915_perf *perf, enum drm_i915_oa_format format)
{
	return test_bit(format, perf->format_mask);
}

static __always_inline void
oa_format_add(struct i915_perf *perf, enum drm_i915_oa_format format)
{
	__set_bit(format, perf->format_mask);
}
-Chris
Umesh Nerlige Ramappa Feb. 2, 2021, 8:10 p.m. UTC | #2
On Tue, Feb 02, 2021 at 08:24:15AM +0000, Chris Wilson wrote:
>Quoting Umesh Nerlige Ramappa (2021-02-02 07:54:15)
>> 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>
>> ---
>>  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));
>
>!! is already provided by the implicit cast to (bool)
>
>> +}
>> +
>> +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;
>
>Ok, this looks as compact and readable as writing it as a bunch of
>tables. I presume there's a reason you didn't just use generation rather
>than platform.
>
>switch (gen) {
>case 7:
>	haswell();
>	break;
>case 8 .. 11:
>	broadwell();
>	break;
>case 12:
>	tigerlake();
>	break;
>}
>if you wanted to stick with a switch rather than an if-else tree for the
>ranges.

only haswell is supported on gen7 and gen12 may define new formats that 
are platform specific.

How about a mix? -

if (gen == 7 && haswell)
	haswell();
else if (gen >= 8 && gen <= 11)
	broadwell;
else
	gen12_formats();

gen12_formats can choose to use the switch if formats vary between 
platforms.

Thanks,
Umesh
>
>Note you could equally do
>	case INTEL_BROADWELL .. INTEL_JASPERLAKE:
>but I expect that to cause confusion for the reader.
>
>> +       /**
>> +        * 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];
>
>This is just open-coding set_bit/test_bit
>
>#define FORMAT_MASK_SIZE DIV_ROUND_UP(I915_OA_FORMAT_MAX - 1, BITS_PER_LONG)
>unsigned long format_mask[FORMAT_MASK_SIZE];
>
>static __always_inline bool
>oa_format_valid(struct i915_perf *perf, enum drm_i915_oa_format format)
>{
>	return test_bit(format, perf->format_mask);
>}
>
>static __always_inline void
>oa_format_add(struct i915_perf *perf, enum drm_i915_oa_format format)
>{
>	__set_bit(format, perf->format_mask);
>}
>-Chris
Chris Wilson Feb. 2, 2021, 8:44 p.m. UTC | #3
Quoting Umesh Nerlige Ramappa (2021-02-02 20:10:44)
> On Tue, Feb 02, 2021 at 08:24:15AM +0000, Chris Wilson wrote:
> >Ok, this looks as compact and readable as writing it as a bunch of
> >tables. I presume there's a reason you didn't just use generation rather
> >than platform.
> >
> >switch (gen) {
> >case 7:
> >       haswell();
> >       break;
> >case 8 .. 11:
> >       broadwell();
> >       break;
> >case 12:
> >       tigerlake();
> >       break;
> >}
> >if you wanted to stick with a switch rather than an if-else tree for the
> >ranges.
> 
> only haswell is supported on gen7 and gen12 may define new formats that 
> are platform specific.
> 
> How about a mix? -
> 
> if (gen == 7 && haswell)
>         haswell();
> else if (gen >= 8 && gen <= 11)
>         broadwell;
> else
>         gen12_formats();
> 
> gen12_formats can choose to use the switch if formats vary between 
> platforms.

I didn't mind the platform switch too much, so no need to change at the
moment. I just worry that it's more typing to maintain :)

What I thought you were going to do (from the subject) were tables with
a platform_mask for applicability,  but that I feell would be just as
much typing, now and in the future.


I thought support started at Haswell, so the other gen7 were not a
concern? But yes, if we look at how we end up doing it else where it's a
mix of gen and platform

if (gen >= 12)
	gen12_formats;
else if (gen >= 8)
	gen8_formats;
else if (IS_HSW)
	hsw_formats;
else
	MISSING_CASE(gen)

At the end of the day, you're the person who is typing this, so it's up
to you how much effort you want to spend now to save later. :)
-Chris
Lionel Landwerlin Feb. 3, 2021, 9:55 a.m. UTC | #4
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;
>   };
>
diff mbox series

Patch

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;
 };