On 02/06/2017 09:54 AM, Takashi Iwai wrote:
On Mon, 06 Feb 2017 16:46:53 +0100,
Pierre-Louis Bossart wrote:
Looks nice, with one comment below:
+/* process a bd, advance to the next */
+static void had_advance_ringbuf(struct snd_pcm_substream *substream,
+ struct snd_intelhad *intelhaddata)
+{
+ int num_periods = substream->runtime->periods;
+
+ /* reprogram the next buffer */
+ had_prog_bd(substream, intelhaddata);
+
+ /* proceed to next */
+ intelhaddata->pcmbuf_head++;
+ intelhaddata->pcmbuf_head %= num_periods;
+}
+
+/* process the current BD(s);
+ * returns the current PCM buffer byte position, or -EPIPE for underrun.
+ */
+static int had_process_ringbuf(struct snd_pcm_substream *substream,
+ struct snd_intelhad *intelhaddata)
+{
+ int len, processed;
+ unsigned long flags;
+
+ processed = 0;
+ spin_lock_irqsave(&intelhaddata->had_spinlock, flags);
+ for (;;) {
+ /* get the remaining bytes on the buffer */
+ had_read_register(intelhaddata,
+ AUD_BUF_LEN(intelhaddata->bd_head),
+ &len);
+ if (len < 0 || len > intelhaddata->period_bytes) {
+ dev_dbg(intelhaddata->dev, "Invalid buf length %d\n",
+ len);
+ len = -EPIPE;
+ goto out;
+ }
+
+ if (len > 0) /* OK, this is the current buffer */
+ break;
+
+ /* len=0 => already empty, check the next buffer */
+ if (++processed >= intelhaddata->num_bds) {
+ len = -EPIPE; /* all empty? - report underrun */
+ goto out;
+ }
+ had_advance_ringbuf(substream, intelhaddata);
+ }
+
+ len = intelhaddata->period_bytes - len;
+ len += intelhaddata->period_bytes * intelhaddata->pcmbuf_head;
I don't know if this code is completely correct (and I had similar
concerns with David's).
If the len==0, then the new buffer descriptor will be used in the next
iteration. If the register is read immediately, there is a risk that
the DMA position has not moved and len then becomes
intelhaddata->period_bytes, but the last line will increase the number
of bytes by a period. I think there should be a test here to handle
this corner case.
That's OK. When len=0, the loop goes to the next buffer -- i.e.
pcm_buf is also increased. Then it reads len=period_bytes and quits
the loop. Now len is re-calculated as
len = period_bytes - len;
--> len = 0
len += period_bytes * pcmbuf_head;
--> len = new head position in bytes
which is exactly the expected position.
ok, i guess I need more coffee... David's code did not include the
additional read and I wanted to check this was fine.
Thanks for the precision.
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
