[alsa-devel] [PATCH 0/3] ASoC: core: dapm more updates
This series contains three updates we have for dapm
First marks the SND_SOC_BYTES_EXT as deprecated as discussed
Second one adds support for dapm sink widget. This is useful in DSP based systems where a module can act as a sink of data
Last changes order of power state to before rechecking endpoint. This ensure DAPM is restored properly before userspace can access the card.
Jeeja KP (1): ASoC: core: Change power state before rechecking endpoint
Vinod Koul (2): ASoC: core: mark SND_SOC_BYTES_EXT as deprecated ASoC: core: add a dapm sink widget
include/sound/soc-dapm.h | 4 ++++ include/sound/soc.h | 3 +++ sound/soc/soc-core.c | 6 +++--- sound/soc/soc-dapm.c | 5 +++++ 4 files changed, 15 insertions(+), 3 deletions(-)
Since we have SND_SOC_BYTES_TLV control to lets devices have larger size data sent, we do not need SND_SOC_BYTES_EXT with 512 byte limitation so mark it deprecated
Signed-off-by: Vinod Koul vinod.koul@intel.com --- include/sound/soc.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/include/sound/soc.h b/include/sound/soc.h index d2a80508a048..03431e76c932 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -293,6 +293,9 @@ {.base = xbase, .num_regs = xregs, \ .mask = xmask }) }
+/* + * SND_SOC_BYTES_EXT is deprecated, please USE SND_SOC_BYTES_TLV instead + */ #define SND_SOC_BYTES_EXT(xname, xcount, xhandler_get, xhandler_put) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \ .info = snd_soc_bytes_info_ext, \
The patch
ASoC: core: mark SND_SOC_BYTES_EXT as deprecated
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 50a4f98d3452ea3818eb364f9c34a9de139df654 Mon Sep 17 00:00:00 2001
From: Vinod Koul vinod.koul@intel.com Date: Mon, 23 Nov 2015 21:22:29 +0530 Subject: [PATCH] ASoC: core: mark SND_SOC_BYTES_EXT as deprecated
Since we have SND_SOC_BYTES_TLV control to lets devices have larger size data sent, we do not need SND_SOC_BYTES_EXT with 512 byte limitation so mark it deprecated
Signed-off-by: Vinod Koul vinod.koul@intel.com Signed-off-by: Mark Brown broonie@kernel.org --- include/sound/soc.h | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/include/sound/soc.h b/include/sound/soc.h index a8b4b9c8b1d2..6dbd24a36ef8 100644 --- a/include/sound/soc.h +++ b/include/sound/soc.h @@ -293,6 +293,9 @@ {.base = xbase, .num_regs = xregs, \ .mask = xmask }) }
+/* + * SND_SOC_BYTES_EXT is deprecated, please USE SND_SOC_BYTES_TLV instead + */ #define SND_SOC_BYTES_EXT(xname, xcount, xhandler_get, xhandler_put) \ { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = xname, \ .info = snd_soc_bytes_info_ext, \
DAPM models various widgets but lacks a sink widget. DSPs can have modules which take audio data, process it and are capable of generating events thus acting as a sink of data.
To make the dapm graph complete for such paths we need a dapm sink widget for these modules, so add a SND_SOC_DAPM_SINK to declare such a widget. This widget will be treated as SND_SOC_DAPM_EP_SINK endpoint in the dapm graph
Signed-off-by: Vinod Koul vinod.koul@intel.com --- include/sound/soc-dapm.h | 4 ++++ sound/soc/soc-dapm.c | 5 +++++ 2 files changed, 9 insertions(+)
diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h index 7855cfe46b69..fe19dd3abb00 100644 --- a/include/sound/soc-dapm.h +++ b/include/sound/soc-dapm.h @@ -49,6 +49,9 @@ struct device; #define SND_SOC_DAPM_SIGGEN(wname) \ { .id = snd_soc_dapm_siggen, .name = wname, .kcontrol_news = NULL, \ .num_kcontrols = 0, .reg = SND_SOC_NOPM } +#define SND_SOC_DAPM_SINK(wname) \ +{ .id = snd_soc_dapm_sink, .name = wname, .kcontrol_news = NULL, \ + .num_kcontrols = 0, .reg = SND_SOC_NOPM } #define SND_SOC_DAPM_INPUT(wname) \ { .id = snd_soc_dapm_input, .name = wname, .kcontrol_news = NULL, \ .num_kcontrols = 0, .reg = SND_SOC_NOPM } @@ -484,6 +487,7 @@ enum snd_soc_dapm_type { snd_soc_dapm_aif_in, /* audio interface input */ snd_soc_dapm_aif_out, /* audio interface output */ snd_soc_dapm_siggen, /* signal generator */ + snd_soc_dapm_sink, snd_soc_dapm_dai_in, /* link to DAI structure */ snd_soc_dapm_dai_out, snd_soc_dapm_dai_link, /* link between two DAI structures */ diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 3eba72c6f9dd..506677b12787 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3351,6 +3351,11 @@ snd_soc_dapm_new_control_unlocked(struct snd_soc_dapm_context *dapm, w->is_ep = SND_SOC_DAPM_EP_SOURCE; w->power_check = dapm_always_on_check_power; break; + case snd_soc_dapm_sink: + w->is_ep = SND_SOC_DAPM_EP_SINK; + w->power_check = dapm_always_on_check_power; + break; + case snd_soc_dapm_mux: case snd_soc_dapm_demux: case snd_soc_dapm_switch:
On 11/23/2015 04:52 PM, Vinod Koul wrote:
DAPM models various widgets but lacks a sink widget. DSPs can have modules which take audio data, process it and are capable of generating events thus acting as a sink of data.
To make the dapm graph complete for such paths we need a dapm sink widget for these modules, so add a SND_SOC_DAPM_SINK to declare such a widget. This widget will be treated as SND_SOC_DAPM_EP_SINK endpoint in the dapm graph
Except for the strange formating of the commit message this looks good.
Reviewed-by: Lars-Peter Clausen lars@metafoo.de
On Tue, Nov 24, 2015 at 10:48:42AM +0100, Lars-Peter Clausen wrote:
On 11/23/2015 04:52 PM, Vinod Koul wrote:
DAPM models various widgets but lacks a sink widget. DSPs can have modules which take audio data, process it and are capable of generating events thus acting as a sink of data.
To make the dapm graph complete for such paths we need a dapm sink widget for these modules, so add a SND_SOC_DAPM_SINK to declare such a widget. This widget will be treated as SND_SOC_DAPM_EP_SINK endpoint in the dapm graph
Except for the strange formating of the commit message this looks good.
Checkpath now seems to warn if commit message > 65chars so have set it for commit message
Reviewed-by: Lars-Peter Clausen lars@metafoo.de
Thanks
On Tue, Nov 24, 2015 at 03:49:07PM +0530, Vinod Koul wrote:
On Tue, Nov 24, 2015 at 10:48:42AM +0100, Lars-Peter Clausen wrote:
On 11/23/2015 04:52 PM, Vinod Koul wrote:
To make the dapm graph complete for such paths we need a dapm sink widget for these modules, so add a SND_SOC_DAPM_SINK to declare such a widget. This widget will be treated as SND_SOC_DAPM_EP_SINK endpoint in the dapm graph
Except for the strange formating of the commit message this looks good.
Checkpath now seems to warn if commit message > 65chars so have set it for commit message
I think it's more the random wrapping above - it looks like those are either two paragraphs or one paragraph with random word wrapping within it.
The patch
ASoC: dapm: add a dapm sink widget
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 56b4437f15ed2003413b857e08740576332e72d7 Mon Sep 17 00:00:00 2001
From: Vinod Koul vinod.koul@intel.com Date: Mon, 23 Nov 2015 21:22:30 +0530 Subject: [PATCH] ASoC: dapm: add a dapm sink widget
DAPM models various widgets but lacks a sink widget. DSPs can have modules which take audio data, process it and are capable of generating events thus acting as a sink of data.
To make the dapm graph complete for such paths we need a dapm sink widget for these modules, so add a SND_SOC_DAPM_SINK to declare such a widget. This widget will be treated as SND_SOC_DAPM_EP_SINK endpoint in the dapm graph
Signed-off-by: Vinod Koul vinod.koul@intel.com Reviewed-by: Lars-Peter Clausen lars@metafoo.de Signed-off-by: Mark Brown broonie@kernel.org --- include/sound/soc-dapm.h | 4 ++++ sound/soc/soc-dapm.c | 5 +++++ 2 files changed, 9 insertions(+)
diff --git a/include/sound/soc-dapm.h b/include/sound/soc-dapm.h index 7855cfe46b69..fe19dd3abb00 100644 --- a/include/sound/soc-dapm.h +++ b/include/sound/soc-dapm.h @@ -49,6 +49,9 @@ struct device; #define SND_SOC_DAPM_SIGGEN(wname) \ { .id = snd_soc_dapm_siggen, .name = wname, .kcontrol_news = NULL, \ .num_kcontrols = 0, .reg = SND_SOC_NOPM } +#define SND_SOC_DAPM_SINK(wname) \ +{ .id = snd_soc_dapm_sink, .name = wname, .kcontrol_news = NULL, \ + .num_kcontrols = 0, .reg = SND_SOC_NOPM } #define SND_SOC_DAPM_INPUT(wname) \ { .id = snd_soc_dapm_input, .name = wname, .kcontrol_news = NULL, \ .num_kcontrols = 0, .reg = SND_SOC_NOPM } @@ -484,6 +487,7 @@ enum snd_soc_dapm_type { snd_soc_dapm_aif_in, /* audio interface input */ snd_soc_dapm_aif_out, /* audio interface output */ snd_soc_dapm_siggen, /* signal generator */ + snd_soc_dapm_sink, snd_soc_dapm_dai_in, /* link to DAI structure */ snd_soc_dapm_dai_out, snd_soc_dapm_dai_link, /* link between two DAI structures */ diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 016eba10b1ec..6760044f6aae 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -3351,6 +3351,11 @@ snd_soc_dapm_new_control_unlocked(struct snd_soc_dapm_context *dapm, w->is_ep = SND_SOC_DAPM_EP_SOURCE; w->power_check = dapm_always_on_check_power; break; + case snd_soc_dapm_sink: + w->is_ep = SND_SOC_DAPM_EP_SINK; + w->power_check = dapm_always_on_check_power; + break; + case snd_soc_dapm_mux: case snd_soc_dapm_demux: case snd_soc_dapm_switch:
From: Jeeja KP jeeja.kp@intel.com
For DAPM resume, we should first change the power state of the card and then recheck the endpoints. This ensures the dapm is resumed first and then userspace can resume the streams.
Signed-off-by: Jeeja KP jeeja.kp@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com --- sound/soc/soc-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 0a26ac0fb513..6dd704a6b76f 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -846,12 +846,12 @@ static void soc_resume_deferred(struct work_struct *work)
dev_dbg(card->dev, "ASoC: resume work completed\n");
- /* userspace can access us now we are back as we were before */ - snd_power_change_state(card->snd_card, SNDRV_CTL_POWER_D0); - /* Recheck all endpoints too, their state is affected by suspend */ dapm_mark_endpoints_dirty(card); snd_soc_dapm_sync(&card->dapm); + + /* userspace can access us now we are back as we were before */ + snd_power_change_state(card->snd_card, SNDRV_CTL_POWER_D0); }
/* powers up audio subsystem after a suspend */
On 11/23/2015 04:52 PM, Vinod Koul wrote:
From: Jeeja KP jeeja.kp@intel.com
For DAPM resume, we should first change the power state of the card and then recheck the endpoints. This ensures the dapm is resumed first and then userspace can resume the streams.
The problem is that DAPM uses the userspace state to decide whether to power up or not. So this change won't work, it will keep the endpoints suspended.
On Mon, Nov 23, 2015 at 05:06:41PM +0100, Lars-Peter Clausen wrote:
On 11/23/2015 04:52 PM, Vinod Koul wrote:
From: Jeeja KP jeeja.kp@intel.com
For DAPM resume, we should first change the power state of the card and then recheck the endpoints. This ensures the dapm is resumed first and then userspace can resume the streams.
The problem is that DAPM uses the userspace state to decide whether to power up or not. So this change won't work, it will keep the endpoints suspended.
Well our observation are bit different...
When testing with aplay and suspending the system the DAPM was suspending fine and then alsa suspends PCMs, but on resume the DAPM is resumed last and the PCMs first, due to which the devices start underun! The DAPM should be resumed first and then we should allow userspace access (PCM resume from aplay)
The change of order here helps in that
On 11/23/2015 06:28 PM, Vinod Koul wrote:
On Mon, Nov 23, 2015 at 05:06:41PM +0100, Lars-Peter Clausen wrote:
On 11/23/2015 04:52 PM, Vinod Koul wrote:
From: Jeeja KP jeeja.kp@intel.com
For DAPM resume, we should first change the power state of the card and then recheck the endpoints. This ensures the dapm is resumed first and then userspace can resume the streams.
The problem is that DAPM uses the userspace state to decide whether to power up or not. So this change won't work, it will keep the endpoints suspended.
Well our observation are bit different...
When testing with aplay and suspending the system the DAPM was suspending fine and then alsa suspends PCMs, but on resume the DAPM is resumed last and the PCMs first, due to which the devices start underun! The DAPM should be resumed first and then we should allow userspace access (PCM resume from aplay)
The change of order here helps in that
Sorry, overlooked something. We go to D2 earlier on and DAPM only is suspended when the state is D3 and not only when it is not in D0.
Patch looks good.
Reviewed-by: Lars-Peter Clausen lars@metafoo.de
On Mon, Nov 23, 2015 at 07:13:12PM +0100, Lars-Peter Clausen wrote:
On 11/23/2015 06:28 PM, Vinod Koul wrote:
On Mon, Nov 23, 2015 at 05:06:41PM +0100, Lars-Peter Clausen wrote:
On 11/23/2015 04:52 PM, Vinod Koul wrote:
From: Jeeja KP jeeja.kp@intel.com
For DAPM resume, we should first change the power state of the card and then recheck the endpoints. This ensures the dapm is resumed first and then userspace can resume the streams.
The problem is that DAPM uses the userspace state to decide whether to power up or not. So this change won't work, it will keep the endpoints suspended.
Well our observation are bit different...
When testing with aplay and suspending the system the DAPM was suspending fine and then alsa suspends PCMs, but on resume the DAPM is resumed last and the PCMs first, due to which the devices start underun! The DAPM should be resumed first and then we should allow userspace access (PCM resume from aplay)
The change of order here helps in that
Sorry, overlooked something. We go to D2 earlier on and DAPM only is suspended when the state is D3 and not only when it is not in D0.
No issues
Patch looks good.
Reviewed-by: Lars-Peter Clausen lars@metafoo.de
Thanks for the review
The patch
ASoC: core: Change power state before rechecking endpoint
has been applied to the asoc tree at
git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
From 1a7aaa58ec7aaa389cd6b200809908ec472d316b Mon Sep 17 00:00:00 2001
From: Jeeja KP jeeja.kp@intel.com Date: Mon, 23 Nov 2015 21:22:31 +0530 Subject: [PATCH] ASoC: core: Change power state before rechecking endpoint
For DAPM resume, we should first change the power state of the card and then recheck the endpoints. This ensures the dapm is resumed first and then userspace can resume the streams.
Signed-off-by: Jeeja KP jeeja.kp@intel.com Signed-off-by: Vinod Koul vinod.koul@intel.com Reviewed-by: Lars-Peter Clausen lars@metafoo.de Signed-off-by: Mark Brown broonie@kernel.org --- sound/soc/soc-core.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 24b096066a07..a1305f827a98 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -795,12 +795,12 @@ static void soc_resume_deferred(struct work_struct *work)
dev_dbg(card->dev, "ASoC: resume work completed\n");
- /* userspace can access us now we are back as we were before */ - snd_power_change_state(card->snd_card, SNDRV_CTL_POWER_D0); - /* Recheck all endpoints too, their state is affected by suspend */ dapm_mark_endpoints_dirty(card); snd_soc_dapm_sync(&card->dapm); + + /* userspace can access us now we are back as we were before */ + snd_power_change_state(card->snd_card, SNDRV_CTL_POWER_D0); }
/* powers up audio subsystem after a suspend */
participants (3)
-
Lars-Peter Clausen -
Mark Brown -
Vinod Koul