All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] drm/edid: Improve RGB limited range handling a bit
@ 2017-01-11 12:57 ville.syrjala
  2017-01-11 12:57 ` [PATCH 1/5] drm/edid: Have drm_edid.h include hdmi.h ville.syrjala
                   ` (7 more replies)
  0 siblings, 8 replies; 25+ messages in thread
From: ville.syrjala @ 2017-01-11 12:57 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

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

While reading the HDMI 2.0 spec I noticed some new things related to
the RGB quantization range stuff, and after cross checking with
CEA-861-F I spotted a some other things as well. So I figured I should
pimp up the code a bit.

And since we now have two drivers that deal with this stuff, I decided
to move a bunch of the code to the core to avoid duplicating the code
and having different bugs/features for each driver. I still left the state
computation part in the drivers, but eventually we might want to move that
code into some helper as well.

Entire series available here:
git://github.com/vsyrjala/linux.git hdmi_quant_range_helpers

Ville Syrjälä (5):
  drm/edid: Have drm_edid.h include hdmi.h
  drm/edid: Introduce drm_default_rgb_quant_range()
  drm/edid: Introduce drm_hdmi_avi_infoframe_quant_range()
  drm/edid: Set AVI infoframe Q even when QS=0
  drm/edid: Set YQ bits in the AVI infoframe according to CEA-861-F

 drivers/gpu/drm/drm_edid.c        | 64 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c   |  4 ++-
 drivers/gpu/drm/i915/intel_hdmi.c | 20 ++++++------
 drivers/gpu/drm/vc4/vc4_hdmi.c    | 18 +++++------
 include/drm/drm_edid.h            | 10 ++++--
 5 files changed, 93 insertions(+), 23 deletions(-)

-- 
2.10.2

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH 1/5] drm/edid: Have drm_edid.h include hdmi.h
  2017-01-11 12:57 [PATCH 0/5] drm/edid: Improve RGB limited range handling a bit ville.syrjala
@ 2017-01-11 12:57 ` ville.syrjala
  2017-01-12  9:22   ` Jani Nikula
  2017-01-11 12:57 ` [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range() ville.syrjala
                   ` (6 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: ville.syrjala @ 2017-01-11 12:57 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

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

drm_edid.h depends on hdmi.h on account of enum hdmi_picture_aspect,
so let's just include hdmi.h and drop some useless struct declarations.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 include/drm/drm_edid.h | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 38eabf65f19d..838eaf2b42e9 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -24,6 +24,7 @@
 #define __DRM_EDID_H__
 
 #include <linux/types.h>
+#include <linux/hdmi.h>
 
 struct drm_device;
 struct i2c_adapter;
@@ -322,8 +323,6 @@ struct cea_sad {
 struct drm_encoder;
 struct drm_connector;
 struct drm_display_mode;
-struct hdmi_avi_infoframe;
-struct hdmi_vendor_infoframe;
 
 void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid);
 int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads);
-- 
2.10.2

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range()
  2017-01-11 12:57 [PATCH 0/5] drm/edid: Improve RGB limited range handling a bit ville.syrjala
  2017-01-11 12:57 ` [PATCH 1/5] drm/edid: Have drm_edid.h include hdmi.h ville.syrjala
@ 2017-01-11 12:57 ` ville.syrjala
  2017-01-11 14:18   ` [PATCH v2 " ville.syrjala
                     ` (2 more replies)
  2017-01-11 12:57 ` [PATCH 3/5] drm/edid: Introduce drm_hdmi_avi_infoframe_quant_range() ville.syrjala
                   ` (5 subsequent siblings)
  7 siblings, 3 replies; 25+ messages in thread
From: ville.syrjala @ 2017-01-11 12:57 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

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

Make the code selecting the RGB quantization range a little less magicy
by wrapping it up in a small helper.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c        | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c   |  4 +++-
 drivers/gpu/drm/i915/intel_hdmi.c |  3 ++-
 drivers/gpu/drm/vc4/vc4_hdmi.c    |  4 +++-
 include/drm/drm_edid.h            |  2 ++
 5 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 4ff04aa84dd0..304c583b8000 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3768,6 +3768,24 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
 }
 EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
 
+/**
+ * drm_default_rgb_quant_range - default RGB quantization range
+ * @mode: display mode
+ *
+ * Determine the default RGB quantization range for the mode,
+ * as specified in CEA-861.
+ *
+ * Return: The default RGB quantization range for the mode
+ */
+enum hdmi_quantization_range
+drm_default_rgb_quant_range(const struct drm_display_mode *mode)
+{
+	return drm_match_cea_mode(mode) > 1 ?
+		HDMI_QUANTIZATION_RANGE_LIMITED :
+		HDMI_QUANTIZATION_RANGE_FULL;
+}
+EXPORT_SYMBOL(drm_default_rgb_quant_range);
+
 static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
 					   const u8 *hdmi)
 {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 343e1d9fa761..d4befbbe834a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1713,7 +1713,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		 * VESA DisplayPort Ver.1.2a - 5.1.1.1 Video Colorimetry
 		 */
 		pipe_config->limited_color_range =
-			bpp != 18 && drm_match_cea_mode(adjusted_mode) > 1;
+			bpp != 18 &&
+			drm_default_rgb_quant_range(adjusted_mode) ==
+			HDMI_QUANTIZATION_RANGE_LIMITED;
 	} else {
 		pipe_config->limited_color_range =
 			intel_dp->limited_color_range;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 0bcfead14571..19bd13f53729 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1330,7 +1330,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 		/* See CEA-861-E - 5.1 Default Encoding Parameters */
 		pipe_config->limited_color_range =
 			pipe_config->has_hdmi_sink &&
-			drm_match_cea_mode(adjusted_mode) > 1;
+			drm_default_rgb_quant_range(adjusted_mode) ==
+			HDMI_QUANTIZATION_RANGE_LIMITED;
 	} else {
 		pipe_config->limited_color_range =
 			intel_hdmi->limited_color_range;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index c4cb2e26de32..d79466a42690 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -463,7 +463,9 @@ static void vc4_hdmi_encoder_mode_set(struct drm_encoder *encoder,
 	csc_ctl = VC4_SET_FIELD(VC4_HD_CSC_CTL_ORDER_BGR,
 				VC4_HD_CSC_CTL_ORDER);
 
-	if (vc4_encoder->hdmi_monitor && drm_match_cea_mode(mode) > 1) {
+	if (vc4_encoder->hdmi_monitor &&
+	    drm_default_rgb_quant_range(adjusted_mode) ==
+	    HDMI_QUANTIZATION_RANGE_LIMITED) {
 		/* CEA VICs other than #1 requre limited range RGB
 		 * output unless overridden by an AVI infoframe.
 		 * Apply a colorspace conversion to squash 0-255 down
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 838eaf2b42e9..25cdf5f7a0d8 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -441,6 +441,8 @@ enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);
 bool drm_detect_hdmi_monitor(struct edid *edid);
 bool drm_detect_monitor_audio(struct edid *edid);
 bool drm_rgb_quant_range_selectable(struct edid *edid);
+enum hdmi_quantization_range
+drm_default_rgb_quant_range(const struct drm_display_mode *mode);
 int drm_add_modes_noedid(struct drm_connector *connector,
 			 int hdisplay, int vdisplay);
 void drm_set_preferred_mode(struct drm_connector *connector,
-- 
2.10.2

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 3/5] drm/edid: Introduce drm_hdmi_avi_infoframe_quant_range()
  2017-01-11 12:57 [PATCH 0/5] drm/edid: Improve RGB limited range handling a bit ville.syrjala
  2017-01-11 12:57 ` [PATCH 1/5] drm/edid: Have drm_edid.h include hdmi.h ville.syrjala
  2017-01-11 12:57 ` [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range() ville.syrjala
@ 2017-01-11 12:57 ` ville.syrjala
  2017-01-12  9:37   ` Jani Nikula
  2017-01-11 12:57 ` [PATCH 4/5] drm/edid: Set AVI infoframe Q even when QS=0 ville.syrjala
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: ville.syrjala @ 2017-01-11 12:57 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

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

Pull the logic to populate the quantization range information
in the AVI infoframe into a small helper. We'll be adding a bit
more logic to it, and having it in a central place seems like a
good idea since it's based on the CEA-861 spec.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c        | 26 ++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_hdmi.c | 13 +++++--------
 drivers/gpu/drm/vc4/vc4_hdmi.c    | 14 +++++---------
 include/drm/drm_edid.h            |  4 ++++
 4 files changed, 40 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 304c583b8000..548c20250b95 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4291,6 +4291,32 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 }
 EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
 
+/**
+ * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe
+ *                                        quantization range information
+ * @frame: HDMI AVI infoframe
+ * @rgb_quant_range: RGB quantization range (Q)
+ * @rgb_quant_range_selectable: Sink support selectable RGB quantization range (QS)
+ */
+void
+drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
+				   enum hdmi_quantization_range rgb_quant_range,
+				   bool rgb_quant_range_selectable)
+{
+	/*
+	 * CEA-861:
+	 * "A Source shall not send a non-zero Q value that does not correspond
+	 *  to the default RGB Quantization Range for the transmitted Picture
+	 *  unless the Sink indicates support for the Q bit in a Video
+	 *  Capabilities Data Block."
+	 */
+	if (rgb_quant_range_selectable)
+		frame->quantization_range = rgb_quant_range;
+	else
+		frame->quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT;
+}
+EXPORT_SYMBOL(drm_hdmi_avi_infoframe_quant_range);
+
 static enum hdmi_3d_structure
 s3d_structure_from_display_mode(const struct drm_display_mode *mode)
 {
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 19bd13f53729..351f837b09a0 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -465,14 +465,11 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 		return;
 	}
 
-	if (intel_hdmi->rgb_quant_range_selectable) {
-		if (crtc_state->limited_color_range)
-			frame.avi.quantization_range =
-				HDMI_QUANTIZATION_RANGE_LIMITED;
-		else
-			frame.avi.quantization_range =
-				HDMI_QUANTIZATION_RANGE_FULL;
-	}
+	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
+					   crtc_state->limited_color_range ?
+					   HDMI_QUANTIZATION_RANGE_LIMITED :
+					   HDMI_QUANTIZATION_RANGE_FULL,
+					   intel_hdmi->rgb_quant_range_selectable);
 
 	intel_write_infoframe(encoder, crtc_state, &frame);
 }
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index d79466a42690..a588156b5410 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -356,15 +356,11 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
 		return;
 	}
 
-	if (vc4_encoder->rgb_range_selectable) {
-		if (vc4_encoder->limited_rgb_range) {
-			frame.avi.quantization_range =
-				HDMI_QUANTIZATION_RANGE_LIMITED;
-		} else {
-			frame.avi.quantization_range =
-				HDMI_QUANTIZATION_RANGE_FULL;
-		}
-	}
+	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
+					   vc4_encoder->limited_rgb_range ?
+					   HDMI_QUANTIZATION_RANGE_LIMITED :
+					   HDMI_QUANTIZATION_RANGE_FULL,
+					   vc4_encoder->rgb_range_selectable);
 
 	vc4_hdmi_write_infoframe(encoder, &frame);
 }
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 25cdf5f7a0d8..cfad4d89589f 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -345,6 +345,10 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 int
 drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
 					    const struct drm_display_mode *mode);
+void
+drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
+				   enum hdmi_quantization_range rgb_quant_range,
+				   bool rgb_quant_range_selectable);
 
 /**
  * drm_eld_mnl - Get ELD monitor name length in bytes.
-- 
2.10.2

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 4/5] drm/edid: Set AVI infoframe Q even when QS=0
  2017-01-11 12:57 [PATCH 0/5] drm/edid: Improve RGB limited range handling a bit ville.syrjala
                   ` (2 preceding siblings ...)
  2017-01-11 12:57 ` [PATCH 3/5] drm/edid: Introduce drm_hdmi_avi_infoframe_quant_range() ville.syrjala
@ 2017-01-11 12:57 ` ville.syrjala
  2017-01-12  9:45   ` Jani Nikula
  2017-01-11 12:57 ` [PATCH 5/5] drm/edid: Set YQ bits in the AVI infoframe according to CEA-861-F ville.syrjala
                   ` (3 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: ville.syrjala @ 2017-01-11 12:57 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

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

HDMI 2.0 recommends that we set the Q bits in the AVI infoframe
even when the sink does not support quantization range selection (QS=0).
According to CEA-861 we can do that as long as the Q we send matches
the default quantization range for the mode.

Previosuly I think I had misread the spec as saying that you can't
send a non-zero Q at all when QS=0. But that's not what the spec
actually says.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c        | 8 +++++++-
 drivers/gpu/drm/i915/intel_hdmi.c | 6 ++++--
 drivers/gpu/drm/vc4/vc4_hdmi.c    | 2 +-
 include/drm/drm_edid.h            | 1 +
 4 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 548c20250b95..caa2435bac31 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4295,11 +4295,13 @@ EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
  * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe
  *                                        quantization range information
  * @frame: HDMI AVI infoframe
+ * @mode: DRM display mode
  * @rgb_quant_range: RGB quantization range (Q)
  * @rgb_quant_range_selectable: Sink support selectable RGB quantization range (QS)
  */
 void
 drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
+				   const struct drm_display_mode *mode,
 				   enum hdmi_quantization_range rgb_quant_range,
 				   bool rgb_quant_range_selectable)
 {
@@ -4309,8 +4311,12 @@ drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
 	 *  to the default RGB Quantization Range for the transmitted Picture
 	 *  unless the Sink indicates support for the Q bit in a Video
 	 *  Capabilities Data Block."
+	 *
+	 * HDMI 2.0 recommends sending non-zero Q when it does match the
+	 * default RGB quantization range for the mode, even when QS=0.
 	 */
-	if (rgb_quant_range_selectable)
+	if (rgb_quant_range_selectable ||
+	    rgb_quant_range == drm_default_rgb_quant_range(mode))
 		frame->quantization_range = rgb_quant_range;
 	else
 		frame->quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 351f837b09a0..af16b0fa6b69 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -455,17 +455,19 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
 					 const struct intel_crtc_state *crtc_state)
 {
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->base.adjusted_mode;
 	union hdmi_infoframe frame;
 	int ret;
 
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
-						       &crtc_state->base.adjusted_mode);
+						       adjusted_mode);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return;
 	}
 
-	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
+	drm_hdmi_avi_infoframe_quant_range(&frame.avi, adjusted_mode,
 					   crtc_state->limited_color_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
 					   HDMI_QUANTIZATION_RANGE_FULL,
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index a588156b5410..f38fdbac2878 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -356,7 +356,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
 		return;
 	}
 
-	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
+	drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
 					   vc4_encoder->limited_rgb_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
 					   HDMI_QUANTIZATION_RANGE_FULL,
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index cfad4d89589f..43fb0ac5eb9c 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -347,6 +347,7 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
 					    const struct drm_display_mode *mode);
 void
 drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
+				   const struct drm_display_mode *mode,
 				   enum hdmi_quantization_range rgb_quant_range,
 				   bool rgb_quant_range_selectable);
 
-- 
2.10.2

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [PATCH 5/5] drm/edid: Set YQ bits in the AVI infoframe according to CEA-861-F
  2017-01-11 12:57 [PATCH 0/5] drm/edid: Improve RGB limited range handling a bit ville.syrjala
                   ` (3 preceding siblings ...)
  2017-01-11 12:57 ` [PATCH 4/5] drm/edid: Set AVI infoframe Q even when QS=0 ville.syrjala
@ 2017-01-11 12:57 ` ville.syrjala
  2017-01-12 10:13   ` Jani Nikula
  2017-01-11 13:53 ` ✓ Fi.CI.BAT: success for drm/edid: Improve RGB limited range handling a bit Patchwork
                   ` (2 subsequent siblings)
  7 siblings, 1 reply; 25+ messages in thread
From: ville.syrjala @ 2017-01-11 12:57 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

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

CEA-861-F tells us:
"When transmitting any RGB colorimetry, the Source should set the
 YQ-field to match the RGB Quantization Range being transmitted
 (e.g., when Limited Range RGB, set YQ=0 or when Full Range RGB,
 set YQ=1) and the Sink shall ignore the YQ-field."

So let's go ahead and do that. Perhaps there are sinks that don't
ignore the YQ as they should for RGB?

I wasn't able to find similar text in CEA-861-E, so it would seem
to be a fairly "recent" addition.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index caa2435bac31..6ba9a1a6eae4 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4320,6 +4320,20 @@ drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
 		frame->quantization_range = rgb_quant_range;
 	else
 		frame->quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT;
+
+	/*
+	 * CEA-861-F:
+	 * "When transmitting any RGB colorimetry, the Source should set the
+	 *  YQ-field to match the RGB Quantization Range being transmitted
+	 *  (e.g., when Limited Range RGB, set YQ=0 or when Full Range RGB,
+	 *  set YQ=1) and the Sink shall ignore the YQ-field."
+	 */
+	if (rgb_quant_range == HDMI_QUANTIZATION_RANGE_LIMITED)
+		frame->ycc_quantization_range =
+			HDMI_YCC_QUANTIZATION_RANGE_LIMITED;
+	else
+		frame->ycc_quantization_range =
+			HDMI_YCC_QUANTIZATION_RANGE_FULL;
 }
 EXPORT_SYMBOL(drm_hdmi_avi_infoframe_quant_range);
 
-- 
2.10.2

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* ✓ Fi.CI.BAT: success for drm/edid: Improve RGB limited range handling a bit
  2017-01-11 12:57 [PATCH 0/5] drm/edid: Improve RGB limited range handling a bit ville.syrjala
                   ` (4 preceding siblings ...)
  2017-01-11 12:57 ` [PATCH 5/5] drm/edid: Set YQ bits in the AVI infoframe according to CEA-861-F ville.syrjala
@ 2017-01-11 13:53 ` Patchwork
  2017-01-11 18:53 ` ✓ Fi.CI.BAT: success for drm/edid: Improve RGB limited range handling a bit (rev2) Patchwork
  2017-01-26 19:08 ` [PATCH 0/5] drm/edid: Improve RGB limited range handling a bit Ville Syrjälä
  7 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2017-01-11 13:53 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/edid: Improve RGB limited range handling a bit
URL   : https://patchwork.freedesktop.org/series/17825/
State : success

== Summary ==

Series 17825v1 drm/edid: Improve RGB limited range handling a bit
https://patchwork.freedesktop.org/api/1.0/series/17825/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                incomplete -> SKIP       (fi-bsw-n3050)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                incomplete -> PASS       (fi-byt-n2820)

fi-bdw-5557u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:83   pass:71   dwarn:0   dfail:0   fail:0   skip:11 
fi-skl-6260u     total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

f0350ffa1b2bc16dc49fdc2fce10776d604a1c5f drm-tip: 2017y-01m-11d-12h-34m-12s UTC integration manifest
af78a86 drm/edid: Set YQ bits in the AVI infoframe according to CEA-861-F
6e2f2ab drm/edid: Set AVI infoframe Q even when QS=0
ac00d36 drm/edid: Introduce drm_hdmi_avi_infoframe_quant_range()
d7e4b07 drm/edid: Introduce drm_default_rgb_quant_range()
142daf0 drm/edid: Have drm_edid.h include hdmi.h

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3479/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [PATCH v2 2/5] drm/edid: Introduce drm_default_rgb_quant_range()
  2017-01-11 12:57 ` [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range() ville.syrjala
@ 2017-01-11 14:18   ` ville.syrjala
  2017-01-12  9:29     ` Jani Nikula
  2017-01-11 16:16   ` [PATCH " Daniel Vetter
  2017-01-20 19:50   ` Eric Anholt
  2 siblings, 1 reply; 25+ messages in thread
From: ville.syrjala @ 2017-01-11 14:18 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

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

Make the code selecting the RGB quantization range a little less magicy
by wrapping it up in a small helper.

v2: s/adjusted_mode/mode in vc4 to make it actually compile

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c        | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c   |  4 +++-
 drivers/gpu/drm/i915/intel_hdmi.c |  3 ++-
 drivers/gpu/drm/vc4/vc4_hdmi.c    |  4 +++-
 include/drm/drm_edid.h            |  2 ++
 5 files changed, 28 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 4ff04aa84dd0..304c583b8000 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3768,6 +3768,24 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
 }
 EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
 
+/**
+ * drm_default_rgb_quant_range - default RGB quantization range
+ * @mode: display mode
+ *
+ * Determine the default RGB quantization range for the mode,
+ * as specified in CEA-861.
+ *
+ * Return: The default RGB quantization range for the mode
+ */
+enum hdmi_quantization_range
+drm_default_rgb_quant_range(const struct drm_display_mode *mode)
+{
+	return drm_match_cea_mode(mode) > 1 ?
+		HDMI_QUANTIZATION_RANGE_LIMITED :
+		HDMI_QUANTIZATION_RANGE_FULL;
+}
+EXPORT_SYMBOL(drm_default_rgb_quant_range);
+
 static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
 					   const u8 *hdmi)
 {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 343e1d9fa761..d4befbbe834a 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1713,7 +1713,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 		 * VESA DisplayPort Ver.1.2a - 5.1.1.1 Video Colorimetry
 		 */
 		pipe_config->limited_color_range =
-			bpp != 18 && drm_match_cea_mode(adjusted_mode) > 1;
+			bpp != 18 &&
+			drm_default_rgb_quant_range(adjusted_mode) ==
+			HDMI_QUANTIZATION_RANGE_LIMITED;
 	} else {
 		pipe_config->limited_color_range =
 			intel_dp->limited_color_range;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 0bcfead14571..19bd13f53729 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1330,7 +1330,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
 		/* See CEA-861-E - 5.1 Default Encoding Parameters */
 		pipe_config->limited_color_range =
 			pipe_config->has_hdmi_sink &&
-			drm_match_cea_mode(adjusted_mode) > 1;
+			drm_default_rgb_quant_range(adjusted_mode) ==
+			HDMI_QUANTIZATION_RANGE_LIMITED;
 	} else {
 		pipe_config->limited_color_range =
 			intel_hdmi->limited_color_range;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index c4cb2e26de32..5d49bf948162 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -463,7 +463,9 @@ static void vc4_hdmi_encoder_mode_set(struct drm_encoder *encoder,
 	csc_ctl = VC4_SET_FIELD(VC4_HD_CSC_CTL_ORDER_BGR,
 				VC4_HD_CSC_CTL_ORDER);
 
-	if (vc4_encoder->hdmi_monitor && drm_match_cea_mode(mode) > 1) {
+	if (vc4_encoder->hdmi_monitor &&
+	    drm_default_rgb_quant_range(mode) ==
+	    HDMI_QUANTIZATION_RANGE_LIMITED) {
 		/* CEA VICs other than #1 requre limited range RGB
 		 * output unless overridden by an AVI infoframe.
 		 * Apply a colorspace conversion to squash 0-255 down
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 838eaf2b42e9..25cdf5f7a0d8 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -441,6 +441,8 @@ enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);
 bool drm_detect_hdmi_monitor(struct edid *edid);
 bool drm_detect_monitor_audio(struct edid *edid);
 bool drm_rgb_quant_range_selectable(struct edid *edid);
+enum hdmi_quantization_range
+drm_default_rgb_quant_range(const struct drm_display_mode *mode);
 int drm_add_modes_noedid(struct drm_connector *connector,
 			 int hdisplay, int vdisplay);
 void drm_set_preferred_mode(struct drm_connector *connector,
-- 
2.10.2

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

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range()
  2017-01-11 12:57 ` [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range() ville.syrjala
  2017-01-11 14:18   ` [PATCH v2 " ville.syrjala
@ 2017-01-11 16:16   ` Daniel Vetter
  2017-01-11 16:31     ` Ville Syrjälä
  2017-01-20 19:50   ` Eric Anholt
  2 siblings, 1 reply; 25+ messages in thread
From: Daniel Vetter @ 2017-01-11 16:16 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx, dri-devel

On Wed, Jan 11, 2017 at 02:57:22PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Make the code selecting the RGB quantization range a little less magicy
> by wrapping it up in a small helper.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Needs cc: for driver maintainers, Eric for vc4 here.
-Daniel

> ---
>  drivers/gpu/drm/drm_edid.c        | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c   |  4 +++-
>  drivers/gpu/drm/i915/intel_hdmi.c |  3 ++-
>  drivers/gpu/drm/vc4/vc4_hdmi.c    |  4 +++-
>  include/drm/drm_edid.h            |  2 ++
>  5 files changed, 28 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 4ff04aa84dd0..304c583b8000 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3768,6 +3768,24 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
>  }
>  EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
>  
> +/**
> + * drm_default_rgb_quant_range - default RGB quantization range
> + * @mode: display mode
> + *
> + * Determine the default RGB quantization range for the mode,
> + * as specified in CEA-861.
> + *
> + * Return: The default RGB quantization range for the mode
> + */
> +enum hdmi_quantization_range
> +drm_default_rgb_quant_range(const struct drm_display_mode *mode)
> +{
> +	return drm_match_cea_mode(mode) > 1 ?
> +		HDMI_QUANTIZATION_RANGE_LIMITED :
> +		HDMI_QUANTIZATION_RANGE_FULL;
> +}
> +EXPORT_SYMBOL(drm_default_rgb_quant_range);
> +
>  static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>  					   const u8 *hdmi)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 343e1d9fa761..d4befbbe834a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1713,7 +1713,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  		 * VESA DisplayPort Ver.1.2a - 5.1.1.1 Video Colorimetry
>  		 */
>  		pipe_config->limited_color_range =
> -			bpp != 18 && drm_match_cea_mode(adjusted_mode) > 1;
> +			bpp != 18 &&
> +			drm_default_rgb_quant_range(adjusted_mode) ==
> +			HDMI_QUANTIZATION_RANGE_LIMITED;
>  	} else {
>  		pipe_config->limited_color_range =
>  			intel_dp->limited_color_range;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 0bcfead14571..19bd13f53729 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1330,7 +1330,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  		/* See CEA-861-E - 5.1 Default Encoding Parameters */
>  		pipe_config->limited_color_range =
>  			pipe_config->has_hdmi_sink &&
> -			drm_match_cea_mode(adjusted_mode) > 1;
> +			drm_default_rgb_quant_range(adjusted_mode) ==
> +			HDMI_QUANTIZATION_RANGE_LIMITED;
>  	} else {
>  		pipe_config->limited_color_range =
>  			intel_hdmi->limited_color_range;
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index c4cb2e26de32..d79466a42690 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -463,7 +463,9 @@ static void vc4_hdmi_encoder_mode_set(struct drm_encoder *encoder,
>  	csc_ctl = VC4_SET_FIELD(VC4_HD_CSC_CTL_ORDER_BGR,
>  				VC4_HD_CSC_CTL_ORDER);
>  
> -	if (vc4_encoder->hdmi_monitor && drm_match_cea_mode(mode) > 1) {
> +	if (vc4_encoder->hdmi_monitor &&
> +	    drm_default_rgb_quant_range(adjusted_mode) ==
> +	    HDMI_QUANTIZATION_RANGE_LIMITED) {
>  		/* CEA VICs other than #1 requre limited range RGB
>  		 * output unless overridden by an AVI infoframe.
>  		 * Apply a colorspace conversion to squash 0-255 down
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 838eaf2b42e9..25cdf5f7a0d8 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -441,6 +441,8 @@ enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);
>  bool drm_detect_hdmi_monitor(struct edid *edid);
>  bool drm_detect_monitor_audio(struct edid *edid);
>  bool drm_rgb_quant_range_selectable(struct edid *edid);
> +enum hdmi_quantization_range
> +drm_default_rgb_quant_range(const struct drm_display_mode *mode);
>  int drm_add_modes_noedid(struct drm_connector *connector,
>  			 int hdisplay, int vdisplay);
>  void drm_set_preferred_mode(struct drm_connector *connector,
> -- 
> 2.10.2
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range()
  2017-01-11 16:16   ` [PATCH " Daniel Vetter
@ 2017-01-11 16:31     ` Ville Syrjälä
  2017-01-12  8:00       ` Daniel Vetter
  0 siblings, 1 reply; 25+ messages in thread
From: Ville Syrjälä @ 2017-01-11 16:31 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx, dri-devel

On Wed, Jan 11, 2017 at 05:16:54PM +0100, Daniel Vetter wrote:
> On Wed, Jan 11, 2017 at 02:57:22PM +0200, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Make the code selecting the RGB quantization range a little less magicy
> > by wrapping it up in a small helper.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Needs cc: for driver maintainers, Eric for vc4 here.

Eric was cc:d. I was just too lazy to add the cc:s to all the commit
messages, and so i just used --cc on the whole lot.

> -Daniel
> 
> > ---
> >  drivers/gpu/drm/drm_edid.c        | 18 ++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_dp.c   |  4 +++-
> >  drivers/gpu/drm/i915/intel_hdmi.c |  3 ++-
> >  drivers/gpu/drm/vc4/vc4_hdmi.c    |  4 +++-
> >  include/drm/drm_edid.h            |  2 ++
> >  5 files changed, 28 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 4ff04aa84dd0..304c583b8000 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -3768,6 +3768,24 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
> >  }
> >  EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
> >  
> > +/**
> > + * drm_default_rgb_quant_range - default RGB quantization range
> > + * @mode: display mode
> > + *
> > + * Determine the default RGB quantization range for the mode,
> > + * as specified in CEA-861.
> > + *
> > + * Return: The default RGB quantization range for the mode
> > + */
> > +enum hdmi_quantization_range
> > +drm_default_rgb_quant_range(const struct drm_display_mode *mode)
> > +{
> > +	return drm_match_cea_mode(mode) > 1 ?
> > +		HDMI_QUANTIZATION_RANGE_LIMITED :
> > +		HDMI_QUANTIZATION_RANGE_FULL;
> > +}
> > +EXPORT_SYMBOL(drm_default_rgb_quant_range);
> > +
> >  static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
> >  					   const u8 *hdmi)
> >  {
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 343e1d9fa761..d4befbbe834a 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1713,7 +1713,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >  		 * VESA DisplayPort Ver.1.2a - 5.1.1.1 Video Colorimetry
> >  		 */
> >  		pipe_config->limited_color_range =
> > -			bpp != 18 && drm_match_cea_mode(adjusted_mode) > 1;
> > +			bpp != 18 &&
> > +			drm_default_rgb_quant_range(adjusted_mode) ==
> > +			HDMI_QUANTIZATION_RANGE_LIMITED;
> >  	} else {
> >  		pipe_config->limited_color_range =
> >  			intel_dp->limited_color_range;
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 0bcfead14571..19bd13f53729 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1330,7 +1330,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> >  		/* See CEA-861-E - 5.1 Default Encoding Parameters */
> >  		pipe_config->limited_color_range =
> >  			pipe_config->has_hdmi_sink &&
> > -			drm_match_cea_mode(adjusted_mode) > 1;
> > +			drm_default_rgb_quant_range(adjusted_mode) ==
> > +			HDMI_QUANTIZATION_RANGE_LIMITED;
> >  	} else {
> >  		pipe_config->limited_color_range =
> >  			intel_hdmi->limited_color_range;
> > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > index c4cb2e26de32..d79466a42690 100644
> > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > @@ -463,7 +463,9 @@ static void vc4_hdmi_encoder_mode_set(struct drm_encoder *encoder,
> >  	csc_ctl = VC4_SET_FIELD(VC4_HD_CSC_CTL_ORDER_BGR,
> >  				VC4_HD_CSC_CTL_ORDER);
> >  
> > -	if (vc4_encoder->hdmi_monitor && drm_match_cea_mode(mode) > 1) {
> > +	if (vc4_encoder->hdmi_monitor &&
> > +	    drm_default_rgb_quant_range(adjusted_mode) ==
> > +	    HDMI_QUANTIZATION_RANGE_LIMITED) {
> >  		/* CEA VICs other than #1 requre limited range RGB
> >  		 * output unless overridden by an AVI infoframe.
> >  		 * Apply a colorspace conversion to squash 0-255 down
> > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> > index 838eaf2b42e9..25cdf5f7a0d8 100644
> > --- a/include/drm/drm_edid.h
> > +++ b/include/drm/drm_edid.h
> > @@ -441,6 +441,8 @@ enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);
> >  bool drm_detect_hdmi_monitor(struct edid *edid);
> >  bool drm_detect_monitor_audio(struct edid *edid);
> >  bool drm_rgb_quant_range_selectable(struct edid *edid);
> > +enum hdmi_quantization_range
> > +drm_default_rgb_quant_range(const struct drm_display_mode *mode);
> >  int drm_add_modes_noedid(struct drm_connector *connector,
> >  			 int hdisplay, int vdisplay);
> >  void drm_set_preferred_mode(struct drm_connector *connector,
> > -- 
> > 2.10.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* ✓ Fi.CI.BAT: success for drm/edid: Improve RGB limited range handling a bit (rev2)
  2017-01-11 12:57 [PATCH 0/5] drm/edid: Improve RGB limited range handling a bit ville.syrjala
                   ` (5 preceding siblings ...)
  2017-01-11 13:53 ` ✓ Fi.CI.BAT: success for drm/edid: Improve RGB limited range handling a bit Patchwork
@ 2017-01-11 18:53 ` Patchwork
  2017-01-26 19:08 ` [PATCH 0/5] drm/edid: Improve RGB limited range handling a bit Ville Syrjälä
  7 siblings, 0 replies; 25+ messages in thread
From: Patchwork @ 2017-01-11 18:53 UTC (permalink / raw)
  To: ville.syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/edid: Improve RGB limited range handling a bit (rev2)
URL   : https://patchwork.freedesktop.org/series/17825/
State : success

== Summary ==

Series 17825v2 drm/edid: Improve RGB limited range handling a bit
https://patchwork.freedesktop.org/api/1.0/series/17825/revisions/2/mbox/


fi-bdw-5557u     total:246  pass:232  dwarn:0   dfail:0   fail:0   skip:14 
fi-bsw-n3050     total:246  pass:207  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:246  pass:224  dwarn:0   dfail:0   fail:0   skip:22 
fi-bxt-t5700     total:82   pass:69   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:246  pass:219  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-hsw-4770r     total:246  pass:227  dwarn:0   dfail:0   fail:0   skip:19 
fi-ivb-3520m     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-ivb-3770      total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-kbl-7500u     total:246  pass:225  dwarn:0   dfail:0   fail:0   skip:21 
fi-skl-6260u     total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-skl-6700hq    total:246  pass:226  dwarn:0   dfail:0   fail:0   skip:20 
fi-skl-6700k     total:246  pass:222  dwarn:3   dfail:0   fail:0   skip:21 
fi-skl-6770hq    total:246  pass:233  dwarn:0   dfail:0   fail:0   skip:13 
fi-snb-2520m     total:246  pass:215  dwarn:0   dfail:0   fail:0   skip:31 
fi-snb-2600      total:246  pass:214  dwarn:0   dfail:0   fail:0   skip:32 

b69fc4c941bef6d10750ce3f07daedfffc7017d1 drm-tip: 2017y-01m-11d-17h-30m-02s UTC integration manifest
20e8661 drm/edid: Set YQ bits in the AVI infoframe according to CEA-861-F
3d4bb98 drm/edid: Set AVI infoframe Q even when QS=0
0784091 drm/edid: Introduce drm_hdmi_avi_infoframe_quant_range()
36b475a drm/edid: Introduce drm_default_rgb_quant_range()
e46f7c5 drm/edid: Have drm_edid.h include hdmi.h

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3484/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range()
  2017-01-11 16:31     ` Ville Syrjälä
@ 2017-01-12  8:00       ` Daniel Vetter
  2017-01-12  8:13         ` Michel Dänzer
  0 siblings, 1 reply; 25+ messages in thread
From: Daniel Vetter @ 2017-01-12  8:00 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, dri-devel

On Wed, Jan 11, 2017 at 06:31:52PM +0200, Ville Syrjälä wrote:
> On Wed, Jan 11, 2017 at 05:16:54PM +0100, Daniel Vetter wrote:
> > On Wed, Jan 11, 2017 at 02:57:22PM +0200, ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Make the code selecting the RGB quantization range a little less magicy
> > > by wrapping it up in a small helper.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Needs cc: for driver maintainers, Eric for vc4 here.
> 
> Eric was cc:d. I was just too lazy to add the cc:s to all the commit
> messages, and so i just used --cc on the whole lot.

Hm, he's not on the cc: list over here (on the mails, not in the patch
body).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range()
  2017-01-12  8:00       ` Daniel Vetter
@ 2017-01-12  8:13         ` Michel Dänzer
  0 siblings, 0 replies; 25+ messages in thread
From: Michel Dänzer @ 2017-01-12  8:13 UTC (permalink / raw)
  To: Daniel Vetter, Ville Syrjälä; +Cc: intel-gfx, dri-devel

On 12/01/17 05:00 PM, Daniel Vetter wrote:
> On Wed, Jan 11, 2017 at 06:31:52PM +0200, Ville Syrjälä wrote:
>> On Wed, Jan 11, 2017 at 05:16:54PM +0100, Daniel Vetter wrote:
>>> On Wed, Jan 11, 2017 at 02:57:22PM +0200, ville.syrjala@linux.intel.com wrote:
>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>
>>>> Make the code selecting the RGB quantization range a little less magicy
>>>> by wrapping it up in a small helper.
>>>>
>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> Needs cc: for driver maintainers, Eric for vc4 here.
>>
>> Eric was cc:d. I was just too lazy to add the cc:s to all the commit
>> messages, and so i just used --cc on the whole lot.
> 
> Hm, he's not on the cc: list over here (on the mails, not in the patch
> body).

That could be because he has "Avoid duplicate copies of messages?"
enabled in his mailman subscription options.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 1/5] drm/edid: Have drm_edid.h include hdmi.h
  2017-01-11 12:57 ` [PATCH 1/5] drm/edid: Have drm_edid.h include hdmi.h ville.syrjala
@ 2017-01-12  9:22   ` Jani Nikula
  0 siblings, 0 replies; 25+ messages in thread
From: Jani Nikula @ 2017-01-12  9:22 UTC (permalink / raw)
  To: ville.syrjala, dri-devel; +Cc: intel-gfx

On Wed, 11 Jan 2017, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> drm_edid.h depends on hdmi.h on account of enum hdmi_picture_aspect,
> so let's just include hdmi.h and drop some useless struct declarations.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  include/drm/drm_edid.h | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 38eabf65f19d..838eaf2b42e9 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -24,6 +24,7 @@
>  #define __DRM_EDID_H__
>  
>  #include <linux/types.h>
> +#include <linux/hdmi.h>
>  
>  struct drm_device;
>  struct i2c_adapter;
> @@ -322,8 +323,6 @@ struct cea_sad {
>  struct drm_encoder;
>  struct drm_connector;
>  struct drm_display_mode;
> -struct hdmi_avi_infoframe;
> -struct hdmi_vendor_infoframe;
>  
>  void drm_edid_to_eld(struct drm_connector *connector, struct edid *edid);
>  int drm_edid_to_sad(struct edid *edid, struct cea_sad **sads);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH v2 2/5] drm/edid: Introduce drm_default_rgb_quant_range()
  2017-01-11 14:18   ` [PATCH v2 " ville.syrjala
@ 2017-01-12  9:29     ` Jani Nikula
  2017-01-12 14:24       ` [Intel-gfx] " Ville Syrjälä
  0 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2017-01-12  9:29 UTC (permalink / raw)
  To: ville.syrjala, dri-devel; +Cc: intel-gfx

On Wed, 11 Jan 2017, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Make the code selecting the RGB quantization range a little less magicy
> by wrapping it up in a small helper.
>
> v2: s/adjusted_mode/mode in vc4 to make it actually compile
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c        | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c   |  4 +++-
>  drivers/gpu/drm/i915/intel_hdmi.c |  3 ++-
>  drivers/gpu/drm/vc4/vc4_hdmi.c    |  4 +++-
>  include/drm/drm_edid.h            |  2 ++
>  5 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 4ff04aa84dd0..304c583b8000 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3768,6 +3768,24 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
>  }
>  EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
>  
> +/**
> + * drm_default_rgb_quant_range - default RGB quantization range
> + * @mode: display mode
> + *
> + * Determine the default RGB quantization range for the mode,
> + * as specified in CEA-861.
> + *
> + * Return: The default RGB quantization range for the mode
> + */
> +enum hdmi_quantization_range
> +drm_default_rgb_quant_range(const struct drm_display_mode *mode)
> +{
> +	return drm_match_cea_mode(mode) > 1 ?
> +		HDMI_QUANTIZATION_RANGE_LIMITED :
> +		HDMI_QUANTIZATION_RANGE_FULL;
> +}
> +EXPORT_SYMBOL(drm_default_rgb_quant_range);

Or just bool drm_default_to_limited_range() or similar?

*shrug*

Reviewed-by: Jani Nikula <jani.nikula@intel.com>



> +
>  static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
>  					   const u8 *hdmi)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 343e1d9fa761..d4befbbe834a 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1713,7 +1713,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  		 * VESA DisplayPort Ver.1.2a - 5.1.1.1 Video Colorimetry
>  		 */
>  		pipe_config->limited_color_range =
> -			bpp != 18 && drm_match_cea_mode(adjusted_mode) > 1;
> +			bpp != 18 &&
> +			drm_default_rgb_quant_range(adjusted_mode) ==
> +			HDMI_QUANTIZATION_RANGE_LIMITED;
>  	} else {
>  		pipe_config->limited_color_range =
>  			intel_dp->limited_color_range;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 0bcfead14571..19bd13f53729 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1330,7 +1330,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
>  		/* See CEA-861-E - 5.1 Default Encoding Parameters */
>  		pipe_config->limited_color_range =
>  			pipe_config->has_hdmi_sink &&
> -			drm_match_cea_mode(adjusted_mode) > 1;
> +			drm_default_rgb_quant_range(adjusted_mode) ==
> +			HDMI_QUANTIZATION_RANGE_LIMITED;
>  	} else {
>  		pipe_config->limited_color_range =
>  			intel_hdmi->limited_color_range;
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index c4cb2e26de32..5d49bf948162 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -463,7 +463,9 @@ static void vc4_hdmi_encoder_mode_set(struct drm_encoder *encoder,
>  	csc_ctl = VC4_SET_FIELD(VC4_HD_CSC_CTL_ORDER_BGR,
>  				VC4_HD_CSC_CTL_ORDER);
>  
> -	if (vc4_encoder->hdmi_monitor && drm_match_cea_mode(mode) > 1) {
> +	if (vc4_encoder->hdmi_monitor &&
> +	    drm_default_rgb_quant_range(mode) ==
> +	    HDMI_QUANTIZATION_RANGE_LIMITED) {
>  		/* CEA VICs other than #1 requre limited range RGB
>  		 * output unless overridden by an AVI infoframe.
>  		 * Apply a colorspace conversion to squash 0-255 down
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 838eaf2b42e9..25cdf5f7a0d8 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -441,6 +441,8 @@ enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);
>  bool drm_detect_hdmi_monitor(struct edid *edid);
>  bool drm_detect_monitor_audio(struct edid *edid);
>  bool drm_rgb_quant_range_selectable(struct edid *edid);
> +enum hdmi_quantization_range
> +drm_default_rgb_quant_range(const struct drm_display_mode *mode);
>  int drm_add_modes_noedid(struct drm_connector *connector,
>  			 int hdisplay, int vdisplay);
>  void drm_set_preferred_mode(struct drm_connector *connector,

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 3/5] drm/edid: Introduce drm_hdmi_avi_infoframe_quant_range()
  2017-01-11 12:57 ` [PATCH 3/5] drm/edid: Introduce drm_hdmi_avi_infoframe_quant_range() ville.syrjala
@ 2017-01-12  9:37   ` Jani Nikula
  0 siblings, 0 replies; 25+ messages in thread
From: Jani Nikula @ 2017-01-12  9:37 UTC (permalink / raw)
  To: ville.syrjala, dri-devel; +Cc: intel-gfx

On Wed, 11 Jan 2017, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Pull the logic to populate the quantization range information
> in the AVI infoframe into a small helper. We'll be adding a bit
> more logic to it, and having it in a central place seems like a
> good idea since it's based on the CEA-861 spec.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c        | 26 ++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_hdmi.c | 13 +++++--------
>  drivers/gpu/drm/vc4/vc4_hdmi.c    | 14 +++++---------
>  include/drm/drm_edid.h            |  4 ++++
>  4 files changed, 40 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 304c583b8000..548c20250b95 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4291,6 +4291,32 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>  }
>  EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
>  
> +/**
> + * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe
> + *                                        quantization range information

I'm a fan of having a verb in the function name for functions that do
stuff. Would be nice to have "set" in there somewhere, although the name
is growing a bit unwieldy. (Btw, same for drm_default_rgb_quant_range()
in the previous patch, "get" would be nice.)

Up to you.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> + * @frame: HDMI AVI infoframe
> + * @rgb_quant_range: RGB quantization range (Q)
> + * @rgb_quant_range_selectable: Sink support selectable RGB quantization range (QS)
> + */
> +void
> +drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
> +				   enum hdmi_quantization_range rgb_quant_range,
> +				   bool rgb_quant_range_selectable)
> +{
> +	/*
> +	 * CEA-861:
> +	 * "A Source shall not send a non-zero Q value that does not correspond
> +	 *  to the default RGB Quantization Range for the transmitted Picture
> +	 *  unless the Sink indicates support for the Q bit in a Video
> +	 *  Capabilities Data Block."
> +	 */
> +	if (rgb_quant_range_selectable)
> +		frame->quantization_range = rgb_quant_range;
> +	else
> +		frame->quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT;
> +}
> +EXPORT_SYMBOL(drm_hdmi_avi_infoframe_quant_range);
> +
>  static enum hdmi_3d_structure
>  s3d_structure_from_display_mode(const struct drm_display_mode *mode)
>  {
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 19bd13f53729..351f837b09a0 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -465,14 +465,11 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
>  		return;
>  	}
>  
> -	if (intel_hdmi->rgb_quant_range_selectable) {
> -		if (crtc_state->limited_color_range)
> -			frame.avi.quantization_range =
> -				HDMI_QUANTIZATION_RANGE_LIMITED;
> -		else
> -			frame.avi.quantization_range =
> -				HDMI_QUANTIZATION_RANGE_FULL;
> -	}
> +	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> +					   crtc_state->limited_color_range ?
> +					   HDMI_QUANTIZATION_RANGE_LIMITED :
> +					   HDMI_QUANTIZATION_RANGE_FULL,
> +					   intel_hdmi->rgb_quant_range_selectable);
>  
>  	intel_write_infoframe(encoder, crtc_state, &frame);
>  }
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index d79466a42690..a588156b5410 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -356,15 +356,11 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
>  		return;
>  	}
>  
> -	if (vc4_encoder->rgb_range_selectable) {
> -		if (vc4_encoder->limited_rgb_range) {
> -			frame.avi.quantization_range =
> -				HDMI_QUANTIZATION_RANGE_LIMITED;
> -		} else {
> -			frame.avi.quantization_range =
> -				HDMI_QUANTIZATION_RANGE_FULL;
> -		}
> -	}
> +	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> +					   vc4_encoder->limited_rgb_range ?
> +					   HDMI_QUANTIZATION_RANGE_LIMITED :
> +					   HDMI_QUANTIZATION_RANGE_FULL,
> +					   vc4_encoder->rgb_range_selectable);
>  
>  	vc4_hdmi_write_infoframe(encoder, &frame);
>  }
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 25cdf5f7a0d8..cfad4d89589f 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -345,6 +345,10 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>  int
>  drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
>  					    const struct drm_display_mode *mode);
> +void
> +drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
> +				   enum hdmi_quantization_range rgb_quant_range,
> +				   bool rgb_quant_range_selectable);
>  
>  /**
>   * drm_eld_mnl - Get ELD monitor name length in bytes.

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 4/5] drm/edid: Set AVI infoframe Q even when QS=0
  2017-01-11 12:57 ` [PATCH 4/5] drm/edid: Set AVI infoframe Q even when QS=0 ville.syrjala
@ 2017-01-12  9:45   ` Jani Nikula
  0 siblings, 0 replies; 25+ messages in thread
From: Jani Nikula @ 2017-01-12  9:45 UTC (permalink / raw)
  To: ville.syrjala, dri-devel; +Cc: intel-gfx

On Wed, 11 Jan 2017, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> HDMI 2.0 recommends that we set the Q bits in the AVI infoframe
> even when the sink does not support quantization range selection (QS=0).
> According to CEA-861 we can do that as long as the Q we send matches
> the default quantization range for the mode.
>
> Previosuly I think I had misread the spec as saying that you can't
  ^^^^^^^^^^

> send a non-zero Q at all when QS=0. But that's not what the spec
> actually says.

Agreed.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c        | 8 +++++++-
>  drivers/gpu/drm/i915/intel_hdmi.c | 6 ++++--
>  drivers/gpu/drm/vc4/vc4_hdmi.c    | 2 +-
>  include/drm/drm_edid.h            | 1 +
>  4 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 548c20250b95..caa2435bac31 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4295,11 +4295,13 @@ EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
>   * drm_hdmi_avi_infoframe_quant_range() - fill the HDMI AVI infoframe
>   *                                        quantization range information
>   * @frame: HDMI AVI infoframe
> + * @mode: DRM display mode
>   * @rgb_quant_range: RGB quantization range (Q)
>   * @rgb_quant_range_selectable: Sink support selectable RGB quantization range (QS)
>   */
>  void
>  drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
> +				   const struct drm_display_mode *mode,
>  				   enum hdmi_quantization_range rgb_quant_range,
>  				   bool rgb_quant_range_selectable)
>  {
> @@ -4309,8 +4311,12 @@ drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
>  	 *  to the default RGB Quantization Range for the transmitted Picture
>  	 *  unless the Sink indicates support for the Q bit in a Video
>  	 *  Capabilities Data Block."
> +	 *
> +	 * HDMI 2.0 recommends sending non-zero Q when it does match the
> +	 * default RGB quantization range for the mode, even when QS=0.
>  	 */
> -	if (rgb_quant_range_selectable)
> +	if (rgb_quant_range_selectable ||
> +	    rgb_quant_range == drm_default_rgb_quant_range(mode))
>  		frame->quantization_range = rgb_quant_range;
>  	else
>  		frame->quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT;
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 351f837b09a0..af16b0fa6b69 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -455,17 +455,19 @@ static void intel_hdmi_set_avi_infoframe(struct drm_encoder *encoder,
>  					 const struct intel_crtc_state *crtc_state)
>  {
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(encoder);
> +	const struct drm_display_mode *adjusted_mode =
> +		&crtc_state->base.adjusted_mode;
>  	union hdmi_infoframe frame;
>  	int ret;
>  
>  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> -						       &crtc_state->base.adjusted_mode);
> +						       adjusted_mode);
>  	if (ret < 0) {
>  		DRM_ERROR("couldn't fill AVI infoframe\n");
>  		return;
>  	}
>  
> -	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> +	drm_hdmi_avi_infoframe_quant_range(&frame.avi, adjusted_mode,
>  					   crtc_state->limited_color_range ?
>  					   HDMI_QUANTIZATION_RANGE_LIMITED :
>  					   HDMI_QUANTIZATION_RANGE_FULL,
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index a588156b5410..f38fdbac2878 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -356,7 +356,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
>  		return;
>  	}
>  
> -	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> +	drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
>  					   vc4_encoder->limited_rgb_range ?
>  					   HDMI_QUANTIZATION_RANGE_LIMITED :
>  					   HDMI_QUANTIZATION_RANGE_FULL,
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index cfad4d89589f..43fb0ac5eb9c 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -347,6 +347,7 @@ drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
>  					    const struct drm_display_mode *mode);
>  void
>  drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
> +				   const struct drm_display_mode *mode,
>  				   enum hdmi_quantization_range rgb_quant_range,
>  				   bool rgb_quant_range_selectable);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 5/5] drm/edid: Set YQ bits in the AVI infoframe according to CEA-861-F
  2017-01-11 12:57 ` [PATCH 5/5] drm/edid: Set YQ bits in the AVI infoframe according to CEA-861-F ville.syrjala
@ 2017-01-12 10:13   ` Jani Nikula
  2017-01-12 10:35     ` Ville Syrjälä
  0 siblings, 1 reply; 25+ messages in thread
From: Jani Nikula @ 2017-01-12 10:13 UTC (permalink / raw)
  To: ville.syrjala, dri-devel; +Cc: intel-gfx

On Wed, 11 Jan 2017, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> CEA-861-F tells us:
> "When transmitting any RGB colorimetry, the Source should set the
>  YQ-field to match the RGB Quantization Range being transmitted
>  (e.g., when Limited Range RGB, set YQ=0 or when Full Range RGB,
>  set YQ=1) and the Sink shall ignore the YQ-field."
>
> So let's go ahead and do that. Perhaps there are sinks that don't
> ignore the YQ as they should for RGB?
>
> I wasn't able to find similar text in CEA-861-E, so it would seem
> to be a fairly "recent" addition.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index caa2435bac31..6ba9a1a6eae4 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4320,6 +4320,20 @@ drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
>  		frame->quantization_range = rgb_quant_range;
>  	else
>  		frame->quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT;
> +
> +	/*
> +	 * CEA-861-F:
> +	 * "When transmitting any RGB colorimetry, the Source should set the
> +	 *  YQ-field to match the RGB Quantization Range being transmitted
> +	 *  (e.g., when Limited Range RGB, set YQ=0 or when Full Range RGB,
> +	 *  set YQ=1) and the Sink shall ignore the YQ-field."
> +	 */

*rolls eyes* but that's what the spec says.

> +	if (rgb_quant_range == HDMI_QUANTIZATION_RANGE_LIMITED)
> +		frame->ycc_quantization_range =
> +			HDMI_YCC_QUANTIZATION_RANGE_LIMITED;
> +	else
> +		frame->ycc_quantization_range =
> +			HDMI_YCC_QUANTIZATION_RANGE_FULL;

Shouldn't this take into account QS=0 and rgb_quant_range != default?

BR,
Jani.



>  }
>  EXPORT_SYMBOL(drm_hdmi_avi_infoframe_quant_range);

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 5/5] drm/edid: Set YQ bits in the AVI infoframe according to CEA-861-F
  2017-01-12 10:13   ` Jani Nikula
