Re: [PATCH v3] drm/i915: rewrite hsw/bdw audio codec enable/disable sequences

Daniel Vetter
2014-11-03T12:08:48Z

On Fri, Oct 31, 2014 at 11:12:33AM +0200, Jani Nikula wrote:
> We have drm_crtc_vblank_off very early in the disable sequence, and
> drm_crtc_vblank_on very late in the enable sequence. Especially
> early/late since
>
> commit 4b3a9526fc3228e74011b88f58088336acd2c9e2
> Author: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Date: Thu Aug 14 22:04:37 2014 +0300
>
> drm/i915: Move vblank enable earlier and disable later
>
> All of the audio codec enable/disable stuff, like most of mode set, is
> done with vblank disabled.
>
> Since
>
> commit 44bd93a3d367913d883be6abba9a6e51a53c4e90
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date: Fri Jul 25 23:36:44 2014 +0200
>
> drm/i915: Use generic vblank wait
>
> we started using drm_wait_one_vblank instead of directly checking the
> registers in intel_wait_for_vblank, and this specifically disallows its
> use during mode set (and IIRC we've seen bug reports about that too).
>
> I don't know what the correct answer should be here. I've ensured the
> audio codec enable doesn't happen until after the port/pipe are up and
> running, but I still can't vblank wait.

Yeah, this is a big ugly. We really can't enable vblanks earlier since on
some ports the pipe really only starts running later on.

Imo the proper fix is to push the audio/infoframe setup into a vblank work
item, and the code here would only schedule that on the very next vblank.
Unfortunately that infrastructure isn't there yet, so meanwhile I think we
need to locally resurrect the old vblank_wait functions with a big XXX
comment.

Or given that we've never really cared about actually running in the
vblank (the wait_for macro uses msleeps, so we're pretty much guaranteed
to miss the actual vblank) just insert FIXME comments stating what should
be done here.

I'm ok with either approach really.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch