LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
* Re: [PATCH 2/3] ASoC: fsl_asrc: update supported sample format
@ 2019-09-10  2:07 S.j. Wang
  2019-09-12 23:51 ` Nicolin Chen
  0 siblings, 1 reply; 7+ messages in thread
From: S.j. Wang @ 2019-09-10  2:07 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: alsa-devel, timur, Xiubo.Lee, linuxppc-dev, tiwai, lgirdwood,
	perex, broonie, festevam, linux-kernel

Hi

> 
> On Mon, Sep 09, 2019 at 06:33:20PM -0400, Shengjiu Wang wrote:
> > The ASRC support 24bit/16bit/8bit input width, so S20_3LE format
> > should not be supported, it is word width is 20bit.
> 
> I thought 3LE used 24-bit physical width. And the driver assigns
> ASRC_WIDTH_24_BIT to "width" for all non-16bit cases, so 20-bit would go
> for that 24-bit slot also. I don't clearly recall if I had explicitly tested
> S20_3LE, but I feel it should work since I put there...
> 
> Thanks
> Nicolin
> 

For S20_3LE, the width is 20bit,  but the ASRC only support 24bit, if set the
ASRMCR1n.IWD= 24bit, because the actual width is 20 bit, the volume is
Lower than expected,  it likes 24bit data right shift 4 bit. 
So it is not supported.

Best regards
Wang shengjiu 

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] ASoC: fsl_asrc: update supported sample format
  2019-09-10  2:07 [PATCH 2/3] ASoC: fsl_asrc: update supported sample format S.j. Wang
@ 2019-09-12 23:51 ` Nicolin Chen
  2019-09-13  5:48   ` [EXT] " S.j. Wang
  0 siblings, 1 reply; 7+ messages in thread
From: Nicolin Chen @ 2019-09-12 23:51 UTC (permalink / raw)
  To: S.j. Wang
  Cc: alsa-devel, timur, Xiubo.Lee, linuxppc-dev, tiwai, lgirdwood,
	perex, broonie, festevam, linux-kernel

On Tue, Sep 10, 2019 at 02:07:25AM +0000, S.j. Wang wrote:
> > On Mon, Sep 09, 2019 at 06:33:20PM -0400, Shengjiu Wang wrote:
> > > The ASRC support 24bit/16bit/8bit input width, so S20_3LE format
> > > should not be supported, it is word width is 20bit.
> > 
> > I thought 3LE used 24-bit physical width. And the driver assigns
> > ASRC_WIDTH_24_BIT to "width" for all non-16bit cases, so 20-bit would go
> > for that 24-bit slot also. I don't clearly recall if I had explicitly tested
> > S20_3LE, but I feel it should work since I put there...
> 
> For S20_3LE, the width is 20bit,  but the ASRC only support 24bit, if set the
> ASRMCR1n.IWD= 24bit, because the actual width is 20 bit, the volume is
> Lower than expected,  it likes 24bit data right shift 4 bit.
> So it is not supported.

Hmm..S20_3LE right-aligns 20 bits in a 24-bit slot? I thought
they're left aligned...

If this is the case...shouldn't we have the same lower-volume
problem for all hardwares that support S20_3LE now?

^ permalink raw reply	[flat|nested] 7+ messages in thread

* RE: [EXT] Re: [PATCH 2/3] ASoC: fsl_asrc: update supported sample format
  2019-09-12 23:51 ` Nicolin Chen
@ 2019-09-13  5:48   ` " S.j. Wang
  2019-09-16 22:42     ` Nicolin Chen
  0 siblings, 1 reply; 7+ messages in thread
From: S.j. Wang @ 2019-09-13  5:48 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: alsa-devel, timur, Xiubo.Lee, linuxppc-dev, tiwai, lgirdwood,
	perex, broonie, festevam, linux-kernel

Hi

