On Sat, 2008-07-19 at 14:01 +0200, 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...
>
>
> > 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;
> ^^^^^^^^^^^^^^^^^^^
>
> This means the interrupt never gets disabled, doesn't it?Yes, this was not correct, it prevents any of the pre/post modeset code from running. I've fixed this on the bsd side. I'll try and get the linux side fixed up later and send an updated patch. > 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. I've changed this to vblank_disable_allowed = 0 as an additional safeguard against disabling interupts across modeset. It shouldn't be needed though, as the vblank_get will bump the refcount and prevent the callout from disabling interupts. One other issue that I spotted while testing is that we initialize vblank_disable_allowed to 0. The only place that it ever becomes true is in post modeset. So, for me... vblanks were never getting disabled. For now, I'm initializing it to 1 and it will only toggle over modeset. I'm still (or again) sometimes getting the error about get_vblank_count being called on disabled pipe that I need to try and chase down. I also realized that I was inadvertently setting the timeout too long in the bsd code, which I've fixed... Otherwise, it seems to be working well. robert.
signature.asc
Description: This is a digitally signed message part
------------------------------------------------------------------------- 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
