On Wed, 2007-06-13 at 14:26 -0700, Jesse Barnes wrote: > On Wednesday, June 13, 2007 12:24:05 Michel Dänzer wrote: > > On Tue, 2007-06-12 at 13:37 -0700, Jesse Barnes wrote: > > > > +typedef struct drm_modeset_ctl { > > > + drm_modeset_ctl_cmd_t cmd; > > > + unsigned long arg; > > > +} drm_modeset_ctl_t; > > > > unsigned long is bad for 32 bit userland on a 64 bit kernel. > > Yeah, it should probably be u64, or a real per-subioctl command union.
You have to be extra careful though because different alignment rules can screw you up even if the individual members have the same size. See e.g. the radeon setparam ioctl that recently needed to grow a 32 bit compatibility handler for this reason. On Wed, 2007-06-13 at 14:33 -0700, Jesse Barnes wrote: > > > > + if (temp & VSYNC_PIPEA_FLAG) > > > > + atomic_add(i915_get_vblank_counter(dev, 0), > > > > + &dev->vblank_count[0]); > > > > + if (temp & VSYNC_PIPEB_FLAG) > > > > + atomic_add(i915_get_vblank_counter(dev, 1), > > > > + &dev->vblank_count[1]); > > > > > > I think atomic_add is wrong here. Actually, AFAICT dev->vblank_count is currently completely mixed up between reference counting for enabling the interrupt and the driver/API counters. > > Hm yeah it should be just atomic_set(), duh. > > On second thought, maybe I'll just go back to atomic_inc, otherwise I'll > have to figure out the difference between the last count and the new > count, then atomically add that to the current count, to deal with > missed interrupts. > > Then again, maybe I could unify it with the counter update code that I > pull out of drm_vblank_get. I'll see about that. Yeah, I think there should be a dedicated raw driver counter, and the API counter should probably only be available via an accessor function which adds dev->vblank_offset. Then the driver interrupt handler could just atomic_set() the raw counter, or maybe the drm_vblank_handler() function I suggested in another post could just take the raw counter value as an argument. -- Earthling Michel Dänzer | http://tungstengraphics.com Libre software enthusiast | Debian, X and DRI developer ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ -- _______________________________________________ Dri-devel mailing list Dri-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/dri-devel