On Thu, Feb 10, 2022 at 11:03:43AM +0100, Maxime Ripard wrote:
> On Thu, Feb 03, 2022 at 10:07:22PM +0200, Ville Syrjälä wrote:
> > On Thu, Jan 27, 2022 at 03:10:21PM +0100, Maxime Ripard wrote:
> > > +/*
> > > + * Conversion between Full Range RGB and Full Range YUV444 using the
> > > + * BT.709 Colorspace
> > > + *
> > > + * [ -0.117208 -0.394207 0.511416 128 ]
> > > + * [ 0.511416 -0.464524 -0.046891 128 ]
> > > + * [ 0.212639 0.715169 0.072192 0 ]
> > > + *
> > > + * Matrix is signed 2p13 fixed point, with signed 9p6 offsets
> > > + */
> > > +static const u16 vc5_hdmi_csc_full_rgb_to_full_yuv444_bt709[3][4] = {
> >
> > I think YCbCr output is supposed to be limited range. Or at least
> > that was the case with DP. For HDMI/CTA IIRC the spec is super
> > unclear/confusing when it talks about the YCC quantizaton range
> > stuff.
> >
> > Currently i915 will only do limited range BT.709 YCbCr output.
>
> Indeed I haven't been able to find anything in the spec, so I'll just
> drop it, if only to remain consistent across drivers.
>
> > > + { 0xfc41, 0xf364, 0x105e, 0x2000 },
> > > + { 0x105e, 0xf124, 0xfe81, 0x2000 },
> > > + { 0x06ce, 0x16e3, 0x024f, 0x0000 },
> > > +};
> > > +
> > > static void vc5_hdmi_set_csc_coeffs(struct vc4_hdmi *vc4_hdmi,
> > > const u16 coeffs[3][4])
> > > {
> > <snip>
> > > @@ -1323,11 +1534,56 @@ vc4_hdmi_encoder_compute_clock(const struct vc4_hdmi *vc4_hdmi,
> > > return 0;
> > > }
> > >
> > > +static int
> > > +vc4_hdmi_encoder_compute_format(const struct vc4_hdmi *vc4_hdmi,
> > > + struct vc4_hdmi_connector_state *vc4_state,
> > > + const struct drm_display_mode *mode,
> > > + unsigned int bpc)
> > > +{
> > > + struct drm_device *dev = vc4_hdmi->connector.dev;
> > > + const struct drm_connector *connector = &vc4_hdmi->connector;
> > > + const struct drm_display_info *info = &connector->display_info;
> > > + unsigned int format;
> > > +
> > > + drm_dbg(dev, "Trying with an RGB output\n");
> > > +
> > > + format = VC4_HDMI_OUTPUT_RGB;
> > > + if (vc4_hdmi_sink_supports_format_bpc(vc4_hdmi, info, mode, format, bpc)) {
> > > + int ret;
> > > +
> > > + ret = vc4_hdmi_encoder_compute_clock(vc4_hdmi, vc4_state,
> > > + mode, bpc, format);
> > > + if (!ret) {
> > > + vc4_state->output_format = format;
> > > + return 0;
> > > + }
> > > + }
> >
> > You seem to have the bpc vs. format selection in the opposite order to
> > i915. We try to exhaust all RGB color depths first, and only then fall
> > back to YCbCr 4:2:0. The automagic YCbCr 4:2:0 fallback is only
> > really there because without it you may not be able to light up the
> > display at all (at least if you want the higher resolutions).
> >
> > With the current i915 logic 4:2:2 doesn't make any sense since it has
> > the same requirements as 8bpc RGB. Hence we don't even implement
> > YCbCr 4:2:2 (also hw didn't have it until recently).
>
> Our use-case is slightly different, but the basic idea is the same:
> since we support from 8 to 12 bpc, an output to YUV422 output can prove
> useful if we are reading a 12 bpc content and we don't have the
> bandwidth to use RGB.
>
> Thus, when we have a max_bpc of 12, we try RGB and YUV422, if none of
> them work we try RGB in 10 and 8 bpc.
>
> This is indeed a bit different than i915, but it doesn't seem
> fundamentally different to me.
I guess as long as the user gets a semi-correct picture it's
mostly irrelevant.
But at least on i915 gamma correction becomes harder on certain
platforms (GLK) if we do YCbCr output since the LUT is after
the CSC matrix :( Pretty sure we don't handle that case in any
kind of sane way atm. So that's another reason why we prefer
RGB over YCbCr.
--
Ville Syrjälä
Intel
