Re: [PATCH v3 09/17] drm/amd/display: Register Colorspace property for DP and HDMI

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 4446 bytes --]

On Fri, 17 Mar 2023 01:01:38 +0200
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> On Thu, Mar 16, 2023 at 10:13:54PM +0100, Sebastian Wick wrote:
> > On Thu, Mar 16, 2023 at 1:35 PM Ville Syrjälä
> > <ville.syrjala@linux.intel.com> wrote:  
> > >
> > > On Thu, Mar 16, 2023 at 01:34:49PM +0200, Pekka Paalanen wrote:  
> > > > On Thu, 16 Mar 2023 12:47:51 +0200
> > > > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > > >  
> > > > > On Thu, Mar 16, 2023 at 12:07:01PM +0200, Pekka Paalanen wrote:  
> > > > > > On Thu, 16 Mar 2023 11:50:27 +0200
> > > > > > Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > > > > >  
> > > > > > > On Thu, Mar 16, 2023 at 01:37:24AM +0100, Sebastian Wick wrote:  
> > > > > > > > On Tue, Mar 7, 2023 at 4:12 PM Harry Wentland <harry.wentland@amd.com> wrote:  
> > > > > > > > >
> > > > > > > > > We want compositors to be able to set the output
> > > > > > > > > colorspace on DP and HDMI outputs, based on the
> > > > > > > > > caps reported from the receiver via EDID.  
> > > > > > > >
> > > > > > > > About that... The documentation says that user space has to check the
> > > > > > > > EDID for what the sink actually supports. So whatever is in
> > > > > > > > supported_colorspaces is just what the driver/hardware is able to set
> > > > > > > > but doesn't actually indicate that the sink supports it.
> > > > > > > >
> > > > > > > > So the only way to enable bt2020 is by checking if the sink supports
> > > > > > > > both RGB and YUV variants because both could be used by the driver.
> > > > > > > > Not great at all. Something to remember for the new property.  
> > > > > > >
> > > > > > > Hmm. I wonder if that's even legal... Looks like maybe it
> > > > > > > is since I can't immediately spot anything in CTA-861 to
> > > > > > > forbid it :/  
> > > > > >
> > > > > > Wouldn't the driver do the same EDID check before choosing whether it
> > > > > > uses RGB or YCbCr signalling?  
> > > > >
> > > > > I suppose it could. The modeset would then fail, which is perhaps  
> > > >
> > > > Could? What are they missing?  
> > >
> > > The fact that the new property that also affects the rgb->ycbcr matrix
> > > doesn't even exist?  
> > 
> > I think the question was about the current Colorspace property.

Yes.

We need to be able to set ColourPrimaries infoframe field for the sink.
Only userspace knows what ColourPrimaries it uses, and the driver has
no need to care at all, other than tell the sink what we have.

When a driver chooses to use YCbCr, it needs to use the
MatrixCoefficients the sink expects.

If we send the infoframe to the sink telling the signal uses BT.2020
ColourPrimaries, does that same bit pattern also tell the sink we are
using the BT.2020 NCL MatrixCoefficients if the driver chooses YCbCr?

Do drivers actually use BT.2020 NCL MatrixCoefficients in that case?

If they don't, then YCbCr BT.2020 has never worked, which is another
nail in the coffin for "Colorspace" property. But it still means that
RGB BT.2020 may have worked correctly, and then drivers would regress
if they started picking YCbCr for any reason where they previously used
RGB.

> > > >
> > > > I mean, drivers are already automatically choosing between RGB and YCbCr
> > > > signalling based on e.g. available bandwidth. Surely they already will
> > > > not attempt to send a signal format to a monitor that does not say it
> > > > supports that?  
> > 
> > That's exactly what they do. The drivers don't check the EDID for the
> > colorimetry the sink supports and the responsibility is punted off to
> > user space.

I suspect there are two different things:

- which of RGB, YCbCr 4:4:4, YCbCr 4:2:0 can the sink take
- the supported MatrixCoefficients for each of the YCbCr

Surely drivers are already checking the former point?

I'm not surprised if they are not checking the latter point, but they
do need to, because it is the driver making the choice between RGB and
some YCbCr.

> > >
> > > We just signal default/bt.709 colorimetry. There is nothing to
> > > check for those IIRC.  
> > 
> > You do support bt.2020, no?  
> 
> Not for rgb->ycbcr conversion.

I see there is confusion about what "colorimetry" is.

Please, let's use the H.273 terminology to make the difference clear
between ColourPrimaries and MatrixCoefficients.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]