Re: [PATCH v2 4/7] drm/connector: hdmi: Use YUV420 output format as an RGB fallback

Dmitry Baryshkov
2025-03-14T15:00:23Z

On Fri, Mar 14, 2025 at 02:47:53PM +0100, Maxime Ripard wrote:
> On Tue, Mar 11, 2025 at 09:46:39PM +0200, Dmitry Baryshkov wrote:
> > On Tue, Mar 11, 2025 at 04:55:17PM +0100, Maxime Ripard wrote:
> > > Hi,
> > >
> > > I think the first thing we need to address is that we will need to
> > > differentiate between HDMI 1.4 devices and HDMI 2.0.
> > >
> > > It applies to YUV420, which is HDMI 2.0-only, and I guess your patches
> > > are good enough if you consider YUV420 support only, but scrambler setup
> > > for example is a thing we want to support in that infrastructure
> > > eventually, and is conditioned on HDMI 2.0 as well.
> > >
> > > On Tue, Mar 11, 2025 at 12:57:36PM +0200, Cristian Ciocaltea wrote:
> > > > Try to make use of YUV420 when computing the best output format and
> > > > RGB cannot be supported for any of the available color depths.
> > > >
> > > > Signed-off-by: Cristian Ciocaltea <cristian.ciocaltea@collabora.com>
> > > > ---
> > > > drivers/gpu/drm/display/drm_hdmi_state_helper.c | 69 +++++++++++++------------
> > > > 1 file changed, 35 insertions(+), 34 deletions(-)
> > > >
> >
> > [...]
> >
> > > > return -EINVAL;
> > > > }
> > > >
> > > > +static int
> > > > +hdmi_compute_config(const struct drm_connector *connector,
> > > > + struct drm_connector_state *conn_state,
> > > > + const struct drm_display_mode *mode)
> > > > +{
> > > > + unsigned int max_bpc = clamp_t(unsigned int,
> > > > + conn_state->max_bpc,
> > > > + 8, connector->max_bpc);
> > > > + int ret;
> > > > +
> > > > + ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
> > > > + HDMI_COLORSPACE_RGB);
> > > > + if (!ret)
> > > > + return 0;
> > > > +
> > > > + if (connector->ycbcr_420_allowed)
> > > > + ret = hdmi_try_format(connector, conn_state, mode, max_bpc,
> > > > + HDMI_COLORSPACE_YUV420);
> > >
> > > I think that's conditioned on a few more things:
> > > - That the driver supports HDMI 2.0
> >
> > Isn't that included into connector->ycbcr_420_allowed? I'd expect that
> > HDMI 1.4-only drivers don't set that flag.
>
> Yeah, I guess that's one way to do it, but we don't have any way to
> express it at the moment

Yes, we do not have a way to specify that the connector is HDMI 1.x or
2.0. However I think the code that we currently have ensures that the
flag is set if and only if the HDMI Host and all the chain after it
actually supports YUV 420, which would imply HDMI 2.0.

I know that drm_bridge_connector has one deficiency wrt. YCbCr 420 flag:
it is impossible to "inject" YUV420 in the middle of the chain (e.g. DSI
host outputs RGB data, but then comes DSI2HDMI bridge which can convert
RGB data to YUV). I kept that in mind, but I'd like to see an actual
usecase first. And anyway, this currently limits YUV support rather than
enabing it in the unwanted cases.

--
With best wishes
Dmitry