2002-01-01 22:49:18

by Thomas Gschwind

[permalink] [raw]
Subject: [patch] i810 audio driver bugfix and SiS7012 support

Hello *!

I have found a serious bug and a couple of smaller ones in the i810
audio driver (kernel version 2.4.17). However, I found the bugs by
reading the source code and doing some experimentations with my
SiS7012 (an i810 clone) mainboard. Hopefully, somebody using the i810
audio driver can confirm or deny the bugs below (especially the first
bug!).

I have also attached a patch that corrects these problems and adds
support for the SiS7012 (now, playback and recording!). It applies to
linux/drivers/sound/i810_audio.c of a 2.4.17 kernel. Any i810 or
SiS7012 owners, please tell me whether the patch works on your system
or whether it adds a different problem. Please mail me directly since
I am not on the kernel mailing list. THANKS!

The bugs:

* The first bug is that i810_read does not set the current state of
the task to running and does not remove the wait queue if a signal
occurs. As a side effect the system _crashes_ at the next read.
This is more serious since it allows any user with read access to
/dev/dsp to crash the system. Just try
$ sox -t ossdsp /dev/dsp x.wav
and press Ctrl-C (I used sox-12.17.1-4 on a RedHat-7.2 system).
FIX: Replace return ret with goto done in a couple of places.

* A signed/unsigned comparison in i810_read
(cnt>(dmabuf->dmasize-swptr)) that results in true if cnt<0. This
bug is everything but serious since it only prepends about 2/3s of
silence to the recording.
FIX: Reordering of cnt computation.

* Sometimes a DMA buffer overrun can occur in drain_dac. This one
only seems to be a spurious one. Probably, the timing on a i810
system is more perfect than on my K7S5A. Anyway, it is cleaner to
use interruptible_sleep_on_timeout than schedule_timeout to be less
dependent on a perfect time. Especially since the wait queue was
already declared but never used.
FIX: interruptible_sleep_on_timeout instead of schedule_timeout

* SNDCTL_DSP_GETISPACE returns the size of the DMA buffer minus
the number of bytes in the buffer. i810_read, however, only allows
the ring buffer to grow up to dmabuf->dmasize-dmabuf->fragsize.
Hence, this IOCTL returns to many bytes. [This was likely a mixup
with SNDCTL_DSP_GETOSPACE]
FIX: Subtract dmabuf->fragsize from the available bytes.

* SNDCTL_DSP_GETOSPACE returns dmabuf->fragsize to little bytes
available since i810_write used the full DMA buffer. Fortunately,
this is not a real problem.
FIX: The SiS7012 chipset does not like the full DMA buffer to be
used. Hence, I changed i810_write to use only
dmabuf->dmasize-dmabuf->fragsize bytes of the ring buffer.

Changes for SiS7012 support:

* SR and PICB registers are swapped.
FIX: Added some ifs at various places.

* DMA transfer is counted in bytes and not in 16bit-words.
FIX: Conditional * 2 in prog_dmabuf, Conditional / 2 in get_dma_address

Thomas

PS: The patch can also be found at
http://www.infosys.tuwien.ac.at/Staff/tom/SiS7012/
--
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


Attachments:
(No filename) (3.14 kB)
sis7012.patch-2 (8.35 kB)
Download all attachments