* [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.