On Thu, 2008-07-17 at 09:32 -0700, Jesse Barnes wrote:
> On Wednesday, July 16, 2008 11:51 pm Michel Dänzer wrote:
> > On Wed, 2008-07-16 at 16:01 -0700, Jesse Barnes wrote:
> > > @@ -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.
>
> Oh right, I forgot about the wraparound logic.
It's not about that; drm_update_vblank_count() just needs to increase
dev->_vblank_count[crtc] by the number of frames since the interrupt was
disabled, not since the last time it was enabled.
> I also got to thinking about the modeset ioctl. With a few changes I think
> we
> can avoid the huge jumps we used to see (both forward & backward).
I thought we'd been over that before on IRC... Please revert the
drm_modeset_ctl() changes, what was there before should DTRT under any
circumstances and certainly does IME. If you've found specific cases
where it doesn't, please provide more information about those. (Thought
experiments are fine :)
> > > @@ -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.
>
> Yeah, I think just a get_vblank_counter() in vblank_disable_fn() is
> sufficient, since we'll hit the wraparound logic again when we re-enable.
> Look ok?
I'm afraid not. Again, the wraparound logic is irrelevant for this, it's
in drm_update_vblank_count() after all...
What's wrong with
if (!dev->vblank_enabled[crtc])
drm_update_vblank_count(dev, crtc);
seq = drm_vblank_count(dev, crtc);
...
?
--
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/dri-devel
