LinuxPPC-Dev Archive on lore.kernel.org
 help / color / Atom feed
From: Nicolin Chen <nicoleotsuka@gmail.com>
To: Shengjiu Wang <shengjiu.wang@nxp.com>
Cc: alsa-devel@alsa-project.org, timur@kernel.org,
	Xiubo.Lee@gmail.com, linuxppc-dev@lists.ozlabs.org,
	tiwai@suse.com, lgirdwood@gmail.com, perex@perex.cz,
	broonie@kernel.org, festevam@gmail.com,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH 3/3] ASoC: fsl_asrc: Fix error with S24_3LE format bitstream in i.MX8
Date: Mon, 9 Sep 2019 18:52:13 -0700
Message-ID: <20190910015212.GA16760@Asurada-Nvidia.nvidia.com> (raw)
In-Reply-To: <2b6e028ca27b8569da4ab7868d7b90ff8c3225d0.1568025083.git.shengjiu.wang@nxp.com>

On Mon, Sep 09, 2019 at 06:33:21PM -0400, Shengjiu Wang wrote:
> 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.

> @@ -285,8 +293,81 @@ 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);
> +	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");
> +		return ret;
> +	}
> +
> +	dma_data = snd_soc_dai_get_dma_data(rtd->cpu_dai, substream);
> +
> +	/* Request a temp pair, which is release in the end */
> +	fsl_asrc_request_pair(1, pair);

Not sure if it'd be practical, but a pair request could fail. Will
probably need to check return value.

And a quick feeling is that below code is mostly identical to what
is in the soc-generic-dmaengine-pcm.c file. So I'm wondering if we
could abstract a helper function somewhere in the ASoC core: Mark?

Thanks
Nicolin

> +	tmp_chan = fsl_asrc_get_dma_channel(pair, dir);
> +	if (!tmp_chan) {
> +		dev_err(dev, "can't get dma channel\n");
> +		return -EINVAL;
> +	}
> +
> +	ret = dma_get_slave_caps(tmp_chan, &dma_caps);
> +	if (ret == 0) {
> +		if (dma_caps.cmd_pause)
> +			snd_imx_hardware.info |= SNDRV_PCM_INFO_PAUSE |
> +						 SNDRV_PCM_INFO_RESUME;
> +		if (dma_caps.residue_granularity <=
> +			DMA_RESIDUE_GRANULARITY_SEGMENT)
> +			snd_imx_hardware.info |= SNDRV_PCM_INFO_BATCH;
> +
> +		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> +			addr_widths = dma_caps.dst_addr_widths;
> +		else
> +			addr_widths = dma_caps.src_addr_widths;
> +	}
> +
> +	/*
> +	 * If SND_DMAENGINE_PCM_DAI_FLAG_PACK is set keep
> +	 * hw.formats set to 0, meaning no restrictions are in place.
> +	 * In this case it's the responsibility of the DAI driver to
> +	 * provide the supported format information.
> +	 */
> +	if (!(dma_data->flags & SND_DMAENGINE_PCM_DAI_FLAG_PACK))
> +		/*
> +		 * Prepare formats mask for valid/allowed sample types. If the
> +		 * dma does not have support for the given physical word size,
> +		 * it needs to be masked out so user space can not use the
> +		 * format which produces corrupted audio.
> +		 * In case the dma driver does not implement the slave_caps the
> +		 * default assumption is that it supports 1, 2 and 4 bytes
> +		 * widths.
> +		 */
> +		for (i = 0; i <= SNDRV_PCM_FORMAT_LAST; i++) {
> +			int bits = snd_pcm_format_physical_width(i);
> +
> +			/*
> +			 * Enable only samples with DMA supported physical
> +			 * widths
> +			 */
> +			switch (bits) {
> +			case 8:
> +			case 16:
> +			case 24:
> +			case 32:
> +			case 64:
> +				if (addr_widths & (1 << (bits / 8)))
> +					snd_imx_hardware.formats |= (1LL << i);
> +				break;
> +			default:
> +				/* Unsupported types */
> +				break;
> +			}
> +		}
> +
> +	if (tmp_chan)
> +		dma_release_channel(tmp_chan);
> +	fsl_asrc_release_pair(pair);
> +
>  	snd_soc_set_runtime_hwparams(substream, &snd_imx_hardware);
>  
>  	return 0;
> -- 
> 2.21.0
> 

  reply index

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-09 22:33 [PATCH 0/3] update supported sample format Shengjiu Wang
2019-09-09 22:33 ` [PATCH 1/3] ASoC: fsl_asrc: Use in(out)put_format instead of in(out)put_word_width Shengjiu Wang
2019-09-09 20:01   ` Nicolin Chen
2019-09-10  2:22     ` [EXT] " S.j. Wang
2019-09-12 23:21       ` Nicolin Chen
2019-09-12 23:21   ` Nicolin Chen
2019-09-09 22:33 ` [PATCH 2/3] ASoC: fsl_asrc: update supported sample format Shengjiu Wang
2019-09-09 20:19   ` Nicolin Chen
2019-09-09 22:33 ` [PATCH 3/3] ASoC: fsl_asrc: Fix error with S24_3LE format bitstream in i.MX8 Shengjiu Wang
2019-09-10  1:52   ` Nicolin Chen [this message]
2019-09-10  2:24     ` [EXT] " S.j. Wang
2019-09-11 11:08     ` Mark Brown
2019-09-12 23:53       ` Nicolin Chen

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190910015212.GA16760@Asurada-Nvidia.nvidia.com \
    --to=nicoleotsuka@gmail.com \
    --cc=Xiubo.Lee@gmail.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=festevam@gmail.com \
    --cc=lgirdwood@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=perex@perex.cz \
    --cc=shengjiu.wang@nxp.com \
    --cc=timur@kernel.org \
    --cc=tiwai@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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
	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.git