[PATCH 0/7] drm/i915: Fix SDVO HDMI audio

[PATCH 0/7] drm/i915: Fix SDVO HDMI audio

[Ville Syrjala]

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

Here's a series to fix SDVO HDMI audio handling which got broken
quite a while ago. I also tossed in a few extra cleanups.

Ville Syrjälä (7):
  drm/i915/sdvo: Fix AVI infoframe TX rate readout
  drm/i915/sdvo: Implement proper HDMI audio support for SDVO
  drm/i915: Rename SDVO_AUDIO_ENABLE to HDMI_AUDIO_ENABLE
  drm/i915/sdvo: Check that we have space for the infoframe
  drm/i915/sdvo: Don't unpack stack garbage
  drm/i915/sdvo: Don't write stack garbage into the hbuf
  drm/i915/sdvo: Actually print the reason why the SDVO command failed

 drivers/gpu/drm/i915/i915_reg.h        |  2 +-
 drivers/gpu/drm/i915/intel_hdmi.c      | 10 ++--
 drivers/gpu/drm/i915/intel_sdvo.c      | 80 +++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_sdvo_regs.h |  3 +
 4 files changed, 69 insertions(+), 26 deletions(-)

-- 
2.21.0

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

[PATCH 1/7] drm/i915/sdvo: Fix AVI infoframe TX rate readout

[Ville Syrjala]

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

The AVI infoframe readout code currently issues a
SDVO_CMD_GET_HBUF_TXRATE before SDVO_CMD_SET_HBUF_INDEX, which is
not the correct order for these two operations. So far this wasn't
a problem since we left the index pointing at the AVI infoframe
buffer at the end of the modeset. However once we start to write
to other buffers (namely ELD) that is no longer going to be true.
Fix up the order so that we always read out the TX rate for the
correct buffer.

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

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 0e3d91d9ef13..61db07244296 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1001,6 +1001,11 @@ static ssize_t intel_sdvo_read_infoframe(struct intel_sdvo *intel_sdvo,
 	if (av_split < if_index)
 		return 0;
 
+	if (!intel_sdvo_set_value(intel_sdvo,
+				  SDVO_CMD_SET_HBUF_INDEX,
+				  set_buf_index, 2))
+		return -ENXIO;
+
 	if (!intel_sdvo_get_value(intel_sdvo,
 				  SDVO_CMD_GET_HBUF_TXRATE,
 				  &tx_rate, 1))
@@ -1009,11 +1014,6 @@ static ssize_t intel_sdvo_read_infoframe(struct intel_sdvo *intel_sdvo,
 	if (tx_rate == SDVO_HBUF_TX_DISABLED)
 		return 0;
 
-	if (!intel_sdvo_set_value(intel_sdvo,
-				  SDVO_CMD_SET_HBUF_INDEX,
-				  set_buf_index, 2))
-		return -ENXIO;
-
 	if (!intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_HBUF_INFO,
 				  &hbuf_size, 1))
 		return -ENXIO;
-- 
2.21.0

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

[PATCH 2/7] drm/i915/sdvo: Implement proper HDMI audio support for SDVO

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

Our SDVO audio support is pretty bogus. We can't push audio over the
SDVO bus, so trying to enable audio in the SDVO control register doesn't
do anything. In fact it looks like the SDVO encoder will always mix in
the audio coming over HDA, and there's no (at least documented) way to
disable that from our side. So HDMI audio does work currently but only by
luck really. What is missing though is the ELD.

To pass the ELD to the audio driver we need to write it to magic buffer
in the SDVO encoder hardware which then gets pulled out via HDA in the
other end. Ie. pretty much the same thing we had for native HDMI before
we started to just pass the ELD between the drivers. This sort of
explains why we even have that silly hardware buffer with native HDMI.

$ cat /proc/asound/card0/eld#1.0
-monitor_present		0
-eld_valid		0
+monitor_present		1
+eld_valid		1
+monitor_name		LG TV
+connection_type		HDMI
+...

This also fixes our state readout since we can now query the SDVO
encoder about the state of the "ELD valid" and "presence detect"
bits. As mentioned those don't actually control whether audio
gets sent over the HDMI cable, but it's the best we can do.

Cc: stable@vger.kernel.org
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: zardam@gmail.com
Tested-by: zardam@gmail.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108976
Fixes: de44e256b92c ("drm/i915/sdvo: Shut up state checker with hdmi cards on gen3")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sdvo.c      | 58 +++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_sdvo_regs.h |  3 ++
 2 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 61db07244296..7f64352a3413 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -916,6 +916,13 @@ static bool intel_sdvo_set_colorimetry(struct intel_sdvo *intel_sdvo,
 	return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_COLORIMETRY, &mode, 1);
 }
 
+static bool intel_sdvo_set_audio_state(struct intel_sdvo *intel_sdvo,
+				       u8 audio_state)
+{
+	return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_AUDIO_STAT,
+				    &audio_state, 1);
+}
+
 #if 0
 static void intel_sdvo_dump_hdmi_buf(struct intel_sdvo *intel_sdvo)
 {
@@ -1487,11 +1494,6 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder,
 	else
 		sdvox |= SDVO_PIPE_SEL(crtc->pipe);
 
-	if (crtc_state->has_audio) {
-		WARN_ON_ONCE(INTEL_GEN(dev_priv) < 4);
-		sdvox |= SDVO_AUDIO_ENABLE;
-	}
-
 	if (INTEL_GEN(dev_priv) >= 4) {
 		/* done in crtc_mode_set as the dpll_md reg must be written early */
 	} else if (IS_I945G(dev_priv) || IS_I945GM(dev_priv) ||
@@ -1635,8 +1637,13 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
 	if (sdvox & HDMI_COLOR_RANGE_16_235)
 		pipe_config->limited_color_range = true;
 
-	if (sdvox & SDVO_AUDIO_ENABLE)
-		pipe_config->has_audio = true;
+	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_AUDIO_STAT,
+				 &val, 1)) {
+		u8 mask = SDVO_AUDIO_ELD_VALID | SDVO_AUDIO_PRESENCE_DETECT;
+
+		if ((val & mask) == mask)
+			pipe_config->has_audio = true;
+	}
 
 	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_ENCODE,
 				 &val, 1)) {
@@ -1647,6 +1654,32 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
 	intel_sdvo_get_avi_infoframe(intel_sdvo, pipe_config);
 }
 
+static void intel_sdvo_disable_audio(struct intel_sdvo *intel_sdvo)
+{
+	intel_sdvo_set_audio_state(intel_sdvo, 0);
+}
+
+static void intel_sdvo_enable_audio(struct intel_sdvo *intel_sdvo,
+				    const struct intel_crtc_state *crtc_state,
+				    const struct drm_connector_state *conn_state)
+{
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->base.adjusted_mode;
+	struct drm_connector *connector = conn_state->connector;
+	u8 *eld = connector->eld;
+
+	eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
+
+	intel_sdvo_set_audio_state(intel_sdvo, 0);
+
+	intel_sdvo_write_infoframe(intel_sdvo, SDVO_HBUF_INDEX_ELD,
+				   SDVO_HBUF_TX_DISABLED,
+				   eld, drm_eld_size(eld));
+
+	intel_sdvo_set_audio_state(intel_sdvo, SDVO_AUDIO_ELD_VALID |
+				   SDVO_AUDIO_PRESENCE_DETECT);
+}
+
 static void intel_disable_sdvo(struct intel_encoder *encoder,
 			       const struct intel_crtc_state *old_crtc_state,
 			       const struct drm_connector_state *conn_state)
@@ -1656,6 +1689,9 @@ static void intel_disable_sdvo(struct intel_encoder *encoder,
 	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
 	u32 temp;
 
+	if (old_crtc_state->has_audio)
+		intel_sdvo_disable_audio(intel_sdvo);
+
 	intel_sdvo_set_active_outputs(intel_sdvo, 0);
 	if (0)
 		intel_sdvo_set_encoder_power_state(intel_sdvo,
@@ -1741,6 +1777,9 @@ static void intel_enable_sdvo(struct intel_encoder *encoder,
 		intel_sdvo_set_encoder_power_state(intel_sdvo,
 						   DRM_MODE_DPMS_ON);
 	intel_sdvo_set_active_outputs(intel_sdvo, intel_sdvo->attached_output);
+
+	if (pipe_config->has_audio)
+		intel_sdvo_enable_audio(intel_sdvo, pipe_config, conn_state);
 }
 
 static enum drm_mode_status
@@ -2603,7 +2642,6 @@ static bool
 intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
 {
 	struct drm_encoder *encoder = &intel_sdvo->base.base;
-	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
 	struct drm_connector *connector;
 	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
 	struct intel_connector *intel_connector;
@@ -2640,9 +2678,7 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
 	encoder->encoder_type = DRM_MODE_ENCODER_TMDS;
 	connector->connector_type = DRM_MODE_CONNECTOR_DVID;
 
-	/* gen3 doesn't do the hdmi bits in the SDVO register */
-	if (INTEL_GEN(dev_priv) >= 4 &&
-	    intel_sdvo_is_hdmi_connector(intel_sdvo, device)) {
+	if (intel_sdvo_is_hdmi_connector(intel_sdvo, device)) {
 		connector->connector_type = DRM_MODE_CONNECTOR_HDMIA;
 		intel_sdvo_connector->is_hdmi = true;
 	}
diff --git a/drivers/gpu/drm/i915/intel_sdvo_regs.h b/drivers/gpu/drm/i915/intel_sdvo_regs.h
index db0ed499268a..e9ba3b047f93 100644
--- a/drivers/gpu/drm/i915/intel_sdvo_regs.h
+++ b/drivers/gpu/drm/i915/intel_sdvo_regs.h
@@ -707,6 +707,9 @@ struct intel_sdvo_enhancements_arg {
 #define SDVO_CMD_GET_AUDIO_ENCRYPT_PREFER 0x90
 #define SDVO_CMD_SET_AUDIO_STAT		0x91
 #define SDVO_CMD_GET_AUDIO_STAT		0x92
+  #define SDVO_AUDIO_ELD_VALID		(1 << 0)
+  #define SDVO_AUDIO_PRESENCE_DETECT	(1 << 1)
+  #define SDVO_AUDIO_CP_READY		(1 << 2)
 #define SDVO_CMD_SET_HBUF_INDEX		0x93
   #define SDVO_HBUF_INDEX_ELD		0
   #define SDVO_HBUF_INDEX_AVI_IF	1
-- 
2.21.0

[PATCH 2/7] drm/i915/sdvo: Implement proper HDMI audio support for SDVO

[Ville Syrjala]

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

Our SDVO audio support is pretty bogus. We can't push audio over the
SDVO bus, so trying to enable audio in the SDVO control register doesn't
do anything. In fact it looks like the SDVO encoder will always mix in
the audio coming over HDA, and there's no (at least documented) way to
disable that from our side. So HDMI audio does work currently but only by
luck really. What is missing though is the ELD.

To pass the ELD to the audio driver we need to write it to magic buffer
in the SDVO encoder hardware which then gets pulled out via HDA in the
other end. Ie. pretty much the same thing we had for native HDMI before
we started to just pass the ELD between the drivers. This sort of
explains why we even have that silly hardware buffer with native HDMI.

$ cat /proc/asound/card0/eld#1.0
-monitor_present		0
-eld_valid		0
+monitor_present		1
+eld_valid		1
+monitor_name		LG TV
+connection_type		HDMI
+...

This also fixes our state readout since we can now query the SDVO
encoder about the state of the "ELD valid" and "presence detect"
bits. As mentioned those don't actually control whether audio
gets sent over the HDMI cable, but it's the best we can do.

Cc: stable@vger.kernel.org
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: zardam@gmail.com
Tested-by: zardam@gmail.com
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108976
Fixes: de44e256b92c ("drm/i915/sdvo: Shut up state checker with hdmi cards on gen3")
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_sdvo.c      | 58 +++++++++++++++++++++-----
 drivers/gpu/drm/i915/intel_sdvo_regs.h |  3 ++
 2 files changed, 50 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 61db07244296..7f64352a3413 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -916,6 +916,13 @@ static bool intel_sdvo_set_colorimetry(struct intel_sdvo *intel_sdvo,
 	return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_COLORIMETRY, &mode, 1);
 }
 
+static bool intel_sdvo_set_audio_state(struct intel_sdvo *intel_sdvo,
+				       u8 audio_state)
+{
+	return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_AUDIO_STAT,
+				    &audio_state, 1);
+}
+
 #if 0
 static void intel_sdvo_dump_hdmi_buf(struct intel_sdvo *intel_sdvo)
 {
@@ -1487,11 +1494,6 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder,
 	else
 		sdvox |= SDVO_PIPE_SEL(crtc->pipe);
 
-	if (crtc_state->has_audio) {
-		WARN_ON_ONCE(INTEL_GEN(dev_priv) < 4);
-		sdvox |= SDVO_AUDIO_ENABLE;
-	}
-
 	if (INTEL_GEN(dev_priv) >= 4) {
 		/* done in crtc_mode_set as the dpll_md reg must be written early */
 	} else if (IS_I945G(dev_priv) || IS_I945GM(dev_priv) ||
@@ -1635,8 +1637,13 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
 	if (sdvox & HDMI_COLOR_RANGE_16_235)
 		pipe_config->limited_color_range = true;
 
-	if (sdvox & SDVO_AUDIO_ENABLE)
-		pipe_config->has_audio = true;
+	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_AUDIO_STAT,
+				 &val, 1)) {
+		u8 mask = SDVO_AUDIO_ELD_VALID | SDVO_AUDIO_PRESENCE_DETECT;
+
+		if ((val & mask) == mask)
+			pipe_config->has_audio = true;
+	}
 
 	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_ENCODE,
 				 &val, 1)) {
@@ -1647,6 +1654,32 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
 	intel_sdvo_get_avi_infoframe(intel_sdvo, pipe_config);
 }
 
+static void intel_sdvo_disable_audio(struct intel_sdvo *intel_sdvo)
+{
+	intel_sdvo_set_audio_state(intel_sdvo, 0);
+}
+
+static void intel_sdvo_enable_audio(struct intel_sdvo *intel_sdvo,
+				    const struct intel_crtc_state *crtc_state,
+				    const struct drm_connector_state *conn_state)
+{
+	const struct drm_display_mode *adjusted_mode =
+		&crtc_state->base.adjusted_mode;
+	struct drm_connector *connector = conn_state->connector;
+	u8 *eld = connector->eld;
+
+	eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
+
+	intel_sdvo_set_audio_state(intel_sdvo, 0);
+
+	intel_sdvo_write_infoframe(intel_sdvo, SDVO_HBUF_INDEX_ELD,
+				   SDVO_HBUF_TX_DISABLED,
+				   eld, drm_eld_size(eld));
+
+	intel_sdvo_set_audio_state(intel_sdvo, SDVO_AUDIO_ELD_VALID |
+				   SDVO_AUDIO_PRESENCE_DETECT);
+}
+
 static void intel_disable_sdvo(struct intel_encoder *encoder,
 			       const struct intel_crtc_state *old_crtc_state,
 			       const struct drm_connector_state *conn_state)
@@ -1656,6 +1689,9 @@ static void intel_disable_sdvo(struct intel_encoder *encoder,
 	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
 	u32 temp;
 
+	if (old_crtc_state->has_audio)
+		intel_sdvo_disable_audio(intel_sdvo);
+
 	intel_sdvo_set_active_outputs(intel_sdvo, 0);
 	if (0)
 		intel_sdvo_set_encoder_power_state(intel_sdvo,
@@ -1741,6 +1777,9 @@ static void intel_enable_sdvo(struct intel_encoder *encoder,
 		intel_sdvo_set_encoder_power_state(intel_sdvo,
 						   DRM_MODE_DPMS_ON);
 	intel_sdvo_set_active_outputs(intel_sdvo, intel_sdvo->attached_output);
+
+	if (pipe_config->has_audio)
+		intel_sdvo_enable_audio(intel_sdvo, pipe_config, conn_state);
 }
 
 static enum drm_mode_status
@@ -2603,7 +2642,6 @@ static bool
 intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
 {
 	struct drm_encoder *encoder = &intel_sdvo->base.base;
-	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
 	struct drm_connector *connector;
 	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
 	struct intel_connector *intel_connector;
@@ -2640,9 +2678,7 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
 	encoder->encoder_type = DRM_MODE_ENCODER_TMDS;
 	connector->connector_type = DRM_MODE_CONNECTOR_DVID;
 
-	/* gen3 doesn't do the hdmi bits in the SDVO register */
-	if (INTEL_GEN(dev_priv) >= 4 &&
-	    intel_sdvo_is_hdmi_connector(intel_sdvo, device)) {
+	if (intel_sdvo_is_hdmi_connector(intel_sdvo, device)) {
 		connector->connector_type = DRM_MODE_CONNECTOR_HDMIA;
 		intel_sdvo_connector->is_hdmi = true;
 	}
diff --git a/drivers/gpu/drm/i915/intel_sdvo_regs.h b/drivers/gpu/drm/i915/intel_sdvo_regs.h
index db0ed499268a..e9ba3b047f93 100644
--- a/drivers/gpu/drm/i915/intel_sdvo_regs.h
+++ b/drivers/gpu/drm/i915/intel_sdvo_regs.h
@@ -707,6 +707,9 @@ struct intel_sdvo_enhancements_arg {
 #define SDVO_CMD_GET_AUDIO_ENCRYPT_PREFER 0x90
 #define SDVO_CMD_SET_AUDIO_STAT		0x91
 #define SDVO_CMD_GET_AUDIO_STAT		0x92
+  #define SDVO_AUDIO_ELD_VALID		(1 << 0)
+  #define SDVO_AUDIO_PRESENCE_DETECT	(1 << 1)
+  #define SDVO_AUDIO_CP_READY		(1 << 2)
 #define SDVO_CMD_SET_HBUF_INDEX		0x93
   #define SDVO_HBUF_INDEX_ELD		0
   #define SDVO_HBUF_INDEX_AVI_IF	1
-- 
2.21.0

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

[PATCH 3/7] drm/i915: Rename SDVO_AUDIO_ENABLE to HDMI_AUDIO_ENABLE

[Ville Syrjala]

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

The "audio enable" bit on the SDVO/HDMI control register is only meant
for HDMI. Audio is never delivered over the SDVO bus. Rename the define
to reflect this fact.

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

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 9c206e803ab3..fe976c11a69e 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -4551,7 +4551,7 @@ enum {
 #define   HDMI_MODE_SELECT_HDMI			(1 << 9) /* HDMI only */
 #define   HDMI_MODE_SELECT_DVI			(0 << 9) /* HDMI only */
 #define   HDMI_COLOR_RANGE_16_235		(1 << 8) /* HDMI only */
-#define   SDVO_AUDIO_ENABLE			(1 << 6)
+#define   HDMI_AUDIO_ENABLE			(1 << 6) /* HDMI only */
 /* VSYNC/HSYNC bits new with 965, default is to be set */
 #define   SDVO_VSYNC_ACTIVE_HIGH		(1 << 4)
 #define   SDVO_HSYNC_ACTIVE_HIGH		(1 << 3)
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index e1005d7b75fd..b9b3aad94add 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1772,7 +1772,7 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 	if (pipe_config->infoframes.enable)
 		pipe_config->has_infoframe = true;
 
-	if (tmp & SDVO_AUDIO_ENABLE)
+	if (tmp & HDMI_AUDIO_ENABLE)
 		pipe_config->has_audio = true;
 
 	if (!HAS_PCH_SPLIT(dev_priv) &&
@@ -1831,7 +1831,7 @@ static void g4x_enable_hdmi(struct intel_encoder *encoder,
 
 	temp |= SDVO_ENABLE;
 	if (pipe_config->has_audio)
-		temp |= SDVO_AUDIO_ENABLE;
+		temp |= HDMI_AUDIO_ENABLE;
 
 	I915_WRITE(intel_hdmi->hdmi_reg, temp);
 	POSTING_READ(intel_hdmi->hdmi_reg);
@@ -1853,7 +1853,7 @@ static void ibx_enable_hdmi(struct intel_encoder *encoder,
 
 	temp |= SDVO_ENABLE;
 	if (pipe_config->has_audio)
-		temp |= SDVO_AUDIO_ENABLE;
+		temp |= HDMI_AUDIO_ENABLE;
 
 	/*
 	 * HW workaround, need to write this twice for issue
@@ -1905,7 +1905,7 @@ static void cpt_enable_hdmi(struct intel_encoder *encoder,
 
 	temp |= SDVO_ENABLE;
 	if (pipe_config->has_audio)
-		temp |= SDVO_AUDIO_ENABLE;
+		temp |= HDMI_AUDIO_ENABLE;
 
 	/*
 	 * WaEnableHDMI8bpcBefore12bpc:snb,ivb
@@ -1965,7 +1965,7 @@ static void intel_disable_hdmi(struct intel_encoder *encoder,
 
 	temp = I915_READ(intel_hdmi->hdmi_reg);
 
-	temp &= ~(SDVO_ENABLE | SDVO_AUDIO_ENABLE);
+	temp &= ~(SDVO_ENABLE | HDMI_AUDIO_ENABLE);
 	I915_WRITE(intel_hdmi->hdmi_reg, temp);
 	POSTING_READ(intel_hdmi->hdmi_reg);
 
-- 
2.21.0

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

[PATCH 4/7] drm/i915/sdvo: Check that we have space for the infoframe

[Ville Syrjala]

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

Before we go writing the infoframe let's make sure we have
the space for it. Not that it really matters since the write
loop would just terminate early in that case.

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

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 7f64352a3413..1e0102f1710f 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -970,6 +970,9 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
 				  &hbuf_size, 1))
 		return false;
 
+	if (hbuf_size < length)
+		return false;
+
 	/* Buffer size is 0 based, hooray! */
 	hbuf_size++;
 
-- 
2.21.0

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

[PATCH 5/7] drm/i915/sdvo: Don't unpack stack garbage

[Ville Syrjala]

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

Pass the length returned by intel_sdvo_read_infoframe() to
hdmi_infoframe_unpack() so that we don't try to unpack any
leftover stack garbage.

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

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 1e0102f1710f..4e701c8f8971 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1128,7 +1128,7 @@ static void intel_sdvo_get_avi_infoframe(struct intel_sdvo *intel_sdvo,
 	crtc_state->infoframes.enable |=
 		intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_AVI);
 
-	ret = hdmi_infoframe_unpack(frame, sdvo_data, sizeof(sdvo_data));
+	ret = hdmi_infoframe_unpack(frame, sdvo_data, len);
 	if (ret) {
 		DRM_DEBUG_KMS("Failed to unpack AVI infoframe\n");
 		return;
-- 
2.21.0

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

[PATCH 6/7] drm/i915/sdvo: Don't write stack garbage into the hbuf

[Ville Syrjala]

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

Pass the length returned by hdmi_infoframe_pack_only() to
intel_sdvo_write_infoframe() so that we don't end up writing
stack garbage into the hbuf.

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

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 4e701c8f8971..d5a95eca23ba 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1102,7 +1102,7 @@ static bool intel_sdvo_set_avi_infoframe(struct intel_sdvo *intel_sdvo,
 
 	return intel_sdvo_write_infoframe(intel_sdvo, SDVO_HBUF_INDEX_AVI_IF,
 					  SDVO_HBUF_TX_VSYNC,
-					  sdvo_data, sizeof(sdvo_data));
+					  sdvo_data, len);
 }
 
 static void intel_sdvo_get_avi_infoframe(struct intel_sdvo *intel_sdvo,
-- 
2.21.0

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

[PATCH 7/7] drm/i915/sdvo: Actually print the reason why the SDVO command failed

[Ville Syrjala]

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

It's much easier to figure out why the SDVO encoder refuses to cooperate
if we can see what status we got back.

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

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index d5a95eca23ba..5d928f6d0028 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -516,7 +516,7 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
 	u8 status;
 	int i, pos = 0;
 #define BUF_LEN 256
-	char buffer[BUF_LEN];
+	char buffer[BUF_LEN] = {};
 
 
 	/*
@@ -581,7 +581,8 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
 	return true;
 
 log_fail:
-	DRM_DEBUG_KMS("%s: R: ... failed\n", SDVO_NAME(intel_sdvo));
+	DRM_DEBUG_KMS("%s: R: ... failed %s\n",
+		      SDVO_NAME(intel_sdvo), buffer);
 	return false;
 }
 
-- 
2.21.0

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

✓ Fi.CI.BAT: success for drm/i915: Fix SDVO HDMI audio

[Patchwork]

== Series Details ==

Series: drm/i915: Fix SDVO HDMI audio
URL   : https://patchwork.freedesktop.org/series/59233/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5897 -> Patchwork_12739
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@sync-fork-compute0:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109315] +17

  * igt@gem_exec_basic@gtt-bsd1:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109276] +7

  * igt@gem_exec_parse@basic-rejected:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109289] +1

  * igt@gem_exec_store@basic-bsd1:
    - fi-kbl-r:           NOTRUN -> SKIP [fdo#109271] +41

  * igt@i915_selftest@live_contexts:
    - fi-icl-u3:          NOTRUN -> DMESG-FAIL [fdo#108569]

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         PASS -> INCOMPLETE [fdo#103927] / [fdo#109720]

  * igt@i915_selftest@live_hangcheck:
    - fi-bxt-dsi:         PASS -> INCOMPLETE [fdo#103927]

  * igt@kms_busy@basic-flip-a:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_busy@basic-flip-c:
    - fi-byt-j1900:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] +62
    - fi-byt-j1900:       NOTRUN -> SKIP [fdo#109271] +52

  * igt@kms_chamelium@hdmi-edid-read:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109284] +8

  * igt@kms_force_connector_basic@prune-stale-modes:
    - fi-icl-u3:          NOTRUN -> SKIP [fdo#109285] +3

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u3:          NOTRUN -> FAIL [fdo#103167]

  * igt@runner@aborted:
    - fi-glk-dsi:         NOTRUN -> FAIL [k.org#202321]
    - fi-apl-guc:         NOTRUN -> FAIL [fdo#108622] / [fdo#109720]

  
#### Possible fixes ####

  * igt@gem_ctx_exec@basic:
    - fi-icl-u3:          INCOMPLETE [fdo#107713] -> PASS

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS
    - fi-skl-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS

  
#### Warnings ####

  * igt@i915_pm_rpm@module-reload:
    - fi-glk-dsi:         INCOMPLETE [fdo#103359] / [k.org#198133] -> DMESG-WARN [fdo#105538] / [fdo#107732] / [fdo#109513]

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#105538]: https://bugs.freedesktop.org/show_bug.cgi?id=105538
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107732]: https://bugs.freedesktop.org/show_bug.cgi?id=107732
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108622]: https://bugs.freedesktop.org/show_bug.cgi?id=108622
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109513]: https://bugs.freedesktop.org/show_bug.cgi?id=109513
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133
  [k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321


Participating hosts (44 -> 44)
------------------------------

  Additional (3): fi-byt-j1900 fi-kbl-r fi-bsw-n3050 
  Missing    (3): fi-byt-squawks fi-bsw-cyan fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5897 -> Patchwork_12739

  CI_DRM_5897: 7d07e025e78603d6270bc115fdb6c1efea6e66a5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4934: dc4f45eb6874331daec870dc1e4cfc3ac5c49311 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12739: 35b215101d202bb3f213620d23029e0e09f5c65b @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

35b215101d20 drm/i915/sdvo: Actually print the reason why the SDVO command failed
f283258eaf5e drm/i915/sdvo: Don't write stack garbage into the hbuf
780194ab786d drm/i915/sdvo: Don't unpack stack garbage
acff651b8dfc drm/i915/sdvo: Check that we have space for the infoframe
a1c898c367ad drm/i915: Rename SDVO_AUDIO_ENABLE to HDMI_AUDIO_ENABLE
366debf8e447 drm/i915/sdvo: Implement proper HDMI audio support for SDVO
eaaa5efcbbc1 drm/i915/sdvo: Fix AVI infoframe TX rate readout

== Logs ==

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

Re: [PATCH 5/7] drm/i915/sdvo: Don't unpack stack garbage

[Chris Wilson]

Quoting Ville Syrjala (2019-04-09 15:40:52)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Pass the length returned by intel_sdvo_read_infoframe() to
> hdmi_infoframe_unpack() so that we don't try to unpack any
> leftover stack garbage.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/7] drm/i915/sdvo: Don't write stack garbage into the hbuf

[Chris Wilson]

Quoting Ville Syrjala (2019-04-09 15:40:53)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Pass the length returned by hdmi_infoframe_pack_only() to
> intel_sdvo_write_infoframe() so that we don't end up writing
> stack garbage into the hbuf.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Ok, write_infoframe 0 extends if len is too short.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/7] drm/i915/sdvo: Actually print the reason why the SDVO command failed

[Chris Wilson]

Quoting Ville Syrjala (2019-04-09 15:40:54)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> It's much easier to figure out why the SDVO encoder refuses to cooperate
> if we can see what status we got back.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sdvo.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index d5a95eca23ba..5d928f6d0028 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -516,7 +516,7 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
>         u8 status;
>         int i, pos = 0;
>  #define BUF_LEN 256
> -       char buffer[BUF_LEN];
> +       char buffer[BUF_LEN] = {};

I should stop quibbling over a 256b memset.

>         /*
> @@ -581,7 +581,8 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
>         return true;
>  
>  log_fail:
> -       DRM_DEBUG_KMS("%s: R: ... failed\n", SDVO_NAME(intel_sdvo));
> +       DRM_DEBUG_KMS("%s: R: ... failed %s\n",
> +                     SDVO_NAME(intel_sdvo), buffer);

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915/sdvo: Check that we have space for the infoframe

[Chris Wilson]

Quoting Ville Syrjala (2019-04-09 15:40:51)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Before we go writing the infoframe let's make sure we have
> the space for it. Not that it really matters since the write
> loop would just terminate early in that case.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 7f64352a3413..1e0102f1710f 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -970,6 +970,9 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
>                                   &hbuf_size, 1))
>                 return false;
>  
> +       if (hbuf_size < length)
> +               return false;
> +

Maybe after reporting the hbuf_size and length in the following
DRM_DEBUG_KMS?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/7] drm/i915/sdvo: Check that we have space for the infoframe

[Ville Syrjälä]

On Tue, Apr 09, 2019 at 08:46:42PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-04-09 15:40:51)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Before we go writing the infoframe let's make sure we have
> > the space for it. Not that it really matters since the write
> > loop would just terminate early in that case.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > index 7f64352a3413..1e0102f1710f 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -970,6 +970,9 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
> >                                   &hbuf_size, 1))
> >                 return false;
> >  
> > +       if (hbuf_size < length)
> > +               return false;
> > +
> 
> Maybe after reporting the hbuf_size and length in the following
> DRM_DEBUG_KMS?

Yes, that does seem more sensible. Avoids the off by one as well.

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

Re: [PATCH 2/7] drm/i915/sdvo: Implement proper HDMI audio support for SDVO

[Ville Syrjälä]

On Tue, Apr 09, 2019 at 05:40:49PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Our SDVO audio support is pretty bogus. We can't push audio over the
> SDVO bus, so trying to enable audio in the SDVO control register doesn't
> do anything. In fact it looks like the SDVO encoder will always mix in
> the audio coming over HDA, and there's no (at least documented) way to
> disable that from our side. So HDMI audio does work currently but only by
> luck really. What is missing though is the ELD.

Hmm. Looks like I forgot to update this text after the gen3 bug was
reported. The situation is that audio works on gen4 by luck. On gen3
it got broken by the referenced commit since we no longer enable
HDMI encoding on the SDVO device (that will stop audio transmission
entirely).

> 
> To pass the ELD to the audio driver we need to write it to magic buffer
> in the SDVO encoder hardware which then gets pulled out via HDA in the
> other end. Ie. pretty much the same thing we had for native HDMI before
> we started to just pass the ELD between the drivers. This sort of
> explains why we even have that silly hardware buffer with native HDMI.
> 
> $ cat /proc/asound/card0/eld#1.0
> -monitor_present		0
> -eld_valid		0
> +monitor_present		1
> +eld_valid		1
> +monitor_name		LG TV
> +connection_type		HDMI
> +...
> 
> This also fixes our state readout since we can now query the SDVO
> encoder about the state of the "ELD valid" and "presence detect"
> bits. As mentioned those don't actually control whether audio
> gets sent over the HDMI cable, but it's the best we can do.
> 
> Cc: stable@vger.kernel.org
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: zardam@gmail.com
> Tested-by: zardam@gmail.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108976
> Fixes: de44e256b92c ("drm/i915/sdvo: Shut up state checker with hdmi cards on gen3")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sdvo.c      | 58 +++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_sdvo_regs.h |  3 ++
>  2 files changed, 50 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 61db07244296..7f64352a3413 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -916,6 +916,13 @@ static bool intel_sdvo_set_colorimetry(struct intel_sdvo *intel_sdvo,
>  	return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_COLORIMETRY, &mode, 1);
>  }
>  
> +static bool intel_sdvo_set_audio_state(struct intel_sdvo *intel_sdvo,
> +				       u8 audio_state)
> +{
> +	return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_AUDIO_STAT,
> +				    &audio_state, 1);
> +}
> +
>  #if 0
>  static void intel_sdvo_dump_hdmi_buf(struct intel_sdvo *intel_sdvo)
>  {
> @@ -1487,11 +1494,6 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder,
>  	else
>  		sdvox |= SDVO_PIPE_SEL(crtc->pipe);
>  
> -	if (crtc_state->has_audio) {
> -		WARN_ON_ONCE(INTEL_GEN(dev_priv) < 4);
> -		sdvox |= SDVO_AUDIO_ENABLE;
> -	}
> -
>  	if (INTEL_GEN(dev_priv) >= 4) {
>  		/* done in crtc_mode_set as the dpll_md reg must be written early */
>  	} else if (IS_I945G(dev_priv) || IS_I945GM(dev_priv) ||
> @@ -1635,8 +1637,13 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
>  	if (sdvox & HDMI_COLOR_RANGE_16_235)
>  		pipe_config->limited_color_range = true;
>  
> -	if (sdvox & SDVO_AUDIO_ENABLE)
> -		pipe_config->has_audio = true;
> +	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_AUDIO_STAT,
> +				 &val, 1)) {
> +		u8 mask = SDVO_AUDIO_ELD_VALID | SDVO_AUDIO_PRESENCE_DETECT;
> +
> +		if ((val & mask) == mask)
> +			pipe_config->has_audio = true;
> +	}
>  
>  	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_ENCODE,
>  				 &val, 1)) {
> @@ -1647,6 +1654,32 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
>  	intel_sdvo_get_avi_infoframe(intel_sdvo, pipe_config);
>  }
>  
> +static void intel_sdvo_disable_audio(struct intel_sdvo *intel_sdvo)
> +{
> +	intel_sdvo_set_audio_state(intel_sdvo, 0);
> +}
> +
> +static void intel_sdvo_enable_audio(struct intel_sdvo *intel_sdvo,
> +				    const struct intel_crtc_state *crtc_state,
> +				    const struct drm_connector_state *conn_state)
> +{
> +	const struct drm_display_mode *adjusted_mode =
> +		&crtc_state->base.adjusted_mode;
> +	struct drm_connector *connector = conn_state->connector;
> +	u8 *eld = connector->eld;
> +
> +	eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
> +
> +	intel_sdvo_set_audio_state(intel_sdvo, 0);
> +
> +	intel_sdvo_write_infoframe(intel_sdvo, SDVO_HBUF_INDEX_ELD,
> +				   SDVO_HBUF_TX_DISABLED,
> +				   eld, drm_eld_size(eld));
> +
> +	intel_sdvo_set_audio_state(intel_sdvo, SDVO_AUDIO_ELD_VALID |
> +				   SDVO_AUDIO_PRESENCE_DETECT);
> +}
> +
>  static void intel_disable_sdvo(struct intel_encoder *encoder,
>  			       const struct intel_crtc_state *old_crtc_state,
>  			       const struct drm_connector_state *conn_state)
> @@ -1656,6 +1689,9 @@ static void intel_disable_sdvo(struct intel_encoder *encoder,
>  	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
>  	u32 temp;
>  
> +	if (old_crtc_state->has_audio)
> +		intel_sdvo_disable_audio(intel_sdvo);
> +
>  	intel_sdvo_set_active_outputs(intel_sdvo, 0);
>  	if (0)
>  		intel_sdvo_set_encoder_power_state(intel_sdvo,
> @@ -1741,6 +1777,9 @@ static void intel_enable_sdvo(struct intel_encoder *encoder,
>  		intel_sdvo_set_encoder_power_state(intel_sdvo,
>  						   DRM_MODE_DPMS_ON);
>  	intel_sdvo_set_active_outputs(intel_sdvo, intel_sdvo->attached_output);
> +
> +	if (pipe_config->has_audio)
> +		intel_sdvo_enable_audio(intel_sdvo, pipe_config, conn_state);
>  }
>  
>  static enum drm_mode_status
> @@ -2603,7 +2642,6 @@ static bool
>  intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
>  {
>  	struct drm_encoder *encoder = &intel_sdvo->base.base;
> -	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
>  	struct drm_connector *connector;
>  	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
>  	struct intel_connector *intel_connector;
> @@ -2640,9 +2678,7 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
>  	encoder->encoder_type = DRM_MODE_ENCODER_TMDS;
>  	connector->connector_type = DRM_MODE_CONNECTOR_DVID;
>  
> -	/* gen3 doesn't do the hdmi bits in the SDVO register */
> -	if (INTEL_GEN(dev_priv) >= 4 &&
> -	    intel_sdvo_is_hdmi_connector(intel_sdvo, device)) {
> +	if (intel_sdvo_is_hdmi_connector(intel_sdvo, device)) {
>  		connector->connector_type = DRM_MODE_CONNECTOR_HDMIA;
>  		intel_sdvo_connector->is_hdmi = true;
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_sdvo_regs.h b/drivers/gpu/drm/i915/intel_sdvo_regs.h
> index db0ed499268a..e9ba3b047f93 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo_regs.h
> +++ b/drivers/gpu/drm/i915/intel_sdvo_regs.h
> @@ -707,6 +707,9 @@ struct intel_sdvo_enhancements_arg {
>  #define SDVO_CMD_GET_AUDIO_ENCRYPT_PREFER 0x90
>  #define SDVO_CMD_SET_AUDIO_STAT		0x91
>  #define SDVO_CMD_GET_AUDIO_STAT		0x92
> +  #define SDVO_AUDIO_ELD_VALID		(1 << 0)
> +  #define SDVO_AUDIO_PRESENCE_DETECT	(1 << 1)
> +  #define SDVO_AUDIO_CP_READY		(1 << 2)
>  #define SDVO_CMD_SET_HBUF_INDEX		0x93
>    #define SDVO_HBUF_INDEX_ELD		0
>    #define SDVO_HBUF_INDEX_AVI_IF	1
> -- 
> 2.21.0

-- 
Ville Syrjälä
Intel

Re: [PATCH 2/7] drm/i915/sdvo: Implement proper HDMI audio support for SDVO

[Ville Syrjälä]

On Tue, Apr 09, 2019 at 05:40:49PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Our SDVO audio support is pretty bogus. We can't push audio over the
> SDVO bus, so trying to enable audio in the SDVO control register doesn't
> do anything. In fact it looks like the SDVO encoder will always mix in
> the audio coming over HDA, and there's no (at least documented) way to
> disable that from our side. So HDMI audio does work currently but only by
> luck really. What is missing though is the ELD.

Hmm. Looks like I forgot to update this text after the gen3 bug was
reported. The situation is that audio works on gen4 by luck. On gen3
it got broken by the referenced commit since we no longer enable
HDMI encoding on the SDVO device (that will stop audio transmission
entirely).

> 
> To pass the ELD to the audio driver we need to write it to magic buffer
> in the SDVO encoder hardware which then gets pulled out via HDA in the
> other end. Ie. pretty much the same thing we had for native HDMI before
> we started to just pass the ELD between the drivers. This sort of
> explains why we even have that silly hardware buffer with native HDMI.
> 
> $ cat /proc/asound/card0/eld#1.0
> -monitor_present		0
> -eld_valid		0
> +monitor_present		1
> +eld_valid		1
> +monitor_name		LG TV
> +connection_type		HDMI
> +...
> 
> This also fixes our state readout since we can now query the SDVO
> encoder about the state of the "ELD valid" and "presence detect"
> bits. As mentioned those don't actually control whether audio
> gets sent over the HDMI cable, but it's the best we can do.
> 
> Cc: stable@vger.kernel.org
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: zardam@gmail.com
> Tested-by: zardam@gmail.com
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108976
> Fixes: de44e256b92c ("drm/i915/sdvo: Shut up state checker with hdmi cards on gen3")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sdvo.c      | 58 +++++++++++++++++++++-----
>  drivers/gpu/drm/i915/intel_sdvo_regs.h |  3 ++
>  2 files changed, 50 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 61db07244296..7f64352a3413 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -916,6 +916,13 @@ static bool intel_sdvo_set_colorimetry(struct intel_sdvo *intel_sdvo,
>  	return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_COLORIMETRY, &mode, 1);
>  }
>  
> +static bool intel_sdvo_set_audio_state(struct intel_sdvo *intel_sdvo,
> +				       u8 audio_state)
> +{
> +	return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_AUDIO_STAT,
> +				    &audio_state, 1);
> +}
> +
>  #if 0
>  static void intel_sdvo_dump_hdmi_buf(struct intel_sdvo *intel_sdvo)
>  {
> @@ -1487,11 +1494,6 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder,
>  	else
>  		sdvox |= SDVO_PIPE_SEL(crtc->pipe);
>  
> -	if (crtc_state->has_audio) {
> -		WARN_ON_ONCE(INTEL_GEN(dev_priv) < 4);
> -		sdvox |= SDVO_AUDIO_ENABLE;
> -	}
> -
>  	if (INTEL_GEN(dev_priv) >= 4) {
>  		/* done in crtc_mode_set as the dpll_md reg must be written early */
>  	} else if (IS_I945G(dev_priv) || IS_I945GM(dev_priv) ||
> @@ -1635,8 +1637,13 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
>  	if (sdvox & HDMI_COLOR_RANGE_16_235)
>  		pipe_config->limited_color_range = true;
>  
> -	if (sdvox & SDVO_AUDIO_ENABLE)
> -		pipe_config->has_audio = true;
> +	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_AUDIO_STAT,
> +				 &val, 1)) {
> +		u8 mask = SDVO_AUDIO_ELD_VALID | SDVO_AUDIO_PRESENCE_DETECT;
> +
> +		if ((val & mask) == mask)
> +			pipe_config->has_audio = true;
> +	}
>  
>  	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_ENCODE,
>  				 &val, 1)) {
> @@ -1647,6 +1654,32 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
>  	intel_sdvo_get_avi_infoframe(intel_sdvo, pipe_config);
>  }
>  
> +static void intel_sdvo_disable_audio(struct intel_sdvo *intel_sdvo)
> +{
> +	intel_sdvo_set_audio_state(intel_sdvo, 0);
> +}
> +
> +static void intel_sdvo_enable_audio(struct intel_sdvo *intel_sdvo,
> +				    const struct intel_crtc_state *crtc_state,
> +				    const struct drm_connector_state *conn_state)
> +{
> +	const struct drm_display_mode *adjusted_mode =
> +		&crtc_state->base.adjusted_mode;
> +	struct drm_connector *connector = conn_state->connector;
> +	u8 *eld = connector->eld;
> +
> +	eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
> +
> +	intel_sdvo_set_audio_state(intel_sdvo, 0);
> +
> +	intel_sdvo_write_infoframe(intel_sdvo, SDVO_HBUF_INDEX_ELD,
> +				   SDVO_HBUF_TX_DISABLED,
> +				   eld, drm_eld_size(eld));
> +
> +	intel_sdvo_set_audio_state(intel_sdvo, SDVO_AUDIO_ELD_VALID |
> +				   SDVO_AUDIO_PRESENCE_DETECT);
> +}
> +
>  static void intel_disable_sdvo(struct intel_encoder *encoder,
>  			       const struct intel_crtc_state *old_crtc_state,
>  			       const struct drm_connector_state *conn_state)
> @@ -1656,6 +1689,9 @@ static void intel_disable_sdvo(struct intel_encoder *encoder,
>  	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
>  	u32 temp;
>  
> +	if (old_crtc_state->has_audio)
> +		intel_sdvo_disable_audio(intel_sdvo);
> +
>  	intel_sdvo_set_active_outputs(intel_sdvo, 0);
>  	if (0)
>  		intel_sdvo_set_encoder_power_state(intel_sdvo,
> @@ -1741,6 +1777,9 @@ static void intel_enable_sdvo(struct intel_encoder *encoder,
>  		intel_sdvo_set_encoder_power_state(intel_sdvo,
>  						   DRM_MODE_DPMS_ON);
>  	intel_sdvo_set_active_outputs(intel_sdvo, intel_sdvo->attached_output);
> +
> +	if (pipe_config->has_audio)
> +		intel_sdvo_enable_audio(intel_sdvo, pipe_config, conn_state);
>  }
>  
>  static enum drm_mode_status
> @@ -2603,7 +2642,6 @@ static bool
>  intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
>  {
>  	struct drm_encoder *encoder = &intel_sdvo->base.base;
> -	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
>  	struct drm_connector *connector;
>  	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
>  	struct intel_connector *intel_connector;
> @@ -2640,9 +2678,7 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
>  	encoder->encoder_type = DRM_MODE_ENCODER_TMDS;
>  	connector->connector_type = DRM_MODE_CONNECTOR_DVID;
>  
> -	/* gen3 doesn't do the hdmi bits in the SDVO register */
> -	if (INTEL_GEN(dev_priv) >= 4 &&
> -	    intel_sdvo_is_hdmi_connector(intel_sdvo, device)) {
> +	if (intel_sdvo_is_hdmi_connector(intel_sdvo, device)) {
>  		connector->connector_type = DRM_MODE_CONNECTOR_HDMIA;
>  		intel_sdvo_connector->is_hdmi = true;
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_sdvo_regs.h b/drivers/gpu/drm/i915/intel_sdvo_regs.h
> index db0ed499268a..e9ba3b047f93 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo_regs.h
> +++ b/drivers/gpu/drm/i915/intel_sdvo_regs.h
> @@ -707,6 +707,9 @@ struct intel_sdvo_enhancements_arg {
>  #define SDVO_CMD_GET_AUDIO_ENCRYPT_PREFER 0x90
>  #define SDVO_CMD_SET_AUDIO_STAT		0x91
>  #define SDVO_CMD_GET_AUDIO_STAT		0x92
> +  #define SDVO_AUDIO_ELD_VALID		(1 << 0)
> +  #define SDVO_AUDIO_PRESENCE_DETECT	(1 << 1)
> +  #define SDVO_AUDIO_CP_READY		(1 << 2)
>  #define SDVO_CMD_SET_HBUF_INDEX		0x93
>    #define SDVO_HBUF_INDEX_ELD		0
>    #define SDVO_HBUF_INDEX_AVI_IF	1
> -- 
> 2.21.0

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

Re: [PATCH 7/7] drm/i915/sdvo: Actually print the reason why the SDVO command failed

[Ville Syrjälä]

On Tue, Apr 09, 2019 at 08:44:26PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-04-09 15:40:54)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > It's much easier to figure out why the SDVO encoder refuses to cooperate
> > if we can see what status we got back.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_sdvo.c | 5 +++--
> >  1 file changed, 3 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > index d5a95eca23ba..5d928f6d0028 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -516,7 +516,7 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
> >         u8 status;
> >         int i, pos = 0;
> >  #define BUF_LEN 256
> > -       char buffer[BUF_LEN];
> > +       char buffer[BUF_LEN] = {};
> 
> I should stop quibbling over a 256b memset.

Hmm. I wonder if 256 bytes isn't a bit excessive actually.

Max 8 responses 3 chars each, and 21 or so bytes for the status string.
Comes to a total of 45 chars. A bit more for intel_sdvo_debug_write()
since it wants to print the command name.

> 
> >         /*
> > @@ -581,7 +581,8 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
> >         return true;
> >  
> >  log_fail:
> > -       DRM_DEBUG_KMS("%s: R: ... failed\n", SDVO_NAME(intel_sdvo));
> > +       DRM_DEBUG_KMS("%s: R: ... failed %s\n",
> > +                     SDVO_NAME(intel_sdvo), buffer);
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> -Chris

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

Re: [PATCH 4/7] drm/i915/sdvo: Check that we have space for the infoframe

[Chris Wilson]

Quoting Ville Syrjälä (2019-04-09 20:59:43)
> On Tue, Apr 09, 2019 at 08:46:42PM +0100, Chris Wilson wrote:
> > Quoting Ville Syrjala (2019-04-09 15:40:51)
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Before we go writing the infoframe let's make sure we have
> > > the space for it. Not that it really matters since the write
> > > loop would just terminate early in that case.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > > index 7f64352a3413..1e0102f1710f 100644
> > > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > > @@ -970,6 +970,9 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
> > >                                   &hbuf_size, 1))
> > >                 return false;
> > >  
> > > +       if (hbuf_size < length)
> > > +               return false;
> > > +
> > 
> > Maybe after reporting the hbuf_size and length in the following
> > DRM_DEBUG_KMS?
> 
> Yes, that does seem more sensible. Avoids the off by one as well.

Ok, predictive
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 1/7] drm/i915/sdvo: Fix AVI infoframe TX rate readout

[Chris Wilson]

Quoting Ville Syrjala (2019-04-09 15:40:48)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The AVI infoframe readout code currently issues a
> SDVO_CMD_GET_HBUF_TXRATE before SDVO_CMD_SET_HBUF_INDEX, which is
> not the correct order for these two operations. So far this wasn't
> a problem since we left the index pointing at the AVI infoframe
> buffer at the end of the modeset. However once we start to write
> to other buffers (namely ELD) that is no longer going to be true.
> Fix up the order so that we always read out the TX rate for the
> correct buffer.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sdvo.c | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> index 0e3d91d9ef13..61db07244296 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -1001,6 +1001,11 @@ static ssize_t intel_sdvo_read_infoframe(struct intel_sdvo *intel_sdvo,
>         if (av_split < if_index)
>                 return 0;
>  
> +       if (!intel_sdvo_set_value(intel_sdvo,
> +                                 SDVO_CMD_SET_HBUF_INDEX,
> +                                 set_buf_index, 2))
> +               return -ENXIO;

That is consistent with write_infoframe and makes a modicum of sense.
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Why is the av_split separate from the if_index? What does that mean, at
some point I might have to read the docs!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/7] drm/i915: Rename SDVO_AUDIO_ENABLE to HDMI_AUDIO_ENABLE

[Chris Wilson]

Quoting Ville Syrjala (2019-04-09 15:40:50)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> The "audio enable" bit on the SDVO/HDMI control register is only meant
> for HDMI. Audio is never delivered over the SDVO bus. Rename the define
> to reflect this fact.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Just going by that it's only used by the hdmi code :)
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.IGT: failure for drm/i915: Fix SDVO HDMI audio

[Patchwork]

== Series Details ==

Series: drm/i915: Fix SDVO HDMI audio
URL   : https://patchwork.freedesktop.org/series/59233/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5897_full -> Patchwork_12739_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_12739_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_12739_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_12739_full:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_panel_fitting@legacy:
    - shard-iclb:         NOTRUN -> INCOMPLETE

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-iclb:         PASS -> INCOMPLETE

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@in-flight-suspend:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#108566] +1

  * igt@gem_eio@suspend:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#103313] / [fdo#103558] / [fdo#105079] / [fdo#105602]

  * igt@gem_stolen@stolen-clear:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109277] +1

  * igt@gem_tiled_fence_blits@normal:
    - shard-iclb:         PASS -> TIMEOUT [fdo#109673]

  * igt@i915_pm_rpm@gem-execbuf-stress-pc8:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109506]

  * igt@i915_pm_rpm@modeset-stress-extra-wait:
    - shard-skl:          NOTRUN -> INCOMPLETE [fdo#107807]

  * igt@i915_suspend@sysfs-reader:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103375]

  * igt@kms_atomic_transition@6x-modeset-transitions:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278] +1

  * igt@kms_atomic_transition@6x-modeset-transitions-nonblocking:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +13

  * igt@kms_chamelium@vga-edid-read:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109284]

  * igt@kms_content_protection@atomic:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109300]

  * igt@kms_content_protection@atomic-dpms:
    - shard-apl:          NOTRUN -> FAIL [fdo#110321] / [fdo#110336]

  * igt@kms_crtc_background_color:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109305]

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-glk:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_flip@2x-flip-vs-absolute-wf_vblank-interruptible:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274] +2

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-apl:          PASS -> FAIL [fdo#102887] / [fdo#105363]

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-pri-indfb-draw-blt:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +154

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
    - shard-iclb:         PASS -> FAIL [fdo#103167] +6

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-blt:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +7

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-spr-indfb-fullscreen:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +16

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-draw-mmap-gtt:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +4

  * igt@kms_lease@setcrtc_implicit_plane:
    - shard-apl:          NOTRUN -> FAIL [fdo#110281]

  * igt@kms_panel_fitting@legacy:
    - shard-skl:          NOTRUN -> FAIL [fdo#105456]

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-c:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#103313] / [fdo#103558] / [fdo#105602] +3

  * igt@kms_pipe_crc_basic@read-crc-pipe-d:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +10

  * igt@kms_plane@pixel-format-pipe-a-planes-source-clamping:
    - shard-glk:          PASS -> SKIP [fdo#109271]

  * igt@kms_plane_alpha_blend@pipe-a-alpha-7efc:
    - shard-skl:          NOTRUN -> FAIL [fdo#107815] / [fdo#108145] +1

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-alpha-basic:
    - shard-apl:          NOTRUN -> FAIL [fdo#108145] +3

  * igt@kms_plane_scaling@pipe-b-scaler-with-clipping-clamping:
    - shard-glk:          PASS -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         PASS -> SKIP [fdo#109441] +1

  * igt@kms_psr@sprite_mmap_gtt:
    - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215] +5

  * igt@kms_tv_load_detect@load-detect:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109309]

  * igt@perf_pmu@busy-accuracy-50-vcs1:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +7

  * igt@prime_vgem@sync-bsd1:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +195

  
#### Possible fixes ####

  * igt@gem_mmap_gtt@forked-big-copy-xy:
    - shard-iclb:         TIMEOUT [fdo#109673] -> PASS

  * igt@gem_ppgtt@blt-vs-render-ctx0:
    - shard-iclb:         INCOMPLETE [fdo#109801] -> PASS +1

  * igt@i915_pm_rpm@system-suspend-execbuf:
    - shard-glk:          DMESG-WARN [fdo#109513] -> PASS

  * igt@i915_selftest@live_workarounds:
    - shard-iclb:         DMESG-FAIL [fdo#108954] -> PASS

  * igt@kms_cursor_legacy@cursor-vs-flip-varying-size:
    - shard-iclb:         FAIL [fdo#103355] -> PASS +1

  * igt@kms_flip@2x-flip-vs-expired-vblank-interruptible:
    - shard-glk:          FAIL [fdo#102887] -> PASS

  * igt@kms_flip@flip-vs-expired-vblank:
    - shard-glk:          FAIL [fdo#102887] / [fdo#105363] -> PASS

  * igt@kms_flip@flip-vs-rmfb:
    - shard-iclb:         INCOMPLETE [fdo#107713] -> PASS

  * igt@kms_flip@flip-vs-suspend:
    - shard-iclb:         FAIL [fdo#103375] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-rgb565-draw-pwrite:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-blt:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +16

  * igt@kms_psr@no_drrs:
    - shard-iclb:         FAIL [fdo#108341] -> PASS

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +1

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          FAIL [fdo#109016] -> PASS +1

  
#### Warnings ####

  * igt@runner@aborted:
    - shard-glk:          ( 2 FAIL ) [fdo#109373] / [k.org#202321] -> FAIL [fdo#109373] / [k.org#202321]

  
  [fdo#102887]: https://bugs.freedesktop.org/show_bug.cgi?id=102887
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103313]: https://bugs.freedesktop.org/show_bug.cgi?id=103313
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103558]: https://bugs.freedesktop.org/show_bug.cgi?id=103558
  [fdo#105079]: https://bugs.freedesktop.org/show_bug.cgi?id=105079
  [fdo#105363]: https://bugs.freedesktop.org/show_bug.cgi?id=105363
  [fdo#105456]: https://bugs.freedesktop.org/show_bug.cgi?id=105456
  [fdo#105602]: https://bugs.freedesktop.org/show_bug.cgi?id=105602
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#107815]: https://bugs.freedesktop.org/show_bug.cgi?id=107815
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108341]: https://bugs.freedesktop.org/show_bug.cgi?id=108341
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108954]: https://bugs.freedesktop.org/show_bug.cgi?id=108954
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109277]: https://bugs.freedesktop.org/show_bug.cgi?id=109277
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109300]: https://bugs.freedesktop.org/show_bug.cgi?id=109300
  [fdo#109305]: https://bugs.freedesktop.org/show_bug.cgi?id=109305
  [fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
  [fdo#109373]: https://bugs.freedesktop.org/show_bug.cgi?id=109373
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#109506]: https://bugs.freedesktop.org/show_bug.cgi?id=109506
  [fdo#109513]: https://bugs.freedesktop.org/show_bug.cgi?id=109513
  [fdo#109673]: https://bugs.freedesktop.org/show_bug.cgi?id=109673
  [fdo#109801]: https://bugs.freedesktop.org/show_bug.cgi?id=109801
  [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
  [fdo#110281]: https://bugs.freedesktop.org/show_bug.cgi?id=110281
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [fdo#110336]: https://bugs.freedesktop.org/show_bug.cgi?id=110336
  [k.org#202321]: https://bugzilla.kernel.org/show_bug.cgi?id=202321


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


Build changes
-------------

    * Linux: CI_DRM_5897 -> Patchwork_12739

  CI_DRM_5897: 7d07e025e78603d6270bc115fdb6c1efea6e66a5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4934: dc4f45eb6874331daec870dc1e4cfc3ac5c49311 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12739: 35b215101d202bb3f213620d23029e0e09f5c65b @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH 1/7] drm/i915/sdvo: Fix AVI infoframe TX rate readout

[Ville Syrjälä]

On Tue, Apr 09, 2019 at 10:16:04PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2019-04-09 15:40:48)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > The AVI infoframe readout code currently issues a
> > SDVO_CMD_GET_HBUF_TXRATE before SDVO_CMD_SET_HBUF_INDEX, which is
> > not the correct order for these two operations. So far this wasn't
> > a problem since we left the index pointing at the AVI infoframe
> > buffer at the end of the modeset. However once we start to write
> > to other buffers (namely ELD) that is no longer going to be true.
> > Fix up the order so that we always read out the TX rate for the
> > correct buffer.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_sdvo.c | 10 +++++-----
> >  1 file changed, 5 insertions(+), 5 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > index 0e3d91d9ef13..61db07244296 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -1001,6 +1001,11 @@ static ssize_t intel_sdvo_read_infoframe(struct intel_sdvo *intel_sdvo,
> >         if (av_split < if_index)
> >                 return 0;
> >  
> > +       if (!intel_sdvo_set_value(intel_sdvo,
> > +                                 SDVO_CMD_SET_HBUF_INDEX,
> > +                                 set_buf_index, 2))
> > +               return -ENXIO;
> 
> That is consistent with write_infoframe and makes a modicum of sense.
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Why is the av_split separate from the if_index? What does that mean, at
> some point I might have to read the docs!

av_split specifies how the set of available buffers is split
between video and audio. if_index specifies which buffers the
other commands operate on.

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

[PATCH v2 4/7] drm/i915/sdvo: Check that we have space for the infoframe

[Ville Syrjala]

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

Before we go writing the infoframe let's make sure we have
the space for it. Not that it really matters since the write
loop would just terminate early in that case.

v2: Check after the debug print and ++ (Chris)

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 7f64352a3413..14348dfb024a 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -976,6 +976,9 @@ static bool intel_sdvo_write_infoframe(struct intel_sdvo *intel_sdvo,
 	DRM_DEBUG_KMS("writing sdvo hbuf: %i, hbuf_size %i, hbuf_size: %i\n",
 		      if_index, length, hbuf_size);
 
+	if (hbuf_size < length)
+		return false;
+
 	for (i = 0; i < hbuf_size; i += 8) {
 		memset(tmp, 0, 8);
 		if (i < length)
-- 
2.21.0

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

[PATCH v2 7/7] drm/i915/sdvo: Actually print the reason why the SDVO command failed

[Ville Syrjala]

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

It's much easier to figure out why the SDVO encoder refuses to cooperate
if we can see what status we got back.

v2: Zero initialize only the first character, not the whole buffer

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> #v1
---
 drivers/gpu/drm/i915/intel_sdvo.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 937f9abf8a0a..358ee0065a7e 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -518,6 +518,7 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
 #define BUF_LEN 256
 	char buffer[BUF_LEN];
 
+	buffer[0] = '\0';
 
 	/*
 	 * The documentation states that all commands will be
@@ -581,7 +582,8 @@ static bool intel_sdvo_read_response(struct intel_sdvo *intel_sdvo,
 	return true;
 
 log_fail:
-	DRM_DEBUG_KMS("%s: R: ... failed\n", SDVO_NAME(intel_sdvo));
+	DRM_DEBUG_KMS("%s: R: ... failed %s\n",
+		      SDVO_NAME(intel_sdvo), buffer);
 	return false;
 }
 
-- 
2.21.0

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

✓ Fi.CI.BAT: success for drm/i915: Fix SDVO HDMI audio (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Fix SDVO HDMI audio (rev3)
URL   : https://patchwork.freedesktop.org/series/59233/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5901 -> Patchwork_12758
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/59233/revisions/3/mbox/

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_basic@gtt-bsd2:
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] +57

  * igt@i915_pm_rpm@basic-pci-d3-state:
    - fi-hsw-4770:        PASS -> SKIP [fdo#109271] +3

  * igt@i915_selftest@live_hangcheck:
    - fi-bxt-dsi:         PASS -> INCOMPLETE [fdo#103927]

  * igt@kms_busy@basic-flip-c:
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_force_connector_basic@force-edid:
    - fi-glk-dsi:         NOTRUN -> SKIP [fdo#109271] +36

  * igt@kms_frontbuffer_tracking@basic:
    - fi-glk-dsi:         NOTRUN -> FAIL [fdo#103167]
    - fi-byt-clapper:     NOTRUN -> FAIL [fdo#103167]

  
#### Possible fixes ####

  * igt@gem_exec_create@basic:
    - {fi-icl-u2}:        INCOMPLETE -> PASS

  * igt@gem_mmap_gtt@basic-copy:
    - fi-glk-dsi:         INCOMPLETE [fdo#103359] / [k.org#198133] -> PASS

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      DMESG-FAIL [fdo#110235 ] -> PASS

  * igt@i915_selftest@live_execlists:
    - fi-apl-guc:         INCOMPLETE [fdo#103927] / [fdo#109720] -> PASS

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109289]: https://bugs.freedesktop.org/show_bug.cgi?id=109289
  [fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#109316]: https://bugs.freedesktop.org/show_bug.cgi?id=109316
  [fdo#109720]: https://bugs.freedesktop.org/show_bug.cgi?id=109720
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (51 -> 45)
------------------------------

  Additional (1): fi-byt-clapper 
  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-bdw-samus 


Build changes
-------------

    * Linux: CI_DRM_5901 -> Patchwork_12758

  CI_DRM_5901: b98dd01c0ecc2b44c010d0372f35c87abdd4382f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4942: ff8929d4d5b57b544e699fa428930f0fd66dd2dc @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12758: 68a1ff7a23e70b0d053dcddec443ce668395b0cd @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

68a1ff7a23e7 drm/i915/sdvo: Actually print the reason why the SDVO command failed
30a08be915cd drm/i915/sdvo: Don't write stack garbage into the hbuf
f0553da8c483 drm/i915/sdvo: Don't unpack stack garbage
d7582b927aab drm/i915/sdvo: Check that we have space for the infoframe
1435c2f97a75 drm/i915: Rename SDVO_AUDIO_ENABLE to HDMI_AUDIO_ENABLE
c315b5ed8f4d drm/i915/sdvo: Implement proper HDMI audio support for SDVO
c2d2ff8faada drm/i915/sdvo: Fix AVI infoframe TX rate readout

== Logs ==

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

✗ Fi.CI.IGT: failure for drm/i915: Fix SDVO HDMI audio (rev3)

[Patchwork]

== Series Details ==

Series: drm/i915: Fix SDVO HDMI audio (rev3)
URL   : https://patchwork.freedesktop.org/series/59233/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_5901_full -> Patchwork_12758_full
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_12758_full absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_12758_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_12758_full:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_frontbuffer_tracking@psr-2p-primscrn-indfb-msflip-blt:
    - shard-iclb:         NOTRUN -> INCOMPLETE

  * igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
    - shard-skl:          PASS -> FAIL +1

  * igt@kms_vblank@pipe-b-wait-forked-hang:
    - shard-apl:          PASS -> FAIL

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@in-flight-suspend:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#108566] +3

  * igt@gem_exec_params@no-blt:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109283]

  * igt@gem_mocs_settings@mocs-reset-ctx-dirty-render:
    - shard-iclb:         NOTRUN -> SKIP [fdo#110206]

  * igt@gem_softpin@evict-snoop-interruptible:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109312]

  * igt@gem_stolen@stolen-pwrite:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109277]

  * igt@i915_pm_rpm@fences:
    - shard-skl:          PASS -> INCOMPLETE [fdo#107807]

  * igt@i915_pm_rpm@modeset-non-lpsp-stress-no-wait:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109308]

  * igt@kms_atomic_transition@6x-modeset-transitions-nonblocking-fencing:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-e:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-modeset-hang-oldfb-render-d:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +10

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-d:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109278] +1

  * igt@kms_chamelium@dp-frame-dump:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109284] +2

  * igt@kms_color@pipe-a-gamma:
    - shard-iclb:         NOTRUN -> FAIL [fdo#104782]

  * igt@kms_cursor_crc@cursor-512x170-onscreen:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109279]

  * igt@kms_cursor_legacy@2x-flip-vs-cursor-atomic:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109274] +4

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-skl:          NOTRUN -> FAIL [fdo#103833]

  * igt@kms_flip@2x-flip-vs-panning-vs-hang-interruptible:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +31

  * igt@kms_force_connector_basic@force-edid:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109285]

  * igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         NOTRUN -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbc-2p-indfb-fliptrack:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109280] +13

  * igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-render:
    - shard-snb:          PASS -> SKIP [fdo#109271]

  * igt@kms_frontbuffer_tracking@fbcpsr-rgb101010-draw-pwrite:
    - shard-iclb:         PASS -> FAIL [fdo#105682] / [fdo#109247] +1

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-cur-indfb-onoff:
    - shard-iclb:         NOTRUN -> FAIL [fdo#109247] +2

  * igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-indfb-draw-mmap-cpu:
    - shard-iclb:         PASS -> FAIL [fdo#109247] +16

  * igt@kms_lease@setcrtc_implicit_plane:
    - shard-iclb:         NOTRUN -> FAIL [fdo#110281]

  * igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
    - shard-apl:          PASS -> DMESG-WARN [fdo#108566] +5

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-apl:          NOTRUN -> FAIL [fdo#108145] +1

  * igt@kms_plane_alpha_blend@pipe-c-alpha-basic:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
    - shard-skl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_plane_lowres@pipe-a-tiling-y:
    - shard-iclb:         PASS -> FAIL [fdo#103166]

  * igt@kms_plane_scaling@pipe-c-scaler-with-rotation:
    - shard-iclb:         NOTRUN -> FAIL [fdo#109052]

  * igt@kms_psr@cursor_render:
    - shard-iclb:         NOTRUN -> FAIL [fdo#107383] / [fdo#110215]

  * igt@kms_psr@psr2_cursor_plane_move:
    - shard-iclb:         PASS -> SKIP [fdo#109441]

  * igt@kms_psr@psr2_primary_mmap_gtt:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109441] +1

  * igt@kms_psr@sprite_render:
    - shard-iclb:         PASS -> FAIL [fdo#107383] / [fdo#110215] +1

  * igt@kms_setmode@basic:
    - shard-skl:          NOTRUN -> FAIL [fdo#99912]

  * igt@kms_sysfs_edid_timing:
    - shard-iclb:         PASS -> FAIL [fdo#100047]

  * igt@kms_tv_load_detect@load-detect:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109309]

  * igt@perf_pmu@most-busy-check-all-vcs1:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109276] +8

  * igt@perf_pmu@semaphore-wait-vcs1:
    - shard-skl:          NOTRUN -> SKIP [fdo#109271] +77

  * igt@prime_nv_api@i915_nv_import_vs_close:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +5

  * igt@prime_nv_pcopy@test2:
    - shard-iclb:         NOTRUN -> SKIP [fdo#109291] +2

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-suspend:
    - shard-skl:          INCOMPLETE [fdo#104108] / [fdo#107773] -> PASS +1

  * igt@gem_mmap_gtt@forked-big-copy-odd:
    - shard-apl:          DMESG-WARN -> PASS

  * igt@gem_tiled_swapping@non-threaded:
    - shard-iclb:         FAIL [fdo#108686] -> PASS

  * igt@i915_pm_rpm@universal-planes-dpms:
    - shard-skl:          INCOMPLETE [fdo#107807] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-mmap-cpu:
    - shard-skl:          FAIL [fdo#103167] / [fdo#105682] / [fdo#110379] -> PASS

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-pwrite:
    - shard-iclb:         FAIL [fdo#103167] -> PASS +3

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-pri-indfb-draw-pwrite:
    - shard-iclb:         FAIL [fdo#109247] -> PASS +10

  * igt@kms_pipe_crc_basic@nonblocking-crc-pipe-a-frame-sequence:
    - shard-skl:          FAIL [fdo#103191] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - shard-apl:          DMESG-WARN [fdo#108566] -> PASS +1

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-min:
    - shard-skl:          FAIL [fdo#108145] -> PASS

  * igt@kms_psr@dpms:
    - shard-iclb:         FAIL [fdo#107383] / [fdo#110215] -> PASS

  * igt@kms_psr@primary_mmap_cpu:
    - shard-iclb:         INCOMPLETE -> PASS

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         SKIP [fdo#109441] -> PASS +3

  
  [fdo#100047]: https://bugs.freedesktop.org/show_bug.cgi?id=100047
  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103833]: https://bugs.freedesktop.org/show_bug.cgi?id=103833
  [fdo#104108]: https://bugs.freedesktop.org/show_bug.cgi?id=104108
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#107383]: https://bugs.freedesktop.org/show_bug.cgi?id=107383
  [fdo#107773]: https://bugs.freedesktop.org/show_bug.cgi?id=107773
  [fdo#107807]: https://bugs.freedesktop.org/show_bug.cgi?id=107807
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108566]: https://bugs.freedesktop.org/show_bug.cgi?id=108566
  [fdo#108686]: https://bugs.freedesktop.org/show_bug.cgi?id=108686
  [fdo#109052]: https://bugs.freedesktop.org/show_bug.cgi?id=109052
  [fdo#109247]: https://bugs.freedesktop.org/show_bug.cgi?id=109247
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109274]: https://bugs.freedesktop.org/show_bug.cgi?id=109274
  [fdo#109276]: https://bugs.freedesktop.org/show_bug.cgi?id=109276
  [fdo#109277]: https://bugs.freedesktop.org/show_bug.cgi?id=109277
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109279]: https://bugs.freedesktop.org/show_bug.cgi?id=109279
  [fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
  [fdo#109283]: https://bugs.freedesktop.org/show_bug.cgi?id=109283
  [fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109291]: https://bugs.freedesktop.org/show_bug.cgi?id=109291
  [fdo#109308]: https://bugs.freedesktop.org/show_bug.cgi?id=109308
  [fdo#109309]: https://bugs.freedesktop.org/show_bug.cgi?id=109309
  [fdo#109312]: https://bugs.freedesktop.org/show_bug.cgi?id=109312
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#110206]: https://bugs.freedesktop.org/show_bug.cgi?id=110206
  [fdo#110215]: https://bugs.freedesktop.org/show_bug.cgi?id=110215
  [fdo#110281]: https://bugs.freedesktop.org/show_bug.cgi?id=110281
  [fdo#110379]: https://bugs.freedesktop.org/show_bug.cgi?id=110379
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 9)
------------------------------

  Missing    (1): shard-hsw 


Build changes
-------------

    * Linux: CI_DRM_5901 -> Patchwork_12758

  CI_DRM_5901: b98dd01c0ecc2b44c010d0372f35c87abdd4382f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4942: ff8929d4d5b57b544e699fa428930f0fd66dd2dc @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_12758: 68a1ff7a23e70b0d053dcddec443ce668395b0cd @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

Re: [PATCH v2 4/7] drm/i915/sdvo: Check that we have space for the infoframe

[Sripada, Radhakrishna]

On Wed, 2019-04-10 at 20:08 +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Before we go writing the infoframe let's make sure we have
> the space for it. Not that it really matters since the write
> loop would just terminate early in that case.
> 
> v2: Check after the debug print and ++ (Chris)
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> b/drivers/gpu/drm/i915/intel_sdvo.c
> index 7f64352a3413..14348dfb024a 100644
> --- a/drivers/gpu/drm/i915/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> @@ -976,6 +976,9 @@ static bool intel_sdvo_write_infoframe(struct
> intel_sdvo *intel_sdvo,
>  	DRM_DEBUG_KMS("writing sdvo hbuf: %i, hbuf_size %i, hbuf_size:
> %i\n",
>  		      if_index, length, hbuf_size);
If the above line is newly added why dont I See a + at the beginning of
the line? Is it from a previous version of the patch?

-Radhakrishna(RK) Sripada
>  
> +	if (hbuf_size < length)
> +		return false;
> +
>  	for (i = 0; i < hbuf_size; i += 8) {
>  		memset(tmp, 0, 8);
>  		if (i < length)
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 4/7] drm/i915/sdvo: Check that we have space for the infoframe

[Ville Syrjälä]

On Mon, Apr 22, 2019 at 06:37:48PM +0000, Sripada, Radhakrishna wrote:
> On Wed, 2019-04-10 at 20:08 +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Before we go writing the infoframe let's make sure we have
> > the space for it. Not that it really matters since the write
> > loop would just terminate early in that case.
> > 
> > v2: Check after the debug print and ++ (Chris)
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
> >  1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> > b/drivers/gpu/drm/i915/intel_sdvo.c
> > index 7f64352a3413..14348dfb024a 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -976,6 +976,9 @@ static bool intel_sdvo_write_infoframe(struct
> > intel_sdvo *intel_sdvo,
> >  	DRM_DEBUG_KMS("writing sdvo hbuf: %i, hbuf_size %i, hbuf_size:
> > %i\n",
> >  		      if_index, length, hbuf_size);
> If the above line is newly added why dont I See a + at the beginning of
> the line? Is it from a previous version of the patch?

It's not newly added. What gave you the idea that it was?

> 
> -Radhakrishna(RK) Sripada
> >  
> > +	if (hbuf_size < length)
> > +		return false;
> > +
> >  	for (i = 0; i < hbuf_size; i += 8) {
> >  		memset(tmp, 0, 8);
> >  		if (i < length)

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

Re: [PATCH v2 4/7] drm/i915/sdvo: Check that we have space for the infoframe

[Sripada, Radhakrishna]

On Tue, 2019-04-23 at 17:34 +0300, Ville Syrjälä wrote:
> On Mon, Apr 22, 2019 at 06:37:48PM +0000, Sripada, Radhakrishna
> wrote:
> > On Wed, 2019-04-10 at 20:08 +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Before we go writing the infoframe let's make sure we have
> > > the space for it. Not that it really matters since the write
> > > loop would just terminate early in that case.
> > > 
> > > v2: Check after the debug print and ++ (Chris)
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > >  drivers/gpu/drm/i915/intel_sdvo.c | 3 +++
> > >  1 file changed, 3 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c
> > > b/drivers/gpu/drm/i915/intel_sdvo.c
> > > index 7f64352a3413..14348dfb024a 100644
> > > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > > @@ -976,6 +976,9 @@ static bool intel_sdvo_write_infoframe(struct
> > > intel_sdvo *intel_sdvo,
> > >  	DRM_DEBUG_KMS("writing sdvo hbuf: %i, hbuf_size %i, hbuf_size:
> > > %i\n",
> > >  		      if_index, length, hbuf_size);
> > 
> > If the above line is newly added why dont I See a + at the
> > beginning of
> > the line? Is it from a previous version of the patch?
> 
> It's not newly added. What gave you the idea that it was?
NVM. I was comparing the raw diffs between v1 and v2 of the patch and
missed the above line in the context lines in v1 of the patch. Just a
minor code move around. Will be cautious in future reviews.

LGTM.
Reviewed-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> 
> > 
> > -Radhakrishna(RK) Sripada
> > >  
> > > +	if (hbuf_size < length)
> > > +		return false;
> > > +
> > >  	for (i = 0; i < hbuf_size; i += 8) {
> > >  		memset(tmp, 0, 8);
> > >  		if (i < length)
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 2/7] drm/i915/sdvo: Implement proper HDMI audio support for SDVO

[Imre Deak]

On Tue, Apr 09, 2019 at 11:00:10PM +0300, Ville Syrjälä wrote:
> On Tue, Apr 09, 2019 at 05:40:49PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Our SDVO audio support is pretty bogus. We can't push audio over the
> > SDVO bus, so trying to enable audio in the SDVO control register doesn't
> > do anything. In fact it looks like the SDVO encoder will always mix in
> > the audio coming over HDA, and there's no (at least documented) way to
> > disable that from our side. So HDMI audio does work currently but only by
> > luck really. What is missing though is the ELD.
> 
> Hmm. Looks like I forgot to update this text after the gen3 bug was
> reported. The situation is that audio works on gen4 by luck. On gen3
> it got broken by the referenced commit since we no longer enable
> HDMI encoding on the SDVO device (that will stop audio transmission
> entirely).
> 
> > 
> > To pass the ELD to the audio driver we need to write it to magic buffer
> > in the SDVO encoder hardware which then gets pulled out via HDA in the
> > other end. Ie. pretty much the same thing we had for native HDMI before
> > we started to just pass the ELD between the drivers. This sort of
> > explains why we even have that silly hardware buffer with native HDMI.
> > 
> > $ cat /proc/asound/card0/eld#1.0
> > -monitor_present		0
> > -eld_valid		0
> > +monitor_present		1
> > +eld_valid		1
> > +monitor_name		LG TV
> > +connection_type		HDMI
> > +...
> > 
> > This also fixes our state readout since we can now query the SDVO
> > encoder about the state of the "ELD valid" and "presence detect"
> > bits. As mentioned those don't actually control whether audio
> > gets sent over the HDMI cable, but it's the best we can do.
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: zardam@gmail.com
> > Tested-by: zardam@gmail.com
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108976
> > Fixes: de44e256b92c ("drm/i915/sdvo: Shut up state checker with hdmi cards on gen3")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Matches the sdvo specs and bspec (SDVO_AUDIO_ENABLE is a reserved/MBZ
bit on GEN3,3.5, and on GEN4 it's probably HDMI specific, since there is
no audio traffic over the SDVO bus):

Reviewed-by: Imre Deak <imre.deak@intel.com>

Btw, is it guaranteed that we have a valid ELD when
force_audio == HDMI_AUDIO_ON ?

> > ---
> >  drivers/gpu/drm/i915/intel_sdvo.c      | 58 +++++++++++++++++++++-----
> >  drivers/gpu/drm/i915/intel_sdvo_regs.h |  3 ++
> >  2 files changed, 50 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > index 61db07244296..7f64352a3413 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -916,6 +916,13 @@ static bool intel_sdvo_set_colorimetry(struct intel_sdvo *intel_sdvo,
> >  	return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_COLORIMETRY, &mode, 1);
> >  }
> >  
> > +static bool intel_sdvo_set_audio_state(struct intel_sdvo *intel_sdvo,
> > +				       u8 audio_state)
> > +{
> > +	return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_AUDIO_STAT,
> > +				    &audio_state, 1);
> > +}
> > +
> >  #if 0
> >  static void intel_sdvo_dump_hdmi_buf(struct intel_sdvo *intel_sdvo)
> >  {
> > @@ -1487,11 +1494,6 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder,
> >  	else
> >  		sdvox |= SDVO_PIPE_SEL(crtc->pipe);
> >  
> > -	if (crtc_state->has_audio) {
> > -		WARN_ON_ONCE(INTEL_GEN(dev_priv) < 4);
> > -		sdvox |= SDVO_AUDIO_ENABLE;
> > -	}
> > -
> >  	if (INTEL_GEN(dev_priv) >= 4) {
> >  		/* done in crtc_mode_set as the dpll_md reg must be written early */
> >  	} else if (IS_I945G(dev_priv) || IS_I945GM(dev_priv) ||
> > @@ -1635,8 +1637,13 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
> >  	if (sdvox & HDMI_COLOR_RANGE_16_235)
> >  		pipe_config->limited_color_range = true;
> >  
> > -	if (sdvox & SDVO_AUDIO_ENABLE)
> > -		pipe_config->has_audio = true;
> > +	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_AUDIO_STAT,
> > +				 &val, 1)) {
> > +		u8 mask = SDVO_AUDIO_ELD_VALID | SDVO_AUDIO_PRESENCE_DETECT;
> > +
> > +		if ((val & mask) == mask)
> > +			pipe_config->has_audio = true;
> > +	}
> >  
> >  	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_ENCODE,
> >  				 &val, 1)) {
> > @@ -1647,6 +1654,32 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
> >  	intel_sdvo_get_avi_infoframe(intel_sdvo, pipe_config);
> >  }
> >  
> > +static void intel_sdvo_disable_audio(struct intel_sdvo *intel_sdvo)
> > +{
> > +	intel_sdvo_set_audio_state(intel_sdvo, 0);
> > +}
> > +
> > +static void intel_sdvo_enable_audio(struct intel_sdvo *intel_sdvo,
> > +				    const struct intel_crtc_state *crtc_state,
> > +				    const struct drm_connector_state *conn_state)
> > +{
> > +	const struct drm_display_mode *adjusted_mode =
> > +		&crtc_state->base.adjusted_mode;
> > +	struct drm_connector *connector = conn_state->connector;
> > +	u8 *eld = connector->eld;
> > +
> > +	eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
> > +
> > +	intel_sdvo_set_audio_state(intel_sdvo, 0);
> > +
> > +	intel_sdvo_write_infoframe(intel_sdvo, SDVO_HBUF_INDEX_ELD,
> > +				   SDVO_HBUF_TX_DISABLED,
> > +				   eld, drm_eld_size(eld));
> > +
> > +	intel_sdvo_set_audio_state(intel_sdvo, SDVO_AUDIO_ELD_VALID |
> > +				   SDVO_AUDIO_PRESENCE_DETECT);
> > +}
> > +
> >  static void intel_disable_sdvo(struct intel_encoder *encoder,
> >  			       const struct intel_crtc_state *old_crtc_state,
> >  			       const struct drm_connector_state *conn_state)
> > @@ -1656,6 +1689,9 @@ static void intel_disable_sdvo(struct intel_encoder *encoder,
> >  	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
> >  	u32 temp;
> >  
> > +	if (old_crtc_state->has_audio)
> > +		intel_sdvo_disable_audio(intel_sdvo);
> > +
> >  	intel_sdvo_set_active_outputs(intel_sdvo, 0);
> >  	if (0)
> >  		intel_sdvo_set_encoder_power_state(intel_sdvo,
> > @@ -1741,6 +1777,9 @@ static void intel_enable_sdvo(struct intel_encoder *encoder,
> >  		intel_sdvo_set_encoder_power_state(intel_sdvo,
> >  						   DRM_MODE_DPMS_ON);
> >  	intel_sdvo_set_active_outputs(intel_sdvo, intel_sdvo->attached_output);
> > +
> > +	if (pipe_config->has_audio)
> > +		intel_sdvo_enable_audio(intel_sdvo, pipe_config, conn_state);
> >  }
> >  
> >  static enum drm_mode_status
> > @@ -2603,7 +2642,6 @@ static bool
> >  intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
> >  {
> >  	struct drm_encoder *encoder = &intel_sdvo->base.base;
> > -	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
> >  	struct drm_connector *connector;
> >  	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
> >  	struct intel_connector *intel_connector;
> > @@ -2640,9 +2678,7 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
> >  	encoder->encoder_type = DRM_MODE_ENCODER_TMDS;
> >  	connector->connector_type = DRM_MODE_CONNECTOR_DVID;
> >  
> > -	/* gen3 doesn't do the hdmi bits in the SDVO register */
> > -	if (INTEL_GEN(dev_priv) >= 4 &&
> > -	    intel_sdvo_is_hdmi_connector(intel_sdvo, device)) {
> > +	if (intel_sdvo_is_hdmi_connector(intel_sdvo, device)) {
> >  		connector->connector_type = DRM_MODE_CONNECTOR_HDMIA;
> >  		intel_sdvo_connector->is_hdmi = true;
> >  	}
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo_regs.h b/drivers/gpu/drm/i915/intel_sdvo_regs.h
> > index db0ed499268a..e9ba3b047f93 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo_regs.h
> > +++ b/drivers/gpu/drm/i915/intel_sdvo_regs.h
> > @@ -707,6 +707,9 @@ struct intel_sdvo_enhancements_arg {
> >  #define SDVO_CMD_GET_AUDIO_ENCRYPT_PREFER 0x90
> >  #define SDVO_CMD_SET_AUDIO_STAT		0x91
> >  #define SDVO_CMD_GET_AUDIO_STAT		0x92
> > +  #define SDVO_AUDIO_ELD_VALID		(1 << 0)
> > +  #define SDVO_AUDIO_PRESENCE_DETECT	(1 << 1)
> > +  #define SDVO_AUDIO_CP_READY		(1 << 2)
> >  #define SDVO_CMD_SET_HBUF_INDEX		0x93
> >    #define SDVO_HBUF_INDEX_ELD		0
> >    #define SDVO_HBUF_INDEX_AVI_IF	1
> > -- 
> > 2.21.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/7] drm/i915/sdvo: Implement proper HDMI audio support for SDVO

[Imre Deak]

On Tue, Apr 09, 2019 at 11:00:10PM +0300, Ville Syrjälä wrote:
> On Tue, Apr 09, 2019 at 05:40:49PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Our SDVO audio support is pretty bogus. We can't push audio over the
> > SDVO bus, so trying to enable audio in the SDVO control register doesn't
> > do anything. In fact it looks like the SDVO encoder will always mix in
> > the audio coming over HDA, and there's no (at least documented) way to
> > disable that from our side. So HDMI audio does work currently but only by
> > luck really. What is missing though is the ELD.
> 
> Hmm. Looks like I forgot to update this text after the gen3 bug was
> reported. The situation is that audio works on gen4 by luck. On gen3
> it got broken by the referenced commit since we no longer enable
> HDMI encoding on the SDVO device (that will stop audio transmission
> entirely).
> 
> > 
> > To pass the ELD to the audio driver we need to write it to magic buffer
> > in the SDVO encoder hardware which then gets pulled out via HDA in the
> > other end. Ie. pretty much the same thing we had for native HDMI before
> > we started to just pass the ELD between the drivers. This sort of
> > explains why we even have that silly hardware buffer with native HDMI.
> > 
> > $ cat /proc/asound/card0/eld#1.0
> > -monitor_present		0
> > -eld_valid		0
> > +monitor_present		1
> > +eld_valid		1
> > +monitor_name		LG TV
> > +connection_type		HDMI
> > +...
> > 
> > This also fixes our state readout since we can now query the SDVO
> > encoder about the state of the "ELD valid" and "presence detect"
> > bits. As mentioned those don't actually control whether audio
> > gets sent over the HDMI cable, but it's the best we can do.
> > 
> > Cc: stable@vger.kernel.org
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: zardam@gmail.com
> > Tested-by: zardam@gmail.com
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=108976
> > Fixes: de44e256b92c ("drm/i915/sdvo: Shut up state checker with hdmi cards on gen3")
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Matches the sdvo specs and bspec (SDVO_AUDIO_ENABLE is a reserved/MBZ
bit on GEN3,3.5, and on GEN4 it's probably HDMI specific, since there is
no audio traffic over the SDVO bus):

Reviewed-by: Imre Deak <imre.deak@intel.com>

Btw, is it guaranteed that we have a valid ELD when
force_audio == HDMI_AUDIO_ON ?

> > ---
> >  drivers/gpu/drm/i915/intel_sdvo.c      | 58 +++++++++++++++++++++-----
> >  drivers/gpu/drm/i915/intel_sdvo_regs.h |  3 ++
> >  2 files changed, 50 insertions(+), 11 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
> > index 61db07244296..7f64352a3413 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo.c
> > +++ b/drivers/gpu/drm/i915/intel_sdvo.c
> > @@ -916,6 +916,13 @@ static bool intel_sdvo_set_colorimetry(struct intel_sdvo *intel_sdvo,
> >  	return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_COLORIMETRY, &mode, 1);
> >  }
> >  
> > +static bool intel_sdvo_set_audio_state(struct intel_sdvo *intel_sdvo,
> > +				       u8 audio_state)
> > +{
> > +	return intel_sdvo_set_value(intel_sdvo, SDVO_CMD_SET_AUDIO_STAT,
> > +				    &audio_state, 1);
> > +}
> > +
> >  #if 0
> >  static void intel_sdvo_dump_hdmi_buf(struct intel_sdvo *intel_sdvo)
> >  {
> > @@ -1487,11 +1494,6 @@ static void intel_sdvo_pre_enable(struct intel_encoder *intel_encoder,
> >  	else
> >  		sdvox |= SDVO_PIPE_SEL(crtc->pipe);
> >  
> > -	if (crtc_state->has_audio) {
> > -		WARN_ON_ONCE(INTEL_GEN(dev_priv) < 4);
> > -		sdvox |= SDVO_AUDIO_ENABLE;
> > -	}
> > -
> >  	if (INTEL_GEN(dev_priv) >= 4) {
> >  		/* done in crtc_mode_set as the dpll_md reg must be written early */
> >  	} else if (IS_I945G(dev_priv) || IS_I945GM(dev_priv) ||
> > @@ -1635,8 +1637,13 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
> >  	if (sdvox & HDMI_COLOR_RANGE_16_235)
> >  		pipe_config->limited_color_range = true;
> >  
> > -	if (sdvox & SDVO_AUDIO_ENABLE)
> > -		pipe_config->has_audio = true;
> > +	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_AUDIO_STAT,
> > +				 &val, 1)) {
> > +		u8 mask = SDVO_AUDIO_ELD_VALID | SDVO_AUDIO_PRESENCE_DETECT;
> > +
> > +		if ((val & mask) == mask)
> > +			pipe_config->has_audio = true;
> > +	}
> >  
> >  	if (intel_sdvo_get_value(intel_sdvo, SDVO_CMD_GET_ENCODE,
> >  				 &val, 1)) {
> > @@ -1647,6 +1654,32 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
> >  	intel_sdvo_get_avi_infoframe(intel_sdvo, pipe_config);
> >  }
> >  
> > +static void intel_sdvo_disable_audio(struct intel_sdvo *intel_sdvo)
> > +{
> > +	intel_sdvo_set_audio_state(intel_sdvo, 0);
> > +}
> > +
> > +static void intel_sdvo_enable_audio(struct intel_sdvo *intel_sdvo,
> > +				    const struct intel_crtc_state *crtc_state,
> > +				    const struct drm_connector_state *conn_state)
> > +{
> > +	const struct drm_display_mode *adjusted_mode =
> > +		&crtc_state->base.adjusted_mode;
> > +	struct drm_connector *connector = conn_state->connector;
> > +	u8 *eld = connector->eld;
> > +
> > +	eld[6] = drm_av_sync_delay(connector, adjusted_mode) / 2;
> > +
> > +	intel_sdvo_set_audio_state(intel_sdvo, 0);
> > +
> > +	intel_sdvo_write_infoframe(intel_sdvo, SDVO_HBUF_INDEX_ELD,
> > +				   SDVO_HBUF_TX_DISABLED,
> > +				   eld, drm_eld_size(eld));
> > +
> > +	intel_sdvo_set_audio_state(intel_sdvo, SDVO_AUDIO_ELD_VALID |
> > +				   SDVO_AUDIO_PRESENCE_DETECT);
> > +}
> > +
> >  static void intel_disable_sdvo(struct intel_encoder *encoder,
> >  			       const struct intel_crtc_state *old_crtc_state,
> >  			       const struct drm_connector_state *conn_state)
> > @@ -1656,6 +1689,9 @@ static void intel_disable_sdvo(struct intel_encoder *encoder,
> >  	struct intel_crtc *crtc = to_intel_crtc(old_crtc_state->base.crtc);
> >  	u32 temp;
> >  
> > +	if (old_crtc_state->has_audio)
> > +		intel_sdvo_disable_audio(intel_sdvo);
> > +
> >  	intel_sdvo_set_active_outputs(intel_sdvo, 0);
> >  	if (0)
> >  		intel_sdvo_set_encoder_power_state(intel_sdvo,
> > @@ -1741,6 +1777,9 @@ static void intel_enable_sdvo(struct intel_encoder *encoder,
> >  		intel_sdvo_set_encoder_power_state(intel_sdvo,
> >  						   DRM_MODE_DPMS_ON);
> >  	intel_sdvo_set_active_outputs(intel_sdvo, intel_sdvo->attached_output);
> > +
> > +	if (pipe_config->has_audio)
> > +		intel_sdvo_enable_audio(intel_sdvo, pipe_config, conn_state);
> >  }
> >  
> >  static enum drm_mode_status
> > @@ -2603,7 +2642,6 @@ static bool
> >  intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
> >  {
> >  	struct drm_encoder *encoder = &intel_sdvo->base.base;
> > -	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
> >  	struct drm_connector *connector;
> >  	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
> >  	struct intel_connector *intel_connector;
> > @@ -2640,9 +2678,7 @@ intel_sdvo_dvi_init(struct intel_sdvo *intel_sdvo, int device)
> >  	encoder->encoder_type = DRM_MODE_ENCODER_TMDS;
> >  	connector->connector_type = DRM_MODE_CONNECTOR_DVID;
> >  
> > -	/* gen3 doesn't do the hdmi bits in the SDVO register */
> > -	if (INTEL_GEN(dev_priv) >= 4 &&
> > -	    intel_sdvo_is_hdmi_connector(intel_sdvo, device)) {
> > +	if (intel_sdvo_is_hdmi_connector(intel_sdvo, device)) {
> >  		connector->connector_type = DRM_MODE_CONNECTOR_HDMIA;
> >  		intel_sdvo_connector->is_hdmi = true;
> >  	}
> > diff --git a/drivers/gpu/drm/i915/intel_sdvo_regs.h b/drivers/gpu/drm/i915/intel_sdvo_regs.h
> > index db0ed499268a..e9ba3b047f93 100644
> > --- a/drivers/gpu/drm/i915/intel_sdvo_regs.h
> > +++ b/drivers/gpu/drm/i915/intel_sdvo_regs.h
> > @@ -707,6 +707,9 @@ struct intel_sdvo_enhancements_arg {
> >  #define SDVO_CMD_GET_AUDIO_ENCRYPT_PREFER 0x90
> >  #define SDVO_CMD_SET_AUDIO_STAT		0x91
> >  #define SDVO_CMD_GET_AUDIO_STAT		0x92
> > +  #define SDVO_AUDIO_ELD_VALID		(1 << 0)
> > +  #define SDVO_AUDIO_PRESENCE_DETECT	(1 << 1)
> > +  #define SDVO_AUDIO_CP_READY		(1 << 2)
> >  #define SDVO_CMD_SET_HBUF_INDEX		0x93
> >    #define SDVO_HBUF_INDEX_ELD		0
> >    #define SDVO_HBUF_INDEX_AVI_IF	1
> > -- 
> > 2.21.0
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx