* [PATCH 1/2] ASoC: adau17x1: Handling of DSP_RUN register during fw setup @ 2018-04-03 8:05 Robert Rosengren 2018-04-03 8:05 ` [PATCH 2/2] ASoC: adau17x1: Do not reload dsp-fw if samplerate has not changed Robert Rosengren 2018-04-05 15:44 ` [PATCH 1/2] ASoC: adau17x1: Handling of DSP_RUN register during fw setup Lars-Peter Clausen 0 siblings, 2 replies; 5+ messages in thread From: Robert Rosengren @ 2018-04-03 8:05 UTC (permalink / raw) To: Lars-Peter Clausen, alsa-devel; +Cc: Danny Smith From: Danny Smith <dannys@axis.com> DSP_RUN needs to be disabled during firmware write otherwise we can end up with undefined behavior if writing to a dsp which is already running firmware. Signed-off-by: Danny Smith <dannys@axis.com> --- sound/soc/codecs/adau17x1.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/sound/soc/codecs/adau17x1.c b/sound/soc/codecs/adau17x1.c index 80c2a06285bb..5636b9522462 100644 --- a/sound/soc/codecs/adau17x1.c +++ b/sound/soc/codecs/adau17x1.c @@ -838,14 +838,19 @@ EXPORT_SYMBOL_GPL(adau17x1_volatile_register); int adau17x1_setup_firmware(struct adau *adau, unsigned int rate) { int ret; - int dspsr; + int dspsr, dsp_run; ret = regmap_read(adau->regmap, ADAU17X1_DSP_SAMPLING_RATE, &dspsr); if (ret) return ret; + ret = regmap_read(adau->regmap, ADAU17X1_DSP_RUN, &dsp_run); + if (ret) + return ret; + regmap_write(adau->regmap, ADAU17X1_DSP_ENABLE, 1); regmap_write(adau->regmap, ADAU17X1_DSP_SAMPLING_RATE, 0xf); + regmap_write(adau->regmap, ADAU17X1_DSP_RUN, 0); ret = sigmadsp_setup(adau->sigmadsp, rate); if (ret) { @@ -853,6 +858,7 @@ int adau17x1_setup_firmware(struct adau *adau, unsigned int rate) return ret; } regmap_write(adau->regmap, ADAU17X1_DSP_SAMPLING_RATE, dspsr); + regmap_write(adau->regmap, ADAU17X1_DSP_RUN, dsp_run); return 0; } -- 2.11.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] ASoC: adau17x1: Do not reload dsp-fw if samplerate has not changed 2018-04-03 8:05 [PATCH 1/2] ASoC: adau17x1: Handling of DSP_RUN register during fw setup Robert Rosengren @ 2018-04-03 8:05 ` Robert Rosengren 2018-04-05 15:33 ` Lars-Peter Clausen 2018-04-05 15:44 ` [PATCH 1/2] ASoC: adau17x1: Handling of DSP_RUN register during fw setup Lars-Peter Clausen 1 sibling, 1 reply; 5+ messages in thread From: Robert Rosengren @ 2018-04-03 8:05 UTC (permalink / raw) To: Lars-Peter Clausen, alsa-devel; +Cc: Danny Smith From: Danny Smith <dannys@axis.com> Reloading fw causes an audiable popping sound, we can avoid this by not reloading if the samplerate is the same as before. Signed-off-by: Danny Smith <dannys@axis.com> --- sound/soc/codecs/adau17x1.c | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/sound/soc/codecs/adau17x1.c b/sound/soc/codecs/adau17x1.c index 5636b9522462..3c28b7191ecd 100644 --- a/sound/soc/codecs/adau17x1.c +++ b/sound/soc/codecs/adau17x1.c @@ -840,25 +840,34 @@ int adau17x1_setup_firmware(struct adau *adau, unsigned int rate) int ret; int dspsr, dsp_run; - ret = regmap_read(adau->regmap, ADAU17X1_DSP_SAMPLING_RATE, &dspsr); - if (ret) - return ret; + /* Check if sample rate is the same as before. If it is there is no + * point in performing the below steps as the call to + * sigmadsp_setup(...) will return directly when it finds the sample + * rate to be the same as before. By checking this we can prevent an + * audiable popping noise which occours when toggling DSP_RUN. + */ + if (adau->sigmadsp->current_samplerate != rate) { + ret = regmap_read(adau->regmap, ADAU17X1_DSP_SAMPLING_RATE, + &dspsr); + if (ret) + return ret; - ret = regmap_read(adau->regmap, ADAU17X1_DSP_RUN, &dsp_run); - if (ret) - return ret; + ret = regmap_read(adau->regmap, ADAU17X1_DSP_RUN, &dsp_run); + if (ret) + return ret; - regmap_write(adau->regmap, ADAU17X1_DSP_ENABLE, 1); - regmap_write(adau->regmap, ADAU17X1_DSP_SAMPLING_RATE, 0xf); - regmap_write(adau->regmap, ADAU17X1_DSP_RUN, 0); + regmap_write(adau->regmap, ADAU17X1_DSP_ENABLE, 1); + regmap_write(adau->regmap, ADAU17X1_DSP_SAMPLING_RATE, 0xf); + regmap_write(adau->regmap, ADAU17X1_DSP_RUN, 0); - ret = sigmadsp_setup(adau->sigmadsp, rate); - if (ret) { - regmap_write(adau->regmap, ADAU17X1_DSP_ENABLE, 0); - return ret; + ret = sigmadsp_setup(adau->sigmadsp, rate); + if (ret) { + regmap_write(adau->regmap, ADAU17X1_DSP_ENABLE, 0); + return ret; + } + regmap_write(adau->regmap, ADAU17X1_DSP_SAMPLING_RATE, dspsr); + regmap_write(adau->regmap, ADAU17X1_DSP_RUN, dsp_run); } - regmap_write(adau->regmap, ADAU17X1_DSP_SAMPLING_RATE, dspsr); - regmap_write(adau->regmap, ADAU17X1_DSP_RUN, dsp_run); return 0; } -- 2.11.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 2/2] ASoC: adau17x1: Do not reload dsp-fw if samplerate has not changed 2018-04-03 8:05 ` [PATCH 2/2] ASoC: adau17x1: Do not reload dsp-fw if samplerate has not changed Robert Rosengren @ 2018-04-05 15:33 ` Lars-Peter Clausen 0 siblings, 0 replies; 5+ messages in thread From: Lars-Peter Clausen @ 2018-04-05 15:33 UTC (permalink / raw) To: Robert Rosengren, alsa-devel; +Cc: Danny Smith On 04/03/2018 10:05 AM, Robert Rosengren wrote: > From: Danny Smith <dannys@axis.com> > > Reloading fw causes an audiable popping sound, we can avoid this > by not reloading if the samplerate is the same as before. > Seems like a sensible idea thanks. > Signed-off-by: Danny Smith <dannys@axis.com> > --- > sound/soc/codecs/adau17x1.c | 39 ++++++++++++++++++++++++--------------- > 1 file changed, 24 insertions(+), 15 deletions(-) > > diff --git a/sound/soc/codecs/adau17x1.c b/sound/soc/codecs/adau17x1.c > index 5636b9522462..3c28b7191ecd 100644 > --- a/sound/soc/codecs/adau17x1.c > +++ b/sound/soc/codecs/adau17x1.c > @@ -840,25 +840,34 @@ int adau17x1_setup_firmware(struct adau *adau, unsigned int rate) > int ret; > int dspsr, dsp_run; > > - ret = regmap_read(adau->regmap, ADAU17X1_DSP_SAMPLING_RATE, &dspsr); > - if (ret) > - return ret; > + /* Check if sample rate is the same as before. If it is there is no > + * point in performing the below steps as the call to > + * sigmadsp_setup(...) will return directly when it finds the sample > + * rate to be the same as before. By checking this we can prevent an > + * audiable popping noise which occours when toggling DSP_RUN. > + */ > + if (adau->sigmadsp->current_samplerate != rate) { I think in order to avoid all that re-indention you could just if (adau->sigmadsp->current_samplerate == rate) return 0; [...] ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] ASoC: adau17x1: Handling of DSP_RUN register during fw setup 2018-04-03 8:05 [PATCH 1/2] ASoC: adau17x1: Handling of DSP_RUN register during fw setup Robert Rosengren 2018-04-03 8:05 ` [PATCH 2/2] ASoC: adau17x1: Do not reload dsp-fw if samplerate has not changed Robert Rosengren @ 2018-04-05 15:44 ` Lars-Peter Clausen 2018-04-11 5:16 ` Robert Rosengren 1 sibling, 1 reply; 5+ messages in thread From: Lars-Peter Clausen @ 2018-04-05 15:44 UTC (permalink / raw) To: Robert Rosengren, alsa-devel; +Cc: Danny Smith On 04/03/2018 10:05 AM, Robert Rosengren wrote: > From: Danny Smith <dannys@axis.com> > > DSP_RUN needs to be disabled during firmware write otherwise > we can end up with undefined behavior if writing to a dsp which > is already running firmware. > Good point, thanks. I believe there is a possible (but rare under normal circumstances) race condition here though, where DAPM could run at the same time and enable/disable the DSP while the firmware is being loaded. To avoid this the firmware load sequence should be encapsulated in snd_soc_dapm_mutex_lock()/snd_soc_dapm_mutex_unlock(). Pass struct snd_soc_component instead of struct adau to this function and then use snd_soc_component_get_dapm() to get the DAPM context. For the next version of the patches please also add the ASoC maintainers to the reciver list since they have to apply the patch. The maintainers are: Liam Girdwood <lgirdwood@gmail.com> Mark Brown <broonie@kernel.org> > Signed-off-by: Danny Smith <dannys@axis.com> > --- > sound/soc/codecs/adau17x1.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/sound/soc/codecs/adau17x1.c b/sound/soc/codecs/adau17x1.c > index 80c2a06285bb..5636b9522462 100644 > --- a/sound/soc/codecs/adau17x1.c > +++ b/sound/soc/codecs/adau17x1.c > @@ -838,14 +838,19 @@ EXPORT_SYMBOL_GPL(adau17x1_volatile_register); > int adau17x1_setup_firmware(struct adau *adau, unsigned int rate) > { > int ret; > - int dspsr; > + int dspsr, dsp_run; > > ret = regmap_read(adau->regmap, ADAU17X1_DSP_SAMPLING_RATE, &dspsr); > if (ret) > return ret; > > + ret = regmap_read(adau->regmap, ADAU17X1_DSP_RUN, &dsp_run); > + if (ret) > + return ret; > + > regmap_write(adau->regmap, ADAU17X1_DSP_ENABLE, 1); > regmap_write(adau->regmap, ADAU17X1_DSP_SAMPLING_RATE, 0xf); > + regmap_write(adau->regmap, ADAU17X1_DSP_RUN, 0); > > ret = sigmadsp_setup(adau->sigmadsp, rate); > if (ret) { > @@ -853,6 +858,7 @@ int adau17x1_setup_firmware(struct adau *adau, unsigned int rate) > return ret; > } > regmap_write(adau->regmap, ADAU17X1_DSP_SAMPLING_RATE, dspsr); > + regmap_write(adau->regmap, ADAU17X1_DSP_RUN, dsp_run); > > return 0; > } > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] ASoC: adau17x1: Handling of DSP_RUN register during fw setup 2018-04-05 15:44 ` [PATCH 1/2] ASoC: adau17x1: Handling of DSP_RUN register during fw setup Lars-Peter Clausen @ 2018-04-11 5:16 ` Robert Rosengren 0 siblings, 0 replies; 5+ messages in thread From: Robert Rosengren @ 2018-04-11 5:16 UTC (permalink / raw) To: Lars-Peter Clausen, alsa-devel; +Cc: Danny Smith On 04/05/2018 05:44 PM, Lars-Peter Clausen wrote: > On 04/03/2018 10:05 AM, Robert Rosengren wrote: >> From: Danny Smith <dannys@axis.com> >> >> DSP_RUN needs to be disabled during firmware write otherwise >> we can end up with undefined behavior if writing to a dsp which >> is already running firmware. >> > > Good point, thanks. > > I believe there is a possible (but rare under normal circumstances) race > condition here though, where DAPM could run at the same time and > enable/disable the DSP while the firmware is being loaded. > > To avoid this the firmware load sequence should be encapsulated in > snd_soc_dapm_mutex_lock()/snd_soc_dapm_mutex_unlock(). > > Pass struct snd_soc_component instead of struct adau to this function and > then use snd_soc_component_get_dapm() to get the DAPM context. > > For the next version of the patches please also add the ASoC maintainers to > the reciver list since they have to apply the patch. The maintainers are: > > Liam Girdwood <lgirdwood@gmail.com> > Mark Brown <broonie@kernel.org> > Thanks for your comments! New patches are already sent for your review :) / Robert > >> Signed-off-by: Danny Smith <dannys@axis.com> >> --- >> sound/soc/codecs/adau17x1.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-04-11 5:16 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-04-03 8:05 [PATCH 1/2] ASoC: adau17x1: Handling of DSP_RUN register during fw setup Robert Rosengren 2018-04-03 8:05 ` [PATCH 2/2] ASoC: adau17x1: Do not reload dsp-fw if samplerate has not changed Robert Rosengren 2018-04-05 15:33 ` Lars-Peter Clausen 2018-04-05 15:44 ` [PATCH 1/2] ASoC: adau17x1: Handling of DSP_RUN register during fw setup Lars-Peter Clausen 2018-04-11 5:16 ` Robert Rosengren
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox