[v8][PATCH 00/10] drm/i915: adding state checker for gamma lut value

[v8][PATCH 00/10] drm/i915: adding state checker for gamma lut value

[Swati Sharma]

In this patch series, added state checker to validate gamma (8BIT and 10BIT).
This reads hardware state, and compares the originally requested state(s/w) to the
state read from the hardware.
This is done for legacy, i965, ilk, glk and their variant platforms. 

Intentionally, excluded bdw and ivb since they have spilt gamma mode; for 
which degamma read outs are required (which I think shouldn't be included in this
patch series). Will include after degamma state checker is completed.

v1: -Implementation done for legacy platforms
     (removed all the placeholders) (Jani)
v2: -Restructured code and created platform specific patch series for 
     gamma validation
v3: -Rebase
v4: -Minor changes-function name changes mainly
v5: -Added degamma validation (Ville)
v6: -Removed degamma changes, debugging was becoming difficult
    -Added function to assign bit_precision for gamma/degamma
     lut values /platform
    -Added debug info into intel_dump_pipe_config() (Jani)
v7: -Added platform specific functions to compute gamma bit precision
     on the basis of GAMMA_MODE (Ville)
    -Corrected checkpatch warnings
v8: -Restructured code
    -Removed bdw and ivb platform state checker

Swati Sharma (10):
  drm/i915/display: Add func to get gamma bit precision
  drm/i915/display: Add debug log for color parameters
  drm/i915/display: Add func to compare hw/sw gamma lut
  drm/i915/display: Add macro to compare gamma hw/sw lut
  drm/i915/display: Extract i9xx_read_luts()
  drm/i91/display: Extract i965_read_luts()
  drm/i915/display: Extract chv_read_luts()
  drm/i915/display: Extract ilk_read_luts()
  drm/i915/display: Extract glk_read_luts()
  FOR_TESTING_ONLY: Print rgb values of hw and sw blobs

 drivers/gpu/drm/i915/display/intel_color.c   | 370 ++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_color.h   |   7 +
 drivers/gpu/drm/i915/display/intel_display.c |  34 +++
 drivers/gpu/drm/i915/i915_reg.h              |  15 ++
 4 files changed, 423 insertions(+), 3 deletions(-)

-- 
1.9.1

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

[v8][PATCH 01/10] drm/i915/display: Add func to get gamma bit precision

[Swati Sharma]

Each platform supports different gamma modes and each gamma mode
has different bit precision. Add func/platform to get bit precision
corresponding to gamma mode.

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

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 71a0201..d2c1297 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1371,6 +1371,85 @@ static int icl_color_check(struct intel_crtc_state *crtc_state)
 	return 0;
 }
 
+static int i9xx_gamma_precision(const struct intel_crtc_state *crtc_state)
+{
+	if (!crtc_state->gamma_enable)
+		return 0;
+
+	switch (crtc_state->gamma_mode) {
+	case GAMMA_MODE_MODE_8BIT:
+		return 8;
+	case GAMMA_MODE_MODE_10BIT:
+		return 16;
+	default:
+		MISSING_CASE(crtc_state->gamma_mode);
+		return 0;
+	}
+}
+
+static int chv_gamma_precision(const struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
+		return 10;
+	else
+		return i9xx_gamma_precision(crtc_state);
+}
+
+static int glk_gamma_precision(const struct intel_crtc_state *crtc_state)
+{
+	if (!crtc_state->gamma_enable)
+		return 0;
+
+	switch (crtc_state->gamma_mode) {
+	case GAMMA_MODE_MODE_8BIT:
+		return 8;
+	case GAMMA_MODE_MODE_10BIT:
+		return 10;
+	default:
+		MISSING_CASE(crtc_state->gamma_mode);
+		return 0;
+	}
+}
+
+static int ilk_gamma_precision(const struct intel_crtc_state *crtc_state)
+{
+	if (!crtc_state->gamma_enable)
+		return 0;
+
+	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
+		return 0;
+
+	switch (crtc_state->gamma_mode) {
+	case GAMMA_MODE_MODE_8BIT:
+		return 8;
+	case GAMMA_MODE_MODE_10BIT:
+		return 10;
+	default:
+		MISSING_CASE(crtc_state->gamma_mode);
+		return 0;
+	}
+}
+
+int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+
+	if (HAS_GMCH(dev_priv)) {
+		if (IS_CHERRYVIEW(dev_priv))
+			return chv_gamma_precision(crtc_state);
+		else
+			return i9xx_gamma_precision(crtc_state);
+	} else {
+		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+			return glk_gamma_precision(crtc_state);
+		else if (IS_IRONLAKE(dev_priv))
+			return ilk_gamma_precision(crtc_state);
+	}
+
+	return 0;
+}
+
 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/display/intel_color.h b/drivers/gpu/drm/i915/display/intel_color.h
index 057e8ac..0226d3a 100644
--- a/drivers/gpu/drm/i915/display/intel_color.h
+++ b/drivers/gpu/drm/i915/display/intel_color.h
@@ -14,5 +14,6 @@
 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);
+int intel_color_get_gamma_bit_precision(const 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

Re: [v8][PATCH 01/10] drm/i915/display: Add func to get gamma bit precision

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Swati2
>Sent: Monday, August 26, 2019 11:56 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Nikula, Jani <jani.nikula@intel.com>; Sharma, Shashank
><shashank.sharma@intel.com>; Manna, Animesh <animesh.manna@intel.com>;
>Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; daniel.vetter@ffwll.ch;
>ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; Sharma,
>Swati2 <swati2.sharma@intel.com>
>Subject: [v8][PATCH 01/10] drm/i915/display: Add func to get gamma bit precision
>
>Each platform supports different gamma modes and each gamma mode has different
>bit precision. Add func/platform to get bit precision corresponding to gamma mode.

I think it would be good to add some explanation as to why this is needed.
Also what all platforms are supported by this currently. 

>Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_color.c | 79 ++++++++++++++++++++++++++++++
>drivers/gpu/drm/i915/display/intel_color.h |  1 +
> 2 files changed, 80 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_color.c
>b/drivers/gpu/drm/i915/display/intel_color.c
>index 71a0201..d2c1297 100644
>--- a/drivers/gpu/drm/i915/display/intel_color.c
>+++ b/drivers/gpu/drm/i915/display/intel_color.c
>@@ -1371,6 +1371,85 @@ static int icl_color_check(struct intel_crtc_state
>*crtc_state)
> 	return 0;
> }
>
>+static int i9xx_gamma_precision(const struct intel_crtc_state
>+*crtc_state) {
>+	if (!crtc_state->gamma_enable)
>+		return 0;
>+
>+	switch (crtc_state->gamma_mode) {
>+	case GAMMA_MODE_MODE_8BIT:
>+		return 8;
>+	case GAMMA_MODE_MODE_10BIT:
>+		return 16;
>+	default:
>+		MISSING_CASE(crtc_state->gamma_mode);
>+		return 0;
>+	}
>+}
>+
>+static int chv_gamma_precision(const struct intel_crtc_state
>+*crtc_state) {
>+	if (crtc_state->cgm_mode & CGM_PIPE_MODE_GAMMA)
>+		return 10;
>+	else
>+		return i9xx_gamma_precision(crtc_state); }
>+
>+static int glk_gamma_precision(const struct intel_crtc_state
>+*crtc_state) {
>+	if (!crtc_state->gamma_enable)
>+		return 0;
>+
>+	switch (crtc_state->gamma_mode) {
>+	case GAMMA_MODE_MODE_8BIT:
>+		return 8;
>+	case GAMMA_MODE_MODE_10BIT:
>+		return 10;
>+	default:
>+		MISSING_CASE(crtc_state->gamma_mode);
>+		return 0;
>+	}
>+}
>+
>+static int ilk_gamma_precision(const struct intel_crtc_state
>+*crtc_state) {
>+	if (!crtc_state->gamma_enable)
>+		return 0;
>+
>+	if ((crtc_state->csc_mode & CSC_POSITION_BEFORE_GAMMA) == 0)
>+		return 0;
>+
>+	switch (crtc_state->gamma_mode) {
>+	case GAMMA_MODE_MODE_8BIT:
>+		return 8;
>+	case GAMMA_MODE_MODE_10BIT:
>+		return 10;
>+	default:
>+		MISSING_CASE(crtc_state->gamma_mode);
>+		return 0;
>+	}
>+}
>+
>+int intel_color_get_gamma_bit_precision(const struct intel_crtc_state
>+*crtc_state) {
>+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>+
>+	if (HAS_GMCH(dev_priv)) {
>+		if (IS_CHERRYVIEW(dev_priv))
>+			return chv_gamma_precision(crtc_state);
>+		else
>+			return i9xx_gamma_precision(crtc_state);
>+	} else {
>+		if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>+			return glk_gamma_precision(crtc_state);
>+		else if (IS_IRONLAKE(dev_priv))
>+			return ilk_gamma_precision(crtc_state);
>+	}
>+
>+	return 0;
>+}
>+
> 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/display/intel_color.h
>b/drivers/gpu/drm/i915/display/intel_color.h
>index 057e8ac..0226d3a 100644
>--- a/drivers/gpu/drm/i915/display/intel_color.h
>+++ b/drivers/gpu/drm/i915/display/intel_color.h
>@@ -14,5 +14,6 @@
> 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);
>+int intel_color_get_gamma_bit_precision(const 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

[v8][PATCH 02/10] drm/i915/display: Add debug log for color parameters

[Swati Sharma]

Add debug log for color related parameters like gamma_mode, gamma_enable,
csc_enable, etc inside intel_dump_pipe_config().

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index b51d1ce..ca88c70 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -12138,6 +12138,15 @@ static void intel_dump_pipe_config(const struct intel_crtc_state *pipe_config,
 
 	intel_dpll_dump_hw_state(dev_priv, &pipe_config->dpll_hw_state);
 
+	if (IS_CHERRYVIEW(dev_priv))
+		DRM_DEBUG_KMS("cgm_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
+			       pipe_config->cgm_mode, pipe_config->gamma_mode,
+			       pipe_config->gamma_enable, pipe_config->csc_enable);
+	else
+		DRM_DEBUG_KMS("csc_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
+			       pipe_config->csc_mode, pipe_config->gamma_mode,
+			       pipe_config->gamma_enable, pipe_config->csc_enable);
+
 dump_planes:
 	if (!state)
 		return;
-- 
1.9.1

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

Re: [v8][PATCH 02/10] drm/i915/display: Add debug log for color parameters

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Swati2
>Sent: Monday, August 26, 2019 11:56 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Nikula, Jani <jani.nikula@intel.com>; Sharma, Shashank
><shashank.sharma@intel.com>; Manna, Animesh <animesh.manna@intel.com>;
>Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; daniel.vetter@ffwll.ch;
>ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; Sharma,
>Swati2 <swati2.sharma@intel.com>
>Subject: [v8][PATCH 02/10] drm/i915/display: Add debug log for color parameters
>
>Add debug log for color related parameters like gamma_mode, gamma_enable,
>csc_enable, etc inside intel_dump_pipe_config().
>
>Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_display.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_display.c
>b/drivers/gpu/drm/i915/display/intel_display.c
>index b51d1ce..ca88c70 100644
>--- a/drivers/gpu/drm/i915/display/intel_display.c
>+++ b/drivers/gpu/drm/i915/display/intel_display.c
>@@ -12138,6 +12138,15 @@ static void intel_dump_pipe_config(const struct
>intel_crtc_state *pipe_config,
>
> 	intel_dpll_dump_hw_state(dev_priv, &pipe_config->dpll_hw_state);
>
>+	if (IS_CHERRYVIEW(dev_priv))
>+		DRM_DEBUG_KMS("cgm_mode:%d gamma_mode:%d
>gamma_enable:%d csc_enable:%d\n",
>+			       pipe_config->cgm_mode, pipe_config->gamma_mode,
>+			       pipe_config->gamma_enable, pipe_config->csc_enable);

Alignment seems to be off.

>+	else
>+		DRM_DEBUG_KMS("csc_mode:%d gamma_mode:%d
>gamma_enable:%d csc_enable:%d\n",
>+			       pipe_config->csc_mode, pipe_config->gamma_mode,
>+			       pipe_config->gamma_enable, pipe_config->csc_enable);

Same here.

>+
> dump_planes:
> 	if (!state)
> 		return;
>--
>1.9.1

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

[v8][PATCH 03/10] drm/i915/display: Add func to compare hw/sw gamma lut

[Swati Sharma]

Add func intel_color_lut_equal() to compare hw/sw gamma
lut values. Since hw/sw gamma lut sizes and lut enteries comparsion
will be different for different gamma modes, add gamma mode dependent
checks.

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

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index d2c1297..27727a1 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1450,6 +1450,77 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat
 	return 0;
 }
 
+static inline bool err_check(struct drm_color_lut *sw_lut,
+			     struct drm_color_lut *hw_lut, u32 err)
+{
+	return ((abs((long)hw_lut->red - sw_lut->red)) <= err) &&
+		((abs((long)hw_lut->blue - sw_lut->blue)) <= err) &&
+		((abs((long)hw_lut->green - sw_lut->green)) <= err);
+}
+
+static inline bool intel_color_lut_entry_equal(struct drm_color_lut *sw_lut,
+					       struct drm_color_lut *hw_lut,
+					       int hw_lut_size, u32 err)
+{
+	int i;
+
+	for (i = 0; i < hw_lut_size; i++) {
+		if (!err_check(&hw_lut[i], &sw_lut[i], err))
+			return false;
+	}
+
+	return true;
+}
+
+bool intel_color_lut_equal(struct drm_property_blob *blob1,
+			   struct drm_property_blob *blob2,
+			   u32 gamma_mode, u32 bit_precision)
+{
+	struct drm_color_lut *sw_lut, *hw_lut;
+	int sw_lut_size, hw_lut_size;
+	u32 err;
+
+	if (!blob1 != !blob2)
+		return false;
+
+	if (!blob1)
+		return true;
+
+	sw_lut_size = drm_color_lut_size(blob1);
+	hw_lut_size = drm_color_lut_size(blob2);
+
+	switch (gamma_mode) {
+	case GAMMA_MODE_MODE_8BIT:
+	case GAMMA_MODE_MODE_10BIT:
+		if (sw_lut_size != hw_lut_size)
+			return false;
+		else
+			break;
+	default:
+		MISSING_CASE(gamma_mode);
+			return false;
+	}
+
+	sw_lut = blob1->data;
+	hw_lut = blob2->data;
+
+	err = 0xffff >> bit_precision;
+
+	switch (gamma_mode) {
+	case GAMMA_MODE_MODE_8BIT:
+	case GAMMA_MODE_MODE_10BIT:
+		if (!intel_color_lut_entry_equal(sw_lut, hw_lut, hw_lut_size, err))
+				return false;
+		else
+				break;
+	default:
+		MISSING_CASE(gamma_mode);
+			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/display/intel_color.h b/drivers/gpu/drm/i915/display/intel_color.h
index 0226d3a..173727a 100644
--- a/drivers/gpu/drm/i915/display/intel_color.h
+++ b/drivers/gpu/drm/i915/display/intel_color.h
@@ -6,8 +6,11 @@
 #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);
@@ -15,5 +18,8 @@
 void intel_color_load_luts(const struct intel_crtc_state *crtc_state);
 void intel_color_get_config(struct intel_crtc_state *crtc_state);
 int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_state);
+bool intel_color_lut_equal(struct drm_property_blob *blob1,
+			   struct drm_property_blob *blob2,
+			   u32 gamma_mode, u32 bit_precision);
 
 #endif /* __INTEL_COLOR_H__ */
-- 
1.9.1

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

Re: [v8][PATCH 03/10] drm/i915/display: Add func to compare hw/sw gamma lut

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Swati2
>Sent: Monday, August 26, 2019 11:56 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Nikula, Jani <jani.nikula@intel.com>; Sharma, Shashank
><shashank.sharma@intel.com>; Manna, Animesh <animesh.manna@intel.com>;
>Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; daniel.vetter@ffwll.ch;
>ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; Sharma,
>Swati2 <swati2.sharma@intel.com>
>Subject: [v8][PATCH 03/10] drm/i915/display: Add func to compare hw/sw gamma lut
>
>Add func intel_color_lut_equal() to compare hw/sw gamma lut values. Since hw/sw
>gamma lut sizes and lut enteries comparsion will be different for different gamma

Typo here in entries and comparison.

>modes, add gamma mode dependent checks.

Please keep the revision history of patch as well. Also add what is changed from the
previous version, like limiting to gamma mode alone.

>Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_color.c | 71 ++++++++++++++++++++++++++++++
>drivers/gpu/drm/i915/display/intel_color.h |  6 +++
> 2 files changed, 77 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_color.c
>b/drivers/gpu/drm/i915/display/intel_color.c
>index d2c1297..27727a1 100644
>--- a/drivers/gpu/drm/i915/display/intel_color.c
>+++ b/drivers/gpu/drm/i915/display/intel_color.c
>@@ -1450,6 +1450,77 @@ int intel_color_get_gamma_bit_precision(const struct
>intel_crtc_state *crtc_stat
> 	return 0;
> }
>
>+static inline bool err_check(struct drm_color_lut *sw_lut,
>+			     struct drm_color_lut *hw_lut, u32 err) {
>+	return ((abs((long)hw_lut->red - sw_lut->red)) <= err) &&
>+		((abs((long)hw_lut->blue - sw_lut->blue)) <= err) &&
>+		((abs((long)hw_lut->green - sw_lut->green)) <= err); }
>+
>+static inline bool intel_color_lut_entry_equal(struct drm_color_lut *sw_lut,
>+					       struct drm_color_lut *hw_lut,
>+					       int hw_lut_size, u32 err)
>+{
>+	int i;
>+
>+	for (i = 0; i < hw_lut_size; i++) {
>+		if (!err_check(&hw_lut[i], &sw_lut[i], err))
>+			return false;
>+	}
>+
>+	return true;
>+}
>+
>+bool intel_color_lut_equal(struct drm_property_blob *blob1,
>+			   struct drm_property_blob *blob2,
>+			   u32 gamma_mode, u32 bit_precision) {
>+	struct drm_color_lut *sw_lut, *hw_lut;
>+	int sw_lut_size, hw_lut_size;
>+	u32 err;
>+
>+	if (!blob1 != !blob2)
>+		return false;
>+
>+	if (!blob1)
>+		return true;
>+
>+	sw_lut_size = drm_color_lut_size(blob1);
>+	hw_lut_size = drm_color_lut_size(blob2);
>+
>+	switch (gamma_mode) {
>+	case GAMMA_MODE_MODE_8BIT:
>+	case GAMMA_MODE_MODE_10BIT:
>+		if (sw_lut_size != hw_lut_size)
>+			return false;
>+		else

You may drop else, just simply put break.

>+			break;
>+	default:
>+		MISSING_CASE(gamma_mode);
>+			return false;
>+	}
>+
>+	sw_lut = blob1->data;
>+	hw_lut = blob2->data;
>+
>+	err = 0xffff >> bit_precision;
>+
>+	switch (gamma_mode) {
>+	case GAMMA_MODE_MODE_8BIT:
>+	case GAMMA_MODE_MODE_10BIT:
>+		if (!intel_color_lut_entry_equal(sw_lut, hw_lut, hw_lut_size, err))

Limit it within 80 characters.

>+				return false;
>+		else
>+				break;

Drop else here as well. Indentation also seems off.

>+	default:
>+		MISSING_CASE(gamma_mode);
>+			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/display/intel_color.h
>b/drivers/gpu/drm/i915/display/intel_color.h
>index 0226d3a..173727a 100644
>--- a/drivers/gpu/drm/i915/display/intel_color.h
>+++ b/drivers/gpu/drm/i915/display/intel_color.h
>@@ -6,8 +6,11 @@
> #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); @@ -15,5 +18,8 @@  void intel_color_load_luts(const
>struct intel_crtc_state *crtc_state);  void intel_color_get_config(struct
>intel_crtc_state *crtc_state);  int intel_color_get_gamma_bit_precision(const struct
>intel_crtc_state *crtc_state);
>+bool intel_color_lut_equal(struct drm_property_blob *blob1,
>+			   struct drm_property_blob *blob2,
>+			   u32 gamma_mode, u32 bit_precision);
>
> #endif /* __INTEL_COLOR_H__ */
>--
>1.9.1

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

[v8][PATCH 04/10] drm/i915/display: Add macro to compare gamma hw/sw lut

[Swati Sharma]

Add macro to compare hw/sw gamma lut values. First need to
check whether hw/sw gamma mode matches or not. If not
no need to compare lut values, if matches then only compare
lut entries.

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

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index ca88c70..63b7ad7 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -12529,6 +12529,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 {
 	struct drm_i915_private *dev_priv = to_i915(current_config->base.crtc->dev);
 	bool ret = true;
+	u32 bp_gamma = 0;
 	bool fixup_inherited = fastset &&
 		(current_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED) &&
 		!(pipe_config->base.mode.private_flags & I915_MODE_FLAG_INHERITED);
@@ -12680,6 +12681,24 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 	} \
 } while (0)
 
+#define PIPE_CONF_CHECK_COLOR_LUT(name1, name2, bit_precision) do { \
+	if (current_config->name1 != pipe_config->name1) { \
+		pipe_config_mismatch(fastset, __stringify(name1), \
+				"(expected %i, found %i, won't compare lut values)\n", \
+				current_config->name1, \
+				pipe_config->name1); \
+		ret = false;\
+	} else { \
+		if (!intel_color_lut_equal(current_config->name2, \
+					pipe_config->name2, pipe_config->name1, \
+					bit_precision)) { \
+			pipe_config_mismatch(fastset, __stringify(name2), \
+					"hw_state doesn't match sw_state\n"); \
+			ret = false; \
+		} \
+	} \
+} while (0)
+
 #define PIPE_CONF_QUIRK(quirk) \
 	((current_config->quirks | pipe_config->quirks) & (quirk))
 
@@ -12775,6 +12794,11 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 			PIPE_CONF_CHECK_X(csc_mode);
 		PIPE_CONF_CHECK_BOOL(gamma_enable);
 		PIPE_CONF_CHECK_BOOL(csc_enable);
+
+		bp_gamma = intel_color_get_gamma_bit_precision(pipe_config);
+		if (bp_gamma)
+			PIPE_CONF_CHECK_COLOR_LUT(gamma_mode, base.gamma_lut, bp_gamma);
+
 	}
 
 	PIPE_CONF_CHECK_BOOL(double_wide);
@@ -12837,6 +12861,7 @@ static bool fastboot_enabled(struct drm_i915_private *dev_priv)
 #undef PIPE_CONF_CHECK_P
 #undef PIPE_CONF_CHECK_FLAGS
 #undef PIPE_CONF_CHECK_CLOCK_FUZZY
+#undef PIPE_CONF_CHECK_COLOR_LUT
 #undef PIPE_CONF_QUIRK
 
 	return ret;
-- 
1.9.1

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

Re: [v8][PATCH 04/10] drm/i915/display: Add macro to compare gamma hw/sw lut

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Swati2
>Sent: Monday, August 26, 2019 11:56 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Nikula, Jani <jani.nikula@intel.com>; Sharma, Shashank
><shashank.sharma@intel.com>; Manna, Animesh <animesh.manna@intel.com>;
>Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; daniel.vetter@ffwll.ch;
>ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; Sharma,
>Swati2 <swati2.sharma@intel.com>
>Subject: [v8][PATCH 04/10] drm/i915/display: Add macro to compare gamma hw/sw
>lut
>
>Add macro to compare hw/sw gamma lut values. First need to check whether hw/sw
>gamma mode matches or not. If not no need to compare lut values, if matches then
>only compare lut entries.

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

>Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_display.c | 25 +++++++++++++++++++++++++
> 1 file changed, 25 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_display.c
>b/drivers/gpu/drm/i915/display/intel_display.c
>index ca88c70..63b7ad7 100644
>--- a/drivers/gpu/drm/i915/display/intel_display.c
>+++ b/drivers/gpu/drm/i915/display/intel_display.c
>@@ -12529,6 +12529,7 @@ static bool fastboot_enabled(struct drm_i915_private
>*dev_priv)  {
> 	struct drm_i915_private *dev_priv = to_i915(current_config->base.crtc-
>>dev);
> 	bool ret = true;
>+	u32 bp_gamma = 0;
> 	bool fixup_inherited = fastset &&
> 		(current_config->base.mode.private_flags &
>I915_MODE_FLAG_INHERITED) &&
> 		!(pipe_config->base.mode.private_flags &
>I915_MODE_FLAG_INHERITED); @@ -12680,6 +12681,24 @@ static bool
>fastboot_enabled(struct drm_i915_private *dev_priv)
> 	} \
> } while (0)
>
>+#define PIPE_CONF_CHECK_COLOR_LUT(name1, name2, bit_precision) do { \
>+	if (current_config->name1 != pipe_config->name1) { \
>+		pipe_config_mismatch(fastset, __stringify(name1), \
>+				"(expected %i, found %i, won't compare lut
>values)\n", \
>+				current_config->name1, \
>+				pipe_config->name1); \
>+		ret = false;\
>+	} else { \
>+		if (!intel_color_lut_equal(current_config->name2, \
>+					pipe_config->name2, pipe_config->name1, \
>+					bit_precision)) { \
>+			pipe_config_mismatch(fastset, __stringify(name2), \
>+					"hw_state doesn't match sw_state\n"); \
>+			ret = false; \
>+		} \
>+	} \
>+} while (0)
>+
> #define PIPE_CONF_QUIRK(quirk) \
> 	((current_config->quirks | pipe_config->quirks) & (quirk))
>
>@@ -12775,6 +12794,11 @@ static bool fastboot_enabled(struct drm_i915_private
>*dev_priv)
> 			PIPE_CONF_CHECK_X(csc_mode);
> 		PIPE_CONF_CHECK_BOOL(gamma_enable);
> 		PIPE_CONF_CHECK_BOOL(csc_enable);
>+
>+		bp_gamma = intel_color_get_gamma_bit_precision(pipe_config);
>+		if (bp_gamma)
>+			PIPE_CONF_CHECK_COLOR_LUT(gamma_mode,
>base.gamma_lut, bp_gamma);
>+
> 	}
>
> 	PIPE_CONF_CHECK_BOOL(double_wide);
>@@ -12837,6 +12861,7 @@ static bool fastboot_enabled(struct drm_i915_private
>*dev_priv)  #undef PIPE_CONF_CHECK_P  #undef PIPE_CONF_CHECK_FLAGS  #undef
>PIPE_CONF_CHECK_CLOCK_FUZZY
>+#undef PIPE_CONF_CHECK_COLOR_LUT
> #undef PIPE_CONF_QUIRK
>
> 	return ret;
>--
>1.9.1

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

[v8][PATCH 05/10] drm/i915/display: Extract i9xx_read_luts()

[Swati Sharma]

For the legacy gamma, have hw read out to create hw blob of gamma
lut values. Also, add function intel_color_lut_pack to convert hw value with
given bit_precision to lut property val.

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

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 27727a1..45e0ee8 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1521,6 +1521,56 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
 	return true;
 }
 
+/* convert hw value with given bit_precision to lut property val */
+static u32 intel_color_lut_pack(u32 val, u32 bit_precision)
+{
+	u32 max = 0xffff >> (16 - bit_precision);
+
+	val = clamp_val(val, 0, max);
+
+	if (bit_precision < 16)
+		val <<= 16 - bit_precision;
+
+	return val;
+}
+
+static struct drm_property_blob *
+i9xx_read_lut_8(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	enum pipe pipe = crtc->pipe;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+	u32 i, val;
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_lut) * 256,
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < 256; i++) {
+		if (HAS_GMCH(dev_priv))
+			val = I915_READ(PALETTE(pipe, i));
+		else
+			val = I915_READ(LGC_PALETTE(pipe, i));
+
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_RED_MASK, val), 8);
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_GREEN_MASK, val), 8);
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_BLUE_MASK, val), 8);
+	}
+
+	return blob;
+}
+
+void i9xx_read_luts(struct intel_crtc_state *crtc_state)
+{
+	crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1541,6 +1591,7 @@ void intel_color_init(struct intel_crtc *crtc)
 			dev_priv->display.color_check = i9xx_color_check;
 			dev_priv->display.color_commit = i9xx_color_commit;
 			dev_priv->display.load_luts = i9xx_load_luts;
+			dev_priv->display.read_luts = i9xx_read_luts;
 		}
 	} else {
 		if (INTEL_GEN(dev_priv) >= 11)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a092b34..b687faa 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7192,6 +7192,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 */
-- 
1.9.1

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

Re: [v8][PATCH 05/10] drm/i915/display: Extract i9xx_read_luts()

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Swati2
>Sent: Monday, August 26, 2019 11:56 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Nikula, Jani <jani.nikula@intel.com>; Sharma, Shashank
><shashank.sharma@intel.com>; Manna, Animesh <animesh.manna@intel.com>;
>Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; daniel.vetter@ffwll.ch;
>ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; Sharma,
>Swati2 <swati2.sharma@intel.com>
>Subject: [v8][PATCH 05/10] drm/i915/display: Extract i9xx_read_luts()
>
>For the legacy gamma, have hw read out to create hw blob of gamma lut values.

Would be better if we define platforms for which this is applicable (I mean what all is
considered legacy here)

>Also, add function intel_color_lut_pack to convert hw value with given bit_precision

Wrap this up within 75 characters.

>to lut property val.

Keep the version history, don't drop that.

>Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_color.c | 51 ++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_reg.h            |  3 ++
> 2 files changed, 54 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_color.c
>b/drivers/gpu/drm/i915/display/intel_color.c
>index 27727a1..45e0ee8 100644
>--- a/drivers/gpu/drm/i915/display/intel_color.c
>+++ b/drivers/gpu/drm/i915/display/intel_color.c
>@@ -1521,6 +1521,56 @@ bool intel_color_lut_equal(struct drm_property_blob
>*blob1,
> 	return true;
> }
>
>+/* convert hw value with given bit_precision to lut property val */
>+static u32 intel_color_lut_pack(u32 val, u32 bit_precision) {
>+	u32 max = 0xffff >> (16 - bit_precision);
>+
>+	val = clamp_val(val, 0, max);
>+
>+	if (bit_precision < 16)
>+		val <<= 16 - bit_precision;
>+
>+	return val;
>+}
>+
>+static struct drm_property_blob *
>+i9xx_read_lut_8(struct intel_crtc_state *crtc_state) {

Would be good to add some comments describing the rationale of this
function. Why 8 etc.

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

Have a macro for 256. Explain why 256, add comments.

>+					NULL);
>+	if (IS_ERR(blob))
>+		return NULL;
>+
>+	blob_data = blob->data;
>+
>+	for (i = 0; i < 256; i++) {

Add the macro for 256.

>+		if (HAS_GMCH(dev_priv))
>+			val = I915_READ(PALETTE(pipe, i));
>+		else
>+			val = I915_READ(LGC_PALETTE(pipe, i));
>+
>+		blob_data[i].red =
>intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_RED_MASK, val), 8);
>+		blob_data[i].green =
>intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_GREEN_MASK, val), 8);
>+		blob_data[i].blue =
>intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_BLUE_MASK, val), 8);
>+	}
>+
>+	return blob;
>+}
>+
>+void i9xx_read_luts(struct intel_crtc_state *crtc_state) {
>+	crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state); }
>+
> void intel_color_init(struct intel_crtc *crtc)  {
> 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -1541,6
>+1591,7 @@ void intel_color_init(struct intel_crtc *crtc)
> 			dev_priv->display.color_check = i9xx_color_check;
> 			dev_priv->display.color_commit = i9xx_color_commit;
> 			dev_priv->display.load_luts = i9xx_load_luts;
>+			dev_priv->display.read_luts = i9xx_read_luts;
> 		}
> 	} else {
> 		if (INTEL_GEN(dev_priv) >= 11)
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index
>a092b34..b687faa 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -7192,6 +7192,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 */
>--
>1.9.1

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

Re: [v8][PATCH 05/10] drm/i915/display: Extract i9xx_read_luts()

[Sharma, Swati2]

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

On 28-Aug-19 9:25 PM, Shankar, Uma wrote:
>
>> -----Original Message-----
>> From: Sharma, Swati2
>> Sent: Monday, August 26, 2019 11:56 AM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: Nikula, Jani <jani.nikula@intel.com>; Sharma, Shashank
>> <shashank.sharma@intel.com>; Manna, Animesh <animesh.manna@intel.com>;
>> Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; daniel.vetter@ffwll.ch;
>> ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; Sharma,
>> Swati2 <swati2.sharma@intel.com>
>> Subject: [v8][PATCH 05/10] drm/i915/display: Extract i9xx_read_luts()
>>
>> For the legacy gamma, have hw read out to create hw blob of gamma lut values.
> Would be better if we define platforms for which this is applicable (I mean what all is
> considered legacy here)
>
>> Also, add function intel_color_lut_pack to convert hw value with given bit_precision
> Wrap this up within 75 characters.
>
>> to lut property val.
> Keep the version history, don't drop that.
>
>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>> ---
>> drivers/gpu/drm/i915/display/intel_color.c | 51 ++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/i915_reg.h            |  3 ++
>> 2 files changed, 54 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
>> b/drivers/gpu/drm/i915/display/intel_color.c
>> index 27727a1..45e0ee8 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>> @@ -1521,6 +1521,56 @@ bool intel_color_lut_equal(struct drm_property_blob
>> *blob1,
>> 	return true;
>> }
>>
>> +/* convert hw value with given bit_precision to lut property val */
>> +static u32 intel_color_lut_pack(u32 val, u32 bit_precision) {
>> +	u32 max = 0xffff >> (16 - bit_precision);
>> +
>> +	val = clamp_val(val, 0, max);
>> +
>> +	if (bit_precision < 16)
>> +		val <<= 16 - bit_precision;
>> +
>> +	return val;
>> +}
>> +
>> +static struct drm_property_blob *
>> +i9xx_read_lut_8(struct intel_crtc_state *crtc_state) {
> Would be good to add some comments describing the rationale of this
> function. Why 8 etc.
Func is written similar to load luts for i9xx.Therefore didn't explain 8.
Do I need to add comment for all the functions/platform? Won't commit 
message
sufficient enough?
>
>> +	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,
> Have a macro for 256. Explain why 256, add comments.
This is similar to load luts, since nothing new i added so didn't give
explanation. I can re-use LEGACY_LUT_LENGTH for this, since wanted these
functions to be similar to load_luts, therefore kept same.
>
>> +					NULL);
>> +	if (IS_ERR(blob))
>> +		return NULL;
>> +
>> +	blob_data = blob->data;
>> +
>> +	for (i = 0; i < 256; i++) {
> Add the macro for 256.
Macro already there LEGACY_LUT_LENGTH. Should i reuse that?
>
>> +		if (HAS_GMCH(dev_priv))
>> +			val = I915_READ(PALETTE(pipe, i));
>> +		else
>> +			val = I915_READ(LGC_PALETTE(pipe, i));
>> +
>> +		blob_data[i].red =
>> intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_RED_MASK, val), 8);
>> +		blob_data[i].green =
>> intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_GREEN_MASK, val), 8);
>> +		blob_data[i].blue =
>> intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_BLUE_MASK, val), 8);
>> +	}
>> +
>> +	return blob;
>> +}
>> +
>> +void i9xx_read_luts(struct intel_crtc_state *crtc_state) {
>> +	crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state); }
>> +
>> void intel_color_init(struct intel_crtc *crtc)  {
>> 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -1541,6
>> +1591,7 @@ void intel_color_init(struct intel_crtc *crtc)
>> 			dev_priv->display.color_check = i9xx_color_check;
>> 			dev_priv->display.color_commit = i9xx_color_commit;
>> 			dev_priv->display.load_luts = i9xx_load_luts;
>> +			dev_priv->display.read_luts = i9xx_read_luts;
>> 		}
>> 	} else {
>> 		if (INTEL_GEN(dev_priv) >= 11)
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index
>> a092b34..b687faa 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7192,6 +7192,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 */
>> --
>> 1.9.1


-- 
~Swati Sharma


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

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

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

Re: [v8][PATCH 05/10] drm/i915/display: Extract i9xx_read_luts()

[Shankar, Uma]

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



From: Sharma, Swati2
Sent: Thursday, August 29, 2019 2:00 AM
To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org
Cc: Nikula, Jani <jani.nikula@intel.com>; Sharma, Shashank <shashank.sharma@intel.com>; Manna, Animesh <animesh.manna@intel.com>; Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; daniel.vetter@ffwll.ch; ville.syrjala@linux.intel.com
Subject: Re: [v8][PATCH 05/10] drm/i915/display: Extract i9xx_read_luts()

On 28-Aug-19 9:25 PM, Shankar, Uma wrote:





-----Original Message-----

From: Sharma, Swati2

Sent: Monday, August 26, 2019 11:56 AM

To: intel-gfx@lists.freedesktop.org<mailto:intel-gfx@lists.freedesktop.org>

Cc: Nikula, Jani <jani.nikula@intel.com><mailto:jani.nikula@intel.com>; Sharma, Shashank

<shashank.sharma@intel.com><mailto:shashank.sharma@intel.com>; Manna, Animesh <animesh.manna@intel.com><mailto:animesh.manna@intel.com>;

Nautiyal, Ankit K <ankit.k.nautiyal@intel.com><mailto:ankit.k.nautiyal@intel.com>; daniel.vetter@ffwll.ch<mailto:daniel.vetter@ffwll.ch>;

ville.syrjala@linux.intel.com<mailto:ville.syrjala@linux.intel.com>; Shankar, Uma <uma.shankar@intel.com><mailto:uma.shankar@intel.com>; Sharma,

Swati2 <swati2.sharma@intel.com><mailto:swati2.sharma@intel.com>

Subject: [v8][PATCH 05/10] drm/i915/display: Extract i9xx_read_luts()



For the legacy gamma, have hw read out to create hw blob of gamma lut values.



Would be better if we define platforms for which this is applicable (I mean what all is

considered legacy here)



Also, add function intel_color_lut_pack to convert hw value with given bit_precision



Wrap this up within 75 characters.



to lut property val.



Keep the version history, don't drop that.



Signed-off-by: Swati Sharma <swati2.sharma@intel.com><mailto:swati2.sharma@intel.com>

---

drivers/gpu/drm/i915/display/intel_color.c | 51 ++++++++++++++++++++++++++++++

drivers/gpu/drm/i915/i915_reg.h            |  3 ++

2 files changed, 54 insertions(+)



diff --git a/drivers/gpu/drm/i915/display/intel_color.c

b/drivers/gpu/drm/i915/display/intel_color.c

index 27727a1..45e0ee8 100644

--- a/drivers/gpu/drm/i915/display/intel_color.c

+++ b/drivers/gpu/drm/i915/display/intel_color.c

@@ -1521,6 +1521,56 @@ bool intel_color_lut_equal(struct drm_property_blob

*blob1,

   return true;

}



+/* convert hw value with given bit_precision to lut property val */

+static u32 intel_color_lut_pack(u32 val, u32 bit_precision) {

+  u32 max = 0xffff >> (16 - bit_precision);

+

+  val = clamp_val(val, 0, max);

+

+  if (bit_precision < 16)

+         val <<= 16 - bit_precision;

+

+  return val;

+}

+

+static struct drm_property_blob *

+i9xx_read_lut_8(struct intel_crtc_state *crtc_state) {



Would be good to add some comments describing the rationale of this

function. Why 8 etc.
Func is written similar to load luts for i9xx.Therefore didn't explain 8.
Do I need to add comment for all the functions/platform? Won't commit message
sufficient enough?

Commit message will not be visible while browsing the code. Would be good to add comments explaining the rationale.



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



Have a macro for 256. Explain why 256, add comments.
This is similar to load luts, since nothing new i added so didn't give
explanation. I can re-use LEGACY_LUT_LENGTH for this, since wanted these
functions to be similar to load_luts, therefore kept same.

Ok, we still can use the macro.



+                                NULL);

+  if (IS_ERR(blob))

+         return NULL;

+

+  blob_data = blob->data;

+

+  for (i = 0; i < 256; i++) {



Add the macro for 256.
Macro already there LEGACY_LUT_LENGTH. Should i reuse that?

Yes you can reuse it.





+         if (HAS_GMCH(dev_priv))

+                 val = I915_READ(PALETTE(pipe, i));

+         else

+                 val = I915_READ(LGC_PALETTE(pipe, i));

+

+         blob_data[i].red =

intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_RED_MASK, val), 8);

+         blob_data[i].green =

intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_GREEN_MASK, val), 8);

+         blob_data[i].blue =

intel_color_lut_pack(REG_FIELD_GET(LGC_PALETTE_BLUE_MASK, val), 8);

+  }

+

+  return blob;

+}

+

+void i9xx_read_luts(struct intel_crtc_state *crtc_state) {

+  crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state); }

+

void intel_color_init(struct intel_crtc *crtc)  {

   struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -1541,6

+1591,7 @@ void intel_color_init(struct intel_crtc *crtc)

                  dev_priv->display.color_check = i9xx_color_check;

                  dev_priv->display.color_commit = i9xx_color_commit;

                  dev_priv->display.load_luts = i9xx_load_luts;

+                 dev_priv->display.read_luts = i9xx_read_luts;

          }

   } else {

          if (INTEL_GEN(dev_priv) >= 11)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index

a092b34..b687faa 100644

--- a/drivers/gpu/drm/i915/i915_reg.h

+++ b/drivers/gpu/drm/i915/i915_reg.h

@@ -7192,6 +7192,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 */

--

1.9.1






--

~Swati Sharma

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

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

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

[v8][PATCH 06/10] drm/i91/display: Extract i965_read_luts()

[Swati Sharma]

For i965, have hw read out to create hw blob of gamma
lut values.

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

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 45e0ee8..c77bbed 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1571,6 +1571,44 @@ void i9xx_read_luts(struct intel_crtc_state *crtc_state)
 	crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
 }
 
+static struct drm_property_blob *
+i965_read_gamma_lut_10p6(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	u32 i, val1, val2, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+	enum pipe pipe = crtc->pipe;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_lut) * lut_size,
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < lut_size - 1; i++) {
+		val1 = I915_READ(PALETTE(pipe, 2 * i + 0));
+		val2 = I915_READ(PALETTE(pipe, 2 * i + 1));
+
+		blob_data[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val1) << 8 | REG_FIELD_GET(PALETTE_RED_MASK, val2);
+		blob_data[i].green = REG_FIELD_GET(PALETTE_GREEN_MASK, val1) << 8 | REG_FIELD_GET(PALETTE_GREEN_MASK, val2);
+		blob_data[i].blue = REG_FIELD_GET(PALETTE_BLUE_MASK, val1) << 8 | REG_FIELD_GET(PALETTE_BLUE_MASK, val2) ;
+	}
+
+	return blob;
+}
+
+static void i965_read_luts(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
+		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
+	else
+		crtc_state->base.gamma_lut = i965_read_gamma_lut_10p6(crtc_state);
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1586,6 +1624,7 @@ void intel_color_init(struct intel_crtc *crtc)
 		} else if (INTEL_GEN(dev_priv) >= 4) {
 			dev_priv->display.color_check = i9xx_color_check;
 			dev_priv->display.color_commit = i9xx_color_commit;
+			dev_priv->display.read_luts = i965_read_luts;
 			dev_priv->display.load_luts = i965_load_luts;
 		} else {
 			dev_priv->display.color_check = i9xx_color_check;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index b687faa..b30b0c6b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3558,6 +3558,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #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) + \
-- 
1.9.1

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

Re: [v8][PATCH 06/10] drm/i91/display: Extract i965_read_luts()

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Swati2
>Sent: Monday, August 26, 2019 11:56 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Nikula, Jani <jani.nikula@intel.com>; Sharma, Shashank
><shashank.sharma@intel.com>; Manna, Animesh <animesh.manna@intel.com>;
>Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; daniel.vetter@ffwll.ch;
>ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; Sharma,
>Swati2 <swati2.sharma@intel.com>
>Subject: [v8][PATCH 06/10] drm/i91/display: Extract i965_read_luts()

Typo in i915.

>
>For i965, have hw read out to create hw blob of gamma lut values.
Instead of "have", I feel "add" would sound better.

Also, don't drop version history.

>Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_color.c | 39 ++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_reg.h            |  3 +++
> 2 files changed, 42 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_color.c
>b/drivers/gpu/drm/i915/display/intel_color.c
>index 45e0ee8..c77bbed 100644
>--- a/drivers/gpu/drm/i915/display/intel_color.c
>+++ b/drivers/gpu/drm/i915/display/intel_color.c
>@@ -1571,6 +1571,44 @@ void i9xx_read_luts(struct intel_crtc_state *crtc_state)
> 	crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);  }
>
>+static struct drm_property_blob *
>+i965_read_gamma_lut_10p6(struct intel_crtc_state *crtc_state) {

You can rename this as " i965_read_lut_10p6" as pointed by Ville as well.
Will help extend for de-gamma later and can be re-used.

Also make these const, as recommended by Ville. 

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

Move to next line to honour 80 character limits.

>+	enum pipe pipe = crtc->pipe;
>+	struct drm_property_blob *blob;
>+	struct drm_color_lut *blob_data;
>+
>+	blob = drm_property_create_blob(&dev_priv->drm,
>+					sizeof(struct drm_color_lut) * lut_size,
>+					NULL);
>+	if (IS_ERR(blob))
>+		return NULL;
>+
>+	blob_data = blob->data;
>+
>+	for (i = 0; i < lut_size - 1; i++) {
>+		val1 = I915_READ(PALETTE(pipe, 2 * i + 0));
>+		val2 = I915_READ(PALETTE(pipe, 2 * i + 1));
>+
>+		blob_data[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val1) << 8 |
>REG_FIELD_GET(PALETTE_RED_MASK, val2);
>+		blob_data[i].green = REG_FIELD_GET(PALETTE_GREEN_MASK, val1)
><< 8 | REG_FIELD_GET(PALETTE_GREEN_MASK, val2);
>+		blob_data[i].blue = REG_FIELD_GET(PALETTE_BLUE_MASK, val1) << 8
>| REG_FIELD_GET(PALETTE_BLUE_MASK, val2) ;
>+	}
>+
>+	return blob;
>+}
>+
>+static void i965_read_luts(struct intel_crtc_state *crtc_state) {
>+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
>+		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
>+	else
>+		crtc_state->base.gamma_lut =
>i965_read_gamma_lut_10p6(crtc_state);
>+}
>+
> void intel_color_init(struct intel_crtc *crtc)  {
> 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -1586,6
>+1624,7 @@ void intel_color_init(struct intel_crtc *crtc)
> 		} else if (INTEL_GEN(dev_priv) >= 4) {
> 			dev_priv->display.color_check = i9xx_color_check;
> 			dev_priv->display.color_commit = i9xx_color_commit;
>+			dev_priv->display.read_luts = i965_read_luts;
> 			dev_priv->display.load_luts = i965_load_luts;
> 		} else {
> 			dev_priv->display.color_check = i9xx_color_check; diff --git
>a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index
>b687faa..b30b0c6b 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -3558,6 +3558,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
> #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) + \
>--
>1.9.1

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

Re: [v8][PATCH 06/10] drm/i91/display: Extract i965_read_luts()

[Shankar, Uma]

>-----Original Message-----
>From: Intel-gfx <intel-gfx-bounces@lists.freedesktop.org> On Behalf Of Shankar,
>Uma
>Sent: Wednesday, August 28, 2019 9:38 PM
>To: Sharma, Swati2 <swati2.sharma@intel.com>; intel-gfx@lists.freedesktop.org
>Cc: Nikula, Jani <jani.nikula@intel.com>; daniel.vetter@ffwll.ch; Nautiyal, Ankit K
><ankit.k.nautiyal@intel.com>
>Subject: Re: [Intel-gfx] [v8][PATCH 06/10] drm/i91/display: Extract i965_read_luts()
>
>
>
>>-----Original Message-----
>>From: Sharma, Swati2
>>Sent: Monday, August 26, 2019 11:56 AM
>>To: intel-gfx@lists.freedesktop.org
>>Cc: Nikula, Jani <jani.nikula@intel.com>; Sharma, Shashank
>><shashank.sharma@intel.com>; Manna, Animesh <animesh.manna@intel.com>;
>>Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; daniel.vetter@ffwll.ch;
>>ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>;
>>Sharma,
>>Swati2 <swati2.sharma@intel.com>
>>Subject: [v8][PATCH 06/10] drm/i91/display: Extract i965_read_luts()
>
>Typo in i915.
>
>>
>>For i965, have hw read out to create hw blob of gamma lut values.
>Instead of "have", I feel "add" would sound better.
>
>Also, don't drop version history.
>
>>Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>>---
>> drivers/gpu/drm/i915/display/intel_color.c | 39
>++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/i915_reg.h            |  3 +++
>> 2 files changed, 42 insertions(+)
>>
>>diff --git a/drivers/gpu/drm/i915/display/intel_color.c
>>b/drivers/gpu/drm/i915/display/intel_color.c
>>index 45e0ee8..c77bbed 100644
>>--- a/drivers/gpu/drm/i915/display/intel_color.c
>>+++ b/drivers/gpu/drm/i915/display/intel_color.c
>>@@ -1571,6 +1571,44 @@ void i9xx_read_luts(struct intel_crtc_state *crtc_state)
>> 	crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);  }
>>
>>+static struct drm_property_blob *
>>+i965_read_gamma_lut_10p6(struct intel_crtc_state *crtc_state) {
>
>You can rename this as " i965_read_lut_10p6" as pointed by Ville as well.
>Will help extend for de-gamma later and can be re-used.
>
>Also make these const, as recommended by Ville.
>
>>+	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;
>
>Move to next line to honour 80 character limits.
>
>>+	enum pipe pipe = crtc->pipe;
>>+	struct drm_property_blob *blob;
>>+	struct drm_color_lut *blob_data;
>>+
>>+	blob = drm_property_create_blob(&dev_priv->drm,
>>+					sizeof(struct drm_color_lut) * lut_size,
>>+					NULL);
>>+	if (IS_ERR(blob))
>>+		return NULL;
>>+
>>+	blob_data = blob->data;
>>+
>>+	for (i = 0; i < lut_size - 1; i++) {
>>+		val1 = I915_READ(PALETTE(pipe, 2 * i + 0));
>>+		val2 = I915_READ(PALETTE(pipe, 2 * i + 1));
>>+
>>+		blob_data[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val1) << 8 |
>>REG_FIELD_GET(PALETTE_RED_MASK, val2);
>>+		blob_data[i].green = REG_FIELD_GET(PALETTE_GREEN_MASK, val1)
>><< 8 | REG_FIELD_GET(PALETTE_GREEN_MASK, val2);
>>+		blob_data[i].blue = REG_FIELD_GET(PALETTE_BLUE_MASK, val1) << 8
>>| REG_FIELD_GET(PALETTE_BLUE_MASK, val2) ;

Missed these earlier.
Please drop space before the ;. Also wrap these lines for 80 characters limits.

>>+	}
>>+
>>+	return blob;
>>+}
>>+
>>+static void i965_read_luts(struct intel_crtc_state *crtc_state) {
>>+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
>>+		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
>>+	else
>>+		crtc_state->base.gamma_lut =
>>i965_read_gamma_lut_10p6(crtc_state);
>>+}
>>+
>> void intel_color_init(struct intel_crtc *crtc)  {
>> 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@
>> -1586,6
>>+1624,7 @@ void intel_color_init(struct intel_crtc *crtc)
>> 		} else if (INTEL_GEN(dev_priv) >= 4) {
>> 			dev_priv->display.color_check = i9xx_color_check;
>> 			dev_priv->display.color_commit = i9xx_color_commit;
>>+			dev_priv->display.read_luts = i965_read_luts;
>> 			dev_priv->display.load_luts = i965_load_luts;
>> 		} else {
>> 			dev_priv->display.color_check = i9xx_color_check; diff --git
>>a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>>index b687faa..b30b0c6b 100644
>>--- a/drivers/gpu/drm/i915/i915_reg.h
>>+++ b/drivers/gpu/drm/i915/i915_reg.h
>>@@ -3558,6 +3558,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>> #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) + \
>>--
>>1.9.1
>
>_______________________________________________
>Intel-gfx mailing list
>Intel-gfx@lists.freedesktop.org
>https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [v8][PATCH 06/10] drm/i91/display: Extract i965_read_luts()

[Sharma, Swati2]

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

On 28-Aug-19 9:38 PM, Shankar, Uma wrote:
>
>> -----Original Message-----
>> From: Sharma, Swati2
>> Sent: Monday, August 26, 2019 11:56 AM
>> To: intel-gfx@lists.freedesktop.org
>> Cc: Nikula, Jani <jani.nikula@intel.com>; Sharma, Shashank
>> <shashank.sharma@intel.com>; Manna, Animesh <animesh.manna@intel.com>;
>> Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; daniel.vetter@ffwll.ch;
>> ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; Sharma,
>> Swati2 <swati2.sharma@intel.com>
>> Subject: [v8][PATCH 06/10] drm/i91/display: Extract i965_read_luts()
> Typo in i915.
>
>> For i965, have hw read out to create hw blob of gamma lut values.
> Instead of "have", I feel "add" would sound better.
>
> Also, don't drop version history.
>
>> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>> ---
>> drivers/gpu/drm/i915/display/intel_color.c | 39 ++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/i915_reg.h            |  3 +++
>> 2 files changed, 42 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
>> b/drivers/gpu/drm/i915/display/intel_color.c
>> index 45e0ee8..c77bbed 100644
>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>> @@ -1571,6 +1571,44 @@ void i9xx_read_luts(struct intel_crtc_state *crtc_state)
>> 	crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);  }
>>
>> +static struct drm_property_blob *
>> +i965_read_gamma_lut_10p6(struct intel_crtc_state *crtc_state) {
> You can rename this as " i965_read_lut_10p6" as pointed by Ville as well.
> Will help extend for de-gamma later and can be re-used.
Did this since we may need to create a new func for degamma readout and
this can't be reused.
>
> Also make these const, as recommended by Ville.
>
>> +	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;
> Move to next line to honour 80 character limits.
>
>> +	enum pipe pipe = crtc->pipe;
>> +	struct drm_property_blob *blob;
>> +	struct drm_color_lut *blob_data;
>> +
>> +	blob = drm_property_create_blob(&dev_priv->drm,
>> +					sizeof(struct drm_color_lut) * lut_size,
>> +					NULL);
>> +	if (IS_ERR(blob))
>> +		return NULL;
>> +
>> +	blob_data = blob->data;
>> +
>> +	for (i = 0; i < lut_size - 1; i++) {
>> +		val1 = I915_READ(PALETTE(pipe, 2 * i + 0));
>> +		val2 = I915_READ(PALETTE(pipe, 2 * i + 1));
>> +
>> +		blob_data[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val1) << 8 |
>> REG_FIELD_GET(PALETTE_RED_MASK, val2);
>> +		blob_data[i].green = REG_FIELD_GET(PALETTE_GREEN_MASK, val1)
>> << 8 | REG_FIELD_GET(PALETTE_GREEN_MASK, val2);
>> +		blob_data[i].blue = REG_FIELD_GET(PALETTE_BLUE_MASK, val1) << 8
>> | REG_FIELD_GET(PALETTE_BLUE_MASK, val2) ;
>> +	}
>> +
>> +	return blob;
>> +}
>> +
>> +static void i965_read_luts(struct intel_crtc_state *crtc_state) {
>> +	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
>> +		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
>> +	else
>> +		crtc_state->base.gamma_lut =
>> i965_read_gamma_lut_10p6(crtc_state);
>> +}
>> +
>> void intel_color_init(struct intel_crtc *crtc)  {
>> 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -1586,6
>> +1624,7 @@ void intel_color_init(struct intel_crtc *crtc)
>> 		} else if (INTEL_GEN(dev_priv) >= 4) {
>> 			dev_priv->display.color_check = i9xx_color_check;
>> 			dev_priv->display.color_commit = i9xx_color_commit;
>> +			dev_priv->display.read_luts = i965_read_luts;
>> 			dev_priv->display.load_luts = i965_load_luts;
>> 		} else {
>> 			dev_priv->display.color_check = i9xx_color_check; diff --git
>> a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index
>> b687faa..b30b0c6b 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -3558,6 +3558,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>> #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) + \
>> --
>> 1.9.1


-- 
~Swati Sharma


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

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

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

Re: [v8][PATCH 06/10] drm/i91/display: Extract i965_read_luts()

[Sharma, Swati2]

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

On 29-Aug-19 2:18 AM, Sharma, Swati2 wrote:
> On 28-Aug-19 9:38 PM, Shankar, Uma wrote:
>>> -----Original Message-----
>>> From: Sharma, Swati2
>>> Sent: Monday, August 26, 2019 11:56 AM
>>> To:intel-gfx@lists.freedesktop.org
>>> Cc: Nikula, Jani<jani.nikula@intel.com>; Sharma, Shashank
>>> <shashank.sharma@intel.com>; Manna, Animesh<animesh.manna@intel.com>;
>>> Nautiyal, Ankit K<ankit.k.nautiyal@intel.com>;daniel.vetter@ffwll.ch;
>>> ville.syrjala@linux.intel.com; Shankar, Uma<uma.shankar@intel.com>; Sharma,
>>> Swati2<swati2.sharma@intel.com>
>>> Subject: [v8][PATCH 06/10] drm/i91/display: Extract i965_read_luts()
>> Typo in i915.
>>
>>> For i965, have hw read out to create hw blob of gamma lut values.
>> Instead of "have", I feel "add" would sound better.
>>
>> Also, don't drop version history.
>>
>>> Signed-off-by: Swati Sharma<swati2.sharma@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/display/intel_color.c | 39 ++++++++++++++++++++++++++++++
>>> drivers/gpu/drm/i915/i915_reg.h            |  3 +++
>>> 2 files changed, 42 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_color.c
>>> b/drivers/gpu/drm/i915/display/intel_color.c
>>> index 45e0ee8..c77bbed 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_color.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_color.c
>>> @@ -1571,6 +1571,44 @@ void i9xx_read_luts(struct intel_crtc_state *crtc_state)
>>> 	crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);  }
>>>
>>> +static struct drm_property_blob *
>>> +i965_read_gamma_lut_10p6(struct intel_crtc_state *crtc_state) {
>> You can rename this as " i965_read_lut_10p6" as pointed by Ville as well.
>> Will help extend for de-gamma later and can be re-used.
> Did this since we may need to create a new func for degamma readout and
> this can't be reused.
Sorry, already made this change earlier..missed somehow.
>> Also make these const, as recommended by Ville.
>>
>>> +	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;
>> Move to next line to honour 80 character limits.
>>
>>> +	enum pipe pipe = crtc->pipe;
>>> +	struct drm_property_blob *blob;
>>> +	struct drm_color_lut *blob_data;
>>> +
>>> +	blob = drm_property_create_blob(&dev_priv->drm,
>>> +					sizeof(struct drm_color_lut) * lut_size,
>>> +					NULL);
>>> +	if (IS_ERR(blob))
>>> +		return NULL;
>>> +
>>> +	blob_data = blob->data;
>>> +
>>> +	for (i = 0; i < lut_size - 1; i++) {
>>> +		val1 = I915_READ(PALETTE(pipe, 2 * i + 0));
>>> +		val2 = I915_READ(PALETTE(pipe, 2 * i + 1));
>>> +
>>> +		blob_data[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val1) << 8 |
>>> REG_FIELD_GET(PALETTE_RED_MASK, val2);
>>> +		blob_data[i].green = REG_FIELD_GET(PALETTE_GREEN_MASK, val1)
>>> << 8 | REG_FIELD_GET(PALETTE_GREEN_MASK, val2);
>>> +		blob_data[i].blue = REG_FIELD_GET(PALETTE_BLUE_MASK, val1) << 8
>>> | REG_FIELD_GET(PALETTE_BLUE_MASK, val2) ;
>>> +	}
>>> +
>>> +	return blob;
>>> +}
>>> +
>>> +static void i965_read_luts(struct intel_crtc_state *crtc_state) {
>>> +	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
>>> +		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
>>> +	else
>>> +		crtc_state->base.gamma_lut =
>>> i965_read_gamma_lut_10p6(crtc_state);
>>> +}
>>> +
>>> void intel_color_init(struct intel_crtc *crtc)  {
>>> 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -1586,6
>>> +1624,7 @@ void intel_color_init(struct intel_crtc *crtc)
>>> 		} else if (INTEL_GEN(dev_priv) >= 4) {
>>> 			dev_priv->display.color_check = i9xx_color_check;
>>> 			dev_priv->display.color_commit = i9xx_color_commit;
>>> +			dev_priv->display.read_luts = i965_read_luts;
>>> 			dev_priv->display.load_luts = i965_load_luts;
>>> 		} else {
>>> 			dev_priv->display.color_check = i9xx_color_check; diff --git
>>> a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index
>>> b687faa..b30b0c6b 100644
>>> --- a/drivers/gpu/drm/i915/i915_reg.h
>>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>>> @@ -3558,6 +3558,9 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>>> #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) + \
>>> --
>>> 1.9.1
>
>
> -- 
> ~Swati Sharma


-- 
~Swati Sharma


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

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

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

[v8][PATCH 09/10] drm/i915/display: Extract glk_read_luts()

[Swati Sharma]

For glk, have hw read out to create hw blob of gamma
lut values.

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

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 4b9b28f..3762bdf 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1684,6 +1684,49 @@ static void ilk_read_luts(struct intel_crtc_state *crtc_state)
 		crtc_state->base.gamma_lut = ilk_read_gamma_lut(crtc_state);
 }
 
+static struct drm_property_blob *
+glk_read_lut_10(struct intel_crtc_state *crtc_state, u32 prec_index)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	int hw_lut_size = ivb_lut_10_size(prec_index);
+	enum pipe pipe = crtc->pipe;
+	struct drm_property_blob *blob;
+	struct drm_color_lut *blob_data;
+	u32 i, val;
+
+	I915_WRITE(PREC_PAL_INDEX(pipe), prec_index |
+		   PAL_PREC_AUTO_INCREMENT);
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_lut) * hw_lut_size,
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	blob_data = blob->data;
+
+	for (i = 0; i < hw_lut_size; i++) {
+		val = I915_READ(PREC_PAL_DATA(pipe));
+
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_RED_MASK, val), 10);
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_GREEN_MASK, val), 10);
+		blob_data[i].blue = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_BLUE_MASK, val), 10);
+	}
+
+	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
+
+	return blob;
+}
+
+static void glk_read_luts(struct intel_crtc_state *crtc_state)
+{
+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
+		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
+	else
+		crtc_state->base.gamma_lut = glk_read_lut_10(crtc_state, PAL_PREC_INDEX_VALUE(0));
+}
+
 void intel_color_init(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -1727,9 +1770,10 @@ void intel_color_init(struct intel_crtc *crtc)
 
 		if (INTEL_GEN(dev_priv) >= 11)
 			dev_priv->display.load_luts = icl_load_luts;
-		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
+		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
 			dev_priv->display.load_luts = glk_load_luts;
-		else if (INTEL_GEN(dev_priv) >= 8)
+			dev_priv->display.read_luts = glk_read_luts;
+		} else if (INTEL_GEN(dev_priv) >= 8)
 			dev_priv->display.load_luts = bdw_load_luts;
 		else if (INTEL_GEN(dev_priv) >= 7)
 			dev_priv->display.load_luts = ivb_load_luts;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index acc9239..7613e14 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -10258,6 +10258,9 @@ enum skl_power_gate {
 #define _PAL_PREC_GC_MAX_A	0x4A410
 #define _PAL_PREC_GC_MAX_B	0x4AC10
 #define _PAL_PREC_GC_MAX_C	0x4B410
+#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_EXT_GC_MAX_A	0x4A420
 #define _PAL_PREC_EXT_GC_MAX_B	0x4AC20
 #define _PAL_PREC_EXT_GC_MAX_C	0x4B420
-- 
1.9.1

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

Re: [v8][PATCH 09/10] drm/i915/display: Extract glk_read_luts()

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Swati2
>Sent: Monday, August 26, 2019 11:56 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Nikula, Jani <jani.nikula@intel.com>; Sharma, Shashank
><shashank.sharma@intel.com>; Manna, Animesh <animesh.manna@intel.com>;
>Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; daniel.vetter@ffwll.ch;
>ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; Sharma,
>Swati2 <swati2.sharma@intel.com>
>Subject: [v8][PATCH 09/10] drm/i915/display: Extract glk_read_luts()
>
>For glk, have hw read out to create hw blob of gamma lut values.

Same as earlier patches.

>Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_color.c | 48 ++++++++++++++++++++++++++++--
> drivers/gpu/drm/i915/i915_reg.h            |  3 ++
> 2 files changed, 49 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_color.c
>b/drivers/gpu/drm/i915/display/intel_color.c
>index 4b9b28f..3762bdf 100644
>--- a/drivers/gpu/drm/i915/display/intel_color.c
>+++ b/drivers/gpu/drm/i915/display/intel_color.c
>@@ -1684,6 +1684,49 @@ static void ilk_read_luts(struct intel_crtc_state
>*crtc_state)
> 		crtc_state->base.gamma_lut = ilk_read_gamma_lut(crtc_state);  }
>
>+static struct drm_property_blob *
>+glk_read_lut_10(struct intel_crtc_state *crtc_state, u32 prec_index) {

Make "crtc_state" as const.

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

Wrap this.

>+					NULL);
>+	if (IS_ERR(blob))
>+		return NULL;
>+
>+	blob_data = blob->data;
>+
>+	for (i = 0; i < hw_lut_size; i++) {
>+		val = I915_READ(PREC_PAL_DATA(pipe));
>+
>+		blob_data[i].red =
>intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_RED_MASK, val), 10);
>+		blob_data[i].green =
>intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_GREEN_MASK, val), 10);
>+		blob_data[i].blue =
>intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_BLUE_MASK, val), 10);
>+	}
>+
>+	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
>+
>+	return blob;
>+}
>+
>+static void glk_read_luts(struct intel_crtc_state *crtc_state) {
>+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
>+		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
>+	else
>+		crtc_state->base.gamma_lut = glk_read_lut_10(crtc_state,
>+PAL_PREC_INDEX_VALUE(0)); }
>+
> void intel_color_init(struct intel_crtc *crtc)  {
> 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -1727,9
>+1770,10 @@ void intel_color_init(struct intel_crtc *crtc)
>
> 		if (INTEL_GEN(dev_priv) >= 11)
> 			dev_priv->display.load_luts = icl_load_luts;
>-		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv))
>+		else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
> 			dev_priv->display.load_luts = glk_load_luts;
>-		else if (INTEL_GEN(dev_priv) >= 8)
>+			dev_priv->display.read_luts = glk_read_luts;
>+		} else if (INTEL_GEN(dev_priv) >= 8)
> 			dev_priv->display.load_luts = bdw_load_luts;
> 		else if (INTEL_GEN(dev_priv) >= 7)
> 			dev_priv->display.load_luts = ivb_load_luts; diff --git
>a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index
>acc9239..7613e14 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -10258,6 +10258,9 @@ enum skl_power_gate {
> #define _PAL_PREC_GC_MAX_A	0x4A410
> #define _PAL_PREC_GC_MAX_B	0x4AC10
> #define _PAL_PREC_GC_MAX_C	0x4B410
>+#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_EXT_GC_MAX_A	0x4A420
> #define _PAL_PREC_EXT_GC_MAX_B	0x4AC20
> #define _PAL_PREC_EXT_GC_MAX_C	0x4B420
>--
>1.9.1

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

[v8][PATCH 10/10] FOR_TESTING_ONLY: Print rgb values of hw and sw blobs

[Swati Sharma]

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

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 3762bdf..56fdf83 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1453,6 +1453,8 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat
 static inline bool err_check(struct drm_color_lut *sw_lut,
 			     struct drm_color_lut *hw_lut, u32 err)
 {
+	DRM_DEBUG_KMS("hw_lut->red=0x%x sw_lut->red=0x%x hw_lut->blue=0x%x sw_lut->blue=0x%x hw_lut->green=0x%x sw_lut->green=0x%x", hw_lut->red, sw_lut->red, hw_lut->blue, sw_lut->blue, hw_lut->green, sw_lut->green);
+
 	return ((abs((long)hw_lut->red - sw_lut->red)) <= err) &&
 		((abs((long)hw_lut->blue - sw_lut->blue)) <= err) &&
 		((abs((long)hw_lut->green - sw_lut->green)) <= err);
-- 
1.9.1

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

✗ Fi.CI.CHECKPATCH: warning for drm/i915: adding state checker for gamma lut value

[Patchwork]

== Series Details ==

Series: drm/i915: adding state checker for gamma lut value
URL   : https://patchwork.freedesktop.org/series/65784/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
2922e0d8ceeb drm/i915/display: Add func to get gamma bit precision
766a11160765 drm/i915/display: Add debug log for color parameters
-:21: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#21: FILE: drivers/gpu/drm/i915/display/intel_display.c:12143:
+		DRM_DEBUG_KMS("cgm_mode:%d gamma_mode:%d gamma_enable:%d csc_enable:%d\n",
+			       pipe_config->cgm_mode, pipe_config->gamma_mode,

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

total: 0 errors, 0 warnings, 2 checks, 15 lines checked
659b0a1d2d40 drm/i915/display: Add func to compare hw/sw gamma lut
-:7: WARNING:TYPO_SPELLING: 'comparsion' may be misspelled - perhaps 'comparison'?
#7: 
lut values. Since hw/sw gamma lut sizes and lut enteries comparsion

-:65: WARNING:UNNECESSARY_ELSE: else is not generally useful after a break or return
#65: FILE: drivers/gpu/drm/i915/display/intel_color.c:1497:
+			return false;
+		else

-:80: WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (16, 32)
#80: FILE: drivers/gpu/drm/i915/display/intel_color.c:1512:
+		if (!intel_color_lut_entry_equal(sw_lut, hw_lut, hw_lut_size, err))
+				return false;

-:82: WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (16, 32)
#82: FILE: drivers/gpu/drm/i915/display/intel_color.c:1514:
+		else
+				break;

total: 0 errors, 4 warnings, 0 checks, 96 lines checked
76cee29da173 drm/i915/display: Add macro to compare gamma hw/sw lut
-:29: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name1' - possible side-effects?
#29: FILE: drivers/gpu/drm/i915/display/intel_display.c:12684:
+#define PIPE_CONF_CHECK_COLOR_LUT(name1, name2, bit_precision) do { \
+	if (current_config->name1 != pipe_config->name1) { \
+		pipe_config_mismatch(fastset, __stringify(name1), \
+				"(expected %i, found %i, won't compare lut values)\n", \
+				current_config->name1, \
+				pipe_config->name1); \
+		ret = false;\
+	} else { \
+		if (!intel_color_lut_equal(current_config->name2, \
+					pipe_config->name2, pipe_config->name1, \
+					bit_precision)) { \
+			pipe_config_mismatch(fastset, __stringify(name2), \
+					"hw_state doesn't match sw_state\n"); \
+			ret = false; \
+		} \
+	} \
+} while (0)

-:29: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name1' may be better as '(name1)' to avoid precedence issues
#29: FILE: drivers/gpu/drm/i915/display/intel_display.c:12684:
+#define PIPE_CONF_CHECK_COLOR_LUT(name1, name2, bit_precision) do { \
+	if (current_config->name1 != pipe_config->name1) { \
+		pipe_config_mismatch(fastset, __stringify(name1), \
+				"(expected %i, found %i, won't compare lut values)\n", \
+				current_config->name1, \
+				pipe_config->name1); \
+		ret = false;\
+	} else { \
+		if (!intel_color_lut_equal(current_config->name2, \
+					pipe_config->name2, pipe_config->name1, \
+					bit_precision)) { \
+			pipe_config_mismatch(fastset, __stringify(name2), \
+					"hw_state doesn't match sw_state\n"); \
+			ret = false; \
+		} \
+	} \
+} while (0)

-:29: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name2' - possible side-effects?
#29: FILE: drivers/gpu/drm/i915/display/intel_display.c:12684:
+#define PIPE_CONF_CHECK_COLOR_LUT(name1, name2, bit_precision) do { \
+	if (current_config->name1 != pipe_config->name1) { \
+		pipe_config_mismatch(fastset, __stringify(name1), \
+				"(expected %i, found %i, won't compare lut values)\n", \
+				current_config->name1, \
+				pipe_config->name1); \
+		ret = false;\
+	} else { \
+		if (!intel_color_lut_equal(current_config->name2, \
+					pipe_config->name2, pipe_config->name1, \
+					bit_precision)) { \
+			pipe_config_mismatch(fastset, __stringify(name2), \
+					"hw_state doesn't match sw_state\n"); \
+			ret = false; \
+		} \
+	} \
+} while (0)

-:29: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name2' may be better as '(name2)' to avoid precedence issues
#29: FILE: drivers/gpu/drm/i915/display/intel_display.c:12684:
+#define PIPE_CONF_CHECK_COLOR_LUT(name1, name2, bit_precision) do { \
+	if (current_config->name1 != pipe_config->name1) { \
+		pipe_config_mismatch(fastset, __stringify(name1), \
+				"(expected %i, found %i, won't compare lut values)\n", \
+				current_config->name1, \
+				pipe_config->name1); \
+		ret = false;\
+	} else { \
+		if (!intel_color_lut_equal(current_config->name2, \
+					pipe_config->name2, pipe_config->name1, \
+					bit_precision)) { \
+			pipe_config_mismatch(fastset, __stringify(name2), \
+					"hw_state doesn't match sw_state\n"); \
+			ret = false; \
+		} \
+	} \
+} while (0)

total: 0 errors, 0 warnings, 4 checks, 49 lines checked
2dbaebe31071 drm/i915/display: Extract i9xx_read_luts()
-:7: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#7: 
lut values. Also, add function intel_color_lut_pack to convert hw value with

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

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

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

total: 0 errors, 4 warnings, 0 checks, 72 lines checked
615f1d72ea29 drm/i91/display: Extract i965_read_luts()
-:41: WARNING:LONG_LINE: line over 100 characters
#41: FILE: drivers/gpu/drm/i915/display/intel_color.c:1596:
+		blob_data[i].red = REG_FIELD_GET(PALETTE_RED_MASK, val1) << 8 | REG_FIELD_GET(PALETTE_RED_MASK, val2);

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

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

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

total: 0 errors, 4 warnings, 0 checks, 60 lines checked
f27642b0cd05 drm/i915/display: Extract chv_read_luts()
-:39: WARNING:LONG_LINE: line over 100 characters
#39: FILE: drivers/gpu/drm/i915/display/intel_color.c:1632:
+		blob_data[i].green = intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_GREEN_MASK, val), 10);

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

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

total: 0 errors, 3 warnings, 0 checks, 60 lines checked
5540e8e48bdc drm/i915/display: Extract ilk_read_luts()
-:40: WARNING:LONG_LINE: line over 100 characters
#40: FILE: drivers/gpu/drm/i915/display/intel_color.c:1671:
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(PREC_PALETTE_RED_MASK, val), 10);

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

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

-:64: CHECK:BRACES: Unbalanced braces around else statement
#64: FILE: drivers/gpu/drm/i915/display/intel_color.c:1736:
+		else {

total: 0 errors, 3 warnings, 1 checks, 63 lines checked
0ffafe03ab60 drm/i915/display: Extract glk_read_luts()
-:44: WARNING:LONG_LINE: line over 100 characters
#44: FILE: drivers/gpu/drm/i915/display/intel_color.c:1712:
+		blob_data[i].red = intel_color_lut_pack(REG_FIELD_GET(PREC_PAL_DATA_RED_MASK, val), 10);

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

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

total: 0 errors, 3 warnings, 0 checks, 70 lines checked
d185e0dd059f FOR_TESTING_ONLY: Print rgb values of hw and sw blobs
-:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

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

total: 0 errors, 2 warnings, 0 checks, 8 lines checked

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

✗ Fi.CI.SPARSE: warning for drm/i915: adding state checker for gamma lut value

[Patchwork]

== Series Details ==

Series: drm/i915: adding state checker for gamma lut value
URL   : https://patchwork.freedesktop.org/series/65784/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.6.0
Commit: drm/i915/display: Add func to get gamma bit precision
Okay!

Commit: drm/i915/display: Add debug log for color parameters
Okay!

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

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

Commit: drm/i915/display: Extract i9xx_read_luts()
+drivers/gpu/drm/i915/display/intel_color.c:1569:6: warning: symbol 'i9xx_read_luts' was not declared. Should it be static?

Commit: drm/i91/display: Extract i965_read_luts()
Okay!

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

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

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

Commit: FOR_TESTING_ONLY: Print rgb values of hw and sw blobs
Okay!

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

✓ Fi.CI.BAT: success for drm/i915: adding state checker for gamma lut value

[Patchwork]

== Series Details ==

Series: drm/i915: adding state checker for gamma lut value
URL   : https://patchwork.freedesktop.org/series/65784/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6783 -> Patchwork_14186
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_create@basic-files:
    - fi-bxt-dsi:         [PASS][1] -> [INCOMPLETE][2] ([fdo#103927])
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/fi-bxt-dsi/igt@gem_ctx_create@basic-files.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/fi-bxt-dsi/igt@gem_ctx_create@basic-files.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [PASS][3] -> [DMESG-WARN][4] ([fdo#102614])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  
#### Possible fixes ####

  * igt@i915_module_load@reload-no-display:
    - fi-icl-u3:          [DMESG-WARN][5] ([fdo#107724]) -> [PASS][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/fi-icl-u3/igt@i915_module_load@reload-no-display.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/fi-icl-u3/igt@i915_module_load@reload-no-display.html

  
  [fdo#102614]: https://bugs.freedesktop.org/show_bug.cgi?id=102614
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724


Participating hosts (55 -> 46)
------------------------------

  Missing    (9): fi-kbl-soraka fi-ilk-m540 fi-tgl-u fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6783 -> Patchwork_14186

  CI-20190529: 20190529
  CI_DRM_6783: c8d316e9005aee1ae6c9f2214da1c95d9c65fd5f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5149: 6756ede680ee12745393360d7cc87cc0eb733ff6 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14186: d185e0dd059fc5eabdd65228c3989109c5292d05 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

d185e0dd059f FOR_TESTING_ONLY: Print rgb values of hw and sw blobs
0ffafe03ab60 drm/i915/display: Extract glk_read_luts()
5540e8e48bdc drm/i915/display: Extract ilk_read_luts()
f27642b0cd05 drm/i915/display: Extract chv_read_luts()
615f1d72ea29 drm/i91/display: Extract i965_read_luts()
2dbaebe31071 drm/i915/display: Extract i9xx_read_luts()
76cee29da173 drm/i915/display: Add macro to compare gamma hw/sw lut
659b0a1d2d40 drm/i915/display: Add func to compare hw/sw gamma lut
766a11160765 drm/i915/display: Add debug log for color parameters
2922e0d8ceeb drm/i915/display: Add func to get gamma bit precision

== Logs ==

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

✓ Fi.CI.IGT: success for drm/i915: adding state checker for gamma lut value

[Patchwork]

== Series Details ==

Series: drm/i915: adding state checker for gamma lut value
URL   : https://patchwork.freedesktop.org/series/65784/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_6783_full -> Patchwork_14186_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_schedule@in-order-bsd:
    - shard-iclb:         [PASS][1] -> [SKIP][2] ([fdo#111325]) +2 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb7/igt@gem_exec_schedule@in-order-bsd.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb2/igt@gem_exec_schedule@in-order-bsd.html

  * igt@gem_workarounds@suspend-resume-context:
    - shard-apl:          [PASS][3] -> [DMESG-WARN][4] ([fdo#108566]) +2 similar issues
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-apl6/igt@gem_workarounds@suspend-resume-context.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-apl5/igt@gem_workarounds@suspend-resume-context.html

  * igt@kms_cursor_crc@pipe-a-cursor-suspend:
    - shard-skl:          [PASS][5] -> [INCOMPLETE][6] ([fdo#110741])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-skl7/igt@kms_cursor_crc@pipe-a-cursor-suspend.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-skl5/igt@kms_cursor_crc@pipe-a-cursor-suspend.html

  * igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-xtiled:
    - shard-skl:          [PASS][7] -> [FAIL][8] ([fdo#103184] / [fdo#103232])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-skl7/igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-xtiled.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-skl3/igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-xtiled.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [PASS][9] -> [FAIL][10] ([fdo#103167]) +1 similar issue
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb6/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_plane@pixel-format-pipe-b-planes:
    - shard-iclb:         [PASS][11] -> [INCOMPLETE][12] ([fdo#107713] / [fdo#110036 ])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb8/igt@kms_plane@pixel-format-pipe-b-planes.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb7/igt@kms_plane@pixel-format-pipe-b-planes.html

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([fdo#108145])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-skl7/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-skl3/igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min.html

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         [PASS][15] -> [FAIL][16] ([fdo#103166])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb7/igt@kms_plane_lowres@pipe-a-tiling-y.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-y.html

  * igt@kms_psr@psr2_primary_page_flip:
    - shard-iclb:         [PASS][17] -> [SKIP][18] ([fdo#109441]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb2/igt@kms_psr@psr2_primary_page_flip.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb1/igt@kms_psr@psr2_primary_page_flip.html

  * igt@kms_vblank@pipe-a-query-forked-busy-hang:
    - shard-snb:          [PASS][19] -> [SKIP][20] ([fdo#109271]) +5 similar issues
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-snb1/igt@kms_vblank@pipe-a-query-forked-busy-hang.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-snb2/igt@kms_vblank@pipe-a-query-forked-busy-hang.html

  * igt@kms_vblank@pipe-b-query-busy:
    - shard-apl:          [PASS][21] -> [INCOMPLETE][22] ([fdo#103927]) +1 similar issue
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-apl2/igt@kms_vblank@pipe-b-query-busy.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-apl3/igt@kms_vblank@pipe-b-query-busy.html

  * igt@kms_vblank@pipe-c-query-busy:
    - shard-iclb:         [PASS][23] -> [INCOMPLETE][24] ([fdo#107713])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb2/igt@kms_vblank@pipe-c-query-busy.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb1/igt@kms_vblank@pipe-c-query-busy.html

  * igt@perf@blocking:
    - shard-skl:          [PASS][25] -> [FAIL][26] ([fdo#110728])
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-skl1/igt@perf@blocking.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-skl8/igt@perf@blocking.html

  * igt@prime_vgem@fence-wait-bsd2:
    - shard-iclb:         [PASS][27] -> [SKIP][28] ([fdo#109276]) +10 similar issues
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb1/igt@prime_vgem@fence-wait-bsd2.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb5/igt@prime_vgem@fence-wait-bsd2.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@bcs0-s3:
    - shard-apl:          [DMESG-WARN][29] ([fdo#108566]) -> [PASS][30] +5 similar issues
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-apl7/igt@gem_ctx_isolation@bcs0-s3.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-apl4/igt@gem_ctx_isolation@bcs0-s3.html

  * igt@gem_ctx_shared@exec-single-timeline-bsd:
    - shard-iclb:         [SKIP][31] ([fdo#110841]) -> [PASS][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb2/igt@gem_ctx_shared@exec-single-timeline-bsd.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb8/igt@gem_ctx_shared@exec-single-timeline-bsd.html

  * igt@gem_exec_schedule@preempt-other-bsd:
    - shard-iclb:         [SKIP][33] ([fdo#111325]) -> [PASS][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb2/igt@gem_exec_schedule@preempt-other-bsd.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb7/igt@gem_exec_schedule@preempt-other-bsd.html

  * igt@gem_exec_schedule@preempt-queue-chain-bsd2:
    - shard-iclb:         [SKIP][35] ([fdo#109276]) -> [PASS][36] +6 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb8/igt@gem_exec_schedule@preempt-queue-chain-bsd2.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb2/igt@gem_exec_schedule@preempt-queue-chain-bsd2.html

  * igt@kms_color@pipe-b-ctm-0-25:
    - shard-skl:          [FAIL][37] ([fdo#108682]) -> [PASS][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-skl10/igt@kms_color@pipe-b-ctm-0-25.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-skl10/igt@kms_color@pipe-b-ctm-0-25.html

  * igt@kms_draw_crc@draw-method-xrgb8888-render-ytiled:
    - shard-apl:          [INCOMPLETE][39] ([fdo#103927]) -> [PASS][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-apl4/igt@kms_draw_crc@draw-method-xrgb8888-render-ytiled.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-apl6/igt@kms_draw_crc@draw-method-xrgb8888-render-ytiled.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible:
    - shard-skl:          [FAIL][41] ([fdo#105363]) -> [PASS][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-skl7/igt@kms_flip@flip-vs-expired-vblank-interruptible.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-skl5/igt@kms_flip@flip-vs-expired-vblank-interruptible.html

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render:
    - shard-iclb:         [FAIL][43] ([fdo#103167]) -> [PASS][44] +7 similar issues
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb3/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb1/igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-render.html

  * igt@kms_plane_alpha_blend@pipe-b-coverage-7efc:
    - shard-skl:          [FAIL][45] ([fdo#108145] / [fdo#110403]) -> [PASS][46]
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-skl10/igt@kms_plane_alpha_blend@pipe-b-coverage-7efc.html

  * igt@kms_plane_lowres@pipe-a-tiling-x:
    - shard-iclb:         [FAIL][47] ([fdo#103166]) -> [PASS][48]
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb8/igt@kms_plane_lowres@pipe-a-tiling-x.html
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb2/igt@kms_plane_lowres@pipe-a-tiling-x.html

  * igt@kms_psr@psr2_no_drrs:
    - shard-iclb:         [SKIP][49] ([fdo#109441]) -> [PASS][50] +2 similar issues
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb8/igt@kms_psr@psr2_no_drrs.html
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb2/igt@kms_psr@psr2_no_drrs.html

  * igt@kms_vblank@pipe-b-query-busy:
    - shard-iclb:         [INCOMPLETE][51] ([fdo#107713]) -> [PASS][52]
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb7/igt@kms_vblank@pipe-b-query-busy.html
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb5/igt@kms_vblank@pipe-b-query-busy.html

  
#### Warnings ####

  * igt@gem_mocs_settings@mocs-isolation-bsd2:
    - shard-iclb:         [SKIP][53] ([fdo#109276]) -> [FAIL][54] ([fdo#111330])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb7/igt@gem_mocs_settings@mocs-isolation-bsd2.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb2/igt@gem_mocs_settings@mocs-isolation-bsd2.html

  * igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait:
    - shard-iclb:         [SKIP][55] ([fdo#110892]) -> [INCOMPLETE][56] ([fdo#107713] / [fdo#108840])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-iclb8/igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-iclb7/igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-indfb-fliptrack:
    - shard-skl:          [FAIL][57] ([fdo#103167]) -> [FAIL][58] ([fdo#108040])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-skl7/igt@kms_frontbuffer_tracking@fbcpsr-1p-indfb-fliptrack.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-skl3/igt@kms_frontbuffer_tracking@fbcpsr-1p-indfb-fliptrack.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-fullscreen:
    - shard-skl:          [FAIL][59] ([fdo#108040]) -> [FAIL][60] ([fdo#103167])
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_6783/shard-skl7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-fullscreen.html
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_14186/shard-skl3/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-fullscreen.html

  
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103184]: https://bugs.freedesktop.org/show_bug.cgi?id=103184
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#108040]: https://bugs.freedesktop.org/show_bug.cgi?id=108040
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108682]: https://bugs.freedesktop.org/show_bug.cgi?id=108682
  [fdo#108840]: https://bugs.freedesktop.org/show_bug.cgi?id=108840
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110036 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110036 
  [fdo#110403]: https://bugs.freedesktop.org/show_bug.cgi?id=110403
  [fdo#110728]: https://bugs.freedesktop.org/show_bug.cgi?id=110728
  [fdo#110741]: https://bugs.freedesktop.org/show_bug.cgi?id=110741
  [fdo#110841]: https://bugs.freedesktop.org/show_bug.cgi?id=110841
  [fdo#110892]: https://bugs.freedesktop.org/show_bug.cgi?id=110892
  [fdo#111325]: https://bugs.freedesktop.org/show_bug.cgi?id=111325
  [fdo#111330]: https://bugs.freedesktop.org/show_bug.cgi?id=111330


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

  No changes in participating hosts


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_6783 -> Patchwork_14186

  CI-20190529: 20190529
  CI_DRM_6783: c8d316e9005aee1ae6c9f2214da1c95d9c65fd5f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5149: 6756ede680ee12745393360d7cc87cc0eb733ff6 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_14186: d185e0dd059fc5eabdd65228c3989109c5292d05 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [v8][PATCH 00/10] drm/i915: adding state checker for gamma lut value

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Swati2
>Sent: Monday, August 26, 2019 11:56 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Nikula, Jani <jani.nikula@intel.com>; Sharma, Shashank
><shashank.sharma@intel.com>; Manna, Animesh <animesh.manna@intel.com>;
>Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; daniel.vetter@ffwll.ch;
>ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; Sharma,
>Swati2 <swati2.sharma@intel.com>
>Subject: [v8][PATCH 00/10] drm/i915: adding state checker for gamma lut value

The series name is changed "value" instead of "values". This makes it a new series
for patchwork. Please fix that in the next version and lets maintain the name whatever
has been there from the beginning.

>In this patch series, added state checker to validate gamma (8BIT and 10BIT).
>This reads hardware state, and compares the originally requested state(s/w) to the
>state read from the hardware.
>This is done for legacy, i965, ilk, glk and their variant platforms.
>
>Intentionally, excluded bdw and ivb since they have spilt gamma mode; for which
>degamma read outs are required (which I think shouldn't be included in this patch
>series). Will include after degamma state checker is completed.
>
>v1: -Implementation done for legacy platforms
>     (removed all the placeholders) (Jani)
>v2: -Restructured code and created platform specific patch series for
>     gamma validation
>v3: -Rebase
>v4: -Minor changes-function name changes mainly
>v5: -Added degamma validation (Ville)
>v6: -Removed degamma changes, debugging was becoming difficult
>    -Added function to assign bit_precision for gamma/degamma
>     lut values /platform
>    -Added debug info into intel_dump_pipe_config() (Jani)
>v7: -Added platform specific functions to compute gamma bit precision
>     on the basis of GAMMA_MODE (Ville)
>    -Corrected checkpatch warnings
>v8: -Restructured code
>    -Removed bdw and ivb platform state checker
>
>Swati Sharma (10):
>  drm/i915/display: Add func to get gamma bit precision
>  drm/i915/display: Add debug log for color parameters
>  drm/i915/display: Add func to compare hw/sw gamma lut
>  drm/i915/display: Add macro to compare gamma hw/sw lut
>  drm/i915/display: Extract i9xx_read_luts()
>  drm/i91/display: Extract i965_read_luts()
>  drm/i915/display: Extract chv_read_luts()
>  drm/i915/display: Extract ilk_read_luts()
>  drm/i915/display: Extract glk_read_luts()
>  FOR_TESTING_ONLY: Print rgb values of hw and sw blobs
>
> drivers/gpu/drm/i915/display/intel_color.c   | 370 ++++++++++++++++++++++++++-
> drivers/gpu/drm/i915/display/intel_color.h   |   7 +
> drivers/gpu/drm/i915/display/intel_display.c |  34 +++
> drivers/gpu/drm/i915/i915_reg.h              |  15 ++
> 4 files changed, 423 insertions(+), 3 deletions(-)
>
>--
>1.9.1

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

Re: [v8][PATCH 07/10] drm/i915/display: Extract chv_read_luts()

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Swati2
>Sent: Monday, August 26, 2019 11:56 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Nikula, Jani <jani.nikula@intel.com>; Sharma, Shashank
><shashank.sharma@intel.com>; Manna, Animesh <animesh.manna@intel.com>;
>Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; daniel.vetter@ffwll.ch;
>ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; Sharma,
>Swati2 <swati2.sharma@intel.com>
>Subject: [v8][PATCH 07/10] drm/i915/display: Extract chv_read_luts()
>
>For cherryview, have hw read out to create hw blob of gamma lut values.

Same comments as previous patch.

>Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_color.c | 39 ++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_reg.h            |  3 +++
> 2 files changed, 42 insertions(+)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_color.c
>b/drivers/gpu/drm/i915/display/intel_color.c
>index c77bbed..1ec2fa0 100644
>--- a/drivers/gpu/drm/i915/display/intel_color.c
>+++ b/drivers/gpu/drm/i915/display/intel_color.c
>@@ -1609,6 +1609,44 @@ static void i965_read_luts(struct intel_crtc_state
>*crtc_state)
> 		crtc_state->base.gamma_lut =
>i965_read_gamma_lut_10p6(crtc_state);
> }
>
>+static struct drm_property_blob *
>+chv_read_cgm_gamma_lut(struct intel_crtc_state *crtc_state) {

Make it const. If planning to use same for degamma as well, drop gamma
from function name to have it generic.

>+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>+	u32 i, val, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
>+	enum pipe pipe = crtc->pipe;
>+	struct drm_property_blob *blob;
>+	struct drm_color_lut *blob_data;
>+
>+	blob = drm_property_create_blob(&dev_priv->drm,
>+					sizeof(struct drm_color_lut) * lut_size,
>+					NULL);
>+	if (IS_ERR(blob))
>+		return NULL;
>+
>+	blob_data = blob->data;
>+
>+	for (i = 0; i < lut_size; i++) {
>+		val = I915_READ(CGM_PIPE_GAMMA(pipe, i, 0));
>+		blob_data[i].green =
>intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_GREEN_MASK, val), 10);
>+		blob_data[i].blue =
>+intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_BLUE_MASK, val), 10);

Wrap these lines.

>+
>+		val = I915_READ(CGM_PIPE_GAMMA(pipe, i, 1));
>+		blob_data[i].red =
>intel_color_lut_pack(REG_FIELD_GET(CGM_PIPE_GAMMA_RED_MASK, val), 10);
>+	}
>+
>+	return blob;
>+}
>+
>+static void chv_read_luts(struct intel_crtc_state *crtc_state) {
>+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
>+		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
>+	else
>+		crtc_state->base.gamma_lut =
>chv_read_cgm_gamma_lut(crtc_state);
>+}
>+
> void intel_color_init(struct intel_crtc *crtc)  {
> 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -1621,6
>+1659,7 @@ void intel_color_init(struct intel_crtc *crtc)
> 			dev_priv->display.color_check = chv_color_check;
> 			dev_priv->display.color_commit = i9xx_color_commit;
> 			dev_priv->display.load_luts = chv_load_luts;
>+			dev_priv->display.read_luts = chv_read_luts;
> 		} else if (INTEL_GEN(dev_priv) >= 4) {
> 			dev_priv->display.color_check = i9xx_color_check;
> 			dev_priv->display.color_commit = i9xx_color_commit; diff --
>git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index
>b30b0c6b..e76e779 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -10307,6 +10307,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)
>--
>1.9.1

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

Re: [v8][PATCH 08/10] drm/i915/display: Extract ilk_read_luts()

[Shankar, Uma]

>-----Original Message-----
>From: Sharma, Swati2
>Sent: Monday, August 26, 2019 11:56 AM
>To: intel-gfx@lists.freedesktop.org
>Cc: Nikula, Jani <jani.nikula@intel.com>; Sharma, Shashank
><shashank.sharma@intel.com>; Manna, Animesh <animesh.manna@intel.com>;
>Nautiyal, Ankit K <ankit.k.nautiyal@intel.com>; daniel.vetter@ffwll.ch;
>ville.syrjala@linux.intel.com; Shankar, Uma <uma.shankar@intel.com>; Sharma,
>Swati2 <swati2.sharma@intel.com>
>Subject: [v8][PATCH 08/10] drm/i915/display: Extract ilk_read_luts()
>
>For ilk, have hw read out to create hw blob of gamma lut values.

Same as earlier patch.

>Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
>---
> drivers/gpu/drm/i915/display/intel_color.c | 41 +++++++++++++++++++++++++++++-
> drivers/gpu/drm/i915/i915_reg.h            |  3 +++
> 2 files changed, 43 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/display/intel_color.c
>b/drivers/gpu/drm/i915/display/intel_color.c
>index 1ec2fa0..4b9b28f 100644
>--- a/drivers/gpu/drm/i915/display/intel_color.c
>+++ b/drivers/gpu/drm/i915/display/intel_color.c
>@@ -1647,6 +1647,43 @@ static void chv_read_luts(struct intel_crtc_state
>*crtc_state)
> 		crtc_state->base.gamma_lut =
>chv_read_cgm_gamma_lut(crtc_state);
> }
>
>+static struct drm_property_blob *
>+ilk_read_gamma_lut(struct intel_crtc_state *crtc_state) {

Rename this to " ilk_read_lut_10()" as per Ville's suggestion.
Also add const.

>+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>+	u32 i, val, lut_size = INTEL_INFO(dev_priv)->color.gamma_lut_size;
>+	enum pipe pipe = crtc->pipe;
>+	struct drm_property_blob *blob;
>+	struct drm_color_lut *blob_data;
>+
>+	blob = drm_property_create_blob(&dev_priv->drm,
>+					sizeof(struct drm_color_lut) * lut_size,
>+					NULL);
>+	if (IS_ERR(blob))
>+		return NULL;
>+
>+	blob_data = blob->data;
>+
>+	for (i = 0; i < lut_size - 1; i++) {
>+		val = I915_READ(PREC_PALETTE(pipe, i));
>+
>+		blob_data[i].red =
>intel_color_lut_pack(REG_FIELD_GET(PREC_PALETTE_RED_MASK, val), 10);
>+		blob_data[i].green =
>intel_color_lut_pack(REG_FIELD_GET(PREC_PALETTE_GREEN_MASK, val), 10);
>+		blob_data[i].blue =
>intel_color_lut_pack(REG_FIELD_GET(PREC_PALETTE_BLUE_MASK, val), 10);
>+	}
>+
>+	return blob;
>+}
>+
>+static void ilk_read_luts(struct intel_crtc_state *crtc_state) {
>+	if (crtc_state->gamma_mode == GAMMA_MODE_MODE_8BIT)
>+		crtc_state->base.gamma_lut = i9xx_read_lut_8(crtc_state);
>+	else
>+		crtc_state->base.gamma_lut = ilk_read_gamma_lut(crtc_state); }
>+
> void intel_color_init(struct intel_crtc *crtc)  {
> 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); @@ -1696,8
>+1733,10 @@ void intel_color_init(struct intel_crtc *crtc)
> 			dev_priv->display.load_luts = bdw_load_luts;
> 		else if (INTEL_GEN(dev_priv) >= 7)
> 			dev_priv->display.load_luts = ivb_load_luts;
>-		else
>+		else {

This make unbalanced braces. Add braces for above else cases as well.

> 			dev_priv->display.load_luts = ilk_load_luts;
>+			dev_priv->display.read_luts = ilk_read_luts;
>+		}
> 	}
>
> 	drm_crtc_enable_color_mgmt(&crtc->base,
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index
>e76e779..acc9239 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -7203,6 +7203,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
>--
>1.9.1

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