Attachment 72746 Details for Bug 46800 – Proposed patch to fix the RGB color range property

[patch] Proposed patch to fix the RGB color range property

0001-drm-i915-Fix-RGB-color-range-property-for-PCH-platfo.patch (text/plain), 7.75 KB, created by Ville Syrjala on 2013-01-09 18:39:06 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Ville Syrjala

Created: 2013-01-09 18:39:06 UTC

Size: 7.75 KB

patch

obsolete

>From 5433ab53513b5563687a0154863b617e50d8bf54 Mon Sep 17 00:00:00 2001
>From: =?UTF-8?q?Ville=20Syrj=C3=A4l=C3=A4?= <ville.syrjala@linux.intel.com>
>Date: Wed, 9 Jan 2013 18:18:00 +0200
>Subject: [PATCH] drm/i915: Fix RGB color range property for PCH platforms
>MIME-Version: 1.0
>Content-Type: text/plain; charset=UTF-8
>Content-Transfer-Encoding: 8bit
>
>The RGB color range select bit on the DP/SDVO/HDMI registers
>disappeared when PCH was introduced, and instead a new PIPECONF bit
>was added that performs the same function.
>
>Add a new intel_encoder function pointer to query the encoder whether
>limited or full range should be selected, and set the PIPECONF bit 13
>accordingly.
>
>Experimentation showed that simply toggling the bit while the pipe is
>active doesn't work. We need to restart the pipe, which luckily already
>happens.
>
>The DP/SDVO/HDMI bit 8 is marked MBZ in the docs, so avoid setting it,
>although it doesn't seem to do any harm in practice.
>
>TODO:
>- the PIPECONF bit too seems to have disappeared from HSW. Need a
>  volunteer to test if it's just a documentation issue or if it's really
>  gone. If the bit is gone and no easy replacement is found, then I suppose
>  we may need to use the pipe CSC unit to perform the range compression.
>- move color_range to intel_encoder to avoid the silly func pointer?
>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=46800
>Signed-off-by: Ville SyrjÃ¤lÃ¤ <ville.syrjala@linux.intel.com>
>---
> drivers/gpu/drm/i915/i915_reg.h      |    1 +
> drivers/gpu/drm/i915/intel_display.c |   19 +++++++++++++++++++
> drivers/gpu/drm/i915/intel_dp.c      |   11 ++++++++++-
> drivers/gpu/drm/i915/intel_drv.h     |    1 +
> drivers/gpu/drm/i915/intel_hdmi.c    |    8 ++++++++
> drivers/gpu/drm/i915/intel_sdvo.c    |   10 +++++++++-
> 6 files changed, 48 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>index 3b039f4..a653b64 100644
>--- a/drivers/gpu/drm/i915/i915_reg.h
>+++ b/drivers/gpu/drm/i915/i915_reg.h
>@@ -2650,6 +2650,7 @@
> #define   PIPECONF_INTERLACED_DBL_ILK		(4 << 21) /* ilk/snb only */
> #define   PIPECONF_PFIT_PF_INTERLACED_DBL_ILK	(5 << 21) /* ilk/snb only */
> #define   PIPECONF_CXSR_DOWNCLOCK	(1<<16)
>+#define   PIPECONF_COLOR_RANGE_SELECT	(1 << 13)
> #define   PIPECONF_BPC_MASK	(0x7 << 5)
> #define   PIPECONF_8BPC		(0<<5)
> #define   PIPECONF_10BPC	(1<<5)
>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>index cf0c713..4023383 100644
>--- a/drivers/gpu/drm/i915/intel_display.c
>+++ b/drivers/gpu/drm/i915/intel_display.c
>@@ -5053,6 +5053,20 @@ static int ironlake_get_refclk(struct drm_crtc *crtc)
> 	return 120000;
> }
> 
>+static bool intel_limited_color_range(struct drm_crtc *crtc)
>+{
>+	struct drm_device *dev = crtc->dev;
>+	struct intel_encoder *intel_encoder;
>+	bool limited_color_range = false;
>+
>+	for_each_encoder_on_crtc(dev, crtc, intel_encoder) {
>+		if (intel_encoder->limited_color_range)
>+			limited_color_range |= intel_encoder->limited_color_range(intel_encoder);
>+	}
>+
>+	return limited_color_range;
>+}
>+
> static void ironlake_set_pipeconf(struct drm_crtc *crtc,
> 				  struct drm_display_mode *adjusted_mode,
> 				  bool dither)
>@@ -5093,6 +5107,11 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
> 	else
> 		val |= PIPECONF_PROGRESSIVE;
> 
>+	if (intel_limited_color_range(crtc))
>+		val |= PIPECONF_COLOR_RANGE_SELECT;
>+	else
>+		val &= ~PIPECONF_COLOR_RANGE_SELECT;
>+
> 	I915_WRITE(PIPECONF(pipe), val);
> 	POSTING_READ(PIPECONF(pipe));
> }
>diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>index 5f12eb2..14cb3d2 100644
>--- a/drivers/gpu/drm/i915/intel_dp.c
>+++ b/drivers/gpu/drm/i915/intel_dp.c
>@@ -967,7 +967,8 @@ intel_dp_mode_set(struct drm_encoder *encoder, struct drm_display_mode *mode,
> 		else
> 			intel_dp->DP |= DP_PLL_FREQ_270MHZ;
> 	} else if (!HAS_PCH_CPT(dev) || is_cpu_edp(intel_dp)) {
>-		intel_dp->DP |= intel_dp->color_range;
>+		if (!HAS_PCH_SPLIT(dev))
>+			intel_dp->DP |= intel_dp->color_range;
> 
> 		if (adjusted_mode->flags & DRM_MODE_FLAG_PHSYNC)
> 			intel_dp->DP |= DP_SYNC_HS_HIGH;
>@@ -1392,6 +1393,13 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
> 	return true;
> }
> 
>+static bool intel_dp_limited_color_range(struct intel_encoder *encoder)
>+{
>+	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>+
>+	return intel_dp->color_range;
>+}
>+
> static void intel_disable_dp(struct intel_encoder *encoder)
> {
> 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
>@@ -2913,6 +2921,7 @@ intel_dp_init(struct drm_device *dev, int output_reg, enum port port)
> 	intel_encoder->disable = intel_disable_dp;
> 	intel_encoder->post_disable = intel_post_disable_dp;
> 	intel_encoder->get_hw_state = intel_dp_get_hw_state;
>+	intel_encoder->limited_color_range = intel_dp_limited_color_range;
> 
> 	intel_dig_port->port = port;
> 	intel_dig_port->dp.output_reg = output_reg;
>diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>index 54a034c..1f9de7c 100644
>--- a/drivers/gpu/drm/i915/intel_drv.h
>+++ b/drivers/gpu/drm/i915/intel_drv.h
>@@ -162,6 +162,7 @@ struct intel_encoder {
> 	 * the encoder is active. If the encoder is enabled it also set the pipe
> 	 * it is connected to in the pipe parameter. */
> 	bool (*get_hw_state)(struct intel_encoder *, enum pipe *pipe);
>+	bool (*limited_color_range)(struct intel_encoder *);
> 	int crtc_mask;
> };
> 
>diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>index 6387f9b..7ef617c 100644
>--- a/drivers/gpu/drm/i915/intel_hdmi.c
>+++ b/drivers/gpu/drm/i915/intel_hdmi.c
>@@ -646,6 +646,13 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
> 	return true;
> }
> 
>+static bool intel_hdmi_limited_color_range(struct intel_encoder *encoder)
>+{
>+	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>+
>+	return intel_hdmi->color_range;
>+}
>+
> static void intel_enable_hdmi(struct intel_encoder *encoder)
> {
> 	struct drm_device *dev = encoder->base.dev;
>@@ -1062,6 +1069,7 @@ void intel_hdmi_init(struct drm_device *dev, int sdvox_reg, enum port port)
> 	intel_encoder->enable = intel_enable_hdmi;
> 	intel_encoder->disable = intel_disable_hdmi;
> 	intel_encoder->get_hw_state = intel_hdmi_get_hw_state;
>+	intel_encoder->limited_color_range = intel_hdmi_limited_color_range;
> 
> 	intel_encoder->type = INTEL_OUTPUT_HDMI;
> 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
>index 153377b..bfe855b 100644
>--- a/drivers/gpu/drm/i915/intel_sdvo.c
>+++ b/drivers/gpu/drm/i915/intel_sdvo.c
>@@ -1153,7 +1153,7 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
> 		/* The real mode polarity is set by the SDVO commands, using
> 		 * struct intel_sdvo_dtd. */
> 		sdvox = SDVO_VSYNC_ACTIVE_HIGH | SDVO_HSYNC_ACTIVE_HIGH;
>-		if (intel_sdvo->is_hdmi)
>+		if (!HAS_PCH_SPLIT(dev) && intel_sdvo->is_hdmi)
> 			sdvox |= intel_sdvo->color_range;
> 		if (INTEL_INFO(dev)->gen < 5)
> 			sdvox |= SDVO_BORDER_ENABLE;
>@@ -1228,6 +1228,13 @@ static bool intel_sdvo_get_hw_state(struct intel_encoder *encoder,
> 	return true;
> }
> 
>+static bool intel_sdvo_limited_color_range(struct intel_encoder *encoder)
>+{
>+	struct intel_sdvo *intel_sdvo = to_intel_sdvo(&encoder->base);
>+
>+	return intel_sdvo->color_range;
>+}
>+
> static void intel_disable_sdvo(struct intel_encoder *encoder)
> {
> 	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>@@ -2746,6 +2753,7 @@ bool intel_sdvo_init(struct drm_device *dev, uint32_t sdvo_reg, bool is_sdvob)
> 	intel_encoder->disable = intel_disable_sdvo;
> 	intel_encoder->enable = intel_enable_sdvo;
> 	intel_encoder->get_hw_state = intel_sdvo_get_hw_state;
>+	intel_encoder->limited_color_range = intel_sdvo_limited_color_range;
> 
> 	/* In default case sdvo lvds is false */
> 	if (!intel_sdvo_get_capabilities(intel_sdvo, &intel_sdvo->caps))
>-- 
>1.7.8.6
>

Actions: View | Splinter Review

Attachments on bug 46800: 72746 | 72776

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.