@ 2017-01-12 10:35     ` Ville Syrjälä
  2017-01-12 11:13       ` Jani Nikula
  0 siblings, 1 reply; 25+ messages in thread
From: Ville Syrjälä @ 2017-01-12 10:35 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, dri-devel

On Thu, Jan 12, 2017 at 12:13:47PM +0200, Jani Nikula wrote:
> On Wed, 11 Jan 2017, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > CEA-861-F tells us:
> > "When transmitting any RGB colorimetry, the Source should set the
> >  YQ-field to match the RGB Quantization Range being transmitted
> >  (e.g., when Limited Range RGB, set YQ=0 or when Full Range RGB,
> >  set YQ=1) and the Sink shall ignore the YQ-field."
> >
> > So let's go ahead and do that. Perhaps there are sinks that don't
> > ignore the YQ as they should for RGB?
> >
> > I wasn't able to find similar text in CEA-861-E, so it would seem
> > to be a fairly "recent" addition.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_edid.c | 14 ++++++++++++++
> >  1 file changed, 14 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index caa2435bac31..6ba9a1a6eae4 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -4320,6 +4320,20 @@ drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
> >  		frame->quantization_range = rgb_quant_range;
> >  	else
> >  		frame->quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT;
> > +
> > +	/*
> > +	 * CEA-861-F:
> > +	 * "When transmitting any RGB colorimetry, the Source should set the
> > +	 *  YQ-field to match the RGB Quantization Range being transmitted
> > +	 *  (e.g., when Limited Range RGB, set YQ=0 or when Full Range RGB,
> > +	 *  set YQ=1) and the Sink shall ignore the YQ-field."
> > +	 */
> 
> *rolls eyes* but that's what the spec says.
> 
> > +	if (rgb_quant_range == HDMI_QUANTIZATION_RANGE_LIMITED)
> > +		frame->ycc_quantization_range =
> > +			HDMI_YCC_QUANTIZATION_RANGE_LIMITED;
> > +	else
> > +		frame->ycc_quantization_range =
> > +			HDMI_YCC_QUANTIZATION_RANGE_FULL;
> 
> Shouldn't this take into account QS=0 and rgb_quant_range != default?

There is no YQ setting similar to the default Q=0. YQ=0 means "limited",
YQ=1 means "full", others values are reserved. So we can't really not
specify the YCC quantization range. So I can't really see any better
option than telling the truth.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 5/5] drm/edid: Set YQ bits in the AVI infoframe according to CEA-861-F
  2017-01-12 10:35     ` Ville Syrjälä
@ 2017-01-12 11:13       ` Jani Nikula
  0 siblings, 0 replies; 25+ messages in thread
From: Jani Nikula @ 2017-01-12 11:13 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx, dri-devel

On Thu, 12 Jan 2017, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Thu, Jan 12, 2017 at 12:13:47PM +0200, Jani Nikula wrote:
>> On Wed, 11 Jan 2017, ville.syrjala@linux.intel.com wrote:
>> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> >
>> > CEA-861-F tells us:
>> > "When transmitting any RGB colorimetry, the Source should set the
>> >  YQ-field to match the RGB Quantization Range being transmitted
>> >  (e.g., when Limited Range RGB, set YQ=0 or when Full Range RGB,
>> >  set YQ=1) and the Sink shall ignore the YQ-field."
>> >
>> > So let's go ahead and do that. Perhaps there are sinks that don't
>> > ignore the YQ as they should for RGB?
>> >
>> > I wasn't able to find similar text in CEA-861-E, so it would seem
>> > to be a fairly "recent" addition.
>> >
>> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/drm_edid.c | 14 ++++++++++++++
>> >  1 file changed, 14 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>> > index caa2435bac31..6ba9a1a6eae4 100644
>> > --- a/drivers/gpu/drm/drm_edid.c
>> > +++ b/drivers/gpu/drm/drm_edid.c
>> > @@ -4320,6 +4320,20 @@ drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
>> >  		frame->quantization_range = rgb_quant_range;
>> >  	else
>> >  		frame->quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT;
>> > +
>> > +	/*
>> > +	 * CEA-861-F:
>> > +	 * "When transmitting any RGB colorimetry, the Source should set the
>> > +	 *  YQ-field to match the RGB Quantization Range being transmitted
>> > +	 *  (e.g., when Limited Range RGB, set YQ=0 or when Full Range RGB,
>> > +	 *  set YQ=1) and the Sink shall ignore the YQ-field."
>> > +	 */
>> 
>> *rolls eyes* but that's what the spec says.
>> 
>> > +	if (rgb_quant_range == HDMI_QUANTIZATION_RANGE_LIMITED)
>> > +		frame->ycc_quantization_range =
>> > +			HDMI_YCC_QUANTIZATION_RANGE_LIMITED;
>> > +	else
>> > +		frame->ycc_quantization_range =
>> > +			HDMI_YCC_QUANTIZATION_RANGE_FULL;
>> 
>> Shouldn't this take into account QS=0 and rgb_quant_range != default?
>
> There is no YQ setting similar to the default Q=0. YQ=0 means "limited",
> YQ=1 means "full", others values are reserved. So we can't really not
> specify the YCC quantization range. So I can't really see any better
> option than telling the truth.

Hmm, I was confused about the caller choosing the range based on the
mode already.

Reviewed-by: Jani Nikula <jani.nikula@intel.com>



-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Intel-gfx] [PATCH v2 2/5] drm/edid: Introduce drm_default_rgb_quant_range()
  2017-01-12  9:29     ` Jani Nikula
@ 2017-01-12 14:24       ` Ville Syrjälä
  0 siblings, 0 replies; 25+ messages in thread
From: Ville Syrjälä @ 2017-01-12 14:24 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx, dri-devel

On Thu, Jan 12, 2017 at 11:29:18AM +0200, Jani Nikula wrote:
> On Wed, 11 Jan 2017, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Make the code selecting the RGB quantization range a little less magicy
> > by wrapping it up in a small helper.
> >
> > v2: s/adjusted_mode/mode in vc4 to make it actually compile
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_edid.c        | 18 ++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_dp.c   |  4 +++-
> >  drivers/gpu/drm/i915/intel_hdmi.c |  3 ++-
> >  drivers/gpu/drm/vc4/vc4_hdmi.c    |  4 +++-
> >  include/drm/drm_edid.h            |  2 ++
> >  5 files changed, 28 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 4ff04aa84dd0..304c583b8000 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -3768,6 +3768,24 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
> >  }
> >  EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
> >  
> > +/**
> > + * drm_default_rgb_quant_range - default RGB quantization range
> > + * @mode: display mode
> > + *
> > + * Determine the default RGB quantization range for the mode,
> > + * as specified in CEA-861.
> > + *
> > + * Return: The default RGB quantization range for the mode
> > + */
> > +enum hdmi_quantization_range
> > +drm_default_rgb_quant_range(const struct drm_display_mode *mode)
> > +{
> > +	return drm_match_cea_mode(mode) > 1 ?
> > +		HDMI_QUANTIZATION_RANGE_LIMITED :
> > +		HDMI_QUANTIZATION_RANGE_FULL;
> > +}
> > +EXPORT_SYMBOL(drm_default_rgb_quant_range);
> 
> Or just bool drm_default_to_limited_range() or similar?

That's what I had initially, but then I thought it might be better to
start moving towards something a bit more explicit everwhere. But I
stopped short of actually making the drivers store the enum in their
state. We might want to do that, I think.

> 
> *shrug*
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> 
> > +
> >  static void drm_parse_hdmi_deep_color_info(struct drm_connector *connector,
> >  					   const u8 *hdmi)
> >  {
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 343e1d9fa761..d4befbbe834a 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -1713,7 +1713,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
> >  		 * VESA DisplayPort Ver.1.2a - 5.1.1.1 Video Colorimetry
> >  		 */
> >  		pipe_config->limited_color_range =
> > -			bpp != 18 && drm_match_cea_mode(adjusted_mode) > 1;
> > +			bpp != 18 &&
> > +			drm_default_rgb_quant_range(adjusted_mode) ==
> > +			HDMI_QUANTIZATION_RANGE_LIMITED;
> >  	} else {
> >  		pipe_config->limited_color_range =
> >  			intel_dp->limited_color_range;
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 0bcfead14571..19bd13f53729 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1330,7 +1330,8 @@ bool intel_hdmi_compute_config(struct intel_encoder *encoder,
> >  		/* See CEA-861-E - 5.1 Default Encoding Parameters */
> >  		pipe_config->limited_color_range =
> >  			pipe_config->has_hdmi_sink &&
> > -			drm_match_cea_mode(adjusted_mode) > 1;
> > +			drm_default_rgb_quant_range(adjusted_mode) ==
> > +			HDMI_QUANTIZATION_RANGE_LIMITED;
> >  	} else {
> >  		pipe_config->limited_color_range =
> >  			intel_hdmi->limited_color_range;
> > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > index c4cb2e26de32..5d49bf948162 100644
> > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> > @@ -463,7 +463,9 @@ static void vc4_hdmi_encoder_mode_set(struct drm_encoder *encoder,
> >  	csc_ctl = VC4_SET_FIELD(VC4_HD_CSC_CTL_ORDER_BGR,
> >  				VC4_HD_CSC_CTL_ORDER);
> >  
> > -	if (vc4_encoder->hdmi_monitor && drm_match_cea_mode(mode) > 1) {
> > +	if (vc4_encoder->hdmi_monitor &&
> > +	    drm_default_rgb_quant_range(mode) ==
> > +	    HDMI_QUANTIZATION_RANGE_LIMITED) {
> >  		/* CEA VICs other than #1 requre limited range RGB
> >  		 * output unless overridden by an AVI infoframe.
> >  		 * Apply a colorspace conversion to squash 0-255 down
> > diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> > index 838eaf2b42e9..25cdf5f7a0d8 100644
> > --- a/include/drm/drm_edid.h
> > +++ b/include/drm/drm_edid.h
> > @@ -441,6 +441,8 @@ enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);
> >  bool drm_detect_hdmi_monitor(struct edid *edid);
> >  bool drm_detect_monitor_audio(struct edid *edid);
> >  bool drm_rgb_quant_range_selectable(struct edid *edid);
> > +enum hdmi_quantization_range
> > +drm_default_rgb_quant_range(const struct drm_display_mode *mode);
> >  int drm_add_modes_noedid(struct drm_connector *connector,
> >  			 int hdisplay, int vdisplay);
> >  void drm_set_preferred_mode(struct drm_connector *connector,
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range()
  2017-01-11 12:57 ` [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range() ville.syrjala
  2017-01-11 14:18   ` [PATCH v2 " ville.syrjala
  2017-01-11 16:16   ` [PATCH " Daniel Vetter
@ 2017-01-20 19:50   ` Eric Anholt
  2017-01-20 20:00     ` Ville Syrjälä
  2 siblings, 1 reply; 25+ messages in thread
From: Eric Anholt @ 2017-01-20 19:50 UTC (permalink / raw)
  To: ville.syrjala, dri-devel; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 2018 bytes --]

ville.syrjala@linux.intel.com writes:

> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Make the code selecting the RGB quantization range a little less magicy
> by wrapping it up in a small helper.

This series seems good.  I won't have the ability to test it on vc4
within a reasonable time frame, so you can add an Acked-by from me if
you'd like.  Just one comment that you can take or leave:

> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_edid.c        | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c   |  4 +++-
>  drivers/gpu/drm/i915/intel_hdmi.c |  3 ++-
>  drivers/gpu/drm/vc4/vc4_hdmi.c    |  4 +++-
>  include/drm/drm_edid.h            |  2 ++
>  5 files changed, 28 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index 4ff04aa84dd0..304c583b8000 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -3768,6 +3768,24 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
>  }
>  EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
>  
> +/**
> + * drm_default_rgb_quant_range - default RGB quantization range
> + * @mode: display mode
> + *
> + * Determine the default RGB quantization range for the mode,
> + * as specified in CEA-861.
> + *
> + * Return: The default RGB quantization range for the mode
> + */
> +enum hdmi_quantization_range
> +drm_default_rgb_quant_range(const struct drm_display_mode *mode)
> +{
> +	return drm_match_cea_mode(mode) > 1 ?
> +		HDMI_QUANTIZATION_RANGE_LIMITED :
> +		HDMI_QUANTIZATION_RANGE_FULL;

It might be nice to add a comment here like

/* All CEA modes other than VIC 1 use limited quantization range. */

When I first had to put this logic in vc4, I was surprised by it and
figured that it was an off-by-one bug in code that was trying to say "if
it's any CEA mode then it should be limited range"


> +}
> +EXPORT_SYMBOL(drm_default_rgb_quant_range);

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range()
  2017-01-20 19:50   ` Eric Anholt
@ 2017-01-20 20:00     ` Ville Syrjälä
  2017-01-20 20:37       ` [Intel-gfx] " Ville Syrjälä
  0 siblings, 1 reply; 25+ messages in thread
From: Ville Syrjälä @ 2017-01-20 20:00 UTC (permalink / raw)
  To: Eric Anholt; +Cc: intel-gfx, dri-devel

On Sat, Jan 21, 2017 at 06:50:57AM +1100, Eric Anholt wrote:
> ville.syrjala@linux.intel.com writes:
> 
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Make the code selecting the RGB quantization range a little less magicy
> > by wrapping it up in a small helper.
> 
> This series seems good.  I won't have the ability to test it on vc4
> within a reasonable time frame, so you can add an Acked-by from me if
> you'd like.  Just one comment that you can take or leave:
> 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_edid.c        | 18 ++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_dp.c   |  4 +++-
> >  drivers/gpu/drm/i915/intel_hdmi.c |  3 ++-
> >  drivers/gpu/drm/vc4/vc4_hdmi.c    |  4 +++-
> >  include/drm/drm_edid.h            |  2 ++
> >  5 files changed, 28 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index 4ff04aa84dd0..304c583b8000 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -3768,6 +3768,24 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
> >  }
> >  EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
> >  
> > +/**
> > + * drm_default_rgb_quant_range - default RGB quantization range
> > + * @mode: display mode
> > + *
> > + * Determine the default RGB quantization range for the mode,
> > + * as specified in CEA-861.
> > + *
> > + * Return: The default RGB quantization range for the mode
> > + */
> > +enum hdmi_quantization_range
> > +drm_default_rgb_quant_range(const struct drm_display_mode *mode)
> > +{
> > +	return drm_match_cea_mode(mode) > 1 ?
> > +		HDMI_QUANTIZATION_RANGE_LIMITED :
> > +		HDMI_QUANTIZATION_RANGE_FULL;
> 
> It might be nice to add a comment here like
> 
> /* All CEA modes other than VIC 1 use limited quantization range. */
> 
> When I first had to put this logic in vc4, I was surprised by it and
> figured that it was an off-by-one bug in code that was trying to say "if
> it's any CEA mode then it should be limited range"

Seems like a good idea. I could even try to dig up a specific spec
quote perhaps? And if I can't find anything succinct in the spec we
can always go with your proposal.

> 
> 
> > +}
> > +EXPORT_SYMBOL(drm_default_rgb_quant_range);

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [Intel-gfx] [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range()
  2017-01-20 20:00     ` Ville Syrjälä
@ 2017-01-20 20:37       ` Ville Syrjälä
  0 siblings, 0 replies; 25+ messages in thread
From: Ville Syrjälä @ 2017-01-20 20:37 UTC (permalink / raw)
  To: Eric Anholt; +Cc: intel-gfx, dri-devel

On Fri, Jan 20, 2017 at 10:00:34PM +0200, Ville Syrjälä wrote:
> On Sat, Jan 21, 2017 at 06:50:57AM +1100, Eric Anholt wrote:
> > ville.syrjala@linux.intel.com writes:
> > 
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > Make the code selecting the RGB quantization range a little less magicy
> > > by wrapping it up in a small helper.
> > 
> > This series seems good.  I won't have the ability to test it on vc4
> > within a reasonable time frame, so you can add an Acked-by from me if
> > you'd like.  Just one comment that you can take or leave:
> > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_edid.c        | 18 ++++++++++++++++++
> > >  drivers/gpu/drm/i915/intel_dp.c   |  4 +++-
> > >  drivers/gpu/drm/i915/intel_hdmi.c |  3 ++-
> > >  drivers/gpu/drm/vc4/vc4_hdmi.c    |  4 +++-
> > >  include/drm/drm_edid.h            |  2 ++
> > >  5 files changed, 28 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > > index 4ff04aa84dd0..304c583b8000 100644
> > > --- a/drivers/gpu/drm/drm_edid.c
> > > +++ b/drivers/gpu/drm/drm_edid.c
> > > @@ -3768,6 +3768,24 @@ bool drm_rgb_quant_range_selectable(struct edid *edid)
> > >  }
> > >  EXPORT_SYMBOL(drm_rgb_quant_range_selectable);
> > >  
> > > +/**
> > > + * drm_default_rgb_quant_range - default RGB quantization range
> > > + * @mode: display mode
> > > + *
> > > + * Determine the default RGB quantization range for the mode,
> > > + * as specified in CEA-861.
> > > + *
> > > + * Return: The default RGB quantization range for the mode
> > > + */
> > > +enum hdmi_quantization_range
> > > +drm_default_rgb_quant_range(const struct drm_display_mode *mode)
> > > +{
> > > +	return drm_match_cea_mode(mode) > 1 ?
> > > +		HDMI_QUANTIZATION_RANGE_LIMITED :
> > > +		HDMI_QUANTIZATION_RANGE_FULL;
> > 
> > It might be nice to add a comment here like
> > 
> > /* All CEA modes other than VIC 1 use limited quantization range. */
> > 
> > When I first had to put this logic in vc4, I was surprised by it and
> > figured that it was an off-by-one bug in code that was trying to say "if
> > it's any CEA mode then it should be limited range"
> 
> Seems like a good idea. I could even try to dig up a specific spec
> quote perhaps? And if I can't find anything succinct in the spec we
> can always go with your proposal.

Meh. Didn't manage to dig up anything really nice from the spec, so
I'll just go with your idea.

> 
> > 
> > 
> > > +}
> > > +EXPORT_SYMBOL(drm_default_rgb_quant_range);
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 25+ messages in thread

* Re: [PATCH 0/5] drm/edid: Improve RGB limited range handling a bit
  2017-01-11 12:57 [PATCH 0/5] drm/edid: Improve RGB limited range handling a bit ville.syrjala
                   ` (6 preceding siblings ...)
  2017-01-11 18:53 ` ✓ Fi.CI.BAT: success for drm/edid: Improve RGB limited range handling a bit (rev2) Patchwork
@ 2017-01-26 19:08 ` Ville Syrjälä
  7 siblings, 0 replies; 25+ messages in thread
From: Ville Syrjälä @ 2017-01-26 19:08 UTC (permalink / raw)
  To: dri-devel; +Cc: intel-gfx

On Wed, Jan 11, 2017 at 02:57:20PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> While reading the HDMI 2.0 spec I noticed some new things related to
> the RGB quantization range stuff, and after cross checking with
> CEA-861-F I spotted a some other things as well. So I figured I should
> pimp up the code a bit.
> 
> And since we now have two drivers that deal with this stuff, I decided
> to move a bunch of the code to the core to avoid duplicating the code
> and having different bugs/features for each driver. I still left the state
> computation part in the drivers, but eventually we might want to move that
> code into some helper as well.
> 
> Entire series available here:
> git://github.com/vsyrjala/linux.git hdmi_quant_range_helpers
> 
> Ville Syrjälä (5):
>   drm/edid: Have drm_edid.h include hdmi.h
>   drm/edid: Introduce drm_default_rgb_quant_range()
>   drm/edid: Introduce drm_hdmi_avi_infoframe_quant_range()
>   drm/edid: Set AVI infoframe Q even when QS=0
>   drm/edid: Set YQ bits in the AVI infoframe according to CEA-861-F

Series pushed to drm-misc-next. Thanks for the reviews/acks.

> 
>  drivers/gpu/drm/drm_edid.c        | 64 +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c   |  4 ++-
>  drivers/gpu/drm/i915/intel_hdmi.c | 20 ++++++------
>  drivers/gpu/drm/vc4/vc4_hdmi.c    | 18 +++++------
>  include/drm/drm_edid.h            | 10 ++++--
>  5 files changed, 93 insertions(+), 23 deletions(-)
> 
> -- 
> 2.10.2

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2017-01-26 19:08 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-01-11 12:57 [PATCH 0/5] drm/edid: Improve RGB limited range handling a bit ville.syrjala
2017-01-11 12:57 ` [PATCH 1/5] drm/edid: Have drm_edid.h include hdmi.h ville.syrjala
2017-01-12  9:22   ` Jani Nikula
2017-01-11 12:57 ` [PATCH 2/5] drm/edid: Introduce drm_default_rgb_quant_range() ville.syrjala
2017-01-11 14:18   ` [PATCH v2 " ville.syrjala
2017-01-12  9:29     ` Jani Nikula
2017-01-12 14:24       ` [Intel-gfx] " Ville Syrjälä
2017-01-11 16:16   ` [PATCH " Daniel Vetter
2017-01-11 16:31     ` Ville Syrjälä
2017-01-12  8:00       ` Daniel Vetter
2017-01-12  8:13         ` Michel Dänzer
2017-01-20 19:50   ` Eric Anholt
2017-01-20 20:00     ` Ville Syrjälä
2017-01-20 20:37       ` [Intel-gfx] " Ville Syrjälä
2017-01-11 12:57 ` [PATCH 3/5] drm/edid: Introduce drm_hdmi_avi_infoframe_quant_range() ville.syrjala
2017-01-12  9:37   ` Jani Nikula
2017-01-11 12:57 ` [PATCH 4/5] drm/edid: Set AVI infoframe Q even when QS=0 ville.syrjala
2017-01-12  9:45   ` Jani Nikula
2017-01-11 12:57 ` [PATCH 5/5] drm/edid: Set YQ bits in the AVI infoframe according to CEA-861-F ville.syrjala
2017-01-12 10:13   ` Jani Nikula
2017-01-12 10:35     ` Ville Syrjälä
2017-01-12 11:13       ` Jani Nikula
2017-01-11 13:53 ` ✓ Fi.CI.BAT: success for drm/edid: Improve RGB limited range handling a bit Patchwork
2017-01-11 18:53 ` ✓ Fi.CI.BAT: success for drm/edid: Improve RGB limited range handling a bit (rev2) Patchwork
2017-01-26 19:08 ` [PATCH 0/5] drm/edid: Improve RGB limited range handling a bit 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.