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

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

[Swati Sharma]

In this patch series, added state checker to validate gamma
and will be extended to validate degamma lut values aswell.
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)
v2: -Restructured code and created platform specific patch series for
     gamma validation
v3: -Rebase
v4: -Minor changes-function name changes mainly
v5: -Added degamma validation (Ville)
v6: -Removed degamma changes, debugging was becoming difficult
    -Added function to assign bit_precision for gamma/degamma
     lut values /platform
    -Added debug info into intel_dump_pipe_config() (Jani)
v7: -Added platform specific functions to compute gamma bit precision
     on the basis of GAMMA_MODE (Ville)
    -Corrected checkpatch warnings

Swati Sharma (12):
  drm/i915: Introduce vfunc read_luts() to create hw lut
  drm/i915: Enable intel_color_get_config()
  drm/i915: Add func to compare hw/sw gamma lut
  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()
  FOR_TESTING_ONLY: Print rgb values of hw and sw blobs

 drivers/gpu/drm/i915/i915_drv.h      |   1 +
 drivers/gpu/drm/i915/i915_reg.h      |  15 ++
 drivers/gpu/drm/i915/intel_color.c   | 465 ++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_color.h   |   8 +
 drivers/gpu/drm/i915/intel_display.c |  28 +++
 5 files changed, 512 insertions(+), 5 deletions(-)

-- 
1.9.1

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

[v7][PATCH 01/12] 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
v6: -Renamed intel_color_read_luts() back to intel_color_get_config()
     [Jani and Ville]

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 d025780..6343e70 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -343,6 +343,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);
 };
 
 struct intel_csr {
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 962db12..50b98ee 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_get_config(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..057e8ac 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_get_config(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

[v7][PATCH 02/12] drm/i915: Enable intel_color_get_config()

[Swati Sharma]

In this patch, intel_color_get_config() 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]
v6: -Renamed intel_color_read_luts() to intel_color_get_config()
     [Jani and 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 05177f3..3e01028 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8351,6 +8351,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_get_config(pipe_config);
 
 	if (INTEL_GEN(dev_priv) < 4)
 		pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
@@ -9426,6 +9427,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_get_config(pipe_config);
 
 	if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
 		struct intel_shared_dpll *pll;
@@ -9874,6 +9876,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 		i9xx_get_pipe_color_config(pipe_config);
 	}
 
+	intel_color_get_config(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

[v7][PATCH 03/12] drm/i915: Add func to compare hw/sw gamma lut

[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]
v6: -Added func intel_color_get_bit_precision() to get bit precision for
     gamma and degamma lut readout depending upon platform and
     corresponding to load_luts() [Ankit]
    -Added debug log for color para in intel_dump_pipe_config [Jani]
    -Made patch11 as patch3 [Jani]
v7: -Renamed func intel_color_get_bit_precision() to
     intel_color_get_gamma_bit_precision()
    -Added separate function/platform for gamma bit precision [Ville]
    -Corrected checkpatch warnings

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

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 50b98ee..b20a2c6 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1251,6 +1251,172 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
 	return 0;
 }
 
+static int i9xx_gamma_precision(struct intel_crtc_state *crtc_state)
+{
+	if (!crtc_state->gamma_enable)
+		return 0;
+
+	switch (crtc_state->gamma_mode) {
+	case GAMMA_MODE_MODE_8BIT:
+		return 8;
+	case GAMMA_MODE_MODE_10BIT:
+		return 16;
+	default:
+		MISSING_CASE(crtc_state->gamma_mode);
+		return 0;
+	}
+}
+
+static int chv_gamma_precision(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
+		return 10;
+	else
+		return i9xx_gamma_precision(crtc_state);
+}
+
+static int ilk_gamma_precision(struct intel_crtc_state *crtc_state)
+{
+	if (!crtc_state->gamma_enable)
+		return 0;
+
+	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
+		return 0;
+
+	switch (crtc_state->gamma_mode) {
+	case GAMMA_MODE_MODE_8BIT:
+		return 8;
+	case GAMMA_MODE_MODE_10BIT:
+		return 10;
+	default:
+		MISSING_CASE(crtc_state->gamma_mode);
+		return 0;
+	}
+}
+
+static int ivb_gamma_precision(struct intel_crtc_state *crtc_state)
+{
+	if (!crtc_state->gamma_enable)
+		return 0;
+
+	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
+		return 0;
+
+	switch (crtc_state->gamma_mode) {
+	case GAMMA_MODE_MODE_8BIT:
+		return 8;
+	case GAMMA_MODE_MODE_SPLIT:
+	case GAMMA_MODE_MODE_10BIT:
+		return 10;
+	default:
+		MISSING_CASE(crtc_state->gamma_mode);
+		return 0;
+	}
+}
+
+static int glk_gamma_precision(struct intel_crtc_state *crtc_state)
+{
+	if (!crtc_state->gamma_enable)
+		return 0;
+
+	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
+		return 0;
+
+	switch (crtc_state->gamma_mode) {
+	case GAMMA_MODE_MODE_8BIT:
+		return 8;
+	case GAMMA_MODE_MODE_10BIT:
+		return 10;
+	default:
+		MISSING_CASE(crtc_state->gamma_mode);
+		return 0;
+	}
+}
+
+static int icl_gamma_precision(struct intel_crtc_state *crtc_state)
+{
+	if ((crtc_state->gamma_mode & PRE_CSC_GAMMA_ENABLE) == 0)
+		return 0;
+
+	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
+	case GAMMA_MODE_MODE_8BIT:
+		return 8;
+	case GAMMA_MODE_MODE_10BIT:
+		return 10;
+	default:
+		MISSING_CASE(crtc_state->gamma_mode);
+		return 0;
+	}
+}
+
+int intel_color_get_gamma_bit_precision(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);
+
+	if (HAS_GMCH(dev_priv)) {
+		if (IS_CHERRYVIEW(dev_priv))
+			return chv_gamma_precision(crtc_state);
+		else
+			return i9xx_gamma_precision(crtc_state);
+	} else {
+		if (INTEL_GEN(dev_priv) >= 11)
+			return icl_gamma_precision(crtc_state);
+		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+			return glk_gamma_precision(crtc_state);
+		else if (INTEL_GEN(dev_priv) >= 7)
+			return ivb_gamma_precision(crtc_state);
+		else
+			return ilk_gamma_precision(crtc_state);
+	}
+
+	return 0;
+}
+
+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 bit_precision)
+{
+	struct drm_color_lut *sw_lut, *hw_lut;
+	int sw_lut_size, hw_lut_size, i;
+	u32 err;
+
+	if (!blob1 && !blob2)
+		return true;
+
+	if (!blob1)
+		return true;
+
+	if (!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;
+
+	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 057e8ac..02ea1bc 100644
--- a/drivers/gpu/drm/i915/intel_color.h
+++ b/drivers/gpu/drm/i915/intel_color.h
@@ -6,13 +6,20 @@
 #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_get_config(struct intel_crtc_state *crtc_state);
+bool intel_color_lut_equal(struct drm_property_blob *blob1,
+			   struct drm_property_blob *blob2,
+			   u32 bit_precision);
+int intel_color_get_gamma_bit_precision(struct intel_crtc_state *crtc_state);
 
 #endif /* __INTEL_COLOR_H__ */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3e01028..b8ff3f4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11570,6 +11570,15 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 				      drm_rect_width(&state->base.dst),
 				      drm_rect_height(&state->base.dst));
 	}
+
+	if (IS_CHERRYVIEW(dev_priv))
+		DRM_DEBUG_KMS("cgm_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
+			       pipe_config->cgm_mode, pipe_config->gamma_mode,
+			       pipe_config->gamma_enable, pipe_config->csc_enable);
+	else
+		DRM_DEBUG_KMS("csc_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
+			       pipe_config->csc_mode, pipe_config->gamma_mode,
+			       pipe_config->gamma_enable, pipe_config->csc_enable);
 }
 
 static bool check_digital_port_conflicts(struct drm_atomic_state *state)
@@ -11947,6 +11956,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 			  bool adjust)
 {
 	bool ret = true;
+	u32 bp_gamma = 0;
 	bool fixup_inherited = adjust &&
 		(current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
 		!(pipe_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
@@ -12098,6 +12108,15 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 	} \
 } while (0)
 
+#define PIPE_CONF_CHECK_COLOR_LUT(name, bit_precision) do { \
+	if (!intel_color_lut_equal(current_config->name, \
+				   pipe_config->name, bit_precision)) { \
+		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))
 
@@ -12193,6 +12212,10 @@ 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);
+
+		bp_gamma = intel_color_get_gamma_bit_precision(pipe_config);
+		if (bp_gamma)
+			PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, bp_gamma);
 	}
 
 	PIPE_CONF_CHECK_BOOL(double_wide);
@@ -12255,6 +12278,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

[v7][PATCH 04/12] 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 within 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 e97c47f..d8475f2 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 b20a2c6..e37e902 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1417,6 +1417,56 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
 	return true;
 }
 
+/* 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);
@@ -1437,6 +1487,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

[v7][PATCH 05/12] 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]

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 d8475f2..b58c66d 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10160,6 +10160,9 @@ 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_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 e37e902..6ed851b 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1467,6 +1467,44 @@ 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_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.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);
@@ -1479,6 +1517,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

[v7][PATCH 06/12] 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 b58c66d..7988fa5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3584,6 +3584,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 6ed851b..3ec84af 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1505,6 +1505,44 @@ static void chv_read_luts(struct intel_crtc_state *crtc_state)
 		crtc_state->base.gamma_lut = chv_read_cgm_gamma_lut(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);
@@ -1522,6 +1560,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

[v7][PATCH 07/12] drm/i915: Extract icl_read_luts()

[Swati Sharma]

In this patch, gamma 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]

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7988fa5..249296b 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
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 3ec84af..7271f1a 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1543,6 +1543,50 @@ 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 *
+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)
+{
+	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);
@@ -1584,8 +1628,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

[v7][PATCH 08/12] 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 degamma validation [Ville]

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

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 7271f1a..1cf83c4 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1587,6 +1587,14 @@ 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->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);
@@ -1631,9 +1639,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

[v7][PATCH 09/12] 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]

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

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 1cf83c4..b349932 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1595,6 +1595,17 @@ 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.gamma_lut = bdw_read_lut_10(crtc_state, PAL_PREC_SPLIT_MODE |
+			                            	     PAL_PREC_INDEX_VALUE(512));
+	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);
@@ -1642,9 +1653,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

[v7][PATCH 10/12] 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]

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

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index b349932..6e6e54b 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1606,6 +1606,50 @@ static void bdw_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 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.gamma_lut = ivb_read_lut_10(crtc_state, PAL_PREC_SPLIT_MODE |
+							     PAL_PREC_INDEX_VALUE(512));
+	else
+		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);
@@ -1656,9 +1700,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

[v7][PATCH 11/12] 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 249296b..d5ff323 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 6e6e54b..7568b13 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1650,6 +1650,43 @@ static void ivb_read_luts(struct intel_crtc_state *crtc_state)
 		crtc_state->base.gamma_lut = ivb_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
 }
 
+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);
@@ -1703,9 +1740,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

[v7][PATCH 12/12] FOR_TESTING_ONLY: Print rgb values of hw and sw blobs

[Swati Sharma]

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

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 7568b13..c27280e 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1376,6 +1376,8 @@ int intel_color_get_gamma_bit_precision(struct intel_crtc_state *crtc_state)
 static inline bool err_check(struct drm_color_lut *sw_lut,
 			     struct drm_color_lut *hw_lut, u32 err)
 {
+	DRM_DEBUG_KMS("hw_lut->red=0x%x sw_lut->red=0x%x hw_lut->blue=0x%x sw_lut->blue=0x%x hw_lut->green=0x%x sw_lut->green=0x%x", hw_lut->red, sw_lut->red, hw_lut->blue, sw_lut->blue, hw_lut->green, sw_lut->green);
+
 	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);
-- 
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 (rev12)

[Patchwork]

== Series Details ==

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

== Summary ==

$ dim checkpatch origin/drm-tip
1cfa0479b4b6 drm/i915: Introduce vfunc read_luts() to create hw lut
22a23431817f drm/i915: Enable intel_color_get_config()
8d9a348d4327 drm/i915: Add func to compare hw/sw gamma lut
-:239: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#239: FILE: drivers/gpu/drm/i915/intel_display.c:11925:
+		DRM_DEBUG_KMS("cgm_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
+			       pipe_config->cgm_mode, pipe_config->gamma_mode,

-:243: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#243: FILE: drivers/gpu/drm/i915/intel_display.c:11929:
+		DRM_DEBUG_KMS("csc_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
+			       pipe_config->csc_mode, pipe_config->gamma_mode,

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

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

total: 0 errors, 0 warnings, 4 checks, 246 lines checked
f77fdd96babd drm/i915: Extract i9xx_read_luts()
-:79: WARNING:LONG_LINE: line over 100 characters
#79: FILE: drivers/gpu/drm/i915/intel_color.c:1459:
+		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:1460:
+		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:1461:
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_BLUE_MASK, val), 8);

total: 0 errors, 3 warnings, 0 checks, 72 lines checked
efd33dc03527 drm/i915: Extract chv_read_luts()
-:62: WARNING:LONG_LINE: line over 100 characters
#62: FILE: drivers/gpu/drm/i915/intel_color.c:1492:
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_GREEN_MASK, val), 10);

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

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

total: 0 errors, 3 warnings, 0 checks, 60 lines checked
249db384188a drm/i915: Extract i965_read_luts()
-:63: WARNING:LONG_LINE: line over 100 characters
#63: FILE: drivers/gpu/drm/i915/intel_color.c:1532:
+		blob_data[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val1) << 8 | REG_FIELD_GET(PALETTE_RED_MASK, val2);

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

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

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

total: 0 errors, 4 warnings, 0 checks, 60 lines checked
bc2cf6233920 drm/i915: Extract icl_read_luts()
-:65: WARNING:LONG_LINE: line over 100 characters
#65: 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);

-:66: WARNING:LONG_LINE: line over 100 characters
#66: 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);

-:67: WARNING:LONG_LINE: line over 100 characters
#67: 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);

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

total: 0 errors, 3 warnings, 1 checks, 70 lines checked
d9e96249b9c8 drm/i915: Extract glk_read_luts()
b58a2fab4b53 drm/i915: Extract bdw_read_luts()
-:31: ERROR:CODE_INDENT: code indent should use tabs where possible
#31: FILE: drivers/gpu/drm/i915/intel_color.c:1606:
+^I^I^I                            ^I     PAL_PREC_INDEX_VALUE(512));$

-:31: WARNING:SPACE_BEFORE_TAB: please, no space before tabs
#31: FILE: drivers/gpu/drm/i915/intel_color.c:1606:
+^I^I^I                            ^I     PAL_PREC_INDEX_VALUE(512));$

total: 1 errors, 1 warnings, 0 checks, 29 lines checked
d5b1ab7565a1 drm/i915: Extract ivb_read_luts()
-:48: WARNING:LONG_LINE: line over 100 characters
#48: FILE: drivers/gpu/drm/i915/intel_color.c:1634:
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_RED_MASK, val), 10);

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

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

total: 0 errors, 3 warnings, 0 checks, 62 lines checked
26d12d725ac9 drm/i915: Extract ilk_read_luts()
-:60: WARNING:LONG_LINE: line over 100 characters
#60: FILE: drivers/gpu/drm/i915/intel_color.c:1676:
+		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:1677:
+		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:1678:
+		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
616a35ac094b FOR_TESTING_ONLY: Print rgb values of hw and sw blobs
-:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

-:16: WARNING:LONG_LINE: line over 100 characters
#16: FILE: drivers/gpu/drm/i915/intel_color.c:1381:
+	DRM_DEBUG_KMS("hw_lut->red=0x%x sw_lut->red=0x%x hw_lut->blue=0x%x sw_lut->blue=0x%x hw_lut->green=0x%x sw_lut->green=0x%x", hw_lut->red, sw_lut->red, hw_lut->blue, sw_lut->blue, hw_lut->green, sw_lut->green);

total: 0 errors, 2 warnings, 0 checks, 8 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 (rev12)

[Patchwork]

== Series Details ==

Series: drm/i915: adding state checker for gamma lut values (rev12)
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_get_config()
Okay!

Commit: drm/i915: Add func to compare hw/sw gamma lut
Okay!

Commit: drm/i915: Extract i9xx_read_luts()
+drivers/gpu/drm/i915/intel_color.c:1427:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1427:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1427:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1427:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1427:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1427:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1427:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1427:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1467: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: FOR_TESTING_ONLY: Print rgb values of hw and sw blobs
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 (rev12)

[Patchwork]

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_6164 -> Patchwork_13131
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_13131 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_13131, 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_13131/

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_hugepages:
    - fi-icl-u2:          NOTRUN -> [DMESG-WARN][1] +16 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-icl-u2/igt@i915_selftest@live_hugepages.html

  
#### Suppressed ####

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

  * {igt@i915_selftest@live_mman}:
    - fi-icl-u2:          NOTRUN -> [DMESG-WARN][2] +2 similar issues
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-icl-u2/igt@i915_selftest@live_mman.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_contexts:
    - fi-hsw-peppy:       [PASS][3] -> [DMESG-FAIL][4] ([fdo#110235])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/fi-hsw-peppy/igt@i915_selftest@live_contexts.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-hsw-peppy/igt@i915_selftest@live_contexts.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - fi-icl-u3:          [PASS][5] -> [DMESG-WARN][6] ([fdo#107724]) +4 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-ilk-650:         [PASS][7] -> [DMESG-WARN][8] ([fdo#106387]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/fi-ilk-650/igt@prime_vgem@basic-fence-flip.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-ilk-650/igt@prime_vgem@basic-fence-flip.html

  
#### Possible fixes ####

  * {igt@gem_exec_basic@basic-all}:
    - fi-icl-y:           [INCOMPLETE][9] ([fdo#107713]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/fi-icl-y/igt@gem_exec_basic@basic-all.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-icl-y/igt@gem_exec_basic@basic-all.html

  * igt@gem_exec_create@basic:
    - fi-icl-u2:          [INCOMPLETE][11] ([fdo#107713]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/fi-icl-u2/igt@gem_exec_create@basic.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-icl-u2/igt@gem_exec_create@basic.html

  * igt@gem_mmap_gtt@basic-write-gtt:
    - fi-icl-u3:          [DMESG-WARN][13] ([fdo#107724]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/fi-icl-u3/igt@gem_mmap_gtt@basic-write-gtt.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-icl-u3/igt@gem_mmap_gtt@basic-write-gtt.html

  * igt@i915_module_load@reload:
    - fi-blb-e6850:       [INCOMPLETE][15] ([fdo#107718]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/fi-blb-e6850/igt@i915_module_load@reload.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-blb-e6850/igt@i915_module_load@reload.html

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

  [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235


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

  Additional (2): fi-skl-lmem fi-cml-u 
  Missing    (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_6164 -> Patchwork_13131

  CI_DRM_6164: c274f3cd1c250b11a6f3468882fa2e3501934233 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5024: f414756be2ac57e194919973da7b86644ba61241 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13131: 616a35ac094b848be99fe2566a698d2f8c06acb7 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

616a35ac094b FOR_TESTING_ONLY: Print rgb values of hw and sw blobs
26d12d725ac9 drm/i915: Extract ilk_read_luts()
d5b1ab7565a1 drm/i915: Extract ivb_read_luts()
b58a2fab4b53 drm/i915: Extract bdw_read_luts()
d9e96249b9c8 drm/i915: Extract glk_read_luts()
bc2cf6233920 drm/i915: Extract icl_read_luts()
249db384188a drm/i915: Extract i965_read_luts()
efd33dc03527 drm/i915: Extract chv_read_luts()
f77fdd96babd drm/i915: Extract i9xx_read_luts()
8d9a348d4327 drm/i915: Add func to compare hw/sw gamma lut
22a23431817f drm/i915: Enable intel_color_get_config()
1cfa0479b4b6 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_13131/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: adding state checker for gamma lut values (rev12)

[Patchwork]

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_6164 -> Patchwork_13131
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live_contexts:
    - fi-hsw-peppy:       [PASS][1] -> [DMESG-FAIL][2] ([fdo#110235])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/fi-hsw-peppy/igt@i915_selftest@live_contexts.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-hsw-peppy/igt@i915_selftest@live_contexts.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-legacy:
    - fi-icl-u3:          [PASS][3] -> [DMESG-WARN][4] ([fdo#107724]) +4 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-icl-u3/igt@kms_cursor_legacy@basic-flip-after-cursor-legacy.html

  * igt@prime_vgem@basic-fence-flip:
    - fi-ilk-650:         [PASS][5] -> [DMESG-WARN][6] ([fdo#106387]) +1 similar issue
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/fi-ilk-650/igt@prime_vgem@basic-fence-flip.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-ilk-650/igt@prime_vgem@basic-fence-flip.html

  
#### Possible fixes ####

  * igt@gem_exec_basic@basic-all:
    - fi-icl-y:           [INCOMPLETE][7] ([fdo#107713]) -> [PASS][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/fi-icl-y/igt@gem_exec_basic@basic-all.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-icl-y/igt@gem_exec_basic@basic-all.html

  * igt@gem_exec_create@basic:
    - fi-icl-u2:          [INCOMPLETE][9] ([fdo#107713]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/fi-icl-u2/igt@gem_exec_create@basic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-icl-u2/igt@gem_exec_create@basic.html

  * igt@gem_mmap_gtt@basic-write-gtt:
    - fi-icl-u3:          [DMESG-WARN][11] ([fdo#107724]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/fi-icl-u3/igt@gem_mmap_gtt@basic-write-gtt.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-icl-u3/igt@gem_mmap_gtt@basic-write-gtt.html

  * igt@i915_module_load@reload:
    - fi-blb-e6850:       [INCOMPLETE][13] ([fdo#107718]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/fi-blb-e6850/igt@i915_module_load@reload.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-blb-e6850/igt@i915_module_load@reload.html

  
#### Warnings ####

  * igt@i915_selftest@live_contexts:
    - fi-icl-dsi:         [DMESG-WARN][15] ([fdo#110801]) -> [INCOMPLETE][16] ([fdo#107713] / [fdo#108569])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/fi-icl-dsi/igt@i915_selftest@live_contexts.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/fi-icl-dsi/igt@i915_selftest@live_contexts.html

  
  [fdo#106387]: https://bugs.freedesktop.org/show_bug.cgi?id=106387
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#110235]: https://bugs.freedesktop.org/show_bug.cgi?id=110235
  [fdo#110801]: https://bugs.freedesktop.org/show_bug.cgi?id=110801


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

  Additional (2): fi-skl-lmem fi-cml-u 
  Missing    (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus 


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

  * Linux: CI_DRM_6164 -> Patchwork_13131

  CI_DRM_6164: c274f3cd1c250b11a6f3468882fa2e3501934233 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5024: f414756be2ac57e194919973da7b86644ba61241 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13131: 616a35ac094b848be99fe2566a698d2f8c06acb7 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

616a35ac094b FOR_TESTING_ONLY: Print rgb values of hw and sw blobs
26d12d725ac9 drm/i915: Extract ilk_read_luts()
d5b1ab7565a1 drm/i915: Extract ivb_read_luts()
b58a2fab4b53 drm/i915: Extract bdw_read_luts()
d9e96249b9c8 drm/i915: Extract glk_read_luts()
bc2cf6233920 drm/i915: Extract icl_read_luts()
249db384188a drm/i915: Extract i965_read_luts()
efd33dc03527 drm/i915: Extract chv_read_luts()
f77fdd96babd drm/i915: Extract i9xx_read_luts()
8d9a348d4327 drm/i915: Add func to compare hw/sw gamma lut
22a23431817f drm/i915: Enable intel_color_get_config()
1cfa0479b4b6 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_13131/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v7][PATCH 03/12] drm/i915: Add func to compare hw/sw gamma lut

[Ville Syrjälä]

On Wed, May 29, 2019 at 03:20:53PM +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]
> v6: -Added func intel_color_get_bit_precision() to get bit precision for
>      gamma and degamma lut readout depending upon platform and
>      corresponding to load_luts() [Ankit]
>     -Added debug log for color para in intel_dump_pipe_config [Jani]
>     -Made patch11 as patch3 [Jani]
> v7: -Renamed func intel_color_get_bit_precision() to
>      intel_color_get_gamma_bit_precision()
>     -Added separate function/platform for gamma bit precision [Ville]
>     -Corrected checkpatch warnings
> 
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c   | 166 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_color.h   |   7 ++
>  drivers/gpu/drm/i915/intel_display.c |  24 +++++
>  3 files changed, 197 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 50b98ee..b20a2c6 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -1251,6 +1251,172 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
>  	return 0;
>  }
>  
> +static int i9xx_gamma_precision(struct intel_crtc_state *crtc_state)
> +{
> +	if (!crtc_state->gamma_enable)
> +		return 0;
> +
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
> +		return 8;
> +	case GAMMA_MODE_MODE_10BIT:
> +		return 16;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		return 0;
> +	}
> +}
> +
> +static int chv_gamma_precision(struct intel_crtc_state *crtc_state)
> +{
> +	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
> +		return 10;
> +	else
> +		return i9xx_gamma_precision(crtc_state);
> +}
> +
> +static int ilk_gamma_precision(struct intel_crtc_state *crtc_state)
> +{
> +	if (!crtc_state->gamma_enable)
> +		return 0;
> +
> +	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
> +		return 0;
> +
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
> +		return 8;
> +	case GAMMA_MODE_MODE_10BIT:
> +		return 10;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		return 0;
> +	}
> +}
> +
> +static int ivb_gamma_precision(struct intel_crtc_state *crtc_state)
> +{
> +	if (!crtc_state->gamma_enable)
> +		return 0;
> +
> +	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
> +		return 0;
> +
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
> +		return 8;
> +	case GAMMA_MODE_MODE_SPLIT:
> +	case GAMMA_MODE_MODE_10BIT:
> +		return 10;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		return 0;
> +	}
> +}
> +
> +static int glk_gamma_precision(struct intel_crtc_state *crtc_state)
> +{
> +	if (!crtc_state->gamma_enable)
> +		return 0;
> +
> +	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
> +		return 0;

glk doens't have that bit.

> +
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
> +		return 8;
> +	case GAMMA_MODE_MODE_10BIT:
> +		return 10;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		return 0;
> +	}
> +}
> +
> +static int icl_gamma_precision(struct intel_crtc_state *crtc_state)
> +{
> +	if ((crtc_state->gamma_mode & PRE_CSC_GAMMA_ENABLE) == 0)
> +		return 0;

POST_CSC_GAMMA

> +
> +	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
> +	case GAMMA_MODE_MODE_8BIT:
> +		return 8;
> +	case GAMMA_MODE_MODE_10BIT:
> +		return 10;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		return 0;
> +	}
> +}
> +
> +int intel_color_get_gamma_bit_precision(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);
> +
> +	if (HAS_GMCH(dev_priv)) {
> +		if (IS_CHERRYVIEW(dev_priv))
> +			return chv_gamma_precision(crtc_state);
> +		else
> +			return i9xx_gamma_precision(crtc_state);
> +	} else {
> +		if (INTEL_GEN(dev_priv) >= 11)
> +			return icl_gamma_precision(crtc_state);
> +		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> +			return glk_gamma_precision(crtc_state);
> +		else if (INTEL_GEN(dev_priv) >= 7)
> +			return ivb_gamma_precision(crtc_state);
> +		else
> +			return ilk_gamma_precision(crtc_state);
> +	}
> +
> +	return 0;
> +}
> +
> +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 bit_precision)
> +{
> +	struct drm_color_lut *sw_lut, *hw_lut;
> +	int sw_lut_size, hw_lut_size, i;
> +	u32 err;
> +
> +	if (!blob1 && !blob2)
> +		return true;
> +
> +	if (!blob1)
> +		return true;
> +
> +	if (!blob2)
> +		return false;

What is this logic trying to do?

Something like?

if (!blob1 != !blob2)
	return false;
if (!blob1)
	return true;


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

This is going to trigger with the split gamma mode. We'll need to be
more clever when comparing the two.

> +
> +	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 057e8ac..02ea1bc 100644
> --- a/drivers/gpu/drm/i915/intel_color.h
> +++ b/drivers/gpu/drm/i915/intel_color.h
> @@ -6,13 +6,20 @@
>  #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_get_config(struct intel_crtc_state *crtc_state);
> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
> +			   struct drm_property_blob *blob2,
> +			   u32 bit_precision);
> +int intel_color_get_gamma_bit_precision(struct intel_crtc_state *crtc_state);
>  
>  #endif /* __INTEL_COLOR_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3e01028..b8ff3f4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11570,6 +11570,15 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  				      drm_rect_width(&state->base.dst),
>  				      drm_rect_height(&state->base.dst));
>  	}
> +
> +	if (IS_CHERRYVIEW(dev_priv))
> +		DRM_DEBUG_KMS("cgm_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
> +			       pipe_config->cgm_mode, pipe_config->gamma_mode,
> +			       pipe_config->gamma_enable, pipe_config->csc_enable);
> +	else
> +		DRM_DEBUG_KMS("csc_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
> +			       pipe_config->csc_mode, pipe_config->gamma_mode,
> +			       pipe_config->gamma_enable, pipe_config->csc_enable);
>  }
>  
>  static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> @@ -11947,6 +11956,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>  			  bool adjust)
>  {
>  	bool ret = true;
> +	u32 bp_gamma = 0;
>  	bool fixup_inherited = adjust &&
>  		(current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
>  		!(pipe_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
> @@ -12098,6 +12108,15 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>  	} \
>  } while (0)
>  
> +#define PIPE_CONF_CHECK_COLOR_LUT(name, bit_precision) do { \
> +	if (!intel_color_lut_equal(current_config->name, \
> +				   pipe_config->name, bit_precision)) { \
> +		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))
>  
> @@ -12193,6 +12212,10 @@ 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);
> +
> +		bp_gamma = intel_color_get_gamma_bit_precision(pipe_config);
> +		if (bp_gamma)
> +			PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, bp_gamma);

Sometimes the sw gamma gets programmed in the hw degamma, so we do need
degamma readout, and we'll need to somehow figure out which hw thing
is which sw thing.

I'm thinking the following sort of logic should work since so
far it should be just the the sw.gamma that can end up in 
hw.degamma:

if (sw.gamma && !hw.gamma) {
	CHECK(sw.gamma, hw.degamma);
	CHECK(sw.degamma, hw.gamma);
} else {
	CHECK(sw.gamma, hw.gamma);
	CHECK(sw.degamma, hw.degamma);
}

>  	}
>  
>  	PIPE_CONF_CHECK_BOOL(double_wide);
> @@ -12255,6 +12278,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

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

Re: [v7][PATCH 06/12] drm/i915: Extract i965_read_luts()

[Ville Syrjälä]

On Wed, May 29, 2019 at 03:20:56PM +0530, Swati Sharma wrote:
> 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 b58c66d..7988fa5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3584,6 +3584,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 6ed851b..3ec84af 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -1505,6 +1505,44 @@ static void chv_read_luts(struct intel_crtc_state *crtc_state)
>  		crtc_state->base.gamma_lut = chv_read_cgm_gamma_lut(crtc_state);
>  }
>  
> +static struct drm_property_blob *
> +i965_read_gamma_lut_10p6(struct intel_crtc_state *crtc_state)

i965_read_lut_10p6() would match the name of the counterpart better.

I think crtc_state can be const in all of these functions. Or you could
pass just the crtc itself rather than its 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);
> @@ -1522,6 +1560,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

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

Re: [v7][PATCH 11/12] drm/i915: Extract ilk_read_luts()

[Ville Syrjälä]

On Wed, May 29, 2019 at 03:21:01PM +0530, Swati Sharma wrote:
> 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 249296b..d5ff323 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 6e6e54b..7568b13 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -1650,6 +1650,43 @@ static void ivb_read_luts(struct intel_crtc_state *crtc_state)
>  		crtc_state->base.gamma_lut = ivb_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
>  }
>  
> +static struct drm_property_blob *
> +ilk_read_gamma_lut(struct intel_crtc_state *crtc_state)

ilk_read_lut_10()

> +{
> +	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);
> @@ -1703,9 +1740,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

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

✓ Fi.CI.IGT: success for drm/i915: adding state checker for gamma lut values (rev12)

[Patchwork]

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_6164_full -> Patchwork_13131_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [PASS][1] -> [DMESG-WARN][2] ([fdo#108566]) +4 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-apl1/igt@i915_suspend@sysfs-reader.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-apl7/igt@i915_suspend@sysfs-reader.html

  * igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen:
    - shard-kbl:          [PASS][3] -> [DMESG-WARN][4] ([fdo#103558] / [fdo#105602]) +30 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-kbl1/igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-kbl6/igt@kms_cursor_crc@pipe-a-cursor-64x64-onscreen.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-skl:          [PASS][5] -> [FAIL][6] ([fdo#102670])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-skl5/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-skl10/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-skl:          [PASS][7] -> [INCOMPLETE][8] ([fdo#109507])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-skl3/igt@kms_flip@flip-vs-suspend-interruptible.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-skl9/igt@kms_flip@flip-vs-suspend-interruptible.html

  * igt@kms_flip@plain-flip-ts-check:
    - shard-kbl:          [PASS][9] -> [FAIL][10] ([fdo#100368])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-kbl1/igt@kms_flip@plain-flip-ts-check.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-kbl6/igt@kms_flip@plain-flip-ts-check.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-move:
    - shard-hsw:          [PASS][11] -> [SKIP][12] ([fdo#109271]) +11 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-hsw2/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-move.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-hsw1/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-move.html

  * igt@kms_frontbuffer_tracking@fbc-suspend:
    - shard-kbl:          [PASS][13] -> [DMESG-WARN][14] ([fdo#108566]) +2 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-kbl3/igt@kms_frontbuffer_tracking@fbc-suspend.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-kbl6/igt@kms_frontbuffer_tracking@fbc-suspend.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite:
    - shard-iclb:         [PASS][15] -> [FAIL][16] ([fdo#103167]) +4 similar issues
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-iclb5/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [PASS][17] -> [FAIL][18] ([fdo#103166])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-iclb6/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr@psr2_primary_mmap_gtt:
    - shard-iclb:         [PASS][19] -> [SKIP][20] ([fdo#109441])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-iclb2/igt@kms_psr@psr2_primary_mmap_gtt.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-iclb5/igt@kms_psr@psr2_primary_mmap_gtt.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-kbl:          [PASS][21] -> [DMESG-WARN][22] ([fdo#103558] / [fdo#105602] / [fdo#108566])
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-kbl3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-kbl6/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  
#### Possible fixes ####

  * igt@gem_ctx_switch@basic-all-heavy:
    - shard-glk:          [INCOMPLETE][23] ([fdo#103359] / [k.org#198133]) -> [PASS][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-glk1/igt@gem_ctx_switch@basic-all-heavy.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-glk3/igt@gem_ctx_switch@basic-all-heavy.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-glk:          [DMESG-WARN][25] ([fdo#108686]) -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-glk1/igt@gem_tiled_swapping@non-threaded.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-glk3/igt@gem_tiled_swapping@non-threaded.html

  * igt@gem_workarounds@suspend-resume-fd:
    - shard-iclb:         [FAIL][27] ([fdo#110802]) -> [PASS][28] +2 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-iclb2/igt@gem_workarounds@suspend-resume-fd.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-iclb5/igt@gem_workarounds@suspend-resume-fd.html

  * igt@kms_flip@2x-flip-vs-dpms-interruptible:
    - shard-hsw:          [SKIP][29] ([fdo#109271]) -> [PASS][30] +11 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-hsw1/igt@kms_flip@2x-flip-vs-dpms-interruptible.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-hsw5/igt@kms_flip@2x-flip-vs-dpms-interruptible.html

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          [FAIL][31] ([fdo#105363]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-glk7/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-glk7/igt@kms_flip@2x-flip-vs-expired-vblank-interruptible.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [FAIL][33] ([fdo#105363]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-skl9/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-skl5/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         [FAIL][35] ([fdo#103167]) -> [PASS][36] +2 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-iclb7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-iclb4/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite.html

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-gtt:
    - shard-skl:          [FAIL][37] ([fdo#103167]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-skl9/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-gtt.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-skl5/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-gtt.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min:
    - shard-skl:          [FAIL][39] ([fdo#108145]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-skl9/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-skl5/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-min.html

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         [FAIL][41] ([fdo#100047]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-iclb2/igt@kms_sysfs_edid_timing.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-iclb5/igt@kms_sysfs_edid_timing.html

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-apl:          [DMESG-WARN][43] ([fdo#108566]) -> [PASS][44] +3 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-apl5/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-apl2/igt@kms_vblank@pipe-a-ts-continuation-suspend.html

  
#### Warnings ####

  * igt@gem_mmap_gtt@forked-big-copy-xy:
    - shard-iclb:         [TIMEOUT][45] ([fdo#109673]) -> [INCOMPLETE][46] ([fdo#107713] / [fdo#109100])
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-iclb4/igt@gem_mmap_gtt@forked-big-copy-xy.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-iclb2/igt@gem_mmap_gtt@forked-big-copy-xy.html

  * igt@gem_tiled_swapping@non-threaded:
    - shard-hsw:          [INCOMPLETE][47] ([fdo#103540]) -> [FAIL][48] ([fdo#108686])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-hsw6/igt@gem_tiled_swapping@non-threaded.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-hsw1/igt@gem_tiled_swapping@non-threaded.html

  * igt@kms_busy@extended-modeset-hang-newfb-render-e:
    - shard-kbl:          [SKIP][49] ([fdo#109271] / [fdo#109278]) -> [SKIP][50] ([fdo#105602] / [fdo#109271] / [fdo#109278]) +3 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-kbl3/igt@kms_busy@extended-modeset-hang-newfb-render-e.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-kbl6/igt@kms_busy@extended-modeset-hang-newfb-render-e.html

  * igt@kms_panel_fitting@legacy:
    - shard-kbl:          [SKIP][51] ([fdo#109271]) -> [SKIP][52] ([fdo#105602] / [fdo#109271]) +24 similar issues
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6164/shard-kbl1/igt@kms_panel_fitting@legacy.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_13131/shard-kbl6/igt@kms_panel_fitting@legacy.html

  
  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#100368]: https://bugs.freedesktop.org/show_bug.cgi?id=100368
  [fdo#102670]: https://bugs.freedesktop.org/show_bug.cgi?id=102670
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109507]: https://bugs.freedesktop.org/show_bug.cgi?id=109507
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#110802]: https://bugs.freedesktop.org/show_bug.cgi?id=110802
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


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

  No changes in participating hosts


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

  * Linux: CI_DRM_6164 -> Patchwork_13131

  CI_DRM_6164: c274f3cd1c250b11a6f3468882fa2e3501934233 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5024: f414756be2ac57e194919973da7b86644ba61241 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_13131: 616a35ac094b848be99fe2566a698d2f8c06acb7 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [v7][PATCH 03/12] drm/i915: Add func to compare hw/sw gamma lut

[Jani Nikula]

On Wed, 29 May 2019, Swati Sharma <swati2.sharma@intel.com> 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]
> v6: -Added func intel_color_get_bit_precision() to get bit precision for
>      gamma and degamma lut readout depending upon platform and
>      corresponding to load_luts() [Ankit]
>     -Added debug log for color para in intel_dump_pipe_config [Jani]
>     -Made patch11 as patch3 [Jani]
> v7: -Renamed func intel_color_get_bit_precision() to
>      intel_color_get_gamma_bit_precision()
>     -Added separate function/platform for gamma bit precision [Ville]
>     -Corrected checkpatch warnings

The patch revisions are great, but you do need to add an actual commit
message first as well.

In general, please don't reference "this patch" because on the one hand
it's kind of obvious and on the other hand it ceases to be a patch when
it gets applied and becomes a commit.

Please also use the imperative style in the commit message like you have
in the title/subject line.

So instead of "In this patch, foo is added." use "Add foo."

BR,
Jani.


>
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c   | 166 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_color.h   |   7 ++
>  drivers/gpu/drm/i915/intel_display.c |  24 +++++
>  3 files changed, 197 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 50b98ee..b20a2c6 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -1251,6 +1251,172 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
>  	return 0;
>  }
>  
> +static int i9xx_gamma_precision(struct intel_crtc_state *crtc_state)
> +{
> +	if (!crtc_state->gamma_enable)
> +		return 0;
> +
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
> +		return 8;
> +	case GAMMA_MODE_MODE_10BIT:
> +		return 16;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		return 0;
> +	}
> +}
> +
> +static int chv_gamma_precision(struct intel_crtc_state *crtc_state)
> +{
> +	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
> +		return 10;
> +	else
> +		return i9xx_gamma_precision(crtc_state);
> +}
> +
> +static int ilk_gamma_precision(struct intel_crtc_state *crtc_state)
> +{
> +	if (!crtc_state->gamma_enable)
> +		return 0;
> +
> +	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
> +		return 0;
> +
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
> +		return 8;
> +	case GAMMA_MODE_MODE_10BIT:
> +		return 10;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		return 0;
> +	}
> +}
> +
> +static int ivb_gamma_precision(struct intel_crtc_state *crtc_state)
> +{
> +	if (!crtc_state->gamma_enable)
> +		return 0;
> +
> +	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
> +		return 0;
> +
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
> +		return 8;
> +	case GAMMA_MODE_MODE_SPLIT:
> +	case GAMMA_MODE_MODE_10BIT:
> +		return 10;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		return 0;
> +	}
> +}
> +
> +static int glk_gamma_precision(struct intel_crtc_state *crtc_state)
> +{
> +	if (!crtc_state->gamma_enable)
> +		return 0;
> +
> +	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
> +		return 0;
> +
> +	switch (crtc_state->gamma_mode) {
> +	case GAMMA_MODE_MODE_8BIT:
> +		return 8;
> +	case GAMMA_MODE_MODE_10BIT:
> +		return 10;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		return 0;
> +	}
> +}
> +
> +static int icl_gamma_precision(struct intel_crtc_state *crtc_state)
> +{
> +	if ((crtc_state->gamma_mode & PRE_CSC_GAMMA_ENABLE) == 0)
> +		return 0;
> +
> +	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
> +	case GAMMA_MODE_MODE_8BIT:
> +		return 8;
> +	case GAMMA_MODE_MODE_10BIT:
> +		return 10;
> +	default:
> +		MISSING_CASE(crtc_state->gamma_mode);
> +		return 0;
> +	}
> +}
> +
> +int intel_color_get_gamma_bit_precision(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);
> +
> +	if (HAS_GMCH(dev_priv)) {
> +		if (IS_CHERRYVIEW(dev_priv))
> +			return chv_gamma_precision(crtc_state);
> +		else
> +			return i9xx_gamma_precision(crtc_state);
> +	} else {
> +		if (INTEL_GEN(dev_priv) >= 11)
> +			return icl_gamma_precision(crtc_state);
> +		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> +			return glk_gamma_precision(crtc_state);
> +		else if (INTEL_GEN(dev_priv) >= 7)
> +			return ivb_gamma_precision(crtc_state);
> +		else
> +			return ilk_gamma_precision(crtc_state);
> +	}
> +
> +	return 0;
> +}
> +
> +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 bit_precision)
> +{
> +	struct drm_color_lut *sw_lut, *hw_lut;
> +	int sw_lut_size, hw_lut_size, i;
> +	u32 err;
> +
> +	if (!blob1 && !blob2)
> +		return true;
> +
> +	if (!blob1)
> +		return true;
> +
> +	if (!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;
> +
> +	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 057e8ac..02ea1bc 100644
> --- a/drivers/gpu/drm/i915/intel_color.h
> +++ b/drivers/gpu/drm/i915/intel_color.h
> @@ -6,13 +6,20 @@
>  #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_get_config(struct intel_crtc_state *crtc_state);
> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
> +			   struct drm_property_blob *blob2,
> +			   u32 bit_precision);
> +int intel_color_get_gamma_bit_precision(struct intel_crtc_state *crtc_state);
>  
>  #endif /* __INTEL_COLOR_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3e01028..b8ff3f4 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11570,6 +11570,15 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  				      drm_rect_width(&state->base.dst),
>  				      drm_rect_height(&state->base.dst));
>  	}
> +
> +	if (IS_CHERRYVIEW(dev_priv))
> +		DRM_DEBUG_KMS("cgm_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
> +			       pipe_config->cgm_mode, pipe_config->gamma_mode,
> +			       pipe_config->gamma_enable, pipe_config->csc_enable);
> +	else
> +		DRM_DEBUG_KMS("csc_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
> +			       pipe_config->csc_mode, pipe_config->gamma_mode,
> +			       pipe_config->gamma_enable, pipe_config->csc_enable);
>  }
>  
>  static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> @@ -11947,6 +11956,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>  			  bool adjust)
>  {
>  	bool ret = true;
> +	u32 bp_gamma = 0;
>  	bool fixup_inherited = adjust &&
>  		(current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
>  		!(pipe_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
> @@ -12098,6 +12108,15 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>  	} \
>  } while (0)
>  
> +#define PIPE_CONF_CHECK_COLOR_LUT(name, bit_precision) do { \
> +	if (!intel_color_lut_equal(current_config->name, \
> +				   pipe_config->name, bit_precision)) { \
> +		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))
>  
> @@ -12193,6 +12212,10 @@ 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);
> +
> +		bp_gamma = intel_color_get_gamma_bit_precision(pipe_config);
> +		if (bp_gamma)
> +			PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, bp_gamma);
>  	}
>  
>  	PIPE_CONF_CHECK_BOOL(double_wide);
> @@ -12255,6 +12278,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;
>  }

-- 
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: [v7][PATCH 02/12] drm/i915: Enable intel_color_get_config()

[Jani Nikula]

On Wed, 29 May 2019, Swati Sharma <swati2.sharma@intel.com> wrote:
> In this patch, intel_color_get_config() 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]
> v6: -Renamed intel_color_read_luts() to intel_color_get_config()
>      [Jani and Ville]
>
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>

I pushed patches 1 and 2, thanks for the patches.

BR,
Jani.


> ---
>  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 05177f3..3e01028 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8351,6 +8351,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_get_config(pipe_config);
>  
>  	if (INTEL_GEN(dev_priv) < 4)
>  		pipe_config->double_wide = tmp & PIPECONF_DOUBLE_WIDE;
> @@ -9426,6 +9427,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_get_config(pipe_config);
>  
>  	if (I915_READ(PCH_TRANSCONF(crtc->pipe)) & TRANS_ENABLE) {
>  		struct intel_shared_dpll *pll;
> @@ -9874,6 +9876,8 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  		i9xx_get_pipe_color_config(pipe_config);
>  	}
>  
> +	intel_color_get_config(pipe_config);
> +
>  	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
>  	WARN_ON(power_domain_mask & BIT_ULL(power_domain));

-- 
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: [v7][PATCH 03/12] drm/i915: Add func to compare hw/sw gamma lut

[Sharma, Swati2]

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

On 31-May-19 8:58 PM, Ville Syrjälä wrote:

> On Wed, May 29, 2019 at 03:20:53PM +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]
>> v6: -Added func intel_color_get_bit_precision() to get bit precision for
>>       gamma and degamma lut readout depending upon platform and
>>       corresponding to load_luts() [Ankit]
>>      -Added debug log for color para in intel_dump_pipe_config [Jani]
>>      -Made patch11 as patch3 [Jani]
>> v7: -Renamed func intel_color_get_bit_precision() to
>>       intel_color_get_gamma_bit_precision()
>>      -Added separate function/platform for gamma bit precision [Ville]
>>      -Corrected checkpatch warnings
>>
>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_color.c   | 166 +++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_color.h   |   7 ++
>>   drivers/gpu/drm/i915/intel_display.c |  24 +++++
>>   3 files changed, 197 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>> index 50b98ee..b20a2c6 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -1251,6 +1251,172 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
>>   	return 0;
>>   }
>>   
>> +static int i9xx_gamma_precision(struct intel_crtc_state *crtc_state)
>> +{
>> +	if (!crtc_state->gamma_enable)
>> +		return 0;
>> +
>> +	switch (crtc_state->gamma_mode) {
>> +	case GAMMA_MODE_MODE_8BIT:
>> +		return 8;
>> +	case GAMMA_MODE_MODE_10BIT:
>> +		return 16;
>> +	default:
>> +		MISSING_CASE(crtc_state->gamma_mode);
>> +		return 0;
>> +	}
>> +}
>> +
>> +static int chv_gamma_precision(struct intel_crtc_state *crtc_state)
>> +{
>> +	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
>> +		return 10;
>> +	else
>> +		return i9xx_gamma_precision(crtc_state);
>> +}
>> +
>> +static int ilk_gamma_precision(struct intel_crtc_state *crtc_state)
>> +{
>> +	if (!crtc_state->gamma_enable)
>> +		return 0;
>> +
>> +	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
>> +		return 0;
>> +
>> +	switch (crtc_state->gamma_mode) {
>> +	case GAMMA_MODE_MODE_8BIT:
>> +		return 8;
>> +	case GAMMA_MODE_MODE_10BIT:
>> +		return 10;
>> +	default:
>> +		MISSING_CASE(crtc_state->gamma_mode);
>> +		return 0;
>> +	}
>> +}
>> +
>> +static int ivb_gamma_precision(struct intel_crtc_state *crtc_state)
>> +{
>> +	if (!crtc_state->gamma_enable)
>> +		return 0;
>> +
>> +	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
>> +		return 0;
>> +
>> +	switch (crtc_state->gamma_mode) {
>> +	case GAMMA_MODE_MODE_8BIT:
>> +		return 8;
>> +	case GAMMA_MODE_MODE_SPLIT:
>> +	case GAMMA_MODE_MODE_10BIT:
>> +		return 10;
>> +	default:
>> +		MISSING_CASE(crtc_state->gamma_mode);
>> +		return 0;
>> +	}
>> +}
>> +
>> +static int glk_gamma_precision(struct intel_crtc_state *crtc_state)
>> +{
>> +	if (!crtc_state->gamma_enable)
>> +		return 0;
>> +
>> +	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
>> +		return 0;
> glk doens't have that bit.
>
>> +
>> +	switch (crtc_state->gamma_mode) {
>> +	case GAMMA_MODE_MODE_8BIT:
>> +		return 8;
>> +	case GAMMA_MODE_MODE_10BIT:
>> +		return 10;
>> +	default:
>> +		MISSING_CASE(crtc_state->gamma_mode);
>> +		return 0;
>> +	}
>> +}
>> +
>> +static int icl_gamma_precision(struct intel_crtc_state *crtc_state)
>> +{
>> +	if ((crtc_state->gamma_mode & PRE_CSC_GAMMA_ENABLE) == 0)
>> +		return 0;
> POST_CSC_GAMMA
>
>> +
>> +	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
>> +	case GAMMA_MODE_MODE_8BIT:
>> +		return 8;
>> +	case GAMMA_MODE_MODE_10BIT:
>> +		return 10;
>> +	default:
>> +		MISSING_CASE(crtc_state->gamma_mode);
>> +		return 0;
>> +	}
>> +}
>> +
>> +int intel_color_get_gamma_bit_precision(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);
>> +
>> +	if (HAS_GMCH(dev_priv)) {
>> +		if (IS_CHERRYVIEW(dev_priv))
>> +			return chv_gamma_precision(crtc_state);
>> +		else
>> +			return i9xx_gamma_precision(crtc_state);
>> +	} else {
>> +		if (INTEL_GEN(dev_priv) >= 11)
>> +			return icl_gamma_precision(crtc_state);
>> +		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>> +			return glk_gamma_precision(crtc_state);
>> +		else if (INTEL_GEN(dev_priv) >= 7)
>> +			return ivb_gamma_precision(crtc_state);
>> +		else
>> +			return ilk_gamma_precision(crtc_state);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +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 bit_precision)
>> +{
>> +	struct drm_color_lut *sw_lut, *hw_lut;
>> +	int sw_lut_size, hw_lut_size, i;
>> +	u32 err;
>> +
>> +	if (!blob1 && !blob2)
>> +		return true;
>> +
>> +	if (!blob1)
>> +		return true;
>> +
>> +	if (!blob2)
>> +		return false;
> What is this logic trying to do?
>
> Something like?
>
> if (!blob1 != !blob2)
> 	return false;
> if (!blob1)
> 	return true;
>
>
>> +
>> +	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;
> This is going to trigger with the split gamma mode. We'll need to be
> more clever when comparing the two.

So, we need check for gamma_mode here as-well?

>
>> +
>> +	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 057e8ac..02ea1bc 100644
>> --- a/drivers/gpu/drm/i915/intel_color.h
>> +++ b/drivers/gpu/drm/i915/intel_color.h
>> @@ -6,13 +6,20 @@
>>   #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_get_config(struct intel_crtc_state *crtc_state);
>> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
>> +			   struct drm_property_blob *blob2,
>> +			   u32 bit_precision);
>> +int intel_color_get_gamma_bit_precision(struct intel_crtc_state *crtc_state);
>>   
>>   #endif /* __INTEL_COLOR_H__ */
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 3e01028..b8ff3f4 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -11570,6 +11570,15 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>>   				      drm_rect_width(&state->base.dst),
>>   				      drm_rect_height(&state->base.dst));
>>   	}
>> +
>> +	if (IS_CHERRYVIEW(dev_priv))
>> +		DRM_DEBUG_KMS("cgm_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
>> +			       pipe_config->cgm_mode, pipe_config->gamma_mode,
>> +			       pipe_config->gamma_enable, pipe_config->csc_enable);
>> +	else
>> +		DRM_DEBUG_KMS("csc_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
>> +			       pipe_config->csc_mode, pipe_config->gamma_mode,
>> +			       pipe_config->gamma_enable, pipe_config->csc_enable);
>>   }
>>   
>>   static bool check_digital_port_conflicts(struct drm_atomic_state *state)
>> @@ -11947,6 +11956,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>>   			  bool adjust)
>>   {
>>   	bool ret = true;
>> +	u32 bp_gamma = 0;
>>   	bool fixup_inherited = adjust &&
>>   		(current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
>>   		!(pipe_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
>> @@ -12098,6 +12108,15 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>>   	} \
>>   } while (0)
>>   
>> +#define PIPE_CONF_CHECK_COLOR_LUT(name, bit_precision) do { \
>> +	if (!intel_color_lut_equal(current_config->name, \
>> +				   pipe_config->name, bit_precision)) { \
>> +		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))
>>   
>> @@ -12193,6 +12212,10 @@ 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);
>> +
>> +		bp_gamma = intel_color_get_gamma_bit_precision(pipe_config);
>> +		if (bp_gamma)
>> +			PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, bp_gamma);
> Sometimes the sw gamma gets programmed in the hw degamma, so we do need
> degamma readout, and we'll need to somehow figure out which hw thing
> is which sw thing.

Can you please little bit elaborate on this? What's the usecase?

>
> I'm thinking the following sort of logic should work since so
> far it should be just the the sw.gamma that can end up in
> hw.degamma:
>
> if (sw.gamma && !hw.gamma) {
> 	CHECK(sw.gamma, hw.degamma);
> 	CHECK(sw.degamma, hw.gamma);
> } else {
> 	CHECK(sw.gamma, hw.gamma);
> 	CHECK(sw.degamma, hw.degamma);
> }
>
>>   	}
>>   
>>   	PIPE_CONF_CHECK_BOOL(double_wide);
>> @@ -12255,6 +12278,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


-- 
~Swati Sharma


[-- Attachment #1.2: Type: text/html, Size: 11951 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: [v7][PATCH 03/12] drm/i915: Add func to compare hw/sw gamma lut

[Ville Syrjälä]

On Mon, Jun 10, 2019 at 04:24:54PM +0530, Sharma, Swati2 wrote:
> On 31-May-19 8:58 PM, Ville Syrjälä wrote:
> 
> > On Wed, May 29, 2019 at 03:20:53PM +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]
> >> v6: -Added func intel_color_get_bit_precision() to get bit precision for
> >>       gamma and degamma lut readout depending upon platform and
> >>       corresponding to load_luts() [Ankit]
> >>      -Added debug log for color para in intel_dump_pipe_config [Jani]
> >>      -Made patch11 as patch3 [Jani]
> >> v7: -Renamed func intel_color_get_bit_precision() to
> >>       intel_color_get_gamma_bit_precision()
> >>      -Added separate function/platform for gamma bit precision [Ville]
> >>      -Corrected checkpatch warnings
> >>
> >> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_color.c   | 166 +++++++++++++++++++++++++++++++++++
> >>   drivers/gpu/drm/i915/intel_color.h   |   7 ++
> >>   drivers/gpu/drm/i915/intel_display.c |  24 +++++
> >>   3 files changed, 197 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> >> index 50b98ee..b20a2c6 100644
> >> --- a/drivers/gpu/drm/i915/intel_color.c
> >> +++ b/drivers/gpu/drm/i915/intel_color.c
> >> @@ -1251,6 +1251,172 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
> >>   	return 0;
> >>   }
> >>   
> >> +static int i9xx_gamma_precision(struct intel_crtc_state *crtc_state)
> >> +{
> >> +	if (!crtc_state->gamma_enable)
> >> +		return 0;
> >> +
> >> +	switch (crtc_state->gamma_mode) {
> >> +	case GAMMA_MODE_MODE_8BIT:
> >> +		return 8;
> >> +	case GAMMA_MODE_MODE_10BIT:
> >> +		return 16;
> >> +	default:
> >> +		MISSING_CASE(crtc_state->gamma_mode);
> >> +		return 0;
> >> +	}
> >> +}
> >> +
> >> +static int chv_gamma_precision(struct intel_crtc_state *crtc_state)
> >> +{
> >> +	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
> >> +		return 10;
> >> +	else
> >> +		return i9xx_gamma_precision(crtc_state);
> >> +}
> >> +
> >> +static int ilk_gamma_precision(struct intel_crtc_state *crtc_state)
> >> +{
> >> +	if (!crtc_state->gamma_enable)
> >> +		return 0;
> >> +
> >> +	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
> >> +		return 0;
> >> +
> >> +	switch (crtc_state->gamma_mode) {
> >> +	case GAMMA_MODE_MODE_8BIT:
> >> +		return 8;
> >> +	case GAMMA_MODE_MODE_10BIT:
> >> +		return 10;
> >> +	default:
> >> +		MISSING_CASE(crtc_state->gamma_mode);
> >> +		return 0;
> >> +	}
> >> +}
> >> +
> >> +static int ivb_gamma_precision(struct intel_crtc_state *crtc_state)
> >> +{
> >> +	if (!crtc_state->gamma_enable)
> >> +		return 0;
> >> +
> >> +	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
> >> +		return 0;
> >> +
> >> +	switch (crtc_state->gamma_mode) {
> >> +	case GAMMA_MODE_MODE_8BIT:
> >> +		return 8;
> >> +	case GAMMA_MODE_MODE_SPLIT:
> >> +	case GAMMA_MODE_MODE_10BIT:
> >> +		return 10;
> >> +	default:
> >> +		MISSING_CASE(crtc_state->gamma_mode);
> >> +		return 0;
> >> +	}
> >> +}
> >> +
> >> +static int glk_gamma_precision(struct intel_crtc_state *crtc_state)
> >> +{
> >> +	if (!crtc_state->gamma_enable)
> >> +		return 0;
> >> +
> >> +	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
> >> +		return 0;
> > glk doens't have that bit.
> >
> >> +
> >> +	switch (crtc_state->gamma_mode) {
> >> +	case GAMMA_MODE_MODE_8BIT:
> >> +		return 8;
> >> +	case GAMMA_MODE_MODE_10BIT:
> >> +		return 10;
> >> +	default:
> >> +		MISSING_CASE(crtc_state->gamma_mode);
> >> +		return 0;
> >> +	}
> >> +}
> >> +
> >> +static int icl_gamma_precision(struct intel_crtc_state *crtc_state)
> >> +{
> >> +	if ((crtc_state->gamma_mode & PRE_CSC_GAMMA_ENABLE) == 0)
> >> +		return 0;
> > POST_CSC_GAMMA
> >
> >> +
> >> +	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
> >> +	case GAMMA_MODE_MODE_8BIT:
> >> +		return 8;
> >> +	case GAMMA_MODE_MODE_10BIT:
> >> +		return 10;
> >> +	default:
> >> +		MISSING_CASE(crtc_state->gamma_mode);
> >> +		return 0;
> >> +	}
> >> +}
> >> +
> >> +int intel_color_get_gamma_bit_precision(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);
> >> +
> >> +	if (HAS_GMCH(dev_priv)) {
> >> +		if (IS_CHERRYVIEW(dev_priv))
> >> +			return chv_gamma_precision(crtc_state);
> >> +		else
> >> +			return i9xx_gamma_precision(crtc_state);
> >> +	} else {
> >> +		if (INTEL_GEN(dev_priv) >= 11)
> >> +			return icl_gamma_precision(crtc_state);
> >> +		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
> >> +			return glk_gamma_precision(crtc_state);
> >> +		else if (INTEL_GEN(dev_priv) >= 7)
> >> +			return ivb_gamma_precision(crtc_state);
> >> +		else
> >> +			return ilk_gamma_precision(crtc_state);
> >> +	}
> >> +
> >> +	return 0;
> >> +}
> >> +
> >> +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 bit_precision)
> >> +{
> >> +	struct drm_color_lut *sw_lut, *hw_lut;
> >> +	int sw_lut_size, hw_lut_size, i;
> >> +	u32 err;
> >> +
> >> +	if (!blob1 && !blob2)
> >> +		return true;
> >> +
> >> +	if (!blob1)
> >> +		return true;
> >> +
> >> +	if (!blob2)
> >> +		return false;
> > What is this logic trying to do?
> >
> > Something like?
> >
> > if (!blob1 != !blob2)
> > 	return false;
> > if (!blob1)
> > 	return true;
> >
> >
> >> +
> >> +	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;
> > This is going to trigger with the split gamma mode. We'll need to be
> > more clever when comparing the two.
> 
> So, we need check for gamma_mode here as-well?

Maybe. We're going to have to make the exception somewhere,
and then we need to pick the LUT entries to compare similarly
as how the relevanty load_lut() function picks them.


> 
> >
> >> +
> >> +	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 057e8ac..02ea1bc 100644
> >> --- a/drivers/gpu/drm/i915/intel_color.h
> >> +++ b/drivers/gpu/drm/i915/intel_color.h
> >> @@ -6,13 +6,20 @@
> >>   #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_get_config(struct intel_crtc_state *crtc_state);
> >> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
> >> +			   struct drm_property_blob *blob2,
> >> +			   u32 bit_precision);
> >> +int intel_color_get_gamma_bit_precision(struct intel_crtc_state *crtc_state);
> >>   
> >>   #endif /* __INTEL_COLOR_H__ */
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index 3e01028..b8ff3f4 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -11570,6 +11570,15 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
> >>   				      drm_rect_width(&state->base.dst),
> >>   				      drm_rect_height(&state->base.dst));
> >>   	}
> >> +
> >> +	if (IS_CHERRYVIEW(dev_priv))
> >> +		DRM_DEBUG_KMS("cgm_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
> >> +			       pipe_config->cgm_mode, pipe_config->gamma_mode,
> >> +			       pipe_config->gamma_enable, pipe_config->csc_enable);
> >> +	else
> >> +		DRM_DEBUG_KMS("csc_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
> >> +			       pipe_config->csc_mode, pipe_config->gamma_mode,
> >> +			       pipe_config->gamma_enable, pipe_config->csc_enable);
> >>   }
> >>   
> >>   static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> >> @@ -11947,6 +11956,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
> >>   			  bool adjust)
> >>   {
> >>   	bool ret = true;
> >> +	u32 bp_gamma = 0;
> >>   	bool fixup_inherited = adjust &&
> >>   		(current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
> >>   		!(pipe_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
> >> @@ -12098,6 +12108,15 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
> >>   	} \
> >>   } while (0)
> >>   
> >> +#define PIPE_CONF_CHECK_COLOR_LUT(name, bit_precision) do { \
> >> +	if (!intel_color_lut_equal(current_config->name, \
> >> +				   pipe_config->name, bit_precision)) { \
> >> +		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))
> >>   
> >> @@ -12193,6 +12212,10 @@ 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);
> >> +
> >> +		bp_gamma = intel_color_get_gamma_bit_precision(pipe_config);
> >> +		if (bp_gamma)
> >> +			PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, bp_gamma);
> > Sometimes the sw gamma gets programmed in the hw degamma, so we do need
> > degamma readout, and we'll need to somehow figure out which hw thing
> > is which sw thing.
> 
> Can you please little bit elaborate on this? What's the usecase?

Any time gamma+ycbcr/limited range rgb output is used.
See ivb_csc_mode().


> 
> >
> > I'm thinking the following sort of logic should work since so
> > far it should be just the the sw.gamma that can end up in
> > hw.degamma:
> >
> > if (sw.gamma && !hw.gamma) {
> > 	CHECK(sw.gamma, hw.degamma);
> > 	CHECK(sw.degamma, hw.gamma);
> > } else {
> > 	CHECK(sw.gamma, hw.gamma);
> > 	CHECK(sw.degamma, hw.degamma);
> > }
> >
> >>   	}
> >>   
> >>   	PIPE_CONF_CHECK_BOOL(double_wide);
> >> @@ -12255,6 +12278,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
> 
> 
> -- 
> ~Swati Sharma
> 

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

Re: [v7][PATCH 03/12] drm/i915: Add func to compare hw/sw gamma lut

[Sharma, Swati2]

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

On 05-Jun-19 3:37 PM, Jani Nikula wrote:
> On Wed, 29 May 2019, Swati Sharma <swati2.sharma@intel.com> 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]
>> v6: -Added func intel_color_get_bit_precision() to get bit precision for
>>       gamma and degamma lut readout depending upon platform and
>>       corresponding to load_luts() [Ankit]
>>      -Added debug log for color para in intel_dump_pipe_config [Jani]
>>      -Made patch11 as patch3 [Jani]
>> v7: -Renamed func intel_color_get_bit_precision() to
>>       intel_color_get_gamma_bit_precision()
>>      -Added separate function/platform for gamma bit precision [Ville]
>>      -Corrected checkpatch warnings
> The patch revisions are great, but you do need to add an actual commit
> message first as well.
>
> In general, please don't reference "this patch" because on the one hand
> it's kind of obvious and on the other hand it ceases to be a patch when
> it gets applied and becomes a commit.
>
> Please also use the imperative style in the commit message like you have
> in the title/subject line.
>
> So instead of "In this patch, foo is added." use "Add foo."
>
> BR,
> Jani.

Sure, thanks Jani. Will correct other patches.

>
>
>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_color.c   | 166 +++++++++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_color.h   |   7 ++
>>   drivers/gpu/drm/i915/intel_display.c |  24 +++++
>>   3 files changed, 197 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
>> index 50b98ee..b20a2c6 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -1251,6 +1251,172 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
>>   	return 0;
>>   }
>>   
>> +static int i9xx_gamma_precision(struct intel_crtc_state *crtc_state)
>> +{
>> +	if (!crtc_state->gamma_enable)
>> +		return 0;
>> +
>> +	switch (crtc_state->gamma_mode) {
>> +	case GAMMA_MODE_MODE_8BIT:
>> +		return 8;
>> +	case GAMMA_MODE_MODE_10BIT:
>> +		return 16;
>> +	default:
>> +		MISSING_CASE(crtc_state->gamma_mode);
>> +		return 0;
>> +	}
>> +}
>> +
>> +static int chv_gamma_precision(struct intel_crtc_state *crtc_state)
>> +{
>> +	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
>> +		return 10;
>> +	else
>> +		return i9xx_gamma_precision(crtc_state);
>> +}
>> +
>> +static int ilk_gamma_precision(struct intel_crtc_state *crtc_state)
>> +{
>> +	if (!crtc_state->gamma_enable)
>> +		return 0;
>> +
>> +	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
>> +		return 0;
>> +
>> +	switch (crtc_state->gamma_mode) {
>> +	case GAMMA_MODE_MODE_8BIT:
>> +		return 8;
>> +	case GAMMA_MODE_MODE_10BIT:
>> +		return 10;
>> +	default:
>> +		MISSING_CASE(crtc_state->gamma_mode);
>> +		return 0;
>> +	}
>> +}
>> +
>> +static int ivb_gamma_precision(struct intel_crtc_state *crtc_state)
>> +{
>> +	if (!crtc_state->gamma_enable)
>> +		return 0;
>> +
>> +	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
>> +		return 0;
>> +
>> +	switch (crtc_state->gamma_mode) {
>> +	case GAMMA_MODE_MODE_8BIT:
>> +		return 8;
>> +	case GAMMA_MODE_MODE_SPLIT:
>> +	case GAMMA_MODE_MODE_10BIT:
>> +		return 10;
>> +	default:
>> +		MISSING_CASE(crtc_state->gamma_mode);
>> +		return 0;
>> +	}
>> +}
>> +
>> +static int glk_gamma_precision(struct intel_crtc_state *crtc_state)
>> +{
>> +	if (!crtc_state->gamma_enable)
>> +		return 0;
>> +
>> +	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
>> +		return 0;
>> +
>> +	switch (crtc_state->gamma_mode) {
>> +	case GAMMA_MODE_MODE_8BIT:
>> +		return 8;
>> +	case GAMMA_MODE_MODE_10BIT:
>> +		return 10;
>> +	default:
>> +		MISSING_CASE(crtc_state->gamma_mode);
>> +		return 0;
>> +	}
>> +}
>> +
>> +static int icl_gamma_precision(struct intel_crtc_state *crtc_state)
>> +{
>> +	if ((crtc_state->gamma_mode & PRE_CSC_GAMMA_ENABLE) == 0)
>> +		return 0;
>> +
>> +	switch (crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) {
>> +	case GAMMA_MODE_MODE_8BIT:
>> +		return 8;
>> +	case GAMMA_MODE_MODE_10BIT:
>> +		return 10;
>> +	default:
>> +		MISSING_CASE(crtc_state->gamma_mode);
>> +		return 0;
>> +	}
>> +}
>> +
>> +int intel_color_get_gamma_bit_precision(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);
>> +
>> +	if (HAS_GMCH(dev_priv)) {
>> +		if (IS_CHERRYVIEW(dev_priv))
>> +			return chv_gamma_precision(crtc_state);
>> +		else
>> +			return i9xx_gamma_precision(crtc_state);
>> +	} else {
>> +		if (INTEL_GEN(dev_priv) >= 11)
>> +			return icl_gamma_precision(crtc_state);
>> +		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>> +			return glk_gamma_precision(crtc_state);
>> +		else if (INTEL_GEN(dev_priv) >= 7)
>> +			return ivb_gamma_precision(crtc_state);
>> +		else
>> +			return ilk_gamma_precision(crtc_state);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +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 bit_precision)
>> +{
>> +	struct drm_color_lut *sw_lut, *hw_lut;
>> +	int sw_lut_size, hw_lut_size, i;
>> +	u32 err;
>> +
>> +	if (!blob1 && !blob2)
>> +		return true;
>> +
>> +	if (!blob1)
>> +		return true;
>> +
>> +	if (!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;
>> +
>> +	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 057e8ac..02ea1bc 100644
>> --- a/drivers/gpu/drm/i915/intel_color.h
>> +++ b/drivers/gpu/drm/i915/intel_color.h
>> @@ -6,13 +6,20 @@
>>   #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_get_config(struct intel_crtc_state *crtc_state);
>> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
>> +			   struct drm_property_blob *blob2,
>> +			   u32 bit_precision);
>> +int intel_color_get_gamma_bit_precision(struct intel_crtc_state *crtc_state);
>>   
>>   #endif /* __INTEL_COLOR_H__ */
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 3e01028..b8ff3f4 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -11570,6 +11570,15 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>>   				      drm_rect_width(&state->base.dst),
>>   				      drm_rect_height(&state->base.dst));
>>   	}
>> +
>> +	if (IS_CHERRYVIEW(dev_priv))
>> +		DRM_DEBUG_KMS("cgm_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
>> +			       pipe_config->cgm_mode, pipe_config->gamma_mode,
>> +			       pipe_config->gamma_enable, pipe_config->csc_enable);
>> +	else
>> +		DRM_DEBUG_KMS("csc_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
>> +			       pipe_config->csc_mode, pipe_config->gamma_mode,
>> +			       pipe_config->gamma_enable, pipe_config->csc_enable);
>>   }
>>   
>>   static bool check_digital_port_conflicts(struct drm_atomic_state *state)
>> @@ -11947,6 +11956,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>>   			  bool adjust)
>>   {
>>   	bool ret = true;
>> +	u32 bp_gamma = 0;
>>   	bool fixup_inherited = adjust &&
>>   		(current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
>>   		!(pipe_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
>> @@ -12098,6 +12108,15 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>>   	} \
>>   } while (0)
>>   
>> +#define PIPE_CONF_CHECK_COLOR_LUT(name, bit_precision) do { \
>> +	if (!intel_color_lut_equal(current_config->name, \
>> +				   pipe_config->name, bit_precision)) { \
>> +		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))
>>   
>> @@ -12193,6 +12212,10 @@ 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);
>> +
>> +		bp_gamma = intel_color_get_gamma_bit_precision(pipe_config);
>> +		if (bp_gamma)
>> +			PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, bp_gamma);
>>   	}
>>   
>>   	PIPE_CONF_CHECK_BOOL(double_wide);
>> @@ -12255,6 +12278,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;
>>   }


-- 
~Swati Sharma


[-- Attachment #1.2: Type: text/html, Size: 10555 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: [v7][PATCH 00/12] drm/i915: adding state checker for gamma lut values

[Jani Nikula]

On Wed, 29 May 2019, Swati Sharma <swati2.sharma@intel.com> wrote:
> In this patch series, added state checker to validate gamma
> and will be extended to validate degamma lut values aswell.
> This reads hardware state, and compares the originally
> requested state to the state read from hardware.

What happened to this patch series?

BR,
Jani.

-- 
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: [v7][PATCH 00/12] drm/i915: adding state checker for gamma lut values

[Sharma, Swati2]

Hi Jani,

I was involved in other activities. Will resume work on this now. 

Thanks and Regards,
Swati

-----Original Message-----
From: Jani Nikula <jani.nikula@linux.intel.com> 
Sent: Thursday, August 15, 2019 1:44 PM
To: Sharma, Swati2 <swati2.sharma@intel.com>; intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [v7][PATCH 00/12] drm/i915: adding state checker for gamma lut values

On Wed, 29 May 2019, Swati Sharma <swati2.sharma@intel.com> wrote:
> In this patch series, added state checker to validate gamma and will 
> be extended to validate degamma lut values aswell.
> This reads hardware state, and compares the originally requested state 
> to the state read from hardware.

What happened to this patch series?

BR,
Jani.

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