On Wed, 25 Mar 2026 at 13:43, Ville Syrjälä <[email protected]> wrote: > > On Wed, Mar 25, 2026 at 12:49:19PM +0000, Dave Stevenson wrote: > > On Tue, 24 Mar 2026 at 16:02, Nicolas Frattaroli > > <[email protected]> wrote: > > > > > > Add a new general DRM property named "color format" which can be used by > > > userspace to request the display driver to output a particular color > > > format. > > > > > > Possible options are: > > > - auto (setup by default, driver internally picks the color format) > > > - rgb > > > - ycbcr444 > > > - ycbcr422 > > > - ycbcr420 > > > > > > Drivers should advertise from this list which formats they support. > > > Together with this list and EDID data from the sink we should be able > > > to relay a list of usable color formats to users to pick from. > > > > > > Co-developed-by: Werner Sembach <[email protected]> > > > Signed-off-by: Werner Sembach <[email protected]> > > > Co-developed-by: Andri Yngvason <[email protected]> > > > Signed-off-by: Andri Yngvason <[email protected]> > > > Signed-off-by: Marius Vlad <[email protected]> > > > Reviewed-by: Maxime Ripard <[email protected]> > > > Signed-off-by: Nicolas Frattaroli <[email protected]> > > > --- > > > drivers/gpu/drm/drm_atomic_helper.c | 5 ++ > > > drivers/gpu/drm/drm_atomic_uapi.c | 11 ++++ > > > drivers/gpu/drm/drm_connector.c | 108 > > > ++++++++++++++++++++++++++++++++++++ > > > include/drm/drm_connector.h | 104 > > > ++++++++++++++++++++++++++++++++++ > > > 4 files changed, 228 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > > > b/drivers/gpu/drm/drm_atomic_helper.c > > > index 26953ed6b53e..b7753454b777 100644 > > > --- a/drivers/gpu/drm/drm_atomic_helper.c > > > +++ b/drivers/gpu/drm/drm_atomic_helper.c > > > @@ -737,6 +737,11 @@ drm_atomic_helper_check_modeset(struct drm_device > > > *dev, > > > if (old_connector_state->max_requested_bpc != > > > new_connector_state->max_requested_bpc) > > > new_crtc_state->connectors_changed = true; > > > + > > > + if (old_connector_state->color_format != > > > + new_connector_state->color_format) > > > + new_crtc_state->connectors_changed = true; > > > + > > > } > > > > > > if (funcs->atomic_check) > > > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > > > b/drivers/gpu/drm/drm_atomic_uapi.c > > > index 5bd5bf6661df..dee510c85e59 100644 > > > --- a/drivers/gpu/drm/drm_atomic_uapi.c > > > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > > > @@ -935,6 +935,15 @@ static int drm_atomic_connector_set_property(struct > > > drm_connector *connector, > > > state->privacy_screen_sw_state = val; > > > } else if (property == connector->broadcast_rgb_property) { > > > state->hdmi.broadcast_rgb = val; > > > + } else if (property == connector->color_format_property) { > > > + if (val > INT_MAX || > > > !drm_connector_color_format_valid(val)) { > > > + drm_dbg_atomic(connector->dev, > > > + "[CONNECTOR:%d:%s] unknown color > > > format %llu\n", > > > + connector->base.id, > > > connector->name, val); > > > + return -EINVAL; > > > + } > > > + > > > + state->color_format = val; > > > } else if (connector->funcs->atomic_set_property) { > > > return connector->funcs->atomic_set_property(connector, > > > state, property, val); > > > @@ -1020,6 +1029,8 @@ drm_atomic_connector_get_property(struct > > > drm_connector *connector, > > > *val = state->privacy_screen_sw_state; > > > } else if (property == connector->broadcast_rgb_property) { > > > *val = state->hdmi.broadcast_rgb; > > > + } else if (property == connector->color_format_property) { > > > + *val = state->color_format; > > > } else if (connector->funcs->atomic_get_property) { > > > return connector->funcs->atomic_get_property(connector, > > > state, property, val); > > > diff --git a/drivers/gpu/drm/drm_connector.c > > > b/drivers/gpu/drm/drm_connector.c > > > index 47dc53c4a738..e848374dee0b 100644 > > > --- a/drivers/gpu/drm/drm_connector.c > > > +++ b/drivers/gpu/drm/drm_connector.c > > > @@ -1388,6 +1388,18 @@ static const u32 hdmi_colorspaces = > > > BIT(DRM_MODE_COLORIMETRY_DCI_P3_RGB_D65) | > > > BIT(DRM_MODE_COLORIMETRY_DCI_P3_RGB_THEATER); > > > > > > +static const u32 hdmi_colorformats = > > > + BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444) | > > > + BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) | > > > + BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422) | > > > + BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420); > > > + > > > +static const u32 dp_colorformats = > > > + BIT(DRM_OUTPUT_COLOR_FORMAT_RGB444) | > > > + BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR444) | > > > + BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR422) | > > > + BIT(DRM_OUTPUT_COLOR_FORMAT_YCBCR420); > > > + > > > /* > > > * As per DP 1.4a spec, 2.2.5.7.5 VSC SDP Payload for Pixel > > > Encoding/Colorimetry > > > * Format Table 2-120 > > > @@ -2940,6 +2952,102 @@ int > > > drm_connector_attach_colorspace_property(struct drm_connector *connector) > > > } > > > EXPORT_SYMBOL(drm_connector_attach_colorspace_property); > > > > > > +/** > > > + * drm_connector_attach_color_format_property - create and attach color > > > format property > > > + * @connector: connector to create the color format property on > > > + * @supported_color_formats: bitmask of bit-shifted &enum > > > drm_output_color_format > > > + * values the connector supports > > > + * > > > + * Called by a driver to create a color format property. The property is > > > + * attached to the connector automatically on success. > > > + * > > > + * @supported_color_formats should only include color formats the > > > connector > > > + * type can actually support. > > > + * > > > + * Returns: > > > + * 0 on success, negative errno on error > > > + */ > > > +int drm_connector_attach_color_format_property(struct drm_connector > > > *connector, > > > + unsigned long > > > supported_color_formats) > > > +{ > > > + struct drm_device *dev = connector->dev; > > > + struct drm_prop_enum_list > > > enum_list[DRM_CONNECTOR_COLOR_FORMAT_COUNT]; > > > + unsigned int i = 0; > > > + unsigned long fmt; > > > + > > > + if (connector->color_format_property) > > > + return 0; > > > + > > > + if (!supported_color_formats) { > > > + drm_err(dev, "No supported color formats provided on > > > [CONNECTOR:%d:%s]\n", > > > + connector->base.id, connector->name); > > > + return -EINVAL; > > > + } > > > + > > > + if (supported_color_formats & > > > ~GENMASK(DRM_OUTPUT_COLOR_FORMAT_COUNT - 1, 0)) { > > > + drm_err(dev, "Unknown color formats provided on > > > [CONNECTOR:%d:%s]\n", > > > + connector->base.id, connector->name); > > > + return -EINVAL; > > > + } > > > + > > > + switch (connector->connector_type) { > > > + case DRM_MODE_CONNECTOR_HDMIA: > > > + case DRM_MODE_CONNECTOR_HDMIB: > > > + if (supported_color_formats & ~hdmi_colorformats) { > > > + drm_err(dev, "Color formats not allowed for HDMI > > > on [CONNECTOR:%d:%s]\n", > > > + connector->base.id, connector->name); > > > + return -EINVAL; > > > + } > > > + break; > > > + case DRM_MODE_CONNECTOR_DisplayPort: > > > + case DRM_MODE_CONNECTOR_eDP: > > > + if (supported_color_formats & ~dp_colorformats) { > > > + drm_err(dev, "Color formats not allowed for DP on > > > [CONNECTOR:%d:%s]\n", > > > + connector->base.id, connector->name); > > > + return -EINVAL; > > > + } > > > + break; > > > + } > > > + > > > + enum_list[0].name = "AUTO"; > > > + enum_list[0].type = DRM_CONNECTOR_COLOR_FORMAT_AUTO; > > > + > > > + for_each_set_bit(fmt, &supported_color_formats, > > > DRM_OUTPUT_COLOR_FORMAT_COUNT) { > > > + switch (fmt) { > > > + case DRM_OUTPUT_COLOR_FORMAT_RGB444: > > > + enum_list[++i].type = > > > DRM_CONNECTOR_COLOR_FORMAT_RGB444; > > > + break; > > > + case DRM_OUTPUT_COLOR_FORMAT_YCBCR444: > > > + enum_list[++i].type = > > > DRM_CONNECTOR_COLOR_FORMAT_YCBCR444; > > > + break; > > > + case DRM_OUTPUT_COLOR_FORMAT_YCBCR422: > > > + enum_list[++i].type = > > > DRM_CONNECTOR_COLOR_FORMAT_YCBCR422; > > > + break; > > > + case DRM_OUTPUT_COLOR_FORMAT_YCBCR420: > > > + enum_list[++i].type = > > > DRM_CONNECTOR_COLOR_FORMAT_YCBCR420; > > > + break; > > > + default: > > > + drm_warn(dev, "Unknown supported format %ld on > > > [CONNECTOR:%d:%s]\n", > > > + fmt, connector->base.id, > > > connector->name); > > > + continue; > > > + } > > > + enum_list[i].name = > > > drm_hdmi_connector_get_output_format_name(fmt); > > > + } > > > + > > > + connector->color_format_property = > > > + drm_property_create_enum(dev, DRM_MODE_PROP_ENUM, "color > > > format", > > > + enum_list, i + 1); > > > + > > > + if (!connector->color_format_property) > > > + return -ENOMEM; > > > + > > > + drm_object_attach_property(&connector->base, > > > connector->color_format_property, > > > + DRM_CONNECTOR_COLOR_FORMAT_AUTO); > > > + > > > + return 0; > > > +} > > > +EXPORT_SYMBOL(drm_connector_attach_color_format_property); > > > + > > > /** > > > * drm_connector_atomic_hdr_metadata_equal - checks if the hdr metadata > > > changed > > > * @old_state: old connector state to compare > > > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h > > > index af8b92d2d5b7..bd549f912b76 100644 > > > --- a/include/drm/drm_connector.h > > > +++ b/include/drm/drm_connector.h > > > @@ -571,14 +571,102 @@ enum drm_colorspace { > > > * YCbCr 4:2:2 output format (ie. with horizontal subsampling) > > > * @DRM_OUTPUT_COLOR_FORMAT_YCBCR420: > > > * YCbCr 4:2:0 output format (ie. with horizontal and vertical > > > subsampling) > > > + * @DRM_OUTPUT_COLOR_FORMAT_COUNT: > > > + * Number of valid output color format values in this enum > > > */ > > > enum drm_output_color_format { > > > DRM_OUTPUT_COLOR_FORMAT_RGB444 = 0, > > > DRM_OUTPUT_COLOR_FORMAT_YCBCR444, > > > DRM_OUTPUT_COLOR_FORMAT_YCBCR422, > > > DRM_OUTPUT_COLOR_FORMAT_YCBCR420, > > > + DRM_OUTPUT_COLOR_FORMAT_COUNT, > > > }; > > > > > > +/** > > > + * enum drm_connector_color_format - Connector Color Format Request > > > + * > > > + * This enum, unlike &enum drm_output_color_format, is used to specify > > > requests > > > + * for a specific color format on a connector through the DRM "color > > > format" > > > + * property. The difference is that it has an "AUTO" value to specify > > > that > > > + * no specific choice has been made. > > > + */ > > > +enum drm_connector_color_format { > > > + /** > > > + * @DRM_CONNECTOR_COLOR_FORMAT_AUTO: The driver or display > > > protocol > > > + * helpers should pick a suitable color format. All > > > implementations of a > > > + * specific display protocol must behave the same way with > > > "AUTO", but > > > + * different display protocols do not necessarily have the same > > > "AUTO" > > > + * semantics. > > > + * > > > + * For HDMI, "AUTO" picks RGB, but falls back to YCbCr 4:2:0 if > > > the > > > + * bandwidth required for full-scale RGB is not available, or the > > > mode > > > + * is YCbCr 4:2:0-only, as long as the mode and output both > > > support > > > + * YCbCr 4:2:0. > > > > Is there a reason you propose dropping back to YCbCr 4:2:0 without > > trying YCbCr 4:2:2 first? Minimising the subsampling is surely > > beneficial, and vc4 for one can do 4:2:2 but not 4:2:0. > > On HDMI 4:2:2 is always 12bpc, so it doesn't save any bandwidth > compared to 8bpc 4:4:4.
It does save bandwidth against 10 or 12bpc RGB 4:4:4. Or is the implication that max_bpc = 12 and DRM_CONNECTOR_COLOR_FORMAT_AUTO should drop bpc down to 8 and select RGB in preference to selecting 4:2:2? Dave > -- > Ville Syrjälä > Intel