> 
> On Tue, Sep 10, 2019 at 02:07:25AM +0000, S.j. Wang wrote:
> > > On Mon, Sep 09, 2019 at 06:33:20PM -0400, Shengjiu Wang wrote:
> > > > The ASRC support 24bit/16bit/8bit input width, so S20_3LE format
> > > > should not be supported, it is word width is 20bit.
> > >
> > > I thought 3LE used 24-bit physical width. And the driver assigns
> > > ASRC_WIDTH_24_BIT to "width" for all non-16bit cases, so 20-bit
> > > would go for that 24-bit slot also. I don't clearly recall if I had
> > > explicitly tested S20_3LE, but I feel it should work since I put there...
> >
> > For S20_3LE, the width is 20bit,  but the ASRC only support 24bit, if
> > set the ASRMCR1n.IWD= 24bit, because the actual width is 20 bit, the
> > volume is Lower than expected,  it likes 24bit data right shift 4 bit.
> > So it is not supported.
> 
> Hmm..S20_3LE right-aligns 20 bits in a 24-bit slot? I thought they're left
> aligned...
> 
> If this is the case...shouldn't we have the same lower-volume problem for
> all hardwares that support S20_3LE now?

Actually some hardware/module when they do transmission from FIFO
to shift register, they can select the start bit, for example from the 20th
bit. but not all module have this capability.

For ASRC, it haven't.  IWD can only cover the data width,  there is no
Other bit for slot width.

Best regards
Wang shengjiu




^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [EXT] Re: [PATCH 2/3] ASoC: fsl_asrc: update supported sample format
  2019-09-13  5:48   ` [EXT] " S.j. Wang
@ 2019-09-16 22:42     ` Nicolin Chen
  0 siblings, 0 replies; 7+ messages in thread
From: Nicolin Chen @ 2019-09-16 22:42 UTC (permalink / raw)
  To: S.j. Wang
  Cc: alsa-devel, timur, Xiubo.Lee, linuxppc-dev, tiwai, lgirdwood,
	perex, broonie, festevam, linux-kernel

On Fri, Sep 13, 2019 at 05:48:40AM +0000, S.j. Wang wrote:
> Hi
> 
> > 
> > On Tue, Sep 10, 2019 at 02:07:25AM +0000, S.j. Wang wrote:
> > > > On Mon, Sep 09, 2019 at 06:33:20PM -0400, Shengjiu Wang wrote:
> > > > > The ASRC support 24bit/16bit/8bit input width, so S20_3LE format
> > > > > should not be supported, it is word width is 20bit.
> > > >
> > > > I thought 3LE used 24-bit physical width. And the driver assigns
> > > > ASRC_WIDTH_24_BIT to "width" for all non-16bit cases, so 20-bit
> > > > would go for that 24-bit slot also. I don't clearly recall if I had
> > > > explicitly tested S20_3LE, but I feel it should work since I put there...
> > >
> > > For S20_3LE, the width is 20bit,  but the ASRC only support 24bit, if
> > > set the ASRMCR1n.IWD= 24bit, because the actual width is 20 bit, the
> > > volume is Lower than expected,  it likes 24bit data right shift 4 bit.
> > > So it is not supported.
> > 
> > Hmm..S20_3LE right-aligns 20 bits in a 24-bit slot? I thought they're left
> > aligned...
> > 
> > If this is the case...shouldn't we have the same lower-volume problem for
> > all hardwares that support S20_3LE now?
> 
> Actually some hardware/module when they do transmission from FIFO
> to shift register, they can select the start bit, for example from the 20th
> bit. but not all module have this capability.
> 
> For ASRC, it haven't.  IWD can only cover the data width,  there is no
> Other bit for slot width.

Okay..let's drop the S20_3LE then. But would it be possible
for you to elaborate the reasoning into the commit message
also? Just for case when people ask why we remove it simply.

Thanks

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] ASoC: fsl_asrc: update supported sample format
@ 2019-09-18  2:58 S.j. Wang
  0 siblings, 0 replies; 7+ messages in thread
From: S.j. Wang @ 2019-09-18  2:58 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: alsa-devel, timur, Xiubo.Lee, linuxppc-dev, tiwai, lgirdwood,
	perex, broonie, festevam, linux-kernel

Hi

> 
> On Fri, Sep 13, 2019 at 05:48:40AM +0000, S.j. Wang wrote:
> > Hi
> >
> > >
> > > On Tue, Sep 10, 2019 at 02:07:25AM +0000, S.j. Wang wrote:
> > > > > On Mon, Sep 09, 2019 at 06:33:20PM -0400, Shengjiu Wang wrote:
> > > > > > The ASRC support 24bit/16bit/8bit input width, so S20_3LE
> > > > > > format should not be supported, it is word width is 20bit.
> > > > >
> > > > > I thought 3LE used 24-bit physical width. And the driver assigns
> > > > > ASRC_WIDTH_24_BIT to "width" for all non-16bit cases, so 20-bit
> > > > > would go for that 24-bit slot also. I don't clearly recall if I
> > > > > had explicitly tested S20_3LE, but I feel it should work since I put
> there...
> > > >
> > > > For S20_3LE, the width is 20bit,  but the ASRC only support 24bit,
> > > > if set the ASRMCR1n.IWD= 24bit, because the actual width is 20
> > > > bit, the volume is Lower than expected,  it likes 24bit data right shift 4
> bit.
> > > > So it is not supported.
> > >
> > > Hmm..S20_3LE right-aligns 20 bits in a 24-bit slot? I thought
> > > they're left aligned...
> > >
> > > If this is the case...shouldn't we have the same lower-volume
> > > problem for all hardwares that support S20_3LE now?
> >
> > Actually some hardware/module when they do transmission from FIFO to
> > shift register, they can select the start bit, for example from the
> > 20th bit. but not all module have this capability.
> >
> > For ASRC, it haven't.  IWD can only cover the data width,  there is no
> > Other bit for slot width.
> 
> Okay..let's drop the S20_3LE then. But would it be possible for you to
> elaborate the reasoning into the commit message also? Just for case when
> people ask why we remove it simply.
> 
> Thanks

OK.
Best regards
Wang shengjiu

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 2/3] ASoC: fsl_asrc: update supported sample format
  2019-09-09 22:33 [PATCH 0/3] " Shengjiu Wang
@ 2019-09-09 22:33 ` " Shengjiu Wang
  2019-09-09 20:19   ` Nicolin Chen
  0 siblings, 1 reply; 7+ messages in thread
From: Shengjiu Wang @ 2019-09-09 22:33 UTC (permalink / raw)
  To: timur, nicoleotsuka, Xiubo.Lee, festevam, lgirdwood, broonie,
	perex, tiwai, alsa-devel, linuxppc-dev, linux-kernel

The ASRC support 24bit/16bit/8bit input width, so S20_3LE format
should not be supported, it is word width is 20bit.
So replace S20_3LE with S24_3LE in supported list and add S8
format in TX supported list

Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
---
 sound/soc/fsl/fsl_asrc.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
index 4d3804a1ea55..584badf956d2 100644
--- a/sound/soc/fsl/fsl_asrc.c
+++ b/sound/soc/fsl/fsl_asrc.c
@@ -624,7 +624,7 @@ static int fsl_asrc_dai_probe(struct snd_soc_dai *dai)
 
 #define FSL_ASRC_FORMATS	(SNDRV_PCM_FMTBIT_S24_LE | \
 				 SNDRV_PCM_FMTBIT_S16_LE | \
-				 SNDRV_PCM_FMTBIT_S20_3LE)
+				 SNDRV_PCM_FMTBIT_S24_3LE)
 
 static struct snd_soc_dai_driver fsl_asrc_dai = {
 	.probe = fsl_asrc_dai_probe,
@@ -635,7 +635,8 @@ static struct snd_soc_dai_driver fsl_asrc_dai = {
 		.rate_min = 5512,
 		.rate_max = 192000,
 		.rates = SNDRV_PCM_RATE_KNOT,
-		.formats = FSL_ASRC_FORMATS,
+		.formats = FSL_ASRC_FORMATS |
+			   SNDRV_PCM_FMTBIT_S8,
 	},
 	.capture = {
 		.stream_name = "ASRC-Capture",
-- 
2.21.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH 2/3] ASoC: fsl_asrc: update supported sample format
  2019-09-09 22:33 ` [PATCH 2/3] ASoC: fsl_asrc: " Shengjiu Wang
@ 2019-09-09 20:19   ` Nicolin Chen
  0 siblings, 0 replies; 7+ messages in thread
From: Nicolin Chen @ 2019-09-09 20:19 UTC (permalink / raw)
  To: Shengjiu Wang
  Cc: alsa-devel, timur, Xiubo.Lee, linuxppc-dev, tiwai, lgirdwood,
	perex, broonie, festevam, linux-kernel

On Mon, Sep 09, 2019 at 06:33:20PM -0400, Shengjiu Wang wrote:
> The ASRC support 24bit/16bit/8bit input width, so S20_3LE format
> should not be supported, it is word width is 20bit.

I thought 3LE used 24-bit physical width. And the driver assigns
ASRC_WIDTH_24_BIT to "width" for all non-16bit cases, so 20-bit
would go for that 24-bit slot also. I don't clearly recall if I
had explicitly tested S20_3LE, but I feel it should work since
I put there...

Thanks
Nicolin

> So replace S20_3LE with S24_3LE in supported list and add S8
> format in TX supported list
> 
> Signed-off-by: Shengjiu Wang <shengjiu.wang@nxp.com>
> ---
>  sound/soc/fsl/fsl_asrc.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/sound/soc/fsl/fsl_asrc.c b/sound/soc/fsl/fsl_asrc.c
> index 4d3804a1ea55..584badf956d2 100644
> --- a/sound/soc/fsl/fsl_asrc.c
> +++ b/sound/soc/fsl/fsl_asrc.c
> @@ -624,7 +624,7 @@ static int fsl_asrc_dai_probe(struct snd_soc_dai *dai)
>  
>  #define FSL_ASRC_FORMATS	(SNDRV_PCM_FMTBIT_S24_LE | \
>  				 SNDRV_PCM_FMTBIT_S16_LE | \
> -				 SNDRV_PCM_FMTBIT_S20_3LE)
> +				 SNDRV_PCM_FMTBIT_S24_3LE)
>  
>  static struct snd_soc_dai_driver fsl_asrc_dai = {
>  	.probe = fsl_asrc_dai_probe,
> @@ -635,7 +635,8 @@ static struct snd_soc_dai_driver fsl_asrc_dai = {
>  		.rate_min = 5512,
>  		.rate_max = 192000,
>  		.rates = SNDRV_PCM_RATE_KNOT,
> -		.formats = FSL_ASRC_FORMATS,
> +		.formats = FSL_ASRC_FORMATS |
> +			   SNDRV_PCM_FMTBIT_S8,
>  	},
>  	.capture = {
>  		.stream_name = "ASRC-Capture",
> -- 
> 2.21.0
> 

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, back to index

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-09-10  2:07 [PATCH 2/3] ASoC: fsl_asrc: update supported sample format S.j. Wang
2019-09-12 23:51 ` Nicolin Chen
2019-09-13  5:48   ` [EXT] " S.j. Wang
2019-09-16 22:42     ` Nicolin Chen
  -- strict thread matches above, loose matches on Subject: below --
2019-09-18  2:58 S.j. Wang
2019-09-09 22:33 [PATCH 0/3] " Shengjiu Wang
2019-09-09 22:33 ` [PATCH 2/3] ASoC: fsl_asrc: " Shengjiu Wang
2019-09-09 20:19   ` Nicolin Chen

LinuxPPC-Dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linuxppc-dev/0 linuxppc-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linuxppc-dev linuxppc-dev/ https://lore.kernel.org/linuxppc-dev \
		linuxppc-dev@lists.ozlabs.org linuxppc-dev@ozlabs.org linuxppc-dev@archiver.kernel.org
	public-inbox-index linuxppc-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.ozlabs.lists.linuxppc-dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox