From 66de6beb933d373224f350834fbab68093d24627 Mon Sep 17 00:00:00 2001 From: Ranjani Sridharan Date: Wed, 25 Mar 2020 16:12:29 -0500 Subject: [PATCH 1/5] ASoC: SOF: Intel: hda: Improve DSP state logging Improve the DSP power state logs with the state names instead of values. Signed-off-by: Ranjani Sridharan Signed-off-by: Pierre-Louis Bossart Link: https://lore.kernel.org/r/20200325211233.27394-2-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/hda-dsp.c | 43 +++++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index 79ce52c32ef1..c396b7ef0328 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -452,6 +452,46 @@ revert: return ret; } +/* helper to log DSP state */ +static void hda_dsp_state_log(struct snd_sof_dev *sdev) +{ + switch (sdev->dsp_power_state.state) { + case SOF_DSP_PM_D0: + switch (sdev->dsp_power_state.substate) { + case SOF_HDA_DSP_PM_D0I0: + dev_dbg(sdev->dev, "Current DSP power state: D0I0\n"); + break; + case SOF_HDA_DSP_PM_D0I3: + dev_dbg(sdev->dev, "Current DSP power state: D0I3\n"); + break; + default: + dev_dbg(sdev->dev, "Unknown DSP D0 substate: %d\n", + sdev->dsp_power_state.substate); + break; + } + break; + case SOF_DSP_PM_D1: + dev_dbg(sdev->dev, "Current DSP power state: D1\n"); + break; + case SOF_DSP_PM_D2: + dev_dbg(sdev->dev, "Current DSP power state: D2\n"); + break; + case SOF_DSP_PM_D3_HOT: + dev_dbg(sdev->dev, "Current DSP power state: D3_HOT\n"); + break; + case SOF_DSP_PM_D3: + dev_dbg(sdev->dev, "Current DSP power state: D3\n"); + break; + case SOF_DSP_PM_D3_COLD: + dev_dbg(sdev->dev, "Current DSP power state: D3_COLD\n"); + break; + default: + dev_dbg(sdev->dev, "Unknown DSP power state: %d\n", + sdev->dsp_power_state.state); + break; + } +} + /* * All DSP power state transitions are initiated by the driver. * If the requested state change fails, the error is simply returned. @@ -511,8 +551,7 @@ set_state: } sdev->dsp_power_state = *target_state; - dev_dbg(sdev->dev, "New DSP state %d substate %d\n", - target_state->state, target_state->substate); + hda_dsp_state_log(sdev); return ret; } From c688cf1d3a2cc1bca5737e7849325b3ac8e69a41 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Wed, 25 Mar 2020 16:12:30 -0500 Subject: [PATCH 2/5] ASoC: SOF: (cosmetic) use for_each_pcm_streams() in sof_dai_load() Use for_each_pcm_streams() to enumerate streams in sof_dai_load() instead of doing that manually. Signed-off-by: Guennadi Liakhovetski Signed-off-by: Pierre-Louis Bossart Reviewed-by: Kai Vehmanen Reviewed-by: Ranjani Sridharan Link: https://lore.kernel.org/r/20200325211233.27394-3-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/topology.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 058de94fb8cf..54437caf9488 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -2448,7 +2448,7 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, struct snd_soc_tplg_stream_caps *caps; struct snd_soc_tplg_private *private = &pcm->priv; struct snd_sof_pcm *spcm; - int stream = SNDRV_PCM_STREAM_PLAYBACK; + int stream; int ret = 0; /* nothing to do for BEs atm */ @@ -2460,8 +2460,9 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, return -ENOMEM; spcm->scomp = scomp; - spcm->stream[SNDRV_PCM_STREAM_PLAYBACK].comp_id = COMP_ID_UNASSIGNED; - spcm->stream[SNDRV_PCM_STREAM_CAPTURE].comp_id = COMP_ID_UNASSIGNED; + + for_each_pcm_streams(stream) + spcm->stream[stream].comp_id = COMP_ID_UNASSIGNED; spcm->pcm = *pcm; dev_dbg(scomp->dev, "tplg: load pcm %s\n", pcm->dai_name); @@ -2482,8 +2483,10 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, if (!spcm->pcm.playback) goto capture; + stream = SNDRV_PCM_STREAM_PLAYBACK; + dev_vdbg(scomp->dev, "tplg: pcm %s stream tokens: playback d0i3:%d\n", - spcm->pcm.pcm_name, spcm->stream[0].d0i3_compatible); + spcm->pcm.pcm_name, spcm->stream[stream].d0i3_compatible); caps = &spcm->pcm.caps[stream]; @@ -2513,7 +2516,7 @@ capture: return ret; dev_vdbg(scomp->dev, "tplg: pcm %s stream tokens: capture d0i3:%d\n", - spcm->pcm.pcm_name, spcm->stream[1].d0i3_compatible); + spcm->pcm.pcm_name, spcm->stream[stream].d0i3_compatible); caps = &spcm->pcm.caps[stream]; From 9ef91cad92ba75d17d5a5203230746c9d9009705 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Wed, 25 Mar 2020 16:12:31 -0500 Subject: [PATCH 3/5] ASoC: SOF: fix uninitialised "work" with VirtIO In the VirtIO case the sof_pcm_open() function isn't called on the host during guest streaming, which then leaves "work" structures uninitialised. However it is then used to handle position update messages from the DSP. Move their initialisation to immediately after allocation of the containing structure. Signed-off-by: Guennadi Liakhovetski Signed-off-by: Pierre-Louis Bossart Reviewed-by: Kai Vehmanen Reviewed-by: Ranjani Sridharan Link: https://lore.kernel.org/r/20200325211233.27394-4-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/pcm.c | 4 +--- sound/soc/sof/sof-audio.h | 3 +++ sound/soc/sof/topology.c | 6 +++++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c index f4769e19965a..47cd741f2a8c 100644 --- a/sound/soc/sof/pcm.c +++ b/sound/soc/sof/pcm.c @@ -57,7 +57,7 @@ static int sof_pcm_dsp_params(struct snd_sof_pcm *spcm, struct snd_pcm_substream /* * sof pcm period elapse work */ -static void sof_pcm_period_elapsed_work(struct work_struct *work) +void snd_sof_pcm_period_elapsed_work(struct work_struct *work) { struct snd_sof_pcm_stream *sps = container_of(work, struct snd_sof_pcm_stream, @@ -475,8 +475,6 @@ static int sof_pcm_open(struct snd_soc_component *component, dev_dbg(component->dev, "pcm: open stream %d dir %d\n", spcm->pcm.pcm_id, substream->stream); - INIT_WORK(&spcm->stream[substream->stream].period_elapsed_work, - sof_pcm_period_elapsed_work); caps = &spcm->pcm.caps[substream->stream]; diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index eacd10e4da11..bf65f31af858 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -11,6 +11,8 @@ #ifndef __SOUND_SOC_SOF_AUDIO_H #define __SOUND_SOC_SOF_AUDIO_H +#include + #include #include #include /* needs to be included before control.h */ @@ -189,6 +191,7 @@ struct snd_sof_pcm *snd_sof_find_spcm_comp(struct snd_soc_component *scomp, struct snd_sof_pcm *snd_sof_find_spcm_pcm_id(struct snd_soc_component *scomp, unsigned int pcm_id); void snd_sof_pcm_period_elapsed(struct snd_pcm_substream *substream); +void snd_sof_pcm_period_elapsed_work(struct work_struct *work); /* * Mixer IPC diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 54437caf9488..fe8ba3e05e08 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -9,6 +9,7 @@ // #include +#include #include #include #include @@ -2461,8 +2462,11 @@ static int sof_dai_load(struct snd_soc_component *scomp, int index, spcm->scomp = scomp; - for_each_pcm_streams(stream) + for_each_pcm_streams(stream) { spcm->stream[stream].comp_id = COMP_ID_UNASSIGNED; + INIT_WORK(&spcm->stream[stream].period_elapsed_work, + snd_sof_pcm_period_elapsed_work); + } spcm->pcm = *pcm; dev_dbg(scomp->dev, "tplg: load pcm %s\n", pcm->dai_name); From aae5a6e92f3f3411fdf6abcf41ecadd771abaa4b Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Wed, 25 Mar 2020 16:12:32 -0500 Subject: [PATCH 4/5] ASoC: SOF: Intel: hda: do not leave clock gating off upon error The misc clock gating (MISCBDCGE) is disabled for controller reset and reenabled once reset is complete. Fix the case when error happens during reset, and clock gating is left disabled. The clock gating should be reenabled also in this case. Signed-off-by: Kai Vehmanen Signed-off-by: Pierre-Louis Bossart Reviewed-by: Ranjani Sridharan Link: https://lore.kernel.org/r/20200325211233.27394-5-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/hda-ctrl.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/sound/soc/sof/intel/hda-ctrl.c b/sound/soc/sof/intel/hda-ctrl.c index 871b71a15a63..93be6fc51ccd 100644 --- a/sound/soc/sof/intel/hda-ctrl.c +++ b/sound/soc/sof/intel/hda-ctrl.c @@ -183,7 +183,7 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset) ret = hda_dsp_ctrl_link_reset(sdev, true); if (ret < 0) { dev_err(sdev->dev, "error: failed to reset HDA controller\n"); - return ret; + goto err; } usleep_range(500, 1000); @@ -192,7 +192,7 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset) ret = hda_dsp_ctrl_link_reset(sdev, false); if (ret < 0) { dev_err(sdev->dev, "error: failed to exit HDA controller reset\n"); - return ret; + goto err; } usleep_range(1000, 1200); @@ -202,7 +202,8 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset) /* check to see if controller is ready */ if (!snd_hdac_chip_readb(bus, GCTL)) { dev_dbg(bus->dev, "controller not ready!\n"); - return -EBUSY; + ret = -EBUSY; + goto err; } /* Accept unsolicited responses */ @@ -268,6 +269,7 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset) bus->chip_init = true; +err: hda_dsp_ctrl_misc_clock_gating(sdev, true); return ret; From 7e26df0ced1643679922d197e798d469ac3bf9c0 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Wed, 25 Mar 2020 16:12:33 -0500 Subject: [PATCH 5/5] ASoC: SOF: Intel: hda: call codec wake at chip init Further align HDA init sequence to the legacy non-DSP HDA driver by calling snd_hdac_set_codec_wakeup() during the chip init sequence. Signed-off-by: Kai Vehmanen Signed-off-by: Pierre-Louis Bossart Reviewed-by: Ranjani Sridharan Link: https://lore.kernel.org/r/20200325211233.27394-6-pierre-louis.bossart@linux.intel.com Signed-off-by: Mark Brown --- sound/soc/sof/intel/hda-ctrl.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/sound/soc/sof/intel/hda-ctrl.c b/sound/soc/sof/intel/hda-ctrl.c index 93be6fc51ccd..f88dbcc4ba66 100644 --- a/sound/soc/sof/intel/hda-ctrl.c +++ b/sound/soc/sof/intel/hda-ctrl.c @@ -18,6 +18,7 @@ #include #include #include +#include #include "../ops.h" #include "hda.h" @@ -176,6 +177,9 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset) if (bus->chip_init) return 0; +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) + snd_hdac_set_codec_wakeup(bus, true); +#endif hda_dsp_ctrl_misc_clock_gating(sdev, false); if (full_reset) { @@ -271,6 +275,9 @@ int hda_dsp_ctrl_init_chip(struct snd_sof_dev *sdev, bool full_reset) err: hda_dsp_ctrl_misc_clock_gating(sdev, true); +#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) + snd_hdac_set_codec_wakeup(bus, false); +#endif return ret; }