Re: [PATCH v2 4/9] drm/i915/display: add display platforms structure with platform members

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Aug 19, 2024 at 09:44:31PM +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   |  8 ++++++--
>  .../drm/i915/display/intel_display_device.h   | 19 +++++++++++++++++++
>  2 files changed, 25 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.c b/drivers/gpu/drm/i915/display/intel_display_device.c
> index 336cc8e717f5..55ab1923c8b3 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.c
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.c
> @@ -21,6 +21,7 @@ __diag_push();
>  __diag_ignore_all("-Woverride-init", "Allow field initialization overrides for display info");
>  
>  struct subplatform_desc {
> +	struct intel_display_platforms platforms;
>  	enum intel_display_platform subplatform;

singular and plural here gets a bit confusing as well...
perhaps we should change the enum to something else?
intel_display_plat_names :/


But well, again, I'm bad with names and without anything better to suggest:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

>  	const char *name;
>  	const u16 *pciidlist;
> @@ -28,9 +29,11 @@ struct subplatform_desc {
>  
>  #define SUBPLATFORM(_platform, _subplatform)				\
>  	.subplatform = (INTEL_DISPLAY_##_platform##_##_subplatform),	\
> -	.name = #_subplatform
> +	.name = #_subplatform,						\
> +	.platforms._platform##_##_subplatform = 1
>  
>  struct platform_desc {
> +	struct intel_display_platforms platforms;
>  	enum intel_display_platform platform;
>  	const char *name;
>  	const struct subplatform_desc *subplatforms;
> @@ -39,7 +42,8 @@ struct platform_desc {
>  
>  #define PLATFORM(_platform)			 \
>  	.platform = (INTEL_DISPLAY_##_platform), \
> -	.name = #_platform
> +	.name = #_platform,			 \
> +	.platforms._platform = 1
>  
>  #define ID(id) (id)
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> index 0a78f7438035..6f8814d1fabd 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"
> @@ -101,6 +102,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); \
> -- 
> 2.39.2
>