Re: [Intel-gfx] [PATCH 5/6] drm/i915: Implement color management on bdw/skl/bxt/kbl

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