Re: [PATCH] ASoC: dapm: Fix snd_soc_dapm_put_volsw() connect

["Benoît Thébaudeau" <benoit.thebaudeau@advansee.com>]

On Fri, Jun 29, 2012 at 09:09:49PM +0200, Benoît Thébaudeau wrote:
> On Fri, Jun 29, 2012 at 06:22:28PM +0200, Takashi Iwai wrote:
> > At Fri, 29 Jun 2012 18:09:35 +0200 (CEST),
> > Benoît Thébaudeau wrote:
> > > 
> > > On Fri, Jun 29, 2012 at 05:44:41PM +0200, Takashi Iwai wrote:
> > > > BTW, looking at snd_soc_dapm_vol_getsw(), I found that
> > > > snd_soc_dapm_vol_putsw() doesn't handle a stereo case.
> > > > This needs a fix, too...
> > > 
> > > I saw that too. Is it the stereo case that should be added to put
> > > or removed
> > > from get, i.e. is it possible to handle a stereo case with only
> > > one
> > > connect
> > > flag?
> > 
> > I would assume that connect should be up when either left or right
> > is
> > on.  A stereo widget doesn't mean multiple connections.
> 
> I have started to look into that:
>  - Give a choice between reg and shift stereo for
>  snd_soc_dapm_get_volsw(), like
>    in snd_soc_get_volsw().
>  - Add stereo to the first case of dapm_set_path_status().
>  - Add stereo to dapm_widget_update() and struct snd_soc_dapm_update.
>  - Then comes snd_soc_dapm_get_volsw(): What should we do with
>    "widget->value = val;"? Add an rvalue to struct
>    snd_soc_dapm_widget? This
>    field does not seem to be read anywhere except for enums. There is
>    also
>    saved_value and other fields that do not seem to be used anywhere.

I've finished. The following patch should be applied after my patch
"ASoC: dapm: Fix dapm_set_path_status() connect".

Regards,
Benoît


[PATCH] ASoC: dapm: Fix/add support for stereo widgets

This patch:
* adds a choice in snd_soc_dapm_get_volsw_mut() for stereo between 1 and 2
  registers, like in snd_soc_get_volsw().
* fixes the missing stereo in other parts of dapm.
* removes the unused saved_value from struct snd_soc_dapm_widget.

Cc: Liam Girdwood <lrg@ti.com>
Cc: Mark Brown <broonie@opensource.wolfsonmicro.com>
Cc: <alsa-devel@alsa-project.org>
Signed-off-by: Benoît Thébaudeau <benoit.thebaudeau@advansee.com>
---
 .../include/sound/soc-dapm.h                       |   10 +--
 .../sound/soc/soc-dapm.c                           |   92 ++++++++++++++++----
 2 files changed, 82 insertions(+), 20 deletions(-)

diff --git linux-next-HEAD-d1d2d3a.orig/include/sound/soc-dapm.h linux-next-HEAD-d1d2d3a/include/sound/soc-dapm.h
index 05559e5..b5d38b9 100644
--- linux-next-HEAD-d1d2d3a.orig/include/sound/soc-dapm.h
+++ linux-next-HEAD-d1d2d3a/include/sound/soc-dapm.h
@@ -508,8 +508,8 @@ struct snd_soc_dapm_widget {
 	/* dapm control */
 	int reg;				/* negative reg = no direct dapm */
 	unsigned char shift;			/* bits to shift */
-	unsigned int saved_value;		/* widget saved value */
-	unsigned int value;				/* widget current value */
+	unsigned int value;			/* widget current value */
+	unsigned int rvalue;			/* widget current value of right channel */
 	unsigned int mask;			/* non-shifted mask */
 	unsigned int on_val;			/* on state value */
 	unsigned int off_val;			/* off state value */
@@ -552,9 +552,9 @@ struct snd_soc_dapm_widget {
 struct snd_soc_dapm_update {
 	struct snd_soc_dapm_widget *widget;
 	struct snd_kcontrol *kcontrol;
-	int reg;
-	int mask;
-	int val;
+	int reg, rreg;
+	int mask, rmask;
+	int val, rval;
 };
 
 /* DAPM context */
diff --git linux-next-HEAD-d1d2d3a.orig/sound/soc/soc-dapm.c linux-next-HEAD-d1d2d3a/sound/soc/soc-dapm.c
index 7f2a4bb..341dade 100644
--- linux-next-HEAD-d1d2d3a.orig/sound/soc/soc-dapm.c
+++ linux-next-HEAD-d1d2d3a/sound/soc/soc-dapm.c
@@ -313,11 +313,13 @@ static void dapm_set_path_status(struct snd_soc_dapm_widget *w,
 	case snd_soc_dapm_switch:
 	case snd_soc_dapm_mixer:
 	case snd_soc_dapm_mixer_named_ctl: {
-		int val;
+		int val, val2 = 0;
 		struct soc_mixer_control *mc = (struct soc_mixer_control *)
 			w->kcontrol_news[i].private_value;
 		unsigned int reg = mc->reg;
+		unsigned int reg2 = mc->rreg;
 		unsigned int shift = mc->shift;
+		unsigned int rshift = mc->rshift;
 		int max = mc->max;
 		unsigned int mask = (1 << fls(max)) - 1;
 		unsigned int invert = mc->invert;
@@ -327,7 +329,19 @@ static void dapm_set_path_status(struct snd_soc_dapm_widget *w,
 		if (invert)
 			val = max - val;
 
-		p->connect = !!val;
+		if (snd_soc_volsw_is_stereo(mc)) {
+			if (reg == reg2) {
+				val2 = soc_widget_read(w, reg);
+				val2 = (val2 >> rshift) & mask;
+			} else {
+				val2 = soc_widget_read(w, reg2);
+				val2 = (val2 >> shift) & mask;
+			}
+			if (invert)
+				val2 = max - val2;
+		}
+
+		p->connect = val || val2;
 	}
 	break;
 	case snd_soc_dapm_mux: {
@@ -1397,6 +1411,14 @@ static void dapm_widget_update(struct snd_soc_dapm_context *dapm)
 	if (ret < 0)
 		pr_err("%s DAPM update failed: %d\n", w->name, ret);
 
+	if (update->rmask) {
+		ret = soc_widget_update_bits_locked(w, update->rreg,
+					  update->rmask, update->rval);
+		if (ret < 0)
+			pr_err("%s DAPM right channel update failed: %d\n",
+			       w->name, ret);
+	}
+
 	if (w->event &&
 	    (w->event_flags & SND_SOC_DAPM_POST_REG)) {
 		ret = w->event(w, update->kcontrol, SND_SOC_DAPM_POST_REG);
@@ -2462,21 +2484,29 @@ int snd_soc_dapm_get_volsw(struct snd_kcontrol *kcontrol,
 	struct soc_mixer_control *mc =
 		(struct soc_mixer_control *)kcontrol->private_value;
 	unsigned int reg = mc->reg;
+	unsigned int reg2 = mc->rreg;
 	unsigned int shift = mc->shift;
 	unsigned int rshift = mc->rshift;
 	int max = mc->max;
-	unsigned int invert = mc->invert;
 	unsigned int mask = (1 << fls(max)) - 1;
+	unsigned int invert = mc->invert;
 
 	ucontrol->value.integer.value[0] =
 		(snd_soc_read(widget->codec, reg) >> shift) & mask;
-	if (shift != rshift)
-		ucontrol->value.integer.value[1] =
-			(snd_soc_read(widget->codec, reg) >> rshift) & mask;
-	if (invert) {
+	if (invert)
 		ucontrol->value.integer.value[0] =
 			max - ucontrol->value.integer.value[0];
-		if (shift != rshift)
+
+	if (snd_soc_volsw_is_stereo(mc)) {
+		if (reg == reg2)
+			ucontrol->value.integer.value[1] =
+				(snd_soc_read(widget->codec, reg) >> rshift) &
+						mask;
+		else
+			ucontrol->value.integer.value[1] =
+				(snd_soc_read(widget->codec, reg2) >> shift) &
+						mask;
+		if (invert)
 			ucontrol->value.integer.value[1] =
 				max - ucontrol->value.integer.value[1];
 	}
@@ -2504,11 +2534,15 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol,
 	struct soc_mixer_control *mc =
 		(struct soc_mixer_control *)kcontrol->private_value;
 	unsigned int reg = mc->reg;
+	unsigned int reg2 = mc->rreg;
 	unsigned int shift = mc->shift;
+	unsigned int rshift = mc->rshift;
 	int max = mc->max;
 	unsigned int mask = (1 << fls(max)) - 1;
 	unsigned int invert = mc->invert;
-	unsigned int val;
+	bool type_2r = 0;
+	unsigned int val2 = 0;
+	unsigned int val, val_mask;
 	int connect, change;
 	struct snd_soc_dapm_update update;
 	int wi;
@@ -2518,27 +2552,53 @@ int snd_soc_dapm_put_volsw(struct snd_kcontrol *kcontrol,
 
 	if (invert)
 		val = max - val;
-	mask = mask << shift;
+	val_mask = mask << shift;
 	val = val << shift;
 
+	if (snd_soc_volsw_is_stereo(mc)) {
+		val2 = (ucontrol->value.integer.value[1] & mask);
+		connect |= !!val2;
+
+		if (invert)
+			val2 = max - val2;
+		if (reg == reg2) {
+			val_mask |= mask << rshift;
+			val |= val2 << rshift;
+		} else {
+			val2 = val2 << shift;
+			type_2r = 1;
+		}
+	}
+
 	mutex_lock_nested(&card->dapm_mutex, SND_SOC_DAPM_CLASS_RUNTIME);
 
-	change = snd_soc_test_bits(widget->codec, reg, mask, val);
+	change = snd_soc_test_bits(widget->codec, reg, val_mask, val);
+	if (type_2r)
+		change |= snd_soc_test_bits(widget->codec,
+					    reg2, val_mask, val2);
+
 	if (change) {
 		for (wi = 0; wi < wlist->num_widgets; wi++) {
 			widget = wlist->widgets[wi];
 
 			widget->value = val;
-
 			update.kcontrol = kcontrol;
 			update.widget = widget;
 			update.reg = reg;
-			update.mask = mask;
+			update.mask = val_mask;
 			update.val = val;
-			widget->dapm->update = &update;
 
-			soc_dapm_mixer_update_power(widget, kcontrol, connect);
+			if (type_2r) {
+				widget->rvalue = val2;
+				update.rreg = reg2;
+				update.rmask = val_mask;
+				update.rval = val2;
+			} else {
+				update.rmask = 0;
+			}
 
+			widget->dapm->update = &update;
+			soc_dapm_mixer_update_power(widget, kcontrol, connect);
 			widget->dapm->update = NULL;
 		}
 	}
@@ -2627,6 +2687,7 @@ int snd_soc_dapm_put_enum_double(struct snd_kcontrol *kcontrol,
 			update.reg = e->reg;
 			update.mask = mask;
 			update.val = val;
+			update.rmask = 0;
 			widget->dapm->update = &update;
 
 			soc_dapm_mux_update_power(widget, kcontrol, mux, e);
@@ -2793,6 +2854,7 @@ int snd_soc_dapm_put_value_enum_double(struct snd_kcontrol *kcontrol,
 			update.reg = e->reg;
 			update.mask = mask;
 			update.val = val;
+			update.rmask = 0;
 			widget->dapm->update = &update;
 
 			soc_dapm_mux_update_power(widget, kcontrol, mux, e);
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel