[PATCH V10 33/46] drm: Add Enhanced LUT precision structure

[Shankar, Uma]

> -----Original Message-----
> From: Borah, Chaitanya Kumar <chaitanya.kumar.borah at intel.com>
> Sent: Tuesday, July 15, 2025 10:08 AM
> To: Alex Hung <alex.hung at amd.com>; Simon Ser <contact at emersion.fr>;
> Shankar, Uma <uma.shankar at intel.com>
> Cc: dri-devel at lists.freedesktop.org; amd-gfx at lists.freedesktop.org; wayland-
> devel at lists.freedesktop.org; harry.wentland at amd.com; 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; louis.chauvet at bootlin.com; arthurgrillo at riseup.net
> Subject: RE: [PATCH V10 33/46] drm: Add Enhanced LUT precision structure
> 
> 
> 
> > -----Original Message-----
> > From: Alex Hung <alex.hung at amd.com>
> > Sent: Friday, July 11, 2025 9:59 PM
> > To: Simon Ser <contact at emersion.fr>; Shankar, Uma
> > <uma.shankar at intel.com>
> > Cc: dri-devel at lists.freedesktop.org; amd-gfx at lists.freedesktop.org;
> > wayland- devel at lists.freedesktop.org; harry.wentland at amd.com;
> > 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;
> > arthurgrillo at riseup.net
> > Subject: Re: [PATCH V10 33/46] drm: Add Enhanced LUT precision
> > structure
> >
> >
> >
> > On 7/8/25 11:10, Simon Ser wrote:
> > > On Tuesday, June 17th, 2025 at 06:26, Alex Hung <alex.hung at amd.com>
> > wrote:
> > >
> > >> diff --git a/include/uapi/drm/drm_mode.h
> > >> b/include/uapi/drm/drm_mode.h index 651bdf48b766..21bd96f437e0
> > 100644
> > >> --- a/include/uapi/drm/drm_mode.h
> > >> +++ b/include/uapi/drm/drm_mode.h
> > >> @@ -872,6 +872,16 @@ struct drm_color_lut {
> > >>   	__u16 reserved;
> > >>   };
> > >>
> > >> +struct drm_color_lut_32 {
> > >
> > > Subjective nit: I would personally prefer "lut32" over "lut_32", but
> > > that's very deep into bike-shedding territory. Feel free to ignore.
> >
> > I think it's a good idea.
> >
> > I got this from Uma's patchset. I will sync up with him to avoid
> > future conflicts.
> >
> 
> Just for the record, we took some inspiration from struct drm_color_ctm_3x4 for
> this.
> That said, I don’t mind the bike-shedding — naming things is half the fun anyway.
> Happy to go with lut32 if that looks better!
> 
> @Uma feel free to disagree.

Yeah, Agree. Also Alex, you can make the change and have it as part of your series.
Let us know if any help needed.

Regards,
Uma Shankar

>
> Regards
> 
> Chaitanya
> 
> > >
> > >> +	/*
> > >> +	 * Similar to drm_color_lut but for high precision LUTs
> > >> +	 */
> > >
> > > Instead of this code comment, would be nicer to have a proper doc
> > comment.
> > > In doc comments, "struct XXX" is enough to create a link. (struct
> > > drm_color_lut doesn't have a doc comment, but has been introduced
> > > ages
> > > ago.)
> > >
> > > "high precision" is a bit subjective, proibably "32-bit precision"
> > > would be better.
> > >
> > >> +	__u32 red;
> > >> +	__u32 green;
> > >> +	__u32 blue;
> > >> +	__u32 reserved;
> > >> +};
> > >> +
> > >>   /**
> > >>    * enum drm_colorop_type - Type of color operation
> > >>    *
> > >> @@ -879,6 +889,7 @@ struct drm_color_lut {
> > >>    * and defines a different set of properties. This enum defines
> > >> all types
> > and
> > >>    * gives a high-level description.
> > >>    */
> > >> +
> > >
> > > Nit: probably this extra newline should get dropped?
> > >
> > >>   enum drm_colorop_type {
> > >>   	/**
> > >>   	 * @DRM_COLOROP_1D_CURVE:
> > >> --
> > >> 2.43.0