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

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4714 bytes --]

On Tue, 7 Mar 2023 10:29:34 -0500
Harry Wentland <harry.wentland@amd.com> 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 <harry.wentland@amd.com>
> Reviewed-by: Simon Ser <contact@emersion.fr>
> 
> Cc: Pekka Paalanen <ppaalanen@gmail.com>
> Cc: Sebastian Wick <sebastian.wick@redhat.com>
> Cc: Vitaly.Prosyak@amd.com
> Cc: Uma Shankar <uma.shankar@intel.com>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Joshua Ashton <joshua@froggi.es>
> Cc: dri-devel@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_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_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 <pekka.paalanen@collabora.com>


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 &drm_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


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]