Re: [PATCH 3/3] drm/i915: remove the vblank_wait hack from HSW+

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Feb 12, 2014 at 01:13:17PM +0200, Ville Syrjälä wrote:
> On Thu, Dec 19, 2013 at 07:12:31PM -0200, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > 
> > When I forked haswell_crtc_enable I copied all the code from
> > ironlake_crtc_enable. The last piece of the function contains a big
> > comment with a call to intel_wait_for_vblank. After this fork, we
> > rearranged the Haswell code so that it enables the planes as the very
> > last step of the modeset sequence, so we're sure that we call
> > intel_enable_primary_plane after the pipe is really running, so the
> > vblank waiting functions work as expected. I really believe this is
> > what fixes the problem described by the big comment, so let's give it
> > a try and get rid of that intel_wait_for_vblank, saving around 16ms
> > per modeset (and init/resume). We can always revert if needed :)
> 
> I noticed this got merged, but I'd actually prefer we go the other way.
> Ie. remove the vblank wait from enable_primary_plane(). We're going to
> want to use the atomic update also for enabling the planes at modeset
> soon enough, so I think this change is going in the wrong direction.
> 
> I think the only issue with dropping the vblank wait from primary
> enable is the IPS enable, but that should be made async anyway. For
> now we could just move the vblank wait into enable_ips().

Oh and additionally, if we want to, we could get rid of the vblank wait
(at least on some platforms) by  using the hardware flip counter to make
sure we don't prematurely complete a subsequent page flip.

> 
> > 
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 10 ----------
> >  1 file changed, 10 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index f0f78d3..4f933f2 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -3720,16 +3720,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
> >  	 * to change the workaround. */
> >  	haswell_mode_set_planes_workaround(intel_crtc);
> >  	haswell_crtc_enable_planes(crtc);
> > -
> > -	/*
> > -	 * There seems to be a race in PCH platform hw (at least on some
> > -	 * outputs) where an enabled pipe still completes any pageflip right
> > -	 * away (as if the pipe is off) instead of waiting for vblank. As soon
> > -	 * as the first vblank happend, everything works as expected. Hence just
> > -	 * wait for one vblank before returning to avoid strange things
> > -	 * happening.
> > -	 */
> > -	intel_wait_for_vblank(dev, intel_crtc->pipe);
> >  }
> >  
> >  static void ironlake_pfit_disable(struct intel_crtc *crtc)
> > -- 
> > 1.8.3.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

-- 
Ville Syrjälä
Intel OTC