[PATCH] sis7019: fix capture issues with multiple periods per buffer

[PATCH] sis7019: fix capture issues with multiple periods per buffer

[David Dillow]

When using a timing voice to clock out periods during capture, the
driver would slowly loose synchronization and never catch up, eventually
reaching a point where it no longer generated interrupts. To avoid
this situation, the virtual period clocking was changed to shorten the
next timing period when our timing voice falls too far behind the
capture voice. In addition, the first virtual period for the timing
voice was slightly too short, causing the timing voice to initially be
ahead of the capture voice.
    
While tracking down this problem, I noticed that the expected sample
offset was being incorrectly initialized, causing an overrun to be
incorrectly reported when the timing voice happened to be perfectly
synchronized.
    
Reported-by: Hans Schou <linux@schou.dk>
Signed-off-by: Dave Dillow <dave@thedillows.org>
---
This has survived overnight testing at 44.1 kHz/16 bit/mono without issue.

diff --git a/sound/pci/sis7019.c b/sound/pci/sis7019.c
index 9cc1b5a..614ff6e 100644
--- a/sound/pci/sis7019.c
+++ b/sound/pci/sis7019.c
@@ -264,11 +264,13 @@ static void sis_update_voice(struct voice *voice)
 		 * if using small periods.
 		 *
 		 * If we're less than 9 samples behind, we're on target.
+		 * Otherwise, shorten the next vperiod by the amount we've
+		 * been delayed.
 		 */
 		if (sync > -9)
 			voice->vperiod = voice->sync_period_size + 1;
 		else
-			voice->vperiod = voice->sync_period_size - 4;
+			voice->vperiod = voice->sync_period_size + sync + 10;
 
 		if (voice->vperiod < voice->buffer_size) {
 			sis_update_sso(voice, voice->vperiod);
@@ -736,7 +738,7 @@ static void sis_prepare_timing_voice(struct voice *voice,
 	period_size = buffer_size;
 
 	/* Initially, we want to interrupt just a bit behind the end of
-	 * the period we're clocking out. 10 samples seems to give a good
+	 * the period we're clocking out. 12 samples seems to give a good
 	 * delay.
 	 *
 	 * We want to spread our interrupts throughout the virtual period,
@@ -747,7 +749,7 @@ static void sis_prepare_timing_voice(struct voice *voice,
 	 *
 	 * This is all moot if we don't need to use virtual periods.
 	 */
-	vperiod = runtime->period_size + 10;
+	vperiod = runtime->period_size + 12;
 	if (vperiod > period_size) {
 		u16 tail = vperiod % period_size;
 		u16 quarter_period = period_size / 4;
@@ -776,7 +778,7 @@ static void sis_prepare_timing_voice(struct voice *voice,
 	 */
 	timing->flags |= VOICE_SYNC_TIMING;
 	timing->sync_base = voice->ctrl_base;
-	timing->sync_cso = runtime->period_size - 1;
+	timing->sync_cso = runtime->period_size;
 	timing->sync_period_size = runtime->period_size;
 	timing->sync_buffer_size = runtime->buffer_size;
 	timing->period_size = period_size;

Re: [PATCH] sis7019: fix capture issues with multiple periods per buffer

[David Dillow]

On Sat, 2010-06-26 at 18:04 -0400, David Dillow wrote:
> When using a timing voice to clock out periods during capture, the
> driver would slowly loose synchronization and never catch up, eventually
> reaching a point where it no longer generated interrupts.

> ---
> This has survived overnight testing at 44.1 kHz/16 bit/mono without issue.

Only to die at 12 hours and ~15 minutes. It did not trip any of the
instrumentation I was using to verify operation, so this may be a
different problem. I'm looking into it.

Dave

Re: [PATCH] sis7019: fix capture issues with multiple periods per buffer

[David Dillow]

On Sun, 2010-06-27 at 10:46 -0400, David Dillow wrote:
> On Sat, 2010-06-26 at 18:04 -0400, David Dillow wrote:
> > When using a timing voice to clock out periods during capture, the
> > driver would slowly loose synchronization and never catch up, eventually
> > reaching a point where it no longer generated interrupts.
> 
> > ---
> > This has survived overnight testing at 44.1 kHz/16 bit/mono without issue.
> 
> Only to die at 12 hours and ~15 minutes. It did not trip any of the
> instrumentation I was using to verify operation, so this may be a
> different problem. I'm looking into it.

Ok, definitely different problem, as it dies 12:15 into a run using 2
periods per buffer, so the virtual timing code does not come into it.
Interestingly enough, it died after ~44092 seconds while recording at
44100 Hz.

The patch should be applied. If and when I find the source of this new
bug, I'll submit a separate patch.

Re: [PATCH] sis7019: fix capture issues with multiple periods per buffer

[Jaroslav Kysela]

On Sat, 26 Jun 2010, David Dillow wrote:

> When using a timing voice to clock out periods during capture, the
> driver would slowly loose synchronization and never catch up, eventually
> reaching a point where it no longer generated interrupts. To avoid
> this situation, the virtual period clocking was changed to shorten the
> next timing period when our timing voice falls too far behind the
> capture voice. In addition, the first virtual period for the timing
> voice was slightly too short, causing the timing voice to initially be
> ahead of the capture voice.
>
> While tracking down this problem, I noticed that the expected sample
> offset was being incorrectly initialized, causing an overrun to be
> incorrectly reported when the timing voice happened to be perfectly
> synchronized.
>
> Reported-by: Hans Schou <linux@schou.dk>
> Signed-off-by: Dave Dillow <dave@thedillows.org>

Thanks for the patch. I applied it to my GIT tree.

 					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: [PATCH] sis7019: fix capture issues with multiple periods per buffer

[David Dillow]

On Mon, 2010-06-28 at 01:33 -0400, David Dillow wrote:
> Ok, definitely different problem, as it dies 12:15 into a run using 2
> periods per buffer, so the virtual timing code does not come into it.
> Interestingly enough, it died after ~44092 seconds while recording at
> 44100 Hz.
> 
> The patch should be applied. If and when I find the source of this new
> bug, I'll submit a separate patch.

For the Google record: this was the boundary wrap issue fixed by Clemens
Ladisch in b406e6103baa3da85950f22d3d46d21a8da654c5. It's survived many
boundary crossings with a shorted setting (256 seconds at 48kHz) so I'm
very confident.

And I went with 2.6.34 to avoid the potential of instabilities in the
2.6.35-rc series. Oops :)