[alsa-devel] [PATCH 1/2] ASoC: soc-compress: add a config item for soc-compress
We don't always need soc-compress in soc, here add a config item SND_SOC_COMPRESS, please add 'select SND_SOC_COMPRESS' to driver Kconfig when it is needed.
Signed-off-by: Jie Yang yang.jie@intel.com --- include/sound/soc.h | 7 +++++++ sound/soc/Kconfig | 6 +++++- sound/soc/Makefile | 3 ++- 3 files changed, 14 insertions(+), 2 deletions(-)
diff --git a/include/sound/soc.h b/include/sound/soc.h index 93df8bf..03c53a9 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -440,7 +440,14 @@ int snd_soc_platform_read(struct snd_soc_platform *platform, int snd_soc_platform_write(struct snd_soc_platform *platform, unsigned int reg, unsigned int val); int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num); +#ifdef CONFIG_SND_SOC_COMPRESS int soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num); +#else +static inline int soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num) +{ + return -EPERM; +} +#endif
struct snd_pcm_substream *snd_soc_get_dai_substream(struct snd_soc_card *card, const char *dai_link, int stream); diff --git a/sound/soc/Kconfig b/sound/soc/Kconfig index e2828e1..de743d9 100644 --- a/sound/soc/Kconfig +++ b/sound/soc/Kconfig @@ -9,7 +9,6 @@ menuconfig SND_SOC select SND_JACK if INPUT=y || INPUT=SND select REGMAP_I2C if I2C select REGMAP_SPI if SPI_MASTER - select SND_COMPRESS_OFFLOAD ---help---
If you want ASoC support, you should say Y here and also to the @@ -30,6 +29,11 @@ config SND_SOC_GENERIC_DMAENGINE_PCM bool select SND_DMAENGINE_PCM
+config SND_SOC_COMPRESS + tristate + select SND_COMPRESS_OFFLOAD + default n + # All the supported SoCs source "sound/soc/adi/Kconfig" source "sound/soc/atmel/Kconfig" diff --git a/sound/soc/Makefile b/sound/soc/Makefile index a0e1ee6..184c1e6 100644 --- a/sound/soc/Makefile +++ b/sound/soc/Makefile @@ -1,6 +1,7 @@ snd-soc-core-objs := soc-core.o soc-dapm.o soc-jack.o soc-cache.o soc-utils.o -snd-soc-core-objs += soc-pcm.o soc-compress.o soc-io.o soc-devres.o soc-ops.o +snd-soc-core-objs += soc-pcm.o soc-io.o soc-devres.o soc-ops.o snd-soc-core-objs += soc-topology.o +snd-soc-core-$(CONFIG_SND_SOC_COMPRESS) += soc-compress.o
ifneq ($(CONFIG_SND_SOC_GENERIC_DMAENGINE_PCM),) snd-soc-core-objs += soc-generic-dmaengine-pcm.o
Split soc-compress into a separate module so we only compile/load it when it's needed.
Signed-off-by: Jie Yang yang.jie@intel.com --- sound/soc/Makefile | 4 +++- sound/soc/soc-compress.c | 20 ++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/sound/soc/Makefile b/sound/soc/Makefile index 184c1e6..2936088 100644 --- a/sound/soc/Makefile +++ b/sound/soc/Makefile @@ -1,7 +1,9 @@ snd-soc-core-objs := soc-core.o soc-dapm.o soc-jack.o soc-cache.o soc-utils.o snd-soc-core-objs += soc-pcm.o soc-io.o soc-devres.o soc-ops.o snd-soc-core-objs += soc-topology.o -snd-soc-core-$(CONFIG_SND_SOC_COMPRESS) += soc-compress.o +snd-soc-compress-objs := soc-compress.o + +obj-$(CONFIG_SND_SOC_COMPRESS) += snd-soc-compress.o
ifneq ($(CONFIG_SND_SOC_GENERIC_DMAENGINE_PCM),) snd-soc-core-objs += soc-generic-dmaengine-pcm.o diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index 025c38f..9683f53 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -19,6 +19,7 @@ #include <linux/delay.h> #include <linux/slab.h> #include <linux/workqueue.h> +#include <linux/module.h> #include <sound/core.h> #include <sound/compress_params.h> #include <sound/compress_driver.h> @@ -703,3 +704,22 @@ compr_err: kfree(compr); return ret; } + +static int __init soc_compress_init(void) +{ + return 0; +} +module_init(soc_compress_init); + +static void __exit soc_compress_exit(void) +{ +} +module_exit(soc_compress_exit); + +/* Module information */ +MODULE_AUTHOR("Namarta Kohli namartax.kohli@intel.com"); +MODULE_AUTHOR("Ramesh Babu K V ramesh.babu@linux.intel.com"); +MODULE_AUTHOR("Vinod Koul vinod.koul@linux.intel.com"); +MODULE_DESCRIPTION("ALSA SoC Compress"); +MODULE_LICENSE("GPL"); +MODULE_ALIAS("platform:soc-compress");
On Fri, Jun 12, 2015 at 07:59:16PM +0800, Jie Yang wrote:
+static int __init soc_compress_init(void) +{
return 0;+} +module_init(soc_compress_init);
+static void __exit soc_compress_exit(void) +{ +} +module_exit(soc_compress_exit);
These are completely empty, why are they being added?
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Friday, June 12, 2015 8:15 PM To: Jie, Yang Cc: tiwai@suse.de; alsa-devel@alsa-project.org; Girdwood, Liam R; vinod.koul@linux.intel.com; ramesh.babu@linux.intel.com; Zhang, Vivian Subject: Re: [PATCH 2/2] ASoC: soc-compress: split soc-compress to a module
On Fri, Jun 12, 2015 at 07:59:16PM +0800, Jie Yang wrote:
+static int __init soc_compress_init(void) {
return 0;+} +module_init(soc_compress_init);
+static void __exit soc_compress_exit(void) { } +module_exit(soc_compress_exit);
These are completely empty, why are they being added?
I imitated that from sound/core/compress_offload.c, let me remove them.
~Keyon
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Friday, June 12, 2015 8:16 PM To: Jie, Yang Cc: tiwai@suse.de; alsa-devel@alsa-project.org; Girdwood, Liam R; vinod.koul@linux.intel.com; ramesh.babu@linux.intel.com; Zhang, Vivian Subject: Re: [PATCH 2/2] ASoC: soc-compress: split soc-compress to a module
On Fri, Jun 12, 2015 at 07:59:16PM +0800, Jie Yang wrote:
+MODULE_ALIAS("platform:soc-compress");
Oh, and this is incorrect - there is no platform device driver in this file.
OK, will remove this line.
On Fri, Jun 12, 2015 at 07:59:15PM +0800, Jie Yang wrote:
+config SND_SOC_COMPRESS
- tristate
- select SND_COMPRESS_OFFLOAD
- default n
No need to specify default n, it is the default. I'm not seeing any change here to make the drivers that do use compressed support select this symbol and since there's no config text there's no way for users to enable this on a per system basis so this breaks all existing compressed audio support.
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Friday, June 12, 2015 8:15 PM To: Jie, Yang Cc: tiwai@suse.de; alsa-devel@alsa-project.org; Girdwood, Liam R; vinod.koul@linux.intel.com; ramesh.babu@linux.intel.com; Zhang, Vivian Subject: Re: [PATCH 1/2] ASoC: soc-compress: add a config item for soc- compress
On Fri, Jun 12, 2015 at 07:59:15PM +0800, Jie Yang wrote:
+config SND_SOC_COMPRESS
- tristate
- select SND_COMPRESS_OFFLOAD
- default n
No need to specify default n, it is the default. I'm not seeing any change
OK, will remove this default setting.
here to make the drivers that do use compressed support select this symbol
As mentioned in commit message, we need add 'select SND_SOC_COMPRESS' to driver Kconfig(e.g. sound/soc/intel/Kconfig) when it is needed.
and since there's no config text there's no way for users to enable this on a per system basis so this breaks all existing compressed audio support.
Do you mean I should add this message to 'Help' for this config item? Per my understanding, no existing driver really uses this compressed audio support currently, is that true?
~Keyon
On Fri, Jun 12, 2015 at 12:31:25PM +0000, Jie, Yang wrote:
here to make the drivers that do use compressed support select this symbol
As mentioned in commit message, we need add 'select SND_SOC_COMPRESS' to driver Kconfig(e.g. sound/soc/intel/Kconfig) when it is needed.
and since there's no config text there's no way for users to enable this on a per system basis so this breaks all existing compressed audio support.
Do you mean I should add this message to 'Help' for this config item? Per my understanding, no existing driver really uses this compressed audio support currently, is that true?
Some of the existing Intel machine drivers have references to compressed audio, it certainly looks like they're trying to use it. Are you saying that this is not actually the case?
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Friday, June 12, 2015 9:00 PM To: Jie, Yang Cc: tiwai@suse.de; alsa-devel@alsa-project.org; Girdwood, Liam R; vinod.koul@linux.intel.com; ramesh.babu@linux.intel.com; Zhang, Vivian Subject: Re: [PATCH 1/2] ASoC: soc-compress: add a config item for soc- compress
On Fri, Jun 12, 2015 at 12:31:25PM +0000, Jie, Yang wrote:
here to make the drivers that do use compressed support select this symbol
As mentioned in commit message, we need add 'select
SND_SOC_COMPRESS'
to driver Kconfig(e.g. sound/soc/intel/Kconfig) when it is needed.
and since there's no config text there's no way for users to enable this on a per system basis so this breaks all existing compressed audio
support.
Do you mean I should add this message to 'Help' for this config item? Per my understanding, no existing driver really uses this compressed audio support currently, is that true?
Some of the existing Intel machine drivers have references to compressed audio, it certainly looks like they're trying to use it. Are you saying that this is not actually the case?
Let me double confirm if the atom drivers are using it. Seems mfld platform is using that.
~Keyon
On Fri, Jun 12, 2015 at 01:33:25PM +0000, Jie, Yang wrote:
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Friday, June 12, 2015 9:00 PM To: Jie, Yang Cc: tiwai@suse.de; alsa-devel@alsa-project.org; Girdwood, Liam R; vinod.koul@linux.intel.com; ramesh.babu@linux.intel.com; Zhang, Vivian Subject: Re: [PATCH 1/2] ASoC: soc-compress: add a config item for soc- compress
Okay didnt realise that its posted to alsa :( I need to remove all ref to @linux id as am not using that anymore
On Fri, Jun 12, 2015 at 12:31:25PM +0000, Jie, Yang wrote:
here to make the drivers that do use compressed support select this symbol
As mentioned in commit message, we need add 'select
SND_SOC_COMPRESS'
to driver Kconfig(e.g. sound/soc/intel/Kconfig) when it is needed.
and since there's no config text there's no way for users to enable this on a per system basis so this breaks all existing compressed audio
support.
Do you mean I should add this message to 'Help' for this config item? Per my understanding, no existing driver really uses this compressed audio support currently, is that true?
Some of the existing Intel machine drivers have references to compressed audio, it certainly looks like they're trying to use it. Are you saying that this is not actually the case?
Let me double confirm if the atom drivers are using it. Seems mfld platform is using that.
Yes and we will get runtime errors on all mfld-atom based drivers so please add that on all atom machines
Since the dummy symbols are present so we wont get build errors. If a driver is using the compressed API maybe we should not add dummy so that we catch ones which are not including this config option, so for that point I would say removal of dummay handlers looks best to me.
-----Original Message----- From: Koul, Vinod Sent: Tuesday, June 16, 2015 4:14 PM To: Jie, Yang Cc: Mark Brown; ramesh.babu@linux.intel.com; alsa-devel@alsa-project.org; tiwai@suse.de; Zhang, Vivian; vinod.koul@linux.intel.com; Girdwood, Liam R Subject: Re: [alsa-devel] [PATCH 1/2] ASoC: soc-compress: add a config item for soc-compress
On Fri, Jun 12, 2015 at 01:33:25PM +0000, Jie, Yang wrote:
-----Original Message----- From: Mark Brown [mailto:broonie@kernel.org] Sent: Friday, June 12, 2015 9:00 PM To: Jie, Yang Cc: tiwai@suse.de; alsa-devel@alsa-project.org; Girdwood, Liam R; vinod.koul@linux.intel.com; ramesh.babu@linux.intel.com; Zhang, Vivian Subject: Re: [PATCH 1/2] ASoC: soc-compress: add a config item for soc- compress
Okay didnt realise that its posted to alsa :( I need to remove all ref to @linux id as am not using that anymore
Seems that's a big patch for replacing. :)
On Fri, Jun 12, 2015 at 12:31:25PM +0000, Jie, Yang wrote:
here to make the drivers that do use compressed support select this symbol
As mentioned in commit message, we need add 'select
SND_SOC_COMPRESS'
to driver Kconfig(e.g. sound/soc/intel/Kconfig) when it is needed.
and since there's no config text there's no way for users to enable this on a per system basis so this breaks all existing compressed audio
support.
Do you mean I should add this message to 'Help' for this config item? Per my understanding, no existing driver really uses this compressed audio support currently, is that true?
Some of the existing Intel machine drivers have references to compressed audio, it certainly looks like they're trying to use it. Are you saying that this is not actually the case?
Let me double confirm if the atom drivers are using it. Seems mfld platform is using that.
Yes and we will get runtime errors on all mfld-atom based drivers so please add that on all atom machines
Since the dummy symbols are present so we wont get build errors. If a driver is using the compressed API maybe we should not add dummy so that we catch ones which are not including this config option, so for that point I would say removal of dummay handlers looks best to me.
Hi, Vinod, as I mentioned in replying to v2 patch in another thread, here cpu_dai->driver->compress_dai is a runtime value, which means we don't know if we need compress API(for our driver) or not at compiling stage. So, reporting error at compiling stage looks not easy to be implemented.
At the same time, we return -EPERM to return and tell caller that compress operation is not permitted.
Anyway, let's discuss further on v2 patches thread.
Thanks, ~Keyon
-- ~Vinod
participants (4)
-
Jie Yang -
Jie, Yang
-
Mark Brown -
Vinod Koul