On Wed, 03 Oct 2012 11:08:40 +0300, Imre Deak <imre.deak at intel.com> 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: stable at vger.kernel.org
> > Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>
> Tested-by: Imre Deak <imre.deak at intel.com>
>
> > ---
> > 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 <chris at chris-wilson.co.uk>
Subject: Re: [PATCH 2/2] drm/i915: don't frob the vblank ts in finish_page_flip
To: Daniel Vetter <daniel.vetter at ffwll.ch>, Intel Graphics Development <intel-gfx at lists.freedesktop.org>
Cc: Imre Deak <imre.deak at intel.com>, Daniel Vetter <daniel.vetter at ffwll.ch>
In-Reply-To: <1349193276-13908-2-git-send-email-daniel.vetter at ffwll.ch>
References: <1349193276-13908-1-git-send-email-daniel.vetter at ffwll.ch> <1349193276-13908-2-git-send-email-daniel.vetter at ffwll.ch>
On Tue, 2 Oct 2012 17:54:36 +0200, Daniel Vetter <daniel.vetter at ffwll.ch> 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 <daniel.vetter at ffwll.ch>
> ---
> 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 <chris at chris-wilson.co.uk>
Subject: Re: [PATCH 1/2] drm/i915: call drm_handle_vblank before finish_page_flip
To: Daniel Vetter <daniel.vetter at ffwll.ch>, Intel Graphics Development <intel-gfx at lists.freedesktop.org>
Cc: Imre Deak <imre.deak at intel.com>, Daniel Vetter <daniel.vetter at ffwll.ch>, stable at vger.kernel.org
In-Reply-To: <1349193276-13908-1-git-send-email-daniel.vetter at ffwll.ch>
References: <1349193276-13908-1-git-send-email-daniel.vetter at ffwll.ch>
On Tue, 2 Oct 2012 17:54:35 +0200, Daniel Vetter <daniel.vetter at ffwll.ch> 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
