[alsa-devel] es1938 - patch trying to improve capture hw pointer reads

[Hermann Lauer]

On Thu, Jan 24, 2008 at 04:40:18PM +0100, Takashi Iwai wrote:

> > > The patch is, however, unable to apply as is because of coding style
> > > problems.  Try $LINUXKERNEL/scripts/checkpatch.pl, fix what are
> > > suggested there, and repost the patch together with your sign-off.
> > 
> > Will do that when I digged trough the details (have to learn about signing).
> 
> See $LINUX/Documentation/SubmittingPatches.

Appended is a cleaned up version of the patch. checkpatch.pl is now
only mucking about the '#if 0' commented out old branch. Should this
be removed or the if-else parts exchanged, which probably will calm down
checkpatch.pl ?

Have to check compilation, then I will send a signed version.

Hermann

-- 
Netzwerkadministration/Zentrale Dienste, Interdiziplinaeres 
Zentrum fuer wissenschaftliches Rechnen der Universitaet Heidelberg
IWR; INF 368; 69120 Heidelberg; Tel: (06221)54-8236 Fax: -5224
Email: Hermann.Lauer at iwr.uni-heidelberg.de
-------------- next part --------------
--- linux-2.6.23.12/sound/pci/es1938.c.trydma	2008-01-10 19:50:14.000000000 +0000
+++ linux-2.6.23.12/sound/pci/es1938.c	2008-01-29 07:33:53.000000000 +0000
@@ -227,6 +227,8 @@
 	unsigned int dma2_start;
 	unsigned int dma1_shift;
 	unsigned int dma2_shift;
+	void *last_capture_dmaaddr;
+
 	unsigned int active;
 
 	spinlock_t reg_lock;
@@ -770,19 +772,42 @@
 	return -EINVAL;
 }
 
+/* during the incrementing of dma counters the DMA register reads sometimes
+   returns garbage. To ensure a valid hw pointer, the following checks which
+   should be very unlikely to fail are used:
+   - is the current DMA address in the valid DMA range ?
+   - is the sum of DMA address and DMA counter pointing to the last DMA byte ?
+   One can argue this could differ by one byte depending on which register is
+   updated first, so the implemention below allows for that.
+*/
 static snd_pcm_uframes_t snd_es1938_capture_pointer(struct snd_pcm_substream *substream)
 {
 	struct es1938 *chip = snd_pcm_substream_chip(substream);
 	size_t ptr;
+#if 0
 	size_t old, new;
-#if 1
 	/* This stuff is *needed*, don't ask why - AB */
 	old = inw(SLDM_REG(chip, DMACOUNT));
 	while ((new = inw(SLDM_REG(chip, DMACOUNT))) != old)
 		old = new;
 	ptr = chip->dma1_size - 1 - new;
 #else
-	ptr = inl(SLDM_REG(chip, DMAADDR)) - chip->dma1_start;
+	size_t stat, count;
+	unsigned int diff;
+	struct snd_pcm_runtime *runtime;
+
+	runtime = substream->runtime;
+	ptr = inl(SLDM_REG(chip, DMAADDR));
+	count = inw(SLDM_REG(chip, DMACOUNT));
+	diff = chip->dma1_start+chip->dma1_size-ptr-count;
+
+	if (diff > 3 || ptr < chip->dma1_start
+	      || ptr >= chip->dma1_start+chip->dma1_size)
+	  ptr = chip->last_capture_dmaaddr;            /* bad, use last saved */
+	else
+	  chip->last_capture_dmaaddr = ptr;            /* good, remember it */
+
+	ptr -= chip->dma1_start;
 #endif
 	return ptr >> chip->dma1_shift;
 }