[PATCH v2] ASoC: au1x: declare that FIFO is reported in frames

[PATCH v2] ASoC: au1x: declare that FIFO is reported in frames

[Oswald Buddenhagen]

Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
---

Is this right? The comment on the field says "fifo entries of AC97/I2S
PSC", which doesn't suggest bytes. The data sheet speaks of "words" and
"byte masks", but without digging into it I can't tell how it would
behave with different sample widths and channel counts (which the driver
does not seem to limit _at all_? what am I missing?).

This is just something I stumbled over while surveying how different
drivers set the field. If this is actually a bug, it is minor.
---
 sound/soc/au1x/dbdma2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/au1x/dbdma2.c b/sound/soc/au1x/dbdma2.c
index ea01d6490cec..2c8c01a5fb8f 100644
--- a/sound/soc/au1x/dbdma2.c
+++ b/sound/soc/au1x/dbdma2.c
@@ -66,7 +66,8 @@ struct au1xpsc_audio_dmadata {
 /* PCM hardware DMA capabilities - platform specific */
 static const struct snd_pcm_hardware au1xpsc_pcm_hardware = {
 	.info		  = SNDRV_PCM_INFO_MMAP | SNDRV_PCM_INFO_MMAP_VALID |
-			    SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BATCH,
+			    SNDRV_PCM_INFO_INTERLEAVED | SNDRV_PCM_INFO_BATCH |
+			    SNDRV_PCM_INFO_FIFO_IN_FRAMES,
 	.period_bytes_min = AU1XPSC_PERIOD_MIN_BYTES,
 	.period_bytes_max = 4096 * 1024 - 1,
 	.periods_min	  = 2,
--
2.42.0.419.g70bf8a5751

Re: [PATCH v2] ASoC: au1x: declare that FIFO is reported in frames

[Manuel Lauss]

Hello,

On Mon, Apr 1, 2024 at 10:11 PM Oswald Buddenhagen
<oswald.buddenhagen@gmx.de> wrote:
>
> Signed-off-by: Oswald Buddenhagen <oswald.buddenhagen@gmx.de>
> ---
>
> Is this right? The comment on the field says "fifo entries of AC97/I2S
> PSC", which doesn't suggest bytes. The data sheet speaks of "words" and
> "byte masks", but without digging into it I can't tell how it would
> behave with different sample widths and channel counts (which the driver
> does not seem to limit _at all_? what am I missing?).

Each fifo entry contains a sample; wordsize is configured in register
0x08[24:21].
The fifo is 16 samples deep, so you can have 8 stereo frames or 16
mono frames queued up.
But the fifo isn't managed by hand, dma transfers are used instead and
the amount
of transmitted bytes is extracted from DMA information, so I think this change
is pointless.

Manuel

Re: [PATCH v2] ASoC: au1x: declare that FIFO is reported in frames

[Oswald Buddenhagen]

On Fri, Apr 05, 2024 at 01:22:41PM +0200, Manuel Lauss wrote:
>On Mon, Apr 1, 2024 at 10:11 PM Oswald Buddenhagen
><oswald.buddenhagen@gmx.de> wrote:
>> Is this right? The comment on the field says "fifo entries of
>> AC97/I2S
>> PSC", which doesn't suggest bytes. The data sheet speaks of "words" and
>> "byte masks", but without digging into it I can't tell how it would
>> behave with different sample widths and channel counts (which the driver
>> does not seem to limit _at all_? what am I missing?).
>
>Each fifo entry contains a sample; wordsize is configured in register
>0x08[24:21].
>The fifo is 16 samples deep, so you can have 8 stereo frames or 16
>mono frames queued up.
>
ok, so i'm apparently onto something, though the patch is wrong.

>But the fifo isn't managed by hand, dma transfers are used instead and
>the amount
>of transmitted bytes is extracted from DMA information, so I think this change
>is pointless.
>
not *quite* pointless - that field is used for calculating latencies
which are reported to user space. this doesn't really matter except for
pro audio equipment. it also isn't very precise if converter latencies
(which can be rather substantial for delta-sigma convs) are not also
taken into account. it's even more "interesting" when a single pcm
stream is connected to multiple ports with different latencies (e.g.,
s/pdif and analog).

HDA has the proper framework for tracking the entire path's latency and
the driver seems to support that, but from the drivers i surveyed that
was the only one which takes the matter seriously. most (legacy?)
drivers just set it explicitly to zero or leave it at the zero default,
which is a somewhat clear expression of "no clue / don't care".

so if you don't want to bother with getting it right or it is hopeless
due to inaccessible variables, just deleting the code is an option. or
simply ignoring the problem ...

i'm sure the alsa maints will correct me if i got anything wrong. ^^