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

[Borah, Chaitanya Kumar]

> -----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.

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