[alsa-devel] [RFC] ALSA: usb-audio: Add custom mixer status quirks for RME CC devices

[Takashi Iwai]

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 at alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel