All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] remove emu10k1 pops/clicks at the beginning and end of playback (fwd)
@ 2005-03-04  8:33 Jaroslav Kysela
  2005-03-04 18:17 ` Lee Revell
  0 siblings, 1 reply; 12+ messages in thread
From: Jaroslav Kysela @ 2005-03-04  8:33 UTC (permalink / raw)
  To: ALSA development; +Cc: Jindrich Makovicka

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1232 bytes --]

Hi,

	could emu10k1 hackers review this patch?

					Thanks,
						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project, SUSE Labs

---------- Forwarded message ----------
Date: Fri, 04 Mar 2005 09:17:15 +0100
From: Jindrich Makovicka <makovick@kmlinux.fjfi.cvut.cz>
To: perex@suse.cz
Subject: [PATCH] remove emu10k1 pops/clicks at the beginning and end of
    playback

Hello,

I experienced very annoying pops and clicks et the end (and sometimes at 
the beginning) of audio playback with ALSA. This is especially bad for 
UI sounds like instant mesaging beeps.

I don't know emu10k architecture much, but there seems to be a bug with 
buffer setup for 16bit samples (as the pop doesn't occur for 8bit) - 
this causes the pop at the end, and also improper purging of the cache - 
which causes a pop at the beginning when playing 16bit sample after an 
8bit one and vice versa.

The attached patch fixes the issue for me. It changes the calculation of 
the start address offset for 16bit samples (previously it was multiplied 
by 2, while the start and end addresses were divided), and it purges all 
cache registers for both channels in invalidate_cache.

Best regards,
-- 
Jindrich Makovicka

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: TEXT/X-PATCH; NAME="emupcm.c.patch", Size: 1326 bytes --]

--- emupcm.c.orig	2005-03-01 11:18:37.000000000 +0100
+++ emupcm.c	2005-03-03 23:28:07.158769431 +0100
@@ -276,7 +276,7 @@
 	if (master) {
 		unsigned int ccis = stereo ? 28 : 30;
 		if (w_16)
-			ccis *= 2;
+			ccis >>= 1;
 		evoice->epcm->ccca_start_addr = start_addr + ccis;
 		if (extra) {
 			start_addr += ccis;
@@ -490,15 +490,19 @@
 	sample = snd_pcm_format_width(runtime->format) == 16 ? 0 : 0x80808080;
 	if (runtime->channels > 1) {
 		ccis = 28;
-		cs = 4;
+		cs = 16;
 	} else {
 		ccis = 30;
-		cs = 2;
+		cs = 16;
 	}
 	if (sample == 0)	/* 16-bit */
 		ccis *= 2;
-	for (i = 0; i < cs; i++)
-		snd_emu10k1_ptr_write(emu, CD0 + i, voice, sample);
+	for (i = 0; i < cs; i++) {
+    		snd_emu10k1_ptr_write(emu, CD0 + i, voice, sample);
+    		if (runtime->channels > 1) {
+        		snd_emu10k1_ptr_write(emu, CD0 + i, voice + 1, sample);
+		}
+	}
 	// reset cache
 	snd_emu10k1_ptr_write(emu, CCR_CACHEINVALIDSIZE, voice, 0);
 	snd_emu10k1_ptr_write(emu, CCR_READADDRESS, voice, cra);
@@ -508,6 +512,9 @@
 	}
 	// fill cache
 	snd_emu10k1_ptr_write(emu, CCR_CACHEINVALIDSIZE, voice, ccis);
+	if (runtime->channels > 1) {
+		snd_emu10k1_ptr_write(emu, CCR_CACHEINVALIDSIZE, voice + 1, ccis);
+	}
 }
 
 static void snd_emu10k1_playback_prepare_voice(emu10k1_t *emu, emu10k1_voice_t *evoice, int master, int extra)

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] remove emu10k1 pops/clicks at the beginning and end of playback (fwd)
  2005-03-04  8:33 [PATCH] remove emu10k1 pops/clicks at the beginning and end of playback (fwd) Jaroslav Kysela
@ 2005-03-04 18:17 ` Lee Revell
  2005-03-05  8:29   ` Jindrich Makovicka
  0 siblings, 1 reply; 12+ messages in thread
From: Lee Revell @ 2005-03-04 18:17 UTC (permalink / raw)
  To: Jaroslav Kysela; +Cc: ALSA development, Jindrich Makovicka

On Fri, 2005-03-04 at 09:33 +0100, Jaroslav Kysela wrote:
> Hi,
> 
> 	could emu10k1 hackers review this patch?
> 

Excellent!  Seems correct to me.  I have experienced the problem too,
and noticed a while back that we didn't clear the cache properly.

Lee



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] remove emu10k1 pops/clicks at the   beginning and end of playback (fwd)
  2005-03-04 18:17 ` Lee Revell
@ 2005-03-05  8:29   ` Jindrich Makovicka
  2005-03-05 11:05     ` James Courtier-Dutton
  0 siblings, 1 reply; 12+ messages in thread
From: Jindrich Makovicka @ 2005-03-05  8:29 UTC (permalink / raw)
  To: alsa-devel

[-- Attachment #1: Type: text/plain, Size: 2360 bytes --]

Lee Revell wrote:
> On Fri, 2005-03-04 at 22:30 +0100, Jindrich Makovicka wrote:
> 
>>> Lee Revell wrote:
>>
>>>> > It's not that the cciss register is buggy, it's just poorly understood.
>>
>>> 
>>> I don't mean the register alone, I mean the way ALSA sets it.
>>> 
>>
>>>> > See US patent 6138207 for the exact function of the cache control
>>>> > circuitry.  I can't get my brain around it, but maybe you can.
>>>> > 
>>>> > http://patft.uspto.gov/netacgi/nph-Parser?u=/netahtml/srchnum.htm&Sect1=PTO1&Sect2=HITOFF&p=1&r=1&l=50&f=G&d=PALL&s1=6138207.WKU.&OS=PN/6138207&RS=PN/6138207
>>
>>> 
>>> Thanks for the brain food, but today I am too tired to dive in  :) 
>>> 
>>
>>>> > Have you tried leaving the cciss setting alone, and just increasing cs
>>>> > to 16, which should cause it to silence the entire cache, not just the
>>>> > beginning?
>>
>>> 
>>> When I leave cciss alone, audio still heavily pops (16bit at the end, 
>>> 8bit at the beginning). When I change cciss the way OSS driver has it, 
>>> which I believe is correct, there is still a slight pop at the end for 
>>> both. After decreasing cciss a bit (4 samples), the pop at the end seems 
>>> to be gone. Unfortunately I cannot test it on other cards than my SB 1024.
>>> 
>>> My current patch is attached (applies to 2.6.11-mm1). I also moved cciss 
>>> setting to one place and made invalidate_cache honor the "extra" flag - 
>>> if extra is on, it does not invalidate the second channel.
>>> 
> 
> 
> OK, sounds good.
> 
> You will have to generate the patch against ALSA CVS.  This will require
> updating it to support the multichannel device, which is made up of 16
> mono channels, 16 bit, 48KHz only  (plus an extra voice for timing).
> Should be easy.
> 
> Then, repost it to alsa-devel.

So, as the the above patch appeared to be rather a proof of concept, 
here is the (hopefully) correct fix. It changes the following:

- CCIS setting, which was previously "upside down" is fixed and modified 
to supress the pop at the end

- CCIS values are now in one spot to allow tweaking

- invalidate_cache now purges complete cache for both channels.

Big thanks to Lee for his advices.

I don't see any reason the patch shouldn't apply to CVS, as the only 
change with wider impact is in invalidate_cache arguments, and all 
callers are updated.

Best regards,
-- 
Jindrich Makovicka

[-- Attachment #2: emupcm.c.diff2 --]
[-- Type: text/plain, Size: 3878 bytes --]

--- emupcm.c.orig	2005-03-02 09:40:37.000000000 +0100
+++ emupcm.c	2005-03-05 09:13:09.074485436 +0100
@@ -250,6 +250,22 @@
 		return CCCA_INTERPROM_2;
 }
 
+/*
+ * calculate cache invalidate size 
+ *
+ * stereo: channel is stereo
+ * w_16: using 16bit samples
+ *
+ * returns: cache invalidate size in samples
+ */
+static int inline emu10k1_ccis(int stereo, int w_16)
+{
+	if (w_16) {
+		return stereo ? 24 : 26;
+	} else {
+		return stereo ? 24*2 : 26*2;
+	}
+}
 
 static void snd_emu10k1_pcm_init_voice(emu10k1_t *emu,
 				       int master, int extra,
@@ -304,9 +320,7 @@
 		memcpy(send_amount, &mix->send_volume[tmp][0], 8);
 	}
 
-	ccis = stereo ? 28 : 30;
-	if (w_16)
-		ccis *= 2;
+	ccis = emu10k1_ccis(stereo, w_16);
 	
 	if (master) {
 		evoice->epcm->ccca_start_addr = start_addr + ccis;
@@ -473,11 +487,14 @@
 
 	start_addr = epcm->start_addr;
 	end_addr = snd_pcm_lib_period_bytes(substream);
-	if (runtime->channels == 2)
+	if (runtime->channels == 2) {
+		start_addr >>= 1;
 		end_addr >>= 1;
+	}
 	end_addr += start_addr;
 	snd_emu10k1_pcm_init_voice(emu, 1, 1, epcm->extra,
 				   start_addr, end_addr);
+	start_addr = epcm->start_addr;
 	end_addr = epcm->start_addr + snd_pcm_lib_buffer_bytes(substream);
 	snd_emu10k1_pcm_init_voice(emu, 1, 0, epcm->voices[0],
 				   start_addr, end_addr);
@@ -598,7 +615,7 @@
 	return 0;
 }
 
-static void snd_emu10k1_playback_invalidate_cache(emu10k1_t *emu, emu10k1_voice_t *evoice)
+static void snd_emu10k1_playback_invalidate_cache(emu10k1_t *emu, int extra, emu10k1_voice_t *evoice)
 {
 	snd_pcm_runtime_t *runtime;
 	unsigned int voice, i, ccis, cra = 64, cs, sample;
@@ -608,27 +625,26 @@
 	runtime = evoice->epcm->substream->runtime;
 	voice = evoice->number;
 	sample = snd_pcm_format_width(runtime->format) == 16 ? 0 : 0x80808080;
-	if (runtime->channels == 2) {
-		ccis = 28;
-		cs = 4;
-	} else {
-		ccis = 30;
-		cs = 2;
-	}
-	if (sample == 0)	/* 16-bit */
-		ccis *= 2;
-	for (i = 0; i < cs; i++)
+	cs = 16;
+	ccis = emu10k1_ccis(!extra && runtime->channels == 2, sample == 0);
+	for (i = 0; i < cs; i++) {
 		snd_emu10k1_ptr_write(emu, CD0 + i, voice, sample);
-		
+		if (!extra && runtime->channels == 2) {
+			snd_emu10k1_ptr_write(emu, CD0 + i, voice + 1, sample);
+		}
+	}
 	// reset cache
 	snd_emu10k1_ptr_write(emu, CCR_CACHEINVALIDSIZE, voice, 0);
 	snd_emu10k1_ptr_write(emu, CCR_READADDRESS, voice, cra);
-	if (runtime->channels == 2) {
+	if (!extra && runtime->channels == 2) {
 		snd_emu10k1_ptr_write(emu, CCR_CACHEINVALIDSIZE, voice + 1, 0);
 		snd_emu10k1_ptr_write(emu, CCR_READADDRESS, voice + 1, cra);
 	}
 	// fill cache
 	snd_emu10k1_ptr_write(emu, CCR_CACHEINVALIDSIZE, voice, ccis);
+	if (!extra && runtime->channels == 2) {
+		snd_emu10k1_ptr_write(emu, CCR_CACHEINVALIDSIZE, voice+1, ccis);
+	}
 }
 
 static void snd_emu10k1_playback_prepare_voice(emu10k1_t *emu, emu10k1_voice_t *evoice, int master, int extra)
@@ -707,8 +723,8 @@
 	spin_lock(&emu->reg_lock);
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
-		snd_emu10k1_playback_invalidate_cache(emu, epcm->extra);	/* do we need this? */
-		snd_emu10k1_playback_invalidate_cache(emu, epcm->voices[0]);
+		snd_emu10k1_playback_invalidate_cache(emu, 1, epcm->extra);	/* do we need this? */
+		snd_emu10k1_playback_invalidate_cache(emu, 0, epcm->voices[0]);
 		/* follow thru */
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		snd_emu10k1_playback_prepare_voice(emu, epcm->voices[0], 1, 0);
@@ -836,9 +852,9 @@
 	case SNDRV_PCM_TRIGGER_START:
 		// prepare voices
 		for (i = 0; i < NUM_EFX_PLAYBACK; i++) {	
-			snd_emu10k1_playback_invalidate_cache(emu, epcm->voices[i]);
+			snd_emu10k1_playback_invalidate_cache(emu, 0, epcm->voices[i]);
 		}
-		snd_emu10k1_playback_invalidate_cache(emu, epcm->extra);
+		snd_emu10k1_playback_invalidate_cache(emu, 1, epcm->extra);
 
 		/* follow thru */
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Re: [PATCH] remove emu10k1 pops/clicks at the   beginning and end of playback (fwd)
  2005-03-05  8:29   ` Jindrich Makovicka
@ 2005-03-05 11:05     ` James Courtier-Dutton
  2005-03-05 11:46       ` Jindrich Makovicka
  0 siblings, 1 reply; 12+ messages in thread
From: James Courtier-Dutton @ 2005-03-05 11:05 UTC (permalink / raw)
  To: Jindrich Makovicka; +Cc: alsa-devel

Jindrich Makovicka wrote:
> Lee Revell wrote:
> 
>> On Fri, 2005-03-04 at 22:30 +0100, Jindrich Makovicka wrote:
>>
>>>> Lee Revell wrote:
>>>
>>>
>>>>> > It's not that the cciss register is buggy, it's just poorly 
>>>>> understood.
>>>
>>>
>>>>
>>>> I don't mean the register alone, I mean the way ALSA sets it.
>>>>
>>>
>>>>> > See US patent 6138207 for the exact function of the cache control
>>>>> > circuitry.  I can't get my brain around it, but maybe you can.
>>>>> > > 
>>>>> http://patft.uspto.gov/netacgi/nph-Parser?u=/netahtml/srchnum.htm&Sect1=PTO1&Sect2=HITOFF&p=1&r=1&l=50&f=G&d=PALL&s1=6138207.WKU.&OS=PN/6138207&RS=PN/6138207 
>>>>>
>>>
>>>
>>>>
>>>> Thanks for the brain food, but today I am too tired to dive in  :)
>>>
>>>
>>>>> > Have you tried leaving the cciss setting alone, and just 
>>>>> increasing cs
>>>>> > to 16, which should cause it to silence the entire cache, not 
>>>>> just the
>>>>> > beginning?
>>>
>>>
>>>>
>>>> When I leave cciss alone, audio still heavily pops (16bit at the 
>>>> end, 8bit at the beginning). When I change cciss the way OSS driver 
>>>> has it, which I believe is correct, there is still a slight pop at 
>>>> the end for both. After decreasing cciss a bit (4 samples), the pop 
>>>> at the end seems to be gone. Unfortunately I cannot test it on other 
>>>> cards than my SB 1024.
>>>>
>>>> My current patch is attached (applies to 2.6.11-mm1). I also moved 
>>>> cciss setting to one place and made invalidate_cache honor the 
>>>> "extra" flag - if extra is on, it does not invalidate the second 
>>>> channel.
>>>>
>>
>>
>> OK, sounds good.
>>
>> You will have to generate the patch against ALSA CVS.  This will require
>> updating it to support the multichannel device, which is made up of 16
>> mono channels, 16 bit, 48KHz only  (plus an extra voice for timing).
>> Should be easy.
>>
>> Then, repost it to alsa-devel.
> 
> 
> So, as the the above patch appeared to be rather a proof of concept, 
> here is the (hopefully) correct fix. It changes the following:
> 
> - CCIS setting, which was previously "upside down" is fixed and modified 
> to supress the pop at the end
> 
> - CCIS values are now in one spot to allow tweaking
> 
> - invalidate_cache now purges complete cache for both channels.
> 
> Big thanks to Lee for his advices.
> 
> I don't see any reason the patch shouldn't apply to CVS, as the only 
> change with wider impact is in invalidate_cache arguments, and all 
> callers are updated.
> 
> Best regards,
> 
> 

If it is of interest, the PCI transfers happen in blocks of 64 BYTES.
So, if the playback pointer is in the middle of a block of 64 BYTES, and 
you wish to send new samples to the card, without missing any of them, 
you should start sending them at the next 64 bytes boundry.


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] remove emu10k1 pops/clicks at the     beginning and end of playback (fwd)
  2005-03-05 11:05     ` James Courtier-Dutton
@ 2005-03-05 11:46       ` Jindrich Makovicka
  2005-03-05 14:37         ` James Courtier-Dutton
  2005-03-05 18:39         ` Lee Revell
  0 siblings, 2 replies; 12+ messages in thread
From: Jindrich Makovicka @ 2005-03-05 11:46 UTC (permalink / raw)
  To: alsa-devel

James Courtier-Dutton wrote:
> If it is of interest, the PCI transfers happen in blocks of 64 BYTES.
> So, if the playback pointer is in the middle of a block of 64 BYTES, and 
> you wish to send new samples to the card, without missing any of them, 
> you should start sending them at the next 64 bytes boundry.

AFAIK, emupcm (at least the part I try to fix) operates in DMA busmaster 
mode, so "sending" samples is out of its jurisdiction. It just sets up 
looping over the DMA buffer and lets the card do the rest.

FX8010 & ASIO is a different story, but I really don't know much in this 
matter.

Regards,
-- 
Jindrich Makovicka



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Re: [PATCH] remove emu10k1 pops/clicks at the   beginning and end of playback (fwd)
  2005-03-05 11:46       ` Jindrich Makovicka
@ 2005-03-05 14:37         ` James Courtier-Dutton
  2005-03-05 14:46           ` Jindrich Makovicka
  2005-03-05 18:39         ` Lee Revell
  1 sibling, 1 reply; 12+ messages in thread
From: James Courtier-Dutton @ 2005-03-05 14:37 UTC (permalink / raw)
  To: Jindrich Makovicka; +Cc: alsa-devel

Jindrich Makovicka wrote:
> James Courtier-Dutton wrote:
> 
>> If it is of interest, the PCI transfers happen in blocks of 64 BYTES.
>> So, if the playback pointer is in the middle of a block of 64 BYTES, 
>> and you wish to send new samples to the card, without missing any of 
>> them, you should start sending them at the next 64 bytes boundry.
> 
> 
> AFAIK, emupcm (at least the part I try to fix) operates in DMA busmaster 
> mode, so "sending" samples is out of its jurisdiction. It just sets up 
> looping over the DMA buffer and lets the card do the rest.
> 
> FX8010 & ASIO is a different story, but I really don't know much in this 
> matter.
> 
> Regards,

Same thing really. I meant DMA mode. The card requests the transfers in 
64 byte blocks.



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Re: [PATCH] remove emu10k1 pops/clicks at the   beginning and end of playback (fwd)
  2005-03-05 14:37         ` James Courtier-Dutton
@ 2005-03-05 14:46           ` Jindrich Makovicka
  2005-03-05 19:08             ` Jindrich Makovicka
  0 siblings, 1 reply; 12+ messages in thread
From: Jindrich Makovicka @ 2005-03-05 14:46 UTC (permalink / raw)
  To: James Courtier-Dutton; +Cc: alsa-devel

James Courtier-Dutton wrote:
> Jindrich Makovicka wrote:
> 
>> James Courtier-Dutton wrote:
>>
>>> If it is of interest, the PCI transfers happen in blocks of 64 BYTES.
>>> So, if the playback pointer is in the middle of a block of 64 BYTES, 
>>> and you wish to send new samples to the card, without missing any of 
>>> them, you should start sending them at the next 64 bytes boundry.
>>
>>
>>
>> AFAIK, emupcm (at least the part I try to fix) operates in DMA 
>> busmaster mode, so "sending" samples is out of its jurisdiction. It 
>> just sets up looping over the DMA buffer and lets the card do the rest.
>>
>> FX8010 & ASIO is a different story, but I really don't know much in 
>> this matter.
>>
>> Regards,
> 
> 
> Same thing really. I meant DMA mode. The card requests the transfers in 
> 64 byte blocks.

I think transfer alignment is handled by higher layers, due to

static snd_pcm_hardware_t snd_emu10k1_playback = 

{
...
	.period_bytes_min = 64,
...
}

The above bug had nothing to do with alignment, only the sample caches 
were improperly purged and ccis was set to wrong values.

-- 
Jindrich Makovicka


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Re: [PATCH] remove emu10k1 pops/clicks at the     beginning and end of playback (fwd)
  2005-03-05 11:46       ` Jindrich Makovicka
  2005-03-05 14:37         ` James Courtier-Dutton
@ 2005-03-05 18:39         ` Lee Revell
  1 sibling, 0 replies; 12+ messages in thread
From: Lee Revell @ 2005-03-05 18:39 UTC (permalink / raw)
  To: Jindrich Makovicka; +Cc: alsa-devel

On Sat, 2005-03-05 at 12:46 +0100, Jindrich Makovicka wrote:
> James Courtier-Dutton wrote:
> > If it is of interest, the PCI transfers happen in blocks of 64 BYTES.
> > So, if the playback pointer is in the middle of a block of 64 BYTES, and 
> > you wish to send new samples to the card, without missing any of them, 
> > you should start sending them at the next 64 bytes boundry.
> 
> AFAIK, emupcm (at least the part I try to fix) operates in DMA busmaster 
> mode, so "sending" samples is out of its jurisdiction. It just sets up 
> looping over the DMA buffer and lets the card do the rest.
> 
> FX8010 & ASIO is a different story, but I really don't know much in this 
> matter.

There's nothing magic about the ASIO implementation(s) for this card.
They work the same way as the multichannel devices in the Linux driver,
which is the same as the normal PCM devices, via DMA bus mastering.

You are probably referring to the "FX8010 PCM" aka "Raw S/PDIF PCM".
This one is different, because it creates a special PCM for AC3
passthrough using emu10k1 DSP code.

Lee 



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] remove emu10k1 pops/clicks at the       beginning and end of playback (fwd)
  2005-03-05 14:46           ` Jindrich Makovicka
@ 2005-03-05 19:08             ` Jindrich Makovicka
  2005-03-05 20:25               ` Lee Revell
  2005-03-08 16:11               ` Takashi Iwai
  0 siblings, 2 replies; 12+ messages in thread
From: Jindrich Makovicka @ 2005-03-05 19:08 UTC (permalink / raw)
  To: alsa-devel

[-- Attachment #1: Type: text/plain, Size: 227 bytes --]

Here is one more update of the patch. This version clears only a part of 
the cache, saving some register writes. Tested for few hours, no pops so 
far. But it might be better to use brute force anyway.

-- 
Jindrich Makovicka

[-- Attachment #2: emupcm.c.diff3 --]
[-- Type: text/plain, Size: 4031 bytes --]

--- emupcm.c.orig	2005-03-04 17:04:32.000000000 +0100
+++ emupcm.c	2005-03-05 14:21:36.932147794 +0100
@@ -250,6 +250,22 @@
 		return CCCA_INTERPROM_2;
 }
 
+/*
+ * calculate cache invalidate size 
+ *
+ * stereo: channel is stereo
+ * w_16: using 16bit samples
+ *
+ * returns: cache invalidate size in samples
+ */
+static int inline emu10k1_ccis(int stereo, int w_16)
+{
+	if (w_16) {
+		return stereo ? 24 : 26;
+	} else {
+		return stereo ? 24*2 : 26*2;
+	}
+}
 
 static void snd_emu10k1_pcm_init_voice(emu10k1_t *emu,
 				       int master, int extra,
@@ -304,9 +320,7 @@
 		memcpy(send_amount, &mix->send_volume[tmp][0], 8);
 	}
 
-	ccis = stereo ? 28 : 30;
-	if (w_16)
-		ccis *= 2;
+	ccis = emu10k1_ccis(stereo, w_16);
 	
 	if (master) {
 		evoice->epcm->ccca_start_addr = start_addr + ccis;
@@ -473,11 +487,14 @@
 
 	start_addr = epcm->start_addr;
 	end_addr = snd_pcm_lib_period_bytes(substream);
-	if (runtime->channels == 2)
+	if (runtime->channels == 2) {
+		start_addr >>= 1;
 		end_addr >>= 1;
+	}
 	end_addr += start_addr;
 	snd_emu10k1_pcm_init_voice(emu, 1, 1, epcm->extra,
 				   start_addr, end_addr);
+	start_addr = epcm->start_addr;
 	end_addr = epcm->start_addr + snd_pcm_lib_buffer_bytes(substream);
 	snd_emu10k1_pcm_init_voice(emu, 1, 0, epcm->voices[0],
 				   start_addr, end_addr);
@@ -598,37 +615,39 @@
 	return 0;
 }
 
-static void snd_emu10k1_playback_invalidate_cache(emu10k1_t *emu, emu10k1_voice_t *evoice)
+static void snd_emu10k1_playback_invalidate_cache(emu10k1_t *emu, int extra, emu10k1_voice_t *evoice)
 {
 	snd_pcm_runtime_t *runtime;
-	unsigned int voice, i, ccis, cra = 64, cs, sample;
+	unsigned int voice, stereo, i, ccis, cra = 64, cs, sample;
 
 	if (evoice == NULL)
 		return;
 	runtime = evoice->epcm->substream->runtime;
 	voice = evoice->number;
+	stereo = (!extra && runtime->channels == 2);
 	sample = snd_pcm_format_width(runtime->format) == 16 ? 0 : 0x80808080;
-	if (runtime->channels == 2) {
-		ccis = 28;
-		cs = 4;
-	} else {
-		ccis = 30;
-		cs = 2;
-	}
-	if (sample == 0)	/* 16-bit */
-		ccis *= 2;
-	for (i = 0; i < cs; i++)
+	ccis = emu10k1_ccis(stereo, sample == 0);
+	// set cs to 2 * number of cache registers beside the invalidated
+	cs = (sample == 0) ? (32-ccis) : (64-ccis+1) >> 1;
+	if (cs > 16) cs = 16;
+	for (i = 0; i < cs; i++) {
 		snd_emu10k1_ptr_write(emu, CD0 + i, voice, sample);
-		
+		if (stereo) {
+			snd_emu10k1_ptr_write(emu, CD0 + i, voice + 1, sample);
+		}
+	}
 	// reset cache
 	snd_emu10k1_ptr_write(emu, CCR_CACHEINVALIDSIZE, voice, 0);
 	snd_emu10k1_ptr_write(emu, CCR_READADDRESS, voice, cra);
-	if (runtime->channels == 2) {
+	if (stereo) {
 		snd_emu10k1_ptr_write(emu, CCR_CACHEINVALIDSIZE, voice + 1, 0);
 		snd_emu10k1_ptr_write(emu, CCR_READADDRESS, voice + 1, cra);
 	}
 	// fill cache
 	snd_emu10k1_ptr_write(emu, CCR_CACHEINVALIDSIZE, voice, ccis);
+	if (stereo) {
+		snd_emu10k1_ptr_write(emu, CCR_CACHEINVALIDSIZE, voice+1, ccis);
+	}
 }
 
 static void snd_emu10k1_playback_prepare_voice(emu10k1_t *emu, emu10k1_voice_t *evoice, int master, int extra)
@@ -707,8 +726,8 @@
 	spin_lock(&emu->reg_lock);
 	switch (cmd) {
 	case SNDRV_PCM_TRIGGER_START:
-		snd_emu10k1_playback_invalidate_cache(emu, epcm->extra);	/* do we need this? */
-		snd_emu10k1_playback_invalidate_cache(emu, epcm->voices[0]);
+		snd_emu10k1_playback_invalidate_cache(emu, 1, epcm->extra);	/* do we need this? */
+		snd_emu10k1_playback_invalidate_cache(emu, 0, epcm->voices[0]);
 		/* follow thru */
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:
 		snd_emu10k1_playback_prepare_voice(emu, epcm->voices[0], 1, 0);
@@ -836,9 +855,9 @@
 	case SNDRV_PCM_TRIGGER_START:
 		// prepare voices
 		for (i = 0; i < NUM_EFX_PLAYBACK; i++) {	
-			snd_emu10k1_playback_invalidate_cache(emu, epcm->voices[i]);
+			snd_emu10k1_playback_invalidate_cache(emu, 0, epcm->voices[i]);
 		}
-		snd_emu10k1_playback_invalidate_cache(emu, epcm->extra);
+		snd_emu10k1_playback_invalidate_cache(emu, 1, epcm->extra);
 
 		/* follow thru */
 	case SNDRV_PCM_TRIGGER_PAUSE_RELEASE:

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Re: [PATCH] remove emu10k1 pops/clicks at the     beginning and end of playback (fwd)
  2005-03-05 19:08             ` Jindrich Makovicka
@ 2005-03-05 20:25               ` Lee Revell
  2005-03-05 21:17                 ` Jindrich Makovicka
  2005-03-08 16:11               ` Takashi Iwai
  1 sibling, 1 reply; 12+ messages in thread
From: Lee Revell @ 2005-03-05 20:25 UTC (permalink / raw)
  To: Jindrich Makovicka; +Cc: alsa-devel

On Sat, 2005-03-05 at 20:08 +0100, Jindrich Makovicka wrote:
> Here is one more update of the patch. This version clears only a part of 
> the cache, saving some register writes. Tested for few hours, no pops so 
> far. But it might be better to use brute force anyway.
> 

Tested this on my SBLive, it definitely eliminates the pop at the end of
playback for 44100Hz mono samples.  Have you tested with a wide variety
of sample rates?

The emu10k1 supports continous sample rates from 0 to 192KHz so you
could never test them all.  But I think testing all of the sample rates
supported by the capture device would work.

Lee



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] remove emu10k1 pops/clicks at the        beginning and end of playback (fwd)
  2005-03-05 20:25               ` Lee Revell
@ 2005-03-05 21:17                 ` Jindrich Makovicka
  0 siblings, 0 replies; 12+ messages in thread
From: Jindrich Makovicka @ 2005-03-05 21:17 UTC (permalink / raw)
  To: alsa-devel

Lee Revell wrote:
> On Sat, 2005-03-05 at 20:08 +0100, Jindrich Makovicka wrote:
> 
>>Here is one more update of the patch. This version clears only a part of 
>>the cache, saving some register writes. Tested for few hours, no pops so 
>>far. But it might be better to use brute force anyway.
> 
> Tested this on my SBLive, it definitely eliminates the pop at the end of
> playback for 44100Hz mono samples.  Have you tested with a wide variety
> of sample rates?
> 
> The emu10k1 supports continous sample rates from 0 to 192KHz so you
> could never test them all.  But I think testing all of the sample rates
> supported by the capture device would work.

Tested for all sample rates specified in capture_rates (8000, 11025, 
16000, 22050, 24000, 32000, 44100, 48000) plus couple of others in the 
range from 4kHz to 192kHz, everything for stereo/mono/8/16bit. No 
audible popping.

-- 
Jindrich Makovicka



-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: Re: [PATCH] remove emu10k1 pops/clicks at the       beginning and end of playback (fwd)
  2005-03-05 19:08             ` Jindrich Makovicka
  2005-03-05 20:25               ` Lee Revell
@ 2005-03-08 16:11               ` Takashi Iwai
  1 sibling, 0 replies; 12+ messages in thread
From: Takashi Iwai @ 2005-03-08 16:11 UTC (permalink / raw)
  To: Jindrich Makovicka; +Cc: alsa-devel

At Sat, 05 Mar 2005 20:08:40 +0100,
Jindrich Makovicka wrote:
> 
> Here is one more update of the patch. This version clears only a part of 
> the cache, saving some register writes. Tested for few hours, no pops so 
> far. But it might be better to use brute force anyway.

I'll apply this to CVS.

Could you give the signed-off-by line to add your credit to changelog?


thanks,

Takashi


-------------------------------------------------------
SF email is sponsored by - The IT Product Guide
Read honest & candid reviews on hundreds of IT Products from real users.
Discover which products truly live up to the hype. Start reading now.
http://ads.osdn.com/?ad_id=6595&alloc_id=14396&op=click

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2005-03-08 16:11 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2005-03-04  8:33 [PATCH] remove emu10k1 pops/clicks at the beginning and end of playback (fwd) Jaroslav Kysela
2005-03-04 18:17 ` Lee Revell
2005-03-05  8:29   ` Jindrich Makovicka
2005-03-05 11:05     ` James Courtier-Dutton
2005-03-05 11:46       ` Jindrich Makovicka
2005-03-05 14:37         ` James Courtier-Dutton
2005-03-05 14:46           ` Jindrich Makovicka
2005-03-05 19:08             ` Jindrich Makovicka
2005-03-05 20:25               ` Lee Revell
2005-03-05 21:17                 ` Jindrich Makovicka
2005-03-08 16:11               ` Takashi Iwai
2005-03-05 18:39         ` Lee Revell

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.