All of lore.kernel.org
 help / color / mirror / Atom feed
From: Hans de Goede <hdegoede@redhat.com>
To: Jaroslav Kysela <perex@perex.cz>, alsa-devel@alsa-project.org
Subject: Re: [PATCH alsa-lib 0/5] Add generic exception mechanism for non-standard control-names
Date: Wed, 23 Jun 2021 20:59:15 +0200	[thread overview]
Message-ID: <cc686b48-b27d-74bf-4fb4-21fdd23a9c89@redhat.com> (raw)
In-Reply-To: <95924d6e-e2b7-0b9b-b62c-27e57fe88a80@perex.cz>

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



  reply	other threads:[~2021-06-23 19:00 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-03 20:52 [PATCH alsa-lib 0/5] Add generic exception mechanism for non-standard control-names Hans de Goede
2021-05-03 20:52 ` [PATCH alsa-lib 1/5] mixer: simple - " Hans de Goede
2021-05-03 20:52 ` [PATCH alsa-lib 2/5] mixer: simple - Move handling of 3D Control - Depth controls to the exceptions list Hans de Goede
2021-05-03 20:52 ` [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
2021-05-03 20:52 ` [PATCH alsa-lib 4/5] mixer: simple - Add exceptions for some capture-vol-ctls which have a " Volume" suffix Hans de Goede
2021-05-03 20:52 ` [PATCH alsa-lib 5/5] mixer: simple - Add exceptions for some Playback Switches with a " Channel Switch" suffix Hans de Goede
2021-05-04  8:53 ` [PATCH alsa-lib 0/5] Add generic exception mechanism for non-standard control-names Jaroslav Kysela
2021-05-04 15:47   ` Hans de Goede
2021-05-18 16:16     ` Jaroslav Kysela
2021-06-23 18:59       ` Hans de Goede [this message]
2021-06-23 19:27         ` Jaroslav Kysela
2021-06-25 13:05           ` Hans de Goede
2021-05-04 15:51   ` Hans de Goede

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=cc686b48-b27d-74bf-4fb4-21fdd23a9c89@redhat.com \
    --to=hdegoede@redhat.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=perex@perex.cz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.