Re: [PATCH 1/3] ALSA: pcm: add support for 128kHz sample rate

[Takashi Iwai <tiwai@suse.de>]

On Mon, 08 Jul 2024 15:34:44 +0200,
Jerome Brunet wrote:
> 
> On Mon 01 Jul 2024 at 16:07, Takashi Iwai <tiwai@suse.de> wrote:
> 
> > On Mon, 01 Jul 2024 10:50:02 +0200,
> > Amadeusz Sławiński wrote:
> >> 
> >> On 6/28/2024 2:23 PM, Jerome Brunet wrote:
> >> > The usual sample rate possible on an SPDIF link are
> >> > 32k, 44.1k, 48k, 88.2k, 96k, 172.4k and 192k.
> >> > 
> >> > With higher bandwidth variant, such as eARC, and the introduction of 8
> >> > channels mode, the spdif frame rate may be multiplied by 4. This happens
> >> > when the interface use an IEC958_SUBFRAME format.
> >> > 
> >> > The spdif 8 channel mode rate list is:
> >> > 128k, 176.4k, 192k, 352.8k, 384k, 705.4k and 768k.
> >> > 
> >> > All are already supported by ASLA expect for the 128kHz one.
> >> > Add support for it but do not insert it the SNDRV_PCM_RATE_8000_192000
> >> > macro. Doing so would silently add 128k support to a lot of HW which
> >> > probably do not support it.
> >> > 
> >> > Signed-off-by: Jerome Brunet <jbrunet@baylibre.com>
> >> > ---
> >> 
> >> From what I remember the recommendation is to not add new rates, but
> >> use SNDRV_PCM_RATE_KNOT for all rates not included already.
> >
> > In general yes -- unless the new rate is used for significant amount
> > of drivers.
> >
> > So this case is a sort of on a border line; if it's only for ASoC
> > SPDIF codec driver, I'd rather implement with an extra rate list
> > instead of extending the common bits (that has some potential risks by
> > breaking the existing numbers).
> 
> At the moment it would be used by ASoC spdif codec yes (and with Amlogic
> eARC support reasonnably soon, hopefully) 
> 
> However it is likely to be a common rate of any derivative of an IEC958
> interface, with a sufficiently high bandwidth. I suspect there might be
> more of those in the future. Also, it is not an exotic rate for some obscure
> codec no one really has. It is part of specified interface that every TV
> with HDMI 2 is likely to have nowadays. This is why I thought it made
> sense to add it to the usual list. It is the only rate missing,
> everything else is already there.
> 
> Changing the spdif codecs with SNDRV_PCM_RATE_KNOT and a custom rate
> list is doable I suppose, if the new ID is not OK. 
> 
> BTW, I tried not changing the existing numbers and add 128k last but that
> broke. I guess something requires the IDs to be ordered. I did not check
> this further since updating the IDs worked fine (for me, at least :) )
> 
> > OTOH, if we can take this for further
> > cleanups of the existing requirement of 128khz rate, we can go with
> > it.
> >
> 
> Apart from the problem reported in sound/usb/caiaq/audio.c, is there
> another clean up expected ?

The change for caiaq/audio.c is rather a "fix" :)
As a cleanup, I meant, whether this extension can be applied to the
other existing drivers that already use 128kHz with RATE_KNOT and an
extra list.


thanks,

Takashi