[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Add state checker for CSC coeff values

[Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: Add state checker for CSC coeff values

[Patchwork]

== Series Details ==

Series: drm/i915: Add state checker for CSC coeff values
URL   : https://patchwork.freedesktop.org/series/88242/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
6b4acba31287 drm/i915/display: Introduce vfunc read_csc() to create hw ctm
36688ee5a039 drm/i915/display: Add func to compare hw/sw CSC matrix
-:100: CHECK:PARENTHESIS_ALIGNMENT: Alignment should match open parenthesis
#100: FILE: drivers/gpu/drm/i915/display/intel_color.h:25:
+bool intel_csc_equal(const struct intel_crtc_state *crtc_state,
+			   struct drm_property_blob *blob1,

total: 0 errors, 0 warnings, 1 checks, 80 lines checked
9330fece07c8 drm/i915/display: Add macro to compare hw/sw CSC matrix
-:23: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name1' - possible side-effects?
#23: FILE: drivers/gpu/drm/i915/display/intel_display.c:8928:
+#define PIPE_CONF_CHECK_CSC(name1, name2, name3) do { \
+	if (current_config->name1 != pipe_config->name1) { \
+		pipe_config_mismatch(fastset, crtc, __stringify(name1), \
+				"(expected %s, found %s, won't compare csc coeffs)", \
+				yesno(current_config->name1), \
+				yesno(pipe_config->name1)); \
+		ret = false;\
+	} else if (current_config->name2 != pipe_config->name2) { \
+		pipe_config_mismatch(fastset, crtc, __stringify(name2), \
+				"(expected %i, found %i, won't compare csc coeffs)", \
+				current_config->name2, \
+				pipe_config->name2); \
+		ret = false;\
+	} else { \
+		if (!intel_csc_equal(current_config, current_config->name3, \
+				pipe_config->name3)) { \
+			pipe_config_mismatch(fastset, crtc, __stringify(name3), \
+					"hw_state doesn't match sw_state"); \
+			ret = false; \
+		} \
+	} \
+} while (0)

-:23: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name1' may be better as '(name1)' to avoid precedence issues
#23: FILE: drivers/gpu/drm/i915/display/intel_display.c:8928:
+#define PIPE_CONF_CHECK_CSC(name1, name2, name3) do { \
+	if (current_config->name1 != pipe_config->name1) { \
+		pipe_config_mismatch(fastset, crtc, __stringify(name1), \
+				"(expected %s, found %s, won't compare csc coeffs)", \
+				yesno(current_config->name1), \
+				yesno(pipe_config->name1)); \
+		ret = false;\
+	} else if (current_config->name2 != pipe_config->name2) { \
+		pipe_config_mismatch(fastset, crtc, __stringify(name2), \
+				"(expected %i, found %i, won't compare csc coeffs)", \
+				current_config->name2, \
+				pipe_config->name2); \
+		ret = false;\
+	} else { \
+		if (!intel_csc_equal(current_config, current_config->name3, \
+				pipe_config->name3)) { \
+			pipe_config_mismatch(fastset, crtc, __stringify(name3), \
+					"hw_state doesn't match sw_state"); \
+			ret = false; \
+		} \
+	} \
+} while (0)

-:23: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name2' - possible side-effects?
#23: FILE: drivers/gpu/drm/i915/display/intel_display.c:8928:
+#define PIPE_CONF_CHECK_CSC(name1, name2, name3) do { \
+	if (current_config->name1 != pipe_config->name1) { \
+		pipe_config_mismatch(fastset, crtc, __stringify(name1), \
+				"(expected %s, found %s, won't compare csc coeffs)", \
+				yesno(current_config->name1), \
+				yesno(pipe_config->name1)); \
+		ret = false;\
+	} else if (current_config->name2 != pipe_config->name2) { \
+		pipe_config_mismatch(fastset, crtc, __stringify(name2), \
+				"(expected %i, found %i, won't compare csc coeffs)", \
+				current_config->name2, \
+				pipe_config->name2); \
+		ret = false;\
+	} else { \
+		if (!intel_csc_equal(current_config, current_config->name3, \
+				pipe_config->name3)) { \
+			pipe_config_mismatch(fastset, crtc, __stringify(name3), \
+					"hw_state doesn't match sw_state"); \
+			ret = false; \
+		} \
+	} \
+} while (0)

-:23: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name2' may be better as '(name2)' to avoid precedence issues
#23: FILE: drivers/gpu/drm/i915/display/intel_display.c:8928:
+#define PIPE_CONF_CHECK_CSC(name1, name2, name3) do { \
+	if (current_config->name1 != pipe_config->name1) { \
+		pipe_config_mismatch(fastset, crtc, __stringify(name1), \
+				"(expected %s, found %s, won't compare csc coeffs)", \
+				yesno(current_config->name1), \
+				yesno(pipe_config->name1)); \
+		ret = false;\
+	} else if (current_config->name2 != pipe_config->name2) { \
+		pipe_config_mismatch(fastset, crtc, __stringify(name2), \
+				"(expected %i, found %i, won't compare csc coeffs)", \
+				current_config->name2, \
+				pipe_config->name2); \
+		ret = false;\
+	} else { \
+		if (!intel_csc_equal(current_config, current_config->name3, \
+				pipe_config->name3)) { \
+			pipe_config_mismatch(fastset, crtc, __stringify(name3), \
+					"hw_state doesn't match sw_state"); \
+			ret = false; \
+		} \
+	} \
+} while (0)

-:23: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'name3' - possible side-effects?
#23: FILE: drivers/gpu/drm/i915/display/intel_display.c:8928:
+#define PIPE_CONF_CHECK_CSC(name1, name2, name3) do { \
+	if (current_config->name1 != pipe_config->name1) { \
+		pipe_config_mismatch(fastset, crtc, __stringify(name1), \
+				"(expected %s, found %s, won't compare csc coeffs)", \
+				yesno(current_config->name1), \
+				yesno(pipe_config->name1)); \
+		ret = false;\
+	} else if (current_config->name2 != pipe_config->name2) { \
+		pipe_config_mismatch(fastset, crtc, __stringify(name2), \
+				"(expected %i, found %i, won't compare csc coeffs)", \
+				current_config->name2, \
+				pipe_config->name2); \
+		ret = false;\
+	} else { \
+		if (!intel_csc_equal(current_config, current_config->name3, \
+				pipe_config->name3)) { \
+			pipe_config_mismatch(fastset, crtc, __stringify(name3), \
+					"hw_state doesn't match sw_state"); \
+			ret = false; \
+		} \
+	} \
+} while (0)

-:23: CHECK:MACRO_ARG_PRECEDENCE: Macro argument 'name3' may be better as '(name3)' to avoid precedence issues
#23: FILE: drivers/gpu/drm/i915/display/intel_display.c:8928:
+#define PIPE_CONF_CHECK_CSC(name1, name2, name3) do { \
+	if (current_config->name1 != pipe_config->name1) { \
+		pipe_config_mismatch(fastset, crtc, __stringify(name1), \
+				"(expected %s, found %s, won't compare csc coeffs)", \
+				yesno(current_config->name1), \
+				yesno(pipe_config->name1)); \
+		ret = false;\
+	} else if (current_config->name2 != pipe_config->name2) { \
+		pipe_config_mismatch(fastset, crtc, __stringify(name2), \
+				"(expected %i, found %i, won't compare csc coeffs)", \
+				current_config->name2, \
+				pipe_config->name2); \
+		ret = false;\
+	} else { \
+		if (!intel_csc_equal(current_config, current_config->name3, \
+				pipe_config->name3)) { \
+			pipe_config_mismatch(fastset, crtc, __stringify(name3), \
+					"hw_state doesn't match sw_state"); \
+			ret = false; \
+		} \
+	} \
+} while (0)

total: 0 errors, 0 warnings, 6 checks, 49 lines checked
9aaab4f5f76f drm/i915/display: Extract chv_read_csc()
44ae7fddb56d drm/i915/display: Extract ilk_read_csc()
260c2b3ee5a6 drm/i915/display: Extract icl_read_csc()
a629803a4aa1 FOR_TESTING_ONLY: Print coeffs of hw and sw CTM
-:7: WARNING:COMMIT_MESSAGE: Missing commit description - Add an appropriate one

-:26: WARNING:LONG_LINE: line length of 116 exceeds 100 columns
#26: FILE: drivers/gpu/drm/i915/display/intel_color.c:1929:
+		DRM_DEBUG_KMS("Bhanu debug--> sw_ctm[%d]=0x%x hw_ctm[%d]=0x%llx", i, coeffs[i], i, ctm2->matrix[i]);

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


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

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: Add state checker for CSC coeff values

[Patchwork]

== Series Details ==

Series: drm/i915: Add state checker for CSC coeff values
URL   : https://patchwork.freedesktop.org/series/88242/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
-
+drivers/gpu/drm/i915/gt/intel_reset.c:1323:5: warning: context imbalance in 'intel_gt_reset_trylock' - different lock contexts for basic block
+drivers/gpu/drm/i915/gvt/mmio.c:295:23: warning: memcpy with byte count of 279040
+drivers/gpu/drm/i915/i915_perf.c:1434:15: warning: memset with byte count of 16777216
+drivers/gpu/drm/i915/i915_perf.c:1488:15: warning: memset with byte count of 16777216
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen11_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen12_fwtable_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read64' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_read8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen6_write8' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write16' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write32' - different lock contexts for basic block
+./include/linux/spinlock.h:409:9: warning: context imbalance in 'gen8_write8' - different lock contexts for basic block


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

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Add state checker for CSC coeff values

[Patchwork]

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

== Series Details ==

Series: drm/i915: Add state checker for CSC coeff values
URL   : https://patchwork.freedesktop.org/series/88242/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_9879 -> Patchwork_19820
====================================================

Summary
-------

  **FAILURE**

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

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

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@runner@aborted:
    - fi-cml-u2:          NOTRUN -> [FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19820/fi-cml-u2/igt@runner@aborted.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@hangcheck:
    - fi-snb-2600:        [PASS][2] -> [INCOMPLETE][3] ([i915#2782])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9879/fi-snb-2600/igt@i915_selftest@live@hangcheck.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19820/fi-snb-2600/igt@i915_selftest@live@hangcheck.html

  * igt@i915_selftest@live@late_gt_pm:
    - fi-bsw-nick:        [PASS][4] -> [DMESG-FAIL][5] ([i915#2927])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9879/fi-bsw-nick/igt@i915_selftest@live@late_gt_pm.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19820/fi-bsw-nick/igt@i915_selftest@live@late_gt_pm.html

  * igt@runner@aborted:
    - fi-kbl-x1275:       NOTRUN -> [FAIL][6] ([i915#1569] / [i915#192] / [i915#193] / [i915#194])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19820/fi-kbl-x1275/igt@runner@aborted.html
    - fi-bsw-nick:        NOTRUN -> [FAIL][7] ([i915#1436])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19820/fi-bsw-nick/igt@runner@aborted.html
    - fi-kbl-7500u:       NOTRUN -> [FAIL][8] ([i915#1569] / [i915#192] / [i915#193] / [i915#194])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19820/fi-kbl-7500u/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@gem_tiled_blits@basic:
    - fi-kbl-8809g:       [TIMEOUT][9] ([i915#2502] / [i915#3145]) -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9879/fi-kbl-8809g/igt@gem_tiled_blits@basic.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19820/fi-kbl-8809g/igt@gem_tiled_blits@basic.html

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

  [i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
  [i915#1569]: https://gitlab.freedesktop.org/drm/intel/issues/1569
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#192]: https://gitlab.freedesktop.org/drm/intel/issues/192
  [i915#193]: https://gitlab.freedesktop.org/drm/intel/issues/193
  [i915#194]: https://gitlab.freedesktop.org/drm/intel/issues/194
  [i915#2502]: https://gitlab.freedesktop.org/drm/intel/issues/2502
  [i915#2782]: https://gitlab.freedesktop.org/drm/intel/issues/2782
  [i915#2927]: https://gitlab.freedesktop.org/drm/intel/issues/2927
  [i915#3145]: https://gitlab.freedesktop.org/drm/intel/issues/3145
  [i915#3180]: https://gitlab.freedesktop.org/drm/intel/issues/3180


Participating hosts (43 -> 40)
------------------------------

  Missing    (3): fi-ilk-m540 fi-bsw-cyan fi-bdw-samus 


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

  * Linux: CI_DRM_9879 -> Patchwork_19820

  CI-20190529: 20190529
  CI_DRM_9879: 892960f907bece8a020b30ade522352ded06c94c @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6039: 8c4a2cda2a92bdd87797969ef299ad7f6e8e993b @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_19820: a629803a4aa1a1a5b767483dad6d33504f632f97 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

a629803a4aa1 FOR_TESTING_ONLY: Print coeffs of hw and sw CTM
260c2b3ee5a6 drm/i915/display: Extract icl_read_csc()
44ae7fddb56d drm/i915/display: Extract ilk_read_csc()
9aaab4f5f76f drm/i915/display: Extract chv_read_csc()
9330fece07c8 drm/i915/display: Add macro to compare hw/sw CSC matrix
36688ee5a039 drm/i915/display: Add func to compare hw/sw CSC matrix
6b4acba31287 drm/i915/display: Introduce vfunc read_csc() to create hw ctm

== Logs ==

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

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

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

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

[Intel-gfx] [PATCH 0/7] drm/i915: Add state checker for CSC coeff values

[Bhanuprakash Modem]

In this patch series, added state checker to validate CSC. This reads
hardware state, and compares the originally requested state(s/w). This
is done for chv, ilk, glk and their variant platforms. Rest of
the platforms will be enabled on top of this later.

Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>

Bhanuprakash Modem (7):
  drm/i915/display: Introduce vfunc read_csc() to create hw ctm
  drm/i915/display: Add func to compare hw/sw CSC matrix
  drm/i915/display: Add macro to compare hw/sw CSC matrix
  drm/i915/display: Extract chv_read_csc()
  drm/i915/display: Extract ilk_read_csc()
  drm/i915/display: Extract icl_read_csc()
  FOR_TESTING_ONLY: Print coeffs of hw and sw CTM

 drivers/gpu/drm/i915/display/intel_color.c   | 209 ++++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_color.h   |   3 +
 drivers/gpu/drm/i915/display/intel_display.c |  31 +++
 drivers/gpu/drm/i915/i915_drv.h              |   1 +
 4 files changed, 239 insertions(+), 5 deletions(-)

--
2.20.1

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

[Intel-gfx] [PATCH 1/7] drm/i915/display: Introduce vfunc read_csc() to create hw ctm

[Bhanuprakash Modem]

In this patch, a vfunc read_csc() is introduced to create a hw ctm
i.e. ctm having values read from CSC registers which will later be
used to compare with sw ctm to validate CSC coeff values.

Cc: Swati Sharma <swati2.sharma@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 3 +++
 drivers/gpu/drm/i915/i915_drv.h            | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index ff7dcb7088bf..17bb08e9cf64 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1209,6 +1209,9 @@ void intel_color_get_config(struct intel_crtc_state *crtc_state)
 
 	if (dev_priv->display.read_luts)
 		dev_priv->display.read_luts(crtc_state);
+
+	if (dev_priv->display.read_csc)
+		dev_priv->display.read_csc(crtc_state);
 }
 
 static bool need_plane_update(struct intel_plane *plane,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 214fe10eb092..a8b55aca5fc4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -327,6 +327,7 @@ struct drm_i915_display_funcs {
 	 */
 	void (*load_luts)(const struct intel_crtc_state *crtc_state);
 	void (*read_luts)(struct intel_crtc_state *crtc_state);
+	void (*read_csc)(struct intel_crtc_state *crtc_state);
 };
 
 struct intel_csr {
-- 
2.20.1

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

[Intel-gfx] [PATCH 2/7] drm/i915/display: Add func to compare hw/sw CSC matrix

[Bhanuprakash Modem]

Add func intel_csc_equal() to compare hw/sw CSC coeff values.

Cc: Swati Sharma <swati2.sharma@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 51 +++++++++++++++++++---
 drivers/gpu/drm/i915/display/intel_color.h |  3 ++
 2 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 17bb08e9cf64..54dfd3523272 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -348,16 +348,11 @@ static void icl_load_csc_matrix(const struct intel_crtc_state *crtc_state)
 		       crtc_state->csc_mode);
 }
 
-static void chv_load_cgm_csc(struct intel_crtc *crtc,
-			     const struct drm_property_blob *blob)
+static void chv_csc_convert_ctm(const struct drm_color_ctm *ctm, u16 coeffs[9])
 {
-	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
-	const struct drm_color_ctm *ctm = blob->data;
-	enum pipe pipe = crtc->pipe;
-	u16 coeffs[9];
 	int i;
 
-	for (i = 0; i < ARRAY_SIZE(coeffs); i++) {
+	for (i = 0; i < 9; i++) {
 		u64 abs_coeff = ((1ULL << 63) - 1) & ctm->matrix[i];
 
 		/* Round coefficient. */
@@ -374,6 +369,17 @@ static void chv_load_cgm_csc(struct intel_crtc *crtc,
 		coeffs[i] |= ((abs_coeff >> 32) & 7) << 12;
 		coeffs[i] |= (abs_coeff >> 20) & 0xfff;
 	}
+}
+
+static void chv_load_cgm_csc(struct intel_crtc *crtc,
+			     const struct drm_property_blob *blob)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+	const struct drm_color_ctm *ctm = blob->data;
+	enum pipe pipe = crtc->pipe;
+	u16 coeffs[9];
+
+	chv_csc_convert_ctm(ctm, coeffs);
 
 	intel_de_write(dev_priv, CGM_PIPE_CSC_COEFF01(pipe),
 		       coeffs[1] << 16 | coeffs[0]);
@@ -1792,6 +1798,37 @@ bool intel_color_lut_equal(struct drm_property_blob *blob1,
 	return true;
 }
 
+bool intel_csc_equal(const struct intel_crtc_state *current_config,
+		     struct drm_property_blob *blob1,
+		     struct drm_property_blob *blob2)
+{
+	struct drm_i915_private *dev_priv = to_i915(current_config->uapi.crtc->dev);
+	struct drm_color_ctm *ctm1, *ctm2;
+	u16 coeffs[9];
+	int i;
+
+	if (!blob1 != !blob2)
+		return false;
+
+	if (!blob1)
+		return true;
+
+	ctm1 = blob1->data;
+	ctm2 = blob2->data;
+
+	if (IS_CHERRYVIEW(dev_priv))
+		chv_csc_convert_ctm(ctm1, coeffs);
+	else
+		ilk_csc_convert_ctm(current_config, coeffs);
+
+	for (i = 0; i < 9; i++) {
+		if (abs(coeffs[i] - ctm2->matrix[i]))
+			return false;
+	}
+
+	return true;
+}
+
 static struct drm_property_blob *i9xx_read_lut_8(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 173727aaa24d..e6bd9aa0c04a 100644
--- a/drivers/gpu/drm/i915/display/intel_color.h
+++ b/drivers/gpu/drm/i915/display/intel_color.h
@@ -21,5 +21,8 @@ int intel_color_get_gamma_bit_precision(const struct intel_crtc_state *crtc_stat
 bool intel_color_lut_equal(struct drm_property_blob *blob1,
 			   struct drm_property_blob *blob2,
 			   u32 gamma_mode, u32 bit_precision);
+bool intel_csc_equal(const struct intel_crtc_state *crtc_state,
+			   struct drm_property_blob *blob1,
+			   struct drm_property_blob *blob2);
 
 #endif /* __INTEL_COLOR_H__ */
-- 
2.20.1

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

[Intel-gfx] [PATCH 3/7] drm/i915/display: Add macro to compare hw/sw CSC matrix

[Bhanuprakash Modem]

Add macro to compare hw/sw CSC coeff values. First need to
check whether hw/sw csc enable and csc mode matches or not.
If not no need to compare coeff values, if matches then only
compare.

Cc: Swati Sharma <swati2.sharma@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 drivers/gpu/drm/i915/display/intel_display.c | 31 ++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 7b38b9a38b85..c4b9d4a238b9 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -8925,6 +8925,29 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 	} \
 } while (0)
 
+#define PIPE_CONF_CHECK_CSC(name1, name2, name3) do { \
+	if (current_config->name1 != pipe_config->name1) { \
+		pipe_config_mismatch(fastset, crtc, __stringify(name1), \
+				"(expected %s, found %s, won't compare csc coeffs)", \
+				yesno(current_config->name1), \
+				yesno(pipe_config->name1)); \
+		ret = false;\
+	} else if (current_config->name2 != pipe_config->name2) { \
+		pipe_config_mismatch(fastset, crtc, __stringify(name2), \
+				"(expected %i, found %i, won't compare csc coeffs)", \
+				current_config->name2, \
+				pipe_config->name2); \
+		ret = false;\
+	} else { \
+		if (!intel_csc_equal(current_config, current_config->name3, \
+				pipe_config->name3)) { \
+			pipe_config_mismatch(fastset, crtc, __stringify(name3), \
+					"hw_state doesn't match sw_state"); \
+			ret = false; \
+		} \
+	} \
+} while (0)
+
 #define PIPE_CONF_QUIRK(quirk) \
 	((current_config->quirks | pipe_config->quirks) & (quirk))
 
@@ -9052,6 +9075,13 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 		bp_gamma = intel_color_get_gamma_bit_precision(pipe_config);
 		if (bp_gamma)
 			PIPE_CONF_CHECK_COLOR_LUT(gamma_mode, hw.gamma_lut, bp_gamma);
+
+		if (HAS_GMCH(dev_priv)) {
+			if (IS_CHERRYVIEW(dev_priv))
+				PIPE_CONF_CHECK_CSC(csc_enable, cgm_mode, hw.ctm);
+		} else {
+			PIPE_CONF_CHECK_CSC(csc_enable, csc_mode, hw.ctm);
+		}
 	}
 
 	PIPE_CONF_CHECK_BOOL(double_wide);
@@ -9143,6 +9173,7 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 #undef PIPE_CONF_CHECK_FLAGS
 #undef PIPE_CONF_CHECK_CLOCK_FUZZY
 #undef PIPE_CONF_CHECK_COLOR_LUT
+#undef PIPE_CONF_CHECK_CSC
 #undef PIPE_CONF_QUIRK
 
 	return ret;
-- 
2.20.1

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

[Intel-gfx] [PATCH 4/7] drm/i915/display: Extract chv_read_csc()

[Bhanuprakash Modem]

For Cherryview, add hw read out to create hw blob of ctm coeff
values.

Cc: Swati Sharma <swati2.sharma@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 47 ++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 54dfd3523272..15f97fbb77b3 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1943,6 +1943,52 @@ static void chv_read_luts(struct intel_crtc_state *crtc_state)
 		i965_read_luts(crtc_state);
 }
 
+static struct drm_property_blob *chv_read_cgm_ctm(struct intel_crtc *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_ctm *ctm;
+	u32 temp;
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_ctm),
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	ctm = blob->data;
+
+	temp = intel_de_read(dev_priv, CGM_PIPE_CSC_COEFF01(pipe));
+	ctm->matrix[0] = (temp >> 16) & 0xffff;
+	ctm->matrix[1] = temp & 0xffff;
+
+	temp = intel_de_read(dev_priv, CGM_PIPE_CSC_COEFF23(pipe));
+	ctm->matrix[2] = (temp >> 16) & 0xffff;
+	ctm->matrix[3] = temp & 0xffff;
+
+	temp = intel_de_read(dev_priv, CGM_PIPE_CSC_COEFF45(pipe));
+	ctm->matrix[4] = (temp >> 16) & 0xffff;
+	ctm->matrix[5] = temp & 0xffff;
+
+	temp = intel_de_read(dev_priv, CGM_PIPE_CSC_COEFF67(pipe));
+	ctm->matrix[6] = (temp >> 16) & 0xffff;
+	ctm->matrix[7] = temp & 0xffff;
+
+	temp = intel_de_read(dev_priv, CGM_PIPE_CSC_COEFF8(pipe));
+	ctm->matrix[8] = (temp >> 16) & 0xffff;
+
+	return blob;
+}
+
+static void chv_read_csc(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
+	if (crtc_state->cgm_mode & CGM_PIPE_MODE_CSC)
+		crtc_state->hw.ctm = chv_read_cgm_ctm(crtc);
+}
+
 static struct drm_property_blob *ilk_read_lut_8(struct intel_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
@@ -2145,6 +2191,7 @@ void intel_color_init(struct intel_crtc *crtc)
 			dev_priv->display.color_commit = i9xx_color_commit;
 			dev_priv->display.load_luts = chv_load_luts;
 			dev_priv->display.read_luts = chv_read_luts;
+			dev_priv->display.read_csc = chv_read_csc;
 		} else if (INTEL_GEN(dev_priv) >= 4) {
 			dev_priv->display.color_check = i9xx_color_check;
 			dev_priv->display.color_commit = i9xx_color_commit;
-- 
2.20.1

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

[Intel-gfx] [PATCH 5/7] drm/i915/display: Extract ilk_read_csc()

[Bhanuprakash Modem]

For ilk+, add hw read out to create hw blob of ctm coeff values.

Cc: Swati Sharma <swati2.sharma@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 52 ++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 15f97fbb77b3..877833f294bb 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -321,6 +321,54 @@ static void ilk_load_csc_matrix(const struct intel_crtc_state *crtc_state)
 		       crtc_state->csc_mode);
 }
 
+static struct drm_property_blob *ilk_read_csc_matrix(struct intel_crtc *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_ctm *ctm;
+	u32 temp;
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_ctm),
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	ctm = blob->data;
+
+	temp = intel_de_read(dev_priv, PIPE_CSC_COEFF_RY_GY(pipe));
+	ctm->matrix[0] = (temp >> 16) & 0xffff;
+	ctm->matrix[1] = temp & 0xffff;
+
+	temp = intel_de_read(dev_priv, PIPE_CSC_COEFF_BY(pipe));
+	ctm->matrix[2] = (temp >> 16) & 0xffff;
+
+	temp = intel_de_read(dev_priv, PIPE_CSC_COEFF_RU_GU(pipe));
+	ctm->matrix[3] = (temp >> 16) & 0xffff;
+	ctm->matrix[4] = temp & 0xffff;
+
+	temp = intel_de_read(dev_priv, PIPE_CSC_COEFF_BU(pipe));
+	ctm->matrix[5] = (temp >> 16) & 0xffff;
+
+	temp = intel_de_read(dev_priv, PIPE_CSC_COEFF_RV_GV(pipe));
+	ctm->matrix[6] = (temp >> 16) & 0xffff;
+	ctm->matrix[7] = temp & 0xffff;
+
+	temp = intel_de_read(dev_priv, PIPE_CSC_COEFF_BV(pipe));
+	ctm->matrix[8] = (temp >> 16) & 0xffff;
+
+	return blob;
+}
+
+static void ilk_read_csc(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
+	if (crtc_state->csc_enable)
+		crtc_state->hw.ctm = ilk_read_csc_matrix(crtc);
+}
+
 static void icl_load_csc_matrix(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
@@ -2226,13 +2274,17 @@ void intel_color_init(struct intel_crtc *crtc)
 		} else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
 			dev_priv->display.load_luts = glk_load_luts;
 			dev_priv->display.read_luts = glk_read_luts;
+			dev_priv->display.read_csc = ilk_read_csc;
 		} else if (INTEL_GEN(dev_priv) >= 8) {
 			dev_priv->display.load_luts = bdw_load_luts;
+			dev_priv->display.read_csc = ilk_read_csc;
 		} else if (INTEL_GEN(dev_priv) >= 7) {
 			dev_priv->display.load_luts = ivb_load_luts;
+			dev_priv->display.read_csc = ilk_read_csc;
 		} else {
 			dev_priv->display.load_luts = ilk_load_luts;
 			dev_priv->display.read_luts = ilk_read_luts;
+			dev_priv->display.read_csc = ilk_read_csc;
 		}
 	}
 
-- 
2.20.1

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

[Intel-gfx] [PATCH 6/7] drm/i915/display: Extract icl_read_csc()

[Bhanuprakash Modem]

For icl+, add hw read out to create hw blob of ctm coeff values.

Cc: Swati Sharma <swati2.sharma@intel.com>
Cc: Uma Shankar <uma.shankar@intel.com>
Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 877833f294bb..8f9727553c45 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -396,6 +396,60 @@ static void icl_load_csc_matrix(const struct intel_crtc_state *crtc_state)
 		       crtc_state->csc_mode);
 }
 
+static struct drm_property_blob *icl_read_output_csc_matrix(struct intel_crtc *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_ctm *ctm;
+	u32 temp;
+
+	blob = drm_property_create_blob(&dev_priv->drm,
+					sizeof(struct drm_color_ctm),
+					NULL);
+	if (IS_ERR(blob))
+		return NULL;
+
+	ctm = blob->data;
+
+	temp = intel_de_read(dev_priv, PIPE_CSC_OUTPUT_COEFF_RY_GY(pipe));
+	ctm->matrix[0] = (temp >> 16) & 0xffff;
+	ctm->matrix[1] = temp & 0xffff;
+
+	temp = intel_de_read(dev_priv, PIPE_CSC_OUTPUT_COEFF_BY(pipe));
+	ctm->matrix[2] = (temp >> 16) & 0xffff;
+
+	temp = intel_de_read(dev_priv, PIPE_CSC_OUTPUT_COEFF_RU_GU(pipe));
+	ctm->matrix[3] = (temp >> 16) & 0xffff;
+	ctm->matrix[4] = temp & 0xffff;
+
+	temp = intel_de_read(dev_priv, PIPE_CSC_OUTPUT_COEFF_BU(pipe));
+	ctm->matrix[5] = (temp >> 16) & 0xffff;
+
+	temp = intel_de_read(dev_priv, PIPE_CSC_OUTPUT_COEFF_RV_GV(pipe));
+	ctm->matrix[6] = (temp >> 16) & 0xffff;
+	ctm->matrix[7] = temp & 0xffff;
+
+	temp = intel_de_read(dev_priv, PIPE_CSC_OUTPUT_COEFF_BV(pipe));
+	ctm->matrix[8] = (temp >> 16) & 0xffff;
+
+	return blob;
+}
+
+static void icl_read_csc(struct intel_crtc_state *crtc_state)
+{
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
+
+	if (!crtc_state->csc_enable)
+		return;
+
+	if (crtc_state->csc_mode & ICL_CSC_ENABLE)
+		crtc_state->hw.ctm = ilk_read_csc_matrix(crtc);
+
+	if (crtc_state->csc_mode & ICL_OUTPUT_CSC_ENABLE)
+		crtc_state->hw.ctm = icl_read_output_csc_matrix(crtc);
+}
+
 static void chv_csc_convert_ctm(const struct drm_color_ctm *ctm, u16 coeffs[9])
 {
 	int i;
@@ -2271,6 +2325,7 @@ void intel_color_init(struct intel_crtc *crtc)
 		if (INTEL_GEN(dev_priv) >= 11) {
 			dev_priv->display.load_luts = icl_load_luts;
 			dev_priv->display.read_luts = icl_read_luts;
+			dev_priv->display.read_csc = icl_read_csc;
 		} else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
 			dev_priv->display.load_luts = glk_load_luts;
 			dev_priv->display.read_luts = glk_read_luts;
-- 
2.20.1

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

[Intel-gfx] [PATCH 7/7] FOR_TESTING_ONLY: Print coeffs of hw and sw CTM

[Bhanuprakash Modem]

Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
---
 drivers/gpu/drm/i915/display/intel_color.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
index 8f9727553c45..caf4c3442b9e 100644
--- a/drivers/gpu/drm/i915/display/intel_color.c
+++ b/drivers/gpu/drm/i915/display/intel_color.c
@@ -1909,6 +1909,8 @@ bool intel_csc_equal(const struct intel_crtc_state *current_config,
 	u16 coeffs[9];
 	int i;
 
+	DRM_DEBUG_KMS("Bhanu debug--> sw_ctm[%s] hw_ctm[%s]", yesno(!!blob1), yesno(!!blob2));
+
 	if (!blob1 != !blob2)
 		return false;
 
@@ -1923,6 +1925,9 @@ bool intel_csc_equal(const struct intel_crtc_state *current_config,
 	else
 		ilk_csc_convert_ctm(current_config, coeffs);
 
+	for (i = 0; i < 9; i++)
+		DRM_DEBUG_KMS("Bhanu debug--> sw_ctm[%d]=0x%x hw_ctm[%d]=0x%llx", i, coeffs[i], i, ctm2->matrix[i]);
+
 	for (i = 0; i < 9; i++) {
 		if (abs(coeffs[i] - ctm2->matrix[i]))
 			return false;
-- 
2.20.1

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

Re: [Intel-gfx] [PATCH 4/7] drm/i915/display: Extract chv_read_csc()

[Ville Syrjälä]

On Mon, Mar 22, 2021 at 08:00:05PM +0530, Bhanuprakash Modem wrote:
> For Cherryview, add hw read out to create hw blob of ctm coeff
> values.
> 
> Cc: Swati Sharma <swati2.sharma@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 47 ++++++++++++++++++++++
>  1 file changed, 47 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index 54dfd3523272..15f97fbb77b3 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -1943,6 +1943,52 @@ static void chv_read_luts(struct intel_crtc_state *crtc_state)
>  		i965_read_luts(crtc_state);
>  }
>  
> +static struct drm_property_blob *chv_read_cgm_ctm(struct intel_crtc *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_ctm *ctm;
> +	u32 temp;
> +
> +	blob = drm_property_create_blob(&dev_priv->drm,
> +					sizeof(struct drm_color_ctm),
> +					NULL);
> +	if (IS_ERR(blob))
> +		return NULL;
> +
> +	ctm = blob->data;
> +
> +	temp = intel_de_read(dev_priv, CGM_PIPE_CSC_COEFF01(pipe));
> +	ctm->matrix[0] = (temp >> 16) & 0xffff;
> +	ctm->matrix[1] = temp & 0xffff;

We can't store hardware specific junk into the ctm. It needs to get
converted to the proper format.

Would also be a good time to fix the CHV CSC programming to handle
negative values correct, and should probably extend the test cases
to actually catch that particular failure...

> +
> +	temp = intel_de_read(dev_priv, CGM_PIPE_CSC_COEFF23(pipe));
> +	ctm->matrix[2] = (temp >> 16) & 0xffff;
> +	ctm->matrix[3] = temp & 0xffff;
> +
> +	temp = intel_de_read(dev_priv, CGM_PIPE_CSC_COEFF45(pipe));
> +	ctm->matrix[4] = (temp >> 16) & 0xffff;
> +	ctm->matrix[5] = temp & 0xffff;
> +
> +	temp = intel_de_read(dev_priv, CGM_PIPE_CSC_COEFF67(pipe));
> +	ctm->matrix[6] = (temp >> 16) & 0xffff;
> +	ctm->matrix[7] = temp & 0xffff;
> +
> +	temp = intel_de_read(dev_priv, CGM_PIPE_CSC_COEFF8(pipe));
> +	ctm->matrix[8] = (temp >> 16) & 0xffff;
> +
> +	return blob;
> +}
> +
> +static void chv_read_csc(struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +
> +	if (crtc_state->cgm_mode & CGM_PIPE_MODE_CSC)
> +		crtc_state->hw.ctm = chv_read_cgm_ctm(crtc);
> +}
> +
>  static struct drm_property_blob *ilk_read_lut_8(struct intel_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> @@ -2145,6 +2191,7 @@ void intel_color_init(struct intel_crtc *crtc)
>  			dev_priv->display.color_commit = i9xx_color_commit;
>  			dev_priv->display.load_luts = chv_load_luts;
>  			dev_priv->display.read_luts = chv_read_luts;
> +			dev_priv->display.read_csc = chv_read_csc;
>  		} else if (INTEL_GEN(dev_priv) >= 4) {
>  			dev_priv->display.color_check = i9xx_color_check;
>  			dev_priv->display.color_commit = i9xx_color_commit;
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH 0/7] drm/i915: Add state checker for CSC coeff values

[Ville Syrjälä]

On Mon, Mar 22, 2021 at 08:00:01PM +0530, Bhanuprakash Modem wrote:
> In this patch series, added state checker to validate CSC. This reads
> hardware state, and compares the originally requested state(s/w). This
> is done for chv, ilk, glk and their variant platforms. Rest of
> the platforms will be enabled on top of this later.
> 
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> 
> Bhanuprakash Modem (7):
>   drm/i915/display: Introduce vfunc read_csc() to create hw ctm
>   drm/i915/display: Add func to compare hw/sw CSC matrix
>   drm/i915/display: Add macro to compare hw/sw CSC matrix
>   drm/i915/display: Extract chv_read_csc()
>   drm/i915/display: Extract ilk_read_csc()
>   drm/i915/display: Extract icl_read_csc()

The patch subjects are confusing me. You're not extracting any code
AFAICS?

>   FOR_TESTING_ONLY: Print coeffs of hw and sw CTM
> 
>  drivers/gpu/drm/i915/display/intel_color.c   | 209 ++++++++++++++++++-
>  drivers/gpu/drm/i915/display/intel_color.h   |   3 +
>  drivers/gpu/drm/i915/display/intel_display.c |  31 +++
>  drivers/gpu/drm/i915/i915_drv.h              |   1 +
>  4 files changed, 239 insertions(+), 5 deletions(-)
> 
> --
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

Re: [Intel-gfx] [PATCH 5/7] drm/i915/display: Extract ilk_read_csc()

[Ville Syrjälä]

On Mon, Mar 22, 2021 at 08:00:06PM +0530, Bhanuprakash Modem wrote:
> For ilk+, add hw read out to create hw blob of ctm coeff values.

We need an explanation why this is safe given display w/a 1184...

> 
> Cc: Swati Sharma <swati2.sharma@intel.com>
> Cc: Uma Shankar <uma.shankar@intel.com>
> Signed-off-by: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_color.c | 52 ++++++++++++++++++++++
>  1 file changed, 52 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c
> index 15f97fbb77b3..877833f294bb 100644
> --- a/drivers/gpu/drm/i915/display/intel_color.c
> +++ b/drivers/gpu/drm/i915/display/intel_color.c
> @@ -321,6 +321,54 @@ static void ilk_load_csc_matrix(const struct intel_crtc_state *crtc_state)
>  		       crtc_state->csc_mode);
>  }
>  
> +static struct drm_property_blob *ilk_read_csc_matrix(struct intel_crtc *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_ctm *ctm;
> +	u32 temp;
> +
> +	blob = drm_property_create_blob(&dev_priv->drm,
> +					sizeof(struct drm_color_ctm),
> +					NULL);
> +	if (IS_ERR(blob))
> +		return NULL;
> +
> +	ctm = blob->data;
> +
> +	temp = intel_de_read(dev_priv, PIPE_CSC_COEFF_RY_GY(pipe));
> +	ctm->matrix[0] = (temp >> 16) & 0xffff;
> +	ctm->matrix[1] = temp & 0xffff;
> +
> +	temp = intel_de_read(dev_priv, PIPE_CSC_COEFF_BY(pipe));
> +	ctm->matrix[2] = (temp >> 16) & 0xffff;
> +
> +	temp = intel_de_read(dev_priv, PIPE_CSC_COEFF_RU_GU(pipe));
> +	ctm->matrix[3] = (temp >> 16) & 0xffff;
> +	ctm->matrix[4] = temp & 0xffff;
> +
> +	temp = intel_de_read(dev_priv, PIPE_CSC_COEFF_BU(pipe));
> +	ctm->matrix[5] = (temp >> 16) & 0xffff;
> +
> +	temp = intel_de_read(dev_priv, PIPE_CSC_COEFF_RV_GV(pipe));
> +	ctm->matrix[6] = (temp >> 16) & 0xffff;
> +	ctm->matrix[7] = temp & 0xffff;
> +
> +	temp = intel_de_read(dev_priv, PIPE_CSC_COEFF_BV(pipe));
> +	ctm->matrix[8] = (temp >> 16) & 0xffff;
> +
> +	return blob;
> +}
> +
> +static void ilk_read_csc(struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> +
> +	if (crtc_state->csc_enable)
> +		crtc_state->hw.ctm = ilk_read_csc_matrix(crtc);
> +}
> +
>  static void icl_load_csc_matrix(const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_crtc *crtc = to_intel_crtc(crtc_state->uapi.crtc);
> @@ -2226,13 +2274,17 @@ void intel_color_init(struct intel_crtc *crtc)
>  		} else if (IS_CANNONLAKE(dev_priv) || IS_GEMINILAKE(dev_priv)) {
>  			dev_priv->display.load_luts = glk_load_luts;
>  			dev_priv->display.read_luts = glk_read_luts;
> +			dev_priv->display.read_csc = ilk_read_csc;
>  		} else if (INTEL_GEN(dev_priv) >= 8) {
>  			dev_priv->display.load_luts = bdw_load_luts;
> +			dev_priv->display.read_csc = ilk_read_csc;
>  		} else if (INTEL_GEN(dev_priv) >= 7) {
>  			dev_priv->display.load_luts = ivb_load_luts;
> +			dev_priv->display.read_csc = ilk_read_csc;
>  		} else {
>  			dev_priv->display.load_luts = ilk_load_luts;
>  			dev_priv->display.read_luts = ilk_read_luts;
> +			dev_priv->display.read_csc = ilk_read_csc;
>  		}
>  	}
>  
> -- 
> 2.20.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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