All of lore.kernel.org
 help / color / mirror / Atom feed
* [v2 0/2] Enable Plane Input CSC for ICL
@ 2018-10-24 16:18 Uma Shankar
  2018-10-24 16:18 ` [v2 1/2] drm/i915/icl: Define Plane Input CSC Coefficient Registers Uma Shankar
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Uma Shankar @ 2018-10-24 16:18 UTC (permalink / raw)
  To: intel-gfx; +Cc: ville.syrjala, maarten.lankhorst

This patch series enables plane input csc feature for
ICL. This is needed for YUV to RGB conversion on bottom
3 planes on ICL, other planes are handled in the legacy
way using fixed function hardware.

v2: Separated the patch into 2 parts as per Maarten's comments.
Addressed Ville and Maarten's review comment.

Uma Shankar (2):
  drm/i915/icl: Define Plane Input CSC Coefficient Registers
  drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion

 drivers/gpu/drm/i915/i915_reg.h      | 217 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_color.c   |  49 ++++++++
 drivers/gpu/drm/i915/intel_display.c |  24 +++-
 drivers/gpu/drm/i915/intel_drv.h     |   2 +
 4 files changed, 286 insertions(+), 6 deletions(-)

-- 
1.9.1

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

^ permalink raw reply	[flat|nested] 13+ messages in thread
* [v2 1/2] drm/i915/icl: Define Plane Input CSC Coefficient Registers
  2018-10-24 16:18 [v2 0/2] Enable Plane Input CSC for ICL Uma Shankar
@ 2018-10-24 16:18 ` Uma Shankar
  2018-10-24 20:31   ` Matt Roper
  2018-10-24 16:18 ` [v2 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion Uma Shankar
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Uma Shankar @ 2018-10-24 16:18 UTC (permalink / raw)
  To: intel-gfx; +Cc: ville.syrjala, maarten.lankhorst

Defined the plane input csc coefficient registers and macros.
6 registers are used to program a total of 9 coefficients,
added macros to define each of them for all the planes
supporting the feature on pipes. On ICL, bottom 3 planes
have this capability.

v2: Segregated the register macro definition as separate patch
as per Maarten's suggestion.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 217 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 217 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 69eb573..6a363a32 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6569,6 +6569,7 @@ enum {
 #define _PLANE_COLOR_CTL_3_A			0x703CC /* GLK+ */
 #define   PLANE_COLOR_PIPE_GAMMA_ENABLE		(1 << 30) /* Pre-ICL */
 #define   PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE	(1 << 28)
+#define   PLANE_COLOR_INPUT_CSC_ENABLE		(1 << 20) /* Pre-ICL */
 #define   PLANE_COLOR_PIPE_CSC_ENABLE		(1 << 23) /* Pre-ICL */
 #define   PLANE_COLOR_CSC_MODE_BYPASS			(0 << 17)
 #define   PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709		(1 << 17)
@@ -6585,6 +6586,222 @@ enum {
 #define _PLANE_NV12_BUF_CFG_1_A		0x70278
 #define _PLANE_NV12_BUF_CFG_2_A		0x70378
 
+/* Input CSC Register Definitions */
+#define _PLANE_INPUT_CSC_RY_GY_1_A	0x701E0
+#define _PLANE_INPUT_CSC_RY_GY_2_A	0x702E0
+#define _PLANE_INPUT_CSC_RY_GY_3_A	0x703E0
+
+#define _PLANE_INPUT_CSC_RY_GY_1_B	0x711E0
+#define _PLANE_INPUT_CSC_RY_GY_2_B	0x712E0
+#define _PLANE_INPUT_CSC_RY_GY_3_B	0x713E0
+
+#define _PLANE_INPUT_CSC_RY_GY_1(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_RY_GY_1_A, \
+	     _PLANE_INPUT_CSC_RY_GY_1_B)
+#define _PLANE_INPUT_CSC_RY_GY_2(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_RY_GY_2_A, \
+	     _PLANE_INPUT_CSC_RY_GY_2_B)
+#define PLANE_INPUT_CSC_RY_GY(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_RY_GY_1(pipe), \
+		    _PLANE_INPUT_CSC_RY_GY_2(pipe))
+
+#define _PLANE_INPUT_CSC_BY_1_A		0x701E4
+#define _PLANE_INPUT_CSC_BY_2_A		0x702E4
+#define _PLANE_INPUT_CSC_BY_3_A		0x703E4
+
+#define _PLANE_INPUT_CSC_BY_1_B		0x711E4
+#define _PLANE_INPUT_CSC_BY_2_B		0x712E4
+#define _PLANE_INPUT_CSC_BY_3_B		0x713E4
+
+#define _PLANE_INPUT_CSC_BY_1(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_BY_1_A, \
+	     _PLANE_INPUT_CSC_BY_1_B)
+#define _PLANE_INPUT_CSC_BY_2(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_BY_2_A, \
+	     _PLANE_INPUT_CSC_BY_2_B)
+#define PLANE_INPUT_CSC_BY(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_BY_1(pipe), \
+		    _PLANE_INPUT_CSC_BY_2(pipe))
+
+#define _PLANE_INPUT_CSC_RU_GU_1_A	0x701E8
+#define _PLANE_INPUT_CSC_RU_GU_2_A	0x702E8
+#define _PLANE_INPUT_CSC_RU_GU_3_A	0x703E8
+
+#define _PLANE_INPUT_CSC_RU_GU_1_B	0x711E8
+#define _PLANE_INPUT_CSC_RU_GU_2_B	0x712E8
+#define _PLANE_INPUT_CSC_RU_GU_3_B	0x713E8
+
+#define _PLANE_INPUT_CSC_RU_GU_1(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_RU_GU_1_A, \
+	     _PLANE_INPUT_CSC_RU_GU_1_B)
+#define _PLANE_INPUT_CSC_RU_GU_2(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_RU_GU_2_A, \
+	     _PLANE_INPUT_CSC_RU_GU_2_B)
+#define PLANE_INPUT_CSC_RU_GU(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_RU_GU_1(pipe), \
+		    _PLANE_INPUT_CSC_RU_GU_2(pipe))
+
+#define _PLANE_INPUT_CSC_BU_1_A		0x701EC
+#define _PLANE_INPUT_CSC_BU_2_A		0x702EC
+#define _PLANE_INPUT_CSC_BU_3_A		0x703EC
+
+#define _PLANE_INPUT_CSC_BU_1_B		0x711EC
+#define _PLANE_INPUT_CSC_BU_2_B		0x712EC
+#define _PLANE_INPUT_CSC_BU_3_B		0x713EC
+
+#define _PLANE_INPUT_CSC_BU_1(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_BU_1_A, \
+	     _PLANE_INPUT_CSC_BU_1_B)
+#define _PLANE_INPUT_CSC_BU_2(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_BU_2_A, \
+	     _PLANE_INPUT_CSC_BU_2_B)
+#define PLANE_INPUT_CSC_BU(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_BU_1(pipe), \
+		    _PLANE_INPUT_CSC_BU_2(pipe))
+
+#define _PLANE_INPUT_CSC_RV_GV_1_A	0x701F0
+#define _PLANE_INPUT_CSC_RV_GV_2_A	0x702F0
+#define _PLANE_INPUT_CSC_RV_GV_3_A	0x703F0
+
+#define _PLANE_INPUT_CSC_RV_GV_1_B	0x711F0
+#define _PLANE_INPUT_CSC_RV_GV_2_B	0x712F0
+#define _PLANE_INPUT_CSC_RV_GV_3_B	0x713F0
+
+#define _PLANE_INPUT_CSC_RV_GV_1(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_RV_GV_1_A, \
+	     _PLANE_INPUT_CSC_RV_GV_1_B)
+#define _PLANE_INPUT_CSC_RV_GV_2(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_RV_GV_2_A, \
+	     _PLANE_INPUT_CSC_RV_GV_2_B)
+#define PLANE_INPUT_CSC_RV_GV(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_RV_GV_1(pipe), \
+		    _PLANE_INPUT_CSC_RV_GV_2(pipe))
+
+#define _PLANE_INPUT_CSC_BV_1_A		0x701F4
+#define _PLANE_INPUT_CSC_BV_2_A		0x702F4
+#define _PLANE_INPUT_CSC_BV_3_A		0x703F4
+
+#define _PLANE_INPUT_CSC_BV_1_B		0x711F4
+#define _PLANE_INPUT_CSC_BV_2_B		0x712F4
+#define _PLANE_INPUT_CSC_BV_3_B		0x713F4
+
+#define _PLANE_INPUT_CSC_BV_1(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_BV_1_A, \
+	     _PLANE_INPUT_CSC_BV_1_B)
+#define _PLANE_INPUT_CSC_BV_2(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_BV_2_A, \
+	     _PLANE_INPUT_CSC_BV_2_B)
+#define PLANE_INPUT_CSC_BV(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_BV_1(pipe), \
+		    _PLANE_INPUT_CSC_BV_2(pipe))
+
+#define _PLANE_INPUT_CSC_PREOFF_HI_1_A		0x701F8
+#define _PLANE_INPUT_CSC_PREOFF_HI_2_A		0x702F8
+#define _PLANE_INPUT_CSC_PREOFF_HI_3_A		0x703F8
+
+#define _PLANE_INPUT_CSC_PREOFF_HI_1_B		0x711F8
+#define _PLANE_INPUT_CSC_PREOFF_HI_2_B		0x712F8
+#define _PLANE_INPUT_CSC_PREOFF_HI_3_B		0x713F8
+
+#define _PLANE_INPUT_CSC_PREOFF_HI_1(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_HI_1_A, \
+	     _PLANE_INPUT_CSC_PREOFF_HI_1_B)
+#define _PLANE_INPUT_CSC_PREOFF_HI_2(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_HI_2_A, \
+	     _PLANE_INPUT_CSC_PREOFF_HI_2_B)
+#define PLANE_INPUT_CSC_PREOFF_HI(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_PREOFF_HI_1(pipe), \
+		    _PLANE_INPUT_CSC_PREOFF_HI_2(pipe))
+
+#define _PLANE_INPUT_CSC_PREOFF_ME_1_A		0x701FC
+#define _PLANE_INPUT_CSC_PREOFF_ME_2_A		0x702FC
+#define _PLANE_INPUT_CSC_PREOFF_ME_3_A		0x703FC
+
+#define _PLANE_INPUT_CSC_PREOFF_ME_1_B		0x711FC
+#define _PLANE_INPUT_CSC_PREOFF_ME_2_B		0x712FC
+#define _PLANE_INPUT_CSC_PREOFF_ME_3_B		0x713FC
+
+#define _PLANE_INPUT_CSC_PREOFF_ME_1(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_ME_1_A, \
+	     _PLANE_INPUT_CSC_PREOFF_ME_1_B)
+#define _PLANE_INPUT_CSC_PREOFF_ME_2(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_ME_2_A, \
+	     _PLANE_INPUT_CSC_PREOFF_ME_2_B)
+#define PLANE_INPUT_CSC_PREOFF_ME(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_PREOFF_ME_1(pipe), \
+		    _PLANE_INPUT_CSC_PREOFF_ME_2(pipe))
+
+#define _PLANE_INPUT_CSC_PREOFF_LO_1_A		0x70200
+#define _PLANE_INPUT_CSC_PREOFF_LO_2_A		0x70300
+#define _PLANE_INPUT_CSC_PREOFF_LO_3_A		0x70400
+
+#define _PLANE_INPUT_CSC_PREOFF_LO_1_B		0x71200
+#define _PLANE_INPUT_CSC_PREOFF_LO_2_B		0x71300
+#define _PLANE_INPUT_CSC_PREOFF_LO_3_B		0x71400
+
+#define _PLANE_INPUT_CSC_PREOFF_LO_1(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_LO_1_A, \
+	     _PLANE_INPUT_CSC_PREOFF_LO_1_B)
+#define _PLANE_INPUT_CSC_PREOFF_LO_2(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_LO_2_A, \
+	     _PLANE_INPUT_CSC_PREOFF_LO_2_B)
+#define PLANE_INPUT_CSC_PREOFF_LO(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_PREOFF_LO_1(pipe), \
+		    _PLANE_INPUT_CSC_PREOFF_LO_2(pipe))
+
+#define _PLANE_INPUT_CSC_POSTOFF_HI_1_A		0x70204
+#define _PLANE_INPUT_CSC_POSTOFF_HI_2_A		0x70304
+#define _PLANE_INPUT_CSC_POSTOFF_HI_3_A		0x70404
+
+#define _PLANE_INPUT_CSC_POSTOFF_HI_1_B		0x71204
+#define _PLANE_INPUT_CSC_POSTOFF_HI_2_B		0x71304
+#define _PLANE_INPUT_CSC_POSTOFF_HI_3_B		0x71404
+
+#define _PLANE_INPUT_CSC_POSTOFF_HI_1(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_HI_1_A, \
+	     _PLANE_INPUT_CSC_POSTOFF_HI_1_B)
+#define _PLANE_INPUT_CSC_POSTOFF_HI_2(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_HI_2_A, \
+	     _PLANE_INPUT_CSC_POSTOFF_HI_2_B)
+#define PLANE_INPUT_CSC_POSTOFF_HI(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_POSTOFF_HI_1(pipe), \
+		    _PLANE_INPUT_CSC_POSTOFF_HI_2(pipe))
+
+#define _PLANE_INPUT_CSC_POSTOFF_ME_1_A		0x70208
+#define _PLANE_INPUT_CSC_POSTOFF_ME_2_A		0x70308
+#define _PLANE_INPUT_CSC_POSTOFF_ME_3_A		0x70408
+
+#define _PLANE_INPUT_CSC_POSTOFF_ME_1_B		0x71208
+#define _PLANE_INPUT_CSC_POSTOFF_ME_2_B		0x71308
+#define _PLANE_INPUT_CSC_POSTOFF_ME_3_B		0x71408
+
+#define _PLANE_INPUT_CSC_POSTOFF_ME_1(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_ME_1_A, \
+	     _PLANE_INPUT_CSC_POSTOFF_ME_1_B)
+#define _PLANE_INPUT_CSC_POSTOFF_ME_2(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_ME_2_A, \
+	     _PLANE_INPUT_CSC_POSTOFF_ME_2_B)
+#define PLANE_INPUT_CSC_POSTOFF_ME(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_POSTOFF_ME_1(pipe), \
+		    _PLANE_INPUT_CSC_POSTOFF_ME_2(pipe))
+
+#define _PLANE_INPUT_CSC_POSTOFF_LO_1_A		0x7020C
+#define _PLANE_INPUT_CSC_POSTOFF_LO_2_A		0x7030C
+#define _PLANE_INPUT_CSC_POSTOFF_LO_3_A		0x7040C
+
+#define _PLANE_INPUT_CSC_POSTOFF_LO_1_B		0x7120C
+#define _PLANE_INPUT_CSC_POSTOFF_LO_2_B		0x7130C
+#define _PLANE_INPUT_CSC_POSTOFF_LO_3_B		0x7140C
+
+#define _PLANE_INPUT_CSC_POSTOFF_LO_1(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_LO_1_A, \
+	     _PLANE_INPUT_CSC_POSTOFF_LO_1_B)
+#define _PLANE_INPUT_CSC_POSTOFF_LO_2(pipe)	\
+	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_LO_2_A, \
+	     _PLANE_INPUT_CSC_POSTOFF_LO_2_B)
+#define PLANE_INPUT_CSC_POSTOFF_LO(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_POSTOFF_LO_1(pipe), \
+		    _PLANE_INPUT_CSC_POSTOFF_LO_2(pipe))
 
 #define _PLANE_CTL_1_B				0x71180
 #define _PLANE_CTL_2_B				0x71280
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* [v2 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
  2018-10-24 16:18 [v2 0/2] Enable Plane Input CSC for ICL Uma Shankar
  2018-10-24 16:18 ` [v2 1/2] drm/i915/icl: Define Plane Input CSC Coefficient Registers Uma Shankar
@ 2018-10-24 16:18 ` Uma Shankar
  2018-10-24 23:22   ` Matt Roper
  2018-10-25  9:47   ` Maarten Lankhorst
  2018-10-24 17:41 ` ✗ Fi.CI.CHECKPATCH: warning for Enable Plane Input CSC for ICL Patchwork
                   ` (3 subsequent siblings)
  5 siblings, 2 replies; 13+ messages in thread
From: Uma Shankar @ 2018-10-24 16:18 UTC (permalink / raw)
  To: intel-gfx; +Cc: ville.syrjala, maarten.lankhorst

Plane input CSC needs to be enabled to convert frambuffers from
YUV to RGB. This is needed for bottom 3 planes on ICL, rest of
the planes have hardcoded conversion and taken care by the legacy
code.

This patch defines the co-efficient values for YUV to RGB conversion
in BT709 and BT601 formats. It programs the coefficients and enables
the plane input csc unit in hardware.

Note: This is currently untested and floated to get an early feedback
on the design and implementation for this feature. In parallel,
I will test this on actual ICL hardware and confirm with planar
formats.

v2: Addressed Maarten's and Ville's review comments and added the
coefficients in a 2D array instead of independent Macros.

Signed-off-by: Uma Shankar <uma.shankar@intel.com>
---
 drivers/gpu/drm/i915/intel_color.c   | 49 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c | 24 +++++++++++++-----
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 3 files changed, 69 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 5127da2..de733cf 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -57,6 +57,11 @@
 #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8
 #define CSC_RGB_TO_YUV_BV 0x1e080000
 
+/* Preoffset values for YUV to RGB Conversion */
+#define PREOFF_YUV_TO_RGB_HI		0x800
+#define PREOFF_YUV_TO_RGB_ME		0xF00
+#define PREOFF_YUV_TO_RGB_LO		0x800
+
 /*
  * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
  * format). This macro takes the coefficient we want transformed and the
@@ -643,6 +648,50 @@ int intel_color_check(struct drm_crtc *crtc,
 	return -EINVAL;
 }
 
+void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state,
+				 const struct intel_plane_state *plane_state)
+{
+	struct drm_i915_private *dev_priv =
+		to_i915(plane_state->base.plane->dev);
+	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
+	enum pipe pipe = crtc->pipe;
+	struct intel_plane *intel_plane =
+		to_intel_plane(plane_state->base.plane);
+	enum plane_id plane = intel_plane->id;
+
+	static const u32 input_csc_matrix[][6] = {
+		/* BT.601 full range YCbCr -> full range RGB */
+		[DRM_COLOR_YCBCR_BT601] = {
+			0x7AF87800, 0x0, 0x8B287800,
+			0x9AC0, 0x7800, 0x7DD8,
+		},
+		/* BT.709 full range YCbCr -> full range RGB */
+		[DRM_COLOR_YCBCR_BT709] = {
+			0x7C987800, 0x0, 0x9EF87800,
+			0xABF8, 0x7800,  0x7ED8,
+		},
+	};
+	const u32 *csc = input_csc_matrix[plane_state->base.color_encoding];
+
+	I915_WRITE(PLANE_INPUT_CSC_RY_GY(pipe, plane), csc[0]);
+	I915_WRITE(PLANE_INPUT_CSC_BY(pipe, plane), csc[1]);
+	I915_WRITE(PLANE_INPUT_CSC_RU_GU(pipe, plane), csc[2]);
+	I915_WRITE(PLANE_INPUT_CSC_BU(pipe, plane), csc[3]);
+	I915_WRITE(PLANE_INPUT_CSC_RV_GV(pipe, plane), csc[4]);
+	I915_WRITE(PLANE_INPUT_CSC_BV(pipe, plane), csc[5]);
+
+	I915_WRITE(PLANE_INPUT_CSC_PREOFF_HI(pipe, plane),
+		   PREOFF_YUV_TO_RGB_HI);
+	I915_WRITE(PLANE_INPUT_CSC_PREOFF_ME(pipe, plane),
+		   PREOFF_YUV_TO_RGB_ME);
+	I915_WRITE(PLANE_INPUT_CSC_PREOFF_LO(pipe, plane),
+		   PREOFF_YUV_TO_RGB_LO);
+
+	I915_WRITE(PLANE_INPUT_CSC_POSTOFF_HI(pipe, plane), 0x0);
+	I915_WRITE(PLANE_INPUT_CSC_POSTOFF_ME(pipe, plane), 0x0);
+	I915_WRITE(PLANE_INPUT_CSC_POSTOFF_LO(pipe, plane), 0x0);
+}
+
 void intel_color_init(struct drm_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index fe045ab..be65419 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3666,6 +3666,8 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
 	struct drm_i915_private *dev_priv =
 		to_i915(plane_state->base.plane->dev);
 	const struct drm_framebuffer *fb = plane_state->base.fb;
+	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
+
 	u32 plane_color_ctl = 0;
 
 	if (INTEL_GEN(dev_priv) < 11) {
@@ -3676,13 +3678,23 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
 	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
 
 	if (fb->format->is_yuv) {
-		if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
-			plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
-		else
-			plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
+		if (!icl_is_hdr_plane(plane)) {
+			if (plane_state->base.color_encoding ==
+					DRM_COLOR_YCBCR_BT709)
+				plane_color_ctl |=
+					PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
+			else
+				plane_color_ctl |=
+					PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
 
-		if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
-			plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
+			if (plane_state->base.color_range ==
+					DRM_COLOR_YCBCR_FULL_RANGE)
+				plane_color_ctl |=
+				PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
+		} else {
+			icl_program_input_csc_coeff(crtc_state, plane_state);
+			plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
+		}
 	}
 
 	return plane_color_ctl;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index db24308..bd9e946 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -2285,6 +2285,8 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
 int intel_color_check(struct drm_crtc *crtc, struct drm_crtc_state *state);
 void intel_color_set_csc(struct drm_crtc_state *crtc_state);
 void intel_color_load_luts(struct drm_crtc_state *crtc_state);
+void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state,
+				 const struct intel_plane_state *plane_state);
 
 /* intel_lspcon.c */
 bool lspcon_init(struct intel_digital_port *intel_dig_port);
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 13+ messages in thread
* ✗ Fi.CI.CHECKPATCH: warning for Enable Plane Input CSC for ICL
  2018-10-24 16:18 [v2 0/2] Enable Plane Input CSC for ICL Uma Shankar
  2018-10-24 16:18 ` [v2 1/2] drm/i915/icl: Define Plane Input CSC Coefficient Registers Uma Shankar
  2018-10-24 16:18 ` [v2 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion Uma Shankar
@ 2018-10-24 17:41 ` Patchwork
  2018-10-24 17:42 ` ✗ Fi.CI.SPARSE: " Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2018-10-24 17:41 UTC (permalink / raw)
  To: Uma Shankar; +Cc: intel-gfx

== Series Details ==

Series: Enable Plane Input CSC for ICL
URL   : https://patchwork.freedesktop.org/series/51463/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
41d6b4c477ae drm/i915/icl: Define Plane Input CSC Coefficient Registers
-:48: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pipe' - possible side-effects?
#48: FILE: drivers/gpu/drm/i915/i915_reg.h:6604:
+#define PLANE_INPUT_CSC_RY_GY(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_RY_GY_1(pipe), \
+		    _PLANE_INPUT_CSC_RY_GY_2(pipe))

-:66: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pipe' - possible side-effects?
#66: FILE: drivers/gpu/drm/i915/i915_reg.h:6622:
+#define PLANE_INPUT_CSC_BY(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_BY_1(pipe), \
+		    _PLANE_INPUT_CSC_BY_2(pipe))

-:84: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pipe' - possible side-effects?
#84: FILE: drivers/gpu/drm/i915/i915_reg.h:6640:
+#define PLANE_INPUT_CSC_RU_GU(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_RU_GU_1(pipe), \
+		    _PLANE_INPUT_CSC_RU_GU_2(pipe))

-:102: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pipe' - possible side-effects?
#102: FILE: drivers/gpu/drm/i915/i915_reg.h:6658:
+#define PLANE_INPUT_CSC_BU(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_BU_1(pipe), \
+		    _PLANE_INPUT_CSC_BU_2(pipe))

-:120: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pipe' - possible side-effects?
#120: FILE: drivers/gpu/drm/i915/i915_reg.h:6676:
+#define PLANE_INPUT_CSC_RV_GV(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_RV_GV_1(pipe), \
+		    _PLANE_INPUT_CSC_RV_GV_2(pipe))

-:138: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pipe' - possible side-effects?
#138: FILE: drivers/gpu/drm/i915/i915_reg.h:6694:
+#define PLANE_INPUT_CSC_BV(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_BV_1(pipe), \
+		    _PLANE_INPUT_CSC_BV_2(pipe))

-:156: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pipe' - possible side-effects?
#156: FILE: drivers/gpu/drm/i915/i915_reg.h:6712:
+#define PLANE_INPUT_CSC_PREOFF_HI(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_PREOFF_HI_1(pipe), \
+		    _PLANE_INPUT_CSC_PREOFF_HI_2(pipe))

-:174: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pipe' - possible side-effects?
#174: FILE: drivers/gpu/drm/i915/i915_reg.h:6730:
+#define PLANE_INPUT_CSC_PREOFF_ME(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_PREOFF_ME_1(pipe), \
+		    _PLANE_INPUT_CSC_PREOFF_ME_2(pipe))

-:192: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pipe' - possible side-effects?
#192: FILE: drivers/gpu/drm/i915/i915_reg.h:6748:
+#define PLANE_INPUT_CSC_PREOFF_LO(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_PREOFF_LO_1(pipe), \
+		    _PLANE_INPUT_CSC_PREOFF_LO_2(pipe))

-:210: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pipe' - possible side-effects?
#210: FILE: drivers/gpu/drm/i915/i915_reg.h:6766:
+#define PLANE_INPUT_CSC_POSTOFF_HI(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_POSTOFF_HI_1(pipe), \
+		    _PLANE_INPUT_CSC_POSTOFF_HI_2(pipe))

-:228: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pipe' - possible side-effects?
#228: FILE: drivers/gpu/drm/i915/i915_reg.h:6784:
+#define PLANE_INPUT_CSC_POSTOFF_ME(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_POSTOFF_ME_1(pipe), \
+		    _PLANE_INPUT_CSC_POSTOFF_ME_2(pipe))

-:246: CHECK:MACRO_ARG_REUSE: Macro argument reuse 'pipe' - possible side-effects?
#246: FILE: drivers/gpu/drm/i915/i915_reg.h:6802:
+#define PLANE_INPUT_CSC_POSTOFF_LO(pipe, plane)	\
+	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_POSTOFF_LO_1(pipe), \
+		    _PLANE_INPUT_CSC_POSTOFF_LO_2(pipe))

total: 0 errors, 0 warnings, 12 checks, 229 lines checked
99c424470fe4 drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion

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

^ permalink raw reply	[flat|nested] 13+ messages in thread
* ✗ Fi.CI.SPARSE: warning for Enable Plane Input CSC for ICL
  2018-10-24 16:18 [v2 0/2] Enable Plane Input CSC for ICL Uma Shankar
                   ` (2 preceding siblings ...)
  2018-10-24 17:41 ` ✗ Fi.CI.CHECKPATCH: warning for Enable Plane Input CSC for ICL Patchwork
@ 2018-10-24 17:42 ` Patchwork
  2018-10-24 17:59 ` ✓ Fi.CI.BAT: success " Patchwork
  2018-10-24 22:55 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2018-10-24 17:42 UTC (permalink / raw)
  To: Uma Shankar; +Cc: intel-gfx

== Series Details ==

Series: Enable Plane Input CSC for ICL
URL   : https://patchwork.freedesktop.org/series/51463/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Sparse version: v0.5.2
Commit: drm/i915/icl: Define Plane Input CSC Coefficient Registers
Okay!

Commit: drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
+drivers/gpu/drm/i915/intel_color.c:101:33: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:101:33: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:101:33: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:101:33: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:101:33: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:101:33: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/intel_color.c:101:33: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/intel_color.c:101:33: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/intel_color.c:101:33: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/intel_color.c:101:33: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/intel_color.c:101:33: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/intel_color.c:101:33: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/intel_color.c:101:33: warning: expression using sizeof(void)
-drivers/gpu/drm/i915/intel_color.c:101:33: warning: expression using sizeof(void)

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

^ permalink raw reply	[flat|nested] 13+ messages in thread
* ✓ Fi.CI.BAT: success for Enable Plane Input CSC for ICL
  2018-10-24 16:18 [v2 0/2] Enable Plane Input CSC for ICL Uma Shankar
                   ` (3 preceding siblings ...)
  2018-10-24 17:42 ` ✗ Fi.CI.SPARSE: " Patchwork
@ 2018-10-24 17:59 ` Patchwork
  2018-10-24 22:55 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2018-10-24 17:59 UTC (permalink / raw)
  To: Uma Shankar; +Cc: intel-gfx

== Series Details ==

Series: Enable Plane Input CSC for ICL
URL   : https://patchwork.freedesktop.org/series/51463/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5029 -> Patchwork_10566 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/51463/revisions/1/mbox/

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_exec_suspend@basic-s4-devices:
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    igt@kms_flip@basic-flip-vs-modeset:
      fi-skl-6700hq:      PASS -> DMESG-WARN (fdo#105998)

    igt@kms_pipe_crc_basic@read-crc-pipe-a:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-byt-clapper:     PASS -> FAIL (fdo#107362, fdo#103191)

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718


== Participating hosts (47 -> 43) ==

  Missing    (4): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan 


== Build changes ==

    * Linux: CI_DRM_5029 -> Patchwork_10566

  CI_DRM_5029: 3d6302c20ddeff4b564c71fa479871b8afc11c4b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4691: d445be01f5edc7e7a324444c73e221c9ed75602e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10566: 99c424470fe4bb50b67228965220aabfbbe9ce70 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

99c424470fe4 drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
41d6b4c477ae drm/i915/icl: Define Plane Input CSC Coefficient Registers

== Logs ==

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

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [v2 1/2] drm/i915/icl: Define Plane Input CSC Coefficient Registers
  2018-10-24 16:18 ` [v2 1/2] drm/i915/icl: Define Plane Input CSC Coefficient Registers Uma Shankar
@ 2018-10-24 20:31   ` Matt Roper
  2018-10-25 14:48     ` Shankar, Uma
  0 siblings, 1 reply; 13+ messages in thread
From: Matt Roper @ 2018-10-24 20:31 UTC (permalink / raw)
  To: Uma Shankar; +Cc: intel-gfx, ville.syrjala, maarten.lankhorst

On Wed, Oct 24, 2018 at 09:48:12PM +0530, Uma Shankar wrote:
> Defined the plane input csc coefficient registers and macros.
> 6 registers are used to program a total of 9 coefficients,
> added macros to define each of them for all the planes
> supporting the feature on pipes. On ICL, bottom 3 planes
> have this capability.
> 
> v2: Segregated the register macro definition as separate patch
> as per Maarten's suggestion.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 217 ++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 217 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 69eb573..6a363a32 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -6569,6 +6569,7 @@ enum {
>  #define _PLANE_COLOR_CTL_3_A			0x703CC /* GLK+ */
>  #define   PLANE_COLOR_PIPE_GAMMA_ENABLE		(1 << 30) /* Pre-ICL */
>  #define   PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE	(1 << 28)
> +#define   PLANE_COLOR_INPUT_CSC_ENABLE		(1 << 20) /* Pre-ICL */

Is the "Pre-ICL" comment here a copy/paste error?  The other registers
marked that way say "This field is deprecated" in the bspec, but that
isn't the case for the input CSC.

>  #define   PLANE_COLOR_PIPE_CSC_ENABLE		(1 << 23) /* Pre-ICL */
>  #define   PLANE_COLOR_CSC_MODE_BYPASS			(0 << 17)
>  #define   PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709		(1 << 17)
> @@ -6585,6 +6586,222 @@ enum {
>  #define _PLANE_NV12_BUF_CFG_1_A		0x70278
>  #define _PLANE_NV12_BUF_CFG_2_A		0x70378
>  
> +/* Input CSC Register Definitions */
> +#define _PLANE_INPUT_CSC_RY_GY_1_A	0x701E0
> +#define _PLANE_INPUT_CSC_RY_GY_2_A	0x702E0
> +#define _PLANE_INPUT_CSC_RY_GY_3_A	0x703E0

I don't think we usually add the third address to i915_reg.h when
planes/pipes are equally spaced since our macros can calculate all the
subsequent offsets automatically.  Same also applied to other regs
farther down.

> +
> +#define _PLANE_INPUT_CSC_RY_GY_1_B	0x711E0
> +#define _PLANE_INPUT_CSC_RY_GY_2_B	0x712E0
> +#define _PLANE_INPUT_CSC_RY_GY_3_B	0x713E0
> +
> +#define _PLANE_INPUT_CSC_RY_GY_1(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_RY_GY_1_A, \
> +	     _PLANE_INPUT_CSC_RY_GY_1_B)
> +#define _PLANE_INPUT_CSC_RY_GY_2(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_RY_GY_2_A, \
> +	     _PLANE_INPUT_CSC_RY_GY_2_B)
> +#define PLANE_INPUT_CSC_RY_GY(pipe, plane)	\
> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_RY_GY_1(pipe), \
> +		    _PLANE_INPUT_CSC_RY_GY_2(pipe))
> +
> +#define _PLANE_INPUT_CSC_BY_1_A		0x701E4
> +#define _PLANE_INPUT_CSC_BY_2_A		0x702E4
> +#define _PLANE_INPUT_CSC_BY_3_A		0x703E4
> +
> +#define _PLANE_INPUT_CSC_BY_1_B		0x711E4
> +#define _PLANE_INPUT_CSC_BY_2_B		0x712E4
> +#define _PLANE_INPUT_CSC_BY_3_B		0x713E4
> +
> +#define _PLANE_INPUT_CSC_BY_1(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_BY_1_A, \
> +	     _PLANE_INPUT_CSC_BY_1_B)
> +#define _PLANE_INPUT_CSC_BY_2(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_BY_2_A, \
> +	     _PLANE_INPUT_CSC_BY_2_B)

Since these are really just the next dword of the same logical register
as above, maybe it would be simpler to just define these (and the other
dwords of PLANE_INPUT_CSC_COEFF below) as _RY_GY + offset?


Matt

> +#define PLANE_INPUT_CSC_BY(pipe, plane)	\
> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_BY_1(pipe), \
> +		    _PLANE_INPUT_CSC_BY_2(pipe))
> +
> +#define _PLANE_INPUT_CSC_RU_GU_1_A	0x701E8
> +#define _PLANE_INPUT_CSC_RU_GU_2_A	0x702E8
> +#define _PLANE_INPUT_CSC_RU_GU_3_A	0x703E8
> +
> +#define _PLANE_INPUT_CSC_RU_GU_1_B	0x711E8
> +#define _PLANE_INPUT_CSC_RU_GU_2_B	0x712E8
> +#define _PLANE_INPUT_CSC_RU_GU_3_B	0x713E8
> +
> +#define _PLANE_INPUT_CSC_RU_GU_1(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_RU_GU_1_A, \
> +	     _PLANE_INPUT_CSC_RU_GU_1_B)
> +#define _PLANE_INPUT_CSC_RU_GU_2(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_RU_GU_2_A, \
> +	     _PLANE_INPUT_CSC_RU_GU_2_B)
> +#define PLANE_INPUT_CSC_RU_GU(pipe, plane)	\
> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_RU_GU_1(pipe), \
> +		    _PLANE_INPUT_CSC_RU_GU_2(pipe))
> +
> +#define _PLANE_INPUT_CSC_BU_1_A		0x701EC
> +#define _PLANE_INPUT_CSC_BU_2_A		0x702EC
> +#define _PLANE_INPUT_CSC_BU_3_A		0x703EC
> +
> +#define _PLANE_INPUT_CSC_BU_1_B		0x711EC
> +#define _PLANE_INPUT_CSC_BU_2_B		0x712EC
> +#define _PLANE_INPUT_CSC_BU_3_B		0x713EC
> +
> +#define _PLANE_INPUT_CSC_BU_1(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_BU_1_A, \
> +	     _PLANE_INPUT_CSC_BU_1_B)
> +#define _PLANE_INPUT_CSC_BU_2(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_BU_2_A, \
> +	     _PLANE_INPUT_CSC_BU_2_B)
> +#define PLANE_INPUT_CSC_BU(pipe, plane)	\
> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_BU_1(pipe), \
> +		    _PLANE_INPUT_CSC_BU_2(pipe))
> +
> +#define _PLANE_INPUT_CSC_RV_GV_1_A	0x701F0
> +#define _PLANE_INPUT_CSC_RV_GV_2_A	0x702F0
> +#define _PLANE_INPUT_CSC_RV_GV_3_A	0x703F0
> +
> +#define _PLANE_INPUT_CSC_RV_GV_1_B	0x711F0
> +#define _PLANE_INPUT_CSC_RV_GV_2_B	0x712F0
> +#define _PLANE_INPUT_CSC_RV_GV_3_B	0x713F0
> +
> +#define _PLANE_INPUT_CSC_RV_GV_1(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_RV_GV_1_A, \
> +	     _PLANE_INPUT_CSC_RV_GV_1_B)
> +#define _PLANE_INPUT_CSC_RV_GV_2(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_RV_GV_2_A, \
> +	     _PLANE_INPUT_CSC_RV_GV_2_B)
> +#define PLANE_INPUT_CSC_RV_GV(pipe, plane)	\
> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_RV_GV_1(pipe), \
> +		    _PLANE_INPUT_CSC_RV_GV_2(pipe))
> +
> +#define _PLANE_INPUT_CSC_BV_1_A		0x701F4
> +#define _PLANE_INPUT_CSC_BV_2_A		0x702F4
> +#define _PLANE_INPUT_CSC_BV_3_A		0x703F4
> +
> +#define _PLANE_INPUT_CSC_BV_1_B		0x711F4
> +#define _PLANE_INPUT_CSC_BV_2_B		0x712F4
> +#define _PLANE_INPUT_CSC_BV_3_B		0x713F4
> +
> +#define _PLANE_INPUT_CSC_BV_1(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_BV_1_A, \
> +	     _PLANE_INPUT_CSC_BV_1_B)
> +#define _PLANE_INPUT_CSC_BV_2(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_BV_2_A, \
> +	     _PLANE_INPUT_CSC_BV_2_B)
> +#define PLANE_INPUT_CSC_BV(pipe, plane)	\
> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_BV_1(pipe), \
> +		    _PLANE_INPUT_CSC_BV_2(pipe))
> +
> +#define _PLANE_INPUT_CSC_PREOFF_HI_1_A		0x701F8
> +#define _PLANE_INPUT_CSC_PREOFF_HI_2_A		0x702F8
> +#define _PLANE_INPUT_CSC_PREOFF_HI_3_A		0x703F8
> +
> +#define _PLANE_INPUT_CSC_PREOFF_HI_1_B		0x711F8
> +#define _PLANE_INPUT_CSC_PREOFF_HI_2_B		0x712F8
> +#define _PLANE_INPUT_CSC_PREOFF_HI_3_B		0x713F8
> +
> +#define _PLANE_INPUT_CSC_PREOFF_HI_1(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_HI_1_A, \
> +	     _PLANE_INPUT_CSC_PREOFF_HI_1_B)
> +#define _PLANE_INPUT_CSC_PREOFF_HI_2(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_HI_2_A, \
> +	     _PLANE_INPUT_CSC_PREOFF_HI_2_B)
> +#define PLANE_INPUT_CSC_PREOFF_HI(pipe, plane)	\
> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_PREOFF_HI_1(pipe), \
> +		    _PLANE_INPUT_CSC_PREOFF_HI_2(pipe))
> +
> +#define _PLANE_INPUT_CSC_PREOFF_ME_1_A		0x701FC
> +#define _PLANE_INPUT_CSC_PREOFF_ME_2_A		0x702FC
> +#define _PLANE_INPUT_CSC_PREOFF_ME_3_A		0x703FC
> +
> +#define _PLANE_INPUT_CSC_PREOFF_ME_1_B		0x711FC
> +#define _PLANE_INPUT_CSC_PREOFF_ME_2_B		0x712FC
> +#define _PLANE_INPUT_CSC_PREOFF_ME_3_B		0x713FC
> +
> +#define _PLANE_INPUT_CSC_PREOFF_ME_1(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_ME_1_A, \
> +	     _PLANE_INPUT_CSC_PREOFF_ME_1_B)
> +#define _PLANE_INPUT_CSC_PREOFF_ME_2(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_ME_2_A, \
> +	     _PLANE_INPUT_CSC_PREOFF_ME_2_B)
> +#define PLANE_INPUT_CSC_PREOFF_ME(pipe, plane)	\
> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_PREOFF_ME_1(pipe), \
> +		    _PLANE_INPUT_CSC_PREOFF_ME_2(pipe))
> +
> +#define _PLANE_INPUT_CSC_PREOFF_LO_1_A		0x70200
> +#define _PLANE_INPUT_CSC_PREOFF_LO_2_A		0x70300
> +#define _PLANE_INPUT_CSC_PREOFF_LO_3_A		0x70400
> +
> +#define _PLANE_INPUT_CSC_PREOFF_LO_1_B		0x71200
> +#define _PLANE_INPUT_CSC_PREOFF_LO_2_B		0x71300
> +#define _PLANE_INPUT_CSC_PREOFF_LO_3_B		0x71400
> +
> +#define _PLANE_INPUT_CSC_PREOFF_LO_1(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_LO_1_A, \
> +	     _PLANE_INPUT_CSC_PREOFF_LO_1_B)
> +#define _PLANE_INPUT_CSC_PREOFF_LO_2(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_LO_2_A, \
> +	     _PLANE_INPUT_CSC_PREOFF_LO_2_B)
> +#define PLANE_INPUT_CSC_PREOFF_LO(pipe, plane)	\
> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_PREOFF_LO_1(pipe), \
> +		    _PLANE_INPUT_CSC_PREOFF_LO_2(pipe))
> +
> +#define _PLANE_INPUT_CSC_POSTOFF_HI_1_A		0x70204
> +#define _PLANE_INPUT_CSC_POSTOFF_HI_2_A		0x70304
> +#define _PLANE_INPUT_CSC_POSTOFF_HI_3_A		0x70404
> +
> +#define _PLANE_INPUT_CSC_POSTOFF_HI_1_B		0x71204
> +#define _PLANE_INPUT_CSC_POSTOFF_HI_2_B		0x71304
> +#define _PLANE_INPUT_CSC_POSTOFF_HI_3_B		0x71404
> +
> +#define _PLANE_INPUT_CSC_POSTOFF_HI_1(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_HI_1_A, \
> +	     _PLANE_INPUT_CSC_POSTOFF_HI_1_B)
> +#define _PLANE_INPUT_CSC_POSTOFF_HI_2(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_HI_2_A, \
> +	     _PLANE_INPUT_CSC_POSTOFF_HI_2_B)
> +#define PLANE_INPUT_CSC_POSTOFF_HI(pipe, plane)	\
> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_POSTOFF_HI_1(pipe), \
> +		    _PLANE_INPUT_CSC_POSTOFF_HI_2(pipe))
> +
> +#define _PLANE_INPUT_CSC_POSTOFF_ME_1_A		0x70208
> +#define _PLANE_INPUT_CSC_POSTOFF_ME_2_A		0x70308
> +#define _PLANE_INPUT_CSC_POSTOFF_ME_3_A		0x70408
> +
> +#define _PLANE_INPUT_CSC_POSTOFF_ME_1_B		0x71208
> +#define _PLANE_INPUT_CSC_POSTOFF_ME_2_B		0x71308
> +#define _PLANE_INPUT_CSC_POSTOFF_ME_3_B		0x71408
> +
> +#define _PLANE_INPUT_CSC_POSTOFF_ME_1(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_ME_1_A, \
> +	     _PLANE_INPUT_CSC_POSTOFF_ME_1_B)
> +#define _PLANE_INPUT_CSC_POSTOFF_ME_2(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_ME_2_A, \
> +	     _PLANE_INPUT_CSC_POSTOFF_ME_2_B)
> +#define PLANE_INPUT_CSC_POSTOFF_ME(pipe, plane)	\
> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_POSTOFF_ME_1(pipe), \
> +		    _PLANE_INPUT_CSC_POSTOFF_ME_2(pipe))
> +
> +#define _PLANE_INPUT_CSC_POSTOFF_LO_1_A		0x7020C
> +#define _PLANE_INPUT_CSC_POSTOFF_LO_2_A		0x7030C
> +#define _PLANE_INPUT_CSC_POSTOFF_LO_3_A		0x7040C
> +
> +#define _PLANE_INPUT_CSC_POSTOFF_LO_1_B		0x7120C
> +#define _PLANE_INPUT_CSC_POSTOFF_LO_2_B		0x7130C
> +#define _PLANE_INPUT_CSC_POSTOFF_LO_3_B		0x7140C
> +
> +#define _PLANE_INPUT_CSC_POSTOFF_LO_1(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_LO_1_A, \
> +	     _PLANE_INPUT_CSC_POSTOFF_LO_1_B)
> +#define _PLANE_INPUT_CSC_POSTOFF_LO_2(pipe)	\
> +	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_LO_2_A, \
> +	     _PLANE_INPUT_CSC_POSTOFF_LO_2_B)
> +#define PLANE_INPUT_CSC_POSTOFF_LO(pipe, plane)	\
> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_POSTOFF_LO_1(pipe), \
> +		    _PLANE_INPUT_CSC_POSTOFF_LO_2(pipe))
>  
>  #define _PLANE_CTL_1_B				0x71180
>  #define _PLANE_CTL_2_B				0x71280
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread
* ✓ Fi.CI.IGT: success for Enable Plane Input CSC for ICL
  2018-10-24 16:18 [v2 0/2] Enable Plane Input CSC for ICL Uma Shankar
                   ` (4 preceding siblings ...)
  2018-10-24 17:59 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-10-24 22:55 ` Patchwork
  5 siblings, 0 replies; 13+ messages in thread
From: Patchwork @ 2018-10-24 22:55 UTC (permalink / raw)
  To: Uma Shankar; +Cc: intel-gfx

== Series Details ==

Series: Enable Plane Input CSC for ICL
URL   : https://patchwork.freedesktop.org/series/51463/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5029_full -> Patchwork_10566_full =

== Summary - SUCCESS ==

  No regressions found.

  

== Known issues ==

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

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_busy@extended-modeset-hang-newfb-render-a:
      shard-skl:          NOTRUN -> DMESG-WARN (fdo#107956) +2

    igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
      shard-snb:          NOTRUN -> DMESG-WARN (fdo#107956) +1

    igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
      shard-glk:          PASS -> DMESG-WARN (fdo#107956)

    igt@kms_chv_cursor_fail@pipe-a-128x128-bottom-edge:
      shard-skl:          NOTRUN -> FAIL (fdo#104671)

    igt@kms_cursor_crc@cursor-64x21-random:
      shard-apl:          PASS -> FAIL (fdo#103232) +1

    igt@kms_cursor_crc@cursor-64x64-sliding:
      shard-skl:          NOTRUN -> FAIL (fdo#103232)

    igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
      shard-skl:          PASS -> FAIL (fdo#102670)

    igt@kms_draw_crc@draw-method-xrgb2101010-mmap-cpu-xtiled:
      shard-skl:          NOTRUN -> FAIL (fdo#103184)

    igt@kms_fbcon_fbt@fbc-suspend:
      shard-skl:          NOTRUN -> FAIL (fdo#103833, fdo#105682)

    igt@kms_frontbuffer_tracking@fbc-2p-pri-indfb-multidraw:
      shard-glk:          PASS -> FAIL (fdo#103167) +1

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-blt:
      shard-skl:          NOTRUN -> FAIL (fdo#105682)

    igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-gtt:
      shard-skl:          NOTRUN -> FAIL (fdo#103167) +5

    igt@kms_mmap_write_crc:
      shard-skl:          NOTRUN -> FAIL (fdo#108228)

    igt@kms_plane@plane-position-covered-pipe-a-planes:
      shard-apl:          PASS -> FAIL (fdo#103166)

    igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
      shard-skl:          NOTRUN -> FAIL (fdo#107815, fdo#108145) +1

    igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
      shard-skl:          NOTRUN -> FAIL (fdo#108145) +2

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-glk:          PASS -> FAIL (fdo#103166)

    igt@kms_setmode@basic:
      shard-apl:          PASS -> FAIL (fdo#99912)

    igt@perf@blocking:
      shard-hsw:          PASS -> FAIL (fdo#102252)

    
    ==== Possible fixes ====

    igt@gem_ctx_switch@basic-all-light:
      shard-apl:          INCOMPLETE (fdo#103927) -> PASS

    igt@kms_cursor_crc@cursor-128x128-onscreen:
      shard-glk:          FAIL (fdo#103232) -> PASS +1

    igt@kms_flip@flip-vs-expired-vblank:
      shard-skl:          FAIL (fdo#105363) -> PASS

    igt@kms_flip@wf_vblank-ts-check:
      shard-apl:          DMESG-WARN (fdo#103558, fdo#105602) -> PASS +4

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
      shard-apl:          FAIL (fdo#103167) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-rte:
      shard-apl:          DMESG-WARN (fdo#103558, fdo#108131) -> PASS

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-fullscreen:
      shard-glk:          FAIL (fdo#103167) -> PASS

    igt@kms_universal_plane@universal-plane-pipe-a-functional:
      shard-glk:          FAIL (fdo#103166) -> PASS

    igt@pm_rpm@system-suspend-execbuf:
      shard-apl:          DMESG-WARN -> PASS

    
    ==== Warnings ====

    igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-gtt:
      shard-snb:          DMESG-WARN (fdo#107469) -> INCOMPLETE (fdo#105411)

    
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103184 https://bugs.freedesktop.org/show_bug.cgi?id=103184
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103558 https://bugs.freedesktop.org/show_bug.cgi?id=103558
  fdo#103833 https://bugs.freedesktop.org/show_bug.cgi?id=103833
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104671 https://bugs.freedesktop.org/show_bug.cgi?id=104671
  fdo#105363 https://bugs.freedesktop.org/show_bug.cgi?id=105363
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#105682 https://bugs.freedesktop.org/show_bug.cgi?id=105682
  fdo#107469 https://bugs.freedesktop.org/show_bug.cgi?id=107469
  fdo#107815 https://bugs.freedesktop.org/show_bug.cgi?id=107815
  fdo#107956 https://bugs.freedesktop.org/show_bug.cgi?id=107956
  fdo#108131 https://bugs.freedesktop.org/show_bug.cgi?id=108131
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#108228 https://bugs.freedesktop.org/show_bug.cgi?id=108228
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912


== Participating hosts (6 -> 6) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_5029 -> Patchwork_10566

  CI_DRM_5029: 3d6302c20ddeff4b564c71fa479871b8afc11c4b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4691: d445be01f5edc7e7a324444c73e221c9ed75602e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10566: 99c424470fe4bb50b67228965220aabfbbe9ce70 @ 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_10566/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [v2 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
  2018-10-24 16:18 ` [v2 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion Uma Shankar
@ 2018-10-24 23:22   ` Matt Roper
  2018-10-25 14:39     ` Shankar, Uma
  2018-10-25  9:47   ` Maarten Lankhorst
  1 sibling, 1 reply; 13+ messages in thread
From: Matt Roper @ 2018-10-24 23:22 UTC (permalink / raw)
  To: Uma Shankar; +Cc: intel-gfx, ville.syrjala, maarten.lankhorst

On Wed, Oct 24, 2018 at 09:48:13PM +0530, Uma Shankar wrote:
> Plane input CSC needs to be enabled to convert frambuffers from
> YUV to RGB. This is needed for bottom 3 planes on ICL, rest of
> the planes have hardcoded conversion and taken care by the legacy
> code.

Just to confirm, we're specifically talking YUV444(packed) -> RGB here,
right?  So for formats like NV12 (YUV 420), the chroma upsampler first
merges/scales the Y and UV planes to produce YUV444(packed) data which
then has this input CSC matrix applied to convert it to RGB?  And then
after that point there's a separate set of userspace-programmable
degamma/CSC/gamma set of steps, which is what your other series "[RFC v5
0/8] Add Plane Color Properties" adds, right?  And then after that comes
the pipe-level degamma/CSC/gamma...

Is there somewhere in the bspec that talks about this input CSC in more
detail?  I don't see any mention on the general YUV programming or plane
capability pages, and the register details themselves are pretty sparse,
so it's hard to fully review these for correctness.  Based on your code
here, it appears the INPUT_CSC follows the same general layout and
expected coefficient values as the pipe CSC, but I'm not sure where in
the spec to confirm that.


Matt

> 
> This patch defines the co-efficient values for YUV to RGB conversion
> in BT709 and BT601 formats. It programs the coefficients and enables
> the plane input csc unit in hardware.
> 
> Note: This is currently untested and floated to get an early feedback
> on the design and implementation for this feature. In parallel,
> I will test this on actual ICL hardware and confirm with planar
> formats.
> 
> v2: Addressed Maarten's and Ville's review comments and added the
> coefficients in a 2D array instead of independent Macros.
> 
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c   | 49 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c | 24 +++++++++++++-----
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  3 files changed, 69 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 5127da2..de733cf 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -57,6 +57,11 @@
>  #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8
>  #define CSC_RGB_TO_YUV_BV 0x1e080000
>  
> +/* Preoffset values for YUV to RGB Conversion */
> +#define PREOFF_YUV_TO_RGB_HI		0x800
> +#define PREOFF_YUV_TO_RGB_ME		0xF00
> +#define PREOFF_YUV_TO_RGB_LO		0x800
> +
>  /*
>   * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
>   * format). This macro takes the coefficient we want transformed and the
> @@ -643,6 +648,50 @@ int intel_color_check(struct drm_crtc *crtc,
>  	return -EINVAL;
>  }
>  
> +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state,
> +				 const struct intel_plane_state *plane_state)
> +{
> +	struct drm_i915_private *dev_priv =
> +		to_i915(plane_state->base.plane->dev);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	enum pipe pipe = crtc->pipe;
> +	struct intel_plane *intel_plane =
> +		to_intel_plane(plane_state->base.plane);
> +	enum plane_id plane = intel_plane->id;
> +
> +	static const u32 input_csc_matrix[][6] = {
> +		/* BT.601 full range YCbCr -> full range RGB */
> +		[DRM_COLOR_YCBCR_BT601] = {
> +			0x7AF87800, 0x0, 0x8B287800,
> +			0x9AC0, 0x7800, 0x7DD8,
> +		},
> +		/* BT.709 full range YCbCr -> full range RGB */
> +		[DRM_COLOR_YCBCR_BT709] = {
> +			0x7C987800, 0x0, 0x9EF87800,
> +			0xABF8, 0x7800,  0x7ED8,
> +		},
> +	};
> +	const u32 *csc = input_csc_matrix[plane_state->base.color_encoding];
> +
> +	I915_WRITE(PLANE_INPUT_CSC_RY_GY(pipe, plane), csc[0]);
> +	I915_WRITE(PLANE_INPUT_CSC_BY(pipe, plane), csc[1]);
> +	I915_WRITE(PLANE_INPUT_CSC_RU_GU(pipe, plane), csc[2]);
> +	I915_WRITE(PLANE_INPUT_CSC_BU(pipe, plane), csc[3]);
> +	I915_WRITE(PLANE_INPUT_CSC_RV_GV(pipe, plane), csc[4]);
> +	I915_WRITE(PLANE_INPUT_CSC_BV(pipe, plane), csc[5]);
> +
> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF_HI(pipe, plane),
> +		   PREOFF_YUV_TO_RGB_HI);
> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF_ME(pipe, plane),
> +		   PREOFF_YUV_TO_RGB_ME);
> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF_LO(pipe, plane),
> +		   PREOFF_YUV_TO_RGB_LO);
> +
> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF_HI(pipe, plane), 0x0);
> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF_ME(pipe, plane), 0x0);
> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF_LO(pipe, plane), 0x0);
> +}
> +
>  void intel_color_init(struct drm_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fe045ab..be65419 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3666,6 +3666,8 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
>  	struct drm_i915_private *dev_priv =
>  		to_i915(plane_state->base.plane->dev);
>  	const struct drm_framebuffer *fb = plane_state->base.fb;
> +	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> +
>  	u32 plane_color_ctl = 0;
>  
>  	if (INTEL_GEN(dev_priv) < 11) {
> @@ -3676,13 +3678,23 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
>  	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
>  
>  	if (fb->format->is_yuv) {
> -		if (plane_state->base.color_encoding == DRM_COLOR_YCBCR_BT709)
> -			plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> -		else
> -			plane_color_ctl |= PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
> +		if (!icl_is_hdr_plane(plane)) {
> +			if (plane_state->base.color_encoding ==
> +					DRM_COLOR_YCBCR_BT709)
> +				plane_color_ctl |=
> +					PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> +			else
> +				plane_color_ctl |=
> +					PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
>  
> -		if (plane_state->base.color_range == DRM_COLOR_YCBCR_FULL_RANGE)
> -			plane_color_ctl |= PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> +			if (plane_state->base.color_range ==
> +					DRM_COLOR_YCBCR_FULL_RANGE)
> +				plane_color_ctl |=
> +				PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> +		} else {
> +			icl_program_input_csc_coeff(crtc_state, plane_state);
> +			plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
> +		}
>  	}
>  
>  	return plane_color_ctl;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index db24308..bd9e946 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2285,6 +2285,8 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
>  int intel_color_check(struct drm_crtc *crtc, struct drm_crtc_state *state);
>  void intel_color_set_csc(struct drm_crtc_state *crtc_state);
>  void intel_color_load_luts(struct drm_crtc_state *crtc_state);
> +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state,
> +				 const struct intel_plane_state *plane_state);
>  
>  /* intel_lspcon.c */
>  bool lspcon_init(struct intel_digital_port *intel_dig_port);
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Matt Roper
Graphics Software Engineer
IoTG Platform Enabling & Development
Intel Corporation
(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [v2 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
  2018-10-24 16:18 ` [v2 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion Uma Shankar
  2018-10-24 23:22   ` Matt Roper
@ 2018-10-25  9:47   ` Maarten Lankhorst
  2018-10-25 16:03     ` Shankar, Uma
  1 sibling, 1 reply; 13+ messages in thread
From: Maarten Lankhorst @ 2018-10-25  9:47 UTC (permalink / raw)
  To: Uma Shankar, intel-gfx; +Cc: ville.syrjala, maarten.lankhorst

Op 24-10-18 om 18:18 schreef Uma Shankar:
> Plane input CSC needs to be enabled to convert frambuffers from
> YUV to RGB. This is needed for bottom 3 planes on ICL, rest of
> the planes have hardcoded conversion and taken care by the legacy
> code.
>
> This patch defines the co-efficient values for YUV to RGB conversion
> in BT709 and BT601 formats. It programs the coefficients and enables
> the plane input csc unit in hardware.
>
> Note: This is currently untested and floated to get an early feedback
> on the design and implementation for this feature. In parallel,
> I will test this on actual ICL hardware and confirm with planar
> formats.
>
> v2: Addressed Maarten's and Ville's review comments and added the
> coefficients in a 2D array instead of independent Macros.
>
> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c   | 49 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c | 24 +++++++++++++-----
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  3 files changed, 69 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 5127da2..de733cf 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -57,6 +57,11 @@
>  #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8
>  #define CSC_RGB_TO_YUV_BV 0x1e080000
>  
> +/* Preoffset values for YUV to RGB Conversion */
> +#define PREOFF_YUV_TO_RGB_HI		0x800
> +#define PREOFF_YUV_TO_RGB_ME		0xF00
> +#define PREOFF_YUV_TO_RGB_LO		0x800
> +
>  /*
>   * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
>   * format). This macro takes the coefficient we want transformed and the
> @@ -643,6 +648,50 @@ int intel_color_check(struct drm_crtc *crtc,
>  	return -EINVAL;
>  }
>  
> +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state,
> +				 const struct intel_plane_state *plane_state)
> +{
> +	struct drm_i915_private *dev_priv =
> +		to_i915(plane_state->base.plane->dev);
> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
> +	enum pipe pipe = crtc->pipe;
> +	struct intel_plane *intel_plane =
> +		to_intel_plane(plane_state->base.plane);
> +	enum plane_id plane = intel_plane->id;
> +
> +	static const u32 input_csc_matrix[][6] = {
> +		/* BT.601 full range YCbCr -> full range RGB */
> +		[DRM_COLOR_YCBCR_BT601] = {
> +			0x7AF87800, 0x0, 0x8B287800,
> +			0x9AC0, 0x7800, 0x7DD8,
> +		},
Maybe each component separately? uint16 [][9]

Also BY/BU/BV are programmed in bits 31-16 according to bspec. :)
Seems that bug was already there in the original submission?

Otherwise looking much better!

And once more, I still miss any form of limited/full range handling?
> +		/* BT.709 full range YCbCr -> full range RGB */
> +		[DRM_COLOR_YCBCR_BT709] = {
> +			0x7C987800, 0x0, 0x9EF87800,
> +			0xABF8, 0x7800,  0x7ED8,
> +		},
> +	};
> +	const u32 *csc = input_csc_matrix[plane_state->base.color_encoding];
> +
> +	I915_WRITE(PLANE_INPUT_CSC_RY_GY(pipe, plane), csc[0]);
> +	I915_WRITE(PLANE_INPUT_CSC_BY(pipe, plane), csc[1]);
> +	I915_WRITE(PLANE_INPUT_CSC_RU_GU(pipe, plane), csc[2]);
> +	I915_WRITE(PLANE_INPUT_CSC_BU(pipe, plane), csc[3]);
> +	I915_WRITE(PLANE_INPUT_CSC_RV_GV(pipe, plane), csc[4]);
> +	I915_WRITE(PLANE_INPUT_CSC_BV(pipe, plane), csc[5]);
> +
> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF_HI(pipe, plane),
> +		   PREOFF_YUV_TO_RGB_HI);
> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF_ME(pipe, plane),
> +		   PREOFF_YUV_TO_RGB_ME);
> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF_LO(pipe, plane),
> +		   PREOFF_YUV_TO_RGB_LO);
> +
> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF_HI(pipe, plane), 0x0);
> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF_ME(pipe, plane), 0x0);
> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF_LO(pipe, plane), 0x0);
> +}
> +
>  void intel_color_init(struct drm_crtc *crtc)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index fe045ab..be65419 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -3666,6 +3666,8 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
>  	struct drm_i915_private *dev_priv =
>  		to_i915(plane_state->base.plane->dev);
>  	const struct drm_framebuffer *fb = plane_state->base.fb;
> +	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
> +
>  	u32 plane_color_ctl = 0;
>  
>  	if (INTEL_GEN(dev_priv) < 11) {
> @@ -3676,13 +3678,23 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state *crtc_state,
>  	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
>  
>  	if (fb->format->is_yuv) {
> +		if (!icl_is_hdr_plane(plane)) {
> +			if (plane_state->base.color_encoding ==
> +					DRM_COLOR_YCBCR_BT709)
> +				plane_color_ctl |=
> +					PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
> +			else
> +				plane_color_ctl |=
> +					PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
>  
> +			if (plane_state->base.color_range ==
> +					DRM_COLOR_YCBCR_FULL_RANGE)
> +				plane_color_ctl |=
> +				PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
> +		} else {
> +			icl_program_input_csc_coeff(crtc_state, plane_state);
> +			plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
> +		}
>  	}
>  
>  	return plane_color_ctl;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index db24308..bd9e946 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -2285,6 +2285,8 @@ int intel_plane_atomic_check_with_state(const struct intel_crtc_state *old_crtc_
>  int intel_color_check(struct drm_crtc *crtc, struct drm_crtc_state *state);
>  void intel_color_set_csc(struct drm_crtc_state *crtc_state);
>  void intel_color_load_luts(struct drm_crtc_state *crtc_state);
> +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state,
> +				 const struct intel_plane_state *plane_state);
>  
>  /* intel_lspcon.c */
>  bool lspcon_init(struct intel_digital_port *intel_dig_port);


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

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [v2 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
  2018-10-24 23:22   ` Matt Roper
@ 2018-10-25 14:39     ` Shankar, Uma
  0 siblings, 0 replies; 13+ messages in thread
From: Shankar, Uma @ 2018-10-25 14:39 UTC (permalink / raw)
  To: Roper, Matthew D; +Cc: intel-gfx, Syrjala, Ville, Lankhorst, Maarten



>-----Original Message-----
>From: Roper, Matthew D
>Sent: Thursday, October 25, 2018 4:52 AM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrjala@intel.com>;
>Lankhorst, Maarten <maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [v2 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to
>RGB Conversion
>
>On Wed, Oct 24, 2018 at 09:48:13PM +0530, Uma Shankar wrote:
>> Plane input CSC needs to be enabled to convert frambuffers from YUV to
>> RGB. This is needed for bottom 3 planes on ICL, rest of the planes
>> have hardcoded conversion and taken care by the legacy code.
>
>Just to confirm, we're specifically talking YUV444(packed) -> RGB here, right?  So
>for formats like NV12 (YUV 420), the chroma upsampler first merges/scales the Y
>and UV planes to produce YUV444(packed) data which then has this input CSC
>matrix applied to convert it to RGB?  And then after that point there's a separate
>set of userspace-programmable degamma/CSC/gamma set of steps, which is
>what your other series "[RFC v5 0/8] Add Plane Color Properties" adds, right?  And
>then after that comes the pipe-level degamma/CSC/gamma...

This is correct, all the planar formats first should be upsampled to YUV444 and then
the general  YUV->RGB conversion will happen using the Plane Input CSC hardware block.
Once this conversion is done, later the other plane CSC hardware (after de-gamma) can be
used to convert the colorspace if required (SRGB to BT2020 etc).

>Is there somewhere in the bspec that talks about this input CSC in more detail?  I
>don't see any mention on the general YUV programming or plane capability
>pages, and the register details themselves are pretty sparse, so it's hard to fully
>review these for correctness.  Based on your code here, it appears the
>INPUT_CSC follows the same general layout and expected coefficient values as
>the pipe CSC, but I'm not sure where in the spec to confirm that.

Unfortunately not much details are available in bspec, this is based on my understanding
of the hardware pipeline taking reference from few DCN document for HDR. But, in
general what you said is correct and this is how input csc hardware block is supposed to work.
Though it's a multipurpose programmable unit, but YUV444->RGB conversion is its primary
usecase in a real world scenario.

Regards,
Uma Shankar
>
>Matt
>
>>
>> This patch defines the co-efficient values for YUV to RGB conversion
>> in BT709 and BT601 formats. It programs the coefficients and enables
>> the plane input csc unit in hardware.
>>
>> Note: This is currently untested and floated to get an early feedback
>> on the design and implementation for this feature. In parallel, I will
>> test this on actual ICL hardware and confirm with planar formats.
>>
>> v2: Addressed Maarten's and Ville's review comments and added the
>> coefficients in a 2D array instead of independent Macros.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_color.c   | 49
>++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_display.c | 24 +++++++++++++-----
>>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>>  3 files changed, 69 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index 5127da2..de733cf 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -57,6 +57,11 @@
>>  #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8  #define
>CSC_RGB_TO_YUV_BV
>> 0x1e080000
>>
>> +/* Preoffset values for YUV to RGB Conversion */
>> +#define PREOFF_YUV_TO_RGB_HI		0x800
>> +#define PREOFF_YUV_TO_RGB_ME		0xF00
>> +#define PREOFF_YUV_TO_RGB_LO		0x800
>> +
>>  /*
>>   * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
>>   * format). This macro takes the coefficient we want transformed and
>> the @@ -643,6 +648,50 @@ int intel_color_check(struct drm_crtc *crtc,
>>  	return -EINVAL;
>>  }
>>
>> +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state,
>> +				 const struct intel_plane_state *plane_state) {
>> +	struct drm_i915_private *dev_priv =
>> +		to_i915(plane_state->base.plane->dev);
>> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> +	enum pipe pipe = crtc->pipe;
>> +	struct intel_plane *intel_plane =
>> +		to_intel_plane(plane_state->base.plane);
>> +	enum plane_id plane = intel_plane->id;
>> +
>> +	static const u32 input_csc_matrix[][6] = {
>> +		/* BT.601 full range YCbCr -> full range RGB */
>> +		[DRM_COLOR_YCBCR_BT601] = {
>> +			0x7AF87800, 0x0, 0x8B287800,
>> +			0x9AC0, 0x7800, 0x7DD8,
>> +		},
>> +		/* BT.709 full range YCbCr -> full range RGB */
>> +		[DRM_COLOR_YCBCR_BT709] = {
>> +			0x7C987800, 0x0, 0x9EF87800,
>> +			0xABF8, 0x7800,  0x7ED8,
>> +		},
>> +	};
>> +	const u32 *csc = input_csc_matrix[plane_state->base.color_encoding];
>> +
>> +	I915_WRITE(PLANE_INPUT_CSC_RY_GY(pipe, plane), csc[0]);
>> +	I915_WRITE(PLANE_INPUT_CSC_BY(pipe, plane), csc[1]);
>> +	I915_WRITE(PLANE_INPUT_CSC_RU_GU(pipe, plane), csc[2]);
>> +	I915_WRITE(PLANE_INPUT_CSC_BU(pipe, plane), csc[3]);
>> +	I915_WRITE(PLANE_INPUT_CSC_RV_GV(pipe, plane), csc[4]);
>> +	I915_WRITE(PLANE_INPUT_CSC_BV(pipe, plane), csc[5]);
>> +
>> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF_HI(pipe, plane),
>> +		   PREOFF_YUV_TO_RGB_HI);
>> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF_ME(pipe, plane),
>> +		   PREOFF_YUV_TO_RGB_ME);
>> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF_LO(pipe, plane),
>> +		   PREOFF_YUV_TO_RGB_LO);
>> +
>> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF_HI(pipe, plane), 0x0);
>> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF_ME(pipe, plane), 0x0);
>> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF_LO(pipe, plane), 0x0); }
>> +
>>  void intel_color_init(struct drm_crtc *crtc)  {
>>  	struct drm_i915_private *dev_priv = to_i915(crtc->dev); diff --git
>> a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index fe045ab..be65419 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -3666,6 +3666,8 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state
>*crtc_state,
>>  	struct drm_i915_private *dev_priv =
>>  		to_i915(plane_state->base.plane->dev);
>>  	const struct drm_framebuffer *fb = plane_state->base.fb;
>> +	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
>> +
>>  	u32 plane_color_ctl = 0;
>>
>>  	if (INTEL_GEN(dev_priv) < 11) {
>> @@ -3676,13 +3678,23 @@ u32 glk_plane_color_ctl(const struct
>intel_crtc_state *crtc_state,
>>  	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
>>
>>  	if (fb->format->is_yuv) {
>> -		if (plane_state->base.color_encoding ==
>DRM_COLOR_YCBCR_BT709)
>> -			plane_color_ctl |=
>PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
>> -		else
>> -			plane_color_ctl |=
>PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
>> +		if (!icl_is_hdr_plane(plane)) {
>> +			if (plane_state->base.color_encoding ==
>> +					DRM_COLOR_YCBCR_BT709)
>> +				plane_color_ctl |=
>> +
>	PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
>> +			else
>> +				plane_color_ctl |=
>> +
>	PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
>>
>> -		if (plane_state->base.color_range ==
>DRM_COLOR_YCBCR_FULL_RANGE)
>> -			plane_color_ctl |=
>PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
>> +			if (plane_state->base.color_range ==
>> +					DRM_COLOR_YCBCR_FULL_RANGE)
>> +				plane_color_ctl |=
>> +
>	PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
>> +		} else {
>> +			icl_program_input_csc_coeff(crtc_state, plane_state);
>> +			plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
>> +		}
>>  	}
>>
>>  	return plane_color_ctl;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index db24308..bd9e946 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -2285,6 +2285,8 @@ int intel_plane_atomic_check_with_state(const
>> struct intel_crtc_state *old_crtc_  int intel_color_check(struct
>> drm_crtc *crtc, struct drm_crtc_state *state);  void
>> intel_color_set_csc(struct drm_crtc_state *crtc_state);  void
>> intel_color_load_luts(struct drm_crtc_state *crtc_state);
>> +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state,
>> +				 const struct intel_plane_state *plane_state);
>>
>>  /* intel_lspcon.c */
>>  bool lspcon_init(struct intel_digital_port *intel_dig_port);
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Matt Roper
>Graphics Software Engineer
>IoTG Platform Enabling & Development
>Intel Corporation
>(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [v2 1/2] drm/i915/icl: Define Plane Input CSC Coefficient Registers
  2018-10-24 20:31   ` Matt Roper
@ 2018-10-25 14:48     ` Shankar, Uma
  0 siblings, 0 replies; 13+ messages in thread
From: Shankar, Uma @ 2018-10-25 14:48 UTC (permalink / raw)
  To: Roper, Matthew D; +Cc: intel-gfx, Syrjala, Ville, Lankhorst, Maarten



>-----Original Message-----
>From: Roper, Matthew D
>Sent: Thursday, October 25, 2018 2:02 AM
>To: Shankar, Uma <uma.shankar@intel.com>
>Cc: intel-gfx@lists.freedesktop.org; Syrjala, Ville <ville.syrjala@intel.com>;
>Lankhorst, Maarten <maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [v2 1/2] drm/i915/icl: Define Plane Input CSC Coefficient
>Registers
>
>On Wed, Oct 24, 2018 at 09:48:12PM +0530, Uma Shankar wrote:
>> Defined the plane input csc coefficient registers and macros.
>> 6 registers are used to program a total of 9 coefficients, added
>> macros to define each of them for all the planes supporting the
>> feature on pipes. On ICL, bottom 3 planes have this capability.
>>
>> v2: Segregated the register macro definition as separate patch as per
>> Maarten's suggestion.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h | 217
>> ++++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 217 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h index 69eb573..6a363a32 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -6569,6 +6569,7 @@ enum {
>>  #define _PLANE_COLOR_CTL_3_A			0x703CC /* GLK+ */
>>  #define   PLANE_COLOR_PIPE_GAMMA_ENABLE		(1 << 30) /*
>Pre-ICL */
>>  #define   PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE	(1 << 28)
>> +#define   PLANE_COLOR_INPUT_CSC_ENABLE		(1 << 20) /* Pre-ICL */
>
>Is the "Pre-ICL" comment here a copy/paste error?  The other registers marked
>that way say "This field is deprecated" in the bspec, but that isn't the case for the
>input CSC.

Yes, this should be ICL+. I will update that. Thanks

>>  #define   PLANE_COLOR_PIPE_CSC_ENABLE		(1 << 23) /* Pre-ICL */
>>  #define   PLANE_COLOR_CSC_MODE_BYPASS			(0 << 17)
>>  #define   PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709		(1 <<
>17)
>> @@ -6585,6 +6586,222 @@ enum {
>>  #define _PLANE_NV12_BUF_CFG_1_A		0x70278
>>  #define _PLANE_NV12_BUF_CFG_2_A		0x70378
>>
>> +/* Input CSC Register Definitions */
>> +#define _PLANE_INPUT_CSC_RY_GY_1_A	0x701E0
>> +#define _PLANE_INPUT_CSC_RY_GY_2_A	0x702E0
>> +#define _PLANE_INPUT_CSC_RY_GY_3_A	0x703E0
>
>I don't think we usually add the third address to i915_reg.h when planes/pipes are
>equally spaced since our macros can calculate all the subsequent offsets
>automatically.  Same also applied to other regs farther down.

Yes, there are some PLANE MACROS defined using 3rd plane, but it's really not
required, Will drop the same.

>> +
>> +#define _PLANE_INPUT_CSC_RY_GY_1_B	0x711E0
>> +#define _PLANE_INPUT_CSC_RY_GY_2_B	0x712E0
>> +#define _PLANE_INPUT_CSC_RY_GY_3_B	0x713E0
>> +
>> +#define _PLANE_INPUT_CSC_RY_GY_1(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_RY_GY_1_A, \
>> +	     _PLANE_INPUT_CSC_RY_GY_1_B)
>> +#define _PLANE_INPUT_CSC_RY_GY_2(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_RY_GY_2_A, \
>> +	     _PLANE_INPUT_CSC_RY_GY_2_B)
>> +#define PLANE_INPUT_CSC_RY_GY(pipe, plane)	\
>> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_RY_GY_1(pipe), \
>> +		    _PLANE_INPUT_CSC_RY_GY_2(pipe))
>> +
>> +#define _PLANE_INPUT_CSC_BY_1_A		0x701E4
>> +#define _PLANE_INPUT_CSC_BY_2_A		0x702E4
>> +#define _PLANE_INPUT_CSC_BY_3_A		0x703E4
>> +
>> +#define _PLANE_INPUT_CSC_BY_1_B		0x711E4
>> +#define _PLANE_INPUT_CSC_BY_2_B		0x712E4
>> +#define _PLANE_INPUT_CSC_BY_3_B		0x713E4
>> +
>> +#define _PLANE_INPUT_CSC_BY_1(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_BY_1_A, \
>> +	     _PLANE_INPUT_CSC_BY_1_B)
>> +#define _PLANE_INPUT_CSC_BY_2(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_BY_2_A, \
>> +	     _PLANE_INPUT_CSC_BY_2_B)
>
>Since these are really just the next dword of the same logical register as above,
>maybe it would be simpler to just define these (and the other dwords of
>PLANE_INPUT_CSC_COEFF below) as _RY_GY + offset?

Yes, this can be done. Will modify these macros. 

Thanks Matt for the review comments. Will address and refloat the series.

Regards,
Uma Shankar

>
>Matt
>
>> +#define PLANE_INPUT_CSC_BY(pipe, plane)	\
>> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_BY_1(pipe), \
>> +		    _PLANE_INPUT_CSC_BY_2(pipe))
>> +
>> +#define _PLANE_INPUT_CSC_RU_GU_1_A	0x701E8
>> +#define _PLANE_INPUT_CSC_RU_GU_2_A	0x702E8
>> +#define _PLANE_INPUT_CSC_RU_GU_3_A	0x703E8
>> +
>> +#define _PLANE_INPUT_CSC_RU_GU_1_B	0x711E8
>> +#define _PLANE_INPUT_CSC_RU_GU_2_B	0x712E8
>> +#define _PLANE_INPUT_CSC_RU_GU_3_B	0x713E8
>> +
>> +#define _PLANE_INPUT_CSC_RU_GU_1(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_RU_GU_1_A, \
>> +	     _PLANE_INPUT_CSC_RU_GU_1_B)
>> +#define _PLANE_INPUT_CSC_RU_GU_2(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_RU_GU_2_A, \
>> +	     _PLANE_INPUT_CSC_RU_GU_2_B)
>> +#define PLANE_INPUT_CSC_RU_GU(pipe, plane)	\
>> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_RU_GU_1(pipe), \
>> +		    _PLANE_INPUT_CSC_RU_GU_2(pipe))
>> +
>> +#define _PLANE_INPUT_CSC_BU_1_A		0x701EC
>> +#define _PLANE_INPUT_CSC_BU_2_A		0x702EC
>> +#define _PLANE_INPUT_CSC_BU_3_A		0x703EC
>> +
>> +#define _PLANE_INPUT_CSC_BU_1_B		0x711EC
>> +#define _PLANE_INPUT_CSC_BU_2_B		0x712EC
>> +#define _PLANE_INPUT_CSC_BU_3_B		0x713EC
>> +
>> +#define _PLANE_INPUT_CSC_BU_1(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_BU_1_A, \
>> +	     _PLANE_INPUT_CSC_BU_1_B)
>> +#define _PLANE_INPUT_CSC_BU_2(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_BU_2_A, \
>> +	     _PLANE_INPUT_CSC_BU_2_B)
>> +#define PLANE_INPUT_CSC_BU(pipe, plane)	\
>> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_BU_1(pipe), \
>> +		    _PLANE_INPUT_CSC_BU_2(pipe))
>> +
>> +#define _PLANE_INPUT_CSC_RV_GV_1_A	0x701F0
>> +#define _PLANE_INPUT_CSC_RV_GV_2_A	0x702F0
>> +#define _PLANE_INPUT_CSC_RV_GV_3_A	0x703F0
>> +
>> +#define _PLANE_INPUT_CSC_RV_GV_1_B	0x711F0
>> +#define _PLANE_INPUT_CSC_RV_GV_2_B	0x712F0
>> +#define _PLANE_INPUT_CSC_RV_GV_3_B	0x713F0
>> +
>> +#define _PLANE_INPUT_CSC_RV_GV_1(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_RV_GV_1_A, \
>> +	     _PLANE_INPUT_CSC_RV_GV_1_B)
>> +#define _PLANE_INPUT_CSC_RV_GV_2(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_RV_GV_2_A, \
>> +	     _PLANE_INPUT_CSC_RV_GV_2_B)
>> +#define PLANE_INPUT_CSC_RV_GV(pipe, plane)	\
>> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_RV_GV_1(pipe), \
>> +		    _PLANE_INPUT_CSC_RV_GV_2(pipe))
>> +
>> +#define _PLANE_INPUT_CSC_BV_1_A		0x701F4
>> +#define _PLANE_INPUT_CSC_BV_2_A		0x702F4
>> +#define _PLANE_INPUT_CSC_BV_3_A		0x703F4
>> +
>> +#define _PLANE_INPUT_CSC_BV_1_B		0x711F4
>> +#define _PLANE_INPUT_CSC_BV_2_B		0x712F4
>> +#define _PLANE_INPUT_CSC_BV_3_B		0x713F4
>> +
>> +#define _PLANE_INPUT_CSC_BV_1(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_BV_1_A, \
>> +	     _PLANE_INPUT_CSC_BV_1_B)
>> +#define _PLANE_INPUT_CSC_BV_2(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_BV_2_A, \
>> +	     _PLANE_INPUT_CSC_BV_2_B)
>> +#define PLANE_INPUT_CSC_BV(pipe, plane)	\
>> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_BV_1(pipe), \
>> +		    _PLANE_INPUT_CSC_BV_2(pipe))
>> +
>> +#define _PLANE_INPUT_CSC_PREOFF_HI_1_A		0x701F8
>> +#define _PLANE_INPUT_CSC_PREOFF_HI_2_A		0x702F8
>> +#define _PLANE_INPUT_CSC_PREOFF_HI_3_A		0x703F8
>> +
>> +#define _PLANE_INPUT_CSC_PREOFF_HI_1_B		0x711F8
>> +#define _PLANE_INPUT_CSC_PREOFF_HI_2_B		0x712F8
>> +#define _PLANE_INPUT_CSC_PREOFF_HI_3_B		0x713F8
>> +
>> +#define _PLANE_INPUT_CSC_PREOFF_HI_1(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_HI_1_A, \
>> +	     _PLANE_INPUT_CSC_PREOFF_HI_1_B)
>> +#define _PLANE_INPUT_CSC_PREOFF_HI_2(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_HI_2_A, \
>> +	     _PLANE_INPUT_CSC_PREOFF_HI_2_B)
>> +#define PLANE_INPUT_CSC_PREOFF_HI(pipe, plane)	\
>> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_PREOFF_HI_1(pipe), \
>> +		    _PLANE_INPUT_CSC_PREOFF_HI_2(pipe))
>> +
>> +#define _PLANE_INPUT_CSC_PREOFF_ME_1_A		0x701FC
>> +#define _PLANE_INPUT_CSC_PREOFF_ME_2_A		0x702FC
>> +#define _PLANE_INPUT_CSC_PREOFF_ME_3_A		0x703FC
>> +
>> +#define _PLANE_INPUT_CSC_PREOFF_ME_1_B		0x711FC
>> +#define _PLANE_INPUT_CSC_PREOFF_ME_2_B		0x712FC
>> +#define _PLANE_INPUT_CSC_PREOFF_ME_3_B		0x713FC
>> +
>> +#define _PLANE_INPUT_CSC_PREOFF_ME_1(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_ME_1_A, \
>> +	     _PLANE_INPUT_CSC_PREOFF_ME_1_B)
>> +#define _PLANE_INPUT_CSC_PREOFF_ME_2(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_ME_2_A, \
>> +	     _PLANE_INPUT_CSC_PREOFF_ME_2_B)
>> +#define PLANE_INPUT_CSC_PREOFF_ME(pipe, plane)	\
>> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_PREOFF_ME_1(pipe), \
>> +		    _PLANE_INPUT_CSC_PREOFF_ME_2(pipe))
>> +
>> +#define _PLANE_INPUT_CSC_PREOFF_LO_1_A		0x70200
>> +#define _PLANE_INPUT_CSC_PREOFF_LO_2_A		0x70300
>> +#define _PLANE_INPUT_CSC_PREOFF_LO_3_A		0x70400
>> +
>> +#define _PLANE_INPUT_CSC_PREOFF_LO_1_B		0x71200
>> +#define _PLANE_INPUT_CSC_PREOFF_LO_2_B		0x71300
>> +#define _PLANE_INPUT_CSC_PREOFF_LO_3_B		0x71400
>> +
>> +#define _PLANE_INPUT_CSC_PREOFF_LO_1(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_LO_1_A, \
>> +	     _PLANE_INPUT_CSC_PREOFF_LO_1_B)
>> +#define _PLANE_INPUT_CSC_PREOFF_LO_2(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_PREOFF_LO_2_A, \
>> +	     _PLANE_INPUT_CSC_PREOFF_LO_2_B)
>> +#define PLANE_INPUT_CSC_PREOFF_LO(pipe, plane)	\
>> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_PREOFF_LO_1(pipe), \
>> +		    _PLANE_INPUT_CSC_PREOFF_LO_2(pipe))
>> +
>> +#define _PLANE_INPUT_CSC_POSTOFF_HI_1_A		0x70204
>> +#define _PLANE_INPUT_CSC_POSTOFF_HI_2_A		0x70304
>> +#define _PLANE_INPUT_CSC_POSTOFF_HI_3_A		0x70404
>> +
>> +#define _PLANE_INPUT_CSC_POSTOFF_HI_1_B		0x71204
>> +#define _PLANE_INPUT_CSC_POSTOFF_HI_2_B		0x71304
>> +#define _PLANE_INPUT_CSC_POSTOFF_HI_3_B		0x71404
>> +
>> +#define _PLANE_INPUT_CSC_POSTOFF_HI_1(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_HI_1_A, \
>> +	     _PLANE_INPUT_CSC_POSTOFF_HI_1_B)
>> +#define _PLANE_INPUT_CSC_POSTOFF_HI_2(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_HI_2_A, \
>> +	     _PLANE_INPUT_CSC_POSTOFF_HI_2_B)
>> +#define PLANE_INPUT_CSC_POSTOFF_HI(pipe, plane)	\
>> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_POSTOFF_HI_1(pipe), \
>> +		    _PLANE_INPUT_CSC_POSTOFF_HI_2(pipe))
>> +
>> +#define _PLANE_INPUT_CSC_POSTOFF_ME_1_A		0x70208
>> +#define _PLANE_INPUT_CSC_POSTOFF_ME_2_A		0x70308
>> +#define _PLANE_INPUT_CSC_POSTOFF_ME_3_A		0x70408
>> +
>> +#define _PLANE_INPUT_CSC_POSTOFF_ME_1_B		0x71208
>> +#define _PLANE_INPUT_CSC_POSTOFF_ME_2_B		0x71308
>> +#define _PLANE_INPUT_CSC_POSTOFF_ME_3_B		0x71408
>> +
>> +#define _PLANE_INPUT_CSC_POSTOFF_ME_1(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_ME_1_A, \
>> +	     _PLANE_INPUT_CSC_POSTOFF_ME_1_B)
>> +#define _PLANE_INPUT_CSC_POSTOFF_ME_2(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_ME_2_A, \
>> +	     _PLANE_INPUT_CSC_POSTOFF_ME_2_B)
>> +#define PLANE_INPUT_CSC_POSTOFF_ME(pipe, plane)	\
>> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_POSTOFF_ME_1(pipe), \
>> +		    _PLANE_INPUT_CSC_POSTOFF_ME_2(pipe))
>> +
>> +#define _PLANE_INPUT_CSC_POSTOFF_LO_1_A		0x7020C
>> +#define _PLANE_INPUT_CSC_POSTOFF_LO_2_A		0x7030C
>> +#define _PLANE_INPUT_CSC_POSTOFF_LO_3_A		0x7040C
>> +
>> +#define _PLANE_INPUT_CSC_POSTOFF_LO_1_B		0x7120C
>> +#define _PLANE_INPUT_CSC_POSTOFF_LO_2_B		0x7130C
>> +#define _PLANE_INPUT_CSC_POSTOFF_LO_3_B		0x7140C
>> +
>> +#define _PLANE_INPUT_CSC_POSTOFF_LO_1(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_LO_1_A, \
>> +	     _PLANE_INPUT_CSC_POSTOFF_LO_1_B)
>> +#define _PLANE_INPUT_CSC_POSTOFF_LO_2(pipe)	\
>> +	_PIPE(pipe, _PLANE_INPUT_CSC_POSTOFF_LO_2_A, \
>> +	     _PLANE_INPUT_CSC_POSTOFF_LO_2_B)
>> +#define PLANE_INPUT_CSC_POSTOFF_LO(pipe, plane)	\
>> +	_MMIO_PLANE(plane, _PLANE_INPUT_CSC_POSTOFF_LO_1(pipe), \
>> +		    _PLANE_INPUT_CSC_POSTOFF_LO_2(pipe))
>>
>>  #define _PLANE_CTL_1_B				0x71180
>>  #define _PLANE_CTL_2_B				0x71280
>> --
>> 1.9.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
>--
>Matt Roper
>Graphics Software Engineer
>IoTG Platform Enabling & Development
>Intel Corporation
>(916) 356-2795
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 13+ messages in thread
* Re: [v2 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion
  2018-10-25  9:47   ` Maarten Lankhorst
@ 2018-10-25 16:03     ` Shankar, Uma
  0 siblings, 0 replies; 13+ messages in thread
From: Shankar, Uma @ 2018-10-25 16:03 UTC (permalink / raw)
  To: Maarten Lankhorst, intel-gfx; +Cc: Syrjala, Ville, Lankhorst, Maarten



>-----Original Message-----
>From: Maarten Lankhorst [mailto:maarten.lankhorst@linux.intel.com]
>Sent: Thursday, October 25, 2018 3:17 PM
>To: Shankar, Uma <uma.shankar@intel.com>; intel-gfx@lists.freedesktop.org
>Cc: Syrjala, Ville <ville.syrjala@intel.com>; Lankhorst, Maarten
><maarten.lankhorst@intel.com>
>Subject: Re: [Intel-gfx] [v2 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to
>RGB Conversion
>
>Op 24-10-18 om 18:18 schreef Uma Shankar:
>> Plane input CSC needs to be enabled to convert frambuffers from YUV to
>> RGB. This is needed for bottom 3 planes on ICL, rest of the planes
>> have hardcoded conversion and taken care by the legacy code.
>>
>> This patch defines the co-efficient values for YUV to RGB conversion
>> in BT709 and BT601 formats. It programs the coefficients and enables
>> the plane input csc unit in hardware.
>>
>> Note: This is currently untested and floated to get an early feedback
>> on the design and implementation for this feature. In parallel, I will
>> test this on actual ICL hardware and confirm with planar formats.
>>
>> v2: Addressed Maarten's and Ville's review comments and added the
>> coefficients in a 2D array instead of independent Macros.
>>
>> Signed-off-by: Uma Shankar <uma.shankar@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_color.c   | 49
>++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_display.c | 24 +++++++++++++-----
>>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>>  3 files changed, 69 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_color.c
>> b/drivers/gpu/drm/i915/intel_color.c
>> index 5127da2..de733cf 100644
>> --- a/drivers/gpu/drm/i915/intel_color.c
>> +++ b/drivers/gpu/drm/i915/intel_color.c
>> @@ -57,6 +57,11 @@
>>  #define CSC_RGB_TO_YUV_RV_GV 0xbce89ad8  #define
>CSC_RGB_TO_YUV_BV
>> 0x1e080000
>>
>> +/* Preoffset values for YUV to RGB Conversion */
>> +#define PREOFF_YUV_TO_RGB_HI		0x800
>> +#define PREOFF_YUV_TO_RGB_ME		0xF00
>> +#define PREOFF_YUV_TO_RGB_LO		0x800
>> +
>>  /*
>>   * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
>>   * format). This macro takes the coefficient we want transformed and
>> the @@ -643,6 +648,50 @@ int intel_color_check(struct drm_crtc *crtc,
>>  	return -EINVAL;
>>  }
>>
>> +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state,
>> +				 const struct intel_plane_state *plane_state) {
>> +	struct drm_i915_private *dev_priv =
>> +		to_i915(plane_state->base.plane->dev);
>> +	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
>> +	enum pipe pipe = crtc->pipe;
>> +	struct intel_plane *intel_plane =
>> +		to_intel_plane(plane_state->base.plane);
>> +	enum plane_id plane = intel_plane->id;
>> +
>> +	static const u32 input_csc_matrix[][6] = {
>> +		/* BT.601 full range YCbCr -> full range RGB */
>> +		[DRM_COLOR_YCBCR_BT601] = {
>> +			0x7AF87800, 0x0, 0x8B287800,
>> +			0x9AC0, 0x7800, 0x7DD8,
>> +		},
>Maybe each component separately? uint16 [][9]

Sure, I will do that.

>Also BY/BU/BV are programmed in bits 31-16 according to bspec. :) Seems that
>bug was already there in the original submission?

Yes, thanks for spotting this. Will correct it.

>
>Otherwise looking much better!
>
>And once more, I still miss any form of limited/full range handling?

My plan is to add limited range and other colorspace in a later patch since
we need to calculate the exact coefficients for those cases. If its ok, I can
re-submit the next version just limiting to full range as of now. 

Regards,
Uma Shankar

>> +		/* BT.709 full range YCbCr -> full range RGB */
>> +		[DRM_COLOR_YCBCR_BT709] = {
>> +			0x7C987800, 0x0, 0x9EF87800,
>> +			0xABF8, 0x7800,  0x7ED8,
>> +		},
>> +	};
>> +	const u32 *csc = input_csc_matrix[plane_state->base.color_encoding];
>> +
>> +	I915_WRITE(PLANE_INPUT_CSC_RY_GY(pipe, plane), csc[0]);
>> +	I915_WRITE(PLANE_INPUT_CSC_BY(pipe, plane), csc[1]);
>> +	I915_WRITE(PLANE_INPUT_CSC_RU_GU(pipe, plane), csc[2]);
>> +	I915_WRITE(PLANE_INPUT_CSC_BU(pipe, plane), csc[3]);
>> +	I915_WRITE(PLANE_INPUT_CSC_RV_GV(pipe, plane), csc[4]);
>> +	I915_WRITE(PLANE_INPUT_CSC_BV(pipe, plane), csc[5]);
>> +
>> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF_HI(pipe, plane),
>> +		   PREOFF_YUV_TO_RGB_HI);
>> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF_ME(pipe, plane),
>> +		   PREOFF_YUV_TO_RGB_ME);
>> +	I915_WRITE(PLANE_INPUT_CSC_PREOFF_LO(pipe, plane),
>> +		   PREOFF_YUV_TO_RGB_LO);
>> +
>> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF_HI(pipe, plane), 0x0);
>> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF_ME(pipe, plane), 0x0);
>> +	I915_WRITE(PLANE_INPUT_CSC_POSTOFF_LO(pipe, plane), 0x0); }
>> +
>>  void intel_color_init(struct drm_crtc *crtc)  {
>>  	struct drm_i915_private *dev_priv = to_i915(crtc->dev); diff --git
>> a/drivers/gpu/drm/i915/intel_display.c
>> b/drivers/gpu/drm/i915/intel_display.c
>> index fe045ab..be65419 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -3666,6 +3666,8 @@ u32 glk_plane_color_ctl(const struct intel_crtc_state
>*crtc_state,
>>  	struct drm_i915_private *dev_priv =
>>  		to_i915(plane_state->base.plane->dev);
>>  	const struct drm_framebuffer *fb = plane_state->base.fb;
>> +	struct intel_plane *plane = to_intel_plane(plane_state->base.plane);
>> +
>>  	u32 plane_color_ctl = 0;
>>
>>  	if (INTEL_GEN(dev_priv) < 11) {
>> @@ -3676,13 +3678,23 @@ u32 glk_plane_color_ctl(const struct
>intel_crtc_state *crtc_state,
>>  	plane_color_ctl |= glk_plane_color_ctl_alpha(plane_state);
>>
>>  	if (fb->format->is_yuv) {
>> +		if (!icl_is_hdr_plane(plane)) {
>> +			if (plane_state->base.color_encoding ==
>> +					DRM_COLOR_YCBCR_BT709)
>> +				plane_color_ctl |=
>> +
>	PLANE_COLOR_CSC_MODE_YUV709_TO_RGB709;
>> +			else
>> +				plane_color_ctl |=
>> +
>	PLANE_COLOR_CSC_MODE_YUV601_TO_RGB709;
>>
>> +			if (plane_state->base.color_range ==
>> +					DRM_COLOR_YCBCR_FULL_RANGE)
>> +				plane_color_ctl |=
>> +
>	PLANE_COLOR_YUV_RANGE_CORRECTION_DISABLE;
>> +		} else {
>> +			icl_program_input_csc_coeff(crtc_state, plane_state);
>> +			plane_color_ctl |= PLANE_COLOR_INPUT_CSC_ENABLE;
>> +		}
>>  	}
>>
>>  	return plane_color_ctl;
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h
>> b/drivers/gpu/drm/i915/intel_drv.h
>> index db24308..bd9e946 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -2285,6 +2285,8 @@ int intel_plane_atomic_check_with_state(const
>> struct intel_crtc_state *old_crtc_  int intel_color_check(struct
>> drm_crtc *crtc, struct drm_crtc_state *state);  void
>> intel_color_set_csc(struct drm_crtc_state *crtc_state);  void
>> intel_color_load_luts(struct drm_crtc_state *crtc_state);
>> +void icl_program_input_csc_coeff(const struct intel_crtc_state *crtc_state,
>> +				 const struct intel_plane_state *plane_state);
>>
>>  /* intel_lspcon.c */
>>  bool lspcon_init(struct intel_digital_port *intel_dig_port);
>

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

^ permalink raw reply	[flat|nested] 13+ messages in thread
end of thread, other threads:[~2018-10-25 16:03 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-24 16:18 [v2 0/2] Enable Plane Input CSC for ICL Uma Shankar
2018-10-24 16:18 ` [v2 1/2] drm/i915/icl: Define Plane Input CSC Coefficient Registers Uma Shankar
2018-10-24 20:31   ` Matt Roper
2018-10-25 14:48     ` Shankar, Uma
2018-10-24 16:18 ` [v2 2/2] drm/i915/icl: Enable Plane Input CSC for YUV to RGB Conversion Uma Shankar
2018-10-24 23:22   ` Matt Roper
2018-10-25 14:39     ` Shankar, Uma
2018-10-25  9:47   ` Maarten Lankhorst
2018-10-25 16:03     ` Shankar, Uma
2018-10-24 17:41 ` ✗ Fi.CI.CHECKPATCH: warning for Enable Plane Input CSC for ICL Patchwork
2018-10-24 17:42 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-10-24 17:59 ` ✓ Fi.CI.BAT: success " Patchwork
2018-10-24 22:55 ` ✓ Fi.CI.IGT: " Patchwork

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