| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <53B3FF9C.6060102@metafoo.de>
Date: Wed, 02 Jul 2014 14:48:28 +0200
From: Lars-Peter Clausen <lars@...afoo.de>
To: Peter Ujfalusi <peter.ujfalusi@...com>, vinod.koul@...el.com,
dan.j.williams@...el.com, tiwai@...e.de,
Mark Brown <broonie@...nel.org>,
Liam Girdwood <lgirdwood@...il.com>, joelf@...com,
nsekhar@...com
CC: linux-kernel@...r.kernel.org, alsa-devel@...a-project.org,
dmaengine@...r.kernel.org
Subject: Re: [PATCH 4/4] ALSA: pcm_dmaengine: Correct support for 3 physical
bytes samples
On 07/02/2014 02:44 PM, Peter Ujfalusi wrote:
> On 07/02/2014 02:29 PM, Peter Ujfalusi wrote:
>> In case of _3LE/_3BE formats the samples are stored in 3 consecutive bytes
>> without padding it to 4 bytes. This means that the DMA needs to be able to
>> support 3 bytes word length in order to read/write the samples from memory
>> correctly. Originally the code treated 24 bits physical length samples as
>> they were 32 bits which leads to corruption when playing or recording audio.
>>
>> In snd_dmaengine_pcm_open() we should check the slave_caps of the dma if it
>> supports DMA_SLAVE_BUSWIDTH_3_BYTES. Based on this information we initialize
>> the runtime->hw.formats: if DMA_SLAVE_BUSWIDTH_3_BYTES is not supported or
>> the slave_caps is not provided by the driver we mask out the 24 bits
>> physical width sample formats so they will be not available for user space
>> to pick. If the DMA_SLAVE_BUSWIDTH_3_BYTES is supported _3LE/_3BE formats
>> will not be masked so later on they can be valid if both CPU and codec dai
>> supports them.
>>
>> Signed-off-by: Peter Ujfalusi <peter.ujfalusi@...com>
>> ---
>> sound/core/pcm_dmaengine.c | 39 ++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 38 insertions(+), 1 deletion(-)
>>
>> diff --git a/sound/core/pcm_dmaengine.c b/sound/core/pcm_dmaengine.c
>> index d5611ec80381..519b500c5669 100644
>> --- a/sound/core/pcm_dmaengine.c
>> +++ b/sound/core/pcm_dmaengine.c
>> @@ -72,6 +72,8 @@ int snd_hwparams_to_dma_slave_config(const struct snd_pcm_substream *substream,
>> buswidth = DMA_SLAVE_BUSWIDTH_1_BYTE;
>> else if (bits == 16)
>> buswidth = DMA_SLAVE_BUSWIDTH_2_BYTES;
>> + else if (bits == 24)
>> + buswidth = DMA_SLAVE_BUSWIDTH_3_BYTES;
>> else if (bits <= 32)
>> buswidth = DMA_SLAVE_BUSWIDTH_4_BYTES;
>> else
>> @@ -292,8 +294,12 @@ EXPORT_SYMBOL_GPL(snd_dmaengine_pcm_request_channel);
>> int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
>> struct dma_chan *chan)
>> {
>> + struct snd_pcm_runtime *runtime = substream->runtime;
>> struct dmaengine_pcm_runtime_data *prtd;
>> - int ret;
>> + struct dma_slave_caps dma_caps;
>> + bool dma_3bytes_supported = false;
>> + u64 fmt_mask = 0;
>> + int i, ret;
>>
>> if (!chan)
>> return -ENXIO;
>> @@ -311,6 +317,37 @@ int snd_dmaengine_pcm_open(struct snd_pcm_substream *substream,
>>
>> substream->runtime->private_data = prtd;
>>
>> + /*
>> + * Prepare formats mask for valid/allowed sample types. If the dma does
>> + * not support 3bytes word size, it needs to be masked out so user space
>> + * can not use the format which produces corrupted audio.
>> + * If the dma driver does not implement get_slave_caps callback, treat
>> + * it as no 3bytes word support.
>> + */
>> + if (!dma_get_slave_caps(prtd->dma_chan, &dma_caps)) {
>> + u32 addr_widths;
>> +
>> + if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
>> + addr_widths = dma_caps.dstn_addr_widths;
>> + else
>> + addr_widths = dma_caps.src_addr_widths;
>> +
>> + if (addr_widths & BIT(DMA_SLAVE_BUSWIDTH_3_BYTES))
>> + dma_3bytes_supported = true;
>> + }
>> +
>> + for (i = 0; i <= SNDRV_PCM_FORMAT_LAST; i++) {
>> + int bits = snd_pcm_format_physical_width(i);
>> +
>> + if (bits == 24 && !dma_3bytes_supported)
>> + fmt_mask |= (1LL << i);
>> + }
>
> per discussion over the irc with Lars we could extend the masking to other
> widths as well, which would look something like this:
> u32 addr_widths = BIT(DMA_SLAVE_BUSWIDTH_1_BYTE) |
> BIT(DMA_SLAVE_BUSWIDTH_2_BYTES) |
> BIT(DMA_SLAVE_BUSWIDTH_4_BYTES);
> ...
> /*
> * 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.
> */
> if (!dma_get_slave_caps(prtd->dma_chan, &dma_caps)) {
> if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
> addr_widths = dma_caps.dstn_addr_widths;
> else
> addr_widths = dma_caps.src_addr_widths;
> }
>
> for (i = 0; i <= SNDRV_PCM_FORMAT_LAST; i++) {
> int bits = snd_pcm_format_physical_width(i);
>
> switch (bits) {
> case 8:
> if (!(addr_widths & BIT(DMA_SLAVE_BUSWIDTH_1_BYTE)))
> fmt_mask |= (1LL << i);
> break;
> case 16:
> if (!(addr_widths & BIT(DMA_SLAVE_BUSWIDTH_2_BYTES)))
> fmt_mask |= (1LL << i);
> break;
> case 24:
> if (!(addr_widths & BIT(DMA_SLAVE_BUSWIDTH_3_BYTES)))
> fmt_mask |= (1LL << i);
> break;
> case 32:
> if (!(addr_widths & BIT(DMA_SLAVE_BUSWIDTH_4_BYTES)))
> fmt_mask |= (1LL << i);
> break;
> case 64:
> if (!(addr_widths & BIT(DMA_SLAVE_BUSWIDTH_8_BYTES)))
> fmt_mask |= (1LL << i);
> break;
> default:
> fmt_mask |= (1LL << i);
> break;
> }
> }
>
> Is this sounds better?
Sounds good. I think we can assume that DMA_SLAVE_BUSWIDTH_1_BYTE = 1, ...
So (addr_width & BIT(bits / 8)) should work fine and we do not need to
duplicate the case branches.
The other thing is this should go into dmaengine_pcm_set_runtime_hwparams()
where we also restrict the other hardware parameters based on the dmaengine
capabilities.
- Lars
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists