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@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? 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 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel