[PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Ville Syrjala]

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

Make life easier for drivers by simply passing the connector
to drm_hdmi_avi_infoframe_from_display_mode() and
drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
need to worry about is_hdmi2_sink mess.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
Cc: Archit Taneja <architt@codeaurora.org>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: CK Hu <ck.hu@mediatek.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Sandy Huang <hjc@rock-chips.com>
Cc: "Heiko Stübner" <heiko@sntech.de>
Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Vincent Abriou <vincent.abriou@st.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Ilia Mirkin <imirkin@alum.mit.edu>
Cc: amd-gfx@lists.freedesktop.org
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org
Cc: linux-tegra@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  3 ++-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  2 +-
 drivers/gpu/drm/bridge/analogix-anx78xx.c |  5 ++--
 drivers/gpu/drm/bridge/sii902x.c          |  3 ++-
 drivers/gpu/drm/bridge/sil-sii8620.c      |  3 +--
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  3 ++-
 drivers/gpu/drm/drm_edid.c                | 33 ++++++++++++++---------
 drivers/gpu/drm/exynos/exynos_hdmi.c      |  3 ++-
 drivers/gpu/drm/i2c/tda998x_drv.c         |  3 ++-
 drivers/gpu/drm/i915/intel_hdmi.c         | 14 +++++-----
 drivers/gpu/drm/i915/intel_lspcon.c       | 15 ++++++-----
 drivers/gpu/drm/i915/intel_sdvo.c         | 10 ++++---
 drivers/gpu/drm/mediatek/mtk_hdmi.c       |  3 ++-
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |  3 ++-
 drivers/gpu/drm/nouveau/dispnv50/disp.c   |  7 +++--
 drivers/gpu/drm/omapdrm/omap_encoder.c    |  5 ++--
 drivers/gpu/drm/radeon/radeon_audio.c     |  2 +-
 drivers/gpu/drm/rockchip/inno_hdmi.c      |  4 ++-
 drivers/gpu/drm/sti/sti_hdmi.c            |  3 ++-
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |  3 ++-
 drivers/gpu/drm/tegra/hdmi.c              |  3 ++-
 drivers/gpu/drm/tegra/sor.c               |  3 ++-
 drivers/gpu/drm/vc4/vc4_hdmi.c            | 11 +++++---
 drivers/gpu/drm/zte/zx_hdmi.c             |  4 ++-
 include/drm/drm_edid.h                    |  8 +++---
 27 files changed, 94 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index 4cfecdce29a3..1f0426d2fc2a 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -1682,7 +1682,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder *encoder,
 	dce_v10_0_audio_write_sad_regs(encoder);
 	dce_v10_0_audio_write_latency_fields(encoder, mode);
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
 	if (err < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
 		return;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 7c868916d90f..2280b971d758 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -1724,7 +1724,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder *encoder,
 	dce_v11_0_audio_write_sad_regs(encoder);
 	dce_v11_0_audio_write_latency_fields(encoder, mode);
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
 	if (err < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
 		return;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index 17eaaba36017..db443ec53d3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -1423,6 +1423,7 @@ static void dce_v6_0_audio_set_avi_infoframe(struct drm_encoder *encoder,
 	struct amdgpu_device *adev = dev->dev_private;
 	struct amdgpu_encoder *amdgpu_encoder = to_amdgpu_encoder(encoder);
 	struct amdgpu_encoder_atom_dig *dig = amdgpu_encoder->enc_priv;
+	struct drm_connector *connector = amdgpu_get_connector_for_encoder(encoder);
 	struct hdmi_avi_infoframe frame;
 	u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE];
 	uint8_t *payload = buffer + 3;
@@ -1430,7 +1431,7 @@ static void dce_v6_0_audio_set_avi_infoframe(struct drm_encoder *encoder,
 	ssize_t err;
 	u32 tmp;
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
 	if (err < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
 		return;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index 8c0576978d36..13da915991dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -1616,7 +1616,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
 	dce_v8_0_audio_write_sad_regs(encoder);
 	dce_v8_0_audio_write_latency_fields(encoder, mode);
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
 	if (err < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
 		return;
diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index f8433c93f463..e11309e9bc4f 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -1094,8 +1094,9 @@ static void anx78xx_bridge_mode_set(struct drm_bridge *bridge,
 
 	mutex_lock(&anx78xx->lock);
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode,
-						       false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
+						       &anx78xx->connector,
+						       adjusted_mode);
 	if (err) {
 		DRM_ERROR("Failed to setup AVI infoframe: %d\n", err);
 		goto unlock;
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index bfa902013aa4..a9b4f45ae87c 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -258,7 +258,8 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
 	if (ret)
 		return;
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj, false);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame,
+						       &sii902x->connector, adj);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return;
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
index a6e8f4591e63..0cc293a6ac24 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -1104,8 +1104,7 @@ static void sii8620_set_infoframes(struct sii8620 *ctx,
 	int ret;
 
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
-						       mode,
-						       true);
+						       NULL, mode);
 	if (ctx->use_packed_pixel)
 		frm.avi.colorspace = HDMI_COLORSPACE_YUV422;
 
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 64c3cf027518..88b720b63126 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1344,7 +1344,8 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
 	u8 val;
 
 	/* Initialise info frame from DRM mode */
-	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	drm_hdmi_avi_infoframe_from_display_mode(&frame,
+						 &hdmi->connector, mode);
 
 	if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
 		frame.colorspace = HDMI_COLORSPACE_YUV444;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index b506e3622b08..501ac05ba7da 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4830,19 +4830,32 @@ void drm_set_preferred_mode(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_set_preferred_mode);
 
+static bool is_hdmi2_sink(struct drm_connector *connector)
+{
+	/*
+	 * FIXME: sil-sii8620 doesn't have a connector around when
+	 * we need one, so we have to be prepared for a NULL connector.
+	 */
+	if (!connector)
+		return false;
+
+	return connector->display_info.hdmi.scdc.supported ||
+		connector->display_info.color_formats & DRM_COLOR_FORMAT_YCRCB420;
+}
+
 /**
  * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with
  *                                              data from a DRM display mode
  * @frame: HDMI AVI infoframe
+ * @connector: the connector
  * @mode: DRM display mode
- * @is_hdmi2_sink: Sink is HDMI 2.0 compliant
  *
  * Return: 0 on success or a negative error code on failure.
  */
 int
 drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
-					 const struct drm_display_mode *mode,
-					 bool is_hdmi2_sink)
+					 struct drm_connector *connector,
+					 const struct drm_display_mode *mode)
 {
 	enum hdmi_picture_aspect picture_aspect;
 	int err;
@@ -4864,7 +4877,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 	 * HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
 	 * have to make sure we dont break HDMI 1.4 sinks.
 	 */
-	if (!is_hdmi2_sink && frame->video_code > 64)
+	if (!is_hdmi2_sink(connector) && frame->video_code > 64)
 		frame->video_code = 0;
 
 	/*
@@ -4923,21 +4936,17 @@ 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
+ * @connector: the connector
  * @mode: DRM display mode
  * @rgb_quant_range: RGB quantization range (Q)
  * @rgb_quant_range_selectable: Sink support selectable RGB quantization range (QS)
- * @is_hdmi2_sink: HDMI 2.0 sink, which has different default recommendations
- *
- * Note that @is_hdmi2_sink can be derived by looking at the
- * &drm_scdc.supported flag stored in &drm_hdmi_info.scdc,
- * &drm_display_info.hdmi, which can be found in &drm_connector.display_info.
  */
 void
 drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
+				   struct drm_connector *connector,
 				   const struct drm_display_mode *mode,
 				   enum hdmi_quantization_range rgb_quant_range,
-				   bool rgb_quant_range_selectable,
-				   bool is_hdmi2_sink)
+				   bool rgb_quant_range_selectable)
 {
 	/*
 	 * CEA-861:
@@ -4968,7 +4977,7 @@ drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
 	 * we limit non-zero YQ to HDMI 2.0 sinks only as HDMI 2.0 is based
 	 * on on CEA-861-F.
 	 */
-	if (!is_hdmi2_sink ||
+	if (!is_hdmi2_sink(connector) ||
 	    rgb_quant_range == HDMI_QUANTIZATION_RANGE_LIMITED)
 		frame->ycc_quantization_range =
 			HDMI_YCC_QUANTIZATION_RANGE_LIMITED;
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 2092a650df7d..b857df67aff0 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -819,7 +819,8 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata)
 		return;
 	}
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi, m, false);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
+						       &hdata->connector, m);
 	if (!ret)
 		ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
 	if (ret > 0) {
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index a7c39f39793f..38c66fbc8276 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -849,7 +849,8 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode)
 {
 	union hdmi_infoframe frame;
 
-	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
+	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
+						 &priv->connector, mode);
 	frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL;
 
 	tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index e2c6a2b3e8f2..8a260bd6954c 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -465,15 +465,12 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	const struct drm_display_mode *adjusted_mode =
 		&crtc_state->base.adjusted_mode;
-	struct drm_connector *connector = &intel_hdmi->attached_connector->base;
-	bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported ||
-	   connector->display_info.color_formats & DRM_COLOR_FORMAT_YCRCB420;
 	union hdmi_infoframe frame;
 	int ret;
 
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
-						       adjusted_mode,
-						       is_hdmi2_sink);
+						       conn_state->connector,
+						       adjusted_mode);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return;
@@ -486,12 +483,13 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder,
 	else
 		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
 
-	drm_hdmi_avi_infoframe_quant_range(&frame.avi, adjusted_mode,
+	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
+					   conn_state->connector,
+					   adjusted_mode,
 					   crtc_state->limited_color_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
 					   HDMI_QUANTIZATION_RANGE_FULL,
-					   intel_hdmi->rgb_quant_range_selectable,
-					   is_hdmi2_sink);
+					   intel_hdmi->rgb_quant_range_selectable);
 
 	drm_hdmi_avi_infoframe_content_type(&frame.avi,
 					    conn_state);
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 96a8d9524b0c..c18f14d41f1d 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -462,10 +462,8 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
 	uint8_t buf[VIDEO_DIP_DATA_SIZE];
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 	struct intel_lspcon *lspcon = &dig_port->lspcon;
-	struct intel_dp *intel_dp = &dig_port->dp;
-	struct drm_connector *connector = &intel_dp->attached_connector->base;
-	const struct drm_display_mode *mode = &crtc_state->base.adjusted_mode;
-	bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported;
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->base.adjusted_mode;
 
 	if (!lspcon->active) {
 		DRM_ERROR("Writing infoframes while LSPCON disabled ?\n");
@@ -473,7 +471,8 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
 	}
 
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
-						       mode, is_hdmi2_sink);
+						       conn_state->connector,
+						       adjusted_mode);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return;
@@ -488,11 +487,13 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
 		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
 	}
 
-	drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
+	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
+					   conn_state->connector,
+					   adjusted_mode,
 					   crtc_state->limited_color_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
 					   HDMI_QUANTIZATION_RANGE_FULL,
-					   false, is_hdmi2_sink);
+					   false);
 
 	ret = hdmi_infoframe_pack(&frame, buf, sizeof(buf));
 	if (ret < 0) {
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 5805ec1aba12..1277d31adb54 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -981,7 +981,8 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
 }
 
 static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
-					 const struct intel_crtc_state *pipe_config)
+					 const struct intel_crtc_state *pipe_config,
+					 const struct drm_connector_state *conn_state)
 {
 	uint8_t sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
 	union hdmi_infoframe frame;
@@ -989,8 +990,8 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
 	ssize_t len;
 
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
-						       &pipe_config->base.adjusted_mode,
-						       false);
+						       conn_state->connector,
+						       &pipe_config->base.adjusted_mode);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return false;
@@ -1316,7 +1317,8 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_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, crtc_state);
+		intel_sdvo_set_avi_infoframe(intel_sdvo,
+					     crtc_state, conn_state);
 	} else
 		intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_DVI);
 
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 862f3ec22131..f3ef7bf80563 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -981,7 +981,8 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi,
 	u8 buffer[17];
 	ssize_t err;
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
+						       &hdmi->conn, mode);
 	if (err < 0) {
 		dev_err(hdmi->dev,
 			"Failed to get AVI infoframe from mode: %zd\n", err);
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 7e357077ed26..5ed4cab2819f 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -101,7 +101,8 @@ static void msm_hdmi_config_avi_infoframe(struct hdmi *hdmi)
 	u32 val;
 	int len;
 
-	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
+	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
+						 hdmi->connector, mode);
 
 	len = hdmi_infoframe_pack(&frame, buffer, sizeof(buffer));
 	if (len < 0) {
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 6cbbae3f438b..d8e512c83211 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -554,7 +554,7 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
 	u32 max_ac_packet;
 	union hdmi_infoframe avi_frame;
 	union hdmi_infoframe vendor_frame;
-	bool scdc_supported, high_tmds_clock_ratio = false, scrambling = false;
+	bool high_tmds_clock_ratio = false, scrambling = false;
 	u8 config;
 	int ret;
 	int size;
@@ -564,10 +564,9 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
 		return;
 
 	hdmi = &nv_connector->base.display_info.hdmi;
-	scdc_supported = hdmi->scdc.supported;
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame.avi, mode,
-						       scdc_supported);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame.avi,
+						       &nv_connector->base, mode);
 	if (!ret) {
 		/* We have an AVI InfoFrame, populate it to the display */
 		args.pwr.avi_infoframe_length
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index 452e625f6ce3..281b2164969c 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -128,8 +128,9 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder,
 	if (hdmi_mode && dssdev->ops->hdmi.set_infoframe) {
 		struct hdmi_avi_infoframe avi;
 
-		r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode,
-							     false);
+		r = drm_hdmi_avi_infoframe_from_display_mode(&avi,
+							     connector,
+							     adjusted_mode);
 		if (r == 0)
 			dssdev->ops->hdmi.set_infoframe(dssdev, &avi);
 	}
diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c
index 770e31f5fd1b..5a7d48339b32 100644
--- a/drivers/gpu/drm/radeon/radeon_audio.c
+++ b/drivers/gpu/drm/radeon/radeon_audio.c
@@ -516,7 +516,7 @@ static int radeon_audio_set_avi_packet(struct drm_encoder *encoder,
 	if (!connector)
 		return -EINVAL;
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
 	if (err < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %d\n", err);
 		return err;
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
index 1c02b3e61299..27c945e030a0 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -295,7 +295,9 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
 	union hdmi_infoframe frame;
 	int rc;
 
-	rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
+	rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
+						      &hdmi->connector,
+						      mode);
 
 	if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444)
 		frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index ccf718404a1c..4b86878f8ddf 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -434,7 +434,8 @@ static int hdmi_avi_infoframe_config(struct sti_hdmi *hdmi)
 
 	DRM_DEBUG_DRIVER("\n");
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode, false);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe,
+						       hdmi->drm_connector, mode);
 	if (ret < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %d\n", ret);
 		return ret;
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 061d2e0d9011..554a6f4561f3 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -52,7 +52,8 @@ static int sun4i_hdmi_setup_avi_infoframes(struct sun4i_hdmi *hdmi,
 	u8 buffer[17];
 	int i, ret;
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame,
+						       &hdmi->connector, mode);
 	if (ret < 0) {
 		DRM_ERROR("Failed to get infoframes from mode\n");
 		return ret;
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index 0082468f703c..a7566c67bfb0 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -741,7 +741,8 @@ static void tegra_hdmi_setup_avi_infoframe(struct tegra_hdmi *hdmi,
 	u8 buffer[17];
 	ssize_t err;
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
+						       &hdmi->output.connector, mode);
 	if (err < 0) {
 		dev_err(hdmi->dev, "failed to setup AVI infoframe: %zd\n", err);
 		return;
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index b129da2e5afd..d90bf5f6a67a 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -2116,7 +2116,8 @@ tegra_sor_hdmi_setup_avi_infoframe(struct tegra_sor *sor,
 	value &= ~INFOFRAME_CTRL_ENABLE;
 	tegra_sor_writel(sor, value, SOR_HDMI_AVI_INFOFRAME_CTRL);
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
+						       &sor->output.connector, mode);
 	if (err < 0) {
 		dev_err(sor->dev, "failed to setup AVI infoframe: %d\n", err);
 		return err;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index fd5522fd179e..cb3dca7426c8 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -409,22 +409,25 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
 {
 	struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
 	struct drm_crtc *crtc = encoder->crtc;
+	struct vc4_dev *vc4 = to_vc4_dev(encoder->dev);
+	struct vc4_hdmi *hdmi = vc4->hdmi;
 	const struct drm_display_mode *mode = &crtc->state->adjusted_mode;
 	union hdmi_infoframe frame;
 	int ret;
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
+						       hdmi->connector, mode);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return;
 	}
 
-	drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
+	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
+					   hdmi->connector, mode,
 					   vc4_encoder->limited_rgb_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
 					   HDMI_QUANTIZATION_RANGE_FULL,
-					   vc4_encoder->rgb_range_selectable,
-					   false);
+					   vc4_encoder->rgb_range_selectable);
 
 	vc4_hdmi_write_infoframe(encoder, &frame);
 }
diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c
index 78655269d843..9fc98bb4f3d9 100644
--- a/drivers/gpu/drm/zte/zx_hdmi.c
+++ b/drivers/gpu/drm/zte/zx_hdmi.c
@@ -125,7 +125,9 @@ static int zx_hdmi_config_video_avi(struct zx_hdmi *hdmi,
 	union hdmi_infoframe frame;
 	int ret;
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
+						       &hdmi->connector,
+						       mode);
 	if (ret) {
 		DRM_DEV_ERROR(hdmi->dev, "failed to get avi infoframe: %d\n",
 			      ret);
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index e3c404833115..9db6f130df65 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -352,18 +352,18 @@ drm_load_edid_firmware(struct drm_connector *connector)
 
 int
 drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
-					 const struct drm_display_mode *mode,
-					 bool is_hdmi2_sink);
+					 struct drm_connector *connector,
+					 const struct drm_display_mode *mode);
 int
 drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
 					    struct drm_connector *connector,
 					    const struct drm_display_mode *mode);
 void
 drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
+				   struct drm_connector *connector,
 				   const struct drm_display_mode *mode,
 				   enum hdmi_quantization_range rgb_quant_range,
-				   bool rgb_quant_range_selectable,
-				   bool is_hdmi2_sink);
+				   bool rgb_quant_range_selectable);
 
 /**
  * drm_eld_mnl - Get ELD monitor name length in bytes.
-- 
2.18.1

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

[PATCH 2/4] drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well

[Ville Syrjala]

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

Fill out the AVI infoframe quantization range bits using
drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI encoder as well.

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

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 1277d31adb54..9c16e273fb8d 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -984,6 +984,8 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
 					 const struct intel_crtc_state *pipe_config,
 					 const struct drm_connector_state *conn_state)
 {
+	const struct drm_display_mode *adjusted_mode =
+		&pipe_config->base.adjusted_mode;
 	uint8_t sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
 	union hdmi_infoframe frame;
 	int ret;
@@ -991,20 +993,19 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
 
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
 						       conn_state->connector,
-						       &pipe_config->base.adjusted_mode);
+						       adjusted_mode);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return false;
 	}
 
-	if (intel_sdvo->rgb_quant_range_selectable) {
-		if (pipe_config->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,
+					   conn_state->connector,
+					   adjusted_mode,
+					   pipe_config->limited_color_range ?
+					   HDMI_QUANTIZATION_RANGE_LIMITED :
+					   HDMI_QUANTIZATION_RANGE_FULL,
+					   intel_sdvo->rgb_quant_range_selectable);
 
 	len = hdmi_infoframe_pack(&frame, sdvo_data, sizeof(sdvo_data));
 	if (len < 0)
-- 
2.18.1

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

Re: [PATCH 2/4] drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well

[Dhinakaran Pandiyan]

On Tue, 2018-11-20 at 18:13 +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Fill out the AVI infoframe quantization range bits using
> drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI encoder as well.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sdvo.c | 19 ++++++++++---------
>  1 file changed, 10 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> b/drivers/gpu/drm/i915/intel_sdvo.c
> index 1277d31adb54..9c16e273fb8d 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -984,6 +984,8 @@ static bool intel_sdvo_set_avi_infoframe(struct
> intel_sdvo *intel_sdvo,
>  					 const struct intel_crtc_state
> *pipe_config,
>  					 const struct
> drm_connector_state *conn_state)
>  {
> +	const struct drm_display_mode *adjusted_mode =
> +		&pipe_config->base.adjusted_mode;
>  	uint8_t sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
>  	union hdmi_infoframe frame;
>  	int ret;
> @@ -991,20 +993,19 @@ static bool intel_sdvo_set_avi_infoframe(struct
> intel_sdvo *intel_sdvo,
>  
>  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
>  						       conn_state-
> >connector,
> -						       &pipe_config-
> >base.adjusted_mode);
> +						       adjusted_mode);
>  	if (ret < 0) {
>  		DRM_ERROR("couldn't fill AVI infoframe\n");
>  		return false;
>  	}
>  
> -	if (intel_sdvo->rgb_quant_range_selectable) {
> -		if (pipe_config->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,
> +					   conn_state->connector,
> +					   adjusted_mode,
> +					   pipe_config-
> >limited_color_range ?
> +					   rgb_quant_range_selectableTE
> D :
> +					   HDMI_QUANTIZATION_RANGE_FULL
> ,
> +					   intel_sdvo-
> >rgb_quant_range_selectable);

Seems like avi.quantization_range can now get set to _LIMITED or _FULL
even when ->rgb_quant_range_selectable == false, i.e., it is not
_DEFAULT anymore. Is that change in behavior intended?


>  
>  	len = hdmi_infoframe_pack(&frame, sdvo_data,
> sizeof(sdvo_data));
>  	if (len < 0)

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

Re: [PATCH 2/4] drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well

[Ville Syrjälä]

On Wed, Dec 12, 2018 at 04:32:02PM -0800, Dhinakaran Pandiyan wrote:
> On Tue, 2018-11-20 at 18:13 +0200, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Fill out the AVI infoframe quantization range bits using
> > drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI encoder as well.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_sdvo.c | 19 ++++++++++---------
> >  1 file changed, 10 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> > b/drivers/gpu/drm/i915/intel_sdvo.c
> > index 1277d31adb54..9c16e273fb8d 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -984,6 +984,8 @@ static bool intel_sdvo_set_avi_infoframe(struct
> > intel_sdvo *intel_sdvo,
> >  					 const struct intel_crtc_state
> > *pipe_config,
> >  					 const struct
> > drm_connector_state *conn_state)
> >  {
> > +	const struct drm_display_mode *adjusted_mode =
> > +		&pipe_config->base.adjusted_mode;
> >  	uint8_t sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
> >  	union hdmi_infoframe frame;
> >  	int ret;
> > @@ -991,20 +993,19 @@ static bool intel_sdvo_set_avi_infoframe(struct
> > intel_sdvo *intel_sdvo,
> >  
> >  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> >  						       conn_state-
> > >connector,
> > -						       &pipe_config-
> > >base.adjusted_mode);
> > +						       adjusted_mode);
> >  	if (ret < 0) {
> >  		DRM_ERROR("couldn't fill AVI infoframe\n");
> >  		return false;
> >  	}
> >  
> > -	if (intel_sdvo->rgb_quant_range_selectable) {
> > -		if (pipe_config->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,
> > +					   conn_state->connector,
> > +					   adjusted_mode,
> > +					   pipe_config-
> > >limited_color_range ?
> > +					   rgb_quant_range_selectableTE
> > D :
> > +					   HDMI_QUANTIZATION_RANGE_FULL
> > ,
> > +					   intel_sdvo-
> > >rgb_quant_range_selectable);
> 
> Seems like avi.quantization_range can now get set to _LIMITED or _FULL
> even when ->rgb_quant_range_selectable == false, i.e., it is not
> _DEFAULT anymore. Is that change in behavior intended?

->quant_range_selectable will be passed to
drm_hdmi_avi_infoframe_quant_range() which will do the right thing with
it.

That said, there is a slight behavioural change in that it will set
the Q bit even with QS==1 iff the quantization range matches the
default quantization range for the mode. I noted this in the radeon
patch but forgot to mention it here.

> 
> 
> >  
> >  	len = hdmi_infoframe_pack(&frame, sdvo_data,
> > sizeof(sdvo_data));
> >  	if (len < 0)

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

Re: [PATCH 2/4] drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well

[Dhinakaran Pandiyan]

On Thu, 2018-12-13 at 07:18 +0200, Ville Syrjälä wrote:
> On Wed, Dec 12, 2018 at 04:32:02PM -0800, Dhinakaran Pandiyan wrote:
> > On Tue, 2018-11-20 at 18:13 +0200, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Fill out the AVI infoframe quantization range bits using
> > > drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI encoder as
> > > well.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_sdvo.c | 19 ++++++++++---------
> > >  1 file changed, 10 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> > > b/drivers/gpu/drm/i915/intel_sdvo.c
> > > index 1277d31adb54..9c16e273fb8d 100644
> > > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > > @@ -984,6 +984,8 @@ static bool
> > > intel_sdvo_set_avi_infoframe(struct
> > > intel_sdvo *intel_sdvo,
> > >  					 const struct intel_crtc_state
> > > *pipe_config,
> > >  					 const struct
> > > drm_connector_state *conn_state)
> > >  {
> > > +	const struct drm_display_mode *adjusted_mode =
> > > +		&pipe_config->base.adjusted_mode;
> > >  	uint8_t sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
> > >  	union hdmi_infoframe frame;
> > >  	int ret;
> > > @@ -991,20 +993,19 @@ static bool
> > > intel_sdvo_set_avi_infoframe(struct
> > > intel_sdvo *intel_sdvo,
> > >  
> > >  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> > >  						       conn_state-
> > > > connector,
> > > 
> > > -						       &pipe_config-
> > > > base.adjusted_mode);
> > > 
> > > +						       adjusted_mode);
> > >  	if (ret < 0) {
> > >  		DRM_ERROR("couldn't fill AVI infoframe\n");
> > >  		return false;
> > >  	}
> > >  
> > > -	if (intel_sdvo->rgb_quant_range_selectable) {
> > > -		if (pipe_config->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,
> > > +					   conn_state->connector,
> > > +					   adjusted_mode,
> > > +					   pipe_config-
> > > > limited_color_range ?
> > > 
> > > +					   rgb_quant_range_selectableTE
> > > D :
> > > +					   HDMI_QUANTIZATION_RANGE_FULL
> > > ,
> > > +					   intel_sdvo-
> > > > rgb_quant_range_selectable);
> > 
> > Seems like avi.quantization_range can now get set to _LIMITED or
> > _FULL
> > even when ->rgb_quant_range_selectable == false, i.e., it is not
> > _DEFAULT anymore. Is that change in behavior intended?
> 
> ->quant_range_selectable will be passed to
> drm_hdmi_avi_infoframe_quant_range() which will do the right thing
> with
> it.
> 
> That said, there is a slight behavioural change in that it will set
Okay, I was indeed referring to this case.

> the Q bit even with QS==1 iff the quantization range matches the
> default quantization range for the mode. I noted this in the radeon
> patch but forgot to mention it here.
I'll let someone else with knowledge of HDMI to review this behavioral 
change. I'm trying to get hold of the HDMI spec now and will review if
this hasn't been looked at by then.


> 
> > 
> > 
> > >  
> > >  	len = hdmi_infoframe_pack(&frame, sdvo_data,
> > > sizeof(sdvo_data));
> > >  	if (len < 0)
> 
> 

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

Re: [Intel-gfx] [PATCH 2/4] drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well

[Dhinakaran Pandiyan]

On Thu, 2018-12-13 at 15:09 -0800, Dhinakaran Pandiyan wrote:
> On Thu, 2018-12-13 at 07:18 +0200, Ville Syrjälä wrote:
> > On Wed, Dec 12, 2018 at 04:32:02PM -0800, Dhinakaran Pandiyan
> > wrote:
> > > On Tue, 2018-11-20 at 18:13 +0200, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > Fill out the AVI infoframe quantization range bits using
> > > > drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI encoder as
> > > > well.
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_sdvo.c | 19 ++++++++++---------
> > > >  1 file changed, 10 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> > > > b/drivers/gpu/drm/i915/intel_sdvo.c
> > > > index 1277d31adb54..9c16e273fb8d 100644
> > > > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > > > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > > > @@ -984,6 +984,8 @@ static bool
> > > > intel_sdvo_set_avi_infoframe(struct
> > > > intel_sdvo *intel_sdvo,
> > > >  					 const struct
> > > > intel_crtc_state
> > > > *pipe_config,
> > > >  					 const struct
> > > > drm_connector_state *conn_state)
> > > >  {
> > > > +	const struct drm_display_mode *adjusted_mode =
> > > > +		&pipe_config->base.adjusted_mode;
> > > >  	uint8_t sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
> > > >  	union hdmi_infoframe frame;
> > > >  	int ret;
> > > > @@ -991,20 +993,19 @@ static bool
> > > > intel_sdvo_set_avi_infoframe(struct
> > > > intel_sdvo *intel_sdvo,
> > > >  
> > > >  	ret =
> > > > drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> > > >  						       conn_sta
> > > > te-
> > > > > connector,
> > > > 
> > > > -						       &pipe_co
> > > > nfig-
> > > > > base.adjusted_mode);
> > > > 
> > > > +						       adjusted
> > > > _mode);
> > > >  	if (ret < 0) {
> > > >  		DRM_ERROR("couldn't fill AVI infoframe\n");
> > > >  		return false;
> > > >  	}
> > > >  
> > > > -	if (intel_sdvo->rgb_quant_range_selectable) {
> > > > -		if (pipe_config->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,
> > > > +					   conn_state-
> > > > >connector,
> > > > +					   adjusted_mode,
> > > > +					   pipe_config-
> > > > > limited_color_range ?
> > > > 
> > > > +					   rgb_quant_range_sele
> > > > ctableTE
> > > > D :
> > > > +					   HDMI_QUANTIZATION_RA
> > > > NGE_FULL
> > > > ,
> > > > +					   intel_sdvo-
> > > > > rgb_quant_range_selectable);
> > > 
> > > Seems like avi.quantization_range can now get set to _LIMITED or
> > > _FULL
> > > even when ->rgb_quant_range_selectable == false, i.e., it is not
> > > _DEFAULT anymore. Is that change in behavior intended?
> > 
> > ->quant_range_selectable will be passed to
> > drm_hdmi_avi_infoframe_quant_range() which will do the right thing
> > with
> > it.
> > 
> > That said, there is a slight behavioural change in that it will set
> 
> Okay, I was indeed referring to this case.
> 
> > the Q bit even with QS==1 iff the quantization range matches the
> > default quantization range for the mode. I noted this in the radeon
> > patch but forgot to mention it here.
> 
> I'll let someone else with knowledge of HDMI to review this
> behavioral 
> change. I'm trying to get hold of the HDMI spec now and will review
> if
> this hasn't been looked at by then.
> 
Looks alright now that I went through the specs. With commit message
updated to make note of the Q value changes

Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> > 
> > > 
> > > 
> > > >  
> > > >  	len = hdmi_infoframe_pack(&frame, sdvo_data,
> > > > sizeof(sdvo_data));
> > > >  	if (len < 0)
> > 
> > 

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

[PATCH 4/4] drm/edid: Add display_info.rgb_quant_range_selectable

[Ville Syrjala]

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

Move the CEA-861 QS bit handling entirely into the edid code. No
need to bother the drivers with this.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Cc: Eric Anholt <eric@anholt.net> (supporter:DRM DRIVERS FOR VC4)
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_edid.c            | 70 ++++++++++++---------------
 drivers/gpu/drm/i915/intel_drv.h      |  1 -
 drivers/gpu/drm/i915/intel_hdmi.c     |  8 +--
 drivers/gpu/drm/i915/intel_lspcon.c   |  3 +-
 drivers/gpu/drm/i915/intel_sdvo.c     |  7 +--
 drivers/gpu/drm/radeon/radeon_audio.c |  3 +-
 drivers/gpu/drm/vc4/vc4_hdmi.c        |  9 +---
 include/drm/drm_connector.h           |  6 +++
 include/drm/drm_edid.h                |  4 +-
 9 files changed, 43 insertions(+), 68 deletions(-)

diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index 501ac05ba7da..cbee2f745548 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -3641,6 +3641,20 @@ static bool cea_db_is_hdmi_forum_vsdb(const u8 *db)
 	return oui == HDMI_FORUM_IEEE_OUI;
 }
 
+static bool cea_db_is_vcdb(const u8 *db)
+{
+	if (cea_db_tag(db) != USE_EXTENDED_TAG)
+		return false;
+
+	if (cea_db_payload_len(db) != 2)
+		return false;
+
+	if (cea_db_extended_tag(db) != EXT_VIDEO_CAPABILITY_BLOCK)
+		return false;
+
+	return true;
+}
+
 static bool cea_db_is_y420cmdb(const u8 *db)
 {
 	if (cea_db_tag(db) != USE_EXTENDED_TAG)
@@ -4223,41 +4237,6 @@ bool drm_detect_monitor_audio(struct edid *edid)
 }
 EXPORT_SYMBOL(drm_detect_monitor_audio);
 
-/**
- * drm_rgb_quant_range_selectable - is RGB quantization range selectable?
- * @edid: EDID block to scan
- *
- * Check whether the monitor reports the RGB quantization range selection
- * as supported. The AVI infoframe can then be used to inform the monitor
- * which quantization range (full or limited) is used.
- *
- * Return: True if the RGB quantization range is selectable, false otherwise.
- */
-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]) == USE_EXTENDED_TAG &&
-		    cea_db_payload_len(&edid_ext[i]) == 2 &&
-		    cea_db_extended_tag(&edid_ext[i]) ==
-			EXT_VIDEO_CAPABILITY_BLOCK) {
-			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_default_rgb_quant_range - default RGB quantization range
@@ -4278,6 +4257,16 @@ drm_default_rgb_quant_range(const struct drm_display_mode *mode)
 }
 EXPORT_SYMBOL(drm_default_rgb_quant_range);
 
+static void drm_parse_vcdb(struct drm_connector *connector, const u8 *db)
+{
+	struct drm_display_info *info = &connector->display_info;
+
+	DRM_DEBUG_KMS("CEA VCDB 0x%02x\n", db[2]);
+
+	if (db[2] & EDID_CEA_VCDB_QS)
+		info->rgb_quant_range_selectable = true;
+}
+
 static void drm_parse_ycbcr420_deep_color_info(struct drm_connector *connector,
 					       const u8 *db)
 {
@@ -4452,6 +4441,8 @@ static void drm_parse_cea_ext(struct drm_connector *connector,
 			drm_parse_hdmi_forum_vsdb(connector, db);
 		if (cea_db_is_y420cmdb(db))
 			drm_parse_y420cmdb_bitmap(connector, db);
+		if (cea_db_is_vcdb(db))
+			drm_parse_vcdb(connector, db);
 	}
 }
 
@@ -4472,6 +4463,7 @@ drm_reset_display_info(struct drm_connector *connector)
 	info->max_tmds_clock = 0;
 	info->dvi_dual = false;
 	info->has_hdmi_infoframe = false;
+	info->rgb_quant_range_selectable = false;
 	memset(&info->hdmi, 0, sizeof(info->hdmi));
 
 	info->non_desktop = 0;
@@ -4939,15 +4931,15 @@ EXPORT_SYMBOL(drm_hdmi_avi_infoframe_from_display_mode);
  * @connector: the connector
  * @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,
 				   struct drm_connector *connector,
 				   const struct drm_display_mode *mode,
-				   enum hdmi_quantization_range rgb_quant_range,
-				   bool rgb_quant_range_selectable)
+				   enum hdmi_quantization_range rgb_quant_range)
 {
+	const struct drm_display_info *info = &connector->display_info;
+
 	/*
 	 * CEA-861:
 	 * "A Source shall not send a non-zero Q value that does not correspond
@@ -4958,7 +4950,7 @@ drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
 	 * 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 (info->rgb_quant_range_selectable ||
 	    rgb_quant_range == drm_default_rgb_quant_range(mode))
 		frame->quantization_range = rgb_quant_range;
 	else
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index f575ba2a59da..257e05345522 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1056,7 +1056,6 @@ struct intel_hdmi {
 	} dp_dual_mode;
 	bool has_hdmi_sink;
 	bool has_audio;
-	bool rgb_quant_range_selectable;
 	struct intel_connector *attached_connector;
 	struct cec_notifier *cec_notifier;
 };
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 8a260bd6954c..4d264e577e3b 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -462,7 +462,6 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder,
 					 const struct intel_crtc_state *crtc_state,
 					 const struct drm_connector_state *conn_state)
 {
-	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	const struct drm_display_mode *adjusted_mode =
 		&crtc_state->base.adjusted_mode;
 	union hdmi_infoframe frame;
@@ -488,8 +487,7 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder,
 					   adjusted_mode,
 					   crtc_state->limited_color_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
-					   HDMI_QUANTIZATION_RANGE_FULL,
-					   intel_hdmi->rgb_quant_range_selectable);
+					   HDMI_QUANTIZATION_RANGE_FULL);
 
 	drm_hdmi_avi_infoframe_content_type(&frame.avi,
 					    conn_state);
@@ -1816,7 +1814,6 @@ intel_hdmi_unset_edid(struct drm_connector *connector)
 
 	intel_hdmi->has_hdmi_sink = false;
 	intel_hdmi->has_audio = false;
-	intel_hdmi->rgb_quant_range_selectable = false;
 
 	intel_hdmi->dp_dual_mode.type = DRM_DP_DUAL_MODE_NONE;
 	intel_hdmi->dp_dual_mode.max_tmds_clock = 0;
@@ -1900,9 +1897,6 @@ intel_hdmi_set_edid(struct drm_connector *connector)
 
 	to_intel_connector(connector)->detect_edid = edid;
 	if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
-		intel_hdmi->rgb_quant_range_selectable =
-			drm_rgb_quant_range_selectable(edid);
-
 		intel_hdmi->has_audio = drm_detect_monitor_audio(edid);
 		intel_hdmi->has_hdmi_sink = drm_detect_hdmi_monitor(edid);
 
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index c18f14d41f1d..7d15be5932e0 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -492,8 +492,7 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
 					   adjusted_mode,
 					   crtc_state->limited_color_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
-					   HDMI_QUANTIZATION_RANGE_FULL,
-					   false);
+					   HDMI_QUANTIZATION_RANGE_FULL);
 
 	ret = hdmi_infoframe_pack(&frame, buf, sizeof(buf));
 	if (ret < 0) {
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 9c16e273fb8d..669fa9faad70 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -103,7 +103,6 @@ struct intel_sdvo {
 
 	bool has_hdmi_monitor;
 	bool has_hdmi_audio;
-	bool rgb_quant_range_selectable;
 
 	/* DDC bus used by this SDVO encoder */
 	uint8_t ddc_bus;
@@ -1004,8 +1003,7 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
 					   adjusted_mode,
 					   pipe_config->limited_color_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
-					   HDMI_QUANTIZATION_RANGE_FULL,
-					   intel_sdvo->rgb_quant_range_selectable);
+					   HDMI_QUANTIZATION_RANGE_FULL);
 
 	len = hdmi_infoframe_pack(&frame, sdvo_data, sizeof(sdvo_data));
 	if (len < 0)
@@ -1805,8 +1803,6 @@ intel_sdvo_tmds_sink_detect(struct drm_connector *connector)
 			if (intel_sdvo_connector->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;
@@ -1855,7 +1851,6 @@ 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;
diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c
index 708765bf9e66..96f71114237a 100644
--- a/drivers/gpu/drm/radeon/radeon_audio.c
+++ b/drivers/gpu/drm/radeon/radeon_audio.c
@@ -526,8 +526,7 @@ static int radeon_audio_set_avi_packet(struct drm_encoder *encoder,
 		drm_hdmi_avi_infoframe_quant_range(&frame, connector, mode,
 						   radeon_encoder->output_csc == RADEON_OUTPUT_CSC_TVRGB ?
 						   HDMI_QUANTIZATION_RANGE_LIMITED :
-						   HDMI_QUANTIZATION_RANGE_FULL,
-						   drm_rgb_quant_range_selectable(radeon_connector_edid(connector)));
+						   HDMI_QUANTIZATION_RANGE_FULL);
 	}
 
 	err = hdmi_avi_infoframe_pack(&frame, buffer, sizeof(buffer));
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index cb3dca7426c8..6d15cd06896d 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -109,7 +109,6 @@ struct vc4_hdmi_encoder {
 	struct vc4_encoder base;
 	bool hdmi_monitor;
 	bool limited_rgb_range;
-	bool rgb_range_selectable;
 };
 
 static inline struct vc4_hdmi_encoder *
@@ -280,11 +279,6 @@ static int vc4_hdmi_connector_get_modes(struct drm_connector *connector)
 
 	vc4_encoder->hdmi_monitor = drm_detect_hdmi_monitor(edid);
 
-	if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
-		vc4_encoder->rgb_range_selectable =
-			drm_rgb_quant_range_selectable(edid);
-	}
-
 	drm_connector_update_edid_property(connector, edid);
 	ret = drm_add_edid_modes(connector, edid);
 	kfree(edid);
@@ -426,8 +420,7 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
 					   hdmi->connector, mode,
 					   vc4_encoder->limited_rgb_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
-					   HDMI_QUANTIZATION_RANGE_FULL,
-					   vc4_encoder->rgb_range_selectable);
+					   HDMI_QUANTIZATION_RANGE_FULL);
 
 	vc4_hdmi_write_infoframe(encoder, &frame);
 }
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index af0a761f52f0..939c8acfe351 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -365,6 +365,12 @@ struct drm_display_info {
 	 */
 	bool has_hdmi_infoframe;
 
+	/**
+	 * @rgb_quant_range_selectable: Does the sink support selecting
+	 * the RGB quantization range?
+	 */
+	bool rgb_quant_range_selectable;
+
 	/**
 	 * @edid_hdmi_dc_modes: Mask of supported hdmi deep color modes. Even
 	 * more stuff redundant with @bus_formats.
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 9db6f130df65..8dc1a081fb36 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -362,8 +362,7 @@ void
 drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
 				   struct drm_connector *connector,
 				   const struct drm_display_mode *mode,
-				   enum hdmi_quantization_range rgb_quant_range,
-				   bool rgb_quant_range_selectable);
+				   enum hdmi_quantization_range rgb_quant_range);
 
 /**
  * drm_eld_mnl - Get ELD monitor name length in bytes.
@@ -471,7 +470,6 @@ u8 drm_match_cea_mode(const struct drm_display_mode *to_match);
 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,
-- 
2.18.1

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

Re: [PATCH 4/4] drm/edid: Add display_info.rgb_quant_range_selectable

[Eric Anholt]

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

Ville Syrjala <ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> writes:

> From: Ville Syrjälä <ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
>
> Move the CEA-861 QS bit handling entirely into the edid code. No
> need to bother the drivers with this.
>
> Cc: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>
> Cc: "Christian König" <christian.koenig-5C7GfCeVMHo@public.gmane.org>
> Cc: "David (ChunMing) Zhou" <David1.Zhou-5C7GfCeVMHo@public.gmane.org>
> Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> Cc: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> (supporter:DRM DRIVERS FOR VC4)
> Signed-off-by: Ville Syrjälä <ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>

For vc4,
Acked-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>

Looks like a nice cleanup!

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

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

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

Re: [PATCH 4/4] drm/edid: Add display_info.rgb_quant_range_selectable

[Alex Deucher]

On Wed, Nov 28, 2018 at 12:19 PM Eric Anholt <eric@anholt.net> wrote:
>
> Ville Syrjala <ville.syrjala@linux.intel.com> writes:
>
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Move the CEA-861 QS bit handling entirely into the edid code. No
> > need to bother the drivers with this.
> >
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> > Cc: amd-gfx@lists.freedesktop.org
> > Cc: Eric Anholt <eric@anholt.net> (supporter:DRM DRIVERS FOR VC4)
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> For vc4,
> Acked-by: Eric Anholt <eric@anholt.net>
>
> Looks like a nice cleanup!

for radeon:
Acked-by: Alex Deucher <alexander.deucher@amd.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/4] drm/edid: Pass connector to AVI inforframe functions

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/edid: Pass connector to AVI inforframe functions
URL   : https://patchwork.freedesktop.org/series/52767/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
3b5027b538c5 drm/edid: Pass connector to AVI inforframe functions
25296a851990 drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well
9be3dca0d438 drm/radeon: Use drm_hdmi_avi_infoframe_quant_range()
-:41: WARNING:LONG_LINE: line over 100 characters
#41: FILE: drivers/gpu/drm/radeon/radeon_audio.c:527:
+						   radeon_encoder->output_csc == RADEON_OUTPUT_CSC_TVRGB ?

-:44: WARNING:LONG_LINE: line over 100 characters
#44: FILE: drivers/gpu/drm/radeon/radeon_audio.c:530:
+						   drm_rgb_quant_range_selectable(radeon_connector_edid(connector)));

total: 0 errors, 2 warnings, 0 checks, 19 lines checked
ceb81320d146 drm/edid: Add display_info.rgb_quant_range_selectable
-:313: CHECK:BOOL_MEMBER: Avoid using bool structure members because of possible alignment issues - see: https://lkml.org/lkml/2017/11/21/384
#313: FILE: include/drm/drm_connector.h:372:
+	bool rgb_quant_range_selectable;

total: 0 errors, 0 warnings, 1 checks, 261 lines checked

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

✓ Fi.CI.BAT: success for series starting with [1/4] drm/edid: Pass connector to AVI inforframe functions

[Patchwork]

== Series Details ==

Series: series starting with [1/4] drm/edid: Pass connector to AVI inforframe functions
URL   : https://patchwork.freedesktop.org/series/52767/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_5174 -> Patchwork_10864 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/52767/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10864 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_ctx_switch@basic-default:
      fi-icl-u2:          PASS -> DMESG-WARN (fdo#107724)

    igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
      fi-byt-clapper:     PASS -> FAIL (fdo#103191, fdo#107362)

    
    ==== Possible fixes ====

    igt@gem_exec_suspend@basic-s3:
      fi-icl-u2:          DMESG-WARN (fdo#107724) -> PASS

    igt@i915_selftest@live_execlists:
      fi-apl-guc:         DMESG-WARN (fdo#108622) -> PASS

    igt@kms_flip@basic-flip-vs-dpms:
      fi-skl-6700hq:      DMESG-WARN (fdo#105998) -> PASS

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107724 https://bugs.freedesktop.org/show_bug.cgi?id=107724
  fdo#108622 https://bugs.freedesktop.org/show_bug.cgi?id=108622


== Participating hosts (52 -> 45) ==

  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-u3 fi-icl-y 


== Build changes ==

    * Linux: CI_DRM_5174 -> Patchwork_10864

  CI_DRM_5174: 0bfa7192170c039a271ebc27222b4b91516e73f6 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4722: fdcdfa1e220c5070072d5dac9523cd105e7406c2 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10864: ceb81320d146bab4e835083c29ad14f3444cd080 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

ceb81320d146 drm/edid: Add display_info.rgb_quant_range_selectable
9be3dca0d438 drm/radeon: Use drm_hdmi_avi_infoframe_quant_range()
25296a851990 drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well
3b5027b538c5 drm/edid: Pass connector to AVI inforframe functions

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10864/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/4] drm/radeon: Use drm_hdmi_avi_infoframe_quant_range()

[Ville Syrjala]

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

Fill out the AVI infoframe quantization range bits using
drm_hdmi_avi_infoframe_quant_range() instead of hand rolling it.

This changes the behaviour slightly as
drm_hdmi_avi_infoframe_quant_range() will set a non-zero Q bit
even when QS==0 iff the Q bit matched the default quantization
range for the given mode. This matches the recommendation in
HDMI 2.0 and is allowed even before that.

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/radeon/radeon_audio.c | 13 +++++--------
 1 file changed, 5 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c
index 5a7d48339b32..708765bf9e66 100644
--- a/drivers/gpu/drm/radeon/radeon_audio.c
+++ b/drivers/gpu/drm/radeon/radeon_audio.c
@@ -523,14 +523,11 @@ static int radeon_audio_set_avi_packet(struct drm_encoder *encoder,
 	}
 
 	if (radeon_encoder->output_csc != RADEON_OUTPUT_CSC_BYPASS) {
-		if (drm_rgb_quant_range_selectable(radeon_connector_edid(connector))) {
-			if (radeon_encoder->output_csc == RADEON_OUTPUT_CSC_TVRGB)
-				frame.quantization_range = HDMI_QUANTIZATION_RANGE_LIMITED;
-			else
-				frame.quantization_range = HDMI_QUANTIZATION_RANGE_FULL;
-		} else {
-			frame.quantization_range = HDMI_QUANTIZATION_RANGE_DEFAULT;
-		}
+		drm_hdmi_avi_infoframe_quant_range(&frame, connector, mode,
+						   radeon_encoder->output_csc == RADEON_OUTPUT_CSC_TVRGB ?
+						   HDMI_QUANTIZATION_RANGE_LIMITED :
+						   HDMI_QUANTIZATION_RANGE_FULL,
+						   drm_rgb_quant_range_selectable(radeon_connector_edid(connector)));
 	}
 
 	err = hdmi_avi_infoframe_pack(&frame, buffer, sizeof(buffer));
-- 
2.18.1

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

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Thierry Reding]

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

On Tue, Nov 20, 2018 at 06:13:42PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> 
> Make life easier for drivers by simply passing the connector
> to drm_hdmi_avi_infoframe_from_display_mode() and
> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
> need to worry about is_hdmi2_sink mess.
> 
> Cc: Alex Deucher <alexander.deucher-5C7GfCeVMHo@public.gmane.org>
> Cc: "Christian König" <christian.koenig-5C7GfCeVMHo@public.gmane.org>
> Cc: "David (ChunMing) Zhou" <David1.Zhou-5C7GfCeVMHo@public.gmane.org>
> Cc: Archit Taneja <architt-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
> Cc: Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Laurent Pinchart <Laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw@public.gmane.org>
> Cc: Inki Dae <inki.dae-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Joonyoung Shim <jy0922.shim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Seung-Woo Kim <sw0312.kim-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Kyungmin Park <kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Cc: Russell King <linux-I+IVW8TIWO2tmTQ+vhA3Yw@public.gmane.org>
> Cc: CK Hu <ck.hu-NuS5LvNUpcJWk0Htik3J/w@public.gmane.org>
> Cc: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Cc: Rob Clark <robdclark-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Ben Skeggs <bskeggs-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
> Cc: Tomi Valkeinen <tomi.valkeinen-l0cyMroinI0@public.gmane.org>
> Cc: Sandy Huang <hjc-TNX95d0MmH7DzftRWevZcw@public.gmane.org>
> Cc: "Heiko Stübner" <heiko-4mtYJXux2i+zQB+pC5nmwQ@public.gmane.org>
> Cc: Benjamin Gaignard <benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> Cc: Vincent Abriou <vincent.abriou-qxv4g6HH51o@public.gmane.org>
> Cc: Thierry Reding <thierry.reding-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Cc: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> Cc: Shawn Guo <shawnguo-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Ilia Mirkin <imirkin-FrUbXkNCsVf2fBVCVOL8/A@public.gmane.org>
> Cc: amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> Cc: linux-arm-msm-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: freedreno-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> Cc: nouveau-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org
> Cc: linux-tegra-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Signed-off-by: Ville Syrjälä <ville.syrjala-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
> ---
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  2 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  2 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  3 ++-
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  2 +-
>  drivers/gpu/drm/bridge/analogix-anx78xx.c |  5 ++--
>  drivers/gpu/drm/bridge/sii902x.c          |  3 ++-
>  drivers/gpu/drm/bridge/sil-sii8620.c      |  3 +--
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  3 ++-
>  drivers/gpu/drm/drm_edid.c                | 33 ++++++++++++++---------
>  drivers/gpu/drm/exynos/exynos_hdmi.c      |  3 ++-
>  drivers/gpu/drm/i2c/tda998x_drv.c         |  3 ++-
>  drivers/gpu/drm/i915/intel_hdmi.c         | 14 +++++-----
>  drivers/gpu/drm/i915/intel_lspcon.c       | 15 ++++++-----
>  drivers/gpu/drm/i915/intel_sdvo.c         | 10 ++++---
>  drivers/gpu/drm/mediatek/mtk_hdmi.c       |  3 ++-
>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |  3 ++-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c   |  7 +++--
>  drivers/gpu/drm/omapdrm/omap_encoder.c    |  5 ++--
>  drivers/gpu/drm/radeon/radeon_audio.c     |  2 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c      |  4 ++-
>  drivers/gpu/drm/sti/sti_hdmi.c            |  3 ++-
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |  3 ++-
>  drivers/gpu/drm/tegra/hdmi.c              |  3 ++-
>  drivers/gpu/drm/tegra/sor.c               |  3 ++-
>  drivers/gpu/drm/vc4/vc4_hdmi.c            | 11 +++++---
>  drivers/gpu/drm/zte/zx_hdmi.c             |  4 ++-
>  include/drm/drm_edid.h                    |  8 +++---
>  27 files changed, 94 insertions(+), 66 deletions(-)

That's actually a lot nicer:

Acked-by: Thierry Reding <treding-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>

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

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

_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Jani Nikula]

On Tue, 20 Nov 2018, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Make life easier for drivers by simply passing the connector
> to drm_hdmi_avi_infoframe_from_display_mode() and
> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
> need to worry about is_hdmi2_sink mess.

Overall looks about right and nice,

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

But please do take that with a grain of salt for everything outside of
i915 and drm core.

Please also find a few comments inline below.

> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: CK Hu <ck.hu@mediatek.com>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Sandy Huang <hjc@rock-chips.com>
> Cc: "Heiko Stübner" <heiko@sntech.de>
> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Cc: Vincent Abriou <vincent.abriou@st.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Cc: nouveau@lists.freedesktop.org
> Cc: linux-tegra@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  2 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  2 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  3 ++-
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  2 +-
>  drivers/gpu/drm/bridge/analogix-anx78xx.c |  5 ++--
>  drivers/gpu/drm/bridge/sii902x.c          |  3 ++-
>  drivers/gpu/drm/bridge/sil-sii8620.c      |  3 +--
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  3 ++-
>  drivers/gpu/drm/drm_edid.c                | 33 ++++++++++++++---------
>  drivers/gpu/drm/exynos/exynos_hdmi.c      |  3 ++-
>  drivers/gpu/drm/i2c/tda998x_drv.c         |  3 ++-
>  drivers/gpu/drm/i915/intel_hdmi.c         | 14 +++++-----
>  drivers/gpu/drm/i915/intel_lspcon.c       | 15 ++++++-----
>  drivers/gpu/drm/i915/intel_sdvo.c         | 10 ++++---
>  drivers/gpu/drm/mediatek/mtk_hdmi.c       |  3 ++-
>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |  3 ++-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c   |  7 +++--
>  drivers/gpu/drm/omapdrm/omap_encoder.c    |  5 ++--
>  drivers/gpu/drm/radeon/radeon_audio.c     |  2 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c      |  4 ++-
>  drivers/gpu/drm/sti/sti_hdmi.c            |  3 ++-
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |  3 ++-
>  drivers/gpu/drm/tegra/hdmi.c              |  3 ++-
>  drivers/gpu/drm/tegra/sor.c               |  3 ++-
>  drivers/gpu/drm/vc4/vc4_hdmi.c            | 11 +++++---
>  drivers/gpu/drm/zte/zx_hdmi.c             |  4 ++-
>  include/drm/drm_edid.h                    |  8 +++---
>  27 files changed, 94 insertions(+), 66 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> index 4cfecdce29a3..1f0426d2fc2a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> @@ -1682,7 +1682,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder *encoder,
>  	dce_v10_0_audio_write_sad_regs(encoder);
>  	dce_v10_0_audio_write_latency_fields(encoder, mode);
>  
> -	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> +	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
>  	if (err < 0) {
>  		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
>  		return;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> index 7c868916d90f..2280b971d758 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> @@ -1724,7 +1724,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder *encoder,
>  	dce_v11_0_audio_write_sad_regs(encoder);
>  	dce_v11_0_audio_write_latency_fields(encoder, mode);
>  
> -	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> +	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
>  	if (err < 0) {
>  		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
>  		return;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> index 17eaaba36017..db443ec53d3a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> @@ -1423,6 +1423,7 @@ static void dce_v6_0_audio_set_avi_infoframe(struct drm_encoder *encoder,
>  	struct amdgpu_device *adev = dev->dev_private;
>  	struct amdgpu_encoder *amdgpu_encoder = to_amdgpu_encoder(encoder);
>  	struct amdgpu_encoder_atom_dig *dig = amdgpu_encoder->enc_priv;
> +	struct drm_connector *connector = amdgpu_get_connector_for_encoder(encoder);
>  	struct hdmi_avi_infoframe frame;
>  	u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE];
>  	uint8_t *payload = buffer + 3;
> @@ -1430,7 +1431,7 @@ static void dce_v6_0_audio_set_avi_infoframe(struct drm_encoder *encoder,
>  	ssize_t err;
>  	u32 tmp;
>  
> -	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> +	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
>  	if (err < 0) {
>  		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
>  		return;
> diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> index 8c0576978d36..13da915991dd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> @@ -1616,7 +1616,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
>  	dce_v8_0_audio_write_sad_regs(encoder);
>  	dce_v8_0_audio_write_latency_fields(encoder, mode);
>  
> -	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> +	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
>  	if (err < 0) {
>  		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
>  		return;
> diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
> index f8433c93f463..e11309e9bc4f 100644
> --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
> +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
> @@ -1094,8 +1094,9 @@ static void anx78xx_bridge_mode_set(struct drm_bridge *bridge,
>  
>  	mutex_lock(&anx78xx->lock);
>  
> -	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode,
> -						       false);
> +	err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
> +						       &anx78xx->connector,
> +						       adjusted_mode);
>  	if (err) {
>  		DRM_ERROR("Failed to setup AVI infoframe: %d\n", err);
>  		goto unlock;
> diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> index bfa902013aa4..a9b4f45ae87c 100644
> --- a/drivers/gpu/drm/bridge/sii902x.c
> +++ b/drivers/gpu/drm/bridge/sii902x.c
> @@ -258,7 +258,8 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
>  	if (ret)
>  		return;
>  
> -	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj, false);
> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame,
> +						       &sii902x->connector, adj);
>  	if (ret < 0) {
>  		DRM_ERROR("couldn't fill AVI infoframe\n");
>  		return;
> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
> index a6e8f4591e63..0cc293a6ac24 100644
> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> @@ -1104,8 +1104,7 @@ static void sii8620_set_infoframes(struct sii8620 *ctx,
>  	int ret;
>  
>  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
> -						       mode,
> -						       true);
> +						       NULL, mode);
>  	if (ctx->use_packed_pixel)
>  		frm.avi.colorspace = HDMI_COLORSPACE_YUV422;
>  
> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 64c3cf027518..88b720b63126 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> @@ -1344,7 +1344,8 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
>  	u8 val;
>  
>  	/* Initialise info frame from DRM mode */
> -	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> +	drm_hdmi_avi_infoframe_from_display_mode(&frame,
> +						 &hdmi->connector, mode);
>  
>  	if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
>  		frame.colorspace = HDMI_COLORSPACE_YUV444;
> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> index b506e3622b08..501ac05ba7da 100644
> --- a/drivers/gpu/drm/drm_edid.c
> +++ b/drivers/gpu/drm/drm_edid.c
> @@ -4830,19 +4830,32 @@ void drm_set_preferred_mode(struct drm_connector *connector,
>  }
>  EXPORT_SYMBOL(drm_set_preferred_mode);
>  
> +static bool is_hdmi2_sink(struct drm_connector *connector)

You're usually known for adding const all around, why not const pointer
here and in all the other drm_* functions that call this?

> +{
> +	/*
> +	 * FIXME: sil-sii8620 doesn't have a connector around when
> +	 * we need one, so we have to be prepared for a NULL connector.
> +	 */
> +	if (!connector)
> +		return false;

This actually changes the is_hdmi2_sink value for sil-sii8620.

> +
> +	return connector->display_info.hdmi.scdc.supported ||
> +		connector->display_info.color_formats & DRM_COLOR_FORMAT_YCRCB420;
> +}
> +
>  /**
>   * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with
>   *                                              data from a DRM display mode
>   * @frame: HDMI AVI infoframe
> + * @connector: the connector
>   * @mode: DRM display mode
> - * @is_hdmi2_sink: Sink is HDMI 2.0 compliant
>   *
>   * Return: 0 on success or a negative error code on failure.
>   */
>  int
>  drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
> -					 const struct drm_display_mode *mode,
> -					 bool is_hdmi2_sink)
> +					 struct drm_connector *connector,
> +					 const struct drm_display_mode *mode)
>  {
>  	enum hdmi_picture_aspect picture_aspect;
>  	int err;
> @@ -4864,7 +4877,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
>  	 * HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
>  	 * have to make sure we dont break HDMI 1.4 sinks.
>  	 */
> -	if (!is_hdmi2_sink && frame->video_code > 64)
> +	if (!is_hdmi2_sink(connector) && frame->video_code > 64)
>  		frame->video_code = 0;
>  
>  	/*
> @@ -4923,21 +4936,17 @@ 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
> + * @connector: the connector
>   * @mode: DRM display mode
>   * @rgb_quant_range: RGB quantization range (Q)
>   * @rgb_quant_range_selectable: Sink support selectable RGB quantization range (QS)
> - * @is_hdmi2_sink: HDMI 2.0 sink, which has different default recommendations
> - *
> - * Note that @is_hdmi2_sink can be derived by looking at the
> - * &drm_scdc.supported flag stored in &drm_hdmi_info.scdc,
> - * &drm_display_info.hdmi, which can be found in &drm_connector.display_info.
>   */
>  void
>  drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
> +				   struct drm_connector *connector,
>  				   const struct drm_display_mode *mode,
>  				   enum hdmi_quantization_range rgb_quant_range,
> -				   bool rgb_quant_range_selectable,
> -				   bool is_hdmi2_sink)
> +				   bool rgb_quant_range_selectable)
>  {
>  	/*
>  	 * CEA-861:
> @@ -4968,7 +4977,7 @@ drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
>  	 * we limit non-zero YQ to HDMI 2.0 sinks only as HDMI 2.0 is based
>  	 * on on CEA-861-F.
>  	 */
> -	if (!is_hdmi2_sink ||
> +	if (!is_hdmi2_sink(connector) ||
>  	    rgb_quant_range == HDMI_QUANTIZATION_RANGE_LIMITED)
>  		frame->ycc_quantization_range =
>  			HDMI_YCC_QUANTIZATION_RANGE_LIMITED;
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 2092a650df7d..b857df67aff0 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -819,7 +819,8 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata)
>  		return;
>  	}
>  
> -	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi, m, false);
> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
> +						       &hdata->connector, m);
>  	if (!ret)
>  		ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
>  	if (ret > 0) {
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index a7c39f39793f..38c66fbc8276 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -849,7 +849,8 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode)
>  {
>  	union hdmi_infoframe frame;
>  
> -	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
> +	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> +						 &priv->connector, mode);
>  	frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL;
>  
>  	tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame);
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index e2c6a2b3e8f2..8a260bd6954c 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -465,15 +465,12 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder,
>  	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
>  	const struct drm_display_mode *adjusted_mode =
>  		&crtc_state->base.adjusted_mode;
> -	struct drm_connector *connector = &intel_hdmi->attached_connector->base;
> -	bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported ||
> -	   connector->display_info.color_formats & DRM_COLOR_FORMAT_YCRCB420;
>  	union hdmi_infoframe frame;
>  	int ret;
>  
>  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> -						       adjusted_mode,
> -						       is_hdmi2_sink);
> +						       conn_state->connector,
> +						       adjusted_mode);
>  	if (ret < 0) {
>  		DRM_ERROR("couldn't fill AVI infoframe\n");
>  		return;
> @@ -486,12 +483,13 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder,
>  	else
>  		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>  
> -	drm_hdmi_avi_infoframe_quant_range(&frame.avi, adjusted_mode,
> +	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> +					   conn_state->connector,
> +					   adjusted_mode,
>  					   crtc_state->limited_color_range ?
>  					   HDMI_QUANTIZATION_RANGE_LIMITED :
>  					   HDMI_QUANTIZATION_RANGE_FULL,
> -					   intel_hdmi->rgb_quant_range_selectable,
> -					   is_hdmi2_sink);
> +					   intel_hdmi->rgb_quant_range_selectable);
>  
>  	drm_hdmi_avi_infoframe_content_type(&frame.avi,
>  					    conn_state);
> diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
> index 96a8d9524b0c..c18f14d41f1d 100644
> --- a/drivers/gpu/drm/i915/intel_lspcon.c
> +++ b/drivers/gpu/drm/i915/intel_lspcon.c
> @@ -462,10 +462,8 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
>  	uint8_t buf[VIDEO_DIP_DATA_SIZE];
>  	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>  	struct intel_lspcon *lspcon = &dig_port->lspcon;
> -	struct intel_dp *intel_dp = &dig_port->dp;
> -	struct drm_connector *connector = &intel_dp->attached_connector->base;
> -	const struct drm_display_mode *mode = &crtc_state->base.adjusted_mode;
> -	bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported;
> +	const struct drm_display_mode *adjusted_mode =
> +		&crtc_state->base.adjusted_mode;
>  
>  	if (!lspcon->active) {
>  		DRM_ERROR("Writing infoframes while LSPCON disabled ?\n");
> @@ -473,7 +471,8 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
>  	}
>  
>  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> -						       mode, is_hdmi2_sink);
> +						       conn_state->connector,
> +						       adjusted_mode);
>  	if (ret < 0) {
>  		DRM_ERROR("couldn't fill AVI infoframe\n");
>  		return;
> @@ -488,11 +487,13 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
>  		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
>  	}
>  
> -	drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
> +	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> +					   conn_state->connector,
> +					   adjusted_mode,
>  					   crtc_state->limited_color_range ?
>  					   HDMI_QUANTIZATION_RANGE_LIMITED :
>  					   HDMI_QUANTIZATION_RANGE_FULL,
> -					   false, is_hdmi2_sink);
> +					   false);
>  
>  	ret = hdmi_infoframe_pack(&frame, buf, sizeof(buf));
>  	if (ret < 0) {
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 5805ec1aba12..1277d31adb54 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -981,7 +981,8 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
>  }
>  
>  static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
> -					 const struct intel_crtc_state *pipe_config)
> +					 const struct intel_crtc_state *pipe_config,
> +					 const struct drm_connector_state *conn_state)
>  {
>  	uint8_t sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
>  	union hdmi_infoframe frame;
> @@ -989,8 +990,8 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
>  	ssize_t len;
>  
>  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> -						       &pipe_config->base.adjusted_mode,
> -						       false);
> +						       conn_state->connector,
> +						       &pipe_config->base.adjusted_mode);
>  	if (ret < 0) {
>  		DRM_ERROR("couldn't fill AVI infoframe\n");
>  		return false;
> @@ -1316,7 +1317,8 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_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, crtc_state);
> +		intel_sdvo_set_avi_infoframe(intel_sdvo,
> +					     crtc_state, conn_state);
>  	} else
>  		intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_DVI);
>  
> diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> index 862f3ec22131..f3ef7bf80563 100644
> --- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
> +++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
> @@ -981,7 +981,8 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi,
>  	u8 buffer[17];
>  	ssize_t err;
>  
> -	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> +	err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
> +						       &hdmi->conn, mode);
>  	if (err < 0) {
>  		dev_err(hdmi->dev,
>  			"Failed to get AVI infoframe from mode: %zd\n", err);
> diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> index 7e357077ed26..5ed4cab2819f 100644
> --- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> +++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
> @@ -101,7 +101,8 @@ static void msm_hdmi_config_avi_infoframe(struct hdmi *hdmi)
>  	u32 val;
>  	int len;
>  
> -	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
> +	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> +						 hdmi->connector, mode);
>  
>  	len = hdmi_infoframe_pack(&frame, buffer, sizeof(buffer));
>  	if (len < 0) {
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 6cbbae3f438b..d8e512c83211 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -554,7 +554,7 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
>  	u32 max_ac_packet;
>  	union hdmi_infoframe avi_frame;
>  	union hdmi_infoframe vendor_frame;
> -	bool scdc_supported, high_tmds_clock_ratio = false, scrambling = false;
> +	bool high_tmds_clock_ratio = false, scrambling = false;
>  	u8 config;
>  	int ret;
>  	int size;
> @@ -564,10 +564,9 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
>  		return;
>  
>  	hdmi = &nv_connector->base.display_info.hdmi;
> -	scdc_supported = hdmi->scdc.supported;
>  
> -	ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame.avi, mode,
> -						       scdc_supported);
> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame.avi,
> +						       &nv_connector->base, mode);
>  	if (!ret) {
>  		/* We have an AVI InfoFrame, populate it to the display */
>  		args.pwr.avi_infoframe_length
> diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
> index 452e625f6ce3..281b2164969c 100644
> --- a/drivers/gpu/drm/omapdrm/omap_encoder.c
> +++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
> @@ -128,8 +128,9 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder,
>  	if (hdmi_mode && dssdev->ops->hdmi.set_infoframe) {
>  		struct hdmi_avi_infoframe avi;
>  
> -		r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode,
> -							     false);
> +		r = drm_hdmi_avi_infoframe_from_display_mode(&avi,
> +							     connector,
> +							     adjusted_mode);
>  		if (r == 0)
>  			dssdev->ops->hdmi.set_infoframe(dssdev, &avi);
>  	}
> diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c
> index 770e31f5fd1b..5a7d48339b32 100644
> --- a/drivers/gpu/drm/radeon/radeon_audio.c
> +++ b/drivers/gpu/drm/radeon/radeon_audio.c
> @@ -516,7 +516,7 @@ static int radeon_audio_set_avi_packet(struct drm_encoder *encoder,
>  	if (!connector)
>  		return -EINVAL;
>  
> -	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> +	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
>  	if (err < 0) {
>  		DRM_ERROR("failed to setup AVI infoframe: %d\n", err);
>  		return err;
> diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
> index 1c02b3e61299..27c945e030a0 100644
> --- a/drivers/gpu/drm/rockchip/inno_hdmi.c
> +++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
> @@ -295,7 +295,9 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
>  	union hdmi_infoframe frame;
>  	int rc;
>  
> -	rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
> +	rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> +						      &hdmi->connector,
> +						      mode);
>  
>  	if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444)
>  		frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
> diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
> index ccf718404a1c..4b86878f8ddf 100644
> --- a/drivers/gpu/drm/sti/sti_hdmi.c
> +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> @@ -434,7 +434,8 @@ static int hdmi_avi_infoframe_config(struct sti_hdmi *hdmi)
>  
>  	DRM_DEBUG_DRIVER("\n");
>  
> -	ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode, false);
> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe,
> +						       hdmi->drm_connector, mode);
>  	if (ret < 0) {
>  		DRM_ERROR("failed to setup AVI infoframe: %d\n", ret);
>  		return ret;
> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> index 061d2e0d9011..554a6f4561f3 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> @@ -52,7 +52,8 @@ static int sun4i_hdmi_setup_avi_infoframes(struct sun4i_hdmi *hdmi,
>  	u8 buffer[17];
>  	int i, ret;
>  
> -	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame,
> +						       &hdmi->connector, mode);
>  	if (ret < 0) {
>  		DRM_ERROR("Failed to get infoframes from mode\n");
>  		return ret;
> diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
> index 0082468f703c..a7566c67bfb0 100644
> --- a/drivers/gpu/drm/tegra/hdmi.c
> +++ b/drivers/gpu/drm/tegra/hdmi.c
> @@ -741,7 +741,8 @@ static void tegra_hdmi_setup_avi_infoframe(struct tegra_hdmi *hdmi,
>  	u8 buffer[17];
>  	ssize_t err;
>  
> -	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> +	err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
> +						       &hdmi->output.connector, mode);
>  	if (err < 0) {
>  		dev_err(hdmi->dev, "failed to setup AVI infoframe: %zd\n", err);
>  		return;
> diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
> index b129da2e5afd..d90bf5f6a67a 100644
> --- a/drivers/gpu/drm/tegra/sor.c
> +++ b/drivers/gpu/drm/tegra/sor.c
> @@ -2116,7 +2116,8 @@ tegra_sor_hdmi_setup_avi_infoframe(struct tegra_sor *sor,
>  	value &= ~INFOFRAME_CTRL_ENABLE;
>  	tegra_sor_writel(sor, value, SOR_HDMI_AVI_INFOFRAME_CTRL);
>  
> -	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> +	err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
> +						       &sor->output.connector, mode);
>  	if (err < 0) {
>  		dev_err(sor->dev, "failed to setup AVI infoframe: %d\n", err);
>  		return err;
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index fd5522fd179e..cb3dca7426c8 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -409,22 +409,25 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
>  {
>  	struct vc4_hdmi_encoder *vc4_encoder = to_vc4_hdmi_encoder(encoder);
>  	struct drm_crtc *crtc = encoder->crtc;
> +	struct vc4_dev *vc4 = to_vc4_dev(encoder->dev);
> +	struct vc4_hdmi *hdmi = vc4->hdmi;
>  	const struct drm_display_mode *mode = &crtc->state->adjusted_mode;
>  	union hdmi_infoframe frame;
>  	int ret;
>  
> -	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> +						       hdmi->connector, mode);
>  	if (ret < 0) {
>  		DRM_ERROR("couldn't fill AVI infoframe\n");
>  		return;
>  	}
>  
> -	drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
> +	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
> +					   hdmi->connector, mode,
>  					   vc4_encoder->limited_rgb_range ?
>  					   HDMI_QUANTIZATION_RANGE_LIMITED :
>  					   HDMI_QUANTIZATION_RANGE_FULL,
> -					   vc4_encoder->rgb_range_selectable,
> -					   false);
> +					   vc4_encoder->rgb_range_selectable);
>  
>  	vc4_hdmi_write_infoframe(encoder, &frame);
>  }
> diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c
> index 78655269d843..9fc98bb4f3d9 100644
> --- a/drivers/gpu/drm/zte/zx_hdmi.c
> +++ b/drivers/gpu/drm/zte/zx_hdmi.c
> @@ -125,7 +125,9 @@ static int zx_hdmi_config_video_avi(struct zx_hdmi *hdmi,
>  	union hdmi_infoframe frame;
>  	int ret;
>  
> -	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> +						       &hdmi->connector,
> +						       mode);
>  	if (ret) {
>  		DRM_DEV_ERROR(hdmi->dev, "failed to get avi infoframe: %d\n",
>  			      ret);
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index e3c404833115..9db6f130df65 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -352,18 +352,18 @@ drm_load_edid_firmware(struct drm_connector *connector)
>  
>  int
>  drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
> -					 const struct drm_display_mode *mode,
> -					 bool is_hdmi2_sink);
> +					 struct drm_connector *connector,
> +					 const struct drm_display_mode *mode);
>  int
>  drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
>  					    struct drm_connector *connector,
>  					    const struct drm_display_mode *mode);
>  void
>  drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
> +				   struct drm_connector *connector,
>  				   const struct drm_display_mode *mode,
>  				   enum hdmi_quantization_range rgb_quant_range,
> -				   bool rgb_quant_range_selectable,
> -				   bool is_hdmi2_sink);
> +				   bool rgb_quant_range_selectable);
>  
>  /**
>   * drm_eld_mnl - Get ELD monitor name length in bytes.

-- 
Jani Nikula, Intel Open Source Graphics Center
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Ville Syrjälä]

On Wed, Nov 21, 2018 at 01:40:43PM +0200, Jani Nikula wrote:
> On Tue, 20 Nov 2018, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Make life easier for drivers by simply passing the connector
> > to drm_hdmi_avi_infoframe_from_display_mode() and
> > drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
> > need to worry about is_hdmi2_sink mess.
> 
> Overall looks about right and nice,
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> But please do take that with a grain of salt for everything outside of
> i915 and drm core.
> 
> Please also find a few comments inline below.
> 
> > Cc: Alex Deucher <alexander.deucher@amd.com>
> > Cc: "Christian König" <christian.koenig@amd.com>
> > Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> > Cc: Archit Taneja <architt@codeaurora.org>
> > Cc: Andrzej Hajda <a.hajda@samsung.com>
> > Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> > Cc: Inki Dae <inki.dae@samsung.com>
> > Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> > Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> > Cc: Kyungmin Park <kyungmin.park@samsung.com>
> > Cc: Russell King <linux@armlinux.org.uk>
> > Cc: CK Hu <ck.hu@mediatek.com>
> > Cc: Philipp Zabel <p.zabel@pengutronix.de>
> > Cc: Rob Clark <robdclark@gmail.com>
> > Cc: Ben Skeggs <bskeggs@redhat.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: Sandy Huang <hjc@rock-chips.com>
> > Cc: "Heiko Stübner" <heiko@sntech.de>
> > Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> > Cc: Vincent Abriou <vincent.abriou@st.com>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Cc: Eric Anholt <eric@anholt.net>
> > Cc: Shawn Guo <shawnguo@kernel.org>
> > Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> > Cc: amd-gfx@lists.freedesktop.org
> > Cc: linux-arm-msm@vger.kernel.org
> > Cc: freedreno@lists.freedesktop.org
> > Cc: nouveau@lists.freedesktop.org
> > Cc: linux-tegra@vger.kernel.org
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  2 +-
> >  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  2 +-
> >  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  3 ++-
> >  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  2 +-
> >  drivers/gpu/drm/bridge/analogix-anx78xx.c |  5 ++--
> >  drivers/gpu/drm/bridge/sii902x.c          |  3 ++-
> >  drivers/gpu/drm/bridge/sil-sii8620.c      |  3 +--
> >  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  3 ++-
> >  drivers/gpu/drm/drm_edid.c                | 33 ++++++++++++++---------
> >  drivers/gpu/drm/exynos/exynos_hdmi.c      |  3 ++-
> >  drivers/gpu/drm/i2c/tda998x_drv.c         |  3 ++-
> >  drivers/gpu/drm/i915/intel_hdmi.c         | 14 +++++-----
> >  drivers/gpu/drm/i915/intel_lspcon.c       | 15 ++++++-----
> >  drivers/gpu/drm/i915/intel_sdvo.c         | 10 ++++---
> >  drivers/gpu/drm/mediatek/mtk_hdmi.c       |  3 ++-
> >  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |  3 ++-
> >  drivers/gpu/drm/nouveau/dispnv50/disp.c   |  7 +++--
> >  drivers/gpu/drm/omapdrm/omap_encoder.c    |  5 ++--
> >  drivers/gpu/drm/radeon/radeon_audio.c     |  2 +-
> >  drivers/gpu/drm/rockchip/inno_hdmi.c      |  4 ++-
> >  drivers/gpu/drm/sti/sti_hdmi.c            |  3 ++-
> >  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |  3 ++-
> >  drivers/gpu/drm/tegra/hdmi.c              |  3 ++-
> >  drivers/gpu/drm/tegra/sor.c               |  3 ++-
> >  drivers/gpu/drm/vc4/vc4_hdmi.c            | 11 +++++---
> >  drivers/gpu/drm/zte/zx_hdmi.c             |  4 ++-
> >  include/drm/drm_edid.h                    |  8 +++---
> >  27 files changed, 94 insertions(+), 66 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> > index 4cfecdce29a3..1f0426d2fc2a 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
> > @@ -1682,7 +1682,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder *encoder,
> >  	dce_v10_0_audio_write_sad_regs(encoder);
> >  	dce_v10_0_audio_write_latency_fields(encoder, mode);
> >  
> > -	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> > +	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
> >  	if (err < 0) {
> >  		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> >  		return;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> > index 7c868916d90f..2280b971d758 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
> > @@ -1724,7 +1724,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder *encoder,
> >  	dce_v11_0_audio_write_sad_regs(encoder);
> >  	dce_v11_0_audio_write_latency_fields(encoder, mode);
> >  
> > -	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> > +	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
> >  	if (err < 0) {
> >  		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> >  		return;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > index 17eaaba36017..db443ec53d3a 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
> > @@ -1423,6 +1423,7 @@ static void dce_v6_0_audio_set_avi_infoframe(struct drm_encoder *encoder,
> >  	struct amdgpu_device *adev = dev->dev_private;
> >  	struct amdgpu_encoder *amdgpu_encoder = to_amdgpu_encoder(encoder);
> >  	struct amdgpu_encoder_atom_dig *dig = amdgpu_encoder->enc_priv;
> > +	struct drm_connector *connector = amdgpu_get_connector_for_encoder(encoder);
> >  	struct hdmi_avi_infoframe frame;
> >  	u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE];
> >  	uint8_t *payload = buffer + 3;
> > @@ -1430,7 +1431,7 @@ static void dce_v6_0_audio_set_avi_infoframe(struct drm_encoder *encoder,
> >  	ssize_t err;
> >  	u32 tmp;
> >  
> > -	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> > +	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
> >  	if (err < 0) {
> >  		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> >  		return;
> > diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> > index 8c0576978d36..13da915991dd 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
> > @@ -1616,7 +1616,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
> >  	dce_v8_0_audio_write_sad_regs(encoder);
> >  	dce_v8_0_audio_write_latency_fields(encoder, mode);
> >  
> > -	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> > +	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
> >  	if (err < 0) {
> >  		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
> >  		return;
> > diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
> > index f8433c93f463..e11309e9bc4f 100644
> > --- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
> > +++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
> > @@ -1094,8 +1094,9 @@ static void anx78xx_bridge_mode_set(struct drm_bridge *bridge,
> >  
> >  	mutex_lock(&anx78xx->lock);
> >  
> > -	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode,
> > -						       false);
> > +	err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
> > +						       &anx78xx->connector,
> > +						       adjusted_mode);
> >  	if (err) {
> >  		DRM_ERROR("Failed to setup AVI infoframe: %d\n", err);
> >  		goto unlock;
> > diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
> > index bfa902013aa4..a9b4f45ae87c 100644
> > --- a/drivers/gpu/drm/bridge/sii902x.c
> > +++ b/drivers/gpu/drm/bridge/sii902x.c
> > @@ -258,7 +258,8 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
> >  	if (ret)
> >  		return;
> >  
> > -	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj, false);
> > +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame,
> > +						       &sii902x->connector, adj);
> >  	if (ret < 0) {
> >  		DRM_ERROR("couldn't fill AVI infoframe\n");
> >  		return;
> > diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
> > index a6e8f4591e63..0cc293a6ac24 100644
> > --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> > +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> > @@ -1104,8 +1104,7 @@ static void sii8620_set_infoframes(struct sii8620 *ctx,
> >  	int ret;
> >  
> >  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
> > -						       mode,
> > -						       true);
> > +						       NULL, mode);
> >  	if (ctx->use_packed_pixel)
> >  		frm.avi.colorspace = HDMI_COLORSPACE_YUV422;
> >  
> > diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > index 64c3cf027518..88b720b63126 100644
> > --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> > @@ -1344,7 +1344,8 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
> >  	u8 val;
> >  
> >  	/* Initialise info frame from DRM mode */
> > -	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> > +	drm_hdmi_avi_infoframe_from_display_mode(&frame,
> > +						 &hdmi->connector, mode);
> >  
> >  	if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
> >  		frame.colorspace = HDMI_COLORSPACE_YUV444;
> > diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> > index b506e3622b08..501ac05ba7da 100644
> > --- a/drivers/gpu/drm/drm_edid.c
> > +++ b/drivers/gpu/drm/drm_edid.c
> > @@ -4830,19 +4830,32 @@ void drm_set_preferred_mode(struct drm_connector *connector,
> >  }
> >  EXPORT_SYMBOL(drm_set_preferred_mode);
> >  
> > +static bool is_hdmi2_sink(struct drm_connector *connector)
> 
> You're usually known for adding const all around, why not const pointer
> here and in all the other drm_* functions that call this?

My current approach is to constify states/fbs/etc. but not so much
crtcs/connectors/etc. Too much const can sometimes get in the way
of things requiring that you remove the const later. But I guess
in this case the const shouldn't really get in the way of anything
because these are pretty much supposed to be pure functions.

> 
> > +{
> > +	/*
> > +	 * FIXME: sil-sii8620 doesn't have a connector around when
> > +	 * we need one, so we have to be prepared for a NULL connector.
> > +	 */
> > +	if (!connector)
> > +		return false;
> 
> This actually changes the is_hdmi2_sink value for sil-sii8620.

Hmm. No idea why they would have set that to true when everyone else is
passing false. I guess I can change this to true to not change it. IIRC
that was the only driver that didn't have a connector around.

That said, I was actually thinking of removing this hdmi2 vs. not
stuff from drm_hdmi_avi_infoframe_from_display_mode(). We added it
"just in case", but we already have a similar issue with earlier
cea-861 revisions and haven't seen any bugs where an older monitor
would get confused by a VIC not yet defined when the monitor was
produced.

-- 
Ville Syrjälä
Intel
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Andrzej Hajda]

Quite late, hopefully not too late.


On 21.11.2018 12:51, Ville Syrjälä wrote:
> On Wed, Nov 21, 2018 at 01:40:43PM +0200, Jani Nikula wrote:
>>
>>>  		return;
>>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
>>> index a6e8f4591e63..0cc293a6ac24 100644
>>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>>> @@ -1104,8 +1104,7 @@ static void sii8620_set_infoframes(struct sii8620 *ctx,
>>>  	int ret;
>>>  
>>>  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
>>> -						       mode,
>>> -						       true);
>>> +						       NULL, mode);
>>>  	if (ctx->use_packed_pixel)
>>>  		frm.avi.colorspace = HDMI_COLORSPACE_YUV422;
>>>  
>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> index 64c3cf027518..88b720b63126 100644
>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>> @@ -1344,7 +1344,8 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
>>>  	u8 val;
>>>  
>>>  	/* Initialise info frame from DRM mode */
>>> -	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>>> +	drm_hdmi_avi_infoframe_from_display_mode(&frame,
>>> +						 &hdmi->connector, mode);
>>>  
>>>  	if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
>>>  		frame.colorspace = HDMI_COLORSPACE_YUV444;
>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>> index b506e3622b08..501ac05ba7da 100644
>>> --- a/drivers/gpu/drm/drm_edid.c
>>> +++ b/drivers/gpu/drm/drm_edid.c
>>> @@ -4830,19 +4830,32 @@ void drm_set_preferred_mode(struct drm_connector *connector,
>>>  }
>>>  EXPORT_SYMBOL(drm_set_preferred_mode);
>>>  
>>> +static bool is_hdmi2_sink(struct drm_connector *connector)
>> You're usually known for adding const all around, why not const pointer
>> here and in all the other drm_* functions that call this?
> My current approach is to constify states/fbs/etc. but not so much
> crtcs/connectors/etc. Too much const can sometimes get in the way
> of things requiring that you remove the const later. But I guess
> in this case the const shouldn't really get in the way of anything
> because these are pretty much supposed to be pure functions.
>
>>> +{
>>> +	/*
>>> +	 * FIXME: sil-sii8620 doesn't have a connector around when
>>> +	 * we need one, so we have to be prepared for a NULL connector.
>>> +	 */
>>> +	if (!connector)
>>> +		return false;
>> This actually changes the is_hdmi2_sink value for sil-sii8620.
> Hmm. No idea why they would have set that to true when everyone else is
> passing false. 


Because false does not work :) More precisely MHLv3 (used in Sii8620)
uses CTA-861-F standard for infoframes, which is specific to HDMI2.0.

Unfortunately I have no access to MHL specs, but my experiments and
vendor drivers strongly suggests it is done this way.

This is important in case of 4K modes which are handled differently by
HDMI 1.4 and HDMI2.0.

The pipeline looks like (in parenthesis HDMI version on the stream):

exynos_hdmi --(1.4)--> SII8620 --(2.0)--> MHL_dongle --(1.4)--> TV


> I guess I can change this to true to not change it. IIRC
> that was the only driver that didn't have a connector around.
>
> That said, I was actually thinking of removing this hdmi2 vs. not
> stuff from drm_hdmi_avi_infoframe_from_display_mode(). We added it
> "just in case", but we already have a similar issue with earlier
> cea-861 revisions and haven't seen any bugs where an older monitor
> would get confused by a VIC not yet defined when the monitor was
> produced.
>
Are you sure this is a good idea? As I said earlier 4K modes are present
in HDMI 1.4 and 2.0 but they are handled differently, so this is not
only matter of unknown VIC in AVIF.


Regards

Andrzej

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

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Ville Syrjälä]

On Thu, Nov 29, 2018 at 09:46:16AM +0100, Andrzej Hajda wrote:
> Quite late, hopefully not too late.
> 
> 
> On 21.11.2018 12:51, Ville Syrjälä wrote:
> > On Wed, Nov 21, 2018 at 01:40:43PM +0200, Jani Nikula wrote:
> >>
> >>>  		return;
> >>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
> >>> index a6e8f4591e63..0cc293a6ac24 100644
> >>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> >>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> >>> @@ -1104,8 +1104,7 @@ static void sii8620_set_infoframes(struct sii8620 *ctx,
> >>>  	int ret;
> >>>  
> >>>  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
> >>> -						       mode,
> >>> -						       true);
> >>> +						       NULL, mode);
> >>>  	if (ctx->use_packed_pixel)
> >>>  		frm.avi.colorspace = HDMI_COLORSPACE_YUV422;
> >>>  
> >>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>> index 64c3cf027518..88b720b63126 100644
> >>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>> @@ -1344,7 +1344,8 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
> >>>  	u8 val;
> >>>  
> >>>  	/* Initialise info frame from DRM mode */
> >>> -	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> >>> +	drm_hdmi_avi_infoframe_from_display_mode(&frame,
> >>> +						 &hdmi->connector, mode);
> >>>  
> >>>  	if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
> >>>  		frame.colorspace = HDMI_COLORSPACE_YUV444;
> >>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >>> index b506e3622b08..501ac05ba7da 100644
> >>> --- a/drivers/gpu/drm/drm_edid.c
> >>> +++ b/drivers/gpu/drm/drm_edid.c
> >>> @@ -4830,19 +4830,32 @@ void drm_set_preferred_mode(struct drm_connector *connector,
> >>>  }
> >>>  EXPORT_SYMBOL(drm_set_preferred_mode);
> >>>  
> >>> +static bool is_hdmi2_sink(struct drm_connector *connector)
> >> You're usually known for adding const all around, why not const pointer
> >> here and in all the other drm_* functions that call this?
> > My current approach is to constify states/fbs/etc. but not so much
> > crtcs/connectors/etc. Too much const can sometimes get in the way
> > of things requiring that you remove the const later. But I guess
> > in this case the const shouldn't really get in the way of anything
> > because these are pretty much supposed to be pure functions.
> >
> >>> +{
> >>> +	/*
> >>> +	 * FIXME: sil-sii8620 doesn't have a connector around when
> >>> +	 * we need one, so we have to be prepared for a NULL connector.
> >>> +	 */
> >>> +	if (!connector)
> >>> +		return false;
> >> This actually changes the is_hdmi2_sink value for sil-sii8620.
> > Hmm. No idea why they would have set that to true when everyone else is
> > passing false. 
> 
> 
> Because false does not work :) More precisely MHLv3 (used in Sii8620)
> uses CTA-861-F standard for infoframes, which is specific to HDMI2.0.
> 
> Unfortunately I have no access to MHL specs, but my experiments and
> vendor drivers strongly suggests it is done this way.
> 
> This is important in case of 4K modes which are handled differently by
> HDMI 1.4 and HDMI2.0.

HDMI 2.0 handles 4k just like 1.4 handled it when you use one of
the 4k modes defined in 1.4. Only if you use features beyond 1.4 do we
switch over to the HDMI 2.0 specific signalling.

> 
> The pipeline looks like (in parenthesis HDMI version on the stream):
> 
> exynos_hdmi --(1.4)--> SII8620 --(2.0)--> MHL_dongle --(1.4)--> TV
> 
> 
> > I guess I can change this to true to not change it. IIRC
> > that was the only driver that didn't have a connector around.
> >
> > That said, I was actually thinking of removing this hdmi2 vs. not
> > stuff from drm_hdmi_avi_infoframe_from_display_mode(). We added it
> > "just in case", but we already have a similar issue with earlier
> > cea-861 revisions and haven't seen any bugs where an older monitor
> > would get confused by a VIC not yet defined when the monitor was
> > produced.
> >
> Are you sure this is a good idea? As I said earlier 4K modes are present
> in HDMI 1.4 and 2.0 but they are handled differently, so this is not
> only matter of unknown VIC in AVIF.
> 
> 
> Regards
> 
> Andrzej

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

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Andrzej Hajda]

On 03.12.2018 22:48, Ville Syrjälä wrote:
> On Thu, Nov 29, 2018 at 09:46:16AM +0100, Andrzej Hajda wrote:
>> Quite late, hopefully not too late.
>>
>>
>> On 21.11.2018 12:51, Ville Syrjälä wrote:
>>> On Wed, Nov 21, 2018 at 01:40:43PM +0200, Jani Nikula wrote:
>>>>>  		return;
>>>>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
>>>>> index a6e8f4591e63..0cc293a6ac24 100644
>>>>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>>>>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>>>>> @@ -1104,8 +1104,7 @@ static void sii8620_set_infoframes(struct sii8620 *ctx,
>>>>>  	int ret;
>>>>>  
>>>>>  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
>>>>> -						       mode,
>>>>> -						       true);
>>>>> +						       NULL, mode);
>>>>>  	if (ctx->use_packed_pixel)
>>>>>  		frm.avi.colorspace = HDMI_COLORSPACE_YUV422;
>>>>>  
>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> index 64c3cf027518..88b720b63126 100644
>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>> @@ -1344,7 +1344,8 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
>>>>>  	u8 val;
>>>>>  
>>>>>  	/* Initialise info frame from DRM mode */
>>>>> -	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>>>>> +	drm_hdmi_avi_infoframe_from_display_mode(&frame,
>>>>> +						 &hdmi->connector, mode);
>>>>>  
>>>>>  	if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
>>>>>  		frame.colorspace = HDMI_COLORSPACE_YUV444;
>>>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>>>> index b506e3622b08..501ac05ba7da 100644
>>>>> --- a/drivers/gpu/drm/drm_edid.c
>>>>> +++ b/drivers/gpu/drm/drm_edid.c
>>>>> @@ -4830,19 +4830,32 @@ void drm_set_preferred_mode(struct drm_connector *connector,
>>>>>  }
>>>>>  EXPORT_SYMBOL(drm_set_preferred_mode);
>>>>>  
>>>>> +static bool is_hdmi2_sink(struct drm_connector *connector)
>>>> You're usually known for adding const all around, why not const pointer
>>>> here and in all the other drm_* functions that call this?
>>> My current approach is to constify states/fbs/etc. but not so much
>>> crtcs/connectors/etc. Too much const can sometimes get in the way
>>> of things requiring that you remove the const later. But I guess
>>> in this case the const shouldn't really get in the way of anything
>>> because these are pretty much supposed to be pure functions.
>>>
>>>>> +{
>>>>> +	/*
>>>>> +	 * FIXME: sil-sii8620 doesn't have a connector around when
>>>>> +	 * we need one, so we have to be prepared for a NULL connector.
>>>>> +	 */
>>>>> +	if (!connector)
>>>>> +		return false;
>>>> This actually changes the is_hdmi2_sink value for sil-sii8620.
>>> Hmm. No idea why they would have set that to true when everyone else is
>>> passing false. 
>>
>> Because false does not work :) More precisely MHLv3 (used in Sii8620)
>> uses CTA-861-F standard for infoframes, which is specific to HDMI2.0.
>>
>> Unfortunately I have no access to MHL specs, but my experiments and
>> vendor drivers strongly suggests it is done this way.
>>
>> This is important in case of 4K modes which are handled differently by
>> HDMI 1.4 and HDMI2.0.
> HDMI 2.0 handles 4k just like 1.4 handled it when you use one of
> the 4k modes defined in 1.4. Only if you use features beyond 1.4 do we
> switch over to the HDMI 2.0 specific signalling.


The difference is in infoframes:

HDMI 1.4 sets AVI infoframe VIC to 0, and sends HDMI_VIC in VSI.

HDMI 2.0 sets AVI infoframe to non zero VICs introduced by
HDMI2.0/CEA-861-F, VSI can be omitted if I remember correctly, unless 3d
is in use.


So setting VICs to non-zero in case of HDMI1.4 sinks and 4k modes seems
risky.


Regards

Andrzej


>
>> The pipeline looks like (in parenthesis HDMI version on the stream):
>>
>> exynos_hdmi --(1.4)--> SII8620 --(2.0)--> MHL_dongle --(1.4)--> TV
>>
>>
>>> I guess I can change this to true to not change it. IIRC
>>> that was the only driver that didn't have a connector around.
>>>
>>> That said, I was actually thinking of removing this hdmi2 vs. not
>>> stuff from drm_hdmi_avi_infoframe_from_display_mode(). We added it
>>> "just in case", but we already have a similar issue with earlier
>>> cea-861 revisions and haven't seen any bugs where an older monitor
>>> would get confused by a VIC not yet defined when the monitor was
>>> produced.
>>>
>> Are you sure this is a good idea? As I said earlier 4K modes are present
>> in HDMI 1.4 and 2.0 but they are handled differently, so this is not
>> only matter of unknown VIC in AVIF.
>>
>>
>> Regards
>>
>> Andrzej


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

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Ville Syrjälä]

On Tue, Dec 04, 2018 at 08:03:53AM +0100, Andrzej Hajda wrote:
> On 03.12.2018 22:48, Ville Syrjälä wrote:
> > On Thu, Nov 29, 2018 at 09:46:16AM +0100, Andrzej Hajda wrote:
> >> Quite late, hopefully not too late.
> >>
> >>
> >> On 21.11.2018 12:51, Ville Syrjälä wrote:
> >>> On Wed, Nov 21, 2018 at 01:40:43PM +0200, Jani Nikula wrote:
> >>>>>  		return;
> >>>>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
> >>>>> index a6e8f4591e63..0cc293a6ac24 100644
> >>>>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> >>>>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> >>>>> @@ -1104,8 +1104,7 @@ static void sii8620_set_infoframes(struct sii8620 *ctx,
> >>>>>  	int ret;
> >>>>>  
> >>>>>  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
> >>>>> -						       mode,
> >>>>> -						       true);
> >>>>> +						       NULL, mode);
> >>>>>  	if (ctx->use_packed_pixel)
> >>>>>  		frm.avi.colorspace = HDMI_COLORSPACE_YUV422;
> >>>>>  
> >>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>>> index 64c3cf027518..88b720b63126 100644
> >>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>>> @@ -1344,7 +1344,8 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
> >>>>>  	u8 val;
> >>>>>  
> >>>>>  	/* Initialise info frame from DRM mode */
> >>>>> -	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> >>>>> +	drm_hdmi_avi_infoframe_from_display_mode(&frame,
> >>>>> +						 &hdmi->connector, mode);
> >>>>>  
> >>>>>  	if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
> >>>>>  		frame.colorspace = HDMI_COLORSPACE_YUV444;
> >>>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >>>>> index b506e3622b08..501ac05ba7da 100644
> >>>>> --- a/drivers/gpu/drm/drm_edid.c
> >>>>> +++ b/drivers/gpu/drm/drm_edid.c
> >>>>> @@ -4830,19 +4830,32 @@ void drm_set_preferred_mode(struct drm_connector *connector,
> >>>>>  }
> >>>>>  EXPORT_SYMBOL(drm_set_preferred_mode);
> >>>>>  
> >>>>> +static bool is_hdmi2_sink(struct drm_connector *connector)
> >>>> You're usually known for adding const all around, why not const pointer
> >>>> here and in all the other drm_* functions that call this?
> >>> My current approach is to constify states/fbs/etc. but not so much
> >>> crtcs/connectors/etc. Too much const can sometimes get in the way
> >>> of things requiring that you remove the const later. But I guess
> >>> in this case the const shouldn't really get in the way of anything
> >>> because these are pretty much supposed to be pure functions.
> >>>
> >>>>> +{
> >>>>> +	/*
> >>>>> +	 * FIXME: sil-sii8620 doesn't have a connector around when
> >>>>> +	 * we need one, so we have to be prepared for a NULL connector.
> >>>>> +	 */
> >>>>> +	if (!connector)
> >>>>> +		return false;
> >>>> This actually changes the is_hdmi2_sink value for sil-sii8620.
> >>> Hmm. No idea why they would have set that to true when everyone else is
> >>> passing false. 
> >>
> >> Because false does not work :) More precisely MHLv3 (used in Sii8620)
> >> uses CTA-861-F standard for infoframes, which is specific to HDMI2.0.
> >>
> >> Unfortunately I have no access to MHL specs, but my experiments and
> >> vendor drivers strongly suggests it is done this way.
> >>
> >> This is important in case of 4K modes which are handled differently by
> >> HDMI 1.4 and HDMI2.0.
> > HDMI 2.0 handles 4k just like 1.4 handled it when you use one of
> > the 4k modes defined in 1.4. Only if you use features beyond 1.4 do we
> > switch over to the HDMI 2.0 specific signalling.
> 
> 
> The difference is in infoframes:
> 
> HDMI 1.4 sets AVI infoframe VIC to 0, and sends HDMI_VIC in VSI.
> 
> HDMI 2.0 sets AVI infoframe to non zero VICs introduced by
> HDMI2.0/CEA-861-F, VSI can be omitted if I remember correctly, unless 3d
> is in use.

Like I said, The HDMI 1.4 method is used even with HDMI 2.0 sinks unless
some feature gets used which can't be signalled via the HDMI 1.4 vendor
specific infoframe.

> 
> 
> So setting VICs to non-zero in case of HDMI1.4 sinks and 4k modes seems
> risky.

That is not what I was proposing.

> 
> 
> Regards
> 
> Andrzej
> 
> 
> >
> >> The pipeline looks like (in parenthesis HDMI version on the stream):
> >>
> >> exynos_hdmi --(1.4)--> SII8620 --(2.0)--> MHL_dongle --(1.4)--> TV
> >>
> >>
> >>> I guess I can change this to true to not change it. IIRC
> >>> that was the only driver that didn't have a connector around.
> >>>
> >>> That said, I was actually thinking of removing this hdmi2 vs. not
> >>> stuff from drm_hdmi_avi_infoframe_from_display_mode(). We added it
> >>> "just in case", but we already have a similar issue with earlier
> >>> cea-861 revisions and haven't seen any bugs where an older monitor
> >>> would get confused by a VIC not yet defined when the monitor was
> >>> produced.
> >>>
> >> Are you sure this is a good idea? As I said earlier 4K modes are present
> >> in HDMI 1.4 and 2.0 but they are handled differently, so this is not
> >> only matter of unknown VIC in AVIF.
> >>
> >>
> >> Regards
> >>
> >> Andrzej
> 

-- 
Ville Syrjälä
Intel
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Andrzej Hajda]

On 04.12.2018 20:02, Ville Syrjälä wrote:
> On Tue, Dec 04, 2018 at 08:03:53AM +0100, Andrzej Hajda wrote:
>> On 03.12.2018 22:48, Ville Syrjälä wrote:
>>> On Thu, Nov 29, 2018 at 09:46:16AM +0100, Andrzej Hajda wrote:
>>>> Quite late, hopefully not too late.
>>>>
>>>>
>>>> On 21.11.2018 12:51, Ville Syrjälä wrote:
>>>>> On Wed, Nov 21, 2018 at 01:40:43PM +0200, Jani Nikula wrote:
>>>>>>>  		return;
>>>>>>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
>>>>>>> index a6e8f4591e63..0cc293a6ac24 100644
>>>>>>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
>>>>>>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
>>>>>>> @@ -1104,8 +1104,7 @@ static void sii8620_set_infoframes(struct sii8620 *ctx,
>>>>>>>  	int ret;
>>>>>>>  
>>>>>>>  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
>>>>>>> -						       mode,
>>>>>>> -						       true);
>>>>>>> +						       NULL, mode);
>>>>>>>  	if (ctx->use_packed_pixel)
>>>>>>>  		frm.avi.colorspace = HDMI_COLORSPACE_YUV422;
>>>>>>>  
>>>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>>> index 64c3cf027518..88b720b63126 100644
>>>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>>>>>>> @@ -1344,7 +1344,8 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
>>>>>>>  	u8 val;
>>>>>>>  
>>>>>>>  	/* Initialise info frame from DRM mode */
>>>>>>> -	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
>>>>>>> +	drm_hdmi_avi_infoframe_from_display_mode(&frame,
>>>>>>> +						 &hdmi->connector, mode);
>>>>>>>  
>>>>>>>  	if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
>>>>>>>  		frame.colorspace = HDMI_COLORSPACE_YUV444;
>>>>>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
>>>>>>> index b506e3622b08..501ac05ba7da 100644
>>>>>>> --- a/drivers/gpu/drm/drm_edid.c
>>>>>>> +++ b/drivers/gpu/drm/drm_edid.c
>>>>>>> @@ -4830,19 +4830,32 @@ void drm_set_preferred_mode(struct drm_connector *connector,
>>>>>>>  }
>>>>>>>  EXPORT_SYMBOL(drm_set_preferred_mode);
>>>>>>>  
>>>>>>> +static bool is_hdmi2_sink(struct drm_connector *connector)
>>>>>> You're usually known for adding const all around, why not const pointer
>>>>>> here and in all the other drm_* functions that call this?
>>>>> My current approach is to constify states/fbs/etc. but not so much
>>>>> crtcs/connectors/etc. Too much const can sometimes get in the way
>>>>> of things requiring that you remove the const later. But I guess
>>>>> in this case the const shouldn't really get in the way of anything
>>>>> because these are pretty much supposed to be pure functions.
>>>>>
>>>>>>> +{
>>>>>>> +	/*
>>>>>>> +	 * FIXME: sil-sii8620 doesn't have a connector around when
>>>>>>> +	 * we need one, so we have to be prepared for a NULL connector.
>>>>>>> +	 */
>>>>>>> +	if (!connector)
>>>>>>> +		return false;
>>>>>> This actually changes the is_hdmi2_sink value for sil-sii8620.
>>>>> Hmm. No idea why they would have set that to true when everyone else is
>>>>> passing false. 
>>>> Because false does not work :) More precisely MHLv3 (used in Sii8620)
>>>> uses CTA-861-F standard for infoframes, which is specific to HDMI2.0.
>>>>
>>>> Unfortunately I have no access to MHL specs, but my experiments and
>>>> vendor drivers strongly suggests it is done this way.
>>>>
>>>> This is important in case of 4K modes which are handled differently by
>>>> HDMI 1.4 and HDMI2.0.
>>> HDMI 2.0 handles 4k just like 1.4 handled it when you use one of
>>> the 4k modes defined in 1.4. Only if you use features beyond 1.4 do we
>>> switch over to the HDMI 2.0 specific signalling.
>>
>> The difference is in infoframes:
>>
>> HDMI 1.4 sets AVI infoframe VIC to 0, and sends HDMI_VIC in VSI.
>>
>> HDMI 2.0 sets AVI infoframe to non zero VICs introduced by
>> HDMI2.0/CEA-861-F, VSI can be omitted if I remember correctly, unless 3d
>> is in use.
> Like I said, The HDMI 1.4 method is used even with HDMI 2.0 sinks unless
> some feature gets used which can't be signalled via the HDMI 1.4 vendor
> specific infoframe.


Do you mean that 4K VICs 95, 94, 93, 98 defined in CEA-861-F are not
used at all for non-3d video in HDMI 2.0?

Chapter 10.1 of HDMI2.0 spec says clearly:

> When transmitting any additional Video Format for which a VIC value
> has been defined in
> CEA-861-F tables 1, 2, and 3, an HDMI Source shall set the VIC field
> to the Video Code for
> that format.


It contradicts your statement, or am I missing something?


>
>>
>> So setting VICs to non-zero in case of HDMI1.4 sinks and 4k modes seems
>> risky.
> That is not what I was proposing.


Maybe I have misread your statement:

On 21.11.2018 12:51, Ville Syrjälä wrote:

> That said, I was actually thinking of removing this hdmi2 vs. not
> stuff from drm_hdmi_avi_infoframe_from_display_mode(). We added it
> "just in case", but we already have a similar issue with earlier
> cea-861 revisions and haven't seen any bugs where an older monitor
> would get confused by a VIC not yet defined when the monitor was
> produced.


I do not know what it could mean other that allowing sending VICs
unknown to sink?

Maybe better would be just to wait for the patch to avoid misunderstandings.


Regards

Andrzej


>
>>
>> Regards
>>
>> Andrzej
>>
>>
>>>> The pipeline looks like (in parenthesis HDMI version on the stream):
>>>>
>>>> exynos_hdmi --(1.4)--> SII8620 --(2.0)--> MHL_dongle --(1.4)--> TV
>>>>
>>>>
>>>>> I guess I can change this to true to not change it. IIRC
>>>>> that was the only driver that didn't have a connector around.
>>>>>
>>>>> That said, I was actually thinking of removing this hdmi2 vs. not
>>>>> stuff from drm_hdmi_avi_infoframe_from_display_mode(). We added it
>>>>> "just in case", but we already have a similar issue with earlier
>>>>> cea-861 revisions and haven't seen any bugs where an older monitor
>>>>> would get confused by a VIC not yet defined when the monitor was
>>>>> produced.
>>>>>
>>>> Are you sure this is a good idea? As I said earlier 4K modes are present
>>>> in HDMI 1.4 and 2.0 but they are handled differently, so this is not
>>>> only matter of unknown VIC in AVIF.
>>>>
>>>>
>>>> Regards
>>>>
>>>> Andrzej


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

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Ville Syrjälä]

On Wed, Dec 05, 2018 at 08:40:34AM +0100, Andrzej Hajda wrote:
> On 04.12.2018 20:02, Ville Syrjälä wrote:
> > On Tue, Dec 04, 2018 at 08:03:53AM +0100, Andrzej Hajda wrote:
> >> On 03.12.2018 22:48, Ville Syrjälä wrote:
> >>> On Thu, Nov 29, 2018 at 09:46:16AM +0100, Andrzej Hajda wrote:
> >>>> Quite late, hopefully not too late.
> >>>>
> >>>>
> >>>> On 21.11.2018 12:51, Ville Syrjälä wrote:
> >>>>> On Wed, Nov 21, 2018 at 01:40:43PM +0200, Jani Nikula wrote:
> >>>>>>>  		return;
> >>>>>>> diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
> >>>>>>> index a6e8f4591e63..0cc293a6ac24 100644
> >>>>>>> --- a/drivers/gpu/drm/bridge/sil-sii8620.c
> >>>>>>> +++ b/drivers/gpu/drm/bridge/sil-sii8620.c
> >>>>>>> @@ -1104,8 +1104,7 @@ static void sii8620_set_infoframes(struct sii8620 *ctx,
> >>>>>>>  	int ret;
> >>>>>>>  
> >>>>>>>  	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
> >>>>>>> -						       mode,
> >>>>>>> -						       true);
> >>>>>>> +						       NULL, mode);
> >>>>>>>  	if (ctx->use_packed_pixel)
> >>>>>>>  		frm.avi.colorspace = HDMI_COLORSPACE_YUV422;
> >>>>>>>  
> >>>>>>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>>>>> index 64c3cf027518..88b720b63126 100644
> >>>>>>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>>>>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> >>>>>>> @@ -1344,7 +1344,8 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
> >>>>>>>  	u8 val;
> >>>>>>>  
> >>>>>>>  	/* Initialise info frame from DRM mode */
> >>>>>>> -	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
> >>>>>>> +	drm_hdmi_avi_infoframe_from_display_mode(&frame,
> >>>>>>> +						 &hdmi->connector, mode);
> >>>>>>>  
> >>>>>>>  	if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
> >>>>>>>  		frame.colorspace = HDMI_COLORSPACE_YUV444;
> >>>>>>> diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
> >>>>>>> index b506e3622b08..501ac05ba7da 100644
> >>>>>>> --- a/drivers/gpu/drm/drm_edid.c
> >>>>>>> +++ b/drivers/gpu/drm/drm_edid.c
> >>>>>>> @@ -4830,19 +4830,32 @@ void drm_set_preferred_mode(struct drm_connector *connector,
> >>>>>>>  }
> >>>>>>>  EXPORT_SYMBOL(drm_set_preferred_mode);
> >>>>>>>  
> >>>>>>> +static bool is_hdmi2_sink(struct drm_connector *connector)
> >>>>>> You're usually known for adding const all around, why not const pointer
> >>>>>> here and in all the other drm_* functions that call this?
> >>>>> My current approach is to constify states/fbs/etc. but not so much
> >>>>> crtcs/connectors/etc. Too much const can sometimes get in the way
> >>>>> of things requiring that you remove the const later. But I guess
> >>>>> in this case the const shouldn't really get in the way of anything
> >>>>> because these are pretty much supposed to be pure functions.
> >>>>>
> >>>>>>> +{
> >>>>>>> +	/*
> >>>>>>> +	 * FIXME: sil-sii8620 doesn't have a connector around when
> >>>>>>> +	 * we need one, so we have to be prepared for a NULL connector.
> >>>>>>> +	 */
> >>>>>>> +	if (!connector)
> >>>>>>> +		return false;
> >>>>>> This actually changes the is_hdmi2_sink value for sil-sii8620.
> >>>>> Hmm. No idea why they would have set that to true when everyone else is
> >>>>> passing false. 
> >>>> Because false does not work :) More precisely MHLv3 (used in Sii8620)
> >>>> uses CTA-861-F standard for infoframes, which is specific to HDMI2.0.
> >>>>
> >>>> Unfortunately I have no access to MHL specs, but my experiments and
> >>>> vendor drivers strongly suggests it is done this way.
> >>>>
> >>>> This is important in case of 4K modes which are handled differently by
> >>>> HDMI 1.4 and HDMI2.0.
> >>> HDMI 2.0 handles 4k just like 1.4 handled it when you use one of
> >>> the 4k modes defined in 1.4. Only if you use features beyond 1.4 do we
> >>> switch over to the HDMI 2.0 specific signalling.
> >>
> >> The difference is in infoframes:
> >>
> >> HDMI 1.4 sets AVI infoframe VIC to 0, and sends HDMI_VIC in VSI.
> >>
> >> HDMI 2.0 sets AVI infoframe to non zero VICs introduced by
> >> HDMI2.0/CEA-861-F, VSI can be omitted if I remember correctly, unless 3d
> >> is in use.
> > Like I said, The HDMI 1.4 method is used even with HDMI 2.0 sinks unless
> > some feature gets used which can't be signalled via the HDMI 1.4 vendor
> > specific infoframe.
> 
> 
> Do you mean that 4K VICs 95, 94, 93, 98 defined in CEA-861-F are not
> used at all for non-3d video in HDMI 2.0?
> 
> Chapter 10.1 of HDMI2.0 spec says clearly:
> 
> > When transmitting any additional Video Format for which a VIC value
> > has been defined in
> > CEA-861-F tables 1, 2, and 3, an HDMI Source shall set the VIC field
> > to the Video Code for
> > that format.
> 
> 
> It contradicts your statement, or am I missing something?

IIRC the spec said you can only use either one or the other.

Section 10.2 doesn't explicity say that the AVI VIC must be zero when
HDMI_VIC is non-zero, but it does more or less imply it. And looks
like appendix E outright forbids having both as non-zero.

> 
> 
> >
> >>
> >> So setting VICs to non-zero in case of HDMI1.4 sinks and 4k modes seems
> >> risky.
> > That is not what I was proposing.
> 
> 
> Maybe I have misread your statement:
> 
> On 21.11.2018 12:51, Ville Syrjälä wrote:
> 
> > That said, I was actually thinking of removing this hdmi2 vs. not
> > stuff from drm_hdmi_avi_infoframe_from_display_mode(). We added it
> > "just in case", but we already have a similar issue with earlier
> > cea-861 revisions and haven't seen any bugs where an older monitor
> > would get confused by a VIC not yet defined when the monitor was
> > produced.
> 
> 
> I do not know what it could mean other that allowing sending VICs
> unknown to sink?

Yes it might send unknown VICs to sinks. But that happens anyway becasue
we can't tell which VICs the sink supports. We can only try to deduce
the spec version indirectly which is not very robust. And we only do that
for CTA-861-F, earlier revisions of the spec we don't try to deduce at all.
And alternative might be to send VICs only for those modes that the sink
actually declared in the EDID, but that opens up the question of whether
we should only do that for modes that have an SVD, or whether we should
do it for modes we get by other means (eg. detailed timing descriptors).

What I wasn't suggesting was to send AVI VIC != 0 + HDMI_VIC != 0
because that is not legal.

> 
> Maybe better would be just to wait for the patch to avoid misunderstandings.
> 
> 
> Regards
> 
> Andrzej
> 
> 
> >
> >>
> >> Regards
> >>
> >> Andrzej
> >>
> >>
> >>>> The pipeline looks like (in parenthesis HDMI version on the stream):
> >>>>
> >>>> exynos_hdmi --(1.4)--> SII8620 --(2.0)--> MHL_dongle --(1.4)--> TV
> >>>>
> >>>>
> >>>>> I guess I can change this to true to not change it. IIRC
> >>>>> that was the only driver that didn't have a connector around.
> >>>>>
> >>>>> That said, I was actually thinking of removing this hdmi2 vs. not
> >>>>> stuff from drm_hdmi_avi_infoframe_from_display_mode(). We added it
> >>>>> "just in case", but we already have a similar issue with earlier
> >>>>> cea-861 revisions and haven't seen any bugs where an older monitor
> >>>>> would get confused by a VIC not yet defined when the monitor was
> >>>>> produced.
> >>>>>
> >>>> Are you sure this is a good idea? As I said earlier 4K modes are present
> >>>> in HDMI 1.4 and 2.0 but they are handled differently, so this is not
> >>>> only matter of unknown VIC in AVIF.
> >>>>
> >>>>
> >>>> Regards
> >>>>
> >>>> Andrzej
> 

-- 
Ville Syrjälä
Intel
_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Laurent Pinchart]

Hi Ville,

Thank you for the patch.

On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Make life easier for drivers by simply passing the connector
> to drm_hdmi_avi_infoframe_from_display_mode() and
> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
> need to worry about is_hdmi2_sink mess.

While this is good for display controller drivers, the change isn't great for 
bridge drivers. Down the road we're looking at moving connector support out of 
the bridge drivers. Adding an additional dependency to connectors in the 
bridges will make that more difficult. Ideally bridges should retrieve the 
information from their sink, regardless of whether it is a connector or 
another bridge.

Please see below for an additional comment.

> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> Cc: Archit Taneja <architt@codeaurora.org>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> Cc: Inki Dae <inki.dae@samsung.com>
> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> Cc: Russell King <linux@armlinux.org.uk>
> Cc: CK Hu <ck.hu@mediatek.com>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Ben Skeggs <bskeggs@redhat.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Sandy Huang <hjc@rock-chips.com>
> Cc: "Heiko Stübner" <heiko@sntech.de>
> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Cc: Vincent Abriou <vincent.abriou@st.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Eric Anholt <eric@anholt.net>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> Cc: amd-gfx@lists.freedesktop.org
> Cc: linux-arm-msm@vger.kernel.org
> Cc: freedreno@lists.freedesktop.org
> Cc: nouveau@lists.freedesktop.org
> Cc: linux-tegra@vger.kernel.org
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  2 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  2 +-
>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  3 ++-
>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  2 +-
>  drivers/gpu/drm/bridge/analogix-anx78xx.c |  5 ++--
>  drivers/gpu/drm/bridge/sii902x.c          |  3 ++-
>  drivers/gpu/drm/bridge/sil-sii8620.c      |  3 +--
>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  3 ++-
>  drivers/gpu/drm/drm_edid.c                | 33 ++++++++++++++---------
>  drivers/gpu/drm/exynos/exynos_hdmi.c      |  3 ++-
>  drivers/gpu/drm/i2c/tda998x_drv.c         |  3 ++-
>  drivers/gpu/drm/i915/intel_hdmi.c         | 14 +++++-----
>  drivers/gpu/drm/i915/intel_lspcon.c       | 15 ++++++-----
>  drivers/gpu/drm/i915/intel_sdvo.c         | 10 ++++---
>  drivers/gpu/drm/mediatek/mtk_hdmi.c       |  3 ++-
>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |  3 ++-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c   |  7 +++--
>  drivers/gpu/drm/omapdrm/omap_encoder.c    |  5 ++--
>  drivers/gpu/drm/radeon/radeon_audio.c     |  2 +-
>  drivers/gpu/drm/rockchip/inno_hdmi.c      |  4 ++-
>  drivers/gpu/drm/sti/sti_hdmi.c            |  3 ++-
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |  3 ++-
>  drivers/gpu/drm/tegra/hdmi.c              |  3 ++-
>  drivers/gpu/drm/tegra/sor.c               |  3 ++-
>  drivers/gpu/drm/vc4/vc4_hdmi.c            | 11 +++++---
>  drivers/gpu/drm/zte/zx_hdmi.c             |  4 ++-
>  include/drm/drm_edid.h                    |  8 +++---
>  27 files changed, 94 insertions(+), 66 deletions(-)

For dw-hdmi and omapdrm,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart



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

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Andrzej Hajda]

On 21.11.2018 19:19, Laurent Pinchart wrote:
> Hi Ville,
>
> Thank you for the patch.
>
> On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> Make life easier for drivers by simply passing the connector
>> to drm_hdmi_avi_infoframe_from_display_mode() and
>> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
>> need to worry about is_hdmi2_sink mess.
> While this is good for display controller drivers, the change isn't great for 
> bridge drivers. Down the road we're looking at moving connector support out of 
> the bridge drivers. Adding an additional dependency to connectors in the 
> bridges will make that more difficult. Ideally bridges should retrieve the 
> information from their sink, regardless of whether it is a connector or 
> another bridge.


I agree with it, and case of sii8620 shows that there are cases where
bridge has no direct access to the connector.

On the other side,  since you are passing connector to
drm_hdmi_avi_infoframe_from_display_mode(), you could drop mode
parameter and rename the function to
drm_hdmi_avi_infoframe_from_connector() then, unless mode passed and
mode set on the connector differs?


Regards

Andrzej


>
> Please see below for an additional comment.
>
>> Cc: Alex Deucher <alexander.deucher@amd.com>
>> Cc: "Christian König" <christian.koenig@amd.com>
>> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
>> Cc: Archit Taneja <architt@codeaurora.org>
>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
>> Cc: Inki Dae <inki.dae@samsung.com>
>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>> Cc: Russell King <linux@armlinux.org.uk>
>> Cc: CK Hu <ck.hu@mediatek.com>
>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>> Cc: Rob Clark <robdclark@gmail.com>
>> Cc: Ben Skeggs <bskeggs@redhat.com>
>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> Cc: Sandy Huang <hjc@rock-chips.com>
>> Cc: "Heiko Stübner" <heiko@sntech.de>
>> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>> Cc: Vincent Abriou <vincent.abriou@st.com>
>> Cc: Thierry Reding <thierry.reding@gmail.com>
>> Cc: Eric Anholt <eric@anholt.net>
>> Cc: Shawn Guo <shawnguo@kernel.org>
>> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
>> Cc: amd-gfx@lists.freedesktop.org
>> Cc: linux-arm-msm@vger.kernel.org
>> Cc: freedreno@lists.freedesktop.org
>> Cc: nouveau@lists.freedesktop.org
>> Cc: linux-tegra@vger.kernel.org
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  2 +-
>>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  2 +-
>>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  3 ++-
>>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  2 +-
>>  drivers/gpu/drm/bridge/analogix-anx78xx.c |  5 ++--
>>  drivers/gpu/drm/bridge/sii902x.c          |  3 ++-
>>  drivers/gpu/drm/bridge/sil-sii8620.c      |  3 +--
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  3 ++-
>>  drivers/gpu/drm/drm_edid.c                | 33 ++++++++++++++---------
>>  drivers/gpu/drm/exynos/exynos_hdmi.c      |  3 ++-
>>  drivers/gpu/drm/i2c/tda998x_drv.c         |  3 ++-
>>  drivers/gpu/drm/i915/intel_hdmi.c         | 14 +++++-----
>>  drivers/gpu/drm/i915/intel_lspcon.c       | 15 ++++++-----
>>  drivers/gpu/drm/i915/intel_sdvo.c         | 10 ++++---
>>  drivers/gpu/drm/mediatek/mtk_hdmi.c       |  3 ++-
>>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |  3 ++-
>>  drivers/gpu/drm/nouveau/dispnv50/disp.c   |  7 +++--
>>  drivers/gpu/drm/omapdrm/omap_encoder.c    |  5 ++--
>>  drivers/gpu/drm/radeon/radeon_audio.c     |  2 +-
>>  drivers/gpu/drm/rockchip/inno_hdmi.c      |  4 ++-
>>  drivers/gpu/drm/sti/sti_hdmi.c            |  3 ++-
>>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |  3 ++-
>>  drivers/gpu/drm/tegra/hdmi.c              |  3 ++-
>>  drivers/gpu/drm/tegra/sor.c               |  3 ++-
>>  drivers/gpu/drm/vc4/vc4_hdmi.c            | 11 +++++---
>>  drivers/gpu/drm/zte/zx_hdmi.c             |  4 ++-
>>  include/drm/drm_edid.h                    |  8 +++---
>>  27 files changed, 94 insertions(+), 66 deletions(-)
> For dw-hdmi and omapdrm,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>

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

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Ville Syrjälä]

On Thu, Nov 29, 2018 at 10:08:07AM +0100, Andrzej Hajda wrote:
> On 21.11.2018 19:19, Laurent Pinchart wrote:
> > Hi Ville,
> >
> > Thank you for the patch.
> >
> > On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote:
> >> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >> Make life easier for drivers by simply passing the connector
> >> to drm_hdmi_avi_infoframe_from_display_mode() and
> >> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
> >> need to worry about is_hdmi2_sink mess.
> > While this is good for display controller drivers, the change isn't great for 
> > bridge drivers. Down the road we're looking at moving connector support out of 
> > the bridge drivers. Adding an additional dependency to connectors in the 
> > bridges will make that more difficult. Ideally bridges should retrieve the 
> > information from their sink, regardless of whether it is a connector or 
> > another bridge.
> 
> 
> I agree with it, and case of sii8620 shows that there are cases where
> bridge has no direct access to the connector.

It's just a matter of plumbing it through.

> 
> On the other side,  since you are passing connector to
> drm_hdmi_avi_infoframe_from_display_mode(), you could drop mode
> parameter and rename the function to
> drm_hdmi_avi_infoframe_from_connector() then, unless mode passed and
> mode set on the connector differs?

Connectors don't have a mode.

> 
> 
> Regards
> 
> Andrzej
> 
> 
> >
> > Please see below for an additional comment.
> >
> >> Cc: Alex Deucher <alexander.deucher@amd.com>
> >> Cc: "Christian König" <christian.koenig@amd.com>
> >> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> >> Cc: Archit Taneja <architt@codeaurora.org>
> >> Cc: Andrzej Hajda <a.hajda@samsung.com>
> >> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> >> Cc: Inki Dae <inki.dae@samsung.com>
> >> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> >> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> >> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> >> Cc: Russell King <linux@armlinux.org.uk>
> >> Cc: CK Hu <ck.hu@mediatek.com>
> >> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> >> Cc: Rob Clark <robdclark@gmail.com>
> >> Cc: Ben Skeggs <bskeggs@redhat.com>
> >> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> >> Cc: Sandy Huang <hjc@rock-chips.com>
> >> Cc: "Heiko Stübner" <heiko@sntech.de>
> >> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> >> Cc: Vincent Abriou <vincent.abriou@st.com>
> >> Cc: Thierry Reding <thierry.reding@gmail.com>
> >> Cc: Eric Anholt <eric@anholt.net>
> >> Cc: Shawn Guo <shawnguo@kernel.org>
> >> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> >> Cc: amd-gfx@lists.freedesktop.org
> >> Cc: linux-arm-msm@vger.kernel.org
> >> Cc: freedreno@lists.freedesktop.org
> >> Cc: nouveau@lists.freedesktop.org
> >> Cc: linux-tegra@vger.kernel.org
> >> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> ---
> >>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  2 +-
> >>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  2 +-
> >>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  3 ++-
> >>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  2 +-
> >>  drivers/gpu/drm/bridge/analogix-anx78xx.c |  5 ++--
> >>  drivers/gpu/drm/bridge/sii902x.c          |  3 ++-
> >>  drivers/gpu/drm/bridge/sil-sii8620.c      |  3 +--
> >>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  3 ++-
> >>  drivers/gpu/drm/drm_edid.c                | 33 ++++++++++++++---------
> >>  drivers/gpu/drm/exynos/exynos_hdmi.c      |  3 ++-
> >>  drivers/gpu/drm/i2c/tda998x_drv.c         |  3 ++-
> >>  drivers/gpu/drm/i915/intel_hdmi.c         | 14 +++++-----
> >>  drivers/gpu/drm/i915/intel_lspcon.c       | 15 ++++++-----
> >>  drivers/gpu/drm/i915/intel_sdvo.c         | 10 ++++---
> >>  drivers/gpu/drm/mediatek/mtk_hdmi.c       |  3 ++-
> >>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |  3 ++-
> >>  drivers/gpu/drm/nouveau/dispnv50/disp.c   |  7 +++--
> >>  drivers/gpu/drm/omapdrm/omap_encoder.c    |  5 ++--
> >>  drivers/gpu/drm/radeon/radeon_audio.c     |  2 +-
> >>  drivers/gpu/drm/rockchip/inno_hdmi.c      |  4 ++-
> >>  drivers/gpu/drm/sti/sti_hdmi.c            |  3 ++-
> >>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |  3 ++-
> >>  drivers/gpu/drm/tegra/hdmi.c              |  3 ++-
> >>  drivers/gpu/drm/tegra/sor.c               |  3 ++-
> >>  drivers/gpu/drm/vc4/vc4_hdmi.c            | 11 +++++---
> >>  drivers/gpu/drm/zte/zx_hdmi.c             |  4 ++-
> >>  include/drm/drm_edid.h                    |  8 +++---
> >>  27 files changed, 94 insertions(+), 66 deletions(-)
> > For dw-hdmi and omapdrm,
> >
> > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >

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

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Andrzej Hajda]

On 03.12.2018 22:38, Ville Syrjälä wrote:
> On Thu, Nov 29, 2018 at 10:08:07AM +0100, Andrzej Hajda wrote:
>> On 21.11.2018 19:19, Laurent Pinchart wrote:
>>> Hi Ville,
>>>
>>> Thank you for the patch.
>>>
>>> On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote:
>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>
>>>> Make life easier for drivers by simply passing the connector
>>>> to drm_hdmi_avi_infoframe_from_display_mode() and
>>>> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
>>>> need to worry about is_hdmi2_sink mess.
>>> While this is good for display controller drivers, the change isn't great for 
>>> bridge drivers. Down the road we're looking at moving connector support out of 
>>> the bridge drivers. Adding an additional dependency to connectors in the 
>>> bridges will make that more difficult. Ideally bridges should retrieve the 
>>> information from their sink, regardless of whether it is a connector or 
>>> another bridge.
>>
>> I agree with it, and case of sii8620 shows that there are cases where
>> bridge has no direct access to the connector.
> It's just a matter of plumbing it through.


What do you mean exactly?


>
>> On the other side,  since you are passing connector to
>> drm_hdmi_avi_infoframe_from_display_mode(), you could drop mode
>> parameter and rename the function to
>> drm_hdmi_avi_infoframe_from_connector() then, unless mode passed and
>> mode set on the connector differs?
> Connectors don't have a mode.


As they are passing video stream they should have it, even if not
directly, for example:

connector->state->crtc->mode

In moment of creating infoframe it should be set properly.


Regards

Andrzej


>
>>
>> Regards
>>
>> Andrzej
>>
>>
>>> Please see below for an additional comment.
>>>
>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>>> Cc: "Christian König" <christian.koenig@amd.com>
>>>> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
>>>> Cc: Archit Taneja <architt@codeaurora.org>
>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>>>> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
>>>> Cc: Inki Dae <inki.dae@samsung.com>
>>>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
>>>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
>>>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>>>> Cc: Russell King <linux@armlinux.org.uk>
>>>> Cc: CK Hu <ck.hu@mediatek.com>
>>>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>>>> Cc: Rob Clark <robdclark@gmail.com>
>>>> Cc: Ben Skeggs <bskeggs@redhat.com>
>>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>>> Cc: Sandy Huang <hjc@rock-chips.com>
>>>> Cc: "Heiko Stübner" <heiko@sntech.de>
>>>> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>>>> Cc: Vincent Abriou <vincent.abriou@st.com>
>>>> Cc: Thierry Reding <thierry.reding@gmail.com>
>>>> Cc: Eric Anholt <eric@anholt.net>
>>>> Cc: Shawn Guo <shawnguo@kernel.org>
>>>> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
>>>> Cc: amd-gfx@lists.freedesktop.org
>>>> Cc: linux-arm-msm@vger.kernel.org
>>>> Cc: freedreno@lists.freedesktop.org
>>>> Cc: nouveau@lists.freedesktop.org
>>>> Cc: linux-tegra@vger.kernel.org
>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>> ---
>>>>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  2 +-
>>>>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  2 +-
>>>>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  3 ++-
>>>>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  2 +-
>>>>  drivers/gpu/drm/bridge/analogix-anx78xx.c |  5 ++--
>>>>  drivers/gpu/drm/bridge/sii902x.c          |  3 ++-
>>>>  drivers/gpu/drm/bridge/sil-sii8620.c      |  3 +--
>>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  3 ++-
>>>>  drivers/gpu/drm/drm_edid.c                | 33 ++++++++++++++---------
>>>>  drivers/gpu/drm/exynos/exynos_hdmi.c      |  3 ++-
>>>>  drivers/gpu/drm/i2c/tda998x_drv.c         |  3 ++-
>>>>  drivers/gpu/drm/i915/intel_hdmi.c         | 14 +++++-----
>>>>  drivers/gpu/drm/i915/intel_lspcon.c       | 15 ++++++-----
>>>>  drivers/gpu/drm/i915/intel_sdvo.c         | 10 ++++---
>>>>  drivers/gpu/drm/mediatek/mtk_hdmi.c       |  3 ++-
>>>>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |  3 ++-
>>>>  drivers/gpu/drm/nouveau/dispnv50/disp.c   |  7 +++--
>>>>  drivers/gpu/drm/omapdrm/omap_encoder.c    |  5 ++--
>>>>  drivers/gpu/drm/radeon/radeon_audio.c     |  2 +-
>>>>  drivers/gpu/drm/rockchip/inno_hdmi.c      |  4 ++-
>>>>  drivers/gpu/drm/sti/sti_hdmi.c            |  3 ++-
>>>>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |  3 ++-
>>>>  drivers/gpu/drm/tegra/hdmi.c              |  3 ++-
>>>>  drivers/gpu/drm/tegra/sor.c               |  3 ++-
>>>>  drivers/gpu/drm/vc4/vc4_hdmi.c            | 11 +++++---
>>>>  drivers/gpu/drm/zte/zx_hdmi.c             |  4 ++-
>>>>  include/drm/drm_edid.h                    |  8 +++---
>>>>  27 files changed, 94 insertions(+), 66 deletions(-)
>>> For dw-hdmi and omapdrm,
>>>
>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>>>

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

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Ville Syrjälä]

On Tue, Dec 04, 2018 at 08:46:53AM +0100, Andrzej Hajda wrote:
> On 03.12.2018 22:38, Ville Syrjälä wrote:
> > On Thu, Nov 29, 2018 at 10:08:07AM +0100, Andrzej Hajda wrote:
> >> On 21.11.2018 19:19, Laurent Pinchart wrote:
> >>> Hi Ville,
> >>>
> >>> Thank you for the patch.
> >>>
> >>> On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote:
> >>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>
> >>>> Make life easier for drivers by simply passing the connector
> >>>> to drm_hdmi_avi_infoframe_from_display_mode() and
> >>>> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
> >>>> need to worry about is_hdmi2_sink mess.
> >>> While this is good for display controller drivers, the change isn't great for 
> >>> bridge drivers. Down the road we're looking at moving connector support out of 
> >>> the bridge drivers. Adding an additional dependency to connectors in the 
> >>> bridges will make that more difficult. Ideally bridges should retrieve the 
> >>> information from their sink, regardless of whether it is a connector or 
> >>> another bridge.
> >>
> >> I agree with it, and case of sii8620 shows that there are cases where
> >> bridge has no direct access to the connector.
> > It's just a matter of plumbing it through.
> 
> 
> What do you mean exactly?

void bridge_foo(...
+               ,struct drm_connector *connector);

> 
> 
> >
> >> On the other side,  since you are passing connector to
> >> drm_hdmi_avi_infoframe_from_display_mode(), you could drop mode
> >> parameter and rename the function to
> >> drm_hdmi_avi_infoframe_from_connector() then, unless mode passed and
> >> mode set on the connector differs?
> > Connectors don't have a mode.
> 
> 
> As they are passing video stream they should have it, even if not
> directly, for example:
> 
> connector->state->crtc->mode

That's not really how atomic works. One shouldn't go digging
through the obj->state pointers when we're not holding the
relevant locks anymore. The atomic way would be to pass either
both crtc state and connector state, or drm_atomic_state +
crtc/connector.

> 
> In moment of creating infoframe it should be set properly.
> 
> 
> Regards
> 
> Andrzej
> 
> 
> >
> >>
> >> Regards
> >>
> >> Andrzej
> >>
> >>
> >>> Please see below for an additional comment.
> >>>
> >>>> Cc: Alex Deucher <alexander.deucher@amd.com>
> >>>> Cc: "Christian König" <christian.koenig@amd.com>
> >>>> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> >>>> Cc: Archit Taneja <architt@codeaurora.org>
> >>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
> >>>> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> >>>> Cc: Inki Dae <inki.dae@samsung.com>
> >>>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> >>>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> >>>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> >>>> Cc: Russell King <linux@armlinux.org.uk>
> >>>> Cc: CK Hu <ck.hu@mediatek.com>
> >>>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> >>>> Cc: Rob Clark <robdclark@gmail.com>
> >>>> Cc: Ben Skeggs <bskeggs@redhat.com>
> >>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> >>>> Cc: Sandy Huang <hjc@rock-chips.com>
> >>>> Cc: "Heiko Stübner" <heiko@sntech.de>
> >>>> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> >>>> Cc: Vincent Abriou <vincent.abriou@st.com>
> >>>> Cc: Thierry Reding <thierry.reding@gmail.com>
> >>>> Cc: Eric Anholt <eric@anholt.net>
> >>>> Cc: Shawn Guo <shawnguo@kernel.org>
> >>>> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> >>>> Cc: amd-gfx@lists.freedesktop.org
> >>>> Cc: linux-arm-msm@vger.kernel.org
> >>>> Cc: freedreno@lists.freedesktop.org
> >>>> Cc: nouveau@lists.freedesktop.org
> >>>> Cc: linux-tegra@vger.kernel.org
> >>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>> ---
> >>>>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  2 +-
> >>>>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  2 +-
> >>>>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  3 ++-
> >>>>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  2 +-
> >>>>  drivers/gpu/drm/bridge/analogix-anx78xx.c |  5 ++--
> >>>>  drivers/gpu/drm/bridge/sii902x.c          |  3 ++-
> >>>>  drivers/gpu/drm/bridge/sil-sii8620.c      |  3 +--
> >>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  3 ++-
> >>>>  drivers/gpu/drm/drm_edid.c                | 33 ++++++++++++++---------
> >>>>  drivers/gpu/drm/exynos/exynos_hdmi.c      |  3 ++-
> >>>>  drivers/gpu/drm/i2c/tda998x_drv.c         |  3 ++-
> >>>>  drivers/gpu/drm/i915/intel_hdmi.c         | 14 +++++-----
> >>>>  drivers/gpu/drm/i915/intel_lspcon.c       | 15 ++++++-----
> >>>>  drivers/gpu/drm/i915/intel_sdvo.c         | 10 ++++---
> >>>>  drivers/gpu/drm/mediatek/mtk_hdmi.c       |  3 ++-
> >>>>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |  3 ++-
> >>>>  drivers/gpu/drm/nouveau/dispnv50/disp.c   |  7 +++--
> >>>>  drivers/gpu/drm/omapdrm/omap_encoder.c    |  5 ++--
> >>>>  drivers/gpu/drm/radeon/radeon_audio.c     |  2 +-
> >>>>  drivers/gpu/drm/rockchip/inno_hdmi.c      |  4 ++-
> >>>>  drivers/gpu/drm/sti/sti_hdmi.c            |  3 ++-
> >>>>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |  3 ++-
> >>>>  drivers/gpu/drm/tegra/hdmi.c              |  3 ++-
> >>>>  drivers/gpu/drm/tegra/sor.c               |  3 ++-
> >>>>  drivers/gpu/drm/vc4/vc4_hdmi.c            | 11 +++++---
> >>>>  drivers/gpu/drm/zte/zx_hdmi.c             |  4 ++-
> >>>>  include/drm/drm_edid.h                    |  8 +++---
> >>>>  27 files changed, 94 insertions(+), 66 deletions(-)
> >>> For dw-hdmi and omapdrm,
> >>>
> >>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> >>>

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

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Laurent Pinchart]

Hi Ville,

On Tuesday, 4 December 2018 21:13:20 EET Ville Syrjälä wrote:
> On Tue, Dec 04, 2018 at 08:46:53AM +0100, Andrzej Hajda wrote:
> > On 03.12.2018 22:38, Ville Syrjälä wrote:
> >> On Thu, Nov 29, 2018 at 10:08:07AM +0100, Andrzej Hajda wrote:
> >>> On 21.11.2018 19:19, Laurent Pinchart wrote:
> >>>> On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote:
> >>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>> 
> >>>>> Make life easier for drivers by simply passing the connector
> >>>>> to drm_hdmi_avi_infoframe_from_display_mode() and
> >>>>> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
> >>>>> need to worry about is_hdmi2_sink mess.
> >>>> 
> >>>> While this is good for display controller drivers, the change isn't
> >>>> great for bridge drivers. Down the road we're looking at moving
> >>>> connector support out of the bridge drivers. Adding an additional
> >>>> dependency to connectors in the bridges will make that more
> >>>> difficult. Ideally bridges should retrieve the information from their
> >>>> sink, regardless of whether it is a connector or another bridge.
> >>> 
> >>> I agree with it, and case of sii8620 shows that there are cases where
> >>> bridge has no direct access to the connector.
> >> 
> >> It's just a matter of plumbing it through.
> > 
> > What do you mean exactly?
> 
> void bridge_foo(...
> +               ,struct drm_connector *connector);
> 
> >>> On the other side,  since you are passing connector to
> >>> drm_hdmi_avi_infoframe_from_display_mode(), you could drop mode
> >>> parameter and rename the function to
> >>> drm_hdmi_avi_infoframe_from_connector() then, unless mode passed and
> >>> mode set on the connector differs?
> >> 
> >> Connectors don't have a mode.
> > 
> > As they are passing video stream they should have it, even if not
> > directly, for example:
> > 
> > connector->state->crtc->mode
> 
> That's not really how atomic works. One shouldn't go digging
> through the obj->state pointers when we're not holding the
> relevant locks anymore. The atomic way would be to pass either
> both crtc state and connector state, or drm_atomic_state +
> crtc/connector.

Or a bridge state ? With chained bridges the mode can vary along the pipeline, 
the CRTC adjusted mode will only cover the link between the CRTC and the first 
bridge. It's only a matter of time until we need to store other intermediate 
modes in states. I'd rather prepare for that instead of passing the CRTC state 
to bridges.

> > In moment of creating infoframe it should be set properly.
> > 
> >>>> Please see below for an additional comment.
> >>>> 
> >>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
> >>>>> Cc: "Christian König" <christian.koenig@amd.com>
> >>>>> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> >>>>> Cc: Archit Taneja <architt@codeaurora.org>
> >>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
> >>>>> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> >>>>> Cc: Inki Dae <inki.dae@samsung.com>
> >>>>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> >>>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> >>>>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> >>>>> Cc: Russell King <linux@armlinux.org.uk>
> >>>>> Cc: CK Hu <ck.hu@mediatek.com>
> >>>>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> >>>>> Cc: Rob Clark <robdclark@gmail.com>
> >>>>> Cc: Ben Skeggs <bskeggs@redhat.com>
> >>>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> >>>>> Cc: Sandy Huang <hjc@rock-chips.com>
> >>>>> Cc: "Heiko Stübner" <heiko@sntech.de>
> >>>>> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> >>>>> Cc: Vincent Abriou <vincent.abriou@st.com>
> >>>>> Cc: Thierry Reding <thierry.reding@gmail.com>
> >>>>> Cc: Eric Anholt <eric@anholt.net>
> >>>>> Cc: Shawn Guo <shawnguo@kernel.org>
> >>>>> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> >>>>> Cc: amd-gfx@lists.freedesktop.org
> >>>>> Cc: linux-arm-msm@vger.kernel.org
> >>>>> Cc: freedreno@lists.freedesktop.org
> >>>>> Cc: nouveau@lists.freedesktop.org
> >>>>> Cc: linux-tegra@vger.kernel.org
> >>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>> ---
> >>>>> 
> >>>>>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  2 +-
> >>>>>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  2 +-
> >>>>>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  3 ++-
> >>>>>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  2 +-
> >>>>>  drivers/gpu/drm/bridge/analogix-anx78xx.c |  5 ++--
> >>>>>  drivers/gpu/drm/bridge/sii902x.c          |  3 ++-
> >>>>>  drivers/gpu/drm/bridge/sil-sii8620.c      |  3 +--
> >>>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  3 ++-
> >>>>>  drivers/gpu/drm/drm_edid.c                | 33 ++++++++++++--------
> >>>>>  drivers/gpu/drm/exynos/exynos_hdmi.c      |  3 ++-
> >>>>>  drivers/gpu/drm/i2c/tda998x_drv.c         |  3 ++-
> >>>>>  drivers/gpu/drm/i915/intel_hdmi.c         | 14 +++++-----
> >>>>>  drivers/gpu/drm/i915/intel_lspcon.c       | 15 ++++++-----
> >>>>>  drivers/gpu/drm/i915/intel_sdvo.c         | 10 ++++---
> >>>>>  drivers/gpu/drm/mediatek/mtk_hdmi.c       |  3 ++-
> >>>>>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |  3 ++-
> >>>>>  drivers/gpu/drm/nouveau/dispnv50/disp.c   |  7 +++--
> >>>>>  drivers/gpu/drm/omapdrm/omap_encoder.c    |  5 ++--
> >>>>>  drivers/gpu/drm/radeon/radeon_audio.c     |  2 +-
> >>>>>  drivers/gpu/drm/rockchip/inno_hdmi.c      |  4 ++-
> >>>>>  drivers/gpu/drm/sti/sti_hdmi.c            |  3 ++-
> >>>>>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |  3 ++-
> >>>>>  drivers/gpu/drm/tegra/hdmi.c              |  3 ++-
> >>>>>  drivers/gpu/drm/tegra/sor.c               |  3 ++-
> >>>>>  drivers/gpu/drm/vc4/vc4_hdmi.c            | 11 +++++---
> >>>>>  drivers/gpu/drm/zte/zx_hdmi.c             |  4 ++-
> >>>>>  include/drm/drm_edid.h                    |  8 +++---
> >>>>>  27 files changed, 94 insertions(+), 66 deletions(-)
> >>>> 
> >>>> For dw-hdmi and omapdrm,
> >>>> 
> >>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

-- 
Regards,

Laurent Pinchart



_______________________________________________
Freedreno mailing list
Freedreno@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/freedreno

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Andrzej Hajda]

On 05.12.2018 07:32, Laurent Pinchart wrote:
> Hi Ville,
>
> On Tuesday, 4 December 2018 21:13:20 EET Ville Syrjälä wrote:
>> On Tue, Dec 04, 2018 at 08:46:53AM +0100, Andrzej Hajda wrote:
>>> On 03.12.2018 22:38, Ville Syrjälä wrote:
>>>> On Thu, Nov 29, 2018 at 10:08:07AM +0100, Andrzej Hajda wrote:
>>>>> On 21.11.2018 19:19, Laurent Pinchart wrote:
>>>>>> On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote:
>>>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>>>
>>>>>>> Make life easier for drivers by simply passing the connector
>>>>>>> to drm_hdmi_avi_infoframe_from_display_mode() and
>>>>>>> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
>>>>>>> need to worry about is_hdmi2_sink mess.
>>>>>> While this is good for display controller drivers, the change isn't
>>>>>> great for bridge drivers. Down the road we're looking at moving
>>>>>> connector support out of the bridge drivers. Adding an additional
>>>>>> dependency to connectors in the bridges will make that more
>>>>>> difficult. Ideally bridges should retrieve the information from their
>>>>>> sink, regardless of whether it is a connector or another bridge.
>>>>> I agree with it, and case of sii8620 shows that there are cases where
>>>>> bridge has no direct access to the connector.
>>>> It's just a matter of plumbing it through.
>>> What do you mean exactly?
>> void bridge_foo(...
>> +               ,struct drm_connector *connector);
>>
>>>>> On the other side,  since you are passing connector to
>>>>> drm_hdmi_avi_infoframe_from_display_mode(), you could drop mode
>>>>> parameter and rename the function to
>>>>> drm_hdmi_avi_infoframe_from_connector() then, unless mode passed and
>>>>> mode set on the connector differs?
>>>> Connectors don't have a mode.
>>> As they are passing video stream they should have it, even if not
>>> directly, for example:
>>>
>>> connector->state->crtc->mode
>> That's not really how atomic works. One shouldn't go digging
>> through the obj->state pointers when we're not holding the
>> relevant locks anymore. The atomic way would be to pass either
>> both crtc state and connector state, or drm_atomic_state +
>> crtc/connector.


Usually infoframe contents is generated in modesetting/enable callbacks
so the locks should be in place.

And the locks should be hold for
drm_hdmi_avi_infoframe_from_display_mode as well - it wouldn't be correct if

generated infoframe is not relevant to actual video mode. I guess that
if connector->state->crtc->mode

differs from mode passed to drm_hdmi_avi_infoframe_from_display_mode it
is a sign of possible problem.

And if they do not differ passing mode as an extra argument is redundant.


> Or a bridge state ? With chained bridges the mode can vary along the pipeline, 
> the CRTC adjusted mode will only cover the link between the CRTC and the first 
> bridge. It's only a matter of time until we need to store other intermediate 
> modes in states. I'd rather prepare for that instead of passing the CRTC state 
> to bridges.


Yes, optional bridge states seems reasonable, volunteers needed :)


Regards

Andrzej


>
>>> In moment of creating infoframe it should be set properly.
>>>
>>>>>> Please see below for an additional comment.
>>>>>>
>>>>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
>>>>>>> Cc: "Christian König" <christian.koenig@amd.com>
>>>>>>> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
>>>>>>> Cc: Archit Taneja <architt@codeaurora.org>
>>>>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>>>>>>> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
>>>>>>> Cc: Inki Dae <inki.dae@samsung.com>
>>>>>>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
>>>>>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
>>>>>>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
>>>>>>> Cc: Russell King <linux@armlinux.org.uk>
>>>>>>> Cc: CK Hu <ck.hu@mediatek.com>
>>>>>>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
>>>>>>> Cc: Rob Clark <robdclark@gmail.com>
>>>>>>> Cc: Ben Skeggs <bskeggs@redhat.com>
>>>>>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>>>>>> Cc: Sandy Huang <hjc@rock-chips.com>
>>>>>>> Cc: "Heiko Stübner" <heiko@sntech.de>
>>>>>>> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
>>>>>>> Cc: Vincent Abriou <vincent.abriou@st.com>
>>>>>>> Cc: Thierry Reding <thierry.reding@gmail.com>
>>>>>>> Cc: Eric Anholt <eric@anholt.net>
>>>>>>> Cc: Shawn Guo <shawnguo@kernel.org>
>>>>>>> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
>>>>>>> Cc: amd-gfx@lists.freedesktop.org
>>>>>>> Cc: linux-arm-msm@vger.kernel.org
>>>>>>> Cc: freedreno@lists.freedesktop.org
>>>>>>> Cc: nouveau@lists.freedesktop.org
>>>>>>> Cc: linux-tegra@vger.kernel.org
>>>>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>>>>> ---
>>>>>>>
>>>>>>>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  2 +-
>>>>>>>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  2 +-
>>>>>>>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  3 ++-
>>>>>>>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  2 +-
>>>>>>>  drivers/gpu/drm/bridge/analogix-anx78xx.c |  5 ++--
>>>>>>>  drivers/gpu/drm/bridge/sii902x.c          |  3 ++-
>>>>>>>  drivers/gpu/drm/bridge/sil-sii8620.c      |  3 +--
>>>>>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  3 ++-
>>>>>>>  drivers/gpu/drm/drm_edid.c                | 33 ++++++++++++--------
>>>>>>>  drivers/gpu/drm/exynos/exynos_hdmi.c      |  3 ++-
>>>>>>>  drivers/gpu/drm/i2c/tda998x_drv.c         |  3 ++-
>>>>>>>  drivers/gpu/drm/i915/intel_hdmi.c         | 14 +++++-----
>>>>>>>  drivers/gpu/drm/i915/intel_lspcon.c       | 15 ++++++-----
>>>>>>>  drivers/gpu/drm/i915/intel_sdvo.c         | 10 ++++---
>>>>>>>  drivers/gpu/drm/mediatek/mtk_hdmi.c       |  3 ++-
>>>>>>>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |  3 ++-
>>>>>>>  drivers/gpu/drm/nouveau/dispnv50/disp.c   |  7 +++--
>>>>>>>  drivers/gpu/drm/omapdrm/omap_encoder.c    |  5 ++--
>>>>>>>  drivers/gpu/drm/radeon/radeon_audio.c     |  2 +-
>>>>>>>  drivers/gpu/drm/rockchip/inno_hdmi.c      |  4 ++-
>>>>>>>  drivers/gpu/drm/sti/sti_hdmi.c            |  3 ++-
>>>>>>>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |  3 ++-
>>>>>>>  drivers/gpu/drm/tegra/hdmi.c              |  3 ++-
>>>>>>>  drivers/gpu/drm/tegra/sor.c               |  3 ++-
>>>>>>>  drivers/gpu/drm/vc4/vc4_hdmi.c            | 11 +++++---
>>>>>>>  drivers/gpu/drm/zte/zx_hdmi.c             |  4 ++-
>>>>>>>  include/drm/drm_edid.h                    |  8 +++---
>>>>>>>  27 files changed, 94 insertions(+), 66 deletions(-)
>>>>>> For dw-hdmi and omapdrm,
>>>>>>
>>>>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>


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

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Laurent Pinchart]

Hi Andrzej,

On Wednesday, 5 December 2018 10:46:40 EET Andrzej Hajda wrote:
> On 05.12.2018 07:32, Laurent Pinchart wrote:
> > On Tuesday, 4 December 2018 21:13:20 EET Ville Syrjälä wrote:
> >> On Tue, Dec 04, 2018 at 08:46:53AM +0100, Andrzej Hajda wrote:
> >>> On 03.12.2018 22:38, Ville Syrjälä wrote:
> >>>> On Thu, Nov 29, 2018 at 10:08:07AM +0100, Andrzej Hajda wrote:
> >>>>> On 21.11.2018 19:19, Laurent Pinchart wrote:
> >>>>>> On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote:
> >>>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>>>> 
> >>>>>>> Make life easier for drivers by simply passing the connector
> >>>>>>> to drm_hdmi_avi_infoframe_from_display_mode() and
> >>>>>>> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
> >>>>>>> need to worry about is_hdmi2_sink mess.
> >>>>>> 
> >>>>>> While this is good for display controller drivers, the change isn't
> >>>>>> great for bridge drivers. Down the road we're looking at moving
> >>>>>> connector support out of the bridge drivers. Adding an additional
> >>>>>> dependency to connectors in the bridges will make that more
> >>>>>> difficult. Ideally bridges should retrieve the information from their
> >>>>>> sink, regardless of whether it is a connector or another bridge.
> >>>>> 
> >>>>> I agree with it, and case of sii8620 shows that there are cases where
> >>>>> bridge has no direct access to the connector.
> >>>> 
> >>>> It's just a matter of plumbing it through.
> >>> 
> >>> What do you mean exactly?
> >> 
> >> void bridge_foo(...
> >> +               ,struct drm_connector *connector);
> >> 
> >>>>> On the other side,  since you are passing connector to
> >>>>> drm_hdmi_avi_infoframe_from_display_mode(), you could drop mode
> >>>>> parameter and rename the function to
> >>>>> drm_hdmi_avi_infoframe_from_connector() then, unless mode passed and
> >>>>> mode set on the connector differs?
> >>>> 
> >>>> Connectors don't have a mode.
> >>> 
> >>> As they are passing video stream they should have it, even if not
> >>> directly, for example:
> >>> 
> >>> connector->state->crtc->mode
> >> 
> >> That's not really how atomic works. One shouldn't go digging
> >> through the obj->state pointers when we're not holding the
> >> relevant locks anymore. The atomic way would be to pass either
> >> both crtc state and connector state, or drm_atomic_state +
> >> crtc/connector.
> 
> Usually infoframe contents is generated in modesetting/enable callbacks
> so the locks should be in place.
> 
> And the locks should be hold for
> drm_hdmi_avi_infoframe_from_display_mode as well - it wouldn't be correct if
> 
> generated infoframe is not relevant to actual video mode. I guess that
> if connector->state->crtc->mode
> 
> differs from mode passed to drm_hdmi_avi_infoframe_from_display_mode it
> is a sign of possible problem.
> 
> And if they do not differ passing mode as an extra argument is redundant.
> 
> > Or a bridge state ? With chained bridges the mode can vary along the
> > pipeline, the CRTC adjusted mode will only cover the link between the
> > CRTC and the first bridge. It's only a matter of time until we need to
> > store other intermediate modes in states. I'd rather prepare for that
> > instead of passing the CRTC state to bridges.
> 
> Yes, optional bridge states seems reasonable, volunteers needed :)

I'll give it a go eventually, if nobody beats me to it. The exact timing will 
depend on many variables so I can't estimate it I'm afraid. All I'm asking is 
to avoid extending the drm_bridge API in a way that would make introduction of 
bridge states more complex. If someone needs bridge states, for instance to 
solve the above issue, then they should be added.

[snip]

-- 
Regards,

Laurent Pinchart



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

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Ville Syrjälä]

On Wed, Dec 05, 2018 at 09:46:40AM +0100, Andrzej Hajda wrote:
> On 05.12.2018 07:32, Laurent Pinchart wrote:
> > Hi Ville,
> >
> > On Tuesday, 4 December 2018 21:13:20 EET Ville Syrjälä wrote:
> >> On Tue, Dec 04, 2018 at 08:46:53AM +0100, Andrzej Hajda wrote:
> >>> On 03.12.2018 22:38, Ville Syrjälä wrote:
> >>>> On Thu, Nov 29, 2018 at 10:08:07AM +0100, Andrzej Hajda wrote:
> >>>>> On 21.11.2018 19:19, Laurent Pinchart wrote:
> >>>>>> On Tuesday, 20 November 2018 18:13:42 EET Ville Syrjala wrote:
> >>>>>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>>>>
> >>>>>>> Make life easier for drivers by simply passing the connector
> >>>>>>> to drm_hdmi_avi_infoframe_from_display_mode() and
> >>>>>>> drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
> >>>>>>> need to worry about is_hdmi2_sink mess.
> >>>>>> While this is good for display controller drivers, the change isn't
> >>>>>> great for bridge drivers. Down the road we're looking at moving
> >>>>>> connector support out of the bridge drivers. Adding an additional
> >>>>>> dependency to connectors in the bridges will make that more
> >>>>>> difficult. Ideally bridges should retrieve the information from their
> >>>>>> sink, regardless of whether it is a connector or another bridge.
> >>>>> I agree with it, and case of sii8620 shows that there are cases where
> >>>>> bridge has no direct access to the connector.
> >>>> It's just a matter of plumbing it through.
> >>> What do you mean exactly?
> >> void bridge_foo(...
> >> +               ,struct drm_connector *connector);
> >>
> >>>>> On the other side,  since you are passing connector to
> >>>>> drm_hdmi_avi_infoframe_from_display_mode(), you could drop mode
> >>>>> parameter and rename the function to
> >>>>> drm_hdmi_avi_infoframe_from_connector() then, unless mode passed and
> >>>>> mode set on the connector differs?
> >>>> Connectors don't have a mode.
> >>> As they are passing video stream they should have it, even if not
> >>> directly, for example:
> >>>
> >>> connector->state->crtc->mode
> >> That's not really how atomic works. One shouldn't go digging
> >> through the obj->state pointers when we're not holding the
> >> relevant locks anymore. The atomic way would be to pass either
> >> both crtc state and connector state, or drm_atomic_state +
> >> crtc/connector.
> 
> 
> Usually infoframe contents is generated in modesetting/enable callbacks
> so the locks should be in place.

Not when doing a nonblocking modeset.

> 
> And the locks should be hold for
> drm_hdmi_avi_infoframe_from_display_mode as well - it wouldn't be correct if
> 
> generated infoframe is not relevant to actual video mode. I guess that
> if connector->state->crtc->mode
> 
> differs from mode passed to drm_hdmi_avi_infoframe_from_display_mode it
> is a sign of possible problem.
> 
> And if they do not differ passing mode as an extra argument is redundant.
> 
> 
> > Or a bridge state ? With chained bridges the mode can vary along the pipeline, 
> > the CRTC adjusted mode will only cover the link between the CRTC and the first 
> > bridge. It's only a matter of time until we need to store other intermediate 
> > modes in states. I'd rather prepare for that instead of passing the CRTC state 
> > to bridges.
> 
> 
> Yes, optional bridge states seems reasonable, volunteers needed :)
> 
> 
> Regards
> 
> Andrzej
> 
> 
> >
> >>> In moment of creating infoframe it should be set properly.
> >>>
> >>>>>> Please see below for an additional comment.
> >>>>>>
> >>>>>>> Cc: Alex Deucher <alexander.deucher@amd.com>
> >>>>>>> Cc: "Christian König" <christian.koenig@amd.com>
> >>>>>>> Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
> >>>>>>> Cc: Archit Taneja <architt@codeaurora.org>
> >>>>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
> >>>>>>> Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
> >>>>>>> Cc: Inki Dae <inki.dae@samsung.com>
> >>>>>>> Cc: Joonyoung Shim <jy0922.shim@samsung.com>
> >>>>>> Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
> >>>>>>> Cc: Kyungmin Park <kyungmin.park@samsung.com>
> >>>>>>> Cc: Russell King <linux@armlinux.org.uk>
> >>>>>>> Cc: CK Hu <ck.hu@mediatek.com>
> >>>>>>> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> >>>>>>> Cc: Rob Clark <robdclark@gmail.com>
> >>>>>>> Cc: Ben Skeggs <bskeggs@redhat.com>
> >>>>>>> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> >>>>>>> Cc: Sandy Huang <hjc@rock-chips.com>
> >>>>>>> Cc: "Heiko Stübner" <heiko@sntech.de>
> >>>>>>> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> >>>>>>> Cc: Vincent Abriou <vincent.abriou@st.com>
> >>>>>>> Cc: Thierry Reding <thierry.reding@gmail.com>
> >>>>>>> Cc: Eric Anholt <eric@anholt.net>
> >>>>>>> Cc: Shawn Guo <shawnguo@kernel.org>
> >>>>>>> Cc: Ilia Mirkin <imirkin@alum.mit.edu>
> >>>>>>> Cc: amd-gfx@lists.freedesktop.org
> >>>>>>> Cc: linux-arm-msm@vger.kernel.org
> >>>>>>> Cc: freedreno@lists.freedesktop.org
> >>>>>>> Cc: nouveau@lists.freedesktop.org
> >>>>>>> Cc: linux-tegra@vger.kernel.org
> >>>>>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>>>>> ---
> >>>>>>>
> >>>>>>>  drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  2 +-
> >>>>>>>  drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  2 +-
> >>>>>>>  drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  3 ++-
> >>>>>>>  drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  2 +-
> >>>>>>>  drivers/gpu/drm/bridge/analogix-anx78xx.c |  5 ++--
> >>>>>>>  drivers/gpu/drm/bridge/sii902x.c          |  3 ++-
> >>>>>>>  drivers/gpu/drm/bridge/sil-sii8620.c      |  3 +--
> >>>>>>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  3 ++-
> >>>>>>>  drivers/gpu/drm/drm_edid.c                | 33 ++++++++++++--------
> >>>>>>>  drivers/gpu/drm/exynos/exynos_hdmi.c      |  3 ++-
> >>>>>>>  drivers/gpu/drm/i2c/tda998x_drv.c         |  3 ++-
> >>>>>>>  drivers/gpu/drm/i915/intel_hdmi.c         | 14 +++++-----
> >>>>>>>  drivers/gpu/drm/i915/intel_lspcon.c       | 15 ++++++-----
> >>>>>>>  drivers/gpu/drm/i915/intel_sdvo.c         | 10 ++++---
> >>>>>>>  drivers/gpu/drm/mediatek/mtk_hdmi.c       |  3 ++-
> >>>>>>>  drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |  3 ++-
> >>>>>>>  drivers/gpu/drm/nouveau/dispnv50/disp.c   |  7 +++--
> >>>>>>>  drivers/gpu/drm/omapdrm/omap_encoder.c    |  5 ++--
> >>>>>>>  drivers/gpu/drm/radeon/radeon_audio.c     |  2 +-
> >>>>>>>  drivers/gpu/drm/rockchip/inno_hdmi.c      |  4 ++-
> >>>>>>>  drivers/gpu/drm/sti/sti_hdmi.c            |  3 ++-
> >>>>>>>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |  3 ++-
> >>>>>>>  drivers/gpu/drm/tegra/hdmi.c              |  3 ++-
> >>>>>>>  drivers/gpu/drm/tegra/sor.c               |  3 ++-
> >>>>>>>  drivers/gpu/drm/vc4/vc4_hdmi.c            | 11 +++++---
> >>>>>>>  drivers/gpu/drm/zte/zx_hdmi.c             |  4 ++-
> >>>>>>>  include/drm/drm_edid.h                    |  8 +++---
> >>>>>>>  27 files changed, 94 insertions(+), 66 deletions(-)
> >>>>>> For dw-hdmi and omapdrm,
> >>>>>>
> >>>>>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> 

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

Re: [PATCH 1/4] drm/edid: Pass connector to AVI inforframe functions

[Russell King - ARM Linux]

On Tue, Nov 20, 2018 at 06:13:42PM +0200, Ville Syrjala wrote:
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
> index a7c39f39793f..38c66fbc8276 100644
> --- a/drivers/gpu/drm/i2c/tda998x_drv.c
> +++ b/drivers/gpu/drm/i2c/tda998x_drv.c
> @@ -849,7 +849,8 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode)
>  {
>  	union hdmi_infoframe frame;
>  
> -	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
> +	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
> +						 &priv->connector, mode);
>  	frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL;
>  
>  	tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame);

For this,

Acked-by: Russell King <rmk+kernel@armlinux.org.uk>

Thanks.

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up
According to speedtest.net: 11.9Mbps down 500kbps up
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/4] drm/edid: Pass connector to AVI infoframe functions

[Ville Syrjala]

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

Make life easier for drivers by simply passing the connector
to drm_hdmi_avi_infoframe_from_display_mode() and
drm_hdmi_avi_infoframe_quant_range(). That way drivers don't
need to worry about is_hdmi2_sink mess.

v2: Make is_hdmi2_sink() return true for sil-sii8620
    Adapt to omap/vc4 changes

Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: "David (ChunMing) Zhou" <David1.Zhou@amd.com>
Cc: Archit Taneja <architt@codeaurora.org>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Joonyoung Shim <jy0922.shim@samsung.com>
Cc: Seung-Woo Kim <sw0312.kim@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: CK Hu <ck.hu@mediatek.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Ben Skeggs <bskeggs@redhat.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Sandy Huang <hjc@rock-chips.com>
Cc: "Heiko Stübner" <heiko@sntech.de>
Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Cc: Vincent Abriou <vincent.abriou@st.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Eric Anholt <eric@anholt.net>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: Ilia Mirkin <imirkin@alum.mit.edu>
Cc: amd-gfx@lists.freedesktop.org
Cc: linux-arm-msm@vger.kernel.org
Cc: freedreno@lists.freedesktop.org
Cc: nouveau@lists.freedesktop.org
Cc: linux-tegra@vger.kernel.org
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Thierry Reding <treding@nvidia.com>
Acked-by: Russell King <rmk+kernel@armlinux.org.uk>
Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/amd/amdgpu/dce_v10_0.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v11_0.c    |  2 +-
 drivers/gpu/drm/amd/amdgpu/dce_v6_0.c     |  3 ++-
 drivers/gpu/drm/amd/amdgpu/dce_v8_0.c     |  2 +-
 drivers/gpu/drm/bridge/analogix-anx78xx.c |  5 ++--
 drivers/gpu/drm/bridge/sii902x.c          |  3 ++-
 drivers/gpu/drm/bridge/sil-sii8620.c      |  3 +--
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c |  3 ++-
 drivers/gpu/drm/drm_edid.c                | 33 ++++++++++++++---------
 drivers/gpu/drm/exynos/exynos_hdmi.c      |  3 ++-
 drivers/gpu/drm/i2c/tda998x_drv.c         |  3 ++-
 drivers/gpu/drm/i915/intel_hdmi.c         | 14 +++++-----
 drivers/gpu/drm/i915/intel_lspcon.c       | 15 ++++++-----
 drivers/gpu/drm/i915/intel_sdvo.c         | 10 ++++---
 drivers/gpu/drm/mediatek/mtk_hdmi.c       |  3 ++-
 drivers/gpu/drm/msm/hdmi/hdmi_bridge.c    |  3 ++-
 drivers/gpu/drm/nouveau/dispnv50/disp.c   |  7 +++--
 drivers/gpu/drm/omapdrm/omap_encoder.c    |  4 +--
 drivers/gpu/drm/radeon/radeon_audio.c     |  2 +-
 drivers/gpu/drm/rockchip/inno_hdmi.c      |  4 ++-
 drivers/gpu/drm/sti/sti_hdmi.c            |  3 ++-
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c    |  3 ++-
 drivers/gpu/drm/tegra/hdmi.c              |  3 ++-
 drivers/gpu/drm/tegra/sor.c               |  3 ++-
 drivers/gpu/drm/vc4/vc4_hdmi.c            |  9 ++++---
 drivers/gpu/drm/zte/zx_hdmi.c             |  4 ++-
 include/drm/drm_edid.h                    |  8 +++---
 27 files changed, 91 insertions(+), 66 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
index 4cfecdce29a3..1f0426d2fc2a 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v10_0.c
@@ -1682,7 +1682,7 @@ static void dce_v10_0_afmt_setmode(struct drm_encoder *encoder,
 	dce_v10_0_audio_write_sad_regs(encoder);
 	dce_v10_0_audio_write_latency_fields(encoder, mode);
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
 	if (err < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
 		return;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 7c868916d90f..2280b971d758 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -1724,7 +1724,7 @@ static void dce_v11_0_afmt_setmode(struct drm_encoder *encoder,
 	dce_v11_0_audio_write_sad_regs(encoder);
 	dce_v11_0_audio_write_latency_fields(encoder, mode);
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
 	if (err < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
 		return;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
index 17eaaba36017..db443ec53d3a 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v6_0.c
@@ -1423,6 +1423,7 @@ static void dce_v6_0_audio_set_avi_infoframe(struct drm_encoder *encoder,
 	struct amdgpu_device *adev = dev->dev_private;
 	struct amdgpu_encoder *amdgpu_encoder = to_amdgpu_encoder(encoder);
 	struct amdgpu_encoder_atom_dig *dig = amdgpu_encoder->enc_priv;
+	struct drm_connector *connector = amdgpu_get_connector_for_encoder(encoder);
 	struct hdmi_avi_infoframe frame;
 	u8 buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE];
 	uint8_t *payload = buffer + 3;
@@ -1430,7 +1431,7 @@ static void dce_v6_0_audio_set_avi_infoframe(struct drm_encoder *encoder,
 	ssize_t err;
 	u32 tmp;
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
 	if (err < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
 		return;
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
index 8c0576978d36..13da915991dd 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c
@@ -1616,7 +1616,7 @@ static void dce_v8_0_afmt_setmode(struct drm_encoder *encoder,
 	dce_v8_0_audio_write_sad_regs(encoder);
 	dce_v8_0_audio_write_latency_fields(encoder, mode);
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
 	if (err < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %zd\n", err);
 		return;
diff --git a/drivers/gpu/drm/bridge/analogix-anx78xx.c b/drivers/gpu/drm/bridge/analogix-anx78xx.c
index f8433c93f463..e11309e9bc4f 100644
--- a/drivers/gpu/drm/bridge/analogix-anx78xx.c
+++ b/drivers/gpu/drm/bridge/analogix-anx78xx.c
@@ -1094,8 +1094,9 @@ static void anx78xx_bridge_mode_set(struct drm_bridge *bridge,
 
 	mutex_lock(&anx78xx->lock);
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, adjusted_mode,
-						       false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
+						       &anx78xx->connector,
+						       adjusted_mode);
 	if (err) {
 		DRM_ERROR("Failed to setup AVI infoframe: %d\n", err);
 		goto unlock;
diff --git a/drivers/gpu/drm/bridge/sii902x.c b/drivers/gpu/drm/bridge/sii902x.c
index bfa902013aa4..a9b4f45ae87c 100644
--- a/drivers/gpu/drm/bridge/sii902x.c
+++ b/drivers/gpu/drm/bridge/sii902x.c
@@ -258,7 +258,8 @@ static void sii902x_bridge_mode_set(struct drm_bridge *bridge,
 	if (ret)
 		return;
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj, false);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame,
+						       &sii902x->connector, adj);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return;
diff --git a/drivers/gpu/drm/bridge/sil-sii8620.c b/drivers/gpu/drm/bridge/sil-sii8620.c
index a6e8f4591e63..0cc293a6ac24 100644
--- a/drivers/gpu/drm/bridge/sil-sii8620.c
+++ b/drivers/gpu/drm/bridge/sil-sii8620.c
@@ -1104,8 +1104,7 @@ static void sii8620_set_infoframes(struct sii8620 *ctx,
 	int ret;
 
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
-						       mode,
-						       true);
+						       NULL, mode);
 	if (ctx->use_packed_pixel)
 		frm.avi.colorspace = HDMI_COLORSPACE_YUV422;
 
diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index 64c3cf027518..88b720b63126 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -1344,7 +1344,8 @@ static void hdmi_config_AVI(struct dw_hdmi *hdmi, struct drm_display_mode *mode)
 	u8 val;
 
 	/* Initialise info frame from DRM mode */
-	drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	drm_hdmi_avi_infoframe_from_display_mode(&frame,
+						 &hdmi->connector, mode);
 
 	if (hdmi_bus_fmt_is_yuv444(hdmi->hdmi_data.enc_out_bus_format))
 		frame.colorspace = HDMI_COLORSPACE_YUV444;
diff --git a/drivers/gpu/drm/drm_edid.c b/drivers/gpu/drm/drm_edid.c
index b506e3622b08..cd25bd08bf53 100644
--- a/drivers/gpu/drm/drm_edid.c
+++ b/drivers/gpu/drm/drm_edid.c
@@ -4830,19 +4830,32 @@ void drm_set_preferred_mode(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_set_preferred_mode);
 
+static bool is_hdmi2_sink(struct drm_connector *connector)
+{
+	/*
+	 * FIXME: sil-sii8620 doesn't have a connector around when
+	 * we need one, so we have to be prepared for a NULL connector.
+	 */
+	if (!connector)
+		return true;
+
+	return connector->display_info.hdmi.scdc.supported ||
+		connector->display_info.color_formats & DRM_COLOR_FORMAT_YCRCB420;
+}
+
 /**
  * drm_hdmi_avi_infoframe_from_display_mode() - fill an HDMI AVI infoframe with
  *                                              data from a DRM display mode
  * @frame: HDMI AVI infoframe
+ * @connector: the connector
  * @mode: DRM display mode
- * @is_hdmi2_sink: Sink is HDMI 2.0 compliant
  *
  * Return: 0 on success or a negative error code on failure.
  */
 int
 drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
-					 const struct drm_display_mode *mode,
-					 bool is_hdmi2_sink)
+					 struct drm_connector *connector,
+					 const struct drm_display_mode *mode)
 {
 	enum hdmi_picture_aspect picture_aspect;
 	int err;
@@ -4864,7 +4877,7 @@ drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
 	 * HDMI 2.0 VIC range: 1 <= VIC <= 107 (CEA-861-F). So we
 	 * have to make sure we dont break HDMI 1.4 sinks.
 	 */
-	if (!is_hdmi2_sink && frame->video_code > 64)
+	if (!is_hdmi2_sink(connector) && frame->video_code > 64)
 		frame->video_code = 0;
 
 	/*
@@ -4923,21 +4936,17 @@ 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
+ * @connector: the connector
  * @mode: DRM display mode
  * @rgb_quant_range: RGB quantization range (Q)
  * @rgb_quant_range_selectable: Sink support selectable RGB quantization range (QS)
- * @is_hdmi2_sink: HDMI 2.0 sink, which has different default recommendations
- *
- * Note that @is_hdmi2_sink can be derived by looking at the
- * &drm_scdc.supported flag stored in &drm_hdmi_info.scdc,
- * &drm_display_info.hdmi, which can be found in &drm_connector.display_info.
  */
 void
 drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
+				   struct drm_connector *connector,
 				   const struct drm_display_mode *mode,
 				   enum hdmi_quantization_range rgb_quant_range,
-				   bool rgb_quant_range_selectable,
-				   bool is_hdmi2_sink)
+				   bool rgb_quant_range_selectable)
 {
 	/*
 	 * CEA-861:
@@ -4968,7 +4977,7 @@ drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
 	 * we limit non-zero YQ to HDMI 2.0 sinks only as HDMI 2.0 is based
 	 * on on CEA-861-F.
 	 */
-	if (!is_hdmi2_sink ||
+	if (!is_hdmi2_sink(connector) ||
 	    rgb_quant_range == HDMI_QUANTIZATION_RANGE_LIMITED)
 		frame->ycc_quantization_range =
 			HDMI_YCC_QUANTIZATION_RANGE_LIMITED;
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 2092a650df7d..b857df67aff0 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -819,7 +819,8 @@ static void hdmi_reg_infoframes(struct hdmi_context *hdata)
 		return;
 	}
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi, m, false);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frm.avi,
+						       &hdata->connector, m);
 	if (!ret)
 		ret = hdmi_avi_infoframe_pack(&frm.avi, buf, sizeof(buf));
 	if (ret > 0) {
diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c
index a7c39f39793f..38c66fbc8276 100644
--- a/drivers/gpu/drm/i2c/tda998x_drv.c
+++ b/drivers/gpu/drm/i2c/tda998x_drv.c
@@ -849,7 +849,8 @@ tda998x_write_avi(struct tda998x_priv *priv, struct drm_display_mode *mode)
 {
 	union hdmi_infoframe frame;
 
-	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
+	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
+						 &priv->connector, mode);
 	frame.avi.quantization_range = HDMI_QUANTIZATION_RANGE_FULL;
 
 	tda998x_write_if(priv, DIP_IF_FLAGS_IF2, REG_IF2_HB0, &frame);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 07e803a604bd..286cdfac2f3e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -482,15 +482,12 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
 	const struct drm_display_mode *adjusted_mode =
 		&crtc_state->base.adjusted_mode;
-	struct drm_connector *connector = &intel_hdmi->attached_connector->base;
-	bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported ||
-	   connector->display_info.color_formats & DRM_COLOR_FORMAT_YCRCB420;
 	union hdmi_infoframe frame;
 	int ret;
 
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
-						       adjusted_mode,
-						       is_hdmi2_sink);
+						       conn_state->connector,
+						       adjusted_mode);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return;
@@ -503,12 +500,13 @@ static void intel_hdmi_set_avi_infoframe(struct intel_encoder *encoder,
 	else
 		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
 
-	drm_hdmi_avi_infoframe_quant_range(&frame.avi, adjusted_mode,
+	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
+					   conn_state->connector,
+					   adjusted_mode,
 					   crtc_state->limited_color_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
 					   HDMI_QUANTIZATION_RANGE_FULL,
-					   intel_hdmi->rgb_quant_range_selectable,
-					   is_hdmi2_sink);
+					   intel_hdmi->rgb_quant_range_selectable);
 
 	drm_hdmi_avi_infoframe_content_type(&frame.avi,
 					    conn_state);
diff --git a/drivers/gpu/drm/i915/intel_lspcon.c b/drivers/gpu/drm/i915/intel_lspcon.c
index 96a8d9524b0c..c18f14d41f1d 100644
--- a/drivers/gpu/drm/i915/intel_lspcon.c
+++ b/drivers/gpu/drm/i915/intel_lspcon.c
@@ -462,10 +462,8 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
 	uint8_t buf[VIDEO_DIP_DATA_SIZE];
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 	struct intel_lspcon *lspcon = &dig_port->lspcon;
-	struct intel_dp *intel_dp = &dig_port->dp;
-	struct drm_connector *connector = &intel_dp->attached_connector->base;
-	const struct drm_display_mode *mode = &crtc_state->base.adjusted_mode;
-	bool is_hdmi2_sink = connector->display_info.hdmi.scdc.supported;
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->base.adjusted_mode;
 
 	if (!lspcon->active) {
 		DRM_ERROR("Writing infoframes while LSPCON disabled ?\n");
@@ -473,7 +471,8 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
 	}
 
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
-						       mode, is_hdmi2_sink);
+						       conn_state->connector,
+						       adjusted_mode);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return;
@@ -488,11 +487,13 @@ void lspcon_set_infoframes(struct intel_encoder *encoder,
 		frame.avi.colorspace = HDMI_COLORSPACE_RGB;
 	}
 
-	drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
+	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
+					   conn_state->connector,
+					   adjusted_mode,
 					   crtc_state->limited_color_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
 					   HDMI_QUANTIZATION_RANGE_FULL,
-					   false, is_hdmi2_sink);
+					   false);
 
 	ret = hdmi_infoframe_pack(&frame, buf, sizeof(buf));
 	if (ret < 0) {
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 5805ec1aba12..1277d31adb54 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -981,7 +981,8 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
 }
 
 static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
-					 const struct intel_crtc_state *pipe_config)
+					 const struct intel_crtc_state *pipe_config,
+					 const struct drm_connector_state *conn_state)
 {
 	uint8_t sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
 	union hdmi_infoframe frame;
@@ -989,8 +990,8 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
 	ssize_t len;
 
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
-						       &pipe_config->base.adjusted_mode,
-						       false);
+						       conn_state->connector,
+						       &pipe_config->base.adjusted_mode);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return false;
@@ -1316,7 +1317,8 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_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, crtc_state);
+		intel_sdvo_set_avi_infoframe(intel_sdvo,
+					     crtc_state, conn_state);
 	} else
 		intel_sdvo_set_encode(intel_sdvo, SDVO_ENCODE_DVI);
 
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 862f3ec22131..f3ef7bf80563 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -981,7 +981,8 @@ static int mtk_hdmi_setup_avi_infoframe(struct mtk_hdmi *hdmi,
 	u8 buffer[17];
 	ssize_t err;
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
+						       &hdmi->conn, mode);
 	if (err < 0) {
 		dev_err(hdmi->dev,
 			"Failed to get AVI infoframe from mode: %zd\n", err);
diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
index 98d61c690260..7ba6f52ed72c 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_bridge.c
@@ -101,7 +101,8 @@ static void msm_hdmi_config_avi_infoframe(struct hdmi *hdmi)
 	u32 val;
 	int len;
 
-	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
+	drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
+						 hdmi->connector, mode);
 
 	len = hdmi_infoframe_pack(&frame, buffer, sizeof(buffer));
 	if (len < 0) {
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 26af45785939..bc06aa79363f 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -561,7 +561,7 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
 	u32 max_ac_packet;
 	union hdmi_infoframe avi_frame;
 	union hdmi_infoframe vendor_frame;
-	bool scdc_supported, high_tmds_clock_ratio = false, scrambling = false;
+	bool high_tmds_clock_ratio = false, scrambling = false;
 	u8 config;
 	int ret;
 	int size;
@@ -571,10 +571,9 @@ nv50_hdmi_enable(struct drm_encoder *encoder, struct drm_display_mode *mode)
 		return;
 
 	hdmi = &nv_connector->base.display_info.hdmi;
-	scdc_supported = hdmi->scdc.supported;
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame.avi, mode,
-						       scdc_supported);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&avi_frame.avi,
+						       &nv_connector->base, mode);
 	if (!ret) {
 		/* We have an AVI InfoFrame, populate it to the display */
 		args.pwr.avi_infoframe_length
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index 933ebc9f9faa..148b6b20274f 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -76,8 +76,8 @@ static void omap_encoder_hdmi_mode_set(struct drm_encoder *encoder,
 		struct hdmi_avi_infoframe avi;
 		int r;
 
-		r = drm_hdmi_avi_infoframe_from_display_mode(&avi, adjusted_mode,
-							     false);
+		r = drm_hdmi_avi_infoframe_from_display_mode(&avi, connector,
+							     adjusted_mode);
 		if (r == 0)
 			dssdev->ops->hdmi.set_infoframe(dssdev, &avi);
 	}
diff --git a/drivers/gpu/drm/radeon/radeon_audio.c b/drivers/gpu/drm/radeon/radeon_audio.c
index 770e31f5fd1b..5a7d48339b32 100644
--- a/drivers/gpu/drm/radeon/radeon_audio.c
+++ b/drivers/gpu/drm/radeon/radeon_audio.c
@@ -516,7 +516,7 @@ static int radeon_audio_set_avi_packet(struct drm_encoder *encoder,
 	if (!connector)
 		return -EINVAL;
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, connector, mode);
 	if (err < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %d\n", err);
 		return err;
diff --git a/drivers/gpu/drm/rockchip/inno_hdmi.c b/drivers/gpu/drm/rockchip/inno_hdmi.c
index 1c02b3e61299..27c945e030a0 100644
--- a/drivers/gpu/drm/rockchip/inno_hdmi.c
+++ b/drivers/gpu/drm/rockchip/inno_hdmi.c
@@ -295,7 +295,9 @@ static int inno_hdmi_config_video_avi(struct inno_hdmi *hdmi,
 	union hdmi_infoframe frame;
 	int rc;
 
-	rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
+	rc = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
+						      &hdmi->connector,
+						      mode);
 
 	if (hdmi->hdmi_data.enc_out_format == HDMI_COLORSPACE_YUV444)
 		frame.avi.colorspace = HDMI_COLORSPACE_YUV444;
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index ccf718404a1c..4b86878f8ddf 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -434,7 +434,8 @@ static int hdmi_avi_infoframe_config(struct sti_hdmi *hdmi)
 
 	DRM_DEBUG_DRIVER("\n");
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe, mode, false);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&infoframe,
+						       hdmi->drm_connector, mode);
 	if (ret < 0) {
 		DRM_ERROR("failed to setup AVI infoframe: %d\n", ret);
 		return ret;
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 061d2e0d9011..554a6f4561f3 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -52,7 +52,8 @@ static int sun4i_hdmi_setup_avi_infoframes(struct sun4i_hdmi *hdmi,
 	u8 buffer[17];
 	int i, ret;
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame,
+						       &hdmi->connector, mode);
 	if (ret < 0) {
 		DRM_ERROR("Failed to get infoframes from mode\n");
 		return ret;
diff --git a/drivers/gpu/drm/tegra/hdmi.c b/drivers/gpu/drm/tegra/hdmi.c
index 0082468f703c..a7566c67bfb0 100644
--- a/drivers/gpu/drm/tegra/hdmi.c
+++ b/drivers/gpu/drm/tegra/hdmi.c
@@ -741,7 +741,8 @@ static void tegra_hdmi_setup_avi_infoframe(struct tegra_hdmi *hdmi,
 	u8 buffer[17];
 	ssize_t err;
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
+						       &hdmi->output.connector, mode);
 	if (err < 0) {
 		dev_err(hdmi->dev, "failed to setup AVI infoframe: %zd\n", err);
 		return;
diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index ef8692b7075a..44feac2a0359 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -2116,7 +2116,8 @@ tegra_sor_hdmi_setup_avi_infoframe(struct tegra_sor *sor,
 	value &= ~INFOFRAME_CTRL_ENABLE;
 	tegra_sor_writel(sor, value, SOR_HDMI_AVI_INFOFRAME_CTRL);
 
-	err = drm_hdmi_avi_infoframe_from_display_mode(&frame, mode, false);
+	err = drm_hdmi_avi_infoframe_from_display_mode(&frame,
+						       &sor->output.connector, mode);
 	if (err < 0) {
 		dev_err(sor->dev, "failed to setup AVI infoframe: %d\n", err);
 		return err;
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 2f276222e30f..4f9a80f7f242 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -424,18 +424,19 @@ static void vc4_hdmi_set_avi_infoframe(struct drm_encoder *encoder)
 	union hdmi_infoframe frame;
 	int ret;
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
+						       hdmi->connector, mode);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return;
 	}
 
-	drm_hdmi_avi_infoframe_quant_range(&frame.avi, mode,
+	drm_hdmi_avi_infoframe_quant_range(&frame.avi,
+					   hdmi->connector, mode,
 					   vc4_encoder->limited_rgb_range ?
 					   HDMI_QUANTIZATION_RANGE_LIMITED :
 					   HDMI_QUANTIZATION_RANGE_FULL,
-					   vc4_encoder->rgb_range_selectable,
-					   false);
+					   vc4_encoder->rgb_range_selectable);
 
 	frame.avi.right_bar = cstate->tv.margins.right;
 	frame.avi.left_bar = cstate->tv.margins.left;
diff --git a/drivers/gpu/drm/zte/zx_hdmi.c b/drivers/gpu/drm/zte/zx_hdmi.c
index 78655269d843..9fc98bb4f3d9 100644
--- a/drivers/gpu/drm/zte/zx_hdmi.c
+++ b/drivers/gpu/drm/zte/zx_hdmi.c
@@ -125,7 +125,9 @@ static int zx_hdmi_config_video_avi(struct zx_hdmi *hdmi,
 	union hdmi_infoframe frame;
 	int ret;
 
-	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi, mode, false);
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
+						       &hdmi->connector,
+						       mode);
 	if (ret) {
 		DRM_DEV_ERROR(hdmi->dev, "failed to get avi infoframe: %d\n",
 			      ret);
diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index e3c404833115..9db6f130df65 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -352,18 +352,18 @@ drm_load_edid_firmware(struct drm_connector *connector)
 
 int
 drm_hdmi_avi_infoframe_from_display_mode(struct hdmi_avi_infoframe *frame,
-					 const struct drm_display_mode *mode,
-					 bool is_hdmi2_sink);
+					 struct drm_connector *connector,
+					 const struct drm_display_mode *mode);
 int
 drm_hdmi_vendor_infoframe_from_display_mode(struct hdmi_vendor_infoframe *frame,
 					    struct drm_connector *connector,
 					    const struct drm_display_mode *mode);
 void
 drm_hdmi_avi_infoframe_quant_range(struct hdmi_avi_infoframe *frame,
+				   struct drm_connector *connector,
 				   const struct drm_display_mode *mode,
 				   enum hdmi_quantization_range rgb_quant_range,
-				   bool rgb_quant_range_selectable,
-				   bool is_hdmi2_sink);
+				   bool rgb_quant_range_selectable);
 
 /**
  * drm_eld_mnl - Get ELD monitor name length in bytes.
-- 
2.19.2

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

[PATCH 2/4] drm/i915: Use drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI as well

[Ville Syrjala]

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

Fill out the AVI infoframe quantization range bits using
drm_hdmi_avi_infoframe_quant_range() for SDVO HDMI encoder as well.

This changes the behaviour slightly as
drm_hdmi_avi_infoframe_quant_range() will set a non-zero Q bit
even when QS==0 iff the Q bit matched the default quantization
range for the given mode. This matches the recommendation in
HDMI 2.0 and is allowed even before that.

v2: Pimp commit msg (DK)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
---
 drivers/gpu/drm/i915/intel_sdvo.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 1277d31adb54..9c16e273fb8d 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -984,6 +984,8 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
 					 const struct intel_crtc_state *pipe_config,
 					 const struct drm_connector_state *conn_state)
 {
+	const struct drm_display_mode *adjusted_mode =
+		&pipe_config->base.adjusted_mode;
 	uint8_t sdvo_data[HDMI_INFOFRAME_SIZE(AVI)];
 	union hdmi_infoframe frame;
 	int ret;
@@ -991,20 +993,19 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
 
 	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame.avi,
 						       conn_state->connector,
-						       &pipe_config->base.adjusted_mode);
+						       adjusted_mode);
 	if (ret < 0) {
 		DRM_ERROR("couldn't fill AVI infoframe\n");
 		return false;
 	}
 
-	if (intel_sdvo->rgb_quant_range_selectable) {
-		if (pipe_config->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,
+					   conn_state->connector,
+					   adjusted_mode,
+					   pipe_config->limited_color_range ?
+					   HDMI_QUANTIZATION_RANGE_LIMITED :
+					   HDMI_QUANTIZATION_RANGE_FULL,
+					   intel_sdvo->rgb_quant_range_selectable);
 
 	len = hdmi_infoframe_pack(&frame, sdvo_data, sizeof(sdvo_data));
 	if (len < 0)
-- 
2.19.2

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