[Intel-gfx] [v6 01/16] drm: Add Enhanced Gamma LUT precision structure

[Shankar, Uma]

>-----Original Message-----
>From: Lankhorst, Maarten
>Sent: Wednesday, March 27, 2019 5:43 PM
>To: Shankar, Uma <uma.shankar at intel.com>; intel-gfx at lists.freedesktop.org
>Cc: Sharma, Shashank <shashank.sharma at intel.com>; Roper, Matthew D
><matthew.d.roper at intel.com>; Syrjala, Ville <ville.syrjala at intel.com>
>Subject: Re: [v6 01/16] drm: Add Enhanced Gamma LUT precision structure
>
>tis 2019-03-19 klockan 14:14 +0530 skrev Uma Shankar:
>> Existing LUT precision structure is having only 16 bit
>> precision. This is not enough for upcoming enhanced hardwares
>> and advance usecases like HDR processing. Hence added a new
>> structure with 32 bit precision values. Also added the code,
>> for extracting the same from values passed from userspace.
>>
>> v4: Rebase
>>
>> v5: Relocated the helper function to drm_color_mgmt.c. Declared
>> the same in a header file (Alexandru Gheorghe)
>>
>> v6: Enhanced gamma lut structure to take U32.32 format as input.
>> This is needed for HDR usecase which require higher precision.
>>
>> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
>> Reviewed-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe at arm.com>
>> ---
>>  drivers/gpu/drm/drm_color_mgmt.c | 19 +++++++++++++++++++
>>  include/drm/drm_color_mgmt.h     |  1 +
>>  include/uapi/drm/drm_mode.h      | 15 +++++++++++++++
>>  3 files changed, 35 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_color_mgmt.c
>> b/drivers/gpu/drm/drm_color_mgmt.c
>> index d5d34d0..9dbfe1d 100644
>> --- a/drivers/gpu/drm/drm_color_mgmt.c
>> +++ b/drivers/gpu/drm/drm_color_mgmt.c
>> @@ -128,6 +128,25 @@ uint32_t drm_color_lut_extract(uint32_t
>> user_input, uint32_t bit_precision)
>>  }
>>  EXPORT_SYMBOL(drm_color_lut_extract);
>>
>> +/*
>> + * Added to accommodate enhanced LUT precision.
>> + * Max LUT precision is 32 bits.
>> + */
>> +u64 drm_color_lut_extract_ext(u64 user_input, u32 bit_precision)
>> +{
>> +	u32 val = user_input & 0xffffffff;
>> +	u32 max = 0xffffffff >> (32 - bit_precision);
>> +
>> +	/* Round only if we're not using full precision. */
>> +	if (bit_precision < 32) {
>> +		val += 1UL << (32 - bit_precision - 1);
>> +		val >>= 32 - bit_precision;
>> +	}
>> +
>> +	return ((user_input & 0xffffffff) | clamp_val(val, 0, max));
>I thought the userspace precision was U32.32, so the precision here is
>max 64-bits?
>
>Anyway the math looks wrong for a U32.32 value, it's probably:
>user_input >> (64ULL - precision)

Hi Maarten,
Since we have a limitation of 64bit container and overflow if we left shift. My plan is
to preserve the fixed value of 32bit separately and just round up the floating value ie
32bit after decimal which mostly represent values below 1.0.  Kept bit precision as 32 for that
purpose only.

But you are right, I need to have the val also as a long variable to make this logic work.
Will update the val to long and this should work fine.

Thanks & Regards,
Uma Shankar

>
>> +}
>> +EXPORT_SYMBOL(drm_color_lut_extract_ext);
>> +
>>  /**
>>   * drm_crtc_enable_color_mgmt - enable color management properties
>>   * @crtc: DRM CRTC
>> diff --git a/include/drm/drm_color_mgmt.h
>> b/include/drm/drm_color_mgmt.h
>> index d1c662d..c9d2746 100644
>> --- a/include/drm/drm_color_mgmt.h
>> +++ b/include/drm/drm_color_mgmt.h
>> @@ -30,6 +30,7 @@
>>  struct drm_plane;
>>
>>  uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t
>> bit_precision);
>> +u64 drm_color_lut_extract_ext(u64 user_input, u32 bit_precision);
>>
>>  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>>  				uint degamma_lut_size,
>> diff --git a/include/uapi/drm/drm_mode.h
>> b/include/uapi/drm/drm_mode.h
>> index a439c2e..a0fae71 100644
>> --- a/include/uapi/drm/drm_mode.h
>> +++ b/include/uapi/drm/drm_mode.h
>> @@ -630,6 +630,21 @@ struct drm_color_lut {
>>  	__u16 reserved;
>>  };
>>
>> +/*
>> + * Creating 64 bit palette entries for better data
>> + * precision. This will be required for HDR and
>> + * similar color processing usecases.
>> + */
>> +struct drm_color_lut_ext {
>> +	/*
>> +	 * Data is U32.32 fixed point format.
>> +	 */
>> +	__u64 red;
>> +	__u64 green;
>> +	__u64 blue;
>> +	__u64 reserved;
>> +};
>> +
>>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01
>>  #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
>>  #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4