* [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs
@ 2011-12-23 0:26 Sangbeom Kim
2011-12-23 11:36 ` Lars-Peter Clausen
0 siblings, 1 reply; 10+ messages in thread
From: Sangbeom Kim @ 2011-12-23 0:26 UTC (permalink / raw)
To: alsa-devel, lrg, broonie; +Cc: Sangbeom Kim, Sangsu Park
From: Sangsu Park <sangsu4u.park@samsung.com>
The original code does not cover the case that two DAIs(CPU) have different
ASoC core PCM operations(like mmap, pointer...). Currently we have only one
global soc_pcm_ops for ASoC core PCM operation. When two DAIs have different
pointer functions, second DAI's pointer function is set for both first DAI
and second DAI in case of original code.
This patch allocates ASoC core PCM operations dynamically for each DAIs. So
each DAIs can have different ASoC core PCM operations. This is needed to
support multiple DAIs.
Signed-off-by: Sangsu Park <sangsu4u.park@samsung.com>
Signed-off-by: Sangbeom Kim <sbkim73@samsung.com>
---
sound/core/pcm.c | 1 +
sound/soc/soc-pcm.c | 44 ++++++++++++++++++++++++--------------------
2 files changed, 25 insertions(+), 20 deletions(-)
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 8928ca8..15cf27a 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -769,6 +769,7 @@ static void snd_pcm_free_stream(struct snd_pcm_str * pstr)
substream_next = substream->next;
snd_pcm_timer_done(substream);
snd_pcm_substream_proc_done(substream);
+ kfree(substream->ops);
kfree(substream);
substream = substream_next;
}
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 8aa7cec..3a52534 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -598,17 +598,6 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
return offset;
}
-/* ASoC PCM operations */
-static struct snd_pcm_ops soc_pcm_ops = {
- .open = soc_pcm_open,
- .close = soc_pcm_close,
- .hw_params = soc_pcm_hw_params,
- .hw_free = soc_pcm_hw_free,
- .prepare = soc_pcm_prepare,
- .trigger = soc_pcm_trigger,
- .pointer = soc_pcm_pointer,
-};
-
/* create a new pcm */
int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
{
@@ -616,10 +605,25 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
struct snd_soc_platform *platform = rtd->platform;
struct snd_soc_dai *codec_dai = rtd->codec_dai;
struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_pcm_ops *soc_pcm_ops;
struct snd_pcm *pcm;
char new_name[64];
int ret = 0, playback = 0, capture = 0;
+ soc_pcm_ops = kzalloc(sizeof(*soc_pcm_ops), GFP_KERNEL);
+ if (soc_pcm_ops == NULL) {
+ snd_printk(KERN_ERR "Cannot allocate PCM OPS\n");
+ return -ENOMEM;
+ }
+
+ soc_pcm_ops->open = soc_pcm_open;
+ soc_pcm_ops->close = soc_pcm_close;
+ soc_pcm_ops->hw_params = soc_pcm_hw_params;
+ soc_pcm_ops->hw_free = soc_pcm_hw_free;
+ soc_pcm_ops->prepare = soc_pcm_prepare;
+ soc_pcm_ops->trigger = soc_pcm_trigger;
+ soc_pcm_ops->pointer = soc_pcm_pointer;
+
/* check client and interface hw capabilities */
snprintf(new_name, sizeof(new_name), "%s %s-%d",
rtd->dai_link->stream_name, codec_dai->name, num);
@@ -643,20 +647,20 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
rtd->pcm = pcm;
pcm->private_data = rtd;
if (platform->driver->ops) {
- soc_pcm_ops.mmap = platform->driver->ops->mmap;
- soc_pcm_ops.pointer = platform->driver->ops->pointer;
- soc_pcm_ops.ioctl = platform->driver->ops->ioctl;
- soc_pcm_ops.copy = platform->driver->ops->copy;
- soc_pcm_ops.silence = platform->driver->ops->silence;
- soc_pcm_ops.ack = platform->driver->ops->ack;
- soc_pcm_ops.page = platform->driver->ops->page;
+ soc_pcm_ops->mmap = platform->driver->ops->mmap;
+ soc_pcm_ops->pointer = platform->driver->ops->pointer;
+ soc_pcm_ops->ioctl = platform->driver->ops->ioctl;
+ soc_pcm_ops->copy = platform->driver->ops->copy;
+ soc_pcm_ops->silence = platform->driver->ops->silence;
+ soc_pcm_ops->ack = platform->driver->ops->ack;
+ soc_pcm_ops->page = platform->driver->ops->page;
}
if (playback)
- snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &soc_pcm_ops);
+ snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, soc_pcm_ops);
if (capture)
- snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &soc_pcm_ops);
+ snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, soc_pcm_ops);
if (platform->driver->pcm_new) {
ret = platform->driver->pcm_new(rtd);
--
1.7.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs
@ 2011-12-23 1:37 Sangbeom Kim
2011-12-23 9:42 ` Liam Girdwood
2011-12-23 10:24 ` Mark Brown
0 siblings, 2 replies; 10+ messages in thread
From: Sangbeom Kim @ 2011-12-23 1:37 UTC (permalink / raw)
To: lrg, 'Mark Brown', alsa-devel; +Cc: sbkim73, sangsu4u.park
The original code does not cover the case that two DAIs(CPU) have different
ASoC core PCM operations(like mmap, pointer...). Currently we have only one
global soc_pcm_ops for ASoC core PCM operation. When two DAIs have different
pointer functions, second DAI's pointer function is set for both first DAI
and second DAI in case of original code.
This patch allocates ASoC core PCM operations dynamically for each DAIs. So
each DAIs can have different ASoC core PCM operations. This is needed to
support multiple DAIs.
Signed-off-by: Sangsu Park <sangsu4u.park@samsung.com>
Signed-off-by: Sangbeom Kim <sbkim73@samsung.com>
---
sound/core/pcm.c | 1 +
sound/soc/soc-pcm.c | 44 ++++++++++++++++++++++++--------------------
2 files changed, 25 insertions(+), 20 deletions(-)
diff --git a/sound/core/pcm.c b/sound/core/pcm.c
index 8928ca8..15cf27a 100644
--- a/sound/core/pcm.c
+++ b/sound/core/pcm.c
@@ -769,6 +769,7 @@ static void snd_pcm_free_stream(struct snd_pcm_str * pstr)
substream_next = substream->next;
snd_pcm_timer_done(substream);
snd_pcm_substream_proc_done(substream);
+ kfree(substream->ops);
kfree(substream);
substream = substream_next;
}
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 8aa7cec..3a52534 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -598,17 +598,6 @@ static snd_pcm_uframes_t soc_pcm_pointer(struct snd_pcm_substream *substream)
return offset;
}
-/* ASoC PCM operations */
-static struct snd_pcm_ops soc_pcm_ops = {
- .open = soc_pcm_open,
- .close = soc_pcm_close,
- .hw_params = soc_pcm_hw_params,
- .hw_free = soc_pcm_hw_free,
- .prepare = soc_pcm_prepare,
- .trigger = soc_pcm_trigger,
- .pointer = soc_pcm_pointer,
-};
-
/* create a new pcm */
int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
{
@@ -616,10 +605,25 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
struct snd_soc_platform *platform = rtd->platform;
struct snd_soc_dai *codec_dai = rtd->codec_dai;
struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_pcm_ops *soc_pcm_ops;
struct snd_pcm *pcm;
char new_name[64];
int ret = 0, playback = 0, capture = 0;
+ soc_pcm_ops = kzalloc(sizeof(*soc_pcm_ops), GFP_KERNEL);
+ if (soc_pcm_ops == NULL) {
+ snd_printk(KERN_ERR "Cannot allocate PCM OPS\n");
+ return -ENOMEM;
+ }
+
+ soc_pcm_ops->open = soc_pcm_open;
+ soc_pcm_ops->close = soc_pcm_close;
+ soc_pcm_ops->hw_params = soc_pcm_hw_params;
+ soc_pcm_ops->hw_free = soc_pcm_hw_free;
+ soc_pcm_ops->prepare = soc_pcm_prepare;
+ soc_pcm_ops->trigger = soc_pcm_trigger;
+ soc_pcm_ops->pointer = soc_pcm_pointer;
+
/* check client and interface hw capabilities */
snprintf(new_name, sizeof(new_name), "%s %s-%d",
rtd->dai_link->stream_name, codec_dai->name, num);
@@ -643,20 +647,20 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
rtd->pcm = pcm;
pcm->private_data = rtd;
if (platform->driver->ops) {
- soc_pcm_ops.mmap = platform->driver->ops->mmap;
- soc_pcm_ops.pointer = platform->driver->ops->pointer;
- soc_pcm_ops.ioctl = platform->driver->ops->ioctl;
- soc_pcm_ops.copy = platform->driver->ops->copy;
- soc_pcm_ops.silence = platform->driver->ops->silence;
- soc_pcm_ops.ack = platform->driver->ops->ack;
- soc_pcm_ops.page = platform->driver->ops->page;
+ soc_pcm_ops->mmap = platform->driver->ops->mmap;
+ soc_pcm_ops->pointer = platform->driver->ops->pointer;
+ soc_pcm_ops->ioctl = platform->driver->ops->ioctl;
+ soc_pcm_ops->copy = platform->driver->ops->copy;
+ soc_pcm_ops->silence = platform->driver->ops->silence;
+ soc_pcm_ops->ack = platform->driver->ops->ack;
+ soc_pcm_ops->page = platform->driver->ops->page;
}
if (playback)
- snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &soc_pcm_ops);
+ snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, soc_pcm_ops);
if (capture)
- snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &soc_pcm_ops);
+ snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, soc_pcm_ops);
if (platform->driver->pcm_new) {
ret = platform->driver->pcm_new(rtd);
--
1.7.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs
2011-12-23 1:37 [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs Sangbeom Kim
@ 2011-12-23 9:42 ` Liam Girdwood
2011-12-23 10:21 ` Mark Brown
2011-12-23 10:24 ` Mark Brown
1 sibling, 1 reply; 10+ messages in thread
From: Liam Girdwood @ 2011-12-23 9:42 UTC (permalink / raw)
To: Sangbeom Kim; +Cc: alsa-devel, 'Mark Brown', sangsu4u.park
On Fri, 2011-12-23 at 10:37 +0900, Sangbeom Kim wrote:
> The original code does not cover the case that two DAIs(CPU) have different
> ASoC core PCM operations(like mmap, pointer...). Currently we have only one
> global soc_pcm_ops for ASoC core PCM operation. When two DAIs have different
> pointer functions, second DAI's pointer function is set for both first DAI
> and second DAI in case of original code.
>
> This patch allocates ASoC core PCM operations dynamically for each DAIs. So
> each DAIs can have different ASoC core PCM operations. This is needed to
> support multiple DAIs.
>
I assume that you have two different DMA controllers (with a separate
DMA platform drivers) here ?
If so, why can you not specify each DMA platform driver in your machine
driver DAI link ?
Thanks
Liam
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs
2011-12-23 9:42 ` Liam Girdwood
@ 2011-12-23 10:21 ` Mark Brown
0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2011-12-23 10:21 UTC (permalink / raw)
To: Liam Girdwood; +Cc: Sangbeom Kim, alsa-devel, sangsu4u.park
On Fri, Dec 23, 2011 at 09:42:01AM +0000, Liam Girdwood wrote:
> I assume that you have two different DMA controllers (with a separate
> DMA platform drivers) here ?
> If so, why can you not specify each DMA platform driver in your machine
> driver DAI link ?
Alan Tull reported the same problem before with a patch that needed
respinning. The issue is that we have a static soc_pcm_ops and we don't
properly indirect everything, we copy some of the ops into snd_pcm_ops
directly. This means that if those ops end up being used then they end
up being the same for both drivers.
I was actually intending to fix this over Christmas.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs
2011-12-23 1:37 [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs Sangbeom Kim
2011-12-23 9:42 ` Liam Girdwood
@ 2011-12-23 10:24 ` Mark Brown
1 sibling, 0 replies; 10+ messages in thread
From: Mark Brown @ 2011-12-23 10:24 UTC (permalink / raw)
To: Sangbeom Kim; +Cc: alsa-devel, lrg, sangsu4u.park
On Fri, Dec 23, 2011 at 10:37:51AM +0900, Sangbeom Kim wrote:
> + soc_pcm_ops = kzalloc(sizeof(*soc_pcm_ops), GFP_KERNEL);
> + if (soc_pcm_ops == NULL) {
> + snd_printk(KERN_ERR "Cannot allocate PCM OPS\n");
> + return -ENOMEM;
> + }
This patch is good and fixes a real problem but can you please change it
slightly so that instead of dynamically allocating the soc_pcm_ops we
just embed it directly in the runtime structure. Since every runtime
needs an ops structure we may as well just have it embedded directly and
not write the allocation, freeing and error handling code.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs
2011-12-23 0:26 Sangbeom Kim
@ 2011-12-23 11:36 ` Lars-Peter Clausen
0 siblings, 0 replies; 10+ messages in thread
From: Lars-Peter Clausen @ 2011-12-23 11:36 UTC (permalink / raw)
To: Sangbeom Kim; +Cc: alsa-devel, broonie, Sangsu Park, lrg
On 12/23/2011 01:26 AM, Sangbeom Kim wrote:
> From: Sangsu Park <sangsu4u.park@samsung.com>
>
> The original code does not cover the case that two DAIs(CPU) have different
> ASoC core PCM operations(like mmap, pointer...). Currently we have only one
> global soc_pcm_ops for ASoC core PCM operation. When two DAIs have different
> pointer functions, second DAI's pointer function is set for both first DAI
> and second DAI in case of original code.
>
> This patch allocates ASoC core PCM operations dynamically for each DAIs. So
> each DAIs can have different ASoC core PCM operations. This is needed to
> support multiple DAIs.
>
> Signed-off-by: Sangsu Park <sangsu4u.park@samsung.com>
> Signed-off-by: Sangbeom Kim <sbkim73@samsung.com>
> ---
> sound/core/pcm.c | 1 +
> sound/soc/soc-pcm.c | 44 ++++++++++++++++++++++++--------------------
> 2 files changed, 25 insertions(+), 20 deletions(-)
>
> diff --git a/sound/core/pcm.c b/sound/core/pcm.c
> index 8928ca8..15cf27a 100644
> --- a/sound/core/pcm.c
> +++ b/sound/core/pcm.c
> @@ -769,6 +769,7 @@ static void snd_pcm_free_stream(struct snd_pcm_str * pstr)
> substream_next = substream->next;
> snd_pcm_timer_done(substream);
> snd_pcm_substream_proc_done(substream);
> + kfree(substream->ops);
This looks wrong. It will probably cause regression for all non ASoC sound
card drivers.
The issue this patch addresses came up before and I think the conclusion was
that it is best to embed the snd_pcm_ops struct into the snd_soc_pcm_runtime
struct.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs
@ 2011-12-27 7:49 Sangsu Park
2011-12-27 10:34 ` Mark Brown
0 siblings, 1 reply; 10+ messages in thread
From: Sangsu Park @ 2011-12-27 7:49 UTC (permalink / raw)
To: broonie; +Cc: sbkim73, alsa-devel, lrg, sangsu4u.park
> On Fri, Dec 23, 2011 at 10:37:51AM +0900, Sangbeom Kim wrote:
>
> > + soc_pcm_ops = kzalloc(sizeof(*soc_pcm_ops), GFP_KERNEL);
> > + if (soc_pcm_ops == NULL) {
> > + snd_printk(KERN_ERR "Cannot allocate PCM OPS\n");
> > + return -ENOMEM;
> > + }
>
> This patch is good and fixes a real problem but can you please change it
> slightly so that instead of dynamically allocating the soc_pcm_ops we
> just embed it directly in the runtime structure. Since every runtime
> needs an ops structure we may as well just have it embedded directly and
> not write the allocation, freeing and error handling code.
Hi, I'm Sangsu Park. (sangsu4u.park@samsung.com)
I'm using gmail because of company's firewall problems...
I've implement your guide like follows.
//--------------------------------------------------------------------------------------------------------------------------------
struct snd_soc_dai *codec_dai = rtd->codec_dai;
struct snd_soc_dai *cpu_dai = rtd->cpu_dai;
+ struct snd_pcm_ops *soc_pcm_ops = platform->driver->ops;
struct snd_pcm *pcm;
char new_name[64];
int ret = 0, playback = 0, capture = 0;
if (playback)
- snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &soc_pcm_ops);
+ snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, soc_pcm_ops);
if (capture)
- snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &soc_pcm_ops);
+ snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, soc_pcm_ops);
--------------------------------------------------------------------------------------------------------------------------------//
But, it seems to make mutex dead lock.
When starting playback, it tells some messages.
//--------------------------------------------------------------------------------------------------------------------------------
# aplay -twav Test.wav
INFO: task aplay:1071 blocked for more than 120 seconds.
"echo 0 > /proc/sys/kernel/hung_task_timeout_secs" disables this message.
aplay D c01d1ba4 0 1071 1051 0x00000000
[<c01d1ba4>] (__schedule+0x244/0x6cc) from [<c01d2db0>]
(__mutex_lock_slowpath+0x180/0x360)
[<c01d2db0>] (__mutex_lock_slowpath+0x180/0x360) from [<c01d2f9c>]
(mutex_lock+0xc/0x24)
[<c01d2f9c>] (mutex_lock+0xc/0x24) from [<c01c10c4>] (soc_pcm_open+0x38/0x48c)
[<c01c10c4>] (soc_pcm_open+0x38/0x48c) from [<c01c1114>]
(soc_pcm_open+0x88/0x48c)
[<c01c1114>] (soc_pcm_open+0x88/0x48c) from [<c01b0fcc>]
(snd_pcm_open_substream+0x60/0xa0)
[<c01b0fcc>] (snd_pcm_open_substream+0x60/0xa0) from [<c01b1134>]
(snd_pcm_open+0x128/0x270)
[<c01b1134>] (snd_pcm_open+0x128/0x270) from [<c01a4034>] (snd_open+0x130/0x2ec)
[<c01a4034>] (snd_open+0x130/0x2ec) from [<c00b00e4>] (chrdev_open+0x10c/0x1f0)
[<c00b00e4>] (chrdev_open+0x10c/0x1f0) from [<c00aad24>]
(__dentry_open+0x220/0x328)
[<c00aad24>] (__dentry_open+0x220/0x328) from [<c00aaef4>]
(nameidata_to_filp+0x60/0x68)
[<c00aaef4>] (nameidata_to_filp+0x60/0x68) from [<c00bb5d4>]
(do_last+0x214/0xcc0)
[<c00bb5d4>] (do_last+0x214/0xcc0) from [<c00bc13c>] (path_openat+0xbc/0x3a8)
[<c00bc13c>] (path_openat+0xbc/0x3a8) from [<c00bc510>] (do_filp_open+0x30/0x84)
[<c00bc510>] (do_filp_open+0x30/0x84) from [<c00aa954>] (do_sys_open+0xe4/0x184)
[<c00aa954>] (do_sys_open+0xe4/0x184) from [<c000f3c0>]
(ret_fast_syscall+0x0/0x30)
--------------------------------------------------------------------------------------------------------------------------------//
"sound/core/" uses soc_pcm_ops using substream->ops,
and "sound/soc/" uses soc_pcm_ops using platform->driver->ops.
So, I think that they must have their own soc_pcm_ops object.
How do you think about this?
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs
@ 2011-12-27 8:06 Sangsu Park
2011-12-27 10:40 ` Mark Brown
0 siblings, 1 reply; 10+ messages in thread
From: Sangsu Park @ 2011-12-27 8:06 UTC (permalink / raw)
To: lars; +Cc: sbkim73, alsa-devel, broonie, lrg, sangsu4u.park
> This looks wrong. It will probably cause regression for all non ASoC sound
> card drivers.
>
> The issue this patch addresses came up before and I think the conclusion was
> that it is best to embed the snd_pcm_ops struct into the snd_soc_pcm_runtime
> struct.
Hi, I'm Sangsu Park. (sangsu4u.park@samsung.com)
I'm using gmail because of company's firewall problems...
I agree you, So I've changed some code like follows.
If you like that, I'll make patch version 2nd using that.
//------------------------------------------------------------------------------
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
index 42ad2db..d1468aa 100644
--- a/sound/soc/soc-core.c
+++ b/sound/soc/soc-core.c
@@ -918,6 +918,9 @@ static void soc_remove_dai_link(struct
snd_soc_card *card, int num, int order)
list_del(&cpu_dai->card_list);
module_put(cpu_dai->dev->driver->owner);
}
+
+ /* free the ops memory */
+ if (rtd->ops) kfree(rtd->ops);
}
diff --git a/sound/soc/soc-pcm.c b/sound/soc/soc-pcm.c
index 8aa7cec..4eb793b 100644
--- a/sound/soc/soc-pcm.c
+++ b/sound/soc/soc-pcm.c
@@ -641,22 +645,23 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
INIT_DELAYED_WORK(&rtd->delayed_work, close_delayed_work);
rtd->pcm = pcm;
+ rtd->ops = soc_pcm_ops;
pcm->private_data = rtd;
if (platform->driver->ops) {
------------------------------------------------------------------------------//
Thanks.
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs
2011-12-27 7:49 Sangsu Park
@ 2011-12-27 10:34 ` Mark Brown
0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2011-12-27 10:34 UTC (permalink / raw)
To: Sangsu Park; +Cc: sbkim73, alsa-devel, lrg, sangsu4u.park
On Tue, Dec 27, 2011 at 04:49:46PM +0900, Sangsu Park wrote:
> > On Fri, Dec 23, 2011 at 10:37:51AM +0900, Sangbeom Kim wrote:
> + struct snd_pcm_ops *soc_pcm_ops = platform->driver->ops;
> struct snd_pcm *pcm;
> char new_name[64];
> int ret = 0, playback = 0, capture = 0;
>
> if (playback)
> - snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, &soc_pcm_ops);
> + snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_PLAYBACK, soc_pcm_ops);
>
> if (capture)
> - snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, &soc_pcm_ops);
> + snd_pcm_set_ops(pcm, SNDRV_PCM_STREAM_CAPTURE, soc_pcm_ops);
> But, it seems to make mutex dead lock.
> When starting playback, it tells some messages.
We still need to use the indirections in the SoC ops in the same way as
we do at present so that the DAI drivers and DAPM get called as well.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs
2011-12-27 8:06 Sangsu Park
@ 2011-12-27 10:40 ` Mark Brown
0 siblings, 0 replies; 10+ messages in thread
From: Mark Brown @ 2011-12-27 10:40 UTC (permalink / raw)
To: Sangsu Park; +Cc: sbkim73, alsa-devel, lars, lrg, sangsu4u.park
On Tue, Dec 27, 2011 at 05:06:56PM +0900, Sangsu Park wrote:
> + /* free the ops memory */
> + if (rtd->ops) kfree(rtd->ops);
We shouldn't be doing a kfree() on the ops at all.
> +++ b/sound/soc/soc-pcm.c
> @@ -641,22 +645,23 @@ int soc_new_pcm(struct snd_soc_pcm_runtime *rtd, int num)
> INIT_DELAYED_WORK(&rtd->delayed_work, close_delayed_work);
> rtd->pcm = pcm;
> + rtd->ops = soc_pcm_ops;
That *might* work if ops were changed to be a member variable, though
it's racy against two things initializing simultaneously.
What I'd expect is something like:
INIT_DELAYED_WORK(&rtd->delayed_work, close_delayed_work);
rtd->pcm = pcm;
rtd->pcm_ops = soc_pcm_ops;
pcm->private_data = rtd;
if (platform->driver->ops) {
rtd->pcm_ops.mmap = platform->driver->ops->mmap;
rtd->pcm_ops.pointer = platform->driver->ops->pointer;
and so on so we start off by taking a copy of the default ops then copy
the new ops in in the same fashion.
Ideally we should also have an indirection for ioctl() though using it
at all is dodgy.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2011-12-27 10:40 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-12-23 1:37 [PATCH] ASoC: Allocate PCM operations dynamically to support multiple DAIs Sangbeom Kim
2011-12-23 9:42 ` Liam Girdwood
2011-12-23 10:21 ` Mark Brown
2011-12-23 10:24 ` Mark Brown
-- strict thread matches above, loose matches on Subject: below --
2011-12-27 8:06 Sangsu Park
2011-12-27 10:40 ` Mark Brown
2011-12-27 7:49 Sangsu Park
2011-12-27 10:34 ` Mark Brown
2011-12-23 0:26 Sangbeom Kim
2011-12-23 11:36 ` Lars-Peter Clausen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).