[PATCH v2 0/7] drm/i915: Finish the GAMMA_LUT stuff

[PATCH v2 0/7] drm/i915: Finish the GAMMA_LUT stuff

[Ville Syrjala]

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

Rebased due to Uma's EXT_GC_MAX fix, and I added Matt's proposed
behavioural change (expose 1024 entry LUTs in split gamma mode
and just discard half the entries) as an extra patch on top.

Everything is reviewed except patches 2 and 7.

Ville Syrjälä (7):
  drm/i915: Extract ilk_lut_10()
  drm/i915: Don't use split gamma when we don't have to
  drm/i915: Implement split/10bit gamma for ivb/hsw
  drm/i915: Add 10bit LUT for ilk/snb
  drm/i915: Add "10.6" LUT mode for i965+
  drm/i915: Expose the legacy LUT via the GAMMA_LUT/GAMMA_LUT_SIZE props
    on gen2/3
  drm/i915: Expose full 1024 LUT entries on ivb+

 drivers/gpu/drm/i915/i915_pci.c    |  23 +-
 drivers/gpu/drm/i915/i915_reg.h    |  15 ++
 drivers/gpu/drm/i915/intel_color.c | 375 ++++++++++++++++++++---------
 3 files changed, 292 insertions(+), 121 deletions(-)

-- 
2.19.2

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

[PATCH v2 1/7] drm/i915: Extract ilk_lut_10()

[Ville Syrjala]

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

Extract a helper to calculate the ILK+ 10bit gamma LUT entry.
It's already duplicated twice, and soon we'll have more.

v2: s/it/bit/ (Matt)

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_color.c | 27 +++++++++++----------------
 1 file changed, 11 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index f2907cfd116a..d5b3060c2645 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -359,6 +359,13 @@ static void cherryview_load_csc_matrix(const struct intel_crtc_state *crtc_state
 	I915_WRITE(CGM_PIPE_MODE(pipe), crtc_state->cgm_mode);
 }
 
+static u32 ilk_lut_10(const struct drm_color_lut *color)
+{
+	return drm_color_lut_extract(color->red, 10) << 20 |
+		drm_color_lut_extract(color->green, 10) << 10 |
+		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)
@@ -473,14 +480,8 @@ static void bdw_load_degamma_lut(const struct intel_crtc_state *crtc_state)
 	if (degamma_lut) {
 		const struct drm_color_lut *lut = degamma_lut->data;
 
-		for (i = 0; i < lut_size; i++) {
-			u32 word =
-			drm_color_lut_extract(lut[i].red, 10) << 20 |
-			drm_color_lut_extract(lut[i].green, 10) << 10 |
-			drm_color_lut_extract(lut[i].blue, 10);
-
-			I915_WRITE(PREC_PAL_DATA(pipe), word);
-		}
+		for (i = 0; i < lut_size; i++)
+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
 	} else {
 		for (i = 0; i < lut_size; i++) {
 			u32 v = (i * ((1 << 10) - 1)) / (lut_size - 1);
@@ -509,14 +510,8 @@ static void bdw_load_gamma_lut(const struct intel_crtc_state *crtc_state, u32 of
 	if (gamma_lut) {
 		const struct drm_color_lut *lut = gamma_lut->data;
 
-		for (i = 0; i < lut_size; i++) {
-			u32 word =
-			(drm_color_lut_extract(lut[i].red, 10) << 20) |
-			(drm_color_lut_extract(lut[i].green, 10) << 10) |
-			drm_color_lut_extract(lut[i].blue, 10);
-
-			I915_WRITE(PREC_PAL_DATA(pipe), word);
-		}
+		for (i = 0; i < lut_size; i++)
+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
 
 		/*
 		 * Program the max register to clamp values > 1.0.
-- 
2.19.2

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

[PATCH v2 2/7] drm/i915: Don't use split gamma when we don't have to

[Ville Syrjala]

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

Using the split gamma mode when we don't have to has the annoying
requirement of loading a linear LUT to the unused half. Instead
let's make life simpler by switching to the 10bit gamma mode
and duplicating each entry.

This also allows us to load the software gamma LUT into the
hardware degamma LUT, thus removing some of the buggy
configurations we currently allow (YCbCr/limited range RGB
+ gamma LUT). We do still have other configurations that are
also buggy, but those will need more complicated fixes
or they just need to be rejected. Sadly GLK doesn't have
this flexibility anymore and the degamma and gamma LUTs
are very different so no help there.

v2: Apply a mask when checking gamma_mode on icl since it
    contains more bits than just the gamma mode
v3: Rebase due to EXT_GC_MAX/EXT2_GC_MAX changes

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 341f03e00536..bed2c52aebd8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7214,6 +7214,7 @@ enum {
 #define GAMMA_MODE(pipe) _MMIO_PIPE(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B)
 #define  PRE_CSC_GAMMA_ENABLE	(1 << 31)
 #define  POST_CSC_GAMMA_ENABLE	(1 << 30)
+#define  GAMMA_MODE_MODE_MASK	(3 << 0)
 #define  GAMMA_MODE_MODE_8BIT	(0 << 0)
 #define  GAMMA_MODE_MODE_10BIT	(1 << 0)
 #define  GAMMA_MODE_MODE_12BIT	(2 << 0)
@@ -10127,6 +10128,7 @@ enum skl_power_gate {
 #define   PAL_PREC_SPLIT_MODE		(1 << 31)
 #define   PAL_PREC_AUTO_INCREMENT	(1 << 15)
 #define   PAL_PREC_INDEX_VALUE_MASK	(0x3ff << 0)
+#define   PAL_PREC_INDEX_VALUE(x)	((x) << 0)
 #define _PAL_PREC_DATA_A	0x4A404
 #define _PAL_PREC_DATA_B	0x4AC04
 #define _PAL_PREC_DATA_C	0x4B404
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index d5b3060c2645..5ef93c43afcf 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -466,115 +466,83 @@ static void skl_color_commit(const struct intel_crtc_state *crtc_state)
 		ilk_load_csc_matrix(crtc_state);
 }
 
-static void bdw_load_degamma_lut(const struct intel_crtc_state *crtc_state)
+static void bdw_load_lut_10(struct intel_crtc *crtc,
+			    const struct drm_property_blob *blob,
+			    u32 prec_index, bool duplicate)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	const struct drm_property_blob *degamma_lut = crtc_state->base.degamma_lut;
-	u32 i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
+	const struct drm_color_lut *lut = blob->data;
+	int i, lut_size = drm_color_lut_size(blob);
 	enum pipe pipe = crtc->pipe;
 
-	I915_WRITE(PREC_PAL_INDEX(pipe),
-		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT);
-
-	if (degamma_lut) {
-		const struct drm_color_lut *lut = degamma_lut->data;
+	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
+		   PAL_PREC_AUTO_INCREMENT);
 
-		for (i = 0; i < lut_size; i++)
-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
-	} else {
+	/*
+	 * We advertize the split gamma sizes. When not using split
+	 * gamma we just duplicate each entry.
+	 *
+	 * TODO: expose the full LUT to userspace
+	 */
+	if (duplicate) {
 		for (i = 0; i < lut_size; i++) {
-			u32 v = (i * ((1 << 10) - 1)) / (lut_size - 1);
-
-			I915_WRITE(PREC_PAL_DATA(pipe),
-				   (v << 20) | (v << 10) | v);
+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
 		}
+	} else {
+		for (i = 0; i < lut_size; i++)
+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
 	}
+
+	/*
+	 * Reset the index, otherwise it prevents the legacy palette to be
+	 * written properly.
+	 */
+	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
 }
 
-static void bdw_load_gamma_lut(const struct intel_crtc_state *crtc_state, u32 offset)
+static void bdw_load_lut_10_max(struct intel_crtc *crtc)
 {
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
-	u32 i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
 	enum pipe pipe = crtc->pipe;
 
-	WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
-
-	I915_WRITE(PREC_PAL_INDEX(pipe),
-		   (offset ? PAL_PREC_SPLIT_MODE : 0) |
-		   PAL_PREC_AUTO_INCREMENT |
-		   offset);
-
-	if (gamma_lut) {
-		const struct drm_color_lut *lut = gamma_lut->data;
-
-		for (i = 0; i < lut_size; i++)
-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
-
-		/*
-		 * Program the max register to clamp values > 1.0.
-		 * ToDo: Extend the ABI to be able to program values
-		 * from 1.0 to 3.0
-		 */
-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), (1 << 16));
-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), (1 << 16));
-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), (1 << 16));
-
-		/*
-		 * Program the gc max 2 register to clamp values > 1.0.
-		 * ToDo: Extend the ABI to be able to program values
-		 * from 3.0 to 7.0
-		 */
-		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), (1 << 16));
-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), (1 << 16));
-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), (1 << 16));
-		}
-	} else {
-		for (i = 0; i < lut_size; i++) {
-			u32 v = (i * ((1 << 10) - 1)) / (lut_size - 1);
-
-			I915_WRITE(PREC_PAL_DATA(pipe),
-				   (v << 20) | (v << 10) | v);
-		}
-
-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), (1 << 16));
-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), (1 << 16));
-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), (1 << 16));
-
-		/*
-		 * Program the gc max 2 register to clamp values > 1.0.
-		 * ToDo: Extend the ABI to be able to program values
-		 * from 3.0 to 7.0
-		 */
-		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), (1 << 16));
-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), (1 << 16));
-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), (1 << 16));
-		}
-	}
+	/* Program the max register to clamp values > 1.0. */
+	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), 1 << 16);
+	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), 1 << 16);
+	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), 1 << 16);
 
 	/*
-	 * Reset the index, otherwise it prevents the legacy palette to be
-	 * written properly.
+	 * Program the gc max 2 register to clamp values > 1.0.
+	 * ToDo: Extend the ABI to be able to program values
+	 * from 3.0 to 7.0
 	 */
-	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
+	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
+		I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), 1 << 16);
+		I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), 1 << 16);
+		I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), 1 << 16);
+	}
 }
 
-/* Loads the palette/gamma unit for the CRTC on Broadwell+. */
-static void broadwell_load_luts(const struct intel_crtc_state *crtc_state)
+static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
+	const struct drm_property_blob *degamma_lut = crtc_state->base.degamma_lut;
 
-	if (crtc_state_is_legacy_gamma(crtc_state)) {
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
 		i9xx_load_luts(crtc_state);
+	} 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), false);
+		bdw_load_lut_10_max(crtc);
+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
+				PAL_PREC_INDEX_VALUE(512),  false);
 	} else {
-		bdw_load_degamma_lut(crtc_state);
-		bdw_load_gamma_lut(crtc_state,
-				   INTEL_INFO(dev_priv)->color.degamma_lut_size);
+		const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
+
+		bdw_load_lut_10(crtc, blob,
+				PAL_PREC_INDEX_VALUE(0), true);
+		bdw_load_lut_10_max(crtc);
 	}
 }
 
@@ -646,6 +614,9 @@ static void glk_load_degamma_lut_linear(const struct intel_crtc_state *crtc_stat
 
 static void glk_load_luts(const struct intel_crtc_state *crtc_state)
 {
+	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+
 	/*
 	 * On GLK+ both pipe CSC and degamma LUT are controlled
 	 * by csc_enable. Hence for the cases where the CSC is
@@ -659,22 +630,29 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
 	else
 		glk_load_degamma_lut_linear(crtc_state);
 
-	if (crtc_state_is_legacy_gamma(crtc_state))
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
 		i9xx_load_luts(crtc_state);
-	else
-		bdw_load_gamma_lut(crtc_state, 0);
+	} else {
+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0), false);
+		bdw_load_lut_10_max(crtc);
+	}
 }
 
 static void icl_load_luts(const struct intel_crtc_state *crtc_state)
 {
+	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+
 	if (crtc_state->base.degamma_lut)
 		glk_load_degamma_lut(crtc_state);
 
-	if (crtc_state_is_legacy_gamma(crtc_state))
+	if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
+	    GAMMA_MODE_MODE_8BIT) {
 		i9xx_load_luts(crtc_state);
-	else
-		/* ToDo: Add support for multi segment gamma LUT */
-		bdw_load_gamma_lut(crtc_state, 0);
+	} else {
+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0), false);
+		bdw_load_lut_10_max(crtc);
+	}
 }
 
 static void cherryview_load_luts(const struct intel_crtc_state *crtc_state)
@@ -959,8 +937,25 @@ static u32 bdw_gamma_mode(const struct intel_crtc_state *crtc_state)
 	if (!crtc_state->gamma_enable ||
 	    crtc_state_is_legacy_gamma(crtc_state))
 		return GAMMA_MODE_MODE_8BIT;
-	else
+	else if (crtc_state->base.gamma_lut &&
+		 crtc_state->base.degamma_lut)
 		return GAMMA_MODE_MODE_SPLIT;
+	else
+		return GAMMA_MODE_MODE_10BIT;
+}
+
+static u32 bdw_csc_mode(const struct intel_crtc_state *crtc_state)
+{
+	/*
+	 * CSC comes after the LUT in degamma, RGB->YCbCr,
+	 * and RGB full->limited range mode.
+	 */
+	if (crtc_state->base.degamma_lut ||
+	    crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
+	    crtc_state->limited_color_range)
+		return 0;
+
+	return CSC_POSITION_BEFORE_GAMMA;
 }
 
 static int bdw_color_check(struct intel_crtc_state *crtc_state)
@@ -982,7 +977,7 @@ static int bdw_color_check(struct intel_crtc_state *crtc_state)
 
 	crtc_state->gamma_mode = bdw_gamma_mode(crtc_state);
 
-	crtc_state->csc_mode = 0;
+	crtc_state->csc_mode = bdw_csc_mode(crtc_state);
 
 	ret = intel_color_add_affected_planes(crtc_state);
 	if (ret)
@@ -1116,7 +1111,7 @@ void intel_color_init(struct intel_crtc *crtc)
 		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
 			dev_priv->display.load_luts = glk_load_luts;
 		else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
