diff mbox series

[12/14] ASoC: Intel: avs: Power management

Message ID 20220426172346.3508411-13-cezary.rojewski@intel.com (mailing list archive)
State Superseded
Headers show
Series ASoC: Intel: avs: Driver core and PCM operations | expand

Commit Message

Cezary Rojewski April 26, 2022, 5:23 p.m. UTC 
To preserve power during sleep operations, handle suspend (S3),
hibernation (S4) and runtime (RTD3) transitions. As flow for all of
is shared, define common handlers to reduce code size.

Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
---
 sound/soc/intel/avs/core.c | 125 +++++++++++++++++++++++++++++++++++++
 1 file changed, 125 insertions(+)

Comments

Pierre-Louis Bossart April 26, 2022, 10:18 p.m. UTC | #1 
On 4/26/22 12:23, Cezary Rojewski wrote:
> To preserve power during sleep operations, handle suspend (S3),
> hibernation (S4) and runtime (RTD3) transitions. As flow for all of
> is shared, define common handlers to reduce code size.
> 
> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
> ---
>  sound/soc/intel/avs/core.c | 125 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 125 insertions(+)
> 
> diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
> index 93180c22032d..c2f8fb87cfc2 100644
> --- a/sound/soc/intel/avs/core.c
> +++ b/sound/soc/intel/avs/core.c
> @@ -536,6 +536,128 @@ static void avs_pci_remove(struct pci_dev *pci)
>  	pm_runtime_get_noresume(&pci->dev);
>  }
>  
> +static int __maybe_unused avs_suspend_common(struct avs_dev *adev, bool low_power)

low_power is not used so....

> +{
> +	struct hdac_bus *bus = &adev->base.core;
> +	int ret;
> +
> +	flush_work(&adev->probe_work);
> +
> +	snd_hdac_ext_bus_link_power_down_all(bus);
> +
> +	ret = avs_ipc_set_dx(adev, AVS_MAIN_CORE_MASK, false);
> +	/*
> +	 * pm_runtime is blocked on DSP failure but system-wide suspend is not.
> +	 * Do not block entire system from suspending if that's the case.
> +	 */
> +	if (ret && ret != -EPERM) {
> +		dev_err(adev->dev, "set dx failed: %d\n", ret);
> +		return AVS_IPC_RET(ret);
> +	}
> +
> +	avs_dsp_op(adev, int_control, false);
> +	snd_hdac_ext_bus_ppcap_int_enable(bus, false);
> +
> +	ret = avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
> +	if (ret < 0) {
> +		dev_err(adev->dev, "core_mask %ld disable failed: %d\n", AVS_MAIN_CORE_MASK, ret);
> +		return ret;
> +	}
> +
> +	snd_hdac_ext_bus_ppcap_enable(bus, false);
> +	/* disable LP SRAM retention */
> +	avs_hda_power_gating_enable(adev, false);
> +	snd_hdac_bus_stop_chip(bus);
> +	/* disable CG when putting controller to reset */
> +	avs_hdac_clock_gating_enable(bus, false);
> +	snd_hdac_bus_enter_link_reset(bus);
> +	avs_hdac_clock_gating_enable(bus, true);
> +
> +	snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, false);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused avs_resume_common(struct avs_dev *adev, bool low_power, bool purge)
> +{
> +	struct hdac_bus *bus = &adev->base.core;
> +	struct hdac_ext_link *hlink;
> +	int ret;
> +
> +	snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, true);
> +	avs_hdac_bus_init_chip(bus, true);
> +
> +	snd_hdac_ext_bus_ppcap_enable(bus, true);
> +	snd_hdac_ext_bus_ppcap_int_enable(bus, true);
> +
> +	ret = avs_dsp_boot_firmware(adev, purge);
> +	if (ret < 0) {
> +		dev_err(adev->dev, "firmware boot failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/* turn off the links that were off before suspend */
> +	list_for_each_entry(hlink, &bus->hlink_list, list) {
> +		if (!hlink->ref_count)
> +			snd_hdac_ext_bus_link_power_down(hlink);
> +	}
> +
> +	/* check dma status and clean up CORB/RIRB buffers */
> +	if (!bus->cmd_dma_state)
> +		snd_hdac_bus_stop_cmd_io(bus);
> +
> +	return 0;
> +}
> +
> +static int __maybe_unused avs_suspend(struct device *dev)
> +{
> +	return avs_suspend_common(to_avs_dev(dev), true);
> +}
> +
> +static int __maybe_unused avs_resume(struct device *dev)
> +{
> +	return avs_resume_common(to_avs_dev(dev), true, true);
> +}
> +
> +static int __maybe_unused avs_runtime_suspend(struct device *dev)
> +{
> +	return avs_suspend_common(to_avs_dev(dev), true);
> +}
> +
> +static int __maybe_unused avs_runtime_resume(struct device *dev)
> +{
> +	return avs_resume_common(to_avs_dev(dev), true, false);
> +}
> +
> +static int __maybe_unused avs_freeze(struct device *dev)
> +{
> +	return avs_suspend_common(to_avs_dev(dev), false);
> +}
> +static int __maybe_unused avs_thaw(struct device *dev)
> +{
> +	return avs_resume_common(to_avs_dev(dev), false, true);
> +}
> +
> +static int __maybe_unused avs_poweroff(struct device *dev)
> +{
> +	return avs_suspend_common(to_avs_dev(dev), false);
> +}
> +
> +static int __maybe_unused avs_restore(struct device *dev)
> +{
> +	return avs_resume_common(to_avs_dev(dev), false, true);
> +}
> +
> +static const struct dev_pm_ops avs_dev_pm = {
> +	.suspend = avs_suspend,
> +	.resume = avs_resume,

... all the 4 functions below seem unnecessary?

> +	.freeze = avs_freeze,
> +	.thaw = avs_thaw,
> +	.poweroff = avs_poweroff,
> +	.restore = avs_restore,

we've historically never used those, what drives this new usage?

on the SOF side, we've used prepare and complete, along with idle...


> +	SET_RUNTIME_PM_OPS(avs_runtime_suspend, avs_runtime_resume, NULL)
> +};
> +
>  static const struct pci_device_id avs_ids[] = {
>  	{ 0 }
>  };
> @@ -546,6 +668,9 @@ static struct pci_driver avs_pci_driver = {
>  	.id_table = avs_ids,
>  	.probe = avs_pci_probe,
>  	.remove = avs_pci_remove,
> +	.driver = {
> +		.pm = &avs_dev_pm,
> +	},
>  };
>  module_pci_driver(avs_pci_driver);
>
Cezary Rojewski April 29, 2022, 1:44 p.m. UTC | #2 
On 2022-04-27 12:18 AM, Pierre-Louis Bossart wrote:
> On 4/26/22 12:23, Cezary Rojewski wrote:
>> To preserve power during sleep operations, handle suspend (S3),
>> hibernation (S4) and runtime (RTD3) transitions. As flow for all of
>> is shared, define common handlers to reduce code size.
>>
>> Signed-off-by: Amadeusz Sławiński <amadeuszx.slawinski@linux.intel.com>
>> Signed-off-by: Cezary Rojewski <cezary.rojewski@intel.com>
>> ---
>>   sound/soc/intel/avs/core.c | 125 +++++++++++++++++++++++++++++++++++++
>>   1 file changed, 125 insertions(+)
>>
>> diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
>> index 93180c22032d..c2f8fb87cfc2 100644
>> --- a/sound/soc/intel/avs/core.c
>> +++ b/sound/soc/intel/avs/core.c
>> @@ -536,6 +536,128 @@ static void avs_pci_remove(struct pci_dev *pci)
>>   	pm_runtime_get_noresume(&pci->dev);
>>   }
>>   
>> +static int __maybe_unused avs_suspend_common(struct avs_dev *adev, bool low_power)
> 
> low_power is not used so....


Indeed, this argument should be part of the patch which introduces its 
usage. Removing in v2!

>> +{
>> +	struct hdac_bus *bus = &adev->base.core;
>> +	int ret;
>> +
>> +	flush_work(&adev->probe_work);
>> +
>> +	snd_hdac_ext_bus_link_power_down_all(bus);
>> +
>> +	ret = avs_ipc_set_dx(adev, AVS_MAIN_CORE_MASK, false);
>> +	/*
>> +	 * pm_runtime is blocked on DSP failure but system-wide suspend is not.
>> +	 * Do not block entire system from suspending if that's the case.
>> +	 */
>> +	if (ret && ret != -EPERM) {
>> +		dev_err(adev->dev, "set dx failed: %d\n", ret);
>> +		return AVS_IPC_RET(ret);
>> +	}
>> +
>> +	avs_dsp_op(adev, int_control, false);
>> +	snd_hdac_ext_bus_ppcap_int_enable(bus, false);
>> +
>> +	ret = avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
>> +	if (ret < 0) {
>> +		dev_err(adev->dev, "core_mask %ld disable failed: %d\n", AVS_MAIN_CORE_MASK, ret);
>> +		return ret;
>> +	}
>> +
>> +	snd_hdac_ext_bus_ppcap_enable(bus, false);
>> +	/* disable LP SRAM retention */
>> +	avs_hda_power_gating_enable(adev, false);
>> +	snd_hdac_bus_stop_chip(bus);
>> +	/* disable CG when putting controller to reset */
>> +	avs_hdac_clock_gating_enable(bus, false);
>> +	snd_hdac_bus_enter_link_reset(bus);
>> +	avs_hdac_clock_gating_enable(bus, true);
>> +
>> +	snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, false);
>> +
>> +	return 0;
>> +}
>> +
>> +static int __maybe_unused avs_resume_common(struct avs_dev *adev, bool low_power, bool purge)
>> +{
>> +	struct hdac_bus *bus = &adev->base.core;
>> +	struct hdac_ext_link *hlink;
>> +	int ret;
>> +
>> +	snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, true);
>> +	avs_hdac_bus_init_chip(bus, true);
>> +
>> +	snd_hdac_ext_bus_ppcap_enable(bus, true);
>> +	snd_hdac_ext_bus_ppcap_int_enable(bus, true);
>> +
>> +	ret = avs_dsp_boot_firmware(adev, purge);
>> +	if (ret < 0) {
>> +		dev_err(adev->dev, "firmware boot failed: %d\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	/* turn off the links that were off before suspend */
>> +	list_for_each_entry(hlink, &bus->hlink_list, list) {
>> +		if (!hlink->ref_count)
>> +			snd_hdac_ext_bus_link_power_down(hlink);
>> +	}
>> +
>> +	/* check dma status and clean up CORB/RIRB buffers */
>> +	if (!bus->cmd_dma_state)
>> +		snd_hdac_bus_stop_cmd_io(bus);
>> +
>> +	return 0;
>> +}
>> +
>> +static int __maybe_unused avs_suspend(struct device *dev)
>> +{
>> +	return avs_suspend_common(to_avs_dev(dev), true);
>> +}
>> +
>> +static int __maybe_unused avs_resume(struct device *dev)
>> +{
>> +	return avs_resume_common(to_avs_dev(dev), true, true);
>> +}
>> +
>> +static int __maybe_unused avs_runtime_suspend(struct device *dev)
>> +{
>> +	return avs_suspend_common(to_avs_dev(dev), true);
>> +}
>> +
>> +static int __maybe_unused avs_runtime_resume(struct device *dev)
>> +{
>> +	return avs_resume_common(to_avs_dev(dev), true, false);
>> +}
>> +
>> +static int __maybe_unused avs_freeze(struct device *dev)
>> +{
>> +	return avs_suspend_common(to_avs_dev(dev), false);
>> +}
>> +static int __maybe_unused avs_thaw(struct device *dev)
>> +{
>> +	return avs_resume_common(to_avs_dev(dev), false, true);
>> +}
>> +
>> +static int __maybe_unused avs_poweroff(struct device *dev)
>> +{
>> +	return avs_suspend_common(to_avs_dev(dev), false);
>> +}
>> +
>> +static int __maybe_unused avs_restore(struct device *dev)
>> +{
>> +	return avs_resume_common(to_avs_dev(dev), false, true);
>> +}
>> +
>> +static const struct dev_pm_ops avs_dev_pm = {
>> +	.suspend = avs_suspend,
>> +	.resume = avs_resume,
> 
> ... all the 4 functions below seem unnecessary?


If we exclude 'low_power' then this is true. Will move this to the 
'low_power' patch as requested!

>> +	.freeze = avs_freeze,
>> +	.thaw = avs_thaw,
>> +	.poweroff = avs_poweroff,
>> +	.restore = avs_restore,
> 
> we've historically never used those, what drives this new usage?
> 
> on the SOF side, we've used prepare and complete, along with idle...


No streams may survive S4 what is not the case for S3. That's the main 
difference.

Yes, there are many opses vailable in dev_pm_ops, perhaps a different 
set of them is more appropriate. Let's have this talk once 'low_power' 
patch is here (PCM-power management patches are not part of this 
patchset but depend on it).

>> +	SET_RUNTIME_PM_OPS(avs_runtime_suspend, avs_runtime_resume, NULL)
>> +};
>> +
>>   static const struct pci_device_id avs_ids[] = {
>>   	{ 0 }
>>   };
>> @@ -546,6 +668,9 @@ static struct pci_driver avs_pci_driver = {
>>   	.id_table = avs_ids,
>>   	.probe = avs_pci_probe,
>>   	.remove = avs_pci_remove,
>> +	.driver = {
>> +		.pm = &avs_dev_pm,
>> +	},
>>   };
>>   module_pci_driver(avs_pci_driver);
>>
diff mbox series

Patch

diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c
index 93180c22032d..c2f8fb87cfc2 100644
--- a/sound/soc/intel/avs/core.c
+++ b/sound/soc/intel/avs/core.c
@@ -536,6 +536,128 @@  static void avs_pci_remove(struct pci_dev *pci)
 	pm_runtime_get_noresume(&pci->dev);
 }
 
+static int __maybe_unused avs_suspend_common(struct avs_dev *adev, bool low_power)
+{
+	struct hdac_bus *bus = &adev->base.core;
+	int ret;
+
+	flush_work(&adev->probe_work);
+
+	snd_hdac_ext_bus_link_power_down_all(bus);
+
+	ret = avs_ipc_set_dx(adev, AVS_MAIN_CORE_MASK, false);
+	/*
+	 * pm_runtime is blocked on DSP failure but system-wide suspend is not.
+	 * Do not block entire system from suspending if that's the case.
+	 */
+	if (ret && ret != -EPERM) {
+		dev_err(adev->dev, "set dx failed: %d\n", ret);
+		return AVS_IPC_RET(ret);
+	}
+
+	avs_dsp_op(adev, int_control, false);
+	snd_hdac_ext_bus_ppcap_int_enable(bus, false);
+
+	ret = avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
+	if (ret < 0) {
+		dev_err(adev->dev, "core_mask %ld disable failed: %d\n", AVS_MAIN_CORE_MASK, ret);
+		return ret;
+	}
+
+	snd_hdac_ext_bus_ppcap_enable(bus, false);
+	/* disable LP SRAM retention */
+	avs_hda_power_gating_enable(adev, false);
+	snd_hdac_bus_stop_chip(bus);
+	/* disable CG when putting controller to reset */
+	avs_hdac_clock_gating_enable(bus, false);
+	snd_hdac_bus_enter_link_reset(bus);
+	avs_hdac_clock_gating_enable(bus, true);
+
+	snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, false);
+
+	return 0;
+}
+
+static int __maybe_unused avs_resume_common(struct avs_dev *adev, bool low_power, bool purge)
+{
+	struct hdac_bus *bus = &adev->base.core;
+	struct hdac_ext_link *hlink;
+	int ret;
+
+	snd_hdac_display_power(bus, HDA_CODEC_IDX_CONTROLLER, true);
+	avs_hdac_bus_init_chip(bus, true);
+
+	snd_hdac_ext_bus_ppcap_enable(bus, true);
+	snd_hdac_ext_bus_ppcap_int_enable(bus, true);
+
+	ret = avs_dsp_boot_firmware(adev, purge);
+	if (ret < 0) {
+		dev_err(adev->dev, "firmware boot failed: %d\n", ret);
+		return ret;
+	}
+
+	/* turn off the links that were off before suspend */
+	list_for_each_entry(hlink, &bus->hlink_list, list) {
+		if (!hlink->ref_count)
+			snd_hdac_ext_bus_link_power_down(hlink);
+	}
+
+	/* check dma status and clean up CORB/RIRB buffers */
+	if (!bus->cmd_dma_state)
+		snd_hdac_bus_stop_cmd_io(bus);
+
+	return 0;
+}
+
+static int __maybe_unused avs_suspend(struct device *dev)
+{
+	return avs_suspend_common(to_avs_dev(dev), true);
+}
+
+static int __maybe_unused avs_resume(struct device *dev)
+{
+	return avs_resume_common(to_avs_dev(dev), true, true);
+}
+
+static int __maybe_unused avs_runtime_suspend(struct device *dev)
+{
+	return avs_suspend_common(to_avs_dev(dev), true);
+}
+
+static int __maybe_unused avs_runtime_resume(struct device *dev)
+{
+	return avs_resume_common(to_avs_dev(dev), true, false);
+}
+
+static int __maybe_unused avs_freeze(struct device *dev)
+{
+	return avs_suspend_common(to_avs_dev(dev), false);
+}
+static int __maybe_unused avs_thaw(struct device *dev)
+{
+	return avs_resume_common(to_avs_dev(dev), false, true);
+}
+
+static int __maybe_unused avs_poweroff(struct device *dev)
+{
+	return avs_suspend_common(to_avs_dev(dev), false);
+}
+
+static int __maybe_unused avs_restore(struct device *dev)
+{
+	return avs_resume_common(to_avs_dev(dev), false, true);
+}
+
+static const struct dev_pm_ops avs_dev_pm = {
+	.suspend = avs_suspend,
+	.resume = avs_resume,
+	.freeze = avs_freeze,
+	.thaw = avs_thaw,
+	.poweroff = avs_poweroff,
+	.restore = avs_restore,
+	SET_RUNTIME_PM_OPS(avs_runtime_suspend, avs_runtime_resume, NULL)
+};
+
 static const struct pci_device_id avs_ids[] = {
 	{ 0 }
 };
@@ -546,6 +668,9 @@  static struct pci_driver avs_pci_driver = {
 	.id_table = avs_ids,
 	.probe = avs_pci_probe,
 	.remove = avs_pci_remove,
+	.driver = {
+		.pm = &avs_dev_pm,
+	},
 };
 module_pci_driver(avs_pci_driver);