[alsa-devel] [PATCH 0/9] ASoC: SOF: update S0ix/D0ix support

[alsa-devel] [PATCH 0/9] ASoC: SOF: update S0ix/D0ix support

[Pierre-Louis Bossart]

The initial support for S0ix had limitations that were identified
during testing, and opportunitic transitions such as S0/D0ix was not a
case that was handled.

Ranjani suggested a major rework reviewed at length on GitHub
[1][2]. So far all known issues are solved and no regressions are
identified on 5 different platforms, so it's time to share
upstream.

This is not intended for 5.6 and can wait after the merge window
closes. But since the code isn't simple we want to allow plenty of
time for further reviews.

Thanks Ranjani for this important update!

[1] https://github.com/thesofproject/linux/pull/1631
[2] https://github.com/thesofproject/linux/pull/1702

Ranjani Sridharan (9):
  ASoC: SOF: Do not reset hw_params for streams that ignored suspend
  ASoC: SOF: pm: Unify suspend/resume routines
  ASoC: SOF: Add system_suspend_target field to struct snd_sof_dev
  ASoC: SOF: pm: Introduce DSP power states
  ASoC: SOF: Move DSP power state transitions to platform-specific ops
  ASoC: SOF: audio: Add helper to check if only D0i3 streams are active
  ASoC: SOF: Intel: hda: Amend the DSP state transition diagram
  ASoC: SOF: Intel: cnl: Implement feature to support DSP D0i3 in S0
  ASoC: SOF: Intel: hda: Allow trace DMA in S0 when DSP is in D0I3 for
    debug

 sound/soc/sof/core.c          |   4 +-
 sound/soc/sof/intel/cnl.c     |  37 ++++-
 sound/soc/sof/intel/hda-dsp.c | 289 +++++++++++++++++++++++++++++++---
 sound/soc/sof/intel/hda.c     |   5 +
 sound/soc/sof/intel/hda.h     |  21 ++-
 sound/soc/sof/ipc.c           |  29 +++-
 sound/soc/sof/ops.h           |  16 +-
 sound/soc/sof/pcm.c           |   2 +-
 sound/soc/sof/pm.c            | 176 ++++++++-------------
 sound/soc/sof/sof-audio.c     |  42 ++++-
 sound/soc/sof/sof-audio.h     |   3 +-
 sound/soc/sof/sof-priv.h      |  43 +++--
 12 files changed, 502 insertions(+), 165 deletions(-)

-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 1/9] ASoC: SOF: Do not reset hw_params for streams that ignored suspend

[Pierre-Louis Bossart]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Setting the prepared flag to false marks the streams for the
hw_params to be reset upon resuming. In the case of
the D0i3-compatible streams that ignored suspend to
keep the pipeline active in the DSP during suspend,
this should not be done.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/sof-audio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index 0d8f65b9ae25..345e42ee4783 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -39,6 +39,13 @@ int sof_set_hw_params_upon_resume(struct device *dev)
 	 */
 	list_for_each_entry(spcm, &sdev->pcm_list, list) {
 		for (dir = 0; dir <= SNDRV_PCM_STREAM_CAPTURE; dir++) {
+			/*
+			 * do not reset hw_params upon resume for streams that
+			 * were kept running during suspend
+			 */
+			if (spcm->stream[dir].suspend_ignored)
+				continue;
+
 			substream = spcm->stream[dir].substream;
 			if (!substream || !substream->runtime)
 				continue;
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] Applied "ASoC: SOF: Do not reset hw_params for streams that ignored suspend" to the asoc tree

[Mark Brown]

The patch

   ASoC: SOF: Do not reset hw_params for streams that ignored suspend

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.7

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 3f06501ea4d2d8add203e66d225274f106cb4029 Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Date: Wed, 29 Jan 2020 16:07:18 -0600
Subject: [PATCH] ASoC: SOF: Do not reset hw_params for streams that ignored
 suspend

Setting the prepared flag to false marks the streams for the
hw_params to be reset upon resuming. In the case of
the D0i3-compatible streams that ignored suspend to
keep the pipeline active in the DSP during suspend,
this should not be done.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200129220726.31792-2-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sof/sof-audio.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index 0d8f65b9ae25..345e42ee4783 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -39,6 +39,13 @@ int sof_set_hw_params_upon_resume(struct device *dev)
 	 */
 	list_for_each_entry(spcm, &sdev->pcm_list, list) {
 		for (dir = 0; dir <= SNDRV_PCM_STREAM_CAPTURE; dir++) {
+			/*
+			 * do not reset hw_params upon resume for streams that
+			 * were kept running during suspend
+			 */
+			if (spcm->stream[dir].suspend_ignored)
+				continue;
+
 			substream = spcm->stream[dir].substream;
 			if (!substream || !substream->runtime)
 				continue;
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 2/9] ASoC: SOF: pm: Unify suspend/resume routines

[Pierre-Louis Bossart]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Unify the suspend/resume routines for both the D0I3/D3
DSP targets in sof_suspend()/sof_resume().

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/pm.c | 91 ++++++++++++++++++++--------------------------
 1 file changed, 39 insertions(+), 52 deletions(-)

diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c
index a0cde053b61a..5b186bceedb9 100644
--- a/sound/soc/sof/pm.c
+++ b/sound/soc/sof/pm.c
@@ -50,6 +50,7 @@ static void sof_cache_debugfs(struct snd_sof_dev *sdev)
 static int sof_resume(struct device *dev, bool runtime_resume)
 {
 	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
+	enum sof_d0_substate old_d0_substate = sdev->d0_substate;
 	int ret;
 
 	/* do nothing if dsp resume callbacks are not set */
@@ -60,6 +61,17 @@ static int sof_resume(struct device *dev, bool runtime_resume)
 	if (sdev->first_boot)
 		return 0;
 
+	/* resume from D0I3 */
+	if (!runtime_resume && old_d0_substate == SOF_DSP_D0I3) {
+		ret = snd_sof_set_d0_substate(sdev, SOF_DSP_D0I0);
+		if (ret < 0 && ret != -ENOTSUPP) {
+			dev_err(sdev->dev,
+				"error: failed to resume from D0I3 %d\n",
+				ret);
+			return ret;
+		}
+	}
+
 	/*
 	 * if the runtime_resume flag is set, call the runtime_resume routine
 	 * or else call the system resume routine
@@ -74,6 +86,10 @@ static int sof_resume(struct device *dev, bool runtime_resume)
 		return ret;
 	}
 
+	/* Nothing further to do if resuming from D0I3 */
+	if (!runtime_resume && old_d0_substate == SOF_DSP_D0I3)
+		return 0;
+
 	sdev->fw_state = SOF_FW_BOOT_PREPARE;
 
 	/* load the firmware */
@@ -140,10 +156,7 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
 		return 0;
 
 	if (sdev->fw_state != SOF_FW_BOOT_COMPLETE)
-		goto power_down;
-
-	/* release trace */
-	snd_sof_release_trace(sdev);
+		goto suspend;
 
 	/* set restore_stream for all streams during system suspend */
 	if (!runtime_suspend) {
@@ -156,6 +169,22 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
 		}
 	}
 
+	if (snd_sof_dsp_d0i3_on_suspend(sdev)) {
+		/* suspend to D0i3 */
+		ret = snd_sof_set_d0_substate(sdev, SOF_DSP_D0I3);
+		if (ret < 0) {
+			dev_err(sdev->dev, "error: failed to enter D0I3, %d\n",
+				ret);
+			return ret;
+		}
+
+		/* Skip to platform-specific suspend if DSP is entering D0I3 */
+		goto suspend;
+	}
+
+	/* release trace */
+	snd_sof_release_trace(sdev);
+
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_ENABLE_DEBUGFS_CACHE)
 	/* cache debugfs contents during runtime suspend */
 	if (runtime_suspend)
@@ -179,13 +208,13 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
 			 ret);
 	}
 
-power_down:
+suspend:
 
 	/* return if the DSP was not probed successfully */
 	if (sdev->fw_state == SOF_FW_BOOT_NOT_STARTED)
 		return 0;
 
-	/* power down all DSP cores */
+	/* platform-specific suspend */
 	if (runtime_suspend)
 		ret = snd_sof_dsp_runtime_suspend(sdev);
 	else
@@ -195,6 +224,10 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
 			"error: failed to power down DSP during suspend %d\n",
 			ret);
 
+	/* Do not reset FW state if DSP is in D0I3 */
+	if (sdev->d0_substate == SOF_DSP_D0I3)
+		return ret;
+
 	/* reset FW state */
 	sdev->fw_state = SOF_FW_BOOT_NOT_STARTED;
 
@@ -275,58 +308,12 @@ EXPORT_SYMBOL(snd_sof_set_d0_substate);
 
 int snd_sof_resume(struct device *dev)
 {
-	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
-	int ret;
-
-	if (snd_sof_dsp_d0i3_on_suspend(sdev)) {
-		/* resume from D0I3 */
-		dev_dbg(sdev->dev, "DSP will exit from D0i3...\n");
-		ret = snd_sof_set_d0_substate(sdev, SOF_DSP_D0I0);
-		if (ret == -ENOTSUPP) {
-			/* fallback to resume from D3 */
-			dev_dbg(sdev->dev, "D0i3 not supported, fall back to resume from D3...\n");
-			goto d3_resume;
-		} else if (ret < 0) {
-			dev_err(sdev->dev, "error: failed to exit from D0I3 %d\n",
-				ret);
-			return ret;
-		}
-
-		/* platform-specific resume from D0i3 */
-		return snd_sof_dsp_resume(sdev);
-	}
-
-d3_resume:
-	/* resume from D3 */
 	return sof_resume(dev, false);
 }
 EXPORT_SYMBOL(snd_sof_resume);
 
 int snd_sof_suspend(struct device *dev)
 {
-	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
-	int ret;
-
-	if (snd_sof_dsp_d0i3_on_suspend(sdev)) {
-		/* suspend to D0i3 */
-		dev_dbg(sdev->dev, "DSP is trying to enter D0i3...\n");
-		ret = snd_sof_set_d0_substate(sdev, SOF_DSP_D0I3);
-		if (ret == -ENOTSUPP) {
-			/* fallback to D3 suspend */
-			dev_dbg(sdev->dev, "D0i3 not supported, fall back to D3...\n");
-			goto d3_suspend;
-		} else if (ret < 0) {
-			dev_err(sdev->dev, "error: failed to enter D0I3, %d\n",
-				ret);
-			return ret;
-		}
-
-		/* platform-specific suspend to D0i3 */
-		return snd_sof_dsp_suspend(sdev);
-	}
-
-d3_suspend:
-	/* suspend to D3 */
 	return sof_suspend(dev, false);
 }
 EXPORT_SYMBOL(snd_sof_suspend);
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] Applied "ASoC: SOF: pm: Unify suspend/resume routines" to the asoc tree

[Mark Brown]

The patch

   ASoC: SOF: pm: Unify suspend/resume routines

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.7

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 fb9a81192d44ae9f334b1d88651dec427fa751d8 Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Date: Wed, 29 Jan 2020 16:07:19 -0600
Subject: [PATCH] ASoC: SOF: pm: Unify suspend/resume routines

Unify the suspend/resume routines for both the D0I3/D3
DSP targets in sof_suspend()/sof_resume().

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200129220726.31792-3-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sof/pm.c | 91 ++++++++++++++++++++--------------------------
 1 file changed, 39 insertions(+), 52 deletions(-)

diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c
index a0cde053b61a..5b186bceedb9 100644
--- a/sound/soc/sof/pm.c
+++ b/sound/soc/sof/pm.c
@@ -50,6 +50,7 @@ static void sof_cache_debugfs(struct snd_sof_dev *sdev)
 static int sof_resume(struct device *dev, bool runtime_resume)
 {
 	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
+	enum sof_d0_substate old_d0_substate = sdev->d0_substate;
 	int ret;
 
 	/* do nothing if dsp resume callbacks are not set */
@@ -60,6 +61,17 @@ static int sof_resume(struct device *dev, bool runtime_resume)
 	if (sdev->first_boot)
 		return 0;
 
+	/* resume from D0I3 */
+	if (!runtime_resume && old_d0_substate == SOF_DSP_D0I3) {
+		ret = snd_sof_set_d0_substate(sdev, SOF_DSP_D0I0);
+		if (ret < 0 && ret != -ENOTSUPP) {
+			dev_err(sdev->dev,
+				"error: failed to resume from D0I3 %d\n",
+				ret);
+			return ret;
+		}
+	}
+
 	/*
 	 * if the runtime_resume flag is set, call the runtime_resume routine
 	 * or else call the system resume routine
@@ -74,6 +86,10 @@ static int sof_resume(struct device *dev, bool runtime_resume)
 		return ret;
 	}
 
+	/* Nothing further to do if resuming from D0I3 */
+	if (!runtime_resume && old_d0_substate == SOF_DSP_D0I3)
+		return 0;
+
 	sdev->fw_state = SOF_FW_BOOT_PREPARE;
 
 	/* load the firmware */
@@ -140,10 +156,7 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
 		return 0;
 
 	if (sdev->fw_state != SOF_FW_BOOT_COMPLETE)
-		goto power_down;
-
-	/* release trace */
-	snd_sof_release_trace(sdev);
+		goto suspend;
 
 	/* set restore_stream for all streams during system suspend */
 	if (!runtime_suspend) {
@@ -156,6 +169,22 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
 		}
 	}
 
+	if (snd_sof_dsp_d0i3_on_suspend(sdev)) {
+		/* suspend to D0i3 */
+		ret = snd_sof_set_d0_substate(sdev, SOF_DSP_D0I3);
+		if (ret < 0) {
+			dev_err(sdev->dev, "error: failed to enter D0I3, %d\n",
+				ret);
+			return ret;
+		}
+
+		/* Skip to platform-specific suspend if DSP is entering D0I3 */
+		goto suspend;
+	}
+
+	/* release trace */
+	snd_sof_release_trace(sdev);
+
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_ENABLE_DEBUGFS_CACHE)
 	/* cache debugfs contents during runtime suspend */
 	if (runtime_suspend)
@@ -179,13 +208,13 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
 			 ret);
 	}
 
-power_down:
+suspend:
 
 	/* return if the DSP was not probed successfully */
 	if (sdev->fw_state == SOF_FW_BOOT_NOT_STARTED)
 		return 0;
 
-	/* power down all DSP cores */
+	/* platform-specific suspend */
 	if (runtime_suspend)
 		ret = snd_sof_dsp_runtime_suspend(sdev);
 	else
@@ -195,6 +224,10 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
 			"error: failed to power down DSP during suspend %d\n",
 			ret);
 
+	/* Do not reset FW state if DSP is in D0I3 */
+	if (sdev->d0_substate == SOF_DSP_D0I3)
+		return ret;
+
 	/* reset FW state */
 	sdev->fw_state = SOF_FW_BOOT_NOT_STARTED;
 
@@ -275,58 +308,12 @@ EXPORT_SYMBOL(snd_sof_set_d0_substate);
 
 int snd_sof_resume(struct device *dev)
 {
-	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
-	int ret;
-
-	if (snd_sof_dsp_d0i3_on_suspend(sdev)) {
-		/* resume from D0I3 */
-		dev_dbg(sdev->dev, "DSP will exit from D0i3...\n");
-		ret = snd_sof_set_d0_substate(sdev, SOF_DSP_D0I0);
-		if (ret == -ENOTSUPP) {
-			/* fallback to resume from D3 */
-			dev_dbg(sdev->dev, "D0i3 not supported, fall back to resume from D3...\n");
-			goto d3_resume;
-		} else if (ret < 0) {
-			dev_err(sdev->dev, "error: failed to exit from D0I3 %d\n",
-				ret);
-			return ret;
-		}
-
-		/* platform-specific resume from D0i3 */
-		return snd_sof_dsp_resume(sdev);
-	}
-
-d3_resume:
-	/* resume from D3 */
 	return sof_resume(dev, false);
 }
 EXPORT_SYMBOL(snd_sof_resume);
 
 int snd_sof_suspend(struct device *dev)
 {
-	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
-	int ret;
-
-	if (snd_sof_dsp_d0i3_on_suspend(sdev)) {
-		/* suspend to D0i3 */
-		dev_dbg(sdev->dev, "DSP is trying to enter D0i3...\n");
-		ret = snd_sof_set_d0_substate(sdev, SOF_DSP_D0I3);
-		if (ret == -ENOTSUPP) {
-			/* fallback to D3 suspend */
-			dev_dbg(sdev->dev, "D0i3 not supported, fall back to D3...\n");
-			goto d3_suspend;
-		} else if (ret < 0) {
-			dev_err(sdev->dev, "error: failed to enter D0I3, %d\n",
-				ret);
-			return ret;
-		}
-
-		/* platform-specific suspend to D0i3 */
-		return snd_sof_dsp_suspend(sdev);
-	}
-
-d3_suspend:
-	/* suspend to D3 */
 	return sof_suspend(dev, false);
 }
 EXPORT_SYMBOL(snd_sof_suspend);
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 3/9] ASoC: SOF: Add system_suspend_target field to struct snd_sof_dev

[Pierre-Louis Bossart]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Add the system_suspend_target field to struct snd_sof_dev
to track the intended system suspend power target. This will
be used as one of the criteria for determining the
final DSP power state.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/hda-dsp.c |  4 ++--
 sound/soc/sof/pcm.c           |  2 +-
 sound/soc/sof/pm.c            |  9 ++++++---
 sound/soc/sof/sof-priv.h      | 12 ++++++++++--
 4 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index 4a4d318f97ff..fddf2c48904f 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -481,7 +481,7 @@ int hda_dsp_resume(struct snd_sof_dev *sdev)
 	struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
 	struct pci_dev *pci = to_pci_dev(sdev->dev);
 
-	if (sdev->s0_suspend) {
+	if (sdev->system_suspend_target == SOF_SUSPEND_S0IX) {
 		/* restore L1SEN bit */
 		if (hda->l1_support_changed)
 			snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
@@ -530,7 +530,7 @@ int hda_dsp_suspend(struct snd_sof_dev *sdev)
 	struct pci_dev *pci = to_pci_dev(sdev->dev);
 	int ret;
 
-	if (sdev->s0_suspend) {
+	if (sdev->system_suspend_target == SOF_SUSPEND_S0IX) {
 		/* enable L1SEN to make sure the system can enter S0Ix */
 		hda->l1_support_changed =
 			snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c
index 29435ba2d329..db3df02c7398 100644
--- a/sound/soc/sof/pcm.c
+++ b/sound/soc/sof/pcm.c
@@ -372,7 +372,7 @@ static int sof_pcm_trigger(struct snd_soc_component *component,
 		stream.hdr.cmd |= SOF_IPC_STREAM_TRIG_START;
 		break;
 	case SNDRV_PCM_TRIGGER_SUSPEND:
-		if (sdev->s0_suspend &&
+		if (sdev->system_suspend_target == SOF_SUSPEND_S0IX &&
 		    spcm->stream[substream->stream].d0i3_compatible) {
 			/*
 			 * trap the event, not sending trigger stop to
diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c
index 5b186bceedb9..c86ac1e84bd7 100644
--- a/sound/soc/sof/pm.c
+++ b/sound/soc/sof/pm.c
@@ -323,10 +323,13 @@ int snd_sof_prepare(struct device *dev)
 	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
 
 #if defined(CONFIG_ACPI)
-	sdev->s0_suspend = acpi_target_system_state() == ACPI_STATE_S0;
+	if (acpi_target_system_state() == ACPI_STATE_S0)
+		sdev->system_suspend_target = SOF_SUSPEND_S0IX;
+	else
+		sdev->system_suspend_target = SOF_SUSPEND_S3;
 #else
 	/* will suspend to S3 by default */
-	sdev->s0_suspend = false;
+	sdev->system_suspend_target = SOF_SUSPEND_S3;
 #endif
 
 	return 0;
@@ -337,6 +340,6 @@ void snd_sof_complete(struct device *dev)
 {
 	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
 
-	sdev->s0_suspend = false;
+	sdev->system_suspend_target = SOF_SUSPEND_NONE;
 }
 EXPORT_SYMBOL(snd_sof_complete);
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index bc2337cf1142..1839cc51957d 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -60,6 +60,13 @@ enum sof_d0_substate {
 	SOF_DSP_D0I3,		/* DSP D0i3(low power) substate*/
 };
 
+/* System suspend target state */
+enum sof_system_suspend_state {
+	SOF_SUSPEND_NONE = 0,
+	SOF_SUSPEND_S0IX,
+	SOF_SUSPEND_S3,
+};
+
 struct snd_sof_dev;
 struct snd_sof_ipc_msg;
 struct snd_sof_ipc;
@@ -325,8 +332,9 @@ struct snd_sof_dev {
 
 	/* power states related */
 	enum sof_d0_substate d0_substate;
-	/* flag to track if the intended power target of suspend is S0ix */
-	bool s0_suspend;
+
+	/* Intended power target of system suspend */
+	enum sof_system_suspend_state system_suspend_target;
 
 	/* DSP firmware boot */
 	wait_queue_head_t boot_wait;
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] Applied "ASoC: SOF: Add system_suspend_target field to struct snd_sof_dev" to the asoc tree

[Mark Brown]

The patch

   ASoC: SOF: Add system_suspend_target field to struct snd_sof_dev

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.7

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 043ae13bbd558971ce91596ce09c03d6ef6a4a0c Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Date: Wed, 29 Jan 2020 16:07:20 -0600
Subject: [PATCH] ASoC: SOF: Add system_suspend_target field to struct
 snd_sof_dev

Add the system_suspend_target field to struct snd_sof_dev
to track the intended system suspend power target. This will
be used as one of the criteria for determining the
final DSP power state.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200129220726.31792-4-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sof/intel/hda-dsp.c |  4 ++--
 sound/soc/sof/pcm.c           |  2 +-
 sound/soc/sof/pm.c            |  9 ++++++---
 sound/soc/sof/sof-priv.h      | 12 ++++++++++--
 4 files changed, 19 insertions(+), 8 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index 4a4d318f97ff..fddf2c48904f 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -481,7 +481,7 @@ int hda_dsp_resume(struct snd_sof_dev *sdev)
 	struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
 	struct pci_dev *pci = to_pci_dev(sdev->dev);
 
-	if (sdev->s0_suspend) {
+	if (sdev->system_suspend_target == SOF_SUSPEND_S0IX) {
 		/* restore L1SEN bit */
 		if (hda->l1_support_changed)
 			snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
@@ -530,7 +530,7 @@ int hda_dsp_suspend(struct snd_sof_dev *sdev)
 	struct pci_dev *pci = to_pci_dev(sdev->dev);
 	int ret;
 
-	if (sdev->s0_suspend) {
+	if (sdev->system_suspend_target == SOF_SUSPEND_S0IX) {
 		/* enable L1SEN to make sure the system can enter S0Ix */
 		hda->l1_support_changed =
 			snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
diff --git a/sound/soc/sof/pcm.c b/sound/soc/sof/pcm.c
index 29435ba2d329..db3df02c7398 100644
--- a/sound/soc/sof/pcm.c
+++ b/sound/soc/sof/pcm.c
@@ -372,7 +372,7 @@ static int sof_pcm_trigger(struct snd_soc_component *component,
 		stream.hdr.cmd |= SOF_IPC_STREAM_TRIG_START;
 		break;
 	case SNDRV_PCM_TRIGGER_SUSPEND:
-		if (sdev->s0_suspend &&
+		if (sdev->system_suspend_target == SOF_SUSPEND_S0IX &&
 		    spcm->stream[substream->stream].d0i3_compatible) {
 			/*
 			 * trap the event, not sending trigger stop to
diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c
index 5b186bceedb9..c86ac1e84bd7 100644
--- a/sound/soc/sof/pm.c
+++ b/sound/soc/sof/pm.c
@@ -323,10 +323,13 @@ int snd_sof_prepare(struct device *dev)
 	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
 
 #if defined(CONFIG_ACPI)
-	sdev->s0_suspend = acpi_target_system_state() == ACPI_STATE_S0;
+	if (acpi_target_system_state() == ACPI_STATE_S0)
+		sdev->system_suspend_target = SOF_SUSPEND_S0IX;
+	else
+		sdev->system_suspend_target = SOF_SUSPEND_S3;
 #else
 	/* will suspend to S3 by default */
-	sdev->s0_suspend = false;
+	sdev->system_suspend_target = SOF_SUSPEND_S3;
 #endif
 
 	return 0;
@@ -337,6 +340,6 @@ void snd_sof_complete(struct device *dev)
 {
 	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
 
-	sdev->s0_suspend = false;
+	sdev->system_suspend_target = SOF_SUSPEND_NONE;
 }
 EXPORT_SYMBOL(snd_sof_complete);
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index bc2337cf1142..1839cc51957d 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -60,6 +60,13 @@ enum sof_d0_substate {
 	SOF_DSP_D0I3,		/* DSP D0i3(low power) substate*/
 };
 
+/* System suspend target state */
+enum sof_system_suspend_state {
+	SOF_SUSPEND_NONE = 0,
+	SOF_SUSPEND_S0IX,
+	SOF_SUSPEND_S3,
+};
+
 struct snd_sof_dev;
 struct snd_sof_ipc_msg;
 struct snd_sof_ipc;
@@ -325,8 +332,9 @@ struct snd_sof_dev {
 
 	/* power states related */
 	enum sof_d0_substate d0_substate;
-	/* flag to track if the intended power target of suspend is S0ix */
-	bool s0_suspend;
+
+	/* Intended power target of system suspend */
+	enum sof_system_suspend_state system_suspend_target;
 
 	/* DSP firmware boot */
 	wait_queue_head_t boot_wait;
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 4/9] ASoC: SOF: pm: Introduce DSP power states

[Pierre-Louis Bossart]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Add a new enum sof_dsp_power_states for all the possible
the DSP device states. The SOF driver currently handles
only the D0 and D3 states and support for other states
will be added later as needed.

Also, add a helper to determine the target DSP power state
based on the system suspend target.
The snd_sof_dsp_d0i3_on_suspend() function is renamed to
snd_sof_stream_suspend_ignored() to be more indicative
of what it does and it used to determine the target
DSP state during system suspend.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/pm.c        | 38 +++++++++++++++++++++++++++++++++++++-
 sound/soc/sof/sof-audio.c |  2 +-
 sound/soc/sof/sof-audio.h |  2 +-
 sound/soc/sof/sof-priv.h  | 10 ++++++++++
 4 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c
index c86ac1e84bd7..bec25cb6beec 100644
--- a/sound/soc/sof/pm.c
+++ b/sound/soc/sof/pm.c
@@ -12,6 +12,42 @@
 #include "sof-priv.h"
 #include "sof-audio.h"
 
+/*
+ * Helper function to determine the target DSP state during
+ * system suspend. This function only cares about the device
+ * D-states. Platform-specific substates, if any, should be
+ * handled by the platform-specific parts.
+ */
+static u32 snd_sof_dsp_power_target(struct snd_sof_dev *sdev)
+{
+	u32 target_dsp_state;
+
+	switch (sdev->system_suspend_target) {
+	case SOF_SUSPEND_S3:
+		/* DSP should be in D3 if the system is suspending to S3 */
+		target_dsp_state = SOF_DSP_PM_D3;
+		break;
+	case SOF_SUSPEND_S0IX:
+		/*
+		 * Currently, the only criterion for retaining the DSP in D0
+		 * is that there are streams that ignored the suspend trigger.
+		 * Additional criteria such Soundwire clock-stop mode and
+		 * device suspend latency considerations will be added later.
+		 */
+		if (snd_sof_stream_suspend_ignored(sdev))
+			target_dsp_state = SOF_DSP_PM_D0;
+		else
+			target_dsp_state = SOF_DSP_PM_D3;
+		break;
+	default:
+		/* This case would be during runtime suspend */
+		target_dsp_state = SOF_DSP_PM_D3;
+		break;
+	}
+
+	return target_dsp_state;
+}
+
 static int sof_send_pm_ctx_ipc(struct snd_sof_dev *sdev, int cmd)
 {
 	struct sof_ipc_pm_ctx pm_ctx;
@@ -169,7 +205,7 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
 		}
 	}
 
-	if (snd_sof_dsp_d0i3_on_suspend(sdev)) {
+	if (snd_sof_dsp_power_target(sdev) == SOF_DSP_PM_D0) {
 		/* suspend to D0i3 */
 		ret = snd_sof_set_d0_substate(sdev, SOF_DSP_D0I3);
 		if (ret < 0) {
diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index 345e42ee4783..d16571ca129c 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -11,7 +11,7 @@
 #include "sof-audio.h"
 #include "ops.h"
 
-bool snd_sof_dsp_d0i3_on_suspend(struct snd_sof_dev *sdev)
+bool snd_sof_stream_suspend_ignored(struct snd_sof_dev *sdev)
 {
 	struct snd_sof_pcm *spcm;
 
diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h
index a62fb2da6a6e..a2702afbd9a1 100644
--- a/sound/soc/sof/sof-audio.h
+++ b/sound/soc/sof/sof-audio.h
@@ -202,7 +202,7 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol,
 /* PM */
 int sof_restore_pipelines(struct device *dev);
 int sof_set_hw_params_upon_resume(struct device *dev);
-bool snd_sof_dsp_d0i3_on_suspend(struct snd_sof_dev *sdev);
+bool snd_sof_stream_suspend_ignored(struct snd_sof_dev *sdev);
 
 /* Machine driver enumeration */
 int sof_machine_register(struct snd_sof_dev *sdev, void *pdata);
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index 1839cc51957d..a7c6109acd98 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -54,6 +54,16 @@ extern int sof_core_debug;
 	(IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_ENABLE_DEBUGFS_CACHE) || \
 	 IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST))
 
+/* DSP power state */
+enum sof_dsp_power_states {
+	SOF_DSP_PM_D0,
+	SOF_DSP_PM_D1,
+	SOF_DSP_PM_D2,
+	SOF_DSP_PM_D3_HOT,
+	SOF_DSP_PM_D3,
+	SOF_DSP_PM_D3_COLD,
+};
+
 /* DSP D0ix sub-state */
 enum sof_d0_substate {
 	SOF_DSP_D0I0 = 0,	/* DSP default D0 substate */
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] Applied "ASoC: SOF: pm: Introduce DSP power states" to the asoc tree

[Mark Brown]

The patch

   ASoC: SOF: pm: Introduce DSP power states

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.7

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 700d167739a099cdf12ed15c25fec7f4cb563d42 Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Date: Wed, 29 Jan 2020 16:07:21 -0600
Subject: [PATCH] ASoC: SOF: pm: Introduce DSP power states

Add a new enum sof_dsp_power_states for all the possible
the DSP device states. The SOF driver currently handles
only the D0 and D3 states and support for other states
will be added later as needed.

Also, add a helper to determine the target DSP power state
based on the system suspend target.
The snd_sof_dsp_d0i3_on_suspend() function is renamed to
snd_sof_stream_suspend_ignored() to be more indicative
of what it does and it used to determine the target
DSP state during system suspend.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200129220726.31792-5-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sof/pm.c        | 38 +++++++++++++++++++++++++++++++++++++-
 sound/soc/sof/sof-audio.c |  2 +-
 sound/soc/sof/sof-audio.h |  2 +-
 sound/soc/sof/sof-priv.h  | 10 ++++++++++
 4 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c
index c86ac1e84bd7..bec25cb6beec 100644
--- a/sound/soc/sof/pm.c
+++ b/sound/soc/sof/pm.c
@@ -12,6 +12,42 @@
 #include "sof-priv.h"
 #include "sof-audio.h"
 
+/*
+ * Helper function to determine the target DSP state during
+ * system suspend. This function only cares about the device
+ * D-states. Platform-specific substates, if any, should be
+ * handled by the platform-specific parts.
+ */
+static u32 snd_sof_dsp_power_target(struct snd_sof_dev *sdev)
+{
+	u32 target_dsp_state;
+
+	switch (sdev->system_suspend_target) {
+	case SOF_SUSPEND_S3:
+		/* DSP should be in D3 if the system is suspending to S3 */
+		target_dsp_state = SOF_DSP_PM_D3;
+		break;
+	case SOF_SUSPEND_S0IX:
+		/*
+		 * Currently, the only criterion for retaining the DSP in D0
+		 * is that there are streams that ignored the suspend trigger.
+		 * Additional criteria such Soundwire clock-stop mode and
+		 * device suspend latency considerations will be added later.
+		 */
+		if (snd_sof_stream_suspend_ignored(sdev))
+			target_dsp_state = SOF_DSP_PM_D0;
+		else
+			target_dsp_state = SOF_DSP_PM_D3;
+		break;
+	default:
+		/* This case would be during runtime suspend */
+		target_dsp_state = SOF_DSP_PM_D3;
+		break;
+	}
+
+	return target_dsp_state;
+}
+
 static int sof_send_pm_ctx_ipc(struct snd_sof_dev *sdev, int cmd)
 {
 	struct sof_ipc_pm_ctx pm_ctx;
@@ -169,7 +205,7 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
 		}
 	}
 
-	if (snd_sof_dsp_d0i3_on_suspend(sdev)) {
+	if (snd_sof_dsp_power_target(sdev) == SOF_DSP_PM_D0) {
 		/* suspend to D0i3 */
 		ret = snd_sof_set_d0_substate(sdev, SOF_DSP_D0I3);
 		if (ret < 0) {
diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index 345e42ee4783..d16571ca129c 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -11,7 +11,7 @@
 #include "sof-audio.h"
 #include "ops.h"
 
-bool snd_sof_dsp_d0i3_on_suspend(struct snd_sof_dev *sdev)
+bool snd_sof_stream_suspend_ignored(struct snd_sof_dev *sdev)
 {
 	struct snd_sof_pcm *spcm;
 
diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h
index a62fb2da6a6e..a2702afbd9a1 100644
--- a/sound/soc/sof/sof-audio.h
+++ b/sound/soc/sof/sof-audio.h
@@ -202,7 +202,7 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol,
 /* PM */
 int sof_restore_pipelines(struct device *dev);
 int sof_set_hw_params_upon_resume(struct device *dev);
-bool snd_sof_dsp_d0i3_on_suspend(struct snd_sof_dev *sdev);
+bool snd_sof_stream_suspend_ignored(struct snd_sof_dev *sdev);
 
 /* Machine driver enumeration */
 int sof_machine_register(struct snd_sof_dev *sdev, void *pdata);
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index 1839cc51957d..a7c6109acd98 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -54,6 +54,16 @@ extern int sof_core_debug;
 	(IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_ENABLE_DEBUGFS_CACHE) || \
 	 IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG_IPC_FLOOD_TEST))
 
+/* DSP power state */
+enum sof_dsp_power_states {
+	SOF_DSP_PM_D0,
+	SOF_DSP_PM_D1,
+	SOF_DSP_PM_D2,
+	SOF_DSP_PM_D3_HOT,
+	SOF_DSP_PM_D3,
+	SOF_DSP_PM_D3_COLD,
+};
+
 /* DSP D0ix sub-state */
 enum sof_d0_substate {
 	SOF_DSP_D0I0 = 0,	/* DSP default D0 substate */
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 5/9] ASoC: SOF: Move DSP power state transitions to platform-specific ops

[Pierre-Louis Bossart]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

The DSP device substates such as D0I0/D0I3
are platform-specific. Therefore, the d0_substate
field of struct snd_sof_dev is replaced
with the dsp_power_state field which represents the current
state of the DSP. This field holds both the device state
and the platform-specific substate values.

With the DSP device substates being platform-specific,
the DSP power state transitions need to be performed in
the platform-specific suspend/resume ops as well.

In order to achieve this, the ops signature has to be
modified to pass the target device state as an
argument. The target substate will be determined by
the platform-specific ops before performing the transition.
For example, in the case of the system suspending to S0IX,
the top-level SOF device suspend callback needs to
only determine if the DSP will be entering
D3 or remain in D0. The target substate in case the device
needs to remain in D0 (D0I0 or D0I3) will be determined
by the platform-specific suspend op.

With the addition of the extended set of power states for the DSP,
the set_power_state op for HDA platforms has to be extended
to handle only the appropriate state transitions. So, the
implementation for the Intel HDA platforms is also modified.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/core.c          |   4 +-
 sound/soc/sof/intel/hda-dsp.c | 223 +++++++++++++++++++++++++++++++---
 sound/soc/sof/intel/hda.h     |  10 +-
 sound/soc/sof/ops.h           |  16 +--
 sound/soc/sof/pm.c            |  92 ++------------
 sound/soc/sof/sof-priv.h      |  18 ++-
 6 files changed, 242 insertions(+), 121 deletions(-)

diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c
index 34cefbaf2d2a..1d07450aff77 100644
--- a/sound/soc/sof/core.c
+++ b/sound/soc/sof/core.c
@@ -286,8 +286,8 @@ int snd_sof_device_probe(struct device *dev, struct snd_sof_pdata *plat_data)
 	/* initialize sof device */
 	sdev->dev = dev;
 
-	/* initialize default D0 sub-state */
-	sdev->d0_substate = SOF_DSP_D0I0;
+	/* initialize default DSP power state */
+	sdev->dsp_power_state.state = SOF_DSP_PM_D0;
 
 	sdev->pdata = plat_data;
 	sdev->first_boot = true;
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index fddf2c48904f..8c00e128a7b0 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -338,13 +338,10 @@ static int hda_dsp_send_pm_gate_ipc(struct snd_sof_dev *sdev, u32 flags)
 				  sizeof(pm_gate), &reply, sizeof(reply));
 }
 
-int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
-			    enum sof_d0_substate d0_substate)
+static int hda_dsp_update_d0i3c_register(struct snd_sof_dev *sdev, u8 value)
 {
 	struct hdac_bus *bus = sof_to_bus(sdev);
-	u32 flags;
 	int ret;
-	u8 value;
 
 	/* Write to D0I3C after Command-In-Progress bit is cleared */
 	ret = hda_dsp_wait_d0i3c_done(sdev);
@@ -354,7 +351,6 @@ int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
 	}
 
 	/* Update D0I3C register */
-	value = d0_substate == SOF_DSP_D0I3 ? SOF_HDA_VS_D0I3C_I3 : 0;
 	snd_hdac_chip_updateb(bus, VS_D0I3C, SOF_HDA_VS_D0I3C_I3, value);
 
 	/* Wait for cmd in progress to be cleared before exiting the function */
@@ -367,20 +363,160 @@ int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
 	dev_vdbg(bus->dev, "D0I3C updated, register = 0x%x\n",
 		 snd_hdac_chip_readb(bus, VS_D0I3C));
 
-	if (d0_substate == SOF_DSP_D0I0)
-		flags = HDA_PM_PPG;/* prevent power gating in D0 */
-	else
-		flags = HDA_PM_NO_DMA_TRACE;/* disable DMA trace in D0I3*/
+	return 0;
+}
 
-	/* sending pm_gate IPC */
-	ret = hda_dsp_send_pm_gate_ipc(sdev, flags);
+static int hda_dsp_set_D0_state(struct snd_sof_dev *sdev,
+				const struct sof_dsp_power_state *target_state)
+{
+	u32 flags = 0;
+	int ret;
+	u8 value = 0;
+
+	/*
+	 * Sanity check for illegal state transitions
+	 * The only allowed transitions are:
+	 * 1. D3 -> D0I0
+	 * 2. D0I0 -> D0I3
+	 * 3. D0I3 -> D0I0
+	 */
+	switch (sdev->dsp_power_state.state) {
+	case SOF_DSP_PM_D0:
+		/* Follow the sequence below for D0 substate transitions */
+		break;
+	case SOF_DSP_PM_D3:
+		/* Follow regular flow for D3 -> D0 transition */
+		return 0;
+	default:
+		dev_err(sdev->dev, "error: transition from %d to %d not allowed\n",
+			sdev->dsp_power_state.state, target_state->state);
+		return -EINVAL;
+	}
+
+	/* Set flags and register value for D0 target substate */
+	if (target_state->substate == SOF_HDA_DSP_PM_D0I3) {
+		value = SOF_HDA_VS_D0I3C_I3;
+
+		/* disable DMA trace in D0I3 */
+		flags = HDA_PM_NO_DMA_TRACE;
+	} else {
+		/* prevent power gating in D0I0 */
+		flags = HDA_PM_PPG;
+	}
+
+	/* update D0I3C register */
+	ret = hda_dsp_update_d0i3c_register(sdev, value);
 	if (ret < 0)
+		return ret;
+
+	/*
+	 * Notify the DSP of the state change.
+	 * If this IPC fails, revert the D0I3C register update in order
+	 * to prevent partial state change.
+	 */
+	ret = hda_dsp_send_pm_gate_ipc(sdev, flags);
+	if (ret < 0) {
 		dev_err(sdev->dev,
 			"error: PM_GATE ipc error %d\n", ret);
+		goto revert;
+	}
+
+	return ret;
+
+revert:
+	/* fallback to the previous register value */
+	value = value ? 0 : SOF_HDA_VS_D0I3C_I3;
+
+	/*
+	 * This can fail but return the IPC error to signal that
+	 * the state change failed.
+	 */
+	hda_dsp_update_d0i3c_register(sdev, value);
 
 	return ret;
 }
 
+/*
+ * All DSP power state transitions are initiated by the driver.
+ * If the requested state change fails, the error is simply returned.
+ * Further state transitions are attempted only when the set_power_save() op
+ * is called again either because of a new IPC sent to the DSP or
+ * during system suspend/resume.
+ */
+int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
+			    const struct sof_dsp_power_state *target_state)
+{
+	int ret = 0;
+
+	/* Nothing to do if the DSP is already in the requested state */
+	if (target_state->state == sdev->dsp_power_state.state &&
+	    target_state->substate == sdev->dsp_power_state.substate)
+		return 0;
+
+	switch (target_state->state) {
+	case SOF_DSP_PM_D0:
+		ret = hda_dsp_set_D0_state(sdev, target_state);
+		break;
+	case SOF_DSP_PM_D3:
+		/* The only allowed transition is: D0I0 -> D3 */
+		if (sdev->dsp_power_state.state == SOF_DSP_PM_D0 &&
+		    sdev->dsp_power_state.substate == SOF_HDA_DSP_PM_D0I0)
+			break;
+
+		dev_err(sdev->dev,
+			"error: transition from %d to %d not allowed\n",
+			sdev->dsp_power_state.state, target_state->state);
+		return -EINVAL;
+	default:
+		dev_err(sdev->dev, "error: target state unsupported %d\n",
+			target_state->state);
+		return -EINVAL;
+	}
+	if (ret < 0) {
+		dev_err(sdev->dev,
+			"failed to set requested target DSP state %d substate %d\n",
+			target_state->state, target_state->substate);
+		return ret;
+	}
+
+	sdev->dsp_power_state = *target_state;
+	dev_dbg(sdev->dev, "New DSP state %d substate %d\n",
+		target_state->state, target_state->substate);
+	return ret;
+}
+
+/*
+ * Audio DSP states may transform as below:-
+ *
+ *                                         D0I3 compatible stream
+ *     Runtime    +---------------------+   opened only, timeout
+ *     suspend    |                     +--------------------+
+ *   +------------+       D0(active)    |                    |
+ *   |            |                     <---------------+    |
+ *   |   +-------->                     |               |    |
+ *   |   |Runtime +--^--+---------^--+--+ The last      |    |
+ *   |   |resume     |  |         |  |    opened D0I3   |    |
+ *   |   |           |  |         |  |    compatible    |    |
+ *   |   |     resume|  |         |  |    stream closed |    |
+ *   |   |      from |  | D3      |  |                  |    |
+ *   |   |       D3  |  |suspend  |  | d0i3             |    |
+ *   |   |           |  |         |  |suspend           |    |
+ *   |   |           |  |         |  |                  |    |
+ *   |   |           |  |         |  |                  |    |
+ * +-v---+-----------+--v-------+ |  |           +------+----v----+
+ * |                            | |  +----------->                |
+ * |       D3 (suspended)       | |              |      D0I3      +-----+
+ * |                            | +--------------+                |     |
+ * |                            |  resume from   |                |     |
+ * +-------------------^--------+  d0i3 suspend  +----------------+     |
+ *                     |                                                |
+ *                     |                       D3 suspend               |
+ *                     +------------------------------------------------+
+ *
+ * d0i3_suspend = s0_suspend && D0I3 stream opened,
+ * D3 suspend = !d0i3_suspend,
+ */
+
 static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend)
 {
 	struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
@@ -480,8 +616,22 @@ int hda_dsp_resume(struct snd_sof_dev *sdev)
 {
 	struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
 	struct pci_dev *pci = to_pci_dev(sdev->dev);
+	const struct sof_dsp_power_state target_state = {
+		.state = SOF_DSP_PM_D0,
+		.substate = SOF_HDA_DSP_PM_D0I0,
+	};
+	int ret;
+
+	/* resume from D0I3 */
+	if (sdev->dsp_power_state.state == SOF_DSP_PM_D0) {
+		/* Set DSP power state */
+		ret = hda_dsp_set_power_state(sdev, &target_state);
+		if (ret < 0) {
+			dev_err(sdev->dev, "error: setting dsp state %d substate %d\n",
+				target_state.state, target_state.substate);
+			return ret;
+		}
 
-	if (sdev->system_suspend_target == SOF_SUSPEND_S0IX) {
 		/* restore L1SEN bit */
 		if (hda->l1_support_changed)
 			snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
@@ -495,13 +645,27 @@ int hda_dsp_resume(struct snd_sof_dev *sdev)
 	}
 
 	/* init hda controller. DSP cores will be powered up during fw boot */
-	return hda_resume(sdev, false);
+	ret = hda_resume(sdev, false);
+	if (ret < 0)
+		return ret;
+
+	hda_dsp_set_power_state(sdev, &target_state);
+	return ret;
 }
 
 int hda_dsp_runtime_resume(struct snd_sof_dev *sdev)
 {
+	const struct sof_dsp_power_state target_state = {
+		.state = SOF_DSP_PM_D0,
+	};
+	int ret;
+
 	/* init hda controller. DSP cores will be powered up during fw boot */
-	return hda_resume(sdev, true);
+	ret = hda_resume(sdev, true);
+	if (ret < 0)
+		return ret;
+
+	return hda_dsp_set_power_state(sdev, &target_state);
 }
 
 int hda_dsp_runtime_idle(struct snd_sof_dev *sdev)
@@ -519,18 +683,41 @@ int hda_dsp_runtime_idle(struct snd_sof_dev *sdev)
 
 int hda_dsp_runtime_suspend(struct snd_sof_dev *sdev)
 {
+	const struct sof_dsp_power_state target_state = {
+		.state = SOF_DSP_PM_D3,
+	};
+	int ret;
+
 	/* stop hda controller and power dsp off */
-	return hda_suspend(sdev, true);
+	ret = hda_suspend(sdev, true);
+	if (ret < 0)
+		return ret;
+
+	return hda_dsp_set_power_state(sdev, &target_state);
 }
 
-int hda_dsp_suspend(struct snd_sof_dev *sdev)
+int hda_dsp_suspend(struct snd_sof_dev *sdev, u32 target_state)
 {
 	struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
 	struct hdac_bus *bus = sof_to_bus(sdev);
 	struct pci_dev *pci = to_pci_dev(sdev->dev);
+	const struct sof_dsp_power_state target_dsp_state = {
+		.state = target_state,
+		.substate = target_state == SOF_DSP_PM_D0 ?
+				SOF_HDA_DSP_PM_D0I3 : 0,
+	};
 	int ret;
 
-	if (sdev->system_suspend_target == SOF_SUSPEND_S0IX) {
+	if (target_state == SOF_DSP_PM_D0) {
+		/* Set DSP power state */
+		ret = hda_dsp_set_power_state(sdev, &target_dsp_state);
+		if (ret < 0) {
+			dev_err(sdev->dev, "error: setting dsp state %d substate %d\n",
+				target_dsp_state.state,
+				target_dsp_state.substate);
+			return ret;
+		}
+
 		/* enable L1SEN to make sure the system can enter S0Ix */
 		hda->l1_support_changed =
 			snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
@@ -551,7 +738,7 @@ int hda_dsp_suspend(struct snd_sof_dev *sdev)
 		return ret;
 	}
 
-	return 0;
+	return hda_dsp_set_power_state(sdev, &target_dsp_state);
 }
 
 int hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev)
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 6191d9192fae..02c2a7eadb1b 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -392,6 +392,12 @@ struct sof_intel_dsp_bdl {
 #define SOF_HDA_PLAYBACK		0
 #define SOF_HDA_CAPTURE			1
 
+/* HDA DSP D0 substate */
+enum sof_hda_D0_substate {
+	SOF_HDA_DSP_PM_D0I0,	/* default D0 substate */
+	SOF_HDA_DSP_PM_D0I3,	/* low power D0 substate */
+};
+
 /* represents DSP HDA controller frontend - i.e. host facing control */
 struct sof_intel_hda_dev {
 
@@ -469,9 +475,9 @@ void hda_dsp_ipc_int_enable(struct snd_sof_dev *sdev);
 void hda_dsp_ipc_int_disable(struct snd_sof_dev *sdev);
 
 int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
-			    enum sof_d0_substate d0_substate);
+			    const struct sof_dsp_power_state *target_state);
 
-int hda_dsp_suspend(struct snd_sof_dev *sdev);
+int hda_dsp_suspend(struct snd_sof_dev *sdev, u32 target_state);
 int hda_dsp_resume(struct snd_sof_dev *sdev);
 int hda_dsp_runtime_suspend(struct snd_sof_dev *sdev);
 int hda_dsp_runtime_resume(struct snd_sof_dev *sdev);
diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h
index e929a6e0058f..7f532bcc8e9d 100644
--- a/sound/soc/sof/ops.h
+++ b/sound/soc/sof/ops.h
@@ -146,10 +146,11 @@ static inline int snd_sof_dsp_resume(struct snd_sof_dev *sdev)
 	return 0;
 }
 
-static inline int snd_sof_dsp_suspend(struct snd_sof_dev *sdev)
+static inline int snd_sof_dsp_suspend(struct snd_sof_dev *sdev,
+				      u32 target_state)
 {
 	if (sof_ops(sdev)->suspend)
-		return sof_ops(sdev)->suspend(sdev);
+		return sof_ops(sdev)->suspend(sdev, target_state);
 
 	return 0;
 }
@@ -193,14 +194,15 @@ static inline int snd_sof_dsp_set_clk(struct snd_sof_dev *sdev, u32 freq)
 	return 0;
 }
 
-static inline int snd_sof_dsp_set_power_state(struct snd_sof_dev *sdev,
-					      enum sof_d0_substate substate)
+static inline int
+snd_sof_dsp_set_power_state(struct snd_sof_dev *sdev,
+			    const struct sof_dsp_power_state *target_state)
 {
 	if (sof_ops(sdev)->set_power_state)
-		return sof_ops(sdev)->set_power_state(sdev, substate);
+		return sof_ops(sdev)->set_power_state(sdev, target_state);
 
-	/* D0 substate is not supported */
-	return -ENOTSUPP;
+	/* D0 substate is not supported, do nothing here. */
+	return 0;
 }
 
 /* debug */
diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c
index bec25cb6beec..c410822d9920 100644
--- a/sound/soc/sof/pm.c
+++ b/sound/soc/sof/pm.c
@@ -86,7 +86,7 @@ static void sof_cache_debugfs(struct snd_sof_dev *sdev)
 static int sof_resume(struct device *dev, bool runtime_resume)
 {
 	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
-	enum sof_d0_substate old_d0_substate = sdev->d0_substate;
+	u32 old_state = sdev->dsp_power_state.state;
 	int ret;
 
 	/* do nothing if dsp resume callbacks are not set */
@@ -97,17 +97,6 @@ static int sof_resume(struct device *dev, bool runtime_resume)
 	if (sdev->first_boot)
 		return 0;
 
-	/* resume from D0I3 */
-	if (!runtime_resume && old_d0_substate == SOF_DSP_D0I3) {
-		ret = snd_sof_set_d0_substate(sdev, SOF_DSP_D0I0);
-		if (ret < 0 && ret != -ENOTSUPP) {
-			dev_err(sdev->dev,
-				"error: failed to resume from D0I3 %d\n",
-				ret);
-			return ret;
-		}
-	}
-
 	/*
 	 * if the runtime_resume flag is set, call the runtime_resume routine
 	 * or else call the system resume routine
@@ -122,8 +111,8 @@ static int sof_resume(struct device *dev, bool runtime_resume)
 		return ret;
 	}
 
-	/* Nothing further to do if resuming from D0I3 */
-	if (!runtime_resume && old_d0_substate == SOF_DSP_D0I3)
+	/* Nothing further to do if resuming from a low-power D0 substate */
+	if (!runtime_resume && old_state == SOF_DSP_PM_D0)
 		return 0;
 
 	sdev->fw_state = SOF_FW_BOOT_PREPARE;
@@ -176,15 +165,13 @@ static int sof_resume(struct device *dev, bool runtime_resume)
 			"error: ctx_restore ipc error during resume %d\n",
 			ret);
 
-	/* initialize default D0 sub-state */
-	sdev->d0_substate = SOF_DSP_D0I0;
-
 	return ret;
 }
 
 static int sof_suspend(struct device *dev, bool runtime_suspend)
 {
 	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
+	u32 target_state = 0;
 	int ret;
 
 	/* do nothing if dsp suspend callback is not set */
@@ -205,18 +192,11 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
 		}
 	}
 
-	if (snd_sof_dsp_power_target(sdev) == SOF_DSP_PM_D0) {
-		/* suspend to D0i3 */
-		ret = snd_sof_set_d0_substate(sdev, SOF_DSP_D0I3);
-		if (ret < 0) {
-			dev_err(sdev->dev, "error: failed to enter D0I3, %d\n",
-				ret);
-			return ret;
-		}
+	target_state = snd_sof_dsp_power_target(sdev);
 
-		/* Skip to platform-specific suspend if DSP is entering D0I3 */
+	/* Skip to platform-specific suspend if DSP is entering D0 */
+	if (target_state == SOF_DSP_PM_D0)
 		goto suspend;
-	}
 
 	/* release trace */
 	snd_sof_release_trace(sdev);
@@ -254,14 +234,14 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
 	if (runtime_suspend)
 		ret = snd_sof_dsp_runtime_suspend(sdev);
 	else
-		ret = snd_sof_dsp_suspend(sdev);
+		ret = snd_sof_dsp_suspend(sdev, target_state);
 	if (ret < 0)
 		dev_err(sdev->dev,
 			"error: failed to power down DSP during suspend %d\n",
 			ret);
 
-	/* Do not reset FW state if DSP is in D0I3 */
-	if (sdev->d0_substate == SOF_DSP_D0I3)
+	/* Do not reset FW state if DSP is in D0 */
+	if (target_state == SOF_DSP_PM_D0)
 		return ret;
 
 	/* reset FW state */
@@ -290,58 +270,6 @@ int snd_sof_runtime_resume(struct device *dev)
 }
 EXPORT_SYMBOL(snd_sof_runtime_resume);
 
-int snd_sof_set_d0_substate(struct snd_sof_dev *sdev,
-			    enum sof_d0_substate d0_substate)
-{
-	int ret;
-
-	if (sdev->d0_substate == d0_substate)
-		return 0;
-
-	/* do platform specific set_state */
-	ret = snd_sof_dsp_set_power_state(sdev, d0_substate);
-	if (ret < 0)
-		return ret;
-
-	/* update dsp D0 sub-state */
-	sdev->d0_substate = d0_substate;
-
-	return 0;
-}
-EXPORT_SYMBOL(snd_sof_set_d0_substate);
-
-/*
- * Audio DSP states may transform as below:-
- *
- *                                         D0I3 compatible stream
- *     Runtime    +---------------------+   opened only, timeout
- *     suspend    |                     +--------------------+
- *   +------------+       D0(active)    |                    |
- *   |            |                     <---------------+    |
- *   |   +-------->                     |               |    |
- *   |   |Runtime +--^--+---------^--+--+ The last      |    |
- *   |   |resume     |  |         |  |    opened D0I3   |    |
- *   |   |           |  |         |  |    compatible    |    |
- *   |   |     resume|  |         |  |    stream closed |    |
- *   |   |      from |  | D3      |  |                  |    |
- *   |   |       D3  |  |suspend  |  | d0i3             |    |
- *   |   |           |  |         |  |suspend           |    |
- *   |   |           |  |         |  |                  |    |
- *   |   |           |  |         |  |                  |    |
- * +-v---+-----------+--v-------+ |  |           +------+----v----+
- * |                            | |  +----------->                |
- * |       D3 (suspended)       | |              |      D0I3      +-----+
- * |                            | +--------------+                |     |
- * |                            |  resume from   |                |     |
- * +-------------------^--------+  d0i3 suspend  +----------------+     |
- *                     |                                                |
- *                     |                       D3 suspend               |
- *                     +------------------------------------------------+
- *
- * d0i3_suspend = s0_suspend && D0I3 stream opened,
- * D3 suspend = !d0i3_suspend,
- */
-
 int snd_sof_resume(struct device *dev)
 {
 	return sof_resume(dev, false);
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index a7c6109acd98..ef33aaadbc31 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -64,10 +64,9 @@ enum sof_dsp_power_states {
 	SOF_DSP_PM_D3_COLD,
 };
 
-/* DSP D0ix sub-state */
-enum sof_d0_substate {
-	SOF_DSP_D0I0 = 0,	/* DSP default D0 substate */
-	SOF_DSP_D0I3,		/* DSP D0i3(low power) substate*/
+struct sof_dsp_power_state {
+	u32 state;
+	u32 substate; /* platform-specific */
 };
 
 /* System suspend target state */
@@ -186,14 +185,15 @@ struct snd_sof_dsp_ops {
 	int (*post_fw_run)(struct snd_sof_dev *sof_dev); /* optional */
 
 	/* DSP PM */
-	int (*suspend)(struct snd_sof_dev *sof_dev); /* optional */
+	int (*suspend)(struct snd_sof_dev *sof_dev,
+		       u32 target_state); /* optional */
 	int (*resume)(struct snd_sof_dev *sof_dev); /* optional */
 	int (*runtime_suspend)(struct snd_sof_dev *sof_dev); /* optional */
 	int (*runtime_resume)(struct snd_sof_dev *sof_dev); /* optional */
 	int (*runtime_idle)(struct snd_sof_dev *sof_dev); /* optional */
 	int (*set_hw_params_upon_resume)(struct snd_sof_dev *sdev); /* optional */
 	int (*set_power_state)(struct snd_sof_dev *sdev,
-			       enum sof_d0_substate d0_substate); /* optional */
+			       const struct sof_dsp_power_state *target_state); /* optional */
 
 	/* DSP clocking */
 	int (*set_clk)(struct snd_sof_dev *sof_dev, u32 freq); /* optional */
@@ -340,8 +340,8 @@ struct snd_sof_dev {
 	 */
 	struct snd_soc_component_driver plat_drv;
 
-	/* power states related */
-	enum sof_d0_substate d0_substate;
+	/* current DSP power state */
+	struct sof_dsp_power_state dsp_power_state;
 
 	/* Intended power target of system suspend */
 	enum sof_system_suspend_state system_suspend_target;
@@ -435,8 +435,6 @@ int snd_sof_resume(struct device *dev);
 int snd_sof_suspend(struct device *dev);
 int snd_sof_prepare(struct device *dev);
 void snd_sof_complete(struct device *dev);
-int snd_sof_set_d0_substate(struct snd_sof_dev *sdev,
-			    enum sof_d0_substate d0_substate);
 
 void snd_sof_new_platform_drv(struct snd_sof_dev *sdev);
 
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH 5/9] ASoC: SOF: Move DSP power state transitions to platform-specific ops

[Amadeusz Sławiński]

On 1/29/2020 11:07 PM, Pierre-Louis Bossart wrote:
> From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> 
> The DSP device substates such as D0I0/D0I3
> are platform-specific. Therefore, the d0_substate
> field of struct snd_sof_dev is replaced
> with the dsp_power_state field which represents the current
> state of the DSP. This field holds both the device state
> and the platform-specific substate values.
> 
> With the DSP device substates being platform-specific,
> the DSP power state transitions need to be performed in
> the platform-specific suspend/resume ops as well.
> 
> In order to achieve this, the ops signature has to be
> modified to pass the target device state as an
> argument. The target substate will be determined by
> the platform-specific ops before performing the transition.
> For example, in the case of the system suspending to S0IX,
> the top-level SOF device suspend callback needs to
> only determine if the DSP will be entering
> D3 or remain in D0. The target substate in case the device
> needs to remain in D0 (D0I0 or D0I3) will be determined
> by the platform-specific suspend op.
> 
> With the addition of the extended set of power states for the DSP,
> the set_power_state op for HDA platforms has to be extended
> to handle only the appropriate state transitions. So, the
> implementation for the Intel HDA platforms is also modified.
> 
> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
> ---
>   sound/soc/sof/core.c          |   4 +-
>   sound/soc/sof/intel/hda-dsp.c | 223 +++++++++++++++++++++++++++++++---
>   sound/soc/sof/intel/hda.h     |  10 +-
>   sound/soc/sof/ops.h           |  16 +--
>   sound/soc/sof/pm.c            |  92 ++------------
>   sound/soc/sof/sof-priv.h      |  18 ++-
>   6 files changed, 242 insertions(+), 121 deletions(-)
> 
> diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c
> index 34cefbaf2d2a..1d07450aff77 100644
> --- a/sound/soc/sof/core.c
> +++ b/sound/soc/sof/core.c
> @@ -286,8 +286,8 @@ int snd_sof_device_probe(struct device *dev, struct snd_sof_pdata *plat_data)
>   	/* initialize sof device */
>   	sdev->dev = dev;
>   
> -	/* initialize default D0 sub-state */
> -	sdev->d0_substate = SOF_DSP_D0I0;
> +	/* initialize default DSP power state */
> +	sdev->dsp_power_state.state = SOF_DSP_PM_D0;
>   
>   	sdev->pdata = plat_data;
>   	sdev->first_boot = true;
> diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
> index fddf2c48904f..8c00e128a7b0 100644
> --- a/sound/soc/sof/intel/hda-dsp.c
> +++ b/sound/soc/sof/intel/hda-dsp.c
> @@ -338,13 +338,10 @@ static int hda_dsp_send_pm_gate_ipc(struct snd_sof_dev *sdev, u32 flags)
>   				  sizeof(pm_gate), &reply, sizeof(reply));
>   }
>   
> -int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
> -			    enum sof_d0_substate d0_substate)
> +static int hda_dsp_update_d0i3c_register(struct snd_sof_dev *sdev, u8 value)
>   {
>   	struct hdac_bus *bus = sof_to_bus(sdev);
> -	u32 flags;
>   	int ret;
> -	u8 value;
>   
>   	/* Write to D0I3C after Command-In-Progress bit is cleared */
>   	ret = hda_dsp_wait_d0i3c_done(sdev);
> @@ -354,7 +351,6 @@ int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
>   	}
>   
>   	/* Update D0I3C register */
> -	value = d0_substate == SOF_DSP_D0I3 ? SOF_HDA_VS_D0I3C_I3 : 0;
>   	snd_hdac_chip_updateb(bus, VS_D0I3C, SOF_HDA_VS_D0I3C_I3, value);
>   
>   	/* Wait for cmd in progress to be cleared before exiting the function */
> @@ -367,20 +363,160 @@ int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
>   	dev_vdbg(bus->dev, "D0I3C updated, register = 0x%x\n",
>   		 snd_hdac_chip_readb(bus, VS_D0I3C));
>   
> -	if (d0_substate == SOF_DSP_D0I0)
> -		flags = HDA_PM_PPG;/* prevent power gating in D0 */
> -	else
> -		flags = HDA_PM_NO_DMA_TRACE;/* disable DMA trace in D0I3*/
> +	return 0;
> +}
>   
> -	/* sending pm_gate IPC */
> -	ret = hda_dsp_send_pm_gate_ipc(sdev, flags);
> +static int hda_dsp_set_D0_state(struct snd_sof_dev *sdev,
> +				const struct sof_dsp_power_state *target_state)
> +{
> +	u32 flags = 0;
> +	int ret;
> +	u8 value = 0;
> +
> +	/*
> +	 * Sanity check for illegal state transitions
> +	 * The only allowed transitions are:
> +	 * 1. D3 -> D0I0
> +	 * 2. D0I0 -> D0I3
> +	 * 3. D0I3 -> D0I0
> +	 */
> +	switch (sdev->dsp_power_state.state) {
> +	case SOF_DSP_PM_D0:
> +		/* Follow the sequence below for D0 substate transitions */
> +		break;
> +	case SOF_DSP_PM_D3:
> +		/* Follow regular flow for D3 -> D0 transition */
> +		return 0;
> +	default:
> +		dev_err(sdev->dev, "error: transition from %d to %d not allowed\n",
> +			sdev->dsp_power_state.state, target_state->state);
> +		return -EINVAL;
> +	}
> +
> +	/* Set flags and register value for D0 target substate */
> +	if (target_state->substate == SOF_HDA_DSP_PM_D0I3) {
> +		value = SOF_HDA_VS_D0I3C_I3;
> +
> +		/* disable DMA trace in D0I3 */
> +		flags = HDA_PM_NO_DMA_TRACE;
> +	} else {
> +		/* prevent power gating in D0I0 */
> +		flags = HDA_PM_PPG;
> +	}
> +
> +	/* update D0I3C register */
> +	ret = hda_dsp_update_d0i3c_register(sdev, value);
>   	if (ret < 0)
> +		return ret;
> +
> +	/*
> +	 * Notify the DSP of the state change.
> +	 * If this IPC fails, revert the D0I3C register update in order
> +	 * to prevent partial state change.
> +	 */
> +	ret = hda_dsp_send_pm_gate_ipc(sdev, flags);
> +	if (ret < 0) {
>   		dev_err(sdev->dev,
>   			"error: PM_GATE ipc error %d\n", ret);
> +		goto revert;
> +	}
> +
> +	return ret;
> +
> +revert:
> +	/* fallback to the previous register value */
> +	value = value ? 0 : SOF_HDA_VS_D0I3C_I3;
> +
> +	/*
> +	 * This can fail but return the IPC error to signal that
> +	 * the state change failed.
> +	 */
> +	hda_dsp_update_d0i3c_register(sdev, value);
>   
>   	return ret;
>   }
>   
> +/*
> + * All DSP power state transitions are initiated by the driver.
> + * If the requested state change fails, the error is simply returned.
> + * Further state transitions are attempted only when the set_power_save() op
> + * is called again either because of a new IPC sent to the DSP or
> + * during system suspend/resume.
> + */
> +int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
> +			    const struct sof_dsp_power_state *target_state)
> +{
> +	int ret = 0;
> +
> +	/* Nothing to do if the DSP is already in the requested state */
> +	if (target_state->state == sdev->dsp_power_state.state &&
> +	    target_state->substate == sdev->dsp_power_state.substate)
> +		return 0;
> +
> +	switch (target_state->state) {
> +	case SOF_DSP_PM_D0:
> +		ret = hda_dsp_set_D0_state(sdev, target_state);
> +		break;
> +	case SOF_DSP_PM_D3:
> +		/* The only allowed transition is: D0I0 -> D3 */
> +		if (sdev->dsp_power_state.state == SOF_DSP_PM_D0 &&
> +		    sdev->dsp_power_state.substate == SOF_HDA_DSP_PM_D0I0)
> +			break;
> +
> +		dev_err(sdev->dev,
> +			"error: transition from %d to %d not allowed\n",
> +			sdev->dsp_power_state.state, target_state->state);
> +		return -EINVAL;
> +	default:
> +		dev_err(sdev->dev, "error: target state unsupported %d\n",
> +			target_state->state);
> +		return -EINVAL;
> +	}
> +	if (ret < 0) {
> +		dev_err(sdev->dev,
> +			"failed to set requested target DSP state %d substate %d\n",
> +			target_state->state, target_state->substate);
> +		return ret;
> +	}
> +
> +	sdev->dsp_power_state = *target_state;
> +	dev_dbg(sdev->dev, "New DSP state %d substate %d\n",
> +		target_state->state, target_state->substate);
> +	return ret;
> +}
> +
> +/*
> + * Audio DSP states may transform as below:-
> + *
> + *                                         D0I3 compatible stream
> + *     Runtime    +---------------------+   opened only, timeout
> + *     suspend    |                     +--------------------+
> + *   +------------+       D0(active)    |                    |
> + *   |            |                     <---------------+    |
> + *   |   +-------->                     |               |    |
> + *   |   |Runtime +--^--+---------^--+--+ The last      |    |
> + *   |   |resume     |  |         |  |    opened D0I3   |    |
> + *   |   |           |  |         |  |    compatible    |    |
> + *   |   |     resume|  |         |  |    stream closed |    |
> + *   |   |      from |  | D3      |  |                  |    |
> + *   |   |       D3  |  |suspend  |  | d0i3             |    |
> + *   |   |           |  |         |  |suspend           |    |
> + *   |   |           |  |         |  |                  |    |
> + *   |   |           |  |         |  |                  |    |
> + * +-v---+-----------+--v-------+ |  |           +------+----v----+
> + * |                            | |  +----------->                |
> + * |       D3 (suspended)       | |              |      D0I3      +-----+
> + * |                            | +--------------+                |     |
> + * |                            |  resume from   |                |     |
> + * +-------------------^--------+  d0i3 suspend  +----------------+     |
> + *                     |                                                |
> + *                     |                       D3 suspend               |
> + *                     +------------------------------------------------+
> + *
> + * d0i3_suspend = s0_suspend && D0I3 stream opened,
> + * D3 suspend = !d0i3_suspend,
> + */
> +
>   static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend)
>   {
>   	struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
> @@ -480,8 +616,22 @@ int hda_dsp_resume(struct snd_sof_dev *sdev)
>   {
>   	struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
>   	struct pci_dev *pci = to_pci_dev(sdev->dev);
> +	const struct sof_dsp_power_state target_state = {
> +		.state = SOF_DSP_PM_D0,
> +		.substate = SOF_HDA_DSP_PM_D0I0,
> +	};
> +	int ret;
> +
> +	/* resume from D0I3 */
> +	if (sdev->dsp_power_state.state == SOF_DSP_PM_D0) {
> +		/* Set DSP power state */
> +		ret = hda_dsp_set_power_state(sdev, &target_state);
> +		if (ret < 0) {
> +			dev_err(sdev->dev, "error: setting dsp state %d substate %d\n",
> +				target_state.state, target_state.substate);
> +			return ret;
> +		}
>   
> -	if (sdev->system_suspend_target == SOF_SUSPEND_S0IX) {
>   		/* restore L1SEN bit */
>   		if (hda->l1_support_changed)
>   			snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
> @@ -495,13 +645,27 @@ int hda_dsp_resume(struct snd_sof_dev *sdev)
>   	}
>   
>   	/* init hda controller. DSP cores will be powered up during fw boot */
> -	return hda_resume(sdev, false);
> +	ret = hda_resume(sdev, false);
> +	if (ret < 0)
> +		return ret;
> +
> +	hda_dsp_set_power_state(sdev, &target_state);

Return value  of hda_dsp_set_power_state() seems to be checked or 
directly returned in other functions, any reason to not do it here?

> +	return ret;
>   }
>   
>   int hda_dsp_runtime_resume(struct snd_sof_dev *sdev)
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH 5/9] ASoC: SOF: Move DSP power state transitions to platform-specific ops

[Mark Brown]

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

On Thu, Jan 30, 2020 at 08:47:28AM +0100, Amadeusz Sławiński wrote:
> 
> 
> On 1/29/2020 11:07 PM, Pierre-Louis Bossart wrote:
> > From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
> > 
> > The DSP device substates such as D0I0/D0I3
> > are platform-specific. Therefore, the d0_substate
> > field of struct snd_sof_dev is replaced
> > with the dsp_power_state field which represents the current
> > state of the DSP. This field holds both the device state
> > and the platform-specific substate values.

Please delete unneeded context from mails when replying.  Doing this
makes it much easier to find your reply in the message, helping ensure
it won't be missed by people scrolling through the irrelevant quoted
material.

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

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

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH 5/9] ASoC: SOF: Move DSP power state transitions to platform-specific ops

[Pierre-Louis Bossart]

>> @@ -495,13 +645,27 @@ int hda_dsp_resume(struct snd_sof_dev *sdev)
>>       }
>>       /* init hda controller. DSP cores will be powered up during fw 
>> boot */
>> -    return hda_resume(sdev, false);
>> +    ret = hda_resume(sdev, false);
>> +    if (ret < 0)
>> +        return ret;
>> +
>> +    hda_dsp_set_power_state(sdev, &target_state);
> 
> Return value  of hda_dsp_set_power_state() seems to be checked or 
> directly returned in other functions, any reason to not do it here?

Good point Amadeusz, not sure why. And looking at the code, I am not 
sure either why we don't use the abstraction w/ .set_power_state() ?

intel/apl.c:    .set_power_state        = hda_dsp_set_power_state,
intel/cnl.c:    .set_power_state        = hda_dsp_set_power_state,


git grep snd_sof_dsp_set_power_state
sof/ipc.c:      ret = snd_sof_dsp_set_power_state(ipc->sdev, &target_state);
sof/ops.h:snd_sof_dsp_set_power_state(struct snd_sof_dev *sdev,

If the code can be platform-specific, we shouldn't make a direct call 
but go through the platform indirection. it's fine for now since the 
same routine is used in all cases but it's not scalable/future-proof.

Ranjani?

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH 5/9] ASoC: SOF: Move DSP power state transitions to platform-specific ops

[Sridharan, Ranjani]

On Thu, Jan 30, 2020 at 7:07 AM Pierre-Louis Bossart <
pierre-louis.bossart@linux.intel.com> wrote:

>
> >> @@ -495,13 +645,27 @@ int hda_dsp_resume(struct snd_sof_dev *sdev)
> >>       }
> >>       /* init hda controller. DSP cores will be powered up during fw
> >> boot */
> >> -    return hda_resume(sdev, false);
> >> +    ret = hda_resume(sdev, false);
> >> +    if (ret < 0)
> >> +        return ret;
> >> +
> >> +    hda_dsp_set_power_state(sdev, &target_state);
> >
> > Return value  of hda_dsp_set_power_state() seems to be checked or
> > directly returned in other functions, any reason to not do it here?
>
> Good point Amadeusz, not sure why. And looking at the code, I am not
> sure either why we don't use the abstraction w/ .set_power_state() ?
>
> intel/apl.c:    .set_power_state        = hda_dsp_set_power_state,
> intel/cnl.c:    .set_power_state        = hda_dsp_set_power_state,
>
>
> git grep snd_sof_dsp_set_power_state
> sof/ipc.c:      ret = snd_sof_dsp_set_power_state(ipc->sdev,
> &target_state);
> sof/ops.h:snd_sof_dsp_set_power_state(struct snd_sof_dev *sdev,
>
> If the code can be platform-specific, we shouldn't make a direct call
> but go through the platform indirection. it's fine for now since the
> same routine is used in all cases but it's not scalable/future-proof.
>
> Ranjani?
>
Hi Amadeusz/Pierre,

This seems like a miss. We should have returned the value of
hda_set_power_state() directly here.

And to address your point, Pierre. This is the platform-specific code, so
the use of hda_dsp_set_power_state() should be permitted no? Whereas, the
part that uses this function in ipc.c is not platform-specific.
Thanks,
Ranjani
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [alsa-devel] [PATCH 5/9] ASoC: SOF: Move DSP power state transitions to platform-specific ops

[Pierre-Louis Bossart]

>>>> @@ -495,13 +645,27 @@ int hda_dsp_resume(struct snd_sof_dev *sdev)
>>>>        }
>>>>        /* init hda controller. DSP cores will be powered up during fw
>>>> boot */
>>>> -    return hda_resume(sdev, false);
>>>> +    ret = hda_resume(sdev, false);
>>>> +    if (ret < 0)
>>>> +        return ret;
>>>> +
>>>> +    hda_dsp_set_power_state(sdev, &target_state);
>>>
>>> Return value  of hda_dsp_set_power_state() seems to be checked or
>>> directly returned in other functions, any reason to not do it here?

> This seems like a miss. We should have returned the value of
> hda_set_power_state() directly here.
> 
> And to address your point, Pierre. This is the platform-specific code, so
> the use of hda_dsp_set_power_state() should be permitted no? Whereas, the
> part that uses this function in ipc.c is not platform-specific.

Well, what do we mean by 'platform-specific'? Here we have two different 
architectures (APL and CNL) and we'll likely see more.
We can also consider that all this code is a common library for all 
HDaudio platforms. I just wanted to make sure we don't take shortcuts.

At any rate, the return value needs to be fixed, we can discuss further 
on the HDaudio code partition in future evolutions. I am not 100% happy 
with which file deals with what, things are a bit convoluted between 
hda.c hda-ctrl.c hda-dsp.c hda-pcm.c, etc.

Thanks
-Pierre
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] Applied "ASoC: SOF: Move DSP power state transitions to platform-specific ops" to the asoc tree

[Mark Brown]

The patch

   ASoC: SOF: Move DSP power state transitions to platform-specific ops

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.7

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 61e285caf40fef18e8bd7cea5237ee6723609a1c Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Date: Wed, 29 Jan 2020 16:07:22 -0600
Subject: [PATCH] ASoC: SOF: Move DSP power state transitions to
 platform-specific ops

The DSP device substates such as D0I0/D0I3
are platform-specific. Therefore, the d0_substate
field of struct snd_sof_dev is replaced
with the dsp_power_state field which represents the current
state of the DSP. This field holds both the device state
and the platform-specific substate values.

With the DSP device substates being platform-specific,
the DSP power state transitions need to be performed in
the platform-specific suspend/resume ops as well.

In order to achieve this, the ops signature has to be
modified to pass the target device state as an
argument. The target substate will be determined by
the platform-specific ops before performing the transition.
For example, in the case of the system suspending to S0IX,
the top-level SOF device suspend callback needs to
only determine if the DSP will be entering
D3 or remain in D0. The target substate in case the device
needs to remain in D0 (D0I0 or D0I3) will be determined
by the platform-specific suspend op.

With the addition of the extended set of power states for the DSP,
the set_power_state op for HDA platforms has to be extended
to handle only the appropriate state transitions. So, the
implementation for the Intel HDA platforms is also modified.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200129220726.31792-6-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sof/core.c          |   4 +-
 sound/soc/sof/intel/hda-dsp.c | 223 +++++++++++++++++++++++++++++++---
 sound/soc/sof/intel/hda.h     |  10 +-
 sound/soc/sof/ops.h           |  16 +--
 sound/soc/sof/pm.c            |  92 ++------------
 sound/soc/sof/sof-priv.h      |  18 ++-
 6 files changed, 242 insertions(+), 121 deletions(-)

diff --git a/sound/soc/sof/core.c b/sound/soc/sof/core.c
index 34cefbaf2d2a..1d07450aff77 100644
--- a/sound/soc/sof/core.c
+++ b/sound/soc/sof/core.c
@@ -286,8 +286,8 @@ int snd_sof_device_probe(struct device *dev, struct snd_sof_pdata *plat_data)
 	/* initialize sof device */
 	sdev->dev = dev;
 
-	/* initialize default D0 sub-state */
-	sdev->d0_substate = SOF_DSP_D0I0;
+	/* initialize default DSP power state */
+	sdev->dsp_power_state.state = SOF_DSP_PM_D0;
 
 	sdev->pdata = plat_data;
 	sdev->first_boot = true;
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index fddf2c48904f..8c00e128a7b0 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -338,13 +338,10 @@ static int hda_dsp_send_pm_gate_ipc(struct snd_sof_dev *sdev, u32 flags)
 				  sizeof(pm_gate), &reply, sizeof(reply));
 }
 
-int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
-			    enum sof_d0_substate d0_substate)
+static int hda_dsp_update_d0i3c_register(struct snd_sof_dev *sdev, u8 value)
 {
 	struct hdac_bus *bus = sof_to_bus(sdev);
-	u32 flags;
 	int ret;
-	u8 value;
 
 	/* Write to D0I3C after Command-In-Progress bit is cleared */
 	ret = hda_dsp_wait_d0i3c_done(sdev);
@@ -354,7 +351,6 @@ int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
 	}
 
 	/* Update D0I3C register */
-	value = d0_substate == SOF_DSP_D0I3 ? SOF_HDA_VS_D0I3C_I3 : 0;
 	snd_hdac_chip_updateb(bus, VS_D0I3C, SOF_HDA_VS_D0I3C_I3, value);
 
 	/* Wait for cmd in progress to be cleared before exiting the function */
@@ -367,20 +363,160 @@ int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
 	dev_vdbg(bus->dev, "D0I3C updated, register = 0x%x\n",
 		 snd_hdac_chip_readb(bus, VS_D0I3C));
 
-	if (d0_substate == SOF_DSP_D0I0)
-		flags = HDA_PM_PPG;/* prevent power gating in D0 */
-	else
-		flags = HDA_PM_NO_DMA_TRACE;/* disable DMA trace in D0I3*/
+	return 0;
+}
 
-	/* sending pm_gate IPC */
-	ret = hda_dsp_send_pm_gate_ipc(sdev, flags);
+static int hda_dsp_set_D0_state(struct snd_sof_dev *sdev,
+				const struct sof_dsp_power_state *target_state)
+{
+	u32 flags = 0;
+	int ret;
+	u8 value = 0;
+
+	/*
+	 * Sanity check for illegal state transitions
+	 * The only allowed transitions are:
+	 * 1. D3 -> D0I0
+	 * 2. D0I0 -> D0I3
+	 * 3. D0I3 -> D0I0
+	 */
+	switch (sdev->dsp_power_state.state) {
+	case SOF_DSP_PM_D0:
+		/* Follow the sequence below for D0 substate transitions */
+		break;
+	case SOF_DSP_PM_D3:
+		/* Follow regular flow for D3 -> D0 transition */
+		return 0;
+	default:
+		dev_err(sdev->dev, "error: transition from %d to %d not allowed\n",
+			sdev->dsp_power_state.state, target_state->state);
+		return -EINVAL;
+	}
+
+	/* Set flags and register value for D0 target substate */
+	if (target_state->substate == SOF_HDA_DSP_PM_D0I3) {
+		value = SOF_HDA_VS_D0I3C_I3;
+
+		/* disable DMA trace in D0I3 */
+		flags = HDA_PM_NO_DMA_TRACE;
+	} else {
+		/* prevent power gating in D0I0 */
+		flags = HDA_PM_PPG;
+	}
+
+	/* update D0I3C register */
+	ret = hda_dsp_update_d0i3c_register(sdev, value);
 	if (ret < 0)
+		return ret;
+
+	/*
+	 * Notify the DSP of the state change.
+	 * If this IPC fails, revert the D0I3C register update in order
+	 * to prevent partial state change.
+	 */
+	ret = hda_dsp_send_pm_gate_ipc(sdev, flags);
+	if (ret < 0) {
 		dev_err(sdev->dev,
 			"error: PM_GATE ipc error %d\n", ret);
+		goto revert;
+	}
+
+	return ret;
+
+revert:
+	/* fallback to the previous register value */
+	value = value ? 0 : SOF_HDA_VS_D0I3C_I3;
+
+	/*
+	 * This can fail but return the IPC error to signal that
+	 * the state change failed.
+	 */
+	hda_dsp_update_d0i3c_register(sdev, value);
 
 	return ret;
 }
 
+/*
+ * All DSP power state transitions are initiated by the driver.
+ * If the requested state change fails, the error is simply returned.
+ * Further state transitions are attempted only when the set_power_save() op
+ * is called again either because of a new IPC sent to the DSP or
+ * during system suspend/resume.
+ */
+int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
+			    const struct sof_dsp_power_state *target_state)
+{
+	int ret = 0;
+
+	/* Nothing to do if the DSP is already in the requested state */
+	if (target_state->state == sdev->dsp_power_state.state &&
+	    target_state->substate == sdev->dsp_power_state.substate)
+		return 0;
+
+	switch (target_state->state) {
+	case SOF_DSP_PM_D0:
+		ret = hda_dsp_set_D0_state(sdev, target_state);
+		break;
+	case SOF_DSP_PM_D3:
+		/* The only allowed transition is: D0I0 -> D3 */
+		if (sdev->dsp_power_state.state == SOF_DSP_PM_D0 &&
+		    sdev->dsp_power_state.substate == SOF_HDA_DSP_PM_D0I0)
+			break;
+
+		dev_err(sdev->dev,
+			"error: transition from %d to %d not allowed\n",
+			sdev->dsp_power_state.state, target_state->state);
+		return -EINVAL;
+	default:
+		dev_err(sdev->dev, "error: target state unsupported %d\n",
+			target_state->state);
+		return -EINVAL;
+	}
+	if (ret < 0) {
+		dev_err(sdev->dev,
+			"failed to set requested target DSP state %d substate %d\n",
+			target_state->state, target_state->substate);
+		return ret;
+	}
+
+	sdev->dsp_power_state = *target_state;
+	dev_dbg(sdev->dev, "New DSP state %d substate %d\n",
+		target_state->state, target_state->substate);
+	return ret;
+}
+
+/*
+ * Audio DSP states may transform as below:-
+ *
+ *                                         D0I3 compatible stream
+ *     Runtime    +---------------------+   opened only, timeout
+ *     suspend    |                     +--------------------+
+ *   +------------+       D0(active)    |                    |
+ *   |            |                     <---------------+    |
+ *   |   +-------->                     |               |    |
+ *   |   |Runtime +--^--+---------^--+--+ The last      |    |
+ *   |   |resume     |  |         |  |    opened D0I3   |    |
+ *   |   |           |  |         |  |    compatible    |    |
+ *   |   |     resume|  |         |  |    stream closed |    |
+ *   |   |      from |  | D3      |  |                  |    |
+ *   |   |       D3  |  |suspend  |  | d0i3             |    |
+ *   |   |           |  |         |  |suspend           |    |
+ *   |   |           |  |         |  |                  |    |
+ *   |   |           |  |         |  |                  |    |
+ * +-v---+-----------+--v-------+ |  |           +------+----v----+
+ * |                            | |  +----------->                |
+ * |       D3 (suspended)       | |              |      D0I3      +-----+
+ * |                            | +--------------+                |     |
+ * |                            |  resume from   |                |     |
+ * +-------------------^--------+  d0i3 suspend  +----------------+     |
+ *                     |                                                |
+ *                     |                       D3 suspend               |
+ *                     +------------------------------------------------+
+ *
+ * d0i3_suspend = s0_suspend && D0I3 stream opened,
+ * D3 suspend = !d0i3_suspend,
+ */
+
 static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend)
 {
 	struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
@@ -480,8 +616,22 @@ int hda_dsp_resume(struct snd_sof_dev *sdev)
 {
 	struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
 	struct pci_dev *pci = to_pci_dev(sdev->dev);
+	const struct sof_dsp_power_state target_state = {
+		.state = SOF_DSP_PM_D0,
+		.substate = SOF_HDA_DSP_PM_D0I0,
+	};
+	int ret;
+
+	/* resume from D0I3 */
+	if (sdev->dsp_power_state.state == SOF_DSP_PM_D0) {
+		/* Set DSP power state */
+		ret = hda_dsp_set_power_state(sdev, &target_state);
+		if (ret < 0) {
+			dev_err(sdev->dev, "error: setting dsp state %d substate %d\n",
+				target_state.state, target_state.substate);
+			return ret;
+		}
 
-	if (sdev->system_suspend_target == SOF_SUSPEND_S0IX) {
 		/* restore L1SEN bit */
 		if (hda->l1_support_changed)
 			snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
@@ -495,13 +645,27 @@ int hda_dsp_resume(struct snd_sof_dev *sdev)
 	}
 
 	/* init hda controller. DSP cores will be powered up during fw boot */
-	return hda_resume(sdev, false);
+	ret = hda_resume(sdev, false);
+	if (ret < 0)
+		return ret;
+
+	hda_dsp_set_power_state(sdev, &target_state);
+	return ret;
 }
 
 int hda_dsp_runtime_resume(struct snd_sof_dev *sdev)
 {
+	const struct sof_dsp_power_state target_state = {
+		.state = SOF_DSP_PM_D0,
+	};
+	int ret;
+
 	/* init hda controller. DSP cores will be powered up during fw boot */
-	return hda_resume(sdev, true);
+	ret = hda_resume(sdev, true);
+	if (ret < 0)
+		return ret;
+
+	return hda_dsp_set_power_state(sdev, &target_state);
 }
 
 int hda_dsp_runtime_idle(struct snd_sof_dev *sdev)
@@ -519,18 +683,41 @@ int hda_dsp_runtime_idle(struct snd_sof_dev *sdev)
 
 int hda_dsp_runtime_suspend(struct snd_sof_dev *sdev)
 {
+	const struct sof_dsp_power_state target_state = {
+		.state = SOF_DSP_PM_D3,
+	};
+	int ret;
+
 	/* stop hda controller and power dsp off */
-	return hda_suspend(sdev, true);
+	ret = hda_suspend(sdev, true);
+	if (ret < 0)
+		return ret;
+
+	return hda_dsp_set_power_state(sdev, &target_state);
 }
 
-int hda_dsp_suspend(struct snd_sof_dev *sdev)
+int hda_dsp_suspend(struct snd_sof_dev *sdev, u32 target_state)
 {
 	struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
 	struct hdac_bus *bus = sof_to_bus(sdev);
 	struct pci_dev *pci = to_pci_dev(sdev->dev);
+	const struct sof_dsp_power_state target_dsp_state = {
+		.state = target_state,
+		.substate = target_state == SOF_DSP_PM_D0 ?
+				SOF_HDA_DSP_PM_D0I3 : 0,
+	};
 	int ret;
 
-	if (sdev->system_suspend_target == SOF_SUSPEND_S0IX) {
+	if (target_state == SOF_DSP_PM_D0) {
+		/* Set DSP power state */
+		ret = hda_dsp_set_power_state(sdev, &target_dsp_state);
+		if (ret < 0) {
+			dev_err(sdev->dev, "error: setting dsp state %d substate %d\n",
+				target_dsp_state.state,
+				target_dsp_state.substate);
+			return ret;
+		}
+
 		/* enable L1SEN to make sure the system can enter S0Ix */
 		hda->l1_support_changed =
 			snd_sof_dsp_update_bits(sdev, HDA_DSP_HDA_BAR,
@@ -551,7 +738,7 @@ int hda_dsp_suspend(struct snd_sof_dev *sdev)
 		return ret;
 	}
 
-	return 0;
+	return hda_dsp_set_power_state(sdev, &target_dsp_state);
 }
 
 int hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev)
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 6191d9192fae..02c2a7eadb1b 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -392,6 +392,12 @@ struct sof_intel_dsp_bdl {
 #define SOF_HDA_PLAYBACK		0
 #define SOF_HDA_CAPTURE			1
 
+/* HDA DSP D0 substate */
+enum sof_hda_D0_substate {
+	SOF_HDA_DSP_PM_D0I0,	/* default D0 substate */
+	SOF_HDA_DSP_PM_D0I3,	/* low power D0 substate */
+};
+
 /* represents DSP HDA controller frontend - i.e. host facing control */
 struct sof_intel_hda_dev {
 
@@ -469,9 +475,9 @@ void hda_dsp_ipc_int_enable(struct snd_sof_dev *sdev);
 void hda_dsp_ipc_int_disable(struct snd_sof_dev *sdev);
 
 int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
-			    enum sof_d0_substate d0_substate);
+			    const struct sof_dsp_power_state *target_state);
 
-int hda_dsp_suspend(struct snd_sof_dev *sdev);
+int hda_dsp_suspend(struct snd_sof_dev *sdev, u32 target_state);
 int hda_dsp_resume(struct snd_sof_dev *sdev);
 int hda_dsp_runtime_suspend(struct snd_sof_dev *sdev);
 int hda_dsp_runtime_resume(struct snd_sof_dev *sdev);
diff --git a/sound/soc/sof/ops.h b/sound/soc/sof/ops.h
index e929a6e0058f..7f532bcc8e9d 100644
--- a/sound/soc/sof/ops.h
+++ b/sound/soc/sof/ops.h
@@ -146,10 +146,11 @@ static inline int snd_sof_dsp_resume(struct snd_sof_dev *sdev)
 	return 0;
 }
 
-static inline int snd_sof_dsp_suspend(struct snd_sof_dev *sdev)
+static inline int snd_sof_dsp_suspend(struct snd_sof_dev *sdev,
+				      u32 target_state)
 {
 	if (sof_ops(sdev)->suspend)
-		return sof_ops(sdev)->suspend(sdev);
+		return sof_ops(sdev)->suspend(sdev, target_state);
 
 	return 0;
 }
@@ -193,14 +194,15 @@ static inline int snd_sof_dsp_set_clk(struct snd_sof_dev *sdev, u32 freq)
 	return 0;
 }
 
-static inline int snd_sof_dsp_set_power_state(struct snd_sof_dev *sdev,
-					      enum sof_d0_substate substate)
+static inline int
+snd_sof_dsp_set_power_state(struct snd_sof_dev *sdev,
+			    const struct sof_dsp_power_state *target_state)
 {
 	if (sof_ops(sdev)->set_power_state)
-		return sof_ops(sdev)->set_power_state(sdev, substate);
+		return sof_ops(sdev)->set_power_state(sdev, target_state);
 
-	/* D0 substate is not supported */
-	return -ENOTSUPP;
+	/* D0 substate is not supported, do nothing here. */
+	return 0;
 }
 
 /* debug */
diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c
index bec25cb6beec..c410822d9920 100644
--- a/sound/soc/sof/pm.c
+++ b/sound/soc/sof/pm.c
@@ -86,7 +86,7 @@ static void sof_cache_debugfs(struct snd_sof_dev *sdev)
 static int sof_resume(struct device *dev, bool runtime_resume)
 {
 	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
-	enum sof_d0_substate old_d0_substate = sdev->d0_substate;
+	u32 old_state = sdev->dsp_power_state.state;
 	int ret;
 
 	/* do nothing if dsp resume callbacks are not set */
@@ -97,17 +97,6 @@ static int sof_resume(struct device *dev, bool runtime_resume)
 	if (sdev->first_boot)
 		return 0;
 
-	/* resume from D0I3 */
-	if (!runtime_resume && old_d0_substate == SOF_DSP_D0I3) {
-		ret = snd_sof_set_d0_substate(sdev, SOF_DSP_D0I0);
-		if (ret < 0 && ret != -ENOTSUPP) {
-			dev_err(sdev->dev,
-				"error: failed to resume from D0I3 %d\n",
-				ret);
-			return ret;
-		}
-	}
-
 	/*
 	 * if the runtime_resume flag is set, call the runtime_resume routine
 	 * or else call the system resume routine
@@ -122,8 +111,8 @@ static int sof_resume(struct device *dev, bool runtime_resume)
 		return ret;
 	}
 
-	/* Nothing further to do if resuming from D0I3 */
-	if (!runtime_resume && old_d0_substate == SOF_DSP_D0I3)
+	/* Nothing further to do if resuming from a low-power D0 substate */
+	if (!runtime_resume && old_state == SOF_DSP_PM_D0)
 		return 0;
 
 	sdev->fw_state = SOF_FW_BOOT_PREPARE;
@@ -176,15 +165,13 @@ static int sof_resume(struct device *dev, bool runtime_resume)
 			"error: ctx_restore ipc error during resume %d\n",
 			ret);
 
-	/* initialize default D0 sub-state */
-	sdev->d0_substate = SOF_DSP_D0I0;
-
 	return ret;
 }
 
 static int sof_suspend(struct device *dev, bool runtime_suspend)
 {
 	struct snd_sof_dev *sdev = dev_get_drvdata(dev);
+	u32 target_state = 0;
 	int ret;
 
 	/* do nothing if dsp suspend callback is not set */
@@ -205,18 +192,11 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
 		}
 	}
 
-	if (snd_sof_dsp_power_target(sdev) == SOF_DSP_PM_D0) {
-		/* suspend to D0i3 */
-		ret = snd_sof_set_d0_substate(sdev, SOF_DSP_D0I3);
-		if (ret < 0) {
-			dev_err(sdev->dev, "error: failed to enter D0I3, %d\n",
-				ret);
-			return ret;
-		}
+	target_state = snd_sof_dsp_power_target(sdev);
 
-		/* Skip to platform-specific suspend if DSP is entering D0I3 */
+	/* Skip to platform-specific suspend if DSP is entering D0 */
+	if (target_state == SOF_DSP_PM_D0)
 		goto suspend;
-	}
 
 	/* release trace */
 	snd_sof_release_trace(sdev);
@@ -254,14 +234,14 @@ static int sof_suspend(struct device *dev, bool runtime_suspend)
 	if (runtime_suspend)
 		ret = snd_sof_dsp_runtime_suspend(sdev);
 	else
-		ret = snd_sof_dsp_suspend(sdev);
+		ret = snd_sof_dsp_suspend(sdev, target_state);
 	if (ret < 0)
 		dev_err(sdev->dev,
 			"error: failed to power down DSP during suspend %d\n",
 			ret);
 
-	/* Do not reset FW state if DSP is in D0I3 */
-	if (sdev->d0_substate == SOF_DSP_D0I3)
+	/* Do not reset FW state if DSP is in D0 */
+	if (target_state == SOF_DSP_PM_D0)
 		return ret;
 
 	/* reset FW state */
@@ -290,58 +270,6 @@ int snd_sof_runtime_resume(struct device *dev)
 }
 EXPORT_SYMBOL(snd_sof_runtime_resume);
 
-int snd_sof_set_d0_substate(struct snd_sof_dev *sdev,
-			    enum sof_d0_substate d0_substate)
-{
-	int ret;
-
-	if (sdev->d0_substate == d0_substate)
-		return 0;
-
-	/* do platform specific set_state */
-	ret = snd_sof_dsp_set_power_state(sdev, d0_substate);
-	if (ret < 0)
-		return ret;
-
-	/* update dsp D0 sub-state */
-	sdev->d0_substate = d0_substate;
-
-	return 0;
-}
-EXPORT_SYMBOL(snd_sof_set_d0_substate);
-
-/*
- * Audio DSP states may transform as below:-
- *
- *                                         D0I3 compatible stream
- *     Runtime    +---------------------+   opened only, timeout
- *     suspend    |                     +--------------------+
- *   +------------+       D0(active)    |                    |
- *   |            |                     <---------------+    |
- *   |   +-------->                     |               |    |
- *   |   |Runtime +--^--+---------^--+--+ The last      |    |
- *   |   |resume     |  |         |  |    opened D0I3   |    |
- *   |   |           |  |         |  |    compatible    |    |
- *   |   |     resume|  |         |  |    stream closed |    |
- *   |   |      from |  | D3      |  |                  |    |
- *   |   |       D3  |  |suspend  |  | d0i3             |    |
- *   |   |           |  |         |  |suspend           |    |
- *   |   |           |  |         |  |                  |    |
- *   |   |           |  |         |  |                  |    |
- * +-v---+-----------+--v-------+ |  |           +------+----v----+
- * |                            | |  +----------->                |
- * |       D3 (suspended)       | |              |      D0I3      +-----+
- * |                            | +--------------+                |     |
- * |                            |  resume from   |                |     |
- * +-------------------^--------+  d0i3 suspend  +----------------+     |
- *                     |                                                |
- *                     |                       D3 suspend               |
- *                     +------------------------------------------------+
- *
- * d0i3_suspend = s0_suspend && D0I3 stream opened,
- * D3 suspend = !d0i3_suspend,
- */
-
 int snd_sof_resume(struct device *dev)
 {
 	return sof_resume(dev, false);
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index a7c6109acd98..ef33aaadbc31 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -64,10 +64,9 @@ enum sof_dsp_power_states {
 	SOF_DSP_PM_D3_COLD,
 };
 
-/* DSP D0ix sub-state */
-enum sof_d0_substate {
-	SOF_DSP_D0I0 = 0,	/* DSP default D0 substate */
-	SOF_DSP_D0I3,		/* DSP D0i3(low power) substate*/
+struct sof_dsp_power_state {
+	u32 state;
+	u32 substate; /* platform-specific */
 };
 
 /* System suspend target state */
@@ -186,14 +185,15 @@ struct snd_sof_dsp_ops {
 	int (*post_fw_run)(struct snd_sof_dev *sof_dev); /* optional */
 
 	/* DSP PM */
-	int (*suspend)(struct snd_sof_dev *sof_dev); /* optional */
+	int (*suspend)(struct snd_sof_dev *sof_dev,
+		       u32 target_state); /* optional */
 	int (*resume)(struct snd_sof_dev *sof_dev); /* optional */
 	int (*runtime_suspend)(struct snd_sof_dev *sof_dev); /* optional */
 	int (*runtime_resume)(struct snd_sof_dev *sof_dev); /* optional */
 	int (*runtime_idle)(struct snd_sof_dev *sof_dev); /* optional */
 	int (*set_hw_params_upon_resume)(struct snd_sof_dev *sdev); /* optional */
 	int (*set_power_state)(struct snd_sof_dev *sdev,
-			       enum sof_d0_substate d0_substate); /* optional */
+			       const struct sof_dsp_power_state *target_state); /* optional */
 
 	/* DSP clocking */
 	int (*set_clk)(struct snd_sof_dev *sof_dev, u32 freq); /* optional */
@@ -340,8 +340,8 @@ struct snd_sof_dev {
 	 */
 	struct snd_soc_component_driver plat_drv;
 
-	/* power states related */
-	enum sof_d0_substate d0_substate;
+	/* current DSP power state */
+	struct sof_dsp_power_state dsp_power_state;
 
 	/* Intended power target of system suspend */
 	enum sof_system_suspend_state system_suspend_target;
@@ -435,8 +435,6 @@ int snd_sof_resume(struct device *dev);
 int snd_sof_suspend(struct device *dev);
 int snd_sof_prepare(struct device *dev);
 void snd_sof_complete(struct device *dev);
-int snd_sof_set_d0_substate(struct snd_sof_dev *sdev,
-			    enum sof_d0_substate d0_substate);
 
 void snd_sof_new_platform_drv(struct snd_sof_dev *sdev);
 
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 6/9] ASoC: SOF: audio: Add helper to check if only D0i3 streams are active

[Pierre-Louis Bossart]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Add a helper function to check if only D0i3-compatible streams
are active.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/sof-audio.c | 33 +++++++++++++++++++++++++++++++++
 sound/soc/sof/sof-audio.h |  1 +
 2 files changed, 34 insertions(+)

diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index d16571ca129c..75f2ef2bd94b 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -11,6 +11,39 @@
 #include "sof-audio.h"
 #include "ops.h"
 
+/*
+ * helper to determine if there are only D0i3 compatible
+ * streams active
+ */
+bool snd_sof_dsp_only_d0i3_compatible_stream_active(struct snd_sof_dev *sdev)
+{
+	struct snd_pcm_substream *substream;
+	struct snd_sof_pcm *spcm;
+	bool d0i3_compatible_active = false;
+	int dir;
+
+	list_for_each_entry(spcm, &sdev->pcm_list, list) {
+		for (dir = 0; dir <= SNDRV_PCM_STREAM_CAPTURE; dir++) {
+			substream = spcm->stream[dir].substream;
+			if (!substream || !substream->runtime)
+				continue;
+
+			/*
+			 * substream->runtime being not NULL indicates that
+			 * that the stream is open. No need to check the
+			 * stream state.
+			 */
+			if (!spcm->stream[dir].d0i3_compatible)
+				return false;
+
+			d0i3_compatible_active = true;
+		}
+	}
+
+	return d0i3_compatible_active;
+}
+EXPORT_SYMBOL(snd_sof_dsp_only_d0i3_compatible_stream_active);
+
 bool snd_sof_stream_suspend_ignored(struct snd_sof_dev *sdev)
 {
 	struct snd_sof_pcm *spcm;
diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h
index a2702afbd9a1..eacd10e4da11 100644
--- a/sound/soc/sof/sof-audio.h
+++ b/sound/soc/sof/sof-audio.h
@@ -203,6 +203,7 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol,
 int sof_restore_pipelines(struct device *dev);
 int sof_set_hw_params_upon_resume(struct device *dev);
 bool snd_sof_stream_suspend_ignored(struct snd_sof_dev *sdev);
+bool snd_sof_dsp_only_d0i3_compatible_stream_active(struct snd_sof_dev *sdev);
 
 /* Machine driver enumeration */
 int sof_machine_register(struct snd_sof_dev *sdev, void *pdata);
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] Applied "ASoC: SOF: audio: Add helper to check if only D0i3 streams are active" to the asoc tree

[Mark Brown]

The patch

   ASoC: SOF: audio: Add helper to check if only D0i3 streams are active

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.7

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 de23a838d8d61767c6232f229f019eb46401cb93 Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Date: Wed, 29 Jan 2020 16:07:23 -0600
Subject: [PATCH] ASoC: SOF: audio: Add helper to check if only D0i3 streams
 are active

Add a helper function to check if only D0i3-compatible streams
are active.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200129220726.31792-7-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sof/sof-audio.c | 33 +++++++++++++++++++++++++++++++++
 sound/soc/sof/sof-audio.h |  1 +
 2 files changed, 34 insertions(+)

diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c
index d16571ca129c..75f2ef2bd94b 100644
--- a/sound/soc/sof/sof-audio.c
+++ b/sound/soc/sof/sof-audio.c
@@ -11,6 +11,39 @@
 #include "sof-audio.h"
 #include "ops.h"
 
+/*
+ * helper to determine if there are only D0i3 compatible
+ * streams active
+ */
+bool snd_sof_dsp_only_d0i3_compatible_stream_active(struct snd_sof_dev *sdev)
+{
+	struct snd_pcm_substream *substream;
+	struct snd_sof_pcm *spcm;
+	bool d0i3_compatible_active = false;
+	int dir;
+
+	list_for_each_entry(spcm, &sdev->pcm_list, list) {
+		for (dir = 0; dir <= SNDRV_PCM_STREAM_CAPTURE; dir++) {
+			substream = spcm->stream[dir].substream;
+			if (!substream || !substream->runtime)
+				continue;
+
+			/*
+			 * substream->runtime being not NULL indicates that
+			 * that the stream is open. No need to check the
+			 * stream state.
+			 */
+			if (!spcm->stream[dir].d0i3_compatible)
+				return false;
+
+			d0i3_compatible_active = true;
+		}
+	}
+
+	return d0i3_compatible_active;
+}
+EXPORT_SYMBOL(snd_sof_dsp_only_d0i3_compatible_stream_active);
+
 bool snd_sof_stream_suspend_ignored(struct snd_sof_dev *sdev)
 {
 	struct snd_sof_pcm *spcm;
diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h
index a2702afbd9a1..eacd10e4da11 100644
--- a/sound/soc/sof/sof-audio.h
+++ b/sound/soc/sof/sof-audio.h
@@ -203,6 +203,7 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol,
 int sof_restore_pipelines(struct device *dev);
 int sof_set_hw_params_upon_resume(struct device *dev);
 bool snd_sof_stream_suspend_ignored(struct snd_sof_dev *sdev);
+bool snd_sof_dsp_only_d0i3_compatible_stream_active(struct snd_sof_dev *sdev);
 
 /* Machine driver enumeration */
 int sof_machine_register(struct snd_sof_dev *sdev, void *pdata);
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 7/9] ASoC: SOF: Intel: hda: Amend the DSP state transition diagram

[Pierre-Louis Bossart]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Amend the DSP state transition diagram in preparation
for introducing the feature to support opportunistic
DSP D0I3 state when the system is in S0.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/hda-dsp.c | 36 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index 8c00e128a7b0..7b8425330ae0 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -488,33 +488,31 @@ int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
 /*
  * Audio DSP states may transform as below:-
  *
- *                                         D0I3 compatible stream
- *     Runtime    +---------------------+   opened only, timeout
+ *                                         Opportunistic D0I3 in S0
+ *     Runtime    +---------------------+  Delayed D0i3 work timeout
  *     suspend    |                     +--------------------+
- *   +------------+       D0(active)    |                    |
+ *   +------------+       D0I0(active)  |                    |
  *   |            |                     <---------------+    |
- *   |   +-------->                     |               |    |
- *   |   |Runtime +--^--+---------^--+--+ The last      |    |
- *   |   |resume     |  |         |  |    opened D0I3   |    |
- *   |   |           |  |         |  |    compatible    |    |
- *   |   |     resume|  |         |  |    stream closed |    |
- *   |   |      from |  | D3      |  |                  |    |
- *   |   |       D3  |  |suspend  |  | d0i3             |    |
+ *   |   +-------->                     |    New IPC	|    |
+ *   |   |Runtime +--^--+---------^--+--+ (via mailbox)	|    |
+ *   |   |resume     |  |         |  |			|    |
+ *   |   |           |  |         |  |			|    |
+ *   |   |     System|  |         |  |			|    |
+ *   |   |     resume|  | S3/S0IX |  |                  |    |
+ *   |   |	     |  | suspend |  | S0IX             |    |
  *   |   |           |  |         |  |suspend           |    |
  *   |   |           |  |         |  |                  |    |
  *   |   |           |  |         |  |                  |    |
  * +-v---+-----------+--v-------+ |  |           +------+----v----+
  * |                            | |  +----------->                |
- * |       D3 (suspended)       | |              |      D0I3      +-----+
- * |                            | +--------------+                |     |
- * |                            |  resume from   |                |     |
- * +-------------------^--------+  d0i3 suspend  +----------------+     |
- *                     |                                                |
- *                     |                       D3 suspend               |
- *                     +------------------------------------------------+
+ * |       D3 (suspended)       | |              |      D0I3      |
+ * |                            | +--------------+                |
+ * |                            |  System resume |                |
+ * +----------------------------+		 +----------------+
  *
- * d0i3_suspend = s0_suspend && D0I3 stream opened,
- * D3 suspend = !d0i3_suspend,
+ * S0IX suspend: The DSP is in D0I3 if any D0I3-compatible streams
+ *		 ignored the suspend trigger. Otherwise the DSP
+ *		 is in D3.
  */
 
 static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend)
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] Applied "ASoC: SOF: Intel: hda: Amend the DSP state transition diagram" to the asoc tree

[Mark Brown]

The patch

   ASoC: SOF: Intel: hda: Amend the DSP state transition diagram

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.7

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 207bf12f642f39e749ca65d3efca9d48311e629f Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Date: Wed, 29 Jan 2020 16:07:24 -0600
Subject: [PATCH] ASoC: SOF: Intel: hda: Amend the DSP state transition diagram

Amend the DSP state transition diagram in preparation
for introducing the feature to support opportunistic
DSP D0I3 state when the system is in S0.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200129220726.31792-8-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sof/intel/hda-dsp.c | 36 +++++++++++++++++------------------
 1 file changed, 17 insertions(+), 19 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index 8c00e128a7b0..7b8425330ae0 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -488,33 +488,31 @@ int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
 /*
  * Audio DSP states may transform as below:-
  *
- *                                         D0I3 compatible stream
- *     Runtime    +---------------------+   opened only, timeout
+ *                                         Opportunistic D0I3 in S0
+ *     Runtime    +---------------------+  Delayed D0i3 work timeout
  *     suspend    |                     +--------------------+
- *   +------------+       D0(active)    |                    |
+ *   +------------+       D0I0(active)  |                    |
  *   |            |                     <---------------+    |
- *   |   +-------->                     |               |    |
- *   |   |Runtime +--^--+---------^--+--+ The last      |    |
- *   |   |resume     |  |         |  |    opened D0I3   |    |
- *   |   |           |  |         |  |    compatible    |    |
- *   |   |     resume|  |         |  |    stream closed |    |
- *   |   |      from |  | D3      |  |                  |    |
- *   |   |       D3  |  |suspend  |  | d0i3             |    |
+ *   |   +-------->                     |    New IPC	|    |
+ *   |   |Runtime +--^--+---------^--+--+ (via mailbox)	|    |
+ *   |   |resume     |  |         |  |			|    |
+ *   |   |           |  |         |  |			|    |
+ *   |   |     System|  |         |  |			|    |
+ *   |   |     resume|  | S3/S0IX |  |                  |    |
+ *   |   |	     |  | suspend |  | S0IX             |    |
  *   |   |           |  |         |  |suspend           |    |
  *   |   |           |  |         |  |                  |    |
  *   |   |           |  |         |  |                  |    |
  * +-v---+-----------+--v-------+ |  |           +------+----v----+
  * |                            | |  +----------->                |
- * |       D3 (suspended)       | |              |      D0I3      +-----+
- * |                            | +--------------+                |     |
- * |                            |  resume from   |                |     |
- * +-------------------^--------+  d0i3 suspend  +----------------+     |
- *                     |                                                |
- *                     |                       D3 suspend               |
- *                     +------------------------------------------------+
+ * |       D3 (suspended)       | |              |      D0I3      |
+ * |                            | +--------------+                |
+ * |                            |  System resume |                |
+ * +----------------------------+		 +----------------+
  *
- * d0i3_suspend = s0_suspend && D0I3 stream opened,
- * D3 suspend = !d0i3_suspend,
+ * S0IX suspend: The DSP is in D0I3 if any D0I3-compatible streams
+ *		 ignored the suspend trigger. Otherwise the DSP
+ *		 is in D3.
  */
 
 static int hda_suspend(struct snd_sof_dev *sdev, bool runtime_suspend)
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 8/9] ASoC: SOF: Intel: cnl: Implement feature to support DSP D0i3 in S0

[Pierre-Louis Bossart]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

This patch implements support for DSP D0i3 when the system
is in S0. The basic idea is to schedule a delayed work after
every successful IPC TX that checks if there are only
D0I3-compatible streams active and if so transition
the DSP to D0I3.

With the introduction of DSP D0I3 in S0, we need to
ensure that the DSP is in D0I0 before sending any new
IPCs. The exception for this would be the
compact IPCs that are used to set the DSP in
D0I3/D0I0 states.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/cnl.c     | 37 +++++++++++++++++++++++++++------
 sound/soc/sof/intel/hda-dsp.c | 39 +++++++++++++++++++++++++++++++++--
 sound/soc/sof/intel/hda.c     |  5 +++++
 sound/soc/sof/intel/hda.h     | 11 ++++++++++
 sound/soc/sof/ipc.c           | 29 ++++++++++++++++++++++++--
 sound/soc/sof/sof-priv.h      |  3 +++
 6 files changed, 114 insertions(+), 10 deletions(-)

diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c
index 9e2d8afe0535..8a59fec72919 100644
--- a/sound/soc/sof/intel/cnl.c
+++ b/sound/soc/sof/intel/cnl.c
@@ -171,23 +171,48 @@ static bool cnl_compact_ipc_compress(struct snd_sof_ipc_msg *msg,
 static int cnl_ipc_send_msg(struct snd_sof_dev *sdev,
 			    struct snd_sof_ipc_msg *msg)
 {
+	struct sof_intel_hda_dev *hdev = sdev->pdata->hw_pdata;
+	struct sof_ipc_cmd_hdr *hdr;
 	u32 dr = 0;
 	u32 dd = 0;
 
+	/*
+	 * Currently the only compact IPC supported is the PM_GATE
+	 * IPC which is used for transitioning the DSP between the
+	 * D0I0 and D0I3 states. And these are sent only during the
+	 * set_power_state() op. Therefore, there will never be a case
+	 * that a compact IPC results in the DSP exiting D0I3 without
+	 * the host and FW being in sync.
+	 */
 	if (cnl_compact_ipc_compress(msg, &dr, &dd)) {
 		/* send the message via IPC registers */
 		snd_sof_dsp_write(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDD,
 				  dd);
 		snd_sof_dsp_write(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDR,
 				  CNL_DSP_REG_HIPCIDR_BUSY | dr);
-	} else {
-		/* send the message via mailbox */
-		sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data,
-				  msg->msg_size);
-		snd_sof_dsp_write(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDR,
-				  CNL_DSP_REG_HIPCIDR_BUSY);
+		return 0;
 	}
 
+	/* send the message via mailbox */
+	sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data,
+			  msg->msg_size);
+	snd_sof_dsp_write(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDR,
+			  CNL_DSP_REG_HIPCIDR_BUSY);
+
+	hdr = msg->msg_data;
+
+	/*
+	 * Use mod_delayed_work() to schedule the delayed work
+	 * to avoid scheduling multiple workqueue items when
+	 * IPCs are sent at a high-rate. mod_delayed_work()
+	 * modifies the timer if the work is pending.
+	 * Also, a new delayed work should not be queued after the
+	 * the CTX_SAVE IPC, which is sent before the DSP enters D3.
+	 */
+	if (hdr->cmd != (SOF_IPC_GLB_PM_MSG | SOF_IPC_PM_CTX_SAVE))
+		mod_delayed_work(system_wq, &hdev->d0i3_work,
+				 msecs_to_jiffies(SOF_HDA_D0I3_WORK_DELAY_MS));
+
 	return 0;
 }
 
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index 7b8425330ae0..ee604be715b9 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -17,6 +17,7 @@
 
 #include <sound/hdaudio_ext.h>
 #include <sound/hda_register.h>
+#include "../sof-audio.h"
 #include "../ops.h"
 #include "hda.h"
 #include "hda-ipc.h"
@@ -334,8 +335,9 @@ static int hda_dsp_send_pm_gate_ipc(struct snd_sof_dev *sdev, u32 flags)
 	pm_gate.flags = flags;
 
 	/* send pm_gate ipc to dsp */
-	return sof_ipc_tx_message(sdev->ipc, pm_gate.hdr.cmd, &pm_gate,
-				  sizeof(pm_gate), &reply, sizeof(reply));
+	return sof_ipc_tx_message_no_pm(sdev->ipc, pm_gate.hdr.cmd,
+					&pm_gate, sizeof(pm_gate), &reply,
+					sizeof(reply));
 }
 
 static int hda_dsp_update_d0i3c_register(struct snd_sof_dev *sdev, u8 value)
@@ -706,6 +708,9 @@ int hda_dsp_suspend(struct snd_sof_dev *sdev, u32 target_state)
 	};
 	int ret;
 
+	/* cancel any attempt for DSP D0I3 */
+	cancel_delayed_work_sync(&hda->d0i3_work);
+
 	if (target_state == SOF_DSP_PM_D0) {
 		/* Set DSP power state */
 		ret = hda_dsp_set_power_state(sdev, &target_dsp_state);
@@ -780,3 +785,33 @@ int hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev)
 #endif
 	return 0;
 }
+
+void hda_dsp_d0i3_work(struct work_struct *work)
+{
+	struct sof_intel_hda_dev *hdev = container_of(work,
+						      struct sof_intel_hda_dev,
+						      d0i3_work.work);
+	struct hdac_bus *bus = &hdev->hbus.core;
+	struct snd_sof_dev *sdev = dev_get_drvdata(bus->dev);
+	struct sof_dsp_power_state target_state;
+	int ret;
+
+	target_state.state = SOF_DSP_PM_D0;
+
+	/* DSP can enter D0I3 iff only D0I3-compatible streams are active */
+	if (snd_sof_dsp_only_d0i3_compatible_stream_active(sdev))
+		target_state.substate = SOF_HDA_DSP_PM_D0I3;
+	else
+		target_state.substate = SOF_HDA_DSP_PM_D0I0;
+
+	/* remain in D0I0 */
+	if (target_state.substate == SOF_HDA_DSP_PM_D0I0)
+		return;
+
+	/* This can fail but error cannot be propagated */
+	ret = hda_dsp_set_power_state(sdev, &target_state);
+	if (ret < 0)
+		dev_err_ratelimited(sdev->dev,
+				    "error: failed to set DSP state %d substate %d\n",
+				    target_state.state, target_state.substate);
+}
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 65b86dd044f1..2b8754a76584 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -598,6 +598,8 @@ int hda_dsp_probe(struct snd_sof_dev *sdev)
 	/* set default mailbox offset for FW ready message */
 	sdev->dsp_box.offset = HDA_DSP_MBOX_UPLINK_OFFSET;
 
+	INIT_DELAYED_WORK(&hdev->d0i3_work, hda_dsp_d0i3_work);
+
 	return 0;
 
 free_ipc_irq:
@@ -622,6 +624,9 @@ int hda_dsp_remove(struct snd_sof_dev *sdev)
 	struct pci_dev *pci = to_pci_dev(sdev->dev);
 	const struct sof_intel_dsp_desc *chip = hda->desc;
 
+	/* cancel any attempt for DSP D0I3 */
+	cancel_delayed_work_sync(&hda->d0i3_work);
+
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
 	/* codec removal, invoke bus_device_remove */
 	snd_hdac_ext_bus_device_remove(bus);
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 02c2a7eadb1b..a46b66437a3d 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -392,6 +392,13 @@ struct sof_intel_dsp_bdl {
 #define SOF_HDA_PLAYBACK		0
 #define SOF_HDA_CAPTURE			1
 
+/*
+ * Time in ms for opportunistic D0I3 entry delay.
+ * This has been deliberately chosen to be long to avoid race conditions.
+ * Could be optimized in future.
+ */
+#define SOF_HDA_D0I3_WORK_DELAY_MS	5000
+
 /* HDA DSP D0 substate */
 enum sof_hda_D0_substate {
 	SOF_HDA_DSP_PM_D0I0,	/* default D0 substate */
@@ -420,6 +427,9 @@ struct sof_intel_hda_dev {
 
 	/* DMIC device */
 	struct platform_device *dmic_dev;
+
+	/* delayed work to enter D0I3 opportunistically */
+	struct delayed_work d0i3_work;
 };
 
 static inline struct hdac_bus *sof_to_bus(struct snd_sof_dev *s)
@@ -487,6 +497,7 @@ void hda_dsp_dump_skl(struct snd_sof_dev *sdev, u32 flags);
 void hda_dsp_dump(struct snd_sof_dev *sdev, u32 flags);
 void hda_ipc_dump(struct snd_sof_dev *sdev);
 void hda_ipc_irq_dump(struct snd_sof_dev *sdev);
+void hda_dsp_d0i3_work(struct work_struct *work);
 
 /*
  * DSP PCM Operations.
diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c
index b63fc529b456..22d296f95761 100644
--- a/sound/soc/sof/ipc.c
+++ b/sound/soc/sof/ipc.c
@@ -268,7 +268,6 @@ static int sof_ipc_tx_message_unlocked(struct snd_sof_ipc *ipc, u32 header,
 	spin_unlock_irq(&sdev->ipc_lock);
 
 	if (ret < 0) {
-		/* So far IPC TX never fails, consider making the above void */
 		dev_err_ratelimited(sdev->dev,
 				    "error: ipc tx failed with error %d\n",
 				    ret);
@@ -288,6 +287,32 @@ static int sof_ipc_tx_message_unlocked(struct snd_sof_ipc *ipc, u32 header,
 int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header,
 		       void *msg_data, size_t msg_bytes, void *reply_data,
 		       size_t reply_bytes)
+{
+	const struct sof_dsp_power_state target_state = {
+		.state = SOF_DSP_PM_D0,
+	};
+	int ret;
+
+	/* ensure the DSP is in D0 before sending a new IPC */
+	ret = snd_sof_dsp_set_power_state(ipc->sdev, &target_state);
+	if (ret < 0) {
+		dev_err(ipc->sdev->dev, "error: resuming DSP %d\n", ret);
+		return ret;
+	}
+
+	return sof_ipc_tx_message_no_pm(ipc, header, msg_data, msg_bytes,
+					reply_data, reply_bytes);
+}
+EXPORT_SYMBOL(sof_ipc_tx_message);
+
+/*
+ * send IPC message from host to DSP without modifying the DSP state.
+ * This will be used for IPC's that can be handled by the DSP
+ * even in a low-power D0 substate.
+ */
+int sof_ipc_tx_message_no_pm(struct snd_sof_ipc *ipc, u32 header,
+			     void *msg_data, size_t msg_bytes,
+			     void *reply_data, size_t reply_bytes)
 {
 	int ret;
 
@@ -305,7 +330,7 @@ int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header,
 
 	return ret;
 }
-EXPORT_SYMBOL(sof_ipc_tx_message);
+EXPORT_SYMBOL(sof_ipc_tx_message_no_pm);
 
 /* handle reply message from DSP */
 int snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id)
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index ef33aaadbc31..00084471d0de 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -470,6 +470,9 @@ int snd_sof_ipc_valid(struct snd_sof_dev *sdev);
 int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header,
 		       void *msg_data, size_t msg_bytes, void *reply_data,
 		       size_t reply_bytes);
+int sof_ipc_tx_message_no_pm(struct snd_sof_ipc *ipc, u32 header,
+			     void *msg_data, size_t msg_bytes,
+			     void *reply_data, size_t reply_bytes);
 
 /*
  * Trace/debug
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] Applied "ASoC: SOF: Intel: cnl: Implement feature to support DSP D0i3 in S0" to the asoc tree

[Mark Brown]

The patch

   ASoC: SOF: Intel: cnl: Implement feature to support DSP D0i3 in S0

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.7

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 63e51fd33fef04b634a0c32ae491ab16a19cb17c Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Date: Wed, 29 Jan 2020 16:07:25 -0600
Subject: [PATCH] ASoC: SOF: Intel: cnl: Implement feature to support DSP D0i3
 in S0

This patch implements support for DSP D0i3 when the system
is in S0. The basic idea is to schedule a delayed work after
every successful IPC TX that checks if there are only
D0I3-compatible streams active and if so transition
the DSP to D0I3.

With the introduction of DSP D0I3 in S0, we need to
ensure that the DSP is in D0I0 before sending any new
IPCs. The exception for this would be the
compact IPCs that are used to set the DSP in
D0I3/D0I0 states.

Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200129220726.31792-9-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sof/intel/cnl.c     | 37 +++++++++++++++++++++++++++------
 sound/soc/sof/intel/hda-dsp.c | 39 +++++++++++++++++++++++++++++++++--
 sound/soc/sof/intel/hda.c     |  5 +++++
 sound/soc/sof/intel/hda.h     | 11 ++++++++++
 sound/soc/sof/ipc.c           | 29 ++++++++++++++++++++++++--
 sound/soc/sof/sof-priv.h      |  3 +++
 6 files changed, 114 insertions(+), 10 deletions(-)

diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c
index 9e2d8afe0535..8a59fec72919 100644
--- a/sound/soc/sof/intel/cnl.c
+++ b/sound/soc/sof/intel/cnl.c
@@ -171,23 +171,48 @@ static bool cnl_compact_ipc_compress(struct snd_sof_ipc_msg *msg,
 static int cnl_ipc_send_msg(struct snd_sof_dev *sdev,
 			    struct snd_sof_ipc_msg *msg)
 {
+	struct sof_intel_hda_dev *hdev = sdev->pdata->hw_pdata;
+	struct sof_ipc_cmd_hdr *hdr;
 	u32 dr = 0;
 	u32 dd = 0;
 
+	/*
+	 * Currently the only compact IPC supported is the PM_GATE
+	 * IPC which is used for transitioning the DSP between the
+	 * D0I0 and D0I3 states. And these are sent only during the
+	 * set_power_state() op. Therefore, there will never be a case
+	 * that a compact IPC results in the DSP exiting D0I3 without
+	 * the host and FW being in sync.
+	 */
 	if (cnl_compact_ipc_compress(msg, &dr, &dd)) {
 		/* send the message via IPC registers */
 		snd_sof_dsp_write(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDD,
 				  dd);
 		snd_sof_dsp_write(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDR,
 				  CNL_DSP_REG_HIPCIDR_BUSY | dr);
-	} else {
-		/* send the message via mailbox */
-		sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data,
-				  msg->msg_size);
-		snd_sof_dsp_write(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDR,
-				  CNL_DSP_REG_HIPCIDR_BUSY);
+		return 0;
 	}
 
+	/* send the message via mailbox */
+	sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data,
+			  msg->msg_size);
+	snd_sof_dsp_write(sdev, HDA_DSP_BAR, CNL_DSP_REG_HIPCIDR,
+			  CNL_DSP_REG_HIPCIDR_BUSY);
+
+	hdr = msg->msg_data;
+
+	/*
+	 * Use mod_delayed_work() to schedule the delayed work
+	 * to avoid scheduling multiple workqueue items when
+	 * IPCs are sent at a high-rate. mod_delayed_work()
+	 * modifies the timer if the work is pending.
+	 * Also, a new delayed work should not be queued after the
+	 * the CTX_SAVE IPC, which is sent before the DSP enters D3.
+	 */
+	if (hdr->cmd != (SOF_IPC_GLB_PM_MSG | SOF_IPC_PM_CTX_SAVE))
+		mod_delayed_work(system_wq, &hdev->d0i3_work,
+				 msecs_to_jiffies(SOF_HDA_D0I3_WORK_DELAY_MS));
+
 	return 0;
 }
 
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index 7b8425330ae0..ee604be715b9 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -17,6 +17,7 @@
 
 #include <sound/hdaudio_ext.h>
 #include <sound/hda_register.h>
+#include "../sof-audio.h"
 #include "../ops.h"
 #include "hda.h"
 #include "hda-ipc.h"
@@ -334,8 +335,9 @@ static int hda_dsp_send_pm_gate_ipc(struct snd_sof_dev *sdev, u32 flags)
 	pm_gate.flags = flags;
 
 	/* send pm_gate ipc to dsp */
-	return sof_ipc_tx_message(sdev->ipc, pm_gate.hdr.cmd, &pm_gate,
-				  sizeof(pm_gate), &reply, sizeof(reply));
+	return sof_ipc_tx_message_no_pm(sdev->ipc, pm_gate.hdr.cmd,
+					&pm_gate, sizeof(pm_gate), &reply,
+					sizeof(reply));
 }
 
 static int hda_dsp_update_d0i3c_register(struct snd_sof_dev *sdev, u8 value)
@@ -706,6 +708,9 @@ int hda_dsp_suspend(struct snd_sof_dev *sdev, u32 target_state)
 	};
 	int ret;
 
+	/* cancel any attempt for DSP D0I3 */
+	cancel_delayed_work_sync(&hda->d0i3_work);
+
 	if (target_state == SOF_DSP_PM_D0) {
 		/* Set DSP power state */
 		ret = hda_dsp_set_power_state(sdev, &target_dsp_state);
@@ -780,3 +785,33 @@ int hda_dsp_set_hw_params_upon_resume(struct snd_sof_dev *sdev)
 #endif
 	return 0;
 }
+
+void hda_dsp_d0i3_work(struct work_struct *work)
+{
+	struct sof_intel_hda_dev *hdev = container_of(work,
+						      struct sof_intel_hda_dev,
+						      d0i3_work.work);
+	struct hdac_bus *bus = &hdev->hbus.core;
+	struct snd_sof_dev *sdev = dev_get_drvdata(bus->dev);
+	struct sof_dsp_power_state target_state;
+	int ret;
+
+	target_state.state = SOF_DSP_PM_D0;
+
+	/* DSP can enter D0I3 iff only D0I3-compatible streams are active */
+	if (snd_sof_dsp_only_d0i3_compatible_stream_active(sdev))
+		target_state.substate = SOF_HDA_DSP_PM_D0I3;
+	else
+		target_state.substate = SOF_HDA_DSP_PM_D0I0;
+
+	/* remain in D0I0 */
+	if (target_state.substate == SOF_HDA_DSP_PM_D0I0)
+		return;
+
+	/* This can fail but error cannot be propagated */
+	ret = hda_dsp_set_power_state(sdev, &target_state);
+	if (ret < 0)
+		dev_err_ratelimited(sdev->dev,
+				    "error: failed to set DSP state %d substate %d\n",
+				    target_state.state, target_state.substate);
+}
diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c
index 65b86dd044f1..2b8754a76584 100644
--- a/sound/soc/sof/intel/hda.c
+++ b/sound/soc/sof/intel/hda.c
@@ -598,6 +598,8 @@ int hda_dsp_probe(struct snd_sof_dev *sdev)
 	/* set default mailbox offset for FW ready message */
 	sdev->dsp_box.offset = HDA_DSP_MBOX_UPLINK_OFFSET;
 
+	INIT_DELAYED_WORK(&hdev->d0i3_work, hda_dsp_d0i3_work);
+
 	return 0;
 
 free_ipc_irq:
@@ -622,6 +624,9 @@ int hda_dsp_remove(struct snd_sof_dev *sdev)
 	struct pci_dev *pci = to_pci_dev(sdev->dev);
 	const struct sof_intel_dsp_desc *chip = hda->desc;
 
+	/* cancel any attempt for DSP D0I3 */
+	cancel_delayed_work_sync(&hda->d0i3_work);
+
 #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
 	/* codec removal, invoke bus_device_remove */
 	snd_hdac_ext_bus_device_remove(bus);
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 02c2a7eadb1b..a46b66437a3d 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -392,6 +392,13 @@ struct sof_intel_dsp_bdl {
 #define SOF_HDA_PLAYBACK		0
 #define SOF_HDA_CAPTURE			1
 
+/*
+ * Time in ms for opportunistic D0I3 entry delay.
+ * This has been deliberately chosen to be long to avoid race conditions.
+ * Could be optimized in future.
+ */
+#define SOF_HDA_D0I3_WORK_DELAY_MS	5000
+
 /* HDA DSP D0 substate */
 enum sof_hda_D0_substate {
 	SOF_HDA_DSP_PM_D0I0,	/* default D0 substate */
@@ -420,6 +427,9 @@ struct sof_intel_hda_dev {
 
 	/* DMIC device */
 	struct platform_device *dmic_dev;
+
+	/* delayed work to enter D0I3 opportunistically */
+	struct delayed_work d0i3_work;
 };
 
 static inline struct hdac_bus *sof_to_bus(struct snd_sof_dev *s)
@@ -487,6 +497,7 @@ void hda_dsp_dump_skl(struct snd_sof_dev *sdev, u32 flags);
 void hda_dsp_dump(struct snd_sof_dev *sdev, u32 flags);
 void hda_ipc_dump(struct snd_sof_dev *sdev);
 void hda_ipc_irq_dump(struct snd_sof_dev *sdev);
+void hda_dsp_d0i3_work(struct work_struct *work);
 
 /*
  * DSP PCM Operations.
diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c
index b63fc529b456..22d296f95761 100644
--- a/sound/soc/sof/ipc.c
+++ b/sound/soc/sof/ipc.c
@@ -268,7 +268,6 @@ static int sof_ipc_tx_message_unlocked(struct snd_sof_ipc *ipc, u32 header,
 	spin_unlock_irq(&sdev->ipc_lock);
 
 	if (ret < 0) {
-		/* So far IPC TX never fails, consider making the above void */
 		dev_err_ratelimited(sdev->dev,
 				    "error: ipc tx failed with error %d\n",
 				    ret);
@@ -288,6 +287,32 @@ static int sof_ipc_tx_message_unlocked(struct snd_sof_ipc *ipc, u32 header,
 int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header,
 		       void *msg_data, size_t msg_bytes, void *reply_data,
 		       size_t reply_bytes)
+{
+	const struct sof_dsp_power_state target_state = {
+		.state = SOF_DSP_PM_D0,
+	};
+	int ret;
+
+	/* ensure the DSP is in D0 before sending a new IPC */
+	ret = snd_sof_dsp_set_power_state(ipc->sdev, &target_state);
+	if (ret < 0) {
+		dev_err(ipc->sdev->dev, "error: resuming DSP %d\n", ret);
+		return ret;
+	}
+
+	return sof_ipc_tx_message_no_pm(ipc, header, msg_data, msg_bytes,
+					reply_data, reply_bytes);
+}
+EXPORT_SYMBOL(sof_ipc_tx_message);
+
+/*
+ * send IPC message from host to DSP without modifying the DSP state.
+ * This will be used for IPC's that can be handled by the DSP
+ * even in a low-power D0 substate.
+ */
+int sof_ipc_tx_message_no_pm(struct snd_sof_ipc *ipc, u32 header,
+			     void *msg_data, size_t msg_bytes,
+			     void *reply_data, size_t reply_bytes)
 {
 	int ret;
 
@@ -305,7 +330,7 @@ int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header,
 
 	return ret;
 }
-EXPORT_SYMBOL(sof_ipc_tx_message);
+EXPORT_SYMBOL(sof_ipc_tx_message_no_pm);
 
 /* handle reply message from DSP */
 int snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id)
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index ef33aaadbc31..00084471d0de 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -470,6 +470,9 @@ int snd_sof_ipc_valid(struct snd_sof_dev *sdev);
 int sof_ipc_tx_message(struct snd_sof_ipc *ipc, u32 header,
 		       void *msg_data, size_t msg_bytes, void *reply_data,
 		       size_t reply_bytes);
+int sof_ipc_tx_message_no_pm(struct snd_sof_ipc *ipc, u32 header,
+			     void *msg_data, size_t msg_bytes,
+			     void *reply_data, size_t reply_bytes);
 
 /*
  * Trace/debug
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] [PATCH 9/9] ASoC: SOF: Intel: hda: Allow trace DMA in S0 when DSP is in D0I3 for debug

[Pierre-Louis Bossart]

From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Trace DMA is disabled by default when the DSP is in D0I3.
Add a debug option to keep trace DMA enabled when the DSP
is in D0I3 during S0.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/intel/hda-dsp.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index ee604be715b9..14228b4931d6 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -15,6 +15,7 @@
  * Hardware interface for generic Intel audio DSP HDA IP
  */
 
+#include <linux/module.h>
 #include <sound/hdaudio_ext.h>
 #include <sound/hda_register.h>
 #include "../sof-audio.h"
@@ -22,6 +23,13 @@
 #include "hda.h"
 #include "hda-ipc.h"
 
+static bool hda_enable_trace_D0I3_S0;
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG)
+module_param_named(enable_trace_D0I3_S0, hda_enable_trace_D0I3_S0, bool, 0444);
+MODULE_PARM_DESC(enable_trace_D0I3_S0,
+		 "SOF HDA enable trace when the DSP is in D0I3 in S0");
+#endif
+
 /*
  * DSP Core control.
  */
@@ -399,8 +407,14 @@ static int hda_dsp_set_D0_state(struct snd_sof_dev *sdev,
 	if (target_state->substate == SOF_HDA_DSP_PM_D0I3) {
 		value = SOF_HDA_VS_D0I3C_I3;
 
-		/* disable DMA trace in D0I3 */
-		flags = HDA_PM_NO_DMA_TRACE;
+		/*
+		 * Trace DMA is disabled by default when the DSP enters D0I3.
+		 * But it can be kept enabled when the DSP enters D0I3 while the
+		 * system is in S0 for debug.
+		 */
+		if (hda_enable_trace_D0I3_S0 &&
+		    sdev->system_suspend_target != SOF_SUSPEND_NONE)
+			flags = HDA_PM_NO_DMA_TRACE;
 	} else {
 		/* prevent power gating in D0I0 */
 		flags = HDA_PM_PPG;
@@ -450,11 +464,26 @@ int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
 {
 	int ret = 0;
 
-	/* Nothing to do if the DSP is already in the requested state */
+	/*
+	 * When the DSP is already in D0I3 and the target state is D0I3,
+	 * it could be the case that the DSP is in D0I3 during S0
+	 * and the system is suspending to S0Ix. Therefore,
+	 * hda_dsp_set_D0_state() must be called to disable trace DMA
+	 * by sending the PM_GATE IPC to the FW.
+	 */
+	if (target_state->substate == SOF_HDA_DSP_PM_D0I3 &&
+	    sdev->system_suspend_target == SOF_SUSPEND_S0IX)
+		goto set_state;
+
+	/*
+	 * For all other cases, return without doing anything if
+	 * the DSP is already in the target state.
+	 */
 	if (target_state->state == sdev->dsp_power_state.state &&
 	    target_state->substate == sdev->dsp_power_state.substate)
 		return 0;
 
+set_state:
 	switch (target_state->state) {
 	case SOF_DSP_PM_D0:
 		ret = hda_dsp_set_D0_state(sdev, target_state);
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel

[alsa-devel] Applied "ASoC: SOF: Intel: hda: Allow trace DMA in S0 when DSP is in D0I3 for debug" to the asoc tree

[Mark Brown]

The patch

   ASoC: SOF: Intel: hda: Allow trace DMA in S0 when DSP is in D0I3 for debug

has been applied to the asoc tree at

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-5.7

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 851fd87324430dfe56cd55dfd05a8114ac82d168 Mon Sep 17 00:00:00 2001
From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Date: Wed, 29 Jan 2020 16:07:26 -0600
Subject: [PATCH] ASoC: SOF: Intel: hda: Allow trace DMA in S0 when DSP is in
 D0I3 for debug

Trace DMA is disabled by default when the DSP is in D0I3.
Add a debug option to keep trace DMA enabled when the DSP
is in D0I3 during S0.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Signed-off-by: Keyon Jie <yang.jie@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Link: https://lore.kernel.org/r/20200129220726.31792-10-pierre-louis.bossart@linux.intel.com
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/sof/intel/hda-dsp.c | 35 ++++++++++++++++++++++++++++++++---
 1 file changed, 32 insertions(+), 3 deletions(-)

diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index ee604be715b9..14228b4931d6 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -15,6 +15,7 @@
  * Hardware interface for generic Intel audio DSP HDA IP
  */
 
+#include <linux/module.h>
 #include <sound/hdaudio_ext.h>
 #include <sound/hda_register.h>
 #include "../sof-audio.h"
@@ -22,6 +23,13 @@
 #include "hda.h"
 #include "hda-ipc.h"
 
+static bool hda_enable_trace_D0I3_S0;
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_DEBUG)
+module_param_named(enable_trace_D0I3_S0, hda_enable_trace_D0I3_S0, bool, 0444);
+MODULE_PARM_DESC(enable_trace_D0I3_S0,
+		 "SOF HDA enable trace when the DSP is in D0I3 in S0");
+#endif
+
 /*
  * DSP Core control.
  */
@@ -399,8 +407,14 @@ static int hda_dsp_set_D0_state(struct snd_sof_dev *sdev,
 	if (target_state->substate == SOF_HDA_DSP_PM_D0I3) {
 		value = SOF_HDA_VS_D0I3C_I3;
 
-		/* disable DMA trace in D0I3 */
-		flags = HDA_PM_NO_DMA_TRACE;
+		/*
+		 * Trace DMA is disabled by default when the DSP enters D0I3.
+		 * But it can be kept enabled when the DSP enters D0I3 while the
+		 * system is in S0 for debug.
+		 */
+		if (hda_enable_trace_D0I3_S0 &&
+		    sdev->system_suspend_target != SOF_SUSPEND_NONE)
+			flags = HDA_PM_NO_DMA_TRACE;
 	} else {
 		/* prevent power gating in D0I0 */
 		flags = HDA_PM_PPG;
@@ -450,11 +464,26 @@ int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
 {
 	int ret = 0;
 
-	/* Nothing to do if the DSP is already in the requested state */
+	/*
+	 * When the DSP is already in D0I3 and the target state is D0I3,
+	 * it could be the case that the DSP is in D0I3 during S0
+	 * and the system is suspending to S0Ix. Therefore,
+	 * hda_dsp_set_D0_state() must be called to disable trace DMA
+	 * by sending the PM_GATE IPC to the FW.
+	 */
+	if (target_state->substate == SOF_HDA_DSP_PM_D0I3 &&
+	    sdev->system_suspend_target == SOF_SUSPEND_S0IX)
+		goto set_state;
+
+	/*
+	 * For all other cases, return without doing anything if
+	 * the DSP is already in the target state.
+	 */
 	if (target_state->state == sdev->dsp_power_state.state &&
 	    target_state->substate == sdev->dsp_power_state.substate)
 		return 0;
 
+set_state:
 	switch (target_state->state) {
 	case SOF_DSP_PM_D0:
 		ret = hda_dsp_set_D0_state(sdev, target_state);
-- 
2.20.1

_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
https://mailman.alsa-project.org/mailman/listinfo/alsa-devel