On Mon, 11 Jan 2021 10:43:23 +0100,
Lauri Kasanen wrote:

On Mon, 11 Jan 2021 09:05:04 +0100
Takashi Iwai [off-list ref] wrote:

quoted

On Sun, 10 Jan 2021 18:41:46 +0100,
Lauri Kasanen wrote:

quoted

It was returning nextpos, but the pointer printk was in bytes. 8192
bytes = 2048 frames.

OK, then it must be right.

Then I suppose that the update of pos should be changed in a different
way; it should always point to the previous nextpos.  That is,
something like:

static void n64audio_push(struct n64audio_t *priv, uint8_t irq)
{
	....
	if (irq)
		priv->chan.pos = priv->chan.nextpos;
	priv->chan.nextpos += count;
	priv->chan.nextpos %= priv->chan.bufsize;

If we use nextpos as the position, it'll lead to the double steps at
the first IRQ handling without snd_pcm_period_elapsed() call (the
first step missed it), and this may confuse PCM core.

This almost works, speed is correct, but the last part is played twice.

Oh yes, at the last IRQ, the push should be avoided.
I guess that the code order should be changed to the following way:

  1. advance the position for a period size
  2. call snd_pcm_period_elapsed()
  3. check if the stream is still running
  4. copy the next chunk and update nextpos

So, it's something like:

static irqreturn_t n64audio_isr(int irq, void *dev_id)
{
	struct n64audio *priv = dev_id;

	// Check it's ours
	const u32 intrs = n64mi_read_reg(MI_INTR_REG);
	if (!(intrs & MI_INTR_AI))
		return IRQ_NONE;

	n64audio_write_reg(AI_STATUS_REG, 1);

	priv->chan.pos += priv->chan.nextpos;
	snd_pcm_period_elapsed(priv->chan.substream);
	if (priv->chan.substream == SNDRV_PCM_STATE_RUNNING)
		n64audio_push(priv);

	return IRQ_HANDLED;
}

By calling snd_pcm_period_elapsed(), PCM core detects the end of the
stream and it stops with the state change.  Then we can avoid the
unnecessary push after the stop.

The irq argument in n64audio_push() is dropped in the above, as it's
superfluous now.


Takashi