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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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 <uma.shankar@intel.com>
> > ---
> >  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