All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] drm/i915: Finish the GAMMA_LUT stuff
@ 2019-03-28 21:04 Ville Syrjala
  2019-03-28 21:05 ` [PATCH 1/6] drm/i915: Extract ilk_lut_10() Ville Syrjala
                   ` (9 more replies)
  0 siblings, 10 replies; 23+ messages in thread
From: Ville Syrjala @ 2019-03-28 21:04 UTC (permalink / raw)
  To: intel-gfx

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

This series exposes the GAMMA_LUT/GAMMA_LUT_SIZE props on all
platforms, finally getting us back to some kind of uniformity.
On i965+ we also gain the ability to gamma correct 10bpc data
without crusing it to 8bpc with the legacy LUT.

To go beyond this we'll need the new uapi to expose different
gamma modes so that userspace can make the best use of the
available hardware. We'll get to that later.

One thing we still need to do is deal with the current bugs
relating to the overloaded use of the pipe CSC for YCbCr and
limited range RGB output. This series already removes some
of those bugs on BDW+, and we can probably eliminate a bit
more if we start to frob the gamma LUT to do the limited
range compression. I've not decided what we're going to do
with GLK however. It has sadly lost the flexibility we
desperately need.

Ville Syrjälä (6):
  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

 drivers/gpu/drm/i915/i915_pci.c    |  21 +-
 drivers/gpu/drm/i915/i915_reg.h    |  14 ++
 drivers/gpu/drm/i915/intel_color.c | 353 +++++++++++++++++++++--------
 3 files changed, 292 insertions(+), 96 deletions(-)

-- 
2.19.2

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/6] drm/i915: Extract ilk_lut_10()
  2019-03-28 21:04 [PATCH 0/6] drm/i915: Finish the GAMMA_LUT stuff Ville Syrjala
@ 2019-03-28 21:05 ` Ville Syrjala
  2019-03-29  0:15   ` Matt Roper
  2019-03-28 21:05 ` [PATCH 2/6] drm/i915: Don't use split gamma when we don't have to Ville Syrjala
                   ` (8 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Ville Syrjala @ 2019-03-28 21:05 UTC (permalink / raw)
  To: intel-gfx

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

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

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 ff910ed08468..d7c38a2bbd8f 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. */
 		i = lut_size - 1;
-- 
2.19.2

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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/6] drm/i915: Don't use split gamma when we don't have to
  2019-03-28 21:04 [PATCH 0/6] drm/i915: Finish the GAMMA_LUT stuff Ville Syrjala
  2019-03-28 21:05 ` [PATCH 1/6] drm/i915: Extract ilk_lut_10() Ville Syrjala
@ 2019-03-28 21:05 ` Ville Syrjala
  2019-03-29  0:16   ` Matt Roper
  2019-03-29 14:14   ` [PATCH v2 " Ville Syrjala
  2019-03-28 21:05 ` [PATCH 3/6] drm/i915: Implement split/10bit gamma for ivb/hsw Ville Syrjala
                   ` (7 subsequent siblings)
  9 siblings, 2 replies; 23+ messages in thread
From: Ville Syrjala @ 2019-03-28 21:05 UTC (permalink / raw)
  To: intel-gfx

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.

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c866379a521b..eb7e93354cfe 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10127,6 +10127,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 d7c38a2bbd8f..ed4bd9bd15f5 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -466,72 +466,32 @@ 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]));
 		}
-	}
-}
-
-static void bdw_load_gamma_lut(const struct intel_crtc_state *crtc_state, u32 offset)
-{
-	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;
-
+	} else {
 		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. */
-		i = lut_size - 1;
-		I915_WRITE(PREC_PAL_GC_MAX(pipe, 0),
-			   drm_color_lut_extract(lut[i].red, 16));
-		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1),
-			   drm_color_lut_extract(lut[i].green, 16));
-		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2),
-			   drm_color_lut_extract(lut[i].blue, 16));
-	} else {
-		for (i = 0; i < lut_size; i++) {
-			u32 v = (i * ((1 << 10) - 1)) / (lut_size - 1);
-
-			I915_WRITE(PREC_PAL_DATA(pipe),
-				   (v << 20) | (v << 10) | v);
-		}
-
-		I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16) - 1);
-		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16) - 1);
-		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16) - 1);
 	}
 
 	/*
@@ -541,18 +501,43 @@ static void bdw_load_gamma_lut(const struct intel_crtc_state *crtc_state, u32 of
 	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
 }
 
-/* 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_lut_10_max(struct intel_crtc *crtc,
+				const struct drm_property_blob *blob)
 {
-	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_color_lut *lut = blob->data;
+	int i = drm_color_lut_size(blob) - 1;
+	enum pipe pipe = crtc->pipe;
 
-	if (crtc_state_is_legacy_gamma(crtc_state)) {
+	/* Program the max register to clamp values > 1.0. */
+	I915_WRITE(PREC_PAL_GC_MAX(pipe, 0),
+		   drm_color_lut_extract(lut[i].red, 16));
+	I915_WRITE(PREC_PAL_GC_MAX(pipe, 1),
+		   drm_color_lut_extract(lut[i].green, 16));
+	I915_WRITE(PREC_PAL_GC_MAX(pipe, 2),
+		   drm_color_lut_extract(lut[i].blue, 16));
+}
+
+static void bdw_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) {
+		bdw_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
+				PAL_PREC_INDEX_VALUE(0), false);
+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
+				PAL_PREC_INDEX_VALUE(512),  false);
+		bdw_load_lut_10_max(crtc, gamma_lut);
 	} 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, blob);
 	}
 }
 
@@ -624,6 +609,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
@@ -637,22 +625,28 @@ 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, gamma_lut);
+	}
 }
 
 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_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, gamma_lut);
+	}
 }
 
 static void cherryview_load_luts(const struct intel_crtc_state *crtc_state)
@@ -937,8 +931,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)
@@ -960,7 +971,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)
@@ -1094,7 +1105,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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 3/6] drm/i915: Implement split/10bit gamma for ivb/hsw
  2019-03-28 21:04 [PATCH 0/6] drm/i915: Finish the GAMMA_LUT stuff Ville Syrjala
  2019-03-28 21:05 ` [PATCH 1/6] drm/i915: Extract ilk_lut_10() Ville Syrjala
  2019-03-28 21:05 ` [PATCH 2/6] drm/i915: Don't use split gamma when we don't have to Ville Syrjala
@ 2019-03-28 21:05 ` Ville Syrjala
  2019-03-29  0:16   ` Matt Roper
  2019-03-28 21:05 ` [PATCH 4/6] drm/i915: Add 10bit LUT for ilk/snb Ville Syrjala
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Ville Syrjala @ 2019-03-28 21:05 UTC (permalink / raw)
  To: intel-gfx

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.

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 a7e1611af26d..385056752939 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, \
@@ -399,6 +399,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 \
@@ -494,7 +495,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, \
@@ -629,7 +629,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 ed4bd9bd15f5..70a71c92e3e5 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,
 				const struct drm_property_blob *blob)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -518,6 +562,29 @@ static void bdw_load_lut_10_max(struct intel_crtc *crtc,
 		   drm_color_lut_extract(lut[i].blue, 16));
 }
 
+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(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
+				PAL_PREC_INDEX_VALUE(512),  false);
+		ivb_load_lut_10_max(crtc, gamma_lut);
+	} 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, blob);
+	}
+}
+
 static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
@@ -531,13 +598,13 @@ static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
 				PAL_PREC_INDEX_VALUE(0), false);
 		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
 				PAL_PREC_INDEX_VALUE(512),  false);
-		bdw_load_lut_10_max(crtc, gamma_lut);
+		ivb_load_lut_10_max(crtc, gamma_lut);
 	} else {
 		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, blob);
+		ivb_load_lut_10_max(crtc, blob);
 	}
 }
 
@@ -629,7 +696,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, gamma_lut);
+		ivb_load_lut_10_max(crtc, gamma_lut);
 	}
 }
 
@@ -645,7 +712,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, gamma_lut);
+		ivb_load_lut_10_max(crtc, gamma_lut);
 	}
 }
 
@@ -907,14 +974,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;
@@ -926,7 +992,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))
@@ -938,22 +1004,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);
@@ -967,11 +1036,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)
@@ -1088,8 +1157,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;
 
@@ -1104,8 +1173,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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 4/6] drm/i915: Add 10bit LUT for ilk/snb
  2019-03-28 21:04 [PATCH 0/6] drm/i915: Finish the GAMMA_LUT stuff Ville Syrjala
                   ` (2 preceding siblings ...)
  2019-03-28 21:05 ` [PATCH 3/6] drm/i915: Implement split/10bit gamma for ivb/hsw Ville Syrjala
@ 2019-03-28 21:05 ` Ville Syrjala
  2019-03-29  0:17   ` Matt Roper
  2019-03-29 10:07   ` Maarten Lankhorst
  2019-03-28 21:05 ` [PATCH 5/6] drm/i915: Add "10.6" LUT mode for i965+ Ville Syrjala
                   ` (5 subsequent siblings)
  9 siblings, 2 replies; 23+ messages in thread
From: Ville Syrjala @ 2019-03-28 21:05 UTC (permalink / raw)
  To: intel-gfx

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.

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 | 44 ++++++++++++++++++++++++++----
 3 files changed, 51 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 385056752939..0971eee4a4d1 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 \
@@ -325,6 +327,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 = {
@@ -353,6 +356,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 eb7e93354cfe..f6a5d8f11368 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 70a71c92e3e5..8e03f066adf7 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_FW(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
@@ -961,6 +984,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;
@@ -980,8 +1012,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;
 
@@ -1178,14 +1209,15 @@ 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 &&
+	/* Enable color management support when we have degamma and/or gamma LUT. */
+	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.degamma_lut_size &&
+					   INTEL_INFO(dev_priv)->color.gamma_lut_size,
 					   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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 5/6] drm/i915: Add "10.6" LUT mode for i965+
  2019-03-28 21:04 [PATCH 0/6] drm/i915: Finish the GAMMA_LUT stuff Ville Syrjala
                   ` (3 preceding siblings ...)
  2019-03-28 21:05 ` [PATCH 4/6] drm/i915: Add 10bit LUT for ilk/snb Ville Syrjala
@ 2019-03-28 21:05 ` Ville Syrjala
  2019-04-04 23:52   ` Sripada, Radhakrishna
  2019-03-28 21:05 ` [PATCH 6/6] drm/i915: Expose the legacy LUT via the GAMMA_LUT/GAMMA_LUT_SIZE props on gen2/3 Ville Syrjala
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Ville Syrjala @ 2019-03-28 21:05 UTC (permalink / raw)
  To: intel-gfx

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.

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 0971eee4a4d1..0c5258aa13bb 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 \
@@ -278,6 +282,7 @@ static const struct intel_device_info intel_pineview_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 = {
@@ -462,6 +467,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 f6a5d8f11368..0437a3ab6cdc 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 8e03f066adf7..07d62c7cb386 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_FW(PALETTE(pipe, 2 * i + 0),
+			      i965_lut_10p6_ldw(&lut[i]));
+		I915_WRITE_FW(PALETTE(pipe, 2 * i + 1),
+			      i965_lut_10p6_udw(&lut[i]));
+	}
+
+	I915_WRITE_FW(PIPEGCMAX(pipe, 0), lut[i].red);
+	I915_WRITE_FW(PIPEGCMAX(pipe, 1), lut[i].green);
+	I915_WRITE_FW(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)
 {
@@ -911,6 +958,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;
@@ -923,7 +979,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)
@@ -1178,6 +1234,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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 6/6] drm/i915: Expose the legacy LUT via the GAMMA_LUT/GAMMA_LUT_SIZE props on gen2/3
  2019-03-28 21:04 [PATCH 0/6] drm/i915: Finish the GAMMA_LUT stuff Ville Syrjala
                   ` (4 preceding siblings ...)
  2019-03-28 21:05 ` [PATCH 5/6] drm/i915: Add "10.6" LUT mode for i965+ Ville Syrjala
@ 2019-03-28 21:05 ` Ville Syrjala
  2019-04-04 16:52   ` Sripada, Radhakrishna
  2019-03-28 22:06 ` ✓ Fi.CI.BAT: success for drm/i915: Finish the GAMMA_LUT stuff Patchwork
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Ville Syrjala @ 2019-03-28 21:05 UTC (permalink / raw)
  To: intel-gfx

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.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c    |  5 +++++
 drivers/gpu/drm/i915/intel_color.c | 13 +++++--------
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index 0c5258aa13bb..0e76df27f151 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 = {
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 07d62c7cb386..fd4a65af5cc4 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1272,12 +1272,9 @@ void intel_color_init(struct intel_crtc *crtc)
 			dev_priv->display.load_luts = ilk_load_luts;
 	}
 
-	/* Enable color management support when we have degamma and/or gamma LUT. */
-	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,
-					   INTEL_INFO(dev_priv)->color.degamma_lut_size &&
-					   INTEL_INFO(dev_priv)->color.gamma_lut_size,
-					   INTEL_INFO(dev_priv)->color.gamma_lut_size);
+	drm_crtc_enable_color_mgmt(&crtc->base,
+				   INTEL_INFO(dev_priv)->color.degamma_lut_size,
+				   INTEL_INFO(dev_priv)->color.degamma_lut_size &&
+				   INTEL_INFO(dev_priv)->color.gamma_lut_size,
+				   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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Finish the GAMMA_LUT stuff
  2019-03-28 21:04 [PATCH 0/6] drm/i915: Finish the GAMMA_LUT stuff Ville Syrjala
                   ` (5 preceding siblings ...)
  2019-03-28 21:05 ` [PATCH 6/6] drm/i915: Expose the legacy LUT via the GAMMA_LUT/GAMMA_LUT_SIZE props on gen2/3 Ville Syrjala
@ 2019-03-28 22:06 ` Patchwork
  2019-03-29  8:34 ` ✗ Fi.CI.IGT: failure " Patchwork
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-03-28 22:06 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_5832 -> Patchwork_12623
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

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

  * igt@i915_selftest@live_evict:
    - fi-bsw-kefka:       PASS -> DMESG-WARN [fdo#107709]

  * igt@i915_selftest@live_requests:
    - fi-icl-u2:          PASS -> INCOMPLETE [fdo#109644]

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

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

  * igt@runner@aborted:
    - fi-bsw-kefka:       NOTRUN -> FAIL [fdo#107709]

  
#### Possible fixes ####

  * igt@i915_selftest@live_uncore:
    - fi-ivb-3770:        DMESG-FAIL [fdo#110210] -> PASS

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

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#107709]: https://bugs.freedesktop.org/show_bug.cgi?id=107709
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#109644]: https://bugs.freedesktop.org/show_bug.cgi?id=109644
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


Participating hosts (43 -> 37)
------------------------------

  Missing    (6): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-bsw-cyan fi-kbl-7500u fi-kbl-r 


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

    * Linux: CI_DRM_5832 -> Patchwork_12623

  CI_DRM_5832: f1fc30ad3723a8b6265c2edf50a7f637ecd75a23 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4911: d9fe699ea45406e279b78d1afdb4d57a205a3c99 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12623: 49c92870dd5d4ca83f2ceb7163ef5dc3d248712a @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

49c92870dd5d drm/i915: Expose the legacy LUT via the GAMMA_LUT/GAMMA_LUT_SIZE props on gen2/3
72ab741ecfe0 drm/i915: Add "10.6" LUT mode for i965+
ccc74c4fbe18 drm/i915: Add 10bit LUT for ilk/snb
45f15d269be4 drm/i915: Implement split/10bit gamma for ivb/hsw
e73e007ddfdf drm/i915: Don't use split gamma when we don't have to
75a71a08c077 drm/i915: Extract ilk_lut_10()

== Logs ==

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/6] drm/i915: Extract ilk_lut_10()
  2019-03-28 21:05 ` [PATCH 1/6] drm/i915: Extract ilk_lut_10() Ville Syrjala
@ 2019-03-29  0:15   ` Matt Roper
  0 siblings, 0 replies; 23+ messages in thread
From: Matt Roper @ 2019-03-29  0:15 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Thu, Mar 28, 2019 at 11:05:00PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Extract a helper to calculate the ILK+ 10it gamma LUT entry.

Missing a 'b' in '10it' but otherwise:

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

> It's already duplicated twice, and soon we'll have more.
> 
> 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 ff910ed08468..d7c38a2bbd8f 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. */
>  		i = lut_size - 1;
> -- 
> 2.19.2
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/6] drm/i915: Don't use split gamma when we don't have to
  2019-03-28 21:05 ` [PATCH 2/6] drm/i915: Don't use split gamma when we don't have to Ville Syrjala
@ 2019-03-29  0:16   ` Matt Roper
  2019-03-29 10:47     ` Ville Syrjälä
  2019-03-29 14:14   ` [PATCH v2 " Ville Syrjala
  1 sibling, 1 reply; 23+ messages in thread
From: Matt Roper @ 2019-03-29  0:16 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Thu, Mar 28, 2019 at 11:05:01PM +0200, Ville Syrjala wrote:
> 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.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |   1 +
>  drivers/gpu/drm/i915/intel_color.c | 159 +++++++++++++++--------------
>  2 files changed, 86 insertions(+), 74 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index c866379a521b..eb7e93354cfe 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -10127,6 +10127,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 d7c38a2bbd8f..ed4bd9bd15f5 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -466,72 +466,32 @@ 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

Any reason not to just do this immediately?  Throwing away half the
table entries if we decide we need split mode doesn't seem any harder
than duplicating the entries when we decide we don't.  The color
management kerneldoc already explicitly recommends this approach for
hardware that can support multiple gamma modes, so I don't think we need
any new ABI to handle it.

> +	 */
> +	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]));
>  		}
> -	}
> -}
> -
> -static void bdw_load_gamma_lut(const struct intel_crtc_state *crtc_state, u32 offset)
> -{
> -	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;
> -
> +	} else {
>  		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. */
> -		i = lut_size - 1;
> -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 0),
> -			   drm_color_lut_extract(lut[i].red, 16));
> -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1),
> -			   drm_color_lut_extract(lut[i].green, 16));
> -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2),
> -			   drm_color_lut_extract(lut[i].blue, 16));
> -	} else {
> -		for (i = 0; i < lut_size; i++) {
> -			u32 v = (i * ((1 << 10) - 1)) / (lut_size - 1);
> -
> -			I915_WRITE(PREC_PAL_DATA(pipe),
> -				   (v << 20) | (v << 10) | v);
> -		}
> -
> -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16) - 1);
> -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16) - 1);
> -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16) - 1);
>  	}
>  
>  	/*
> @@ -541,18 +501,43 @@ static void bdw_load_gamma_lut(const struct intel_crtc_state *crtc_state, u32 of
>  	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
>  }
>  
> -/* 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_lut_10_max(struct intel_crtc *crtc,
> +				const struct drm_property_blob *blob)
>  {
> -	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_color_lut *lut = blob->data;
> +	int i = drm_color_lut_size(blob) - 1;
> +	enum pipe pipe = crtc->pipe;
>  
> -	if (crtc_state_is_legacy_gamma(crtc_state)) {
> +	/* Program the max register to clamp values > 1.0. */
> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 0),
> +		   drm_color_lut_extract(lut[i].red, 16));
> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 1),
> +		   drm_color_lut_extract(lut[i].green, 16));
> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 2),
> +		   drm_color_lut_extract(lut[i].blue, 16));

Are these the right registers to use?  The "Pipe Palette and Gamma" page
of the bspec indicates we should be using PAL_EXT_GC_MAX (e.g., 0x4A420
instead of 0x4A410) for both the 10-bit and split gamma modes.  I only
see PAL_GC_MAX mentioned for the interpolated gamma mode.


Matt

> +}
> +
> +static void bdw_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) {
> +		bdw_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
> +				PAL_PREC_INDEX_VALUE(0), false);
> +		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
> +				PAL_PREC_INDEX_VALUE(512),  false);
> +		bdw_load_lut_10_max(crtc, gamma_lut);
>  	} 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, blob);
>  	}
>  }
>  
> @@ -624,6 +609,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
> @@ -637,22 +625,28 @@ 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, gamma_lut);
> +	}
>  }
>  
>  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_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, gamma_lut);
> +	}
>  }
>  
>  static void cherryview_load_luts(const struct intel_crtc_state *crtc_state)
> @@ -937,8 +931,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)
> @@ -960,7 +971,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)
> @@ -1094,7 +1105,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
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 3/6] drm/i915: Implement split/10bit gamma for ivb/hsw
  2019-03-28 21:05 ` [PATCH 3/6] drm/i915: Implement split/10bit gamma for ivb/hsw Ville Syrjala
@ 2019-03-29  0:16   ` Matt Roper
  0 siblings, 0 replies; 23+ messages in thread
From: Matt Roper @ 2019-03-29  0:16 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Thu, Mar 28, 2019 at 11:05:02PM +0200, Ville Syrjala wrote:
> 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.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Matt Roper <matthew.d.roper@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 a7e1611af26d..385056752939 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, \
> @@ -399,6 +399,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 \
> @@ -494,7 +495,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, \
> @@ -629,7 +629,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 ed4bd9bd15f5..70a71c92e3e5 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,
>  				const struct drm_property_blob *blob)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -518,6 +562,29 @@ static void bdw_load_lut_10_max(struct intel_crtc *crtc,
>  		   drm_color_lut_extract(lut[i].blue, 16));
>  }
>  
> +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(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
> +				PAL_PREC_INDEX_VALUE(512),  false);
> +		ivb_load_lut_10_max(crtc, gamma_lut);
> +	} 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, blob);
> +	}
> +}
> +
>  static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> @@ -531,13 +598,13 @@ static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
>  				PAL_PREC_INDEX_VALUE(0), false);
>  		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
>  				PAL_PREC_INDEX_VALUE(512),  false);
> -		bdw_load_lut_10_max(crtc, gamma_lut);
> +		ivb_load_lut_10_max(crtc, gamma_lut);
>  	} else {
>  		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, blob);
> +		ivb_load_lut_10_max(crtc, blob);
>  	}
>  }
>  
> @@ -629,7 +696,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, gamma_lut);
> +		ivb_load_lut_10_max(crtc, gamma_lut);
>  	}
>  }
>  
> @@ -645,7 +712,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, gamma_lut);
> +		ivb_load_lut_10_max(crtc, gamma_lut);
>  	}
>  }
>  
> @@ -907,14 +974,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;
> @@ -926,7 +992,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))
> @@ -938,22 +1004,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);
> @@ -967,11 +1036,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)
> @@ -1088,8 +1157,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;
>  
> @@ -1104,8 +1173,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
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/6] drm/i915: Add 10bit LUT for ilk/snb
  2019-03-28 21:05 ` [PATCH 4/6] drm/i915: Add 10bit LUT for ilk/snb Ville Syrjala
@ 2019-03-29  0:17   ` Matt Roper
  2019-03-29 10:07   ` Maarten Lankhorst
  1 sibling, 0 replies; 23+ messages in thread
From: Matt Roper @ 2019-03-29  0:17 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

On Thu, Mar 28, 2019 at 11:05:03PM +0200, Ville Syrjala wrote:
> 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.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>


I also glanced through patches 5 and 6 and didn't notice anything
obviously wrong, but I'm not sure how to find bspec-level information
for those platforms.


Matt

> ---
>  drivers/gpu/drm/i915/i915_pci.c    |  4 +++
>  drivers/gpu/drm/i915/i915_reg.h    |  9 ++++++
>  drivers/gpu/drm/i915/intel_color.c | 44 ++++++++++++++++++++++++++----
>  3 files changed, 51 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 385056752939..0971eee4a4d1 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 \
> @@ -325,6 +327,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 = {
> @@ -353,6 +356,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 eb7e93354cfe..f6a5d8f11368 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 70a71c92e3e5..8e03f066adf7 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_FW(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
> @@ -961,6 +984,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;
> @@ -980,8 +1012,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;
>  
> @@ -1178,14 +1209,15 @@ 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 &&
> +	/* Enable color management support when we have degamma and/or gamma LUT. */
> +	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.degamma_lut_size &&
> +					   INTEL_INFO(dev_priv)->color.gamma_lut_size,
>  					   INTEL_INFO(dev_priv)->color.gamma_lut_size);
>  }
> -- 
> 2.19.2
> 

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* ✗ Fi.CI.IGT: failure for drm/i915: Finish the GAMMA_LUT stuff
  2019-03-28 21:04 [PATCH 0/6] drm/i915: Finish the GAMMA_LUT stuff Ville Syrjala
                   ` (6 preceding siblings ...)
  2019-03-28 22:06 ` ✓ Fi.CI.BAT: success for drm/i915: Finish the GAMMA_LUT stuff Patchwork
@ 2019-03-29  8:34 ` Patchwork
  2019-03-29 10:57   ` Ville Syrjälä
  2019-03-29 15:26 ` ✓ Fi.CI.BAT: success for drm/i915: Finish the GAMMA_LUT stuff (rev2) Patchwork
  2019-03-29 19:13 ` ✗ Fi.CI.IGT: failure " Patchwork
  9 siblings, 1 reply; 23+ messages in thread
From: Patchwork @ 2019-03-29  8:34 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Finish the GAMMA_LUT stuff
URL   : https://patchwork.freedesktop.org/series/58698/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5832_full -> Patchwork_12623_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_12623_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_12623_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_12623_full:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_color@pipe-b-ctm-green-to-red:
    - shard-iclb:         PASS -> DMESG-WARN +10

  * igt@kms_color@pipe-c-ctm-0-25:
    - shard-iclb:         NOTRUN -> DMESG-WARN

  * igt@kms_color@pipe-invalid-lut-sizes:
    - shard-snb:          PASS -> FAIL

  
#### Warnings ####

  * igt@kms_color@pipe-a-degamma:
    - shard-iclb:         FAIL [fdo#104782] -> DMESG-FAIL +1

  * igt@kms_color@pipe-c-ctm-max:
    - shard-iclb:         FAIL [fdo#108147] -> DMESG-FAIL +1

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@kms_plane@pixel-format-pipe-a-planes}:
    - shard-iclb:         PASS -> FAIL +5

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_isolation@bcs0-s3:
    - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]

  * igt@gem_partial_pwrite_pread@writes-after-reads-snoop:
    - shard-iclb:         PASS -> TIMEOUT [fdo#109673] +2

  * igt@i915_pm_rpm@i2c:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107807]

  * igt@i915_pm_rpm@legacy-planes:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#108840] / [fdo#109369]

  * igt@i915_pm_rpm@system-suspend-execbuf:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#109638]

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

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

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#110222] +1

  * igt@kms_chamelium@hdmi-crc-abgr8888:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109284]

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-iclb:         PASS -> FAIL [fdo#104782] +2

  * igt@kms_cursor_crc@cursor-128x128-dpms:
    - shard-skl:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_legacy@cursor-vs-flip-atomic:
    - shard-iclb:         PASS -> FAIL [fdo#103355]

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          NOTRUN -> FAIL [fdo#105363]

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#109507]

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +3

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-skl:          NOTRUN -> FAIL [fdo#103167]

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

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-msflip-blt:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +2

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-shrfb-pgflip-blt:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +100

  * igt@kms_panel_fitting@legacy:
    - shard-skl:          NOTRUN -> FAIL [fdo#105456]

  * igt@kms_pipe_crc_basic@read-crc-pipe-d:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +7

  * igt@kms_plane@plane-panning-bottom-right-pipe-c-planes:
    - shard-iclb:         PASS -> FAIL [fdo#103166]

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

  * igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145]

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

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

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#105763]

  * igt@kms_setmode@basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#99912]

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

  * igt@perf@blocking:
    - shard-iclb:         PASS -> FAIL [fdo#108587] +1

  * igt@perf_pmu@busy-accuracy-50-vcs1:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +107

  
#### Possible fixes ####

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

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

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-snb:          FAIL [fdo#106641] -> PASS

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-iclb:         FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_legacy@flip-vs-cursor-legacy:
    - shard-iclb:         FAIL [fdo#102670] -> PASS

  * igt@kms_draw_crc@draw-method-xrgb8888-render-xtiled:
    - shard-glk:          FAIL [fdo#107791] -> PASS

  * igt@kms_flip@busy-flip:
    - shard-skl:          FAIL [fdo#103257] -> PASS

  * igt@kms_flip@flip-vs-suspend:
    - shard-skl:          INCOMPLETE [fdo#109507] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-render:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +6

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-blt:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +18

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

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

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-iclb:         DMESG-WARN [fdo#106885] -> PASS

  
#### Warnings ####

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

  * igt@i915_selftest@live_contexts:
    - shard-iclb:         DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108569]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#102670]: https://bugs.freedesktop.org/show_bug.cgi?id=102670
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103257]: https://bugs.freedesktop.org/show_bug.cgi?id=103257
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [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#105456]: https://bugs.freedesktop.org/show_bug.cgi?id=105456
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107791]: https://bugs.freedesktop.org/show_bug.cgi?id=107791
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108587]: https://bugs.freedesktop.org/show_bug.cgi?id=108587
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109369]: https://bugs.freedesktop.org/show_bug.cgi?id=109369
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#109638]: https://bugs.freedesktop.org/show_bug.cgi?id=109638
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [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#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


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

  Missing    (1): shard-hsw 


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

    * Linux: CI_DRM_5832 -> Patchwork_12623

  CI_DRM_5832: f1fc30ad3723a8b6265c2edf50a7f637ecd75a23 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4911: d9fe699ea45406e279b78d1afdb4d57a205a3c99 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12623: 49c92870dd5d4ca83f2ceb7163ef5dc3d248712a @ 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_12623/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 4/6] drm/i915: Add 10bit LUT for ilk/snb
  2019-03-28 21:05 ` [PATCH 4/6] drm/i915: Add 10bit LUT for ilk/snb Ville Syrjala
  2019-03-29  0:17   ` Matt Roper
@ 2019-03-29 10:07   ` Maarten Lankhorst
  1 sibling, 0 replies; 23+ messages in thread
From: Maarten Lankhorst @ 2019-03-29 10:07 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

Op 28-03-2019 om 22:05 schreef 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.
>
> 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 | 44 ++++++++++++++++++++++++++----
>  3 files changed, 51 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index 385056752939..0971eee4a4d1 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 \
> @@ -325,6 +327,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 = {
> @@ -353,6 +356,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 eb7e93354cfe..f6a5d8f11368 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 70a71c92e3e5..8e03f066adf7 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_FW(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
> @@ -961,6 +984,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;
> @@ -980,8 +1012,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;
>  
> @@ -1178,14 +1209,15 @@ 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 &&
> +	/* Enable color management support when we have degamma and/or gamma LUT. */
> +	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.degamma_lut_size &&
> +					   INTEL_INFO(dev_priv)->color.gamma_lut_size,
>  					   INTEL_INFO(dev_priv)->color.gamma_lut_size);
>  }

Maybe nitpicking, but ctm seems to only be enabled when degamma_lut_size is set.
It could probably be used as bool has_ctm = degamma_lut_size.

Furthermore it's harmless to call drm_crtc_enable_color_mgmt() unconditionally here
instead of patch 6; calling it with 0 gamma and degamma size and has_ctm = false is
a noop. :)

Otherwise patch series looks good. For all patches except 2:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/6] drm/i915: Don't use split gamma when we don't have to
  2019-03-29  0:16   ` Matt Roper
@ 2019-03-29 10:47     ` Ville Syrjälä
  2019-03-29 12:25       ` Shankar, Uma
  2019-03-29 13:49       ` Ville Syrjälä
  0 siblings, 2 replies; 23+ messages in thread
From: Ville Syrjälä @ 2019-03-29 10:47 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx

On Thu, Mar 28, 2019 at 05:16:03PM -0700, Matt Roper wrote:
> On Thu, Mar 28, 2019 at 11:05:01PM +0200, Ville Syrjala wrote:
> > 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.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_reg.h    |   1 +
> >  drivers/gpu/drm/i915/intel_color.c | 159 +++++++++++++++--------------
> >  2 files changed, 86 insertions(+), 74 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index c866379a521b..eb7e93354cfe 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -10127,6 +10127,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 d7c38a2bbd8f..ed4bd9bd15f5 100644
> > --- a/drivers/gpu/drm/i915/intel_color.c
> > +++ b/drivers/gpu/drm/i915/intel_color.c
> > @@ -466,72 +466,32 @@ 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
> 
> Any reason not to just do this immediately?  Throwing away half the
> table entries if we decide we need split mode doesn't seem any harder
> than duplicating the entries when we decide we don't.  The color
> management kerneldoc already explicitly recommends this approach for
> hardware that can support multiple gamma modes, so I don't think we need
> any new ABI to handle it.

Hmm. I guess that apporach could be doable. It might be a bit annoying
for userspace though if it expects a direct color visual. But at least
for X we won't use degamma/ctm anyway so seems like it should work out
just fine.

> 
> > +	 */
> > +	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]));
> >  		}
> > -	}
> > -}
> > -
> > -static void bdw_load_gamma_lut(const struct intel_crtc_state *crtc_state, u32 offset)
> > -{
> > -	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;
> > -
> > +	} else {
> >  		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. */
> > -		i = lut_size - 1;
> > -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 0),
> > -			   drm_color_lut_extract(lut[i].red, 16));
> > -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1),
> > -			   drm_color_lut_extract(lut[i].green, 16));
> > -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2),
> > -			   drm_color_lut_extract(lut[i].blue, 16));
> > -	} else {
> > -		for (i = 0; i < lut_size; i++) {
> > -			u32 v = (i * ((1 << 10) - 1)) / (lut_size - 1);
> > -
> > -			I915_WRITE(PREC_PAL_DATA(pipe),
> > -				   (v << 20) | (v << 10) | v);
> > -		}
> > -
> > -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16) - 1);
> > -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16) - 1);
> > -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16) - 1);
> >  	}
> >  
> >  	/*
> > @@ -541,18 +501,43 @@ static void bdw_load_gamma_lut(const struct intel_crtc_state *crtc_state, u32 of
> >  	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
> >  }
> >  
> > -/* 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_lut_10_max(struct intel_crtc *crtc,
> > +				const struct drm_property_blob *blob)
> >  {
> > -	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_color_lut *lut = blob->data;
> > +	int i = drm_color_lut_size(blob) - 1;
> > +	enum pipe pipe = crtc->pipe;
> >  
> > -	if (crtc_state_is_legacy_gamma(crtc_state)) {
> > +	/* Program the max register to clamp values > 1.0. */
> > +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 0),
> > +		   drm_color_lut_extract(lut[i].red, 16));
> > +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 1),
> > +		   drm_color_lut_extract(lut[i].green, 16));
> > +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 2),
> > +		   drm_color_lut_extract(lut[i].blue, 16));
> 
> Are these the right registers to use?  The "Pipe Palette and Gamma" page
> of the bspec indicates we should be using PAL_EXT_GC_MAX (e.g., 0x4A420
> instead of 0x4A410) for both the 10-bit and split gamma modes.  I only
> see PAL_GC_MAX mentioned for the interpolated gamma mode.

Yeah, these aren't the registers we're looking for. Maybe Uma will send
a patch to fix this? I'm also thinking we should just program these to
1.0. There's also EXT_GC_MAX2 on some more recent platforms.

> 
> 
> Matt
> 
> > +}
> > +
> > +static void bdw_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) {
> > +		bdw_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
> > +				PAL_PREC_INDEX_VALUE(0), false);
> > +		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
> > +				PAL_PREC_INDEX_VALUE(512),  false);
> > +		bdw_load_lut_10_max(crtc, gamma_lut);
> >  	} 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, blob);
> >  	}
> >  }
> >  
> > @@ -624,6 +609,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
> > @@ -637,22 +625,28 @@ 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, gamma_lut);
> > +	}
> >  }
> >  
> >  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_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, gamma_lut);
> > +	}
> >  }
> >  
> >  static void cherryview_load_luts(const struct intel_crtc_state *crtc_state)
> > @@ -937,8 +931,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)
> > @@ -960,7 +971,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)
> > @@ -1094,7 +1105,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
> > 
> 
> -- 
> Matt Roper
> Graphics Software Engineer
> IoTG Platform Enabling & Development
> Intel Corporation
> (916) 356-2795

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: ✗ Fi.CI.IGT: failure for drm/i915: Finish the GAMMA_LUT stuff
  2019-03-29  8:34 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2019-03-29 10:57   ` Ville Syrjälä
  0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2019-03-29 10:57 UTC (permalink / raw)
  To: intel-gfx

On Fri, Mar 29, 2019 at 08:34:03AM -0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Finish the GAMMA_LUT stuff
> URL   : https://patchwork.freedesktop.org/series/58698/
> State : failure
> 
> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_5832_full -> Patchwork_12623_full
> ====================================================
> 
> Summary
> -------
> 
>   **FAILURE**
> 
>   Serious unknown changes coming with Patchwork_12623_full absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_12623_full, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   
> 
> Possible new issues
> -------------------
> 
>   Here are the unknown changes that may have been introduced in Patchwork_12623_full:
> 
> ### IGT changes ###
> 
> #### Possible regressions ####
> 
>   * igt@kms_color@pipe-b-ctm-green-to-red:
>     - shard-iclb:         PASS -> DMESG-WARN +10

<1> [1836.559333] BUG: unable to handle kernel NULL pointer dereference at 0000000000000048
<1> [1836.559337] #PF error: [normal kernel read fault]
<6> [1836.559339] PGD 0 P4D 0 
<4> [1836.559342] Oops: 0000 [#1] PREEMPT SMP NOPTI
<4> [1836.559344] CPU: 6 PID: 4350 Comm: kms_color Tainted: G     U            5.1.0-rc2-CI-Patchwork_12623+ #1
<4> [1836.559346] Hardware name: Intel Corporation Ice Lake Client Platform/IceLake U DDR4 SODIMM PD RVP, BIOS ICLSFWR1.R00.3087.A00.1902250334 02/25/2019
<4> [1836.559391] RIP: 0010:bdw_load_lut_10+0x1b/0x130 [i915]

Whoops. Will need to be a bit more careful with gamma_mode on icl.

> 
>   * igt@kms_color@pipe-c-ctm-0-25:
>     - shard-iclb:         NOTRUN -> DMESG-WARN
> 
>   * igt@kms_color@pipe-invalid-lut-sizes:
>     - shard-snb:          PASS -> FAIL
> 
>   
> #### Warnings ####
> 
>   * igt@kms_color@pipe-a-degamma:
>     - shard-iclb:         FAIL [fdo#104782] -> DMESG-FAIL +1
> 
>   * igt@kms_color@pipe-c-ctm-max:
>     - shard-iclb:         FAIL [fdo#108147] -> DMESG-FAIL +1
> 
>   
> #### Suppressed ####
> 
>   The following results come from untrusted machines, tests, or statuses.
>   They do not affect the overall result.
> 
>   * {igt@kms_plane@pixel-format-pipe-a-planes}:
>     - shard-iclb:         PASS -> FAIL +5
> 
>   
> Known issues
> ------------
> 
>   Here are the changes found in Patchwork_12623_full that come from known issues:
> 
> ### IGT changes ###
> 
> #### Issues hit ####
> 
>   * igt@gem_ctx_isolation@bcs0-s3:
>     - shard-skl:          PASS -> INCOMPLETE [fdo#104108] / [fdo#107773]
> 
>   * igt@gem_partial_pwrite_pread@writes-after-reads-snoop:
>     - shard-iclb:         PASS -> TIMEOUT [fdo#109673] +2
> 
>   * igt@i915_pm_rpm@i2c:
>     - shard-skl:          PASS -> INCOMPLETE [fdo#107807]
> 
>   * igt@i915_pm_rpm@legacy-planes:
>     - shard-iclb:         PASS -> INCOMPLETE [fdo#108840] / [fdo#109369]
> 
>   * igt@i915_pm_rpm@system-suspend-execbuf:
>     - shard-iclb:         PASS -> DMESG-WARN [fdo#109638]
> 
>   * igt@kms_atomic_transition@3x-modeset-transitions:
>     - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +12
> 
>   * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
>     - shard-glk:          PASS -> DMESG-WARN [fdo#110222]
> 
>   * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
>     - shard-skl:          NOTRUN -> DMESG-WARN [fdo#110222] +1
> 
>   * igt@kms_chamelium@hdmi-crc-abgr8888:
>     - shard-iclb:         NOTRUN -> SKIP [fdo#109284]
> 
>   * igt@kms_color@pipe-b-legacy-gamma:
>     - shard-iclb:         PASS -> FAIL [fdo#104782] +2
> 
>   * igt@kms_cursor_crc@cursor-128x128-dpms:
>     - shard-skl:          NOTRUN -> FAIL [fdo#103232]
> 
>   * igt@kms_cursor_legacy@cursor-vs-flip-atomic:
>     - shard-iclb:         PASS -> FAIL [fdo#103355]
> 
>   * igt@kms_flip@flip-vs-expired-vblank:
>     - shard-skl:          NOTRUN -> FAIL [fdo#105363]
> 
>   * igt@kms_flip@flip-vs-suspend-interruptible:
>     - shard-skl:          NOTRUN -> INCOMPLETE [fdo#109507]
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-blt:
>     - shard-iclb:         PASS -> FAIL [fdo#103167] +3
> 
>   * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-cpu:
>     - shard-skl:          NOTRUN -> FAIL [fdo#103167]
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-cpu:
>     - shard-iclb:         PASS -> FAIL [fdo#105682] / [fdo#109247]
> 
>   * igt@kms_frontbuffer_tracking@psr-1p-primscrn-indfb-msflip-blt:
>     - shard-iclb:         PASS -> FAIL [fdo#109247] +2
> 
>   * igt@kms_frontbuffer_tracking@psr-1p-primscrn-shrfb-pgflip-blt:
>     - shard-snb:          NOTRUN -> SKIP [fdo#109271] +100
> 
>   * igt@kms_panel_fitting@legacy:
>     - shard-skl:          NOTRUN -> FAIL [fdo#105456]
> 
>   * igt@kms_pipe_crc_basic@read-crc-pipe-d:
>     - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +7
> 
>   * igt@kms_plane@plane-panning-bottom-right-pipe-c-planes:
>     - shard-iclb:         PASS -> FAIL [fdo#103166]
> 
>   * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
>     - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145]
> 
>   * igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
>     - shard-skl:          NOTRUN -> FAIL [fdo#108145]
> 
>   * igt@kms_psr@cursor_mmap_cpu:
>     - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215]
> 
>   * igt@kms_psr@psr2_no_drrs:
>     - shard-iclb:         PASS -> SKIP [fdo#109441] +1
> 
>   * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
>     - shard-kbl:          PASS -> DMESG-FAIL [fdo#105763]
> 
>   * igt@kms_setmode@basic:
>     - shard-skl:          NOTRUN -> FAIL [fdo#99912]
> 
>   * igt@kms_vblank@pipe-b-ts-continuation-suspend:
>     - shard-iclb:         PASS -> FAIL [fdo#104894]
> 
>   * igt@perf@blocking:
>     - shard-iclb:         PASS -> FAIL [fdo#108587] +1
> 
>   * igt@perf_pmu@busy-accuracy-50-vcs1:
>     - shard-skl:          NOTRUN -> SKIP [fdo#109271] +107
> 
>   
> #### Possible fixes ####
> 
>   * igt@gem_create@create-clear:
>     - shard-snb:          INCOMPLETE [fdo#105411] -> PASS
> 
>   * igt@i915_pm_rpm@i2c:
>     - shard-iclb:         DMESG-WARN [fdo#109982] -> PASS
> 
>   * igt@kms_available_modes_crc@available_mode_test_crc:
>     - shard-snb:          FAIL [fdo#106641] -> PASS
> 
>   * igt@kms_cursor_crc@cursor-128x128-suspend:
>     - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS
> 
>   * igt@kms_cursor_crc@cursor-64x64-suspend:
>     - shard-iclb:         FAIL [fdo#103232] -> PASS
> 
>   * igt@kms_cursor_legacy@flip-vs-cursor-legacy:
>     - shard-iclb:         FAIL [fdo#102670] -> PASS
> 
>   * igt@kms_draw_crc@draw-method-xrgb8888-render-xtiled:
>     - shard-glk:          FAIL [fdo#107791] -> PASS
> 
>   * igt@kms_flip@busy-flip:
>     - shard-skl:          FAIL [fdo#103257] -> PASS
> 
>   * igt@kms_flip@flip-vs-suspend:
>     - shard-skl:          INCOMPLETE [fdo#109507] -> PASS
> 
>   * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-render:
>     - shard-iclb:         FAIL [fdo#103167] -> PASS +6
> 
>   * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-blt:
>     - shard-iclb:         FAIL [fdo#109247] -> PASS +18
> 
>   * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
>     - shard-skl:          FAIL [fdo#107815] -> PASS +1
> 
>   * igt@kms_psr@cursor_mmap_gtt:
>     - shard-iclb:         FAIL [fdo#107383] / [fdo#110215] -> PASS +5
> 
>   * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
>     - shard-iclb:         DMESG-WARN [fdo#106885] -> PASS
> 
>   
> #### Warnings ####
> 
>   * igt@gem_tiled_swapping@non-threaded:
>     - shard-iclb:         DMESG-WARN [fdo#108686] -> FAIL [fdo#108686]
> 
>   * igt@i915_selftest@live_contexts:
>     - shard-iclb:         DMESG-FAIL [fdo#108569] -> INCOMPLETE [fdo#108569]
> 
>   
>   {name}: This element is suppressed. This means it is ignored when computing
>           the status of the difference (SUCCESS, WARNING, or FAILURE).
> 
>   [fdo#102670]: https://bugs.freedesktop.org/show_bug.cgi?id=102670
>   [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
>   [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
>   [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
>   [fdo#103257]: https://bugs.freedesktop.org/show_bug.cgi?id=103257
>   [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
>   [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
>   [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
>   [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#105456]: https://bugs.freedesktop.org/show_bug.cgi?id=105456
>   [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
>   [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
>   [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
>   [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
>   [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
>   [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
>   [fdo#107791]: https://bugs.freedesktop.org/show_bug.cgi?id=107791
>   [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
>   [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
>   [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
>   [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
>   [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
>   [fdo#108587]: https://bugs.freedesktop.org/show_bug.cgi?id=108587
>   [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
>   [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
>   [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
>   [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
>   [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
>   [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
>   [fdo#109369]: https://bugs.freedesktop.org/show_bug.cgi?id=109369
>   [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
>   [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
>   [fdo#109638]: https://bugs.freedesktop.org/show_bug.cgi?id=109638
>   [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
>   [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#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
> 
> 
> Participating hosts (10 -> 9)
> ------------------------------
> 
>   Missing    (1): shard-hsw 
> 
> 
> Build changes
> -------------
> 
>     * Linux: CI_DRM_5832 -> Patchwork_12623
> 
>   CI_DRM_5832: f1fc30ad3723a8b6265c2edf50a7f637ecd75a23 @ git://anongit.freedesktop.org/gfx-ci/linux
>   IGT_4911: d9fe699ea45406e279b78d1afdb4d57a205a3c99 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
>   Patchwork_12623: 49c92870dd5d4ca83f2ceb7163ef5dc3d248712a @ 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_12623/

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/6] drm/i915: Don't use split gamma when we don't have to
  2019-03-29 10:47     ` Ville Syrjälä
@ 2019-03-29 12:25       ` Shankar, Uma
  2019-03-29 13:49       ` Ville Syrjälä
  1 sibling, 0 replies; 23+ messages in thread
From: Shankar, Uma @ 2019-03-29 12:25 UTC (permalink / raw)
  To: Ville Syrjälä, Roper, Matthew D; +Cc: intel-gfx



>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Friday, March 29, 2019 4:17 PM
>To: Roper, Matthew D <matthew.d.roper@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Shankar, Uma <uma.shankar@intel.com>
>Subject: Re: [PATCH 2/6] drm/i915: Don't use split gamma when we don't have to
>
>On Thu, Mar 28, 2019 at 05:16:03PM -0700, Matt Roper wrote:
>> On Thu, Mar 28, 2019 at 11:05:01PM +0200, Ville Syrjala wrote:
>> > 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.
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_reg.h    |   1 +
>> >  drivers/gpu/drm/i915/intel_color.c | 159
>> > +++++++++++++++--------------
>> >  2 files changed, 86 insertions(+), 74 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> > b/drivers/gpu/drm/i915/i915_reg.h index c866379a521b..eb7e93354cfe
>> > 100644
>> > --- a/drivers/gpu/drm/i915/i915_reg.h
>> > +++ b/drivers/gpu/drm/i915/i915_reg.h
>> > @@ -10127,6 +10127,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 d7c38a2bbd8f..ed4bd9bd15f5 100644
>> > --- a/drivers/gpu/drm/i915/intel_color.c
>> > +++ b/drivers/gpu/drm/i915/intel_color.c
>> > @@ -466,72 +466,32 @@ 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
>>
>> Any reason not to just do this immediately?  Throwing away half the
>> table entries if we decide we need split mode doesn't seem any harder
>> than duplicating the entries when we decide we don't.  The color
>> management kerneldoc already explicitly recommends this approach for
>> hardware that can support multiple gamma modes, so I don't think we
>> need any new ABI to handle it.
>
>Hmm. I guess that apporach could be doable. It might be a bit annoying for userspace
>though if it expects a direct color visual. But at least for X we won't use
>degamma/ctm anyway so seems like it should work out just fine.
>
>>
>> > +	 */
>> > +	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]));
>> >  		}
>> > -	}
>> > -}
>> > -
>> > -static void bdw_load_gamma_lut(const struct intel_crtc_state
>> > *crtc_state, u32 offset) -{
>> > -	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;
>> > -
>> > +	} else {
>> >  		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. */
>> > -		i = lut_size - 1;
>> > -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 0),
>> > -			   drm_color_lut_extract(lut[i].red, 16));
>> > -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1),
>> > -			   drm_color_lut_extract(lut[i].green, 16));
>> > -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2),
>> > -			   drm_color_lut_extract(lut[i].blue, 16));
>> > -	} else {
>> > -		for (i = 0; i < lut_size; i++) {
>> > -			u32 v = (i * ((1 << 10) - 1)) / (lut_size - 1);
>> > -
>> > -			I915_WRITE(PREC_PAL_DATA(pipe),
>> > -				   (v << 20) | (v << 10) | v);
>> > -		}
>> > -
>> > -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16) - 1);
>> > -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16) - 1);
>> > -		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16) - 1);
>> >  	}
>> >
>> >  	/*
>> > @@ -541,18 +501,43 @@ static void bdw_load_gamma_lut(const struct
>intel_crtc_state *crtc_state, u32 of
>> >  	I915_WRITE(PREC_PAL_INDEX(pipe), 0);  }
>> >
>> > -/* 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_lut_10_max(struct intel_crtc *crtc,
>> > +				const struct drm_property_blob *blob)
>> >  {
>> > -	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_color_lut *lut = blob->data;
>> > +	int i = drm_color_lut_size(blob) - 1;
>> > +	enum pipe pipe = crtc->pipe;
>> >
>> > -	if (crtc_state_is_legacy_gamma(crtc_state)) {
>> > +	/* Program the max register to clamp values > 1.0. */
>> > +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 0),
>> > +		   drm_color_lut_extract(lut[i].red, 16));
>> > +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 1),
>> > +		   drm_color_lut_extract(lut[i].green, 16));
>> > +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 2),
>> > +		   drm_color_lut_extract(lut[i].blue, 16));
>>
>> Are these the right registers to use?  The "Pipe Palette and Gamma"
>> page of the bspec indicates we should be using PAL_EXT_GC_MAX (e.g.,
>> 0x4A420 instead of 0x4A410) for both the 10-bit and split gamma modes.
>> I only see PAL_GC_MAX mentioned for the interpolated gamma mode.
>
>Yeah, these aren't the registers we're looking for. Maybe Uma will send a patch to fix
>this? I'm also thinking we should just program these to 1.0. There's also
>EXT_GC_MAX2 on some more recent platforms.

Yes, sent now to list :). Please review.

Regards,
Uma Shankar

>>
>>
>> Matt
>>
>> > +}
>> > +
>> > +static void bdw_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) {
>> > +		bdw_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
>> > +				PAL_PREC_INDEX_VALUE(0), false);
>> > +		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
>> > +				PAL_PREC_INDEX_VALUE(512),  false);
>> > +		bdw_load_lut_10_max(crtc, gamma_lut);
>> >  	} 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, blob);
>> >  	}
>> >  }
>> >
>> > @@ -624,6 +609,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 @@ -637,22
>> > +625,28 @@ 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, gamma_lut);
>> > +	}
>> >  }
>> >
>> >  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_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, gamma_lut);
>> > +	}
>> >  }
>> >
>> >  static void cherryview_load_luts(const struct intel_crtc_state
>> > *crtc_state) @@ -937,8 +931,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) @@
>> > -960,7 +971,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)
>> > @@ -1094,7 +1105,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
>> >
>>
>> --
>> Matt Roper
>> Graphics Software Engineer
>> IoTG Platform Enabling & Development
>> Intel Corporation
>> (916) 356-2795
>
>--
>Ville Syrjälä
>Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/6] drm/i915: Don't use split gamma when we don't have to
  2019-03-29 10:47     ` Ville Syrjälä
  2019-03-29 12:25       ` Shankar, Uma
@ 2019-03-29 13:49       ` Ville Syrjälä
  1 sibling, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2019-03-29 13:49 UTC (permalink / raw)
  To: Matt Roper; +Cc: intel-gfx

On Fri, Mar 29, 2019 at 12:47:02PM +0200, Ville Syrjälä wrote:
> On Thu, Mar 28, 2019 at 05:16:03PM -0700, Matt Roper wrote:
> > On Thu, Mar 28, 2019 at 11:05:01PM +0200, Ville Syrjala wrote:
> > > 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.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/i915_reg.h    |   1 +
> > >  drivers/gpu/drm/i915/intel_color.c | 159 +++++++++++++++--------------
> > >  2 files changed, 86 insertions(+), 74 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > index c866379a521b..eb7e93354cfe 100644
> > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > @@ -10127,6 +10127,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 d7c38a2bbd8f..ed4bd9bd15f5 100644
> > > --- a/drivers/gpu/drm/i915/intel_color.c
> > > +++ b/drivers/gpu/drm/i915/intel_color.c
> > > @@ -466,72 +466,32 @@ 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
> > 
> > Any reason not to just do this immediately?  Throwing away half the
> > table entries if we decide we need split mode doesn't seem any harder
> > than duplicating the entries when we decide we don't.  The color
> > management kerneldoc already explicitly recommends this approach for
> > hardware that can support multiple gamma modes, so I don't think we need
> > any new ABI to handle it.
> 
> Hmm. I guess that apporach could be doable. It might be a bit annoying
> for userspace though if it expects a direct color visual. But at least
> for X we won't use degamma/ctm anyway so seems like it should work out
> just fine.

As usual this needs a bit of care when picking the LUT entries we
use. I think I'll send that as a followup.

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH v2 2/6] drm/i915: Don't use split gamma when we don't have to
  2019-03-28 21:05 ` [PATCH 2/6] drm/i915: Don't use split gamma when we don't have to Ville Syrjala
  2019-03-29  0:16   ` Matt Roper
@ 2019-03-29 14:14   ` Ville Syrjala
  1 sibling, 0 replies; 23+ messages in thread
From: Ville Syrjala @ 2019-03-29 14:14 UTC (permalink / raw)
  To: intel-gfx

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

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 | 160 ++++++++++++++++-------------
 2 files changed, 88 insertions(+), 74 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c866379a521b..863ba687226c 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 d7c38a2bbd8f..f789a9fd9ba2 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -466,72 +466,32 @@ 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]));
 		}
-	}
-}
-
-static void bdw_load_gamma_lut(const struct intel_crtc_state *crtc_state, u32 offset)
-{
-	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;
-
+	} else {
 		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. */
-		i = lut_size - 1;
-		I915_WRITE(PREC_PAL_GC_MAX(pipe, 0),
-			   drm_color_lut_extract(lut[i].red, 16));
-		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1),
-			   drm_color_lut_extract(lut[i].green, 16));
-		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2),
-			   drm_color_lut_extract(lut[i].blue, 16));
-	} else {
-		for (i = 0; i < lut_size; i++) {
-			u32 v = (i * ((1 << 10) - 1)) / (lut_size - 1);
-
-			I915_WRITE(PREC_PAL_DATA(pipe),
-				   (v << 20) | (v << 10) | v);
-		}
-
-		I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16) - 1);
-		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16) - 1);
-		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16) - 1);
 	}
 
 	/*
@@ -541,18 +501,43 @@ static void bdw_load_gamma_lut(const struct intel_crtc_state *crtc_state, u32 of
 	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
 }
 
-/* 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_lut_10_max(struct intel_crtc *crtc,
+				const struct drm_property_blob *blob)
 {
-	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_color_lut *lut = blob->data;
+	int i = drm_color_lut_size(blob) - 1;
+	enum pipe pipe = crtc->pipe;
 
-	if (crtc_state_is_legacy_gamma(crtc_state)) {
+	/* Program the max register to clamp values > 1.0. */
+	I915_WRITE(PREC_PAL_GC_MAX(pipe, 0),
+		   drm_color_lut_extract(lut[i].red, 16));
+	I915_WRITE(PREC_PAL_GC_MAX(pipe, 1),
+		   drm_color_lut_extract(lut[i].green, 16));
+	I915_WRITE(PREC_PAL_GC_MAX(pipe, 2),
+		   drm_color_lut_extract(lut[i].blue, 16));
+}
+
+static void bdw_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) {
+		bdw_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
+				PAL_PREC_INDEX_VALUE(0), false);
+		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
+				PAL_PREC_INDEX_VALUE(512),  false);
+		bdw_load_lut_10_max(crtc, gamma_lut);
 	} 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, blob);
 	}
 }
 
@@ -624,6 +609,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
@@ -637,22 +625,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, gamma_lut);
+	}
 }
 
 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, gamma_lut);
+	}
 }
 
 static void cherryview_load_luts(const struct intel_crtc_state *crtc_state)
@@ -937,8 +932,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)
@@ -960,7 +972,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)
@@ -1094,7 +1106,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

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Finish the GAMMA_LUT stuff (rev2)
  2019-03-28 21:04 [PATCH 0/6] drm/i915: Finish the GAMMA_LUT stuff Ville Syrjala
                   ` (7 preceding siblings ...)
  2019-03-29  8:34 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2019-03-29 15:26 ` Patchwork
  2019-03-29 19:13 ` ✗ Fi.CI.IGT: failure " Patchwork
  9 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-03-29 15:26 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_5837 -> Patchwork_12631
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_basic@cs-compute:
    - fi-kbl-8809g:       NOTRUN -> FAIL [fdo#108094]

  * igt@gem_exec_store@basic-bsd1:
    - fi-kbl-r:           NOTRUN -> SKIP [fdo#109271] +41

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

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

  * igt@i915_selftest@live_hangcheck:
    - fi-skl-iommu:       PASS -> INCOMPLETE [fdo#108602] / [fdo#108744]

  * igt@kms_busy@basic-flip-a:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  * igt@kms_chamelium@vga-edid-read:
    - fi-hsw-4770r:       NOTRUN -> SKIP [fdo#109271] +45

  * igt@kms_pipe_crc_basic@read-crc-pipe-a:
    - fi-byt-clapper:     PASS -> FAIL [fdo#107362]

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

  * igt@runner@aborted:
    - fi-skl-iommu:       NOTRUN -> FAIL [fdo#104108] / [fdo#108602]
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720]

  
#### Possible fixes ####

  * igt@amdgpu/amd_basic@userptr:
    - fi-kbl-8809g:       DMESG-WARN [fdo#108965] -> PASS

  * igt@i915_selftest@live_uncore:
    - fi-ivb-3770:        DMESG-FAIL [fdo#110210] -> PASS

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

  * igt@prime_vgem@basic-fence-flip:
    - fi-gdg-551:         FAIL [fdo#103182] -> PASS

  
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108094]: https://bugs.freedesktop.org/show_bug.cgi?id=108094
  [fdo#108602]: https://bugs.freedesktop.org/show_bug.cgi?id=108602
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#108744]: https://bugs.freedesktop.org/show_bug.cgi?id=108744
  [fdo#108965]: https://bugs.freedesktop.org/show_bug.cgi?id=108965
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


Participating hosts (46 -> 40)
------------------------------

  Additional (2): fi-hsw-4770r fi-kbl-r 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-icl-u2 fi-bsw-cyan fi-byt-n2820 fi-bdw-samus 


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

    * Linux: CI_DRM_5837 -> Patchwork_12631

  CI_DRM_5837: 1a35af6fa0d612425e325024cbac10e6fa9a9cd5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4912: 66deae8b6fa69540f069d6551cd22013f5343948 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12631: b44543b6b7646b03ab01690656847bcb18b5e033 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

b44543b6b764 drm/i915: Expose the legacy LUT via the GAMMA_LUT/GAMMA_LUT_SIZE props on gen2/3
07bfd29fc7f8 drm/i915: Add "10.6" LUT mode for i965+
5c7693fa7681 drm/i915: Add 10bit LUT for ilk/snb
60c673dde0b6 drm/i915: Implement split/10bit gamma for ivb/hsw
f4ad2738a70b drm/i915: Don't use split gamma when we don't have to
f87bd1be9aa8 drm/i915: Extract ilk_lut_10()

== Logs ==

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* ✗ Fi.CI.IGT: failure for drm/i915: Finish the GAMMA_LUT stuff (rev2)
  2019-03-28 21:04 [PATCH 0/6] drm/i915: Finish the GAMMA_LUT stuff Ville Syrjala
                   ` (8 preceding siblings ...)
  2019-03-29 15:26 ` ✓ Fi.CI.BAT: success for drm/i915: Finish the GAMMA_LUT stuff (rev2) Patchwork
@ 2019-03-29 19:13 ` Patchwork
  9 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-03-29 19:13 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_5837_full -> Patchwork_12631_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_12631_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_12631_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_12631_full:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_color@pipe-invalid-lut-sizes:
    - shard-snb:          PASS -> FAIL

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

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

### IGT changes ###

#### Issues hit ####

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

  * igt@kms_atomic_transition@4x-modeset-transitions-nonblocking-fencing:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +11

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

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

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#110222]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-c:
    - shard-kbl:          NOTRUN -> DMESG-WARN [fdo#110222]

  * igt@kms_cursor_legacy@cursor-vs-flip-legacy:
    - shard-iclb:         PASS -> FAIL [fdo#103355]

  * igt@kms_flip@2x-plain-flip-ts-check:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +19

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

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-render:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +70

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +3

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-fullscreen:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +15

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-mmap-wc:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +23

  * igt@kms_pipe_crc_basic@read-crc-pipe-d:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +2

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

  * igt@kms_plane_alpha_blend@pipe-b-alpha-opaque-fb:
    - shard-apl:          PASS -> FAIL [fdo#108145]

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

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

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

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

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

  * igt@kms_sysfs_edid_timing:
    - shard-skl:          NOTRUN -> FAIL [fdo#100047]

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-rpm:
    - shard-apl:          PASS -> FAIL [fdo#104894]

  * igt@perf_pmu@multi-client-vcs1:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +43

  
#### Possible fixes ####

  * igt@gem_eio@hibernate:
    - shard-snb:          FAIL [fdo#107918] -> PASS

  * igt@gem_pwrite@big-cpu-fbr:
    - shard-apl:          INCOMPLETE [fdo#103927] -> PASS

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

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-snb:          FAIL [fdo#106641] -> PASS

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

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-skl:          FAIL [fdo#105363] -> PASS
    - shard-snb:          FAIL [fdo#105363] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-blt:
    - shard-iclb:         FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-rte:
    - shard-iclb:         FAIL [fdo#105682] / [fdo#109247] -> PASS

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-onoff:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +10

  * {igt@kms_plane@pixel-format-pipe-b-planes-source-clamping}:
    - shard-glk:          SKIP [fdo#109271] -> PASS +1

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

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

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> PASS
    - shard-kbl:          FAIL [fdo#99912] -> PASS

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

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107918]: https://bugs.freedesktop.org/show_bug.cgi?id=107918
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [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#110278]: https://bugs.freedesktop.org/show_bug.cgi?id=110278
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


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

  Missing    (1): shard-hsw 


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

    * Linux: CI_DRM_5837 -> Patchwork_12631

  CI_DRM_5837: 1a35af6fa0d612425e325024cbac10e6fa9a9cd5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4912: 66deae8b6fa69540f069d6551cd22013f5343948 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12631: b44543b6b7646b03ab01690656847bcb18b5e033 @ 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_12631/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 6/6] drm/i915: Expose the legacy LUT via the GAMMA_LUT/GAMMA_LUT_SIZE props on gen2/3
  2019-03-28 21:05 ` [PATCH 6/6] drm/i915: Expose the legacy LUT via the GAMMA_LUT/GAMMA_LUT_SIZE props on gen2/3 Ville Syrjala
@ 2019-04-04 16:52   ` Sripada, Radhakrishna
  0 siblings, 0 replies; 23+ messages in thread
From: Sripada, Radhakrishna @ 2019-04-04 16:52 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Thu, 2019-03-28 at 23:05 +0200, Ville Syrjala wrote:
> 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.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
LGTM.
Revieweed-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_pci.c    |  5 +++++
>  drivers/gpu/drm/i915/intel_color.c | 13 +++++--------
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pci.c
> b/drivers/gpu/drm/i915/i915_pci.c
> index 0c5258aa13bb..0e76df27f151 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 = {
> diff --git a/drivers/gpu/drm/i915/intel_color.c
> b/drivers/gpu/drm/i915/intel_color.c
> index 07d62c7cb386..fd4a65af5cc4 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -1272,12 +1272,9 @@ void intel_color_init(struct intel_crtc *crtc)
>  			dev_priv->display.load_luts = ilk_load_luts;
>  	}
>  
> -	/* Enable color management support when we have degamma and/or
> gamma LUT. */
> -	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,
> -					   INTEL_INFO(dev_priv)-
> >color.degamma_lut_size &&
> -					   INTEL_INFO(dev_priv)-
> >color.gamma_lut_size,
> -					   INTEL_INFO(dev_priv)-
> >color.gamma_lut_size);
> +	drm_crtc_enable_color_mgmt(&crtc->base,
> +				   INTEL_INFO(dev_priv)-
> >color.degamma_lut_size,
> +				   INTEL_INFO(dev_priv)-
> >color.degamma_lut_size &&
> +				   INTEL_INFO(dev_priv)-
> >color.gamma_lut_size,
> +				   INTEL_INFO(dev_priv)-
> >color.gamma_lut_size);
>  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 5/6] drm/i915: Add "10.6" LUT mode for i965+
  2019-03-28 21:05 ` [PATCH 5/6] drm/i915: Add "10.6" LUT mode for i965+ Ville Syrjala
@ 2019-04-04 23:52   ` Sripada, Radhakrishna
  0 siblings, 0 replies; 23+ messages in thread
From: Sripada, Radhakrishna @ 2019-04-04 23:52 UTC (permalink / raw)
  To: ville.syrjala, intel-gfx

On Thu, 2019-03-28 at 23:05 +0200, Ville Syrjala wrote:
> 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.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
LGTM
Reviewed-by: Radhakrishna Sripada <radhakrishna.sripada@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 0971eee4a4d1..0c5258aa13bb 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 \
> @@ -278,6 +282,7 @@ static const struct intel_device_info
> intel_pineview_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 = {
> @@ -462,6 +467,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 f6a5d8f11368..0437a3ab6cdc 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 8e03f066adf7..07d62c7cb386 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_FW(PALETTE(pipe, 2 * i + 0),
> +			      i965_lut_10p6_ldw(&lut[i]));
> +		I915_WRITE_FW(PALETTE(pipe, 2 * i + 1),
> +			      i965_lut_10p6_udw(&lut[i]));
> +	}
> +
> +	I915_WRITE_FW(PIPEGCMAX(pipe, 0), lut[i].red);
> +	I915_WRITE_FW(PIPEGCMAX(pipe, 1), lut[i].green);
> +	I915_WRITE_FW(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)
>  {
> @@ -911,6 +958,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;
> @@ -923,7 +979,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)
> @@ -1178,6 +1234,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;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2019-04-04 23:52 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-03-28 21:04 [PATCH 0/6] drm/i915: Finish the GAMMA_LUT stuff Ville Syrjala
2019-03-28 21:05 ` [PATCH 1/6] drm/i915: Extract ilk_lut_10() Ville Syrjala
2019-03-29  0:15   ` Matt Roper
2019-03-28 21:05 ` [PATCH 2/6] drm/i915: Don't use split gamma when we don't have to Ville Syrjala
2019-03-29  0:16   ` Matt Roper
2019-03-29 10:47     ` Ville Syrjälä
2019-03-29 12:25       ` Shankar, Uma
2019-03-29 13:49       ` Ville Syrjälä
2019-03-29 14:14   ` [PATCH v2 " Ville Syrjala
2019-03-28 21:05 ` [PATCH 3/6] drm/i915: Implement split/10bit gamma for ivb/hsw Ville Syrjala
2019-03-29  0:16   ` Matt Roper
2019-03-28 21:05 ` [PATCH 4/6] drm/i915: Add 10bit LUT for ilk/snb Ville Syrjala
2019-03-29  0:17   ` Matt Roper
2019-03-29 10:07   ` Maarten Lankhorst
2019-03-28 21:05 ` [PATCH 5/6] drm/i915: Add "10.6" LUT mode for i965+ Ville Syrjala
2019-04-04 23:52   ` Sripada, Radhakrishna
2019-03-28 21:05 ` [PATCH 6/6] drm/i915: Expose the legacy LUT via the GAMMA_LUT/GAMMA_LUT_SIZE props on gen2/3 Ville Syrjala
2019-04-04 16:52   ` Sripada, Radhakrishna
2019-03-28 22:06 ` ✓ Fi.CI.BAT: success for drm/i915: Finish the GAMMA_LUT stuff Patchwork
2019-03-29  8:34 ` ✗ Fi.CI.IGT: failure " Patchwork
2019-03-29 10:57   ` Ville Syrjälä
2019-03-29 15:26 ` ✓ Fi.CI.BAT: success for drm/i915: Finish the GAMMA_LUT stuff (rev2) Patchwork
2019-03-29 19:13 ` ✗ Fi.CI.IGT: failure " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.