On Friday, February 13, 2009 2:33 am Michel Dänzer wrote: > On Thu, 2009-02-12 at 09:15 -0800, Jesse Barnes wrote: > > It does, but take a look at that code again. If interrupts are disabled > > by the timer, we'll capture the frame count. If, sometime later, they're > > re-enabled, we'll end up in drm_update_vblank_count. And if, between > > those two points, we've had a DPMS event, the frame counter will have > > been reset and will now be a lower value, so our drm_update_vblank_count > > will detect a wraparound. I think any hardware that resets its frame > > count will be susceptible to this problem unless we make wraparounds > > harmless. > > The modeset ioctl makes wraparounds harmless if used correctly.
I don't think it does. And you may not be seeing this problem on radeon because your max_vblank_count is only 0x1fffff, which wouldn't trigger this behavior. Recall our last discussion where I outlined the cases we'd have to deal with in the modeset ioctl if we didn't use get/put to just keep interrupts on around the calls: + /* + * Cases to handle here: + * 1) interrupts are off across both calls + * be sure to add any lost frames + * 2) interrupts are on across both calls + * nothing to do, count will be updated by interupt handlers + * 3) interrupts go from on->off between calls + * add the post mode set frame count + * note that if interrupts go off before the counter resets + * we'll lose some frames. + * and if interrupts go off after it resets we'll end up + * double counting some events since the interrupt handlers will + * have added some of the new frame count value too + * 4) interrupts going from off->on between calls + * this really shouldn't happen much since clients generally don't + * start while modesetting is happening + * we need to account for lost frames in this case too, since + * there may be quite a few of them + */ I couldn't get all of them working right, but maybe you can. If so, I'd be happy to see that patch. > > Can you think of a case where those frames would matter? > > E.g. the GLX_OML_sync_control spec says 'The graphics Media Stream > Counter (or graphics MSC) is a counter that is unique to the graphics > subsystem and increments for each vertical retrace that occurs.' > Vertical retraces that occur while nobody's listening may not matter in > simple cases, but that doesn't mean they're never relevant. But that doesn't say anything about whether the frame count must stay accurate while apps aren't running (i.e. doing anything with video/GL). That's why I asked for an example. If we assume it's important in some cases, we can still remove the update_vblank_counter code and just provide a knob to prevent interrupts from ever being disabled. > In general, I'm a little worried about the trend to just remove stuff > with problems (software engineering is hard, let's go shopping?), in > particular in a case like this where it's working fine for other > drivers. The other side of this coin is over-engineering things for theoretical problems that don't actually occur in real life. We want to avoid that too. -- Jesse Barnes, Intel Open Source Technology Center ------------------------------------------------------------------------------ Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise -Strategies to boost innovation and cut costs with open source participation -Receive a $600 discount off the registration fee with the source code: SFAD http://p.sf.net/sfu/XcvMzF8H -- _______________________________________________ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel
