[RFC PATCH] ASoC: wm_adsp: Change ADSPs to be mixer widgets rather the PGAs

[RFC PATCH] ASoC: wm_adsp: Change ADSPs to be mixer widgets rather the PGAs

[Charles Keepax]

PGAs often control the output from a chip and if the DSP is also marked
as a PGA it may be powered up after the output has been enabled. This
patch changes the ADSP2 cores to be marked as snd_soc_dapm_mixer widgets
so they are powered up before any PGAs.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---

Hi,

Was also considering if it would be worth adding an
additional snd_soc_dapm_dsp id? That could sit between
mixers and pgas, but I can't really see any obvious issue
with treating the DSP as a mixer and it is a much simpler
change. Although I am open to writing the other change if it
is preferred?

Thanks,
Charles

 sound/soc/codecs/wm_adsp.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/sound/soc/codecs/wm_adsp.h b/sound/soc/codecs/wm_adsp.h
index d018dea..795a3b2 100644
--- a/sound/soc/codecs/wm_adsp.h
+++ b/sound/soc/codecs/wm_adsp.h
@@ -66,7 +66,7 @@ struct wm_adsp {
 		wm_adsp1_event, SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD)
 
 #define WM_ADSP2(wname, num) \
-	SND_SOC_DAPM_PGA_E(wname, SND_SOC_NOPM, num, 0, NULL, 0, \
+	SND_SOC_DAPM_MIXER_E(wname, SND_SOC_NOPM, num, 0, NULL, 0, \
 		wm_adsp2_event, SND_SOC_DAPM_POST_PMU | SND_SOC_DAPM_PRE_PMD)
 
 extern const struct snd_kcontrol_new wm_adsp1_fw_controls[];
-- 
1.7.2.5

Re: [RFC PATCH] ASoC: wm_adsp: Change ADSPs to be mixer widgets rather the PGAs

[Mark Brown]

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

On Wed, Dec 18, 2013 at 10:53:44AM +0000, Charles Keepax wrote:

> Was also considering if it would be worth adding an
> additional snd_soc_dapm_dsp id? That could sit between
> mixers and pgas, but I can't really see any obvious issue
> with treating the DSP as a mixer and it is a much simpler
> change. Although I am open to writing the other change if it
> is preferred?

One of the issues here was trying to ensure that the DSP started up with
its inputs stable so noise from them starting didn't propagage into the
algorithm and confuse it.  The expecation with putting it as a PGA was
that it would start with the outputs mute and do a digital unmute to
bring them up.  Since everything is digital this should all be more
robust than it would be for analogue.

If anything I'd have a specific DSP widget type which is one of the last
things to start.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

Re: [RFC PATCH] ASoC: wm_adsp: Change ADSPs to be mixer widgets rather the PGAs

[Charles Keepax]

On Wed, Dec 18, 2013 at 11:27:39AM +0000, Mark Brown wrote:
> On Wed, Dec 18, 2013 at 10:53:44AM +0000, Charles Keepax wrote:
> 
> > Was also considering if it would be worth adding an
> > additional snd_soc_dapm_dsp id? That could sit between
> > mixers and pgas, but I can't really see any obvious issue
> > with treating the DSP as a mixer and it is a much simpler
> > change. Although I am open to writing the other change if it
> > is preferred?
> 
> One of the issues here was trying to ensure that the DSP started up with
> its inputs stable so noise from them starting didn't propagage into the
> algorithm and confuse it.  The expecation with putting it as a PGA was
> that it would start with the outputs mute and do a digital unmute to
> bring them up.  Since everything is digital this should all be more
> robust than it would be for analogue.

Muting the output is a little tricky though as a graph walk will
be required to determine which output was connected, unless you
have any handy ideas I have not spotted? I will start having a
look to see what could be done on the muting front.

Thanks,
Charles

Re: [RFC PATCH] ASoC: wm_adsp: Change ADSPs to be mixer widgets rather the PGAs

[Mark Brown]

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

On Wed, Dec 18, 2013 at 01:14:36PM +0000, Charles Keepax wrote:
> On Wed, Dec 18, 2013 at 11:27:39AM +0000, Mark Brown wrote:

> > One of the issues here was trying to ensure that the DSP started up with
> > its inputs stable so noise from them starting didn't propagage into the
> > algorithm and confuse it.  The expecation with putting it as a PGA was
> > that it would start with the outputs mute and do a digital unmute to
> > bring them up.  Since everything is digital this should all be more
> > robust than it would be for analogue.

> Muting the output is a little tricky though as a graph walk will
> be required to determine which output was connected, unless you
> have any handy ideas I have not spotted? I will start having a
> look to see what could be done on the muting front.

Not the physical output, the DSP output into the rest of the device.  It
would be very surprising if it powered up making noise.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

Re: [RFC PATCH] ASoC: wm_adsp: Change ADSPs to be mixer widgets rather the PGAs

[Charles Keepax]

On Wed, Dec 18, 2013 at 01:23:43PM +0000, Mark Brown wrote:
> On Wed, Dec 18, 2013 at 01:14:36PM +0000, Charles Keepax wrote:
> > Muting the output is a little tricky though as a graph walk will
> > be required to determine which output was connected, unless you
> > have any handy ideas I have not spotted? I will start having a
> > look to see what could be done on the muting front.
> 
> Not the physical output, the DSP output into the rest of the device.  It
> would be very surprising if it powered up making noise.

Regrettably, I don't believe there is a way to mute the output of
the DSP. Muting the physical output does fix the issue.  I could
look at seperating out the physical power up of the core and
starting the firmware to be seperate widgets, that might also
fix the issue and would mean the firmware wouldn't run until the
input signal was clean, but I am not sure if feels that neat a
solution.

Thanks,
Charles

Re: [RFC PATCH] ASoC: wm_adsp: Change ADSPs to be mixer widgets rather the PGAs

[Mark Brown]

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

On Wed, Dec 18, 2013 at 02:14:23PM +0000, Charles Keepax wrote:
> On Wed, Dec 18, 2013 at 01:23:43PM +0000, Mark Brown wrote:

> > Not the physical output, the DSP output into the rest of the device.  It
> > would be very surprising if it powered up making noise.

> Regrettably, I don't believe there is a way to mute the output of
> the DSP. Muting the physical output does fix the issue.  I could

So the DSP powers up outputing stuff?  That seems like an interesting
design decision (it might need something to reset the buffers on power
down I guess).

> look at seperating out the physical power up of the core and
> starting the firmware to be seperate widgets, that might also
> fix the issue and would mean the firmware wouldn't run until the
> input signal was clean, but I am not sure if feels that neat a
> solution.

That sort of split is something I'd considered doing in the past anyway
- it would be beneficial to be able to overlap the DSP startup with
other activities so the delays while bringing up the outputs could be
happening while the DSPs are downloading firmware or the firmware is
starting up for example.  You could also overlap stuff with the RAM
startup if that's now taking longer.

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

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

Re: [RFC PATCH] ASoC: wm_adsp: Change ADSPs to be mixer widgets rather the PGAs

[Charles Keepax]

On Wed, Dec 18, 2013 at 04:19:02PM +0000, Mark Brown wrote:
> On Wed, Dec 18, 2013 at 02:14:23PM +0000, Charles Keepax wrote:
> > On Wed, Dec 18, 2013 at 01:23:43PM +0000, Mark Brown wrote:
> 
> > > Not the physical output, the DSP output into the rest of the device.  It
> > > would be very surprising if it powered up making noise.
> 
> > Regrettably, I don't believe there is a way to mute the output of
> > the DSP. Muting the physical output does fix the issue.  I could
> 
> So the DSP powers up outputing stuff?  That seems like an interesting
> design decision (it might need something to reset the buffers on power
> down I guess).

It is not sure it is so much a design decision but there is an
audible pop when the DSP core enable bit is set if it is
connected to an unmuted output.

> 
> > look at seperating out the physical power up of the core and
> > starting the firmware to be seperate widgets, that might also
> > fix the issue and would mean the firmware wouldn't run until the
> > input signal was clean, but I am not sure if feels that neat a
> > solution.
> 
> That sort of split is something I'd considered doing in the past anyway
> - it would be beneficial to be able to overlap the DSP startup with
> other activities so the delays while bringing up the outputs could be
> happening while the DSPs are downloading firmware or the firmware is
> starting up for example.  You could also overlap stuff with the RAM
> startup if that's now taking longer.

Ok this looks like a potentially viable solution, and I hadn't
considered the potential benefits it could have to speed up the
audio path bring up time, I shall have a bash at implementing it
and see where I get to.

Thanks,
Charles