Re: [PATCH v2] drm/i915/hsw: Add display Audio codec disable sequence for Haswell

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Sep 05, 2013 at 02:33:20PM +0300, Ville Syrjälä wrote:
> On Wed, Sep 04, 2013 at 08:50:13PM +0200, Daniel Vetter wrote:
> > On Fri, Aug 30, 2013 at 1:50 AM,  <mengdong.lin@intel.com> wrote:
> > > +       /* Wait for 2 vertical blanks */
> > > +       intel_wait_for_vblank(dev, pipe);
> > > +       intel_wait_for_vblank(dev, pipe);
> > > +
> > > +       /* Disable audio PD. This is optional as per Bspec.  */
> > > +       temp = I915_READ(HSW_AUD_PIN_ELD_CP_VLD);
> > > +       temp &= ~(AUDIO_OUTPUT_ENABLE_A << (pipe * 4));
> > > +       I915_WRITE(HSW_AUD_PIN_ELD_CP_VLD, temp);
> > 
> > If this is optional do we really need the two vblank waits above?
> > Adding them just for fun when we generally try to rip out as many
> > vblank waits as possible from the modeset code isn't all that great
> > ...
> 
> One idea I had for these kinds of vblank waits (there also one required
> for IPS for instance) is that we might just sample a vblank counter
> after the first step, then at the latest point we can, we'd wait for the
> frame counter to have passed the sampled vaoue + whatever extra is
> needed. That might allow us to do other stuff in parallel while the
> required number of vblanks will elapese.

My solution for this is to have vblank work items that we can use to chain
off all these things. We also need them for pageflips e.g. when
re-enabling fbc or similar stuff. The problem is a bit that for switching
things off like in a modeset the synchronization can get hairy and will be
little-tested. For enabling as long as we share the code with the nuclear
pageflip code we should be fine though ...

Hence why I think we should try rather hard to avoid these vblank waits in
the first place.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch