diff mbox series

ASoC: sunxi: sun4i-i2s: Enable 32-bit audio formats

Message ID 20240326-sunxi_s32-v1-1-899f71dcb1e6@jookia.org (mailing list archive)
State New, archived
Headers show
Series ASoC: sunxi: sun4i-i2s: Enable 32-bit audio formats | expand

Commit Message

John Watts March 26, 2024, 3:50 a.m. UTC
The I2S driver already supports and checks for 32-bit sample sizes,
so indicate this size is available for use in the DAI driver.

This change has been tested on the T113-S3, but it should work just
fine on older hardware as get_sr checks for sample size support
correctly according to each core's datasheet.

Signed-off-by: John Watts <contact@jookia.org>
---
This patch enables support for 32-bit audio in the sunxi I2S driver.

I have only tested this on the Allwinner T113 but I'm fairly
certain it will work on older boards.
---
 sound/soc/sunxi/sun4i-i2s.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


---
base-commit: 72fb52fb0ac44b6a1edd9bc390e44bce3acccd26
change-id: 20240326-sunxi_s32-f2ab25b8e688

Best regards,

Comments

Jernej Škrabec March 27, 2024, 7:53 p.m. UTC | #1
Dne torek, 26. marec 2024 ob 04:50:03 CET je John Watts napisal(a):
> The I2S driver already supports and checks for 32-bit sample sizes,
> so indicate this size is available for use in the DAI driver.
> 
> This change has been tested on the T113-S3, but it should work just
> fine on older hardware as get_sr checks for sample size support
> correctly according to each core's datasheet.
> 
> Signed-off-by: John Watts <contact@jookia.org>
> ---
> This patch enables support for 32-bit audio in the sunxi I2S driver.
> 
> I have only tested this on the Allwinner T113 but I'm fairly
> certain it will work on older boards.
> ---
>  sound/soc/sunxi/sun4i-i2s.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
> index a736f632bf0b..aeea852141c1 100644
> --- a/sound/soc/sunxi/sun4i-i2s.c
> +++ b/sound/soc/sunxi/sun4i-i2s.c
> @@ -1103,7 +1103,8 @@ static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = {
>  
>  #define SUN4I_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | \
>  			 SNDRV_PCM_FMTBIT_S20_LE | \
> -			 SNDRV_PCM_FMTBIT_S24_LE)
> +			 SNDRV_PCM_FMTBIT_S24_LE | \
> +			 SNDRV_PCM_FMTBIT_S32_LE)

I wish it would be that simple. SUN4I_FORMATS is cross section of all I2S
variants that are supported by this driver. If you check A10, you'll see that
it doesn't support S32.

If you want to add support for S32, you'll have to add new quirk for each
variant.

Best regards,
Jernej

>  
>  static struct snd_soc_dai_driver sun4i_i2s_dai = {
>  	.capture = {
> 
> ---
> base-commit: 72fb52fb0ac44b6a1edd9bc390e44bce3acccd26
> change-id: 20240326-sunxi_s32-f2ab25b8e688
> 
> Best regards,
>
John Watts March 28, 2024, 2:21 a.m. UTC | #2
On Wed, Mar 27, 2024 at 08:53:32PM +0100, Jernej Škrabec wrote:
> I wish it would be that simple. SUN4I_FORMATS is cross section of all I2S
> variants that are supported by this driver. If you check A10, you'll see that
> it doesn't support S32.
> 
> If you want to add support for S32, you'll have to add new quirk for each
> variant.

Yes, A10 doesn't support it. But it should error out in hw_params due to
get_sr not supporting 32-bit, no?

> 
> Best regards,
> Jernej

John.
Mark Brown March 28, 2024, 1:56 p.m. UTC | #3
On Thu, Mar 28, 2024 at 01:21:21PM +1100, John Watts wrote:
> On Wed, Mar 27, 2024 at 08:53:32PM +0100, Jernej Škrabec wrote:

> > I wish it would be that simple. SUN4I_FORMATS is cross section of all I2S
> > variants that are supported by this driver. If you check A10, you'll see that
> > it doesn't support S32.

> > If you want to add support for S32, you'll have to add new quirk for each
> > variant.

> Yes, A10 doesn't support it. But it should error out in hw_params due to
> get_sr not supporting 32-bit, no?

The constraints shouldn't be advertising things that hw_params() will
error out on, sometimes there are contingent constraints that prevent
this but something that just doesn't work isn't one of those times.
John Watts March 28, 2024, 2:56 p.m. UTC | #4
On Thu, Mar 28, 2024 at 01:56:42PM +0000, Mark Brown wrote:
> On Thu, Mar 28, 2024 at 01:21:21PM +1100, John Watts wrote:
> > On Wed, Mar 27, 2024 at 08:53:32PM +0100, Jernej Škrabec wrote:
> 
> > > I wish it would be that simple. SUN4I_FORMATS is cross section of all I2S
> > > variants that are supported by this driver. If you check A10, you'll see that
> > > it doesn't support S32.
> 
> > > If you want to add support for S32, you'll have to add new quirk for each
> > > variant.
> 
> > Yes, A10 doesn't support it. But it should error out in hw_params due to
> > get_sr not supporting 32-bit, no?
> 
> The constraints shouldn't be advertising things that hw_params() will
> error out on, sometimes there are contingent constraints that prevent
> this but something that just doesn't work isn't one of those times.

Oh right! I'm not sure how I forgot about this, given I went through this
exact discussion for my wm8782 patch.

I'll try and get v2 patch for this soon. :)

John.
diff mbox series

Patch

diff --git a/sound/soc/sunxi/sun4i-i2s.c b/sound/soc/sunxi/sun4i-i2s.c
index a736f632bf0b..aeea852141c1 100644
--- a/sound/soc/sunxi/sun4i-i2s.c
+++ b/sound/soc/sunxi/sun4i-i2s.c
@@ -1103,7 +1103,8 @@  static const struct snd_soc_dai_ops sun4i_i2s_dai_ops = {
 
 #define SUN4I_FORMATS	(SNDRV_PCM_FMTBIT_S16_LE | \
 			 SNDRV_PCM_FMTBIT_S20_LE | \
-			 SNDRV_PCM_FMTBIT_S24_LE)
+			 SNDRV_PCM_FMTBIT_S24_LE | \
+			 SNDRV_PCM_FMTBIT_S32_LE)
 
 static struct snd_soc_dai_driver sun4i_i2s_dai = {
 	.capture = {