[PATCH 00/12] drm/i915: Gamma cleanups

[PATCH 00/12] drm/i915: Gamma cleanups

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Some polishings and at least one minor fix for the gamma stuff.

Ville Syrjälä (12):
  drm: Inline drm_color_lut_extract()
  drm/i915: Polish CHV .load_luts() a bit
  drm/i915: Polish CHV CGM CSC loading
  drm/i915: Add i9xx_lut_8()
  drm/i915: Clean up i9xx_load_luts_internal()
  drm/i915: Split i9xx_read_lut_8() to gmch vs. ilk variants
  drm/i915: s/blob_data/lut/
  drm/i915: s/chv_read_cgm_lut/chv_read_cgm_gamma/
  drm/i915: Clean up integer types in color code
  drm/i915: Refactor LUT read functions
  drm/i915: Fix readout of PIPEGCMAX
  drm/i915: Pass the crtc to the low level read_lut() funcs

 drivers/gpu/drm/drm_color_mgmt.c           |  24 --
 drivers/gpu/drm/i915/display/intel_color.c | 435 +++++++++++----------
 drivers/gpu/drm/i915/i915_reg.h            |   1 -
 include/drm/drm_color_mgmt.h               |  23 +-
 4 files changed, 258 insertions(+), 225 deletions(-)

-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 00/12] drm/i915: Gamma cleanups

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Some polishings and at least one minor fix for the gamma stuff.

Ville Syrjälä (12):
  drm: Inline drm_color_lut_extract()
  drm/i915: Polish CHV .load_luts() a bit
  drm/i915: Polish CHV CGM CSC loading
  drm/i915: Add i9xx_lut_8()
  drm/i915: Clean up i9xx_load_luts_internal()
  drm/i915: Split i9xx_read_lut_8() to gmch vs. ilk variants
  drm/i915: s/blob_data/lut/
  drm/i915: s/chv_read_cgm_lut/chv_read_cgm_gamma/
  drm/i915: Clean up integer types in color code
  drm/i915: Refactor LUT read functions
  drm/i915: Fix readout of PIPEGCMAX
  drm/i915: Pass the crtc to the low level read_lut() funcs

 drivers/gpu/drm/drm_color_mgmt.c           |  24 --
 drivers/gpu/drm/i915/display/intel_color.c | 435 +++++++++++----------
 drivers/gpu/drm/i915/i915_reg.h            |   1 -
 include/drm/drm_color_mgmt.h               |  23 +-
 4 files changed, 258 insertions(+), 225 deletions(-)

-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 01/12] drm: Inline drm_color_lut_extract()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

This thing can get called several thousand times per LUT
so seems like we want to inline it to:
- avoid the function call overhead
- allow constant folding

A quick synthetic test (w/o any hardware interaction) with
a ridiculously large LUT size shows about 50% reduction in
runtime on my HSW and BSW boxes. Slightly less with more
reasonable LUT size but still easily measurable in tens
of microseconds.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_color_mgmt.c | 24 ------------------------
 include/drm/drm_color_mgmt.h     | 23 ++++++++++++++++++++++-
 2 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 4ce5c6d8de99..19c5f635992a 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -108,30 +108,6 @@
  * 	standard enum values supported by the DRM plane.
  */
 
-/**
- * drm_color_lut_extract - clamp and round LUT entries
- * @user_input: input value
- * @bit_precision: number of bits the hw LUT supports
- *
- * Extract a degamma/gamma LUT value provided by user (in the form of
- * &drm_color_lut entries) and round it to the precision supported by the
- * hardware.
- */
-uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
-{
-	uint32_t val = user_input;
-	uint32_t max = 0xffff >> (16 - bit_precision);
-
-	/* Round only if we're not using full precision. */
-	if (bit_precision < 16) {
-		val += 1UL << (16 - bit_precision - 1);
-		val >>= 16 - bit_precision;
-	}
-
-	return clamp_val(val, 0, max);
-}
-EXPORT_SYMBOL(drm_color_lut_extract);
-
 /**
  * drm_crtc_enable_color_mgmt - enable color management properties
  * @crtc: DRM CRTC
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index d1c662d92ab7..069b21d61871 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -29,7 +29,28 @@
 struct drm_crtc;
 struct drm_plane;
 
-uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
+/**
+ * drm_color_lut_extract - clamp and round LUT entries
+ * @user_input: input value
+ * @bit_precision: number of bits the hw LUT supports
+ *
+ * Extract a degamma/gamma LUT value provided by user (in the form of
+ * &drm_color_lut entries) and round it to the precision supported by the
+ * hardware.
+ */
+static inline u32 drm_color_lut_extract(u32 user_input, int bit_precision)
+{
+	u32 val = user_input;
+	u32 max = 0xffff >> (16 - bit_precision);
+
+	/* Round only if we're not using full precision. */
+	if (bit_precision < 16) {
+		val += 1UL << (16 - bit_precision - 1);
+		val >>= 16 - bit_precision;
+	}
+
+	return clamp_val(val, 0, max);
+}
 
 void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 				uint degamma_lut_size,
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 01/12] drm: Inline drm_color_lut_extract()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

This thing can get called several thousand times per LUT
so seems like we want to inline it to:
- avoid the function call overhead
- allow constant folding

A quick synthetic test (w/o any hardware interaction) with
a ridiculously large LUT size shows about 50% reduction in
runtime on my HSW and BSW boxes. Slightly less with more
reasonable LUT size but still easily measurable in tens
of microseconds.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_color_mgmt.c | 24 ------------------------
 include/drm/drm_color_mgmt.h     | 23 ++++++++++++++++++++++-
 2 files changed, 22 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 4ce5c6d8de99..19c5f635992a 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -108,30 +108,6 @@
  * 	standard enum values supported by the DRM plane.
  */
 
-/**
- * drm_color_lut_extract - clamp and round LUT entries
- * @user_input: input value
- * @bit_precision: number of bits the hw LUT supports
- *
- * Extract a degamma/gamma LUT value provided by user (in the form of
- * &drm_color_lut entries) and round it to the precision supported by the
- * hardware.
- */
-uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
-{
-	uint32_t val = user_input;
-	uint32_t max = 0xffff >> (16 - bit_precision);
-
-	/* Round only if we're not using full precision. */
-	if (bit_precision < 16) {
-		val += 1UL << (16 - bit_precision - 1);
-		val >>= 16 - bit_precision;
-	}
-
-	return clamp_val(val, 0, max);
-}
-EXPORT_SYMBOL(drm_color_lut_extract);
-
 /**
  * drm_crtc_enable_color_mgmt - enable color management properties
  * @crtc: DRM CRTC
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index d1c662d92ab7..069b21d61871 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -29,7 +29,28 @@
 struct drm_crtc;
 struct drm_plane;
 
-uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
+/**
+ * drm_color_lut_extract - clamp and round LUT entries
+ * @user_input: input value
+ * @bit_precision: number of bits the hw LUT supports
+ *
+ * Extract a degamma/gamma LUT value provided by user (in the form of
+ * &drm_color_lut entries) and round it to the precision supported by the
+ * hardware.
+ */
+static inline u32 drm_color_lut_extract(u32 user_input, int bit_precision)
+{
+	u32 val = user_input;
+	u32 max = 0xffff >> (16 - bit_precision);
+
+	/* Round only if we're not using full precision. */
+	if (bit_precision < 16) {
+		val += 1UL << (16 - bit_precision - 1);
+		val >>= 16 - bit_precision;
+	}
+
+	return clamp_val(val, 0, max);
+}
 
 void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 				uint degamma_lut_size,
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 02/12] drm/i915: Polish CHV .load_luts() a bit

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

It irks me to use crtc_state_is_legacy_gamma() inside the guts
of the CHV color management code. Let's get rid of it and instead
just consult cgm_mode to figure out if we want to enable the pipe
gamma or the CGM gamma.

Also CHV display engine is based on i965/g4x so we should fall back
to the i965 path when the CGM gamma is not used.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 3980e8b50c28..d8ee90b7774a 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -996,16 +996,13 @@ static void chv_load_luts(const struct intel_crtc_state *crtc_state)
 
 	cherryview_load_csc_matrix(crtc_state);
 
-	if (crtc_state_is_legacy_gamma(crtc_state)) {
-		i9xx_load_luts(crtc_state);
-		return;
-	}
-
-	if (degamma_lut)
+	if (crtc_state->cgm_mode & CGM_PIPE_MODE_DEGAMMA)
 		chv_load_cgm_degamma(crtc, degamma_lut);
 
-	if (gamma_lut)
+	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
 		chv_load_cgm_gamma(crtc, gamma_lut);
+	else
+		i965_load_luts(crtc_state);
 }
 
 void intel_color_load_luts(const struct intel_crtc_state *crtc_state)
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 02/12] drm/i915: Polish CHV .load_luts() a bit

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

It irks me to use crtc_state_is_legacy_gamma() inside the guts
of the CHV color management code. Let's get rid of it and instead
just consult cgm_mode to figure out if we want to enable the pipe
gamma or the CGM gamma.

Also CHV display engine is based on i965/g4x so we should fall back
to the i965 path when the CGM gamma is not used.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 3980e8b50c28..d8ee90b7774a 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -996,16 +996,13 @@ static void chv_load_luts(const struct intel_crtc_state *crtc_state)
 
 	cherryview_load_csc_matrix(crtc_state);
 
-	if (crtc_state_is_legacy_gamma(crtc_state)) {
-		i9xx_load_luts(crtc_state);
-		return;
-	}
-
-	if (degamma_lut)
+	if (crtc_state->cgm_mode & CGM_PIPE_MODE_DEGAMMA)
 		chv_load_cgm_degamma(crtc, degamma_lut);
 
-	if (gamma_lut)
+	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
 		chv_load_cgm_gamma(crtc, gamma_lut);
+	else
+		i965_load_luts(crtc_state);
 }
 
 void intel_color_load_luts(const struct intel_crtc_state *crtc_state)
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/12] drm/i915: Polish CHV CGM CSC loading

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Only load the CGM CSC based on the cgm_mode bit like we
do with the gamma/degamma LUTs. And make the function
naming and arguments consistent as well.

TODO: the code to convert the coefficients look totally
bogus. IIRC CHV uses two's complement format but the code
certainly doesn't generate that, so probably negative
coefficients are totally busted.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 69 ++++++++++------------
 1 file changed, 32 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index d8ee90b7774a..f20809d91f85 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -333,48 +333,38 @@ static void icl_load_csc_matrix(const struct intel_crtc_state *crtc_state)
 	I915_WRITE(PIPE_CSC_MODE(crtc->pipe), crtc_state->csc_mode);
 }
 
-/*
- * Set up the pipe CSC unit on CherryView.
- */
-static void cherryview_load_csc_matrix(const struct intel_crtc_state *crtc_state)
+static void chv_load_cgm_csc(struct intel_crtc *crtc,
+			     const struct drm_property_blob *blob)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	const struct drm_color_ctm *ctm = blob->data;
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
+	u16 coeffs[9];
+	int i;
 
-	if (crtc_state->hw.ctm) {
-		const struct drm_color_ctm *ctm = crtc_state->hw.ctm->data;
-		u16 coeffs[9] = {};
-		int i;
-
-		for (i = 0; i < ARRAY_SIZE(coeffs); i++) {
-			u64 abs_coeff =
-				((1ULL << 63) - 1) & ctm->matrix[i];
-
-			/* Round coefficient. */
-			abs_coeff += 1 << (32 - 13);
-			/* Clamp to hardware limits. */
-			abs_coeff = clamp_val(abs_coeff, 0, CTM_COEFF_8_0 - 1);
-
-			/* Write coefficients in S3.12 format. */
-			if (ctm->matrix[i] & (1ULL << 63))
-				coeffs[i] = 1 << 15;
-			coeffs[i] |= ((abs_coeff >> 32) & 7) << 12;
-			coeffs[i] |= (abs_coeff >> 20) & 0xfff;
-		}
+	for (i = 0; i < ARRAY_SIZE(coeffs); i++) {
+		u64 abs_coeff = ((1ULL << 63) - 1) & ctm->matrix[i];
 
-		I915_WRITE(CGM_PIPE_CSC_COEFF01(pipe),
-			   coeffs[1] << 16 | coeffs[0]);
-		I915_WRITE(CGM_PIPE_CSC_COEFF23(pipe),
-			   coeffs[3] << 16 | coeffs[2]);
-		I915_WRITE(CGM_PIPE_CSC_COEFF45(pipe),
-			   coeffs[5] << 16 | coeffs[4]);
-		I915_WRITE(CGM_PIPE_CSC_COEFF67(pipe),
-			   coeffs[7] << 16 | coeffs[6]);
-		I915_WRITE(CGM_PIPE_CSC_COEFF8(pipe), coeffs[8]);
+		/* Round coefficient. */
+		abs_coeff += 1 << (32 - 13);
+		/* Clamp to hardware limits. */
+		abs_coeff = clamp_val(abs_coeff, 0, CTM_COEFF_8_0 - 1);
+
+		coeffs[i] = 0;
+
+		/* Write coefficients in S3.12 format. */
+		if (ctm->matrix[i] & (1ULL << 63))
+			coeffs[i] |= 1 << 15;
+
+		coeffs[i] |= ((abs_coeff >> 32) & 7) << 12;
+		coeffs[i] |= (abs_coeff >> 20) & 0xfff;
 	}
 
-	I915_WRITE(CGM_PIPE_MODE(pipe), crtc_state->cgm_mode);
+	I915_WRITE(CGM_PIPE_CSC_COEFF01(pipe), coeffs[1] << 16 | coeffs[0]);
+	I915_WRITE(CGM_PIPE_CSC_COEFF23(pipe), coeffs[3] << 16 | coeffs[2]);
+	I915_WRITE(CGM_PIPE_CSC_COEFF45(pipe), coeffs[5] << 16 | coeffs[4]);
+	I915_WRITE(CGM_PIPE_CSC_COEFF67(pipe), coeffs[7] << 16 | coeffs[6]);
+	I915_WRITE(CGM_PIPE_CSC_COEFF8(pipe), coeffs[8]);
 }
 
 /* i965+ "10.6" bit interpolated format "even DW" (low 8 bits) */
@@ -991,10 +981,13 @@ static void chv_load_cgm_gamma(struct intel_crtc *crtc,
 static void chv_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
+	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
+	const struct drm_property_blob *ctm = crtc_state->hw.ctm;
 
-	cherryview_load_csc_matrix(crtc_state);
+	if (crtc_state->cgm_mode & CGM_PIPE_MODE_CSC)
+		chv_load_cgm_csc(crtc, ctm);
 
 	if (crtc_state->cgm_mode & CGM_PIPE_MODE_DEGAMMA)
 		chv_load_cgm_degamma(crtc, degamma_lut);
@@ -1003,6 +996,8 @@ static void chv_load_luts(const struct intel_crtc_state *crtc_state)
 		chv_load_cgm_gamma(crtc, gamma_lut);
 	else
 		i965_load_luts(crtc_state);
+
+	I915_WRITE(CGM_PIPE_MODE(crtc->pipe), crtc_state->cgm_mode);
 }
 
 void intel_color_load_luts(const struct intel_crtc_state *crtc_state)
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 03/12] drm/i915: Polish CHV CGM CSC loading

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Only load the CGM CSC based on the cgm_mode bit like we
do with the gamma/degamma LUTs. And make the function
naming and arguments consistent as well.

TODO: the code to convert the coefficients look totally
bogus. IIRC CHV uses two's complement format but the code
certainly doesn't generate that, so probably negative
coefficients are totally busted.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 69 ++++++++++------------
 1 file changed, 32 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index d8ee90b7774a..f20809d91f85 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -333,48 +333,38 @@ static void icl_load_csc_matrix(const struct intel_crtc_state *crtc_state)
 	I915_WRITE(PIPE_CSC_MODE(crtc->pipe), crtc_state->csc_mode);
 }
 
-/*
- * Set up the pipe CSC unit on CherryView.
- */
-static void cherryview_load_csc_matrix(const struct intel_crtc_state *crtc_state)
+static void chv_load_cgm_csc(struct intel_crtc *crtc,
+			     const struct drm_property_blob *blob)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	const struct drm_color_ctm *ctm = blob->data;
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
+	u16 coeffs[9];
+	int i;
 
-	if (crtc_state->hw.ctm) {
-		const struct drm_color_ctm *ctm = crtc_state->hw.ctm->data;
-		u16 coeffs[9] = {};
-		int i;
-
-		for (i = 0; i < ARRAY_SIZE(coeffs); i++) {
-			u64 abs_coeff =
-				((1ULL << 63) - 1) & ctm->matrix[i];
-
-			/* Round coefficient. */
-			abs_coeff += 1 << (32 - 13);
-			/* Clamp to hardware limits. */
-			abs_coeff = clamp_val(abs_coeff, 0, CTM_COEFF_8_0 - 1);
-
-			/* Write coefficients in S3.12 format. */
-			if (ctm->matrix[i] & (1ULL << 63))
-				coeffs[i] = 1 << 15;
-			coeffs[i] |= ((abs_coeff >> 32) & 7) << 12;
-			coeffs[i] |= (abs_coeff >> 20) & 0xfff;
-		}
+	for (i = 0; i < ARRAY_SIZE(coeffs); i++) {
+		u64 abs_coeff = ((1ULL << 63) - 1) & ctm->matrix[i];
 
-		I915_WRITE(CGM_PIPE_CSC_COEFF01(pipe),
-			   coeffs[1] << 16 | coeffs[0]);
-		I915_WRITE(CGM_PIPE_CSC_COEFF23(pipe),
-			   coeffs[3] << 16 | coeffs[2]);
-		I915_WRITE(CGM_PIPE_CSC_COEFF45(pipe),
-			   coeffs[5] << 16 | coeffs[4]);
-		I915_WRITE(CGM_PIPE_CSC_COEFF67(pipe),
-			   coeffs[7] << 16 | coeffs[6]);
-		I915_WRITE(CGM_PIPE_CSC_COEFF8(pipe), coeffs[8]);
+		/* Round coefficient. */
+		abs_coeff += 1 << (32 - 13);
+		/* Clamp to hardware limits. */
+		abs_coeff = clamp_val(abs_coeff, 0, CTM_COEFF_8_0 - 1);
+
+		coeffs[i] = 0;
+
+		/* Write coefficients in S3.12 format. */
+		if (ctm->matrix[i] & (1ULL << 63))
+			coeffs[i] |= 1 << 15;
+
+		coeffs[i] |= ((abs_coeff >> 32) & 7) << 12;
+		coeffs[i] |= (abs_coeff >> 20) & 0xfff;
 	}
 
-	I915_WRITE(CGM_PIPE_MODE(pipe), crtc_state->cgm_mode);
+	I915_WRITE(CGM_PIPE_CSC_COEFF01(pipe), coeffs[1] << 16 | coeffs[0]);
+	I915_WRITE(CGM_PIPE_CSC_COEFF23(pipe), coeffs[3] << 16 | coeffs[2]);
+	I915_WRITE(CGM_PIPE_CSC_COEFF45(pipe), coeffs[5] << 16 | coeffs[4]);
+	I915_WRITE(CGM_PIPE_CSC_COEFF67(pipe), coeffs[7] << 16 | coeffs[6]);
+	I915_WRITE(CGM_PIPE_CSC_COEFF8(pipe), coeffs[8]);
 }
 
 /* i965+ "10.6" bit interpolated format "even DW" (low 8 bits) */
@@ -991,10 +981,13 @@ static void chv_load_cgm_gamma(struct intel_crtc *crtc,
 static void chv_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
+	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
+	const struct drm_property_blob *ctm = crtc_state->hw.ctm;
 
-	cherryview_load_csc_matrix(crtc_state);
+	if (crtc_state->cgm_mode & CGM_PIPE_MODE_CSC)
+		chv_load_cgm_csc(crtc, ctm);
 
 	if (crtc_state->cgm_mode & CGM_PIPE_MODE_DEGAMMA)
 		chv_load_cgm_degamma(crtc, degamma_lut);
@@ -1003,6 +996,8 @@ static void chv_load_luts(const struct intel_crtc_state *crtc_state)
 		chv_load_cgm_gamma(crtc, gamma_lut);
 	else
 		i965_load_luts(crtc_state);
+
+	I915_WRITE(CGM_PIPE_MODE(crtc->pipe), crtc_state->cgm_mode);
 }
 
 void intel_color_load_luts(const struct intel_crtc_state *crtc_state)
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 04/12] drm/i915: Add i9xx_lut_8()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We have a nice little helper to compute a single LUT entry
for everything except the 8bpc legacy gamma mode. Let's
complete the set.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index f20809d91f85..5443b8ec0a4c 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -367,6 +367,13 @@ static void chv_load_cgm_csc(struct intel_crtc *crtc,
 	I915_WRITE(CGM_PIPE_CSC_COEFF8(pipe), coeffs[8]);
 }
 
+static u32 i9xx_lut_8(const struct drm_color_lut *color)
+{
+	return drm_color_lut_extract(color->red, 8) << 16 |
+		drm_color_lut_extract(color->green, 8) << 8 |
+		drm_color_lut_extract(color->blue, 8);
+}
+
 /* i965+ "10.6" bit interpolated format "even DW" (low 8 bits) */
 static u32 i965_lut_10p6_ldw(const struct drm_color_lut *color)
 {
@@ -410,10 +417,7 @@ static void i9xx_load_luts_internal(const struct intel_crtc_state *crtc_state,
 		const struct drm_color_lut *lut = blob->data;
 
 		for (i = 0; i < 256; i++) {
-			u32 word =
-				(drm_color_lut_extract(lut[i].red, 8) << 16) |
-				(drm_color_lut_extract(lut[i].green, 8) << 8) |
-				drm_color_lut_extract(lut[i].blue, 8);
+			u32 word = i9xx_lut_8(&lut[i]);
 
 			if (HAS_GMCH(dev_priv))
 				I915_WRITE(PALETTE(pipe, i), word);
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 05/12] drm/i915: Clean up i9xx_load_luts_internal()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Split i9xx_load_luts_internal() into neat gmch vs. ilk+ chunks.
Avoids at least one branch in the inner loop, and makes life
a bit less confusing.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 98 +++++++++++++---------
 1 file changed, 57 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 5443b8ec0a4c..992290a07086 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -397,41 +397,6 @@ static u32 ilk_lut_10(const struct drm_color_lut *color)
 		drm_color_lut_extract(color->blue, 10);
 }
 
-/* Loads the legacy palette/gamma unit for the CRTC. */
-static void i9xx_load_luts_internal(const struct intel_crtc_state *crtc_state,
-				    const struct drm_property_blob *blob)
-{
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	enum pipe pipe = crtc->pipe;
-	int i;
-
-	if (HAS_GMCH(dev_priv)) {
-		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
-			assert_dsi_pll_enabled(dev_priv);
-		else
-			assert_pll_enabled(dev_priv, pipe);
-	}
-
-	if (blob) {
-		const struct drm_color_lut *lut = blob->data;
-
-		for (i = 0; i < 256; i++) {
-			u32 word = i9xx_lut_8(&lut[i]);
-
-			if (HAS_GMCH(dev_priv))
-				I915_WRITE(PALETTE(pipe, i), word);
-			else
-				I915_WRITE(LGC_PALETTE(pipe, i), word);
-		}
-	}
-}
-
-static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
-{
-	i9xx_load_luts_internal(crtc_state, crtc_state->hw.gamma_lut);
-}
-
 static void i9xx_color_commit(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
@@ -496,6 +461,34 @@ static void skl_color_commit(const struct intel_crtc_state *crtc_state)
 		ilk_load_csc_matrix(crtc_state);
 }
 
+static void i9xx_load_lut_8(struct intel_crtc *crtc,
+			    const struct drm_property_blob *blob)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct drm_color_lut *lut;
+	enum pipe pipe = crtc->pipe;
+	int i;
+
+	if (!blob)
+		return;
+
+	lut = blob->data;
+
+	for (i = 0; i < 256; i++)
+		I915_WRITE(PALETTE(pipe, i), i9xx_lut_8(&lut[i]));
+}
+
+static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
+
+	assert_pll_enabled(dev_priv, crtc->pipe);
+
+	i9xx_load_lut_8(crtc, gamma_lut);
+}
+
 static void i965_load_lut_10p6(struct intel_crtc *crtc,
 			       const struct drm_property_blob *blob)
 {
@@ -519,14 +512,37 @@ static void i965_load_lut_10p6(struct intel_crtc *crtc,
 static void i965_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
 
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
+		assert_dsi_pll_enabled(dev_priv);
+	else
+		assert_pll_enabled(dev_priv, crtc->pipe);
+
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		i9xx_load_luts(crtc_state);
+		i9xx_load_lut_8(crtc, gamma_lut);
 	else
 		i965_load_lut_10p6(crtc, gamma_lut);
 }
 
+static void ilk_load_lut_8(struct intel_crtc *crtc,
+			   const struct drm_property_blob *blob)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct drm_color_lut *lut;
+	enum pipe pipe = crtc->pipe;
+	int i;
+
+	if (!blob)
+		return;
+
+	lut = blob->data;
+
+	for (i = 0; i < 256; i++)
+		I915_WRITE(LGC_PALETTE(pipe, i), i9xx_lut_8(&lut[i]));
+}
+
 static void ilk_load_lut_10(struct intel_crtc *crtc,
 			    const struct drm_property_blob *blob)
 {
@@ -545,7 +561,7 @@ static void ilk_load_luts(const struct intel_crtc_state *crtc_state)
 	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		i9xx_load_luts(crtc_state);
+		ilk_load_lut_8(crtc, gamma_lut);
 	else
 		ilk_load_lut_10(crtc, gamma_lut);
 }
@@ -653,7 +669,7 @@ static void ivb_load_luts(const struct intel_crtc_state *crtc_state)
 	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
-		i9xx_load_luts(crtc_state);
+		ilk_load_lut_8(crtc, gamma_lut);
 	} else if (crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT) {
 		ivb_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
 				PAL_PREC_INDEX_VALUE(0));
@@ -676,7 +692,7 @@ static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
 	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
-		i9xx_load_luts(crtc_state);
+		ilk_load_lut_8(crtc, gamma_lut);
 	} else if (crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT) {
 		bdw_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
 				PAL_PREC_INDEX_VALUE(0));
@@ -777,7 +793,7 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
 		glk_load_degamma_lut_linear(crtc_state);
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
-		i9xx_load_luts(crtc_state);
+		ilk_load_lut_8(crtc, gamma_lut);
 	} else {
 		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
 		ivb_load_lut_ext_max(crtc);
@@ -913,7 +929,7 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
 
 	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
 	case GAMMA_MODE_MODE_8BIT:
-		i9xx_load_luts(crtc_state);
+		ilk_load_lut_8(crtc, gamma_lut);
 		break;
 	case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
 		icl_program_gamma_superfine_segment(crtc_state);
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 05/12] drm/i915: Clean up i9xx_load_luts_internal()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Split i9xx_load_luts_internal() into neat gmch vs. ilk+ chunks.
Avoids at least one branch in the inner loop, and makes life
a bit less confusing.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 98 +++++++++++++---------
 1 file changed, 57 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 5443b8ec0a4c..992290a07086 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -397,41 +397,6 @@ static u32 ilk_lut_10(const struct drm_color_lut *color)
 		drm_color_lut_extract(color->blue, 10);
 }
 
-/* Loads the legacy palette/gamma unit for the CRTC. */
-static void i9xx_load_luts_internal(const struct intel_crtc_state *crtc_state,
-				    const struct drm_property_blob *blob)
-{
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	enum pipe pipe = crtc->pipe;
-	int i;
-
-	if (HAS_GMCH(dev_priv)) {
-		if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
-			assert_dsi_pll_enabled(dev_priv);
-		else
-			assert_pll_enabled(dev_priv, pipe);
-	}
-
-	if (blob) {
-		const struct drm_color_lut *lut = blob->data;
-
-		for (i = 0; i < 256; i++) {
-			u32 word = i9xx_lut_8(&lut[i]);
-
-			if (HAS_GMCH(dev_priv))
-				I915_WRITE(PALETTE(pipe, i), word);
-			else
-				I915_WRITE(LGC_PALETTE(pipe, i), word);
-		}
-	}
-}
-
-static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
-{
-	i9xx_load_luts_internal(crtc_state, crtc_state->hw.gamma_lut);
-}
-
 static void i9xx_color_commit(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
@@ -496,6 +461,34 @@ static void skl_color_commit(const struct intel_crtc_state *crtc_state)
 		ilk_load_csc_matrix(crtc_state);
 }
 
+static void i9xx_load_lut_8(struct intel_crtc *crtc,
+			    const struct drm_property_blob *blob)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct drm_color_lut *lut;
+	enum pipe pipe = crtc->pipe;
+	int i;
+
+	if (!blob)
+		return;
+
+	lut = blob->data;
+
+	for (i = 0; i < 256; i++)
+		I915_WRITE(PALETTE(pipe, i), i9xx_lut_8(&lut[i]));
+}
+
+static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
+
+	assert_pll_enabled(dev_priv, crtc->pipe);
+
+	i9xx_load_lut_8(crtc, gamma_lut);
+}
+
 static void i965_load_lut_10p6(struct intel_crtc *crtc,
 			       const struct drm_property_blob *blob)
 {
@@ -519,14 +512,37 @@ static void i965_load_lut_10p6(struct intel_crtc *crtc,
 static void i965_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
 
+	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
+		assert_dsi_pll_enabled(dev_priv);
+	else
+		assert_pll_enabled(dev_priv, crtc->pipe);
+
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		i9xx_load_luts(crtc_state);
+		i9xx_load_lut_8(crtc, gamma_lut);
 	else
 		i965_load_lut_10p6(crtc, gamma_lut);
 }
 
+static void ilk_load_lut_8(struct intel_crtc *crtc,
+			   const struct drm_property_blob *blob)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct drm_color_lut *lut;
+	enum pipe pipe = crtc->pipe;
+	int i;
+
+	if (!blob)
+		return;
+
+	lut = blob->data;
+
+	for (i = 0; i < 256; i++)
+		I915_WRITE(LGC_PALETTE(pipe, i), i9xx_lut_8(&lut[i]));
+}
+
 static void ilk_load_lut_10(struct intel_crtc *crtc,
 			    const struct drm_property_blob *blob)
 {
@@ -545,7 +561,7 @@ static void ilk_load_luts(const struct intel_crtc_state *crtc_state)
 	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		i9xx_load_luts(crtc_state);
+		ilk_load_lut_8(crtc, gamma_lut);
 	else
 		ilk_load_lut_10(crtc, gamma_lut);
 }
@@ -653,7 +669,7 @@ static void ivb_load_luts(const struct intel_crtc_state *crtc_state)
 	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
-		i9xx_load_luts(crtc_state);
+		ilk_load_lut_8(crtc, gamma_lut);
 	} else if (crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT) {
 		ivb_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
 				PAL_PREC_INDEX_VALUE(0));
@@ -676,7 +692,7 @@ static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
 	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
-		i9xx_load_luts(crtc_state);
+		ilk_load_lut_8(crtc, gamma_lut);
 	} else if (crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT) {
 		bdw_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
 				PAL_PREC_INDEX_VALUE(0));
@@ -777,7 +793,7 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
 		glk_load_degamma_lut_linear(crtc_state);
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
-		i9xx_load_luts(crtc_state);
+		ilk_load_lut_8(crtc, gamma_lut);
 	} else {
 		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
 		ivb_load_lut_ext_max(crtc);
@@ -913,7 +929,7 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
 
 	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
 	case GAMMA_MODE_MODE_8BIT:
-		i9xx_load_luts(crtc_state);
+		ilk_load_lut_8(crtc, gamma_lut);
 		break;
 	case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
 		icl_program_gamma_superfine_segment(crtc_state);
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 06/12] drm/i915: Split i9xx_read_lut_8() to gmch vs. ilk variants

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

To mirror the load_luts path let's clone an ilk+ version
from i9xx_read_lut_8(). I guess the extra branch isn't a huge
issue but feels better to make a clean split.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 41 ++++++++++++++++++----
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 992290a07086..5890e3896f8d 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1674,10 +1674,7 @@ i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
 	blob_data = blob->data;
 
 	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
-		if (HAS_GMCH(dev_priv))
-			val = I915_READ(PALETTE(pipe, i));
-		else
-			val = I915_READ(LGC_PALETTE(pipe, i));
+		val = I915_READ(PALETTE(pipe, i));
 
 		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							LGC_PALETTE_RED_MASK, val), 8);
@@ -1792,6 +1789,38 @@ static void chv_read_luts(struct intel_crtc_state *crtc_state)
 		i965_read_luts(crtc_state);
 }
 
+static struct drm_property_blob *
+ilk_read_lut_8(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+	u32 i, val;
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_lut) * LEGACY_LUT_LENGTH,
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
+		val = I915_READ(LGC_PALETTE(pipe, i));
+
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(
+							LGC_PALETTE_RED_MASK, val), 8);
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(
+							  LGC_PALETTE_GREEN_MASK, val), 8);
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(
+							 LGC_PALETTE_BLUE_MASK, val), 8);
+	}
+
+	return blob;
+}
+
 static struct drm_property_blob *
 ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
 {
@@ -1834,7 +1863,7 @@ static void ilk_read_luts(struct intel_crtc_state *crtc_state)
 		return;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc_state);
+		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc_state);
 	else
 		crtc_state->hw.gamma_lut = ilk_read_lut_10(crtc_state);
 }
@@ -1883,7 +1912,7 @@ static void glk_read_luts(struct intel_crtc_state *crtc_state)
 		return;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc_state);
+		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc_state);
 	else
 		crtc_state->hw.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
 }
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 06/12] drm/i915: Split i9xx_read_lut_8() to gmch vs. ilk variants

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

To mirror the load_luts path let's clone an ilk+ version
from i9xx_read_lut_8(). I guess the extra branch isn't a huge
issue but feels better to make a clean split.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 41 ++++++++++++++++++----
 1 file changed, 35 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 992290a07086..5890e3896f8d 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1674,10 +1674,7 @@ i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
 	blob_data = blob->data;
 
 	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
-		if (HAS_GMCH(dev_priv))
-			val = I915_READ(PALETTE(pipe, i));
-		else
-			val = I915_READ(LGC_PALETTE(pipe, i));
+		val = I915_READ(PALETTE(pipe, i));
 
 		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							LGC_PALETTE_RED_MASK, val), 8);
@@ -1792,6 +1789,38 @@ static void chv_read_luts(struct intel_crtc_state *crtc_state)
 		i965_read_luts(crtc_state);
 }
 
+static struct drm_property_blob *
+ilk_read_lut_8(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+	u32 i, val;
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_lut) * LEGACY_LUT_LENGTH,
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
+		val = I915_READ(LGC_PALETTE(pipe, i));
+
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(
+							LGC_PALETTE_RED_MASK, val), 8);
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(
+							  LGC_PALETTE_GREEN_MASK, val), 8);
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(
+							 LGC_PALETTE_BLUE_MASK, val), 8);
+	}
+
+	return blob;
+}
+
 static struct drm_property_blob *
 ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
 {
@@ -1834,7 +1863,7 @@ static void ilk_read_luts(struct intel_crtc_state *crtc_state)
 		return;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc_state);
+		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc_state);
 	else
 		crtc_state->hw.gamma_lut = ilk_read_lut_10(crtc_state);
 }
@@ -1883,7 +1912,7 @@ static void glk_read_luts(struct intel_crtc_state *crtc_state)
 		return;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc_state);
+		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc_state);
 	else
 		crtc_state->hw.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
 }
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 07/12] drm/i915: s/blob_data/lut/

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We're talking about LUT contents here so let's call the thing
'lut' rather than 'blob_data'. This is the name the load_lut()
code used before already.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 66 +++++++++++-----------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 5890e3896f8d..43435ed343f2 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1662,7 +1662,7 @@ i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
-	struct drm_color_lut *blob_data;
+	struct drm_color_lut *lut;
 	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
@@ -1671,16 +1671,16 @@ i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
 	if (IS_ERR(blob))
 		return NULL;
 
-	blob_data = blob->data;
+	lut = blob->data;
 
 	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
 		val = I915_READ(PALETTE(pipe, i));
 
-		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							LGC_PALETTE_RED_MASK, val), 8);
-		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
 							  LGC_PALETTE_GREEN_MASK, val), 8);
-		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].blue = intel_color_lut_pack(REG_FIELD_GET(
 							 LGC_PALETTE_BLUE_MASK, val), 8);
 	}
 
@@ -1703,7 +1703,7 @@ i965_read_lut_10p6(const struct intel_crtc_state *crtc_state)
 	u32 lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
-	struct drm_color_lut *blob_data;
+	struct drm_color_lut *lut;
 	u32 i, val1, val2;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
@@ -1712,25 +1712,25 @@ i965_read_lut_10p6(const struct intel_crtc_state *crtc_state)
 	if (IS_ERR(blob))
 		return NULL;
 
-	blob_data = blob->data;
+	lut = blob->data;
 
 	for (i = 0; i < lut_size - 1; i++) {
 		val1 = I915_READ(PALETTE(pipe, 2 * i + 0));
 		val2 = I915_READ(PALETTE(pipe, 2 * i + 1));
 
-		blob_data[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val2) << 8 |
+		lut[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val2) << 8 |
 						 REG_FIELD_GET(PALETTE_RED_MASK, val1);
-		blob_data[i].green = REG_FIELD_GET(PALETTE_GREEN_MASK, val2) << 8 |
+		lut[i].green = REG_FIELD_GET(PALETTE_GREEN_MASK, val2) << 8 |
 						   REG_FIELD_GET(PALETTE_GREEN_MASK, val1);
-		blob_data[i].blue = REG_FIELD_GET(PALETTE_BLUE_MASK, val2) << 8 |
+		lut[i].blue = REG_FIELD_GET(PALETTE_BLUE_MASK, val2) << 8 |
 						  REG_FIELD_GET(PALETTE_BLUE_MASK, val1);
 	}
 
-	blob_data[i].red = REG_FIELD_GET(PIPEGCMAX_RGB_MASK,
+	lut[i].red = REG_FIELD_GET(PIPEGCMAX_RGB_MASK,
 					 I915_READ(PIPEGCMAX(pipe, 0)));
-	blob_data[i].green = REG_FIELD_GET(PIPEGCMAX_RGB_MASK,
+	lut[i].green = REG_FIELD_GET(PIPEGCMAX_RGB_MASK,
 					   I915_READ(PIPEGCMAX(pipe, 1)));
-	blob_data[i].blue = REG_FIELD_GET(PIPEGCMAX_RGB_MASK,
+	lut[i].blue = REG_FIELD_GET(PIPEGCMAX_RGB_MASK,
 					  I915_READ(PIPEGCMAX(pipe, 2)));
 
 	return blob;
@@ -1755,7 +1755,7 @@ chv_read_cgm_lut(const struct intel_crtc_state *crtc_state)
 	u32 lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
-	struct drm_color_lut *blob_data;
+	struct drm_color_lut *lut;
 	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
@@ -1764,17 +1764,17 @@ chv_read_cgm_lut(const struct intel_crtc_state *crtc_state)
 	if (IS_ERR(blob))
 		return NULL;
 
-	blob_data = blob->data;
+	lut = blob->data;
 
 	for (i = 0; i < lut_size; i++) {
 		val = I915_READ(CGM_PIPE_GAMMA(pipe, i, 0));
-		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
 							  CGM_PIPE_GAMMA_GREEN_MASK, val), 10);
-		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].blue = intel_color_lut_pack(REG_FIELD_GET(
 							 CGM_PIPE_GAMMA_BLUE_MASK, val), 10);
 
 		val = I915_READ(CGM_PIPE_GAMMA(pipe, i, 1));
-		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							CGM_PIPE_GAMMA_RED_MASK, val), 10);
 	}
 
@@ -1796,7 +1796,7 @@ ilk_read_lut_8(const struct intel_crtc_state *crtc_state)
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
-	struct drm_color_lut *blob_data;
+	struct drm_color_lut *lut;
 	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
@@ -1805,16 +1805,16 @@ ilk_read_lut_8(const struct intel_crtc_state *crtc_state)
 	if (IS_ERR(blob))
 		return NULL;
 
-	blob_data = blob->data;
+	lut = blob->data;
 
 	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
 		val = I915_READ(LGC_PALETTE(pipe, i));
 
-		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							LGC_PALETTE_RED_MASK, val), 8);
-		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
 							  LGC_PALETTE_GREEN_MASK, val), 8);
-		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].blue = intel_color_lut_pack(REG_FIELD_GET(
 							 LGC_PALETTE_BLUE_MASK, val), 8);
 	}
 
@@ -1829,7 +1829,7 @@ ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
 	u32 lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
-	struct drm_color_lut *blob_data;
+	struct drm_color_lut *lut;
 	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
@@ -1838,16 +1838,16 @@ ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
 	if (IS_ERR(blob))
 		return NULL;
 
-	blob_data = blob->data;
+	lut = blob->data;
 
 	for (i = 0; i < lut_size; i++) {
 		val = I915_READ(PREC_PALETTE(pipe, i));
 
-		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							PREC_PALETTE_RED_MASK, val), 10);
-		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
 							  PREC_PALETTE_GREEN_MASK, val), 10);
-		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].blue = intel_color_lut_pack(REG_FIELD_GET(
 							 PREC_PALETTE_BLUE_MASK, val), 10);
 	}
 
@@ -1876,7 +1876,7 @@ glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
 	int hw_lut_size = ivb_lut_10_size(prec_index);
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
-	struct drm_color_lut *blob_data;
+	struct drm_color_lut *lut;
 	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
@@ -1885,7 +1885,7 @@ glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
 	if (IS_ERR(blob))
 		return NULL;
 
-	blob_data = blob->data;
+	lut = blob->data;
 
 	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
 		   PAL_PREC_AUTO_INCREMENT);
@@ -1893,11 +1893,11 @@ glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
 	for (i = 0; i < hw_lut_size; i++) {
 		val = I915_READ(PREC_PAL_DATA(pipe));
 
-		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							PREC_PAL_DATA_RED_MASK, val), 10);
-		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
 							PREC_PAL_DATA_GREEN_MASK, val), 10);
-		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].blue = intel_color_lut_pack(REG_FIELD_GET(
 							PREC_PAL_DATA_BLUE_MASK, val), 10);
 	}
 
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 07/12] drm/i915: s/blob_data/lut/

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We're talking about LUT contents here so let's call the thing
'lut' rather than 'blob_data'. This is the name the load_lut()
code used before already.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 66 +++++++++++-----------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 5890e3896f8d..43435ed343f2 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1662,7 +1662,7 @@ i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
-	struct drm_color_lut *blob_data;
+	struct drm_color_lut *lut;
 	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
@@ -1671,16 +1671,16 @@ i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
 	if (IS_ERR(blob))
 		return NULL;
 
-	blob_data = blob->data;
+	lut = blob->data;
 
 	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
 		val = I915_READ(PALETTE(pipe, i));
 
-		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							LGC_PALETTE_RED_MASK, val), 8);
-		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
 							  LGC_PALETTE_GREEN_MASK, val), 8);
-		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].blue = intel_color_lut_pack(REG_FIELD_GET(
 							 LGC_PALETTE_BLUE_MASK, val), 8);
 	}
 
@@ -1703,7 +1703,7 @@ i965_read_lut_10p6(const struct intel_crtc_state *crtc_state)
 	u32 lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
-	struct drm_color_lut *blob_data;
+	struct drm_color_lut *lut;
 	u32 i, val1, val2;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
@@ -1712,25 +1712,25 @@ i965_read_lut_10p6(const struct intel_crtc_state *crtc_state)
 	if (IS_ERR(blob))
 		return NULL;
 
