[PATCH V8 32/43] drm/colorop: Add 1D Curve Custom LUT type

[Shankar, Uma]

> -----Original Message-----
> From: wayland-devel <wayland-devel-bounces at lists.freedesktop.org> On Behalf
> Of Simon Ser
> Sent: Tuesday, April 15, 2025 9:55 PM
> To: Harry Wentland <harry.wentland at amd.com>
> Cc: Shankar, Uma <uma.shankar at intel.com>; Alex Hung <alex.hung at amd.com>;
> dri-devel at lists.freedesktop.org; amd-gfx at lists.freedesktop.org; intel-
> gfx at lists.freedesktop.org; wayland-devel at lists.freedesktop.org;
> leo.liu at amd.com; ville.syrjala at linux.intel.com; pekka.paalanen at collabora.com;
> mwen at igalia.com; jadahl at redhat.com; sebastian.wick at redhat.com;
> shashank.sharma at amd.com; agoins at nvidia.com; joshua at froggi.es;
> mdaenzer at redhat.com; aleixpol at kde.org; xaver.hugl at gmail.com;
> victoria at system76.com; daniel at ffwll.ch; quic_naseer at quicinc.com;
> quic_cbraga at quicinc.com; quic_abhinavk at quicinc.com; marcan at marcan.st;
> Liviu.Dudau at arm.com; sashamcintosh at google.com; Borah, Chaitanya Kumar
> <chaitanya.kumar.borah at intel.com>; louis.chauvet at bootlin.com
> Subject: Re: [PATCH V8 32/43] drm/colorop: Add 1D Curve Custom LUT type
> 
> On Tuesday, April 15th, 2025 at 17:05, Harry Wentland
> <harry.wentland at amd.com> wrote:
> 
> > > > > We want to have just one change in the way we expose the
> > > > > hardware capabilities else all looks good in general.
> > > >
> > > > I would really recommend leaving this as a follow-up extension.
> > > > It's a complicated addition that requires more discussion.
> > >
> > > Hi Simon,
> > > We have tried to solve the complex part and made it simple to
> > > understand and implement along with a reference implementation [1] (can
> also help add the same for AMD case as well).
> > > Without this we will end up with up 2 interfaces for 1dL Lut which
> > > is not nice where the one above will be able to cover the current
> > > one. Let us know the problems with the proposed interface and we can
> > > work to fix the same. But having a common and single interface is
> > > good and the current one will not fit Intel's color pipeline distribution so the
> generic one anyways will be needed, and it will benefit userspace to know the
> underlying LUT distribution to compute the LUT samples.
> > >
> > > [1] https://patchwork.freedesktop.org/series/129812/
> >
> > I think there is a lot of value in giving userspace a simple LUT to
> > work with. There are many compositors and many compositor maintainers.
> > When someone new jumps into color management usually same thing
> > happens. It starts with "it's not too complicated", and then over a
> > period of time progresses to "this is very much non-trivial" as
> > understanding one bit usually opens ten more questions.
> >
> > Forcing people to deal with another level of complexity will
> > discourage implementations and be counterproductive to furthering
> > adoption of color operations for HW acceleration, IMO.
> >
> > I'm am not opposed to a complex LUT definition but I don't think it
> > should replace a simple and well-understood definition.
> 
> Agreed. To add on this, I think shipping many additional features from day one
> significantly increases the work load (more code to write, review, test at once)
> and we'd also need to go through supplementary rounds to validate the API
> design and ensure it's not too Intel-specific. Also adding this feature as a second
> step will prove that the API is as extensible as we desire.

Apologies for getting back late to the thread.  Sure, we can go with this and extend it later.

> I don't really understand why it's important to have this feature in the first
> version. Intel has been converting simple LUTs into the fancy distribution for the
> existing GAMMA_LUT and DEGAMMA_LUT for a while, so can do it for colorop as
> well. The upsides of the fancy distribution is more precise and smaller LUTs, but
> that doesn't seem critical?

Yes Simon, Intel has managed to somehow fit multiple segments with the existing design. It was
manageable where we had lut divided in 2-3 segments, but we have some cases where distribution
of luts is totally non-uniform (in logarithmic fashion as an example). But will add this as follow up once
the basic UAPI is merged.

One request though: Can we enhance the lut samples from existing 16bits to 32bits as lut precision is
going to be more than 16 in certain hardware. While adding the new UAPI, lets extend this to 32 to make it future proof.
Reference: https://patchwork.freedesktop.org/patch/642592/?series=129811&rev=4

+/**
+ * struct drm_color_lut_32 - Represents high precision lut values
+ *
+ * Creating 32 bit palette entries for better data
+ * precision. This will be required for HDR and
+ * similar color processing usecases.
+ */
+struct drm_color_lut_32 {
+	/*
+	 * Data for high precision LUTs
+	 */
+	__u32 red;
+	__u32 green;
+	__u32 blue;
+	__u32 reserved;
+};

Regards,
Uma Shankar

> Simon