[alsa-devel] [PATCH] ASOC: utils: Fix the dummy_codec
This patch consists in settings the channels_min, rates and formats of the dummy codec DAI driver to enable the creation of the PCM streams using it.
Signed-off-by: Selma Bensaid selma.bensaid@intel.com --- sound/soc/soc-utils.c | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/sound/soc/soc-utils.c b/sound/soc/soc-utils.c index 6005370..465a094 100644 --- a/sound/soc/soc-utils.c +++ b/sound/soc/soc-utils.c @@ -89,9 +89,27 @@ static struct snd_soc_platform_driver dummy_platform = { .ops = &dummy_dma_ops, };
+#define NULL_FORMATS \ + (SNDRV_PCM_FMTBIT_S16 | SNDRV_PCM_FMTBIT_U16 |\ + SNDRV_PCM_FMTBIT_S24 | SNDRV_PCM_FMTBIT_U24 |\ + SNDRV_PCM_FMTBIT_S32 | SNDRV_PCM_FMTBIT_U32) + static struct snd_soc_codec_driver dummy_codec; static struct snd_soc_dai_driver dummy_dai = { .name = "snd-soc-dummy-dai", + .playback = { + .channels_min = 1, + .channels_max = 16, + .rates = SNDRV_PCM_RATE_CONTINUOUS, + .formats = NULL_FORMATS, + }, + .capture = { + .channels_min = 1, + .channels_max = 16, + .rates = SNDRV_PCM_RATE_CONTINUOUS, + .formats = NULL_FORMATS, + }, + .ops = NULL, };
static __devinit int snd_soc_dummy_probe(struct platform_device *pdev)
On Wed, 2012-06-06 at 10:02 +0200, Selma Bensaid wrote:
This patch consists in settings the channels_min, rates and formats of the dummy codec DAI driver to enable the creation of the PCM streams using it.
patch title is not clear what you are fixing in dummy_codec, pls consider appropriate title.
Signed-off-by: Selma Bensaid selma.bensaid@intel.com
sound/soc/soc-utils.c | 18 ++++++++++++++++++ 1 files changed, 18 insertions(+), 0 deletions(-)
diff --git a/sound/soc/soc-utils.c b/sound/soc/soc-utils.c index 6005370..465a094 100644 --- a/sound/soc/soc-utils.c +++ b/sound/soc/soc-utils.c @@ -89,9 +89,27 @@ static struct snd_soc_platform_driver dummy_platform = { .ops = &dummy_dma_ops, };
+#define NULL_FORMATS \
- (SNDRV_PCM_FMTBIT_S16 | SNDRV_PCM_FMTBIT_U16 |\
- SNDRV_PCM_FMTBIT_S24 | SNDRV_PCM_FMTBIT_U24 |\
- SNDRV_PCM_FMTBIT_S32 | SNDRV_PCM_FMTBIT_U32)
Would it make sense to have a CONTINUOUS format macro instead? As people start using these, I can see this becoming big and ugly.
But are the other formats defined in pcm.h widely used?
static struct snd_soc_codec_driver dummy_codec; static struct snd_soc_dai_driver dummy_dai = { .name = "snd-soc-dummy-dai",
- .playback = {
.channels_min = 1,.channels_max = 16,.rates = SNDRV_PCM_RATE_CONTINUOUS,.formats = NULL_FORMATS,- },
- .capture = {
.channels_min = 1,.channels_max = 16,.rates = SNDRV_PCM_RATE_CONTINUOUS,.formats = NULL_FORMATS,- },
- .ops = NULL,
};
static __devinit int snd_soc_dummy_probe(struct platform_device *pdev)
This patch consists in settings the channels_min, rates and formats of the dummy codec DAI driver to enable the creation of the PCM streams using it.
Signed-off-by: Selma Bensaid selma.bensaid@intel.com --- include/sound/pcm.h | 7 +++++++ sound/soc/soc-utils.c | 13 +++++++++++++ 2 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index ff536f1..649b0d5 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -203,6 +203,13 @@ struct snd_pcm_ops { #define SNDRV_PCM_FMTBIT_IEC958_SUBFRAME SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_BE #endif
+#define SNDRV_PCM_FMTBIT_CONTINUOUS \ + (SNDRV_PCM_FMTBIT_S16 | SNDRV_PCM_FMTBIT_U16 |\ + SNDRV_PCM_FMTBIT_S24 | SNDRV_PCM_FMTBIT_U24 |\ + SNDRV_PCM_FMTBIT_S32 | SNDRV_PCM_FMTBIT_U32 |\ + SNDRV_PCM_FMTBIT_FLOAT | SNDRV_PCM_FMTBIT_FLOAT64 |\ + SNDRV_PCM_FMTBIT_IEC958_SUBFRAME) + struct snd_pcm_file { struct snd_pcm_substream *substream; int no_compat_mmap; diff --git a/sound/soc/soc-utils.c b/sound/soc/soc-utils.c index 6005370..79125a0 100644 --- a/sound/soc/soc-utils.c +++ b/sound/soc/soc-utils.c @@ -92,6 +92,19 @@ static struct snd_soc_platform_driver dummy_platform = { static struct snd_soc_codec_driver dummy_codec; static struct snd_soc_dai_driver dummy_dai = { .name = "snd-soc-dummy-dai", + .playback = { + .channels_min = 1, + .channels_max = 16, + .rates = SNDRV_PCM_RATE_CONTINUOUS, + .formats = SNDRV_PCM_FMTBIT_CONTINUOUS, + }, + .capture = { + .channels_min = 1, + .channels_max = 16, + .rates = SNDRV_PCM_RATE_CONTINUOUS, + .formats = SNDRV_PCM_FMTBIT_CONTINUOUS, + }, + .ops = NULL, };
static __devinit int snd_soc_dummy_probe(struct platform_device *pdev)
On Thu, Jun 07, 2012 at 12:08:41PM +0200, Selma Bensaid wrote:
This patch consists in settings the channels_min, rates and formats of the dummy codec DAI driver to enable the creation of the PCM streams using it.
I still don't understand what this is actually supposed to fix - why are we trying to set up a stream to something that isn't there? Though it looks like this was added for dynamic PCM which has no mainline users so perhaps there's a sensible use for this...
The other question is how we decide that these are sensible values to set for the dummy CODEC.
On 06/07/2012 11:30 PM, Mark Brown wrote:
I still don't understand what this is actually supposed to fix - why are we trying to set up a stream to something that isn't there? Though it looks like this was added for dynamic PCM which has no mainline users so perhaps there's a sensible use for this...
I'm trying to use the dummy codec for BT and Modem device where the coded is handled @ AP side. Without updating the dummy codec, I fails when opening my devices due to the check performed in soc_pcm_open: No matching rates, No matching formats and No matching channels Errors.
The other question is how we decide that these are sensible values to set for the dummy CODEC.
For formats the dummy codec support all formats so it does not impact the runtime->hw.formats = codec_dai_drv->playback.formats & cpu_dai_drv->playback.formats;
For rates, I 'll submit a new version of the patch since the SNDRV_PCM_RATE_CONTINUOUS is not the right value to use.
For channels_min, the value 1 does not impact the runtime->hw.channels_min = max(codec_dai_drv->capture.channels_min, cpu_dai_drv->capture.channels_min);
For channels_max, I 'll submit a new version of the patch to set it to MAX so does not impact the runtime->hw.channels_max = min(codec_dai_drv->playback.channels_max cpu_dai_drv->playback.channels_max); --------------------------------------------------------------------- Intel Corporation SAS (French simplified joint stock company) Registered headquarters: "Les Montalets"- 2, rue de Paris, 92196 Meudon Cedex, France Registration Number: 302 456 199 R.C.S. NANTERRE Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
This patch consists in settings the channels_min, rates and formats of the dummy codec DAI driver to enable the creation of the PCM streams using it.
Signed-off-by: Selma Bensaid selma.bensaid@intel.com --- include/sound/pcm.h | 9 +++++++++ sound/soc/soc-utils.c | 13 +++++++++++++ 2 files changed, 22 insertions(+), 0 deletions(-)
diff --git a/include/sound/pcm.h b/include/sound/pcm.h index ff536f1..c0b5fd4 100644 --- a/include/sound/pcm.h +++ b/include/sound/pcm.h @@ -127,6 +127,8 @@ struct snd_pcm_ops {
#define SNDRV_PCM_RATE_CONTINUOUS (1<<30) /* continuous range */ #define SNDRV_PCM_RATE_KNOT (1<<31) /* supports more non-continuos rates */ +#define SNDRV_PCM_RATE_ALL (0xFFFFFFFF) /* support all RATES */ +
#define SNDRV_PCM_RATE_8000_44100 (SNDRV_PCM_RATE_8000|SNDRV_PCM_RATE_11025|\ SNDRV_PCM_RATE_16000|SNDRV_PCM_RATE_22050|\ @@ -203,6 +205,13 @@ struct snd_pcm_ops { #define SNDRV_PCM_FMTBIT_IEC958_SUBFRAME SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_BE #endif
+#define SNDRV_PCM_FMTBIT_CONTINUOUS \ + (SNDRV_PCM_FMTBIT_S16 | SNDRV_PCM_FMTBIT_U16 |\ + SNDRV_PCM_FMTBIT_S24 | SNDRV_PCM_FMTBIT_U24 |\ + SNDRV_PCM_FMTBIT_S32 | SNDRV_PCM_FMTBIT_U32 |\ + SNDRV_PCM_FMTBIT_FLOAT | SNDRV_PCM_FMTBIT_FLOAT64 |\ + SNDRV_PCM_FMTBIT_IEC958_SUBFRAME) + struct snd_pcm_file { struct snd_pcm_substream *substream; int no_compat_mmap; diff --git a/sound/soc/soc-utils.c b/sound/soc/soc-utils.c index 6005370..2aeaf0b 100644 --- a/sound/soc/soc-utils.c +++ b/sound/soc/soc-utils.c @@ -92,6 +92,19 @@ static struct snd_soc_platform_driver dummy_platform = { static struct snd_soc_codec_driver dummy_codec; static struct snd_soc_dai_driver dummy_dai = { .name = "snd-soc-dummy-dai", + .playback = { + .channels_min = 1, + .channels_max = 0xFFFF, + .rates = SNDRV_PCM_RATE_ALL, + .formats = SNDRV_PCM_FMTBIT_CONTINUOUS, + }, + .capture = { + .channels_min = 1, + .channels_max = 0xFFFF, + .rates = SNDRV_PCM_RATE_ALL, + .formats = SNDRV_PCM_FMTBIT_CONTINUOUS, + }, + .ops = NULL, };
static __devinit int snd_soc_dummy_probe(struct platform_device *pdev)
On Fri, Jun 08, 2012 at 10:02:38AM +0200, Selma Bensaid wrote:
Please check your mailer configuration, it's doing something really odd with the word wrapping which makes things hard to read.
I still don't understand what this is actually supposed to fix - why are we trying to set up a stream to something that isn't there? Though it looks like this was added for dynamic PCM which has no mainline users so perhaps there's a sensible use for this...
I'm trying to use the dummy codec for BT and Modem device where the coded is handled @ AP side.
You shouldn't be doing this, you should have actual devices for these with appropriate capabilities for the device. Ideally we'd even have real drivers for these things! For example, you're currently setting them up as supporting continuous audio rates but both BT and baseband devices won't do anything like that. Even your new version which has all the rates enabled has about the same issue.
On 06/11/2012 05:13 AM, Mark Brown wrote:
On Fri, Jun 08, 2012 at 10:02:38AM +0200, Selma Bensaid wrote:
Please check your mailer configuration, it's doing something really odd with the word wrapping which makes things hard to read.
I still don't understand what this is actually supposed to fix - why are we trying to set up a stream to something that isn't there? Though it looks like this was added for dynamic PCM which has no mainline users so perhaps there's a sensible use for this...
I'm trying to use the dummy codec for BT and Modem device where the coded is handled @ AP side.
You shouldn't be doing this, you should have actual devices for these with appropriate capabilities for the device. Ideally we'd even have real drivers for these things! For example, you're currently setting them up as supporting continuous audio rates but both BT and baseband devices won't do anything like that. Even your new version which has all the rates enabled has about the same issue.
I can implement my own dummy codec where I do not perform any actions since in our platform all modem controls and BT controls are done @AP side. But I really do not understand why ASoC provides the soc-utils dummy codec? The current version of the dummy codec is useless since it blocks the stream creation: please refer to soc_new_pcm.
--------------------------------------------------------------------- Intel Corporation SAS (French simplified joint stock company) Registered headquarters: "Les Montalets"- 2, rue de Paris, 92196 Meudon Cedex, France Registration Number: 302 456 199 R.C.S. NANTERRE Capital: 4,572,000 Euros
This e-mail and any attachments may contain confidential material for the sole use of the intended recipient(s). Any review or distribution by others is strictly prohibited. If you are not the intended recipient, please contact the sender and delete all copies.
On Mon, Jun 11, 2012 at 09:39:59AM +0200, Selma Bensaid wrote:
are done @AP side. But I really do not understand why ASoC provides the soc-utils dummy codec? The current version of the dummy codec is useless since it blocks the stream creation: please refer to soc_new_pcm.
The commit log refers to usage in conjunction with soc-pcm, I presume it's going to be substituted in as a placeholder in cases where there's loopback or otherwise nothing connected. Like I say it shouldn't be used for real devices since we should at least specify constraints.
participants (3)
-
Mark Brown -
Selma Bensaid -
Vinod Koul