-	blob_data = blob->data;
+	lut = blob->data;
 
 	for (i = 0; i < lut_size - 1; i++) {
 		val1 = I915_READ(PALETTE(pipe, 2 * i + 0));
 		val2 = I915_READ(PALETTE(pipe, 2 * i + 1));
 
-		blob_data[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val2) << 8 |
+		lut[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val2) << 8 |
 						 REG_FIELD_GET(PALETTE_RED_MASK, val1);
-		blob_data[i].green = REG_FIELD_GET(PALETTE_GREEN_MASK, val2) << 8 |
+		lut[i].green = REG_FIELD_GET(PALETTE_GREEN_MASK, val2) << 8 |
 						   REG_FIELD_GET(PALETTE_GREEN_MASK, val1);
-		blob_data[i].blue = REG_FIELD_GET(PALETTE_BLUE_MASK, val2) << 8 |
+		lut[i].blue = REG_FIELD_GET(PALETTE_BLUE_MASK, val2) << 8 |
 						  REG_FIELD_GET(PALETTE_BLUE_MASK, val1);
 	}
 
-	blob_data[i].red = REG_FIELD_GET(PIPEGCMAX_RGB_MASK,
+	lut[i].red = REG_FIELD_GET(PIPEGCMAX_RGB_MASK,
 					 I915_READ(PIPEGCMAX(pipe, 0)));
-	blob_data[i].green = REG_FIELD_GET(PIPEGCMAX_RGB_MASK,
+	lut[i].green = REG_FIELD_GET(PIPEGCMAX_RGB_MASK,
 					   I915_READ(PIPEGCMAX(pipe, 1)));
-	blob_data[i].blue = REG_FIELD_GET(PIPEGCMAX_RGB_MASK,
+	lut[i].blue = REG_FIELD_GET(PIPEGCMAX_RGB_MASK,
 					  I915_READ(PIPEGCMAX(pipe, 2)));
 
 	return blob;
@@ -1755,7 +1755,7 @@ chv_read_cgm_lut(const struct intel_crtc_state *crtc_state)
 	u32 lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
-	struct drm_color_lut *blob_data;
+	struct drm_color_lut *lut;
 	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
@@ -1764,17 +1764,17 @@ chv_read_cgm_lut(const struct intel_crtc_state *crtc_state)
 	if (IS_ERR(blob))
 		return NULL;
 
-	blob_data = blob->data;
+	lut = blob->data;
 
 	for (i = 0; i < lut_size; i++) {
 		val = I915_READ(CGM_PIPE_GAMMA(pipe, i, 0));
-		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
 							  CGM_PIPE_GAMMA_GREEN_MASK, val), 10);
-		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].blue = intel_color_lut_pack(REG_FIELD_GET(
 							 CGM_PIPE_GAMMA_BLUE_MASK, val), 10);
 
 		val = I915_READ(CGM_PIPE_GAMMA(pipe, i, 1));
-		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							CGM_PIPE_GAMMA_RED_MASK, val), 10);
 	}
 
@@ -1796,7 +1796,7 @@ ilk_read_lut_8(const struct intel_crtc_state *crtc_state)
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
-	struct drm_color_lut *blob_data;
+	struct drm_color_lut *lut;
 	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
@@ -1805,16 +1805,16 @@ ilk_read_lut_8(const struct intel_crtc_state *crtc_state)
 	if (IS_ERR(blob))
 		return NULL;
 
-	blob_data = blob->data;
+	lut = blob->data;
 
 	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
 		val = I915_READ(LGC_PALETTE(pipe, i));
 
-		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							LGC_PALETTE_RED_MASK, val), 8);
-		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
 							  LGC_PALETTE_GREEN_MASK, val), 8);
-		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].blue = intel_color_lut_pack(REG_FIELD_GET(
 							 LGC_PALETTE_BLUE_MASK, val), 8);
 	}
 
@@ -1829,7 +1829,7 @@ ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
 	u32 lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
-	struct drm_color_lut *blob_data;
+	struct drm_color_lut *lut;
 	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
@@ -1838,16 +1838,16 @@ ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
 	if (IS_ERR(blob))
 		return NULL;
 
-	blob_data = blob->data;
+	lut = blob->data;
 
 	for (i = 0; i < lut_size; i++) {
 		val = I915_READ(PREC_PALETTE(pipe, i));
 
-		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							PREC_PALETTE_RED_MASK, val), 10);
-		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
 							  PREC_PALETTE_GREEN_MASK, val), 10);
-		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].blue = intel_color_lut_pack(REG_FIELD_GET(
 							 PREC_PALETTE_BLUE_MASK, val), 10);
 	}
 
@@ -1876,7 +1876,7 @@ glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
 	int hw_lut_size = ivb_lut_10_size(prec_index);
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
-	struct drm_color_lut *blob_data;
+	struct drm_color_lut *lut;
 	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
@@ -1885,7 +1885,7 @@ glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
 	if (IS_ERR(blob))
 		return NULL;
 
-	blob_data = blob->data;
+	lut = blob->data;
 
 	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
 		   PAL_PREC_AUTO_INCREMENT);
@@ -1893,11 +1893,11 @@ glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
 	for (i = 0; i < hw_lut_size; i++) {
 		val = I915_READ(PREC_PAL_DATA(pipe));
 
-		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							PREC_PAL_DATA_RED_MASK, val), 10);
-		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
 							PREC_PAL_DATA_GREEN_MASK, val), 10);
-		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(
+		lut[i].blue = intel_color_lut_pack(REG_FIELD_GET(
 							PREC_PAL_DATA_BLUE_MASK, val), 10);
 	}
 
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 08/12] drm/i915: s/chv_read_cgm_lut/chv_read_cgm_gamma/

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

chv_read_cgm_lut() specifically reads the CGM _gamma_ LUT so
let's rename it to reflect that fact. This also mirrors
the other direction's chv_load_cgm_gamma().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 43435ed343f2..30c0b939620c 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1748,7 +1748,7 @@ static void i965_read_luts(struct intel_crtc_state *crtc_state)
 }
 
 static struct drm_property_blob *
-chv_read_cgm_lut(const struct intel_crtc_state *crtc_state)
+chv_read_cgm_gamma(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1784,7 +1784,7 @@ chv_read_cgm_lut(const struct intel_crtc_state *crtc_state)
 static void chv_read_luts(struct intel_crtc_state *crtc_state)
 {
 	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
-		crtc_state->hw.gamma_lut = chv_read_cgm_lut(crtc_state);
+		crtc_state->hw.gamma_lut = chv_read_cgm_gamma(crtc_state);
 	else
 		i965_read_luts(crtc_state);
 }
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 08/12] drm/i915: s/chv_read_cgm_lut/chv_read_cgm_gamma/

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

chv_read_cgm_lut() specifically reads the CGM _gamma_ LUT so
let's rename it to reflect that fact. This also mirrors
the other direction's chv_load_cgm_gamma().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 43435ed343f2..30c0b939620c 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1748,7 +1748,7 @@ static void i965_read_luts(struct intel_crtc_state *crtc_state)
 }
 
 static struct drm_property_blob *
-chv_read_cgm_lut(const struct intel_crtc_state *crtc_state)
+chv_read_cgm_gamma(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1784,7 +1784,7 @@ chv_read_cgm_lut(const struct intel_crtc_state *crtc_state)
 static void chv_read_luts(struct intel_crtc_state *crtc_state)
 {
 	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
-		crtc_state->hw.gamma_lut = chv_read_cgm_lut(crtc_state);
+		crtc_state->hw.gamma_lut = chv_read_cgm_gamma(crtc_state);
 	else
 		i965_read_luts(crtc_state);
 }
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/12] drm/i915: Clean up integer types in color code

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

A variable called 'i' having an unsigned type is just looking for
trouble, and using a sized type generally makes no sense either.
Change all of them to just plain old int. And do the same for some
'lut_size' variables which generally provide the loop end codition
for 'i'.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 42 ++++++++++------------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 30c0b939620c..d6a20d7522a9 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -713,9 +713,8 @@ static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state)
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
-	const u32 lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
+	int i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
 	const struct drm_color_lut *lut = crtc_state->hw.degamma_lut->data;
-	u32 i;
 
 	/*
 	 * When setting the auto-increment bit, the hardware seems to
@@ -752,8 +751,7 @@ static void glk_load_degamma_lut_linear(const struct intel_crtc_state *crtc_stat
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
-	const u32 lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
-	u32 i;
+	int i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
 
 	/*
 	 * When setting the auto-increment bit, the hardware seems to
@@ -837,7 +835,7 @@ icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
 	const struct drm_color_lut *lut = blob->data;
 	struct intel_dsb *dsb = intel_dsb_get(crtc);
 	enum pipe pipe = crtc->pipe;
-	u32 i;
+	int i;
 
 	/*
 	 * Program Super Fine segment (let's call it seg1)...
@@ -870,7 +868,7 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
 	const struct drm_color_lut *entry;
 	struct intel_dsb *dsb = intel_dsb_get(crtc);
 	enum pipe pipe = crtc->pipe;
-	u32 i;
+	int i;
 
 	/*
 	 * Program Fine segment (let's call it seg2)...
@@ -1643,7 +1641,7 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
 }
 
 /* convert hw value with given bit_precision to lut property val */
-static u32 intel_color_lut_pack(u32 val, u32 bit_precision)
+static u32 intel_color_lut_pack(u32 val, int bit_precision)
 {
 	u32 max = 0xffff >> (16 - bit_precision);
 
@@ -1663,7 +1661,7 @@ i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
 	struct drm_color_lut *lut;
-	u32 i, val;
+	int i;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
 					sizeof(struct drm_color_lut) * LEGACY_LUT_LENGTH,
@@ -1674,7 +1672,7 @@ i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
 	lut = blob->data;
 
 	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
-		val = I915_READ(PALETTE(pipe, i));
+		u32 val = I915_READ(PALETTE(pipe, i));
 
 		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							LGC_PALETTE_RED_MASK, val), 8);
@@ -1700,11 +1698,10 @@ i965_read_lut_10p6(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	u32 lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+	int i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
 	struct drm_color_lut *lut;
-	u32 i, val1, val2;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
 					sizeof(struct drm_color_lut) * lut_size,
@@ -1715,8 +1712,8 @@ i965_read_lut_10p6(const struct intel_crtc_state *crtc_state)
 	lut = blob->data;
 
 	for (i = 0; i < lut_size - 1; i++) {
-		val1 = I915_READ(PALETTE(pipe, 2 * i + 0));
-		val2 = I915_READ(PALETTE(pipe, 2 * i + 1));
+		u32 val1 = I915_READ(PALETTE(pipe, 2 * i + 0));
+		u32 val2 = I915_READ(PALETTE(pipe, 2 * i + 1));
 
 		lut[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val2) << 8 |
 						 REG_FIELD_GET(PALETTE_RED_MASK, val1);
@@ -1752,11 +1749,10 @@ chv_read_cgm_gamma(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	u32 lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+	int i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
 	struct drm_color_lut *lut;
-	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
 					sizeof(struct drm_color_lut) * lut_size,
@@ -1767,6 +1763,8 @@ chv_read_cgm_gamma(const struct intel_crtc_state *crtc_state)
 	lut = blob->data;
 
 	for (i = 0; i < lut_size; i++) {
+		u32 val;
+
 		val = I915_READ(CGM_PIPE_GAMMA(pipe, i, 0));
 		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
 							  CGM_PIPE_GAMMA_GREEN_MASK, val), 10);
@@ -1797,7 +1795,7 @@ ilk_read_lut_8(const struct intel_crtc_state *crtc_state)
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
 	struct drm_color_lut *lut;
-	u32 i, val;
+	int i;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
 					sizeof(struct drm_color_lut) * LEGACY_LUT_LENGTH,
@@ -1808,7 +1806,7 @@ ilk_read_lut_8(const struct intel_crtc_state *crtc_state)
 	lut = blob->data;
 
 	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
-		val = I915_READ(LGC_PALETTE(pipe, i));
+		u32 val = I915_READ(LGC_PALETTE(pipe, i));
 
 		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							LGC_PALETTE_RED_MASK, val), 8);
@@ -1826,11 +1824,10 @@ ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	u32 lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+	int i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
 	struct drm_color_lut *lut;
-	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
 					sizeof(struct drm_color_lut) * lut_size,
@@ -1841,7 +1838,7 @@ ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
 	lut = blob->data;
 
 	for (i = 0; i < lut_size; i++) {
-		val = I915_READ(PREC_PALETTE(pipe, i));
+		u32 val = I915_READ(PREC_PALETTE(pipe, i));
 
 		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							PREC_PALETTE_RED_MASK, val), 10);
@@ -1873,11 +1870,10 @@ glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	int hw_lut_size = ivb_lut_10_size(prec_index);
+	int i, hw_lut_size = ivb_lut_10_size(prec_index);
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
 	struct drm_color_lut *lut;
-	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
 					sizeof(struct drm_color_lut) * hw_lut_size,
@@ -1891,7 +1887,7 @@ glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
 		   PAL_PREC_AUTO_INCREMENT);
 
 	for (i = 0; i < hw_lut_size; i++) {
-		val = I915_READ(PREC_PAL_DATA(pipe));
+		u32 val = I915_READ(PREC_PAL_DATA(pipe));
 
 		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							PREC_PAL_DATA_RED_MASK, val), 10);
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 09/12] drm/i915: Clean up integer types in color code

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

A variable called 'i' having an unsigned type is just looking for
trouble, and using a sized type generally makes no sense either.
Change all of them to just plain old int. And do the same for some
'lut_size' variables which generally provide the loop end codition
for 'i'.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 42 ++++++++++------------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 30c0b939620c..d6a20d7522a9 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -713,9 +713,8 @@ static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state)
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
-	const u32 lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
+	int i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
 	const struct drm_color_lut *lut = crtc_state->hw.degamma_lut->data;
-	u32 i;
 
 	/*
 	 * When setting the auto-increment bit, the hardware seems to
@@ -752,8 +751,7 @@ static void glk_load_degamma_lut_linear(const struct intel_crtc_state *crtc_stat
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
-	const u32 lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
-	u32 i;
+	int i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
 
 	/*
 	 * When setting the auto-increment bit, the hardware seems to
@@ -837,7 +835,7 @@ icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
 	const struct drm_color_lut *lut = blob->data;
 	struct intel_dsb *dsb = intel_dsb_get(crtc);
 	enum pipe pipe = crtc->pipe;
-	u32 i;
+	int i;
 
 	/*
 	 * Program Super Fine segment (let's call it seg1)...
@@ -870,7 +868,7 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
 	const struct drm_color_lut *entry;
 	struct intel_dsb *dsb = intel_dsb_get(crtc);
 	enum pipe pipe = crtc->pipe;
-	u32 i;
+	int i;
 
 	/*
 	 * Program Fine segment (let's call it seg2)...
@@ -1643,7 +1641,7 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
 }
 
 /* convert hw value with given bit_precision to lut property val */
-static u32 intel_color_lut_pack(u32 val, u32 bit_precision)
+static u32 intel_color_lut_pack(u32 val, int bit_precision)
 {
 	u32 max = 0xffff >> (16 - bit_precision);
 
@@ -1663,7 +1661,7 @@ i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
 	struct drm_color_lut *lut;
-	u32 i, val;
+	int i;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
 					sizeof(struct drm_color_lut) * LEGACY_LUT_LENGTH,
@@ -1674,7 +1672,7 @@ i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
 	lut = blob->data;
 
 	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
-		val = I915_READ(PALETTE(pipe, i));
+		u32 val = I915_READ(PALETTE(pipe, i));
 
 		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							LGC_PALETTE_RED_MASK, val), 8);
@@ -1700,11 +1698,10 @@ i965_read_lut_10p6(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	u32 lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+	int i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
 	struct drm_color_lut *lut;
-	u32 i, val1, val2;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
 					sizeof(struct drm_color_lut) * lut_size,
@@ -1715,8 +1712,8 @@ i965_read_lut_10p6(const struct intel_crtc_state *crtc_state)
 	lut = blob->data;
 
 	for (i = 0; i < lut_size - 1; i++) {
-		val1 = I915_READ(PALETTE(pipe, 2 * i + 0));
-		val2 = I915_READ(PALETTE(pipe, 2 * i + 1));
+		u32 val1 = I915_READ(PALETTE(pipe, 2 * i + 0));
+		u32 val2 = I915_READ(PALETTE(pipe, 2 * i + 1));
 
 		lut[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val2) << 8 |
 						 REG_FIELD_GET(PALETTE_RED_MASK, val1);
@@ -1752,11 +1749,10 @@ chv_read_cgm_gamma(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	u32 lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+	int i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
 	struct drm_color_lut *lut;
-	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
 					sizeof(struct drm_color_lut) * lut_size,
@@ -1767,6 +1763,8 @@ chv_read_cgm_gamma(const struct intel_crtc_state *crtc_state)
 	lut = blob->data;
 
 	for (i = 0; i < lut_size; i++) {
+		u32 val;
+
 		val = I915_READ(CGM_PIPE_GAMMA(pipe, i, 0));
 		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
 							  CGM_PIPE_GAMMA_GREEN_MASK, val), 10);
@@ -1797,7 +1795,7 @@ ilk_read_lut_8(const struct intel_crtc_state *crtc_state)
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
 	struct drm_color_lut *lut;
-	u32 i, val;
+	int i;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
 					sizeof(struct drm_color_lut) * LEGACY_LUT_LENGTH,
@@ -1808,7 +1806,7 @@ ilk_read_lut_8(const struct intel_crtc_state *crtc_state)
 	lut = blob->data;
 
 	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
-		val = I915_READ(LGC_PALETTE(pipe, i));
+		u32 val = I915_READ(LGC_PALETTE(pipe, i));
 
 		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							LGC_PALETTE_RED_MASK, val), 8);
@@ -1826,11 +1824,10 @@ ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	u32 lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+	int i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
 	struct drm_color_lut *lut;
-	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
 					sizeof(struct drm_color_lut) * lut_size,
@@ -1841,7 +1838,7 @@ ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
 	lut = blob->data;
 
 	for (i = 0; i < lut_size; i++) {
-		val = I915_READ(PREC_PALETTE(pipe, i));
+		u32 val = I915_READ(PREC_PALETTE(pipe, i));
 
 		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							PREC_PALETTE_RED_MASK, val), 10);
@@ -1873,11 +1870,10 @@ glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	int hw_lut_size = ivb_lut_10_size(prec_index);
+	int i, hw_lut_size = ivb_lut_10_size(prec_index);
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
 	struct drm_color_lut *lut;
-	u32 i, val;
 
 	blob = drm_property_create_blob(&dev_priv->drm,
 					sizeof(struct drm_color_lut) * hw_lut_size,
@@ -1891,7 +1887,7 @@ glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
 		   PAL_PREC_AUTO_INCREMENT);
 
 	for (i = 0; i < hw_lut_size; i++) {
-		val = I915_READ(PREC_PAL_DATA(pipe));
+		u32 val = I915_READ(PREC_PAL_DATA(pipe));
 
 		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
 							PREC_PAL_DATA_RED_MASK, val), 10);
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 10/12] drm/i915: Refactor LUT read functions

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Extract all the 'hw value -> LUT entry' stuff into small helpers
to make the main 'read out the entire LUT' loop less bogged down
by such mundane details.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 128 ++++++++++-----------
 1 file changed, 64 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index d6a20d7522a9..4b2bd5ac0e8d 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -367,6 +367,19 @@ static void chv_load_cgm_csc(struct intel_crtc *crtc,
 	I915_WRITE(CGM_PIPE_CSC_COEFF8(pipe), coeffs[8]);
 }
 
+/* convert hw value with given bit_precision to lut property val */
+static u32 intel_color_lut_pack(u32 val, int bit_precision)
+{
+	u32 max = 0xffff >> (16 - bit_precision);
+
+	val = clamp_val(val, 0, max);
+
+	if (bit_precision < 16)
+		val <<= 16 - bit_precision;
+
+	return val;
+}
+
 static u32 i9xx_lut_8(const struct drm_color_lut *color)
 {
 	return drm_color_lut_extract(color->red, 8) << 16 |
@@ -374,6 +387,13 @@ static u32 i9xx_lut_8(const struct drm_color_lut *color)
 		drm_color_lut_extract(color->blue, 8);
 }
 
+static void i9xx_lut_8_pack(struct drm_color_lut *entry, u32 val)
+{
+	entry->red = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_RED_MASK, val), 8);
+	entry->green = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_GREEN_MASK, val), 8);
+	entry->blue = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_BLUE_MASK, val), 8);
+}
+
 /* i965+ "10.6" bit interpolated format "even DW" (low 8 bits) */
 static u32 i965_lut_10p6_ldw(const struct drm_color_lut *color)
 {
@@ -390,6 +410,21 @@ static u32 i965_lut_10p6_udw(const struct drm_color_lut *color)
 		(color->blue >> 8);
 }
 
+static void i965_lut_10p6_pack(struct drm_color_lut *entry, u32 ldw, u32 udw)
+{
+	entry->red = REG_FIELD_GET(PALETTE_RED_MASK, udw) << 8 |
+		REG_FIELD_GET(PALETTE_RED_MASK, ldw);
+	entry->green = REG_FIELD_GET(PALETTE_GREEN_MASK, udw) << 8 |
+		REG_FIELD_GET(PALETTE_GREEN_MASK, ldw);
+	entry->blue = REG_FIELD_GET(PALETTE_BLUE_MASK, udw) << 8 |
+		REG_FIELD_GET(PALETTE_BLUE_MASK, ldw);
+}
+
+static u16 i965_lut_11p6_max_pack(u32 val)
+{
+	return REG_FIELD_GET(PIPEGCMAX_RGB_MASK, val);
+}
+
 static u32 ilk_lut_10(const struct drm_color_lut *color)
 {
 	return drm_color_lut_extract(color->red, 10) << 20 |
@@ -397,6 +432,13 @@ static u32 ilk_lut_10(const struct drm_color_lut *color)
 		drm_color_lut_extract(color->blue, 10);
 }
 
+static void ilk_lut_10_pack(struct drm_color_lut *entry, u32 val)
+{
+	entry->red = intel_color_lut_pack(REG_FIELD_GET(PREC_PALETTE_RED_MASK, val), 10);
+	entry->green = intel_color_lut_pack(REG_FIELD_GET(PREC_PALETTE_GREEN_MASK, val), 10);
+	entry->blue = intel_color_lut_pack(REG_FIELD_GET(PREC_PALETTE_BLUE_MASK, val), 10);
+}
+
 static void i9xx_color_commit(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
@@ -953,6 +995,13 @@ static u32 chv_cgm_degamma_udw(const struct drm_color_lut *color)
 	return drm_color_lut_extract(color->red, 14);
 }
 
+static void chv_cgm_gamma_pack(struct drm_color_lut *entry, u32 ldw, u32 udw)
+{
+	entry->green = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_GREEN_MASK, ldw), 10);
+	entry->blue = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_BLUE_MASK, ldw), 10);
+	entry->red = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_RED_MASK, udw), 10);
+}
+
 static void chv_load_cgm_degamma(struct intel_crtc *crtc,
 				 const struct drm_property_blob *blob)
 {
@@ -1640,19 +1689,6 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
 	return true;
 }
 
-/* convert hw value with given bit_precision to lut property val */
-static u32 intel_color_lut_pack(u32 val, int bit_precision)
-{
-	u32 max = 0xffff >> (16 - bit_precision);
-
-	val = clamp_val(val, 0, max);
-
-	if (bit_precision < 16)
-		val <<= 16 - bit_precision;
-
-	return val;
-}
-
 static struct drm_property_blob *
 i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
 {
@@ -1674,12 +1710,7 @@ i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
 	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
 		u32 val = I915_READ(PALETTE(pipe, i));
 
-		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
-							LGC_PALETTE_RED_MASK, val), 8);
-		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
-							  LGC_PALETTE_GREEN_MASK, val), 8);
-		lut[i].blue = intel_color_lut_pack(REG_FIELD_GET(
-							 LGC_PALETTE_BLUE_MASK, val), 8);
+		i9xx_lut_8_pack(&lut[i], val);
 	}
 
 	return blob;
@@ -1712,23 +1743,15 @@ i965_read_lut_10p6(const struct intel_crtc_state *crtc_state)
 	lut = blob->data;
 
 	for (i = 0; i < lut_size - 1; i++) {
-		u32 val1 = I915_READ(PALETTE(pipe, 2 * i + 0));
-		u32 val2 = I915_READ(PALETTE(pipe, 2 * i + 1));
-
-		lut[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val2) << 8 |
-						 REG_FIELD_GET(PALETTE_RED_MASK, val1);
-		lut[i].green = REG_FIELD_GET(PALETTE_GREEN_MASK, val2) << 8 |
-						   REG_FIELD_GET(PALETTE_GREEN_MASK, val1);
-		lut[i].blue = REG_FIELD_GET(PALETTE_BLUE_MASK, val2) << 8 |
-						  REG_FIELD_GET(PALETTE_BLUE_MASK, val1);
+		u32 ldw = I915_READ(PALETTE(pipe, 2 * i + 0));
+		u32 udw = I915_READ(PALETTE(pipe, 2 * i + 1));
+
+		i965_lut_10p6_pack(&lut[i], ldw, udw);
 	}
 
-	lut[i].red = REG_FIELD_GET(PIPEGCMAX_RGB_MASK,
-					 I915_READ(PIPEGCMAX(pipe, 0)));
-	lut[i].green = REG_FIELD_GET(PIPEGCMAX_RGB_MASK,
-					   I915_READ(PIPEGCMAX(pipe, 1)));
-	lut[i].blue = REG_FIELD_GET(PIPEGCMAX_RGB_MASK,
-					  I915_READ(PIPEGCMAX(pipe, 2)));
+	lut[i].red = i965_lut_11p6_max_pack(I915_READ(PIPEGCMAX(pipe, 0)));
+	lut[i].green = i965_lut_11p6_max_pack(I915_READ(PIPEGCMAX(pipe, 1)));
+	lut[i].blue = i965_lut_11p6_max_pack(I915_READ(PIPEGCMAX(pipe, 2)));
 
 	return blob;
 }
@@ -1763,17 +1786,10 @@ chv_read_cgm_gamma(const struct intel_crtc_state *crtc_state)
 	lut = blob->data;
 
 	for (i = 0; i < lut_size; i++) {
-		u32 val;
-
-		val = I915_READ(CGM_PIPE_GAMMA(pipe, i, 0));
-		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
-							  CGM_PIPE_GAMMA_GREEN_MASK, val), 10);
-		lut[i].blue = intel_color_lut_pack(REG_FIELD_GET(
-							 CGM_PIPE_GAMMA_BLUE_MASK, val), 10);
+		u32 ldw = I915_READ(CGM_PIPE_GAMMA(pipe, i, 0));
+		u32 udw = I915_READ(CGM_PIPE_GAMMA(pipe, i, 1));
 
-		val = I915_READ(CGM_PIPE_GAMMA(pipe, i, 1));
-		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
-							CGM_PIPE_GAMMA_RED_MASK, val), 10);
+		chv_cgm_gamma_pack(&lut[i], ldw, udw);
 	}
 
 	return blob;
@@ -1808,12 +1824,7 @@ ilk_read_lut_8(const struct intel_crtc_state *crtc_state)
 	for (i = 0; i < LEGACY_LUT_LENGTH; i++) {
 		u32 val = I915_READ(LGC_PALETTE(pipe, i));
 
-		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
-							LGC_PALETTE_RED_MASK, val), 8);
-		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
-							  LGC_PALETTE_GREEN_MASK, val), 8);
-		lut[i].blue = intel_color_lut_pack(REG_FIELD_GET(
-							 LGC_PALETTE_BLUE_MASK, val), 8);
+		i9xx_lut_8_pack(&lut[i], val);
 	}
 
 	return blob;
@@ -1840,12 +1851,7 @@ ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
 	for (i = 0; i < lut_size; i++) {
 		u32 val = I915_READ(PREC_PALETTE(pipe, i));
 
-		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
-							PREC_PALETTE_RED_MASK, val), 10);
-		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
-							  PREC_PALETTE_GREEN_MASK, val), 10);
-		lut[i].blue = intel_color_lut_pack(REG_FIELD_GET(
-							 PREC_PALETTE_BLUE_MASK, val), 10);
+		ilk_lut_10_pack(&lut[i], val);
 	}
 
 	return blob;
@@ -1883,18 +1889,12 @@ glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
 
 	lut = blob->data;
 
-	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
-		   PAL_PREC_AUTO_INCREMENT);
+	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index | PAL_PREC_AUTO_INCREMENT);
 
 	for (i = 0; i < hw_lut_size; i++) {
 		u32 val = I915_READ(PREC_PAL_DATA(pipe));
 
-		lut[i].red = intel_color_lut_pack(REG_FIELD_GET(
-							PREC_PAL_DATA_RED_MASK, val), 10);
-		lut[i].green = intel_color_lut_pack(REG_FIELD_GET(
-							PREC_PAL_DATA_GREEN_MASK, val), 10);
-		lut[i].blue = intel_color_lut_pack(REG_FIELD_GET(
-							PREC_PAL_DATA_BLUE_MASK, val), 10);
+		ilk_lut_10_pack(&lut[i], val);
 	}
 
 	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 11/12] drm/i915: Fix readout of PIPEGCMAX

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

PIPEGCMAX is a 11.6 (or 1.16 if you will) value. Ie. it can
represent a value of 1.0 when the maximum we can store in the
software LUT is 0.ffff. Clamp the value so that it gets
saturated to the max the uapi supports.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 3 ++-
 drivers/gpu/drm/i915/i915_reg.h            | 1 -
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 4b2bd5ac0e8d..3fd517fa1de5 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -422,7 +422,8 @@ static void i965_lut_10p6_pack(struct drm_color_lut *entry, u32 ldw, u32 udw)
 
 static u16 i965_lut_11p6_max_pack(u32 val)
 {
-	return REG_FIELD_GET(PIPEGCMAX_RGB_MASK, val);
+	/* PIPEGCMAX is 11.6, clamp to 10.6 */
+	return clamp_val(val, 0, 0xffff);
 }
 
 static u32 ilk_lut_10(const struct drm_color_lut *color)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a607ea520829..4a8021a33b64 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5871,7 +5871,6 @@ enum {
 
 #define  _PIPEAGCMAX           0x70010
 #define  _PIPEBGCMAX           0x71010
-#define PIPEGCMAX_RGB_MASK     REG_GENMASK(15, 0)
 #define PIPEGCMAX(pipe, i)     _MMIO_PIPE2(pipe, _PIPEAGCMAX + (i) * 4)
 
 #define _PIPE_MISC_A			0x70030
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 12/12] drm/i915: Pass the crtc to the low level read_lut() funcs

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The low level read_lut() functions don't need the entire crtc state
as they know exactly what they're reading. Just need to pass in the
crtc to get at the pipe. This now neatly mirrors the load_lut()
direction.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 51 +++++++++++-----------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 3fd517fa1de5..f0f372b9b3bd 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1690,10 +1690,8 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
 	return true;
 }
 
-static struct drm_property_blob *
-i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
+static struct drm_property_blob *i9xx_read_lut_8(struct intel_crtc *crtc)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
@@ -1719,16 +1717,16 @@ i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
 
 static void i9xx_read_luts(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
 	if (!crtc_state->gamma_enable)
 		return;
 
-	crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc_state);
+	crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc);
 }
 
-static struct drm_property_blob *
-i965_read_lut_10p6(const struct intel_crtc_state *crtc_state)
+static struct drm_property_blob *i965_read_lut_10p6(struct intel_crtc *crtc)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	int i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
@@ -1759,19 +1757,19 @@ i965_read_lut_10p6(const struct intel_crtc_state *crtc_state)
 
 static void i965_read_luts(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
 	if (!crtc_state->gamma_enable)
 		return;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc_state);
+		crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc);
 	else
-		crtc_state->hw.gamma_lut = i965_read_lut_10p6(crtc_state);
+		crtc_state->hw.gamma_lut = i965_read_lut_10p6(crtc);
 }
 
-static struct drm_property_blob *
-chv_read_cgm_gamma(const struct intel_crtc_state *crtc_state)
+static struct drm_property_blob *chv_read_cgm_gamma(struct intel_crtc *crtc)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	int i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
@@ -1798,16 +1796,16 @@ chv_read_cgm_gamma(const struct intel_crtc_state *crtc_state)
 
 static void chv_read_luts(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
 	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
-		crtc_state->hw.gamma_lut = chv_read_cgm_gamma(crtc_state);
+		crtc_state->hw.gamma_lut = chv_read_cgm_gamma(crtc);
 	else
 		i965_read_luts(crtc_state);
 }
 
-static struct drm_property_blob *
-ilk_read_lut_8(const struct intel_crtc_state *crtc_state)
+static struct drm_property_blob *ilk_read_lut_8(struct intel_crtc *crtc)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
@@ -1831,10 +1829,8 @@ ilk_read_lut_8(const struct intel_crtc_state *crtc_state)
 	return blob;
 }
 
-static struct drm_property_blob *
-ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
+static struct drm_property_blob *ilk_read_lut_10(struct intel_crtc *crtc)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	int i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
@@ -1860,6 +1856,8 @@ ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
 
 static void ilk_read_luts(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
 	if (!crtc_state->gamma_enable)
 		return;
 
@@ -1867,15 +1865,14 @@ static void ilk_read_luts(struct intel_crtc_state *crtc_state)
 		return;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc_state);
+		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc);
 	else
-		crtc_state->hw.gamma_lut = ilk_read_lut_10(crtc_state);
+		crtc_state->hw.gamma_lut = ilk_read_lut_10(crtc);
 }
 
-static struct drm_property_blob *
-glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
+static struct drm_property_blob *glk_read_lut_10(struct intel_crtc *crtc,
+						 u32 prec_index)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	int i, hw_lut_size = ivb_lut_10_size(prec_index);
 	enum pipe pipe = crtc->pipe;
@@ -1905,13 +1902,15 @@ glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
 
 static void glk_read_luts(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
 	if (!crtc_state->gamma_enable)
 		return;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc_state);
+		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc);
 	else
-		crtc_state->hw.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
+		crtc_state->hw.gamma_lut = glk_read_lut_10(crtc, PAL_PREC_INDEX_VALUE(0));
 }
 
 void intel_color_init(struct intel_crtc *crtc)
-- 
2.23.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 12/12] drm/i915: Pass the crtc to the low level read_lut() funcs

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The low level read_lut() functions don't need the entire crtc state
as they know exactly what they're reading. Just need to pass in the
crtc to get at the pipe. This now neatly mirrors the load_lut()
direction.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 51 +++++++++++-----------
 1 file changed, 25 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 3fd517fa1de5..f0f372b9b3bd 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1690,10 +1690,8 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
 	return true;
 }
 
-static struct drm_property_blob *
-i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
+static struct drm_property_blob *i9xx_read_lut_8(struct intel_crtc *crtc)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
@@ -1719,16 +1717,16 @@ i9xx_read_lut_8(const struct intel_crtc_state *crtc_state)
 
 static void i9xx_read_luts(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
 	if (!crtc_state->gamma_enable)
 		return;
 
-	crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc_state);
+	crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc);
 }
 
-static struct drm_property_blob *
-i965_read_lut_10p6(const struct intel_crtc_state *crtc_state)
+static struct drm_property_blob *i965_read_lut_10p6(struct intel_crtc *crtc)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	int i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
@@ -1759,19 +1757,19 @@ i965_read_lut_10p6(const struct intel_crtc_state *crtc_state)
 
 static void i965_read_luts(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
 	if (!crtc_state->gamma_enable)
 		return;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc_state);
+		crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc);
 	else
-		crtc_state->hw.gamma_lut = i965_read_lut_10p6(crtc_state);
+		crtc_state->hw.gamma_lut = i965_read_lut_10p6(crtc);
 }
 
-static struct drm_property_blob *
-chv_read_cgm_gamma(const struct intel_crtc_state *crtc_state)
+static struct drm_property_blob *chv_read_cgm_gamma(struct intel_crtc *crtc)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	int i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
@@ -1798,16 +1796,16 @@ chv_read_cgm_gamma(const struct intel_crtc_state *crtc_state)
 
 static void chv_read_luts(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
 	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
-		crtc_state->hw.gamma_lut = chv_read_cgm_gamma(crtc_state);
+		crtc_state->hw.gamma_lut = chv_read_cgm_gamma(crtc);
 	else
 		i965_read_luts(crtc_state);
 }
 
-static struct drm_property_blob *
-ilk_read_lut_8(const struct intel_crtc_state *crtc_state)
+static struct drm_property_blob *ilk_read_lut_8(struct intel_crtc *crtc)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 	struct drm_property_blob *blob;
@@ -1831,10 +1829,8 @@ ilk_read_lut_8(const struct intel_crtc_state *crtc_state)
 	return blob;
 }
 
-static struct drm_property_blob *
-ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
+static struct drm_property_blob *ilk_read_lut_10(struct intel_crtc *crtc)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	int i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
@@ -1860,6 +1856,8 @@ ilk_read_lut_10(const struct intel_crtc_state *crtc_state)
 
 static void ilk_read_luts(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
 	if (!crtc_state->gamma_enable)
 		return;
 
@@ -1867,15 +1865,14 @@ static void ilk_read_luts(struct intel_crtc_state *crtc_state)
 		return;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc_state);
+		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc);
 	else
-		crtc_state->hw.gamma_lut = ilk_read_lut_10(crtc_state);
+		crtc_state->hw.gamma_lut = ilk_read_lut_10(crtc);
 }
 
-static struct drm_property_blob *
-glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
+static struct drm_property_blob *glk_read_lut_10(struct intel_crtc *crtc,
+						 u32 prec_index)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	int i, hw_lut_size = ivb_lut_10_size(prec_index);
 	enum pipe pipe = crtc->pipe;
@@ -1905,13 +1902,15 @@ glk_read_lut_10(const struct intel_crtc_state *crtc_state, u32 prec_index)
 
 static void glk_read_luts(struct intel_crtc_state *crtc_state)
 {
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
 	if (!crtc_state->gamma_enable)
 		return;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc_state);
+		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc);
 	else
-		crtc_state->hw.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
+		crtc_state->hw.gamma_lut = glk_read_lut_10(crtc, PAL_PREC_INDEX_VALUE(0));
 }
 
 void intel_color_init(struct intel_crtc *crtc)
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/12] drm: Inline drm_color_lut_extract()

[Kazlauskas, Nicholas]

On 2019-11-07 10:17 a.m., Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> This thing can get called several thousand times per LUT
> so seems like we want to inline it to:
> - avoid the function call overhead
> - allow constant folding
> 
> A quick synthetic test (w/o any hardware interaction) with
> a ridiculously large LUT size shows about 50% reduction in
> runtime on my HSW and BSW boxes. Slightly less with more
> reasonable LUT size but still easily measurable in tens
> of microseconds.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Seems reasonable to me. It would probably make sense to even split this 
further into two functions, one for high precision and one for low 
precision so it's purely a calculation and not hitting any branches.

Nicholas Kazlauskas

> ---
>   drivers/gpu/drm/drm_color_mgmt.c | 24 ------------------------
>   include/drm/drm_color_mgmt.h     | 23 ++++++++++++++++++++++-
>   2 files changed, 22 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 4ce5c6d8de99..19c5f635992a 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -108,30 +108,6 @@
>    * 	standard enum values supported by the DRM plane.
>    */
>   
> -/**
> - * drm_color_lut_extract - clamp and round LUT entries
> - * @user_input: input value
> - * @bit_precision: number of bits the hw LUT supports
> - *
> - * Extract a degamma/gamma LUT value provided by user (in the form of
> - * &drm_color_lut entries) and round it to the precision supported by the
> - * hardware.
> - */
> -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> -{
> -	uint32_t val = user_input;
> -	uint32_t max = 0xffff >> (16 - bit_precision);
> -
> -	/* Round only if we're not using full precision. */
> -	if (bit_precision < 16) {
> -		val += 1UL << (16 - bit_precision - 1);
> -		val >>= 16 - bit_precision;
> -	}
> -
> -	return clamp_val(val, 0, max);
> -}
> -EXPORT_SYMBOL(drm_color_lut_extract);
> -
>   /**
>    * drm_crtc_enable_color_mgmt - enable color management properties
>    * @crtc: DRM CRTC
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index d1c662d92ab7..069b21d61871 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -29,7 +29,28 @@
>   struct drm_crtc;
>   struct drm_plane;
>   
> -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> +/**
> + * drm_color_lut_extract - clamp and round LUT entries
> + * @user_input: input value
> + * @bit_precision: number of bits the hw LUT supports
> + *
> + * Extract a degamma/gamma LUT value provided by user (in the form of
> + * &drm_color_lut entries) and round it to the precision supported by the
> + * hardware.
> + */
> +static inline u32 drm_color_lut_extract(u32 user_input, int bit_precision)
> +{
> +	u32 val = user_input;
> +	u32 max = 0xffff >> (16 - bit_precision);
> +
> +	/* Round only if we're not using full precision. */
> +	if (bit_precision < 16) {
> +		val += 1UL << (16 - bit_precision - 1);
> +		val >>= 16 - bit_precision;
> +	}
> +
> +	return clamp_val(val, 0, max);
> +}
>   
>   void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>   				uint degamma_lut_size,
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/12] drm: Inline drm_color_lut_extract()

[Ville Syrjälä]

On Thu, Nov 07, 2019 at 03:31:28PM +0000, Kazlauskas, Nicholas wrote:
> On 2019-11-07 10:17 a.m., Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > This thing can get called several thousand times per LUT
> > so seems like we want to inline it to:
> > - avoid the function call overhead
> > - allow constant folding
> > 
> > A quick synthetic test (w/o any hardware interaction) with
> > a ridiculously large LUT size shows about 50% reduction in
> > runtime on my HSW and BSW boxes. Slightly less with more
> > reasonable LUT size but still easily measurable in tens
> > of microseconds.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> 
> Seems reasonable to me. It would probably make sense to even split this 
> further into two functions, one for high precision and one for low 
> precision so it's purely a calculation and not hitting any branches.

Constant folding gets rid of it.

> 
> Nicholas Kazlauskas
> 
> > ---
> >   drivers/gpu/drm/drm_color_mgmt.c | 24 ------------------------
> >   include/drm/drm_color_mgmt.h     | 23 ++++++++++++++++++++++-
> >   2 files changed, 22 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > index 4ce5c6d8de99..19c5f635992a 100644
> > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > @@ -108,30 +108,6 @@
> >    * 	standard enum values supported by the DRM plane.
> >    */
> >   
> > -/**
> > - * drm_color_lut_extract - clamp and round LUT entries
> > - * @user_input: input value
> > - * @bit_precision: number of bits the hw LUT supports
> > - *
> > - * Extract a degamma/gamma LUT value provided by user (in the form of
> > - * &drm_color_lut entries) and round it to the precision supported by the
> > - * hardware.
> > - */
> > -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> > -{
> > -	uint32_t val = user_input;
> > -	uint32_t max = 0xffff >> (16 - bit_precision);
> > -
> > -	/* Round only if we're not using full precision. */
> > -	if (bit_precision < 16) {
> > -		val += 1UL << (16 - bit_precision - 1);
> > -		val >>= 16 - bit_precision;
> > -	}
> > -
> > -	return clamp_val(val, 0, max);
> > -}
> > -EXPORT_SYMBOL(drm_color_lut_extract);
> > -
> >   /**
> >    * drm_crtc_enable_color_mgmt - enable color management properties
> >    * @crtc: DRM CRTC
> > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > index d1c662d92ab7..069b21d61871 100644
> > --- a/include/drm/drm_color_mgmt.h
> > +++ b/include/drm/drm_color_mgmt.h
> > @@ -29,7 +29,28 @@
> >   struct drm_crtc;
> >   struct drm_plane;
> >   
> > -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> > +/**
> > + * drm_color_lut_extract - clamp and round LUT entries
> > + * @user_input: input value
> > + * @bit_precision: number of bits the hw LUT supports
> > + *
> > + * Extract a degamma/gamma LUT value provided by user (in the form of
> > + * &drm_color_lut entries) and round it to the precision supported by the
> > + * hardware.
> > + */
> > +static inline u32 drm_color_lut_extract(u32 user_input, int bit_precision)
> > +{
> > +	u32 val = user_input;
> > +	u32 max = 0xffff >> (16 - bit_precision);
> > +
> > +	/* Round only if we're not using full precision. */
> > +	if (bit_precision < 16) {
> > +		val += 1UL << (16 - bit_precision - 1);
> > +		val >>= 16 - bit_precision;
> > +	}
> > +
> > +	return clamp_val(val, 0, max);
> > +}
> >   
> >   void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> >   				uint degamma_lut_size,
> > 
> 

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/12] drm: Inline drm_color_lut_extract()

[Kazlauskas, Nicholas]

On 2019-11-07 10:43 a.m., Ville Syrjälä wrote:
> On Thu, Nov 07, 2019 at 03:31:28PM +0000, Kazlauskas, Nicholas wrote:
>> On 2019-11-07 10:17 a.m., Ville Syrjala wrote:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> This thing can get called several thousand times per LUT
>>> so seems like we want to inline it to:
>>> - avoid the function call overhead
>>> - allow constant folding
>>>
>>> A quick synthetic test (w/o any hardware interaction) with
>>> a ridiculously large LUT size shows about 50% reduction in
>>> runtime on my HSW and BSW boxes. Slightly less with more
>>> reasonable LUT size but still easily measurable in tens
>>> of microseconds.
>>>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>>
>> Seems reasonable to me. It would probably make sense to even split this
>> further into two functions, one for high precision and one for low
>> precision so it's purely a calculation and not hitting any branches.
> 
> Constant folding gets rid of it.

I realized after sending that email that moving this to inline is 
probably allowing the compiler to optimize this out and give you that 
large speedup in the first place. Though branch prediction probably 
helped cut down on the cost even when it wasn't inline.

This is fine as is then, thanks.

Nicholas Kazlauskas

> 
>>
>> Nicholas Kazlauskas
>>
>>> ---
>>>    drivers/gpu/drm/drm_color_mgmt.c | 24 ------------------------
>>>    include/drm/drm_color_mgmt.h     | 23 ++++++++++++++++++++++-
>>>    2 files changed, 22 insertions(+), 25 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
>>> index 4ce5c6d8de99..19c5f635992a 100644
>>> --- a/drivers/gpu/drm/drm_color_mgmt.c
>>> +++ b/drivers/gpu/drm/drm_color_mgmt.c
>>> @@ -108,30 +108,6 @@
>>>     * 	standard enum values supported by the DRM plane.
>>>     */
>>>    
>>> -/**
>>> - * drm_color_lut_extract - clamp and round LUT entries
>>> - * @user_input: input value
>>> - * @bit_precision: number of bits the hw LUT supports
>>> - *
>>> - * Extract a degamma/gamma LUT value provided by user (in the form of
>>> - * &drm_color_lut entries) and round it to the precision supported by the
>>> - * hardware.
>>> - */
>>> -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
>>> -{
>>> -	uint32_t val = user_input;
>>> -	uint32_t max = 0xffff >> (16 - bit_precision);
>>> -
>>> -	/* Round only if we're not using full precision. */
>>> -	if (bit_precision < 16) {
>>> -		val += 1UL << (16 - bit_precision - 1);
>>> -		val >>= 16 - bit_precision;
>>> -	}
>>> -
>>> -	return clamp_val(val, 0, max);
>>> -}
>>> -EXPORT_SYMBOL(drm_color_lut_extract);
>>> -
>>>    /**
>>>     * drm_crtc_enable_color_mgmt - enable color management properties
>>>     * @crtc: DRM CRTC
>>> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
>>> index d1c662d92ab7..069b21d61871 100644
>>> --- a/include/drm/drm_color_mgmt.h
>>> +++ b/include/drm/drm_color_mgmt.h
>>> @@ -29,7 +29,28 @@
>>>    struct drm_crtc;
>>>    struct drm_plane;
>>>    
>>> -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
>>> +/**
>>> + * drm_color_lut_extract - clamp and round LUT entries
>>> + * @user_input: input value
>>> + * @bit_precision: number of bits the hw LUT supports
>>> + *
>>> + * Extract a degamma/gamma LUT value provided by user (in the form of
>>> + * &drm_color_lut entries) and round it to the precision supported by the
>>> + * hardware.
>>> + */
>>> +static inline u32 drm_color_lut_extract(u32 user_input, int bit_precision)
>>> +{
>>> +	u32 val = user_input;
>>> +	u32 max = 0xffff >> (16 - bit_precision);
>>> +
>>> +	/* Round only if we're not using full precision. */
>>> +	if (bit_precision < 16) {
>>> +		val += 1UL << (16 - bit_precision - 1);
>>> +		val >>= 16 - bit_precision;
>>> +	}
>>> +
>>> +	return clamp_val(val, 0, max);
>>> +}
>>>    
>>>    void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>>>    				uint degamma_lut_size,
>>>
>>
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 01/12] drm: Inline drm_color_lut_extract()

[Daniel Vetter]

On Thu, Nov 07, 2019 at 05:17:14PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> This thing can get called several thousand times per LUT
> so seems like we want to inline it to:
> - avoid the function call overhead
> - allow constant folding
> 
> A quick synthetic test (w/o any hardware interaction) with
> a ridiculously large LUT size shows about 50% reduction in
> runtime on my HSW and BSW boxes. Slightly less with more
> reasonable LUT size but still easily measurable in tens
> of microseconds.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_color_mgmt.c | 24 ------------------------
>  include/drm/drm_color_mgmt.h     | 23 ++++++++++++++++++++++-

You forgot to add the include stanza in the kerneldoc .rst files, which
means this is now lost from the output. Please fix.
-Daniel

>  2 files changed, 22 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> index 4ce5c6d8de99..19c5f635992a 100644
> --- a/drivers/gpu/drm/drm_color_mgmt.c
> +++ b/drivers/gpu/drm/drm_color_mgmt.c
> @@ -108,30 +108,6 @@
>   * 	standard enum values supported by the DRM plane.
>   */
>  
> -/**
> - * drm_color_lut_extract - clamp and round LUT entries
> - * @user_input: input value
> - * @bit_precision: number of bits the hw LUT supports
> - *
> - * Extract a degamma/gamma LUT value provided by user (in the form of
> - * &drm_color_lut entries) and round it to the precision supported by the
> - * hardware.
> - */
> -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> -{
> -	uint32_t val = user_input;
> -	uint32_t max = 0xffff >> (16 - bit_precision);
> -
> -	/* Round only if we're not using full precision. */
> -	if (bit_precision < 16) {
> -		val += 1UL << (16 - bit_precision - 1);
> -		val >>= 16 - bit_precision;
> -	}
> -
> -	return clamp_val(val, 0, max);
> -}
> -EXPORT_SYMBOL(drm_color_lut_extract);
> -
>  /**
>   * drm_crtc_enable_color_mgmt - enable color management properties
>   * @crtc: DRM CRTC
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index d1c662d92ab7..069b21d61871 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -29,7 +29,28 @@
>  struct drm_crtc;
>  struct drm_plane;
>  
> -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> +/**
> + * drm_color_lut_extract - clamp and round LUT entries
> + * @user_input: input value
> + * @bit_precision: number of bits the hw LUT supports
> + *
> + * Extract a degamma/gamma LUT value provided by user (in the form of
> + * &drm_color_lut entries) and round it to the precision supported by the
> + * hardware.
> + */
> +static inline u32 drm_color_lut_extract(u32 user_input, int bit_precision)
> +{
> +	u32 val = user_input;
> +	u32 max = 0xffff >> (16 - bit_precision);
> +
> +	/* Round only if we're not using full precision. */
> +	if (bit_precision < 16) {
> +		val += 1UL << (16 - bit_precision - 1);
> +		val >>= 16 - bit_precision;
> +	}
> +
> +	return clamp_val(val, 0, max);
> +}
>  
>  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
>  				uint degamma_lut_size,
> -- 
> 2.23.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/12] drm: Inline drm_color_lut_extract()

[Ville Syrjälä]

On Thu, Nov 07, 2019 at 06:40:14PM +0100, Daniel Vetter wrote:
> On Thu, Nov 07, 2019 at 05:17:14PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > This thing can get called several thousand times per LUT
> > so seems like we want to inline it to:
> > - avoid the function call overhead
> > - allow constant folding
> > 
> > A quick synthetic test (w/o any hardware interaction) with
> > a ridiculously large LUT size shows about 50% reduction in
> > runtime on my HSW and BSW boxes. Slightly less with more
> > reasonable LUT size but still easily measurable in tens
> > of microseconds.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_color_mgmt.c | 24 ------------------------
> >  include/drm/drm_color_mgmt.h     | 23 ++++++++++++++++++++++-
> 
> You forgot to add the include stanza in the kerneldoc .rst files, which
> means this is now lost from the output. Please fix.

Aye. A bit funny that we already have a bunch of other kerneldocs
in that header but it's not included in the .rst.

> -Daniel
> 
> >  2 files changed, 22 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > index 4ce5c6d8de99..19c5f635992a 100644
> > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > @@ -108,30 +108,6 @@
> >   * 	standard enum values supported by the DRM plane.
> >   */
> >  
> > -/**
> > - * drm_color_lut_extract - clamp and round LUT entries
> > - * @user_input: input value
> > - * @bit_precision: number of bits the hw LUT supports
> > - *
> > - * Extract a degamma/gamma LUT value provided by user (in the form of
> > - * &drm_color_lut entries) and round it to the precision supported by the
> > - * hardware.
> > - */
> > -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> > -{
> > -	uint32_t val = user_input;
> > -	uint32_t max = 0xffff >> (16 - bit_precision);
> > -
> > -	/* Round only if we're not using full precision. */
> > -	if (bit_precision < 16) {
> > -		val += 1UL << (16 - bit_precision - 1);
> > -		val >>= 16 - bit_precision;
> > -	}
> > -
> > -	return clamp_val(val, 0, max);
> > -}
> > -EXPORT_SYMBOL(drm_color_lut_extract);
> > -
> >  /**
> >   * drm_crtc_enable_color_mgmt - enable color management properties
> >   * @crtc: DRM CRTC
> > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > index d1c662d92ab7..069b21d61871 100644
> > --- a/include/drm/drm_color_mgmt.h
> > +++ b/include/drm/drm_color_mgmt.h
> > @@ -29,7 +29,28 @@
> >  struct drm_crtc;
> >  struct drm_plane;
> >  
> > -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> > +/**
> > + * drm_color_lut_extract - clamp and round LUT entries
> > + * @user_input: input value
> > + * @bit_precision: number of bits the hw LUT supports
> > + *
> > + * Extract a degamma/gamma LUT value provided by user (in the form of
> > + * &drm_color_lut entries) and round it to the precision supported by the
> > + * hardware.
> > + */
> > +static inline u32 drm_color_lut_extract(u32 user_input, int bit_precision)
> > +{
> > +	u32 val = user_input;
> > +	u32 max = 0xffff >> (16 - bit_precision);
> > +
> > +	/* Round only if we're not using full precision. */
> > +	if (bit_precision < 16) {
> > +		val += 1UL << (16 - bit_precision - 1);
> > +		val >>= 16 - bit_precision;
> > +	}
> > +
> > +	return clamp_val(val, 0, max);
> > +}
> >  
> >  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> >  				uint degamma_lut_size,
> > -- 
> > 2.23.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/12] drm: Inline drm_color_lut_extract()

[Ville Syrjälä]

On Thu, Nov 07, 2019 at 06:40:14PM +0100, Daniel Vetter wrote:
> On Thu, Nov 07, 2019 at 05:17:14PM +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > This thing can get called several thousand times per LUT
> > so seems like we want to inline it to:
> > - avoid the function call overhead
> > - allow constant folding
> > 
> > A quick synthetic test (w/o any hardware interaction) with
> > a ridiculously large LUT size shows about 50% reduction in
> > runtime on my HSW and BSW boxes. Slightly less with more
> > reasonable LUT size but still easily measurable in tens
> > of microseconds.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_color_mgmt.c | 24 ------------------------
> >  include/drm/drm_color_mgmt.h     | 23 ++++++++++++++++++++++-
> 
> You forgot to add the include stanza in the kerneldoc .rst files, which
> means this is now lost from the output. Please fix.

Aye. A bit funny that we already have a bunch of other kerneldocs
in that header but it's not included in the .rst.

> -Daniel
> 
> >  2 files changed, 22 insertions(+), 25 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > index 4ce5c6d8de99..19c5f635992a 100644
> > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > @@ -108,30 +108,6 @@
> >   * 	standard enum values supported by the DRM plane.
> >   */
> >  
> > -/**
> > - * drm_color_lut_extract - clamp and round LUT entries
> > - * @user_input: input value
> > - * @bit_precision: number of bits the hw LUT supports
> > - *
> > - * Extract a degamma/gamma LUT value provided by user (in the form of
> > - * &drm_color_lut entries) and round it to the precision supported by the
> > - * hardware.
> > - */
> > -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> > -{
> > -	uint32_t val = user_input;
> > -	uint32_t max = 0xffff >> (16 - bit_precision);
> > -
> > -	/* Round only if we're not using full precision. */
> > -	if (bit_precision < 16) {
> > -		val += 1UL << (16 - bit_precision - 1);
> > -		val >>= 16 - bit_precision;
> > -	}
> > -
> > -	return clamp_val(val, 0, max);
> > -}
> > -EXPORT_SYMBOL(drm_color_lut_extract);
> > -
> >  /**
> >   * drm_crtc_enable_color_mgmt - enable color management properties
> >   * @crtc: DRM CRTC
> > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > index d1c662d92ab7..069b21d61871 100644
> > --- a/include/drm/drm_color_mgmt.h
> > +++ b/include/drm/drm_color_mgmt.h
> > @@ -29,7 +29,28 @@
> >  struct drm_crtc;
> >  struct drm_plane;
> >  
> > -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> > +/**
> > + * drm_color_lut_extract - clamp and round LUT entries
> > + * @user_input: input value
> > + * @bit_precision: number of bits the hw LUT supports
> > + *
> > + * Extract a degamma/gamma LUT value provided by user (in the form of
> > + * &drm_color_lut entries) and round it to the precision supported by the
> > + * hardware.
> > + */
> > +static inline u32 drm_color_lut_extract(u32 user_input, int bit_precision)
> > +{
> > +	u32 val = user_input;
> > +	u32 max = 0xffff >> (16 - bit_precision);
> > +
> > +	/* Round only if we're not using full precision. */
> > +	if (bit_precision < 16) {
> > +		val += 1UL << (16 - bit_precision - 1);
> > +		val >>= 16 - bit_precision;
> > +	}
> > +
> > +	return clamp_val(val, 0, max);
> > +}
> >  
> >  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> >  				uint degamma_lut_size,
> > -- 
> > 2.23.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 01/12] drm: Inline drm_color_lut_extract()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

This thing can get called several thousand times per LUT
so seems like we want to inline it to:
- avoid the function call overhead
- allow constant folding

A quick synthetic test (w/o any hardware interaction) with
a ridiculously large LUT size shows about 50% reduction in
runtime on my HSW and BSW boxes. Slightly less with more
reasonable LUT size but still easily measurable in tens
of microseconds.

v2: Include drm_color_mgmt.h in the .rst (Daniel)

Cc: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
---
 Documentation/gpu/drm-kms.rst    |  3 +++
 drivers/gpu/drm/drm_color_mgmt.c | 24 ------------------------
 include/drm/drm_color_mgmt.h     | 23 ++++++++++++++++++++++-
 3 files changed, 25 insertions(+), 25 deletions(-)

diff --git a/Documentation/gpu/drm-kms.rst b/Documentation/gpu/drm-kms.rst
index 23a3c986ef6d..c68588ce4090 100644
--- a/Documentation/gpu/drm-kms.rst
+++ b/Documentation/gpu/drm-kms.rst
@@ -479,6 +479,9 @@ Color Management Properties
 .. kernel-doc:: drivers/gpu/drm/drm_color_mgmt.c
    :export:
 
+.. kernel-doc:: include/drm/drm_color_mgmt.h
+   :internal:
+
 Tile Group Property
 -------------------
 
diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
index 4ce5c6d8de99..19c5f635992a 100644
--- a/drivers/gpu/drm/drm_color_mgmt.c
+++ b/drivers/gpu/drm/drm_color_mgmt.c
@@ -108,30 +108,6 @@
  * 	standard enum values supported by the DRM plane.
  */
 
-/**
- * drm_color_lut_extract - clamp and round LUT entries
- * @user_input: input value
- * @bit_precision: number of bits the hw LUT supports
- *
- * Extract a degamma/gamma LUT value provided by user (in the form of
- * &drm_color_lut entries) and round it to the precision supported by the
- * hardware.
- */
-uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
-{
-	uint32_t val = user_input;
-	uint32_t max = 0xffff >> (16 - bit_precision);
-
-	/* Round only if we're not using full precision. */
-	if (bit_precision < 16) {
-		val += 1UL << (16 - bit_precision - 1);
-		val >>= 16 - bit_precision;
-	}
-
-	return clamp_val(val, 0, max);
-}
-EXPORT_SYMBOL(drm_color_lut_extract);
-
 /**
  * drm_crtc_enable_color_mgmt - enable color management properties
  * @crtc: DRM CRTC
diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
index d1c662d92ab7..069b21d61871 100644
--- a/include/drm/drm_color_mgmt.h
+++ b/include/drm/drm_color_mgmt.h
@@ -29,7 +29,28 @@
 struct drm_crtc;
 struct drm_plane;
 
-uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
+/**
+ * drm_color_lut_extract - clamp and round LUT entries
+ * @user_input: input value
+ * @bit_precision: number of bits the hw LUT supports
+ *
+ * Extract a degamma/gamma LUT value provided by user (in the form of
+ * &drm_color_lut entries) and round it to the precision supported by the
+ * hardware.
+ */
+static inline u32 drm_color_lut_extract(u32 user_input, int bit_precision)
+{
+	u32 val = user_input;
+	u32 max = 0xffff >> (16 - bit_precision);
+
+	/* Round only if we're not using full precision. */
+	if (bit_precision < 16) {
+		val += 1UL << (16 - bit_precision - 1);
+		val >>= 16 - bit_precision;
+	}
+
+	return clamp_val(val, 0, max);
+}
 
 void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
 				uint degamma_lut_size,
-- 
2.23.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/12] drm: Inline drm_color_lut_extract()

[Daniel Vetter]

On Fri, Nov 08, 2019 at 03:36:57PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 07, 2019 at 06:40:14PM +0100, Daniel Vetter wrote:
> > On Thu, Nov 07, 2019 at 05:17:14PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > This thing can get called several thousand times per LUT
> > > so seems like we want to inline it to:
> > > - avoid the function call overhead
> > > - allow constant folding
> > > 
> > > A quick synthetic test (w/o any hardware interaction) with
> > > a ridiculously large LUT size shows about 50% reduction in
> > > runtime on my HSW and BSW boxes. Slightly less with more
> > > reasonable LUT size but still easily measurable in tens
> > > of microseconds.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_color_mgmt.c | 24 ------------------------
> > >  include/drm/drm_color_mgmt.h     | 23 ++++++++++++++++++++++-
> > 
> > You forgot to add the include stanza in the kerneldoc .rst files, which
> > means this is now lost from the output. Please fix.
> 
> Aye. A bit funny that we already have a bunch of other kerneldocs
> in that header but it's not included in the .rst.

kerneldoc complains if there's no kerneldoc, which is often the case for
headers. So we start out without the header included, and then someone
misses that when adding the first/second/... kerneldoc.

It's rather annoying unfortunately :-/
-Daniel

> 
> > -Daniel
> > 
> > >  2 files changed, 22 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > > index 4ce5c6d8de99..19c5f635992a 100644
> > > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > > @@ -108,30 +108,6 @@
> > >   * 	standard enum values supported by the DRM plane.
> > >   */
> > >  
> > > -/**
> > > - * drm_color_lut_extract - clamp and round LUT entries
> > > - * @user_input: input value
> > > - * @bit_precision: number of bits the hw LUT supports
> > > - *
> > > - * Extract a degamma/gamma LUT value provided by user (in the form of
> > > - * &drm_color_lut entries) and round it to the precision supported by the
> > > - * hardware.
> > > - */
> > > -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> > > -{
> > > -	uint32_t val = user_input;
> > > -	uint32_t max = 0xffff >> (16 - bit_precision);
> > > -
> > > -	/* Round only if we're not using full precision. */
> > > -	if (bit_precision < 16) {
> > > -		val += 1UL << (16 - bit_precision - 1);
> > > -		val >>= 16 - bit_precision;
> > > -	}
> > > -
> > > -	return clamp_val(val, 0, max);
> > > -}
> > > -EXPORT_SYMBOL(drm_color_lut_extract);
> > > -
> > >  /**
> > >   * drm_crtc_enable_color_mgmt - enable color management properties
> > >   * @crtc: DRM CRTC
> > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > > index d1c662d92ab7..069b21d61871 100644
> > > --- a/include/drm/drm_color_mgmt.h
> > > +++ b/include/drm/drm_color_mgmt.h
> > > @@ -29,7 +29,28 @@
> > >  struct drm_crtc;
> > >  struct drm_plane;
> > >  
> > > -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> > > +/**
> > > + * drm_color_lut_extract - clamp and round LUT entries
> > > + * @user_input: input value
> > > + * @bit_precision: number of bits the hw LUT supports
> > > + *
> > > + * Extract a degamma/gamma LUT value provided by user (in the form of
> > > + * &drm_color_lut entries) and round it to the precision supported by the
> > > + * hardware.
> > > + */
> > > +static inline u32 drm_color_lut_extract(u32 user_input, int bit_precision)
> > > +{
> > > +	u32 val = user_input;
> > > +	u32 max = 0xffff >> (16 - bit_precision);
> > > +
> > > +	/* Round only if we're not using full precision. */
> > > +	if (bit_precision < 16) {
> > > +		val += 1UL << (16 - bit_precision - 1);
> > > +		val >>= 16 - bit_precision;
> > > +	}
> > > +
> > > +	return clamp_val(val, 0, max);
> > > +}
> > >  
> > >  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> > >  				uint degamma_lut_size,
> > > -- 
> > > 2.23.0
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 01/12] drm: Inline drm_color_lut_extract()

[Daniel Vetter]

On Fri, Nov 08, 2019 at 03:36:57PM +0200, Ville Syrjälä wrote:
> On Thu, Nov 07, 2019 at 06:40:14PM +0100, Daniel Vetter wrote:
> > On Thu, Nov 07, 2019 at 05:17:14PM +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > This thing can get called several thousand times per LUT
> > > so seems like we want to inline it to:
> > > - avoid the function call overhead
> > > - allow constant folding
> > > 
> > > A quick synthetic test (w/o any hardware interaction) with
> > > a ridiculously large LUT size shows about 50% reduction in
> > > runtime on my HSW and BSW boxes. Slightly less with more
> > > reasonable LUT size but still easily measurable in tens
> > > of microseconds.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_color_mgmt.c | 24 ------------------------
> > >  include/drm/drm_color_mgmt.h     | 23 ++++++++++++++++++++++-
> > 
> > You forgot to add the include stanza in the kerneldoc .rst files, which
> > means this is now lost from the output. Please fix.
> 
> Aye. A bit funny that we already have a bunch of other kerneldocs
> in that header but it's not included in the .rst.

kerneldoc complains if there's no kerneldoc, which is often the case for
headers. So we start out without the header included, and then someone
misses that when adding the first/second/... kerneldoc.

It's rather annoying unfortunately :-/
-Daniel

> 
> > -Daniel
> > 
> > >  2 files changed, 22 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_color_mgmt.c b/drivers/gpu/drm/drm_color_mgmt.c
> > > index 4ce5c6d8de99..19c5f635992a 100644
> > > --- a/drivers/gpu/drm/drm_color_mgmt.c
> > > +++ b/drivers/gpu/drm/drm_color_mgmt.c
> > > @@ -108,30 +108,6 @@
> > >   * 	standard enum values supported by the DRM plane.
> > >   */
> > >  
> > > -/**
> > > - * drm_color_lut_extract - clamp and round LUT entries
> > > - * @user_input: input value
> > > - * @bit_precision: number of bits the hw LUT supports
> > > - *
> > > - * Extract a degamma/gamma LUT value provided by user (in the form of
> > > - * &drm_color_lut entries) and round it to the precision supported by the
> > > - * hardware.
> > > - */
> > > -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision)
> > > -{
> > > -	uint32_t val = user_input;
> > > -	uint32_t max = 0xffff >> (16 - bit_precision);
> > > -
> > > -	/* Round only if we're not using full precision. */
> > > -	if (bit_precision < 16) {
> > > -		val += 1UL << (16 - bit_precision - 1);
> > > -		val >>= 16 - bit_precision;
> > > -	}
> > > -
> > > -	return clamp_val(val, 0, max);
> > > -}
> > > -EXPORT_SYMBOL(drm_color_lut_extract);
> > > -
> > >  /**
> > >   * drm_crtc_enable_color_mgmt - enable color management properties
> > >   * @crtc: DRM CRTC
> > > diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> > > index d1c662d92ab7..069b21d61871 100644
> > > --- a/include/drm/drm_color_mgmt.h
> > > +++ b/include/drm/drm_color_mgmt.h
> > > @@ -29,7 +29,28 @@
> > >  struct drm_crtc;
> > >  struct drm_plane;
> > >  
> > > -uint32_t drm_color_lut_extract(uint32_t user_input, uint32_t bit_precision);
> > > +/**
> > > + * drm_color_lut_extract - clamp and round LUT entries
> > > + * @user_input: input value
> > > + * @bit_precision: number of bits the hw LUT supports
> > > + *
> > > + * Extract a degamma/gamma LUT value provided by user (in the form of
> > > + * &drm_color_lut entries) and round it to the precision supported by the
> > > + * hardware.
> > > + */
> > > +static inline u32 drm_color_lut_extract(u32 user_input, int bit_precision)
> > > +{
> > > +	u32 val = user_input;
> > > +	u32 max = 0xffff >> (16 - bit_precision);
> > > +
> > > +	/* Round only if we're not using full precision. */
> > > +	if (bit_precision < 16) {
> > > +		val += 1UL << (16 - bit_precision - 1);
> > > +		val >>= 16 - bit_precision;
> > > +	}
> > > +
> > > +	return clamp_val(val, 0, max);
> > > +}
> > >  
> > >  void drm_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> > >  				uint degamma_lut_size,
> > > -- 
> > > 2.23.0
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Ville Syrjälä
> Intel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 04/12] drm/i915: Add i9xx_lut_8()

[Emil Velikov]

On Thu, 7 Nov 2019 at 15:17, Ville Syrjala
<ville.syrjala@linux.intel.com> wrote:
>
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> We have a nice little helper to compute a single LUT entry
> for everything except the 8bpc legacy gamma mode. Let's
> complete the set.
>
At a later stage one could rename this & the 10bit one, moving them to
include/drm/.
There are other drivers doing the same thing... not sure if that's
worth it though.

Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 04/12] drm/i915: Add i9xx_lut_8()

[Ville Syrjälä]

On Thu, Feb 20, 2020 at 11:20:05AM +0000, Emil Velikov wrote:
> On Thu, 7 Nov 2019 at 15:17, Ville Syrjala
> <ville.syrjala@linux.intel.com> wrote:
> >
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > We have a nice little helper to compute a single LUT entry
> > for everything except the 8bpc legacy gamma mode. Let's
> > complete the set.
> >
> At a later stage one could rename this & the 10bit one, moving them to
> include/drm/.
> There are other drivers doing the same thing... not sure if that's
> worth it though.

I'd say no. These are specifically about formatting the LUT entry for
the hw register. I don't really see much benefit from sharing code to
compute hw register values across totally different hardware, even if
the bits happen to match by accident.

The only good exception I can think of are cases where said 
register value comes more or less straight from some cross
vendor spec.

-- 
Ville Syrjälä
Intel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/12] drm/i915: Polish CHV .load_luts() a bit

[Sharma, Swati2]

Hi Ville,
Can you please rebase the series? There are intel_de_write()
changes in existing code.

On 07-Nov-19 8:47 PM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> It irks me to use crtc_state_is_legacy_gamma() inside the guts
> of the CHV color management code. Let's get rid of it and instead
> just consult cgm_mode to figure out if we want to enable the pipe
> gamma or the CGM gamma.
> 
> Also CHV display engine is based on i965/g4x so we should fall back
> to the i965 path when the CGM gamma is not used.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_color.c | 11 ++++-------
>   1 file changed, 4 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index 3980e8b50c28..d8ee90b7774a 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -996,16 +996,13 @@ static void chv_load_luts(const struct intel_crtc_state *crtc_state)
>   
>   	cherryview_load_csc_matrix(crtc_state);
>   
> -	if (crtc_state_is_legacy_gamma(crtc_state)) {
> -		i9xx_load_luts(crtc_state);
> -		return;
> -	}
> -
> -	if (degamma_lut)
> +	if (crtc_state->cgm_mode & CGM_PIPE_MODE_DEGAMMA)
>   		chv_load_cgm_degamma(crtc, degamma_lut);
>   
> -	if (gamma_lut)
> +	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
>   		chv_load_cgm_gamma(crtc, gamma_lut);
> +	else
> +		i965_load_luts(crtc_state);
>   }
>   
>   void intel_color_load_luts(const struct intel_crtc_state *crtc_state)
> 
Reviewed-by: Swati Sharma <swati2.sharma@intel.com>

-- 
~Swati Sharma
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel