On Tuesday, 9 December 2025 15:16:58 Central European Standard Time Maxime Ripard wrote: > On Fri, Nov 28, 2025 at 10:05:41PM +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 <[email protected]> > > --- > > drivers/gpu/drm/display/drm_hdmi_state_helper.c | 12 +++++++++++- > > 1 file changed, 11 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..5da956bdd68c 100644 > > --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c > > +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c > > @@ -649,11 +649,21 @@ hdmi_compute_config(const struct drm_connector > > *connector, > > unsigned int max_bpc = clamp_t(unsigned int, > > conn_state->max_bpc, > > 8, connector->max_bpc); > > + enum hdmi_colorspace hdmi_colorspace = > > + drm_color_format_to_hdmi_colorspace(conn_state->color_format); > > int ret; > > > > ret = hdmi_compute_format_bpc(connector, conn_state, mode, max_bpc, > > - HDMI_COLORSPACE_RGB); > > + hdmi_colorspace); > > if (ret) { > > + /* If a color format was explicitly requested, don't fall back > > */ > > + if (conn_state->color_format) { > > + drm_dbg_kms(connector->dev, > > + "Explicitly set color format '%s' doesn't > > work.\n", > > + > > drm_get_color_format_name(conn_state->color_format)); > > + return ret; > > + } > > + > > I think the following would be more readable: > > > if (conn_state->color_format && conn_state->color_format != > DRM_COLOR_FORMAT_AUTO) { > enum hdmi_colorspace hdmi_colorspace = > drm_color_format_to_hdmi_colorspace(conn_state->color_format); > > return hdmi_compute_format_bpc(connector, conn_state, mode, max_bpc, > hdmi_colorspace) > } > > ret = ret = hdmi_compute_format_bpc(connector, conn_state, mode, max_bpc, > HDMI_COLORSPACE_RGB); > > ...
I think I get what you mean: if conn_state->color_format is set, return directly, instead of bailing out in the fallback path. I can do that. However, `conn_state->color_format && conn_state->color_format != DRM_COLOR_FORMAT_AUTO` is redundant now as of v5 since DRM_COLOR_FORMAT_AUTO is 0 (and I use its falsy nature in many other places, and don't intend to stop doing that). > > Maxime >
