diff mbox series

[v3,03/15] drm/i915/display: use a macro to define platform enumerations

Message ID c10f2ca9980a1f62aad26b8e349552db475933ff.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 
We'll be needing a macro based list of platforms for more things in the
future. Start by defining the platform enumerations with it.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 .../drm/i915/display/intel_display_device.h   | 115 ++++++++++--------
 1 file changed, 61 insertions(+), 54 deletions(-)

Comments

Rodrigo Vivi Oct. 7, 2024, 7:48 p.m. UTC | #1 
On Mon, Sep 30, 2024 at 03:31:04PM +0300, Jani Nikula wrote:
> We'll be needing a macro based list of platforms for more things in the
> future. Start by defining the platform enumerations with it.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  .../drm/i915/display/intel_display_device.h   | 115 ++++++++++--------
>  1 file changed, 61 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> index 5306bbd13e59..1cc1a2de9e6a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -15,63 +15,70 @@ struct drm_i915_private;
>  struct drm_printer;
>  
>  /* Keep in gen based order, and chronological order within a gen */
> +#define INTEL_DISPLAY_PLATFORMS(func) \
> +	func(PLATFORM_UNINITIALIZED) \
> +	/* Display ver 2 */ \
> +	func(I830) \
> +	func(I845G) \
> +	func(I85X) \
> +	func(I865G) \
> +	/* Display ver 3 */ \
> +	func(I915G) \
> +	func(I915GM) \
> +	func(I945G) \
> +	func(I945GM) \
> +	func(G33) \
> +	func(PINEVIEW) \
> +	/* Display ver 4 */ \
> +	func(I965G) \
> +	func(I965GM) \
> +	func(G45) \
> +	func(GM45) \
> +	/* Display ver 5 */ \
> +	func(IRONLAKE) \
> +	/* Display ver 6 */ \
> +	func(SANDYBRIDGE) \
> +	/* Display ver 7 */ \
> +	func(IVYBRIDGE) \
> +	func(VALLEYVIEW) \
> +	func(HASWELL) \
> +	/* Display ver 8 */ \
> +	func(BROADWELL) \
> +	func(CHERRYVIEW) \
> +	/* Display ver 9 */ \
> +	func(SKYLAKE) \
> +	func(BROXTON) \
> +	func(KABYLAKE) \
> +	func(GEMINILAKE) \
> +	func(COFFEELAKE) \
> +	func(COMETLAKE) \
> +	/* Display ver 11 */ \
> +	func(ICELAKE) \
> +	func(JASPERLAKE) \
> +	func(ELKHARTLAKE) \
> +	/* Display ver 12 */ \
> +	func(TIGERLAKE) \
> +	func(ROCKETLAKE) \
> +	func(DG1) \
> +	func(ALDERLAKE_S) \
> +	/* Display ver 13 */ \
> +	func(ALDERLAKE_P) \
> +	func(DG2) \
> +	/* Display ver 14 (based on GMD ID) */ \
> +	func(METEORLAKE) \
> +	/* Display ver 20 (based on GMD ID) */ \
> +	func(LUNARLAKE) \
> +	/* Display ver 14.1 (based on GMD ID) */ \
> +	func(BATTLEMAGE)
> +
> +#define __ENUM(x) INTEL_DISPLAY_ ## x,
> +
>  enum intel_display_platform {
> -	INTEL_DISPLAY_PLATFORM_UNINITIALIZED = 0,
> -	/* Display ver 2 */
> -	INTEL_DISPLAY_I830,
> -	INTEL_DISPLAY_I845G,
> -	INTEL_DISPLAY_I85X,
> -	INTEL_DISPLAY_I865G,
> -	/* Display ver 3 */
> -	INTEL_DISPLAY_I915G,
> -	INTEL_DISPLAY_I915GM,
> -	INTEL_DISPLAY_I945G,
> -	INTEL_DISPLAY_I945GM,
> -	INTEL_DISPLAY_G33,
> -	INTEL_DISPLAY_PINEVIEW,
> -	/* Display ver 4 */
> -	INTEL_DISPLAY_I965G,
> -	INTEL_DISPLAY_I965GM,
> -	INTEL_DISPLAY_G45,
> -	INTEL_DISPLAY_GM45,
> -	/* Display ver 5 */
> -	INTEL_DISPLAY_IRONLAKE,
> -	/* Display ver 6 */
> -	INTEL_DISPLAY_SANDYBRIDGE,
> -	/* Display ver 7 */
> -	INTEL_DISPLAY_IVYBRIDGE,
> -	INTEL_DISPLAY_VALLEYVIEW,
> -	INTEL_DISPLAY_HASWELL,
> -	/* Display ver 8 */
> -	INTEL_DISPLAY_BROADWELL,
> -	INTEL_DISPLAY_CHERRYVIEW,
> -	/* Display ver 9 */
> -	INTEL_DISPLAY_SKYLAKE,
> -	INTEL_DISPLAY_BROXTON,
> -	INTEL_DISPLAY_KABYLAKE,
> -	INTEL_DISPLAY_GEMINILAKE,
> -	INTEL_DISPLAY_COFFEELAKE,
> -	INTEL_DISPLAY_COMETLAKE,
> -	/* Display ver 11 */
> -	INTEL_DISPLAY_ICELAKE,
> -	INTEL_DISPLAY_JASPERLAKE,
> -	INTEL_DISPLAY_ELKHARTLAKE,
> -	/* Display ver 12 */
> -	INTEL_DISPLAY_TIGERLAKE,
> -	INTEL_DISPLAY_ROCKETLAKE,
> -	INTEL_DISPLAY_DG1,
> -	INTEL_DISPLAY_ALDERLAKE_S,
> -	/* Display ver 13 */
> -	INTEL_DISPLAY_ALDERLAKE_P,
> -	INTEL_DISPLAY_DG2,
> -	/* Display ver 14 (based on GMD ID) */
> -	INTEL_DISPLAY_METEORLAKE,
> -	/* Display ver 20 (based on GMD ID) */
> -	INTEL_DISPLAY_LUNARLAKE,
> -	/* Display ver 14.1 (based on GMD ID) */
> -	INTEL_DISPLAY_BATTLEMAGE,
> +	INTEL_DISPLAY_PLATFORMS(__ENUM)

these func macros gets me confused so easily, but I believe
everything looks okay here and I'm trusting your compiler and
experiments more.

And I don't believe the CI issues could be root caused here,
so

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

>  };
>  
> +#undef __ENUM
> +
>  enum intel_display_subplatform {
>  	INTEL_DISPLAY_SUBPLATFORM_UNINITIALIZED = 0,
>  	INTEL_DISPLAY_HASWELL_ULT,
> -- 
> 2.39.5
>
Michal Wajdeczko Oct. 7, 2024, 8:36 p.m. UTC | #2 
On 30.09.2024 14:31, Jani Nikula wrote:
> We'll be needing a macro based list of platforms for more things in the
> future. Start by defining the platform enumerations with it.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  .../drm/i915/display/intel_display_device.h   | 115 ++++++++++--------
>  1 file changed, 61 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> index 5306bbd13e59..1cc1a2de9e6a 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -15,63 +15,70 @@ struct drm_i915_private;
>  struct drm_printer;
>  
>  /* Keep in gen based order, and chronological order within a gen */
> +#define INTEL_DISPLAY_PLATFORMS(func) \
> +	func(PLATFORM_UNINITIALIZED) \

maybe this one should be defined in the old-fashion way so the
INTEL_DISPLAY_PLATFORMS macro will contain only valid IDs?
Jani Nikula Oct. 8, 2024, 9:30 a.m. UTC | #3 
On Mon, 07 Oct 2024, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
> On 30.09.2024 14:31, Jani Nikula wrote:
>> We'll be needing a macro based list of platforms for more things in the
>> future. Start by defining the platform enumerations with it.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  .../drm/i915/display/intel_display_device.h   | 115 ++++++++++--------
>>  1 file changed, 61 insertions(+), 54 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
>> index 5306bbd13e59..1cc1a2de9e6a 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
>> @@ -15,63 +15,70 @@ struct drm_i915_private;
>>  struct drm_printer;
>>  
>>  /* Keep in gen based order, and chronological order within a gen */
>> +#define INTEL_DISPLAY_PLATFORMS(func) \
>> +	func(PLATFORM_UNINITIALIZED) \
>
> maybe this one should be defined in the old-fashion way so the
> INTEL_DISPLAY_PLATFORMS macro will contain only valid IDs?

I don't understand.

BR,
Jani.
Michal Wajdeczko Oct. 8, 2024, 9:42 a.m. UTC | #4 
On 08.10.2024 11:30, Jani Nikula wrote:
> On Mon, 07 Oct 2024, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
>> On 30.09.2024 14:31, Jani Nikula wrote:
>>> We'll be needing a macro based list of platforms for more things in the
>>> future. Start by defining the platform enumerations with it.
>>>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>  .../drm/i915/display/intel_display_device.h   | 115 ++++++++++--------
>>>  1 file changed, 61 insertions(+), 54 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
>>> index 5306bbd13e59..1cc1a2de9e6a 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
>>> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
>>> @@ -15,63 +15,70 @@ struct drm_i915_private;
>>>  struct drm_printer;
>>>  
>>>  /* Keep in gen based order, and chronological order within a gen */
>>> +#define INTEL_DISPLAY_PLATFORMS(func) \
>>> +	func(PLATFORM_UNINITIALIZED) \
>>
>> maybe this one should be defined in the old-fashion way so the
>> INTEL_DISPLAY_PLATFORMS macro will contain only valid IDs?
> 
> I don't understand.
> 

I mean something like this:

enum intel_display_platform {
	INTEL_DISPLAY_PLATFORM_UNINITIALIZED = 0,
	INTEL_DISPLAY_PLATFORMS(__ENUM)
};

with that INTEL_DISPLAY_PLATFORMS macro could be used in some other
cases (maybe tests) without worrying about that uninitialized case.
Jani Nikula Oct. 8, 2024, 10:08 a.m. UTC | #5 
On Tue, 08 Oct 2024, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
> On 08.10.2024 11:30, Jani Nikula wrote:
>> On Mon, 07 Oct 2024, Michal Wajdeczko <michal.wajdeczko@intel.com> wrote:
>>> On 30.09.2024 14:31, Jani Nikula wrote:
>>>> We'll be needing a macro based list of platforms for more things in the
>>>> future. Start by defining the platform enumerations with it.
>>>>
>>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>>> ---
>>>>  .../drm/i915/display/intel_display_device.h   | 115 ++++++++++--------
>>>>  1 file changed, 61 insertions(+), 54 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
>>>> index 5306bbd13e59..1cc1a2de9e6a 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
>>>> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
>>>> @@ -15,63 +15,70 @@ struct drm_i915_private;
>>>>  struct drm_printer;
>>>>  
>>>>  /* Keep in gen based order, and chronological order within a gen */
>>>> +#define INTEL_DISPLAY_PLATFORMS(func) \
>>>> +	func(PLATFORM_UNINITIALIZED) \
>>>
>>> maybe this one should be defined in the old-fashion way so the
>>> INTEL_DISPLAY_PLATFORMS macro will contain only valid IDs?
>> 
>> I don't understand.
>> 
>
> I mean something like this:
>
> enum intel_display_platform {
> 	INTEL_DISPLAY_PLATFORM_UNINITIALIZED = 0,
> 	INTEL_DISPLAY_PLATFORMS(__ENUM)
> };
>
> with that INTEL_DISPLAY_PLATFORMS macro could be used in some other
> cases (maybe tests) without worrying about that uninitialized case.

Patch 8 removes the enum altogether.


BR,
Jani.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index 5306bbd13e59..1cc1a2de9e6a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -15,63 +15,70 @@  struct drm_i915_private;
 struct drm_printer;
 
 /* Keep in gen based order, and chronological order within a gen */
+#define INTEL_DISPLAY_PLATFORMS(func) \
+	func(PLATFORM_UNINITIALIZED) \
+	/* Display ver 2 */ \
+	func(I830) \
+	func(I845G) \
+	func(I85X) \
+	func(I865G) \
+	/* Display ver 3 */ \
+	func(I915G) \
+	func(I915GM) \
+	func(I945G) \
+	func(I945GM) \
+	func(G33) \
+	func(PINEVIEW) \
+	/* Display ver 4 */ \
+	func(I965G) \
+	func(I965GM) \
+	func(G45) \
+	func(GM45) \
+	/* Display ver 5 */ \
+	func(IRONLAKE) \
+	/* Display ver 6 */ \
+	func(SANDYBRIDGE) \
+	/* Display ver 7 */ \
+	func(IVYBRIDGE) \
+	func(VALLEYVIEW) \
+	func(HASWELL) \
+	/* Display ver 8 */ \
+	func(BROADWELL) \
+	func(CHERRYVIEW) \
+	/* Display ver 9 */ \
+	func(SKYLAKE) \
+	func(BROXTON) \
+	func(KABYLAKE) \
+	func(GEMINILAKE) \
+	func(COFFEELAKE) \
+	func(COMETLAKE) \
+	/* Display ver 11 */ \
+	func(ICELAKE) \
+	func(JASPERLAKE) \
+	func(ELKHARTLAKE) \
+	/* Display ver 12 */ \
+	func(TIGERLAKE) \
+	func(ROCKETLAKE) \
+	func(DG1) \
+	func(ALDERLAKE_S) \
+	/* Display ver 13 */ \
+	func(ALDERLAKE_P) \
+	func(DG2) \
+	/* Display ver 14 (based on GMD ID) */ \
+	func(METEORLAKE) \
+	/* Display ver 20 (based on GMD ID) */ \
+	func(LUNARLAKE) \
+	/* Display ver 14.1 (based on GMD ID) */ \
+	func(BATTLEMAGE)
+
+#define __ENUM(x) INTEL_DISPLAY_ ## x,
+
 enum intel_display_platform {
-	INTEL_DISPLAY_PLATFORM_UNINITIALIZED = 0,
-	/* Display ver 2 */
-	INTEL_DISPLAY_I830,
-	INTEL_DISPLAY_I845G,
-	INTEL_DISPLAY_I85X,
-	INTEL_DISPLAY_I865G,
-	/* Display ver 3 */
-	INTEL_DISPLAY_I915G,
-	INTEL_DISPLAY_I915GM,
-	INTEL_DISPLAY_I945G,
-	INTEL_DISPLAY_I945GM,
-	INTEL_DISPLAY_G33,
-	INTEL_DISPLAY_PINEVIEW,
-	/* Display ver 4 */
-	INTEL_DISPLAY_I965G,
-	INTEL_DISPLAY_I965GM,
-	INTEL_DISPLAY_G45,
-	INTEL_DISPLAY_GM45,
-	/* Display ver 5 */
-	INTEL_DISPLAY_IRONLAKE,
-	/* Display ver 6 */
-	INTEL_DISPLAY_SANDYBRIDGE,
-	/* Display ver 7 */
-	INTEL_DISPLAY_IVYBRIDGE,
-	INTEL_DISPLAY_VALLEYVIEW,
-	INTEL_DISPLAY_HASWELL,
-	/* Display ver 8 */
-	INTEL_DISPLAY_BROADWELL,
-	INTEL_DISPLAY_CHERRYVIEW,
-	/* Display ver 9 */
-	INTEL_DISPLAY_SKYLAKE,
-	INTEL_DISPLAY_BROXTON,
-	INTEL_DISPLAY_KABYLAKE,
-	INTEL_DISPLAY_GEMINILAKE,
-	INTEL_DISPLAY_COFFEELAKE,
-	INTEL_DISPLAY_COMETLAKE,
-	/* Display ver 11 */
-	INTEL_DISPLAY_ICELAKE,
-	INTEL_DISPLAY_JASPERLAKE,
-	INTEL_DISPLAY_ELKHARTLAKE,
-	/* Display ver 12 */
-	INTEL_DISPLAY_TIGERLAKE,
-	INTEL_DISPLAY_ROCKETLAKE,
-	INTEL_DISPLAY_DG1,
-	INTEL_DISPLAY_ALDERLAKE_S,
-	/* Display ver 13 */
-	INTEL_DISPLAY_ALDERLAKE_P,
-	INTEL_DISPLAY_DG2,
-	/* Display ver 14 (based on GMD ID) */
-	INTEL_DISPLAY_METEORLAKE,
-	/* Display ver 20 (based on GMD ID) */
-	INTEL_DISPLAY_LUNARLAKE,
-	/* Display ver 14.1 (based on GMD ID) */
-	INTEL_DISPLAY_BATTLEMAGE,
+	INTEL_DISPLAY_PLATFORMS(__ENUM)
 };
 
+#undef __ENUM
+
 enum intel_display_subplatform {
 	INTEL_DISPLAY_SUBPLATFORM_UNINITIALIZED = 0,
 	INTEL_DISPLAY_HASWELL_ULT,