* Kernel crash
@ 2004-09-05 18:51 Giuliano Pochini
2004-09-06 15:16 ` Takashi Iwai
0 siblings, 1 reply; 19+ messages in thread
From: Giuliano Pochini @ 2004-09-05 18:51 UTC (permalink / raw)
To: Alsa-devel
[-- Attachment #1: Type: text/plain, Size: 942 bytes --]
10 days passed since the last time I wrote about this problem... but the
problems is still here. Perhaps I didn't explain clearly enough. The program
I attached was buggy, but I meant it makes the kernel crash, not the program
itself. It's attached again to this message. No need to be root. IMHO it's a
quite serious issue the should be fixed ASAP.
It successfully crashes my system after a few attempts:
Linux Jay 2.6.8.1 #2 SMP Wed Aug 18 15:14:51 CEST 2004 ppc unknown
booted with maxcpus=1 and without preempt.
alsa-driver-1.0.6a and CVS-2004-09-05
alsa-lib-1.0.4
gcc 3.4.1
with both the powermac (Snapper) driver and my Echoaudio driver.
I also tested it on a PC with an intel8x1 chip, but I don't know the version
of alsa. Probably 1.0.2. It's the one include with linux 2.6.7-13-mandrake
edition. On this system the kernel doesn't crash nor it oopses, but often
the sound doesn't stop after my proggy segfaults.
--
Giuliano.
[-- Attachment #2: playrec.c --]
[-- Type: text/plain, Size: 7442 bytes --]
#include <math.h>
#include <stdio.h>
#include <stdlib.h>
#include <stdint.h>
#include <fcntl.h>
#include <unistd.h>
#include <string.h>
#include <errno.h>
#include <sys/poll.h>
#define ALSA_PCM_NEW_HW_PARAMS_API
#define ALSA_PCM_NEW_SW_PARAMS_API
#include <alsa/asoundlib.h>
int samplerate = 44100, periods = 2, periodframes = 4096;
typedef struct {
int16_t frame[2];
} sample_t;
#define NCH 2
#define TEST(err) do { if (err<0) { printf("Error: %s at line %d\n", snd_strerror(err), __LINE__); return 0; } } while(0)
snd_pcm_t *init_channel(const char *pcm_name, snd_pcm_stream_t stream) {
int n, err;
snd_pcm_hw_params_t *hwparams;
snd_pcm_sw_params_t *swparams;
snd_pcm_t *pcm_handle;
snd_pcm_hw_params_alloca(&hwparams);
snd_pcm_sw_params_alloca(&swparams);
err = snd_pcm_open(&pcm_handle, pcm_name, stream, SND_PCM_NONBLOCK);
TEST(err);
/* Init hwparams with full configuration space */
err = snd_pcm_hw_params_any(pcm_handle, hwparams);
TEST(err);
// printf("snd_pcm_hw_params_can_sync_start: %d\n", snd_pcm_hw_params_can_sync_start(hwparams));
// return(0);
err = snd_pcm_hw_params_set_access(pcm_handle, hwparams, SND_PCM_ACCESS_RW_INTERLEAVED);
TEST(err);
/* Set sample format */
err = snd_pcm_hw_params_set_format(pcm_handle, hwparams, SND_PCM_FORMAT_S16_BE);
TEST(err);
/* Set sample rate. If the exact rate is not supported */
/* by the hardware, use nearest possible rate. */
n = samplerate;
snd_pcm_hw_params_set_rate_near(pcm_handle, hwparams, &samplerate, 0);
if (n != samplerate) {
fprintf(stderr, "The rate %d Hz is not supported by your hardware.\nUsing %d Hz instead.\n", n, samplerate);
}
/* Set number of channels */
err = snd_pcm_hw_params_set_channels(pcm_handle, hwparams, 2);
TEST(err);
/* Set number of periods. Periods used to be called fragments. */
err = snd_pcm_hw_params_set_periods(pcm_handle, hwparams, periods, 0);
TEST(err);
err = snd_pcm_hw_params_set_buffer_size(pcm_handle, hwparams, periodframes * periods);
TEST(err);
err = snd_pcm_hw_params(pcm_handle, hwparams);
TEST(err);
if (stream == SND_PCM_STREAM_PLAYBACK) {
err = snd_pcm_sw_params_current(pcm_handle, swparams);
TEST(err);
err = snd_pcm_sw_params_set_start_threshold(pcm_handle, swparams, 0x7fffffff);
TEST(err);
}
// snd_pcm_hw_params_free(hwparams);
return (pcm_handle);
}
void Riempibuf(sample_t * buf, float frq, int n) {
int s;
static double fase = 0;
//printf("fase i=%f ", fase);
for (s = 0; s < n; s++) {
buf[s].frame[0] = buf[s].frame[1] = ceil(31000.0 * sin(fase * 2 * M_PI));
// buf[s].frame[0] = buf[s].frame[1] = ceil(15000.0 * sin(fase * 2 * M_PI) + 15000.0 * sin(fase*2 * 2 * M_PI));
//buf[s].frame[0] = buf[s].frame[1] = ceil(-32765 + 65530 * fmod(fase, 1));
//buf[s].frame[0] = buf[s].frame[1] = fmod(fase, 1)>=0.5 ? 15000.0 : -15000.0;
fase += frq / samplerate;
}
//printf("f=%f\n", fase);
while (fase > 10) fase -= 10;
}
/*
void Riempibuf(sample_t * buf, float frq, int n) {
int s;
static double fase1 = 0, fase2 = 0;
//printf("fase i=%f ", fase);
for (s = 0; s < n; s++) {
buf[s].frame[0] = buf[s].frame[1] = ceil(15000.0 * sin(fase1 * 2 * M_PI) + 15000.0 * sin(fase2 * 2 * M_PI));
fase1 += 10000.0 / samplerate;
fase2 += 11000.0 / samplerate;
}
}
*/
int writebuf(snd_pcm_t * handle, sample_t * buf, size_t frames) {
int r, tot;
tot = 0;
while (frames > 0) {
r = snd_pcm_writei(handle, buf + tot, frames);
if (r == -EAGAIN) {
// return(0);
continue;
} else if (r > 0) {
frames -= r;
tot += r;
} else {
printf("wr err\n");
return (r);
}
}
// printf("Scritti %d\n", tot);
return (0);
}
int main(int argc, char **argv) {
sample_t *buffer_in, *buffer_out;
double frq;
int i, c, r, bufsize, scritti, ricevuti, f;
int min[NCH], max[NCH];
snd_pcm_t *pcm_handle_in, *pcm_handle_out;
pcm_handle_out = init_channel("plughw:0,0,0", SND_PCM_STREAM_PLAYBACK);
if (!pcm_handle_out)
return (-1);
pcm_handle_in = init_channel("plughw:0,0,0", SND_PCM_STREAM_CAPTURE);
if (!pcm_handle_in)
return (-1);
if ((r = snd_pcm_prepare(pcm_handle_out)) < 0) {
printf("Prepare error: %s\n", snd_strerror(r));
exit(0);
}
if ((r = snd_pcm_prepare(pcm_handle_in)) < 0) {
printf("Prepare error: %s\n", snd_strerror(r));
exit(0);
}
if ((r = snd_pcm_link(pcm_handle_out, pcm_handle_in)) < 0) {
printf("Streams link error: %s\n", snd_strerror(r));
exit(0);
}
/*
if (writebuf(pcm_handle_out, buffer_out, BUFR) < 0) {
fprintf(stderr, "write error\n");
exit(0);
}
*/
/*
if ((r = snd_pcm_start(pcm_handle_in)) < 0) {
printf("Go error: %s\n", snd_strerror(r));
exit(0);
}
*/
bufsize=samplerate;
buffer_in=malloc(bufsize * sizeof(sample_t));
buffer_out=malloc(bufsize * sizeof(sample_t));
f = open("Merd.raw", O_TRUNC | O_WRONLY | O_CREAT, 0664);
// for (frq = (float)samplerate / 8192.0; frq <= (float)samplerate / 2; frq *= 1.189207115) { //frq*=1.414213562) {
for (frq = 1000; frq <= 10000; frq += 100) {
// for (frq = 5.0; frq <= (float)samplerate / 2; frq *= sqrt(sqrt(2.0))) {
//int yyy=3;
// Prepara 1 sec. di roba da suonare
Riempibuf(buffer_out, frq, bufsize);
scritti=ricevuti=0;
while (ricevuti<bufsize) {
poll(0, 0, 5);
if (0)
snd_pcm_wait(pcm_handle_in, 1000);
if (scritti == bufsize) {
//snd_pcm_drain(pcm_handle_out);
printf("spedito\n");
//write(f, buffer_out + scritti, rimanenti * sizeof(sample_t));
} else {
r = snd_pcm_writei(pcm_handle_out, buffer_out+scritti, bufsize-scritti);
if (r < 0 && r != -EAGAIN) {
fprintf(stderr, "write error: %s\n", snd_strerror(r));
exit(r);
} else if (r > 0) {
//printf("scr %d %d\n",r,scritti);
//write(f, buffer_out + scritti, r * sizeof(sample_t));
scritti += r;
}
}
do {
r = snd_pcm_readi(pcm_handle_in, buffer_in+ricevuti, bufsize-ricevuti);
if (r > 0) {
write(f, buffer_in, r * sizeof(sample_t));
ricevuti+=r;
printf("rice %d (%d)\n", r, ricevuti);
} else if (r < 0 && r != -EAGAIN) {
printf("rice err: %s\n", snd_strerror(r));
exit(r);
}
} while (r > 0);
}
/* BOOM ! */
snd_pcm_drain(pcm_handle_out);
snd_pcm_drop(pcm_handle_in);
/*
if ((r = snd_pcm_prepare(pcm_handle_out)) < 0) {
printf("Prepare error: %s\n", snd_strerror(r));
exit(0);
}
if ((r = snd_pcm_prepare(pcm_handle_in)) < 0) {
printf("Prepare error: %s\n", snd_strerror(r));
exit(0);
}
*/
min[c] = max[c] = 0;
printf("%f %f %f\n", frq, 20 * log10((double)(max[0] - min[0]) / 60000), 20 * log10((double)(max[1] - min[1]) / 60000));
// printf("Frq=%7.1f dB: Sin=%6.3f Des=%6.3f\n", frq, 20 * log10((double)(max[0] - min[0]) / 60000), 20 * log10((double)(max[1] - min[1]) / 60000));
// printf("Frq=%7.1f min=%6d max=%6d dB: Sin=%6.3f Des=%6.3f\n", frq, min, max, 20 * log10((double)(max[0] - min[0]) / 60000), 20 * log10((double)(max[1] - min[1]) / 60000));
}
close(f);
/* snd_pcm_drop(pcm_handle_out);
snd_pcm_drop(pcm_handle_in);*/
snd_pcm_close(pcm_handle_out);
snd_pcm_close(pcm_handle_in);
return (0);
}
^ permalink raw reply [flat|nested] 19+ messages in thread* Re: Kernel crash 2004-09-05 18:51 Kernel crash Giuliano Pochini @ 2004-09-06 15:16 ` Takashi Iwai 2004-09-07 7:55 ` Giuliano Pochini 0 siblings, 1 reply; 19+ messages in thread From: Takashi Iwai @ 2004-09-06 15:16 UTC (permalink / raw) To: Giuliano Pochini; +Cc: Alsa-devel At Sun, 5 Sep 2004 20:51:22 +0200, Giuliano Pochini wrote: > > 10 days passed since the last time I wrote about this problem... but the > problems is still here. Perhaps I didn't explain clearly enough. The program > I attached was buggy, but I meant it makes the kernel crash, not the program > itself. It's attached again to this message. No need to be root. IMHO it's a > quite serious issue the should be fixed ASAP. > > It successfully crashes my system after a few attempts: > > Linux Jay 2.6.8.1 #2 SMP Wed Aug 18 15:14:51 CEST 2004 ppc unknown > booted with maxcpus=1 and without preempt. > alsa-driver-1.0.6a and CVS-2004-09-05 > alsa-lib-1.0.4 > gcc 3.4.1 > > with both the powermac (Snapper) driver and my Echoaudio driver. It doesn't crash on my system with SB live with the latest CVS version but segfaults at line 263 because c is bogus. The crash looks likely driver-specific, then. Did you try a UP kernel? Takashi ------------------------------------------------------- This SF.Net email is sponsored by BEA Weblogic Workshop FREE Java Enterprise J2EE developer tools! Get your free copy of BEA WebLogic Workshop 8.1 today. http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: Kernel crash 2004-09-06 15:16 ` Takashi Iwai @ 2004-09-07 7:55 ` Giuliano Pochini 2004-09-11 19:02 ` [PATCH] " Giuliano Pochini 0 siblings, 1 reply; 19+ messages in thread From: Giuliano Pochini @ 2004-09-07 7:55 UTC (permalink / raw) To: Takashi Iwai; +Cc: Alsa-devel On 06-Sep-2004 Takashi Iwai wrote: >> It successfully crashes my system after a few attempts: >> >> Linux Jay 2.6.8.1 #2 SMP Wed Aug 18 15:14:51 CEST 2004 ppc unknown >> booted with maxcpus=1 and without preempt. >> alsa-driver-1.0.6a and CVS-2004-09-05 >> alsa-lib-1.0.4 >> gcc 3.4.1 >> >> with both the powermac (Snapper) driver and my Echoaudio driver. > > It doesn't crash on my system with SB live with the latest CVS > version but segfaults at line 263 because c is bogus. > The crash looks likely driver-specific, then. No, I don't think so. I already found (one of?) the cause, but I forgot to mention it again, sorry. It happens because ALSA calls the .free() pcm callback without calling .trigger(STOP) and .close() first. Jaroslav posted a patch which didn't work, then the discussion stopped. The problem occurs only when the substreams are linked and the userpace app is badly broken like mine. > Did you try a UP kernel? Just tried. Same thing with Echo driver, but the Pmac driver doen't seem to crash anymore. And about the PC I don't know if that kernel is MP of UP. -- Giuliano. ------------------------------------------------------- This SF.Net email is sponsored by BEA Weblogic Workshop FREE Java Enterprise J2EE developer tools! Get your free copy of BEA WebLogic Workshop 8.1 today. http://ads.osdn.com/?ad_id=5047&alloc_id=10808&op=click ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-07 7:55 ` Giuliano Pochini @ 2004-09-11 19:02 ` Giuliano Pochini 2004-09-13 15:00 ` Takashi Iwai 0 siblings, 1 reply; 19+ messages in thread From: Giuliano Pochini @ 2004-09-11 19:02 UTC (permalink / raw) To: tiwai; +Cc: alsa-devel On Tue, 07 Sep 2004 09:55:02 +0200 (CEST) Giuliano Pochini <pochini@shiny.it> wrote: > On 06-Sep-2004 Takashi Iwai wrote: > > >> It successfully crashes my system after a few attempts: > >> > >> Linux Jay 2.6.8.1 #2 SMP Wed Aug 18 15:14:51 CEST 2004 ppc unknown > >> booted with maxcpus=1 and without preempt. > >> alsa-driver-1.0.6a and CVS-2004-09-05 > >> alsa-lib-1.0.4 > >> gcc 3.4.1 > >> > >> with both the powermac (Snapper) driver and my Echoaudio driver. > > > > It doesn't crash on my system with SB live with the latest CVS > > version but segfaults at line 263 because c is bogus. > > The crash looks likely driver-specific, then. > > No, I don't think so. I already found (one of?) the cause, but > I forgot to mention it again, sorry. It happens because ALSA > calls the .free() pcm callback without calling .trigger(STOP) > and .close() first. Jaroslav posted a patch which didn't work, > then the discussion stopped. The problem occurs only when the > substreams are linked and the userpace app is badly broken > like mine. I added some code in the hw_free callback to avoid the crash and some debug lines in pcm_native.c and I got this trace: pcm_trigger start [...] pcm_native.c:1241: snd_pcm_playback_drain (state=RUNNING) pcm_native.c:1294: snd_pcm_playback_drain not empty pcm_native.c:1204: snd_pcm_change_state(DRAINING) pcm_native.c:1457: snd_pcm_capture_drop (state=DRAINING) pcm_native.c:1204: snd_pcm_change_state(SETUP) This changes the state of both substreams to SETUP, so snd_pcm_playback/capture_drain/drop() functions will not call _stop() anymore. pcm_native.c:1359: snd_pcm_playback_drop (state=SETUP) pcm_hw_free(play) <--- BUG! pcm_close pcm_native.c:snd_pcm_capture_drop (state=SETUP) pcm_hw_free(capt) pcm_close I developed this patch, but I'm not sure it's ok. The first part probably is not required but it seems more correct IMHO. --- alsa-kernel/core/pcm_native.c_orig Sat Sep 11 15:29:51 2004 +++ alsa-kernel/core/pcm_native.c Sat Sep 11 20:54:32 2004 @@ -1280,7 +1280,8 @@ break; } - if (runtime->status->state == SNDRV_PCM_STATE_RUNNING) { + if (runtime->status->state == SNDRV_PCM_STATE_RUNNING || + runtime->status->state == SNDRV_PCM_STATE_DRAINING) { if (snd_pcm_playback_empty(substream)) { snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); goto _end; @@ -1467,7 +1468,10 @@ case SNDRV_PCM_STATE_PREPARED: case SNDRV_PCM_STATE_DRAINING: case SNDRV_PCM_STATE_XRUN: - snd_pcm_change_state(substream, SNDRV_PCM_STATE_SETUP); + if (snd_pcm_update_hw_ptr(substream) >= 0) + snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); + else + snd_pcm_change_state(substream, SNDRV_PCM_STATE_SETUP); break; default: break; -- Giuliano. ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 13. Go here: http://sf.net/ppc_contest.php ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-11 19:02 ` [PATCH] " Giuliano Pochini @ 2004-09-13 15:00 ` Takashi Iwai 2004-09-13 20:07 ` Giuliano Pochini 0 siblings, 1 reply; 19+ messages in thread From: Takashi Iwai @ 2004-09-13 15:00 UTC (permalink / raw) To: Giuliano Pochini; +Cc: alsa-devel At Sat, 11 Sep 2004 21:02:12 +0200, Giuliano Pochini wrote: > > On Tue, 07 Sep 2004 09:55:02 +0200 (CEST) > Giuliano Pochini <pochini@shiny.it> wrote: > > > On 06-Sep-2004 Takashi Iwai wrote: > > > > >> It successfully crashes my system after a few attempts: > > >> > > >> Linux Jay 2.6.8.1 #2 SMP Wed Aug 18 15:14:51 CEST 2004 ppc unknown > > >> booted with maxcpus=1 and without preempt. > > >> alsa-driver-1.0.6a and CVS-2004-09-05 > > >> alsa-lib-1.0.4 > > >> gcc 3.4.1 > > >> > > >> with both the powermac (Snapper) driver and my Echoaudio driver. > > > > > > It doesn't crash on my system with SB live with the latest CVS > > > version but segfaults at line 263 because c is bogus. > > > The crash looks likely driver-specific, then. > > > > No, I don't think so. I already found (one of?) the cause, but > > I forgot to mention it again, sorry. It happens because ALSA > > calls the .free() pcm callback without calling .trigger(STOP) > > and .close() first. Jaroslav posted a patch which didn't work, > > then the discussion stopped. The problem occurs only when the > > substreams are linked and the userpace app is badly broken > > like mine. > > I added some code in the hw_free callback to avoid the crash and some debug > lines in pcm_native.c and I got this trace: > > pcm_trigger start > [...] > pcm_native.c:1241: snd_pcm_playback_drain (state=RUNNING) > pcm_native.c:1294: snd_pcm_playback_drain not empty > pcm_native.c:1204: snd_pcm_change_state(DRAINING) > pcm_native.c:1457: snd_pcm_capture_drop (state=DRAINING) > pcm_native.c:1204: snd_pcm_change_state(SETUP) > > This changes the state of both substreams to SETUP, so > snd_pcm_playback/capture_drain/drop() functions will not call _stop() > anymore. > > pcm_native.c:1359: snd_pcm_playback_drop (state=SETUP) > pcm_hw_free(play) <--- BUG! > pcm_close > pcm_native.c:snd_pcm_capture_drop (state=SETUP) > pcm_hw_free(capt) > pcm_close I see. > I developed this patch, but I'm not sure it's ok. The first part probably is > not required but it seems more correct IMHO. The first one should be ok. I'm also not 100% sure about the second one after the short glance. The problem is that we have no DMA running state in runtime struct. DMA is supposed to be running in DRAINING state for playback, while it's to be stopped in DRAINING capture. Takashi > > > --- alsa-kernel/core/pcm_native.c_orig Sat Sep 11 15:29:51 2004 > +++ alsa-kernel/core/pcm_native.c Sat Sep 11 20:54:32 2004 > @@ -1280,7 +1280,8 @@ > break; > } > > - if (runtime->status->state == SNDRV_PCM_STATE_RUNNING) { > + if (runtime->status->state == SNDRV_PCM_STATE_RUNNING || > + runtime->status->state == SNDRV_PCM_STATE_DRAINING) { > if (snd_pcm_playback_empty(substream)) { > snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); > goto _end; > @@ -1467,7 +1468,10 @@ > case SNDRV_PCM_STATE_PREPARED: > case SNDRV_PCM_STATE_DRAINING: > case SNDRV_PCM_STATE_XRUN: > - snd_pcm_change_state(substream, SNDRV_PCM_STATE_SETUP); > + if (snd_pcm_update_hw_ptr(substream) >= 0) > + snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); > + else > + snd_pcm_change_state(substream, SNDRV_PCM_STATE_SETUP); > break; > default: > break; > > > > -- > Giuliano. > ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 13. Go here: http://sf.net/ppc_contest.php ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-13 15:00 ` Takashi Iwai @ 2004-09-13 20:07 ` Giuliano Pochini 2004-09-15 10:24 ` Takashi Iwai 0 siblings, 1 reply; 19+ messages in thread From: Giuliano Pochini @ 2004-09-13 20:07 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel On Mon, 13 Sep 2004 17:00:21 +0200 Takashi Iwai <tiwai@suse.de> wrote: > > I added some code in the hw_free callback to avoid the crash and some debug > > lines in pcm_native.c and I got this trace: > > > > pcm_trigger start > > [...] > > pcm_native.c:1241: snd_pcm_playback_drain (state=RUNNING) > > pcm_native.c:1294: snd_pcm_playback_drain not empty > > pcm_native.c:1204: snd_pcm_change_state(DRAINING) > > pcm_native.c:1457: snd_pcm_capture_drop (state=DRAINING) > > pcm_native.c:1204: snd_pcm_change_state(SETUP) > > > > This changes the state of both substreams to SETUP, so > > snd_pcm_playback/capture_drain/drop() functions will not call _stop() > > anymore. > > > > pcm_native.c:1359: snd_pcm_playback_drop (state=SETUP) > > pcm_hw_free(play) <--- BUG! > > pcm_close > > pcm_native.c:snd_pcm_capture_drop (state=SETUP) > > pcm_hw_free(capt) > > pcm_close > > I see. > > > I developed this patch, but I'm not sure it's ok. The first part probably is > > not required but it seems more correct IMHO. > > The first one should be ok. > I'm also not 100% sure about the second one after the short glance. > > The problem is that we have no DMA running state in runtime struct. > DMA is supposed to be running in DRAINING state for playback, while > it's to be stopped in DRAINING capture. I guessed the DMA is going on in RUNNING and DRAINING states only. Actually I just noticed that my patch is wrong: in PREPARED state DMA is off and snd_pcm_stop() should not be called if state is SNDRV_PCM_STATE_XRUN because snd_pcm_xrun() already stopped it. (compiled and tested ok) --- pcm_native.c_orig Sat Sep 11 15:29:51 2004 +++ pcm_native.c Mon Sep 13 21:51:05 2004 @@ -1280,7 +1280,8 @@ break; } - if (runtime->status->state == SNDRV_PCM_STATE_RUNNING) { + if (runtime->status->state == SNDRV_PCM_STATE_RUNNING || + runtime->status->state == SNDRV_PCM_STATE_DRAINING) { if (snd_pcm_playback_empty(substream)) { snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); goto _end; @@ -1464,9 +1465,14 @@ if (res < 0) goto _end; /* Fall through */ - case SNDRV_PCM_STATE_PREPARED: case SNDRV_PCM_STATE_DRAINING: + if (snd_pcm_update_hw_ptr(substream) >= 0) { + snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); + break; + } + /* Fall through */ case SNDRV_PCM_STATE_XRUN: + case SNDRV_PCM_STATE_PREPARED: snd_pcm_change_state(substream, SNDRV_PCM_STATE_SETUP); break; default: -- Giuliano. ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 13. Go here: http://sf.net/ppc_contest.php ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-13 20:07 ` Giuliano Pochini @ 2004-09-15 10:24 ` Takashi Iwai 2004-09-15 17:50 ` Giuliano Pochini 0 siblings, 1 reply; 19+ messages in thread From: Takashi Iwai @ 2004-09-15 10:24 UTC (permalink / raw) To: Giuliano Pochini; +Cc: alsa-devel At Mon, 13 Sep 2004 22:07:29 +0200, Giuliano Pochini wrote: > > On Mon, 13 Sep 2004 17:00:21 +0200 > Takashi Iwai <tiwai@suse.de> wrote: > > > > I added some code in the hw_free callback to avoid the crash and some debug > > > lines in pcm_native.c and I got this trace: > > > > > > pcm_trigger start > > > [...] > > > pcm_native.c:1241: snd_pcm_playback_drain (state=RUNNING) > > > pcm_native.c:1294: snd_pcm_playback_drain not empty > > > pcm_native.c:1204: snd_pcm_change_state(DRAINING) > > > pcm_native.c:1457: snd_pcm_capture_drop (state=DRAINING) > > > pcm_native.c:1204: snd_pcm_change_state(SETUP) > > > > > > This changes the state of both substreams to SETUP, so > > > snd_pcm_playback/capture_drain/drop() functions will not call _stop() > > > anymore. > > > > > > pcm_native.c:1359: snd_pcm_playback_drop (state=SETUP) > > > pcm_hw_free(play) <--- BUG! > > > pcm_close > > > pcm_native.c:snd_pcm_capture_drop (state=SETUP) > > > pcm_hw_free(capt) > > > pcm_close > > > > I see. > > > > > I developed this patch, but I'm not sure it's ok. The first part probably is > > > not required but it seems more correct IMHO. > > > > The first one should be ok. > > I'm also not 100% sure about the second one after the short glance. > > > > The problem is that we have no DMA running state in runtime struct. > > DMA is supposed to be running in DRAINING state for playback, while > > it's to be stopped in DRAINING capture. > > I guessed the DMA is going on in RUNNING and DRAINING states only. Actually > I just noticed that my patch is wrong: in PREPARED state DMA is off and > snd_pcm_stop() should not be called if state is SNDRV_PCM_STATE_XRUN because > snd_pcm_xrun() already stopped it. > > (compiled and tested ok) I believe the second part is not needed. DRAINING in capture implies that the DMA is already stopped (like XRUN). If the second part really fixes the problem, the bug is in somewhere else. Takashi > > > --- pcm_native.c_orig Sat Sep 11 15:29:51 2004 > +++ pcm_native.c Mon Sep 13 21:51:05 2004 > @@ -1280,7 +1280,8 @@ > break; > } > > - if (runtime->status->state == SNDRV_PCM_STATE_RUNNING) { > + if (runtime->status->state == SNDRV_PCM_STATE_RUNNING || > + runtime->status->state == SNDRV_PCM_STATE_DRAINING) { > if (snd_pcm_playback_empty(substream)) { > snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); > goto _end; > @@ -1464,9 +1465,14 @@ > if (res < 0) > goto _end; > /* Fall through */ > - case SNDRV_PCM_STATE_PREPARED: > case SNDRV_PCM_STATE_DRAINING: > + if (snd_pcm_update_hw_ptr(substream) >= 0) { > + snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); > + break; > + } > + /* Fall through */ > case SNDRV_PCM_STATE_XRUN: > + case SNDRV_PCM_STATE_PREPARED: > snd_pcm_change_state(substream, SNDRV_PCM_STATE_SETUP); > break; > default: > > > > -- > Giuliano. > > > ------------------------------------------------------- > This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 > Project Admins to receive an Apple iPod Mini FREE for your judgement on > who ports your project to Linux PPC the best. Sponsored by IBM. > Deadline: Sept. 13. Go here: http://sf.net/ppc_contest.php > _______________________________________________ > Alsa-devel mailing list > Alsa-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/alsa-devel > ------------------------------------------------------- This SF.Net email is sponsored by: thawte's Crypto Challenge Vl Crack the code and win a Sony DCRHC40 MiniDV Digital Handycam Camcorder. More prizes in the weekly Lunch Hour Challenge. Sign up NOW http://ad.doubleclick.net/clk;10740251;10262165;m ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-15 10:24 ` Takashi Iwai @ 2004-09-15 17:50 ` Giuliano Pochini 2004-09-15 18:01 ` Takashi Iwai 2004-09-17 6:54 ` Jaroslav Kysela 0 siblings, 2 replies; 19+ messages in thread From: Giuliano Pochini @ 2004-09-15 17:50 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel On Wed, 15 Sep 2004 12:24:03 +0200 Takashi Iwai <tiwai@suse.de> wrote: > I believe the second part is not needed. DRAINING in capture implies > that the DMA is already stopped (like XRUN). snd_pcm_capture_drain() says: case SNDRV_PCM_STATE_RUNNING: if (snd_pcm_update_hw_ptr(substream) >= 0) { snd_pcm_stop(substream, snd_pcm_capture_avail(runtime) > 0 ? SNDRV_PCM_STATE_DRAINING : SNDRV_PCM_STATE_SETUP); break; } /* Fall through */ case SNDRV_PCM_STATE_XRUN: _xrun_recovery: snd_pcm_change_state(substream, snd_pcm_capture_avail(runtime) > 0 ? SNDRV_PCM_STATE_DRAINING : SNDRV_PCM_STATE_SETUP); break; It may not be stopped when snd_pcm_update_hw_ptr()<0, if I'm not missing something. > If the second part really fixes the problem, the bug is in somewhere > else. It fixes it and yes, there is another problem: snd_pcm_change_state() overrides the state machine. snd_pcm_playback_drain() (when state=RUNNING) calls snd_pcm_change_state(DRAINING). When substreams are linked, it changes the state of all linked substreams *without* passing through snd_pcm_capture_drain(DRAINING). I checked all calls to snd_pcm_change_state() and the only problematic one is at line 1288 inside snd_pcm_capture_drain(). The problem exists only if capture and playback substreams are linked because a capture substream in DRAINING state implies it's stopped, while a playback substream doesn't. My patch is not the proper fix, but it workarounds the bug by fixing that asymmetry. I make snd_pcm_capture_drop() and snd_pcm_playback_drop() behave the same wrt DRAINING substreams. -- Giuliano. ------------------------------------------------------- This SF.Net email is sponsored by: thawte's Crypto Challenge Vl Crack the code and win a Sony DCRHC40 MiniDV Digital Handycam Camcorder. More prizes in the weekly Lunch Hour Challenge. Sign up NOW http://ad.doubleclick.net/clk;10740251;10262165;m ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-15 17:50 ` Giuliano Pochini @ 2004-09-15 18:01 ` Takashi Iwai 2004-09-16 11:16 ` Takashi Iwai 2004-09-17 6:54 ` Jaroslav Kysela 1 sibling, 1 reply; 19+ messages in thread From: Takashi Iwai @ 2004-09-15 18:01 UTC (permalink / raw) To: Giuliano Pochini; +Cc: alsa-devel At Wed, 15 Sep 2004 19:50:08 +0200, Giuliano Pochini wrote: > > On Wed, 15 Sep 2004 12:24:03 +0200 > Takashi Iwai <tiwai@suse.de> wrote: > > > I believe the second part is not needed. DRAINING in capture implies > > that the DMA is already stopped (like XRUN). > > snd_pcm_capture_drain() says: > > case SNDRV_PCM_STATE_RUNNING: > if (snd_pcm_update_hw_ptr(substream) >= 0) { > snd_pcm_stop(substream, > snd_pcm_capture_avail(runtime) > 0 ? > SNDRV_PCM_STATE_DRAINING : SNDRV_PCM_STATE_SETUP); > break; > } > /* Fall through */ > case SNDRV_PCM_STATE_XRUN: > _xrun_recovery: > snd_pcm_change_state(substream, > snd_pcm_capture_avail(runtime) > 0 ? > SNDRV_PCM_STATE_DRAINING : SNDRV_PCM_STATE_SETUP); > break; > > It may not be stopped when snd_pcm_update_hw_ptr()<0, if I'm not missing > something. snd_pcm_update_hw_ptr() returns a negative value only when xrun happens or the drain is finished. In both cases, snd_pcm_stop() is called internally before returning from this function. > > If the second part really fixes the problem, the bug is in somewhere > > else. > > It fixes it and yes, there is another problem: snd_pcm_change_state() > overrides the state machine. > > snd_pcm_playback_drain() (when state=RUNNING) calls > snd_pcm_change_state(DRAINING). When substreams are linked, it changes the > state of all linked substreams *without* passing through > snd_pcm_capture_drain(DRAINING). > > I checked all calls to snd_pcm_change_state() and the only problematic one > is at line 1288 inside snd_pcm_capture_drain(). The problem exists only if > capture and playback substreams are linked because a capture substream in > DRAINING state implies it's stopped, while a playback substream doesn't. My > patch is not the proper fix, but it workarounds the bug by fixing that > asymmetry. I make snd_pcm_capture_drop() and snd_pcm_playback_drop() behave > the same wrt DRAINING substreams. I think it'd be better to add a new field to substream to indicate the DMA running status so that snd_pcm_stop() can be called safely regardless of the current status. The situation right now is a mess, the DMA running state depending on the stream direction and the state. Takashi ------------------------------------------------------- This SF.Net email is sponsored by: thawte's Crypto Challenge Vl Crack the code and win a Sony DCRHC40 MiniDV Digital Handycam Camcorder. More prizes in the weekly Lunch Hour Challenge. Sign up NOW http://ad.doubleclick.net/clk;10740251;10262165;m ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-15 18:01 ` Takashi Iwai @ 2004-09-16 11:16 ` Takashi Iwai 2004-09-16 19:56 ` Giuliano Pochini 0 siblings, 1 reply; 19+ messages in thread From: Takashi Iwai @ 2004-09-16 11:16 UTC (permalink / raw) To: Giuliano Pochini; +Cc: alsa-devel [-- Attachment #1: Type: text/plain, Size: 700 bytes --] At Wed, 15 Sep 2004 20:01:51 +0200, I wrote: > > I think it'd be better to add a new field to substream to indicate the > DMA running status so that snd_pcm_stop() can be called safely > regardless of the current status. > > The situation right now is a mess, the DMA running state depending on > the stream direction and the state. We have a snd_pcm_running() so the flag is not always necessary (although it would be nice). In the current code, the PCM state is changed somehow unconditionally among all linked substreams via snd_pcm_change_state(), and apparently this causes the confusion. I did clean up and rewrite of pcm core stuff. Could you check the attached patch? thanks, Takashi [-- Attachment #2: Type: text/plain, Size: 31678 bytes --] Index: alsa-kernel/core/pcm_lib.c =================================================================== RCS file: /home/tiwai/cvs/alsa/alsa-kernel/core/pcm_lib.c,v retrieving revision 1.56 diff -u -r1.56 pcm_lib.c --- alsa-kernel/core/pcm_lib.c 27 Jul 2004 11:20:08 -0000 1.56 +++ alsa-kernel/core/pcm_lib.c 15 Sep 2004 23:04:17 -0000 @@ -176,7 +176,7 @@ runtime->avail_max = avail; if (avail >= runtime->stop_threshold) { if (substream->runtime->status->state == SNDRV_PCM_STATE_DRAINING) - snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); + snd_pcm_drain_done(substream); else xrun(substream); return -EPIPE; Index: alsa-kernel/core/pcm_native.c =================================================================== RCS file: /home/tiwai/cvs/alsa/alsa-kernel/core/pcm_native.c,v retrieving revision 1.83 diff -u -r1.83 pcm_native.c --- alsa-kernel/core/pcm_native.c 15 Sep 2004 09:04:26 -0000 1.83 +++ alsa-kernel/core/pcm_native.c 16 Sep 2004 00:49:49 -0000 @@ -451,7 +451,7 @@ static int snd_pcm_hw_free(snd_pcm_substream_t * substream) { snd_pcm_runtime_t *runtime; - int result; + int result = 0; snd_assert(substream != NULL, return -ENXIO); runtime = substream->runtime; @@ -468,11 +468,8 @@ snd_pcm_stream_unlock_irq(substream); if (atomic_read(&runtime->mmap_count)) return -EBADFD; - if (substream->ops->hw_free == NULL) { - runtime->status->state = SNDRV_PCM_STATE_OPEN; - return 0; - } - result = substream->ops->hw_free(substream); + if (substream->ops->hw_free) + result = substream->ops->hw_free(substream); runtime->status->state = SNDRV_PCM_STATE_OPEN; return result; } @@ -652,6 +649,7 @@ struct action_ops { int (*pre_action)(snd_pcm_substream_t *substream, int state); int (*do_action)(snd_pcm_substream_t *substream, int state); + void (*undo_action)(snd_pcm_substream_t *substream, int state); void (*post_action)(snd_pcm_substream_t *substream, int state); }; @@ -666,7 +664,8 @@ { struct list_head *pos; snd_pcm_substream_t *s = NULL; - int err, res = 0; + snd_pcm_substream_t *s1; + int res = 0; snd_pcm_group_for_each(pos, substream) { s = snd_pcm_group_substream_entry(pos); @@ -674,24 +673,31 @@ spin_lock(&s->self_group.lock); res = ops->pre_action(s, state); if (res < 0) - break; + goto _unlock; } - if (res >= 0) { - snd_pcm_group_for_each(pos, substream) { - s = snd_pcm_group_substream_entry(pos); - err = ops->do_action(s, state); - if (err < 0) { - if (res == 0) - res = err; - } else { - ops->post_action(s, state); + snd_pcm_group_for_each(pos, substream) { + s = snd_pcm_group_substream_entry(pos); + res = ops->do_action(s, state); + if (res < 0) { + if (ops->undo_action) { + snd_pcm_group_for_each(pos, substream) { + s1 = snd_pcm_group_substream_entry(pos); + if (s1 == s) /* failed stream */ + break; + ops->undo_action(s1, state); + } } - if (do_lock && s != substream) - spin_unlock(&s->self_group.lock); + s = NULL; /* unlock all */ + goto _unlock; } - } else if (do_lock) { - snd_pcm_substream_t *s1; - /* unlock all streams */ + } + snd_pcm_group_for_each(pos, substream) { + s = snd_pcm_group_substream_entry(pos); + ops->post_action(s, state); + } + _unlock: + if (do_lock) { + /* unlock streams */ snd_pcm_group_for_each(pos, substream) { s1 = snd_pcm_group_substream_entry(pos); if (s1 != substream) @@ -716,9 +722,10 @@ if (res < 0) return res; res = ops->do_action(substream, state); - if (res == 0) { + if (res == 0) ops->post_action(substream, state); - } + else if (ops->undo_action) + ops->undo_action(substream, state); return res; } @@ -787,6 +794,9 @@ return res; } +/* + * start callbacks + */ static int snd_pcm_pre_start(snd_pcm_substream_t *substream, int state) { snd_pcm_runtime_t *runtime = substream->runtime; @@ -803,14 +813,20 @@ { if (substream->runtime->trigger_master != substream) return 0; - return substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_START); + return substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_START); +} + +static void snd_pcm_undo_start(snd_pcm_substream_t *substream, int state) +{ + if (substream->runtime->trigger_master == substream) + substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_STOP); } static void snd_pcm_post_start(snd_pcm_substream_t *substream, int state) { snd_pcm_runtime_t *runtime = substream->runtime; snd_pcm_trigger_tstamp(substream); - runtime->status->state = SNDRV_PCM_STATE_RUNNING; + runtime->status->state = state; if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK && runtime->silence_size > 0) snd_pcm_playback_silence(substream, ULONG_MAX); @@ -823,22 +839,27 @@ static struct action_ops snd_pcm_action_start = { .pre_action = snd_pcm_pre_start, .do_action = snd_pcm_do_start, + .undo_action = snd_pcm_undo_start, .post_action = snd_pcm_post_start }; /** * snd_pcm_start + * + * Start all linked streams. */ int snd_pcm_start(snd_pcm_substream_t *substream) { - return snd_pcm_action(&snd_pcm_action_start, substream, 0); + return snd_pcm_action(&snd_pcm_action_start, substream, SNDRV_PCM_STATE_RUNNING); } +/* + * stop callbacks + */ static int snd_pcm_pre_stop(snd_pcm_substream_t *substream, int state) { snd_pcm_runtime_t *runtime = substream->runtime; - if (substream->runtime->status->state != SNDRV_PCM_STATE_RUNNING && - substream->runtime->status->state != SNDRV_PCM_STATE_DRAINING) + if (runtime->status->state == SNDRV_PCM_STATE_OPEN) return -EBADFD; runtime->trigger_master = substream; return 0; @@ -846,19 +867,22 @@ static int snd_pcm_do_stop(snd_pcm_substream_t *substream, int state) { - if (substream->runtime->trigger_master != substream) - return 0; - return substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_STOP); + if (substream->runtime->trigger_master == substream && + snd_pcm_running(substream)) + substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_STOP); + return 0; /* unconditonally stop all substreams */ } static void snd_pcm_post_stop(snd_pcm_substream_t *substream, int state) { snd_pcm_runtime_t *runtime = substream->runtime; - snd_pcm_trigger_tstamp(substream); - if (substream->timer) - snd_timer_notify(substream->timer, SNDRV_TIMER_EVENT_MSTOP, &runtime->trigger_tstamp); - runtime->status->state = state; - snd_pcm_tick_set(substream, 0); + if (runtime->status->state != state) { + snd_pcm_trigger_tstamp(substream); + if (substream->timer) + snd_timer_notify(substream->timer, SNDRV_TIMER_EVENT_MSTOP, &runtime->trigger_tstamp); + runtime->status->state = state; + snd_pcm_tick_set(substream, 0); + } wake_up(&runtime->sleep); } @@ -870,12 +894,30 @@ /** * snd_pcm_stop + * + * Try to stop all running streams in the substream group. + * The state of each stream is changed to the given value after that unconditionally. */ int snd_pcm_stop(snd_pcm_substream_t *substream, int state) { return snd_pcm_action(&snd_pcm_action_stop, substream, state); } +/** + * snd_pcm_drain_done + * + * Stop the DMA only when the given stream is playback. + * The state is changed to SETUP. + * Unlike snd_pcm_stop(), this affects only the given stream. + */ +int snd_pcm_drain_done(snd_pcm_substream_t *substream) +{ + return snd_pcm_action_single(&snd_pcm_action_stop, substream, SNDRV_PCM_STATE_SETUP); +} + +/* + * pause callbacks + */ static int snd_pcm_pre_pause(snd_pcm_substream_t *substream, int push) { snd_pcm_runtime_t *runtime = substream->runtime; @@ -899,6 +941,14 @@ SNDRV_PCM_TRIGGER_PAUSE_RELEASE); } +static void snd_pcm_undo_pause(snd_pcm_substream_t *substream, int push) +{ + if (substream->runtime->trigger_master == substream) + substream->ops->trigger(substream, + push ? SNDRV_PCM_TRIGGER_PAUSE_RELEASE : + SNDRV_PCM_TRIGGER_PAUSE_PUSH); +} + static void snd_pcm_post_pause(snd_pcm_substream_t *substream, int push) { snd_pcm_runtime_t *runtime = substream->runtime; @@ -921,9 +971,13 @@ static struct action_ops snd_pcm_action_pause = { .pre_action = snd_pcm_pre_pause, .do_action = snd_pcm_do_pause, + .undo_action = snd_pcm_undo_pause, .post_action = snd_pcm_post_pause }; +/* + * Push/release the pause for all linked streams. + */ static int snd_pcm_pause(snd_pcm_substream_t *substream, int push) { return snd_pcm_action(&snd_pcm_action_pause, substream, push); @@ -937,7 +991,6 @@ snd_pcm_runtime_t *runtime = substream->runtime; if (runtime->status->state == SNDRV_PCM_STATE_SUSPENDED) return -EBUSY; - runtime->status->suspended_state = runtime->status->state; runtime->trigger_master = substream; return 0; } @@ -947,10 +1000,10 @@ snd_pcm_runtime_t *runtime = substream->runtime; if (runtime->trigger_master != substream) return 0; - if (runtime->status->suspended_state != SNDRV_PCM_STATE_RUNNING && - runtime->status->suspended_state != SNDRV_PCM_STATE_DRAINING) + if (! snd_pcm_running(substream)) return 0; - return substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_SUSPEND); + substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_SUSPEND); + return 0; /* suspend unconditionally */ } static void snd_pcm_post_suspend(snd_pcm_substream_t *substream, int state) @@ -959,6 +1012,7 @@ snd_pcm_trigger_tstamp(substream); if (substream->timer) snd_timer_notify(substream->timer, SNDRV_TIMER_EVENT_MPAUSE, &runtime->trigger_tstamp); + runtime->status->suspended_state = runtime->status->state; runtime->status->state = SNDRV_PCM_STATE_SUSPENDED; snd_pcm_tick_set(substream, 0); wake_up(&runtime->sleep); @@ -972,6 +1026,9 @@ /** * snd_pcm_suspend + * + * Trigger SUSPEND to all linked streams. + * After this call, all streams are changed to SUSPENDED state. */ int snd_pcm_suspend(snd_pcm_substream_t *substream) { @@ -980,11 +1037,14 @@ /** * snd_pcm_suspend_all + * + * Trigger SUSPEND to all substreams in the given pcm. + * After this call, all streams are changed to SUSPENDED state. */ int snd_pcm_suspend_all(snd_pcm_t *pcm) { snd_pcm_substream_t *substream; - int stream, err; + int stream, err = 0; for (stream = 0; stream < 2; stream++) { for (substream = pcm->streams[stream].substream; substream; substream = substream->next) { @@ -992,15 +1052,11 @@ if (substream->runtime == NULL) continue; snd_pcm_stream_lock(substream); - if (substream->runtime->status->state == SNDRV_PCM_STATE_SUSPENDED) { - snd_pcm_stream_unlock(substream); - continue; - } - if ((err = snd_pcm_suspend(substream)) < 0) { - snd_pcm_stream_unlock(substream); - return err; - } + if (substream->runtime->status->state != SNDRV_PCM_STATE_SUSPENDED) + err = snd_pcm_suspend(substream); snd_pcm_stream_unlock(substream); + if (err < 0) + return err; } } return 0; @@ -1022,12 +1078,21 @@ snd_pcm_runtime_t *runtime = substream->runtime; if (runtime->trigger_master != substream) return 0; + /* DMA not running previously? */ if (runtime->status->suspended_state != SNDRV_PCM_STATE_RUNNING && - runtime->status->suspended_state != SNDRV_PCM_STATE_DRAINING) + (runtime->status->suspended_state != SNDRV_PCM_STATE_DRAINING || + substream->stream != SNDRV_PCM_STREAM_PLAYBACK)) return 0; return substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_RESUME); } +static void snd_pcm_undo_resume(snd_pcm_substream_t *substream, int state) +{ + if (substream->runtime->trigger_master == substream && + snd_pcm_running(substream)) + substream->ops->trigger(substream, SNDRV_PCM_TRIGGER_SUSPEND); +} + static void snd_pcm_post_resume(snd_pcm_substream_t *substream, int state) { snd_pcm_runtime_t *runtime = substream->runtime; @@ -1042,6 +1107,7 @@ static struct action_ops snd_pcm_action_resume = { .pre_action = snd_pcm_pre_resume, .do_action = snd_pcm_do_resume, + .undo_action = snd_pcm_undo_resume, .post_action = snd_pcm_post_resume }; @@ -1066,6 +1132,11 @@ #endif /* CONFIG_PM */ +/* + * xrun ioctl + * + * Change the RUNNING stream(s) to XRUN state. + */ static int snd_pcm_xrun(snd_pcm_substream_t *substream) { snd_card_t *card = substream->pcm->card; @@ -1073,8 +1144,13 @@ int result; snd_power_lock(card); + if (runtime->status->state == SNDRV_PCM_STATE_SUSPENDED) { + result = snd_power_wait(card, SNDRV_CTL_POWER_D0, substream->ffile); + if (result < 0) + goto _unlock; + } + snd_pcm_stream_lock_irq(substream); - _xrun_recovery: switch (runtime->status->state) { case SNDRV_PCM_STATE_XRUN: result = 0; /* already there */ @@ -1082,21 +1158,18 @@ case SNDRV_PCM_STATE_RUNNING: result = snd_pcm_stop(substream, SNDRV_PCM_STATE_XRUN); break; - case SNDRV_PCM_STATE_SUSPENDED: - snd_pcm_stream_unlock_irq(substream); - result = snd_power_wait(card, SNDRV_CTL_POWER_D0, substream->ffile); - snd_pcm_stream_lock_irq(substream); - if (result >= 0) - goto _xrun_recovery; - break; default: result = -EBADFD; } snd_pcm_stream_unlock_irq(substream); + _unlock: snd_power_unlock(card); return result; } +/* + * reset ioctl + */ static int snd_pcm_pre_reset(snd_pcm_substream_t * substream, int state) { snd_pcm_runtime_t *runtime = substream->runtime; @@ -1145,17 +1218,17 @@ return snd_pcm_action_nonatomic(&snd_pcm_action_reset, substream, 0); } +/* + * prepare ioctl + */ static int snd_pcm_pre_prepare(snd_pcm_substream_t * substream, int state) { snd_pcm_runtime_t *runtime = substream->runtime; - switch (runtime->status->state) { - case SNDRV_PCM_STATE_OPEN: + if (runtime->status->state == SNDRV_PCM_STATE_OPEN) return -EBADFD; - case SNDRV_PCM_STATE_RUNNING: + if (snd_pcm_running(substream)) return -EBUSY; - default: - return 0; - } + return 0; } static int snd_pcm_do_prepare(snd_pcm_substream_t * substream, int state) @@ -1195,111 +1268,147 @@ return res; } -static void snd_pcm_change_state(snd_pcm_substream_t *substream, int state) +/* + * drain ioctl + */ + +static int snd_pcm_pre_drain_init(snd_pcm_substream_t * substream, int state) { - struct list_head *pos; - snd_pcm_substream_t *s; + if (substream->ffile->f_flags & O_NONBLOCK) + return -EAGAIN; + substream->runtime->trigger_master = substream; + return 0; +} - if (snd_pcm_stream_linked(substream)) { - if (!spin_trylock(&substream->group->lock)) { - spin_unlock(&substream->self_group.lock); - spin_lock(&substream->group->lock); - spin_lock(&substream->self_group.lock); - } - snd_pcm_group_for_each(pos, substream) { - s = snd_pcm_group_substream_entry(pos); - if (s != substream) - spin_lock(&s->self_group.lock); - s->runtime->status->state = state; - if (s != substream) - spin_unlock(&s->self_group.lock); +static int snd_pcm_do_drain_init(snd_pcm_substream_t * substream, int state) +{ + snd_pcm_runtime_t *runtime = substream->runtime; + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) { + switch (runtime->status->state) { + case SNDRV_PCM_STATE_PREPARED: + /* start playback stream if possible */ + if (! snd_pcm_playback_empty(substream)) { + snd_pcm_do_start(substream, SNDRV_PCM_STATE_DRAINING); + snd_pcm_post_start(substream, SNDRV_PCM_STATE_DRAINING); + } + break; + case SNDRV_PCM_STATE_RUNNING: + runtime->status->state = SNDRV_PCM_STATE_DRAINING; + break; + default: + break; } - spin_unlock(&substream->group->lock); } else { - substream->runtime->status->state = state; + /* stop running stream */ + if (runtime->status->state == SNDRV_PCM_STATE_RUNNING) { + int state = snd_pcm_capture_avail(runtime) > 0 ? + SNDRV_PCM_STATE_DRAINING : SNDRV_PCM_STATE_SETUP; + snd_pcm_do_stop(substream, state); + snd_pcm_post_stop(substream, state); + } } + return 0; +} + +static void snd_pcm_post_drain_init(snd_pcm_substream_t * substream, int state) +{ } -static int snd_pcm_playback_drop(snd_pcm_substream_t *substream); +static struct action_ops snd_pcm_action_drain_init = { + .pre_action = snd_pcm_pre_drain_init, + .do_action = snd_pcm_do_drain_init, + .post_action = snd_pcm_post_drain_init +}; -static int snd_pcm_playback_drain(snd_pcm_substream_t * substream) +struct drain_rec { + snd_pcm_substream_t *substream; + wait_queue_t wait; + snd_pcm_uframes_t stop_threshold; +}; + +static int snd_pcm_drop(snd_pcm_substream_t *substream); + +/* + * Drain the stream(s). + * When the substream is linked, sync until the draining of all playback streams + * is finished. + * After this call, all streams are supposed to be either SETUP or DRAINING + * (capture only) state. + */ +static int snd_pcm_drain(snd_pcm_substream_t *substream) { snd_card_t *card; snd_pcm_runtime_t *runtime; - int err, result = 0; - wait_queue_t wait; - enum { READY, EXPIRED, SUSPENDED, SIGNALED } state = READY; - snd_pcm_uframes_t stop_threshold; + struct list_head *pos; + int result = 0; + int i, num_drecs; + struct drain_rec *drec, drec_tmp, *d; snd_assert(substream != NULL, return -ENXIO); - snd_assert(substream->stream == SNDRV_PCM_STREAM_PLAYBACK, return -EINVAL); - runtime = substream->runtime; card = substream->pcm->card; + runtime = substream->runtime; + if (runtime->status->state == SNDRV_PCM_STATE_OPEN) + return -EBADFD; + + down_read(&snd_pcm_link_rwsem); snd_power_lock(card); - snd_pcm_stream_lock_irq(substream); + if (runtime->status->state == SNDRV_PCM_STATE_SUSPENDED) { + result = snd_power_wait(card, SNDRV_CTL_POWER_D0, substream->ffile); + if (result < 0) + goto _unlock; + } - /* stop_threshold fixup to avoid endless loop when */ - /* stop_threshold > buffer_size */ - stop_threshold = runtime->stop_threshold; - if (runtime->stop_threshold > runtime->buffer_size) - runtime->stop_threshold = runtime->buffer_size; + /* allocate temporary record for drain sync */ + if (snd_pcm_stream_linked(substream)) { + drec = kmalloc(substream->group->count * sizeof(*drec), GFP_KERNEL); + if (! drec) { + result = -ENOMEM; + goto _unlock; + } + } else + drec = &drec_tmp; - switch (runtime->status->state) { - case SNDRV_PCM_STATE_PAUSED: + snd_pcm_stream_lock_irq(substream); + /* resume pause */ + if (runtime->status->state == SNDRV_PCM_STATE_PAUSED) snd_pcm_pause(substream, 0); - /* Fall through */ - case SNDRV_PCM_STATE_RUNNING: - case SNDRV_PCM_STATE_DRAINING: - break; - case SNDRV_PCM_STATE_SUSPENDED: - snd_pcm_stream_unlock_irq(substream); - result = snd_power_wait(card, SNDRV_CTL_POWER_D0, substream->ffile); - snd_pcm_stream_lock_irq(substream); - if (result >= 0) - snd_pcm_change_state(substream, SNDRV_PCM_STATE_SETUP); - goto _end; - case SNDRV_PCM_STATE_OPEN: - result = -EBADFD; - goto _end; - case SNDRV_PCM_STATE_PREPARED: - if (!snd_pcm_playback_empty(substream)) { - err = snd_pcm_start(substream); - if (err < 0) { - result = err; - goto _end; - } - break; - } - /* Fall through */ - case SNDRV_PCM_STATE_XRUN: - snd_pcm_change_state(substream, SNDRV_PCM_STATE_SETUP); - /* Fall through */ - case SNDRV_PCM_STATE_SETUP: + + /* pre-start/stop - all running streams are changed to DRAINING state */ + result = snd_pcm_action(&snd_pcm_action_drain_init, substream, 0); + if (result < 0) goto _end; - default: - break; - } - if (runtime->status->state == SNDRV_PCM_STATE_RUNNING) { - if (snd_pcm_playback_empty(substream)) { - snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); - goto _end; + /* check streams with PLAYBACK & DRAINING */ + num_drecs = 0; + snd_pcm_group_for_each(pos, substream) { + snd_pcm_substream_t *s = snd_pcm_group_substream_entry(pos); + runtime = s->runtime; + if (runtime->status->state != SNDRV_PCM_STATE_DRAINING) { + runtime->status->state = SNDRV_PCM_STATE_SETUP; + continue; + } + if (s->stream == SNDRV_PCM_STREAM_PLAYBACK) { + d = &drec[num_drecs++]; + d->substream = s; + init_waitqueue_entry(&d->wait, current); + add_wait_queue(&runtime->sleep, &d->wait); + /* stop_threshold fixup to avoid endless loop when + * stop_threshold > buffer_size + */ + d->stop_threshold = runtime->stop_threshold; + if (runtime->stop_threshold > runtime->buffer_size) + runtime->stop_threshold = runtime->buffer_size; } - snd_pcm_change_state(substream, SNDRV_PCM_STATE_DRAINING); } - if (substream->ffile->f_flags & O_NONBLOCK) { - result = -EAGAIN; + if (! num_drecs) goto _end; - } - init_waitqueue_entry(&wait, current); - add_wait_queue(&runtime->sleep, &wait); - while (1) { + for (;;) { long tout; if (signal_pending(current)) { - state = SIGNALED; + result = -ERESTARTSYS; break; } set_current_state(TASK_INTERRUPTIBLE); @@ -1309,177 +1418,81 @@ snd_power_lock(card); snd_pcm_stream_lock_irq(substream); if (tout == 0) { - state = runtime->status->state == SNDRV_PCM_STATE_SUSPENDED ? SUSPENDED : EXPIRED; + if (substream->runtime->status->state == SNDRV_PCM_STATE_SUSPENDED) + result = -ESTRPIPE; + else { + snd_printd("playback drain error (DMA or IRQ trouble?)\n"); + snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); + result = -EIO; + } break; } - if (runtime->status->state != SNDRV_PCM_STATE_DRAINING) { - state = READY; - break; + /* all finished? */ + for (i = 0; i < num_drecs; i++) { + runtime = drec[i].substream->runtime; + if (runtime->status->state == SNDRV_PCM_STATE_DRAINING) + break; } + if (i == num_drecs) + break; } - remove_wait_queue(&runtime->sleep, &wait); - - switch (state) { - case SIGNALED: - result = -ERESTARTSYS; - goto _end; - case SUSPENDED: - result = -ESTRPIPE; - goto _end; - case EXPIRED: - snd_printd("playback drain error (DMA or IRQ trouble?)\n"); - result = -EIO; - goto _end; - default: - break; + for (i = 0; i < num_drecs; i++) { + d = &drec[i]; + runtime = d->substream->runtime; + remove_wait_queue(&runtime->sleep, &d->wait); + runtime->stop_threshold = d->stop_threshold; } - _end: - runtime->stop_threshold = stop_threshold; + _end: snd_pcm_stream_unlock_irq(substream); + if (drec && drec != &drec_tmp) + kfree(drec); + _unlock: snd_power_unlock(card); - if (state == EXPIRED) - snd_pcm_playback_drop(substream); + up_read(&snd_pcm_link_rwsem); return result; } -static int snd_pcm_playback_drop(snd_pcm_substream_t *substream) +/* + * drop ioctl + * + * Immediately put all linked substreams into SETUP state. + */ +static int snd_pcm_drop(snd_pcm_substream_t *substream) { - snd_pcm_runtime_t *runtime = substream->runtime; - snd_card_t *card = substream->pcm->card; - int res = 0; + snd_pcm_runtime_t *runtime; + snd_card_t *card; + int result = 0; - snd_power_lock(card); - snd_pcm_stream_lock_irq(substream); - switch (runtime->status->state) { - case SNDRV_PCM_STATE_OPEN: - res = -EBADFD; - break; - case SNDRV_PCM_STATE_SETUP: - break; - case SNDRV_PCM_STATE_PAUSED: - snd_pcm_pause(substream, 0); - /* Fall through */ - case SNDRV_PCM_STATE_RUNNING: - case SNDRV_PCM_STATE_DRAINING: - if (snd_pcm_update_hw_ptr(substream) >= 0) { - snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); - break; - } - /* Fall through */ - case SNDRV_PCM_STATE_PREPARED: - case SNDRV_PCM_STATE_XRUN: - snd_pcm_change_state(substream, SNDRV_PCM_STATE_SETUP); - break; - case SNDRV_PCM_STATE_SUSPENDED: - snd_pcm_stream_unlock_irq(substream); - res = snd_power_wait(card, SNDRV_CTL_POWER_D0, substream->ffile); - snd_pcm_stream_lock_irq(substream); - if (res >= 0) - snd_pcm_change_state(substream, SNDRV_PCM_STATE_SETUP); - break; - default: - break; - } - runtime->control->appl_ptr = runtime->status->hw_ptr; - snd_pcm_stream_unlock_irq(substream); - snd_power_unlock(card); - return res; -} + snd_assert(substream != NULL, return -ENXIO); + runtime = substream->runtime; + card = substream->pcm->card; -static int snd_pcm_capture_drain(snd_pcm_substream_t * substream) -{ - snd_pcm_runtime_t *runtime = substream->runtime; - snd_card_t *card = substream->pcm->card; - int res = 0; + if (runtime->status->state == SNDRV_PCM_STATE_OPEN) + return -EBADFD; snd_power_lock(card); - snd_pcm_stream_lock_irq(substream); - switch (runtime->status->state) { - case SNDRV_PCM_STATE_OPEN: - res = -EBADFD; - break; - case SNDRV_PCM_STATE_PREPARED: - snd_pcm_change_state(substream, SNDRV_PCM_STATE_SETUP); - break; - case SNDRV_PCM_STATE_SETUP: - case SNDRV_PCM_STATE_DRAINING: - break; - case SNDRV_PCM_STATE_PAUSED: - snd_pcm_pause(substream, 0); - /* Fall through */ - case SNDRV_PCM_STATE_RUNNING: - if (snd_pcm_update_hw_ptr(substream) >= 0) { - snd_pcm_stop(substream, - snd_pcm_capture_avail(runtime) > 0 ? - SNDRV_PCM_STATE_DRAINING : SNDRV_PCM_STATE_SETUP); - break; - } - /* Fall through */ - case SNDRV_PCM_STATE_XRUN: - _xrun_recovery: - snd_pcm_change_state(substream, - snd_pcm_capture_avail(runtime) > 0 ? - SNDRV_PCM_STATE_DRAINING : SNDRV_PCM_STATE_SETUP); - break; - case SNDRV_PCM_STATE_SUSPENDED: - snd_pcm_stream_unlock_irq(substream); - res = snd_power_wait(card, SNDRV_CTL_POWER_D0, substream->ffile); - snd_pcm_stream_lock_irq(substream); - if (res >= 0) - goto _xrun_recovery; - break; - default: - break; + if (runtime->status->state == SNDRV_PCM_STATE_SUSPENDED) { + result = snd_power_wait(card, SNDRV_CTL_POWER_D0, substream->ffile); + if (result < 0) + goto _unlock; } - snd_pcm_stream_unlock_irq(substream); - snd_power_unlock(card); - return res; -} -static int snd_pcm_capture_drop(snd_pcm_substream_t * substream) -{ - snd_pcm_runtime_t *runtime = substream->runtime; - snd_card_t *card = substream->pcm->card; - int res = 0; - - snd_power_lock(card); snd_pcm_stream_lock_irq(substream); - switch (runtime->status->state) { - case SNDRV_PCM_STATE_OPEN: - res = -EBADFD; - break; - case SNDRV_PCM_STATE_SETUP: - break; - case SNDRV_PCM_STATE_PAUSED: + /* resume pause */ + if (runtime->status->state == SNDRV_PCM_STATE_PAUSED) snd_pcm_pause(substream, 0); - /* Fall through */ - case SNDRV_PCM_STATE_RUNNING: - snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); - break; - case SNDRV_PCM_STATE_SUSPENDED: - snd_pcm_stream_unlock_irq(substream); - res = snd_power_wait(card, SNDRV_CTL_POWER_D0, substream->ffile); - snd_pcm_stream_lock_irq(substream); - if (res < 0) - goto _end; - /* Fall through */ - case SNDRV_PCM_STATE_PREPARED: - case SNDRV_PCM_STATE_DRAINING: - case SNDRV_PCM_STATE_XRUN: - snd_pcm_change_state(substream, SNDRV_PCM_STATE_SETUP); - break; - default: - break; - } - runtime->control->appl_ptr = runtime->status->hw_ptr; - _end: + + snd_pcm_stop(substream, SNDRV_PCM_STATE_SETUP); + /* runtime->control->appl_ptr = runtime->status->hw_ptr; */ snd_pcm_stream_unlock_irq(substream); + _unlock: snd_power_unlock(card); - return res; + return result; } + /* WARNING: Don't forget to fput back the file */ extern int snd_major; static struct file *snd_pcm_file_fd(int fd) @@ -1505,6 +1518,9 @@ return file; } +/* + * PCM link handling + */ static int snd_pcm_link(snd_pcm_substream_t *substream, int fd) { int res = 0; @@ -1512,12 +1528,6 @@ snd_pcm_file_t *pcm_file; snd_pcm_substream_t *substream1; - snd_pcm_stream_lock_irq(substream); - if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN) { - snd_pcm_stream_unlock_irq(substream); - return -EBADFD; - } - snd_pcm_stream_unlock_irq(substream); file = snd_pcm_file_fd(fd); if (!file) return -EBADFD; @@ -1525,7 +1535,8 @@ substream1 = pcm_file->substream; down_write(&snd_pcm_link_rwsem); write_lock_irq(&snd_pcm_link_rwlock); - if (substream->runtime->status->state != substream1->runtime->status->state) { + if (substream->runtime->status->state == SNDRV_PCM_STATE_OPEN || + substream->runtime->status->state != substream1->runtime->status->state) { res = -EBADFD; goto _end; } @@ -1542,8 +1553,10 @@ spin_lock_init(&substream->group->lock); INIT_LIST_HEAD(&substream->group->substreams); list_add_tail(&substream->link_list, &substream->group->substreams); + substream->group->count = 1; } list_add_tail(&substream1->link_list, &substream->group->substreams); + substream->group->count++; substream1->group = substream->group; _end: write_unlock_irq(&snd_pcm_link_rwlock); @@ -1562,7 +1575,7 @@ static int snd_pcm_unlink(snd_pcm_substream_t *substream) { struct list_head *pos; - int res = 0, count = 0; + int res = 0; down_write(&snd_pcm_link_rwsem); write_lock_irq(&snd_pcm_link_rwlock); @@ -1571,11 +1584,8 @@ goto _end; } list_del(&substream->link_list); - snd_pcm_group_for_each(pos, substream) { - if (++count > 1) - break; - } - if (count == 1) { /* detach the last stream, too */ + substream->group->count--; + if (substream->group->count == 1) { /* detach the last stream, too */ snd_pcm_group_for_each(pos, substream) { relink_to_local(snd_pcm_group_substream_entry(pos)); break; @@ -1589,6 +1599,9 @@ return res; } +/* + * hw configurator + */ static int snd_pcm_hw_rule_mul(snd_pcm_hw_params_t *params, snd_pcm_hw_rule_t *rule) { @@ -2077,10 +2090,7 @@ snd_assert(substream != NULL, return -ENXIO); snd_assert(!atomic_read(&substream->runtime->mmap_count), ); pcm = substream->pcm; - if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK) - snd_pcm_playback_drop(substream); - else - snd_pcm_capture_drop(substream); + snd_pcm_drop(substream); fasync_helper(-1, file, 0, &substream->runtime->fasync); down(&pcm->open_mutex); snd_pcm_release_file(pcm_file); @@ -2436,7 +2446,7 @@ case SNDRV_PCM_IOCTL_RESET: return snd_pcm_reset(substream); case SNDRV_PCM_IOCTL_START: - return snd_pcm_action_lock_irq(&snd_pcm_action_start, substream, 0); + return snd_pcm_action_lock_irq(&snd_pcm_action_start, substream, SNDRV_PCM_STATE_RUNNING); case SNDRV_PCM_IOCTL_LINK: return snd_pcm_link(substream, (int)(unsigned long) arg); case SNDRV_PCM_IOCTL_UNLINK: @@ -2455,6 +2465,10 @@ return snd_pcm_hw_refine_old_user(substream, arg); case SNDRV_PCM_IOCTL_HW_PARAMS_OLD: return snd_pcm_hw_params_old_user(substream, arg); + case SNDRV_PCM_IOCTL_DRAIN: + return snd_pcm_drain(substream); + case SNDRV_PCM_IOCTL_DROP: + return snd_pcm_drop(substream); } snd_printd("unknown ioctl = 0x%x\n", cmd); return -ENOTTY; @@ -2543,10 +2557,6 @@ snd_pcm_stream_unlock_irq(substream); return res; } - case SNDRV_PCM_IOCTL_DRAIN: - return snd_pcm_playback_drain(substream); - case SNDRV_PCM_IOCTL_DROP: - return snd_pcm_playback_drop(substream); } return snd_pcm_common_ioctl1(substream, cmd, arg); } @@ -2626,10 +2636,6 @@ __put_user(result, _frames); return result < 0 ? result : 0; } - case SNDRV_PCM_IOCTL_DRAIN: - return snd_pcm_capture_drain(substream); - case SNDRV_PCM_IOCTL_DROP: - return snd_pcm_capture_drop(substream); } return snd_pcm_common_ioctl1(substream, cmd, arg); } Index: alsa-kernel/include/pcm.h =================================================================== RCS file: /home/tiwai/cvs/alsa/alsa-kernel/include/pcm.h,v retrieving revision 1.49 diff -u -r1.49 pcm.h --- alsa-kernel/include/pcm.h 15 Sep 2004 09:04:26 -0000 1.49 +++ alsa-kernel/include/pcm.h 15 Sep 2004 23:04:59 -0000 @@ -364,6 +364,7 @@ typedef struct _snd_pcm_group { /* keep linked substreams */ spinlock_t lock; struct list_head substreams; + int count; } snd_pcm_group_t; struct _snd_pcm_substream { @@ -488,6 +489,7 @@ int snd_pcm_prepare(snd_pcm_substream_t *substream); int snd_pcm_start(snd_pcm_substream_t *substream); int snd_pcm_stop(snd_pcm_substream_t *substream, int status); +int snd_pcm_drain_done(snd_pcm_substream_t *substream); #ifdef CONFIG_PM int snd_pcm_suspend(snd_pcm_substream_t *substream); int snd_pcm_suspend_all(snd_pcm_t *pcm); ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-16 11:16 ` Takashi Iwai @ 2004-09-16 19:56 ` Giuliano Pochini 0 siblings, 0 replies; 19+ messages in thread From: Giuliano Pochini @ 2004-09-16 19:56 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel On Thu, 16 Sep 2004 13:16:41 +0200 Takashi Iwai <tiwai@suse.de> wrote: > > I think it'd be better to add a new field to substream to indicate the > > DMA running status so that snd_pcm_stop() can be called safely > > regardless of the current status. > > > > The situation right now is a mess, the DMA running state depending on > > the stream direction and the state. > > We have a snd_pcm_running() so the flag is not always necessary > (although it would be nice). > > In the current code, the PCM state is changed somehow unconditionally > among all linked substreams via snd_pcm_change_state(), and apparently > this causes the confusion. > > I did clean up and rewrite of pcm core stuff. > Could you check the attached patch? I had no time to check the code carefully, but it seems ok. I tested it and it worked flawlessly. I'll do more testing tomorrow. Tnx. -- Giuliano.-- Giuliano. ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-15 17:50 ` Giuliano Pochini 2004-09-15 18:01 ` Takashi Iwai @ 2004-09-17 6:54 ` Jaroslav Kysela 2004-09-17 10:22 ` Takashi Iwai 1 sibling, 1 reply; 19+ messages in thread From: Jaroslav Kysela @ 2004-09-17 6:54 UTC (permalink / raw) To: Giuliano Pochini; +Cc: Takashi Iwai, alsa-devel On Wed, 15 Sep 2004, Giuliano Pochini wrote: > snd_pcm_playback_drain() (when state=RUNNING) calls > snd_pcm_change_state(DRAINING). When substreams are linked, it changes the > state of all linked substreams *without* passing through > snd_pcm_capture_drain(DRAINING). Guys, it's because Takashi changed the linked stream behaviour at some time. The first definition for linked streams was that only start of all stream was linked. The drain/drop operations were separated. The linking makes sense for hardware that support that. Otherwise, we can start streams in most close time, but draining and dropping doesn't make much sense, because different clocking sources for different cards will produce jitter, thus there is no real sync and application must care. I suggest to return back to state when only snd_pcm_start() is synchronized. You can look to pcm_multi.c and test/latency.c - the drop/drain is called for all streams. Jaroslav ----- Jaroslav Kysela <perex@suse.cz> Linux Kernel Sound Maintainer ALSA Project, SUSE Labs ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-17 6:54 ` Jaroslav Kysela @ 2004-09-17 10:22 ` Takashi Iwai 2004-09-17 10:32 ` Takashi Iwai 0 siblings, 1 reply; 19+ messages in thread From: Takashi Iwai @ 2004-09-17 10:22 UTC (permalink / raw) To: Jaroslav Kysela; +Cc: Giuliano Pochini, alsa-devel At Fri, 17 Sep 2004 08:54:40 +0200 (CEST), Jaroslav wrote: > > On Wed, 15 Sep 2004, Giuliano Pochini wrote: > > > snd_pcm_playback_drain() (when state=RUNNING) calls > > snd_pcm_change_state(DRAINING). When substreams are linked, it changes the > > state of all linked substreams *without* passing through > > snd_pcm_capture_drain(DRAINING). > > Guys, it's because Takashi changed the linked stream behaviour at some > time. The first definition for linked streams was that only start of all > stream was linked. The drain/drop operations were separated. No, it's because snd_pcm_*_drain() and snd_pcm_*_drop() call snd_pcm_start(), snd_pcm_stop() and snd_pcm_change_state() internally. These three functions change the state of all linked streams (the last one does even unconditionally). Well, hey, this was coded so since long long time ago ;) > The linking makes sense for hardware that support that. Otherwise, we can > start streams in most close time, but draining and dropping doesn't make > much sense, because different clocking sources for different cards will > produce jitter, thus there is no real sync and application must care. Agreed. I've assumed that drain/drop are supposed to be synchronous, but they should not. > I suggest to return back to state when only snd_pcm_start() > is synchronized. > > You can look to pcm_multi.c and test/latency.c - the drop/drain is called > for all streams. JACK does it, too. I'll rewrite the patch now... Takashi ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-17 10:22 ` Takashi Iwai @ 2004-09-17 10:32 ` Takashi Iwai 2004-09-22 8:08 ` Jaroslav Kysela 0 siblings, 1 reply; 19+ messages in thread From: Takashi Iwai @ 2004-09-17 10:32 UTC (permalink / raw) To: Jaroslav Kysela; +Cc: Giuliano Pochini, alsa-devel At Fri, 17 Sep 2004 12:22:07 +0200, I wrote: > > > The linking makes sense for hardware that support that. Otherwise, we can > > start streams in most close time, but draining and dropping doesn't make > > much sense, because different clocking sources for different cards will > > produce jitter, thus there is no real sync and application must care. > > Agreed. I've assumed that drain/drop are supposed to be synchronous, > but they should not. I just noticed that pcm_multi.c _assumes_ indeed that linked draining/dropping works as synchronized. So, the current semantics is correct. If snd_pcm_drain() affects only the given stream, then it's difficult to get sync among all streams, because basically draining consists of two phases: - trigger DRAIN state to all streams - wait until all streams get drained We cannot do the procedure above in the serialized manner. Also, another question is how to handle XRUN state. When one of the linked streams gets XRUN, should all streams be stopped and marked as XRUN? I think yes. Takashi ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-17 10:32 ` Takashi Iwai @ 2004-09-22 8:08 ` Jaroslav Kysela 2004-09-22 10:08 ` Takashi Iwai 0 siblings, 1 reply; 19+ messages in thread From: Jaroslav Kysela @ 2004-09-22 8:08 UTC (permalink / raw) To: Takashi Iwai; +Cc: Giuliano Pochini, alsa-devel On Fri, 17 Sep 2004, Takashi Iwai wrote: > At Fri, 17 Sep 2004 12:22:07 +0200, > I wrote: > > > > > The linking makes sense for hardware that support that. Otherwise, we can > > > start streams in most close time, but draining and dropping doesn't make > > > much sense, because different clocking sources for different cards will > > > produce jitter, thus there is no real sync and application must care. > > > > Agreed. I've assumed that drain/drop are supposed to be synchronous, > > but they should not. > > I just noticed that pcm_multi.c _assumes_ indeed that linked > draining/dropping works as synchronized. So, the current semantics is > correct. Yes, sorry - my fault. I had vacation and I read e-mails only quickly. > If snd_pcm_drain() affects only the given stream, then it's difficult > to get sync among all streams, because basically draining consists of > two phases: > > - trigger DRAIN state to all streams > - wait until all streams get drained > > We cannot do the procedure above in the serialized manner. Application might care independently for all streams. But on other side, it's too late to change midlevel behaviour, so we should only correct the problems. > Also, another question is how to handle XRUN state. > When one of the linked streams gets XRUN, should all streams be > stopped and marked as XRUN? I think yes. Probably yes. In all cases, the application (or pcm_multi plugin) must take care about correcting jitter, otherwise the streams will go out of sync after awhile - resulting in xrun. Jaroslav ----- Jaroslav Kysela <perex@suse.cz> Linux Kernel Sound Maintainer ALSA Project, SUSE Labs ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-22 8:08 ` Jaroslav Kysela @ 2004-09-22 10:08 ` Takashi Iwai 2004-09-22 14:12 ` Giuliano Pochini 2004-09-23 7:43 ` Jaroslav Kysela 0 siblings, 2 replies; 19+ messages in thread From: Takashi Iwai @ 2004-09-22 10:08 UTC (permalink / raw) To: Jaroslav Kysela; +Cc: Giuliano Pochini, alsa-devel At Wed, 22 Sep 2004 10:08:14 +0200 (CEST), Jaroslav wrote: > > On Fri, 17 Sep 2004, Takashi Iwai wrote: > > > At Fri, 17 Sep 2004 12:22:07 +0200, > > I wrote: > > > > > > > The linking makes sense for hardware that support that. Otherwise, we can > > > > start streams in most close time, but draining and dropping doesn't make > > > > much sense, because different clocking sources for different cards will > > > > produce jitter, thus there is no real sync and application must care. > > > > > > Agreed. I've assumed that drain/drop are supposed to be synchronous, > > > but they should not. > > > > I just noticed that pcm_multi.c _assumes_ indeed that linked > > draining/dropping works as synchronized. So, the current semantics is > > correct. > > Yes, sorry - my fault. I had vacation and I read e-mails only quickly. I hope you enjoyed vacation ;) > > If snd_pcm_drain() affects only the given stream, then it's difficult > > to get sync among all streams, because basically draining consists of > > two phases: > > > > - trigger DRAIN state to all streams > > - wait until all streams get drained > > > > We cannot do the procedure above in the serialized manner. > > Application might care independently for all streams. But on other side, > it's too late to change midlevel behaviour, so we should only correct the > problems. As written above, for draining, we'll need two actions: changing the streams to DRAINING state and then syncing. If we call snd_pcm_drain() for each stream in the serial manner, the situation like below can happen: - snd_pcm_drain(pcm1) - stream 1 goes to DRAIN - wait until synced - meanwhile, stream 2 got out of data. all linked streams are changed to XRUN. - returns -EFIFO. So, if we implement seprate draining, two functions should be provided, such as snd_pcm_drain_trigger() snd_pcm_drain_sync() and the multiple streams can be handled like for_each_stream { snd_pcm_drain_trigger(pcm); } for_each_stream { snd_pcm_drain_sync(pcm); } > > Also, another question is how to handle XRUN state. > > When one of the linked streams gets XRUN, should all streams be > > stopped and marked as XRUN? I think yes. > > Probably yes. In all cases, the application (or pcm_multi plugin) must > take care about correcting jitter, otherwise the streams will go out of > sync after awhile - resulting in xrun. In the case of multi pcm, we can do nothing against clock difference because all data is sent synchronously to several streams. Hence, when one of streams gets XRUN, the whole pcm should be handled as XRUN. OTOH, when you link streams but feed/read data separately for each stream, the clock difference doesn't matter. Each stream may have even different periods, buffer sizes, whatever. So, in the first case, handling xrun/drain/drop in sync of all linked streams makes sense. But what about the second case...? Well, the link can be removed dynamically. So, in theory, we can propose like the following: - Start, drain, drop and xrun of linked streams are operated to all streams by a single call. Especially, drain assures that the all streams get sync'ed. - When one wants to operate drain and drop separately, explicitly unlink streams after the sync'ed start. Or, we can create new PCM functions or flags for single-stream operations, but I don't think it's needed. Takashi ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-22 10:08 ` Takashi Iwai @ 2004-09-22 14:12 ` Giuliano Pochini 2004-09-23 7:43 ` Jaroslav Kysela 1 sibling, 0 replies; 19+ messages in thread From: Giuliano Pochini @ 2004-09-22 14:12 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel, Jaroslav Kysela On 22-Sep-2004 Takashi Iwai wrote: > OTOH, when you link streams but feed/read data separately for each > stream, the clock difference doesn't matter. Each stream may have > even different periods, buffer sizes, whatever. > > So, in the first case, handling xrun/drain/drop in sync of all linked > streams makes sense. But what about the second case...? It makes sense if the streams were started syncronously and the clock is the same, but there isn't a way to know that right now. It would also require driver support. > Well, the link can be removed dynamically. So, in theory, we can > propose like the following: > > - Start, drain, drop and xrun of linked streams are operated to all > streams by a single call. Especially, drain assures that the all > streams get sync'ed. > > - When one wants to operate drain and drop separately, explicitly > unlink streams after the sync'ed start. It's ok IMHO. -- Giuliano. ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-22 10:08 ` Takashi Iwai 2004-09-22 14:12 ` Giuliano Pochini @ 2004-09-23 7:43 ` Jaroslav Kysela 2004-09-24 13:30 ` Takashi Iwai 1 sibling, 1 reply; 19+ messages in thread From: Jaroslav Kysela @ 2004-09-23 7:43 UTC (permalink / raw) To: Takashi Iwai; +Cc: Giuliano Pochini, alsa-devel On Wed, 22 Sep 2004, Takashi Iwai wrote: > OTOH, when you link streams but feed/read data separately for each > stream, the clock difference doesn't matter. Each stream may have > even different periods, buffer sizes, whatever. > > So, in the first case, handling xrun/drain/drop in sync of all linked > streams makes sense. But what about the second case...? > > Well, the link can be removed dynamically. So, in theory, we can > propose like the following: > > - Start, drain, drop and xrun of linked streams are operated to all > streams by a single call. Especially, drain assures that the all > streams get sync'ed. > > - When one wants to operate drain and drop separately, explicitly > unlink streams after the sync'ed start. This proposal seems reasonable. Jaroslav ----- Jaroslav Kysela <perex@suse.cz> Linux Kernel Sound Maintainer ALSA Project, SUSE Labs ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php ^ permalink raw reply [flat|nested] 19+ messages in thread
* Re: [PATCH] Kernel crash 2004-09-23 7:43 ` Jaroslav Kysela @ 2004-09-24 13:30 ` Takashi Iwai 0 siblings, 0 replies; 19+ messages in thread From: Takashi Iwai @ 2004-09-24 13:30 UTC (permalink / raw) To: Jaroslav Kysela; +Cc: Giuliano Pochini, alsa-devel At Thu, 23 Sep 2004 09:43:39 +0200 (CEST), Jaroslav wrote: > > On Wed, 22 Sep 2004, Takashi Iwai wrote: > > > OTOH, when you link streams but feed/read data separately for each > > stream, the clock difference doesn't matter. Each stream may have > > even different periods, buffer sizes, whatever. > > > > So, in the first case, handling xrun/drain/drop in sync of all linked > > streams makes sense. But what about the second case...? > > > > Well, the link can be removed dynamically. So, in theory, we can > > propose like the following: > > > > - Start, drain, drop and xrun of linked streams are operated to all > > streams by a single call. Especially, drain assures that the all > > streams get sync'ed. > > > > - When one wants to operate drain and drop separately, explicitly > > unlink streams after the sync'ed start. > > This proposal seems reasonable. Shall I apply my patch to CVS, or would you like to audit it more? thanks, Takashi ------------------------------------------------------- This SF.Net email is sponsored by: YOU BE THE JUDGE. Be one of 170 Project Admins to receive an Apple iPod Mini FREE for your judgement on who ports your project to Linux PPC the best. Sponsored by IBM. Deadline: Sept. 24. Go here: http://sf.net/ppc_contest.php ^ permalink raw reply [flat|nested] 19+ messages in thread
end of thread, other threads:[~2004-09-24 13:30 UTC | newest] Thread overview: 19+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2004-09-05 18:51 Kernel crash Giuliano Pochini 2004-09-06 15:16 ` Takashi Iwai 2004-09-07 7:55 ` Giuliano Pochini 2004-09-11 19:02 ` [PATCH] " Giuliano Pochini 2004-09-13 15:00 ` Takashi Iwai 2004-09-13 20:07 ` Giuliano Pochini 2004-09-15 10:24 ` Takashi Iwai 2004-09-15 17:50 ` Giuliano Pochini 2004-09-15 18:01 ` Takashi Iwai 2004-09-16 11:16 ` Takashi Iwai 2004-09-16 19:56 ` Giuliano Pochini 2004-09-17 6:54 ` Jaroslav Kysela 2004-09-17 10:22 ` Takashi Iwai 2004-09-17 10:32 ` Takashi Iwai 2004-09-22 8:08 ` Jaroslav Kysela 2004-09-22 10:08 ` Takashi Iwai 2004-09-22 14:12 ` Giuliano Pochini 2004-09-23 7:43 ` Jaroslav Kysela 2004-09-24 13:30 ` Takashi Iwai
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.