dri-devel Archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm: Don't zero vblank timestamps from the irq handler
@ 2015-09-30 16:21 ville.syrjala
  2015-10-01  7:27 ` Thierry Reding
  0 siblings, 1 reply; 3+ messages in thread
From: ville.syrjala @ 2015-09-30 16:21 UTC (permalink / raw
  To: dri-devel

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

If we couldn't get a high precisions vblank timestamp, we currently
store a zeroed timestamp instead and assume the next vblank irq to
get us something better. This makes sense when trying to update the
timestamp from eg. vblank enable. But if we do this from the vblank
irq we will never get a vblank timestamp unless we high precision
timestamps are available and succeeded. This break weston for instance
on drivers lacking high precision timestamps.

To fix this, zero the timestamp only when not called from vbl irq.
When called from the irq, we still want the timestamp, even if not
perfect.

This fixes a regression from
4dfd64862ff852df drm: Use vblank timestamps to guesstimate how many vblanks were missed

Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Reported-by: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_irq.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index f24c57c..1b9faa0 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -232,10 +232,11 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
 
 	/*
 	 * Only reinitialize corresponding vblank timestamp if high-precision query
-	 * available and didn't fail. Otherwise reinitialize delayed at next vblank
-	 * interrupt and assign 0 for now, to mark the vblanktimestamp as invalid.
+	 * available and didn't fail, or we were called from the vblank interrupt.
+	 * Otherwise reinitialize delayed at next vblank interrupt and assign 0
+	 * for now, to mark the vblanktimestamp as invalid.
 	 */
-	if (!rc)
+	if (!rc && (flags & DRM_CALLED_FROM_VBLIRQ) == 0)
 		t_vblank = (struct timeval) {0, 0};
 
 	store_vblank(dev, pipe, diff, &t_vblank, cur_vblank);
-- 
2.4.6

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm: Don't zero vblank timestamps from the irq handler
  2015-09-30 16:21 [PATCH] drm: Don't zero vblank timestamps from the irq handler ville.syrjala
@ 2015-10-01  7:27 ` Thierry Reding
  2015-10-01  7:50   ` Daniel Vetter
  0 siblings, 1 reply; 3+ messages in thread
From: Thierry Reding @ 2015-10-01  7:27 UTC (permalink / raw
  To: ville.syrjala; +Cc: dri-devel


[-- Attachment #1.1: Type: text/plain, Size: 1489 bytes --]

On Wed, Sep 30, 2015 at 07:21:34PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> If we couldn't get a high precisions vblank timestamp, we currently
> store a zeroed timestamp instead and assume the next vblank irq to
> get us something better. This makes sense when trying to update the
> timestamp from eg. vblank enable. But if we do this from the vblank
> irq we will never get a vblank timestamp unless we high precision
> timestamps are available and succeeded. This break weston for instance
> on drivers lacking high precision timestamps.
> 
> To fix this, zero the timestamp only when not called from vbl irq.
> When called from the irq, we still want the timestamp, even if not
> perfect.
> 
> This fixes a regression from
> 4dfd64862ff852df drm: Use vblank timestamps to guesstimate how many vblanks were missed
> 
> Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Reported-by: Thierry Reding <thierry.reding@gmail.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/drm_irq.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)

Applied on top of next-20151001 and the weston problem I was seeing is
gone, so:

Tested-by: Thierry Reding <treding@nvidia.com>

I think it might be worth considering squashing this into the offending
commit to avoid breaking bisectibility.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm: Don't zero vblank timestamps from the irq handler
  2015-10-01  7:27 ` Thierry Reding
@ 2015-10-01  7:50   ` Daniel Vetter
  0 siblings, 0 replies; 3+ messages in thread
From: Daniel Vetter @ 2015-10-01  7:50 UTC (permalink / raw
  To: Thierry Reding; +Cc: dri-devel

On Thu, Oct 01, 2015 at 09:27:20AM +0200, Thierry Reding wrote:
> On Wed, Sep 30, 2015 at 07:21:34PM +0300, ville.syrjala@linux.intel.com wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > If we couldn't get a high precisions vblank timestamp, we currently
> > store a zeroed timestamp instead and assume the next vblank irq to
> > get us something better. This makes sense when trying to update the
> > timestamp from eg. vblank enable. But if we do this from the vblank
> > irq we will never get a vblank timestamp unless we high precision
> > timestamps are available and succeeded. This break weston for instance
> > on drivers lacking high precision timestamps.
> > 
> > To fix this, zero the timestamp only when not called from vbl irq.
> > When called from the irq, we still want the timestamp, even if not
> > perfect.
> > 
> > This fixes a regression from
> > 4dfd64862ff852df drm: Use vblank timestamps to guesstimate how many vblanks were missed
> > 
> > Cc: Mario Kleiner <mario.kleiner.de@gmail.com>
> > Cc: Thierry Reding <thierry.reding@gmail.com>
> > Reported-by: Thierry Reding <thierry.reding@gmail.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/drm_irq.c | 7 ++++---
> >  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> Applied on top of next-20151001 and the weston problem I was seeing is
> gone, so:
> 
> Tested-by: Thierry Reding <treding@nvidia.com>
> 
> I think it might be worth considering squashing this into the offending
> commit to avoid breaking bisectibility.

Can't squash any more since history is frozen in drm-next. Applied to
drm-misc, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2015-10-01  7:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-30 16:21 [PATCH] drm: Don't zero vblank timestamps from the irq handler ville.syrjala
2015-10-01  7:27 ` Thierry Reding
2015-10-01  7:50   ` Daniel Vetter

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).