On Thu, Sep 05, 2013 at 01:45:47PM +0200, Daniel Vetter wrote: > 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@xxxxxxxxx> 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. Yeah, for enabling we definitely want to execute all that stuff asynchronously. The disable case is more problematic. For atomic pageflip we might get away with returning -EAGAIN or something, but we have to block in the full modeset case unless we want to move the entire modeset to a work queue. So this trick would mostly be relevant for the disable during modeset case. But of course we could do that stuff via some kind of schedule_vblank_work(currentl_vbl_count + X) and synchronize_vblank_work() just before the critical step that needs the vblank work to be done. But the locking and bugs might be more complicated in this case. -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx