From: Michel D. <mi...@tu...> - 2008-07-17 06:51:34
|
On Wed, 2008-07-16 at 16:01 -0700, Jesse Barnes wrote: > Here's a patch that implements what we've been talking about: > - uses the atomic counter while interrupts are on, which means the > get_vblank_counter callback is only used when going from off->on to > account for missed events > - won't disable interrupts until the modeset ioctl is called, to preserve > old behavior with old clients Sounds good, thanks for tackling this. > I also found a bug in the i915 code while I was at it. I was trying to figure > out why when my test client exited I no longer saw interrupts, and then I saw > the code in i915_driver_irq_handler that disabled pipe b's vblank events if > the vblank_pipe bit for pipe b wasn't set. Since the driver is managing > interrupts now, I just made the pipe get/set routines into stubs and set the > appropriate bits at irq install time instead. This make the 2D driver > impotent when it comes to enabling/disabling interrupts, but that should be > ok (though it means more interrupts with an old 2D and new DRM). Yeah, the old scheme was quirky. > diff --git a/linux-core/drm_irq.c b/linux-core/drm_irq.c > index abedbe7..55a2544 100644 > --- a/linux-core/drm_irq.c > +++ b/linux-core/drm_irq.c > @@ -71,16 +71,29 @@ int drm_irq_by_busid(struct drm_device *dev, void *data, > return 0; > } > > +/* > + * At load time, disabling the vblank interrupt won't be allowed since > + * old clients may not call the modeset ioctl and therefore misbehave. > + * Once the modeset ioctl *has* been called though, we can safely > + * disable them when unused. > + */ > +static int vblank_disable_allowed; This should probably be tracked per device instead of globally. > @@ -408,8 +426,10 @@ int drm_vblank_get(struct drm_device *dev, int crtc) > ret = dev->driver->enable_vblank(dev, crtc); > if (ret) > atomic_dec(&dev->vblank_refcount[crtc]); > - else > + else { > dev->vblank_enabled[crtc] = 1; > + drm_update_vblank_count(dev, crtc); > + } I think this is missing a corresponding dev->last_vblank[crtc] = dev->driver->get_vblank_counter(dev, crtc); when the interrupt gets disabled, to make sure this does the right thing. > @@ -528,7 +550,6 @@ int drm_wait_vblank(struct drm_device *dev, void *data, > if (crtc >= dev->num_crtcs) > return -EINVAL; > > - drm_update_vblank_count(dev, crtc); > seq = drm_vblank_count(dev, crtc); > > switch (vblwait->request.type & _DRM_VBLANK_TYPES_MASK) { I don't think this can be removed altogether, or seq will be stale if the interrupt is disabled when drm_wait_vblank() is called. So I guess call drm_update_vblank_count() when the interrupt is disabled, or just bail inside drm_update_vblank_count() when it is enabled. -- Earthling Michel Dänzer | http://tungstengraphics.com Libre software enthusiast | Debian, X and DRI developer |