diff mbox series

[v3] ALSA: hda: hdmi - fix regression in connect list handling

Message ID 20191127161240.17026-1-kai.vehmanen@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series [v3] ALSA: hda: hdmi - fix regression in connect list handling | expand

Commit Message

Kai Vehmanen Nov. 27, 2019, 4:12 p.m. UTC 
Fix regression in how intel_haswell_fixup_connect_list()
results are used in hda_read_pin_conn(). Use of
snd_hda_get_raw_connections() in hda_read_pin_conn() bypasses
the cache and thus also bypasses the overridden pin connection
list. On platforms that require the connection list fixup,
mux list will be empty and HDMI playback will fail to -EBUSY
at open.

Fix the regression in hda_read_pinn_conn(). Simplify code
as suggested by Takashi Iwai to remove old
intel_haswell_fixup_connect_list() and copy the cvt_nid list
directly and not use snd_hda_override_conn_list() at all.

Fixes: 9c32fea83692 ("ALSA: hda - Add DP-MST support for non-acomp codecs")
BugLink: https://github.com/thesofproject/linux/issues/1537
Cc: Nikhil Mahale <nmahale@nvidia.com>
Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
---
 sound/pci/hda/patch_hdmi.c | 38 ++++++++++++--------------------------
 1 file changed, 12 insertions(+), 26 deletions(-)

Comments

Takashi Iwai Nov. 27, 2019, 4:39 p.m. UTC | #1 
On Wed, 27 Nov 2019 17:12:40 +0100,
Kai Vehmanen wrote:
> 
> Fix regression in how intel_haswell_fixup_connect_list()
> results are used in hda_read_pin_conn(). Use of
> snd_hda_get_raw_connections() in hda_read_pin_conn() bypasses
> the cache and thus also bypasses the overridden pin connection
> list. On platforms that require the connection list fixup,
> mux list will be empty and HDMI playback will fail to -EBUSY
> at open.
> 
> Fix the regression in hda_read_pinn_conn(). Simplify code
> as suggested by Takashi Iwai to remove old
> intel_haswell_fixup_connect_list() and copy the cvt_nid list
> directly and not use snd_hda_override_conn_list() at all.
> 
> Fixes: 9c32fea83692 ("ALSA: hda - Add DP-MST support for non-acomp codecs")
> BugLink: https://github.com/thesofproject/linux/issues/1537
> Cc: Nikhil Mahale <nmahale@nvidia.com>
> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>

Applied now.  Thanks.


Takashi
Nikhil Mahale Nov. 28, 2019, 4:11 a.m. UTC | #2 
Sorry for this regression,

This idea to copy the values directly from spec->cvt_nids here without the
override hack as we have now, seems good to me.

Thanks,
Nikhil mahale

On 11/27/19 9:42 PM, Kai Vehmanen wrote:
> Fix regression in how intel_haswell_fixup_connect_list()
> results are used in hda_read_pin_conn(). Use of
> snd_hda_get_raw_connections() in hda_read_pin_conn() bypasses
> the cache and thus also bypasses the overridden pin connection
> list. On platforms that require the connection list fixup,
> mux list will be empty and HDMI playback will fail to -EBUSY
> at open.
> 
> Fix the regression in hda_read_pinn_conn(). Simplify code
> as suggested by Takashi Iwai to remove old
> intel_haswell_fixup_connect_list() and copy the cvt_nid list
> directly and not use snd_hda_override_conn_list() at all.
> 
> Fixes: 9c32fea83692 ("ALSA: hda - Add DP-MST support for non-acomp codecs")
> BugLink: https://github.com/thesofproject/linux/issues/1537
> Cc: Nikhil Mahale <nmahale@nvidia.com>
> Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
> ---
>  sound/pci/hda/patch_hdmi.c | 38 ++++++++++++--------------------------
>  1 file changed, 12 insertions(+), 26 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 55d20e40a195..373ca99b7a2f 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -1302,6 +1302,7 @@ static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx)
>  	struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
>  	hda_nid_t pin_nid = per_pin->pin_nid;
>  	int dev_id = per_pin->dev_id;
> +	int conns;
>  
>  	if (!(get_wcaps(codec, pin_nid) & AC_WCAP_CONN_LIST)) {
>  		codec_warn(codec,
> @@ -1312,10 +1313,18 @@ static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx)
>  
>  	snd_hda_set_dev_select(codec, pin_nid, dev_id);
>  
> +	if (spec->intel_hsw_fixup) {
> +		conns = spec->num_cvts;
> +		memcpy(per_pin->mux_nids, spec->cvt_nids,
> +		       sizeof(hda_nid_t) * conns);
> +	} else {
> +		conns = snd_hda_get_raw_connections(codec, pin_nid,
> +						    per_pin->mux_nids,
> +						    HDA_MAX_CONNECTIONS);
> +	}
> +
>  	/* all the device entries on the same pin have the same conn list */
> -	per_pin->num_mux_nids =
> -		snd_hda_get_raw_connections(codec, pin_nid, per_pin->mux_nids,
> -					    HDA_MAX_CONNECTIONS);
> +	per_pin->num_mux_nids = conns;
>  
>  	return 0;
>  }
> @@ -1713,9 +1722,6 @@ static void hdmi_repoll_eld(struct work_struct *work)
>  	mutex_unlock(&spec->pcm_lock);
>  }
>  
> -static void intel_haswell_fixup_connect_list(struct hda_codec *codec,
> -					     hda_nid_t nid);
> -
>  static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
>  {
>  	struct hdmi_spec *spec = codec->spec;
> @@ -1790,8 +1796,6 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
>  		per_pin->dev_id = i;
>  		per_pin->non_pcm = false;
>  		snd_hda_set_dev_select(codec, pin_nid, i);
> -		if (spec->intel_hsw_fixup)
> -			intel_haswell_fixup_connect_list(codec, pin_nid);
>  		err = hdmi_read_pin_conn(codec, pin_idx);
>  		if (err < 0)
>  			return err;
> @@ -2603,24 +2607,6 @@ static void generic_acomp_init(struct hda_codec *codec,
>   * Intel codec parsers and helpers
>   */
>  
> -static void intel_haswell_fixup_connect_list(struct hda_codec *codec,
> -					     hda_nid_t nid)
> -{
> -	struct hdmi_spec *spec = codec->spec;
> -	hda_nid_t conns[4];
> -	int nconns;
> -
> -	nconns = snd_hda_get_raw_connections(codec, nid, conns,
> -					     ARRAY_SIZE(conns));
> -	if (nconns == spec->num_cvts &&
> -	    !memcmp(conns, spec->cvt_nids, spec->num_cvts * sizeof(hda_nid_t)))
> -		return;
> -
> -	/* override pins connection list */
> -	codec_dbg(codec, "hdmi: haswell: override pin connection 0x%x\n", nid);
> -	snd_hda_override_conn_list(codec, nid, spec->num_cvts, spec->cvt_nids);
> -}
> -
>  #define INTEL_GET_VENDOR_VERB	0xf81
>  #define INTEL_SET_VENDOR_VERB	0x781
>  #define INTEL_EN_DP12		0x02	/* enable DP 1.2 features */
>
diff mbox series

Patch

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 55d20e40a195..373ca99b7a2f 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1302,6 +1302,7 @@  static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx)
 	struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
 	hda_nid_t pin_nid = per_pin->pin_nid;
 	int dev_id = per_pin->dev_id;
+	int conns;
 
 	if (!(get_wcaps(codec, pin_nid) & AC_WCAP_CONN_LIST)) {
 		codec_warn(codec,
@@ -1312,10 +1313,18 @@  static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx)
 
 	snd_hda_set_dev_select(codec, pin_nid, dev_id);
 
+	if (spec->intel_hsw_fixup) {
+		conns = spec->num_cvts;
+		memcpy(per_pin->mux_nids, spec->cvt_nids,
+		       sizeof(hda_nid_t) * conns);
+	} else {
+		conns = snd_hda_get_raw_connections(codec, pin_nid,
+						    per_pin->mux_nids,
+						    HDA_MAX_CONNECTIONS);
+	}
+
 	/* all the device entries on the same pin have the same conn list */
-	per_pin->num_mux_nids =
-		snd_hda_get_raw_connections(codec, pin_nid, per_pin->mux_nids,
-					    HDA_MAX_CONNECTIONS);
+	per_pin->num_mux_nids = conns;
 
 	return 0;
 }
@@ -1713,9 +1722,6 @@  static void hdmi_repoll_eld(struct work_struct *work)
 	mutex_unlock(&spec->pcm_lock);
 }
 
-static void intel_haswell_fixup_connect_list(struct hda_codec *codec,
-					     hda_nid_t nid);
-
 static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
 {
 	struct hdmi_spec *spec = codec->spec;
@@ -1790,8 +1796,6 @@  static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
 		per_pin->dev_id = i;
 		per_pin->non_pcm = false;
 		snd_hda_set_dev_select(codec, pin_nid, i);
-		if (spec->intel_hsw_fixup)
-			intel_haswell_fixup_connect_list(codec, pin_nid);
 		err = hdmi_read_pin_conn(codec, pin_idx);
 		if (err < 0)
 			return err;
@@ -2603,24 +2607,6 @@  static void generic_acomp_init(struct hda_codec *codec,
  * Intel codec parsers and helpers
  */
 
-static void intel_haswell_fixup_connect_list(struct hda_codec *codec,
-					     hda_nid_t nid)
-{
-	struct hdmi_spec *spec = codec->spec;
-	hda_nid_t conns[4];
-	int nconns;
-
-	nconns = snd_hda_get_raw_connections(codec, nid, conns,
-					     ARRAY_SIZE(conns));
-	if (nconns == spec->num_cvts &&
-	    !memcmp(conns, spec->cvt_nids, spec->num_cvts * sizeof(hda_nid_t)))
-		return;
-
-	/* override pins connection list */
-	codec_dbg(codec, "hdmi: haswell: override pin connection 0x%x\n", nid);
-	snd_hda_override_conn_list(codec, nid, spec->num_cvts, spec->cvt_nids);
-}
-
 #define INTEL_GET_VENDOR_VERB	0xf81
 #define INTEL_SET_VENDOR_VERB	0x781
 #define INTEL_EN_DP12		0x02	/* enable DP 1.2 features */