[PATCH 0/3] [5.18.y backport] ASoC: SOF: Intel: fix resume from hibernate

[PATCH 0/3] [5.18.y backport] ASoC: SOF: Intel: fix resume from hibernate

[Pierre-Louis Bossart]

Backport for 5.18 stable, the first two patches were not modified,
only the third patch had a missing dependency with 2a68ff846164
("ASoC: SOF: Intel: hda: Revisit IMR boot sequence")

Pierre-Louis Bossart (3):
  ASoC: SOF: pm: add explicit behavior for ACPI S1 and S2
  ASoC: SOF: pm: add definitions for S4 and S5 states
  ASoC: SOF: Intel: disable IMR boot when resuming from ACPI S4 and S5
    states

 sound/soc/sof/intel/hda-loader.c |  3 ++-
 sound/soc/sof/pm.c               | 21 ++++++++++++++++++++-
 sound/soc/sof/sof-priv.h         |  2 ++
 3 files changed, 24 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH 1/3] ASoC: SOF: pm: add explicit behavior for ACPI S1 and S2

[Pierre-Louis Bossart]

commit 6639990dbb25257eeb3df4d03e38e7d26c2484ab upstream.

The existing code only deals with S0 and S3, let's start adding S1 and S2.

No functional change.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 sound/soc/sof/pm.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c
index 1c319582ca6f..937354c25856 100644
--- a/sound/soc/sof/pm.c
+++ b/sound/soc/sof/pm.c
@@ -327,8 +327,18 @@ int snd_sof_prepare(struct device *dev)
 		return 0;
 
 #if defined(CONFIG_ACPI)
-	if (acpi_target_system_state() == ACPI_STATE_S0)
+	switch (acpi_target_system_state()) {
+	case ACPI_STATE_S0:
 		sdev->system_suspend_target = SOF_SUSPEND_S0IX;
+		break;
+	case ACPI_STATE_S1:
+	case ACPI_STATE_S2:
+	case ACPI_STATE_S3:
+		sdev->system_suspend_target = SOF_SUSPEND_S3;
+		break;
+	default:
+		break;
+	}
 #endif
 
 	return 0;
-- 
2.34.1

Re: [PATCH 1/3] ASoC: SOF: pm: add explicit behavior for ACPI S1 and S2

[Greg KH]

On Mon, Jul 11, 2022 at 10:57:17AM -0500, Pierre-Louis Bossart wrote:
> commit 6639990dbb25257eeb3df4d03e38e7d26c2484ab upstream.

This is not a commit in Linus's tree :(

[PATCH 2/3] ASoC: SOF: pm: add definitions for S4 and S5 states

[Pierre-Louis Bossart]

commit 7a5974e035a6d496797547e4b469bc88938343c2 upstream.

We currently don't have a means to differentiate between S3, S4 and
S5. Add definitions so that we have select different code paths
depending on the target state in follow-up patches.

Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 sound/soc/sof/pm.c       | 9 +++++++++
 sound/soc/sof/sof-priv.h | 2 ++
 2 files changed, 11 insertions(+)

diff --git a/sound/soc/sof/pm.c b/sound/soc/sof/pm.c
index 937354c25856..76351f7f3243 100644
--- a/sound/soc/sof/pm.c
+++ b/sound/soc/sof/pm.c
@@ -23,6 +23,9 @@ static u32 snd_sof_dsp_power_target(struct snd_sof_dev *sdev)
 	u32 target_dsp_state;
 
 	switch (sdev->system_suspend_target) {
+	case SOF_SUSPEND_S5:
+	case SOF_SUSPEND_S4:
+		/* DSP should be in D3 if the system is suspending to S3+ */
 	case SOF_SUSPEND_S3:
 		/* DSP should be in D3 if the system is suspending to S3 */
 		target_dsp_state = SOF_DSP_PM_D3;
@@ -336,6 +339,12 @@ int snd_sof_prepare(struct device *dev)
 	case ACPI_STATE_S3:
 		sdev->system_suspend_target = SOF_SUSPEND_S3;
 		break;
+	case ACPI_STATE_S4:
+		sdev->system_suspend_target = SOF_SUSPEND_S4;
+		break;
+	case ACPI_STATE_S5:
+		sdev->system_suspend_target = SOF_SUSPEND_S5;
+		break;
 	default:
 		break;
 	}
diff --git a/sound/soc/sof/sof-priv.h b/sound/soc/sof/sof-priv.h
index 0d9b640ae24c..c856f0d84e49 100644
--- a/sound/soc/sof/sof-priv.h
+++ b/sound/soc/sof/sof-priv.h
@@ -85,6 +85,8 @@ enum sof_system_suspend_state {
 	SOF_SUSPEND_NONE = 0,
 	SOF_SUSPEND_S0IX,
 	SOF_SUSPEND_S3,
+	SOF_SUSPEND_S4,
+	SOF_SUSPEND_S5,
 };
 
 enum sof_dfsentry_type {
-- 
2.34.1

Re: [PATCH 2/3] ASoC: SOF: pm: add definitions for S4 and S5 states

[Greg KH]

On Mon, Jul 11, 2022 at 10:57:18AM -0500, Pierre-Louis Bossart wrote:
> commit 7a5974e035a6d496797547e4b469bc88938343c2 upstream.

This is not a commit in Linus's tree :(

[PATCH 3/3] ASoC: SOF: Intel: disable IMR boot when resuming from ACPI S4 and S5 states

[Pierre-Louis Bossart]

commit 58ecb11eab44dd5d64e35664ac4d62fecb6328f4 upstream.

The IMR was assumed to be preserved when suspending to S4 and S5
states, but community reports invalidate that assumption, the hardware
seems to be powered off and the IMR memory content cleared.

Make sure regular boot with firmware download is used for S4 and S5.

BugLink: https://github.com/thesofproject/sof/issues/5892
Fixes: 5fb5f51185126 ("ASoC: SOF: Intel: hda-loader: add IMR restore support")
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 sound/soc/sof/intel/hda-loader.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c
index 2ac5d9d0719b..bb7726830c25 100644
--- a/sound/soc/sof/intel/hda-loader.c
+++ b/sound/soc/sof/intel/hda-loader.c
@@ -397,7 +397,8 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev)
 	struct firmware stripped_firmware;
 	int ret, ret1, i;
 
-	if ((sdev->fw_ready.flags & SOF_IPC_INFO_D3_PERSISTENT) &&
+	if ((sdev->system_suspend_target < SOF_SUSPEND_S4) &&
+	    (sdev->fw_ready.flags & SOF_IPC_INFO_D3_PERSISTENT) &&
 	    !(sof_debug_check_flag(SOF_DBG_IGNORE_D3_PERSISTENT)) &&
 	    !sdev->first_boot) {
 		dev_dbg(sdev->dev, "IMR restore supported, booting from IMR directly\n");
-- 
2.34.1

Re: [PATCH 3/3] ASoC: SOF: Intel: disable IMR boot when resuming from ACPI S4 and S5 states

[Greg KH]

On Mon, Jul 11, 2022 at 10:57:19AM -0500, Pierre-Louis Bossart wrote:
> commit 58ecb11eab44dd5d64e35664ac4d62fecb6328f4 upstream.

Again, not a valid commit :(

Where did these come from?

confused,

greg k-h

Re: [PATCH 3/3] ASoC: SOF: Intel: disable IMR boot when resuming from ACPI S4 and S5 states

[Pierre-Louis Bossart]

On 7/23/22 08:52, Greg KH wrote:
> On Mon, Jul 11, 2022 at 10:57:19AM -0500, Pierre-Louis Bossart wrote:
>> commit 58ecb11eab44dd5d64e35664ac4d62fecb6328f4 upstream.
> 
> Again, not a valid commit :(
> 
> Where did these come from?
> 
> confused,

There are on Mark Brown's for-next branch, I must have looked at the IDs
after a rebase or something. We always report the SHA IDs from that
for-next branch, and linux-next check those values. I didn't realize
they could be different in Linus' tree.

I am still a bit confused since our Fixes tag could be right at the
moment we submit the patches to Mark but wrong long-term after merge by
Linus. Either I need more coffee, or I am missing a key concept, or both.

The commit IDs on Linus' tree should be:

a933084558c6 ASoC: SOF: pm: add explicit behavior for ACPI S1 and S2

9d2d46271338 ASoC: SOF: pm: add definitions for S4 and S5 states

391153522d18 ASoC: SOF: Intel: disable IMR boot when resuming from ACPI
S4 and S5 states


Do you want me to resend?
Thanks
-Pierre

Re: [PATCH 3/3] ASoC: SOF: Intel: disable IMR boot when resuming from ACPI S4 and S5 states

[Greg KH]

On Mon, Jul 25, 2022 at 10:25:07AM -0500, Pierre-Louis Bossart wrote:
> 
> 
> On 7/23/22 08:52, Greg KH wrote:
> > On Mon, Jul 11, 2022 at 10:57:19AM -0500, Pierre-Louis Bossart wrote:
> >> commit 58ecb11eab44dd5d64e35664ac4d62fecb6328f4 upstream.
> > 
> > Again, not a valid commit :(
> > 
> > Where did these come from?
> > 
> > confused,
> 
> There are on Mark Brown's for-next branch, I must have looked at the IDs
> after a rebase or something. We always report the SHA IDs from that
> for-next branch, and linux-next check those values. I didn't realize
> they could be different in Linus' tree.
> 
> I am still a bit confused since our Fixes tag could be right at the
> moment we submit the patches to Mark but wrong long-term after merge by
> Linus. Either I need more coffee, or I am missing a key concept, or both.
> 
> The commit IDs on Linus' tree should be:
> 
> a933084558c6 ASoC: SOF: pm: add explicit behavior for ACPI S1 and S2
> 
> 9d2d46271338 ASoC: SOF: pm: add definitions for S4 and S5 states
> 
> 391153522d18 ASoC: SOF: Intel: disable IMR boot when resuming from ACPI
> S4 and S5 states
> 
> 
> Do you want me to resend?

Yes please.

[PATCH 0/3] ASoC: SOF: Intel: fix resume from hibernate

[Pierre-Louis Bossart]

The enablement of IMR-based DSP boot helped reduce resume latency, but
unfortunately the context is not saved in S4 and S5 which leads to
multiple reports of boot failures.

This patchset forces a full firmware reload/reboot when resuming from
S4/S5 and restores functionality.

Pierre-Louis Bossart (3):
  ASoC: SOF: pm: add explicit behavior for ACPI S1 and S2
  ASoC: SOF: pm: add definitions for S4 and S5 states
  ASoC: SOF: Intel: disable IMR boot when resuming from ACPI S4 and S5
    states

 sound/soc/sof/intel/hda-loader.c |  3 ++-
 sound/soc/sof/pm.c               | 21 ++++++++++++++++++++-
 sound/soc/sof/sof-priv.h         |  2 ++
 3 files changed, 24 insertions(+), 2 deletions(-)

-- 
2.34.1

[PATCH 3/3] ASoC: SOF: Intel: disable IMR boot when resuming from ACPI S4 and S5 states

[Pierre-Louis Bossart]

The IMR was assumed to be preserved when suspending to S4 and S5
states, but community reports invalidate that assumption, the hardware
seems to be powered off and the IMR memory content cleared.

Make sure regular boot with firmware download is used for S4 and S5.

BugLink: https://github.com/thesofproject/sof/issues/5892
Fixes: 5fb5f51185126 ("ASoC: SOF: Intel: hda-loader: add IMR restore support")
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Péter Ujfalusi <peter.ujfalusi@linux.intel.com>
---
 sound/soc/sof/intel/hda-loader.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c
index bca9dc5917f42..819b3b08c6557 100644
--- a/sound/soc/sof/intel/hda-loader.c
+++ b/sound/soc/sof/intel/hda-loader.c
@@ -395,7 +395,8 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev)
 	struct snd_dma_buffer dmab;
 	int ret, ret1, i;
 
-	if (hda->imrboot_supported && !sdev->first_boot) {
+	if (sdev->system_suspend_target < SOF_SUSPEND_S4 &&
+	    hda->imrboot_supported && !sdev->first_boot) {
 		dev_dbg(sdev->dev, "IMR restore supported, booting from IMR directly\n");
 		hda->boot_iteration = 0;
 		ret = hda_dsp_boot_imr(sdev);
-- 
2.34.1