diff mbox series

[v3,06/15] drm/i915/display: add display platforms structure with platform members

Message ID d824d6675f7f8b5da7de6c28fa83fed6b990c59b.1727699233.git.jani.nikula@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/display: platform identification with display->platform.<platform> | expand

Commit Message

Jani Nikula Sept. 30, 2024, 12:31 p.m. UTC 
Add a structure with a bitfield member for each platform and
subplatform, and initialize them in platform and subplatform descs.

The structure also contains a bitmap in a union for easier manipulation
of the bits.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 .../drm/i915/display/intel_display_device.c   |  4 ++++
 .../drm/i915/display/intel_display_device.h   | 19 +++++++++++++++++++
 2 files changed, 23 insertions(+)

Comments

Rodrigo Vivi Oct. 7, 2024, 7:54 p.m. UTC | #1 
On Mon, Sep 30, 2024 at 03:31:07PM +0300, Jani Nikula wrote:
> Add a structure with a bitfield member for each platform and
> subplatform, and initialize them in platform and subplatform descs.
> 
> The structure also contains a bitmap in a union for easier manipulation
> of the bits.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  .../drm/i915/display/intel_display_device.c   |  4 ++++
>  .../drm/i915/display/intel_display_device.h   | 19 +++++++++++++++++++
>  2 files changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c
> index b2610217f5c5..258660b49eba 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
> @@ -31,6 +31,7 @@ struct stepping_desc {
>  	.step_info.size = ARRAY_SIZE(_map)
>  
>  struct subplatform_desc {
> +	struct intel_display_platforms platforms;
>  	enum intel_display_platform subplatform;
>  	const char *name;
>  	const u16 *pciidlist;
> @@ -38,10 +39,12 @@ struct subplatform_desc {
>  };
>  
>  #define SUBPLATFORM(_platform, _subplatform)				\
> +	.platforms._platform##_##_subplatform = 1,			\
>  	.subplatform = (INTEL_DISPLAY_##_platform##_##_subplatform),	\
>  	.name = #_subplatform
>  
>  struct platform_desc {
> +	struct intel_display_platforms platforms;
>  	enum intel_display_platform platform;
>  	const char *name;
>  	const struct subplatform_desc *subplatforms;
> @@ -50,6 +53,7 @@ struct platform_desc {
>  };
>  
>  #define PLATFORM(_platform)			 \
> +	.platforms._platform = 1,		 \
>  	.platform = (INTEL_DISPLAY_##_platform), \
>  	.name = #_platform
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> index c4208c34cc31..f166d4698949 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -6,6 +6,7 @@
>  #ifndef __INTEL_DISPLAY_DEVICE_H__
>  #define __INTEL_DISPLAY_DEVICE_H__
>  
> +#include <linux/bitops.h>
>  #include <linux/types.h>
>  
>  #include "intel_display_conversion.h"
> @@ -104,6 +105,24 @@ enum intel_display_platform {
>  
>  #undef __ENUM
>  
> +#define __MEMBER(name) unsigned long name:1;
> +#define __COUNT(x) 1 +
> +
> +#define __NUM_PLATFORMS (INTEL_DISPLAY_PLATFORMS(__COUNT) 0)
> +
> +struct intel_display_platforms {
> +	union {
> +		struct {
> +			INTEL_DISPLAY_PLATFORMS(__MEMBER);
> +		};
> +		DECLARE_BITMAP(bitmap, __NUM_PLATFORMS);
> +	};
> +};
> +
> +#undef __MEMBER
> +#undef __COUNT

why do we need the count here?
I'm afraid I didn't fully followed this macro...

> +#undef __NUM_PLATFORMS
> +
>  #define DEV_INFO_DISPLAY_FOR_EACH_FLAG(func) \
>  	/* Keep in alphabetical order */ \
>  	func(cursor_needs_physical); \
> -- 
> 2.39.5
>
Jani Nikula Oct. 8, 2024, 9:36 a.m. UTC | #2 
On Mon, 07 Oct 2024, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Mon, Sep 30, 2024 at 03:31:07PM +0300, Jani Nikula wrote:
>> Add a structure with a bitfield member for each platform and
>> subplatform, and initialize them in platform and subplatform descs.
>> 
>> The structure also contains a bitmap in a union for easier manipulation
>> of the bits.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  .../drm/i915/display/intel_display_device.c   |  4 ++++
>>  .../drm/i915/display/intel_display_device.h   | 19 +++++++++++++++++++
>>  2 files changed, 23 insertions(+)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c
>> index b2610217f5c5..258660b49eba 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_device.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
>> @@ -31,6 +31,7 @@ struct stepping_desc {
>>  	.step_info.size = ARRAY_SIZE(_map)
>>  
>>  struct subplatform_desc {
>> +	struct intel_display_platforms platforms;
>>  	enum intel_display_platform subplatform;
>>  	const char *name;
>>  	const u16 *pciidlist;
>> @@ -38,10 +39,12 @@ struct subplatform_desc {
>>  };
>>  
>>  #define SUBPLATFORM(_platform, _subplatform)				\
>> +	.platforms._platform##_##_subplatform = 1,			\
>>  	.subplatform = (INTEL_DISPLAY_##_platform##_##_subplatform),	\
>>  	.name = #_subplatform
>>  
>>  struct platform_desc {
>> +	struct intel_display_platforms platforms;
>>  	enum intel_display_platform platform;
>>  	const char *name;
>>  	const struct subplatform_desc *subplatforms;
>> @@ -50,6 +53,7 @@ struct platform_desc {
>>  };
>>  
>>  #define PLATFORM(_platform)			 \
>> +	.platforms._platform = 1,		 \
>>  	.platform = (INTEL_DISPLAY_##_platform), \
>>  	.name = #_platform
>>  
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
>> index c4208c34cc31..f166d4698949 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
>> @@ -6,6 +6,7 @@
>>  #ifndef __INTEL_DISPLAY_DEVICE_H__
>>  #define __INTEL_DISPLAY_DEVICE_H__
>>  
>> +#include <linux/bitops.h>
>>  #include <linux/types.h>
>>  
>>  #include "intel_display_conversion.h"
>> @@ -104,6 +105,24 @@ enum intel_display_platform {
>>  
>>  #undef __ENUM
>>  
>> +#define __MEMBER(name) unsigned long name:1;
>> +#define __COUNT(x) 1 +
>> +
>> +#define __NUM_PLATFORMS (INTEL_DISPLAY_PLATFORMS(__COUNT) 0)
>> +
>> +struct intel_display_platforms {
>> +	union {
>> +		struct {
>> +			INTEL_DISPLAY_PLATFORMS(__MEMBER);
>> +		};
>> +		DECLARE_BITMAP(bitmap, __NUM_PLATFORMS);
>> +	};
>> +};
>> +
>> +#undef __MEMBER
>> +#undef __COUNT
>
> why do we need the count here?
> I'm afraid I didn't fully followed this macro...

"1 +" gets substituted for each platform in INTEL_DISPLAY_PLATFORMS(),
resulting in the number of platforms, stored in __NUM_PLATFORMS.

We use that for the DECLARE_BITMAP() above. And we're using a bitmap to
avoid hand-rolling bitmap_or(). Yeah, it's a bit icky, but a) gets the
job done, b) should not need to be manually updated when the number of
platforms grows.

BR,
Jani.

>
>> +#undef __NUM_PLATFORMS
>> +
>>  #define DEV_INFO_DISPLAY_FOR_EACH_FLAG(func) \
>>  	/* Keep in alphabetical order */ \
>>  	func(cursor_needs_physical); \
>> -- 
>> 2.39.5
>>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c
index b2610217f5c5..258660b49eba 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.c
+++ b/drivers/gpu/drm/i915/display/intel_display_device.c
@@ -31,6 +31,7 @@  struct stepping_desc {
 	.step_info.size = ARRAY_SIZE(_map)
 
 struct subplatform_desc {
+	struct intel_display_platforms platforms;
 	enum intel_display_platform subplatform;
 	const char *name;
 	const u16 *pciidlist;
@@ -38,10 +39,12 @@  struct subplatform_desc {
 };
 
 #define SUBPLATFORM(_platform, _subplatform)				\
+	.platforms._platform##_##_subplatform = 1,			\
 	.subplatform = (INTEL_DISPLAY_##_platform##_##_subplatform),	\
 	.name = #_subplatform
 
 struct platform_desc {
+	struct intel_display_platforms platforms;
 	enum intel_display_platform platform;
 	const char *name;
 	const struct subplatform_desc *subplatforms;
@@ -50,6 +53,7 @@  struct platform_desc {
 };
 
 #define PLATFORM(_platform)			 \
+	.platforms._platform = 1,		 \
 	.platform = (INTEL_DISPLAY_##_platform), \
 	.name = #_platform
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index c4208c34cc31..f166d4698949 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -6,6 +6,7 @@ 
 #ifndef __INTEL_DISPLAY_DEVICE_H__
 #define __INTEL_DISPLAY_DEVICE_H__
 
+#include <linux/bitops.h>
 #include <linux/types.h>
 
 #include "intel_display_conversion.h"
@@ -104,6 +105,24 @@  enum intel_display_platform {
 
 #undef __ENUM
 
+#define __MEMBER(name) unsigned long name:1;
+#define __COUNT(x) 1 +
+
+#define __NUM_PLATFORMS (INTEL_DISPLAY_PLATFORMS(__COUNT) 0)
+
+struct intel_display_platforms {
+	union {
+		struct {
+			INTEL_DISPLAY_PLATFORMS(__MEMBER);
+		};
+		DECLARE_BITMAP(bitmap, __NUM_PLATFORMS);
+	};
+};
+
+#undef __MEMBER
+#undef __COUNT
+#undef __NUM_PLATFORMS
+
 #define DEV_INFO_DISPLAY_FOR_EACH_FLAG(func) \
 	/* Keep in alphabetical order */ \
 	func(cursor_needs_physical); \