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