Hi folks,
[+Takashi from ALSA]
On Mon, 6 Jan 2020, Matt Roper wrote:
>>> On Tue, Dec 31, 2019 at 04:00:07PM +0200, Kai Vehmanen wrote:
>>>> Revert changes done in commit f6ec9483091f ("drm/i915: extend audio
>>>> CDCLK>=2*BCLK constraint to more platforms"). Audio drivers
>
> Agreed; GLK's minimum cdclk goes down to 79.2 mhz so it makes sense that
> it needs to be handled differently than CNL (and beyond). I.e., this
> isn't a pure revert of the original patch.
unfortunately it seems this fix that was done is not holding up in wider
testing. It now looks we need to enforce the constraint in one form or
another for newer platforms as well. We can't revert the revert as that
will bring the boot flicker back, so we'll need something else.
Now as we've gone back-and-forth multiple times, I want to get some early
feedback before opting for one path or another.
To recap in short:
- audio driver calls i915 acomp get_power() multiple times during boot
-> this can cause annoying flicker at boot on platforms where
each get_power() leads to a modeset change
- example: https://gitlab.freedesktop.org/drm/intel/issues/913
- systems with more complex audio subsystems (DSP enabled) will be
calling get_power() many more times (as i915 iDisp link is needed in
multiple phases of audio controller, DSP and codecs initialization),
making the flicker worse
I've gone through (once more) possible options, and it starts to seem that
trying to minimize # of get_power() cycles is not going to work well in
the long run. We could certainly reduce number of distinct get_power
calls e.g. during boot by refactoring the audio DSP setup, but there would
still be more than a few, and it's not just the boot. We now need to call
get_power() when the audio controller comes out from runtime suspend
(otherwise the HDA link is not ok between i915 and audio). This can be pretty
annoying if there are visible artifacts to the end-user in such a case
where potentially no HDMI/DP monitors are even connected).
Similarly on i915 side, it would seem pretty unlikely that we are going
to get smooth changes of CDCLK. It might work better on some platforms,
but generally (depending on the available dividers provided), it's not
going to be guaranteed to be flicker-free.
So how about: We move the glk_force_audio_cdclk() logic from
intel_audio.c:i915_audio_component_get_power() to acomp init.
This has some notable implications:
- audio driver can safely call get_power without worrying
about creating display artifacts,
- on some platforms, with specific HDA link params in BIOS,
this will mean some lower CDCLK frequencies, will not be used
at all
- e.g. on ICL system with 96Mhz BCLK for iDisp, 172800 and
180000 are blocked out, and 192000 is the practical minimum
unless you unload the audio driver
- if BCLK is 48Mhz, no impact to CDCLK selection on ICL
Any chance to get this through? I understand this effectively removes the
lower clocks from some systems, so this needs to be evaluated carefully.
I don't really have other options on the table now. We could maybe use
idle-timers to delay powering off the audio domain until certain amount of
inactivity, but this is both ugly and won't solve the full thing. Or we
just keep reducing get_power() calls on audio side, and just mitigate the
the severity of the flicker -- again not fully solving the problem.
Thoughts and feedback is welcome.
Br, Kai
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
