* [PATCH 1/2] drm/i915: call drm_handle_vblank before finish_page_flip @ 2012-10-02 15:54 Daniel Vetter 2012-10-02 15:54 ` [PATCH 2/2] drm/i915: don't frob the vblank ts in finish_page_flip Daniel Vetter 2012-10-03 8:08 ` [PATCH 1/2] drm/i915: call drm_handle_vblank before finish_page_flip Imre Deak 0 siblings, 2 replies; 5+ messages in thread From: Daniel Vetter @ 2012-10-02 15:54 UTC (permalink / raw) To: Intel Graphics Development; +Cc: Daniel Vetter, stable ... 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(-) 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) -- 1.7.10 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH 2/2] drm/i915: don't frob the vblank ts in finish_page_flip 2012-10-02 15:54 [PATCH 1/2] drm/i915: call drm_handle_vblank before finish_page_flip Daniel Vetter @ 2012-10-02 15:54 ` Daniel Vetter 2012-10-03 8:08 ` [PATCH 1/2] drm/i915: call drm_handle_vblank before finish_page_flip Imre Deak 1 sibling, 0 replies; 5+ messages in thread From: Daniel Vetter @ 2012-10-02 15:54 UTC (permalink / raw) To: Intel Graphics Development; +Cc: 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. 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 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] drm/i915: call drm_handle_vblank before finish_page_flip 2012-10-02 15:54 [PATCH 1/2] drm/i915: call drm_handle_vblank before finish_page_flip Daniel Vetter 2012-10-02 15:54 ` [PATCH 2/2] drm/i915: don't frob the vblank ts in finish_page_flip Daniel Vetter @ 2012-10-03 8:08 ` Imre Deak 2012-10-03 9:05 ` Chris Wilson 1 sibling, 1 reply; 5+ messages in thread From: Imre Deak @ 2012-10-03 8:08 UTC (permalink / raw) To: Daniel Vetter; +Cc: Intel Graphics Development, stable 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) ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] drm/i915: call drm_handle_vblank before finish_page_flip 2012-10-03 8:08 ` [PATCH 1/2] drm/i915: call drm_handle_vblank before finish_page_flip Imre Deak @ 2012-10-03 9:05 ` Chris Wilson 2012-10-03 11:50 ` Daniel Vetter 0 siblings, 1 reply; 5+ messages in thread From: Chris Wilson @ 2012-10-03 9:05 UTC (permalink / raw) To: Imre Deak, Daniel Vetter; +Cc: Intel Graphics Development, stable 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 -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH 1/2] drm/i915: call drm_handle_vblank before finish_page_flip 2012-10-03 9:05 ` Chris Wilson @ 2012-10-03 11:50 ` Daniel Vetter 0 siblings, 0 replies; 5+ messages in thread From: Daniel Vetter @ 2012-10-03 11:50 UTC (permalink / raw) To: Chris Wilson; +Cc: Intel Graphics Development, stable 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 -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2012-10-03 11:50 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-10-02 15:54 [PATCH 1/2] drm/i915: call drm_handle_vblank before finish_page_flip Daniel Vetter 2012-10-02 15:54 ` [PATCH 2/2] drm/i915: don't frob the vblank ts in finish_page_flip Daniel Vetter 2012-10-03 8:08 ` [PATCH 1/2] drm/i915: call drm_handle_vblank before finish_page_flip Imre Deak 2012-10-03 9:05 ` Chris Wilson 2012-10-03 11:50 ` Daniel Vetter
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.