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
-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/
--
_______________________________________________
Dri-devel mailing list
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
