On Saturday, July 19, 2008 5:01 am Michel Dänzer wrote:
> On Fri, 2008-07-18 at 14:41 -0700, Jesse Barnes wrote:
> > I failed to get the old update_vblank approach working, messing with the
> > counter is different under the new interrupt counter scheme.
>
> Okay, I don't really understand why it's so hard, but maybe I'm missing
> something...
I don't think it's hard, just impossible unless I re-add the (my) crappy code
that made us need the compensation in the first place (i.e. the spurious
wraparounds caused by updating the vblank count after a mode set). Now that
we're using the counter for everything, we may as well keep the code
understandable too, imo.
> > If you feel strongly that we should stick with drm_update_vblank_count
> > even though it's changed a lot (therefore invaliding earlier tests), I
> > can change over to using that.
>
> Let's give your approach a spin. Note that due to the not fully
> deterministic nature of the issues caused by the hardware frame counter
> getting reset, it'll take at least a couple of days for me to become
> confident it's solid.
>
> > And of course I'm interested to hear about any bugs you discover in
> > testing.
>
> I noticed one on a quick look over:
> > @@ -455,47 +455,52 @@ EXPORT_SYMBOL(drm_vblank_put);
> > *
> > * Generally the counter will reset across mode sets. If interrupts are
> > * enabled around this call, we don't have to do anything since the
> > counter - * will have already been incremented. If interrupts are off
> > though, we need - * to make sure we account for the lost events at
> > %_DRM_POST_MODESET time. + * will have already been incremented.
> > */
> > int drm_modeset_ctl(struct drm_device *dev, void *data,
> > struct drm_file *file_priv)
> > {
> > struct drm_modeset_ctl *modeset = data;
> > unsigned long irqflags;
> > - u32 new, diff;
> > int crtc, ret = 0;
> >
> > + /* If drm_vblank_init() hasn't been called yet, just no-op */
> > + if (!dev->num_crtcs)
> > + goto out;
> > +
> > crtc = modeset->crtc;
> > if (crtc >= dev->num_crtcs) {
> > ret = -EINVAL;
> > goto out;
> > }
> >
> > + return 0;
>
> ^^^^^^^^^^^^^^^^^^^
Oh, heh that was some debugging I left in.
> This means the interrupt never gets disabled, doesn't it?
>
> On another minor note, the
>
> dev->vblank_disable_allowed = 1;
>
> assignment in the _DRM_PRE_MODESET case is superfluous, as the interrupt
> won't actually get disabled until the _DRM_POST_MODESET case is hit as
> well.
Yeah that's true, I can move it to just post modeset to avoid confusion.
Jesse
-------------------------------------------------------------------------
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
