Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

2021-11-23 Thread Harry Wentland 



On 2021-11-12 03:37, Pekka Paalanen wrote:
> On Thu, 11 Nov 2021 21:58:35 +
> "Shankar, Uma"  wrote:
> 
>>> -Original Message-
>>> From: Harry Wentland 
>>> Sent: Friday, November 12, 2021 2:41 AM
>>> To: Shankar, Uma ; Ville Syrjälä
>>> 
>>> Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
>>> ppaala...@gmail.com; brian.star...@arm.com; sebast...@sebastianwick.net;
>>> shashank.sha...@amd.com
>>> Subject: Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct 
>>> for
>>> HDR planes
>>>
>>>
>>>
>>> On 2021-11-11 15:42, Shankar, Uma wrote:  
>>>>
>>>>  
>>>>> -Original Message-
>>>>> From: Ville Syrjälä 
>>>>> Sent: Thursday, November 11, 2021 10:13 PM
>>>>> To: Harry Wentland 
>>>>> Cc: Shankar, Uma ;
>>>>> intel-...@lists.freedesktop.org; dri- de...@lists.freedesktop.org;
>>>>> ppaala...@gmail.com; brian.star...@arm.com;
>>>>> sebast...@sebastianwick.net; shashank.sha...@amd.com
>>>>> Subject: Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range
>>>>> struct for HDR planes
>>>>>
>>>>> On Thu, Nov 11, 2021 at 10:17:17AM -0500, Harry Wentland wrote:  
>>>>>>
>>>>>>
>>>>>> On 2021-09-06 17:38, Uma Shankar wrote:  
>>>>>>> Define the structure with XE_LPD degamma lut ranges. HDR and SDR
>>>>>>> planes have different capabilities, implemented respective
>>>>>>> structure for the HDR planes.
>>>>>>>
>>>>>>> Signed-off-by: Uma Shankar 
>>>>>>> ---
>>>>>>>  drivers/gpu/drm/i915/display/intel_color.c | 52
>>>>>>> ++
>>>>>>>  1 file changed, 52 insertions(+)
>>>>>>>
>>>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
>>>>>>> b/drivers/gpu/drm/i915/display/intel_color.c
>>>>>>> index afcb4bf3826c..6403bd74324b 100644
>>>>>>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>>>>>>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>>>>>>> @@ -2092,6 +2092,58 @@ static void icl_read_luts(struct
>>>>>>> intel_crtc_state  
>>>>> *crtc_state)  
>>>>>>> }
>>>>>>>  }
>>>>>>>
>>>>>>> + /* FIXME input bpc? */
>>>>>>> +__maybe_unused
>>>>>>> +static const struct drm_color_lut_range d13_degamma_hdr[] = {
>>>>>>> +   /* segment 1 */
>>>>>>> +   {
>>>>>>> +   .flags = (DRM_MODE_LUT_GAMMA |
>>>>>>> + DRM_MODE_LUT_REFLECT_NEGATIVE |
>>>>>>> + DRM_MODE_LUT_INTERPOLATE |
>>>>>>> + DRM_MODE_LUT_NON_DECREASING),
>>>>>>> +   .count = 128,
>>>>>>> +   .input_bpc = 24, .output_bpc = 16,
>>>>>>> +   .start = 0, .end = (1 << 24) - 1,
>>>>>>> +   .min = 0, .max = (1 << 24) - 1,
>>>>>>> +   },
>>>>>>> +   /* segment 2 */
>>>>>>> +   {
>>>>>>> +   .flags = (DRM_MODE_LUT_GAMMA |
>>>>>>> + DRM_MODE_LUT_REFLECT_NEGATIVE |
>>>>>>> + DRM_MODE_LUT_INTERPOLATE |
>>>>>>> + DRM_MODE_LUT_REUSE_LAST |
>>>>>>> + DRM_MODE_LUT_NON_DECREASING),
>>>>>>> +   .count = 1,
>>>>>>> +   .input_bpc = 24, .output_bpc = 16,
>>>>>>> +   .start = (1 << 24) - 1, .end = 1 << 24,
>>>>>>> +   .min = 0, .max = (1 << 27) - 1,
>>>>>>> +   },
>>>>>>> +   /* Segment 3 */
>>>>>>> +   {
>>>>>>> +   .flags = (DRM_MODE_LUT_GAMMA |
>>>>>>> + DRM_MODE_LUT_REFLECT_NEGATIVE |
>>>>>>> + DRM_MODE_LUT_INTERPO

Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

2021-11-16 Thread Pekka Paalanen 
On Fri, 12 Nov 2021 16:54:35 +0200
Ville Syrjälä  wrote:

> On Thu, Nov 11, 2021 at 04:10:41PM -0500, Harry Wentland wrote:
> > 
> > 
> > On 2021-11-11 15:42, Shankar, Uma wrote:  
> > > 
> > >   
> > >> -Original Message-
> > >> From: Ville Syrjälä 
> > >> Sent: Thursday, November 11, 2021 10:13 PM
> > >> To: Harry Wentland 
> > >> Cc: Shankar, Uma ; 
> > >> intel-...@lists.freedesktop.org; dri-
> > >> de...@lists.freedesktop.org; ppaala...@gmail.com; brian.star...@arm.com;
> > >> sebast...@sebastianwick.net; shashank.sha...@amd.com
> > >> Subject: Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range 
> > >> struct for
> > >> HDR planes
> > >>
> > >> On Thu, Nov 11, 2021 at 10:17:17AM -0500, Harry Wentland wrote:  
> > >>>
> > >>>
> > >>> On 2021-09-06 17:38, Uma Shankar wrote:  
> > >>>> Define the structure with XE_LPD degamma lut ranges. HDR and SDR
> > >>>> planes have different capabilities, implemented respective structure
> > >>>> for the HDR planes.
> > >>>>
> > >>>> Signed-off-by: Uma Shankar 
> > >>>> ---
> > >>>>  drivers/gpu/drm/i915/display/intel_color.c | 52
> > >>>> ++
> > >>>>  1 file changed, 52 insertions(+)
> > >>>>
> > >>>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> > >>>> b/drivers/gpu/drm/i915/display/intel_color.c
> > >>>> index afcb4bf3826c..6403bd74324b 100644
> > >>>> --- a/drivers/gpu/drm/i915/display/intel_color.c
> > >>>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > >>>> @@ -2092,6 +2092,58 @@ static void icl_read_luts(struct 
> > >>>> intel_crtc_state  
> > >> *crtc_state)  
> > >>>>}
> > >>>>  }
> > >>>>
> > >>>> + /* FIXME input bpc? */
> > >>>> +__maybe_unused
> > >>>> +static const struct drm_color_lut_range d13_degamma_hdr[] = {
> > >>>> +  /* segment 1 */
> > >>>> +  {
> > >>>> +  .flags = (DRM_MODE_LUT_GAMMA |
> > >>>> +DRM_MODE_LUT_REFLECT_NEGATIVE |
> > >>>> +DRM_MODE_LUT_INTERPOLATE |
> > >>>> +DRM_MODE_LUT_NON_DECREASING),
> > >>>> +  .count = 128,
> > >>>> +  .input_bpc = 24, .output_bpc = 16,
> > >>>> +  .start = 0, .end = (1 << 24) - 1,
> > >>>> +  .min = 0, .max = (1 << 24) - 1,
> > >>>> +  },
> > >>>> +  /* segment 2 */
> > >>>> +  {
> > >>>> +  .flags = (DRM_MODE_LUT_GAMMA |
> > >>>> +DRM_MODE_LUT_REFLECT_NEGATIVE |
> > >>>> +DRM_MODE_LUT_INTERPOLATE |
> > >>>> +DRM_MODE_LUT_REUSE_LAST |
> > >>>> +DRM_MODE_LUT_NON_DECREASING),
> > >>>> +  .count = 1,
> > >>>> +  .input_bpc = 24, .output_bpc = 16,
> > >>>> +  .start = (1 << 24) - 1, .end = 1 << 24,
> > >>>> +  .min = 0, .max = (1 << 27) - 1,
> > >>>> +  },
> > >>>> +  /* Segment 3 */
> > >>>> +  {
> > >>>> +  .flags = (DRM_MODE_LUT_GAMMA |
> > >>>> +DRM_MODE_LUT_REFLECT_NEGATIVE |
> > >>>> +DRM_MODE_LUT_INTERPOLATE |
> > >>>> +DRM_MODE_LUT_REUSE_LAST |
> > >>>> +DRM_MODE_LUT_NON_DECREASING),
> > >>>> +  .count = 1,
> > >>>> +  .input_bpc = 24, .output_bpc = 16,
> > >>>> +  .start = 1 << 24, .end = 3 << 24,
> > >>>> +  .min = 0, .max = (1 << 27) - 1,
> > >>>> +  },
> > >>>> +  /* Segment 4 */
> > >>>> +  {
> > >>>> +  .flags = (DRM_MODE_LUT_GAMMA |
> > >>>&

Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

2021-11-12 Thread Ville Syrjälä 
On Thu, Nov 11, 2021 at 04:10:41PM -0500, Harry Wentland wrote:
> 
> 
> On 2021-11-11 15:42, Shankar, Uma wrote:
> > 
> > 
> >> -Original Message-
> >> From: Ville Syrjälä 
> >> Sent: Thursday, November 11, 2021 10:13 PM
> >> To: Harry Wentland 
> >> Cc: Shankar, Uma ; intel-...@lists.freedesktop.org; 
> >> dri-
> >> de...@lists.freedesktop.org; ppaala...@gmail.com; brian.star...@arm.com;
> >> sebast...@sebastianwick.net; shashank.sha...@amd.com
> >> Subject: Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range 
> >> struct for
> >> HDR planes
> >>
> >> On Thu, Nov 11, 2021 at 10:17:17AM -0500, Harry Wentland wrote:
> >>>
> >>>
> >>> On 2021-09-06 17:38, Uma Shankar wrote:
> >>>> Define the structure with XE_LPD degamma lut ranges. HDR and SDR
> >>>> planes have different capabilities, implemented respective structure
> >>>> for the HDR planes.
> >>>>
> >>>> Signed-off-by: Uma Shankar 
> >>>> ---
> >>>>  drivers/gpu/drm/i915/display/intel_color.c | 52
> >>>> ++
> >>>>  1 file changed, 52 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> >>>> b/drivers/gpu/drm/i915/display/intel_color.c
> >>>> index afcb4bf3826c..6403bd74324b 100644
> >>>> --- a/drivers/gpu/drm/i915/display/intel_color.c
> >>>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> >>>> @@ -2092,6 +2092,58 @@ static void icl_read_luts(struct intel_crtc_state
> >> *crtc_state)
> >>>>  }
> >>>>  }
> >>>>
> >>>> + /* FIXME input bpc? */
> >>>> +__maybe_unused
> >>>> +static const struct drm_color_lut_range d13_degamma_hdr[] = {
> >>>> +/* segment 1 */
> >>>> +{
> >>>> +.flags = (DRM_MODE_LUT_GAMMA |
> >>>> +  DRM_MODE_LUT_REFLECT_NEGATIVE |
> >>>> +  DRM_MODE_LUT_INTERPOLATE |
> >>>> +  DRM_MODE_LUT_NON_DECREASING),
> >>>> +.count = 128,
> >>>> +.input_bpc = 24, .output_bpc = 16,
> >>>> +.start = 0, .end = (1 << 24) - 1,
> >>>> +.min = 0, .max = (1 << 24) - 1,
> >>>> +},
> >>>> +/* segment 2 */
> >>>> +{
> >>>> +.flags = (DRM_MODE_LUT_GAMMA |
> >>>> +  DRM_MODE_LUT_REFLECT_NEGATIVE |
> >>>> +  DRM_MODE_LUT_INTERPOLATE |
> >>>> +  DRM_MODE_LUT_REUSE_LAST |
> >>>> +  DRM_MODE_LUT_NON_DECREASING),
> >>>> +.count = 1,
> >>>> +.input_bpc = 24, .output_bpc = 16,
> >>>> +.start = (1 << 24) - 1, .end = 1 << 24,
> >>>> +.min = 0, .max = (1 << 27) - 1,
> >>>> +},
> >>>> +/* Segment 3 */
> >>>> +{
> >>>> +.flags = (DRM_MODE_LUT_GAMMA |
> >>>> +  DRM_MODE_LUT_REFLECT_NEGATIVE |
> >>>> +  DRM_MODE_LUT_INTERPOLATE |
> >>>> +  DRM_MODE_LUT_REUSE_LAST |
> >>>> +  DRM_MODE_LUT_NON_DECREASING),
> >>>> +.count = 1,
> >>>> +.input_bpc = 24, .output_bpc = 16,
> >>>> +.start = 1 << 24, .end = 3 << 24,
> >>>> +.min = 0, .max = (1 << 27) - 1,
> >>>> +},
> >>>> +/* Segment 4 */
> >>>> +{
> >>>> +.flags = (DRM_MODE_LUT_GAMMA |
> >>>> +  DRM_MODE_LUT_REFLECT_NEGATIVE |
> >>>> +  DRM_MODE_LUT_INTERPOLATE |
> >>>> +  DRM_MODE_LUT_REUSE_LAST |
> >>>> +  DRM_MODE_LUT_NON_DECREASING),
> >>>> +.count = 1,
> >>>> +.input_bpc = 24, .output_bpc = 16,
> >>>> +

Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

2021-11-12 Thread Pekka Paalanen 
On Thu, 11 Nov 2021 21:58:35 +
"Shankar, Uma"  wrote:

> > -Original Message-
> > From: Harry Wentland 
> > Sent: Friday, November 12, 2021 2:41 AM
> > To: Shankar, Uma ; Ville Syrjälä
> > 
> > Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> > ppaala...@gmail.com; brian.star...@arm.com; sebast...@sebastianwick.net;
> > shashank.sha...@amd.com
> > Subject: Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct 
> > for
> > HDR planes
> > 
> > 
> > 
> > On 2021-11-11 15:42, Shankar, Uma wrote:  
> > >
> > >  
> > >> -Original Message-
> > >> From: Ville Syrjälä 
> > >> Sent: Thursday, November 11, 2021 10:13 PM
> > >> To: Harry Wentland 
> > >> Cc: Shankar, Uma ;
> > >> intel-...@lists.freedesktop.org; dri- de...@lists.freedesktop.org;
> > >> ppaala...@gmail.com; brian.star...@arm.com;
> > >> sebast...@sebastianwick.net; shashank.sha...@amd.com
> > >> Subject: Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range
> > >> struct for HDR planes
> > >>
> > >> On Thu, Nov 11, 2021 at 10:17:17AM -0500, Harry Wentland wrote:  
> > >>>
> > >>>
> > >>> On 2021-09-06 17:38, Uma Shankar wrote:  
> > >>>> Define the structure with XE_LPD degamma lut ranges. HDR and SDR
> > >>>> planes have different capabilities, implemented respective
> > >>>> structure for the HDR planes.
> > >>>>
> > >>>> Signed-off-by: Uma Shankar 
> > >>>> ---
> > >>>>  drivers/gpu/drm/i915/display/intel_color.c | 52
> > >>>> ++
> > >>>>  1 file changed, 52 insertions(+)
> > >>>>
> > >>>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> > >>>> b/drivers/gpu/drm/i915/display/intel_color.c
> > >>>> index afcb4bf3826c..6403bd74324b 100644
> > >>>> --- a/drivers/gpu/drm/i915/display/intel_color.c
> > >>>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > >>>> @@ -2092,6 +2092,58 @@ static void icl_read_luts(struct
> > >>>> intel_crtc_state  
> > >> *crtc_state)  
> > >>>>}
> > >>>>  }
> > >>>>
> > >>>> + /* FIXME input bpc? */
> > >>>> +__maybe_unused
> > >>>> +static const struct drm_color_lut_range d13_degamma_hdr[] = {
> > >>>> +  /* segment 1 */
> > >>>> +  {
> > >>>> +  .flags = (DRM_MODE_LUT_GAMMA |
> > >>>> +DRM_MODE_LUT_REFLECT_NEGATIVE |
> > >>>> +DRM_MODE_LUT_INTERPOLATE |
> > >>>> +DRM_MODE_LUT_NON_DECREASING),
> > >>>> +  .count = 128,
> > >>>> +  .input_bpc = 24, .output_bpc = 16,
> > >>>> +  .start = 0, .end = (1 << 24) - 1,
> > >>>> +  .min = 0, .max = (1 << 24) - 1,
> > >>>> +  },
> > >>>> +  /* segment 2 */
> > >>>> +  {
> > >>>> +  .flags = (DRM_MODE_LUT_GAMMA |
> > >>>> +DRM_MODE_LUT_REFLECT_NEGATIVE |
> > >>>> +DRM_MODE_LUT_INTERPOLATE |
> > >>>> +DRM_MODE_LUT_REUSE_LAST |
> > >>>> +DRM_MODE_LUT_NON_DECREASING),
> > >>>> +  .count = 1,
> > >>>> +  .input_bpc = 24, .output_bpc = 16,
> > >>>> +  .start = (1 << 24) - 1, .end = 1 << 24,
> > >>>> +  .min = 0, .max = (1 << 27) - 1,
> > >>>> +  },
> > >>>> +  /* Segment 3 */
> > >>>> +  {
> > >>>> +  .flags = (DRM_MODE_LUT_GAMMA |
> > >>>> +DRM_MODE_LUT_REFLECT_NEGATIVE |
> > >>>> +DRM_MODE_LUT_INTERPOLATE |
> > >>>> +DRM_MODE_LUT_REUSE_LAST |
> > >>>> +DRM_MODE_LUT_NON_DECREASING),
> > >>>> +  .count = 1,
> > >>>> +  .input_bpc =

RE: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

2021-11-11 Thread Shankar, Uma 


> -Original Message-
> From: Harry Wentland 
> Sent: Friday, November 12, 2021 2:41 AM
> To: Shankar, Uma ; Ville Syrjälä
> 
> Cc: intel-...@lists.freedesktop.org; dri-devel@lists.freedesktop.org;
> ppaala...@gmail.com; brian.star...@arm.com; sebast...@sebastianwick.net;
> shashank.sha...@amd.com
> Subject: Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct 
> for
> HDR planes
> 
> 
> 
> On 2021-11-11 15:42, Shankar, Uma wrote:
> >
> >
> >> -Original Message-
> >> From: Ville Syrjälä 
> >> Sent: Thursday, November 11, 2021 10:13 PM
> >> To: Harry Wentland 
> >> Cc: Shankar, Uma ;
> >> intel-...@lists.freedesktop.org; dri- de...@lists.freedesktop.org;
> >> ppaala...@gmail.com; brian.star...@arm.com;
> >> sebast...@sebastianwick.net; shashank.sha...@amd.com
> >> Subject: Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range
> >> struct for HDR planes
> >>
> >> On Thu, Nov 11, 2021 at 10:17:17AM -0500, Harry Wentland wrote:
> >>>
> >>>
> >>> On 2021-09-06 17:38, Uma Shankar wrote:
> >>>> Define the structure with XE_LPD degamma lut ranges. HDR and SDR
> >>>> planes have different capabilities, implemented respective
> >>>> structure for the HDR planes.
> >>>>
> >>>> Signed-off-by: Uma Shankar 
> >>>> ---
> >>>>  drivers/gpu/drm/i915/display/intel_color.c | 52
> >>>> ++
> >>>>  1 file changed, 52 insertions(+)
> >>>>
> >>>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> >>>> b/drivers/gpu/drm/i915/display/intel_color.c
> >>>> index afcb4bf3826c..6403bd74324b 100644
> >>>> --- a/drivers/gpu/drm/i915/display/intel_color.c
> >>>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> >>>> @@ -2092,6 +2092,58 @@ static void icl_read_luts(struct
> >>>> intel_crtc_state
> >> *crtc_state)
> >>>>  }
> >>>>  }
> >>>>
> >>>> + /* FIXME input bpc? */
> >>>> +__maybe_unused
> >>>> +static const struct drm_color_lut_range d13_degamma_hdr[] = {
> >>>> +/* segment 1 */
> >>>> +{
> >>>> +.flags = (DRM_MODE_LUT_GAMMA |
> >>>> +  DRM_MODE_LUT_REFLECT_NEGATIVE |
> >>>> +  DRM_MODE_LUT_INTERPOLATE |
> >>>> +  DRM_MODE_LUT_NON_DECREASING),
> >>>> +.count = 128,
> >>>> +.input_bpc = 24, .output_bpc = 16,
> >>>> +.start = 0, .end = (1 << 24) - 1,
> >>>> +.min = 0, .max = (1 << 24) - 1,
> >>>> +},
> >>>> +/* segment 2 */
> >>>> +{
> >>>> +.flags = (DRM_MODE_LUT_GAMMA |
> >>>> +  DRM_MODE_LUT_REFLECT_NEGATIVE |
> >>>> +  DRM_MODE_LUT_INTERPOLATE |
> >>>> +  DRM_MODE_LUT_REUSE_LAST |
> >>>> +  DRM_MODE_LUT_NON_DECREASING),
> >>>> +.count = 1,
> >>>> +.input_bpc = 24, .output_bpc = 16,
> >>>> +.start = (1 << 24) - 1, .end = 1 << 24,
> >>>> +.min = 0, .max = (1 << 27) - 1,
> >>>> +},
> >>>> +/* Segment 3 */
> >>>> +{
> >>>> +.flags = (DRM_MODE_LUT_GAMMA |
> >>>> +  DRM_MODE_LUT_REFLECT_NEGATIVE |
> >>>> +  DRM_MODE_LUT_INTERPOLATE |
> >>>> +  DRM_MODE_LUT_REUSE_LAST |
> >>>> +  DRM_MODE_LUT_NON_DECREASING),
> >>>> +.count = 1,
> >>>> +.input_bpc = 24, .output_bpc = 16,
> >>>> +.start = 1 << 24, .end = 3 << 24,
> >>>> +.min = 0, .max = (1 << 27) - 1,
> >>>> +},
> >>>> +/* Segment 4 */
> >>>> +{
> >>>> +.flags = (DRM_MODE_LUT_GAMMA |
> >>>> +  DRM_MODE_LUT_REFLECT_NEGATIVE |
>

Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

2021-11-11 Thread Harry Wentland 



On 2021-11-11 15:42, Shankar, Uma wrote:
> 
> 
>> -Original Message-
>> From: Ville Syrjälä 
>> Sent: Thursday, November 11, 2021 10:13 PM
>> To: Harry Wentland 
>> Cc: Shankar, Uma ; intel-...@lists.freedesktop.org; 
>> dri-
>> de...@lists.freedesktop.org; ppaala...@gmail.com; brian.star...@arm.com;
>> sebast...@sebastianwick.net; shashank.sha...@amd.com
>> Subject: Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct 
>> for
>> HDR planes
>>
>> On Thu, Nov 11, 2021 at 10:17:17AM -0500, Harry Wentland wrote:
>>>
>>>
>>> On 2021-09-06 17:38, Uma Shankar wrote:
>>>> Define the structure with XE_LPD degamma lut ranges. HDR and SDR
>>>> planes have different capabilities, implemented respective structure
>>>> for the HDR planes.
>>>>
>>>> Signed-off-by: Uma Shankar 
>>>> ---
>>>>  drivers/gpu/drm/i915/display/intel_color.c | 52
>>>> ++
>>>>  1 file changed, 52 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
>>>> b/drivers/gpu/drm/i915/display/intel_color.c
>>>> index afcb4bf3826c..6403bd74324b 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>>>> @@ -2092,6 +2092,58 @@ static void icl_read_luts(struct intel_crtc_state
>> *crtc_state)
>>>>}
>>>>  }
>>>>
>>>> + /* FIXME input bpc? */
>>>> +__maybe_unused
>>>> +static const struct drm_color_lut_range d13_degamma_hdr[] = {
>>>> +  /* segment 1 */
>>>> +  {
>>>> +  .flags = (DRM_MODE_LUT_GAMMA |
>>>> +DRM_MODE_LUT_REFLECT_NEGATIVE |
>>>> +DRM_MODE_LUT_INTERPOLATE |
>>>> +DRM_MODE_LUT_NON_DECREASING),
>>>> +  .count = 128,
>>>> +  .input_bpc = 24, .output_bpc = 16,
>>>> +  .start = 0, .end = (1 << 24) - 1,
>>>> +  .min = 0, .max = (1 << 24) - 1,
>>>> +  },
>>>> +  /* segment 2 */
>>>> +  {
>>>> +  .flags = (DRM_MODE_LUT_GAMMA |
>>>> +DRM_MODE_LUT_REFLECT_NEGATIVE |
>>>> +DRM_MODE_LUT_INTERPOLATE |
>>>> +DRM_MODE_LUT_REUSE_LAST |
>>>> +DRM_MODE_LUT_NON_DECREASING),
>>>> +  .count = 1,
>>>> +  .input_bpc = 24, .output_bpc = 16,
>>>> +  .start = (1 << 24) - 1, .end = 1 << 24,
>>>> +  .min = 0, .max = (1 << 27) - 1,
>>>> +  },
>>>> +  /* Segment 3 */
>>>> +  {
>>>> +  .flags = (DRM_MODE_LUT_GAMMA |
>>>> +DRM_MODE_LUT_REFLECT_NEGATIVE |
>>>> +DRM_MODE_LUT_INTERPOLATE |
>>>> +DRM_MODE_LUT_REUSE_LAST |
>>>> +DRM_MODE_LUT_NON_DECREASING),
>>>> +  .count = 1,
>>>> +  .input_bpc = 24, .output_bpc = 16,
>>>> +  .start = 1 << 24, .end = 3 << 24,
>>>> +  .min = 0, .max = (1 << 27) - 1,
>>>> +  },
>>>> +  /* Segment 4 */
>>>> +  {
>>>> +  .flags = (DRM_MODE_LUT_GAMMA |
>>>> +DRM_MODE_LUT_REFLECT_NEGATIVE |
>>>> +DRM_MODE_LUT_INTERPOLATE |
>>>> +DRM_MODE_LUT_REUSE_LAST |
>>>> +DRM_MODE_LUT_NON_DECREASING),
>>>> +  .count = 1,
>>>> +  .input_bpc = 24, .output_bpc = 16,
>>>> +  .start = 3 << 24, .end = 7 << 24,
>>>> +  .min = 0, .max = (1 << 27) - 1,
>>>> +  },
>>>> +};
>>>
>>> If I understand this right, userspace would need this definition in
>>> order to populate the degamma blob. Should this sit in a UAPI header?
> 
> Hi Harry, Pekka and Ville,
> Sorry for being a bit late on the replies, got side tracked with various 
> issues.
> I am back on this. Apologies for delay.
> 
>> My original idea (not sure it's fully realized in this series) is to have a 
>> new
>> GAMMA_MODE/etc. enum property on each crtc (or plane) for which each enum
>> value points to a kernel provided blob that contains one of these

RE: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

2021-11-11 Thread Shankar, Uma 



> -Original Message-
> From: Ville Syrjälä 
> Sent: Thursday, November 11, 2021 10:13 PM
> To: Harry Wentland 
> Cc: Shankar, Uma ; intel-...@lists.freedesktop.org; 
> dri-
> de...@lists.freedesktop.org; ppaala...@gmail.com; brian.star...@arm.com;
> sebast...@sebastianwick.net; shashank.sha...@amd.com
> Subject: Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct 
> for
> HDR planes
> 
> On Thu, Nov 11, 2021 at 10:17:17AM -0500, Harry Wentland wrote:
> >
> >
> > On 2021-09-06 17:38, Uma Shankar wrote:
> > > Define the structure with XE_LPD degamma lut ranges. HDR and SDR
> > > planes have different capabilities, implemented respective structure
> > > for the HDR planes.
> > >
> > > Signed-off-by: Uma Shankar 
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_color.c | 52
> > > ++
> > >  1 file changed, 52 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> > > b/drivers/gpu/drm/i915/display/intel_color.c
> > > index afcb4bf3826c..6403bd74324b 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > > @@ -2092,6 +2092,58 @@ static void icl_read_luts(struct intel_crtc_state
> *crtc_state)
> > >   }
> > >  }
> > >
> > > + /* FIXME input bpc? */
> > > +__maybe_unused
> > > +static const struct drm_color_lut_range d13_degamma_hdr[] = {
> > > + /* segment 1 */
> > > + {
> > > + .flags = (DRM_MODE_LUT_GAMMA |
> > > +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> > > +   DRM_MODE_LUT_INTERPOLATE |
> > > +   DRM_MODE_LUT_NON_DECREASING),
> > > + .count = 128,
> > > + .input_bpc = 24, .output_bpc = 16,
> > > + .start = 0, .end = (1 << 24) - 1,
> > > + .min = 0, .max = (1 << 24) - 1,
> > > + },
> > > + /* segment 2 */
> > > + {
> > > + .flags = (DRM_MODE_LUT_GAMMA |
> > > +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> > > +   DRM_MODE_LUT_INTERPOLATE |
> > > +   DRM_MODE_LUT_REUSE_LAST |
> > > +   DRM_MODE_LUT_NON_DECREASING),
> > > + .count = 1,
> > > + .input_bpc = 24, .output_bpc = 16,
> > > + .start = (1 << 24) - 1, .end = 1 << 24,
> > > + .min = 0, .max = (1 << 27) - 1,
> > > + },
> > > + /* Segment 3 */
> > > + {
> > > + .flags = (DRM_MODE_LUT_GAMMA |
> > > +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> > > +   DRM_MODE_LUT_INTERPOLATE |
> > > +   DRM_MODE_LUT_REUSE_LAST |
> > > +   DRM_MODE_LUT_NON_DECREASING),
> > > + .count = 1,
> > > + .input_bpc = 24, .output_bpc = 16,
> > > + .start = 1 << 24, .end = 3 << 24,
> > > + .min = 0, .max = (1 << 27) - 1,
> > > + },
> > > + /* Segment 4 */
> > > + {
> > > + .flags = (DRM_MODE_LUT_GAMMA |
> > > +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> > > +   DRM_MODE_LUT_INTERPOLATE |
> > > +   DRM_MODE_LUT_REUSE_LAST |
> > > +   DRM_MODE_LUT_NON_DECREASING),
> > > + .count = 1,
> > > + .input_bpc = 24, .output_bpc = 16,
> > > + .start = 3 << 24, .end = 7 << 24,
> > > + .min = 0, .max = (1 << 27) - 1,
> > > + },
> > > +};
> >
> > If I understand this right, userspace would need this definition in
> > order to populate the degamma blob. Should this sit in a UAPI header?

Hi Harry, Pekka and Ville,
Sorry for being a bit late on the replies, got side tracked with various issues.
I am back on this. Apologies for delay.

> My original idea (not sure it's fully realized in this series) is to have a 
> new
> GAMMA_MODE/etc. enum property on each crtc (or plane) for which each enum
> value points to a kernel provided blob that contains one of these LUT 
> descriptors.
> Userspace can then query them dynamically and pick the best one for its 
> current use
> case.

We have this as part of the series Ville. Patch 3 of this series creates a 
DEGAMMA_MODE
property just for this. With that userspace can just query the blob_id's and 
will get the
various degamma mode possible and the respective segment

Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

2021-11-11 Thread Ville Syrjälä 
On Thu, Nov 11, 2021 at 10:17:17AM -0500, Harry Wentland wrote:
> 
> 
> On 2021-09-06 17:38, Uma Shankar wrote:
> > Define the structure with XE_LPD degamma lut ranges. HDR and SDR
> > planes have different capabilities, implemented respective
> > structure for the HDR planes.
> > 
> > Signed-off-by: Uma Shankar 
> > ---
> >  drivers/gpu/drm/i915/display/intel_color.c | 52 ++
> >  1 file changed, 52 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
> > b/drivers/gpu/drm/i915/display/intel_color.c
> > index afcb4bf3826c..6403bd74324b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > @@ -2092,6 +2092,58 @@ static void icl_read_luts(struct intel_crtc_state 
> > *crtc_state)
> > }
> >  }
> >  
> > + /* FIXME input bpc? */
> > +__maybe_unused
> > +static const struct drm_color_lut_range d13_degamma_hdr[] = {
> > +   /* segment 1 */
> > +   {
> > +   .flags = (DRM_MODE_LUT_GAMMA |
> > + DRM_MODE_LUT_REFLECT_NEGATIVE |
> > + DRM_MODE_LUT_INTERPOLATE |
> > + DRM_MODE_LUT_NON_DECREASING),
> > +   .count = 128,
> > +   .input_bpc = 24, .output_bpc = 16,
> > +   .start = 0, .end = (1 << 24) - 1,
> > +   .min = 0, .max = (1 << 24) - 1,
> > +   },
> > +   /* segment 2 */
> > +   {
> > +   .flags = (DRM_MODE_LUT_GAMMA |
> > + DRM_MODE_LUT_REFLECT_NEGATIVE |
> > + DRM_MODE_LUT_INTERPOLATE |
> > + DRM_MODE_LUT_REUSE_LAST |
> > + DRM_MODE_LUT_NON_DECREASING),
> > +   .count = 1,
> > +   .input_bpc = 24, .output_bpc = 16,
> > +   .start = (1 << 24) - 1, .end = 1 << 24,
> > +   .min = 0, .max = (1 << 27) - 1,
> > +   },
> > +   /* Segment 3 */
> > +   {
> > +   .flags = (DRM_MODE_LUT_GAMMA |
> > + DRM_MODE_LUT_REFLECT_NEGATIVE |
> > + DRM_MODE_LUT_INTERPOLATE |
> > + DRM_MODE_LUT_REUSE_LAST |
> > + DRM_MODE_LUT_NON_DECREASING),
> > +   .count = 1,
> > +   .input_bpc = 24, .output_bpc = 16,
> > +   .start = 1 << 24, .end = 3 << 24,
> > +   .min = 0, .max = (1 << 27) - 1,
> > +   },
> > +   /* Segment 4 */
> > +   {
> > +   .flags = (DRM_MODE_LUT_GAMMA |
> > + DRM_MODE_LUT_REFLECT_NEGATIVE |
> > + DRM_MODE_LUT_INTERPOLATE |
> > + DRM_MODE_LUT_REUSE_LAST |
> > + DRM_MODE_LUT_NON_DECREASING),
> > +   .count = 1,
> > +   .input_bpc = 24, .output_bpc = 16,
> > +   .start = 3 << 24, .end = 7 << 24,
> > +   .min = 0, .max = (1 << 27) - 1,
> > +   },
> > +};
> 
> If I understand this right, userspace would need this definition in order
> to populate the degamma blob. Should this sit in a UAPI header?

My original idea (not sure it's fully realized in this series) is to
have a new GAMMA_MODE/etc. enum property on each crtc (or plane) for
which each enum value points to a kernel provided blob that contains
one of these LUT descriptors. Userspace can then query them dynamically
and pick the best one for its current use case.

The algorithm for choosing the best one might be something like:
- prefer LUT with bpc >= FB bpc, but perhaps not needlessly high bpc
- prefer interpolated vs. direct lookup based on current needs (eg. X
  could prefer direct lookup to get directcolor visuals).
- prefer one with extended range values if needed
- for HDR prefer smaller step size in dark tones,
  for SDR perhaps prefer a more uniform step size

Or maybe we should include some kind of usage hints as well?

And I was thinking of even adding a new property type (eg.
ENUM_BLOB) just for this sort of usecase. That could let us
have a bit more generic code to do all the validation around
the property values and whatnot.

The one nagging concern I really have with GAMMA_MODE is how a
mix of old and new userspace would work. Though that is more 
of a generic issue with any new property really.

-- 
Ville Syrjälä
Intel

Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

2021-11-11 Thread Harry Wentland 



On 2021-09-06 17:38, Uma Shankar wrote:
> Define the structure with XE_LPD degamma lut ranges. HDR and SDR
> planes have different capabilities, implemented respective
> structure for the HDR planes.
> 
> Signed-off-by: Uma Shankar 
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 52 ++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
> b/drivers/gpu/drm/i915/display/intel_color.c
> index afcb4bf3826c..6403bd74324b 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -2092,6 +2092,58 @@ static void icl_read_luts(struct intel_crtc_state 
> *crtc_state)
>   }
>  }
>  
> + /* FIXME input bpc? */
> +__maybe_unused
> +static const struct drm_color_lut_range d13_degamma_hdr[] = {
> + /* segment 1 */
> + {
> + .flags = (DRM_MODE_LUT_GAMMA |
> +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> +   DRM_MODE_LUT_INTERPOLATE |
> +   DRM_MODE_LUT_NON_DECREASING),
> + .count = 128,
> + .input_bpc = 24, .output_bpc = 16,
> + .start = 0, .end = (1 << 24) - 1,
> + .min = 0, .max = (1 << 24) - 1,
> + },
> + /* segment 2 */
> + {
> + .flags = (DRM_MODE_LUT_GAMMA |
> +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> +   DRM_MODE_LUT_INTERPOLATE |
> +   DRM_MODE_LUT_REUSE_LAST |
> +   DRM_MODE_LUT_NON_DECREASING),
> + .count = 1,
> + .input_bpc = 24, .output_bpc = 16,
> + .start = (1 << 24) - 1, .end = 1 << 24,
> + .min = 0, .max = (1 << 27) - 1,
> + },
> + /* Segment 3 */
> + {
> + .flags = (DRM_MODE_LUT_GAMMA |
> +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> +   DRM_MODE_LUT_INTERPOLATE |
> +   DRM_MODE_LUT_REUSE_LAST |
> +   DRM_MODE_LUT_NON_DECREASING),
> + .count = 1,
> + .input_bpc = 24, .output_bpc = 16,
> + .start = 1 << 24, .end = 3 << 24,
> + .min = 0, .max = (1 << 27) - 1,
> + },
> + /* Segment 4 */
> + {
> + .flags = (DRM_MODE_LUT_GAMMA |
> +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> +   DRM_MODE_LUT_INTERPOLATE |
> +   DRM_MODE_LUT_REUSE_LAST |
> +   DRM_MODE_LUT_NON_DECREASING),
> + .count = 1,
> + .input_bpc = 24, .output_bpc = 16,
> + .start = 3 << 24, .end = 7 << 24,
> + .min = 0, .max = (1 << 27) - 1,
> + },
> +};

If I understand this right, userspace would need this definition in order
to populate the degamma blob. Should this sit in a UAPI header?

Harry

> +
>  void intel_color_init(struct intel_crtc *crtc)
>  {
>   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>

Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

2021-11-09 Thread Harry Wentland 



On 2021-11-09 16:45, Ville Syrjälä wrote:
> On Tue, Nov 09, 2021 at 03:19:47PM -0500, Harry Wentland wrote:
>> On 2021-11-05 08:59, Ville Syrjälä wrote:
>>> On Wed, Nov 03, 2021 at 11:10:37AM -0400, Harry Wentland wrote:


 On 2021-09-06 17:38, Uma Shankar wrote:
> Define the structure with XE_LPD degamma lut ranges. HDR and SDR
> planes have different capabilities, implemented respective
> structure for the HDR planes.
>
> Signed-off-by: Uma Shankar 
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 52 ++
>  1 file changed, 52 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
> b/drivers/gpu/drm/i915/display/intel_color.c
> index afcb4bf3826c..6403bd74324b 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -2092,6 +2092,58 @@ static void icl_read_luts(struct intel_crtc_state 
> *crtc_state)
>   }
>  }
>  
> + /* FIXME input bpc? */
> +__maybe_unused
> +static const struct drm_color_lut_range d13_degamma_hdr[] = {
> + /* segment 1 */
> + {
> + .flags = (DRM_MODE_LUT_GAMMA |
> +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> +   DRM_MODE_LUT_INTERPOLATE |
> +   DRM_MODE_LUT_NON_DECREASING),
> + .count = 128,

 Is the distribution of the 128 entries uniform?
>>>
>>> I guess this is the plane gamma thing despite being in intel_color.c,
>>> so yeah I think that's correct.
>>>
 If so, is a
 uniform distribution of 128 points across most of the LUT
 good enough for HDR with 128 entries?
>>>
>>> No idea how good this actually is. It is .24 so at least
>>> it does have a fair bit of precision.
>>>
>>
>> Precision is good but you also need enough samples. Though that's
>> probably less my concern and more your concern and should become
>> apparent once its used.
> 
> Yeah, for pipe gamma we have a few different variants with
> non-uniform spacing of the samples. But not here AFAICS for 
> whatever reason.
> 
>>

> + .input_bpc = 24, .output_bpc = 16,
> + .start = 0, .end = (1 << 24) - 1,
> + .min = 0, .max = (1 << 24) - 1,
> + },
> + /* segment 2 */
> + {
> + .flags = (DRM_MODE_LUT_GAMMA |
> +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> +   DRM_MODE_LUT_INTERPOLATE |
> +   DRM_MODE_LUT_REUSE_LAST |
> +   DRM_MODE_LUT_NON_DECREASING),
> + .count = 1,
> + .input_bpc = 24, .output_bpc = 16,
> + .start = (1 << 24) - 1, .end = 1 << 24,

 .start and .end are only a single entry apart. Is this correct?
>>>
>>> One think I wanted to do is simplify this stuff by getting rid of
>>> .end entirely. So I think this should just be '.start=1<<24' (or
>>> whatever way we decide to specify the input precision, which is
>>> I think another slightly open question).
>>>
>>> So for this thing we could just have:
>>> { .count = 128, .min = 0, .max = (1 << 24) - 1, .start = 0   },
>>> { .count = 1,   .min = 0, .max = (7 << 24) - 1, .start = 1 << 24 },
>>> { .count = 1,   .min = 0, .max = (7 << 24) - 1, .start = 3 << 24 },
>>> { .count = 1,   .min = 0, .max = (7 << 24) - 1, .start = 7 << 24 },
>>>
>>> + flags/etc. which I left out for brevity.
>>>
>>
>> Makes sense. I like this.
>>
>>> So that is trying to indicate that the first 129 entries are equally
>>> spaced, and would be used to interpolate for input values [0.0,1.0).
>>> Input values [1.0,3.0) would interpolate between entry 128 and 129,
>>> and [3.0,7.0) would interpolate between entry 129 and 130.
>>>
>>
>> What in the segment definition defines the 1.0 mark? In your example
>> it seems to be at (1 << 24) but then we would have values that go
>> beyond the input_bpc for the last three segments.
> 
> Yes, input_bpc would define the precision of the input values (.start).
> so 1.0 would be at 1< extend outside the 0.0-1.0 range.
> 
>>
>> How about output_bpc? Would output_bpc somehow limit the U32.32 (or
>> S31.32) entries, and if so, how?
> 
> output_bpc would define the actual precision of the output values,
> so again 1.0 would be 1< min/max values (which can extend outside the 0.0-1.0 range). The
> alternative I guess would be to not have .output_bpc at all and
> just have .min/.max be s32.32 values. Though then you can't tell
> what the actual precision is. Same could be done for .input_bpc
> I suppose.
> 
>>
>> Or should we treat input_/output_bpc only as capability reporting, so
>> userspace can calculate the possible error when programming the LUT?
>> Again, this leaves us with the question what the input_/output_bpc
>> means for our PWL entries.
> 
> Yeah, I mostly thought they might be interesting if userspace wants
> to know the exact precision. But not strictly necessary if you

Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

2021-11-09 Thread Ville Syrjälä 
On Tue, Nov 09, 2021 at 03:19:47PM -0500, Harry Wentland wrote:
> On 2021-11-05 08:59, Ville Syrjälä wrote:
> > On Wed, Nov 03, 2021 at 11:10:37AM -0400, Harry Wentland wrote:
> >>
> >>
> >> On 2021-09-06 17:38, Uma Shankar wrote:
> >>> Define the structure with XE_LPD degamma lut ranges. HDR and SDR
> >>> planes have different capabilities, implemented respective
> >>> structure for the HDR planes.
> >>>
> >>> Signed-off-by: Uma Shankar 
> >>> ---
> >>>  drivers/gpu/drm/i915/display/intel_color.c | 52 ++
> >>>  1 file changed, 52 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
> >>> b/drivers/gpu/drm/i915/display/intel_color.c
> >>> index afcb4bf3826c..6403bd74324b 100644
> >>> --- a/drivers/gpu/drm/i915/display/intel_color.c
> >>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> >>> @@ -2092,6 +2092,58 @@ static void icl_read_luts(struct intel_crtc_state 
> >>> *crtc_state)
> >>>   }
> >>>  }
> >>>  
> >>> + /* FIXME input bpc? */
> >>> +__maybe_unused
> >>> +static const struct drm_color_lut_range d13_degamma_hdr[] = {
> >>> + /* segment 1 */
> >>> + {
> >>> + .flags = (DRM_MODE_LUT_GAMMA |
> >>> +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> >>> +   DRM_MODE_LUT_INTERPOLATE |
> >>> +   DRM_MODE_LUT_NON_DECREASING),
> >>> + .count = 128,
> >>
> >> Is the distribution of the 128 entries uniform?
> > 
> > I guess this is the plane gamma thing despite being in intel_color.c,
> > so yeah I think that's correct.
> > 
> >> If so, is a
> >> uniform distribution of 128 points across most of the LUT
> >> good enough for HDR with 128 entries?
> > 
> > No idea how good this actually is. It is .24 so at least
> > it does have a fair bit of precision.
> > 
> 
> Precision is good but you also need enough samples. Though that's
> probably less my concern and more your concern and should become
> apparent once its used.

Yeah, for pipe gamma we have a few different variants with
non-uniform spacing of the samples. But not here AFAICS for 
whatever reason.

> 
> >>
> >>> + .input_bpc = 24, .output_bpc = 16,
> >>> + .start = 0, .end = (1 << 24) - 1,
> >>> + .min = 0, .max = (1 << 24) - 1,
> >>> + },
> >>> + /* segment 2 */
> >>> + {
> >>> + .flags = (DRM_MODE_LUT_GAMMA |
> >>> +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> >>> +   DRM_MODE_LUT_INTERPOLATE |
> >>> +   DRM_MODE_LUT_REUSE_LAST |
> >>> +   DRM_MODE_LUT_NON_DECREASING),
> >>> + .count = 1,
> >>> + .input_bpc = 24, .output_bpc = 16,
> >>> + .start = (1 << 24) - 1, .end = 1 << 24,
> >>
> >> .start and .end are only a single entry apart. Is this correct?
> > 
> > One think I wanted to do is simplify this stuff by getting rid of
> > .end entirely. So I think this should just be '.start=1<<24' (or
> > whatever way we decide to specify the input precision, which is
> > I think another slightly open question).
> > 
> > So for this thing we could just have:
> > { .count = 128, .min = 0, .max = (1 << 24) - 1, .start = 0   },
> > { .count = 1,   .min = 0, .max = (7 << 24) - 1, .start = 1 << 24 },
> > { .count = 1,   .min = 0, .max = (7 << 24) - 1, .start = 3 << 24 },
> > { .count = 1,   .min = 0, .max = (7 << 24) - 1, .start = 7 << 24 },
> > 
> > + flags/etc. which I left out for brevity.
> > 
> 
> Makes sense. I like this.
> 
> > So that is trying to indicate that the first 129 entries are equally
> > spaced, and would be used to interpolate for input values [0.0,1.0).
> > Input values [1.0,3.0) would interpolate between entry 128 and 129,
> > and [3.0,7.0) would interpolate between entry 129 and 130.
> > 
> 
> What in the segment definition defines the 1.0 mark? In your example
> it seems to be at (1 << 24) but then we would have values that go
> beyond the input_bpc for the last three segments.

Yes, input_bpc would define the precision of the input values (.start).
so 1.0 would be at 1< 
> How about output_bpc? Would output_bpc somehow limit the U32.32 (or
> S31.32) entries, and if so, how?

output_bpc would define the actual precision of the output values,
so again 1.0 would be 1< 
> Or should we treat input_/output_bpc only as capability reporting, so
> userspace can calculate the possible error when programming the LUT?
> Again, this leaves us with the question what the input_/output_bpc
> means for our PWL entries.

Yeah, I mostly thought they might be interesting if userspace wants
to know the exact precision. But not strictly necessary if you want
just to go generate a "close enough" curve. 

What's PWL?

-- 
Ville Syrjälä
Intel

Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

2021-11-09 Thread Harry Wentland 
On 2021-11-05 08:59, Ville Syrjälä wrote:
> On Wed, Nov 03, 2021 at 11:10:37AM -0400, Harry Wentland wrote:
>>
>>
>> On 2021-09-06 17:38, Uma Shankar wrote:
>>> Define the structure with XE_LPD degamma lut ranges. HDR and SDR
>>> planes have different capabilities, implemented respective
>>> structure for the HDR planes.
>>>
>>> Signed-off-by: Uma Shankar 
>>> ---
>>>  drivers/gpu/drm/i915/display/intel_color.c | 52 ++
>>>  1 file changed, 52 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
>>> b/drivers/gpu/drm/i915/display/intel_color.c
>>> index afcb4bf3826c..6403bd74324b 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>>> @@ -2092,6 +2092,58 @@ static void icl_read_luts(struct intel_crtc_state 
>>> *crtc_state)
>>> }
>>>  }
>>>  
>>> + /* FIXME input bpc? */
>>> +__maybe_unused
>>> +static const struct drm_color_lut_range d13_degamma_hdr[] = {
>>> +   /* segment 1 */
>>> +   {
>>> +   .flags = (DRM_MODE_LUT_GAMMA |
>>> + DRM_MODE_LUT_REFLECT_NEGATIVE |
>>> + DRM_MODE_LUT_INTERPOLATE |
>>> + DRM_MODE_LUT_NON_DECREASING),
>>> +   .count = 128,
>>
>> Is the distribution of the 128 entries uniform?
> 
> I guess this is the plane gamma thing despite being in intel_color.c,
> so yeah I think that's correct.
> 
>> If so, is a
>> uniform distribution of 128 points across most of the LUT
>> good enough for HDR with 128 entries?
> 
> No idea how good this actually is. It is .24 so at least
> it does have a fair bit of precision.
> 

Precision is good but you also need enough samples. Though that's
probably less my concern and more your concern and should become
apparent once its used.

>>
>>> +   .input_bpc = 24, .output_bpc = 16,
>>> +   .start = 0, .end = (1 << 24) - 1,
>>> +   .min = 0, .max = (1 << 24) - 1,
>>> +   },
>>> +   /* segment 2 */
>>> +   {
>>> +   .flags = (DRM_MODE_LUT_GAMMA |
>>> + DRM_MODE_LUT_REFLECT_NEGATIVE |
>>> + DRM_MODE_LUT_INTERPOLATE |
>>> + DRM_MODE_LUT_REUSE_LAST |
>>> + DRM_MODE_LUT_NON_DECREASING),
>>> +   .count = 1,
>>> +   .input_bpc = 24, .output_bpc = 16,
>>> +   .start = (1 << 24) - 1, .end = 1 << 24,
>>
>> .start and .end are only a single entry apart. Is this correct?
> 
> One think I wanted to do is simplify this stuff by getting rid of
> .end entirely. So I think this should just be '.start=1<<24' (or
> whatever way we decide to specify the input precision, which is
> I think another slightly open question).
> 
> So for this thing we could just have:
> { .count = 128, .min = 0, .max = (1 << 24) - 1, .start = 0   },
> { .count = 1,   .min = 0, .max = (7 << 24) - 1, .start = 1 << 24 },
> { .count = 1,   .min = 0, .max = (7 << 24) - 1, .start = 3 << 24 },
> { .count = 1,   .min = 0, .max = (7 << 24) - 1, .start = 7 << 24 },
> 
> + flags/etc. which I left out for brevity.
> 

Makes sense. I like this.

> So that is trying to indicate that the first 129 entries are equally
> spaced, and would be used to interpolate for input values [0.0,1.0).
> Input values [1.0,3.0) would interpolate between entry 128 and 129,
> and [3.0,7.0) would interpolate between entry 129 and 130.
> 

What in the segment definition defines the 1.0 mark? In your example
it seems to be at (1 << 24) but then we would have values that go
beyond the input_bpc for the last three segments.

How about output_bpc? Would output_bpc somehow limit the U32.32 (or
S31.32) entries, and if so, how?

Or should we treat input_/output_bpc only as capability reporting, so
userspace can calculate the possible error when programming the LUT?
Again, this leaves us with the question what the input_/output_bpc
means for our PWL entries.

Harry

Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

2021-11-05 Thread Ville Syrjälä 
On Wed, Nov 03, 2021 at 11:10:37AM -0400, Harry Wentland wrote:
> 
> 
> On 2021-09-06 17:38, Uma Shankar wrote:
> > Define the structure with XE_LPD degamma lut ranges. HDR and SDR
> > planes have different capabilities, implemented respective
> > structure for the HDR planes.
> > 
> > Signed-off-by: Uma Shankar 
> > ---
> >  drivers/gpu/drm/i915/display/intel_color.c | 52 ++
> >  1 file changed, 52 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
> > b/drivers/gpu/drm/i915/display/intel_color.c
> > index afcb4bf3826c..6403bd74324b 100644
> > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > @@ -2092,6 +2092,58 @@ static void icl_read_luts(struct intel_crtc_state 
> > *crtc_state)
> > }
> >  }
> >  
> > + /* FIXME input bpc? */
> > +__maybe_unused
> > +static const struct drm_color_lut_range d13_degamma_hdr[] = {
> > +   /* segment 1 */
> > +   {
> > +   .flags = (DRM_MODE_LUT_GAMMA |
> > + DRM_MODE_LUT_REFLECT_NEGATIVE |
> > + DRM_MODE_LUT_INTERPOLATE |
> > + DRM_MODE_LUT_NON_DECREASING),
> > +   .count = 128,
> 
> Is the distribution of the 128 entries uniform?

I guess this is the plane gamma thing despite being in intel_color.c,
so yeah I think that's correct.

> If so, is a
> uniform distribution of 128 points across most of the LUT
> good enough for HDR with 128 entries?

No idea how good this actually is. It is .24 so at least
it does have a fair bit of precision.

> 
> > +   .input_bpc = 24, .output_bpc = 16,
> > +   .start = 0, .end = (1 << 24) - 1,
> > +   .min = 0, .max = (1 << 24) - 1,
> > +   },
> > +   /* segment 2 */
> > +   {
> > +   .flags = (DRM_MODE_LUT_GAMMA |
> > + DRM_MODE_LUT_REFLECT_NEGATIVE |
> > + DRM_MODE_LUT_INTERPOLATE |
> > + DRM_MODE_LUT_REUSE_LAST |
> > + DRM_MODE_LUT_NON_DECREASING),
> > +   .count = 1,
> > +   .input_bpc = 24, .output_bpc = 16,
> > +   .start = (1 << 24) - 1, .end = 1 << 24,
> 
> .start and .end are only a single entry apart. Is this correct?

One think I wanted to do is simplify this stuff by getting rid of
.end entirely. So I think this should just be '.start=1<<24' (or
whatever way we decide to specify the input precision, which is
I think another slightly open question).

So for this thing we could just have:
{ .count = 128, .min = 0, .max = (1 << 24) - 1, .start = 0   },
{ .count = 1,   .min = 0, .max = (7 << 24) - 1, .start = 1 << 24 },
{ .count = 1,   .min = 0, .max = (7 << 24) - 1, .start = 3 << 24 },
{ .count = 1,   .min = 0, .max = (7 << 24) - 1, .start = 7 << 24 },

+ flags/etc. which I left out for brevity.

So that is trying to indicate that the first 129 entries are equally
spaced, and would be used to interpolate for input values [0.0,1.0).
Input values [1.0,3.0) would interpolate between entry 128 and 129,
and [3.0,7.0) would interpolate between entry 129 and 130.

-- 
Ville Syrjälä
Intel

Re: [RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

2021-11-03 Thread Harry Wentland 



On 2021-09-06 17:38, Uma Shankar wrote:
> Define the structure with XE_LPD degamma lut ranges. HDR and SDR
> planes have different capabilities, implemented respective
> structure for the HDR planes.
> 
> Signed-off-by: Uma Shankar 
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 52 ++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
> b/drivers/gpu/drm/i915/display/intel_color.c
> index afcb4bf3826c..6403bd74324b 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -2092,6 +2092,58 @@ static void icl_read_luts(struct intel_crtc_state 
> *crtc_state)
>   }
>  }
>  
> + /* FIXME input bpc? */
> +__maybe_unused
> +static const struct drm_color_lut_range d13_degamma_hdr[] = {
> + /* segment 1 */
> + {
> + .flags = (DRM_MODE_LUT_GAMMA |
> +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> +   DRM_MODE_LUT_INTERPOLATE |
> +   DRM_MODE_LUT_NON_DECREASING),
> + .count = 128,

Is the distribution of the 128 entries uniform? If so, is a
uniform distribution of 128 points across most of the LUT
good enough for HDR with 128 entries?

> + .input_bpc = 24, .output_bpc = 16,
> + .start = 0, .end = (1 << 24) - 1,
> + .min = 0, .max = (1 << 24) - 1,
> + },
> + /* segment 2 */
> + {
> + .flags = (DRM_MODE_LUT_GAMMA |
> +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> +   DRM_MODE_LUT_INTERPOLATE |
> +   DRM_MODE_LUT_REUSE_LAST |
> +   DRM_MODE_LUT_NON_DECREASING),
> + .count = 1,
> + .input_bpc = 24, .output_bpc = 16,
> + .start = (1 << 24) - 1, .end = 1 << 24,

.start and .end are only a single entry apart. Is this correct?

Harry

> + .min = 0, .max = (1 << 27) - 1,
> + },
> + /* Segment 3 */
> + {
> + .flags = (DRM_MODE_LUT_GAMMA |
> +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> +   DRM_MODE_LUT_INTERPOLATE |
> +   DRM_MODE_LUT_REUSE_LAST |
> +   DRM_MODE_LUT_NON_DECREASING),
> + .count = 1,
> + .input_bpc = 24, .output_bpc = 16,
> + .start = 1 << 24, .end = 3 << 24,
> + .min = 0, .max = (1 << 27) - 1,
> + },
> + /* Segment 4 */
> + {
> + .flags = (DRM_MODE_LUT_GAMMA |
> +   DRM_MODE_LUT_REFLECT_NEGATIVE |
> +   DRM_MODE_LUT_INTERPOLATE |
> +   DRM_MODE_LUT_REUSE_LAST |
> +   DRM_MODE_LUT_NON_DECREASING),
> + .count = 1,
> + .input_bpc = 24, .output_bpc = 16,
> + .start = 3 << 24, .end = 7 << 24,
> + .min = 0, .max = (1 << 27) - 1,
> + },
> +};
> +
>  void intel_color_init(struct intel_crtc *crtc)
>  {
>   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>

[RFC v2 05/22] drm/i915/xelpd: Define Degamma Lut range struct for HDR planes

2021-09-06 Thread Uma Shankar 
Define the structure with XE_LPD degamma lut ranges. HDR and SDR
planes have different capabilities, implemented respective
structure for the HDR planes.

Signed-off-by: Uma Shankar 
---
 drivers/gpu/drm/i915/display/intel_color.c | 52 ++
 1 file changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c 
b/drivers/gpu/drm/i915/display/intel_color.c
index afcb4bf3826c..6403bd74324b 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -2092,6 +2092,58 @@ static void icl_read_luts(struct intel_crtc_state 
*crtc_state)
}
 }
 
+ /* FIXME input bpc? */
+__maybe_unused
+static const struct drm_color_lut_range d13_degamma_hdr[] = {
+   /* segment 1 */
+   {
+   .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_NON_DECREASING),
+   .count = 128,
+   .input_bpc = 24, .output_bpc = 16,
+   .start = 0, .end = (1 << 24) - 1,
+   .min = 0, .max = (1 << 24) - 1,
+   },
+   /* segment 2 */
+   {
+   .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+   .count = 1,
+   .input_bpc = 24, .output_bpc = 16,
+   .start = (1 << 24) - 1, .end = 1 << 24,
+   .min = 0, .max = (1 << 27) - 1,
+   },
+   /* Segment 3 */
+   {
+   .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+   .count = 1,
+   .input_bpc = 24, .output_bpc = 16,
+   .start = 1 << 24, .end = 3 << 24,
+   .min = 0, .max = (1 << 27) - 1,
+   },
+   /* Segment 4 */
+   {
+   .flags = (DRM_MODE_LUT_GAMMA |
+ DRM_MODE_LUT_REFLECT_NEGATIVE |
+ DRM_MODE_LUT_INTERPOLATE |
+ DRM_MODE_LUT_REUSE_LAST |
+ DRM_MODE_LUT_NON_DECREASING),
+   .count = 1,
+   .input_bpc = 24, .output_bpc = 16,
+   .start = 3 << 24, .end = 7 << 24,
+   .min = 0, .max = (1 << 27) - 1,
+   },
+};
+
 void intel_color_init(struct intel_crtc *crtc)
 {
struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-- 
2.26.2