Re: [PATCH v4 01/17] drm/connector: Convert DRM_MODE_COLORIMETRY to enum

2023-03-08 Thread Pekka Paalanen
On Tue, 7 Mar 2023 10:29:34 -0500
Harry Wentland  wrote:

> This allows us to use strongly typed arguments.
> 
> v2:
>  - Bring NO_DATA back
>  - Provide explicit enum values
> 
> v4: Drop unnecessary '&' from kerneldoc (emersion)
> 
> Signed-off-by: Harry Wentland 
> Reviewed-by: Simon Ser 
> 
> Cc: Pekka Paalanen 
> Cc: Sebastian Wick 
> Cc: vitaly.pros...@amd.com
> Cc: Uma Shankar 
> Cc: Ville Syrjälä 
> Cc: Joshua Ashton 
> Cc: dri-de...@lists.freedesktop.org
> Cc: amd-gfx@lists.freedesktop.org
> ---
>  include/drm/display/drm_dp.h |  2 +-
>  include/drm/drm_connector.h  | 49 ++--
>  2 files changed, 26 insertions(+), 25 deletions(-)
> 
> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
> index ed10e6b6f99d..dae5e9c201e4 100644
> --- a/include/drm/display/drm_dp.h
> +++ b/include/drm/display/drm_dp.h
> @@ -1623,7 +1623,7 @@ enum dp_pixelformat {
>   *
>   * This enum is used to indicate DP VSC SDP Colorimetry formats.
>   * It is based on DP 1.4 spec [Table 2-117: VSC SDP Payload for DB16 through
> - * DB18] and a name of enum member follows DRM_MODE_COLORIMETRY definition.
> + * DB18] and a name of enum member follows enum drm_colorimetry definition.
>   *
>   * @DP_COLORIMETRY_DEFAULT: sRGB (IEC 61966-2-1) or
>   *  ITU-R BT.601 colorimetry format
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 4d830fc55a3d..6d6a53a6b010 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -371,29 +371,30 @@ enum drm_privacy_screen_status {
>   * a colorspace property which will be created and exposed to
>   * userspace.
>   */
> -
> -/* For Default case, driver will set the colorspace */
> -#define DRM_MODE_COLORIMETRY_DEFAULT 0
> -/* CEA 861 Normal Colorimetry options */
> -#define DRM_MODE_COLORIMETRY_NO_DATA 0
> -#define DRM_MODE_COLORIMETRY_SMPTE_170M_YCC  1
> -#define DRM_MODE_COLORIMETRY_BT709_YCC   2
> -/* CEA 861 Extended Colorimetry Options */
> -#define DRM_MODE_COLORIMETRY_XVYCC_601   3
> -#define DRM_MODE_COLORIMETRY_XVYCC_709   4
> -#define DRM_MODE_COLORIMETRY_SYCC_6015
> -#define DRM_MODE_COLORIMETRY_OPYCC_601   6
> -#define DRM_MODE_COLORIMETRY_OPRGB   7
> -#define DRM_MODE_COLORIMETRY_BT2020_CYCC 8
> -#define DRM_MODE_COLORIMETRY_BT2020_RGB  9
> -#define DRM_MODE_COLORIMETRY_BT2020_YCC  10
> -/* Additional Colorimetry extension added as part of CTA 861.G */
> -#define DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65  11
> -#define DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER  12
> -/* Additional Colorimetry Options added for DP 1.4a VSC Colorimetry Format */
> -#define DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED  13
> -#define DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT  14
> -#define DRM_MODE_COLORIMETRY_BT601_YCC   15
> +enum drm_colorspace {
> + /* For Default case, driver will set the colorspace */
> + DRM_MODE_COLORIMETRY_DEFAULT= 0,
> + DRM_MODE_COLORIMETRY_NO_DATA= 0,
> + /* CEA 861 Normal Colorimetry options */

This comment seems to be in the wrong place, NO_DATA should be under
this comment.

With that fixed:
Reviewed-by: Pekka Paalanen 


Thanks,
pq

> + DRM_MODE_COLORIMETRY_SMPTE_170M_YCC = 1,
> + DRM_MODE_COLORIMETRY_BT709_YCC  = 2,
> + /* CEA 861 Extended Colorimetry Options */
> + DRM_MODE_COLORIMETRY_XVYCC_601  = 3,
> + DRM_MODE_COLORIMETRY_XVYCC_709  = 4,
> + DRM_MODE_COLORIMETRY_SYCC_601   = 5,
> + DRM_MODE_COLORIMETRY_OPYCC_601  = 6,
> + DRM_MODE_COLORIMETRY_OPRGB  = 7,
> + DRM_MODE_COLORIMETRY_BT2020_CYCC= 8,
> + DRM_MODE_COLORIMETRY_BT2020_RGB = 9,
> + DRM_MODE_COLORIMETRY_BT2020_YCC = 10,
> + /* Additional Colorimetry extension added as part of CTA 861.G */
> + DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65 = 11,
> + DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER = 12,
> + /* Additional Colorimetry Options added for DP 1.4a VSC Colorimetry 
> Format */
> + DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED = 13,
> + DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT = 14,
> + DRM_MODE_COLORIMETRY_BT601_YCC  = 15,
> +};
>  
>  /**
>   * enum drm_bus_flags - bus_flags info for _display_info
> @@ -826,7 +827,7 @@ struct drm_connector_state {
>* colorspace change on Sink. This is most commonly used to switch
>* to wider color gamuts like BT2020.
>*/
> - u32 colorspace;
> + enum drm_colorspace colorspace;
>  
>   /**
>* @writeback_job: Writeback job for writeback connectors



pgpGJV06OAMQw.pgp
Description: OpenPGP digital signature


[PATCH v4 01/17] drm/connector: Convert DRM_MODE_COLORIMETRY to enum

2023-03-07 Thread Harry Wentland
This allows us to use strongly typed arguments.

v2:
 - Bring NO_DATA back
 - Provide explicit enum values

v4: Drop unnecessary '&' from kerneldoc (emersion)

Signed-off-by: Harry Wentland 
Reviewed-by: Simon Ser 

Cc: Pekka Paalanen 
Cc: Sebastian Wick 
Cc: vitaly.pros...@amd.com
Cc: Uma Shankar 
Cc: Ville Syrjälä 
Cc: Joshua Ashton 
Cc: dri-de...@lists.freedesktop.org
Cc: amd-gfx@lists.freedesktop.org
---
 include/drm/display/drm_dp.h |  2 +-
 include/drm/drm_connector.h  | 49 ++--
 2 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
index ed10e6b6f99d..dae5e9c201e4 100644
--- a/include/drm/display/drm_dp.h
+++ b/include/drm/display/drm_dp.h
@@ -1623,7 +1623,7 @@ enum dp_pixelformat {
  *
  * This enum is used to indicate DP VSC SDP Colorimetry formats.
  * It is based on DP 1.4 spec [Table 2-117: VSC SDP Payload for DB16 through
- * DB18] and a name of enum member follows DRM_MODE_COLORIMETRY definition.
+ * DB18] and a name of enum member follows enum drm_colorimetry definition.
  *
  * @DP_COLORIMETRY_DEFAULT: sRGB (IEC 61966-2-1) or
  *  ITU-R BT.601 colorimetry format
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 4d830fc55a3d..6d6a53a6b010 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -371,29 +371,30 @@ enum drm_privacy_screen_status {
  * a colorspace property which will be created and exposed to
  * userspace.
  */
-
-/* For Default case, driver will set the colorspace */
-#define DRM_MODE_COLORIMETRY_DEFAULT   0
-/* CEA 861 Normal Colorimetry options */
-#define DRM_MODE_COLORIMETRY_NO_DATA   0
-#define DRM_MODE_COLORIMETRY_SMPTE_170M_YCC1
-#define DRM_MODE_COLORIMETRY_BT709_YCC 2
-/* CEA 861 Extended Colorimetry Options */
-#define DRM_MODE_COLORIMETRY_XVYCC_601 3
-#define DRM_MODE_COLORIMETRY_XVYCC_709 4
-#define DRM_MODE_COLORIMETRY_SYCC_601  5
-#define DRM_MODE_COLORIMETRY_OPYCC_601 6
-#define DRM_MODE_COLORIMETRY_OPRGB 7
-#define DRM_MODE_COLORIMETRY_BT2020_CYCC   8
-#define DRM_MODE_COLORIMETRY_BT2020_RGB9
-#define DRM_MODE_COLORIMETRY_BT2020_YCC10
-/* Additional Colorimetry extension added as part of CTA 861.G */
-#define DRM_MODE_COLORIMETRY_DCI_P3_RGB_D6511
-#define DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER12
-/* Additional Colorimetry Options added for DP 1.4a VSC Colorimetry Format */
-#define DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED13
-#define DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT14
-#define DRM_MODE_COLORIMETRY_BT601_YCC 15
+enum drm_colorspace {
+   /* For Default case, driver will set the colorspace */
+   DRM_MODE_COLORIMETRY_DEFAULT= 0,
+   DRM_MODE_COLORIMETRY_NO_DATA= 0,
+   /* CEA 861 Normal Colorimetry options */
+   DRM_MODE_COLORIMETRY_SMPTE_170M_YCC = 1,
+   DRM_MODE_COLORIMETRY_BT709_YCC  = 2,
+   /* CEA 861 Extended Colorimetry Options */
+   DRM_MODE_COLORIMETRY_XVYCC_601  = 3,
+   DRM_MODE_COLORIMETRY_XVYCC_709  = 4,
+   DRM_MODE_COLORIMETRY_SYCC_601   = 5,
+   DRM_MODE_COLORIMETRY_OPYCC_601  = 6,
+   DRM_MODE_COLORIMETRY_OPRGB  = 7,
+   DRM_MODE_COLORIMETRY_BT2020_CYCC= 8,
+   DRM_MODE_COLORIMETRY_BT2020_RGB = 9,
+   DRM_MODE_COLORIMETRY_BT2020_YCC = 10,
+   /* Additional Colorimetry extension added as part of CTA 861.G */
+   DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65 = 11,
+   DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER = 12,
+   /* Additional Colorimetry Options added for DP 1.4a VSC Colorimetry 
Format */
+   DRM_MODE_COLORIMETRY_RGB_WIDE_FIXED = 13,
+   DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT = 14,
+   DRM_MODE_COLORIMETRY_BT601_YCC  = 15,
+};
 
 /**
  * enum drm_bus_flags - bus_flags info for _display_info
@@ -826,7 +827,7 @@ struct drm_connector_state {
 * colorspace change on Sink. This is most commonly used to switch
 * to wider color gamuts like BT2020.
 */
-   u32 colorspace;
+   enum drm_colorspace colorspace;
 
/**
 * @writeback_job: Writeback job for writeback connectors
-- 
2.39.2