[RFC PATCH 1/3] drm/color: Add RGB Color encodings

[Ville Syrjälä]

On Mon, Apr 26, 2021 at 02:56:26PM -0400, Harry Wentland wrote:
> On 2021-04-26 2:07 p.m., Ville Syrjälä wrote:
> > On Mon, Apr 26, 2021 at 01:38:50PM -0400, Harry Wentland wrote:
> >> From: Bhawanpreet Lakha <Bhawanpreet.Lakha at amd.com>
> >>
> >> Add the following color encodings
> >> - RGB versions for BT601, BT709, BT2020
> >> - DCI-P3: Used for digital movies
> >>
> >> Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha at amd.com>
> >> Signed-off-by: Harry Wentland <harry.wentland at amd.com>
> >> ---
> >>   drivers/gpu/drm/drm_color_mgmt.c | 4 ++++
> >>   include/drm/drm_color_mgmt.h     | 4 ++++
> >>   2 files changed, 8 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> >> index bb14f488c8f6..a183ebae2941 100644
> >> --- a/drivers/gpu/drm/drm_color_mgmt.c
> >> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> >> @@ -469,6 +469,10 @@ static const char * const color_encoding_name[] = {
> >>   	[DRM_COLOR_YCBCR_BT601] = "ITU-R BT.601 YCbCr",
> >>   	[DRM_COLOR_YCBCR_BT709] = "ITU-R BT.709 YCbCr",
> >>   	[DRM_COLOR_YCBCR_BT2020] = "ITU-R BT.2020 YCbCr",
> >> +	[DRM_COLOR_RGB_BT601] = "ITU-R BT.601 RGB",
> >> +	[DRM_COLOR_RGB_BT709] = "ITU-R BT.709 RGB",
> >> +	[DRM_COLOR_RGB_BT2020] = "ITU-R BT.2020 RGB",
> >> +	[DRM_COLOR_P3] = "DCI-P3",
> > 
> > These are a totally different thing than the YCbCr stuff.
> > The YCbCr stuff just specifies the YCbCr<->RGB converison matrix,
> > whereas these are I guess supposed to specify the primaries/whitepoint?
> > But without specifying what we're converting *to* these mean absolutely
> > nothing. Ie. I don't think they belong in this property.
> > 
> 
> If this is the intention I don't see it documented.
> 
> I might have overlooked something but do we have a way to explicitly 
> specify today what *to* format the YCbCr color encodings convert into? 

These just specific which YCbCr<->RGB matrix to use as specificed
in the relevant standards. The primaries/whitepoint/etc. don't
change at all.

> Would that be a combination of the output color encoding specified via 
> colorspace_property and the color space encoded in the primaries and 
> whitepoint of the hdr_output_metadata?

Those propertis only affect the infoframes. They don't apply any
color processing to the data.

> 
> Fundamentally I don't see how the use of this property differs, whether 
> you translate from YCbCr or from RGB. In either case you're converting 
> from the defined input color space and pixel format to an output color 
> space and pixel format.

The gamut does not change when you do YCbCr<->RGB conversion.

> 
> > The previous proposals around this topic have suggested a new
> > property to specify a conversion matrix either explicitly, or
> > via a separate enum (which would specify both the src and dst
> > colorspaces). I've always argued the enum approach is needed
> > anyway since not all hardware has a programmable matrix for
> > this. But a fully programmable matrix could be nice for tone
> > mapping purposes/etc, so we may want to make sure both are
> > possible.
> > 
> > As for the transfer func, the proposals so far have mostly just
> > been to expose a programmable degamma/gamma LUTs for each plane.
> > But considering how poor the current gamma uapi is we've thrown
> > around some ideas how to allow the kernel to properly expose the
> > hw capabilities. This is one of those ideas:
> > https://lists.freedesktop.org/archives/dri-devel/2019-April/212886.html>> I think that basic idea could be also be extended to allow fixed
> > curves in case the hw doesn't have a fully programmable LUT. But
> > dunno if that's relevant for your hw.
> > 
> 
> The problem with exposing gamma, whether per-plane or per-crtc, is that 
> it is hard to define an API that works for all the HW out there. The 
> capabilities for different HW differ a lot, not just between vendors but 
> also between generations of a vendor's HW.
> 
> Another reason I'm proposing to define the color space (and gamma) of a 
> plane is to make this explicit. Up until the color space and gamma of a 
> plane or framebuffer are not well defined, which leads to drivers 
> assuming the color space and gamma of a buffer (for blending and other 
> purposes) and might lead to sub-optimal outcomes.

The current state is that things just get passed through as is
(apart from the crtc LUTs/CTM).

-- 
Ville Syrjälä
Intel