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