[PATCH v4 03/10] drm: Add enum conversion from/to HDMI_COLORSPACE to DRM_COLOR_FORMAT

Nicolas Frattaroli posted 10 patches 2 weeks ago
There is a newer version of this series
[PATCH v4 03/10] drm: Add enum conversion from/to HDMI_COLORSPACE to DRM_COLOR_FORMAT
Posted by Nicolas Frattaroli 2 weeks ago
From: Marius Vlad <marius.vlad@collabora.com>

This would please the compiler to have a enum transformation from one to
another even though the values are the same. It should also make things
obvious that we use different enums.

Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
---
 drivers/gpu/drm/drm_connector.c | 18 ++++++++++++++++++
 include/drm/drm_connector.h     |  3 +++
 2 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 0ad7be0a2d09..61c077b67ac3 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1384,6 +1384,24 @@ drm_color_format_enum_to_color_format(enum drm_color_format_enum fmt_enum)
 	}
 }
 
+enum hdmi_colorspace
+color_format_to_hdmi_colorspace(enum drm_color_format fmt)
+{
+	switch (fmt) {
+	default:
+	case DRM_COLOR_FORMAT_AUTO:
+	case DRM_COLOR_FORMAT_RGB444:
+		return HDMI_COLORSPACE_RGB;
+	case DRM_COLOR_FORMAT_YCBCR444:
+		return HDMI_COLORSPACE_YUV444;
+	case DRM_COLOR_FORMAT_YCBCR422:
+		return HDMI_COLORSPACE_YUV422;
+	case DRM_COLOR_FORMAT_YCBCR420:
+		return HDMI_COLORSPACE_YUV420;
+	}
+}
+EXPORT_SYMBOL(color_format_to_hdmi_colorspace);
+
 /**
  * drm_get_color_format_name - return a string for color format
  * @colorspace: color format to compute name of
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index a071079fd3ad..e044976c8d76 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2586,6 +2586,9 @@ drm_color_format_to_color_format_enum(enum drm_color_format fmt);
 u32
 drm_color_format_enum_to_color_format(enum drm_color_format_enum fmt_enum);
 
+enum hdmi_colorspace
+color_format_to_hdmi_colorspace(enum drm_color_format fmt);
+
 /**
  * drm_for_each_connector_iter - connector_list iterator macro
  * @connector: &struct drm_connector pointer used as cursor

-- 
2.51.2
Re: [PATCH v4 03/10] drm: Add enum conversion from/to HDMI_COLORSPACE to DRM_COLOR_FORMAT
Posted by Laurent Pinchart 1 week, 5 days ago
On Mon, Nov 17, 2025 at 08:11:47PM +0100, Nicolas Frattaroli wrote:
> From: Marius Vlad <marius.vlad@collabora.com>
> 
> This would please the compiler to have a enum transformation from one to
> another even though the values are the same. It should also make things

The hdmi_colorspace enumerators are defined as (with comments added to
make the values explicit)

enum hdmi_colorspace {
	HDMI_COLORSPACE_RGB,			/* 0 */
	HDMI_COLORSPACE_YUV422,			/* 1 */
	HDMI_COLORSPACE_YUV444,			/* 2 */
	HDMI_COLORSPACE_YUV420,			/* 3 */
	HDMI_COLORSPACE_RESERVED4,		/* 4 */
	HDMI_COLORSPACE_RESERVED5,		/* 5 */
	HDMI_COLORSPACE_RESERVED6,		/* 6 */
	HDMI_COLORSPACE_IDO_DEFINED,		/* 7 */
};

and the DRM color formats as (after patch 02/10)

enum drm_color_format {
	DRM_COLOR_FORMAT_NONE			= 0,		/* 0 */
	DRM_COLOR_FORMAT_RGB444			= (1 << 0),	/* 1 */
	DRM_COLOR_FORMAT_YCBCR422		= (1 << 1),	/* 2 */
	DRM_COLOR_FORMAT_YCBCR444		= (1 << 2),	/* 4 */
	DRM_COLOR_FORMAT_YCBCR420		= (1 << 3),	/* 8 */
	/* auto case, driver will set the color_format */
	DRM_COLOR_FORMAT_AUTO			= (1 << 4),	/* 16 */
};

Contrary to what is stated in the commit message, the values are not the
same. Maybe you confused DRM_COLOR_FORMAT_* with DRM_MODE_COLOR_FORMAT_*
?

> obvious that we use different enums.
> 
> Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 18 ++++++++++++++++++
>  include/drm/drm_connector.h     |  3 +++
>  2 files changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 0ad7be0a2d09..61c077b67ac3 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1384,6 +1384,24 @@ drm_color_format_enum_to_color_format(enum drm_color_format_enum fmt_enum)
>  	}
>  }
>  
> +enum hdmi_colorspace
> +color_format_to_hdmi_colorspace(enum drm_color_format fmt)
> +{
> +	switch (fmt) {
> +	default:
> +	case DRM_COLOR_FORMAT_AUTO:
> +	case DRM_COLOR_FORMAT_RGB444:
> +		return HDMI_COLORSPACE_RGB;
> +	case DRM_COLOR_FORMAT_YCBCR444:
> +		return HDMI_COLORSPACE_YUV444;
> +	case DRM_COLOR_FORMAT_YCBCR422:
> +		return HDMI_COLORSPACE_YUV422;
> +	case DRM_COLOR_FORMAT_YCBCR420:
> +		return HDMI_COLORSPACE_YUV420;
> +	}
> +}
> +EXPORT_SYMBOL(color_format_to_hdmi_colorspace);
> +
>  /**
>   * drm_get_color_format_name - return a string for color format
>   * @colorspace: color format to compute name of
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index a071079fd3ad..e044976c8d76 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -2586,6 +2586,9 @@ drm_color_format_to_color_format_enum(enum drm_color_format fmt);
>  u32
>  drm_color_format_enum_to_color_format(enum drm_color_format_enum fmt_enum);
>  
> +enum hdmi_colorspace
> +color_format_to_hdmi_colorspace(enum drm_color_format fmt);
> +
>  /**
>   * drm_for_each_connector_iter - connector_list iterator macro
>   * @connector: &struct drm_connector pointer used as cursor

-- 
Regards,

Laurent Pinchart
Re: [PATCH v4 03/10] drm: Add enum conversion from/to HDMI_COLORSPACE to DRM_COLOR_FORMAT
Posted by Nicolas Frattaroli 1 week, 5 days ago
On Wednesday, 19 November 2025 05:16:48 Central European Standard Time Laurent Pinchart wrote:
> On Mon, Nov 17, 2025 at 08:11:47PM +0100, Nicolas Frattaroli wrote:
> > From: Marius Vlad <marius.vlad@collabora.com>
> > 
> > This would please the compiler to have a enum transformation from one to
> > another even though the values are the same. It should also make things
> 
> The hdmi_colorspace enumerators are defined as (with comments added to
> make the values explicit)
> 
> enum hdmi_colorspace {
> 	HDMI_COLORSPACE_RGB,			/* 0 */
> 	HDMI_COLORSPACE_YUV422,			/* 1 */
> 	HDMI_COLORSPACE_YUV444,			/* 2 */
> 	HDMI_COLORSPACE_YUV420,			/* 3 */
> 	HDMI_COLORSPACE_RESERVED4,		/* 4 */
> 	HDMI_COLORSPACE_RESERVED5,		/* 5 */
> 	HDMI_COLORSPACE_RESERVED6,		/* 6 */
> 	HDMI_COLORSPACE_IDO_DEFINED,		/* 7 */
> };
> 
> and the DRM color formats as (after patch 02/10)
> 
> enum drm_color_format {
> 	DRM_COLOR_FORMAT_NONE			= 0,		/* 0 */
> 	DRM_COLOR_FORMAT_RGB444			= (1 << 0),	/* 1 */
> 	DRM_COLOR_FORMAT_YCBCR422		= (1 << 1),	/* 2 */
> 	DRM_COLOR_FORMAT_YCBCR444		= (1 << 2),	/* 4 */
> 	DRM_COLOR_FORMAT_YCBCR420		= (1 << 3),	/* 8 */
> 	/* auto case, driver will set the color_format */
> 	DRM_COLOR_FORMAT_AUTO			= (1 << 4),	/* 16 */
> };
> 
> Contrary to what is stated in the commit message, the values are not the
> same. Maybe you confused DRM_COLOR_FORMAT_* with DRM_MODE_COLOR_FORMAT_*
> ?

It's possible the equality got lost during my refactor, but I think
the needless enum conversions between like 4 different things should
be minimised anyway, so I'll look into getting rid of as much of this
as possible.

> 
> > obvious that we use different enums.
> > 
> > Signed-off-by: Marius Vlad <marius.vlad@collabora.com>
> > ---
> >  drivers/gpu/drm/drm_connector.c | 18 ++++++++++++++++++
> >  include/drm/drm_connector.h     |  3 +++
> >  2 files changed, 21 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index 0ad7be0a2d09..61c077b67ac3 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -1384,6 +1384,24 @@ drm_color_format_enum_to_color_format(enum drm_color_format_enum fmt_enum)
> >  	}
> >  }
> >  
> > +enum hdmi_colorspace
> > +color_format_to_hdmi_colorspace(enum drm_color_format fmt)
> > +{
> > +	switch (fmt) {
> > +	default:
> > +	case DRM_COLOR_FORMAT_AUTO:
> > +	case DRM_COLOR_FORMAT_RGB444:
> > +		return HDMI_COLORSPACE_RGB;
> > +	case DRM_COLOR_FORMAT_YCBCR444:
> > +		return HDMI_COLORSPACE_YUV444;
> > +	case DRM_COLOR_FORMAT_YCBCR422:
> > +		return HDMI_COLORSPACE_YUV422;
> > +	case DRM_COLOR_FORMAT_YCBCR420:
> > +		return HDMI_COLORSPACE_YUV420;
> > +	}
> > +}
> > +EXPORT_SYMBOL(color_format_to_hdmi_colorspace);
> > +
> >  /**
> >   * drm_get_color_format_name - return a string for color format
> >   * @colorspace: color format to compute name of
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index a071079fd3ad..e044976c8d76 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -2586,6 +2586,9 @@ drm_color_format_to_color_format_enum(enum drm_color_format fmt);
> >  u32
> >  drm_color_format_enum_to_color_format(enum drm_color_format_enum fmt_enum);
> >  
> > +enum hdmi_colorspace
> > +color_format_to_hdmi_colorspace(enum drm_color_format fmt);
> > +
> >  /**
> >   * drm_for_each_connector_iter - connector_list iterator macro
> >   * @connector: &struct drm_connector pointer used as cursor
> 
>