[alsa-devel] [RFC]Connection list of hda widget

Takashi Iwai tiwai at suse.de
Thu Feb 7 18:27:27 CET 2013


At Thu, 07 Feb 2013 16:09:35 +0100,
David Henningsson wrote:
> 
> On 02/07/2013 06:38 AM, Raymond Yau wrote:
> >
> > Treat vendor defined widget different from audio selector/audio input
> > widget since hda_proc.c query the connection list of vendor defined widget
> >
> > ADI codecs  have vendor defined widgets ( BIAS Power-Down and VREF
> > Power-Down) which have connection list
> >
> > static void print_conn_list(struct snd_info_buffer *buffer,
> >      struct hda_codec *codec, hda_nid_t nid,
> >      unsigned int wid_type, hda_nid_t *conn,
> >      int conn_len)
> > {
> > int c, curr = -1;
> >
> > if (conn_len > 1 &&
> >      wid_type != AC_WID_AUD_MIX &&
> >      wid_type != AC_WID_VOL_KNB &&
> > +     wide_type != AC_WID_VENDOR &&
> >      wid_type != AC_WID_POWER)
> > curr = snd_hda_codec_read(codec, nid, 0,
> >    AC_VERB_GET_CONNECT_SEL, 0);
> > snd_iprintf(buffer, "  Connection: %d\n", conn_len);
> >
> 
> I think there are some Sigmatel/IDT (or was it Analog Devices?) codecs 
> that actually have sensible connections for their vendor specific nodes, 
> so I would prefer not to change this.

Right.  The widget connection longer than 32 itself is OK. It's just
ugly.  That being said, it's basically a bug that we restrict the max
connection size in the core code.

So I applied the patch below.  This removes the limit while we still
keep the efficiency (except for proc output, which isn't too
critical).  The patch became bigger than I wanted originally, but it's
life.


thanks,

Takashi

---
From: Takashi Iwai <tiwai at suse.de>
Subject: [PATCH] ALSA: hda - Remove limit of widget connections

Currently we set the max number of connections to be 32, but there
seems codec that gives longer connection lists like AD1988, and we see
errors in proc output and else.  (Though, in the case of AD1988, it's
a list of all codecs connected to a single vendor widget, so this must
be something fishy, but it's still valid from the h/w design POV.)

This patch tries to remove this restriction.  For efficiency, we still
use the fixed size array in the parser, but takes a dynamic array when
the size is reported to be greater than that.

Now the fixed array size is found only in patch_hdmi.c, but it should
be fine, as the codec itself can't support so many pins.

Reported-by: Raymond Yau <superquad.vortex2 at gmail.com>
Signed-off-by: Takashi Iwai <tiwai at suse.de>
---
 sound/pci/hda/hda_codec.c  | 62 ++++++++++++++++++++++++++++++----------------
 sound/pci/hda/hda_codec.h  |  4 +--
 sound/pci/hda/hda_proc.c   | 19 +++++++++++---
 sound/pci/hda/patch_hdmi.c |  3 +++
 4 files changed, 60 insertions(+), 28 deletions(-)

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index f82a64d..3c92514 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -382,13 +382,23 @@ static void remove_conn_list(struct hda_codec *codec)
 /* read the connection and add to the cache */
 static int read_and_add_raw_conns(struct hda_codec *codec, hda_nid_t nid)
 {
-	hda_nid_t list[HDA_MAX_CONNECTIONS];
+	hda_nid_t list[32];
+	hda_nid_t *result = list;
 	int len;
 
 	len = snd_hda_get_raw_connections(codec, nid, list, ARRAY_SIZE(list));
-	if (len < 0)
-		return len;
-	return snd_hda_override_conn_list(codec, nid, len, list);
+	if (len == -ENOSPC) {
+		len = snd_hda_get_num_raw_conns(codec, nid);
+		result = kmalloc(sizeof(hda_nid_t) * len, GFP_KERNEL);
+		if (!result)
+			return -ENOMEM;
+		len = snd_hda_get_raw_connections(codec, nid, result, len);
+	}
+	if (len >= 0)
+		len = snd_hda_override_conn_list(codec, nid, len, result);
+	if (result != list)
+		kfree(result);
+	return len;
 }
 
 /**
@@ -466,6 +476,27 @@ int snd_hda_get_connections(struct hda_codec *codec, hda_nid_t nid,
 }
 EXPORT_SYMBOL_HDA(snd_hda_get_connections);
 
+/* return CONNLIST_LEN parameter of the given widget */
+static unsigned int get_num_conns(struct hda_codec *codec, hda_nid_t nid)
+{
+	unsigned int wcaps = get_wcaps(codec, nid);
+	unsigned int parm;
+
+	if (!(wcaps & AC_WCAP_CONN_LIST) &&
+	    get_wcaps_type(wcaps) != AC_WID_VOL_KNB)
+		return 0;
+
+	parm = snd_hda_param_read(codec, nid, AC_PAR_CONNLIST_LEN);
+	if (parm == -1)
+		parm = 0;
+	return parm;
+}
+
+int snd_hda_get_num_raw_conns(struct hda_codec *codec, hda_nid_t nid)
+{
+	return get_num_conns(codec, nid) & AC_CLIST_LENGTH;
+}
+
 /**
  * snd_hda_get_raw_connections - copy connection list without cache
  * @codec: the HDA codec
@@ -483,19 +514,16 @@ int snd_hda_get_raw_connections(struct hda_codec *codec, hda_nid_t nid,
 	unsigned int parm;
 	int i, conn_len, conns;
 	unsigned int shift, num_elems, mask;
-	unsigned int wcaps;
 	hda_nid_t prev_nid;
 	int null_count = 0;
 
 	if (snd_BUG_ON(!conn_list || max_conns <= 0))
 		return -EINVAL;
 
-	wcaps = get_wcaps(codec, nid);
-	if (!(wcaps & AC_WCAP_CONN_LIST) &&
-	    get_wcaps_type(wcaps) != AC_WID_VOL_KNB)
+	parm = get_num_conns(codec, nid);
+	if (!parm)
 		return 0;
 
-	parm = snd_hda_param_read(codec, nid, AC_PAR_CONNLIST_LEN);
 	if (parm & AC_CLIST_LONG) {
 		/* long form */
 		shift = 16;
@@ -552,21 +580,13 @@ int snd_hda_get_raw_connections(struct hda_codec *codec, hda_nid_t nid,
 				continue;
 			}
 			for (n = prev_nid + 1; n <= val; n++) {
-				if (conns >= max_conns) {
-					snd_printk(KERN_ERR "hda_codec: "
-						   "Too many connections %d for NID 0x%x\n",
-						   conns, nid);
-					return -EINVAL;
-				}
+				if (conns >= max_conns)
+					return -ENOSPC;
 				conn_list[conns++] = n;
 			}
 		} else {
-			if (conns >= max_conns) {
-				snd_printk(KERN_ERR "hda_codec: "
-					   "Too many connections %d for NID 0x%x\n",
-					   conns, nid);
-				return -EINVAL;
-			}
+			if (conns >= max_conns)
+				return -ENOSPC;
 			conn_list[conns++] = val;
 		}
 		prev_nid = val;
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 6c59272..0be1826 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -551,9 +551,6 @@ enum {
 	AC_JACK_PORT_BOTH,
 };
 
-/* max. connections to a widget */
-#define HDA_MAX_CONNECTIONS	32
-
 /* max. codec address */
 #define HDA_MAX_CODEC_ADDRESS	0x0f
 
@@ -958,6 +955,7 @@ snd_hda_get_num_conns(struct hda_codec *codec, hda_nid_t nid)
 {
 	return snd_hda_get_connections(codec, nid, NULL, 0);
 }
+int snd_hda_get_num_raw_conns(struct hda_codec *codec, hda_nid_t nid);
 int snd_hda_get_raw_connections(struct hda_codec *codec, hda_nid_t nid,
 			    hda_nid_t *conn_list, int max_conns);
 int snd_hda_get_conn_list(struct hda_codec *codec, hda_nid_t nid,
diff --git a/sound/pci/hda/hda_proc.c b/sound/pci/hda/hda_proc.c
index 5e02f26..97cdbef 100644
--- a/sound/pci/hda/hda_proc.c
+++ b/sound/pci/hda/hda_proc.c
@@ -623,7 +623,7 @@ static void print_codec_info(struct snd_info_entry *entry,
 			snd_hda_param_read(codec, nid,
 					   AC_PAR_AUDIO_WIDGET_CAP);
 		unsigned int wid_type = get_wcaps_type(wid_caps);
-		hda_nid_t conn[HDA_MAX_CONNECTIONS];
+		hda_nid_t *conn = NULL;
 		int conn_len = 0;
 
 		snd_iprintf(buffer, "Node 0x%02x [%s] wcaps 0x%x:", nid,
@@ -660,9 +660,18 @@ static void print_codec_info(struct snd_info_entry *entry,
 		if (wid_type == AC_WID_VOL_KNB)
 			wid_caps |= AC_WCAP_CONN_LIST;
 
-		if (wid_caps & AC_WCAP_CONN_LIST)
-			conn_len = snd_hda_get_raw_connections(codec, nid, conn,
-							   HDA_MAX_CONNECTIONS);
+		if (wid_caps & AC_WCAP_CONN_LIST) {
+			conn_len = snd_hda_get_num_raw_conns(codec, nid);
+			if (conn_len > 0) {
+				conn = kmalloc(sizeof(hda_nid_t) * conn_len,
+					       GFP_KERNEL);
+				if (!conn)
+					return;
+				if (snd_hda_get_raw_connections(codec, nid, conn,
+								conn_len) < 0)
+					conn_len = 0;
+			}
+		}
 
 		if (wid_caps & AC_WCAP_IN_AMP) {
 			snd_iprintf(buffer, "  Amp-In caps: ");
@@ -735,6 +744,8 @@ static void print_codec_info(struct snd_info_entry *entry,
 
 		if (codec->proc_widget_hook)
 			codec->proc_widget_hook(buffer, codec, nid);
+
+		kfree(conn);
 	}
 	snd_hda_power_down(codec);
 }
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 85236da..899c4fb 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -64,6 +64,9 @@ struct hdmi_spec_per_cvt {
 	unsigned int maxbps;
 };
 
+/* max. connections to a widget */
+#define HDA_MAX_CONNECTIONS	32
+
 struct hdmi_spec_per_pin {
 	hda_nid_t pin_nid;
 	int num_mux_nids;
-- 
1.8.1.2



More information about the Alsa-devel mailing list