diff mbox

[v3,3/5] ASoC: generic-dmaengine-pcm: Prepare formats mask for valid physical sample sizes

Message ID 1404363116-10538-4-git-send-email-peter.ujfalusi@ti.com (mailing list archive)
State Accepted
Headers show

Commit Message

Peter Ujfalusi July 3, 2014, 4:51 a.m. UTC
Based on the dma_slave_caps's addr_widths queried from the dma driver
prepare the hw.formats mask to include only formats which is supported by
the DMA engine.
In case the dma driver does not implement the slave_caps the default
assumption is that it supports 1, 2 and 4 bytes widths.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>
---

I know that the format validation could be done like this:

+		if (bits > 0 && bits <=64 && !(bits % 8) &&
+		    (addr_widths & (1 << (bits / 8))))
+				hw.formats |= (1LL << i);

But I think the switch() implementation is more readable.


 sound/soc/soc-generic-dmaengine-pcm.c | 37 ++++++++++++++++++++++++++++++++++-
 1 file changed, 36 insertions(+), 1 deletion(-)

Comments

Lars-Peter Clausen July 3, 2014, 8:29 a.m. UTC | #1
On 07/03/2014 06:51 AM, Peter Ujfalusi wrote:
> Based on the dma_slave_caps's addr_widths queried from the dma driver
> prepare the hw.formats mask to include only formats which is supported by
> the DMA engine.
> In case the dma driver does not implement the slave_caps the default
> assumption is that it supports 1, 2 and 4 bytes widths.
>
> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@ti.com>

Looks good, thanks.

Acked-by: Lars-Peter Clausen <lars@metafoo.de>

> ---
>
> I know that the format validation could be done like this:
>
> +		if (bits > 0 && bits <=64 && !(bits % 8) &&
> +		    (addr_widths & (1 << (bits / 8))))
> +				hw.formats |= (1LL << i);
>
> But I think the switch() implementation is more readable.
>
>
>   sound/soc/soc-generic-dmaengine-pcm.c | 37 ++++++++++++++++++++++++++++++++++-
>   1 file changed, 36 insertions(+), 1 deletion(-)
>
> diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
> index 5bace124ef43..6307f85e871b 100644
> --- a/sound/soc/soc-generic-dmaengine-pcm.c
> +++ b/sound/soc/soc-generic-dmaengine-pcm.c
> @@ -119,7 +119,10 @@ static int dmaengine_pcm_set_runtime_hwparams(struct snd_pcm_substream *substrea
>   	struct snd_dmaengine_dai_dma_data *dma_data;
>   	struct dma_slave_caps dma_caps;
>   	struct snd_pcm_hardware hw;
> -	int ret;
> +	u32 addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> +			  BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
> +			  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> +	int i, ret;
>
>   	if (pcm->config && pcm->config->pcm_hardware)
>   		return snd_soc_set_runtime_hwparams(substream,
> @@ -146,6 +149,38 @@ static int dmaengine_pcm_set_runtime_hwparams(struct snd_pcm_substream *substrea
>   			hw.info |= SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME;
>   		if (dma_caps.residue_granularity <= DMA_RESIDUE_GRANULARITY_SEGMENT)
>   			hw.info |= SNDRV_PCM_INFO_BATCH;
> +
> +		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> +			addr_widths = dma_caps.dstn_addr_widths;
> +		else
> +			addr_widths = dma_caps.src_addr_widths;
> +	}
> +
> +	/*
> +	 * 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)))
> +				hw.formats |= (1LL << i);
> +			break;
> +		default:
> +			/* Unsupported types */
> +			break;
> +		}
>   	}
>
>   	return snd_soc_set_runtime_hwparams(substream, &hw);
>

--
To unsubscribe from this list: send the line "unsubscribe dmaengine" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mark Brown July 3, 2014, 12:01 p.m. UTC | #2
On Thu, Jul 03, 2014 at 07:51:54AM +0300, Peter Ujfalusi wrote:
> Based on the dma_slave_caps's addr_widths queried from the dma driver
> prepare the hw.formats mask to include only formats which is supported by
> the DMA engine.
> In case the dma driver does not implement the slave_caps the default
> assumption is that it supports 1, 2 and 4 bytes widths.

Acked-by: Mark Brown <broonie@linaro.org>
Vinod Koul July 3, 2014, 12:02 p.m. UTC | #3
On Thu, Jul 03, 2014 at 01:01:53PM +0100, Mark Brown wrote:
> On Thu, Jul 03, 2014 at 07:51:54AM +0300, Peter Ujfalusi wrote:
> > Based on the dma_slave_caps's addr_widths queried from the dma driver
> > prepare the hw.formats mask to include only formats which is supported by
> > the DMA engine.
> > In case the dma driver does not implement the slave_caps the default
> > assumption is that it supports 1, 2 and 4 bytes widths.
> 
> Acked-by: Mark Brown <broonie@linaro.org>

Do you want this to go thru dmaengine tree??
Mark Brown July 3, 2014, 1:42 p.m. UTC | #4
On Thu, Jul 03, 2014 at 05:32:56PM +0530, Vinod Koul wrote:
> On Thu, Jul 03, 2014 at 01:01:53PM +0100, Mark Brown wrote:

> > Acked-by: Mark Brown <broonie@linaro.org>

> Do you want this to go thru dmaengine tree??

Well, I was expecting either ASoC or core ALSA.
Vinod Koul July 3, 2014, 4:17 p.m. UTC | #5
On Thu, Jul 03, 2014 at 02:42:10PM +0100, Mark Brown wrote:
> On Thu, Jul 03, 2014 at 05:32:56PM +0530, Vinod Koul wrote:
> > On Thu, Jul 03, 2014 at 01:01:53PM +0100, Mark Brown wrote:
> 
> > > Acked-by: Mark Brown <broonie@linaro.org>
> 
> > Do you want this to go thru dmaengine tree??
> 
> Well, I was expecting either ASoC or core ALSA.
Ah okay, wasnt thinking ALSA, either ways am fine.
Usually its asoc/dmaengine :)
Mark Brown July 3, 2014, 4:21 p.m. UTC | #6
On Thu, Jul 03, 2014 at 09:47:08PM +0530, Vinod Koul wrote:
> On Thu, Jul 03, 2014 at 02:42:10PM +0100, Mark Brown wrote:

> > Well, I was expecting either ASoC or core ALSA.

> Ah okay, wasnt thinking ALSA, either ways am fine.
> Usually its asoc/dmaengine :)

Yeah, I don't mind - I just figured if I ack then I won't be blocking if
it's not ASoC.
diff mbox

Patch

diff --git a/sound/soc/soc-generic-dmaengine-pcm.c b/sound/soc/soc-generic-dmaengine-pcm.c
index 5bace124ef43..6307f85e871b 100644
--- a/sound/soc/soc-generic-dmaengine-pcm.c
+++ b/sound/soc/soc-generic-dmaengine-pcm.c
@@ -119,7 +119,10 @@  static int dmaengine_pcm_set_runtime_hwparams(struct snd_pcm_substream *substrea
 	struct snd_dmaengine_dai_dma_data *dma_data;
 	struct dma_slave_caps dma_caps;
 	struct snd_pcm_hardware hw;
-	int ret;
+	u32 addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
+			  BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
+			  BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
+	int i, ret;
 
 	if (pcm->config && pcm->config->pcm_hardware)
 		return snd_soc_set_runtime_hwparams(substream,
@@ -146,6 +149,38 @@  static int dmaengine_pcm_set_runtime_hwparams(struct snd_pcm_substream *substrea
 			hw.info |= SNDRV_PCM_INFO_PAUSE | SNDRV_PCM_INFO_RESUME;
 		if (dma_caps.residue_granularity <= DMA_RESIDUE_GRANULARITY_SEGMENT)
 			hw.info |= SNDRV_PCM_INFO_BATCH;
+
+		if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
+			addr_widths = dma_caps.dstn_addr_widths;
+		else
+			addr_widths = dma_caps.src_addr_widths;
+	}
+
+	/*
+	 * 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)))
+				hw.formats |= (1LL << i);
+			break;
+		default:
+			/* Unsupported types */
+			break;
+		}
 	}
 
 	return snd_soc_set_runtime_hwparams(substream, &hw);