Re: [PATCH v9 02/19] drm: Add new general DRM property "color format"

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

On Thursday, 5 March 2026 10:28:11 Central European Standard Time Maxime Ripard wrote:
> Hi,
> 
> On Fri, Feb 27, 2026 at 08:20:07PM +0100, Nicolas Frattaroli wrote:
> > diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> > index 4af91e252fbd..b5bc93856ad1 100644
> > --- a/include/drm/drm_connector.h
> > +++ b/include/drm/drm_connector.h
> > @@ -579,6 +579,91 @@ enum drm_output_color_format {
> >  	DRM_OUTPUT_COLOR_FORMAT_YCBCR420,
> >  };
> >  
> > +/* Do not forget to adjust after modifying &enum drm_output_color_format */
> > +#define DRM_OUTPUT_COLOR_FORMAT_COUNT 4
> 
> Maybe we can put that as the last variant of our enum so we don't have
> to always update it?

That will then cause a bunch of potential warnings for any of the switch
cases that convert from one thing to the other. At least my LSP indicated
so. I guess I can ignore those (and wouldn't be surprised if they were
already ignored by the Makefiles and the LSP just didn't pick up on this
somehow.) I'll do that in the next revision if you're fine with this.
Potentially, I'll have to add a default case to some switch statements
that just does a WARN() or something.

I do wish C had a better way to deal with setting a symbol to the number
of valid enum values without making it itself a valid enum value.

> 
> > +/**
> > + * 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.
> > +	 *
> > +	 * 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,
> > +};
> > +
> > +/* Do not forget to adjust after modifying &enum drm_connector_color_format */
> > +#define DRM_CONNECTOR_COLOR_FORMAT_COUNT 5
> 
> Ditto
> 
> Maxime
>