On Mon, Apr 29, 2019 at 05:00:21PM +0530, Sharma, Shashank wrote: > > On 4/26/2019 11:59 PM, Ville Syrjälä wrote: > > On Fri, Apr 26, 2019 at 11:31:51PM +0530, Shashank Sharma wrote: > >> ICL introduces a new gamma correction mode in display engine, called > >> multi-segmented-gamma mode. This mode allows users to program the > >> darker region of the gamma curve with sueprfine precision. An > >> example use case for this is HDR curves (like PQ ST-2084). > >> > >> If we plot a gamma correction curve from value range between 0.0 to 1.0, > >> ICL's multi-segment has 3 different sections: > >> - superfine segment: 9 values, ranges between 0 - 1/(128 * 256) > >> - fine segment: 257 values, ranges between 0 - 1/(128) > >> - corase segment: 257 values, ranges between 0 - 1 > >> > >> This patch: > >> - Changes gamma LUTs size for ICL/GEN11 to 262144 entries (8 * 128 * 256), > >> so that userspace can program with highest precision supported. > >> - Changes default gamma mode (non-legacy) to multi-segmented-gamma mode. > >> - Adds functions to program/detect multi-segment gamma. > >> > >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > >> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > >> > >> Suggested-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > >> Signed-off-by: Shashank Sharma <shashank.sharma@xxxxxxxxx> > >> Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx> > >> --- > >> drivers/gpu/drm/i915/i915_pci.c | 3 +- > >> drivers/gpu/drm/i915/intel_color.c | 155 ++++++++++++++++++++++++++++- > >> 2 files changed, 156 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > >> index ffa2ee70a03d..83698951760b 100644 > >> --- a/drivers/gpu/drm/i915/i915_pci.c > >> +++ b/drivers/gpu/drm/i915/i915_pci.c > >> @@ -749,7 +749,8 @@ static const struct intel_device_info intel_cannonlake_info = { > >> GEN(11), \ > >> .ddb_size = 2048, \ > >> .has_logical_ring_elsq = 1, \ > >> - .color = { .degamma_lut_size = 33, .gamma_lut_size = 1024 } > >> + .color = { .degamma_lut_size = 33, .gamma_lut_size = 262144 } > >> + > >> > >> static const struct intel_device_info intel_icelake_11_info = { > >> GEN11_FEATURES, > >> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > >> index ca341a9e47e6..d1fb79a5d764 100644 > >> --- a/drivers/gpu/drm/i915/intel_color.c > >> +++ b/drivers/gpu/drm/i915/intel_color.c > >> @@ -41,6 +41,7 @@ > >> #define CTM_COEFF_ABS(coeff) ((coeff) & (CTM_COEFF_SIGN - 1)) > >> > >> #define LEGACY_LUT_LENGTH 256 > >> +#define ICL_MULTISEG_LUT_LENGTH (256 * 128 * 8) > >> /* > >> * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point > >> * format). This macro takes the coefficient we want transformed and the > >> @@ -58,6 +59,12 @@ > >> > >> #define ILK_CSC_POSTOFF_LIMITED_RANGE (16 * (1 << 12) / 255) > >> > >> +enum icl_ms_gamma_segments { > >> + ICL_MS_GAMMA_SEG_SUPERFINE, > >> + ICL_MS_GAMMA_SEG_FINE, > >> + ICL_MS_GAMMA_SEG_COARSE, > >> +}; > >> + > >> static const u16 ilk_csc_off_zero[3] = {}; > >> > >> static const u16 ilk_csc_coeff_identity[9] = { > >> @@ -767,6 +774,149 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state) > >> } > >> } > >> > >> +/* ilk+ "12.4" interpolated format (high 10 bits) */ > >> +static u32 ilk_lut_12p4_ldw(const struct drm_color_lut *color) > >> +{ > >> + return (color->red >> 6) << 20 | (color->green >> 6) << 10 | > >> + (color->blue >> 6); > >> +} > >> + > >> +/* ilk+ "12.4" interpolated format (low 6 bits) */ > >> +static u32 ilk_lut_12p4_udw(const struct drm_color_lut *color) > >> +{ > >> + return (color->red & 0x3f) << 24 | (color->green & 0x3f) << 14 | > >> + (color->blue & 0x3f); > >> +} > >> + > >> +static void > >> +icl_program_gamma_gcmax(const struct intel_crtc_state *crtc_state) > >> +{ > >> + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); > >> + struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); > >> + enum pipe pipe = crtc->pipe; > >> + > >> + /* > >> + * Program the max register to clamp values > 1.0. > >> + * ToDo: Extend the ABI to be able to program values > >> + * from 1.0 > >> + */ > >> + I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16)); > >> + I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16)); > >> + I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16)); > > This one I think we want to program based on the provide LUT. It's the > > last entry that still gets used in interpolation for <1.0 values. > > Or at least that's the way it works with the 12p4 mode IIRC. I don't > > actually remember how it goes with the multi segment mode. > I am a bit skeptical if its a good idea to program from the LUT, as you > know, the drm_color_lut elements are 16bit per channel, and in order to > program 1.0 we need 17 bits. So even if userspace wants, it cant set the > value of 1.0 as of now. Also, wouldn't it would be good for the > interpolation to have the accurate max value (1.0) of the curve ? If userspace doesn't want the max to be 1.0 then we will do the wrong thing here. But I suppose that wouldn't be a very common thing to do. The better arguemnt for doing what I suggest is that we already do it like that on the other platforms with interpolated gamma (gen4/vlv/chv). -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx