| Message ID | 1349193276-13908-1-git-send-email-daniel.vetter@ffwll.ch (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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@vger.kernel.org > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> Tested-by: Imre Deak <imre.deak@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)
On Wed, 03 Oct 2012 11:08:40 +0300, Imre Deak <imre.deak@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@vger.kernel.org > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> > > Tested-by: Imre Deak <imre.deak@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@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@ffwll.ch>, Intel Graphics Development <intel-gfx@lists.freedesktop.org> Cc: Imre Deak <imre.deak@intel.com>, Daniel Vetter <daniel.vetter@ffwll.ch> In-Reply-To: <1349193276-13908-2-git-send-email-daniel.vetter@ffwll.ch> References: <1349193276-13908-1-git-send-email-daniel.vetter@ffwll.ch> <1349193276-13908-2-git-send-email-daniel.vetter@ffwll.ch> On Tue, 2 Oct 2012 17:54:36 +0200, Daniel Vetter <daniel.vetter@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@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@chris-wilson.co.uk> Subject: Re: [PATCH 1/2] drm/i915: call drm_handle_vblank before finish_page_flip To: Daniel Vetter <daniel.vetter@ffwll.ch>, Intel Graphics Development <intel-gfx@lists.freedesktop.org> Cc: Imre Deak <imre.deak@intel.com>, Daniel Vetter <daniel.vetter@ffwll.ch>, stable@vger.kernel.org In-Reply-To: <1349193276-13908-1-git-send-email-daniel.vetter@ffwll.ch> References: <1349193276-13908-1-git-send-email-daniel.vetter@ffwll.ch> On Tue, 2 Oct 2012 17:54:35 +0200, Daniel Vetter <daniel.vetter@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
On Wed, Oct 3, 2012 at 11:05 AM, Chris Wilson <chris@chris-wilson.co.uk> wrote: > > 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! I share your concern, but I think with the hack removed we should notice this rather quickly with the new flip_test ... Eventually I want to rid us of the vblank irq (at least for unscheduled pageflips) completely anyway. -Daniel
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)
... 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@vger.kernel.org Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> --- drivers/gpu/drm/i915/i915_irq.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-)