Hi everybody,
On Wed, Jan 02, 2002 at 04:56:39PM -0500, Nathan Bryant wrote:
> Can you have a look at Doug Ledford's 0.13 driver? this incorporates
> most or all of the fixes you mentioned, except for SiS support, and some
> other fixes; it hasn't been incorporated into the main kernel quite yet
> because it needs more testing.
I have integrated the SiS patches into Doug's code. Chances are that
it works now also in combination with artsd. Can anybody test this
please? And report your success (or failure)? In case it does not
work you might want to change the fragsize to dmabuf->userfragsize
inside the i810_poll function (line 1596). If I use
dmabuf->userfragsize, however, I get loads of DMA buffer
over/underruns in combination with xmms. According to the sepc, I
think dmabuf->fragsize should be as correct as dmabuf->userfragsize.
I have not found and minimum available space requirement in the poll
man page or the OSS documentation I have?
The patch attached to this email is still relative to the
drivers/sound/i810_audio.c file from the 2.4.17 kernel distribution.
A patched i810_audio.c can be found at
http://www.infosys.tuwien.ac.at/Staff/tom/SiS7012/
Fixes I applied except for the SiS integration:
* drain_dac
Use interruptible_sleep_on_timeout instead of schedule_timeout.
I think this is cleaner. Set the timeout to
(dmabuf->count * HZ * 2) / (dmabuf->rate * 4)
since we play dmabuf->rate*4 bytes per second (16bit samples stereo).
Added stop_dac at the end. Otherwise the system gets locked up sometimes.
* i810_read, i810_write
Set the timeout to (dmabuf->size * HZ * 2) / (dmabuf->rate * 4)
since we record / play dmabuf->rate*4 bytes per second (16bit samples
stereo).
* SNDCTL_DSP_GETxPTR
Don't change the recording / playback buffer. A detailed explanation
can be found within the patch
Please correct me, if any of the above is wrong.
> i've put a lot of work into this driver and i'd like to see everyone
> working on i810 code continue to work off a single base of source code
> rather than ending up with yet another fork...
Thanks for the hint! Never was my plan; just did not know that Doug
had more recent code than the code found in the standard kernel
distribution.
Thomas
--
Thomas Gschwind Email: [email protected]
Technische Universit?t Wien
Argentinierstra?e 8/E1841 Tel: +43 (1) 58801 ext. 18412
A-1040 Wien, Austria, Europe Fax: +43 (1) 58801 ext. 18491
Thomas Gschwind wrote:
>
>+ /* TG: can this possibly be correct?
>+ * according to OSS doc, cinfo.blocks should return the
>+ * number of fragment transitions since the previous call
>+ * to this ioctl. Shouldn't it be better to set
>+ * cinfo.blocks to 0 if we do not support this?
>+ * Why would we want to update the playback pointer? This
>+ * is not mentioned in the OSS doc!
>+ * val==number_free_bytes, hence we increase swptr by val.
>+ * This means that we set the buffer to full. It is
>+ * likely, however, that the hwptr has increased since the
>+ * call to this ioctl, hence part of the buffer is being
>+ * played twice!
>+ */
>
It is correct. MMAP mode doesn't work without it, and we only care about
this IOCTL during mmap mode. I think you'll find that it should have
been setting blocks correctly in mmap mode, the api sequence look like
this (pseudocode:)
open("/dev/dsp");
mmap(...);
// apps that use mmap mode (only Quake* AFAIK, maybe some other games)
will write zeroes to fill mmap buffer at this point
ioctl(SNDCTL_DSP_SETTRIGGER, &PCM_ENABLE_OUTPUT); // this updates LVI to
point to entire 64K buffer and starts DAC. card begins playing silence.
at this point, dmabuf->count = 64K and begins counting down to zero.
ioctl(SNDCTL_DSP_GETOPTR, &foo); // Quake will call this right before it
needs to actually start playing sound (several seconds after the initial
SETTRIGGER call, long enough for the 64K of silence to be played and for
the DAC to stop with DCH interrupt.) Since the game *needs* to have the
current output pointer before it can start writing data to be played, we
take this as a signal that it *intends to do so*, and update the LVI to
play the next 64K of ring-buffer. Note that the mmap-api spec in OSS
docs says that the hardware should be set to loop around the ring buffer
*ad infinitum* in mmap mode, with no further input, so setting it to
simply play the first 64K--intead of infinity--can't hurt. (We can't
tell the hardware to loop around the buffer forever since there is no
command to do so. The only other way to handle that would be to make the
completition interrupt handlers take care of it, which is more error
prone in my opinion, interrupts could be missed plus it pays to keep the
interrupt handlers as simple as possible.) So we rely on the app calling
GETOPTR often enough to keep the LVI chasing around the buffer - which
is *has* to do anyway if it wants to play meaningful data. If it stops
calling GETOPTR, that means it has frozen or slowed down to much to
handle audio anyway. Neat solution, no? ;-)
Note that, after every call to GETOPTR, dmabuf->count gets set to 64K,
and the returned value for blocks is based on dmabuf->count, so the
semantics should actually be correct; we do:
cinfo.blocks = (dmabuf->dmasize - dmabuf->count)/dmabuf->userfragsize;
initially after GETOPTR, count = 64K, so blocks = (64K - 64K) /
fragsize; ( = 0, correct according to docs)
count winds down to zero, updated by i810_update_ptr: assume next call
to GETOPTR occurs exactly 64K later:
blocks = (64K - 0) / userfragsize = the number of blocks played since
last call
so, please put it back the way it was ;)
>
>+#ifndef USE_ORIGINAL_SNDCTL_DSP_GETxPTR_CODE
>+ cinfo.blocks = 0;
>+#else
>+ cinfo.blocks = val/dmabuf->userfragsize;
>+ if (dmabuf->mapped && (dmabuf->trigger & PCM_ENABLE_OUTPUT)) {
>+ dmabuf->count += val;
>+ dmabuf->swptr = (dmabuf->swptr + val) % dmabuf->dmasize;
> __i810_update_lvi(state, 0);
> }
>+#endif
>
Thomas Gschwind wrote:
>I have integrated the SiS patches into Doug's code. Chances are that
>it works now also in combination with artsd. Can anybody test this
>please? And report your success (or failure)? In case it does not
>work you might want to change the fragsize to dmabuf->userfragsize
>inside the i810_poll function (line 1596). If I use
>dmabuf->userfragsize, however, I get loads of DMA buffer
>over/underruns in combination with xmms. According to the sepc, I
>think dmabuf->fragsize should be as correct as dmabuf->userfragsize.
>I have not found and minimum available space requirement in the poll
>man page or the OSS documentation I have?
>
Can you elaborate a little more? What do the buffer underflow sound
like, does it stop playing (silence) for a very short period and then
start again, or does it write ahead too far and overrite data that's
currently playing, causing garbled or repeated sound? Your email to me
describing this scenario left me a little confused.
The former would indicate simple scheduling latency--nothing that can be
done about that--and the latter might be a problem with i810_update_ptr
or get_free_write_space in your implementation. I haven't looked at your
code too much yet...
I assume you're using xmms with artsd. What is your artsd fragment size?
(Click on kde control center, go to sound/sound server/sound i/o)
I assume you're using the artsd default of 4096, which is larger than
the actual hardware chunk size of 2048. If your problem is nothing more
than latency-induced underruns, perhaps it would make more sense to use
MIN(userfragsize, fragsize) to determine the return status for
i810_poll. Doug, do you have any thoughts?
A buffer overrun is not the same as an underflow, even when we're
talking about a ring buffer ;)
>Fixes I applied except for the SiS integration:
>* drain_dac
> Use interruptible_sleep_on_timeout instead of schedule_timeout.
> I think this is cleaner. Set the timeout to
> (dmabuf->count * HZ * 2) / (dmabuf->rate * 4)
> since we play dmabuf->rate*4 bytes per second (16bit samples stereo).
> Added stop_dac at the end. Otherwise the system gets locked up sometimes.
>
Some sort of fix in the drain_dac area is probably needed for the real
Intel chips too; with 0.13 I was seeing a drain_dac: dma_timeout printk
occasionally on my setup which I hadn't bothered to debug yet. Didn't
hurt the machine and I figured it was maybe just dropped or latent
interrupts, so I got lazy. ;)
>* i810_read, i810_write
> Set the timeout to (dmabuf->size * HZ * 2) / (dmabuf->rate * 4)
> since we record / play dmabuf->rate*4 bytes per second (16bit samples
> stereo).
>
Does this solve a problem for your SiS chip? i810_write seemed to be
working fine on my setup. (Intel hardware.)
Nathan Bryant wrote:
> Thomas Gschwind wrote:
>
>> I have integrated the SiS patches into Doug's code. Chances are that
>> it works now also in combination with artsd. Can anybody test this
>> please? And report your success (or failure)? In case it does not
>> work you might want to change the fragsize to dmabuf->userfragsize
>> inside the i810_poll function (line 1596). If I use
>> dmabuf->userfragsize, however, I get loads of DMA buffer
>> over/underruns in combination with xmms. According to the sepc, I
>> think dmabuf->fragsize should be as correct as dmabuf->userfragsize.
>> I have not found and minimum available space requirement in the poll
>> man page or the OSS documentation I have?
>>
> Can you elaborate a little more? What do the buffer underflow sound
> like, does it stop playing (silence) for a very short period and then
> start again, or does it write ahead too far and overrite data that's
> currently playing, causing garbled or repeated sound? Your email to me
> describing this scenario left me a little confused.
Actually, this is moot. Making it user fragsize instead of userfragsize is
the *wrong* thing to do. They are not the same and the semantics are pretty
clear cut.
fragsize: The actual size of each dma ring buffer, of which we *always* have
32. This is a hardware limitation. It's used for various calculations, but
is only ever set in prog_dmabuf().
userfragsize: The SETFRAGMENT ioctl allows the user to tell us how often
they want interrupted with updates about output/input progress and what size
block they expect that update to indicate. So, when the user requests a
userfragsize of 4k, they are saying "Tell me about output completion with
every 4k block you complete". They also tell us how many blocks they want
allocated. The only requirement we have is that userfragsize *
userfragcount (can't remember the variable name I actually call this in the
code) == fragsize * 32. Then, we use userfragsize to tell us how often to
program the ring buffers for completion interrupts. For instance, if we
have 32 buffers of 512 bytes and userfragsize is 4k and userfragcount is 4,
then we would program every 8th ring buffer to deliver a completion
interrupt. That's the relationship between the two. So, in the poll
routine, we need to alert userspace when userfragsize is reached, not
fragsize. They might be the same, and they might not.
> The former would indicate simple scheduling latency--nothing that can be
> done about that--and the latter might be a problem with i810_update_ptr
> or get_free_write_space in your implementation. I haven't looked at your
> code too much yet...
>
> I assume you're using xmms with artsd. What is your artsd fragment size?
> (Click on kde control center, go to sound/sound server/sound i/o)
>
> I assume you're using the artsd default of 4096, which is larger than
> the actual hardware chunk size of 2048. If your problem is nothing more
> than latency-induced underruns, perhaps it would make more sense to use
> MIN(userfragsize, fragsize) to determine the return status for
> i810_poll. Doug, do you have any thoughts?
If the above doesn't answer any questions, then let me know and I'll
elaborate further.
> A buffer overrun is not the same as an underflow, even when we're
> talking about a ring buffer ;)
>
>> Fixes I applied except for the SiS integration:
>> * drain_dac
>> Use interruptible_sleep_on_timeout instead of schedule_timeout.
>> I think this is cleaner. Set the timeout to (dmabuf->count * HZ *
>> 2) / (dmabuf->rate * 4)
>> since we play dmabuf->rate*4 bytes per second (16bit samples stereo).
>> Added stop_dac at the end. Otherwise the system gets locked up
>> sometimes.
>
> Some sort of fix in the drain_dac area is probably needed for the real
> Intel chips too; with 0.13 I was seeing a drain_dac: dma_timeout printk
> occasionally on my setup which I hadn't bothered to debug yet. Didn't
> hurt the machine and I figured it was maybe just dropped or latent
> interrupts, so I got lazy. ;)
>
>> * i810_read, i810_write
>> Set the timeout to (dmabuf->size * HZ * 2) / (dmabuf->rate * 4)
>> since we record / play dmabuf->rate*4 bytes per second (16bit samples
>> stereo).
>>
> Does this solve a problem for your SiS chip? i810_write seemed to be
> working fine on my setup. (Intel hardware.)
>
The timeout in these areas is intentionally left over long. I don't mind it
being close to the actual real expect time of completion, but do make sure
it's a few ticks past completion time or else you might race against the
completion interrupt depending on where you are in the current timer tick cycle.
--
Doug Ledford <[email protected]> http://people.redhat.com/dledford
Please check my web site for aic7xxx updates/answers before
e-mailing me about problems
Hi Thomas. I've been looking over the patch and trying to evaluate the
various bug fixes that you mention against the patch. My comments are
inline (and ignore the fact that I'm positive my cut and past job will
mangle the patch itself, this is for comments, not for applying). I'm also
replying to your comments.
Tom wrote:
> Doug,
>
> attched you can find the latest SiS7012 patch. This one is relative
> to your i810_audio.c driver.
>
> Fixes it applies except for the SiS integration:
> * drain_dac
> Use interruptible_sleep_on_timeout instead of schedule_timeout.
> I think this is cleaner.
This is fine.
> Set the timeout to
> (dmabuf->count * HZ * 2) / (dmabuf->rate * 4)
> since we play dmabuf->rate*4 bytes per second (16bit samples stereo).
This is fine as well.
> Added stop_dac at the end. Otherwise the system gets locked up sometimes.
This is (I think) a red herring. I would suspect that the lockup you are
seeing is more likely to have something to do with the changes in
i810_get_dma_address(). I'm referring to the change to the do {}while()
loop in particular. Also, if the change you made in i810_get_dma_address()
is needed for the SiS chipset, then there would appear to be a hardware bug
that needs worked around. However, I think this is the wrong way to solve
it. In prog_dmabuf(), we set the control bits on the device such that when
it finishes the LVI, it's *suppossed* to stop playback. Now, the only way I
can think of that the CIV register would update as fast as you can read it
in i810_get_dma_address() is if at the end of LVI, the card went into a loop
and the CIV is essentially running from 0 to 31 and going round and round
without doing anything. In that case, the proper fix may actually be to go
into the interrupt handler and on LVI interrupt with count==0 (for playback)
call stop_dac and do similarly for record. This is because I suspect that
we aren't ever getting the DCH Stopped interrupt on the SiS card. Of
course, the DCH Stop interrupt causes us to call stop_{dac,adc} as
appropriate, so the call to stop_dac in drain_dac is redundant if this code
is working as advertised. I suspect that fixing things in the interrupt
handler will make the stop_dac call in drain_dac and the change in
i810_get_dma_address() both unnecessary. Can you test that for me?
Something else you might want to check into on the SiS chipset. I actually
wish Intel had done the i810 with the SR and PICB registers swapped because
you are suppossed to be able to do a 32bit read from the CIV register
location and get CIV, LVI, and SR all in one go. With the SiS that means
you could get CIV, LVI, and PCIB all in one go, making the while loop in
i810_get_dma_address totally unnecessary on the SiS chipset (and making a
slight boost to performance as well) as long as the SiS also supports a
32bit read from CIV.
> * i810_read, i810_write
> Set the timeout to (dmabuf->size * HZ * 2) / (dmabuf->rate * 4)
> since we record / play dmabuf->rate*4 bytes per second (16bit samples
> stereo).
Again, fine.
> * SNDCTL_DSP_GETxPTR
> Don't change the recording / playback buffer. A detailed explanation
> can be found within the patch
As commented on by Nathan, this isn't right.
> Please correct me, if any of the above is wrong.
>
> Thomas
>
Now, I've been talking with Ben LaHaise here at Red Hat and he pointed out
to me that the wait queue handling in i810_read and likely also in
i810_write are both wrong. Ben and I are looking into that and I'll put out
a new version of the file that fixes those (obvious) problems shortly. When
I've got my 0.14 version out, could you please repatch your stuff against it
and look into the items I've mentioned here?
--
Doug Ledford <[email protected]> http://people.redhat.com/dledford
Please check my web site for aic7xxx updates/answers before
e-mailing me about problems
Doug Ledford wrote:
>> Fixes it applies except for the SiS integration:
>> * drain_dac
>> Use interruptible_sleep_on_timeout instead of schedule_timeout.
>> I think this is cleaner.
>
>
>
> This is fine.
Oops, this is one of the mistakes Ben pointed out to me.
interruptible_sleep_on_timeout(wait_queue head, timeout) overwrites the wait
queue that we've already added ourselves to. schedule_timeout() does the
right thing here. (interruptible_sleep_on_timeout doesn't really buy us
much of anything we care about either, so it's find to stay with
schedule_timeout)
--
Doug Ledford <[email protected]> http://people.redhat.com/dledford
Please check my web site for aic7xxx updates/answers before
e-mailing me about problems
Thomas Gschwind wrote:
>
>I have integrated the SiS patches into Doug's code. Chances are that
>
I would just like to report a full sccess with the aforementioned patch.
Hardware is a SIS7012, on a SIS735 chipset board. Even artsd seems to
work fine.
Doug Ledford wrote:
> Doug Ledford wrote:
>
>
>>> Fixes it applies except for the SiS integration:
>>> * drain_dac
>>> Use interruptible_sleep_on_timeout instead of schedule_timeout.
>>> I think this is cleaner.
>>
>>
>>
>>
>> This is fine.
>
>
>
> Oops, this is one of the mistakes Ben pointed out to me.
> interruptible_sleep_on_timeout(wait_queue head, timeout) overwrites the
> wait queue that we've already added ourselves to. schedule_timeout()
> does the right thing here. (interruptible_sleep_on_timeout doesn't
> really buy us much of anything we care about either, so it's find to
> stay with schedule_timeout)
>
>
>
>
OK, various clean ups made, and enough of the SiS code included that I think
it should work, plus one change to the i810 interrupt handler that will
(hopefully) render the other change you made to get_dma_addr and drain_dac
unnecessary. If people could please download and test the new 0.14 version
of the driver on my site, I would appreciate it.
http://people.redhat.com/dledford/i810_audio.c.gz
--
Doug Ledford <[email protected]> http://people.redhat.com/dledford
Please check my web site for aic7xxx updates/answers before
e-mailing me about problems
Hi,
> OK, various clean ups made, and enough of the SiS code included that I think
> it should work, plus one change to the i810 interrupt handler that will
> (hopefully) render the other change you made to get_dma_addr and drain_dac
> unnecessary. If people could please download and test the new 0.14 version
> of the driver on my site, I would appreciate it.
>
> http://people.redhat.com/dledford/i810_audio.c.gz
Hmmm, maybe way too much cleanups !? :)
-->
i810_audio.c: In function `i810_get_dma_addr':
i810_audio.c:658: warning: unused variable `c'
i810_audio.c: In function `__stop_dac':
i810_audio.c:747: `PI_OR' undeclared (first use in this function)
i810_audio.c:747: (Each undeclared identifier is reported only once
i810_audio.c:747: for each function it appears in.)
make[2]: *** [i810_audio.o] Error 1
make[1]: *** [_modsubdir_sound] Error 2
make: *** [_mod_drivers] Error 2
ps
just got a note from a friend that .13 has tendency to lockup with
heavy network traffic in the same time, no oops, nothing, ..
--
Mario Miko?evi? (Mozgy)
mozgy at hinet dot hr
My favourite FUBAR ...
Mario Mikocevic wrote:
> Hi,
>
>
>>OK, various clean ups made, and enough of the SiS code included that I think
>>it should work, plus one change to the i810 interrupt handler that will
>>(hopefully) render the other change you made to get_dma_addr and drain_dac
>>unnecessary. If people could please download and test the new 0.14 version
>>of the driver on my site, I would appreciate it.
>>
>>http://people.redhat.com/dledford/i810_audio.c.gz
>>
>
> Hmmm, maybe way too much cleanups !? :)
>
> -->
>
> i810_audio.c: In function `i810_get_dma_addr':
> i810_audio.c:658: warning: unused variable `c'
> i810_audio.c: In function `__stop_dac':
> i810_audio.c:747: `PI_OR' undeclared (first use in this function)
> i810_audio.c:747: (Each undeclared identifier is reported only once
> i810_audio.c:747: for each function it appears in.)
> make[2]: *** [i810_audio.o] Error 1
> make[1]: *** [_modsubdir_sound] Error 2
> make: *** [_mod_drivers] Error 2
>
Sorry. Version that compiles is now on my web site.
> ps
> just got a note from a friend that .13 has tendency to lockup with
> heavy network traffic in the same time, no oops, nothing, ..
>
>
--
Doug Ledford <[email protected]> http://people.redhat.com/dledford
Please check my web site for aic7xxx updates/answers before
e-mailing me about problems
--- i810_audio.c.14 Tue Jan 8 03:28:31 2002
+++ linux/drivers/sound/i810_audio.c Tue Jan 8 14:01:26 2002
@@ -655,7 +655,6 @@
{
struct dmabuf *dmabuf = &state->dmabuf;
unsigned int civ, offset, port, port_picb, bytes = 2;
- struct i810_channel *c;
if (!dmabuf->enable)
return 0;
@@ -744,7 +743,7 @@
if(card->pci_id == PCI_DEVICE_ID_SI_7012)
outb( inb(card->iobase + PO_PICB), card->iobase + PO_PICB );
else
- outb( inb(card->iobase + PO_SR), card->iobase + PI_OR );
+ outb( inb(card->iobase + PO_SR), card->iobase + PO_SR );
outl( inl(card->iobase + GLOB_STA) & INT_PO, card->iobase + GLOB_STA);
}
Doug Ledford wrote:
>> Added stop_dac at the end. Otherwise the system gets locked up
>> sometimes.
>
>
>
> This is (I think) a red herring. I would suspect that the lockup you
> are seeing is more likely to have something to do with the changes in
> i810_get_dma_address(). I'm referring to the change to the do
> {}while() loop in particular. Also, if the change you made in
> i810_get_dma_address() is needed for the SiS chipset, then there would
> appear to be a hardware bug that needs worked around. However, I
> think this is the wrong way to solve it. In prog_dmabuf(), we set the
> control bits on the device such that when it finishes the LVI, it's
> *suppossed* to stop playback. Now, the only way I can think of that
> the CIV register would update as fast as you can read it in
> i810_get_dma_address() is if at the end of LVI, the card went into a
> loop and the CIV is essentially running from 0 to 31 and going round
> and round without doing anything. In that case, the proper fix may
> actually be to go into the interrupt handler and on LVI interrupt with
> count==0 (for playback) call stop_dac and do similarly for record.
> This is because I suspect that we aren't ever getting the DCH Stopped
> interrupt on the SiS card. Of course, the DCH Stop interrupt causes
> us to call stop_{dac,adc} as appropriate, so the call to stop_dac in
> drain_dac is redundant if this code is working as advertised. I
> suspect that fixing things in the interrupt handler will make the
> stop_dac call in drain_dac and the change in i810_get_dma_address()
> both unnecessary. Can you test that for me?
1) Is the LVI interrupt supposed to arrive when the chip *starts*
playing the last buffer?
2) Does SiS actually do it this way?
If your theory on why the registers are spinning is correct, and if we
receive the LVI interrupt with too much latency, your code will still
deadlock, Doug. (The LVI interrupt handler calls update_ptr first thing,
which calls get_dma_address.) Furthermore, if this turns out to be the
case, the LVI IRQ handler uses dmabuf->count to determine whether to
call stop_dac, and needs to call update_ptr to update dmabuf->count...
so an explicit stop_dac might be needed elsewhere.
Even if the LVI interrupt comes at the beginning of the buffer, those
2048 bytes will play in 10.67 ms. Can we really guarantee that kind of
latency?
Nathan Bryant wrote:
> Doug Ledford wrote:
>
>>> Added stop_dac at the end. Otherwise the system gets locked up
>>> sometimes.
>>
>>
>>
>>
>> This is (I think) a red herring. I would suspect that the lockup you
>> are seeing is more likely to have something to do with the changes in
>> i810_get_dma_address(). I'm referring to the change to the do
>> {}while() loop in particular. Also, if the change you made in
>> i810_get_dma_address() is needed for the SiS chipset, then there would
>> appear to be a hardware bug that needs worked around. However, I
>> think this is the wrong way to solve it. In prog_dmabuf(), we set the
>> control bits on the device such that when it finishes the LVI, it's
>> *suppossed* to stop playback. Now, the only way I can think of that
>> the CIV register would update as fast as you can read it in
>> i810_get_dma_address() is if at the end of LVI, the card went into a
>> loop and the CIV is essentially running from 0 to 31 and going round
>> and round without doing anything. In that case, the proper fix may
>> actually be to go into the interrupt handler and on LVI interrupt with
>> count==0 (for playback) call stop_dac and do similarly for record.
>> This is because I suspect that we aren't ever getting the DCH Stopped
>> interrupt on the SiS card. Of course, the DCH Stop interrupt causes
>> us to call stop_{dac,adc} as appropriate, so the call to stop_dac in
>> drain_dac is redundant if this code is working as advertised. I
>> suspect that fixing things in the interrupt handler will make the
>> stop_dac call in drain_dac and the change in i810_get_dma_address()
>> both unnecessary. Can you test that for me?
>
>
> 1) Is the LVI interrupt supposed to arrive when the chip *starts*
> playing the last buffer?
No, when the last buffer is complete.
> 2) Does SiS actually do it this way?
No clue. If they follow the spec, then yes.
> If your theory on why the registers are spinning is correct, and if we
> receive the LVI interrupt with too much latency, your code will still
> deadlock, Doug. (The LVI interrupt handler calls update_ptr first thing,
> which calls get_dma_address.)
My theory about get_dma_address at this point is that the upper 3 bits of
CIV are garbage and are subject to different states each time you read the
register. Since I bounded the read by using &31 to mask off the garbage
bits, that should no longer be an issue.
> Furthermore, if this turns out to be the
> case, the LVI IRQ handler uses dmabuf->count to determine whether to
> call stop_dac, and needs to call update_ptr to update dmabuf->count...
> so an explicit stop_dac might be needed elsewhere.
I don't think so. See above.
> Even if the LVI interrupt comes at the beginning of the buffer, those
> 2048 bytes will play in 10.67 ms. Can we really guarantee that kind of
> latency?
>
We don't need to. This is also where the spurious DMA Overrun on write
errors were coming from. We *expect* to have a dma overrun on write if the
last block was not completely filled (notice in i810_write() that we filled
the remainder of the block with silence). So, we check if LVI == CIV when
we appear to have an overrun. If they are the same, then no overrun, we
just played the silence until the end of the buffer, then things stopped
because this was the LVI, and all is good and right in the world. If they
aren't the same (which they aren't if you are getting garbage in the upper 3
bits that is random), then we print out spurious messages. I think a lot of
the problems that people have been chasing with the SiS chipset are this
exact kind of thing. You'll also notice that support for the NVIDIA nForce
Audio is now in 0.15. Based upon the fact that the guy from NVIDIA that
contacted me had been seeing the same sort of random DMA Overrun on write
messages, I'm guessing that both the SiS and the NVIDIA audio devices have
this same garbage in the upper 3 bits of CIV register problem.
--
Doug Ledford <[email protected]> http://people.redhat.com/dledford
Please check my web site for aic7xxx updates/answers before
e-mailing me about problems
Nathan Bryant wrote:
> 1) Is the LVI interrupt supposed to arrive when the chip *starts*
> playing the last buffer?
> 2) Does SiS actually do it this way?
>
> If your theory on why the registers are spinning is correct, and if we
> receive the LVI interrupt with too much latency, your code will still
> deadlock, Doug. (The LVI interrupt handler calls update_ptr first
> thing, which calls get_dma_address.) Furthermore, if this turns out to
> be the case, the LVI IRQ handler uses dmabuf->count to determine
> whether to call stop_dac, and needs to call update_ptr to update
> dmabuf->count... so an explicit stop_dac might be needed elsewhere.
>
> Even if the LVI interrupt comes at the beginning of the buffer, those
> 2048 bytes will play in 10.67 ms. Can we really guarantee that kind of
> latency?
Add to this, if SiS isn't sending DCH, and LVI arrives at the beginning
of the last buffer, count is still > 0, so we don't call stop_dac. And
we're right back where we started.
Hi,
On Tue, Jan 08, 2002 at 04:02:35AM -0500, Doug Ledford wrote:
> OK, various clean ups made, and enough of the SiS code included that I think
> it should work, plus one change to the i810 interrupt handler that will
> (hopefully) render the other change you made to get_dma_addr and drain_dac
> unnecessary. If people could please download and test the new 0.14 version
> of the driver on my site, I would appreciate it.
>
> http://people.redhat.com/dledford/i810_audio.c.gz
Well, ver 0.18 works fine for me _and_ for a friend of mine who _had_
problems with high network traffic while playing sound.
I haven't tried it on my webcast farm yet.
Intel 810 + AC97 Audio, version 0.18, 09:27:34 Jan 9 2002
PCI: Found IRQ 9 for device 00:1f.5
PCI: Sharing IRQ 9 with 00:1f.3
PCI: Sharing IRQ 9 with 02:09.0
PCI: Setting latency timer of device 00:1f.5 to 64
i810: Intel ICH2 found at IO 0xc400 and 0xc300, IRQ 9
i810_audio: Audio Controller supports 6 channels.
ac97_codec: AC97 Audio codec, id: 0x4352:0x5934 (Cirrus Logic CS4299 rev D)
i810_audio: AC'97 codec 0 supports AMAP, total channels = 2
--
Mario Miko?evi? (Mozgy)
mozgy at hinet dot hr
My favourite FUBAR ...