ALSA Control Questions

ALSA Control Questions

[Tobias Schneider]

I've got some questions concerning the ALSA control interface...

1. What is a "simple" control? I would like to use the control interface 
to set more than just volume (e.g. I would like to set input gain, 
output attenuation, input channel selection, output channel selection, 
output channel activation...), so I am wondering if I should use 
SNDRV_CTL_ELEM_IFACE_CARD or SNDRV_CTL_ELEM_IFACE_MIXER, as they are 
obviously in context with "simple" control...

2. The more interesting question: as written in "Writing an alsa driver" 
callbacks are basically not atomic, but I am getting  BUG: scheduling 
while atomic: amixer/0x00000001/430 Call Trace: .[..] in "put" 
callback?? So what is meant with "basically not atomic"?? What is 
allowed and what is not allowed, am I able to get hardware interrupts in 
this context?

Re: ALSA Control Questions

[Daniel Mack]

On Thu, Dec 03, 2009 at 07:30:38PM +0100, Tobias Schneider wrote:
> 2. The more interesting question: as written in "Writing an alsa driver" 
> callbacks are basically not atomic, but I am getting  BUG: scheduling 
> while atomic: amixer/0x00000001/430 Call Trace: .[..] in "put" 
> callback?? So what is meant with "basically not atomic"?? What is 
> allowed and what is not allowed, am I able to get hardware interrupts in 
> this context?

That message basically means that you called a scheduling function while
you have preemption switched off. One common pitfall is to let the CPU
reschedule from an interrupt handler or while holding a spinlock.

The basic problem is that if you hold a spinlock and let the CPU do
something else, some other execution path (on the same CPU) could
also try to access the same lock which then deadlocks your system.

It all boils down to: from interrupt handlers or while you hold a
spinlock, you must never call any function that will sleep (and thereby
reschedule). Some functions such like kmalloc have a flag that you can
pass depending on whether you want it to allow to sleep (GFP_KERNEL)
or not (GFP_ATOMIC). If you necessarily _have_ to sleep from atomic
context, you must postpone your work using a work_struct or something
similar.

You should post your code so people can have a look.

HTH,
Daniel

Re: ALSA Control Questions (atomicity, error handling)

[Tobias Schneider]

Thanks for the reply.

Actually I know where the problem in my code is: inside the put callback 
(see below) I am calling a function (dsp_setoutput) that is sending the 
signal to the DSP, there I wanted to wait for the reply of the DSP to 
get a feedback whether the request was successful or not. This was done 
by a semaphore (going down and waiting for IRQ from DSP that is setting 
sema up), and thats the point where the posted bug occured.

So I was just wondering that it seems that there is no possibility to 
wait for a reply of the DSP, to be sure that a given value has been set?


(for the interested ones, here is the code..)

static int snd_mychip_output_set_put(struct snd_kcontrol *kcontrol,
                                     struct snd_ctl_elem_value *ucontrol)
{
  struct snd_mychip *mychip = snd_kcontrol_chip(kcontrol);
  int addr = kcontrol->private_value;
  short change = 0; 

  spin_lock_irq(&mychip->mixer_lock);
  if (ucontrol->value.integer.value[0] != mychip->output_set[addr]) {
    if (dsp_setoutput(addr+1, ucontrol->value.integer.value[0]) >= 0)
    {
      mychip->output_set[addr] = ucontrol->value.integer.value[0];
      change = 1;
    }
    else  // error sending signal
    {
      change = -EBUSY;  // device or resource busy
    }
  }
  spin_unlock_irq(&mychip->mixer_lock);
  return change;
}

later on in the called function...
 // wait for reply via semaphore
  while(!end)
  {
    if (down_interruptible(&dsp_reply_sema)==-EINTR)   // HERE WE GET 
BUG: scheduling while atomic: amixer/0x00000001/430 ...
      // waked by signal (terminate), leave thread
      printk(INFO "reply: catched signal\n");
      end=1;
      break;
    }   
    printk(INFO "dsp_reply signaled (saved status = 
%i)\n",stDspReply.status);
    end=1;
 ...
}



Another point: I figured out that the "ALSA middlelayer" seems to filter 
the given values of a control. So if the user sets value=1000 where 
maximum is 10, I will get 10 instead of 1000 in the put callback...so 
it's not necessary to check values in those callbacks, but unfortunately 
that's not always good - because sometimes it doesn't make sense to use 
the max or min values if the user just entered a wrong number...

thanks so far

Re: ALSA Control Questions (atomicity, error handling)

[Daniel Mack]

On Fri, Dec 04, 2009 at 10:31:06AM +0100, Tobias Schneider wrote:
> Actually I know where the problem in my code is: inside the put
> callback (see below) I am calling a function (dsp_setoutput) that is
> sending the signal to the DSP, there I wanted to wait for the reply
> of the DSP to get a feedback whether the request was successful or
> not. This was done by a semaphore (going down and waiting for IRQ
> from DSP that is setting sema up), and thats the point where the
> posted bug occured.
> 
> So I was just wondering that it seems that there is no possibility
> to wait for a reply of the DSP, to be sure that a given value has
> been set?
> 
> 
> (for the interested ones, here is the code..)
> 
> static int snd_mychip_output_set_put(struct snd_kcontrol *kcontrol,
>                                     struct snd_ctl_elem_value *ucontrol)
> {
>  struct snd_mychip *mychip = snd_kcontrol_chip(kcontrol);
>  int addr = kcontrol->private_value;
>  short change = 0;
> 
>  spin_lock_irq(&mychip->mixer_lock);

As I said - here you're holding a spinlock ...

>  if (ucontrol->value.integer.value[0] != mychip->output_set[addr]) {
>    if (dsp_setoutput(addr+1, ucontrol->value.integer.value[0]) >= 0)
>    {
>      mychip->output_set[addr] = ucontrol->value.integer.value[0];
>      change = 1;
>    }
>    else  // error sending signal
>    {
>      change = -EBUSY;  // device or resource busy
>    }
>  }
>  spin_unlock_irq(&mychip->mixer_lock);
>  return change;
> }
> 
> later on in the called function...
> // wait for reply via semaphore
>  while(!end)
>  {
>    if (down_interruptible(&dsp_reply_sema)==-EINTR)   // HERE WE GET

And here you allow the CPU to reschedule while waiting for a mutex.
That doesn't fly. You should remove the spinlock from the function above
and move it to the actual lowlevel I/O function (whatever
dsp_setoutput() does eventually).

Think about where locking is actually needed, at which point your
function flow must not be interrupted by anything to avoid data
corruption. Use locking there, and remove it from other places.

This might also be a good read to get an overview:

  http://www.kernel.org/pub/linux/kernel/people/rusty/kernel-locking/index.html

> Another point: I figured out that the "ALSA middlelayer" seems to
> filter the given values of a control. So if the user sets value=1000
> where maximum is 10, I will get 10 instead of 1000 in the put
> callback...so it's not necessary to check values in those callbacks,
> but unfortunately that's not always good - because sometimes it
> doesn't make sense to use the max or min values if the user just
> entered a wrong number...

For integer values, it does make sense to clamp values to their limits.
For enums, that might be different. Not sure what the ALSA core does
there.

Daniel

Re: ALSA Control Questions (atomicity, error handling)

[Daniel Mack]

On Fri, Dec 04, 2009 at 10:31:06AM +0100, Tobias Schneider wrote:
> (for the interested ones, here is the code..)

One more thing: in case you intend to bring your code mainline, please
run scripts/checkpatch.pl on the patches before you submit. The sniplet
you posted has a number of style issues that should be fixed. In
particular, pay attention to indentation, comment style and parenthesis
positions - checkpatch.pl will help pointing out most if them.

Daniel

> static int snd_mychip_output_set_put(struct snd_kcontrol *kcontrol,
>                                     struct snd_ctl_elem_value *ucontrol)
> {
>  struct snd_mychip *mychip = snd_kcontrol_chip(kcontrol);
>  int addr = kcontrol->private_value;
>  short change = 0;
> 
>  spin_lock_irq(&mychip->mixer_lock);
>  if (ucontrol->value.integer.value[0] != mychip->output_set[addr]) {
>    if (dsp_setoutput(addr+1, ucontrol->value.integer.value[0]) >= 0)
>    {
>      mychip->output_set[addr] = ucontrol->value.integer.value[0];
>      change = 1;
>    }
>    else  // error sending signal
>    {
>      change = -EBUSY;  // device or resource busy
>    }
>  }
>  spin_unlock_irq(&mychip->mixer_lock);
>  return change;
> }
> 
> later on in the called function...
> // wait for reply via semaphore
>  while(!end)
>  {
>    if (down_interruptible(&dsp_reply_sema)==-EINTR)   // HERE WE GET
> BUG: scheduling while atomic: amixer/0x00000001/430 ...
>      // waked by signal (terminate), leave thread
>      printk(INFO "reply: catched signal\n");
>      end=1;
>      break;
>    }      printk(INFO "dsp_reply signaled (saved status =
> %i)\n",stDspReply.status);
>    end=1;
> ...
> }

Re: ALSA Control Questions (atomicity, error handling)

[Tobias Schneider]

okay, so it was my fault, thanks!


> One more thing: in case you intend to bring your code mainline, please
> run scripts/checkpatch.pl on the patches before you submit. The sniplet
> you posted has a number of style issues that should be fixed. In
> particular, pay attention to indentation, comment style and parenthesis
> positions - checkpatch.pl will help pointing out most if them.
>
> Daniel
is there also any document (something like ALSA coding style guide)?

so far,
thanks a lot
Tobias

Re: ALSA Control Questions (atomicity, error handling)

[Daniel Mack]

On Fri, Dec 04, 2009 at 01:02:35PM +0100, Tobias Schneider wrote:
> >One more thing: in case you intend to bring your code mainline, please
> >run scripts/checkpatch.pl on the patches before you submit. The sniplet
> >you posted has a number of style issues that should be fixed. In
> >particular, pay attention to indentation, comment style and parenthesis
> >positions - checkpatch.pl will help pointing out most if them.
> >
> >Daniel
> is there also any document (something like ALSA coding style guide)?

It's not ALSA specific but applies (or should apply) to all kernel code.
Read Documentation/CodingStyle :)

Daniel

Re: ALSA Control Questions (atomicity, error handling)

[Clemens Ladisch]

Tobias Schneider wrote:
> Another point: I figured out that the "ALSA middlelayer" seems to filter
> the given values of a control. So if the user sets value=1000 where
> maximum is 10, I will get 10 instead of 1000 in the put callback...so
> it's not necessary to check values in those callbacks,

It is, because it is possible to use low-level ALSA functions or to call
the IOCTLs directly, and in this way arbitrary values can reach your
driver's put callback.


Best regards,
Clemens