Re: [PATCH 01/11] snd: iec958: split status creation and fill

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 07 May 2021 16:03:24 +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>

The changes look almost good, but please use EXPORT_SYMBOL_GPL() for
newly introduced symbols.  Also, it'd be worth to mention that some
bits update are done only for the default values; if a rate value has
been already set, it won't be overridden by this *_fill_*() call,
that's the intentional behavior, right?

Last but not least, the subject prefix should be "ALSA:" in general :)


thanks,

Takashi

> ---
>  include/sound/pcm_iec958.h |   8 +++
>  sound/core/pcm_iec958.c    | 131 +++++++++++++++++++++++++------------
>  2 files changed, 96 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..a60908efe159 100644
> --- a/sound/core/pcm_iec958.c
> +++ b/sound/core/pcm_iec958.c
> @@ -9,41 +9,68 @@
>  #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)
> +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(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 +91,30 @@ 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;
>  }
>  
> +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(snd_pcm_fill_iec958_consumer_hw_params);
> +
> +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(snd_pcm_fill_iec958_consumer);
> +
>  /**
>   * snd_pcm_create_iec958_consumer - create consumer format IEC958 channel status
>   * @runtime: pcm runtime structure with ->rate filled in
> @@ -95,9 +131,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 +157,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
> 



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux