Skip to content

Conversation

@juimonen
Copy link

SOF sets its own pm_runtime suspend delay, but never calls
snd_hda_set_power_save for the HDA codec. This means that HDA codec will
have no pm_runtime suspend delay at all, so it will suspend very fast
after idle. This is not very common "mode" for the codec PM as for
example legacy HDA is always setting some pm_runtime suspend delay or
disabling the PM altogether.

If SOF is in suspend and there's a jack event, SOF will wake up, but the
HDA codec will go immediately to suspend. We noticed that with some
codecs (like ALC285) the quick suspend after wake up is messing up the
jack detection as the codec might need some time for the detect
measurement.

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

Signed-off-by: Jaska Uimonen jaska.uimonen@linux.intel.com

SOF sets its own pm_runtime suspend delay, but never calls
snd_hda_set_power_save for the HDA codec. This means that HDA codec will
have no pm_runtime suspend delay at all, so it will suspend very fast
after idle. This is not very common "mode" for the codec PM as for
example legacy HDA is always setting some pm_runtime suspend delay or
disabling the PM altogether.

If SOF is in suspend and there's a jack event, SOF will wake up, but the
HDA codec will go immediately to suspend. We noticed that with some
codecs (like ALC285) the quick suspend after wake up is messing up the
jack detection as the codec might need some time for the detect
measurement.

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

Signed-off-by: Jaska Uimonen <jaska.uimonen@linux.intel.com>
@ClarexZhou
Copy link

One side effect on Mantis via below steps:

  1. Connect Headset
  2. "sudo reboot" to reboot system
  3. Open sound setting ASAP, just after boot up. Paplay to check-> no sound output.
    Recover method: Close sound setting and stop paplay, wait till runtime PM status to suspend, then paplay again, audio can output successfully.

In step3, if open paplay ASAP instead of open sound setting, error also can be reproduced.

This issue not exist without this PR

@lyakh
Copy link
Collaborator

lyakh commented Jan 20, 2020

One side effect on Mantis via below steps:

1. Connect Headset

2. "sudo reboot" to reboot system

3. Open sound setting ASAP, just after boot up. Paplay to check-> no sound output.
   Recover method: Close sound setting and stop paplay, wait till runtime PM status to suspend, then paplay again, audio can output successfully.

In step3, if open paplay ASAP instead of open sound setting, error also can be reproduced.

This issue not exist without this PR

This seems like a problem with the startup processing, which must be fixed, but it doesn't necessarily mean, that this patch is wrong.

Do we understand how it works without this patch? This patch doesn't change the reboot behaviour, so, at the end of the probing the status should be the same with or without this patch. But then without this patch the codec immediately suspends, whereas with this patch it doesn't. That means, that if in that state we suspend the codec with headphones plugged in, we immediately get a wake up event from the codec, as if the headphones were just plugged in? Once we understand that well, we should be able to also fix this issue with this patch.

@plbossart
Copy link
Member

@juimonen can we conclude if this PR is needed or not, and make it go away otherwise?

@juimonen
Copy link
Author

juimonen commented Feb 5, 2020

@plbossart I've made my opinion clear: this is needed and the right thing to do.

Issues that support this PR:

  • This makes the mic jack detection work in lenovo X1
  • This makes sof hda part behave more like legacy hda: legacy hda will never set immediate suspend which will happen without this patch.

Issues against:

  • Similar PR was added to our 5.0 branch by @ranj063 and then removed by @RanderWang because it made detection fail at boot time in some Dell devices.

I'm not sure how to tackle the boot issue, but not merging this PR is not the way to go forward. I really have no idea how to satisfy both lenovo and dell case. We should have the PM off in boot and then enable it later?

@plbossart
Copy link
Member

@juimonen well, put yourself in my shoes. If we have a fix for Lenovo that breaks Dell, should we merged this at all...

@plbossart plbossart added the Unclear No agreement on problem statement and resolution label Feb 5, 2020
@juimonen
Copy link
Author

juimonen commented Feb 5, 2020

@plbossart yes I understand. My point still is that this is a correct thing to do overall, and the Dell should be fixed in some other way. I think it is really strange operational mode that sof suspends in 2s, but codec is put to sleep at light speed. As I understand it is also strange from PM point of view having 0 suspend delay. Legacy HDA is either 3s, some integer value set in kernel config, someone setting something from userspace (I don't know what is the resolution there) or PM disabled (never sleep).

I can only say, that maybe I have to start digging why Dell needs this obscure 0 suspend delay (PM enabled) in boot for the detection to work...

@plbossart
Copy link
Member

@plbossart yes I understand. My point still is that this is a correct thing to do overall, and the Dell should be fixed in some other way. I think it is really strange operational mode that sof suspends in 2s, but codec is put to sleep at light speed. As I understand it is also strange from PM point of view having 0 suspend delay. Legacy HDA is either 3s, some integer value set in kernel config, someone setting something from userspace (I don't know what is the resolution there) or PM disabled (never sleep).

I can only say, that maybe I have to start digging why Dell needs this obscure 0 suspend delay (PM enabled) in boot for the detection to work...

If you have access to both devices, I'd start from the legacy driver on both, see what's going on, then apply similar fixes on the SOF side. it's painful I know but we all have to deal with hardware quirks.

@plbossart
Copy link
Member

@juimonen can this be closed?

@juimonen
Copy link
Author

yes we have solution sent upstream, so closing

@juimonen juimonen closed this Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Unclear No agreement on problem statement and resolution

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants