ASoC: dapm: Add support for multi register mux

Commit Message

Takashi Iwai April 3, 2014, 3:06 p.m. UTC

At Thu, 03 Apr 2014 15:31:58 +0200,
Lars-Peter Clausen wrote:
> 
> On 04/03/2014 11:53 AM, Mark Brown wrote:
> > On Thu, Apr 03, 2014 at 11:47:15AM +0200, Takashi Iwai wrote:
> >
> >> I'm a bit late in the game, but I feel a bit uneasy through looking
> >> at the whole changes.  My primary question is, whether do we really
> >> need to share the same struct soc_enum for the onehot type?  What
> >> makes hard to use a struct soc_enum_onehot for them?  You need
> >> different individual get/put for each type.  We may still need to
> >> change soc_dapm_update stuff, but it's different from sharing
> >> soc_enum.
> >
> > Indeed, I had thought this was where the discussion was heading - not
> > looked at this version of the patch yet.
> >
> 
> It would be nice, but it also requires some slight restructuring. The issue 
> we have right now is that there is  strictly speaking a bit of a layering 
> violation. The DAPM widgets should not need to know how the kcontrols that 
> are attached to the widget do their IO. What we essentially do in 
> dapm_connect_mux() (and also dapm_connect_mixer) is an open-coded version of 
> the controls get handler. Replacing that by calling the get handler instead 
> should allow us to use different structs for enums and onehot enums.

So, something like below?  It's totally untested, just a proof of
concept.

If the performance matters, we can optimize it by checking
kcontrol->get == snd_soc_dapm_get_enum_double or kcontrol->get ==
snd_soc_dapm_get_volsw and bypass to the current open-code functions
instead of the generic get/put callers.


thanks,

Takashi

---

Comments

Mark Brown April 3, 2014, 4:02 p.m. UTC | #1

On Thu, Apr 03, 2014 at 05:06:28PM +0200, Takashi Iwai wrote:
> Lars-Peter Clausen wrote:

> > It would be nice, but it also requires some slight restructuring. The issue 
> > we have right now is that there is  strictly speaking a bit of a layering 
> > violation. The DAPM widgets should not need to know how the kcontrols that 
> > are attached to the widget do their IO. What we essentially do in 
> > dapm_connect_mux() (and also dapm_connect_mixer) is an open-coded version of 
> > the controls get handler. Replacing that by calling the get handler instead 
> > should allow us to use different structs for enums and onehot enums.

Right, that's because DAPM has always just reimplemented everything and
the virtual controls were grafted on the side of the existing register
stuff rather than adding an abstraction layer.

> So, something like below?  It's totally untested, just a proof of
> concept.

Yes, that looks sensible to me but I didn't test it yet either.

> If the performance matters, we can optimize it by checking
> kcontrol->get == snd_soc_dapm_get_enum_double or kcontrol->get ==
> snd_soc_dapm_get_volsw and bypass to the current open-code functions
> instead of the generic get/put callers.

Performance isn't that big a deal, probably avoiding the alloc/frees
by just keeping one around somewhere convenient would be useful too.

Patch

diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c
index c8a780d0d057..5947c6e2fcc8 100644
--- a/sound/soc/soc-dapm.c
+++ b/sound/soc/soc-dapm.c
@@ -508,64 +508,71 @@  out:
 static int dapm_connect_mux(struct snd_soc_dapm_context *dapm,
 	struct snd_soc_dapm_widget *src, struct snd_soc_dapm_widget *dest,
 	struct snd_soc_dapm_path *path, const char *control_name,
-	const struct snd_kcontrol_new *kcontrol)
+	struct snd_kcontrol *kcontrol)
 {
-	struct soc_enum *e = (struct soc_enum *)kcontrol->private_value;
-	unsigned int val, item;
-	int i;
+	struct snd_ctl_elem_info *uinfo;
+	struct snd_ctl_elem_value *ucontrol;
+	unsigned int i, item, items;
+	int err;
 
-	if (e->reg != SND_SOC_NOPM) {
-		soc_widget_read(dest, e->reg, &val);
-		val = (val >> e->shift_l) & e->mask;
-		item = snd_soc_enum_val_to_item(e, val);
-	} else {
-		/* since a virtual mux has no backing registers to
-		 * decide which path to connect, it will try to match
-		 * with the first enumeration.  This is to ensure
-		 * that the default mux choice (the first) will be
-		 * correctly powered up during initialization.
-		 */
-		item = 0;
+	uinfo = kzalloc(sizeof(*uinfo), GFP_KERNEL);
+	ucontrol = kzalloc(sizeof(*ucontrol), GFP_KERNEL);
+	if (!uinfo || !ucontrol) {
+		err = -ENOMEM;
+		goto out;
+	}
+
+	err = kcontrol->info(kcontrol, uinfo);
+	if (err < 0)
+		goto out;
+	if (WARN_ON(uinfo->type != SNDRV_CTL_ELEM_TYPE_ENUMERATED)) {
+		err = -EINVAL;
+		goto out;
 	}
+	items = uinfo->value.enumerated.items;
 
-	for (i = 0; i < e->items; i++) {
-		if (!(strcmp(control_name, e->texts[i]))) {
+	err = kcontrol->get(kcontrol, ucontrol);
+	if (err < 0)
+		goto out;
+	item = ucontrol->value.enumerated.item[0];
+
+	for (i = 0; i < items; i++) {
+		uinfo->value.enumerated.item = i;
+		err = kcontrol->info(kcontrol, uinfo);
+		if (err < 0)
+			goto out;
+		if (!(strcmp(control_name, uinfo->value.enumerated.name))) {
 			list_add(&path->list, &dapm->card->paths);
 			list_add(&path->list_sink, &dest->sources);
 			list_add(&path->list_source, &src->sinks);
-			path->name = (char*)e->texts[i];
+			path->name = control_name;
 			if (i == item)
 				path->connect = 1;
 			else
 				path->connect = 0;
-			return 0;
+			goto out;
 		}
 	}
 
-	return -ENODEV;
+	err = -ENODEV;
+ out:
+	kfree(ucontrol);
+	kfree(uinfo);
+	return err < 0 ? err : 0;
 }
 
 /* set up initial codec paths */
 static void dapm_set_mixer_path_status(struct snd_soc_dapm_widget *w,
 	struct snd_soc_dapm_path *p, int i)
 {
-	struct soc_mixer_control *mc = (struct soc_mixer_control *)
-		w->kcontrol_news[i].private_value;
-	unsigned int reg = mc->reg;
-	unsigned int shift = mc->shift;
-	unsigned int max = mc->max;
-	unsigned int mask = (1 << fls(max)) - 1;
-	unsigned int invert = mc->invert;
-	unsigned int val;
+	struct snd_kcontrol *kcontrol = w->kcontrols[i];
+	struct snd_ctl_elem_value *ucontrol =
+		kzalloc(sizeof(*ucontrol), GFP_KERNEL);
 
-	if (reg != SND_SOC_NOPM) {
-		soc_widget_read(w, reg, &val);
-		val = (val >> shift) & mask;
-		if (invert)
-			val = max - val;
-		p->connect = !!val;
-	} else {
-		p->connect = 0;
+	if (ucontrol) {
+		if (kcontrol->get(kcontrol, ucontrol) >= 0)
+			p->connect = !!ucontrol->value.integer.value[0];
+		kfree(ucontrol);
 	}
 }
 
@@ -2415,7 +2422,7 @@  static int snd_soc_dapm_add_path(struct snd_soc_dapm_context *dapm,
 		return 0;
 	case snd_soc_dapm_mux:
 		ret = dapm_connect_mux(dapm, wsource, wsink, path, control,
-			&wsink->kcontrol_news[0]);
+				       wsink->kcontrols[0]);
 		if (ret != 0)
 			goto err;
 		break;