On Fri, 2008-07-18 at 09:41 -0700, Jesse Barnes wrote:
> On Friday, July 18, 2008 1:12 am Michel Dänzer wrote:
> > 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:
> > > > > @@ -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);
> > ...
>
> That's one way of addressing the interrupts-are-off counter update, but
> really
> I had intended to get rid of drm_update_vblank_count() as part of the API
> since it led to the spurious wraparound problem in the first place, and
> isolate all the update logic to drm_vblank_get(). My thinking was that
> get/put around any API usage would be simpler, but maybe keeping the
> update_vblank_count() call in place is better, I dunno.
While it may be possible to fix the problems of your change in the long
run, I think it's a pretty bad idea to make such fundamental changes to
a basically mature implementation this shortly before submitting the
functionality to the kernel. Maybe we can try your approach again once
it's made it into a kernel release.
--
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
