[PATCH] ASoC: codec: hdac_hdmi: fix pin connections at cvt enable

[PATCH] ASoC: codec: hdac_hdmi: fix pin connections at cvt enable

[Kai Vehmanen]

In display codecs supported by hdac_hdmi, the connection indices are
shared by all converters. At boot and resume from suspend,
the connection state may be reset to default values.

In case of multiple connected pins (multiple monitors connected
with audio capability), routing and mute status of pins that
are not connected to any PCM, may interfere with other pins.
E.g. after resume from S3 with multiple monitors, unless
all converters are in active use, playback to some PCMs may
be muted due to the default settings of unrelated converters.

Avoid this by ensuring all pin:cvt selections are correct
in codec whenever a converter is enabled for playback.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 41 ++++++++++++++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 660e0587f3999..bb87240d2fb22 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -546,6 +546,39 @@ static struct hdac_hdmi_port *hdac_hdmi_get_port_from_cvt(
 	return NULL;
 }
 
+/**
+ * Go through all converters and ensure connection is set to
+ * the correct pin as set via kcontrols.
+ */
+static void hdac_hdmi_verify_connect_sel_all_pins(struct hdac_device *hdev)
+{
+	struct hdac_hdmi_port *port;
+	struct hdac_hdmi_priv *hdmi = hdev_to_hdmi_priv(hdev);
+	struct hdac_hdmi_cvt *cvt;
+	int cvt_idx = 0;
+	int curr;
+
+	list_for_each_entry(cvt, &hdmi->cvt_list, head) {
+		port = hdac_hdmi_get_port_from_cvt(hdev, hdmi, cvt);
+		if (port && port->pin) {
+			curr = snd_hdac_codec_read(hdev, port->pin->nid,
+						   0, AC_VERB_GET_CONNECT_SEL,
+						   0);
+			if (curr != cvt_idx) {
+				snd_hdac_codec_write(hdev,
+						     port->pin->nid, 0,
+						     AC_VERB_SET_CONNECT_SEL,
+						     cvt_idx);
+				dev_dbg(&hdev->dev,
+					"%s: %s set connect %d -> %d\n",
+					__func__, cvt->name, port->pin->nid,
+					cvt_idx);
+			}
+		}
+		++cvt_idx;
+	}
+}
+
 /*
  * This tries to get a valid pin and set the HW constraints based on the
  * ELD. Even if a valid pin is not found return success so that device open
@@ -806,6 +839,14 @@ static int hdac_hdmi_cvt_output_widget_event(struct snd_soc_dapm_widget *w,
 				AC_VERB_SET_CHANNEL_STREAMID, pcm->stream_tag);
 		snd_hdac_codec_write(hdev, cvt->nid, 0,
 				AC_VERB_SET_STREAM_FORMAT, pcm->format);
+
+		/**
+		 * The connection indices are shared by all converters and
+		 * may interfere with each other. Ensure correct
+		 * routing for all converters at stream start.
+		 */
+		hdac_hdmi_verify_connect_sel_all_pins(hdev);
+
 		break;
 
 	case SND_SOC_DAPM_POST_PMD:
-- 
2.17.1

Re: [PATCH] ASoC: codec: hdac_hdmi: fix pin connections at cvt enable

[Takashi Iwai]

On Wed, 12 Jun 2019 13:55:09 +0200,
Kai Vehmanen wrote:
> 
> In display codecs supported by hdac_hdmi, the connection indices are
> shared by all converters. At boot and resume from suspend,
> the connection state may be reset to default values.
> 
> In case of multiple connected pins (multiple monitors connected
> with audio capability), routing and mute status of pins that
> are not connected to any PCM, may interfere with other pins.
> E.g. after resume from S3 with multiple monitors, unless
> all converters are in active use, playback to some PCMs may
> be muted due to the default settings of unrelated converters.
> 
> Avoid this by ensuring all pin:cvt selections are correct
> in codec whenever a converter is enabled for playback.
> 
> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> ---
>  sound/soc/codecs/hdac_hdmi.c | 41 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 41 insertions(+)
> 
> diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
> index 660e0587f3999..bb87240d2fb22 100644
> --- a/sound/soc/codecs/hdac_hdmi.c
> +++ b/sound/soc/codecs/hdac_hdmi.c
> @@ -546,6 +546,39 @@ static struct hdac_hdmi_port *hdac_hdmi_get_port_from_cvt(
>  	return NULL;
>  }
>  
> +/**
> + * Go through all converters and ensure connection is set to
> + * the correct pin as set via kcontrols.
> + */
> +static void hdac_hdmi_verify_connect_sel_all_pins(struct hdac_device *hdev)
> +{
> +	struct hdac_hdmi_port *port;
> +	struct hdac_hdmi_priv *hdmi = hdev_to_hdmi_priv(hdev);
> +	struct hdac_hdmi_cvt *cvt;
> +	int cvt_idx = 0;
> +	int curr;
> +
> +	list_for_each_entry(cvt, &hdmi->cvt_list, head) {
> +		port = hdac_hdmi_get_port_from_cvt(hdev, hdmi, cvt);
> +		if (port && port->pin) {
> +			curr = snd_hdac_codec_read(hdev, port->pin->nid,
> +						   0, AC_VERB_GET_CONNECT_SEL,
> +						   0);
> +			if (curr != cvt_idx) {
> +				snd_hdac_codec_write(hdev,
> +						     port->pin->nid, 0,
> +						     AC_VERB_SET_CONNECT_SEL,
> +						     cvt_idx);
> +				dev_dbg(&hdev->dev,
> +					"%s: %s set connect %d -> %d\n",
> +					__func__, cvt->name, port->pin->nid,
> +					cvt_idx);

You can simply restore all pins without reading.  The read costs much
more time than writes.


thanks,

Takashi

Re: [PATCH] ASoC: codec: hdac_hdmi: fix pin connections at cvt enable

[Kai Vehmanen]

Hi,


On Wed, 12 Jun 2019, Takashi Iwai wrote:

> On Wed, 12 Jun 2019 13:55:09 +0200,
> Kai Vehmanen wrote:
> > +			curr = snd_hdac_codec_read(hdev, port->pin->nid,
> > +						   0, AC_VERB_GET_CONNECT_SEL,
> > +						   0);
> > +			if (curr != cvt_idx) {
> > +				snd_hdac_codec_write(hdev,
> > +						     port->pin->nid, 0,
> > +						     AC_VERB_SET_CONNECT_SEL,
> > +						     cvt_idx);
> > +				dev_dbg(&hdev->dev,
> > +					"%s: %s set connect %d -> %d\n",
> > +					__func__, cvt->name, port->pin->nid,
> > +					cvt_idx);
> 
> You can simply restore all pins without reading.  The read costs much
> more time than writes.

aa, thanks, you are probably right. I actually had just the write in my 
first version of the patch, but I ended up adding the read just be on the 
safe side. But true, write with same connection should not have any 
side-effects. I'll change this in v2.

Br, Kai