[Intel-gfx] [PATCH v2] drm/i915: Limit audio CDCLK>=2*BCLK constraint back to GLK only

[Ville Syrjälä]

On Thu, Mar 12, 2020 at 07:27:58PM +0200, Kai Vehmanen wrote:
> Hey,
> 
> On Tue, 10 Mar 2020, Ville Syrjälä wrote:
> 
> > On Tue, Mar 10, 2020 at 07:18:58PM +0200, Kai Vehmanen wrote:
> >> On Tue, 10 Mar 2020, Ville Syrjälä wrote:
> >>> audio at init time. And we could maybe try to remove the modeset from the
> >>> put_power() so that at least if you get a blink it's just the one. I did
> >>
> >> Hmm, this is interesting and maybe a better compromise for the in-between 
> >> generations. Could it be as simple as not setting 
> > 
> > The logic around the cdclk computation is still a bit messy.
> > 
> > First draft of just doing the lazy force_min_cdclk reduction in put_power():
> > git://github.com/vsyrjala/linux.git no_cdclk_in_audio_put_power
> > 
> > Very lightly smoke tested, but not sure if it achieves anything useful :P
> 
> I tested this today and no issues found. I can see clock bumped if there 
> is audio activity, but clock is kept after audio goes back to sleep. 
> But then e.g. at next display-off timeout, clk is brought back down.
> So works as expected.
> 
> But, but, then I also tested...
> 
> >> 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 
> 
> Now actually hitting this requires some effort. On most systems I tried, 
> with display active, the clock will stay above the limit for other 
> reasons, but yup, when this happens, it is pretty, pretty bad.
> 
> Your no_cdclk_in_audio_put_power patch does reduce the level of annoyance 
> also in this case -- you only get one flash instead of two. But does not 
> seem acceptable still. If you happen to have a system where the conditions 
> are met, then this happens all the time. In case of UI notification sounds 
> being the trigger, we could consider the visual flash as a feature, but 
> probably not widely appreciated. ;) .. and especially as you cannot turn 
> it off.
> 
> So I think this starts to look that we should move calling glk_force_audio 
> to bind/unbind pair. I can make a patch for this.

That would stop us from doing dynamic cdclk changes once we get the hw
that can do that properly. Rather I think I'd just hardcode the 2xbclk
requirement in i915 for the platforms that suck.

-- 
Ville Syrjälä
Intel