| Message ID | 1455020358-18926-6-git-send-email-lionel.g.landwerlin@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Feb 09, 2016 at 12:19:17PM +0000, Lionel Landwerlin wrote: > Patch based on a previous series by Shashank Sharma. > > v2: Do not read GAMMA_MODE register to figure what mode we're in > > v3: Program PREC_PAL_GC_MAX to clamp pixel values > 1.0 > > Add documentation on how the Broadcast RGB property is affected by > CTM_MATRIX > > v4: Update contributors > > Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> > Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com> > Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com> > --- > Documentation/DocBook/gpu.tmpl | 6 +- > drivers/gpu/drm/i915/i915_drv.c | 24 ++- > drivers/gpu/drm/i915/i915_drv.h | 9 + > drivers/gpu/drm/i915/i915_reg.h | 22 +++ > drivers/gpu/drm/i915/intel_color.c | 367 ++++++++++++++++++++++++++++++----- > drivers/gpu/drm/i915/intel_display.c | 22 ++- > drivers/gpu/drm/i915/intel_drv.h | 6 +- > 7 files changed, 396 insertions(+), 60 deletions(-) > > diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl > index 7c49a92..78b8877 100644 > --- a/Documentation/DocBook/gpu.tmpl > +++ b/Documentation/DocBook/gpu.tmpl > @@ -2152,7 +2152,11 @@ void intel_crt_init(struct drm_device *dev) > <td valign="top" >ENUM</td> > <td valign="top" >{ "Automatic", "Full", "Limited 16:235" }</td> > <td valign="top" >Connector</td> > - <td valign="top" >TBD</td> > + <td valign="top" >When this property is set to Limited 16:235 > + and CTM_MATRIX is set, the hardware will be programmed with > + the result of the multiplication of CTM_MATRIX by the limited > + range matrix to ensure the pixels normaly in the range 0..1.0 > + are remapped to the range 16/255..235/255.</td> > </tr> > <tr> > <td valign="top" >“audio”</td> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 44912ec..b65aa20 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -66,6 +66,9 @@ static struct drm_driver driver; > #define IVB_CURSOR_OFFSETS \ > .cursor_offsets = { CURSOR_A_OFFSET, IVB_CURSOR_B_OFFSET, IVB_CURSOR_C_OFFSET } > > +#define BDW_COLORS \ > + .color = { .degamma_lut_size = 512, .gamma_lut_size = 512 } > + > static const struct intel_device_info intel_i830_info = { > .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2, > .has_overlay = 1, .overlay_needs_physical = 1, > @@ -288,24 +291,28 @@ static const struct intel_device_info intel_haswell_m_info = { > .is_mobile = 1, > }; > > +#define BDW_FEATURES \ > + HSW_FEATURES, \ > + BDW_COLORS > + > static const struct intel_device_info intel_broadwell_d_info = { > - HSW_FEATURES, > + BDW_FEATURES, > .gen = 8, > }; > > static const struct intel_device_info intel_broadwell_m_info = { > - HSW_FEATURES, > + BDW_FEATURES, > .gen = 8, .is_mobile = 1, > }; > > static const struct intel_device_info intel_broadwell_gt3d_info = { > - HSW_FEATURES, > + BDW_FEATURES, > .gen = 8, > .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, > }; > > static const struct intel_device_info intel_broadwell_gt3m_info = { > - HSW_FEATURES, > + BDW_FEATURES, > .gen = 8, .is_mobile = 1, > .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, > }; > @@ -321,13 +328,13 @@ static const struct intel_device_info intel_cherryview_info = { > }; > > static const struct intel_device_info intel_skylake_info = { > - HSW_FEATURES, > + BDW_FEATURES, > .is_skylake = 1, > .gen = 9, > }; > > static const struct intel_device_info intel_skylake_gt3_info = { > - HSW_FEATURES, > + BDW_FEATURES, > .is_skylake = 1, > .gen = 9, > .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, > @@ -345,17 +352,18 @@ static const struct intel_device_info intel_broxton_info = { > .has_fbc = 1, > GEN_DEFAULT_PIPEOFFSETS, > IVB_CURSOR_OFFSETS, > + BDW_COLORS, > }; > > static const struct intel_device_info intel_kabylake_info = { > - HSW_FEATURES, > + BDW_FEATURES, > .is_preliminary = 1, > .is_kabylake = 1, > .gen = 9, > }; > > static const struct intel_device_info intel_kabylake_gt3_info = { > - HSW_FEATURES, > + BDW_FEATURES, > .is_preliminary = 1, > .is_kabylake = 1, > .gen = 9, > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 8216665..c1ca4d0 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -659,6 +659,10 @@ struct drm_i915_display_funcs { > /* render clock increase/decrease */ > /* display clock increase/decrease */ > /* pll clock increase/decrease */ > + > + void (*load_degamma_lut)(struct drm_crtc *crtc); > + void (*load_csc_matrix)(struct drm_crtc *crtc); > + void (*load_gamma_lut)(struct drm_crtc *crtc); > }; > > enum forcewake_domain_id { > @@ -806,6 +810,11 @@ struct intel_device_info { > u8 has_slice_pg:1; > u8 has_subslice_pg:1; > u8 has_eu_pg:1; > + > + struct color_luts { > + u16 degamma_lut_size; > + u16 gamma_lut_size; > + } color; > }; > > #undef DEFINE_FLAG > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index f8b0d6c..1dc5d3b 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -7663,6 +7663,28 @@ enum skl_disp_power_wells { > #define PIPE_CSC_POSTOFF_ME(pipe) _MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME, _PIPE_C_CSC_POSTOFF_ME) > #define PIPE_CSC_POSTOFF_LO(pipe) _MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO, _PIPE_C_CSC_POSTOFF_LO) > > +/* pipe degamma/gamma LUTs on IVB+ */ > +#define _PAL_PREC_INDEX_A 0x4A400 > +#define _PAL_PREC_INDEX_B 0x4AC00 > +#define _PAL_PREC_INDEX_C 0x4B400 > +#define PAL_PREC_10_12_BIT (0 << 31) > +#define PAL_PREC_SPLIT_MODE (1 << 31) > +#define PAL_PREC_AUTO_INCREMENT (1 << 15) > +#define _PAL_PREC_DATA_A 0x4A404 > +#define _PAL_PREC_DATA_B 0x4AC04 > +#define _PAL_PREC_DATA_C 0x4B404 > +#define _PAL_PREC_GC_MAX_A 0x4A410 > +#define _PAL_PREC_GC_MAX_B 0x4AC10 > +#define _PAL_PREC_GC_MAX_C 0x4B410 > +#define _PAL_PREC_EXT_GC_MAX_A 0x4A420 > +#define _PAL_PREC_EXT_GC_MAX_B 0x4AC20 > +#define _PAL_PREC_EXT_GC_MAX_C 0x4B420 > + > +#define PREC_PAL_INDEX(pipe) _MMIO_PIPE3(pipe, _PAL_PREC_INDEX_A, _PAL_PREC_INDEX_B, _PAL_PREC_INDEX_C) > +#define PREC_PAL_DATA(pipe) _MMIO_PIPE3(pipe, _PAL_PREC_DATA_A, _PAL_PREC_DATA_B, _PAL_PREC_DATA_C) > +#define PREC_PAL_GC_MAX(pipe, i) _MMIO(_PIPE3(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B, _PAL_PREC_GC_MAX_C) + (i) * 4) > +#define PREC_PAL_EXT_GC_MAX(pipe, i) _MMIO(_PIPE3(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B, _PAL_PREC_EXT_GC_MAX_C) + (i) * 4) > + > /* MIPI DSI registers */ > > #define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */ > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index 39ca215..782b9d8 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -24,44 +24,169 @@ > > #include "intel_drv.h" > > +#define CTM_COEFF_SIGN (1ULL << 63) > + > +#define CTM_COEFF_1_0 (1ULL << 32) > +#define CTM_COEFF_2_0 (CTM_COEFF_1_0 << 1) > +#define CTM_COEFF_4_0 (CTM_COEFF_2_0 << 1) > +#define CTM_COEFF_0_5 (CTM_COEFF_1_0 >> 1) > +#define CTM_COEFF_0_25 (CTM_COEFF_0_5 >> 1) > +#define CTM_COEFF_0_125 (CTM_COEFF_0_25 >> 1) > + > +#define CTM_COEFF_LIMITED_RANGE ((235ULL - 16ULL) * CTM_COEFF_1_0 / 255) > + > +#define CTM_COEFF_NEGATIVE(coeff) (((coeff) & CTM_COEFF_SIGN) != 0) > +#define CTM_COEFF_ABS(coeff) ((coeff) & (CTM_COEFF_SIGN - 1)) > + > +/* > + * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point > + * format). This macro takes the coefficient we want transformed and the > + * number of fractional bits. > + * > + * We only have a 9 bits precision window which slides depending on the value > + * of the CTM coefficient and we write the value from bit 3. We also round the > + * value. > + */ > +#define I9XX_CSC_COEFF_FP(coeff, fbits) \ > + (clamp_val(((coeff) >> (32 - (fbits) - 3)) + 4, 0, 0xfff) & 0xff8) > + > +#define I9XX_CSC_COEFF_LIMITED_RANGE \ > + I9XX_CSC_COEFF_FP(CTM_COEFF_LIMITED_RANGE, 9) > +#define I9XX_CSC_COEFF_1_0 \ > + ((7 << 12) | I9XX_CSC_COEFF_FP(CTM_COEFF_1_0, 8)) > + > +/* > + * When using limited range, multiply the matrix given by userspace by > + * the matrix that we would use for the limited range. We do the > + * multiplication in U2.30 format. > + */ > +static void ctm_matrix_mult_by_limited(uint64_t *result, > + int64_t *input) > +{ > + int i, j; > + uint64_t limited_coeffs[9] = { CTM_COEFF_LIMITED_RANGE, 0, 0, > + 0, CTM_COEFF_LIMITED_RANGE, 0, > + 0, 0, CTM_COEFF_LIMITED_RANGE }; > + > + for (i = 0; i < ARRAY_SIZE(limited_coeffs); i++) { > + int column = i % 3, row = i / 3; > + int negative = 0; > + > + input[i] = 0; Maybe I'm not understanding the matrix math here, but why are we clearing input[i]? Was this supposed to be result[i] instead? > + for (j = 0; j < 3; j++) { > + int64_t user_coeff = input[j * 3 + column]; > + uint64_t limited_coeff = > + limited_coeffs[row * 3 + j] >> 2; > + uint64_t abs_coeff = > + clamp_val(CTM_COEFF_ABS(user_coeff), > + 0, > + CTM_COEFF_4_0 - 1) >> 2; > + > + if (CTM_COEFF_NEGATIVE(user_coeff)) > + negative = !negative; > + result[i] += limited_coeff * abs_coeff; > + } > + > + result[i] >>= 27; > + if (negative) > + result[i] |= CTM_COEFF_SIGN; Given that the limited_coeffs[] matrix is a diagonal matrix, we don't actually need an inner loop here, right? Would it be simpler to just replace this with something more like the following? absuser_coeff = clamp_val(input[i]); limited_coeff = limited_coeffs[column * 3 + column] >> 2; result[i] = (absuser_coeff * limited_coeff) >> 27; if (CTM_COEFF_NEGATIVE(input[i])) result[i] |= CTM_COEFF_SIGN; Technically you don't even need to actually store limited_coeffs[] in a square matrix, although doing so does help clarify for the reader the kind of matrix math you're performing. > + } > +} > + > /* > * Set up the pipe CSC unit. > * > - * Currently only full range RGB to limited range RGB conversion > - * is supported, but eventually this should handle various > - * RGB<->YCbCr scenarios as well. > + * Currently only full range RGB to limited range RGB conversion is supported, > + * but eventually this should handle various RGB<->YCbCr scenarios as well. > */ > static void i9xx_load_csc_matrix(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > + struct drm_crtc_state *crtc_state = crtc->state; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - int pipe = intel_crtc->pipe; > - uint16_t coeff = 0x7800; /* 1.0 */ > + int i, pipe = intel_crtc->pipe; > + uint16_t coeffs[9] = { 0, }; > > - /* > - * TODO: Check what kind of values actually come out of the pipe > - * with these coeff/postoff values and adjust to get the best > - * accuracy. Perhaps we even need to take the bpc value into > - * consideration. > - */ > + if (crtc_state->ctm_matrix) { > + struct drm_color_ctm *ctm = > + (struct drm_color_ctm *)crtc_state->ctm_matrix->data; > + uint64_t input[9] = { 0, }; > + > + if (intel_crtc->config->limited_color_range) > + ctm_matrix_mult_by_limited(input, ctm->matrix); > + else { Minor nitpick; kernel coding standard says we need to use braces on both branches of an if/else if either of them needs it. > + for (i = 0; i < ARRAY_SIZE(input); i++) > + input[i] = ctm->matrix[i]; > + } > > - if (intel_crtc->config->limited_color_range) > - coeff = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */ > + > + /* > + * Convert fixed point S31.32 input to format supported by the > + * hardware. > + */ > + for (i = 0; i < ARRAY_SIZE(coeffs); i++) { > + uint64_t abs_coeff = ((1ULL << 63) - 1) & input[i]; > + > + /* > + * Clamp input value to min/max supported by > + * hardware. > + */ > + abs_coeff = clamp_val(abs_coeff, 0, CTM_COEFF_4_0 - 1); > + > + /* sign bit */ > + if (CTM_COEFF_NEGATIVE(input[i])) > + coeffs[i] |= 1 << 15; > + > + if (abs_coeff < CTM_COEFF_0_125) > + coeffs[i] |= (3 << 12) | > + I9XX_CSC_COEFF_FP(abs_coeff, 12); > + else if (abs_coeff < CTM_COEFF_0_25) > + coeffs[i] |= (2 << 12) | > + I9XX_CSC_COEFF_FP(abs_coeff, 11); > + else if (abs_coeff < CTM_COEFF_0_5) > + coeffs[i] |= (1 << 12) | > + I9XX_CSC_COEFF_FP(abs_coeff, 10); > + else if (abs_coeff < CTM_COEFF_1_0) > + coeffs[i] |= I9XX_CSC_COEFF_FP(abs_coeff, 9); > + else if (abs_coeff < CTM_COEFF_2_0) > + coeffs[i] |= (7 << 12) | > + I9XX_CSC_COEFF_FP(abs_coeff, 8); > + else > + coeffs[i] |= (6 << 12) | > + I9XX_CSC_COEFF_FP(abs_coeff, 7); > + } > + } else { > + /* > + * Load an identify matrix if no coefficients are provided. > + * > + * TODO: Check what kind of values actually come out of the > + * pipe with these coeff/postoff values and adjust to get the > + * best accuracy. Perhaps we even need to take the bpc value > + * into consideration. > + */ > + for (i = 0; i < 3; i++) { > + if (intel_crtc->config->limited_color_range) > + coeffs[i * 3 + i] = > + I9XX_CSC_COEFF_LIMITED_RANGE; > + else > + coeffs[i * 3 + i] = I9XX_CSC_COEFF_1_0; > + } > + } > > /* > * GY/GU and RY/RU should be the other way around according > * to BSpec, but reality doesn't agree. Just set them up in > * a way that results in the correct picture. > */ Is this comment still relevant/correct? It looks to me like you're programming the same way the bspec recommends now, at least as far as the BDW-SKL section goes. > - I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeff << 16); > - I915_WRITE(PIPE_CSC_COEFF_BY(pipe), 0); > + I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeffs[0] << 16 | coeffs[1]); > + I915_WRITE(PIPE_CSC_COEFF_BY(pipe), coeffs[2] << 16); > > - I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeff); > - I915_WRITE(PIPE_CSC_COEFF_BU(pipe), 0); > + I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeffs[3] << 16 | coeffs[4]); > + I915_WRITE(PIPE_CSC_COEFF_BU(pipe), coeffs[5] << 16); > > - I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), 0); > - I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeff << 16); > + I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), coeffs[6] << 16 | coeffs[7]); > + I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeffs[8] << 16); > > I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0); > I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0); > @@ -88,21 +213,146 @@ static void i9xx_load_csc_matrix(struct drm_crtc *crtc) > } > } > > -void intel_color_set_csc(struct drm_crtc *crtc) > +/** Loads the legacy palette/gamma unit for the CRTC with the prepared > + * values. > + */ > +static void i9xx_load_legacy_gamma_lut(struct drm_crtc *crtc) > { > - i9xx_load_csc_matrix(crtc); > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct intel_crtc_state *intel_state = to_intel_crtc_state(crtc->state); > + enum pipe pipe = to_intel_crtc(crtc)->pipe; > + int i; > + > + for (i = 0; i < 256; i++) { > + uint32_t word = (intel_crtc->lut_r[i] << 16) | > + (intel_crtc->lut_g[i] << 8) | > + intel_crtc->lut_b[i]; > + if (HAS_GMCH_DISPLAY(dev)) > + I915_WRITE(PALETTE(pipe, i), word); > + else > + I915_WRITE(LGC_PALETTE(pipe, i), word); > + } > + > + intel_state->gamma_mode = GAMMA_MODE_MODE_8BIT; > + I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT); > } > > -/** Loads the palette/gamma unit for the CRTC with the prepared values on */ > -static void i9xx_load_legacy_gamma_lut(struct drm_crtc *crtc) > +static void broadwell_load_degamma_lut(struct drm_crtc *crtc) > +{ > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_crtc_state *state = crtc->state; > + struct intel_crtc_state *intel_state = to_intel_crtc_state(state); > + enum pipe pipe = to_intel_crtc(crtc)->pipe; > + uint32_t i, lut_size = INTEL_INFO(dev)->color.degamma_lut_size; > + > + I915_WRITE(PREC_PAL_INDEX(pipe), > + PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT); > + > + if (state->degamma_lut) { > + struct drm_color_lut *lut = > + (struct drm_color_lut *) state->degamma_lut->data; > + > + for (i = 0; i < lut_size; i++) { > + uint32_t word = > + drm_color_lut_extract(lut[i].red, 10) << 20 | > + drm_color_lut_extract(lut[i].green, 10) << 10 | > + drm_color_lut_extract(lut[i].blue, 10); > + > + I915_WRITE(PREC_PAL_DATA(pipe), word); > + } > + } else { > + for (i = 0; i < lut_size; i++) { > + uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1); > + > + I915_WRITE(PREC_PAL_DATA(pipe), > + (v << 20) | (v << 10) | v); > + } > + } > + > + intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT; > + I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT); > + POSTING_READ(GAMMA_MODE(pipe)); > + > + /* Reset the index, otherwise it prevents the legacy palette to be > + * written properly. > + */ > + I915_WRITE(PREC_PAL_INDEX(pipe), 0); > +} > + > +static void broadwell_load_gamma_lut(struct drm_crtc *crtc) > +{ > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + struct drm_crtc_state *state = crtc->state; > + struct intel_crtc_state *intel_state = to_intel_crtc_state(state); > + enum pipe pipe = to_intel_crtc(crtc)->pipe; > + uint32_t i, lut_offset = INTEL_INFO(dev)->color.degamma_lut_size, > + lut_size = INTEL_INFO(dev)->color.gamma_lut_size; > + > + > + I915_WRITE(PREC_PAL_INDEX(pipe), > + PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT | lut_offset); > + > + if (state->gamma_lut) { > + struct drm_color_lut *lut = > + (struct drm_color_lut *) state->gamma_lut->data; > + > + for (i = 0; i < lut_size; i++) { > + uint32_t word = > + (drm_color_lut_extract(lut[i].red, 10) << 20) | > + (drm_color_lut_extract(lut[i].green, 10) << 10) | > + drm_color_lut_extract(lut[i].blue, 10); > + > + I915_WRITE(PREC_PAL_DATA(pipe), word); > + } > + > + /* Program the max register to clamp values > 1.0. */ > + I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), > + drm_color_lut_extract(lut[i].red, 16)); > + I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), > + drm_color_lut_extract(lut[i].green, 16)); > + I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), > + drm_color_lut_extract(lut[i].blue, 16)); > + } else { > + for (i = 0; i < lut_size; i++) { > + uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1); > + > + I915_WRITE(PREC_PAL_DATA(pipe), > + (v << 20) | (v << 10) | v); > + } > + > + I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16) - 1); > + I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16) - 1); > + I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16) - 1); > + } > + > + intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT; > + I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT); > + POSTING_READ(GAMMA_MODE(pipe)); > + > + /* Reset the index, otherwise it prevents the legacy palette to be > + * written properly. > + */ > + I915_WRITE(PREC_PAL_INDEX(pipe), 0); > +} > + > +static void intel_color_load_luts_internal(struct drm_crtc *crtc, > + bool legacy) > { > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > - enum pipe pipe = intel_crtc->pipe; > - int i; > + struct intel_crtc_state *intel_state = to_intel_crtc_state(crtc->state); > + enum pipe pipe = to_intel_crtc(crtc)->pipe; > bool reenable_ips = false; > > + /* The clocks have to be on to load the palette. */ > + if (!crtc->state->active) > + return; > + > if (HAS_GMCH_DISPLAY(dev)) { > if (intel_crtc->config->has_dsi_encoder) > assert_dsi_pll_enabled(dev_priv); > @@ -114,42 +364,32 @@ static void i9xx_load_legacy_gamma_lut(struct drm_crtc *crtc) > * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled. > */ > if (IS_HASWELL(dev) && intel_crtc->config->ips_enabled && > - ((I915_READ(GAMMA_MODE(pipe)) & GAMMA_MODE_MODE_MASK) == > - GAMMA_MODE_MODE_SPLIT)) { > + intel_state->gamma_mode == GAMMA_MODE_MODE_SPLIT) { > hsw_disable_ips(intel_crtc); > reenable_ips = true; > } > > - for (i = 0; i < 256; i++) { > - uint32_t word = (intel_crtc->lut_r[i] << 16) | > - (intel_crtc->lut_g[i] << 8) | > - intel_crtc->lut_b[i]; > - if (HAS_GMCH_DISPLAY(dev)) > - I915_WRITE(PALETTE(pipe, i), word); > - else > - I915_WRITE(LGC_PALETTE(pipe, i), word); > + if (legacy) > + i9xx_load_legacy_gamma_lut(crtc); > + else { > + dev_priv->display.load_degamma_lut(crtc); > + dev_priv->display.load_gamma_lut(crtc); > } > > - I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT); > - > if (reenable_ips) > hsw_enable_ips(intel_crtc); > } > > -void intel_color_load_gamma_lut(struct drm_crtc *crtc) > +void intel_color_legacy_load_lut(struct drm_crtc *crtc) > { > - /* The clocks have to be on to load the palette. */ > - if (!crtc->state->active) > - return; > - > - i9xx_load_legacy_gamma_lut(crtc); > + intel_color_load_luts_internal(crtc, true); > } > > void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, > u16 *blue, uint32_t start, uint32_t size) > { > - int end = (start + size > 256) ? 256 : start + size, i; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + int end = (start + size > 256) ? 256 : start + size, i; > > for (i = start; i < end; i++) { > intel_crtc->lut_r[i] = red[i] >> 8; > @@ -157,11 +397,29 @@ void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, > intel_crtc->lut_b[i] = blue[i] >> 8; > } > > - intel_color_load_gamma_lut(crtc); > + intel_color_load_luts_internal(crtc, true); > +} > + > +void intel_color_load_luts(struct drm_crtc *crtc) > +{ > + intel_color_load_luts_internal(crtc, > + !crtc->state->degamma_lut && > + !crtc->state->gamma_lut); > +} > + > +void intel_color_set_csc(struct drm_crtc *crtc) > +{ > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > + > + if (dev_priv->display.load_csc_matrix) > + dev_priv->display.load_csc_matrix(crtc); > } > > void intel_color_init(struct drm_crtc *crtc) > { > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = dev->dev_private; > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > int i; > > @@ -171,4 +429,23 @@ void intel_color_init(struct drm_crtc *crtc) > intel_crtc->lut_g[i] = i; > intel_crtc->lut_b[i] = i; > } > + > + if (IS_BROADWELL(dev) || IS_SKYLAKE(dev) || > + IS_BROXTON(dev) || IS_KABYLAKE(dev)) { > + dev_priv->display.load_degamma_lut = broadwell_load_degamma_lut; > + dev_priv->display.load_gamma_lut = broadwell_load_gamma_lut; > + dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix; > + } else { > + dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix; > + } > + > + if (INTEL_INFO(dev)->color.degamma_lut_size != 0 && > + INTEL_INFO(dev)->color.gamma_lut_size != 0) { > + WARN_ON(!dev_priv->display.load_degamma_lut || > + !dev_priv->display.load_gamma_lut || > + !dev_priv->display.load_csc_matrix); > + drm_helper_crtc_enable_color_mgmt(crtc, > + INTEL_INFO(dev)->color.degamma_lut_size, > + INTEL_INFO(dev)->color.gamma_lut_size); > + } > } > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index f10bba2..e38b31b 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4864,7 +4864,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) > * On ILK+ LUT must be loaded before the pipe is running but with > * clocks enabled > */ > - intel_color_load_gamma_lut(crtc); > + intel_color_load_luts(crtc); > > intel_update_watermarks(crtc); > intel_enable_pipe(intel_crtc); > @@ -4959,7 +4959,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) > * On ILK+ LUT must be loaded before the pipe is running but with > * clocks enabled > */ > - intel_color_load_gamma_lut(crtc); > + intel_color_load_luts(crtc); > > intel_ddi_set_pipe_settings(crtc); > if (!intel_crtc->config->has_dsi_encoder) > @@ -6174,7 +6174,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) > > i9xx_pfit_enable(intel_crtc); > > - intel_color_load_gamma_lut(crtc); > + intel_color_load_luts(crtc); > > intel_enable_pipe(intel_crtc); > > @@ -6227,7 +6227,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) > > i9xx_pfit_enable(intel_crtc); > > - intel_color_load_gamma_lut(crtc); > + intel_color_load_luts(crtc); > > intel_update_watermarks(crtc); > intel_enable_pipe(intel_crtc); > @@ -11898,7 +11898,7 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, > > static const struct drm_crtc_helper_funcs intel_helper_funcs = { > .mode_set_base_atomic = intel_pipe_set_base_atomic, > - .load_lut = intel_color_load_gamma_lut, > + .load_lut = intel_color_legacy_load_lut, > .atomic_begin = intel_begin_crtc_commit, > .atomic_flush = intel_finish_crtc_commit, > .atomic_check = intel_crtc_atomic_check, > @@ -13428,6 +13428,17 @@ static int intel_atomic_commit(struct drm_device *dev, > hw_check = true; > } > > + if (!modeset && > + crtc->state->active && > + crtc->state->color_mgmt_changed) { > + /* Only update color management when not doing Minor nitpick; kernel coding style says the opening "/*" of multi-line comments should be on a line by itself. > + * a modeset as this will be done by > + * crtc_enable already. > + */ > + intel_color_set_csc(crtc); > + intel_color_load_luts(crtc); > + } > + > if (!modeset) > intel_pre_plane_update(to_intel_crtc_state(crtc_state)); > > @@ -13519,6 +13530,7 @@ out: > static const struct drm_crtc_funcs intel_crtc_funcs = { > .gamma_set = intel_color_legacy_gamma_set, > .set_config = drm_atomic_helper_set_config, > + .set_property = drm_atomic_helper_crtc_set_property, > .destroy = intel_crtc_destroy, > .page_flip = intel_crtc_page_flip, > .atomic_duplicate_state = intel_crtc_duplicate_state, > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 2b5d03a..c384c78 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -507,6 +507,8 @@ struct intel_crtc_state { > /* IVB sprite scaling w/a (WaCxSRDisabledForSpriteScaling:ivb) */ > bool disable_lp_wm; > > + uint32_t gamma_mode; > + > struct { > /* > * optimal watermarks, programmed post-vblank when this state > @@ -1620,9 +1622,11 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs; > > /* intel_color.c */ > void intel_color_init(struct drm_crtc *crtc); > +void intel_color_update(struct drm_crtc *crtc); > void intel_color_set_csc(struct drm_crtc *crtc); > -void intel_color_load_gamma_lut(struct drm_crtc *crtc); > +void intel_color_load_luts(struct drm_crtc *crtc); > void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, > u16 *blue, uint32_t start, uint32_t size); > +void intel_color_legacy_load_lut(struct drm_crtc *crtc); intel_color_load_gamma_lut() was a rename from patch #1 of the series. Maybe we should squash this name change back into that patch, even though we only have a single LUT at that point? I guess it doesn't really matter too much either way. Matt > > #endif /* __INTEL_DRV_H__ */ > -- > 2.7.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl index 7c49a92..78b8877 100644 --- a/Documentation/DocBook/gpu.tmpl +++ b/Documentation/DocBook/gpu.tmpl @@ -2152,7 +2152,11 @@ void intel_crt_init(struct drm_device *dev) <td valign="top" >ENUM</td> <td valign="top" >{ "Automatic", "Full", "Limited 16:235" }</td> <td valign="top" >Connector</td> - <td valign="top" >TBD</td> + <td valign="top" >When this property is set to Limited 16:235 + and CTM_MATRIX is set, the hardware will be programmed with + the result of the multiplication of CTM_MATRIX by the limited + range matrix to ensure the pixels normaly in the range 0..1.0 + are remapped to the range 16/255..235/255.</td> </tr> <tr> <td valign="top" >“audio”</td> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 44912ec..b65aa20 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -66,6 +66,9 @@ static struct drm_driver driver; #define IVB_CURSOR_OFFSETS \ .cursor_offsets = { CURSOR_A_OFFSET, IVB_CURSOR_B_OFFSET, IVB_CURSOR_C_OFFSET } +#define BDW_COLORS \ + .color = { .degamma_lut_size = 512, .gamma_lut_size = 512 } + static const struct intel_device_info intel_i830_info = { .gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2, .has_overlay = 1, .overlay_needs_physical = 1, @@ -288,24 +291,28 @@ static const struct intel_device_info intel_haswell_m_info = { .is_mobile = 1, }; +#define BDW_FEATURES \ + HSW_FEATURES, \ + BDW_COLORS + static const struct intel_device_info intel_broadwell_d_info = { - HSW_FEATURES, + BDW_FEATURES, .gen = 8, }; static const struct intel_device_info intel_broadwell_m_info = { - HSW_FEATURES, + BDW_FEATURES, .gen = 8, .is_mobile = 1, }; static const struct intel_device_info intel_broadwell_gt3d_info = { - HSW_FEATURES, + BDW_FEATURES, .gen = 8, .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, }; static const struct intel_device_info intel_broadwell_gt3m_info = { - HSW_FEATURES, + BDW_FEATURES, .gen = 8, .is_mobile = 1, .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, }; @@ -321,13 +328,13 @@ static const struct intel_device_info intel_cherryview_info = { }; static const struct intel_device_info intel_skylake_info = { - HSW_FEATURES, + BDW_FEATURES, .is_skylake = 1, .gen = 9, }; static const struct intel_device_info intel_skylake_gt3_info = { - HSW_FEATURES, + BDW_FEATURES, .is_skylake = 1, .gen = 9, .ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING, @@ -345,17 +352,18 @@ static const struct intel_device_info intel_broxton_info = { .has_fbc = 1, GEN_DEFAULT_PIPEOFFSETS, IVB_CURSOR_OFFSETS, + BDW_COLORS, }; static const struct intel_device_info intel_kabylake_info = { - HSW_FEATURES, + BDW_FEATURES, .is_preliminary = 1, .is_kabylake = 1, .gen = 9, }; static const struct intel_device_info intel_kabylake_gt3_info = { - HSW_FEATURES, + BDW_FEATURES, .is_preliminary = 1, .is_kabylake = 1, .gen = 9, diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8216665..c1ca4d0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -659,6 +659,10 @@ struct drm_i915_display_funcs { /* render clock increase/decrease */ /* display clock increase/decrease */ /* pll clock increase/decrease */ + + void (*load_degamma_lut)(struct drm_crtc *crtc); + void (*load_csc_matrix)(struct drm_crtc *crtc); + void (*load_gamma_lut)(struct drm_crtc *crtc); }; enum forcewake_domain_id { @@ -806,6 +810,11 @@ struct intel_device_info { u8 has_slice_pg:1; u8 has_subslice_pg:1; u8 has_eu_pg:1; + + struct color_luts { + u16 degamma_lut_size; + u16 gamma_lut_size; + } color; }; #undef DEFINE_FLAG diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index f8b0d6c..1dc5d3b 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -7663,6 +7663,28 @@ enum skl_disp_power_wells { #define PIPE_CSC_POSTOFF_ME(pipe) _MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME, _PIPE_C_CSC_POSTOFF_ME) #define PIPE_CSC_POSTOFF_LO(pipe) _MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO, _PIPE_C_CSC_POSTOFF_LO) +/* pipe degamma/gamma LUTs on IVB+ */ +#define _PAL_PREC_INDEX_A 0x4A400 +#define _PAL_PREC_INDEX_B 0x4AC00 +#define _PAL_PREC_INDEX_C 0x4B400 +#define PAL_PREC_10_12_BIT (0 << 31) +#define PAL_PREC_SPLIT_MODE (1 << 31) +#define PAL_PREC_AUTO_INCREMENT (1 << 15) +#define _PAL_PREC_DATA_A 0x4A404 +#define _PAL_PREC_DATA_B 0x4AC04 +#define _PAL_PREC_DATA_C 0x4B404 +#define _PAL_PREC_GC_MAX_A 0x4A410 +#define _PAL_PREC_GC_MAX_B 0x4AC10 +#define _PAL_PREC_GC_MAX_C 0x4B410 +#define _PAL_PREC_EXT_GC_MAX_A 0x4A420 +#define _PAL_PREC_EXT_GC_MAX_B 0x4AC20 +#define _PAL_PREC_EXT_GC_MAX_C 0x4B420 + +#define PREC_PAL_INDEX(pipe) _MMIO_PIPE3(pipe, _PAL_PREC_INDEX_A, _PAL_PREC_INDEX_B, _PAL_PREC_INDEX_C) +#define PREC_PAL_DATA(pipe) _MMIO_PIPE3(pipe, _PAL_PREC_DATA_A, _PAL_PREC_DATA_B, _PAL_PREC_DATA_C) +#define PREC_PAL_GC_MAX(pipe, i) _MMIO(_PIPE3(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B, _PAL_PREC_GC_MAX_C) + (i) * 4) +#define PREC_PAL_EXT_GC_MAX(pipe, i) _MMIO(_PIPE3(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B, _PAL_PREC_EXT_GC_MAX_C) + (i) * 4) + /* MIPI DSI registers */ #define _MIPI_PORT(port, a, c) _PORT3(port, a, 0, c) /* ports A and C only */ diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 39ca215..782b9d8 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -24,44 +24,169 @@ #include "intel_drv.h" +#define CTM_COEFF_SIGN (1ULL << 63) + +#define CTM_COEFF_1_0 (1ULL << 32) +#define CTM_COEFF_2_0 (CTM_COEFF_1_0 << 1) +#define CTM_COEFF_4_0 (CTM_COEFF_2_0 << 1) +#define CTM_COEFF_0_5 (CTM_COEFF_1_0 >> 1) +#define CTM_COEFF_0_25 (CTM_COEFF_0_5 >> 1) +#define CTM_COEFF_0_125 (CTM_COEFF_0_25 >> 1) + +#define CTM_COEFF_LIMITED_RANGE ((235ULL - 16ULL) * CTM_COEFF_1_0 / 255) + +#define CTM_COEFF_NEGATIVE(coeff) (((coeff) & CTM_COEFF_SIGN) != 0) +#define CTM_COEFF_ABS(coeff) ((coeff) & (CTM_COEFF_SIGN - 1)) + +/* + * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point + * format). This macro takes the coefficient we want transformed and the + * number of fractional bits. + * + * We only have a 9 bits precision window which slides depending on the value + * of the CTM coefficient and we write the value from bit 3. We also round the + * value. + */ +#define I9XX_CSC_COEFF_FP(coeff, fbits) \ + (clamp_val(((coeff) >> (32 - (fbits) - 3)) + 4, 0, 0xfff) & 0xff8) + +#define I9XX_CSC_COEFF_LIMITED_RANGE \ + I9XX_CSC_COEFF_FP(CTM_COEFF_LIMITED_RANGE, 9) +#define I9XX_CSC_COEFF_1_0 \ + ((7 << 12) | I9XX_CSC_COEFF_FP(CTM_COEFF_1_0, 8)) + +/* + * When using limited range, multiply the matrix given by userspace by + * the matrix that we would use for the limited range. We do the + * multiplication in U2.30 format. + */ +static void ctm_matrix_mult_by_limited(uint64_t *result, + int64_t *input) +{ + int i, j; + uint64_t limited_coeffs[9] = { CTM_COEFF_LIMITED_RANGE, 0, 0, + 0, CTM_COEFF_LIMITED_RANGE, 0, + 0, 0, CTM_COEFF_LIMITED_RANGE }; + + for (i = 0; i < ARRAY_SIZE(limited_coeffs); i++) { + int column = i % 3, row = i / 3; + int negative = 0; + + input[i] = 0; + for (j = 0; j < 3; j++) { + int64_t user_coeff = input[j * 3 + column]; + uint64_t limited_coeff = + limited_coeffs[row * 3 + j] >> 2; + uint64_t abs_coeff = + clamp_val(CTM_COEFF_ABS(user_coeff), + 0, + CTM_COEFF_4_0 - 1) >> 2; + + if (CTM_COEFF_NEGATIVE(user_coeff)) + negative = !negative; + result[i] += limited_coeff * abs_coeff; + } + + result[i] >>= 27; + if (negative) + result[i] |= CTM_COEFF_SIGN; + } +} + /* * Set up the pipe CSC unit. * - * Currently only full range RGB to limited range RGB conversion - * is supported, but eventually this should handle various - * RGB<->YCbCr scenarios as well. + * Currently only full range RGB to limited range RGB conversion is supported, + * but eventually this should handle various RGB<->YCbCr scenarios as well. */ static void i9xx_load_csc_matrix(struct drm_crtc *crtc) { struct drm_device *dev = crtc->dev; + struct drm_crtc_state *crtc_state = crtc->state; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - int pipe = intel_crtc->pipe; - uint16_t coeff = 0x7800; /* 1.0 */ + int i, pipe = intel_crtc->pipe; + uint16_t coeffs[9] = { 0, }; - /* - * TODO: Check what kind of values actually come out of the pipe - * with these coeff/postoff values and adjust to get the best - * accuracy. Perhaps we even need to take the bpc value into - * consideration. - */ + if (crtc_state->ctm_matrix) { + struct drm_color_ctm *ctm = + (struct drm_color_ctm *)crtc_state->ctm_matrix->data; + uint64_t input[9] = { 0, }; + + if (intel_crtc->config->limited_color_range) + ctm_matrix_mult_by_limited(input, ctm->matrix); + else { + for (i = 0; i < ARRAY_SIZE(input); i++) + input[i] = ctm->matrix[i]; + } - if (intel_crtc->config->limited_color_range) - coeff = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */ + + /* + * Convert fixed point S31.32 input to format supported by the + * hardware. + */ + for (i = 0; i < ARRAY_SIZE(coeffs); i++) { + uint64_t abs_coeff = ((1ULL << 63) - 1) & input[i]; + + /* + * Clamp input value to min/max supported by + * hardware. + */ + abs_coeff = clamp_val(abs_coeff, 0, CTM_COEFF_4_0 - 1); + + /* sign bit */ + if (CTM_COEFF_NEGATIVE(input[i])) + coeffs[i] |= 1 << 15; + + if (abs_coeff < CTM_COEFF_0_125) + coeffs[i] |= (3 << 12) | + I9XX_CSC_COEFF_FP(abs_coeff, 12); + else if (abs_coeff < CTM_COEFF_0_25) + coeffs[i] |= (2 << 12) | + I9XX_CSC_COEFF_FP(abs_coeff, 11); + else if (abs_coeff < CTM_COEFF_0_5) + coeffs[i] |= (1 << 12) | + I9XX_CSC_COEFF_FP(abs_coeff, 10); + else if (abs_coeff < CTM_COEFF_1_0) + coeffs[i] |= I9XX_CSC_COEFF_FP(abs_coeff, 9); + else if (abs_coeff < CTM_COEFF_2_0) + coeffs[i] |= (7 << 12) | + I9XX_CSC_COEFF_FP(abs_coeff, 8); + else + coeffs[i] |= (6 << 12) | + I9XX_CSC_COEFF_FP(abs_coeff, 7); + } + } else { + /* + * Load an identify matrix if no coefficients are provided. + * + * TODO: Check what kind of values actually come out of the + * pipe with these coeff/postoff values and adjust to get the + * best accuracy. Perhaps we even need to take the bpc value + * into consideration. + */ + for (i = 0; i < 3; i++) { + if (intel_crtc->config->limited_color_range) + coeffs[i * 3 + i] = + I9XX_CSC_COEFF_LIMITED_RANGE; + else + coeffs[i * 3 + i] = I9XX_CSC_COEFF_1_0; + } + } /* * GY/GU and RY/RU should be the other way around according * to BSpec, but reality doesn't agree. Just set them up in * a way that results in the correct picture. */ - I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeff << 16); - I915_WRITE(PIPE_CSC_COEFF_BY(pipe), 0); + I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeffs[0] << 16 | coeffs[1]); + I915_WRITE(PIPE_CSC_COEFF_BY(pipe), coeffs[2] << 16); - I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeff); - I915_WRITE(PIPE_CSC_COEFF_BU(pipe), 0); + I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeffs[3] << 16 | coeffs[4]); + I915_WRITE(PIPE_CSC_COEFF_BU(pipe), coeffs[5] << 16); - I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), 0); - I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeff << 16); + I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), coeffs[6] << 16 | coeffs[7]); + I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeffs[8] << 16); I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0); I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0); @@ -88,21 +213,146 @@ static void i9xx_load_csc_matrix(struct drm_crtc *crtc) } } -void intel_color_set_csc(struct drm_crtc *crtc) +/** Loads the legacy palette/gamma unit for the CRTC with the prepared + * values. + */ +static void i9xx_load_legacy_gamma_lut(struct drm_crtc *crtc) { - i9xx_load_csc_matrix(crtc); + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_crtc_state *intel_state = to_intel_crtc_state(crtc->state); + enum pipe pipe = to_intel_crtc(crtc)->pipe; + int i; + + for (i = 0; i < 256; i++) { + uint32_t word = (intel_crtc->lut_r[i] << 16) | + (intel_crtc->lut_g[i] << 8) | + intel_crtc->lut_b[i]; + if (HAS_GMCH_DISPLAY(dev)) + I915_WRITE(PALETTE(pipe, i), word); + else + I915_WRITE(LGC_PALETTE(pipe, i), word); + } + + intel_state->gamma_mode = GAMMA_MODE_MODE_8BIT; + I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT); } -/** Loads the palette/gamma unit for the CRTC with the prepared values on */ -static void i9xx_load_legacy_gamma_lut(struct drm_crtc *crtc) +static void broadwell_load_degamma_lut(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_crtc_state *state = crtc->state; + struct intel_crtc_state *intel_state = to_intel_crtc_state(state); + enum pipe pipe = to_intel_crtc(crtc)->pipe; + uint32_t i, lut_size = INTEL_INFO(dev)->color.degamma_lut_size; + + I915_WRITE(PREC_PAL_INDEX(pipe), + PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT); + + if (state->degamma_lut) { + struct drm_color_lut *lut = + (struct drm_color_lut *) state->degamma_lut->data; + + for (i = 0; i < lut_size; i++) { + uint32_t word = + drm_color_lut_extract(lut[i].red, 10) << 20 | + drm_color_lut_extract(lut[i].green, 10) << 10 | + drm_color_lut_extract(lut[i].blue, 10); + + I915_WRITE(PREC_PAL_DATA(pipe), word); + } + } else { + for (i = 0; i < lut_size; i++) { + uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1); + + I915_WRITE(PREC_PAL_DATA(pipe), + (v << 20) | (v << 10) | v); + } + } + + intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT; + I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT); + POSTING_READ(GAMMA_MODE(pipe)); + + /* Reset the index, otherwise it prevents the legacy palette to be + * written properly. + */ + I915_WRITE(PREC_PAL_INDEX(pipe), 0); +} + +static void broadwell_load_gamma_lut(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + struct drm_crtc_state *state = crtc->state; + struct intel_crtc_state *intel_state = to_intel_crtc_state(state); + enum pipe pipe = to_intel_crtc(crtc)->pipe; + uint32_t i, lut_offset = INTEL_INFO(dev)->color.degamma_lut_size, + lut_size = INTEL_INFO(dev)->color.gamma_lut_size; + + + I915_WRITE(PREC_PAL_INDEX(pipe), + PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT | lut_offset); + + if (state->gamma_lut) { + struct drm_color_lut *lut = + (struct drm_color_lut *) state->gamma_lut->data; + + for (i = 0; i < lut_size; i++) { + uint32_t word = + (drm_color_lut_extract(lut[i].red, 10) << 20) | + (drm_color_lut_extract(lut[i].green, 10) << 10) | + drm_color_lut_extract(lut[i].blue, 10); + + I915_WRITE(PREC_PAL_DATA(pipe), word); + } + + /* Program the max register to clamp values > 1.0. */ + I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), + drm_color_lut_extract(lut[i].red, 16)); + I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), + drm_color_lut_extract(lut[i].green, 16)); + I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), + drm_color_lut_extract(lut[i].blue, 16)); + } else { + for (i = 0; i < lut_size; i++) { + uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1); + + I915_WRITE(PREC_PAL_DATA(pipe), + (v << 20) | (v << 10) | v); + } + + I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16) - 1); + I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16) - 1); + I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16) - 1); + } + + intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT; + I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT); + POSTING_READ(GAMMA_MODE(pipe)); + + /* Reset the index, otherwise it prevents the legacy palette to be + * written properly. + */ + I915_WRITE(PREC_PAL_INDEX(pipe), 0); +} + +static void intel_color_load_luts_internal(struct drm_crtc *crtc, + bool legacy) { struct drm_device *dev = crtc->dev; struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - enum pipe pipe = intel_crtc->pipe; - int i; + struct intel_crtc_state *intel_state = to_intel_crtc_state(crtc->state); + enum pipe pipe = to_intel_crtc(crtc)->pipe; bool reenable_ips = false; + /* The clocks have to be on to load the palette. */ + if (!crtc->state->active) + return; + if (HAS_GMCH_DISPLAY(dev)) { if (intel_crtc->config->has_dsi_encoder) assert_dsi_pll_enabled(dev_priv); @@ -114,42 +364,32 @@ static void i9xx_load_legacy_gamma_lut(struct drm_crtc *crtc) * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled. */ if (IS_HASWELL(dev) && intel_crtc->config->ips_enabled && - ((I915_READ(GAMMA_MODE(pipe)) & GAMMA_MODE_MODE_MASK) == - GAMMA_MODE_MODE_SPLIT)) { + intel_state->gamma_mode == GAMMA_MODE_MODE_SPLIT) { hsw_disable_ips(intel_crtc); reenable_ips = true; } - for (i = 0; i < 256; i++) { - uint32_t word = (intel_crtc->lut_r[i] << 16) | - (intel_crtc->lut_g[i] << 8) | - intel_crtc->lut_b[i]; - if (HAS_GMCH_DISPLAY(dev)) - I915_WRITE(PALETTE(pipe, i), word); - else - I915_WRITE(LGC_PALETTE(pipe, i), word); + if (legacy) + i9xx_load_legacy_gamma_lut(crtc); + else { + dev_priv->display.load_degamma_lut(crtc); + dev_priv->display.load_gamma_lut(crtc); } - I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT); - if (reenable_ips) hsw_enable_ips(intel_crtc); } -void intel_color_load_gamma_lut(struct drm_crtc *crtc) +void intel_color_legacy_load_lut(struct drm_crtc *crtc) { - /* The clocks have to be on to load the palette. */ - if (!crtc->state->active) - return; - - i9xx_load_legacy_gamma_lut(crtc); + intel_color_load_luts_internal(crtc, true); } void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue, uint32_t start, uint32_t size) { - int end = (start + size > 256) ? 256 : start + size, i; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + int end = (start + size > 256) ? 256 : start + size, i; for (i = start; i < end; i++) { intel_crtc->lut_r[i] = red[i] >> 8; @@ -157,11 +397,29 @@ void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, intel_crtc->lut_b[i] = blue[i] >> 8; } - intel_color_load_gamma_lut(crtc); + intel_color_load_luts_internal(crtc, true); +} + +void intel_color_load_luts(struct drm_crtc *crtc) +{ + intel_color_load_luts_internal(crtc, + !crtc->state->degamma_lut && + !crtc->state->gamma_lut); +} + +void intel_color_set_csc(struct drm_crtc *crtc) +{ + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; + + if (dev_priv->display.load_csc_matrix) + dev_priv->display.load_csc_matrix(crtc); } void intel_color_init(struct drm_crtc *crtc) { + struct drm_device *dev = crtc->dev; + struct drm_i915_private *dev_priv = dev->dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int i; @@ -171,4 +429,23 @@ void intel_color_init(struct drm_crtc *crtc) intel_crtc->lut_g[i] = i; intel_crtc->lut_b[i] = i; } + + if (IS_BROADWELL(dev) || IS_SKYLAKE(dev) || + IS_BROXTON(dev) || IS_KABYLAKE(dev)) { + dev_priv->display.load_degamma_lut = broadwell_load_degamma_lut; + dev_priv->display.load_gamma_lut = broadwell_load_gamma_lut; + dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix; + } else { + dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix; + } + + if (INTEL_INFO(dev)->color.degamma_lut_size != 0 && + INTEL_INFO(dev)->color.gamma_lut_size != 0) { + WARN_ON(!dev_priv->display.load_degamma_lut || + !dev_priv->display.load_gamma_lut || + !dev_priv->display.load_csc_matrix); + drm_helper_crtc_enable_color_mgmt(crtc, + INTEL_INFO(dev)->color.degamma_lut_size, + INTEL_INFO(dev)->color.gamma_lut_size); + } } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index f10bba2..e38b31b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4864,7 +4864,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc) * On ILK+ LUT must be loaded before the pipe is running but with * clocks enabled */ - intel_color_load_gamma_lut(crtc); + intel_color_load_luts(crtc); intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); @@ -4959,7 +4959,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc) * On ILK+ LUT must be loaded before the pipe is running but with * clocks enabled */ - intel_color_load_gamma_lut(crtc); + intel_color_load_luts(crtc); intel_ddi_set_pipe_settings(crtc); if (!intel_crtc->config->has_dsi_encoder) @@ -6174,7 +6174,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc) i9xx_pfit_enable(intel_crtc); - intel_color_load_gamma_lut(crtc); + intel_color_load_luts(crtc); intel_enable_pipe(intel_crtc); @@ -6227,7 +6227,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc) i9xx_pfit_enable(intel_crtc); - intel_color_load_gamma_lut(crtc); + intel_color_load_luts(crtc); intel_update_watermarks(crtc); intel_enable_pipe(intel_crtc); @@ -11898,7 +11898,7 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc, static const struct drm_crtc_helper_funcs intel_helper_funcs = { .mode_set_base_atomic = intel_pipe_set_base_atomic, - .load_lut = intel_color_load_gamma_lut, + .load_lut = intel_color_legacy_load_lut, .atomic_begin = intel_begin_crtc_commit, .atomic_flush = intel_finish_crtc_commit, .atomic_check = intel_crtc_atomic_check, @@ -13428,6 +13428,17 @@ static int intel_atomic_commit(struct drm_device *dev, hw_check = true; } + if (!modeset && + crtc->state->active && + crtc->state->color_mgmt_changed) { + /* Only update color management when not doing + * a modeset as this will be done by + * crtc_enable already. + */ + intel_color_set_csc(crtc); + intel_color_load_luts(crtc); + } + if (!modeset) intel_pre_plane_update(to_intel_crtc_state(crtc_state)); @@ -13519,6 +13530,7 @@ out: static const struct drm_crtc_funcs intel_crtc_funcs = { .gamma_set = intel_color_legacy_gamma_set, .set_config = drm_atomic_helper_set_config, + .set_property = drm_atomic_helper_crtc_set_property, .destroy = intel_crtc_destroy, .page_flip = intel_crtc_page_flip, .atomic_duplicate_state = intel_crtc_duplicate_state, diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 2b5d03a..c384c78 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -507,6 +507,8 @@ struct intel_crtc_state { /* IVB sprite scaling w/a (WaCxSRDisabledForSpriteScaling:ivb) */ bool disable_lp_wm; + uint32_t gamma_mode; + struct { /* * optimal watermarks, programmed post-vblank when this state @@ -1620,9 +1622,11 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs; /* intel_color.c */ void intel_color_init(struct drm_crtc *crtc); +void intel_color_update(struct drm_crtc *crtc); void intel_color_set_csc(struct drm_crtc *crtc); -void intel_color_load_gamma_lut(struct drm_crtc *crtc); +void intel_color_load_luts(struct drm_crtc *crtc); void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green, u16 *blue, uint32_t start, uint32_t size); +void intel_color_legacy_load_lut(struct drm_crtc *crtc); #endif /* __INTEL_DRV_H__ */
Patch based on a previous series by Shashank Sharma. v2: Do not read GAMMA_MODE register to figure what mode we're in v3: Program PREC_PAL_GC_MAX to clamp pixel values > 1.0 Add documentation on how the Broadcast RGB property is affected by CTM_MATRIX v4: Update contributors Signed-off-by: Shashank Sharma <shashank.sharma@intel.com> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com> Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com> --- Documentation/DocBook/gpu.tmpl | 6 +- drivers/gpu/drm/i915/i915_drv.c | 24 ++- drivers/gpu/drm/i915/i915_drv.h | 9 + drivers/gpu/drm/i915/i915_reg.h | 22 +++ drivers/gpu/drm/i915/intel_color.c | 367 ++++++++++++++++++++++++++++++----- drivers/gpu/drm/i915/intel_display.c | 22 ++- drivers/gpu/drm/i915/intel_drv.h | 6 +- 7 files changed, 396 insertions(+), 60 deletions(-)