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@xxxxxxxxx> > > --- > > 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