On Thu, Dec 11, 2025 at 08:42:00PM +0100, Nicolas Frattaroli wrote: > 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.
Yes > 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). Ok, that makes sense. Maxime
signature.asc
Description: PGP signature
