All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 00/11] drm/i915: adding state checker for gamma lut values
@ 2019-04-17 14:03 Swati Sharma
  2019-04-17 14:03 ` [PATCH 01/11] [v4] drm/i915: Introduce vfunc intel_get_color_config to create hw lut Swati Sharma
                   ` (17 more replies)
  0 siblings, 18 replies; 23+ messages in thread
From: Swati Sharma @ 2019-04-17 14:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal

Thanks to Jani N, Matt and Ville for the review comments. Hopefully
I have addressed all the current review comments and ready to receive more :)

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

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

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

v3: -Rebase

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

TODO:  -Matt suggested to rename get_gamma_config to get_gamma_lut or
        i9xx_read_out_gamma_lut, Ville also named functions like i9xx_read_lut_8(),
        i9xx_read_lut_10(). Should we follow this?
       -Answering Matt's query regarding intension to read and compare degamma/ctm".
        Degamma will be my first priority after this and later ctm.
        Should we have combined func or separate func for gamma/degamma val?
       -Add separate func to log errors. I am not sure, what needs to be printed here?
        Mismatched RGB value? 
       -Will make changes in next rev for changes made by Ville for cherryview_load_luts()


Swati Sharma (11):
  [v4] drm/i915: Introduce vfunc intel_get_color_config to create hw lut
  [v4] drm/i915: Enable intel_color_get_config()
  [v4] drm/i915: Extract i9xx_get_color_config()
  [v4] drm/i915: Extract cherryview_get_color_config()
  [v4] drm/i915: Extract i965_get_color_config()
  [v4] drm/i915: Extract icl_get_color_config()
  [v4] drm/i915: Extract glk_get_color_config()
  [v4] drm/i915: Extract bdw_get_color_config()
  [v4] drm/i915: Extract ivb_get_color_config()
  [v4] drm/i915: Extract ilk_get_color_config()
  [v4] drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma
    lut values

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

-- 
1.9.1

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

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

* [PATCH 01/11] [v4] drm/i915: Introduce vfunc intel_get_color_config to create hw lut
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
@ 2019-04-17 14:03 ` Swati Sharma
  2019-04-17 14:03 ` [PATCH 02/11] [v4] drm/i915: Enable intel_color_get_config() Swati Sharma
                   ` (16 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Swati Sharma @ 2019-04-17 14:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal

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

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 63eca30..69ed05c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -342,6 +342,7 @@ struct drm_i915_display_funcs {
 	 * involved with the same commit.
 	 */
 	void (*load_luts)(const struct intel_crtc_state *crtc_state);
+	void (*get_color_config)(struct intel_crtc_state *crtc_state);
 };
 
 #define CSR_VERSION(major, minor)	((major) << 16 | (minor))
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index ca341a9..309f714 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -857,6 +857,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.get_color_config)
+		dev_priv->display.get_color_config(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

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

* [PATCH 02/11] [v4] drm/i915: Enable intel_color_get_config()
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
  2019-04-17 14:03 ` [PATCH 01/11] [v4] drm/i915: Introduce vfunc intel_get_color_config to create hw lut Swati Sharma
@ 2019-04-17 14:03 ` Swati Sharma
  2019-04-23 15:57   ` Ville Syrjälä
  2019-04-17 14:03 ` [PATCH 03/11] [v4] drm/i915: Extract i9xx_get_color_config() Swati Sharma
                   ` (15 subsequent siblings)
  17 siblings, 1 reply; 23+ messages in thread
From: Swati Sharma @ 2019-04-17 14:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal

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]

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

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f29a348..0715b4e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8275,6 +8275,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;
@@ -9348,6 +9349,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;
@@ -10011,6 +10013,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 			pipe_config->csc_enable = true;
 	} else {
 		i9xx_get_pipe_color_config(pipe_config);
+		intel_color_get_config(pipe_config);
 	}
 
 	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
-- 
1.9.1

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

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

* [PATCH 03/11] [v4] drm/i915: Extract i9xx_get_color_config()
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
  2019-04-17 14:03 ` [PATCH 01/11] [v4] drm/i915: Introduce vfunc intel_get_color_config to create hw lut Swati Sharma
  2019-04-17 14:03 ` [PATCH 02/11] [v4] drm/i915: Enable intel_color_get_config() Swati Sharma
@ 2019-04-17 14:03 ` Swati Sharma
  2019-04-23 16:13   ` Ville Syrjälä
  2019-04-17 14:03 ` [PATCH 04/11] [v4] drm/i915: Extract cherryview_get_color_config() Swati Sharma
                   ` (14 subsequent siblings)
  17 siblings, 1 reply; 23+ messages in thread
From: Swati Sharma @ 2019-04-17 14:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal

v4: -No need to initialize *blob [Jani]
    -Removed right shifts [Jani]
    -Dropped dev local var [Jani]

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9c206e8..8f2ae8a 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7175,6 +7175,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 309f714..33223e0 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1229,6 +1229,55 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
 	return 0;
 }
 
+/* convert hw value with given bit_precision to lut property val */
+static u32 intel_color_lut_pack(u32 val, u32 bit_precision)
+{
+	u32 max = 0xffff >> (16 - bit_precision);
+
+	val = clamp_val(val, 0, max);
+
+	if (bit_precision < 16)
+		val <<= 16 - bit_precision;
+
+	return val;
+}
+
+static void i9xx_get_config_internal(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;
+
+	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);
+	}
+
+	crtc_state->base.gamma_lut = blob;
+}
+
+static void i9xx_get_color_config(struct intel_crtc_state *crtc_state)
+{
+	i9xx_get_config_internal(crtc_state);
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1249,6 +1298,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.get_color_config = i9xx_get_color_config;
 		}
 	} 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

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

* [PATCH 04/11] [v4] drm/i915: Extract cherryview_get_color_config()
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
                   ` (2 preceding siblings ...)
  2019-04-17 14:03 ` [PATCH 03/11] [v4] drm/i915: Extract i9xx_get_color_config() Swati Sharma
@ 2019-04-17 14:03 ` Swati Sharma
  2019-04-17 14:03 ` [PATCH 05/11] [v4] drm/i915: Extract i965_get_color_config() Swati Sharma
                   ` (13 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Swati Sharma @ 2019-04-17 14:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal

v4: -No need to initialize *blob [Jani]
    -Removed right shifts [Jani]
    -Dropped dev local var [Jani]

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8f2ae8a..c9ae61e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10151,6 +10151,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 33223e0..818e062 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1278,6 +1278,43 @@ static void i9xx_get_color_config(struct intel_crtc_state *crtc_state)
 	i9xx_get_config_internal(crtc_state);
 }
 
+static void cherryview_get_gamma_config(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) * 256,
+					NULL);
+	if (IS_ERR(blob))
+		return;
+
+	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);
+	}
+
+	crtc_state->base.gamma_lut = blob;
+}
+
+static void cherryview_get_color_config(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
+		i9xx_get_color_config(crtc_state);
+	else
+		cherryview_get_gamma_config(crtc_state);
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1290,6 +1327,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 = cherryview_load_luts;
+			dev_priv->display.get_color_config = cherryview_get_color_config;
 		} 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

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

* [PATCH 05/11] [v4] drm/i915: Extract i965_get_color_config()
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
                   ` (3 preceding siblings ...)
  2019-04-17 14:03 ` [PATCH 04/11] [v4] drm/i915: Extract cherryview_get_color_config() Swati Sharma
@ 2019-04-17 14:03 ` Swati Sharma
  2019-04-17 14:03 ` [PATCH 06/11] [v4] drm/i915: Extract icl_get_color_config() Swati Sharma
                   ` (12 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Swati Sharma @ 2019-04-17 14:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal

v4: -No need to initialize *blob [Jani]
    -Removed right shifts [Jani]
    -Dropped dev local var [Jani]

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index c9ae61e..1d0575f 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3582,6 +3582,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 818e062..8b8dac6 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1315,6 +1315,43 @@ static void cherryview_get_color_config(struct intel_crtc_state *crtc_state)
 		cherryview_get_gamma_config(crtc_state);
 }
 
+static void i965_get_gamma_config_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;
+
+	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) ;
+	}
+
+	crtc_state->base.gamma_lut = blob;
+}
+
+static void i965_get_color_config(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
+		i9xx_get_color_config(crtc_state);
+	else
+		i965_get_gamma_config_10p6(crtc_state);
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1332,6 +1369,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.get_color_config = i965_get_color_config;
 		} 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

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

* [PATCH 06/11] [v4] drm/i915: Extract icl_get_color_config()
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
                   ` (4 preceding siblings ...)
  2019-04-17 14:03 ` [PATCH 05/11] [v4] drm/i915: Extract i965_get_color_config() Swati Sharma
@ 2019-04-17 14:03 ` Swati Sharma
  2019-04-17 14:03 ` [PATCH 07/11] [v4] drm/i915: Extract glk_get_color_config() Swati Sharma
                   ` (11 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Swati Sharma @ 2019-04-17 14:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal

v4: -No need to initialize *blob [Jani]
    -Removed right shifts [Jani]
    -Dropped dev local var [Jani]

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 1d0575f..38d6684 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10115,6 +10115,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 8b8dac6..2e3b35c 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1352,6 +1352,50 @@ static void i965_get_color_config(struct intel_crtc_state *crtc_state)
 		i965_get_gamma_config_10p6(crtc_state);
 }
 
+static void bdw_get_gamma_config(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;
+
+	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);
+
+	crtc_state->base.gamma_lut = blob;
+}
+
+static void icl_get_color_config(struct intel_crtc_state *crtc_state)
+{
+	if ((crtc_state->gamma_mode & GAMMA_MODE_MODE_MASK) ==
+	    GAMMA_MODE_MODE_8BIT)
+		i9xx_get_color_config(crtc_state);
+	else
+		bdw_get_gamma_config(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);
@@ -1393,8 +1437,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.get_color_config = icl_get_color_config;
+		}
 		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

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

* [PATCH 07/11] [v4] drm/i915: Extract glk_get_color_config()
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
                   ` (5 preceding siblings ...)
  2019-04-17 14:03 ` [PATCH 06/11] [v4] drm/i915: Extract icl_get_color_config() Swati Sharma
@ 2019-04-17 14:03 ` Swati Sharma
  2019-04-17 14:03 ` [PATCH 08/11] [v4] drm/i915: Extract bdw_get_color_config() Swati Sharma
                   ` (10 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Swati Sharma @ 2019-04-17 14:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal

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 2e3b35c..28cb490 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1396,6 +1396,14 @@ static void icl_get_color_config(struct intel_crtc_state *crtc_state)
 		bdw_get_gamma_config(crtc_state, PAL_PREC_INDEX_VALUE(0));
 }
 
+static void glk_get_color_config(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
+		i9xx_get_color_config(crtc_state);
+	else
+		bdw_get_gamma_config(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);
@@ -1440,9 +1448,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.get_color_config = icl_get_color_config;
-		}
-		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.get_color_config = glk_get_color_config;
+		}
 		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

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

* [PATCH 08/11] [v4] drm/i915: Extract bdw_get_color_config()
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
                   ` (6 preceding siblings ...)
  2019-04-17 14:03 ` [PATCH 07/11] [v4] drm/i915: Extract glk_get_color_config() Swati Sharma
@ 2019-04-17 14:03 ` Swati Sharma
  2019-04-17 14:03 ` [PATCH 09/11] [v4] drm/i915: Extract ivb_get_color_config() Swati Sharma
                   ` (9 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Swati Sharma @ 2019-04-17 14:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal

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 28cb490..0d0d57c 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1404,6 +1404,17 @@ static void glk_get_color_config(struct intel_crtc_state *crtc_state)
 		bdw_get_gamma_config(crtc_state, PAL_PREC_INDEX_VALUE(0));
 }
 
+static void bdw_get_color_config(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
+		i9xx_get_color_config(crtc_state);
+	else if (crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT)
+		bdw_get_gamma_config(crtc_state, PAL_PREC_SPLIT_MODE |
+				     PAL_PREC_INDEX_VALUE(512));
+	else
+		bdw_get_gamma_config(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);
@@ -1451,9 +1462,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.get_color_config = glk_get_color_config;
-		}
-		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.get_color_config = bdw_get_color_config;
+		}
 		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

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

* [PATCH 09/11] [v4] drm/i915: Extract ivb_get_color_config()
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
                   ` (7 preceding siblings ...)
  2019-04-17 14:03 ` [PATCH 08/11] [v4] drm/i915: Extract bdw_get_color_config() Swati Sharma
@ 2019-04-17 14:03 ` Swati Sharma
  2019-04-17 14:03 ` [PATCH 10/11] [v4] drm/i915: Extract ilk_get_color_config() Swati Sharma
                   ` (8 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Swati Sharma @ 2019-04-17 14:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal

v4: -No need to initialize *blob [Jani]
    -Removed right shifts [Jani]
    -Dropped dev local var [Jani]

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 0d0d57c..88fc1be 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1415,6 +1415,50 @@ static void bdw_get_color_config(struct intel_crtc_state *crtc_state)
 		bdw_get_gamma_config(crtc_state, PAL_PREC_INDEX_VALUE(0));
 }
 
+static void ivb_get_gamma_config(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;
+
+	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);
+
+	crtc_state->base.gamma_lut = blob;
+}
+
+static void ivb_get_color_config(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
+		i9xx_get_color_config(crtc_state);
+	else if (crtc_state->gamma_mode == GAMMA_MODE_MODE_SPLIT)
+		ivb_get_gamma_config(crtc_state, PAL_PREC_SPLIT_MODE |
+				     PAL_PREC_INDEX_VALUE(512));
+	else
+		bdw_get_gamma_config(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);
@@ -1465,9 +1509,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.get_color_config = bdw_get_color_config;
-		}
-		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.get_color_config = ivb_get_color_config;
+		}
 		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

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

* [PATCH 10/11] [v4] drm/i915: Extract ilk_get_color_config()
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
                   ` (8 preceding siblings ...)
  2019-04-17 14:03 ` [PATCH 09/11] [v4] drm/i915: Extract ivb_get_color_config() Swati Sharma
@ 2019-04-17 14:03 ` Swati Sharma
  2019-04-17 14:03 ` [PATCH 11/11] [v4] drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma lut values Swati Sharma
                   ` (7 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Swati Sharma @ 2019-04-17 14:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal

v4: -No need to initialize *blob [Jani]
    -Removed right shifts [Jani]
    -Dropped dev local var [Jani]

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 38d6684..f6a41f7 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7186,6 +7186,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 88fc1be..731cb97 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1459,6 +1459,42 @@ static void ivb_get_color_config(struct intel_crtc_state *crtc_state)
 		bdw_get_gamma_config(crtc_state, PAL_PREC_INDEX_VALUE(0));
 }
 
+static void ilk_get_gamma_config(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;
+
+	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);
+	}
+
+	crtc_state->base.gamma_lut = blob;
+}
+
+static void ilk_get_color_config(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
+		i9xx_get_color_config(crtc_state);
+	else
+		ilk_get_gamma_config(crtc_state);
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1512,9 +1548,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.get_color_config = ivb_get_color_config;
-		}
-		else
+		} else {
 			dev_priv->display.load_luts = ilk_load_luts;
+			dev_priv->display.get_color_config = ilk_get_color_config;
+		}
 	}
 
 	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

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

* [PATCH 11/11] [v4] drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma lut values
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
                   ` (9 preceding siblings ...)
  2019-04-17 14:03 ` [PATCH 10/11] [v4] drm/i915: Extract ilk_get_color_config() Swati Sharma
@ 2019-04-17 14:03 ` Swati Sharma
  2019-04-25 12:35   ` Jani Nikula
  2019-04-17 14:23 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: adding state checker for gamma lut values (rev5) Patchwork
                   ` (6 subsequent siblings)
  17 siblings, 1 reply; 23+ messages in thread
From: Swati Sharma @ 2019-04-17 14:03 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula, daniel.vetter, ankit.k.nautiyal

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)

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

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 731cb97..592c328 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -1495,6 +1495,56 @@ static void ilk_get_color_config(struct intel_crtc_state *crtc_state)
 		ilk_get_gamma_config(crtc_state);
 }
 
+static inline bool gamma_err_check(struct drm_color_lut *sw_lut, struct drm_color_lut *hw_lut, u32 err)
+{
+	 return ((abs((long)hw_lut->red - sw_lut->red)) >= err) ||
+	        ((abs((long)hw_lut->blue - sw_lut->blue)) >= err) ||
+	        ((abs((long)hw_lut->green - sw_lut->green)) >= err);
+}
+
+bool intel_color_lut_equal(struct drm_property_blob *blob1,
+			   struct drm_property_blob *blob2,
+			   u32 gamma_mode)
+{
+	struct drm_color_lut *sw_lut, *hw_lut;
+	int sw_lut_size, hw_lut_size, i;
+	u32 bit_precision, err;
+
+	if (!blob1 || !blob2)
+		return false;
+
+	sw_lut = blob1->data;
+	hw_lut = blob2->data;
+
+	switch(gamma_mode) {
+	default:
+	case GAMMA_MODE_MODE_8BIT:
+		bit_precision = 8;
+		break;
+	case GAMMA_MODE_MODE_10BIT:
+		bit_precision = 10;
+		break;
+	case GAMMA_MODE_MODE_12BIT:
+		bit_precision = 12;
+		break;
+	}
+
+	err = 0xffff >> bit_precision;
+
+	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;
+
+	for (i = 0; i < sw_lut_size; i++) {
+		 if (!gamma_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..2ae615e 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 gamma_mode);
 
 #endif /* __INTEL_COLOR_H__ */
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 0715b4e..bb1ccd7 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12236,6 +12236,14 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 	} \
 } while (0)
 
+#define PIPE_CONF_CHECK_COLOR_LUT(name, gamma_mode) do { \
+	if (!intel_color_lut_equal(current_config->name, pipe_config->name, gamma_mode)) { \
+		pipe_config_err(adjust, __stringify(name), \
+				"hw_state doesn't match sw_state\n"); \
+		ret = false; \
+	} \
+} while (0)
+
 #define PIPE_CONF_QUIRK(quirk) \
 	((current_config->quirks | pipe_config->quirks) & (quirk))
 
@@ -12379,6 +12387,8 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 	PIPE_CONF_CHECK_INFOFRAME(spd);
 	PIPE_CONF_CHECK_INFOFRAME(hdmi);
 
+	PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, pipe_config->gamma_mode);
+
 #undef PIPE_CONF_CHECK_X
 #undef PIPE_CONF_CHECK_I
 #undef PIPE_CONF_CHECK_BOOL
-- 
1.9.1

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

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

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: adding state checker for gamma lut values (rev5)
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
                   ` (10 preceding siblings ...)
  2019-04-17 14:03 ` [PATCH 11/11] [v4] drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma lut values Swati Sharma
@ 2019-04-17 14:23 ` Patchwork
  2019-04-17 14:28 ` ✗ Fi.CI.SPARSE: " Patchwork
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-04-17 14:23 UTC (permalink / raw)
  To: Swati Sharma; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

$ dim checkpatch origin/drm-tip
3100439817a3 drm/i915: Introduce vfunc intel_get_color_config to create hw lut
153c16eea81c drm/i915: Enable intel_color_get_config()
330f4381f694 drm/i915: Extract i9xx_get_color_config()
-:70: WARNING:LONG_LINE: line over 100 characters
#70: FILE: drivers/gpu/drm/i915/intel_color.c:1268:
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_RED_MASK, val), 8);

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

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

total: 0 errors, 3 warnings, 0 checks, 71 lines checked
423d40988765 drm/i915: Extract cherryview_get_color_config()
-:53: WARNING:LONG_LINE: line over 100 characters
#53: FILE: drivers/gpu/drm/i915/intel_color.c:1300:
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_GREEN_MASK, val), 10);

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

-:57: WARNING:LONG_LINE: line over 100 characters
#57: FILE: drivers/gpu/drm/i915/intel_color.c:1304:
+		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, 59 lines checked
43969f51c6e2 drm/i915: Extract i965_get_color_config()
-:55: WARNING:LONG_LINE: line over 100 characters
#55: FILE: drivers/gpu/drm/i915/intel_color.c:1339:
+		blob_data[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val1) << 8 | REG_FIELD_GET(PALETTE_RED_MASK, val2);

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

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

-:57: WARNING:SPACING: space prohibited before semicolon
#57: FILE: drivers/gpu/drm/i915/intel_color.c:1341:
+		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, 59 lines checked
aa35ccf89f32 drm/i915: Extract icl_get_color_config()
-:35: ERROR:CODE_INDENT: code indent should use tabs where possible
#35: FILE: drivers/gpu/drm/i915/intel_color.c:1356:
+^I^I^I         u32 prec_index)$

-:35: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#35: FILE: drivers/gpu/drm/i915/intel_color.c:1356:
+static void bdw_get_gamma_config(struct intel_crtc_state *crtc_state,
+			         u32 prec_index)

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

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

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

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

total: 1 errors, 3 warnings, 2 checks, 70 lines checked
4d9073678178 drm/i915: Extract glk_get_color_config()
-:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

total: 0 errors, 1 warnings, 0 checks, 26 lines checked
d953908e718e drm/i915: Extract bdw_get_color_config()
-:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

total: 0 errors, 1 warnings, 0 checks, 29 lines checked
c5ea122a30b2 drm/i915: Extract ivb_get_color_config()
-:21: ERROR:CODE_INDENT: code indent should use tabs where possible
#21: FILE: drivers/gpu/drm/i915/intel_color.c:1419:
+^I^I^I         u32 prec_index)$

-:21: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#21: FILE: drivers/gpu/drm/i915/intel_color.c:1419:
+static void ivb_get_gamma_config(struct intel_crtc_state *crtc_state,
+			         u32 prec_index)

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

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

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

total: 1 errors, 3 warnings, 1 checks, 62 lines checked
aa17ac0b1937 drm/i915: Extract ilk_get_color_config()
-:54: WARNING:LONG_LINE: line over 100 characters
#54: FILE: drivers/gpu/drm/i915/intel_color.c:1482:
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(PREC_PALETTE_RED_MASK, val), 10);

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

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

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

-:22: WARNING:LONG_LINE: line over 100 characters
#22: FILE: drivers/gpu/drm/i915/intel_color.c:1498:
+static inline bool gamma_err_check(struct drm_color_lut *sw_lut, struct drm_color_lut *hw_lut, u32 err)

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

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

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

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

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

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

-:83: CHECK:LINE_SPACING: Please don't use multiple blank lines
#83: FILE: drivers/gpu/drm/i915/intel_color.h:9:
 
+

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

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

total: 3 errors, 5 warnings, 3 checks, 98 lines checked

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

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

* ✗ Fi.CI.SPARSE: warning for drm/i915: adding state checker for gamma lut values (rev5)
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
                   ` (11 preceding siblings ...)
  2019-04-17 14:23 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: adding state checker for gamma lut values (rev5) Patchwork
@ 2019-04-17 14:28 ` Patchwork
  2019-04-17 14:37 ` ✗ Fi.CI.BAT: failure " Patchwork
                   ` (4 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-04-17 14:28 UTC (permalink / raw)
  To: Swati Sharma; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: adding state checker for gamma lut values (rev5)
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 intel_get_color_config to create hw lut
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3616:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3617:16: warning: expression using sizeof(void)

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

Commit: drm/i915: Extract i9xx_get_color_config()
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)

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

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

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

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

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

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

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

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

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

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

* ✗ Fi.CI.BAT: failure for drm/i915: adding state checker for gamma lut values (rev5)
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
                   ` (12 preceding siblings ...)
  2019-04-17 14:28 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-04-17 14:37 ` Patchwork
  2019-04-18 10:03 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: adding state checker for gamma lut values (rev6) Patchwork
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-04-17 14:37 UTC (permalink / raw)
  To: Swati Sharma; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_5947 -> Patchwork_12822
====================================================

Summary
-------

  **FAILURE**

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

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

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@runner@aborted:
    - fi-ilk-650:         NOTRUN -> FAIL
    - fi-pnv-d510:        NOTRUN -> FAIL
    - fi-bdw-gvtdvm:      NOTRUN -> FAIL
    - fi-hsw-peppy:       NOTRUN -> FAIL
    - fi-gdg-551:         NOTRUN -> FAIL
    - fi-snb-2520m:       NOTRUN -> FAIL
    - fi-hsw-4770:        NOTRUN -> FAIL
    - fi-whl-u:           NOTRUN -> FAIL
    - fi-icl-u3:          NOTRUN -> FAIL
    - fi-ivb-3770:        NOTRUN -> FAIL
    - fi-byt-j1900:       NOTRUN -> FAIL
    - fi-icl-y:           NOTRUN -> FAIL
    - fi-bsw-n3050:       NOTRUN -> FAIL
    - fi-blb-e6850:       NOTRUN -> FAIL
    - fi-hsw-4770r:       NOTRUN -> FAIL
    - fi-byt-clapper:     NOTRUN -> FAIL
    - fi-bdw-5557u:       NOTRUN -> FAIL
    - fi-bwr-2160:        NOTRUN -> FAIL
    - fi-byt-n2820:       NOTRUN -> FAIL
    - fi-elk-e7500:       NOTRUN -> FAIL

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

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

### IGT changes ###

#### Issues hit ####

  * igt@runner@aborted:
    - fi-cfl-8109u:       NOTRUN -> FAIL [k.org#202107] / [k.org#202109]
    - fi-glk-dsi:         NOTRUN -> FAIL [k.org#202321]
    - fi-bxt-j4205:       NOTRUN -> FAIL [fdo#109516]
    - fi-bxt-dsi:         NOTRUN -> FAIL [fdo#109516]
    - fi-skl-iommu:       NOTRUN -> FAIL [fdo#104108]
    - fi-cfl-guc:         NOTRUN -> FAIL [k.org#202107] / [k.org#202109]
    - fi-skl-guc:         NOTRUN -> FAIL [fdo#104108]
    - fi-skl-6700k2:      NOTRUN -> FAIL [fdo#104108]
    - fi-kbl-x1275:       NOTRUN -> FAIL [fdo#108903] / [fdo#108904] / [fdo#108905]
    - fi-cfl-8700k:       NOTRUN -> FAIL [k.org#202107] / [k.org#202109]
    - fi-skl-6600u:       NOTRUN -> FAIL [fdo#104108]
    - fi-skl-lmem:        NOTRUN -> FAIL [fdo#104108]
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#109373]
    - fi-kbl-r:           NOTRUN -> FAIL [fdo#108903] / [fdo#108904] / [fdo#108905]
    - fi-skl-6770hq:      NOTRUN -> FAIL [fdo#104108]
    - fi-skl-gvtdvm:      NOTRUN -> FAIL [fdo#104108]
    - fi-snb-2600:        NOTRUN -> FAIL [fdo#108929]
    - fi-skl-6260u:       NOTRUN -> FAIL [fdo#104108]

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

  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#108903]: https://bugs.freedesktop.org/show_bug.cgi?id=108903
  [fdo#108904]: https://bugs.freedesktop.org/show_bug.cgi?id=108904
  [fdo#108905]: https://bugs.freedesktop.org/show_bug.cgi?id=108905
  [fdo#108929]: https://bugs.freedesktop.org/show_bug.cgi?id=108929
  [fdo#109373]: https://bugs.freedesktop.org/show_bug.cgi?id=109373
  [fdo#109516]: https://bugs.freedesktop.org/show_bug.cgi?id=109516
  [k.org#202107]: https://bugzilla.kernel.org/show_bug.cgi?id=202107
  [k.org#202109]: https://bugzilla.kernel.org/show_bug.cgi?id=202109
  [k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321


Participating hosts (48 -> 43)
------------------------------

  Missing    (5): fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-icl-u2 fi-ctg-p8600 


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

    * Linux: CI_DRM_5947 -> Patchwork_12822

  CI_DRM_5947: 2d16e0372a8a0035697b5ce0bdb98f64731810af @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4956: 1d921615b0b706f25c856aa0eb096f274380c199 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12822: 082fe8983b84ee33d1915855f94bb239c42a2c6e @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

082fe8983b84 drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma lut values
aa17ac0b1937 drm/i915: Extract ilk_get_color_config()
c5ea122a30b2 drm/i915: Extract ivb_get_color_config()
d953908e718e drm/i915: Extract bdw_get_color_config()
4d9073678178 drm/i915: Extract glk_get_color_config()
aa35ccf89f32 drm/i915: Extract icl_get_color_config()
43969f51c6e2 drm/i915: Extract i965_get_color_config()
423d40988765 drm/i915: Extract cherryview_get_color_config()
330f4381f694 drm/i915: Extract i9xx_get_color_config()
153c16eea81c drm/i915: Enable intel_color_get_config()
3100439817a3 drm/i915: Introduce vfunc intel_get_color_config to create hw lut

== Logs ==

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

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

* ✗ Fi.CI.CHECKPATCH: warning for drm/i915: adding state checker for gamma lut values (rev6)
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
                   ` (13 preceding siblings ...)
  2019-04-17 14:37 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2019-04-18 10:03 ` Patchwork
  2019-04-18 10:08 ` ✗ Fi.CI.SPARSE: " Patchwork
                   ` (2 subsequent siblings)
  17 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-04-18 10:03 UTC (permalink / raw)
  To: Swati Sharma; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

$ dim checkpatch origin/drm-tip
4912527e1aba drm/i915: Introduce vfunc intel_get_color_config to create hw lut
0bfa41d5f0d7 drm/i915: Enable intel_color_get_config()
c4668870f66f drm/i915: Extract i9xx_get_color_config()
-:70: WARNING:LONG_LINE: line over 100 characters
#70: FILE: drivers/gpu/drm/i915/intel_color.c:1268:
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_RED_MASK, val), 8);

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

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

total: 0 errors, 3 warnings, 0 checks, 71 lines checked
5d9703416dc0 drm/i915: Extract cherryview_get_color_config()
-:53: WARNING:LONG_LINE: line over 100 characters
#53: FILE: drivers/gpu/drm/i915/intel_color.c:1300:
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_GREEN_MASK, val), 10);

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

-:57: WARNING:LONG_LINE: line over 100 characters
#57: FILE: drivers/gpu/drm/i915/intel_color.c:1304:
+		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, 59 lines checked
d60da8743997 drm/i915: Extract i965_get_color_config()
-:55: WARNING:LONG_LINE: line over 100 characters
#55: FILE: drivers/gpu/drm/i915/intel_color.c:1339:
+		blob_data[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val1) << 8 | REG_FIELD_GET(PALETTE_RED_MASK, val2);

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

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

-:57: WARNING:SPACING: space prohibited before semicolon
#57: FILE: drivers/gpu/drm/i915/intel_color.c:1341:
+		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, 59 lines checked
3de95a9fd6c4 drm/i915: Extract icl_get_color_config()
-:35: ERROR:CODE_INDENT: code indent should use tabs where possible
#35: FILE: drivers/gpu/drm/i915/intel_color.c:1356:
+^I^I^I         u32 prec_index)$

-:35: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#35: FILE: drivers/gpu/drm/i915/intel_color.c:1356:
+static void bdw_get_gamma_config(struct intel_crtc_state *crtc_state,
+			         u32 prec_index)

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

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

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

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

total: 1 errors, 3 warnings, 2 checks, 70 lines checked
072235c17597 drm/i915: Extract glk_get_color_config()
-:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

total: 0 errors, 1 warnings, 0 checks, 26 lines checked
fa8d86057b5c drm/i915: Extract bdw_get_color_config()
-:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

total: 0 errors, 1 warnings, 0 checks, 29 lines checked
17c09be650d6 drm/i915: Extract ivb_get_color_config()
-:21: ERROR:CODE_INDENT: code indent should use tabs where possible
#21: FILE: drivers/gpu/drm/i915/intel_color.c:1419:
+^I^I^I         u32 prec_index)$

-:21: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#21: FILE: drivers/gpu/drm/i915/intel_color.c:1419:
+static void ivb_get_gamma_config(struct intel_crtc_state *crtc_state,
+			         u32 prec_index)

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

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

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

total: 1 errors, 3 warnings, 1 checks, 62 lines checked
b880360402ee drm/i915: Extract ilk_get_color_config()
-:54: WARNING:LONG_LINE: line over 100 characters
#54: FILE: drivers/gpu/drm/i915/intel_color.c:1482:
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(PREC_PALETTE_RED_MASK, val), 10);

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

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

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

-:22: WARNING:LONG_LINE: line over 100 characters
#22: FILE: drivers/gpu/drm/i915/intel_color.c:1498:
+static inline bool gamma_err_check(struct drm_color_lut *sw_lut, struct drm_color_lut *hw_lut, u32 err)

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

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

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

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

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

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

-:83: CHECK:LINE_SPACING: Please don't use multiple blank lines
#83: FILE: drivers/gpu/drm/i915/intel_color.h:9:
 
+

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

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

total: 3 errors, 5 warnings, 3 checks, 98 lines checked

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

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

* ✗ Fi.CI.SPARSE: warning for drm/i915: adding state checker for gamma lut values (rev6)
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
                   ` (14 preceding siblings ...)
  2019-04-18 10:03 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: adding state checker for gamma lut values (rev6) Patchwork
@ 2019-04-18 10:08 ` Patchwork
  2019-04-18 10:31 ` ✗ Fi.CI.BAT: failure " Patchwork
  2019-04-25 12:26 ` [PATCH 00/11] drm/i915: adding state checker for gamma lut values Jani Nikula
  17 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2019-04-18 10:08 UTC (permalink / raw)
  To: Swati Sharma; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: adding state checker for gamma lut values (rev6)
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 intel_get_color_config to create hw lut
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3616:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3617:16: warning: expression using sizeof(void)

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

Commit: drm/i915: Extract i9xx_get_color_config()
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:1237:15: warning: expression using sizeof(void)

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

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

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

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

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

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

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

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

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

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

* ✗ Fi.CI.BAT: failure for drm/i915: adding state checker for gamma lut values (rev6)
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
                   ` (15 preceding siblings ...)
  2019-04-18 10:08 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2019-04-18 10:31 ` Patchwork
  2019-04-23 10:59   ` Arkadiusz Hiler
  2019-04-25 12:26 ` [PATCH 00/11] drm/i915: adding state checker for gamma lut values Jani Nikula
  17 siblings, 1 reply; 23+ messages in thread
From: Patchwork @ 2019-04-18 10:31 UTC (permalink / raw)
  To: Swati Sharma; +Cc: intel-gfx

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_5952 -> Patchwork_12827
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://patchwork.freedesktop.org/api/1.0/series/58039/revisions/6/mbox/

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@runner@aborted:
    - fi-ilk-650:         NOTRUN -> FAIL
    - fi-pnv-d510:        NOTRUN -> FAIL
    - fi-bdw-gvtdvm:      NOTRUN -> FAIL
    - fi-hsw-peppy:       NOTRUN -> FAIL
    - fi-gdg-551:         NOTRUN -> FAIL
    - fi-snb-2520m:       NOTRUN -> FAIL
    - fi-hsw-4770:        NOTRUN -> FAIL
    - fi-bxt-j4205:       NOTRUN -> FAIL
    - fi-whl-u:           NOTRUN -> FAIL
    - fi-icl-u3:          NOTRUN -> FAIL
    - fi-ivb-3770:        NOTRUN -> FAIL
    - fi-bxt-dsi:         NOTRUN -> FAIL
    - fi-byt-j1900:       NOTRUN -> FAIL
    - fi-icl-y:           NOTRUN -> FAIL
    - fi-blb-e6850:       NOTRUN -> FAIL
    - fi-bsw-kefka:       NOTRUN -> FAIL
    - fi-hsw-4770r:       NOTRUN -> FAIL
    - fi-byt-clapper:     NOTRUN -> FAIL
    - fi-bdw-5557u:       NOTRUN -> FAIL
    - fi-bwr-2160:        NOTRUN -> FAIL
    - fi-elk-e7500:       NOTRUN -> FAIL

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

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

### IGT changes ###

#### Issues hit ####

  * igt@runner@aborted:
    - fi-cfl-8109u:       NOTRUN -> FAIL [k.org#202107] / [k.org#202109]
    - fi-glk-dsi:         NOTRUN -> FAIL [k.org#202321]
    - fi-skl-iommu:       NOTRUN -> FAIL [fdo#104108]
    - fi-cfl-guc:         NOTRUN -> FAIL [k.org#202107] / [k.org#202109]
    - fi-skl-guc:         NOTRUN -> FAIL [fdo#104108]
    - fi-skl-6700k2:      NOTRUN -> FAIL [fdo#104108]
    - fi-kbl-x1275:       NOTRUN -> FAIL [fdo#108903] / [fdo#108904] / [fdo#108905]
    - fi-cfl-8700k:       NOTRUN -> FAIL [k.org#202107] / [k.org#202109]
    - fi-skl-6600u:       NOTRUN -> FAIL [fdo#104108]
    - fi-skl-lmem:        NOTRUN -> FAIL [fdo#104108]
    - fi-kbl-r:           NOTRUN -> FAIL [fdo#108903] / [fdo#108904] / [fdo#108905]
    - fi-skl-6770hq:      NOTRUN -> FAIL [fdo#104108]
    - fi-skl-gvtdvm:      NOTRUN -> FAIL [fdo#104108]
    - fi-snb-2600:        NOTRUN -> FAIL [fdo#108929]
    - fi-skl-6260u:       NOTRUN -> FAIL [fdo#104108]

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

  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#108903]: https://bugs.freedesktop.org/show_bug.cgi?id=108903
  [fdo#108904]: https://bugs.freedesktop.org/show_bug.cgi?id=108904
  [fdo#108905]: https://bugs.freedesktop.org/show_bug.cgi?id=108905
  [fdo#108929]: https://bugs.freedesktop.org/show_bug.cgi?id=108929
  [fdo#109593]: https://bugs.freedesktop.org/show_bug.cgi?id=109593
  [k.org#202107]: https://bugzilla.kernel.org/show_bug.cgi?id=202107
  [k.org#202109]: https://bugzilla.kernel.org/show_bug.cgi?id=202109
  [k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321
  [k.org#202973]: https://bugzilla.kernel.org/show_bug.cgi?id=202973


Participating hosts (44 -> 43)
------------------------------

  Additional (5): fi-hsw-peppy fi-icl-u2 fi-byt-clapper fi-bsw-kefka fi-icl-dsi 
  Missing    (6): fi-ilk-m540 fi-bsw-n3050 fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


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

    * Linux: CI_DRM_5952 -> Patchwork_12827

  CI_DRM_5952: 65305a057be0e155321a0765a3a24115063f3a32 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4956: 1d921615b0b706f25c856aa0eb096f274380c199 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12827: db98bed6746b82037b4aaf68107189ac053aa820 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

db98bed6746b drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma lut values
b880360402ee drm/i915: Extract ilk_get_color_config()
17c09be650d6 drm/i915: Extract ivb_get_color_config()
fa8d86057b5c drm/i915: Extract bdw_get_color_config()
072235c17597 drm/i915: Extract glk_get_color_config()
3de95a9fd6c4 drm/i915: Extract icl_get_color_config()
d60da8743997 drm/i915: Extract i965_get_color_config()
5d9703416dc0 drm/i915: Extract cherryview_get_color_config()
c4668870f66f drm/i915: Extract i9xx_get_color_config()
0bfa41d5f0d7 drm/i915: Enable intel_color_get_config()
4912527e1aba drm/i915: Introduce vfunc intel_get_color_config to create hw lut

== Logs ==

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

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

* Re: ✗ Fi.CI.BAT: failure for drm/i915: adding state checker for gamma lut values (rev6)
  2019-04-18 10:31 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2019-04-23 10:59   ` Arkadiusz Hiler
  0 siblings, 0 replies; 23+ messages in thread
From: Arkadiusz Hiler @ 2019-04-23 10:59 UTC (permalink / raw)
  To: intel-gfx; +Cc: Lakshmi

On Thu, Apr 18, 2019 at 10:31:32AM +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: adding state checker for gamma lut values (rev6)
> URL   : https://patchwork.freedesktop.org/series/58039/
> State : failure


Hey,

This series is a rerun, which means that someone went to patchwork web
interface and clicked on "test this series again". This should be used
only when you thing there is something seriously wrong with CI results.

If you have something that looks like a false positive (e.g. a failure
in tests that your change should not affect see below).

> == Summary ==
> 
> CI Bug Log - changes from CI_DRM_5952 -> Patchwork_12827
> ====================================================
> 
> Summary
> -------
> 
>   **FAILURE**
> 
>   Serious unknown changes coming with Patchwork_12827 absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_12827, please notify your bug team to allow them
>   to document this new failure mode, which will reduce false positives in CI.
> 
>   External URL: https://patchwork.freedesktop.org/api/1.0/series/58039/revisions/6/mbox/

This is the relevant part about false positives. Just provide some
explanation why you think this is a false positive and CC:
Martin <martin.peres@linux.intel.com> and/or
Lakshmi <lakshminarayana.vudum@intel.com>

> Possible new issues
> -------------------
> 
>   Here are the unknown changes that may have been introduced in Patchwork_12827:
> 
> ### IGT changes ###
> 
> #### Possible regressions ####
> 
>   * igt@runner@aborted:
>     - fi-ilk-650:         NOTRUN -> FAIL
>     - fi-pnv-d510:        NOTRUN -> FAIL
>     - fi-bdw-gvtdvm:      NOTRUN -> FAIL
>     - fi-hsw-peppy:       NOTRUN -> FAIL
>     - fi-gdg-551:         NOTRUN -> FAIL
>     - fi-snb-2520m:       NOTRUN -> FAIL
>     - fi-hsw-4770:        NOTRUN -> FAIL
>     - fi-bxt-j4205:       NOTRUN -> FAIL
>     - fi-whl-u:           NOTRUN -> FAIL
>     - fi-icl-u3:          NOTRUN -> FAIL
>     - fi-ivb-3770:        NOTRUN -> FAIL
>     - fi-bxt-dsi:         NOTRUN -> FAIL
>     - fi-byt-j1900:       NOTRUN -> FAIL
>     - fi-icl-y:           NOTRUN -> FAIL
>     - fi-blb-e6850:       NOTRUN -> FAIL
>     - fi-bsw-kefka:       NOTRUN -> FAIL
>     - fi-hsw-4770r:       NOTRUN -> FAIL
>     - fi-byt-clapper:     NOTRUN -> FAIL
>     - fi-bdw-5557u:       NOTRUN -> FAIL
>     - fi-bwr-2160:        NOTRUN -> FAIL
>     - fi-elk-e7500:       NOTRUN -> FAIL

This looks like a real issue though. Seems like igt_runner has aborted
execution on almost all the machines due to hitting a WARN.

Let's see the logs:
https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12822/fi-cfl-8700k/igt@runner@aborted.html
----------------------------------------------------------
Aborting.
Previous test: nothing
Next test: core_auth (basic-auth)

Kernel badly tainted (0x200) (check dmesg for details):
	(0x200) TAINT_WARN: WARN_ON has happened.
----------------------------------------------------------

(You can get them through patchwork -> see full logs -> red cell in the
igt@runner@aborted row, on a machcine that has not aborted prior to that.)

This tells us that the kernel was tainted in a way that may cause
unexpeted behavior if testing would coninued (TAINT_WARN), so we abort.

Here it happned before we even run a single tests ("Previous test:
nothing"), so we have to look for the result in the boot dmesg. Link to
boot log is on top of the page with the result (boot0).

https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_12822/fi-cfl-8700k/boot0.log
----------------------------------------------------------
<4>[    3.842910] ------------[ cut here ]------------
<4>[    3.842911] pipe state doesn't match!
<4>[    3.842952] WARNING: CPU: 3 PID: 224 at drivers/gpu/drm/i915/intel_display.c:12700 intel_atomic_commit_tail+0x127d/0x12e0 [i915]
<4>[    3.842953] Modules linked in: i915 x86_pkg_temp_thermal mei_hdcp coretemp snd_hda_intel crct10dif_pclmul snd_hda_codec snd_hwdep snd_hda_core crc32_pclmul e1000e snd_pcm mei_me ghash_clmulni_intel mei ptp pps_core prime_numbers
<4>[    3.842961] CPU: 3 PID: 224 Comm: kworker/u24:7 Not tainted 5.1.0-rc5-CI-Patchwork_12822+ #1
<4>[    3.842962] Hardware name: Micro-Star International Co., Ltd. MS-7B54/Z370M MORTAR (MS-7B54), BIOS 1.00 10/31/2017
<4>[    3.842964] Workqueue: events_unbound async_run_entry_fn
<4>[    3.842993] RIP: 0010:intel_atomic_commit_tail+0x127d/0x12e0 [i915]
<4>[    3.842995] Code: 45 0f b6 84 24 d2 07 00 00 e8 3f 36 3d e1 5e 5f e9 80 fa ff ff e8 d3 8d e2 e0 0f 0b 0f b6 0c 24 e9 42 fc ff ff e8 c3 8d e2 e0 <0f> 0b e9 d2 f3 ff ff e8 b7 8d e2 e0 0f 0b 49 8b 44 24 50 e9 ae fc
<4>[    3.842996] RSP: 0018:ffffc90001aa7998 EFLAGS: 00010282
<4>[    3.842997] RAX: 0000000000000000 RBX: ffff888253eb92a8 RCX: 0000000000000000
<4>[    3.842998] RDX: 0000000000000007 RSI: ffff88826274d9f8 RDI: 00000000ffffffff
<4>[    3.842998] RBP: ffff888255fe9158 R08: 0000000038571494 R09: 0000000000000000
<4>[    3.842999] R10: ffff888255fe9450 R11: 0000000000000000 R12: ffff8882524f5698
<4>[    3.843000] R13: ffff888252880758 R14: ffff888252880000 R15: ffff888252880750
<4>[    3.843001] FS:  0000000000000000(0000) GS:ffff8882668c0000(0000) knlGS:0000000000000000
<4>[    3.843002] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
<4>[    3.843003] CR2: 00007ffe816c3ff8 CR3: 0000000005214003 CR4: 00000000003606e0
<4>[    3.843004] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
<4>[    3.843004] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
<4>[    3.843005] Call Trace:
<4>[    3.843042]  intel_atomic_commit+0x240/0x2e0 [i915]
<4>[    3.843046]  restore_fbdev_mode_atomic+0x1da/0x1f0
<4>[    3.843054]  drm_fb_helper_restore_fbdev_mode_unlocked+0x42/0x90
<4>[    3.843056]  drm_fb_helper_set_par+0x24/0x50
<4>[    3.843087]  intel_fbdev_set_par+0x11/0x40 [i915]
<4>[    3.843089]  fbcon_init+0x45e/0x620
<4>[    3.843094]  visual_init+0xc9/0x130
<4>[    3.843096]  do_bind_con_driver+0x1f5/0x420
<4>[    3.843101]  do_take_over_console+0x71/0x190
<4>[    3.843104]  do_fbcon_takeover+0x53/0xb0
<4>[    3.843105]  notifier_call_chain+0x34/0x90
<4>[    3.843109]  blocking_notifier_call_chain+0x3f/0x60
<4>[    3.843110]  ? lock_fb_info+0x13/0x40
<4>[    3.843112]  register_framebuffer+0x2a5/0x360
<4>[    3.843119]  __drm_fb_helper_initial_config_and_unlock+0x2d4/0x560
<4>[    3.843152]  intel_fbdev_initial_config+0xf/0x20 [i915]
<4>[    3.843154]  async_run_entry_fn+0x34/0x160
<4>[    3.843156]  process_one_work+0x245/0x610
<4>[    3.843162]  worker_thread+0x37/0x380
<4>[    3.843164]  ? process_one_work+0x610/0x610
<4>[    3.843166]  kthread+0x119/0x130
<4>[    3.843168]  ? kthread_park+0x80/0x80
<4>[    3.843171]  ret_from_fork+0x3a/0x50
<4>[    3.843178] irq event stamp: 18642
<4>[    3.843181] hardirqs last  enabled at (18641): [<ffffffff811268c2>] vprintk_emit+0x302/0x320
<4>[    3.843183] hardirqs last disabled at (18642): [<ffffffff810019b0>] trace_hardirqs_off_thunk+0x1a/0x1c
<4>[    3.843184] softirqs last  enabled at (18480): [<ffffffff81c0033a>] __do_softirq+0x33a/0x4b9
<4>[    3.843186] softirqs last disabled at (18469): [<ffffffff810b5389>] irq_exit+0xa9/0xc0
<4>[    3.843216] WARNING: CPU: 3 PID: 224 at drivers/gpu/drm/i915/intel_display.c:12700 intel_atomic_commit_tail+0x127d/0x12e0 [i915]
<4>[    3.843217] ---[ end trace df64dd148cbc195e ]---
----------------------------------------------------------

So this is the reason why all the machines aborted so early.

I hope this helps,
Arek

PS. There is work being done to make those logs more easily accessible
tied to a test, but it's not a trivial problem to solve, so it will take
time.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 02/11] [v4] drm/i915: Enable intel_color_get_config()
  2019-04-17 14:03 ` [PATCH 02/11] [v4] drm/i915: Enable intel_color_get_config() Swati Sharma
@ 2019-04-23 15:57   ` Ville Syrjälä
  0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2019-04-23 15:57 UTC (permalink / raw)
  To: Swati Sharma; +Cc: jani.nikula, daniel.vetter, intel-gfx, ankit.k.nautiyal

On Wed, Apr 17, 2019 at 07:33:24PM +0530, Swati Sharma wrote:
> 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]
> 
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f29a348..0715b4e 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8275,6 +8275,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;
> @@ -9348,6 +9349,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;
> @@ -10011,6 +10013,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  			pipe_config->csc_enable = true;
>  	} else {
>  		i9xx_get_pipe_color_config(pipe_config);
> +		intel_color_get_config(pipe_config);

That's not the right place for it if you want to cover all the
platforms.

>  	}
>  
>  	power_domain = POWER_DOMAIN_PIPE_PANEL_FITTER(crtc->pipe);
> -- 
> 1.9.1

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

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

* Re: [PATCH 03/11] [v4] drm/i915: Extract i9xx_get_color_config()
  2019-04-17 14:03 ` [PATCH 03/11] [v4] drm/i915: Extract i9xx_get_color_config() Swati Sharma
@ 2019-04-23 16:13   ` Ville Syrjälä
  0 siblings, 0 replies; 23+ messages in thread
From: Ville Syrjälä @ 2019-04-23 16:13 UTC (permalink / raw)
  To: Swati Sharma; +Cc: jani.nikula, daniel.vetter, intel-gfx, ankit.k.nautiyal

On Wed, Apr 17, 2019 at 07:33:25PM +0530, Swati Sharma wrote:
> v4: -No need to initialize *blob [Jani]
>     -Removed right shifts [Jani]
>     -Dropped dev local var [Jani]
> 
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h    |  3 +++
>  drivers/gpu/drm/i915/intel_color.c | 50 ++++++++++++++++++++++++++++++++++++++
>  2 files changed, 53 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 9c206e8..8f2ae8a 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7175,6 +7175,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 309f714..33223e0 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -1229,6 +1229,55 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
>  	return 0;
>  }
>  
> +/* convert hw value with given bit_precision to lut property val */
> +static u32 intel_color_lut_pack(u32 val, u32 bit_precision)
> +{
> +	u32 max = 0xffff >> (16 - bit_precision);
> +
> +	val = clamp_val(val, 0, max);
> +
> +	if (bit_precision < 16)
> +		val <<= 16 - bit_precision;
> +
> +	return val;
> +}
> +
> +static void i9xx_get_config_internal(struct intel_crtc_state *crtc_state)

The naming of these functions isn't particularly descriptive. I would
probably go for eg. i9xx_read_lut_8(), chv_read_cgm_gamma_lut(), etc.
AFAICS the series is also missing readout for degamma LUTs entirely.

We could also make these functions just return the blob instead of
assigning it internally. That way we can have the higher level
.get_color_config() implementation assign each gamma/degamma lut
apporpriately. Eg. might looks something like this for bdw:

bdw_get_color_config()
{
	if (gamma_mode == 8bit) {
		crtc_state->gamma_lut = i9xx_read_lut_8();
	} else if (gamma_mode == split) {
		crtc_state->degamma_lut = bdw_read_lut_10(SPLIT | 0);
		crtc_state->gamma_lut = bdw_read_lut_10(SPLIT | 512);
	} else if (csc_mode == gamma_before_csc) {
		crtc_state->degamma_lut = bdw_read_lut_10(0);
	} else {
		crtc_state->gamma_lut = bdw_read_lut_10(0);
	}
}

The tricky part will be to figure out how the gamma vs. degamma gets
assigned and/or checked since the choice depends on the presence of
the ctm, rgb limit range, and rgb->ycbcr conversion.

Oh, and most (all maybe?) patches seem to be missing a proper commit
message.

> +{
> +	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;
> +
> +	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);
> +	}
> +
> +	crtc_state->base.gamma_lut = blob;
> +}
> +
> +static void i9xx_get_color_config(struct intel_crtc_state *crtc_state)
> +{
> +	i9xx_get_config_internal(crtc_state);
> +}
> +
>  void intel_color_init(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -1249,6 +1298,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.get_color_config = i9xx_get_color_config;
>  		}
>  	} else {
>  		if (INTEL_GEN(dev_priv) >= 11)
> -- 
> 1.9.1

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

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

* Re: [PATCH 00/11] drm/i915: adding state checker for gamma lut values
  2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
                   ` (16 preceding siblings ...)
  2019-04-18 10:31 ` ✗ Fi.CI.BAT: failure " Patchwork
@ 2019-04-25 12:26 ` Jani Nikula
  17 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2019-04-25 12:26 UTC (permalink / raw)
  To: Swati Sharma, intel-gfx; +Cc: daniel.vetter, ankit.k.nautiyal

On Wed, 17 Apr 2019, Swati Sharma <swati2.sharma@intel.com> wrote:
> Thanks to Jani N, Matt and Ville for the review comments. Hopefully
> I have addressed all the current review comments and ready to receive
> more :)

Sorry to take for so long to notice... but all of the patches need a
proper commit message. It doesn't have to be long if the change is
simple, but there has to be more than just the changelog.

BR,
Jani.



>
> In this patch series, added state checker to validate gamma_lut values. This
> reads hardware state, and compares the originally requested
> state to the state read from hardware.
>
> v1: -Implementation done for legacy platforms (removed all the placeholders) (Jani)
>     -Added inverse function of drm_color_lut_extract to convert hardware
>      read values back to user values (code written by Jani)
>     -Renamed get_config() to color_config() (Jani)
>     -Placed all platform specific shifts and masks in i915_reg.h (Jani)
>     -Renamed i9xx_get_config to i9xx_color_config and all related
>      functions (Jani)
>     -Removed debug logs from compare function (Jani)
>     -Renamed intel_compare_blob to intel_compare_lut and added platform specific
>      bit precision of the readout into the function (Jani)
>     -Renamed macro PIPE_CONF_CHECK_BLOB to PIPE_CONF_CHECK_COLOR_LUT (Jani)
>     -Added check if blobs can be NULL (Jani)
>     -Added function in intel_color.c that returns the bit precision (Jani),
>      didn't add in device info since its gonna die soon (Ville)
>
> v2: -Moved intel_compare_lut() from intel_display.c to intel_color.c (Jani)
>     -Changed color_config() func names to get_color_config() and same
>      for gamma func too (Jani)
>     -Removed blank line in i915_reg.h and aligned MASK with their
>      register name (Jani)
>     -Mask definition code indented and defined using REG_GENMASK() and
>      used using REG_FIELD_GET() (Jani)
>     -Made bit_precision func inline (Jani/Matt)
>     -Assigned bit_precision according to GAMMA_MODE (Matt/Ville)
>     -Changed IS_ERR(blob) condition to (!blob) (Jani)
>     -Rearranged blob check and lut_size conditions (Jani)
>     -Used inline function for the comparison (Jani)
>     -Separated the get config part from the state checker part to
>      another patch (Jani)
>     -Retained "internal" i9xx_internal_gamma_config for consistency
>      (Matt)
>     -Removed crtc_state_is_legacy_gamma check and replaced with
>      GAMMA_MODE (Matt)
>     -Created whole platform specific patch series as submitted by Ville for
>      clean up intel_color_check()
>     -Rebased on top of Ville's changes
>
> v3: -Rebase
>
> v4: -Renamed intel_get_color_config to intel_color_get_config (Jani)
>     -Wrapped get_color_config() (Jani)
>     -Added the user early on such that support for get_color_config()
>      can be added platform by platform incrementally (Jani)
>     -Few changes in get_config_func (Jani)
>     -Renamed intel_compare_color_lut() to intel_color_lut_equal() (Jani)
>     -Corrected smatch warn "variable dereferenced before check" (Dan Carpenter)
>
> TODO:  -Matt suggested to rename get_gamma_config to get_gamma_lut or
>         i9xx_read_out_gamma_lut, Ville also named functions like i9xx_read_lut_8(),
>         i9xx_read_lut_10(). Should we follow this?
>        -Answering Matt's query regarding intension to read and compare degamma/ctm".
>         Degamma will be my first priority after this and later ctm.
>         Should we have combined func or separate func for gamma/degamma val?
>        -Add separate func to log errors. I am not sure, what needs to be printed here?
>         Mismatched RGB value? 
>        -Will make changes in next rev for changes made by Ville for cherryview_load_luts()
>
>
> Swati Sharma (11):
>   [v4] drm/i915: Introduce vfunc intel_get_color_config to create hw lut
>   [v4] drm/i915: Enable intel_color_get_config()
>   [v4] drm/i915: Extract i9xx_get_color_config()
>   [v4] drm/i915: Extract cherryview_get_color_config()
>   [v4] drm/i915: Extract i965_get_color_config()
>   [v4] drm/i915: Extract icl_get_color_config()
>   [v4] drm/i915: Extract glk_get_color_config()
>   [v4] drm/i915: Extract bdw_get_color_config()
>   [v4] drm/i915: Extract ivb_get_color_config()
>   [v4] drm/i915: Extract ilk_get_color_config()
>   [v4] drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma
>     lut values
>
>  drivers/gpu/drm/i915/i915_drv.h      |   1 +
>  drivers/gpu/drm/i915/i915_reg.h      |  15 ++
>  drivers/gpu/drm/i915/intel_color.c   | 343 ++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/i915/intel_color.h   |   8 +
>  drivers/gpu/drm/i915/intel_display.c |  13 ++
>  5 files changed, 375 insertions(+), 5 deletions(-)

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

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

* Re: [PATCH 11/11] [v4] drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma lut values
  2019-04-17 14:03 ` [PATCH 11/11] [v4] drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma lut values Swati Sharma
@ 2019-04-25 12:35   ` Jani Nikula
  0 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2019-04-25 12:35 UTC (permalink / raw)
  To: Swati Sharma, intel-gfx; +Cc: daniel.vetter, ankit.k.nautiyal

On Wed, 17 Apr 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)
>
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c   | 50 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_color.h   |  7 +++++
>  drivers/gpu/drm/i915/intel_display.c | 10 ++++++++
>  3 files changed, 67 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 731cb97..592c328 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -1495,6 +1495,56 @@ static void ilk_get_color_config(struct intel_crtc_state *crtc_state)
>  		ilk_get_gamma_config(crtc_state);
>  }
>  
> +static inline bool gamma_err_check(struct drm_color_lut *sw_lut, struct drm_color_lut *hw_lut, u32 err)
> +{
> +	 return ((abs((long)hw_lut->red - sw_lut->red)) >= err) ||
> +	        ((abs((long)hw_lut->blue - sw_lut->blue)) >= err) ||
> +	        ((abs((long)hw_lut->green - sw_lut->green)) >= err);
> +}
> +
> +bool intel_color_lut_equal(struct drm_property_blob *blob1,
> +			   struct drm_property_blob *blob2,
> +			   u32 gamma_mode)
> +{
> +	struct drm_color_lut *sw_lut, *hw_lut;
> +	int sw_lut_size, hw_lut_size, i;
> +	u32 bit_precision, err;
> +
> +	if (!blob1 || !blob2)
> +		return false;

(!blob1 && !blob2) must return true, right?

> +
> +	sw_lut = blob1->data;
> +	hw_lut = blob2->data;
> +
> +	switch(gamma_mode) {
> +	default:
> +	case GAMMA_MODE_MODE_8BIT:
> +		bit_precision = 8;
> +		break;
> +	case GAMMA_MODE_MODE_10BIT:
> +		bit_precision = 10;
> +		break;
> +	case GAMMA_MODE_MODE_12BIT:
> +		bit_precision = 12;
> +		break;
> +	}
> +
> +	err = 0xffff >> bit_precision;
> +
> +	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;
> +
> +	for (i = 0; i < sw_lut_size; i++) {
> +		 if (!gamma_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..2ae615e 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 gamma_mode);
>  
>  #endif /* __INTEL_COLOR_H__ */
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 0715b4e..bb1ccd7 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12236,6 +12236,14 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>  	} \
>  } while (0)
>  
> +#define PIPE_CONF_CHECK_COLOR_LUT(name, gamma_mode) do { \
> +	if (!intel_color_lut_equal(current_config->name, pipe_config->name, gamma_mode)) { \
> +		pipe_config_err(adjust, __stringify(name), \
> +				"hw_state doesn't match sw_state\n"); \
> +		ret = false; \
> +	} \
> +} while (0)
> +
>  #define PIPE_CONF_QUIRK(quirk) \
>  	((current_config->quirks | pipe_config->quirks) & (quirk))
>  
> @@ -12379,6 +12387,8 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
>  	PIPE_CONF_CHECK_INFOFRAME(spd);
>  	PIPE_CONF_CHECK_INFOFRAME(hdmi);
>  
> +	PIPE_CONF_CHECK_COLOR_LUT(base.gamma_lut, pipe_config->gamma_mode);
> +

Hmm, we check gamma_mode and gamma_enable within if (!adjust), I suppose
this should be there too.

>  #undef PIPE_CONF_CHECK_X
>  #undef PIPE_CONF_CHECK_I
>  #undef PIPE_CONF_CHECK_BOOL

Please add

#undef PIPE_CONF_CHECK_COLOR_LUT


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

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

end of thread, other threads:[~2019-04-25 12:33 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-17 14:03 [PATCH 00/11] drm/i915: adding state checker for gamma lut values Swati Sharma
2019-04-17 14:03 ` [PATCH 01/11] [v4] drm/i915: Introduce vfunc intel_get_color_config to create hw lut Swati Sharma
2019-04-17 14:03 ` [PATCH 02/11] [v4] drm/i915: Enable intel_color_get_config() Swati Sharma
2019-04-23 15:57   ` Ville Syrjälä
2019-04-17 14:03 ` [PATCH 03/11] [v4] drm/i915: Extract i9xx_get_color_config() Swati Sharma
2019-04-23 16:13   ` Ville Syrjälä
2019-04-17 14:03 ` [PATCH 04/11] [v4] drm/i915: Extract cherryview_get_color_config() Swati Sharma
2019-04-17 14:03 ` [PATCH 05/11] [v4] drm/i915: Extract i965_get_color_config() Swati Sharma
2019-04-17 14:03 ` [PATCH 06/11] [v4] drm/i915: Extract icl_get_color_config() Swati Sharma
2019-04-17 14:03 ` [PATCH 07/11] [v4] drm/i915: Extract glk_get_color_config() Swati Sharma
2019-04-17 14:03 ` [PATCH 08/11] [v4] drm/i915: Extract bdw_get_color_config() Swati Sharma
2019-04-17 14:03 ` [PATCH 09/11] [v4] drm/i915: Extract ivb_get_color_config() Swati Sharma
2019-04-17 14:03 ` [PATCH 10/11] [v4] drm/i915: Extract ilk_get_color_config() Swati Sharma
2019-04-17 14:03 ` [PATCH 11/11] [v4] drm/i915: Add intel_color_lut_equal() to compare hw and sw gamma lut values Swati Sharma
2019-04-25 12:35   ` Jani Nikula
2019-04-17 14:23 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: adding state checker for gamma lut values (rev5) Patchwork
2019-04-17 14:28 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-04-17 14:37 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-04-18 10:03 ` ✗ Fi.CI.CHECKPATCH: warning for drm/i915: adding state checker for gamma lut values (rev6) Patchwork
2019-04-18 10:08 ` ✗ Fi.CI.SPARSE: " Patchwork
2019-04-18 10:31 ` ✗ Fi.CI.BAT: failure " Patchwork
2019-04-23 10:59   ` Arkadiusz Hiler
2019-04-25 12:26 ` [PATCH 00/11] drm/i915: adding state checker for gamma lut values Jani Nikula

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