[PATCH 3/3] drm/connector: Deprecate split for BT.2020 in drm_colorspace enum

[Pekka Paalanen]

On Wed, 8 Feb 2023 16:49:31 +0200
Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:

> On Wed, Feb 08, 2023 at 11:18:42AM +0200, Pekka Paalanen wrote:
> > On Fri, 3 Feb 2023 16:02:51 +0200
> > Ville Syrjälä <ville.syrjala at linux.intel.com> wrote:
> >   
> > > On Fri, Feb 03, 2023 at 02:52:50PM +0100, Sebastian Wick wrote:  
> > > > On Fri, Feb 3, 2023 at 2:35 PM Ville Syrjälä
> > > > <ville.syrjala at linux.intel.com> wrote:    
> > > > >
> > > > > On Fri, Feb 03, 2023 at 01:59:07PM +0100, Sebastian Wick wrote:    
> > > > > > On Fri, Feb 3, 2023 at 11:40 AM Ville Syrjälä
> > > > > > <ville.syrjala at linux.intel.com> wrote:    
> > > > > > >
> > > > > > > On Fri, Feb 03, 2023 at 02:07:44AM +0000, Joshua Ashton wrote:    
> > > > > > > > Userspace has no way of controlling or knowing the pixel encoding
> > > > > > > > currently, so there is no way for it to ever get the right values here.    
> > > > > > >
> > > > > > > That applies to a lot of the other values as well (they are
> > > > > > > explicitly RGB or YCC). The idea was that this property sets the
> > > > > > > infoframe/MSA/SDP value exactly, and other properties should be
> > > > > > > added to for use userspace to control the pixel encoding/colorspace
> > > > > > > conversion(if desired, or userspace just makes sure to
> > > > > > > directly feed in correct kind of data).    
> > > > > >
> > > > > > I'm all for getting userspace control over pixel encoding but even
> > > > > > then the kernel always knows which pixel encoding is selected and
> > > > > > which InfoFrame has to be sent. Is there a reason why userspace would
> > > > > > want to control the variant explicitly to the wrong value?    
> > > > >
> > > > > What do you mean wrong value? Userspace sets it based on what
> > > > > kind of data it has generated (or asked the display hardware
> > > > > to generate if/when we get explicit control over that part).    
> > > > 
> > > > Wrong in the sense of sending the YCC variant when the pixel encoding
> > > > is RGB for example.
> > > > 
> > > > Maybe I'm missing something here but my assumption is that the kernel
> > > > always has to know the pixel encoding anyway. The color pipeline also
> > > > assumes that the pixel values are RGB. User space might be able to
> > > > generate YCC content but for subsampling etc the pixel encoding still
> > > > has to be explicitly set.    
> > > 
> > > The kernel doesn't really know much atm. In theory you can just
> > > configure the thing to do a straight passthough and put anything you
> > > want into your pixels.  
> > 
> > But it's impossible to use a YCbCr framebuffer and have that *not*
> > converted to RGB for the KMS color pipeline even if userspace wanted it
> > to be strictly pass-through, only to be converted again to YCbCr for
> > the cable, is it not?
> > 
> > Even more so with 4:2:0.
> > 
> > How could it be possible to stop the driver from doing those two
> > YUV-to-RGB and RGB-to-YCbCr conversions at the beginning and at the end
> > of the KMS color pipeline?  
> 
> You can stop the conversion at the start of the pipeline by
> using a "RGB" framebuffer. At the end of the pipe it's not
> possible with the current props.

But there is no such thing as a 4:2:0 sub-sampled RGB framebuffer to be
abused for YUV content. It would be possible for some kind of xYUV
4:4:4 content though, but then the pipeline wouldn't work.

Joshua had the excellent point that disabling the conversion at the end
of the pipeline is not possible for a non-RGB output signal, period.
The KMS color pipeline is defined in terms of RGB channels, that's the
only(?) way alpha-blending could work, and the LUT-like elements cannot
handle negative values.

On one hand I very much agree that the definition of "Broadcast RGB"
property was a mistake by combining pixel operations with infoframe
settings. OTOH, since the pipeline end conversion is today chosen by
the driver, then the KMS color pipeline output must be known to the
driver so that the driver can pick the right conversion. That's what
"Broadcast RGB" did: it assumed the pipeline produces full range
values, so that it is able to insert the right conversion and the right
infoframe data. It rules out possible use cases, but the infoframe
matches.

As for the pipe-end RGB-to-YCbCr conversion, the situation is partly
similar. There is the assumption that the pipeline produces RGB model
values. However, this assumption is likely never going to change,
because the pipeline is inherently RGB, always.

A better question is, does it need other assumptions as well?

Quantization range?

RGB (electrical encoding) transfer function?

Most RGB-to-YCbCr conversions are just a matrix applied to the
electrical RGB values, but not all. Particularly the constant luminance
encoding requires optical, not electrical, RGB values, and it also
needs the transfer function since it emits electrical values. I haven't
looked if e.g. BT.2100 has more cases making the RGB-to-something
conversion complex.

Even having a doubt about that really does point towards userspace
needing to configure the pipe-end conversion by mathematical formula,
not by video standard colorspace. A consequence of that is that the
infoframe settings need to be an independent property separate from the
end-conversion property.

If so, it is not even possible for a driver to automatically set the
pipe-end conversion without telling it a lot more about what the KMS
color pipeline RGB output is.

I have been advocating that we should not tell the kernel about color
spaces, and instead we need to program the mathematical operations in
the color pipeline. Following that reasoning, we cannot make it
possible for drivers to choose the pipe-end conversion automatically.

Hmm...



Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20230209/828e32c7/attachment.sig>