On Mon, 2007-06-11 at 15:20 -0700, Jesse Barnes wrote:
> On Monday, June 11, 2007 11:36:10 Keith Packard wrote:
> > 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.
>
> Ok, here's an updated version:
> - move wraparound logic to the core
> - add pre/post modeset ioctls (per-driver right now, making them core would
> mean lots more DDX changes I think),
Shouldn't really matter, DDX drivers can call driver independent ioctls.
> hope I got this right
> - add vblank_get/put calls so interrupts are enabled at the right time
>
> I haven't implemented Ville's suggestion of adding a short timer before
> disabling interrupts again, but it should be easy now that the get/put
> routines are in place and we think it's worth it (might make vblank
> calls a little cheaper, but it would probably be hard to detect).
Yeah, I'm doubtful. Ville, can you explain some use cases you're
thinking of?
> Any more thoughts? If it looks good, I'll go ahead and test it, clean it up
> a bit,
> and convert my old radeon patch over to this new scheme (other drivers will
> need
> work too though).
The general direction looks good.
> @@ -265,13 +301,13 @@ int drm_wait_vblank(DRM_IOCTL_ARGS)
> }
>
> flags = vblwait.request.type & _DRM_VBLANK_FLAGS_MASK;
> + crtc = flags & _DRM_VBLANK_SECONDARY ? 1 : 0;
>
> if (!drm_core_check_feature(dev, (flags & _DRM_VBLANK_SECONDARY) ?
> DRIVER_IRQ_VBL2 : DRIVER_IRQ_VBL))
> return -EINVAL;
>
> - seq = atomic_read((flags & _DRM_VBLANK_SECONDARY) ? &dev->vbl_received2
> - : &dev->vbl_received);
> + seq = atomic_read(&dev->vblank_count[crtc]);
>
> switch (vblwait.request.type & _DRM_VBLANK_TYPES_MASK) {
> case _DRM_VBLANK_RELATIVE:
This value is used as the basis for relative waits, so you need to make
sure it's up to date.
> @@ -311,15 +347,15 @@ int drm_wait_vblank(DRM_IOCTL_ARGS)
> }
> }
>
> - if (dev->vbl_pending >= 100) {
> + if (atomic_read(&dev->vbl_pending[crtc]) >= 100) {
> spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
> return -EBUSY;
> }
Previously, dev->vbl_pending was only used to make sure userspace can't
exhaust memory by scheduling an infinite number of signals. I don't
think this is necessary for blocking calls (as every process can only do
one such call at a time), is it?
> @@ -313,14 +313,14 @@ irqreturn_t
> i915_driver_irq_handler(DRM_IRQ_ARGS)
> (DRM_I915_VBLANK_PIPE_A | DRM_I915_VBLANK_PIPE_B))
> == (DRM_I915_VBLANK_PIPE_A | DRM_I915_VBLANK_PIPE_B)) {
> if (temp & VSYNC_PIPEA_FLAG)
> - atomic_inc(&dev->vbl_received);
> + atomic_inc(&dev->vblank_count[0]);
> if (temp & VSYNC_PIPEB_FLAG)
> - atomic_inc(&dev->vbl_received2);
> + atomic_inc(&dev->vblank_count[1]);
> } else if (((temp & VSYNC_PIPEA_FLAG) &&
> (vblank_pipe & DRM_I915_VBLANK_PIPE_A)) ||
> ((temp & VSYNC_PIPEB_FLAG) &&
> (vblank_pipe & DRM_I915_VBLANK_PIPE_B)))
> - atomic_inc(&dev->vbl_received);
> + atomic_inc(&dev->vblank_count[0]);
>
> DRM_WAKEUP(&dev->vbl_queue);
> drm_vbl_send_signals(dev);
Maybe this should use i915_get_vblank_counter() instead of just
incrementing, in case e.g. an interrupt is missed.
> @@ -489,15 +458,116 @@ int i915_irq_wait(DRM_IOCTL_ARGS)
> return i915_wait_irq(dev, irqwait.irq_seq);
> }
>
> -static void i915_enable_interrupt (drm_device_t *dev)
> +void i915_enable_vblank(drm_device_t *dev, int crtc)
> {
> drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
>
> - dev_priv->irq_enable_reg = USER_INT_FLAG;
> - if (dev_priv->vblank_pipe & DRM_I915_VBLANK_PIPE_A)
> + switch (crtc) {
> + case 0:
> dev_priv->irq_enable_reg |= VSYNC_PIPEA_FLAG;
> - if (dev_priv->vblank_pipe & DRM_I915_VBLANK_PIPE_B)
> + break;
> + case 1:
> dev_priv->irq_enable_reg |= VSYNC_PIPEB_FLAG;
> + break;
> + default:
> + DRM_ERROR("tried to enable vblank on non-existent crtc %d\n",
> + crtc);
> + break;
> + }
> +
> + I915_WRITE16(I915REG_INT_ENABLE_R, dev_priv->irq_enable_reg);
> +}
Does this need to check that the X server enabled the vblank interrupt
for the pipe? What would happen if this ended up enabling it for an
inactive pipe?
> @@ -607,7 +677,7 @@ int i915_vblank_swap(DRM_IOCTL_ARGS)
>
> spin_unlock_irqrestore(&dev->drw_lock, irqflags);
>
> - curseq = atomic_read(pipe ? &dev->vbl_received2 : &dev->vbl_received);
> + curseq = atomic_read(&dev->vblank_count[pipe]);
>
> if (seqtype == _DRM_VBLANK_RELATIVE)
> swap.sequence += curseq;
Again, need to make sure this is up to date. This function probably
needs more changes for the new scheme, I'm actually not quite sure yet
how it fits in.
> @@ -721,6 +792,27 @@ void i915_driver_irq_postinstall(drm_device_t * dev)
>
> dev_priv->user_irq_lock = SPIN_LOCK_UNLOCKED;
> dev_priv->user_irq_refcount = 0;
> + dev_priv->irq_enable_reg = 0;
> + dev->vblank_count = kmalloc(sizeof(atomic_t) * 2, GFP_KERNEL);
> + if (!dev->vblank_count) {
> + DRM_ERROR("out of memory\n");
> + return;
> + }
> + dev->vbl_pending = kmalloc(sizeof(atomic_t) * 2, GFP_KERNEL);
> + if (!dev->vbl_pending) {
> + DRM_ERROR("out of memory\n");
> + kfree(dev->vblank_count);
> + return;
> + }
> +
> + /* Zero per-crtc vblank stuff */
> + for (i = 0; i < 2; i++) {
> + atomic_set(&dev->vbl_pending[i], 0);
> + atomic_set(&dev->vblank_count[i], 0);
> + dev_priv->vblank_offset[i] = 0;
> + }
> +
> + dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */
This code is shared between OSs, so please use drm_alloc instead of
kmalloc. Then again, it might be better to do all this in a core
function instead of duplicating it in each driver.
--
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
[email protected]
https://lists.sourceforge.net/lists/listinfo/dri-devel
