Hi,
On 2/11/21 12:13 PM, Jaroslav Kysela wrote:
> This patchset tries to resolve the diversity in the audio LED
> control among the ALSA drivers.
>
> A new control layer registration is introduced which allows
> to run additional operations on top of the elementary ALSA
> sound controls.
>
> A new control access group (three bits in the access flags)
> was introduced to carry the LED group information for
> the sound controls. The low-level sound drivers can just
> mark those controls using this access group. This information
> is exported to the user space and eventually the user space
> can create sound controls which can belong to a LED group.
>
> The actual state ('route') evaluation is really easy
> (the minimal value check for all channels / controls / cards).
> If there's more complicated logic for a given hardware,
> the card driver may eventually export a new read-only
> sound control for the LED group and do the logic itself.
>
> The new LED trigger control code is completely separated
> and possibly optional (there's no symbol dependency).
> The full code separation allows eventually to move this
> LED trigger control to the user space in future.
> Actually it replaces the already present functionality
> in the kernel space (HDA drivers) and allows a quick adoption
> for the recent hardware (SoundWire ASoC codecs).
>
> # lsmod | grep snd_ctl_led
> snd_ctl_led 16384 0
>
> The sound driver implementation is really easy:
>
> 1) call snd_ctl_led_request() when control LED layer should be
> automatically activated
> / it calls module_request("snd-ctl-led") on demand /
> 2) mark all related kcontrols with
> SNDRV_CTL_ELEM_ACCESS_SPK_LED or
> SNDRV_CTL_ELEM_ACCESS_MIC_LED
So I've been running some tests with this,combined with writing
UCM profiles for hw volume control, for some Intel Bay- and
CherryTrail devices using Intel's Low Power Engine (LPE) for audio,
which uses the ASoC framework.
My work / experiments for this are progressing a bit slower then I
would like, but that is not the fault of this patch-set, but rather
an issue with hw-volume control mapping, see below for details.
Leaving the ASoC implementation details aside, this patch-set
works quite nicely to get the speaker mute-LED to work.
There is one issue, I'm running my kernels with lockdep and
this patchset triggers a lockdep warning:
[ 24.487200] input: sof-bytcht rt5672 Headset as /devices/platform/80860F28:00/cht-bsw-rt5672/sound/card1/input19
[ 24.532006] ======================================================
[ 24.532010] WARNING: possible circular locking dependency detected
[ 24.532015] 5.11.0-rc7+ #248 Tainted: G E
[ 24.532019] ------------------------------------------------------
[ 24.532022] systemd-udevd/394 is trying to acquire lock:
[ 24.532027] ffff922888ead720 (&card->controls_rwsem){++++}-{3:3}, at: snd_ctl_led_hello+0x65a/0x9f0 [snd_ctl_led]
[ 24.532048]
but task is already holding lock:
[ 24.532051] ffffffffc0b0eb88 (snd_ctl_led_mutex){+.+.}-{3:3}, at: snd_ctl_led_hello+0x453/0x9f0 [snd_ctl_led]
[ 24.532066]
which lock already depends on the new lock.
[ 24.532069]
the existing dependency chain (in reverse order) is:
[ 24.532072]
-> #2 (snd_ctl_led_mutex){+.+.}-{3:3}:
[ 24.532083] __mutex_lock+0xb8/0x7f0
[ 24.532094] snd_ctl_led_hello+0x8fd/0x9f0 [snd_ctl_led]
[ 24.532100] snd_ctl_register_layer+0x48/0x360 [snd]
[ 24.532112] 0xffffffffc078f149
[ 24.532119] do_one_initcall+0x6e/0x2e0
[ 24.532127] do_init_module+0x5c/0x260
[ 24.532135] load_module+0x2570/0x27e0
[ 24.532142] __do_sys_init_module+0x130/0x190
[ 24.532148] do_syscall_64+0x33/0x40
[ 24.532154] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 24.532161]
-> #1 (snd_ctl_layer_rwsem){++++}-{3:3}:
[ 24.532172] down_read+0x40/0x50
[ 24.532177] snd_ctl_notify_one+0x8d/0x150 [snd]
[ 24.532188] snd_ctl_find_id+0x24e/0x350 [snd]
[ 24.532197] snd_ctl_find_id+0x2f3/0x350 [snd]
[ 24.532207] 0xffffffffc0786bab
[ 24.532212] platform_probe+0x3f/0x90
[ 24.532219] really_probe+0xf2/0x440
[ 24.532225] driver_probe_device+0xe1/0x150
[ 24.532231] device_driver_attach+0xa8/0xb0
[ 24.532237] __driver_attach+0x8c/0x150
[ 24.532243] bus_for_each_dev+0x78/0xa0
[ 24.532249] bus_add_driver+0x12e/0x1f0
[ 24.532254] driver_register+0x8f/0xe0
[ 24.532260] do_one_initcall+0x6e/0x2e0
[ 24.532266] do_init_module+0x5c/0x260
[ 24.532272] load_module+0x2570/0x27e0
[ 24.532278] __do_sys_init_module+0x130/0x190
[ 24.532285] do_syscall_64+0x33/0x40
[ 24.532290] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 24.532296]
-> #0 (&card->controls_rwsem){++++}-{3:3}:
[ 24.532307] __lock_acquire+0x113d/0x1e10
[ 24.532314] lock_acquire+0xe4/0x390
[ 24.532320] down_read+0x40/0x50
[ 24.532325] snd_ctl_led_hello+0x65a/0x9f0 [snd_ctl_led]
[ 24.532331] snd_ctl_led_hello+0x959/0x9f0 [snd_ctl_led]
[ 24.532337] snd_ctl_register_layer+0x183/0x360 [snd]
[ 24.532347] snd_device_register_all+0x4c/0x60 [snd]
[ 24.532357] snd_card_register+0x74/0x1d0 [snd]
[ 24.532366] snd_soc_set_dmi_name+0xb2a/0xc30 [snd_soc_core]
[ 24.532393] devm_snd_soc_register_card+0x43/0x90 [snd_soc_core]
[ 24.532412] 0xffffffffc0cf3579
[ 24.532419] platform_probe+0x3f/0x90
[ 24.532424] really_probe+0xf2/0x440
[ 24.532430] driver_probe_device+0xe1/0x150
[ 24.532436] device_driver_attach+0xa8/0xb0
[ 24.532442] __driver_attach+0x8c/0x150
[ 24.532447] bus_for_each_dev+0x78/0xa0
[ 24.532453] bus_add_driver+0x12e/0x1f0
[ 24.532459] driver_register+0x8f/0xe0
[ 24.532465] do_one_initcall+0x6e/0x2e0
[ 24.532471] do_init_module+0x5c/0x260
[ 24.532477] load_module+0x2570/0x27e0
[ 24.532483] __do_sys_init_module+0x130/0x190
[ 24.532489] do_syscall_64+0x33/0x40
[ 24.532494] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 24.532501]
other info that might help us debug this:
[ 24.532504] Chain exists of:
&card->controls_rwsem --> snd_ctl_layer_rwsem --> snd_ctl_led_mutex
[ 24.532517] Possible unsafe locking scenario:
[ 24.532520] CPU0 CPU1
[ 24.532523] ---- ----
[ 24.532526] lock(snd_ctl_led_mutex);
[ 24.532532] lock(snd_ctl_layer_rwsem);
[ 24.532538] lock(snd_ctl_led_mutex);
[ 24.532544] lock(&card->controls_rwsem);
[ 24.532550]
*** DEADLOCK ***
[ 24.532553] 5 locks held by systemd-udevd/394:
[ 24.532558] #0: ffff922883ec2988 (&dev->mutex){....}-{3:3}, at: device_driver_attach+0x3b/0xb0
[ 24.532574] #1: ffffffffc088e1c8 (client_mutex){+.+.}-{3:3}, at: snd_soc_set_dmi_name+0x2f8/0xc30 [snd_soc_core]
[ 24.532604] #2: ffffffffc0cf71f0 (&card->mutex){+.+.}-{3:3}, at: snd_soc_set_dmi_name+0x30e/0xc30 [snd_soc_core]
[ 24.532632] #3: ffffffffc07562b0 (snd_ctl_layer_rwsem){++++}-{3:3}, at: snd_ctl_register_layer+0x16b/0x360 [snd]
[ 24.532652] #4: ffffffffc0b0eb88 (snd_ctl_led_mutex){+.+.}-{3:3}, at: snd_ctl_led_hello+0x453/0x9f0 [snd_ctl_led]
[ 24.532668]
stack backtrace:
[ 24.532672] CPU: 2 PID: 394 Comm: systemd-udevd Tainted: G E 5.11.0-rc7+ #248
[ 24.532679] Hardware name: LENOVO 20C1000VMH/20C1000VMH, BIOS GUET86WW (1.86) 10/21/2019
[ 24.532683] Call Trace:
[ 24.532691] dump_stack+0x8b/0xb0
[ 24.532702] check_noncircular+0xfb/0x110
[ 24.532713] __lock_acquire+0x113d/0x1e10
[ 24.532722] ? lock_acquire+0xe4/0x390
[ 24.532730] lock_acquire+0xe4/0x390
[ 24.532737] ? snd_ctl_led_hello+0x65a/0x9f0 [snd_ctl_led]
[ 24.532747] down_read+0x40/0x50
[ 24.532754] ? snd_ctl_led_hello+0x65a/0x9f0 [snd_ctl_led]
[ 24.532761] snd_ctl_led_hello+0x65a/0x9f0 [snd_ctl_led]
[ 24.532771] snd_ctl_led_hello+0x959/0x9f0 [snd_ctl_led]
[ 24.532779] snd_ctl_register_layer+0x183/0x360 [snd]
[ 24.532791] snd_device_register_all+0x4c/0x60 [snd]
[ 24.532803] snd_card_register+0x74/0x1d0 [snd]
[ 24.532816] snd_soc_set_dmi_name+0xb2a/0xc30 [snd_soc_core]
[ 24.532838] devm_snd_soc_register_card+0x43/0x90 [snd_soc_core]
[ 24.532860] 0xffffffffc0cf3579
[ 24.532869] platform_probe+0x3f/0x90
[ 24.532876] really_probe+0xf2/0x440
[ 24.532885] driver_probe_device+0xe1/0x150
[ 24.532893] device_driver_attach+0xa8/0xb0
[ 24.532901] __driver_attach+0x8c/0x150
[ 24.532908] ? device_driver_attach+0xb0/0xb0
[ 24.532914] ? device_driver_attach+0xb0/0xb0
[ 24.532922] bus_for_each_dev+0x78/0xa0
[ 24.532930] bus_add_driver+0x12e/0x1f0
[ 24.532937] driver_register+0x8f/0xe0
[ 24.532944] ? 0xffffffffc0cfa000
[ 24.532950] do_one_initcall+0x6e/0x2e0
[ 24.532961] do_init_module+0x5c/0x260
[ 24.532970] load_module+0x2570/0x27e0
[ 24.532988] ? __do_sys_init_module+0x130/0x190
[ 24.532995] __do_sys_init_module+0x130/0x190
[ 24.533007] do_syscall_64+0x33/0x40
[ 24.533014] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 24.533022] RIP: 0033:0x7f5d8ed4640e
[ 24.533030] Code: 48 8b 0d 65 1a 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 af 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 32 1a 0c 00 f7 d8 64 89 01 48
[ 24.533037] RSP: 002b:00007ffc37771de8 EFLAGS: 00000246 ORIG_RAX: 00000000000000af
[ 24.533044] RAX: ffffffffffffffda RBX: 000056459db8f3f0 RCX: 00007f5d8ed4640e
[ 24.533049] RDX: 00007f5d8eea632c RSI: 00000000000064d0 RDI: 000056459dd220b0
[ 24.533054] RBP: 000056459dd220b0 R08: 000056459dced5f0 R09: 00007ffc3776e160
[ 24.533058] R10: 00005640f99694ad R11: 0000000000000246 R12: 00007f5d8eea632c
[ 24.533062] R13: 000056459dbe48c0 R14: 0000000000000007 R15: 000056459dd16a30
Regards,
Hans
p.s.
The promised details of the issues which I'm hitting in implementing
this on Intel devs using the ASoC framework:
E.g. on the rt5672 the speaker amplifier has no volume control.
So I need to use the DAC1 digital volume control, which has no mute bits.
I have this working now, but due to there not being enough steps in the
hw-vol-control, it reaches 0 when the GNOME UI is displaying that the
sound is soft, but not off/muted. Yes the mute-led goes on because
the control reaches it lowest value. So I need to fake a mute control
in the codec driver for the LED to work reliable it seems...
