Skip to content

ASoC: SOF: Intel: hda: call snd_hda_set_power_save from SOF code #1689

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
juimonen wants to merge 1 commit into from
Closed

ASoC: SOF: Intel: hda: call snd_hda_set_power_save from SOF code #1689

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
9 changes: 9 additions & 0 deletions sound/soc/sof/intel/hda.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -597,6 +597,15 @@ int hda_dsp_probe(struct snd_sof_dev *sdev)
/* set default mailbox offset for FW ready message */
sdev->dsp_box.offset = HDA_DSP_MBOX_UPLINK_OFFSET;

/*
* set pm suspend delay for the hda codec. Use the same value as SOF
* uses. If this is not set, HDA codec will suspend with 0ms suspend
* delay i.e. very quickly after idle. Very quick wakeup/suspend might
* cause for example jack inserts not being detected as the codec setup
* and jack connection measurement need some time.
*/
snd_hda_set_power_save(sof_to_hbus(sdev), SND_SOF_SUSPEND_DELAY_MS);
Copy link
Member

@lgirdwood lgirdwood Jan 11, 2020

Choose a reason for hiding this comment

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

This can do with a comment explaining why this is being manually done here so anyone looking at the code undertsnds the flow with HDA codec.

Copy link
Author

@juimonen juimonen Jan 16, 2020

Choose a reason for hiding this comment

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

added comment


Copy link
Member

@plbossart plbossart Jan 15, 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 far from clear to me, so I don't get the problem statement;

Even though SOF sets its own suspend delay, the possible HDA codec of
the driver ignores it.

SOF sets is own pm_runtime autosuspend delay?
what does "the possible HDA codec of the driver ignores it." mean? What is ignored by what? Which driver?

 So even as SOF sets the suspend delay to 2s, hda
codec will suspend immediately or depending on the value of
CONFIG_SND_HDA_POWER_SAVE_DEFAULT.

Why? Is this because there is no parent-child relationship between the SOF PCI device and the codec device? What causes this behavior? You are describing the result, not the reason for the immediate suspend.

This will lead in some cases to jack
pins being detected incorrectly.

Why? what does pin/jack detection have to do with suspend?

So set the suspend delay time in HDA
codec to same value as SOF tries to set itself.

why does this solve anything? The autosuspend delay on the SOF side is not a strict value, it's subject to kernel scheduling, so I don't see how you avoid a race condition with transitions on the codec side. If you can't control the scheduling, you have to use a significantly larger value on one side so that even in the worst possible case nothing bad happens.

INIT_DELAYED_WORK(&hdev->d0i3_work, hda_dsp_d0i3_work);

return 0;
Expand Down