[PATCH] ASoC: rockchip: use generic DMA engine configuration

[PATCH] ASoC: rockchip: use generic DMA engine configuration

[John Keeping]

This effectively reverts commit 75b31192fe6a ("ASoC: rockchip: add
config for rockchip dmaengine pcm register").

There doesn't seem to be any rationale given for why these specific
values are helpful.  The generic DMA engine provides sensible defaults
here and works well with Rockchip I2S.

In fact the period size here is really quite restrictive when dealing
with 8 channels of 32-bit data as the effective period size is just 256
frames.

Signed-off-by: John Keeping <john@metanate.com>
---
 sound/soc/rockchip/Makefile       |  3 +--
 sound/soc/rockchip/rockchip_i2s.c |  3 +--
 sound/soc/rockchip/rockchip_pcm.c | 44 -------------------------------
 sound/soc/rockchip/rockchip_pcm.h | 11 --------
 4 files changed, 2 insertions(+), 59 deletions(-)
 delete mode 100644 sound/soc/rockchip/rockchip_pcm.c
 delete mode 100644 sound/soc/rockchip/rockchip_pcm.h

diff --git a/sound/soc/rockchip/Makefile b/sound/soc/rockchip/Makefile
index b10f5e7b136d..6a3e61178152 100644
--- a/sound/soc/rockchip/Makefile
+++ b/sound/soc/rockchip/Makefile
@@ -2,11 +2,10 @@
 # ROCKCHIP Platform Support
 snd-soc-rockchip-i2s-objs := rockchip_i2s.o
 snd-soc-rockchip-i2s-tdm-objs := rockchip_i2s_tdm.o
-snd-soc-rockchip-pcm-objs := rockchip_pcm.o
 snd-soc-rockchip-pdm-objs := rockchip_pdm.o
 snd-soc-rockchip-spdif-objs := rockchip_spdif.o
 
-obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S) += snd-soc-rockchip-i2s.o snd-soc-rockchip-pcm.o
+obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S) += snd-soc-rockchip-i2s.o
 obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S_TDM) += snd-soc-rockchip-i2s-tdm.o
 obj-$(CONFIG_SND_SOC_ROCKCHIP_PDM) += snd-soc-rockchip-pdm.o
 obj-$(CONFIG_SND_SOC_ROCKCHIP_SPDIF) += snd-soc-rockchip-spdif.o
diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
index 7e89f5b0c237..a6d7656c206e 100644
--- a/sound/soc/rockchip/rockchip_i2s.c
+++ b/sound/soc/rockchip/rockchip_i2s.c
@@ -20,7 +20,6 @@
 #include <sound/dmaengine_pcm.h>
 
 #include "rockchip_i2s.h"
-#include "rockchip_pcm.h"
 
 #define DRV_NAME "rockchip-i2s"
 
@@ -756,7 +755,7 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
 		goto err_suspend;
 	}
 
-	ret = rockchip_pcm_platform_register(&pdev->dev);
+	ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
 	if (ret) {
 		dev_err(&pdev->dev, "Could not register PCM\n");
 		goto err_suspend;
diff --git a/sound/soc/rockchip/rockchip_pcm.c b/sound/soc/rockchip/rockchip_pcm.c
deleted file mode 100644
index 02254e42135e..000000000000
--- a/sound/soc/rockchip/rockchip_pcm.c
+++ /dev/null
@@ -1,44 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-only
-/*
- * Copyright (c) 2018 Rockchip Electronics Co. Ltd.
- */
-
-#include <linux/device.h>
-#include <linux/init.h>
-#include <linux/module.h>
-
-#include <sound/core.h>
-#include <sound/pcm.h>
-#include <sound/soc.h>
-#include <sound/dmaengine_pcm.h>
-
-#include "rockchip_pcm.h"
-
-static const struct snd_pcm_hardware snd_rockchip_hardware = {
-	.info			= SNDRV_PCM_INFO_MMAP |
-				  SNDRV_PCM_INFO_MMAP_VALID |
-				  SNDRV_PCM_INFO_PAUSE |
-				  SNDRV_PCM_INFO_RESUME |
-				  SNDRV_PCM_INFO_INTERLEAVED,
-	.period_bytes_min	= 32,
-	.period_bytes_max	= 8192,
-	.periods_min		= 1,
-	.periods_max		= 52,
-	.buffer_bytes_max	= 64 * 1024,
-	.fifo_size		= 32,
-};
-
-static const struct snd_dmaengine_pcm_config rk_dmaengine_pcm_config = {
-	.pcm_hardware = &snd_rockchip_hardware,
-	.prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,
-	.prealloc_buffer_size = 32 * 1024,
-};
-
-int rockchip_pcm_platform_register(struct device *dev)
-{
-	return devm_snd_dmaengine_pcm_register(dev, &rk_dmaengine_pcm_config,
-		SND_DMAENGINE_PCM_FLAG_COMPAT);
-}
-EXPORT_SYMBOL_GPL(rockchip_pcm_platform_register);
-
-MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/rockchip/rockchip_pcm.h b/sound/soc/rockchip/rockchip_pcm.h
deleted file mode 100644
index 7f00e2ce3603..000000000000
--- a/sound/soc/rockchip/rockchip_pcm.h
+++ /dev/null
@@ -1,11 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0-only */
-/*
- * Copyright (c) 2018 Rockchip Electronics Co. Ltd.
- */
-
-#ifndef _ROCKCHIP_PCM_H
-#define _ROCKCHIP_PCM_H
-
-int rockchip_pcm_platform_register(struct device *dev);
-
-#endif
-- 
2.33.1

Re: [PATCH] ASoC: rockchip: use generic DMA engine configuration

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 4830 bytes --]

On Mon, Oct 18, 2021 at 12:48:44PM +0100, John Keeping wrote:
> This effectively reverts commit 75b31192fe6a ("ASoC: rockchip: add
> config for rockchip dmaengine pcm register").

> There doesn't seem to be any rationale given for why these specific
> values are helpful.  The generic DMA engine provides sensible defaults
> here and works well with Rockchip I2S.

> In fact the period size here is really quite restrictive when dealing
> with 8 channels of 32-bit data as the effective period size is just 256
> frames.

Copying in Jianqun who wrote that patch.  If you're reverting a patch
it's generally good to make sure the original author is aware,
particularly if you're unsure as to why the patch does what it does.

> Signed-off-by: John Keeping <john@metanate.com>
> ---
>  sound/soc/rockchip/Makefile       |  3 +--
>  sound/soc/rockchip/rockchip_i2s.c |  3 +--
>  sound/soc/rockchip/rockchip_pcm.c | 44 -------------------------------
>  sound/soc/rockchip/rockchip_pcm.h | 11 --------
>  4 files changed, 2 insertions(+), 59 deletions(-)
>  delete mode 100644 sound/soc/rockchip/rockchip_pcm.c
>  delete mode 100644 sound/soc/rockchip/rockchip_pcm.h
> 
> diff --git a/sound/soc/rockchip/Makefile b/sound/soc/rockchip/Makefile
> index b10f5e7b136d..6a3e61178152 100644
> --- a/sound/soc/rockchip/Makefile
> +++ b/sound/soc/rockchip/Makefile
> @@ -2,11 +2,10 @@
>  # ROCKCHIP Platform Support
>  snd-soc-rockchip-i2s-objs := rockchip_i2s.o
>  snd-soc-rockchip-i2s-tdm-objs := rockchip_i2s_tdm.o
> -snd-soc-rockchip-pcm-objs := rockchip_pcm.o
>  snd-soc-rockchip-pdm-objs := rockchip_pdm.o
>  snd-soc-rockchip-spdif-objs := rockchip_spdif.o
>  
> -obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S) += snd-soc-rockchip-i2s.o snd-soc-rockchip-pcm.o
> +obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S) += snd-soc-rockchip-i2s.o
>  obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S_TDM) += snd-soc-rockchip-i2s-tdm.o
>  obj-$(CONFIG_SND_SOC_ROCKCHIP_PDM) += snd-soc-rockchip-pdm.o
>  obj-$(CONFIG_SND_SOC_ROCKCHIP_SPDIF) += snd-soc-rockchip-spdif.o
> diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
> index 7e89f5b0c237..a6d7656c206e 100644
> --- a/sound/soc/rockchip/rockchip_i2s.c
> +++ b/sound/soc/rockchip/rockchip_i2s.c
> @@ -20,7 +20,6 @@
>  #include <sound/dmaengine_pcm.h>
>  
>  #include "rockchip_i2s.h"
> -#include "rockchip_pcm.h"
>  
>  #define DRV_NAME "rockchip-i2s"
>  
> @@ -756,7 +755,7 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
>  		goto err_suspend;
>  	}
>  
> -	ret = rockchip_pcm_platform_register(&pdev->dev);
> +	ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
>  	if (ret) {
>  		dev_err(&pdev->dev, "Could not register PCM\n");
>  		goto err_suspend;
> diff --git a/sound/soc/rockchip/rockchip_pcm.c b/sound/soc/rockchip/rockchip_pcm.c
> deleted file mode 100644
> index 02254e42135e..000000000000
> --- a/sound/soc/rockchip/rockchip_pcm.c
> +++ /dev/null
> @@ -1,44 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-only
> -/*
> - * Copyright (c) 2018 Rockchip Electronics Co. Ltd.
> - */
> -
> -#include <linux/device.h>
> -#include <linux/init.h>
> -#include <linux/module.h>
> -
> -#include <sound/core.h>
> -#include <sound/pcm.h>
> -#include <sound/soc.h>
> -#include <sound/dmaengine_pcm.h>
> -
> -#include "rockchip_pcm.h"
> -
> -static const struct snd_pcm_hardware snd_rockchip_hardware = {
> -	.info			= SNDRV_PCM_INFO_MMAP |
> -				  SNDRV_PCM_INFO_MMAP_VALID |
> -				  SNDRV_PCM_INFO_PAUSE |
> -				  SNDRV_PCM_INFO_RESUME |
> -				  SNDRV_PCM_INFO_INTERLEAVED,
> -	.period_bytes_min	= 32,
> -	.period_bytes_max	= 8192,
> -	.periods_min		= 1,
> -	.periods_max		= 52,
> -	.buffer_bytes_max	= 64 * 1024,
> -	.fifo_size		= 32,
> -};
> -
> -static const struct snd_dmaengine_pcm_config rk_dmaengine_pcm_config = {
> -	.pcm_hardware = &snd_rockchip_hardware,
> -	.prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,
> -	.prealloc_buffer_size = 32 * 1024,
> -};
> -
> -int rockchip_pcm_platform_register(struct device *dev)
> -{
> -	return devm_snd_dmaengine_pcm_register(dev, &rk_dmaengine_pcm_config,
> -		SND_DMAENGINE_PCM_FLAG_COMPAT);
> -}
> -EXPORT_SYMBOL_GPL(rockchip_pcm_platform_register);
> -
> -MODULE_LICENSE("GPL v2");
> diff --git a/sound/soc/rockchip/rockchip_pcm.h b/sound/soc/rockchip/rockchip_pcm.h
> deleted file mode 100644
> index 7f00e2ce3603..000000000000
> --- a/sound/soc/rockchip/rockchip_pcm.h
> +++ /dev/null
> @@ -1,11 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0-only */
> -/*
> - * Copyright (c) 2018 Rockchip Electronics Co. Ltd.
> - */
> -
> -#ifndef _ROCKCHIP_PCM_H
> -#define _ROCKCHIP_PCM_H
> -
> -int rockchip_pcm_platform_register(struct device *dev);
> -
> -#endif
> -- 
> 2.33.1
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: rockchip: use generic DMA engine configuration【请注意，邮件由linux-rockchip-bounces+sugar.zhang=rock-chips.com@lists.infradead.org代发】

[John Keeping]

On Tue, 19 Oct 2021 10:11:27 +0800
sugar zhang <sugar.zhang@rock-chips.com> wrote:

> Actually, I have submit patch[1] to do the same thing a few weeks ago, 
> and explain the original purpose.
> 
> [1] 
> https://x-lore.kernel.org/linux-rockchip/1632792957-80428-1-git-send-email-sugar.zhang@rock-chips.com/

I missed that one!  I've added my R-b now.

Mark, please ignore this patch and consider Sugar's.


Thanks,
John

> On 2021/10/18 20:46, Mark Brown wrote:
> > On Mon, Oct 18, 2021 at 12:48:44PM +0100, John Keeping wrote:  
> >> This effectively reverts commit 75b31192fe6a ("ASoC: rockchip: add
> >> config for rockchip dmaengine pcm register").
> >> There doesn't seem to be any rationale given for why these specific
> >> values are helpful.  The generic DMA engine provides sensible defaults
> >> here and works well with Rockchip I2S.
> >> In fact the period size here is really quite restrictive when dealing
> >> with 8 channels of 32-bit data as the effective period size is just 256
> >> frames.  
> > Copying in Jianqun who wrote that patch.  If you're reverting a patch
> > it's generally good to make sure the original author is aware,
> > particularly if you're unsure as to why the patch does what it does.
> >  
> >> Signed-off-by: John Keeping <john@metanate.com>
> >> ---
> >>   sound/soc/rockchip/Makefile       |  3 +--
> >>   sound/soc/rockchip/rockchip_i2s.c |  3 +--
> >>   sound/soc/rockchip/rockchip_pcm.c | 44 -------------------------------
> >>   sound/soc/rockchip/rockchip_pcm.h | 11 --------
> >>   4 files changed, 2 insertions(+), 59 deletions(-)
> >>   delete mode 100644 sound/soc/rockchip/rockchip_pcm.c
> >>   delete mode 100644 sound/soc/rockchip/rockchip_pcm.h
> >>
> >> diff --git a/sound/soc/rockchip/Makefile b/sound/soc/rockchip/Makefile
> >> index b10f5e7b136d..6a3e61178152 100644
> >> --- a/sound/soc/rockchip/Makefile
> >> +++ b/sound/soc/rockchip/Makefile
> >> @@ -2,11 +2,10 @@
> >>   # ROCKCHIP Platform Support
> >>   snd-soc-rockchip-i2s-objs := rockchip_i2s.o
> >>   snd-soc-rockchip-i2s-tdm-objs := rockchip_i2s_tdm.o
> >> -snd-soc-rockchip-pcm-objs := rockchip_pcm.o
> >>   snd-soc-rockchip-pdm-objs := rockchip_pdm.o
> >>   snd-soc-rockchip-spdif-objs := rockchip_spdif.o
> >>   
> >> -obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S) += snd-soc-rockchip-i2s.o snd-soc-rockchip-pcm.o
> >> +obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S) += snd-soc-rockchip-i2s.o
> >>   obj-$(CONFIG_SND_SOC_ROCKCHIP_I2S_TDM) += snd-soc-rockchip-i2s-tdm.o
> >>   obj-$(CONFIG_SND_SOC_ROCKCHIP_PDM) += snd-soc-rockchip-pdm.o
> >>   obj-$(CONFIG_SND_SOC_ROCKCHIP_SPDIF) += snd-soc-rockchip-spdif.o
> >> diff --git a/sound/soc/rockchip/rockchip_i2s.c b/sound/soc/rockchip/rockchip_i2s.c
> >> index 7e89f5b0c237..a6d7656c206e 100644
> >> --- a/sound/soc/rockchip/rockchip_i2s.c
> >> +++ b/sound/soc/rockchip/rockchip_i2s.c
> >> @@ -20,7 +20,6 @@
> >>   #include <sound/dmaengine_pcm.h>
> >>   
> >>   #include "rockchip_i2s.h"
> >> -#include "rockchip_pcm.h"
> >>   
> >>   #define DRV_NAME "rockchip-i2s"
> >>   
> >> @@ -756,7 +755,7 @@ static int rockchip_i2s_probe(struct platform_device *pdev)
> >>   		goto err_suspend;
> >>   	}
> >>   
> >> -	ret = rockchip_pcm_platform_register(&pdev->dev);
> >> +	ret = devm_snd_dmaengine_pcm_register(&pdev->dev, NULL, 0);
> >>   	if (ret) {
> >>   		dev_err(&pdev->dev, "Could not register PCM\n");
> >>   		goto err_suspend;
> >> diff --git a/sound/soc/rockchip/rockchip_pcm.c b/sound/soc/rockchip/rockchip_pcm.c
> >> deleted file mode 100644
> >> index 02254e42135e..000000000000
> >> --- a/sound/soc/rockchip/rockchip_pcm.c
> >> +++ /dev/null
> >> @@ -1,44 +0,0 @@
> >> -// SPDX-License-Identifier: GPL-2.0-only
> >> -/*
> >> - * Copyright (c) 2018 Rockchip Electronics Co. Ltd.
> >> - */
> >> -
> >> -#include <linux/device.h>
> >> -#include <linux/init.h>
> >> -#include <linux/module.h>
> >> -
> >> -#include <sound/core.h>
> >> -#include <sound/pcm.h>
> >> -#include <sound/soc.h>
> >> -#include <sound/dmaengine_pcm.h>
> >> -
> >> -#include "rockchip_pcm.h"
> >> -
> >> -static const struct snd_pcm_hardware snd_rockchip_hardware = {
> >> -	.info			= SNDRV_PCM_INFO_MMAP |
> >> -				  SNDRV_PCM_INFO_MMAP_VALID |
> >> -				  SNDRV_PCM_INFO_PAUSE |
> >> -				  SNDRV_PCM_INFO_RESUME |
> >> -				  SNDRV_PCM_INFO_INTERLEAVED,
> >> -	.period_bytes_min	= 32,
> >> -	.period_bytes_max	= 8192,
> >> -	.periods_min		= 1,
> >> -	.periods_max		= 52,
> >> -	.buffer_bytes_max	= 64 * 1024,
> >> -	.fifo_size		= 32,
> >> -};
> >> -
> >> -static const struct snd_dmaengine_pcm_config rk_dmaengine_pcm_config = {
> >> -	.pcm_hardware = &snd_rockchip_hardware,
> >> -	.prepare_slave_config = snd_dmaengine_pcm_prepare_slave_config,
> >> -	.prealloc_buffer_size = 32 * 1024,
> >> -};
> >> -
> >> -int rockchip_pcm_platform_register(struct device *dev)
> >> -{
> >> -	return devm_snd_dmaengine_pcm_register(dev, &rk_dmaengine_pcm_config,
> >> -		SND_DMAENGINE_PCM_FLAG_COMPAT);
> >> -}
> >> -EXPORT_SYMBOL_GPL(rockchip_pcm_platform_register);
> >> -
> >> -MODULE_LICENSE("GPL v2");
> >> diff --git a/sound/soc/rockchip/rockchip_pcm.h b/sound/soc/rockchip/rockchip_pcm.h
> >> deleted file mode 100644
> >> index 7f00e2ce3603..000000000000
> >> --- a/sound/soc/rockchip/rockchip_pcm.h
> >> +++ /dev/null
> >> @@ -1,11 +0,0 @@
> >> -/* SPDX-License-Identifier: GPL-2.0-only */
> >> -/*
> >> - * Copyright (c) 2018 Rockchip Electronics Co. Ltd.
> >> - */
> >> -
> >> -#ifndef _ROCKCHIP_PCM_H
> >> -#define _ROCKCHIP_PCM_H
> >> -
> >> -int rockchip_pcm_platform_register(struct device *dev);
> >> -
> >> -#endif
> >> -- 
> >> 2.33.1
> >>
> >>
> >> _______________________________________________
> >> Linux-rockchip mailing list
> >> Linux-rockchip@lists.infradead.org
> >> http://lists.infradead.org/mailman/listinfo/linux-rockchip  
> 

Re: [PATCH] ASoC: rockchip: use generic DMA engine configuration【请注意，邮件由linux-rockchip-bounces+sugar.zhang=rock-chips.com@lists.infradead.org代发】

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 892 bytes --]

On Tue, Oct 19, 2021 at 10:11:27AM +0800, sugar zhang wrote:
> Hi Mark, John,
> 
> Actually, I have submit patch[1] to do the same thing a few weeks ago, and
> explain the original purpose.
> 
> [1] https://x-lore.kernel.org/linux-rockchip/1632792957-80428-1-git-send-email-sugar.zhang@rock-chips.com/

Please submit patches using subject lines reflecting the style for the
subsystem, this makes it easier for people to identify relevant patches.
Look at what existing commits in the area you're changing are doing and
make sure your subject lines visually resemble what they're doing.
There's no need to resubmit to fix this alone.

Please don't top post, reply in line with needed context.  This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: rockchip: use generic DMA engine configuration【请注意，邮件由linux-rockchip-bounces+sugar.zhang=rock-chips.com@lists.infradead.org代发】

[sugar zhang]

Hi Mark,

On 2021/10/19 20:18, Mark Brown wrote:
> On Tue, Oct 19, 2021 at 10:11:27AM +0800, sugar zhang wrote:
>> Hi Mark, John,
>>
>> Actually, I have submit patch[1] to do the same thing a few weeks ago, and
>> explain the original purpose.
>>
>> [1] https://x-lore.kernel.org/linux-rockchip/1632792957-80428-1-git-send-email-sugar.zhang@rock-chips.com/
> Please submit patches using subject lines reflecting the style for the
> subsystem, this makes it easier for people to identify relevant patches.
> Look at what existing commits in the area you're changing are doing and
> make sure your subject lines visually resemble what they're doing.
> There's no need to resubmit to fix this alone.
ok.
> Please don't top post, reply in line with needed context.  This allows
> readers to readily follow the flow of conversation and understand what
> you are talking about and also helps ensure that everything in the
> discussion is being addressed.
got it, thanks.

-- 
Best Regards!
张学广/Sugar
瑞芯微电子股份有限公司
Rockchip Electronics Co., Ltd.