* HDMI Repoll after pm? @ 2015-12-10 9:47 David Henningsson 2015-12-10 9:53 ` Takashi Iwai 0 siblings, 1 reply; 6+ messages in thread From: David Henningsson @ 2015-12-10 9:47 UTC (permalink / raw) To: alsa-devel, tiwai Hi Takashi, A recent change in sound.git seems to have introduced some repoll after pm. Is this an intentional change? I don't think it's wrong per se (it stops after 6 repolls), but it breaks my hda-emu tester when "list" does not return the expected input. To reproduce, try running: ./hda-emu codecs/canonical/92hd91bxx-dell-inspiron-5423-ccert-201208-11456 -i 1 > pm .... lots of data .... ** TEST_PM DONE ** HDMI status: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=0 JACK report HDMI/DP, status 0 > list 1: HDMI/DP Jack:0 (1) 2: IEC958 Playback Con Mask:0 (1) 3: IEC958 Playback Pro Mask:0 (1) 4: IEC958 Playback Default:0 (1) 5: IEC958 Playback Switch:0 (1) 6: ELD:0 (1) 7: Playback Channel Map:0 (1) HDMI status: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=0 JACK report HDMI/DP, status 0 > ...notice how the last two lines are somewhat unexpected output for a "list" command, but I can make my tool more robust in case you confirm this is an expected change. // David ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: HDMI Repoll after pm? 2015-12-10 9:47 HDMI Repoll after pm? David Henningsson @ 2015-12-10 9:53 ` Takashi Iwai 2015-12-10 12:59 ` David Henningsson 0 siblings, 1 reply; 6+ messages in thread From: Takashi Iwai @ 2015-12-10 9:53 UTC (permalink / raw) To: David Henningsson; +Cc: alsa-devel On Thu, 10 Dec 2015 10:47:59 +0100, David Henningsson wrote: > > Hi Takashi, > > A recent change in sound.git seems to have introduced some repoll after > pm. Is this an intentional change? I don't think it's wrong per se (it > stops after 6 repolls), but it breaks my hda-emu tester when "list" does > not return the expected input. Hm, there is no such an intended change. > To reproduce, try running: > > ./hda-emu > codecs/canonical/92hd91bxx-dell-inspiron-5423-ccert-201208-11456 -i 1 > > pm > .... lots of data .... > ** TEST_PM DONE ** > HDMI status: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=0 > JACK report HDMI/DP, status 0 > > list > 1: HDMI/DP Jack:0 (1) > 2: IEC958 Playback Con Mask:0 (1) > 3: IEC958 Playback Pro Mask:0 (1) > 4: IEC958 Playback Default:0 (1) > 5: IEC958 Playback Switch:0 (1) > 6: ELD:0 (1) > 7: Playback Channel Map:0 (1) > HDMI status: Codec=3 Pin=5 Presence_Detect=0 ELD_Valid=0 > JACK report HDMI/DP, status 0 > > > > ...notice how the last two lines are somewhat unexpected output for a > "list" command, but I can make my tool more robust in case you confirm > this is an expected change. When the list may trigger some runtime PM, the jack status report itself should have been present at resume in earlier version, too. Do you see the repoll there? Takashi ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: HDMI Repoll after pm? 2015-12-10 9:53 ` Takashi Iwai @ 2015-12-10 12:59 ` David Henningsson 2015-12-10 13:18 ` Takashi Iwai 0 siblings, 1 reply; 6+ messages in thread From: David Henningsson @ 2015-12-10 12:59 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel On 2015-12-10 10:53, Takashi Iwai wrote: > On Thu, 10 Dec 2015 10:47:59 +0100, > David Henningsson wrote: >> >> Hi Takashi, >> >> A recent change in sound.git seems to have introduced some repoll after >> pm. Is this an intentional change? I don't think it's wrong per se (it >> stops after 6 repolls), but it breaks my hda-emu tester when "list" does >> not return the expected input. > > Hm, there is no such an intended change. After testing some more, I've found that reverting e90247f9 (Split ELD update code from hdmi_present_sense) fixes the issue, so it seems like this commit indeed contains a functional change... >> ...notice how the last two lines are somewhat unexpected output for a >> "list" command, but I can make my tool more robust in case you confirm >> this is an expected change. > > When the list may trigger some runtime PM, the jack status report > itself should have been present at resume in earlier version, too. > Do you see the repoll there? To clarify, it's not power up or power down, it's flush_delayed_work that runs after every hda-emu command that causes "list" to also report the jack status. -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: HDMI Repoll after pm? 2015-12-10 12:59 ` David Henningsson @ 2015-12-10 13:18 ` Takashi Iwai 2015-12-10 13:28 ` David Henningsson 0 siblings, 1 reply; 6+ messages in thread From: Takashi Iwai @ 2015-12-10 13:18 UTC (permalink / raw) To: David Henningsson; +Cc: alsa-devel On Thu, 10 Dec 2015 13:59:59 +0100, David Henningsson wrote: > > > > On 2015-12-10 10:53, Takashi Iwai wrote: > > On Thu, 10 Dec 2015 10:47:59 +0100, > > David Henningsson wrote: > >> > >> Hi Takashi, > >> > >> A recent change in sound.git seems to have introduced some repoll after > >> pm. Is this an intentional change? I don't think it's wrong per se (it > >> stops after 6 repolls), but it breaks my hda-emu tester when "list" does > >> not return the expected input. > > > > Hm, there is no such an intended change. > > After testing some more, I've found that reverting e90247f9 (Split ELD > update code from hdmi_present_sense) fixes the issue, so it seems like > this commit indeed contains a functional change... Thanks for chasing it. Does the following patch work better? Takashi --- diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 35a78a6f87a6..2a7d29a07f31 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -1605,6 +1605,7 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) */ int present; bool ret; + bool do_repoll = false; snd_hda_power_up_pm(codec); present = snd_hda_pin_sense(codec, pin_nid); @@ -1629,9 +1630,11 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) eld->eld_size) < 0) eld->eld_valid = false; } + if (!eld->eld_valid && repoll) + do_repoll = true; } - if (!eld->eld_valid && repoll) + if (do_repoll) schedule_delayed_work(&per_pin->work, msecs_to_jiffies(300)); else update_eld(codec, per_pin, eld); ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: HDMI Repoll after pm? 2015-12-10 13:18 ` Takashi Iwai @ 2015-12-10 13:28 ` David Henningsson 2015-12-10 13:38 ` Takashi Iwai 0 siblings, 1 reply; 6+ messages in thread From: David Henningsson @ 2015-12-10 13:28 UTC (permalink / raw) To: Takashi Iwai; +Cc: alsa-devel On 2015-12-10 14:18, Takashi Iwai wrote: > On Thu, 10 Dec 2015 13:59:59 +0100, > David Henningsson wrote: >> >> >> >> On 2015-12-10 10:53, Takashi Iwai wrote: >>> On Thu, 10 Dec 2015 10:47:59 +0100, >>> David Henningsson wrote: >>>> >>>> Hi Takashi, >>>> >>>> A recent change in sound.git seems to have introduced some repoll after >>>> pm. Is this an intentional change? I don't think it's wrong per se (it >>>> stops after 6 repolls), but it breaks my hda-emu tester when "list" does >>>> not return the expected input. >>> >>> Hm, there is no such an intended change. >> >> After testing some more, I've found that reverting e90247f9 (Split ELD >> update code from hdmi_present_sense) fixes the issue, so it seems like >> this commit indeed contains a functional change... > > Thanks for chasing it. It's almost unreal...my tester framework finally caught a bug (that you didn't catch before I saw it!). I cannot believe it, it only took, like, three years or so :-) > Does the following patch work better? It certainly does. Thanks! Tested-by: David Henningsson <david.henningsson@canonical.com> > > > Takashi > > --- > diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c > index 35a78a6f87a6..2a7d29a07f31 100644 > --- a/sound/pci/hda/patch_hdmi.c > +++ b/sound/pci/hda/patch_hdmi.c > @@ -1605,6 +1605,7 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) > */ > int present; > bool ret; > + bool do_repoll = false; > > snd_hda_power_up_pm(codec); > present = snd_hda_pin_sense(codec, pin_nid); > @@ -1629,9 +1630,11 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll) > eld->eld_size) < 0) > eld->eld_valid = false; > } > + if (!eld->eld_valid && repoll) > + do_repoll = true; > } > > - if (!eld->eld_valid && repoll) > + if (do_repoll) > schedule_delayed_work(&per_pin->work, msecs_to_jiffies(300)); > else > update_eld(codec, per_pin, eld); > -- David Henningsson, Canonical Ltd. https://launchpad.net/~diwic ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: HDMI Repoll after pm? 2015-12-10 13:28 ` David Henningsson @ 2015-12-10 13:38 ` Takashi Iwai 0 siblings, 0 replies; 6+ messages in thread From: Takashi Iwai @ 2015-12-10 13:38 UTC (permalink / raw) To: David Henningsson; +Cc: alsa-devel On Thu, 10 Dec 2015 14:28:16 +0100, David Henningsson wrote: > > > > On 2015-12-10 14:18, Takashi Iwai wrote: > > On Thu, 10 Dec 2015 13:59:59 +0100, > > David Henningsson wrote: > >> > >> > >> > >> On 2015-12-10 10:53, Takashi Iwai wrote: > >>> On Thu, 10 Dec 2015 10:47:59 +0100, > >>> David Henningsson wrote: > >>>> > >>>> Hi Takashi, > >>>> > >>>> A recent change in sound.git seems to have introduced some repoll after > >>>> pm. Is this an intentional change? I don't think it's wrong per se (it > >>>> stops after 6 repolls), but it breaks my hda-emu tester when "list" does > >>>> not return the expected input. > >>> > >>> Hm, there is no such an intended change. > >> > >> After testing some more, I've found that reverting e90247f9 (Split ELD > >> update code from hdmi_present_sense) fixes the issue, so it seems like > >> this commit indeed contains a functional change... > > > > Thanks for chasing it. > > It's almost unreal...my tester framework finally caught a bug (that you > didn't catch before I saw it!). I cannot believe it, it only took, like, > three years or so :-) > > > Does the following patch work better? > > It certainly does. Thanks! > > Tested-by: David Henningsson <david.henningsson@canonical.com> Thanks! I merged the fix now. Takashi ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2015-12-10 13:38 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-10 9:47 HDMI Repoll after pm? David Henningsson 2015-12-10 9:53 ` Takashi Iwai 2015-12-10 12:59 ` David Henningsson 2015-12-10 13:18 ` Takashi Iwai 2015-12-10 13:28 ` David Henningsson 2015-12-10 13:38 ` Takashi Iwai
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.