On Monday, June 11, 2007 11:36:10 Keith Packard wrote:
> On Mon, 2007-06-11 at 10:59 -0700, Jesse Barnes wrote:
> > The patch doesn't yet deal with two obvious cases (and probably more
> > that I'm missing, it's untested yet):
> > - the hardware counter resets on mode switch, we need to rebase
> > the appropriate last_counter at that point so it's not treated as
> > a counter wrap
>
> I think this should be handled in the core by having an offset that
> relates the hardware counter to the software visible value. Mode setting
> would then 'rebase' the offset to make the user-visible value change
> smoothly:
>
> pre_mode_set
> old = current raw hardware counter
> post_mode_set
> new = current raw hardware counter
> offset += old - new
>
> now create a 'cooked' hardware counter:
>
> cooked hardware counter = offset + raw hardware counter
Yep, sounds reasonable.
>
> > - a client interested in signals but also blocked on a vblank event
> > may cause vblanks to be disabled if it received signal at the wrong
> > time
>
> the core should track a per-crtc enable/disable count and call the
> driver when it transitions through zero.
Yeah, I think there's even a 'vbl_pending' counter I can use for this, though
I'll have to make it atomic.
> > + unsigned long last_vblank_count[2];
> > + atomic_t vblank_count[2];
>
> For a first implementation, I suggest that the driver never store this
> value and just read from the registers every time it fetches the frame
> counter. You could 'optimize' reading when the interrupt was running by
> caching the last value, but this will be race-prone.
I'm using last_vblank_count to deal with wrapping (as you see below), and
vblank_count is the total vblank value (including all wraps).
>
> > + /*
> > + * Now update the appropriate counter.
> > + * Assume we haven't missed a full 24 bits of vblank
> > + * events, or that it won't matter if they're not accounted
> > + * for when we adjust for wrapping.
> > + * FIXME: if count was zero'd due to modeset, need to rebase
> > + */
> > + spin_lock_irqsave(&dev->vbl_lock, irqflags);
> > + if (count < dev_priv->last_vblank_count[crtc]) {
> > + diff = 0xffffff - dev_priv->last_vblank_count[crtc];
> > + diff += count;
> > + } else {
> > + diff = count - dev_priv->last_vblank_count[crtc];
> > + }
> > + dev_priv->last_vblank_count[crtc] = count;
> > + spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
> > + atomic_add(diff, &dev_priv->vblank_count[crtc]);
> > + return atomic_read(&dev_priv->vblank_count[crtc]);
>
> ick. just read the registers and return the value here. We should place
> wrap-detection in the core code by reporting the range of the register
> values; with the offset suggested above, that would result in a single
> addition to convert from raw to cooked frame counts.
Hm, yeah I guess it might be nicer in the core. I was thinking that this
would give the driver maximum flexibility and avoid the need for a 'max
value' callback or field, but that may be a net win.
Thanks,
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
Dri-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/dri-devel
