pcm512x driver: Mixer control naming issue

pcm512x driver: Mixer control naming issue

[Howard Mitchell]

Hi,

As you may be aware the Raspbian distribution for the RaspberryPi has 
recently been upgraded to kernel version 3.18. It was previously at 
version 3.12. This has had the unfortunate side effect that users of the 
PCM5122 DAC have found that the output volume has been significantly 
reduced. The issue is related to this commit 
<https://git.kernel.org/cgit/linux/kernel/git/stable/linux-stable.git/commit/sound/soc/codecs/pcm512x.c?id=5be2fc20b101b5138c4f54a584dc11790ef16598> 
which adds support for a couple of analogue gain controls. One control 
(Playback Volume) sets a gain of either 0db or -6dB and is intended to 
set the full scale output at 2Vrms or 1Vrms respectively. The other 
(Playback Boost Volume) provides a boost of +0.8dB. The hardware reset 
value of both of these gain controls is 0dB, however, in the Raspbian 
distribution 'Playback Volume' is being defaulted to -6dB.

After some investigation I figured out that this was caused by the alsa 
restore mechanism. If '/var/lib/alsa/asound.state' doesn't exist or 
there is no saved state for a particular control then the defaults in 
'/usr/share/alsa/init/default' are used. In this file the default for a 
control named 'Playback Volume' is -20dB which is forcing the control to 
it's minimum value of -6dB. This seems like incorrect behaviour for 
something that is essentially an implementation detail, i.e. a choice 
that should be made by the board designer not the end user. Therefore 
I'd like to see what you think of the following suggestions (which I'd 
be more than happy to implement and submit patches for):

  * Either change the names of these controls to something that's not
    affected by the alsa restore mechanism,
  * or remove them as mixer controls, but make them accessible some
    other way - e.g. device tree.

Howard Mitchell

Re: pcm512x driver: Mixer control naming issue

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 893 bytes --]

On Fri, Mar 13, 2015 at 12:41:05PM +0000, Howard Mitchell wrote:

> Boost Volume) provides a boost of +0.8dB. The hardware reset value of both
> of these gain controls is 0dB, however, in the Raspbian distribution
> 'Playback Volume' is being defaulted to -6dB.

>  * Either change the names of these controls to something that's not
>    affected by the alsa restore mechanism,

You say "these controls" but it seems like only "Playback Volume" has
a problem?  My first suggestion would be to define "Analog" or
"Analogue" as a prefix in ControlNames and then use that, that would
avoid confusing applications while still fittig in with the naming
convention.

>  * or remove them as mixer controls, but make them accessible some
>    other way - e.g. device tree.

It may not be policy that someone wants to tweak at runtime on your
board but it almost certainly will be on someone's board.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: pcm512x driver: Mixer control naming issue

[Howard Mitchell]

On 13/03/15 18:27, Mark Brown wrote:
> On Fri, Mar 13, 2015 at 12:41:05PM +0000, Howard Mitchell wrote:
>
>> Boost Volume) provides a boost of +0.8dB. The hardware reset value of both
>> of these gain controls is 0dB, however, in the Raspbian distribution
>> 'Playback Volume' is being defaulted to -6dB.
>>   * Either change the names of these controls to something that's not
>>     affected by the alsa restore mechanism,
> You say "these controls" but it seems like only "Playback Volume" has
> a problem?  My first suggestion would be to define "Analog" or
> "Analogue" as a prefix in ControlNames and then use that, that would
> avoid confusing applications while still fittig in with the naming
> convention.
Yes you are correct that it's only "Playback Volume" that is causing a 
problem. However, I included "Playback Boost Volume" as it also provides 
a selection of analogue gain so I think it should be treated similarly 
for consistency.

Defining a new prefix sounds like a reasonable idea in principle but 
calling it "Analog(ue)" seems like it's saying this is a proper analogue 
volume control whereas these particular controls merely provide very 
limited selections of gain. I could imagine that at some point in the 
future a default for "Analogue Playback Volume" may well be added to 
"alsactl restore" and then we end up back in the same situation. How 
about defining a prefix that is documented as saying that a default 
should not be used if there's nothing in asound.state for that control?

>>   * or remove them as mixer controls, but make them accessible some
>>     other way - e.g. device tree.
> It may not be policy that someone wants to tweak at runtime on your
> board but it almost certainly will be on someone's board.
>
Fair enough, if that's the general alsa ethos I'm happy to go along with it.

Re: pcm512x driver: Mixer control naming issue

[Takashi Iwai]

At Sun, 15 Mar 2015 22:28:01 +0000,
Howard Mitchell wrote:
> 
> On 13/03/15 18:27, Mark Brown wrote:
> > On Fri, Mar 13, 2015 at 12:41:05PM +0000, Howard Mitchell wrote:
> >
> >> Boost Volume) provides a boost of +0.8dB. The hardware reset value of both
> >> of these gain controls is 0dB, however, in the Raspbian distribution
> >> 'Playback Volume' is being defaulted to -6dB.
> >>   * Either change the names of these controls to something that's not
> >>     affected by the alsa restore mechanism,
> > You say "these controls" but it seems like only "Playback Volume" has
> > a problem?  My first suggestion would be to define "Analog" or
> > "Analogue" as a prefix in ControlNames and then use that, that would
> > avoid confusing applications while still fittig in with the naming
> > convention.
> Yes you are correct that it's only "Playback Volume" that is causing a 
> problem. However, I included "Playback Boost Volume" as it also provides 
> a selection of analogue gain so I think it should be treated similarly 
> for consistency.

Did you read Documentation/sound/alsa/ControlNames.txt?
It shows why "Playback Volume" is taken in that way while "Playback
Boost Volume" not.  Namely, the latter implies a volume control named
"Playback Boost" which is applied generically -- i.e. the direction is
unknown or applied to both playback and capture.


Takashi

Re: pcm512x driver: Mixer control naming issue

[Howard Mitchell]

On 16/03/15 07:47, Takashi Iwai wrote:
> At Sun, 15 Mar 2015 22:28:01 +0000,
> Howard Mitchell wrote:
>> On 13/03/15 18:27, Mark Brown wrote:
>>> On Fri, Mar 13, 2015 at 12:41:05PM +0000, Howard Mitchell wrote:
>>>
>>>> Boost Volume) provides a boost of +0.8dB. The hardware reset value of both
>>>> of these gain controls is 0dB, however, in the Raspbian distribution
>>>> 'Playback Volume' is being defaulted to -6dB.
>>>>    * Either change the names of these controls to something that's not
>>>>      affected by the alsa restore mechanism,
>>> You say "these controls" but it seems like only "Playback Volume" has
>>> a problem?  My first suggestion would be to define "Analog" or
>>> "Analogue" as a prefix in ControlNames and then use that, that would
>>> avoid confusing applications while still fittig in with the naming
>>> convention.
>> Yes you are correct that it's only "Playback Volume" that is causing a
>> problem. However, I included "Playback Boost Volume" as it also provides
>> a selection of analogue gain so I think it should be treated similarly
>> for consistency.
> Did you read Documentation/sound/alsa/ControlNames.txt?
> It shows why "Playback Volume" is taken in that way while "Playback
> Boost Volume" not.  Namely, the latter implies a volume control named
> "Playback Boost" which is applied generically -- i.e. the direction is
> unknown or applied to both playback and capture.
Sorry, what I meant by "treated similarly for consistency" is that 
whatever solution is decided upon to fix the issue with "Playback 
Volume" should also be applied to "Playback Boost Volume".

Your comment about the direction being unknown for "Playback Boost 
Volume" is interesting though as when running alsamixer I find that 
"Playback Volume" is also listed as a capture control. Neither makes 
sense of course as this device does not have a capture path.

- Howard

Re: pcm512x driver: Mixer control naming issue

[Takashi Iwai]

At Mon, 16 Mar 2015 09:47:07 +0000,
Howard Mitchell wrote:
> 
> On 16/03/15 07:47, Takashi Iwai wrote:
> > At Sun, 15 Mar 2015 22:28:01 +0000,
> > Howard Mitchell wrote:
> >> On 13/03/15 18:27, Mark Brown wrote:
> >>> On Fri, Mar 13, 2015 at 12:41:05PM +0000, Howard Mitchell wrote:
> >>>
> >>>> Boost Volume) provides a boost of +0.8dB. The hardware reset value of both
> >>>> of these gain controls is 0dB, however, in the Raspbian distribution
> >>>> 'Playback Volume' is being defaulted to -6dB.
> >>>>    * Either change the names of these controls to something that's not
> >>>>      affected by the alsa restore mechanism,
> >>> You say "these controls" but it seems like only "Playback Volume" has
> >>> a problem?  My first suggestion would be to define "Analog" or
> >>> "Analogue" as a prefix in ControlNames and then use that, that would
> >>> avoid confusing applications while still fittig in with the naming
> >>> convention.
> >> Yes you are correct that it's only "Playback Volume" that is causing a
> >> problem. However, I included "Playback Boost Volume" as it also provides
> >> a selection of analogue gain so I think it should be treated similarly
> >> for consistency.
> > Did you read Documentation/sound/alsa/ControlNames.txt?
> > It shows why "Playback Volume" is taken in that way while "Playback
> > Boost Volume" not.  Namely, the latter implies a volume control named
> > "Playback Boost" which is applied generically -- i.e. the direction is
> > unknown or applied to both playback and capture.
> Sorry, what I meant by "treated similarly for consistency" is that 
> whatever solution is decided upon to fix the issue with "Playback 
> Volume" should also be applied to "Playback Boost Volume".
> 
> Your comment about the direction being unknown for "Playback Boost 
> Volume" is interesting though as when running alsamixer I find that 
> "Playback Volume" is also listed as a capture control. Neither makes 
> sense of course as this device does not have a capture path.

Congrats, you hit a bug in alsa-lib simple mixer layer.
Feel free to submit a fix patch :)


Takashi

Re: pcm512x driver: Mixer control naming issue

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 2005 bytes --]

On Sun, Mar 15, 2015 at 10:28:01PM +0000, Howard Mitchell wrote:
> On 13/03/15 18:27, Mark Brown wrote:

> >You say "these controls" but it seems like only "Playback Volume" has
> >a problem?  My first suggestion would be to define "Analog" or
> >"Analogue" as a prefix in ControlNames and then use that, that would
> >avoid confusing applications while still fittig in with the naming
> >convention.

> Yes you are correct that it's only "Playback Volume" that is causing a
> problem. However, I included "Playback Boost Volume" as it also provides a
> selection of analogue gain so I think it should be treated similarly for
> consistency.

Oh, I see.

> Defining a new prefix sounds like a reasonable idea in principle but calling
> it "Analog(ue)" seems like it's saying this is a proper analogue volume
> control whereas these particular controls merely provide very limited
> selections of gain. I could imagine that at some point in the future a

Userspace can determine how much control there is from the TLV
information.  Ideally a smart userspace will manage to combine all the
different controls it finds to make the best use it can of them - if it
can use analogue volume control for some of the control that's generally
better since it maximizes the resolution of the digital parts.

> default for "Analogue Playback Volume" may well be added to "alsactl
> restore" and then we end up back in the same situation. How about defining a
> prefix that is documented as saying that a default should not be used if
> there's nothing in asound.state for that control?

Really the problem here is using asound.state - it's far too dumb a
means of controlling random cards, and attempting to change it is most
likely going to break something else.  Any attempt to introduce a
default for analogue control will inevitably result in problems on cards
that currently work fine with digital/default control unless those are
changed which would then cause problems for things without the analogue
control.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: pcm512x driver: Mixer control naming issue

[Takashi Iwai]

At Mon, 16 Mar 2015 11:38:45 +0000,
Mark Brown wrote:
> 
> > default for "Analogue Playback Volume" may well be added to "alsactl
> > restore" and then we end up back in the same situation. How about defining a
> > prefix that is documented as saying that a default should not be used if
> > there's nothing in asound.state for that control?
> 
> Really the problem here is using asound.state - it's far too dumb a
> means of controlling random cards, and attempting to change it is most
> likely going to break something else.

alsactl tries to initialize some common controls, and the question is
how common is common.

> Any attempt to introduce a
> default for analogue control will inevitably result in problems on cards
> that currently work fine with digital/default control unless those are
> changed which would then cause problems for things without the analogue
> control.

It's why I raised an item about standard control names every year in
the audio meeting :)


Takashi

Re: pcm512x driver: Mixer control naming issue

[Howard Mitchell]

On 16/03/15 11:38, Mark Brown wrote:
> On Sun, Mar 15, 2015 at 10:28:01PM +0000, Howard Mitchell wrote:
>
>> Defining a new prefix sounds like a reasonable idea in principle but calling
>> it "Analog(ue)" seems like it's saying this is a proper analogue volume
>> control whereas these particular controls merely provide very limited
>> selections of gain. I could imagine that at some point in the future a
> Userspace can determine how much control there is from the TLV
> information.  Ideally a smart userspace will manage to combine all the
> different controls it finds to make the best use it can of them - if it
> can use analogue volume control for some of the control that's generally
> better since it maximizes the resolution of the digital parts.

Makes sense.
>> default for "Analogue Playback Volume" may well be added to "alsactl
>> restore" and then we end up back in the same situation. How about defining a
>> prefix that is documented as saying that a default should not be used if
>> there's nothing in asound.state for that control?
> Really the problem here is using asound.state - it's far too dumb a
> means of controlling random cards, and attempting to change it is most
> likely going to break something else.  Any attempt to introduce a
> default for analogue control will inevitably result in problems on cards
> that currently work fine with digital/default control unless those are
> changed which would then cause problems for things without the analogue
> control.
So it sounds like adding an analogue prefix should be fairly safe in
which case are you happy for me to go ahead and prepare a patch?

- Howard

Re: pcm512x driver: Mixer control naming issue

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 727 bytes --]

On Mon, Mar 16, 2015 at 05:37:28PM +0000, Howard Mitchell wrote:
> On 16/03/15 11:38, Mark Brown wrote:

> >Really the problem here is using asound.state - it's far too dumb a
> >means of controlling random cards, and attempting to change it is most
> >likely going to break something else.  Any attempt to introduce a
> >default for analogue control will inevitably result in problems on cards
> >that currently work fine with digital/default control unless those are
> >changed which would then cause problems for things without the analogue
> >control.

> So it sounds like adding an analogue prefix should be fairly safe in
> which case are you happy for me to go ahead and prepare a patch?

I'm certainly happy with that.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]