From: Shengjiu Wang <hidden> Date: 2019-09-19 12:13:17
This patch serial is to update the supported format for fsl_asrc
and fix some format issue.
Shengjiu Wang (4):
ASoC: fsl_asrc: Use in(out)put_format instead of in(out)put_word_width
ASoC: fsl_asrc: update supported sample format
ASoC: pcm_dmaengine: Extract snd_dmaengine_pcm_refine_runtime_hwparams
ASoC: fsl_asrc: Fix error with S24_3LE format bitstream in i.MX8
changes in v2
- extract snd_dmaengine_pcm_set_runtime_hwparams in one
separate path.
- 4th patch depends on 3rd patch
changes in v3
- Fix build report by kbuild test robot [off-list ref]
- change snd_dmaengine_pcm_set_runtime_hwparams to
snd_dmaengine_pcm_refine_runtime_hwparams
include/sound/dmaengine_pcm.h | 5 ++
sound/core/pcm_dmaengine.c | 83 +++++++++++++++++++++++++++
sound/soc/fsl/fsl_asrc.c | 65 ++++++++++++++-------
sound/soc/fsl/fsl_asrc.h | 7 ++-
sound/soc/fsl/fsl_asrc_dma.c | 52 ++++++++++++++---
sound/soc/soc-generic-dmaengine-pcm.c | 62 +++-----------------
6 files changed, 188 insertions(+), 86 deletions(-)
--
2.21.0
From: Shengjiu Wang <hidden> Date: 2019-09-19 12:13:21
snd_pcm_format_t is more formal than enum asrc_word_width, which has
two property, width and physical width, which is more accurate than
enum asrc_word_width. So it is better to use in(out)put_format
instead of in(out)put_word_width.
Signed-off-by: Shengjiu Wang <redacted>
Acked-by: Nicolin Chen <nicoleotsuka@gmail.com>
---
sound/soc/fsl/fsl_asrc.c | 56 +++++++++++++++++++++++++++-------------
sound/soc/fsl/fsl_asrc.h | 4 +--
2 files changed, 40 insertions(+), 20 deletions(-)
@@ -265,6 +265,8 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair)structasrc_config*config=pair->config;structfsl_asrc*asrc_priv=pair->asrc_priv;enumasrc_pair_indexindex=pair->index;+enumasrc_word_widthinput_word_width;+enumasrc_word_widthoutput_word_width;u32inrate,outrate,indiv,outdiv;u32clk_index[2],div[2];intin,out,channels;
@@ -283,9 +285,32 @@ static int fsl_asrc_config_pair(struct fsl_asrc_pair *pair)return-EINVAL;}-/* Validate output width */-if(config->output_word_width==ASRC_WIDTH_8_BIT){-pair_err("does not support 8bit width output\n");+switch(snd_pcm_format_width(config->input_format)){+case8:+input_word_width=ASRC_WIDTH_8_BIT;+break;+case16:+input_word_width=ASRC_WIDTH_16_BIT;+break;+case24:+input_word_width=ASRC_WIDTH_24_BIT;+break;+default:+pair_err("does not support this input format, %d\n",+config->input_format);+return-EINVAL;+}++switch(snd_pcm_format_width(config->output_format)){+case16:+output_word_width=ASRC_WIDTH_16_BIT;+break;+case24:+output_word_width=ASRC_WIDTH_24_BIT;+break;+default:+pair_err("does not support this output format, %d\n",+config->output_format);return-EINVAL;}
From: Shengjiu Wang <hidden> Date: 2019-09-19 12:13:23
When set the runtime hardware parameters, we may need to query
the capability of DMA to complete the parameters.
This patch is to Extract this operation from
dmaengine_pcm_set_runtime_hwparams function to a separate function
snd_dmaengine_pcm_refine_runtime_hwparams, that other components
which need this feature can call this function.
Signed-off-by: Shengjiu Wang <redacted>
---
include/sound/dmaengine_pcm.h | 5 ++
sound/core/pcm_dmaengine.c | 83 +++++++++++++++++++++++++++
sound/soc/soc-generic-dmaengine-pcm.c | 62 +++-----------------
3 files changed, 95 insertions(+), 55 deletions(-)
On Thu, Sep 19, 2019 at 08:11:41PM +0800, Shengjiu Wang wrote:
When set the runtime hardware parameters, we may need to query
the capability of DMA to complete the parameters.
This patch is to Extract this operation from
dmaengine_pcm_set_runtime_hwparams function to a separate function
snd_dmaengine_pcm_refine_runtime_hwparams, that other components
which need this feature can call this function.
Signed-off-by: Shengjiu Wang <redacted>
quoted hunk
@@ -145,58 +140,15 @@ static int dmaengine_pcm_set_runtime_hwparams(struct snd_pcm_substream *substrea
Just a nit, why add a line here? :)
The rest looks good to me, not sure whether the name "refine"
would be the best one though, would like to wait for opinions
from others.
Thanks
From: Shengjiu Wang <hidden> Date: 2019-09-19 12:13:27
There is error "aplay: pcm_write:2023: write error: Input/output error"
on i.MX8QM/i.MX8QXP platform for S24_3LE format.
In i.MX8QM/i.MX8QXP, the DMA is EDMA, which don't support 24bit
sample, but we didn't add any constraint, that cause issues.
So we need to query the caps of dma, then update the hw parameters
according to the caps.
Signed-off-by: Shengjiu Wang <redacted>
---
sound/soc/fsl/fsl_asrc.c | 4 +--
sound/soc/fsl/fsl_asrc.h | 3 +++
sound/soc/fsl/fsl_asrc_dma.c | 52 +++++++++++++++++++++++++++++++-----
3 files changed, 50 insertions(+), 9 deletions(-)
@@ -276,6 +274,11 @@ static int fsl_asrc_dma_startup(struct snd_pcm_substream *substream)structdevice*dev=component->dev;structfsl_asrc*asrc_priv=dev_get_drvdata(dev);structfsl_asrc_pair*pair;+booltx=substream->stream==SNDRV_PCM_STREAM_PLAYBACK;+u8dir=tx?OUT:IN;+structdma_chan*tmp_chan;+structsnd_dmaengine_dai_dma_data*dma_data;+intret;pair=kzalloc(sizeof(structfsl_asrc_pair),GFP_KERNEL);if(!pair)
@@ -285,9 +288,44 @@ static int fsl_asrc_dma_startup(struct snd_pcm_substream *substream)runtime->private_data=pair;-snd_pcm_hw_constraint_integer(substream->runtime,-SNDRV_PCM_HW_PARAM_PERIODS);-snd_soc_set_runtime_hwparams(substream,&snd_imx_hardware);+ret=snd_pcm_hw_constraint_integer(substream->runtime,+SNDRV_PCM_HW_PARAM_PERIODS);+if(ret<0){+dev_err(dev,"failed to set pcm hw params periods\n");+returnret;+}++/* Request a temp pair, which is release in the end */+ret=fsl_asrc_request_pair(1,pair);+if(ret<0){+dev_err(dev,"failed to request asrc pair\n");+returnret;+}++tmp_chan=fsl_asrc_get_dma_channel(pair,dir);+if(!tmp_chan){+dev_err(dev,"can't get dma channel\n");+return-EINVAL;+}++dma_data=snd_soc_dai_get_dma_data(rtd->cpu_dai,substream);++ret=snd_dmaengine_pcm_refine_runtime_hwparams(substream,+dma_data,+&snd_imx_hardware,+tmp_chan);+if(ret<0){+dev_err(dev,"failed to set runtime hwparams\n");+returnret;+}++ret=snd_soc_set_runtime_hwparams(substream,&snd_imx_hardware);+if(ret)+returnret;++dma_release_channel(tmp_chan);+fsl_asrc_release_pair(pair);+return0;}
Hello Shengjiu,
One issue for error-out and some nit-pickings inline. Thanks.
On Thu, Sep 19, 2019 at 08:11:42PM +0800, Shengjiu Wang wrote:
quoted hunk
There is error "aplay: pcm_write:2023: write error: Input/output error"
on i.MX8QM/i.MX8QXP platform for S24_3LE format.
In i.MX8QM/i.MX8QXP, the DMA is EDMA, which don't support 24bit
sample, but we didn't add any constraint, that cause issues.
So we need to query the caps of dma, then update the hw parameters
according to the caps.
Signed-off-by: Shengjiu Wang <redacted>
---
sound/soc/fsl/fsl_asrc.c | 4 +--
sound/soc/fsl/fsl_asrc.h | 3 +++
sound/soc/fsl/fsl_asrc_dma.c | 52 +++++++++++++++++++++++++++++++-----
3 files changed, 50 insertions(+), 9 deletions(-)
Nit: would it be possible to reorganize these a bit? Usually
we put struct things together unless there is a dependency,
similar to fsl_asrc_dma_hw_params().
quoted hunk
@@ -285,9 +288,44 @@ static int fsl_asrc_dma_startup(struct snd_pcm_substream *substream) runtime->private_data = pair;+ /* Request a temp pair, which is release in the end */
Nit: "which will be released later" or "and will release it
later"? And could we use a work like "dummy"? Or at least I
would love to see the comments explaining the parameter "1"
in the function call below.
+ ret = fsl_asrc_request_pair(1, pair);
+ if (ret < 0) {
+ dev_err(dev, "failed to request asrc pair\n");
+ return ret;
+ }
+
+ tmp_chan = fsl_asrc_get_dma_channel(pair, dir);
+ if (!tmp_chan) {
+ dev_err(dev, "can't get dma channel\n");
Could we align with other error messages using "failed to"?
+ ret = snd_soc_set_runtime_hwparams(substream, &snd_imx_hardware);
+ if (ret)
+ return ret;
+
From: Shengjiu Wang <hidden> Date: 2019-09-19 12:13:39
The ASRC support 24bit/16bit/8bit input width, which is
data width, not slot width.
For the S20_3LE format, the data with is 20bit, slot width
is 24bit, if we set ASRMCR1n.IWD to be 24bits, the result
is the volume is lower than expected, it likes 24bit data
right shift 4 bits
So replace S20_3LE with S24_3LE in supported list and add S8
format in TX supported list
Signed-off-by: Shengjiu Wang <redacted>
---
sound/soc/fsl/fsl_asrc.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
On Thu, Sep 19, 2019 at 08:11:40PM +0800, Shengjiu Wang wrote:
The ASRC support 24bit/16bit/8bit input width, which is
data width, not slot width.
For the S20_3LE format, the data with is 20bit, slot width
is 24bit, if we set ASRMCR1n.IWD to be 24bits, the result
is the volume is lower than expected, it likes 24bit data
right shift 4 bits
So replace S20_3LE with S24_3LE in supported list and add S8
format in TX supported list
Signed-off-by: Shengjiu Wang <redacted>