All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 00/10] drm/i915: Prep work for finishing (de)gamma readout
@ 2022-09-29  7:15 Ville Syrjala
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 01/10] drm/i915: Remove PLL asserts from .load_luts() Ville Syrjala
                   ` (12 more replies)
  0 siblings, 13 replies; 26+ messages in thread
From: Ville Syrjala @ 2022-09-29  7:15 UTC (permalink / raw)
  To: intel-gfx

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

Add another layer of LUT blobs in order to make gamma
state readout/check possible on ilk-skl. As a bonus we
can also simplify the glk degamma vs. csc mess.

The actual state readout/checker stuff that we're
currently missing will follow later.

Ville Syrjälä (10):
  drm/i915: Remove PLL asserts from .load_luts()
  drm/i915: Split up intel_color_init()
  drm/i915: Simplify the intel_color_init_hooks() if ladder
  drm/i915: Clean up intel_color_init_hooks()
  drm/i915: Change glk_load_degamma_lut() calling convention
  drm/i915: Make ilk_load_luts() deal with degamma
  drm/i915: Introduce crtc_state->{pre,post}_csc_lut
  drm/i915: Assert {pre,post}_csc_lut were assigned sensibly
  drm/i915: Get rid of glk_load_degamma_lut_linear()
  drm/i915: Stop loading linear degammma LUT on glk needlessly

 drivers/gpu/drm/i915/display/intel_atomic.c   |   8 +
 drivers/gpu/drm/i915/display/intel_color.c    | 356 ++++++++++--------
 drivers/gpu/drm/i915/display/intel_color.h    |   6 +-
 drivers/gpu/drm/i915/display/intel_crtc.c     |   3 +-
 .../drm/i915/display/intel_crtc_state_dump.c  |  10 +-
 drivers/gpu/drm/i915/display/intel_display.c  |   9 +-
 .../gpu/drm/i915/display/intel_display_core.h |   5 +
 .../drm/i915/display/intel_display_types.h    |   4 +
 .../drm/i915/display/intel_modeset_setup.c    |   6 +
 9 files changed, 251 insertions(+), 156 deletions(-)

-- 
2.35.1


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

* [Intel-gfx] [PATCH 01/10] drm/i915: Remove PLL asserts from .load_luts()
  2022-09-29  7:15 [Intel-gfx] [PATCH 00/10] drm/i915: Prep work for finishing (de)gamma readout Ville Syrjala
@ 2022-09-29  7:15 ` Ville Syrjala
  2022-09-29 11:54   ` Jani Nikula
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 02/10] drm/i915: Split up intel_color_init() Ville Syrjala
                   ` (11 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjala @ 2022-09-29  7:15 UTC (permalink / raw)
  To: intel-gfx

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

.load_luts() potentially runs from the vblank worker, and is
under a deadline to complete within the vblank. Thus we can't
do expesive stuff like talk to the Punit, etc.

To that end get rid of the assert_dsi_pll_enabled() call for
vlv/chv. We'll just have to trust that the PLL is already enabled
here.

And I don't think the normal assert_pll_enabled() really buys us
anything useful on gmch platforms either, so nuke that one too.
We don't have corresponding asserts in the ilk+ codepaths anyway
despite the hardware (IIRC) still requiring the clock to be
enabled when we access the LUT.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 6bda4274eae9..bbc56affb3ec 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -25,9 +25,7 @@
 #include "intel_color.h"
 #include "intel_de.h"
 #include "intel_display_types.h"
-#include "intel_dpll.h"
 #include "intel_dsb.h"
-#include "vlv_dsi_pll.h"
 
 struct intel_color_funcs {
 	int (*color_check)(struct intel_crtc_state *crtc_state);
@@ -580,11 +578,8 @@ static void i9xx_load_lut_8(struct intel_crtc *crtc,
 static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
 
-	assert_pll_enabled(dev_priv, crtc->pipe);
-
 	i9xx_load_lut_8(crtc, gamma_lut);
 }
 
@@ -611,14 +606,8 @@ static void i965_load_lut_10p6(struct intel_crtc *crtc,
 static void i965_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
 
-	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
-		assert_dsi_pll_enabled(dev_priv);
-	else
-		assert_pll_enabled(dev_priv, crtc->pipe);
-
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
 		i9xx_load_lut_8(crtc, gamma_lut);
 	else
-- 
2.35.1


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

* [Intel-gfx] [PATCH 02/10] drm/i915: Split up intel_color_init()
  2022-09-29  7:15 [Intel-gfx] [PATCH 00/10] drm/i915: Prep work for finishing (de)gamma readout Ville Syrjala
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 01/10] drm/i915: Remove PLL asserts from .load_luts() Ville Syrjala
@ 2022-09-29  7:15 ` Ville Syrjala
  2022-09-29 10:41   ` Jani Nikula
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 03/10] drm/i915: Simplify the intel_color_init_hooks() if ladder Ville Syrjala
                   ` (10 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjala @ 2022-09-29  7:15 UTC (permalink / raw)
  To: intel-gfx

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

intel_color_init() does both device level and crtc level stuff.
Split it up accordingly.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c   | 15 +++++++++------
 drivers/gpu/drm/i915/display/intel_color.h   |  4 +++-
 drivers/gpu/drm/i915/display/intel_crtc.c    |  3 +--
 drivers/gpu/drm/i915/display/intel_display.c |  1 +
 4 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index bbc56affb3ec..ddfe7c257a72 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -2206,13 +2206,21 @@ static const struct intel_color_funcs ilk_color_funcs = {
 	.read_luts = ilk_read_luts,
 };
 
-void intel_color_init(struct intel_crtc *crtc)
+void intel_crtc_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	bool has_ctm = INTEL_INFO(dev_priv)->display.color.degamma_lut_size != 0;
 
 	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
 
+	drm_crtc_enable_color_mgmt(&crtc->base,
+				   INTEL_INFO(dev_priv)->display.color.degamma_lut_size,
+				   has_ctm,
+				   INTEL_INFO(dev_priv)->display.color.gamma_lut_size);
+}
+
+void intel_color_init_hooks(struct drm_i915_private *dev_priv)
+{
 	if (HAS_GMCH(dev_priv)) {
 		if (IS_CHERRYVIEW(dev_priv)) {
 			dev_priv->display.funcs.color = &chv_color_funcs;
@@ -2238,9 +2246,4 @@ void intel_color_init(struct intel_crtc *crtc)
 		} else
 			dev_priv->display.funcs.color = &ilk_color_funcs;
 	}
-
-	drm_crtc_enable_color_mgmt(&crtc->base,
-				   INTEL_INFO(dev_priv)->display.color.degamma_lut_size,
-				   has_ctm,
-				   INTEL_INFO(dev_priv)->display.color.gamma_lut_size);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_color.h b/drivers/gpu/drm/i915/display/intel_color.h
index fd873425e082..67702451e2fd 100644
--- a/drivers/gpu/drm/i915/display/intel_color.h
+++ b/drivers/gpu/drm/i915/display/intel_color.h
@@ -10,9 +10,11 @@
 
 struct intel_crtc_state;
 struct intel_crtc;
+struct drm_i915_private;
 struct drm_property_blob;
 
-void intel_color_init(struct intel_crtc *crtc);
+void intel_color_init_hooks(struct drm_i915_private *i915);
+void intel_crtc_color_init(struct intel_crtc *crtc);
 int intel_color_check(struct intel_crtc_state *crtc_state);
 void intel_color_commit_noarm(const struct intel_crtc_state *crtc_state);
 void intel_color_commit_arm(const struct intel_crtc_state *crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
index 6792a9056f46..2d9fc7383bfc 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc.c
@@ -365,8 +365,7 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
 						BIT(DRM_SCALING_FILTER_DEFAULT) |
 						BIT(DRM_SCALING_FILTER_NEAREST_NEIGHBOR));
 
-	intel_color_init(crtc);
-
+	intel_crtc_color_init(crtc);
 	intel_crtc_drrs_init(crtc);
 	intel_crtc_crc_init(crtc);
 
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index eb8eaeb19881..a103413cb081 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8326,6 +8326,7 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
 	if (!HAS_DISPLAY(dev_priv))
 		return;
 
+	intel_color_init_hooks(dev_priv);
 	intel_init_cdclk_hooks(dev_priv);
 	intel_audio_hooks_init(dev_priv);
 
-- 
2.35.1


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

* [Intel-gfx] [PATCH 03/10] drm/i915: Simplify the intel_color_init_hooks() if ladder
  2022-09-29  7:15 [Intel-gfx] [PATCH 00/10] drm/i915: Prep work for finishing (de)gamma readout Ville Syrjala
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 01/10] drm/i915: Remove PLL asserts from .load_luts() Ville Syrjala
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 02/10] drm/i915: Split up intel_color_init() Ville Syrjala
@ 2022-09-29  7:15 ` Ville Syrjala
  2022-09-29 11:56   ` Jani Nikula
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 04/10] drm/i915: Clean up intel_color_init_hooks() Ville Syrjala
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjala @ 2022-09-29  7:15 UTC (permalink / raw)
  To: intel-gfx

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

Get rid of the funny hsw vs. ivb extra indentation level in
intel_color_init_hooks().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index ddfe7c257a72..ce8a4be9b292 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -2238,12 +2238,11 @@ void intel_color_init_hooks(struct drm_i915_private *dev_priv)
 			dev_priv->display.funcs.color = &skl_color_funcs;
 		else if (DISPLAY_VER(dev_priv) == 8)
 			dev_priv->display.funcs.color = &bdw_color_funcs;
-		else if (DISPLAY_VER(dev_priv) == 7) {
-			if (IS_HASWELL(dev_priv))
-				dev_priv->display.funcs.color = &hsw_color_funcs;
-			else
-				dev_priv->display.funcs.color = &ivb_color_funcs;
-		} else
+		else if (IS_HASWELL(dev_priv))
+			dev_priv->display.funcs.color = &hsw_color_funcs;
+		else if (DISPLAY_VER(dev_priv) == 7)
+			dev_priv->display.funcs.color = &ivb_color_funcs;
+		else
 			dev_priv->display.funcs.color = &ilk_color_funcs;
 	}
 }
-- 
2.35.1


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

* [Intel-gfx] [PATCH 04/10] drm/i915: Clean up intel_color_init_hooks()
  2022-09-29  7:15 [Intel-gfx] [PATCH 00/10] drm/i915: Prep work for finishing (de)gamma readout Ville Syrjala
                   ` (2 preceding siblings ...)
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 03/10] drm/i915: Simplify the intel_color_init_hooks() if ladder Ville Syrjala
@ 2022-09-29  7:15 ` Ville Syrjala
  2022-09-29 11:57   ` Jani Nikula
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 05/10] drm/i915: Change glk_load_degamma_lut() calling convention Ville Syrjala
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjala @ 2022-09-29  7:15 UTC (permalink / raw)
  To: intel-gfx

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

Remove a bunch of pointless curly brackets and do
the s/dev_priv/i915/ while at it.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 43 +++++++++++-----------
 1 file changed, 21 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index ce8a4be9b292..96687ec30b19 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -2219,30 +2219,29 @@ void intel_crtc_color_init(struct intel_crtc *crtc)
 				   INTEL_INFO(dev_priv)->display.color.gamma_lut_size);
 }
 
-void intel_color_init_hooks(struct drm_i915_private *dev_priv)
+void intel_color_init_hooks(struct drm_i915_private *i915)
 {
-	if (HAS_GMCH(dev_priv)) {
-		if (IS_CHERRYVIEW(dev_priv)) {
-			dev_priv->display.funcs.color = &chv_color_funcs;
-		} else if (DISPLAY_VER(dev_priv) >= 4) {
-			dev_priv->display.funcs.color = &i965_color_funcs;
-		} else {
-			dev_priv->display.funcs.color = &i9xx_color_funcs;
-		}
+	if (HAS_GMCH(i915)) {
+		if (IS_CHERRYVIEW(i915))
+			i915->display.funcs.color = &chv_color_funcs;
+		else if (DISPLAY_VER(i915) >= 4)
+			i915->display.funcs.color = &i965_color_funcs;
+		else
+			i915->display.funcs.color = &i9xx_color_funcs;
 	} else {
-		if (DISPLAY_VER(dev_priv) >= 11)
-			dev_priv->display.funcs.color = &icl_color_funcs;
-		else if (DISPLAY_VER(dev_priv) == 10)
-			dev_priv->display.funcs.color = &glk_color_funcs;
-		else if (DISPLAY_VER(dev_priv) == 9)
-			dev_priv->display.funcs.color = &skl_color_funcs;
-		else if (DISPLAY_VER(dev_priv) == 8)
-			dev_priv->display.funcs.color = &bdw_color_funcs;
-		else if (IS_HASWELL(dev_priv))
-			dev_priv->display.funcs.color = &hsw_color_funcs;
-		else if (DISPLAY_VER(dev_priv) == 7)
-			dev_priv->display.funcs.color = &ivb_color_funcs;
+		if (DISPLAY_VER(i915) >= 11)
+			i915->display.funcs.color = &icl_color_funcs;
+		else if (DISPLAY_VER(i915) == 10)
+			i915->display.funcs.color = &glk_color_funcs;
+		else if (DISPLAY_VER(i915) == 9)
+			i915->display.funcs.color = &skl_color_funcs;
+		else if (DISPLAY_VER(i915) == 8)
+			i915->display.funcs.color = &bdw_color_funcs;
+		else if (IS_HASWELL(i915))
+			i915->display.funcs.color = &hsw_color_funcs;
+		else if (DISPLAY_VER(i915) == 7)
+			i915->display.funcs.color = &ivb_color_funcs;
 		else
-			dev_priv->display.funcs.color = &ilk_color_funcs;
+			i915->display.funcs.color = &ilk_color_funcs;
 	}
 }
-- 
2.35.1


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

* [Intel-gfx] [PATCH 05/10] drm/i915: Change glk_load_degamma_lut() calling convention
  2022-09-29  7:15 [Intel-gfx] [PATCH 00/10] drm/i915: Prep work for finishing (de)gamma readout Ville Syrjala
                   ` (3 preceding siblings ...)
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 04/10] drm/i915: Clean up intel_color_init_hooks() Ville Syrjala
@ 2022-09-29  7:15 ` Ville Syrjala
  2022-09-29 11:58   ` Jani Nikula
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 06/10] drm/i915: Make ilk_load_luts() deal with degamma Ville Syrjala
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjala @ 2022-09-29  7:15 UTC (permalink / raw)
  To: intel-gfx

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

Make glk_load_degamma_lut() more like most everyone else and
pass in the LUT explicitly.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 96687ec30b19..0c73b2ba1283 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -826,13 +826,14 @@ static int glk_degamma_lut_size(struct drm_i915_private *i915)
 		return 35;
 }
 
-static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state)
+static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state,
+				 const struct drm_property_blob *blob)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct drm_color_lut *lut = blob->data;
+	int i, lut_size = drm_color_lut_size(blob);
 	enum pipe pipe = crtc->pipe;
-	int i, lut_size = INTEL_INFO(dev_priv)->display.color.degamma_lut_size;
-	const struct drm_color_lut *lut = crtc_state->hw.degamma_lut->data;
 
 	/*
 	 * When setting the auto-increment bit, the hardware seems to
@@ -899,6 +900,7 @@ static void glk_load_degamma_lut_linear(const struct intel_crtc_state *crtc_stat
 
 static void glk_load_luts(const struct intel_crtc_state *crtc_state)
 {
+	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
 	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 
@@ -910,8 +912,8 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
 	 * the degama LUT so that we don't have to reload
 	 * it every time the pipe CSC is being enabled.
 	 */
-	if (crtc_state->hw.degamma_lut)
-		glk_load_degamma_lut(crtc_state);
+	if (degamma_lut)
+		glk_load_degamma_lut(crtc_state, degamma_lut);
 	else
 		glk_load_degamma_lut_linear(crtc_state);
 
@@ -1043,11 +1045,12 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
 
 static void icl_load_luts(const struct intel_crtc_state *crtc_state)
 {
+	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
 	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 
-	if (crtc_state->hw.degamma_lut)
-		glk_load_degamma_lut(crtc_state);
+	if (degamma_lut)
+		glk_load_degamma_lut(crtc_state, degamma_lut);
 
 	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
 	case GAMMA_MODE_MODE_8BIT:
-- 
2.35.1


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

* [Intel-gfx] [PATCH 06/10] drm/i915: Make ilk_load_luts() deal with degamma
  2022-09-29  7:15 [Intel-gfx] [PATCH 00/10] drm/i915: Prep work for finishing (de)gamma readout Ville Syrjala
                   ` (4 preceding siblings ...)
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 05/10] drm/i915: Change glk_load_degamma_lut() calling convention Ville Syrjala
@ 2022-09-29  7:15 ` Ville Syrjala
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 07/10] drm/i915: Introduce crtc_state->{pre, post}_csc_lut Ville Syrjala
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 26+ messages in thread
From: Ville Syrjala @ 2022-09-29  7:15 UTC (permalink / raw)
  To: intel-gfx

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

Make ilk_load_luts() ready for a degamma lut. Currently we never
have one, but soon we may get one from readout, and I think we
may want to change the state computation such that we may end up
with one even when userspace has simply supplied a gamma lut.

At least the code now follows the path laid out by the ivb/bdw
counterpars.

Reviewed-by: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 0c73b2ba1283..abaa1abab64d 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -649,13 +649,15 @@ static void ilk_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
+	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
+	const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
 
 	switch (crtc_state->gamma_mode) {
 	case GAMMA_MODE_MODE_8BIT:
-		ilk_load_lut_8(crtc, gamma_lut);
+		ilk_load_lut_8(crtc, blob);
 		break;
 	case GAMMA_MODE_MODE_10BIT:
-		ilk_load_lut_10(crtc, gamma_lut);
+		ilk_load_lut_10(crtc, blob);
 		break;
 	default:
 		MISSING_CASE(crtc_state->gamma_mode);
-- 
2.35.1


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

* [Intel-gfx] [PATCH 07/10] drm/i915: Introduce crtc_state->{pre, post}_csc_lut
  2022-09-29  7:15 [Intel-gfx] [PATCH 00/10] drm/i915: Prep work for finishing (de)gamma readout Ville Syrjala
                   ` (5 preceding siblings ...)
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 06/10] drm/i915: Make ilk_load_luts() deal with degamma Ville Syrjala
@ 2022-09-29  7:15 ` Ville Syrjala
  2022-10-19  7:06   ` Shankar, Uma
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 08/10] drm/i915: Assert {pre, post}_csc_lut were assigned sensibly Ville Syrjala
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjala @ 2022-09-29  7:15 UTC (permalink / raw)
  To: intel-gfx

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

Add an extra remapping step between the logical state of the LUTs
(hw.(de)gamma_lut) as specified via uapi/bigjoiner copy vs.
the actual state of the LUTs programmed into the hardware.

With this we should be finally able finish the (de)gamma
readout/state checker support for the remaining platforms
(ilk-skl) where the same hardware LUT can be positioned
either before or after the pipe CSC unit. Where we position
it depends on factors such as presence of the logical degamma
LUT, RGB vs. YCbCr output, full vs. limited RGB quantization
range.

Without the extra remapping step the state readout doesn't
really know whether the LUT read from the hardware is the
degamma or gamma LUT, and so we is unable to accurately store
it into our crtc state. With the remapping step we know
exactly where to put it given the order of the LUT vs. CSC
in the hardware state.

Only the initial hw->uapi state readout done during driver
load/resume still has the problem of not really knowing
what to do with the LUT(s). But we can just assume 1:1
mapping there and let subsequent commits fix things up.

Another benefit is that we now have a place for purely
internal LUTs, without complicating the bigjoiner uapi->hw
copy logic. This should prove useful for streamlining
glk degamma LUT handling.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_atomic.c   |   8 +
 drivers/gpu/drm/i915/display/intel_color.c    | 141 +++++++++++-------
 .../drm/i915/display/intel_crtc_state_dump.c  |  10 +-
 drivers/gpu/drm/i915/display/intel_display.c  |   2 +-
 .../drm/i915/display/intel_display_types.h    |   4 +
 .../drm/i915/display/intel_modeset_setup.c    |   6 +
 6 files changed, 114 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c b/drivers/gpu/drm/i915/display/intel_atomic.c
index 18f0a5ae3bac..6621aa245caf 100644
--- a/drivers/gpu/drm/i915/display/intel_atomic.c
+++ b/drivers/gpu/drm/i915/display/intel_atomic.c
@@ -252,6 +252,11 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
 	if (crtc_state->hw.gamma_lut)
 		drm_property_blob_get(crtc_state->hw.gamma_lut);
 
+	if (crtc_state->pre_csc_lut)
+		drm_property_blob_get(crtc_state->pre_csc_lut);
+	if (crtc_state->post_csc_lut)
+		drm_property_blob_get(crtc_state->post_csc_lut);
+
 	crtc_state->update_pipe = false;
 	crtc_state->disable_lp_wm = false;
 	crtc_state->disable_cxsr = false;
@@ -274,6 +279,9 @@ static void intel_crtc_put_color_blobs(struct intel_crtc_state *crtc_state)
 	drm_property_blob_put(crtc_state->hw.degamma_lut);
 	drm_property_blob_put(crtc_state->hw.gamma_lut);
 	drm_property_blob_put(crtc_state->hw.ctm);
+
+	drm_property_blob_put(crtc_state->pre_csc_lut);
+	drm_property_blob_put(crtc_state->post_csc_lut);
 }
 
 void intel_crtc_free_hw_state(struct intel_crtc_state *crtc_state)
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index abaa1abab64d..380f44720fe6 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -578,9 +578,9 @@ static void i9xx_load_lut_8(struct intel_crtc *crtc,
 static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
+	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
 
-	i9xx_load_lut_8(crtc, gamma_lut);
+	i9xx_load_lut_8(crtc, post_csc_lut);
 }
 
 static void i965_load_lut_10p6(struct intel_crtc *crtc,
@@ -606,12 +606,12 @@ static void i965_load_lut_10p6(struct intel_crtc *crtc,
 static void i965_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
+	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		i9xx_load_lut_8(crtc, gamma_lut);
+		i9xx_load_lut_8(crtc, post_csc_lut);
 	else
-		i965_load_lut_10p6(crtc, gamma_lut);
+		i965_load_lut_10p6(crtc, post_csc_lut);
 }
 
 static void ilk_load_lut_8(struct intel_crtc *crtc,
@@ -648,9 +648,9 @@ static void ilk_load_lut_10(struct intel_crtc *crtc,
 static void ilk_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
-	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
-	const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
+	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
+	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
+	const struct drm_property_blob *blob = post_csc_lut ?: pre_csc_lut;
 
 	switch (crtc_state->gamma_mode) {
 	case GAMMA_MODE_MODE_8BIT:
@@ -764,19 +764,19 @@ static void ivb_load_lut_ext_max(const struct intel_crtc_state *crtc_state)
 static void ivb_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
-	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
-	const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
+	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
+	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
+	const struct drm_property_blob *blob = post_csc_lut ?: pre_csc_lut;
 
 	switch (crtc_state->gamma_mode) {
 	case GAMMA_MODE_MODE_8BIT:
 		ilk_load_lut_8(crtc, blob);
 		break;
 	case GAMMA_MODE_MODE_SPLIT:
-		ivb_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
+		ivb_load_lut_10(crtc, pre_csc_lut, PAL_PREC_SPLIT_MODE |
 				PAL_PREC_INDEX_VALUE(0));
 		ivb_load_lut_ext_max(crtc_state);
-		ivb_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
+		ivb_load_lut_10(crtc, post_csc_lut, PAL_PREC_SPLIT_MODE |
 				PAL_PREC_INDEX_VALUE(512));
 		break;
 	case GAMMA_MODE_MODE_10BIT:
@@ -793,19 +793,19 @@ static void ivb_load_luts(const struct intel_crtc_state *crtc_state)
 static void bdw_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
-	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
-	const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
+	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
+	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
+	const struct drm_property_blob *blob = post_csc_lut ?: pre_csc_lut;
 
 	switch (crtc_state->gamma_mode) {
 	case GAMMA_MODE_MODE_8BIT:
 		ilk_load_lut_8(crtc, blob);
 		break;
 	case GAMMA_MODE_MODE_SPLIT:
-		bdw_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
+		bdw_load_lut_10(crtc, pre_csc_lut, PAL_PREC_SPLIT_MODE |
 				PAL_PREC_INDEX_VALUE(0));
 		ivb_load_lut_ext_max(crtc_state);
-		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
+		bdw_load_lut_10(crtc, post_csc_lut, PAL_PREC_SPLIT_MODE |
 				PAL_PREC_INDEX_VALUE(512));
 		break;
 	case GAMMA_MODE_MODE_10BIT:
@@ -902,8 +902,8 @@ static void glk_load_degamma_lut_linear(const struct intel_crtc_state *crtc_stat
 
 static void glk_load_luts(const struct intel_crtc_state *crtc_state)
 {
-	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
-	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
+	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
+	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 
 	/*
@@ -914,17 +914,17 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
 	 * the degama LUT so that we don't have to reload
 	 * it every time the pipe CSC is being enabled.
 	 */
-	if (degamma_lut)
-		glk_load_degamma_lut(crtc_state, degamma_lut);
+	if (pre_csc_lut)
+		glk_load_degamma_lut(crtc_state, pre_csc_lut);
 	else
 		glk_load_degamma_lut_linear(crtc_state);
 
 	switch (crtc_state->gamma_mode) {
 	case GAMMA_MODE_MODE_8BIT:
-		ilk_load_lut_8(crtc, gamma_lut);
+		ilk_load_lut_8(crtc, post_csc_lut);
 		break;
 	case GAMMA_MODE_MODE_10BIT:
-		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
+		bdw_load_lut_10(crtc, post_csc_lut, PAL_PREC_INDEX_VALUE(0));
 		ivb_load_lut_ext_max(crtc_state);
 		break;
 	default:
@@ -964,7 +964,7 @@ static void
 icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	const struct drm_property_blob *blob = crtc_state->hw.gamma_lut;
+	const struct drm_property_blob *blob = crtc_state->post_csc_lut;
 	const struct drm_color_lut *lut = blob->data;
 	enum pipe pipe = crtc->pipe;
 	int i;
@@ -993,7 +993,7 @@ static void
 icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	const struct drm_property_blob *blob = crtc_state->hw.gamma_lut;
+	const struct drm_property_blob *blob = crtc_state->post_csc_lut;
 	const struct drm_color_lut *lut = blob->data;
 	const struct drm_color_lut *entry;
 	enum pipe pipe = crtc->pipe;
@@ -1047,23 +1047,23 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
 
 static void icl_load_luts(const struct intel_crtc_state *crtc_state)
 {
-	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
-	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
+	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
+	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 
-	if (degamma_lut)
-		glk_load_degamma_lut(crtc_state, degamma_lut);
+	if (pre_csc_lut)
+		glk_load_degamma_lut(crtc_state, pre_csc_lut);
 
 	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
 	case GAMMA_MODE_MODE_8BIT:
-		ilk_load_lut_8(crtc, gamma_lut);
+		ilk_load_lut_8(crtc, post_csc_lut);
 		break;
 	case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
 		icl_program_gamma_superfine_segment(crtc_state);
 		icl_program_gamma_multi_segment(crtc_state);
 		break;
 	case GAMMA_MODE_MODE_10BIT:
-		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
+		bdw_load_lut_10(crtc, post_csc_lut, PAL_PREC_INDEX_VALUE(0));
 		ivb_load_lut_ext_max(crtc_state);
 		break;
 	default:
@@ -1139,18 +1139,18 @@ static void chv_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
-	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
+	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
+	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
 	const struct drm_property_blob *ctm = crtc_state->hw.ctm;
 
 	if (crtc_state->cgm_mode & CGM_PIPE_MODE_CSC)
 		chv_load_cgm_csc(crtc, ctm);
 
 	if (crtc_state->cgm_mode & CGM_PIPE_MODE_DEGAMMA)
-		chv_load_cgm_degamma(crtc, degamma_lut);
+		chv_load_cgm_degamma(crtc, pre_csc_lut);
 
 	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
-		chv_load_cgm_gamma(crtc, gamma_lut);
+		chv_load_cgm_gamma(crtc, post_csc_lut);
 	else
 		i965_load_luts(crtc_state);
 
@@ -1188,8 +1188,8 @@ static bool intel_can_preload_luts(const struct intel_crtc_state *new_crtc_state
 	const struct intel_crtc_state *old_crtc_state =
 		intel_atomic_get_old_crtc_state(state, crtc);
 
-	return !old_crtc_state->hw.gamma_lut &&
-		!old_crtc_state->hw.degamma_lut;
+	return !old_crtc_state->post_csc_lut &&
+		!old_crtc_state->pre_csc_lut;
 }
 
 static bool chv_can_preload_luts(const struct intel_crtc_state *new_crtc_state)
@@ -1208,7 +1208,7 @@ static bool chv_can_preload_luts(const struct intel_crtc_state *new_crtc_state)
 	if (old_crtc_state->cgm_mode || new_crtc_state->cgm_mode)
 		return false;
 
-	return !old_crtc_state->hw.gamma_lut;
+	return !old_crtc_state->post_csc_lut;
 }
 
 static bool glk_can_preload_luts(const struct intel_crtc_state *new_crtc_state)
@@ -1226,7 +1226,7 @@ static bool glk_can_preload_luts(const struct intel_crtc_state *new_crtc_state)
 	 * linear hardware degamma mid scanout.
 	 */
 	return !old_crtc_state->csc_enable &&
-		!old_crtc_state->hw.gamma_lut;
+		!old_crtc_state->post_csc_lut;
 }
 
 int intel_color_check(struct intel_crtc_state *crtc_state)
@@ -1355,6 +1355,14 @@ static u32 i9xx_gamma_mode(struct intel_crtc_state *crtc_state)
 		return GAMMA_MODE_MODE_10BIT; /* i965+ only */
 }
 
+static void intel_assign_luts(struct intel_crtc_state *crtc_state)
+{
+	drm_property_replace_blob(&crtc_state->pre_csc_lut,
+				  crtc_state->hw.degamma_lut);
+	drm_property_replace_blob(&crtc_state->post_csc_lut,
+				  crtc_state->hw.gamma_lut);
+}
+
 static int i9xx_color_check(struct intel_crtc_state *crtc_state)
 {
 	int ret;
@@ -1373,6 +1381,8 @@ static int i9xx_color_check(struct intel_crtc_state *crtc_state)
 	if (ret)
 		return ret;
 
+	intel_assign_luts(crtc_state);
+
 	crtc_state->preload_luts = intel_can_preload_luts(crtc_state);
 
 	return 0;
@@ -1427,6 +1437,8 @@ static int chv_color_check(struct intel_crtc_state *crtc_state)
 	if (ret)
 		return ret;
 
+	intel_assign_luts(crtc_state);
+
 	crtc_state->preload_luts = chv_can_preload_luts(crtc_state);
 
 	return 0;
@@ -1452,10 +1464,29 @@ static u32 ilk_csc_mode(const struct intel_crtc_state *crtc_state)
 	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB)
 		return CSC_BLACK_SCREEN_OFFSET;
 
+	if (crtc_state->hw.degamma_lut)
+		return CSC_MODE_YUV_TO_RGB;
+
 	return CSC_MODE_YUV_TO_RGB |
 		CSC_POSITION_BEFORE_GAMMA;
 }
 
+static void ilk_assign_luts(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->hw.degamma_lut ||
+	    crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) {
+		drm_property_replace_blob(&crtc_state->pre_csc_lut,
+					  crtc_state->hw.degamma_lut);
+		drm_property_replace_blob(&crtc_state->post_csc_lut,
+					  crtc_state->hw.gamma_lut);
+	} else {
+		drm_property_replace_blob(&crtc_state->pre_csc_lut,
+					  crtc_state->hw.gamma_lut);
+		drm_property_replace_blob(&crtc_state->post_csc_lut,
+					  NULL);
+	}
+}
+
 static int ilk_color_check(struct intel_crtc_state *crtc_state)
 {
 	int ret;
@@ -1483,6 +1514,8 @@ static int ilk_color_check(struct intel_crtc_state *crtc_state)
 	if (ret)
 		return ret;
 
+	ilk_assign_luts(crtc_state);
+
 	crtc_state->preload_luts = intel_can_preload_luts(crtc_state);
 
 	return 0;
@@ -1550,6 +1583,8 @@ static int ivb_color_check(struct intel_crtc_state *crtc_state)
 	if (ret)
 		return ret;
 
+	ilk_assign_luts(crtc_state);
+
 	crtc_state->preload_luts = intel_can_preload_luts(crtc_state);
 
 	return 0;
@@ -1598,6 +1633,8 @@ static int glk_color_check(struct intel_crtc_state *crtc_state)
 	if (ret)
 		return ret;
 
+	intel_assign_luts(crtc_state);
+
 	crtc_state->preload_luts = glk_can_preload_luts(crtc_state);
 
 	return 0;
@@ -1658,6 +1695,8 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
 
 	crtc_state->csc_mode = icl_csc_mode(crtc_state);
 
+	intel_assign_luts(crtc_state);
+
 	crtc_state->preload_luts = intel_can_preload_luts(crtc_state);
 
 	return 0;
@@ -1863,7 +1902,7 @@ static void i9xx_read_luts(struct intel_crtc_state *crtc_state)
 	if (!crtc_state->gamma_enable)
 		return;
 
-	crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc);
+	crtc_state->post_csc_lut = i9xx_read_lut_8(crtc);
 }
 
 static struct drm_property_blob *i965_read_lut_10p6(struct intel_crtc *crtc)
@@ -1904,9 +1943,9 @@ static void i965_read_luts(struct intel_crtc_state *crtc_state)
 		return;
 
 	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
-		crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc);
+		crtc_state->post_csc_lut = i9xx_read_lut_8(crtc);
 	else
-		crtc_state->hw.gamma_lut = i965_read_lut_10p6(crtc);
+		crtc_state->post_csc_lut = i965_read_lut_10p6(crtc);
 }
 
 static struct drm_property_blob *chv_read_cgm_gamma(struct intel_crtc *crtc)
@@ -1940,7 +1979,7 @@ static void chv_read_luts(struct intel_crtc_state *crtc_state)
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 
 	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
-		crtc_state->hw.gamma_lut = chv_read_cgm_gamma(crtc);
+		crtc_state->post_csc_lut = chv_read_cgm_gamma(crtc);
 	else
 		i965_read_luts(crtc_state);
 }
@@ -2007,10 +2046,10 @@ static void ilk_read_luts(struct intel_crtc_state *crtc_state)
 
 	switch (crtc_state->gamma_mode) {
 	case GAMMA_MODE_MODE_8BIT:
-		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc);
+		crtc_state->post_csc_lut = ilk_read_lut_8(crtc);
 		break;
 	case GAMMA_MODE_MODE_10BIT:
-		crtc_state->hw.gamma_lut = ilk_read_lut_10(crtc);
+		crtc_state->post_csc_lut = ilk_read_lut_10(crtc);
 		break;
 	default:
 		MISSING_CASE(crtc_state->gamma_mode);
@@ -2062,10 +2101,10 @@ static void glk_read_luts(struct intel_crtc_state *crtc_state)
 
 	switch (crtc_state->gamma_mode) {
 	case GAMMA_MODE_MODE_8BIT:
-		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc);
+		crtc_state->post_csc_lut = ilk_read_lut_8(crtc);
 		break;
 	case GAMMA_MODE_MODE_10BIT:
-		crtc_state->hw.gamma_lut = bdw_read_lut_10(crtc, PAL_PREC_INDEX_VALUE(0));
+		crtc_state->post_csc_lut = bdw_read_lut_10(crtc, PAL_PREC_INDEX_VALUE(0));
 		break;
 	default:
 		MISSING_CASE(crtc_state->gamma_mode);
@@ -2120,13 +2159,13 @@ static void icl_read_luts(struct intel_crtc_state *crtc_state)
 
 	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
 	case GAMMA_MODE_MODE_8BIT:
-		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc);
+		crtc_state->post_csc_lut = ilk_read_lut_8(crtc);
 		break;
 	case GAMMA_MODE_MODE_10BIT:
-		crtc_state->hw.gamma_lut = bdw_read_lut_10(crtc, PAL_PREC_INDEX_VALUE(0));
+		crtc_state->post_csc_lut = bdw_read_lut_10(crtc, PAL_PREC_INDEX_VALUE(0));
 		break;
 	case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
-		crtc_state->hw.gamma_lut = icl_read_lut_multi_segment(crtc);
+		crtc_state->post_csc_lut = icl_read_lut_multi_segment(crtc);
 		break;
 	default:
 		MISSING_CASE(crtc_state->gamma_mode);
diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
index e9212f69c360..98e36ab55e9e 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -298,11 +298,11 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
 			    pipe_config->csc_mode, pipe_config->gamma_mode,
 			    pipe_config->gamma_enable, pipe_config->csc_enable);
 
-	drm_dbg_kms(&i915->drm, "degamma lut: %d entries, gamma lut: %d entries\n",
-		    pipe_config->hw.degamma_lut ?
-		    drm_color_lut_size(pipe_config->hw.degamma_lut) : 0,
-		    pipe_config->hw.gamma_lut ?
-		    drm_color_lut_size(pipe_config->hw.gamma_lut) : 0);
+	drm_dbg_kms(&i915->drm, "pre csc lut: %d entries, post csc lut: %d entries\n",
+		    pipe_config->pre_csc_lut ?
+		    drm_color_lut_size(pipe_config->pre_csc_lut) : 0,
+		    pipe_config->post_csc_lut ?
+		    drm_color_lut_size(pipe_config->post_csc_lut) : 0);
 
 dump_planes:
 	if (!state)
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index a103413cb081..20569b6838d1 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5815,7 +5815,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 
 		bp_gamma = intel_color_get_gamma_bit_precision(pipe_config);
 		if (bp_gamma)
