Re: [PATCH v11 03/22] drm: Add new general DRM property "color format"

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

On Tuesday, 24 March 2026 18:00:45 Central European Standard Time Ville Syrjälä wrote:
> On Tue, Mar 24, 2026 at 05:01:07PM +0100, Nicolas Frattaroli wrote:
> > +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.
> > +	 *
> > +	 * For display protocols other than HDMI, the recursive bridge chain
> > +	 * format selection picks the first chain of bridge formats that works,
> > +	 * as has already been the case before the introduction of the "color
> > +	 * format" property. Non-HDMI bridges should therefore either sort their
> > +	 * bus output formats by preference, or agree on a unified auto format
> > +	 * selection logic that's implemented in a common state helper (like
> > +	 * how HDMI does it).
> > +	 */
> > +	DRM_CONNECTOR_COLOR_FORMAT_AUTO = 0,
> > +
> > +	/**
> > +	 * @DRM_CONNECTOR_COLOR_FORMAT_RGB444: RGB output format
> > +	 */
> > +	DRM_CONNECTOR_COLOR_FORMAT_RGB444,
> > +
> > +	/**
> > +	 * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR444: YCbCr 4:4:4 output format (ie.
> > +	 * not subsampled)
> > +	 */
> > +	DRM_CONNECTOR_COLOR_FORMAT_YCBCR444,
> > +
> > +	/**
> > +	 * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR422: YCbCr 4:2:2 output format (ie.
> > +	 * with horizontal subsampling)
> > +	 */
> > +	DRM_CONNECTOR_COLOR_FORMAT_YCBCR422,
> > +
> > +	/**
> > +	 * @DRM_CONNECTOR_COLOR_FORMAT_YCBCR420: YCbCr 4:2:0 output format (ie.
> > +	 * with horizontal and vertical subsampling)
> > +	 */
> > +	DRM_CONNECTOR_COLOR_FORMAT_YCBCR420,
> 
> Seems like this should document what the quantization range
> should be for each format.
> 

I don't think so? If you want per-component bit depth values,
DRM_FORMAT_* defines would be the appropriate values to use. This
enum is more abstract than that, and is there to communicate
YUV vs. RGB and chroma subsampling, with bit depth being handled
by other properties.

If you mean the factor used for subsampling, then that'd only be
relevant if YCBCR410 was supported where one chroma plane isn't
halved but quartered in resolution. I suspect 4:1:0 will never
be added; no digital display protocol standard supports it to my
knowledge, and hopefully none ever will.

> > +
> > +	/**
> > +	 * @DRM_CONNECTOR_COLOR_FORMAT_COUNT: Number of valid connector color
> > +	 * format values in this enum
> > +	 */
> > +	DRM_CONNECTOR_COLOR_FORMAT_COUNT,
> > +};
> > +
> > +/**
> > + * drm_connector_color_format_valid - Validate drm_connector_color_format value
> > + * @fmt: value to check against all values of &enum drm_connector_color_format
> > + *
> > + * Checks whether the passed in value of @fmt is one of the allowable values in
> > + * &enum drm_connector_color_format.
> > + *
> > + * Returns: %true if it's a valid value for the enum, %false otherwise.
> > + */
> > +static inline bool __pure
> > +drm_connector_color_format_valid(enum drm_connector_color_format fmt)
> > +{
> > +	switch (fmt) {
> > +	case DRM_CONNECTOR_COLOR_FORMAT_AUTO:
> > +	case DRM_CONNECTOR_COLOR_FORMAT_RGB444:
> > +	case DRM_CONNECTOR_COLOR_FORMAT_YCBCR444:
> > +	case DRM_CONNECTOR_COLOR_FORMAT_YCBCR422:
> > +	case DRM_CONNECTOR_COLOR_FORMAT_YCBCR420:
> > +		return true;
> > +	default:
> > +		return false;
> > +	}
> > +}
> > +
> >  const char *
> >  drm_hdmi_connector_get_output_format_name(enum drm_output_color_format fmt);
> >  
> > @@ -1129,6 +1217,13 @@ struct drm_connector_state {
> >  	 */
> >  	enum drm_colorspace colorspace;
> >  
> > +	/**
> > +	 * @color_format: State variable for Connector property to request
> > +	 * color format change on Sink. This is most commonly used to switch
> > +	 * between RGB to YUV and vice-versa.
> > +	 */
> > +	enum drm_connector_color_format color_format;
> > +
> >  	/**
> >  	 * @writeback_job: Writeback job for writeback connectors
> >  	 *
> > @@ -2127,6 +2222,12 @@ struct drm_connector {
> >  	 */
> >  	struct drm_property *colorspace_property;
> >  
> > +	/**
> > +	 * @color_format_property: Connector property to set the suitable
> > +	 * color format supported by the sink.
> > +	 */
> > +	struct drm_property *color_format_property;
> > +
> >  	/**
> >  	 * @path_blob_ptr:
> >  	 *
> > @@ -2610,6 +2711,9 @@ bool drm_connector_has_possible_encoder(struct drm_connector *connector,
> >  					struct drm_encoder *encoder);
> >  const char *drm_get_colorspace_name(enum drm_colorspace colorspace);
> >  
> > +int drm_connector_attach_color_format_property(struct drm_connector *connector,
> > +					       unsigned long supported_color_formats);
> > +
> >  /**
> >   * drm_for_each_connector_iter - connector_list iterator macro
> >   * @connector: &struct drm_connector pointer used as cursor
> > 
> 
>