Re: [PATCH v2 07/34] drm/amd/display: explicitly define EOTF and inverse EOTF

[Pekka Paalanen <pekka.paalanen@collabora.com>]

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

On Thu, 7 Sep 2023 10:10:50 -0400
Harry Wentland <harry.wentland@amd.com> wrote:

> On 2023-09-07 03:49, Pekka Paalanen wrote:
> > On Wed, 6 Sep 2023 16:15:10 -0400
> > Harry Wentland <harry.wentland@amd.com> wrote:
> >   
> >> On 2023-08-25 10:18, Melissa Wen wrote:  
> >>> On 08/22, Pekka Paalanen wrote:    
> >>>> On Thu, 10 Aug 2023 15:02:47 -0100
> >>>> Melissa Wen <mwen@igalia.com> wrote:
> >>>>    
> >>>>> Instead of relying on color block names to get the transfer function
> >>>>> intention regarding encoding pixel's luminance, define supported
> >>>>> Electro-Optical Transfer Functions (EOTFs) and inverse EOTFs, that
> >>>>> includes pure gamma or standardized transfer functions.
> >>>>>
> >>>>> Suggested-by: Harry Wentland <harry.wentland@amd.com>
> >>>>> Signed-off-by: Melissa Wen <mwen@igalia.com>
> >>>>> ---
> >>>>>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.h | 19 +++--
> >>>>>  .../amd/display/amdgpu_dm/amdgpu_dm_color.c   | 69 +++++++++++++++----
> >>>>>  2 files changed, 67 insertions(+), 21 deletions(-)

...

> >> While I'm okay to move ahead with these AMD driver-specific properties
> >> without IGT tests (since they're not enabled and not UABI) we really
> >> need IGT tests once they become UABI with the Color Pipeline API. And we
> >> need more than just CRC testing. We'll need to do pixel-by-pixel comparison
> >> so we can verify that the KMS driver behaves exactly how we expect for a
> >> large range of values.  
> > 
> > Yes, please, very much, about the generic color UAPI.
> > 
> > I believe IGT should contain the reference curve for all named fixed
> > curves computed with standard libc math functions in double precision,
> > and compute error statistics between that and hardware results.
> > The actual test image would iterate through e.g. 1024 (all 10-bit
> > values for integer format framebuffer) different values - 1024 is
> > nothing as a number of pixels. Then we decide on acceptable error
> > thresholds.
> >   
> 
> 1024 isn't a lot of values and fine if we test R, G, and B independently.
> Unfortunately 1024^3 is about a billion pixels, so for testing 3DLUTs
> (or other cases where we need to test the combination of RGB together)
> we won't be able to cover all inputs with a single framebuffer.

Of course, runtimes need to be practical. That idea was for 1D curves,
and 3D mappings need a different distribution.

> > It should also be tested with a floating-point framebuffer format, FP16
> > or FP32, with a value distribution designed to be sensitive to typical
> > numerical problems. For example, an inverse EOTF should be carefully
> > tested with values near zero, since those are the most problematic and
> > likely cause the most visible errors.
> > 
> > Once all that is done, we can be very sure of what curve any hardware
> > actually implements.
> > 
> > I might even go far enough to suggest that any generic color UAPI with
> > named fixed curves cannot land without such tests.
> >   
> 
> I tend to agree, though I think the same should on some level apply to
> custom LUTs or other custom transforms.
> 
> The IGT tests I'm writing will each have a "transform" function that does
> the transform in CPU as reference.

Sounds good!

For testing optical-to-electrical kind of operations, one idea is to
sample the electrical target space, and reverse the reference transform
to come up with the test input values. That way one can test if the
output space is sufficiently covered, and the rounding behavior as well.

Electrical space usually tends to be integer encoded with not too many
bits, making even exhaustive sampling feasible for 1D curves.


Thanks,
pq


> >>> Then EOTF and inverse EOTF for PQ [2], and OETF and it seems an inverse
> >>> OETF but called EOTF for HLG[3]. But I'm an external dev, better if
> >>> Harry can confirm.
> >>>
> >>> Thank you for pointing it out.
> >>>
> >>> [1] https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/amd/display/modules/color/color_gamma.c#n55
> >>> [2] https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/amd/display/modules/color/color_gamma.c#n106
> >>> [3] https://cgit.freedesktop.org/drm/drm-misc/tree/drivers/gpu/drm/amd/display/modules/color/color_gamma.c#n174
> >>>     
> >>>>
> >>>> The others seem fine to me.
> >>>>
> >>>>
> >>>> Thanks,
> >>>> pq  
> 


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