1. Patchew
  2. linux
  3. [PATCH v4 00/10] Add new general DRM property "color format"
  4. View patch

[PATCH v4 07/10] drm/display: hdmi-state-helper: Act on color format DRM property

Nicolas Frattaroli posted 10 patches 2 weeks ago
There is a newer version of this series
[PATCH v4 07/10] drm/display: hdmi-state-helper: Act on color format DRM property
Posted by Nicolas Frattaroli 2 weeks ago 
With the introduction of the "color format" DRM property, which allows
userspace to request a specific color format, the HDMI state helper
should implement this.

Implement it by checking whether the property is set and set to
something other than auto. If so, pass the requested color format, and
otherwise set RGB.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/display/drm_hdmi_state_helper.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index a561f124be99..add0d51fce33 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -650,9 +650,15 @@ hdmi_compute_config(const struct drm_connector *connector,
 				       conn_state->max_bpc,
 				       8, connector->max_bpc);
 	int ret;
+	enum hdmi_colorspace hdmi_colorspace;
+
+	if (conn_state->color_format && conn_state->color_format != DRM_COLOR_FORMAT_AUTO)
+		hdmi_colorspace = color_format_to_hdmi_colorspace(conn_state->color_format);
+	else
+		hdmi_colorspace = HDMI_COLORSPACE_RGB;
 
 	ret = hdmi_compute_format_bpc(connector, conn_state, mode, max_bpc,
-				      HDMI_COLORSPACE_RGB);
+				      hdmi_colorspace);
 	if (ret) {
 		if (connector->ycbcr_420_allowed) {
 			ret = hdmi_compute_format_bpc(connector, conn_state,

-- 
2.51.2
Re: [PATCH v4 07/10] drm/display: hdmi-state-helper: Act on color format DRM property
Posted by Maxime Ripard 1 week, 5 days ago 
Hi,

On Mon, Nov 17, 2025 at 08:11:51PM +0100, Nicolas Frattaroli wrote:
> With the introduction of the "color format" DRM property, which allows
> userspace to request a specific color format, the HDMI state helper
> should implement this.
> 
> Implement it by checking whether the property is set and set to
> something other than auto. If so, pass the requested color format, and
> otherwise set RGB.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index a561f124be99..add0d51fce33 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> @@ -650,9 +650,15 @@ hdmi_compute_config(const struct drm_connector *connector,
>  				       conn_state->max_bpc,
>  				       8, connector->max_bpc);
>  	int ret;
> +	enum hdmi_colorspace hdmi_colorspace;
> +
> +	if (conn_state->color_format && conn_state->color_format != DRM_COLOR_FORMAT_AUTO)
> +		hdmi_colorspace = color_format_to_hdmi_colorspace(conn_state->color_format);
> +	else
> +		hdmi_colorspace = HDMI_COLORSPACE_RGB;
>  
>  	ret = hdmi_compute_format_bpc(connector, conn_state, mode, max_bpc,
> -				      HDMI_COLORSPACE_RGB);
> +				      hdmi_colorspace);

I don't think we want the fallback to yuv420 for anything but auto, so
I'd rather have something like

if (conn_state->color_format != DRM_COLOR_FORMAT_AUTO)
   return hdmi_compute_format_bpc(connector, conn_state, mode, max_bpc,
                                  color_format_to_hdmi_colorspace(conn_state->color_format))

We'll also need unit tests.

Maxime
Re: [PATCH v4 07/10] drm/display: hdmi-state-helper: Act on color format DRM property
Posted by Nicolas Frattaroli 1 week, 5 days ago 
On Wednesday, 19 November 2025 10:09:12 Central European Standard Time Maxime Ripard wrote:
> Hi,
> 
> On Mon, Nov 17, 2025 at 08:11:51PM +0100, Nicolas Frattaroli wrote:
> > With the introduction of the "color format" DRM property, which allows
> > userspace to request a specific color format, the HDMI state helper
> > should implement this.
> > 
> > Implement it by checking whether the property is set and set to
> > something other than auto. If so, pass the requested color format, and
> > otherwise set RGB.
> > 
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > index a561f124be99..add0d51fce33 100644
> > --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > @@ -650,9 +650,15 @@ hdmi_compute_config(const struct drm_connector *connector,
> >  				       conn_state->max_bpc,
> >  				       8, connector->max_bpc);
> >  	int ret;
> > +	enum hdmi_colorspace hdmi_colorspace;
> > +
> > +	if (conn_state->color_format && conn_state->color_format != DRM_COLOR_FORMAT_AUTO)
> > +		hdmi_colorspace = color_format_to_hdmi_colorspace(conn_state->color_format);
> > +	else
> > +		hdmi_colorspace = HDMI_COLORSPACE_RGB;
> >  
> >  	ret = hdmi_compute_format_bpc(connector, conn_state, mode, max_bpc,
> > -				      HDMI_COLORSPACE_RGB);
> > +				      hdmi_colorspace);
> 
> I don't think we want the fallback to yuv420 for anything but auto, so

Okay. Changing all the non-hdmi-state-helper drivers (amdgpu, i915)
to do this as well would require some more work however, especially
in the case of amdgpu where the code flow is not always obvious.

> I'd rather have something like
> 
> if (conn_state->color_format != DRM_COLOR_FORMAT_AUTO)
>    return hdmi_compute_format_bpc(connector, conn_state, mode, max_bpc,
>                                   color_format_to_hdmi_colorspace(conn_state->color_format))
> 
> We'll also need unit tests.

Sure, am I guessing correctly that they'd go in
drm_hdmi_state_helper_test.c?

> Maxime
>
Re: [PATCH v4 07/10] drm/display: hdmi-state-helper: Act on color format DRM property
Posted by Maxime Ripard 1 week, 4 days ago 
On Wed, Nov 19, 2025 at 01:41:18PM +0100, Nicolas Frattaroli wrote:
> On Wednesday, 19 November 2025 10:09:12 Central European Standard Time Maxime Ripard wrote:
> > Hi,
> > 
> > On Mon, Nov 17, 2025 at 08:11:51PM +0100, Nicolas Frattaroli wrote:
> > > With the introduction of the "color format" DRM property, which allows
> > > userspace to request a specific color format, the HDMI state helper
> > > should implement this.
> > > 
> > > Implement it by checking whether the property is set and set to
> > > something other than auto. If so, pass the requested color format, and
> > > otherwise set RGB.
> > > 
> > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > > ---
> > >  drivers/gpu/drm/display/drm_hdmi_state_helper.c | 8 +++++++-
> > >  1 file changed, 7 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > > index a561f124be99..add0d51fce33 100644
> > > --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > > +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> > > @@ -650,9 +650,15 @@ hdmi_compute_config(const struct drm_connector *connector,
> > >  				       conn_state->max_bpc,
> > >  				       8, connector->max_bpc);
> > >  	int ret;
> > > +	enum hdmi_colorspace hdmi_colorspace;
> > > +
> > > +	if (conn_state->color_format && conn_state->color_format != DRM_COLOR_FORMAT_AUTO)
> > > +		hdmi_colorspace = color_format_to_hdmi_colorspace(conn_state->color_format);
> > > +	else
> > > +		hdmi_colorspace = HDMI_COLORSPACE_RGB;
> > >  
> > >  	ret = hdmi_compute_format_bpc(connector, conn_state, mode, max_bpc,
> > > -				      HDMI_COLORSPACE_RGB);
> > > +				      hdmi_colorspace);
> > 
> > I don't think we want the fallback to yuv420 for anything but auto, so
> 
> Okay. Changing all the non-hdmi-state-helper drivers (amdgpu, i915)
> to do this as well would require some more work however, especially
> in the case of amdgpu where the code flow is not always obvious.

Yeah, I think we want to be consistent here, the whole point of the HDMI
state helpers was to be consistently consistent with Intel's behaviour
anyway :)

> > I'd rather have something like
> > 
> > if (conn_state->color_format != DRM_COLOR_FORMAT_AUTO)
> >    return hdmi_compute_format_bpc(connector, conn_state, mode, max_bpc,
> >                                   color_format_to_hdmi_colorspace(conn_state->color_format))
> > 
> > We'll also need unit tests.
> 
> Sure, am I guessing correctly that they'd go in
> drm_hdmi_state_helper_test.c?

Yes

Maxime

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.