-			PIPE_CONF_CHECK_COLOR_LUT(gamma_mode, hw.gamma_lut, bp_gamma);
+			PIPE_CONF_CHECK_COLOR_LUT(gamma_mode, post_csc_lut, bp_gamma);
 
 		if (current_config->active_planes) {
 			PIPE_CONF_CHECK_BOOL(has_psr);
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index e2b853e9e51d..cd76236d5fa3 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1001,11 +1001,15 @@ struct intel_crtc_state {
 	 */
 	struct {
 		bool active, enable;
+		/* logical state of LUTs */
 		struct drm_property_blob *degamma_lut, *gamma_lut, *ctm;
 		struct drm_display_mode mode, pipe_mode, adjusted_mode;
 		enum drm_scaling_filter scaling_filter;
 	} hw;
 
+	/* actual state of LUTs */
+	struct drm_property_blob *pre_csc_lut, *post_csc_lut;
+
 	/**
 	 * quirks - bitfield with hw state readout quirks
 	 *
diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
index cbfabd58b75a..55ccdafefd27 100644
--- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
+++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
@@ -155,6 +155,12 @@ static void intel_crtc_copy_hw_to_uapi_state(struct intel_crtc_state *crtc_state
 	crtc_state->uapi.adjusted_mode = crtc_state->hw.adjusted_mode;
 	crtc_state->uapi.scaling_filter = crtc_state->hw.scaling_filter;
 
+	/* assume 1:1 mapping */
+	drm_property_replace_blob(&crtc_state->hw.degamma_lut,
+				  crtc_state->pre_csc_lut);
+	drm_property_replace_blob(&crtc_state->hw.gamma_lut,
+				  crtc_state->post_csc_lut);
+
 	drm_property_replace_blob(&crtc_state->uapi.degamma_lut,
 				  crtc_state->hw.degamma_lut);
 	drm_property_replace_blob(&crtc_state->uapi.gamma_lut,
-- 
2.35.1


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

* [Intel-gfx] [PATCH 08/10] drm/i915: Assert {pre, post}_csc_lut were assigned sensibly
  2022-09-29  7:15 [Intel-gfx] [PATCH 00/10] drm/i915: Prep work for finishing (de)gamma readout Ville Syrjala
                   ` (6 preceding siblings ...)
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 07/10] drm/i915: Introduce crtc_state->{pre, post}_csc_lut Ville Syrjala
@ 2022-09-29  7:15 ` Ville Syrjala
  2022-10-19  7:09   ` Shankar, Uma
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 09/10] drm/i915: Get rid of glk_load_degamma_lut_linear() Ville Syrjala
                   ` (4 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjala @ 2022-09-29  7:15 UTC (permalink / raw)
  To: intel-gfx

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

Since we now have the extra step from hw.(de)gamma_lut into
{pre,post}_csc_lut let's make sure we didn't forget to assign
them appropriately. Ie. basically making sure intel_color_check()
was called when necessary (and that it did its job suitable well).

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c   | 20 ++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_color.h   |  1 +
 drivers/gpu/drm/i915/display/intel_display.c |  2 ++
 3 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 380f44720fe6..575d2a23682a 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1355,6 +1355,26 @@ static u32 i9xx_gamma_mode(struct intel_crtc_state *crtc_state)
 		return GAMMA_MODE_MODE_10BIT; /* i965+ only */
 }
 
+void intel_color_assert_luts(const struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+
+	/* make sure {pre,post}_csc_lut were correctly assigned */
+	if (DISPLAY_VER(i915) >= 10 || HAS_GMCH(i915)) {
+		drm_WARN_ON(&i915->drm,
+			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut);
+		drm_WARN_ON(&i915->drm,
+			    crtc_state->post_csc_lut != crtc_state->hw.gamma_lut);
+	} else {
+		drm_WARN_ON(&i915->drm,
+			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut &&
+			    crtc_state->pre_csc_lut != crtc_state->hw.gamma_lut);
+		drm_WARN_ON(&i915->drm,
+			    crtc_state->post_csc_lut != crtc_state->hw.degamma_lut &&
+			    crtc_state->post_csc_lut != crtc_state->hw.gamma_lut);
+	}
+}
+
 static void intel_assign_luts(struct intel_crtc_state *crtc_state)
 {
 	drm_property_replace_blob(&crtc_state->pre_csc_lut,
diff --git a/drivers/gpu/drm/i915/display/intel_color.h b/drivers/gpu/drm/i915/display/intel_color.h
index 67702451e2fd..b76f18e6c452 100644
--- a/drivers/gpu/drm/i915/display/intel_color.h
+++ b/drivers/gpu/drm/i915/display/intel_color.h
@@ -24,5 +24,6 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat
 bool intel_color_lut_equal(struct drm_property_blob *blob1,
 			   struct drm_property_blob *blob2,
 			   u32 gamma_mode, u32 bit_precision);
+void intel_color_assert_luts(const struct intel_crtc_state *crtc_state);
 
 #endif /* __INTEL_COLOR_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 20569b6838d1..441811ac0ab0 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -6906,6 +6906,8 @@ static int intel_atomic_check(struct drm_device *dev,
 
 	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
 					    new_crtc_state, i) {
+		intel_color_assert_luts(new_crtc_state);
+
 		ret = intel_async_flip_check_hw(state, crtc);
 		if (ret)
 			goto fail;
-- 
2.35.1


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

* [Intel-gfx] [PATCH 09/10] drm/i915: Get rid of glk_load_degamma_lut_linear()
  2022-09-29  7:15 [Intel-gfx] [PATCH 00/10] drm/i915: Prep work for finishing (de)gamma readout Ville Syrjala
                   ` (7 preceding siblings ...)
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 08/10] drm/i915: Assert {pre, post}_csc_lut were assigned sensibly Ville Syrjala
@ 2022-09-29  7:15 ` Ville Syrjala
  2022-10-19  7:20   ` Shankar, Uma
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 10/10] drm/i915: Stop loading linear degammma LUT on glk needlessly Ville Syrjala
                   ` (3 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjala @ 2022-09-29  7:15 UTC (permalink / raw)
  To: intel-gfx

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

Since we now have a place (pre_csc_lut) to stuff a purely
internal LUT we can replace glk_load_degamma_lut_linear()
with such a thing and just rely on the normal
glk_load_degamma_lut() to load it as well.

drm_mode_config_cleanup() will clean this up for us.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c    | 110 +++++++++++-------
 drivers/gpu/drm/i915/display/intel_color.h    |   1 +
 drivers/gpu/drm/i915/display/intel_display.c  |   4 +
 .../gpu/drm/i915/display/intel_display_core.h |   5 +
 4 files changed, 79 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 575d2a23682a..de530bf1aba1 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -557,6 +557,32 @@ static void skl_color_commit_arm(const struct intel_crtc_state *crtc_state)
 			  crtc_state->csc_mode);
 }
 
+static struct drm_property_blob *
+create_linear_lut(struct drm_i915_private *i915, int lut_size)
+{
+	struct drm_property_blob *blob;
+	struct drm_color_lut *lut;
+	int i;
+
+	blob = drm_property_create_blob(&i915->drm,
+					sizeof(struct drm_color_lut) * lut_size,
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	lut = blob->data;
+
+	for (i = 0; i < lut_size; i++) {
+		u16 val = 0xffff * i / (lut_size - 1);
+
+		lut[i].red = val;
+		lut[i].green = val;
+		lut[i].blue = val;
+	}
+
+	return blob;
+}
+
 static void i9xx_load_lut_8(struct intel_crtc *crtc,
 			    const struct drm_property_blob *blob)
 {
@@ -871,53 +897,14 @@ static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state,
 	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
 }
 
-static void glk_load_degamma_lut_linear(const struct intel_crtc_state *crtc_state)
-{
-	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	enum pipe pipe = crtc->pipe;
-	int i, lut_size = INTEL_INFO(dev_priv)->display.color.degamma_lut_size;
-
-	/*
-	 * When setting the auto-increment bit, the hardware seems to
-	 * ignore the index bits, so we need to reset it to index 0
-	 * separately.
-	 */
-	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
-	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
-			  PRE_CSC_GAMC_AUTO_INCREMENT);
-
-	for (i = 0; i < lut_size; i++) {
-		u32 v = (i << 16) / (lut_size - 1);
-
-		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe), v);
-	}
-
-	/* Clamp values > 1.0. */
-	while (i++ < 35)
-		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe), 1 << 16);
-
-	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
-}
-
 static void glk_load_luts(const struct intel_crtc_state *crtc_state)
 {
 	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
 	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
 
-	/*
-	 * On GLK+ both pipe CSC and degamma LUT are controlled
-	 * by csc_enable. Hence for the cases where the CSC is
-	 * needed but degamma LUT is not we need to load a
-	 * linear degamma LUT. In fact we'll just always load
-	 * the degama LUT so that we don't have to reload
-	 * it every time the pipe CSC is being enabled.
-	 */
 	if (pre_csc_lut)
 		glk_load_degamma_lut(crtc_state, pre_csc_lut);
-	else
-		glk_load_degamma_lut_linear(crtc_state);
 
 	switch (crtc_state->gamma_mode) {
 	case GAMMA_MODE_MODE_8BIT:
@@ -1360,11 +1347,17 @@ void intel_color_assert_luts(const struct intel_crtc_state *crtc_state)
 	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
 
 	/* make sure {pre,post}_csc_lut were correctly assigned */
-	if (DISPLAY_VER(i915) >= 10 || HAS_GMCH(i915)) {
+	if (DISPLAY_VER(i915) >= 11 || HAS_GMCH(i915)) {
 		drm_WARN_ON(&i915->drm,
 			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut);
 		drm_WARN_ON(&i915->drm,
 			    crtc_state->post_csc_lut != crtc_state->hw.gamma_lut);
+	} else if (DISPLAY_VER(i915) == 10) {
+		drm_WARN_ON(&i915->drm,
+			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut &&
+			    crtc_state->pre_csc_lut != i915->display.color.glk_linear_degamma_lut);
+		drm_WARN_ON(&i915->drm,
+			    crtc_state->post_csc_lut != crtc_state->hw.gamma_lut);
 	} else {
 		drm_WARN_ON(&i915->drm,
 			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut &&
@@ -1619,6 +1612,25 @@ static u32 glk_gamma_mode(const struct intel_crtc_state *crtc_state)
 		return GAMMA_MODE_MODE_10BIT;
 }
 
+static void glk_assign_luts(struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
+
+	intel_assign_luts(crtc_state);
+
+	/*
+	 * On GLK+ both pipe CSC and degamma LUT are controlled
+	 * by csc_enable. Hence for the cases where the CSC is
+	 * needed but degamma LUT is not we need to load a
+	 * linear degamma LUT. In fact we'll just always load
+	 * the degama LUT so that we don't have to reload
+	 * it every time the pipe CSC is being enabled.
+	 */
+	if (!crtc_state->pre_csc_lut)
+		drm_property_replace_blob(&crtc_state->pre_csc_lut,
+					  i915->display.color.glk_linear_degamma_lut);
+}
+
 static int glk_color_check(struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
@@ -1653,7 +1665,7 @@ static int glk_color_check(struct intel_crtc_state *crtc_state)
 	if (ret)
 		return ret;
 
-	intel_assign_luts(crtc_state);
+	glk_assign_luts(crtc_state);
 
 	crtc_state->preload_luts = glk_can_preload_luts(crtc_state);
 
@@ -2283,6 +2295,22 @@ void intel_crtc_color_init(struct intel_crtc *crtc)
 				   INTEL_INFO(dev_priv)->display.color.gamma_lut_size);
 }
 
+int intel_color_init(struct drm_i915_private *i915)
+{
+	struct drm_property_blob *blob;
+
+	if (DISPLAY_VER(i915) != 10)
+		return 0;
+
+	blob = create_linear_lut(i915, INTEL_INFO(i915)->display.color.degamma_lut_size);
+	if (IS_ERR(blob))
+		return PTR_ERR(blob);
+
+	i915->display.color.glk_linear_degamma_lut = blob;
+
+	return 0;
+}
+
 void intel_color_init_hooks(struct drm_i915_private *i915)
 {
 	if (HAS_GMCH(i915)) {
diff --git a/drivers/gpu/drm/i915/display/intel_color.h b/drivers/gpu/drm/i915/display/intel_color.h
index b76f18e6c452..f7b0591abd73 100644
--- a/drivers/gpu/drm/i915/display/intel_color.h
+++ b/drivers/gpu/drm/i915/display/intel_color.h
@@ -14,6 +14,7 @@ struct drm_i915_private;
 struct drm_property_blob;
 
 void intel_color_init_hooks(struct drm_i915_private *i915);
+int intel_color_init(struct drm_i915_private *i915);
 void intel_crtc_color_init(struct intel_crtc *crtc);
 int intel_color_check(struct intel_crtc_state *crtc_state);
 void intel_color_commit_noarm(const struct intel_crtc_state *crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 441811ac0ab0..95cbc2e2d3c2 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8659,6 +8659,10 @@ int intel_modeset_init_noirq(struct drm_i915_private *i915)
 	if (ret)
 		goto cleanup_vga_client_pw_domain_dmc;
 
+	ret = intel_color_init(i915);
+	if (ret)
+		goto cleanup_vga_client_pw_domain_dmc;
+
 	ret = intel_dbuf_init(i915);
 	if (ret)
 		goto cleanup_vga_client_pw_domain_dmc;
diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
index 96cf994b0ad1..b4b9c4cef78e 100644
--- a/drivers/gpu/drm/i915/display/intel_display_core.h
+++ b/drivers/gpu/drm/i915/display/intel_display_core.h
@@ -28,6 +28,7 @@
 
 struct drm_i915_private;
 struct drm_property;
+struct drm_property_blob;
 struct i915_audio_component;
 struct i915_hdcp_comp_master;
 struct intel_atomic_state;
@@ -308,6 +309,10 @@ struct intel_display {
 		unsigned int max_cdclk_freq;
 	} cdclk;
 
+	struct {
+		struct drm_property_blob *glk_linear_degamma_lut;
+	} color;
+
 	struct {
 		/* The current hardware dbuf configuration */
 		u8 enabled_slices;
-- 
2.35.1


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

* [Intel-gfx] [PATCH 10/10] drm/i915: Stop loading linear degammma LUT on glk needlessly
  2022-09-29  7:15 [Intel-gfx] [PATCH 00/10] drm/i915: Prep work for finishing (de)gamma readout Ville Syrjala
                   ` (8 preceding siblings ...)
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 09/10] drm/i915: Get rid of glk_load_degamma_lut_linear() Ville Syrjala
@ 2022-09-29  7:15 ` Ville Syrjala
  2022-10-19  7:24   ` Shankar, Uma
  2022-09-29  9:19 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Prep work for finishing (de)gamma readout Patchwork
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjala @ 2022-09-29  7:15 UTC (permalink / raw)
  To: intel-gfx

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

Make glk_load_luts() a bit lighter for the common case
where neither the degamma LUT nor pipe CSC are enabled
by not loading the linear degamma LUT. Making .load_luts()
as lightweight as possible is a good idea since it may need
to execute from a vblank worker under tight deadlines.

My earlier reasoning for always loading the linear degamma LUT
was to avoid an extra LUT load when just enabling/disabling the
pipe CSC, but that is nonsense since we load the LUTs on every
flagged color manaement change/modeset anyway (either of which
is needed for a pipe CSC toggle).

We can also get rid of the glk_can_preload_luts() special
case since the presence of the degamma LUT will now always
match csc_enable.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 26 +++-------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index de530bf1aba1..a3066d942f58 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1198,24 +1198,6 @@ static bool chv_can_preload_luts(const struct intel_crtc_state *new_crtc_state)
 	return !old_crtc_state->post_csc_lut;
 }
 
-static bool glk_can_preload_luts(const struct intel_crtc_state *new_crtc_state)
-{
-	struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
-	struct intel_atomic_state *state =
-		to_intel_atomic_state(new_crtc_state->uapi.state);
-	const struct intel_crtc_state *old_crtc_state =
-		intel_atomic_get_old_crtc_state(state, crtc);
-
-	/*
-	 * The hardware degamma is active whenever the pipe
-	 * CSC is active. Thus even if the old state has no
-	 * software degamma we need to avoid clobbering the
-	 * linear hardware degamma mid scanout.
-	 */
-	return !old_crtc_state->csc_enable &&
-		!old_crtc_state->post_csc_lut;
-}
-
 int intel_color_check(struct intel_crtc_state *crtc_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
@@ -1622,11 +1604,9 @@ static void glk_assign_luts(struct intel_crtc_state *crtc_state)
 	 * On GLK+ both pipe CSC and degamma LUT are controlled
 	 * by csc_enable. Hence for the cases where the CSC is
 	 * needed but degamma LUT is not we need to load a
-	 * linear degamma LUT. In fact we'll just always load
-	 * the degama LUT so that we don't have to reload
-	 * it every time the pipe CSC is being enabled.
+	 * linear degamma LUT.
 	 */
-	if (!crtc_state->pre_csc_lut)
+	if (crtc_state->csc_enable && !crtc_state->pre_csc_lut)
 		drm_property_replace_blob(&crtc_state->pre_csc_lut,
 					  i915->display.color.glk_linear_degamma_lut);
 }
@@ -1667,7 +1647,7 @@ static int glk_color_check(struct intel_crtc_state *crtc_state)
 
 	glk_assign_luts(crtc_state);
 
-	crtc_state->preload_luts = glk_can_preload_luts(crtc_state);
+	crtc_state->preload_luts = intel_can_preload_luts(crtc_state);
 
 	return 0;
 }
-- 
2.35.1


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

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Prep work for finishing (de)gamma readout
  2022-09-29  7:15 [Intel-gfx] [PATCH 00/10] drm/i915: Prep work for finishing (de)gamma readout Ville Syrjala
                   ` (9 preceding siblings ...)
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 10/10] drm/i915: Stop loading linear degammma LUT on glk needlessly Ville Syrjala
@ 2022-09-29  9:19 ` Patchwork
  2022-09-29  9:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
  2022-09-30  6:35 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  12 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2022-09-29  9:19 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Prep work for finishing (de)gamma readout
URL   : https://patchwork.freedesktop.org/series/109229/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.



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

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915: Prep work for finishing (de)gamma readout
  2022-09-29  7:15 [Intel-gfx] [PATCH 00/10] drm/i915: Prep work for finishing (de)gamma readout Ville Syrjala
                   ` (10 preceding siblings ...)
  2022-09-29  9:19 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Prep work for finishing (de)gamma readout Patchwork
@ 2022-09-29  9:40 ` Patchwork
  2022-09-30  6:35 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  12 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2022-09-29  9:40 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 10432 bytes --]

== Series Details ==

Series: drm/i915: Prep work for finishing (de)gamma readout
URL   : https://patchwork.freedesktop.org/series/109229/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12197 -> Patchwork_109229v1
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/index.html

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

  Additional (2): fi-rkl-guc fi-tgl-dsi 
  Missing    (2): bat-dg2-9 fi-bdw-samus 

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_lmem_swapping@basic:
    - fi-rkl-guc:         NOTRUN -> [SKIP][1] ([i915#4613]) +3 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-rkl-guc/igt@gem_lmem_swapping@basic.html

  * igt@gem_lmem_swapping@random-engines:
    - fi-icl-u2:          NOTRUN -> [SKIP][2] ([i915#4613]) +3 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-icl-u2/igt@gem_lmem_swapping@random-engines.html

  * igt@gem_tiled_pread_basic:
    - fi-rkl-guc:         NOTRUN -> [SKIP][3] ([i915#3282])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-rkl-guc/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - fi-rkl-guc:         NOTRUN -> [SKIP][4] ([i915#3012])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-rkl-guc/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_pm_rps@basic-api:
    - fi-rkl-guc:         NOTRUN -> [SKIP][5] ([i915#6621])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-rkl-guc/igt@i915_pm_rps@basic-api.html

  * igt@kms_chamelium@common-hpd-after-suspend:
    - fi-hsw-4770:        NOTRUN -> [SKIP][6] ([fdo#109271] / [fdo#111827])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-hsw-4770/igt@kms_chamelium@common-hpd-after-suspend.html
    - bat-dg1-5:          NOTRUN -> [SKIP][7] ([fdo#111827])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/bat-dg1-5/igt@kms_chamelium@common-hpd-after-suspend.html

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-rkl-guc:         NOTRUN -> [SKIP][8] ([fdo#111827]) +8 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-rkl-guc/igt@kms_chamelium@hdmi-crc-fast.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          NOTRUN -> [SKIP][9] ([fdo#111827]) +8 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor:
    - fi-icl-u2:          NOTRUN -> [SKIP][10] ([i915#4103])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html
    - fi-rkl-guc:         NOTRUN -> [SKIP][11] ([i915#4103])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-rkl-guc/igt@kms_cursor_legacy@basic-busy-flip-before-cursor.html

  * igt@kms_force_connector_basic@force-connector-state:
    - fi-icl-u2:          NOTRUN -> [WARN][12] ([i915#6008])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-icl-u2/igt@kms_force_connector_basic@force-connector-state.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-icl-u2:          NOTRUN -> [SKIP][13] ([fdo#109285])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-icl-u2/igt@kms_force_connector_basic@force-load-detect.html
    - fi-rkl-guc:         NOTRUN -> [SKIP][14] ([fdo#109285])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-rkl-guc/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@suspend-read-crc:
    - bat-dg1-5:          NOTRUN -> [SKIP][15] ([i915#4078])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/bat-dg1-5/igt@kms_pipe_crc_basic@suspend-read-crc.html

  * igt@kms_psr@sprite_plane_onoff:
    - fi-rkl-guc:         NOTRUN -> [SKIP][16] ([i915#1072]) +3 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-rkl-guc/igt@kms_psr@sprite_plane_onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-rkl-guc:         NOTRUN -> [SKIP][17] ([i915#3555] / [i915#4098])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-rkl-guc/igt@kms_setmode@basic-clone-single-crtc.html
    - fi-icl-u2:          NOTRUN -> [SKIP][18] ([i915#3555])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-icl-u2/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-userptr:
    - fi-icl-u2:          NOTRUN -> [SKIP][19] ([fdo#109295] / [i915#3301])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-icl-u2/igt@prime_vgem@basic-userptr.html
    - fi-rkl-guc:         NOTRUN -> [SKIP][20] ([fdo#109295] / [i915#3301] / [i915#3708])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-rkl-guc/igt@prime_vgem@basic-userptr.html

  * igt@prime_vgem@basic-write:
    - fi-rkl-guc:         NOTRUN -> [SKIP][21] ([fdo#109295] / [i915#3291] / [i915#3708]) +2 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-rkl-guc/igt@prime_vgem@basic-write.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3@smem:
    - {bat-adlm-1}:       [DMESG-WARN][22] ([i915#2867]) -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/bat-adlm-1/igt@gem_exec_suspend@basic-s3@smem.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/bat-adlm-1/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@i915_selftest@live@gt_engines:
    - bat-dg1-5:          [INCOMPLETE][24] ([i915#4418]) -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/bat-dg1-5/igt@i915_selftest@live@gt_engines.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/bat-dg1-5/igt@i915_selftest@live@gt_engines.html

  * igt@i915_selftest@live@hangcheck:
    - fi-hsw-4770:        [INCOMPLETE][26] ([i915#4785]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html

  * igt@kms_addfb_basic@bad-pitch-999:
    - fi-icl-u2:          [DMESG-WARN][28] ([i915#4890]) -> [PASS][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/fi-icl-u2/igt@kms_addfb_basic@bad-pitch-999.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/fi-icl-u2/igt@kms_addfb_basic@bad-pitch-999.html

  * igt@kms_pipe_crc_basic@suspend-read-crc@pipe-d-dp-2:
    - {bat-dg2-11}:       [FAIL][30] ([i915#6818]) -> [PASS][31]
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/bat-dg2-11/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-d-dp-2.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/bat-dg2-11/igt@kms_pipe_crc_basic@suspend-read-crc@pipe-d-dp-2.html

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

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2867]: https://gitlab.freedesktop.org/drm/intel/issues/2867
  [i915#3012]: https://gitlab.freedesktop.org/drm/intel/issues/3012
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3291]: https://gitlab.freedesktop.org/drm/intel/issues/3291
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4078]: https://gitlab.freedesktop.org/drm/intel/issues/4078
  [i915#4098]: https://gitlab.freedesktop.org/drm/intel/issues/4098
  [i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4418]: https://gitlab.freedesktop.org/drm/intel/issues/4418
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
  [i915#4890]: https://gitlab.freedesktop.org/drm/intel/issues/4890
  [i915#6008]: https://gitlab.freedesktop.org/drm/intel/issues/6008
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6818]: https://gitlab.freedesktop.org/drm/intel/issues/6818
  [i915#6856]: https://gitlab.freedesktop.org/drm/intel/issues/6856


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

  * Linux: CI_DRM_12197 -> Patchwork_109229v1

  CI-20190529: 20190529
  CI_DRM_12197: cafa326de8e7860d45639e61bf66ec9c218207b1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6668: 5f29c9369550164b35b65baaaeba4b370f434cf1 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_109229v1: cafa326de8e7860d45639e61bf66ec9c218207b1 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

36274452d453 drm/i915: Stop loading linear degammma LUT on glk needlessly
fd393db161f2 drm/i915: Get rid of glk_load_degamma_lut_linear()
1eb45ab86fae drm/i915: Assert {pre, post}_csc_lut were assigned sensibly
b2ff8103aedb drm/i915: Introduce crtc_state->{pre, post}_csc_lut
3a6c5328f5b1 drm/i915: Make ilk_load_luts() deal with degamma
0e957e10a216 drm/i915: Change glk_load_degamma_lut() calling convention
e694422c24a9 drm/i915: Clean up intel_color_init_hooks()
9e8f7bbadfe7 drm/i915: Simplify the intel_color_init_hooks() if ladder
80b692a7a273 drm/i915: Split up intel_color_init()
91c332c437e2 drm/i915: Remove PLL asserts from .load_luts()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/index.html

[-- Attachment #2: Type: text/html, Size: 12122 bytes --]

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

* Re: [Intel-gfx] [PATCH 02/10] drm/i915: Split up intel_color_init()
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 02/10] drm/i915: Split up intel_color_init() Ville Syrjala
@ 2022-09-29 10:41   ` Jani Nikula
  2022-09-29 11:55     ` Jani Nikula
  0 siblings, 1 reply; 26+ messages in thread
From: Jani Nikula @ 2022-09-29 10:41 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Thu, 29 Sep 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> intel_color_init() does both device level and crtc level stuff.
> Split it up accordingly.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c   | 15 +++++++++------
>  drivers/gpu/drm/i915/display/intel_color.h   |  4 +++-
>  drivers/gpu/drm/i915/display/intel_crtc.c    |  3 +--
>  drivers/gpu/drm/i915/display/intel_display.c |  1 +
>  4 files changed, 14 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index bbc56affb3ec..ddfe7c257a72 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -2206,13 +2206,21 @@ static const struct intel_color_funcs ilk_color_funcs = {
>  	.read_luts = ilk_read_luts,
>  };
>  
> -void intel_color_init(struct intel_crtc *crtc)
> +void intel_crtc_color_init(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	bool has_ctm = INTEL_INFO(dev_priv)->display.color.degamma_lut_size != 0;
>  
>  	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
>  
> +	drm_crtc_enable_color_mgmt(&crtc->base,
> +				   INTEL_INFO(dev_priv)->display.color.degamma_lut_size,
> +				   has_ctm,
> +				   INTEL_INFO(dev_priv)->display.color.gamma_lut_size);
> +}
> +
> +void intel_color_init_hooks(struct drm_i915_private *dev_priv)
> +{
>  	if (HAS_GMCH(dev_priv)) {
>  		if (IS_CHERRYVIEW(dev_priv)) {
>  			dev_priv->display.funcs.color = &chv_color_funcs;
> @@ -2238,9 +2246,4 @@ void intel_color_init(struct intel_crtc *crtc)
>  		} else
>  			dev_priv->display.funcs.color = &ilk_color_funcs;
>  	}
> -
> -	drm_crtc_enable_color_mgmt(&crtc->base,
> -				   INTEL_INFO(dev_priv)->display.color.degamma_lut_size,
> -				   has_ctm,
> -				   INTEL_INFO(dev_priv)->display.color.gamma_lut_size);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_color.h b/drivers/gpu/drm/i915/display/intel_color.h
> index fd873425e082..67702451e2fd 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.h
> +++ b/drivers/gpu/drm/i915/display/intel_color.h
> @@ -10,9 +10,11 @@
>  
>  struct intel_crtc_state;
>  struct intel_crtc;
> +struct drm_i915_private;
>  struct drm_property_blob;
>  
> -void intel_color_init(struct intel_crtc *crtc);
> +void intel_color_init_hooks(struct drm_i915_private *i915);
> +void intel_crtc_color_init(struct intel_crtc *crtc);
>  int intel_color_check(struct intel_crtc_state *crtc_state);
>  void intel_color_commit_noarm(const struct intel_crtc_state *crtc_state);
>  void intel_color_commit_arm(const struct intel_crtc_state *crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
> index 6792a9056f46..2d9fc7383bfc 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
> @@ -365,8 +365,7 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>  						BIT(DRM_SCALING_FILTER_DEFAULT) |
>  						BIT(DRM_SCALING_FILTER_NEAREST_NEIGHBOR));
>  
> -	intel_color_init(crtc);
> -
> +	intel_crtc_color_init(crtc);
>  	intel_crtc_drrs_init(crtc);
>  	intel_crtc_crc_init(crtc);

I'd name all of these like:

	intel_color_crtc_init();
        intel_drrs_crtc_init();
        intel_crc_crtc_init();

I think in gem side the "name functions based on first/context argument"
style has lead to serious problems wrt code organization. They still
stick to it, but I can't make heads or tails where function definitions
or declarations are supposed to be placed or found.

BR,
Jani.


>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index eb8eaeb19881..a103413cb081 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8326,6 +8326,7 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>  	if (!HAS_DISPLAY(dev_priv))
>  		return;
>  
> +	intel_color_init_hooks(dev_priv);
>  	intel_init_cdclk_hooks(dev_priv);
>  	intel_audio_hooks_init(dev_priv);

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH 01/10] drm/i915: Remove PLL asserts from .load_luts()
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 01/10] drm/i915: Remove PLL asserts from .load_luts() Ville Syrjala
@ 2022-09-29 11:54   ` Jani Nikula
  0 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2022-09-29 11:54 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Thu, 29 Sep 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> .load_luts() potentially runs from the vblank worker, and is
> under a deadline to complete within the vblank. Thus we can't
> do expesive stuff like talk to the Punit, etc.
>
> To that end get rid of the assert_dsi_pll_enabled() call for
> vlv/chv. We'll just have to trust that the PLL is already enabled
> here.
>
> And I don't think the normal assert_pll_enabled() really buys us
> anything useful on gmch platforms either, so nuke that one too.
> We don't have corresponding asserts in the ilk+ codepaths anyway
> despite the hardware (IIRC) still requiring the clock to be
> enabled when we access the LUT.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Acked-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 11 -----------
>  1 file changed, 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index 6bda4274eae9..bbc56affb3ec 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -25,9 +25,7 @@
>  #include "intel_color.h"
>  #include "intel_de.h"
>  #include "intel_display_types.h"
> -#include "intel_dpll.h"
>  #include "intel_dsb.h"
> -#include "vlv_dsi_pll.h"
>  
>  struct intel_color_funcs {
>  	int (*color_check)(struct intel_crtc_state *crtc_state);
> @@ -580,11 +578,8 @@ static void i9xx_load_lut_8(struct intel_crtc *crtc,
>  static void i9xx_load_luts(const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
>  
> -	assert_pll_enabled(dev_priv, crtc->pipe);
> -
>  	i9xx_load_lut_8(crtc, gamma_lut);
>  }
>  
> @@ -611,14 +606,8 @@ static void i965_load_lut_10p6(struct intel_crtc *crtc,
>  static void i965_load_luts(const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
>  
> -	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DSI))
> -		assert_dsi_pll_enabled(dev_priv);
> -	else
> -		assert_pll_enabled(dev_priv, crtc->pipe);
> -
>  	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
>  		i9xx_load_lut_8(crtc, gamma_lut);
>  	else

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH 02/10] drm/i915: Split up intel_color_init()
  2022-09-29 10:41   ` Jani Nikula
@ 2022-09-29 11:55     ` Jani Nikula
  0 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2022-09-29 11:55 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Thu, 29 Sep 2022, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Thu, 29 Sep 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> intel_color_init() does both device level and crtc level stuff.
>> Split it up accordingly.
>>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_color.c   | 15 +++++++++------
>>  drivers/gpu/drm/i915/display/intel_color.h   |  4 +++-
>>  drivers/gpu/drm/i915/display/intel_crtc.c    |  3 +--
>>  drivers/gpu/drm/i915/display/intel_display.c |  1 +
>>  4 files changed, 14 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
>> index bbc56affb3ec..ddfe7c257a72 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>> @@ -2206,13 +2206,21 @@ static const struct intel_color_funcs ilk_color_funcs = {
>>  	.read_luts = ilk_read_luts,
>>  };
>>  
>> -void intel_color_init(struct intel_crtc *crtc)
>> +void intel_crtc_color_init(struct intel_crtc *crtc)
>>  {
>>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>  	bool has_ctm = INTEL_INFO(dev_priv)->display.color.degamma_lut_size != 0;
>>  
>>  	drm_mode_crtc_set_gamma_size(&crtc->base, 256);
>>  
>> +	drm_crtc_enable_color_mgmt(&crtc->base,
>> +				   INTEL_INFO(dev_priv)->display.color.degamma_lut_size,
>> +				   has_ctm,
>> +				   INTEL_INFO(dev_priv)->display.color.gamma_lut_size);
>> +}
>> +
>> +void intel_color_init_hooks(struct drm_i915_private *dev_priv)
>> +{
>>  	if (HAS_GMCH(dev_priv)) {
>>  		if (IS_CHERRYVIEW(dev_priv)) {
>>  			dev_priv->display.funcs.color = &chv_color_funcs;
>> @@ -2238,9 +2246,4 @@ void intel_color_init(struct intel_crtc *crtc)
>>  		} else
>>  			dev_priv->display.funcs.color = &ilk_color_funcs;
>>  	}
>> -
>> -	drm_crtc_enable_color_mgmt(&crtc->base,
>> -				   INTEL_INFO(dev_priv)->display.color.degamma_lut_size,
>> -				   has_ctm,
>> -				   INTEL_INFO(dev_priv)->display.color.gamma_lut_size);
>>  }
>> diff --git a/drivers/gpu/drm/i915/display/intel_color.h b/drivers/gpu/drm/i915/display/intel_color.h
>> index fd873425e082..67702451e2fd 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color.h
>> +++ b/drivers/gpu/drm/i915/display/intel_color.h
>> @@ -10,9 +10,11 @@
>>  
>>  struct intel_crtc_state;
>>  struct intel_crtc;
>> +struct drm_i915_private;
>>  struct drm_property_blob;
>>  
>> -void intel_color_init(struct intel_crtc *crtc);
>> +void intel_color_init_hooks(struct drm_i915_private *i915);
>> +void intel_crtc_color_init(struct intel_crtc *crtc);
>>  int intel_color_check(struct intel_crtc_state *crtc_state);
>>  void intel_color_commit_noarm(const struct intel_crtc_state *crtc_state);
>>  void intel_color_commit_arm(const struct intel_crtc_state *crtc_state);
>> diff --git a/drivers/gpu/drm/i915/display/intel_crtc.c b/drivers/gpu/drm/i915/display/intel_crtc.c
>> index 6792a9056f46..2d9fc7383bfc 100644
>> --- a/drivers/gpu/drm/i915/display/intel_crtc.c
>> +++ b/drivers/gpu/drm/i915/display/intel_crtc.c
>> @@ -365,8 +365,7 @@ int intel_crtc_init(struct drm_i915_private *dev_priv, enum pipe pipe)
>>  						BIT(DRM_SCALING_FILTER_DEFAULT) |
>>  						BIT(DRM_SCALING_FILTER_NEAREST_NEIGHBOR));
>>  
>> -	intel_color_init(crtc);
>> -
>> +	intel_crtc_color_init(crtc);
>>  	intel_crtc_drrs_init(crtc);
>>  	intel_crtc_crc_init(crtc);
>
> I'd name all of these like:
>
> 	intel_color_crtc_init();
>         intel_drrs_crtc_init();
>         intel_crc_crtc_init();
>
> I think in gem side the "name functions based on first/context argument"
> style has lead to serious problems wrt code organization. They still
> stick to it, but I can't make heads or tails where function definitions
> or declarations are supposed to be placed or found.

The patch itself is fine, and follows the precedent set by the above
functions, but I'd *really* like to see all the above functions renamed
afterwards.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
> BR,
> Jani.
>
>
>>  
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index eb8eaeb19881..a103413cb081 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -8326,6 +8326,7 @@ void intel_init_display_hooks(struct drm_i915_private *dev_priv)
>>  	if (!HAS_DISPLAY(dev_priv))
>>  		return;
>>  
>> +	intel_color_init_hooks(dev_priv);
>>  	intel_init_cdclk_hooks(dev_priv);
>>  	intel_audio_hooks_init(dev_priv);

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH 03/10] drm/i915: Simplify the intel_color_init_hooks() if ladder
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 03/10] drm/i915: Simplify the intel_color_init_hooks() if ladder Ville Syrjala
@ 2022-09-29 11:56   ` Jani Nikula
  0 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2022-09-29 11:56 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Thu, 29 Sep 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Get rid of the funny hsw vs. ivb extra indentation level in
> intel_color_init_hooks().
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index ddfe7c257a72..ce8a4be9b292 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -2238,12 +2238,11 @@ void intel_color_init_hooks(struct drm_i915_private *dev_priv)
>  			dev_priv->display.funcs.color = &skl_color_funcs;
>  		else if (DISPLAY_VER(dev_priv) == 8)
>  			dev_priv->display.funcs.color = &bdw_color_funcs;
> -		else if (DISPLAY_VER(dev_priv) == 7) {
> -			if (IS_HASWELL(dev_priv))
> -				dev_priv->display.funcs.color = &hsw_color_funcs;
> -			else
> -				dev_priv->display.funcs.color = &ivb_color_funcs;
> -		} else
> +		else if (IS_HASWELL(dev_priv))
> +			dev_priv->display.funcs.color = &hsw_color_funcs;
> +		else if (DISPLAY_VER(dev_priv) == 7)
> +			dev_priv->display.funcs.color = &ivb_color_funcs;
> +		else
>  			dev_priv->display.funcs.color = &ilk_color_funcs;
>  	}
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH 04/10] drm/i915: Clean up intel_color_init_hooks()
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 04/10] drm/i915: Clean up intel_color_init_hooks() Ville Syrjala
@ 2022-09-29 11:57   ` Jani Nikula
  0 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2022-09-29 11:57 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Thu, 29 Sep 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Remove a bunch of pointless curly brackets and do
> the s/dev_priv/i915/ while at it.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 43 +++++++++++-----------
>  1 file changed, 21 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index ce8a4be9b292..96687ec30b19 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -2219,30 +2219,29 @@ void intel_crtc_color_init(struct intel_crtc *crtc)
>  				   INTEL_INFO(dev_priv)->display.color.gamma_lut_size);
>  }
>  
> -void intel_color_init_hooks(struct drm_i915_private *dev_priv)
> +void intel_color_init_hooks(struct drm_i915_private *i915)
>  {
> -	if (HAS_GMCH(dev_priv)) {
> -		if (IS_CHERRYVIEW(dev_priv)) {
> -			dev_priv->display.funcs.color = &chv_color_funcs;
> -		} else if (DISPLAY_VER(dev_priv) >= 4) {
> -			dev_priv->display.funcs.color = &i965_color_funcs;
> -		} else {
> -			dev_priv->display.funcs.color = &i9xx_color_funcs;
> -		}
> +	if (HAS_GMCH(i915)) {
> +		if (IS_CHERRYVIEW(i915))
> +			i915->display.funcs.color = &chv_color_funcs;
> +		else if (DISPLAY_VER(i915) >= 4)
> +			i915->display.funcs.color = &i965_color_funcs;
> +		else
> +			i915->display.funcs.color = &i9xx_color_funcs;
>  	} else {
> -		if (DISPLAY_VER(dev_priv) >= 11)
> -			dev_priv->display.funcs.color = &icl_color_funcs;
> -		else if (DISPLAY_VER(dev_priv) == 10)
> -			dev_priv->display.funcs.color = &glk_color_funcs;
> -		else if (DISPLAY_VER(dev_priv) == 9)
> -			dev_priv->display.funcs.color = &skl_color_funcs;
> -		else if (DISPLAY_VER(dev_priv) == 8)
> -			dev_priv->display.funcs.color = &bdw_color_funcs;
> -		else if (IS_HASWELL(dev_priv))
> -			dev_priv->display.funcs.color = &hsw_color_funcs;
> -		else if (DISPLAY_VER(dev_priv) == 7)
> -			dev_priv->display.funcs.color = &ivb_color_funcs;
> +		if (DISPLAY_VER(i915) >= 11)
> +			i915->display.funcs.color = &icl_color_funcs;
> +		else if (DISPLAY_VER(i915) == 10)
> +			i915->display.funcs.color = &glk_color_funcs;
> +		else if (DISPLAY_VER(i915) == 9)
> +			i915->display.funcs.color = &skl_color_funcs;
> +		else if (DISPLAY_VER(i915) == 8)
> +			i915->display.funcs.color = &bdw_color_funcs;
> +		else if (IS_HASWELL(i915))
> +			i915->display.funcs.color = &hsw_color_funcs;
> +		else if (DISPLAY_VER(i915) == 7)
> +			i915->display.funcs.color = &ivb_color_funcs;
>  		else
> -			dev_priv->display.funcs.color = &ilk_color_funcs;
> +			i915->display.funcs.color = &ilk_color_funcs;
>  	}
>  }

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH 05/10] drm/i915: Change glk_load_degamma_lut() calling convention
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 05/10] drm/i915: Change glk_load_degamma_lut() calling convention Ville Syrjala
@ 2022-09-29 11:58   ` Jani Nikula
  0 siblings, 0 replies; 26+ messages in thread
From: Jani Nikula @ 2022-09-29 11:58 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

On Thu, 29 Sep 2022, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Make glk_load_degamma_lut() more like most everyone else and
> pass in the LUT explicitly.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index 96687ec30b19..0c73b2ba1283 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -826,13 +826,14 @@ static int glk_degamma_lut_size(struct drm_i915_private *i915)
>  		return 35;
>  }
>  
> -static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state)
> +static void glk_load_degamma_lut(const struct intel_crtc_state *crtc_state,
> +				 const struct drm_property_blob *blob)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> +	const struct drm_color_lut *lut = blob->data;
> +	int i, lut_size = drm_color_lut_size(blob);
>  	enum pipe pipe = crtc->pipe;
> -	int i, lut_size = INTEL_INFO(dev_priv)->display.color.degamma_lut_size;
> -	const struct drm_color_lut *lut = crtc_state->hw.degamma_lut->data;
>  
>  	/*
>  	 * When setting the auto-increment bit, the hardware seems to
> @@ -899,6 +900,7 @@ static void glk_load_degamma_lut_linear(const struct intel_crtc_state *crtc_stat
>  
>  static void glk_load_luts(const struct intel_crtc_state *crtc_state)
>  {
> +	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
>  	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  
> @@ -910,8 +912,8 @@ static void glk_load_luts(const struct intel_crtc_state *crtc_state)
>  	 * the degama LUT so that we don't have to reload
>  	 * it every time the pipe CSC is being enabled.
>  	 */
> -	if (crtc_state->hw.degamma_lut)
> -		glk_load_degamma_lut(crtc_state);
> +	if (degamma_lut)
> +		glk_load_degamma_lut(crtc_state, degamma_lut);
>  	else
>  		glk_load_degamma_lut_linear(crtc_state);
>  
> @@ -1043,11 +1045,12 @@ icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)
>  
>  static void icl_load_luts(const struct intel_crtc_state *crtc_state)
>  {
> +	const struct drm_property_blob *degamma_lut = crtc_state->hw.degamma_lut;
>  	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  
> -	if (crtc_state->hw.degamma_lut)
> -		glk_load_degamma_lut(crtc_state);
> +	if (degamma_lut)
> +		glk_load_degamma_lut(crtc_state, degamma_lut);
>  
>  	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
>  	case GAMMA_MODE_MODE_8BIT:

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915: Prep work for finishing (de)gamma readout
  2022-09-29  7:15 [Intel-gfx] [PATCH 00/10] drm/i915: Prep work for finishing (de)gamma readout Ville Syrjala
                   ` (11 preceding siblings ...)
  2022-09-29  9:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2022-09-30  6:35 ` Patchwork
  12 siblings, 0 replies; 26+ messages in thread
From: Patchwork @ 2022-09-30  6:35 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 23488 bytes --]

== Series Details ==

Series: drm/i915: Prep work for finishing (de)gamma readout
URL   : https://patchwork.freedesktop.org/series/109229/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_12197_full -> Patchwork_109229v1_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (12 -> 9)
------------------------------

  Missing    (3): shard-rkl shard-dg1 shard-tglu 

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

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

### CI changes ###

#### Possible fixes ####

  * boot:
    - shard-glk:          ([PASS][1], [PASS][2], [PASS][3], [PASS][4], [PASS][5], [PASS][6], [PASS][7], [PASS][8], [PASS][9], [PASS][10], [PASS][11], [PASS][12], [FAIL][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], [PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24], [PASS][25]) ([i915#4392]) -> ([PASS][26], [PASS][27], [PASS][28], [PASS][29], [PASS][30], [PASS][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], [PASS][36], [PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42], [PASS][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47], [PASS][48], [PASS][49], [PASS][50])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk7/boot.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk8/boot.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk7/boot.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk1/boot.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk1/boot.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk1/boot.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk2/boot.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk2/boot.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk2/boot.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk3/boot.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk3/boot.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk3/boot.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk3/boot.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk5/boot.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk5/boot.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk5/boot.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk6/boot.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk6/boot.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk6/boot.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk7/boot.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk9/boot.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk9/boot.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk9/boot.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk8/boot.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk8/boot.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk1/boot.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk1/boot.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk1/boot.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk2/boot.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk2/boot.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk2/boot.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk3/boot.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk3/boot.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk3/boot.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk3/boot.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk5/boot.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk5/boot.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk5/boot.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk6/boot.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk6/boot.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk6/boot.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk7/boot.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk7/boot.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk7/boot.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk8/boot.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk8/boot.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk8/boot.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk9/boot.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk9/boot.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk9/boot.html

  

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_balancer@parallel-keep-in-fence:
    - shard-iclb:         [PASS][51] -> [SKIP][52] ([i915#4525]) +3 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-iclb4/igt@gem_exec_balancer@parallel-keep-in-fence.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb3/igt@gem_exec_balancer@parallel-keep-in-fence.html

  * igt@gem_exec_fair@basic-none-share@rcs0:
    - shard-iclb:         [PASS][53] -> [FAIL][54] ([i915#2842])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-iclb4/igt@gem_exec_fair@basic-none-share@rcs0.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb3/igt@gem_exec_fair@basic-none-share@rcs0.html

  * igt@gem_exec_fair@basic-none-solo@rcs0:
    - shard-apl:          [PASS][55] -> [FAIL][56] ([i915#2842]) +1 similar issue
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-apl3/igt@gem_exec_fair@basic-none-solo@rcs0.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl3/igt@gem_exec_fair@basic-none-solo@rcs0.html

  * igt@gem_exec_suspend@basic-s3@smem:
    - shard-apl:          [PASS][57] -> [DMESG-WARN][58] ([i915#180]) +2 similar issues
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-apl1/igt@gem_exec_suspend@basic-s3@smem.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl6/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@gem_huc_copy@huc-copy:
    - shard-tglb:         [PASS][59] -> [SKIP][60] ([i915#2190])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-tglb8/igt@gem_huc_copy@huc-copy.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-tglb7/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - shard-glk:          NOTRUN -> [SKIP][61] ([fdo#109271] / [i915#4613])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk3/igt@gem_lmem_swapping@basic.html

  * igt@gem_lmem_swapping@massive-random:
    - shard-apl:          NOTRUN -> [SKIP][62] ([fdo#109271] / [i915#4613])
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl2/igt@gem_lmem_swapping@massive-random.html

  * igt@gem_mmap_offset@clear@smem0:
    - shard-snb:          [PASS][63] -> [FAIL][64] ([i915#6973])
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-snb4/igt@gem_mmap_offset@clear@smem0.html
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-snb5/igt@gem_mmap_offset@clear@smem0.html

  * igt@kms_ccs@pipe-a-bad-aux-stride-y_tiled_gen12_rc_ccs_cc:
    - shard-glk:          NOTRUN -> [SKIP][65] ([fdo#109271] / [i915#3886]) +3 similar issues
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk3/igt@kms_ccs@pipe-a-bad-aux-stride-y_tiled_gen12_rc_ccs_cc.html

  * igt@kms_ccs@pipe-c-bad-aux-stride-y_tiled_gen12_mc_ccs:
    - shard-apl:          NOTRUN -> [SKIP][66] ([fdo#109271] / [i915#3886]) +8 similar issues
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl2/igt@kms_ccs@pipe-c-bad-aux-stride-y_tiled_gen12_mc_ccs.html

  * igt@kms_ccs@pipe-c-bad-rotation-90-y_tiled_gen12_rc_ccs:
    - shard-glk:          NOTRUN -> [SKIP][67] ([fdo#109271]) +44 similar issues
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk3/igt@kms_ccs@pipe-c-bad-rotation-90-y_tiled_gen12_rc_ccs.html

  * igt@kms_chamelium@hdmi-hpd-after-suspend:
    - shard-glk:          NOTRUN -> [SKIP][68] ([fdo#109271] / [fdo#111827]) +1 similar issue
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk3/igt@kms_chamelium@hdmi-hpd-after-suspend.html

  * igt@kms_chamelium@vga-hpd-after-suspend:
    - shard-apl:          NOTRUN -> [SKIP][69] ([fdo#109271] / [fdo#111827]) +6 similar issues
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl8/igt@kms_chamelium@vga-hpd-after-suspend.html

  * igt@kms_content_protection@uevent:
    - shard-apl:          NOTRUN -> [FAIL][70] ([i915#2105])
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl8/igt@kms_content_protection@uevent.html

  * igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions:
    - shard-glk:          [PASS][71] -> [FAIL][72] ([i915#2346])
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk8/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk5/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions.html

  * igt@kms_fbcon_fbt@fbc-suspend:
    - shard-apl:          [PASS][73] -> [INCOMPLETE][74] ([i915#180] / [i915#1982] / [i915#4939])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-apl7/igt@kms_fbcon_fbt@fbc-suspend.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl8/igt@kms_fbcon_fbt@fbc-suspend.html

  * igt@kms_flip@2x-nonexisting-fb:
    - shard-apl:          NOTRUN -> [SKIP][75] ([fdo#109271]) +119 similar issues
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl2/igt@kms_flip@2x-nonexisting-fb.html

  * igt@kms_flip_scaled_crc@flip-32bpp-4tile-to-64bpp-4tile-upscaling@pipe-a-default-mode:
    - shard-iclb:         NOTRUN -> [SKIP][76] ([i915#2672]) +2 similar issues
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb2/igt@kms_flip_scaled_crc@flip-32bpp-4tile-to-64bpp-4tile-upscaling@pipe-a-default-mode.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-downscaling@pipe-a-default-mode:
    - shard-iclb:         NOTRUN -> [SKIP][77] ([i915#3555]) +1 similar issue
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb2/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytileccs-downscaling@pipe-a-default-mode.html

  * igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs-downscaling@pipe-a-default-mode:
    - shard-iclb:         NOTRUN -> [SKIP][78] ([i915#2672] / [i915#3555])
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb3/igt@kms_flip_scaled_crc@flip-32bpp-ytile-to-32bpp-ytilegen12rcccs-downscaling@pipe-a-default-mode.html

  * igt@kms_flip_scaled_crc@flip-64bpp-4tile-to-16bpp-4tile-upscaling@pipe-a-valid-mode:
    - shard-iclb:         NOTRUN -> [SKIP][79] ([i915#2587] / [i915#2672]) +2 similar issues
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb5/igt@kms_flip_scaled_crc@flip-64bpp-4tile-to-16bpp-4tile-upscaling@pipe-a-valid-mode.html

  * igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf:
    - shard-glk:          NOTRUN -> [SKIP][80] ([fdo#109271] / [i915#658])
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk3/igt@kms_psr2_sf@overlay-plane-move-continuous-exceed-sf.html

  * igt@kms_psr2_su@page_flip-xrgb8888:
    - shard-iclb:         [PASS][81] -> [SKIP][82] ([fdo#109642] / [fdo#111068] / [i915#658])
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-iclb2/igt@kms_psr2_su@page_flip-xrgb8888.html
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb5/igt@kms_psr2_su@page_flip-xrgb8888.html
    - shard-apl:          NOTRUN -> [SKIP][83] ([fdo#109271] / [i915#658])
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl2/igt@kms_psr2_su@page_flip-xrgb8888.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         [PASS][84] -> [SKIP][85] ([fdo#109441]) +1 similar issue
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb5/igt@kms_psr@psr2_primary_page_flip.html

  * igt@kms_vblank@pipe-d-wait-idle:
    - shard-apl:          NOTRUN -> [SKIP][86] ([fdo#109271] / [i915#533])
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl8/igt@kms_vblank@pipe-d-wait-idle.html

  * igt@kms_writeback@writeback-invalid-parameters:
    - shard-glk:          NOTRUN -> [SKIP][87] ([fdo#109271] / [i915#2437])
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk3/igt@kms_writeback@writeback-invalid-parameters.html

  
#### Possible fixes ####

  * igt@feature_discovery@psr2:
    - shard-iclb:         [SKIP][88] ([i915#658]) -> [PASS][89]
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-iclb6/igt@feature_discovery@psr2.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb2/igt@feature_discovery@psr2.html

  * igt@gem_ctx_exec@basic-nohangcheck:
    - shard-tglb:         [FAIL][90] ([i915#6268]) -> [PASS][91]
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-tglb7/igt@gem_ctx_exec@basic-nohangcheck.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-tglb7/igt@gem_ctx_exec@basic-nohangcheck.html

  * igt@gem_exec_balancer@parallel-balancer:
    - shard-iclb:         [SKIP][92] ([i915#4525]) -> [PASS][93] +1 similar issue
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-iclb7/igt@gem_exec_balancer@parallel-balancer.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb1/igt@gem_exec_balancer@parallel-balancer.html

  * igt@gen9_exec_parse@allowed-all:
    - shard-glk:          [DMESG-WARN][94] ([i915#5566] / [i915#716]) -> [PASS][95]
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-glk5/igt@gen9_exec_parse@allowed-all.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-glk9/igt@gen9_exec_parse@allowed-all.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [DMESG-WARN][96] ([i915#180]) -> [PASS][97] +3 similar issues
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-apl6/igt@i915_suspend@sysfs-reader.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl2/igt@i915_suspend@sysfs-reader.html

  * igt@kms_flip@flip-vs-expired-vblank@b-dp1:
    - shard-apl:          [FAIL][98] ([i915#79]) -> [PASS][99] +1 similar issue
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-apl2/igt@kms_flip@flip-vs-expired-vblank@b-dp1.html
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl7/igt@kms_flip@flip-vs-expired-vblank@b-dp1.html

  * igt@kms_plane_scaling@plane-scaler-with-clipping-clamping-pixel-formats@pipe-b-edp-1:
    - shard-iclb:         [SKIP][100] ([i915#5176]) -> [PASS][101] +1 similar issue
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-iclb3/igt@kms_plane_scaling@plane-scaler-with-clipping-clamping-pixel-formats@pipe-b-edp-1.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb2/igt@kms_plane_scaling@plane-scaler-with-clipping-clamping-pixel-formats@pipe-b-edp-1.html

  * igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-5@pipe-a-edp-1:
    - shard-iclb:         [SKIP][102] ([i915#5235]) -> [PASS][103] +2 similar issues
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-iclb2/igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-5@pipe-a-edp-1.html
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb5/igt@kms_plane_scaling@planes-upscale-factor-0-25-downscale-factor-0-5@pipe-a-edp-1.html

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         [SKIP][104] ([fdo#109441]) -> [PASS][105] +2 similar issues
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-iclb6/igt@kms_psr@psr2_basic.html
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb2/igt@kms_psr@psr2_basic.html

  * igt@perf@polling-small-buf:
    - shard-tglb:         [FAIL][106] ([i915#1722]) -> [PASS][107]
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-tglb8/igt@perf@polling-small-buf.html
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-tglb7/igt@perf@polling-small-buf.html

  
#### Warnings ####

  * igt@gem_exec_balancer@parallel-ordering:
    - shard-iclb:         [FAIL][108] ([i915#6117]) -> [SKIP][109] ([i915#4525])
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-iclb4/igt@gem_exec_balancer@parallel-ordering.html
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb3/igt@gem_exec_balancer@parallel-ordering.html

  * igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-sf:
    - shard-iclb:         [SKIP][110] ([i915#2920]) -> [SKIP][111] ([i915#658]) +1 similar issue
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-iclb2/igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-sf.html
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb5/igt@kms_psr2_sf@cursor-plane-move-continuous-exceed-sf.html

  * igt@kms_psr2_sf@overlay-plane-move-continuous-sf:
    - shard-iclb:         [SKIP][112] ([i915#658]) -> [SKIP][113] ([i915#2920])
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-iclb3/igt@kms_psr2_sf@overlay-plane-move-continuous-sf.html
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-iclb2/igt@kms_psr2_sf@overlay-plane-move-continuous-sf.html

  * igt@runner@aborted:
    - shard-apl:          ([FAIL][114], [FAIL][115], [FAIL][116], [FAIL][117], [FAIL][118], [FAIL][119]) ([i915#180] / [i915#3002] / [i915#4312]) -> ([FAIL][120], [FAIL][121], [FAIL][122], [FAIL][123], [FAIL][124], [FAIL][125]) ([fdo#109271] / [i915#180] / [i915#3002] / [i915#4312])
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-apl1/igt@runner@aborted.html
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-apl6/igt@runner@aborted.html
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-apl1/igt@runner@aborted.html
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-apl8/igt@runner@aborted.html
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-apl2/igt@runner@aborted.html
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12197/shard-apl1/igt@runner@aborted.html
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl2/igt@runner@aborted.html
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl6/igt@runner@aborted.html
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl8/igt@runner@aborted.html
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl1/igt@runner@aborted.html
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl1/igt@runner@aborted.html
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_109229v1/shard-apl6/igt@runner@aborted.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109642]: https://bugs.freedesktop.org/show_bug.cgi?id=109642
  [fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#1722]: https://gitlab.freedesktop.org/drm/intel/issues/1722
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#2105]: https://gitlab.freedesktop.org/drm/intel/issues/2105
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2437]: https://gitlab.freedesktop.org/drm/intel/issues/2437
  [i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
  [i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
  [i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
  [i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
  [i915#3002]: https://gitlab.freedesktop.org/drm/intel/issues/3002
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4392]: https://gitlab.freedesktop.org/drm/intel/issues/4392
  [i915#4525]: https://gitlab.freedesktop.org/drm/intel/issues/4525
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4939]: https://gitlab.freedesktop.org/drm/intel/issues/4939
  [i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
  [i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533
  [i915#5566]: https://gitlab.freedesktop.org/drm/intel/issues/5566
  [i915#6117]: https://gitlab.freedesktop.org/drm/intel/issues/6117
  [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
  [i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
  [i915#6973]: https://gitlab.freedesktop.org/drm/intel/issues/6973
  [i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79


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

  * Linux: CI_DRM_12197 -> Patchwork_109229v1

  CI-20190529: 20190529
  CI_DRM_12197: cafa326de8e7860d45639e61bf66ec9c218207b1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6668: 5f29c9369550164b35b65baaaeba4b370f434cf1 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_109229v1: cafa326de8e7860d45639e61bf66ec9c218207b1 @ 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_109229v1/index.html

[-- Attachment #2: Type: text/html, Size: 27548 bytes --]

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

* Re: [Intel-gfx] [PATCH 07/10] drm/i915: Introduce crtc_state->{pre, post}_csc_lut
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 07/10] drm/i915: Introduce crtc_state->{pre, post}_csc_lut Ville Syrjala
@ 2022-10-19  7:06   ` Shankar, Uma
  0 siblings, 0 replies; 26+ messages in thread
From: Shankar, Uma @ 2022-10-19  7:06 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala
> Sent: Thursday, September 29, 2022 12:45 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 07/10] drm/i915: Introduce crtc_state->{pre,
> post}_csc_lut
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Add an extra remapping step between the logical state of the LUTs
> (hw.(de)gamma_lut) as specified via uapi/bigjoiner copy vs.
> the actual state of the LUTs programmed into the hardware.
> 
> With this we should be finally able finish the (de)gamma readout/state checker
> support for the remaining platforms
> (ilk-skl) where the same hardware LUT can be positioned either before or after the
> pipe CSC unit. Where we position it depends on factors such as presence of the
> logical degamma LUT, RGB vs. YCbCr output, full vs. limited RGB quantization range.
> 
> Without the extra remapping step the state readout doesn't really know whether the
> LUT read from the hardware is the degamma or gamma LUT, and so we is unable to
> accurately store it into our crtc state. With the remapping step we know exactly
> where to put it given the order of the LUT vs. CSC in the hardware state.
> 
> Only the initial hw->uapi state readout done during driver load/resume still has the
> problem of not really knowing what to do with the LUT(s). But we can just assume
> 1:1 mapping there and let subsequent commits fix things up.
> 
> Another benefit is that we now have a place for purely internal LUTs, without
> complicating the bigjoiner uapi->hw copy logic. This should prove useful for
> streamlining glk degamma LUT handling.

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

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_atomic.c   |   8 +
>  drivers/gpu/drm/i915/display/intel_color.c    | 141 +++++++++++-------
>  .../drm/i915/display/intel_crtc_state_dump.c  |  10 +-
>  drivers/gpu/drm/i915/display/intel_display.c  |   2 +-
>  .../drm/i915/display/intel_display_types.h    |   4 +
>  .../drm/i915/display/intel_modeset_setup.c    |   6 +
>  6 files changed, 114 insertions(+), 57 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_atomic.c
> b/drivers/gpu/drm/i915/display/intel_atomic.c
> index 18f0a5ae3bac..6621aa245caf 100644
> --- a/drivers/gpu/drm/i915/display/intel_atomic.c
> +++ b/drivers/gpu/drm/i915/display/intel_atomic.c
> @@ -252,6 +252,11 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
>  	if (crtc_state->hw.gamma_lut)
>  		drm_property_blob_get(crtc_state->hw.gamma_lut);
> 
> +	if (crtc_state->pre_csc_lut)
> +		drm_property_blob_get(crtc_state->pre_csc_lut);
> +	if (crtc_state->post_csc_lut)
> +		drm_property_blob_get(crtc_state->post_csc_lut);
> +
>  	crtc_state->update_pipe = false;
>  	crtc_state->disable_lp_wm = false;
>  	crtc_state->disable_cxsr = false;
> @@ -274,6 +279,9 @@ static void intel_crtc_put_color_blobs(struct intel_crtc_state
> *crtc_state)
>  	drm_property_blob_put(crtc_state->hw.degamma_lut);
>  	drm_property_blob_put(crtc_state->hw.gamma_lut);
>  	drm_property_blob_put(crtc_state->hw.ctm);
> +
> +	drm_property_blob_put(crtc_state->pre_csc_lut);
> +	drm_property_blob_put(crtc_state->post_csc_lut);
>  }
> 
>  void intel_crtc_free_hw_state(struct intel_crtc_state *crtc_state) diff --git
> a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index abaa1abab64d..380f44720fe6 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -578,9 +578,9 @@ static void i9xx_load_lut_8(struct intel_crtc *crtc,  static
> void i9xx_load_luts(const struct intel_crtc_state *crtc_state)  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
> +	const struct drm_property_blob *post_csc_lut =
> +crtc_state->post_csc_lut;
> 
> -	i9xx_load_lut_8(crtc, gamma_lut);
> +	i9xx_load_lut_8(crtc, post_csc_lut);
>  }
> 
>  static void i965_load_lut_10p6(struct intel_crtc *crtc, @@ -606,12 +606,12 @@
> static void i965_load_lut_10p6(struct intel_crtc *crtc,  static void
> i965_load_luts(const struct intel_crtc_state *crtc_state)  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
> +	const struct drm_property_blob *post_csc_lut =
> +crtc_state->post_csc_lut;
> 
>  	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
> -		i9xx_load_lut_8(crtc, gamma_lut);
> +		i9xx_load_lut_8(crtc, post_csc_lut);
>  	else
> -		i965_load_lut_10p6(crtc, gamma_lut);
> +		i965_load_lut_10p6(crtc, post_csc_lut);
>  }
> 
>  static void ilk_load_lut_8(struct intel_crtc *crtc, @@ -648,9 +648,9 @@ static void
> ilk_load_lut_10(struct intel_crtc *crtc,  static void ilk_load_luts(const struct
> intel_crtc_state *crtc_state)  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
> -	const struct drm_property_blob *degamma_lut = crtc_state-
> >hw.degamma_lut;
> -	const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
> +	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
> +	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
> +	const struct drm_property_blob *blob = post_csc_lut ?: pre_csc_lut;
> 
>  	switch (crtc_state->gamma_mode) {
>  	case GAMMA_MODE_MODE_8BIT:
> @@ -764,19 +764,19 @@ static void ivb_load_lut_ext_max(const struct
> intel_crtc_state *crtc_state)  static void ivb_load_luts(const struct intel_crtc_state
> *crtc_state)  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
> -	const struct drm_property_blob *degamma_lut = crtc_state-
> >hw.degamma_lut;
> -	const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
> +	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
> +	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
> +	const struct drm_property_blob *blob = post_csc_lut ?: pre_csc_lut;
> 
>  	switch (crtc_state->gamma_mode) {
>  	case GAMMA_MODE_MODE_8BIT:
>  		ilk_load_lut_8(crtc, blob);
>  		break;
>  	case GAMMA_MODE_MODE_SPLIT:
> -		ivb_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
> +		ivb_load_lut_10(crtc, pre_csc_lut, PAL_PREC_SPLIT_MODE |
>  				PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
> -		ivb_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
> +		ivb_load_lut_10(crtc, post_csc_lut, PAL_PREC_SPLIT_MODE |
>  				PAL_PREC_INDEX_VALUE(512));
>  		break;
>  	case GAMMA_MODE_MODE_10BIT:
> @@ -793,19 +793,19 @@ static void ivb_load_luts(const struct intel_crtc_state
> *crtc_state)  static void bdw_load_luts(const struct intel_crtc_state *crtc_state)  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
> -	const struct drm_property_blob *degamma_lut = crtc_state-
> >hw.degamma_lut;
> -	const struct drm_property_blob *blob = gamma_lut ?: degamma_lut;
> +	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
> +	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
> +	const struct drm_property_blob *blob = post_csc_lut ?: pre_csc_lut;
> 
>  	switch (crtc_state->gamma_mode) {
>  	case GAMMA_MODE_MODE_8BIT:
>  		ilk_load_lut_8(crtc, blob);
>  		break;
>  	case GAMMA_MODE_MODE_SPLIT:
> -		bdw_load_lut_10(crtc, degamma_lut, PAL_PREC_SPLIT_MODE |
> +		bdw_load_lut_10(crtc, pre_csc_lut, PAL_PREC_SPLIT_MODE |
>  				PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
> -		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_SPLIT_MODE |
> +		bdw_load_lut_10(crtc, post_csc_lut, PAL_PREC_SPLIT_MODE |
>  				PAL_PREC_INDEX_VALUE(512));
>  		break;
>  	case GAMMA_MODE_MODE_10BIT:
> @@ -902,8 +902,8 @@ static void glk_load_degamma_lut_linear(const struct
> intel_crtc_state *crtc_stat
> 
>  static void glk_load_luts(const struct intel_crtc_state *crtc_state)  {
> -	const struct drm_property_blob *degamma_lut = crtc_state-
> >hw.degamma_lut;
> -	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
> +	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
> +	const struct drm_property_blob *post_csc_lut =
> +crtc_state->post_csc_lut;
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> 
>  	/*
> @@ -914,17 +914,17 @@ static void glk_load_luts(const struct intel_crtc_state
> *crtc_state)
>  	 * the degama LUT so that we don't have to reload
>  	 * it every time the pipe CSC is being enabled.
>  	 */
> -	if (degamma_lut)
> -		glk_load_degamma_lut(crtc_state, degamma_lut);
> +	if (pre_csc_lut)
> +		glk_load_degamma_lut(crtc_state, pre_csc_lut);
>  	else
>  		glk_load_degamma_lut_linear(crtc_state);
> 
>  	switch (crtc_state->gamma_mode) {
>  	case GAMMA_MODE_MODE_8BIT:
> -		ilk_load_lut_8(crtc, gamma_lut);
> +		ilk_load_lut_8(crtc, post_csc_lut);
>  		break;
>  	case GAMMA_MODE_MODE_10BIT:
> -		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
> +		bdw_load_lut_10(crtc, post_csc_lut, PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
>  		break;
>  	default:
> @@ -964,7 +964,7 @@ static void
>  icl_program_gamma_superfine_segment(const struct intel_crtc_state *crtc_state)
> {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	const struct drm_property_blob *blob = crtc_state->hw.gamma_lut;
> +	const struct drm_property_blob *blob = crtc_state->post_csc_lut;
>  	const struct drm_color_lut *lut = blob->data;
>  	enum pipe pipe = crtc->pipe;
>  	int i;
> @@ -993,7 +993,7 @@ static void
>  icl_program_gamma_multi_segment(const struct intel_crtc_state *crtc_state)  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	const struct drm_property_blob *blob = crtc_state->hw.gamma_lut;
> +	const struct drm_property_blob *blob = crtc_state->post_csc_lut;
>  	const struct drm_color_lut *lut = blob->data;
>  	const struct drm_color_lut *entry;
>  	enum pipe pipe = crtc->pipe;
> @@ -1047,23 +1047,23 @@ icl_program_gamma_multi_segment(const struct
> intel_crtc_state *crtc_state)
> 
>  static void icl_load_luts(const struct intel_crtc_state *crtc_state)  {
> -	const struct drm_property_blob *degamma_lut = crtc_state-
> >hw.degamma_lut;
> -	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
> +	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
> +	const struct drm_property_blob *post_csc_lut =
> +crtc_state->post_csc_lut;
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> 
> -	if (degamma_lut)
> -		glk_load_degamma_lut(crtc_state, degamma_lut);
> +	if (pre_csc_lut)
> +		glk_load_degamma_lut(crtc_state, pre_csc_lut);
> 
>  	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
>  	case GAMMA_MODE_MODE_8BIT:
> -		ilk_load_lut_8(crtc, gamma_lut);
> +		ilk_load_lut_8(crtc, post_csc_lut);
>  		break;
>  	case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
>  		icl_program_gamma_superfine_segment(crtc_state);
>  		icl_program_gamma_multi_segment(crtc_state);
>  		break;
>  	case GAMMA_MODE_MODE_10BIT:
> -		bdw_load_lut_10(crtc, gamma_lut, PAL_PREC_INDEX_VALUE(0));
> +		bdw_load_lut_10(crtc, post_csc_lut, PAL_PREC_INDEX_VALUE(0));
>  		ivb_load_lut_ext_max(crtc_state);
>  		break;
>  	default:
> @@ -1139,18 +1139,18 @@ static void chv_load_luts(const struct intel_crtc_state
> *crtc_state)  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	const struct drm_property_blob *degamma_lut = crtc_state-
> >hw.degamma_lut;
> -	const struct drm_property_blob *gamma_lut = crtc_state->hw.gamma_lut;
> +	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
> +	const struct drm_property_blob *post_csc_lut =
> +crtc_state->post_csc_lut;
>  	const struct drm_property_blob *ctm = crtc_state->hw.ctm;
> 
>  	if (crtc_state->cgm_mode & CGM_PIPE_MODE_CSC)
>  		chv_load_cgm_csc(crtc, ctm);
> 
>  	if (crtc_state->cgm_mode & CGM_PIPE_MODE_DEGAMMA)
> -		chv_load_cgm_degamma(crtc, degamma_lut);
> +		chv_load_cgm_degamma(crtc, pre_csc_lut);
> 
>  	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
> -		chv_load_cgm_gamma(crtc, gamma_lut);
> +		chv_load_cgm_gamma(crtc, post_csc_lut);
>  	else
>  		i965_load_luts(crtc_state);
> 
> @@ -1188,8 +1188,8 @@ static bool intel_can_preload_luts(const struct
> intel_crtc_state *new_crtc_state
>  	const struct intel_crtc_state *old_crtc_state =
>  		intel_atomic_get_old_crtc_state(state, crtc);
> 
> -	return !old_crtc_state->hw.gamma_lut &&
> -		!old_crtc_state->hw.degamma_lut;
> +	return !old_crtc_state->post_csc_lut &&
> +		!old_crtc_state->pre_csc_lut;
>  }
> 
>  static bool chv_can_preload_luts(const struct intel_crtc_state *new_crtc_state)
> @@ -1208,7 +1208,7 @@ static bool chv_can_preload_luts(const struct
> intel_crtc_state *new_crtc_state)
>  	if (old_crtc_state->cgm_mode || new_crtc_state->cgm_mode)
>  		return false;
> 
> -	return !old_crtc_state->hw.gamma_lut;
> +	return !old_crtc_state->post_csc_lut;
>  }
> 
>  static bool glk_can_preload_luts(const struct intel_crtc_state *new_crtc_state) @@
> -1226,7 +1226,7 @@ static bool glk_can_preload_luts(const struct intel_crtc_state
> *new_crtc_state)
>  	 * linear hardware degamma mid scanout.
>  	 */
>  	return !old_crtc_state->csc_enable &&
> -		!old_crtc_state->hw.gamma_lut;
> +		!old_crtc_state->post_csc_lut;
>  }
> 
>  int intel_color_check(struct intel_crtc_state *crtc_state) @@ -1355,6 +1355,14 @@
> static u32 i9xx_gamma_mode(struct intel_crtc_state *crtc_state)
>  		return GAMMA_MODE_MODE_10BIT; /* i965+ only */  }
> 
> +static void intel_assign_luts(struct intel_crtc_state *crtc_state) {
> +	drm_property_replace_blob(&crtc_state->pre_csc_lut,
> +				  crtc_state->hw.degamma_lut);
> +	drm_property_replace_blob(&crtc_state->post_csc_lut,
> +				  crtc_state->hw.gamma_lut);
> +}
> +
>  static int i9xx_color_check(struct intel_crtc_state *crtc_state)  {
>  	int ret;
> @@ -1373,6 +1381,8 @@ static int i9xx_color_check(struct intel_crtc_state
> *crtc_state)
>  	if (ret)
>  		return ret;
> 
> +	intel_assign_luts(crtc_state);
> +
>  	crtc_state->preload_luts = intel_can_preload_luts(crtc_state);
> 
>  	return 0;
> @@ -1427,6 +1437,8 @@ static int chv_color_check(struct intel_crtc_state
> *crtc_state)
>  	if (ret)
>  		return ret;
> 
> +	intel_assign_luts(crtc_state);
> +
>  	crtc_state->preload_luts = chv_can_preload_luts(crtc_state);
> 
>  	return 0;
> @@ -1452,10 +1464,29 @@ static u32 ilk_csc_mode(const struct intel_crtc_state
> *crtc_state)
>  	if (crtc_state->output_format != INTEL_OUTPUT_FORMAT_RGB)
>  		return CSC_BLACK_SCREEN_OFFSET;
> 
> +	if (crtc_state->hw.degamma_lut)
> +		return CSC_MODE_YUV_TO_RGB;
> +
>  	return CSC_MODE_YUV_TO_RGB |
>  		CSC_POSITION_BEFORE_GAMMA;
>  }
> 
> +static void ilk_assign_luts(struct intel_crtc_state *crtc_state) {
> +	if (crtc_state->hw.degamma_lut ||
> +	    crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) {
> +		drm_property_replace_blob(&crtc_state->pre_csc_lut,
> +					  crtc_state->hw.degamma_lut);
> +		drm_property_replace_blob(&crtc_state->post_csc_lut,
> +					  crtc_state->hw.gamma_lut);
> +	} else {
> +		drm_property_replace_blob(&crtc_state->pre_csc_lut,
> +					  crtc_state->hw.gamma_lut);
> +		drm_property_replace_blob(&crtc_state->post_csc_lut,
> +					  NULL);
> +	}
> +}
> +
>  static int ilk_color_check(struct intel_crtc_state *crtc_state)  {
>  	int ret;
> @@ -1483,6 +1514,8 @@ static int ilk_color_check(struct intel_crtc_state
> *crtc_state)
>  	if (ret)
>  		return ret;
> 
> +	ilk_assign_luts(crtc_state);
> +
>  	crtc_state->preload_luts = intel_can_preload_luts(crtc_state);
> 
>  	return 0;
> @@ -1550,6 +1583,8 @@ static int ivb_color_check(struct intel_crtc_state
> *crtc_state)
>  	if (ret)
>  		return ret;
> 
> +	ilk_assign_luts(crtc_state);
> +
>  	crtc_state->preload_luts = intel_can_preload_luts(crtc_state);
> 
>  	return 0;
> @@ -1598,6 +1633,8 @@ static int glk_color_check(struct intel_crtc_state
> *crtc_state)
>  	if (ret)
>  		return ret;
> 
> +	intel_assign_luts(crtc_state);
> +
>  	crtc_state->preload_luts = glk_can_preload_luts(crtc_state);
> 
>  	return 0;
> @@ -1658,6 +1695,8 @@ static int icl_color_check(struct intel_crtc_state
> *crtc_state)
> 
>  	crtc_state->csc_mode = icl_csc_mode(crtc_state);
> 
> +	intel_assign_luts(crtc_state);
> +
>  	crtc_state->preload_luts = intel_can_preload_luts(crtc_state);
> 
>  	return 0;
> @@ -1863,7 +1902,7 @@ static void i9xx_read_luts(struct intel_crtc_state
> *crtc_state)
>  	if (!crtc_state->gamma_enable)
>  		return;
> 
> -	crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc);
> +	crtc_state->post_csc_lut = i9xx_read_lut_8(crtc);
>  }
> 
>  static struct drm_property_blob *i965_read_lut_10p6(struct intel_crtc *crtc) @@ -
> 1904,9 +1943,9 @@ static void i965_read_luts(struct intel_crtc_state *crtc_state)
>  		return;
> 
>  	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
> -		crtc_state->hw.gamma_lut = i9xx_read_lut_8(crtc);
> +		crtc_state->post_csc_lut = i9xx_read_lut_8(crtc);
>  	else
> -		crtc_state->hw.gamma_lut = i965_read_lut_10p6(crtc);
> +		crtc_state->post_csc_lut = i965_read_lut_10p6(crtc);
>  }
> 
>  static struct drm_property_blob *chv_read_cgm_gamma(struct intel_crtc *crtc)
> @@ -1940,7 +1979,7 @@ static void chv_read_luts(struct intel_crtc_state
> *crtc_state)
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> 
>  	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
> -		crtc_state->hw.gamma_lut = chv_read_cgm_gamma(crtc);
> +		crtc_state->post_csc_lut = chv_read_cgm_gamma(crtc);
>  	else
>  		i965_read_luts(crtc_state);
>  }
> @@ -2007,10 +2046,10 @@ static void ilk_read_luts(struct intel_crtc_state
> *crtc_state)
> 
>  	switch (crtc_state->gamma_mode) {
>  	case GAMMA_MODE_MODE_8BIT:
> -		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc);
> +		crtc_state->post_csc_lut = ilk_read_lut_8(crtc);
>  		break;
>  	case GAMMA_MODE_MODE_10BIT:
> -		crtc_state->hw.gamma_lut = ilk_read_lut_10(crtc);
> +		crtc_state->post_csc_lut = ilk_read_lut_10(crtc);
>  		break;
>  	default:
>  		MISSING_CASE(crtc_state->gamma_mode);
> @@ -2062,10 +2101,10 @@ static void glk_read_luts(struct intel_crtc_state
> *crtc_state)
> 
>  	switch (crtc_state->gamma_mode) {
>  	case GAMMA_MODE_MODE_8BIT:
> -		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc);
> +		crtc_state->post_csc_lut = ilk_read_lut_8(crtc);
>  		break;
>  	case GAMMA_MODE_MODE_10BIT:
> -		crtc_state->hw.gamma_lut = bdw_read_lut_10(crtc,
> PAL_PREC_INDEX_VALUE(0));
> +		crtc_state->post_csc_lut = bdw_read_lut_10(crtc,
> +PAL_PREC_INDEX_VALUE(0));
>  		break;
>  	default:
>  		MISSING_CASE(crtc_state->gamma_mode);
> @@ -2120,13 +2159,13 @@ static void icl_read_luts(struct intel_crtc_state
> *crtc_state)
> 
>  	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
>  	case GAMMA_MODE_MODE_8BIT:
> -		crtc_state->hw.gamma_lut = ilk_read_lut_8(crtc);
> +		crtc_state->post_csc_lut = ilk_read_lut_8(crtc);
>  		break;
>  	case GAMMA_MODE_MODE_10BIT:
> -		crtc_state->hw.gamma_lut = bdw_read_lut_10(crtc,
> PAL_PREC_INDEX_VALUE(0));
> +		crtc_state->post_csc_lut = bdw_read_lut_10(crtc,
> +PAL_PREC_INDEX_VALUE(0));
>  		break;
>  	case GAMMA_MODE_MODE_12BIT_MULTI_SEGMENTED:
> -		crtc_state->hw.gamma_lut = icl_read_lut_multi_segment(crtc);
> +		crtc_state->post_csc_lut = icl_read_lut_multi_segment(crtc);
>  		break;
>  	default:
>  		MISSING_CASE(crtc_state->gamma_mode);
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> index e9212f69c360..98e36ab55e9e 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> @@ -298,11 +298,11 @@ void intel_crtc_state_dump(const struct intel_crtc_state
> *pipe_config,
>  			    pipe_config->csc_mode, pipe_config->gamma_mode,
>  			    pipe_config->gamma_enable, pipe_config->csc_enable);
> 
> -	drm_dbg_kms(&i915->drm, "degamma lut: %d entries, gamma lut: %d
> entries\n",
> -		    pipe_config->hw.degamma_lut ?
> -		    drm_color_lut_size(pipe_config->hw.degamma_lut) : 0,
> -		    pipe_config->hw.gamma_lut ?
> -		    drm_color_lut_size(pipe_config->hw.gamma_lut) : 0);
> +	drm_dbg_kms(&i915->drm, "pre csc lut: %d entries, post csc lut: %d
> entries\n",
> +		    pipe_config->pre_csc_lut ?
> +		    drm_color_lut_size(pipe_config->pre_csc_lut) : 0,
> +		    pipe_config->post_csc_lut ?
> +		    drm_color_lut_size(pipe_config->post_csc_lut) : 0);
> 
>  dump_planes:
>  	if (!state)
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index a103413cb081..20569b6838d1 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5815,7 +5815,7 @@ intel_pipe_config_compare(const struct intel_crtc_state
> *current_config,
> 
>  		bp_gamma = intel_color_get_gamma_bit_precision(pipe_config);
>  		if (bp_gamma)
> -			PIPE_CONF_CHECK_COLOR_LUT(gamma_mode,
> hw.gamma_lut, bp_gamma);
> +			PIPE_CONF_CHECK_COLOR_LUT(gamma_mode,
> post_csc_lut, bp_gamma);
> 
>  		if (current_config->active_planes) {
>  			PIPE_CONF_CHECK_BOOL(has_psr);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h
> b/drivers/gpu/drm/i915/display/intel_display_types.h
> index e2b853e9e51d..cd76236d5fa3 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1001,11 +1001,15 @@ struct intel_crtc_state {
>  	 */
>  	struct {
>  		bool active, enable;
> +		/* logical state of LUTs */
>  		struct drm_property_blob *degamma_lut, *gamma_lut, *ctm;
>  		struct drm_display_mode mode, pipe_mode, adjusted_mode;
>  		enum drm_scaling_filter scaling_filter;
>  	} hw;
> 
> +	/* actual state of LUTs */
> +	struct drm_property_blob *pre_csc_lut, *post_csc_lut;
> +
>  	/**
>  	 * quirks - bitfield with hw state readout quirks
>  	 *
> diff --git a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> index cbfabd58b75a..55ccdafefd27 100644
> --- a/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> +++ b/drivers/gpu/drm/i915/display/intel_modeset_setup.c
> @@ -155,6 +155,12 @@ static void intel_crtc_copy_hw_to_uapi_state(struct
> intel_crtc_state *crtc_state
>  	crtc_state->uapi.adjusted_mode = crtc_state->hw.adjusted_mode;
>  	crtc_state->uapi.scaling_filter = crtc_state->hw.scaling_filter;
> 
> +	/* assume 1:1 mapping */
> +	drm_property_replace_blob(&crtc_state->hw.degamma_lut,
> +				  crtc_state->pre_csc_lut);
> +	drm_property_replace_blob(&crtc_state->hw.gamma_lut,
> +				  crtc_state->post_csc_lut);
> +
>  	drm_property_replace_blob(&crtc_state->uapi.degamma_lut,
>  				  crtc_state->hw.degamma_lut);
>  	drm_property_replace_blob(&crtc_state->uapi.gamma_lut,
> --
> 2.35.1


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

* Re: [Intel-gfx] [PATCH 08/10] drm/i915: Assert {pre, post}_csc_lut were assigned sensibly
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 08/10] drm/i915: Assert {pre, post}_csc_lut were assigned sensibly Ville Syrjala
@ 2022-10-19  7:09   ` Shankar, Uma
  0 siblings, 0 replies; 26+ messages in thread
From: Shankar, Uma @ 2022-10-19  7:09 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala
> Sent: Thursday, September 29, 2022 12:45 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 08/10] drm/i915: Assert {pre, post}_csc_lut were
> assigned sensibly
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Since we now have the extra step from hw.(de)gamma_lut into {pre,post}_csc_lut
> let's make sure we didn't forget to assign them appropriately. Ie. basically making
> sure intel_color_check() was called when necessary (and that it did its job suitable
> well).

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

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c   | 20 ++++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_color.h   |  1 +
>  drivers/gpu/drm/i915/display/intel_display.c |  2 ++
>  3 files changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 380f44720fe6..575d2a23682a 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1355,6 +1355,26 @@ static u32 i9xx_gamma_mode(struct intel_crtc_state
> *crtc_state)
>  		return GAMMA_MODE_MODE_10BIT; /* i965+ only */  }
> 
> +void intel_color_assert_luts(const struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +
> +	/* make sure {pre,post}_csc_lut were correctly assigned */
> +	if (DISPLAY_VER(i915) >= 10 || HAS_GMCH(i915)) {
> +		drm_WARN_ON(&i915->drm,
> +			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut);
> +		drm_WARN_ON(&i915->drm,
> +			    crtc_state->post_csc_lut != crtc_state->hw.gamma_lut);
> +	} else {
> +		drm_WARN_ON(&i915->drm,
> +			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut
> &&
> +			    crtc_state->pre_csc_lut != crtc_state->hw.gamma_lut);
> +		drm_WARN_ON(&i915->drm,
> +			    crtc_state->post_csc_lut != crtc_state->hw.degamma_lut
> &&
> +			    crtc_state->post_csc_lut != crtc_state->hw.gamma_lut);
> +	}
> +}
> +
>  static void intel_assign_luts(struct intel_crtc_state *crtc_state)  {
>  	drm_property_replace_blob(&crtc_state->pre_csc_lut,
> diff --git a/drivers/gpu/drm/i915/display/intel_color.h
> b/drivers/gpu/drm/i915/display/intel_color.h
> index 67702451e2fd..b76f18e6c452 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.h
> +++ b/drivers/gpu/drm/i915/display/intel_color.h
> @@ -24,5 +24,6 @@ int intel_color_get_gamma_bit_precision(const struct
> intel_crtc_state *crtc_stat  bool intel_color_lut_equal(struct drm_property_blob
> *blob1,
>  			   struct drm_property_blob *blob2,
>  			   u32 gamma_mode, u32 bit_precision);
> +void intel_color_assert_luts(const struct intel_crtc_state
> +*crtc_state);
> 
>  #endif /* __INTEL_COLOR_H__ */
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 20569b6838d1..441811ac0ab0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -6906,6 +6906,8 @@ static int intel_atomic_check(struct drm_device *dev,
> 
>  	for_each_oldnew_intel_crtc_in_state(state, crtc, old_crtc_state,
>  					    new_crtc_state, i) {
> +		intel_color_assert_luts(new_crtc_state);
> +
>  		ret = intel_async_flip_check_hw(state, crtc);
>  		if (ret)
>  			goto fail;
> --
> 2.35.1


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

* Re: [Intel-gfx] [PATCH 09/10] drm/i915: Get rid of glk_load_degamma_lut_linear()
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 09/10] drm/i915: Get rid of glk_load_degamma_lut_linear() Ville Syrjala
@ 2022-10-19  7:20   ` Shankar, Uma
  2022-10-19  7:29     ` Ville Syrjälä
  0 siblings, 1 reply; 26+ messages in thread
From: Shankar, Uma @ 2022-10-19  7:20 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala
> Sent: Thursday, September 29, 2022 12:45 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 09/10] drm/i915: Get rid of
> glk_load_degamma_lut_linear()
> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Since we now have a place (pre_csc_lut) to stuff a purely internal LUT we can
> replace glk_load_degamma_lut_linear() with such a thing and just rely on the normal
> glk_load_degamma_lut() to load it as well.
> 
> drm_mode_config_cleanup() will clean this up for us.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c    | 110 +++++++++++-------
>  drivers/gpu/drm/i915/display/intel_color.h    |   1 +
>  drivers/gpu/drm/i915/display/intel_display.c  |   4 +
>  .../gpu/drm/i915/display/intel_display_core.h |   5 +
>  4 files changed, 79 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index 575d2a23682a..de530bf1aba1 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -557,6 +557,32 @@ static void skl_color_commit_arm(const struct
> intel_crtc_state *crtc_state)
>  			  crtc_state->csc_mode);
>  }
> 
> +static struct drm_property_blob *
> +create_linear_lut(struct drm_i915_private *i915, int lut_size) {
> +	struct drm_property_blob *blob;
> +	struct drm_color_lut *lut;
> +	int i;
> +
> +	blob = drm_property_create_blob(&i915->drm,
> +					sizeof(struct drm_color_lut) * lut_size,
> +					NULL);
> +	if (IS_ERR(blob))
> +		return NULL;
> +
> +	lut = blob->data;
> +
> +	for (i = 0; i < lut_size; i++) {
> +		u16 val = 0xffff * i / (lut_size - 1);
> +
> +		lut[i].red = val;
> +		lut[i].green = val;
> +		lut[i].blue = val;
> +	}
> +
> +	return blob;
> +}
> +
>  static void i9xx_load_lut_8(struct intel_crtc *crtc,
>  			    const struct drm_property_blob *blob)  { @@ -871,53
> +897,14 @@ static void glk_load_degamma_lut(const struct intel_crtc_state
> *crtc_state,
>  	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);  }
> 
> -static void glk_load_degamma_lut_linear(const struct intel_crtc_state *crtc_state) -
> {
> -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> -	enum pipe pipe = crtc->pipe;
> -	int i, lut_size = INTEL_INFO(dev_priv)->display.color.degamma_lut_size;
> -
> -	/*
> -	 * When setting the auto-increment bit, the hardware seems to
> -	 * ignore the index bits, so we need to reset it to index 0
> -	 * separately.
> -	 */
> -	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> -	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
> -			  PRE_CSC_GAMC_AUTO_INCREMENT);
> -
> -	for (i = 0; i < lut_size; i++) {
> -		u32 v = (i << 16) / (lut_size - 1);
> -
> -		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe), v);
> -	}
> -
> -	/* Clamp values > 1.0. */
> -	while (i++ < 35)
> -		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe), 1 << 16);
> -
> -	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> -}
> -
>  static void glk_load_luts(const struct intel_crtc_state *crtc_state)  {
>  	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
>  	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> 
> -	/*
> -	 * On GLK+ both pipe CSC and degamma LUT are controlled
> -	 * by csc_enable. Hence for the cases where the CSC is
> -	 * needed but degamma LUT is not we need to load a
> -	 * linear degamma LUT. In fact we'll just always load
> -	 * the degama LUT so that we don't have to reload
> -	 * it every time the pipe CSC is being enabled.
> -	 */
>  	if (pre_csc_lut)
>  		glk_load_degamma_lut(crtc_state, pre_csc_lut);
> -	else
> -		glk_load_degamma_lut_linear(crtc_state);
> 
>  	switch (crtc_state->gamma_mode) {
>  	case GAMMA_MODE_MODE_8BIT:
> @@ -1360,11 +1347,17 @@ void intel_color_assert_luts(const struct
> intel_crtc_state *crtc_state)
>  	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> 
>  	/* make sure {pre,post}_csc_lut were correctly assigned */
> -	if (DISPLAY_VER(i915) >= 10 || HAS_GMCH(i915)) {
> +	if (DISPLAY_VER(i915) >= 11 || HAS_GMCH(i915)) {
>  		drm_WARN_ON(&i915->drm,
>  			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut);
>  		drm_WARN_ON(&i915->drm,
>  			    crtc_state->post_csc_lut != crtc_state->hw.gamma_lut);
> +	} else if (DISPLAY_VER(i915) == 10) {
> +		drm_WARN_ON(&i915->drm,
> +			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut
> &&
> +			    crtc_state->pre_csc_lut != i915-
> >display.color.glk_linear_degamma_lut);
> +		drm_WARN_ON(&i915->drm,
> +			    crtc_state->post_csc_lut != crtc_state->hw.gamma_lut);
>  	} else {
>  		drm_WARN_ON(&i915->drm,
>  			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut
> && @@ -1619,6 +1612,25 @@ static u32 glk_gamma_mode(const struct
> intel_crtc_state *crtc_state)
>  		return GAMMA_MODE_MODE_10BIT;
>  }
> 
> +static void glk_assign_luts(struct intel_crtc_state *crtc_state) {
> +	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> +
> +	intel_assign_luts(crtc_state);
> +
> +	/*
> +	 * On GLK+ both pipe CSC and degamma LUT are controlled
> +	 * by csc_enable. Hence for the cases where the CSC is
> +	 * needed but degamma LUT is not we need to load a
> +	 * linear degamma LUT. In fact we'll just always load
> +	 * the degama LUT so that we don't have to reload
> +	 * it every time the pipe CSC is being enabled.
> +	 */
> +	if (!crtc_state->pre_csc_lut)
> +		drm_property_replace_blob(&crtc_state->pre_csc_lut,
> +					  i915-
> >display.color.glk_linear_degamma_lut);
> +}
> +
>  static int glk_color_check(struct intel_crtc_state *crtc_state)  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> @@ -1653,7 +1665,7 @@ static int glk_color_check(struct intel_crtc_state
> *crtc_state)
>  	if (ret)
>  		return ret;
> 
> -	intel_assign_luts(crtc_state);
> +	glk_assign_luts(crtc_state);
> 
>  	crtc_state->preload_luts = glk_can_preload_luts(crtc_state);
> 
> @@ -2283,6 +2295,22 @@ void intel_crtc_color_init(struct intel_crtc *crtc)
>  				   INTEL_INFO(dev_priv)-
> >display.color.gamma_lut_size);
>  }
> 
> +int intel_color_init(struct drm_i915_private *i915) {
> +	struct drm_property_blob *blob;
> +
> +	if (DISPLAY_VER(i915) != 10)
> +		return 0;

This is very specific to Gen10. Should we rename intel_color_init which sounds more global
to a gen10 specific name.

Will leave that to your discretion, don't see any issues logically
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> +	blob = create_linear_lut(i915, INTEL_INFO(i915)-
> >display.color.degamma_lut_size);
> +	if (IS_ERR(blob))
> +		return PTR_ERR(blob);
> +
> +	i915->display.color.glk_linear_degamma_lut = blob;
> +
> +	return 0;
> +}
> +
>  void intel_color_init_hooks(struct drm_i915_private *i915)  {
>  	if (HAS_GMCH(i915)) {
> diff --git a/drivers/gpu/drm/i915/display/intel_color.h
> b/drivers/gpu/drm/i915/display/intel_color.h
> index b76f18e6c452..f7b0591abd73 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.h
> +++ b/drivers/gpu/drm/i915/display/intel_color.h
> @@ -14,6 +14,7 @@ struct drm_i915_private;  struct drm_property_blob;
> 
>  void intel_color_init_hooks(struct drm_i915_private *i915);
> +int intel_color_init(struct drm_i915_private *i915);
>  void intel_crtc_color_init(struct intel_crtc *crtc);  int intel_color_check(struct
> intel_crtc_state *crtc_state);  void intel_color_commit_noarm(const struct
> intel_crtc_state *crtc_state); diff --git
> a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 441811ac0ab0..95cbc2e2d3c2 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -8659,6 +8659,10 @@ int intel_modeset_init_noirq(struct drm_i915_private
> *i915)
>  	if (ret)
>  		goto cleanup_vga_client_pw_domain_dmc;
> 
> +	ret = intel_color_init(i915);
> +	if (ret)
> +		goto cleanup_vga_client_pw_domain_dmc;
> +
>  	ret = intel_dbuf_init(i915);
>  	if (ret)
>  		goto cleanup_vga_client_pw_domain_dmc; diff --git
> a/drivers/gpu/drm/i915/display/intel_display_core.h
> b/drivers/gpu/drm/i915/display/intel_display_core.h
> index 96cf994b0ad1..b4b9c4cef78e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> @@ -28,6 +28,7 @@
> 
>  struct drm_i915_private;
>  struct drm_property;
> +struct drm_property_blob;
>  struct i915_audio_component;
>  struct i915_hdcp_comp_master;
>  struct intel_atomic_state;
> @@ -308,6 +309,10 @@ struct intel_display {
>  		unsigned int max_cdclk_freq;
>  	} cdclk;
> 
> +	struct {
> +		struct drm_property_blob *glk_linear_degamma_lut;
> +	} color;
> +
>  	struct {
>  		/* The current hardware dbuf configuration */
>  		u8 enabled_slices;
> --
> 2.35.1


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

* Re: [Intel-gfx] [PATCH 10/10] drm/i915: Stop loading linear degammma LUT on glk needlessly
  2022-09-29  7:15 ` [Intel-gfx] [PATCH 10/10] drm/i915: Stop loading linear degammma LUT on glk needlessly Ville Syrjala
@ 2022-10-19  7:24   ` Shankar, Uma
  0 siblings, 0 replies; 26+ messages in thread
From: Shankar, Uma @ 2022-10-19  7:24 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala
> Sent: Thursday, September 29, 2022 12:45 PM
> To: intel-gfx@lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 10/10] drm/i915: Stop loading linear degammma LUT on
> glk needlessly

Typo in degamma

> 
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Make glk_load_luts() a bit lighter for the common case where neither the degamma
> LUT nor pipe CSC are enabled by not loading the linear degamma LUT. Making
> .load_luts() as lightweight as possible is a good idea since it may need to execute
> from a vblank worker under tight deadlines.
> 
> My earlier reasoning for always loading the linear degamma LUT was to avoid an
> extra LUT load when just enabling/disabling the pipe CSC, but that is nonsense since
> we load the LUTs on every flagged color manaement change/modeset anyway

Nit pick: Typo in management.

With above fixed,
Reviewed-by: Uma Shankar <uma.shankar@intel.com>

> (either of which is needed for a pipe CSC toggle).
> 
> We can also get rid of the glk_can_preload_luts() special case since the presence of
> the degamma LUT will now always match csc_enable.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 26 +++-------------------
>  1 file changed, 3 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> b/drivers/gpu/drm/i915/display/intel_color.c
> index de530bf1aba1..a3066d942f58 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1198,24 +1198,6 @@ static bool chv_can_preload_luts(const struct
> intel_crtc_state *new_crtc_state)
>  	return !old_crtc_state->post_csc_lut;
>  }
> 
> -static bool glk_can_preload_luts(const struct intel_crtc_state *new_crtc_state) -{
> -	struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->uapi.crtc);
> -	struct intel_atomic_state *state =
> -		to_intel_atomic_state(new_crtc_state->uapi.state);
> -	const struct intel_crtc_state *old_crtc_state =
> -		intel_atomic_get_old_crtc_state(state, crtc);
> -
> -	/*
> -	 * The hardware degamma is active whenever the pipe
> -	 * CSC is active. Thus even if the old state has no
> -	 * software degamma we need to avoid clobbering the
> -	 * linear hardware degamma mid scanout.
> -	 */
> -	return !old_crtc_state->csc_enable &&
> -		!old_crtc_state->post_csc_lut;
> -}
> -
>  int intel_color_check(struct intel_crtc_state *crtc_state)  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> @@ -1622,11 +1604,9 @@ static void glk_assign_luts(struct intel_crtc_state
> *crtc_state)
>  	 * On GLK+ both pipe CSC and degamma LUT are controlled
>  	 * by csc_enable. Hence for the cases where the CSC is
>  	 * needed but degamma LUT is not we need to load a
> -	 * linear degamma LUT. In fact we'll just always load
> -	 * the degama LUT so that we don't have to reload
> -	 * it every time the pipe CSC is being enabled.
> +	 * linear degamma LUT.
>  	 */
> -	if (!crtc_state->pre_csc_lut)
> +	if (crtc_state->csc_enable && !crtc_state->pre_csc_lut)
>  		drm_property_replace_blob(&crtc_state->pre_csc_lut,
>  					  i915-
> >display.color.glk_linear_degamma_lut);
>  }
> @@ -1667,7 +1647,7 @@ static int glk_color_check(struct intel_crtc_state
> *crtc_state)
> 
>  	glk_assign_luts(crtc_state);
> 
> -	crtc_state->preload_luts = glk_can_preload_luts(crtc_state);
> +	crtc_state->preload_luts = intel_can_preload_luts(crtc_state);
> 
>  	return 0;
>  }
> --
> 2.35.1


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

* Re: [Intel-gfx] [PATCH 09/10] drm/i915: Get rid of glk_load_degamma_lut_linear()
  2022-10-19  7:20   ` Shankar, Uma
@ 2022-10-19  7:29     ` Ville Syrjälä
  2022-10-19  7:49       ` Shankar, Uma
  0 siblings, 1 reply; 26+ messages in thread
From: Ville Syrjälä @ 2022-10-19  7:29 UTC (permalink / raw)
  To: Shankar, Uma; +Cc: intel-gfx

On Wed, Oct 19, 2022 at 07:20:13AM +0000, Shankar, Uma wrote:
> 
> 
> > -----Original Message-----
> > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Ville Syrjala
> > Sent: Thursday, September 29, 2022 12:45 PM
> > To: intel-gfx@lists.freedesktop.org
> > Subject: [Intel-gfx] [PATCH 09/10] drm/i915: Get rid of
> > glk_load_degamma_lut_linear()
> > 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Since we now have a place (pre_csc_lut) to stuff a purely internal LUT we can
> > replace glk_load_degamma_lut_linear() with such a thing and just rely on the normal
> > glk_load_degamma_lut() to load it as well.
> > 
> > drm_mode_config_cleanup() will clean this up for us.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_color.c    | 110 +++++++++++-------
> >  drivers/gpu/drm/i915/display/intel_color.h    |   1 +
> >  drivers/gpu/drm/i915/display/intel_display.c  |   4 +
> >  .../gpu/drm/i915/display/intel_display_core.h |   5 +
> >  4 files changed, 79 insertions(+), 41 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> > b/drivers/gpu/drm/i915/display/intel_color.c
> > index 575d2a23682a..de530bf1aba1 100644
> > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > @@ -557,6 +557,32 @@ static void skl_color_commit_arm(const struct
> > intel_crtc_state *crtc_state)
> >  			  crtc_state->csc_mode);
> >  }
> > 
> > +static struct drm_property_blob *
> > +create_linear_lut(struct drm_i915_private *i915, int lut_size) {
> > +	struct drm_property_blob *blob;
> > +	struct drm_color_lut *lut;
> > +	int i;
> > +
> > +	blob = drm_property_create_blob(&i915->drm,
> > +					sizeof(struct drm_color_lut) * lut_size,
> > +					NULL);
> > +	if (IS_ERR(blob))
> > +		return NULL;
> > +
> > +	lut = blob->data;
> > +
> > +	for (i = 0; i < lut_size; i++) {
> > +		u16 val = 0xffff * i / (lut_size - 1);
> > +
> > +		lut[i].red = val;
> > +		lut[i].green = val;
> > +		lut[i].blue = val;
> > +	}
> > +
> > +	return blob;
> > +}
> > +
> >  static void i9xx_load_lut_8(struct intel_crtc *crtc,
> >  			    const struct drm_property_blob *blob)  { @@ -871,53
> > +897,14 @@ static void glk_load_degamma_lut(const struct intel_crtc_state
> > *crtc_state,
> >  	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);  }
> > 
> > -static void glk_load_degamma_lut_linear(const struct intel_crtc_state *crtc_state) -
> > {
> > -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > -	enum pipe pipe = crtc->pipe;
> > -	int i, lut_size = INTEL_INFO(dev_priv)->display.color.degamma_lut_size;
> > -
> > -	/*
> > -	 * When setting the auto-increment bit, the hardware seems to
> > -	 * ignore the index bits, so we need to reset it to index 0
> > -	 * separately.
> > -	 */
> > -	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> > -	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
> > -			  PRE_CSC_GAMC_AUTO_INCREMENT);
> > -
> > -	for (i = 0; i < lut_size; i++) {
> > -		u32 v = (i << 16) / (lut_size - 1);
> > -
> > -		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe), v);
> > -	}
> > -
> > -	/* Clamp values > 1.0. */
> > -	while (i++ < 35)
> > -		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe), 1 << 16);
> > -
> > -	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> > -}
> > -
> >  static void glk_load_luts(const struct intel_crtc_state *crtc_state)  {
> >  	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
> >  	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
> >  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > 
> > -	/*
> > -	 * On GLK+ both pipe CSC and degamma LUT are controlled
> > -	 * by csc_enable. Hence for the cases where the CSC is
> > -	 * needed but degamma LUT is not we need to load a
> > -	 * linear degamma LUT. In fact we'll just always load
> > -	 * the degama LUT so that we don't have to reload
> > -	 * it every time the pipe CSC is being enabled.
> > -	 */
> >  	if (pre_csc_lut)
> >  		glk_load_degamma_lut(crtc_state, pre_csc_lut);
> > -	else
> > -		glk_load_degamma_lut_linear(crtc_state);
> > 
> >  	switch (crtc_state->gamma_mode) {
> >  	case GAMMA_MODE_MODE_8BIT:
> > @@ -1360,11 +1347,17 @@ void intel_color_assert_luts(const struct
> > intel_crtc_state *crtc_state)
> >  	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> > 
> >  	/* make sure {pre,post}_csc_lut were correctly assigned */
> > -	if (DISPLAY_VER(i915) >= 10 || HAS_GMCH(i915)) {
> > +	if (DISPLAY_VER(i915) >= 11 || HAS_GMCH(i915)) {
> >  		drm_WARN_ON(&i915->drm,
> >  			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut);
> >  		drm_WARN_ON(&i915->drm,
> >  			    crtc_state->post_csc_lut != crtc_state->hw.gamma_lut);
> > +	} else if (DISPLAY_VER(i915) == 10) {
> > +		drm_WARN_ON(&i915->drm,
> > +			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut
> > &&
> > +			    crtc_state->pre_csc_lut != i915-
> > >display.color.glk_linear_degamma_lut);
> > +		drm_WARN_ON(&i915->drm,
> > +			    crtc_state->post_csc_lut != crtc_state->hw.gamma_lut);
> >  	} else {
> >  		drm_WARN_ON(&i915->drm,
> >  			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut
> > && @@ -1619,6 +1612,25 @@ static u32 glk_gamma_mode(const struct
> > intel_crtc_state *crtc_state)
> >  		return GAMMA_MODE_MODE_10BIT;
> >  }
> > 
> > +static void glk_assign_luts(struct intel_crtc_state *crtc_state) {
> > +	struct drm_i915_private *i915 = to_i915(crtc_state->uapi.crtc->dev);
> > +
> > +	intel_assign_luts(crtc_state);
> > +
> > +	/*
> > +	 * On GLK+ both pipe CSC and degamma LUT are controlled
> > +	 * by csc_enable. Hence for the cases where the CSC is
> > +	 * needed but degamma LUT is not we need to load a
> > +	 * linear degamma LUT. In fact we'll just always load
> > +	 * the degama LUT so that we don't have to reload
> > +	 * it every time the pipe CSC is being enabled.
> > +	 */
> > +	if (!crtc_state->pre_csc_lut)
> > +		drm_property_replace_blob(&crtc_state->pre_csc_lut,
> > +					  i915-
> > >display.color.glk_linear_degamma_lut);
> > +}
> > +
> >  static int glk_color_check(struct intel_crtc_state *crtc_state)  {
> >  	struct drm_i915_private *dev_priv = to_i915(crtc_state->uapi.crtc->dev);
> > @@ -1653,7 +1665,7 @@ static int glk_color_check(struct intel_crtc_state
> > *crtc_state)
> >  	if (ret)
> >  		return ret;
> > 
> > -	intel_assign_luts(crtc_state);
> > +	glk_assign_luts(crtc_state);
> > 
> >  	crtc_state->preload_luts = glk_can_preload_luts(crtc_state);
> > 
> > @@ -2283,6 +2295,22 @@ void intel_crtc_color_init(struct intel_crtc *crtc)
> >  				   INTEL_INFO(dev_priv)-
> > >display.color.gamma_lut_size);
> >  }
> > 
> > +int intel_color_init(struct drm_i915_private *i915) {
> > +	struct drm_property_blob *blob;
> > +
> > +	if (DISPLAY_VER(i915) != 10)
> > +		return 0;
> 
> This is very specific to Gen10. Should we rename intel_color_init which sounds more global
> to a gen10 specific name.

I guess I could have added a glk specific function for this and
call it from intel_color_init(), but should be easy enough to do
that refcatoring if/when someone needs to add more stuff to
intel_color_init().

> 
> Will leave that to your discretion, don't see any issues logically
> Reviewed-by: Uma Shankar <uma.shankar@intel.com>

Thanks.

-- 
Ville Syrjälä
Intel

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

* Re: [Intel-gfx] [PATCH 09/10] drm/i915: Get rid of glk_load_degamma_lut_linear()
  2022-10-19  7:29     ` Ville Syrjälä
@ 2022-10-19  7:49       ` Shankar, Uma
  0 siblings, 0 replies; 26+ messages in thread
From: Shankar, Uma @ 2022-10-19  7:49 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx



> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Sent: Wednesday, October 19, 2022 1:00 PM
> To: Shankar, Uma <uma.shankar@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH 09/10] drm/i915: Get rid of
> glk_load_degamma_lut_linear()
> 
> On Wed, Oct 19, 2022 at 07:20:13AM +0000, Shankar, Uma wrote:
> >
> >
> > > -----Original Message-----
> > > From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf
> > > Of Ville Syrjala
> > > Sent: Thursday, September 29, 2022 12:45 PM
> > > To: intel-gfx@lists.freedesktop.org
> > > Subject: [Intel-gfx] [PATCH 09/10] drm/i915: Get rid of
> > > glk_load_degamma_lut_linear()
> > >
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Since we now have a place (pre_csc_lut) to stuff a purely internal
> > > LUT we can replace glk_load_degamma_lut_linear() with such a thing
> > > and just rely on the normal
> > > glk_load_degamma_lut() to load it as well.
> > >
> > > drm_mode_config_cleanup() will clean this up for us.
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_color.c    | 110 +++++++++++-------
> > >  drivers/gpu/drm/i915/display/intel_color.h    |   1 +
> > >  drivers/gpu/drm/i915/display/intel_display.c  |   4 +
> > >  .../gpu/drm/i915/display/intel_display_core.h |   5 +
> > >  4 files changed, 79 insertions(+), 41 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> > > b/drivers/gpu/drm/i915/display/intel_color.c
> > > index 575d2a23682a..de530bf1aba1 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > > @@ -557,6 +557,32 @@ static void skl_color_commit_arm(const struct
> > > intel_crtc_state *crtc_state)
> > >  			  crtc_state->csc_mode);
> > >  }
> > >
> > > +static struct drm_property_blob *
> > > +create_linear_lut(struct drm_i915_private *i915, int lut_size) {
> > > +	struct drm_property_blob *blob;
> > > +	struct drm_color_lut *lut;
> > > +	int i;
> > > +
> > > +	blob = drm_property_create_blob(&i915->drm,
> > > +					sizeof(struct drm_color_lut) * lut_size,
> > > +					NULL);
> > > +	if (IS_ERR(blob))
> > > +		return NULL;
> > > +
> > > +	lut = blob->data;
> > > +
> > > +	for (i = 0; i < lut_size; i++) {
> > > +		u16 val = 0xffff * i / (lut_size - 1);
> > > +
> > > +		lut[i].red = val;
> > > +		lut[i].green = val;
> > > +		lut[i].blue = val;
> > > +	}
> > > +
> > > +	return blob;
> > > +}
> > > +
> > >  static void i9xx_load_lut_8(struct intel_crtc *crtc,
> > >  			    const struct drm_property_blob *blob)  { @@ -871,53
> > > +897,14 @@ static void glk_load_degamma_lut(const struct
> > > +intel_crtc_state
> > > *crtc_state,
> > >  	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);  }
> > >
> > > -static void glk_load_degamma_lut_linear(const struct
> > > intel_crtc_state *crtc_state) - {
> > > -	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > > -	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> > > -	enum pipe pipe = crtc->pipe;
> > > -	int i, lut_size = INTEL_INFO(dev_priv)->display.color.degamma_lut_size;
> > > -
> > > -	/*
> > > -	 * When setting the auto-increment bit, the hardware seems to
> > > -	 * ignore the index bits, so we need to reset it to index 0
> > > -	 * separately.
> > > -	 */
> > > -	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> > > -	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe),
> > > -			  PRE_CSC_GAMC_AUTO_INCREMENT);
> > > -
> > > -	for (i = 0; i < lut_size; i++) {
> > > -		u32 v = (i << 16) / (lut_size - 1);
> > > -
> > > -		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe), v);
> > > -	}
> > > -
> > > -	/* Clamp values > 1.0. */
> > > -	while (i++ < 35)
> > > -		intel_de_write_fw(dev_priv, PRE_CSC_GAMC_DATA(pipe), 1 << 16);
> > > -
> > > -	intel_de_write_fw(dev_priv, PRE_CSC_GAMC_INDEX(pipe), 0);
> > > -}
> > > -
> > >  static void glk_load_luts(const struct intel_crtc_state *crtc_state)  {
> > >  	const struct drm_property_blob *pre_csc_lut = crtc_state->pre_csc_lut;
> > >  	const struct drm_property_blob *post_csc_lut = crtc_state->post_csc_lut;
> > >  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> > >
> > > -	/*
> > > -	 * On GLK+ both pipe CSC and degamma LUT are controlled
> > > -	 * by csc_enable. Hence for the cases where the CSC is
> > > -	 * needed but degamma LUT is not we need to load a
> > > -	 * linear degamma LUT. In fact we'll just always load
> > > -	 * the degama LUT so that we don't have to reload
> > > -	 * it every time the pipe CSC is being enabled.
> > > -	 */
> > >  	if (pre_csc_lut)
> > >  		glk_load_degamma_lut(crtc_state, pre_csc_lut);
> > > -	else
> > > -		glk_load_degamma_lut_linear(crtc_state);
> > >
> > >  	switch (crtc_state->gamma_mode) {
> > >  	case GAMMA_MODE_MODE_8BIT:
> > > @@ -1360,11 +1347,17 @@ void intel_color_assert_luts(const struct
> > > intel_crtc_state *crtc_state)
> > >  	struct drm_i915_private *i915 =
> > > to_i915(crtc_state->uapi.crtc->dev);
> > >
> > >  	/* make sure {pre,post}_csc_lut were correctly assigned */
> > > -	if (DISPLAY_VER(i915) >= 10 || HAS_GMCH(i915)) {
> > > +	if (DISPLAY_VER(i915) >= 11 || HAS_GMCH(i915)) {
> > >  		drm_WARN_ON(&i915->drm,
> > >  			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut);
> > >  		drm_WARN_ON(&i915->drm,
> > >  			    crtc_state->post_csc_lut != crtc_state->hw.gamma_lut);
> > > +	} else if (DISPLAY_VER(i915) == 10) {
> > > +		drm_WARN_ON(&i915->drm,
> > > +			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut
> > > &&
> > > +			    crtc_state->pre_csc_lut != i915-
> > > >display.color.glk_linear_degamma_lut);
> > > +		drm_WARN_ON(&i915->drm,
> > > +			    crtc_state->post_csc_lut != crtc_state->hw.gamma_lut);
> > >  	} else {
> > >  		drm_WARN_ON(&i915->drm,
> > >  			    crtc_state->pre_csc_lut != crtc_state->hw.degamma_lut
> && @@
> > > -1619,6 +1612,25 @@ static u32 glk_gamma_mode(const struct
> > > intel_crtc_state *crtc_state)
> > >  		return GAMMA_MODE_MODE_10BIT;
> > >  }
> > >
> > > +static void glk_assign_luts(struct intel_crtc_state *crtc_state) {
> > > +	struct drm_i915_private *i915 =
> > > +to_i915(crtc_state->uapi.crtc->dev);
> > > +
> > > +	intel_assign_luts(crtc_state);
> > > +
> > > +	/*
> > > +	 * On GLK+ both pipe CSC and degamma LUT are controlled
> > > +	 * by csc_enable. Hence for the cases where the CSC is
> > > +	 * needed but degamma LUT is not we need to load a
> > > +	 * linear degamma LUT. In fact we'll just always load
> > > +	 * the degama LUT so that we don't have to reload
> > > +	 * it every time the pipe CSC is being enabled.
> > > +	 */
> > > +	if (!crtc_state->pre_csc_lut)
> > > +		drm_property_replace_blob(&crtc_state->pre_csc_lut,
> > > +					  i915-
> > > >display.color.glk_linear_degamma_lut);
> > > +}
> > > +
> > >  static int glk_color_check(struct intel_crtc_state *crtc_state)  {
> > >  	struct drm_i915_private *dev_priv =
> > > to_i915(crtc_state->uapi.crtc->dev);
> > > @@ -1653,7 +1665,7 @@ static int glk_color_check(struct
> > > intel_crtc_state
> > > *crtc_state)
> > >  	if (ret)
> > >  		return ret;
> > >
> > > -	intel_assign_luts(crtc_state);
> > > +	glk_assign_luts(crtc_state);
> > >
> > >  	crtc_state->preload_luts = glk_can_preload_luts(crtc_state);
> > >
> > > @@ -2283,6 +2295,22 @@ void intel_crtc_color_init(struct intel_crtc *crtc)
> > >  				   INTEL_INFO(dev_priv)-
> > > >display.color.gamma_lut_size);
> > >  }
> > >
> > > +int intel_color_init(struct drm_i915_private *i915) {
> > > +	struct drm_property_blob *blob;
> > > +
> > > +	if (DISPLAY_VER(i915) != 10)
> > > +		return 0;
> >
> > This is very specific to Gen10. Should we rename intel_color_init
> > which sounds more global to a gen10 specific name.
> 
> I guess I could have added a glk specific function for this and call it from
> intel_color_init(), but should be easy enough to do that refcatoring if/when
> someone needs to add more stuff to intel_color_init().

Yeah, fair enough.

Regards,
Uma Shankar
> >
> > Will leave that to your discretion, don't see any issues logically
> > Reviewed-by: Uma Shankar <uma.shankar@intel.com>
> 
> Thanks.
> 
> --
> Ville Syrjälä
> Intel

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

end of thread, other threads:[~2022-10-19  7:50 UTC | newest]

Thread overview: 26+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-29  7:15 [Intel-gfx] [PATCH 00/10] drm/i915: Prep work for finishing (de)gamma readout Ville Syrjala
2022-09-29  7:15 ` [Intel-gfx] [PATCH 01/10] drm/i915: Remove PLL asserts from .load_luts() Ville Syrjala
2022-09-29 11:54   ` Jani Nikula
2022-09-29  7:15 ` [Intel-gfx] [PATCH 02/10] drm/i915: Split up intel_color_init() Ville Syrjala
2022-09-29 10:41   ` Jani Nikula
2022-09-29 11:55     ` Jani Nikula
2022-09-29  7:15 ` [Intel-gfx] [PATCH 03/10] drm/i915: Simplify the intel_color_init_hooks() if ladder Ville Syrjala
2022-09-29 11:56   ` Jani Nikula
2022-09-29  7:15 ` [Intel-gfx] [PATCH 04/10] drm/i915: Clean up intel_color_init_hooks() Ville Syrjala
2022-09-29 11:57   ` Jani Nikula
2022-09-29  7:15 ` [Intel-gfx] [PATCH 05/10] drm/i915: Change glk_load_degamma_lut() calling convention Ville Syrjala
2022-09-29 11:58   ` Jani Nikula
2022-09-29  7:15 ` [Intel-gfx] [PATCH 06/10] drm/i915: Make ilk_load_luts() deal with degamma Ville Syrjala
2022-09-29  7:15 ` [Intel-gfx] [PATCH 07/10] drm/i915: Introduce crtc_state->{pre, post}_csc_lut Ville Syrjala
2022-10-19  7:06   ` Shankar, Uma
2022-09-29  7:15 ` [Intel-gfx] [PATCH 08/10] drm/i915: Assert {pre, post}_csc_lut were assigned sensibly Ville Syrjala
2022-10-19  7:09   ` Shankar, Uma
2022-09-29  7:15 ` [Intel-gfx] [PATCH 09/10] drm/i915: Get rid of glk_load_degamma_lut_linear() Ville Syrjala
2022-10-19  7:20   ` Shankar, Uma
2022-10-19  7:29     ` Ville Syrjälä
2022-10-19  7:49       ` Shankar, Uma
2022-09-29  7:15 ` [Intel-gfx] [PATCH 10/10] drm/i915: Stop loading linear degammma LUT on glk needlessly Ville Syrjala
2022-10-19  7:24   ` Shankar, Uma
2022-09-29  9:19 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Prep work for finishing (de)gamma readout Patchwork
2022-09-29  9:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-09-30  6:35 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

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