Re: [PATCH v2 02/12] ALSA: iec958: Split status creation and fill
From: Takashi Iwai
Date: Tue May 25 2021 - 12:01:20 EST
On Tue, 25 May 2021 15:23:44 +0200,
Maxime Ripard wrote:
>
> In some situations, like a codec probe, we need to provide an IEC status
> default but don't have access to the sampling rate and width yet since
> no stream has been configured yet.
>
> Each and every driver has its own default, whereas the core iec958 code
> also has some buried in the snd_pcm_create_iec958_consumer functions.
>
> Let's split these functions in two to provide a default that doesn't
> rely on the sampling rate and width, and another function to fill them
> when available.
>
> Signed-off-by: Maxime Ripard <maxime@xxxxxxxxxx>
Reviewed-by: Takashi Iwai <tiwai@xxxxxxx>
thanks,
Takashi
> ---
> include/sound/pcm_iec958.h | 8 ++
> sound/core/pcm_iec958.c | 176 ++++++++++++++++++++++++++++---------
> 2 files changed, 141 insertions(+), 43 deletions(-)
>
> diff --git a/include/sound/pcm_iec958.h b/include/sound/pcm_iec958.h
> index 0939aa45e2fe..64e84441cde1 100644
> --- a/include/sound/pcm_iec958.h
> +++ b/include/sound/pcm_iec958.h
> @@ -4,6 +4,14 @@
>
> #include <linux/types.h>
>
> +int snd_pcm_create_iec958_consumer_default(u8 *cs, size_t len);
> +
> +int snd_pcm_fill_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs,
> + size_t len);
> +
> +int snd_pcm_fill_iec958_consumer_hw_params(struct snd_pcm_hw_params *params,
> + u8 *cs, size_t len);
> +
> int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs,
> size_t len);
>
> diff --git a/sound/core/pcm_iec958.c b/sound/core/pcm_iec958.c
> index f9a211cc1f2c..7a1b816f67cc 100644
> --- a/sound/core/pcm_iec958.c
> +++ b/sound/core/pcm_iec958.c
> @@ -9,41 +9,85 @@
> #include <sound/pcm_params.h>
> #include <sound/pcm_iec958.h>
>
> -static int create_iec958_consumer(uint rate, uint sample_width,
> - u8 *cs, size_t len)
> +/**
> + * snd_pcm_create_iec958_consumer_default - create default consumer format IEC958 channel status
> + * @cs: channel status buffer, at least four bytes
> + * @len: length of channel status buffer
> + *
> + * Create the consumer format channel status data in @cs of maximum size
> + * @len. When relevant, the configuration-dependant bits will be set as
> + * unspecified.
> + *
> + * Drivers should then call einter snd_pcm_fill_iec958_consumer() or
> + * snd_pcm_fill_iec958_consumer_hw_params() to replace these unspecified
> + * bits by their actual values.
> + *
> + * Drivers may wish to tweak the contents of the buffer after creation.
> + *
> + * Returns: length of buffer, or negative error code if something failed.
> + */
> +int snd_pcm_create_iec958_consumer_default(u8 *cs, size_t len)
> {
> - unsigned int fs, ws;
> -
> if (len < 4)
> return -EINVAL;
>
> - switch (rate) {
> - case 32000:
> - fs = IEC958_AES3_CON_FS_32000;
> - break;
> - case 44100:
> - fs = IEC958_AES3_CON_FS_44100;
> - break;
> - case 48000:
> - fs = IEC958_AES3_CON_FS_48000;
> - break;
> - case 88200:
> - fs = IEC958_AES3_CON_FS_88200;
> - break;
> - case 96000:
> - fs = IEC958_AES3_CON_FS_96000;
> - break;
> - case 176400:
> - fs = IEC958_AES3_CON_FS_176400;
> - break;
> - case 192000:
> - fs = IEC958_AES3_CON_FS_192000;
> - break;
> - default:
> + memset(cs, 0, len);
> +
> + cs[0] = IEC958_AES0_CON_NOT_COPYRIGHT | IEC958_AES0_CON_EMPHASIS_NONE;
> + cs[1] = IEC958_AES1_CON_GENERAL;
> + cs[2] = IEC958_AES2_CON_SOURCE_UNSPEC | IEC958_AES2_CON_CHANNEL_UNSPEC;
> + cs[3] = IEC958_AES3_CON_CLOCK_1000PPM | IEC958_AES3_CON_FS_NOTID;
> +
> + if (len > 4)
> + cs[4] = IEC958_AES4_CON_WORDLEN_NOTID;
> +
> + return len;
> +}
> +EXPORT_SYMBOL_GPL(snd_pcm_create_iec958_consumer_default);
> +
> +static int fill_iec958_consumer(uint rate, uint sample_width,
> + u8 *cs, size_t len)
> +{
> + if (len < 4)
> return -EINVAL;
> +
> + if ((cs[3] & IEC958_AES3_CON_FS) == IEC958_AES3_CON_FS_NOTID) {
> + unsigned int fs;
> +
> + switch (rate) {
> + case 32000:
> + fs = IEC958_AES3_CON_FS_32000;
> + break;
> + case 44100:
> + fs = IEC958_AES3_CON_FS_44100;
> + break;
> + case 48000:
> + fs = IEC958_AES3_CON_FS_48000;
> + break;
> + case 88200:
> + fs = IEC958_AES3_CON_FS_88200;
> + break;
> + case 96000:
> + fs = IEC958_AES3_CON_FS_96000;
> + break;
> + case 176400:
> + fs = IEC958_AES3_CON_FS_176400;
> + break;
> + case 192000:
> + fs = IEC958_AES3_CON_FS_192000;
> + break;
> + default:
> + return -EINVAL;
> + }
> +
> + cs[3] &= ~IEC958_AES3_CON_FS;
> + cs[3] |= fs;
> }
>
> - if (len > 4) {
> + if (len > 4 &&
> + (cs[4] & IEC958_AES4_CON_WORDLEN) == IEC958_AES4_CON_WORDLEN_NOTID) {
> + unsigned int ws;
> +
> switch (sample_width) {
> case 16:
> ws = IEC958_AES4_CON_WORDLEN_20_16;
> @@ -64,21 +108,58 @@ static int create_iec958_consumer(uint rate, uint sample_width,
> default:
> return -EINVAL;
> }
> +
> + cs[4] &= ~IEC958_AES4_CON_WORDLEN;
> + cs[4] |= ws;
> }
>
> - memset(cs, 0, len);
> -
> - cs[0] = IEC958_AES0_CON_NOT_COPYRIGHT | IEC958_AES0_CON_EMPHASIS_NONE;
> - cs[1] = IEC958_AES1_CON_GENERAL;
> - cs[2] = IEC958_AES2_CON_SOURCE_UNSPEC | IEC958_AES2_CON_CHANNEL_UNSPEC;
> - cs[3] = IEC958_AES3_CON_CLOCK_1000PPM | fs;
> -
> - if (len > 4)
> - cs[4] = ws;
> -
> return len;
> }
>
> +/**
> + * snd_pcm_fill_iec958_consumer - Fill consumer format IEC958 channel status
> + * @runtime: pcm runtime structure with ->rate filled in
> + * @cs: channel status buffer, at least four bytes
> + * @len: length of channel status buffer
> + *
> + * Fill the unspecified bits in an IEC958 status bits array using the
> + * parameters of the PCM runtime @runtime.
> + *
> + * Drivers may wish to tweak the contents of the buffer after its been
> + * filled.
> + *
> + * Returns: length of buffer, or negative error code if something failed.
> + */
> +int snd_pcm_fill_iec958_consumer(struct snd_pcm_runtime *runtime,
> + u8 *cs, size_t len)
> +{
> + return fill_iec958_consumer(runtime->rate,
> + snd_pcm_format_width(runtime->format),
> + cs, len);
> +}
> +EXPORT_SYMBOL_GPL(snd_pcm_fill_iec958_consumer);
> +
> +/**
> + * snd_pcm_fill_iec958_consumer_hw_params - Fill consumer format IEC958 channel status
> + * @params: the hw_params instance for extracting rate and sample format
> + * @cs: channel status buffer, at least four bytes
> + * @len: length of channel status buffer
> + *
> + * Fill the unspecified bits in an IEC958 status bits array using the
> + * parameters of the PCM hardware parameters @params.
> + *
> + * Drivers may wish to tweak the contents of the buffer after its been
> + * filled..
> + *
> + * Returns: length of buffer, or negative error code if something failed.
> + */
> +int snd_pcm_fill_iec958_consumer_hw_params(struct snd_pcm_hw_params *params,
> + u8 *cs, size_t len)
> +{
> + return fill_iec958_consumer(params_rate(params), params_width(params), cs, len);
> +}
> +EXPORT_SYMBOL_GPL(snd_pcm_fill_iec958_consumer_hw_params);
> +
> /**
> * snd_pcm_create_iec958_consumer - create consumer format IEC958 channel status
> * @runtime: pcm runtime structure with ->rate filled in
> @@ -95,9 +176,13 @@ static int create_iec958_consumer(uint rate, uint sample_width,
> int snd_pcm_create_iec958_consumer(struct snd_pcm_runtime *runtime, u8 *cs,
> size_t len)
> {
> - return create_iec958_consumer(runtime->rate,
> - snd_pcm_format_width(runtime->format),
> - cs, len);
> + int ret;
> +
> + ret = snd_pcm_create_iec958_consumer_default(cs, len);
> + if (ret < 0)
> + return ret;
> +
> + return snd_pcm_fill_iec958_consumer(runtime, cs, len);
> }
> EXPORT_SYMBOL(snd_pcm_create_iec958_consumer);
>
> @@ -117,7 +202,12 @@ EXPORT_SYMBOL(snd_pcm_create_iec958_consumer);
> int snd_pcm_create_iec958_consumer_hw_params(struct snd_pcm_hw_params *params,
> u8 *cs, size_t len)
> {
> - return create_iec958_consumer(params_rate(params), params_width(params),
> - cs, len);
> + int ret;
> +
> + ret = snd_pcm_create_iec958_consumer_default(cs, len);
> + if (ret < 0)
> + return ret;
> +
> + return fill_iec958_consumer(params_rate(params), params_width(params), cs, len);
> }
> EXPORT_SYMBOL(snd_pcm_create_iec958_consumer_hw_params);
> --
> 2.31.1
>