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

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3925 bytes --]

On Tue, 18 May 2021 10:32:48 -0400
Harry Wentland <harry.wentland@amd.com> wrote:

> On 2021-05-17 4:34 a.m., Pekka Paalanen wrote:
> > On Fri, 14 May 2021 17:04:51 -0400
> > Harry Wentland <harry.wentland@amd.com> wrote:
> >   
> >> On 2021-04-30 8:53 p.m., Sebastian Wick wrote:  
> >>> On 2021-04-26 20:56, Harry Wentland wrote:    
> > 
> > ...
> >   
> >>>> 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.
> >>>     
> >>
> >> The only reason the kernel needs to be involved is to take full advantage
> >> of the available HW without requiring KMS clients to be aware of
> >> the difference in display HW.  
> > 
> > Can you explain in more tangible examples, why you think so, please?
> > 
> > Is it because hardware does not fit the KMS UAPI model of the abstract
> > pixel pipeline?
> >   
> 
> I'd wager no HW is designed to meet KMS UAPI, rather KMS UAPI is designed
> to abstract HW.

Of course, but you are in big trouble in any case if there is a
fundamental mismatch. You may have to declare that all existing KMS
properties for this stuff will be mutually exclusive with your new
properties, so that you can introduce a new generic abstract pipeline
in KMS.

By mutually exclusive I mean that a driver must advertise only one or
the other set of properties and never both. If you want to support
userspace that doesn't understand the alternative set, maybe you also
need a drm client cap to switch to the alternative set per-drm-client.

> > Or is it because you have fixed-function hardware elements that you can
> > only make use of when userspace uses an enum-based UAPI?
> >   
> 
> One example is our degamma on our latest generation HW, where we have
> fixed-function "degamma" (rather de-EOTF):
> 
> https://gitlab.freedesktop.org/agd5f/linux/-/blob/amd-staging-drm-next/drivers/gpu/drm/amd/display/dc/dcn30/dcn30_dpp.c#L166

Ok.

> > I would totally agree that the driver does not want to be analysing LUT
> > entries to decipher if it could use a fixed-function element or not. It
> > would introduce uncertainty in the UAPI. So fixed-function elements
> > would need their own properties, but I don't know if that is feasible
> > as generic UAPI or if it should be driver-specific (and so left unused
> > by generic userspace).
> >   
> 
> 
> For the CRTC LUTs we actually do a linearity check to program the
> HW into bypass when the LUT is linear since the KMS LUT definition
> doesn't map well onto the LUT definition used by our HW and leads
> to rounding errors and failing IGT kms_color tests (if I remember
> this correctly).
> 
> https://gitlab.freedesktop.org/agd5f/linux/-/blob/amd-staging-drm-next/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_color.c#L330
> 
> Hence the suggestion to define pre-defined TFs right at a KMS level
> for usecases where we can assume the display will tonemap the 
> content.

Right. Explaining this would have been a good introduction in your
cover letter.

Maybe you want to define new KMS properties that shall be mutually
exclusive with the existing KMS GAMMA/CTM/DEGAMMA properties and
clearly document them as such.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]