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

From: Daniel Vetter <daniel@ffwll.ch>
To: Jani Nikula <jani.nikula@intel.com>
Cc: intel-gfx <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH v3] drm/i915: rewrite hsw/bdw audio codec enable/disable sequences
Date: Mon, 3 Nov 2014 13:09:02 +0100	[thread overview]
Message-ID: <20141103120902.GS26941@phenom.ffwll.local> (raw)
In-Reply-To: <87h9ykiovy.fsf@intel.com>

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx