From: Jesse B. <jes...@in...> - 2009-02-26 21:24:28
|
This patchset fixes a number of bugs in the last set I sent out that I found after updating the xf86-video-intel code and testing a bit. Seems solid again with my modified compiz, but the fallback handling (when we blit instead of flip) is broken; I'm open to suggestions on how to make the code in i830_dri.c better. -- Jesse Barnes, Intel Open Source Technology Center |
From: Jesse B. <jb...@vi...> - 2009-02-26 21:29:19
|
Add a new page flip ioctl to the i915 driver. The new ioctl takes the new drm_i915_gem_page_flip arg, which contains a buffer object target for the flip, an offset into that buffer, and other buffer parameters to be used for the flip. If a flip is already pending on the object in question, the ioctl waits for it to finish first, then queues the flip. An optional wait flag causes the ioctl to wait for the flip to complete before the it returns. If an execbuffer containing an object with a pending flip comes in, it will stall until the flip completes, to preserve glXSwapBuffers semantics. Signed-off-by: Jesse Barnes <jb...@vi...> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 2d797ff..0d6a6d3 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -829,6 +829,175 @@ static int i915_set_status_page(struct drm_device *dev, void *data, return 0; } +static int i915_pipe_to_plane(struct drm_device *dev, int pipe) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + u32 reg, pipe_mask; + + if (pipe == 0) + pipe_mask = DISPPLANE_SEL_PIPE_A; + else + pipe_mask = DISPPLANE_SEL_PIPE_B; + + pipe_mask |= DISPLAY_PLANE_ENABLE; + + reg = I915_READ(DSPACNTR); + if ((reg & (DISPLAY_PLANE_ENABLE | DISPPLANE_SEL_PIPE_MASK)) == + pipe_mask) + return 0; + + reg = I915_READ(DSPBCNTR); + if ((reg & (DISPLAY_PLANE_ENABLE | DISPPLANE_SEL_PIPE_MASK)) == + pipe_mask) + return 1; + + return -1; +} + +bool +i915_gem_flip_pending(struct drm_gem_object *obj) +{ + struct drm_device *dev = obj->dev; + drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_gem_object *obj_priv = obj->driver_private; + unsigned long flags; + bool pending = false; + + spin_lock_irqsave(&dev_priv->vblank_lock, flags); + if (!list_empty(&obj_priv->vblank_head)) + pending = true; + spin_unlock_irqrestore(&dev_priv->vblank_lock, flags); + + return pending; +} + +static int i915_gem_page_flip(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_i915_gem_page_flip *flip_data = data; + drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_gem_object *obj; + struct drm_i915_gem_object *obj_priv; + unsigned long flags; + uint32_t offset; + unsigned int pitch, pipe, plane, tiled; + int ret = 0, vblank_ref_err = 0, reqno; + RING_LOCALS; + + if (!(dev->driver->driver_features & DRIVER_GEM)) + return -ENODEV; + + /* + * Reject unknown flags so future userspace knows what we (don't) + * support + */ + if (flip_data->flags & (~I915_PAGE_FLIP_WAIT)) { + DRM_ERROR("bad page flip flags\n"); + return -EINVAL; + } + + if ((pipe = flip_data->pipe) > 1) { + DRM_ERROR("bad pipe\n"); + return -EINVAL; + } + + plane = i915_pipe_to_plane(dev, pipe); + if (plane < 0) { + DRM_ERROR("bad pipe (no planes enabled?)\n"); + return -EINVAL; + } + + obj = drm_gem_object_lookup(dev, file_priv, flip_data->handle); + if (obj == NULL) + return -EBADF; + + /* + * Make sure the new buffer is in bounds + * FIXME: should probably check against current mode as well + */ + if (flip_data->offset >= obj->size) { + DRM_ERROR("bad flip offset\n"); + ret = -EINVAL; + goto out_unref_prelock; + } + + obj_priv = obj->driver_private; + + if (i915_gem_flip_pending(obj)) + wait_for_completion(&obj_priv->vblank); + + mutex_lock(&dev->struct_mutex); + if (obj_priv->tiling_mode != I915_TILING_NONE && + obj_priv->tiling_mode != I915_TILING_X) { + DRM_ERROR("can only flip non-tiled or X tiled pages\n"); + ret = -EINVAL; + goto out_unref; + } + + vblank_ref_err = drm_vblank_get(dev, pipe); + if (!vblank_ref_err) { + ret = i915_gem_object_pin(obj, 0); + if (ret) { + DRM_ERROR("failed to pin object for flip\n"); + ret = -EBUSY; + goto out_unref; + } + } + + /* + * Put the object in the GTT domain before the flip, + * since there may be outstanding rendering + */ + i915_gem_object_set_to_gtt_domain(obj, 0); + + offset = obj_priv->gtt_offset + flip_data->offset; + + pitch = obj_priv->stride; + tiled = (obj_priv->tiling_mode == I915_TILING_X); + + BEGIN_LP_RING(4); + OUT_RING(CMD_OP_DISPLAYBUFFER_INFO | (plane << 20)); + OUT_RING(pitch); + if (IS_I965G(dev)) { + OUT_RING(offset | tiled); + OUT_RING(((flip_data->x - 1) << 16) | (flip_data->y - 1)); + } else { + OUT_RING(offset); + OUT_RING(MI_NOOP); + } + ADVANCE_LP_RING(); + + reqno = i915_add_request(dev, 0); + + mutex_unlock(&dev->struct_mutex); + + /* + * This is a bit racy; the flip above may have already happened + * by the time we get here. If that happens, the new back buffer + * won't be available for rendering for one extra frame, since + * the vblank list won't have the object. + */ + spin_lock_irqsave(&dev_priv->vblank_lock, flags); + if (!vblank_ref_err) + list_add_tail(&obj_priv->vblank_head, + &dev_priv->mm.vblank_list[pipe]); + + obj_priv->flip_seqno = reqno; + spin_unlock_irqrestore(&dev_priv->vblank_lock, flags); + + if (!vblank_ref_err && (flip_data->flags & I915_PAGE_FLIP_WAIT)) + wait_for_completion(&obj_priv->vblank); + +out_unref_prelock: + /* Take lock again for unref */ + mutex_lock(&dev->struct_mutex); +out_unref: + drm_gem_object_unreference(obj); + mutex_unlock(&dev->struct_mutex); + + return ret; +} + /** * i915_probe_agp - get AGP bootup configuration * @pdev: PCI device @@ -1307,6 +1476,7 @@ struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF(DRM_I915_GEM_SET_TILING, i915_gem_set_tiling, 0), DRM_IOCTL_DEF(DRM_I915_GEM_GET_TILING, i915_gem_get_tiling, 0), DRM_IOCTL_DEF(DRM_I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, 0), + DRM_IOCTL_DEF(DRM_I915_GEM_PAGE_FLIP, i915_gem_page_flip, DRM_AUTH|DRM_MASTER), }; int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3951a12..148e80a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -159,6 +159,10 @@ typedef struct drm_i915_private { u32 irq_mask_reg; u32 pipestat[2]; + /** Protects vblank list */ + spinlock_t vblank_lock; + struct work_struct vblank_work; + int tex_lru_log_granularity; int allow_batchbuffer; struct mem_block *agp_heap; @@ -338,6 +342,11 @@ typedef struct drm_i915_private { */ struct delayed_work retire_work; + /** + * List of objects waiting on vblank events (one per pipe) + */ + struct list_head vblank_list[2]; + uint32_t next_gem_seqno; /** @@ -389,6 +398,13 @@ struct drm_i915_gem_object { /** This object's place on the active/flushing/inactive lists */ struct list_head list; + /** Object's place on the vblank list (protected by vblank_lock)*/ + struct list_head vblank_head; + /** sequence number for flip (when it passes the flip is done), + * protected by vblank lock + */ + int flip_seqno; + /** * This is set if the object is on the active or flushing lists * (has pending rendering), and is not set if it's on inactive (ready @@ -457,6 +473,9 @@ struct drm_i915_gem_object { /** for phy allocated objects */ struct drm_i915_gem_phys_object *phys_obj; + + /** for page flips and other vblank related blocks */ + struct completion vblank; }; /** @@ -516,6 +535,7 @@ extern long i915_compat_ioctl(struct file *filp, unsigned int cmd, extern int i915_emit_box(struct drm_device *dev, struct drm_clip_rect __user *boxes, int i, int DR1, int DR4); +extern bool i915_gem_flip_pending(struct drm_gem_object *obj); /* i915_irq.c */ extern int i915_irq_emit(struct drm_device *dev, void *data, @@ -625,6 +645,9 @@ int i915_gem_attach_phys_object(struct drm_device *dev, void i915_gem_detach_phys_object(struct drm_device *dev, struct drm_gem_object *obj); void i915_gem_free_all_phys_object(struct drm_device *dev); +uint32_t i915_add_request(struct drm_device *dev, uint32_t flush_domains); +int i915_seqno_passed(uint32_t seq1, uint32_t seq2); +uint32_t i915_get_gem_seqno(struct drm_device *dev); /* i915_gem_tiling.c */ void i915_gem_detect_bit_6_swizzle(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 3e025d5..5270d25 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -900,7 +900,7 @@ i915_gem_object_move_to_inactive(struct drm_gem_object *obj) * * Returned sequence numbers are nonzero on success. */ -static uint32_t +uint32_t i915_add_request(struct drm_device *dev, uint32_t flush_domains) { drm_i915_private_t *dev_priv = dev->dev_private; @@ -1028,7 +1028,7 @@ i915_gem_retire_request(struct drm_device *dev, /** * Returns true if seq1 is later than seq2. */ -static int +int i915_seqno_passed(uint32_t seq1, uint32_t seq2) { return (int32_t)(seq1 - seq2) >= 0; @@ -2498,6 +2498,25 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, goto pre_mutex_err; } + /* Look up object handles */ + for (i = 0; i < args->buffer_count; i++) { + object_list[i] = drm_gem_object_lookup(dev, file_priv, + exec_list[i].handle); + if (object_list[i] == NULL) { + DRM_ERROR("Invalid object handle %d at index %d\n", + exec_list[i].handle, i); + ret = -EBADF; + goto pre_mutex_err; + } + + if (i915_gem_flip_pending(object_list[i])) { + struct drm_i915_gem_object *obj_priv; + + obj_priv = object_list[i]->driver_private; + wait_for_completion(&obj_priv->vblank); + } + } + mutex_lock(&dev->struct_mutex); i915_verify_inactive(dev, __FILE__, __LINE__); @@ -2516,18 +2535,6 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, goto pre_mutex_err; } - /* Look up object handles */ - for (i = 0; i < args->buffer_count; i++) { - object_list[i] = drm_gem_object_lookup(dev, file_priv, - exec_list[i].handle); - if (object_list[i] == NULL) { - DRM_ERROR("Invalid object handle %d at index %d\n", - exec_list[i].handle, i); - ret = -EBADF; - goto err; - } - } - /* Pin and relocate */ for (pin_tries = 0; ; pin_tries++) { ret = 0; @@ -2920,6 +2927,9 @@ int i915_gem_init_object(struct drm_gem_object *obj) obj_priv->obj = obj; obj_priv->fence_reg = I915_FENCE_REG_NONE; INIT_LIST_HEAD(&obj_priv->list); + INIT_LIST_HEAD(&obj_priv->vblank_head); + + init_completion(&obj_priv->vblank); return 0; } @@ -2980,8 +2990,10 @@ int i915_gem_idle(struct drm_device *dev) { drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_gem_object *obj_priv, *tmp; + unsigned long irqflags; uint32_t seqno, cur_seqno, last_seqno; - int stuck, ret; + int stuck, ret, pipe; mutex_lock(&dev->struct_mutex); @@ -2995,10 +3007,25 @@ i915_gem_idle(struct drm_device *dev) */ dev_priv->mm.suspended = 1; + mutex_unlock(&dev->struct_mutex); + + /* Wait for any outstanding flips */ + spin_lock_irqsave(&dev_priv->vblank_lock, irqflags); + for (pipe = 0; pipe < 2; pipe++) { + list_for_each_entry_safe(obj_priv, tmp, + &dev_priv->mm.vblank_list[pipe], + vblank_head) { + spin_unlock_irqrestore(&dev_priv->vblank_lock, irqflags); + wait_for_completion(&obj_priv->vblank); + spin_lock_irqsave(&dev_priv->vblank_lock, irqflags); + } + } + spin_unlock_irqrestore(&dev_priv->vblank_lock, irqflags); + /* Cancel the retire work handler, wait for it to finish if running */ - mutex_unlock(&dev->struct_mutex); cancel_delayed_work_sync(&dev_priv->mm.retire_work); + mutex_lock(&dev->struct_mutex); i915_kernel_lost_context(dev); @@ -3358,9 +3385,12 @@ i915_gem_load(struct drm_device *dev) INIT_LIST_HEAD(&dev_priv->mm.flushing_list); INIT_LIST_HEAD(&dev_priv->mm.inactive_list); INIT_LIST_HEAD(&dev_priv->mm.request_list); + INIT_LIST_HEAD(&dev_priv->mm.vblank_list[0]); + INIT_LIST_HEAD(&dev_priv->mm.vblank_list[1]); INIT_DELAYED_WORK(&dev_priv->mm.retire_work, i915_gem_retire_work_handler); dev_priv->mm.next_gem_seqno = 1; + spin_lock_init(&dev_priv->vblank_lock); /* Old X drivers will take 0-2 for front, back, depth buffers */ dev_priv->fence_reg_start = 3; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 548ff2c..f50df88 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -187,6 +187,36 @@ u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe) return I915_READ(reg); } +static void i915_vblank_work_func(struct work_struct *work) +{ + drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t, + vblank_work); + struct drm_device *dev = dev_priv->dev; + struct drm_i915_gem_object *obj_priv, *tmp; + unsigned long irqflags; + int pipe, cur_seqno; + + mutex_lock(&dev->struct_mutex); + + spin_lock_irqsave(&dev_priv->vblank_lock, irqflags); + for (pipe = 0; pipe < 2; pipe++) { + list_for_each_entry_safe(obj_priv, tmp, + &dev_priv->mm.vblank_list[pipe], + vblank_head) { + cur_seqno = i915_get_gem_seqno(dev); + if (i915_seqno_passed(cur_seqno, + obj_priv->flip_seqno)) { + list_del_init(&obj_priv->vblank_head); + drm_vblank_put(dev, pipe); + i915_gem_object_unpin(obj_priv->obj); + complete(&obj_priv->vblank); + } + } + } + spin_unlock_irqrestore(&dev_priv->vblank_lock, irqflags); + mutex_unlock(&dev->struct_mutex); +} + irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS) { struct drm_device *dev = (struct drm_device *) arg; @@ -269,6 +299,9 @@ irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS) drm_handle_vblank(dev, 1); } + if (vblank) + schedule_work(&dev_priv->vblank_work); + if ((pipeb_stats & I915_LEGACY_BLC_EVENT_STATUS) || (iir & I915_ASLE_INTERRUPT)) opregion_asle_intr(dev); @@ -533,6 +566,7 @@ void i915_driver_irq_preinstall(struct drm_device * dev) I915_WRITE(IMR, 0xffffffff); I915_WRITE(IER, 0x0); (void) I915_READ(IER); + INIT_WORK(&dev_priv->vblank_work, i915_vblank_work_func); } int i915_driver_irq_postinstall(struct drm_device *dev) diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h index 912cd52..1ac4ded 100644 --- a/include/drm/i915_drm.h +++ b/include/drm/i915_drm.h @@ -184,6 +184,7 @@ typedef struct _drm_i915_sarea { #define DRM_I915_GEM_GET_TILING 0x22 #define DRM_I915_GEM_GET_APERTURE 0x23 #define DRM_I915_GEM_MMAP_GTT 0x24 +#define DRM_I915_GEM_PAGE_FLIP 0x25 #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) @@ -219,6 +220,7 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_GEM_SET_TILING DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_SET_TILING, struct drm_i915_gem_set_tiling) #define DRM_IOCTL_I915_GEM_GET_TILING DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_GET_TILING, struct drm_i915_gem_get_tiling) #define DRM_IOCTL_I915_GEM_GET_APERTURE DRM_IOR (DRM_COMMAND_BASE + DRM_I915_GEM_GET_APERTURE, struct drm_i915_gem_get_aperture) +#define DRM_IOCTL_I915_GEM_PAGE_FLIP DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PAGE_FLIP, struct drm_i915_gem_page_flip) /* Allow drivers to submit batchbuffers directly to hardware, relying * on the security mechanisms provided by hardware. @@ -654,4 +656,30 @@ struct drm_i915_gem_get_aperture { uint64_t aper_available_size; }; +#define I915_PAGE_FLIP_WAIT (1<<0) /* block on page flip completion */ + +struct drm_i915_gem_page_flip { + /** Handle of new front buffer */ + uint32_t handle; + + /** Offset into the object to use */ + uint64_t offset; + + /** + * page flip flags (wait on flip only for now) + */ + uint32_t flags; + + /** + * pipe to flip + */ + uint32_t pipe; + + /** + * screen dimensions for flip + */ + uint32_t x; + uint32_t y; +}; + #endif /* _I915_DRM_H_ */ |
From: Zhao, C. <chu...@in...> - 2009-02-26 21:30:48
|
Hi Jesse, For the page flipping to work, is there any need to change the old application to make it work? Thanks! Calvin -----Original Message----- From: Jesse Barnes [mailto:jes...@in...] Sent: Thursday, February 26, 2009 1:24 PM To: dri...@li... Cc: int...@li... Subject: Another page flipping update This patchset fixes a number of bugs in the last set I sent out that I found after updating the xf86-video-intel code and testing a bit. Seems solid again with my modified compiz, but the fallback handling (when we blit instead of flip) is broken; I'm open to suggestions on how to make the code in i830_dri.c better. -- Jesse Barnes, Intel Open Source Technology Center ------------------------------------------------------------------------------ Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise -Strategies to boost innovation and cut costs with open source participation -Receive a $600 discount off the registration fee with the source code: SFAD http://p.sf.net/sfu/XcvMzF8H -- _______________________________________________ Dri-devel mailing list Dri...@li... https://lists.sourceforge.net/lists/listinfo/dri-devel |
From: Jesse B. <jes...@in...> - 2009-02-26 21:43:34
|
Applications that use glXSwapBuffers will benefit without modification. Apps that use the Mesa copy sub buffer extension won't benefit though. compiz is in between; it uses swapbuffers for when the whole screen is damaged, but uses sub buffer copies otherwise. So most of its drawing will tear by default. Jesse On Thursday, February 26, 2009 1:30:43 pm Zhao, Chunfeng wrote: > Hi Jesse, > For the page flipping to work, is there any need to change the old > application to make it work? > > Thanks! > > Calvin > > -----Original Message----- > From: Jesse Barnes [mailto:jes...@in...] > Sent: Thursday, February 26, 2009 1:24 PM > To: dri...@li... > Cc: int...@li... > Subject: Another page flipping update > > This patchset fixes a number of bugs in the last set I sent out that I > found after updating the xf86-video-intel code and testing a bit. Seems > solid again with my modified compiz, but the fallback handling (when we > blit instead of flip) is broken; I'm open to suggestions on how to make the > code in i830_dri.c better. -- Jesse Barnes, Intel Open Source Technology Center |
From: Jesse B. <jb...@vi...> - 2009-02-26 21:31:20
|
Add support to Mesa and the intel driver for the new DRI2 swapbuffers interface. Uses the new flush hook to make sure any outstanding rendering is complete before sending the swapbuffers request. Signed-off-by: Jesse Barnes <jb...@vi...> diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h index a726b93..b663028 100644 --- a/include/GL/internal/dri_interface.h +++ b/include/GL/internal/dri_interface.h @@ -681,6 +681,9 @@ struct __DRIdri2ExtensionRec { __DRIcontext *shared, void *loaderPrivate); + void (*setBuffers)(__DRIdrawable *drawable, + __DRIbuffer *buffers, + int count); }; #endif diff --git a/src/glx/x11/dri2.c b/src/glx/x11/dri2.c index f967432..7af5484 100644 --- a/src/glx/x11/dri2.c +++ b/src/glx/x11/dri2.c @@ -30,7 +30,7 @@ * Kristian Høgsberg (kr...@re...) */ - +#include <stdio.h> #define NEED_REPLIES #include <X11/Xlibint.h> #include <X11/extensions/Xext.h> @@ -299,3 +299,51 @@ void DRI2CopyRegion(Display *dpy, XID drawable, XserverRegion region, UnlockDisplay(dpy); SyncHandle(); } + +DRI2Buffer *DRI2SwapBuffers(Display *dpy, XID drawable, int *recv_count) +{ + XExtDisplayInfo *info = DRI2FindDisplay(dpy); + xDRI2SwapBuffersReq *req; + xDRI2SwapBuffersReply rep; + xDRI2Buffer repBuffer; + DRI2Buffer *new_buffers; + int i; + + XextCheckExtension (dpy, info, dri2ExtensionName, False); + + LockDisplay(dpy); + GetReq(DRI2SwapBuffers, req); + req->reqType = info->codes->major_opcode; + req->dri2ReqType = X_DRI2SwapBuffers; + req->drawable = drawable; + + if (!_XReply(dpy, (xReply *)&rep, 0, xFalse)) { + UnlockDisplay(dpy); + SyncHandle(); + return NULL; + } + + new_buffers = Xmalloc(rep.count * sizeof *new_buffers); + if (new_buffers == NULL) { + _XEatData(dpy, rep.count * sizeof repBuffer); + UnlockDisplay(dpy); + SyncHandle(); + return NULL; + } + + for (i = 0; i < rep.count; i++) { + _XReadPad(dpy, (char *) &repBuffer, sizeof repBuffer); + new_buffers[i].attachment = repBuffer.attachment; + new_buffers[i].name = repBuffer.name; + new_buffers[i].pitch = repBuffer.pitch; + new_buffers[i].cpp = repBuffer.cpp; + new_buffers[i].flags = repBuffer.flags; + } + + *recv_count = rep.count; + + UnlockDisplay(dpy); + SyncHandle(); + + return new_buffers; +} diff --git a/src/glx/x11/dri2.h b/src/glx/x11/dri2.h index 356c6bc..83dfaf6 100644 --- a/src/glx/x11/dri2.h +++ b/src/glx/x11/dri2.h @@ -67,4 +67,7 @@ extern void DRI2CopyRegion(Display *dpy, XID drawable, XserverRegion region, CARD32 dest, CARD32 src); +extern DRI2Buffer * +DRI2SwapBuffers(Display *dpy, XID drawable, int *count); + #endif diff --git a/src/glx/x11/dri2_glx.c b/src/glx/x11/dri2_glx.c index 0ef5d3a..f39decf 100644 --- a/src/glx/x11/dri2_glx.c +++ b/src/glx/x11/dri2_glx.c @@ -35,6 +35,7 @@ #include <X11/Xlib.h> #include <X11/extensions/Xfixes.h> #include <X11/extensions/Xdamage.h> +#include "glapi.h" #include "glxclient.h" #include "glcontextmodes.h" #include "xf86dri.h" @@ -61,6 +62,8 @@ struct __GLXDRIdisplayPrivateRec { int driMinor; int driPatch; + int swapAvailable; + unsigned long configureSeqno; Bool (*oldConfigProc)(Display *, XEvent *, xEvent *); }; @@ -223,8 +226,40 @@ static void dri2CopySubBuffer(__GLXDRIdrawable *pdraw, static void dri2SwapBuffers(__GLXDRIdrawable *pdraw) { __GLXDRIdrawablePrivate *priv = (__GLXDRIdrawablePrivate *) pdraw; + __GLXdisplayPrivate *dpyPriv = __glXInitialize(priv->base.psc->dpy); + __GLXDRIdisplayPrivate *pdp = + (__GLXDRIdisplayPrivate *)dpyPriv->dri2Display; + __GLXscreenConfigs *psc = pdraw->psc; + DRI2Buffer *buffers; + int i, count; + + #ifdef __DRI2_FLUSH + if (pdraw->psc->f) + (*pdraw->psc->f->flush)(pdraw->driDrawable); +#endif - dri2CopySubBuffer(pdraw, 0, 0, priv->width, priv->height); + /* Old servers can't handle swapbuffers */ + if (!pdp->swapAvailable) + return dri2CopySubBuffer(pdraw, 0, 0, priv->width, priv->height); + + buffers = DRI2SwapBuffers(pdraw->psc->dpy, pdraw->drawable, &count); + if (buffers == NULL || !count) + return dri2CopySubBuffer(pdraw, 0, 0, priv->width, priv->height); + + /* Update our buffer list with what was returned */ + for (i = 0; i < count; i++) { + priv->buffers[i].attachment = buffers[i].attachment; + priv->buffers[i].name = buffers[i].name; + priv->buffers[i].pitch = buffers[i].pitch; + priv->buffers[i].cpp = buffers[i].cpp; + priv->buffers[i].flags = buffers[i].flags; + } + + priv->bufferCount = count; + + (*psc->dri2->setBuffers)(pdraw->driDrawable, priv->buffers, count); + + Xfree(buffers); } static void dri2WaitX(__GLXDRIdrawable *pdraw) @@ -517,6 +552,12 @@ _X_HIDDEN __GLXDRIdisplay *dri2CreateDisplay(Display *dpy) pdp->driPatch = 0; pdp->configureSeqno = 0; + pdp->swapAvailable = 0; + printf("dri2 version %d.%d\n", pdp->driMajor, pdp->driMinor); + if (pdp->driMinor > 0) { + printf("enabling buffer swaps\n"); + pdp->swapAvailable = 1; + } pdp->base.destroyDisplay = dri2DestroyDisplay; pdp->base.createScreen = dri2CreateScreen; diff --git a/src/mesa/drivers/dri/common/dri_util.c b/src/mesa/drivers/dri/common/dri_util.c index ae79055..75afc45 100644 --- a/src/mesa/drivers/dri/common/dri_util.c +++ b/src/mesa/drivers/dri/common/dri_util.c @@ -488,6 +488,14 @@ dri2CreateNewDrawable(__DRIscreen *screen, return pdraw; } +static void +dri2SetBuffers(__DRIdrawable *draw, __DRIbuffer *buffers, int count) +{ + __DRIscreen *psp = draw->driScreenPriv; + + (*psp->DriverAPI.SetBuffers)(draw, buffers, count); +} + static void driDestroyDrawable(__DRIdrawable *pdp) @@ -832,6 +840,7 @@ const __DRIdri2Extension driDRI2Extension = { dri2CreateNewScreen, dri2CreateNewDrawable, dri2CreateNewContext, + dri2SetBuffers, }; /* This is the table of extensions that the loader will dlsym() for. */ diff --git a/src/mesa/drivers/dri/common/dri_util.h b/src/mesa/drivers/dri/common/dri_util.h index c95a5c8..2a6e1cd 100644 --- a/src/mesa/drivers/dri/common/dri_util.h +++ b/src/mesa/drivers/dri/common/dri_util.h @@ -225,6 +225,13 @@ struct __DriverAPIRec { /* DRI2 Entry point */ const __DRIconfig **(*InitScreen2) (__DRIscreen * priv); + + /** + * Update the render buffers with a new set + */ + void (*SetBuffers) ( __DRIdrawable *drawable, + __DRIbuffer *buffers, + int count); }; extern const struct __DriverAPIRec driDriverAPI; diff --git a/src/mesa/drivers/dri/intel/intel_context.c b/src/mesa/drivers/dri/intel/intel_context.c index 1aa173d..bef1907 100644 --- a/src/mesa/drivers/dri/intel/intel_context.c +++ b/src/mesa/drivers/dri/intel/intel_context.c @@ -169,6 +169,30 @@ intelGetString(GLcontext * ctx, GLenum name) } } +/* + * DRI may give us with a new set of buffers after a flip or swap, so + * update the attachments here. + */ +void intelSetBuffers(__DRIdrawable *drawable, + __DRIbuffer *buffers, + int count) +{ + __DRIcontext *context = drawable->driContextPriv; + struct intel_framebuffer *intel_fb = drawable->driverPrivate; + struct intel_context *intel = context->driverPrivate; + struct intel_renderbuffer *rb; + struct intel_region *region; + const char *region_name; + int i; + + /* + * Ignore the buffers & count args, we'll just pick them up from our + * drawable. + */ + intel_update_renderbuffers(context, drawable); + intel_draw_buffer(&intel->ctx, &intel_fb->Base); +} + void intel_update_renderbuffers(__DRIcontext *context, __DRIdrawable *drawable) { @@ -399,6 +423,16 @@ intel_glFlush(GLcontext *ctx) intel_flush(ctx, GL_TRUE); } +void intelFlushDrawable(__DRIdrawable *drawable) +{ + __DRIdrawablePrivate * dPriv = drawable->driverPrivate; + struct intel_context *intel = + (struct intel_context *) dPriv->driContextPriv->driverPrivate; + GLcontext *ctx = &intel->ctx; + + intel_flush(ctx, GL_TRUE); +} + void intelFinish(GLcontext * ctx) { diff --git a/src/mesa/drivers/dri/intel/intel_extensions.h b/src/mesa/drivers/dri/intel/intel_extensions.h index 97147ec..9283ee9 100644 --- a/src/mesa/drivers/dri/intel/intel_extensions.h +++ b/src/mesa/drivers/dri/intel/intel_extensions.h @@ -32,5 +32,8 @@ extern void intelInitExtensions(GLcontext *ctx, GLboolean enable_imaging); +extern void +intelFlushDrawable(__DRIdrawable *drawable); + #endif diff --git a/src/mesa/drivers/dri/intel/intel_screen.c b/src/mesa/drivers/dri/intel/intel_screen.c index 09eba13..38ab561 100644 --- a/src/mesa/drivers/dri/intel/intel_screen.c +++ b/src/mesa/drivers/dri/intel/intel_screen.c @@ -211,6 +211,11 @@ static const __DRItexBufferExtension intelTexBufferExtension = { intelSetTexBuffer, }; +static const __DRI2flushExtension intelFlushExtension = { + { __DRI2_FLUSH, __DRI2_FLUSH_VERSION }, + intelFlushDrawable, +}; + static const __DRIextension *intelScreenExtensions[] = { &driReadDrawableExtension, &driCopySubBufferExtension.base, @@ -828,4 +833,6 @@ const struct __DriverAPIRec driDriverAPI = { .CopySubBuffer = intelCopySubBuffer, .InitScreen2 = intelInitScreen2, + + .SetBuffers = intelSetBuffers, }; diff --git a/src/mesa/drivers/dri/intel/intel_screen.h b/src/mesa/drivers/dri/intel/intel_screen.h index e1036de..b23dcee 100644 --- a/src/mesa/drivers/dri/intel/intel_screen.h +++ b/src/mesa/drivers/dri/intel/intel_screen.h @@ -103,4 +103,7 @@ intelMakeCurrent(__DRIcontextPrivate * driContextPriv, extern struct intel_context *intelScreenContext(intelScreenPrivate *intelScreen); +extern void intelSetBuffers(__DRIdrawable *drawable, __DRIbuffer *buffers, + int count); + #endif |
From: Jesse B. <jb...@vi...> - 2009-02-26 21:33:13
|
Add support for the new DRI2 swapbuffers request by supporting the protocol and adding a new driver hook to perform the swap. If it fails we'll fall back to blitting. Signed-off-by: Jesse Barnes <jb...@vi...> diff --git a/glx/glxdri2.c b/glx/glxdri2.c index 4e76c71..b76a786 100644 --- a/glx/glxdri2.c +++ b/glx/glxdri2.c @@ -131,10 +131,25 @@ __glXDRIdrawableCopySubBuffer(__GLXdrawable *drawable, static GLboolean __glXDRIdrawableSwapBuffers(__GLXdrawable *drawable) { - __GLXDRIdrawable *private = (__GLXDRIdrawable *) drawable; + __GLXDRIdrawable *priv = (__GLXDRIdrawable *) drawable; + DRI2BufferPtr buffers; + int i, count; + + buffers = DRI2SwapBuffers(drawable->pDraw, &count); + if (!buffers) + return FALSE; + + for (i = 0; i < count; i++) { + priv->buffers[i].attachment = buffers[i].attachment; + priv->buffers[i].name = buffers[i].name; + priv->buffers[i].pitch = buffers[i].pitch; + priv->buffers[i].cpp = buffers[i].cpp; + priv->buffers[i].flags = buffers[i].flags; + } + + priv->count = count; - __glXDRIdrawableCopySubBuffer(drawable, 0, 0, - private->width, private->height); + xfree(buffers); return TRUE; } diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 0f2e24b..375a13d 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -66,6 +66,7 @@ typedef struct _DRI2Screen { DRI2CreateBuffersProcPtr CreateBuffers; DRI2DestroyBuffersProcPtr DestroyBuffers; DRI2CopyRegionProcPtr CopyRegion; + DRI2SwapBuffersProcPtr SwapBuffers; HandleExposuresProcPtr HandleExposures; } DRI2ScreenRec, *DRI2ScreenPtr; @@ -188,6 +189,32 @@ DRI2CopyRegion(DrawablePtr pDraw, RegionPtr pRegion, return Success; } +DRI2BufferPtr +DRI2SwapBuffers(DrawablePtr pDraw, int *reply_count) +{ + DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); + DRI2DrawablePtr pPriv; + DRI2BufferPtr buffers; + + pPriv = DRI2GetDrawable(pDraw); + if (pPriv == NULL) + return NULL; + + /* Driver will give us a new set of buffers, so free the old ones */ + buffers = (*ds->SwapBuffers)(pDraw, pPriv->buffers, pPriv->bufferCount); + if (!buffers) { + *reply_count = 0; + return NULL; + } + + (*ds->DestroyBuffers)(pDraw, pPriv->buffers, pPriv->bufferCount); + pPriv->buffers = buffers; + + *reply_count = pPriv->bufferCount; + + return buffers; +} + void DRI2DestroyDrawable(DrawablePtr pDraw) { @@ -264,6 +291,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) ds->CreateBuffers = info->CreateBuffers; ds->DestroyBuffers = info->DestroyBuffers; ds->CopyRegion = info->CopyRegion; + ds->SwapBuffers = info->SwapBuffers; dixSetPrivate(&pScreen->devPrivates, dri2ScreenPrivateKey, ds); diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h index 5e7fd65..8cb9e94 100644 --- a/hw/xfree86/dri2/dri2.h +++ b/hw/xfree86/dri2/dri2.h @@ -54,6 +54,9 @@ typedef void (*DRI2CopyRegionProcPtr)(DrawablePtr pDraw, RegionPtr pRegion, DRI2BufferPtr pDestBuffer, DRI2BufferPtr pSrcBuffer); +typedef DRI2BufferPtr (*DRI2SwapBuffersProcPtr)(DrawablePtr pDraw, + DRI2BufferPtr buffers, + int count); typedef void (*DRI2WaitProcPtr)(WindowPtr pWin, unsigned int sequence); @@ -67,6 +70,7 @@ typedef struct { DRI2CreateBuffersProcPtr CreateBuffers; DRI2DestroyBuffersProcPtr DestroyBuffers; DRI2CopyRegionProcPtr CopyRegion; + DRI2SwapBuffersProcPtr SwapBuffers; DRI2WaitProcPtr Wait; } DRI2InfoRec, *DRI2InfoPtr; @@ -100,4 +104,6 @@ int DRI2CopyRegion(DrawablePtr pDraw, unsigned int dest, unsigned int src); +DRI2BufferPtr DRI2SwapBuffers(DrawablePtr pDraw, int *count); + #endif diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c index 0a1dce4..5582a5f 100644 --- a/hw/xfree86/dri2/dri2ext.c +++ b/hw/xfree86/dri2/dri2ext.c @@ -269,6 +269,45 @@ ProcDRI2CopyRegion(ClientPtr client) } static int +ProcDRI2SwapBuffers(ClientPtr client) +{ + REQUEST(xDRI2SwapBuffersReq); + xDRI2SwapBuffersReply rep; + DrawablePtr pDrawable; + DRI2BufferPtr buffers; + xDRI2Buffer buffer; + int status; + int i, count; + + REQUEST_SIZE_MATCH(xDRI2SwapBuffersReq); + + if (!validDrawable(client, stuff->drawable, &pDrawable, &status)) + return status; + + buffers = DRI2SwapBuffers(pDrawable, &count); + if (!buffers) { + return BadAlloc; + } + + rep.type = X_Reply; + rep.length = count * sizeof(xDRI2Buffer) / 4; + rep.count = count; + rep.sequenceNumber = client->sequence; + WriteToClient(client, sizeof(xDRI2GetBuffersReply), &rep); + + for (i = 0; i < count; i++) { + buffer.attachment = buffers[i].attachment; + buffer.name = buffers[i].name; + buffer.pitch = buffers[i].pitch; + buffer.cpp = buffers[i].cpp; + buffer.flags = buffers[i].flags; + WriteToClient(client, sizeof(xDRI2Buffer), &buffer); + } + + return client->noClientException; +} + +static int ProcDRI2Dispatch (ClientPtr client) { REQUEST(xReq); @@ -294,6 +333,8 @@ ProcDRI2Dispatch (ClientPtr client) return ProcDRI2GetBuffers(client); case X_DRI2CopyRegion: return ProcDRI2CopyRegion(client); + case X_DRI2SwapBuffers: + return ProcDRI2SwapBuffers(client); default: return BadRequest; } |
From: Jesse B. <jb...@vi...> - 2009-02-26 21:36:45
|
Support the new swapbuffers request using the new page flipping ioctl if possible. This patch still needs some work; there's a bug in the no-flip case that causes us to lose track of pixmaps, and the pipe is still hardcoded to 1, but that should be easy to fix. The code is pretty ugly too; it seems like getbuffers and swapbuffers could probably share more code, but we need to copy all the buffers in swapbuffers to return them... diff --git a/src/i830.h b/src/i830.h index 7904b9f..5c65ce8 100644 --- a/src/i830.h +++ b/src/i830.h @@ -452,6 +452,7 @@ typedef struct _I830Rec { #endif XF86ModReqInfo shadowReq; /* to test for later libshadow */ + Bool shadow_present; Rotation rotation; void (*PointerMoved)(int, int, int); CreateScreenResourcesProcPtr CreateScreenResources; diff --git a/src/i830_bios.c b/src/i830_bios.c index 6baacd4..72408f0 100644 --- a/src/i830_bios.c +++ b/src/i830_bios.c @@ -135,12 +135,6 @@ parse_panel_data(I830Ptr pI830, struct bdb_header *bdb) fixed_mode->Clock = _PIXEL_CLOCK(timing_ptr) / 1000; fixed_mode->type = M_T_PREFERRED; - /* Some VBTs have bogus h/vtotal values */ - if (fixed_mode->HSyncEnd > fixed_mode->HTotal) - fixed_mode->HTotal = fixed_mode->HSyncEnd + 1; - if (fixed_mode->VSyncEnd > fixed_mode->VTotal) - fixed_mode->VTotal = fixed_mode->VSyncEnd + 1; - xf86SetModeDefaultName(fixed_mode); pI830->lvds_fixed_mode = fixed_mode; diff --git a/src/i830_display.c b/src/i830_display.c index 8a5cf24..692349e 100644 --- a/src/i830_display.c +++ b/src/i830_display.c @@ -1669,6 +1669,9 @@ i830_crtc_shadow_create(xf86CrtcPtr crtc, void *data, int width, int height) } if (intel_crtc->rotate_mem && intel_crtc->rotate_mem->bo) i830_set_pixmap_bo(rotate_pixmap, intel_crtc->rotate_mem->bo); + + pI830->shadow_present = TRUE; + return rotate_pixmap; } @@ -1677,6 +1680,7 @@ i830_crtc_shadow_destroy(xf86CrtcPtr crtc, PixmapPtr rotate_pixmap, void *data) { ScrnInfoPtr pScrn = crtc->scrn; I830CrtcPrivatePtr intel_crtc = crtc->driver_private; + I830Ptr pI830 = I830PTR(pScrn); if (rotate_pixmap) FreeScratchPixmapHeader(rotate_pixmap); @@ -1687,6 +1691,7 @@ i830_crtc_shadow_destroy(xf86CrtcPtr crtc, PixmapPtr rotate_pixmap, void *data) i830_free_memory(pScrn, intel_crtc->rotate_mem); intel_crtc->rotate_mem = NULL; } + pI830->shadow_present = FALSE; } #if RANDR_13_INTERFACE diff --git a/src/i830_dri.c b/src/i830_dri.c index f03be43..00458b7 100644 --- a/src/i830_dri.c +++ b/src/i830_dri.c @@ -70,6 +70,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE. #include <errno.h> #include <unistd.h> #include <fcntl.h> +#include <sys/time.h> +#include <time.h> #include "xf86.h" #include "xf86_OSproc.h" @@ -1537,18 +1539,13 @@ I830DRI2CreateBuffers(DrawablePtr pDraw, unsigned int *attachments, int count) I830Ptr pI830 = I830PTR(pScrn); DRI2BufferPtr buffers; dri_bo *bo; - int i; - I830DRI2BufferPrivatePtr privates; + int i, j; + I830DRI2BufferPrivatePtr private; PixmapPtr pPixmap, pDepthPixmap; buffers = xcalloc(count, sizeof *buffers); if (buffers == NULL) return NULL; - privates = xcalloc(count, sizeof *privates); - if (privates == NULL) { - xfree(buffers); - return NULL; - } pDepthPixmap = NULL; for (i = 0; i < count; i++) { @@ -1597,12 +1594,21 @@ I830DRI2CreateBuffers(DrawablePtr pDraw, unsigned int *attachments, int count) if (attachments[i] == DRI2BufferDepth) pDepthPixmap = pPixmap; + private = xcalloc(1, sizeof *private); + if (!private) { + for (j = 0; j < i; j++) + xfree(buffers[j].driverPrivate); + xfree(buffers); + return NULL; + } + + buffers[i].attachment = attachments[i]; buffers[i].pitch = pPixmap->devKind; buffers[i].cpp = pPixmap->drawable.bitsPerPixel / 8; - buffers[i].driverPrivate = &privates[i]; + buffers[i].driverPrivate = private; buffers[i].flags = 0; /* not tiled */ - privates[i].pPixmap = pPixmap; + private->pPixmap = pPixmap; bo = i830_get_pixmap_bo (pPixmap); if (dri_bo_flink(bo, &buffers[i].name) != 0) { @@ -1625,11 +1631,11 @@ I830DRI2DestroyBuffers(DrawablePtr pDraw, DRI2BufferPtr buffers, int count) { private = buffers[i].driverPrivate; (*pScreen->DestroyPixmap)(private->pPixmap); + xfree(buffers[i].driverPrivate); + buffers[i].driverPrivate = NULL; } - if (buffers) - { - xfree(buffers[0].driverPrivate); + if (buffers) { xfree(buffers); } } @@ -1672,6 +1678,168 @@ I830DRI2CopyRegion(DrawablePtr pDraw, RegionPtr pRegion, } +/* + * At flip time we need to: + * - update X screen pixmap with the new front buffer info + * - update new back buffer info with old front buffer info + * - queue the flip + * - queue a wait so we don't clobber rendering + * - return the new front & back buffer info + */ +static Bool +i830_do_pageflip(DrawablePtr pDraw, DRI2BufferPtr front, DRI2BufferPtr back) +{ + ScreenPtr pScreen = pDraw->pScreen; + ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum]; + I830Ptr pI830 = I830PTR(pScrn); + int tmp; + I830DRI2BufferPrivatePtr front_priv, back_priv; + dri_bo *front_bo, *back_bo; + struct drm_i915_gem_page_flip flip; + int ret; + + front_priv = front->driverPrivate; + back_priv = back->driverPrivate; + front_bo = i830_get_pixmap_bo(front_priv->pPixmap); + back_bo = i830_get_pixmap_bo(back_priv->pPixmap); + + tmp = front->name; + front->name = back->name; + back->name = tmp; + i830_set_pixmap_bo(back_priv->pPixmap, front_bo); + + pScrn->fbOffset = back_bo->offset; + /* If we're in charge of the front buffer, we can flip */ + if (!pI830->shadow_present) { + flip.handle = back_bo->handle; + flip.pipe = 1; + flip.x = pScrn->virtualX; + flip.y = pScrn->virtualY; + flip.flags = 0; flip.offset = 0; + + ret = drmCommandWrite(pI830->drmSubFD, DRM_I915_GEM_PAGE_FLIP, &flip, + sizeof(flip)); + if (ret) { + xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "Page flip failed: %s\n", + strerror(errno)); + return FALSE; + } + } + + i830_set_pixmap_bo(pScreen->GetScreenPixmap(pScreen), back_bo); + + return TRUE; +} + +/* Check various flip constraints (drawable parameters vs screen params) */ +static Bool +i830_flip_ok(DrawablePtr pDraw) +{ + ScreenPtr pScreen = pDraw->pScreen; + ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum]; + + if (pDraw->width != pScrn->virtualX) + return FALSE; + if (pDraw->height != pScrn->virtualY) + return FALSE; + if (pDraw->depth != pScrn->depth) + return FALSE; + + return TRUE; +} + +/* + * DRI2SwapBuffers should try to do a buffer swap if possible, however: + * - if we're swapping buffers smaller than the screen, we have to blit + * - if the back buffer doesn't match the screen depth, we have to blit + * - otherwise we try to swap, and return to the caller the new front + * and back buffers + */ +static DRI2BufferPtr +I830DRI2SwapBuffers(DrawablePtr pDraw, DRI2BufferPtr buffers, int count) +{ + ScreenPtr pScreen = pDraw->pScreen; + ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum]; + BoxRec box; + RegionRec region; + DRI2BufferPtr new_buffers, back = NULL, front = NULL; + I830DRI2BufferPrivatePtr private, old_priv; + PixmapPtr pPixmap, pDepthPixmap = NULL; + int i, j; + dri_bo *bo; + + new_buffers = xcalloc(count, sizeof *buffers); + if (new_buffers == NULL) + return NULL; + + for (i = 0; i < count; i++) { + old_priv = buffers[i].driverPrivate; + + if (buffers[i].attachment == DRI2BufferFrontLeft) { + if (pDraw->type == DRAWABLE_PIXMAP) + pPixmap = (PixmapPtr) pDraw; + else + pPixmap = (*pScreen->GetWindowPixmap)((WindowPtr)pDraw); + pPixmap->refcnt++; + } else if (buffers[i].attachment == DRI2BufferStencil && pDepthPixmap) { + pPixmap = pDepthPixmap; + pPixmap->refcnt++; + } else { + pPixmap = (*pScreen->CreatePixmap)(pScreen, 0, 0, pDraw->depth, 0); + (*pScreen->ModifyPixmapHeader)(pPixmap, pDraw->width, pDraw->height, + 0, 0, buffers[i].pitch, NULL); + } + + if (buffers[i].attachment == DRI2BufferDepth) + pDepthPixmap = pPixmap; + + private = xcalloc(1, sizeof *private); + if (!private) { + for (j = 0; j < i; j++) + xfree(new_buffers[j].driverPrivate); + xfree(new_buffers); + return NULL; + } + + new_buffers[i].attachment = buffers[i].attachment; + new_buffers[i].pitch = buffers[i].pitch; + new_buffers[i].cpp = buffers[i].cpp; + new_buffers[i].driverPrivate = private; + new_buffers[i].name = buffers[i].name; + new_buffers[i].flags = 0; /* not tiled */ + private->pPixmap = pPixmap; + + bo = i830_get_pixmap_bo(old_priv->pPixmap); + dri_bo_reference(bo); + i830_set_pixmap_bo(pPixmap, bo); + + if (buffers[i].attachment == DRI2BufferFrontLeft) + front = &new_buffers[i]; + if (buffers[i].attachment == DRI2BufferBackLeft) + back = &new_buffers[i]; + } + + if (i830_flip_ok(pDraw)) { + /* Page flip the full screen buffer */ + I830Sync(pScrn); + if (i830_do_pageflip(pDraw, front, back)) + goto out; + } + + /* Fall back to a blit */ + box.x1 = 0; + box.y1 = 0; + box.x2 = pDraw->width; + box.y2 = pDraw->height; + REGION_INIT(pScreen, ®ion, &box, 0); + + I830DRI2CopyRegion(pDraw, ®ion, front, back); + REGION_UNINIT(pScreen, ®ion); + +out: + return new_buffers; +} + Bool I830DRI2ScreenInit(ScreenPtr pScreen) { ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum]; @@ -1742,6 +1910,7 @@ Bool I830DRI2ScreenInit(ScreenPtr pScreen) info.CreateBuffers = I830DRI2CreateBuffers; info.DestroyBuffers = I830DRI2DestroyBuffers; info.CopyRegion = I830DRI2CopyRegion; + info.SwapBuffers = I830DRI2SwapBuffers; pI830->drmSubFD = info.fd; diff --git a/src/i830_exa.c b/src/i830_exa.c index ebc6624..972d1f1 100644 --- a/src/i830_exa.c +++ b/src/i830_exa.c @@ -921,7 +921,7 @@ i830_uxa_destroy_pixmap (PixmapPtr pixmap) dri_bo *bo = i830_get_pixmap_bo (pixmap); if (bo) - dri_bo_unreference (bo); + ;//dri_bo_unreference (bo); } fbDestroyPixmap (pixmap); return TRUE; |
From: Jesse B. <jb...@vi...> - 2009-02-27 01:08:44
|
On Thursday, February 26, 2009 1:36:26 pm Jesse Barnes wrote: > Support the new swapbuffers request using the new page flipping ioctl > if possible. > > This patch still needs some work; there's a bug in the no-flip case that > causes us to lose track of pixmaps, and the pipe is still hardcoded to 1, > but that should be easy to fix. > > The code is pretty ugly too; it seems like getbuffers and swapbuffers could > probably share more code, but we need to copy all the buffers in > swapbuffers to return them... This version is a little better, and with the X server fix I just posted it no longer crashes with current compiz. There are two open issues left: 1) how to deal with pinning the new front & unpinning the old back 2) figure out why mixed SwapBuffers/CopyRegion code (e.g. current compiz) render right; the front buffer doesn't get any rendering Problem (2) is probably something simple I'm missing in the flip code, but (1) is a little trickier. We need to keep the new front buffer pinned until the next swap, but the old buffer has to be kept pinned until the swap completes; one ugly option would be to pass both buffers into the flip ioctl and let the kernel take care of it. Any opinions? Thanks, -- Jesse Barnes, Intel Open Source Technology Center i830.h | 1 i830_display.c | 5 + i830_dri.c | 188 +++++++++++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 182 insertions(+), 12 deletions(-) diff --git a/src/i830.h b/src/i830.h index 7904b9f..5c65ce8 100644 --- a/src/i830.h +++ b/src/i830.h @@ -452,6 +452,7 @@ typedef struct _I830Rec { #endif XF86ModReqInfo shadowReq; /* to test for later libshadow */ + Bool shadow_present; Rotation rotation; void (*PointerMoved)(int, int, int); CreateScreenResourcesProcPtr CreateScreenResources; diff --git a/src/i830_display.c b/src/i830_display.c index 8a5cf24..692349e 100644 --- a/src/i830_display.c +++ b/src/i830_display.c @@ -1669,6 +1669,9 @@ i830_crtc_shadow_create(xf86CrtcPtr crtc, void *data, int width, int height) } if (intel_crtc->rotate_mem && intel_crtc->rotate_mem->bo) i830_set_pixmap_bo(rotate_pixmap, intel_crtc->rotate_mem->bo); + + pI830->shadow_present = TRUE; + return rotate_pixmap; } @@ -1677,6 +1680,7 @@ i830_crtc_shadow_destroy(xf86CrtcPtr crtc, PixmapPtr rotate_pixmap, void *data) { ScrnInfoPtr pScrn = crtc->scrn; I830CrtcPrivatePtr intel_crtc = crtc->driver_private; + I830Ptr pI830 = I830PTR(pScrn); if (rotate_pixmap) FreeScratchPixmapHeader(rotate_pixmap); @@ -1687,6 +1691,7 @@ i830_crtc_shadow_destroy(xf86CrtcPtr crtc, PixmapPtr rotate_pixmap, void *data) i830_free_memory(pScrn, intel_crtc->rotate_mem); intel_crtc->rotate_mem = NULL; } + pI830->shadow_present = FALSE; } #if RANDR_13_INTERFACE diff --git a/src/i830_dri.c b/src/i830_dri.c index f03be43..1c25b81 100644 --- a/src/i830_dri.c +++ b/src/i830_dri.c @@ -70,6 +70,8 @@ USE OR OTHER DEALINGS IN THE SOFTWARE. #include <errno.h> #include <unistd.h> #include <fcntl.h> +#include <sys/time.h> +#include <time.h> #include "xf86.h" #include "xf86_OSproc.h" @@ -1537,18 +1539,13 @@ I830DRI2CreateBuffers(DrawablePtr pDraw, unsigned int *attachments, int count) I830Ptr pI830 = I830PTR(pScrn); DRI2BufferPtr buffers; dri_bo *bo; - int i; - I830DRI2BufferPrivatePtr privates; + int i, j; + I830DRI2BufferPrivatePtr private; PixmapPtr pPixmap, pDepthPixmap; buffers = xcalloc(count, sizeof *buffers); if (buffers == NULL) return NULL; - privates = xcalloc(count, sizeof *privates); - if (privates == NULL) { - xfree(buffers); - return NULL; - } pDepthPixmap = NULL; for (i = 0; i < count; i++) { @@ -1597,12 +1594,21 @@ I830DRI2CreateBuffers(DrawablePtr pDraw, unsigned int *attachments, int count) if (attachments[i] == DRI2BufferDepth) pDepthPixmap = pPixmap; + private = xcalloc(1, sizeof *private); + if (!private) { + for (j = 0; j < i; j++) + xfree(buffers[j].driverPrivate); + xfree(buffers); + return NULL; + } + + buffers[i].attachment = attachments[i]; buffers[i].pitch = pPixmap->devKind; buffers[i].cpp = pPixmap->drawable.bitsPerPixel / 8; - buffers[i].driverPrivate = &privates[i]; + buffers[i].driverPrivate = private; buffers[i].flags = 0; /* not tiled */ - privates[i].pPixmap = pPixmap; + private->pPixmap = pPixmap; bo = i830_get_pixmap_bo (pPixmap); if (dri_bo_flink(bo, &buffers[i].name) != 0) { @@ -1625,11 +1631,11 @@ I830DRI2DestroyBuffers(DrawablePtr pDraw, DRI2BufferPtr buffers, int count) { private = buffers[i].driverPrivate; (*pScreen->DestroyPixmap)(private->pPixmap); + xfree(buffers[i].driverPrivate); + buffers[i].driverPrivate = NULL; } - if (buffers) - { - xfree(buffers[0].driverPrivate); + if (buffers) { xfree(buffers); } } @@ -1672,6 +1678,163 @@ I830DRI2CopyRegion(DrawablePtr pDraw, RegionPtr pRegion, } +/* + * At flip time we need to: + * - update X screen pixmap with the new front buffer info + * - update new back buffer info with old front buffer info + * - queue the flip + * - queue a wait so we don't clobber rendering + * - return the new front & back buffer info + */ +static Bool +i830_do_pageflip(DrawablePtr pDraw, DRI2BufferPtr front, DRI2BufferPtr back) +{ + ScreenPtr pScreen = pDraw->pScreen; + ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum]; + I830Ptr pI830 = I830PTR(pScrn); + I830DRI2BufferPrivatePtr front_priv, back_priv; + dri_bo *front_bo, *back_bo; + struct drm_i915_gem_page_flip flip; + int ret, tmp_name; + + front_priv = front->driverPrivate; + back_priv = back->driverPrivate; + + front_bo = i830_get_pixmap_bo(front_priv->pPixmap); + back_bo = i830_get_pixmap_bo(back_priv->pPixmap); + + i830_set_pixmap_bo(front_priv->pPixmap, back_bo); + i830_set_pixmap_bo(back_priv->pPixmap, front_bo); + + tmp_name = front->name; + front->name = back->name; + back->name = tmp_name; + + dri_bo_pin(back_bo, 0); + + pScrn->fbOffset = back_bo->offset; + /* If we're in charge of the front buffer, we can flip */ + if (!pI830->shadow_present) { + flip.handle = back_bo->handle; + flip.pipe = 1; + flip.x = pScrn->virtualX; + flip.y = pScrn->virtualY; + flip.flags = 0; + flip.offset = 0; + + ret = drmCommandWrite(pI830->drmSubFD, DRM_I915_GEM_PAGE_FLIP, &flip, + sizeof(flip)); + if (ret) { + xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "Page flip failed: %s\n", + strerror(errno)); + return FALSE; + } + } + + return TRUE; +} + +/* Check various flip constraints (drawable parameters vs screen params) */ +static Bool +i830_flip_ok(DrawablePtr pDraw) +{ + ScreenPtr pScreen = pDraw->pScreen; + ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum]; + + if (pDraw->width != pScrn->virtualX) + return FALSE; + if (pDraw->height != pScrn->virtualY) + return FALSE; + if (pDraw->depth != pScrn->depth) + return FALSE; + + return TRUE; +} + +/* + * DRI2SwapBuffers should try to do a buffer swap if possible, however: + * - if we're swapping buffers smaller than the screen, we have to blit + * - if the back buffer doesn't match the screen depth, we have to blit + * - otherwise we try to swap, and return to the caller the new front + * and back buffers + */ +static DRI2BufferPtr +I830DRI2SwapBuffers(DrawablePtr pDraw, DRI2BufferPtr buffers, int count) +{ + ScreenPtr pScreen = pDraw->pScreen; + ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum]; + DRI2BufferPtr new_buffers; + I830DRI2BufferPrivatePtr private, old_priv; + PixmapPtr pPixmap, pDepthPixmap = NULL; + int i, j, front = 0, back = 1; + dri_bo *bo; + + if (!i830_flip_ok(pDraw)) + return NULL; + + new_buffers = xcalloc(count, sizeof *buffers); + if (new_buffers == NULL) + return NULL; + + for (i = 0; i < count; i++) { + old_priv = buffers[i].driverPrivate; + + if (buffers[i].attachment == DRI2BufferFrontLeft) { + if (pDraw->type == DRAWABLE_PIXMAP) + pPixmap = (PixmapPtr) pDraw; + else + pPixmap = (*pScreen->GetWindowPixmap)((WindowPtr)pDraw); + pPixmap->refcnt++; + } else if (buffers[i].attachment == DRI2BufferStencil && pDepthPixmap) { + pPixmap = pDepthPixmap; + pPixmap->refcnt++; + } else { + pPixmap = (*pScreen->CreatePixmap)(pScreen, 0, 0, pDraw->depth, 0); + (*pScreen->ModifyPixmapHeader)(pPixmap, pDraw->width, pDraw->height, + 0, 0, buffers[i].pitch, NULL); + } + + if (buffers[i].attachment == DRI2BufferDepth) + pDepthPixmap = pPixmap; + + private = xcalloc(1, sizeof *private); + if (!private) { + for (j = 0; j < i; j++) + xfree(new_buffers[j].driverPrivate); + xfree(new_buffers); + return NULL; + } + + new_buffers[i].attachment = buffers[i].attachment; + new_buffers[i].pitch = buffers[i].pitch; + new_buffers[i].cpp = buffers[i].cpp; + new_buffers[i].driverPrivate = private; + new_buffers[i].name = buffers[i].name; + new_buffers[i].flags = 0; /* not tiled */ + private->pPixmap = pPixmap; + + bo = i830_get_pixmap_bo(old_priv->pPixmap); + dri_bo_reference(bo); + i830_set_pixmap_bo(pPixmap, bo); + + if (buffers[i].attachment == DRI2BufferFrontLeft) + front = i; + if (buffers[i].attachment == DRI2BufferBackLeft) + back = i; + } + + /* Page flip the full screen buffer */ + I830Sync(pScrn); + if (!i830_do_pageflip(pDraw, &new_buffers[front], &new_buffers[back])) { + for (i = 0; i < count; i++) + xfree(new_buffers[i].driverPrivate); + xfree(new_buffers); + return NULL; + } + + return new_buffers; +} + Bool I830DRI2ScreenInit(ScreenPtr pScreen) { ScrnInfoPtr pScrn = xf86Screens[pScreen->myNum]; @@ -1742,6 +1905,7 @@ Bool I830DRI2ScreenInit(ScreenPtr pScreen) info.CreateBuffers = I830DRI2CreateBuffers; info.DestroyBuffers = I830DRI2DestroyBuffers; info.CopyRegion = I830DRI2CopyRegion; + info.SwapBuffers = I830DRI2SwapBuffers; pI830->drmSubFD = info.fd; |
From: Eric A. <er...@an...> - 2009-03-07 00:24:54
|
On Thu, 2009-02-26 at 13:36 -0800, Jesse Barnes wrote: > Support the new swapbuffers request using the new page flipping ioctl > if possible. > > This patch still needs some work; there's a bug in the no-flip case that > causes us to lose track of pixmaps, and the pipe is still hardcoded to 1, > but that should be easy to fix. > > The code is pretty ugly too; it seems like getbuffers and swapbuffers could > probably share more code, but we need to copy all the buffers in swapbuffers > to return them... I haven't reviewed the thing, but a comment from other discussion: ... > + /* If we're in charge of the front buffer, we can flip */ > + if (!pI830->shadow_present) { > + flip.handle = back_bo->handle; > + flip.pipe = 1; > + flip.x = pScrn->virtualX; > + flip.y = pScrn->virtualY; > + flip.flags = 0; flip.offset = 0; > + > + ret = drmCommandWrite(pI830->drmSubFD, DRM_I915_GEM_PAGE_FLIP, &flip, > + sizeof(flip)); > + if (ret) { > + xf86DrvMsg(pScrn->scrnIndex, X_WARNING, "Page flip failed: %s\n", > + strerror(errno)); > + return FALSE; > + } I think this should be in libdrm, so that the bufmgr can know that the BO is getting a outside-of-this-client reference to it and avoid putting it in the BO cache if it gets unreferenced while still being scanned out. -- Eric Anholt er...@an... eri...@in... |
From: Chris W. <ch...@ch...> - 2009-02-26 21:57:12
|
On Thu, 2009-02-26 at 13:28 -0800, Jesse Barnes wrote: > Add a new page flip ioctl to the i915 driver. The new ioctl takes the new > drm_i915_gem_page_flip arg, which contains a buffer object target for the > flip, an offset into that buffer, and other buffer parameters to be used for > the flip. > > If a flip is already pending on the object in question, the ioctl waits for > it to finish first, then queues the flip. An optional wait flag causes the > ioctl to wait for the flip to complete before the it returns. > > If an execbuffer containing an object with a pending flip comes in, it will > stall until the flip completes, to preserve glXSwapBuffers semantics. > > Signed-off-by: Jesse Barnes <jb...@vi...> Didn't do too bad in spotting the missing checks... ;-) > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index 2d797ff..0d6a6d3 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -829,6 +829,175 @@ static int i915_set_status_page(struct drm_device *dev, void *data, > return 0; > } > > +static int i915_pipe_to_plane(struct drm_device *dev, int pipe) > +{ > + drm_i915_private_t *dev_priv = dev->dev_private; > + u32 reg, pipe_mask; > + > + if (pipe == 0) > + pipe_mask = DISPPLANE_SEL_PIPE_A; > + else > + pipe_mask = DISPPLANE_SEL_PIPE_B; > + > + pipe_mask |= DISPLAY_PLANE_ENABLE; > + > + reg = I915_READ(DSPACNTR); > + if ((reg & (DISPLAY_PLANE_ENABLE | DISPPLANE_SEL_PIPE_MASK)) == > + pipe_mask) > + return 0; > + > + reg = I915_READ(DSPBCNTR); > + if ((reg & (DISPLAY_PLANE_ENABLE | DISPPLANE_SEL_PIPE_MASK)) == > + pipe_mask) > + return 1; > + > + return -1; > +} > + > +bool > +i915_gem_flip_pending(struct drm_gem_object *obj) > +{ > + struct drm_device *dev = obj->dev; > + drm_i915_private_t *dev_priv = dev->dev_private; > + struct drm_i915_gem_object *obj_priv = obj->driver_private; > + unsigned long flags; > + bool pending = false; > + > + spin_lock_irqsave(&dev_priv->vblank_lock, flags); > + if (!list_empty(&obj_priv->vblank_head)) > + pending = true; This has annoyed me every time (but I'm running out of things I can complain about ;-), can we just say pending = !list_empty(); and be done with the conditional. > + spin_unlock_irqrestore(&dev_priv->vblank_lock, flags); > + > + return pending; > +} > + > +static int i915_gem_page_flip(struct drm_device *dev, void *data, > + struct drm_file *file_priv) > +{ > + struct drm_i915_gem_page_flip *flip_data = data; > + drm_i915_private_t *dev_priv = dev->dev_private; > + struct drm_gem_object *obj; > + struct drm_i915_gem_object *obj_priv; > + unsigned long flags; > + uint32_t offset; > + unsigned int pitch, pipe, plane, tiled; > + int ret = 0, vblank_ref_err = 0, reqno; > + RING_LOCALS; > + > + if (!(dev->driver->driver_features & DRIVER_GEM)) > + return -ENODEV; Guess I'll just have to comment this out to continue testing this feature. :-) > + > + /* > + * Reject unknown flags so future userspace knows what we (don't) > + * support > + */ > + if (flip_data->flags & (~I915_PAGE_FLIP_WAIT)) { > + DRM_ERROR("bad page flip flags\n"); > + return -EINVAL; > + } > + > + if ((pipe = flip_data->pipe) > 1) { > + DRM_ERROR("bad pipe\n"); > + return -EINVAL; > + } > + > + plane = i915_pipe_to_plane(dev, pipe); > + if (plane < 0) { > + DRM_ERROR("bad pipe (no planes enabled?)\n"); > + return -EINVAL; > + } > + > + obj = drm_gem_object_lookup(dev, file_priv, flip_data->handle); > + if (obj == NULL) > + return -EBADF; > + > + /* > + * Make sure the new buffer is in bounds > + * FIXME: should probably check against current mode as well > + */ > + if (flip_data->offset >= obj->size) { > + DRM_ERROR("bad flip offset\n"); > + ret = -EINVAL; > + goto out_unref_prelock; > + } > + > + obj_priv = obj->driver_private; Need to check obj_priv->stride&7 == 0. Hmm, that may be impossible anyway. > + > + if (i915_gem_flip_pending(obj)) > + wait_for_completion(&obj_priv->vblank); > + > + mutex_lock(&dev->struct_mutex); > + if (obj_priv->tiling_mode != I915_TILING_NONE && > + obj_priv->tiling_mode != I915_TILING_X) { > + DRM_ERROR("can only flip non-tiled or X tiled pages\n"); > + ret = -EINVAL; > + goto out_unref; > + } > + > + vblank_ref_err = drm_vblank_get(dev, pipe); > + if (!vblank_ref_err) { Ok, this seems like a reasonable fallback. If vblank is not enabled then you just queue the flip without waiting. > + ret = i915_gem_object_pin(obj, 0); > + if (ret) { > + DRM_ERROR("failed to pin object for flip\n"); > + ret = -EBUSY; What's the rationale for changing the reported error here? (And it might be helpful to printk the original error value.) > + goto out_unref; > + } > + } > + > + /* > + * Put the object in the GTT domain before the flip, > + * since there may be outstanding rendering > + */ > + i915_gem_object_set_to_gtt_domain(obj, 0); Need to check, report and handle errors here. And yes, these do occur in the wild for some as of yet unknown reason. > + > + offset = obj_priv->gtt_offset + flip_data->offset; > + > + pitch = obj_priv->stride; > + tiled = (obj_priv->tiling_mode == I915_TILING_X); > + > + BEGIN_LP_RING(4); > + OUT_RING(CMD_OP_DISPLAYBUFFER_INFO | (plane << 20)); > + OUT_RING(pitch); > + if (IS_I965G(dev)) { > + OUT_RING(offset | tiled); > + OUT_RING(((flip_data->x - 1) << 16) | (flip_data->y - 1)); > + } else { > + OUT_RING(offset); > + OUT_RING(MI_NOOP); > + } > + ADVANCE_LP_RING(); > + > + reqno = i915_add_request(dev, 0); Be consistent and call this seqno like the rest of the code, please. > + > + mutex_unlock(&dev->struct_mutex); > + > + /* > + * This is a bit racy; the flip above may have already happened > + * by the time we get here. If that happens, the new back buffer > + * won't be available for rendering for one extra frame, since > + * the vblank list won't have the object. > + */ > + spin_lock_irqsave(&dev_priv->vblank_lock, flags); > + if (!vblank_ref_err) > + list_add_tail(&obj_priv->vblank_head, > + &dev_priv->mm.vblank_list[pipe]); > + > + obj_priv->flip_seqno = reqno; > + spin_unlock_irqrestore(&dev_priv->vblank_lock, flags); > + > + if (!vblank_ref_err && (flip_data->flags & I915_PAGE_FLIP_WAIT)) > + wait_for_completion(&obj_priv->vblank); > + > +out_unref_prelock: > + /* Take lock again for unref */ > + mutex_lock(&dev->struct_mutex); > +out_unref: > + drm_gem_object_unreference(obj); > + mutex_unlock(&dev->struct_mutex); > + > + return ret; > +} > + > /** > * i915_probe_agp - get AGP bootup configuration > * @pdev: PCI device > @@ -1307,6 +1476,7 @@ struct drm_ioctl_desc i915_ioctls[] = { > DRM_IOCTL_DEF(DRM_I915_GEM_SET_TILING, i915_gem_set_tiling, 0), > DRM_IOCTL_DEF(DRM_I915_GEM_GET_TILING, i915_gem_get_tiling, 0), > DRM_IOCTL_DEF(DRM_I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, 0), > + DRM_IOCTL_DEF(DRM_I915_GEM_PAGE_FLIP, i915_gem_page_flip, DRM_AUTH|DRM_MASTER), > }; > > int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 3951a12..148e80a 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -159,6 +159,10 @@ typedef struct drm_i915_private { > u32 irq_mask_reg; > u32 pipestat[2]; > > + /** Protects vblank list */ > + spinlock_t vblank_lock; > + struct work_struct vblank_work; > + > int tex_lru_log_granularity; > int allow_batchbuffer; > struct mem_block *agp_heap; > @@ -338,6 +342,11 @@ typedef struct drm_i915_private { > */ > struct delayed_work retire_work; > > + /** > + * List of objects waiting on vblank events (one per pipe) > + */ > + struct list_head vblank_list[2]; > + > uint32_t next_gem_seqno; > > /** > @@ -389,6 +398,13 @@ struct drm_i915_gem_object { > /** This object's place on the active/flushing/inactive lists */ > struct list_head list; > > + /** Object's place on the vblank list (protected by vblank_lock)*/ > + struct list_head vblank_head; > + /** sequence number for flip (when it passes the flip is done), > + * protected by vblank lock > + */ > + int flip_seqno; > + > /** > * This is set if the object is on the active or flushing lists > * (has pending rendering), and is not set if it's on inactive (ready > @@ -457,6 +473,9 @@ struct drm_i915_gem_object { > > /** for phy allocated objects */ > struct drm_i915_gem_phys_object *phys_obj; > + > + /** for page flips and other vblank related blocks */ > + struct completion vblank; > }; > > /** > @@ -516,6 +535,7 @@ extern long i915_compat_ioctl(struct file *filp, unsigned int cmd, > extern int i915_emit_box(struct drm_device *dev, > struct drm_clip_rect __user *boxes, > int i, int DR1, int DR4); > +extern bool i915_gem_flip_pending(struct drm_gem_object *obj); > > /* i915_irq.c */ > extern int i915_irq_emit(struct drm_device *dev, void *data, > @@ -625,6 +645,9 @@ int i915_gem_attach_phys_object(struct drm_device *dev, > void i915_gem_detach_phys_object(struct drm_device *dev, > struct drm_gem_object *obj); > void i915_gem_free_all_phys_object(struct drm_device *dev); > +uint32_t i915_add_request(struct drm_device *dev, uint32_t flush_domains); > +int i915_seqno_passed(uint32_t seq1, uint32_t seq2); > +uint32_t i915_get_gem_seqno(struct drm_device *dev); > > /* i915_gem_tiling.c */ > void i915_gem_detect_bit_6_swizzle(struct drm_device *dev); > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 3e025d5..5270d25 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -900,7 +900,7 @@ i915_gem_object_move_to_inactive(struct drm_gem_object *obj) > * > * Returned sequence numbers are nonzero on success. > */ > -static uint32_t > +uint32_t > i915_add_request(struct drm_device *dev, uint32_t flush_domains) > { > drm_i915_private_t *dev_priv = dev->dev_private; > @@ -1028,7 +1028,7 @@ i915_gem_retire_request(struct drm_device *dev, > /** > * Returns true if seq1 is later than seq2. > */ > -static int > +int > i915_seqno_passed(uint32_t seq1, uint32_t seq2) > { > return (int32_t)(seq1 - seq2) >= 0; > @@ -2498,6 +2498,25 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, > goto pre_mutex_err; > } > > + /* Look up object handles */ > + for (i = 0; i < args->buffer_count; i++) { > + object_list[i] = drm_gem_object_lookup(dev, file_priv, > + exec_list[i].handle); > + if (object_list[i] == NULL) { > + DRM_ERROR("Invalid object handle %d at index %d\n", > + exec_list[i].handle, i); > + ret = -EBADF; > + goto pre_mutex_err; Hmm, can't jump straight to pre_mutex_err as that means you leak the references on object already looked up. Take the mutex and goto err; > + } > + > + if (i915_gem_flip_pending(object_list[i])) { > + struct drm_i915_gem_object *obj_priv; > + > + obj_priv = object_list[i]->driver_private; > + wait_for_completion(&obj_priv->vblank); > + } > + } > + > mutex_lock(&dev->struct_mutex); > > i915_verify_inactive(dev, __FILE__, __LINE__); > @@ -2516,18 +2535,6 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, > goto pre_mutex_err; > } > > - /* Look up object handles */ > - for (i = 0; i < args->buffer_count; i++) { > - object_list[i] = drm_gem_object_lookup(dev, file_priv, > - exec_list[i].handle); > - if (object_list[i] == NULL) { > - DRM_ERROR("Invalid object handle %d at index %d\n", > - exec_list[i].handle, i); > - ret = -EBADF; > - goto err; > - } > - } > - > /* Pin and relocate */ > for (pin_tries = 0; ; pin_tries++) { > ret = 0; > @@ -2920,6 +2927,9 @@ int i915_gem_init_object(struct drm_gem_object *obj) > obj_priv->obj = obj; > obj_priv->fence_reg = I915_FENCE_REG_NONE; > INIT_LIST_HEAD(&obj_priv->list); > + INIT_LIST_HEAD(&obj_priv->vblank_head); > + > + init_completion(&obj_priv->vblank); > > return 0; > } > @@ -2980,8 +2990,10 @@ int > i915_gem_idle(struct drm_device *dev) > { > drm_i915_private_t *dev_priv = dev->dev_private; > + struct drm_i915_gem_object *obj_priv, *tmp; > + unsigned long irqflags; > uint32_t seqno, cur_seqno, last_seqno; > - int stuck, ret; > + int stuck, ret, pipe; > > mutex_lock(&dev->struct_mutex); > > @@ -2995,10 +3007,25 @@ i915_gem_idle(struct drm_device *dev) > */ > dev_priv->mm.suspended = 1; > > + mutex_unlock(&dev->struct_mutex); > + > + /* Wait for any outstanding flips */ > + spin_lock_irqsave(&dev_priv->vblank_lock, irqflags); > + for (pipe = 0; pipe < 2; pipe++) { > + list_for_each_entry_safe(obj_priv, tmp, > + &dev_priv->mm.vblank_list[pipe], > + vblank_head) { > + spin_unlock_irqrestore(&dev_priv->vblank_lock, irqflags); > + wait_for_completion(&obj_priv->vblank); > + spin_lock_irqsave(&dev_priv->vblank_lock, irqflags); > + } > + } > + spin_unlock_irqrestore(&dev_priv->vblank_lock, irqflags); > + > /* Cancel the retire work handler, wait for it to finish if running > */ > - mutex_unlock(&dev->struct_mutex); > cancel_delayed_work_sync(&dev_priv->mm.retire_work); > + > mutex_lock(&dev->struct_mutex); > > i915_kernel_lost_context(dev); > @@ -3358,9 +3385,12 @@ i915_gem_load(struct drm_device *dev) > INIT_LIST_HEAD(&dev_priv->mm.flushing_list); > INIT_LIST_HEAD(&dev_priv->mm.inactive_list); > INIT_LIST_HEAD(&dev_priv->mm.request_list); > + INIT_LIST_HEAD(&dev_priv->mm.vblank_list[0]); > + INIT_LIST_HEAD(&dev_priv->mm.vblank_list[1]); > INIT_DELAYED_WORK(&dev_priv->mm.retire_work, > i915_gem_retire_work_handler); > dev_priv->mm.next_gem_seqno = 1; > + spin_lock_init(&dev_priv->vblank_lock); > > /* Old X drivers will take 0-2 for front, back, depth buffers */ > dev_priv->fence_reg_start = 3; > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 548ff2c..f50df88 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -187,6 +187,36 @@ u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe) > return I915_READ(reg); > } > > +static void i915_vblank_work_func(struct work_struct *work) > +{ > + drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t, > + vblank_work); > + struct drm_device *dev = dev_priv->dev; > + struct drm_i915_gem_object *obj_priv, *tmp; > + unsigned long irqflags; > + int pipe, cur_seqno; > + > + mutex_lock(&dev->struct_mutex); > + > + spin_lock_irqsave(&dev_priv->vblank_lock, irqflags); > + for (pipe = 0; pipe < 2; pipe++) { > + list_for_each_entry_safe(obj_priv, tmp, > + &dev_priv->mm.vblank_list[pipe], > + vblank_head) { > + cur_seqno = i915_get_gem_seqno(dev); > + if (i915_seqno_passed(cur_seqno, > + obj_priv->flip_seqno)) { > + list_del_init(&obj_priv->vblank_head); > + drm_vblank_put(dev, pipe); > + i915_gem_object_unpin(obj_priv->obj); > + complete(&obj_priv->vblank); > + } > + } > + } > + spin_unlock_irqrestore(&dev_priv->vblank_lock, irqflags); > + mutex_unlock(&dev->struct_mutex); > +} > + > irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS) > { > struct drm_device *dev = (struct drm_device *) arg; > @@ -269,6 +299,9 @@ irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS) > drm_handle_vblank(dev, 1); > } > > + if (vblank) > + schedule_work(&dev_priv->vblank_work); > + > if ((pipeb_stats & I915_LEGACY_BLC_EVENT_STATUS) || > (iir & I915_ASLE_INTERRUPT)) > opregion_asle_intr(dev); > @@ -533,6 +566,7 @@ void i915_driver_irq_preinstall(struct drm_device * dev) > I915_WRITE(IMR, 0xffffffff); > I915_WRITE(IER, 0x0); > (void) I915_READ(IER); > + INIT_WORK(&dev_priv->vblank_work, i915_vblank_work_func); > } > > int i915_driver_irq_postinstall(struct drm_device *dev) > diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h > index 912cd52..1ac4ded 100644 > --- a/include/drm/i915_drm.h > +++ b/include/drm/i915_drm.h > @@ -184,6 +184,7 @@ typedef struct _drm_i915_sarea { > #define DRM_I915_GEM_GET_TILING 0x22 > #define DRM_I915_GEM_GET_APERTURE 0x23 > #define DRM_I915_GEM_MMAP_GTT 0x24 > +#define DRM_I915_GEM_PAGE_FLIP 0x25 > > #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) > #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) > @@ -219,6 +220,7 @@ typedef struct _drm_i915_sarea { > #define DRM_IOCTL_I915_GEM_SET_TILING DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_SET_TILING, struct > drm_i915_gem_set_tiling) > #define DRM_IOCTL_I915_GEM_GET_TILING DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_GET_TILING, struct > drm_i915_gem_get_tiling) > #define DRM_IOCTL_I915_GEM_GET_APERTURE DRM_IOR (DRM_COMMAND_BASE + DRM_I915_GEM_GET_APERTURE, struct > drm_i915_gem_get_aperture) > +#define DRM_IOCTL_I915_GEM_PAGE_FLIP DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PAGE_FLIP, struct > drm_i915_gem_page_flip) > > /* Allow drivers to submit batchbuffers directly to hardware, relying > * on the security mechanisms provided by hardware. > @@ -654,4 +656,30 @@ struct drm_i915_gem_get_aperture { > uint64_t aper_available_size; > }; > > +#define I915_PAGE_FLIP_WAIT (1<<0) /* block on page flip completion */ > + > +struct drm_i915_gem_page_flip { > + /** Handle of new front buffer */ > + uint32_t handle; > + > + /** Offset into the object to use */ > + uint64_t offset; > + > + /** > + * page flip flags (wait on flip only for now) > + */ > + uint32_t flags; > + > + /** > + * pipe to flip > + */ > + uint32_t pipe; > + > + /** > + * screen dimensions for flip > + */ > + uint32_t x; > + uint32_t y; > +}; > + > #endif /* _I915_DRM_H_ */ > > _______________________________________________ > Intel-gfx mailing list > Int...@li... > http://lists.freedesktop.org/mailman/listinfo/intel-gfx |
From: Jesse B. <jb...@vi...> - 2009-02-26 22:35:58
|
On Thursday, February 26, 2009 1:56:52 pm Chris Wilson wrote: > On Thu, 2009-02-26 at 13:28 -0800, Jesse Barnes wrote: > > Add a new page flip ioctl to the i915 driver. The new ioctl takes the > > new drm_i915_gem_page_flip arg, which contains a buffer object target for > > the flip, an offset into that buffer, and other buffer parameters to be > > used for the flip. > > > > If a flip is already pending on the object in question, the ioctl waits > > for it to finish first, then queues the flip. An optional wait flag > > causes the ioctl to wait for the flip to complete before the it returns. > > > > If an execbuffer containing an object with a pending flip comes in, it > > will stall until the flip completes, to preserve glXSwapBuffers > > semantics. > > > > Signed-off-by: Jesse Barnes <jb...@vi...> > > Didn't do too bad in spotting the missing checks... ;-) ... but I just had to leave a few nuggets for you. :) > This has annoyed me every time (but I'm running out of things I can > complain about ;-), can we just say > pending = !list_empty(); > and be done with the conditional. Sure, fixed. > > + obj_priv = obj->driver_private; > > Need to check obj_priv->stride&7 == 0. Hmm, that may be impossible > anyway. I've left that out, though I guess it wouldn't hurt. > > + vblank_ref_err = drm_vblank_get(dev, pipe); > > + if (!vblank_ref_err) { > > Ok, this seems like a reasonable fallback. If vblank is not enabled then > you just queue the flip without waiting. Right, but I also forgot to unref in the pin failure case, so I've fixed that. > > + ret = i915_gem_object_pin(obj, 0); > > + if (ret) { > > + DRM_ERROR("failed to pin object for flip\n"); > > + ret = -EBUSY; > > What's the rationale for changing the reported error here? (And it might > be helpful to printk the original error value.) Oh I'm not sure what's going on there; I've changed it to just return the pin failure. > > + * Put the object in the GTT domain before the flip, > > + * since there may be outstanding rendering > > + */ > > + i915_gem_object_set_to_gtt_domain(obj, 0); > > Need to check, report and handle errors here. And yes, these do occur in > the wild for some as of yet unknown reason. Fixed. Though Mesa is now flushing before calling the server so it should happen less often. > > + reqno = i915_add_request(dev, 0); > > Be consistent and call this seqno like the rest of the code, please. Sure, fixed. > > + /* Look up object handles */ > > + for (i = 0; i < args->buffer_count; i++) { > > + object_list[i] = drm_gem_object_lookup(dev, file_priv, > > + exec_list[i].handle); > > + if (object_list[i] == NULL) { > > + DRM_ERROR("Invalid object handle %d at index %d\n", > > + exec_list[i].handle, i); > > + ret = -EBADF; > > + goto pre_mutex_err; > > Hmm, can't jump straight to pre_mutex_err as that means you leak the > references on object already looked up. Take the mutex and goto err; Ah yeah, missed that, looks like 'i' will be right if we jump there too; fixed. -- Jesse Barnes, Intel Open Source Technology Center diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 2d797ff..050682a 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -829,6 +829,178 @@ static int i915_set_status_page(struct drm_device *dev, void *data, return 0; } +static int i915_pipe_to_plane(struct drm_device *dev, int pipe) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + u32 reg, pipe_mask; + + if (pipe == 0) + pipe_mask = DISPPLANE_SEL_PIPE_A; + else + pipe_mask = DISPPLANE_SEL_PIPE_B; + + pipe_mask |= DISPLAY_PLANE_ENABLE; + + reg = I915_READ(DSPACNTR); + if ((reg & (DISPLAY_PLANE_ENABLE | DISPPLANE_SEL_PIPE_MASK)) == + pipe_mask) + return 0; + + reg = I915_READ(DSPBCNTR); + if ((reg & (DISPLAY_PLANE_ENABLE | DISPPLANE_SEL_PIPE_MASK)) == + pipe_mask) + return 1; + + return -1; +} + +bool +i915_gem_flip_pending(struct drm_gem_object *obj) +{ + struct drm_device *dev = obj->dev; + drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_gem_object *obj_priv = obj->driver_private; + unsigned long flags; + bool pending; + + spin_lock_irqsave(&dev_priv->vblank_lock, flags); + pending = !list_empty(&obj_priv->vblank_head); + spin_unlock_irqrestore(&dev_priv->vblank_lock, flags); + + return pending; +} + +static int i915_gem_page_flip(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_i915_gem_page_flip *flip_data = data; + drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_gem_object *obj; + struct drm_i915_gem_object *obj_priv; + unsigned long flags; + uint32_t offset; + unsigned int pitch, pipe, plane, tiled; + int ret = 0, vblank_ref_err = 0, seqno; + RING_LOCALS; + + if (!(dev->driver->driver_features & DRIVER_GEM)) + return -ENODEV; + + /* + * Reject unknown flags so future userspace knows what we (don't) + * support + */ + if (flip_data->flags & (~I915_PAGE_FLIP_WAIT)) { + DRM_ERROR("bad page flip flags\n"); + return -EINVAL; + } + + if ((pipe = flip_data->pipe) > 1) { + DRM_ERROR("bad pipe\n"); + return -EINVAL; + } + + plane = i915_pipe_to_plane(dev, pipe); + if (plane < 0) { + DRM_ERROR("bad pipe (no planes enabled?)\n"); + return -EINVAL; + } + + obj = drm_gem_object_lookup(dev, file_priv, flip_data->handle); + if (obj == NULL) + return -EBADF; + + /* + * Make sure the new buffer is in bounds + * FIXME: should probably check against current mode as well + */ + if (flip_data->offset >= obj->size) { + DRM_ERROR("bad flip offset\n"); + ret = -EINVAL; + goto out_unref_prelock; + } + + obj_priv = obj->driver_private; + + if (i915_gem_flip_pending(obj)) + wait_for_completion(&obj_priv->vblank); + + mutex_lock(&dev->struct_mutex); + if (obj_priv->tiling_mode != I915_TILING_NONE && + obj_priv->tiling_mode != I915_TILING_X) { + DRM_ERROR("can only flip non-tiled or X tiled pages\n"); + ret = -EINVAL; + goto out_unref; + } + + vblank_ref_err = drm_vblank_get(dev, pipe); + if (!vblank_ref_err) { + ret = i915_gem_object_pin(obj, 0); + if (ret) { + drm_vblank_put(dev, pipe); + DRM_ERROR("failed to pin object for flip\n"); + goto out_unref; + } + } + + /* + * Put the object in the GTT domain before the flip, + * since there may be outstanding rendering + */ + ret = i915_gem_object_set_to_gtt_domain(obj, 0); + if (ret) { + DRM_ERROR("move to GTT failed\n"); + goto out_unref; + } + + offset = obj_priv->gtt_offset + flip_data->offset; + + pitch = obj_priv->stride; + tiled = (obj_priv->tiling_mode == I915_TILING_X); + + BEGIN_LP_RING(4); + OUT_RING(CMD_OP_DISPLAYBUFFER_INFO | (plane << 20)); + OUT_RING(pitch); + if (IS_I965G(dev)) { + OUT_RING(offset | tiled); + OUT_RING(((flip_data->x - 1) << 16) | (flip_data->y - 1)); + } else { + OUT_RING(offset); + OUT_RING(MI_NOOP); + } + ADVANCE_LP_RING(); + + seqno = i915_add_request(dev, 0); + + mutex_unlock(&dev->struct_mutex); + + /* + * This is a bit racy; the flip above may have already happened + * by the time we get here. If that happens, the new back buffer + * won't be available for rendering for one extra frame, since + * the vblank list won't have the object. + */ + spin_lock_irqsave(&dev_priv->vblank_lock, flags); + if (!vblank_ref_err) + list_add_tail(&obj_priv->vblank_head, + &dev_priv->mm.vblank_list[pipe]); + + obj_priv->flip_seqno = seqno; + spin_unlock_irqrestore(&dev_priv->vblank_lock, flags); + + if (!vblank_ref_err && (flip_data->flags & I915_PAGE_FLIP_WAIT)) + wait_for_completion(&obj_priv->vblank); + +out_unref_prelock: + /* Take lock again for unref */ + mutex_lock(&dev->struct_mutex); +out_unref: + drm_gem_object_unreference(obj); + mutex_unlock(&dev->struct_mutex); + + return ret; +} + /** * i915_probe_agp - get AGP bootup configuration * @pdev: PCI device @@ -1307,6 +1479,7 @@ struct drm_ioctl_desc i915_ioctls[] = { DRM_IOCTL_DEF(DRM_I915_GEM_SET_TILING, i915_gem_set_tiling, 0), DRM_IOCTL_DEF(DRM_I915_GEM_GET_TILING, i915_gem_get_tiling, 0), DRM_IOCTL_DEF(DRM_I915_GEM_GET_APERTURE, i915_gem_get_aperture_ioctl, 0), + DRM_IOCTL_DEF(DRM_I915_GEM_PAGE_FLIP, i915_gem_page_flip, DRM_AUTH|DRM_MASTER), }; int i915_max_ioctl = DRM_ARRAY_SIZE(i915_ioctls); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3951a12..148e80a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -159,6 +159,10 @@ typedef struct drm_i915_private { u32 irq_mask_reg; u32 pipestat[2]; + /** Protects vblank list */ + spinlock_t vblank_lock; + struct work_struct vblank_work; + int tex_lru_log_granularity; int allow_batchbuffer; struct mem_block *agp_heap; @@ -338,6 +342,11 @@ typedef struct drm_i915_private { */ struct delayed_work retire_work; + /** + * List of objects waiting on vblank events (one per pipe) + */ + struct list_head vblank_list[2]; + uint32_t next_gem_seqno; /** @@ -389,6 +398,13 @@ struct drm_i915_gem_object { /** This object's place on the active/flushing/inactive lists */ struct list_head list; + /** Object's place on the vblank list (protected by vblank_lock)*/ + struct list_head vblank_head; + /** sequence number for flip (when it passes the flip is done), + * protected by vblank lock + */ + int flip_seqno; + /** * This is set if the object is on the active or flushing lists * (has pending rendering), and is not set if it's on inactive (ready @@ -457,6 +473,9 @@ struct drm_i915_gem_object { /** for phy allocated objects */ struct drm_i915_gem_phys_object *phys_obj; + + /** for page flips and other vblank related blocks */ + struct completion vblank; }; /** @@ -516,6 +535,7 @@ extern long i915_compat_ioctl(struct file *filp, unsigned int cmd, extern int i915_emit_box(struct drm_device *dev, struct drm_clip_rect __user *boxes, int i, int DR1, int DR4); +extern bool i915_gem_flip_pending(struct drm_gem_object *obj); /* i915_irq.c */ extern int i915_irq_emit(struct drm_device *dev, void *data, @@ -625,6 +645,9 @@ int i915_gem_attach_phys_object(struct drm_device *dev, void i915_gem_detach_phys_object(struct drm_device *dev, struct drm_gem_object *obj); void i915_gem_free_all_phys_object(struct drm_device *dev); +uint32_t i915_add_request(struct drm_device *dev, uint32_t flush_domains); +int i915_seqno_passed(uint32_t seq1, uint32_t seq2); +uint32_t i915_get_gem_seqno(struct drm_device *dev); /* i915_gem_tiling.c */ void i915_gem_detect_bit_6_swizzle(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 3e025d5..f0996fb 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -900,7 +900,7 @@ i915_gem_object_move_to_inactive(struct drm_gem_object *obj) * * Returned sequence numbers are nonzero on success. */ -static uint32_t +uint32_t i915_add_request(struct drm_device *dev, uint32_t flush_domains) { drm_i915_private_t *dev_priv = dev->dev_private; @@ -1028,7 +1028,7 @@ i915_gem_retire_request(struct drm_device *dev, /** * Returns true if seq1 is later than seq2. */ -static int +int i915_seqno_passed(uint32_t seq1, uint32_t seq2) { return (int32_t)(seq1 - seq2) >= 0; @@ -2498,6 +2498,26 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, goto pre_mutex_err; } + /* Look up object handles */ + for (i = 0; i < args->buffer_count; i++) { + object_list[i] = drm_gem_object_lookup(dev, file_priv, + exec_list[i].handle); + if (object_list[i] == NULL) { + DRM_ERROR("Invalid object handle %d at index %d\n", + exec_list[i].handle, i); + ret = -EBADF; + mutex_lock(&dev->struct_mutex); + goto err; + } + + if (i915_gem_flip_pending(object_list[i])) { + struct drm_i915_gem_object *obj_priv; + + obj_priv = object_list[i]->driver_private; + wait_for_completion(&obj_priv->vblank); + } + } + mutex_lock(&dev->struct_mutex); i915_verify_inactive(dev, __FILE__, __LINE__); @@ -2516,18 +2536,6 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, goto pre_mutex_err; } - /* Look up object handles */ - for (i = 0; i < args->buffer_count; i++) { - object_list[i] = drm_gem_object_lookup(dev, file_priv, - exec_list[i].handle); - if (object_list[i] == NULL) { - DRM_ERROR("Invalid object handle %d at index %d\n", - exec_list[i].handle, i); - ret = -EBADF; - goto err; - } - } - /* Pin and relocate */ for (pin_tries = 0; ; pin_tries++) { ret = 0; @@ -2920,6 +2928,9 @@ int i915_gem_init_object(struct drm_gem_object *obj) obj_priv->obj = obj; obj_priv->fence_reg = I915_FENCE_REG_NONE; INIT_LIST_HEAD(&obj_priv->list); + INIT_LIST_HEAD(&obj_priv->vblank_head); + + init_completion(&obj_priv->vblank); return 0; } @@ -2980,8 +2991,10 @@ int i915_gem_idle(struct drm_device *dev) { drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_gem_object *obj_priv, *tmp; + unsigned long irqflags; uint32_t seqno, cur_seqno, last_seqno; - int stuck, ret; + int stuck, ret, pipe; mutex_lock(&dev->struct_mutex); @@ -2995,10 +3008,25 @@ i915_gem_idle(struct drm_device *dev) */ dev_priv->mm.suspended = 1; + mutex_unlock(&dev->struct_mutex); + + /* Wait for any outstanding flips */ + spin_lock_irqsave(&dev_priv->vblank_lock, irqflags); + for (pipe = 0; pipe < 2; pipe++) { + list_for_each_entry_safe(obj_priv, tmp, + &dev_priv->mm.vblank_list[pipe], + vblank_head) { + spin_unlock_irqrestore(&dev_priv->vblank_lock, irqflags); + wait_for_completion(&obj_priv->vblank); + spin_lock_irqsave(&dev_priv->vblank_lock, irqflags); + } + } + spin_unlock_irqrestore(&dev_priv->vblank_lock, irqflags); + /* Cancel the retire work handler, wait for it to finish if running */ - mutex_unlock(&dev->struct_mutex); cancel_delayed_work_sync(&dev_priv->mm.retire_work); + mutex_lock(&dev->struct_mutex); i915_kernel_lost_context(dev); @@ -3358,9 +3386,12 @@ i915_gem_load(struct drm_device *dev) INIT_LIST_HEAD(&dev_priv->mm.flushing_list); INIT_LIST_HEAD(&dev_priv->mm.inactive_list); INIT_LIST_HEAD(&dev_priv->mm.request_list); + INIT_LIST_HEAD(&dev_priv->mm.vblank_list[0]); + INIT_LIST_HEAD(&dev_priv->mm.vblank_list[1]); INIT_DELAYED_WORK(&dev_priv->mm.retire_work, i915_gem_retire_work_handler); dev_priv->mm.next_gem_seqno = 1; + spin_lock_init(&dev_priv->vblank_lock); /* Old X drivers will take 0-2 for front, back, depth buffers */ dev_priv->fence_reg_start = 3; diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 548ff2c..f50df88 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -187,6 +187,36 @@ u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe) return I915_READ(reg); } +static void i915_vblank_work_func(struct work_struct *work) +{ + drm_i915_private_t *dev_priv = container_of(work, drm_i915_private_t, + vblank_work); + struct drm_device *dev = dev_priv->dev; + struct drm_i915_gem_object *obj_priv, *tmp; + unsigned long irqflags; + int pipe, cur_seqno; + + mutex_lock(&dev->struct_mutex); + + spin_lock_irqsave(&dev_priv->vblank_lock, irqflags); + for (pipe = 0; pipe < 2; pipe++) { + list_for_each_entry_safe(obj_priv, tmp, + &dev_priv->mm.vblank_list[pipe], + vblank_head) { + cur_seqno = i915_get_gem_seqno(dev); + if (i915_seqno_passed(cur_seqno, + obj_priv->flip_seqno)) { + list_del_init(&obj_priv->vblank_head); + drm_vblank_put(dev, pipe); + i915_gem_object_unpin(obj_priv->obj); + complete(&obj_priv->vblank); + } + } + } + spin_unlock_irqrestore(&dev_priv->vblank_lock, irqflags); + mutex_unlock(&dev->struct_mutex); +} + irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS) { struct drm_device *dev = (struct drm_device *) arg; @@ -269,6 +299,9 @@ irqreturn_t i915_driver_irq_handler(DRM_IRQ_ARGS) drm_handle_vblank(dev, 1); } + if (vblank) + schedule_work(&dev_priv->vblank_work); + if ((pipeb_stats & I915_LEGACY_BLC_EVENT_STATUS) || (iir & I915_ASLE_INTERRUPT)) opregion_asle_intr(dev); @@ -533,6 +566,7 @@ void i915_driver_irq_preinstall(struct drm_device * dev) I915_WRITE(IMR, 0xffffffff); I915_WRITE(IER, 0x0); (void) I915_READ(IER); + INIT_WORK(&dev_priv->vblank_work, i915_vblank_work_func); } int i915_driver_irq_postinstall(struct drm_device *dev) diff --git a/include/drm/i915_drm.h b/include/drm/i915_drm.h index 912cd52..1ac4ded 100644 --- a/include/drm/i915_drm.h +++ b/include/drm/i915_drm.h @@ -184,6 +184,7 @@ typedef struct _drm_i915_sarea { #define DRM_I915_GEM_GET_TILING 0x22 #define DRM_I915_GEM_GET_APERTURE 0x23 #define DRM_I915_GEM_MMAP_GTT 0x24 +#define DRM_I915_GEM_PAGE_FLIP 0x25 #define DRM_IOCTL_I915_INIT DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t) #define DRM_IOCTL_I915_FLUSH DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH) @@ -219,6 +220,7 @@ typedef struct _drm_i915_sarea { #define DRM_IOCTL_I915_GEM_SET_TILING DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_SET_TILING, struct drm_i915_gem_set_tiling) #define DRM_IOCTL_I915_GEM_GET_TILING DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_GET_TILING, struct drm_i915_gem_get_tiling) #define DRM_IOCTL_I915_GEM_GET_APERTURE DRM_IOR (DRM_COMMAND_BASE + DRM_I915_GEM_GET_APERTURE, struct drm_i915_gem_get_aperture) +#define DRM_IOCTL_I915_GEM_PAGE_FLIP DRM_IOW (DRM_COMMAND_BASE + DRM_I915_GEM_PAGE_FLIP, struct drm_i915_gem_page_flip) /* Allow drivers to submit batchbuffers directly to hardware, relying * on the security mechanisms provided by hardware. @@ -654,4 +656,30 @@ struct drm_i915_gem_get_aperture { uint64_t aper_available_size; }; +#define I915_PAGE_FLIP_WAIT (1<<0) /* block on page flip completion */ + +struct drm_i915_gem_page_flip { + /** Handle of new front buffer */ + uint32_t handle; + + /** Offset into the object to use */ + uint64_t offset; + + /** + * page flip flags (wait on flip only for now) + */ + uint32_t flags; + + /** + * pipe to flip + */ + uint32_t pipe; + + /** + * screen dimensions for flip + */ + uint32_t x; + uint32_t y; +}; + #endif /* _I915_DRM_H_ */ |
From: Chris W. <ch...@ch...> - 2009-03-03 10:55:06
|
On Thu, 2009-02-26 at 14:35 -0800, Jesse Barnes wrote: > > > + ret = i915_gem_object_pin(obj, 0); > > > + if (ret) { > > > + DRM_ERROR("failed to pin object for flip\n"); > > > + ret = -EBUSY; > > > > What's the rationale for changing the reported error here? (And it might > > be helpful to printk the original error value.) > > Oh I'm not sure what's going on there; I've changed it to just return the pin > failure. > > > > + * Put the object in the GTT domain before the flip, > > > + * since there may be outstanding rendering > > > + */ > > > + i915_gem_object_set_to_gtt_domain(obj, 0); > > > > Need to check, report and handle errors here. And yes, these do occur in > > the wild for some as of yet unknown reason. > > Fixed. Though Mesa is now flushing before calling the server so it should > happen less often. I found the source of the errors - ERESTARTSYS, so presumably we need to add if (ret == -ERESTARTSYS) ret = -EINTR; handling here (after pining and flushing)? -ickle |
From: Jesse B. <jb...@vi...> - 2009-02-27 00:35:54
|
Found a bug in the AIGLX code; can't free the new buffers since we need them. -- Jesse Barnes, Intel Open Source Technology Center diff --git a/glx/glxdri2.c b/glx/glxdri2.c index 4e76c71..28bd3c4 100644 --- a/glx/glxdri2.c +++ b/glx/glxdri2.c @@ -131,10 +131,23 @@ __glXDRIdrawableCopySubBuffer(__GLXdrawable *drawable, static GLboolean __glXDRIdrawableSwapBuffers(__GLXdrawable *drawable) { - __GLXDRIdrawable *private = (__GLXDRIdrawable *) drawable; + __GLXDRIdrawable *priv = (__GLXDRIdrawable *) drawable; + DRI2BufferPtr buffers; + int i, count; + + buffers = DRI2SwapBuffers(drawable->pDraw, &count); + if (!buffers) + return FALSE; + + for (i = 0; i < count; i++) { + priv->buffers[i].attachment = buffers[i].attachment; + priv->buffers[i].name = buffers[i].name; + priv->buffers[i].pitch = buffers[i].pitch; + priv->buffers[i].cpp = buffers[i].cpp; + priv->buffers[i].flags = buffers[i].flags; + } - __glXDRIdrawableCopySubBuffer(drawable, 0, 0, - private->width, private->height); + priv->count = count; return TRUE; } diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 0f2e24b..375a13d 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -66,6 +66,7 @@ typedef struct _DRI2Screen { DRI2CreateBuffersProcPtr CreateBuffers; DRI2DestroyBuffersProcPtr DestroyBuffers; DRI2CopyRegionProcPtr CopyRegion; + DRI2SwapBuffersProcPtr SwapBuffers; HandleExposuresProcPtr HandleExposures; } DRI2ScreenRec, *DRI2ScreenPtr; @@ -188,6 +189,32 @@ DRI2CopyRegion(DrawablePtr pDraw, RegionPtr pRegion, return Success; } +DRI2BufferPtr +DRI2SwapBuffers(DrawablePtr pDraw, int *reply_count) +{ + DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); + DRI2DrawablePtr pPriv; + DRI2BufferPtr buffers; + + pPriv = DRI2GetDrawable(pDraw); + if (pPriv == NULL) + return NULL; + + /* Driver will give us a new set of buffers, so free the old ones */ + buffers = (*ds->SwapBuffers)(pDraw, pPriv->buffers, pPriv->bufferCount); + if (!buffers) { + *reply_count = 0; + return NULL; + } + + (*ds->DestroyBuffers)(pDraw, pPriv->buffers, pPriv->bufferCount); + pPriv->buffers = buffers; + + *reply_count = pPriv->bufferCount; + + return buffers; +} + void DRI2DestroyDrawable(DrawablePtr pDraw) { @@ -264,6 +291,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) ds->CreateBuffers = info->CreateBuffers; ds->DestroyBuffers = info->DestroyBuffers; ds->CopyRegion = info->CopyRegion; + ds->SwapBuffers = info->SwapBuffers; dixSetPrivate(&pScreen->devPrivates, dri2ScreenPrivateKey, ds); diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h index 5e7fd65..8cb9e94 100644 --- a/hw/xfree86/dri2/dri2.h +++ b/hw/xfree86/dri2/dri2.h @@ -54,6 +54,9 @@ typedef void (*DRI2CopyRegionProcPtr)(DrawablePtr pDraw, RegionPtr pRegion, DRI2BufferPtr pDestBuffer, DRI2BufferPtr pSrcBuffer); +typedef DRI2BufferPtr (*DRI2SwapBuffersProcPtr)(DrawablePtr pDraw, + DRI2BufferPtr buffers, + int count); typedef void (*DRI2WaitProcPtr)(WindowPtr pWin, unsigned int sequence); @@ -67,6 +70,7 @@ typedef struct { DRI2CreateBuffersProcPtr CreateBuffers; DRI2DestroyBuffersProcPtr DestroyBuffers; DRI2CopyRegionProcPtr CopyRegion; + DRI2SwapBuffersProcPtr SwapBuffers; DRI2WaitProcPtr Wait; } DRI2InfoRec, *DRI2InfoPtr; @@ -100,4 +104,6 @@ int DRI2CopyRegion(DrawablePtr pDraw, unsigned int dest, unsigned int src); +DRI2BufferPtr DRI2SwapBuffers(DrawablePtr pDraw, int *count); + #endif diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c index 0a1dce4..5582a5f 100644 --- a/hw/xfree86/dri2/dri2ext.c +++ b/hw/xfree86/dri2/dri2ext.c @@ -269,6 +269,45 @@ ProcDRI2CopyRegion(ClientPtr client) } static int +ProcDRI2SwapBuffers(ClientPtr client) +{ + REQUEST(xDRI2SwapBuffersReq); + xDRI2SwapBuffersReply rep; + DrawablePtr pDrawable; + DRI2BufferPtr buffers; + xDRI2Buffer buffer; + int status; + int i, count; + + REQUEST_SIZE_MATCH(xDRI2SwapBuffersReq); + + if (!validDrawable(client, stuff->drawable, &pDrawable, &status)) + return status; + + buffers = DRI2SwapBuffers(pDrawable, &count); + if (!buffers) { + return BadAlloc; + } + + rep.type = X_Reply; + rep.length = count * sizeof(xDRI2Buffer) / 4; + rep.count = count; + rep.sequenceNumber = client->sequence; + WriteToClient(client, sizeof(xDRI2GetBuffersReply), &rep); + + for (i = 0; i < count; i++) { + buffer.attachment = buffers[i].attachment; + buffer.name = buffers[i].name; + buffer.pitch = buffers[i].pitch; + buffer.cpp = buffers[i].cpp; + buffer.flags = buffers[i].flags; + WriteToClient(client, sizeof(xDRI2Buffer), &buffer); + } + + return client->noClientException; +} + +static int ProcDRI2Dispatch (ClientPtr client) { REQUEST(xReq); @@ -294,6 +333,8 @@ ProcDRI2Dispatch (ClientPtr client) return ProcDRI2GetBuffers(client); case X_DRI2CopyRegion: return ProcDRI2CopyRegion(client); + case X_DRI2SwapBuffers: + return ProcDRI2SwapBuffers(client); default: return BadRequest; } |
From: Jesse B. <jb...@vi...> - 2009-02-27 19:22:15
|
On Thursday, February 26, 2009 4:35:34 pm Jesse Barnes wrote: > Found a bug in the AIGLX code; can't free the new buffers since we need > them. And another bug in the indirect case: if swapbuffers fails we should fall back to a copyregion. This should fix the old compiz case. -- Jesse Barnes, Intel Open Source Technology Center diff --git a/glx/glxdri2.c b/glx/glxdri2.c index 4e76c71..9d7d9bf 100644 --- a/glx/glxdri2.c +++ b/glx/glxdri2.c @@ -131,10 +131,26 @@ __glXDRIdrawableCopySubBuffer(__GLXdrawable *drawable, static GLboolean __glXDRIdrawableSwapBuffers(__GLXdrawable *drawable) { - __GLXDRIdrawable *private = (__GLXDRIdrawable *) drawable; + __GLXDRIdrawable *priv = (__GLXDRIdrawable *) drawable; + DRI2BufferPtr buffers; + int i, count; + + buffers = DRI2SwapBuffers(drawable->pDraw, &count); + if (!buffers) { + __glXDRIdrawableCopySubBuffer(drawable, 0, 0, + priv->width, priv->height); + return TRUE; + } + + for (i = 0; i < count; i++) { + priv->buffers[i].attachment = buffers[i].attachment; + priv->buffers[i].name = buffers[i].name; + priv->buffers[i].pitch = buffers[i].pitch; + priv->buffers[i].cpp = buffers[i].cpp; + priv->buffers[i].flags = buffers[i].flags; + } - __glXDRIdrawableCopySubBuffer(drawable, 0, 0, - private->width, private->height); + priv->count = count; return TRUE; } diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c index 0f2e24b..1ee92dc 100644 --- a/hw/xfree86/dri2/dri2.c +++ b/hw/xfree86/dri2/dri2.c @@ -66,6 +66,7 @@ typedef struct _DRI2Screen { DRI2CreateBuffersProcPtr CreateBuffers; DRI2DestroyBuffersProcPtr DestroyBuffers; DRI2CopyRegionProcPtr CopyRegion; + DRI2SwapBuffersProcPtr SwapBuffers; HandleExposuresProcPtr HandleExposures; } DRI2ScreenRec, *DRI2ScreenPtr; @@ -188,6 +189,35 @@ DRI2CopyRegion(DrawablePtr pDraw, RegionPtr pRegion, return Success; } +DRI2BufferPtr +DRI2SwapBuffers(DrawablePtr pDraw, int *reply_count) +{ + DRI2ScreenPtr ds = DRI2GetScreen(pDraw->pScreen); + DRI2DrawablePtr pPriv; + DRI2BufferPtr buffers; + + pPriv = DRI2GetDrawable(pDraw); + if (pPriv == NULL) + return NULL; + + if (!ds->SwapBuffers) + return NULL; + + /* Driver will give us a new set of buffers, so free the old ones */ + buffers = (*ds->SwapBuffers)(pDraw, pPriv->buffers, pPriv->bufferCount); + if (!buffers) { + *reply_count = 0; + return NULL; + } + + (*ds->DestroyBuffers)(pDraw, pPriv->buffers, pPriv->bufferCount); + pPriv->buffers = buffers; + + *reply_count = pPriv->bufferCount; + + return buffers; +} + void DRI2DestroyDrawable(DrawablePtr pDraw) { @@ -264,6 +294,7 @@ DRI2ScreenInit(ScreenPtr pScreen, DRI2InfoPtr info) ds->CreateBuffers = info->CreateBuffers; ds->DestroyBuffers = info->DestroyBuffers; ds->CopyRegion = info->CopyRegion; + ds->SwapBuffers = info->SwapBuffers; dixSetPrivate(&pScreen->devPrivates, dri2ScreenPrivateKey, ds); diff --git a/hw/xfree86/dri2/dri2.h b/hw/xfree86/dri2/dri2.h index 5e7fd65..8cb9e94 100644 --- a/hw/xfree86/dri2/dri2.h +++ b/hw/xfree86/dri2/dri2.h @@ -54,6 +54,9 @@ typedef void (*DRI2CopyRegionProcPtr)(DrawablePtr pDraw, RegionPtr pRegion, DRI2BufferPtr pDestBuffer, DRI2BufferPtr pSrcBuffer); +typedef DRI2BufferPtr (*DRI2SwapBuffersProcPtr)(DrawablePtr pDraw, + DRI2BufferPtr buffers, + int count); typedef void (*DRI2WaitProcPtr)(WindowPtr pWin, unsigned int sequence); @@ -67,6 +70,7 @@ typedef struct { DRI2CreateBuffersProcPtr CreateBuffers; DRI2DestroyBuffersProcPtr DestroyBuffers; DRI2CopyRegionProcPtr CopyRegion; + DRI2SwapBuffersProcPtr SwapBuffers; DRI2WaitProcPtr Wait; } DRI2InfoRec, *DRI2InfoPtr; @@ -100,4 +104,6 @@ int DRI2CopyRegion(DrawablePtr pDraw, unsigned int dest, unsigned int src); +DRI2BufferPtr DRI2SwapBuffers(DrawablePtr pDraw, int *count); + #endif diff --git a/hw/xfree86/dri2/dri2ext.c b/hw/xfree86/dri2/dri2ext.c index 0a1dce4..5582a5f 100644 --- a/hw/xfree86/dri2/dri2ext.c +++ b/hw/xfree86/dri2/dri2ext.c @@ -269,6 +269,45 @@ ProcDRI2CopyRegion(ClientPtr client) } static int +ProcDRI2SwapBuffers(ClientPtr client) +{ + REQUEST(xDRI2SwapBuffersReq); + xDRI2SwapBuffersReply rep; + DrawablePtr pDrawable; + DRI2BufferPtr buffers; + xDRI2Buffer buffer; + int status; + int i, count; + + REQUEST_SIZE_MATCH(xDRI2SwapBuffersReq); + + if (!validDrawable(client, stuff->drawable, &pDrawable, &status)) + return status; + + buffers = DRI2SwapBuffers(pDrawable, &count); + if (!buffers) { + return BadAlloc; + } + + rep.type = X_Reply; + rep.length = count * sizeof(xDRI2Buffer) / 4; + rep.count = count; + rep.sequenceNumber = client->sequence; + WriteToClient(client, sizeof(xDRI2GetBuffersReply), &rep); + + for (i = 0; i < count; i++) { + buffer.attachment = buffers[i].attachment; + buffer.name = buffers[i].name; + buffer.pitch = buffers[i].pitch; + buffer.cpp = buffers[i].cpp; + buffer.flags = buffers[i].flags; + WriteToClient(client, sizeof(xDRI2Buffer), &buffer); + } + + return client->noClientException; +} + +static int ProcDRI2Dispatch (ClientPtr client) { REQUEST(xReq); @@ -294,6 +333,8 @@ ProcDRI2Dispatch (ClientPtr client) return ProcDRI2GetBuffers(client); case X_DRI2CopyRegion: return ProcDRI2CopyRegion(client); + case X_DRI2SwapBuffers: + return ProcDRI2SwapBuffers(client); default: return BadRequest; } |
From: Jesse B. <jb...@vi...> - 2009-02-27 19:23:00
|
On Thursday, February 26, 2009 1:31:03 pm Jesse Barnes wrote: > Add support to Mesa and the intel driver for the new DRI2 swapbuffers > interface. Uses the new flush hook to make sure any outstanding rendering > is complete before sending the swapbuffers request. Need to update the FBconfigs too; we now support the exchange method. -- Jesse Barnes, Intel Open Source Technology Center diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h index a726b93..b663028 100644 --- a/include/GL/internal/dri_interface.h +++ b/include/GL/internal/dri_interface.h @@ -681,6 +681,9 @@ struct __DRIdri2ExtensionRec { __DRIcontext *shared, void *loaderPrivate); + void (*setBuffers)(__DRIdrawable *drawable, + __DRIbuffer *buffers, + int count); }; #endif diff --git a/src/glx/x11/dri2.c b/src/glx/x11/dri2.c index f967432..7af5484 100644 --- a/src/glx/x11/dri2.c +++ b/src/glx/x11/dri2.c @@ -30,7 +30,7 @@ * Kristian Høgsberg (kr...@re...) */ - +#include <stdio.h> #define NEED_REPLIES #include <X11/Xlibint.h> #include <X11/extensions/Xext.h> @@ -299,3 +299,51 @@ void DRI2CopyRegion(Display *dpy, XID drawable, XserverRegion region, UnlockDisplay(dpy); SyncHandle(); } + +DRI2Buffer *DRI2SwapBuffers(Display *dpy, XID drawable, int *recv_count) +{ + XExtDisplayInfo *info = DRI2FindDisplay(dpy); + xDRI2SwapBuffersReq *req; + xDRI2SwapBuffersReply rep; + xDRI2Buffer repBuffer; + DRI2Buffer *new_buffers; + int i; + + XextCheckExtension (dpy, info, dri2ExtensionName, False); + + LockDisplay(dpy); + GetReq(DRI2SwapBuffers, req); + req->reqType = info->codes->major_opcode; + req->dri2ReqType = X_DRI2SwapBuffers; + req->drawable = drawable; + + if (!_XReply(dpy, (xReply *)&rep, 0, xFalse)) { + UnlockDisplay(dpy); + SyncHandle(); + return NULL; + } + + new_buffers = Xmalloc(rep.count * sizeof *new_buffers); + if (new_buffers == NULL) { + _XEatData(dpy, rep.count * sizeof repBuffer); + UnlockDisplay(dpy); + SyncHandle(); + return NULL; + } + + for (i = 0; i < rep.count; i++) { + _XReadPad(dpy, (char *) &repBuffer, sizeof repBuffer); + new_buffers[i].attachment = repBuffer.attachment; + new_buffers[i].name = repBuffer.name; + new_buffers[i].pitch = repBuffer.pitch; + new_buffers[i].cpp = repBuffer.cpp; + new_buffers[i].flags = repBuffer.flags; + } + + *recv_count = rep.count; + + UnlockDisplay(dpy); + SyncHandle(); + + return new_buffers; +} diff --git a/src/glx/x11/dri2.h b/src/glx/x11/dri2.h index 356c6bc..83dfaf6 100644 --- a/src/glx/x11/dri2.h +++ b/src/glx/x11/dri2.h @@ -67,4 +67,7 @@ extern void DRI2CopyRegion(Display *dpy, XID drawable, XserverRegion region, CARD32 dest, CARD32 src); +extern DRI2Buffer * +DRI2SwapBuffers(Display *dpy, XID drawable, int *count); + #endif diff --git a/src/glx/x11/dri2_glx.c b/src/glx/x11/dri2_glx.c index 0ef5d3a..f80f201 100644 --- a/src/glx/x11/dri2_glx.c +++ b/src/glx/x11/dri2_glx.c @@ -35,6 +35,7 @@ #include <X11/Xlib.h> #include <X11/extensions/Xfixes.h> #include <X11/extensions/Xdamage.h> +#include "glapi.h" #include "glxclient.h" #include "glcontextmodes.h" #include "xf86dri.h" @@ -61,6 +62,8 @@ struct __GLXDRIdisplayPrivateRec { int driMinor; int driPatch; + int swapAvailable; + unsigned long configureSeqno; Bool (*oldConfigProc)(Display *, XEvent *, xEvent *); }; @@ -223,8 +226,40 @@ static void dri2CopySubBuffer(__GLXDRIdrawable *pdraw, static void dri2SwapBuffers(__GLXDRIdrawable *pdraw) { __GLXDRIdrawablePrivate *priv = (__GLXDRIdrawablePrivate *) pdraw; + __GLXdisplayPrivate *dpyPriv = __glXInitialize(priv->base.psc->dpy); + __GLXDRIdisplayPrivate *pdp = + (__GLXDRIdisplayPrivate *)dpyPriv->dri2Display; + __GLXscreenConfigs *psc = pdraw->psc; + DRI2Buffer *buffers; + int i, count; + +#ifdef __DRI2_FLUSH + if (pdraw->psc->f) + (*pdraw->psc->f->flush)(pdraw->driDrawable); +#endif + + /* Old servers can't handle swapbuffers */ + if (!pdp->swapAvailable) + return dri2CopySubBuffer(pdraw, 0, 0, priv->width, priv->height); + + buffers = DRI2SwapBuffers(pdraw->psc->dpy, pdraw->drawable, &count); + if (buffers == NULL || !count) + return dri2CopySubBuffer(pdraw, 0, 0, priv->width, priv->height); + + /* Update our buffer list with what was returned */ + for (i = 0; i < count; i++) { + priv->buffers[i].attachment = buffers[i].attachment; + priv->buffers[i].name = buffers[i].name; + priv->buffers[i].pitch = buffers[i].pitch; + priv->buffers[i].cpp = buffers[i].cpp; + priv->buffers[i].flags = buffers[i].flags; + } - dri2CopySubBuffer(pdraw, 0, 0, priv->width, priv->height); + priv->bufferCount = count; + + (*psc->dri2->setBuffers)(pdraw->driDrawable, priv->buffers, count); + + Xfree(buffers); } static void dri2WaitX(__GLXDRIdrawable *pdraw) @@ -517,6 +552,9 @@ _X_HIDDEN __GLXDRIdisplay *dri2CreateDisplay(Display *dpy) pdp->driPatch = 0; pdp->configureSeqno = 0; + pdp->swapAvailable = 0; + if (pdp->driMinor > 0) + pdp->swapAvailable = 1; pdp->base.destroyDisplay = dri2DestroyDisplay; pdp->base.createScreen = dri2CreateScreen; diff --git a/src/mesa/drivers/dri/common/dri_util.c b/src/mesa/drivers/dri/common/dri_util.c index ae79055..90a39df 100644 --- a/src/mesa/drivers/dri/common/dri_util.c +++ b/src/mesa/drivers/dri/common/dri_util.c @@ -488,6 +488,15 @@ dri2CreateNewDrawable(__DRIscreen *screen, return pdraw; } +static void +dri2SetBuffers(__DRIdrawable *draw, __DRIbuffer *buffers, int count) +{ + __DRIscreen *psp = draw->driScreenPriv; + + if (psp->DriverAPI.SetBuffers) + (*psp->DriverAPI.SetBuffers)(draw, buffers, count); +} + static void driDestroyDrawable(__DRIdrawable *pdp) @@ -832,6 +841,7 @@ const __DRIdri2Extension driDRI2Extension = { dri2CreateNewScreen, dri2CreateNewDrawable, dri2CreateNewContext, + dri2SetBuffers, }; /* This is the table of extensions that the loader will dlsym() for. */ diff --git a/src/mesa/drivers/dri/common/dri_util.h b/src/mesa/drivers/dri/common/dri_util.h index c95a5c8..2a6e1cd 100644 --- a/src/mesa/drivers/dri/common/dri_util.h +++ b/src/mesa/drivers/dri/common/dri_util.h @@ -225,6 +225,13 @@ struct __DriverAPIRec { /* DRI2 Entry point */ const __DRIconfig **(*InitScreen2) (__DRIscreen * priv); + + /** + * Update the render buffers with a new set + */ + void (*SetBuffers) ( __DRIdrawable *drawable, + __DRIbuffer *buffers, + int count); }; extern const struct __DriverAPIRec driDriverAPI; diff --git a/src/mesa/drivers/dri/intel/intel_context.c b/src/mesa/drivers/dri/intel/intel_context.c index 1aa173d..bef1907 100644 --- a/src/mesa/drivers/dri/intel/intel_context.c +++ b/src/mesa/drivers/dri/intel/intel_context.c @@ -169,6 +169,30 @@ intelGetString(GLcontext * ctx, GLenum name) } } +/* + * DRI may give us with a new set of buffers after a flip or swap, so + * update the attachments here. + */ +void intelSetBuffers(__DRIdrawable *drawable, + __DRIbuffer *buffers, + int count) +{ + __DRIcontext *context = drawable->driContextPriv; + struct intel_framebuffer *intel_fb = drawable->driverPrivate; + struct intel_context *intel = context->driverPrivate; + struct intel_renderbuffer *rb; + struct intel_region *region; + const char *region_name; + int i; + + /* + * Ignore the buffers & count args, we'll just pick them up from our + * drawable. + */ + intel_update_renderbuffers(context, drawable); + intel_draw_buffer(&intel->ctx, &intel_fb->Base); +} + void intel_update_renderbuffers(__DRIcontext *context, __DRIdrawable *drawable) { @@ -399,6 +423,16 @@ intel_glFlush(GLcontext *ctx) intel_flush(ctx, GL_TRUE); } +void intelFlushDrawable(__DRIdrawable *drawable) +{ + __DRIdrawablePrivate * dPriv = drawable->driverPrivate; + struct intel_context *intel = + (struct intel_context *) dPriv->driContextPriv->driverPrivate; + GLcontext *ctx = &intel->ctx; + + intel_flush(ctx, GL_TRUE); +} + void intelFinish(GLcontext * ctx) { diff --git a/src/mesa/drivers/dri/intel/intel_extensions.h b/src/mesa/drivers/dri/intel/intel_extensions.h index 97147ec..9283ee9 100644 --- a/src/mesa/drivers/dri/intel/intel_extensions.h +++ b/src/mesa/drivers/dri/intel/intel_extensions.h @@ -32,5 +32,8 @@ extern void intelInitExtensions(GLcontext *ctx, GLboolean enable_imaging); +extern void +intelFlushDrawable(__DRIdrawable *drawable); + #endif diff --git a/src/mesa/drivers/dri/intel/intel_screen.c b/src/mesa/drivers/dri/intel/intel_screen.c index 09eba13..8a50a17 100644 --- a/src/mesa/drivers/dri/intel/intel_screen.c +++ b/src/mesa/drivers/dri/intel/intel_screen.c @@ -211,6 +211,11 @@ static const __DRItexBufferExtension intelTexBufferExtension = { intelSetTexBuffer, }; +static const __DRI2flushExtension intelFlushExtension = { + { __DRI2_FLUSH, __DRI2_FLUSH_VERSION }, + intelFlushDrawable, +}; + static const __DRIextension *intelScreenExtensions[] = { &driReadDrawableExtension, &driCopySubBufferExtension.base, @@ -475,11 +480,9 @@ intelFillInModes(__DRIscreenPrivate *psp, unsigned back_buffer_factor; int i; - /* GLX_SWAP_COPY_OML is only supported because the Intel driver doesn't - * support pageflipping at all. - */ static const GLenum back_buffer_modes[] = { - GLX_NONE, GLX_SWAP_UNDEFINED_OML, GLX_SWAP_COPY_OML + GLX_NONE, GLX_SWAP_UNDEFINED_OML, + GLX_SWAP_EXCHANGE_OML, GLX_SWAP_COPY_OML }; uint8_t depth_bits_array[3]; @@ -697,11 +700,10 @@ __DRIconfig **intelInitScreen2(__DRIscreenPrivate *psp) intelScreenPrivate *intelScreen; GLenum fb_format[3]; GLenum fb_type[3]; - /* GLX_SWAP_COPY_OML is only supported because the Intel driver doesn't - * support pageflipping at all. - */ + static const GLenum back_buffer_modes[] = { - GLX_NONE, GLX_SWAP_UNDEFINED_OML, GLX_SWAP_COPY_OML + GLX_NONE, GLX_SWAP_UNDEFINED_OML, + GLX_SWAP_EXCHANGE_OML, GLX_SWAP_COPY_OML }; uint8_t depth_bits[4], stencil_bits[4], msaa_samples_array[1]; int color; @@ -828,4 +830,6 @@ const struct __DriverAPIRec driDriverAPI = { .CopySubBuffer = intelCopySubBuffer, .InitScreen2 = intelInitScreen2, + + .SetBuffers = intelSetBuffers, }; diff --git a/src/mesa/drivers/dri/intel/intel_screen.h b/src/mesa/drivers/dri/intel/intel_screen.h index e1036de..b23dcee 100644 --- a/src/mesa/drivers/dri/intel/intel_screen.h +++ b/src/mesa/drivers/dri/intel/intel_screen.h @@ -103,4 +103,7 @@ intelMakeCurrent(__DRIcontextPrivate * driContextPriv, extern struct intel_context *intelScreenContext(intelScreenPrivate *intelScreen); +extern void intelSetBuffers(__DRIdrawable *drawable, __DRIbuffer *buffers, + int count); + #endif |
From: Kristian H. <kr...@bi...> - 2009-02-27 20:34:28
|
On Fri, Feb 27, 2009 at 2:22 PM, Jesse Barnes <jb...@vi...> wrote: > On Thursday, February 26, 2009 1:31:03 pm Jesse Barnes wrote: >> Add support to Mesa and the intel driver for the new DRI2 swapbuffers >> interface. Uses the new flush hook to make sure any outstanding rendering >> is complete before sending the swapbuffers request. > > Need to update the FBconfigs too; we now support the exchange method. > > -- > Jesse Barnes, Intel Open Source Technology Center > > diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h > index a726b93..b663028 100644 > --- a/include/GL/internal/dri_interface.h > +++ b/include/GL/internal/dri_interface.h > @@ -681,6 +681,9 @@ struct __DRIdri2ExtensionRec { > __DRIcontext *shared, > void *loaderPrivate); > > + void (*setBuffers)(__DRIdrawable *drawable, > + __DRIbuffer *buffers, > + int count); > }; We don't need setBuffers, we can just require the flush extension. When the flush entry point is called, mark the buffers as invalid which will cause the dri driver to call getBuffers() before doing any further rendering. > @@ -223,8 +226,40 @@ static void dri2CopySubBuffer(__GLXDRIdrawable *pdraw, > static void dri2SwapBuffers(__GLXDRIdrawable *pdraw) > { > __GLXDRIdrawablePrivate *priv = (__GLXDRIdrawablePrivate *) pdraw; > + __GLXdisplayPrivate *dpyPriv = __glXInitialize(priv->base.psc->dpy); > + __GLXDRIdisplayPrivate *pdp = > + (__GLXDRIdisplayPrivate *)dpyPriv->dri2Display; > + __GLXscreenConfigs *psc = pdraw->psc; > + DRI2Buffer *buffers; > + int i, count; > + > +#ifdef __DRI2_FLUSH > + if (pdraw->psc->f) > + (*pdraw->psc->f->flush)(pdraw->driDrawable); > +#endif > + > + /* Old servers can't handle swapbuffers */ > + if (!pdp->swapAvailable) > + return dri2CopySubBuffer(pdraw, 0, 0, priv->width, priv->height); > + > + buffers = DRI2SwapBuffers(pdraw->psc->dpy, pdraw->drawable, &count); > + if (buffers == NULL || !count) > + return dri2CopySubBuffer(pdraw, 0, 0, priv->width, priv->height); As for the AIGLX case, DRI2SwapBuffers should swap the buffers or fallback to CopyRegion in the server. This requires an extra round trip in the fallback case and we have implement the fallback logic everywhere we use DRI2SwapBuffers instead of just once in the server. cheers, Kristian |
From: Jesse B. <jb...@vi...> - 2009-04-13 20:59:09
|
On Fri, 27 Feb 2009 15:34:26 -0500 Kristian Høgsberg <kr...@bi...> wrote: > On Fri, Feb 27, 2009 at 2:22 PM, Jesse Barnes > <jb...@vi...> wrote: > > On Thursday, February 26, 2009 1:31:03 pm Jesse Barnes wrote: > >> Add support to Mesa and the intel driver for the new DRI2 > >> swapbuffers interface. Uses the new flush hook to make sure any > >> outstanding rendering is complete before sending the swapbuffers > >> request. > > > > Need to update the FBconfigs too; we now support the exchange > > method. > > > > -- > > Jesse Barnes, Intel Open Source Technology Center > > > > diff --git a/include/GL/internal/dri_interface.h > > b/include/GL/internal/dri_interface.h index a726b93..b663028 100644 > > --- a/include/GL/internal/dri_interface.h > > +++ b/include/GL/internal/dri_interface.h > > @@ -681,6 +681,9 @@ struct __DRIdri2ExtensionRec { > > __DRIcontext *shared, > > void *loaderPrivate); > > > > + void (*setBuffers)(__DRIdrawable *drawable, > > + __DRIbuffer *buffers, > > + int count); > > }; > > We don't need setBuffers, we can just require the flush extension. > When the flush entry point is called, mark the buffers as invalid > which will cause the dri driver to call getBuffers() before doing any > further rendering. Finally getting back to this... So if we drop setBuffers, it means we need a way of invalidating the current set of drawables in a reliable way. The closest thing we have today with DRI2 is the Viewport hook: it will update the renderbuffers down in the driver so we could call it at swapbuffers time after the swap. However it won't work without server changes since the server will short circuit getbuffers requests without size changes... Also, ->flush doesn't seem like the right place to invalidate things, since we need to flush before the swap, and only update/invalidate the buffers after it completes. So I'm not sure how best to handle this. Given that the fake front buffer discussion has opened up a can of worms in this area, I expect changes here anyway... > > > @@ -223,8 +226,40 @@ static void dri2CopySubBuffer(__GLXDRIdrawable > > *pdraw, static void dri2SwapBuffers(__GLXDRIdrawable *pdraw) > > { > > __GLXDRIdrawablePrivate *priv = (__GLXDRIdrawablePrivate *) > > pdraw; > > + __GLXdisplayPrivate *dpyPriv = > > __glXInitialize(priv->base.psc->dpy); > > + __GLXDRIdisplayPrivate *pdp = > > + (__GLXDRIdisplayPrivate *)dpyPriv->dri2Display; > > + __GLXscreenConfigs *psc = pdraw->psc; > > + DRI2Buffer *buffers; > > + int i, count; > > + > > +#ifdef __DRI2_FLUSH > > + if (pdraw->psc->f) > > + (*pdraw->psc->f->flush)(pdraw->driDrawable); > > +#endif > > + > > + /* Old servers can't handle swapbuffers */ > > + if (!pdp->swapAvailable) > > + return dri2CopySubBuffer(pdraw, 0, 0, priv->width, > > priv->height); + > > + buffers = DRI2SwapBuffers(pdraw->psc->dpy, pdraw->drawable, > > &count); > > + if (buffers == NULL || !count) > > + return dri2CopySubBuffer(pdraw, 0, 0, priv->width, > > priv->height); > > As for the AIGLX case, DRI2SwapBuffers should swap the buffers or > fallback to CopyRegion in the server. This requires an extra round > trip in the fallback case and we have implement the fallback logic > everywhere we use DRI2SwapBuffers instead of just once in the server. I wanted to support old servers here; if the server doesn't support swapping we shouldn't even try the call (since as you say it would incur an extra round trip for nothing). In the server side, yes I agree if we get a request we can't swap we should just copy internally and not have the client send a new request. I think that's how things are handled with this set... -- Jesse Barnes, Intel Open Source Technology Center |
From: Chris W. <ch...@ch...> - 2009-03-13 21:09:07
|
On Thu, 2009-02-26 at 14:35 -0800, Jesse Barnes wrote: > On Thursday, February 26, 2009 1:56:52 pm Chris Wilson wrote: > > On Thu, 2009-02-26 at 13:28 -0800, Jesse Barnes wrote: > > > Add a new page flip ioctl to the i915 driver. The new ioctl takes the > > > new drm_i915_gem_page_flip arg, which contains a buffer object target for > > > the flip, an offset into that buffer, and other buffer parameters to be > > > used for the flip. > > > > > > If a flip is already pending on the object in question, the ioctl waits > > > for it to finish first, then queues the flip. An optional wait flag > > > causes the ioctl to wait for the flip to complete before the it returns. > > > > > > If an execbuffer containing an object with a pending flip comes in, it > > > will stall until the flip completes, to preserve glXSwapBuffers > > > semantics. > > > > > > Signed-off-by: Jesse Barnes <jb...@vi...> Hmm, I finally got around to trying this updated variant of the page-flipping patch having hit a situation whereby the lack of unpinning of the new scanout buffer was consuming all the fence registers. The downside of this version, which unpins after vblank, is that this leaves the fence register covering the scanout buffer on the i915 available for reuse. [This also confirms that the scanout memory is vulnerable to an unbind().] The bo must remain pinned until it is no longer the scanout target -- the flickering (as the scanout rapidly switches between tiled and untiled) on my i915 is unbearable :( -ickle |
From: Jesse B. <jb...@vi...> - 2009-03-13 21:13:52
|
On Friday, March 13, 2009 2:08:50 pm Chris Wilson wrote: > On Thu, 2009-02-26 at 14:35 -0800, Jesse Barnes wrote: > > On Thursday, February 26, 2009 1:56:52 pm Chris Wilson wrote: > > > On Thu, 2009-02-26 at 13:28 -0800, Jesse Barnes wrote: > > > > Add a new page flip ioctl to the i915 driver. The new ioctl takes > > > > the new drm_i915_gem_page_flip arg, which contains a buffer object > > > > target for the flip, an offset into that buffer, and other buffer > > > > parameters to be used for the flip. > > > > > > > > If a flip is already pending on the object in question, the ioctl > > > > waits for it to finish first, then queues the flip. An optional wait > > > > flag causes the ioctl to wait for the flip to complete before the it > > > > returns. > > > > > > > > If an execbuffer containing an object with a pending flip comes in, > > > > it will stall until the flip completes, to preserve glXSwapBuffers > > > > semantics. > > > > > > > > Signed-off-by: Jesse Barnes <jb...@vi...> > > Hmm, I finally got around to trying this updated variant of the > page-flipping patch having hit a situation whereby the lack of unpinning > of the new scanout buffer was consuming all the fence registers. The > downside of this version, which unpins after vblank, is that this leaves > the fence register covering the scanout buffer on the i915 available for > reuse. [This also confirms that the scanout memory is vulnerable to an > unbind().] The bo must remain pinned until it is no longer the scanout > target -- the flickering (as the scanout rapidly switches between tiled > and untiled) on my i915 is unbearable :( > -ickle Yuck, yeah we'll have to figure out a good way of handling the pin/unpin. The 2D driver can't really deal with it since it won't know when it's safe to unpin the old (current) buffer, though it can pin the new one. Maybe the 2D driver should pin the new one and the kernel should be responsible for unpinning the old? And of course we should add a check to make sure the new one is pinned in the ioctl, and return -EINVAL if it isn't... -- Jesse Barnes, Intel Open Source Technology Center |