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:
>>>
>>> 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.
>
> 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
--
Jaroslav Kysela <perex@xxxxxxxx>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
