diff mbox series

ASoC: SOF: Intel: Split the set_power_op for IPC3 and IPC4

Message ID 20230420104714.29573-1-peter.ujfalusi@linux.intel.com (mailing list archive)
State Accepted
Commit 996b07efe49620325332081afdb0dc0bd6fe5cd0
Headers show
Series ASoC: SOF: Intel: Split the set_power_op for IPC3 and IPC4 | expand

Commit Message

Peter Ujfalusi April 20, 2023, 10:47 a.m. UTC 
From: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>

Suspending to S0iX with IPC3 requires the PM_GATE IPC to be sent again
to stop the DMA trace. But with IPC4, this is not needed as the trace is
stopped with the LARGE_CONFIG_SET IPC. Also, sending the MOD_D0IX IPC to
set the D0I3 state again when the DSP is in D0I3 already results in an
imbalance in PM runtime states in the firmware. So split the
set_power_state ops for IPC3 and IPC4 to avoid sending the MOD_D0IX IPC
when the DSP is already in D0I3 with IPC4.

Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Rander Wang <rander.wang@intel.com>
Reviewed-by: Bard Liao <yung-chuan.liao@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
Mark,

last minute patch for 6.4 as the issue this patch addressing was discovered
and fixed recently for IPC4.

Regards,
Peter

 sound/soc/sof/intel/apl.c            |  4 ++
 sound/soc/sof/intel/cnl.c            |  4 ++
 sound/soc/sof/intel/hda-common-ops.c |  1 -
 sound/soc/sof/intel/hda-dsp.c        | 60 ++++++++++++++++++----------
 sound/soc/sof/intel/hda.h            |  6 ++-
 sound/soc/sof/intel/icl.c            |  4 ++
 sound/soc/sof/intel/mtl.c            |  2 +
 sound/soc/sof/intel/tgl.c            |  4 ++
 8 files changed, 60 insertions(+), 25 deletions(-)

Comments

Mark Brown April 21, 2023, 4:06 p.m. UTC | #1 
On Thu, 20 Apr 2023 13:47:14 +0300, Peter Ujfalusi wrote:
> Suspending to S0iX with IPC3 requires the PM_GATE IPC to be sent again
> to stop the DMA trace. But with IPC4, this is not needed as the trace is
> stopped with the LARGE_CONFIG_SET IPC. Also, sending the MOD_D0IX IPC to
> set the D0I3 state again when the DSP is in D0I3 already results in an
> imbalance in PM runtime states in the firmware. So split the
> set_power_state ops for IPC3 and IPC4 to avoid sending the MOD_D0IX IPC
> when the DSP is already in D0I3 with IPC4.
> 
> [...]

Applied to

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

Thanks!

[1/1] ASoC: SOF: Intel: Split the set_power_op for IPC3 and IPC4
      commit: 996b07efe49620325332081afdb0dc0bd6fe5cd0

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
diff mbox series

Patch

diff --git a/sound/soc/sof/intel/apl.c b/sound/soc/sof/intel/apl.c
index 0e7a7e4ad976..e1f25a8f0c32 100644
--- a/sound/soc/sof/intel/apl.c
+++ b/sound/soc/sof/intel/apl.c
@@ -48,6 +48,8 @@  int sof_apl_ops_init(struct snd_sof_dev *sdev)
 
 		/* debug */
 		sof_apl_ops.ipc_dump	= hda_ipc_dump;
+
+		sof_apl_ops.set_power_state = hda_dsp_set_power_state_ipc3;
 	}
 
 	if (sdev->pdata->ipc_type == SOF_INTEL_IPC4) {
@@ -73,6 +75,8 @@  int sof_apl_ops_init(struct snd_sof_dev *sdev)
 
 		/* debug */
 		sof_apl_ops.ipc_dump	= hda_ipc4_dump;
+
+		sof_apl_ops.set_power_state = hda_dsp_set_power_state_ipc4;
 	}
 
 	/* set DAI driver ops */
diff --git a/sound/soc/sof/intel/cnl.c b/sound/soc/sof/intel/cnl.c
index a08a77fa946b..a95222e53ecf 100644
--- a/sound/soc/sof/intel/cnl.c
+++ b/sound/soc/sof/intel/cnl.c
@@ -395,6 +395,8 @@  int sof_cnl_ops_init(struct snd_sof_dev *sdev)
 
 		/* debug */
 		sof_cnl_ops.ipc_dump	= cnl_ipc_dump;
+
+		sof_cnl_ops.set_power_state = hda_dsp_set_power_state_ipc3;
 	}
 
 	if (sdev->pdata->ipc_type == SOF_INTEL_IPC4) {
@@ -420,6 +422,8 @@  int sof_cnl_ops_init(struct snd_sof_dev *sdev)
 
 		/* debug */
 		sof_cnl_ops.ipc_dump	= cnl_ipc4_dump;
+
+		sof_cnl_ops.set_power_state = hda_dsp_set_power_state_ipc4;
 	}
 
 	/* set DAI driver ops */
diff --git a/sound/soc/sof/intel/hda-common-ops.c b/sound/soc/sof/intel/hda-common-ops.c
index 397303b3ac9d..8e1cd0babd32 100644
--- a/sound/soc/sof/intel/hda-common-ops.c
+++ b/sound/soc/sof/intel/hda-common-ops.c
@@ -89,7 +89,6 @@  struct snd_sof_dsp_ops sof_hda_common_ops = {
 	.runtime_resume		= hda_dsp_runtime_resume,
 	.runtime_idle		= hda_dsp_runtime_idle,
 	.set_hw_params_upon_resume = hda_dsp_set_hw_params_upon_resume,
-	.set_power_state	= hda_dsp_set_power_state,
 
 	/* ALSA HW info flags */
 	.hw_info =	SNDRV_PCM_INFO_MMAP |
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c
index 77df536cf901..44f39a520bb3 100644
--- a/sound/soc/sof/intel/hda-dsp.c
+++ b/sound/soc/sof/intel/hda-dsp.c
@@ -574,31 +574,11 @@  static void hda_dsp_state_log(struct snd_sof_dev *sdev)
  * 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)
+static int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
+				   const struct sof_dsp_power_state *target_state)
 {
 	int ret = 0;
 
-	/*
-	 * 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);
@@ -630,6 +610,42 @@  int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
 	return ret;
 }
 
+int hda_dsp_set_power_state_ipc3(struct snd_sof_dev *sdev,
+				 const struct sof_dsp_power_state *target_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)
+		return hda_dsp_set_power_state(sdev, target_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;
+
+	return hda_dsp_set_power_state(sdev, target_state);
+}
+
+int hda_dsp_set_power_state_ipc4(struct snd_sof_dev *sdev,
+				 const struct sof_dsp_power_state *target_state)
+{
+	/* 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;
+
+	return hda_dsp_set_power_state(sdev, target_state);
+}
+
 /*
  * Audio DSP states may transform as below:-
  *
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h
index 0e0cfa81a8f2..c4befacde23e 100644
--- a/sound/soc/sof/intel/hda.h
+++ b/sound/soc/sof/intel/hda.h
@@ -584,8 +584,10 @@  void hda_dsp_ipc_int_enable(struct snd_sof_dev *sdev);
 void hda_dsp_ipc_int_disable(struct snd_sof_dev *sdev);
 bool hda_dsp_core_is_enabled(struct snd_sof_dev *sdev, unsigned int core_mask);
 
-int hda_dsp_set_power_state(struct snd_sof_dev *sdev,
-			    const struct sof_dsp_power_state *target_state);
+int hda_dsp_set_power_state_ipc3(struct snd_sof_dev *sdev,
+				 const struct sof_dsp_power_state *target_state);
+int hda_dsp_set_power_state_ipc4(struct snd_sof_dev *sdev,
+				 const struct sof_dsp_power_state *target_state);
 
 int hda_dsp_suspend(struct snd_sof_dev *sdev, u32 target_state);
 int hda_dsp_resume(struct snd_sof_dev *sdev);
diff --git a/sound/soc/sof/intel/icl.c b/sound/soc/sof/intel/icl.c
index 435941a1692f..0f249efc6a5a 100644
--- a/sound/soc/sof/intel/icl.c
+++ b/sound/soc/sof/intel/icl.c
@@ -116,6 +116,8 @@  int sof_icl_ops_init(struct snd_sof_dev *sdev)
 
 		/* debug */
 		sof_icl_ops.ipc_dump	= cnl_ipc_dump;
+
+		sof_icl_ops.set_power_state = hda_dsp_set_power_state_ipc3;
 	}
 
 	if (sdev->pdata->ipc_type == SOF_INTEL_IPC4) {
@@ -141,6 +143,8 @@  int sof_icl_ops_init(struct snd_sof_dev *sdev)
 
 		/* debug */
 		sof_icl_ops.ipc_dump	= cnl_ipc4_dump;
+
+		sof_icl_ops.set_power_state = hda_dsp_set_power_state_ipc4;
 	}
 
 	/* debug */
diff --git a/sound/soc/sof/intel/mtl.c b/sound/soc/sof/intel/mtl.c
index 9f969e07fc27..46caf3ccde66 100644
--- a/sound/soc/sof/intel/mtl.c
+++ b/sound/soc/sof/intel/mtl.c
@@ -668,6 +668,8 @@  int sof_mtl_ops_init(struct snd_sof_dev *sdev)
 	/* set DAI ops */
 	hda_set_dai_drv_ops(sdev, &sof_mtl_ops);
 
+	sof_mtl_ops.set_power_state = hda_dsp_set_power_state_ipc4;
+
 	return 0;
 };
 EXPORT_SYMBOL_NS(sof_mtl_ops_init, SND_SOC_SOF_INTEL_HDA_COMMON);
diff --git a/sound/soc/sof/intel/tgl.c b/sound/soc/sof/intel/tgl.c
index 58ac3a46e6a7..2713b7dc7931 100644
--- a/sound/soc/sof/intel/tgl.c
+++ b/sound/soc/sof/intel/tgl.c
@@ -71,6 +71,8 @@  int sof_tgl_ops_init(struct snd_sof_dev *sdev)
 
 		/* debug */
 		sof_tgl_ops.ipc_dump	= cnl_ipc_dump;
+
+		sof_tgl_ops.set_power_state = hda_dsp_set_power_state_ipc3;
 	}
 
 	if (sdev->pdata->ipc_type == SOF_INTEL_IPC4) {
@@ -96,6 +98,8 @@  int sof_tgl_ops_init(struct snd_sof_dev *sdev)
 
 		/* debug */
 		sof_tgl_ops.ipc_dump	= cnl_ipc4_dump;
+
+		sof_tgl_ops.set_power_state = hda_dsp_set_power_state_ipc4;
 	}
 
 	/* set DAI driver ops */