On Saturday, January 31, 2009 4:49 am Michel Dänzer wrote:
> On Fri, 2009-01-30 at 10:43 -0800, Jesse Barnes wrote:
> > On Friday, January 30, 2009 9:26 am Jesse Barnes wrote:
> > > But maybe there's an even simpler way to handle this (the wait on
> > > disabled pipe/irq problem). The DRM_WAIT_ON already checks if irqs are
> > > disabled before waiting (or at least it's supposed to), and the
> > > vblank_get call will check to make sure the pipe is enabled before
> > > waiting... maybe one of those checks is failing in this case, or we're
> > > not waking clients up at IRQ disable time like we should. I'll do some
> > > more testing and see.
> >
> > Since in the VT switch case at least, userspace is calling in with a
> > stale vblank value (one much smaller than the current count), another
> > option would be something like the below. Needs to be a bit smarter to
> > deal with counter wraparound though,
>
> I don't think an additional test should be needed; the
>
> (drm_vblank_count(dev, crtc) - vblwait->request.sequence) <= (1 << 23)
>
> test is supposed to catch insane conditions, if it isn't appropriate,
> just fix it.
I don't want to add another test, just to replace the existing one. Below is
what I'd like to push, since it matches other wraparound handling in the
kernel. How did you settle on 1<<23 originally?
> > but OTOH wraparound wouldn't happen as often if our
> > drm_update_vblank_count() wasn't so eager to add ~max_frame_count to
> > the counter at enable time.
>
> Huh? If the counter jumps, that's a bug (most likely due to the display
> driver not calling the pre/post modesetting ioctl around the CRTC frame
> counter resetting?) that needs to be fixed. I'm not seeing any such
> issues with radeon anymore though; maybe you;d like to take a look at
> how the X radeon driver calls the ioctl in Leave/EnterVT.
Lost frame accounting is done in drm_update_vblank_count, which uses
last_vblank[crtc], which is updated by the expiration timer function (should
have renamed this stuff to "frames" when we moved back to the interrupt only
scheme).
If it gets called before the timer has expired, but after a frame counter
reset (e.g. a quick VT switch), you'll see cur_vblank < last_vblank on that
crtc, and add max_vblank_count. It's mostly harmless, we probably see it on
Intel because we do an IRQ disable across VT switch, which would trigger the
update_vblank_count call at the first post-VT switch drm_vblank_get. Should
be easy to fix up though, we can just capture last_vblank_count at IRQ
disable time too.
--
Jesse Barnes, Intel Open Source Technology Center
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 69aa0ab..c41cba4 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -341,7 +341,7 @@ int drm_control(struct drm_device *dev, void *data,
* vblank events since the system was booted, including lost events due to
* modesetting activity.
*/
-u32 drm_vblank_count(struct drm_device *dev, int crtc)
+unsigned int drm_vblank_count(struct drm_device *dev, int crtc)
{
return atomic_read(&dev->_vblank_count[crtc]);
}
@@ -522,6 +522,11 @@ out:
return ret;
}
+#define frame_after_eq(a,b) \
+ (typecheck(unsigned int, a) && \
+ typecheck(unsigned int, b) && \
+ ((int)(a) - (int)(b) >= 0))
+
/**
* Wait for VBLANK.
*
@@ -589,10 +594,12 @@ int drm_wait_vblank(struct drm_device *dev, void *data,
DRM_DEBUG("waiting on vblank count %d, crtc %d\n",
vblwait->request.sequence, crtc);
dev->last_vblank_wait[crtc] = vblwait->request.sequence;
+
+ /* Wait for the sequence number to pass or IRQs to get disabled */
DRM_WAIT_ON(ret, dev->vbl_queue[crtc], 3 * DRM_HZ,
- (((drm_vblank_count(dev, crtc) -
- vblwait->request.sequence) <= (1 << 23)) ||
- !dev->irq_enabled));
+ frame_after_eq(drm_vblank_count(dev, crtc),
+ vblwait->request.sequence) ||
+ !dev->irq_enabled);
if (ret != -EINTR) {
struct timeval now;
------------------------------------------------------------------------------
This SF.net email is sponsored by:
SourcForge Community
SourceForge wants to tell your story.
http://p.sf.net/sfu/sf-spreadtheword
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
