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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 3/17/23 13:35, 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.

No, sorry, this is completely nonsensical.

Even with the current Colorspace property that we want to deprecate, drivers doing an implicit conversion from RGB -> to YCC should respect the colorspace property to pick the right matrix coefficients here.

Doing so simply introduces a useless mismatch that is unavoidable from userspace and makes absolutely no sense.

Arguing about this is kind of completely useless anyway... as we are deprecating this property... Let's pleeeease let it die.

I am not sure why these patches were re-submitted with my RB again after we had the discussion previously about making a new one that's actually going to get tested and have userspace consumers.

(FTR, I guess Gamescope *is* a userspace consumer for this broken property right now, but I am completely happy for AMDGPU upstream to never support this and to just move onto the new property and leave this one behind).


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.


And the driver will never use BT.2020 NCL MatrixCoefficients in any
circumstances?

That is what Ville is describing and what I disagree with, yes.

But whether or not Ville or I agree on that is kind of irrelevant as we are going to deprecate the property... right... right?


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.

Yeah.

- Joshie 🐸✨




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?

What about BT.2020 MatrixCoefficients?


Thanks,
pq



[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux