Re: [PATCH] ASoC: dapm: Add support for multi register mux

[Takashi Iwai <tiwai@suse.de>]

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

---
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;