Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1674024 > unrolled thread
| Started by | Danny Milosavljevic <dannym@scratchpost.org> |
|---|---|
| First post | 2017-06-24 08:30 +0200 |
| Last post | 2017-06-28 11:20 +0200 |
| Articles | 9 — 4 participants |
Back to article view | Back to linux.kernel
[PATCH v13 00/14] ASoC: sun4i-codec: Add Line-In, FM-In, Mic 2, Capture Source, Differential Line-In Danny Milosavljevic <dannym@scratchpost.org> - 2017-06-24 08:30 +0200
[PATCH v13 05/14] ASoC: sun4i-codec: Merge sun4i_codec_left_mixer_controls and sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls. Danny Milosavljevic <dannym@scratchpost.org> - 2017-06-24 08:30 +0200
Re: [PATCH v13 05/14] ASoC: sun4i-codec: Merge sun4i_codec_left_mixer_controls and sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls. Chen-Yu Tsai <wens@csie.org> - 2017-06-26 03:50 +0200
Re: [PATCH v13 05/14] ASoC: sun4i-codec: Merge sun4i_codec_left_mixer_controls and sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls. Maxime Ripard <maxime.ripard@free-electrons.com> - 2017-06-26 21:30 +0200
Re: [linux-sunxi] Re: [PATCH v13 05/14] ASoC: sun4i-codec: Merge sun4i_codec_left_mixer_controls and sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls. Danny Milosavljevic <dannym@scratchpost.org> - 2017-06-28 10:30 +0200
Re: [linux-sunxi] Re: [PATCH v13 05/14] ASoC: sun4i-codec: Merge sun4i_codec_left_mixer_controls and sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls. Chen-Yu Tsai <wens@csie.org> - 2017-06-28 10:40 +0200
Re: [PATCH v13 05/14] ASoC: sun4i-codec: Merge sun4i_codec_left_mixer_controls and sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls. Mark Brown <broonie@kernel.org> - 2017-06-28 20:20 +0200
[PATCH v13 08/14] ASoC: sun4i-codec: Add FM Left, FM Right, FM Playback Switch. Danny Milosavljevic <dannym@scratchpost.org> - 2017-06-24 08:30 +0200
Re: [PATCH v13 00/14] ASoC: sun4i-codec: Add Line-In, FM-In, Mic 2, Capture Source, Differential Line-In Chen-Yu Tsai <wens@csie.org> - 2017-06-28 11:20 +0200
| From | Danny Milosavljevic <dannym@scratchpost.org> |
|---|---|
| Date | 2017-06-24 08:30 +0200 |
| Subject | [PATCH v13 00/14] ASoC: sun4i-codec: Add Line-In, FM-In, Mic 2, Capture Source, Differential Line-In |
| Message-ID | <tVMJX-4iL-5@gated-at.bofh.it> |
This patchset adds some mixer controls to sun4i-codec for the Allwinner A10
and the Allwinner A20.
It also adds a mux for the capture source and the PGA for the MIC2 preamp.
Where possible, it uses SOC_DAPM_DOUBLE in order to cut down on the number
of distinct controls in alsamixer.
v13 changes compared to v12 are:
- Added my "Signed-off-by".
- Clarified some commit message text.
v12 changes compared to v11 are:
- Split up patchset in another way.
- Renamed "Mic1 Capture Volume" to "Mic1 Boost Volume".
- Renamed "Mic2 Capture Volume" to "Mic2 Boost Volume".
- Renamed "Line Capture Volume" to "Line Boost Volume".
- Renamed "Differential Line Capture Switch" to "Differential Line Source".
v11 changes compared to v10 are:
- Split up patchset.
- Fixed typo in Differential Line Capture Switch.
- Renamed "Non-Differential" value to "Stereo".
- Removed duplicate PA Volume mixer control.
v10 changes compared to v9 are:
- Use SOC_DAPM_DOUBLE where possible and it makes sense in order to cut
down on the number of controls.
v9 changes compared to v8 are:
- added Line Differential Capture Switch.
- split Capture Source into Left Capture Select, Right Capture Select.
- added Line Capture Volume.
- rename "sun4i_codec_widgets" to "sun4i_codec_controls" for
consistency with the struct field it's used in.
- rename "Line-In" to "Line".
- rename "Power Amplifier Playback Volume" to "Headphone Playback Volume".
v8 changes compared to v7 are:
- fixed the routes for line and mic capturing.
v7 changes compared to v6 are:
- preparation for different A20, A10 controls is now in an extra patch.
- all register definitions are now at the top.
- sun7i-specific things (A20-specific things) are now less
grouped-together.
- rename "Power Amplifier Volume" to "Power Amplifier Playback Volume".
v6 changes compared to v5 are:
- Mic preamplifier special cases for A20 and A10 now are now not
icky: There are two different _widget arrays and the probe() function
now selects the right one to pass to snd_soc_register_codec() unmodified.
- sun7i-specific things (A20-specific things) are now grouped together.
v5 changes compared to v4 are:
- Mic preamplifier controls have more common names now.
- Mic preamplifier scale has a 0 dB entry as well now, as documented in the
A20 user manual.
- Mic preamplifier has special cases for A20 and A10 now.
- Gain controls have "Gain" in the name now.
v4 changes compared to v3 are:
- names of the input are not uppercase anymore.
- bit index constants are now named as in the A20 user manual v1.4.
- added Mic1-In, Mac2-In.
- added Mic1 and Mic2 Pre-Amplifiers.
v3 changes compared to v2 are:
- added DAPM routes.
v2 changes compared to v1 are:
- moved Line-In and FM-In playback switches to their respective
sun4i_codec_*_mixer_controls.
v1 changes:
- added linein, fmin output volumes and switches.
Danny Milosavljevic (14):
sun4i-codec: Add MIC2 Pre-Amplifier, Mic2 input, Mic2 routes.
sun4i-codec: Add Mic Playback Volume.
sun4i-codec: Add support for extra controls to struct
sun4i_codec_quirks and use them.
sun4i-codec: Add Mic1 Boost Volume, Mic2 Boost Volume.
sun4i-codec: Merge sun4i_codec_left_mixer_controls and
sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls.
sun4i-codec: Add Mic1 Playback Switch, Mic2 Playback Switch.
sun4i-codec: Add FM Playback Volume.
sun4i-codec: Add FM Left, FM Right, FM Playback Switch.
sun4i-codec: Add Line Playback Volume.
sun4i-codec: Add Line Boost Volume.
sun4i-codec: Add Line Right, Line Left, Line Playback Switch.
sun4i-codec: Add Differential Line Source.
sun4i-codec: Add Left Capture Select, Right Capture Select.
sun4i-codec: Add Capture Volume.
sound/soc/sunxi/sun4i-codec.c | 233 +++++++++++++++++++++++++++++++++++++-----
1 file changed, 210 insertions(+), 23 deletions(-)
--
2.1.4
[toc] | [next] | [standalone]
| From | Danny Milosavljevic <dannym@scratchpost.org> |
|---|---|
| Date | 2017-06-24 08:30 +0200 |
| Subject | [PATCH v13 05/14] ASoC: sun4i-codec: Merge sun4i_codec_left_mixer_controls and sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls. |
| Message-ID | <tVMJY-4iL-29@gated-at.bofh.it> |
| In reply to | #1674024 |
Since it's now possible to have a DAPM mixer control with multiple channels,
use it to cut down the total number of controls.
Signed-off-by: Danny Milosavljevic <dannym@scratchpost.org>
---
sound/soc/sunxi/sun4i-codec.c | 27 ++++++++++++---------------
1 file changed, 12 insertions(+), 15 deletions(-)
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index c69b55c..3718137 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -685,15 +685,12 @@ static const struct snd_kcontrol_new sun7i_codec_extra_controls[] = {
sun7i_codec_micin_preamp_gain_scale),
};
-static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
- SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
- SUN4I_CODEC_DAC_ACTL_LDACLMIXS, 1, 0),
-};
-
-static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
- SOC_DAPM_SINGLE("Right DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
+static const struct snd_kcontrol_new sun4i_codec_mixer_controls[] = {
+ SOC_DAPM_DOUBLE("DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_LDACLMIXS,
SUN4I_CODEC_DAC_ACTL_RDACRMIXS, 1, 0),
- SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SOC_DAPM_SINGLE("Right Mixer Left DAC Playback Switch",
+ SUN4I_CODEC_DAC_ACTL,
SUN4I_CODEC_DAC_ACTL_LDACRMIXS, 1, 0),
};
@@ -729,11 +726,11 @@ static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
/* Mixers */
SND_SOC_DAPM_MIXER("Left Mixer", SND_SOC_NOPM, 0, 0,
- sun4i_codec_left_mixer_controls,
- ARRAY_SIZE(sun4i_codec_left_mixer_controls)),
+ sun4i_codec_mixer_controls,
+ ARRAY_SIZE(sun4i_codec_mixer_controls)),
SND_SOC_DAPM_MIXER("Right Mixer", SND_SOC_NOPM, 0, 0,
- sun4i_codec_right_mixer_controls,
- ARRAY_SIZE(sun4i_codec_right_mixer_controls)),
+ sun4i_codec_mixer_controls,
+ ARRAY_SIZE(sun4i_codec_mixer_controls)),
/* Global Mixer Enable */
SND_SOC_DAPM_SUPPLY("Mixer Enable", SUN4I_CODEC_DAC_ACTL,
@@ -775,12 +772,12 @@ static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
/* Right Mixer Routes */
{ "Right Mixer", NULL, "Mixer Enable" },
- { "Right Mixer", "Left DAC Playback Switch", "Left DAC" },
- { "Right Mixer", "Right DAC Playback Switch", "Right DAC" },
+ { "Right Mixer", "DAC Playback Switch", "Left DAC" },
+ { "Right Mixer", "Right Mixer Left DAC Playback Switch", "Left DAC" },
/* Left Mixer Routes */
{ "Left Mixer", NULL, "Mixer Enable" },
- { "Left Mixer", "Left DAC Playback Switch", "Left DAC" },
+ { "Left Mixer", "DAC Playback Switch", "Left DAC" },
/* Power Amplifier Routes */
{ "Power Amplifier", "Mixer Playback Switch", "Left Mixer" },
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Chen-Yu Tsai <wens@csie.org> |
|---|---|
| Date | 2017-06-26 03:50 +0200 |
| Subject | Re: [PATCH v13 05/14] ASoC: sun4i-codec: Merge sun4i_codec_left_mixer_controls and sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls. |
| Message-ID | <tWrk5-4f5-13@gated-at.bofh.it> |
| In reply to | #1674025 |
On Sat, Jun 24, 2017 at 2:24 PM, Danny Milosavljevic
<dannym@scratchpost.org> wrote:
> Since it's now possible to have a DAPM mixer control with multiple channels,
> use it to cut down the total number of controls.
>
> Signed-off-by: Danny Milosavljevic <dannym@scratchpost.org>
> ---
> sound/soc/sunxi/sun4i-codec.c | 27 ++++++++++++---------------
> 1 file changed, 12 insertions(+), 15 deletions(-)
>
> diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> index c69b55c..3718137 100644
> --- a/sound/soc/sunxi/sun4i-codec.c
> +++ b/sound/soc/sunxi/sun4i-codec.c
> @@ -685,15 +685,12 @@ static const struct snd_kcontrol_new sun7i_codec_extra_controls[] = {
> sun7i_codec_micin_preamp_gain_scale),
> };
>
> -static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
> - SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> - SUN4I_CODEC_DAC_ACTL_LDACLMIXS, 1, 0),
> -};
> -
> -static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
> - SOC_DAPM_SINGLE("Right DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> +static const struct snd_kcontrol_new sun4i_codec_mixer_controls[] = {
> + SOC_DAPM_DOUBLE("DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SUN4I_CODEC_DAC_ACTL_LDACLMIXS,
> SUN4I_CODEC_DAC_ACTL_RDACRMIXS, 1, 0),
This changes the mixer control names exposed to userspace. While I think
consolidating mixer controls is nice, giving the user a less cluttered
view, I'm not sure about the changing a possible userspace ABI.
Maxime, ALSA and ASoC people, any comments about this?
ChenYu
> - SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> + SOC_DAPM_SINGLE("Right Mixer Left DAC Playback Switch",
> + SUN4I_CODEC_DAC_ACTL,
> SUN4I_CODEC_DAC_ACTL_LDACRMIXS, 1, 0),
> };
>
> @@ -729,11 +726,11 @@ static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
>
> /* Mixers */
> SND_SOC_DAPM_MIXER("Left Mixer", SND_SOC_NOPM, 0, 0,
> - sun4i_codec_left_mixer_controls,
> - ARRAY_SIZE(sun4i_codec_left_mixer_controls)),
> + sun4i_codec_mixer_controls,
> + ARRAY_SIZE(sun4i_codec_mixer_controls)),
> SND_SOC_DAPM_MIXER("Right Mixer", SND_SOC_NOPM, 0, 0,
> - sun4i_codec_right_mixer_controls,
> - ARRAY_SIZE(sun4i_codec_right_mixer_controls)),
> + sun4i_codec_mixer_controls,
> + ARRAY_SIZE(sun4i_codec_mixer_controls)),
>
> /* Global Mixer Enable */
> SND_SOC_DAPM_SUPPLY("Mixer Enable", SUN4I_CODEC_DAC_ACTL,
> @@ -775,12 +772,12 @@ static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
>
> /* Right Mixer Routes */
> { "Right Mixer", NULL, "Mixer Enable" },
> - { "Right Mixer", "Left DAC Playback Switch", "Left DAC" },
> - { "Right Mixer", "Right DAC Playback Switch", "Right DAC" },
> + { "Right Mixer", "DAC Playback Switch", "Left DAC" },
> + { "Right Mixer", "Right Mixer Left DAC Playback Switch", "Left DAC" },
>
> /* Left Mixer Routes */
> { "Left Mixer", NULL, "Mixer Enable" },
> - { "Left Mixer", "Left DAC Playback Switch", "Left DAC" },
> + { "Left Mixer", "DAC Playback Switch", "Left DAC" },
>
> /* Power Amplifier Routes */
> { "Power Amplifier", "Mixer Playback Switch", "Left Mixer" },
> --
> 2.1.4
>
[toc] | [prev] | [next] | [standalone]
| From | Maxime Ripard <maxime.ripard@free-electrons.com> |
|---|---|
| Date | 2017-06-26 21:30 +0200 |
| Subject | Re: [PATCH v13 05/14] ASoC: sun4i-codec: Merge sun4i_codec_left_mixer_controls and sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls. |
| Message-ID | <tWHRU-6oQ-15@gated-at.bofh.it> |
| In reply to | #1674374 |
[Multipart message — attachments visible in raw view] — view raw
On Mon, Jun 26, 2017 at 09:48:59AM +0800, Chen-Yu Tsai wrote:
> On Sat, Jun 24, 2017 at 2:24 PM, Danny Milosavljevic
> <dannym@scratchpost.org> wrote:
> > Since it's now possible to have a DAPM mixer control with multiple channels,
> > use it to cut down the total number of controls.
> >
> > Signed-off-by: Danny Milosavljevic <dannym@scratchpost.org>
> > ---
> > sound/soc/sunxi/sun4i-codec.c | 27 ++++++++++++---------------
> > 1 file changed, 12 insertions(+), 15 deletions(-)
> >
> > diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
> > index c69b55c..3718137 100644
> > --- a/sound/soc/sunxi/sun4i-codec.c
> > +++ b/sound/soc/sunxi/sun4i-codec.c
> > @@ -685,15 +685,12 @@ static const struct snd_kcontrol_new sun7i_codec_extra_controls[] = {
> > sun7i_codec_micin_preamp_gain_scale),
> > };
> >
> > -static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
> > - SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> > - SUN4I_CODEC_DAC_ACTL_LDACLMIXS, 1, 0),
> > -};
> > -
> > -static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
> > - SOC_DAPM_SINGLE("Right DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> > +static const struct snd_kcontrol_new sun4i_codec_mixer_controls[] = {
> > + SOC_DAPM_DOUBLE("DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> > + SUN4I_CODEC_DAC_ACTL_LDACLMIXS,
> > SUN4I_CODEC_DAC_ACTL_RDACRMIXS, 1, 0),
>
> This changes the mixer control names exposed to userspace. While I think
> consolidating mixer controls is nice, giving the user a less cluttered
> view, I'm not sure about the changing a possible userspace ABI.
>
> Maxime, ALSA and ASoC people, any comments about this?
Yeah, breaking userspace is definitely not ok.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[toc] | [prev] | [next] | [standalone]
| From | Danny Milosavljevic <dannym@scratchpost.org> |
|---|---|
| Date | 2017-06-28 10:30 +0200 |
| Subject | Re: [linux-sunxi] Re: [PATCH v13 05/14] ASoC: sun4i-codec: Merge sun4i_codec_left_mixer_controls and sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls. |
| Message-ID | <tXgwi-4q8-7@gated-at.bofh.it> |
| In reply to | #1675048 |
Hi,
> > > -static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
> > > - SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> > > - SUN4I_CODEC_DAC_ACTL_LDACLMIXS, 1, 0),
> > > -};
> > > -
> > > -static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
> > > - SOC_DAPM_SINGLE("Right DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> > > +static const struct snd_kcontrol_new sun4i_codec_mixer_controls[] = {
> > > + SOC_DAPM_DOUBLE("DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> > > + SUN4I_CODEC_DAC_ACTL_LDACLMIXS,
> > > SUN4I_CODEC_DAC_ACTL_RDACRMIXS, 1, 0),
> >
> > This changes the mixer control names exposed to userspace. While I think
> > consolidating mixer controls is nice, giving the user a less cluttered
> > view, I'm not sure about the changing a possible userspace ABI.
> >
> > Maxime, ALSA and ASoC people, any comments about this?
>Yeah, breaking userspace is definitely not ok.
Okay. I can restore DAC Playback Switch as two different controls with the same
names as before.
Is the mixer itself (SND_SOC_DAPM_MIXER) - not talking about its controls -
exposed to userspace?
Because if not, I can still merge the left mixer and right mixer and just fix up
the control names for DAC Playback Switch to be the same as before, something
like this:
static const struct snd_kcontrol_new sun4i_codec_mixer_controls[] = {
SOC_DAPM_SINGLE("Left Mixer Left DAC Playback Switch", // !!!
SUN4I_CODEC_DAC_ACTL,
SUN4I_CODEC_DAC_ACTL_LDACLMIXS, 1, 0),
SOC_DAPM_SINGLE("Right Mixer Right DAC Playback Switch", // !!!
SUN4I_CODEC_DAC_ACTL,
SUN4I_CODEC_DAC_ACTL_RDACRMIXS, 1, 0),
SOC_DAPM_SINGLE("Right Mixer Left DAC Playback Switch",
SUN4I_CODEC_DAC_ACTL,
SUN4I_CODEC_DAC_ACTL_LDACRMIXS, 1, 0),
SOC_DAPM_DOUBLE("Line Playback Switch", SUN4I_CODEC_DAC_ACTL,
SUN4I_CODEC_DAC_ACTL_LLNS,
SUN4I_CODEC_DAC_ACTL_RLNS, 1, 0),
SOC_DAPM_DOUBLE("FM Playback Switch", SUN4I_CODEC_DAC_ACTL,
SUN4I_CODEC_DAC_ACTL_LFMS,
SUN4I_CODEC_DAC_ACTL_RFMS, 1, 0),
SOC_DAPM_DOUBLE("Mic1 Playback Switch", SUN4I_CODEC_DAC_ACTL,
SUN4I_CODEC_DAC_ACTL_MIC1LS,
SUN4I_CODEC_DAC_ACTL_MIC1RS, 1, 0),
SOC_DAPM_DOUBLE("Mic2 Playback Switch", SUN4I_CODEC_DAC_ACTL,
SUN4I_CODEC_DAC_ACTL_MIC2LS,
SUN4I_CODEC_DAC_ACTL_MIC2RS, 1, 0),
};
static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
...
SND_SOC_DAPM_MIXER("Left Mixer", SND_SOC_NOPM, 0, 0,
sun4i_codec_mixer_controls,
ARRAY_SIZE(sun4i_codec_mixer_controls)), // !!!
SND_SOC_DAPM_MIXER("Right Mixer", SND_SOC_NOPM, 0, 0,
sun4i_codec_mixer_controls,
ARRAY_SIZE(sun4i_codec_mixer_controls)), // !!!
};
Would this be a good way?
[toc] | [prev] | [next] | [standalone]
| From | Chen-Yu Tsai <wens@csie.org> |
|---|---|
| Date | 2017-06-28 10:40 +0200 |
| Subject | Re: [linux-sunxi] Re: [PATCH v13 05/14] ASoC: sun4i-codec: Merge sun4i_codec_left_mixer_controls and sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls. |
| Message-ID | <tXgFX-4t8-3@gated-at.bofh.it> |
| In reply to | #1676437 |
On Wed, Jun 28, 2017 at 4:25 PM, Danny Milosavljevic
<dannym@scratchpost.org> wrote:
> Hi,
>
>> > > -static const struct snd_kcontrol_new sun4i_codec_left_mixer_controls[] = {
>> > > - SOC_DAPM_SINGLE("Left DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
>> > > - SUN4I_CODEC_DAC_ACTL_LDACLMIXS, 1, 0),
>> > > -};
>> > > -
>> > > -static const struct snd_kcontrol_new sun4i_codec_right_mixer_controls[] = {
>> > > - SOC_DAPM_SINGLE("Right DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
>> > > +static const struct snd_kcontrol_new sun4i_codec_mixer_controls[] = {
>> > > + SOC_DAPM_DOUBLE("DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
>> > > + SUN4I_CODEC_DAC_ACTL_LDACLMIXS,
>> > > SUN4I_CODEC_DAC_ACTL_RDACRMIXS, 1, 0),
>> >
>> > This changes the mixer control names exposed to userspace. While I think
>> > consolidating mixer controls is nice, giving the user a less cluttered
>> > view, I'm not sure about the changing a possible userspace ABI.
>> >
>> > Maxime, ALSA and ASoC people, any comments about this?
>
>>Yeah, breaking userspace is definitely not ok.
>
> Okay. I can restore DAC Playback Switch as two different controls with the same
> names as before.
>
> Is the mixer itself (SND_SOC_DAPM_MIXER) - not talking about its controls -
> exposed to userspace?
Other than debugfs, no it is not.
> Because if not, I can still merge the left mixer and right mixer and just fix up
> the control names for DAC Playback Switch to be the same as before, something
> like this:
>
> static const struct snd_kcontrol_new sun4i_codec_mixer_controls[] = {
> SOC_DAPM_SINGLE("Left Mixer Left DAC Playback Switch", // !!!
> SUN4I_CODEC_DAC_ACTL,
> SUN4I_CODEC_DAC_ACTL_LDACLMIXS, 1, 0),
> SOC_DAPM_SINGLE("Right Mixer Right DAC Playback Switch", // !!!
> SUN4I_CODEC_DAC_ACTL,
> SUN4I_CODEC_DAC_ACTL_RDACRMIXS, 1, 0),
> SOC_DAPM_SINGLE("Right Mixer Left DAC Playback Switch",
> SUN4I_CODEC_DAC_ACTL,
> SUN4I_CODEC_DAC_ACTL_LDACRMIXS, 1, 0),
> SOC_DAPM_DOUBLE("Line Playback Switch", SUN4I_CODEC_DAC_ACTL,
> SUN4I_CODEC_DAC_ACTL_LLNS,
> SUN4I_CODEC_DAC_ACTL_RLNS, 1, 0),
> SOC_DAPM_DOUBLE("FM Playback Switch", SUN4I_CODEC_DAC_ACTL,
> SUN4I_CODEC_DAC_ACTL_LFMS,
> SUN4I_CODEC_DAC_ACTL_RFMS, 1, 0),
> SOC_DAPM_DOUBLE("Mic1 Playback Switch", SUN4I_CODEC_DAC_ACTL,
> SUN4I_CODEC_DAC_ACTL_MIC1LS,
> SUN4I_CODEC_DAC_ACTL_MIC1RS, 1, 0),
> SOC_DAPM_DOUBLE("Mic2 Playback Switch", SUN4I_CODEC_DAC_ACTL,
> SUN4I_CODEC_DAC_ACTL_MIC2LS,
> SUN4I_CODEC_DAC_ACTL_MIC2RS, 1, 0),
> };
>
> static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
> ...
> SND_SOC_DAPM_MIXER("Left Mixer", SND_SOC_NOPM, 0, 0,
> sun4i_codec_mixer_controls,
> ARRAY_SIZE(sun4i_codec_mixer_controls)), // !!!
> SND_SOC_DAPM_MIXER("Right Mixer", SND_SOC_NOPM, 0, 0,
> sun4i_codec_mixer_controls,
> ARRAY_SIZE(sun4i_codec_mixer_controls)), // !!!
> };
>
> Would this be a good way?
I think that would work. We would need to verify that the DAPM routing
graph for the existing single channel controls work as intended though.
ChenYu
[toc] | [prev] | [next] | [standalone]
| From | Mark Brown <broonie@kernel.org> |
|---|---|
| Date | 2017-06-28 20:20 +0200 |
| Subject | Re: [PATCH v13 05/14] ASoC: sun4i-codec: Merge sun4i_codec_left_mixer_controls and sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls. |
| Message-ID | <tXpJi-Rv-35@gated-at.bofh.it> |
| In reply to | #1674374 |
[Multipart message — attachments visible in raw view] — view raw
On Mon, Jun 26, 2017 at 09:48:59AM +0800, Chen-Yu Tsai wrote:
> On Sat, Jun 24, 2017 at 2:24 PM, Danny Milosavljevic
> > +static const struct snd_kcontrol_new sun4i_codec_mixer_controls[] = {
> > + SOC_DAPM_DOUBLE("DAC Playback Switch", SUN4I_CODEC_DAC_ACTL,
> > + SUN4I_CODEC_DAC_ACTL_LDACLMIXS,
> > SUN4I_CODEC_DAC_ACTL_RDACRMIXS, 1, 0),
> This changes the mixer control names exposed to userspace. While I think
> consolidating mixer controls is nice, giving the user a less cluttered
> view, I'm not sure about the changing a possible userspace ABI.
> Maxime, ALSA and ASoC people, any comments about this?
It's better not to change things but on the other hand it really depends
if there's any users who will notice - if nobody notices it doesn't
matter.
Can we *please* get some review of the rest of this series? It's
enormous, we're on version *13* and the first review on this version
here is on patch 5. If the first four patches are fine then please says
so, if they're not then let's have some review comments so things can
move towards merging them.
[toc] | [prev] | [next] | [standalone]
| From | Danny Milosavljevic <dannym@scratchpost.org> |
|---|---|
| Date | 2017-06-24 08:30 +0200 |
| Subject | [PATCH v13 08/14] ASoC: sun4i-codec: Add FM Left, FM Right, FM Playback Switch. |
| Message-ID | <tVMJY-4iL-31@gated-at.bofh.it> |
| In reply to | #1674024 |
Add FM Left, FM Right, FM Playback Switch for Allwinner A10 and Allwinner A20.
Signed-off-by: Danny Milosavljevic <dannym@scratchpost.org>
---
sound/soc/sunxi/sun4i-codec.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/sound/soc/sunxi/sun4i-codec.c b/sound/soc/sunxi/sun4i-codec.c
index 5b6f100..234ded2 100644
--- a/sound/soc/sunxi/sun4i-codec.c
+++ b/sound/soc/sunxi/sun4i-codec.c
@@ -702,6 +702,9 @@ static const struct snd_kcontrol_new sun4i_codec_mixer_controls[] = {
SOC_DAPM_SINGLE("Right Mixer Left DAC Playback Switch",
SUN4I_CODEC_DAC_ACTL,
SUN4I_CODEC_DAC_ACTL_LDACRMIXS, 1, 0),
+ SOC_DAPM_DOUBLE("FM Playback Switch", SUN4I_CODEC_DAC_ACTL,
+ SUN4I_CODEC_DAC_ACTL_LFMS,
+ SUN4I_CODEC_DAC_ACTL_RFMS, 1, 0),
SOC_DAPM_DOUBLE("Mic1 Playback Switch", SUN4I_CODEC_DAC_ACTL,
SUN4I_CODEC_DAC_ACTL_MIC1LS,
SUN4I_CODEC_DAC_ACTL_MIC1RS, 1, 0),
@@ -770,6 +773,8 @@ static const struct snd_soc_dapm_widget sun4i_codec_codec_dapm_widgets[] = {
SND_SOC_DAPM_SWITCH("Power Amplifier Mute", SND_SOC_NOPM, 0, 0,
&sun4i_codec_pa_mute),
+ SND_SOC_DAPM_INPUT("FM Right"),
+ SND_SOC_DAPM_INPUT("FM Left"),
SND_SOC_DAPM_INPUT("Mic1"),
SND_SOC_DAPM_INPUT("Mic2"),
@@ -790,12 +795,14 @@ static const struct snd_soc_dapm_route sun4i_codec_codec_dapm_routes[] = {
{ "Right Mixer", NULL, "Mixer Enable" },
{ "Right Mixer", "DAC Playback Switch", "Left DAC" },
{ "Right Mixer", "Right Mixer Left DAC Playback Switch", "Left DAC" },
+ { "Right Mixer", "FM Playback Switch", "FM Right" },
{ "Right Mixer", "Mic1 Playback Switch", "MIC1 Pre-Amplifier" },
{ "Right Mixer", "Mic2 Playback Switch", "MIC2 Pre-Amplifier" },
/* Left Mixer Routes */
{ "Left Mixer", NULL, "Mixer Enable" },
{ "Left Mixer", "DAC Playback Switch", "Left DAC" },
+ { "Left Mixer", "FM Playback Switch", "FM Left" },
{ "Left Mixer", "Mic1 Playback Switch", "MIC1 Pre-Amplifier" },
{ "Left Mixer", "Mic2 Playback Switch", "MIC2 Pre-Amplifier" },
--
2.1.4
[toc] | [prev] | [next] | [standalone]
| From | Chen-Yu Tsai <wens@csie.org> |
|---|---|
| Date | 2017-06-28 11:20 +0200 |
| Subject | Re: [PATCH v13 00/14] ASoC: sun4i-codec: Add Line-In, FM-In, Mic 2, Capture Source, Differential Line-In |
| Message-ID | <tXhiG-4WZ-9@gated-at.bofh.it> |
| In reply to | #1674024 |
Hi, On Sat, Jun 24, 2017 at 2:24 PM, Danny Milosavljevic <dannym@scratchpost.org> wrote: > This patchset adds some mixer controls to sun4i-codec for the Allwinner A10 > and the Allwinner A20. > > It also adds a mux for the capture source and the PGA for the MIC2 preamp. > > Where possible, it uses SOC_DAPM_DOUBLE in order to cut down on the number > of distinct controls in alsamixer. > > v13 changes compared to v12 are: > - Added my "Signed-off-by". > - Clarified some commit message text. > > v12 changes compared to v11 are: > - Split up patchset in another way. > - Renamed "Mic1 Capture Volume" to "Mic1 Boost Volume". > - Renamed "Mic2 Capture Volume" to "Mic2 Boost Volume". > - Renamed "Line Capture Volume" to "Line Boost Volume". > - Renamed "Differential Line Capture Switch" to "Differential Line Source". > > v11 changes compared to v10 are: > - Split up patchset. > - Fixed typo in Differential Line Capture Switch. > - Renamed "Non-Differential" value to "Stereo". > - Removed duplicate PA Volume mixer control. > > v10 changes compared to v9 are: > - Use SOC_DAPM_DOUBLE where possible and it makes sense in order to cut > down on the number of controls. > > v9 changes compared to v8 are: > - added Line Differential Capture Switch. > - split Capture Source into Left Capture Select, Right Capture Select. > - added Line Capture Volume. > - rename "sun4i_codec_widgets" to "sun4i_codec_controls" for > consistency with the struct field it's used in. > - rename "Line-In" to "Line". > - rename "Power Amplifier Playback Volume" to "Headphone Playback Volume". > > v8 changes compared to v7 are: > - fixed the routes for line and mic capturing. > > v7 changes compared to v6 are: > - preparation for different A20, A10 controls is now in an extra patch. > - all register definitions are now at the top. > - sun7i-specific things (A20-specific things) are now less > grouped-together. > - rename "Power Amplifier Volume" to "Power Amplifier Playback Volume". > > v6 changes compared to v5 are: > - Mic preamplifier special cases for A20 and A10 now are now not > icky: There are two different _widget arrays and the probe() function > now selects the right one to pass to snd_soc_register_codec() unmodified. > - sun7i-specific things (A20-specific things) are now grouped together. > > v5 changes compared to v4 are: > - Mic preamplifier controls have more common names now. > - Mic preamplifier scale has a 0 dB entry as well now, as documented in the > A20 user manual. > - Mic preamplifier has special cases for A20 and A10 now. > - Gain controls have "Gain" in the name now. > > v4 changes compared to v3 are: > - names of the input are not uppercase anymore. > - bit index constants are now named as in the A20 user manual v1.4. > - added Mic1-In, Mac2-In. > - added Mic1 and Mic2 Pre-Amplifiers. > > v3 changes compared to v2 are: > - added DAPM routes. > > v2 changes compared to v1 are: > - moved Line-In and FM-In playback switches to their respective > sun4i_codec_*_mixer_controls. > > v1 changes: > - added linein, fmin output volumes and switches. > > Danny Milosavljevic (14): > sun4i-codec: Add MIC2 Pre-Amplifier, Mic2 input, Mic2 routes. > sun4i-codec: Add Mic Playback Volume. > sun4i-codec: Add support for extra controls to struct > sun4i_codec_quirks and use them. > sun4i-codec: Add Mic1 Boost Volume, Mic2 Boost Volume. > sun4i-codec: Merge sun4i_codec_left_mixer_controls and > sun4i_codec_right_mixer_controls into sun4i_codec_mixer_controls. > sun4i-codec: Add Mic1 Playback Switch, Mic2 Playback Switch. > sun4i-codec: Add FM Playback Volume. > sun4i-codec: Add FM Left, FM Right, FM Playback Switch. > sun4i-codec: Add Line Playback Volume. > sun4i-codec: Add Line Boost Volume. > sun4i-codec: Add Line Right, Line Left, Line Playback Switch. > sun4i-codec: Add Differential Line Source. > sun4i-codec: Add Left Capture Select, Right Capture Select. > sun4i-codec: Add Capture Volume. I think there's still room for improvement regarding the structure of this patch series. What I would like to see is one patch for each new feature or rework of existing code. One major guideline is each patch should be independently verifiable. If you add controls related to Mic2 across multiple patches, and only the last one enables Mic2 playback, you can't really verify if your "Mic2 Boost Volume" or "Mic2 routes" were implemented correctly. Also I suggest doing rework first, then adding controls that are missing for widgets that already exist (such as Mic Playback Volume), and adding new stuff last. So something like the following, one patch per item: * Add missing Mic1 Playback Switch * Add missing Mic Playback Volume * Add extra controls to sun4i_codec_quirks * Add missing Mic1 Boost Volume * Merge/rework left/right mixer controls * Add Mic2 playback support (including pre-amp, input, routes, boost volume, playback switch) * Add FM playback support (widgets, routes, volume, switch) * Add Line playback support (widgets, routes, volume, switch, boost volume, differential) * Add Capture Source mux widget and controls * Add Capture Volume control For each new item added, you can immediately test if all the controls and routes work as intended by toggling them and listening to the output and looking at DAPM debugfs output. Thanks ChenYu > > sound/soc/sunxi/sun4i-codec.c | 233 +++++++++++++++++++++++++++++++++++++----- > 1 file changed, 210 insertions(+), 23 deletions(-) > > -- > 2.1.4 >
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web