On Fri, Mar 17, 2023 at 03:35:53PM +0200, Pekka Paalanen wrote: > On Fri, 17 Mar 2023 14:50:40 +0200 > Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote: > > > On Fri, Mar 17, 2023 at 10:53:35AM +0200, Pekka Paalanen wrote: > > > On Fri, 17 Mar 2023 01:01:38 +0200 > > > Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx> 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@xxxxxxx> 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? > > > > No. I think I've repeated this same line a thousand times already: > > The current colorspace property *only* affects the infoframe/msa/sdp, > > nothing else. > > That's the problem. I don't know what that means. > > Does it mean that the sink expects BT.2020 NCL MatrixCoefficients to > have been used? Yes, assuming that is the colorspace property value you picked. > > And the driver will never use BT.2020 NCL MatrixCoefficients in any > circumstances? Correct. > > See the conflict? The sink will be decoding the signal incorrectly, > because we are encoding it with the wrong MatrixCoefficients if the > driver happens to silently choose YCbCr and userspace wants to send > BT2020 ColourPrimaries indicated in the infoframe. Yes. And hence I thought pretty much everyone already agreed that a new property is needed. To make sure we actually understand what we're implementing I think it should start out very minimal. Eg just three values: - unspecified RGB + BT.601 YCbCr - unspecified RGB + BT.709 YCbCr - BT.2020 RGB + BT.2020 YCbCr NCL And that would control: - basic colorimetry metadata transmitted to the sink - MatrixCoefficients used for the potential RGB->YCbCr conversion Transfer funcs, primaries, etc. would be left out (apart from the potential metadata aspect). > > > > > > > > > If they don't, then YCbCr BT.2020 has never worked, which is another > > > nail in the coffin for "Colorspace" property. > > > > That is the same nail we've been talking about all along I thought. > > > > > 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. > > > > The policy has been to use RGB if at all possible. Only falling back > > to YCbCr 4:2:0 if absolutely necessary (eg. EDID says 4:2:0 must > > be used, or there's not enough bandwidth for 4:4:4, etc.). If the > > behaviour suddenly changes then it probably means the driver was > > doing something illegal before by using RGB 4:4:4. > > Ok. > > > > > > > > > > > > > > > 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? > > > > Yes. > > > > > > > > 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. > > > > This point has been irrelevant since we always select BT.709 > > and there is no optional feature bit in EDID to check for that. > > Presumaly it is mandatory for sinks to support both BT.601 and > > BT.709 whenever they support YCbCr in general. > > Ok, so BT.601 and BT.709 MatrixCoefficients are cool. How do you tell > the sink which one you used, btw? Through the same infoframe/msa/sdp stuff. But that only works correctly if the colorspace property is left at the default value. > > What about BT.2020 MatrixCoefficients? It would have to work the same way, if we actually ever used this. -- Ville Syrjälä Intel
