On Mon, 11 Jan 2021 10:43:23 +0100,
Lauri Kasanen wrote:
>
> On Mon, 11 Jan 2021 09:05:04 +0100
> Takashi Iwai <tiwai@xxxxxxx> wrote:
>
> > On Sun, 10 Jan 2021 18:41:46 +0100,
> > Lauri Kasanen wrote:
> > > 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
