Commit 63e20df1 authored by Takashi Iwai's avatar Takashi Iwai

ALSA: line6: Reorganize PCM stream handling

The current code deals with the stream start / stop solely via
line6_pcm_acquire() and line6_pcm_release().  This was (supposedly)
intended to avoid the races, but it doesn't work as expected.  The
concurrent acquire and release calls can be performed without proper
protections, thus this might result in memory corruption.
Furthermore, we can't take a mutex to protect the whole function
because it can be called from the PCM trigger callback that is an
atomic context.  Also spinlock isn't appropriate because the function
allocates with kmalloc with GFP_KERNEL.  That is, these function just
lead to singular problems.

This is an attempt to reduce the existing races.  First off, separate
both the stream buffer management and the stream URB management.  The
former is protected via a newly introduced state_mutex while the
latter is protected via each line6_pcm_stream lock.

Secondly, the stream state are now managed in opened and running bit
flags of each line6_pcm_stream.  Not only this a bit clearer than
previous combined bit flags, this also gives a better abstraction.
These rewrites allows us to make common hw_params and hw_free
callbacks for both playback and capture directions.

For the monitor and impulse operations, still line6_pcm_acquire() and
line6_pcm_release() are used.  They call internally the corresponding
functions for both playback and capture streams with proper lock or
mutex.  Unlike the previous versions, these function don't take the
bit masks but the only single type value.  Also they are supposed to
be applied only as duplex operations.
Tested-by: default avatarChris Rorvick <chris@rorvick.com>
Signed-off-by: default avatarTakashi Iwai <tiwai@suse.de>
parent f2bb614b
......@@ -67,20 +67,18 @@ static int submit_audio_in_urb(struct snd_line6_pcm *line6pcm)
/*
Submit all currently available capture URBs.
must be called in line6pcm->in.lock context
*/
int line6_submit_audio_in_all_urbs(struct snd_line6_pcm *line6pcm)
{
unsigned long flags;
int ret = 0, i;
spin_lock_irqsave(&line6pcm->in.lock, flags);
for (i = 0; i < LINE6_ISO_BUFFERS; ++i) {
ret = submit_audio_in_urb(line6pcm);
if (ret < 0)
break;
}
spin_unlock_irqrestore(&line6pcm->in.lock, flags);
return ret;
}
......@@ -187,10 +185,10 @@ static void audio_in_callback(struct urb *urb)
line6pcm->prev_fbuf = fbuf;
line6pcm->prev_fsize = fsize;
if (!(line6pcm->flags & LINE6_BITS_PCM_IMPULSE))
if (test_bit(LINE6_INDEX_PCM_ALSA_CAPTURE_STREAM,
&line6pcm->flags) && (fsize > 0))
line6_capture_copy(line6pcm, fbuf, fsize);
if (!test_bit(LINE6_STREAM_IMPULSE, &line6pcm->in.running) &&
test_bit(LINE6_STREAM_PCM, &line6pcm->in.running) &&
fsize > 0)
line6_capture_copy(line6pcm, fbuf, fsize);
}
clear_bit(index, &line6pcm->in.active_urbs);
......@@ -201,10 +199,9 @@ static void audio_in_callback(struct urb *urb)
if (!shutdown) {
submit_audio_in_urb(line6pcm);
if (!(line6pcm->flags & LINE6_BITS_PCM_IMPULSE))
if (test_bit(LINE6_INDEX_PCM_ALSA_CAPTURE_STREAM,
&line6pcm->flags))
line6_capture_check_period(line6pcm, length);
if (!test_bit(LINE6_STREAM_IMPULSE, &line6pcm->in.running) &&
test_bit(LINE6_STREAM_PCM, &line6pcm->in.running))
line6_capture_check_period(line6pcm, length);
}
spin_unlock_irqrestore(&line6pcm->in.lock, flags);
......@@ -234,71 +231,6 @@ static int snd_line6_capture_close(struct snd_pcm_substream *substream)
return 0;
}
/* hw_params capture callback */
static int snd_line6_capture_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *hw_params)
{
int ret;
struct snd_line6_pcm *line6pcm = snd_pcm_substream_chip(substream);
ret = line6_pcm_acquire(line6pcm, LINE6_BIT_PCM_ALSA_CAPTURE_BUFFER);
if (ret < 0)
return ret;
ret = snd_pcm_lib_malloc_pages(substream,
params_buffer_bytes(hw_params));
if (ret < 0) {
line6_pcm_release(line6pcm, LINE6_BIT_PCM_ALSA_CAPTURE_BUFFER);
return ret;
}
line6pcm->in.period = params_period_bytes(hw_params);
return 0;
}
/* hw_free capture callback */
static int snd_line6_capture_hw_free(struct snd_pcm_substream *substream)
{
struct snd_line6_pcm *line6pcm = snd_pcm_substream_chip(substream);
line6_pcm_release(line6pcm, LINE6_BIT_PCM_ALSA_CAPTURE_BUFFER);
return snd_pcm_lib_free_pages(substream);
}
/* trigger callback */
int snd_line6_capture_trigger(struct snd_line6_pcm *line6pcm, int cmd)
{
int err;
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
case SNDRV_PCM_TRIGGER_RESUME:
err = line6_pcm_acquire(line6pcm,
LINE6_BIT_PCM_ALSA_CAPTURE_STREAM);
if (err < 0)
return err;
break;
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_SUSPEND:
err = line6_pcm_release(line6pcm,
LINE6_BIT_PCM_ALSA_CAPTURE_STREAM);
if (err < 0)
return err;
break;
default:
return -EINVAL;
}
return 0;
}
/* capture pointer callback */
static snd_pcm_uframes_t
snd_line6_capture_pointer(struct snd_pcm_substream *substream)
......@@ -313,8 +245,8 @@ struct snd_pcm_ops snd_line6_capture_ops = {
.open = snd_line6_capture_open,
.close = snd_line6_capture_close,
.ioctl = snd_pcm_lib_ioctl,
.hw_params = snd_line6_capture_hw_params,
.hw_free = snd_line6_capture_hw_free,
.hw_params = snd_line6_hw_params,
.hw_free = snd_line6_hw_free,
.prepare = snd_line6_prepare,
.trigger = snd_line6_trigger,
.pointer = snd_line6_capture_pointer,
......
......@@ -25,6 +25,5 @@ extern void line6_capture_check_period(struct snd_line6_pcm *line6pcm,
int length);
extern int line6_create_audio_in_urbs(struct snd_line6_pcm *line6pcm);
extern int line6_submit_audio_in_all_urbs(struct snd_line6_pcm *line6pcm);
extern int snd_line6_capture_trigger(struct snd_line6_pcm *line6pcm, int cmd);
#endif
This diff is collapsed.
......@@ -54,109 +54,33 @@
However, from the device's point of view, there is just a single
capture and playback stream, which must be shared between these
subsystems. It is therefore necessary to maintain the state of the
subsystems with respect to PCM usage. We define several constants of
the form LINE6_BIT_PCM_<subsystem>_<direction>_<resource> with the
following meanings:
*) <subsystem> is one of
-) ALSA: PCM playback and capture via ALSA
-) MONITOR: software monitoring
-) IMPULSE: optional impulse response measurement
*) <direction> is one of
-) PLAYBACK: audio output (from host to device)
-) CAPTURE: audio input (from device to host)
*) <resource> is one of
-) BUFFER: buffer required by PCM data stream
-) STREAM: actual PCM data stream
The subsystems call line6_pcm_acquire() to acquire the (shared)
resources needed for a particular operation (e.g., allocate the buffer
for ALSA playback or start the capture stream for software monitoring).
When a resource is no longer needed, it is released by calling
line6_pcm_release(). Buffer allocation and stream startup are handled
separately to allow the ALSA kernel driver to perform them at
appropriate places (since the callback which starts a PCM stream is not
allowed to sleep).
subsystems with respect to PCM usage.
We define two bit flags, "opened" and "running", for each playback
or capture stream. Both can contain the bit flag corresponding to
LINE6_STREAM_* type,
LINE6_STREAM_PCM = ALSA PCM playback or capture
LINE6_STREAM_MONITOR = software monitoring
IMPULSE = optional impulse response measurement
The opened flag indicates whether the buffer is allocated while
the running flag indicates whether the stream is running.
For monitor or impulse operations, the driver needs to call
snd_line6_duplex_acquire() or snd_line6_duplex_release() with the
appropriate LINE6_STREAM_* flag.
*/
/* stream types */
enum {
LINE6_STREAM_PCM,
LINE6_STREAM_MONITOR,
LINE6_STREAM_IMPULSE,
};
/* misc bit flags for PCM operation */
enum {
/* individual bit indices: */
LINE6_INDEX_PCM_ALSA_PLAYBACK_BUFFER,
LINE6_INDEX_PCM_ALSA_PLAYBACK_STREAM,
LINE6_INDEX_PCM_ALSA_CAPTURE_BUFFER,
LINE6_INDEX_PCM_ALSA_CAPTURE_STREAM,
LINE6_INDEX_PCM_MONITOR_PLAYBACK_BUFFER,
LINE6_INDEX_PCM_MONITOR_PLAYBACK_STREAM,
LINE6_INDEX_PCM_MONITOR_CAPTURE_BUFFER,
LINE6_INDEX_PCM_MONITOR_CAPTURE_STREAM,
LINE6_INDEX_PCM_IMPULSE_PLAYBACK_BUFFER,
LINE6_INDEX_PCM_IMPULSE_PLAYBACK_STREAM,
LINE6_INDEX_PCM_IMPULSE_CAPTURE_BUFFER,
LINE6_INDEX_PCM_IMPULSE_CAPTURE_STREAM,
LINE6_INDEX_PAUSE_PLAYBACK,
LINE6_INDEX_PREPARED,
#define LINE6_BIT(x) LINE6_BIT_ ## x = 1 << LINE6_INDEX_ ## x
/* individual bit masks: */
LINE6_BIT(PCM_ALSA_PLAYBACK_BUFFER),
LINE6_BIT(PCM_ALSA_PLAYBACK_STREAM),
LINE6_BIT(PCM_ALSA_CAPTURE_BUFFER),
LINE6_BIT(PCM_ALSA_CAPTURE_STREAM),
LINE6_BIT(PCM_MONITOR_PLAYBACK_BUFFER),
LINE6_BIT(PCM_MONITOR_PLAYBACK_STREAM),
LINE6_BIT(PCM_MONITOR_CAPTURE_BUFFER),
LINE6_BIT(PCM_MONITOR_CAPTURE_STREAM),
LINE6_BIT(PCM_IMPULSE_PLAYBACK_BUFFER),
LINE6_BIT(PCM_IMPULSE_PLAYBACK_STREAM),
LINE6_BIT(PCM_IMPULSE_CAPTURE_BUFFER),
LINE6_BIT(PCM_IMPULSE_CAPTURE_STREAM),
LINE6_BIT(PAUSE_PLAYBACK),
LINE6_BIT(PREPARED),
/* combined bit masks (by operation): */
LINE6_BITS_PCM_ALSA_BUFFER =
LINE6_BIT_PCM_ALSA_PLAYBACK_BUFFER |
LINE6_BIT_PCM_ALSA_CAPTURE_BUFFER,
LINE6_BITS_PCM_ALSA_STREAM =
LINE6_BIT_PCM_ALSA_PLAYBACK_STREAM |
LINE6_BIT_PCM_ALSA_CAPTURE_STREAM,
LINE6_BITS_PCM_MONITOR =
LINE6_BIT_PCM_MONITOR_PLAYBACK_BUFFER |
LINE6_BIT_PCM_MONITOR_PLAYBACK_STREAM |
LINE6_BIT_PCM_MONITOR_CAPTURE_BUFFER |
LINE6_BIT_PCM_MONITOR_CAPTURE_STREAM,
LINE6_BITS_PCM_IMPULSE =
LINE6_BIT_PCM_IMPULSE_PLAYBACK_BUFFER |
LINE6_BIT_PCM_IMPULSE_PLAYBACK_STREAM |
LINE6_BIT_PCM_IMPULSE_CAPTURE_BUFFER |
LINE6_BIT_PCM_IMPULSE_CAPTURE_STREAM,
/* combined bit masks (by direction): */
LINE6_BITS_PLAYBACK_BUFFER =
LINE6_BIT_PCM_IMPULSE_PLAYBACK_BUFFER |
LINE6_BIT_PCM_ALSA_PLAYBACK_BUFFER |
LINE6_BIT_PCM_MONITOR_PLAYBACK_BUFFER,
LINE6_BITS_PLAYBACK_STREAM =
LINE6_BIT_PCM_IMPULSE_PLAYBACK_STREAM |
LINE6_BIT_PCM_ALSA_PLAYBACK_STREAM |
LINE6_BIT_PCM_MONITOR_PLAYBACK_STREAM,
LINE6_BITS_CAPTURE_BUFFER =
LINE6_BIT_PCM_IMPULSE_CAPTURE_BUFFER |
LINE6_BIT_PCM_ALSA_CAPTURE_BUFFER |
LINE6_BIT_PCM_MONITOR_CAPTURE_BUFFER,
LINE6_BITS_CAPTURE_STREAM =
LINE6_BIT_PCM_IMPULSE_CAPTURE_STREAM |
LINE6_BIT_PCM_ALSA_CAPTURE_STREAM |
LINE6_BIT_PCM_MONITOR_CAPTURE_STREAM,
LINE6_BITS_STREAM =
LINE6_BITS_PLAYBACK_STREAM |
LINE6_BITS_CAPTURE_STREAM
LINE6_FLAG_PAUSE_PLAYBACK,
LINE6_FLAG_PREPARED,
};
struct line6_pcm_properties {
......@@ -205,6 +129,12 @@ struct line6_pcm_stream {
*/
spinlock_t lock;
/* Bit flags for operational stream types */
unsigned long opened;
/* Bit flags for running stream types */
unsigned long running;
int last_frame;
};
......@@ -224,6 +154,9 @@ struct snd_line6_pcm {
*/
struct snd_pcm *pcm;
/* protection to state changes of in/out streams */
struct mutex state_mutex;
/* Capture and playback streams */
struct line6_pcm_stream in;
struct line6_pcm_stream out;
......@@ -269,7 +202,7 @@ struct snd_line6_pcm {
int impulse_count;
/**
Several status bits (see LINE6_BIT_*).
Several status bits (see LINE6_FLAG_*).
*/
unsigned long flags;
};
......@@ -278,8 +211,11 @@ extern int line6_init_pcm(struct usb_line6 *line6,
struct line6_pcm_properties *properties);
extern int snd_line6_trigger(struct snd_pcm_substream *substream, int cmd);
extern int snd_line6_prepare(struct snd_pcm_substream *substream);
extern int snd_line6_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *hw_params);
extern int snd_line6_hw_free(struct snd_pcm_substream *substream);
extern void line6_pcm_disconnect(struct snd_line6_pcm *line6pcm);
extern int line6_pcm_acquire(struct snd_line6_pcm *line6pcm, int channels);
extern int line6_pcm_release(struct snd_line6_pcm *line6pcm, int channels);
extern int line6_pcm_acquire(struct snd_line6_pcm *line6pcm, int type);
extern void line6_pcm_release(struct snd_line6_pcm *line6pcm, int type);
#endif
......@@ -194,8 +194,8 @@ static int submit_audio_out_urb(struct snd_line6_pcm *line6pcm)
urb_out->transfer_buffer_length = urb_size;
urb_out->context = line6pcm;
if (test_bit(LINE6_INDEX_PCM_ALSA_PLAYBACK_STREAM, &line6pcm->flags) &&
!test_bit(LINE6_INDEX_PAUSE_PLAYBACK, &line6pcm->flags)) {
if (test_bit(LINE6_STREAM_PCM, &line6pcm->out.running) &&
!test_bit(LINE6_FLAG_PAUSE_PLAYBACK, &line6pcm->flags)) {
struct snd_pcm_runtime *runtime =
get_substream(line6pcm, SNDRV_PCM_STREAM_PLAYBACK)->runtime;
......@@ -239,11 +239,10 @@ static int submit_audio_out_urb(struct snd_line6_pcm *line6pcm)
spin_lock_nested(&line6pcm->in.lock, SINGLE_DEPTH_NESTING);
if (line6pcm->prev_fbuf) {
if (line6pcm->flags & LINE6_BITS_PCM_IMPULSE) {
if (test_bit(LINE6_STREAM_IMPULSE, &line6pcm->out.running)) {
create_impulse_test_signal(line6pcm, urb_out,
bytes_per_frame);
if (line6pcm->flags &
LINE6_BIT_PCM_ALSA_CAPTURE_STREAM) {
if (test_bit(LINE6_STREAM_PCM, &line6pcm->in.running)) {
line6_capture_copy(line6pcm,
urb_out->transfer_buffer,
urb_out->
......@@ -252,11 +251,8 @@ static int submit_audio_out_urb(struct snd_line6_pcm *line6pcm)
urb_out->transfer_buffer_length);
}
} else {
if (!
(line6pcm->line6->
properties->capabilities & LINE6_CAP_HWMON)
&& (line6pcm->flags & LINE6_BITS_PLAYBACK_STREAM)
&& (line6pcm->flags & LINE6_BITS_CAPTURE_STREAM))
if (!(line6pcm->line6->properties->capabilities & LINE6_CAP_HWMON)
&& line6pcm->out.running && line6pcm->in.running)
add_monitor_signal(urb_out, line6pcm->prev_fbuf,
line6pcm->volume_monitor,
bytes_per_frame);
......@@ -279,20 +275,18 @@ static int submit_audio_out_urb(struct snd_line6_pcm *line6pcm)
/*
Submit all currently available playback URBs.
*/
must be called in line6pcm->out.lock context
*/
int line6_submit_audio_out_all_urbs(struct snd_line6_pcm *line6pcm)
{
unsigned long flags;
int ret = 0, i;
spin_lock_irqsave(&line6pcm->out.lock, flags);
for (i = 0; i < LINE6_ISO_BUFFERS; ++i) {
ret = submit_audio_out_urb(line6pcm);
if (ret < 0)
break;
}
spin_unlock_irqrestore(&line6pcm->out.lock, flags);
return ret;
}
......@@ -326,7 +320,7 @@ static void audio_out_callback(struct urb *urb)
spin_lock_irqsave(&line6pcm->out.lock, flags);
if (test_bit(LINE6_INDEX_PCM_ALSA_PLAYBACK_STREAM, &line6pcm->flags)) {
if (test_bit(LINE6_STREAM_PCM, &line6pcm->out.running)) {
struct snd_pcm_runtime *runtime = substream->runtime;
line6pcm->out.pos_done +=
......@@ -350,8 +344,7 @@ static void audio_out_callback(struct urb *urb)
if (!shutdown) {
submit_audio_out_urb(line6pcm);
if (test_bit(LINE6_INDEX_PCM_ALSA_PLAYBACK_STREAM,
&line6pcm->flags)) {
if (test_bit(LINE6_STREAM_PCM, &line6pcm->out.running)) {
line6pcm->out.bytes += length;
if (line6pcm->out.bytes >= line6pcm->out.period) {
line6pcm->out.bytes %= line6pcm->out.period;
......@@ -387,79 +380,6 @@ static int snd_line6_playback_close(struct snd_pcm_substream *substream)
return 0;
}
/* hw_params playback callback */
static int snd_line6_playback_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *hw_params)
{
int ret;
struct snd_line6_pcm *line6pcm = snd_pcm_substream_chip(substream);
ret = line6_pcm_acquire(line6pcm, LINE6_BIT_PCM_ALSA_PLAYBACK_BUFFER);
if (ret < 0)
return ret;
ret = snd_pcm_lib_malloc_pages(substream,
params_buffer_bytes(hw_params));
if (ret < 0) {
line6_pcm_release(line6pcm, LINE6_BIT_PCM_ALSA_PLAYBACK_BUFFER);
return ret;
}
line6pcm->out.period = params_period_bytes(hw_params);
return 0;
}
/* hw_free playback callback */
static int snd_line6_playback_hw_free(struct snd_pcm_substream *substream)
{
struct snd_line6_pcm *line6pcm = snd_pcm_substream_chip(substream);
line6_pcm_release(line6pcm, LINE6_BIT_PCM_ALSA_PLAYBACK_BUFFER);
return snd_pcm_lib_free_pages(substream);
}
/* trigger playback callback */
int snd_line6_playback_trigger(struct snd_line6_pcm *line6pcm, int cmd)
{
int err;
switch (cmd) {
case SNDRV_PCM_TRIGGER_START:
case SNDRV_PCM_TRIGGER_RESUME:
err = line6_pcm_acquire(line6pcm,
LINE6_BIT_PCM_ALSA_PLAYBACK_STREAM);
if (err < 0)
return err;
break;
case SNDRV_PCM_TRIGGER_STOP:
case SNDRV_PCM_TRIGGER_SUSPEND:
err = line6_pcm_release(line6pcm,
LINE6_BIT_PCM_ALSA_PLAYBACK_STREAM);
if (err < 0)
return err;
break;
case SNDRV_PCM_TRIGGER_PAUSE_PUSH:
set_bit(LINE6_INDEX_PAUSE_PLAYBACK, &line6pcm->flags);
break;
case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
clear_bit(LINE6_INDEX_PAUSE_PLAYBACK, &line6pcm->flags);
break;
default:
return -EINVAL;
}
return 0;
}
/* playback pointer callback */
static snd_pcm_uframes_t
snd_line6_playback_pointer(struct snd_pcm_substream *substream)
......@@ -474,8 +394,8 @@ struct snd_pcm_ops snd_line6_playback_ops = {
.open = snd_line6_playback_open,
.close = snd_line6_playback_close,
.ioctl = snd_pcm_lib_ioctl,
.hw_params = snd_line6_playback_hw_params,
.hw_free = snd_line6_playback_hw_free,
.hw_params = snd_line6_hw_params,
.hw_free = snd_line6_hw_free,
.prepare = snd_line6_prepare,
.trigger = snd_line6_trigger,
.pointer = snd_line6_playback_pointer,
......
......@@ -31,6 +31,5 @@ extern struct snd_pcm_ops snd_line6_playback_ops;
extern int line6_create_audio_out_urbs(struct snd_line6_pcm *line6pcm);
extern int line6_submit_audio_out_all_urbs(struct snd_line6_pcm *line6pcm);
extern int snd_line6_playback_trigger(struct snd_line6_pcm *line6pcm, int cmd);
#endif
......@@ -189,9 +189,9 @@ static int snd_toneport_monitor_put(struct snd_kcontrol *kcontrol,
line6pcm->volume_monitor = ucontrol->value.integer.value[0];
if (line6pcm->volume_monitor > 0)
line6_pcm_acquire(line6pcm, LINE6_BITS_PCM_MONITOR);
line6_pcm_acquire(line6pcm, LINE6_STREAM_MONITOR);
else
line6_pcm_release(line6pcm, LINE6_BITS_PCM_MONITOR);
line6_pcm_release(line6pcm, LINE6_STREAM_MONITOR);
return 1;
}
......@@ -252,7 +252,7 @@ static void toneport_start_pcm(unsigned long arg)
struct usb_line6_toneport *toneport = (struct usb_line6_toneport *)arg;
struct usb_line6 *line6 = &toneport->line6;
line6_pcm_acquire(line6->line6pcm, LINE6_BITS_PCM_MONITOR);
line6_pcm_acquire(line6->line6pcm, LINE6_STREAM_MONITOR);
}
/* control definition */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment