From dac3bcb414a21a77847c96740a1578f3488c774f Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Wed, 11 Jun 2008 11:28:20 -0700 Subject: [gem] Remove carefully-sprinkled i915_kernel_lost_context(). They are not unnecessary since the kernel's the only thing touching the ring. --- linux-core/i915_gem.c | 5 ----- 1 file changed, 5 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 2564f418..3d47ec1a 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1404,7 +1404,6 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, DRM_INFO("buffers_ptr %d buffer_count %d len %08x\n", (int) args->buffers_ptr, args->buffer_count, args->batch_len); #endif - i915_kernel_lost_context(dev); /* Copy in the exec list from userland */ exec_list = drm_calloc(sizeof(*exec_list), args->buffer_count, @@ -1614,7 +1613,6 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data, mutex_lock(&dev->struct_mutex); - i915_kernel_lost_context(dev); obj = drm_gem_object_lookup(dev, file_priv, args->handle); if (obj == NULL) { DRM_ERROR("Bad handle in i915_gem_pin_ioctl(): %d\n", @@ -1647,7 +1645,6 @@ i915_gem_unpin_ioctl(struct drm_device *dev, void *data, mutex_lock(&dev->struct_mutex); - i915_kernel_lost_context(dev); obj = drm_gem_object_lookup(dev, file_priv, args->handle); if (obj == NULL) { DRM_ERROR("Bad handle in i915_gem_unpin_ioctl(): %d\n", @@ -1711,7 +1708,6 @@ int i915_gem_init_object(struct drm_gem_object *obj) void i915_gem_free_object(struct drm_gem_object *obj) { - i915_kernel_lost_context(obj->dev); i915_gem_object_unbind(obj); drm_free(obj->driver_private, 1, DRM_MEM_DRIVER); @@ -1729,7 +1725,6 @@ i915_gem_set_domain(struct drm_gem_object *obj, BUG_ON(!mutex_is_locked(&dev->struct_mutex)); drm_client_lock_take(dev, file_priv); - i915_kernel_lost_context(dev); ret = i915_gem_object_set_domain(obj, read_domains, write_domain); if (ret) { drm_client_lock_release(dev); -- cgit v1.2.3 From 2655005762b8915d5f44d1d1ee7e6c2eb34841d7 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Wed, 11 Jun 2008 14:42:40 -0700 Subject: [gem] Move potentially device-specific ioctls to the intel driver. This is the create (may want location flags), pread/pwrite/mmap (performance tuning hints), and set_domain (will 32 bits be enough for everyone?) ioctls. Left in the generic set are just flink/open/close. The 2D driver must be updated for this change, and API but not ABI is broken for 3D. The driver version is bumped to mark this. --- linux-core/i915_gem.c | 281 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 231 insertions(+), 50 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 3d47ec1a..e51de316 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -40,6 +40,11 @@ static int i915_gem_object_set_domain(struct drm_gem_object *obj, uint32_t read_domains, uint32_t write_domain); +int +i915_gem_set_domain(struct drm_gem_object *obj, + struct drm_file *file_priv, + uint32_t read_domains, + uint32_t write_domain); int i915_gem_init_ioctl(struct drm_device *dev, void *data, @@ -65,6 +70,199 @@ i915_gem_init_ioctl(struct drm_device *dev, void *data, return 0; } + +/** + * Creates a new mm object and returns a handle to it. + */ +int +i915_gem_create_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_i915_gem_create *args = data; + struct drm_gem_object *obj; + int handle, ret; + + args->size = roundup(args->size, PAGE_SIZE); + + /* Allocate the new object */ + obj = drm_gem_object_alloc(dev, args->size); + if (obj == NULL) + return -ENOMEM; + + ret = drm_gem_handle_create(file_priv, obj, &handle); + mutex_lock(&dev->struct_mutex); + drm_gem_object_handle_unreference(obj); + mutex_unlock(&dev->struct_mutex); + + if (ret) + return ret; + + args->handle = handle; + + return 0; +} + +/** + * Reads data from the object referenced by handle. + * + * On error, the contents of *data are undefined. + */ +int +i915_gem_pread_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_i915_gem_pread *args = data; + struct drm_gem_object *obj; + ssize_t read; + loff_t offset; + int ret; + + obj = drm_gem_object_lookup(dev, file_priv, args->handle); + if (obj == NULL) + return -EINVAL; + + mutex_lock(&dev->struct_mutex); + ret = i915_gem_set_domain(obj, file_priv, + I915_GEM_DOMAIN_CPU, 0); + if (ret) { + drm_gem_object_unreference(obj); + mutex_unlock(&dev->struct_mutex); + return ret; + } + offset = args->offset; + + read = vfs_read(obj->filp, (char __user *)(uintptr_t)args->data_ptr, + args->size, &offset); + if (read != args->size) { + drm_gem_object_unreference(obj); + mutex_unlock(&dev->struct_mutex); + if (read < 0) + return read; + else + return -EINVAL; + } + + drm_gem_object_unreference(obj); + mutex_unlock(&dev->struct_mutex); + + return 0; +} + +/** + * Writes data to the object referenced by handle. + * + * On error, the contents of the buffer that were to be modified are undefined. + */ +int +i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_i915_gem_pwrite *args = data; + struct drm_gem_object *obj; + ssize_t written; + loff_t offset; + int ret; + + obj = drm_gem_object_lookup(dev, file_priv, args->handle); + if (obj == NULL) + return -EINVAL; + + mutex_lock(&dev->struct_mutex); + ret = i915_gem_set_domain(obj, file_priv, + I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU); + if (ret) { + drm_gem_object_unreference(obj); + mutex_unlock(&dev->struct_mutex); + return ret; + } + offset = args->offset; + + written = vfs_write(obj->filp, + (char __user *)(uintptr_t) args->data_ptr, + args->size, &offset); + + if (written != args->size) { + drm_gem_object_unreference(obj); + mutex_unlock(&dev->struct_mutex); + if (written < 0) + return written; + else + return -EINVAL; + } + + drm_gem_object_unreference(obj); + mutex_unlock(&dev->struct_mutex); + + return 0; +} + +/** + * Called when user space prepares to use an object + */ +int +i915_gem_set_domain_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_i915_gem_set_domain *args = data; + struct drm_gem_object *obj; + int ret; + + if (!(dev->driver->driver_features & DRIVER_GEM)) + return -ENODEV; + + obj = drm_gem_object_lookup(dev, file_priv, args->handle); + if (obj == NULL) + return -EINVAL; + + mutex_lock(&dev->struct_mutex); + ret = i915_gem_set_domain(obj, file_priv, + args->read_domains, args->write_domain); + drm_gem_object_unreference(obj); + mutex_unlock(&dev->struct_mutex); + return ret; +} + +/** + * Maps the contents of an object, returning the address it is mapped + * into. + * + * While the mapping holds a reference on the contents of the object, it doesn't + * imply a ref on the object itself. + */ +int +i915_gem_mmap_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_i915_gem_mmap *args = data; + struct drm_gem_object *obj; + loff_t offset; + unsigned long addr; + + if (!(dev->driver->driver_features & DRIVER_GEM)) + return -ENODEV; + + obj = drm_gem_object_lookup(dev, file_priv, args->handle); + if (obj == NULL) + return -EINVAL; + + offset = args->offset; + + down_write(¤t->mm->mmap_sem); + addr = do_mmap(obj->filp, 0, args->size, + PROT_READ | PROT_WRITE, MAP_SHARED, + args->offset); + up_write(¤t->mm->mmap_sem); + mutex_lock(&dev->struct_mutex); + drm_gem_object_unreference(obj); + mutex_unlock(&dev->struct_mutex); + if (IS_ERR((void *)addr)) + return addr; + + args->addr_ptr = (uint64_t) addr; + + return 0; +} + static void i915_gem_object_free_page_list(struct drm_gem_object *obj) { @@ -187,7 +385,7 @@ i915_retire_commands(struct drm_device *dev) /* The sampler always gets flushed on i965 (sigh) */ if (IS_I965G(dev)) - flush_domains |= DRM_GEM_DOMAIN_I915_SAMPLER; + flush_domains |= I915_GEM_DOMAIN_SAMPLER; BEGIN_LP_RING(2); OUT_RING(cmd); OUT_RING(0); /* noop */ @@ -377,51 +575,51 @@ i915_gem_flush(struct drm_device *dev, invalidate_domains, flush_domains); #endif - if (flush_domains & DRM_GEM_DOMAIN_CPU) + if (flush_domains & I915_GEM_DOMAIN_CPU) drm_agp_chipset_flush(dev); - if ((invalidate_domains|flush_domains) & ~DRM_GEM_DOMAIN_CPU) { + if ((invalidate_domains|flush_domains) & ~I915_GEM_DOMAIN_CPU) { /* * read/write caches: * - * DRM_GEM_DOMAIN_I915_RENDER is always invalidated, but is + * I915_GEM_DOMAIN_RENDER is always invalidated, but is * only flushed if MI_NO_WRITE_FLUSH is unset. On 965, it is * also flushed at 2d versus 3d pipeline switches. * * read-only caches: * - * DRM_GEM_DOMAIN_I915_SAMPLER is flushed on pre-965 if + * I915_GEM_DOMAIN_SAMPLER is flushed on pre-965 if * MI_READ_FLUSH is set, and is always flushed on 965. * - * DRM_GEM_DOMAIN_I915_COMMAND may not exist? + * I915_GEM_DOMAIN_COMMAND may not exist? * - * DRM_GEM_DOMAIN_I915_INSTRUCTION, which exists on 965, is + * I915_GEM_DOMAIN_INSTRUCTION, which exists on 965, is * invalidated when MI_EXE_FLUSH is set. * - * DRM_GEM_DOMAIN_I915_VERTEX, which exists on 965, is + * I915_GEM_DOMAIN_VERTEX, which exists on 965, is * invalidated with every MI_FLUSH. * * TLBs: * - * On 965, TLBs associated with DRM_GEM_DOMAIN_I915_COMMAND - * and DRM_GEM_DOMAIN_CPU in are invalidated at PTE write and - * DRM_GEM_DOMAIN_I915_RENDER and DRM_GEM_DOMAIN_I915_SAMPLER + * On 965, TLBs associated with I915_GEM_DOMAIN_COMMAND + * and I915_GEM_DOMAIN_CPU in are invalidated at PTE write and + * I915_GEM_DOMAIN_RENDER and I915_GEM_DOMAIN_SAMPLER * are flushed at any MI_FLUSH. */ cmd = CMD_MI_FLUSH | MI_NO_WRITE_FLUSH; if ((invalidate_domains|flush_domains) & - DRM_GEM_DOMAIN_I915_RENDER) + I915_GEM_DOMAIN_RENDER) cmd &= ~MI_NO_WRITE_FLUSH; if (!IS_I965G(dev)) { /* * On the 965, the sampler cache always gets flushed * and this bit is reserved. */ - if (invalidate_domains & DRM_GEM_DOMAIN_I915_SAMPLER) + if (invalidate_domains & I915_GEM_DOMAIN_SAMPLER) cmd |= MI_READ_FLUSH; } - if (invalidate_domains & DRM_GEM_DOMAIN_I915_INSTRUCTION) + if (invalidate_domains & I915_GEM_DOMAIN_INSTRUCTION) cmd |= MI_EXE_FLUSH; #if WATCH_EXEC @@ -448,7 +646,7 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) /* If there are writes queued to the buffer, flush and * create a new seqno to wait for. */ - if (obj->write_domain & ~(DRM_GEM_DOMAIN_CPU)) { + if (obj->write_domain & ~(I915_GEM_DOMAIN_CPU)) { uint32_t write_domain = obj->write_domain; #if WATCH_BUF DRM_INFO("%s: flushing object %p from write domain %08x\n", @@ -503,8 +701,8 @@ i915_gem_object_unbind(struct drm_gem_object *obj) * also ensure that all pending GPU writes are finished * before we unbind. */ - ret = i915_gem_object_set_domain (obj, DRM_GEM_DOMAIN_CPU, - DRM_GEM_DOMAIN_CPU); + ret = i915_gem_object_set_domain (obj, I915_GEM_DOMAIN_CPU, + I915_GEM_DOMAIN_CPU); if (ret) return ret; @@ -805,8 +1003,8 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) * wasn't in the GTT, there shouldn't be any way it could have been in * a GPU cache */ - BUG_ON(obj->read_domains & ~DRM_GEM_DOMAIN_CPU); - BUG_ON(obj->write_domain & ~DRM_GEM_DOMAIN_CPU); + BUG_ON(obj->read_domains & ~I915_GEM_DOMAIN_CPU); + BUG_ON(obj->write_domain & ~I915_GEM_DOMAIN_CPU); return 0; } @@ -973,7 +1171,7 @@ i915_gem_object_set_domain(struct drm_gem_object *obj, * stale data. That is, any new read domains. */ invalidate_domains |= read_domains & ~obj->read_domains; - if ((flush_domains | invalidate_domains) & DRM_GEM_DOMAIN_CPU) { + if ((flush_domains | invalidate_domains) & I915_GEM_DOMAIN_CPU) { #if WATCH_BUF DRM_INFO("%s: CPU domain flush %08x invalidate %08x\n", __func__, flush_domains, invalidate_domains); @@ -983,8 +1181,8 @@ i915_gem_object_set_domain(struct drm_gem_object *obj, * then pause for rendering so that the GPU caches will be * flushed before the cpu cache is invalidated */ - if ((invalidate_domains & DRM_GEM_DOMAIN_CPU) && - (flush_domains & ~DRM_GEM_DOMAIN_CPU)) { + if ((invalidate_domains & I915_GEM_DOMAIN_CPU) && + (flush_domains & ~I915_GEM_DOMAIN_CPU)) { ret = i915_gem_object_wait_rendering(obj); if (ret) return ret; @@ -1225,7 +1423,7 @@ i915_gem_object_bind_and_relocate(struct drm_gem_object *obj, /* As we're writing through the gtt, flush * any CPU writes before we write the relocations */ - if (obj->write_domain & DRM_GEM_DOMAIN_CPU) { + if (obj->write_domain & I915_GEM_DOMAIN_CPU) { i915_gem_clflush_object(obj); drm_agp_chipset_flush(dev); obj->write_domain = 0; @@ -1466,7 +1664,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, /* Set the pending read domains for the batch buffer to COMMAND */ batch_obj = object_list[args->buffer_count-1]; - batch_obj->pending_read_domains = DRM_GEM_DOMAIN_I915_COMMAND; + batch_obj->pending_read_domains = I915_GEM_DOMAIN_COMMAND; batch_obj->pending_write_domain = 0; for (i = 0; i < args->buffer_count; i++) { @@ -1700,6 +1898,15 @@ int i915_gem_init_object(struct drm_gem_object *obj) if (obj_priv == NULL) return -ENOMEM; + /* + * We've just allocated pages from the kernel, + * so they've just been written by the CPU with + * zeros. They'll need to be clflushed before we + * use them with the GPU. + */ + obj->write_domain = I915_GEM_DOMAIN_CPU; + obj->read_domains = I915_GEM_DOMAIN_CPU; + obj->driver_private = obj_priv; obj_priv->obj = obj; INIT_LIST_HEAD(&obj_priv->list); @@ -1735,32 +1942,6 @@ i915_gem_set_domain(struct drm_gem_object *obj, return 0; } -int -i915_gem_flush_pwrite(struct drm_gem_object *obj, - uint64_t offset, uint64_t size) -{ -#if 0 - struct drm_device *dev = obj->dev; - struct drm_i915_gem_object *obj_priv = obj->driver_private; - - /* - * For writes much less than the size of the object and - * which are already pinned in memory, do the flush right now - */ - - if ((size < obj->size >> 1) && obj_priv->page_list != NULL) { - unsigned long first_page = offset / PAGE_SIZE; - unsigned long beyond_page = roundup(offset + size, PAGE_SIZE) / PAGE_SIZE; - - drm_ttm_cache_flush(obj_priv->page_list + first_page, - beyond_page - first_page); - drm_agp_chipset_flush(dev); - obj->write_domain = 0; - } -#endif - return 0; -} - void i915_gem_lastclose(struct drm_device *dev) { -- cgit v1.2.3 From 846d792ac10c4b2738bb5ff59e56df168b9921ff Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Wed, 11 Jun 2008 15:51:17 -0700 Subject: [gem] Another round of cleanups from checkpatch.pl --- linux-core/i915_gem.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index e51de316..d608cb03 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -547,8 +547,8 @@ i915_wait_request(struct drm_device *dev, uint32_t seqno) i915_user_irq_off(dev_priv); } if (ret) - DRM_ERROR ("%s returns %d (awaiting %d at %d)\n", - __func__, ret, seqno, i915_get_gem_seqno(dev)); + DRM_ERROR("%s returns %d (awaiting %d at %d)\n", + __func__, ret, seqno, i915_get_gem_seqno(dev)); /* Directly dispatch request retiring. While we have the work queue * to handle this, the waiter on a request often wants an associated @@ -701,8 +701,8 @@ i915_gem_object_unbind(struct drm_gem_object *obj) * also ensure that all pending GPU writes are finished * before we unbind. */ - ret = i915_gem_object_set_domain (obj, I915_GEM_DOMAIN_CPU, - I915_GEM_DOMAIN_CPU); + ret = i915_gem_object_set_domain(obj, I915_GEM_DOMAIN_CPU, + I915_GEM_DOMAIN_CPU); if (ret) return ret; @@ -1242,7 +1242,7 @@ i915_gem_object_check_coherency(struct drm_gem_object *obj, int handle) int bad_count = 0; DRM_INFO("%s: checking coherency of object %p@0x%08x (%d, %dkb):\n", - __FUNCTION__, obj, obj_priv->gtt_offset, handle, + __func__, obj, obj_priv->gtt_offset, handle, obj->size / 1024); gtt_mapping = ioremap(dev->agp->base + obj_priv->gtt_offset, @@ -1301,9 +1301,7 @@ i915_gem_object_check_coherency(struct drm_gem_object *obj, int handle) #endif /** - * Bind an object to the GTT and evaluate the relocations landing in it - * - * + * Bind an object to the GTT and evaluate the relocations landing in it. */ static int i915_gem_object_bind_and_relocate(struct drm_gem_object *obj, @@ -1567,7 +1565,8 @@ i915_gem_ring_throttle(struct drm_device *dev) list); /* Break out if we're close enough. */ - if ((long) (jiffies - request->emitted_jiffies) <= (20 * HZ) / 1000) { + if ((long) (jiffies - request->emitted_jiffies) <= + (20 * HZ) / 1000) { mutex_unlock(&dev->struct_mutex); return 0; } @@ -1877,7 +1876,7 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, obj_priv = obj->driver_private; args->busy = obj_priv->active; - + drm_gem_object_unreference(obj); mutex_unlock(&dev->struct_mutex); return 0; -- cgit v1.2.3 From b2606e325ac02782297def5ce27028c7fe2287c8 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Wed, 11 Jun 2008 16:19:23 -0700 Subject: [gem] Remove the drm_client_lock_take in set_domain. We no longer need to use it to protect against shared ringbuffer access. --- linux-core/i915_gem.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index d608cb03..030ecf1d 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1930,14 +1930,11 @@ i915_gem_set_domain(struct drm_gem_object *obj, BUG_ON(!mutex_is_locked(&dev->struct_mutex)); - drm_client_lock_take(dev, file_priv); ret = i915_gem_object_set_domain(obj, read_domains, write_domain); - if (ret) { - drm_client_lock_release(dev); + if (ret) return ret; - } i915_gem_dev_set_domain(obj->dev); - drm_client_lock_release(dev); + return 0; } -- cgit v1.2.3 From c892e26bdfcacfe7213085a08dd82e2cb7faa003 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 13 Jun 2008 09:49:05 -0700 Subject: [gem] Don't require the lock in execbuf now that it's not needed for the ring. --- linux-core/i915_gem.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 030ecf1d..634b4f6c 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1595,8 +1595,6 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, uint64_t exec_offset; uint32_t seqno, flush_domains; - LOCK_TEST_WITH_RETURN(dev, file_priv); - #if WATCH_EXEC DRM_INFO("buffers_ptr %d buffer_count %d len %08x\n", (int) args->buffers_ptr, args->buffer_count, args->batch_len); -- cgit v1.2.3 From e5364914ac2b785f9d806c72fff8d2ae914cad61 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Tue, 10 Jun 2008 18:11:15 -0700 Subject: [intel-gem] Reorder i915_add_request to schedule work last i915_add_request was calling schedule_delayed_work before adding the request to the list; it makes more sense to do that last. --- linux-core/i915_gem.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 634b4f6c..a14781f6 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -333,6 +333,7 @@ i915_add_request(struct drm_device *dev, uint32_t flush_domains) drm_i915_private_t *dev_priv = dev->dev_private; struct drm_i915_gem_request *request; uint32_t seqno; + int was_empty; RING_LOCALS; request = drm_calloc(1, sizeof(*request), DRM_MEM_DRIVER); @@ -360,11 +361,11 @@ i915_add_request(struct drm_device *dev, uint32_t flush_domains) request->seqno = seqno; request->emitted_jiffies = jiffies; request->flush_domains = flush_domains; - if (list_empty(&dev_priv->mm.request_list)) - mod_timer(&dev_priv->mm.retire_timer, jiffies + HZ); - + was_empty = list_empty(&dev_priv->mm.request_list); list_add_tail(&request->list, &dev_priv->mm.request_list); + if (was_empty) + schedule_delayed_work (&dev_priv->mm.retire_work, HZ); return seqno; } -- cgit v1.2.3 From 462af73149b9286a74b95b9cda5e4224ebe0dd87 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 13 Jun 2008 09:19:30 -0700 Subject: [intel-gem] Use a delayed_work instead of a timer + work_struct We want request retirement to occur about once a second when the request queue is non-empty. This was done with a timer that queued a work_struct, using a delayed_work instead makes a lot more sense. --- linux-core/i915_gem.c | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index a14781f6..115f8e75 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -503,28 +503,19 @@ i915_gem_retire_requests(struct drm_device *dev) } void -i915_gem_retire_timeout(unsigned long data) -{ - struct drm_device *dev = (struct drm_device *) data; - drm_i915_private_t *dev_priv = dev->dev_private; - - schedule_work(&dev_priv->mm.retire_task); -} - -void -i915_gem_retire_handler(struct work_struct *work) +i915_gem_retire_work_handler(struct work_struct *work) { drm_i915_private_t *dev_priv; struct drm_device *dev; dev_priv = container_of(work, drm_i915_private_t, - mm.retire_task); + mm.retire_work.work); dev = dev_priv->dev; mutex_lock(&dev->struct_mutex); i915_gem_retire_requests(dev); if (!list_empty(&dev_priv->mm.request_list)) - mod_timer(&dev_priv->mm.retire_timer, jiffies + HZ); + schedule_delayed_work (&dev_priv->mm.retire_work, HZ); mutex_unlock(&dev->struct_mutex); } -- cgit v1.2.3 From 6b2cba1ecc5f9f289b5d91e229b7f7b0999bee5b Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 13 Jun 2008 11:33:27 -0700 Subject: [intel-gem] evict_something was failing when wait_request freed objects When i915_wait_request clears object from the active list, it may end up freeing them and not moving them to the inactive list. This ends up unbinding objects from the GTT without there ever being new objects visible to i915_gem_evict_something on the inactive list. As the only success condition required the presence of objects on the inactive list, this would falsely assume that no GTT space had been made available, and end up returning -ENOMEM to the application. --- linux-core/i915_gem.c | 39 ++++++++++++++++++++++----------------- 1 file changed, 22 insertions(+), 17 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 115f8e75..b114192d 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -805,7 +805,7 @@ i915_gem_evict_something(struct drm_device *dev) drm_i915_private_t *dev_priv = dev->dev_private; struct drm_gem_object *obj; struct drm_i915_gem_object *obj_priv; - int ret; + int ret = 0; for (;;) { /* If there's an inactive buffer available now, grab it @@ -817,6 +817,13 @@ i915_gem_evict_something(struct drm_device *dev) list); obj = obj_priv->obj; BUG_ON(obj_priv->pin_count != 0); +#if WATCH_LRU + DRM_INFO("%s: evicting %p\n", __func__, obj); +#endif + BUG_ON(obj_priv->active); + + /* Wait on the rendering and unbind the buffer. */ + ret = i915_gem_object_unbind(obj); break; } @@ -826,17 +833,21 @@ i915_gem_evict_something(struct drm_device *dev) */ if (!list_empty(&dev_priv->mm.request_list)) { struct drm_i915_gem_request *request; - int ret; request = list_first_entry(&dev_priv->mm.request_list, struct drm_i915_gem_request, list); ret = i915_wait_request(dev, request->seqno); - if (ret != 0) - return ret; - - continue; + + /* if waiting caused an object to become inactive, + * then loop around and wait for it. Otherwise, we + * assume that waiting freed and unbound something, + * so there should now be some space in the GTT + */ + if (!list_empty(&dev_priv->mm.inactive_list)) + continue; + break; } /* If we didn't have anything on the request list but there @@ -859,21 +870,15 @@ i915_gem_evict_something(struct drm_device *dev) continue; } + DRM_ERROR("inactive empty %d request empty %d flushing empty %d\n", + list_empty(&dev_priv->mm.inactive_list), + list_empty(&dev_priv->mm.request_list), + list_empty(&dev_priv->mm.flushing_list)); /* If we didn't do any of the above, there's nothing to be done * and we just can't fit it in. */ return -ENOMEM; } - -#if WATCH_LRU - DRM_INFO("%s: evicting %p\n", __func__, obj); -#endif - - BUG_ON(obj_priv->active); - - /* Wait on the rendering and unbind the buffer. */ - ret = i915_gem_object_unbind(obj); - return ret; } @@ -959,7 +964,7 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) ret = i915_gem_evict_something(dev); if (ret != 0) { - DRM_ERROR("Failed to evict a buffer\n"); + DRM_ERROR("Failed to evict a buffer %d\n", ret); return ret; } goto search_free; -- cgit v1.2.3 From ced9ebf64543b4d64a38feee3293040af953acc0 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 13 Jun 2008 12:06:13 -0700 Subject: [intel-gem] throttle based on frames rather than time. Reduces jitter. Record the last execbuffer sequence for each client. Record that sequence in the throttle ioctl as the 'throttle sequence'. Wait for the last throttle sequence in the throttle ioctl. --- linux-core/i915_gem.c | 34 ++++++++++------------------------ 1 file changed, 10 insertions(+), 24 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index b114192d..c2d1fab6 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1548,33 +1548,17 @@ i915_dispatch_gem_execbuffer(struct drm_device *dev, * relatively low latency when blocking on a particular request to finish. */ static int -i915_gem_ring_throttle(struct drm_device *dev) +i915_gem_ring_throttle(struct drm_device *dev, struct drm_file *file_priv) { - drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_file_private *i915_file_priv = file_priv->driver_priv; int ret = 0; + uint32_t seqno; mutex_lock(&dev->struct_mutex); - while (!list_empty(&dev_priv->mm.request_list)) { - struct drm_i915_gem_request *request; - - request = list_first_entry(&dev_priv->mm.request_list, - struct drm_i915_gem_request, - list); - - /* Break out if we're close enough. */ - if ((long) (jiffies - request->emitted_jiffies) <= - (20 * HZ) / 1000) { - mutex_unlock(&dev->struct_mutex); - return 0; - } - - /* Wait on the last request if not. */ - ret = i915_wait_request(dev, request->seqno); - if (ret != 0) { - mutex_unlock(&dev->struct_mutex); - return ret; - } - } + seqno = i915_file_priv->mm.last_gem_throttle_seqno; + i915_file_priv->mm.last_gem_throttle_seqno = i915_file_priv->mm.last_gem_seqno; + if (seqno) + ret = i915_wait_request(dev, seqno); mutex_unlock(&dev->struct_mutex); return ret; } @@ -1584,6 +1568,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, struct drm_file *file_priv) { drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_file_private *i915_file_priv = file_priv->driver_priv; struct drm_i915_gem_execbuffer *args = data; struct drm_i915_gem_exec_object *exec_list = NULL; struct drm_gem_object **object_list = NULL; @@ -1724,6 +1709,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, */ seqno = i915_add_request(dev, flush_domains); BUG_ON(seqno == 0); + i915_file_priv->mm.last_gem_seqno = seqno; for (i = 0; i < args->buffer_count; i++) { struct drm_gem_object *obj = object_list[i]; struct drm_i915_gem_object *obj_priv = obj->driver_private; @@ -1881,7 +1867,7 @@ int i915_gem_throttle_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { - return i915_gem_ring_throttle(dev); + return i915_gem_ring_throttle(dev, file_priv); } int i915_gem_init_object(struct drm_gem_object *obj) -- cgit v1.2.3 From baf521369478eff2842b99feda16f9d145402d27 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 13 Jun 2008 14:28:18 -0700 Subject: [intel-gem] Pin objects during execbuffer Pinning the objects avoids accidentally evicting them while binding other objects. --- linux-core/i915_gem.c | 76 +++++++++++++++++++++++++++++++++++---------------- 1 file changed, 52 insertions(+), 24 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index c2d1fab6..d14b2f2d 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1298,27 +1298,25 @@ i915_gem_object_check_coherency(struct drm_gem_object *obj, int handle) #endif /** - * Bind an object to the GTT and evaluate the relocations landing in it. + * Pin an object to the GTT and evaluate the relocations landing in it. */ static int -i915_gem_object_bind_and_relocate(struct drm_gem_object *obj, - struct drm_file *file_priv, - struct drm_i915_gem_exec_object *entry) +i915_gem_object_pin_and_relocate(struct drm_gem_object *obj, + struct drm_file *file_priv, + struct drm_i915_gem_exec_object *entry) { struct drm_device *dev = obj->dev; struct drm_i915_gem_relocation_entry reloc; struct drm_i915_gem_relocation_entry __user *relocs; struct drm_i915_gem_object *obj_priv = obj->driver_private; - int i; + int i, ret; uint32_t last_reloc_offset = -1; void *reloc_page = NULL; /* Choose the GTT offset for our buffer and put it there. */ - if (obj_priv->gtt_space == NULL) { - i915_gem_object_bind_to_gtt(obj, (unsigned) entry->alignment); - if (obj_priv->gtt_space == NULL) - return -ENOMEM; - } + ret = i915_gem_object_pin(obj, (uint32_t) entry->alignment); + if (ret) + return ret; entry->offset = obj_priv->gtt_offset; @@ -1334,13 +1332,17 @@ i915_gem_object_bind_and_relocate(struct drm_gem_object *obj, int ret; ret = copy_from_user(&reloc, relocs + i, sizeof(reloc)); - if (ret != 0) + if (ret != 0) { + i915_gem_object_unpin(obj); return ret; + } target_obj = drm_gem_object_lookup(obj->dev, file_priv, reloc.target_handle); - if (target_obj == NULL) + if (target_obj == NULL) { + i915_gem_object_unpin(obj); return -EINVAL; + } target_obj_priv = target_obj->driver_private; /* The target buffer should have appeared before us in the @@ -1350,6 +1352,7 @@ i915_gem_object_bind_and_relocate(struct drm_gem_object *obj, DRM_ERROR("No GTT space found for object %d\n", reloc.target_handle); drm_gem_object_unreference(target_obj); + i915_gem_object_unpin(obj); return -EINVAL; } @@ -1359,6 +1362,7 @@ i915_gem_object_bind_and_relocate(struct drm_gem_object *obj, obj, reloc.target_handle, (int) reloc.offset, (int) obj->size); drm_gem_object_unreference(target_obj); + i915_gem_object_unpin(obj); return -EINVAL; } if (reloc.offset & 3) { @@ -1367,6 +1371,7 @@ i915_gem_object_bind_and_relocate(struct drm_gem_object *obj, obj, reloc.target_handle, (int) reloc.offset); drm_gem_object_unreference(target_obj); + i915_gem_object_unpin(obj); return -EINVAL; } @@ -1380,6 +1385,7 @@ i915_gem_object_bind_and_relocate(struct drm_gem_object *obj, reloc.write_domain, target_obj->pending_write_domain); drm_gem_object_unreference(target_obj); + i915_gem_object_unpin(obj); return -EINVAL; } @@ -1440,6 +1446,7 @@ i915_gem_object_bind_and_relocate(struct drm_gem_object *obj, last_reloc_offset = reloc_offset; if (reloc_page == NULL) { drm_gem_object_unreference(target_obj); + i915_gem_object_unpin(obj); return -ENOMEM; } } @@ -1462,6 +1469,7 @@ i915_gem_object_bind_and_relocate(struct drm_gem_object *obj, ret = copy_to_user(relocs + i, &reloc, sizeof(reloc)); if (ret != 0) { drm_gem_object_unreference(target_obj); + i915_gem_object_unpin(obj); return ret; } @@ -1573,7 +1581,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, struct drm_i915_gem_exec_object *exec_list = NULL; struct drm_gem_object **object_list = NULL; struct drm_gem_object *batch_obj; - int ret, i; + int ret, i, pinned = 0; uint64_t exec_offset; uint32_t seqno, flush_domains; @@ -1632,13 +1640,14 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, object_list[i]->pending_read_domains = 0; object_list[i]->pending_write_domain = 0; - ret = i915_gem_object_bind_and_relocate(object_list[i], - file_priv, - &exec_list[i]); + ret = i915_gem_object_pin_and_relocate(object_list[i], + file_priv, + &exec_list[i]); if (ret) { DRM_ERROR("object bind and relocate failed %d\n", ret); goto err; } + pinned = i; } /* Set the pending read domains for the batch buffer to COMMAND */ @@ -1735,6 +1744,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, args->buffer_count, ret); err: if (object_list != NULL) { + for (i = 0; i < pinned; i++) + i915_gem_object_unpin (object_list[i]); for (i = 0; i < args->buffer_count; i++) drm_gem_object_unreference(object_list[i]); } @@ -1752,32 +1763,47 @@ pre_mutex_err: int i915_gem_object_pin(struct drm_gem_object *obj, uint32_t alignment) { - struct drm_device *dev = obj->dev; struct drm_i915_gem_object *obj_priv = obj->driver_private; int ret; if (obj_priv->gtt_space == NULL) { ret = i915_gem_object_bind_to_gtt(obj, alignment); if (ret != 0) { - DRM_ERROR("Failure to bind in " - "i915_gem_pin_ioctl(): %d\n", - ret); - drm_gem_object_unreference(obj); - mutex_unlock(&dev->struct_mutex); + DRM_ERROR("Failure to bind: %d", ret); return ret; } } - obj_priv->pin_count++; + + /* If the object is not active and not pending a flush, + * remove it from the inactive list + */ + if (obj_priv->pin_count == 1 && + !obj_priv->active && + obj->write_domain == 0) + list_del_init(&obj_priv->list); + return 0; } void i915_gem_object_unpin(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; obj_priv->pin_count--; + BUG_ON(obj_priv->pin_count < 0); + BUG_ON(obj_priv->gtt_space == NULL); + + /* If the object is no longer pinned, and is + * neither active nor being flushed, then stick it on + * the inactive list + */ + if (obj_priv->pin_count == 0 && + !obj_priv->active && obj->write_domain == 0) + list_move_tail(&obj_priv->list, &dev_priv->mm.inactive_list); } int @@ -1963,8 +1989,10 @@ i915_gem_init_ringbuffer(struct drm_device *dev) obj_priv = obj->driver_private; ret = i915_gem_object_pin(obj, 4096); - if (ret != 0) + if (ret != 0) { + drm_gem_object_unreference(obj); return ret; + } /* Set up the kernel mapping for the ring. */ dev_priv->ring.Size = obj->size; -- cgit v1.2.3 From 4086cdb6550a4e957fd436c77a6260204e026538 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 13 Jun 2008 15:38:13 -0700 Subject: [intel-gem] Left the last exec buffer pinned. oops. Loop end variable 'pinned' was set one too low. --- linux-core/i915_gem.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index d14b2f2d..210ae9d9 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1647,7 +1647,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, DRM_ERROR("object bind and relocate failed %d\n", ret); goto err; } - pinned = i; + pinned = i + 1; } /* Set the pending read domains for the batch buffer to COMMAND */ @@ -1745,7 +1745,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, err: if (object_list != NULL) { for (i = 0; i < pinned; i++) - i915_gem_object_unpin (object_list[i]); + i915_gem_object_unpin(object_list[i]); + for (i = 0; i < args->buffer_count; i++) drm_gem_object_unreference(object_list[i]); } -- cgit v1.2.3 From 217beb9c8de01417ac6219b54bd25046da6d4c7a Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 13 Jun 2008 15:43:02 -0700 Subject: [intel-gem] add gtt and pin counts to /proc/dri/*/gem_objects Not quite portable, but these are useful for intel. Some more general mechanism could be done... --- linux-core/i915_gem.c | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 210ae9d9..cb5d663e 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -677,6 +677,7 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) static int i915_gem_object_unbind(struct drm_gem_object *obj) { + struct drm_device *dev = obj->dev; struct drm_i915_gem_object *obj_priv = obj->driver_private; int ret = 0; @@ -706,6 +707,9 @@ i915_gem_object_unbind(struct drm_gem_object *obj) i915_gem_object_free_page_list(obj); + atomic_dec(&dev->gtt_count); + atomic_sub(obj->size, &dev->gtt_memory); + drm_memrange_put_block(obj_priv->gtt_space); obj_priv->gtt_space = NULL; @@ -995,6 +999,8 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) obj_priv->gtt_space = NULL; return -ENOMEM; } + atomic_inc(&dev->gtt_count); + atomic_add(obj->size, &dev->gtt_memory); /* Assert that the object is not currently in any GPU domain. As it * wasn't in the GTT, there shouldn't be any way it could have been in @@ -1764,6 +1770,7 @@ pre_mutex_err: int i915_gem_object_pin(struct drm_gem_object *obj, uint32_t alignment) { + struct drm_device *dev = obj->dev; struct drm_i915_gem_object *obj_priv = obj->driver_private; int ret; @@ -1779,10 +1786,12 @@ i915_gem_object_pin(struct drm_gem_object *obj, uint32_t alignment) /* If the object is not active and not pending a flush, * remove it from the inactive list */ - if (obj_priv->pin_count == 1 && - !obj_priv->active && - obj->write_domain == 0) - list_del_init(&obj_priv->list); + if (obj_priv->pin_count == 1) { + atomic_inc(&dev->pin_count); + atomic_add(obj->size, &dev->pin_memory); + if (!obj_priv->active && obj->write_domain == 0) + list_del_init(&obj_priv->list); + } return 0; } @@ -1802,9 +1811,12 @@ i915_gem_object_unpin(struct drm_gem_object *obj) * neither active nor being flushed, then stick it on * the inactive list */ - if (obj_priv->pin_count == 0 && - !obj_priv->active && obj->write_domain == 0) - list_move_tail(&obj_priv->list, &dev_priv->mm.inactive_list); + if (obj_priv->pin_count == 0) { + if (!obj_priv->active && obj->write_domain == 0) + list_move_tail(&obj_priv->list, &dev_priv->mm.inactive_list); + atomic_dec(&dev->pin_count); + atomic_sub(obj->size, &dev->pin_memory); + } } int -- cgit v1.2.3 From 73bc18cad8d1c6b4481a199cebf7f0a28d19c2bb Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 13 Jun 2008 17:06:35 -0700 Subject: [intel-gem] Wait for rendering to complete before unbinding. Moving to the CPU domain doesn't ensure that rendering is finished, the buffer may still be in use as a texture or other data source. --- linux-core/i915_gem.c | 13 +++++++++++++ 1 file changed, 13 insertions(+) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index cb5d663e..0f037acf 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -688,6 +688,19 @@ i915_gem_object_unbind(struct drm_gem_object *obj) if (obj_priv->gtt_space == NULL) return 0; + if (obj_priv->pin_count != 0) { + DRM_ERROR("Attempting to unbind pinned buffer\n"); + return -EINVAL; + } + + /* Wait for any rendering to complete + */ + ret = i915_gem_object_wait_rendering(obj); + if (ret) { + DRM_ERROR ("wait_rendering failed: %d\n", ret); + return ret; + } + /* Move the object to the CPU domain to ensure that * any possible CPU writes while it's not in the GTT * are flushed when we go to remap it. This will -- cgit v1.2.3 From a7139cb8511a9d31d9f79bcaae62020d30e09f90 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 13 Jun 2008 19:35:22 -0700 Subject: [intel-gem] show total GTT space in /proc/dri/*/gem_objects --- linux-core/i915_gem.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 0f037acf..e07cf1b9 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -65,6 +65,8 @@ i915_gem_init_ioctl(struct drm_device *dev, void *data, drm_memrange_init(&dev_priv->mm.gtt_space, args->gtt_start, args->gtt_end - args->gtt_start); + dev->gtt_total = (uint32_t) (args->gtt_end - args->gtt_start); + mutex_unlock(&dev->struct_mutex); return 0; -- cgit v1.2.3 From 732b1960742042eb33f49c2b3cdd2d36eadbc920 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 13 Jun 2008 19:37:44 -0700 Subject: [intel-gem] whitespace fixes --- linux-core/i915_gem.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index e07cf1b9..78d0d952 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -66,7 +66,7 @@ i915_gem_init_ioctl(struct drm_device *dev, void *data, args->gtt_end - args->gtt_start); dev->gtt_total = (uint32_t) (args->gtt_end - args->gtt_start); - + mutex_unlock(&dev->struct_mutex); return 0; @@ -500,7 +500,7 @@ i915_gem_retire_requests(struct drm_device *dev) list_del(&request->list); drm_free(request, sizeof(*request), DRM_MEM_DRIVER); } else - break; + break; } } @@ -858,7 +858,7 @@ i915_gem_evict_something(struct drm_device *dev) list); ret = i915_wait_request(dev, request->seqno); - + /* if waiting caused an object to become inactive, * then loop around and wait for it. Otherwise, we * assume that waiting freed and unbound something, -- cgit v1.2.3 From 68856b619bc1a2e91e67764911c8af4e2466fad9 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 13 Jun 2008 19:40:16 -0700 Subject: [intel-gem] Debugging -- verify inactive list invariants Inactive list elements may not be pinned, active or have non-CPU write domains. --- linux-core/i915_gem.c | 40 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 39 insertions(+), 1 deletion(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 78d0d952..8741eecd 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -35,6 +35,7 @@ #define WATCH_EXEC 0 #define WATCH_LRU 0 #define WATCH_RELOC 0 +#define WATCH_INACTIVE 0 static int i915_gem_object_set_domain(struct drm_gem_object *obj, @@ -303,6 +304,26 @@ i915_gem_object_move_to_active(struct drm_gem_object *obj) &dev_priv->mm.active_list); } +#if WATCH_INACTIVE +static void +i915_verify_inactive(struct drm_device *dev, char *file, int line) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_gem_object *obj; + struct drm_i915_gem_object *obj_priv; + + list_for_each_entry(obj_priv, &dev_priv->mm.inactive_list, list) { + obj = obj_priv->obj; + if (obj_priv->pin_count || obj_priv->active || (obj->write_domain & ~I915_GEM_DOMAIN_CPU)) + DRM_ERROR("inactive %p (p %d a %d w %x) %s:%d\n", + obj, + obj_priv->pin_count, obj_priv->active, obj->write_domain, file, line); + } +} +#else +#define i915_verify_inactive(dev,file,line) +#endif + static void i915_gem_object_move_to_inactive(struct drm_gem_object *obj) { @@ -310,6 +331,7 @@ i915_gem_object_move_to_inactive(struct drm_gem_object *obj) drm_i915_private_t *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj_priv = obj->driver_private; + i915_verify_inactive(dev, __FILE__, __LINE__); if (obj_priv->pin_count != 0) list_del_init(&obj_priv->list); else @@ -319,6 +341,7 @@ i915_gem_object_move_to_inactive(struct drm_gem_object *obj) obj_priv->active = 0; drm_gem_object_unreference(obj); } + i915_verify_inactive(dev, __FILE__, __LINE__); } /** @@ -1635,6 +1658,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, mutex_lock(&dev->struct_mutex); + i915_verify_inactive(dev, __FILE__, __LINE__); if (dev_priv->mm.suspended) { DRM_ERROR("Execbuf while VT-switched.\n"); mutex_unlock(&dev->struct_mutex); @@ -1676,6 +1700,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, batch_obj->pending_read_domains = I915_GEM_DOMAIN_COMMAND; batch_obj->pending_write_domain = 0; + i915_verify_inactive(dev, __FILE__, __LINE__); + for (i = 0; i < args->buffer_count; i++) { struct drm_gem_object *obj = object_list[i]; struct drm_i915_gem_object *obj_priv = obj->driver_private; @@ -1698,9 +1724,13 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, goto err; } + i915_verify_inactive(dev, __FILE__, __LINE__); + /* Flush/invalidate caches and chipset buffer */ flush_domains = i915_gem_dev_set_domain(dev); + i915_verify_inactive(dev, __FILE__, __LINE__); + #if WATCH_COHERENCY for (i = 0; i < args->buffer_count; i++) { i915_gem_object_check_coherency(object_list[i], @@ -1730,6 +1760,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, */ flush_domains |= i915_retire_commands(dev); + i915_verify_inactive(dev, __FILE__, __LINE__); + /* * Get a seqno representing the execution of the current buffer, * which we can wait on. We would like to mitigate these interrupts, @@ -1754,6 +1786,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, i915_dump_lru(dev, __func__); #endif + i915_verify_inactive(dev, __FILE__, __LINE__); + /* Copy the new buffer offsets back to the user's exec list. */ ret = copy_to_user((struct drm_i915_relocation_entry __user *) (uintptr_t) args->buffers_ptr, @@ -1789,6 +1823,7 @@ i915_gem_object_pin(struct drm_gem_object *obj, uint32_t alignment) struct drm_i915_gem_object *obj_priv = obj->driver_private; int ret; + i915_verify_inactive(dev, __FILE__, __LINE__); if (obj_priv->gtt_space == NULL) { ret = i915_gem_object_bind_to_gtt(obj, alignment); if (ret != 0) { @@ -1807,6 +1842,7 @@ i915_gem_object_pin(struct drm_gem_object *obj, uint32_t alignment) if (!obj_priv->active && obj->write_domain == 0) list_del_init(&obj_priv->list); } + i915_verify_inactive(dev, __FILE__, __LINE__); return 0; } @@ -1818,10 +1854,11 @@ i915_gem_object_unpin(struct drm_gem_object *obj) drm_i915_private_t *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj_priv = obj->driver_private; + i915_verify_inactive(dev, __FILE__, __LINE__); obj_priv->pin_count--; BUG_ON(obj_priv->pin_count < 0); BUG_ON(obj_priv->gtt_space == NULL); - + /* If the object is no longer pinned, and is * neither active nor being flushed, then stick it on * the inactive list @@ -1832,6 +1869,7 @@ i915_gem_object_unpin(struct drm_gem_object *obj) atomic_dec(&dev->pin_count); atomic_sub(obj->size, &dev->pin_memory); } + i915_verify_inactive(dev, __FILE__, __LINE__); } int -- cgit v1.2.3 From 93c2871eccc1abde0d88ea439cf963c4895a26fc Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 13 Jun 2008 19:43:40 -0700 Subject: [intel-gem] BUG_ON active objects in gem_object_unbind Now that gem_object_unbind waits for rendering to complete, objects should not be active when they are being pulled from the GTT. BUG_ON if this is broken. --- linux-core/i915_gem.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 8741eecd..a15d7f23 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -734,8 +734,10 @@ i915_gem_object_unbind(struct drm_gem_object *obj) */ ret = i915_gem_object_set_domain(obj, I915_GEM_DOMAIN_CPU, I915_GEM_DOMAIN_CPU); - if (ret) + if (ret) { + DRM_ERROR("set_domain failed: %d\n", ret); return ret; + } if (obj_priv->agp_mem != NULL) { drm_unbind_agp(obj_priv->agp_mem); @@ -743,6 +745,8 @@ i915_gem_object_unbind(struct drm_gem_object *obj) obj_priv->agp_mem = NULL; } + BUG_ON(obj_priv->active); + i915_gem_object_free_page_list(obj); atomic_dec(&dev->gtt_count); @@ -752,15 +756,9 @@ i915_gem_object_unbind(struct drm_gem_object *obj) obj_priv->gtt_space = NULL; /* Remove ourselves from the LRU list if present. */ - if (!list_empty(&obj_priv->list)) { + if (!list_empty(&obj_priv->list)) list_del_init(&obj_priv->list); - if (obj_priv->active) { - DRM_ERROR("Failed to wait on buffer when unbinding, " - "continued anyway.\n"); - obj_priv->active = 0; - drm_gem_object_unreference(obj); - } - } + return 0; } -- cgit v1.2.3 From 19c3418848ccdbb163cd16b354b14b0559813d6c Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 13 Jun 2008 19:47:23 -0700 Subject: [intel-gem] inactive list may contain objects in CPU write domain Pin/unpin need to know whether to remove/add objects from the inactive list, inactive objects cannot be in any GPU write domain as those would be on the flushing list instead. However, inactive objects may be in the CPU write domain. --- linux-core/i915_gem.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index a15d7f23..5e5c0ea1 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1837,7 +1837,8 @@ i915_gem_object_pin(struct drm_gem_object *obj, uint32_t alignment) if (obj_priv->pin_count == 1) { atomic_inc(&dev->pin_count); atomic_add(obj->size, &dev->pin_memory); - if (!obj_priv->active && obj->write_domain == 0) + if (!obj_priv->active && (obj->write_domain & ~I915_GEM_DOMAIN_CPU) == 0 && + !list_empty(&obj_priv->list)) list_del_init(&obj_priv->list); } i915_verify_inactive(dev, __FILE__, __LINE__); @@ -1862,8 +1863,9 @@ i915_gem_object_unpin(struct drm_gem_object *obj) * the inactive list */ if (obj_priv->pin_count == 0) { - if (!obj_priv->active && obj->write_domain == 0) - list_move_tail(&obj_priv->list, &dev_priv->mm.inactive_list); + if (!obj_priv->active && (obj->write_domain & ~I915_GEM_DOMAIN_CPU) == 0) + list_move_tail(&obj_priv->list, + &dev_priv->mm.inactive_list); atomic_dec(&dev->pin_count); atomic_sub(obj->size, &dev->pin_memory); } -- cgit v1.2.3 From 3e48e144992fb11b31875989d45bc8a7c041cdef Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 13 Jun 2008 19:49:47 -0700 Subject: [intel-gem] Execute MI_FLUSH in leavevt_ioctl In leavevt_ioctl, queue an MI_FLUSH and then block waiting for it to complete. This will empty the active and flushing lists. That leaves only the inactive list to evict. --- linux-core/i915_gem.c | 52 ++++++++++++++++++++++++++++++++------------------- 1 file changed, 33 insertions(+), 19 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 5e5c0ea1..308674d8 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -2127,6 +2127,10 @@ i915_gem_entervt_ioctl(struct drm_device *dev, void *data, return ret; mutex_lock(&dev->struct_mutex); + BUG_ON(!list_empty(&dev_priv->mm.active_list)); + BUG_ON(!list_empty(&dev_priv->mm.flushing_list)); + BUG_ON(!list_empty(&dev_priv->mm.inactive_list)); + BUG_ON(!list_empty(&dev_priv->mm.request_list)); dev_priv->mm.suspended = 0; mutex_unlock(&dev->struct_mutex); return 0; @@ -2170,6 +2174,8 @@ i915_gem_leavevt_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { drm_i915_private_t *dev_priv = dev->dev_private; + uint32_t seqno; + int ret; mutex_lock(&dev->struct_mutex); /* Hack! Don't let anybody do execbuf while we don't control the chip. @@ -2177,32 +2183,40 @@ i915_gem_leavevt_ioctl(struct drm_device *dev, void *data, */ dev_priv->mm.suspended = 1; - /* Move all buffers out of the GTT. */ - i915_gem_evict_from_list(dev, &dev_priv->mm.active_list); - i915_gem_evict_from_list(dev, &dev_priv->mm.flushing_list); - i915_gem_evict_from_list(dev, &dev_priv->mm.inactive_list); + i915_kernel_lost_context(dev); - /* Make sure the harware's idle. */ - while (!list_empty(&dev_priv->mm.request_list)) { - struct drm_i915_gem_request *request; - int ret; + /* Flush the GPU along with all non-CPU write domains + */ + i915_gem_flush(dev, ~I915_GEM_DOMAIN_CPU, ~I915_GEM_DOMAIN_CPU); + seqno = i915_add_request(dev, ~I915_GEM_DOMAIN_CPU); + if (seqno == 0) { + mutex_unlock(&dev->struct_mutex); + return -ENOMEM; + } + ret = i915_wait_request(dev, seqno); + if (ret) { + mutex_unlock(&dev->struct_mutex); + return ret; + } - request = list_first_entry(&dev_priv->mm.request_list, - struct drm_i915_gem_request, - list); + /* Active and flushing should now be empty as we've + * waited for a sequence higher than any pending execbuffer + */ + BUG_ON(!list_empty(&dev_priv->mm.active_list)); + BUG_ON(!list_empty(&dev_priv->mm.flushing_list)); - ret = i915_wait_request(dev, request->seqno); - if (ret != 0) { - DRM_ERROR("Error waiting for idle at LeaveVT: %d\n", - ret); - mutex_unlock(&dev->struct_mutex); - return ret; - } - } + /* Request should now be empty as we've also waited + * for the last request in the list + */ + BUG_ON(!list_empty(&dev_priv->mm.request_list)); + + /* Move all buffers out of the GTT. */ + i915_gem_evict_from_list(dev, &dev_priv->mm.inactive_list); BUG_ON(!list_empty(&dev_priv->mm.active_list)); BUG_ON(!list_empty(&dev_priv->mm.flushing_list)); BUG_ON(!list_empty(&dev_priv->mm.inactive_list)); + BUG_ON(!list_empty(&dev_priv->mm.request_list)); i915_gem_cleanup_ringbuffer(dev); -- cgit v1.2.3