[PATCH 0/6] Pipe level color management V4

[PATCH 0/6] Pipe level color management V4

[Lionel Landwerlin]

This series introduces pipe level color management through a set of properties
attached to the CRTC. It also provides an implementation for some Intel
platforms.

This series is based of a previous set of patches by Shashank Sharma.

Cheers,

Lionel


Lionel Landwerlin (6):
  drm/i915: Extract out gamma table and CSC to their own file
  drm: introduce color correction properties
  drm/i915: enable CSC for pipe C
  drm/i915: enable legacy palette for pipe C
  drm/i915: Implement color management on bdw/skl/bxt/kbl
  drm/i915: Implement color management on chv

 Documentation/DocBook/gpu.tmpl       |  64 +++-
 drivers/gpu/drm/drm_atomic.c         | 102 +++++-
 drivers/gpu/drm/drm_atomic_helper.c  |  10 +
 drivers/gpu/drm/drm_crtc.c           |  35 +++
 drivers/gpu/drm/drm_crtc_helper.c    |  33 ++
 drivers/gpu/drm/i915/Makefile        |   1 +
 drivers/gpu/drm/i915/i915_drv.c      |  27 +-
 drivers/gpu/drm/i915/i915_drv.h      |   9 +
 drivers/gpu/drm/i915/i915_reg.h      | 105 ++++++-
 drivers/gpu/drm/i915/intel_color.c   | 586 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c | 175 ++---------
 drivers/gpu/drm/i915/intel_drv.h     |  14 +
 include/drm/drm_crtc.h               |  44 ++-
 include/drm/drm_crtc_helper.h        |   3 +
 include/uapi/drm/drm_mode.h          |  15 +
 15 files changed, 1044 insertions(+), 179 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_color.c

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

[PATCH 1/6] drm/i915: Extract out gamma table and CSC to their own file

[Lionel Landwerlin]

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/Makefile        |   1 +
 drivers/gpu/drm/i915/intel_color.c   | 174 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c | 163 +++-----------------------------
 drivers/gpu/drm/i915/intel_drv.h     |  10 ++
 4 files changed, 197 insertions(+), 151 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_color.c

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 0851de07..0516300 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -55,6 +55,7 @@ i915-y += intel_audio.o \
 	  intel_atomic.o \
 	  intel_atomic_plane.o \
 	  intel_bios.o \
+	  intel_color.o \
 	  intel_display.o \
 	  intel_fbc.o \
 	  intel_fifo_underrun.o \
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
new file mode 100644
index 0000000..39ca215
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -0,0 +1,174 @@
+/*
+ * Copyright © 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+ * DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#include "intel_drv.h"
+
+/*
+ * Set up the pipe CSC unit.
+ *
+ * Currently only full range RGB to limited range RGB conversion
+ * is supported, but eventually this should handle various
+ * RGB<->YCbCr scenarios as well.
+ */
+static void i9xx_load_csc_matrix(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int pipe = intel_crtc->pipe;
+	uint16_t coeff = 0x7800; /* 1.0 */
+
+	/*
+	 * TODO: Check what kind of values actually come out of the pipe
+	 * with these coeff/postoff values and adjust to get the best
+	 * accuracy. Perhaps we even need to take the bpc value into
+	 * consideration.
+	 */
+
+	if (intel_crtc->config->limited_color_range)
+		coeff = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */
+
+	/*
+	 * GY/GU and RY/RU should be the other way around according
+	 * to BSpec, but reality doesn't agree. Just set them up in
+	 * a way that results in the correct picture.
+	 */
+	I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeff << 16);
+	I915_WRITE(PIPE_CSC_COEFF_BY(pipe), 0);
+
+	I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeff);
+	I915_WRITE(PIPE_CSC_COEFF_BU(pipe), 0);
+
+	I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), 0);
+	I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeff << 16);
+
+	I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0);
+	I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0);
+	I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0);
+
+	if (INTEL_INFO(dev)->gen > 6) {
+		uint16_t postoff = 0;
+
+		if (intel_crtc->config->limited_color_range)
+			postoff = (16 * (1 << 12) / 255) & 0x1fff;
+
+		I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff);
+		I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff);
+		I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff);
+
+		I915_WRITE(PIPE_CSC_MODE(pipe), 0);
+	} else {
+		uint32_t mode = CSC_MODE_YUV_TO_RGB;
+
+		if (intel_crtc->config->limited_color_range)
+			mode |= CSC_BLACK_SCREEN_OFFSET;
+
+		I915_WRITE(PIPE_CSC_MODE(pipe), mode);
+	}
+}
+
+void intel_color_set_csc(struct drm_crtc *crtc)
+{
+	i9xx_load_csc_matrix(crtc);
+}
+
+/** Loads the palette/gamma unit for the CRTC with the prepared values on  */
+static void i9xx_load_legacy_gamma_lut(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	enum pipe pipe = intel_crtc->pipe;
+	int i;
+	bool reenable_ips = false;
+
+	if (HAS_GMCH_DISPLAY(dev)) {
+		if (intel_crtc->config->has_dsi_encoder)
+			assert_dsi_pll_enabled(dev_priv);
+		else
+			assert_pll_enabled(dev_priv, pipe);
+	}
+
+	/* Workaround : Do not read or write the pipe palette/gamma data while
+	 * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled.
+	 */
+	if (IS_HASWELL(dev) && intel_crtc->config->ips_enabled &&
+	    ((I915_READ(GAMMA_MODE(pipe)) & GAMMA_MODE_MODE_MASK) ==
+	     GAMMA_MODE_MODE_SPLIT)) {
+		hsw_disable_ips(intel_crtc);
+		reenable_ips = true;
+	}
+
+	for (i = 0; i < 256; i++) {
+		uint32_t word = (intel_crtc->lut_r[i] << 16) |
+			(intel_crtc->lut_g[i] << 8) |
+			intel_crtc->lut_b[i];
+		if (HAS_GMCH_DISPLAY(dev))
+			I915_WRITE(PALETTE(pipe, i), word);
+		else
+			I915_WRITE(LGC_PALETTE(pipe, i), word);
+	}
+
+	I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
+
+	if (reenable_ips)
+		hsw_enable_ips(intel_crtc);
+}
+
+void intel_color_load_gamma_lut(struct drm_crtc *crtc)
+{
+	/* The clocks have to be on to load the palette. */
+	if (!crtc->state->active)
+		return;
+
+	i9xx_load_legacy_gamma_lut(crtc);
+}
+
+void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
+				  u16 *blue, uint32_t start, uint32_t size)
+{
+	int end = (start + size > 256) ? 256 : start + size, i;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	for (i = start; i < end; i++) {
+		intel_crtc->lut_r[i] = red[i] >> 8;
+		intel_crtc->lut_g[i] = green[i] >> 8;
+		intel_crtc->lut_b[i] = blue[i] >> 8;
+	}
+
+	intel_color_load_gamma_lut(crtc);
+}
+
+void intel_color_init(struct drm_crtc *crtc)
+{
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int i;
+
+	drm_mode_crtc_set_gamma_size(crtc, 256);
+	for (i = 0; i < 256; i++) {
+		intel_crtc->lut_r[i] = i;
+		intel_crtc->lut_g[i] = i;
+		intel_crtc->lut_b[i] = i;
+	}
+}
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3b6bd6e..f10bba2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -101,7 +101,6 @@ static void intel_cpu_transcoder_set_m_n(struct intel_crtc *crtc,
 					 struct intel_link_m_n *m2_n2);
 static void ironlake_set_pipeconf(struct drm_crtc *crtc);
 static void haswell_set_pipeconf(struct drm_crtc *crtc);
-static void intel_set_pipe_csc(struct drm_crtc *crtc);
 static void vlv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
 static void chv_prepare_pll(struct intel_crtc *crtc,
@@ -1165,7 +1164,7 @@ void assert_pll(struct drm_i915_private *dev_priv,
 }
 
 /* XXX: the dsi pll is shared between MIPI DSI ports */
-static void assert_dsi_pll(struct drm_i915_private *dev_priv, bool state)
+void assert_dsi_pll(struct drm_i915_private *dev_priv, bool state)
 {
 	u32 val;
 	bool cur_state;
@@ -1179,8 +1178,6 @@ static void assert_dsi_pll(struct drm_i915_private *dev_priv, bool state)
 	     "DSI PLL state assertion failure (expected %s, current %s)\n",
 			onoff(state), onoff(cur_state));
 }
-#define assert_dsi_pll_enabled(d) assert_dsi_pll(d, true)
-#define assert_dsi_pll_disabled(d) assert_dsi_pll(d, false)
 
 struct intel_shared_dpll *
 intel_crtc_to_shared_dpll(struct intel_crtc *crtc)
@@ -3308,7 +3305,7 @@ static void intel_update_pipe_config(struct intel_crtc *crtc,
 		      pipe_config->pipe_src_w, pipe_config->pipe_src_h);
 
 	if (HAS_DDI(dev))
-		intel_set_pipe_csc(&crtc->base);
+		intel_color_set_csc(&crtc->base);
 
 	/*
 	 * Update pipe size and adjust fitter if needed: the reason for this is
@@ -4619,55 +4616,6 @@ void hsw_disable_ips(struct intel_crtc *crtc)
 	intel_wait_for_vblank(dev, crtc->pipe);
 }
 
-/** Loads the palette/gamma unit for the CRTC with the prepared values */
-static void intel_crtc_load_lut(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	enum pipe pipe = intel_crtc->pipe;
-	int i;
-	bool reenable_ips = false;
-
-	/* The clocks have to be on to load the palette. */
-	if (!crtc->state->active)
-		return;
-
-	if (HAS_GMCH_DISPLAY(dev_priv->dev)) {
-		if (intel_crtc->config->has_dsi_encoder)
-			assert_dsi_pll_enabled(dev_priv);
-		else
-			assert_pll_enabled(dev_priv, pipe);
-	}
-
-	/* Workaround : Do not read or write the pipe palette/gamma data while
-	 * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled.
-	 */
-	if (IS_HASWELL(dev) && intel_crtc->config->ips_enabled &&
-	    ((I915_READ(GAMMA_MODE(pipe)) & GAMMA_MODE_MODE_MASK) ==
-	     GAMMA_MODE_MODE_SPLIT)) {
-		hsw_disable_ips(intel_crtc);
-		reenable_ips = true;
-	}
-
-	for (i = 0; i < 256; i++) {
-		i915_reg_t palreg;
-
-		if (HAS_GMCH_DISPLAY(dev))
-			palreg = PALETTE(pipe, i);
-		else
-			palreg = LGC_PALETTE(pipe, i);
-
-		I915_WRITE(palreg,
-			   (intel_crtc->lut_r[i] << 16) |
-			   (intel_crtc->lut_g[i] << 8) |
-			   intel_crtc->lut_b[i]);
-	}
-
-	if (reenable_ips)
-		hsw_enable_ips(intel_crtc);
-}
-
 static void intel_crtc_dpms_overlay_disable(struct intel_crtc *intel_crtc)
 {
 	if (intel_crtc->overlay) {
@@ -4916,7 +4864,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	 * On ILK+ LUT must be loaded before the pipe is running but with
 	 * clocks enabled
 	 */
-	intel_crtc_load_lut(crtc);
+	intel_color_load_gamma_lut(crtc);
 
 	intel_update_watermarks(crtc);
 	intel_enable_pipe(intel_crtc);
@@ -4982,7 +4930,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 
 	haswell_set_pipeconf(crtc);
 
-	intel_set_pipe_csc(crtc);
+	intel_color_set_csc(crtc);
 
 	intel_crtc->active = true;
 
@@ -5011,7 +4959,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	 * On ILK+ LUT must be loaded before the pipe is running but with
 	 * clocks enabled
 	 */
-	intel_crtc_load_lut(crtc);
+	intel_color_load_gamma_lut(crtc);
 
 	intel_ddi_set_pipe_settings(crtc);
 	if (!intel_crtc->config->has_dsi_encoder)
@@ -6226,7 +6174,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 
 	i9xx_pfit_enable(intel_crtc);
 
-	intel_crtc_load_lut(crtc);
+	intel_color_load_gamma_lut(crtc);
 
 	intel_enable_pipe(intel_crtc);
 
@@ -6279,7 +6227,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 
 	i9xx_pfit_enable(intel_crtc);
 
-	intel_crtc_load_lut(crtc);
+	intel_color_load_gamma_lut(crtc);
 
 	intel_update_watermarks(crtc);
 	intel_enable_pipe(intel_crtc);
@@ -8764,70 +8712,6 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc)
 	POSTING_READ(PIPECONF(pipe));
 }
 
-/*
- * Set up the pipe CSC unit.
- *
- * Currently only full range RGB to limited range RGB conversion
- * is supported, but eventually this should handle various
- * RGB<->YCbCr scenarios as well.
- */
-static void intel_set_pipe_csc(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int pipe = intel_crtc->pipe;
-	uint16_t coeff = 0x7800; /* 1.0 */
-
-	/*
-	 * TODO: Check what kind of values actually come out of the pipe
-	 * with these coeff/postoff values and adjust to get the best
-	 * accuracy. Perhaps we even need to take the bpc value into
-	 * consideration.
-	 */
-
-	if (intel_crtc->config->limited_color_range)
-		coeff = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */
-
-	/*
-	 * GY/GU and RY/RU should be the other way around according
-	 * to BSpec, but reality doesn't agree. Just set them up in
-	 * a way that results in the correct picture.
-	 */
-	I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeff << 16);
-	I915_WRITE(PIPE_CSC_COEFF_BY(pipe), 0);
-
-	I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeff);
-	I915_WRITE(PIPE_CSC_COEFF_BU(pipe), 0);
-
-	I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), 0);
-	I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeff << 16);
-
-	I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0);
-	I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0);
-	I915_WRITE(PIPE_CSC_PREOFF_LO(pipe), 0);
-
-	if (INTEL_INFO(dev)->gen > 6) {
-		uint16_t postoff = 0;
-
-		if (intel_crtc->config->limited_color_range)
-			postoff = (16 * (1 << 12) / 255) & 0x1fff;
-
-		I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff);
-		I915_WRITE(PIPE_CSC_POSTOFF_ME(pipe), postoff);
-		I915_WRITE(PIPE_CSC_POSTOFF_LO(pipe), postoff);
-
-		I915_WRITE(PIPE_CSC_MODE(pipe), 0);
-	} else {
-		uint32_t mode = CSC_MODE_YUV_TO_RGB;
-
-		if (intel_crtc->config->limited_color_range)
-			mode |= CSC_BLACK_SCREEN_OFFSET;
-
-		I915_WRITE(PIPE_CSC_MODE(pipe), mode);
-	}
-}
-
 static void haswell_set_pipeconf(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -8850,9 +8734,6 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc)
 	I915_WRITE(PIPECONF(cpu_transcoder), val);
 	POSTING_READ(PIPECONF(cpu_transcoder));
 
-	I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
-	POSTING_READ(GAMMA_MODE(intel_crtc->pipe));
-
 	if (IS_BROADWELL(dev) || INTEL_INFO(dev)->gen >= 9) {
 		val = 0;
 
@@ -10243,21 +10124,6 @@ static bool cursor_size_ok(struct drm_device *dev,
 	return true;
 }
 
-static void intel_crtc_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
-				 u16 *blue, uint32_t start, uint32_t size)
-{
-	int end = (start + size > 256) ? 256 : start + size, i;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-	for (i = start; i < end; i++) {
-		intel_crtc->lut_r[i] = red[i] >> 8;
-		intel_crtc->lut_g[i] = green[i] >> 8;
-		intel_crtc->lut_b[i] = blue[i] >> 8;
-	}
-
-	intel_crtc_load_lut(crtc);
-}
-
 /* VESA 640x480x72Hz mode to set on the pipe */
 static struct drm_display_mode load_detect_mode = {
 	DRM_MODE("640x480", DRM_MODE_TYPE_DEFAULT, 31500, 640, 664,
@@ -12032,7 +11898,7 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
 
 static const struct drm_crtc_helper_funcs intel_helper_funcs = {
 	.mode_set_base_atomic = intel_pipe_set_base_atomic,
-	.load_lut = intel_crtc_load_lut,
+	.load_lut = intel_color_load_gamma_lut,
 	.atomic_begin = intel_begin_crtc_commit,
 	.atomic_flush = intel_finish_crtc_commit,
 	.atomic_check = intel_crtc_atomic_check,
@@ -13651,7 +13517,7 @@ out:
 #undef for_each_intel_crtc_masked
 
 static const struct drm_crtc_funcs intel_crtc_funcs = {
-	.gamma_set = intel_crtc_gamma_set,
+	.gamma_set = intel_color_legacy_gamma_set,
 	.set_config = drm_atomic_helper_set_config,
 	.destroy = intel_crtc_destroy,
 	.page_flip = intel_crtc_page_flip,
@@ -14256,7 +14122,7 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	struct intel_crtc_state *crtc_state = NULL;
 	struct drm_plane *primary = NULL;
 	struct drm_plane *cursor = NULL;
-	int i, ret;
+	int ret;
 
 	intel_crtc = kzalloc(sizeof(*intel_crtc), GFP_KERNEL);
 	if (intel_crtc == NULL)
@@ -14292,13 +14158,6 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	if (ret)
 		goto fail;
 
-	drm_mode_crtc_set_gamma_size(&intel_crtc->base, 256);
-	for (i = 0; i < 256; i++) {
-		intel_crtc->lut_r[i] = i;
-		intel_crtc->lut_g[i] = i;
-		intel_crtc->lut_b[i] = i;
-	}
-
 	/*
 	 * On gen2/3 only plane A can do fbc, but the panel fitter and lvds port
 	 * is hooked to pipe B. Hence we want plane A feeding pipe B.
@@ -14323,6 +14182,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 
 	drm_crtc_helper_add(&intel_crtc->base, &intel_helper_funcs);
 
+	intel_color_init(&intel_crtc->base);
+
 	WARN_ON(drm_crtc_index(&intel_crtc->base) != intel_crtc->pipe);
 	return;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 878172a..2b5d03a 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1166,6 +1166,9 @@ void assert_pll(struct drm_i915_private *dev_priv,
 		enum pipe pipe, bool state);
 #define assert_pll_enabled(d, p) assert_pll(d, p, true)
 #define assert_pll_disabled(d, p) assert_pll(d, p, false)
+void assert_dsi_pll(struct drm_i915_private *dev_priv, bool state);
+#define assert_dsi_pll_enabled(d) assert_dsi_pll(d, true)
+#define assert_dsi_pll_disabled(d) assert_dsi_pll(d, false)
 void assert_fdi_rx_pll(struct drm_i915_private *dev_priv,
 		       enum pipe pipe, bool state);
 #define assert_fdi_rx_pll_enabled(d, p) assert_fdi_rx_pll(d, p, true)
@@ -1615,4 +1618,11 @@ void intel_plane_destroy_state(struct drm_plane *plane,
 			       struct drm_plane_state *state);
 extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
 
+/* intel_color.c */
+void intel_color_init(struct drm_crtc *crtc);
+void intel_color_set_csc(struct drm_crtc *crtc);
+void intel_color_load_gamma_lut(struct drm_crtc *crtc);
+void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
+				  u16 *blue, uint32_t start, uint32_t size);
+
 #endif /* __INTEL_DRV_H__ */
-- 
2.7.0

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

[PATCH 2/6] drm: introduce color correction properties

[Lionel Landwerlin]

Patch based on a previous series by Shashank Sharma.

v2: Register LUT size properties as range

v3: Fix round in drm_color_lut_get_value() helper
    More docs on how degamma/gamma properties are used

v4: Update contributors

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
---
 Documentation/DocBook/gpu.tmpl      |  58 +++++++++++++++++++-
 drivers/gpu/drm/drm_atomic.c        | 102 +++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/drm_atomic_helper.c |  10 ++++
 drivers/gpu/drm/drm_crtc.c          |  35 +++++++++++++
 drivers/gpu/drm/drm_crtc_helper.c   |  33 ++++++++++++
 include/drm/drm_crtc.h              |  44 +++++++++++++++-
 include/drm/drm_crtc_helper.h       |   3 ++
 include/uapi/drm/drm_mode.h         |  15 ++++++
 8 files changed, 295 insertions(+), 5 deletions(-)

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index fe6b36a..7c49a92 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -1816,7 +1816,7 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >Description/Restrictions</td>
 	</tr>
 	<tr>
-	<td rowspan="37" valign="top" >DRM</td>
+	<td rowspan="42" valign="top" >DRM</td>
 	<td valign="top" >Generic</td>
 	<td valign="top" >“rotation”</td>
 	<td valign="top" >BITMASK</td>
@@ -2068,7 +2068,7 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >property to suggest an Y offset for a connector</td>
 	</tr>
 	<tr>
-	<td rowspan="3" valign="top" >Optional</td>
+	<td rowspan="8" valign="top" >Optional</td>
 	<td valign="top" >“scaling mode”</td>
 	<td valign="top" >ENUM</td>
 	<td valign="top" >{ "None", "Full", "Center", "Full aspect" }</td>
@@ -2092,6 +2092,60 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
+	<td valign="top" >“DEGAMMA_LUT”</td>
+	<td valign="top" >BLOB</td>
+	<td valign="top" >0</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to set the degamma LUT mapping
+		pixel data from the framebuffer before it is given to the
+		transformation matrix. The data is an interpreted as an array
+		of struct drm_color_lut elements. Hardware might choose not to
+		use the full precision of the LUT elements nor use all the
+		elements of the LUT (for example the hardware might choose to
+		interpolate between LUT[0] and LUT[4]). </td>
+	</tr>
+	<tr>
+	<td valign="top" >“DEGAMMA_LUT_SIZE”</td>
+	<td valign="top" >RANGE | IMMUTABLE</td>
+	<td valign="top" >Min=0, Max=UINT_MAX</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to gives the size of the LUT to
+		be set on the DEGAMMA_LUT property (the size depends on the
+		underlying hardware).</td>
+	</tr>
+	<tr>
+	<td valign="top" >“CTM_MATRIX”</td>
+	<td valign="top" >BLOB</td>
+	<td valign="top" >0</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to set the transformation
+		matrix apply to pixel data after the lookup through the
+		degamma LUT and before the lookup through the gamma LUT. The
+		data is an interpreted as a struct drm_color_ctm.</td>
+	</tr>
+	<tr>
+	<td valign="top" >“GAMMA_LUT”</td>
+	<td valign="top" >BLOB</td>
+	<td valign="top" >0</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to set the gamma LUT mapping
+		pixel data after to the transformation matrix to data sent to
+		the connector. The data is an interpreted as an array
+		of struct drm_color_lut elements. Hardware might choose not to
+		use the full precision of the LUT elements nor use all the
+		elements of the LUT (for example the hardware might choose to
+		interpolate between LUT[0] and LUT[4]).</td>
+	</tr>
+	<tr>
+	<td valign="top" >“GAMMA_LUT_SIZE”</td>
+	<td valign="top" >RANGE | IMMUTABLE</td>
+	<td valign="top" >Min=0, Max=UINT_MAX</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to gives the size of the LUT to
+		be set on the GAMMA_LUT property (the size depends on the
+		underlying hardware).</td>
+	</tr>
+	<tr>
 	<td rowspan="20" valign="top" >i915</td>
 	<td rowspan="2" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 8fb469c..7507762 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -28,6 +28,7 @@
 
 #include <drm/drmP.h>
 #include <drm/drm_atomic.h>
+#include <drm/drm_mode.h>
 #include <drm/drm_plane_helper.h>
 
 /**
@@ -388,6 +389,58 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
 EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
 
 /**
+ * drm_atomic_replace_property_blob - replace a blob property
+ * @blob: a pointer to the member blob to be replaced
+ * @new_blob: the new blob to replace with
+ * @expected_size: the expected size of the new blob
+ * @replaced: whether the blob has been replaced
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+static int
+drm_atomic_replace_property_blob(struct drm_property_blob **blob,
+				 struct drm_property_blob *new_blob,
+				 size_t expected_size,
+				 bool *replaced)
+{
+	struct drm_property_blob *old_blob = *blob;
+
+	if (old_blob == new_blob)
+		return 0;
+
+	if (new_blob != NULL && new_blob->length != expected_size)
+		return -EINVAL;
+
+	if (old_blob != NULL)
+		drm_property_unreference_blob(old_blob);
+	*blob = new_blob;
+	*replaced = true;
+
+	return 0;
+}
+
+static int
+drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc,
+					 struct drm_property_blob **blob,
+					 uint64_t blob_id,
+					 size_t expected_size,
+					 bool *replaced)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_property_blob *new_blob = NULL;
+
+	if (blob_id != 0) {
+		new_blob = drm_property_lookup_blob(dev, blob_id);
+		if (new_blob == NULL)
+			return -EINVAL;
+	}
+
+	return drm_atomic_replace_property_blob(blob, new_blob,
+						expected_size, replaced);
+}
+
+/**
  * drm_atomic_crtc_set_property - set property on CRTC
  * @crtc: the drm CRTC to set a property on
  * @state: the state object to update with the new property value
@@ -419,8 +472,47 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
 		drm_property_unreference_blob(mode);
 		return ret;
-	}
-	else if (crtc->funcs->atomic_set_property)
+	} else if (property == config->degamma_lut_property) {
+		bool replaced = false;
+		uint64_t lut_size = 0;
+
+		ret = drm_object_property_get_value(&crtc->base,
+					config->degamma_lut_size_property,
+					&lut_size);
+		if (ret == 0)
+			ret = drm_atomic_replace_property_blob_from_id(crtc,
+					&state->degamma_lut,
+					val,
+					lut_size * sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed = replaced;
+		return ret;
+	} else if (property == config->ctm_matrix_property) {
+		bool replaced = false;
+
+		ret = drm_atomic_replace_property_blob_from_id(crtc,
+					&state->ctm_matrix,
+					val,
+					sizeof(struct drm_color_ctm),
+							       &replaced);
+		state->color_mgmt_changed = replaced;
+		return ret;
+	} else if (property == config->gamma_lut_property) {
+		bool replaced = false;
+		uint64_t lut_size = 0;
+
+		ret = drm_object_property_get_value(&crtc->base,
+					config->gamma_lut_size_property,
+					&lut_size);
+		if (ret == 0)
+			ret = drm_atomic_replace_property_blob_from_id(crtc,
+					&state->gamma_lut,
+					val,
+					lut_size * sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed = replaced;
+		return ret;
+	} else if (crtc->funcs->atomic_set_property)
 		return crtc->funcs->atomic_set_property(crtc, state, property, val);
 	else
 		return -EINVAL;
@@ -456,6 +548,12 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
 		*val = state->active;
 	else if (property == config->prop_mode_id)
 		*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
+	else if (property == config->degamma_lut_property)
+		*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
+	else if (property == config->ctm_matrix_property)
+		*val = (state->ctm_matrix) ? state->ctm_matrix->base.id : 0;
+	else if (property == config->gamma_lut_property)
+		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
 	else if (crtc->funcs->atomic_get_property)
 		return crtc->funcs->atomic_get_property(crtc, state, property, val);
 	else
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 2b430b0..d13dfd8 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2513,10 +2513,17 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
 
 	if (state->mode_blob)
 		drm_property_reference_blob(state->mode_blob);
+	if (state->degamma_lut)
+		drm_property_reference_blob(state->degamma_lut);
+	if (state->ctm_matrix)
+		drm_property_reference_blob(state->ctm_matrix);
+	if (state->gamma_lut)
+		drm_property_reference_blob(state->gamma_lut);
 	state->mode_changed = false;
 	state->active_changed = false;
 	state->planes_changed = false;
 	state->connectors_changed = false;
+	state->color_mgmt_changed = false;
 	state->event = NULL;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
@@ -2557,6 +2564,9 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
 					    struct drm_crtc_state *state)
 {
 	drm_property_unreference_blob(state->mode_blob);
+	drm_property_unreference_blob(state->degamma_lut);
+	drm_property_unreference_blob(state->ctm_matrix);
+	drm_property_unreference_blob(state->gamma_lut);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
 
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index b19c8f4..8e8f57a 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1565,6 +1565,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.prop_mode_id = prop;
 
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"DEGAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.degamma_lut_property = prop;
+
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"DEGAMMA_LUT_SIZE", 0, UINT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.degamma_lut_size_property = prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"CTM_MATRIX", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.ctm_matrix_property = prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"GAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.gamma_lut_property = prop;
+
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"GAMMA_LUT_SIZE", 0, UINT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.gamma_lut_size_property = prop;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 9f8b894..3cd8cbb 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -1073,3 +1073,36 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	return drm_plane_helper_commit(plane, plane_state, old_fb);
 }
 EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
+
+/**
+ * drm_helper_crtc_enable_color_mgmt - enable color management properties
+ * @crtc: DRM CRTC
+ * @degamma_lut_size: the size of the degamma lut (before CSC)
+ * @gamma_lut_size: the size of the gamma lut (after CSC)
+ *
+ * This function lets the driver enable the color correction properties on a
+ * CRTC. This includes 3 degamma, csc and gamma properties that userspace can
+ * set and 2 size properties to inform the userspace of the lut sizes.
+ */
+void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
+				       int degamma_lut_size,
+				       int gamma_lut_size)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+
+	drm_object_attach_property(&crtc->base,
+				   config->degamma_lut_property, 0);
+	drm_object_attach_property(&crtc->base,
+				   config->ctm_matrix_property, 0);
+	drm_object_attach_property(&crtc->base,
+				   config->gamma_lut_property, 0);
+
+	drm_object_attach_property(&crtc->base,
+				   config->degamma_lut_size_property,
+				   degamma_lut_size);
+	drm_object_attach_property(&crtc->base,
+				   config->gamma_lut_size_property,
+				   gamma_lut_size);
+}
+EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8c7fb3d..e07b174 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -305,6 +305,8 @@ struct drm_plane_helper_funcs;
  * @mode_changed: crtc_state->mode or crtc_state->enable has been changed
  * @active_changed: crtc_state->active has been toggled.
  * @connectors_changed: connectors to this crtc have been updated
+ * @color_mgmt_changed: color management properties have changed (degamma or
+ *	gamma LUT or CSC matrix)
  * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
  * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors
  * @encoder_mask: bitmask of (1 << drm_encoder_index(encoder)) of attached encoders
@@ -312,6 +314,11 @@ struct drm_plane_helper_funcs;
  * 	update to ensure framebuffer cleanup isn't done too early
  * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
  * @mode: current mode timings
+ * @degamma_lut: Lookup table for converting framebuffer pixel data
+ *	before apply the conversion matrix
+ * @ctm_matrix: Transformation matrix
+ * @gamma_lut: Lookup table for converting pixel data after the
+ *	conversion matrix
  * @event: optional pointer to a DRM event to signal upon completion of the
  * 	state update
  * @state: backpointer to global drm_atomic_state
@@ -333,6 +340,7 @@ struct drm_crtc_state {
 	bool mode_changed : 1;
 	bool active_changed : 1;
 	bool connectors_changed : 1;
+	bool color_mgmt_changed : 1;
 
 	/* attached planes bitmask:
 	 * WARNING: transitional helpers do not maintain plane_mask so
@@ -355,6 +363,11 @@ struct drm_crtc_state {
 	/* blob property to expose current mode to atomic userspace */
 	struct drm_property_blob *mode_blob;
 
+	/* blob property to expose color management to userspace */
+	struct drm_property_blob *degamma_lut;
+	struct drm_property_blob *ctm_matrix;
+	struct drm_property_blob *gamma_lut;
+
 	struct drm_pending_vblank_event *event;
 
 	struct drm_atomic_state *state;
@@ -757,7 +770,7 @@ struct drm_crtc {
 	int x, y;
 	const struct drm_crtc_funcs *funcs;
 
-	/* CRTC gamma size for reporting to userspace */
+	/* Legacy FB CRTC gamma size for reporting to userspace */
 	uint32_t gamma_size;
 	uint16_t *gamma_store;
 
@@ -2025,6 +2038,15 @@ struct drm_mode_config_funcs {
  * @property_blob_list: list of all the blob property objects
  * @blob_lock: mutex for blob property allocation and management
  * @*_property: core property tracking
+ * @degamma_lut_property: LUT used to convert the framebuffer's colors to linear
+ *	gamma
+ * @degamma_lut_size_property: size of the degamma LUT as supported by the
+ *	driver (read-only)
+ * @ctm_matrix_property: Matrix used to convert colors after the lookup in the
+ *	degamma LUT
+ * @gamma_lut_property: LUT used to convert the colors, after the CSC matrix, to
+ *	the gamma space of the connected screen (read-only)
+ * @gamma_lut_size_property: size of the gamma LUT as supported by the driver
  * @preferred_depth: preferred RBG pixel depth, used by fb helpers
  * @prefer_shadow: hint to userspace to prefer shadow-fb rendering
  * @async_page_flip: does this device support async flips on the primary plane?
@@ -2126,6 +2148,13 @@ struct drm_mode_config {
 	struct drm_property *aspect_ratio_property;
 	struct drm_property *dirty_info_property;
 
+	/* Optional color correction properties */
+	struct drm_property *degamma_lut_property;
+	struct drm_property *degamma_lut_size_property;
+	struct drm_property *ctm_matrix_property;
+	struct drm_property *gamma_lut_property;
+	struct drm_property *gamma_lut_size_property;
+
 	/* properties for virtual machine layout */
 	struct drm_property *suggested_x_property;
 	struct drm_property *suggested_y_property;
@@ -2548,6 +2577,19 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
 	return mo ? obj_to_property(mo) : NULL;
 }
 
+/*
+ * Extract a degamma/gamma LUT value provided by user and round it to the
+ * precision supported by the hardware.
+ */
+static inline uint32_t drm_color_lut_extract(uint32_t user_input,
+					     uint32_t bit_precision)
+{
+	uint32_t val = ((user_input << 1) + 1) >> (16 - bit_precision + 1);
+	uint32_t max = 0xffff >> (16 - bit_precision);
+
+	return clamp_val(val, 0, max);
+}
+
 /* Plane list iterator for legacy (overlay only) planes. */
 #define drm_for_each_legacy_plane(plane, dev) \
 	list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 4b37afa..97fa894 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -48,6 +48,9 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
 				     struct drm_display_mode *mode,
 				     int x, int y,
 				     struct drm_framebuffer *old_fb);
+extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
+					      int degamma_lut_size,
+					      int gamma_lut_size);
 extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc);
 extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
 
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 50adb46..c021743 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -487,6 +487,21 @@ struct drm_mode_crtc_lut {
 	__u64 blue;
 };
 
+struct drm_color_ctm {
+	/* Conversion matrix in S31.32 format. */
+	__s64 matrix[9];
+};
+
+struct drm_color_lut {
+	/*
+	 * Data is U0.16 fixed point format.
+	 */
+	__u16 red;
+	__u16 green;
+	__u16 blue;
+	__u16 reserved;
+};
+
 #define DRM_MODE_PAGE_FLIP_EVENT 0x01
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT|DRM_MODE_PAGE_FLIP_ASYNC)
-- 
2.7.0

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

[PATCH 3/6] drm/i915: enable CSC for pipe C

[Lionel Landwerlin]

Patch based on a previous series by Shashank Sharma.

v2: Update contributors

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 40 +++++++++++++++++++++++++++-------------
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 188ad5d..7ba8a99 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7634,19 +7634,33 @@ enum skl_disp_power_wells {
 #define _PIPE_B_CSC_POSTOFF_ME	0x49144
 #define _PIPE_B_CSC_POSTOFF_LO	0x49148
 
-#define PIPE_CSC_COEFF_RY_GY(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_RY_GY, _PIPE_B_CSC_COEFF_RY_GY)
-#define PIPE_CSC_COEFF_BY(pipe)		_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_BY, _PIPE_B_CSC_COEFF_BY)
-#define PIPE_CSC_COEFF_RU_GU(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_RU_GU, _PIPE_B_CSC_COEFF_RU_GU)
-#define PIPE_CSC_COEFF_BU(pipe)		_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_BU, _PIPE_B_CSC_COEFF_BU)
-#define PIPE_CSC_COEFF_RV_GV(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_RV_GV, _PIPE_B_CSC_COEFF_RV_GV)
-#define PIPE_CSC_COEFF_BV(pipe)		_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_BV, _PIPE_B_CSC_COEFF_BV)
-#define PIPE_CSC_MODE(pipe)		_MMIO_PIPE(pipe, _PIPE_A_CSC_MODE, _PIPE_B_CSC_MODE)
-#define PIPE_CSC_PREOFF_HI(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_PREOFF_HI, _PIPE_B_CSC_PREOFF_HI)
-#define PIPE_CSC_PREOFF_ME(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_PREOFF_ME, _PIPE_B_CSC_PREOFF_ME)
-#define PIPE_CSC_PREOFF_LO(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_PREOFF_LO, _PIPE_B_CSC_PREOFF_LO)
-#define PIPE_CSC_POSTOFF_HI(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_HI, _PIPE_B_CSC_POSTOFF_HI)
-#define PIPE_CSC_POSTOFF_ME(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME)
-#define PIPE_CSC_POSTOFF_LO(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO)
+#define _PIPE_C_CSC_COEFF_RY_GY	0x49210
+#define _PIPE_C_CSC_COEFF_BY	0x49214
+#define _PIPE_C_CSC_COEFF_RU_GU	0x49218
+#define _PIPE_C_CSC_COEFF_BU	0x4921c
+#define _PIPE_C_CSC_COEFF_RV_GV	0x49220
+#define _PIPE_C_CSC_COEFF_BV	0x49224
+#define _PIPE_C_CSC_MODE	0x49228
+#define _PIPE_C_CSC_PREOFF_HI	0x49230
+#define _PIPE_C_CSC_PREOFF_ME	0x49234
+#define _PIPE_C_CSC_PREOFF_LO	0x49238
+#define _PIPE_C_CSC_POSTOFF_HI	0x49240
+#define _PIPE_C_CSC_POSTOFF_ME	0x49244
+#define _PIPE_C_CSC_POSTOFF_LO	0x49248
+
+#define PIPE_CSC_COEFF_RY_GY(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_RY_GY, _PIPE_B_CSC_COEFF_RY_GY, _PIPE_C_CSC_COEFF_RY_GY)
+#define PIPE_CSC_COEFF_BY(pipe)		_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_BY, _PIPE_B_CSC_COEFF_BY, _PIPE_C_CSC_COEFF_BY)
+#define PIPE_CSC_COEFF_RU_GU(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_RU_GU, _PIPE_B_CSC_COEFF_RU_GU, _PIPE_C_CSC_COEFF_RU_GU)
+#define PIPE_CSC_COEFF_BU(pipe)		_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_BU, _PIPE_B_CSC_COEFF_BU, _PIPE_C_CSC_COEFF_BU)
+#define PIPE_CSC_COEFF_RV_GV(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_RV_GV, _PIPE_B_CSC_COEFF_RV_GV, _PIPE_C_CSC_COEFF_RV_GV)
+#define PIPE_CSC_COEFF_BV(pipe)		_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_BV, _PIPE_B_CSC_COEFF_BV, _PIPE_C_CSC_COEFF_BV)
+#define PIPE_CSC_MODE(pipe)		_MMIO_PIPE3(pipe, _PIPE_A_CSC_MODE, _PIPE_B_CSC_MODE, _PIPE_C_CSC_MODE)
+#define PIPE_CSC_PREOFF_HI(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_PREOFF_HI, _PIPE_B_CSC_PREOFF_HI, _PIPE_C_CSC_PREOFF_HI)
+#define PIPE_CSC_PREOFF_ME(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_PREOFF_ME, _PIPE_B_CSC_PREOFF_ME, _PIPE_C_CSC_PREOFF_ME)
+#define PIPE_CSC_PREOFF_LO(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_PREOFF_LO, _PIPE_B_CSC_PREOFF_LO, _PIPE_C_CSC_PREOFF_LO)
+#define PIPE_CSC_POSTOFF_HI(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_HI, _PIPE_B_CSC_POSTOFF_HI, _PIPE_C_CSC_POSTOFF_HI)
+#define PIPE_CSC_POSTOFF_ME(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME, _PIPE_C_CSC_POSTOFF_ME)
+#define PIPE_CSC_POSTOFF_LO(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO, _PIPE_C_CSC_POSTOFF_LO)
 
 /* MIPI DSI registers */
 
-- 
2.7.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/6] drm/i915: enable legacy palette for pipe C

[Lionel Landwerlin]

Patch based on a previous series by Shashank Sharma.

v2: Update contributors

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7ba8a99..f8b0d6c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -5757,7 +5757,8 @@ enum skl_disp_power_wells {
 /* legacy palette */
 #define _LGC_PALETTE_A           0x4a000
 #define _LGC_PALETTE_B           0x4a800
-#define LGC_PALETTE(pipe, i) _MMIO(_PIPE(pipe, _LGC_PALETTE_A, _LGC_PALETTE_B) + (i) * 4)
+#define _LGC_PALETTE_C           0x4b000
+#define LGC_PALETTE(pipe, i) _MMIO(_PIPE3(pipe, _LGC_PALETTE_A, _LGC_PALETTE_B, _LGC_PALETTE_C) + (i) * 4)
 
 #define _GAMMA_MODE_A		0x4a480
 #define _GAMMA_MODE_B		0x4ac80
-- 
2.7.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/6] drm/i915: Implement color management on bdw/skl/bxt/kbl

[Lionel Landwerlin]

Patch based on a previous series by Shashank Sharma.

v2: Do not read GAMMA_MODE register to figure what mode we're in

v3: Program PREC_PAL_GC_MAX to clamp pixel values > 1.0

    Add documentation on how the Broadcast RGB property is affected by
    CTM_MATRIX

v4: Update contributors

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
---
 Documentation/DocBook/gpu.tmpl       |   6 +-
 drivers/gpu/drm/i915/i915_drv.c      |  24 ++-
 drivers/gpu/drm/i915/i915_drv.h      |   9 +
 drivers/gpu/drm/i915/i915_reg.h      |  22 +++
 drivers/gpu/drm/i915/intel_color.c   | 367 ++++++++++++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_display.c |  22 ++-
 drivers/gpu/drm/i915/intel_drv.h     |   6 +-
 7 files changed, 396 insertions(+), 60 deletions(-)

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 7c49a92..78b8877 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -2152,7 +2152,11 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >ENUM</td>
 	<td valign="top" >{ "Automatic", "Full", "Limited 16:235" }</td>
 	<td valign="top" >Connector</td>
-	<td valign="top" >TBD</td>
+	<td valign="top" >When this property is set to Limited 16:235
+		and CTM_MATRIX is set, the hardware will be programmed with
+		the result of the multiplication of CTM_MATRIX by the limited
+		range matrix to ensure the pixels normaly in the range 0..1.0
+		are remapped to the range 16/255..235/255.</td>
 	</tr>
 	<tr>
 	<td valign="top" >“audio”</td>
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 44912ec..b65aa20 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -66,6 +66,9 @@ static struct drm_driver driver;
 #define IVB_CURSOR_OFFSETS \
 	.cursor_offsets = { CURSOR_A_OFFSET, IVB_CURSOR_B_OFFSET, IVB_CURSOR_C_OFFSET }
 
+#define BDW_COLORS \
+	.color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }
+
 static const struct intel_device_info intel_i830_info = {
 	.gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2,
 	.has_overlay = 1, .overlay_needs_physical = 1,
@@ -288,24 +291,28 @@ static const struct intel_device_info intel_haswell_m_info = {
 	.is_mobile = 1,
 };
 
+#define BDW_FEATURES \
+	HSW_FEATURES, \
+	BDW_COLORS
+
 static const struct intel_device_info intel_broadwell_d_info = {
-	HSW_FEATURES,
+	BDW_FEATURES,
 	.gen = 8,
 };
 
 static const struct intel_device_info intel_broadwell_m_info = {
-	HSW_FEATURES,
+	BDW_FEATURES,
 	.gen = 8, .is_mobile = 1,
 };
 
 static const struct intel_device_info intel_broadwell_gt3d_info = {
-	HSW_FEATURES,
+	BDW_FEATURES,
 	.gen = 8,
 	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
 };
 
 static const struct intel_device_info intel_broadwell_gt3m_info = {
-	HSW_FEATURES,
+	BDW_FEATURES,
 	.gen = 8, .is_mobile = 1,
 	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
 };
@@ -321,13 +328,13 @@ static const struct intel_device_info intel_cherryview_info = {
 };
 
 static const struct intel_device_info intel_skylake_info = {
-	HSW_FEATURES,
+	BDW_FEATURES,
 	.is_skylake = 1,
 	.gen = 9,
 };
 
 static const struct intel_device_info intel_skylake_gt3_info = {
-	HSW_FEATURES,
+	BDW_FEATURES,
 	.is_skylake = 1,
 	.gen = 9,
 	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
@@ -345,17 +352,18 @@ static const struct intel_device_info intel_broxton_info = {
 	.has_fbc = 1,
 	GEN_DEFAULT_PIPEOFFSETS,
 	IVB_CURSOR_OFFSETS,
+	BDW_COLORS,
 };
 
 static const struct intel_device_info intel_kabylake_info = {
-	HSW_FEATURES,
+	BDW_FEATURES,
 	.is_preliminary = 1,
 	.is_kabylake = 1,
 	.gen = 9,
 };
 
 static const struct intel_device_info intel_kabylake_gt3_info = {
-	HSW_FEATURES,
+	BDW_FEATURES,
 	.is_preliminary = 1,
 	.is_kabylake = 1,
 	.gen = 9,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8216665..c1ca4d0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -659,6 +659,10 @@ struct drm_i915_display_funcs {
 	/* render clock increase/decrease */
 	/* display clock increase/decrease */
 	/* pll clock increase/decrease */
+
+	void (*load_degamma_lut)(struct drm_crtc *crtc);
+	void (*load_csc_matrix)(struct drm_crtc *crtc);
+	void (*load_gamma_lut)(struct drm_crtc *crtc);
 };
 
 enum forcewake_domain_id {
@@ -806,6 +810,11 @@ struct intel_device_info {
 	u8 has_slice_pg:1;
 	u8 has_subslice_pg:1;
 	u8 has_eu_pg:1;
+
+	struct color_luts {
+		u16 degamma_lut_size;
+		u16 gamma_lut_size;
+	} color;
 };
 
 #undef DEFINE_FLAG
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f8b0d6c..1dc5d3b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7663,6 +7663,28 @@ enum skl_disp_power_wells {
 #define PIPE_CSC_POSTOFF_ME(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME, _PIPE_C_CSC_POSTOFF_ME)
 #define PIPE_CSC_POSTOFF_LO(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO, _PIPE_C_CSC_POSTOFF_LO)
 
+/* pipe degamma/gamma LUTs on IVB+ */
+#define _PAL_PREC_INDEX_A	0x4A400
+#define _PAL_PREC_INDEX_B	0x4AC00
+#define _PAL_PREC_INDEX_C	0x4B400
+#define   PAL_PREC_10_12_BIT		(0 << 31)
+#define   PAL_PREC_SPLIT_MODE		(1 << 31)
+#define   PAL_PREC_AUTO_INCREMENT	(1 << 15)
+#define _PAL_PREC_DATA_A	0x4A404
+#define _PAL_PREC_DATA_B	0x4AC04
+#define _PAL_PREC_DATA_C	0x4B404
+#define _PAL_PREC_GC_MAX_A	0x4A410
+#define _PAL_PREC_GC_MAX_B	0x4AC10
+#define _PAL_PREC_GC_MAX_C	0x4B410
+#define _PAL_PREC_EXT_GC_MAX_A	0x4A420
+#define _PAL_PREC_EXT_GC_MAX_B	0x4AC20
+#define _PAL_PREC_EXT_GC_MAX_C	0x4B420
+
+#define PREC_PAL_INDEX(pipe)		_MMIO_PIPE3(pipe, _PAL_PREC_INDEX_A, _PAL_PREC_INDEX_B, _PAL_PREC_INDEX_C)
+#define PREC_PAL_DATA(pipe)		_MMIO_PIPE3(pipe, _PAL_PREC_DATA_A, _PAL_PREC_DATA_B, _PAL_PREC_DATA_C)
+#define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE3(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B, _PAL_PREC_GC_MAX_C) + (i) * 4)
+#define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE3(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B, _PAL_PREC_EXT_GC_MAX_C) + (i) * 4)
+
 /* MIPI DSI registers */
 
 #define _MIPI_PORT(port, a, c)	_PORT3(port, a, 0, c)	/* ports A and C only */
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 39ca215..782b9d8 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -24,44 +24,169 @@
 
 #include "intel_drv.h"
 
+#define CTM_COEFF_SIGN	(1ULL << 63)
+
+#define CTM_COEFF_1_0	(1ULL << 32)
+#define CTM_COEFF_2_0	(CTM_COEFF_1_0 << 1)
+#define CTM_COEFF_4_0	(CTM_COEFF_2_0 << 1)
+#define CTM_COEFF_0_5	(CTM_COEFF_1_0 >> 1)
+#define CTM_COEFF_0_25	(CTM_COEFF_0_5 >> 1)
+#define CTM_COEFF_0_125	(CTM_COEFF_0_25 >> 1)
+
+#define CTM_COEFF_LIMITED_RANGE ((235ULL - 16ULL) * CTM_COEFF_1_0 / 255)
+
+#define CTM_COEFF_NEGATIVE(coeff)	(((coeff) & CTM_COEFF_SIGN) != 0)
+#define CTM_COEFF_ABS(coeff)		((coeff) & (CTM_COEFF_SIGN - 1))
+
+/*
+ * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
+ * format). This macro takes the coefficient we want transformed and the
+ * number of fractional bits.
+ *
+ * We only have a 9 bits precision window which slides depending on the value
+ * of the CTM coefficient and we write the value from bit 3. We also round the
+ * value.
+ */
+#define I9XX_CSC_COEFF_FP(coeff, fbits)	\
+	(clamp_val(((coeff) >> (32 - (fbits) - 3)) + 4, 0, 0xfff) & 0xff8)
+
+#define I9XX_CSC_COEFF_LIMITED_RANGE	\
+	I9XX_CSC_COEFF_FP(CTM_COEFF_LIMITED_RANGE, 9)
+#define I9XX_CSC_COEFF_1_0		\
+	((7 << 12) | I9XX_CSC_COEFF_FP(CTM_COEFF_1_0, 8))
+
+/*
+ * When using limited range, multiply the matrix given by userspace by
+ * the matrix that we would use for the limited range. We do the
+ * multiplication in U2.30 format.
+ */
+static void ctm_matrix_mult_by_limited(uint64_t *result,
+				       int64_t *input)
+{
+	int i, j;
+	uint64_t limited_coeffs[9] = { CTM_COEFF_LIMITED_RANGE, 0, 0,
+				       0, CTM_COEFF_LIMITED_RANGE, 0,
+				       0, 0, CTM_COEFF_LIMITED_RANGE };
+
+	for (i = 0; i < ARRAY_SIZE(limited_coeffs); i++) {
+		int column = i % 3, row = i / 3;
+		int negative = 0;
+
+		input[i] = 0;
+		for (j = 0; j < 3; j++) {
+			int64_t user_coeff = input[j * 3 + column];
+			uint64_t limited_coeff =
+				limited_coeffs[row * 3 + j] >> 2;
+			uint64_t abs_coeff =
+				clamp_val(CTM_COEFF_ABS(user_coeff),
+					  0,
+					  CTM_COEFF_4_0 - 1) >> 2;
+
+			if (CTM_COEFF_NEGATIVE(user_coeff))
+				negative = !negative;
+			result[i] += limited_coeff * abs_coeff;
+		}
+
+		result[i] >>= 27;
+		if (negative)
+			result[i] |= CTM_COEFF_SIGN;
+	}
+}
+
 /*
  * Set up the pipe CSC unit.
  *
- * Currently only full range RGB to limited range RGB conversion
- * is supported, but eventually this should handle various
- * RGB<->YCbCr scenarios as well.
+ * Currently only full range RGB to limited range RGB conversion is supported,
+ * but eventually this should handle various RGB<->YCbCr scenarios as well.
  */
 static void i9xx_load_csc_matrix(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
+	struct drm_crtc_state *crtc_state = crtc->state;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	int pipe = intel_crtc->pipe;
-	uint16_t coeff = 0x7800; /* 1.0 */
+	int i, pipe = intel_crtc->pipe;
+	uint16_t coeffs[9] = { 0, };
 
-	/*
-	 * TODO: Check what kind of values actually come out of the pipe
-	 * with these coeff/postoff values and adjust to get the best
-	 * accuracy. Perhaps we even need to take the bpc value into
-	 * consideration.
-	 */
+	if (crtc_state->ctm_matrix) {
+		struct drm_color_ctm *ctm =
+			(struct drm_color_ctm *)crtc_state->ctm_matrix->data;
+		uint64_t input[9] = { 0, };
+
+		if (intel_crtc->config->limited_color_range)
+			ctm_matrix_mult_by_limited(input, ctm->matrix);
+		else {
+			for (i = 0; i < ARRAY_SIZE(input); i++)
+				input[i] = ctm->matrix[i];
+		}
 
-	if (intel_crtc->config->limited_color_range)
-		coeff = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */
+
+		/*
+		 * Convert fixed point S31.32 input to format supported by the
+		 * hardware.
+		 */
+		for (i = 0; i < ARRAY_SIZE(coeffs); i++) {
+			uint64_t abs_coeff = ((1ULL << 63) - 1) & input[i];
+
+			/*
+			 * Clamp input value to min/max supported by
+			 * hardware.
+			 */
+			abs_coeff = clamp_val(abs_coeff, 0, CTM_COEFF_4_0 - 1);
+
+			/* sign bit */
+			if (CTM_COEFF_NEGATIVE(input[i]))
+				coeffs[i] |= 1 << 15;
+
+			if (abs_coeff < CTM_COEFF_0_125)
+				coeffs[i] |= (3 << 12) |
+					I9XX_CSC_COEFF_FP(abs_coeff, 12);
+			else if (abs_coeff < CTM_COEFF_0_25)
+				coeffs[i] |= (2 << 12) |
+					I9XX_CSC_COEFF_FP(abs_coeff, 11);
+			else if (abs_coeff < CTM_COEFF_0_5)
+				coeffs[i] |= (1 << 12) |
+					I9XX_CSC_COEFF_FP(abs_coeff, 10);
+			else if (abs_coeff < CTM_COEFF_1_0)
+				coeffs[i] |= I9XX_CSC_COEFF_FP(abs_coeff, 9);
+			else if (abs_coeff < CTM_COEFF_2_0)
+				coeffs[i] |= (7 << 12) |
+					I9XX_CSC_COEFF_FP(abs_coeff, 8);
+			else
+				coeffs[i] |= (6 << 12) |
+					I9XX_CSC_COEFF_FP(abs_coeff, 7);
+		}
+	} else {
+		/*
+		 * Load an identify matrix if no coefficients are provided.
+		 *
+		 * TODO: Check what kind of values actually come out of the
+		 * pipe with these coeff/postoff values and adjust to get the
+		 * best accuracy. Perhaps we even need to take the bpc value
+		 * into consideration.
+		 */
+		for (i = 0; i < 3; i++) {
+			if (intel_crtc->config->limited_color_range)
+				coeffs[i * 3 + i] =
+					I9XX_CSC_COEFF_LIMITED_RANGE;
+			else
+				coeffs[i * 3 + i] = I9XX_CSC_COEFF_1_0;
+		}
+	}
 
 	/*
 	 * GY/GU and RY/RU should be the other way around according
 	 * to BSpec, but reality doesn't agree. Just set them up in
 	 * a way that results in the correct picture.
 	 */
-	I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeff << 16);
-	I915_WRITE(PIPE_CSC_COEFF_BY(pipe), 0);
+	I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeffs[0] << 16 | coeffs[1]);
+	I915_WRITE(PIPE_CSC_COEFF_BY(pipe), coeffs[2] << 16);
 
-	I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeff);
-	I915_WRITE(PIPE_CSC_COEFF_BU(pipe), 0);
+	I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeffs[3] << 16 | coeffs[4]);
+	I915_WRITE(PIPE_CSC_COEFF_BU(pipe), coeffs[5] << 16);
 
-	I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), 0);
-	I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeff << 16);
+	I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), coeffs[6] << 16 | coeffs[7]);
+	I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeffs[8] << 16);
 
 	I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0);
 	I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0);
@@ -88,21 +213,146 @@ static void i9xx_load_csc_matrix(struct drm_crtc *crtc)
 	}
 }
 
-void intel_color_set_csc(struct drm_crtc *crtc)
+/** Loads the legacy palette/gamma unit for the CRTC with the prepared
+ * values.
+ */
+static void i9xx_load_legacy_gamma_lut(struct drm_crtc *crtc)
 {
-	i9xx_load_csc_matrix(crtc);
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_crtc_state *intel_state = to_intel_crtc_state(crtc->state);
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+	int i;
+
+	for (i = 0; i < 256; i++) {
+		uint32_t word = (intel_crtc->lut_r[i] << 16) |
+			(intel_crtc->lut_g[i] << 8) |
+			intel_crtc->lut_b[i];
+		if (HAS_GMCH_DISPLAY(dev))
+			I915_WRITE(PALETTE(pipe, i), word);
+		else
+			I915_WRITE(LGC_PALETTE(pipe, i), word);
+	}
+
+	intel_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
+	I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
 }
 
-/** Loads the palette/gamma unit for the CRTC with the prepared values on  */
-static void i9xx_load_legacy_gamma_lut(struct drm_crtc *crtc)
+static void broadwell_load_degamma_lut(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc_state *state = crtc->state;
+	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+	uint32_t i, lut_size = INTEL_INFO(dev)->color.degamma_lut_size;
+
+	I915_WRITE(PREC_PAL_INDEX(pipe),
+		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT);
+
+	if (state->degamma_lut) {
+		struct drm_color_lut *lut =
+			(struct drm_color_lut *) state->degamma_lut->data;
+
+		for (i = 0; i < lut_size; i++) {
+			uint32_t word =
+			drm_color_lut_extract(lut[i].red, 10) << 20 |
+			drm_color_lut_extract(lut[i].green, 10) << 10 |
+			drm_color_lut_extract(lut[i].blue, 10);
+
+			I915_WRITE(PREC_PAL_DATA(pipe), word);
+		}
+	} else {
+		for (i = 0; i < lut_size; i++) {
+			uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1);
+
+			I915_WRITE(PREC_PAL_DATA(pipe),
+				   (v << 20) | (v << 10) | v);
+		}
+	}
+
+	intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT;
+	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT);
+	POSTING_READ(GAMMA_MODE(pipe));
+
+	/* Reset the index, otherwise it prevents the legacy palette to be
+	 * written properly.
+	 */
+	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
+}
+
+static void broadwell_load_gamma_lut(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc_state *state = crtc->state;
+	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+	uint32_t i, lut_offset = INTEL_INFO(dev)->color.degamma_lut_size,
+		lut_size = INTEL_INFO(dev)->color.gamma_lut_size;
+
+
+	I915_WRITE(PREC_PAL_INDEX(pipe),
+		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT | lut_offset);
+
+	if (state->gamma_lut) {
+		struct drm_color_lut *lut =
+			(struct drm_color_lut *) state->gamma_lut->data;
+
+		for (i = 0; i < lut_size; i++) {
+			uint32_t word =
+			(drm_color_lut_extract(lut[i].red, 10) << 20) |
+			(drm_color_lut_extract(lut[i].green, 10) << 10) |
+			drm_color_lut_extract(lut[i].blue, 10);
+
+			I915_WRITE(PREC_PAL_DATA(pipe), word);
+		}
+
+		/* Program the max register to clamp values > 1.0. */
+		I915_WRITE(PREC_PAL_GC_MAX(pipe, 0),
+			   drm_color_lut_extract(lut[i].red, 16));
+		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1),
+			   drm_color_lut_extract(lut[i].green, 16));
+		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2),
+			   drm_color_lut_extract(lut[i].blue, 16));
+	} else {
+		for (i = 0; i < lut_size; i++) {
+			uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1);
+
+			I915_WRITE(PREC_PAL_DATA(pipe),
+				   (v << 20) | (v << 10) | v);
+		}
+
+		I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16) - 1);
+		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16) - 1);
+		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16) - 1);
+	}
+
+	intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT;
+	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT);
+	POSTING_READ(GAMMA_MODE(pipe));
+
+	/* Reset the index, otherwise it prevents the legacy palette to be
+	 * written properly.
+	 */
+	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
+}
+
+static void intel_color_load_luts_internal(struct drm_crtc *crtc,
+					  bool legacy)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	enum pipe pipe = intel_crtc->pipe;
-	int i;
+	struct intel_crtc_state *intel_state = to_intel_crtc_state(crtc->state);
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
 	bool reenable_ips = false;
 
+	/* The clocks have to be on to load the palette. */
+	if (!crtc->state->active)
+		return;
+
 	if (HAS_GMCH_DISPLAY(dev)) {
 		if (intel_crtc->config->has_dsi_encoder)
 			assert_dsi_pll_enabled(dev_priv);
@@ -114,42 +364,32 @@ static void i9xx_load_legacy_gamma_lut(struct drm_crtc *crtc)
 	 * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled.
 	 */
 	if (IS_HASWELL(dev) && intel_crtc->config->ips_enabled &&
-	    ((I915_READ(GAMMA_MODE(pipe)) & GAMMA_MODE_MODE_MASK) ==
-	     GAMMA_MODE_MODE_SPLIT)) {
+	    intel_state->gamma_mode == GAMMA_MODE_MODE_SPLIT) {
 		hsw_disable_ips(intel_crtc);
 		reenable_ips = true;
 	}
 
-	for (i = 0; i < 256; i++) {
-		uint32_t word = (intel_crtc->lut_r[i] << 16) |
-			(intel_crtc->lut_g[i] << 8) |
-			intel_crtc->lut_b[i];
-		if (HAS_GMCH_DISPLAY(dev))
-			I915_WRITE(PALETTE(pipe, i), word);
-		else
-			I915_WRITE(LGC_PALETTE(pipe, i), word);
+	if (legacy)
+		i9xx_load_legacy_gamma_lut(crtc);
+	else {
+		dev_priv->display.load_degamma_lut(crtc);
+		dev_priv->display.load_gamma_lut(crtc);
 	}
 
-	I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
-
 	if (reenable_ips)
 		hsw_enable_ips(intel_crtc);
 }
 
-void intel_color_load_gamma_lut(struct drm_crtc *crtc)
+void intel_color_legacy_load_lut(struct drm_crtc *crtc)
 {
-	/* The clocks have to be on to load the palette. */
-	if (!crtc->state->active)
-		return;
-
-	i9xx_load_legacy_gamma_lut(crtc);
+	intel_color_load_luts_internal(crtc, true);
 }
 
 void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 				  u16 *blue, uint32_t start, uint32_t size)
 {
-	int end = (start + size > 256) ? 256 : start + size, i;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int end = (start + size > 256) ? 256 : start + size, i;
 
 	for (i = start; i < end; i++) {
 		intel_crtc->lut_r[i] = red[i] >> 8;
@@ -157,11 +397,29 @@ void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 		intel_crtc->lut_b[i] = blue[i] >> 8;
 	}
 
-	intel_color_load_gamma_lut(crtc);
+	intel_color_load_luts_internal(crtc, true);
+}
+
+void intel_color_load_luts(struct drm_crtc *crtc)
+{
+	intel_color_load_luts_internal(crtc,
+				       !crtc->state->degamma_lut &&
+				       !crtc->state->gamma_lut);
+}
+
+void intel_color_set_csc(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (dev_priv->display.load_csc_matrix)
+		dev_priv->display.load_csc_matrix(crtc);
 }
 
 void intel_color_init(struct drm_crtc *crtc)
 {
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int i;
 
@@ -171,4 +429,23 @@ void intel_color_init(struct drm_crtc *crtc)
 		intel_crtc->lut_g[i] = i;
 		intel_crtc->lut_b[i] = i;
 	}
+
+	if (IS_BROADWELL(dev) || IS_SKYLAKE(dev) ||
+	    IS_BROXTON(dev) || IS_KABYLAKE(dev)) {
+		dev_priv->display.load_degamma_lut = broadwell_load_degamma_lut;
+		dev_priv->display.load_gamma_lut = broadwell_load_gamma_lut;
+		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
+	} else {
+		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
+	}
+
+	if (INTEL_INFO(dev)->color.degamma_lut_size != 0 &&
+	    INTEL_INFO(dev)->color.gamma_lut_size != 0) {
+		WARN_ON(!dev_priv->display.load_degamma_lut ||
+			!dev_priv->display.load_gamma_lut ||
+			!dev_priv->display.load_csc_matrix);
+		drm_helper_crtc_enable_color_mgmt(crtc,
+					INTEL_INFO(dev)->color.degamma_lut_size,
+					INTEL_INFO(dev)->color.gamma_lut_size);
+	}
 }
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f10bba2..e38b31b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4864,7 +4864,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 	 * On ILK+ LUT must be loaded before the pipe is running but with
 	 * clocks enabled
 	 */
-	intel_color_load_gamma_lut(crtc);
+	intel_color_load_luts(crtc);
 
 	intel_update_watermarks(crtc);
 	intel_enable_pipe(intel_crtc);
@@ -4959,7 +4959,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	 * On ILK+ LUT must be loaded before the pipe is running but with
 	 * clocks enabled
 	 */
-	intel_color_load_gamma_lut(crtc);
+	intel_color_load_luts(crtc);
 
 	intel_ddi_set_pipe_settings(crtc);
 	if (!intel_crtc->config->has_dsi_encoder)
@@ -6174,7 +6174,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 
 	i9xx_pfit_enable(intel_crtc);
 
-	intel_color_load_gamma_lut(crtc);
+	intel_color_load_luts(crtc);
 
 	intel_enable_pipe(intel_crtc);
 
@@ -6227,7 +6227,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 
 	i9xx_pfit_enable(intel_crtc);
 
-	intel_color_load_gamma_lut(crtc);
+	intel_color_load_luts(crtc);
 
 	intel_update_watermarks(crtc);
 	intel_enable_pipe(intel_crtc);
@@ -11898,7 +11898,7 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
 
 static const struct drm_crtc_helper_funcs intel_helper_funcs = {
 	.mode_set_base_atomic = intel_pipe_set_base_atomic,
-	.load_lut = intel_color_load_gamma_lut,
+	.load_lut = intel_color_legacy_load_lut,
 	.atomic_begin = intel_begin_crtc_commit,
 	.atomic_flush = intel_finish_crtc_commit,
 	.atomic_check = intel_crtc_atomic_check,
@@ -13428,6 +13428,17 @@ static int intel_atomic_commit(struct drm_device *dev,
 			hw_check = true;
 		}
 
+		if (!modeset &&
+		    crtc->state->active &&
+		    crtc->state->color_mgmt_changed) {
+			/* Only update color management when not doing
+			 * a modeset as this will be done by
+			 * crtc_enable already.
+			 */
+			intel_color_set_csc(crtc);
+			intel_color_load_luts(crtc);
+		}
+
 		if (!modeset)
 			intel_pre_plane_update(to_intel_crtc_state(crtc_state));
 
@@ -13519,6 +13530,7 @@ out:
 static const struct drm_crtc_funcs intel_crtc_funcs = {
 	.gamma_set = intel_color_legacy_gamma_set,
 	.set_config = drm_atomic_helper_set_config,
+	.set_property = drm_atomic_helper_crtc_set_property,
 	.destroy = intel_crtc_destroy,
 	.page_flip = intel_crtc_page_flip,
 	.atomic_duplicate_state = intel_crtc_duplicate_state,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2b5d03a..c384c78 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -507,6 +507,8 @@ struct intel_crtc_state {
 	/* IVB sprite scaling w/a (WaCxSRDisabledForSpriteScaling:ivb) */
 	bool disable_lp_wm;
 
+	uint32_t gamma_mode;
+
 	struct {
 		/*
 		 * optimal watermarks, programmed post-vblank when this state
@@ -1620,9 +1622,11 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
 
 /* intel_color.c */
 void intel_color_init(struct drm_crtc *crtc);
+void intel_color_update(struct drm_crtc *crtc);
 void intel_color_set_csc(struct drm_crtc *crtc);
-void intel_color_load_gamma_lut(struct drm_crtc *crtc);
+void intel_color_load_luts(struct drm_crtc *crtc);
 void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
 				  u16 *blue, uint32_t start, uint32_t size);
+void intel_color_legacy_load_lut(struct drm_crtc *crtc);
 
 #endif /* __INTEL_DRV_H__ */
-- 
2.7.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/6] drm/i915: Implement color management on chv

[Lionel Landwerlin]

Patch based on a previous series by Shashank Sharma.

v2: Update contributors

Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
---
 drivers/gpu/drm/i915/i915_drv.c    |   3 +
 drivers/gpu/drm/i915/i915_reg.h    |  40 +++++++++++
 drivers/gpu/drm/i915/intel_color.c | 139 ++++++++++++++++++++++++++++++++++++-
 3 files changed, 180 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b65aa20..eb69ff2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -68,6 +68,8 @@ static struct drm_driver driver;
 
 #define BDW_COLORS \
 	.color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }
+#define CHV_COLORS \
+	.color = { .degamma_lut_size = 65, .gamma_lut_size = 257 }
 
 static const struct intel_device_info intel_i830_info = {
 	.gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2,
@@ -325,6 +327,7 @@ static const struct intel_device_info intel_cherryview_info = {
 	.display_mmio_offset = VLV_DISPLAY_BASE,
 	GEN_CHV_PIPEOFFSETS,
 	CURSOR_OFFSETS,
+	CHV_COLORS,
 };
 
 static const struct intel_device_info intel_skylake_info = {
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 1dc5d3b..c1e5189 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -7685,6 +7685,46 @@ enum skl_disp_power_wells {
 #define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE3(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B, _PAL_PREC_GC_MAX_C) + (i) * 4)
 #define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE3(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B, _PAL_PREC_EXT_GC_MAX_C) + (i) * 4)
 
+/* pipe CSC & degamma/gamma LUTs on CHV */
+#define _CGM_PIPE_A_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x67900)
+#define _CGM_PIPE_A_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x67904)
+#define _CGM_PIPE_A_CSC_COEFF45	(VLV_DISPLAY_BASE + 0x67908)
+#define _CGM_PIPE_A_CSC_COEFF67	(VLV_DISPLAY_BASE + 0x6790C)
+#define _CGM_PIPE_A_CSC_COEFF8	(VLV_DISPLAY_BASE + 0x67910)
+#define _CGM_PIPE_A_DEGAMMA	(VLV_DISPLAY_BASE + 0x66000)
+#define _CGM_PIPE_A_GAMMA	(VLV_DISPLAY_BASE + 0x67000)
+#define _CGM_PIPE_A_MODE	(VLV_DISPLAY_BASE + 0x67A00)
+#define   CGM_PIPE_MODE_GAMMA	(1 << 2)
+#define   CGM_PIPE_MODE_CSC	(1 << 1)
+#define   CGM_PIPE_MODE_DEGAMMA	(1 << 0)
+
+#define _CGM_PIPE_B_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x69900)
+#define _CGM_PIPE_B_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x69904)
+#define _CGM_PIPE_B_CSC_COEFF45	(VLV_DISPLAY_BASE + 0x69908)
+#define _CGM_PIPE_B_CSC_COEFF67	(VLV_DISPLAY_BASE + 0x6990C)
+#define _CGM_PIPE_B_CSC_COEFF8	(VLV_DISPLAY_BASE + 0x69910)
+#define _CGM_PIPE_B_DEGAMMA	(VLV_DISPLAY_BASE + 0x68000)
+#define _CGM_PIPE_B_GAMMA	(VLV_DISPLAY_BASE + 0x69000)
+#define _CGM_PIPE_B_MODE	(VLV_DISPLAY_BASE + 0x69A00)
+
+#define _CGM_PIPE_C_CSC_COEFF01	(VLV_DISPLAY_BASE + 0x6B900)
+#define _CGM_PIPE_C_CSC_COEFF23	(VLV_DISPLAY_BASE + 0x6B904)
+#define _CGM_PIPE_C_CSC_COEFF45	(VLV_DISPLAY_BASE + 0x6B908)
+#define _CGM_PIPE_C_CSC_COEFF67	(VLV_DISPLAY_BASE + 0x6B90C)
+#define _CGM_PIPE_C_CSC_COEFF8	(VLV_DISPLAY_BASE + 0x6B910)
+#define _CGM_PIPE_C_DEGAMMA	(VLV_DISPLAY_BASE + 0x6A000)
+#define _CGM_PIPE_C_GAMMA	(VLV_DISPLAY_BASE + 0x6B000)
+#define _CGM_PIPE_C_MODE	(VLV_DISPLAY_BASE + 0x6BA00)
+
+#define CGM_PIPE_CSC_COEFF01(pipe)	_MMIO_PIPE3(pipe, _CGM_PIPE_A_CSC_COEFF01, _CGM_PIPE_B_CSC_COEFF01, _CGM_PIPE_C_CSC_COEFF01)
+#define CGM_PIPE_CSC_COEFF23(pipe)	_MMIO_PIPE3(pipe, _CGM_PIPE_A_CSC_COEFF23, _CGM_PIPE_B_CSC_COEFF23, _CGM_PIPE_C_CSC_COEFF23)
+#define CGM_PIPE_CSC_COEFF45(pipe)	_MMIO_PIPE3(pipe, _CGM_PIPE_A_CSC_COEFF45, _CGM_PIPE_B_CSC_COEFF45, _CGM_PIPE_C_CSC_COEFF45)
+#define CGM_PIPE_CSC_COEFF67(pipe)	_MMIO_PIPE3(pipe, _CGM_PIPE_A_CSC_COEFF67, _CGM_PIPE_B_CSC_COEFF67, _CGM_PIPE_C_CSC_COEFF67)
+#define CGM_PIPE_CSC_COEFF8(pipe)	_MMIO_PIPE3(pipe, _CGM_PIPE_A_CSC_COEFF8, _CGM_PIPE_B_CSC_COEFF8, _CGM_PIPE_C_CSC_COEFF8)
+#define CGM_PIPE_DEGAMMA(pipe, i, w)	_MMIO(_PIPE3(pipe, _CGM_PIPE_A_DEGAMMA, _CGM_PIPE_B_DEGAMMA, _CGM_PIPE_C_DEGAMMA) + (i) * 4 + (w) * 4)
+#define CGM_PIPE_GAMMA(pipe, i, w)	_MMIO(_PIPE3(pipe, _CGM_PIPE_A_GAMMA, _CGM_PIPE_B_GAMMA, _CGM_PIPE_C_GAMMA) + (i) * 4 + (w) * 4)
+#define CGM_PIPE_MODE(pipe)		_MMIO_PIPE3(pipe, _CGM_PIPE_A_MODE, _CGM_PIPE_B_MODE, _CGM_PIPE_C_MODE)
+
 /* MIPI DSI registers */
 
 #define _MIPI_PORT(port, a, c)	_PORT3(port, a, 0, c)	/* ports A and C only */
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index 782b9d8..da6ff09 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -213,6 +213,54 @@ static void i9xx_load_csc_matrix(struct drm_crtc *crtc)
 	}
 }
 
+/*
+ * Set up the pipe CSC unit on CherryView.
+ */
+static void cherryview_load_csc_matrix(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_crtc_state *crtc_state = crtc->state;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	int pipe = intel_crtc->pipe;
+
+
+	if (crtc_state->ctm_matrix) {
+		struct drm_color_ctm *ctm =
+			(struct drm_color_ctm *)crtc_state->ctm_matrix->data;
+		uint16_t coeffs[9] = { 0, };
+		int i;
+
+		for (i = 0; i < ARRAY_SIZE(coeffs); i++) {
+			uint64_t abs_coeff =
+				((1ULL << 63) - 1) & ctm->matrix[i];
+
+			abs_coeff = clamp_val(abs_coeff, 0, (1 << 15) - 1);
+
+			/* Write coefficients in S3.12 format. */
+			if (ctm->matrix[i] & (1ULL << 63))
+				coeffs[i] = 1 << 15;
+			coeffs[i] |= ((abs_coeff >> 32) & 7) << 12;
+			coeffs[i] |= (((abs_coeff >> 19) + 1) >> 1) & 0xfff;
+		}
+
+		I915_WRITE(CGM_PIPE_CSC_COEFF01(pipe),
+			   coeffs[1] << 16 | coeffs[0]);
+		I915_WRITE(CGM_PIPE_CSC_COEFF23(pipe),
+			   coeffs[3] << 16 | coeffs[2]);
+		I915_WRITE(CGM_PIPE_CSC_COEFF45(pipe),
+			   coeffs[5] << 16 | coeffs[4]);
+		I915_WRITE(CGM_PIPE_CSC_COEFF67(pipe),
+			   coeffs[7] << 16 | coeffs[6]);
+		I915_WRITE(CGM_PIPE_CSC_COEFF8(pipe), coeffs[8]);
+	}
+
+	I915_WRITE(CGM_PIPE_MODE(pipe),
+		   (crtc_state->ctm_matrix ? CGM_PIPE_MODE_CSC : 0) |
+		   (crtc_state->degamma_lut ? CGM_PIPE_MODE_DEGAMMA : 0) |
+		   (crtc_state->gamma_lut ? CGM_PIPE_MODE_GAMMA : 0));
+}
+
 /** Loads the legacy palette/gamma unit for the CRTC with the prepared
  * values.
  */
@@ -282,6 +330,47 @@ static void broadwell_load_degamma_lut(struct drm_crtc *crtc)
 	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
 }
 
+static void cherryview_load_degamma_lut(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc_state *state = crtc->state;
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+
+	if (state->degamma_lut) {
+		struct drm_color_lut *lut =
+			(struct drm_color_lut *) state->degamma_lut->data;
+		uint32_t i, lut_size = INTEL_INFO(dev)->color.degamma_lut_size;
+		uint32_t word0, word1;
+
+		for (i = 0; i < lut_size; i++) {
+			/* Write LUT in U0.14 format. */
+			word0 =
+			(drm_color_lut_extract(lut[i].green, 14) << 16) |
+			drm_color_lut_extract(lut[i].blue, 14);
+			word1 = drm_color_lut_extract(lut[i].red, 14);
+
+			I915_WRITE(CGM_PIPE_DEGAMMA(pipe, i, 0), word0);
+			I915_WRITE(CGM_PIPE_DEGAMMA(pipe, i, 1), word1);
+		}
+		/* Write the 65's entry of the LUT with the last entry given
+		 * by user space to clamp values > 1.0.
+		 */
+		word0 =
+		(drm_color_lut_extract(lut[lut_size - 1].green, 14) << 16) |
+		drm_color_lut_extract(lut[lut_size - 1].blue, 14);
+		word1 = drm_color_lut_extract(lut[lut_size - 1].red, 14);
+
+		I915_WRITE(CGM_PIPE_DEGAMMA(pipe, lut_size, 0), word0);
+		I915_WRITE(CGM_PIPE_DEGAMMA(pipe, lut_size, 1), word1);
+	}
+
+	I915_WRITE(CGM_PIPE_MODE(pipe),
+		   (state->ctm_matrix ? CGM_PIPE_MODE_CSC : 0) |
+		   (state->degamma_lut ? CGM_PIPE_MODE_DEGAMMA : 0) |
+		   (state->gamma_lut ? CGM_PIPE_MODE_GAMMA : 0));
+}
+
 static void broadwell_load_gamma_lut(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -339,6 +428,47 @@ static void broadwell_load_gamma_lut(struct drm_crtc *crtc)
 	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
 }
 
+static void cherryview_load_gamma_lut(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct drm_crtc_state *state = crtc->state;
+	enum pipe pipe = to_intel_crtc(crtc)->pipe;
+
+	if (state->gamma_lut) {
+		struct drm_color_lut *lut =
+			(struct drm_color_lut *) state->gamma_lut->data;
+		uint32_t i, lut_size = INTEL_INFO(dev)->color.gamma_lut_size;
+		uint32_t word0, word1;
+
+		for (i = 0; i < lut_size; i++) {
+			/* Write LUT in U0.10 format. */
+			word0 =
+			(drm_color_lut_extract(lut[i].green, 10) << 16) |
+			drm_color_lut_extract(lut[i].blue, 10);
+			word1 = drm_color_lut_extract(lut[i].red, 10);
+
+			I915_WRITE(CGM_PIPE_GAMMA(pipe, i, 0), word0);
+			I915_WRITE(CGM_PIPE_GAMMA(pipe, i, 1), word1);
+		}
+		/* Write the 257's entry of the LUT with the last entry given
+		 * by user space to clamp values > 1.0.
+		 */
+		word0 =
+		(drm_color_lut_extract(lut[lut_size - 1].green, 10) << 16) |
+		drm_color_lut_extract(lut[lut_size - 1].blue, 10);
+		word1 = drm_color_lut_extract(lut[lut_size - 1].red, 10);
+
+		I915_WRITE(CGM_PIPE_GAMMA(pipe, lut_size, 0), word0);
+		I915_WRITE(CGM_PIPE_GAMMA(pipe, lut_size, 1), word1);
+	}
+
+	I915_WRITE(CGM_PIPE_MODE(pipe),
+		   (state->ctm_matrix ? CGM_PIPE_MODE_CSC : 0) |
+		   (state->degamma_lut ? CGM_PIPE_MODE_DEGAMMA : 0) |
+		   (state->gamma_lut ? CGM_PIPE_MODE_GAMMA : 0));
+}
+
 static void intel_color_load_luts_internal(struct drm_crtc *crtc,
 					  bool legacy)
 {
@@ -430,8 +560,13 @@ void intel_color_init(struct drm_crtc *crtc)
 		intel_crtc->lut_b[i] = i;
 	}
 
-	if (IS_BROADWELL(dev) || IS_SKYLAKE(dev) ||
-	    IS_BROXTON(dev) || IS_KABYLAKE(dev)) {
+	if (IS_CHERRYVIEW(dev)) {
+		dev_priv->display.load_degamma_lut =
+			cherryview_load_degamma_lut;
+		dev_priv->display.load_gamma_lut = cherryview_load_gamma_lut;
+		dev_priv->display.load_csc_matrix = cherryview_load_csc_matrix;
+	} else if (IS_BROADWELL(dev) || IS_SKYLAKE(dev) ||
+		   IS_BROXTON(dev) || IS_KABYLAKE(dev)) {
 		dev_priv->display.load_degamma_lut = broadwell_load_degamma_lut;
 		dev_priv->display.load_gamma_lut = broadwell_load_gamma_lut;
 		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
-- 
2.7.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

✗ Fi.CI.BAT: failure for Pipe level color management (rev4)

[Patchwork]

== Summary ==

Series 2720v4 Pipe level color management
http://patchwork.freedesktop.org/api/1.0/series/2720/revisions/4/mbox/

Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                pass       -> DMESG-WARN (ilk-hp8440p) UNSTABLE
        Subgroup basic-flip-vs-wf_vblank:
                fail       -> PASS       (snb-x220t)
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (skl-i5k-2)
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (skl-i5k-2)
                pass       -> INCOMPLETE (hsw-gt2)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                fail       -> PASS       (hsw-brixbox)
        Subgroup basic-rte:
                dmesg-warn -> PASS       (bsw-nuc-2)
                pass       -> DMESG-WARN (byt-nuc) UNSTABLE

bdw-nuci7        total:161  pass:152  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:164  pass:152  dwarn:0   dfail:0   fail:0   skip:12 
bsw-nuc-2        total:164  pass:136  dwarn:0   dfail:0   fail:0   skip:28 
byt-nuc          total:164  pass:140  dwarn:1   dfail:0   fail:0   skip:23 
hsw-brixbox      total:164  pass:151  dwarn:0   dfail:0   fail:0   skip:13 
hsw-gt2          total:143  pass:134  dwarn:0   dfail:0   fail:0   skip:8  
ilk-hp8440p      total:164  pass:115  dwarn:1   dfail:0   fail:0   skip:48 
ivb-t430s        total:164  pass:150  dwarn:0   dfail:0   fail:0   skip:14 
skl-i5k-2        total:164  pass:149  dwarn:1   dfail:0   fail:0   skip:14 
snb-dellxps      total:164  pass:142  dwarn:0   dfail:0   fail:0   skip:22 
snb-x220t        total:164  pass:142  dwarn:0   dfail:0   fail:1   skip:21 

Results at /archive/results/CI_IGT_test/Patchwork_1381/

d033b3eb38d06039cad3c78109f6e666c3fb9cf4 drm-intel-nightly: 2016y-02m-09d-10h-52m-16s UTC integration manifest
2fdbc2f724c39d260b1808a6cc5c754c66161d5b drm/i915: Implement color management on chv
f0c1f9c7ee77496b458f4bf7b3534496cc08553f drm/i915: Implement color management on bdw/skl/bxt/kbl
0b0c401d55c9239b042dec968d5fbe254ea6f121 drm/i915: enable legacy palette for pipe C
819b94e366ff32cc5ae122d1fb57c350bec21311 drm/i915: enable CSC for pipe C
81d5e67528c425de15f7da75036673f84b8f08e2 drm: introduce color correction properties
7dd958c5472e69591b965a2d67f5a904f0b32f87 drm/i915: Extract out gamma table and CSC to their own file

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

Re: [PATCH 2/6] drm: introduce color correction properties

[Matt Roper]

On Tue, Feb 09, 2016 at 12:19:14PM +0000, Lionel Landwerlin wrote:
> Patch based on a previous series by Shashank Sharma.
> 
> v2: Register LUT size properties as range
> 
> v3: Fix round in drm_color_lut_get_value() helper
>     More docs on how degamma/gamma properties are used
> 
> v4: Update contributors

A couple minor notes:
 * In the kerneldoc, you might want to spell out the first use of the
   acronyms for clarity.  I.e., Color Transformation Matrix, Lookup
   Table.
 * You might want to add DRM_DEBUG_KMS() calls to some of the error
   conditions (e.g., when the blob has an invalid size) so that it will
   be more apparent why we failed when looking at kernel logs.
 * Looks like one minor whitespace error, noted below.
 * One minor helper note below.

Aside from those,
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>


Also, somewhere (probably as a followup patch to this series), I think
we need some kerneldoc text that describes the relationship between this
color management work and the old legacy gamma ioctl.  I.e., what
happens if you intermix calls to the legacy ioctl with updates to the
color manager properties.


Matt


> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
...
> @@ -419,8 +472,47 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>  		ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
>  		drm_property_unreference_blob(mode);
>  		return ret;
> -	}
> -	else if (crtc->funcs->atomic_set_property)
> +	} else if (property == config->degamma_lut_property) {
> +		bool replaced = false;
> +		uint64_t lut_size = 0;
> +
> +		ret = drm_object_property_get_value(&crtc->base,
> +					config->degamma_lut_size_property,
> +					&lut_size);
> +		if (ret == 0)
> +			ret = drm_atomic_replace_property_blob_from_id(crtc,
> +					&state->degamma_lut,
> +					val,
> +					lut_size * sizeof(struct drm_color_lut),
> +					&replaced);
> +		state->color_mgmt_changed = replaced;
> +		return ret;
> +	} else if (property == config->ctm_matrix_property) {
> +		bool replaced = false;
> +
> +		ret = drm_atomic_replace_property_blob_from_id(crtc,
> +					&state->ctm_matrix,
> +					val,
> +					sizeof(struct drm_color_ctm),
> +							       &replaced);

Looks like there might be something off with the whitespace here.


> +		state->color_mgmt_changed = replaced;
> +		return ret;
> +	} else if (property == config->gamma_lut_property) {
> +		bool replaced = false;
> +		uint64_t lut_size = 0;
> +
> +		ret = drm_object_property_get_value(&crtc->base,
> +					config->gamma_lut_size_property,
> +					&lut_size);
> +		if (ret == 0)
> +			ret = drm_atomic_replace_property_blob_from_id(crtc,
> +					&state->gamma_lut,
> +					val,
> +					lut_size * sizeof(struct drm_color_lut),
> +					&replaced);
> +		state->color_mgmt_changed = replaced;
> +		return ret;
> +	} else if (crtc->funcs->atomic_set_property)
>  		return crtc->funcs->atomic_set_property(crtc, state, property, val);
>  	else
>  		return -EINVAL;
> @@ -456,6 +548,12 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>  		*val = state->active;
>  	else if (property == config->prop_mode_id)
>  		*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
> +	else if (property == config->degamma_lut_property)
> +		*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
> +	else if (property == config->ctm_matrix_property)
> +		*val = (state->ctm_matrix) ? state->ctm_matrix->base.id : 0;
> +	else if (property == config->gamma_lut_property)
> +		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>  	else if (crtc->funcs->atomic_get_property)
>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>  	else
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 2b430b0..d13dfd8 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2513,10 +2513,17 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>  
>  	if (state->mode_blob)
>  		drm_property_reference_blob(state->mode_blob);
> +	if (state->degamma_lut)
> +		drm_property_reference_blob(state->degamma_lut);
> +	if (state->ctm_matrix)
> +		drm_property_reference_blob(state->ctm_matrix);
> +	if (state->gamma_lut)
> +		drm_property_reference_blob(state->gamma_lut);
>  	state->mode_changed = false;
>  	state->active_changed = false;
>  	state->planes_changed = false;
>  	state->connectors_changed = false;
> +	state->color_mgmt_changed = false;
>  	state->event = NULL;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
> @@ -2557,6 +2564,9 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
>  					    struct drm_crtc_state *state)
>  {
>  	drm_property_unreference_blob(state->mode_blob);
> +	drm_property_unreference_blob(state->degamma_lut);
> +	drm_property_unreference_blob(state->ctm_matrix);
> +	drm_property_unreference_blob(state->gamma_lut);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
>  
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index b19c8f4..8e8f57a 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1565,6 +1565,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>  		return -ENOMEM;
>  	dev->mode_config.prop_mode_id = prop;
>  
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"DEGAMMA_LUT", 0);

Not a big deal either way, but there's a drm_property_create_blob() you
could use to make the code slightly simpler for the blob properties
here.


> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.degamma_lut_property = prop;
> +
> +	prop = drm_property_create_range(dev,
> +			DRM_MODE_PROP_IMMUTABLE,
> +			"DEGAMMA_LUT_SIZE", 0, UINT_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.degamma_lut_size_property = prop;
> +
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"CTM_MATRIX", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.ctm_matrix_property = prop;
> +
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"GAMMA_LUT", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.gamma_lut_property = prop;
> +
> +	prop = drm_property_create_range(dev,
> +			DRM_MODE_PROP_IMMUTABLE,
> +			"GAMMA_LUT_SIZE", 0, UINT_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.gamma_lut_size_property = prop;
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 9f8b894..3cd8cbb 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -1073,3 +1073,36 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>  	return drm_plane_helper_commit(plane, plane_state, old_fb);
>  }
>  EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
> +
> +/**
> + * drm_helper_crtc_enable_color_mgmt - enable color management properties
> + * @crtc: DRM CRTC
> + * @degamma_lut_size: the size of the degamma lut (before CSC)
> + * @gamma_lut_size: the size of the gamma lut (after CSC)
> + *
> + * This function lets the driver enable the color correction properties on a
> + * CRTC. This includes 3 degamma, csc and gamma properties that userspace can
> + * set and 2 size properties to inform the userspace of the lut sizes.
> + */
> +void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> +				       int degamma_lut_size,
> +				       int gamma_lut_size)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	drm_object_attach_property(&crtc->base,
> +				   config->degamma_lut_property, 0);
> +	drm_object_attach_property(&crtc->base,
> +				   config->ctm_matrix_property, 0);
> +	drm_object_attach_property(&crtc->base,
> +				   config->gamma_lut_property, 0);
> +
> +	drm_object_attach_property(&crtc->base,
> +				   config->degamma_lut_size_property,
> +				   degamma_lut_size);
> +	drm_object_attach_property(&crtc->base,
> +				   config->gamma_lut_size_property,
> +				   gamma_lut_size);
> +}
> +EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 8c7fb3d..e07b174 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -305,6 +305,8 @@ struct drm_plane_helper_funcs;
>   * @mode_changed: crtc_state->mode or crtc_state->enable has been changed
>   * @active_changed: crtc_state->active has been toggled.
>   * @connectors_changed: connectors to this crtc have been updated
> + * @color_mgmt_changed: color management properties have changed (degamma or
> + *	gamma LUT or CSC matrix)
>   * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
>   * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors
>   * @encoder_mask: bitmask of (1 << drm_encoder_index(encoder)) of attached encoders
> @@ -312,6 +314,11 @@ struct drm_plane_helper_funcs;
>   * 	update to ensure framebuffer cleanup isn't done too early
>   * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
>   * @mode: current mode timings
> + * @degamma_lut: Lookup table for converting framebuffer pixel data
> + *	before apply the conversion matrix
> + * @ctm_matrix: Transformation matrix
> + * @gamma_lut: Lookup table for converting pixel data after the
> + *	conversion matrix
>   * @event: optional pointer to a DRM event to signal upon completion of the
>   * 	state update
>   * @state: backpointer to global drm_atomic_state
> @@ -333,6 +340,7 @@ struct drm_crtc_state {
>  	bool mode_changed : 1;
>  	bool active_changed : 1;
>  	bool connectors_changed : 1;
> +	bool color_mgmt_changed : 1;
>  
>  	/* attached planes bitmask:
>  	 * WARNING: transitional helpers do not maintain plane_mask so
> @@ -355,6 +363,11 @@ struct drm_crtc_state {
>  	/* blob property to expose current mode to atomic userspace */
>  	struct drm_property_blob *mode_blob;
>  
> +	/* blob property to expose color management to userspace */
> +	struct drm_property_blob *degamma_lut;
> +	struct drm_property_blob *ctm_matrix;
> +	struct drm_property_blob *gamma_lut;
> +
>  	struct drm_pending_vblank_event *event;
>  
>  	struct drm_atomic_state *state;
> @@ -757,7 +770,7 @@ struct drm_crtc {
>  	int x, y;
>  	const struct drm_crtc_funcs *funcs;
>  
> -	/* CRTC gamma size for reporting to userspace */
> +	/* Legacy FB CRTC gamma size for reporting to userspace */
>  	uint32_t gamma_size;
>  	uint16_t *gamma_store;
>  
> @@ -2025,6 +2038,15 @@ struct drm_mode_config_funcs {
>   * @property_blob_list: list of all the blob property objects
>   * @blob_lock: mutex for blob property allocation and management
>   * @*_property: core property tracking
> + * @degamma_lut_property: LUT used to convert the framebuffer's colors to linear
> + *	gamma
> + * @degamma_lut_size_property: size of the degamma LUT as supported by the
> + *	driver (read-only)
> + * @ctm_matrix_property: Matrix used to convert colors after the lookup in the
> + *	degamma LUT
> + * @gamma_lut_property: LUT used to convert the colors, after the CSC matrix, to
> + *	the gamma space of the connected screen (read-only)
> + * @gamma_lut_size_property: size of the gamma LUT as supported by the driver
>   * @preferred_depth: preferred RBG pixel depth, used by fb helpers
>   * @prefer_shadow: hint to userspace to prefer shadow-fb rendering
>   * @async_page_flip: does this device support async flips on the primary plane?
> @@ -2126,6 +2148,13 @@ struct drm_mode_config {
>  	struct drm_property *aspect_ratio_property;
>  	struct drm_property *dirty_info_property;
>  
> +	/* Optional color correction properties */
> +	struct drm_property *degamma_lut_property;
> +	struct drm_property *degamma_lut_size_property;
> +	struct drm_property *ctm_matrix_property;
> +	struct drm_property *gamma_lut_property;
> +	struct drm_property *gamma_lut_size_property;
> +
>  	/* properties for virtual machine layout */
>  	struct drm_property *suggested_x_property;
>  	struct drm_property *suggested_y_property;
> @@ -2548,6 +2577,19 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
>  	return mo ? obj_to_property(mo) : NULL;
>  }
>  
> +/*
> + * Extract a degamma/gamma LUT value provided by user and round it to the
> + * precision supported by the hardware.
> + */
> +static inline uint32_t drm_color_lut_extract(uint32_t user_input,
> +					     uint32_t bit_precision)
> +{
> +	uint32_t val = ((user_input << 1) + 1) >> (16 - bit_precision + 1);
> +	uint32_t max = 0xffff >> (16 - bit_precision);
> +
> +	return clamp_val(val, 0, max);
> +}
> +
>  /* Plane list iterator for legacy (overlay only) planes. */
>  #define drm_for_each_legacy_plane(plane, dev) \
>  	list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index 4b37afa..97fa894 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -48,6 +48,9 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
>  				     struct drm_display_mode *mode,
>  				     int x, int y,
>  				     struct drm_framebuffer *old_fb);
> +extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> +					      int degamma_lut_size,
> +					      int gamma_lut_size);
>  extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc);
>  extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
>  
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 50adb46..c021743 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -487,6 +487,21 @@ struct drm_mode_crtc_lut {
>  	__u64 blue;
>  };
>  
> +struct drm_color_ctm {
> +	/* Conversion matrix in S31.32 format. */
> +	__s64 matrix[9];
> +};
> +
> +struct drm_color_lut {
> +	/*
> +	 * Data is U0.16 fixed point format.
> +	 */
> +	__u16 red;
> +	__u16 green;
> +	__u16 blue;
> +	__u16 reserved;
> +};
> +
>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01
>  #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
>  #define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT|DRM_MODE_PAGE_FLIP_ASYNC)
> -- 
> 2.7.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [Intel-gfx] [PATCH 3/6] drm/i915: enable CSC for pipe C

[Matt Roper]

On Tue, Feb 09, 2016 at 12:19:15PM +0000, Lionel Landwerlin wrote:
> Patch based on a previous series by Shashank Sharma.
> 
> v2: Update contributors

Is this patch actually necessary?  If I recall correctly, _MMIO_PIPE()
will support any number of pipes if the delta between register offsets
for each pipe is constant (in that case, you only need to know the
offset of the first two pipes' registers to calculate the offset of any
subsequent pipe's register).  I think _MMIO_PIPE3 is necessary only if
the difference between register offsets is non-constant and thus all
three offsets need to be specified individually.

Same comment for patch #4 of your series.


Matt

> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 40 +++++++++++++++++++++++++++-------------
>  1 file changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 188ad5d..7ba8a99 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7634,19 +7634,33 @@ enum skl_disp_power_wells {
>  #define _PIPE_B_CSC_POSTOFF_ME	0x49144
>  #define _PIPE_B_CSC_POSTOFF_LO	0x49148
>  
> -#define PIPE_CSC_COEFF_RY_GY(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_RY_GY, _PIPE_B_CSC_COEFF_RY_GY)
> -#define PIPE_CSC_COEFF_BY(pipe)		_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_BY, _PIPE_B_CSC_COEFF_BY)
> -#define PIPE_CSC_COEFF_RU_GU(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_RU_GU, _PIPE_B_CSC_COEFF_RU_GU)
> -#define PIPE_CSC_COEFF_BU(pipe)		_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_BU, _PIPE_B_CSC_COEFF_BU)
> -#define PIPE_CSC_COEFF_RV_GV(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_RV_GV, _PIPE_B_CSC_COEFF_RV_GV)
> -#define PIPE_CSC_COEFF_BV(pipe)		_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_BV, _PIPE_B_CSC_COEFF_BV)
> -#define PIPE_CSC_MODE(pipe)		_MMIO_PIPE(pipe, _PIPE_A_CSC_MODE, _PIPE_B_CSC_MODE)
> -#define PIPE_CSC_PREOFF_HI(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_PREOFF_HI, _PIPE_B_CSC_PREOFF_HI)
> -#define PIPE_CSC_PREOFF_ME(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_PREOFF_ME, _PIPE_B_CSC_PREOFF_ME)
> -#define PIPE_CSC_PREOFF_LO(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_PREOFF_LO, _PIPE_B_CSC_PREOFF_LO)
> -#define PIPE_CSC_POSTOFF_HI(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_HI, _PIPE_B_CSC_POSTOFF_HI)
> -#define PIPE_CSC_POSTOFF_ME(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME)
> -#define PIPE_CSC_POSTOFF_LO(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO)
> +#define _PIPE_C_CSC_COEFF_RY_GY	0x49210
> +#define _PIPE_C_CSC_COEFF_BY	0x49214
> +#define _PIPE_C_CSC_COEFF_RU_GU	0x49218
> +#define _PIPE_C_CSC_COEFF_BU	0x4921c
> +#define _PIPE_C_CSC_COEFF_RV_GV	0x49220
> +#define _PIPE_C_CSC_COEFF_BV	0x49224
> +#define _PIPE_C_CSC_MODE	0x49228
> +#define _PIPE_C_CSC_PREOFF_HI	0x49230
> +#define _PIPE_C_CSC_PREOFF_ME	0x49234
> +#define _PIPE_C_CSC_PREOFF_LO	0x49238
> +#define _PIPE_C_CSC_POSTOFF_HI	0x49240
> +#define _PIPE_C_CSC_POSTOFF_ME	0x49244
> +#define _PIPE_C_CSC_POSTOFF_LO	0x49248
> +
> +#define PIPE_CSC_COEFF_RY_GY(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_RY_GY, _PIPE_B_CSC_COEFF_RY_GY, _PIPE_C_CSC_COEFF_RY_GY)
> +#define PIPE_CSC_COEFF_BY(pipe)		_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_BY, _PIPE_B_CSC_COEFF_BY, _PIPE_C_CSC_COEFF_BY)
> +#define PIPE_CSC_COEFF_RU_GU(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_RU_GU, _PIPE_B_CSC_COEFF_RU_GU, _PIPE_C_CSC_COEFF_RU_GU)
> +#define PIPE_CSC_COEFF_BU(pipe)		_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_BU, _PIPE_B_CSC_COEFF_BU, _PIPE_C_CSC_COEFF_BU)
> +#define PIPE_CSC_COEFF_RV_GV(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_RV_GV, _PIPE_B_CSC_COEFF_RV_GV, _PIPE_C_CSC_COEFF_RV_GV)
> +#define PIPE_CSC_COEFF_BV(pipe)		_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_BV, _PIPE_B_CSC_COEFF_BV, _PIPE_C_CSC_COEFF_BV)
> +#define PIPE_CSC_MODE(pipe)		_MMIO_PIPE3(pipe, _PIPE_A_CSC_MODE, _PIPE_B_CSC_MODE, _PIPE_C_CSC_MODE)
> +#define PIPE_CSC_PREOFF_HI(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_PREOFF_HI, _PIPE_B_CSC_PREOFF_HI, _PIPE_C_CSC_PREOFF_HI)
> +#define PIPE_CSC_PREOFF_ME(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_PREOFF_ME, _PIPE_B_CSC_PREOFF_ME, _PIPE_C_CSC_PREOFF_ME)
> +#define PIPE_CSC_PREOFF_LO(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_PREOFF_LO, _PIPE_B_CSC_PREOFF_LO, _PIPE_C_CSC_PREOFF_LO)
> +#define PIPE_CSC_POSTOFF_HI(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_HI, _PIPE_B_CSC_POSTOFF_HI, _PIPE_C_CSC_POSTOFF_HI)
> +#define PIPE_CSC_POSTOFF_ME(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME, _PIPE_C_CSC_POSTOFF_ME)
> +#define PIPE_CSC_POSTOFF_LO(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO, _PIPE_C_CSC_POSTOFF_LO)
>  
>  /* MIPI DSI registers */
>  
> -- 
> 2.7.0
> 
> _______________________________________________
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Intel-gfx] [PATCH 3/6] drm/i915: enable CSC for pipe C

[Lionel Landwerlin]

Thanks Matt, I'll drop this patch in the next series.

-
Lionel

On 18/02/16 01:55, Matt Roper wrote:
> On Tue, Feb 09, 2016 at 12:19:15PM +0000, Lionel Landwerlin wrote:
>> Patch based on a previous series by Shashank Sharma.
>>
>> v2: Update contributors
> Is this patch actually necessary?  If I recall correctly, _MMIO_PIPE()
> will support any number of pipes if the delta between register offsets
> for each pipe is constant (in that case, you only need to know the
> offset of the first two pipes' registers to calculate the offset of any
> subsequent pipe's register).  I think _MMIO_PIPE3 is necessary only if
> the difference between register offsets is non-constant and thus all
> three offsets need to be specified individually.
>
> Same comment for patch #4 of your series.
>
>
> Matt
>
>> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
>> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
>> ---
>>   drivers/gpu/drm/i915/i915_reg.h | 40 +++++++++++++++++++++++++++-------------
>>   1 file changed, 27 insertions(+), 13 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 188ad5d..7ba8a99 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -7634,19 +7634,33 @@ enum skl_disp_power_wells {
>>   #define _PIPE_B_CSC_POSTOFF_ME	0x49144
>>   #define _PIPE_B_CSC_POSTOFF_LO	0x49148
>>   
>> -#define PIPE_CSC_COEFF_RY_GY(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_RY_GY, _PIPE_B_CSC_COEFF_RY_GY)
>> -#define PIPE_CSC_COEFF_BY(pipe)		_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_BY, _PIPE_B_CSC_COEFF_BY)
>> -#define PIPE_CSC_COEFF_RU_GU(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_RU_GU, _PIPE_B_CSC_COEFF_RU_GU)
>> -#define PIPE_CSC_COEFF_BU(pipe)		_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_BU, _PIPE_B_CSC_COEFF_BU)
>> -#define PIPE_CSC_COEFF_RV_GV(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_RV_GV, _PIPE_B_CSC_COEFF_RV_GV)
>> -#define PIPE_CSC_COEFF_BV(pipe)		_MMIO_PIPE(pipe, _PIPE_A_CSC_COEFF_BV, _PIPE_B_CSC_COEFF_BV)
>> -#define PIPE_CSC_MODE(pipe)		_MMIO_PIPE(pipe, _PIPE_A_CSC_MODE, _PIPE_B_CSC_MODE)
>> -#define PIPE_CSC_PREOFF_HI(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_PREOFF_HI, _PIPE_B_CSC_PREOFF_HI)
>> -#define PIPE_CSC_PREOFF_ME(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_PREOFF_ME, _PIPE_B_CSC_PREOFF_ME)
>> -#define PIPE_CSC_PREOFF_LO(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_PREOFF_LO, _PIPE_B_CSC_PREOFF_LO)
>> -#define PIPE_CSC_POSTOFF_HI(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_HI, _PIPE_B_CSC_POSTOFF_HI)
>> -#define PIPE_CSC_POSTOFF_ME(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME)
>> -#define PIPE_CSC_POSTOFF_LO(pipe)	_MMIO_PIPE(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO)
>> +#define _PIPE_C_CSC_COEFF_RY_GY	0x49210
>> +#define _PIPE_C_CSC_COEFF_BY	0x49214
>> +#define _PIPE_C_CSC_COEFF_RU_GU	0x49218
>> +#define _PIPE_C_CSC_COEFF_BU	0x4921c
>> +#define _PIPE_C_CSC_COEFF_RV_GV	0x49220
>> +#define _PIPE_C_CSC_COEFF_BV	0x49224
>> +#define _PIPE_C_CSC_MODE	0x49228
>> +#define _PIPE_C_CSC_PREOFF_HI	0x49230
>> +#define _PIPE_C_CSC_PREOFF_ME	0x49234
>> +#define _PIPE_C_CSC_PREOFF_LO	0x49238
>> +#define _PIPE_C_CSC_POSTOFF_HI	0x49240
>> +#define _PIPE_C_CSC_POSTOFF_ME	0x49244
>> +#define _PIPE_C_CSC_POSTOFF_LO	0x49248
>> +
>> +#define PIPE_CSC_COEFF_RY_GY(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_RY_GY, _PIPE_B_CSC_COEFF_RY_GY, _PIPE_C_CSC_COEFF_RY_GY)
>> +#define PIPE_CSC_COEFF_BY(pipe)		_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_BY, _PIPE_B_CSC_COEFF_BY, _PIPE_C_CSC_COEFF_BY)
>> +#define PIPE_CSC_COEFF_RU_GU(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_RU_GU, _PIPE_B_CSC_COEFF_RU_GU, _PIPE_C_CSC_COEFF_RU_GU)
>> +#define PIPE_CSC_COEFF_BU(pipe)		_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_BU, _PIPE_B_CSC_COEFF_BU, _PIPE_C_CSC_COEFF_BU)
>> +#define PIPE_CSC_COEFF_RV_GV(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_RV_GV, _PIPE_B_CSC_COEFF_RV_GV, _PIPE_C_CSC_COEFF_RV_GV)
>> +#define PIPE_CSC_COEFF_BV(pipe)		_MMIO_PIPE3(pipe, _PIPE_A_CSC_COEFF_BV, _PIPE_B_CSC_COEFF_BV, _PIPE_C_CSC_COEFF_BV)
>> +#define PIPE_CSC_MODE(pipe)		_MMIO_PIPE3(pipe, _PIPE_A_CSC_MODE, _PIPE_B_CSC_MODE, _PIPE_C_CSC_MODE)
>> +#define PIPE_CSC_PREOFF_HI(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_PREOFF_HI, _PIPE_B_CSC_PREOFF_HI, _PIPE_C_CSC_PREOFF_HI)
>> +#define PIPE_CSC_PREOFF_ME(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_PREOFF_ME, _PIPE_B_CSC_PREOFF_ME, _PIPE_C_CSC_PREOFF_ME)
>> +#define PIPE_CSC_PREOFF_LO(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_PREOFF_LO, _PIPE_B_CSC_PREOFF_LO, _PIPE_C_CSC_PREOFF_LO)
>> +#define PIPE_CSC_POSTOFF_HI(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_HI, _PIPE_B_CSC_POSTOFF_HI, _PIPE_C_CSC_POSTOFF_HI)
>> +#define PIPE_CSC_POSTOFF_ME(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME, _PIPE_C_CSC_POSTOFF_ME)
>> +#define PIPE_CSC_POSTOFF_LO(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO, _PIPE_C_CSC_POSTOFF_LO)
>>   
>>   /* MIPI DSI registers */
>>   
>> -- 
>> 2.7.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/6] drm/i915: Implement color management on bdw/skl/bxt/kbl

[Matt Roper]

On Tue, Feb 09, 2016 at 12:19:17PM +0000, Lionel Landwerlin wrote:
> Patch based on a previous series by Shashank Sharma.
> 
> v2: Do not read GAMMA_MODE register to figure what mode we're in
> 
> v3: Program PREC_PAL_GC_MAX to clamp pixel values > 1.0
> 
>     Add documentation on how the Broadcast RGB property is affected by
>     CTM_MATRIX
> 
> v4: Update contributors
> 
> Signed-off-by: Shashank Sharma <shashank.sharma@intel.com>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Signed-off-by: Kumar, Kiran S <kiran.s.kumar@intel.com>
> Signed-off-by: Kausal Malladi <kausalmalladi@gmail.com>
> ---
>  Documentation/DocBook/gpu.tmpl       |   6 +-
>  drivers/gpu/drm/i915/i915_drv.c      |  24 ++-
>  drivers/gpu/drm/i915/i915_drv.h      |   9 +
>  drivers/gpu/drm/i915/i915_reg.h      |  22 +++
>  drivers/gpu/drm/i915/intel_color.c   | 367 ++++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_display.c |  22 ++-
>  drivers/gpu/drm/i915/intel_drv.h     |   6 +-
>  7 files changed, 396 insertions(+), 60 deletions(-)
> 
> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> index 7c49a92..78b8877 100644
> --- a/Documentation/DocBook/gpu.tmpl
> +++ b/Documentation/DocBook/gpu.tmpl
> @@ -2152,7 +2152,11 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >ENUM</td>
>  	<td valign="top" >{ "Automatic", "Full", "Limited 16:235" }</td>
>  	<td valign="top" >Connector</td>
> -	<td valign="top" >TBD</td>
> +	<td valign="top" >When this property is set to Limited 16:235
> +		and CTM_MATRIX is set, the hardware will be programmed with
> +		the result of the multiplication of CTM_MATRIX by the limited
> +		range matrix to ensure the pixels normaly in the range 0..1.0
> +		are remapped to the range 16/255..235/255.</td>
>  	</tr>
>  	<tr>
>  	<td valign="top" >“audio”</td>
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 44912ec..b65aa20 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -66,6 +66,9 @@ static struct drm_driver driver;
>  #define IVB_CURSOR_OFFSETS \
>  	.cursor_offsets = { CURSOR_A_OFFSET, IVB_CURSOR_B_OFFSET, IVB_CURSOR_C_OFFSET }
>  
> +#define BDW_COLORS \
> +	.color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }
> +
>  static const struct intel_device_info intel_i830_info = {
>  	.gen = 2, .is_mobile = 1, .cursor_needs_physical = 1, .num_pipes = 2,
>  	.has_overlay = 1, .overlay_needs_physical = 1,
> @@ -288,24 +291,28 @@ static const struct intel_device_info intel_haswell_m_info = {
>  	.is_mobile = 1,
>  };
>  
> +#define BDW_FEATURES \
> +	HSW_FEATURES, \
> +	BDW_COLORS
> +
>  static const struct intel_device_info intel_broadwell_d_info = {
> -	HSW_FEATURES,
> +	BDW_FEATURES,
>  	.gen = 8,
>  };
>  
>  static const struct intel_device_info intel_broadwell_m_info = {
> -	HSW_FEATURES,
> +	BDW_FEATURES,
>  	.gen = 8, .is_mobile = 1,
>  };
>  
>  static const struct intel_device_info intel_broadwell_gt3d_info = {
> -	HSW_FEATURES,
> +	BDW_FEATURES,
>  	.gen = 8,
>  	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>  };
>  
>  static const struct intel_device_info intel_broadwell_gt3m_info = {
> -	HSW_FEATURES,
> +	BDW_FEATURES,
>  	.gen = 8, .is_mobile = 1,
>  	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
>  };
> @@ -321,13 +328,13 @@ static const struct intel_device_info intel_cherryview_info = {
>  };
>  
>  static const struct intel_device_info intel_skylake_info = {
> -	HSW_FEATURES,
> +	BDW_FEATURES,
>  	.is_skylake = 1,
>  	.gen = 9,
>  };
>  
>  static const struct intel_device_info intel_skylake_gt3_info = {
> -	HSW_FEATURES,
> +	BDW_FEATURES,
>  	.is_skylake = 1,
>  	.gen = 9,
>  	.ring_mask = RENDER_RING | BSD_RING | BLT_RING | VEBOX_RING | BSD2_RING,
> @@ -345,17 +352,18 @@ static const struct intel_device_info intel_broxton_info = {
>  	.has_fbc = 1,
>  	GEN_DEFAULT_PIPEOFFSETS,
>  	IVB_CURSOR_OFFSETS,
> +	BDW_COLORS,
>  };
>  
>  static const struct intel_device_info intel_kabylake_info = {
> -	HSW_FEATURES,
> +	BDW_FEATURES,
>  	.is_preliminary = 1,
>  	.is_kabylake = 1,
>  	.gen = 9,
>  };
>  
>  static const struct intel_device_info intel_kabylake_gt3_info = {
> -	HSW_FEATURES,
> +	BDW_FEATURES,
>  	.is_preliminary = 1,
>  	.is_kabylake = 1,
>  	.gen = 9,
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8216665..c1ca4d0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -659,6 +659,10 @@ struct drm_i915_display_funcs {
>  	/* render clock increase/decrease */
>  	/* display clock increase/decrease */
>  	/* pll clock increase/decrease */
> +
> +	void (*load_degamma_lut)(struct drm_crtc *crtc);
> +	void (*load_csc_matrix)(struct drm_crtc *crtc);
> +	void (*load_gamma_lut)(struct drm_crtc *crtc);
>  };
>  
>  enum forcewake_domain_id {
> @@ -806,6 +810,11 @@ struct intel_device_info {
>  	u8 has_slice_pg:1;
>  	u8 has_subslice_pg:1;
>  	u8 has_eu_pg:1;
> +
> +	struct color_luts {
> +		u16 degamma_lut_size;
> +		u16 gamma_lut_size;
> +	} color;
>  };
>  
>  #undef DEFINE_FLAG
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index f8b0d6c..1dc5d3b 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -7663,6 +7663,28 @@ enum skl_disp_power_wells {
>  #define PIPE_CSC_POSTOFF_ME(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_ME, _PIPE_B_CSC_POSTOFF_ME, _PIPE_C_CSC_POSTOFF_ME)
>  #define PIPE_CSC_POSTOFF_LO(pipe)	_MMIO_PIPE3(pipe, _PIPE_A_CSC_POSTOFF_LO, _PIPE_B_CSC_POSTOFF_LO, _PIPE_C_CSC_POSTOFF_LO)
>  
> +/* pipe degamma/gamma LUTs on IVB+ */
> +#define _PAL_PREC_INDEX_A	0x4A400
> +#define _PAL_PREC_INDEX_B	0x4AC00
> +#define _PAL_PREC_INDEX_C	0x4B400
> +#define   PAL_PREC_10_12_BIT		(0 << 31)
> +#define   PAL_PREC_SPLIT_MODE		(1 << 31)
> +#define   PAL_PREC_AUTO_INCREMENT	(1 << 15)
> +#define _PAL_PREC_DATA_A	0x4A404
> +#define _PAL_PREC_DATA_B	0x4AC04
> +#define _PAL_PREC_DATA_C	0x4B404
> +#define _PAL_PREC_GC_MAX_A	0x4A410
> +#define _PAL_PREC_GC_MAX_B	0x4AC10
> +#define _PAL_PREC_GC_MAX_C	0x4B410
> +#define _PAL_PREC_EXT_GC_MAX_A	0x4A420
> +#define _PAL_PREC_EXT_GC_MAX_B	0x4AC20
> +#define _PAL_PREC_EXT_GC_MAX_C	0x4B420
> +
> +#define PREC_PAL_INDEX(pipe)		_MMIO_PIPE3(pipe, _PAL_PREC_INDEX_A, _PAL_PREC_INDEX_B, _PAL_PREC_INDEX_C)
> +#define PREC_PAL_DATA(pipe)		_MMIO_PIPE3(pipe, _PAL_PREC_DATA_A, _PAL_PREC_DATA_B, _PAL_PREC_DATA_C)
> +#define PREC_PAL_GC_MAX(pipe, i)	_MMIO(_PIPE3(pipe, _PAL_PREC_GC_MAX_A, _PAL_PREC_GC_MAX_B, _PAL_PREC_GC_MAX_C) + (i) * 4)
> +#define PREC_PAL_EXT_GC_MAX(pipe, i)	_MMIO(_PIPE3(pipe, _PAL_PREC_EXT_GC_MAX_A, _PAL_PREC_EXT_GC_MAX_B, _PAL_PREC_EXT_GC_MAX_C) + (i) * 4)
> +
>  /* MIPI DSI registers */
>  
>  #define _MIPI_PORT(port, a, c)	_PORT3(port, a, 0, c)	/* ports A and C only */
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 39ca215..782b9d8 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -24,44 +24,169 @@
>  
>  #include "intel_drv.h"
>  
> +#define CTM_COEFF_SIGN	(1ULL << 63)
> +
> +#define CTM_COEFF_1_0	(1ULL << 32)
> +#define CTM_COEFF_2_0	(CTM_COEFF_1_0 << 1)
> +#define CTM_COEFF_4_0	(CTM_COEFF_2_0 << 1)
> +#define CTM_COEFF_0_5	(CTM_COEFF_1_0 >> 1)
> +#define CTM_COEFF_0_25	(CTM_COEFF_0_5 >> 1)
> +#define CTM_COEFF_0_125	(CTM_COEFF_0_25 >> 1)
> +
> +#define CTM_COEFF_LIMITED_RANGE ((235ULL - 16ULL) * CTM_COEFF_1_0 / 255)
> +
> +#define CTM_COEFF_NEGATIVE(coeff)	(((coeff) & CTM_COEFF_SIGN) != 0)
> +#define CTM_COEFF_ABS(coeff)		((coeff) & (CTM_COEFF_SIGN - 1))
> +
> +/*
> + * Extract the CSC coefficient from a CTM coefficient (in U32.32 fixed point
> + * format). This macro takes the coefficient we want transformed and the
> + * number of fractional bits.
> + *
> + * We only have a 9 bits precision window which slides depending on the value
> + * of the CTM coefficient and we write the value from bit 3. We also round the
> + * value.
> + */
> +#define I9XX_CSC_COEFF_FP(coeff, fbits)	\
> +	(clamp_val(((coeff) >> (32 - (fbits) - 3)) + 4, 0, 0xfff) & 0xff8)
> +
> +#define I9XX_CSC_COEFF_LIMITED_RANGE	\
> +	I9XX_CSC_COEFF_FP(CTM_COEFF_LIMITED_RANGE, 9)
> +#define I9XX_CSC_COEFF_1_0		\
> +	((7 << 12) | I9XX_CSC_COEFF_FP(CTM_COEFF_1_0, 8))
> +
> +/*
> + * When using limited range, multiply the matrix given by userspace by
> + * the matrix that we would use for the limited range. We do the
> + * multiplication in U2.30 format.
> + */
> +static void ctm_matrix_mult_by_limited(uint64_t *result,
> +				       int64_t *input)
> +{
> +	int i, j;
> +	uint64_t limited_coeffs[9] = { CTM_COEFF_LIMITED_RANGE, 0, 0,
> +				       0, CTM_COEFF_LIMITED_RANGE, 0,
> +				       0, 0, CTM_COEFF_LIMITED_RANGE };
> +
> +	for (i = 0; i < ARRAY_SIZE(limited_coeffs); i++) {
> +		int column = i % 3, row = i / 3;
> +		int negative = 0;
> +
> +		input[i] = 0;

Maybe I'm not understanding the matrix math here, but why are we
clearing input[i]?  Was this supposed to be result[i] instead?

> +		for (j = 0; j < 3; j++) {
> +			int64_t user_coeff = input[j * 3 + column];
> +			uint64_t limited_coeff =
> +				limited_coeffs[row * 3 + j] >> 2;
> +			uint64_t abs_coeff =
> +				clamp_val(CTM_COEFF_ABS(user_coeff),
> +					  0,
> +					  CTM_COEFF_4_0 - 1) >> 2;
> +
> +			if (CTM_COEFF_NEGATIVE(user_coeff))
> +				negative = !negative;
> +			result[i] += limited_coeff * abs_coeff;
> +		}
> +
> +		result[i] >>= 27;
> +		if (negative)
> +			result[i] |= CTM_COEFF_SIGN;

Given that the limited_coeffs[] matrix is a diagonal matrix, we don't
actually need an inner loop here, right?  Would it be simpler to just
replace this with something more like the following?

        absuser_coeff = clamp_val(input[i]);
        limited_coeff = limited_coeffs[column * 3 + column] >> 2;
        result[i] = (absuser_coeff * limited_coeff) >> 27;
        if (CTM_COEFF_NEGATIVE(input[i]))
                result[i] |= CTM_COEFF_SIGN;

Technically you don't even need to actually store limited_coeffs[] in a
square matrix, although doing so does help clarify for the reader the
kind of matrix math you're performing.


> +	}
> +}
> +
>  /*
>   * Set up the pipe CSC unit.
>   *
> - * Currently only full range RGB to limited range RGB conversion
> - * is supported, but eventually this should handle various
> - * RGB<->YCbCr scenarios as well.
> + * Currently only full range RGB to limited range RGB conversion is supported,
> + * but eventually this should handle various RGB<->YCbCr scenarios as well.
>   */
>  static void i9xx_load_csc_matrix(struct drm_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->dev;
> +	struct drm_crtc_state *crtc_state = crtc->state;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	int pipe = intel_crtc->pipe;
> -	uint16_t coeff = 0x7800; /* 1.0 */
> +	int i, pipe = intel_crtc->pipe;
> +	uint16_t coeffs[9] = { 0, };
>  
> -	/*
> -	 * TODO: Check what kind of values actually come out of the pipe
> -	 * with these coeff/postoff values and adjust to get the best
> -	 * accuracy. Perhaps we even need to take the bpc value into
> -	 * consideration.
> -	 */
> +	if (crtc_state->ctm_matrix) {
> +		struct drm_color_ctm *ctm =
> +			(struct drm_color_ctm *)crtc_state->ctm_matrix->data;
> +		uint64_t input[9] = { 0, };
> +
> +		if (intel_crtc->config->limited_color_range)
> +			ctm_matrix_mult_by_limited(input, ctm->matrix);
> +		else {

Minor nitpick; kernel coding standard says we need to use braces on both
branches of an if/else if either of them needs it.


> +			for (i = 0; i < ARRAY_SIZE(input); i++)
> +				input[i] = ctm->matrix[i];
> +		}
>  
> -	if (intel_crtc->config->limited_color_range)
> -		coeff = ((235 - 16) * (1 << 12) / 255) & 0xff8; /* 0.xxx... */
> +
> +		/*
> +		 * Convert fixed point S31.32 input to format supported by the
> +		 * hardware.
> +		 */
> +		for (i = 0; i < ARRAY_SIZE(coeffs); i++) {
> +			uint64_t abs_coeff = ((1ULL << 63) - 1) & input[i];
> +
> +			/*
> +			 * Clamp input value to min/max supported by
> +			 * hardware.
> +			 */
> +			abs_coeff = clamp_val(abs_coeff, 0, CTM_COEFF_4_0 - 1);
> +
> +			/* sign bit */
> +			if (CTM_COEFF_NEGATIVE(input[i]))
> +				coeffs[i] |= 1 << 15;
> +
> +			if (abs_coeff < CTM_COEFF_0_125)
> +				coeffs[i] |= (3 << 12) |
> +					I9XX_CSC_COEFF_FP(abs_coeff, 12);
> +			else if (abs_coeff < CTM_COEFF_0_25)
> +				coeffs[i] |= (2 << 12) |
> +					I9XX_CSC_COEFF_FP(abs_coeff, 11);
> +			else if (abs_coeff < CTM_COEFF_0_5)
> +				coeffs[i] |= (1 << 12) |
> +					I9XX_CSC_COEFF_FP(abs_coeff, 10);
> +			else if (abs_coeff < CTM_COEFF_1_0)
> +				coeffs[i] |= I9XX_CSC_COEFF_FP(abs_coeff, 9);
> +			else if (abs_coeff < CTM_COEFF_2_0)
> +				coeffs[i] |= (7 << 12) |
> +					I9XX_CSC_COEFF_FP(abs_coeff, 8);
> +			else
> +				coeffs[i] |= (6 << 12) |
> +					I9XX_CSC_COEFF_FP(abs_coeff, 7);
> +		}
> +	} else {
> +		/*
> +		 * Load an identify matrix if no coefficients are provided.
> +		 *
> +		 * TODO: Check what kind of values actually come out of the
> +		 * pipe with these coeff/postoff values and adjust to get the
> +		 * best accuracy. Perhaps we even need to take the bpc value
> +		 * into consideration.
> +		 */
> +		for (i = 0; i < 3; i++) {
> +			if (intel_crtc->config->limited_color_range)
> +				coeffs[i * 3 + i] =
> +					I9XX_CSC_COEFF_LIMITED_RANGE;
> +			else
> +				coeffs[i * 3 + i] = I9XX_CSC_COEFF_1_0;
> +		}
> +	}
>  
>  	/*
>  	 * GY/GU and RY/RU should be the other way around according
>  	 * to BSpec, but reality doesn't agree. Just set them up in
>  	 * a way that results in the correct picture.
>  	 */

Is this comment still relevant/correct?  It looks to me like you're
programming the same way the bspec recommends now, at least as far as
the BDW-SKL section goes.

> -	I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeff << 16);
> -	I915_WRITE(PIPE_CSC_COEFF_BY(pipe), 0);
> +	I915_WRITE(PIPE_CSC_COEFF_RY_GY(pipe), coeffs[0] << 16 | coeffs[1]);
> +	I915_WRITE(PIPE_CSC_COEFF_BY(pipe), coeffs[2] << 16);
>  
> -	I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeff);
> -	I915_WRITE(PIPE_CSC_COEFF_BU(pipe), 0);
> +	I915_WRITE(PIPE_CSC_COEFF_RU_GU(pipe), coeffs[3] << 16 | coeffs[4]);
> +	I915_WRITE(PIPE_CSC_COEFF_BU(pipe), coeffs[5] << 16);
>  
> -	I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), 0);
> -	I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeff << 16);
> +	I915_WRITE(PIPE_CSC_COEFF_RV_GV(pipe), coeffs[6] << 16 | coeffs[7]);
> +	I915_WRITE(PIPE_CSC_COEFF_BV(pipe), coeffs[8] << 16);
>  
>  	I915_WRITE(PIPE_CSC_PREOFF_HI(pipe), 0);
>  	I915_WRITE(PIPE_CSC_PREOFF_ME(pipe), 0);
> @@ -88,21 +213,146 @@ static void i9xx_load_csc_matrix(struct drm_crtc *crtc)
>  	}
>  }
>  
> -void intel_color_set_csc(struct drm_crtc *crtc)
> +/** Loads the legacy palette/gamma unit for the CRTC with the prepared
> + * values.
> + */
> +static void i9xx_load_legacy_gamma_lut(struct drm_crtc *crtc)
>  {
> -	i9xx_load_csc_matrix(crtc);
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	struct intel_crtc_state *intel_state = to_intel_crtc_state(crtc->state);
> +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +	int i;
> +
> +	for (i = 0; i < 256; i++) {
> +		uint32_t word = (intel_crtc->lut_r[i] << 16) |
> +			(intel_crtc->lut_g[i] << 8) |
> +			intel_crtc->lut_b[i];
> +		if (HAS_GMCH_DISPLAY(dev))
> +			I915_WRITE(PALETTE(pipe, i), word);
> +		else
> +			I915_WRITE(LGC_PALETTE(pipe, i), word);
> +	}
> +
> +	intel_state->gamma_mode = GAMMA_MODE_MODE_8BIT;
> +	I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
>  }
>  
> -/** Loads the palette/gamma unit for the CRTC with the prepared values on  */
> -static void i9xx_load_legacy_gamma_lut(struct drm_crtc *crtc)
> +static void broadwell_load_degamma_lut(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_crtc_state *state = crtc->state;
> +	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
> +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +	uint32_t i, lut_size = INTEL_INFO(dev)->color.degamma_lut_size;
> +
> +	I915_WRITE(PREC_PAL_INDEX(pipe),
> +		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT);
> +
> +	if (state->degamma_lut) {
> +		struct drm_color_lut *lut =
> +			(struct drm_color_lut *) state->degamma_lut->data;
> +
> +		for (i = 0; i < lut_size; i++) {
> +			uint32_t word =
> +			drm_color_lut_extract(lut[i].red, 10) << 20 |
> +			drm_color_lut_extract(lut[i].green, 10) << 10 |
> +			drm_color_lut_extract(lut[i].blue, 10);
> +
> +			I915_WRITE(PREC_PAL_DATA(pipe), word);
> +		}
> +	} else {
> +		for (i = 0; i < lut_size; i++) {
> +			uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1);
> +
> +			I915_WRITE(PREC_PAL_DATA(pipe),
> +				   (v << 20) | (v << 10) | v);
> +		}
> +	}
> +
> +	intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT;
> +	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT);
> +	POSTING_READ(GAMMA_MODE(pipe));
> +
> +	/* Reset the index, otherwise it prevents the legacy palette to be
> +	 * written properly.
> +	 */
> +	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
> +}
> +
> +static void broadwell_load_gamma_lut(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct drm_crtc_state *state = crtc->state;
> +	struct intel_crtc_state *intel_state = to_intel_crtc_state(state);
> +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
> +	uint32_t i, lut_offset = INTEL_INFO(dev)->color.degamma_lut_size,
> +		lut_size = INTEL_INFO(dev)->color.gamma_lut_size;
> +
> +
> +	I915_WRITE(PREC_PAL_INDEX(pipe),
> +		   PAL_PREC_SPLIT_MODE | PAL_PREC_AUTO_INCREMENT | lut_offset);
> +
> +	if (state->gamma_lut) {
> +		struct drm_color_lut *lut =
> +			(struct drm_color_lut *) state->gamma_lut->data;
> +
> +		for (i = 0; i < lut_size; i++) {
> +			uint32_t word =
> +			(drm_color_lut_extract(lut[i].red, 10) << 20) |
> +			(drm_color_lut_extract(lut[i].green, 10) << 10) |
> +			drm_color_lut_extract(lut[i].blue, 10);
> +
> +			I915_WRITE(PREC_PAL_DATA(pipe), word);
> +		}
> +
> +		/* Program the max register to clamp values > 1.0. */
> +		I915_WRITE(PREC_PAL_GC_MAX(pipe, 0),
> +			   drm_color_lut_extract(lut[i].red, 16));
> +		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1),
> +			   drm_color_lut_extract(lut[i].green, 16));
> +		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2),
> +			   drm_color_lut_extract(lut[i].blue, 16));
> +	} else {
> +		for (i = 0; i < lut_size; i++) {
> +			uint32_t v = (i * ((1 << 10) - 1)) / (lut_size - 1);
> +
> +			I915_WRITE(PREC_PAL_DATA(pipe),
> +				   (v << 20) | (v << 10) | v);
> +		}
> +
> +		I915_WRITE(PREC_PAL_GC_MAX(pipe, 0), (1 << 16) - 1);
> +		I915_WRITE(PREC_PAL_GC_MAX(pipe, 1), (1 << 16) - 1);
> +		I915_WRITE(PREC_PAL_GC_MAX(pipe, 2), (1 << 16) - 1);
> +	}
> +
> +	intel_state->gamma_mode = GAMMA_MODE_MODE_SPLIT;
> +	I915_WRITE(GAMMA_MODE(pipe), GAMMA_MODE_MODE_SPLIT);
> +	POSTING_READ(GAMMA_MODE(pipe));
> +
> +	/* Reset the index, otherwise it prevents the legacy palette to be
> +	 * written properly.
> +	 */
> +	I915_WRITE(PREC_PAL_INDEX(pipe), 0);
> +}
> +
> +static void intel_color_load_luts_internal(struct drm_crtc *crtc,
> +					  bool legacy)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	enum pipe pipe = intel_crtc->pipe;
> -	int i;
> +	struct intel_crtc_state *intel_state = to_intel_crtc_state(crtc->state);
> +	enum pipe pipe = to_intel_crtc(crtc)->pipe;
>  	bool reenable_ips = false;
>  
> +	/* The clocks have to be on to load the palette. */
> +	if (!crtc->state->active)
> +		return;
> +
>  	if (HAS_GMCH_DISPLAY(dev)) {
>  		if (intel_crtc->config->has_dsi_encoder)
>  			assert_dsi_pll_enabled(dev_priv);
> @@ -114,42 +364,32 @@ static void i9xx_load_legacy_gamma_lut(struct drm_crtc *crtc)
>  	 * GAMMA_MODE is configured for split gamma and IPS_CTL has IPS enabled.
>  	 */
>  	if (IS_HASWELL(dev) && intel_crtc->config->ips_enabled &&
> -	    ((I915_READ(GAMMA_MODE(pipe)) & GAMMA_MODE_MODE_MASK) ==
> -	     GAMMA_MODE_MODE_SPLIT)) {
> +	    intel_state->gamma_mode == GAMMA_MODE_MODE_SPLIT) {
>  		hsw_disable_ips(intel_crtc);
>  		reenable_ips = true;
>  	}
>  
> -	for (i = 0; i < 256; i++) {
> -		uint32_t word = (intel_crtc->lut_r[i] << 16) |
> -			(intel_crtc->lut_g[i] << 8) |
> -			intel_crtc->lut_b[i];
> -		if (HAS_GMCH_DISPLAY(dev))
> -			I915_WRITE(PALETTE(pipe, i), word);
> -		else
> -			I915_WRITE(LGC_PALETTE(pipe, i), word);
> +	if (legacy)
> +		i9xx_load_legacy_gamma_lut(crtc);
> +	else {
> +		dev_priv->display.load_degamma_lut(crtc);
> +		dev_priv->display.load_gamma_lut(crtc);
>  	}
>  
> -	I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
> -
>  	if (reenable_ips)
>  		hsw_enable_ips(intel_crtc);
>  }
>  
> -void intel_color_load_gamma_lut(struct drm_crtc *crtc)
> +void intel_color_legacy_load_lut(struct drm_crtc *crtc)
>  {
> -	/* The clocks have to be on to load the palette. */
> -	if (!crtc->state->active)
> -		return;
> -
> -	i9xx_load_legacy_gamma_lut(crtc);
> +	intel_color_load_luts_internal(crtc, true);
>  }
>  
>  void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>  				  u16 *blue, uint32_t start, uint32_t size)
>  {
> -	int end = (start + size > 256) ? 256 : start + size, i;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	int end = (start + size > 256) ? 256 : start + size, i;
>  
>  	for (i = start; i < end; i++) {
>  		intel_crtc->lut_r[i] = red[i] >> 8;
> @@ -157,11 +397,29 @@ void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>  		intel_crtc->lut_b[i] = blue[i] >> 8;
>  	}
>  
> -	intel_color_load_gamma_lut(crtc);
> +	intel_color_load_luts_internal(crtc, true);
> +}
> +
> +void intel_color_load_luts(struct drm_crtc *crtc)
> +{
> +	intel_color_load_luts_internal(crtc,
> +				       !crtc->state->degamma_lut &&
> +				       !crtc->state->gamma_lut);
> +}
> +
> +void intel_color_set_csc(struct drm_crtc *crtc)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +
> +	if (dev_priv->display.load_csc_matrix)
> +		dev_priv->display.load_csc_matrix(crtc);
>  }
>  
>  void intel_color_init(struct drm_crtc *crtc)
>  {
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	int i;
>  
> @@ -171,4 +429,23 @@ void intel_color_init(struct drm_crtc *crtc)
>  		intel_crtc->lut_g[i] = i;
>  		intel_crtc->lut_b[i] = i;
>  	}
> +
> +	if (IS_BROADWELL(dev) || IS_SKYLAKE(dev) ||
> +	    IS_BROXTON(dev) || IS_KABYLAKE(dev)) {
> +		dev_priv->display.load_degamma_lut = broadwell_load_degamma_lut;
> +		dev_priv->display.load_gamma_lut = broadwell_load_gamma_lut;
> +		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
> +	} else {
> +		dev_priv->display.load_csc_matrix = i9xx_load_csc_matrix;
> +	}
> +
> +	if (INTEL_INFO(dev)->color.degamma_lut_size != 0 &&
> +	    INTEL_INFO(dev)->color.gamma_lut_size != 0) {
> +		WARN_ON(!dev_priv->display.load_degamma_lut ||
> +			!dev_priv->display.load_gamma_lut ||
> +			!dev_priv->display.load_csc_matrix);
> +		drm_helper_crtc_enable_color_mgmt(crtc,
> +					INTEL_INFO(dev)->color.degamma_lut_size,
> +					INTEL_INFO(dev)->color.gamma_lut_size);
> +	}
>  }
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index f10bba2..e38b31b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4864,7 +4864,7 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
>  	 * On ILK+ LUT must be loaded before the pipe is running but with
>  	 * clocks enabled
>  	 */
> -	intel_color_load_gamma_lut(crtc);
> +	intel_color_load_luts(crtc);
>  
>  	intel_update_watermarks(crtc);
>  	intel_enable_pipe(intel_crtc);
> @@ -4959,7 +4959,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  	 * On ILK+ LUT must be loaded before the pipe is running but with
>  	 * clocks enabled
>  	 */
> -	intel_color_load_gamma_lut(crtc);
> +	intel_color_load_luts(crtc);
>  
>  	intel_ddi_set_pipe_settings(crtc);
>  	if (!intel_crtc->config->has_dsi_encoder)
> @@ -6174,7 +6174,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
>  
>  	i9xx_pfit_enable(intel_crtc);
>  
> -	intel_color_load_gamma_lut(crtc);
> +	intel_color_load_luts(crtc);
>  
>  	intel_enable_pipe(intel_crtc);
>  
> @@ -6227,7 +6227,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
>  
>  	i9xx_pfit_enable(intel_crtc);
>  
> -	intel_color_load_gamma_lut(crtc);
> +	intel_color_load_luts(crtc);
>  
>  	intel_update_watermarks(crtc);
>  	intel_enable_pipe(intel_crtc);
> @@ -11898,7 +11898,7 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
>  
>  static const struct drm_crtc_helper_funcs intel_helper_funcs = {
>  	.mode_set_base_atomic = intel_pipe_set_base_atomic,
> -	.load_lut = intel_color_load_gamma_lut,
> +	.load_lut = intel_color_legacy_load_lut,
>  	.atomic_begin = intel_begin_crtc_commit,
>  	.atomic_flush = intel_finish_crtc_commit,
>  	.atomic_check = intel_crtc_atomic_check,
> @@ -13428,6 +13428,17 @@ static int intel_atomic_commit(struct drm_device *dev,
>  			hw_check = true;
>  		}
>  
> +		if (!modeset &&
> +		    crtc->state->active &&
> +		    crtc->state->color_mgmt_changed) {
> +			/* Only update color management when not doing

Minor nitpick; kernel coding style says the opening "/*" of multi-line
comments should be on a line by itself.

> +			 * a modeset as this will be done by
> +			 * crtc_enable already.
> +			 */
> +			intel_color_set_csc(crtc);
> +			intel_color_load_luts(crtc);
> +		}
> +
>  		if (!modeset)
>  			intel_pre_plane_update(to_intel_crtc_state(crtc_state));
>  
> @@ -13519,6 +13530,7 @@ out:
>  static const struct drm_crtc_funcs intel_crtc_funcs = {
>  	.gamma_set = intel_color_legacy_gamma_set,
>  	.set_config = drm_atomic_helper_set_config,
> +	.set_property = drm_atomic_helper_crtc_set_property,
>  	.destroy = intel_crtc_destroy,
>  	.page_flip = intel_crtc_page_flip,
>  	.atomic_duplicate_state = intel_crtc_duplicate_state,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 2b5d03a..c384c78 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -507,6 +507,8 @@ struct intel_crtc_state {
>  	/* IVB sprite scaling w/a (WaCxSRDisabledForSpriteScaling:ivb) */
>  	bool disable_lp_wm;
>  
> +	uint32_t gamma_mode;
> +
>  	struct {
>  		/*
>  		 * optimal watermarks, programmed post-vblank when this state
> @@ -1620,9 +1622,11 @@ extern const struct drm_plane_helper_funcs intel_plane_helper_funcs;
>  
>  /* intel_color.c */
>  void intel_color_init(struct drm_crtc *crtc);
> +void intel_color_update(struct drm_crtc *crtc);
>  void intel_color_set_csc(struct drm_crtc *crtc);
> -void intel_color_load_gamma_lut(struct drm_crtc *crtc);
> +void intel_color_load_luts(struct drm_crtc *crtc);
>  void intel_color_legacy_gamma_set(struct drm_crtc *crtc, u16 *red, u16 *green,
>  				  u16 *blue, uint32_t start, uint32_t size);
> +void intel_color_legacy_load_lut(struct drm_crtc *crtc);

intel_color_load_gamma_lut() was a rename from patch #1 of the series.
Maybe we should squash this name change back into that patch, even
though we only have a single LUT at that point?  I guess it doesn't
really matter too much either way.


Matt

>  
>  #endif /* __INTEL_DRV_H__ */
> -- 
> 2.7.0
> 
> _______________________________________________
> 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

[PATCH 2/6] drm: introduce color correction properties

[Lionel Landwerlin]

Patch based on a previous serie by Shashank Sharma.

v2: Register LUT size properties as range

v3: Fix round in drm_color_lut_get_value() helper
    More docs on how degamma/gamma properties are used

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 Documentation/DocBook/gpu.tmpl      |  58 +++++++++++++++++++-
 drivers/gpu/drm/drm_atomic.c        | 102 +++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/drm_atomic_helper.c |  10 ++++
 drivers/gpu/drm/drm_crtc.c          |  35 +++++++++++++
 drivers/gpu/drm/drm_crtc_helper.c   |  33 ++++++++++++
 include/drm/drm_crtc.h              |  44 +++++++++++++++-
 include/drm/drm_crtc_helper.h       |   3 ++
 include/uapi/drm/drm_mode.h         |  15 ++++++
 8 files changed, 295 insertions(+), 5 deletions(-)

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 49c9791..637e718 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -1816,7 +1816,7 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >Description/Restrictions</td>
 	</tr>
 	<tr>
-	<td rowspan="37" valign="top" >DRM</td>
+	<td rowspan="42" valign="top" >DRM</td>
 	<td valign="top" >Generic</td>
 	<td valign="top" >“rotation”</td>
 	<td valign="top" >BITMASK</td>
@@ -2068,7 +2068,7 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >property to suggest an Y offset for a connector</td>
 	</tr>
 	<tr>
-	<td rowspan="3" valign="top" >Optional</td>
+	<td rowspan="8" valign="top" >Optional</td>
 	<td valign="top" >“scaling mode”</td>
 	<td valign="top" >ENUM</td>
 	<td valign="top" >{ "None", "Full", "Center", "Full aspect" }</td>
@@ -2092,6 +2092,60 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
+	<td valign="top" >“DEGAMMA_LUT”</td>
+	<td valign="top" >BLOB</td>
+	<td valign="top" >0</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to set the degamma LUT mapping
+		pixel data from the framebuffer before it is given to the
+		transformation matrix. The data is an interpreted as an array
+		of struct drm_color_lut elements. Hardware might choose not to
+		use the full precision of the LUT elements nor use all the
+		elements of the LUT (for example the hardware might choose to
+		interpolate between LUT[0] and LUT[4]). </td>
+	</tr>
+	<tr>
+	<td valign="top" >“DEGAMMA_LUT_SIZE”</td>
+	<td valign="top" >RANGE | IMMUTABLE</td>
+	<td valign="top" >Min=0, Max=UINT_MAX</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to gives the size of the LUT to
+		be set on the DEGAMMA_LUT property (the size depends on the
+		underlying hardware).</td>
+	</tr>
+	<tr>
+	<td valign="top" >“CTM_MATRIX”</td>
+	<td valign="top" >BLOB</td>
+	<td valign="top" >0</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to set the transformation
+		matrix apply to pixel data after the lookup through the
+		degamma LUT and before the lookup through the gamma LUT. The
+		data is an interpreted as a struct drm_color_ctm.</td>
+	</tr>
+	<tr>
+	<td valign="top" >“GAMMA_LUT”</td>
+	<td valign="top" >BLOB</td>
+	<td valign="top" >0</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to set the gamma LUT mapping
+		pixel data after to the transformation matrix to data sent to
+		the connector. The data is an interpreted as an array
+		of struct drm_color_lut elements. Hardware might choose not to
+		use the full precision of the LUT elements nor use all the
+		elements of the LUT (for example the hardware might choose to
+		interpolate between LUT[0] and LUT[4]).</td>
+	</tr>
+	<tr>
+	<td valign="top" >“GAMMA_LUT_SIZE”</td>
+	<td valign="top" >RANGE | IMMUTABLE</td>
+	<td valign="top" >Min=0, Max=UINT_MAX</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to gives the size of the LUT to
+		be set on the GAMMA_LUT property (the size depends on the
+		underlying hardware).</td>
+	</tr>
+	<tr>
 	<td rowspan="20" valign="top" >i915</td>
 	<td rowspan="2" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 8fb469c..7507762 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -28,6 +28,7 @@
 
 #include <drm/drmP.h>
 #include <drm/drm_atomic.h>
+#include <drm/drm_mode.h>
 #include <drm/drm_plane_helper.h>
 
 /**
@@ -388,6 +389,58 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
 EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
 
 /**
+ * drm_atomic_replace_property_blob - replace a blob property
+ * @blob: a pointer to the member blob to be replaced
+ * @new_blob: the new blob to replace with
+ * @expected_size: the expected size of the new blob
+ * @replaced: whether the blob has been replaced
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+static int
+drm_atomic_replace_property_blob(struct drm_property_blob **blob,
+				 struct drm_property_blob *new_blob,
+				 size_t expected_size,
+				 bool *replaced)
+{
+	struct drm_property_blob *old_blob = *blob;
+
+	if (old_blob == new_blob)
+		return 0;
+
+	if (new_blob != NULL && new_blob->length != expected_size)
+		return -EINVAL;
+
+	if (old_blob != NULL)
+		drm_property_unreference_blob(old_blob);
+	*blob = new_blob;
+	*replaced = true;
+
+	return 0;
+}
+
+static int
+drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc,
+					 struct drm_property_blob **blob,
+					 uint64_t blob_id,
+					 size_t expected_size,
+					 bool *replaced)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_property_blob *new_blob = NULL;
+
+	if (blob_id != 0) {
+		new_blob = drm_property_lookup_blob(dev, blob_id);
+		if (new_blob == NULL)
+			return -EINVAL;
+	}
+
+	return drm_atomic_replace_property_blob(blob, new_blob,
+						expected_size, replaced);
+}
+
+/**
  * drm_atomic_crtc_set_property - set property on CRTC
  * @crtc: the drm CRTC to set a property on
  * @state: the state object to update with the new property value
@@ -419,8 +472,47 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
 		drm_property_unreference_blob(mode);
 		return ret;
-	}
-	else if (crtc->funcs->atomic_set_property)
+	} else if (property == config->degamma_lut_property) {
+		bool replaced = false;
+		uint64_t lut_size = 0;
+
+		ret = drm_object_property_get_value(&crtc->base,
+					config->degamma_lut_size_property,
+					&lut_size);
+		if (ret == 0)
+			ret = drm_atomic_replace_property_blob_from_id(crtc,
+					&state->degamma_lut,
+					val,
+					lut_size * sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed = replaced;
+		return ret;
+	} else if (property == config->ctm_matrix_property) {
+		bool replaced = false;
+
+		ret = drm_atomic_replace_property_blob_from_id(crtc,
+					&state->ctm_matrix,
+					val,
+					sizeof(struct drm_color_ctm),
+							       &replaced);
+		state->color_mgmt_changed = replaced;
+		return ret;
+	} else if (property == config->gamma_lut_property) {
+		bool replaced = false;
+		uint64_t lut_size = 0;
+
+		ret = drm_object_property_get_value(&crtc->base,
+					config->gamma_lut_size_property,
+					&lut_size);
+		if (ret == 0)
+			ret = drm_atomic_replace_property_blob_from_id(crtc,
+					&state->gamma_lut,
+					val,
+					lut_size * sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed = replaced;
+		return ret;
+	} else if (crtc->funcs->atomic_set_property)
 		return crtc->funcs->atomic_set_property(crtc, state, property, val);
 	else
 		return -EINVAL;
@@ -456,6 +548,12 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
 		*val = state->active;
 	else if (property == config->prop_mode_id)
 		*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
+	else if (property == config->degamma_lut_property)
+		*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
+	else if (property == config->ctm_matrix_property)
+		*val = (state->ctm_matrix) ? state->ctm_matrix->base.id : 0;
+	else if (property == config->gamma_lut_property)
+		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
 	else if (crtc->funcs->atomic_get_property)
 		return crtc->funcs->atomic_get_property(crtc, state, property, val);
 	else
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 0ab7c24..e4dc4da 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2470,10 +2470,17 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
 
 	if (state->mode_blob)
 		drm_property_reference_blob(state->mode_blob);
+	if (state->degamma_lut)
+		drm_property_reference_blob(state->degamma_lut);
+	if (state->ctm_matrix)
+		drm_property_reference_blob(state->ctm_matrix);
+	if (state->gamma_lut)
+		drm_property_reference_blob(state->gamma_lut);
 	state->mode_changed = false;
 	state->active_changed = false;
 	state->planes_changed = false;
 	state->connectors_changed = false;
+	state->color_mgmt_changed = false;
 	state->event = NULL;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
@@ -2514,6 +2521,9 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
 					    struct drm_crtc_state *state)
 {
 	drm_property_unreference_blob(state->mode_blob);
+	drm_property_unreference_blob(state->degamma_lut);
+	drm_property_unreference_blob(state->ctm_matrix);
+	drm_property_unreference_blob(state->gamma_lut);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
 
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 6e6514e..3c9cde5 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1542,6 +1542,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.prop_mode_id = prop;
 
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"DEGAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.degamma_lut_property = prop;
+
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"DEGAMMA_LUT_SIZE", 0, UINT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.degamma_lut_size_property = prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"CTM_MATRIX", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.ctm_matrix_property = prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"GAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.gamma_lut_property = prop;
+
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"GAMMA_LUT_SIZE", 0, UINT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.gamma_lut_size_property = prop;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 9f8b894..3cd8cbb 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -1073,3 +1073,36 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	return drm_plane_helper_commit(plane, plane_state, old_fb);
 }
 EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
+
+/**
+ * drm_helper_crtc_enable_color_mgmt - enable color management properties
+ * @crtc: DRM CRTC
+ * @degamma_lut_size: the size of the degamma lut (before CSC)
+ * @gamma_lut_size: the size of the gamma lut (after CSC)
+ *
+ * This function lets the driver enable the color correction properties on a
+ * CRTC. This includes 3 degamma, csc and gamma properties that userspace can
+ * set and 2 size properties to inform the userspace of the lut sizes.
+ */
+void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
+				       int degamma_lut_size,
+				       int gamma_lut_size)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+
+	drm_object_attach_property(&crtc->base,
+				   config->degamma_lut_property, 0);
+	drm_object_attach_property(&crtc->base,
+				   config->ctm_matrix_property, 0);
+	drm_object_attach_property(&crtc->base,
+				   config->gamma_lut_property, 0);
+
+	drm_object_attach_property(&crtc->base,
+				   config->degamma_lut_size_property,
+				   degamma_lut_size);
+	drm_object_attach_property(&crtc->base,
+				   config->gamma_lut_size_property,
+				   gamma_lut_size);
+}
+EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index c65a212..25a53088 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -305,12 +305,19 @@ struct drm_plane_helper_funcs;
  * @mode_changed: crtc_state->mode or crtc_state->enable has been changed
  * @active_changed: crtc_state->active has been toggled.
  * @connectors_changed: connectors to this crtc have been updated
+ * @color_mgmt_changed: color management properties have changed (degamma or
+ *	gamma LUT or CSC matrix)
  * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
  * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors
  * @last_vblank_count: for helpers and drivers to capture the vblank of the
  * 	update to ensure framebuffer cleanup isn't done too early
  * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
  * @mode: current mode timings
+ * @degamma_lut: Lookup table for converting framebuffer pixel data
+ *	before apply the conversion matrix
+ * @ctm_matrix: Transformation matrix
+ * @gamma_lut: Lookup table for converting pixel data after the
+ *	conversion matrix
  * @event: optional pointer to a DRM event to signal upon completion of the
  * 	state update
  * @state: backpointer to global drm_atomic_state
@@ -332,6 +339,7 @@ struct drm_crtc_state {
 	bool mode_changed : 1;
 	bool active_changed : 1;
 	bool connectors_changed : 1;
+	bool color_mgmt_changed : 1;
 
 	/* attached planes bitmask:
 	 * WARNING: transitional helpers do not maintain plane_mask so
@@ -353,6 +361,11 @@ struct drm_crtc_state {
 	/* blob property to expose current mode to atomic userspace */
 	struct drm_property_blob *mode_blob;
 
+	/* blob property to expose color management to userspace */
+	struct drm_property_blob *degamma_lut;
+	struct drm_property_blob *ctm_matrix;
+	struct drm_property_blob *gamma_lut;
+
 	struct drm_pending_vblank_event *event;
 
 	struct drm_atomic_state *state;
@@ -755,7 +768,7 @@ struct drm_crtc {
 	int x, y;
 	const struct drm_crtc_funcs *funcs;
 
-	/* CRTC gamma size for reporting to userspace */
+	/* Legacy FB CRTC gamma size for reporting to userspace */
 	uint32_t gamma_size;
 	uint16_t *gamma_store;
 
@@ -2023,6 +2036,15 @@ struct drm_mode_config_funcs {
  * @property_blob_list: list of all the blob property objects
  * @blob_lock: mutex for blob property allocation and management
  * @*_property: core property tracking
+ * @degamma_lut_property: LUT used to convert the framebuffer's colors to linear
+ *	gamma
+ * @degamma_lut_size_property: size of the degamma LUT as supported by the
+ *	driver (read-only)
+ * @ctm_matrix_property: Matrix used to convert colors after the lookup in the
+ *	degamma LUT
+ * @gamma_lut_property: LUT used to convert the colors, after the CSC matrix, to
+ *	the gamma space of the connected screen (read-only)
+ * @gamma_lut_size_property: size of the gamma LUT as supported by the driver
  * @preferred_depth: preferred RBG pixel depth, used by fb helpers
  * @prefer_shadow: hint to userspace to prefer shadow-fb rendering
  * @async_page_flip: does this device support async flips on the primary plane?
@@ -2124,6 +2146,13 @@ struct drm_mode_config {
 	struct drm_property *aspect_ratio_property;
 	struct drm_property *dirty_info_property;
 
+	/* Optional color correction properties */
+	struct drm_property *degamma_lut_property;
+	struct drm_property *degamma_lut_size_property;
+	struct drm_property *ctm_matrix_property;
+	struct drm_property *gamma_lut_property;
+	struct drm_property *gamma_lut_size_property;
+
 	/* properties for virtual machine layout */
 	struct drm_property *suggested_x_property;
 	struct drm_property *suggested_y_property;
@@ -2530,6 +2559,19 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
 	return mo ? obj_to_property(mo) : NULL;
 }
 
+/*
+ * Extract a degamma/gamma LUT value provided by user and round it to the
+ * precision supported by the hardware.
+ */
+static inline uint32_t drm_color_lut_extract(uint32_t user_input,
+					     uint32_t bit_precision)
+{
+	uint32_t val = ((user_input << 1) + 1) >> (16 - bit_precision + 1);
+	uint32_t max = 0xffff >> (16 - bit_precision);
+
+	return clamp_val(val, 0, max);
+}
+
 /* Plane list iterator for legacy (overlay only) planes. */
 #define drm_for_each_legacy_plane(plane, dev) \
 	list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 4b37afa..97fa894 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -48,6 +48,9 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
 				     struct drm_display_mode *mode,
 				     int x, int y,
 				     struct drm_framebuffer *old_fb);
+extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
+					      int degamma_lut_size,
+					      int gamma_lut_size);
 extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc);
 extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
 
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 50adb46..c021743 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -487,6 +487,21 @@ struct drm_mode_crtc_lut {
 	__u64 blue;
 };
 
+struct drm_color_ctm {
+	/* Conversion matrix in S31.32 format. */
+	__s64 matrix[9];
+};
+
+struct drm_color_lut {
+	/*
+	 * Data is U0.16 fixed point format.
+	 */
+	__u16 red;
+	__u16 green;
+	__u16 blue;
+	__u16 reserved;
+};
+
 #define DRM_MODE_PAGE_FLIP_EVENT 0x01
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT|DRM_MODE_PAGE_FLIP_ASYNC)
-- 
2.7.0

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

Re: [PATCH 2/6] drm: introduce color correction properties

[Matt Roper]

On Mon, Jan 25, 2016 at 10:57:51AM +0000, Daniel Stone wrote:
> Hi,
> 
> On 22 January 2016 at 21:20, Matt Roper <matthew.d.roper@intel.com> wrote:
> > Probably should have noticed/commented on this on your previous
> > iteration, but should we also restrict these new properties to be
> > atomic-only?  I thought there was a consensus a while back that new
> > functionality would only be exposed for atomic-ready userspace.  That
> > would mean also adding DRM_MODE_PROP_ATOMIC to your flags at creation.
> > Sorry for not noticing this when I commented before!
> 
> I don't really see a reason to mandate atomic for these. They'll still
> work with normal prop sets, every bit as well/badly as pre-atomic has
> always worked.
> 

Yeah, I agree that these could still work for legacy.  But I thought the
goal was to make new common properties atomic-only to help push atomic
as the API of the future and also to reduce the possibility of new
kernel properties confusing any poorly-written existing userspace.

I thought danvet had written something about that a while back, but I
could be completely misremembering.  If the consensus is that exposing
these for non-atomic is okay, then I don't have any serious objections
to that.

Thanks.


Matt

> Cheers,
> Daniel

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

Re: [PATCH 2/6] drm: introduce color correction properties

[Daniel Stone]

Hi,

On 22 January 2016 at 21:20, Matt Roper <matthew.d.roper@intel.com> wrote:
> Probably should have noticed/commented on this on your previous
> iteration, but should we also restrict these new properties to be
> atomic-only?  I thought there was a consensus a while back that new
> functionality would only be exposed for atomic-ready userspace.  That
> would mean also adding DRM_MODE_PROP_ATOMIC to your flags at creation.
> Sorry for not noticing this when I commented before!

I don't really see a reason to mandate atomic for these. They'll still
work with normal prop sets, every bit as well/badly as pre-atomic has
always worked.

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/6] drm: introduce color correction properties

[Matt Roper]

On Fri, Jan 22, 2016 at 12:12:08PM +0000, Lionel Landwerlin wrote:
> v2: Register LUT size properties as range
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

Probably should have noticed/commented on this on your previous
iteration, but should we also restrict these new properties to be
atomic-only?  I thought there was a consensus a while back that new
functionality would only be exposed for atomic-ready userspace.  That
would mean also adding DRM_MODE_PROP_ATOMIC to your flags at creation.
Sorry for not noticing this when I commented before!

Also, you probably want to add a Cc: dri-devel to this patch since it
deals with the DRM core and isn't i915-specific.


Matt

> ---
>  Documentation/DocBook/gpu.tmpl      |  50 +++++++++++++++++-
>  drivers/gpu/drm/drm_atomic.c        | 102 +++++++++++++++++++++++++++++++++++-
>  drivers/gpu/drm/drm_atomic_helper.c |  10 ++++
>  drivers/gpu/drm/drm_crtc.c          |  35 +++++++++++++
>  drivers/gpu/drm/drm_crtc_helper.c   |  33 ++++++++++++
>  include/drm/drm_crtc.h              |  43 ++++++++++++++-
>  include/drm/drm_crtc_helper.h       |   3 ++
>  include/uapi/drm/drm_mode.h         |  15 ++++++
>  8 files changed, 287 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> index 351e801..c17ac29 100644
> --- a/Documentation/DocBook/gpu.tmpl
> +++ b/Documentation/DocBook/gpu.tmpl
> @@ -2068,7 +2068,7 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >property to suggest an Y offset for a connector</td>
>  	</tr>
>  	<tr>
> -	<td rowspan="3" valign="top" >Optional</td>
> +	<td rowspan="8" valign="top" >Optional</td>
>  	<td valign="top" >“scaling mode”</td>
>  	<td valign="top" >ENUM</td>
>  	<td valign="top" >{ "None", "Full", "Center", "Full aspect" }</td>
> @@ -2092,6 +2092,54 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> +	<td valign="top" >“DEGAMMA_LUT”</td>
> +	<td valign="top" >BLOB</td>
> +	<td valign="top" >0</td>
> +	<td valign="top" >CRTC</td>
> +	<td valign="top" >DRM property to set the degamma LUT mapping
> +		pixel data from the framebuffer before it is given to the
> +		transformation matrix. The data is an interpreted as an array
> +		of struct drm_color_lut elements.</td>
> +	</tr>
> +	<tr>
> +	<td valign="top" >“DEGAMMA_LUT_SIZE”</td>
> +	<td valign="top" >RANGE | IMMUTABLE</td>
> +	<td valign="top" >Min=0, Max=UINT_MAX</td>
> +	<td valign="top" >CRTC</td>
> +	<td valign="top" >DRM property to gives the size of the LUT to
> +		be set on the DEGAMMA_LUT property (the size depends on the
> +		underlying hardware).</td>
> +	</tr>
> +	<tr>
> +	<td valign="top" >“CTM_MATRIX”</td>
> +	<td valign="top" >BLOB</td>
> +	<td valign="top" >0</td>
> +	<td valign="top" >CRTC</td>
> +	<td valign="top" >DRM property to set the transformation
> +		matrix apply to pixel data after the lookup through the
> +		degamma LUT and before the lookup through the gamma LUT. The
> +		data is an interpreted as a struct drm_color_ctm.</td>
> +	</tr>
> +	<tr>
> +	<td valign="top" >“GAMMA_LUT”</td>
> +	<td valign="top" >BLOB</td>
> +	<td valign="top" >0</td>
> +	<td valign="top" >CRTC</td>
> +	<td valign="top" >DRM property to set the gamma LUT mapping
> +		pixel data after to the transformation matrix to data sent to
> +		the connector. The data is an interpreted as an array
> +		of struct drm_color_lut elements.</td>
> +	</tr>
> +	<tr>
> +	<td valign="top" >“GAMMA_LUT_SIZE”</td>
> +	<td valign="top" >RANGE | IMMUTABLE</td>
> +	<td valign="top" >Min=0, Max=UINT_MAX</td>
> +	<td valign="top" >CRTC</td>
> +	<td valign="top" >DRM property to gives the size of the LUT to
> +		be set on the GAMMA_LUT property (the size depends on the
> +		underlying hardware).</td>
> +	</tr>
> +	<tr>
>  	<td rowspan="20" valign="top" >i915</td>
>  	<td rowspan="2" valign="top" >Generic</td>
>  	<td valign="top" >"Broadcast RGB"</td>
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 3f74193..5287416 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -28,6 +28,7 @@
>  
>  #include <drm/drmP.h>
>  #include <drm/drm_atomic.h>
> +#include <drm/drm_mode.h>
>  #include <drm/drm_plane_helper.h>
>  
>  /**
> @@ -388,6 +389,58 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
>  EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
>  
>  /**
> + * drm_atomic_replace_property_blob - replace a blob property
> + * @blob: a pointer to the member blob to be replaced
> + * @new_blob: the new blob to replace with
> + * @expected_size: the expected size of the new blob
> + * @replaced: whether the blob has been replaced
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +static int
> +drm_atomic_replace_property_blob(struct drm_property_blob **blob,
> +				 struct drm_property_blob *new_blob,
> +				 size_t expected_size,
> +				 bool *replaced)
> +{
> +	struct drm_property_blob *old_blob = *blob;
> +
> +	if (old_blob == new_blob)
> +		return 0;
> +
> +	if (new_blob != NULL && new_blob->length != expected_size)
> +		return -EINVAL;
> +
> +	if (old_blob != NULL)
> +		drm_property_unreference_blob(old_blob);
> +	*blob = new_blob;
> +	*replaced = true;
> +
> +	return 0;
> +}
> +
> +static int
> +drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc,
> +					 struct drm_property_blob **blob,
> +					 uint64_t blob_id,
> +					 size_t expected_size,
> +					 bool *replaced)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_property_blob *new_blob = NULL;
> +
> +	if (blob_id != 0) {
> +		new_blob = drm_property_lookup_blob(dev, blob_id);
> +		if (new_blob == NULL)
> +			return -EINVAL;
> +	}
> +
> +	return drm_atomic_replace_property_blob(blob, new_blob,
> +						expected_size, replaced);
> +}
> +
> +/**
>   * drm_atomic_crtc_set_property - set property on CRTC
>   * @crtc: the drm CRTC to set a property on
>   * @state: the state object to update with the new property value
> @@ -419,8 +472,47 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
>  		ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
>  		drm_property_unreference_blob(mode);
>  		return ret;
> -	}
> -	else if (crtc->funcs->atomic_set_property)
> +	} else if (property == config->degamma_lut_property) {
> +		bool replaced = false;
> +		uint64_t lut_size = 0;
> +
> +		ret = drm_object_property_get_value(&crtc->base,
> +					config->degamma_lut_size_property,
> +					&lut_size);
> +		if (ret == 0)
> +			ret = drm_atomic_replace_property_blob_from_id(crtc,
> +					&state->degamma_lut,
> +					val,
> +					lut_size * sizeof(struct drm_color_lut),
> +					&replaced);
> +		state->color_mgmt_changed = replaced;
> +		return ret;
> +	} else if (property == config->ctm_matrix_property) {
> +		bool replaced = false;
> +
> +		ret = drm_atomic_replace_property_blob_from_id(crtc,
> +					&state->ctm_matrix,
> +					val,
> +					sizeof(struct drm_color_ctm),
> +							       &replaced);
> +		state->color_mgmt_changed = replaced;
> +		return ret;
> +	} else if (property == config->gamma_lut_property) {
> +		bool replaced = false;
> +		uint64_t lut_size = 0;
> +
> +		ret = drm_object_property_get_value(&crtc->base,
> +					config->gamma_lut_size_property,
> +					&lut_size);
> +		if (ret == 0)
> +			ret = drm_atomic_replace_property_blob_from_id(crtc,
> +					&state->gamma_lut,
> +					val,
> +					lut_size * sizeof(struct drm_color_lut),
> +					&replaced);
> +		state->color_mgmt_changed = replaced;
> +		return ret;
> +	} else if (crtc->funcs->atomic_set_property)
>  		return crtc->funcs->atomic_set_property(crtc, state, property, val);
>  	else
>  		return -EINVAL;
> @@ -456,6 +548,12 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
>  		*val = state->active;
>  	else if (property == config->prop_mode_id)
>  		*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
> +	else if (property == config->degamma_lut_property)
> +		*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
> +	else if (property == config->ctm_matrix_property)
> +		*val = (state->ctm_matrix) ? state->ctm_matrix->base.id : 0;
> +	else if (property == config->gamma_lut_property)
> +		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>  	else if (crtc->funcs->atomic_get_property)
>  		return crtc->funcs->atomic_get_property(crtc, state, property, val);
>  	else
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index f0c3984..e4a5755 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -2454,10 +2454,17 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
>  
>  	if (state->mode_blob)
>  		drm_property_reference_blob(state->mode_blob);
> +	if (state->degamma_lut)
> +		drm_property_reference_blob(state->degamma_lut);
> +	if (state->ctm_matrix)
> +		drm_property_reference_blob(state->ctm_matrix);
> +	if (state->gamma_lut)
> +		drm_property_reference_blob(state->gamma_lut);
>  	state->mode_changed = false;
>  	state->active_changed = false;
>  	state->planes_changed = false;
>  	state->connectors_changed = false;
> +	state->color_mgmt_changed = false;
>  	state->event = NULL;
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
> @@ -2498,6 +2505,9 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
>  					    struct drm_crtc_state *state)
>  {
>  	drm_property_unreference_blob(state->mode_blob);
> +	drm_property_unreference_blob(state->degamma_lut);
> +	drm_property_unreference_blob(state->ctm_matrix);
> +	drm_property_unreference_blob(state->gamma_lut);
>  }
>  EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
>  
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index d40bab2..b1aa38a 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1542,6 +1542,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>  		return -ENOMEM;
>  	dev->mode_config.prop_mode_id = prop;
>  
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"DEGAMMA_LUT", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.degamma_lut_property = prop;
> +
> +	prop = drm_property_create_range(dev,
> +			DRM_MODE_PROP_IMMUTABLE,
> +			"DEGAMMA_LUT_SIZE", 0, UINT_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.degamma_lut_size_property = prop;
> +
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"CTM_MATRIX", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.ctm_matrix_property = prop;
> +
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"GAMMA_LUT", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.gamma_lut_property = prop;
> +
> +	prop = drm_property_create_range(dev,
> +			DRM_MODE_PROP_IMMUTABLE,
> +			"GAMMA_LUT_SIZE", 0, UINT_MAX);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.gamma_lut_size_property = prop;
> +
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
> index 5d4bc64..b9b1929 100644
> --- a/drivers/gpu/drm/drm_crtc_helper.c
> +++ b/drivers/gpu/drm/drm_crtc_helper.c
> @@ -1064,3 +1064,36 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>  	return drm_plane_helper_commit(plane, plane_state, old_fb);
>  }
>  EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
> +
> +/**
> + * drm_helper_crtc_enable_color_mgmt - enable color management properties
> + * @crtc: DRM CRTC
> + * @degamma_lut_size: the size of the degamma lut (before CSC)
> + * @gamma_lut_size: the size of the gamma lut (after CSC)
> + *
> + * This function lets the driver enable the color correction properties on a
> + * CRTC. This includes 3 degamma, csc and gamma properties that userspace can
> + * set and 2 size properties to inform the userspace of the lut sizes.
> + */
> +void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> +				       int degamma_lut_size,
> +				       int gamma_lut_size)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_mode_config *config = &dev->mode_config;
> +
> +	drm_object_attach_property(&crtc->base,
> +				   config->degamma_lut_property, 0);
> +	drm_object_attach_property(&crtc->base,
> +				   config->ctm_matrix_property, 0);
> +	drm_object_attach_property(&crtc->base,
> +				   config->gamma_lut_property, 0);
> +
> +	drm_object_attach_property(&crtc->base,
> +				   config->degamma_lut_size_property,
> +				   degamma_lut_size);
> +	drm_object_attach_property(&crtc->base,
> +				   config->gamma_lut_size_property,
> +				   gamma_lut_size);
> +}
> +EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index c65a212..967d646 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -305,12 +305,19 @@ struct drm_plane_helper_funcs;
>   * @mode_changed: crtc_state->mode or crtc_state->enable has been changed
>   * @active_changed: crtc_state->active has been toggled.
>   * @connectors_changed: connectors to this crtc have been updated
> + * @color_mgmt_changed: color management properties have changed (degamma or
> + *	gamma LUT or CSC matrix)
>   * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
>   * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors
>   * @last_vblank_count: for helpers and drivers to capture the vblank of the
>   * 	update to ensure framebuffer cleanup isn't done too early
>   * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
>   * @mode: current mode timings
> + * @degamma_lut: Lookup table for converting framebuffer pixel data
> + *	before apply the conversion matrix
> + * @ctm_matrix: Transformation matrix
> + * @gamma_lut: Lookup table for converting pixel data after the
> + *	conversion matrix
>   * @event: optional pointer to a DRM event to signal upon completion of the
>   * 	state update
>   * @state: backpointer to global drm_atomic_state
> @@ -332,6 +339,7 @@ struct drm_crtc_state {
>  	bool mode_changed : 1;
>  	bool active_changed : 1;
>  	bool connectors_changed : 1;
> +	bool color_mgmt_changed : 1;
>  
>  	/* attached planes bitmask:
>  	 * WARNING: transitional helpers do not maintain plane_mask so
> @@ -353,6 +361,11 @@ struct drm_crtc_state {
>  	/* blob property to expose current mode to atomic userspace */
>  	struct drm_property_blob *mode_blob;
>  
> +	/* blob property to expose color management to userspace */
> +	struct drm_property_blob *degamma_lut;
> +	struct drm_property_blob *ctm_matrix;
> +	struct drm_property_blob *gamma_lut;
> +
>  	struct drm_pending_vblank_event *event;
>  
>  	struct drm_atomic_state *state;
> @@ -755,7 +768,7 @@ struct drm_crtc {
>  	int x, y;
>  	const struct drm_crtc_funcs *funcs;
>  
> -	/* CRTC gamma size for reporting to userspace */
> +	/* Legacy FB CRTC gamma size for reporting to userspace */
>  	uint32_t gamma_size;
>  	uint16_t *gamma_store;
>  
> @@ -2023,6 +2036,15 @@ struct drm_mode_config_funcs {
>   * @property_blob_list: list of all the blob property objects
>   * @blob_lock: mutex for blob property allocation and management
>   * @*_property: core property tracking
> + * @degamma_lut_property: LUT used to convert the framebuffer's colors to linear
> + *	gamma
> + * @degamma_lut_size_property: size of the degamma LUT as supported by the
> + *	driver (read-only)
> + * @ctm_matrix_property: Matrix used to convert colors after the lookup in the
> + *	degamma LUT
> + * @gamma_lut_property: LUT used to convert the colors, after the CSC matrix, to
> + *	the gamma space of the connected screen (read-only)
> + * @gamma_lut_size_property: size of the gamma LUT as supported by the driver
>   * @preferred_depth: preferred RBG pixel depth, used by fb helpers
>   * @prefer_shadow: hint to userspace to prefer shadow-fb rendering
>   * @async_page_flip: does this device support async flips on the primary plane?
> @@ -2124,6 +2146,13 @@ struct drm_mode_config {
>  	struct drm_property *aspect_ratio_property;
>  	struct drm_property *dirty_info_property;
>  
> +	/* Optional color correction properties */
> +	struct drm_property *degamma_lut_property;
> +	struct drm_property *degamma_lut_size_property;
> +	struct drm_property *ctm_matrix_property;
> +	struct drm_property *gamma_lut_property;
> +	struct drm_property *gamma_lut_size_property;
> +
>  	/* properties for virtual machine layout */
>  	struct drm_property *suggested_x_property;
>  	struct drm_property *suggested_y_property;
> @@ -2530,6 +2559,18 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
>  	return mo ? obj_to_property(mo) : NULL;
>  }
>  
> +/*
> + * Extract a degamma/gamma LUT value provided by user and round it to the
> + * precision supported by the hardware.
> + */
> +static inline uint32_t drm_color_lut_get_value(uint32_t user_input,
> +					       uint32_t bit_precision)
> +{
> +	uint32_t val = (user_input << 1) + (1 << (16 - bit_precision));
> +
> +	return val >> (16 - bit_precision + 1);
> +}
> +
>  /* Plane list iterator for legacy (overlay only) planes. */
>  #define drm_for_each_legacy_plane(plane, dev) \
>  	list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
> diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
> index 4b37afa..97fa894 100644
> --- a/include/drm/drm_crtc_helper.h
> +++ b/include/drm/drm_crtc_helper.h
> @@ -48,6 +48,9 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
>  				     struct drm_display_mode *mode,
>  				     int x, int y,
>  				     struct drm_framebuffer *old_fb);
> +extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
> +					      int degamma_lut_size,
> +					      int gamma_lut_size);
>  extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc);
>  extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
>  
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 50adb46..c021743 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -487,6 +487,21 @@ struct drm_mode_crtc_lut {
>  	__u64 blue;
>  };
>  
> +struct drm_color_ctm {
> +	/* Conversion matrix in S31.32 format. */
> +	__s64 matrix[9];
> +};
> +
> +struct drm_color_lut {
> +	/*
> +	 * Data is U0.16 fixed point format.
> +	 */
> +	__u16 red;
> +	__u16 green;
> +	__u16 blue;
> +	__u16 reserved;
> +};
> +
>  #define DRM_MODE_PAGE_FLIP_EVENT 0x01
>  #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
>  #define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT|DRM_MODE_PAGE_FLIP_ASYNC)
> -- 
> 2.6.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://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
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 2/6] drm: introduce color correction properties

[Lionel Landwerlin]

v2: Register LUT size properties as range

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 Documentation/DocBook/gpu.tmpl      |  50 +++++++++++++++++-
 drivers/gpu/drm/drm_atomic.c        | 102 +++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/drm_atomic_helper.c |  10 ++++
 drivers/gpu/drm/drm_crtc.c          |  35 +++++++++++++
 drivers/gpu/drm/drm_crtc_helper.c   |  33 ++++++++++++
 include/drm/drm_crtc.h              |  43 ++++++++++++++-
 include/drm/drm_crtc_helper.h       |   3 ++
 include/uapi/drm/drm_mode.h         |  15 ++++++
 8 files changed, 287 insertions(+), 4 deletions(-)

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 351e801..c17ac29 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -2068,7 +2068,7 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >property to suggest an Y offset for a connector</td>
 	</tr>
 	<tr>
-	<td rowspan="3" valign="top" >Optional</td>
+	<td rowspan="8" valign="top" >Optional</td>
 	<td valign="top" >“scaling mode”</td>
 	<td valign="top" >ENUM</td>
 	<td valign="top" >{ "None", "Full", "Center", "Full aspect" }</td>
@@ -2092,6 +2092,54 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
+	<td valign="top" >“DEGAMMA_LUT”</td>
+	<td valign="top" >BLOB</td>
+	<td valign="top" >0</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to set the degamma LUT mapping
+		pixel data from the framebuffer before it is given to the
+		transformation matrix. The data is an interpreted as an array
+		of struct drm_color_lut elements.</td>
+	</tr>
+	<tr>
+	<td valign="top" >“DEGAMMA_LUT_SIZE”</td>
+	<td valign="top" >RANGE | IMMUTABLE</td>
+	<td valign="top" >Min=0, Max=UINT_MAX</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to gives the size of the LUT to
+		be set on the DEGAMMA_LUT property (the size depends on the
+		underlying hardware).</td>
+	</tr>
+	<tr>
+	<td valign="top" >“CTM_MATRIX”</td>
+	<td valign="top" >BLOB</td>
+	<td valign="top" >0</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to set the transformation
+		matrix apply to pixel data after the lookup through the
+		degamma LUT and before the lookup through the gamma LUT. The
+		data is an interpreted as a struct drm_color_ctm.</td>
+	</tr>
+	<tr>
+	<td valign="top" >“GAMMA_LUT”</td>
+	<td valign="top" >BLOB</td>
+	<td valign="top" >0</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to set the gamma LUT mapping
+		pixel data after to the transformation matrix to data sent to
+		the connector. The data is an interpreted as an array
+		of struct drm_color_lut elements.</td>
+	</tr>
+	<tr>
+	<td valign="top" >“GAMMA_LUT_SIZE”</td>
+	<td valign="top" >RANGE | IMMUTABLE</td>
+	<td valign="top" >Min=0, Max=UINT_MAX</td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to gives the size of the LUT to
+		be set on the GAMMA_LUT property (the size depends on the
+		underlying hardware).</td>
+	</tr>
+	<tr>
 	<td rowspan="20" valign="top" >i915</td>
 	<td rowspan="2" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3f74193..5287416 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -28,6 +28,7 @@
 
 #include <drm/drmP.h>
 #include <drm/drm_atomic.h>
+#include <drm/drm_mode.h>
 #include <drm/drm_plane_helper.h>
 
 /**
@@ -388,6 +389,58 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
 EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
 
 /**
+ * drm_atomic_replace_property_blob - replace a blob property
+ * @blob: a pointer to the member blob to be replaced
+ * @new_blob: the new blob to replace with
+ * @expected_size: the expected size of the new blob
+ * @replaced: whether the blob has been replaced
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+static int
+drm_atomic_replace_property_blob(struct drm_property_blob **blob,
+				 struct drm_property_blob *new_blob,
+				 size_t expected_size,
+				 bool *replaced)
+{
+	struct drm_property_blob *old_blob = *blob;
+
+	if (old_blob == new_blob)
+		return 0;
+
+	if (new_blob != NULL && new_blob->length != expected_size)
+		return -EINVAL;
+
+	if (old_blob != NULL)
+		drm_property_unreference_blob(old_blob);
+	*blob = new_blob;
+	*replaced = true;
+
+	return 0;
+}
+
+static int
+drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc,
+					 struct drm_property_blob **blob,
+					 uint64_t blob_id,
+					 size_t expected_size,
+					 bool *replaced)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_property_blob *new_blob = NULL;
+
+	if (blob_id != 0) {
+		new_blob = drm_property_lookup_blob(dev, blob_id);
+		if (new_blob == NULL)
+			return -EINVAL;
+	}
+
+	return drm_atomic_replace_property_blob(blob, new_blob,
+						expected_size, replaced);
+}
+
+/**
  * drm_atomic_crtc_set_property - set property on CRTC
  * @crtc: the drm CRTC to set a property on
  * @state: the state object to update with the new property value
@@ -419,8 +472,47 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
 		drm_property_unreference_blob(mode);
 		return ret;
-	}
-	else if (crtc->funcs->atomic_set_property)
+	} else if (property == config->degamma_lut_property) {
+		bool replaced = false;
+		uint64_t lut_size = 0;
+
+		ret = drm_object_property_get_value(&crtc->base,
+					config->degamma_lut_size_property,
+					&lut_size);
+		if (ret == 0)
+			ret = drm_atomic_replace_property_blob_from_id(crtc,
+					&state->degamma_lut,
+					val,
+					lut_size * sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed = replaced;
+		return ret;
+	} else if (property == config->ctm_matrix_property) {
+		bool replaced = false;
+
+		ret = drm_atomic_replace_property_blob_from_id(crtc,
+					&state->ctm_matrix,
+					val,
+					sizeof(struct drm_color_ctm),
+							       &replaced);
+		state->color_mgmt_changed = replaced;
+		return ret;
+	} else if (property == config->gamma_lut_property) {
+		bool replaced = false;
+		uint64_t lut_size = 0;
+
+		ret = drm_object_property_get_value(&crtc->base,
+					config->gamma_lut_size_property,
+					&lut_size);
+		if (ret == 0)
+			ret = drm_atomic_replace_property_blob_from_id(crtc,
+					&state->gamma_lut,
+					val,
+					lut_size * sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed = replaced;
+		return ret;
+	} else if (crtc->funcs->atomic_set_property)
 		return crtc->funcs->atomic_set_property(crtc, state, property, val);
 	else
 		return -EINVAL;
@@ -456,6 +548,12 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
 		*val = state->active;
 	else if (property == config->prop_mode_id)
 		*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
+	else if (property == config->degamma_lut_property)
+		*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
+	else if (property == config->ctm_matrix_property)
+		*val = (state->ctm_matrix) ? state->ctm_matrix->base.id : 0;
+	else if (property == config->gamma_lut_property)
+		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
 	else if (crtc->funcs->atomic_get_property)
 		return crtc->funcs->atomic_get_property(crtc, state, property, val);
 	else
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index f0c3984..e4a5755 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2454,10 +2454,17 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
 
 	if (state->mode_blob)
 		drm_property_reference_blob(state->mode_blob);
+	if (state->degamma_lut)
+		drm_property_reference_blob(state->degamma_lut);
+	if (state->ctm_matrix)
+		drm_property_reference_blob(state->ctm_matrix);
+	if (state->gamma_lut)
+		drm_property_reference_blob(state->gamma_lut);
 	state->mode_changed = false;
 	state->active_changed = false;
 	state->planes_changed = false;
 	state->connectors_changed = false;
+	state->color_mgmt_changed = false;
 	state->event = NULL;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
@@ -2498,6 +2505,9 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
 					    struct drm_crtc_state *state)
 {
 	drm_property_unreference_blob(state->mode_blob);
+	drm_property_unreference_blob(state->degamma_lut);
+	drm_property_unreference_blob(state->ctm_matrix);
+	drm_property_unreference_blob(state->gamma_lut);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
 
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d40bab2..b1aa38a 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1542,6 +1542,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.prop_mode_id = prop;
 
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"DEGAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.degamma_lut_property = prop;
+
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"DEGAMMA_LUT_SIZE", 0, UINT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.degamma_lut_size_property = prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"CTM_MATRIX", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.ctm_matrix_property = prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"GAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.gamma_lut_property = prop;
+
+	prop = drm_property_create_range(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"GAMMA_LUT_SIZE", 0, UINT_MAX);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.gamma_lut_size_property = prop;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 5d4bc64..b9b1929 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -1064,3 +1064,36 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	return drm_plane_helper_commit(plane, plane_state, old_fb);
 }
 EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
+
+/**
+ * drm_helper_crtc_enable_color_mgmt - enable color management properties
+ * @crtc: DRM CRTC
+ * @degamma_lut_size: the size of the degamma lut (before CSC)
+ * @gamma_lut_size: the size of the gamma lut (after CSC)
+ *
+ * This function lets the driver enable the color correction properties on a
+ * CRTC. This includes 3 degamma, csc and gamma properties that userspace can
+ * set and 2 size properties to inform the userspace of the lut sizes.
+ */
+void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
+				       int degamma_lut_size,
+				       int gamma_lut_size)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+
+	drm_object_attach_property(&crtc->base,
+				   config->degamma_lut_property, 0);
+	drm_object_attach_property(&crtc->base,
+				   config->ctm_matrix_property, 0);
+	drm_object_attach_property(&crtc->base,
+				   config->gamma_lut_property, 0);
+
+	drm_object_attach_property(&crtc->base,
+				   config->degamma_lut_size_property,
+				   degamma_lut_size);
+	drm_object_attach_property(&crtc->base,
+				   config->gamma_lut_size_property,
+				   gamma_lut_size);
+}
+EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index c65a212..967d646 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -305,12 +305,19 @@ struct drm_plane_helper_funcs;
  * @mode_changed: crtc_state->mode or crtc_state->enable has been changed
  * @active_changed: crtc_state->active has been toggled.
  * @connectors_changed: connectors to this crtc have been updated
+ * @color_mgmt_changed: color management properties have changed (degamma or
+ *	gamma LUT or CSC matrix)
  * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
  * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors
  * @last_vblank_count: for helpers and drivers to capture the vblank of the
  * 	update to ensure framebuffer cleanup isn't done too early
  * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
  * @mode: current mode timings
+ * @degamma_lut: Lookup table for converting framebuffer pixel data
+ *	before apply the conversion matrix
+ * @ctm_matrix: Transformation matrix
+ * @gamma_lut: Lookup table for converting pixel data after the
+ *	conversion matrix
  * @event: optional pointer to a DRM event to signal upon completion of the
  * 	state update
  * @state: backpointer to global drm_atomic_state
@@ -332,6 +339,7 @@ struct drm_crtc_state {
 	bool mode_changed : 1;
 	bool active_changed : 1;
 	bool connectors_changed : 1;
+	bool color_mgmt_changed : 1;
 
 	/* attached planes bitmask:
 	 * WARNING: transitional helpers do not maintain plane_mask so
@@ -353,6 +361,11 @@ struct drm_crtc_state {
 	/* blob property to expose current mode to atomic userspace */
 	struct drm_property_blob *mode_blob;
 
+	/* blob property to expose color management to userspace */
+	struct drm_property_blob *degamma_lut;
+	struct drm_property_blob *ctm_matrix;
+	struct drm_property_blob *gamma_lut;
+
 	struct drm_pending_vblank_event *event;
 
 	struct drm_atomic_state *state;
@@ -755,7 +768,7 @@ struct drm_crtc {
 	int x, y;
 	const struct drm_crtc_funcs *funcs;
 
-	/* CRTC gamma size for reporting to userspace */
+	/* Legacy FB CRTC gamma size for reporting to userspace */
 	uint32_t gamma_size;
 	uint16_t *gamma_store;
 
@@ -2023,6 +2036,15 @@ struct drm_mode_config_funcs {
  * @property_blob_list: list of all the blob property objects
  * @blob_lock: mutex for blob property allocation and management
  * @*_property: core property tracking
+ * @degamma_lut_property: LUT used to convert the framebuffer's colors to linear
+ *	gamma
+ * @degamma_lut_size_property: size of the degamma LUT as supported by the
+ *	driver (read-only)
+ * @ctm_matrix_property: Matrix used to convert colors after the lookup in the
+ *	degamma LUT
+ * @gamma_lut_property: LUT used to convert the colors, after the CSC matrix, to
+ *	the gamma space of the connected screen (read-only)
+ * @gamma_lut_size_property: size of the gamma LUT as supported by the driver
  * @preferred_depth: preferred RBG pixel depth, used by fb helpers
  * @prefer_shadow: hint to userspace to prefer shadow-fb rendering
  * @async_page_flip: does this device support async flips on the primary plane?
@@ -2124,6 +2146,13 @@ struct drm_mode_config {
 	struct drm_property *aspect_ratio_property;
 	struct drm_property *dirty_info_property;
 
+	/* Optional color correction properties */
+	struct drm_property *degamma_lut_property;
+	struct drm_property *degamma_lut_size_property;
+	struct drm_property *ctm_matrix_property;
+	struct drm_property *gamma_lut_property;
+	struct drm_property *gamma_lut_size_property;
+
 	/* properties for virtual machine layout */
 	struct drm_property *suggested_x_property;
 	struct drm_property *suggested_y_property;
@@ -2530,6 +2559,18 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
 	return mo ? obj_to_property(mo) : NULL;
 }
 
+/*
+ * Extract a degamma/gamma LUT value provided by user and round it to the
+ * precision supported by the hardware.
+ */
+static inline uint32_t drm_color_lut_get_value(uint32_t user_input,
+					       uint32_t bit_precision)
+{
+	uint32_t val = (user_input << 1) + (1 << (16 - bit_precision));
+
+	return val >> (16 - bit_precision + 1);
+}
+
 /* Plane list iterator for legacy (overlay only) planes. */
 #define drm_for_each_legacy_plane(plane, dev) \
 	list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 4b37afa..97fa894 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -48,6 +48,9 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
 				     struct drm_display_mode *mode,
 				     int x, int y,
 				     struct drm_framebuffer *old_fb);
+extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
+					      int degamma_lut_size,
+					      int gamma_lut_size);
 extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc);
 extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
 
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 50adb46..c021743 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -487,6 +487,21 @@ struct drm_mode_crtc_lut {
 	__u64 blue;
 };
 
+struct drm_color_ctm {
+	/* Conversion matrix in S31.32 format. */
+	__s64 matrix[9];
+};
+
+struct drm_color_lut {
+	/*
+	 * Data is U0.16 fixed point format.
+	 */
+	__u16 red;
+	__u16 green;
+	__u16 blue;
+	__u16 reserved;
+};
+
 #define DRM_MODE_PAGE_FLIP_EVENT 0x01
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT|DRM_MODE_PAGE_FLIP_ASYNC)
-- 
2.6.4

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

Re: [PATCH 2/6] drm: introduce color correction properties

[Lionel Landwerlin]

On 21/01/16 19:20, Matt Roper wrote:
> On Thu, Jan 21, 2016 at 03:03:49PM +0000, Lionel Landwerlin wrote:
> ...
>> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
>> index 351e801..d2f682c 100644
>> --- a/Documentation/DocBook/gpu.tmpl
>> +++ b/Documentation/DocBook/gpu.tmpl
>> @@ -2092,6 +2092,54 @@ void intel_crt_init(struct drm_device *dev)
>>   	<td valign="top" >TBD</td>
>>   	</tr>
>>   	<tr>
>> +	<td valign="top" >“DEGAMMA_LUT”</td>
>> +	<td valign="top" >BLOB</td>
>> +	<td valign="top" ></td>
>> +	<td valign="top" >CRTC</td>
>> +	<td valign="top" >DRM property to set the degamma LUT mapping
>> +		pixel data from the framebuffer before it is given to the
>> +		transformation matrix. The data is an interpreted as an array
>> +		of struct drm_color_lut elements.</td>
>> +	</tr>
>> +	<tr>
>> +	<td valign="top" >“DEGAMMA_LUT_SIZE”</td>
>> +	<td valign="top" >IMMUTABLE</td>
> IMMUTABLE by itself isn't a property type, just a flag that you can
> apply in addition to the type.  You probably want "RANGE | IMMUTABLE"
> here (and also on GAMMA_LUT_SIZE below).
>
> ...snip...
>> +	<td valign="top" >“GAMMA_LUT_SIZE”</td>
>> +	<td valign="top" >IMMUTABLE</td>
>> +	<td valign="top" ></td>
>> +	<td valign="top" >CRTC</td>
>> +	<td valign="top" >DRM property to gives the size of the LUT to
>> +		be set on the GAMMA_LUT property (the size depends on the
>> +		underlying hardware).</td>
>> +	</tr>
>> +	<tr>
>>   	<td rowspan="20" valign="top" >i915</td>
>>   	<td rowspan="2" valign="top" >Generic</td>
>>   	<td valign="top" >"Broadcast RGB"</td>
> ...snip...
>> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>> index d40bab2..6f96c04 100644
>> --- a/drivers/gpu/drm/drm_crtc.c
>> +++ b/drivers/gpu/drm/drm_crtc.c
>> @@ -1542,6 +1542,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>>   		return -ENOMEM;
>>   	dev->mode_config.prop_mode_id = prop;
>>   
>> +	prop = drm_property_create(dev,
>> +			DRM_MODE_PROP_BLOB,
>> +			"DEGAMMA_LUT", 0);
>> +	if (!prop)
>> +		return -ENOMEM;
>> +	dev->mode_config.degamma_lut_property = prop;
>> +
>> +	prop = drm_property_create(dev,
>> +			DRM_MODE_PROP_IMMUTABLE,
> As noted above, you still need to OR in a valid property type (in this
> case DRM_MODE_PROP_RANGE); otherwise you'll trip over the the check in
> drm_property_type_valid() that gets called at the end of
> drm_property_create().
>
>
> Matt
>

Thanks Matt,

Just realized that yesterday while talking to Damien.
Will fix in v2.

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

Re: [PATCH 2/6] drm: introduce color correction properties

[Matt Roper]

On Thu, Jan 21, 2016 at 03:03:49PM +0000, Lionel Landwerlin wrote:
...
> diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
> index 351e801..d2f682c 100644
> --- a/Documentation/DocBook/gpu.tmpl
> +++ b/Documentation/DocBook/gpu.tmpl
> @@ -2092,6 +2092,54 @@ void intel_crt_init(struct drm_device *dev)
>  	<td valign="top" >TBD</td>
>  	</tr>
>  	<tr>
> +	<td valign="top" >“DEGAMMA_LUT”</td>
> +	<td valign="top" >BLOB</td>
> +	<td valign="top" ></td>
> +	<td valign="top" >CRTC</td>
> +	<td valign="top" >DRM property to set the degamma LUT mapping
> +		pixel data from the framebuffer before it is given to the
> +		transformation matrix. The data is an interpreted as an array
> +		of struct drm_color_lut elements.</td>
> +	</tr>
> +	<tr>
> +	<td valign="top" >“DEGAMMA_LUT_SIZE”</td>
> +	<td valign="top" >IMMUTABLE</td>

IMMUTABLE by itself isn't a property type, just a flag that you can
apply in addition to the type.  You probably want "RANGE | IMMUTABLE"
here (and also on GAMMA_LUT_SIZE below).

...snip...
> +	<td valign="top" >“GAMMA_LUT_SIZE”</td>
> +	<td valign="top" >IMMUTABLE</td>
> +	<td valign="top" ></td>
> +	<td valign="top" >CRTC</td>
> +	<td valign="top" >DRM property to gives the size of the LUT to
> +		be set on the GAMMA_LUT property (the size depends on the
> +		underlying hardware).</td>
> +	</tr>
> +	<tr>
>  	<td rowspan="20" valign="top" >i915</td>
>  	<td rowspan="2" valign="top" >Generic</td>
>  	<td valign="top" >"Broadcast RGB"</td>
...snip...
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index d40bab2..6f96c04 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -1542,6 +1542,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
>  		return -ENOMEM;
>  	dev->mode_config.prop_mode_id = prop;
>  
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_BLOB,
> +			"DEGAMMA_LUT", 0);
> +	if (!prop)
> +		return -ENOMEM;
> +	dev->mode_config.degamma_lut_property = prop;
> +
> +	prop = drm_property_create(dev,
> +			DRM_MODE_PROP_IMMUTABLE,

As noted above, you still need to OR in a valid property type (in this
case DRM_MODE_PROP_RANGE); otherwise you'll trip over the the check in
drm_property_type_valid() that gets called at the end of
drm_property_create().


Matt

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

[PATCH 2/6] drm: introduce color correction properties

[Lionel Landwerlin]

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 Documentation/DocBook/gpu.tmpl      |  48 +++++++++++++++++
 drivers/gpu/drm/drm_atomic.c        | 102 +++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/drm_atomic_helper.c |  10 ++++
 drivers/gpu/drm/drm_crtc.c          |  35 +++++++++++++
 drivers/gpu/drm/drm_crtc_helper.c   |  33 ++++++++++++
 include/drm/drm_crtc.h              |  43 ++++++++++++++-
 include/drm/drm_crtc_helper.h       |   3 ++
 include/uapi/drm/drm_mode.h         |  15 ++++++
 8 files changed, 286 insertions(+), 3 deletions(-)

diff --git a/Documentation/DocBook/gpu.tmpl b/Documentation/DocBook/gpu.tmpl
index 351e801..d2f682c 100644
--- a/Documentation/DocBook/gpu.tmpl
+++ b/Documentation/DocBook/gpu.tmpl
@@ -2092,6 +2092,54 @@ void intel_crt_init(struct drm_device *dev)
 	<td valign="top" >TBD</td>
 	</tr>
 	<tr>
+	<td valign="top" >“DEGAMMA_LUT”</td>
+	<td valign="top" >BLOB</td>
+	<td valign="top" ></td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to set the degamma LUT mapping
+		pixel data from the framebuffer before it is given to the
+		transformation matrix. The data is an interpreted as an array
+		of struct drm_color_lut elements.</td>
+	</tr>
+	<tr>
+	<td valign="top" >“DEGAMMA_LUT_SIZE”</td>
+	<td valign="top" >IMMUTABLE</td>
+	<td valign="top" ></td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to gives the size of the LUT to
+		be set on the DEGAMMA_LUT property (the size depends on the
+		underlying hardware).</td>
+	</tr>
+	<tr>
+	<td valign="top" >“CTM_MATRIX”</td>
+	<td valign="top" >BLOB</td>
+	<td valign="top" ></td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to set the transformation
+		matrix apply to pixel data after the lookup through the
+		degamma LUT and before the lookup through the gamma LUT. The
+		data is an interpreted as a struct drm_color_ctm.</td>
+	</tr>
+	<tr>
+	<td valign="top" >“GAMMA_LUT”</td>
+	<td valign="top" >BLOB</td>
+	<td valign="top" ></td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to set the gamma LUT mapping
+		pixel data after to the transformation matrix to data sent to
+		the connector. The data is an interpreted as an array
+		of struct drm_color_lut elements.</td>
+	</tr>
+	<tr>
+	<td valign="top" >“GAMMA_LUT_SIZE”</td>
+	<td valign="top" >IMMUTABLE</td>
+	<td valign="top" ></td>
+	<td valign="top" >CRTC</td>
+	<td valign="top" >DRM property to gives the size of the LUT to
+		be set on the GAMMA_LUT property (the size depends on the
+		underlying hardware).</td>
+	</tr>
+	<tr>
 	<td rowspan="20" valign="top" >i915</td>
 	<td rowspan="2" valign="top" >Generic</td>
 	<td valign="top" >"Broadcast RGB"</td>
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3f74193..5287416 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -28,6 +28,7 @@
 
 #include <drm/drmP.h>
 #include <drm/drm_atomic.h>
+#include <drm/drm_mode.h>
 #include <drm/drm_plane_helper.h>
 
 /**
@@ -388,6 +389,58 @@ int drm_atomic_set_mode_prop_for_crtc(struct drm_crtc_state *state,
 EXPORT_SYMBOL(drm_atomic_set_mode_prop_for_crtc);
 
 /**
+ * drm_atomic_replace_property_blob - replace a blob property
+ * @blob: a pointer to the member blob to be replaced
+ * @new_blob: the new blob to replace with
+ * @expected_size: the expected size of the new blob
+ * @replaced: whether the blob has been replaced
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+static int
+drm_atomic_replace_property_blob(struct drm_property_blob **blob,
+				 struct drm_property_blob *new_blob,
+				 size_t expected_size,
+				 bool *replaced)
+{
+	struct drm_property_blob *old_blob = *blob;
+
+	if (old_blob == new_blob)
+		return 0;
+
+	if (new_blob != NULL && new_blob->length != expected_size)
+		return -EINVAL;
+
+	if (old_blob != NULL)
+		drm_property_unreference_blob(old_blob);
+	*blob = new_blob;
+	*replaced = true;
+
+	return 0;
+}
+
+static int
+drm_atomic_replace_property_blob_from_id(struct drm_crtc *crtc,
+					 struct drm_property_blob **blob,
+					 uint64_t blob_id,
+					 size_t expected_size,
+					 bool *replaced)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_property_blob *new_blob = NULL;
+
+	if (blob_id != 0) {
+		new_blob = drm_property_lookup_blob(dev, blob_id);
+		if (new_blob == NULL)
+			return -EINVAL;
+	}
+
+	return drm_atomic_replace_property_blob(blob, new_blob,
+						expected_size, replaced);
+}
+
+/**
  * drm_atomic_crtc_set_property - set property on CRTC
  * @crtc: the drm CRTC to set a property on
  * @state: the state object to update with the new property value
@@ -419,8 +472,47 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
 		ret = drm_atomic_set_mode_prop_for_crtc(state, mode);
 		drm_property_unreference_blob(mode);
 		return ret;
-	}
-	else if (crtc->funcs->atomic_set_property)
+	} else if (property == config->degamma_lut_property) {
+		bool replaced = false;
+		uint64_t lut_size = 0;
+
+		ret = drm_object_property_get_value(&crtc->base,
+					config->degamma_lut_size_property,
+					&lut_size);
+		if (ret == 0)
+			ret = drm_atomic_replace_property_blob_from_id(crtc,
+					&state->degamma_lut,
+					val,
+					lut_size * sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed = replaced;
+		return ret;
+	} else if (property == config->ctm_matrix_property) {
+		bool replaced = false;
+
+		ret = drm_atomic_replace_property_blob_from_id(crtc,
+					&state->ctm_matrix,
+					val,
+					sizeof(struct drm_color_ctm),
+							       &replaced);
+		state->color_mgmt_changed = replaced;
+		return ret;
+	} else if (property == config->gamma_lut_property) {
+		bool replaced = false;
+		uint64_t lut_size = 0;
+
+		ret = drm_object_property_get_value(&crtc->base,
+					config->gamma_lut_size_property,
+					&lut_size);
+		if (ret == 0)
+			ret = drm_atomic_replace_property_blob_from_id(crtc,
+					&state->gamma_lut,
+					val,
+					lut_size * sizeof(struct drm_color_lut),
+					&replaced);
+		state->color_mgmt_changed = replaced;
+		return ret;
+	} else if (crtc->funcs->atomic_set_property)
 		return crtc->funcs->atomic_set_property(crtc, state, property, val);
 	else
 		return -EINVAL;
@@ -456,6 +548,12 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
 		*val = state->active;
 	else if (property == config->prop_mode_id)
 		*val = (state->mode_blob) ? state->mode_blob->base.id : 0;
+	else if (property == config->degamma_lut_property)
+		*val = (state->degamma_lut) ? state->degamma_lut->base.id : 0;
+	else if (property == config->ctm_matrix_property)
+		*val = (state->ctm_matrix) ? state->ctm_matrix->base.id : 0;
+	else if (property == config->gamma_lut_property)
+		*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
 	else if (crtc->funcs->atomic_get_property)
 		return crtc->funcs->atomic_get_property(crtc, state, property, val);
 	else
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index f0c3984..e4a5755 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -2454,10 +2454,17 @@ void __drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc,
 
 	if (state->mode_blob)
 		drm_property_reference_blob(state->mode_blob);
+	if (state->degamma_lut)
+		drm_property_reference_blob(state->degamma_lut);
+	if (state->ctm_matrix)
+		drm_property_reference_blob(state->ctm_matrix);
+	if (state->gamma_lut)
+		drm_property_reference_blob(state->gamma_lut);
 	state->mode_changed = false;
 	state->active_changed = false;
 	state->planes_changed = false;
 	state->connectors_changed = false;
+	state->color_mgmt_changed = false;
 	state->event = NULL;
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_duplicate_state);
@@ -2498,6 +2505,9 @@ void __drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
 					    struct drm_crtc_state *state)
 {
 	drm_property_unreference_blob(state->mode_blob);
+	drm_property_unreference_blob(state->degamma_lut);
+	drm_property_unreference_blob(state->ctm_matrix);
+	drm_property_unreference_blob(state->gamma_lut);
 }
 EXPORT_SYMBOL(__drm_atomic_helper_crtc_destroy_state);
 
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index d40bab2..6f96c04 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1542,6 +1542,41 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
 		return -ENOMEM;
 	dev->mode_config.prop_mode_id = prop;
 
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"DEGAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.degamma_lut_property = prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"DEGAMMA_LUT_SIZE", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.degamma_lut_size_property = prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"CTM_MATRIX", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.ctm_matrix_property = prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_BLOB,
+			"GAMMA_LUT", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.gamma_lut_property = prop;
+
+	prop = drm_property_create(dev,
+			DRM_MODE_PROP_IMMUTABLE,
+			"GAMMA_LUT_SIZE", 0);
+	if (!prop)
+		return -ENOMEM;
+	dev->mode_config.gamma_lut_size_property = prop;
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 5d4bc64..b9b1929 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -1064,3 +1064,36 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	return drm_plane_helper_commit(plane, plane_state, old_fb);
 }
 EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
+
+/**
+ * drm_helper_crtc_enable_color_mgmt - enable color management properties
+ * @crtc: DRM CRTC
+ * @degamma_lut_size: the size of the degamma lut (before CSC)
+ * @gamma_lut_size: the size of the gamma lut (after CSC)
+ *
+ * This function lets the driver enable the color correction properties on a
+ * CRTC. This includes 3 degamma, csc and gamma properties that userspace can
+ * set and 2 size properties to inform the userspace of the lut sizes.
+ */
+void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
+				       int degamma_lut_size,
+				       int gamma_lut_size)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_mode_config *config = &dev->mode_config;
+
+	drm_object_attach_property(&crtc->base,
+				   config->degamma_lut_property, 0);
+	drm_object_attach_property(&crtc->base,
+				   config->ctm_matrix_property, 0);
+	drm_object_attach_property(&crtc->base,
+				   config->gamma_lut_property, 0);
+
+	drm_object_attach_property(&crtc->base,
+				   config->degamma_lut_size_property,
+				   degamma_lut_size);
+	drm_object_attach_property(&crtc->base,
+				   config->gamma_lut_size_property,
+				   gamma_lut_size);
+}
+EXPORT_SYMBOL(drm_helper_crtc_enable_color_mgmt);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index c65a212..967d646 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -305,12 +305,19 @@ struct drm_plane_helper_funcs;
  * @mode_changed: crtc_state->mode or crtc_state->enable has been changed
  * @active_changed: crtc_state->active has been toggled.
  * @connectors_changed: connectors to this crtc have been updated
+ * @color_mgmt_changed: color management properties have changed (degamma or
+ *	gamma LUT or CSC matrix)
  * @plane_mask: bitmask of (1 << drm_plane_index(plane)) of attached planes
  * @connector_mask: bitmask of (1 << drm_connector_index(connector)) of attached connectors
  * @last_vblank_count: for helpers and drivers to capture the vblank of the
  * 	update to ensure framebuffer cleanup isn't done too early
  * @adjusted_mode: for use by helpers and drivers to compute adjusted mode timings
  * @mode: current mode timings
+ * @degamma_lut: Lookup table for converting framebuffer pixel data
+ *	before apply the conversion matrix
+ * @ctm_matrix: Transformation matrix
+ * @gamma_lut: Lookup table for converting pixel data after the
+ *	conversion matrix
  * @event: optional pointer to a DRM event to signal upon completion of the
  * 	state update
  * @state: backpointer to global drm_atomic_state
@@ -332,6 +339,7 @@ struct drm_crtc_state {
 	bool mode_changed : 1;
 	bool active_changed : 1;
 	bool connectors_changed : 1;
+	bool color_mgmt_changed : 1;
 
 	/* attached planes bitmask:
 	 * WARNING: transitional helpers do not maintain plane_mask so
@@ -353,6 +361,11 @@ struct drm_crtc_state {
 	/* blob property to expose current mode to atomic userspace */
 	struct drm_property_blob *mode_blob;
 
+	/* blob property to expose color management to userspace */
+	struct drm_property_blob *degamma_lut;
+	struct drm_property_blob *ctm_matrix;
+	struct drm_property_blob *gamma_lut;
+
 	struct drm_pending_vblank_event *event;
 
 	struct drm_atomic_state *state;
@@ -755,7 +768,7 @@ struct drm_crtc {
 	int x, y;
 	const struct drm_crtc_funcs *funcs;
 
-	/* CRTC gamma size for reporting to userspace */
+	/* Legacy FB CRTC gamma size for reporting to userspace */
 	uint32_t gamma_size;
 	uint16_t *gamma_store;
 
@@ -2023,6 +2036,15 @@ struct drm_mode_config_funcs {
  * @property_blob_list: list of all the blob property objects
  * @blob_lock: mutex for blob property allocation and management
  * @*_property: core property tracking
+ * @degamma_lut_property: LUT used to convert the framebuffer's colors to linear
+ *	gamma
+ * @degamma_lut_size_property: size of the degamma LUT as supported by the
+ *	driver (read-only)
+ * @ctm_matrix_property: Matrix used to convert colors after the lookup in the
+ *	degamma LUT
+ * @gamma_lut_property: LUT used to convert the colors, after the CSC matrix, to
+ *	the gamma space of the connected screen (read-only)
+ * @gamma_lut_size_property: size of the gamma LUT as supported by the driver
  * @preferred_depth: preferred RBG pixel depth, used by fb helpers
  * @prefer_shadow: hint to userspace to prefer shadow-fb rendering
  * @async_page_flip: does this device support async flips on the primary plane?
@@ -2124,6 +2146,13 @@ struct drm_mode_config {
 	struct drm_property *aspect_ratio_property;
 	struct drm_property *dirty_info_property;
 
+	/* Optional color correction properties */
+	struct drm_property *degamma_lut_property;
+	struct drm_property *degamma_lut_size_property;
+	struct drm_property *ctm_matrix_property;
+	struct drm_property *gamma_lut_property;
+	struct drm_property *gamma_lut_size_property;
+
 	/* properties for virtual machine layout */
 	struct drm_property *suggested_x_property;
 	struct drm_property *suggested_y_property;
@@ -2530,6 +2559,18 @@ static inline struct drm_property *drm_property_find(struct drm_device *dev,
 	return mo ? obj_to_property(mo) : NULL;
 }
 
+/*
+ * Extract a degamma/gamma LUT value provided by user and round it to the
+ * precision supported by the hardware.
+ */
+static inline uint32_t drm_color_lut_get_value(uint32_t user_input,
+					       uint32_t bit_precision)
+{
+	uint32_t val = (user_input << 1) + (1 << (16 - bit_precision));
+
+	return val >> (16 - bit_precision + 1);
+}
+
 /* Plane list iterator for legacy (overlay only) planes. */
 #define drm_for_each_legacy_plane(plane, dev) \
 	list_for_each_entry(plane, &(dev)->mode_config.plane_list, head) \
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index 4b37afa..97fa894 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -48,6 +48,9 @@ extern bool drm_crtc_helper_set_mode(struct drm_crtc *crtc,
 				     struct drm_display_mode *mode,
 				     int x, int y,
 				     struct drm_framebuffer *old_fb);
+extern void drm_helper_crtc_enable_color_mgmt(struct drm_crtc *crtc,
+					      int degamma_lut_size,
+					      int gamma_lut_size);
 extern bool drm_helper_crtc_in_use(struct drm_crtc *crtc);
 extern bool drm_helper_encoder_in_use(struct drm_encoder *encoder);
 
diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
index 50adb46..c021743 100644
--- a/include/uapi/drm/drm_mode.h
+++ b/include/uapi/drm/drm_mode.h
@@ -487,6 +487,21 @@ struct drm_mode_crtc_lut {
 	__u64 blue;
 };
 
+struct drm_color_ctm {
+	/* Conversion matrix in S31.32 format. */
+	__s64 matrix[9];
+};
+
+struct drm_color_lut {
+	/*
+	 * Data is U0.16 fixed point format.
+	 */
+	__u16 red;
+	__u16 green;
+	__u16 blue;
+	__u16 reserved;
+};
+
 #define DRM_MODE_PAGE_FLIP_EVENT 0x01
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_FLAGS (DRM_MODE_PAGE_FLIP_EVENT|DRM_MODE_PAGE_FLIP_ASYNC)
-- 
2.6.4

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