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

[Jerome Brunet <jbrunet@xxxxxxxxxxxx>]

On Mon 01 Jul 2024 at 16:07, Takashi Iwai <tiwai@xxxxxxx> 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@xxxxxxxxxxxx>
>> > ---
>> 
>> 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 ?

>
> thanks,
>
> Takashi

-- 
Jerome