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

[Takashi Iwai <tiwai@suse.de>]

On Sun, 10 Jan 2021 08:15:36 +0100,
Lauri Kasanen wrote:
> 
> On Sat, 09 Jan 2021 21:54:12 +0100
> Takashi Iwai <tiwai@suse.de> wrote:
> 
> > When the start starts, it copies the full period size data, and moves
> > nextpos to period size while keeping pos 0.  And, at this moment, it's
> > even possible that no enough data has been filled for the period
> > size; this is practically a buffer underrun.
> > Usually PCM core can catch the buffer underrun by comparing the
> > current position reported by the pointer callback against the filled
> > size, but in this case, PCM core can't know of it because the driver
> > just tells the position 0.  This is one problem.
> >
> > Then, at the first period IRQ, the next period is copied, then nextpos
> > becomes 2*period_size.  At this moment, pos = nextpos, hence it jumps
> > from 0 to 2*periodsize out of sudden.  It's quite confusing behavior
> > for applications.  That's the second problem.
> >
> > I guess that both problems could be avoided if n64audio_pointer()
> > reports always nextpos instead of pos.
> 
> At first there was no nextpos, and _pointer() always reported pos. This
> didn't work, the core played through the audio at chipmunk speed. So
> there must be more that I don't understand here.

Try to set the periods_min=2 and the integer periods hw constraint at
first, and change the pointer callback to return nextpos.  Also, at
the push function, set runtime->delay = period_size as well.

> Let me describe the hw, perhaps a different approach would be better.
> - the DMA unit requires 8-byte alignment and 8-divisible size (two
> frames at the only supported format, s16be stereo)

This restriction needs to be set up as another hw constraint as long
as you use the period size as the transfer size.

> - the DMA unit has errata if (start + len) & 0x3fff == 0x2000, this
> must never happen

Ditto.

> - the audio IRQ is not a timer, it fires when the card's internal
> buffers are empty and need immediate refill

It's the current implementation, right?


Takashi