Skip to content

ASoC: hdac_hda: let the hda codec driver autosuspend #1837

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jason77-wang wants to merge 1 commit into from
Closed

ASoC: hdac_hda: let the hda codec driver autosuspend #1837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions sound/soc/codecs/hdac_hda.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ static const struct snd_soc_dai_ops hdac_hda_dai_ops = {
static struct snd_soc_dai_driver hdac_hda_dais[] = {
{
.id = HDAC_ANALOG_DAI_ID,
.name = "Analog Codec DAI",
.name = HDAC_ANALOG_CODEC_DAI_NAME,
.ops = &hdac_hda_dai_ops,
.playback = {
.stream_name = "Analog Codec Playback",
Expand Down Expand Up @@ -127,7 +127,7 @@ static struct snd_soc_dai_driver hdac_hda_dais[] = {
},
{
.id = HDAC_HDMI_0_DAI_ID,
.name = "intel-hdmi-hifi1",
.name = HDAC_HDMI0_DAI_NAME,
.ops = &hdac_hda_dai_ops,
.playback = {
.stream_name = "hifi1",
Expand Down
3 changes: 3 additions & 0 deletions sound/soc/codecs/hdac_hda.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@ enum {
HDAC_LAST_DAI_ID = HDAC_HDMI_3_DAI_ID,
};

#define HDAC_ANALOG_CODEC_DAI_NAME "Analog Codec DAI"
#define HDAC_HDMI0_DAI_NAME "intel-hdmi-hifi1"

struct hdac_hda_pcm {
int stream_tag[2];
unsigned int format_val[2];
Expand Down
34 changes: 33 additions & 1 deletion sound/soc/intel/boards/skl_hda_dsp_generic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,29 @@ static int skl_hda_fill_card_info(struct snd_soc_acpi_mach_params *mach_params)
return 0;
Copy link
Collaborator

@kv2019i kv2019i Apr 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jason77-wang For the commit message. The driver is already using autosuspend, so the summary is not really accurate. How about:

ASoC: intel/skl/hda - set autosuspend timeout for hda analog codecs

Copy link
Member

@plbossart plbossart Apr 1, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the commit message is also a bit unclear. Assume that all devices are already suspended, and the jack is plugged. something causes a wake-up to handle the first unsolicited answer for headphone. If the devices go back to suspend, what causes the second unsolicited answer for mic to be missed?
In other words, why can we handle the first case and not the second? It's not the suspended status, something is missing in the explanation.

Copy link
Author

@jason77-wang jason77-wang Apr 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jason77-wang For the commit message. The driver is already using autosuspend, so the summary is not really accurate. How about:

ASoC: intel/skl/hda - set autosuspend timeout for hda analog codecs

OK, will change it.

Copy link
Collaborator

@kv2019i kv2019i Apr 2, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@plbossart This was the recommendation from Realtek. Jack detection causes the wake-up from D3, but the detection routine takes time to complete and sent out all the unsolicited response, if driver puts codec immediately back to sleep, it won't wake up again and we lose some responses -- i.e. this is a feature of some codecs. I think the commit msg accurately describes what is seen.

Copy link
Author

@jason77-wang jason77-wang Apr 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the commit message is also a bit unclear. Assume that all devices are already suspended, and the jack is plugged. something causes a wake-up to handle the first unsolicited answer for headphone. If the devices go back to suspend, what causes the second unsolicited answer for mic to be missed?
In other words, why can we handle the first case and not the second? It's not the suspended status, something is missing in the explanation.

That is sth I don't know how to explain, I did a test before, I did not enable the unsol event on headphone pin, only enable the unsol event on the mic pin:
With the legacy hda driver, the mic can generate the unsol event when plugging/unplugging.
With the sof driver, the mic can't generate the unsol event when plugging/unplugging.
It looks like the mic need to depend on the headphone with the sof driver.

}

#define HDA_CODEC_AUTOSUSPEND_DELAY_MS 1000

static void skl_set_hda_codec_autosuspend_delay(struct snd_soc_card *card,
const char *codec_dai_name)
{
struct snd_soc_dai *codec_dai;
struct hdac_hda_priv *hda_pvt;
struct hdac_device *hdev;

codec_dai = snd_soc_card_get_codec_dai(card, codec_dai_name);
if (!codec_dai)
return;

hda_pvt = snd_soc_component_get_drvdata(codec_dai->component);
hdev = &hda_pvt->codec.core;
pm_runtime_get_noresume(&hdev->dev);
pm_runtime_set_autosuspend_delay(&hdev->dev,
HDA_CODEC_AUTOSUSPEND_DELAY_MS);
pm_runtime_use_autosuspend(&hdev->dev);
pm_runtime_mark_last_busy(&hdev->dev);
pm_runtime_put_autosuspend(&hdev->dev);
}

static int skl_hda_audio_probe(struct platform_device *pdev)
{
struct snd_soc_acpi_mach *mach;
Expand Down Expand Up @@ -206,7 +229,16 @@ static int skl_hda_audio_probe(struct platform_device *pdev)
hda_soc_card.components = hda_soc_components;
}

return devm_snd_soc_register_card(&pdev->dev, &hda_soc_card);
ret = devm_snd_soc_register_card(&pdev->dev, &hda_soc_card);
if (!ret) {
/* set hda analog codec */
skl_set_hda_codec_autosuspend_delay(&hda_soc_card,
HDAC_ANALOG_CODEC_DAI_NAME);
/* set hda hdmi/dp codec */
skl_set_hda_codec_autosuspend_delay(&hda_soc_card,
HDAC_HDMI0_DAI_NAME);
}
return ret;
}

static struct platform_driver skl_hda_audio = {
Expand Down