[PATCH v3 0/4] Enable Multi-segmented-gamma for ICL

[PATCH v3 0/4] Enable Multi-segmented-gamma for ICL

[Shashank Sharma]

This patch series enables programming of Multi-segmented-gamma
palette for ICL.

Shashank Sharma (3):
  drm/i915: Change gamma/degamma_lut_size data type to u32
  drm/i915: Rename ivb_load_lut_10_max
  drm/i915/icl: Add Multi-segmented gamma support

Uma Shankar (1):
  drm/i915/icl: Add register definitions for Multi Segmented gamma

 drivers/gpu/drm/i915/i915_pci.c          |   2 +-
 drivers/gpu/drm/i915/i915_reg.h          |  19 ++-
 drivers/gpu/drm/i915/intel_color.c       | 141 +++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_device_info.h |   4 +-
 4 files changed, 151 insertions(+), 15 deletions(-)

-- 
2.17.1

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

[PATCH v3 1/4] drm/i915: Change gamma/degamma_lut_size data type to u32

[Shashank Sharma]

Currently, data type of gamma_lut_size & degamma_lut_size elements
in intel_device_info is u16, which means it can accommodate maximum
64k values. In case of ICL multisegmented gamma, the size of gamma
LUT is 256K.

This patch changes the data type of both of these elements to u32.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_device_info.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 5a2e17d6146b..67677c356716 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -179,8 +179,8 @@ struct intel_device_info {
 	int cursor_offsets[I915_MAX_PIPES];
 
 	struct color_luts {
-		u16 degamma_lut_size;
-		u16 gamma_lut_size;
+		u32 degamma_lut_size;
+		u32 gamma_lut_size;
 		u32 degamma_lut_tests;
 		u32 gamma_lut_tests;
 	} color;
-- 
2.17.1

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

[PATCH v3 2/4] drm/i915/icl: Add register definitions for Multi Segmented gamma

[Shashank Sharma]

From: Uma Shankar <uma.shankar@intel.com>

Add macros to define multi segmented gamma registers

V2: Addressed Ville's comments:
    	Add gen-lable before bit definition
    Addressed Jani's comment
	- Use REG_GENMASK() and REG_BIT()
V3: Addressed Ville's comments:
    - Put comments at the end of line.
    - Change the comment at start of ICL multisegmented gamma registers.
    Added Ville's r-b

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Jani Nikula <jani.nikula@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index e97c47fca645..8b77c067e26b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7198,7 +7198,8 @@ enum {
 #define  GAMMA_MODE_MODE_8BIT	(0 << 0)
 #define  GAMMA_MODE_MODE_10BIT	(1 << 0)
 #define  GAMMA_MODE_MODE_12BIT	(2 << 0)
-#define  GAMMA_MODE_MODE_SPLIT	(3 << 0)
+#define  GAMMA_MODE_MODE_SPLIT	(3 << 0) /* ivb-bdw */
+#define  GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED	(3 << 0) /* icl + */
 
 /* DMC/CSR */
 #define CSR_PROGRAM(i)		_MMIO(0x80000 + (i) * 4)
@@ -10145,6 +10146,22 @@ enum skl_power_gate {
 #define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
 #define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe, _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
 
+/* ICL Multi segmented gamma */
+#define _PAL_PREC_MULTI_SEG_INDEX_A	0x4A408
+#define _PAL_PREC_MULTI_SEG_INDEX_B	0x4AC08
+#define  PAL_PREC_MULTI_SEGMENT_AUTO_INCREMENT		REG_BIT(15)
+#define  PAL_PREC_MULTI_SEGMENT_INDEX_VALUE_MASK	REG_GENMASK(4, 0)
+
+#define _PAL_PREC_MULTI_SEG_DATA_A	0x4A40C
+#define _PAL_PREC_MULTI_SEG_DATA_B	0x4AC0C
+
+#define PREC_PAL_MULTI_SEG_INDEX(pipe)	_MMIO_PIPE(pipe, \
+					_PAL_PREC_MULTI_SEG_INDEX_A, \
+					_PAL_PREC_MULTI_SEG_INDEX_B)
+#define PREC_PAL_MULTI_SEG_DATA(pipe)	_MMIO_PIPE(pipe, \
+					_PAL_PREC_MULTI_SEG_DATA_A, \
+					_PAL_PREC_MULTI_SEG_DATA_B)
+
 /* pipe CSC & degamma/gamma LUTs on CHV */
 #define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
 #define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)
-- 
2.17.1

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

[PATCH v3 3/4] drm/i915: Rename ivb_load_lut_10_max

[Shashank Sharma]

This patch renames function ivb_load_lut_10_max to
ivb_load_lut_ext_max.

V3: Added Vill'es r-b.

Cc: Uma Shankar <uma.shankar@intel.com>

Suggested-by: Ville Syrjala <ville.syrjala@linux.intel.com>
Reviewed-by: Ville Syrjala <ville.syrjala@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 962db1236970..6c341bea514c 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -607,7 +607,7 @@ static void bdw_load_lut_10(struct intel_crtc *crtc,
 	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
 }
 
-static void ivb_load_lut_10_max(struct intel_crtc *crtc)
+static void ivb_load_lut_ext_max(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
@@ -640,7 +640,7 @@ static void ivb_load_luts(const struct intel_crtc_state *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));
-		ivb_load_lut_10_max(crtc);
+		ivb_load_lut_ext_max(crtc);
 		ivb_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
 				PAL_PREC_INDEX_VALUE(512));
 	} else {
@@ -648,7 +648,7 @@ static void ivb_load_luts(const struct intel_crtc_state *crtc_state)
 
 		ivb_load_lut_10(crtc, blob,
 				PAL_PREC_INDEX_VALUE(0));
-		ivb_load_lut_10_max(crtc);
+		ivb_load_lut_ext_max(crtc);
 	}
 }
 
@@ -663,7 +663,7 @@ static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
 	} else if (crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT) {
 		bdw_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
 				PAL_PREC_INDEX_VALUE(0));
-		ivb_load_lut_10_max(crtc);
+		ivb_load_lut_ext_max(crtc);
 		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
 				PAL_PREC_INDEX_VALUE(512));
 	} else {
@@ -671,7 +671,7 @@ static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
 
 		bdw_load_lut_10(crtc, blob,
 				PAL_PREC_INDEX_VALUE(0));
-		ivb_load_lut_10_max(crtc);
+		ivb_load_lut_ext_max(crtc);
 	}
 }
 
@@ -763,7 +763,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));
-		ivb_load_lut_10_max(crtc);
+		ivb_load_lut_ext_max(crtc);
 	}
 }
 
@@ -780,7 +780,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));
-		ivb_load_lut_10_max(crtc);
+		ivb_load_lut_ext_max(crtc);
 	}
 }
 
-- 
2.17.1

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

[PATCH v3 4/4] drm/i915/icl: Add Multi-segmented gamma support

[Shashank Sharma]

ICL introduces a new gamma correction mode in display engine, called
multi-segmented-gamma mode. This mode allows users to program the
darker region of the gamma curve with sueprfine precision. An
example use case for this is HDR curves (like PQ ST-2084).

If we plot a gamma correction curve from value range between 0.0 to 1.0,
ICL's multi-segment has 3 different sections:
- superfine segment: 9 values, ranges between 0 - 1/(128 * 256)
- fine segment: 257 values, ranges between 0 - 1/(128)
- corase segment: 257 values, ranges between 0 - 1

This patch:
- Changes gamma LUTs size for ICL/GEN11 to 262144 entries (8 * 128 * 256),
  so that userspace can program with highest precision supported.
- Changes default gamma mode (non-legacy) to multi-segmented-gamma mode.
- Adds functions to program/detect multi-segment gamma.

V2: Addressed review comments from Ville
    - separate function for superfine and fine segments.
    - remove enum for segments.
    - reuse last entry of the LUT as gc_max value.
    - replace if() ....cond with switch...case in icl_load_luts.
    - add an entry variable, instead of 'word'

V3: Addressed review comments from Ville
    - extra newline
    - s/entry/color/
    - remove LUT size checks
    - program ilk_lut_12p4_ldw value before ilk_lut_12p4_udw
    - Change the comments in description of fine and coarse segments,
      and try to make more sense.
    - use 8 * 128 instead of 1024
    - add 1 entry in LUT for GCMAX

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>

Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c    |   2 +-
 drivers/gpu/drm/i915/intel_color.c | 127 ++++++++++++++++++++++++++++-
 2 files changed, 124 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index ffa2ee70a03d..2f99b585d44b 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -749,7 +749,7 @@ static const struct intel_device_info intel_cannonlake_info = {
 	GEN(11), \
 	.ddb_size = 2048, \
 	.has_logical_ring_elsq = 1, \
-	.color = { .degamma_lut_size = 33, .gamma_lut_size = 1024 }
+	.color = { .degamma_lut_size = 33, .gamma_lut_size = 262145 }
 
 static const struct intel_device_info intel_icelake_11_info = {
 	GEN11_FEATURES,
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 6c341bea514c..c1a9506fd069 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -41,6 +41,8 @@
 #define CTM_COEFF_ABS(coeff)		((coeff) & (CTM_COEFF_SIGN - 1))
 
 #define LEGACY_LUT_LENGTH		256
+#define ICL_GAMMA_MULTISEG_LUT_LENGTH		(256 * 128 * 8)
+
 /*
  * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
  * format). This macro takes the coefficient we want transformed and the
@@ -767,6 +769,116 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
 	}
 }
 
+/* ilk+ "12.4" interpolated format (high 10 bits) */
+static u32 ilk_lut_12p4_ldw(const struct drm_color_lut *color)
+{
+	return (color->red >> 6) << 20 | (color->green >> 6) << 10 |
+		(color->blue >> 6);
+}
+
+/* ilk+ "12.4" interpolated format (low 6 bits) */
+static u32 ilk_lut_12p4_udw(const struct drm_color_lut *color)
+{
+	return (color->red & 0x3f) << 24 | (color->green & 0x3f) << 14 |
+		(color->blue & 0x3f);
+}
+
+static void
+icl_load_gcmax(const struct intel_crtc_state *crtc_state,
+	       const struct drm_color_lut *color)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+
+	/* Fixme: LUT entries are 16 bit only, so we can prog 0xFFFF max */
+	I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), color->red);
+	I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), color->green);
+	I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), color->blue);
+}
+
+static void
+icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
+	const struct drm_color_lut *lut = blob->data;
+	enum pipe pipe = crtc->pipe;
+	u32 i;
+
+	/*
+	 * Every entry in the multi-segment LUT is corresponding to a superfine
+	 * segment step which is 1/(8 * 128 * 256).
+	 *
+	 * Superfine segment has 9 entries, corresponding to values
+	 * 0, 1/(8 * 128 * 256), 2/(8 * 128 * 256) .... 8/(8 * 128 * 256).
+	 */
+	I915_WRITE(PREC_PAL_MULTI_SEG_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
+
+	for (i = 0; i < 9; i++) {
+		const struct drm_color_lut *entry = &lut[i];
+
+		I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe),
+			   ilk_lut_12p4_ldw(entry));
+		I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe),
+			   ilk_lut_12p4_udw(entry));
+	}
+}
+
+static void
+icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
+	const struct drm_color_lut *lut = blob->data;
+	const struct drm_color_lut *entry;
+	enum pipe pipe = crtc->pipe;
+	u32 i;
+
+	/*
+	 *
+	 * Program Fine segment (let's call it seg2)...
+	 *
+	 * Fine segment's step is 1/(128 * 256) ie 1/(128 * 256),  2/(128*256)
+	 * ... 256/(128*256). So in order to program fine segment of LUT we
+	 * need to pick every 8'th entry in LUT, and program 256 indexes.
+	 *
+	 * PAL_PREC_INDEX[0] and PAL_PREC_INDEX[1] map to seg2[1],
+	 * with seg2[0] being unused by the hardware.
+	 */
+	I915_WRITE(PREC_PAL_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
+	for (i = 1; i < 257; i++) {
+		entry = &lut[i * 8];
+		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_ldw(entry));
+		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_udw(entry));
+	}
+
+	/*
+	 * Program Coarse segment (let's call it seg3)...
+	 *
+	 * Coarse segment's starts from index 0 and it's step is 1/256 ie 0,
+	 * 1/256, 2/256 ...256/256. As per the description of each entry in LUT
+	 * above, we need to pick every (8 * 128)th entry in LUT, and
+	 * program 256 of those.
+	 *
+	 * Spec is not very clear about if entries seg3[0] and seg3[1] are
+	 * being used or not, but we still need to program these to advance
+	 * the index.
+	 */
+	for (i = 0; i < 256; i++) {
+		entry = &lut[i * 8 * 128];
+		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_ldw(entry));
+		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_udw(entry));
+	}
+
+	/* The last entry in the LUT is to be programmed in GCMAX */
+	entry = &lut[256 * 8 * 128 + 1];
+	icl_load_gcmax(crtc_state, entry);
+	ivb_load_lut_ext_max(crtc);
+}
+
 static void icl_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
@@ -775,10 +887,17 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
 	if (crtc_state->base.degamma_lut)
 		glk_load_degamma_lut(crtc_state);
 
-	if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
-	    GAMMA_MODE_MODE_8BIT) {
+	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
+	case GAMMA_MODE_MODE_8BIT:
 		i9xx_load_luts(crtc_state);
-	} else {
+		break;
+
+	case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
+		icl_program_gamma_superfine_segment(crtc_state);
+		icl_program_gamma_multi_segment(crtc_state);
+		break;
+
+	default:
 		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
 		ivb_load_lut_ext_max(crtc);
 	}
@@ -1209,7 +1328,7 @@ static u32 icl_gamma_mode(const struct intel_crtc_state *crtc_state)
 	    crtc_state_is_legacy_gamma(crtc_state))
 		gamma_mode |= GAMMA_MODE_MODE_8BIT;
 	else
-		gamma_mode |= GAMMA_MODE_MODE_10BIT;
+		gamma_mode |= GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED;
 
 	return gamma_mode;
 }
-- 
2.17.1

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

Re: [PATCH v3 4/4] drm/i915/icl: Add Multi-segmented gamma support

[Ville Syrjälä]

On Tue, May 07, 2019 at 07:26:44PM +0530, Shashank Sharma wrote:
> ICL introduces a new gamma correction mode in display engine, called
> multi-segmented-gamma mode. This mode allows users to program the
> darker region of the gamma curve with sueprfine precision. An
> example use case for this is HDR curves (like PQ ST-2084).
> 
> If we plot a gamma correction curve from value range between 0.0 to 1.0,
> ICL's multi-segment has 3 different sections:
> - superfine segment: 9 values, ranges between 0 - 1/(128 * 256)
> - fine segment: 257 values, ranges between 0 - 1/(128)
> - corase segment: 257 values, ranges between 0 - 1
> 
> This patch:
> - Changes gamma LUTs size for ICL/GEN11 to 262144 entries (8 * 128 * 256),
>   so that userspace can program with highest precision supported.
> - Changes default gamma mode (non-legacy) to multi-segmented-gamma mode.
> - Adds functions to program/detect multi-segment gamma.
> 
> V2: Addressed review comments from Ville
>     - separate function for superfine and fine segments.
>     - remove enum for segments.
>     - reuse last entry of the LUT as gc_max value.
>     - replace if() ....cond with switch...case in icl_load_luts.
>     - add an entry variable, instead of 'word'
> 
> V3: Addressed review comments from Ville
>     - extra newline
>     - s/entry/color/
>     - remove LUT size checks
>     - program ilk_lut_12p4_ldw value before ilk_lut_12p4_udw
>     - Change the comments in description of fine and coarse segments,
>       and try to make more sense.
>     - use 8 * 128 instead of 1024
>     - add 1 entry in LUT for GCMAX
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_pci.c    |   2 +-
>  drivers/gpu/drm/i915/intel_color.c | 127 ++++++++++++++++++++++++++++-
>  2 files changed, 124 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index ffa2ee70a03d..2f99b585d44b 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -749,7 +749,7 @@ static const struct intel_device_info intel_cannonlake_info = {
>  	GEN(11), \
>  	.ddb_size = 2048, \
>  	.has_logical_ring_elsq = 1, \
> -	.color = { .degamma_lut_size = 33, .gamma_lut_size = 1024 }
> +	.color = { .degamma_lut_size = 33, .gamma_lut_size = 262145 }
>  
>  static const struct intel_device_info intel_icelake_11_info = {
>  	GEN11_FEATURES,
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 6c341bea514c..c1a9506fd069 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -41,6 +41,8 @@
>  #define CTM_COEFF_ABS(coeff)		((coeff) & (CTM_COEFF_SIGN - 1))
>  
>  #define LEGACY_LUT_LENGTH		256
> +#define ICL_GAMMA_MULTISEG_LUT_LENGTH		(256 * 128 * 8)

Unused.

> +
>  /*
>   * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
>   * format). This macro takes the coefficient we want transformed and the
> @@ -767,6 +769,116 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
>  	}
>  }
>  
> +/* ilk+ "12.4" interpolated format (high 10 bits) */
> +static u32 ilk_lut_12p4_ldw(const struct drm_color_lut *color)
> +{
> +	return (color->red >> 6) << 20 | (color->green >> 6) << 10 |
> +		(color->blue >> 6);
> +}
> +
> +/* ilk+ "12.4" interpolated format (low 6 bits) */
> +static u32 ilk_lut_12p4_udw(const struct drm_color_lut *color)
> +{
> +	return (color->red & 0x3f) << 24 | (color->green & 0x3f) << 14 |
> +		(color->blue & 0x3f);

Blue is missing the shift.

I'm not 100% sure if the ldw vs. udw are the right way around. The spec
has at times been inconsistent with the odd vs. even descriptions,
sometimes even contradicting itself. Also it never really defines
whether it starts counting dwords from from 0 or 1, so not sure what
odd and even actually mean. Can I presume someone has checked this
on actual hardware?

> +}
> +
> +static void
> +icl_load_gcmax(const struct intel_crtc_state *crtc_state,
> +	       const struct drm_color_lut *color)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	enum pipe pipe = crtc->pipe;
> +
> +	/* Fixme: LUT entries are 16 bit only, so we can prog 0xFFFF max */
> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), color->red);
> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), color->green);
> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), color->blue);
> +}
> +
> +static void
> +icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
> +	const struct drm_color_lut *lut = blob->data;
> +	enum pipe pipe = crtc->pipe;
> +	u32 i;
> +
> +	/*
> +	 * Every entry in the multi-segment LUT is corresponding to a superfine
> +	 * segment step which is 1/(8 * 128 * 256).
> +	 *
> +	 * Superfine segment has 9 entries, corresponding to values
> +	 * 0, 1/(8 * 128 * 256), 2/(8 * 128 * 256) .... 8/(8 * 128 * 256).
> +	 */
> +	I915_WRITE(PREC_PAL_MULTI_SEG_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
> +
> +	for (i = 0; i < 9; i++) {
> +		const struct drm_color_lut *entry = &lut[i];
> +
> +		I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe),
> +			   ilk_lut_12p4_ldw(entry));
> +		I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe),
> +			   ilk_lut_12p4_udw(entry));
> +	}
> +}
> +
> +static void
> +icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
> +	const struct drm_color_lut *lut = blob->data;
> +	const struct drm_color_lut *entry;
> +	enum pipe pipe = crtc->pipe;
> +	u32 i;
> +
> +	/*
> +	 *
> +	 * Program Fine segment (let's call it seg2)...
> +	 *
> +	 * Fine segment's step is 1/(128 * 256) ie 1/(128 * 256),  2/(128*256)
> +	 * ... 256/(128*256). So in order to program fine segment of LUT we
> +	 * need to pick every 8'th entry in LUT, and program 256 indexes.
> +	 *
> +	 * PAL_PREC_INDEX[0] and PAL_PREC_INDEX[1] map to seg2[1],
> +	 * with seg2[0] being unused by the hardware.
> +	 */
> +	I915_WRITE(PREC_PAL_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
> +	for (i = 1; i < 257; i++) {
> +		entry = &lut[i * 8];
> +		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_ldw(entry));
> +		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_udw(entry));
> +	}
> +
> +	/*
> +	 * Program Coarse segment (let's call it seg3)...
> +	 *
> +	 * Coarse segment's starts from index 0 and it's step is 1/256 ie 0,
> +	 * 1/256, 2/256 ...256/256. As per the description of each entry in LUT
> +	 * above, we need to pick every (8 * 128)th entry in LUT, and
> +	 * program 256 of those.
> +	 *
> +	 * Spec is not very clear about if entries seg3[0] and seg3[1] are
> +	 * being used or not, but we still need to program these to advance
> +	 * the index.
> +	 */
> +	for (i = 0; i < 256; i++) {
> +		entry = &lut[i * 8 * 128];
> +		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_ldw(entry));
> +		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_udw(entry));
> +	}
> +
> +	/* The last entry in the LUT is to be programmed in GCMAX */
> +	entry = &lut[256 * 8 * 128 + 1];

The +1 shouldn't be here.

OK, mostly looks good. With the minor issues addressed this is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +	icl_load_gcmax(crtc_state, entry);
> +	ivb_load_lut_ext_max(crtc);
> +}
> +
>  static void icl_load_luts(const struct intel_crtc_state *crtc_state)
>  {
>  	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
> @@ -775,10 +887,17 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
>  	if (crtc_state->base.degamma_lut)
>  		glk_load_degamma_lut(crtc_state);
>  
> -	if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
> -	    GAMMA_MODE_MODE_8BIT) {
> +	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
> +	case GAMMA_MODE_MODE_8BIT:
>  		i9xx_load_luts(crtc_state);
> -	} else {
> +		break;
> +
> +	case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
> +		icl_program_gamma_superfine_segment(crtc_state);
> +		icl_program_gamma_multi_segment(crtc_state);
> +		break;
> +
> +	default:
>  		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc);
>  	}
> @@ -1209,7 +1328,7 @@ static u32 icl_gamma_mode(const struct intel_crtc_state *crtc_state)
>  	    crtc_state_is_legacy_gamma(crtc_state))
>  		gamma_mode |= GAMMA_MODE_MODE_8BIT;
>  	else
> -		gamma_mode |= GAMMA_MODE_MODE_10BIT;
> +		gamma_mode |= GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED;
>  
>  	return gamma_mode;
>  }
> -- 
> 2.17.1

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

✓ Fi.CI.BAT: success for Enable Multi-segmented-gamma for ICL (rev2)

[Patchwork]

== Series Details ==

Series: Enable Multi-segmented-gamma for ICL (rev2)
URL   : https://patchwork.freedesktop.org/series/60126/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6059 -> Patchwork_12979
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_pm_rpm@module-reload:
    - fi-skl-6770hq:      [PASS][1] -> [FAIL][2] ([fdo#108511])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/fi-skl-6770hq/igt@i915_pm_rpm@module-reload.html

  * igt@i915_selftest@live_hangcheck:
    - fi-apl-guc:         [PASS][3] -> [DMESG-FAIL][4] ([fdo#110620])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/fi-apl-guc/igt@i915_selftest@live_hangcheck.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/fi-apl-guc/igt@i915_selftest@live_hangcheck.html

  
#### Possible fixes ####

  * igt@gem_exec_create@basic:
    - {fi-icl-y}:         [INCOMPLETE][5] ([fdo#107713]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/fi-icl-y/igt@gem_exec_create@basic.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/fi-icl-y/igt@gem_exec_create@basic.html

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         [INCOMPLETE][7] ([fdo#103927] / [fdo#109720]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/fi-apl-guc/igt@i915_selftest@live_execlists.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/fi-apl-guc/igt@i915_selftest@live_execlists.html

  * igt@kms_chamelium@dp-crc-fast:
    - {fi-icl-u2}:        [FAIL][9] ([fdo#109635 ] / [fdo#110387]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/fi-icl-u2/igt@kms_chamelium@dp-crc-fast.html

  
#### Warnings ####

  * igt@runner@aborted:
    - fi-apl-guc:         [FAIL][11] ([fdo#108622] / [fdo#109720]) -> [FAIL][12] ([fdo#110622])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/fi-apl-guc/igt@runner@aborted.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/fi-apl-guc/igt@runner@aborted.html

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

  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108511]: https://bugs.freedesktop.org/show_bug.cgi?id=108511
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#109635 ]: https://bugs.freedesktop.org/show_bug.cgi?id=109635 
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#110387]: https://bugs.freedesktop.org/show_bug.cgi?id=110387
  [fdo#110620]: https://bugs.freedesktop.org/show_bug.cgi?id=110620
  [fdo#110622]: https://bugs.freedesktop.org/show_bug.cgi?id=110622


Participating hosts (52 -> 45)
------------------------------

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_6059 -> Patchwork_12979

  CI_DRM_6059: dd7af767d072fa866d5bc2d24ff225bf82f2ad11 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4972: f052e49a43cc9704ea5f240df15dd9d3dfed68ab @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12979: 7273540608c5ff721a80fe39ef64603e5c00fcbe @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

7273540608c5 drm/i915/icl: Add Multi-segmented gamma support
85df31f91908 drm/i915: Rename ivb_load_lut_10_max
522287b39e22 drm/i915/icl: Add register definitions for Multi Segmented gamma
96f3177a1623 drm/i915: Change gamma/degamma_lut_size data type to u32

== Logs ==

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

✓ Fi.CI.IGT: success for Enable Multi-segmented-gamma for ICL (rev2)

[Patchwork]

== Series Details ==

Series: Enable Multi-segmented-gamma for ICL (rev2)
URL   : https://patchwork.freedesktop.org/series/60126/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6059_full -> Patchwork_12979_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_color@pipe-c-gamma:
    - shard-iclb:         [PASS][1] -> [FAIL][2] ([fdo#104782]) +4 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-iclb4/igt@kms_color@pipe-c-gamma.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-iclb1/igt@kms_color@pipe-c-gamma.html

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-skl:          [PASS][3] -> [INCOMPLETE][4] ([fdo#104108])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-skl5/igt@kms_cursor_crc@cursor-64x64-suspend.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-skl10/igt@kms_cursor_crc@cursor-64x64-suspend.html

  * igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy:
    - shard-hsw:          [PASS][5] -> [FAIL][6] ([fdo#105767])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-hsw5/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-hsw6/igt@kms_cursor_legacy@2x-long-cursor-vs-flip-legacy.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-hsw:          [PASS][7] -> [INCOMPLETE][8] ([fdo#103540])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-hsw4/igt@kms_flip@flip-vs-suspend-interruptible.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-hsw5/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_flip@plain-flip-fb-recreate-interruptible:
    - shard-skl:          [PASS][9] -> [FAIL][10] ([fdo#100368])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-skl4/igt@kms_flip@plain-flip-fb-recreate-interruptible.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-skl4/igt@kms_flip@plain-flip-fb-recreate-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-cpu:
    - shard-iclb:         [PASS][11] -> [FAIL][12] ([fdo#103167]) +5 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-cpu.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-iclb6/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-pri-indfb-draw-mmap-cpu.html

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-glk:          [PASS][13] -> [SKIP][14] ([fdo#109271]) +2 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-glk9/igt@kms_plane@pixel-format-pipe-c-planes-source-clamping.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-glk8/igt@kms_plane@pixel-format-pipe-c-planes-source-clamping.html

  * igt@kms_plane@plane-position-hole-dpms-pipe-b-planes:
    - shard-snb:          [PASS][15] -> [SKIP][16] ([fdo#109271]) +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-snb7/igt@kms_plane@plane-position-hole-dpms-pipe-b-planes.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-snb4/igt@kms_plane@plane-position-hole-dpms-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [PASS][17] -> [FAIL][18] ([fdo#108145])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-skl8/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-skl1/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [PASS][19] -> [FAIL][20] ([fdo#103166])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-iclb4/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_sprite_mmap_gtt:
    - shard-iclb:         [PASS][21] -> [SKIP][22] ([fdo#109441]) +2 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-iclb2/igt@kms_psr@psr2_sprite_mmap_gtt.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-iclb8/igt@kms_psr@psr2_sprite_mmap_gtt.html

  * igt@kms_setmode@basic:
    - shard-apl:          [PASS][23] -> [FAIL][24] ([fdo#99912])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-apl6/igt@kms_setmode@basic.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-apl8/igt@kms_setmode@basic.html

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         [PASS][25] -> [FAIL][26] ([fdo#100047])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-iclb6/igt@kms_sysfs_edid_timing.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-iclb2/igt@kms_sysfs_edid_timing.html

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-apl:          [PASS][27] -> [DMESG-WARN][28] ([fdo#108566]) +5 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-apl4/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-apl1/igt@kms_vblank@pipe-c-ts-continuation-suspend.html

  
#### Possible fixes ####

  * igt@gem_eio@reset-stress:
    - shard-snb:          [FAIL][29] ([fdo#109661]) -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-snb7/igt@gem_eio@reset-stress.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-snb5/igt@gem_eio@reset-stress.html

  * igt@gem_exec_suspend@basic-s4-devices:
    - shard-snb:          [FAIL][31] ([fdo#107918]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-snb2/igt@gem_exec_suspend@basic-s4-devices.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-snb5/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@i915_pm_rpm@basic-rte:
    - shard-skl:          [INCOMPLETE][33] ([fdo#107807]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-skl7/igt@i915_pm_rpm@basic-rte.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-skl9/igt@i915_pm_rpm@basic-rte.html

  * igt@i915_pm_rpm@system-suspend:
    - shard-skl:          [INCOMPLETE][35] ([fdo#104108] / [fdo#107807]) -> [PASS][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-skl2/igt@i915_pm_rpm@system-suspend.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-skl4/igt@i915_pm_rpm@system-suspend.html

  * igt@i915_suspend@debugfs-reader:
    - shard-apl:          [DMESG-WARN][37] ([fdo#108566]) -> [PASS][38] +5 similar issues
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-apl3/igt@i915_suspend@debugfs-reader.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-apl3/igt@i915_suspend@debugfs-reader.html

  * igt@kms_busy@extended-modeset-hang-oldfb-with-reset-render-c:
    - shard-glk:          [INCOMPLETE][39] ([fdo#103359] / [k.org#198133]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-glk3/igt@kms_busy@extended-modeset-hang-oldfb-with-reset-render-c.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-glk3/igt@kms_busy@extended-modeset-hang-oldfb-with-reset-render-c.html

  * igt@kms_flip@2x-flip-vs-suspend-interruptible:
    - shard-hsw:          [INCOMPLETE][41] ([fdo#103540]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-hsw8/igt@kms_flip@2x-flip-vs-suspend-interruptible.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-hsw8/igt@kms_flip@2x-flip-vs-suspend-interruptible.html

  * igt@kms_flip@dpms-vs-vblank-race-interruptible:
    - shard-skl:          [FAIL][43] ([fdo#103060]) -> [PASS][44]
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-skl1/igt@kms_flip@dpms-vs-vblank-race-interruptible.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-skl5/igt@kms_flip@dpms-vs-vblank-race-interruptible.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt:
    - shard-iclb:         [FAIL][45] ([fdo#103167]) -> [PASS][46] +2 similar issues
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-iclb2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-iclb8/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-blt.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-gtt:
    - shard-skl:          [FAIL][47] ([fdo#103167]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-skl3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-gtt.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-skl10/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-mmap-gtt.html

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          [FAIL][49] ([fdo#108145]) -> [PASS][50] +1 similar issue
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-skl2/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-skl4/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html

  * igt@kms_plane_scaling@pipe-a-scaler-with-rotation:
    - shard-glk:          [SKIP][51] ([fdo#109271] / [fdo#109278]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-glk6/igt@kms_plane_scaling@pipe-a-scaler-with-rotation.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-glk9/igt@kms_plane_scaling@pipe-a-scaler-with-rotation.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [SKIP][53] ([fdo#109441]) -> [PASS][54] +2 similar issues
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-iclb7/igt@kms_psr@psr2_cursor_render.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-iclb2/igt@kms_psr@psr2_cursor_render.html

  
#### Warnings ####

  * igt@kms_atomic_transition@3x-modeset-transitions:
    - shard-snb:          [SKIP][55] ([fdo#109271] / [fdo#109278]) -> [SKIP][56] ([fdo#109271])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-snb7/igt@kms_atomic_transition@3x-modeset-transitions.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-snb4/igt@kms_atomic_transition@3x-modeset-transitions.html

  * igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-pri-indfb-draw-pwrite:
    - shard-apl:          [INCOMPLETE][57] ([fdo#103927]) -> [SKIP][58] ([fdo#109271])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6059/shard-apl5/igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-pri-indfb-draw-pwrite.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12979/shard-apl6/igt@kms_frontbuffer_tracking@fbcpsr-2p-primscrn-pri-indfb-draw-pwrite.html

  
  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105767]: https://bugs.freedesktop.org/show_bug.cgi?id=105767
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107918]: https://bugs.freedesktop.org/show_bug.cgi?id=107918
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [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#109661]: https://bugs.freedesktop.org/show_bug.cgi?id=109661
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


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

  No changes in participating hosts


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

  * Linux: CI_DRM_6059 -> Patchwork_12979

  CI_DRM_6059: dd7af767d072fa866d5bc2d24ff225bf82f2ad11 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4972: f052e49a43cc9704ea5f240df15dd9d3dfed68ab @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12979: 7273540608c5ff721a80fe39ef64603e5c00fcbe @ 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_12979/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 4/4] drm/i915/icl: Add Multi-segmented gamma support

[Sharma, Shashank]

On 5/7/2019 7:57 PM, Ville Syrjälä wrote:
> On Tue, May 07, 2019 at 07:26:44PM +0530, Shashank Sharma wrote:
>> ICL introduces a new gamma correction mode in display engine, called
>> multi-segmented-gamma mode. This mode allows users to program the
>> darker region of the gamma curve with sueprfine precision. An
>> example use case for this is HDR curves (like PQ ST-2084).
>>
>> If we plot a gamma correction curve from value range between 0.0 to 1.0,
>> ICL's multi-segment has 3 different sections:
>> - superfine segment: 9 values, ranges between 0 - 1/(128 * 256)
>> - fine segment: 257 values, ranges between 0 - 1/(128)
>> - corase segment: 257 values, ranges between 0 - 1
>>
>> This patch:
>> - Changes gamma LUTs size for ICL/GEN11 to 262144 entries (8 * 128 * 256),
>>    so that userspace can program with highest precision supported.
>> - Changes default gamma mode (non-legacy) to multi-segmented-gamma mode.
>> - Adds functions to program/detect multi-segment gamma.
>>
>> V2: Addressed review comments from Ville
>>      - separate function for superfine and fine segments.
>>      - remove enum for segments.
>>      - reuse last entry of the LUT as gc_max value.
>>      - replace if() ....cond with switch...case in icl_load_luts.
>>      - add an entry variable, instead of 'word'
>>
>> V3: Addressed review comments from Ville
>>      - extra newline
>>      - s/entry/color/
>>      - remove LUT size checks
>>      - program ilk_lut_12p4_ldw value before ilk_lut_12p4_udw
>>      - Change the comments in description of fine and coarse segments,
>>        and try to make more sense.
>>      - use 8 * 128 instead of 1024
>>      - add 1 entry in LUT for GCMAX
>>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>
>> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_pci.c    |   2 +-
>>   drivers/gpu/drm/i915/intel_color.c | 127 ++++++++++++++++++++++++++++-
>>   2 files changed, 124 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
>> index ffa2ee70a03d..2f99b585d44b 100644
>> --- a/drivers/gpu/drm/i915/i915_pci.c
>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>> @@ -749,7 +749,7 @@ static const struct intel_device_info intel_cannonlake_info = {
>>   	GEN(11), \
>>   	.ddb_size = 2048, \
>>   	.has_logical_ring_elsq = 1, \
>> -	.color = { .degamma_lut_size = 33, .gamma_lut_size = 1024 }
>> +	.color = { .degamma_lut_size = 33, .gamma_lut_size = 262145 }
>>   
>>   static const struct intel_device_info intel_icelake_11_info = {
>>   	GEN11_FEATURES,
>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>> index 6c341bea514c..c1a9506fd069 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -41,6 +41,8 @@
>>   #define CTM_COEFF_ABS(coeff)		((coeff) & (CTM_COEFF_SIGN - 1))
>>   
>>   #define LEGACY_LUT_LENGTH		256
>> +#define ICL_GAMMA_MULTISEG_LUT_LENGTH		(256 * 128 * 8)
> Unused.
Got it
>> +
>>   /*
>>    * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
>>    * format). This macro takes the coefficient we want transformed and the
>> @@ -767,6 +769,116 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
>>   	}
>>   }
>>   
>> +/* ilk+ "12.4" interpolated format (high 10 bits) */
>> +static u32 ilk_lut_12p4_ldw(const struct drm_color_lut *color)
>> +{
>> +	return (color->red >> 6) << 20 | (color->green >> 6) << 10 |
>> +		(color->blue >> 6);
>> +}
>> +
>> +/* ilk+ "12.4" interpolated format (low 6 bits) */
>> +static u32 ilk_lut_12p4_udw(const struct drm_color_lut *color)
>> +{
>> +	return (color->red & 0x3f) << 24 | (color->green & 0x3f) << 14 |
>> +		(color->blue & 0x3f);
> Blue is missing the shift.
Ok,
> I'm not 100% sure if the ldw vs. udw are the right way around. The spec
> has at times been inconsistent with the odd vs. even descriptions,
> sometimes even contradicting itself. Also it never really defines
> whether it starts counting dwords from from 0 or 1, so not sure what
> odd and even actually mean. Can I presume someone has checked this
> on actual hardware?
Well, the property was getting set properly, and the display looked ok, 
but dint dump the values in registers. Can check it now.
>> +}
>> +
>> +static void
>> +icl_load_gcmax(const struct intel_crtc_state *crtc_state,
>> +	       const struct drm_color_lut *color)
>> +{
>> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> +	enum pipe pipe = crtc->pipe;
>> +
>> +	/* Fixme: LUT entries are 16 bit only, so we can prog 0xFFFF max */
>> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), color->red);
>> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), color->green);
>> +	I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), color->blue);
>> +}
>> +
>> +static void
>> +icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
>> +{
>> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> +	const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
>> +	const struct drm_color_lut *lut = blob->data;
>> +	enum pipe pipe = crtc->pipe;
>> +	u32 i;
>> +
>> +	/*
>> +	 * Every entry in the multi-segment LUT is corresponding to a superfine
>> +	 * segment step which is 1/(8 * 128 * 256).
>> +	 *
>> +	 * Superfine segment has 9 entries, corresponding to values
>> +	 * 0, 1/(8 * 128 * 256), 2/(8 * 128 * 256) .... 8/(8 * 128 * 256).
>> +	 */
>> +	I915_WRITE(PREC_PAL_MULTI_SEG_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
>> +
>> +	for (i = 0; i < 9; i++) {
>> +		const struct drm_color_lut *entry = &lut[i];
>> +
>> +		I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe),
>> +			   ilk_lut_12p4_ldw(entry));
>> +		I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe),
>> +			   ilk_lut_12p4_udw(entry));
>> +	}
>> +}
>> +
>> +static void
>> +icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
>> +{
>> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> +	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> +	const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
>> +	const struct drm_color_lut *lut = blob->data;
>> +	const struct drm_color_lut *entry;
>> +	enum pipe pipe = crtc->pipe;
>> +	u32 i;
>> +
>> +	/*
>> +	 *
>> +	 * Program Fine segment (let's call it seg2)...
>> +	 *
>> +	 * Fine segment's step is 1/(128 * 256) ie 1/(128 * 256),  2/(128*256)
>> +	 * ... 256/(128*256). So in order to program fine segment of LUT we
>> +	 * need to pick every 8'th entry in LUT, and program 256 indexes.
>> +	 *
>> +	 * PAL_PREC_INDEX[0] and PAL_PREC_INDEX[1] map to seg2[1],
>> +	 * with seg2[0] being unused by the hardware.
>> +	 */
>> +	I915_WRITE(PREC_PAL_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
>> +	for (i = 1; i < 257; i++) {
>> +		entry = &lut[i * 8];
>> +		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_ldw(entry));
>> +		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_udw(entry));
>> +	}
>> +
>> +	/*
>> +	 * Program Coarse segment (let's call it seg3)...
>> +	 *
>> +	 * Coarse segment's starts from index 0 and it's step is 1/256 ie 0,
>> +	 * 1/256, 2/256 ...256/256. As per the description of each entry in LUT
>> +	 * above, we need to pick every (8 * 128)th entry in LUT, and
>> +	 * program 256 of those.
>> +	 *
>> +	 * Spec is not very clear about if entries seg3[0] and seg3[1] are
>> +	 * being used or not, but we still need to program these to advance
>> +	 * the index.
>> +	 */
>> +	for (i = 0; i < 256; i++) {
>> +		entry = &lut[i * 8 * 128];
>> +		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_ldw(entry));
>> +		I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_udw(entry));
>> +	}
>> +
>> +	/* The last entry in the LUT is to be programmed in GCMAX */
>> +	entry = &lut[256 * 8 * 128 + 1];
> The +1 shouldn't be here.
ok
> OK, mostly looks good. With the minor issues addressed this is
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Thanks for the review, I will publish V4 with comments addressed.

- Shashank

>> +	icl_load_gcmax(crtc_state, entry);
>> +	ivb_load_lut_ext_max(crtc);
>> +}
>> +
>>   static void icl_load_luts(const struct intel_crtc_state *crtc_state)
>>   {
>>   	const struct drm_property_blob *gamma_lut = crtc_state->base.gamma_lut;
>> @@ -775,10 +887,17 @@ static void icl_load_luts(const struct intel_crtc_state *crtc_state)
>>   	if (crtc_state->base.degamma_lut)
>>   		glk_load_degamma_lut(crtc_state);
>>   
>> -	if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
>> -	    GAMMA_MODE_MODE_8BIT) {
>> +	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
>> +	case GAMMA_MODE_MODE_8BIT:
>>   		i9xx_load_luts(crtc_state);
>> -	} else {
>> +		break;
>> +
>> +	case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
>> +		icl_program_gamma_superfine_segment(crtc_state);
>> +		icl_program_gamma_multi_segment(crtc_state);
>> +		break;
>> +
>> +	default:
>>   		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
>>   		ivb_load_lut_ext_max(crtc);
>>   	}
>> @@ -1209,7 +1328,7 @@ static u32 icl_gamma_mode(const struct intel_crtc_state *crtc_state)
>>   	    crtc_state_is_legacy_gamma(crtc_state))
>>   		gamma_mode |= GAMMA_MODE_MODE_8BIT;
>>   	else
>> -		gamma_mode |= GAMMA_MODE_MODE_10BIT;
>> +		gamma_mode |= GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED;
>>   
>>   	return gamma_mode;
>>   }
>> -- 
>> 2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 1/4] drm/i915: Change gamma/degamma_lut_size data type to u32

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Shashank
>Sent: Tuesday, May 7, 2019 7:27 PM
>To: intel-gfx@lists.freedesktop.org
>Cc: Sharma, Shashank <shashank.sharma@intel.com>; Ville Syrjälä
><ville.syrjala@linux.intel.com>; Maarten Lankhorst
><maarten.lankhorst@linux.intel.com>; Shankar, Uma <uma.shankar@intel.com>
>Subject: [PATCH v3 1/4] drm/i915: Change gamma/degamma_lut_size data type to
>u32
>
>Currently, data type of gamma_lut_size & degamma_lut_size elements in
>intel_device_info is u16, which means it can accommodate maximum 64k values. In
>case of ICL multisegmented gamma, the size of gamma LUT is 256K.
>
>This patch changes the data type of both of these elements to u32.

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

>Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>Cc: Uma Shankar <uma.shankar@intel.com>
>
>Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>---
> drivers/gpu/drm/i915/intel_device_info.h | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/intel_device_info.h
>b/drivers/gpu/drm/i915/intel_device_info.h
>index 5a2e17d6146b..67677c356716 100644
>--- a/drivers/gpu/drm/i915/intel_device_info.h
>+++ b/drivers/gpu/drm/i915/intel_device_info.h
>@@ -179,8 +179,8 @@ struct intel_device_info {
> 	int cursor_offsets[I915_MAX_PIPES];
>
> 	struct color_luts {
>-		u16 degamma_lut_size;
>-		u16 gamma_lut_size;
>+		u32 degamma_lut_size;
>+		u32 gamma_lut_size;
> 		u32 degamma_lut_tests;
> 		u32 gamma_lut_tests;
> 	} color;
>--
>2.17.1

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

Re: [PATCH v3 4/4] drm/i915/icl: Add Multi-segmented gamma support

[Sharma, Shashank]

We (Me and Uma) confirmed the ICL register programming sequence, by 
dumping the registers.

The correct sequence should be:

ilk_lut_12p4_udw

ilk_lut_12p4_ldw

We passed maximum value LUT (1.0) and saw only blue output, if 
programmed in opposite sequence.

Programming in above mentioned sequence gives a proper white output.

Regards
Shashank
On 5/8/2019 6:35 PM, Sharma, Shashank wrote:
> On 5/7/2019 7:57 PM, Ville Syrjälä wrote:
>> On Tue, May 07, 2019 at 07:26:44PM +0530, Shashank Sharma wrote:
>>> ICL introduces a new gamma correction mode in display engine, called
>>> multi-segmented-gamma mode. This mode allows users to program the
>>> darker region of the gamma curve with sueprfine precision. An
>>> example use case for this is HDR curves (like PQ ST-2084).
>>>
>>> If we plot a gamma correction curve from value range between 0.0 to 
>>> 1.0,
>>> ICL's multi-segment has 3 different sections:
>>> - superfine segment: 9 values, ranges between 0 - 1/(128 * 256)
>>> - fine segment: 257 values, ranges between 0 - 1/(128)
>>> - corase segment: 257 values, ranges between 0 - 1
>>>
>>> This patch:
>>> - Changes gamma LUTs size for ICL/GEN11 to 262144 entries (8 * 128 * 
>>> 256),
>>>    so that userspace can program with highest precision supported.
>>> - Changes default gamma mode (non-legacy) to multi-segmented-gamma 
>>> mode.
>>> - Adds functions to program/detect multi-segment gamma.
>>>
>>> V2: Addressed review comments from Ville
>>>      - separate function for superfine and fine segments.
>>>      - remove enum for segments.
>>>      - reuse last entry of the LUT as gc_max value.
>>>      - replace if() ....cond with switch...case in icl_load_luts.
>>>      - add an entry variable, instead of 'word'
>>>
>>> V3: Addressed review comments from Ville
>>>      - extra newline
>>>      - s/entry/color/
>>>      - remove LUT size checks
>>>      - program ilk_lut_12p4_ldw value before ilk_lut_12p4_udw
>>>      - Change the comments in description of fine and coarse segments,
>>>        and try to make more sense.
>>>      - use 8 * 128 instead of 1024
>>>      - add 1 entry in LUT for GCMAX
>>>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
>>>
>>> Suggested-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_pci.c    |   2 +-
>>>   drivers/gpu/drm/i915/intel_color.c | 127 
>>> ++++++++++++++++++++++++++++-
>>>   2 files changed, 124 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_pci.c 
>>> b/drivers/gpu/drm/i915/i915_pci.c
>>> index ffa2ee70a03d..2f99b585d44b 100644
>>> --- a/drivers/gpu/drm/i915/i915_pci.c
>>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>>> @@ -749,7 +749,7 @@ static const struct intel_device_info 
>>> intel_cannonlake_info = {
>>>       GEN(11), \
>>>       .ddb_size = 2048, \
>>>       .has_logical_ring_elsq = 1, \
>>> -    .color = { .degamma_lut_size = 33, .gamma_lut_size = 1024 }
>>> +    .color = { .degamma_lut_size = 33, .gamma_lut_size = 262145 }
>>>     static const struct intel_device_info intel_icelake_11_info = {
>>>       GEN11_FEATURES,
>>> diff --git a/drivers/gpu/drm/i915/intel_color.c 
>>> b/drivers/gpu/drm/i915/intel_color.c
>>> index 6c341bea514c..c1a9506fd069 100644
>>> --- a/drivers/gpu/drm/i915/intel_color.c
>>> +++ b/drivers/gpu/drm/i915/intel_color.c
>>> @@ -41,6 +41,8 @@
>>>   #define CTM_COEFF_ABS(coeff)        ((coeff) & (CTM_COEFF_SIGN - 1))
>>>     #define LEGACY_LUT_LENGTH        256
>>> +#define ICL_GAMMA_MULTISEG_LUT_LENGTH        (256 * 128 * 8)
>> Unused.
> Got it
>>> +
>>>   /*
>>>    * Extract the CSC coefficient from a CTM coefficient (in U32.32 
>>> fixed point
>>>    * format). This macro takes the coefficient we want transformed 
>>> and the
>>> @@ -767,6 +769,116 @@ static void glk_load_luts(const struct 
>>> intel_crtc_state *crtc_state)
>>>       }
>>>   }
>>>   +/* ilk+ "12.4" interpolated format (high 10 bits) */
>>> +static u32 ilk_lut_12p4_ldw(const struct drm_color_lut *color)
>>> +{
>>> +    return (color->red >> 6) << 20 | (color->green >> 6) << 10 |
>>> +        (color->blue >> 6);
>>> +}
>>> +
>>> +/* ilk+ "12.4" interpolated format (low 6 bits) */
>>> +static u32 ilk_lut_12p4_udw(const struct drm_color_lut *color)
>>> +{
>>> +    return (color->red & 0x3f) << 24 | (color->green & 0x3f) << 14 |
>>> +        (color->blue & 0x3f);
>> Blue is missing the shift.
> Ok,
>> I'm not 100% sure if the ldw vs. udw are the right way around. The spec
>> has at times been inconsistent with the odd vs. even descriptions,
>> sometimes even contradicting itself. Also it never really defines
>> whether it starts counting dwords from from 0 or 1, so not sure what
>> odd and even actually mean. Can I presume someone has checked this
>> on actual hardware?
> Well, the property was getting set properly, and the display looked 
> ok, but dint dump the values in registers. Can check it now.
>>> +}
>>> +
>>> +static void
>>> +icl_load_gcmax(const struct intel_crtc_state *crtc_state,
>>> +           const struct drm_color_lut *color)
>>> +{
>>> +    struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>>> +    struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>> +    enum pipe pipe = crtc->pipe;
>>> +
>>> +    /* Fixme: LUT entries are 16 bit only, so we can prog 0xFFFF 
>>> max */
>>> +    I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), color->red);
>>> +    I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), color->green);
>>> +    I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), color->blue);
>>> +}
>>> +
>>> +static void
>>> +icl_program_gamma_superfine_segment(const struct intel_crtc_state 
>>> *crtc_state)
>>> +{
>>> +    struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>>> +    struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>> +    const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
>>> +    const struct drm_color_lut *lut = blob->data;
>>> +    enum pipe pipe = crtc->pipe;
>>> +    u32 i;
>>> +
>>> +    /*
>>> +     * Every entry in the multi-segment LUT is corresponding to a 
>>> superfine
>>> +     * segment step which is 1/(8 * 128 * 256).
>>> +     *
>>> +     * Superfine segment has 9 entries, corresponding to values
>>> +     * 0, 1/(8 * 128 * 256), 2/(8 * 128 * 256) .... 8/(8 * 128 * 256).
>>> +     */
>>> +    I915_WRITE(PREC_PAL_MULTI_SEG_INDEX(pipe), 
>>> PAL_PREC_AUTO_INCREMENT);
>>> +
>>> +    for (i = 0; i < 9; i++) {
>>> +        const struct drm_color_lut *entry = &lut[i];
>>> +
>>> +        I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe),
>>> +               ilk_lut_12p4_ldw(entry));
>>> +        I915_WRITE(PREC_PAL_MULTI_SEG_DATA(pipe),
>>> +               ilk_lut_12p4_udw(entry));
>>> +    }
>>> +}
>>> +
>>> +static void
>>> +icl_program_gamma_multi_segment(const struct intel_crtc_state 
>>> *crtc_state)
>>> +{
>>> +    struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>>> +    struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>> +    const struct drm_property_blob *blob = crtc_state->base.gamma_lut;
>>> +    const struct drm_color_lut *lut = blob->data;
>>> +    const struct drm_color_lut *entry;
>>> +    enum pipe pipe = crtc->pipe;
>>> +    u32 i;
>>> +
>>> +    /*
>>> +     *
>>> +     * Program Fine segment (let's call it seg2)...
>>> +     *
>>> +     * Fine segment's step is 1/(128 * 256) ie 1/(128 * 256),  
>>> 2/(128*256)
>>> +     * ... 256/(128*256). So in order to program fine segment of 
>>> LUT we
>>> +     * need to pick every 8'th entry in LUT, and program 256 indexes.
>>> +     *
>>> +     * PAL_PREC_INDEX[0] and PAL_PREC_INDEX[1] map to seg2[1],
>>> +     * with seg2[0] being unused by the hardware.
>>> +     */
>>> +    I915_WRITE(PREC_PAL_INDEX(pipe), PAL_PREC_AUTO_INCREMENT);
>>> +    for (i = 1; i < 257; i++) {
>>> +        entry = &lut[i * 8];
>>> +        I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_ldw(entry));
>>> +        I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_udw(entry));
>>> +    }
>>> +
>>> +    /*
>>> +     * Program Coarse segment (let's call it seg3)...
>>> +     *
>>> +     * Coarse segment's starts from index 0 and it's step is 1/256 
>>> ie 0,
>>> +     * 1/256, 2/256 ...256/256. As per the description of each 
>>> entry in LUT
>>> +     * above, we need to pick every (8 * 128)th entry in LUT, and
>>> +     * program 256 of those.
>>> +     *
>>> +     * Spec is not very clear about if entries seg3[0] and seg3[1] are
>>> +     * being used or not, but we still need to program these to 
>>> advance
>>> +     * the index.
>>> +     */
>>> +    for (i = 0; i < 256; i++) {
>>> +        entry = &lut[i * 8 * 128];
>>> +        I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_ldw(entry));
>>> +        I915_WRITE(PREC_PAL_DATA(pipe), ilk_lut_12p4_udw(entry));
>>> +    }
>>> +
>>> +    /* The last entry in the LUT is to be programmed in GCMAX */
>>> +    entry = &lut[256 * 8 * 128 + 1];
>> The +1 shouldn't be here.
> ok
>> OK, mostly looks good. With the minor issues addressed this is
>> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Thanks for the review, I will publish V4 with comments addressed.
>
> - Shashank
>
>>> +    icl_load_gcmax(crtc_state, entry);
>>> +    ivb_load_lut_ext_max(crtc);
>>> +}
>>> +
>>>   static void icl_load_luts(const struct intel_crtc_state *crtc_state)
>>>   {
>>>       const struct drm_property_blob *gamma_lut = 
>>> crtc_state->base.gamma_lut;
>>> @@ -775,10 +887,17 @@ static void icl_load_luts(const struct 
>>> intel_crtc_state *crtc_state)
>>>       if (crtc_state->base.degamma_lut)
>>>           glk_load_degamma_lut(crtc_state);
>>>   -    if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
>>> -        GAMMA_MODE_MODE_8BIT) {
>>> +    switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
>>> +    case GAMMA_MODE_MODE_8BIT:
>>>           i9xx_load_luts(crtc_state);
>>> -    } else {
>>> +        break;
>>> +
>>> +    case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
>>> +        icl_program_gamma_superfine_segment(crtc_state);
>>> +        icl_program_gamma_multi_segment(crtc_state);
>>> +        break;
>>> +
>>> +    default:
>>>           bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
>>>           ivb_load_lut_ext_max(crtc);
>>>       }
>>> @@ -1209,7 +1328,7 @@ static u32 icl_gamma_mode(const struct 
>>> intel_crtc_state *crtc_state)
>>>           crtc_state_is_legacy_gamma(crtc_state))
>>>           gamma_mode |= GAMMA_MODE_MODE_8BIT;
>>>       else
>>> -        gamma_mode |= GAMMA_MODE_MODE_10BIT;
>>> +        gamma_mode |= GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED;
>>>         return gamma_mode;
>>>   }
>>> -- 
>>> 2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx