Re: [PATCH] Implement Limited Video Range

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Thanks for your comments, I tried to implement a "flag driven version" below.

On Fri, Nov 11, 2016 at 2:57 PM, Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> wrote:
On Fri, Nov 11, 2016 at 09:53:35AM +0100, Peter Frühberger wrote:
> Hi,
>
> I was implementing this suggestion today and I think that will confuse
> users and also devs maintaining that code. Out of the following reason:
> compress_color_range can be true or false, it does not reference a mode,
> but an additional setting that only influences color clamping / scaling.
>
> What do we expect if someone runs in Full Range and has
> compress_color_range set to true? Also what do we expect if someone runs in
> Limited Range and additionally set this flag to true? In some cases it
> would do nothing and was not transparent.

I didn't mean you should add a new property for this. Just an internal
flag.
[...]

In my opinion that makes maintainability much harder. Looking forward to your comments.
 
From f3bccf1611108247add0d85e8faed5430971cd71 Mon Sep 17 00:00:00 2001
From: fritsch <peter.fruehberger@xxxxxxxxx>
Date: Sat, 16 Sep 2017 13:54:06 +0200
Subject: [PATCH] i915: Implement uncompressed Video Range output

---
 drivers/gpu/drm/i915/i915_drv.h      |  1 +
 drivers/gpu/drm/i915/intel_color.c   | 12 ++++++++----
 drivers/gpu/drm/i915/intel_display.c |  6 ++++--
 drivers/gpu/drm/i915/intel_dp.c      |  3 ++-
 drivers/gpu/drm/i915/intel_drv.h     | 11 +++++++++--
 drivers/gpu/drm/i915/intel_hdmi.c    | 16 +++++++++++++---
 drivers/gpu/drm/i915/intel_modes.c   |  1 +
 7 files changed, 38 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5aecbf795b55..70bd525317c8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -4230,6 +4230,7 @@ __raw_write(64, q)
 #define INTEL_BROADCAST_RGB_AUTO 0
 #define INTEL_BROADCAST_RGB_FULL 1
 #define INTEL_BROADCAST_RGB_LIMITED 2
+#define INTEL_BROADCAST_RGB_VIDEO 3
 
 static inline i915_reg_t i915_vgacntrl_reg(struct drm_i915_private *dev_priv)
 {
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
index ff9ecd211abb..b72477a43ea4 100644
--- a/drivers/gpu/drm/i915/intel_color.c
+++ b/drivers/gpu/drm/i915/intel_color.c
@@ -149,7 +149,8 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state)
 			(struct drm_color_ctm *)crtc_state->ctm->data;
 		uint64_t input[9] = { 0, };
 
-		if (intel_crtc_state->limited_color_range) {
+		if (intel_crtc_state->limited_color_range &&
+				intel_crtc_state->compress_range) {
 			ctm_mult_by_limited(input, ctm->matrix);
 		} else {
 			for (i = 0; i < ARRAY_SIZE(input); i++)
@@ -201,7 +202,8 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state)
 		 * into consideration.
 		 */
 		for (i = 0; i < 3; i++) {
-			if (intel_crtc_state->limited_color_range)
+			if (intel_crtc_state->limited_color_range &&
+					intel_crtc_state->compress_range)
 				coeffs[i * 3 + i] =
 					I9XX_CSC_COEFF_LIMITED_RANGE;
 			else
@@ -225,7 +227,8 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state)
 	if (INTEL_GEN(dev_priv) > 6) {
 		uint16_t postoff = 0;
 
-		if (intel_crtc_state->limited_color_range)
+		if (intel_crtc_state->limited_color_range &&
+				intel_crtc_state->compress_range)
 			postoff = (16 * (1 << 12) / 255) & 0x1fff;
 
 		I915_WRITE(PIPE_CSC_POSTOFF_HI(pipe), postoff);
@@ -236,7 +239,8 @@ static void i9xx_load_csc_matrix(struct drm_crtc_state *crtc_state)
 	} else {
 		uint32_t mode = CSC_MODE_YUV_TO_RGB;
 
-		if (intel_crtc_state->limited_color_range)
+		if (intel_crtc_state->limited_color_range &&
+				intel_crtc_state->compress_range)
 			mode |= CSC_BLACK_SCREEN_OFFSET;
 
 		I915_WRITE(PIPE_CSC_MODE(pipe), mode);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8599e425abb1..4de43eb12f0e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7247,7 +7247,8 @@ static void i9xx_set_pipeconf(struct intel_crtc *intel_crtc)
 		pipeconf |= PIPECONF_PROGRESSIVE;
 
 	if ((IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv)) &&
-	     intel_crtc->config->limited_color_range)
+	     intel_crtc->config->limited_color_range &&
+	     intel_crtc->config->compress_range)
 		pipeconf |= PIPECONF_COLOR_RANGE_SELECT;
 
 	I915_WRITE(PIPECONF(intel_crtc->pipe), pipeconf);
@@ -8177,7 +8178,8 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc)
 	else
 		val |= PIPECONF_PROGRESSIVE;
 
-	if (intel_crtc->config->limited_color_range)
+	if (intel_crtc->config->limited_color_range &&
+			intel_crtc->config->compress_range)
 		val |= PIPECONF_COLOR_RANGE_SELECT;
 
 	I915_WRITE(PIPECONF(pipe), val);
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 887953c0f495..437dd0465be3 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1926,7 +1926,8 @@ static void intel_dp_prepare(struct intel_encoder *encoder,
 			trans_dp &= ~TRANS_DP_ENH_FRAMING;
 		I915_WRITE(TRANS_DP_CTL(crtc->pipe), trans_dp);
 	} else {
-		if (IS_G4X(dev_priv) && pipe_config->limited_color_range)
+		if (IS_G4X(dev_priv) && pipe_config->limited_color_range &&
+				pipe_config->compress_range)
 			intel_dp->DP |= DP_COLOR_RANGE_16_235;
 
 		if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 307807672896..35602a1ed471 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -649,11 +649,18 @@ struct intel_crtc_state {
 	enum transcoder cpu_transcoder;
 
 	/*
-	 * Use reduced/limited/broadcast rbg range, compressing from the full
-	 * range fed into the crtcs.
+	 * Use reduced/limited/broadcast rbg range.
 	 */
 	bool limited_color_range;
 
+	/*
+	 *
+	 * Compress the input range when doing limited/broadcast rgb range
+	 * output. This is the default for Limited 16:235 Output mode.
+	 *
+	 */
+	bool compress_range;
+
 	/* Bitmask of encoder types (enum intel_output_type)
 	 * driven by the pipe.
 	 */
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index e6f8f30ce7bd..4963903f177e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -880,7 +880,8 @@ static void intel_hdmi_prepare(struct intel_encoder *encoder,
 	intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);
 
 	hdmi_val = SDVO_ENCODING_HDMI;
-	if (!HAS_PCH_SPLIT(dev_priv) && crtc_state->limited_color_range)
+	if (!HAS_PCH_SPLIT(dev_priv) && crtc_state->limited_color_range
+			&& crtc_state->compress_range)
 		hdmi_val |= HDMI_COLOR_RANGE_16_235;
 	if (adjusted_mode->flags & DRM_MODE_FLAG_PVSYNC)
 		hdmi_val |= SDVO_VSYNC_ACTIVE_HIGH;
@@ -1419,9 +1420,18 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 			pipe_config->has_hdmi_sink &&
 			drm_default_rgb_quant_range(adjusted_mode) ==
 			HDMI_QUANTIZATION_RANGE_LIMITED;
+		pipe_config->compress_range = pipe_config->limited_color_range;
+	} else if (intel_conn_state->broadcast_rgb
+			== INTEL_BROADCAST_RGB_LIMITED) {
+		pipe_config->limited_color_range = true;
+		pipe_config->compress_range = true;
+	} else if (intel_conn_state->broadcast_rgb
+			== INTEL_BROADCAST_RGB_FULL) {
+		pipe_config->limited_color_range = false;
+		pipe_config->compress_range = false;
 	} else {
-		pipe_config->limited_color_range =
-			intel_conn_state->broadcast_rgb == INTEL_BROADCAST_RGB_LIMITED;
+		pipe_config->limited_color_range = true;
+		pipe_config->compress_range = false;
 	}
 
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK) {
diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
index 951e834dd274..d817558410eb 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -102,6 +102,7 @@ static const struct drm_prop_enum_list broadcast_rgb_names[] = {
 	{ INTEL_BROADCAST_RGB_AUTO, "Automatic" },
 	{ INTEL_BROADCAST_RGB_FULL, "Full" },
 	{ INTEL_BROADCAST_RGB_LIMITED, "Limited 16:235" },
+	{ INTEL_BROADCAST_RGB_VIDEO, "Video 16:235 pass-through" },
 };
 
 void
-- 
2.11.0

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

[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux