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

Reply via email to