Re: [PATCH] ALSA: aaci: report FIFO size in frames

["Russell King (Oracle)" <linux@armlinux.org.uk>]

On Mon, Apr 01, 2024 at 12:53:18PM +0200, Oswald Buddenhagen wrote:
> On Mon, Apr 01, 2024 at 11:37:27AM +0100, Russell King (Oracle) wrote:
> > runtime->hw.fifo_size is only measured in _frames_ if
> > SNDRV_PCM_INFO_FIFO_IN_FRAMES is set.
> > 
> yes, which is exactly why the other hunk in the patch sets it.
> 
> > So now I have to ask what caused you to generate this patch. I don't
> > think you've actually run this driver, so presumably it's by []
> > code inspection...
> > 
> yes, it was a random find while trying to make sense of this parameter.

The driver worked when I wrote it. The fifo_size contents was correct
when I wrote it. The choice for using bytes here and not setting
SNDRV_PCM_INFO_FIFO_IN_FRAMES means that ALSA correctly takes the
real FIFO size (which is in bytes) and correctly translates that
itself into frames.

I fail to see how this is any better - in fact, I think it's a lot
worse because, as you've pointed out, it doesn't deal with stereo.
In fact, it only supports 16-bit mono, whereas the driver supports
lots more than that.

I think where the confusion comes from is that fifo_depth is the
depth of the hardware FIFO in units of 16-bit quantities, which is
why we multiply by two to get to bytes. 16-bit quantities does not
necessarily equate to ALSA frames - it can be in specific cases but
not always.

As far as I'm concerned, the code is correct as it stands and your
patch will introduce regressions.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!