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:
- call snd_ctl_led_request() when control LED layer should be automatically activated / it calls module_request("snd-ctl-led") on demand /
- 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...