On to, 2015-11-26 at 16:29 +0100, David Henningsson wrote: > > On 2015-11-26 16:23, Takashi Iwai wrote: > > On Thu, 26 Nov 2015 16:08:06 +0100, > > David Henningsson wrote: > > > > > > > > > > > > On 2015-11-26 10:24, Takashi Iwai wrote: > > > > On Thu, 26 Nov 2015 10:16:17 +0100, > > > > Zhang, Xiong Y wrote: > > > > > > > > > > > > > > > > On Thu, 26 Nov 2015 08:57:30 +0100, > > > > > > Zhang, Xiong Y wrote: > > > > > > > > > > > > > > > > > BTW, I have a patchset to avoid the audio h/w > > > > > > > > > > wakeup by a new > > > > > > > > > > component ops to get ELD and connection states. It > > > > > > > > > > was posted to > > > > > > > > > > alsa-devel shortly ago just as a reference, but > > > > > > > > > > this should work well > > > > > > > > > > in such a case, too. The test patches are found in > > > > > > > > > > test/hdmi-jack > > > > > > > > > > branch of my sound git tree. > > > > > > > > > > > > > > > > Did you try this branch (merge onto intel-test- > > > > > > > > nightly)? > > > > > > > > > > > > > > > [Zhang, Xiong Y] Yes, this branch could fix my issue. > > > > > > > > > > > > OK, good to hear. But this isn't for 4.4 or backport, as > > > > > > it's more > > > > > > radical changes. > > > > > > > > > > > > Could you check the patch below instead? This suppresses > > > > > > the notifier > > > > > > handling during suspend. It's bad, but the hotplug should > > > > > > be still > > > > > > handled via normal unsol event, so it should keep working, > > > > > > good enough > > > > > > as a stop gap. > > > > > > > > > > > > > > > > > > Takashi > > > > > > > > > > > > --- > > > > > > diff --git a/sound/pci/hda/patch_hdmi.c > > > > > > b/sound/pci/hda/patch_hdmi.c > > > > > > index bdb6f226d006..f7c70e2ae65c 100644 > > > > > > --- a/sound/pci/hda/patch_hdmi.c > > > > > > +++ b/sound/pci/hda/patch_hdmi.c > > > > > > @@ -33,6 +33,7 @@ > > > > > > #include <linux/delay.h> > > > > > > #include <linux/slab.h> > > > > > > #include <linux/module.h> > > > > > > +#include <linux/pm_runtime.h> > > > > > > #include <sound/core.h> > > > > > > #include <sound/jack.h> > > > > > > #include <sound/asoundef.h> > > > > > > @@ -2352,7 +2353,9 @@ static void intel_pin_eld_notify(void > > > > > > *audio_ptr, int > > > > > > port) > > > > > > struct hda_codec *codec = audio_ptr; > > > > > > int pin_nid = port + 0x04; > > > > > > > > > > > > - check_presence_and_report(codec, pin_nid); > > > > > > + if (!atomic_read(&codec->core.in_pm) && > > > > > > + !pm_runtime_suspended(hda_codec_dev(codec))) > > > > > > + check_presence_and_report(codec, pin_nid); > > > > > > } > > > > > > > > > > > > static int patch_generic_hdmi(struct hda_codec *codec) > > > > > [Zhang, Xiong Y] this patch couldn't remove the error > > > > > message. So audio controller isn't in Runtime D3 after > > > > > azx_suspend(). > > > > > > > > OK, then the problem isn't about the HD-audio driver but rather > > > > i915 > > > > driver. As already mentioned, i915 driver shouldn't issue > > > > eld_notify > > > > callback in the suspend code path. > > > > > > We don't have a complete drm log so I'm only speculating here; > > > but the > > > HDA log seems to indicate the power well is off when we try to > > > talk to > > > the HDA controller. That means either the i915 shut it down while > > > we > > > were holding a lock on it, or HDA tried to lock it and that > > > didn't make > > > it through; in which case the HDA side should handle an error > > > from > > > get_power more gracefully...? > > > > My understanding is that it's triggered *during* i915 > > suspend. Then > > the path calls the HDA callback, and HDA controller tries to get > > power > > and proceeds as it has no idea that it's being shut off. > > Well; that can also be stopped by letting the get_power call return a > failure code in case i915 is trying to suspend itself. That seems > more > robust to me, as it could potentially avoid other S3 races too...? > > > > > Unfortunately, neither get_power callback or the corresponding HDA > > code has a capability to check that state. So, changing / fixing > > this > > there would be nice but very hard. > > Surely the i915 driver has some "am_i_suspending" variable it can > check > in the get_power callback? As I understand this happens during the i915 system suspend/resume hooks are running. There is no reason why the getting a power domain reference would fail there. In fact we are keeping all power wells for the whole duration of these callbacks, see the call to intel_display_set_init_power(true) in i915_drm_suspend() and i915_drm_resume_early()->intel_power_domains_init_hw(). So not sure how the audio power well could be off at that time. > > > > > So I believe it's easier to avoid calling the eld_notify callback > > from > > i915 side during its suspend code. > > > > > > Takashi > > > _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
