Re: [PATCH 14/14] drm/display: hdmi: Use drm_output_color_format instead of hdmi_colorspace

[Nicolas Frattaroli <nicolas.frattaroli@collabora.com>]

On Friday, 27 February 2026 09:29:05 Central European Standard Time Maxime Ripard wrote:
> Hi
> 
> On Thu, Feb 26, 2026 at 05:24:05PM +0100, Nicolas Frattaroli wrote:
> > On Tuesday, 24 February 2026 11:58:53 Central European Standard Time Maxime Ripard wrote:
> > > The hdmi_colorspace enum was defined to represent the colorspace value
> > > of the HDMI infoframes. It was later used by some HDMI drivers to
> > > express the output format they should be setting up.
> > > 
> > > During the introduction of the HDMI helpers, it then was used to
> > > represent it in the drm_connector_hdmi_state structure.
> > > 
> > > However, it's always been somewhat redundant with the DRM_COLOR_FORMAT_*
> > > defines, and now with the drm_output_color_format enum. Let's
> > > consolidate around drm_output_color_format in drm_connector_hdmi_state
> > > to facilitate the current effort to provide a global output format
> > > selection mechanism.
> > > 
> > > Signed-off-by: Maxime Ripard <mripard@kernel.org>
> > > ---
> > >  drivers/gpu/drm/bridge/inno-hdmi.c                 |   6 +-
> > >  drivers/gpu/drm/bridge/ite-it6263.c                |   2 +-
> > >  drivers/gpu/drm/display/drm_hdmi_helper.c          |   7 +-
> > >  drivers/gpu/drm/display/drm_hdmi_state_helper.c    |  52 ++++--
> > >  drivers/gpu/drm/drm_bridge.c                       |   2 +-
> > >  drivers/gpu/drm/drm_connector.c                    |  14 +-
> > >  drivers/gpu/drm/mediatek/mtk_hdmi_v2.c             |   8 +-
> > >  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c             |   2 +-
> > >  drivers/gpu/drm/tests/drm_connector_test.c         |  80 ++++-----
> > >  drivers/gpu/drm/tests/drm_hdmi_state_helper_test.c | 182 ++++++++++-----------
> > >  drivers/gpu/drm/vc4/vc4_hdmi.c                     |  18 +-
> > >  drivers/gpu/drm/vc4/vc4_hdmi.h                     |   2 +-
> > >  include/drm/display/drm_hdmi_helper.h              |   3 +-
> > >  include/drm/drm_connector.h                        |   7 +-
> > >  14 files changed, 205 insertions(+), 180 deletions(-)
> > > 
> > > [... snip ...]
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index 4f5b27fab475c7c733622eb8727927571f3fb8fe..171cd495976a3e16f201fd339d3d42a09dc3b63f 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -589,14 +589,14 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
> > >  
> > >  	if (!(connector_type == DRM_MODE_CONNECTOR_HDMIA ||
> > >  	      connector_type == DRM_MODE_CONNECTOR_HDMIB))
> > >  		return -EINVAL;
> > >  
> > > -	if (!supported_formats || !(supported_formats & BIT(HDMI_COLORSPACE_RGB)))
> > > +	if (!supported_formats || !(supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444)))
> > >  		return -EINVAL;
> > >  
> > > -	if (connector->ycbcr_420_allowed != !!(supported_formats & BIT(HDMI_COLORSPACE_YUV420)))
> > > +	if (connector->ycbcr_420_allowed != !!(supported_formats & BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420)))
> > >  		return -EINVAL;
> > 
> > I don't think this will work as-is. drm_bridge_connector_init calls this
> > function assuming hdmi_colorspace bitmasks in supported_formats.
> 
> Yeah, you're right I missed the conversion in drm_bridge_connector_init.
> It should be fixed now.
> 
> > This may have slipped through the conversion; the synopsys dw-hdmi-qp core
> > (separate from synopsys dw-hdmi) also assumes hdmi_colorspace, see e.g.
> > dw_hdmi_qp_plat_data::supported_formats in include/drm/bridge/dw_hdmi_qp.h
> > 
> > So should be a simple fix I hope.
> 
> For this one, did you identify anything more than the comment in
> dw_hdmi_qp_plat_data? I couldn't find any user of HDMI_COLORSPACE_* left
> for the dw_hdmi_qp_plat_data.supported_formats users.

Nope, it's really just the comment. My series will obviously need
adjusting since it makes use of that member, but that's part of my
rebase. So far, there don't seem to be any users that set the member,
so it's just the comment documenting it.

Kind regards,
Nicolas Frattaroli
 
> Maxime
>