[v4 0/4] Add support for Gen 11 pipe color features

[v4 0/4] 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.

Uma Shankar (4):
  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      |  7 ++-
 drivers/gpu/drm/i915/intel_color.c   | 84 +++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_display.c |  3 --
 drivers/gpu/drm/i915/intel_drv.h     |  3 --
 5 files changed, 86 insertions(+), 14 deletions(-)

-- 
1.9.1

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

[v4 1/4] 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.

Credits-to: Matt Roper <matthew.d.roper@intel.com>

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c   | 5 +----
 drivers/gpu/drm/i915/intel_display.c | 3 ---
 drivers/gpu/drm/i915/intel_drv.h     | 3 ---
 3 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 37fd9dd..f32e4a7 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -370,12 +370,11 @@ static void haswell_load_luts(struct intel_crtc_state *crtc_state)
 	 * 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)) {
+		(I915_READ(GAMMA_MODE(crtc->pipe)) & 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);
@@ -476,7 +475,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 +530,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 3b70948..704d9d3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9679,9 +9679,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 1028af8..7427a36 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -921,9 +921,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

[v4 2/4] 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.

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 02af9b5..1852c33 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7092,6 +7092,9 @@ enum {
 #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)
+
 /* DMC/CSR */
 #define CSR_PROGRAM(i)		_MMIO(0x80000 + (i) * 4)
 #define CSR_SSP_BASE_ADDR_GEN9	0x00002FC0
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index f32e4a7..e72d8d6 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -534,6 +534,71 @@ 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].red);
+		}
+	} 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;
+	u32 gamma_mode = 0;
+
+	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);
+
+	gamma_mode = GAMMA_MODE_MODE_10BIT | PRE_CSC_GAMMA_ENABLE
+			| POST_CSC_GAMMA_ENABLE;
+	I915_WRITE(GAMMA_MODE(pipe), gamma_mode);
+}
+
 /* Loads the palette/gamma unit for the CRTC on CherryView. */
 static void cherryview_load_luts(struct intel_crtc_state *crtc_state)
 {
@@ -649,6 +714,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

[v4 3/4] 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.

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1852c33..565ef6a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -9861,7 +9861,9 @@ enum skl_power_gate {
 #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_ENABLE			(1 << 31)
+#define	  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
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index e72d8d6..d5b240c 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) >= 10)
+		I915_WRITE(PIPE_CSC_MODE(pipe), 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) >= 10)
+			I915_WRITE(PIPE_CSC_MODE(pipe), CSC_ENABLE);
+		else
+			I915_WRITE(PIPE_CSC_MODE(pipe), 0);
 	} else {
 		uint32_t mode = CSC_MODE_YUV_TO_RGB;
 
@@ -715,6 +722,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

[v4 4/4] 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.

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 6350db5..add5642 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -635,7 +635,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 (rev4)

[Patchwork]

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_5337 -> Patchwork_11143
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_create@basic-files:
    - fi-bsw-n3050:       PASS -> INCOMPLETE [fdo#105876] / [fdo#108714]

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-blb-e6850:       PASS -> INCOMPLETE [fdo#107718]

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

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

  * {igt@runner@aborted}:
    - fi-icl-y:           NOTRUN -> FAIL [fdo#108915]

  
#### Possible fixes ####

  * igt@kms_flip@basic-flip-vs-dpms:
    - fi-skl-6700hq:      DMESG-WARN [fdo#105998] -> PASS

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

  
#### Warnings ####

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

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

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#105876]: https://bugs.freedesktop.org/show_bug.cgi?id=105876
  [fdo#105998]: https://bugs.freedesktop.org/show_bug.cgi?id=105998
  [fdo#107362]: https://bugs.freedesktop.org/show_bug.cgi?id=107362
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108315]: https://bugs.freedesktop.org/show_bug.cgi?id=108315
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108714]: https://bugs.freedesktop.org/show_bug.cgi?id=108714
  [fdo#108767]: https://bugs.freedesktop.org/show_bug.cgi?id=108767
  [fdo#108915]: https://bugs.freedesktop.org/show_bug.cgi?id=108915


Participating hosts (48 -> 41)
------------------------------

  Additional (1): fi-icl-y 
  Missing    (8): fi-kbl-soraka fi-ilk-m540 fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-gdg-551 fi-pnv-d510 


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

    * Linux: CI_DRM_5337 -> Patchwork_11143

  CI_DRM_5337: 3ac901085a9fae8699716ac44579dab1dec546c3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4752: 321fe77d32fef32ef820f53924045fe6ef0cf6ed @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11143: 1c61a958fc987d091ee67970494a3c12ab588b38 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

1c61a958fc98 drm/i915/icl: Add degamma and gamma lut size to gen11 caps
01fa72a76c2d drm/i915/icl: Enable ICL Pipe CSC block
dd1779407c88 drm/i915/icl: Add icl pipe degamma and gamma support
0ae0548c84eb drm/i915: Remove gamma_mode state variable

== Logs ==

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

Re: [v4 1/4] drm/i915: Remove gamma_mode state variable

[Matt Roper]

On Fri, Dec 21, 2018 at 01:29:38AM +0530, Uma Shankar wrote:
> 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.
> 
> Credits-to: Matt Roper <matthew.d.roper@intel.com>
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c   | 5 +----
>  drivers/gpu/drm/i915/intel_display.c | 3 ---
>  drivers/gpu/drm/i915/intel_drv.h     | 3 ---
>  3 files changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 37fd9dd..f32e4a7 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -370,12 +370,11 @@ static void haswell_load_luts(struct intel_crtc_state *crtc_state)
>  	 * 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)) {
> +		(I915_READ(GAMMA_MODE(crtc->pipe)) & GAMMA_MODE_MODE_SPLIT)) {

We can probably avoid this read on each LUT load if we just handle it
once during hardware readout/sanitization.  Since our driver never
actually sets MODE_SPLIT on this platform, we only have to worry about
the BIOS leaving us in split gamma mode; that means we could do

        if (ips)
                disable ips

        intel_color_set_csc()
        intel_color_load_luts()

        if (ips)
                re-enable ips

once during intel_sanitize_crtc(), to immediately program the hardware
to a known-good state at startup (linear CTM and disabled LUT's).  That
would also be a little bit more consistent overall; since we don't
actually readout the BIOS-set LUT's or CTM today, we wind up having them
remain active for a while until they go away with the first fastset or
modeset.

Otherwise, this patch looks good, so

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

and you can deal with sanitizing the color management stuff at bootup as
a separate patch later.


Matt

>  		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);
> @@ -476,7 +475,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 +530,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 3b70948..704d9d3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9679,9 +9679,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 1028af8..7427a36 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -921,9 +921,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
> 

-- 
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: [v4 2/4] drm/i915/icl: Add icl pipe degamma and gamma support

[Matt Roper]

On Fri, Dec 21, 2018 at 01:29:39AM +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.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  3 ++
>  drivers/gpu/drm/i915/intel_color.c | 67 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 70 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 02af9b5..1852c33 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7092,6 +7092,9 @@ enum {
>  #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)
> +
>  /* DMC/CSR */
>  #define CSR_PROGRAM(i)		_MMIO(0x80000 + (i) * 4)
>  #define CSR_SSP_BASE_ADDR_GEN9	0x00002FC0
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index f32e4a7..e72d8d6 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -534,6 +534,71 @@ 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].red);

Since 1.0 = 0x100, should we scale this by (0x100 / 0xFF) until we have
the newer segmented gamma ABI you're working on?

> +		}
> +	} 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;
> +	u32 gamma_mode = 0;
> +
> +	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);
> +
> +	gamma_mode = GAMMA_MODE_MODE_10BIT | PRE_CSC_GAMMA_ENABLE
> +			| POST_CSC_GAMMA_ENABLE;
> +	I915_WRITE(GAMMA_MODE(pipe), gamma_mode);

We might as well just stick these constants right in the I915_WRITE
call; no need to create a separate local variable to hold them.


Matt

> +}
> +
>  /* Loads the palette/gamma unit for the CRTC on CherryView. */
>  static void cherryview_load_luts(struct intel_crtc_state *crtc_state)
>  {
> @@ -649,6 +714,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: [v4 1/4] drm/i915: Remove gamma_mode state variable

[Ville Syrjälä]

On Fri, Dec 21, 2018 at 01:29:38AM +0530, Uma Shankar wrote:
> 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.

I'm going to add it back at some point.

> 
> Credits-to: Matt Roper <matthew.d.roper@intel.com>
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c   | 5 +----
>  drivers/gpu/drm/i915/intel_display.c | 3 ---
>  drivers/gpu/drm/i915/intel_drv.h     | 3 ---
>  3 files changed, 1 insertion(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 37fd9dd..f32e4a7 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -370,12 +370,11 @@ static void haswell_load_luts(struct intel_crtc_state *crtc_state)
>  	 * 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)) {
> +		(I915_READ(GAMMA_MODE(crtc->pipe)) & 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);
> @@ -476,7 +475,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 +530,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 3b70948..704d9d3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9679,9 +9679,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 1028af8..7427a36 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -921,9 +921,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

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

Re: [v4 3/4] drm/i915/icl: Enable ICL Pipe CSC block

[Ville Syrjälä]

On Fri, Dec 21, 2018 at 01:29:40AM +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.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  4 +++-
>  drivers/gpu/drm/i915/intel_color.c | 12 ++++++++++--
>  2 files changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 1852c33..565ef6a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -9861,7 +9861,9 @@ enum skl_power_gate {
>  #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_ENABLE			(1 << 31)
> +#define	  OUTPUT_CSC_ENABLE		(1 << 30)
> +#define	  CSC_BLACK_SCREEN_OFFSET	(1 << 2)

Bogus space->tab change.

We should probably document which bit is for which platforms.

>  #define   CSC_POSITION_BEFORE_GAMMA	(1 << 1)
>  #define   CSC_MODE_YUV_TO_RGB		(1 << 0)
>  #define _PIPE_A_CSC_PREOFF_HI	0x49030
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index e72d8d6..d5b240c 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) >= 10)

11

> +		I915_WRITE(PIPE_CSC_MODE(pipe), 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) >= 10)

11

> +			I915_WRITE(PIPE_CSC_MODE(pipe), CSC_ENABLE);
> +		else
> +			I915_WRITE(PIPE_CSC_MODE(pipe), 0);
>  	} else {
>  		uint32_t mode = CSC_MODE_YUV_TO_RGB;
>  
> @@ -715,6 +722,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

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

Re: [v4 2/4] drm/i915/icl: Add icl pipe degamma and gamma support

[Ville Syrjälä]

On Fri, Dec 21, 2018 at 01:29:39AM +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.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  3 ++
>  drivers/gpu/drm/i915/intel_color.c | 67 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 70 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 02af9b5..1852c33 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7092,6 +7092,9 @@ enum {
>  #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)
> +
>  /* DMC/CSR */
>  #define CSR_PROGRAM(i)		_MMIO(0x80000 + (i) * 4)
>  #define CSR_SSP_BASE_ADDR_GEN9	0x00002FC0
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index f32e4a7..e72d8d6 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -534,6 +534,71 @@ 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].red);

red? I would have gone for 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));

This is the glk+ degamma lut is it not? Why is this code 
pretending to be icl+ only?

IMO step 1 should be to just reuse the current glk code for icl.
Which I think boils down to just setting the correct bit(s) in
GAMMA_MODE. After that we can think of extending it to support
user provided degamma. 

There is a bit of a problem with that on glk/cnl though
since we don't have the output csc. Hence when the pipe csc
is used we need to either reject gamma (which doesn't sound 
very prone to work with legacy userspace) or we program the
degamma LUT with the data meant for the gamma LUT which has
its own problems.

> +}
> +
> +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;
> +	u32 gamma_mode = 0;
> +
> +	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);
> +
> +	gamma_mode = GAMMA_MODE_MODE_10BIT | PRE_CSC_GAMMA_ENABLE
> +			| POST_CSC_GAMMA_ENABLE;
> +	I915_WRITE(GAMMA_MODE(pipe), gamma_mode);
> +}
> +
>  /* Loads the palette/gamma unit for the CRTC on CherryView. */
>  static void cherryview_load_luts(struct intel_crtc_state *crtc_state)
>  {
> @@ -649,6 +714,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

-- 
Ville Syrjälä
Intel
_______________________________________________
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 (rev4)

[Patchwork]

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_5337_full -> Patchwork_11143_full
====================================================

Summary
-------

  **FAILURE**

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

### IGT changes ###

#### Possible regressions ####

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

  
#### Warnings ####

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@pi-ringfull-vebox:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103158]

  * igt@gem_ppgtt@blt-vs-render-ctxn:
    - shard-skl:          PASS -> TIMEOUT [fdo#108039]

  * igt@gem_softpin@noreloc-s3:
    - shard-iclb:         PASS -> INCOMPLETE [fdo#107713]

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

  * igt@i915_suspend@shrink:
    - shard-skl:          NOTRUN -> DMESG-WARN [fdo#108784]

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

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

  * igt@kms_busy@extended-modeset-hang-oldfb-with-reset-render-a:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] +32

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

  * igt@kms_color@pipe-c-ctm-red-to-blue:
    - shard-iclb:         SKIP -> DMESG-WARN [fdo#107724] / [fdo#108336] +1

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

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

  * igt@kms_flip@plain-flip-fb-recreate:
    - shard-skl:          PASS -> FAIL [fdo#100368] +1

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

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-mmap-wc:
    - shard-iclb:         PASS -> DMESG-WARN [fdo#107724] / [fdo#108336] +21

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

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

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

  * igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
    - shard-iclb:         PASS -> DMESG-FAIL [fdo#107724] +17

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

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

  * igt@kms_plane_scaling@pipe-a-scaler-with-rotation:
    - shard-iclb:         NOTRUN -> DMESG-WARN [fdo#107724]

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

  * igt@pm_backlight@fade_with_dpms:
    - shard-iclb:         NOTRUN -> INCOMPLETE [fdo#107820]

  
#### Possible fixes ####

  * igt@gem_exec_whisper@normal:
    - shard-skl:          TIMEOUT [fdo#108592] -> PASS

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

  * igt@kms_chv_cursor_fail@pipe-c-64x64-right-edge:
    - shard-skl:          FAIL [fdo#104671] -> PASS +2

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

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

  * igt@kms_cursor_crc@cursor-256x85-random:
    - shard-skl:          FAIL [fdo#103232] -> PASS

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

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

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

  * igt@kms_frontbuffer_tracking@fbc-farfromfence:
    - shard-kbl:          DMESG-WARN [fdo#103558] / [fdo#105602] -> PASS +21

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

  * igt@kms_plane@plane-position-covered-pipe-c-planes:
    - shard-apl:          FAIL [fdo#103166] -> PASS
    - shard-iclb:         FAIL [fdo#103166] -> PASS

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

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

  * igt@pm_rpm@dpms-mode-unset-lpsp:
    - shard-iclb:         INCOMPLETE [fdo#107713] / [fdo#108840] -> PASS

  * igt@pm_rpm@gem-execbuf-stress:
    - shard-skl:          INCOMPLETE [fdo#107803] / [fdo#107807] -> PASS

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

  * igt@pm_rpm@universal-planes-dpms:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS

  
#### Warnings ####

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-iclb:         FAIL [fdo#103232] -> DMESG-WARN [fdo#107724] / [fdo#108336] +1

  * igt@kms_frontbuffer_tracking@fbc-stridechange:
    - shard-iclb:         FAIL [fdo#105683] / [fdo#108040] -> DMESG-FAIL [fdo#107724]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-transparant-fb:
    - shard-kbl:          DMESG-FAIL [fdo#103558] / [fdo#105602] / [fdo#108145] -> FAIL [fdo#108145] +1

  
  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#103158]: https://bugs.freedesktop.org/show_bug.cgi?id=103158
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#104671]: https://bugs.freedesktop.org/show_bug.cgi?id=104671
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [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#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#107803]: https://bugs.freedesktop.org/show_bug.cgi?id=107803
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#107820]: https://bugs.freedesktop.org/show_bug.cgi?id=107820
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108039]: https://bugs.freedesktop.org/show_bug.cgi?id=108039
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108134]: https://bugs.freedesktop.org/show_bug.cgi?id=108134
  [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#108592]: https://bugs.freedesktop.org/show_bug.cgi?id=108592
  [fdo#108784]: https://bugs.freedesktop.org/show_bug.cgi?id=108784
  [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#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
  [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_5337 -> Patchwork_11143

  CI_DRM_5337: 3ac901085a9fae8699716ac44579dab1dec546c3 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4752: 321fe77d32fef32ef820f53924045fe6ef0cf6ed @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_11143: 1c61a958fc987d091ee67970494a3c12ab588b38 @ 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_11143/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v4 2/4] drm/i915/icl: Add icl pipe degamma and gamma support

[Jani Nikula]

On Fri, 21 Dec 2018, Uma Shankar <uma.shankar@intel.com> 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.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  3 ++
>  drivers/gpu/drm/i915/intel_color.c | 67 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 70 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 02af9b5..1852c33 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7092,6 +7092,9 @@ enum {
>  #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)

Please read the big comment near the top of this file about where to
place register content macros, ordering, indentation, spacing, etc.

BR,
Jani.

> +
>  /* DMC/CSR */
>  #define CSR_PROGRAM(i)		_MMIO(0x80000 + (i) * 4)
>  #define CSR_SSP_BASE_ADDR_GEN9	0x00002FC0
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index f32e4a7..e72d8d6 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -534,6 +534,71 @@ 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].red);
> +		}
> +	} 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;
> +	u32 gamma_mode = 0;
> +
> +	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);
> +
> +	gamma_mode = GAMMA_MODE_MODE_10BIT | PRE_CSC_GAMMA_ENABLE
> +			| POST_CSC_GAMMA_ENABLE;
> +	I915_WRITE(GAMMA_MODE(pipe), gamma_mode);
> +}
> +
>  /* Loads the palette/gamma unit for the CRTC on CherryView. */
>  static void cherryview_load_luts(struct intel_crtc_state *crtc_state)
>  {
> @@ -649,6 +714,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;
>  	}

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v4 1/4] drm/i915: Remove gamma_mode state variable

[Shankar, Uma]

>-----Original Message-----
>From: Roper, Matthew D
>Sent: Friday, December 21, 2018 6:54 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: [v4 1/4] drm/i915: Remove gamma_mode state variable
>
>On Fri, Dec 21, 2018 at 01:29:38AM +0530, Uma Shankar wrote:
>> 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.
>>
>> Credits-to: Matt Roper <matthew.d.roper@intel.com>
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_color.c   | 5 +----
>>  drivers/gpu/drm/i915/intel_display.c | 3 ---
>>  drivers/gpu/drm/i915/intel_drv.h     | 3 ---
>>  3 files changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index 37fd9dd..f32e4a7 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -370,12 +370,11 @@ static void haswell_load_luts(struct intel_crtc_state
>*crtc_state)
>>  	 * 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)) {
>> +		(I915_READ(GAMMA_MODE(crtc->pipe)) &
>GAMMA_MODE_MODE_SPLIT)) {
>
>We can probably avoid this read on each LUT load if we just handle it once during
>hardware readout/sanitization.  Since our driver never actually sets MODE_SPLIT
>on this platform, we only have to worry about the BIOS leaving us in split gamma
>mode; that means we could do
>
>        if (ips)
>                disable ips
>
>        intel_color_set_csc()
>        intel_color_load_luts()
>
>        if (ips)
>                re-enable ips
>
>once during intel_sanitize_crtc(), to immediately program the hardware to a
>known-good state at startup (linear CTM and disabled LUT's).  That would also be
>a little bit more consistent overall; since we don't actually readout the BIOS-set
>LUT's or CTM today, we wind up having them remain active for a while until they
>go away with the first fastset or modeset.
>
>Otherwise, this patch looks good, so
>
>Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>
>and you can deal with sanitizing the color management stuff at bootup as a
>separate patch later.

Thanks Matt for these useful suggestions. Will update the patch accordingly.

Regards,
Uma Shankar

>
>Matt
>
>>  		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);
>> @@ -476,7 +475,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 +530,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 3b70948..704d9d3 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -9679,9 +9679,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 1028af8..7427a36 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -921,9 +921,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
>>
>
>--
>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: [v4 1/4] drm/i915: Remove gamma_mode state variable

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Friday, December 21, 2018 10:57 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrjala@intel.com>;
>Lankhorst, Maarten <maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [v4 1/4] drm/i915: Remove gamma_mode state variable
>
>On Fri, Dec 21, 2018 at 01:29:38AM +0530, Uma Shankar wrote:
>> 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.
>
>I'm going to add it back at some point.

Hi Ville,
I believe we can drop it currently and add later as needed and handle the state more 
efficiently as to how was it done currently. I hope that is ok.

Regards,
Uma Shankar

>>
>> Credits-to: Matt Roper <matthew.d.roper@intel.com>
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_color.c   | 5 +----
>>  drivers/gpu/drm/i915/intel_display.c | 3 ---
>>  drivers/gpu/drm/i915/intel_drv.h     | 3 ---
>>  3 files changed, 1 insertion(+), 10 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index 37fd9dd..f32e4a7 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -370,12 +370,11 @@ static void haswell_load_luts(struct intel_crtc_state
>*crtc_state)
>>  	 * 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)) {
>> +		(I915_READ(GAMMA_MODE(crtc->pipe)) &
>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);
>> @@ -476,7 +475,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 +530,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 3b70948..704d9d3 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -9679,9 +9679,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 1028af8..7427a36 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -921,9 +921,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
>
>--
>Ville Syrjälä
>Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v4 2/4] drm/i915/icl: Add icl pipe degamma and gamma support

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Friday, December 21, 2018 11:09 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrjala@intel.com>;
>Lankhorst, Maarten <maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [v4 2/4] drm/i915/icl: Add icl pipe degamma and gamma
>support
>
>On Fri, Dec 21, 2018 at 01:29:39AM +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.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h    |  3 ++
>>  drivers/gpu/drm/i915/intel_color.c | 67
>> ++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 70 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h index 02af9b5..1852c33 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7092,6 +7092,9 @@ enum {
>>  #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)
>> +
>>  /* DMC/CSR */
>>  #define CSR_PROGRAM(i)		_MMIO(0x80000 + (i) * 4)
>>  #define CSR_SSP_BASE_ADDR_GEN9	0x00002FC0
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index f32e4a7..e72d8d6 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -534,6 +534,71 @@ 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].red);
>
>red? I would have gone for green.

Sure, will change it.

>> +		}
>> +	} 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));
>
>This is the glk+ degamma lut is it not? Why is this code pretending to be icl+ only?
>
>IMO step 1 should be to just reuse the current glk code for icl.
>Which I think boils down to just setting the correct bit(s) in GAMMA_MODE. After
>that we can think of extending it to support user provided degamma.
>
>There is a bit of a problem with that on glk/cnl though since we don't have the
>output csc. Hence when the pipe csc is used we need to either reject gamma
>(which doesn't sound very prone to work with legacy userspace) or we program
>the degamma LUT with the data meant for the gamma LUT which has its own
>problems.

I feel there are lot of differences with GLK and Gen11+, hence keeping them separate
looks better. Multi-segmented gamma is another addition along with output csc on GEN11.

For GLK, if we have done linear blending we may not need de-gamma and use CSC for any
range correction etc and then apply gamma to make it non-linear as per the desired colorspace.
In case of non-linear input to pipe (this will mostly happen if we have single non-linear
frame directly flipped to plane), degamma will be required to make it linear in order to really do
accurate CSC operations. As of now, degamma for GLK is just a pass through limiting this usecase.

AFAIK, the biggest usecase of Output CSC on Gen11 will be to work on nonlinear RGB data (blended
and after gamma applied for colorspace transfer function) and convert it to YUV formats.

Please let me know your recommendation. Will change the series accordingly.

Thanks & Regards,
Uma Shankar

>> +}
>> +
>> +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;
>> +	u32 gamma_mode = 0;
>> +
>> +	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);
>> +
>> +	gamma_mode = GAMMA_MODE_MODE_10BIT |
>PRE_CSC_GAMMA_ENABLE
>> +			| POST_CSC_GAMMA_ENABLE;
>> +	I915_WRITE(GAMMA_MODE(pipe), gamma_mode); }
>> +
>>  /* Loads the palette/gamma unit for the CRTC on CherryView. */
>> static void cherryview_load_luts(struct intel_crtc_state *crtc_state)
>> { @@ -649,6 +714,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
>
>--
>Ville Syrjälä
>Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v4 2/4] drm/i915/icl: Add icl pipe degamma and gamma support

[Shankar, Uma]

>-----Original Message-----
>From: Roper, Matthew D
>Sent: Friday, December 21, 2018 6:54 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: [v4 2/4] drm/i915/icl: Add icl pipe degamma and gamma support
>
>On Fri, Dec 21, 2018 at 01:29:39AM +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.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h    |  3 ++
>>  drivers/gpu/drm/i915/intel_color.c | 67
>> ++++++++++++++++++++++++++++++++++++++
>>  2 files changed, 70 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h index 02af9b5..1852c33 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7092,6 +7092,9 @@ enum {
>>  #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)
>> +
>>  /* DMC/CSR */
>>  #define CSR_PROGRAM(i)		_MMIO(0x80000 + (i) * 4)
>>  #define CSR_SSP_BASE_ADDR_GEN9	0x00002FC0
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index f32e4a7..e72d8d6 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -534,6 +534,71 @@ 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].red);
>
>Since 1.0 = 0x100, should we scale this by (0x100 / 0xFF) until we have the newer
>segmented gamma ABI you're working on?

One clarification here, 1.0 is actually 0x10000 (since its 3.16 representation), so we should scale it
up by 0x10000/0xFFFF isn't it ? Not sure if I am missing something.

Also is scaling really a good idea. For values less than 1.0 16bit user input will be accurate and we can
program exactly what user wants to hardware. Problem is only for value of 1.0, making every other value
different may not be a good idea. What do you recommend ?
 
>> +		}
>> +	} 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;
>> +	u32 gamma_mode = 0;
>> +
>> +	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);
>> +
>> +	gamma_mode = GAMMA_MODE_MODE_10BIT |
>PRE_CSC_GAMMA_ENABLE
>> +			| POST_CSC_GAMMA_ENABLE;
>> +	I915_WRITE(GAMMA_MODE(pipe), gamma_mode);
>
>We might as well just stick these constants right in the I915_WRITE call; no need
>to create a separate local variable to hold them.

Sure, will change this. Thanks Matt.

Regards,
Uma Shankar
>
>Matt
>
>> +}
>> +
>>  /* Loads the palette/gamma unit for the CRTC on CherryView. */
>> static void cherryview_load_luts(struct intel_crtc_state *crtc_state)
>> { @@ -649,6 +714,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: [v4 3/4] drm/i915/icl: Enable ICL Pipe CSC block

[Shankar, Uma]

>-----Original Message-----
>From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com]
>Sent: Friday, December 21, 2018 11:02 PM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrjala@intel.com>;
>Lankhorst, Maarten <maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [v4 3/4] drm/i915/icl: Enable ICL Pipe CSC block
>
>On Fri, Dec 21, 2018 at 01:29:40AM +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.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h    |  4 +++-
>>  drivers/gpu/drm/i915/intel_color.c | 12 ++++++++++--
>>  2 files changed, 13 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h index 1852c33..565ef6a 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -9861,7 +9861,9 @@ enum skl_power_gate {
>>  #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_ENABLE			(1 << 31)
>> +#define	  OUTPUT_CSC_ENABLE		(1 << 30)
>> +#define	  CSC_BLACK_SCREEN_OFFSET	(1 << 2)
>
>Bogus space->tab change.
>
>We should probably document which bit is for which platforms.

Will rectify this. Also prefix ICL for CSC and Output CSC fields since they
got introduced from Gen11 onwards.

>
>>  #define   CSC_POSITION_BEFORE_GAMMA	(1 << 1)
>>  #define   CSC_MODE_YUV_TO_RGB		(1 << 0)
>>  #define _PIPE_A_CSC_PREOFF_HI	0x49030
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index e72d8d6..d5b240c 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) >= 10)
>
>11

Yeah, will correct  this.

>> +		I915_WRITE(PIPE_CSC_MODE(pipe), 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) >= 10)
>
>11

Will update. Thanks Ville for your comments.

Regards,
Uma Shankar

>
>> +			I915_WRITE(PIPE_CSC_MODE(pipe), CSC_ENABLE);
>> +		else
>> +			I915_WRITE(PIPE_CSC_MODE(pipe), 0);
>>  	} else {
>>  		uint32_t mode = CSC_MODE_YUV_TO_RGB;
>>
>> @@ -715,6 +722,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
>
>--
>Ville Syrjälä
>Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx