[PATCH alsa-lib 0/5] Add generic exception mechanism for non-standard control-names

[PATCH alsa-lib 0/5] Add generic exception mechanism for non-standard control-names

[Hans de Goede]

Hi All,

This series seems to have fallen through the cracks,
so here is a resend of it.

Regards,

Hans


Hans de Goede (5):
  mixer: simple - Add generic exception mechanism for non-standard
    control-names
  mixer: simple - Move handling of 3D Control - Depth controls to the
    exceptions list
  mixer: simple - Add exceptions for non " Volume" suffixed capture
    vol-ctls used in ASoC realtek codec drivers
  mixer: simple - Add exceptions for some capture-vol-ctls which have a
    " Volume" suffix
  mixer: simple - Add exceptions for some Playback Switches with a "
    Channel Switch" suffix

 src/mixer/simple_none.c | 74 +++++++++++++++++++++++++----------------
 1 file changed, 46 insertions(+), 28 deletions(-)

-- 
2.31.1

[PATCH alsa-lib 1/5] mixer: simple - Add generic exception mechanism for non-standard control-names

[Hans de Goede]

Add a generic exception mechanism for mixer-control-names which
don't use the standard suffixes from the suffixes table.

And move the existing exceptions which consist of a simple !strcmp()
check over to this new mechanism.

This also fixes the "Capture Volume" and "Capture Switch" exceptions
no longer working after commit 86b9c67774bc ("mixer: simple - Unify
simple_none: base_len() exception handling") because they were moved
to after the suffix checking, so they would be treated as
CTL_GLOBAL_VOLUME resp. CTL_GLOBAL_SWITCH based on their suffix
before the exception check has a chance to check for a match.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 src/mixer/simple_none.c | 48 +++++++++++++++++++++++------------------
 1 file changed, 27 insertions(+), 21 deletions(-)

diff --git a/src/mixer/simple_none.c b/src/mixer/simple_none.c
index 71d88486..30d5aa8b 100644
--- a/src/mixer/simple_none.c
+++ b/src/mixer/simple_none.c
@@ -905,14 +905,41 @@ static const struct suf {
 	{" Volume", CTL_GLOBAL_VOLUME},
 	{NULL, 0}
 };
+
+static const struct excep {
+	const char *name;
+	int base_len;
+	selem_ctl_type_t type;
+} exceptions[] = {
+	/* Special case - handle "Input Source" as a capture route.
+	 * Note that it's *NO* capture source.  A capture source is split over
+	 * sub-elements, and multiple capture-sources will result in an error.
+	 * That's why some drivers use "Input Source" as a workaround.
+	 * Hence, this is a workaround for a workaround to get the things
+	 * straight back again.  Sigh.
+	 */
+	{"Input Source", 12, CTL_CAPTURE_ROUTE},
+	/* Avoid these Capture Volume/Switch controls getting seen as GLOBAL VOL/SW */
+	{"Capture Volume", 7, CTL_CAPTURE_VOLUME},
+	{"Capture Switch", 7, CTL_CAPTURE_SWITCH},
+	{NULL,}
+};
 #endif
 
 /* Return base length */
 static int base_len(const char *name, selem_ctl_type_t *type)
 {
+	const struct excep *e;
 	const struct suf *p;
 	size_t nlen = strlen(name);
 
+	for (e = exceptions; e->name; e++) {
+		if (!strcmp(name, e->name)) {
+			*type = e->type;
+			return e->base_len;
+		}
+	}
+
 	for (p = suffixes; p->suffix; p++) {
 		size_t slen = strlen(p->suffix);
 		size_t l;
@@ -926,27 +953,6 @@ static int base_len(const char *name, selem_ctl_type_t *type)
 		}
 	}
 
-	/* exception: "Capture Volume" and "Capture Switch" */
-	if (!strcmp(name, "Capture Volume")) {
-		*type = CTL_CAPTURE_VOLUME;
-		return strlen("Capture");
-	}
-	if (!strcmp(name, "Capture Switch")) {
-		*type = CTL_CAPTURE_SWITCH;
-		return strlen("Capture");
-	}
-
-	/* Special case - handle "Input Source" as a capture route.
-	 * Note that it's *NO* capture source.  A capture source is split over
-	 * sub-elements, and multiple capture-sources will result in an error.
-	 * That's why some drivers use "Input Source" as a workaround.
-	 * Hence, this is a workaround for a workaround to get the things
-	 * straight back again.  Sigh.
-	 */
-	if (!strcmp(name, "Input Source")) {
-		*type = CTL_CAPTURE_ROUTE;
-		return strlen(name);
-	}
 	if (strstr(name, "3D Control")) {
 		if (strstr(name, "Depth")) {
 			*type = CTL_PLAYBACK_VOLUME;
-- 
2.31.1

[PATCH alsa-lib 2/5] mixer: simple - Move handling of 3D Control - Depth controls to the exceptions list

[Hans de Goede]

Remove the custom handling of 3D Control - Depth control-names, replacing
this with adding the 3 full names which are used for such controls to the
exceptions list:

"3D Control - Depth"
"3D Control Sigmatel - Depth"
"3D Control Sigmatel - Rear Depth"

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 src/mixer/simple_none.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/src/mixer/simple_none.c b/src/mixer/simple_none.c
index 30d5aa8b..b29554cb 100644
--- a/src/mixer/simple_none.c
+++ b/src/mixer/simple_none.c
@@ -922,6 +922,10 @@ static const struct excep {
 	/* Avoid these Capture Volume/Switch controls getting seen as GLOBAL VOL/SW */
 	{"Capture Volume", 7, CTL_CAPTURE_VOLUME},
 	{"Capture Switch", 7, CTL_CAPTURE_SWITCH},
+	/* Playback Volume/Switch controls without a " Playback ..." suffix */
+	{"3D Control - Depth", 18, CTL_PLAYBACK_VOLUME},
+	{"3D Control Sigmatel - Depth", 27, CTL_PLAYBACK_VOLUME},
+	{"3D Control Sigmatel - Rear Depth", 32, CTL_PLAYBACK_VOLUME},
 	{NULL,}
 };
 #endif
@@ -953,13 +957,6 @@ static int base_len(const char *name, selem_ctl_type_t *type)
 		}
 	}
 
-	if (strstr(name, "3D Control")) {
-		if (strstr(name, "Depth")) {
-			*type = CTL_PLAYBACK_VOLUME;
-			return strlen(name);
-		}
-	}
-
 	*type = CTL_SINGLE;
 	return strlen(name);
 }
-- 
2.31.1

[PATCH alsa-lib 3/5] mixer: simple - Add exceptions for non " Volume" suffixed capture vol-ctls used in ASoC realtek codec drivers

[Hans de Goede]

The following ASoC codec drivers:

sound/soc/codecs/rt5640.c
sound/soc/codecs/rt5645.c
sound/soc/codecs/rt5651.c
sound/soc/codecs/rt5677.c

Use capture-volume-control names like: "IN1 Boost", note the missing
" Volume" suffix. This causes the mixer code to not identify these as
volume-controls, which causes some of the dB related sm_elem_ops to
return -EINVAL.

This in turn causes alsamixer to not show dB info and causes UCM profile
HW volume control support in pulseaudio to not work properly due to the
lacking dB scale info.

This cannot be fixed on the kernel side because the non " Volume" suffixed
names are used in UCM profiles currently shipping in alsa-ucm-conf.

Add these to the exceptions table, so that these correctly get identified
as CTL_CAPTURE_VOLUME controls.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 src/mixer/simple_none.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/mixer/simple_none.c b/src/mixer/simple_none.c
index b29554cb..8f964959 100644
--- a/src/mixer/simple_none.c
+++ b/src/mixer/simple_none.c
@@ -926,6 +926,11 @@ static const struct excep {
 	{"3D Control - Depth", 18, CTL_PLAYBACK_VOLUME},
 	{"3D Control Sigmatel - Depth", 27, CTL_PLAYBACK_VOLUME},
 	{"3D Control Sigmatel - Rear Depth", 32, CTL_PLAYBACK_VOLUME},
+	/* Capture Volume/Switch controls without a " Capture ..." suffix */
+	{"ADC Boost Gain", 14, CTL_CAPTURE_VOLUME},
+	{"IN1 Boost", 9, CTL_CAPTURE_VOLUME},
+	{"IN2 Boost", 9, CTL_CAPTURE_VOLUME},
+	{"IN3 Boost", 9, CTL_CAPTURE_VOLUME},
 	{NULL,}
 };
 #endif
-- 
2.31.1

[PATCH alsa-lib 4/5] mixer: simple - Add exceptions for some capture-vol-ctls which have a " Volume" suffix

[Hans de Goede]

The following ASoC codec drivers:

sound/soc/codecs/rt5659.c
sound/soc/codecs/rt5660.c
sound/soc/codecs/rt5665.c
sound/soc/codecs/rt5668.c
sound/soc/codecs/rt5670.c
sound/soc/codecs/rt5682.c

Use the following troublesome capture-volume-control names:
"IN1 Boost Volume"
"IN2 Boost Volume"
"IN3 Boost Volume"
"STO1 ADC Boost Gain Volume"
"STO2 ADC Boost Gain Volume"
"Mono ADC Boost Gain Volume"

And sound/soc/codecs/es8316.c uses "ADC PGA Gain Volume".

Note how these are suffixed with just " Volume" instead of
"Capture Volume". Add these to the exceptions table,
so that the type correctly gets set to CTL_CAPTURE_VOLUME instead
of CTL_GLOBAL_VOLUME.

This correctly makes snd_mixer_selem_has_capture_volume() return true for
these (and makes snd_mixer_selem_has_common_volume() return false).

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 src/mixer/simple_none.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/src/mixer/simple_none.c b/src/mixer/simple_none.c
index 8f964959..de81966f 100644
--- a/src/mixer/simple_none.c
+++ b/src/mixer/simple_none.c
@@ -920,8 +920,15 @@ static const struct excep {
 	 */
 	{"Input Source", 12, CTL_CAPTURE_ROUTE},
 	/* Avoid these Capture Volume/Switch controls getting seen as GLOBAL VOL/SW */
+	{"ADC PGA Gain Volume", 12, CTL_CAPTURE_VOLUME},
 	{"Capture Volume", 7, CTL_CAPTURE_VOLUME},
 	{"Capture Switch", 7, CTL_CAPTURE_SWITCH},
+	{"IN1 Boost Volume", 9, CTL_CAPTURE_VOLUME},
+	{"IN2 Boost Volume", 9, CTL_CAPTURE_VOLUME},
+	{"IN3 Boost Volume", 9, CTL_CAPTURE_VOLUME},
+	{"Mono ADC Boost Gain Volume", 19, CTL_CAPTURE_VOLUME},
+	{"STO1 ADC Boost Gain Volume", 19, CTL_CAPTURE_VOLUME},
+	{"STO2 ADC Boost Gain Volume", 19, CTL_CAPTURE_VOLUME},
 	/* Playback Volume/Switch controls without a " Playback ..." suffix */
 	{"3D Control - Depth", 18, CTL_PLAYBACK_VOLUME},
 	{"3D Control Sigmatel - Depth", 27, CTL_PLAYBACK_VOLUME},
-- 
2.31.1

[PATCH alsa-lib 5/5] mixer: simple - Add exceptions for some Playback Switches with a " Channel Switch" suffix

[Hans de Goede]

The following ASoC codec drivers:
sound/soc/codecs/rt5640.c
sound/soc/codecs/rt5645.c

Use the following troublesome playback-switch-control names:

"Headphone Channel Switch"
"HP Channel Switch"
"Speaker Channel Switch"

There are 2 problems with these names:

1. They are the mute controls for the matching:
"Headphone Playback Volume"
"HP Playback Volume"
"Speaker Playback Volume"
controls, to be properly paired, which is necessary for HW volume-control
support, the simple mixer code needs to recognize that the base-name for
these is e.g. "Headphone" not "Headphone Channel".

2. They are playback-switches, yet they get recognized as global-switches.

Add these to the exceptions table so that they get the proper basename
and type set.

Note we can NOT fix this by adding " Channel Switch" as a suffix to the
suffixes table, because the line-out output on these codecs has the
following controls:

"OUT Playback Switch"
"OUT Channel Switch"
"OUT Playback Volume"

Where the 2 switches describe mutes in 2 different places in the graph.

So if we were to add a " Channel Switch" suffix map to CTL_PLAYBACK_SWITCH
then we would get 2 CTL_PLAYBACK_SWITCH controls for the "OUT" mixer
element, which is not allowed.

Signed-off-by: Hans de Goede <hdegoede@redhat.com>
---
 src/mixer/simple_none.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/mixer/simple_none.c b/src/mixer/simple_none.c
index de81966f..d5025f68 100644
--- a/src/mixer/simple_none.c
+++ b/src/mixer/simple_none.c
@@ -933,6 +933,9 @@ static const struct excep {
 	{"3D Control - Depth", 18, CTL_PLAYBACK_VOLUME},
 	{"3D Control Sigmatel - Depth", 27, CTL_PLAYBACK_VOLUME},
 	{"3D Control Sigmatel - Rear Depth", 32, CTL_PLAYBACK_VOLUME},
+	{"Headphone Channel Switch", 9, CTL_PLAYBACK_SWITCH},
+	{"HP Channel Switch", 2, CTL_PLAYBACK_SWITCH},
+	{"Speaker Channel Switch", 7, CTL_PLAYBACK_SWITCH},
 	/* Capture Volume/Switch controls without a " Capture ..." suffix */
 	{"ADC Boost Gain", 14, CTL_CAPTURE_VOLUME},
 	{"IN1 Boost", 9, CTL_CAPTURE_VOLUME},
-- 
2.31.1

Re: [PATCH alsa-lib 0/5] Add generic exception mechanism for non-standard control-names

[Jaroslav Kysela]

Dne 03. 05. 21 v 22:52 Hans de Goede napsal(a):
> Hi All,
> 
> This series seems to have fallen through the cracks,
> so here is a resend of it.
> 
> Regards,

Thank you, Hans. The problem with this implementation is that it's really card
specific. Also, ASoC codec drivers have usually ID names based on registers so
the mapping for the user is problematic anyway (the functionality is different
from the name or not related to the name). I'm actually evaluating another
solution which is more flexible:

1) add control remap plugin to allow the control ID remapping in the
alsa-lib's control API, so we can mangle those identifiers there (already
implemented)

2) add local and global alsa-lib configurations per UCM card specified in the
UCM configuration files; the configurations may be for both control and PCM
devices (restrict or set specific parameters)

I will notify you when I finish my tests.

				Jaroslav

> 
> Hans
> 
> 
> Hans de Goede (5):
>   mixer: simple - Add generic exception mechanism for non-standard
>     control-names
>   mixer: simple - Move handling of 3D Control - Depth controls to the
>     exceptions list
>   mixer: simple - Add exceptions for non " Volume" suffixed capture
>     vol-ctls used in ASoC realtek codec drivers
>   mixer: simple - Add exceptions for some capture-vol-ctls which have a
>     " Volume" suffix
>   mixer: simple - Add exceptions for some Playback Switches with a "
>     Channel Switch" suffix
> 
>  src/mixer/simple_none.c | 74 +++++++++++++++++++++++++----------------
>  1 file changed, 46 insertions(+), 28 deletions(-)
> 


-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [PATCH alsa-lib 0/5] Add generic exception mechanism for non-standard control-names

[Hans de Goede]

Hi Jaroslav,

On 5/4/21 10:53 AM, Jaroslav Kysela wrote:
> Dne 03. 05. 21 v 22:52 Hans de Goede napsal(a):
>> Hi All,
>>
>> This series seems to have fallen through the cracks,
>> so here is a resend of it.
>>
>> Regards,
> 
> Thank you, Hans. The problem with this implementation is that it's really card
> specific. Also, ASoC codec drivers have usually ID names based on registers so
> the mapping for the user is problematic anyway (the functionality is different
> from the name or not related to the name). I'm actually evaluating another
> solution which is more flexible:
> 
> 1) add control remap plugin to allow the control ID remapping in the
> alsa-lib's control API, so we can mangle those identifiers there (already
> implemented)
> 
> 2) add local and global alsa-lib configurations per UCM card specified in the
> UCM configuration files; the configurations may be for both control and PCM
> devices (restrict or set specific parameters)

Ok, thank you for working on this.

> I will notify you when I finish my tests.

Yes, please let me know when you've something ready to test, then I'll take
a look at adding the necessary bits for the bycr-rt5640 and cht-bsw-rt567
UCM profiles, as some control renaming is necessary to make sure that
the hw-volume control on these devices also correctly controls the
hw mute controls (which in turn are necessary for both full muting and
for mute LED control).

Regards,

Hans


>> Hans de Goede (5):
>>   mixer: simple - Add generic exception mechanism for non-standard
>>     control-names
>>   mixer: simple - Move handling of 3D Control - Depth controls to the
>>     exceptions list
>>   mixer: simple - Add exceptions for non " Volume" suffixed capture
>>     vol-ctls used in ASoC realtek codec drivers
>>   mixer: simple - Add exceptions for some capture-vol-ctls which have a
>>     " Volume" suffix
>>   mixer: simple - Add exceptions for some Playback Switches with a "
>>     Channel Switch" suffix
>>
>>  src/mixer/simple_none.c | 74 +++++++++++++++++++++++++----------------
>>  1 file changed, 46 insertions(+), 28 deletions(-)
>>
> 
> 

Re: [PATCH alsa-lib 0/5] Add generic exception mechanism for non-standard control-names

[Jaroslav Kysela]

Dne 04. 05. 21 v 17:47 Hans de Goede napsal(a):
> Hi Jaroslav,
> 
> On 5/4/21 10:53 AM, Jaroslav Kysela wrote:
>> Dne 03. 05. 21 v 22:52 Hans de Goede napsal(a):
>>> Hi All,
>>>
>>> This series seems to have fallen through the cracks,
>>> so here is a resend of it.
>>>
>>> Regards,
>>
>> Thank you, Hans. The problem with this implementation is that it's really card
>> specific. Also, ASoC codec drivers have usually ID names based on registers so
>> the mapping for the user is problematic anyway (the functionality is different
>> from the name or not related to the name). I'm actually evaluating another
>> solution which is more flexible:
>>
>> 1) add control remap plugin to allow the control ID remapping in the
>> alsa-lib's control API, so we can mangle those identifiers there (already
>> implemented)
>>
>> 2) add local and global alsa-lib configurations per UCM card specified in the
>> UCM configuration files; the configurations may be for both control and PCM
>> devices (restrict or set specific parameters)
> 
> Ok, thank you for working on this.
> 
>> I will notify you when I finish my tests.
> 
> Yes, please let me know when you've something ready to test, then I'll take
> a look at adding the necessary bits for the bycr-rt5640 and cht-bsw-rt567
> UCM profiles, as some control renaming is necessary to make sure that
> the hw-volume control on these devices also correctly controls the
> hw mute controls (which in turn are necessary for both full muting and
> for mute LED control).

It seems that things started to work. I pushed everything to the repos
(alsa-lib/alsa-utils/alsa-ucm-conf) and picked bits from your configs. If you
can give a look and a test, it would be nice. The changes for the specific
codecs are quite straight like:

https://github.com/alsa-project/alsa-ucm-conf/commit/2072ab794b69cdf4f070db5467387d08a65c4309

The global alsa-lib's configuration does the redirects to the hw specific
configs (if found) per card. UCM can store this "per card" configuration to
/var/lib/alsa/card<NUMBER>.conf.d tree, which allows us to define the hw
specific configuration. Both control and PCM devices can be (re)configured.

UCM was extended to allow inline the alsa-lib's configuration which can be
private to UCM or saved to a global config file (/var/lib/alsa tree for example).

By default, I made the private alsa-lib's configuration for all UCM
applications, so the users cannot break UCM with their configuration changes.

						Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [PATCH alsa-lib 0/5] Add generic exception mechanism for non-standard control-names

[Hans de Goede]

Hi Jaroslav,

On 5/18/21 6:16 PM, Jaroslav Kysela wrote:
> Dne 04. 05. 21 v 17:47 Hans de Goede napsal(a):
>> Hi Jaroslav,
>>
>> On 5/4/21 10:53 AM, Jaroslav Kysela wrote:
>>> Dne 03. 05. 21 v 22:52 Hans de Goede napsal(a):
>>>> Hi All,
>>>>
>>>> This series seems to have fallen through the cracks,
>>>> so here is a resend of it.
>>>>
>>>> Regards,
>>>
>>> Thank you, Hans. The problem with this implementation is that it's really card
>>> specific. Also, ASoC codec drivers have usually ID names based on registers so
>>> the mapping for the user is problematic anyway (the functionality is different
>>> from the name or not related to the name). I'm actually evaluating another
>>> solution which is more flexible:
>>>
>>> 1) add control remap plugin to allow the control ID remapping in the
>>> alsa-lib's control API, so we can mangle those identifiers there (already
>>> implemented)
>>>
>>> 2) add local and global alsa-lib configurations per UCM card specified in the
>>> UCM configuration files; the configurations may be for both control and PCM
>>> devices (restrict or set specific parameters)
>>
>> Ok, thank you for working on this.
>>
>>> I will notify you when I finish my tests.
>>
>> Yes, please let me know when you've something ready to test, then I'll take
>> a look at adding the necessary bits for the bycr-rt5640 and cht-bsw-rt567
>> UCM profiles, as some control renaming is necessary to make sure that
>> the hw-volume control on these devices also correctly controls the
>> hw mute controls (which in turn are necessary for both full muting and
>> for mute LED control).
> 
> It seems that things started to work. I pushed everything to the repos
> (alsa-lib/alsa-utils/alsa-ucm-conf) and picked bits from your configs. If you
> can give a look and a test, it would be nice. The changes for the specific
> codecs are quite straight like:
> 
> https://github.com/alsa-project/alsa-ucm-conf/commit/2072ab794b69cdf4f070db5467387d08a65c4309
> 
> The global alsa-lib's configuration does the redirects to the hw specific
> configs (if found) per card. UCM can store this "per card" configuration to
> /var/lib/alsa/card<NUMBER>.conf.d tree, which allows us to define the hw
> specific configuration. Both control and PCM devices can be (re)configured.
> 
> UCM was extended to allow inline the alsa-lib's configuration which can be
> private to UCM or saved to a global config file (/var/lib/alsa tree for example).
> 
> By default, I made the private alsa-lib's configuration for all UCM
> applications, so the users cannot break UCM with their configuration changes.

Thank you for your work on this.

I've been testing this on a HP x2 Bay Trail + rt5640 laptop, and I've
found 2 issues:

1. After renaming there are now 2 "Speaker" and "Headphones" switches:

"Speaker Playback Volume" stays    "Speaker Playback Volume"
"Speaker Channel Switch"  becomes  "Speaker Playback Switch"
"Speaker Switch"          stays    "Speaker Switch"

And then alsamixer only shows one of the 2 "Speaker [Playback] Switches"

This can be worked around by changing the renames to e.g. :

                "name='HP Playback Volume'" "name='Headphones Playback Volume'"
                "name='HP Channel Switch'" "name='Headphones Playback Switch'"
                "name='Speaker Playback Volume'" "name='Speakers Playback Volume'"
                "name='Speaker Channel Switch'" "name='Speakers Playback Switch'"

Or to:

                # Rename the 'Headphone Switch' DAPM PIN switch to avoid it getting
                # grouped with 'Headphone Playback Volume'
                "name='Headphone Switch'" "name='Headphone Output Switch'"
                "name='HP Playback Volume'" "name='Headphone Playback Volume'"
                "name='HP Channel Switch'" "name='Headphone Playback Switch'"
                # Idem for the 'Speaker Switch'
                "name='Speaker Switch'" "name='Speaker Output Switch'"
                "name='Speaker Channel Switch'" "name='Speaker Playback Switch'"

So this is not really an issue.

2. PlaybackMixerElem statements don't take the renames into account, this means
that muting the speakers or the headphones output the UCM (pipewire/pulse) level
does not mute the 'Speaker Channel Switch' / 'HP Channel Switch' control, meaning
that we are not muting things at the hw level, which in turn is causing the speaker
mute LED on the HP X2 to not be turned on when muting.

I guess the fix here would be to make the renames apply to PlaybackMixerElem ?

Downside is that that would be a syntax change for the UCM conf language I guess
(e.g. this would require update the PlaybackMixerElem from HP to Headphone in the
rt5640 case)

If you can steer me in the right direction for fixing this I can take a shot at
fixing this. Or alternatively I would be happy to test any patches for this from
you.

Regards,

Hans

Re: [PATCH alsa-lib 0/5] Add generic exception mechanism for non-standard control-names

[Jaroslav Kysela]

On 23. 06. 21 20:59, Hans de Goede wrote:
> Hi Jaroslav,
> 
> On 5/18/21 6:16 PM, Jaroslav Kysela wrote:
>> Dne 04. 05. 21 v 17:47 Hans de Goede napsal(a):
>>> Hi Jaroslav,
>>>
>>> On 5/4/21 10:53 AM, Jaroslav Kysela wrote:
>>>> Dne 03. 05. 21 v 22:52 Hans de Goede napsal(a):
>>>>> Hi All,
>>>>>
>>>>> This series seems to have fallen through the cracks,
>>>>> so here is a resend of it.
>>>>>
>>>>> Regards,
>>>>
>>>> Thank you, Hans. The problem with this implementation is that it's really card
>>>> specific. Also, ASoC codec drivers have usually ID names based on registers so
>>>> the mapping for the user is problematic anyway (the functionality is different
>>>> from the name or not related to the name). I'm actually evaluating another
>>>> solution which is more flexible:
>>>>
>>>> 1) add control remap plugin to allow the control ID remapping in the
>>>> alsa-lib's control API, so we can mangle those identifiers there (already
>>>> implemented)
>>>>
>>>> 2) add local and global alsa-lib configurations per UCM card specified in the
>>>> UCM configuration files; the configurations may be for both control and PCM
>>>> devices (restrict or set specific parameters)
>>>
>>> Ok, thank you for working on this.
>>>
>>>> I will notify you when I finish my tests.
>>>
>>> Yes, please let me know when you've something ready to test, then I'll take
>>> a look at adding the necessary bits for the bycr-rt5640 and cht-bsw-rt567
>>> UCM profiles, as some control renaming is necessary to make sure that
>>> the hw-volume control on these devices also correctly controls the
>>> hw mute controls (which in turn are necessary for both full muting and
>>> for mute LED control).
>>
>> It seems that things started to work. I pushed everything to the repos
>> (alsa-lib/alsa-utils/alsa-ucm-conf) and picked bits from your configs. If you
>> can give a look and a test, it would be nice. The changes for the specific
>> codecs are quite straight like:
>>
>> https://github.com/alsa-project/alsa-ucm-conf/commit/2072ab794b69cdf4f070db5467387d08a65c4309
>>
>> The global alsa-lib's configuration does the redirects to the hw specific
>> configs (if found) per card. UCM can store this "per card" configuration to
>> /var/lib/alsa/card<NUMBER>.conf.d tree, which allows us to define the hw
>> specific configuration. Both control and PCM devices can be (re)configured.
>>
>> UCM was extended to allow inline the alsa-lib's configuration which can be
>> private to UCM or saved to a global config file (/var/lib/alsa tree for example).
>>
>> By default, I made the private alsa-lib's configuration for all UCM
>> applications, so the users cannot break UCM with their configuration changes.
> 
> Thank you for your work on this.
> 
> I've been testing this on a HP x2 Bay Trail + rt5640 laptop, and I've
> found 2 issues:
> 
> 1. After renaming there are now 2 "Speaker" and "Headphones" switches:
> 
> "Speaker Playback Volume" stays    "Speaker Playback Volume"
> "Speaker Channel Switch"  becomes  "Speaker Playback Switch"
> "Speaker Switch"          stays    "Speaker Switch"
> 
> And then alsamixer only shows one of the 2 "Speaker [Playback] Switches"
> 
> This can be worked around by changing the renames to e.g. :
> 
>                 "name='HP Playback Volume'" "name='Headphones Playback Volume'"
>                 "name='HP Channel Switch'" "name='Headphones Playback Switch'"
>                 "name='Speaker Playback Volume'" "name='Speakers Playback Volume'"
>                 "name='Speaker Channel Switch'" "name='Speakers Playback Switch'"
> 
> Or to:
> 
>                 # Rename the 'Headphone Switch' DAPM PIN switch to avoid it getting
>                 # grouped with 'Headphone Playback Volume'
>                 "name='Headphone Switch'" "name='Headphone Output Switch'"
>                 "name='HP Playback Volume'" "name='Headphone Playback Volume'"
>                 "name='HP Channel Switch'" "name='Headphone Playback Switch'"
>                 # Idem for the 'Speaker Switch'
>                 "name='Speaker Switch'" "name='Speaker Output Switch'"
>                 "name='Speaker Channel Switch'" "name='Speaker Playback Switch'"

This variant looks better in my eyes.

> So this is not really an issue.
> 
> 2. PlaybackMixerElem statements don't take the renames into account, this means
> that muting the speakers or the headphones output the UCM (pipewire/pulse) level
> does not mute the 'Speaker Channel Switch' / 'HP Channel Switch' control, meaning
> that we are not muting things at the hw level, which in turn is causing the speaker
> mute LED on the HP X2 to not be turned on when muting.
> 
> I guess the fix here would be to make the renames apply to PlaybackMixerElem ?

Yes, this change is required. I forgot to update this part.

> Downside is that that would be a syntax change for the UCM conf language I guess
> (e.g. this would require update the PlaybackMixerElem from HP to Headphone in the
> rt5640 case)

It's just a configuration value change, so it's fine.

> If you can steer me in the right direction for fixing this I can take a shot at
> fixing this. Or alternatively I would be happy to test any patches for this from
> you.

I would appreciate patches or a pull request on github, if you like. Thank you
for your tests.

					Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.

Re: [PATCH alsa-lib 0/5] Add generic exception mechanism for non-standard control-names

[Hans de Goede]

Hi,

On 6/23/21 9:27 PM, Jaroslav Kysela wrote:
> On 23. 06. 21 20:59, Hans de Goede wrote:
>> Hi Jaroslav,
>>
>> On 5/18/21 6:16 PM, Jaroslav Kysela wrote:
>>> Dne 04. 05. 21 v 17:47 Hans de Goede napsal(a):
>>>> Hi Jaroslav,
>>>>
>>>> On 5/4/21 10:53 AM, Jaroslav Kysela wrote:
>>>>> Dne 03. 05. 21 v 22:52 Hans de Goede napsal(a):
>>>>>> Hi All,
>>>>>>
>>>>>> This series seems to have fallen through the cracks,
>>>>>> so here is a resend of it.
>>>>>>
>>>>>> Regards,
>>>>>
>>>>> Thank you, Hans. The problem with this implementation is that it's really card
>>>>> specific. Also, ASoC codec drivers have usually ID names based on registers so
>>>>> the mapping for the user is problematic anyway (the functionality is different
>>>>> from the name or not related to the name). I'm actually evaluating another
>>>>> solution which is more flexible:
>>>>>
>>>>> 1) add control remap plugin to allow the control ID remapping in the
>>>>> alsa-lib's control API, so we can mangle those identifiers there (already
>>>>> implemented)
>>>>>
>>>>> 2) add local and global alsa-lib configurations per UCM card specified in the
>>>>> UCM configuration files; the configurations may be for both control and PCM
>>>>> devices (restrict or set specific parameters)
>>>>
>>>> Ok, thank you for working on this.
>>>>
>>>>> I will notify you when I finish my tests.
>>>>
>>>> Yes, please let me know when you've something ready to test, then I'll take
>>>> a look at adding the necessary bits for the bycr-rt5640 and cht-bsw-rt567
>>>> UCM profiles, as some control renaming is necessary to make sure that
>>>> the hw-volume control on these devices also correctly controls the
>>>> hw mute controls (which in turn are necessary for both full muting and
>>>> for mute LED control).
>>>
>>> It seems that things started to work. I pushed everything to the repos
>>> (alsa-lib/alsa-utils/alsa-ucm-conf) and picked bits from your configs. If you
>>> can give a look and a test, it would be nice. The changes for the specific
>>> codecs are quite straight like:
>>>
>>> https://github.com/alsa-project/alsa-ucm-conf/commit/2072ab794b69cdf4f070db5467387d08a65c4309
>>>
>>> The global alsa-lib's configuration does the redirects to the hw specific
>>> configs (if found) per card. UCM can store this "per card" configuration to
>>> /var/lib/alsa/card<NUMBER>.conf.d tree, which allows us to define the hw
>>> specific configuration. Both control and PCM devices can be (re)configured.
>>>
>>> UCM was extended to allow inline the alsa-lib's configuration which can be
>>> private to UCM or saved to a global config file (/var/lib/alsa tree for example).
>>>
>>> By default, I made the private alsa-lib's configuration for all UCM
>>> applications, so the users cannot break UCM with their configuration changes.
>>
>> Thank you for your work on this.
>>
>> I've been testing this on a HP x2 Bay Trail + rt5640 laptop, and I've
>> found 2 issues:
>>
>> 1. After renaming there are now 2 "Speaker" and "Headphones" switches:
>>
>> "Speaker Playback Volume" stays    "Speaker Playback Volume"
>> "Speaker Channel Switch"  becomes  "Speaker Playback Switch"
>> "Speaker Switch"          stays    "Speaker Switch"
>>
>> And then alsamixer only shows one of the 2 "Speaker [Playback] Switches"
>>
>> This can be worked around by changing the renames to e.g. :
>>
>>                 "name='HP Playback Volume'" "name='Headphones Playback Volume'"
>>                 "name='HP Channel Switch'" "name='Headphones Playback Switch'"
>>                 "name='Speaker Playback Volume'" "name='Speakers Playback Volume'"
>>                 "name='Speaker Channel Switch'" "name='Speakers Playback Switch'"
>>
>> Or to:
>>
>>                 # Rename the 'Headphone Switch' DAPM PIN switch to avoid it getting
>>                 # grouped with 'Headphone Playback Volume'
>>                 "name='Headphone Switch'" "name='Headphone Output Switch'"
>>                 "name='HP Playback Volume'" "name='Headphone Playback Volume'"
>>                 "name='HP Channel Switch'" "name='Headphone Playback Switch'"
>>                 # Idem for the 'Speaker Switch'
>>                 "name='Speaker Switch'" "name='Speaker Output Switch'"
>>                 "name='Speaker Channel Switch'" "name='Speaker Playback Switch'"
> 
> This variant looks better in my eyes.
> 
>> So this is not really an issue.
>>
>> 2. PlaybackMixerElem statements don't take the renames into account, this means
>> that muting the speakers or the headphones output the UCM (pipewire/pulse) level
>> does not mute the 'Speaker Channel Switch' / 'HP Channel Switch' control, meaning
>> that we are not muting things at the hw level, which in turn is causing the speaker
>> mute LED on the HP X2 to not be turned on when muting.
>>
>> I guess the fix here would be to make the renames apply to PlaybackMixerElem ?
> 
> Yes, this change is required. I forgot to update this part.

Ok, so I've taken a (quick) look at this but I'm afraid that I don't fully
grasp how the control remapping is working. Can you give me some hint how
I can make the renames apply to PlaybackMixerElem or a rough patch for me
to test (and fixup if necessary, I mostly need an idea where to start).

Regards,

Hans

Re: [PATCH alsa-lib 0/5] Add generic exception mechanism for non-standard control-names

[Hans de Goede]

Hi,

On 5/4/21 10:53 AM, Jaroslav Kysela wrote:
> Dne 03. 05. 21 v 22:52 Hans de Goede napsal(a):
>> Hi All,
>>
>> This series seems to have fallen through the cracks,
>> so here is a resend of it.
>>
>> Regards,
> 
> Thank you, Hans. The problem with this implementation is that it's really card
> specific. Also, ASoC codec drivers have usually ID names based on registers so
> the mapping for the user is problematic anyway (the functionality is different
> from the name or not related to the name). I'm actually evaluating another
> solution which is more flexible:
> 
> 1) add control remap plugin to allow the control ID remapping in the
> alsa-lib's control API, so we can mangle those identifiers there (already
> implemented)
> 
> 2) add local and global alsa-lib configurations per UCM card specified in the
> UCM configuration files; the configurations may be for both control and PCM
> devices (restrict or set specific parameters)

p.s.

Note that the first patch in this series also fixes a regression,
quoting from the commit message:

This also fixes the "Capture Volume" and "Capture Switch" exceptions
no longer working after commit 86b9c67774bc ("mixer: simple - Unify
simple_none: base_len() exception handling") because they were moved
to after the suffix checking, so they would be treated as
CTL_GLOBAL_VOLUME resp. CTL_GLOBAL_SWITCH based on their suffix
before the exception check has a chance to check for a match.

In the first patch of this series fixing this regression is a
side-effect of the changes made there.

Since you don't want to take this series, I'll write a new patch
fixing just the regression.

Regards,

Hans