public inbox for alsa-devel@alsa-project.org
 help / color / mirror / Atom feed
From: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
To: lgirdwood@gmail.com, broonie@kernel.org
Cc: alsa-devel@alsa-project.org,
	pierre-louis.bossart@linux.intel.com,
	ranjani.sridharan@linux.intel.com, kai.vehmanen@linux.intel.com,
	rander.wang@intel.com, yung-chuan.liao@linux.intel.com
Subject: [PATCH] ASoC: SOF: Intel: Split the set_power_op for IPC3 and IPC4
Date: Thu, 20 Apr 2023 13:47:14 +0300	[thread overview]
Message-ID: <20230420104714.29573-1-peter.ujfalusi@linux.intel.com> (raw)

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(-)

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 */
-- 
2.40.0


             reply	other threads:[~2023-04-20 10:48 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-04-20 10:47 Peter Ujfalusi [this message]
2023-04-21 16:06 ` [PATCH] ASoC: SOF: Intel: Split the set_power_op for IPC3 and IPC4 Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230420104714.29573-1-peter.ujfalusi@linux.intel.com \
    --to=peter.ujfalusi@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=kai.vehmanen@linux.intel.com \
    --cc=lgirdwood@gmail.com \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=rander.wang@intel.com \
    --cc=ranjani.sridharan@linux.intel.com \
    --cc=yung-chuan.liao@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox