On Thursday, June 14, 2007 12:40:46 Michel Dänzer wrote:
> 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.
Oops, did I mix them up in the last commit? I'll double check and fix,
maybe I should rename the reference count variable to indicate that
it's used for reference counting. :)
> 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.
Hm, but that might cause trouble with wraparound. I was thinking of
just calling drm_update_counter() from the interrupt handler to deal
with the update, I'll have to extract it from drm_vblank_get() first
though... at least I think that will work (though it will involve more
PIOs than just an atomic op in the interrupt handler).
Jesse
-------------------------------------------------------------------------
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/dri-devel
