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.
> There has occasionally been some talk about introducing a new property
> to give the user explicit control over this stuff. If/when that
> happens I guess we might want to revisit the 4:2:2 situation for i915.
This seems to be stalled entirely though, and this approach isn't
incompatible with whatever that would involve.
Maxime
Attachment:
signature.asc
Description: PGP signature
