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 <[email protected]> > --- > 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 > [... snip ...] > > @@ -1424,25 +1424,25 @@ drm_hdmi_connector_get_broadcast_rgb_name(enum > drm_hdmi_broadcast_rgb broadcast_ > return broadcast_rgb_names[broadcast_rgb].name; > } > EXPORT_SYMBOL(drm_hdmi_connector_get_broadcast_rgb_name); > > static const char * const output_format_str[] = { > - [HDMI_COLORSPACE_RGB] = "RGB", > - [HDMI_COLORSPACE_YUV420] = "YUV 4:2:0", > - [HDMI_COLORSPACE_YUV422] = "YUV 4:2:2", > - [HDMI_COLORSPACE_YUV444] = "YUV 4:4:4", > + [DRM_OUTPUT_COLOR_FORMAT_RGB444] = "RGB", > + [DRM_OUTPUT_COLOR_FORMAT_YCBCR420] = "YUV 4:2:0", > + [DRM_OUTPUT_COLOR_FORMAT_YCBCR422] = "YUV 4:2:2", > + [DRM_OUTPUT_COLOR_FORMAT_YCBCR444] = "YUV 4:4:4", > }; > > /* > * drm_hdmi_connector_get_output_format_name() - Return a string for HDMI > connector output format > * @fmt: Output format to compute name of > * > * Returns: the name of the output format, or NULL if the type is not > * valid. > */ > const char * > -drm_hdmi_connector_get_output_format_name(enum hdmi_colorspace fmt) > +drm_hdmi_connector_get_output_format_name(enum drm_output_color_format fmt) > { > if (fmt >= ARRAY_SIZE(output_format_str)) > return NULL;
Almost unrelated nit: we might want to also `fmt < 0 ||` here, since the base type of enums is a signed int. I almost caught myself using this function as a way to quickly check if a supplied color format property from userspace was valid, which would've had some unpleasant consequences for the kernel's memory. Alternatively, I make my own switch-case based "is this a valid enum value" function. I probably should do that actually, but my laziness lead me to make sure we only have a single source of truth of what counts as a valid enum value. > > return output_format_str[fmt]; > [... snip ...] Thanks for your work on this series, I was afraid of getting rid of DRM_COLOR_FORMAT_* myself because I suspected it was going to be a fairly large refactor across every driver. Guess I was both right and wrong: it touches many files, but all the replacements are fairly simple. :) Kind regards, Nicolas Frattaroli
