Dne 15. 02. 21 v 13:02 Hans de Goede napsal(a):
Hi,
On 2/12/21 9:31 PM, Hans de Goede wrote:
On 2/11/21 12:13 PM, Jaroslav Kysela wrote:
<snip>
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.
I've spend some more time this weekend playing with this and I've also added mic MUTE LED support for the ASoC rt5672 codec driver now using this.
I will post a RFC patch series with the ASoC rt5672 codec driver LED support soon, as adding an extra use-case for this will hopefully help with reviewing this.
FWIW there were some challenges, but those were not related to the driver API this patch set adds. The driver API works well for ASoC codec drivers.
Regards,
Hans
p.s.
One open issue is the lockdep issue which I reported in my previous email.
Thank you for tests, Hans. I'm working on the lockdep issue - I'll send v2 of the LED patchset soon.
The actual diff (I'd like to do more tests):
diff --git a/sound/core/control.c b/sound/core/control.c index 4647b3cd41e8..c9f062fada0a 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -2047,6 +2047,7 @@ void snd_ctl_register_layer(struct snd_ctl_layer_ops *lops) down_write(&snd_ctl_layer_rwsem); lops->next = snd_ctl_layer; snd_ctl_layer = lops; + up_write(&snd_ctl_layer_rwsem); for (card_number = 0; card_number < SNDRV_CARDS; card_number++) { card = snd_card_ref(card_number); if (card) { @@ -2054,7 +2055,6 @@ void snd_ctl_register_layer(struct snd_ctl_layer_ops *lops) snd_card_unref(card); } } - up_write(&snd_ctl_layer_rwsem); } EXPORT_SYMBOL_GPL(snd_ctl_register_layer);
diff --git a/sound/core/control_led.c b/sound/core/control_led.c index 638808e397fe..093dce721024 100644 --- a/sound/core/control_led.c +++ b/sound/core/control_led.c @@ -75,7 +75,7 @@ static inline unsigned int group_to_access(unsigned int group) return (group + 1) << SNDRV_CTL_ELEM_ACCESS_LED_SHIFT; }
-struct snd_ctl_led *snd_ctl_led_get_by_access(unsigned int access) +static struct snd_ctl_led *snd_ctl_led_get_by_access(unsigned int access) { unsigned int group = access_to_group(access); if (group >= MAX_LED) @@ -116,15 +116,20 @@ static int snd_ctl_led_get(struct snd_ctl_led_ctl *lctl) return 0; }
-static int snd_ctl_led_get_lock(struct snd_ctl_led_ctl *lctl) +static int snd_ctl_led_get_lock(struct snd_card *ncard, struct snd_ctl_led_ctl *lctl) { struct snd_card *card = lctl->card; int route;
- down_read(&card->controls_rwsem); - route = snd_ctl_led_get(lctl); - up_read(&card->controls_rwsem); - return route; + /* the rwsem is already taken for the notification card */ + if (ncard != card) { + down_read(&card->controls_rwsem); + route = snd_ctl_led_get(lctl); + up_read(&card->controls_rwsem); + return route; + } else { + return snd_ctl_led_get(lctl); + } }
static void snd_ctl_led_set_state(struct snd_card *card, unsigned int access, @@ -149,7 +154,7 @@ static void snd_ctl_led_set_state(struct snd_card *card, unsigned int access, list_for_each_entry(lctl, &led->controls, list) { if (lctl->kctl == kctl && lctl->index_offset == ioff) found = true; - UPDATE_ROUTE(route, snd_ctl_led_get_lock(lctl)); + UPDATE_ROUTE(route, snd_ctl_led_get_lock(card, lctl)); } if (!found && kctl && card) { lctl = kzalloc(sizeof(*lctl), GFP_KERNEL); @@ -158,7 +163,7 @@ static void snd_ctl_led_set_state(struct snd_card *card, unsigned int access, lctl->kctl = kctl; lctl->index_offset = ioff; list_add(&lctl->list, &led->controls); - UPDATE_ROUTE(route, snd_ctl_led_get_lock(lctl)); + UPDATE_ROUTE(route, snd_ctl_led_get_lock(card, lctl)); } } mutex_unlock(&snd_ctl_led_mutex); @@ -246,10 +251,11 @@ static void snd_ctl_led_register(struct snd_card *card) mutex_lock(&snd_ctl_led_mutex); snd_ctl_led_card_valid[card->number] = true; mutex_unlock(&snd_ctl_led_mutex); - /* the register callback is already called with held rwsem for controls */ + down_read(&card->controls_rwsem); list_for_each_entry(kctl, &card->controls, list) for (ioff = 0; ioff < kctl->count; ioff++) snd_ctl_led_notify(card, SNDRV_CTL_EVENT_MASK_VALUE, kctl, ioff); + up_read(&card->controls_rwsem); snd_ctl_led_refresh(); }
@@ -262,17 +268,6 @@ static void snd_ctl_led_disconnect(struct snd_card *card) snd_ctl_led_refresh(); }
-/** - * snd_ctl_led_hello - kernel module reference helper - * - * Call this helper in the module init function when the control LED - * code should be activated for the given driver. - */ -void snd_ctl_led_hello(void) -{ -} -EXPORT_SYMBOL(snd_ctl_led_hello); - /* * sysfs */
Jaroslav