| Message ID | 1444750860-19355-1-git-send-email-yang.jie@intel.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 6f0c42269f000b1e346c84d9a589f17aa94c96d8 |
| Headers | show |
On Tue, Oct 13, 2015 at 11:41:00PM +0800, Jie Yang wrote: > We don't always need soc-compress in soc, here add a config item > SND_SOC_COMPRESS, when nobody select it, the soc-compress will > not be compiled. > > Here also change Kconfig to 'select SND_SOC_COMPRESS' for drivers > that needed soc-compress. > > Signed-off-by: Jie Yang <yang.jie@intel.com> > --- > include/sound/soc-dai.h | 2 +- > include/sound/soc.h | 4 +++- > sound/soc/Kconfig | 5 ++++- > sound/soc/Makefile | 3 ++- > sound/soc/intel/Kconfig | 1 + > sound/soc/intel/atom/sst-mfld-platform-pcm.c | 2 +- > sound/soc/soc-compress.c | 12 ++++++++++-- > sound/soc/soc-core.c | 4 ++-- > 8 files changed, 24 insertions(+), 9 deletions(-) > > diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h > index 2df96b1..238200f 100644 > --- a/include/sound/soc-dai.h > +++ b/include/sound/soc-dai.h > @@ -214,7 +214,7 @@ struct snd_soc_dai_driver { > int (*suspend)(struct snd_soc_dai *dai); > int (*resume)(struct snd_soc_dai *dai); > /* compress dai */ > - bool compress_dai; > + int (*compress_new)(struct snd_soc_pcm_runtime *rtd, int num); This feels a little awkward to be using a function pointer here. It somewhat implies I might want to customise this function but am I every going to want to set this to something other than snd_soc_new_compress? > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c > index 3b471f9..24b0960 100644 > --- a/sound/soc/soc-core.c > +++ b/sound/soc/soc-core.c > @@ -1370,9 +1370,9 @@ static int soc_probe_link_dais(struct snd_soc_card *card, int num, int order) > soc_dpcm_debugfs_add(rtd); > #endif > > - if (cpu_dai->driver->compress_dai) { > + if (cpu_dai->driver->compress_new) { Would it maybe be nicer to just change this to: if (IS_ENABLED(..)) && cpu_dai...) Anyone else have any thoughts on that? > /*create compress_device"*/ > - ret = soc_new_compress(rtd, num); > + ret = cpu_dai->driver->compress_new(rtd, num); > if (ret < 0) { > dev_err(card->dev, "ASoC: can't create compress %s\n", > dai_link->stream_name); Thanks, Charles
On Wed, Oct 14, 2015 at 12:57:38PM +0000, Jie, Yang wrote: > > > int (*suspend)(struct snd_soc_dai *dai); > > > int (*resume)(struct snd_soc_dai *dai); > > > /* compress dai */ > > > - bool compress_dai; > > > + int (*compress_new)(struct snd_soc_pcm_runtime *rtd, int num); > > > > This feels a little awkward to be using a function pointer here. > > It somewhat implies I might want to customise this function but am I every > > going to want to set this to something other than snd_soc_new_compress? > > Hi Charles, we used compress_dai bool before(in v2), but for that we need > add empty inline soc_compress_new(), Takashi suggest we use callback > func pointer here, it can be compress_new or any other creation here. > This is flexible and can be extended if any new stuff has to be handled. If everyone else is happy with it, then I don't mind. It obviously works, just felt a little odd to me. Thanks, Charles
> -----Original Message----- > From: Charles Keepax [mailto:ckeepax@opensource.wolfsonmicro.com] > Sent: Wednesday, October 14, 2015 5:21 PM > To: Jie, Yang > Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R > Subject: Re: [alsa-devel] [PATCH v4] ASoC: soc-compress: add config item for > soc-compress to make it compiled only when needed > > On Tue, Oct 13, 2015 at 11:41:00PM +0800, Jie Yang wrote: > > We don't always need soc-compress in soc, here add a config item > > SND_SOC_COMPRESS, when nobody select it, the soc-compress will not be > > compiled. > > > > Here also change Kconfig to 'select SND_SOC_COMPRESS' for drivers that > > needed soc-compress. > > > > Signed-off-by: Jie Yang <yang.jie@intel.com> > > --- > > include/sound/soc-dai.h | 2 +- > > include/sound/soc.h | 4 +++- > > sound/soc/Kconfig | 5 ++++- > > sound/soc/Makefile | 3 ++- > > sound/soc/intel/Kconfig | 1 + > > sound/soc/intel/atom/sst-mfld-platform-pcm.c | 2 +- > > sound/soc/soc-compress.c | 12 ++++++++++-- > > sound/soc/soc-core.c | 4 ++-- > > 8 files changed, 24 insertions(+), 9 deletions(-) > > > > diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index > > 2df96b1..238200f 100644 > > --- a/include/sound/soc-dai.h > > +++ b/include/sound/soc-dai.h > > @@ -214,7 +214,7 @@ struct snd_soc_dai_driver { > > int (*suspend)(struct snd_soc_dai *dai); > > int (*resume)(struct snd_soc_dai *dai); > > /* compress dai */ > > - bool compress_dai; > > + int (*compress_new)(struct snd_soc_pcm_runtime *rtd, int num); > > This feels a little awkward to be using a function pointer here. > It somewhat implies I might want to customise this function but am I every > going to want to set this to something other than snd_soc_new_compress? Hi Charles, we used compress_dai bool before(in v2), but for that we need add empty inline soc_compress_new(), Takashi suggest we use callback func pointer here, it can be compress_new or any other creation here. This is flexible and can be extended if any new stuff has to be handled. > > > diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index > > 3b471f9..24b0960 100644 > > --- a/sound/soc/soc-core.c > > +++ b/sound/soc/soc-core.c > > @@ -1370,9 +1370,9 @@ static int soc_probe_link_dais(struct > snd_soc_card *card, int num, int order) > > soc_dpcm_debugfs_add(rtd); > > #endif > > > > - if (cpu_dai->driver->compress_dai) { > > + if (cpu_dai->driver->compress_new) { > > Would it maybe be nicer to just change this to: > if (IS_ENABLED(..)) && cpu_dai...) This is roll back to the old compress_dai bool? ~Keyon > > Anyone else have any thoughts on that? > > > /*create compress_device"*/ > > - ret = soc_new_compress(rtd, num); > > + ret = cpu_dai->driver->compress_new(rtd, num); > > if (ret < 0) { > > dev_err(card->dev, "ASoC: can't create > compress %s\n", > > dai_link->stream_name); > > Thanks, > Charles
> -----Original Message----- > From: Charles Keepax [mailto:ckeepax@opensource.wolfsonmicro.com] > Sent: Wednesday, October 14, 2015 8:47 PM > To: Jie, Yang > Cc: broonie@kernel.org; alsa-devel@alsa-project.org; Girdwood, Liam R > Subject: Re: [alsa-devel] [PATCH v4] ASoC: soc-compress: add config item for > soc-compress to make it compiled only when needed > > On Wed, Oct 14, 2015 at 12:57:38PM +0000, Jie, Yang wrote: > > > > int (*suspend)(struct snd_soc_dai *dai); > > > > int (*resume)(struct snd_soc_dai *dai); > > > > /* compress dai */ > > > > - bool compress_dai; > > > > + int (*compress_new)(struct snd_soc_pcm_runtime *rtd, int num); > > > > > > This feels a little awkward to be using a function pointer here. > > > It somewhat implies I might want to customise this function but am I > > > every going to want to set this to something other than > snd_soc_new_compress? > > > > Hi Charles, we used compress_dai bool before(in v2), but for that we > > need add empty inline soc_compress_new(), Takashi suggest we use > > callback func pointer here, it can be compress_new or any other creation > here. > > This is flexible and can be extended if any new stuff has to be handled. > > If everyone else is happy with it, then I don't mind. It obviously works, just > felt a little odd to me. Umm, maybe we can extend it in the future when new stuff emerge. Hi Mark, sorry to bother you, do you have any more comment about this patch? Thanks, ~Keyon > > Thanks, > Charles
On Wed, Oct 14, 2015 at 01:46:59PM +0100, Charles Keepax wrote: > On Wed, Oct 14, 2015 at 12:57:38PM +0000, Jie, Yang wrote: > > Hi Charles, we used compress_dai bool before(in v2), but for that we need > > add empty inline soc_compress_new(), Takashi suggest we use callback > > func pointer here, it can be compress_new or any other creation here. > > This is flexible and can be extended if any new stuff has to be handled. > If everyone else is happy with it, then I don't mind. It > obviously works, just felt a little odd to me. Yes, it's not very nice but it does help finesse things. :/
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 2df96b1..238200f 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -214,7 +214,7 @@ struct snd_soc_dai_driver { int (*suspend)(struct snd_soc_dai *dai); int (*resume)(struct snd_soc_dai *dai); /* compress dai */ - bool compress_dai; + int (*compress_new)(struct snd_soc_pcm_runtime *rtd, int num); /* DAI is also used for the control bus */ bool bus_control; diff --git a/include/sound/soc.h b/include/sound/soc.h index 98e080b..10b7802 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -459,7 +459,9 @@ 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); -int soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num); +#ifdef CONFIG_SND_SOC_COMPRESS +int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num); +#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 7de792b..7ff7d88 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,10 @@ config SND_SOC_GENERIC_DMAENGINE_PCM bool select SND_DMAENGINE_PCM +config SND_SOC_COMPRESS + bool + select SND_COMPRESS_OFFLOAD + config SND_SOC_TOPOLOGY bool diff --git a/sound/soc/Makefile b/sound/soc/Makefile index af0a571..8eb06db 100644 --- a/sound/soc/Makefile +++ b/sound/soc/Makefile @@ -1,5 +1,6 @@ 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-$(CONFIG_SND_SOC_COMPRESS) += soc-compress.o ifneq ($(CONFIG_SND_SOC_TOPOLOGY),) snd-soc-core-objs += soc-topology.o diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig index 664df1f..aaafb13 100644 --- a/sound/soc/intel/Kconfig +++ b/sound/soc/intel/Kconfig @@ -12,6 +12,7 @@ config SND_MFLD_MACHINE config SND_SST_MFLD_PLATFORM tristate + select SND_SOC_COMPRESS config SND_SST_IPC tristate diff --git a/sound/soc/intel/atom/sst-mfld-platform-pcm.c b/sound/soc/intel/atom/sst-mfld-platform-pcm.c index 5e9c316..0487cfa 100644 --- a/sound/soc/intel/atom/sst-mfld-platform-pcm.c +++ b/sound/soc/intel/atom/sst-mfld-platform-pcm.c @@ -512,7 +512,7 @@ static struct snd_soc_dai_driver sst_platform_dai[] = { }, { .name = "compress-cpu-dai", - .compress_dai = 1, + .compress_new = snd_soc_new_compress, .ops = &sst_compr_dai_ops, .playback = { .stream_name = "Compress Playback", diff --git a/sound/soc/soc-compress.c b/sound/soc/soc-compress.c index 025c38f..12a9820 100644 --- a/sound/soc/soc-compress.c +++ b/sound/soc/soc-compress.c @@ -612,8 +612,15 @@ static struct snd_compr_ops soc_compr_dyn_ops = { .get_codec_caps = soc_compr_get_codec_caps }; -/* create a new compress */ -int soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num) +/** + * snd_soc_new_compress - create a new compress. + * + * @rtd: The runtime for which we will create compress + * @num: the device index number (zero based - shared with normal PCMs) + * + * Return: 0 for success, else error. + */ +int snd_soc_new_compress(struct snd_soc_pcm_runtime *rtd, int num) { struct snd_soc_codec *codec = rtd->codec; struct snd_soc_platform *platform = rtd->platform; @@ -703,3 +710,4 @@ compr_err: kfree(compr); return ret; } +EXPORT_SYMBOL_GPL(snd_soc_new_compress); diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 3b471f9..24b0960 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1370,9 +1370,9 @@ static int soc_probe_link_dais(struct snd_soc_card *card, int num, int order) soc_dpcm_debugfs_add(rtd); #endif - if (cpu_dai->driver->compress_dai) { + if (cpu_dai->driver->compress_new) { /*create compress_device"*/ - ret = soc_new_compress(rtd, num); + ret = cpu_dai->driver->compress_new(rtd, num); if (ret < 0) { dev_err(card->dev, "ASoC: can't create compress %s\n", dai_link->stream_name);
We don't always need soc-compress in soc, here add a config item SND_SOC_COMPRESS, when nobody select it, the soc-compress will not be compiled. Here also change Kconfig to 'select SND_SOC_COMPRESS' for drivers that needed soc-compress. Signed-off-by: Jie Yang <yang.jie@intel.com> --- include/sound/soc-dai.h | 2 +- include/sound/soc.h | 4 +++- sound/soc/Kconfig | 5 ++++- sound/soc/Makefile | 3 ++- sound/soc/intel/Kconfig | 1 + sound/soc/intel/atom/sst-mfld-platform-pcm.c | 2 +- sound/soc/soc-compress.c | 12 ++++++++++-- sound/soc/soc-core.c | 4 ++-- 8 files changed, 24 insertions(+), 9 deletions(-)