[PATCH] drm/i915: don't frob the vblank ts in finish_page_flip

[PATCH] drm/i915: don't frob the vblank ts in finish_page_flip

[Daniel Vetter]

Now that we correctly generate it, this hack is no longer required (and
might actually paper over a serious bug).

pageflip timestamps are sanity check in the latest version of the flip-test
in intel-gpu-tools.

v2: Also remove the gettimeofday(&now) which is no longer used.
Noticed by Mario Kleiner.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c |   23 +----------------------
 1 file changed, 1 insertion(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 57c1309..67912fe 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6181,15 +6181,13 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
 	struct intel_unpin_work *work;
 	struct drm_i915_gem_object *obj;
 	struct drm_pending_vblank_event *e;
-	struct timeval tnow, tvbl;
+	struct timeval tvbl;
 	unsigned long flags;
 
 	/* Ignore early vblank irqs */
 	if (intel_crtc == NULL)
 		return;
 
-	do_gettimeofday(&tnow);
-
 	spin_lock_irqsave(&dev->event_lock, flags);
 	work = intel_crtc->unpin_work;
 	if (work == NULL || !work->pending) {
@@ -6203,25 +6201,6 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
 		e = work->event;
 		e->event.sequence = drm_vblank_count_and_time(dev, intel_crtc->pipe, &tvbl);
 
-		/* Called before vblank count and timestamps have
-		 * been updated for the vblank interval of flip
-		 * completion? Need to increment vblank count and
-		 * add one videorefresh duration to returned timestamp
-		 * to account for this. We assume this happened if we
-		 * get called over 0.9 frame durations after the last
-		 * timestamped vblank.
-		 *
-		 * This calculation can not be used with vrefresh rates
-		 * below 5Hz (10Hz to be on the safe side) without
-		 * promoting to 64 integers.
-		 */
-		if (10 * (timeval_to_ns(&tnow) - timeval_to_ns(&tvbl)) >
-		    9 * crtc->framedur_ns) {
-			e->event.sequence++;
-			tvbl = ns_to_timeval(timeval_to_ns(&tvbl) +
-					     crtc->framedur_ns);
-		}
-
 		e->event.tv_sec = tvbl.tv_sec;
 		e->event.tv_usec = tvbl.tv_usec;
 
-- 
1.7.10

Re: [PATCH] drm/i915: don't frob the vblank ts in finish_page_flip

[Imre Deak]

On Tue, 2012-10-02 at 20:10 +0200, Daniel Vetter wrote:
> Now that we correctly generate it, this hack is no longer required (and
> might actually paper over a serious bug).
> 
> pageflip timestamps are sanity check in the latest version of the flip-test
> in intel-gpu-tools.
> 
> v2: Also remove the gettimeofday(&now) which is no longer used.
> Noticed by Mario Kleiner.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Tested-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_display.c |   23 +----------------------
>  1 file changed, 1 insertion(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 57c1309..67912fe 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -6181,15 +6181,13 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
>  	struct intel_unpin_work *work;
>  	struct drm_i915_gem_object *obj;
>  	struct drm_pending_vblank_event *e;
> -	struct timeval tnow, tvbl;
> +	struct timeval tvbl;
>  	unsigned long flags;
>  
>  	/* Ignore early vblank irqs */
>  	if (intel_crtc == NULL)
>  		return;
>  
> -	do_gettimeofday(&tnow);
> -
>  	spin_lock_irqsave(&dev->event_lock, flags);
>  	work = intel_crtc->unpin_work;
>  	if (work == NULL || !work->pending) {
> @@ -6203,25 +6201,6 @@ static void do_intel_finish_page_flip(struct drm_device *dev,
>  		e = work->event;
>  		e->event.sequence = drm_vblank_count_and_time(dev, intel_crtc->pipe, &tvbl);
>  
> -		/* Called before vblank count and timestamps have
> -		 * been updated for the vblank interval of flip
> -		 * completion? Need to increment vblank count and
> -		 * add one videorefresh duration to returned timestamp
> -		 * to account for this. We assume this happened if we
> -		 * get called over 0.9 frame durations after the last
> -		 * timestamped vblank.
> -		 *
> -		 * This calculation can not be used with vrefresh rates
> -		 * below 5Hz (10Hz to be on the safe side) without
> -		 * promoting to 64 integers.
> -		 */
> -		if (10 * (timeval_to_ns(&tnow) - timeval_to_ns(&tvbl)) >
> -		    9 * crtc->framedur_ns) {
> -			e->event.sequence++;
> -			tvbl = ns_to_timeval(timeval_to_ns(&tvbl) +
> -					     crtc->framedur_ns);
> -		}
> -
>  		e->event.tv_sec = tvbl.tv_sec;
>  		e->event.tv_usec = tvbl.tv_usec;
>  

Re: [PATCH] drm/i915: don't frob the vblank ts in finish_page_flip

[Daniel Vetter]

On Wed, Oct 03, 2012 at 11:09:23AM +0300, Imre Deak wrote:
> On Tue, 2012-10-02 at 20:10 +0200, Daniel Vetter wrote:
> > Now that we correctly generate it, this hack is no longer required (and
> > might actually paper over a serious bug).
> > 
> > pageflip timestamps are sanity check in the latest version of the flip-test
> > in intel-gpu-tools.
> > 
> > v2: Also remove the gettimeofday(&now) which is no longer used.
> > Noticed by Mario Kleiner.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Tested-by: Imre Deak <imre.deak@intel.com>

Ok, I've merged these two to -fixes, thanks for testing&review.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch