[v5 0/6] Add support for Gen 11 pipe color features

[v5 0/6] Add support for Gen 11 pipe color features

[Uma Shankar]

This patch series adds support for Gen11 pipe degamma, CSC
and gamma hardware blocks.

CRC checks are not working for 10bit gamma but works for 8bit
pallete modes which seems to be due to some rounding errors in pipe.

ToDo: Support for Multi Segmented Gamma will be added later.

v2: Addressed Maarten's review comments and re-ordered the patch
series.

v3: Addressed Matt's review comments. Removed rmw patterns
as suggested by Matt.

v4: Addressed Matt's review comments.

v5: Addressed Matt's, Ville and Jani Nikula's review comments.

Uma Shankar (6):
  drm/i915: Check for Null for color lut callbacks
  drm/i915: Sanitize crtc gamma mode
  drm/i915: Remove gamma_mode state variable
  drm/i915/icl: Add icl pipe degamma and gamma support
  drm/i915/icl: Enable ICL Pipe CSC block
  drm/i915/icl: Add degamma and gamma lut size to gen11 caps

 drivers/gpu/drm/i915/i915_pci.c      |  3 +-
 drivers/gpu/drm/i915/i915_reg.h      | 22 +++++---
 drivers/gpu/drm/i915/intel_color.c   | 97 ++++++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_display.c | 20 ++++++--
 drivers/gpu/drm/i915/intel_drv.h     |  3 --
 5 files changed, 110 insertions(+), 35 deletions(-)

-- 
1.9.1

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

[v5 1/6] drm/i915: Check for Null for color lut callbacks

[Uma Shankar]

Check if de-gamma/gamma lut callback is assigned before
calling the same.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 37fd9dd..4ff4db6 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -602,7 +602,8 @@ void intel_color_load_luts(struct intel_crtc_state *crtc_state)
 	struct drm_device *dev = crtc_state->base.crtc->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 
-	dev_priv->display.load_luts(crtc_state);
+	if (dev_priv->display.load_luts)
+		dev_priv->display.load_luts(crtc_state);
 }
 
 int intel_color_check(struct intel_crtc_state *crtc_state)
-- 
1.9.1

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

[v5 2/6] drm/i915: Sanitize crtc gamma mode

[Uma Shankar]

Sanitize crtc gamma mode and update the mode in driver in case
BIOS has setup a different gamma mode as to what is expected by
driver. There is restriction on HSW platform not to read/write
color LUT's if ips is enabled. Handled the same accordingly.

Credits-to: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 696e6f5..03c8f68 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15401,6 +15401,23 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
 		}
 	}
 
+	/*
+	 * Sanitize gamma mode incase BIOS leaves it in SPLIT GAMMA MODE
+	 * Workaround HSW : Do not read or write the pipe palette/gamma data
+	 * while GAMMA_MODE is configured for split gamma and IPS_CTL has IPS
+	 * enabled.
+	 */
+	if (IS_HASWELL(dev_priv)) {
+		if (crtc_state->ips_enabled)
+			hsw_disable_ips(crtc_state);
+
+		intel_color_set_csc(crtc_state);
+		intel_color_load_luts(crtc_state);
+
+		if (crtc_state->ips_enabled)
+			hsw_enable_ips(crtc_state);
+	}
+
 	/* Adjust the state of the output pipe according to whether we
 	 * have active connectors/encoders. */
 	if (crtc_state->base.active && !intel_crtc_has_encoders(crtc))
-- 
1.9.1

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

[v5 3/6] drm/i915: Remove gamma_mode state variable

[Uma Shankar]

Removed crtc state variable for gamma mode as it's
redundant since currently we have fixed modes on respective
hardware platforms. This was making this state variable
irrelevant.

v2: Updated logic to check for split gamma mode. This is moved
to a separate patch and handled as part of intel_sanitize_crtc.

Credits-to: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Uma Shankar <uma.shankar@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c   | 17 -----------------
 drivers/gpu/drm/i915/intel_display.c |  3 ---
 drivers/gpu/drm/i915/intel_drv.h     |  3 ---
 3 files changed, 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 4ff4db6..9a72e64 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -363,25 +363,10 @@ static void haswell_load_luts(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);
-	bool reenable_ips = false;
 
-	/*
-	 * Workaround : Do not read or write the pipe palette/gamma data while
-	 * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled.
-	 */
-	if (IS_HASWELL(dev_priv) && crtc_state->ips_enabled &&
-	    (crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT)) {
-		hsw_disable_ips(crtc_state);
-		reenable_ips = true;
-	}
-
-	crtc_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
 	I915_WRITE(GAMMA_MODE(crtc->pipe), GAMMA_MODE_MODE_8BIT);
 
 	i9xx_load_luts(crtc_state);
-
-	if (reenable_ips)
-		hsw_enable_ips(crtc_state);
 }
 
 static void bdw_load_degamma_lut(struct intel_crtc_state *crtc_state)
@@ -476,7 +461,6 @@ static void broadwell_load_luts(struct intel_crtc_state *crtc_state)
 	bdw_load_gamma_lut(crtc_state,
 			   INTEL_INFO(dev_priv)->color.degamma_lut_size);
 
-	crtc_state->gamma_mode = GAMMA_MODE_MODE_SPLIT;
 	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT);
 	POSTING_READ(GAMMA_MODE(pipe));
 
@@ -532,7 +516,6 @@ static void glk_load_luts(struct intel_crtc_state *crtc_state)
 
 	bdw_load_gamma_lut(crtc_state, 0);
 
-	crtc_state->gamma_mode = GAMMA_MODE_MODE_10BIT;
 	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT);
 	POSTING_READ(GAMMA_MODE(pipe));
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 03c8f68..8bd47e2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9699,9 +9699,6 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	intel_get_pipe_src_size(crtc, pipe_config);
 	intel_get_crtc_ycbcr_config(crtc, pipe_config);
 
-	pipe_config->gamma_mode =
-		I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_MASK;
-
 	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
 	if (intel_display_power_get_if_enabled(dev_priv, power_domain)) {
 		power_domain_mask |= BIT_ULL(power_domain);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1a11c2be..048090d 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -924,9 +924,6 @@ struct intel_crtc_state {
 
 	struct intel_crtc_wm_state wm;
 
-	/* Gamma mode programmed on the pipe */
-	uint32_t gamma_mode;
-
 	/* bitmask of visible planes (enum plane_id) */
 	u8 active_planes;
 	u8 nv12_planes;
-- 
1.9.1

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

[v5 4/6] drm/i915/icl: Add icl pipe degamma and gamma support

[Uma Shankar]

Add support for icl pipe degamma and gamma.

v2: Removed a POSTING_READ and corrected the Bit
Definition as per Maarten's comments.

v3: Addressed Matt's review comments. Removed rmw patterns
as suggested by Matt.

v4: Fixed Matt's review comments.

v5: Corrected macro alignment as per Jani Nikula's comments.
Addressed Ville and Matt's  review comments.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    | 12 ++++---
 drivers/gpu/drm/i915/intel_color.c | 65 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 72 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 44958d9..f29eef7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7088,11 +7088,13 @@ enum {
 #define _GAMMA_MODE_A		0x4a480
 #define _GAMMA_MODE_B		0x4ac80
 #define GAMMA_MODE(pipe) _MMIO_PIPE(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B)
-#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)
-#define GAMMA_MODE_MODE_SPLIT	(3 << 0)
+#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)
+#define  GAMMA_MODE_MODE_SPLIT	(3 << 0)
 
 /* DMC/CSR */
 #define CSR_PROGRAM(i)		_MMIO(0x80000 + (i) * 4)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 9a72e64..9cd4646 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -520,6 +520,69 @@ static void glk_load_luts(struct intel_crtc_state *crtc_state)
 	POSTING_READ(GAMMA_MODE(pipe));
 }
 
+static void icl_load_degamma_lut(struct intel_crtc_state *crtc_state)
+{
+	struct drm_device *dev = crtc_state->base.crtc->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
+	const uint32_t lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
+	uint32_t i;
+
+	/*
+	 * When setting the auto-increment bit, the hardware seems to
+	 * ignore the index bits, so we need to reset it to index 0
+	 * separately.
+	 */
+	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0);
+	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), PRE_CSC_GAMC_AUTO_INCREMENT);
+
+	if (crtc_state->base.degamma_lut) {
+		struct drm_color_lut *lut = crtc_state->base.degamma_lut->data;
+
+		for (i = 0; i < lut_size; i++) {
+			/*
+			 * First 33 entries represent range from 0 to 1.0
+			 * 34th and 35th entry will represent extended range
+			 * inputs 3.0 and 7.0 respectively, currently clamped
+			 * at 1.0. Since the precision is 16bit, the user value
+			 * can be directly filled to register.
+			 * ToDo: Extend to max 7.0.
+			 */
+			I915_WRITE(PRE_CSC_GAMC_DATA(pipe), lut[i].green);
+		}
+	} else {
+		/* load a linear table. */
+		for (i = 0; i < lut_size; i++) {
+			uint32_t v = (i * (1 << 16)) / (lut_size - 1);
+
+			I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v);
+		}
+	}
+
+	/* Clamp values > 1.0. */
+	while (i++ < 35)
+		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16));
+}
+
+static void icl_load_luts(struct intel_crtc_state *crtc_state)
+{
+	struct drm_crtc *crtc = crtc_state->base.crtc;
+	struct drm_device *dev = crtc_state->base.crtc->dev;
+	struct drm_i915_private *dev_priv = to_i915(dev);
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+
+	if (crtc_state_is_legacy_gamma(crtc_state)) {
+		haswell_load_luts(crtc_state);
+		return;
+	}
+
+	icl_load_degamma_lut(crtc_state);
+	bdw_load_gamma_lut(crtc_state, 0);
+
+	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT |
+		   PRE_CSC_GAMMA_ENABLE | POST_CSC_GAMMA_ENABLE);
+}
+
 /* Loads the palette/gamma unit for the CRTC on CherryView. */
 static void cherryview_load_luts(struct intel_crtc_state *crtc_state)
 {
@@ -636,6 +699,8 @@ void intel_color_init(struct intel_crtc *crtc)
 	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
 		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
 		dev_priv->display.load_luts = glk_load_luts;
+	} else if (IS_ICELAKE(dev_priv)) {
+		dev_priv->display.load_luts = icl_load_luts;
 	} else {
 		dev_priv->display.load_luts = i9xx_load_luts;
 	}
-- 
1.9.1

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

[v5 5/6] drm/i915/icl: Enable ICL Pipe CSC block

[Uma Shankar]

Enable ICL pipe csc hardware. CSC block is enabled
in CSC_MODE register instead of PLANE_COLOR_CTL.

ToDO: Extend the ABI to accept 32 bit coefficient values
instead of 16bit for future platforms.

v2: Addressed Maarten's review comments.

v3: Addressed Matt's review comments. Removed rmw patterns
as suggested by Matt.

v4: Addressed Matt's review comments.

v5: Addressed Ville's review comments.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    | 10 +++++++---
 drivers/gpu/drm/i915/intel_color.c | 12 ++++++++++--
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f29eef7..5a262c0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9861,10 +9861,14 @@ enum skl_power_gate {
 #define _PIPE_A_CSC_COEFF_BU	0x4901c
 #define _PIPE_A_CSC_COEFF_RV_GV	0x49020
 #define _PIPE_A_CSC_COEFF_BV	0x49024
+
 #define _PIPE_A_CSC_MODE	0x49028
-#define   CSC_BLACK_SCREEN_OFFSET	(1 << 2)
-#define   CSC_POSITION_BEFORE_GAMMA	(1 << 1)
-#define   CSC_MODE_YUV_TO_RGB		(1 << 0)
+#define  ICL_CSC_ENABLE			(1 << 31)
+#define  ICL_OUTPUT_CSC_ENABLE		(1 << 30)
+#define  CSC_BLACK_SCREEN_OFFSET	(1 << 2)
+#define  CSC_POSITION_BEFORE_GAMMA	(1 << 1)
+#define  CSC_MODE_YUV_TO_RGB		(1 << 0)
+
 #define _PIPE_A_CSC_PREOFF_HI	0x49030
 #define _PIPE_A_CSC_PREOFF_ME	0x49034
 #define _PIPE_A_CSC_PREOFF_LO	0x49038
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 9cd4646..c3e4ff6 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -134,7 +134,11 @@ static void ilk_load_ycbcr_conversion_matrix(struct intel_crtc *crtc)
 	I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI);
 	I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), POSTOFF_RGB_TO_YUV_ME);
 	I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO);
-	I915_WRITE(PIPE_CSC_MODE(pipe), 0);
+
+	if (INTEL_GEN(dev_priv) >= 11)
+		I915_WRITE(PIPE_CSC_MODE(pipe), ICL_OUTPUT_CSC_ENABLE);
+	else
+		I915_WRITE(PIPE_CSC_MODE(pipe), 0);
 }
 
 static void ilk_load_csc_matrix(struct intel_crtc_state *crtc_state)
@@ -242,7 +246,10 @@ static void ilk_load_csc_matrix(struct intel_crtc_state *crtc_state)
 		I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff);
 		I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff);
 
-		I915_WRITE(PIPE_CSC_MODE(pipe), 0);
+		if (INTEL_GEN(dev_priv) >= 11)
+			I915_WRITE(PIPE_CSC_MODE(pipe), ICL_CSC_ENABLE);
+		else
+			I915_WRITE(PIPE_CSC_MODE(pipe), 0);
 	} else {
 		uint32_t mode = CSC_MODE_YUV_TO_RGB;
 
@@ -700,6 +707,7 @@ void intel_color_init(struct intel_crtc *crtc)
 		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
 		dev_priv->display.load_luts = glk_load_luts;
 	} else if (IS_ICELAKE(dev_priv)) {
+		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
 		dev_priv->display.load_luts = icl_load_luts;
 	} else {
 		dev_priv->display.load_luts = i9xx_load_luts;
-- 
1.9.1

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

[v5 6/6] drm/i915/icl: Add degamma and gamma lut size to gen11 caps

[Uma Shankar]

Add the degamma and gamma lut sizes to gen11 capability
structure.

Note: Currently this doesn't account for the extended range gamma
entries and this will be addressed with new segmented gamma ABI
in a future patch.

v2: Reorder the patch as per Maarten's suggestion.

v3: Rebase

v4: Updated commit message with a note as per Matt's suggestion.

v5: No Change.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_pci.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index dd4aff2..14e5bb4 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -639,7 +639,8 @@
 	}, \
 	GEN(11), \
 	.ddb_size = 2048, \
-	.has_logical_ring_elsq = 1
+	.has_logical_ring_elsq = 1, \
+	.color = { .degamma_lut_size = 33, .gamma_lut_size = 1024 }
 
 static const struct intel_device_info intel_icelake_11_info = {
 	GEN11_FEATURES,
-- 
1.9.1

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

✓ Fi.CI.BAT: success for Add support for Gen 11 pipe color features (rev5)

[Patchwork]

== Series Details ==

Series: Add support for Gen 11 pipe color features (rev5)
URL   : https://patchwork.freedesktop.org/series/51408/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5372 -> Patchwork_11205
====================================================

Summary
-------

  **WARNING**

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/51408/revisions/5/mbox/

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

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

### IGT changes ###

#### Warnings ####

  * igt@kms_busy@basic-flip-a:
    - fi-kbl-7567u:       SKIP -> PASS +2

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_hangcheck:
    - fi-bwr-2160:        PASS -> DMESG-FAIL [fdo#108735]

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

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       PASS -> FAIL [fdo#108767]

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       PASS -> DMESG-WARN [fdo#102614]
    - fi-byt-clapper:     PASS -> FAIL [fdo#103167]

  
#### Possible fixes ####

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

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

  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#108735]: https://bugs.freedesktop.org/show_bug.cgi?id=108735
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#108915]: https://bugs.freedesktop.org/show_bug.cgi?id=108915
  [fdo#109241]: https://bugs.freedesktop.org/show_bug.cgi?id=109241


Participating hosts (48 -> 45)
------------------------------

  Additional (1): fi-icl-y 
  Missing    (4): fi-kbl-7560u fi-byt-squawks fi-bsw-cyan fi-ilk-m540 


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

    * Linux: CI_DRM_5372 -> Patchwork_11205

  CI_DRM_5372: 5d1d46df77190f1702e3417e9bf9091c9242fd4a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4756: 75081c6bfb9998bd7cbf35a7ac0578c683fe55a8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11205: e87cbe12496ca89a000c362e2a576c85836f23a2 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

e87cbe12496c drm/i915/icl: Add degamma and gamma lut size to gen11 caps
3dbc64967efa drm/i915/icl: Enable ICL Pipe CSC block
749868f3f974 drm/i915/icl: Add icl pipe degamma and gamma support
f9a2b86dd563 drm/i915: Remove gamma_mode state variable
167453134516 drm/i915: Sanitize crtc gamma mode
6036b681a025 drm/i915: Check for Null for color lut callbacks

== Logs ==

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

✗ Fi.CI.IGT: failure for Add support for Gen 11 pipe color features (rev5)

[Patchwork]

== Series Details ==

Series: Add support for Gen 11 pipe color features (rev5)
URL   : https://patchwork.freedesktop.org/series/51408/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5372_full -> Patchwork_11205_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_11205_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_11205_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_11205_full:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_color@pipe-b-gamma:
    - shard-iclb:         SKIP -> FAIL +11

  
#### Warnings ####

  * igt@kms_color@pipe-b-ctm-green-to-red:
    - shard-iclb:         SKIP -> PASS +18

  * igt@pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          SKIP -> PASS

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_userptr_blits@readonly-unsync:
    - shard-skl:          NOTRUN -> TIMEOUT [fdo#108887]
    - shard-kbl:          PASS -> TIMEOUT [fdo#108887]

  * igt@kms_atomic_transition@plane-all-transition-fencing:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] / [fdo#109225]

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-kbl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_chv_cursor_fail@pipe-c-64x64-bottom-edge:
    - shard-kbl:          PASS -> FAIL [fdo#104671]

  * igt@kms_color@pipe-b-ctm-0-75:
    - shard-iclb:         SKIP -> DMESG-WARN [fdo#107724] / [fdo#108336] +2

  * igt@kms_color@pipe-b-ctm-max:
    - shard-apl:          PASS -> FAIL [fdo#108147]

  * igt@kms_color@pipe-c-legacy-gamma:
    - shard-apl:          PASS -> FAIL [fdo#104782]

  * igt@kms_cursor_crc@cursor-64x21-onscreen:
    - shard-glk:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x21-sliding:
    - shard-apl:          PASS -> FAIL [fdo#103232] +4

  * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-wc-untiled:
    - shard-iclb:         PASS -> WARN [fdo#108336] +1

  * igt@kms_fbcon_fbt@psr:
    - shard-iclb:         NOTRUN -> FAIL [fdo#107882]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
    - shard-apl:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite:
    - shard-glk:          PASS -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-stridechange:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#107724] +8

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

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-cpu:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] / [fdo#108336] +13

  * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
    - shard-apl:          PASS -> FAIL [fdo#108948]

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#106885]

  * igt@kms_plane@plane-position-covered-pipe-b-planes:
    - shard-glk:          PASS -> FAIL [fdo#103166] +2

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

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

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-iclb:         PASS -> FAIL [fdo#103166]

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

  * igt@pm_backlight@basic-brightness:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] +28

  * igt@pm_backlight@fade_with_suspend:
    - shard-skl:          NOTRUN -> FAIL [fdo#107847]

  * igt@pm_rpm@dpms-mode-unset-lpsp:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107807]

  * igt@pm_rpm@pm-caching:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#107807]

  * igt@pm_rps@min-max-config-loaded:
    - shard-apl:          PASS -> FAIL [fdo#102250]
    - shard-skl:          PASS -> FAIL [fdo#102250]

  
#### Possible fixes ####

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

  * igt@kms_chv_cursor_fail@pipe-b-64x64-left-edge:
    - shard-skl:          FAIL [fdo#104671] -> PASS

  * igt@kms_cursor_crc@cursor-128x42-onscreen:
    - shard-apl:          FAIL [fdo#103232] -> PASS +2

  * igt@kms_cursor_crc@cursor-256x256-onscreen:
    - shard-glk:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-random:
    - shard-iclb:         FAIL [fdo#103232] -> PASS +18

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

  * igt@kms_cursor_legacy@pipe-c-forked-bo:
    - shard-apl:          INCOMPLETE [fdo#103927] -> PASS

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

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-onoff:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +4

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-glk:          FAIL [fdo#108948] -> PASS

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

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

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-glk:          FAIL [fdo#103166] -> PASS +1
    - shard-apl:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-x:
    - shard-iclb:         FAIL [fdo#103166] -> PASS

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-glk:          DMESG-FAIL [fdo#105763] / [fdo#106538] -> PASS

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         FAIL [fdo#100047] -> PASS

  * igt@kms_vblank@pipe-a-ts-continuation-dpms-suspend:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  
#### Warnings ####

  * igt@kms_cursor_crc@cursor-64x64-onscreen:
    - shard-iclb:         FAIL [fdo#103232] -> DMESG-WARN [fdo#107724] / [fdo#108336]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          DMESG-FAIL [fdo#108950] -> DMESG-WARN [fdo#105604]

  * igt@pm_rpm@legacy-planes:
    - shard-iclb:         DMESG-WARN [fdo#108654] -> INCOMPLETE [fdo#108840]

  
  {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#102250]: https://bugs.freedesktop.org/show_bug.cgi?id=102250
  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104671]: https://bugs.freedesktop.org/show_bug.cgi?id=104671
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105604]: https://bugs.freedesktop.org/show_bug.cgi?id=105604
  [fdo#105683]: https://bugs.freedesktop.org/show_bug.cgi?id=105683
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106538]: https://bugs.freedesktop.org/show_bug.cgi?id=106538
  [fdo#106885]: https://bugs.freedesktop.org/show_bug.cgi?id=106885
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107847]: https://bugs.freedesktop.org/show_bug.cgi?id=107847
  [fdo#107882]: https://bugs.freedesktop.org/show_bug.cgi?id=107882
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#108336]: https://bugs.freedesktop.org/show_bug.cgi?id=108336
  [fdo#108654]: https://bugs.freedesktop.org/show_bug.cgi?id=108654
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#108887]: https://bugs.freedesktop.org/show_bug.cgi?id=108887
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#108950]: https://bugs.freedesktop.org/show_bug.cgi?id=108950
  [fdo#109225]: https://bugs.freedesktop.org/show_bug.cgi?id=109225
  [fdo#109241]: https://bugs.freedesktop.org/show_bug.cgi?id=109241
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 7)
------------------------------

  No changes in participating hosts


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

    * Linux: CI_DRM_5372 -> Patchwork_11205

  CI_DRM_5372: 5d1d46df77190f1702e3417e9bf9091c9242fd4a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4756: 75081c6bfb9998bd7cbf35a7ac0578c683fe55a8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11205: e87cbe12496ca89a000c362e2a576c85836f23a2 @ 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_11205/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v5 1/6] drm/i915: Check for Null for color lut callbacks

[Matt Roper]

On Tue, Jan 08, 2019 at 01:07:28PM +0530, Uma Shankar wrote:
> Check if de-gamma/gamma lut callback is assigned before
> calling the same.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>

Is it possible for this test to fail?  intel_color_init() seems to
always assign a value (even for platforms that don't actually support
color management).

It seem like if you're going to make this change, you'd also want to
update intel_color_init() to only set the load_luts for platforms where
we actually have color management?


Matt

> ---
>  drivers/gpu/drm/i915/intel_color.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 37fd9dd..4ff4db6 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -602,7 +602,8 @@ void intel_color_load_luts(struct intel_crtc_state *crtc_state)
>  	struct drm_device *dev = crtc_state->base.crtc->dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  
> -	dev_priv->display.load_luts(crtc_state);
> +	if (dev_priv->display.load_luts)
> +		dev_priv->display.load_luts(crtc_state);
>  }
>  
>  int intel_color_check(struct intel_crtc_state *crtc_state)
> -- 
> 1.9.1
> 

-- 
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

Re: [v5 2/6] drm/i915: Sanitize crtc gamma mode

[Matt Roper]

On Tue, Jan 08, 2019 at 01:07:29PM +0530, Uma Shankar wrote:
> Sanitize crtc gamma mode and update the mode in driver in case
> BIOS has setup a different gamma mode as to what is expected by
> driver. There is restriction on HSW platform not to read/write
> color LUT's if ips is enabled. Handled the same accordingly.
> 
> Credits-to: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 696e6f5..03c8f68 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15401,6 +15401,23 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
>  		}
>  	}
>  
> +	/*
> +	 * Sanitize gamma mode incase BIOS leaves it in SPLIT GAMMA MODE
> +	 * Workaround HSW : Do not read or write the pipe palette/gamma data
> +	 * while GAMMA_MODE is configured for split gamma and IPS_CTL has IPS
> +	 * enabled.
> +	 */

The other thing that might be worth noting here is that we don't
actually try to read out the LUT's and CTM that the BIOS setup, so at
the moment they stick around for a while until the get unexpectedly
clobbered by a subsequent modeset or fastset.   The change here will
basically force them to be reset to standard/linear values at startup.

Maybe in the future we'll try to actually read out and preserve the
contents of the actual LUT's and CTM that the BIOS had setup, but we
don't do that yet today, so the change here at least makes the behavior
a little bit more consistent than what it has been.

Up to you whether you want to try to describe that in either the comment
and/or commit message.

> +	if (IS_HASWELL(dev_priv)) {
> +		if (crtc_state->ips_enabled)

It looks like both hsw_disable_ips() and hsw_enable_ips() have this test
inside of them already, so we can just call them unconditionally here.

Aside from that,

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

> +			hsw_disable_ips(crtc_state);
> +
> +		intel_color_set_csc(crtc_state);
> +		intel_color_load_luts(crtc_state);
> +
> +		if (crtc_state->ips_enabled)
> +			hsw_enable_ips(crtc_state);
> +	}
> +
>  	/* Adjust the state of the output pipe according to whether we
>  	 * have active connectors/encoders. */
>  	if (crtc_state->base.active && !intel_crtc_has_encoders(crtc))
> -- 
> 1.9.1
> 

-- 
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

Re: [v5 2/6] drm/i915: Sanitize crtc gamma mode

[Shankar, Uma]

>-----Original Message-----
>From: Roper, Matthew D
>Sent: Friday, January 11, 2019 4:08 AM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Lankhorst, Maarten
><maarten.lankhorst@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Sharma,
>Shashank <shashank.sharma@intel.com>
>Subject: Re: [v5 2/6] drm/i915: Sanitize crtc gamma mode
>
>On Tue, Jan 08, 2019 at 01:07:29PM +0530, Uma Shankar wrote:
>> Sanitize crtc gamma mode and update the mode in driver in case BIOS
>> has setup a different gamma mode as to what is expected by driver.
>> There is restriction on HSW platform not to read/write color LUT's if
>> ips is enabled. Handled the same accordingly.
>>
>> Credits-to: Matt Roper <matthew.d.roper@intel.com>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++++++++
>>  1 file changed, 17 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index 696e6f5..03c8f68 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -15401,6 +15401,23 @@ static void intel_sanitize_crtc(struct intel_crtc
>*crtc,
>>  		}
>>  	}
>>
>> +	/*
>> +	 * Sanitize gamma mode incase BIOS leaves it in SPLIT GAMMA MODE
>> +	 * Workaround HSW : Do not read or write the pipe palette/gamma data
>> +	 * while GAMMA_MODE is configured for split gamma and IPS_CTL has
>IPS
>> +	 * enabled.
>> +	 */
>
>The other thing that might be worth noting here is that we don't actually try to
>read out the LUT's and CTM that the BIOS setup, so at the moment they stick
>around for a while until the get unexpectedly
>clobbered by a subsequent modeset or fastset.   The change here will
>basically force them to be reset to standard/linear values at startup.
>
>Maybe in the future we'll try to actually read out and preserve the contents of the
>actual LUT's and CTM that the BIOS had setup, but we don't do that yet today, so
>the change here at least makes the behavior a little bit more consistent than what
>it has been.
>
>Up to you whether you want to try to describe that in either the comment and/or
>commit message.

Sure Matt, I will update the commit message to reflect this as well.

>> +	if (IS_HASWELL(dev_priv)) {
>> +		if (crtc_state->ips_enabled)
>
>It looks like both hsw_disable_ips() and hsw_enable_ips() have this test inside of
>them already, so we can just call them unconditionally here.
>

Yes, this can be dropped. Will do that.

>Aside from that,
>
>Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>

Thanks Matt for the review and valuable comments.

Regards,
Uma Shankar
>> +			hsw_disable_ips(crtc_state);
>> +
>> +		intel_color_set_csc(crtc_state);
>> +		intel_color_load_luts(crtc_state);
>> +
>> +		if (crtc_state->ips_enabled)
>> +			hsw_enable_ips(crtc_state);
>> +	}
>> +
>>  	/* Adjust the state of the output pipe according to whether we
>>  	 * have active connectors/encoders. */
>>  	if (crtc_state->base.active && !intel_crtc_has_encoders(crtc))
>> --
>> 1.9.1
>>
>
>--
>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

Re: [v5 1/6] drm/i915: Check for Null for color lut callbacks

[Shankar, Uma]

>-----Original Message-----
>From: Roper, Matthew D
>Sent: Friday, January 11, 2019 3:57 AM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Lankhorst, Maarten
><maarten.lankhorst@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Sharma,
>Shashank <shashank.sharma@intel.com>
>Subject: Re: [v5 1/6] drm/i915: Check for Null for color lut callbacks
>
>On Tue, Jan 08, 2019 at 01:07:28PM +0530, Uma Shankar wrote:
>> Check if de-gamma/gamma lut callback is assigned before calling the
>> same.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>
>Is it possible for this test to fail?  intel_color_init() seems to always assign a value
>(even for platforms that don't actually support color management).
>
>It seem like if you're going to make this change, you'd also want to update
>intel_color_init() to only set the load_luts for platforms where we actually have
>color management?

Yeah, I was trying to add this check to avoid any processing if call-backs are not
registered. Currently that is not the case so can be dropped and added later if
such a case arise.

Touching and disabling it for legacy platforms, requires to get details from GEN2 onwards
and can break older platforms if not done right. Will drop this change.

Regards,
Uma Shankar

>
>Matt
>
>> ---
>>  drivers/gpu/drm/i915/intel_color.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index 37fd9dd..4ff4db6 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -602,7 +602,8 @@ void intel_color_load_luts(struct intel_crtc_state
>*crtc_state)
>>  	struct drm_device *dev = crtc_state->base.crtc->dev;
>>  	struct drm_i915_private *dev_priv = to_i915(dev);
>>
>> -	dev_priv->display.load_luts(crtc_state);
>> +	if (dev_priv->display.load_luts)
>> +		dev_priv->display.load_luts(crtc_state);
>>  }
>>
>>  int intel_color_check(struct intel_crtc_state *crtc_state)
>> --
>> 1.9.1
>>
>
>--
>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

Re: [v5 4/6] drm/i915/icl: Add icl pipe degamma and gamma support

[Matt Roper]

On Tue, Jan 08, 2019 at 01:07:31PM +0530, Uma Shankar wrote:
> Add support for icl pipe degamma and gamma.
> 
> v2: Removed a POSTING_READ and corrected the Bit
> Definition as per Maarten's comments.
> 
> v3: Addressed Matt's review comments. Removed rmw patterns
> as suggested by Matt.
> 
> v4: Fixed Matt's review comments.
> 
> v5: Corrected macro alignment as per Jani Nikula's comments.
> Addressed Ville and Matt's  review comments.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    | 12 ++++---
>  drivers/gpu/drm/i915/intel_color.c | 65 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 72 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 44958d9..f29eef7 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7088,11 +7088,13 @@ enum {
>  #define _GAMMA_MODE_A		0x4a480
>  #define _GAMMA_MODE_B		0x4ac80
>  #define GAMMA_MODE(pipe) _MMIO_PIPE(pipe, _GAMMA_MODE_A, _GAMMA_MODE_B)
> -#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)
> -#define GAMMA_MODE_MODE_SPLIT	(3 << 0)
> +#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)
> +#define  GAMMA_MODE_MODE_SPLIT	(3 << 0)
>  
>  /* DMC/CSR */
>  #define CSR_PROGRAM(i)		_MMIO(0x80000 + (i) * 4)
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 9a72e64..9cd4646 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -520,6 +520,69 @@ static void glk_load_luts(struct intel_crtc_state *crtc_state)
>  	POSTING_READ(GAMMA_MODE(pipe));
>  }
>  
> +static void icl_load_degamma_lut(struct intel_crtc_state *crtc_state)

As Ville noted, I think the degamma LUT works the same way on GLK-ICL;
the gamma part may be different (and there's extra stuff like the extra
output CSC on ICL), but for degamma specifically I think the code you
wrote below could just be used to replace the current body of
glk_load_degamma_lut rather than adding it as a separate function.

Since GLK-ICL only support equal r/g/b values, we also need to land the
LUT validation patches I wrote in December.  Those are fully reviewed so
I'll do that as soon as I get an ack from Dave/Daniel to merge the drm
core patch through the Intel tree.


Matt

> +{
> +	struct drm_device *dev = crtc_state->base.crtc->dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
> +	const uint32_t lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
> +	uint32_t i;
> +
> +	/*
> +	 * When setting the auto-increment bit, the hardware seems to
> +	 * ignore the index bits, so we need to reset it to index 0
> +	 * separately.
> +	 */
> +	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0);
> +	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), PRE_CSC_GAMC_AUTO_INCREMENT);
> +
> +	if (crtc_state->base.degamma_lut) {
> +		struct drm_color_lut *lut = crtc_state->base.degamma_lut->data;
> +
> +		for (i = 0; i < lut_size; i++) {
> +			/*
> +			 * First 33 entries represent range from 0 to 1.0
> +			 * 34th and 35th entry will represent extended range
> +			 * inputs 3.0 and 7.0 respectively, currently clamped
> +			 * at 1.0. Since the precision is 16bit, the user value
> +			 * can be directly filled to register.
> +			 * ToDo: Extend to max 7.0.
> +			 */
> +			I915_WRITE(PRE_CSC_GAMC_DATA(pipe), lut[i].green);
> +		}
> +	} else {
> +		/* load a linear table. */
> +		for (i = 0; i < lut_size; i++) {
> +			uint32_t v = (i * (1 << 16)) / (lut_size - 1);
> +
> +			I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v);
> +		}
> +	}
> +
> +	/* Clamp values > 1.0. */
> +	while (i++ < 35)
> +		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16));
> +}
> +
> +static void icl_load_luts(struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_crtc *crtc = crtc_state->base.crtc;
> +	struct drm_device *dev = crtc_state->base.crtc->dev;
> +	struct drm_i915_private *dev_priv = to_i915(dev);
> +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +
> +	if (crtc_state_is_legacy_gamma(crtc_state)) {
> +		haswell_load_luts(crtc_state);
> +		return;
> +	}
> +
> +	icl_load_degamma_lut(crtc_state);
> +	bdw_load_gamma_lut(crtc_state, 0);
> +
> +	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT |
> +		   PRE_CSC_GAMMA_ENABLE | POST_CSC_GAMMA_ENABLE);
> +}
> +
>  /* Loads the palette/gamma unit for the CRTC on CherryView. */
>  static void cherryview_load_luts(struct intel_crtc_state *crtc_state)
>  {
> @@ -636,6 +699,8 @@ void intel_color_init(struct intel_crtc *crtc)
>  	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
>  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
>  		dev_priv->display.load_luts = glk_load_luts;
> +	} else if (IS_ICELAKE(dev_priv)) {
> +		dev_priv->display.load_luts = icl_load_luts;
>  	} else {
>  		dev_priv->display.load_luts = i9xx_load_luts;
>  	}
> -- 
> 1.9.1
> 

-- 
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

Re: [v5 5/6] drm/i915/icl: Enable ICL Pipe CSC block

[Matt Roper]

On Tue, Jan 08, 2019 at 01:07:32PM +0530, Uma Shankar wrote:
> Enable ICL pipe csc hardware. CSC block is enabled
> in CSC_MODE register instead of PLANE_COLOR_CTL.
> 
> ToDO: Extend the ABI to accept 32 bit coefficient values
> instead of 16bit for future platforms.
> 
> v2: Addressed Maarten's review comments.
> 
> v3: Addressed Matt's review comments. Removed rmw patterns
> as suggested by Matt.
> 
> v4: Addressed Matt's review comments.
> 
> v5: Addressed Ville's review comments.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    | 10 +++++++---
>  drivers/gpu/drm/i915/intel_color.c | 12 ++++++++++--
>  2 files changed, 17 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f29eef7..5a262c0 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9861,10 +9861,14 @@ enum skl_power_gate {
>  #define _PIPE_A_CSC_COEFF_BU	0x4901c
>  #define _PIPE_A_CSC_COEFF_RV_GV	0x49020
>  #define _PIPE_A_CSC_COEFF_BV	0x49024
> +
>  #define _PIPE_A_CSC_MODE	0x49028
> -#define   CSC_BLACK_SCREEN_OFFSET	(1 << 2)
> -#define   CSC_POSITION_BEFORE_GAMMA	(1 << 1)
> -#define   CSC_MODE_YUV_TO_RGB		(1 << 0)
> +#define  ICL_CSC_ENABLE			(1 << 31)
> +#define  ICL_OUTPUT_CSC_ENABLE		(1 << 30)
> +#define  CSC_BLACK_SCREEN_OFFSET	(1 << 2)
> +#define  CSC_POSITION_BEFORE_GAMMA	(1 << 1)
> +#define  CSC_MODE_YUV_TO_RGB		(1 << 0)
> +
>  #define _PIPE_A_CSC_PREOFF_HI	0x49030
>  #define _PIPE_A_CSC_PREOFF_ME	0x49034
>  #define _PIPE_A_CSC_PREOFF_LO	0x49038
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 9cd4646..c3e4ff6 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -134,7 +134,11 @@ static void ilk_load_ycbcr_conversion_matrix(struct intel_crtc *crtc)
>  	I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI);
>  	I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), POSTOFF_RGB_TO_YUV_ME);
>  	I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO);
> -	I915_WRITE(PIPE_CSC_MODE(pipe), 0);
> +
> +	if (INTEL_GEN(dev_priv) >= 11)
> +		I915_WRITE(PIPE_CSC_MODE(pipe), ICL_OUTPUT_CSC_ENABLE);

For gen11+, shouldn't we be programming OUTPUT_CSC_COEFF instead of
CSC_COEFF if we set this bit?

> +	else
> +		I915_WRITE(PIPE_CSC_MODE(pipe), 0);
>  }
>  
>  static void ilk_load_csc_matrix(struct intel_crtc_state *crtc_state)
> @@ -242,7 +246,10 @@ static void ilk_load_csc_matrix(struct intel_crtc_state *crtc_state)
>  		I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff);
>  		I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff);
>  
> -		I915_WRITE(PIPE_CSC_MODE(pipe), 0);
> +		if (INTEL_GEN(dev_priv) >= 11)
> +			I915_WRITE(PIPE_CSC_MODE(pipe), ICL_CSC_ENABLE);
> +		else
> +			I915_WRITE(PIPE_CSC_MODE(pipe), 0);

I might be misinterpreting how the hardware works, but my impression was
that we had two distinct CSC units now on gen11+.  So we could use the
traditional CSC registers to hold the userspace-provided CTM and the new
output CSC registers to hold the fixed RGB->YUV matrix, and they could
both be used at the same time if necessary.  It looks like this function
is still assuming that the two are mutually exclusive and that if we
need RGB->YUV we never bother programming the userspace matrix.  Is that
intentional or an oversight?  Aside from the register definitions
themselves, the bspec seems to be pretty sparse on how the whole
pipeline goes together...


Matt

>  	} else {
>  		uint32_t mode = CSC_MODE_YUV_TO_RGB;
>  
> @@ -700,6 +707,7 @@ void intel_color_init(struct intel_crtc *crtc)
>  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
>  		dev_priv->display.load_luts = glk_load_luts;
>  	} else if (IS_ICELAKE(dev_priv)) {
> +		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
>  		dev_priv->display.load_luts = icl_load_luts;
>  	} else {
>  		dev_priv->display.load_luts = i9xx_load_luts;
> -- 
> 1.9.1
> 

-- 
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

Re: [v5 6/6] drm/i915/icl: Add degamma and gamma lut size to gen11 caps

[Matt Roper]

On Tue, Jan 08, 2019 at 01:07:33PM +0530, Uma Shankar wrote:
> Add the degamma and gamma lut sizes to gen11 capability
> structure.
> 
> Note: Currently this doesn't account for the extended range gamma
> entries and this will be addressed with new segmented gamma ABI
> in a future patch.
> 
> v2: Reorder the patch as per Maarten's suggestion.
> 
> v3: Rebase
> 
> v4: Updated commit message with a note as per Matt's suggestion.
> 
> v5: No Change.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>

On one of the earlier patches, we suggested using the new degamma
loading function for glk/cnl as well.  If you do that, you'll also
probably want to update the degamma_lut_size for GLK as well, although
that can be done as a separate patch if you like.

The values here look correct for ICL, so

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

> ---
>  drivers/gpu/drm/i915/i915_pci.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
> index dd4aff2..14e5bb4 100644
> --- a/drivers/gpu/drm/i915/i915_pci.c
> +++ b/drivers/gpu/drm/i915/i915_pci.c
> @@ -639,7 +639,8 @@
>  	}, \
>  	GEN(11), \
>  	.ddb_size = 2048, \
> -	.has_logical_ring_elsq = 1
> +	.has_logical_ring_elsq = 1, \
> +	.color = { .degamma_lut_size = 33, .gamma_lut_size = 1024 }
>  
>  static const struct intel_device_info intel_icelake_11_info = {
>  	GEN11_FEATURES,
> -- 
> 1.9.1
> 

-- 
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

Re: [v5 4/6] drm/i915/icl: Add icl pipe degamma and gamma support

[Shankar, Uma]

>-----Original Message-----
>From: Roper, Matthew D
>Sent: Saturday, January 12, 2019 3:49 AM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Lankhorst, Maarten
><maarten.lankhorst@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Sharma,
>Shashank <shashank.sharma@intel.com>
>Subject: Re: [v5 4/6] drm/i915/icl: Add icl pipe degamma and gamma support
>
>On Tue, Jan 08, 2019 at 01:07:31PM +0530, Uma Shankar wrote:
>> Add support for icl pipe degamma and gamma.
>>
>> v2: Removed a POSTING_READ and corrected the Bit Definition as per
>> Maarten's comments.
>>
>> v3: Addressed Matt's review comments. Removed rmw patterns as
>> suggested by Matt.
>>
>> v4: Fixed Matt's review comments.
>>
>> v5: Corrected macro alignment as per Jani Nikula's comments.
>> Addressed Ville and Matt's  review comments.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h    | 12 ++++---
>>  drivers/gpu/drm/i915/intel_color.c | 65
>> ++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 72 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h index 44958d9..f29eef7 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7088,11 +7088,13 @@ enum {
>>  #define _GAMMA_MODE_A		0x4a480
>>  #define _GAMMA_MODE_B		0x4ac80
>>  #define GAMMA_MODE(pipe) _MMIO_PIPE(pipe, _GAMMA_MODE_A,
>_GAMMA_MODE_B)
>> -#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)
>> -#define GAMMA_MODE_MODE_SPLIT	(3 << 0)
>> +#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)
>> +#define  GAMMA_MODE_MODE_SPLIT	(3 << 0)
>>
>>  /* DMC/CSR */
>>  #define CSR_PROGRAM(i)		_MMIO(0x80000 + (i) * 4)
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index 9a72e64..9cd4646 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -520,6 +520,69 @@ static void glk_load_luts(struct intel_crtc_state
>*crtc_state)
>>  	POSTING_READ(GAMMA_MODE(pipe));
>>  }
>>
>> +static void icl_load_degamma_lut(struct intel_crtc_state *crtc_state)
>
>As Ville noted, I think the degamma LUT works the same way on GLK-ICL; the
>gamma part may be different (and there's extra stuff like the extra output CSC on
>ICL), but for degamma specifically I think the code you wrote below could just be
>used to replace the current body of glk_load_degamma_lut rather than adding it
>as a separate function.

Ok, will merge that to one function and extend GLK to handle this instead of the
current pass through.

>Since GLK-ICL only support equal r/g/b values, we also need to land the LUT
>validation patches I wrote in December.  Those are fully reviewed so I'll do that as
>soon as I get an ack from Dave/Daniel to merge the drm core patch through the
>Intel tree.

Yes, this would be needed. Thanks Matt for taking this up.

Regards,
Uma Shankar

>
>Matt
>
>> +{
>> +	struct drm_device *dev = crtc_state->base.crtc->dev;
>> +	struct drm_i915_private *dev_priv = to_i915(dev);
>> +	enum pipe pipe = to_intel_crtc(crtc_state->base.crtc)->pipe;
>> +	const uint32_t lut_size = INTEL_INFO(dev_priv)-
>>color.degamma_lut_size;
>> +	uint32_t i;
>> +
>> +	/*
>> +	 * When setting the auto-increment bit, the hardware seems to
>> +	 * ignore the index bits, so we need to reset it to index 0
>> +	 * separately.
>> +	 */
>> +	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0);
>> +	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe),
>PRE_CSC_GAMC_AUTO_INCREMENT);
>> +
>> +	if (crtc_state->base.degamma_lut) {
>> +		struct drm_color_lut *lut = crtc_state->base.degamma_lut-
>>data;
>> +
>> +		for (i = 0; i < lut_size; i++) {
>> +			/*
>> +			 * First 33 entries represent range from 0 to 1.0
>> +			 * 34th and 35th entry will represent extended range
>> +			 * inputs 3.0 and 7.0 respectively, currently clamped
>> +			 * at 1.0. Since the precision is 16bit, the user value
>> +			 * can be directly filled to register.
>> +			 * ToDo: Extend to max 7.0.
>> +			 */
>> +			I915_WRITE(PRE_CSC_GAMC_DATA(pipe), lut[i].green);
>> +		}
>> +	} else {
>> +		/* load a linear table. */
>> +		for (i = 0; i < lut_size; i++) {
>> +			uint32_t v = (i * (1 << 16)) / (lut_size - 1);
>> +
>> +			I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v);
>> +		}
>> +	}
>> +
>> +	/* Clamp values > 1.0. */
>> +	while (i++ < 35)
>> +		I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16)); }
>> +
>> +static void icl_load_luts(struct intel_crtc_state *crtc_state) {
>> +	struct drm_crtc *crtc = crtc_state->base.crtc;
>> +	struct drm_device *dev = crtc_state->base.crtc->dev;
>> +	struct drm_i915_private *dev_priv = to_i915(dev);
>> +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>> +
>> +	if (crtc_state_is_legacy_gamma(crtc_state)) {
>> +		haswell_load_luts(crtc_state);
>> +		return;
>> +	}
>> +
>> +	icl_load_degamma_lut(crtc_state);
>> +	bdw_load_gamma_lut(crtc_state, 0);
>> +
>> +	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_10BIT |
>> +		   PRE_CSC_GAMMA_ENABLE | POST_CSC_GAMMA_ENABLE); }
>> +
>>  /* Loads the palette/gamma unit for the CRTC on CherryView. */
>> static void cherryview_load_luts(struct intel_crtc_state *crtc_state)
>> { @@ -636,6 +699,8 @@ void intel_color_init(struct intel_crtc *crtc)
>>  	} else if (IS_GEMINILAKE(dev_priv) || IS_CANNONLAKE(dev_priv)) {
>>  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
>>  		dev_priv->display.load_luts = glk_load_luts;
>> +	} else if (IS_ICELAKE(dev_priv)) {
>> +		dev_priv->display.load_luts = icl_load_luts;
>>  	} else {
>>  		dev_priv->display.load_luts = i9xx_load_luts;
>>  	}
>> --
>> 1.9.1
>>
>
>--
>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

Re: [v5 5/6] drm/i915/icl: Enable ICL Pipe CSC block

[Shankar, Uma]

>-----Original Message-----
>From: Roper, Matthew D
>Sent: Saturday, January 12, 2019 4:30 AM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Lankhorst, Maarten
><maarten.lankhorst@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Sharma,
>Shashank <shashank.sharma@intel.com>
>Subject: Re: [v5 5/6] drm/i915/icl: Enable ICL Pipe CSC block
>
>On Tue, Jan 08, 2019 at 01:07:32PM +0530, Uma Shankar wrote:
>> Enable ICL pipe csc hardware. CSC block is enabled in CSC_MODE
>> register instead of PLANE_COLOR_CTL.
>>
>> ToDO: Extend the ABI to accept 32 bit coefficient values instead of
>> 16bit for future platforms.
>>
>> v2: Addressed Maarten's review comments.
>>
>> v3: Addressed Matt's review comments. Removed rmw patterns as
>> suggested by Matt.
>>
>> v4: Addressed Matt's review comments.
>>
>> v5: Addressed Ville's review comments.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h    | 10 +++++++---
>>  drivers/gpu/drm/i915/intel_color.c | 12 ++++++++++--
>>  2 files changed, 17 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h index f29eef7..5a262c0 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -9861,10 +9861,14 @@ enum skl_power_gate {
>>  #define _PIPE_A_CSC_COEFF_BU	0x4901c
>>  #define _PIPE_A_CSC_COEFF_RV_GV	0x49020
>>  #define _PIPE_A_CSC_COEFF_BV	0x49024
>> +
>>  #define _PIPE_A_CSC_MODE	0x49028
>> -#define   CSC_BLACK_SCREEN_OFFSET	(1 << 2)
>> -#define   CSC_POSITION_BEFORE_GAMMA	(1 << 1)
>> -#define   CSC_MODE_YUV_TO_RGB		(1 << 0)
>> +#define  ICL_CSC_ENABLE			(1 << 31)
>> +#define  ICL_OUTPUT_CSC_ENABLE		(1 << 30)
>> +#define  CSC_BLACK_SCREEN_OFFSET	(1 << 2)
>> +#define  CSC_POSITION_BEFORE_GAMMA	(1 << 1)
>> +#define  CSC_MODE_YUV_TO_RGB		(1 << 0)
>> +
>>  #define _PIPE_A_CSC_PREOFF_HI	0x49030
>>  #define _PIPE_A_CSC_PREOFF_ME	0x49034
>>  #define _PIPE_A_CSC_PREOFF_LO	0x49038
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index 9cd4646..c3e4ff6 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -134,7 +134,11 @@ static void ilk_load_ycbcr_conversion_matrix(struct
>intel_crtc *crtc)
>>  	I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), POSTOFF_RGB_TO_YUV_HI);
>>  	I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe),
>POSTOFF_RGB_TO_YUV_ME);
>>  	I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), POSTOFF_RGB_TO_YUV_LO);
>> -	I915_WRITE(PIPE_CSC_MODE(pipe), 0);
>> +
>> +	if (INTEL_GEN(dev_priv) >= 11)
>> +		I915_WRITE(PIPE_CSC_MODE(pipe),
>ICL_OUTPUT_CSC_ENABLE);
>
>For gen11+, shouldn't we be programming OUTPUT_CSC_COEFF instead of
>CSC_COEFF if we set this bit?

Yeah you are right, ideally output CSC coeff should be programmed to utilize this.
Will add that support.

>> +	else
>> +		I915_WRITE(PIPE_CSC_MODE(pipe), 0);
>>  }
>>
>>  static void ilk_load_csc_matrix(struct intel_crtc_state *crtc_state)
>> @@ -242,7 +246,10 @@ static void ilk_load_csc_matrix(struct intel_crtc_state
>*crtc_state)
>>  		I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff);
>>  		I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff);
>>
>> -		I915_WRITE(PIPE_CSC_MODE(pipe), 0);
>> +		if (INTEL_GEN(dev_priv) >= 11)
>> +			I915_WRITE(PIPE_CSC_MODE(pipe), ICL_CSC_ENABLE);
>> +		else
>> +			I915_WRITE(PIPE_CSC_MODE(pipe), 0);
>
>I might be misinterpreting how the hardware works, but my impression was that
>we had two distinct CSC units now on gen11+.  So we could use the traditional
>CSC registers to hold the userspace-provided CTM and the new output CSC
>registers to hold the fixed RGB->YUV matrix, and they could both be used at the
>same time if necessary.  It looks like this function is still assuming that the two are
>mutually exclusive and that if we need RGB->YUV we never bother programming
>the userspace matrix.  Is that intentional or an oversight?  Aside from the register
>definitions themselves, the bspec seems to be pretty sparse on how the whole
>pipeline goes together...

Ideally both CSC blocks can co-exist, current design is indeed limiting this. Will modify
to handle this properly.

Regards,
Uma Shankar
>
>Matt
>
>>  	} else {
>>  		uint32_t mode = CSC_MODE_YUV_TO_RGB;
>>
>> @@ -700,6 +707,7 @@ void intel_color_init(struct intel_crtc *crtc)
>>  		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
>>  		dev_priv->display.load_luts = glk_load_luts;
>>  	} else if (IS_ICELAKE(dev_priv)) {
>> +		dev_priv->display.load_csc_matrix = ilk_load_csc_matrix;
>>  		dev_priv->display.load_luts = icl_load_luts;
>>  	} else {
>>  		dev_priv->display.load_luts = i9xx_load_luts;
>> --
>> 1.9.1
>>
>
>--
>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

Re: [v5 6/6] drm/i915/icl: Add degamma and gamma lut size to gen11 caps

[Shankar, Uma]

>-----Original Message-----
>From: Roper, Matthew D
>Sent: Saturday, January 12, 2019 4:35 AM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Lankhorst, Maarten
><maarten.lankhorst@intel.com>; Syrjala, Ville <ville.syrjala@intel.com>; Sharma,
>Shashank <shashank.sharma@intel.com>
>Subject: Re: [v5 6/6] drm/i915/icl: Add degamma and gamma lut size to gen11
>caps
>
>On Tue, Jan 08, 2019 at 01:07:33PM +0530, Uma Shankar wrote:
>> Add the degamma and gamma lut sizes to gen11 capability structure.
>>
>> Note: Currently this doesn't account for the extended range gamma
>> entries and this will be addressed with new segmented gamma ABI in a
>> future patch.
>>
>> v2: Reorder the patch as per Maarten's suggestion.
>>
>> v3: Rebase
>>
>> v4: Updated commit message with a note as per Matt's suggestion.
>>
>> v5: No Change.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>
>On one of the earlier patches, we suggested using the new degamma loading
>function for glk/cnl as well.  If you do that, you'll also probably want to update
>the degamma_lut_size for GLK as well, although that can be done as a separate
>patch if you like.

I have updated GLK degamma to handle this correctly and ICL to re-use that.

>
>The values here look correct for ICL, so
>
>Reviewed-by: Matt Roper <matthew.d.roper@intel.com>

Thanks Matt for the review and all the valuable corrections and inputs.

Regards,
Uma Shankar

>> ---
>>  drivers/gpu/drm/i915/i915_pci.c | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_pci.c
>> b/drivers/gpu/drm/i915/i915_pci.c index dd4aff2..14e5bb4 100644
>> --- a/drivers/gpu/drm/i915/i915_pci.c
>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>> @@ -639,7 +639,8 @@
>>  	}, \
>>  	GEN(11), \
>>  	.ddb_size = 2048, \
>> -	.has_logical_ring_elsq = 1
>> +	.has_logical_ring_elsq = 1, \
>> +	.color = { .degamma_lut_size = 33, .gamma_lut_size = 1024 }
>>
>>  static const struct intel_device_info intel_icelake_11_info = {
>>  	GEN11_FEATURES,
>> --
>> 1.9.1
>>
>
>--
>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