On Wed, 03 Oct 2012 11:08:40 +0300, Imre Deak <[email protected]> wrote: > On Tue, 2012-10-02 at 17:54 +0200, Daniel Vetter wrote: > > ... since finish_page_flip needs the vblank timestamp generated > > in drm_handle_vblank. Somehow all the gmch platforms get it right, > > but all the pch platform irq handlers get is wrong. Hooray for copy& > > pasting! > > > > Currently this gets papered over by a gross hack in finish_page_flip. > > A second patch will remove that. > > > > Note that without this, the new timestamp sanity checks in flip_test > > occasionally get tripped up, hence the cc: stable tag. > > > > Cc: [email protected] > > Signed-off-by: Daniel Vetter <[email protected]> > > Tested-by: Imre Deak <[email protected]> > > > --- > > drivers/gpu/drm/i915/i915_irq.c | 16 ++++++++-------- > > 1 file changed, 8 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_irq.c > > b/drivers/gpu/drm/i915/i915_irq.c > > index d7f0066..1fc2489 100644 > > --- a/drivers/gpu/drm/i915/i915_irq.c > > +++ b/drivers/gpu/drm/i915/i915_irq.c > > @@ -698,12 +698,12 @@ static irqreturn_t ivybridge_irq_handler(DRM_IRQ_ARGS) > > intel_opregion_gse_intr(dev); > > > > for (i = 0; i < 3; i++) { > > + if (de_iir & (DE_PIPEA_VBLANK_IVB << (5 * i))) > > + drm_handle_vblank(dev, i); > > if (de_iir & (DE_PLANEA_FLIP_DONE_IVB << (5 * i))) { > > intel_prepare_page_flip(dev, i); > > intel_finish_page_flip_plane(dev, i); > > } > > - if (de_iir & (DE_PIPEA_VBLANK_IVB << (5 * i))) > > - drm_handle_vblank(dev, i); > > } > > > > /* check event from PCH */ > > @@ -785,6 +785,12 @@ static irqreturn_t ironlake_irq_handler(DRM_IRQ_ARGS) > > if (de_iir & DE_GSE) > > intel_opregion_gse_intr(dev); > > > > + if (de_iir & DE_PIPEA_VBLANK) > > + drm_handle_vblank(dev, 0); > > + > > + if (de_iir & DE_PIPEB_VBLANK) > > + drm_handle_vblank(dev, 1); > > + > > if (de_iir & DE_PLANEA_FLIP_DONE) { > > intel_prepare_page_flip(dev, 0); > > intel_finish_page_flip_plane(dev, 0); > > @@ -795,12 +801,6 @@ static irqreturn_t ironlake_irq_handler(DRM_IRQ_ARGS) > > intel_finish_page_flip_plane(dev, 1); > > } > > > > - if (de_iir & DE_PIPEA_VBLANK) > > - drm_handle_vblank(dev, 0); > > - > > - if (de_iir & DE_PIPEB_VBLANK) > > - drm_handle_vblank(dev, 1); > > - > > /* check event from PCH */ > > if (de_iir & DE_PCH_EVENT) { > > if (pch_iir & hotplug_mask) > > From: Chris Wilson <[email protected]> Subject: Re: [PATCH 2/2] drm/i915: don't frob the vblank ts in finish_page_flip To: Daniel Vetter <[email protected]>, Intel Graphics Development <[email protected]> Cc: Imre Deak <[email protected]>, Daniel Vetter <[email protected]> In-Reply-To: <[email protected]> References: <[email protected]> <[email protected]>
On Tue, 2 Oct 2012 17:54:36 +0200, Daniel Vetter <[email protected]> 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. > > Signed-off-by: Daniel Vetter <[email protected]> > --- > drivers/gpu/drm/i915/intel_display.c | 19 ------------------- > 1 file changed, 19 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index 57c1309..87f825c 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6203,25 +6203,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 > From: Chris Wilson <[email protected]> Subject: Re: [PATCH 1/2] drm/i915: call drm_handle_vblank before finish_page_flip To: Daniel Vetter <[email protected]>, Intel Graphics Development <[email protected]> Cc: Imre Deak <[email protected]>, Daniel Vetter <[email protected]>, [email protected] In-Reply-To: <[email protected]> References: <[email protected]> On Tue, 2 Oct 2012 17:54:35 +0200, Daniel Vetter <[email protected]> wrote: > ... since finish_page_flip needs the vblank timestamp generated > in drm_handle_vblank. Somehow all the gmch platforms get it right, > but all the pch platform irq handlers get is wrong. Hooray for copy& > pasting! > > Currently this gets papered over by a gross hack in finish_page_flip. > A second patch will remove that. > > Note that without this, the new timestamp sanity checks in flip_test > occasionally get tripped up, hence the cc: stable tag. Just a touch concerned because I recall no details specifying that the vblank interrupt will be delivered before or at the same time as the pageflip-complete interrupt. Not that it would be possible for the pageflip to complete without a vblank! -Chris -- Chris Wilson, Intel Open Source Technology Centre -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
