Dear Peter Ujfalusi,
On Wed, Feb 4, 2015 at 11:02 AM, Peter Ujfalusi peter.ujfalusi@ti.com wrote:
On 02/04/2015 11:33 AM, Benoît Thébaudeau wrote:
What I meant was to do the following instead:
/*
- ADC PGA mix input volumes. From -12 to 0 dB in 1.5 dB steps. Disconnected
- below -12 dB
*/ static const DECLARE_TLV_DB_SCALE(mix_tlv, -1350, 150, 1);
/* Left PGA Mixer for tlv320aic3104 */ static const struct snd_kcontrol_new aic3104_left_pga_mixer_controls[] = { SOC_DAPM_SINGLE_MUT_TLV("Line1L Volume", LINE1L_2_LADC_CTRL, 3, 7, 15, 1, mix_tlv),
You mean SOC_DAPM_SINGLE_TLV() ?
No. It would not fit our needs here.
There is no _MUT_ variant AFAIK.
Indeed, but support could be added, for it, something like (might need updating and some other changes in DAPM):
#define SOC_DAPM_SINGLE_MUT_TLV(xname, reg, shift, min, max, invert, tlv_array) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \ .info = snd_soc_info_volsw_mut, \ .access = SNDRV_CTL_ELEM_ACCESS_TLV_READ | SNDRV_CTL_ELEM_ACCESS_READWRITE,\ .tlv.p = (tlv_array), \ .get = snd_soc_dapm_get_volsw_mut, .put = snd_soc_dapm_put_volsw_mut, \ .private_value = SOC_SINGLE_MUT_VALUE(reg, shift, min, max, invert) }
/** * snd_soc_dapm_get_volsw_mut - dapm mixer get callback * @kcontrol: mixer control * @ucontrol: control element information * * Callback to get the value of a dapm mixer control with a hole separating the * mute value from the other valid values. * * Returns 0 for success. */ int snd_soc_dapm_get_volsw_mut(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct snd_soc_dapm_widget_list *wlist = snd_kcontrol_chip(kcontrol); struct snd_soc_dapm_widget *widget = wlist->widgets[0]; struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; unsigned int reg = mc->reg; unsigned int shift = mc->shift; unsigned int rshift = mc->rshift; int min = mc->min; int max = mc->max; unsigned int invert = mc->invert; unsigned int mask = (1 << fls(max)) - 1;
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) { ucontrol->value.integer.value[0] = mask - ucontrol->value.integer.value[0]; if (shift != rshift) ucontrol->value.integer.value[1] = mask - ucontrol->value.integer.value[1]; } if (ucontrol->value.integer.value[0] > 0) ucontrol->value.integer.value[0] -= min - 1; if (shift != rshift) if (ucontrol->value.integer.value[1] > 0) ucontrol->value.integer.value[1] -= min - 1;
return 0; } EXPORT_SYMBOL_GPL(snd_soc_dapm_get_volsw_mut);
/** * snd_soc_dapm_put_volsw_mut - dapm mixer set callback * @kcontrol: mixer control * @ucontrol: control element information * * Callback to set the value of a dapm mixer control with a hole separating the * mute value from the other valid values. * * Returns 0 for success. */ int snd_soc_dapm_put_volsw_mut(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct snd_soc_dapm_widget_list *wlist = snd_kcontrol_chip(kcontrol); struct snd_soc_dapm_widget *widget = wlist->widgets[0]; struct snd_soc_codec *codec = widget->codec; struct soc_mixer_control *mc = (struct soc_mixer_control *)kcontrol->private_value; unsigned int reg = mc->reg; unsigned int shift = mc->shift; int min = mc->min; int max = mc->max; unsigned int mask = (1 << fls(max)) - 1; unsigned int invert = mc->invert; unsigned int val; int connect, change; struct snd_soc_dapm_update update; int wi;
val = (ucontrol->value.integer.value[0] & mask); connect = !!val;
if (val) val += min - 1; if (invert) val = mask - val; mask = mask << shift; val = val << shift;
mutex_lock(&codec->mutex);
change = snd_soc_test_bits(widget->codec, reg, mask, val); 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.val = val; widget->dapm->update = &update;
dapm_mixer_update_power(widget, kcontrol, connect);
widget->dapm->update = NULL; } }
mutex_unlock(&codec->mutex); return 0; } EXPORT_SYMBOL_GPL(snd_soc_dapm_put_volsw_mut);
According to the datasheets of aic3106, aic3104: 0b0000 - 0b1000 is valid (0 - -12 dB) 0b1001 - 0b1110 is reserved, do not write these sequences to the register 0b1111 is disconnected.
And for fun the aic3007's reg20 for example: 0b0000 - 0dB 0b0001 - 0b0011 - reserved 0b0100 - -6dB 0b0101 - 0b0111 - reserved 0b1000 - -12dB 0b1001 - 0b1110 - reserved 0b1111 - disconnected
Note that the driver never had control for these gains and the aic3104 support is following this behavior, but if we do it for aic3104 we should do it for all other support codecs as well as a followup patch or series. IMHO.
I had not checked the other CODECs. So for all these CODECs, toggling either as 0b0000/0b0001 or as 0b1110/0b1111 (like the driver currently does) is wrong. I agree that they should be fixed too. But for them, it's even more complicated because of the multiple reserved value ranges. Or maybe we could just ignore these reserved ranges and handle the mute value, but that would mean relying on the user not to use reserved values. So maybe use TLV_DB_RANGE_HEAD() and TLV_DB_SCALE_ITEM() to define mix_tlv, then use SOC_(DAPM_)SINGLE_TLV().
Best regards, Benoît