diff mbox series

dmaengine: imx-sdma: Setting DMA_PRIVATE capability during the probe

Message ID 20220524074933.38413-1-hui.wang@canonical.com (mailing list archive)
State Not Applicable
Headers show
Series dmaengine: imx-sdma: Setting DMA_PRIVATE capability during the probe | expand

Commit Message

Hui Wang May 24, 2022, 7:49 a.m. UTC
We have an imx6sx EVB, the audio driver fails to get a valid dma chan
and the audio can't work at all on this board, below is the error log:
 fsl-ssi-dai 202c000.ssi: Missing dma channel for stream: 0
 202c000.ssi-nau8822-hifi: ASoC: pcm constructor failed: -22
 asoc-simple-card sound: ASoC: can't create pcm 202c000.ssi-nau8822-hifi :-22

Then I checked the usage_count of each dma chan through sysfs, all
channels are occupied as below:
ubuntu@ubuntu:cd /sys/devices/platform/soc/2000000.bus/20ec000.sdma/dma
ubuntu@ubuntu:find . -iname in_use | xargs cat
2
2
2
...

Through debugging, we found the root cause, the
crypo/async_tx/async_tx.c calls the dmaengine_get() ahead of
registration of dma_device from imx-sdma.c. In the dmaengine_get(), the
dmaengine_ref_count will be increased, then in the
dma_async_device_register(), the client_count of each chan will be
increased.

To fix this issue, we could set DMA_PRIVATE to the dma_deivce before
registration.

Signed-off-by: Hui Wang <hui.wang@canonical.com>
---
 drivers/dma/imx-sdma.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Hui Wang June 17, 2022, 2:48 a.m. UTC | #1
Sorry, re-send the mail since the previous one contains the 
html-subparts, it is banned by dmaengine mailist.


Hi vkoul, hauer and shawnguo,

Could you take a look at this patch and the problem the patch plans to fix?

It is easy to reproduce the problem on i.mx platforms, just enabling the 
IMX_SDMA, ASYNC_CORE and ASYNC_TX_DMA as below:

CONFIG_MD=y
CONFIG_BLK_DEV_MD=y
CONFIG_MD_AUTODETECT=y
CONFIG_MD_RAID456=y

CONFIG_IMX_SDMA=y

CONFIG_ASYNC_TX_DMA=y
CONFIG_DMATEST=m
CONFIG_DMA_ENGINE_RAID=y

CONFIG_XOR_BLOCKS=y
CONFIG_ASYNC_CORE=y
CONFIG_ASYNC_MEMCPY=y
CONFIG_ASYNC_XOR=y
CONFIG_ASYNC_PQ=y
CONFIG_ASYNC_RAID6_RECOV=y
CONFIG_CRYPTO=y

And this is the .config of the kernel which could reproduce the problem:

https://pastebin.ubuntu.com/p/5Pb3JzrWJZ/


And this is the dmesg of v5.19-rc2 on an i.mx6 platform, we could see 
audio driver fails because of dma allocation:

https://pastebin.ubuntu.com/p/rz7jsXgyJJ/

[    5.779584] fsl-ssi-dai 202c000.ssi: Missing dma channel for stream: 0
[    5.779601] fsl-ssi-dai 202c000.ssi: ASoC: error at 
snd_soc_pcm_component_new on 202c000.ssi: -22
[    5.779616] fsl-asoc-card sound-nau8822: ASoC: can't create pcm HiFi :-22
[    5.784479] fsl-asoc-card sound-nau8822: error -EINVAL: 
snd_soc_register_card failed
[    5.784501] fsl-asoc-card: probe of sound-nau8822 failed with error -22

And this problem also could be reproduced on i.mx8 platforms, If needing 
me to provide log for i.mx8, let me know.

Thanks,

On 5/24/22 15:49, Hui Wang wrote:
> We have an imx6sx EVB, the audio driver fails to get a valid dma chan
> and the audio can't work at all on this board, below is the error log:
>   fsl-ssi-dai 202c000.ssi: Missing dma channel for stream: 0
>   202c000.ssi-nau8822-hifi: ASoC: pcm constructor failed: -22
>   asoc-simple-card sound: ASoC: can't create pcm 202c000.ssi-nau8822-hifi :-22
>
> Then I checked the usage_count of each dma chan through sysfs, all
> channels are occupied as below:
> ubuntu@ubuntu:cd /sys/devices/platform/soc/2000000.bus/20ec000.sdma/dma
> ubuntu@ubuntu:find . -iname in_use | xargs cat
> 2
> 2
> 2
> ...
>
> Through debugging, we found the root cause, the
> crypo/async_tx/async_tx.c calls the dmaengine_get() ahead of
> registration of dma_device from imx-sdma.c. In the dmaengine_get(), the
> dmaengine_ref_count will be increased, then in the
> dma_async_device_register(), the client_count of each chan will be
> increased.
>
> To fix this issue, we could set DMA_PRIVATE to the dma_deivce before
> registration.
>
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>   drivers/dma/imx-sdma.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 95367a8a81a5..aabe8a8069fb 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -2201,6 +2201,7 @@ static int sdma_probe(struct platform_device *pdev)
>   	for (i = 0; i < sizeof(*sdma->script_addrs) / sizeof(s32); i++)
>   		saddr_arr[i] = -EINVAL;
>   
> +	dma_cap_set(DMA_PRIVATE, sdma->dma_device.cap_mask);
>   	dma_cap_set(DMA_SLAVE, sdma->dma_device.cap_mask);
>   	dma_cap_set(DMA_CYCLIC, sdma->dma_device.cap_mask);
>   	dma_cap_set(DMA_MEMCPY, sdma->dma_device.cap_mask);
Sascha Hauer June 20, 2022, 8:33 a.m. UTC | #2
On Tue, May 24, 2022 at 03:49:33PM +0800, Hui Wang wrote:
> We have an imx6sx EVB, the audio driver fails to get a valid dma chan
> and the audio can't work at all on this board, below is the error log:
>  fsl-ssi-dai 202c000.ssi: Missing dma channel for stream: 0
>  202c000.ssi-nau8822-hifi: ASoC: pcm constructor failed: -22
>  asoc-simple-card sound: ASoC: can't create pcm 202c000.ssi-nau8822-hifi :-22
> 
> Then I checked the usage_count of each dma chan through sysfs, all
> channels are occupied as below:
> ubuntu@ubuntu:cd /sys/devices/platform/soc/2000000.bus/20ec000.sdma/dma
> ubuntu@ubuntu:find . -iname in_use | xargs cat
> 2
> 2
> 2
> ...
> 
> Through debugging, we found the root cause, the
> crypo/async_tx/async_tx.c calls the dmaengine_get() ahead of
> registration of dma_device from imx-sdma.c. In the dmaengine_get(), the
> dmaengine_ref_count will be increased, then in the
> dma_async_device_register(), the client_count of each chan will be
> increased.
> 
> To fix this issue, we could set DMA_PRIVATE to the dma_deivce before
> registration.
> 
> Signed-off-by: Hui Wang <hui.wang@canonical.com>
> ---
>  drivers/dma/imx-sdma.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
> index 95367a8a81a5..aabe8a8069fb 100644
> --- a/drivers/dma/imx-sdma.c
> +++ b/drivers/dma/imx-sdma.c
> @@ -2201,6 +2201,7 @@ static int sdma_probe(struct platform_device *pdev)
>  	for (i = 0; i < sizeof(*sdma->script_addrs) / sizeof(s32); i++)
>  		saddr_arr[i] = -EINVAL;
>  
> +	dma_cap_set(DMA_PRIVATE, sdma->dma_device.cap_mask);

I am not sure about the impacts on the memcpy capability of the SDMA
driver when setting this flag. It looks like this flag influences the
way suitable channels are picked for memcpy, but I don't understand
the code just by looking at it. I see that several other drivers
providing memcpy set this flag as well, so I guess it's ok to set it,
but it would be good to hear a word from Vinod about it.

Sascha
Vinod Koul July 1, 2022, 4:34 p.m. UTC | #3
On 20-06-22, 10:33, Sascha Hauer wrote:
> On Tue, May 24, 2022 at 03:49:33PM +0800, Hui Wang wrote:

> > +	dma_cap_set(DMA_PRIVATE, sdma->dma_device.cap_mask);
> 
> I am not sure about the impacts on the memcpy capability of the SDMA
> driver when setting this flag. It looks like this flag influences the
> way suitable channels are picked for memcpy, but I don't understand
> the code just by looking at it. I see that several other drivers
> providing memcpy set this flag as well, so I guess it's ok to set it,
> but it would be good to hear a word from Vinod about it.

So DMA_PRIVATE is used to make channel not available for general dma
allocation. So yes it would impact dma memcpy which is about allocating
generically.
diff mbox series

Patch

diff --git a/drivers/dma/imx-sdma.c b/drivers/dma/imx-sdma.c
index 95367a8a81a5..aabe8a8069fb 100644
--- a/drivers/dma/imx-sdma.c
+++ b/drivers/dma/imx-sdma.c
@@ -2201,6 +2201,7 @@  static int sdma_probe(struct platform_device *pdev)
 	for (i = 0; i < sizeof(*sdma->script_addrs) / sizeof(s32); i++)
 		saddr_arr[i] = -EINVAL;
 
+	dma_cap_set(DMA_PRIVATE, sdma->dma_device.cap_mask);
 	dma_cap_set(DMA_SLAVE, sdma->dma_device.cap_mask);
 	dma_cap_set(DMA_CYCLIC, sdma->dma_device.cap_mask);
 	dma_cap_set(DMA_MEMCPY, sdma->dma_device.cap_mask);