All of lore.kernel.org
 help / color / mirror / Atom feed
* drm/i915: RGB quantization range stuff v2
@ 2013-01-16 17:12 ville.syrjala
  2013-01-16 17:12 ` [PATCH 1/4] drm/i915: Fix RGB color range property for PCH platforms ville.syrjala
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: ville.syrjala @ 2013-01-16 17:12 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

Second attempt at handling the RGB quantization range for HDMI and DP.

The only change to the first set is that I dropped the
has_hdmi_monitor bool. has_hdmi_sink is used instead.

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [PATCH 1/4] drm/i915: Fix RGB color range property for PCH platforms
  2013-01-16 17:12 drm/i915: RGB quantization range stuff v2 ville.syrjala
@ 2013-01-16 17:12 ` ville.syrjala
  2013-01-17 12:56   ` [Intel-gfx] " Paulo Zanoni
  2013-01-16 17:12 ` [PATCH 2/4] drm/i915: Add "Automatic" mode for the "Broadcast RGB" property ville.syrjala
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: ville.syrjala @ 2013-01-16 17:12 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

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_MODE_LIMITED_COLOR_RANGE private mode flag, and set
it in the encoder mode_fixup if limited color range is requested.
Set the the PIPECONF bit 13 based on the flag.

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.

v2: Use mode private_flags instead of intel_encoder virtual functions
v3: Moved the intel_dp color_range handling after bpc check to help
    later patches

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 |    5 +++++
 drivers/gpu/drm/i915/intel_dp.c      |    7 ++++++-
 drivers/gpu/drm/i915/intel_drv.h     |    5 +++++
 drivers/gpu/drm/i915/intel_hdmi.c    |    5 +++++
 drivers/gpu/drm/i915/intel_sdvo.c    |    5 ++++-
 6 files changed, 26 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 36789bf..a2550c5 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2652,6 +2652,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 c7313f8..f48f698 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5097,6 +5097,11 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
 	else
 		val |= PIPECONF_PROGRESSIVE;
 
+	if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE)
+		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 cf25481..64824d0 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -763,6 +763,10 @@ intel_dp_mode_fixup(struct drm_encoder *encoder,
 		return false;
 
 	bpp = adjusted_mode->private_flags & INTEL_MODE_DP_FORCE_6BPC ? 18 : 24;
+
+	if (intel_dp->color_range)
+		adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
+
 	mode_rate = intel_dp_link_required(adjusted_mode->clock, bpp);
 
 	for (clock = 0; clock <= max_clock; clock++) {
@@ -967,7 +971,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;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 54a034c..4df47be 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -109,6 +109,11 @@
  * timings in the mode to prevent the crtc fixup from overwriting them.
  * Currently only lvds needs that. */
 #define INTEL_MODE_CRTC_TIMINGS_SET (0x20)
+/*
+ * Set when limited 16-235 (as opposed to full 0-255) RGB color range is
+ * to be used.
+ */
+#define INTEL_MODE_LIMITED_COLOR_RANGE (0x40)
 
 static inline void
 intel_mode_set_pixel_multiplier(struct drm_display_mode *mode,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 6387f9b..f194d75 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -766,6 +766,11 @@ bool intel_hdmi_mode_fixup(struct drm_encoder *encoder,
 			   const struct drm_display_mode *mode,
 			   struct drm_display_mode *adjusted_mode)
 {
+	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+
+	if (intel_hdmi->color_range)
+		adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
+
 	return true;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 153377b..3b8491a 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1064,6 +1064,9 @@ static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
 	multiplier = intel_sdvo_get_pixel_multiplier(adjusted_mode);
 	intel_mode_set_pixel_multiplier(adjusted_mode, multiplier);
 
+	if (intel_sdvo->color_range)
+		adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
+
 	return true;
 }
 
@@ -1153,7 +1156,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;
-- 
1.7.8.6

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 2/4] drm/i915: Add "Automatic" mode for the "Broadcast RGB" property
  2013-01-16 17:12 drm/i915: RGB quantization range stuff v2 ville.syrjala
  2013-01-16 17:12 ` [PATCH 1/4] drm/i915: Fix RGB color range property for PCH platforms ville.syrjala
@ 2013-01-16 17:12 ` ville.syrjala
  2013-01-17 13:41   ` Paulo Zanoni
  2013-01-16 17:12 ` [PATCH 3/4] drm/edid: Add drm_rgb_quant_range_selectable() ville.syrjala
  2013-01-16 17:12 ` [PATCH 4/4] drm/i915: Provide the quantization range in the AVI infoframe ville.syrjala
  3 siblings, 1 reply; 10+ messages in thread
From: ville.syrjala @ 2013-01-16 17:12 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add a new "Automatic" mode to the "Broadcast RGB" range property.
When selected the driver automagically selects between full range and
limited range output.

Based on CEA-861 guidelines, limited range output is selected if the
mode is a CEA mode, except 640x480. Otherwise full range output is used.
Additionally DVI monitors should most likely default to full range
always.

As per DP1.2a DisplayPort should always use full range for 18bpp, and
otherwise will follow CEA-861 rules.

NOTE: The default value for the property will now be "Automatic"
so some people may be affected in case they're relying on the
current full range default.

v2: Use has_hdmi_sink to check if a HDMI monitor is present

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h    |    4 +++
 drivers/gpu/drm/i915/intel_dp.c    |   28 ++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_drv.h   |    2 +
 drivers/gpu/drm/i915/intel_hdmi.c  |   29 +++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_modes.c |    5 ++-
 drivers/gpu/drm/i915/intel_sdvo.c  |   38 +++++++++++++++++++++++++++++------
 6 files changed, 89 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d0f051b..449bbe0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1803,5 +1803,9 @@ __i915_write(64, q)
 #define POSTING_READ(reg)	(void)I915_READ_NOTRACE(reg)
 #define POSTING_READ16(reg)	(void)I915_READ16_NOTRACE(reg)
 
+/* "Broadcast RGB" property */
+#define INTEL_BROADCAST_RGB_AUTO 0
+#define INTEL_BROADCAST_RGB_FULL 1
+#define INTEL_BROADCAST_RGB_LIMITED 2
 
 #endif
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 64824d0..633a4db 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -764,6 +764,14 @@ intel_dp_mode_fixup(struct drm_encoder *encoder,
 
 	bpp = adjusted_mode->private_flags & INTEL_MODE_DP_FORCE_6BPC ? 18 : 24;
 
+	if (intel_dp->color_range_auto) {
+		/* as per DP1.2a and CEA-861 */
+		if (bpp != 18 && drm_mode_cea_vic(adjusted_mode) > 1)
+			intel_dp->color_range = DP_COLOR_RANGE_16_235;
+		else
+			intel_dp->color_range = 0;
+	}
+
 	if (intel_dp->color_range)
 		adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
 
@@ -2462,10 +2470,21 @@ intel_dp_set_property(struct drm_connector *connector,
 	}
 
 	if (property == dev_priv->broadcast_rgb_property) {
-		if (val == !!intel_dp->color_range)
-			return 0;
-
-		intel_dp->color_range = val ? DP_COLOR_RANGE_16_235 : 0;
+		switch (val) {
+		case INTEL_BROADCAST_RGB_AUTO:
+			intel_dp->color_range_auto = true;
+			break;
+		case INTEL_BROADCAST_RGB_FULL:
+			intel_dp->color_range_auto = false;
+			intel_dp->color_range = 0;
+			break;
+		case INTEL_BROADCAST_RGB_LIMITED:
+			intel_dp->color_range_auto = false;
+			intel_dp->color_range = DP_COLOR_RANGE_16_235;
+			break;
+		default:
+			return -EINVAL;
+		}
 		goto done;
 	}
 
@@ -2606,6 +2625,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 
 	intel_attach_force_audio_property(connector);
 	intel_attach_broadcast_rgb_property(connector);
+	intel_dp->color_range_auto = true;
 
 	if (is_edp(intel_dp)) {
 		drm_mode_create_scaling_mode_property(connector->dev);
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4df47be..1a698c6 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -343,6 +343,7 @@ struct intel_hdmi {
 	u32 sdvox_reg;
 	int ddc_bus;
 	uint32_t color_range;
+	bool color_range_auto;
 	bool has_hdmi_sink;
 	bool has_audio;
 	enum hdmi_force_audio force_audio;
@@ -362,6 +363,7 @@ struct intel_dp {
 	bool has_audio;
 	enum hdmi_force_audio force_audio;
 	uint32_t color_range;
+	bool color_range_auto;
 	uint8_t link_bw;
 	uint8_t lane_count;
 	uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index f194d75..58b072e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -768,6 +768,15 @@ bool intel_hdmi_mode_fixup(struct drm_encoder *encoder,
 {
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 
+	if (intel_hdmi->color_range_auto) {
+		/* as per CEA-861 */
+		if (intel_hdmi->has_hdmi_sink &&
+		    drm_mode_cea_vic(adjusted_mode) > 1)
+			intel_hdmi->color_range = SDVO_COLOR_RANGE_16_235;
+		else
+			intel_hdmi->color_range = 0;
+	}
+
 	if (intel_hdmi->color_range)
 		adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
 
@@ -912,10 +921,21 @@ intel_hdmi_set_property(struct drm_connector *connector,
 	}
 
 	if (property == dev_priv->broadcast_rgb_property) {
-		if (val == !!intel_hdmi->color_range)
-			return 0;
-
-		intel_hdmi->color_range = val ? SDVO_COLOR_RANGE_16_235 : 0;
+		switch (val) {
+		case INTEL_BROADCAST_RGB_AUTO:
+			intel_hdmi->color_range_auto = true;
+			break;
+		case INTEL_BROADCAST_RGB_FULL:
+			intel_hdmi->color_range_auto = false;
+			intel_hdmi->color_range = 0;
+			break;
+		case INTEL_BROADCAST_RGB_LIMITED:
+			intel_hdmi->color_range_auto = false;
+			intel_hdmi->color_range = SDVO_COLOR_RANGE_16_235;
+			break;
+		default:
+			return -EINVAL;
+		}
 		goto done;
 	}
 
@@ -964,6 +984,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
 {
 	intel_attach_force_audio_property(connector);
 	intel_attach_broadcast_rgb_property(connector);
+	intel_hdmi->color_range_auto = true;
 }
 
 void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
index 49249bb..0e860f3 100644
--- a/drivers/gpu/drm/i915/intel_modes.c
+++ b/drivers/gpu/drm/i915/intel_modes.c
@@ -100,8 +100,9 @@ intel_attach_force_audio_property(struct drm_connector *connector)
 }
 
 static const struct drm_prop_enum_list broadcast_rgb_names[] = {
-	{ 0, "Full" },
-	{ 1, "Limited 16:235" },
+	{ INTEL_BROADCAST_RGB_AUTO, "Automatic" },
+	{ INTEL_BROADCAST_RGB_FULL, "Full" },
+	{ INTEL_BROADCAST_RGB_LIMITED, "Limited 16:235" },
 };
 
 void
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 3b8491a..b422109 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -103,6 +103,7 @@ struct intel_sdvo {
 	 * It is only valid when using TMDS encoding and 8 bit per color mode.
 	 */
 	uint32_t color_range;
+	bool color_range_auto;
 
 	/**
 	 * This is set if we're going to treat the device as TV-out.
@@ -1064,6 +1065,15 @@ static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
 	multiplier = intel_sdvo_get_pixel_multiplier(adjusted_mode);
 	intel_mode_set_pixel_multiplier(adjusted_mode, multiplier);
 
+	if (intel_sdvo->color_range_auto) {
+		/* as per CEA-861 */
+		if (intel_sdvo->has_hdmi_monitor &&
+		    drm_mode_cea_vic(adjusted_mode) > 1)
+			intel_sdvo->color_range = SDVO_COLOR_RANGE_16_235;
+		else
+			intel_sdvo->color_range = 0;
+	}
+
 	if (intel_sdvo->color_range)
 		adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
 
@@ -1900,10 +1910,21 @@ intel_sdvo_set_property(struct drm_connector *connector,
 	}
 
 	if (property == dev_priv->broadcast_rgb_property) {
-		if (val == !!intel_sdvo->color_range)
-			return 0;
-
-		intel_sdvo->color_range = val ? SDVO_COLOR_RANGE_16_235 : 0;
+		switch (val) {
+		case INTEL_BROADCAST_RGB_AUTO:
+			intel_sdvo->color_range_auto = true;
+			break;
+		case INTEL_BROADCAST_RGB_FULL:
+			intel_sdvo->color_range_auto = false;
+			intel_sdvo->color_range = 0;
+			break;
+		case INTEL_BROADCAST_RGB_LIMITED:
+			intel_sdvo->color_range_auto = false;
+			intel_sdvo->color_range = SDVO_COLOR_RANGE_16_235;
+			break;
+		default:
+			return -EINVAL;
+		}
 		goto done;
 	}
 
@@ -2200,13 +2221,16 @@ intel_sdvo_connector_init(struct intel_sdvo_connector *connector,
 }
 
 static void
-intel_sdvo_add_hdmi_properties(struct intel_sdvo_connector *connector)
+intel_sdvo_add_hdmi_properties(struct intel_sdvo *intel_sdvo,
+			       struct intel_sdvo_connector *connector)
 {
 	struct drm_device *dev = connector->base.base.dev;
 
 	intel_attach_force_audio_property(&connector->base.base);
-	if (INTEL_INFO(dev)->gen >= 4 && IS_MOBILE(dev))
+	if (INTEL_INFO(dev)->gen >= 4 && IS_MOBILE(dev)) {
 		intel_attach_broadcast_rgb_property(&connector->base.base);
+		intel_sdvo->color_range_auto = true;
+	}
 }
 
 static bool
@@ -2254,7 +2278,7 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
 
 	intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo);
 	if (intel_sdvo->is_hdmi)
-		intel_sdvo_add_hdmi_properties(intel_sdvo_connector);
+		intel_sdvo_add_hdmi_properties(intel_sdvo, intel_sdvo_connector);
 
 	return true;
 }
-- 
1.7.8.6

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 3/4] drm/edid: Add drm_rgb_quant_range_selectable()
  2013-01-16 17:12 drm/i915: RGB quantization range stuff v2 ville.syrjala
  2013-01-16 17:12 ` [PATCH 1/4] drm/i915: Fix RGB color range property for PCH platforms ville.syrjala
  2013-01-16 17:12 ` [PATCH 2/4] drm/i915: Add "Automatic" mode for the "Broadcast RGB" property ville.syrjala
@ 2013-01-16 17:12 ` ville.syrjala
  2013-01-17 12:12   ` [Intel-gfx] " Paulo Zanoni
  2013-01-16 17:12 ` [PATCH 4/4] drm/i915: Provide the quantization range in the AVI infoframe ville.syrjala
  3 siblings, 1 reply; 10+ messages in thread
From: ville.syrjala @ 2013-01-16 17:12 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

drm_rgb_quant_range_selectable() will report whether the monitor
claims to support for RGB quantization range selection.

The information can be found in the CEA Video capability block.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c |   33 +++++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h     |    1 +
 2 files changed, 34 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 5a3770f..deba722 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -1483,9 +1483,11 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
 #define VIDEO_BLOCK     0x02
 #define VENDOR_BLOCK    0x03
 #define SPEAKER_BLOCK	0x04
+#define VIDEO_CAPABILITY_BLOCK	0x07
 #define EDID_BASIC_AUDIO	(1 << 6)
 #define EDID_CEA_YCRCB444	(1 << 5)
 #define EDID_CEA_YCRCB422	(1 << 4)
+#define EDID_CEA_VCDB_QS	(1 << 6)
 
 /**
  * Search EDID for CEA extension block.
@@ -1902,6 +1904,37 @@ end:
 EXPORT_SYMBOL(drm_detect_monitor_audio);
 
 /**
+ * drm_rgb_quant_range_selectable - is RGB quantization range selectable?
+ *
+ * Check whether the monitor reports the RGB quantization range selection
+ * as supported. The AVI infoframe can then be used to inform the monitor
+ * which quantzation range (full or limited) is used.
+ */
+bool drm_rgb_quant_range_selectable(struct edid *edid)
+{
+	u8 *edid_ext;
+	int i, start, end;
+
+	edid_ext = drm_find_cea_extension(edid);
+	if (!edid_ext)
+		return false;
+
+	if (cea_db_offsets(edid_ext, &start, &end))
+		return false;
+
+	for_each_cea_db(edid_ext, i, start, end) {
+		if (cea_db_tag(&edid_ext[i]) == VIDEO_CAPABILITY_BLOCK &&
+		    cea_db_payload_len(&edid_ext[i]) == 2) {
+			DRM_DEBUG_KMS("CEA VCDB 0x%02x\n", edid_ext[i + 2]);
+			return edid_ext[i + 2] & EDID_CEA_VCDB_QS;
+		}
+	}
+
+	return false;
+}
+EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
+
+/**
  * drm_add_display_info - pull display info out if present
  * @edid: EDID data
  * @info: display info (attached to connector)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 00d78b5..30892dc 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1033,6 +1033,7 @@ extern u8 *drm_find_cea_extension(struct edid *edid);
 extern u8 drm_match_cea_mode(struct drm_display_mode *to_match);
 extern bool drm_detect_hdmi_monitor(struct edid *edid);
 extern bool drm_detect_monitor_audio(struct edid *edid);
+extern bool drm_rgb_quant_range_selectable(struct edid *edid);
 extern int drm_mode_page_flip_ioctl(struct drm_device *dev,
 				    void *data, struct drm_file *file_priv);
 extern struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
-- 
1.7.8.6

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* [PATCH 4/4] drm/i915: Provide the quantization range in the AVI infoframe
  2013-01-16 17:12 drm/i915: RGB quantization range stuff v2 ville.syrjala
                   ` (2 preceding siblings ...)
  2013-01-16 17:12 ` [PATCH 3/4] drm/edid: Add drm_rgb_quant_range_selectable() ville.syrjala
@ 2013-01-16 17:12 ` ville.syrjala
  2013-01-17 12:16   ` Paulo Zanoni
  3 siblings, 1 reply; 10+ messages in thread
From: ville.syrjala @ 2013-01-16 17:12 UTC (permalink / raw)
  To: intel-gfx; +Cc: dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The AVI infoframe is able to inform the display whether the source is
sending full or limited range RGB data.

As per CEA-861 we must first check whether the display reports the
quantization range as selectable, and if so we can set the approriate
bits in the AVI inforframe.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h  |    3 +++
 drivers/gpu/drm/i915/intel_hdmi.c |   11 +++++++++++
 drivers/gpu/drm/i915/intel_sdvo.c |   16 ++++++++++++++--
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 1a698c6..c5251d9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -289,6 +289,8 @@ struct cxsr_latency {
 #define DIP_LEN_AVI     13
 #define DIP_AVI_PR_1    0
 #define DIP_AVI_PR_2    1
+#define DIP_AVI_Q0	0x4
+#define DIP_AVI_Q1	0x8
 
 #define DIP_TYPE_SPD	0x83
 #define DIP_VERSION_SPD	0x1
@@ -347,6 +349,7 @@ struct intel_hdmi {
 	bool has_hdmi_sink;
 	bool has_audio;
 	enum hdmi_force_audio force_audio;
+	bool rgb_quant_range_selectable;
 	void (*write_infoframe)(struct drm_encoder *encoder,
 				struct dip_infoframe *frame);
 	void (*set_infoframes)(struct drm_encoder *encoder,
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 58b072e..270d7ee 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -331,6 +331,7 @@ static void intel_set_infoframe(struct drm_encoder *encoder,
 static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 					 struct drm_display_mode *adjusted_mode)
 {
+	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
 	struct dip_infoframe avi_if = {
 		.type = DIP_TYPE_AVI,
 		.ver = DIP_VERSION_AVI,
@@ -340,6 +341,13 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 	if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
 		avi_if.body.avi.YQ_CN_PR |= DIP_AVI_PR_2;
 
+	if (intel_hdmi->rgb_quant_range_selectable) {
+		if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE)
+			avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q0;
+		else
+			avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q1;
+	}
+
 	avi_if.body.avi.VIC = drm_mode_cea_vic(adjusted_mode);
 
 	intel_set_infoframe(encoder, &avi_if);
@@ -825,6 +833,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 
 	intel_hdmi->has_hdmi_sink = false;
 	intel_hdmi->has_audio = false;
+	intel_hdmi->rgb_quant_range_selectable = false;
 	edid = drm_get_edid(connector,
 			    intel_gmbus_get_adapter(dev_priv,
 						    intel_hdmi->ddc_bus));
@@ -836,6 +845,8 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 				intel_hdmi->has_hdmi_sink =
 						drm_detect_hdmi_monitor(edid);
 			intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
+			intel_hdmi->rgb_quant_range_selectable =
+				drm_rgb_quant_range_selectable(edid);
 		}
 		kfree(edid);
 	}
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index b422109..af93999 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -126,6 +126,7 @@ struct intel_sdvo {
 	bool is_hdmi;
 	bool has_hdmi_monitor;
 	bool has_hdmi_audio;
+	bool rgb_quant_range_selectable;
 
 	/**
 	 * This is set if we detect output of sdvo device as LVDS and
@@ -947,7 +948,8 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
 				    &tx_rate, 1);
 }
 
-static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo)
+static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
+					 const struct drm_display_mode *adjusted_mode)
 {
 	struct dip_infoframe avi_if = {
 		.type = DIP_TYPE_AVI,
@@ -956,6 +958,13 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo)
 	};
 	uint8_t sdvo_data[4 + sizeof(avi_if.body.avi)];
 
+	if (intel_sdvo->rgb_quant_range_selectable) {
+		if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE)
+			avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q0;
+		else
+			avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q1;
+	}
+
 	intel_dip_infoframe_csum(&avi_if);
 
 	/* sdvo spec says that the ecc is handled by the hw, and it looks like
@@ -1134,7 +1143,7 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
 		intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_HDMI);
 		intel_sdvo_set_colorimetry(intel_sdvo,
 					   SDVO_COLORIMETRY_RGB256);
-		intel_sdvo_set_avi_infoframe(intel_sdvo);
+		intel_sdvo_set_avi_infoframe(intel_sdvo, adjusted_mode);
 	} else
 		intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_DVI);
 
@@ -1526,6 +1535,8 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector)
 			if (intel_sdvo->is_hdmi) {
 				intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid);
 				intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid);
+				intel_sdvo->rgb_quant_range_selectable =
+					drm_rgb_quant_range_selectable(edid);
 			}
 		} else
 			status = connector_status_disconnected;
@@ -1577,6 +1588,7 @@ intel_sdvo_detect(struct drm_connector *connector, bool force)
 
 	intel_sdvo->has_hdmi_monitor = false;
 	intel_sdvo->has_hdmi_audio = false;
+	intel_sdvo->rgb_quant_range_selectable = false;
 
 	if ((intel_sdvo_connector->output_flag & response) == 0)
 		ret = connector_status_disconnected;
-- 
1.7.8.6

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

^ permalink raw reply related	[flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH 3/4] drm/edid: Add drm_rgb_quant_range_selectable()
  2013-01-16 17:12 ` [PATCH 3/4] drm/edid: Add drm_rgb_quant_range_selectable() ville.syrjala
@ 2013-01-17 12:12   ` Paulo Zanoni
  0 siblings, 0 replies; 10+ messages in thread
From: Paulo Zanoni @ 2013-01-17 12:12 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, dri-devel

Hi

2013/1/16  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> drm_rgb_quant_range_selectable() will report whether the monitor
> claims to support for RGB quantization range selection.
>
> The information can be found in the CEA Video capability block.

Looks correct (checked against the spec, did not really test).

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

See below for optional bikeshedding:

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c |   33 +++++++++++++++++++++++++++++++++
>  include/drm/drm_crtc.h     |    1 +
>  2 files changed, 34 insertions(+), 0 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 5a3770f..deba722 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -1483,9 +1483,11 @@ add_detailed_modes(struct drm_connector *connector, struct edid *edid,
>  #define VIDEO_BLOCK     0x02
>  #define VENDOR_BLOCK    0x03
>  #define SPEAKER_BLOCK  0x04
> +#define VIDEO_CAPABILITY_BLOCK 0x07
>  #define EDID_BASIC_AUDIO       (1 << 6)
>  #define EDID_CEA_YCRCB444      (1 << 5)
>  #define EDID_CEA_YCRCB422      (1 << 4)
> +#define EDID_CEA_VCDB_QS       (1 << 6)
>
>  /**
>   * Search EDID for CEA extension block.
> @@ -1902,6 +1904,37 @@ end:
>  EXPORT_SYMBOL(drm_detect_monitor_audio);
>
>  /**
> + * drm_rgb_quant_range_selectable - is RGB quantization range selectable?
> + *
> + * Check whether the monitor reports the RGB quantization range selection
> + * as supported. The AVI infoframe can then be used to inform the monitor
> + * which quantzation range (full or limited) is used.

s/quantzation/quantization/


> + */
> +bool drm_rgb_quant_range_selectable(struct edid *edid)
> +{
> +       u8 *edid_ext;
> +       int i, start, end;
> +
> +       edid_ext = drm_find_cea_extension(edid);
> +       if (!edid_ext)
> +               return false;
> +
> +       if (cea_db_offsets(edid_ext, &start, &end))
> +               return false;
> +
> +       for_each_cea_db(edid_ext, i, start, end) {
> +               if (cea_db_tag(&edid_ext[i]) == VIDEO_CAPABILITY_BLOCK &&
> +                   cea_db_payload_len(&edid_ext[i]) == 2) {
> +                       DRM_DEBUG_KMS("CEA VCDB 0x%02x\n", edid_ext[i + 2]);
> +                       return edid_ext[i + 2] & EDID_CEA_VCDB_QS;
> +               }
> +       }
> +
> +       return false;
> +}
> +EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
> +
> +/**
>   * drm_add_display_info - pull display info out if present
>   * @edid: EDID data
>   * @info: display info (attached to connector)
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 00d78b5..30892dc 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1033,6 +1033,7 @@ extern u8 *drm_find_cea_extension(struct edid *edid);
>  extern u8 drm_match_cea_mode(struct drm_display_mode *to_match);
>  extern bool drm_detect_hdmi_monitor(struct edid *edid);
>  extern bool drm_detect_monitor_audio(struct edid *edid);
> +extern bool drm_rgb_quant_range_selectable(struct edid *edid);
>  extern int drm_mode_page_flip_ioctl(struct drm_device *dev,
>                                     void *data, struct drm_file *file_priv);
>  extern struct drm_display_mode *drm_cvt_mode(struct drm_device *dev,
> --
> 1.7.8.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] drm/i915: Provide the quantization range in the AVI infoframe
  2013-01-16 17:12 ` [PATCH 4/4] drm/i915: Provide the quantization range in the AVI infoframe ville.syrjala
@ 2013-01-17 12:16   ` Paulo Zanoni
  2013-01-17 12:37     ` Ville Syrjälä
  0 siblings, 1 reply; 10+ messages in thread
From: Paulo Zanoni @ 2013-01-17 12:16 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, dri-devel

Hi

2013/1/16  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The AVI infoframe is able to inform the display whether the source is
> sending full or limited range RGB data.
>
> As per CEA-861 we must first check whether the display reports the
> quantization range as selectable, and if so we can set the approriate
> bits in the AVI inforframe.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h  |    3 +++
>  drivers/gpu/drm/i915/intel_hdmi.c |   11 +++++++++++
>  drivers/gpu/drm/i915/intel_sdvo.c |   16 ++++++++++++++--
>  3 files changed, 28 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 1a698c6..c5251d9 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -289,6 +289,8 @@ struct cxsr_latency {
>  #define DIP_LEN_AVI     13
>  #define DIP_AVI_PR_1    0
>  #define DIP_AVI_PR_2    1
> +#define DIP_AVI_Q0     0x4
> +#define DIP_AVI_Q1     0x8

<bikeshedding optional="true">

I'd define more descriptive names here... How about the following?
#define DIP_AVI_QUANTIZATION_DEFAULT (0 << 2)
#define DIP_AVI_QUANTIZATION_LIMITED (1 << 2)
#define DIP_AVI_QUANTIZATION_FULL (2 << 2)

<bikeshedding optional="true">

Everything else looks fine.

With or without that:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

Also, these things kinda conflict with Thierry's patches, but I know
you're aware because you've reviewed some of them :)

>
>  #define DIP_TYPE_SPD   0x83
>  #define DIP_VERSION_SPD        0x1
> @@ -347,6 +349,7 @@ struct intel_hdmi {
>         bool has_hdmi_sink;
>         bool has_audio;
>         enum hdmi_force_audio force_audio;
> +       bool rgb_quant_range_selectable;
>         void (*write_infoframe)(struct drm_encoder *encoder,
>                                 struct dip_infoframe *frame);
>         void (*set_infoframes)(struct drm_encoder *encoder,
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 58b072e..270d7ee 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -331,6 +331,7 @@ static void intel_set_infoframe(struct drm_encoder *encoder,
>  static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
>                                          struct drm_display_mode *adjusted_mode)
>  {
> +       struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>         struct dip_infoframe avi_if = {
>                 .type = DIP_TYPE_AVI,
>                 .ver = DIP_VERSION_AVI,
> @@ -340,6 +341,13 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
>         if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
>                 avi_if.body.avi.YQ_CN_PR |= DIP_AVI_PR_2;
>
> +       if (intel_hdmi->rgb_quant_range_selectable) {
> +               if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE)
> +                       avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q0;
> +               else
> +                       avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q1;
> +       }
> +
>         avi_if.body.avi.VIC = drm_mode_cea_vic(adjusted_mode);
>
>         intel_set_infoframe(encoder, &avi_if);
> @@ -825,6 +833,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>
>         intel_hdmi->has_hdmi_sink = false;
>         intel_hdmi->has_audio = false;
> +       intel_hdmi->rgb_quant_range_selectable = false;
>         edid = drm_get_edid(connector,
>                             intel_gmbus_get_adapter(dev_priv,
>                                                     intel_hdmi->ddc_bus));
> @@ -836,6 +845,8 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>                                 intel_hdmi->has_hdmi_sink =
>                                                 drm_detect_hdmi_monitor(edid);
>                         intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
> +                       intel_hdmi->rgb_quant_range_selectable =
> +                               drm_rgb_quant_range_selectable(edid);
>                 }
>                 kfree(edid);
>         }
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index b422109..af93999 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -126,6 +126,7 @@ struct intel_sdvo {
>         bool is_hdmi;
>         bool has_hdmi_monitor;
>         bool has_hdmi_audio;
> +       bool rgb_quant_range_selectable;
>
>         /**
>          * This is set if we detect output of sdvo device as LVDS and
> @@ -947,7 +948,8 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
>                                     &tx_rate, 1);
>  }
>
> -static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo)
> +static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
> +                                        const struct drm_display_mode *adjusted_mode)
>  {
>         struct dip_infoframe avi_if = {
>                 .type = DIP_TYPE_AVI,
> @@ -956,6 +958,13 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo)
>         };
>         uint8_t sdvo_data[4 + sizeof(avi_if.body.avi)];
>
> +       if (intel_sdvo->rgb_quant_range_selectable) {
> +               if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE)
> +                       avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q0;
> +               else
> +                       avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q1;
> +       }
> +
>         intel_dip_infoframe_csum(&avi_if);
>
>         /* sdvo spec says that the ecc is handled by the hw, and it looks like
> @@ -1134,7 +1143,7 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
>                 intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_HDMI);
>                 intel_sdvo_set_colorimetry(intel_sdvo,
>                                            SDVO_COLORIMETRY_RGB256);
> -               intel_sdvo_set_avi_infoframe(intel_sdvo);
> +               intel_sdvo_set_avi_infoframe(intel_sdvo, adjusted_mode);
>         } else
>                 intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_DVI);
>
> @@ -1526,6 +1535,8 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector)
>                         if (intel_sdvo->is_hdmi) {
>                                 intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid);
>                                 intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid);
> +                               intel_sdvo->rgb_quant_range_selectable =
> +                                       drm_rgb_quant_range_selectable(edid);
>                         }
>                 } else
>                         status = connector_status_disconnected;
> @@ -1577,6 +1588,7 @@ intel_sdvo_detect(struct drm_connector *connector, bool force)
>
>         intel_sdvo->has_hdmi_monitor = false;
>         intel_sdvo->has_hdmi_audio = false;
> +       intel_sdvo->rgb_quant_range_selectable = false;
>
>         if ((intel_sdvo_connector->output_flag & response) == 0)
>                 ret = connector_status_disconnected;
> --
> 1.7.8.6
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 4/4] drm/i915: Provide the quantization range in the AVI infoframe
  2013-01-17 12:16   ` Paulo Zanoni
@ 2013-01-17 12:37     ` Ville Syrjälä
  0 siblings, 0 replies; 10+ messages in thread
From: Ville Syrjälä @ 2013-01-17 12:37 UTC (permalink / raw)
  To: Paulo Zanoni; +Cc: intel-gfx, dri-devel

On Thu, Jan 17, 2013 at 10:16:46AM -0200, Paulo Zanoni wrote:
> Hi
> 
> 2013/1/16  <ville.syrjala@linux.intel.com>:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > The AVI infoframe is able to inform the display whether the source is
> > sending full or limited range RGB data.
> >
> > As per CEA-861 we must first check whether the display reports the
> > quantization range as selectable, and if so we can set the approriate
> > bits in the AVI inforframe.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_drv.h  |    3 +++
> >  drivers/gpu/drm/i915/intel_hdmi.c |   11 +++++++++++
> >  drivers/gpu/drm/i915/intel_sdvo.c |   16 ++++++++++++++--
> >  3 files changed, 28 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 1a698c6..c5251d9 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -289,6 +289,8 @@ struct cxsr_latency {
> >  #define DIP_LEN_AVI     13
> >  #define DIP_AVI_PR_1    0
> >  #define DIP_AVI_PR_2    1
> > +#define DIP_AVI_Q0     0x4
> > +#define DIP_AVI_Q1     0x8
> 
> <bikeshedding optional="true">
> 
> I'd define more descriptive names here... How about the following?
> #define DIP_AVI_QUANTIZATION_DEFAULT (0 << 2)
> #define DIP_AVI_QUANTIZATION_LIMITED (1 << 2)
> #define DIP_AVI_QUANTIZATION_FULL (2 << 2)

Sure, that seems like sensible idea. I think I'll want the fact that
these only refer to RGB to be included in the name though.

So something like this probably:
DIP_AVI_RGB_QUANT_RANGE_DEFAULT
DIP_AVI_RGB_QUANT_RANGE_LIMITED
DIP_AVI_RGB_QUANT_RANGE_FULL

> <bikeshedding optional="true">
> 
> Everything else looks fine.
> 
> With or without that:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> 
> Also, these things kinda conflict with Thierry's patches, but I know
> you're aware because you've reviewed some of them :)

Yeah. BTW are we (as in i915 folks) mostly OK with those?

> >  #define DIP_TYPE_SPD   0x83
> >  #define DIP_VERSION_SPD        0x1
> > @@ -347,6 +349,7 @@ struct intel_hdmi {
> >         bool has_hdmi_sink;
> >         bool has_audio;
> >         enum hdmi_force_audio force_audio;
> > +       bool rgb_quant_range_selectable;
> >         void (*write_infoframe)(struct drm_encoder *encoder,
> >                                 struct dip_infoframe *frame);
> >         void (*set_infoframes)(struct drm_encoder *encoder,
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 58b072e..270d7ee 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -331,6 +331,7 @@ static void intel_set_infoframe(struct drm_encoder *encoder,
> >  static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
> >                                          struct drm_display_mode *adjusted_mode)
> >  {
> > +       struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> >         struct dip_infoframe avi_if = {
> >                 .type = DIP_TYPE_AVI,
> >                 .ver = DIP_VERSION_AVI,
> > @@ -340,6 +341,13 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
> >         if (adjusted_mode->flags & DRM_MODE_FLAG_DBLCLK)
> >                 avi_if.body.avi.YQ_CN_PR |= DIP_AVI_PR_2;
> >
> > +       if (intel_hdmi->rgb_quant_range_selectable) {
> > +               if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE)
> > +                       avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q0;
> > +               else
> > +                       avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q1;
> > +       }
> > +
> >         avi_if.body.avi.VIC = drm_mode_cea_vic(adjusted_mode);
> >
> >         intel_set_infoframe(encoder, &avi_if);
> > @@ -825,6 +833,7 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> >
> >         intel_hdmi->has_hdmi_sink = false;
> >         intel_hdmi->has_audio = false;
> > +       intel_hdmi->rgb_quant_range_selectable = false;
> >         edid = drm_get_edid(connector,
> >                             intel_gmbus_get_adapter(dev_priv,
> >                                                     intel_hdmi->ddc_bus));
> > @@ -836,6 +845,8 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> >                                 intel_hdmi->has_hdmi_sink =
> >                                                 drm_detect_hdmi_monitor(edid);
> >                         intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
> > +                       intel_hdmi->rgb_quant_range_selectable =
> > +                               drm_rgb_quant_range_selectable(edid);
> >                 }
> >                 kfree(edid);
> >         }
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > index b422109..af93999 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -126,6 +126,7 @@ struct intel_sdvo {
> >         bool is_hdmi;
> >         bool has_hdmi_monitor;
> >         bool has_hdmi_audio;
> > +       bool rgb_quant_range_selectable;
> >
> >         /**
> >          * This is set if we detect output of sdvo device as LVDS and
> > @@ -947,7 +948,8 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
> >                                     &tx_rate, 1);
> >  }
> >
> > -static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo)
> > +static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
> > +                                        const struct drm_display_mode *adjusted_mode)
> >  {
> >         struct dip_infoframe avi_if = {
> >                 .type = DIP_TYPE_AVI,
> > @@ -956,6 +958,13 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo)
> >         };
> >         uint8_t sdvo_data[4 + sizeof(avi_if.body.avi)];
> >
> > +       if (intel_sdvo->rgb_quant_range_selectable) {
> > +               if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE)
> > +                       avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q0;
> > +               else
> > +                       avi_if.body.avi.ITC_EC_Q_SC |= DIP_AVI_Q1;
> > +       }
> > +
> >         intel_dip_infoframe_csum(&avi_if);
> >
> >         /* sdvo spec says that the ecc is handled by the hw, and it looks like
> > @@ -1134,7 +1143,7 @@ static void intel_sdvo_mode_set(struct drm_encoder *encoder,
> >                 intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_HDMI);
> >                 intel_sdvo_set_colorimetry(intel_sdvo,
> >                                            SDVO_COLORIMETRY_RGB256);
> > -               intel_sdvo_set_avi_infoframe(intel_sdvo);
> > +               intel_sdvo_set_avi_infoframe(intel_sdvo, adjusted_mode);
> >         } else
> >                 intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_DVI);
> >
> > @@ -1526,6 +1535,8 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector)
> >                         if (intel_sdvo->is_hdmi) {
> >                                 intel_sdvo->has_hdmi_monitor = drm_detect_hdmi_monitor(edid);
> >                                 intel_sdvo->has_hdmi_audio = drm_detect_monitor_audio(edid);
> > +                               intel_sdvo->rgb_quant_range_selectable =
> > +                                       drm_rgb_quant_range_selectable(edid);
> >                         }
> >                 } else
> >                         status = connector_status_disconnected;
> > @@ -1577,6 +1588,7 @@ intel_sdvo_detect(struct drm_connector *connector, bool force)
> >
> >         intel_sdvo->has_hdmi_monitor = false;
> >         intel_sdvo->has_hdmi_audio = false;
> > +       intel_sdvo->rgb_quant_range_selectable = false;
> >
> >         if ((intel_sdvo_connector->output_flag & response) == 0)
> >                 ret = connector_status_disconnected;
> > --
> > 1.7.8.6
> >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/dri-devel
> 
> 
> 
> -- 
> Paulo Zanoni

-- 
Ville Syrjälä
Intel OTC

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH 1/4] drm/i915: Fix RGB color range property for PCH platforms
  2013-01-16 17:12 ` [PATCH 1/4] drm/i915: Fix RGB color range property for PCH platforms ville.syrjala
@ 2013-01-17 12:56   ` Paulo Zanoni
  0 siblings, 0 replies; 10+ messages in thread
From: Paulo Zanoni @ 2013-01-17 12:56 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, dri-devel

Hi

2013/1/16  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> 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_MODE_LIMITED_COLOR_RANGE private mode flag, and set
> it in the encoder mode_fixup if limited color range is requested.
> Set the the PIPECONF bit 13 based on the flag.
>
> 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.
>
> v2: Use mode private_flags instead of intel_encoder virtual functions
> v3: Moved the intel_dp color_range handling after bpc check to help
>     later patches

Things look correct. As you mention, the only problem seems to be the
Haswell. I could not find anything on the specs, so I think we should
send some emails and maybe consider removing the property on Haswell?

If-you-promise-to-find-a-solution-for-the-Haswell-case:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> 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 |    5 +++++
>  drivers/gpu/drm/i915/intel_dp.c      |    7 ++++++-
>  drivers/gpu/drm/i915/intel_drv.h     |    5 +++++
>  drivers/gpu/drm/i915/intel_hdmi.c    |    5 +++++
>  drivers/gpu/drm/i915/intel_sdvo.c    |    5 ++++-
>  6 files changed, 26 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 36789bf..a2550c5 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2652,6 +2652,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 c7313f8..f48f698 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -5097,6 +5097,11 @@ static void ironlake_set_pipeconf(struct drm_crtc *crtc,
>         else
>                 val |= PIPECONF_PROGRESSIVE;
>
> +       if (adjusted_mode->private_flags & INTEL_MODE_LIMITED_COLOR_RANGE)
> +               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 cf25481..64824d0 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -763,6 +763,10 @@ intel_dp_mode_fixup(struct drm_encoder *encoder,
>                 return false;
>
>         bpp = adjusted_mode->private_flags & INTEL_MODE_DP_FORCE_6BPC ? 18 : 24;
> +
> +       if (intel_dp->color_range)
> +               adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
> +
>         mode_rate = intel_dp_link_required(adjusted_mode->clock, bpp);
>
>         for (clock = 0; clock <= max_clock; clock++) {
> @@ -967,7 +971,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;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 54a034c..4df47be 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -109,6 +109,11 @@
>   * timings in the mode to prevent the crtc fixup from overwriting them.
>   * Currently only lvds needs that. */
>  #define INTEL_MODE_CRTC_TIMINGS_SET (0x20)
> +/*
> + * Set when limited 16-235 (as opposed to full 0-255) RGB color range is
> + * to be used.
> + */
> +#define INTEL_MODE_LIMITED_COLOR_RANGE (0x40)
>
>  static inline void
>  intel_mode_set_pixel_multiplier(struct drm_display_mode *mode,
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 6387f9b..f194d75 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -766,6 +766,11 @@ bool intel_hdmi_mode_fixup(struct drm_encoder *encoder,
>                            const struct drm_display_mode *mode,
>                            struct drm_display_mode *adjusted_mode)
>  {
> +       struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> +
> +       if (intel_hdmi->color_range)
> +               adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
> +
>         return true;
>  }
>
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 153377b..3b8491a 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1064,6 +1064,9 @@ static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
>         multiplier = intel_sdvo_get_pixel_multiplier(adjusted_mode);
>         intel_mode_set_pixel_multiplier(adjusted_mode, multiplier);
>
> +       if (intel_sdvo->color_range)
> +               adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
> +
>         return true;
>  }
>
> @@ -1153,7 +1156,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;
> --
> 1.7.8.6
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 10+ messages in thread
* Re: [PATCH 2/4] drm/i915: Add "Automatic" mode for the "Broadcast RGB" property
  2013-01-16 17:12 ` [PATCH 2/4] drm/i915: Add "Automatic" mode for the "Broadcast RGB" property ville.syrjala
@ 2013-01-17 13:41   ` Paulo Zanoni
  0 siblings, 0 replies; 10+ messages in thread
From: Paulo Zanoni @ 2013-01-17 13:41 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, dri-devel

Hi

2013/1/16  <ville.syrjala@linux.intel.com>:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Add a new "Automatic" mode to the "Broadcast RGB" range property.
> When selected the driver automagically selects between full range and
> limited range output.
>
> Based on CEA-861 guidelines, limited range output is selected if the
> mode is a CEA mode, except 640x480. Otherwise full range output is used.
> Additionally DVI monitors should most likely default to full range
> always.
>
> As per DP1.2a DisplayPort should always use full range for 18bpp, and
> otherwise will follow CEA-861 rules.
>
> NOTE: The default value for the property will now be "Automatic"
> so some people may be affected in case they're relying on the
> current full range default.
>
> v2: Use has_hdmi_sink to check if a HDMI monitor is present

Looks correct. And let's hope this patches fixes even more "my screen
is black" or "my screen has weird colors" bugs.

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h    |    4 +++
>  drivers/gpu/drm/i915/intel_dp.c    |   28 ++++++++++++++++++++++---
>  drivers/gpu/drm/i915/intel_drv.h   |    2 +
>  drivers/gpu/drm/i915/intel_hdmi.c  |   29 +++++++++++++++++++++++---
>  drivers/gpu/drm/i915/intel_modes.c |    5 ++-
>  drivers/gpu/drm/i915/intel_sdvo.c  |   38 +++++++++++++++++++++++++++++------
>  6 files changed, 89 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d0f051b..449bbe0 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1803,5 +1803,9 @@ __i915_write(64, q)
>  #define POSTING_READ(reg)      (void)I915_READ_NOTRACE(reg)
>  #define POSTING_READ16(reg)    (void)I915_READ16_NOTRACE(reg)
>
> +/* "Broadcast RGB" property */
> +#define INTEL_BROADCAST_RGB_AUTO 0
> +#define INTEL_BROADCAST_RGB_FULL 1
> +#define INTEL_BROADCAST_RGB_LIMITED 2
>
>  #endif
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 64824d0..633a4db 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -764,6 +764,14 @@ intel_dp_mode_fixup(struct drm_encoder *encoder,
>
>         bpp = adjusted_mode->private_flags & INTEL_MODE_DP_FORCE_6BPC ? 18 : 24;
>
> +       if (intel_dp->color_range_auto) {
> +               /* as per DP1.2a and CEA-861 */
> +               if (bpp != 18 && drm_mode_cea_vic(adjusted_mode) > 1)
> +                       intel_dp->color_range = DP_COLOR_RANGE_16_235;
> +               else
> +                       intel_dp->color_range = 0;
> +       }
> +
>         if (intel_dp->color_range)
>                 adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
>
> @@ -2462,10 +2470,21 @@ intel_dp_set_property(struct drm_connector *connector,
>         }
>
>         if (property == dev_priv->broadcast_rgb_property) {
> -               if (val == !!intel_dp->color_range)
> -                       return 0;
> -
> -               intel_dp->color_range = val ? DP_COLOR_RANGE_16_235 : 0;
> +               switch (val) {
> +               case INTEL_BROADCAST_RGB_AUTO:
> +                       intel_dp->color_range_auto = true;
> +                       break;
> +               case INTEL_BROADCAST_RGB_FULL:
> +                       intel_dp->color_range_auto = false;
> +                       intel_dp->color_range = 0;
> +                       break;
> +               case INTEL_BROADCAST_RGB_LIMITED:
> +                       intel_dp->color_range_auto = false;
> +                       intel_dp->color_range = DP_COLOR_RANGE_16_235;
> +                       break;
> +               default:
> +                       return -EINVAL;
> +               }
>                 goto done;
>         }
>
> @@ -2606,6 +2625,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>
>         intel_attach_force_audio_property(connector);
>         intel_attach_broadcast_rgb_property(connector);
> +       intel_dp->color_range_auto = true;
>
>         if (is_edp(intel_dp)) {
>                 drm_mode_create_scaling_mode_property(connector->dev);
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 4df47be..1a698c6 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -343,6 +343,7 @@ struct intel_hdmi {
>         u32 sdvox_reg;
>         int ddc_bus;
>         uint32_t color_range;
> +       bool color_range_auto;
>         bool has_hdmi_sink;
>         bool has_audio;
>         enum hdmi_force_audio force_audio;
> @@ -362,6 +363,7 @@ struct intel_dp {
>         bool has_audio;
>         enum hdmi_force_audio force_audio;
>         uint32_t color_range;
> +       bool color_range_auto;
>         uint8_t link_bw;
>         uint8_t lane_count;
>         uint8_t dpcd[DP_RECEIVER_CAP_SIZE];
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index f194d75..58b072e 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -768,6 +768,15 @@ bool intel_hdmi_mode_fixup(struct drm_encoder *encoder,
>  {
>         struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
>
> +       if (intel_hdmi->color_range_auto) {
> +               /* as per CEA-861 */
> +               if (intel_hdmi->has_hdmi_sink &&
> +                   drm_mode_cea_vic(adjusted_mode) > 1)
> +                       intel_hdmi->color_range = SDVO_COLOR_RANGE_16_235;
> +               else
> +                       intel_hdmi->color_range = 0;
> +       }
> +
>         if (intel_hdmi->color_range)
>                 adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
>
> @@ -912,10 +921,21 @@ intel_hdmi_set_property(struct drm_connector *connector,
>         }
>
>         if (property == dev_priv->broadcast_rgb_property) {
> -               if (val == !!intel_hdmi->color_range)
> -                       return 0;
> -
> -               intel_hdmi->color_range = val ? SDVO_COLOR_RANGE_16_235 : 0;
> +               switch (val) {
> +               case INTEL_BROADCAST_RGB_AUTO:
> +                       intel_hdmi->color_range_auto = true;
> +                       break;
> +               case INTEL_BROADCAST_RGB_FULL:
> +                       intel_hdmi->color_range_auto = false;
> +                       intel_hdmi->color_range = 0;
> +                       break;
> +               case INTEL_BROADCAST_RGB_LIMITED:
> +                       intel_hdmi->color_range_auto = false;
> +                       intel_hdmi->color_range = SDVO_COLOR_RANGE_16_235;
> +                       break;
> +               default:
> +                       return -EINVAL;
> +               }
>                 goto done;
>         }
>
> @@ -964,6 +984,7 @@ intel_hdmi_add_properties(struct intel_hdmi *intel_hdmi, struct drm_connector *c
>  {
>         intel_attach_force_audio_property(connector);
>         intel_attach_broadcast_rgb_property(connector);
> +       intel_hdmi->color_range_auto = true;
>  }
>
>  void intel_hdmi_init_connector(struct intel_digital_port *intel_dig_port,
> diff --git a/drivers/gpu/drm/i915/intel_modes.c b/drivers/gpu/drm/i915/intel_modes.c
> index 49249bb..0e860f3 100644
> --- a/drivers/gpu/drm/i915/intel_modes.c
> +++ b/drivers/gpu/drm/i915/intel_modes.c
> @@ -100,8 +100,9 @@ intel_attach_force_audio_property(struct drm_connector *connector)
>  }
>
>  static const struct drm_prop_enum_list broadcast_rgb_names[] = {
> -       { 0, "Full" },
> -       { 1, "Limited 16:235" },
> +       { INTEL_BROADCAST_RGB_AUTO, "Automatic" },
> +       { INTEL_BROADCAST_RGB_FULL, "Full" },
> +       { INTEL_BROADCAST_RGB_LIMITED, "Limited 16:235" },
>  };
>
>  void
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 3b8491a..b422109 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -103,6 +103,7 @@ struct intel_sdvo {
>          * It is only valid when using TMDS encoding and 8 bit per color mode.
>          */
>         uint32_t color_range;
> +       bool color_range_auto;
>
>         /**
>          * This is set if we're going to treat the device as TV-out.
> @@ -1064,6 +1065,15 @@ static bool intel_sdvo_mode_fixup(struct drm_encoder *encoder,
>         multiplier = intel_sdvo_get_pixel_multiplier(adjusted_mode);
>         intel_mode_set_pixel_multiplier(adjusted_mode, multiplier);
>
> +       if (intel_sdvo->color_range_auto) {
> +               /* as per CEA-861 */
> +               if (intel_sdvo->has_hdmi_monitor &&
> +                   drm_mode_cea_vic(adjusted_mode) > 1)
> +                       intel_sdvo->color_range = SDVO_COLOR_RANGE_16_235;
> +               else
> +                       intel_sdvo->color_range = 0;
> +       }
> +
>         if (intel_sdvo->color_range)
>                 adjusted_mode->private_flags |= INTEL_MODE_LIMITED_COLOR_RANGE;
>
> @@ -1900,10 +1910,21 @@ intel_sdvo_set_property(struct drm_connector *connector,
>         }
>
>         if (property == dev_priv->broadcast_rgb_property) {
> -               if (val == !!intel_sdvo->color_range)
> -                       return 0;
> -
> -               intel_sdvo->color_range = val ? SDVO_COLOR_RANGE_16_235 : 0;
> +               switch (val) {
> +               case INTEL_BROADCAST_RGB_AUTO:
> +                       intel_sdvo->color_range_auto = true;
> +                       break;
> +               case INTEL_BROADCAST_RGB_FULL:
> +                       intel_sdvo->color_range_auto = false;
> +                       intel_sdvo->color_range = 0;
> +                       break;
> +               case INTEL_BROADCAST_RGB_LIMITED:
> +                       intel_sdvo->color_range_auto = false;
> +                       intel_sdvo->color_range = SDVO_COLOR_RANGE_16_235;
> +                       break;
> +               default:
> +                       return -EINVAL;
> +               }
>                 goto done;
>         }
>
> @@ -2200,13 +2221,16 @@ intel_sdvo_connector_init(struct intel_sdvo_connector *connector,
>  }
>
>  static void
> -intel_sdvo_add_hdmi_properties(struct intel_sdvo_connector *connector)
> +intel_sdvo_add_hdmi_properties(struct intel_sdvo *intel_sdvo,
> +                              struct intel_sdvo_connector *connector)
>  {
>         struct drm_device *dev = connector->base.base.dev;
>
>         intel_attach_force_audio_property(&connector->base.base);
> -       if (INTEL_INFO(dev)->gen >= 4 && IS_MOBILE(dev))
> +       if (INTEL_INFO(dev)->gen >= 4 && IS_MOBILE(dev)) {
>                 intel_attach_broadcast_rgb_property(&connector->base.base);
> +               intel_sdvo->color_range_auto = true;
> +       }
>  }
>
>  static bool
> @@ -2254,7 +2278,7 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
>
>         intel_sdvo_connector_init(intel_sdvo_connector, intel_sdvo);
>         if (intel_sdvo->is_hdmi)
> -               intel_sdvo_add_hdmi_properties(intel_sdvo_connector);
> +               intel_sdvo_add_hdmi_properties(intel_sdvo, intel_sdvo_connector);
>
>         return true;
>  }
> --
> 1.7.8.6
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel



-- 
Paulo Zanoni

^ permalink raw reply	[flat|nested] 10+ messages in thread
end of thread, other threads:[~2013-01-17 13:41 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-01-16 17:12 drm/i915: RGB quantization range stuff v2 ville.syrjala
2013-01-16 17:12 ` [PATCH 1/4] drm/i915: Fix RGB color range property for PCH platforms ville.syrjala
2013-01-17 12:56   ` [Intel-gfx] " Paulo Zanoni
2013-01-16 17:12 ` [PATCH 2/4] drm/i915: Add "Automatic" mode for the "Broadcast RGB" property ville.syrjala
2013-01-17 13:41   ` Paulo Zanoni
2013-01-16 17:12 ` [PATCH 3/4] drm/edid: Add drm_rgb_quant_range_selectable() ville.syrjala
2013-01-17 12:12   ` [Intel-gfx] " Paulo Zanoni
2013-01-16 17:12 ` [PATCH 4/4] drm/i915: Provide the quantization range in the AVI infoframe ville.syrjala
2013-01-17 12:16   ` Paulo Zanoni
2013-01-17 12:37     ` Ville Syrjälä

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