On Wed, 11 Mar 2020 13:16:56 +0100,
Kai Vehmanen wrote:
>
> Hey,
>
> On Tue, 10 Mar 2020, Takashi Iwai wrote:
> > On Tue, 10 Mar 2020 19:25:22 +0100, Ville Syrjälä wrote:
> >> On Tue, Mar 10, 2020 at 07:18:58PM +0200, Kai Vehmanen wrote:
> >>> One problematic scenario that this doesn't cover:
> >>> - a single display is used (at low cdclk), and
> >>> - audio block goes to runtime suspend while display stays up.
> >>>
> >>> Upon resume (for e.g. UI notification sound), audio will initialize the
> >>> HDA bus and call get_power() on i915, even if the notification goes to
> >>> internal speaker. A modeset at this point is potentially very annoying.
> >>
> >> :( That seems much harder to deal with.
> >
> > I guess it doesn't happen -- at least with the legacy HD-audio and the
> > recent chip, if I understand correctly. When the stream is on the
> > analog codec, the HDMI codec is kept closed / runtime-resumed. And
> > the additional get_power() in the controller side is done only for
> > HSW/BDW (where the HDA-bus is dedicated to HDMI).
>
> I'm afraid it does -- I double checked the legacy driver code. Judging
> from history, since commit "ALSA: hda - Fix Skylake codec timeout",
> azx_runtime_resume() has called "display_power(chip, true)" on all
> platforms, even if the stream is on analog codec. I just recently fixed
> this logic to SOF driver as well. If you don't do this and the link
> parameters are different from hw defaults on i915 side (more likely with
> ICL and newer), you'll hit problems.
Oh indeed, I overlooked that part.
> I don't currently know of a reliable way to recover. In theory, if HDMI/DP
> monitor is connected, we could do a delayed call to i915 get_power and
> reinitialize the HDA controller, but if we are actively streaming to
> analog codec when this happens, this wouldn't work.
>
> And until very recent times, this has not really been an issue. With
> current i915, many conditions have to be met to actually hit this (only
> affects GLK platforms, you need to be using HDA analog codec, runtime PM
> needs to be enabled for HDA, etc). Problem is that going forward, there
> are going to be more platforms that are affected and this starts to show
> up in more places.
The remaining question is whether this display_power() call is still
needed for the recent chips. Basically it's there for HSW/BDW type
chips that need already the power for the HDA link that is dedicated
to HDMI. That is, a patch like below could work for the recent chips
that share both onboard and HDMI codecs.
Takashi
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -992,9 +992,10 @@ static void __azx_runtime_resume(struct azx *chip, bool
from_rt)
struct hda_codec *codec;
int status;
- display_power(chip, true);
- if (hda->need_i915_power)
+ if (hda->need_i915_power) {
+ display_power(chip, true);
snd_hdac_i915_set_bclk(bus);
+ }
/* Read STATESTS before controller reset */
status = azx_readw(chip, STATESTS);
@@ -1008,10 +1009,6 @@ static void __azx_runtime_resume(struct azx *chip, bool
from_rt)
schedule_delayed_work(&codec->jackpoll_work,
codec->jackpoll_interval);
}
-
- /* power down again for link-controlled chips */
- if (!hda->need_i915_power)
- display_power(chip, false);
}
#ifdef CONFIG_PM_SLEEP
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
