[PATCH 0/3] ASoC: core: dapm more updates

[PATCH 0/3] ASoC: core: dapm more updates

[Vinod Koul]

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(-)

-- 
1.9.1

[PATCH 1/3] ASoC: core: mark SND_SOC_BYTES_EXT as deprecated

[Vinod Koul]

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, \
-- 
1.9.1

Applied "ASoC: core: mark SND_SOC_BYTES_EXT as deprecated" to the asoc tree

[Mark Brown]

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, \
-- 
2.6.2

[PATCH 2/3] ASoC: core: add a dapm sink widget

[Vinod Koul]

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:
-- 
1.9.1

Re: [PATCH 2/3] ASoC: core: add a dapm sink widget

[Lars-Peter Clausen]

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>

Re: [PATCH 2/3] ASoC: core: add a dapm sink widget

[Vinod Koul]

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

-- 
~Vinod

Re: [PATCH 2/3] ASoC: core: add a dapm sink widget

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 751 bytes --]

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.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Applied "ASoC: dapm: add a dapm sink widget" to the asoc tree

[Mark Brown]

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:
-- 
2.6.2

[PATCH 3/3] ASoC: core: Change power state before rechecking endpoint

[Vinod Koul]

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 */
-- 
1.9.1

Re: [PATCH 3/3] ASoC: core: Change power state before rechecking endpoint

[Lars-Peter Clausen]

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.

Re: [PATCH 3/3] ASoC: core: Change power state before rechecking endpoint

[Vinod Koul]

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

-- 
~Vinod

Re: [PATCH 3/3] ASoC: core: Change power state before rechecking endpoint

[Lars-Peter Clausen]

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>

Re: [PATCH 3/3] ASoC: core: Change power state before rechecking endpoint

[Vinod Koul]

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

-- 
~Vinod

Applied "ASoC: core: Change power state before rechecking endpoint" to the asoc tree

[Mark Brown]

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 */
-- 
2.6.2