On Fri, 21 Sep 2018 14:53:20 +0200,
Jussi Laako wrote:
>
> On 21.09.2018 11:43, Takashi Iwai wrote:
> > On Fri, 21 Sep 2018 10:35:49 +0200,
> > Jussi Laako wrote:
> >>
> >>> The caching is currently enabled for all elements, but changing it
> >>> should be trivial. The patch below adds is_volatile flag to the
> >>> element, and you can set it to true in the quirk somehow for uncached
> >>> controls.
> >>
> >> This patch looks good and what I need! When trying to figure out where
> >> it is safe to set the volatile flag I thought that I could set it
> >> transparently in add_single_ctl_with_resume() in mixer_quirks.c based
> >> on the SNDRV_CTL_ELEM_ACCESS_VOLATILE flag in provided
> >> snd_kcontrol_new access-member. However, add_single_ctl_with_resume()
> >> is allocating just size of usb_mixer_elem_list unlike
> >> snd_create_std_mono_ctl_offset() which in turn is allocating full
> >> usb_mixer_elem_info size. However, the mixer code seems to be assuming
> >> that the item is always usb_mixer_elem_info instead of just list
> >> header item. Is this allocation behavior correct and is the item
> >> turned into usb_mixer_elem_info somewhere, or is this some kind of
> >> bug? So can I safely turn the allocation in
> >> add_single_ctl_with_resume() into zero initialized usb_mixer_elem_info
> >> instead and set the flag there while keeping correct behavior, or am I
> >> missing something?
> >
> > We can check SNDRV_CTL_ELEM_ACCESS_VOLATILE instead of introducing a
> > new flag like my patch, yes. It's just a matter of easiness.
> > Since there is no link from usb_mixer_elem_info to the assigned
> > snd_kcontorl, we'd need to add an extra argument in the call path to
> > inform about the volatileness, and it may become messy. We'll find
> > the simplest way.
> >
> > And, IMO, it'd be better to check the cache behavior at setting, since
> > the cache may be accessed via multiple paths (at reading each element
> > and at resuming the whole system). But it's no requirement, either.
>
> I have tried with attached patch, on top of your patch, but I'm still
> getting cached values from libasound2. It didn't break anything
> either... However, I don't see a link between this cache behavior and
> specific user space mixer descriptor. Because polling the mixer
> element with snd_mixer_selem_get_capture_volume() keeps always
> returning the same value. While in parallel checking the value by
> running "amixer" from command line gives updated correct value... So
> is there a second layer of cache somewhere in libasound2 to explain
> this discrepancy? Because one process sees updated value while other
> one doesn't. Difference being that the ALSA descriptors are closed and
> reopened every time "amixer" is run...
Erm, I should have looked your *actual* patch before replying.
Basically all your newly created elements are independent from the
other USB standard things, and they are all read-only. That is, you
don't need to use add_single_ctl_with_resume(), but just create each
element via snd_ctl_new1() and add it via snd_ctl_add().
(Also you can pass the mixer object directly in private_value.)
If it still doesn't work as expected, it's rather something wrong in
the alsa-lib side, or ACCESS_VOLATILE flag isn't passed / set
properly.
Takashi
> What is more perplexing is that from HDSPe driver same use case works,
> but it could be due to some event trigger from the HDSPe driver based
> on interrupt?
> - Jussi
> diff --git a/sound/usb/mixer_quirks.c b/sound/usb/mixer_quirks.c
> index 762c190..cff31ce 100644
> --- a/sound/usb/mixer_quirks.c
> +++ b/sound/usb/mixer_quirks.c
> @@ -152,20 +152,24 @@ static int add_single_ctl_with_resume(struct usb_mixer_interface *mixer,
> const struct snd_kcontrol_new *knew,
> struct usb_mixer_elem_list **listp)
> {
> + struct usb_mixer_elem_info *cval;
> struct usb_mixer_elem_list *list;
> struct snd_kcontrol *kctl;
>
> - list = kzalloc(sizeof(*list), GFP_KERNEL);
> - if (!list)
> + cval = kzalloc(sizeof(*cval), GFP_KERNEL);
> + if (!cval)
> return -ENOMEM;
> + list = &cval->head;
> if (listp)
> *listp = list;
> + if (knew->access & SNDRV_CTL_ELEM_ACCESS_VOLATILE)
> + cval->is_volatile = 1;
> list->mixer = mixer;
> list->id = id;
> list->resume = resume;
> - kctl = snd_ctl_new1(knew, list);
> + kctl = snd_ctl_new1(knew, cval);
> if (!kctl) {
> - kfree(list);
> + kfree(cval);
> return -ENOMEM;
> }
> kctl->private_free = snd_usb_mixer_elem_free;
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@xxxxxxxxxxxxxxxx
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