-			dev_priv->display.load_luts = broadwell_load_luts;
+			dev_priv->display.load_luts = bdw_load_luts;
 		else
 			dev_priv->display.load_luts = i9xx_load_luts;
 	}
-- 
2.19.2

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

Re: [PATCH v2 2/7] drm/i915: Don't use split gamma when we don't have to

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjala [mailto:ville.syrjala@linux.intel.com]
>Sent: Tuesday, April 2, 2019 1:32 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Shankar, Uma <uma.shankar@intel.com>; Roper, Matthew D
><matthew.d.roper@intel.com>
>Subject: [PATCH v2 2/7] drm/i915: Don't use split gamma when we don't have to
>
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>Using the split gamma mode when we don't have to has the annoying requirement of
>loading a linear LUT to the unused half. Instead let's make life simpler by switching to
>the 10bit gamma mode and duplicating each entry.
>
>This also allows us to load the software gamma LUT into the hardware degamma
>LUT, thus removing some of the buggy configurations we currently allow
>(YCbCr/limited range RGB
>+ gamma LUT). We do still have other configurations that are
>also buggy, but those will need more complicated fixes or they just need to be
>rejected. Sadly GLK doesn't have this flexibility anymore and the degamma and
>gamma LUTs are very different so no help there.
>
>v2: Apply a mask when checking gamma_mode on icl since it
>    contains more bits than just the gamma mode
>v3: Rebase due to EXT_GC_MAX/EXT2_GC_MAX changes
>
>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>---
> drivers/gpu/drm/i915/i915_reg.h    |   2 +
> drivers/gpu/drm/i915/intel_color.c | 185 ++++++++++++++---------------
> 2 files changed, 92 insertions(+), 95 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index
>341f03e00536..bed2c52aebd8 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -7214,6 +7214,7 @@ enum {
> #define GAMMA_MODE(pipe) _MMIO_PIPE(pipe, _GAMMA_MODE_A,
>_GAMMA_MODE_B)
> #define  PRE_CSC_GAMMA_ENABLE	(1 << 31)
> #define  POST_CSC_GAMMA_ENABLE	(1 << 30)
>+#define  GAMMA_MODE_MODE_MASK	(3 << 0)
> #define  GAMMA_MODE_MODE_8BIT	(0 << 0)
> #define  GAMMA_MODE_MODE_10BIT	(1 << 0)
> #define  GAMMA_MODE_MODE_12BIT	(2 << 0)
>@@ -10127,6 +10128,7 @@ enum skl_power_gate {
> #define   PAL_PREC_SPLIT_MODE		(1 << 31)
> #define   PAL_PREC_AUTO_INCREMENT	(1 << 15)
> #define   PAL_PREC_INDEX_VALUE_MASK	(0x3ff << 0)
>+#define   PAL_PREC_INDEX_VALUE(x)	((x) << 0)
> #define _PAL_PREC_DATA_A	0x4A404
> #define _PAL_PREC_DATA_B	0x4AC04
> #define _PAL_PREC_DATA_C	0x4B404
>diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>index d5b3060c2645..5ef93c43afcf 100644
>--- a/drivers/gpu/drm/i915/intel_color.c
>+++ b/drivers/gpu/drm/i915/intel_color.c
>@@ -466,115 +466,83 @@ static void skl_color_commit(const struct intel_crtc_state
>*crtc_state)
> 		ilk_load_csc_matrix(crtc_state);
> }
>
>-static void bdw_load_degamma_lut(const struct intel_crtc_state *crtc_state)
>+static void bdw_load_lut_10(struct intel_crtc *crtc,
>+			    const struct drm_property_blob *blob,
>+			    u32 prec_index, bool duplicate)
> {
>-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>-	const struct drm_property_blob *degamma_lut = crtc_state-
>>base.degamma_lut;
>-	u32 i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
>+	const struct drm_color_lut *lut = blob->data;
>+	int i, lut_size = drm_color_lut_size(blob);
> 	enum pipe pipe = crtc->pipe;
>
>-	I915_WRITE(PREC_PAL_INDEX(pipe),
>-		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT);
>-
>-	if (degamma_lut) {
>-		const struct drm_color_lut *lut = degamma_lut->data;
>+	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
>+		   PAL_PREC_AUTO_INCREMENT);
>
>-		for (i = 0; i < lut_size; i++)
>-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
>-	} else {
>+	/*
>+	 * We advertize the split gamma sizes. When not using split

Typo in advertise.

>+	 * gamma we just duplicate each entry.
>+	 *
>+	 * TODO: expose the full LUT to userspace
>+	 */
>+	if (duplicate) {
> 		for (i = 0; i < lut_size; i++) {
>-			u32 v = (i * ((1 << 10) - 1)) / (lut_size - 1);
>-
>-			I915_WRITE(PREC_PAL_DATA(pipe),
>-				   (v << 20) | (v << 10) | v);
>+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
>+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
> 		}
>+	} else {
>+		for (i = 0; i < lut_size; i++)
>+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
> 	}
>+
>+	/*
>+	 * Reset the index, otherwise it prevents the legacy palette to be
>+	 * written properly.
>+	 */
>+	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
> }
>
>-static void bdw_load_gamma_lut(const struct intel_crtc_state *crtc_state, u32
>offset)
>+static void bdw_load_lut_10_max(struct intel_crtc *crtc)
> {
>-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>-	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
>-	u32 i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
> 	enum pipe pipe = crtc->pipe;
>
>-	WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
>-
>-	I915_WRITE(PREC_PAL_INDEX(pipe),
>-		   (offset ? PAL_PREC_SPLIT_MODE : 0) |
>-		   PAL_PREC_AUTO_INCREMENT |
>-		   offset);
>-
>-	if (gamma_lut) {
>-		const struct drm_color_lut *lut = gamma_lut->data;
>-
>-		for (i = 0; i < lut_size; i++)
>-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
>-
>-		/*
>-		 * Program the max register to clamp values > 1.0.
>-		 * ToDo: Extend the ABI to be able to program values
>-		 * from 1.0 to 3.0
>-		 */
>-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), (1 << 16));
>-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), (1 << 16));
>-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), (1 << 16));
>-
>-		/*
>-		 * Program the gc max 2 register to clamp values > 1.0.
>-		 * ToDo: Extend the ABI to be able to program values
>-		 * from 3.0 to 7.0
>-		 */
>-		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
>-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), (1 << 16));
>-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), (1 << 16));
>-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), (1 << 16));
>-		}
>-	} else {
>-		for (i = 0; i < lut_size; i++) {
>-			u32 v = (i * ((1 << 10) - 1)) / (lut_size - 1);
>-
>-			I915_WRITE(PREC_PAL_DATA(pipe),
>-				   (v << 20) | (v << 10) | v);
>-		}
>-
>-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), (1 << 16));
>-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), (1 << 16));
>-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), (1 << 16));
>-
>-		/*
>-		 * Program the gc max 2 register to clamp values > 1.0.
>-		 * ToDo: Extend the ABI to be able to program values
>-		 * from 3.0 to 7.0
>-		 */
>-		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
>-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), (1 << 16));
>-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), (1 << 16));
>-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), (1 << 16));
>-		}
>-	}
>+	/* Program the max register to clamp values > 1.0. */
>+	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), 1 << 16);
>+	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), 1 << 16);
>+	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), 1 << 16);
>
> 	/*
>-	 * Reset the index, otherwise it prevents the legacy palette to be
>-	 * written properly.
>+	 * Program the gc max 2 register to clamp values > 1.0.
>+	 * ToDo: Extend the ABI to be able to program values
>+	 * from 3.0 to 7.0
> 	 */
>-	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
>+	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
>+		I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), 1 << 16);
>+		I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), 1 << 16);
>+		I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), 1 << 16);
>+	}
> }
>
>-/* Loads the palette/gamma unit for the CRTC on Broadwell+. */ -static void
>broadwell_load_luts(const struct intel_crtc_state *crtc_state)
>+static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
> {
> 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>+	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
>+	const struct drm_property_blob *degamma_lut =
>+crtc_state->base.degamma_lut;
>
>-	if (crtc_state_is_legacy_gamma(crtc_state)) {
>+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
> 		i9xx_load_luts(crtc_state);
>+	} 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), false);
>+		bdw_load_lut_10_max(crtc);
>+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
>+				PAL_PREC_INDEX_VALUE(512),  false);
> 	} else {
>-		bdw_load_degamma_lut(crtc_state);
>-		bdw_load_gamma_lut(crtc_state,
>-				   INTEL_INFO(dev_priv)->color.degamma_lut_size);
>+		const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
>+
>+		bdw_load_lut_10(crtc, blob,
>+				PAL_PREC_INDEX_VALUE(0), true);
>+		bdw_load_lut_10_max(crtc);
> 	}
> }
>
>@@ -646,6 +614,9 @@ static void glk_load_degamma_lut_linear(const struct
>intel_crtc_state *crtc_stat
>
> static void glk_load_luts(const struct intel_crtc_state *crtc_state)  {
>+	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
>+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>+
> 	/*
> 	 * On GLK+ both pipe CSC and degamma LUT are controlled
> 	 * by csc_enable. Hence for the cases where the CSC is @@ -659,22 +630,29
>@@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
> 	else
> 		glk_load_degamma_lut_linear(crtc_state);
>
>-	if (crtc_state_is_legacy_gamma(crtc_state))
>+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
> 		i9xx_load_luts(crtc_state);
>-	else
>-		bdw_load_gamma_lut(crtc_state, 0);
>+	} else {
>+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0),
>false);
>+		bdw_load_lut_10_max(crtc);
>+	}
> }
>
> static void icl_load_luts(const struct intel_crtc_state *crtc_state)  {
>+	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
>+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>+
> 	if (crtc_state->base.degamma_lut)
> 		glk_load_degamma_lut(crtc_state);
>
>-	if (crtc_state_is_legacy_gamma(crtc_state))
>+	if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
>+	    GAMMA_MODE_MODE_8BIT) {
> 		i9xx_load_luts(crtc_state);
>-	else
>-		/* ToDo: Add support for multi segment gamma LUT */
>-		bdw_load_gamma_lut(crtc_state, 0);
>+	} else {
>+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0),
>false);
>+		bdw_load_lut_10_max(crtc);
>+	}
> }
>
> static void cherryview_load_luts(const struct intel_crtc_state *crtc_state) @@ -
>959,8 +937,25 @@ static u32 bdw_gamma_mode(const struct intel_crtc_state
>*crtc_state)
> 	if (!crtc_state->gamma_enable ||
> 	    crtc_state_is_legacy_gamma(crtc_state))
> 		return GAMMA_MODE_MODE_8BIT;
>-	else
>+	else if (crtc_state->base.gamma_lut &&
>+		 crtc_state->base.degamma_lut)
> 		return GAMMA_MODE_MODE_SPLIT;
>+	else
>+		return GAMMA_MODE_MODE_10BIT;
>+}
>+
>+static u32 bdw_csc_mode(const struct intel_crtc_state *crtc_state) {
>+	/*
>+	 * CSC comes after the LUT in degamma, RGB->YCbCr,
>+	 * and RGB full->limited range mode.
>+	 */
>+	if (crtc_state->base.degamma_lut ||
>+	    crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
>+	    crtc_state->limited_color_range)
>+		return 0;

There may be a scenario that non-linear blending is done and then at pipe level,
some CTM manipulation is expected (adjusting hues, saturation etc) for this we may
have to apply degamma to make the content linear, then CTM and finally gamma.
So is it right to presume that  if degamma is there CSC will always be after gamma ?

>+	return CSC_POSITION_BEFORE_GAMMA;
> }
>
> static int bdw_color_check(struct intel_crtc_state *crtc_state) @@ -982,7 +977,7
>@@ static int bdw_color_check(struct intel_crtc_state *crtc_state)
>
> 	crtc_state->gamma_mode = bdw_gamma_mode(crtc_state);
>
>-	crtc_state->csc_mode = 0;
>+	crtc_state->csc_mode = bdw_csc_mode(crtc_state);
>
> 	ret = intel_color_add_affected_planes(crtc_state);
> 	if (ret)
>@@ -1116,7 +1111,7 @@ void intel_color_init(struct intel_crtc *crtc)
> 		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> 			dev_priv->display.load_luts = glk_load_luts;
> 		else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
>-			dev_priv->display.load_luts = broadwell_load_luts;
>+			dev_priv->display.load_luts = bdw_load_luts;
> 		else
> 			dev_priv->display.load_luts = i9xx_load_luts;
> 	}
>--
>2.19.2

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

Re: [PATCH v2 2/7] drm/i915: Don't use split gamma when we don't have to

[Ville Syrjälä]

On Wed, Apr 03, 2019 at 12:23:06PM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Ville Syrjala [mailto:ville.syrjala@linux.intel.com]
> >Sent: Tuesday, April 2, 2019 1:32 AM
> >To: intel-gfx@lists.freedesktop.org
> >Cc: Shankar, Uma <uma.shankar@intel.com>; Roper, Matthew D
> ><matthew.d.roper@intel.com>
> >Subject: [PATCH v2 2/7] drm/i915: Don't use split gamma when we don't have to
> >
> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >Using the split gamma mode when we don't have to has the annoying requirement of
> >loading a linear LUT to the unused half. Instead let's make life simpler by switching to
> >the 10bit gamma mode and duplicating each entry.
> >
> >This also allows us to load the software gamma LUT into the hardware degamma
> >LUT, thus removing some of the buggy configurations we currently allow
> >(YCbCr/limited range RGB
> >+ gamma LUT). We do still have other configurations that are
> >also buggy, but those will need more complicated fixes or they just need to be
> >rejected. Sadly GLK doesn't have this flexibility anymore and the degamma and
> >gamma LUTs are very different so no help there.
> >
> >v2: Apply a mask when checking gamma_mode on icl since it
> >    contains more bits than just the gamma mode
> >v3: Rebase due to EXT_GC_MAX/EXT2_GC_MAX changes
> >
> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >---
> > drivers/gpu/drm/i915/i915_reg.h    |   2 +
> > drivers/gpu/drm/i915/intel_color.c | 185 ++++++++++++++---------------
> > 2 files changed, 92 insertions(+), 95 deletions(-)
> >
> >diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index
> >341f03e00536..bed2c52aebd8 100644
> >--- a/drivers/gpu/drm/i915/i915_reg.h
> >+++ b/drivers/gpu/drm/i915/i915_reg.h
> >@@ -7214,6 +7214,7 @@ enum {
> > #define GAMMA_MODE(pipe) _MMIO_PIPE(pipe, _GAMMA_MODE_A,
> >_GAMMA_MODE_B)
> > #define  PRE_CSC_GAMMA_ENABLE	(1 << 31)
> > #define  POST_CSC_GAMMA_ENABLE	(1 << 30)
> >+#define  GAMMA_MODE_MODE_MASK	(3 << 0)
> > #define  GAMMA_MODE_MODE_8BIT	(0 << 0)
> > #define  GAMMA_MODE_MODE_10BIT	(1 << 0)
> > #define  GAMMA_MODE_MODE_12BIT	(2 << 0)
> >@@ -10127,6 +10128,7 @@ enum skl_power_gate {
> > #define   PAL_PREC_SPLIT_MODE		(1 << 31)
> > #define   PAL_PREC_AUTO_INCREMENT	(1 << 15)
> > #define   PAL_PREC_INDEX_VALUE_MASK	(0x3ff << 0)
> >+#define   PAL_PREC_INDEX_VALUE(x)	((x) << 0)
> > #define _PAL_PREC_DATA_A	0x4A404
> > #define _PAL_PREC_DATA_B	0x4AC04
> > #define _PAL_PREC_DATA_C	0x4B404
> >diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> >index d5b3060c2645..5ef93c43afcf 100644
> >--- a/drivers/gpu/drm/i915/intel_color.c
> >+++ b/drivers/gpu/drm/i915/intel_color.c
> >@@ -466,115 +466,83 @@ static void skl_color_commit(const struct intel_crtc_state
> >*crtc_state)
> > 		ilk_load_csc_matrix(crtc_state);
> > }
> >
> >-static void bdw_load_degamma_lut(const struct intel_crtc_state *crtc_state)
> >+static void bdw_load_lut_10(struct intel_crtc *crtc,
> >+			    const struct drm_property_blob *blob,
> >+			    u32 prec_index, bool duplicate)
> > {
> >-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >-	const struct drm_property_blob *degamma_lut = crtc_state-
> >>base.degamma_lut;
> >-	u32 i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
> >+	const struct drm_color_lut *lut = blob->data;
> >+	int i, lut_size = drm_color_lut_size(blob);
> > 	enum pipe pipe = crtc->pipe;
> >
> >-	I915_WRITE(PREC_PAL_INDEX(pipe),
> >-		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT);
> >-
> >-	if (degamma_lut) {
> >-		const struct drm_color_lut *lut = degamma_lut->data;
> >+	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
> >+		   PAL_PREC_AUTO_INCREMENT);
> >
> >-		for (i = 0; i < lut_size; i++)
> >-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
> >-	} else {
> >+	/*
> >+	 * We advertize the split gamma sizes. When not using split
> 
> Typo in advertise.

Just a different language. Though maybe the 'z' isn't quite right even
for US English. "Consistency not included" should be on the packaging.

> 
> >+	 * gamma we just duplicate each entry.
> >+	 *
> >+	 * TODO: expose the full LUT to userspace
> >+	 */
> >+	if (duplicate) {
> > 		for (i = 0; i < lut_size; i++) {
> >-			u32 v = (i * ((1 << 10) - 1)) / (lut_size - 1);
> >-
> >-			I915_WRITE(PREC_PAL_DATA(pipe),
> >-				   (v << 20) | (v << 10) | v);
> >+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
> >+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
> > 		}
> >+	} else {
> >+		for (i = 0; i < lut_size; i++)
> >+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
> > 	}
> >+
> >+	/*
> >+	 * Reset the index, otherwise it prevents the legacy palette to be
> >+	 * written properly.
> >+	 */
> >+	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
> > }
> >
> >-static void bdw_load_gamma_lut(const struct intel_crtc_state *crtc_state, u32
> >offset)
> >+static void bdw_load_lut_10_max(struct intel_crtc *crtc)
> > {
> >-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> > 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >-	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
> >-	u32 i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
> > 	enum pipe pipe = crtc->pipe;
> >
> >-	WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
> >-
> >-	I915_WRITE(PREC_PAL_INDEX(pipe),
> >-		   (offset ? PAL_PREC_SPLIT_MODE : 0) |
> >-		   PAL_PREC_AUTO_INCREMENT |
> >-		   offset);
> >-
> >-	if (gamma_lut) {
> >-		const struct drm_color_lut *lut = gamma_lut->data;
> >-
> >-		for (i = 0; i < lut_size; i++)
> >-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
> >-
> >-		/*
> >-		 * Program the max register to clamp values > 1.0.
> >-		 * ToDo: Extend the ABI to be able to program values
> >-		 * from 1.0 to 3.0
> >-		 */
> >-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), (1 << 16));
> >-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), (1 << 16));
> >-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), (1 << 16));
> >-
> >-		/*
> >-		 * Program the gc max 2 register to clamp values > 1.0.
> >-		 * ToDo: Extend the ABI to be able to program values
> >-		 * from 3.0 to 7.0
> >-		 */
> >-		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> >-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), (1 << 16));
> >-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), (1 << 16));
> >-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), (1 << 16));
> >-		}
> >-	} else {
> >-		for (i = 0; i < lut_size; i++) {
> >-			u32 v = (i * ((1 << 10) - 1)) / (lut_size - 1);
> >-
> >-			I915_WRITE(PREC_PAL_DATA(pipe),
> >-				   (v << 20) | (v << 10) | v);
> >-		}
> >-
> >-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), (1 << 16));
> >-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), (1 << 16));
> >-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), (1 << 16));
> >-
> >-		/*
> >-		 * Program the gc max 2 register to clamp values > 1.0.
> >-		 * ToDo: Extend the ABI to be able to program values
> >-		 * from 3.0 to 7.0
> >-		 */
> >-		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> >-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), (1 << 16));
> >-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), (1 << 16));
> >-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), (1 << 16));
> >-		}
> >-	}
> >+	/* Program the max register to clamp values > 1.0. */
> >+	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), 1 << 16);
> >+	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), 1 << 16);
> >+	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), 1 << 16);
> >
> > 	/*
> >-	 * Reset the index, otherwise it prevents the legacy palette to be
> >-	 * written properly.
> >+	 * Program the gc max 2 register to clamp values > 1.0.
> >+	 * ToDo: Extend the ABI to be able to program values
> >+	 * from 3.0 to 7.0
> > 	 */
> >-	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
> >+	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
> >+		I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), 1 << 16);
> >+		I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), 1 << 16);
> >+		I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), 1 << 16);
> >+	}
> > }
> >
> >-/* Loads the palette/gamma unit for the CRTC on Broadwell+. */ -static void
> >broadwell_load_luts(const struct intel_crtc_state *crtc_state)
> >+static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
> > {
> > 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> >-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >+	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
> >+	const struct drm_property_blob *degamma_lut =
> >+crtc_state->base.degamma_lut;
> >
> >-	if (crtc_state_is_legacy_gamma(crtc_state)) {
> >+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
> > 		i9xx_load_luts(crtc_state);
> >+	} 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), false);
> >+		bdw_load_lut_10_max(crtc);
> >+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
> >+				PAL_PREC_INDEX_VALUE(512),  false);
> > 	} else {
> >-		bdw_load_degamma_lut(crtc_state);
> >-		bdw_load_gamma_lut(crtc_state,
> >-				   INTEL_INFO(dev_priv)->color.degamma_lut_size);
> >+		const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
> >+
> >+		bdw_load_lut_10(crtc, blob,
> >+				PAL_PREC_INDEX_VALUE(0), true);
> >+		bdw_load_lut_10_max(crtc);
> > 	}
> > }
> >
> >@@ -646,6 +614,9 @@ static void glk_load_degamma_lut_linear(const struct
> >intel_crtc_state *crtc_stat
> >
> > static void glk_load_luts(const struct intel_crtc_state *crtc_state)  {
> >+	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
> >+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> >+
> > 	/*
> > 	 * On GLK+ both pipe CSC and degamma LUT are controlled
> > 	 * by csc_enable. Hence for the cases where the CSC is @@ -659,22 +630,29
> >@@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
> > 	else
> > 		glk_load_degamma_lut_linear(crtc_state);
> >
> >-	if (crtc_state_is_legacy_gamma(crtc_state))
> >+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
> > 		i9xx_load_luts(crtc_state);
> >-	else
> >-		bdw_load_gamma_lut(crtc_state, 0);
> >+	} else {
> >+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0),
> >false);
> >+		bdw_load_lut_10_max(crtc);
> >+	}
> > }
> >
> > static void icl_load_luts(const struct intel_crtc_state *crtc_state)  {
> >+	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
> >+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> >+
> > 	if (crtc_state->base.degamma_lut)
> > 		glk_load_degamma_lut(crtc_state);
> >
> >-	if (crtc_state_is_legacy_gamma(crtc_state))
> >+	if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
> >+	    GAMMA_MODE_MODE_8BIT) {
> > 		i9xx_load_luts(crtc_state);
> >-	else
> >-		/* ToDo: Add support for multi segment gamma LUT */
> >-		bdw_load_gamma_lut(crtc_state, 0);
> >+	} else {
> >+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0),
> >false);
> >+		bdw_load_lut_10_max(crtc);
> >+	}
> > }
> >
> > static void cherryview_load_luts(const struct intel_crtc_state *crtc_state) @@ -
> >959,8 +937,25 @@ static u32 bdw_gamma_mode(const struct intel_crtc_state
> >*crtc_state)
> > 	if (!crtc_state->gamma_enable ||
> > 	    crtc_state_is_legacy_gamma(crtc_state))
> > 		return GAMMA_MODE_MODE_8BIT;
> >-	else
> >+	else if (crtc_state->base.gamma_lut &&
> >+		 crtc_state->base.degamma_lut)
> > 		return GAMMA_MODE_MODE_SPLIT;
> >+	else
> >+		return GAMMA_MODE_MODE_10BIT;
> >+}
> >+
> >+static u32 bdw_csc_mode(const struct intel_crtc_state *crtc_state) {
> >+	/*
> >+	 * CSC comes after the LUT in degamma, RGB->YCbCr,
> >+	 * and RGB full->limited range mode.
> >+	 */
> >+	if (crtc_state->base.degamma_lut ||
> >+	    crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
> >+	    crtc_state->limited_color_range)
> >+		return 0;
> 
> There may be a scenario that non-linear blending is done and then at pipe level,
> some CTM manipulation is expected (adjusting hues, saturation etc) for this we may
> have to apply degamma to make the content linear, then CTM and finally gamma.
> So is it right to presume that  if degamma is there CSC will always be after gamma ?

Yes, this just determines whether we get lut->csc or csc->lut in
non-split gamma modes. For the case you're taking about we'll use
the split gamma mode so it'll be lut->csc->lut regardless of this
CSC_MODE bit.

> 
> >+	return CSC_POSITION_BEFORE_GAMMA;
> > }
> >
> > static int bdw_color_check(struct intel_crtc_state *crtc_state) @@ -982,7 +977,7
> >@@ static int bdw_color_check(struct intel_crtc_state *crtc_state)
> >
> > 	crtc_state->gamma_mode = bdw_gamma_mode(crtc_state);
> >
> >-	crtc_state->csc_mode = 0;
> >+	crtc_state->csc_mode = bdw_csc_mode(crtc_state);
> >
> > 	ret = intel_color_add_affected_planes(crtc_state);
> > 	if (ret)
> >@@ -1116,7 +1111,7 @@ void intel_color_init(struct intel_crtc *crtc)
> > 		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> > 			dev_priv->display.load_luts = glk_load_luts;
> > 		else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
> >-			dev_priv->display.load_luts = broadwell_load_luts;
> >+			dev_priv->display.load_luts = bdw_load_luts;
> > 		else
> > 			dev_priv->display.load_luts = i9xx_load_luts;
> > 	}
> >--
> >2.19.2
> 

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

Re: [PATCH v2 2/7] drm/i915: Don't use split gamma when we don't have to

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Wednesday, April 3, 2019 6:10 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Roper, Matthew D
><matthew.d.roper@intel.com>
>Subject: Re: [PATCH v2 2/7] drm/i915: Don't use split gamma when we don't have to
>
>On Wed, Apr 03, 2019 at 12:23:06PM +0000, Shankar, Uma wrote:
>>
>>
>> >-----Original Message-----
>> >From: Ville Syrjala [mailto:ville.syrjala@linux.intel.com]
>> >Sent: Tuesday, April 2, 2019 1:32 AM
>> >To: intel-gfx@lists.freedesktop.org
>> >Cc: Shankar, Uma <uma.shankar@intel.com>; Roper, Matthew D
>> ><matthew.d.roper@intel.com>
>> >Subject: [PATCH v2 2/7] drm/i915: Don't use split gamma when we don't
>> >have to
>> >
>> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> >Using the split gamma mode when we don't have to has the annoying
>> >requirement of loading a linear LUT to the unused half. Instead let's
>> >make life simpler by switching to the 10bit gamma mode and duplicating each
>entry.
>> >
>> >This also allows us to load the software gamma LUT into the hardware
>> >degamma LUT, thus removing some of the buggy configurations we
>> >currently allow (YCbCr/limited range RGB
>> >+ gamma LUT). We do still have other configurations that are
>> >also buggy, but those will need more complicated fixes or they just
>> >need to be rejected. Sadly GLK doesn't have this flexibility anymore
>> >and the degamma and gamma LUTs are very different so no help there.
>> >
>> >v2: Apply a mask when checking gamma_mode on icl since it
>> >    contains more bits than just the gamma mode
>> >v3: Rebase due to EXT_GC_MAX/EXT2_GC_MAX changes
>> >
>> >Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >---
>> > drivers/gpu/drm/i915/i915_reg.h    |   2 +
>> > drivers/gpu/drm/i915/intel_color.c | 185
>> >++++++++++++++---------------
>> > 2 files changed, 92 insertions(+), 95 deletions(-)
>> >
>> >diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> >b/drivers/gpu/drm/i915/i915_reg.h index
>> >341f03e00536..bed2c52aebd8 100644
>> >--- a/drivers/gpu/drm/i915/i915_reg.h
>> >+++ b/drivers/gpu/drm/i915/i915_reg.h
>> >@@ -7214,6 +7214,7 @@ enum {
>> > #define GAMMA_MODE(pipe) _MMIO_PIPE(pipe, _GAMMA_MODE_A,
>> >_GAMMA_MODE_B)
>> > #define  PRE_CSC_GAMMA_ENABLE	(1 << 31)
>> > #define  POST_CSC_GAMMA_ENABLE	(1 << 30)
>> >+#define  GAMMA_MODE_MODE_MASK	(3 << 0)
>> > #define  GAMMA_MODE_MODE_8BIT	(0 << 0)
>> > #define  GAMMA_MODE_MODE_10BIT	(1 << 0)
>> > #define  GAMMA_MODE_MODE_12BIT	(2 << 0)
>> >@@ -10127,6 +10128,7 @@ enum skl_power_gate {
>> > #define   PAL_PREC_SPLIT_MODE		(1 << 31)
>> > #define   PAL_PREC_AUTO_INCREMENT	(1 << 15)
>> > #define   PAL_PREC_INDEX_VALUE_MASK	(0x3ff << 0)
>> >+#define   PAL_PREC_INDEX_VALUE(x)	((x) << 0)
>> > #define _PAL_PREC_DATA_A	0x4A404
>> > #define _PAL_PREC_DATA_B	0x4AC04
>> > #define _PAL_PREC_DATA_C	0x4B404
>> >diff --git a/drivers/gpu/drm/i915/intel_color.c
>> >b/drivers/gpu/drm/i915/intel_color.c
>> >index d5b3060c2645..5ef93c43afcf 100644
>> >--- a/drivers/gpu/drm/i915/intel_color.c
>> >+++ b/drivers/gpu/drm/i915/intel_color.c
>> >@@ -466,115 +466,83 @@ static void skl_color_commit(const struct
>> >intel_crtc_state
>> >*crtc_state)
>> > 		ilk_load_csc_matrix(crtc_state);
>> > }
>> >
>> >-static void bdw_load_degamma_lut(const struct intel_crtc_state
>> >*crtc_state)
>> >+static void bdw_load_lut_10(struct intel_crtc *crtc,
>> >+			    const struct drm_property_blob *blob,
>> >+			    u32 prec_index, bool duplicate)
>> > {
>> >-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> > 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> >-	const struct drm_property_blob *degamma_lut = crtc_state-
>> >>base.degamma_lut;
>> >-	u32 i, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
>> >+	const struct drm_color_lut *lut = blob->data;
>> >+	int i, lut_size = drm_color_lut_size(blob);
>> > 	enum pipe pipe = crtc->pipe;
>> >
>> >-	I915_WRITE(PREC_PAL_INDEX(pipe),
>> >-		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT);
>> >-
>> >-	if (degamma_lut) {
>> >-		const struct drm_color_lut *lut = degamma_lut->data;
>> >+	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
>> >+		   PAL_PREC_AUTO_INCREMENT);
>> >
>> >-		for (i = 0; i < lut_size; i++)
>> >-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
>> >-	} else {
>> >+	/*
>> >+	 * We advertize the split gamma sizes. When not using split
>>
>> Typo in advertise.
>
>Just a different language. Though maybe the 'z' isn't quite right even for US English.
>"Consistency not included" should be on the packaging.

Yeah agree :). Opened this on outlook and got this spell check. 

>>
>> >+	 * gamma we just duplicate each entry.
>> >+	 *
>> >+	 * TODO: expose the full LUT to userspace
>> >+	 */
>> >+	if (duplicate) {
>> > 		for (i = 0; i < lut_size; i++) {
>> >-			u32 v = (i * ((1 << 10) - 1)) / (lut_size - 1);
>> >-
>> >-			I915_WRITE(PREC_PAL_DATA(pipe),
>> >-				   (v << 20) | (v << 10) | v);
>> >+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
>> >+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
>> > 		}
>> >+	} else {
>> >+		for (i = 0; i < lut_size; i++)
>> >+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
>> > 	}
>> >+
>> >+	/*
>> >+	 * Reset the index, otherwise it prevents the legacy palette to be
>> >+	 * written properly.
>> >+	 */
>> >+	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
>> > }
>> >
>> >-static void bdw_load_gamma_lut(const struct intel_crtc_state
>> >*crtc_state, u32
>> >offset)
>> >+static void bdw_load_lut_10_max(struct intel_crtc *crtc)
>> > {
>> >-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> > 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> >-	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
>> >-	u32 i, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
>> > 	enum pipe pipe = crtc->pipe;
>> >
>> >-	WARN_ON(offset & ~PAL_PREC_INDEX_VALUE_MASK);
>> >-
>> >-	I915_WRITE(PREC_PAL_INDEX(pipe),
>> >-		   (offset ? PAL_PREC_SPLIT_MODE : 0) |
>> >-		   PAL_PREC_AUTO_INCREMENT |
>> >-		   offset);
>> >-
>> >-	if (gamma_lut) {
>> >-		const struct drm_color_lut *lut = gamma_lut->data;
>> >-
>> >-		for (i = 0; i < lut_size; i++)
>> >-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
>> >-
>> >-		/*
>> >-		 * Program the max register to clamp values > 1.0.
>> >-		 * ToDo: Extend the ABI to be able to program values
>> >-		 * from 1.0 to 3.0
>> >-		 */
>> >-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), (1 << 16));
>> >-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), (1 << 16));
>> >-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), (1 << 16));
>> >-
>> >-		/*
>> >-		 * Program the gc max 2 register to clamp values > 1.0.
>> >-		 * ToDo: Extend the ABI to be able to program values
>> >-		 * from 3.0 to 7.0
>> >-		 */
>> >-		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
>> >-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), (1 << 16));
>> >-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), (1 << 16));
>> >-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), (1 << 16));
>> >-		}
>> >-	} else {
>> >-		for (i = 0; i < lut_size; i++) {
>> >-			u32 v = (i * ((1 << 10) - 1)) / (lut_size - 1);
>> >-
>> >-			I915_WRITE(PREC_PAL_DATA(pipe),
>> >-				   (v << 20) | (v << 10) | v);
>> >-		}
>> >-
>> >-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), (1 << 16));
>> >-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), (1 << 16));
>> >-		I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), (1 << 16));
>> >-
>> >-		/*
>> >-		 * Program the gc max 2 register to clamp values > 1.0.
>> >-		 * ToDo: Extend the ABI to be able to program values
>> >-		 * from 3.0 to 7.0
>> >-		 */
>> >-		if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
>> >-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), (1 << 16));
>> >-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), (1 << 16));
>> >-			I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), (1 << 16));
>> >-		}
>> >-	}
>> >+	/* Program the max register to clamp values > 1.0. */
>> >+	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 0), 1 << 16);
>> >+	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 1), 1 << 16);
>> >+	I915_WRITE(PREC_PAL_EXT_GC_MAX(pipe, 2), 1 << 16);
>> >
>> > 	/*
>> >-	 * Reset the index, otherwise it prevents the legacy palette to be
>> >-	 * written properly.
>> >+	 * Program the gc max 2 register to clamp values > 1.0.
>> >+	 * ToDo: Extend the ABI to be able to program values
>> >+	 * from 3.0 to 7.0
>> > 	 */
>> >-	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
>> >+	if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv)) {
>> >+		I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 0), 1 << 16);
>> >+		I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 1), 1 << 16);
>> >+		I915_WRITE(PREC_PAL_EXT2_GC_MAX(pipe, 2), 1 << 16);
>> >+	}
>> > }
>> >
>> >-/* Loads the palette/gamma unit for the CRTC on Broadwell+. */
>> >-static void broadwell_load_luts(const struct intel_crtc_state
>> >*crtc_state)
>> >+static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
>> > {
>> > 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> >-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> >+	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
>> >+	const struct drm_property_blob *degamma_lut =
>> >+crtc_state->base.degamma_lut;
>> >
>> >-	if (crtc_state_is_legacy_gamma(crtc_state)) {
>> >+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
>> > 		i9xx_load_luts(crtc_state);
>> >+	} 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), false);
>> >+		bdw_load_lut_10_max(crtc);
>> >+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
>> >+				PAL_PREC_INDEX_VALUE(512),  false);
>> > 	} else {
>> >-		bdw_load_degamma_lut(crtc_state);
>> >-		bdw_load_gamma_lut(crtc_state,
>> >-				   INTEL_INFO(dev_priv)->color.degamma_lut_size);
>> >+		const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
>> >+
>> >+		bdw_load_lut_10(crtc, blob,
>> >+				PAL_PREC_INDEX_VALUE(0), true);
>> >+		bdw_load_lut_10_max(crtc);
>> > 	}
>> > }
>> >
>> >@@ -646,6 +614,9 @@ static void glk_load_degamma_lut_linear(const
>> >struct intel_crtc_state *crtc_stat
>> >
>> > static void glk_load_luts(const struct intel_crtc_state *crtc_state)
>> > {
>> >+	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
>> >+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> >+
>> > 	/*
>> > 	 * On GLK+ both pipe CSC and degamma LUT are controlled
>> > 	 * by csc_enable. Hence for the cases where the CSC is @@ -659,22
>> >+630,29 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
>> > 	else
>> > 		glk_load_degamma_lut_linear(crtc_state);
>> >
>> >-	if (crtc_state_is_legacy_gamma(crtc_state))
>> >+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
>> > 		i9xx_load_luts(crtc_state);
>> >-	else
>> >-		bdw_load_gamma_lut(crtc_state, 0);
>> >+	} else {
>> >+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0),
>> >false);
>> >+		bdw_load_lut_10_max(crtc);
>> >+	}
>> > }
>> >
>> > static void icl_load_luts(const struct intel_crtc_state *crtc_state)
>> > {
>> >+	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
>> >+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> >+
>> > 	if (crtc_state->base.degamma_lut)
>> > 		glk_load_degamma_lut(crtc_state);
>> >
>> >-	if (crtc_state_is_legacy_gamma(crtc_state))
>> >+	if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
>> >+	    GAMMA_MODE_MODE_8BIT) {
>> > 		i9xx_load_luts(crtc_state);
>> >-	else
>> >-		/* ToDo: Add support for multi segment gamma LUT */
>> >-		bdw_load_gamma_lut(crtc_state, 0);
>> >+	} else {
>> >+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0),
>> >false);
>> >+		bdw_load_lut_10_max(crtc);
>> >+	}
>> > }
>> >
>> > static void cherryview_load_luts(const struct intel_crtc_state
>> >*crtc_state) @@ -
>> >959,8 +937,25 @@ static u32 bdw_gamma_mode(const struct
>> >intel_crtc_state
>> >*crtc_state)
>> > 	if (!crtc_state->gamma_enable ||
>> > 	    crtc_state_is_legacy_gamma(crtc_state))
>> > 		return GAMMA_MODE_MODE_8BIT;
>> >-	else
>> >+	else if (crtc_state->base.gamma_lut &&
>> >+		 crtc_state->base.degamma_lut)
>> > 		return GAMMA_MODE_MODE_SPLIT;
>> >+	else
>> >+		return GAMMA_MODE_MODE_10BIT;
>> >+}
>> >+
>> >+static u32 bdw_csc_mode(const struct intel_crtc_state *crtc_state) {
>> >+	/*
>> >+	 * CSC comes after the LUT in degamma, RGB->YCbCr,
>> >+	 * and RGB full->limited range mode.
>> >+	 */
>> >+	if (crtc_state->base.degamma_lut ||
>> >+	    crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
>> >+	    crtc_state->limited_color_range)
>> >+		return 0;
>>
>> There may be a scenario that non-linear blending is done and then at
>> pipe level, some CTM manipulation is expected (adjusting hues,
>> saturation etc) for this we may have to apply degamma to make the content linear,
>then CTM and finally gamma.
>> So is it right to presume that  if degamma is there CSC will always be after gamma ?
>
>Yes, this just determines whether we get lut->csc or csc->lut in non-split gamma
>modes. For the case you're taking about we'll use the split gamma mode so it'll be lut-
>>csc->lut regardless of this CSC_MODE bit.

Ok yeah, Split gamma mode by default configures itself as degamma ->csc>gamma, we should be
good here.

Changes look ok to me:
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

>>
>> >+	return CSC_POSITION_BEFORE_GAMMA;
>> > }
>> >
>> > static int bdw_color_check(struct intel_crtc_state *crtc_state) @@
>> >-982,7 +977,7 @@ static int bdw_color_check(struct intel_crtc_state
>> >*crtc_state)
>> >
>> > 	crtc_state->gamma_mode = bdw_gamma_mode(crtc_state);
>> >
>> >-	crtc_state->csc_mode = 0;
>> >+	crtc_state->csc_mode = bdw_csc_mode(crtc_state);
>> >
>> > 	ret = intel_color_add_affected_planes(crtc_state);
>> > 	if (ret)
>> >@@ -1116,7 +1111,7 @@ void intel_color_init(struct intel_crtc *crtc)
>> > 		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>> > 			dev_priv->display.load_luts = glk_load_luts;
>> > 		else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
>> >-			dev_priv->display.load_luts = broadwell_load_luts;
>> >+			dev_priv->display.load_luts = bdw_load_luts;
>> > 		else
>> > 			dev_priv->display.load_luts = i9xx_load_luts;
>> > 	}
>> >--
>> >2.19.2
>>
>
>--
>Ville Syrjälä
>Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v2 3/7] drm/i915: Implement split/10bit gamma for ivb/hsw

[Ville Syrjala]

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

Reuse the bdw+ code to get split/10bit gamma for
ivb/hsw. The hardware is nearly identical. The
only slight snag is that on ivb/hsw the precision
palette auto increment mode does not work. So we
must increment the index manually. We'll probably
want to stick to the auto increment mode on bdw+
in the name of efficiency.

Also we want to avoid using the CSC for limited range
RGB output as PIPECONF will take care of that on IVB.

v2: Rebase due to EXT_GC_MAX/EXT2_GC_MAX changes

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c    |   6 +-
 drivers/gpu/drm/i915/intel_color.c | 113 +++++++++++++++++++++++------
 2 files changed, 95 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 39251586349a..2b0d2f4f8a46 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -116,7 +116,7 @@
 		[PIPE_C] = IVB_CURSOR_C_OFFSET, \
 	}
 
-#define BDW_COLORS \
+#define IVB_COLORS \
 	.color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }
 #define CHV_COLORS \
 	.color = { .degamma_lut_size = 65, .gamma_lut_size = 257, \
@@ -406,6 +406,7 @@ static const struct intel_device_info intel_sandybridge_m_gt2_info = {
 	.ppgtt_size = 31, \
 	IVB_PIPE_OFFSETS, \
 	IVB_CURSOR_OFFSETS, \
+	IVB_COLORS, \
 	GEN_DEFAULT_PAGE_SIZES
 
 #define IVB_D_PLATFORM \
@@ -501,7 +502,6 @@ static const struct intel_device_info intel_haswell_gt3_info = {
 #define GEN8_FEATURES \
 	G75_FEATURES, \
 	GEN(8), \
-	BDW_COLORS, \
 	.page_sizes = I915_GTT_PAGE_SIZE_4K | \
 		      I915_GTT_PAGE_SIZE_2M, \
 	.has_logical_ring_contexts = 1, \
@@ -636,7 +636,7 @@ static const struct intel_device_info intel_skylake_gt4_info = {
 	.display.has_ipc = 1, \
 	HSW_PIPE_OFFSETS, \
 	IVB_CURSOR_OFFSETS, \
-	BDW_COLORS, \
+	IVB_COLORS, \
 	GEN9_DEFAULT_PAGE_SIZES
 
 static const struct intel_device_info intel_broxton_info = {
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 5ef93c43afcf..bb3b8afdb933 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -428,6 +428,8 @@ static void ilk_color_commit(const struct intel_crtc_state *crtc_state)
 	val &= ~PIPECONF_GAMMA_MODE_MASK_ILK;
 	val |= PIPECONF_GAMMA_MODE(crtc_state->gamma_mode);
 	I915_WRITE(PIPECONF(pipe), val);
+
+	ilk_load_csc_matrix(crtc_state);
 }
 
 static void hsw_color_commit(const struct intel_crtc_state *crtc_state)
@@ -466,6 +468,48 @@ static void skl_color_commit(const struct intel_crtc_state *crtc_state)
 		ilk_load_csc_matrix(crtc_state);
 }
 
+/*
+ * IVB/HSW Bspec / PAL_PREC_INDEX:
+ * "Restriction : Index auto increment mode is not
+ *  supported and must not be enabled."
+ */
+static void ivb_load_lut_10(struct intel_crtc *crtc,
+			    const struct drm_property_blob *blob,
+			    u32 prec_index, bool duplicate)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct drm_color_lut *lut = blob->data;
+	int i, lut_size = drm_color_lut_size(blob);
+	enum pipe pipe = crtc->pipe;
+
+	/*
+	 * We advertize the split gamma sizes. When not using split
+	 * gamma we just duplicate each entry.
+	 *
+	 * TODO: expose the full LUT to userspace
+	 */
+	if (duplicate) {
+		for (i = 0; i < lut_size; i++) {
+			I915_WRITE(PREC_PAL_INDEX(pipe), prec_index++);
+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
+			I915_WRITE(PREC_PAL_INDEX(pipe), prec_index++);
+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
+		}
+	} else {
+		for (i = 0; i < lut_size; i++) {
+			I915_WRITE(PREC_PAL_INDEX(pipe), prec_index++);
+			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
+		}
+	}
+
+	/*
+	 * Reset the index, otherwise it prevents the legacy palette to be
+	 * written properly.
+	 */
+	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
+}
+
+/* On BDW+ the index auto increment mode actually works */
 static void bdw_load_lut_10(struct intel_crtc *crtc,
 			    const struct drm_property_blob *blob,
 			    u32 prec_index, bool duplicate)
@@ -501,7 +545,7 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
 	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
 }
 
-static void bdw_load_lut_10_max(struct intel_crtc *crtc)
+static void ivb_load_lut_10_max(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
@@ -523,6 +567,29 @@ static void bdw_load_lut_10_max(struct intel_crtc *crtc)
 	}
 }
 
+static void ivb_load_luts(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
+	const struct drm_property_blob *degamma_lut = crtc_state->base.degamma_lut;
+
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
+		i9xx_load_luts(crtc_state);
+	} 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), false);
+		ivb_load_lut_10_max(crtc);
+		ivb_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
+				PAL_PREC_INDEX_VALUE(512),  false);
+	} else {
+		const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
+
+		ivb_load_lut_10(crtc, blob,
+				PAL_PREC_INDEX_VALUE(0), true);
+		ivb_load_lut_10_max(crtc);
+	}
+}
+
 static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
@@ -534,7 +601,7 @@ static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
 	} 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), false);
-		bdw_load_lut_10_max(crtc);
+		ivb_load_lut_10_max(crtc);
 		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
 				PAL_PREC_INDEX_VALUE(512),  false);
 	} else {
@@ -542,7 +609,7 @@ static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
 
 		bdw_load_lut_10(crtc, blob,
 				PAL_PREC_INDEX_VALUE(0), true);
-		bdw_load_lut_10_max(crtc);
+		ivb_load_lut_10_max(crtc);
 	}
 }
 
@@ -634,7 +701,7 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
 		i9xx_load_luts(crtc_state);
 	} else {
 		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0), false);
-		bdw_load_lut_10_max(crtc);
+		ivb_load_lut_10_max(crtc);
 	}
 }
 
@@ -651,7 +718,7 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
 		i9xx_load_luts(crtc_state);
 	} else {
 		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0), false);
-		bdw_load_lut_10_max(crtc);
+		ivb_load_lut_10_max(crtc);
 	}
 }
 
@@ -913,14 +980,13 @@ static int ilk_color_check(struct intel_crtc_state *crtc_state)
 		!crtc_state->c8_planes;
 
 	/*
-	 * We don't expose the ctm on ilk-hsw currently,
-	 * nor do we enable YCbCr output. Only hsw uses
-	 * the csc for RGB limited range output.
+	 * We don't expose the ctm on ilk/snb currently,
+	 * nor do we enable YCbCr output. Also RGB limited
+	 * range output is handled by the hw automagically.
 	 */
-	crtc_state->csc_enable =
-		ilk_csc_limited_range(crtc_state);
+	crtc_state->csc_enable = false;
 
-	/* We don't expose fancy gamma modes on ilk-hsw currently */
+	/* We don't expose fancy gamma modes on ilk/snb currently */
 	crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
 
 	crtc_state->csc_mode = 0;
@@ -932,7 +998,7 @@ static int ilk_color_check(struct intel_crtc_state *crtc_state)
 	return 0;
 }
 
-static u32 bdw_gamma_mode(const struct intel_crtc_state *crtc_state)
+static u32 ivb_gamma_mode(const struct intel_crtc_state *crtc_state)
 {
 	if (!crtc_state->gamma_enable ||
 	    crtc_state_is_legacy_gamma(crtc_state))
@@ -944,22 +1010,25 @@ static u32 bdw_gamma_mode(const struct intel_crtc_state *crtc_state)
 		return GAMMA_MODE_MODE_10BIT;
 }
 
-static u32 bdw_csc_mode(const struct intel_crtc_state *crtc_state)
+static u32 ivb_csc_mode(const struct intel_crtc_state *crtc_state)
 {
+	bool limited_color_range = ilk_csc_limited_range(crtc_state);
+
 	/*
 	 * CSC comes after the LUT in degamma, RGB->YCbCr,
 	 * and RGB full->limited range mode.
 	 */
 	if (crtc_state->base.degamma_lut ||
 	    crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
-	    crtc_state->limited_color_range)
+	    limited_color_range)
 		return 0;
 
 	return CSC_POSITION_BEFORE_GAMMA;
 }
 
-static int bdw_color_check(struct intel_crtc_state *crtc_state)
+static int ivb_color_check(struct intel_crtc_state *crtc_state)
 {
+	bool limited_color_range = ilk_csc_limited_range(crtc_state);
 	int ret;
 
 	ret = check_luts(crtc_state);
@@ -973,11 +1042,11 @@ static int bdw_color_check(struct intel_crtc_state *crtc_state)
 
 	crtc_state->csc_enable =
 		crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB ||
-		crtc_state->base.ctm || crtc_state->limited_color_range;
+		crtc_state->base.ctm || limited_color_range;
 
-	crtc_state->gamma_mode = bdw_gamma_mode(crtc_state);
+	crtc_state->gamma_mode = ivb_gamma_mode(crtc_state);
 
-	crtc_state->csc_mode = bdw_csc_mode(crtc_state);
+	crtc_state->csc_mode = ivb_csc_mode(crtc_state);
 
 	ret = intel_color_add_affected_planes(crtc_state);
 	if (ret)
@@ -1094,8 +1163,8 @@ void intel_color_init(struct intel_crtc *crtc)
 			dev_priv->display.color_check = icl_color_check;
 		else if (INTEL_GEN(dev_priv) >= 10 || IS_GEMINILAKE(dev_priv))
 			dev_priv->display.color_check = glk_color_check;
-		else if (INTEL_GEN(dev_priv) >= 8)
-			dev_priv->display.color_check = bdw_color_check;
+		else if (INTEL_GEN(dev_priv) >= 7)
+			dev_priv->display.color_check = ivb_color_check;
 		else
 			dev_priv->display.color_check = ilk_color_check;
 
@@ -1110,8 +1179,10 @@ void intel_color_init(struct intel_crtc *crtc)
 			dev_priv->display.load_luts = icl_load_luts;
 		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
 			dev_priv->display.load_luts = glk_load_luts;
-		else if (INTEL_GEN(dev_priv) >= 9 || IS_BROADWELL(dev_priv))
+		else if (INTEL_GEN(dev_priv) >= 8)
 			dev_priv->display.load_luts = bdw_load_luts;
+		else if (INTEL_GEN(dev_priv) >= 7)
+			dev_priv->display.load_luts = ivb_load_luts;
 		else
 			dev_priv->display.load_luts = i9xx_load_luts;
 	}
-- 
2.19.2

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

[PATCH v2 4/7] drm/i915: Add 10bit LUT for ilk/snb

[Ville Syrjala]

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

Plop in support for 10bit LUT on ilk/snb.

There is no split gamma mode on these platforms, so we have
to choose between degamma and gamma. That could be a runtime choice
but for now let's just advertize the gamma as having 1024 entries.
We'll also keep the ctm hidden for now.

v2: Don't use I915_WRITE_FW() yet
    Introduce bool has_ctm (Maarten)
    Call drm_crtc_enable_color_mgmt() uncoditionally (Maarten)

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c    |  4 +++
 drivers/gpu/drm/i915/i915_reg.h    |  9 ++++++
 drivers/gpu/drm/i915/intel_color.c | 49 ++++++++++++++++++++++++------
 3 files changed, 52 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 2b0d2f4f8a46..cbcfe1a5de9a 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -116,6 +116,8 @@
 		[PIPE_C] = IVB_CURSOR_C_OFFSET, \
 	}
 
+#define ILK_COLORS \
+	.color = { .gamma_lut_size = 1024 }
 #define IVB_COLORS \
 	.color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }
 #define CHV_COLORS \
@@ -332,6 +334,7 @@ static const struct intel_device_info intel_gm45_info = {
 	.has_rc6 = 0, \
 	I9XX_PIPE_OFFSETS, \
 	I9XX_CURSOR_OFFSETS, \
+	ILK_COLORS, \
 	GEN_DEFAULT_PAGE_SIZES
 
 static const struct intel_device_info intel_ironlake_d_info = {
@@ -360,6 +363,7 @@ static const struct intel_device_info intel_ironlake_m_info = {
 	.ppgtt_size = 31, \
 	I9XX_PIPE_OFFSETS, \
 	I9XX_CURSOR_OFFSETS, \
+	ILK_COLORS, \
 	GEN_DEFAULT_PAGE_SIZES
 
 #define SNB_D_PLATFORM \
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index bed2c52aebd8..d9b2bd226e57 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7209,6 +7209,15 @@ enum {
 #define _LGC_PALETTE_B           0x4a800
 #define LGC_PALETTE(pipe, i) _MMIO(_PIPE(pipe, _LGC_PALETTE_A, _LGC_PALETTE_B) + (i) * 4)
 
+/* ilk/snb precision palette */
+#define _PREC_PALETTE_A           0x4b000
+#define _PREC_PALETTE_B           0x4c000
+#define PREC_PALETTE(pipe, i) _MMIO(_PIPE(pipe, _PREC_PALETTE_A, _PREC_PALETTE_B) + (i) * 4)
+
+#define  _PREC_PIPEAGCMAX              0x4d000
+#define  _PREC_PIPEBGCMAX              0x4d010
+#define PREC_PIPEGCMAX(pipe, i)        _MMIO(_PIPE(pipe, _PIPEAGCMAX, _PIPEBGCMAX) + (i) * 4)
+
 #define _GAMMA_MODE_A		0x4a480
 #define _GAMMA_MODE_B		0x4ac80
 #define GAMMA_MODE(pipe) _MMIO_PIPE(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index bb3b8afdb933..b9adcb4bbab7 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -468,6 +468,29 @@ static void skl_color_commit(const struct intel_crtc_state *crtc_state)
 		ilk_load_csc_matrix(crtc_state);
 }
 
+static void ilk_load_lut_10(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 = blob->data;
+	int i, lut_size = drm_color_lut_size(blob);
+	enum pipe pipe = crtc->pipe;
+
+	for (i = 0; i < lut_size; i++)
+		I915_WRITE(PREC_PALETTE(pipe, i), ilk_lut_10(&lut[i]));
+}
+
+static void ilk_load_luts(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
+
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
+		i9xx_load_luts(crtc_state);
+	else
+		ilk_load_lut_10(crtc, gamma_lut);
+}
+
 /*
  * IVB/HSW Bspec / PAL_PREC_INDEX:
  * "Restriction : Index auto increment mode is not
@@ -967,6 +990,15 @@ static int chv_color_check(struct intel_crtc_state *crtc_state)
 	return 0;
 }
 
+static u32 ilk_gamma_mode(const struct intel_crtc_state *crtc_state)
+{
+	if (!crtc_state->gamma_enable ||
+	    crtc_state_is_legacy_gamma(crtc_state))
+		return GAMMA_MODE_MODE_8BIT;
+	else
+		return GAMMA_MODE_MODE_10BIT;
+}
+
 static int ilk_color_check(struct intel_crtc_state *crtc_state)
 {
 	int ret;
@@ -986,8 +1018,7 @@ static int ilk_color_check(struct intel_crtc_state *crtc_state)
 	 */
 	crtc_state->csc_enable = false;
 
-	/* We don't expose fancy gamma modes on ilk/snb currently */
-	crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
+	crtc_state->gamma_mode = ilk_gamma_mode(crtc_state);
 
 	crtc_state->csc_mode = 0;
 
@@ -1145,6 +1176,7 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	bool has_ctm = INTEL_INFO(dev_priv)->color.degamma_lut_size != 0;
 
 	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
 
@@ -1184,14 +1216,11 @@ void intel_color_init(struct intel_crtc *crtc)
 		else if (INTEL_GEN(dev_priv) >= 7)
 			dev_priv->display.load_luts = ivb_load_luts;
 		else
-			dev_priv->display.load_luts = i9xx_load_luts;
+			dev_priv->display.load_luts = ilk_load_luts;
 	}
 
-	/* Enable color management support when we have degamma & gamma LUTs. */
-	if (INTEL_INFO(dev_priv)->color.degamma_lut_size != 0 &&
-	    INTEL_INFO(dev_priv)->color.gamma_lut_size != 0)
-		drm_crtc_enable_color_mgmt(&crtc->base,
-					   INTEL_INFO(dev_priv)->color.degamma_lut_size,
-					   true,
-					   INTEL_INFO(dev_priv)->color.gamma_lut_size);
+	drm_crtc_enable_color_mgmt(&crtc->base,
+				   INTEL_INFO(dev_priv)->color.degamma_lut_size,
+				   has_ctm,
+				   INTEL_INFO(dev_priv)->color.gamma_lut_size);
 }
-- 
2.19.2

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

[PATCH v2 5/7] drm/i915: Add "10.6" LUT mode for i965+

[Ville Syrjala]

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

i965+ have an interpolate 10bit LUT mode. Let's expose that so
that we can actually enjoy real 10bpc.

v2: Don't use I915_WRITE_FW() yet

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c    |  6 +++
 drivers/gpu/drm/i915/i915_reg.h    |  4 ++
 drivers/gpu/drm/i915/intel_color.c | 62 +++++++++++++++++++++++++++++-
 3 files changed, 71 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index cbcfe1a5de9a..84078fbdb2d8 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -116,6 +116,10 @@
 		[PIPE_C] = IVB_CURSOR_C_OFFSET, \
 	}
 
+#define I965_COLORS \
+	.color = { .gamma_lut_size = 129, \
+		   .gamma_lut_tests = DRM_COLOR_LUT_NON_DECREASING, \
+	}
 #define ILK_COLORS \
 	.color = { .gamma_lut_size = 1024 }
 #define IVB_COLORS \
@@ -285,6 +289,7 @@ static const struct intel_device_info intel_pineview_m_info = {
 	.has_coherent_ggtt = true, \
 	I9XX_PIPE_OFFSETS, \
 	I9XX_CURSOR_OFFSETS, \
+	I965_COLORS, \
 	GEN_DEFAULT_PAGE_SIZES
 
 static const struct intel_device_info intel_i965g_info = {
@@ -469,6 +474,7 @@ static const struct intel_device_info intel_valleyview_info = {
 	.display_mmio_offset = VLV_DISPLAY_BASE,
 	I9XX_PIPE_OFFSETS,
 	I9XX_CURSOR_OFFSETS,
+	I965_COLORS,
 	GEN_DEFAULT_PAGE_SIZES,
 };
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d9b2bd226e57..00e03560c4e7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5795,6 +5795,10 @@ enum {
 #define PIPEFRAMEPIXEL(pipe)	_MMIO_PIPE2(pipe, _PIPEAFRAMEPIXEL)
 #define PIPESTAT(pipe)		_MMIO_PIPE2(pipe, _PIPEASTAT)
 
+#define  _PIPEAGCMAX           0x70010
+#define  _PIPEBGCMAX           0x71010
+#define PIPEGCMAX(pipe, i)     _MMIO_PIPE2(pipe, _PIPEAGCMAX + (i) * 4)
+
 #define _PIPE_MISC_A			0x70030
 #define _PIPE_MISC_B			0x71030
 #define   PIPEMISC_YUV420_ENABLE	(1 << 27)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index b9adcb4bbab7..faebd0705adb 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -359,6 +359,22 @@ static void cherryview_load_csc_matrix(const struct intel_crtc_state *crtc_state
 	I915_WRITE(CGM_PIPE_MODE(pipe), crtc_state->cgm_mode);
 }
 
+/* i965+ "10.6" bit interpolated format "even DW" (low 8 bits) */
+static u32 i965_lut_10p6_ldw(const struct drm_color_lut *color)
+{
+	return (color->red & 0xff) << 16 |
+		(color->green & 0xff) << 8 |
+		(color->blue & 0xff);
+}
+
+/* i965+ "10.6" interpolated format "odd DW" (high 8 bits) */
+static u32 i965_lut_10p6_udw(const struct drm_color_lut *color)
+{
+	return (color->red >> 8) << 16 |
+		(color->green >> 8) << 8 |
+		(color->blue >> 8);
+}
+
 static u32 ilk_lut_10(const struct drm_color_lut *color)
 {
 	return drm_color_lut_extract(color->red, 10) << 20 |
@@ -468,6 +484,37 @@ static void skl_color_commit(const struct intel_crtc_state *crtc_state)
 		ilk_load_csc_matrix(crtc_state);
 }
 
+static void i965_load_lut_10p6(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 = blob->data;
+	int i, lut_size = drm_color_lut_size(blob);
+	enum pipe pipe = crtc->pipe;
+
+	for (i = 0; i < lut_size - 1; i++) {
+		I915_WRITE(PALETTE(pipe, 2 * i + 0),
+			   i965_lut_10p6_ldw(&lut[i]));
+		I915_WRITE(PALETTE(pipe, 2 * i + 1),
+			   i965_lut_10p6_udw(&lut[i]));
+	}
+
+	I915_WRITE(PIPEGCMAX(pipe, 0), lut[i].red);
+	I915_WRITE(PIPEGCMAX(pipe, 1), lut[i].green);
+	I915_WRITE(PIPEGCMAX(pipe, 2), lut[i].blue);
+}
+
+static void i965_load_luts(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
+
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
+		i9xx_load_luts(crtc_state);
+	else
+		i965_load_lut_10p6(crtc, gamma_lut);
+}
+
 static void ilk_load_lut_10(struct intel_crtc *crtc,
 			    const struct drm_property_blob *blob)
 {
@@ -917,6 +964,15 @@ static int check_luts(const struct intel_crtc_state *crtc_state)
 	return 0;
 }
 
+static u32 i9xx_gamma_mode(struct intel_crtc_state *crtc_state)
+{
+	if (!crtc_state->gamma_enable ||
+	    crtc_state_is_legacy_gamma(crtc_state))
+		return GAMMA_MODE_MODE_8BIT;
+	else
+		return GAMMA_MODE_MODE_10BIT; /* i965+ only */
+}
+
 static int i9xx_color_check(struct intel_crtc_state *crtc_state)
 {
 	int ret;
@@ -929,7 +985,7 @@ static int i9xx_color_check(struct intel_crtc_state *crtc_state)
 		crtc_state->base.gamma_lut &&
 		!crtc_state->c8_planes;
 
-	crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
+	crtc_state->gamma_mode = i9xx_gamma_mode(crtc_state);
 
 	ret = intel_color_add_affected_planes(crtc_state);
 	if (ret)
@@ -1185,6 +1241,10 @@ void intel_color_init(struct intel_crtc *crtc)
 			dev_priv->display.color_check = chv_color_check;
 			dev_priv->display.color_commit = i9xx_color_commit;
 			dev_priv->display.load_luts = cherryview_load_luts;
+		} else if (INTEL_GEN(dev_priv) >= 4) {
+			dev_priv->display.color_check = i9xx_color_check;
+			dev_priv->display.color_commit = i9xx_color_commit;
+			dev_priv->display.load_luts = i965_load_luts;
 		} else {
 			dev_priv->display.color_check = i9xx_color_check;
 			dev_priv->display.color_commit = i9xx_color_commit;
-- 
2.19.2

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

[PATCH v2 6/7] drm/i915: Expose the legacy LUT via the GAMMA_LUT/GAMMA_LUT_SIZE props on gen2/3

[Ville Syrjala]

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

Just so we don't leave gen2/3 out in the cold let's advertize the
legacy LUT via the GAMMA_LUT/GAMMA_LUT_SIZE props. Without the
GAMMA_LUT prop we can't actually load a LUT using the atomic ioctl
(in preparation for the day of 100% atomic driver).

Supposedly some gen2/3 platforms have an interpolated 10bit gamma mode
as well. It's slightly funkier than the i965+ mode since you have to
specify the slope for the interpolation by hand. But when I tried it
I couldn't get it to work, the hardware just insisted on using the
8bit more regardless of the state of the relevant PIPECONF bit.

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 84078fbdb2d8..81d14dc2fa61 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -116,6 +116,8 @@
 		[PIPE_C] = IVB_CURSOR_C_OFFSET, \
 	}
 
+#define I9XX_COLORS \
+	.color = { .gamma_lut_size = 256 }
 #define I965_COLORS \
 	.color = { .gamma_lut_size = 129, \
 		   .gamma_lut_tests = DRM_COLOR_LUT_NON_DECREASING, \
@@ -156,6 +158,7 @@
 	.has_coherent_ggtt = false, \
 	I9XX_PIPE_OFFSETS, \
 	I9XX_CURSOR_OFFSETS, \
+	I9XX_COLORS, \
 	GEN_DEFAULT_PAGE_SIZES
 
 #define I845_FEATURES \
@@ -172,6 +175,7 @@
 	.has_coherent_ggtt = false, \
 	I845_PIPE_OFFSETS, \
 	I845_CURSOR_OFFSETS, \
+	I9XX_COLORS, \
 	GEN_DEFAULT_PAGE_SIZES
 
 static const struct intel_device_info intel_i830_info = {
@@ -205,6 +209,7 @@ static const struct intel_device_info intel_i865g_info = {
 	.has_coherent_ggtt = true, \
 	I9XX_PIPE_OFFSETS, \
 	I9XX_CURSOR_OFFSETS, \
+	I9XX_COLORS, \
 	GEN_DEFAULT_PAGE_SIZES
 
 static const struct intel_device_info intel_i915g_info = {
-- 
2.19.2

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

[PATCH v2 7/7] drm/i915: Expose full 1024 LUT entries on ivb+

[Ville Syrjala]

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

On ivb+ we can select between the regular 10bit LUT mode with
1024 entries, and the split mode where the LUT is split into
seprate degamma and gamma halves (each with 512 entries). Currently
we expose the split gamma size of 512 as the GAMMA/DEGAMMA_LUT_SIZE.

When using only degamma or gamma (not both) we are wasting half of
the hardware LUT entries. Let's flip that around so that we expose
the full 1024 entries and just throw away half of the user provided
entries when using the split gamma mode.

Cc: Matt Roper <matthew.d.roper@intel.com>
Suggested-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c    |  2 +-
 drivers/gpu/drm/i915/intel_color.c | 75 +++++++++++++-----------------
 2 files changed, 34 insertions(+), 43 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 81d14dc2fa61..6ffb85ddac53 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -125,7 +125,7 @@
 #define ILK_COLORS \
 	.color = { .gamma_lut_size = 1024 }
 #define IVB_COLORS \
-	.color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }
+	.color = { .degamma_lut_size = 1024, .gamma_lut_size = 1024 }
 #define CHV_COLORS \
 	.color = { .degamma_lut_size = 65, .gamma_lut_size = 257, \
 		   .degamma_lut_tests = DRM_COLOR_LUT_NON_DECREASING, \
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index faebd0705adb..60f21a1fdbbe 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -538,6 +538,14 @@ static void ilk_load_luts(const struct intel_crtc_state *crtc_state)
 		ilk_load_lut_10(crtc, gamma_lut);
 }
 
+static int ivb_lut_10_size(u32 prec_index)
+{
+	if (prec_index & PAL_PREC_SPLIT_MODE)
+		return 512;
+	else
+		return 1024;
+}
+
 /*
  * IVB/HSW Bspec / PAL_PREC_INDEX:
  * "Restriction : Index auto increment mode is not
@@ -545,31 +553,21 @@ static void ilk_load_luts(const struct intel_crtc_state *crtc_state)
  */
 static void ivb_load_lut_10(struct intel_crtc *crtc,
 			    const struct drm_property_blob *blob,
-			    u32 prec_index, bool duplicate)
+			    u32 prec_index)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	int hw_lut_size = ivb_lut_10_size(prec_index);
 	const struct drm_color_lut *lut = blob->data;
 	int i, lut_size = drm_color_lut_size(blob);
 	enum pipe pipe = crtc->pipe;
 
-	/*
-	 * We advertize the split gamma sizes. When not using split
-	 * gamma we just duplicate each entry.
-	 *
-	 * TODO: expose the full LUT to userspace
-	 */
-	if (duplicate) {
-		for (i = 0; i < lut_size; i++) {
-			I915_WRITE(PREC_PAL_INDEX(pipe), prec_index++);
-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
-			I915_WRITE(PREC_PAL_INDEX(pipe), prec_index++);
-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
-		}
-	} else {
-		for (i = 0; i < lut_size; i++) {
-			I915_WRITE(PREC_PAL_INDEX(pipe), prec_index++);
-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
-		}
+	for (i = 0; i < hw_lut_size; i++) {
+		/* We discard half the user entries in split gamma mode */
+		const struct drm_color_lut *entry =
+			&lut[i * (lut_size - 1) / (hw_lut_size - 1)];
+
+		I915_WRITE(PREC_PAL_INDEX(pipe), prec_index++);
+		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(entry));
 	}
 
 	/*
@@ -582,9 +580,10 @@ static void ivb_load_lut_10(struct intel_crtc *crtc,
 /* On BDW+ the index auto increment mode actually works */
 static void bdw_load_lut_10(struct intel_crtc *crtc,
 			    const struct drm_property_blob *blob,
-			    u32 prec_index, bool duplicate)
+			    u32 prec_index)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	int hw_lut_size = ivb_lut_10_size(prec_index);
 	const struct drm_color_lut *lut = blob->data;
 	int i, lut_size = drm_color_lut_size(blob);
 	enum pipe pipe = crtc->pipe;
@@ -592,20 +591,12 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
 	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
 		   PAL_PREC_AUTO_INCREMENT);
 
-	/*
-	 * We advertize the split gamma sizes. When not using split
-	 * gamma we just duplicate each entry.
-	 *
-	 * TODO: expose the full LUT to userspace
-	 */
-	if (duplicate) {
-		for (i = 0; i < lut_size; i++) {
-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
-		}
-	} else {
-		for (i = 0; i < lut_size; i++)
-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
+	for (i = 0; i < hw_lut_size; i++) {
+		/* We discard half the user entries in split gamma mode */
+		const struct drm_color_lut *entry =
+			&lut[i * (lut_size - 1) / (hw_lut_size - 1)];
+
+		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(entry));
 	}
 
 	/*
@@ -647,15 +638,15 @@ static void ivb_load_luts(const struct intel_crtc_state *crtc_state)
 		i9xx_load_luts(crtc_state);
 	} 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), false);
+				PAL_PREC_INDEX_VALUE(0));
 		ivb_load_lut_10_max(crtc);
 		ivb_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
-				PAL_PREC_INDEX_VALUE(512),  false);
+				PAL_PREC_INDEX_VALUE(512));
 	} else {
 		const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
 
 		ivb_load_lut_10(crtc, blob,
-				PAL_PREC_INDEX_VALUE(0), true);
+				PAL_PREC_INDEX_VALUE(0));
 		ivb_load_lut_10_max(crtc);
 	}
 }
@@ -670,15 +661,15 @@ static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
 		i9xx_load_luts(crtc_state);
 	} 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), false);
+				PAL_PREC_INDEX_VALUE(0));
 		ivb_load_lut_10_max(crtc);
 		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
-				PAL_PREC_INDEX_VALUE(512),  false);
+				PAL_PREC_INDEX_VALUE(512));
 	} else {
 		const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
 
 		bdw_load_lut_10(crtc, blob,
-				PAL_PREC_INDEX_VALUE(0), true);
+				PAL_PREC_INDEX_VALUE(0));
 		ivb_load_lut_10_max(crtc);
 	}
 }
@@ -770,7 +761,7 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
 		i9xx_load_luts(crtc_state);
 	} else {
-		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0), false);
+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
 		ivb_load_lut_10_max(crtc);
 	}
 }
@@ -787,7 +778,7 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
 	    GAMMA_MODE_MODE_8BIT) {
 		i9xx_load_luts(crtc_state);
 	} else {
-		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0), false);
+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
 		ivb_load_lut_10_max(crtc);
 	}
 }
-- 
2.19.2

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

Re: [PATCH v2 7/7] drm/i915: Expose full 1024 LUT entries on ivb+

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjala [mailto:ville.syrjala@linux.intel.com]
>Sent: Tuesday, April 2, 2019 1:33 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Shankar, Uma <uma.shankar@intel.com>; Roper, Matthew D
><matthew.d.roper@intel.com>
>Subject: [PATCH v2 7/7] drm/i915: Expose full 1024 LUT entries on ivb+
>
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>On ivb+ we can select between the regular 10bit LUT mode with
>1024 entries, and the split mode where the LUT is split into seprate degamma and
>gamma halves (each with 512 entries). Currently we expose the split gamma size of
>512 as the GAMMA/DEGAMMA_LUT_SIZE.
>
>When using only degamma or gamma (not both) we are wasting half of the hardware
>LUT entries. Let's flip that around so that we expose the full 1024 entries and just
>throw away half of the user provided entries when using the split gamma mode.

Changes look good to me.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

>Cc: Matt Roper <matthew.d.roper@intel.com>
>Suggested-by: Matt Roper <matthew.d.roper@intel.com>
>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>---
> drivers/gpu/drm/i915/i915_pci.c    |  2 +-
> drivers/gpu/drm/i915/intel_color.c | 75 +++++++++++++-----------------
> 2 files changed, 34 insertions(+), 43 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index
>81d14dc2fa61..6ffb85ddac53 100644
>--- a/drivers/gpu/drm/i915/i915_pci.c
>+++ b/drivers/gpu/drm/i915/i915_pci.c
>@@ -125,7 +125,7 @@
> #define ILK_COLORS \
> 	.color = { .gamma_lut_size = 1024 }
> #define IVB_COLORS \
>-	.color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }
>+	.color = { .degamma_lut_size = 1024, .gamma_lut_size = 1024 }
> #define CHV_COLORS \
> 	.color = { .degamma_lut_size = 65, .gamma_lut_size = 257, \
> 		   .degamma_lut_tests = DRM_COLOR_LUT_NON_DECREASING, \
>diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>index faebd0705adb..60f21a1fdbbe 100644
>--- a/drivers/gpu/drm/i915/intel_color.c
>+++ b/drivers/gpu/drm/i915/intel_color.c
>@@ -538,6 +538,14 @@ static void ilk_load_luts(const struct intel_crtc_state
>*crtc_state)
> 		ilk_load_lut_10(crtc, gamma_lut);
> }
>
>+static int ivb_lut_10_size(u32 prec_index) {
>+	if (prec_index & PAL_PREC_SPLIT_MODE)
>+		return 512;
>+	else
>+		return 1024;
>+}
>+
> /*
>  * IVB/HSW Bspec / PAL_PREC_INDEX:
>  * "Restriction : Index auto increment mode is not @@ -545,31 +553,21 @@ static
>void ilk_load_luts(const struct intel_crtc_state *crtc_state)
>  */
> static void ivb_load_lut_10(struct intel_crtc *crtc,
> 			    const struct drm_property_blob *blob,
>-			    u32 prec_index, bool duplicate)
>+			    u32 prec_index)
> {
> 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>+	int hw_lut_size = ivb_lut_10_size(prec_index);
> 	const struct drm_color_lut *lut = blob->data;
> 	int i, lut_size = drm_color_lut_size(blob);
> 	enum pipe pipe = crtc->pipe;
>
>-	/*
>-	 * We advertize the split gamma sizes. When not using split
>-	 * gamma we just duplicate each entry.
>-	 *
>-	 * TODO: expose the full LUT to userspace
>-	 */
>-	if (duplicate) {
>-		for (i = 0; i < lut_size; i++) {
>-			I915_WRITE(PREC_PAL_INDEX(pipe), prec_index++);
>-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
>-			I915_WRITE(PREC_PAL_INDEX(pipe), prec_index++);
>-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
>-		}
>-	} else {
>-		for (i = 0; i < lut_size; i++) {
>-			I915_WRITE(PREC_PAL_INDEX(pipe), prec_index++);
>-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
>-		}
>+	for (i = 0; i < hw_lut_size; i++) {
>+		/* We discard half the user entries in split gamma mode */
>+		const struct drm_color_lut *entry =
>+			&lut[i * (lut_size - 1) / (hw_lut_size - 1)];
>+
>+		I915_WRITE(PREC_PAL_INDEX(pipe), prec_index++);
>+		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(entry));
> 	}
>
> 	/*
>@@ -582,9 +580,10 @@ static void ivb_load_lut_10(struct intel_crtc *crtc,
> /* On BDW+ the index auto increment mode actually works */  static void
>bdw_load_lut_10(struct intel_crtc *crtc,
> 			    const struct drm_property_blob *blob,
>-			    u32 prec_index, bool duplicate)
>+			    u32 prec_index)
> {
> 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>+	int hw_lut_size = ivb_lut_10_size(prec_index);
> 	const struct drm_color_lut *lut = blob->data;
> 	int i, lut_size = drm_color_lut_size(blob);
> 	enum pipe pipe = crtc->pipe;
>@@ -592,20 +591,12 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
> 	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
> 		   PAL_PREC_AUTO_INCREMENT);
>
>-	/*
>-	 * We advertize the split gamma sizes. When not using split
>-	 * gamma we just duplicate each entry.
>-	 *
>-	 * TODO: expose the full LUT to userspace
>-	 */
>-	if (duplicate) {
>-		for (i = 0; i < lut_size; i++) {
>-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
>-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
>-		}
>-	} else {
>-		for (i = 0; i < lut_size; i++)
>-			I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(&lut[i]));
>+	for (i = 0; i < hw_lut_size; i++) {
>+		/* We discard half the user entries in split gamma mode */
>+		const struct drm_color_lut *entry =
>+			&lut[i * (lut_size - 1) / (hw_lut_size - 1)];
>+
>+		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_10(entry));
> 	}
>
> 	/*
>@@ -647,15 +638,15 @@ static void ivb_load_luts(const struct intel_crtc_state
>*crtc_state)
> 		i9xx_load_luts(crtc_state);
> 	} 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), false);
>+				PAL_PREC_INDEX_VALUE(0));
> 		ivb_load_lut_10_max(crtc);
> 		ivb_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
>-				PAL_PREC_INDEX_VALUE(512),  false);
>+				PAL_PREC_INDEX_VALUE(512));
> 	} else {
> 		const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
>
> 		ivb_load_lut_10(crtc, blob,
>-				PAL_PREC_INDEX_VALUE(0), true);
>+				PAL_PREC_INDEX_VALUE(0));
> 		ivb_load_lut_10_max(crtc);
> 	}
> }
>@@ -670,15 +661,15 @@ static void bdw_load_luts(const struct intel_crtc_state
>*crtc_state)
> 		i9xx_load_luts(crtc_state);
> 	} 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), false);
>+				PAL_PREC_INDEX_VALUE(0));
> 		ivb_load_lut_10_max(crtc);
> 		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
>-				PAL_PREC_INDEX_VALUE(512),  false);
>+				PAL_PREC_INDEX_VALUE(512));
> 	} else {
> 		const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
>
> 		bdw_load_lut_10(crtc, blob,
>-				PAL_PREC_INDEX_VALUE(0), true);
>+				PAL_PREC_INDEX_VALUE(0));
> 		ivb_load_lut_10_max(crtc);
> 	}
> }
>@@ -770,7 +761,7 @@ static void glk_load_luts(const struct intel_crtc_state
>*crtc_state)
> 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
> 		i9xx_load_luts(crtc_state);
> 	} else {
>-		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0),
>false);
>+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
> 		ivb_load_lut_10_max(crtc);
> 	}
> }
>@@ -787,7 +778,7 @@ static void icl_load_luts(const struct intel_crtc_state
>*crtc_state)
> 	    GAMMA_MODE_MODE_8BIT) {
> 		i9xx_load_luts(crtc_state);
> 	} else {
>-		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0),
>false);
>+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
> 		ivb_load_lut_10_max(crtc);
> 	}
> }
>--
>2.19.2

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

✓ Fi.CI.BAT: success for drm/i915: Finish the GAMMA_LUT stuff (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Finish the GAMMA_LUT stuff (rev3)
URL   : https://patchwork.freedesktop.org/series/58698/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5856 -> Patchwork_12654
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58698/revisions/3/mbox/

Known issues
------------

  Here are the changes found in Patchwork_12654 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@fork-compute0:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109315] +17

  * igt@amdgpu/amd_cs_nop@fork-gfx0:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109315] +17

  * igt@gem_exec_basic@basic-bsd2:
    - fi-kbl-7500u:       NOTRUN -> SKIP [fdo#109271] +9
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109276] +7

  * igt@gem_exec_basic@readonly-bsd1:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109276] +7

  * igt@gem_exec_parse@basic-allowed:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109289] +1

  * igt@gem_exec_parse@basic-rejected:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109289] +1

  * igt@i915_selftest@live_contexts:
    - fi-icl-u2:          NOTRUN -> DMESG-FAIL [fdo#108569]
    - fi-icl-y:           NOTRUN -> DMESG-FAIL [fdo#108569]

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         NOTRUN -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@i915_selftest@live_hangcheck:
    - fi-icl-u3:          NOTRUN -> INCOMPLETE [fdo#108569]

  * igt@kms_chamelium@dp-crc-fast:
    - fi-kbl-7500u:       NOTRUN -> DMESG-WARN [fdo#103841]
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109284] +8

  * igt@kms_chamelium@dp-edid-read:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109316] +2

  * igt@kms_chamelium@vga-hpd-fast:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109309] +1

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109285] +3

  * igt@kms_force_connector_basic@prune-stale-modes:
    - fi-icl-u2:          NOTRUN -> SKIP [fdo#109285] +3

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u2:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

  * igt@kms_psr@primary_mmap_gtt:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#110189] +3
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] +23

  * igt@kms_psr@primary_page_flip:
    - fi-apl-guc:         NOTRUN -> SKIP [fdo#109271] +50

  * igt@prime_vgem@basic-fence-flip:
    - fi-icl-y:           NOTRUN -> SKIP [fdo#109294]

  * igt@runner@aborted:
    - fi-kbl-7500u:       NOTRUN -> FAIL [fdo#103841]

  
#### Possible fixes ####

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS
    - fi-skl-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     FAIL [fdo#103167] -> PASS

  * igt@kms_pipe_crc_basic@read-crc-pipe-b-frame-sequence:
    - fi-byt-clapper:     FAIL [fdo#103191] / [fdo#107362] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-byt-clapper:     INCOMPLETE [fdo#102657] -> PASS

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - fi-icl-u3:          INCOMPLETE [fdo#108569] -> DMESG-FAIL [fdo#108569]

  
  [fdo#102657]: https://bugs.freedesktop.org/show_bug.cgi?id=102657
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103841]: https://bugs.freedesktop.org/show_bug.cgi?id=103841
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109294]: https://bugs.freedesktop.org/show_bug.cgi?id=109294
  [fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109316]: https://bugs.freedesktop.org/show_bug.cgi?id=109316
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


Participating hosts (43 -> 42)
------------------------------

  Additional (4): fi-icl-y fi-icl-u2 fi-apl-guc fi-kbl-7500u 
  Missing    (5): fi-ilk-m540 fi-bsw-n3050 fi-byt-squawks fi-skl-6260u fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5856 -> Patchwork_12654

  CI_DRM_5856: 55074bd825098a71779cf65a69786547f0eccbe9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4922: e941e4a29438c7130554492e4daf52afbc99ffdf @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12654: fcdd08664dd489e5a06e1ab8d0093cedfe83fde0 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

fcdd08664dd4 drm/i915: Expose full 1024 LUT entries on ivb+
d40496feed6f drm/i915: Expose the legacy LUT via the GAMMA_LUT/GAMMA_LUT_SIZE props on gen2/3
73257ae76739 drm/i915: Add "10.6" LUT mode for i965+
4d4dcba28c7e drm/i915: Add 10bit LUT for ilk/snb
bd997f8b4491 drm/i915: Implement split/10bit gamma for ivb/hsw
6c27455bbe0a drm/i915: Don't use split gamma when we don't have to
90ca0b6033a6 drm/i915: Extract ilk_lut_10()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12654/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for drm/i915: Finish the GAMMA_LUT stuff (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Finish the GAMMA_LUT stuff (rev3)
URL   : https://patchwork.freedesktop.org/series/58698/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5856_full -> Patchwork_12654_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Known issues
------------

  Here are the changes found in Patchwork_12654_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@preempt-self-bsd1:
    - shard-iclb:         NOTRUN -> SKIP [fdo#108767] / [fdo#109276]

  * igt@gem_exec_store@cachelines-bsd2:
    - shard-glk:          NOTRUN -> SKIP [fdo#108767] / [fdo#109271] +20

  * igt@gem_exec_suspend@basic-s4-devices:
    - shard-iclb:         PASS -> FAIL [fdo#108767] / [fdo#109779]

  * igt@gem_fence_thrash@bo-write-verify-threaded-x:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#108767] / [fdo#109100] +1

  * igt@gem_pwrite@stolen-normal:
    - shard-skl:          NOTRUN -> SKIP [fdo#108767] / [fdo#109271] +102

  * igt@gem_softpin@noreloc-s3:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773] / [fdo#108767]

  * igt@gem_stolen@stolen-no-mmap:
    - shard-iclb:         NOTRUN -> SKIP [fdo#108767] / [fdo#109277]

  * igt@i915_pm_rpm@i2c:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#108767] / [fdo#109982]

  * igt@i915_pm_rpm@modeset-non-lpsp-stress:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#107807] / [fdo#108767]

  * igt@i915_pm_rpm@reg-read-ioctl:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107807] / [fdo#108767]

  * igt@i915_pm_rpm@system-suspend-modeset:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#104108] / [fdo#107807] / [fdo#108767]

  * igt@i915_pm_sseu@full-enable:
    - shard-iclb:         NOTRUN -> SKIP [fdo#108767] / [fdo#109288]

  * igt@i915_selftest@live_workarounds:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#108767] / [fdo#108954]

  * igt@kms_atomic_transition@3x-modeset-transitions:
    - shard-skl:          NOTRUN -> SKIP [fdo#108767] / [fdo#109271] / [fdo#109278] +9

  * igt@kms_busy@basic-modeset-e:
    - shard-kbl:          NOTRUN -> SKIP [fdo#108767] / [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#108767] / [fdo#110222]

  * igt@kms_color@pipe-invalid-lut-sizes:
    - shard-snb:          PASS -> FAIL [fdo#108767]

  * igt@kms_concurrent@pipe-f:
    - shard-iclb:         NOTRUN -> SKIP [fdo#108767] / [fdo#109278] +1

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          PASS -> FAIL [fdo#105363] / [fdo#108767]

  * igt@kms_flip@2x-wf_vblank-ts-check:
    - shard-snb:          NOTRUN -> SKIP [fdo#108767] / [fdo#109271] +73

  * igt@kms_force_connector_basic@force-connector-state:
    - shard-iclb:         NOTRUN -> SKIP [fdo#108767] / [fdo#109285]

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-pwrite:
    - shard-iclb:         PASS -> FAIL [fdo#108767] / [fdo#109247] +18

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite:
    - shard-iclb:         PASS -> FAIL [fdo#103167] / [fdo#108767] +4

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-cur-indfb-draw-mmap-cpu:
    - shard-iclb:         NOTRUN -> SKIP [fdo#108767] / [fdo#109280] +1

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-kbl:          NOTRUN -> SKIP [fdo#108767] / [fdo#109271] +21

  * igt@kms_frontbuffer_tracking@fbcpsr-stridechange:
    - shard-skl:          NOTRUN -> FAIL [fdo#105683] / [fdo#108767]

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-render:
    - shard-skl:          NOTRUN -> FAIL [fdo#103167] / [fdo#108767]

  * igt@kms_lease@atomic_implicit_crtc:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108767] / [fdo#110279]
    - shard-skl:          NOTRUN -> FAIL [fdo#108767] / [fdo#110279]

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#108767]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145] / [fdo#108767] +1
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145] / [fdo#108590] / [fdo#108767]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-transparant-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145] / [fdo#108767] +1

  * igt@kms_plane_scaling@pipe-a-scaler-with-clipping-clamping:
    - shard-glk:          PASS -> SKIP [fdo#108767] / [fdo#109271] / [fdo#109278] +1

  * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
    - shard-glk:          NOTRUN -> SKIP [fdo#108767] / [fdo#109271] / [fdo#109278] +2

  * igt@kms_psr2_su@page_flip:
    - shard-iclb:         PASS -> SKIP [fdo#108767] / [fdo#109642]

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         PASS -> SKIP [fdo#108767] / [fdo#109441] +1

  * igt@kms_psr@sprite_mmap_cpu:
    - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#108767] / [fdo#110215] +1

  * igt@kms_setmode@basic:
    - shard-apl:          PASS -> FAIL [fdo#108767] / [fdo#99912]
    - shard-iclb:         NOTRUN -> FAIL [fdo#108767] / [fdo#99912]

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-rpm:
    - shard-snb:          NOTRUN -> SKIP [fdo#108767] / [fdo#109271] / [fdo#109278] +9

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-iclb:         PASS -> FAIL [fdo#104894] / [fdo#108767] +1

  * igt@kms_vblank@pipe-c-ts-continuation-modeset-hang:
    - shard-apl:          PASS -> FAIL [fdo#104894] / [fdo#108767] +1

  * igt@tools_test@tools_test:
    - shard-snb:          PASS -> SKIP [fdo#108767] / [fdo#109271]

  
#### Possible fixes ####

  * igt@gem_create@create-clear:
    - shard-snb:          INCOMPLETE [fdo#105411] -> PASS

  * igt@gem_ppgtt@blt-vs-render-ctxn:
    - shard-iclb:         INCOMPLETE [fdo#109801] -> PASS

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         DMESG-WARN [fdo#108686] -> PASS

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-glk:          DMESG-WARN [fdo#110222] -> PASS

  * igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
    - shard-glk:          FAIL [fdo#106509] / [fdo#107409] -> PASS

  * igt@kms_flip_tiling@flip-to-x-tiled:
    - shard-iclb:         FAIL [fdo#108134] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-mmap-gtt:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +17

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-indfb-draw-blt:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +18

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-wc:
    - shard-iclb:         FAIL [fdo#105682] / [fdo#109247] -> PASS +1

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          FAIL [fdo#107815] -> PASS

  * igt@kms_plane_scaling@pipe-a-scaler-with-rotation:
    - shard-glk:          SKIP [fdo#109271] / [fdo#109278] -> PASS +1

  * igt@kms_psr@no_drrs:
    - shard-iclb:         FAIL [fdo#108341] -> PASS

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +3

  * igt@kms_psr@sprite_blt:
    - shard-iclb:         FAIL [fdo#107383] / [fdo#110215] -> PASS +5

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-iclb:         FAIL [fdo#104894] -> PASS

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#105683]: https://bugs.freedesktop.org/show_bug.cgi?id=105683
  [fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107409]: https://bugs.freedesktop.org/show_bug.cgi?id=107409
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#108134]: https://bugs.freedesktop.org/show_bug.cgi?id=108134
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109277]: https://bugs.freedesktop.org/show_bug.cgi?id=109277
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109288]: https://bugs.freedesktop.org/show_bug.cgi?id=109288
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#109779]: https://bugs.freedesktop.org/show_bug.cgi?id=109779
  [fdo#109801]: https://bugs.freedesktop.org/show_bug.cgi?id=109801
  [fdo#109982]: https://bugs.freedesktop.org/show_bug.cgi?id=109982
  [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
  [fdo#110222]: https://bugs.freedesktop.org/show_bug.cgi?id=110222
  [fdo#110279]: https://bugs.freedesktop.org/show_bug.cgi?id=110279
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


Build changes
-------------

    * Linux: CI_DRM_5856 -> Patchwork_12654

  CI_DRM_5856: 55074bd825098a71779cf65a69786547f0eccbe9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4922: e941e4a29438c7130554492e4daf52afbc99ffdf @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12654: fcdd08664dd489e5a06e1ab8d0093cedfe83fde0 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12654/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 0/7] drm/i915: Finish the GAMMA_LUT stuff

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjala [mailto:ville.syrjala@linux.intel.com]
>Sent: Tuesday, April 2, 2019 1:32 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Shankar, Uma <uma.shankar@intel.com>; Roper, Matthew D
><matthew.d.roper@intel.com>
>Subject: [PATCH v2 0/7] drm/i915: Finish the GAMMA_LUT stuff
>
>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
>Rebased due to Uma's EXT_GC_MAX fix, and I added Matt's proposed behavioural
>change (expose 1024 entry LUTs in split gamma mode and just discard half the
>entries) as an extra patch on top.
>
>Everything is reviewed except patches 2 and 7.

Reviewed the whole series and it looks perfect.
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

>
>Ville Syrjälä (7):
>  drm/i915: Extract ilk_lut_10()
>  drm/i915: Don't use split gamma when we don't have to
>  drm/i915: Implement split/10bit gamma for ivb/hsw
>  drm/i915: Add 10bit LUT for ilk/snb
>  drm/i915: Add "10.6" LUT mode for i965+
>  drm/i915: Expose the legacy LUT via the GAMMA_LUT/GAMMA_LUT_SIZE props
>    on gen2/3
>  drm/i915: Expose full 1024 LUT entries on ivb+
>
> drivers/gpu/drm/i915/i915_pci.c    |  23 +-
> drivers/gpu/drm/i915/i915_reg.h    |  15 ++
> drivers/gpu/drm/i915/intel_color.c | 375 ++++++++++++++++++++---------
> 3 files changed, 292 insertions(+), 121 deletions(-)
>
>--
>2.19.2

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

Re: [PATCH v2 0/7] drm/i915: Finish the GAMMA_LUT stuff

[Ville Syrjälä]

On Wed, Apr 03, 2019 at 01:57:00PM +0000, Shankar, Uma wrote:
> 
> 
> >-----Original Message-----
> >From: Ville Syrjala [mailto:ville.syrjala@linux.intel.com]
> >Sent: Tuesday, April 2, 2019 1:32 AM
> >To: intel-gfx@lists.freedesktop.org
> >Cc: Shankar, Uma <uma.shankar@intel.com>; Roper, Matthew D
> ><matthew.d.roper@intel.com>
> >Subject: [PATCH v2 0/7] drm/i915: Finish the GAMMA_LUT stuff
> >
> >From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> >Rebased due to Uma's EXT_GC_MAX fix, and I added Matt's proposed behavioural
> >change (expose 1024 entry LUTs in split gamma mode and just discard half the
> >entries) as an extra patch on top.
> >
> >Everything is reviewed except patches 2 and 7.
> 
> Reviewed the whole series and it looks perfect.
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>

Cool. Thanks for reading through it. Now pushed to dinq.

> 
> >
> >Ville Syrjälä (7):
> >  drm/i915: Extract ilk_lut_10()
> >  drm/i915: Don't use split gamma when we don't have to
> >  drm/i915: Implement split/10bit gamma for ivb/hsw
> >  drm/i915: Add 10bit LUT for ilk/snb
> >  drm/i915: Add "10.6" LUT mode for i965+
> >  drm/i915: Expose the legacy LUT via the GAMMA_LUT/GAMMA_LUT_SIZE props
> >    on gen2/3
> >  drm/i915: Expose full 1024 LUT entries on ivb+
> >
> > drivers/gpu/drm/i915/i915_pci.c    |  23 +-
> > drivers/gpu/drm/i915/i915_reg.h    |  15 ++
> > drivers/gpu/drm/i915/intel_color.c | 375 ++++++++++++++++++++---------
> > 3 files changed, 292 insertions(+), 121 deletions(-)
> >
> >--
> >2.19.2
> 

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