All of lore.kernel.org
 help / color / mirror / Atom feed
* intel8x0 has stopped working.
@ 2004-02-02 17:41 James Courtier-Dutton
  2004-02-02 17:59 ` Takashi Iwai
  0 siblings, 1 reply; 21+ messages in thread
From: James Courtier-Dutton @ 2004-02-02 17:41 UTC (permalink / raw)
  To: ALSA development

In alsa 0.9.8, the snd-intel8x0 worked find with my MB. An ICH5 with ALC650.
With alsa 1.0.2, I get: -
device: front -> no-sound output
device: surround40: -> Only sound on rear left, and rear right.
device: surround51: -> Only sound on rear left, and rear right, center, lfe

So, in all cases there is never sound on the front speakers.

The same problem has happened to a person with an ICH2, which only has 
front speakers, so they get no sound at all.

I am investigating, and I suspect that the mixer has just lost the 
mixer/mute for the front speakers.

Is there a tool where I can adjust ac97 registers directly through a 
IOCTL or something, for test purposes?

Cheers
James



-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

* Re: intel8x0 has stopped working.
  2004-02-02 17:41 intel8x0 has stopped working James Courtier-Dutton
@ 2004-02-02 17:59 ` Takashi Iwai
  2004-02-02 19:34   ` Prakash K. Cheemplavam
  2004-02-02 19:42   ` James Courtier-Dutton
  0 siblings, 2 replies; 21+ messages in thread
From: Takashi Iwai @ 2004-02-02 17:59 UTC (permalink / raw)
  To: James Courtier-Dutton; +Cc: ALSA development

At Mon, 02 Feb 2004 17:41:02 +0000,
James Courtier-Dutton wrote:
> 
> In alsa 0.9.8, the snd-intel8x0 worked find with my MB. An ICH5 with ALC650.
> With alsa 1.0.2, I get: -
> device: front -> no-sound output
> device: surround40: -> Only sound on rear left, and rear right.
> device: surround51: -> Only sound on rear left, and rear right, center, lfe
> 
> So, in all cases there is never sound on the front speakers.
> 
> The same problem has happened to a person with an ICH2, which only has 
> front speakers, so they get no sound at all.
> 
> I am investigating, and I suspect that the mixer has just lost the 
> mixer/mute for the front speakers.

hmm, it looks like a generic problem of intel8x0 with ALC65x.
in the recent version, the detection and implementation of PCM streams
have been changed, and i guess it's broken for these codecs.


> Is there a tool where I can adjust ac97 registers directly through a 
> IOCTL or something, for test purposes?

	% cat /proc/asound/card0/codec97#0/ac97#0-0+regs

we'll need to trace the behavior in snd_ac97_pcm_*() functions...


Takashi


-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

* Re: intel8x0 has stopped working.
  2004-02-02 17:59 ` Takashi Iwai
@ 2004-02-02 19:34   ` Prakash K. Cheemplavam
  2004-02-02 19:42   ` James Courtier-Dutton
  1 sibling, 0 replies; 21+ messages in thread
From: Prakash K. Cheemplavam @ 2004-02-02 19:34 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: James Courtier-Dutton, ALSA development

> hmm, it looks like a generic problem of intel8x0 with ALC65x.
> in the recent version, the detection and implementation of PCM streams
> have been changed, and i guess it's broken for these codecs.

I would think the same. I told you/alsa list about oss emulation with 
mmap and quake3 problem (since first 1.0.0pre) as quake now detecs pcm 
rate of 48kHz, which was 22kHz before. (48kHz with quake3 didn't work in 
0.9.x alsa, as well.) Alsa release 1.0.2 seems to break several things, 
like xmms won't play with alsa using mmap (works with alsa1.0.1). Arts 
doesn't play with 48000kHz in xmms, and in 44,1kHz I get distorted 
sound. (I guess this was with earlier version, as well.) There were some 
other things I forgot.

I have ALC650, btw.

Dunno if it of help:

0-0/0: Realtek ALC650 rev 3

Capabilities     :
DAC resolution   : 20-bit
ADC resolution   : 18-bit
3D enhancement   : Realtek 3D Stereo Enhancement

Current setup
Mic gain         : +20dB [+20dB]
POP path         : pre 3D
Sim. stereo      : off
3D enhancement   : off
Loudness         : off
Mono output      : MIX
Mic select       : Mic1
ADC/DAC loopback : off
Extended ID      : codec=0 rev=1 LDAC SDAC CDAC DSA=0 SPDIF DRA VRA
Extended status  : SPCV LDAC SDAC CDAC SPDIF=3/4 VRA
PCM front DAC    : 48000Hz
PCM Surr DAC     : 48000Hz
PCM LFE DAC      : 48000Hz
PCM ADC          : 48000Hz
SPDIF Control    : Consumer PCM Category=0x2 Generation=1 Rate=48kHz
SPDIF In Status  : Not Locked

bye,

Prakash


-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

* Re: intel8x0 has stopped working.
  2004-02-02 17:59 ` Takashi Iwai
  2004-02-02 19:34   ` Prakash K. Cheemplavam
@ 2004-02-02 19:42   ` James Courtier-Dutton
  2004-02-02 23:12     ` [PATCH] Fixes: " James Courtier-Dutton
  2004-02-04 19:35     ` Takashi Iwai
  1 sibling, 2 replies; 21+ messages in thread
From: James Courtier-Dutton @ 2004-02-02 19:42 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: ALSA development

Takashi Iwai wrote:
> At Mon, 02 Feb 2004 17:41:02 +0000,
> James Courtier-Dutton wrote:
> 
>>In alsa 0.9.8, the snd-intel8x0 worked find with my MB. An ICH5 with ALC650.
>>With alsa 1.0.2, I get: -
>>device: front -> no-sound output
>>device: surround40: -> Only sound on rear left, and rear right.
>>device: surround51: -> Only sound on rear left, and rear right, center, lfe
>>
>>So, in all cases there is never sound on the front speakers.
>>
>>The same problem has happened to a person with an ICH2, which only has 
>>front speakers, so they get no sound at all.
>>
>>I am investigating, and I suspect that the mixer has just lost the 
>>mixer/mute for the front speakers.
> 
> 
> hmm, it looks like a generic problem of intel8x0 with ALC65x.
> in the recent version, the detection and implementation of PCM streams
> have been changed, and i guess it's broken for these codecs.
> 
> 
> 
>>Is there a tool where I can adjust ac97 registers directly through a 
>>IOCTL or something, for test purposes?
> 
> 
> 	% cat /proc/asound/card0/codec97#0/ac97#0-0+regs
> 
> we'll need to trace the behavior in snd_ac97_pcm_*() functions...
> 
> 
> Takashi
> 
> 
Ok, the front channel is now working, but I don't know why.
One thing that is definitely broken is sample rate conversion.
E.g. Open card at rate 44100, device="hw:0", fails saying rate 44100 not 
supported.
Open card at rate 48000, device="hw:0" works.
Now, open card at rate 44100, device "plughw:0" succeeds, but then the 
snd_pcm_writei() calls just hangs at poll()

#0  0x401a5ab8 in *__GI___poll (fds=0x402049a8, nfds=1, timeout=-1) at 
../sysdeps/unix/sysv/linux/poll.c:82
#1  0x4006abd4 in snd_pcm_wait (pcm=0x8054630, timeout=-4) at pcm.c:2044
#2  0x400706f8 in snd_pcm_write_areas (pcm=0x8054630, areas=0xbfffe9d4, 
offset=0, size=71, func=0x4007a480 <snd_pcm_plugin_write_areas>)
     at pcm.c:6041
#3  0x4007a88d in snd_pcm_plugin_writei (pcm=0x8054630, 
buffer=0xfffffffc, size=4294967292) at pcm_plugin.c:436
#4  0x40068dd7 in snd_pcm_writei (pcm=0x7fffffff, buffer=0x8054810, 
size=1) at pcm_local.h:368
#5  0x080495e0 in write_loop (handle=0x80544b8, channel=0, periods=3105, 
samples=0x8054810) at speaker-test.c:289
#6  0x08049b73 in main (argc=4, argv=0xbfffee54) at speaker-test.c:445


Once thing I have noticed, is that with the alc650, we used to have VRA 
(alsa 0.9.8), but the 1.0.2 intel8x0 driver ignores the VRA and fixes 
itself at 48000.
So, before I never needed the sample rate converters, but as it now 
fixes the rate at 48000, I need the sample rate converters, and they 
don't actually work.





-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

* [PATCH] Fixes: Re: intel8x0 has stopped working.
  2004-02-02 19:42   ` James Courtier-Dutton
@ 2004-02-02 23:12     ` James Courtier-Dutton
  2004-02-03 16:22       ` Jaroslav Kysela
  2004-02-04 19:35     ` Takashi Iwai
  1 sibling, 1 reply; 21+ messages in thread
From: James Courtier-Dutton @ 2004-02-02 23:12 UTC (permalink / raw)
  To: James Courtier-Dutton; +Cc: Takashi Iwai, ALSA development

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

James Courtier-Dutton wrote:
> 
> Once thing I have noticed, is that with the alc650, we used to have VRA 
> (alsa 0.9.8), but the 1.0.2 intel8x0 driver ignores the VRA and fixes 
> itself at 48000.
> So, before I never needed the sample rate converters, but as it now 
> fixes the rate at 48000, I need the sample rate converters, and they 
> don't actually work.
> 
> 

I have found out what the problem is with the resamplers not working.
I was using: -
err = snd_pcm_sw_params_set_start_threshold(handle, swparams, 
buffer_size);  <- note buffer_size.

So, the state will stay in SND_PCM_STATE_PREPARED state until the buffer 
is full.
Inside snd_pcm_write_areas() [in alsa-lib] we have a snd_pcm_wait 
statement before we write frames into the buffer.
if (((snd_pcm_uframes_t)avail < pcm->avail_min && size > 
(snd_pcm_uframes_t)avail) || (size >= pcm->xfer_align && 
(snd_pcm_uframes_t)avail < pcm->xfer_align)) {
then it calls snd_pcm_wait()

So, this will call snd_pcm_wait() until avail >= pcm->avail_min.
This will only become true if we are in SND_PCM_STATE_RUNNING state.

below the snd_pcm_wait(), we do try to execute snd_pcm_start() to get us 
into SND_PCM_STATE_RUNNING, but this only gets executed if: -
if (state == SND_PCM_STATE_PREPARED && hw_avail >= (snd_pcm_sframes_t) 
pcm->start_threshold) {

So, we will only reach a running state if the buffer is FULL, but the 
buffer will NEVER become full, because of the previous snd_pcm_wait();
CATCH22!!!

I think a fix to this would be to limit as follows: -
start_threshold <= (buffer_size - avail_min)

I have fixed the issue for now, just by setting the "start_threshold" to 
something other than buffer_size. But I think some work needs to be done 
with alsa-lib to prevent this deadlock situation ever happening in alsa-lib.

 From the application writers point of view, if the buffer is X bytes 
long, and one used snd_pcm_writei() to write X+10 bytes, the buffer 
should become full. Currently, that is not always the case, and alsa-lib 
instead locks in snd_pcm_wait().

I attach a patch that fixes alsa-lib.
All it does it call snd_pcm_start() just before snd_pcm_wait() and due 
to the if statement just above it, will only happen when the buffer is 
going to be filled, therefore ensuring that even if "start_threshold" is 
set too high, snd_pcm_start() will always be called if the buffer is 
full, or about to become full.

Cheers
James

[-- Attachment #2: alsa-lib-lockup-fix.diff --]
[-- Type: text/x-patch, Size: 535 bytes --]

--- src/pcm/pcm.c.old	2004-02-02 23:07:14.355587424 +0000
+++ src/pcm/pcm.c	2004-02-02 23:07:19.871748840 +0000
@@ -6033,6 +6033,11 @@
 		} else if (((snd_pcm_uframes_t)avail < pcm->avail_min && size > (snd_pcm_uframes_t)avail) ||
 		           (size >= pcm->xfer_align && (snd_pcm_uframes_t)avail < pcm->xfer_align)) {
 
+			if (state == SND_PCM_STATE_PREPARED ) {
+				err = snd_pcm_start(pcm);
+				if (err < 0)
+					goto _end;
+                        }
 			if (pcm->mode & SND_PCM_NONBLOCK) {
 				err = -EAGAIN;
 				goto _end;

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

* Re: [PATCH] Fixes: Re: intel8x0 has stopped working.
  2004-02-02 23:12     ` [PATCH] Fixes: " James Courtier-Dutton
@ 2004-02-03 16:22       ` Jaroslav Kysela
  2004-02-03 22:21         ` James Courtier-Dutton
  0 siblings, 1 reply; 21+ messages in thread
From: Jaroslav Kysela @ 2004-02-03 16:22 UTC (permalink / raw)
  To: James Courtier-Dutton; +Cc: ALSA development

On Mon, 2 Feb 2004, James Courtier-Dutton wrote:

> James Courtier-Dutton wrote:
> > 
> > Once thing I have noticed, is that with the alc650, we used to have VRA 
> > (alsa 0.9.8), but the 1.0.2 intel8x0 driver ignores the VRA and fixes 
> > itself at 48000.
> > So, before I never needed the sample rate converters, but as it now 
> > fixes the rate at 48000, I need the sample rate converters, and they 
> > don't actually work.
> > 
> > 
> 
> I have found out what the problem is with the resamplers not working.
> I was using: -
> err = snd_pcm_sw_params_set_start_threshold(handle, swparams, 
> buffer_size);  <- note buffer_size.
> 
> So, the state will stay in SND_PCM_STATE_PREPARED state until the buffer 
> is full.
> Inside snd_pcm_write_areas() [in alsa-lib] we have a snd_pcm_wait 
> statement before we write frames into the buffer.
> if (((snd_pcm_uframes_t)avail < pcm->avail_min && size > 
> (snd_pcm_uframes_t)avail) || (size >= pcm->xfer_align && 
> (snd_pcm_uframes_t)avail < pcm->xfer_align)) {
> then it calls snd_pcm_wait()
> 
> So, this will call snd_pcm_wait() until avail >= pcm->avail_min.
> This will only become true if we are in SND_PCM_STATE_RUNNING state.
> 
> below the snd_pcm_wait(), we do try to execute snd_pcm_start() to get us 
> into SND_PCM_STATE_RUNNING, but this only gets executed if: -
> if (state == SND_PCM_STATE_PREPARED && hw_avail >= (snd_pcm_sframes_t) 
> pcm->start_threshold) {
> 
> So, we will only reach a running state if the buffer is FULL, but the 
> buffer will NEVER become full, because of the previous snd_pcm_wait();
> CATCH22!!!
> 
> I think a fix to this would be to limit as follows: -
> start_threshold <= (buffer_size - avail_min)
> 
> I have fixed the issue for now, just by setting the "start_threshold" to 
> something other than buffer_size. But I think some work needs to be done 
> with alsa-lib to prevent this deadlock situation ever happening in alsa-lib.
> 
>  From the application writers point of view, if the buffer is X bytes 
> long, and one used snd_pcm_writei() to write X+10 bytes, the buffer 
> should become full. Currently, that is not always the case, and alsa-lib 
> instead locks in snd_pcm_wait().
> 
> I attach a patch that fixes alsa-lib.
> All it does it call snd_pcm_start() just before snd_pcm_wait() and due 
> to the if statement just above it, will only happen when the buffer is 
> going to be filled, therefore ensuring that even if "start_threshold" is 
> set too high, snd_pcm_start() will always be called if the buffer is 
> full, or about to become full.

I don't think that this is a right fix. The programmer must know what is 
doing and the manual start is still alternative. You must set 
start_threshold to (buffer_size / period_size) * period_size in your case.

						Jaroslav

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


-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

* Re: [PATCH] Fixes: Re: intel8x0 has stopped working.
  2004-02-03 16:22       ` Jaroslav Kysela
@ 2004-02-03 22:21         ` James Courtier-Dutton
  2004-02-05 11:01           ` Jaroslav Kysela
  0 siblings, 1 reply; 21+ messages in thread
From: James Courtier-Dutton @ 2004-02-03 22:21 UTC (permalink / raw)
  To: Jaroslav Kysela; +Cc: ALSA development

Jaroslav Kysela wrote:
> On Mon, 2 Feb 2004, James Courtier-Dutton wrote:
> 
> 
>>James Courtier-Dutton wrote:
>>
>>>Once thing I have noticed, is that with the alc650, we used to have VRA 
>>>(alsa 0.9.8), but the 1.0.2 intel8x0 driver ignores the VRA and fixes 
>>>itself at 48000.
>>>So, before I never needed the sample rate converters, but as it now 
>>>fixes the rate at 48000, I need the sample rate converters, and they 
>>>don't actually work.
>>>
>>>
>>
>>I have found out what the problem is with the resamplers not working.
>>I was using: -
>>err = snd_pcm_sw_params_set_start_threshold(handle, swparams, 
>>buffer_size);  <- note buffer_size.
>>
>>So, the state will stay in SND_PCM_STATE_PREPARED state until the buffer 
>>is full.
>>Inside snd_pcm_write_areas() [in alsa-lib] we have a snd_pcm_wait 
>>statement before we write frames into the buffer.
>>if (((snd_pcm_uframes_t)avail < pcm->avail_min && size > 
>>(snd_pcm_uframes_t)avail) || (size >= pcm->xfer_align && 
>>(snd_pcm_uframes_t)avail < pcm->xfer_align)) {
>>then it calls snd_pcm_wait()
>>
>>So, this will call snd_pcm_wait() until avail >= pcm->avail_min.
>>This will only become true if we are in SND_PCM_STATE_RUNNING state.
>>
>>below the snd_pcm_wait(), we do try to execute snd_pcm_start() to get us 
>>into SND_PCM_STATE_RUNNING, but this only gets executed if: -
>>if (state == SND_PCM_STATE_PREPARED && hw_avail >= (snd_pcm_sframes_t) 
>>pcm->start_threshold) {
>>
>>So, we will only reach a running state if the buffer is FULL, but the 
>>buffer will NEVER become full, because of the previous snd_pcm_wait();
>>CATCH22!!!
>>
>>I think a fix to this would be to limit as follows: -
>>start_threshold <= (buffer_size - avail_min)
>>
>>I have fixed the issue for now, just by setting the "start_threshold" to 
>>something other than buffer_size. But I think some work needs to be done 
>>with alsa-lib to prevent this deadlock situation ever happening in alsa-lib.
>>
>> From the application writers point of view, if the buffer is X bytes 
>>long, and one used snd_pcm_writei() to write X+10 bytes, the buffer 
>>should become full. Currently, that is not always the case, and alsa-lib 
>>instead locks in snd_pcm_wait().
>>
>>I attach a patch that fixes alsa-lib.
>>All it does it call snd_pcm_start() just before snd_pcm_wait() and due 
>>to the if statement just above it, will only happen when the buffer is 
>>going to be filled, therefore ensuring that even if "start_threshold" is 
>>set too high, snd_pcm_start() will always be called if the buffer is 
>>full, or about to become full.
> 
> 
> I don't think that this is a right fix. The programmer must know what is 
> doing and the manual start is still alternative. You must set 
> start_threshold to (buffer_size / period_size) * period_size in your case.
> 
> 						Jaroslav
> 
A don't think this is an application programmer bug. After all your 
program alsa-lib/test/pcm.c had it set to start_threshold==buffer_size, 
and you are one of the main alsa developers. So if you thought that was 
an OK thing to set, why won't other people?
My point is, I don't think setting start_threshold to buffer_size is 
even "wrong" at all. Some people might want the buffer to be full before 
it starts, and my patch allows for that.

I don't think my patch breaks anything.
Maybe you should look deeper, and work out exactly which situation it 
will happen in.
My patch only executes snd_pcm_start() if the previous snd_pcm_writei() 
would in fact fill the buffer.

You have a snd_pcm_start() later on that executes if one passes the 
start_threshold.

The only situation it would break is if the application wants to fill 
the buffer and for some reason not start playing. If you conceptually 
want to allow for that, then I will accept that my patch breaks that 
behaviour.

Cheers
James


-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

* Re: intel8x0 has stopped working.
  2004-02-02 19:42   ` James Courtier-Dutton
  2004-02-02 23:12     ` [PATCH] Fixes: " James Courtier-Dutton
@ 2004-02-04 19:35     ` Takashi Iwai
  2004-02-04 23:03       ` James Courtier-Dutton
  1 sibling, 1 reply; 21+ messages in thread
From: Takashi Iwai @ 2004-02-04 19:35 UTC (permalink / raw)
  To: James Courtier-Dutton; +Cc: ALSA development

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

At Mon, 02 Feb 2004 19:42:37 +0000,
James Courtier-Dutton wrote:
> 
> Once thing I have noticed, is that with the alc650, we used to have VRA 
> (alsa 0.9.8), but the 1.0.2 intel8x0 driver ignores the VRA and fixes 
> itself at 48000.

yes, the detection of sample rate range seems broken for some codecs.
it was completely rewritten using the generic ac97_pcm.c.

could you check the debug messages with the attached patch?


Takashi

[-- Attachment #2: Type: text/plain, Size: 4002 bytes --]

Index: alsa-kernel/pci/ac97/ac97_pcm.c
===================================================================
RCS file: /suse/tiwai/cvs/alsa/alsa-kernel/pci/ac97/ac97_pcm.c,v
retrieving revision 1.10
diff -u -r1.10 ac97_pcm.c
--- alsa-kernel/pci/ac97/ac97_pcm.c	2 Jan 2004 13:39:33 -0000	1.10
+++ alsa-kernel/pci/ac97/ac97_pcm.c	4 Feb 2004 19:32:50 -0000
@@ -284,6 +284,7 @@
 		if (ac97_is_rev22(ac97)) {
 			/* Note: it's simply emulation of AMAP behaviour */
 			u16 es;
+			printk(KERN_DEBUG "get_pslots: rev22 amap emu\n");
 			es = ac97->regs[AC97_EXTENDED_STATUS] &= ~AC97_EI_DACS_SLOT_MASK;
 			switch (ac97->addr) {
 			case 1:
@@ -292,6 +293,7 @@
 			}
 			snd_ac97_write_cache(ac97, AC97_EXTENDED_STATUS, es);
 		}
+		printk(KERN_DEBUG "get_pslots: AMAP: addr=%d, scaps=0x%x, ext_id=0x%x\n", ac97->addr, ac97->scaps, ac97->ext_id);
 		switch (ac97->addr) {
 		case 0:
 			slots |= (1<<AC97_SLOT_PCM_LEFT)|(1<<AC97_SLOT_PCM_RIGHT);
@@ -332,6 +334,7 @@
 		return slots;
 	} else {
 		unsigned short slots;
+		printk(KERN_DEBUG "get_pslots: others: addr=%d, scaps=0x%x, ext_id=0x%x\n", ac97->addr, ac97->scaps, ac97->ext_id);
 		slots = (1<<AC97_SLOT_PCM_LEFT)|(1<<AC97_SLOT_PCM_RIGHT);
 		if (ac97->scaps & AC97_SCAP_SURROUND_DAC)
 			slots |= (1<<AC97_SLOT_PCM_SLEFT)|(1<<AC97_SLOT_PCM_SRIGHT);
@@ -420,16 +423,19 @@
 			continue;
 		avail_slots[0][i] = get_pslots(codec, &rate_table[0][i], &spdif_slots[i]);
 		avail_slots[1][i] = get_cslots(codec);
+		printk(KERN_DEBUG "checking codec %d, slots = 0x%x / 0x%x\n", i, avail_slots[0][i], avail_slots[1][i]);
 		if (!(codec->scaps & AC97_SCAP_INDEP_SDIN)) {
 			for (j = 0; j < i; j++) {
 				if (bus->codec[j])
 					avail_slots[1][i] &= ~avail_slots[1][j];
 			}
+			printk(KERN_DEBUG "-> capture slots = 0x%x\n", avail_slots[1][i]);
 		}
 	}
 	/* FIXME: add double rate allocation */
 	/* first step - exclusive devices */
 	for (i = 0; i < pcms_count; i++) {
+		printk(KERN_DEBUG "probing pcm %d\n", i);
 		pcm = &pcms[i];
 		rpcm = &rpcms[i];
 		/* low-level driver thinks that it's more clever */
@@ -452,22 +458,27 @@
 				tmp = spdif_slots[j];
 			else
 				tmp = avail_slots[pcm->stream][j];
+			printk(KERN_DEBUG ".. probing codec %d, slots = 0x%x, tmp = 0x%x\n", j, slots, tmp);
 			if (pcm->exclusive) {
 				/* exclusive access */
 				tmp &= slots;
+				printk(KERN_DEBUG ".. exclusive tmp = 0x%x\n", tmp);
 				for (k = 0; k < i; k++) {
 					if (rpcm->stream == rpcms[k].stream)
 						tmp &= ~rpcms[k].r[0].rslots[j];
 				}
+				printk(KERN_DEBUG "..... tmp = 0x%x\n", tmp);
 			} else {
 				/* non-exclusive access */
 				tmp &= pcm->r[0].slots;
+				printk(KERN_DEBUG ".. non-exclusive tmp = 0x%x\n", tmp);
 			}
 			if (tmp) {
 				rpcm->r[0].rslots[j] = tmp;
 				rpcm->r[0].codec[j] = bus->codec[j];
 				rpcm->r[0].rate_table[j] = rate_table[pcm->stream][j];
 				rates = get_rates(rpcm, j, tmp, 0);
+				printk(KERN_DEBUG ".. rslots = 0x%x, rate_table = %d, rates = 0x%x\n", tmp, rpcm->r[0].rate_table[j], rates);
 				if (pcm->exclusive)
 					avail_slots[pcm->stream][j] &= ~tmp;
 			}
@@ -475,6 +486,7 @@
 			rpcm->r[0].slots |= tmp;
 			rpcm->rates &= rates;
 		}
+		printk(KERN_DEBUG "--> slots = 0x%x, rates = 0x%x\n", rpcm->r[0].slots, rpcm->rates);
 		if (rpcm->rates == ~0)
 			rpcm->rates = 0; /* not used */
 	}
@@ -501,6 +513,7 @@
 	unsigned char reg;
 	int err = 0;
 
+	printk(KERN_DEBUG "ac97_pcm_open: rate = %d, cfg = %d, slots = 0x%x\n", rate, cfg, slots);
 	if (rate > 48000)	/* FIXME: add support for double rate */
 		return -EINVAL;
 	bus = pcm->bus;
@@ -549,7 +562,7 @@
 				}
 				if (reg_ok & (1 << (reg - AC97_PCM_FRONT_DAC_RATE)))
 					continue;
-				//printk(KERN_DEBUG "setting ac97 reg 0x%x to rate %d\n", reg, rate);
+				printk(KERN_DEBUG "setting ac97 reg 0x%x to rate %d\n", reg, rate);
 				err = snd_ac97_set_rate(pcm->r[r].codec[cidx], reg, rate);
 				if (err < 0)
 					snd_printk(KERN_ERR "error in snd_ac97_set_rate: cidx=%d, reg=0x%x, rate=%d, err=%d\n", cidx, reg, rate, err);

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

* Re: intel8x0 has stopped working.
  2004-02-04 19:35     ` Takashi Iwai
@ 2004-02-04 23:03       ` James Courtier-Dutton
  2004-02-05 18:49         ` Takashi Iwai
  2004-02-05 19:42         ` Takashi Iwai
  0 siblings, 2 replies; 21+ messages in thread
From: James Courtier-Dutton @ 2004-02-04 23:03 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: ALSA development

Takashi Iwai wrote:
> At Mon, 02 Feb 2004 19:42:37 +0000,
> James Courtier-Dutton wrote:
> 
>>Once thing I have noticed, is that with the alc650, we used to have VRA 
>>(alsa 0.9.8), but the 1.0.2 intel8x0 driver ignores the VRA and fixes 
>>itself at 48000.
> 
> 
> yes, the detection of sample rate range seems broken for some codecs.
> it was completely rewritten using the generic ac97_pcm.c.
> 
> could you check the debug messages with the attached patch?
> 
> 
> Takashi
> 

Here is the output as requested.
One thing that is good though, is this allowed me to do lots of testing 
for people that have codecs with fixed rates. If you do get my alc650 
codec working again with variable rates, please make sure there is some 
sort of kernel option to fix it to a single rate, for test purposes.

Feb  4 23:00:21 new kernel: PCI: Setting latency timer of device 
0000:00:1f.5 to 64
Feb  4 23:00:21 new kernel: get_pslots: rev22 amap emu
Feb  4 23:00:21 new kernel: get_pslots: AMAP: addr=0, scaps=0xd, 
ext_id=0x5c7
Feb  4 23:00:21 new kernel: checking codec 0, slots = 0x3d8 / 0x58
Feb  4 23:00:21 new kernel: -> capture slots = 0x58
Feb  4 23:00:21 new kernel: probing pcm 0
Feb  4 23:00:21 new kernel: .. probing codec 0, slots = 0x3d8, tmp = 0x3d8
Feb  4 23:00:21 new kernel: .. exclusive tmp = 0x3d8
Feb  4 23:00:21 new kernel: ..... tmp = 0x3d8
Feb  4 23:00:21 new kernel: .. rslots = 0x3d8, rate_table = 0, rates = 0x80
Feb  4 23:00:21 new kernel: --> slots = 0x3d8, rates = 0x80
Feb  4 23:00:21 new kernel: probing pcm 1
Feb  4 23:00:21 new kernel: .. probing codec 0, slots = 0x18, tmp = 0x58
Feb  4 23:00:21 new kernel: .. exclusive tmp = 0x18
Feb  4 23:00:21 new kernel: ..... tmp = 0x18
Feb  4 23:00:21 new kernel: .. rslots = 0x18, rate_table = 0, rates = 0xfe
Feb  4 23:00:21 new kernel: --> slots = 0x18, rates = 0xfe
Feb  4 23:00:21 new kernel: probing pcm 2
Feb  4 23:00:21 new kernel: .. probing codec 0, slots = 0x40, tmp = 0x40
Feb  4 23:00:21 new kernel: .. exclusive tmp = 0x40
Feb  4 23:00:21 new kernel: ..... tmp = 0x40
Feb  4 23:00:21 new kernel: .. rslots = 0x40, rate_table = 0, rates = 0x80
Feb  4 23:00:21 new kernel: --> slots = 0x40, rates = 0x80
Feb  4 23:00:21 new kernel: probing pcm 3
Feb  4 23:00:21 new kernel: .. probing codec 0, slots = 0xc00, tmp = 0xc00
Feb  4 23:00:21 new kernel: .. exclusive tmp = 0xc00
Feb  4 23:00:21 new kernel: ..... tmp = 0xc00
Feb  4 23:00:21 new kernel: .. rslots = 0xc00, rate_table = 0, rates = 0xe0
Feb  4 23:00:21 new kernel: --> slots = 0xc00, rates = 0xe0
Feb  4 23:00:21 new kernel: probing pcm 4
Feb  4 23:00:21 new kernel: .. probing codec 0, slots = 0x18, tmp = 0x0
Feb  4 23:00:21 new kernel: .. exclusive tmp = 0x0
Feb  4 23:00:21 new kernel: ..... tmp = 0x0
Feb  4 23:00:21 new kernel: --> slots = 0x0, rates = 0xffffffff
Feb  4 23:00:21 new kernel: probing pcm 5
Feb  4 23:00:21 new kernel: .. probing codec 0, slots = 0x40, tmp = 0x0
Feb  4 23:00:21 new kernel: .. exclusive tmp = 0x0
Feb  4 23:00:21 new kernel: ..... tmp = 0x0
Feb  4 23:00:21 new kernel: --> slots = 0x0, rates = 0xffffffff
Feb  4 23:00:21 new kernel: intel8x0: clocking to 48000



-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

* Re: [PATCH] Fixes: Re: intel8x0 has stopped working.
  2004-02-03 22:21         ` James Courtier-Dutton
@ 2004-02-05 11:01           ` Jaroslav Kysela
  2004-02-05 19:15             ` Glenn Maynard
  0 siblings, 1 reply; 21+ messages in thread
From: Jaroslav Kysela @ 2004-02-05 11:01 UTC (permalink / raw)
  To: James Courtier-Dutton; +Cc: ALSA development

On Tue, 3 Feb 2004, James Courtier-Dutton wrote:

> >>I attach a patch that fixes alsa-lib.
> >>All it does it call snd_pcm_start() just before snd_pcm_wait() and due 
> >>to the if statement just above it, will only happen when the buffer is 
> >>going to be filled, therefore ensuring that even if "start_threshold" is 
> >>set too high, snd_pcm_start() will always be called if the buffer is 
> >>full, or about to become full.
> > 
> > 
> > I don't think that this is a right fix. The programmer must know what is 
> > doing and the manual start is still alternative. You must set 
> > start_threshold to (buffer_size / period_size) * period_size in your case.
> > 
> > 						Jaroslav
> > 
> A don't think this is an application programmer bug. After all your 
> program alsa-lib/test/pcm.c had it set to start_threshold==buffer_size, 
> and you are one of the main alsa developers. So if you thought that was 
> an OK thing to set, why won't other people?

Yes, it was clearly my fault.

> My point is, I don't think setting start_threshold to buffer_size is 
> even "wrong" at all. Some people might want the buffer to be full before 
> it starts, and my patch allows for that.

It's not wrong semantics. I see - it's logical, but I don't want to follow
some rule as some API designers does - control magically some things. I
want that developer which uses our API knows what the library / driver
exactly does.

We have clear conditions when the stream is started. That's it.

						Jaroslav

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


-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

* Re: intel8x0 has stopped working.
  2004-02-04 23:03       ` James Courtier-Dutton
@ 2004-02-05 18:49         ` Takashi Iwai
  2004-02-05 19:42         ` Takashi Iwai
  1 sibling, 0 replies; 21+ messages in thread
From: Takashi Iwai @ 2004-02-05 18:49 UTC (permalink / raw)
  To: James Courtier-Dutton; +Cc: ALSA development

At Wed, 04 Feb 2004 23:03:50 +0000,
James Courtier-Dutton wrote:
> 
> One thing that is good though, is this allowed me to do lots of testing 
> for people that have codecs with fixed rates. If you do get my alc650 
> codec working again with variable rates, please make sure there is some 
> sort of kernel option to fix it to a single rate, for test purposes.

you can do always like

pcm.fixed48 {
	type plug
	slave.pcm hw
	slave.rate 48000
}


Takashi


-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

* Re: [PATCH] Fixes: Re: intel8x0 has stopped working.
  2004-02-05 11:01           ` Jaroslav Kysela
@ 2004-02-05 19:15             ` Glenn Maynard
  2004-02-05 19:25               ` Takashi Iwai
  2004-02-05 19:38               ` Jaroslav Kysela
  0 siblings, 2 replies; 21+ messages in thread
From: Glenn Maynard @ 2004-02-05 19:15 UTC (permalink / raw)
  To: ALSA development

On Thu, Feb 05, 2004 at 12:01:28PM +0100, Jaroslav Kysela wrote:
> > My point is, I don't think setting start_threshold to buffer_size is 
> > even "wrong" at all. Some people might want the buffer to be full before 
> > it starts, and my patch allows for that.
> 
> It's not wrong semantics. I see - it's logical, but I don't want to follow
> some rule as some API designers does - control magically some things. I
> want that developer which uses our API knows what the library / driver
> exactly does.
> 
> We have clear conditions when the stream is started. That's it.

If this isn't guaranteed to work, I'd suggest making it never work.
Otherwise, programs will work on some hardware and not others, which is
a case that should be minimized as much as possible; it's these kinds
of subtle differences that make it very hard to write reliable (sound,
video, etc) code.  I've had to play games with setting hardware settings:
always set the sample rate even if I don't care, use a 32k buffer size
and not a 4k or 8k one--in order to make it work on as many systems as
possible without failing mysteriously or triggering alsa-lib asserts.

(I don't quite understand why start_threashold == buffer_size doessn't
mean "start when the buffer is full", though.)

-- 
Glenn Maynard


-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

* Re: [PATCH] Fixes: Re: intel8x0 has stopped working.
  2004-02-05 19:15             ` Glenn Maynard
@ 2004-02-05 19:25               ` Takashi Iwai
  2004-02-05 19:38               ` Jaroslav Kysela
  1 sibling, 0 replies; 21+ messages in thread
From: Takashi Iwai @ 2004-02-05 19:25 UTC (permalink / raw)
  To: Glenn Maynard; +Cc: ALSA development

At Thu, 5 Feb 2004 14:15:22 -0500,
Glenn Maynard wrote:
> 
> On Thu, Feb 05, 2004 at 12:01:28PM +0100, Jaroslav Kysela wrote:
> > > My point is, I don't think setting start_threshold to buffer_size is 
> > > even "wrong" at all. Some people might want the buffer to be full before 
> > > it starts, and my patch allows for that.
> > 
> > It's not wrong semantics. I see - it's logical, but I don't want to follow
> > some rule as some API designers does - control magically some things. I
> > want that developer which uses our API knows what the library / driver
> > exactly does.
> > 
> > We have clear conditions when the stream is started. That's it.
> 
> If this isn't guaranteed to work, I'd suggest making it never work.
> Otherwise, programs will work on some hardware and not others, which is
> a case that should be minimized as much as possible; it's these kinds
> of subtle differences that make it very hard to write reliable (sound,
> video, etc) code.  I've had to play games with setting hardware settings:
> always set the sample rate even if I don't care, use a 32k buffer size
> and not a 4k or 8k one--in order to make it work on as many systems as
> possible without failing mysteriously or triggering alsa-lib asserts.
> 
> (I don't quite understand why start_threashold == buffer_size doessn't
> mean "start when the buffer is full", though.)

because the buffer size is not always aligned to the period size.

when the buffer size is not aligned to period size, the condition will
be never satisfied since start_threshold is checked only when
interrupt occurs (i.e. on period boundary).

maybe we can check such a condition and return an error...


Takashi


-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

* Re: [PATCH] Fixes: Re: intel8x0 has stopped working.
  2004-02-05 19:15             ` Glenn Maynard
  2004-02-05 19:25               ` Takashi Iwai
@ 2004-02-05 19:38               ` Jaroslav Kysela
  2004-02-05 19:58                 ` Glenn Maynard
  2004-02-05 20:44                 ` James Courtier-Dutton
  1 sibling, 2 replies; 21+ messages in thread
From: Jaroslav Kysela @ 2004-02-05 19:38 UTC (permalink / raw)
  To: Glenn Maynard; +Cc: ALSA development

On Thu, 5 Feb 2004, Glenn Maynard wrote:

> On Thu, Feb 05, 2004 at 12:01:28PM +0100, Jaroslav Kysela wrote:
> > > My point is, I don't think setting start_threshold to buffer_size is 
> > > even "wrong" at all. Some people might want the buffer to be full before 
> > > it starts, and my patch allows for that.
> > 
> > It's not wrong semantics. I see - it's logical, but I don't want to follow
> > some rule as some API designers does - control magically some things. I
> > want that developer which uses our API knows what the library / driver
> > exactly does.
> > 
> > We have clear conditions when the stream is started. That's it.
> 
> If this isn't guaranteed to work, I'd suggest making it never work.

It's guaranteed to work.

> Otherwise, programs will work on some hardware and not others, which is
> a case that should be minimized as much as possible; it's these kinds
> of subtle differences that make it very hard to write reliable (sound,
> video, etc) code.  I've had to play games with setting hardware settings:
> always set the sample rate even if I don't care, use a 32k buffer size
> and not a 4k or 8k one--in order to make it work on as many systems as
> possible without failing mysteriously or triggering alsa-lib asserts.

Send us bug-reports if you have problems. We have not a magic ball.

> (I don't quite understand why start_threashold == buffer_size doessn't
> mean "start when the buffer is full", though.)

Because we have the avail_min threshold which says that we don't want to
write new data when buffer can accept less samples than this threshold. So
if start_threshold is greather than '(buffer_size / avail_min) *
avail_min' expression, then stream won't be automatically started, because
we cannot fill data in read/write operations to satisfy the requirement
that start_threshold == buffer_size.

Isn't this clear and right?

Of course, the "endless loop" is questionable in this case when the stream 
is not running and we should probably return from the write function.

						Jaroslav

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


-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

* Re: intel8x0 has stopped working.
  2004-02-04 23:03       ` James Courtier-Dutton
  2004-02-05 18:49         ` Takashi Iwai
@ 2004-02-05 19:42         ` Takashi Iwai
  2004-02-05 22:28           ` James Courtier-Dutton
  1 sibling, 1 reply; 21+ messages in thread
From: Takashi Iwai @ 2004-02-05 19:42 UTC (permalink / raw)
  To: James Courtier-Dutton; +Cc: ALSA development

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

At Wed, 04 Feb 2004 23:03:50 +0000,
James Courtier-Dutton wrote:
> 
> Takashi Iwai wrote:
> > At Mon, 02 Feb 2004 19:42:37 +0000,
> > James Courtier-Dutton wrote:
> > 
> >>Once thing I have noticed, is that with the alc650, we used to have VRA 
> >>(alsa 0.9.8), but the 1.0.2 intel8x0 driver ignores the VRA and fixes 
> >>itself at 48000.
> > 
> > 
> > yes, the detection of sample rate range seems broken for some codecs.
> > it was completely rewritten using the generic ac97_pcm.c.
> > 
> > could you check the debug messages with the attached patch?
> > 
> > 
> > Takashi
> > 
> 
> Here is the output as requested.

thanks, could you try the attached patch again and show the kernel
messages?
(this might fix the detection, too, BTW.)


Takashi

[-- Attachment #2: Type: text/plain, Size: 6814 bytes --]

Index: alsa-kernel/pci/ac97/ac97_codec.c
===================================================================
RCS file: /suse/tiwai/cvs/alsa/alsa-kernel/pci/ac97/ac97_codec.c,v
retrieving revision 1.107
diff -u -r1.107 ac97_codec.c
--- alsa-kernel/pci/ac97/ac97_codec.c	5 Feb 2004 11:30:02 -0000	1.107
+++ alsa-kernel/pci/ac97/ac97_codec.c	5 Feb 2004 19:17:42 -0000
@@ -1867,7 +1867,9 @@
 		snd_ac97_write_cache(ac97, AC97_EXTENDED_STATUS, ac97->ext_id & 0x0189);
 	if (ac97->ext_id & AC97_EI_VRA) {	/* VRA support */
 		snd_ac97_determine_rates(ac97, AC97_PCM_FRONT_DAC_RATE, &ac97->rates[AC97_RATES_FRONT_DAC]);
+		printk(KERN_DEBUG "determined VRA rates: 0x%x\n", ac97->rates[AC97_RATES_FRONT_DAC]);
 		snd_ac97_determine_rates(ac97, AC97_PCM_LR_ADC_RATE, &ac97->rates[AC97_RATES_ADC]);
+		printk(KERN_DEBUG "determined VRA rates: 0x%x\n", ac97->rates[AC97_RATES_ADC]);
 	} else {
 		ac97->rates[AC97_RATES_FRONT_DAC] = SNDRV_PCM_RATE_48000;
 		ac97->rates[AC97_RATES_ADC] = SNDRV_PCM_RATE_48000;
@@ -1882,19 +1884,23 @@
 			ac97->rates[AC97_RATES_SPDIF] = SNDRV_PCM_RATE_48000 |
 						SNDRV_PCM_RATE_44100 |
 						SNDRV_PCM_RATE_32000;
+		printk(KERN_DEBUG "determined SPDIF rates: 0x%x\n", ac97->rates[AC97_RATES_SPDIF]);
 	}
 	if (ac97->ext_id & AC97_EI_VRM) {	/* MIC VRA support */
 		snd_ac97_determine_rates(ac97, AC97_PCM_MIC_ADC_RATE, &ac97->rates[AC97_RATES_MIC_ADC]);
+		printk(KERN_DEBUG "determined MIC rates: 0x%x\n", ac97->rates[AC97_RATES_MIC_ADC]);
 	} else {
 		ac97->rates[AC97_RATES_MIC_ADC] = SNDRV_PCM_RATE_48000;
 	}
 	if (ac97->ext_id & AC97_EI_SDAC) {	/* SDAC support */
 		snd_ac97_determine_rates(ac97, AC97_PCM_SURR_DAC_RATE, &ac97->rates[AC97_RATES_SURR_DAC]);
 		ac97->scaps |= AC97_SCAP_SURROUND_DAC;
+		printk(KERN_DEBUG "determined SDAC rates: 0x%x\n", ac97->rates[AC97_RATES_SURR_DAC]);
 	}
 	if (ac97->ext_id & AC97_EI_LDAC) {	/* LDAC support */
 		snd_ac97_determine_rates(ac97, AC97_PCM_LFE_DAC_RATE, &ac97->rates[AC97_RATES_LFE_DAC]);
 		ac97->scaps |= AC97_SCAP_CENTER_LFE_DAC;
+		printk(KERN_DEBUG "determined LDAC rates: 0x%x\n", ac97->rates[AC97_RATES_LFE_DAC]);
 	}
 	/* additional initializations */
 	if (bus->init)
Index: alsa-kernel/pci/ac97/ac97_pcm.c
===================================================================
RCS file: /suse/tiwai/cvs/alsa/alsa-kernel/pci/ac97/ac97_pcm.c,v
retrieving revision 1.10
diff -u -r1.10 ac97_pcm.c
--- alsa-kernel/pci/ac97/ac97_pcm.c	2 Jan 2004 13:39:33 -0000	1.10
+++ alsa-kernel/pci/ac97/ac97_pcm.c	5 Feb 2004 19:40:04 -0000
@@ -284,6 +284,7 @@
 		if (ac97_is_rev22(ac97)) {
 			/* Note: it's simply emulation of AMAP behaviour */
 			u16 es;
+			printk(KERN_DEBUG "get_pslots: rev22 amap emu\n");
 			es = ac97->regs[AC97_EXTENDED_STATUS] &= ~AC97_EI_DACS_SLOT_MASK;
 			switch (ac97->addr) {
 			case 1:
@@ -292,6 +293,7 @@
 			}
 			snd_ac97_write_cache(ac97, AC97_EXTENDED_STATUS, es);
 		}
+		printk(KERN_DEBUG "get_pslots: AMAP: addr=%d, scaps=0x%x, ext_id=0x%x\n", ac97->addr, ac97->scaps, ac97->ext_id);
 		switch (ac97->addr) {
 		case 0:
 			slots |= (1<<AC97_SLOT_PCM_LEFT)|(1<<AC97_SLOT_PCM_RIGHT);
@@ -332,6 +334,7 @@
 		return slots;
 	} else {
 		unsigned short slots;
+		printk(KERN_DEBUG "get_pslots: others: addr=%d, scaps=0x%x, ext_id=0x%x\n", ac97->addr, ac97->scaps, ac97->ext_id);
 		slots = (1<<AC97_SLOT_PCM_LEFT)|(1<<AC97_SLOT_PCM_RIGHT);
 		if (ac97->scaps & AC97_SCAP_SURROUND_DAC)
 			slots |= (1<<AC97_SLOT_PCM_SLEFT)|(1<<AC97_SLOT_PCM_SRIGHT);
@@ -371,6 +374,10 @@
 		if (!(slots & (1 << i)))
 			continue;
 		reg = get_slot_reg(pcm, cidx, i, dbl);
+		if (reg == 0xff) {
+			printk(KERN_DEBUG "get_rates: cidx=%d, slot=%d, reg=0x%x\n", cidx, i, reg);
+			continue;
+		}
 		switch (reg) {
 		case AC97_PCM_FRONT_DAC_RATE:	idx = AC97_RATES_FRONT_DAC; break;
 		case AC97_PCM_SURR_DAC_RATE:	idx = AC97_RATES_SURR_DAC; break;
@@ -380,6 +387,7 @@
 		default:			idx = AC97_RATES_SPDIF; break;
 		}
 		rates &= pcm->r[dbl].codec[cidx]->rates[idx];
+		printk(KERN_DEBUG "get_rates: cidx=%d, slot=%d, reg=0x%x, rates=0x%x\n", cidx, i, reg, pcm->r[dbl].codec[cidx]->rates[idx]);
 	}
 	return rates;
 }
@@ -420,16 +428,19 @@
 			continue;
 		avail_slots[0][i] = get_pslots(codec, &rate_table[0][i], &spdif_slots[i]);
 		avail_slots[1][i] = get_cslots(codec);
+		printk(KERN_DEBUG "checking codec %d, slots = 0x%x / 0x%x\n", i, avail_slots[0][i], avail_slots[1][i]);
 		if (!(codec->scaps & AC97_SCAP_INDEP_SDIN)) {
 			for (j = 0; j < i; j++) {
 				if (bus->codec[j])
 					avail_slots[1][i] &= ~avail_slots[1][j];
 			}
+			printk(KERN_DEBUG "-> capture slots = 0x%x\n", avail_slots[1][i]);
 		}
 	}
 	/* FIXME: add double rate allocation */
 	/* first step - exclusive devices */
 	for (i = 0; i < pcms_count; i++) {
+		printk(KERN_DEBUG "probing pcm %d\n", i);
 		pcm = &pcms[i];
 		rpcm = &rpcms[i];
 		/* low-level driver thinks that it's more clever */
@@ -452,22 +463,27 @@
 				tmp = spdif_slots[j];
 			else
 				tmp = avail_slots[pcm->stream][j];
+			printk(KERN_DEBUG ".. probing codec %d, slots = 0x%x, tmp = 0x%x\n", j, slots, tmp);
 			if (pcm->exclusive) {
 				/* exclusive access */
 				tmp &= slots;
+				printk(KERN_DEBUG ".. exclusive tmp = 0x%x\n", tmp);
 				for (k = 0; k < i; k++) {
 					if (rpcm->stream == rpcms[k].stream)
 						tmp &= ~rpcms[k].r[0].rslots[j];
 				}
+				printk(KERN_DEBUG "..... tmp = 0x%x\n", tmp);
 			} else {
 				/* non-exclusive access */
 				tmp &= pcm->r[0].slots;
+				printk(KERN_DEBUG ".. non-exclusive tmp = 0x%x\n", tmp);
 			}
 			if (tmp) {
 				rpcm->r[0].rslots[j] = tmp;
 				rpcm->r[0].codec[j] = bus->codec[j];
 				rpcm->r[0].rate_table[j] = rate_table[pcm->stream][j];
 				rates = get_rates(rpcm, j, tmp, 0);
+				printk(KERN_DEBUG ".. rslots = 0x%x, rate_table = %d, rates = 0x%x\n", tmp, rpcm->r[0].rate_table[j], rates);
 				if (pcm->exclusive)
 					avail_slots[pcm->stream][j] &= ~tmp;
 			}
@@ -475,6 +491,7 @@
 			rpcm->r[0].slots |= tmp;
 			rpcm->rates &= rates;
 		}
+		printk(KERN_DEBUG "--> slots = 0x%x, rates = 0x%x\n", rpcm->r[0].slots, rpcm->rates);
 		if (rpcm->rates == ~0)
 			rpcm->rates = 0; /* not used */
 	}
@@ -501,6 +518,7 @@
 	unsigned char reg;
 	int err = 0;
 
+	printk(KERN_DEBUG "ac97_pcm_open: rate = %d, cfg = %d, slots = 0x%x\n", rate, cfg, slots);
 	if (rate > 48000)	/* FIXME: add support for double rate */
 		return -EINVAL;
 	bus = pcm->bus;
@@ -549,7 +567,7 @@
 				}
 				if (reg_ok & (1 << (reg - AC97_PCM_FRONT_DAC_RATE)))
 					continue;
-				//printk(KERN_DEBUG "setting ac97 reg 0x%x to rate %d\n", reg, rate);
+				printk(KERN_DEBUG "setting ac97 reg 0x%x to rate %d\n", reg, rate);
 				err = snd_ac97_set_rate(pcm->r[r].codec[cidx], reg, rate);
 				if (err < 0)
 					snd_printk(KERN_ERR "error in snd_ac97_set_rate: cidx=%d, reg=0x%x, rate=%d, err=%d\n", cidx, reg, rate, err);

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

* Re: [PATCH] Fixes: Re: intel8x0 has stopped working.
  2004-02-05 19:38               ` Jaroslav Kysela
@ 2004-02-05 19:58                 ` Glenn Maynard
  2004-02-05 20:44                 ` James Courtier-Dutton
  1 sibling, 0 replies; 21+ messages in thread
From: Glenn Maynard @ 2004-02-05 19:58 UTC (permalink / raw)
  To: ALSA development

On Thu, Feb 05, 2004 at 08:38:23PM +0100, Jaroslav Kysela wrote:
> > Otherwise, programs will work on some hardware and not others, which is
> > a case that should be minimized as much as possible; it's these kinds
> > of subtle differences that make it very hard to write reliable (sound,
> > video, etc) code.  I've had to play games with setting hardware settings:
> > always set the sample rate even if I don't care, use a 32k buffer size
> > and not a 4k or 8k one--in order to make it work on as many systems as
> > possible without failing mysteriously or triggering alsa-lib asserts.
> 
> Send us bug-reports if you have problems. We have not a magic ball.

Sample rate issue:

Subject: pcm_plug.c:882: snd_pcm_plug_hw_params: Assertion `err >= 0' failed.
Message-ID: <20031224003200.GB25270@zewt.org>

Buffer size issue:

Subject: CS46xx oddness
Message-ID: <20031224094213.GE25270@zewt.org>

In retrospect, the CS46xx issue looks just like a symptom of this
problem: the buffer fills up and never starts.  (I'm not setting
start_threshold at all, and regardless of the buffer size I only write
up to 4k ahead.)  I don't have this card to actually test with, though.

> Because we have the avail_min threshold which says that we don't want to
> write new data when buffer can accept less samples than this threshold. So
> if start_threshold is greather than '(buffer_size / avail_min) *
> avail_min' expression, then stream won't be automatically started, because
> we cannot fill data in read/write operations to satisfy the requirement
> that start_threshold == buffer_size.
> 
> Isn't this clear and right?

I don't know if avail_min is forced by hardware, but xfer_align (Takashi's
explanation) is.  It's easy for a program written on a system with a small
xfer_align to mysteriously not work on a system with a large xfer_align,
resulting in bug reports like "sound doesn't play".  Reporting an error
in cases where sound would never start is helpful.

-- 
Glenn Maynard


-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

* Re: [PATCH] Fixes: Re: intel8x0 has stopped working.
  2004-02-05 19:38               ` Jaroslav Kysela
  2004-02-05 19:58                 ` Glenn Maynard
@ 2004-02-05 20:44                 ` James Courtier-Dutton
  1 sibling, 0 replies; 21+ messages in thread
From: James Courtier-Dutton @ 2004-02-05 20:44 UTC (permalink / raw)
  To: Jaroslav Kysela; +Cc: Glenn Maynard, ALSA development

Jaroslav Kysela wrote:
> On Thu, 5 Feb 2004, Glenn Maynard wrote:
> 
> 
>>On Thu, Feb 05, 2004 at 12:01:28PM +0100, Jaroslav Kysela wrote:
>>
>>>>My point is, I don't think setting start_threshold to buffer_size is 
>>>>even "wrong" at all. Some people might want the buffer to be full before 
>>>>it starts, and my patch allows for that.
>>>
>>>It's not wrong semantics. I see - it's logical, but I don't want to follow
>>>some rule as some API designers does - control magically some things. I
>>>want that developer which uses our API knows what the library / driver
>>>exactly does.
>>>
>>>We have clear conditions when the stream is started. That's it.
>>
>>If this isn't guaranteed to work, I'd suggest making it never work.
> 
> 
> It's guaranteed to work.
> 
> 
>>Otherwise, programs will work on some hardware and not others, which is
>>a case that should be minimized as much as possible; it's these kinds
>>of subtle differences that make it very hard to write reliable (sound,
>>video, etc) code.  I've had to play games with setting hardware settings:
>>always set the sample rate even if I don't care, use a 32k buffer size
>>and not a 4k or 8k one--in order to make it work on as many systems as
>>possible without failing mysteriously or triggering alsa-lib asserts.
> 
> 
> Send us bug-reports if you have problems. We have not a magic ball.
> 
> 
>>(I don't quite understand why start_threashold == buffer_size doessn't
>>mean "start when the buffer is full", though.)
> 
> 
> Because we have the avail_min threshold which says that we don't want to
> write new data when buffer can accept less samples than this threshold. So
> if start_threshold is greather than '(buffer_size / avail_min) *
> avail_min' expression, then stream won't be automatically started, because
> we cannot fill data in read/write operations to satisfy the requirement
> that start_threshold == buffer_size.
> 
> Isn't this clear and right?

What you say is clear, but I don't think it is right.
If I understand it, you don't want to write samples into the buffer if 
less that X samples are free in the buffer.
We then have 2 possible situations: -
1) We wish to write more frames than X
In this case, we know that we wish to write enough frames to fill the 
buffer, so theoretically, the buffer is full, and thus start_threshold 
has been reached even before the actual alsa-lib internal write 
operation has happened. So, we could call snd_pcm_start() at this point.

2) We wish to write less frames than X
In this case, we know that if the write theoretically succeeded, we 
would not have a full buffer, so should not call snd_pcm_start(). But we 
are in catch22 here, because we cannot write any frames until we have X 
free frames in the buffer, but in SND_STATE_PREPARED (i.e. not RUNNING) 
the amount of free frames in the buffer will never change.

Conclusion: -
Due to the problem (2), we have to have start_threshold <= (buffer_size 
- avail_min) if we wish snd_pcm_start() to be automatically called in 
both case (1) and case(2).
If start_threshold > buffer_size, snd_pcm_start will never be called, 
and that is correct because some applications might want to manually 
control snd_pcm_start(). So, we will need range checks in the 
snd_pcm_sw_params_set_start_threshold() call so that the application 
writer is informed at a early stage about wrong settings, rather than 
finding out that snd_pcm_writei() fails on some sound cards but not others!
Summary: -
By me trying to argue that you are wrong, I have proved you right!

> 
> Of course, the "endless loop" is questionable in this case when the stream 
> is not running and we should probably return from the write function.
> 
> 						Jaroslav
> 

Cheers
James


-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

* Re: intel8x0 has stopped working.
  2004-02-05 19:42         ` Takashi Iwai
@ 2004-02-05 22:28           ` James Courtier-Dutton
  2004-02-06 11:29             ` Takashi Iwai
  0 siblings, 1 reply; 21+ messages in thread
From: James Courtier-Dutton @ 2004-02-05 22:28 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: ALSA development

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

Takashi Iwai wrote:
> At Wed, 04 Feb 2004 23:03:50 +0000,
> James Courtier-Dutton wrote:
> 
>>Takashi Iwai wrote:
>>
>>>At Mon, 02 Feb 2004 19:42:37 +0000,
>>>James Courtier-Dutton wrote:
>>>
>>>
>>>>Once thing I have noticed, is that with the alc650, we used to have VRA 
>>>>(alsa 0.9.8), but the 1.0.2 intel8x0 driver ignores the VRA and fixes 
>>>>itself at 48000.
>>>
>>>
>>>yes, the detection of sample rate range seems broken for some codecs.
>>>it was completely rewritten using the generic ac97_pcm.c.
>>>
>>>could you check the debug messages with the attached patch?
>>>
>>>
>>>Takashi
>>>
>>
>>Here is the output as requested.
> 
> 
> thanks, could you try the attached patch again and show the kernel
> messages?
> (this might fix the detection, too, BTW.)
> 
> 
> Takashi
> 
> 

I checked out a new anonymouse cvs of alsa-driver/alsa-kernel, applied 
your patch, and attach the output from doing modprobe snd-intel8x0.

I can only see printf's in the patch, so I don't see how it could have 
fixes the VRA.

I can still only access device "hw" with rate of 48000.

I hope this helps.

Cheers
James


[-- Attachment #2: intel8x0-rep2 --]
[-- Type: text/plain, Size: 3600 bytes --]

Feb  5 22:22:53 new kernel: PCI: Setting latency timer of device 0000:00:1f.5 to 64
Feb  5 22:22:53 new kernel: determined VRA rates: 0xfe
Feb  5 22:22:53 new kernel: determined VRA rates: 0xfe
Feb  5 22:22:53 new kernel: determined SPDIF rates: 0xe0
Feb  5 22:22:53 new kernel: determined SDAC rates: 0x80
Feb  5 22:22:53 new kernel: determined LDAC rates: 0x80
Feb  5 22:22:53 new kernel: ALSA /usr/local/alsacvs/alsa-driver/alsa-kernel/pci/ac97/ac97_codec.c:2188: applying quirk type -430154656 failed (-22)
Feb  5 22:22:53 new kernel: get_pslots: rev22 amap emu
Feb  5 22:22:53 new kernel: get_pslots: AMAP: addr=0, scaps=0xd, ext_id=0x5c7
Feb  5 22:22:53 new kernel: checking codec 0, slots = 0x3d8 / 0x58
Feb  5 22:22:53 new kernel: -> capture slots = 0x58
Feb  5 22:22:53 new kernel: probing pcm 0
Feb  5 22:22:53 new kernel: .. probing codec 0, slots = 0x3d8, tmp = 0x3d8
Feb  5 22:22:53 new kernel: .. exclusive tmp = 0x3d8
Feb  5 22:22:53 new kernel: ..... tmp = 0x3d8
Feb  5 22:22:53 new kernel: get_rates: cidx=0, slot=3, reg=0x2c, rates=0xfe
Feb  5 22:22:53 new kernel: get_rates: cidx=0, slot=4, reg=0x2c, rates=0xfe
Feb  5 22:22:53 new kernel: get_rates: cidx=0, slot=6, reg=0x30, rates=0x80
Feb  5 22:22:53 new kernel: get_rates: cidx=0, slot=7, reg=0x2e, rates=0x80
Feb  5 22:22:53 new kernel: get_rates: cidx=0, slot=8, reg=0x2e, rates=0x80
Feb  5 22:22:53 new kernel: get_rates: cidx=0, slot=9, reg=0x30, rates=0x80
Feb  5 22:22:53 new kernel: .. rslots = 0x3d8, rate_table = 0, rates = 0x80
Feb  5 22:22:53 new kernel: --> slots = 0x3d8, rates = 0x80
Feb  5 22:22:53 new kernel: probing pcm 1
Feb  5 22:22:53 new kernel: .. probing codec 0, slots = 0x18, tmp = 0x58
Feb  5 22:22:53 new kernel: .. exclusive tmp = 0x18
Feb  5 22:22:53 new kernel: ..... tmp = 0x18
Feb  5 22:22:53 new kernel: get_rates: cidx=0, slot=3, reg=0x32, rates=0xfe
Feb  5 22:22:53 new kernel: get_rates: cidx=0, slot=4, reg=0x32, rates=0xfe
Feb  5 22:22:53 new kernel: .. rslots = 0x18, rate_table = 0, rates = 0xfe
Feb  5 22:22:53 new kernel: --> slots = 0x18, rates = 0xfe
Feb  5 22:22:53 new kernel: probing pcm 2
Feb  5 22:22:53 new kernel: .. probing codec 0, slots = 0x40, tmp = 0x40
Feb  5 22:22:53 new kernel: .. exclusive tmp = 0x40
Feb  5 22:22:53 new kernel: ..... tmp = 0x40
Feb  5 22:22:53 new kernel: get_rates: cidx=0, slot=6, reg=0x34, rates=0x80
Feb  5 22:22:53 new kernel: .. rslots = 0x40, rate_table = 0, rates = 0x80
Feb  5 22:22:53 new kernel: --> slots = 0x40, rates = 0x80
Feb  5 22:22:53 new kernel: probing pcm 3
Feb  5 22:22:53 new kernel: .. probing codec 0, slots = 0xc00, tmp = 0xc00
Feb  5 22:22:53 new kernel: .. exclusive tmp = 0xc00
Feb  5 22:22:53 new kernel: ..... tmp = 0xc00
Feb  5 22:22:53 new kernel: get_rates: cidx=0, slot=10, reg=0x3a, rates=0xe0
Feb  5 22:22:53 new kernel: get_rates: cidx=0, slot=11, reg=0x3a, rates=0xe0
Feb  5 22:22:53 new kernel: .. rslots = 0xc00, rate_table = 0, rates = 0xe0
Feb  5 22:22:53 new kernel: --> slots = 0xc00, rates = 0xe0
Feb  5 22:22:53 new kernel: probing pcm 4
Feb  5 22:22:53 new kernel: .. probing codec 0, slots = 0x18, tmp = 0x0
Feb  5 22:22:53 new kernel: .. exclusive tmp = 0x0
Feb  5 22:22:53 new kernel: ..... tmp = 0x0
Feb  5 22:22:53 new kernel: --> slots = 0x0, rates = 0xffffffff
Feb  5 22:22:53 new kernel: probing pcm 5
Feb  5 22:22:53 new kernel: .. probing codec 0, slots = 0x40, tmp = 0x0
Feb  5 22:22:53 new kernel: .. exclusive tmp = 0x0
Feb  5 22:22:53 new kernel: ..... tmp = 0x0
Feb  5 22:22:53 new kernel: --> slots = 0x0, rates = 0xffffffff
Feb  5 22:22:53 new kernel: intel8x0: clocking to 48000


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

* Re: intel8x0 has stopped working.
  2004-02-05 22:28           ` James Courtier-Dutton
@ 2004-02-06 11:29             ` Takashi Iwai
  2004-02-06 20:05               ` James Courtier-Dutton
  0 siblings, 1 reply; 21+ messages in thread
From: Takashi Iwai @ 2004-02-06 11:29 UTC (permalink / raw)
  To: James Courtier-Dutton; +Cc: ALSA development

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

At Thu, 05 Feb 2004 22:28:03 +0000,
James Courtier-Dutton wrote:
> 
> I checked out a new anonymouse cvs of alsa-driver/alsa-kernel, applied 
> your patch, and attach the output from doing modprobe snd-intel8x0.
> 
> I can only see printf's in the patch, so I don't see how it could have 
> fixes the VRA.

no, it doesn't fix yet.

> Feb  5 22:22:53 new kernel: determined VRA rates: 0xfe
> Feb  5 22:22:53 new kernel: determined VRA rates: 0xfe
> Feb  5 22:22:53 new kernel: determined SPDIF rates: 0xe0
> Feb  5 22:22:53 new kernel: determined SDAC rates: 0x80
> Feb  5 22:22:53 new kernel: determined LDAC rates: 0x80

here's the problem.  surround and LFE DAC sample rates are not
detected properly.
i don't figure out yet why...

does the attached patch have influence?  check the messages above.
(it doesn't contain the patch to ac97_pcm.c.  it's not needed now.)


Takashi

[-- Attachment #2: Type: text/plain, Size: 4638 bytes --]

Index: alsa-kernel/pci/ac97/ac97_codec.c
===================================================================
RCS file: /suse/tiwai/cvs/alsa/alsa-kernel/pci/ac97/ac97_codec.c,v
retrieving revision 1.107
diff -u -r1.107 ac97_codec.c
--- alsa-kernel/pci/ac97/ac97_codec.c	5 Feb 2004 11:30:02 -0000	1.107
+++ alsa-kernel/pci/ac97/ac97_codec.c	6 Feb 2004 11:26:41 -0000
@@ -1526,38 +1526,40 @@
 	return 0;
 }
 
-static int snd_ac97_test_rate(ac97_t *ac97, int reg, int rate)
+static int snd_ac97_test_rate(ac97_t *ac97, int reg, int shadow_reg, int rate)
 {
 	unsigned short val;
 	unsigned int tmp;
 
 	tmp = ((unsigned int)rate * ac97->bus->clock) / 48000;
 	snd_ac97_write_cache(ac97, reg, tmp & 0xffff);
+	if (shadow_reg)
+		snd_ac97_write_cache(ac97, shadow_reg, tmp & 0xffff);
 	val = snd_ac97_read(ac97, reg);
 	return val == (tmp & 0xffff);
 }
 
-static void snd_ac97_determine_rates(ac97_t *ac97, int reg, unsigned int *r_result)
+static void snd_ac97_determine_rates(ac97_t *ac97, int reg, int shadow_reg, unsigned int *r_result)
 {
 	unsigned int result = 0;
 
 	/* test a non-standard rate */
-	if (snd_ac97_test_rate(ac97, reg, 11000))
+	if (snd_ac97_test_rate(ac97, reg, shadow_reg, 11000))
 		result |= SNDRV_PCM_RATE_CONTINUOUS;
 	/* let's try to obtain standard rates */
-	if (snd_ac97_test_rate(ac97, reg, 8000))
+	if (snd_ac97_test_rate(ac97, reg, shadow_reg, 8000))
 		result |= SNDRV_PCM_RATE_8000;
-	if (snd_ac97_test_rate(ac97, reg, 11025))
+	if (snd_ac97_test_rate(ac97, reg, shadow_reg, 11025))
 		result |= SNDRV_PCM_RATE_11025;
-	if (snd_ac97_test_rate(ac97, reg, 16000))
+	if (snd_ac97_test_rate(ac97, reg, shadow_reg, 16000))
 		result |= SNDRV_PCM_RATE_16000;
-	if (snd_ac97_test_rate(ac97, reg, 22050))
+	if (snd_ac97_test_rate(ac97, reg, shadow_reg, 22050))
 		result |= SNDRV_PCM_RATE_22050;
-	if (snd_ac97_test_rate(ac97, reg, 32000))
+	if (snd_ac97_test_rate(ac97, reg, shadow_reg, 32000))
 		result |= SNDRV_PCM_RATE_32000;
-	if (snd_ac97_test_rate(ac97, reg, 44100))
+	if (snd_ac97_test_rate(ac97, reg, shadow_reg, 44100))
 		result |= SNDRV_PCM_RATE_44100;
-	if (snd_ac97_test_rate(ac97, reg, 48000))
+	if (snd_ac97_test_rate(ac97, reg, shadow_reg, 48000))
 		result |= SNDRV_PCM_RATE_48000;
 	*r_result = result;
 }
@@ -1866,8 +1868,10 @@
 	if (ac97->ext_id & 0x0189)	/* L/R, MIC, SDAC, LDAC VRA support */
 		snd_ac97_write_cache(ac97, AC97_EXTENDED_STATUS, ac97->ext_id & 0x0189);
 	if (ac97->ext_id & AC97_EI_VRA) {	/* VRA support */
-		snd_ac97_determine_rates(ac97, AC97_PCM_FRONT_DAC_RATE, &ac97->rates[AC97_RATES_FRONT_DAC]);
-		snd_ac97_determine_rates(ac97, AC97_PCM_LR_ADC_RATE, &ac97->rates[AC97_RATES_ADC]);
+		snd_ac97_determine_rates(ac97, AC97_PCM_FRONT_DAC_RATE, 0, &ac97->rates[AC97_RATES_FRONT_DAC]);
+		printk(KERN_DEBUG "determined VRA rates: 0x%x\n", ac97->rates[AC97_RATES_FRONT_DAC]);
+		snd_ac97_determine_rates(ac97, AC97_PCM_LR_ADC_RATE, 0, &ac97->rates[AC97_RATES_ADC]);
+		printk(KERN_DEBUG "determined VRA rates: 0x%x\n", ac97->rates[AC97_RATES_ADC]);
 	} else {
 		ac97->rates[AC97_RATES_FRONT_DAC] = SNDRV_PCM_RATE_48000;
 		ac97->rates[AC97_RATES_ADC] = SNDRV_PCM_RATE_48000;
@@ -1882,19 +1886,23 @@
 			ac97->rates[AC97_RATES_SPDIF] = SNDRV_PCM_RATE_48000 |
 						SNDRV_PCM_RATE_44100 |
 						SNDRV_PCM_RATE_32000;
+		printk(KERN_DEBUG "determined SPDIF rates: 0x%x\n", ac97->rates[AC97_RATES_SPDIF]);
 	}
 	if (ac97->ext_id & AC97_EI_VRM) {	/* MIC VRA support */
-		snd_ac97_determine_rates(ac97, AC97_PCM_MIC_ADC_RATE, &ac97->rates[AC97_RATES_MIC_ADC]);
+		snd_ac97_determine_rates(ac97, AC97_PCM_MIC_ADC_RATE, 0, &ac97->rates[AC97_RATES_MIC_ADC]);
+		printk(KERN_DEBUG "determined MIC rates: 0x%x\n", ac97->rates[AC97_RATES_MIC_ADC]);
 	} else {
 		ac97->rates[AC97_RATES_MIC_ADC] = SNDRV_PCM_RATE_48000;
 	}
 	if (ac97->ext_id & AC97_EI_SDAC) {	/* SDAC support */
-		snd_ac97_determine_rates(ac97, AC97_PCM_SURR_DAC_RATE, &ac97->rates[AC97_RATES_SURR_DAC]);
+		snd_ac97_determine_rates(ac97, AC97_PCM_SURR_DAC_RATE, AC97_PCM_FRONT_DAC_RATE, &ac97->rates[AC97_RATES_SURR_DAC]);
 		ac97->scaps |= AC97_SCAP_SURROUND_DAC;
+		printk(KERN_DEBUG "determined SDAC rates: 0x%x\n", ac97->rates[AC97_RATES_SURR_DAC]);
 	}
 	if (ac97->ext_id & AC97_EI_LDAC) {	/* LDAC support */
-		snd_ac97_determine_rates(ac97, AC97_PCM_LFE_DAC_RATE, &ac97->rates[AC97_RATES_LFE_DAC]);
+		snd_ac97_determine_rates(ac97, AC97_PCM_LFE_DAC_RATE, AC97_PCM_FRONT_DAC_RATE, &ac97->rates[AC97_RATES_LFE_DAC]);
 		ac97->scaps |= AC97_SCAP_CENTER_LFE_DAC;
+		printk(KERN_DEBUG "determined LDAC rates: 0x%x\n", ac97->rates[AC97_RATES_LFE_DAC]);
 	}
 	/* additional initializations */
 	if (bus->init)

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

* Re: intel8x0 has stopped working.
  2004-02-06 11:29             ` Takashi Iwai
@ 2004-02-06 20:05               ` James Courtier-Dutton
  2004-02-09 10:56                 ` Takashi Iwai
  0 siblings, 1 reply; 21+ messages in thread
From: James Courtier-Dutton @ 2004-02-06 20:05 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: ALSA development

Takashi Iwai wrote:
> At Thu, 05 Feb 2004 22:28:03 +0000,
> James Courtier-Dutton wrote:
> 
>>I checked out a new anonymouse cvs of alsa-driver/alsa-kernel, applied 
>>your patch, and attach the output from doing modprobe snd-intel8x0.
>>
>>I can only see printf's in the patch, so I don't see how it could have 
>>fixes the VRA.
> 
> 
> no, it doesn't fix yet.
> 
> 
>>Feb  5 22:22:53 new kernel: determined VRA rates: 0xfe
>>Feb  5 22:22:53 new kernel: determined VRA rates: 0xfe
>>Feb  5 22:22:53 new kernel: determined SPDIF rates: 0xe0
>>Feb  5 22:22:53 new kernel: determined SDAC rates: 0x80
>>Feb  5 22:22:53 new kernel: determined LDAC rates: 0x80
> 
> 
> here's the problem.  surround and LFE DAC sample rates are not
> detected properly.
> i don't figure out yet why...
> 
> does the attached patch have influence?  check the messages above.
> (it doesn't contain the patch to ac97_pcm.c.  it's not needed now.)
> 
> 
> Takashi
> 
> 
I checked out a new cvs of alsa-driver, alsa-kernel
Added your patch, and the rates are detected correctly now, and the 
rates change on the hardware correctly now.
Thanks

By looking at your code, it looks like you are making sure the rate for 
the Front, Surround and Center/LFE all change together, which seems to 
work, although, the alc650 datasheet spec does not seem to mention that 
this has to happen. Why have 3 ac97 registers, if they all have to 
change together?

Anyway, thanks for the fix.

Cheers
James







-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

* Re: intel8x0 has stopped working.
  2004-02-06 20:05               ` James Courtier-Dutton
@ 2004-02-09 10:56                 ` Takashi Iwai
  0 siblings, 0 replies; 21+ messages in thread
From: Takashi Iwai @ 2004-02-09 10:56 UTC (permalink / raw)
  To: James Courtier-Dutton; +Cc: ALSA development

At Fri, 06 Feb 2004 20:05:42 +0000,
James Courtier-Dutton wrote:
> 
> Takashi Iwai wrote:
> > At Thu, 05 Feb 2004 22:28:03 +0000,
> > James Courtier-Dutton wrote:
> > 
> >>I checked out a new anonymouse cvs of alsa-driver/alsa-kernel, applied 
> >>your patch, and attach the output from doing modprobe snd-intel8x0.
> >>
> >>I can only see printf's in the patch, so I don't see how it could have 
> >>fixes the VRA.
> > 
> > 
> > no, it doesn't fix yet.
> > 
> > 
> >>Feb  5 22:22:53 new kernel: determined VRA rates: 0xfe
> >>Feb  5 22:22:53 new kernel: determined VRA rates: 0xfe
> >>Feb  5 22:22:53 new kernel: determined SPDIF rates: 0xe0
> >>Feb  5 22:22:53 new kernel: determined SDAC rates: 0x80
> >>Feb  5 22:22:53 new kernel: determined LDAC rates: 0x80
> > 
> > 
> > here's the problem.  surround and LFE DAC sample rates are not
> > detected properly.
> > i don't figure out yet why...
> > 
> > does the attached patch have influence?  check the messages above.
> > (it doesn't contain the patch to ac97_pcm.c.  it's not needed now.)
> > 
> > 
> > Takashi
> > 
> > 
> I checked out a new cvs of alsa-driver, alsa-kernel
> Added your patch, and the rates are detected correctly now, and the 
> rates change on the hardware correctly now.
> Thanks

thanks for the confirmation.  i'll apply the patch to cvs.

> By looking at your code, it looks like you are making sure the rate for 
> the Front, Surround and Center/LFE all change together, which seems to 
> work, although, the alc650 datasheet spec does not seem to mention that 
> this has to happen. Why have 3 ac97 registers, if they all have to 
> change together?

we're fooled :)

sometime it is explicitly written that the surround and LFE rates are
simply the mirror of the front/center rate.
that's why the fix came into my mind.


Takashi


-------------------------------------------------------
The SF.Net email is sponsored by EclipseCon 2004
Premiere Conference on Open Tools Development and Integration
See the breadth of Eclipse activity. February 3-5 in Anaheim, CA.
http://www.eclipsecon.org/osdn

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

end of thread, other threads:[~2004-02-09 11:04 UTC | newest]

Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2004-02-02 17:41 intel8x0 has stopped working James Courtier-Dutton
2004-02-02 17:59 ` Takashi Iwai
2004-02-02 19:34   ` Prakash K. Cheemplavam
2004-02-02 19:42   ` James Courtier-Dutton
2004-02-02 23:12     ` [PATCH] Fixes: " James Courtier-Dutton
2004-02-03 16:22       ` Jaroslav Kysela
2004-02-03 22:21         ` James Courtier-Dutton
2004-02-05 11:01           ` Jaroslav Kysela
2004-02-05 19:15             ` Glenn Maynard
2004-02-05 19:25               ` Takashi Iwai
2004-02-05 19:38               ` Jaroslav Kysela
2004-02-05 19:58                 ` Glenn Maynard
2004-02-05 20:44                 ` James Courtier-Dutton
2004-02-04 19:35     ` Takashi Iwai
2004-02-04 23:03       ` James Courtier-Dutton
2004-02-05 18:49         ` Takashi Iwai
2004-02-05 19:42         ` Takashi Iwai
2004-02-05 22:28           ` James Courtier-Dutton
2004-02-06 11:29             ` Takashi Iwai
2004-02-06 20:05               ` James Courtier-Dutton
2004-02-09 10:56                 ` 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.