Re: [PATCH 5/6 v2] sound: Add n64 driver

Takashi Iwai
2021-01-13T12:39:17Z

On Wed, 13 Jan 2021 13:14:53 +0100,
Lauri Kasanen wrote:
>
> On Wed, 13 Jan 2021 13:04:50 +0100
> Takashi Iwai <tiwai@suse.de> wrote:
>
> > On Wed, 13 Jan 2021 12:57:16 +0100,
> > Lauri Kasanen wrote:
> > > I figured it out. Turns out the hw registers were double-buffered in a
> > > way that requires two periods' worth of buffers. The IRQ fires when one
> > > buffer is finished and another is queued, not when everything is
> > > finished as I first thought.
> > >
> > > There doesn't seem to be a way to request the PCM core to keep two
> > > periods' distance instead of one? I will deploy memcpy then.
> >
> > We may return to the first approach, i.e. just use nextpos. But then
> > snd_pcm_period_elapsed() has to be called right after the trigger
> > callback without the IRQ, because the trigger START already queued the
> > full period and the position advances. So the first period-elapsed
> > has to be called from a work or such offload instead of IRQ.
> > In anyway, it's a bit tricky, yeah.
>
> I don't think that would work? When I printed out where
> __snd_pcm_lib_xfer wrote data, it only steered clear of the current
> period (pointer up to next period size). It wrote in every other part
> of the buffer. This means the currently playing period would be racy.

By advancing the hwptr, it fetches the full data, and PCM core already
checks whether the data has been filled. If not filled, it emits the
buffer underrun error to the application. So it enforces the next
period filled beforehand.

> The other point is that memcpy doesn't have a downside now - it doesn't
> crackle when properly double-buffered. It's simpler this way vs
> workqueues/etc.

I'm fine with the other workaround as long as it works effectively.
It needs more description in the code, though :)

Takashi