[alsa-devel] [PATCH 0/3] SOF PM and jack detection fixes
This set of patches addresses issues related to runtime suspend and jack detection during runtime suspend.
The first 2 patches handle enabling WAKEEN in the SOF driver to address issue with jack detection when the SOF device is runtime suspended. More details about the issue can be found here: https://github.com/thesofproject/linux/issues/909
The third patch allows the SOF driver to suspend after the autosuspend delay instead of suspending immediately upon boot.
Pan Xiuli (1): ASoC: SOF: mark last_busy value at runtime PM init
Rander Wang (2): ASoC: SOF: Intel: hda: reduce ifdef usage for hda ASoC: SOF: Intel: hda: Enable jack detection in sof hda driver
sound/soc/sof/intel/hda-codec.c | 45 +++++++++++++++++++++++++++-- sound/soc/sof/intel/hda-dsp.c | 51 +++++++++++++++++++++------------ sound/soc/sof/intel/hda.h | 4 +++ sound/soc/sof/sof-pci-dev.c | 3 ++ 4 files changed, 83 insertions(+), 20 deletions(-)
From: Rander Wang rander.wang@linux.intel.com
Move the code for hda to one point
Signed-off-by: Rander Wang rander.wang@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/intel/hda-dsp.c | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index f2c5a12db930..481cfa0b225e 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -359,6 +359,16 @@ static int hda_resume(struct snd_sof_dev *sdev) bus->io_ops->reg_writel(0, hlink->ml_addr + AZX_REG_ML_LOSIDV);
hda_dsp_ctrl_misc_clock_gating(sdev, true); + + /* 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); #else
hda_dsp_ctrl_misc_clock_gating(sdev, false); @@ -391,18 +401,6 @@ static int hda_resume(struct snd_sof_dev *sdev) hda_dsp_ctrl_ppcap_enable(sdev, true); hda_dsp_ctrl_ppcap_int_enable(sdev, true);
-#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) - /* 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); -#endif - return 0; }
From: Rander Wang rander.wang@linux.intel.com
In commit 7d4f606c50ff ("ALSA: hda - WAKEEN feature enabling for runtime pm"), legacy HD-A driver sets hda controller in reset mode after entering runtime-suspend. And when resuming from suspend mode, it checks hda controller & codec status to detect headphone hotplug event. Now this patch does the same job in SOF runtime pm functions.
And we need to check all the non-hdmi codecs for some cases like playback with HDMI or capture with DMIC connected to dsp. In these cases, only controller is active and codecs are suspended, so codecs can't send unsolicited event to controller. The jack polling operation will activate codecs and unsolicited event can work even codecs become suspended later.
Tested on whiskylake with hda codecs.
Signed-off-by: Rander Wang rander.wang@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/intel/hda-codec.c | 45 +++++++++++++++++++++++++++++++-- sound/soc/sof/intel/hda-dsp.c | 29 ++++++++++++++++----- sound/soc/sof/intel/hda.h | 4 +++ 3 files changed, 70 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c index b8b37f082309..c711792534da 100644 --- a/sound/soc/sof/intel/hda-codec.c +++ b/sound/soc/sof/intel/hda-codec.c @@ -10,6 +10,7 @@
#include <linux/module.h> #include <sound/hdaudio_ext.h> +#include <sound/hda_register.h> #include <sound/hda_codec.h> #include <sound/hda_i915.h> #include <sound/sof.h> @@ -37,16 +38,55 @@ static void hda_codec_load_module(struct hda_codec *codec) static void hda_codec_load_module(struct hda_codec *codec) {} #endif
+/* check jack status after resuming from suspend mode */ +void hda_codec_jack_check(struct snd_sof_dev *sdev, int status) +{ + struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; + struct hda_bus *hbus = sof_to_hbus(sdev); + struct hdac_bus *bus = sof_to_bus(sdev); + struct hda_codec *codec; + int mask; + + /* + * there are two reasons for runtime resume + * (1) waken up by interrupt triggered by WAKEEN feature + * (2) waken up by pm get functions for some audio operations + * For case (1), the bits in status mean which codec triggers + * the interrupt and jacks will be checked on these codecs. + * For case (2), we need to check all the non-hdmi codecs for some + * cases like playback with HDMI or capture with DMIC. In these + * cases, only controller is active and codecs are suspended, so + * codecs can't send unsolicited event to controller. The jack polling + * operation will activate codecs and unsolicited event can work + * even codecs become suspended later. + */ + mask = status ? status : hda->hda_codec_mask; + + list_for_each_codec(codec, hbus) + if (mask & BIT(codec->core.addr)) + schedule_delayed_work(&codec->jackpoll_work, + codec->jackpoll_interval); + + /* disable controller Wake Up event*/ + snd_hdac_chip_writew(bus, WAKEEN, + snd_hdac_chip_readw(bus, WAKEEN) & + ~hda->hda_codec_mask); +} +#else +void hda_codec_jack_check(struct snd_sof_dev *sdev, int status) {} #endif /* CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC */ +EXPORT_SYMBOL(hda_codec_jack_check);
/* probe individual codec */ static int hda_codec_probe(struct snd_sof_dev *sdev, int address) { - struct hda_bus *hbus = sof_to_hbus(sdev); - struct hdac_device *hdev; #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC) + struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; struct hdac_hda_priv *hda_priv; #endif + struct hda_bus *hbus = sof_to_hbus(sdev); + struct hdac_device *hdev; + u32 hda_cmd = (address << 28) | (AC_NODE_ROOT << 20) | (AC_VERB_PARAMETERS << 8) | AC_PAR_VENDOR_ID; u32 resp = -1; @@ -77,6 +117,7 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address) /* use legacy bus only for HDA codecs, idisp uses ext bus */ if ((resp & 0xFFFF0000) != IDISP_VID_INTEL) { hdev->type = HDA_DEV_LEGACY; + hda->hda_codec_mask |= BIT(address); hda_codec_load_module(&hda_priv->codec); }
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index 481cfa0b225e..f8ed8a4a60b7 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -282,7 +282,8 @@ void hda_dsp_ipc_int_disable(struct snd_sof_dev *sdev) HDA_DSP_REG_HIPCCTL_BUSY | HDA_DSP_REG_HIPCCTL_DONE, 0); }
-static int hda_suspend(struct snd_sof_dev *sdev, int state) +static int hda_suspend(struct snd_sof_dev *sdev, int state, + bool runtime_suspend) { struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; const struct sof_intel_dsp_desc *chip = hda->desc; @@ -295,6 +296,12 @@ static int hda_suspend(struct snd_sof_dev *sdev, int state) hda_dsp_ipc_int_disable(sdev);
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) + if (IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC) && runtime_suspend) + /* enable controller wake up event */ + snd_hdac_chip_writew(bus, WAKEEN, + snd_hdac_chip_readw(bus, WAKEEN) | + hda->hda_codec_mask); + /* power down all hda link */ snd_hdac_ext_bus_link_power_down_all(bus); #endif @@ -329,11 +336,12 @@ static int hda_suspend(struct snd_sof_dev *sdev, int state) return 0; }
-static int hda_resume(struct snd_sof_dev *sdev) +static int hda_resume(struct snd_sof_dev *sdev, bool runtime_resume) { #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) struct hdac_bus *bus = sof_to_bus(sdev); struct hdac_ext_link *hlink = NULL; + int status; #endif int ret;
@@ -344,6 +352,11 @@ static int hda_resume(struct snd_sof_dev *sdev) snd_sof_pci_update_bits(sdev, PCI_TCSEL, 0x07, 0);
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) + if (runtime_resume) { + /* read STATESTS before controller reset */ + status = snd_hdac_chip_readw(bus, STATESTS); + } + /* reset and start hda controller */ ret = hda_dsp_ctrl_init_chip(sdev, true); if (ret < 0) { @@ -360,6 +373,10 @@ static int hda_resume(struct snd_sof_dev *sdev)
hda_dsp_ctrl_misc_clock_gating(sdev, true);
+ /* check jack status based on controller status */ + if (runtime_resume) + hda_codec_jack_check(sdev, status); + /* turn off the links that were off before suspend */ list_for_each_entry(hlink, &bus->hlink_list, list) { if (!hlink->ref_count) @@ -407,19 +424,19 @@ static int hda_resume(struct snd_sof_dev *sdev) 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); + return hda_resume(sdev, false); }
int hda_dsp_runtime_resume(struct snd_sof_dev *sdev) { /* init hda controller. DSP cores will be powered up during fw boot */ - return hda_resume(sdev); + return hda_resume(sdev, true); }
int hda_dsp_runtime_suspend(struct snd_sof_dev *sdev, int state) { /* stop hda controller and power dsp off */ - return hda_suspend(sdev, state); + return hda_suspend(sdev, state, true); }
int hda_dsp_suspend(struct snd_sof_dev *sdev, int state) @@ -428,7 +445,7 @@ int hda_dsp_suspend(struct snd_sof_dev *sdev, int state) int ret;
/* stop hda controller and power dsp off */ - ret = hda_suspend(sdev, state); + ret = hda_suspend(sdev, state, false); if (ret < 0) { dev_err(bus->dev, "error: suspending dsp\n"); return ret; diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 73d7cc08afc2..dd476d3a745d 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -398,6 +398,9 @@ struct sof_intel_hda_dev {
/* DMIC device */ struct platform_device *dmic_dev; + + /* hda codec mask excluding hdmi */ + u32 hda_codec_mask; };
static inline struct hdac_bus *sof_to_bus(struct snd_sof_dev *s) @@ -556,6 +559,7 @@ void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev, * HDA Codec operations. */ int hda_codec_probe_bus(struct snd_sof_dev *sdev); +void hda_codec_jack_check(struct snd_sof_dev *sdev, int status);
#endif /* CONFIG_SND_SOC_SOF_HDA */
On 2019-06-26 22:22, Ranjani Sridharan wrote:
From: Rander Wang rander.wang@linux.intel.com
In commit 7d4f606c50ff ("ALSA: hda - WAKEEN feature enabling for runtime pm"), legacy HD-A driver sets hda controller in reset mode after entering runtime-suspend. And when resuming from suspend mode, it checks hda controller & codec status to detect headphone hotplug event. Now this patch does the same job in SOF runtime pm functions.
And we need to check all the non-hdmi codecs for some cases like playback with HDMI or capture with DMIC connected to dsp. In these cases, only controller is active and codecs are suspended, so codecs can't send unsolicited event to controller. The jack polling operation will activate codecs and unsolicited event can work even codecs become suspended later.
Tested on whiskylake with hda codecs.
Signed-off-by: Rander Wang rander.wang@linux.intel.com Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com
sound/soc/sof/intel/hda-codec.c | 45 +++++++++++++++++++++++++++++++-- sound/soc/sof/intel/hda-dsp.c | 29 ++++++++++++++++----- sound/soc/sof/intel/hda.h | 4 +++ 3 files changed, 70 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c index b8b37f082309..c711792534da 100644 --- a/sound/soc/sof/intel/hda-codec.c +++ b/sound/soc/sof/intel/hda-codec.c @@ -10,6 +10,7 @@
#include <linux/module.h> #include <sound/hdaudio_ext.h> +#include <sound/hda_register.h> #include <sound/hda_codec.h> #include <sound/hda_i915.h> #include <sound/sof.h> @@ -37,16 +38,55 @@ static void hda_codec_load_module(struct hda_codec *codec) static void hda_codec_load_module(struct hda_codec *codec) {} #endif
+/* check jack status after resuming from suspend mode */ +void hda_codec_jack_check(struct snd_sof_dev *sdev, int status) +{
- struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
- struct hda_bus *hbus = sof_to_hbus(sdev);
- struct hdac_bus *bus = sof_to_bus(sdev);
- struct hda_codec *codec;
- int mask;
- /*
* there are two reasons for runtime resume
* (1) waken up by interrupt triggered by WAKEEN feature
* (2) waken up by pm get functions for some audio operations
* For case (1), the bits in status mean which codec triggers
* the interrupt and jacks will be checked on these codecs.
* For case (2), we need to check all the non-hdmi codecs for some
* cases like playback with HDMI or capture with DMIC. In these
* cases, only controller is active and codecs are suspended, so
* codecs can't send unsolicited event to controller. The jack polling
* operation will activate codecs and unsolicited event can work
* even codecs become suspended later.
*/
This block is huge. I'd suggest entering some whitespaces to make it more readable.
On the second thought, I bet this stuff is not SOF specific and if so, being more strict may prove more informative than being so explicit - reference to HDA spec/ kernel HDA documentation etc.
- mask = status ? status : hda->hda_codec_mask;
- list_for_each_codec(codec, hbus)
if (mask & BIT(codec->core.addr))
schedule_delayed_work(&codec->jackpoll_work,
codec->jackpoll_interval);
- /* disable controller Wake Up event*/
- snd_hdac_chip_writew(bus, WAKEEN,
snd_hdac_chip_readw(bus, WAKEEN) &
~hda->hda_codec_mask);
Any reason for not using snd_hdac_chip_updatew?
+} +#else +void hda_codec_jack_check(struct snd_sof_dev *sdev, int status) {} #endif /* CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC */ +EXPORT_SYMBOL(hda_codec_jack_check);
/* probe individual codec */ static int hda_codec_probe(struct snd_sof_dev *sdev, int address) {
- struct hda_bus *hbus = sof_to_hbus(sdev);
- struct hdac_device *hdev; #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC)
- struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; struct hdac_hda_priv *hda_priv; #endif
- struct hda_bus *hbus = sof_to_hbus(sdev);
- struct hdac_device *hdev;
- u32 hda_cmd = (address << 28) | (AC_NODE_ROOT << 20) | (AC_VERB_PARAMETERS << 8) | AC_PAR_VENDOR_ID; u32 resp = -1;
@@ -77,6 +117,7 @@ static int hda_codec_probe(struct snd_sof_dev *sdev, int address) /* use legacy bus only for HDA codecs, idisp uses ext bus */ if ((resp & 0xFFFF0000) != IDISP_VID_INTEL) { hdev->type = HDA_DEV_LEGACY;
hda_codec_load_module(&hda_priv->codec); }hda->hda_codec_mask |= BIT(address);
diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index 481cfa0b225e..f8ed8a4a60b7 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -282,7 +282,8 @@ void hda_dsp_ipc_int_disable(struct snd_sof_dev *sdev) HDA_DSP_REG_HIPCCTL_BUSY | HDA_DSP_REG_HIPCCTL_DONE, 0); }
-static int hda_suspend(struct snd_sof_dev *sdev, int state) +static int hda_suspend(struct snd_sof_dev *sdev, int state,
{ struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; const struct sof_intel_dsp_desc *chip = hda->desc;bool runtime_suspend)
@@ -295,6 +296,12 @@ static int hda_suspend(struct snd_sof_dev *sdev, int state) hda_dsp_ipc_int_disable(sdev);
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
- if (IS_ENABLED(CONFIG_SND_SOC_SOF_HDA_AUDIO_CODEC) && runtime_suspend)
/* enable controller wake up event */
snd_hdac_chip_writew(bus, WAKEEN,
snd_hdac_chip_readw(bus, WAKEEN) |
hda->hda_codec_mask);
Same here.
- /* power down all hda link */ snd_hdac_ext_bus_link_power_down_all(bus); #endif
@@ -329,11 +336,12 @@ static int hda_suspend(struct snd_sof_dev *sdev, int state) return 0; }
-static int hda_resume(struct snd_sof_dev *sdev) +static int hda_resume(struct snd_sof_dev *sdev, bool runtime_resume) { #if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA) struct hdac_bus *bus = sof_to_bus(sdev); struct hdac_ext_link *hlink = NULL;
- int status; #endif int ret;
@@ -344,6 +352,11 @@ static int hda_resume(struct snd_sof_dev *sdev) snd_sof_pci_update_bits(sdev, PCI_TCSEL, 0x07, 0);
#if IS_ENABLED(CONFIG_SND_SOC_SOF_HDA)
- if (runtime_resume) {
/* read STATESTS before controller reset */
status = snd_hdac_chip_readw(bus, STATESTS);
- }
- /* reset and start hda controller */ ret = hda_dsp_ctrl_init_chip(sdev, true); if (ret < 0) {
@@ -360,6 +373,10 @@ static int hda_resume(struct snd_sof_dev *sdev)
hda_dsp_ctrl_misc_clock_gating(sdev, true);
- /* check jack status based on controller status */
- if (runtime_resume)
hda_codec_jack_check(sdev, status);
- /* turn off the links that were off before suspend */ list_for_each_entry(hlink, &bus->hlink_list, list) { if (!hlink->ref_count)
@@ -407,19 +424,19 @@ static int hda_resume(struct snd_sof_dev *sdev) 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);
return hda_resume(sdev, false); }
int hda_dsp_runtime_resume(struct snd_sof_dev *sdev) { /* init hda controller. DSP cores will be powered up during fw boot */
- return hda_resume(sdev);
return hda_resume(sdev, true); }
int hda_dsp_runtime_suspend(struct snd_sof_dev *sdev, int state) { /* stop hda controller and power dsp off */
- return hda_suspend(sdev, state);
return hda_suspend(sdev, state, true); }
int hda_dsp_suspend(struct snd_sof_dev *sdev, int state)
@@ -428,7 +445,7 @@ int hda_dsp_suspend(struct snd_sof_dev *sdev, int state) int ret;
/* stop hda controller and power dsp off */
- ret = hda_suspend(sdev, state);
- ret = hda_suspend(sdev, state, false); if (ret < 0) { dev_err(bus->dev, "error: suspending dsp\n"); return ret;
diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 73d7cc08afc2..dd476d3a745d 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -398,6 +398,9 @@ struct sof_intel_hda_dev {
/* DMIC device */ struct platform_device *dmic_dev;
/* hda codec mask excluding hdmi */
u32 hda_codec_mask; };
static inline struct hdac_bus *sof_to_bus(struct snd_sof_dev *s)
@@ -556,6 +559,7 @@ void sof_hda_bus_init(struct hdac_bus *bus, struct device *dev,
- HDA Codec operations.
*/ int hda_codec_probe_bus(struct snd_sof_dev *sdev); +void hda_codec_jack_check(struct snd_sof_dev *sdev, int status);
#endif /* CONFIG_SND_SOC_SOF_HDA */
On Wed, 2019-06-26 at 23:03 +0200, Cezary Rojewski wrote:
On 2019-06-26 22:22, Ranjani Sridharan wrote:
From: Rander Wang rander.wang@linux.intel.com
In commit 7d4f606c50ff ("ALSA: hda - WAKEEN feature enabling for runtime pm"), legacy HD-A driver sets hda controller in reset mode after entering runtime-suspend. And when resuming from suspend mode, it checks hda controller & codec status to detect headphone hotplug event. Now this patch does the same job in SOF runtime pm functions.
And we need to check all the non-hdmi codecs for some cases like playback with HDMI or capture with DMIC connected to dsp. In these cases, only controller is active and codecs are suspended, so codecs can't send unsolicited event to controller. The jack polling operation will activate codecs and unsolicited event can work even codecs become suspended later.
Tested on whiskylake with hda codecs.
Signed-off-by: Rander Wang rander.wang@linux.intel.com Signed-off-by: Pierre-Louis Bossart < pierre-louis.bossart@linux.intel.com> Signed-off-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com
sound/soc/sof/intel/hda-codec.c | 45 +++++++++++++++++++++++++++++++-- sound/soc/sof/intel/hda-dsp.c | 29 ++++++++++++++++----- sound/soc/sof/intel/hda.h | 4 +++ 3 files changed, 70 insertions(+), 8 deletions(-)
diff --git a/sound/soc/sof/intel/hda-codec.c b/sound/soc/sof/intel/hda-codec.c index b8b37f082309..c711792534da 100644 --- a/sound/soc/sof/intel/hda-codec.c +++ b/sound/soc/sof/intel/hda-codec.c @@ -10,6 +10,7 @@
#include <linux/module.h> #include <sound/hdaudio_ext.h> +#include <sound/hda_register.h> #include <sound/hda_codec.h> #include <sound/hda_i915.h> #include <sound/sof.h> @@ -37,16 +38,55 @@ static void hda_codec_load_module(struct hda_codec *codec) static void hda_codec_load_module(struct hda_codec *codec) {} #endif
+/* check jack status after resuming from suspend mode */ +void hda_codec_jack_check(struct snd_sof_dev *sdev, int status) +{
- struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata;
- struct hda_bus *hbus = sof_to_hbus(sdev);
- struct hdac_bus *bus = sof_to_bus(sdev);
- struct hda_codec *codec;
- int mask;
- /*
* there are two reasons for runtime resume
* (1) waken up by interrupt triggered by WAKEEN feature
* (2) waken up by pm get functions for some audio operations
* For case (1), the bits in status mean which codec triggers
* the interrupt and jacks will be checked on these codecs.
* For case (2), we need to check all the non-hdmi codecs for
some
* cases like playback with HDMI or capture with DMIC. In these
* cases, only controller is active and codecs are suspended,
so
* codecs can't send unsolicited event to controller. The jack
polling
* operation will activate codecs and unsolicited event can
work
* even codecs become suspended later.
*/
This block is huge. I'd suggest entering some whitespaces to make it more readable.
On the second thought, I bet this stuff is not SOF specific and if so, being more strict may prove more informative than being so explicit
reference to HDA spec/ kernel HDA documentation etc.
- mask = status ? status : hda->hda_codec_mask;
- list_for_each_codec(codec, hbus)
if (mask & BIT(codec->core.addr))
schedule_delayed_work(&codec->jackpoll_work,
codec-
jackpoll_interval);
- /* disable controller Wake Up event*/
- snd_hdac_chip_writew(bus, WAKEEN,
snd_hdac_chip_readw(bus, WAKEEN) &
~hda->hda_codec_mask);
Any reason for not using snd_hdac_chip_updatew?
Thanks, Cezary for the feedback. We're working to optimize the jack detection logic during runtime suspend. Let me get back with a v2 and address your concerns.
Thanks, Ranjani
On Wed, Jun 26, 2019 at 11:03:14PM +0200, Cezary Rojewski wrote:
On 2019-06-26 22:22, Ranjani Sridharan wrote:
From: Rander Wang rander.wang@linux.intel.com
In commit 7d4f606c50ff ("ALSA: hda - WAKEEN feature enabling for runtime pm"), legacy HD-A driver sets hda controller in reset mode after entering runtime-suspend. And when resuming from suspend mode, it checks
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.
From: Pan Xiuli xiuli.pan@linux.intel.com
If last_busy value is not set at runtime PM enable, the device will be suspend immediately after usage counter is 0. Set the last_busy value to make sure delay is working at first boot up.
Signed-off-by: Pan Xiuli xiuli.pan@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/sof-pci-dev.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/sound/soc/sof/sof-pci-dev.c b/sound/soc/sof/sof-pci-dev.c index ffe485f4ba71..3e3ee9c9aefd 100644 --- a/sound/soc/sof/sof-pci-dev.c +++ b/sound/soc/sof/sof-pci-dev.c @@ -232,6 +232,9 @@ static void sof_pci_probe_complete(struct device *dev) */ pm_runtime_allow(dev);
+ /* mark last_busy for pm_runtime to make sure not suspend immediately */ + pm_runtime_mark_last_busy(dev); + /* follow recommendation in pci-driver.c to decrement usage counter */ pm_runtime_put_noidle(dev); }
participants (3)
-
Cezary Rojewski
-
Mark Brown
-
Ranjani Sridharan