Re: [PATCH 01/14] drm/connector: Introduce drm_output_color_format enum

[Philipp Zabel <p.zabel@pengutronix.de>]

On Di, 2026-02-24 at 11:58 +0100, Maxime Ripard wrote:
> The EDID parsing code initially introduced the DRM_COLOR_FORMAT_*
> defines to represent the sink capabilities. Since a given sink could
> support multiple formats, it was first defined as a bitmask.
> 
> However, the core and drivers have since leveraged those defines to
> represent both the supported formats but also the current format being
> used.
> 
> Considering the latter case, the more natural, and consistent, thing to
> do would be to create an enum of all the possible formats, and then list
> the supported formats using a bitmask of the individual enum values.
> 
> Let's create a new enum following that pattern, drm_output_color_format,
> while maintaining the DRM_COLOR_FORMAT_* compatibility to make the
> transition easier.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  include/drm/drm_connector.h | 42 ++++++++++++++++++++++++++++++++++--------
>  1 file changed, 34 insertions(+), 8 deletions(-)
> 
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index 7eaec37ae1c735334afa7dad15a38cf0c8b761b8..c67539708f636ae3905bb8424c63799bd1811f28 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -554,10 +554,35 @@ enum drm_colorspace {
>  	DRM_MODE_COLORIMETRY_RGB_WIDE_FLOAT	= 14,
>  	DRM_MODE_COLORIMETRY_BT601_YCC		= 15,
>  	DRM_MODE_COLORIMETRY_COUNT
>  };
>  
> +/**
> + * enum drm_output_color_format - Output Color Format
> + *
> + * This enum is a consolidated color format list supported by
> + * connectors. It's only ever really been used for HDMI and DP so far,
> + * so it's not exhaustive and can be extended to represent other formats
> + * in the future.
> + *
> + *
> + * @DRM_OUTPUT_COLOR_FORMAT_RGB444:
> + *   RGB output format
> + * @DRM_OUTPUT_COLOR_FORMAT_YCBCR444:
> + *   YCbCr 4:4:4 output format (ie. not subsampled)
> + * @DRM_OUTPUT_COLOR_FORMAT_YCBCR422:
> + *   YCbCr 4:2:2 output format (ie. with horizontal subsampling)
> + * @DRM_OUTPUT_COLOR_FORMAT_YCBCR420:
> + *   YCbCr 4:2:0 output format (ie. with horizontal and vertical subsampling)
> + */
> +enum drm_output_color_format {
> +	DRM_OUTPUT_COLOR_FORMAT_RGB444 = 0,
> +	DRM_OUTPUT_COLOR_FORMAT_YCBCR444,
> +	DRM_OUTPUT_COLOR_FORMAT_YCBCR422,
> +	DRM_OUTPUT_COLOR_FORMAT_YCBCR420,
> +};
> +
>  /**
>   * enum drm_bus_flags - bus_flags info for &drm_display_info
>   *
>   * This enum defines signal polarities and clock edge information for signals on
>   * a bus as bitmask flags.
> @@ -697,14 +722,14 @@ struct drm_display_info {
>  	/**
>  	 * @subpixel_order: Subpixel order of LCD panels.
>  	 */
>  	enum subpixel_order subpixel_order;
>  
> -#define DRM_COLOR_FORMAT_RGB444		(1<<0)
> -#define DRM_COLOR_FORMAT_YCBCR444	(1<<1)
> -#define DRM_COLOR_FORMAT_YCBCR422	(1<<2)
> -#define DRM_COLOR_FORMAT_YCBCR420	(1<<3)
> +#define DRM_COLOR_FORMAT_RGB444		(1 << DRM_OUTPUT_COLOR_FORMAT_RGB444)
> +#define DRM_COLOR_FORMAT_YCBCR444	(1 << DRM_OUTPUT_COLOR_FORMAT_YCBCR444)
> +#define DRM_COLOR_FORMAT_YCBCR422	(1 << DRM_OUTPUT_COLOR_FORMAT_YCBCR422)
> +#define DRM_COLOR_FORMAT_YCBCR420	(1 << DRM_OUTPUT_COLOR_FORMAT_YCBCR420)

Can we move this? Either up, directly after drm_output_color_format, or
down, directly before color_formats.

I think 8d70f395e6cb ("drm: Add support for a panel-orientation
connector property, v6") didn't separate the two on purpose.

>  	/**
>  	 * @panel_orientation: Read only connector property for built-in panels,
>  	 * indicating the orientation of the panel vs the device's casing.
>  	 * drm_connector_init() sets this to DRM_MODE_PANEL_ORIENTATION_UNKNOWN.
> @@ -712,14 +737,15 @@ struct drm_display_info {
>  	 * fb to compensate and gets exported as prop to userspace.
>  	 */
>  	int panel_orientation;
>  
>  	/**
> -	 * @color_formats: HDMI Color formats, selects between RGB and YCrCb
> -	 * modes. Used DRM_COLOR_FORMAT\_ defines, which are _not_ the same ones
> -	 * as used to describe the pixel format in framebuffers, and also don't
> -	 * match the formats in @bus_formats which are shared with v4l.
> +	 * @color_formats: HDMI Color formats, selects between RGB and
> +	 * YCrCb modes. Uses a bitmask of DRM_OUTPUT_COLOR_FORMAT\_

Maybe s/YCrCb/YCbCr/ for consistency?

> +	 * defines, which are _not_ the same ones as used to describe
> +	 * the pixel format in framebuffers, and also don't match the
> +	 * formats in @bus_formats which are shared with v4l.
>  	 */
>  	u32 color_formats;
>  
>  	/**
>  	 * @bus_formats: Pixel data format on the wire, somewhat redundant with

regards
Philipp