Re: Dell USB audio driver workaround

[Takashi Iwai <tiwai@suse.de>]

On Thu, 07 Apr 2016 09:05:52 +0200,
Kailang wrote:
> 
>  
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de] 
> > Sent: Wednesday, April 06, 2016 5:37 PM
> > To: Kailang
> > Cc: (alsa-devel@alsa-project.org)
> > Subject: Re: Dell USB audio driver workaround
> > 
> > On Wed, 06 Apr 2016 11:22:43 +0200,
> > Kailang wrote:
> > > 
> > >  
> > > 
> > > > -----Original Message-----
> > > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > Sent: Wednesday, April 06, 2016 4:57 PM
> > > > To: Kailang
> > > > Cc: (alsa-devel@alsa-project.org)
> > > > Subject: Re: Dell USB audio driver workaround
> > > > 
> > > > On Wed, 06 Apr 2016 08:29:12 +0200,
> > > > Kailang wrote:
> > > > > 
> > > > > Hi Takashi,
> > > > > 
> > > > > Dell had USB dock.
> > > > > Maybe firmware issue, the master volume always keep at 0x57.
> > > > > Attach patch will fix it to keep at Max volume.
> > > > > 
> > > > > Could you help us to update it?
> > > > 
> > > > Well, I don't get how this patch works..
> > > > 
> > > > > diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c index 
> > > > > 4f85757..84d9a4e 100644
> > > > > --- a/sound/usb/mixer.c
> > > > > +++ b/sound/usb/mixer.c
> > > > > @@ -846,6 +846,12 @@ static void volume_control_quirks(struct 
> > > > > usb_mixer_elem_info *cval,  {
> > > > >  	struct snd_usb_audio *chip = cval->head.mixer->chip;
> > > > >  	switch (chip->usb_id) {
> > > > > +	case USB_ID(0x0bda, 0x4014): /* Dell workaround */
> > > > > +		if (strstr(kctl->id.name, "Playback 
> > Volume") != NULL) {
> > > > > +			cval->min = 0x0000;
> > > > > +			cval->max = 0x0000;
> > > > > +		}
> > > > > +		break;
> > > > 
> > > > So, this means that this value has to be fixed to 0?
> > > > If yes, can't we simply remove this volume instead?
> > > 
> > > Yes, but it need to set Max volume to 0. This is test on my site.
> > > Because it will rewrite 0x57 value to codec register when 
> > driver is loaded.
> > > And keep 0x57 volume value all the time. So, Master volume 
> > will lower than windows machine.
> > > This case which driver fail to build the alsa mixer(This is 
> > firmware cause it). So, no hardware volume control on this.
> > > If the Max volume value is 0,  system will not set default 
> > value 0x57 for first time loading driver.
> > 
> > Which values (min/max) does this FU take and which values are buggy?
> Max value. 

Sorry, my question wasn't clear.  "What" min and max are they?  min=0
and max=0x57?

And, your patch changes both min and max to be 0.  That is, it accepts
only 0.  Does it mean that this FU isn't any proper volume control?


> But it is not buggy value. Min and Max value are all safe. It got right value from firmware.
> But if Max value > 0, maybe < 0x57, it will be not rewrited value.
> Please look for belowing function. 
> 
> Our USB firmware fail for build alsa mixer. And we fixed the firmware already.
> But Dell didn't want to update firmware. Because they release this MP more and more.
> So, they request a workaround for this unit.
> 
> Below is in mixer.c
> 
> static int mixer_ctl_feature_put(struct snd_kcontrol *kcontrol,
>      struct snd_ctl_elem_value *ucontrol)
> {
>  struct usb_mixer_elem_info *cval = kcontrol->private_data;
>  int c, cnt, val, oval, err;
>  int changed = 0;
>  
>  if (cval->cmask) {
>   cnt = 0;
>   for (c = 0; c < MAX_CHANNELS; c++) {
>    if (!(cval->cmask & (1 << c)))
>     continue;
>    err = get_cur_mix_value(cval, c + 1, cnt, &oval);
>    if (err < 0)
>     return cval->mixer->ignore_ctl_error ? 0 : err;
>    val = ucontrol->value.integer.value[cnt];   =========> This line will get default value( 0x57) from mixer.

So, do you mean that writing the default value (0x57) from the
firmware causes the trouble, and it has to be 0?


>    val = get_abs_value(cval, val);
>    if (oval != val) {
>     set_cur_mix_value(cval, c + 1, cnt, val);
>     changed = 1;
>    }
>    cnt++;
>   }
>  } else {
>   /* master channel */
>   err = get_cur_mix_value(cval, 0, 0, &oval);
>   if (err < 0)
>    return cval->mixer->ignore_ctl_error ? 0 : err;
>   val = ucontrol->value.integer.value[0];
>   val = get_abs_value(cval, val);
>   if (val != oval) {
>    set_cur_mix_value(cval, 0, 0, val);
>    changed = 1;
>   }
>  }
>  return changed;
> }
> 
> > Judging from your description, the firmware gives the bogus 
> > max value (white min value = 0) and a volume higher than zero 
> > is attenuation?
> 
> I had not test Max value > 0 and < 0x57.
> cval->min = 0x0000;  => this is original. I just post it. Remove this line is OK.

Well, I still don't understand clearly.  What do you mean here as
"original"?


thanks,

Takashi