[PATCH 00/11] drm/i915: adding state checker for gamma lut values

[PATCH 00/11] drm/i915: adding state checker for gamma lut values

[Swati Sharma]

Thanks to Jani N, Matt and Ville for the review comments.

In this patch series, added state checker to validate gamma and degamma lut values.
This reads hardware state, and compares the originally requested state to the state read
from hardware.

v1: -Implementation done for legacy platforms (removed all the placeholders) (Jani)
    -Added inverse function of drm_color_lut_extract to convert hardware
     read values back to user values (code written by Jani)
    -Renamed get_config() to color_config() (Jani)
    -Placed all platform specific shifts and masks in i915_reg.h (Jani)
    -Renamed i9xx_get_config to i9xx_color_config and all related
     functions (Jani)
    -Removed debug logs from compare function (Jani)
    -Renamed intel_compare_blob to intel_compare_lut and added platform specific
     bit precision of the readout into the function (Jani)
    -Renamed macro PIPE_CONF_CHECK_BLOB to PIPE_CONF_CHECK_COLOR_LUT (Jani)
    -Added check if blobs can be NULL (Jani)
    -Added function in intel_color.c that returns the bit precision (Jani),
     didn't add in device info since its gonna die soon (Ville)

v2: -Moved intel_compare_lut() from intel_display.c to intel_color.c (Jani)
    -Changed color_config() func names to get_color_config() and same
     for gamma func too (Jani)
    -Removed blank line in i915_reg.h and aligned MASK with their
     register name (Jani)
    -Mask definition code indented and defined using REG_GENMASK() and
     used using REG_FIELD_GET() (Jani)
    -Made bit_precision func inline (Jani/Matt)
    -Assigned bit_precision according to GAMMA_MODE (Matt/Ville)
    -Changed IS_ERR(blob) condition to (!blob) (Jani)
    -Rearranged blob check and lut_size conditions (Jani)
    -Used inline function for the comparison (Jani)
    -Separated the get config part from the state checker part to
     another patch (Jani)
    -Retained "internal" i9xx_internal_gamma_config for consistency
     (Matt)
    -Removed crtc_state_is_legacy_gamma check and replaced with
     GAMMA_MODE (Matt)
    -Created whole platform specific patch series as submitted by Ville for
     clean up intel_color_check()
    -Rebased on top of Ville's changes

v3: -Rebase

v4: -Renamed intel_get_color_config to intel_color_get_config (Jani)
    -Wrapped get_color_config() (Jani)
    -Added the user early on such that support for get_color_config()
     can be added platform by platform incrementally (Jani)
    -Few changes in get_config_func (Jani)
    -Renamed intel_compare_color_lut() to intel_color_lut_equal() (Jani)
    -Corrected smatch warn "variable dereferenced before check" (Dan Carpenter)

v5: -Added degamma validation (Ville)
    -Few other changes as suggested by Ville and Jani

Swati Sharma (11):
  drm/i915: Introduce vfunc read_luts() to create hw lut
  drm/i915: Enable intel_color_read_luts()
  drm/i915: Extract i9xx_read_luts()
  drm/i915: Extract chv_read_luts()
  drm/i915: Extract i965_read_luts()
  drm/i915: Extract icl_read_luts()
  drm/i915: Extract glk_read_luts()
  drm/i915: Extract bdw_read_luts()
  drm/i915: Extract ivb_read_luts()
  drm/i915: Extract ilk_read_luts()
  drm/i915: Add intel_color_lut_equal() to compare hw and sw
    gamma/degamma lut values

 drivers/gpu/drm/i915/i915_drv.h      |   1 +
 drivers/gpu/drm/i915/i915_reg.h      |  19 ++
 drivers/gpu/drm/i915/intel_color.c   | 460 ++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_color.h   |   7 +
 drivers/gpu/drm/i915/intel_display.c |  16 ++
 5 files changed, 498 insertions(+), 5 deletions(-)

-- 
1.9.1

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

[v5][PATCH 01/11] drm/i915: Introduce vfunc read_luts() to create hw lut

[Swati Sharma]

In this patch, a vfunc read_luts() is introduced to create a hw lut
i.e. lut having values read from gamma/degamma registers which will
later be used to compare with sw lut to validate gamma/degamma lut values.

v3: -Rebase
v4: -Renamed intel_get_color_config to intel_color_get_config [Jani]
    -Wrapped get_color_config() [Jani]
v5: -Renamed intel_color_get_config() to intel_color_read_luts()
    -Renamed get_color_config to read_luts

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h    | 1 +
 drivers/gpu/drm/i915/intel_color.c | 8 ++++++++
 drivers/gpu/drm/i915/intel_color.h | 1 +
 3 files changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 1cea98f..1b6d891 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -342,6 +342,7 @@ struct drm_i915_display_funcs {
 	 * involved with the same commit.
 	 */
 	void (*load_luts)(const struct intel_crtc_state *crtc_state);
+	void (*read_luts)(struct intel_crtc_state *crtc_state);
 };
 
 #define CSR_VERSION(major, minor)	((major) << 16 | (minor))
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 962db12..0048d8a 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -879,6 +879,14 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
 	return dev_priv->display.color_check(crtc_state);
 }
 
+void intel_color_read_luts(struct intel_crtc_state *crtc_state)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
+
+	if (dev_priv->display.read_luts)
+		dev_priv->display.read_luts(crtc_state);
+}
+
 static bool need_plane_update(struct intel_plane *plane,
 			      const struct intel_crtc_state *crtc_state)
 {
diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h
index b8a3ce6..fc53de9 100644
--- a/drivers/gpu/drm/i915/intel_color.h
+++ b/drivers/gpu/drm/i915/intel_color.h
@@ -13,5 +13,6 @@
 int intel_color_check(struct intel_crtc_state *crtc_state);
 void intel_color_commit(const struct intel_crtc_state *crtc_state);
 void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
+void intel_color_read_luts(struct intel_crtc_state *crtc_state);
 
 #endif /* __INTEL_COLOR_H__ */
-- 
1.9.1

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

[v5][PATCH 02/11] drm/i915: Enable intel_color_read_luts()

[Swati Sharma]

In this patch, intel_color_read_luts() is enabled and support
for read_luts() will be added platform by platform incrementally
in the follow-up patches.

v4: -Renamed intel_get_color_config to intel_color_get_config [Jani]
    -Added the user early on such that support for get_color_config()
     can be added platform by platform incrementally [Jani]
v5: -Incorrect place for calling intel_color_get_config() in
     haswell_get_pipe_config() [Ville]

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5453dbe..791974b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8327,6 +8327,7 @@ static bool i9xx_get_pipe_config(struct intel_crtc *crtc,
 		pipe_config->cgm_mode = I915_READ(CGM_PIPE_MODE(crtc->pipe));
 
 	i9xx_get_pipe_color_config(pipe_config);
+	intel_color_read_luts(pipe_config);
 
 	if (INTEL_GEN(dev_priv) < 4)
 		pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
@@ -9400,6 +9401,7 @@ static bool ironlake_get_pipe_config(struct intel_crtc *crtc,
 	pipe_config->csc_mode = I915_READ(PIPE_CSC_MODE(crtc->pipe));
 
 	i9xx_get_pipe_color_config(pipe_config);
+	intel_color_read_luts(pipe_config);
 
 	if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
 		struct intel_shared_dpll *pll;
@@ -10063,6 +10065,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 		i9xx_get_pipe_color_config(pipe_config);
 	}
 
+	intel_color_read_luts(pipe_config);
+
 	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
 	WARN_ON(power_domain_mask & BIT_ULL(power_domain));
 
-- 
1.9.1

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

[v5][PATCH 03/11] drm/i915: Extract i9xx_read_luts()

[Swati Sharma]

In this patch, hw gamma blob is created for the legacy
gamma. Also, function intel_color_lut_pack is added to
convert hw value with given bit_precision to lut property val.

v4: -No need to initialize *blob [Jani]
    -Removed right shifts [Jani]
    -Dropped dev local var [Jani]
v5: -Returned blob instead of assigning it internally withing the
     function [Ville]
    -Renamed function i9xx_get_color_config() to i9xx_read_luts()
    -Renamed i9xx_get_config_internal() to i9xx_read_lut_8() [Ville]

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |  3 +++
 drivers/gpu/drm/i915/intel_color.c | 51 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6f0a086..2e99025 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7178,6 +7178,9 @@ enum {
 /* legacy palette */
 #define _LGC_PALETTE_A           0x4a000
 #define _LGC_PALETTE_B           0x4a800
+#define LGC_PALETTE_RED_MASK     REG_GENMASK(23, 16)
+#define LGC_PALETTE_GREEN_MASK   REG_GENMASK(15, 8)
+#define LGC_PALETTE_BLUE_MASK    REG_GENMASK(7, 0)
 #define LGC_PALETTE(pipe, i) _MMIO(_PIPE(pipe, _LGC_PALETTE_A, _LGC_PALETTE_B) + (i) * 4)
 
 /* ilk/snb precision palette */
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 0048d8a..4915b19 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1251,6 +1251,56 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
 	return 0;
 }
 
+/* convert hw value with given bit_precision to lut property val */
+static u32 intel_color_lut_pack(u32 val, u32 bit_precision)
+{
+	u32 max = 0xffff >> (16 - bit_precision);
+
+	val = clamp_val(val, 0, max);
+
+	if (bit_precision < 16)
+		val <<= 16 - bit_precision;
+
+	return val;
+}
+
+static struct drm_property_blob *
+i9xx_read_lut_8(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+	u32 i, val;
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_lut) * 256,
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < 256; i++) {
+		if (HAS_GMCH(dev_priv))
+			val = I915_READ(PALETTE(pipe, i));
+		else
+			val = I915_READ(LGC_PALETTE(pipe, i));
+
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_RED_MASK, val), 8);
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_GREEN_MASK, val), 8);
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_BLUE_MASK, val), 8);
+	}
+
+	return blob;
+}
+
+void i9xx_read_luts(struct intel_crtc_state *crtc_state)
+{
+       crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1271,6 +1321,7 @@ void intel_color_init(struct intel_crtc *crtc)
 			dev_priv->display.color_check = i9xx_color_check;
 			dev_priv->display.color_commit = i9xx_color_commit;
 			dev_priv->display.load_luts = i9xx_load_luts;
+			dev_priv->display.read_luts = i9xx_read_luts;
 		}
 	} else {
 		if (INTEL_GEN(dev_priv) >= 11)
-- 
1.9.1

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

[v5][PATCH 04/11] drm/i915: Extract chv_read_luts()

[Swati Sharma]

In this patch, hw gamma and degamma blob is created for
cherryview.

 v4: -No need to initialize *blob [Jani]
     -Removed right shifts [Jani]
     -Dropped dev local var [Jani]
 v5: -Returned blob instead of assigning it internally within the
      function [Ville]
     -Renamed function cherryview_get_color_config() to chv_read_luts()
     -Renamed cherryview_get_gamma_config() to chv_read_cgm_gamma_lut() [Ville]
     -Added function chv_read_cgm_degamma_lut() to create degamma hw lut [Ville]

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |  6 ++++
 drivers/gpu/drm/i915/intel_color.c | 71 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 77 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 2e99025..f85c336 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10160,6 +10160,12 @@ enum skl_power_gate {
 #define   CGM_PIPE_MODE_GAMMA	(1 << 2)
 #define   CGM_PIPE_MODE_CSC	(1 << 1)
 #define   CGM_PIPE_MODE_DEGAMMA	(1 << 0)
+#define   CGM_PIPE_DEGAMMA_RED_MASK   REG_GENMASK(13, 0)
+#define   CGM_PIPE_DEGAMMA_GREEN_MASK REG_GENMASK(29, 16)
+#define   CGM_PIPE_DEGAMMA_BLUE_MASK  REG_GENMASK(13, 0)
+#define   CGM_PIPE_GAMMA_RED_MASK   REG_GENMASK(9, 0)
+#define   CGM_PIPE_GAMMA_GREEN_MASK REG_GENMASK(25, 16)
+#define   CGM_PIPE_GAMMA_BLUE_MASK  REG_GENMASK(9, 0)
 
 #define _CGM_PIPE_B_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x69900)
 #define _CGM_PIPE_B_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x69904)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 4915b19..720df4f 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1301,6 +1301,76 @@ void i9xx_read_luts(struct intel_crtc_state *crtc_state)
        crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
 }
 
+static struct drm_property_blob *
+chv_read_cgm_degamma_lut(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	u32 i, val, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
+	enum pipe pipe = crtc->pipe;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_lut) * lut_size,
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < lut_size; i++) {
+		val = I915_READ(CGM_PIPE_DEGAMMA(pipe, i, 0));
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_DEGAMMA_GREEN_MASK, val), 14);
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_DEGAMMA_BLUE_MASK, val), 14);
+
+		val = I915_READ(CGM_PIPE_DEGAMMA(pipe, i, 1));
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_DEGAMMA_RED_MASK, val), 14);
+	}
+
+	return blob;
+}
+
+static struct drm_property_blob *
+chv_read_cgm_gamma_lut(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	u32 i, val, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+	enum pipe pipe = crtc->pipe;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_lut) * lut_size,
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < lut_size; i++) {
+		val = I915_READ(CGM_PIPE_GAMMA(pipe, i, 0));
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_GREEN_MASK, val), 10);
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_BLUE_MASK, val), 10);
+
+		val = I915_READ(CGM_PIPE_GAMMA(pipe, i, 1));
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_RED_MASK, val), 10);
+	}
+
+	return blob;
+}
+
+static void chv_read_luts(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
+		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
+	} else {
+		crtc_state->base.degamma_lut = chv_read_cgm_degamma_lut(crtc_state);
+		crtc_state->base.gamma_lut = chv_read_cgm_gamma_lut(crtc_state);
+	}
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1313,6 +1383,7 @@ void intel_color_init(struct intel_crtc *crtc)
 			dev_priv->display.color_check = chv_color_check;
 			dev_priv->display.color_commit = i9xx_color_commit;
 			dev_priv->display.load_luts = chv_load_luts;
+			dev_priv->display.read_luts = chv_read_luts;
 		} else if (INTEL_GEN(dev_priv) >= 4) {
 			dev_priv->display.color_check = i9xx_color_check;
 			dev_priv->display.color_commit = i9xx_color_commit;
-- 
1.9.1

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

[v5][PATCH 05/11] drm/i915: Extract i965_read_luts()

[Swati Sharma]

In this patch, hw gamma blob is created for i965.

v4: -No need to initialize *blob [Jani]
    -Removed right shifts [Jani]
    -Dropped dev local var [Jani]
v5: -Returned blob instead of assigning it internally
     within the function [Ville]
    -Renamed i965_get_color_config() to i965_read_lut() [Ville]
    -Renamed i965_get_gamma_config_10p6() to i965_read_gamma_lut_10p6() [Ville]

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |  3 +++
 drivers/gpu/drm/i915/intel_color.c | 39 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 42 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f85c336..766ffb1 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3585,6 +3585,9 @@ enum i915_power_well_id {
 #define _PALETTE_A		0xa000
 #define _PALETTE_B		0xa800
 #define _CHV_PALETTE_C		0xc000
+#define PALETTE_RED_MASK        REG_GENMASK(23, 16)
+#define PALETTE_GREEN_MASK      REG_GENMASK(15, 8)
+#define PALETTE_BLUE_MASK       REG_GENMASK(7, 0)
 #define PALETTE(pipe, i)	_MMIO(DISPLAY_MMIO_BASE(dev_priv) + \
 				      _PICK((pipe), _PALETTE_A,		\
 					    _PALETTE_B, _CHV_PALETTE_C) + \
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 720df4f..5e41397 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1371,6 +1371,44 @@ static void chv_read_luts(struct intel_crtc_state *crtc_state)
 	}
 }
 
+static struct drm_property_blob *
+i965_read_gamma_lut_10p6(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	u32 i, val1, val2, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+	enum pipe pipe = crtc->pipe;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_lut) * lut_size,
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < lut_size - 1; i++) {
+		val1 = I915_READ(PALETTE(pipe, 2 * i + 0));
+		val2 = I915_READ(PALETTE(pipe, 2 * i + 1));
+
+		blob_data[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val1) << 8 | REG_FIELD_GET(PALETTE_RED_MASK, val2);
+		blob_data[i].green = REG_FIELD_GET(PALETTE_GREEN_MASK, val1) << 8 | REG_FIELD_GET(PALETTE_GREEN_MASK, val2);
+		blob_data[i].blue = REG_FIELD_GET(PALETTE_BLUE_MASK, val1) << 8 | REG_FIELD_GET(PALETTE_BLUE_MASK, val2) ;
+	}
+
+	return blob;
+}
+
+static void i965_read_luts(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
+		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
+	else
+		crtc_state->base.gamma_lut = i965_read_gamma_lut_10p6(crtc_state);
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1388,6 +1426,7 @@ void intel_color_init(struct intel_crtc *crtc)
 			dev_priv->display.color_check = i9xx_color_check;
 			dev_priv->display.color_commit = i9xx_color_commit;
 			dev_priv->display.load_luts = i965_load_luts;
+			dev_priv->display.read_luts = i965_read_luts;
 		} else {
 			dev_priv->display.color_check = i9xx_color_check;
 			dev_priv->display.color_commit = i9xx_color_commit;
-- 
1.9.1

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

[v5][PATCH 06/11] drm/i915: Extract icl_read_luts()

[Swati Sharma]

In this patch, gamma and degamma hw blobs are created for ICL.

v4: -No need to initialize *blob [Jani]
    -Removed right shifts [Jani]
    -Dropped dev local var [Jani]
v5: -Returned blob instead of assigning it internally within the
     function [Ville]
    -Renamed icl_get_color_config() to icl_read_luts() [Ville]
    -Renamed bdw_get_gamma_config() to bdw_read_lut_10() [Ville]
    -Added glk_read_degamma_lut() to validate degamma blob [Ville]

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |  4 ++
 drivers/gpu/drm/i915/intel_color.c | 83 +++++++++++++++++++++++++++++++++++++-
 2 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 766ffb1..7f47241 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10124,6 +10124,9 @@ enum skl_power_gate {
 #define _PAL_PREC_DATA_A	0x4A404
 #define _PAL_PREC_DATA_B	0x4AC04
 #define _PAL_PREC_DATA_C	0x4B404
+#define   PREC_PAL_DATA_RED_MASK	REG_GENMASK(29, 20)
+#define   PREC_PAL_DATA_GREEN_MASK	REG_GENMASK(19, 10)
+#define   PREC_PAL_DATA_BLUE_MASK	REG_GENMASK(9, 0)
 #define _PAL_PREC_GC_MAX_A	0x4A410
 #define _PAL_PREC_GC_MAX_B	0x4AC10
 #define _PAL_PREC_GC_MAX_C	0x4B410
@@ -10147,6 +10150,7 @@ enum skl_power_gate {
 #define _PRE_CSC_GAMC_DATA_A	0x4A488
 #define _PRE_CSC_GAMC_DATA_B	0x4AC88
 #define _PRE_CSC_GAMC_DATA_C	0x4B488
+#define  PRE_CSC_GAMC_MASK	REG_GENMASK(18, 0)
 
 #define PRE_CSC_GAMC_INDEX(pipe)	_MMIO_PIPE(pipe, _PRE_CSC_GAMC_INDEX_A, _PRE_CSC_GAMC_INDEX_B)
 #define PRE_CSC_GAMC_DATA(pipe)		_MMIO_PIPE(pipe, _PRE_CSC_GAMC_DATA_A, _PRE_CSC_GAMC_DATA_B)
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 5e41397..1807cb97 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1409,6 +1409,85 @@ static void i965_read_luts(struct intel_crtc_state *crtc_state)
 		crtc_state->base.gamma_lut = i965_read_gamma_lut_10p6(crtc_state);
 }
 
+static struct drm_property_blob *
+glk_read_degamma_lut(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	u32 i, val, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
+	enum pipe pipe = crtc->pipe;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+
+	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0);
+	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), PRE_CSC_GAMC_AUTO_INCREMENT);
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_lut) * lut_size,
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < lut_size; i++) {
+		val = I915_READ(PRE_CSC_GAMC_DATA(pipe));
+
+		blob_data[i].red = REG_FIELD_GET(PRE_CSC_GAMC_MASK, val);
+		blob_data[i].green = REG_FIELD_GET(PRE_CSC_GAMC_MASK, val);
+		blob_data[i].blue = REG_FIELD_GET(PRE_CSC_GAMC_MASK, val);
+	}
+
+	return blob;
+}
+
+static struct drm_property_blob *
+bdw_read_lut_10(struct intel_crtc_state *crtc_state,
+		      u32 prec_index)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	int hw_lut_size = ivb_lut_10_size(prec_index);
+	enum pipe pipe = crtc->pipe;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+	u32 i, val;
+
+	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
+		   PAL_PREC_AUTO_INCREMENT);
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_lut) * hw_lut_size,
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < hw_lut_size; i++) {
+		val = I915_READ(PREC_PAL_DATA(pipe));
+
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_RED_MASK, val), 10);
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_GREEN_MASK, val), 10);
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_BLUE_MASK, val), 10);
+	}
+
+	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
+
+	return blob;
+}
+
+static void icl_read_luts(struct intel_crtc_state *crtc_state)
+{
+	crtc_state->base.degamma_lut = glk_read_degamma_lut(crtc_state);
+
+	if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
+	    GAMMA_MODE_MODE_8BIT)
+		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
+	else
+		crtc_state->base.gamma_lut = bdw_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1450,8 +1529,10 @@ void intel_color_init(struct intel_crtc *crtc)
 		else
 			dev_priv->display.color_commit = ilk_color_commit;
 
-		if (INTEL_GEN(dev_priv) >= 11)
+		if (INTEL_GEN(dev_priv) >= 11) {
 			dev_priv->display.load_luts = icl_load_luts;
+			dev_priv->display.read_luts = icl_read_luts;
+		}
 		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
 			dev_priv->display.load_luts = glk_load_luts;
 		else if (INTEL_GEN(dev_priv) >= 8)
-- 
1.9.1

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

[v5][PATCH 07/11] drm/i915: Extract glk_read_luts()

[Swati Sharma]

In this patch, gamma and degamma hw blobs are created for GLK.

v4: -No need to initialize *blob [Jani]
    -Removed right shifts [Jani]
    -Dropped dev local var [Jani]
v5: -Returned blob instead of assigning it internally within the
     function [Ville]
    -Renamed glk_get_color_config() to glk_read_luts() [Ville]
    -Added glk_read_degamma_lut_linear() to validate degamma blob [Ville]

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c | 50 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 1807cb97..32cea6d 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1410,6 +1410,38 @@ static void i965_read_luts(struct intel_crtc_state *crtc_state)
 }
 
 static struct drm_property_blob *
+glk_read_degamma_lut_linear(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	u32 i, val, lut_size = INTEL_INFO(dev_priv)->color.degamma_lut_size;
+	enum pipe pipe = crtc->pipe;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+
+	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), 0);
+	I915_WRITE(PRE_CSC_GAMC_INDEX(pipe), PRE_CSC_GAMC_AUTO_INCREMENT);
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_lut) * lut_size,
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < lut_size; i++) {
+		val = I915_READ(PRE_CSC_GAMC_DATA(pipe));
+
+		blob_data[i].red = REG_FIELD_GET(PRE_CSC_GAMC_MASK, val);
+		blob_data[i].green = REG_FIELD_GET(PRE_CSC_GAMC_MASK, val);
+		blob_data[i].blue = REG_FIELD_GET(PRE_CSC_GAMC_MASK, val);
+	}
+
+	return blob;
+}
+
+static struct drm_property_blob *
 glk_read_degamma_lut(struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
@@ -1488,6 +1520,19 @@ static void icl_read_luts(struct intel_crtc_state *crtc_state)
 		crtc_state->base.gamma_lut = bdw_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
 }
 
+static void glk_read_luts(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->csc_enable)
+		crtc_state->base.degamma_lut = glk_read_degamma_lut_linear(crtc_state);
+	else
+		crtc_state->base.degamma_lut = glk_read_degamma_lut(crtc_state);
+
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
+		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
+	else
+		crtc_state->base.gamma_lut = bdw_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1532,9 +1577,10 @@ void intel_color_init(struct intel_crtc *crtc)
 		if (INTEL_GEN(dev_priv) >= 11) {
 			dev_priv->display.load_luts = icl_load_luts;
 			dev_priv->display.read_luts = icl_read_luts;
-		}
-		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+		} else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
 			dev_priv->display.load_luts = glk_load_luts;
+			dev_priv->display.read_luts = glk_read_luts;
+		}
 		else if (INTEL_GEN(dev_priv) >= 8)
 			dev_priv->display.load_luts = bdw_load_luts;
 		else if (INTEL_GEN(dev_priv) >= 7)
-- 
1.9.1

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

[v5][PATCH 08/11] drm/i915: Extract bdw_read_luts()

[Swati Sharma]

In this patch, gamma and degamma hw blobs are created for BDW.

v4: -No need to initialize *blob [Jani]
    -Removed right shifts [Jani]
    -Dropped dev local var [Jani]
v5: -Returned blob instead of assigning it internally within the
     function [Ville]
    -Renamed bdw_get_color_config() to bdw_read_luts() [Ville]
    -Enabled degamma lut validation [Ville]

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 32cea6d..c5cc1d9 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1533,6 +1533,19 @@ static void glk_read_luts(struct intel_crtc_state *crtc_state)
 		crtc_state->base.gamma_lut = bdw_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
 }
 
+static void bdw_read_luts(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
+		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
+	} else if (crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT) {
+		crtc_state->base.degamma_lut = bdw_read_lut_10(crtc_state, PAL_PREC_SPLIT_MODE | PAL_PREC_INDEX_VALUE(0));
+		crtc_state->base.gamma_lut = bdw_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(512));
+	} else {
+		crtc_state->base.degamma_lut = bdw_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
+		crtc_state->base.gamma_lut = bdw_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
+	}
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1580,9 +1593,10 @@ void intel_color_init(struct intel_crtc *crtc)
 		} else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
 			dev_priv->display.load_luts = glk_load_luts;
 			dev_priv->display.read_luts = glk_read_luts;
-		}
-		else if (INTEL_GEN(dev_priv) >= 8)
+		} else if (INTEL_GEN(dev_priv) >= 8) {
 			dev_priv->display.load_luts = bdw_load_luts;
+			dev_priv->display.read_luts = bdw_read_luts;
+		}
 		else if (INTEL_GEN(dev_priv) >= 7)
 			dev_priv->display.load_luts = ivb_load_luts;
 		else
-- 
1.9.1

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

[v5][PATCH 09/11] drm/i915: Extract ivb_read_luts()

[Swati Sharma]

In this patch, gamma and degamma hw blobs are created for IVB.

v4: -No need to initialize *blob [Jani]
    -Removed right shifts [Jani]
    -Dropped dev local var [Jani]
v5: -Returned blob instead of assigning it internally within the
     function [Ville]
    -Renamed ivb_get_color_config() to ivb_read_luts() [Ville]
    -Enabled degamma blob validation [Ville]

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c | 52 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index c5cc1d9..d8b120a 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1546,6 +1546,53 @@ static void bdw_read_luts(struct intel_crtc_state *crtc_state)
 	}
 }
 
+static struct drm_property_blob *
+ivb_read_lut_10(struct intel_crtc_state *crtc_state,
+	        u32 prec_index)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	int hw_lut_size = ivb_lut_10_size(prec_index);
+	enum pipe pipe = crtc->pipe;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+	u32 i, val;
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_lut) * hw_lut_size,
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < hw_lut_size; i++) {
+		I915_WRITE(PREC_PAL_INDEX(pipe), prec_index++);
+		val = I915_READ(PREC_PAL_DATA(pipe));
+
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_RED_MASK, val), 10);
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_GREEN_MASK, val), 10);
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_BLUE_MASK, val), 10);
+	}
+
+	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
+
+	return blob;
+}
+
+static void ivb_read_luts(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT) {
+		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
+	} else if (crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT) {
+		crtc_state->base.degamma_lut = ivb_read_lut_10(crtc_state, PAL_PREC_SPLIT_MODE | PAL_PREC_INDEX_VALUE(0));
+		crtc_state->base.gamma_lut = ivb_read_lut_10(crtc_state, PAL_PREC_SPLIT_MODE | PAL_PREC_INDEX_VALUE(512));
+	} else {
+		crtc_state->base.degamma_lut = ivb_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
+		crtc_state->base.gamma_lut = ivb_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
+	}
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1596,9 +1643,10 @@ void intel_color_init(struct intel_crtc *crtc)
 		} else if (INTEL_GEN(dev_priv) >= 8) {
 			dev_priv->display.load_luts = bdw_load_luts;
 			dev_priv->display.read_luts = bdw_read_luts;
-		}
-		else if (INTEL_GEN(dev_priv) >= 7)
+		} else if (INTEL_GEN(dev_priv) >= 7) {
 			dev_priv->display.load_luts = ivb_load_luts;
+			dev_priv->display.read_luts = ivb_read_luts;
+		}
 		else
 			dev_priv->display.load_luts = ilk_load_luts;
 	}
-- 
1.9.1

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

[v5][PATCH 10/11] drm/i915: Extract ilk_read_luts()

[Swati Sharma]

In this patch, hw gamma blob is created for ILK.

v4: -No need to initialize *blob [Jani]
    -Removed right shifts [Jani]
    -Dropped dev local var [Jani]
v5: -Returned blob instead of assigning it internally within the
     function [Ville]
    -Renamed ilk_get_color_config() to ilk_read_luts() [Ville]

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h    |  3 +++
 drivers/gpu/drm/i915/intel_color.c | 42 ++++++++++++++++++++++++++++++++++++--
 2 files changed, 43 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7f47241..66127e2 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7189,6 +7189,9 @@ enum {
 /* ilk/snb precision palette */
 #define _PREC_PALETTE_A           0x4b000
 #define _PREC_PALETTE_B           0x4c000
+#define   PREC_PALETTE_RED_MASK   REG_GENMASK(29, 20)
+#define   PREC_PALETTE_GREEN_MASK REG_GENMASK(19, 10)
+#define   PREC_PALETTE_BLUE_MASK  REG_GENMASK(9, 0)
 #define PREC_PALETTE(pipe, i) _MMIO(_PIPE(pipe, _PREC_PALETTE_A, _PREC_PALETTE_B) + (i) * 4)
 
 #define  _PREC_PIPEAGCMAX              0x4d000
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index d8b120a..695b76d 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1593,6 +1593,43 @@ static void ivb_read_luts(struct intel_crtc_state *crtc_state)
 	}
 }
 
+static struct drm_property_blob *
+ilk_read_gamma_lut(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	u32 i, val, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+	enum pipe pipe = crtc->pipe;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_lut) * lut_size,
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < lut_size - 1; i++) {
+		val = I915_READ(PREC_PALETTE(pipe, i));
+
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(PREC_PALETTE_RED_MASK, val), 10);
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(PREC_PALETTE_GREEN_MASK, val), 10);
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(PREC_PALETTE_BLUE_MASK, val), 10);
+	}
+
+	return blob;
+}
+
+static void ilk_read_luts(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
+		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
+	else
+		crtc_state->base.gamma_lut = ilk_read_gamma_lut(crtc_state);
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1646,9 +1683,10 @@ void intel_color_init(struct intel_crtc *crtc)
 		} else if (INTEL_GEN(dev_priv) >= 7) {
 			dev_priv->display.load_luts = ivb_load_luts;
 			dev_priv->display.read_luts = ivb_read_luts;
-		}
-		else
+		} else {
 			dev_priv->display.load_luts = ilk_load_luts;
+			dev_priv->display.read_luts = ilk_read_luts;
+		}
 	}
 
 	drm_crtc_enable_color_mgmt(&crtc->base,
-- 
1.9.1

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

[v5][PATCH 11/11] drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma/degamma lut values

[Swati Sharma]

v3: Rebase
v4: -Renamed intel_compare_color_lut() to intel_color_lut_equal() [Jani]
    -Added the default label above the correct label [Jani]
    -Corrected smatch warn "variable dereferenced before check" [Dan Carpenter]
v5: -Added condition (!blob1 && !blob2) return true [Jani]
    -Called PIPE_CONF_CHECK_COLOR_LUT inside if (!adjust) [Jani]
    -Added #undef PIPE_CONF_CHECK_COLOR_LUT [Jani]

There are few things wrong in this patch:
[1] For chv bit precision is 14, on what basis it should be assigned?
[2] For glk and icl, degamma LUT values are not rounded off, there
should err=0 if using same function, how to make that exception?
[3] For glk, bit precision is 10 but gamma mode is 8BIT?

Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c   | 54 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color.h   |  6 ++++
 drivers/gpu/drm/i915/intel_display.c | 12 ++++++++
 3 files changed, 72 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 695b76d..73cb901 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1630,6 +1630,60 @@ static void ilk_read_luts(struct intel_crtc_state *crtc_state)
 		crtc_state->base.gamma_lut = ilk_read_gamma_lut(crtc_state);
 }
 
+static inline bool err_check(struct drm_color_lut *sw_lut, struct drm_color_lut *hw_lut, u32 err)
+{
+	 return ((abs((long)hw_lut->red - sw_lut->red)) <= err) &&
+	        ((abs((long)hw_lut->blue - sw_lut->blue)) <= err) &&
+	        ((abs((long)hw_lut->green - sw_lut->green)) <= err);
+}
+
+bool intel_color_lut_equal(struct drm_property_blob *blob1,
+			   struct drm_property_blob *blob2,
+			   u32 gamma_mode)
+{
+	struct drm_color_lut *sw_lut, *hw_lut;
+	int sw_lut_size, hw_lut_size, i;
+	u32 bit_precision, err;
+
+	if (!blob1 && !blob2)
+		return true;
+
+	if (!blob1 || !blob2)
+		return false;
+
+	sw_lut_size = drm_color_lut_size(blob1);
+	hw_lut_size = drm_color_lut_size(blob2);
+
+	if (sw_lut_size != hw_lut_size)
+		return false;
+
+	switch(gamma_mode) {
+	default:
+	case GAMMA_MODE_MODE_8BIT:
+		bit_precision = 8;
+		break;
+	case GAMMA_MODE_MODE_10BIT:
+	case GAMMA_MODE_MODE_SPLIT:
+		bit_precision = 10;
+		break;
+	case GAMMA_MODE_MODE_12BIT:
+		bit_precision = 12;
+		break;
+	}
+
+	sw_lut = blob1->data;
+	hw_lut = blob2->data;
+
+	err = 0xffff >> bit_precision;
+
+	for (i = 0; i < sw_lut_size; i++) {
+		 if (!err_check(&hw_lut[i], &sw_lut[i], err))
+			return false;
+	}
+
+	return true;
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h
index fc53de9..b525c80 100644
--- a/drivers/gpu/drm/i915/intel_color.h
+++ b/drivers/gpu/drm/i915/intel_color.h
@@ -6,13 +6,19 @@
 #ifndef __INTEL_COLOR_H__
 #define __INTEL_COLOR_H__
 
+#include <linux/types.h>
+
 struct intel_crtc_state;
 struct intel_crtc;
+struct drm_property_blob;
 
 void intel_color_init(struct intel_crtc *crtc);
 int intel_color_check(struct intel_crtc_state *crtc_state);
 void intel_color_commit(const struct intel_crtc_state *crtc_state);
 void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
 void intel_color_read_luts(struct intel_crtc_state *crtc_state);
+bool intel_color_lut_equal(struct drm_property_blob *blob1,
+				 struct drm_property_blob *blob2,
+				 u32 gamma_mode);
 
 #endif /* __INTEL_COLOR_H__ */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 791974b..a713171 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12287,6 +12287,14 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 	} \
 } while (0)
 
+#define PIPE_CONF_CHECK_COLOR_LUT(name, gamma_mode) do { \
+	if (!intel_color_lut_equal(current_config->name, pipe_config->name, gamma_mode)) { \
+		pipe_config_err(adjust, __stringify(name), \
+				"hw_state doesn't match sw_state\n"); \
+		ret = false; \
+	} \
+} while (0)
+
 #define PIPE_CONF_QUIRK(quirk) \
 	((current_config->quirks | pipe_config->quirks) & (quirk))
 
@@ -12376,6 +12384,9 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 			PIPE_CONF_CHECK_X(csc_mode);
 		PIPE_CONF_CHECK_BOOL(gamma_enable);
 		PIPE_CONF_CHECK_BOOL(csc_enable);
+
+		PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, pipe_config->gamma_mode);
+		PIPE_CONF_CHECK_COLOR_LUT(base.degamma_lut, pipe_config->gamma_mode);
 	}
 
 	PIPE_CONF_CHECK_BOOL(double_wide);
@@ -12438,6 +12449,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 #undef PIPE_CONF_CHECK_FLAGS
 #undef PIPE_CONF_CHECK_CLOCK_FUZZY
 #undef PIPE_CONF_QUIRK
+#undef PIPE_CONF_CHECK_COLOR_LUT
 
 	return ret;
 }
-- 
1.9.1

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

✗ Fi.CI.CHECKPATCH: warning for drm/i915: adding state checker for gamma lut values (rev7)

[Patchwork]

== Series Details ==

Series: drm/i915: adding state checker for gamma lut values (rev7)
URL   : https://patchwork.freedesktop.org/series/58039/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
1e8ed6586232 drm/i915: Introduce vfunc read_luts() to create hw lut
b2195f303ff1 drm/i915: Enable intel_color_read_luts()
8780c2e9fe65 drm/i915: Extract i9xx_read_luts()
-:13: WARNING:TYPO_SPELLING: 'withing' may be misspelled - perhaps 'within'?
#13: 
v5: -Returned blob instead of assigning it internally withing the

-:79: WARNING:LONG_LINE: line over 100 characters
#79: FILE: drivers/gpu/drm/i915/intel_color.c:1291:
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_RED_MASK, val), 8);

-:80: WARNING:LONG_LINE: line over 100 characters
#80: FILE: drivers/gpu/drm/i915/intel_color.c:1292:
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_GREEN_MASK, val), 8);

-:81: WARNING:LONG_LINE: line over 100 characters
#81: FILE: drivers/gpu/drm/i915/intel_color.c:1293:
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_BLUE_MASK, val), 8);

-:89: WARNING:LEADING_SPACE: please, no spaces at the start of a line
#89: FILE: drivers/gpu/drm/i915/intel_color.c:1301:
+       crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);$

total: 0 errors, 5 warnings, 0 checks, 72 lines checked
899ad49ad748 drm/i915: Extract chv_read_luts()
-:15: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#15: 
     -Renamed cherryview_get_gamma_config() to chv_read_cgm_gamma_lut() [Ville]

-:65: WARNING:LONG_LINE: line over 100 characters
#65: FILE: drivers/gpu/drm/i915/intel_color.c:1324:
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_DEGAMMA_GREEN_MASK, val), 14);

-:66: WARNING:LONG_LINE: line over 100 characters
#66: FILE: drivers/gpu/drm/i915/intel_color.c:1325:
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_DEGAMMA_BLUE_MASK, val), 14);

-:69: WARNING:LONG_LINE: line over 100 characters
#69: FILE: drivers/gpu/drm/i915/intel_color.c:1328:
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_DEGAMMA_RED_MASK, val), 14);

-:95: WARNING:LONG_LINE: line over 100 characters
#95: FILE: drivers/gpu/drm/i915/intel_color.c:1354:
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_GREEN_MASK, val), 10);

-:96: WARNING:LONG_LINE: line over 100 characters
#96: FILE: drivers/gpu/drm/i915/intel_color.c:1355:
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_BLUE_MASK, val), 10);

-:99: WARNING:LONG_LINE: line over 100 characters
#99: FILE: drivers/gpu/drm/i915/intel_color.c:1358:
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_RED_MASK, val), 10);

total: 0 errors, 7 warnings, 0 checks, 95 lines checked
fa572fdd813a drm/i915: Extract i965_read_luts()
-:14: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#14: 
    -Renamed i965_get_gamma_config_10p6() to i965_read_gamma_lut_10p6() [Ville]

-:62: WARNING:LONG_LINE: line over 100 characters
#62: FILE: drivers/gpu/drm/i915/intel_color.c:1396:
+		blob_data[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val1) << 8 | REG_FIELD_GET(PALETTE_RED_MASK, val2);

-:63: WARNING:LONG_LINE: line over 100 characters
#63: FILE: drivers/gpu/drm/i915/intel_color.c:1397:
+		blob_data[i].green = REG_FIELD_GET(PALETTE_GREEN_MASK, val1) << 8 | REG_FIELD_GET(PALETTE_GREEN_MASK, val2);

-:64: WARNING:LONG_LINE: line over 100 characters
#64: FILE: drivers/gpu/drm/i915/intel_color.c:1398:
+		blob_data[i].blue = REG_FIELD_GET(PALETTE_BLUE_MASK, val1) << 8 | REG_FIELD_GET(PALETTE_BLUE_MASK, val2) ;

-:64: WARNING:SPACING: space prohibited before semicolon
#64: FILE: drivers/gpu/drm/i915/intel_color.c:1398:
+		blob_data[i].blue = REG_FIELD_GET(PALETTE_BLUE_MASK, val1) << 8 | REG_FIELD_GET(PALETTE_BLUE_MASK, val2) ;

total: 0 errors, 5 warnings, 0 checks, 60 lines checked
584eb0d383dc drm/i915: Extract icl_read_luts()
-:83: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#83: FILE: drivers/gpu/drm/i915/intel_color.c:1446:
+bdw_read_lut_10(struct intel_crtc_state *crtc_state,
+		      u32 prec_index)

-:107: WARNING:LONG_LINE: line over 100 characters
#107: FILE: drivers/gpu/drm/i915/intel_color.c:1470:
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_RED_MASK, val), 10);

-:108: WARNING:LONG_LINE: line over 100 characters
#108: FILE: drivers/gpu/drm/i915/intel_color.c:1471:
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_GREEN_MASK, val), 10);

-:109: WARNING:LONG_LINE: line over 100 characters
#109: FILE: drivers/gpu/drm/i915/intel_color.c:1472:
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_BLUE_MASK, val), 10);

-:136: CHECK:BRACES: braces {} should be used on all arms of this statement
#136: FILE: drivers/gpu/drm/i915/intel_color.c:1532:
+		if (INTEL_GEN(dev_priv) >= 11) {
[...]
 		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
[...]

total: 0 errors, 3 warnings, 2 checks, 112 lines checked
d0c8af000d4b drm/i915: Extract glk_read_luts()
f9abafe9edf4 drm/i915: Extract bdw_read_luts()
-:31: WARNING:LONG_LINE: line over 100 characters
#31: FILE: drivers/gpu/drm/i915/intel_color.c:1541:
+		crtc_state->base.degamma_lut = bdw_read_lut_10(crtc_state, PAL_PREC_SPLIT_MODE | PAL_PREC_INDEX_VALUE(0));

total: 0 errors, 1 warnings, 0 checks, 31 lines checked
7349486b0eb6 drm/i915: Extract ivb_read_luts()
-:28: ERROR:CODE_INDENT: code indent should use tabs where possible
#28: FILE: drivers/gpu/drm/i915/intel_color.c:1551:
+^I        u32 prec_index)$

-:28: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#28: FILE: drivers/gpu/drm/i915/intel_color.c:1551:
+ivb_read_lut_10(struct intel_crtc_state *crtc_state,
+	        u32 prec_index)

-:50: WARNING:LONG_LINE: line over 100 characters
#50: FILE: drivers/gpu/drm/i915/intel_color.c:1573:
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_RED_MASK, val), 10);

-:51: WARNING:LONG_LINE: line over 100 characters
#51: FILE: drivers/gpu/drm/i915/intel_color.c:1574:
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_GREEN_MASK, val), 10);

-:52: WARNING:LONG_LINE: line over 100 characters
#52: FILE: drivers/gpu/drm/i915/intel_color.c:1575:
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_BLUE_MASK, val), 10);

-:65: WARNING:LONG_LINE: line over 100 characters
#65: FILE: drivers/gpu/drm/i915/intel_color.c:1588:
+		crtc_state->base.degamma_lut = ivb_read_lut_10(crtc_state, PAL_PREC_SPLIT_MODE | PAL_PREC_INDEX_VALUE(0));

-:66: WARNING:LONG_LINE: line over 100 characters
#66: FILE: drivers/gpu/drm/i915/intel_color.c:1589:
+		crtc_state->base.gamma_lut = ivb_read_lut_10(crtc_state, PAL_PREC_SPLIT_MODE | PAL_PREC_INDEX_VALUE(512));

total: 1 errors, 5 warnings, 1 checks, 65 lines checked
8a0aa7fcdd4c drm/i915: Extract ilk_read_luts()
-:60: WARNING:LONG_LINE: line over 100 characters
#60: FILE: drivers/gpu/drm/i915/intel_color.c:1617:
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(PREC_PALETTE_RED_MASK, val), 10);

-:61: WARNING:LONG_LINE: line over 100 characters
#61: FILE: drivers/gpu/drm/i915/intel_color.c:1618:
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(PREC_PALETTE_GREEN_MASK, val), 10);

-:62: WARNING:LONG_LINE: line over 100 characters
#62: FILE: drivers/gpu/drm/i915/intel_color.c:1619:
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(PREC_PALETTE_BLUE_MASK, val), 10);

total: 0 errors, 3 warnings, 0 checks, 64 lines checked
a1d9c399c22a drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma/degamma lut values
-:10: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#10: 
    -Corrected smatch warn "variable dereferenced before check" [Dan Carpenter]

-:33: WARNING:TABSTOP: Statements should start on a tabstop
#33: FILE: drivers/gpu/drm/i915/intel_color.c:1635:
+	 return ((abs((long)hw_lut->red - sw_lut->red)) <= err) &&

-:34: ERROR:CODE_INDENT: code indent should use tabs where possible
#34: FILE: drivers/gpu/drm/i915/intel_color.c:1636:
+^I        ((abs((long)hw_lut->blue - sw_lut->blue)) <= err) &&$

-:35: ERROR:CODE_INDENT: code indent should use tabs where possible
#35: FILE: drivers/gpu/drm/i915/intel_color.c:1637:
+^I        ((abs((long)hw_lut->green - sw_lut->green)) <= err);$

-:58: ERROR:SPACING: space required before the open parenthesis '('
#58: FILE: drivers/gpu/drm/i915/intel_color.c:1660:
+	switch(gamma_mode) {

-:77: WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (8, 17)
#77: FILE: drivers/gpu/drm/i915/intel_color.c:1679:
+	for (i = 0; i < sw_lut_size; i++) {
+		 if (!err_check(&hw_lut[i], &sw_lut[i], err))

-:78: WARNING:TABSTOP: Statements should start on a tabstop
#78: FILE: drivers/gpu/drm/i915/intel_color.c:1680:
+		 if (!err_check(&hw_lut[i], &sw_lut[i], err))

-:108: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#108: FILE: drivers/gpu/drm/i915/intel_color.h:21:
+bool intel_color_lut_equal(struct drm_property_blob *blob1,
+				 struct drm_property_blob *blob2,

-:120: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name' - possible side-effects?
#120: FILE: drivers/gpu/drm/i915/intel_display.c:12316:
+#define PIPE_CONF_CHECK_COLOR_LUT(name, gamma_mode) do { \
+	if (!intel_color_lut_equal(current_config->name, pipe_config->name, gamma_mode)) { \
+		pipe_config_err(adjust, __stringify(name), \
+				"hw_state doesn't match sw_state\n"); \
+		ret = false; \
+	} \
+} while (0)

-:120: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name' may be better as '(name)' to avoid precedence issues
#120: FILE: drivers/gpu/drm/i915/intel_display.c:12316:
+#define PIPE_CONF_CHECK_COLOR_LUT(name, gamma_mode) do { \
+	if (!intel_color_lut_equal(current_config->name, pipe_config->name, gamma_mode)) { \
+		pipe_config_err(adjust, __stringify(name), \
+				"hw_state doesn't match sw_state\n"); \
+		ret = false; \
+	} \
+} while (0)

total: 3 errors, 4 warnings, 3 checks, 109 lines checked

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

✗ Fi.CI.SPARSE: warning for drm/i915: adding state checker for gamma lut values (rev7)

[Patchwork]

== Series Details ==

Series: drm/i915: adding state checker for gamma lut values (rev7)
URL   : https://patchwork.freedesktop.org/series/58039/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915: Introduce vfunc read_luts() to create hw lut
Okay!

Commit: drm/i915: Enable intel_color_read_luts()
Okay!

Commit: drm/i915: Extract i9xx_read_luts()
+drivers/gpu/drm/i915/intel_color.c:1259:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1259:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1259:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1259:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1259:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1259:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1259:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1259:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1299:6: warning: symbol 'i9xx_read_luts' was not declared. Should it be static?

Commit: drm/i915: Extract chv_read_luts()
Okay!

Commit: drm/i915: Extract i965_read_luts()
Okay!

Commit: drm/i915: Extract icl_read_luts()
Okay!

Commit: drm/i915: Extract glk_read_luts()
Okay!

Commit: drm/i915: Extract bdw_read_luts()
Okay!

Commit: drm/i915: Extract ivb_read_luts()
Okay!

Commit: drm/i915: Extract ilk_read_luts()
Okay!

Commit: drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma/degamma lut values
Okay!

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

✗ Fi.CI.BAT: failure for drm/i915: adding state checker for gamma lut values (rev7)

[Patchwork]

== Series Details ==

Series: drm/i915: adding state checker for gamma lut values (rev7)
URL   : https://patchwork.freedesktop.org/series/58039/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_6045 -> Patchwork_12967
====================================================

Summary
-------

  **FAILURE**

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

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

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@debugfs_test@read_all_entries:
    - fi-snb-2520m:       [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-snb-2520m/igt@debugfs_test@read_all_entries.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-snb-2520m/igt@debugfs_test@read_all_entries.html
    - fi-elk-e7500:       [PASS][3] -> [DMESG-WARN][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-elk-e7500/igt@debugfs_test@read_all_entries.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-elk-e7500/igt@debugfs_test@read_all_entries.html
    - fi-snb-2600:        [PASS][5] -> [DMESG-WARN][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-snb-2600/igt@debugfs_test@read_all_entries.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-snb-2600/igt@debugfs_test@read_all_entries.html
    - fi-byt-j1900:       [PASS][7] -> [DMESG-WARN][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-byt-j1900/igt@debugfs_test@read_all_entries.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-byt-j1900/igt@debugfs_test@read_all_entries.html

  * igt@gem_exec_suspend@basic-s3:
    - fi-icl-u3:          [PASS][9] -> [DMESG-WARN][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-icl-u3/igt@gem_exec_suspend@basic-s3.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-icl-u3/igt@gem_exec_suspend@basic-s3.html
    - fi-icl-y:           NOTRUN -> [DMESG-WARN][11]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-icl-y/igt@gem_exec_suspend@basic-s3.html

  * igt@kms_busy@basic-flip-a:
    - fi-kbl-7567u:       [PASS][12] -> [DMESG-WARN][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-kbl-7567u/igt@kms_busy@basic-flip-a.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-kbl-7567u/igt@kms_busy@basic-flip-a.html
    - fi-apl-guc:         [PASS][14] -> [DMESG-WARN][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-apl-guc/igt@kms_busy@basic-flip-a.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-apl-guc/igt@kms_busy@basic-flip-a.html
    - fi-whl-u:           [PASS][16] -> [DMESG-WARN][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-whl-u/igt@kms_busy@basic-flip-a.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-whl-u/igt@kms_busy@basic-flip-a.html
    - fi-byt-n2820:       [PASS][18] -> [DMESG-WARN][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-byt-n2820/igt@kms_busy@basic-flip-a.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-byt-n2820/igt@kms_busy@basic-flip-a.html
    - fi-skl-iommu:       [PASS][20] -> [DMESG-WARN][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-skl-iommu/igt@kms_busy@basic-flip-a.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-skl-iommu/igt@kms_busy@basic-flip-a.html
    - fi-skl-6770hq:      [PASS][22] -> [DMESG-WARN][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-skl-6770hq/igt@kms_busy@basic-flip-a.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-skl-6770hq/igt@kms_busy@basic-flip-a.html
    - fi-kbl-x1275:       [PASS][24] -> [DMESG-WARN][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-kbl-x1275/igt@kms_busy@basic-flip-a.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-kbl-x1275/igt@kms_busy@basic-flip-a.html
    - fi-bxt-j4205:       [PASS][26] -> [DMESG-WARN][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-bxt-j4205/igt@kms_busy@basic-flip-a.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-bxt-j4205/igt@kms_busy@basic-flip-a.html
    - fi-skl-6260u:       [PASS][28] -> [DMESG-WARN][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-skl-6260u/igt@kms_busy@basic-flip-a.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-skl-6260u/igt@kms_busy@basic-flip-a.html
    - fi-kbl-r:           [PASS][30] -> [DMESG-WARN][31]
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-kbl-r/igt@kms_busy@basic-flip-a.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-kbl-r/igt@kms_busy@basic-flip-a.html
    - fi-hsw-peppy:       [PASS][32] -> [DMESG-WARN][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-hsw-peppy/igt@kms_busy@basic-flip-a.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-hsw-peppy/igt@kms_busy@basic-flip-a.html
    - fi-gdg-551:         [PASS][34] -> [DMESG-WARN][35]
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-gdg-551/igt@kms_busy@basic-flip-a.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-gdg-551/igt@kms_busy@basic-flip-a.html
    - fi-bwr-2160:        [PASS][36] -> [DMESG-WARN][37]
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-bwr-2160/igt@kms_busy@basic-flip-a.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-bwr-2160/igt@kms_busy@basic-flip-a.html
    - fi-hsw-4770:        [PASS][38] -> [DMESG-WARN][39]
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-hsw-4770/igt@kms_busy@basic-flip-a.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-hsw-4770/igt@kms_busy@basic-flip-a.html
    - fi-cfl-8700k:       [PASS][40] -> [DMESG-WARN][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-cfl-8700k/igt@kms_busy@basic-flip-a.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-cfl-8700k/igt@kms_busy@basic-flip-a.html
    - fi-ivb-3770:        [PASS][42] -> [DMESG-WARN][43]
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-ivb-3770/igt@kms_busy@basic-flip-a.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-ivb-3770/igt@kms_busy@basic-flip-a.html
    - fi-cfl-guc:         [PASS][44] -> [DMESG-WARN][45]
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-cfl-guc/igt@kms_busy@basic-flip-a.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-cfl-guc/igt@kms_busy@basic-flip-a.html
    - fi-blb-e6850:       [PASS][46] -> [DMESG-WARN][47]
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-blb-e6850/igt@kms_busy@basic-flip-a.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-blb-e6850/igt@kms_busy@basic-flip-a.html
    - fi-hsw-4770r:       [PASS][48] -> [DMESG-WARN][49]
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-hsw-4770r/igt@kms_busy@basic-flip-a.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-hsw-4770r/igt@kms_busy@basic-flip-a.html
    - fi-skl-6600u:       [PASS][50] -> [DMESG-WARN][51]
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-skl-6600u/igt@kms_busy@basic-flip-a.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-skl-6600u/igt@kms_busy@basic-flip-a.html
    - fi-bdw-5557u:       [PASS][52] -> [DMESG-WARN][53]
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-bdw-5557u/igt@kms_busy@basic-flip-a.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-bdw-5557u/igt@kms_busy@basic-flip-a.html

  * igt@kms_busy@basic-flip-c:
    - fi-bsw-n3050:       [PASS][54] -> [DMESG-WARN][55]
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-bsw-n3050/igt@kms_busy@basic-flip-c.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-bsw-n3050/igt@kms_busy@basic-flip-c.html

  * igt@runner@aborted:
    - fi-ilk-650:         NOTRUN -> [FAIL][56]
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-ilk-650/igt@runner@aborted.html
    - fi-bdw-gvtdvm:      NOTRUN -> [FAIL][57]
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-bdw-gvtdvm/igt@runner@aborted.html
    - fi-hsw-peppy:       NOTRUN -> [FAIL][58]
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-hsw-peppy/igt@runner@aborted.html
    - fi-gdg-551:         NOTRUN -> [FAIL][59]
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-gdg-551/igt@runner@aborted.html
    - fi-snb-2520m:       NOTRUN -> [FAIL][60]
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-snb-2520m/igt@runner@aborted.html
    - fi-hsw-4770:        NOTRUN -> [FAIL][61]
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-hsw-4770/igt@runner@aborted.html
    - fi-bxt-j4205:       NOTRUN -> [FAIL][62]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-bxt-j4205/igt@runner@aborted.html
    - fi-whl-u:           NOTRUN -> [FAIL][63]
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-whl-u/igt@runner@aborted.html
    - fi-icl-u3:          NOTRUN -> [FAIL][64]
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-icl-u3/igt@runner@aborted.html
    - fi-ivb-3770:        NOTRUN -> [FAIL][65]
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-ivb-3770/igt@runner@aborted.html
    - fi-bxt-dsi:         NOTRUN -> [FAIL][66]
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-bxt-dsi/igt@runner@aborted.html
    - fi-byt-j1900:       NOTRUN -> [FAIL][67]
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-byt-j1900/igt@runner@aborted.html
    - fi-cfl-guc:         NOTRUN -> [FAIL][68]
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-cfl-guc/igt@runner@aborted.html
    - fi-icl-y:           NOTRUN -> [FAIL][69]
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-icl-y/igt@runner@aborted.html
    - fi-kbl-7567u:       NOTRUN -> [FAIL][70]
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-kbl-7567u/igt@runner@aborted.html
    - fi-blb-e6850:       NOTRUN -> [FAIL][71]
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-blb-e6850/igt@runner@aborted.html
    - fi-kbl-x1275:       NOTRUN -> [FAIL][72]
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-kbl-x1275/igt@runner@aborted.html
    - fi-bsw-kefka:       NOTRUN -> [FAIL][73]
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-bsw-kefka/igt@runner@aborted.html
    - fi-cfl-8700k:       NOTRUN -> [FAIL][74]
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-cfl-8700k/igt@runner@aborted.html
    - fi-hsw-4770r:       NOTRUN -> [FAIL][75]
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-hsw-4770r/igt@runner@aborted.html
    - fi-kbl-r:           NOTRUN -> [FAIL][76]
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-kbl-r/igt@runner@aborted.html
    - fi-bdw-5557u:       NOTRUN -> [FAIL][77]
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-bdw-5557u/igt@runner@aborted.html
    - fi-bwr-2160:        NOTRUN -> [FAIL][78]
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-bwr-2160/igt@runner@aborted.html
    - fi-byt-n2820:       NOTRUN -> [FAIL][79]
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-byt-n2820/igt@runner@aborted.html
    - fi-snb-2600:        NOTRUN -> [FAIL][80]
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-snb-2600/igt@runner@aborted.html
    - fi-elk-e7500:       NOTRUN -> [FAIL][81]
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-elk-e7500/igt@runner@aborted.html

  
#### Suppressed ####

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

  * igt@debugfs_test@read_all_entries:
    - {fi-icl-dsi}:       NOTRUN -> [DMESG-WARN][82]
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-icl-dsi/igt@debugfs_test@read_all_entries.html

  * igt@gem_exec_suspend@basic-s3:
    - {fi-icl-u2}:        [PASS][83] -> [DMESG-WARN][84]
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-icl-u2/igt@gem_exec_suspend@basic-s3.html
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-icl-u2/igt@gem_exec_suspend@basic-s3.html

  * igt@kms_busy@basic-flip-a:
    - {fi-cml-u}:         [PASS][85] -> [DMESG-WARN][86]
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-cml-u/igt@kms_busy@basic-flip-a.html
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-cml-u/igt@kms_busy@basic-flip-a.html

  * igt@runner@aborted:
    - {fi-icl-u2}:        NOTRUN -> [FAIL][87]
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-icl-u2/igt@runner@aborted.html
    - {fi-cml-u2}:        NOTRUN -> [FAIL][88]
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-cml-u2/igt@runner@aborted.html
    - {fi-cml-u}:         NOTRUN -> [FAIL][89]
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-cml-u/igt@runner@aborted.html
    - {fi-icl-dsi}:       [FAIL][90] ([fdo#109593]) -> [FAIL][91]
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-icl-dsi/igt@runner@aborted.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-icl-dsi/igt@runner@aborted.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@kms_busy@basic-flip-a:
    - fi-bxt-dsi:         [PASS][92] -> [DMESG-WARN][93] ([fdo#105538])
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-bxt-dsi/igt@kms_busy@basic-flip-a.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-bxt-dsi/igt@kms_busy@basic-flip-a.html

  
#### Possible fixes ####

  * igt@gem_exec_basic@basic-blt:
    - fi-icl-y:           [INCOMPLETE][94] ([fdo#107713] / [fdo#110246] / [fdo#110581]) -> [PASS][95]
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6045/fi-icl-y/igt@gem_exec_basic@basic-blt.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12967/fi-icl-y/igt@gem_exec_basic@basic-blt.html

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

  [fdo#105538]: https://bugs.freedesktop.org/show_bug.cgi?id=105538
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#109593]: https://bugs.freedesktop.org/show_bug.cgi?id=109593
  [fdo#110246]: https://bugs.freedesktop.org/show_bug.cgi?id=110246
  [fdo#110581]: https://bugs.freedesktop.org/show_bug.cgi?id=110581


Participating hosts (51 -> 44)
------------------------------

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


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

  * Linux: CI_DRM_6045 -> Patchwork_12967

  CI_DRM_6045: 4d1b55fbee7836cb6923e51f4c39aec7030fe8f8 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4972: f052e49a43cc9704ea5f240df15dd9d3dfed68ab @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12967: a1d9c399c22adbb54ea745d11bea4ffabce9fdef @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a1d9c399c22a drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma/degamma lut values
8a0aa7fcdd4c drm/i915: Extract ilk_read_luts()
7349486b0eb6 drm/i915: Extract ivb_read_luts()
f9abafe9edf4 drm/i915: Extract bdw_read_luts()
d0c8af000d4b drm/i915: Extract glk_read_luts()
584eb0d383dc drm/i915: Extract icl_read_luts()
fa572fdd813a drm/i915: Extract i965_read_luts()
899ad49ad748 drm/i915: Extract chv_read_luts()
8780c2e9fe65 drm/i915: Extract i9xx_read_luts()
b2195f303ff1 drm/i915: Enable intel_color_read_luts()
1e8ed6586232 drm/i915: Introduce vfunc read_luts() to create hw lut

== Logs ==

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

Re: [v5][PATCH 01/11] drm/i915: Introduce vfunc read_luts() to create hw lut

[Jani Nikula]

On Sat, 04 May 2019, Swati Sharma <swati2.sharma@intel.com> wrote:
> In this patch, a vfunc read_luts() is introduced to create a hw lut
> i.e. lut having values read from gamma/degamma registers which will
> later be used to compare with sw lut to validate gamma/degamma lut values.
>
> v3: -Rebase
> v4: -Renamed intel_get_color_config to intel_color_get_config [Jani]
>     -Wrapped get_color_config() [Jani]
> v5: -Renamed intel_color_get_config() to intel_color_read_luts()

Ville, did you really want this chage? I sure didn't.

BR,
Jani.


>     -Renamed get_color_config to read_luts
>
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h    | 1 +
>  drivers/gpu/drm/i915/intel_color.c | 8 ++++++++
>  drivers/gpu/drm/i915/intel_color.h | 1 +
>  3 files changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 1cea98f..1b6d891 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -342,6 +342,7 @@ struct drm_i915_display_funcs {
>  	 * involved with the same commit.
>  	 */
>  	void (*load_luts)(const struct intel_crtc_state *crtc_state);
> +	void (*read_luts)(struct intel_crtc_state *crtc_state);
>  };
>  
>  #define CSR_VERSION(major, minor)	((major) << 16 | (minor))
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 962db12..0048d8a 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -879,6 +879,14 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
>  	return dev_priv->display.color_check(crtc_state);
>  }
>  
> +void intel_color_read_luts(struct intel_crtc_state *crtc_state)
> +{
> +	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> +
> +	if (dev_priv->display.read_luts)
> +		dev_priv->display.read_luts(crtc_state);
> +}
> +
>  static bool need_plane_update(struct intel_plane *plane,
>  			      const struct intel_crtc_state *crtc_state)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h
> index b8a3ce6..fc53de9 100644
> --- a/drivers/gpu/drm/i915/intel_color.h
> +++ b/drivers/gpu/drm/i915/intel_color.h
> @@ -13,5 +13,6 @@
>  int intel_color_check(struct intel_crtc_state *crtc_state);
>  void intel_color_commit(const struct intel_crtc_state *crtc_state);
>  void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
> +void intel_color_read_luts(struct intel_crtc_state *crtc_state);
>  
>  #endif /* __INTEL_COLOR_H__ */

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

Re: [v5][PATCH 01/11] drm/i915: Introduce vfunc read_luts() to create hw lut

[Ville Syrjälä]

On Mon, May 06, 2019 at 04:21:07PM +0300, Jani Nikula wrote:
> On Sat, 04 May 2019, Swati Sharma <swati2.sharma@intel.com> wrote:
> > In this patch, a vfunc read_luts() is introduced to create a hw lut
> > i.e. lut having values read from gamma/degamma registers which will
> > later be used to compare with sw lut to validate gamma/degamma lut values.
> >
> > v3: -Rebase
> > v4: -Renamed intel_get_color_config to intel_color_get_config [Jani]
> >     -Wrapped get_color_config() [Jani]
> > v5: -Renamed intel_color_get_config() to intel_color_read_luts()
> 
> Ville, did you really want this chage? I sure didn't.

I want the low level funcs to be called something like 
ilk_read_lut_10(), chv_read_cgm_gamma_lut(), etc.
The name of the vfunc is more of a meh.

> 
> BR,
> Jani.
> 
> 
> >     -Renamed get_color_config to read_luts
> >
> > Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h    | 1 +
> >  drivers/gpu/drm/i915/intel_color.c | 8 ++++++++
> >  drivers/gpu/drm/i915/intel_color.h | 1 +
> >  3 files changed, 10 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 1cea98f..1b6d891 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -342,6 +342,7 @@ struct drm_i915_display_funcs {
> >  	 * involved with the same commit.
> >  	 */
> >  	void (*load_luts)(const struct intel_crtc_state *crtc_state);
> > +	void (*read_luts)(struct intel_crtc_state *crtc_state);
> >  };
> >  
> >  #define CSR_VERSION(major, minor)	((major) << 16 | (minor))
> > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> > index 962db12..0048d8a 100644
> > --- a/drivers/gpu/drm/i915/intel_color.c
> > +++ b/drivers/gpu/drm/i915/intel_color.c
> > @@ -879,6 +879,14 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
> >  	return dev_priv->display.color_check(crtc_state);
> >  }
> >  
> > +void intel_color_read_luts(struct intel_crtc_state *crtc_state)
> > +{
> > +	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> > +
> > +	if (dev_priv->display.read_luts)
> > +		dev_priv->display.read_luts(crtc_state);
> > +}
> > +
> >  static bool need_plane_update(struct intel_plane *plane,
> >  			      const struct intel_crtc_state *crtc_state)
> >  {
> > diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h
> > index b8a3ce6..fc53de9 100644
> > --- a/drivers/gpu/drm/i915/intel_color.h
> > +++ b/drivers/gpu/drm/i915/intel_color.h
> > @@ -13,5 +13,6 @@
> >  int intel_color_check(struct intel_crtc_state *crtc_state);
> >  void intel_color_commit(const struct intel_crtc_state *crtc_state);
> >  void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
> > +void intel_color_read_luts(struct intel_crtc_state *crtc_state);
> >  
> >  #endif /* __INTEL_COLOR_H__ */
> 
> -- 
> Jani Nikula, Intel Open Source Graphics Center

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

Re: [v5][PATCH 01/11] drm/i915: Introduce vfunc read_luts() to create hw lut

[Jani Nikula]

On Mon, 06 May 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Mon, May 06, 2019 at 04:21:07PM +0300, Jani Nikula wrote:
>> On Sat, 04 May 2019, Swati Sharma <swati2.sharma@intel.com> wrote:
>> > In this patch, a vfunc read_luts() is introduced to create a hw lut
>> > i.e. lut having values read from gamma/degamma registers which will
>> > later be used to compare with sw lut to validate gamma/degamma lut values.
>> >
>> > v3: -Rebase
>> > v4: -Renamed intel_get_color_config to intel_color_get_config [Jani]
>> >     -Wrapped get_color_config() [Jani]
>> > v5: -Renamed intel_color_get_config() to intel_color_read_luts()
>> 
>> Ville, did you really want this chage? I sure didn't.
>
> I want the low level funcs to be called something like 
> ilk_read_lut_10(), chv_read_cgm_gamma_lut(), etc.
> The name of the vfunc is more of a meh.

And I don't care about the low level functions so much, I care about the
interface which I should emphasize the "get config" stage. :)

BR,
Jani.

>
>> 
>> BR,
>> Jani.
>> 
>> 
>> >     -Renamed get_color_config to read_luts
>> >
>> > Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_drv.h    | 1 +
>> >  drivers/gpu/drm/i915/intel_color.c | 8 ++++++++
>> >  drivers/gpu/drm/i915/intel_color.h | 1 +
>> >  3 files changed, 10 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> > index 1cea98f..1b6d891 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -342,6 +342,7 @@ struct drm_i915_display_funcs {
>> >  	 * involved with the same commit.
>> >  	 */
>> >  	void (*load_luts)(const struct intel_crtc_state *crtc_state);
>> > +	void (*read_luts)(struct intel_crtc_state *crtc_state);
>> >  };
>> >  
>> >  #define CSR_VERSION(major, minor)	((major) << 16 | (minor))
>> > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>> > index 962db12..0048d8a 100644
>> > --- a/drivers/gpu/drm/i915/intel_color.c
>> > +++ b/drivers/gpu/drm/i915/intel_color.c
>> > @@ -879,6 +879,14 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
>> >  	return dev_priv->display.color_check(crtc_state);
>> >  }
>> >  
>> > +void intel_color_read_luts(struct intel_crtc_state *crtc_state)
>> > +{
>> > +	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>> > +
>> > +	if (dev_priv->display.read_luts)
>> > +		dev_priv->display.read_luts(crtc_state);
>> > +}
>> > +
>> >  static bool need_plane_update(struct intel_plane *plane,
>> >  			      const struct intel_crtc_state *crtc_state)
>> >  {
>> > diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h
>> > index b8a3ce6..fc53de9 100644
>> > --- a/drivers/gpu/drm/i915/intel_color.h
>> > +++ b/drivers/gpu/drm/i915/intel_color.h
>> > @@ -13,5 +13,6 @@
>> >  int intel_color_check(struct intel_crtc_state *crtc_state);
>> >  void intel_color_commit(const struct intel_crtc_state *crtc_state);
>> >  void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
>> > +void intel_color_read_luts(struct intel_crtc_state *crtc_state);
>> >  
>> >  #endif /* __INTEL_COLOR_H__ */
>> 
>> -- 
>> Jani Nikula, Intel Open Source Graphics Center

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

Re: [v5][PATCH 01/11] drm/i915: Introduce vfunc read_luts() to create hw lut

[Sharma, Swati2]

On 06-May-19 7:11 PM, Jani Nikula wrote:
> On Mon, 06 May 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>> On Mon, May 06, 2019 at 04:21:07PM +0300, Jani Nikula wrote:
>>> On Sat, 04 May 2019, Swati Sharma <swati2.sharma@intel.com> wrote:
>>>> In this patch, a vfunc read_luts() is introduced to create a hw lut
>>>> i.e. lut having values read from gamma/degamma registers which will
>>>> later be used to compare with sw lut to validate gamma/degamma lut values.
>>>>
>>>> v3: -Rebase
>>>> v4: -Renamed intel_get_color_config to intel_color_get_config [Jani]
>>>>      -Wrapped get_color_config() [Jani]
>>>> v5: -Renamed intel_color_get_config() to intel_color_read_luts()
>>> Ville, did you really want this chage? I sure didn't.
>> I want the low level funcs to be called something like
>> ilk_read_lut_10(), chv_read_cgm_gamma_lut(), etc.
>> The name of the vfunc is more of a meh.
> And I don't care about the low level functions so much, I care about the
> interface which I should emphasize the "get config" stage. :)

Sure, will do that.But my major concerns are in patch 11 :/ Please review and guide.
Also, on CI tests are getting aborted..don't know why. Locally it's OK. Debugging.

>
> BR,
> Jani.
>
>>> BR,
>>> Jani.
>>>
>>>
>>>>      -Renamed get_color_config to read_luts
>>>>
>>>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/i915/i915_drv.h    | 1 +
>>>>   drivers/gpu/drm/i915/intel_color.c | 8 ++++++++
>>>>   drivers/gpu/drm/i915/intel_color.h | 1 +
>>>>   3 files changed, 10 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>> index 1cea98f..1b6d891 100644
>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>> @@ -342,6 +342,7 @@ struct drm_i915_display_funcs {
>>>>   	 * involved with the same commit.
>>>>   	 */
>>>>   	void (*load_luts)(const struct intel_crtc_state *crtc_state);
>>>> +	void (*read_luts)(struct intel_crtc_state *crtc_state);
>>>>   };
>>>>   
>>>>   #define CSR_VERSION(major, minor)	((major) << 16 | (minor))
>>>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>>>> index 962db12..0048d8a 100644
>>>> --- a/drivers/gpu/drm/i915/intel_color.c
>>>> +++ b/drivers/gpu/drm/i915/intel_color.c
>>>> @@ -879,6 +879,14 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
>>>>   	return dev_priv->display.color_check(crtc_state);
>>>>   }
>>>>   
>>>> +void intel_color_read_luts(struct intel_crtc_state *crtc_state)
>>>> +{
>>>> +	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>>>> +
>>>> +	if (dev_priv->display.read_luts)
>>>> +		dev_priv->display.read_luts(crtc_state);
>>>> +}
>>>> +
>>>>   static bool need_plane_update(struct intel_plane *plane,
>>>>   			      const struct intel_crtc_state *crtc_state)
>>>>   {
>>>> diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h
>>>> index b8a3ce6..fc53de9 100644
>>>> --- a/drivers/gpu/drm/i915/intel_color.h
>>>> +++ b/drivers/gpu/drm/i915/intel_color.h
>>>> @@ -13,5 +13,6 @@
>>>>   int intel_color_check(struct intel_crtc_state *crtc_state);
>>>>   void intel_color_commit(const struct intel_crtc_state *crtc_state);
>>>>   void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
>>>> +void intel_color_read_luts(struct intel_crtc_state *crtc_state);
>>>>   
>>>>   #endif /* __INTEL_COLOR_H__ */
>>> -- 
>>> Jani Nikula, Intel Open Source Graphics Center


-- 
~Swati Sharma

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

Re: [v5][PATCH 11/11] drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma/degamma lut values

[Ville Syrjälä]

On Sat, May 04, 2019 at 10:41:40PM +0530, Swati Sharma wrote:
> v3: Rebase
> v4: -Renamed intel_compare_color_lut() to intel_color_lut_equal() [Jani]
>     -Added the default label above the correct label [Jani]
>     -Corrected smatch warn "variable dereferenced before check" [Dan Carpenter]
> v5: -Added condition (!blob1 && !blob2) return true [Jani]
>     -Called PIPE_CONF_CHECK_COLOR_LUT inside if (!adjust) [Jani]
>     -Added #undef PIPE_CONF_CHECK_COLOR_LUT [Jani]
> 
> There are few things wrong in this patch:
> [1] For chv bit precision is 14, on what basis it should be assigned?

Like everything else it will more or less be a reverse of the
compute side. For CHV we need to look at cgm_mode, gamma_enable,
gamma_mode, and c8_planes. Hmm. We actually don't have readout for
c8_planes so I guess we'll have to make some kind of exception for
that one.

> [2] For glk and icl, degamma LUT values are not rounded off, there
> should err=0 if using same function, how to make that exception?

You mean the degamma? Just set precision==16? Maybe I'm not
understanding the question.

> [3] For glk, bit precision is 10 but gamma mode is 8BIT?

Not sure what you mean. glk gamma_mode works just like with
other ilk+ platforms (apart from not having the csc_mode
gamma vs. degamma control).

I suspect the most annoying case is ivb-bdw 10bit gamma mode
since we probably don't have the full readout to do the reverse
mapping of whether the LUT is used as gamma or degamma. But I
guess we'll just have to compare it against whichever one the
software state has.

> 
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c   | 54 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_color.h   |  6 ++++
>  drivers/gpu/drm/i915/intel_display.c | 12 ++++++++
>  3 files changed, 72 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 695b76d..73cb901 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -1630,6 +1630,60 @@ static void ilk_read_luts(struct intel_crtc_state *crtc_state)
>  		crtc_state->base.gamma_lut = ilk_read_gamma_lut(crtc_state);
>  }
>  
> +static inline bool err_check(struct drm_color_lut *sw_lut, struct drm_color_lut *hw_lut, u32 err)
> +{
> +	 return ((abs((long)hw_lut->red - sw_lut->red)) <= err) &&
> +	        ((abs((long)hw_lut->blue - sw_lut->blue)) <= err) &&
> +	        ((abs((long)hw_lut->green - sw_lut->green)) <= err);
> +}
> +
> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
> +			   struct drm_property_blob *blob2,
> +			   u32 gamma_mode)
> +{
> +	struct drm_color_lut *sw_lut, *hw_lut;
> +	int sw_lut_size, hw_lut_size, i;
> +	u32 bit_precision, err;
> +
> +	if (!blob1 && !blob2)
> +		return true;
> +
> +	if (!blob1 || !blob2)
> +		return false;
> +
> +	sw_lut_size = drm_color_lut_size(blob1);
> +	hw_lut_size = drm_color_lut_size(blob2);
> +
> +	if (sw_lut_size != hw_lut_size)
> +		return false;
> +
> +	switch(gamma_mode) {
> +	default:
> +	case GAMMA_MODE_MODE_8BIT:
> +		bit_precision = 8;
> +		break;
> +	case GAMMA_MODE_MODE_10BIT:
> +	case GAMMA_MODE_MODE_SPLIT:
> +		bit_precision = 10;
> +		break;
> +	case GAMMA_MODE_MODE_12BIT:
> +		bit_precision = 12;
> +		break;
> +	}
> +
> +	sw_lut = blob1->data;
> +	hw_lut = blob2->data;
> +
> +	err = 0xffff >> bit_precision;
> +
> +	for (i = 0; i < sw_lut_size; i++) {
> +		 if (!err_check(&hw_lut[i], &sw_lut[i], err))
> +			return false;
> +	}
> +
> +	return true;
> +}
> +
>  void intel_color_init(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h
> index fc53de9..b525c80 100644
> --- a/drivers/gpu/drm/i915/intel_color.h
> +++ b/drivers/gpu/drm/i915/intel_color.h
> @@ -6,13 +6,19 @@
>  #ifndef __INTEL_COLOR_H__
>  #define __INTEL_COLOR_H__
>  
> +#include <linux/types.h>
> +
>  struct intel_crtc_state;
>  struct intel_crtc;
> +struct drm_property_blob;
>  
>  void intel_color_init(struct intel_crtc *crtc);
>  int intel_color_check(struct intel_crtc_state *crtc_state);
>  void intel_color_commit(const struct intel_crtc_state *crtc_state);
>  void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
>  void intel_color_read_luts(struct intel_crtc_state *crtc_state);
> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
> +				 struct drm_property_blob *blob2,
> +				 u32 gamma_mode);
>  
>  #endif /* __INTEL_COLOR_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 791974b..a713171 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12287,6 +12287,14 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>  	} \
>  } while (0)
>  
> +#define PIPE_CONF_CHECK_COLOR_LUT(name, gamma_mode) do { \
> +	if (!intel_color_lut_equal(current_config->name, pipe_config->name, gamma_mode)) { \
> +		pipe_config_err(adjust, __stringify(name), \
> +				"hw_state doesn't match sw_state\n"); \
> +		ret = false; \
> +	} \
> +} while (0)
> +
>  #define PIPE_CONF_QUIRK(quirk) \
>  	((current_config->quirks | pipe_config->quirks) & (quirk))
>  
> @@ -12376,6 +12384,9 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>  			PIPE_CONF_CHECK_X(csc_mode);
>  		PIPE_CONF_CHECK_BOOL(gamma_enable);
>  		PIPE_CONF_CHECK_BOOL(csc_enable);
> +
> +		PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, pipe_config->gamma_mode);
> +		PIPE_CONF_CHECK_COLOR_LUT(base.degamma_lut, pipe_config->gamma_mode);
>  	}
>  
>  	PIPE_CONF_CHECK_BOOL(double_wide);
> @@ -12438,6 +12449,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>  #undef PIPE_CONF_CHECK_FLAGS
>  #undef PIPE_CONF_CHECK_CLOCK_FUZZY
>  #undef PIPE_CONF_QUIRK
> +#undef PIPE_CONF_CHECK_COLOR_LUT
>  
>  	return ret;
>  }
> -- 
> 1.9.1

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

Re: [v5][PATCH 11/11] drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma/degamma lut values

[Sharma, Swati2]

[-- Attachment #1.1: Type: text/plain, Size: 7035 bytes --]

On 07-May-19 12:03 AM, Ville Syrjälä wrote:

> On Sat, May 04, 2019 at 10:41:40PM +0530, Swati Sharma wrote:
>> v3: Rebase
>> v4: -Renamed intel_compare_color_lut() to intel_color_lut_equal() [Jani]
>>      -Added the default label above the correct label [Jani]
>>      -Corrected smatch warn "variable dereferenced before check" [Dan Carpenter]
>> v5: -Added condition (!blob1 && !blob2) return true [Jani]
>>      -Called PIPE_CONF_CHECK_COLOR_LUT inside if (!adjust) [Jani]
>>      -Added #undef PIPE_CONF_CHECK_COLOR_LUT [Jani]
>>
>> There are few things wrong in this patch:
>> [1] For chv bit precision is 14, on what basis it should be assigned?
> Like everything else it will more or less be a reverse of the
> compute side. For CHV we need to look at cgm_mode, gamma_enable,
> gamma_mode, and c8_planes. Hmm. We actually don't have readout for
> c8_planes so I guess we'll have to make some kind of exception for
> that one.

By this I meant was, since I am assigning bit_precision on the basis
of gamma_mode in the compare function like
+	case GAMMA_MODE_MODE_8BIT:
+		bit_precision = 8;
etc. We have 8BIT, 10BIT and 12BIT GAMMA_MODES only.How will I
assign 14BIT on the basis of GAMMA_MODES (or) is there some other
option to assign precision. Please see the comparison code below.

>
>> [2] For glk and icl, degamma LUT values are not rounded off, there
>> should err=0 if using same function, how to make that exception?
> You mean the degamma? Just set precision==16? Maybe I'm not
> understanding the question.

Again same query as above, if I set precision as 16..on what basis?
Which GAMMA_MODE? (or) default i should set as 16?

>
>> [3] For glk, bit precision is 10 but gamma mode is 8BIT?
> Not sure what you mean. glk gamma_mode works just like with
> other ilk+ platforms (apart from not having the csc_mode
> gamma vs. degamma control).

Sorry..my bad!

>
> I suspect the most annoying case is ivb-bdw 10bit gamma mode
> since we probably don't have the full readout to do the reverse
> mapping of whether the LUT is used as gamma or degamma. But I
> guess we'll just have to compare it against whichever one the
> software state has.

One last query, as there is difference in precision of gamma and
degamma..we have one comparison function..how will we get to know whether
blob received is degamma or gamma? Do we need to pass some kind of enum value
to know comparison needs to be done for gamma or degamma?

>
>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_color.c   | 54 ++++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_color.h   |  6 ++++
>>   drivers/gpu/drm/i915/intel_display.c | 12 ++++++++
>>   3 files changed, 72 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>> index 695b76d..73cb901 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -1630,6 +1630,60 @@ static void ilk_read_luts(struct intel_crtc_state *crtc_state)
>>   		crtc_state->base.gamma_lut = ilk_read_gamma_lut(crtc_state);
>>   }
>>   
>> +static inline bool err_check(struct drm_color_lut *sw_lut, struct drm_color_lut *hw_lut, u32 err)
>> +{
>> +	 return ((abs((long)hw_lut->red - sw_lut->red)) <= err) &&
>> +	        ((abs((long)hw_lut->blue - sw_lut->blue)) <= err) &&
>> +	        ((abs((long)hw_lut->green - sw_lut->green)) <= err);
>> +}
>> +
>> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
>> +			   struct drm_property_blob *blob2,
>> +			   u32 gamma_mode)
>> +{
>> +	struct drm_color_lut *sw_lut, *hw_lut;
>> +	int sw_lut_size, hw_lut_size, i;
>> +	u32 bit_precision, err;
>> +
>> +	if (!blob1 && !blob2)
>> +		return true;
>> +
>> +	if (!blob1 || !blob2)
>> +		return false;
>> +
>> +	sw_lut_size = drm_color_lut_size(blob1);
>> +	hw_lut_size = drm_color_lut_size(blob2);
>> +
>> +	if (sw_lut_size != hw_lut_size)
>> +		return false;
>> +
>> +	switch(gamma_mode) {
>> +	default:
>> +	case GAMMA_MODE_MODE_8BIT:
>> +		bit_precision = 8;
>> +		break;
>> +	case GAMMA_MODE_MODE_10BIT:
>> +	case GAMMA_MODE_MODE_SPLIT:
>> +		bit_precision = 10;
>> +		break;
>> +	case GAMMA_MODE_MODE_12BIT:
>> +		bit_precision = 12;
>> +		break;
>> +	}
>> +
>> +	sw_lut = blob1->data;
>> +	hw_lut = blob2->data;
>> +
>> +	err = 0xffff >> bit_precision;
>> +
>> +	for (i = 0; i < sw_lut_size; i++) {
>> +		 if (!err_check(&hw_lut[i], &sw_lut[i], err))
>> +			return false;
>> +	}
>> +
>> +	return true;
>> +}
>> +
>>   void intel_color_init(struct intel_crtc *crtc)
>>   {
>>   	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>> diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h
>> index fc53de9..b525c80 100644
>> --- a/drivers/gpu/drm/i915/intel_color.h
>> +++ b/drivers/gpu/drm/i915/intel_color.h
>> @@ -6,13 +6,19 @@
>>   #ifndef __INTEL_COLOR_H__
>>   #define __INTEL_COLOR_H__
>>   
>> +#include <linux/types.h>
>> +
>>   struct intel_crtc_state;
>>   struct intel_crtc;
>> +struct drm_property_blob;
>>   
>>   void intel_color_init(struct intel_crtc *crtc);
>>   int intel_color_check(struct intel_crtc_state *crtc_state);
>>   void intel_color_commit(const struct intel_crtc_state *crtc_state);
>>   void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
>>   void intel_color_read_luts(struct intel_crtc_state *crtc_state);
>> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
>> +				 struct drm_property_blob *blob2,
>> +				 u32 gamma_mode);
>>   
>>   #endif /* __INTEL_COLOR_H__ */
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 791974b..a713171 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -12287,6 +12287,14 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>>   	} \
>>   } while (0)
>>   
>> +#define PIPE_CONF_CHECK_COLOR_LUT(name, gamma_mode) do { \
>> +	if (!intel_color_lut_equal(current_config->name, pipe_config->name, gamma_mode)) { \
>> +		pipe_config_err(adjust, __stringify(name), \
>> +				"hw_state doesn't match sw_state\n"); \
>> +		ret = false; \
>> +	} \
>> +} while (0)
>> +
>>   #define PIPE_CONF_QUIRK(quirk) \
>>   	((current_config->quirks | pipe_config->quirks) & (quirk))
>>   
>> @@ -12376,6 +12384,9 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>>   			PIPE_CONF_CHECK_X(csc_mode);
>>   		PIPE_CONF_CHECK_BOOL(gamma_enable);
>>   		PIPE_CONF_CHECK_BOOL(csc_enable);
>> +
>> +		PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, pipe_config->gamma_mode);
>> +		PIPE_CONF_CHECK_COLOR_LUT(base.degamma_lut, pipe_config->gamma_mode);
>>   	}
>>   
>>   	PIPE_CONF_CHECK_BOOL(double_wide);
>> @@ -12438,6 +12449,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>>   #undef PIPE_CONF_CHECK_FLAGS
>>   #undef PIPE_CONF_CHECK_CLOCK_FUZZY
>>   #undef PIPE_CONF_QUIRK
>> +#undef PIPE_CONF_CHECK_COLOR_LUT
>>   
>>   	return ret;
>>   }
>> -- 
>> 1.9.1

-- 
~Swati Sharma


[-- Attachment #1.2: Type: text/html, Size: 8351 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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

Re: [v5][PATCH 01/11] drm/i915: Introduce vfunc read_luts() to create hw lut

[Jani Nikula]

On Mon, 06 May 2019, "Sharma, Swati2" <swati2.sharma@intel.com> wrote:
> On 06-May-19 7:11 PM, Jani Nikula wrote:
>> On Mon, 06 May 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>>> On Mon, May 06, 2019 at 04:21:07PM +0300, Jani Nikula wrote:
>>>> On Sat, 04 May 2019, Swati Sharma <swati2.sharma@intel.com> wrote:
>>>>> In this patch, a vfunc read_luts() is introduced to create a hw lut
>>>>> i.e. lut having values read from gamma/degamma registers which will
>>>>> later be used to compare with sw lut to validate gamma/degamma lut values.
>>>>>
>>>>> v3: -Rebase
>>>>> v4: -Renamed intel_get_color_config to intel_color_get_config [Jani]
>>>>>      -Wrapped get_color_config() [Jani]
>>>>> v5: -Renamed intel_color_get_config() to intel_color_read_luts()
>>>> Ville, did you really want this chage? I sure didn't.
>>> I want the low level funcs to be called something like
>>> ilk_read_lut_10(), chv_read_cgm_gamma_lut(), etc.
>>> The name of the vfunc is more of a meh.
>> And I don't care about the low level functions so much, I care about the
>> interface which I should emphasize the "get config" stage. :)
>
> Sure, will do that.But my major concerns are in patch 11 :/ Please review and guide.
> Also, on CI tests are getting aborted..don't know why. Locally it's OK. Debugging.

Please move patch 11 right after current patch 2. This was the point in
making the vfuncs optional; you can then add platforms one by one and
each change is meaningful. Now you add everything but only really enable
the checks in one big lump at the end.

BR,
Jani.


>
>>
>> BR,
>> Jani.
>>
>>>> BR,
>>>> Jani.
>>>>
>>>>
>>>>>      -Renamed get_color_config to read_luts
>>>>>
>>>>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>>>>> ---
>>>>>   drivers/gpu/drm/i915/i915_drv.h    | 1 +
>>>>>   drivers/gpu/drm/i915/intel_color.c | 8 ++++++++
>>>>>   drivers/gpu/drm/i915/intel_color.h | 1 +
>>>>>   3 files changed, 10 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>>> index 1cea98f..1b6d891 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>> @@ -342,6 +342,7 @@ struct drm_i915_display_funcs {
>>>>>   	 * involved with the same commit.
>>>>>   	 */
>>>>>   	void (*load_luts)(const struct intel_crtc_state *crtc_state);
>>>>> +	void (*read_luts)(struct intel_crtc_state *crtc_state);
>>>>>   };
>>>>>   
>>>>>   #define CSR_VERSION(major, minor)	((major) << 16 | (minor))
>>>>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>>>>> index 962db12..0048d8a 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_color.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_color.c
>>>>> @@ -879,6 +879,14 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
>>>>>   	return dev_priv->display.color_check(crtc_state);
>>>>>   }
>>>>>   
>>>>> +void intel_color_read_luts(struct intel_crtc_state *crtc_state)
>>>>> +{
>>>>> +	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>>>>> +
>>>>> +	if (dev_priv->display.read_luts)
>>>>> +		dev_priv->display.read_luts(crtc_state);
>>>>> +}
>>>>> +
>>>>>   static bool need_plane_update(struct intel_plane *plane,
>>>>>   			      const struct intel_crtc_state *crtc_state)
>>>>>   {
>>>>> diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h
>>>>> index b8a3ce6..fc53de9 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_color.h
>>>>> +++ b/drivers/gpu/drm/i915/intel_color.h
>>>>> @@ -13,5 +13,6 @@
>>>>>   int intel_color_check(struct intel_crtc_state *crtc_state);
>>>>>   void intel_color_commit(const struct intel_crtc_state *crtc_state);
>>>>>   void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
>>>>> +void intel_color_read_luts(struct intel_crtc_state *crtc_state);
>>>>>   
>>>>>   #endif /* __INTEL_COLOR_H__ */
>>>> -- 
>>>> Jani Nikula, Intel Open Source Graphics Center

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

Re: [v5][PATCH 01/11] drm/i915: Introduce vfunc read_luts() to create hw lut

[Jani Nikula]

On Tue, 07 May 2019, Jani Nikula <jani.nikula@intel.com> wrote:
> On Mon, 06 May 2019, "Sharma, Swati2" <swati2.sharma@intel.com> wrote:
>> On 06-May-19 7:11 PM, Jani Nikula wrote:
>>> On Mon, 06 May 2019, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
>>>> On Mon, May 06, 2019 at 04:21:07PM +0300, Jani Nikula wrote:
>>>>> On Sat, 04 May 2019, Swati Sharma <swati2.sharma@intel.com> wrote:
>>>>>> In this patch, a vfunc read_luts() is introduced to create a hw lut
>>>>>> i.e. lut having values read from gamma/degamma registers which will
>>>>>> later be used to compare with sw lut to validate gamma/degamma lut values.
>>>>>>
>>>>>> v3: -Rebase
>>>>>> v4: -Renamed intel_get_color_config to intel_color_get_config [Jani]
>>>>>>      -Wrapped get_color_config() [Jani]
>>>>>> v5: -Renamed intel_color_get_config() to intel_color_read_luts()
>>>>> Ville, did you really want this chage? I sure didn't.
>>>> I want the low level funcs to be called something like
>>>> ilk_read_lut_10(), chv_read_cgm_gamma_lut(), etc.
>>>> The name of the vfunc is more of a meh.
>>> And I don't care about the low level functions so much, I care about the
>>> interface which I should emphasize the "get config" stage. :)
>>
>> Sure, will do that.But my major concerns are in patch 11 :/ Please review and guide.
>> Also, on CI tests are getting aborted..don't know why. Locally it's OK. Debugging.
>
> Please move patch 11 right after current patch 2. This was the point in
> making the vfuncs optional; you can then add platforms one by one and
> each change is meaningful. Now you add everything but only really enable
> the checks in one big lump at the end.

Please add a "color" line to intel_dump_pipe_config() with gamma_enable,
gamma_mode, csc_enable, cgm_mode (chv) / csc_mode (others). We don't see
their values in the compare failure, but they check out so they're
equal. My quick guess is gamma is disabled at takeover and the the lut
comparison is pointless.

So I think we'll need to make the lut check in
intel_pipe_config_compare() conditional to gamma actually being enabled.

If that still fails, we'll need to add a lut specific error printer in
PIPE_CONF_CHECK_COLOR_LUT() that will dump the luts. Similar to
pipe_config_infoframe_err() in PIPE_CONF_CHECK_INFOFRAME().

BR,
Jani.



>
> BR,
> Jani.
>
>
>>
>>>
>>> BR,
>>> Jani.
>>>
>>>>> BR,
>>>>> Jani.
>>>>>
>>>>>
>>>>>>      -Renamed get_color_config to read_luts
>>>>>>
>>>>>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>>>>>> ---
>>>>>>   drivers/gpu/drm/i915/i915_drv.h    | 1 +
>>>>>>   drivers/gpu/drm/i915/intel_color.c | 8 ++++++++
>>>>>>   drivers/gpu/drm/i915/intel_color.h | 1 +
>>>>>>   3 files changed, 10 insertions(+)
>>>>>>
>>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>>>>> index 1cea98f..1b6d891 100644
>>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>>> @@ -342,6 +342,7 @@ struct drm_i915_display_funcs {
>>>>>>   	 * involved with the same commit.
>>>>>>   	 */
>>>>>>   	void (*load_luts)(const struct intel_crtc_state *crtc_state);
>>>>>> +	void (*read_luts)(struct intel_crtc_state *crtc_state);
>>>>>>   };
>>>>>>   
>>>>>>   #define CSR_VERSION(major, minor)	((major) << 16 | (minor))
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>>>>>> index 962db12..0048d8a 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_color.c
>>>>>> +++ b/drivers/gpu/drm/i915/intel_color.c
>>>>>> @@ -879,6 +879,14 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
>>>>>>   	return dev_priv->display.color_check(crtc_state);
>>>>>>   }
>>>>>>   
>>>>>> +void intel_color_read_luts(struct intel_crtc_state *crtc_state)
>>>>>> +{
>>>>>> +	struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
>>>>>> +
>>>>>> +	if (dev_priv->display.read_luts)
>>>>>> +		dev_priv->display.read_luts(crtc_state);
>>>>>> +}
>>>>>> +
>>>>>>   static bool need_plane_update(struct intel_plane *plane,
>>>>>>   			      const struct intel_crtc_state *crtc_state)
>>>>>>   {
>>>>>> diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h
>>>>>> index b8a3ce6..fc53de9 100644
>>>>>> --- a/drivers/gpu/drm/i915/intel_color.h
>>>>>> +++ b/drivers/gpu/drm/i915/intel_color.h
>>>>>> @@ -13,5 +13,6 @@
>>>>>>   int intel_color_check(struct intel_crtc_state *crtc_state);
>>>>>>   void intel_color_commit(const struct intel_crtc_state *crtc_state);
>>>>>>   void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
>>>>>> +void intel_color_read_luts(struct intel_crtc_state *crtc_state);
>>>>>>   
>>>>>>   #endif /* __INTEL_COLOR_H__ */
>>>>> -- 
>>>>> Jani Nikula, Intel Open Source Graphics Center

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

Re: [v5][PATCH 11/11] drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma/degamma lut values

[Ville Syrjälä]

On Tue, May 07, 2019 at 12:27:07AM +0530, Sharma, Swati2 wrote:
> On 07-May-19 12:03 AM, Ville Syrjälä wrote:
> 
> > On Sat, May 04, 2019 at 10:41:40PM +0530, Swati Sharma wrote:
> >> v3: Rebase
> >> v4: -Renamed intel_compare_color_lut() to intel_color_lut_equal() [Jani]
> >>      -Added the default label above the correct label [Jani]
> >>      -Corrected smatch warn "variable dereferenced before check" [Dan Carpenter]
> >> v5: -Added condition (!blob1 && !blob2) return true [Jani]
> >>      -Called PIPE_CONF_CHECK_COLOR_LUT inside if (!adjust) [Jani]
> >>      -Added #undef PIPE_CONF_CHECK_COLOR_LUT [Jani]
> >>
> >> There are few things wrong in this patch:
> >> [1] For chv bit precision is 14, on what basis it should be assigned?
> > Like everything else it will more or less be a reverse of the
> > compute side. For CHV we need to look at cgm_mode, gamma_enable,
> > gamma_mode, and c8_planes. Hmm. We actually don't have readout for
> > c8_planes so I guess we'll have to make some kind of exception for
> > that one.
> 
> By this I meant was, since I am assigning bit_precision on the basis
> of gamma_mode in the compare function like
> +	case GAMMA_MODE_MODE_8BIT:
> +		bit_precision = 8;
> etc. We have 8BIT, 10BIT and 12BIT GAMMA_MODES only.How will I
> assign 14BIT on the basis of GAMMA_MODES (or) is there some other
> option to assign precision. Please see the comparison code below.

Yeah, gamma_mode by itself is not sufficient in some of the cases.
It's highly platform dependent how you figure out the precision.

> 
> >
> >> [2] For glk and icl, degamma LUT values are not rounded off, there
> >> should err=0 if using same function, how to make that exception?
> > You mean the degamma? Just set precision==16? Maybe I'm not
> > understanding the question.
> 
> Again same query as above, if I set precision as 16..on what basis?
> Which GAMMA_MODE? (or) default i should set as 16?

The glk+ degamma is always 16bpc, so you just hardcode it.

> 
> >
> >> [3] For glk, bit precision is 10 but gamma mode is 8BIT?
> > Not sure what you mean. glk gamma_mode works just like with
> > other ilk+ platforms (apart from not having the csc_mode
> > gamma vs. degamma control).
> 
> Sorry..my bad!
> 
> >
> > I suspect the most annoying case is ivb-bdw 10bit gamma mode
> > since we probably don't have the full readout to do the reverse
> > mapping of whether the LUT is used as gamma or degamma. But I
> > guess we'll just have to compare it against whichever one the
> > software state has.
> 
> One last query, as there is difference in precision of gamma and
> degamma..we have one comparison function..how will we get to know whether
> blob received is degamma or gamma? Do we need to pass some kind of enum value
> to know comparison needs to be done for gamma or degamma?

I think the comparison should just compare two blobs. Doesn't matter
what they are really.

> 
> >
> >> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_color.c   | 54 ++++++++++++++++++++++++++++++++++++
> >>   drivers/gpu/drm/i915/intel_color.h   |  6 ++++
> >>   drivers/gpu/drm/i915/intel_display.c | 12 ++++++++
> >>   3 files changed, 72 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> >> index 695b76d..73cb901 100644
> >> --- a/drivers/gpu/drm/i915/intel_color.c
> >> +++ b/drivers/gpu/drm/i915/intel_color.c
> >> @@ -1630,6 +1630,60 @@ static void ilk_read_luts(struct intel_crtc_state *crtc_state)
> >>   		crtc_state->base.gamma_lut = ilk_read_gamma_lut(crtc_state);
> >>   }
> >>   
> >> +static inline bool err_check(struct drm_color_lut *sw_lut, struct drm_color_lut *hw_lut, u32 err)
> >> +{
> >> +	 return ((abs((long)hw_lut->red - sw_lut->red)) <= err) &&
> >> +	        ((abs((long)hw_lut->blue - sw_lut->blue)) <= err) &&
> >> +	        ((abs((long)hw_lut->green - sw_lut->green)) <= err);
> >> +}
> >> +
> >> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
> >> +			   struct drm_property_blob *blob2,
> >> +			   u32 gamma_mode)
> >> +{
> >> +	struct drm_color_lut *sw_lut, *hw_lut;
> >> +	int sw_lut_size, hw_lut_size, i;
> >> +	u32 bit_precision, err;
> >> +
> >> +	if (!blob1 && !blob2)
> >> +		return true;
> >> +
> >> +	if (!blob1 || !blob2)
> >> +		return false;
> >> +
> >> +	sw_lut_size = drm_color_lut_size(blob1);
> >> +	hw_lut_size = drm_color_lut_size(blob2);
> >> +
> >> +	if (sw_lut_size != hw_lut_size)
> >> +		return false;
> >> +
> >> +	switch(gamma_mode) {
> >> +	default:
> >> +	case GAMMA_MODE_MODE_8BIT:
> >> +		bit_precision = 8;
> >> +		break;
> >> +	case GAMMA_MODE_MODE_10BIT:
> >> +	case GAMMA_MODE_MODE_SPLIT:
> >> +		bit_precision = 10;
> >> +		break;
> >> +	case GAMMA_MODE_MODE_12BIT:
> >> +		bit_precision = 12;
> >> +		break;
> >> +	}
> >> +
> >> +	sw_lut = blob1->data;
> >> +	hw_lut = blob2->data;
> >> +
> >> +	err = 0xffff >> bit_precision;
> >> +
> >> +	for (i = 0; i < sw_lut_size; i++) {
> >> +		 if (!err_check(&hw_lut[i], &sw_lut[i], err))
> >> +			return false;
> >> +	}
> >> +
> >> +	return true;
> >> +}
> >> +
> >>   void intel_color_init(struct intel_crtc *crtc)
> >>   {
> >>   	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >> diff --git a/drivers/gpu/drm/i915/intel_color.h b/drivers/gpu/drm/i915/intel_color.h
> >> index fc53de9..b525c80 100644
> >> --- a/drivers/gpu/drm/i915/intel_color.h
> >> +++ b/drivers/gpu/drm/i915/intel_color.h
> >> @@ -6,13 +6,19 @@
> >>   #ifndef __INTEL_COLOR_H__
> >>   #define __INTEL_COLOR_H__
> >>   
> >> +#include <linux/types.h>
> >> +
> >>   struct intel_crtc_state;
> >>   struct intel_crtc;
> >> +struct drm_property_blob;
> >>   
> >>   void intel_color_init(struct intel_crtc *crtc);
> >>   int intel_color_check(struct intel_crtc_state *crtc_state);
> >>   void intel_color_commit(const struct intel_crtc_state *crtc_state);
> >>   void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
> >>   void intel_color_read_luts(struct intel_crtc_state *crtc_state);
> >> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
> >> +				 struct drm_property_blob *blob2,
> >> +				 u32 gamma_mode);
> >>   
> >>   #endif /* __INTEL_COLOR_H__ */
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 791974b..a713171 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -12287,6 +12287,14 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
> >>   	} \
> >>   } while (0)
> >>   
> >> +#define PIPE_CONF_CHECK_COLOR_LUT(name, gamma_mode) do { \
> >> +	if (!intel_color_lut_equal(current_config->name, pipe_config->name, gamma_mode)) { \
> >> +		pipe_config_err(adjust, __stringify(name), \
> >> +				"hw_state doesn't match sw_state\n"); \
> >> +		ret = false; \
> >> +	} \
> >> +} while (0)
> >> +
> >>   #define PIPE_CONF_QUIRK(quirk) \
> >>   	((current_config->quirks | pipe_config->quirks) & (quirk))
> >>   
> >> @@ -12376,6 +12384,9 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
> >>   			PIPE_CONF_CHECK_X(csc_mode);
> >>   		PIPE_CONF_CHECK_BOOL(gamma_enable);
> >>   		PIPE_CONF_CHECK_BOOL(csc_enable);
> >> +
> >> +		PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, pipe_config->gamma_mode);
> >> +		PIPE_CONF_CHECK_COLOR_LUT(base.degamma_lut, pipe_config->gamma_mode);
> >>   	}
> >>   
> >>   	PIPE_CONF_CHECK_BOOL(double_wide);
> >> @@ -12438,6 +12449,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
> >>   #undef PIPE_CONF_CHECK_FLAGS
> >>   #undef PIPE_CONF_CHECK_CLOCK_FUZZY
> >>   #undef PIPE_CONF_QUIRK
> >> +#undef PIPE_CONF_CHECK_COLOR_LUT
> >>   
> >>   	return ret;
> >>   }
> >> -- 
> >> 1.9.1
> 
> -- 
> ~Swati Sharma
> 

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