Re: [PATCH 4/4] ALSA: hda - Wake the codec up on pin/ELD notify events

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





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?


So I believe it's easier to avoid calling the eld_notify callback from
i915 side during its suspend code.


Takashi


--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux