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

[Alex Hung <alex.hung@amd.com>]

On 7/8/25 11:10, Simon Ser wrote:
> On Tuesday, June 17th, 2025 at 06:26, Alex Hung <alex.hung@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.

> 
>> +	/*
>> +	 * 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