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

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

 



On 2021-04-26 20:56, 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@xxxxxxx>

Add the following color encodings
- RGB versions for BT601, BT709, BT2020
- DCI-P3: Used for digital movies

Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@xxxxxxx>
Signed-off-by: Harry Wentland <harry.wentland@xxxxxxx>
---
  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?
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?

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 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.

Introducing another API if hardware is sufficiently different doesn't
seem like the worst idea. At least it sounds a lot more tractable than
teaching the kernel about all the different use cases, opinions and
nuances that arise from color management.

In the end generic user space must always be able to fall back to
software so the worst case is that it won't be able to offload an
operation if it doesn't know about a new API.

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.

Blending only is "correct" with linear light so that property of the
color space is important. However, why does the kernel have to be
involved here? As long as user space knows that for correct blending the
data must represent linear light and knows when in the pipeline blending
happens it can make sure that the data at that point in the pipeline
contains linear light.

What other purposes are there?

In general I agree with the others that user space only wants a pipeline
of transformations where the mechanism, the order and ideally the
precision is defined.

Harry
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux