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

Takashi Iwai
2021-01-11T15:25:50Z

On Mon, 11 Jan 2021 13:02:22 +0100,
Lauri Kasanen wrote:
>
> On Mon, 11 Jan 2021 11:11:39 +0100
> Takashi Iwai <tiwai@suse.de> wrote:
>
> > > 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
>
> This order gives correct pointer advancing etc, but now it's hitting a
> new problem: the pcm core is reusing the buffer from under the audio
> card. It's writing new data to the area that is currently being read by
> DMA.

Could you elaborate? I still don't get what's going on there.

> I assume the core expects DMA to be instant, but in this card's case
> it's ondemand, reading bytes as needed.

No, PCM core doesn't expect it. The only expectation is that the data
from the current pointer to the next period boundary will be
transferred until the next period-elapsed call.

> By restoring the memcpy buffer, I get good audio with this new order
> (sans occasional crackling due to the memcpy taking too long).

The overwriting problem has been already present in the original patch
version as I already argued. The driver copies the full next period
to be updated, but this is never guaranteed to have been filled by the
application. Maybe this didn't surface obviously with the original
version because it essentially gives one more period available on the
buffer -- i.e. it might be a matter of number of periods.

Takashi