From 2140e102f942edf7982cee2a3f00caf234551687 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 1 May 2008 11:39:06 -0700 Subject: checkpoint: rename to GEM and a few more i915 bits. --- linux-core/i915_gem.c | 140 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 140 insertions(+) create mode 100644 linux-core/i915_gem.c (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c new file mode 100644 index 00000000..0685a1b9 --- /dev/null +++ b/linux-core/i915_gem.c @@ -0,0 +1,140 @@ +/* + * Copyright © 2008 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the "Software"), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: + * Eric Anholt + * + */ + +#include "drmP.h" +#include "drm.h" +#include "i915_drm.h" +#include "i915_drv.h" + +int +i915_gem_init_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_gem_init *args = data; + + if (args->gtt_start >= args->gtt_end || + (args->gtt_start & (PAGE_SIZE - 1)) != 0 || + (args->gtt_end & (PAGE_SIZE - 1)) != 0) + return -EINVAL; + + drm_memrange_init(&dev_priv->mm.gtt_space, args->gtt_start, + args->gtt_end); + + return 0; +} + +static void +i915_gem_evict_object(struct drm_device *dev, struct drm_gem_object *obj) +{ + +} + +static int +evict_callback(struct drm_memrange_node *node, void *data) +{ + struct drm_device *dev = data; + struct drm_gem_object *obj = node->private; + struct drm_i915_gem_object *obj_priv = obj->driver_private; + + if (obj_priv->pin_count == 0) + i915_gem_evict_object(dev, obj); + + return 0; +} + +int +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_gem_execbuffer *args = data; + struct drm_i915_gem_validate_entry *validate_list; + int ret, i; + RING_LOCALS; + + LOCK_TEST_WITH_RETURN(dev, file_priv); + + /* Big hammer: flush and idle the hardware so we can map things in/out. + */ + BEGIN_LP_RING(2); + OUT_RING(CMD_MI_FLUSH | MI_READ_FLUSH | MI_EXE_FLUSH); + OUT_RING(0); /* noop */ + ADVANCE_LP_RING(); + ret = i915_quiescent(dev); + if (ret != 0) + return ret; + + /* Evict everything so we have space for sure. */ + drm_memrange_for_each(&dev_priv->mm.gtt_space, evict_callback); + + /* Copy in the validate list from userland */ + validate_list = drm_calloc(sizeof(*validate_list), args->buffer_count, + DRM_MEM_DRIVER); + ret = copy_from_user(validate_list, + (struct drm_i915_relocation_entry __user*) + args->buffers, + sizeof(*validate_list) * args->buffer_count); + if (ret != 0) { + drm_free(validate_list, + sizeof(*validate_list) * args->buffer_count, + DRM_MEM_DRIVER); + return ret; + } + + /* Perform the relocations */ + for (i = 0; i < args->buffer_count; i++) { + intel_gem_reloc_and_validate_buffer(dev, &validate_list[i]); + } + + /* Exec the batchbuffer */ + + + drm_free(validate_list, sizeof(*validate_list) * args->buffer_count, + DRM_MEM_DRIVER); + + return 0; +} + +int i915_gem_init_object(struct drm_device *dev, struct drm_gem_object *obj) +{ + struct drm_i915_gem_object *obj_priv; + + obj_priv = drm_calloc(1, sizeof(*obj_priv), DRM_MEM_DRIVER); + if (obj_priv == NULL) + return -ENOMEM; + + obj->driver_private = obj_priv; + + return 0; +} + +void i915_gem_free_object(struct drm_device *dev, struct drm_gem_object *obj) +{ + drm_free(obj->driver_private, 1, DRM_MEM_DRIVER); +} + -- cgit v1.2.3 From 5af87acbc2025b9f72d51b30f176e9c3909695ac Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 1 May 2008 14:20:44 -0700 Subject: checkpoint: gtt binding written. --- linux-core/i915_gem.c | 170 +++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 156 insertions(+), 14 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 0685a1b9..bd030a88 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -49,9 +49,120 @@ i915_gem_init_ioctl(struct drm_device *dev, void *data, } static void -i915_gem_evict_object(struct drm_device *dev, struct drm_gem_object *obj) +i915_gem_object_free_page_list(struct drm_device *dev, + struct drm_gem_object *obj) { - + struct drm_i915_gem_object *obj_priv = obj->driver_private; + int page_count = obj->size / PAGE_SIZE; + int i; + + if (obj_priv->page_list == NULL) + return; + + /* Count how many we had successfully allocated, since release_pages() + * doesn't like NULLs. + */ + for (i = 0; i < obj->size / PAGE_SIZE; i++) { + if (obj_priv->page_list[i] == NULL) + break; + } + release_pages(obj_priv->page_list, i, 0); + + drm_free(obj_priv->page_list, + page_count * sizeof(struct page *), + DRM_MEM_DRIVER); + obj_priv->page_list = NULL; +} + +/** + * Unbinds an object from the GTT aperture. + */ +static void +i915_gem_object_unbind(struct drm_device *dev, struct drm_gem_object *obj) +{ + struct drm_i915_gem_object *obj_priv = obj->driver_private; + + if (obj_priv->agp_mem != NULL) { + drm_unbind_agp(obj_priv->agp_mem); + drm_free_agp(obj_priv->agp_mem, obj->size / PAGE_SIZE); + } + + i915_gem_object_free_page_list(dev, obj); + + drm_memrange_put_block(obj_priv->gtt_space); +} + +/** + * Finds free space in the GTT aperture and binds the object there. + */ +static int +i915_gem_object_bind_to_gtt(struct drm_device *dev, struct drm_gem_object *obj) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_gem_object *obj_priv = obj->driver_private; + struct drm_memrange_node *free_space; + int page_count, i; + + free_space = drm_memrange_search_free(&dev_priv->mm.gtt_space, + obj->size, + PAGE_SIZE, 0); + + obj_priv->gtt_space = drm_memrange_get_block(free_space, + obj->size, + PAGE_SIZE); + + /* Get the list of pages out of our struct file. They'll be pinned + * at this point until we release them. + */ + page_count = obj->size / PAGE_SIZE; + BUG_ON(obj_priv->page_list != NULL); + obj_priv->page_list = drm_calloc(page_count, sizeof(struct page *), + DRM_MEM_DRIVER); + for (i = 0; i < page_count; i++) { + obj_priv->page_list[i] = + find_or_create_page(obj->filp->f_mapping, i, GFP_HIGHUSER); + + if (obj_priv->page_list[i] == NULL) { + i915_gem_object_free_page_list(dev, obj); + return -ENOMEM; + } + } + + /* Create an AGP memory structure pointing at our pages, and bind it + * into the GTT. + */ + obj_priv->agp_mem = drm_agp_bind_pages(dev, + obj_priv->page_list, + page_count, + obj_priv->gtt_offset); + if (obj_priv->agp_mem == NULL) { + i915_gem_object_free_page_list(dev, obj); + return -ENOMEM; + } + + return 0; +} + +static int +i915_gem_reloc_and_validate_object(struct drm_device *dev, + struct drm_file *file_priv, + struct drm_i915_gem_validate_entry *entry, + struct drm_gem_object *obj) +{ + struct drm_i915_gem_reloc *relocs; + struct drm_i915_gem_object *obj_priv = obj->driver_private; + + /* Walk the list of relocations and perform them if necessary. */ + /* XXX */ + + /* Choose the GTT offset for our buffer and put it there. */ + if (obj_priv->gtt_space == NULL) { + i915_gem_object_bind_to_gtt(dev, obj); + if (obj_priv->gtt_space == NULL) + return -ENOMEM; + } + + return 0; } static int @@ -62,18 +173,19 @@ evict_callback(struct drm_memrange_node *node, void *data) struct drm_i915_gem_object *obj_priv = obj->driver_private; if (obj_priv->pin_count == 0) - i915_gem_evict_object(dev, obj); + i915_gem_object_unbind(dev, obj); return 0; } int i915_gem_execbuffer(struct drm_device *dev, void *data, - struct drm_file *file_priv) + struct drm_file *file_priv) { drm_i915_private_t *dev_priv = dev->dev_private; struct drm_i915_gem_execbuffer *args = data; struct drm_i915_gem_validate_entry *validate_list; + struct drm_gem_object **object_list; int ret, i; RING_LOCALS; @@ -90,34 +202,64 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, return ret; /* Evict everything so we have space for sure. */ - drm_memrange_for_each(&dev_priv->mm.gtt_space, evict_callback); + drm_memrange_for_each(&dev_priv->mm.gtt_space, evict_callback, dev); /* Copy in the validate list from userland */ validate_list = drm_calloc(sizeof(*validate_list), args->buffer_count, DRM_MEM_DRIVER); + object_list = drm_calloc(sizeof(*object_list), args->buffer_count, + DRM_MEM_DRIVER); + if (validate_list == NULL || object_list == NULL) { + ret = -ENOMEM; + goto err; + } ret = copy_from_user(validate_list, (struct drm_i915_relocation_entry __user*) args->buffers, sizeof(*validate_list) * args->buffer_count); - if (ret != 0) { - drm_free(validate_list, - sizeof(*validate_list) * args->buffer_count, - DRM_MEM_DRIVER); - return ret; - } + if (ret != 0) + goto err; - /* Perform the relocations */ + /* Look up object handles and perform the relocations */ for (i = 0; i < args->buffer_count; i++) { - intel_gem_reloc_and_validate_buffer(dev, &validate_list[i]); + object_list[i] = drm_gem_object_lookup(dev, file_priv, + validate_list[i].buffer_handle); + if (object_list[i] == NULL) { + ret = -EINVAL; + goto err; + } + + i915_gem_reloc_and_validate_object(dev, file_priv, + &validate_list[i], + object_list[i]); } /* Exec the batchbuffer */ + /* Copy the new buffer offsets back to the user's validate list. */ + for (i = 0; i < args->buffer_count; i++) { + struct drm_i915_gem_object *obj_priv = + object_list[i]->driver_private; + validate_list[i].buffer_offset = obj_priv->gtt_offset; + } + ret = copy_to_user(validate_list, + (struct drm_i915_relocation_entry __user*) + args->buffers, + sizeof(*validate_list) * args->buffer_count); + + /* Clean up and return */ +err: + if (object_list != NULL) { + for (i = 0; i < args->buffer_count; i++) + drm_gem_object_unreference(dev, object_list[i]); + } + drm_free(object_list, sizeof(*object_list) * args->buffer_count, + DRM_MEM_DRIVER); drm_free(validate_list, sizeof(*validate_list) * args->buffer_count, DRM_MEM_DRIVER); - return 0; + return ret; } int i915_gem_init_object(struct drm_device *dev, struct drm_gem_object *obj) -- cgit v1.2.3 From ccd1bae0f676490a88240c62f02e072d2cf3b030 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 1 May 2008 15:22:21 -0700 Subject: checkpoint: relocations support. --- linux-core/i915_gem.c | 104 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 85 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 bd030a88..3e4403c7 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -59,14 +59,10 @@ i915_gem_object_free_page_list(struct drm_device *dev, if (obj_priv->page_list == NULL) return; - /* Count how many we had successfully allocated, since release_pages() - * doesn't like NULLs. - */ for (i = 0; i < obj->size / PAGE_SIZE; i++) { if (obj_priv->page_list[i] == NULL) - break; + put_page(obj_priv->page_list[i]); } - release_pages(obj_priv->page_list, i, 0); drm_free(obj_priv->page_list, page_count * sizeof(struct page *), @@ -149,11 +145,9 @@ i915_gem_reloc_and_validate_object(struct drm_device *dev, struct drm_i915_gem_validate_entry *entry, struct drm_gem_object *obj) { - struct drm_i915_gem_reloc *relocs; + struct drm_i915_gem_relocation_entry reloc; struct drm_i915_gem_object *obj_priv = obj->driver_private; - - /* Walk the list of relocations and perform them if necessary. */ - /* XXX */ + int i; /* Choose the GTT offset for our buffer and put it there. */ if (obj_priv->gtt_space == NULL) { @@ -162,6 +156,64 @@ i915_gem_reloc_and_validate_object(struct drm_device *dev, return -ENOMEM; } + /* Apply the relocations, using the GTT aperture to avoid cache + * flushing requirements. + */ + for (i = 0; i < entry->relocation_count; i++) { + struct drm_gem_object *target_obj; + struct drm_i915_gem_object *target_obj_priv; + void *reloc_page; + uint32_t reloc_val, *reloc_entry; + int ret; + + ret = copy_from_user(&reloc, entry->relocs + i, sizeof(reloc)); + if (ret != 0) + return ret; + + target_obj = drm_gem_object_lookup(dev, file_priv, + reloc.target_handle); + if (target_obj == NULL) + return -EINVAL; + target_obj_priv = target_obj->driver_private; + + /* The target buffer should have appeared before us in the + * validate list, so it should have a GTT space bound by now. + */ + if (target_obj_priv->gtt_space == NULL) { + DRM_ERROR("No GTT space found for object %d\n", + reloc.target_handle); + return -EINVAL; + } + + if (reloc.offset > obj->size - 4) { + DRM_ERROR("Relocation beyond object bounds.\n"); + return -EINVAL; + } + if (reloc.offset & 3) { + DRM_ERROR("Relocation not 4-byte aligned.\n"); + return -EINVAL; + } + + /* Map the page containing the relocation we're going to + * perform. + */ + reloc_page = ioremap(dev->agp->base + + (reloc.offset & ~(PAGE_SIZE - 1)), + PAGE_SIZE); + if (reloc_page == NULL) + return -ENOMEM; + + reloc_entry = (uint32_t *)((char *)reloc_page + + (reloc.offset & (PAGE_SIZE - 1))); + reloc_val = target_obj_priv->gtt_offset + reloc.delta; + + DRM_DEBUG("Applied relocation: %p@0x%08x = 0x%08x\n", + obj, reloc.offset, reloc_val); + *reloc_entry = reloc_val; + + iounmap(reloc_page); + } + return 0; } @@ -178,32 +230,44 @@ evict_callback(struct drm_memrange_node *node, void *data) return 0; } +static int +i915_gem_sync_and_evict(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + int ret; + RING_LOCALS; + + BEGIN_LP_RING(2); + OUT_RING(CMD_MI_FLUSH | MI_READ_FLUSH | MI_EXE_FLUSH); + OUT_RING(0); /* noop */ + ADVANCE_LP_RING(); + ret = i915_quiescent(dev); + if (ret != 0) + return ret; + + /* Evict everything so we have space for sure. */ + drm_memrange_for_each(&dev_priv->mm.gtt_space, evict_callback, dev); + + return 0; +} + int 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_gem_execbuffer *args = data; struct drm_i915_gem_validate_entry *validate_list; struct drm_gem_object **object_list; int ret, i; - RING_LOCALS; LOCK_TEST_WITH_RETURN(dev, file_priv); /* Big hammer: flush and idle the hardware so we can map things in/out. */ - BEGIN_LP_RING(2); - OUT_RING(CMD_MI_FLUSH | MI_READ_FLUSH | MI_EXE_FLUSH); - OUT_RING(0); /* noop */ - ADVANCE_LP_RING(); - ret = i915_quiescent(dev); + ret = i915_gem_sync_and_evict(dev); if (ret != 0) return ret; - /* Evict everything so we have space for sure. */ - drm_memrange_for_each(&dev_priv->mm.gtt_space, evict_callback, dev); - /* Copy in the validate list from userland */ validate_list = drm_calloc(sizeof(*validate_list), args->buffer_count, DRM_MEM_DRIVER); @@ -249,6 +313,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, sizeof(*validate_list) * args->buffer_count); /* Clean up and return */ + ret = i915_gem_sync_and_evict(dev); + err: if (object_list != NULL) { for (i = 0; i < args->buffer_count; i++) -- cgit v1.2.3 From 793549116ee6e9202fc7e474bd382eb19ffeb87f Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 1 May 2008 15:40:02 -0700 Subject: Add pin/unpin object ioctls for gem. --- linux-core/i915_gem.c | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 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 3e4403c7..fc9e53cf 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -328,6 +328,48 @@ err: return ret; } +int +i915_gem_pin(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_i915_gem_pin *args = data; + struct drm_gem_object *obj; + struct drm_i915_gem_object *obj_priv; + int ret; + + obj = drm_gem_object_lookup(dev, file_priv, args->handle); + if (obj == NULL) + return -EINVAL; + + ret = i915_gem_object_bind_to_gtt(dev, obj); + if (ret != 0) + return ret; + + obj_priv = obj->driver_private; + obj_priv->pin_count++; + args->offset = obj_priv->gtt_offset; + + return 0; +} + +int +i915_gem_unpin(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_i915_gem_pin *args = data; + struct drm_gem_object *obj; + struct drm_i915_gem_object *obj_priv; + + obj = drm_gem_object_lookup(dev, file_priv, args->handle); + if (obj == NULL) + return -EINVAL; + + obj_priv = obj->driver_private; + obj_priv->pin_count--; + + return 0; +} + int i915_gem_init_object(struct drm_device *dev, struct drm_gem_object *obj) { struct drm_i915_gem_object *obj_priv; @@ -345,4 +387,3 @@ void i915_gem_free_object(struct drm_device *dev, struct drm_gem_object *obj) { drm_free(obj->driver_private, 1, DRM_MEM_DRIVER); } - -- cgit v1.2.3 From d2529d13961f0df00754393e1ad9b72da5e998a4 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 1 May 2008 16:27:03 -0700 Subject: Remove _args from gem ioctl argument structure tags. --- linux-core/i915_gem.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index fc9e53cf..09c94c3b 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -329,8 +329,8 @@ err: } int -i915_gem_pin(struct drm_device *dev, void *data, - struct drm_file *file_priv) +i915_gem_pin_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) { struct drm_i915_gem_pin *args = data; struct drm_gem_object *obj; @@ -353,8 +353,8 @@ i915_gem_pin(struct drm_device *dev, void *data, } int -i915_gem_unpin(struct drm_device *dev, void *data, - struct drm_file *file_priv) +i915_gem_unpin_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) { struct drm_i915_gem_pin *args = data; struct drm_gem_object *obj; -- cgit v1.2.3 From 3f641b56c79d48f7e11aa3eb1dc678e09c8e01f7 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 1 May 2008 16:48:25 -0700 Subject: Fix missing member settings in obj/obj_priv, and some error paths. --- linux-core/i915_gem.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 09c94c3b..5ba48512 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -106,6 +106,9 @@ i915_gem_object_bind_to_gtt(struct drm_device *dev, struct drm_gem_object *obj) obj_priv->gtt_space = drm_memrange_get_block(free_space, obj->size, PAGE_SIZE); + obj_priv->gtt_offset = obj_priv->gtt_space->start; + + DRM_INFO("Binding object of size %d at 0x%08x\n", obj->size, obj_priv->gtt_offset); /* Get the list of pages out of our struct file. They'll be pinned * at this point until we release them. @@ -120,6 +123,8 @@ i915_gem_object_bind_to_gtt(struct drm_device *dev, struct drm_gem_object *obj) if (obj_priv->page_list[i] == NULL) { i915_gem_object_free_page_list(dev, obj); + drm_memrange_put_block(obj_priv->gtt_space); + obj_priv->gtt_space = NULL; return -ENOMEM; } } @@ -133,6 +138,8 @@ i915_gem_object_bind_to_gtt(struct drm_device *dev, struct drm_gem_object *obj) obj_priv->gtt_offset); if (obj_priv->agp_mem == NULL) { i915_gem_object_free_page_list(dev, obj); + drm_memrange_put_block(obj_priv->gtt_space); + obj_priv->gtt_space = NULL; return -ENOMEM; } -- cgit v1.2.3 From c10695bb7ab44494badc21c822eac3140cf4e117 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 1 May 2008 17:31:57 -0700 Subject: Unbind objects when freeing, fix some error paths, and warn in others. --- linux-core/i915_gem.c | 35 ++++++++++++++++++++++++++++++----- 1 file changed, 30 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 5ba48512..29b2d894 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -78,6 +78,9 @@ i915_gem_object_unbind(struct drm_device *dev, struct drm_gem_object *obj) { struct drm_i915_gem_object *obj_priv = obj->driver_private; + if (obj_priv->gtt_space == NULL) + return; + if (obj_priv->agp_mem != NULL) { drm_unbind_agp(obj_priv->agp_mem); drm_free_agp(obj_priv->agp_mem, obj->size / PAGE_SIZE); @@ -86,6 +89,7 @@ i915_gem_object_unbind(struct drm_device *dev, struct drm_gem_object *obj) i915_gem_object_free_page_list(dev, obj); drm_memrange_put_block(obj_priv->gtt_space); + obj_priv->gtt_space = NULL; } /** @@ -102,13 +106,17 @@ i915_gem_object_bind_to_gtt(struct drm_device *dev, struct drm_gem_object *obj) free_space = drm_memrange_search_free(&dev_priv->mm.gtt_space, obj->size, PAGE_SIZE, 0); - + if (free_space == NULL) + return -ENOMEM; obj_priv->gtt_space = drm_memrange_get_block(free_space, obj->size, PAGE_SIZE); + if (obj_priv->gtt_space == NULL) + return -ENOMEM; + obj_priv->gtt_offset = obj_priv->gtt_space->start; - DRM_INFO("Binding object of size %d at 0x%08x\n", obj->size, obj_priv->gtt_offset); + DRM_DEBUG("Binding object of size %d at 0x%08x\n", obj->size, obj_priv->gtt_offset); /* Get the list of pages out of our struct file. They'll be pinned * at this point until we release them. @@ -117,6 +125,12 @@ i915_gem_object_bind_to_gtt(struct drm_device *dev, struct drm_gem_object *obj) BUG_ON(obj_priv->page_list != NULL); obj_priv->page_list = drm_calloc(page_count, sizeof(struct page *), DRM_MEM_DRIVER); + if (obj_priv->page_list == NULL) { + drm_memrange_put_block(obj_priv->gtt_space); + obj_priv->gtt_space = NULL; + return -ENOMEM; + } + for (i = 0; i < page_count; i++) { obj_priv->page_list[i] = find_or_create_page(obj->filp->f_mapping, i, GFP_HIGHUSER); @@ -345,12 +359,18 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data, int ret; obj = drm_gem_object_lookup(dev, file_priv, args->handle); - if (obj == NULL) + if (obj == NULL) { + DRM_ERROR("Bad handle in i915_gem_pin_ioctl(): %d\n", + args->handle); return -EINVAL; + } ret = i915_gem_object_bind_to_gtt(dev, obj); - if (ret != 0) + if (ret != 0) { + DRM_ERROR("Failure to bind in i915_gem_pin_ioctl(): %d\n", + ret); return ret; + } obj_priv = obj->driver_private; obj_priv->pin_count++; @@ -368,8 +388,11 @@ i915_gem_unpin_ioctl(struct drm_device *dev, void *data, struct drm_i915_gem_object *obj_priv; obj = drm_gem_object_lookup(dev, file_priv, args->handle); - if (obj == NULL) + if (obj == NULL) { + DRM_ERROR("Bad handle in i915_gem_unpin_ioctl(): %d\n", + args->handle); return -EINVAL; + } obj_priv = obj->driver_private; obj_priv->pin_count--; @@ -392,5 +415,7 @@ int i915_gem_init_object(struct drm_device *dev, struct drm_gem_object *obj) void i915_gem_free_object(struct drm_device *dev, struct drm_gem_object *obj) { + i915_gem_object_unbind(dev, obj); + drm_free(obj->driver_private, 1, DRM_MEM_DRIVER); } -- cgit v1.2.3 From 30efad5113944681c1abd6452e10355c105e9c39 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Thu, 1 May 2008 20:31:16 -0700 Subject: Fix gem ioctls to be 32/64-bit clean. mixed 32/64 bit systems need 'special' help for ioctl where the user-space and kernel-space datatypes differ. Fixing the datatypes to be the same size, and align the same way for both 32 and 64-bit ppc and x86 environments will elimiante the need to have magic 32/64-bit ioctl translation code. --- linux-core/i915_gem.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 29b2d894..335f0618 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -167,6 +167,7 @@ i915_gem_reloc_and_validate_object(struct drm_device *dev, struct drm_gem_object *obj) { 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; @@ -177,6 +178,7 @@ i915_gem_reloc_and_validate_object(struct drm_device *dev, return -ENOMEM; } + relocs = (struct drm_i915_gem_relocation_entry __user *) (uintptr_t) entry->relocs_ptr; /* Apply the relocations, using the GTT aperture to avoid cache * flushing requirements. */ @@ -187,7 +189,7 @@ i915_gem_reloc_and_validate_object(struct drm_device *dev, uint32_t reloc_val, *reloc_entry; int ret; - ret = copy_from_user(&reloc, entry->relocs + i, sizeof(reloc)); + ret = copy_from_user(&reloc, relocs + i, sizeof(reloc)); if (ret != 0) return ret; @@ -229,7 +231,7 @@ i915_gem_reloc_and_validate_object(struct drm_device *dev, reloc_val = target_obj_priv->gtt_offset + reloc.delta; DRM_DEBUG("Applied relocation: %p@0x%08x = 0x%08x\n", - obj, reloc.offset, reloc_val); + obj, (unsigned int) reloc.offset, reloc_val); *reloc_entry = reloc_val; iounmap(reloc_page); @@ -299,8 +301,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, goto err; } ret = copy_from_user(validate_list, - (struct drm_i915_relocation_entry __user*) - args->buffers, + (struct drm_i915_relocation_entry __user*)(uintptr_t) + args->buffers_ptr, sizeof(*validate_list) * args->buffer_count); if (ret != 0) goto err; @@ -329,8 +331,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, validate_list[i].buffer_offset = obj_priv->gtt_offset; } ret = copy_to_user(validate_list, - (struct drm_i915_relocation_entry __user*) - args->buffers, + (struct drm_i915_relocation_entry __user*)(uintptr_t) + args->buffers_ptr, sizeof(*validate_list) * args->buffer_count); /* Clean up and return */ -- cgit v1.2.3 From 0d547c9ed92c0183f2c727496154baa2849f326e Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Thu, 1 May 2008 20:41:55 -0700 Subject: Add alignment to all aperture allocation requests. When pinning buffers, or using execbuffer, allow the application to specify the necessary aperture allocation alignment constraints. --- linux-core/i915_gem.c | 15 ++++++++++----- 1 file changed, 10 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 335f0618..30e67742 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -96,21 +96,26 @@ i915_gem_object_unbind(struct drm_device *dev, struct drm_gem_object *obj) * Finds free space in the GTT aperture and binds the object there. */ static int -i915_gem_object_bind_to_gtt(struct drm_device *dev, struct drm_gem_object *obj) +i915_gem_object_bind_to_gtt(struct drm_device *dev, struct drm_gem_object *obj, unsigned alignment) { drm_i915_private_t *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj_priv = obj->driver_private; struct drm_memrange_node *free_space; int page_count, i; + if (alignment == 0) + alignment = PAGE_SIZE; + if (alignment & (PAGE_SIZE - 1)) + return -EINVAL; + free_space = drm_memrange_search_free(&dev_priv->mm.gtt_space, obj->size, - PAGE_SIZE, 0); + alignment, 0); if (free_space == NULL) return -ENOMEM; obj_priv->gtt_space = drm_memrange_get_block(free_space, obj->size, - PAGE_SIZE); + alignment); if (obj_priv->gtt_space == NULL) return -ENOMEM; @@ -173,7 +178,7 @@ i915_gem_reloc_and_validate_object(struct drm_device *dev, /* Choose the GTT offset for our buffer and put it there. */ if (obj_priv->gtt_space == NULL) { - i915_gem_object_bind_to_gtt(dev, obj); + i915_gem_object_bind_to_gtt(dev, obj, (unsigned) entry->alignment); if (obj_priv->gtt_space == NULL) return -ENOMEM; } @@ -367,7 +372,7 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data, return -EINVAL; } - ret = i915_gem_object_bind_to_gtt(dev, obj); + ret = i915_gem_object_bind_to_gtt(dev, obj, (unsigned) args->alignment); if (ret != 0) { DRM_ERROR("Failure to bind in i915_gem_pin_ioctl(): %d\n", ret); -- cgit v1.2.3 From 49e8e3372afcf5fab9ffef5691d87ad8bc19599a Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 2 May 2008 10:36:00 -0700 Subject: Remove drm_driver argument to functions taking drm_gem_object. Now that drm_gem_object has a drm_driver * in it, functions don't need both parameters. --- linux-core/i915_gem.c | 44 +++++++++++++++++++++----------------------- 1 file changed, 21 insertions(+), 23 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 30e67742..16bb3923 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -49,8 +49,7 @@ i915_gem_init_ioctl(struct drm_device *dev, void *data, } static void -i915_gem_object_free_page_list(struct drm_device *dev, - struct drm_gem_object *obj) +i915_gem_object_free_page_list(struct drm_gem_object *obj) { struct drm_i915_gem_object *obj_priv = obj->driver_private; int page_count = obj->size / PAGE_SIZE; @@ -74,7 +73,7 @@ i915_gem_object_free_page_list(struct drm_device *dev, * Unbinds an object from the GTT aperture. */ static void -i915_gem_object_unbind(struct drm_device *dev, struct drm_gem_object *obj) +i915_gem_object_unbind(struct drm_gem_object *obj) { struct drm_i915_gem_object *obj_priv = obj->driver_private; @@ -86,7 +85,7 @@ i915_gem_object_unbind(struct drm_device *dev, struct drm_gem_object *obj) drm_free_agp(obj_priv->agp_mem, obj->size / PAGE_SIZE); } - i915_gem_object_free_page_list(dev, obj); + i915_gem_object_free_page_list(obj); drm_memrange_put_block(obj_priv->gtt_space); obj_priv->gtt_space = NULL; @@ -96,8 +95,9 @@ i915_gem_object_unbind(struct drm_device *dev, struct drm_gem_object *obj) * Finds free space in the GTT aperture and binds the object there. */ static int -i915_gem_object_bind_to_gtt(struct drm_device *dev, struct drm_gem_object *obj, unsigned alignment) +i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) { + 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; struct drm_memrange_node *free_space; @@ -141,7 +141,7 @@ i915_gem_object_bind_to_gtt(struct drm_device *dev, struct drm_gem_object *obj, find_or_create_page(obj->filp->f_mapping, i, GFP_HIGHUSER); if (obj_priv->page_list[i] == NULL) { - i915_gem_object_free_page_list(dev, obj); + i915_gem_object_free_page_list(obj); drm_memrange_put_block(obj_priv->gtt_space); obj_priv->gtt_space = NULL; return -ENOMEM; @@ -156,7 +156,7 @@ i915_gem_object_bind_to_gtt(struct drm_device *dev, struct drm_gem_object *obj, page_count, obj_priv->gtt_offset); if (obj_priv->agp_mem == NULL) { - i915_gem_object_free_page_list(dev, obj); + i915_gem_object_free_page_list(obj); drm_memrange_put_block(obj_priv->gtt_space); obj_priv->gtt_space = NULL; return -ENOMEM; @@ -166,11 +166,11 @@ i915_gem_object_bind_to_gtt(struct drm_device *dev, struct drm_gem_object *obj, } static int -i915_gem_reloc_and_validate_object(struct drm_device *dev, +i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, struct drm_file *file_priv, - struct drm_i915_gem_validate_entry *entry, - struct drm_gem_object *obj) + struct drm_i915_gem_validate_entry *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; @@ -178,7 +178,7 @@ i915_gem_reloc_and_validate_object(struct drm_device *dev, /* Choose the GTT offset for our buffer and put it there. */ if (obj_priv->gtt_space == NULL) { - i915_gem_object_bind_to_gtt(dev, obj, (unsigned) entry->alignment); + i915_gem_object_bind_to_gtt(obj, (unsigned) entry->alignment); if (obj_priv->gtt_space == NULL) return -ENOMEM; } @@ -198,7 +198,7 @@ i915_gem_reloc_and_validate_object(struct drm_device *dev, if (ret != 0) return ret; - target_obj = drm_gem_object_lookup(dev, file_priv, + target_obj = drm_gem_object_lookup(obj->dev, file_priv, reloc.target_handle); if (target_obj == NULL) return -EINVAL; @@ -248,12 +248,11 @@ i915_gem_reloc_and_validate_object(struct drm_device *dev, static int evict_callback(struct drm_memrange_node *node, void *data) { - struct drm_device *dev = data; struct drm_gem_object *obj = node->private; struct drm_i915_gem_object *obj_priv = obj->driver_private; if (obj_priv->pin_count == 0) - i915_gem_object_unbind(dev, obj); + i915_gem_object_unbind(obj); return 0; } @@ -274,7 +273,7 @@ i915_gem_sync_and_evict(struct drm_device *dev) return ret; /* Evict everything so we have space for sure. */ - drm_memrange_for_each(&dev_priv->mm.gtt_space, evict_callback, dev); + drm_memrange_for_each(&dev_priv->mm.gtt_space, evict_callback, NULL); return 0; } @@ -321,9 +320,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, goto err; } - i915_gem_reloc_and_validate_object(dev, file_priv, - &validate_list[i], - object_list[i]); + i915_gem_reloc_and_validate_object(object_list[i], file_priv, + &validate_list[i]); } /* Exec the batchbuffer */ @@ -346,7 +344,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, err: if (object_list != NULL) { for (i = 0; i < args->buffer_count; i++) - drm_gem_object_unreference(dev, object_list[i]); + drm_gem_object_unreference(object_list[i]); } drm_free(object_list, sizeof(*object_list) * args->buffer_count, DRM_MEM_DRIVER); @@ -372,7 +370,7 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data, return -EINVAL; } - ret = i915_gem_object_bind_to_gtt(dev, obj, (unsigned) args->alignment); + ret = i915_gem_object_bind_to_gtt(obj, (unsigned) args->alignment); if (ret != 0) { DRM_ERROR("Failure to bind in i915_gem_pin_ioctl(): %d\n", ret); @@ -407,7 +405,7 @@ i915_gem_unpin_ioctl(struct drm_device *dev, void *data, return 0; } -int i915_gem_init_object(struct drm_device *dev, struct drm_gem_object *obj) +int i915_gem_init_object(struct drm_gem_object *obj) { struct drm_i915_gem_object *obj_priv; @@ -420,9 +418,9 @@ int i915_gem_init_object(struct drm_device *dev, struct drm_gem_object *obj) return 0; } -void i915_gem_free_object(struct drm_device *dev, struct drm_gem_object *obj) +void i915_gem_free_object(struct drm_gem_object *obj) { - i915_gem_object_unbind(dev, obj); + i915_gem_object_unbind(obj); drm_free(obj->driver_private, 1, DRM_MEM_DRIVER); } -- cgit v1.2.3 From 39e20bcd5f4bf9fedac80188fda2e9fcab2f0360 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 2 May 2008 12:28:49 -0700 Subject: Add name/open ioctls, separate handle and pointer ref counts. Names are just another unique integer set (from another idr object). Names are removed when the user refernces (handles) are all destroyed -- this required that handles for objects be counted separately from internal kernel references (so that we can tell when the handles are all gone). --- linux-core/i915_gem.c | 4 +++- 1 file changed, 3 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 16bb3923..2ac74b4b 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -105,8 +105,10 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) if (alignment == 0) alignment = PAGE_SIZE; - if (alignment & (PAGE_SIZE - 1)) + if (alignment & (PAGE_SIZE - 1)) { + DRM_ERROR("Invalid object alignment requested %u\n", alignment); return -EINVAL; + } free_space = drm_memrange_search_free(&dev_priv->mm.gtt_space, obj->size, -- cgit v1.2.3 From ab3549d1336fc6c08581a9fd14a83513649d9187 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 2 May 2008 16:34:16 -0700 Subject: Add a bit of /proc/dri/*/gem support. Clean up some refcount/pagelock issues. Track named objects in /proc/dri/0/gem_names. Track total object count in /proc/dri/0/gem_objects. Initialize device gem data. return -ENODEV for gem ioctls if the driver doesn't support gem. Call unlock_page when unbinding from gtt. Add numerous misssing calls to drm_gem_object_unreference. --- linux-core/i915_gem.c | 36 +++++++++++++++++++++++++----------- 1 file changed, 25 insertions(+), 11 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 2ac74b4b..747cc45c 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -58,11 +58,13 @@ i915_gem_object_free_page_list(struct drm_gem_object *obj) if (obj_priv->page_list == NULL) return; - for (i = 0; i < obj->size / PAGE_SIZE; i++) { - if (obj_priv->page_list[i] == NULL) - put_page(obj_priv->page_list[i]); + + for (i = 0; i < page_count; i++) { + if (obj_priv->page_list[i] != NULL) { + unlock_page (obj_priv->page_list[i]); + page_cache_release (obj_priv->page_list[i]); + } } - drm_free(obj_priv->page_list, page_count * sizeof(struct page *), DRM_MEM_DRIVER); @@ -212,15 +214,18 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, if (target_obj_priv->gtt_space == NULL) { DRM_ERROR("No GTT space found for object %d\n", reloc.target_handle); + drm_gem_object_unreference (target_obj); return -EINVAL; } if (reloc.offset > obj->size - 4) { DRM_ERROR("Relocation beyond object bounds.\n"); + drm_gem_object_unreference (target_obj); return -EINVAL; } if (reloc.offset & 3) { DRM_ERROR("Relocation not 4-byte aligned.\n"); + drm_gem_object_unreference (target_obj); return -EINVAL; } @@ -231,7 +236,10 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, (reloc.offset & ~(PAGE_SIZE - 1)), PAGE_SIZE); if (reloc_page == NULL) + { + drm_gem_object_unreference (target_obj); return -ENOMEM; + } reloc_entry = (uint32_t *)((char *)reloc_page + (reloc.offset & (PAGE_SIZE - 1))); @@ -242,6 +250,7 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, *reloc_entry = reloc_val; iounmap(reloc_page); + drm_gem_object_unreference (target_obj); } return 0; @@ -372,16 +381,21 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data, return -EINVAL; } - ret = i915_gem_object_bind_to_gtt(obj, (unsigned) args->alignment); - if (ret != 0) { - DRM_ERROR("Failure to bind in i915_gem_pin_ioctl(): %d\n", - ret); - return ret; + obj_priv = obj->driver_private; + if (obj_priv->gtt_space == NULL) + { + ret = i915_gem_object_bind_to_gtt(obj, (unsigned) args->alignment); + if (ret != 0) { + DRM_ERROR("Failure to bind in i915_gem_pin_ioctl(): %d\n", + ret); + drm_gem_object_unreference (obj); + return ret; + } } - obj_priv = obj->driver_private; obj_priv->pin_count++; args->offset = obj_priv->gtt_offset; + drm_gem_object_unreference (obj); return 0; } @@ -403,7 +417,7 @@ i915_gem_unpin_ioctl(struct drm_device *dev, void *data, obj_priv = obj->driver_private; obj_priv->pin_count--; - + drm_gem_object_unreference (obj); return 0; } -- cgit v1.2.3 From 5f0614b86ff5760016bef87c6f6012fe4f42e14e Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 2 May 2008 17:13:11 -0700 Subject: Check for do_mmap errors --- linux-core/i915_gem.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 747cc45c..e0beeb43 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -300,12 +300,14 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, LOCK_TEST_WITH_RETURN(dev, file_priv); + DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); /* Big hammer: flush and idle the hardware so we can map things in/out. */ ret = i915_gem_sync_and_evict(dev); if (ret != 0) return ret; + DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); /* Copy in the validate list from userland */ validate_list = drm_calloc(sizeof(*validate_list), args->buffer_count, DRM_MEM_DRIVER); @@ -315,6 +317,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, ret = -ENOMEM; goto err; } + DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); ret = copy_from_user(validate_list, (struct drm_i915_relocation_entry __user*)(uintptr_t) args->buffers_ptr, @@ -322,6 +325,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, if (ret != 0) goto err; + DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); /* Look up object handles and perform the relocations */ for (i = 0; i < args->buffer_count; i++) { object_list[i] = drm_gem_object_lookup(dev, file_priv, @@ -337,6 +341,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, /* Exec the batchbuffer */ + DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); /* Copy the new buffer offsets back to the user's validate list. */ for (i = 0; i < args->buffer_count; i++) { struct drm_i915_gem_object *obj_priv = @@ -344,24 +349,29 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, validate_list[i].buffer_offset = obj_priv->gtt_offset; } + DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); ret = copy_to_user(validate_list, (struct drm_i915_relocation_entry __user*)(uintptr_t) args->buffers_ptr, sizeof(*validate_list) * args->buffer_count); + DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); /* Clean up and return */ ret = i915_gem_sync_and_evict(dev); + DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); err: if (object_list != NULL) { for (i = 0; i < args->buffer_count; i++) drm_gem_object_unreference(object_list[i]); } + DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); drm_free(object_list, sizeof(*object_list) * args->buffer_count, DRM_MEM_DRIVER); drm_free(validate_list, sizeof(*validate_list) * args->buffer_count, DRM_MEM_DRIVER); + DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); return ret; } -- cgit v1.2.3 From 166ff364fb09ec9885a164fca517c079a1d88718 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 2 May 2008 17:50:46 -0700 Subject: Don't forget to set the memrange private, and reset ring on kernel entry. --- linux-core/i915_gem.c | 4 +++- 1 file changed, 3 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 e0beeb43..8a30d846 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -122,7 +122,7 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) alignment); if (obj_priv->gtt_space == NULL) return -ENOMEM; - + obj_priv->gtt_space->private = obj; obj_priv->gtt_offset = obj_priv->gtt_space->start; DRM_DEBUG("Binding object of size %d at 0x%08x\n", obj->size, obj_priv->gtt_offset); @@ -300,6 +300,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, LOCK_TEST_WITH_RETURN(dev, file_priv); + i915_kernel_lost_context(dev); + DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); /* Big hammer: flush and idle the hardware so we can map things in/out. */ -- cgit v1.2.3 From b6f173c4300e90be9bdd3b24003b800afd8819c5 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Mon, 5 May 2008 10:51:49 -0700 Subject: Add i915_dispatch_gem_execbuffer (broken). This function submits a gem-based execbuffer to the ring. It doesn't work yet. --- linux-core/i915_gem.c | 115 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 96 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 8a30d846..95f6a267 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -79,18 +79,24 @@ i915_gem_object_unbind(struct drm_gem_object *obj) { struct drm_i915_gem_object *obj_priv = obj->driver_private; + DRM_INFO ("%s:%d %p\n", __FUNCTION__, __LINE__, obj); + DRM_INFO ("gtt_space %p\n", obj_priv->gtt_space); if (obj_priv->gtt_space == NULL) return; + DRM_INFO ("agp_mem %p %ld pages\n", obj_priv->agp_mem, obj->size / PAGE_SIZE); if (obj_priv->agp_mem != NULL) { drm_unbind_agp(obj_priv->agp_mem); drm_free_agp(obj_priv->agp_mem, obj->size / PAGE_SIZE); } + DRM_INFO ("free_page_list\n"); i915_gem_object_free_page_list(obj); + DRM_INFO ("put_block\n"); drm_memrange_put_block(obj_priv->gtt_space); obj_priv->gtt_space = NULL; + DRM_INFO ("done\n"); } /** @@ -259,9 +265,18 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, static int evict_callback(struct drm_memrange_node *node, void *data) { - struct drm_gem_object *obj = node->private; - struct drm_i915_gem_object *obj_priv = obj->driver_private; + struct drm_gem_object *obj; + struct drm_i915_gem_object *obj_priv; + + DRM_INFO ("evict node %p\n", node); + + obj = node->private; + DRM_INFO ("evict obj %p\n", obj); + + obj_priv = obj->driver_private; + DRM_INFO ("evict priv %p\n", obj_priv); + DRM_INFO ("pin_count %d\n", obj_priv->pin_count); if (obj_priv->pin_count == 0) i915_gem_object_unbind(obj); @@ -289,6 +304,58 @@ i915_gem_sync_and_evict(struct drm_device *dev) return 0; } +static int +i915_dispatch_gem_execbuffer (struct drm_device * dev, + struct drm_i915_gem_execbuffer * exec) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_clip_rect __user *boxes = (struct drm_clip_rect __user *) (uintptr_t) exec->cliprects_ptr; + int nbox = exec->num_cliprects; + int i = 0, count; + RING_LOCALS; + + if ((exec->batch_start_offset | exec->batch_len) & 0x7) { + DRM_ERROR("alignment\n"); + return -EINVAL; + } + + i915_kernel_lost_context(dev); + + count = nbox ? nbox : 1; + + for (i = 0; i < count; i++) { + if (i < nbox) { + int ret = i915_emit_box(dev, boxes, i, + exec->DR1, exec->DR4); + if (ret) + return ret; + } + + if (dev_priv->use_mi_batchbuffer_start) { + BEGIN_LP_RING(2); + if (IS_I965G(dev)) { + OUT_RING(MI_BATCH_BUFFER_START | (2 << 6) | MI_BATCH_NON_SECURE_I965); + OUT_RING(exec->batch_start_offset); + } else { + OUT_RING(MI_BATCH_BUFFER_START | (2 << 6)); + OUT_RING(exec->batch_start_offset | MI_BATCH_NON_SECURE); + } + ADVANCE_LP_RING(); + + } else { + BEGIN_LP_RING(4); + OUT_RING(MI_BATCH_BUFFER); + OUT_RING(exec->batch_start_offset | MI_BATCH_NON_SECURE); + OUT_RING(exec->batch_start_offset + exec->batch_len - 4); + OUT_RING(0); + ADVANCE_LP_RING(); + } + } + + /* XXX breadcrumb */ + return 0; +} + int i915_gem_execbuffer(struct drm_device *dev, void *data, struct drm_file *file_priv) @@ -300,39 +367,45 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, LOCK_TEST_WITH_RETURN(dev, file_priv); + DRM_INFO ("buffers_ptr %d buffer_count %d\n", + (int) args->buffers_ptr, args->buffer_count); i915_kernel_lost_context(dev); - DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); /* Big hammer: flush and idle the hardware so we can map things in/out. */ ret = i915_gem_sync_and_evict(dev); - if (ret != 0) + if (ret != 0) { + DRM_ERROR ("i915_gem_sync_and_evict failed %d\n", ret); return ret; + } - DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); /* Copy in the validate list from userland */ validate_list = drm_calloc(sizeof(*validate_list), args->buffer_count, DRM_MEM_DRIVER); object_list = drm_calloc(sizeof(*object_list), args->buffer_count, DRM_MEM_DRIVER); if (validate_list == NULL || object_list == NULL) { + DRM_ERROR ("Failed to allocate validate or object list for %d buffers\n", + args->buffer_count); ret = -ENOMEM; goto err; } - DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); ret = copy_from_user(validate_list, (struct drm_i915_relocation_entry __user*)(uintptr_t) args->buffers_ptr, sizeof(*validate_list) * args->buffer_count); - if (ret != 0) + if (ret != 0) { + DRM_ERROR ("copy %d validate entries failed %d\n", args->buffer_count, ret); goto err; + } - DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); /* Look up object handles and perform the relocations */ for (i = 0; i < args->buffer_count; i++) { object_list[i] = drm_gem_object_lookup(dev, file_priv, validate_list[i].buffer_handle); if (object_list[i] == NULL) { + DRM_ERROR ("Invalid object handle %d at index %d\n", + validate_list[i].buffer_handle, i); ret = -EINVAL; goto err; } @@ -342,8 +415,13 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, } /* Exec the batchbuffer */ + ret = i915_dispatch_gem_execbuffer (dev, args); + if (ret) + { + DRM_ERROR ("dispatch failed %d\n", ret); + goto err; + } - DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); /* Copy the new buffer offsets back to the user's validate list. */ for (i = 0; i < args->buffer_count; i++) { struct drm_i915_gem_object *obj_priv = @@ -351,29 +429,28 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, validate_list[i].buffer_offset = obj_priv->gtt_offset; } - DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); - ret = copy_to_user(validate_list, - (struct drm_i915_relocation_entry __user*)(uintptr_t) - args->buffers_ptr, - sizeof(*validate_list) * args->buffer_count); + ret = copy_to_user((struct drm_i915_relocation_entry __user*)(uintptr_t) + args->buffers_ptr, + validate_list, + sizeof(*validate_list) * args->buffer_count); + if (ret) + DRM_ERROR ("failed to copy %d validate entries back to user (%d)\n", + args->buffer_count, ret); - DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); /* Clean up and return */ ret = i915_gem_sync_and_evict(dev); - - DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); + if (ret) + DRM_ERROR ("failed to sync/evict buffers %d\n", ret); err: if (object_list != NULL) { for (i = 0; i < args->buffer_count; i++) drm_gem_object_unreference(object_list[i]); } - DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); drm_free(object_list, sizeof(*object_list) * args->buffer_count, DRM_MEM_DRIVER); drm_free(validate_list, sizeof(*validate_list) * args->buffer_count, DRM_MEM_DRIVER); - DRM_INFO ("%s:%d\n", __FUNCTION__, __LINE__); return ret; } -- cgit v1.2.3 From 4511e6cd80b4c47a142db48727753da8d0898857 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Mon, 5 May 2008 11:27:06 -0700 Subject: Correct execbuffer offset. Add memory barrier and chipset flush. --- linux-core/i915_gem.c | 47 ++++++++++++++++++++++++++++++++--------------- 1 file changed, 32 insertions(+), 15 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 95f6a267..7e4e8d5f 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -192,6 +192,7 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, if (obj_priv->gtt_space == NULL) return -ENOMEM; } + entry->buffer_offset = obj_priv->gtt_offset; relocs = (struct drm_i915_gem_relocation_entry __user *) (uintptr_t) entry->relocs_ptr; /* Apply the relocations, using the GTT aperture to avoid cache @@ -306,21 +307,32 @@ i915_gem_sync_and_evict(struct drm_device *dev) static int i915_dispatch_gem_execbuffer (struct drm_device * dev, - struct drm_i915_gem_execbuffer * exec) + struct drm_i915_gem_execbuffer * exec, + uint64_t exec_offset) { drm_i915_private_t *dev_priv = dev->dev_private; struct drm_clip_rect __user *boxes = (struct drm_clip_rect __user *) (uintptr_t) exec->cliprects_ptr; int nbox = exec->num_cliprects; int i = 0, count; + uint32_t exec_start, exec_len; RING_LOCALS; - if ((exec->batch_start_offset | exec->batch_len) & 0x7) { + exec_start = (uint32_t) exec_offset + exec->batch_start_offset; + exec_len = (uint32_t) exec->batch_len; + + if ((exec_start | exec_len) & 0x7) { DRM_ERROR("alignment\n"); return -EINVAL; } i915_kernel_lost_context(dev); + DRM_INFO ("execbuffer at %x+%d len %d\n", + (uint32_t) exec_offset, exec->batch_start_offset, exec_len); + + if (!exec_start) + return -EINVAL; + count = nbox ? nbox : 1; for (i = 0; i < count; i++) { @@ -335,18 +347,18 @@ i915_dispatch_gem_execbuffer (struct drm_device * dev, BEGIN_LP_RING(2); if (IS_I965G(dev)) { OUT_RING(MI_BATCH_BUFFER_START | (2 << 6) | MI_BATCH_NON_SECURE_I965); - OUT_RING(exec->batch_start_offset); + OUT_RING(exec_start); } else { OUT_RING(MI_BATCH_BUFFER_START | (2 << 6)); - OUT_RING(exec->batch_start_offset | MI_BATCH_NON_SECURE); + OUT_RING(exec_start | MI_BATCH_NON_SECURE); } ADVANCE_LP_RING(); } else { BEGIN_LP_RING(4); OUT_RING(MI_BATCH_BUFFER); - OUT_RING(exec->batch_start_offset | MI_BATCH_NON_SECURE); - OUT_RING(exec->batch_start_offset + exec->batch_len - 4); + OUT_RING(exec_start | MI_BATCH_NON_SECURE); + OUT_RING(exec_start + exec_len - 4); OUT_RING(0); ADVANCE_LP_RING(); } @@ -364,6 +376,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, struct drm_i915_gem_validate_entry *validate_list; struct drm_gem_object **object_list; int ret, i; + uint64_t exec_offset; LOCK_TEST_WITH_RETURN(dev, file_priv); @@ -410,12 +423,22 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, goto err; } - i915_gem_reloc_and_validate_object(object_list[i], file_priv, - &validate_list[i]); + ret = i915_gem_reloc_and_validate_object(object_list[i], file_priv, + &validate_list[i]); + if (ret) { + DRM_ERROR ("reloc and validate failed %d\n", ret); + goto err; + } } + exec_offset = validate_list[args->buffer_count - 1].buffer_offset; + + /* make sure all previous memory operations have passed */ + DRM_MEMORYBARRIER(); + drm_agp_chipset_flush(dev); + /* Exec the batchbuffer */ - ret = i915_dispatch_gem_execbuffer (dev, args); + ret = i915_dispatch_gem_execbuffer (dev, args, exec_offset); if (ret) { DRM_ERROR ("dispatch failed %d\n", ret); @@ -423,12 +446,6 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, } /* Copy the new buffer offsets back to the user's validate list. */ - for (i = 0; i < args->buffer_count; i++) { - struct drm_i915_gem_object *obj_priv = - object_list[i]->driver_private; - - validate_list[i].buffer_offset = obj_priv->gtt_offset; - } ret = copy_to_user((struct drm_i915_relocation_entry __user*)(uintptr_t) args->buffers_ptr, validate_list, -- cgit v1.2.3 From 4867780bd6900293880d1db963798d075ec9b01a Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Mon, 5 May 2008 13:32:28 -0700 Subject: Emit clflush and chipset flush when mapping objects to gtt --- linux-core/i915_gem.c | 22 +++++++++++++++++++--- 1 file changed, 19 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 7e4e8d5f..b66c7865 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -131,7 +131,7 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) obj_priv->gtt_space->private = obj; obj_priv->gtt_offset = obj_priv->gtt_space->start; - DRM_DEBUG("Binding object of size %d at 0x%08x\n", obj->size, obj_priv->gtt_offset); + DRM_INFO ("Binding object of size %d at 0x%08x\n", obj->size, obj_priv->gtt_offset); /* Get the list of pages out of our struct file. They'll be pinned * at this point until we release them. @@ -157,6 +157,10 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) return -ENOMEM; } } + + drm_ttm_cache_flush (obj_priv->page_list, page_count); + DRM_MEMORYBARRIER(); + drm_agp_chipset_flush(dev); /* Create an AGP memory structure pointing at our pages, and bind it * into the GTT. @@ -172,6 +176,20 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) return -ENOMEM; } + { + uint32_t *mem = kmap_atomic (obj_priv->page_list[0], KM_USER0); + volatile uint32_t *gtt = ioremap(dev->agp->base + obj_priv->gtt_offset, + PAGE_SIZE); + int i; + + DRM_INFO ("object at offset %08x agp base %08x gtt addr %p\n", + obj_priv->gtt_offset, (int) dev->agp->base, gtt); + for (i = 0; i < 16; i++) + DRM_INFO ("%3d: mem %08x gtt %08x\n", i, mem[i], gtt[i]); + iounmap (gtt); + kunmap_atomic (mem, KM_USER0); + } + return 0; } @@ -434,8 +452,6 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, exec_offset = validate_list[args->buffer_count - 1].buffer_offset; /* make sure all previous memory operations have passed */ - DRM_MEMORYBARRIER(); - drm_agp_chipset_flush(dev); /* Exec the batchbuffer */ ret = i915_dispatch_gem_execbuffer (dev, args, exec_offset); -- cgit v1.2.3 From f0bc796a028dc7c6281d3d0cb2deef9df37e380a Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Mon, 5 May 2008 14:22:42 -0700 Subject: Add object base to relocation store address. The relocated value was being written to the wrong location, missing the object base address. --- linux-core/i915_gem.c | 46 ++++++++++++++++++++++++++-------------------- 1 file changed, 26 insertions(+), 20 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index b66c7865..a1ce139b 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -99,6 +99,23 @@ i915_gem_object_unbind(struct drm_gem_object *obj) DRM_INFO ("done\n"); } +static void +i915_gem_dump_object (struct drm_gem_object *obj, int len, const char *where) +{ + struct drm_device *dev = obj->dev; + struct drm_i915_gem_object *obj_priv = obj->driver_private; + uint32_t *mem = kmap_atomic (obj_priv->page_list[0], KM_USER0); + volatile uint32_t *gtt = ioremap(dev->agp->base + obj_priv->gtt_offset, + PAGE_SIZE); + int i; + + DRM_INFO ("%s: object at offset %08x\n", where, obj_priv->gtt_offset); + for (i = 0; i < len/4; i++) + DRM_INFO ("%3d: mem %08x gtt %08x\n", i, mem[i], readl(gtt + i)); + iounmap (gtt); + kunmap_atomic (mem, KM_USER0); +} + /** * Finds free space in the GTT aperture and binds the object there. */ @@ -176,20 +193,6 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) return -ENOMEM; } - { - uint32_t *mem = kmap_atomic (obj_priv->page_list[0], KM_USER0); - volatile uint32_t *gtt = ioremap(dev->agp->base + obj_priv->gtt_offset, - PAGE_SIZE); - int i; - - DRM_INFO ("object at offset %08x agp base %08x gtt addr %p\n", - obj_priv->gtt_offset, (int) dev->agp->base, gtt); - for (i = 0; i < 16; i++) - DRM_INFO ("%3d: mem %08x gtt %08x\n", i, mem[i], gtt[i]); - iounmap (gtt); - kunmap_atomic (mem, KM_USER0); - } - return 0; } @@ -220,7 +223,7 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, struct drm_gem_object *target_obj; struct drm_i915_gem_object *target_obj_priv; void *reloc_page; - uint32_t reloc_val, *reloc_entry; + uint32_t reloc_val, reloc_offset, *reloc_entry; int ret; ret = copy_from_user(&reloc, relocs + i, sizeof(reloc)); @@ -257,8 +260,9 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, /* Map the page containing the relocation we're going to * perform. */ + reloc_offset = obj_priv->gtt_offset + reloc.offset; reloc_page = ioremap(dev->agp->base + - (reloc.offset & ~(PAGE_SIZE - 1)), + (reloc_offset & ~(PAGE_SIZE - 1)), PAGE_SIZE); if (reloc_page == NULL) { @@ -267,17 +271,19 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, } reloc_entry = (uint32_t *)((char *)reloc_page + - (reloc.offset & (PAGE_SIZE - 1))); + (reloc_offset & (PAGE_SIZE - 1))); reloc_val = target_obj_priv->gtt_offset + reloc.delta; - DRM_DEBUG("Applied relocation: %p@0x%08x = 0x%08x\n", - obj, (unsigned int) reloc.offset, reloc_val); - *reloc_entry = reloc_val; + DRM_INFO("Applied relocation: %p@0x%08x %08x -> %08x\n", + obj, (unsigned int) reloc.offset, + readl (reloc_entry), reloc_val); + writel (reloc_val, reloc_entry); iounmap(reloc_page); drm_gem_object_unreference (target_obj); } + i915_gem_dump_object (obj, 128, __FUNCTION__); return 0; } -- cgit v1.2.3 From d59a9300ec2ec5d6dc606f847a7589c197994793 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Mon, 5 May 2008 14:32:01 -0700 Subject: Remove some debug messages. --- linux-core/i915_gem.c | 29 +++++++++++++---------------- 1 file changed, 13 insertions(+), 16 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index a1ce139b..8d606f85 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -79,24 +79,22 @@ i915_gem_object_unbind(struct drm_gem_object *obj) { struct drm_i915_gem_object *obj_priv = obj->driver_private; +#if 0 DRM_INFO ("%s:%d %p\n", __FUNCTION__, __LINE__, obj); DRM_INFO ("gtt_space %p\n", obj_priv->gtt_space); +#endif if (obj_priv->gtt_space == NULL) return; - DRM_INFO ("agp_mem %p %ld pages\n", obj_priv->agp_mem, obj->size / PAGE_SIZE); if (obj_priv->agp_mem != NULL) { drm_unbind_agp(obj_priv->agp_mem); drm_free_agp(obj_priv->agp_mem, obj->size / PAGE_SIZE); } - DRM_INFO ("free_page_list\n"); i915_gem_object_free_page_list(obj); - DRM_INFO ("put_block\n"); drm_memrange_put_block(obj_priv->gtt_space); obj_priv->gtt_space = NULL; - DRM_INFO ("done\n"); } static void @@ -148,7 +146,9 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) obj_priv->gtt_space->private = obj; obj_priv->gtt_offset = obj_priv->gtt_space->start; +#if 0 DRM_INFO ("Binding object of size %d at 0x%08x\n", obj->size, obj_priv->gtt_offset); +#endif /* Get the list of pages out of our struct file. They'll be pinned * at this point until we release them. @@ -274,34 +274,27 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, (reloc_offset & (PAGE_SIZE - 1))); reloc_val = target_obj_priv->gtt_offset + reloc.delta; +#if 0 DRM_INFO("Applied relocation: %p@0x%08x %08x -> %08x\n", obj, (unsigned int) reloc.offset, readl (reloc_entry), reloc_val); +#endif writel (reloc_val, reloc_entry); iounmap(reloc_page); drm_gem_object_unreference (target_obj); } - i915_gem_dump_object (obj, 128, __FUNCTION__); +/* i915_gem_dump_object (obj, 128, __FUNCTION__); */ return 0; } static int evict_callback(struct drm_memrange_node *node, void *data) { - struct drm_gem_object *obj; - struct drm_i915_gem_object *obj_priv; - - DRM_INFO ("evict node %p\n", node); - - obj = node->private; - DRM_INFO ("evict obj %p\n", obj); + struct drm_gem_object *obj = node->private; + struct drm_i915_gem_object *obj_priv = obj->driver_private; - obj_priv = obj->driver_private; - DRM_INFO ("evict priv %p\n", obj_priv); - - DRM_INFO ("pin_count %d\n", obj_priv->pin_count); if (obj_priv->pin_count == 0) i915_gem_object_unbind(obj); @@ -351,8 +344,10 @@ i915_dispatch_gem_execbuffer (struct drm_device * dev, i915_kernel_lost_context(dev); +#if 0 DRM_INFO ("execbuffer at %x+%d len %d\n", (uint32_t) exec_offset, exec->batch_start_offset, exec_len); +#endif if (!exec_start) return -EINVAL; @@ -404,8 +399,10 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, LOCK_TEST_WITH_RETURN(dev, file_priv); +#if 0 DRM_INFO ("buffers_ptr %d buffer_count %d\n", (int) args->buffers_ptr, args->buffer_count); +#endif i915_kernel_lost_context(dev); /* Big hammer: flush and idle the hardware so we can map things in/out. -- cgit v1.2.3 From dafe48e6239a4e9b49dd87b8c70224e8eeeb6079 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Mon, 5 May 2008 14:38:04 -0700 Subject: GEM: Replace drm_memrange_for_each with just evicting what we brought in. I was wrong about how the data structure worked, and didn't care to fix it to support debugging code. --- linux-core/i915_gem.c | 49 +++++++++++++++---------------------------------- 1 file changed, 15 insertions(+), 34 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index a1ce139b..919f32d7 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -288,45 +288,17 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, } static int -evict_callback(struct drm_memrange_node *node, void *data) -{ - struct drm_gem_object *obj; - struct drm_i915_gem_object *obj_priv; - - DRM_INFO ("evict node %p\n", node); - - obj = node->private; - DRM_INFO ("evict obj %p\n", obj); - - obj_priv = obj->driver_private; - DRM_INFO ("evict priv %p\n", obj_priv); - - DRM_INFO ("pin_count %d\n", obj_priv->pin_count); - if (obj_priv->pin_count == 0) - i915_gem_object_unbind(obj); - - return 0; -} - -static int -i915_gem_sync_and_evict(struct drm_device *dev) +i915_gem_sync(struct drm_device *dev) { drm_i915_private_t *dev_priv = dev->dev_private; - int ret; RING_LOCALS; BEGIN_LP_RING(2); OUT_RING(CMD_MI_FLUSH | MI_READ_FLUSH | MI_EXE_FLUSH); OUT_RING(0); /* noop */ ADVANCE_LP_RING(); - ret = i915_quiescent(dev); - if (ret != 0) - return ret; - - /* Evict everything so we have space for sure. */ - drm_memrange_for_each(&dev_priv->mm.gtt_space, evict_callback, NULL); - return 0; + return i915_quiescent(dev); } static int @@ -410,9 +382,9 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, /* Big hammer: flush and idle the hardware so we can map things in/out. */ - ret = i915_gem_sync_and_evict(dev); + ret = i915_gem_sync(dev); if (ret != 0) { - DRM_ERROR ("i915_gem_sync_and_evict failed %d\n", ret); + DRM_ERROR ("i915_gem_sync failed %d\n", ret); return ret; } @@ -477,9 +449,18 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, args->buffer_count, ret); /* Clean up and return */ - ret = i915_gem_sync_and_evict(dev); + ret = i915_gem_sync(dev); if (ret) - DRM_ERROR ("failed to sync/evict buffers %d\n", ret); + DRM_ERROR ("failed to sync %d\n", ret); + + /* Evict all the buffers we moved in, leaving room for the next guy. */ + 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; + + if (obj_priv->pin_count == 0) + i915_gem_object_unbind(obj); + } err: if (object_list != NULL) { for (i = 0; i < args->buffer_count; i++) -- cgit v1.2.3 From 2c8f970baaba9c72c882677f40ce8271bff03bac Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Mon, 5 May 2008 17:17:19 -0700 Subject: Unlock pages right after getting them. pages come back from find_or_create_page locked, but must not stay locked for long. Unlock them immediately instead of waiting until we're done with them to avoid deadlock when applications try to touch them. --- linux-core/i915_gem.c | 4 ++-- 1 file changed, 2 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 fabbaf9b..e3864612 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -61,7 +61,6 @@ i915_gem_object_free_page_list(struct drm_gem_object *obj) for (i = 0; i < page_count; i++) { if (obj_priv->page_list[i] != NULL) { - unlock_page (obj_priv->page_list[i]); page_cache_release (obj_priv->page_list[i]); } } @@ -109,7 +108,7 @@ i915_gem_dump_object (struct drm_gem_object *obj, int len, const char *where) DRM_INFO ("%s: object at offset %08x\n", where, obj_priv->gtt_offset); for (i = 0; i < len/4; i++) - DRM_INFO ("%3d: mem %08x gtt %08x\n", i, mem[i], readl(gtt + i)); + DRM_INFO ("%04x: mem %08x gtt %08x\n", i * 4, mem[i], readl(gtt + i)); iounmap (gtt); kunmap_atomic (mem, KM_USER0); } @@ -173,6 +172,7 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) obj_priv->gtt_space = NULL; return -ENOMEM; } + unlock_page (obj_priv->page_list[i]); } drm_ttm_cache_flush (obj_priv->page_list, page_count); -- cgit v1.2.3 From 91cba3ae17eb34d1836164f86c13a2a8e08c2a29 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Mon, 5 May 2008 22:10:02 -0700 Subject: Dump last batch buffer when hardware lockup is detected. --- linux-core/i915_gem.c | 87 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 62 insertions(+), 25 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index e3864612..4dcfd2b1 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -30,6 +30,9 @@ #include "i915_drm.h" #include "i915_drv.h" +#define WATCH_BUF 0 +#define WATCH_EXEC 0 + int i915_gem_init_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) @@ -78,7 +81,7 @@ i915_gem_object_unbind(struct drm_gem_object *obj) { struct drm_i915_gem_object *obj_priv = obj->driver_private; -#if 0 +#if WATCH_BUF DRM_INFO ("%s:%d %p\n", __FUNCTION__, __LINE__, obj); DRM_INFO ("gtt_space %p\n", obj_priv->gtt_space); #endif @@ -97,20 +100,44 @@ i915_gem_object_unbind(struct drm_gem_object *obj) } static void -i915_gem_dump_object (struct drm_gem_object *obj, int len, const char *where) +i915_gem_dump_page (struct page *page, uint32_t start, uint32_t end, uint32_t bias, uint32_t mark) +{ + uint32_t *mem = kmap_atomic (page, KM_USER0); + int i; + for (i = start; i < end; i += 4) + DRM_INFO ("%08x: %08x%s\n", + (int) (bias + i), mem[i / 4], + (bias + i == mark) ? " ********" : ""); + kunmap_atomic (mem, KM_USER0); + /* give syslog time to catch up */ + msleep (1); +} + +static void +i915_gem_dump_object (struct drm_gem_object *obj, int len, const char *where, uint32_t mark) { - struct drm_device *dev = obj->dev; struct drm_i915_gem_object *obj_priv = obj->driver_private; - uint32_t *mem = kmap_atomic (obj_priv->page_list[0], KM_USER0); - volatile uint32_t *gtt = ioremap(dev->agp->base + obj_priv->gtt_offset, - PAGE_SIZE); - int i; + int page; DRM_INFO ("%s: object at offset %08x\n", where, obj_priv->gtt_offset); - for (i = 0; i < len/4; i++) - DRM_INFO ("%04x: mem %08x gtt %08x\n", i * 4, mem[i], readl(gtt + i)); - iounmap (gtt); - kunmap_atomic (mem, KM_USER0); + for (page = 0; page < (len + PAGE_SIZE-1) / PAGE_SIZE; page++) + { + int page_len, chunk, chunk_len; + + page_len = len - page * PAGE_SIZE; + if (page_len > PAGE_SIZE) + page_len = PAGE_SIZE; + + for (chunk = 0; chunk < page_len; chunk += 128) { + chunk_len = page_len - chunk; + if (chunk_len > 128) + chunk_len = 128; + i915_gem_dump_page (obj_priv->page_list[page], + chunk, chunk + chunk_len, + obj_priv->gtt_offset + page * PAGE_SIZE, + mark); + } + } } /** @@ -145,7 +172,7 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) obj_priv->gtt_space->private = obj; obj_priv->gtt_offset = obj_priv->gtt_space->start; -#if 0 +#if WATCH_BUF DRM_INFO ("Binding object of size %d at 0x%08x\n", obj->size, obj_priv->gtt_offset); #endif @@ -274,7 +301,7 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, (reloc_offset & (PAGE_SIZE - 1))); reloc_val = target_obj_priv->gtt_offset + reloc.delta; -#if 0 +#if WATCH_BUF DRM_INFO("Applied relocation: %p@0x%08x %08x -> %08x\n", obj, (unsigned int) reloc.offset, readl (reloc_entry), reloc_val); @@ -285,7 +312,9 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, drm_gem_object_unreference (target_obj); } -/* i915_gem_dump_object (obj, 128, __FUNCTION__); */ +#if WATCH_BUF + i915_gem_dump_object (obj, 128, __FUNCTION__, ~0); +#endif return 0; } @@ -323,13 +352,6 @@ i915_dispatch_gem_execbuffer (struct drm_device * dev, return -EINVAL; } - i915_kernel_lost_context(dev); - -#if 0 - DRM_INFO ("execbuffer at %x+%d len %d\n", - (uint32_t) exec_offset, exec->batch_start_offset, exec_len); -#endif - if (!exec_start) return -EINVAL; @@ -380,9 +402,9 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, LOCK_TEST_WITH_RETURN(dev, file_priv); -#if 0 - DRM_INFO ("buffers_ptr %d buffer_count %d\n", - (int) args->buffers_ptr, args->buffer_count); +#if WATCH_EXEC + 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); @@ -437,6 +459,13 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, /* make sure all previous memory operations have passed */ +#if WATCH_EXEC + i915_gem_dump_object (object_list[args->buffer_count - 1], + args->batch_len, + __FUNCTION__, + ~0); +#endif + /* Exec the batchbuffer */ ret = i915_dispatch_gem_execbuffer (dev, args, exec_offset); if (ret) @@ -457,7 +486,15 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, /* Clean up and return */ ret = i915_gem_sync(dev); if (ret) - DRM_ERROR ("failed to sync %d\n", ret); + { + drm_i915_private_t *dev_priv = dev->dev_private; + uint32_t acthd = I915_READ(IS_I965G(dev) ? I965REG_ACTHD : I915REG_ACTHD); + DRM_ERROR ("failed to sync %d acthd %08x\n", ret, acthd); + i915_gem_dump_object (object_list[args->buffer_count - 1], + args->batch_len, + __FUNCTION__, + acthd); + } /* Evict all the buffers we moved in, leaving room for the next guy. */ for (i = 0; i < args->buffer_count; i++) { -- cgit v1.2.3 From 8551bfc6dba03dcd9d182b2099a0906153ecfa01 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Tue, 6 May 2008 11:18:47 -0700 Subject: GEM: Save the last ioremapped page for relocations in case we need it again. --- linux-core/i915_gem.c | 28 +++++++++++++++++++--------- 1 file changed, 19 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 4dcfd2b1..e795ee77 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -233,6 +233,8 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, struct drm_i915_gem_relocation_entry __user *relocs; struct drm_i915_gem_object *obj_priv = obj->driver_private; int i; + 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) { @@ -249,7 +251,6 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, for (i = 0; i < entry->relocation_count; i++) { struct drm_gem_object *target_obj; struct drm_i915_gem_object *target_obj_priv; - void *reloc_page; uint32_t reloc_val, reloc_offset, *reloc_entry; int ret; @@ -288,13 +289,20 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, * perform. */ reloc_offset = obj_priv->gtt_offset + reloc.offset; - reloc_page = ioremap(dev->agp->base + - (reloc_offset & ~(PAGE_SIZE - 1)), - PAGE_SIZE); - if (reloc_page == NULL) - { - drm_gem_object_unreference (target_obj); - return -ENOMEM; + if (reloc_page == NULL || + (last_reloc_offset & ~(PAGE_SIZE - 1)) != + (reloc_offset & ~(PAGE_SIZE - 1))) { + if (reloc_page != NULL) + iounmap(reloc_page); + + reloc_page = ioremap(dev->agp->base + + (reloc_offset & ~(PAGE_SIZE - 1)), + PAGE_SIZE); + last_reloc_offset = reloc_offset; + if (reloc_page == NULL) { + drm_gem_object_unreference (target_obj); + return -ENOMEM; + } } reloc_entry = (uint32_t *)((char *)reloc_page + @@ -308,10 +316,12 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, #endif writel (reloc_val, reloc_entry); - iounmap(reloc_page); drm_gem_object_unreference (target_obj); } + if (reloc_page != NULL) + iounmap(reloc_page); + #if WATCH_BUF i915_gem_dump_object (obj, 128, __FUNCTION__, ~0); #endif -- cgit v1.2.3 From dd6976c56f9f14ea8aa630833e9cc9711157d74f Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Tue, 6 May 2008 11:25:53 -0700 Subject: GEM: Skip relocation if presumed offset matches. --- linux-core/i915_gem.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index e795ee77..4534dae2 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -285,6 +285,9 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, return -EINVAL; } + if (target_obj_priv->gtt_offset == reloc.presumed_offset) + continue; + /* Map the page containing the relocation we're going to * perform. */ -- cgit v1.2.3 From d2373b2a341868882208bb4297ab4f2f51302031 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Tue, 6 May 2008 13:28:26 -0700 Subject: GEM: Use irq-based fencing rather than syncing and evicting every exec. --- linux-core/i915_gem.c | 175 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 130 insertions(+), 45 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 4534dae2..d630b0b2 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -73,6 +73,31 @@ i915_gem_object_free_page_list(struct drm_gem_object *obj) obj_priv->page_list = NULL; } +/** + * Ensures that all rendering to the object has completed and the object is + * safe to unbind from the GTT. + */ +static int +i915_gem_object_wait_rendering(struct drm_gem_object *obj) +{ + struct drm_device *dev = obj->dev; + struct drm_i915_gem_object *obj_priv = obj->driver_private; + int ret; + + /* If there is rendering queued on the buffer being evicted, wait for + * it. + */ + if (obj_priv->last_rendering_cookie != 0) { + ret = i915_wait_irq(dev, obj_priv->last_rendering_cookie); + if (ret != 0) + return ret; + /* Clear it now that we know it's passed. */ + obj_priv->last_rendering_cookie = 0; + } + + return 0; +} + /** * Unbinds an object from the GTT aperture. */ @@ -88,6 +113,8 @@ i915_gem_object_unbind(struct drm_gem_object *obj) if (obj_priv->gtt_space == NULL) return; + i915_gem_object_wait_rendering(obj); + if (obj_priv->agp_mem != NULL) { drm_unbind_agp(obj_priv->agp_mem); drm_free_agp(obj_priv->agp_mem, obj->size / PAGE_SIZE); @@ -97,8 +124,10 @@ i915_gem_object_unbind(struct drm_gem_object *obj) drm_memrange_put_block(obj_priv->gtt_space); obj_priv->gtt_space = NULL; + list_del_init(&obj_priv->gtt_lru_entry); } +#if 0 static void i915_gem_dump_page (struct page *page, uint32_t start, uint32_t end, uint32_t bias, uint32_t mark) { @@ -139,6 +168,39 @@ i915_gem_dump_object (struct drm_gem_object *obj, int len, const char *where, ui } } } +#endif + +static int +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; + + /* Find the LRU buffer. */ + BUG_ON(!list_empty(&dev_priv->mm.gtt_lru)); + obj_priv = list_entry(dev_priv->mm.gtt_lru.prev, + struct drm_i915_gem_object, + gtt_lru_entry); + obj = obj_priv->obj; + + /* Only unpinned buffers should be on this list. */ + BUG_ON(obj_priv->pin_count != 0); + + /* Do this separately from the wait_rendering in + * i915_gem_object_unbind() because we want to catch interrupts and + * return. + */ + ret = i915_gem_object_wait_rendering(obj); + if (ret != 0) + return ret; + + /* Wait on the rendering and unbind the buffer. */ + i915_gem_object_unbind(obj); + + return 0; +} /** * Finds free space in the GTT aperture and binds the object there. @@ -150,7 +212,7 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) drm_i915_private_t *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj_priv = obj->driver_private; struct drm_memrange_node *free_space; - int page_count, i; + int page_count, i, ret; if (alignment == 0) alignment = PAGE_SIZE; @@ -159,18 +221,31 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) return -EINVAL; } + search_free: free_space = drm_memrange_search_free(&dev_priv->mm.gtt_space, obj->size, alignment, 0); - if (free_space == NULL) - return -ENOMEM; - obj_priv->gtt_space = drm_memrange_get_block(free_space, - obj->size, - alignment); - if (obj_priv->gtt_space == NULL) - return -ENOMEM; - obj_priv->gtt_space->private = obj; - obj_priv->gtt_offset = obj_priv->gtt_space->start; + if (free_space != NULL) { + obj_priv->gtt_space = + drm_memrange_get_block(free_space, obj->size, + alignment); + if (obj_priv->gtt_space != NULL) { + obj_priv->gtt_space->private = obj; + obj_priv->gtt_offset = obj_priv->gtt_space->start; + } + } + if (obj_priv->gtt_space == NULL) { + /* If the gtt is empty and we're still having trouble + * fitting our object in, we're out of memory. + */ + if (list_empty(&dev_priv->mm.gtt_lru)) + return -ENOMEM; + + ret = i915_gem_evict_something(dev); + if (ret != 0) + return ret; + goto search_free; + } #if WATCH_BUF DRM_INFO ("Binding object of size %d at 0x%08x\n", obj->size, obj_priv->gtt_offset); @@ -229,6 +304,7 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, struct drm_i915_gem_validate_entry *entry) { struct drm_device *dev = obj->dev; + drm_i915_private_t *dev_priv = dev->dev_private; 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; @@ -244,6 +320,12 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, } entry->buffer_offset = obj_priv->gtt_offset; + if (obj_priv->pin_count == 0) { + /* Move our buffer to the head of the LRU. */ + list_del_init(&obj_priv->gtt_lru_entry); + list_add(&obj_priv->gtt_lru_entry, &dev_priv->mm.gtt_lru); + } + relocs = (struct drm_i915_gem_relocation_entry __user *) (uintptr_t) entry->relocs_ptr; /* Apply the relocations, using the GTT aperture to avoid cache * flushing requirements. @@ -331,8 +413,8 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, return 0; } -static int -i915_gem_sync(struct drm_device *dev) +static void +i915_gem_flush(struct drm_device *dev) { drm_i915_private_t *dev_priv = dev->dev_private; RING_LOCALS; @@ -341,8 +423,6 @@ i915_gem_sync(struct drm_device *dev) OUT_RING(CMD_MI_FLUSH | MI_READ_FLUSH | MI_EXE_FLUSH); OUT_RING(0); /* noop */ ADVANCE_LP_RING(); - - return i915_quiescent(dev); } static int @@ -412,6 +492,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, struct drm_gem_object **object_list; int ret, i; uint64_t exec_offset; + uint32_t cookie; LOCK_TEST_WITH_RETURN(dev, file_priv); @@ -421,14 +502,6 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, #endif i915_kernel_lost_context(dev); - /* Big hammer: flush and idle the hardware so we can map things in/out. - */ - ret = i915_gem_sync(dev); - if (ret != 0) { - DRM_ERROR ("i915_gem_sync failed %d\n", ret); - return ret; - } - /* Copy in the validate list from userland */ validate_list = drm_calloc(sizeof(*validate_list), args->buffer_count, DRM_MEM_DRIVER); @@ -468,6 +541,21 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, } } + 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; + + if (obj_priv->gtt_space == NULL) { + /* We evicted the buffer in the process of validating + * our set of buffers in. We could try to recover by + * kicking them everything out and trying again from + * the start. + */ + ret = -ENOMEM; + goto err; + } + } + exec_offset = validate_list[args->buffer_count - 1].buffer_offset; /* make sure all previous memory operations have passed */ @@ -487,6 +575,25 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, goto err; } + /* Flush the rendering. We want this flush to go away, which will + * require intelligent cache management. + */ + i915_gem_flush(dev); + + /* Get a cookie representing the flush of the current buffer, which we + * can wait on. We would like to mitigate these interrupts, likely by + * only flushing occasionally (so that we have *some* interrupts + * representing completion of buffers that we can wait on when trying + * to clear up gtt space). + */ + cookie = i915_emit_irq(dev); + 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; + + obj_priv->last_rendering_cookie = cookie; + } + /* Copy the new buffer offsets back to the user's validate list. */ ret = copy_to_user((struct drm_i915_relocation_entry __user*)(uintptr_t) args->buffers_ptr, @@ -495,28 +602,6 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, if (ret) DRM_ERROR ("failed to copy %d validate entries back to user (%d)\n", args->buffer_count, ret); - - /* Clean up and return */ - ret = i915_gem_sync(dev); - if (ret) - { - drm_i915_private_t *dev_priv = dev->dev_private; - uint32_t acthd = I915_READ(IS_I965G(dev) ? I965REG_ACTHD : I915REG_ACTHD); - DRM_ERROR ("failed to sync %d acthd %08x\n", ret, acthd); - i915_gem_dump_object (object_list[args->buffer_count - 1], - args->batch_len, - __FUNCTION__, - acthd); - } - - /* Evict all the buffers we moved in, leaving room for the next guy. */ - 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; - - if (obj_priv->pin_count == 0) - i915_gem_object_unbind(obj); - } err: if (object_list != NULL) { for (i = 0; i < args->buffer_count; i++) @@ -595,7 +680,7 @@ int i915_gem_init_object(struct drm_gem_object *obj) return -ENOMEM; obj->driver_private = obj_priv; - + INIT_LIST_HEAD(&obj_priv->gtt_lru_entry); return 0; } -- cgit v1.2.3 From 61253f4f677518537368103799c9510b8b5ad1e3 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Tue, 6 May 2008 20:00:23 -0700 Subject: [intel-GEM] Add memory domain support. Memory domains allow the kernel to track which caches to flush and how to move objects before buffer execution. --- linux-core/i915_gem.c | 156 +++++++++++++++++++++++++++++++++++++++++--------- 1 file changed, 130 insertions(+), 26 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index d630b0b2..911f9aa8 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -30,8 +30,8 @@ #include "i915_drm.h" #include "i915_drv.h" -#define WATCH_BUF 0 -#define WATCH_EXEC 0 +#define WATCH_BUF 1 +#define WATCH_EXEC 1 int i915_gem_init_ioctl(struct drm_device *dev, void *data, @@ -73,9 +73,33 @@ i915_gem_object_free_page_list(struct drm_gem_object *obj) obj_priv->page_list = NULL; } +static void +i915_gem_flush(struct drm_device *dev, uint32_t domains) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + uint32_t cmd; + RING_LOCALS; + +#if WATCH_EXEC + DRM_INFO ("%s: flush %08x\n", __FUNCTION__, domains); +#endif + cmd = CMD_MI_FLUSH | MI_NO_WRITE_FLUSH; + if (domains & DRM_GEM_DOMAIN_I915_RENDER) + cmd &= ~MI_NO_WRITE_FLUSH; + if (domains & DRM_GEM_DOMAIN_I915_SAMPLER) + cmd |= MI_READ_FLUSH; + if (domains & DRM_GEM_DOMAIN_I915_INSTRUCTION) + cmd |= MI_EXE_FLUSH; + + BEGIN_LP_RING(2); + OUT_RING(cmd); + OUT_RING(0); /* noop */ + ADVANCE_LP_RING(); +} + /** * Ensures that all rendering to the object has completed and the object is - * safe to unbind from the GTT. + * safe to unbind from the GTT or access from the CPU. */ static int i915_gem_object_wait_rendering(struct drm_gem_object *obj) @@ -84,10 +108,27 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) struct drm_i915_gem_object *obj_priv = obj->driver_private; int ret; + /* If there are writes queued to the buffer, flush and + * create a new cookie to wait for. + */ + if (obj->write_domain & ~(DRM_GEM_DOMAIN_CPU)) + { +#if WATCH_BUF + DRM_INFO ("%s: flushing object %p from write domain %08x\n", + __FUNCTION__, obj, obj->write_domain); +#endif + i915_gem_flush (dev, obj->write_domain); + obj->write_domain = 0; + obj_priv->last_rendering_cookie = i915_emit_irq (dev); + } /* If there is rendering queued on the buffer being evicted, wait for * it. */ if (obj_priv->last_rendering_cookie != 0) { +#if WATCH_BUF + DRM_INFO ("%s: object %p wait for cookie %08x\n", + __FUNCTION__, obj, obj_priv->last_rendering_cookie); +#endif ret = i915_wait_irq(dev, obj_priv->last_rendering_cookie); if (ret != 0) return ret; @@ -125,6 +166,7 @@ i915_gem_object_unbind(struct drm_gem_object *obj) drm_memrange_put_block(obj_priv->gtt_space); obj_priv->gtt_space = NULL; list_del_init(&obj_priv->gtt_lru_entry); + drm_gem_object_unreference (obj); } #if 0 @@ -277,10 +319,6 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) unlock_page (obj_priv->page_list[i]); } - drm_ttm_cache_flush (obj_priv->page_list, page_count); - DRM_MEMORYBARRIER(); - drm_agp_chipset_flush(dev); - /* Create an AGP memory structure pointing at our pages, and bind it * into the GTT. */ @@ -298,6 +336,73 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) return 0; } +static void +i915_gem_clflush_object (struct drm_gem_object *obj) +{ + struct drm_device *dev = obj->dev; + struct drm_i915_gem_object *obj_priv = obj->driver_private; + + drm_ttm_cache_flush (obj_priv->page_list, obj->size / PAGE_SIZE); + drm_agp_chipset_flush(dev); +} + +/* + * Set the next domain for the specified object. This + * may not actually perform the necessary flushing/invaliding though, + * as that may want to be batched with other set_domain operations + */ +static void +i915_gem_object_set_domain (struct drm_gem_object *obj, + uint32_t read_domains, + uint32_t write_domain) +{ + struct drm_device *dev = obj->dev; + uint32_t invalidate_domains = 0; + uint32_t flush_domains = 0; + +#if WATCH_BUF + DRM_INFO ("%s: object %p read %08x write %08x\n", + __FUNCTION__, obj, read_domains, write_domain); +#endif + /* + * Flush the current write domain if + * the new read domains don't match. Invalidate + * any read domains which differ from the old + * write domain + */ + if (obj->write_domain && obj->write_domain != read_domains) + { + flush_domains |= obj->write_domain; + invalidate_domains |= read_domains & ~obj->write_domain; + } + /* + * Invalidate any read caches which may have + * 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 WATCH_BUF + DRM_INFO ("%s: CPU domain flush %08x invalidate %08x\n", + __FUNCTION__, flush_domains, invalidate_domains); +#endif + /* + * If we're invaliding the CPU cache and flushing a GPU cache, + * 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)) + i915_gem_object_wait_rendering (obj); + i915_gem_clflush_object (obj); + } + + obj->write_domain = write_domain; + obj->read_domains = read_domains; + dev->invalidate_domains |= invalidate_domains & ~DRM_GEM_DOMAIN_CPU; + dev->flush_domains |= flush_domains & ~DRM_GEM_DOMAIN_CPU; +} + static int i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, struct drm_file *file_priv, @@ -318,12 +423,17 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, if (obj_priv->gtt_space == NULL) return -ENOMEM; } + + /* Do domain migration */ + i915_gem_object_set_domain (obj, entry->read_domains, entry->write_domain); + entry->buffer_offset = obj_priv->gtt_offset; if (obj_priv->pin_count == 0) { /* Move our buffer to the head of the LRU. */ list_del_init(&obj_priv->gtt_lru_entry); list_add(&obj_priv->gtt_lru_entry, &dev_priv->mm.gtt_lru); + drm_gem_object_reference (obj); } relocs = (struct drm_i915_gem_relocation_entry __user *) (uintptr_t) entry->relocs_ptr; @@ -413,18 +523,6 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, return 0; } -static void -i915_gem_flush(struct drm_device *dev) -{ - drm_i915_private_t *dev_priv = dev->dev_private; - RING_LOCALS; - - BEGIN_LP_RING(2); - OUT_RING(CMD_MI_FLUSH | MI_READ_FLUSH | MI_EXE_FLUSH); - OUT_RING(0); /* noop */ - ADVANCE_LP_RING(); -} - static int i915_dispatch_gem_execbuffer (struct drm_device * dev, struct drm_i915_gem_execbuffer * exec, @@ -556,6 +654,17 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, } } + if (dev->invalidate_domains | dev->flush_domains) + { +#if WATCH_EXEC + DRM_INFO ("%s: invalidate_domains %08x flush_domains %08x\n", + __FUNCTION__, dev->invalidate_domains, dev->flush_domains); +#endif + i915_gem_flush (dev, dev->invalidate_domains | dev->flush_domains); + dev->invalidate_domains = 0; + dev->flush_domains = 0; + } + exec_offset = validate_list[args->buffer_count - 1].buffer_offset; /* make sure all previous memory operations have passed */ @@ -575,14 +684,9 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, goto err; } - /* Flush the rendering. We want this flush to go away, which will - * require intelligent cache management. - */ - i915_gem_flush(dev); - - /* Get a cookie representing the flush of the current buffer, which we + /* Get a cookie representing the execution of the current buffer, which we * can wait on. We would like to mitigate these interrupts, likely by - * only flushing occasionally (so that we have *some* interrupts + * only creating cookies occasionally (so that we have *some* interrupts * representing completion of buffers that we can wait on when trying * to clear up gtt space). */ -- cgit v1.2.3 From 6a6c37af9ecaabfe1399a1300cadaff730767013 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Tue, 6 May 2008 21:59:06 -0700 Subject: [intel-GEM] ref count objects in gtt-lru. If objects on the lru aren't ref counted, they'll get pulled from the gtt as soon as they are freed. This change does cause objects to get stuck in the gtt until they're forced out by new requests. The lru should get cleaned when the irq occurs. --- linux-core/i915_gem.c | 62 ++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 49 insertions(+), 13 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 911f9aa8..220d6179 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -30,8 +30,9 @@ #include "i915_drm.h" #include "i915_drv.h" -#define WATCH_BUF 1 -#define WATCH_EXEC 1 +#define WATCH_BUF 0 +#define WATCH_EXEC 0 +#define WATCH_LRU 0 int i915_gem_init_ioctl(struct drm_device *dev, void *data, @@ -165,11 +166,14 @@ i915_gem_object_unbind(struct drm_gem_object *obj) drm_memrange_put_block(obj_priv->gtt_space); obj_priv->gtt_space = NULL; - list_del_init(&obj_priv->gtt_lru_entry); - drm_gem_object_unreference (obj); + if (!list_empty (&obj_priv->gtt_lru_entry)) + { + list_del_init(&obj_priv->gtt_lru_entry); + drm_gem_object_unreference (obj); + } } -#if 0 +#if WATCH_BUF | WATCH_EXEC static void i915_gem_dump_page (struct page *page, uint32_t start, uint32_t end, uint32_t bias, uint32_t mark) { @@ -212,6 +216,21 @@ i915_gem_dump_object (struct drm_gem_object *obj, int len, const char *where, ui } #endif +#if WATCH_LRU +static void +i915_dump_lru (struct drm_device *dev, const char *where) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_gem_object *obj_priv; + + DRM_INFO ("GTT LRU %s {\n", where); + list_for_each_entry (obj_priv, &dev_priv->mm.gtt_lru, gtt_lru_entry) { + DRM_INFO (" %p: %08x\n", obj_priv, obj_priv->last_rendering_cookie); + } + DRM_INFO ("}\n"); +} +#endif + static int i915_gem_evict_something(struct drm_device *dev) { @@ -221,11 +240,14 @@ i915_gem_evict_something(struct drm_device *dev) int ret; /* Find the LRU buffer. */ - BUG_ON(!list_empty(&dev_priv->mm.gtt_lru)); - obj_priv = list_entry(dev_priv->mm.gtt_lru.prev, - struct drm_i915_gem_object, - gtt_lru_entry); + BUG_ON(list_empty(&dev_priv->mm.gtt_lru)); + obj_priv = list_first_entry(&dev_priv->mm.gtt_lru, + struct drm_i915_gem_object, + gtt_lru_entry); obj = obj_priv->obj; +#if WATCH_LRU + DRM_INFO ("%s: evicting %p\n", __FUNCTION__, obj); +#endif /* Only unpinned buffers should be on this list. */ BUG_ON(obj_priv->pin_count != 0); @@ -240,6 +262,9 @@ i915_gem_evict_something(struct drm_device *dev) /* Wait on the rendering and unbind the buffer. */ i915_gem_object_unbind(obj); +#if WATCH_LRU + DRM_INFO ("%s: evicted %p\n", __FUNCTION__, obj); +#endif return 0; } @@ -280,8 +305,13 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) /* If the gtt is empty and we're still having trouble * fitting our object in, we're out of memory. */ - if (list_empty(&dev_priv->mm.gtt_lru)) +#if WATCH_LRU + DRM_INFO ("%s: GTT full, evicting something\n", __FUNCTION__); +#endif + if (list_empty(&dev_priv->mm.gtt_lru)) { + DRM_ERROR ("GTT full, but LRU list empty\n"); return -ENOMEM; + } ret = i915_gem_evict_something(dev); if (ret != 0) @@ -431,9 +461,14 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, if (obj_priv->pin_count == 0) { /* Move our buffer to the head of the LRU. */ - list_del_init(&obj_priv->gtt_lru_entry); - list_add(&obj_priv->gtt_lru_entry, &dev_priv->mm.gtt_lru); - drm_gem_object_reference (obj); + if (list_empty (&obj_priv->gtt_lru_entry)) { + drm_gem_object_reference (obj); + list_add_tail(&obj_priv->gtt_lru_entry, &dev_priv->mm.gtt_lru); + } else + list_move_tail(&obj_priv->gtt_lru_entry, &dev_priv->mm.gtt_lru); +#if WATCH_LRU && 0 + i915_dump_lru (dev, __FUNCTION__); +#endif } relocs = (struct drm_i915_gem_relocation_entry __user *) (uintptr_t) entry->relocs_ptr; @@ -784,6 +819,7 @@ int i915_gem_init_object(struct drm_gem_object *obj) return -ENOMEM; obj->driver_private = obj_priv; + obj_priv->obj = obj; INIT_LIST_HEAD(&obj_priv->gtt_lru_entry); return 0; } -- cgit v1.2.3 From 5f5f01ed91f5ad50f2b38e11740a30441ac845a4 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Wed, 7 May 2008 12:46:06 -0700 Subject: GEM: Extend cache domain stuff for 965. One of our MI_FLUSH bits is reserved on 965, being always implied, and there's a vertex cache that was forgotten. --- linux-core/i915_gem.c | 40 +++++++++++++++++++++++++++++++++++++--- 1 file changed, 37 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 220d6179..f3adf39b 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -84,14 +84,41 @@ i915_gem_flush(struct drm_device *dev, uint32_t domains) #if WATCH_EXEC DRM_INFO ("%s: flush %08x\n", __FUNCTION__, domains); #endif + + /* read/write caches: + * DRM_GEM_DOMAIN_I915_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 MI_READ_FLUSH + * is set, and is always flushed on 965. + * DRM_GEM_DOMAIN_I915_COMMAND may not exist? + * DRM_GEM_DOMAIN_I915_INSTRUCTION, which exists on 965, is invalidated + * when MI_EXE_FLUSH is set. + * DRM_GEM_DOMAIN_I915_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 are + * flushed at any MI_FLUSH. + */ + cmd = CMD_MI_FLUSH | MI_NO_WRITE_FLUSH; if (domains & DRM_GEM_DOMAIN_I915_RENDER) cmd &= ~MI_NO_WRITE_FLUSH; - if (domains & DRM_GEM_DOMAIN_I915_SAMPLER) - cmd |= MI_READ_FLUSH; + if (!IS_I965G(dev)) { + /* On the 965, the sampler cache always gets flushed and this + * bit is reserved. + */ + if (domains & DRM_GEM_DOMAIN_I915_SAMPLER) + cmd |= MI_READ_FLUSH; + } if (domains & DRM_GEM_DOMAIN_I915_INSTRUCTION) cmd |= MI_EXE_FLUSH; - + BEGIN_LP_RING(2); OUT_RING(cmd); OUT_RING(0); /* noop */ @@ -363,6 +390,13 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) return -ENOMEM; } + /* When we have just bound an object, we have no valid read + * caches on it, regardless of where it was before. We also need + * an MI_FLUSH to occur so that the render and sampler TLBs + * get flushed and pick up our binding change above. + */ + obj->read_domains = 0; + return 0; } -- cgit v1.2.3 From 06e9761f94599c6378c8fa0cdbd1e1c1776bae7a Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Wed, 7 May 2008 14:10:04 -0700 Subject: GEM: Wait for existing rendering to complete before writing relocation data. This should already have been generally safe since we don't change contents and put in new relocations between execbufs, so if we were writing in a new relocation then we'd already waited rendering to complete when we moved the target of the relocation. However, doing the right thing will be required if we do buffer reuse. --- linux-core/i915_gem.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index f3adf39b..1d55eaa2 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -546,9 +546,18 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, return -EINVAL; } + /* If the relocation already has the right value in it, no + * more work needs to be done. + */ if (target_obj_priv->gtt_offset == reloc.presumed_offset) continue; + /* Now that we're going to actually write some data in, + * make sure that any rendering using this buffer's contents + * is completed. + */ + i915_gem_object_wait_rendering(obj); + /* Map the page containing the relocation we're going to * perform. */ -- cgit v1.2.3 From 9af4c497433398fa4576a7c1c31036448cf4f24c Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Thu, 8 May 2008 10:44:02 -0700 Subject: [intel-gem] Move domains to relocation records. add set_domain ioctl. Domain information is about buffer relationships, not buffer contents. That means a relocation contains the domain information as it knows how the source buffer references the target buffer. This also adds the set_domain ioctl so that user space can move buffers to the cpu domain. --- linux-core/i915_gem.c | 235 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 169 insertions(+), 66 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 1d55eaa2..861e7bb7 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -33,6 +33,7 @@ #define WATCH_BUF 0 #define WATCH_EXEC 0 #define WATCH_LRU 0 +#define WATCH_RELOC 0 int i915_gem_init_ioctl(struct drm_device *dev, void *data, @@ -75,54 +76,61 @@ i915_gem_object_free_page_list(struct drm_gem_object *obj) } static void -i915_gem_flush(struct drm_device *dev, uint32_t domains) +i915_gem_flush(struct drm_device *dev, uint32_t invalidate_domains, uint32_t flush_domains) { drm_i915_private_t *dev_priv = dev->dev_private; uint32_t cmd; RING_LOCALS; #if WATCH_EXEC - DRM_INFO ("%s: flush %08x\n", __FUNCTION__, domains); + DRM_INFO ("%s: invalidate %08x flush %08x\n", __FUNCTION__, + invalidate_domains, flush_domains); #endif - /* read/write caches: - * DRM_GEM_DOMAIN_I915_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 MI_READ_FLUSH - * is set, and is always flushed on 965. - * DRM_GEM_DOMAIN_I915_COMMAND may not exist? - * DRM_GEM_DOMAIN_I915_INSTRUCTION, which exists on 965, is invalidated - * when MI_EXE_FLUSH is set. - * DRM_GEM_DOMAIN_I915_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 are - * flushed at any MI_FLUSH. - */ - - cmd = CMD_MI_FLUSH | MI_NO_WRITE_FLUSH; - if (domains & DRM_GEM_DOMAIN_I915_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 (flush_domains & DRM_GEM_DOMAIN_CPU) + drm_agp_chipset_flush(dev); + + if ((invalidate_domains|flush_domains) & ~DRM_GEM_DOMAIN_CPU) + { + /* read/write caches: + * DRM_GEM_DOMAIN_I915_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 MI_READ_FLUSH + * is set, and is always flushed on 965. + * DRM_GEM_DOMAIN_I915_COMMAND may not exist? + * DRM_GEM_DOMAIN_I915_INSTRUCTION, which exists on 965, is invalidated + * when MI_EXE_FLUSH is set. + * DRM_GEM_DOMAIN_I915_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 are + * flushed at any MI_FLUSH. */ - if (domains & DRM_GEM_DOMAIN_I915_SAMPLER) - cmd |= MI_READ_FLUSH; + + cmd = CMD_MI_FLUSH | MI_NO_WRITE_FLUSH; + if ((invalidate_domains|flush_domains) & DRM_GEM_DOMAIN_I915_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) + cmd |= MI_READ_FLUSH; + } + if (invalidate_domains & DRM_GEM_DOMAIN_I915_INSTRUCTION) + cmd |= MI_EXE_FLUSH; + + BEGIN_LP_RING(2); + OUT_RING(cmd); + OUT_RING(0); /* noop */ + ADVANCE_LP_RING(); } - if (domains & DRM_GEM_DOMAIN_I915_INSTRUCTION) - cmd |= MI_EXE_FLUSH; - - BEGIN_LP_RING(2); - OUT_RING(cmd); - OUT_RING(0); /* noop */ - ADVANCE_LP_RING(); } /** @@ -145,8 +153,10 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) DRM_INFO ("%s: flushing object %p from write domain %08x\n", __FUNCTION__, obj, obj->write_domain); #endif - i915_gem_flush (dev, obj->write_domain); + i915_gem_flush (dev, 0, obj->write_domain); obj->write_domain = 0; + if (obj_priv->last_rendering_cookie == 0) + drm_gem_object_reference (obj); obj_priv->last_rendering_cookie = i915_emit_irq (dev); } /* If there is rendering queued on the buffer being evicted, wait for @@ -162,6 +172,9 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) return ret; /* Clear it now that we know it's passed. */ obj_priv->last_rendering_cookie = 0; + + /* The cookie held a reference to the object, release that now */ + drm_gem_object_unreference (obj); } return 0; @@ -194,10 +207,7 @@ i915_gem_object_unbind(struct drm_gem_object *obj) drm_memrange_put_block(obj_priv->gtt_space); obj_priv->gtt_space = NULL; if (!list_empty (&obj_priv->gtt_lru_entry)) - { list_del_init(&obj_priv->gtt_lru_entry); - drm_gem_object_unreference (obj); - } } #if WATCH_BUF | WATCH_EXEC @@ -403,11 +413,9 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) static void i915_gem_clflush_object (struct drm_gem_object *obj) { - struct drm_device *dev = obj->dev; struct drm_i915_gem_object *obj_priv = obj->driver_private; drm_ttm_cache_flush (obj_priv->page_list, obj->size / PAGE_SIZE); - drm_agp_chipset_flush(dev); } /* @@ -463,8 +471,32 @@ i915_gem_object_set_domain (struct drm_gem_object *obj, obj->write_domain = write_domain; obj->read_domains = read_domains; - dev->invalidate_domains |= invalidate_domains & ~DRM_GEM_DOMAIN_CPU; - dev->flush_domains |= flush_domains & ~DRM_GEM_DOMAIN_CPU; + dev->invalidate_domains |= invalidate_domains; + dev->flush_domains |= flush_domains; +} + +/** + * Once all of the objects have been set in the proper domain, + * perform the necessary flush and invalidate operations + */ + +static void +i915_gem_dev_set_domain (struct drm_device *dev) +{ + /* + * Now that all the buffers are synced to the proper domains, + * flush and invalidate the collected domains + */ + if (dev->invalidate_domains | dev->flush_domains) + { +#if WATCH_EXEC + DRM_INFO ("%s: invalidate_domains %08x flush_domains %08x\n", + __FUNCTION__, dev->invalidate_domains, dev->flush_domains); +#endif + i915_gem_flush (dev, dev->invalidate_domains, dev->flush_domains); + dev->invalidate_domains = 0; + dev->flush_domains = 0; + } } static int @@ -488,17 +520,13 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, return -ENOMEM; } - /* Do domain migration */ - i915_gem_object_set_domain (obj, entry->read_domains, entry->write_domain); - entry->buffer_offset = obj_priv->gtt_offset; if (obj_priv->pin_count == 0) { /* Move our buffer to the head of the LRU. */ - if (list_empty (&obj_priv->gtt_lru_entry)) { - drm_gem_object_reference (obj); + if (list_empty (&obj_priv->gtt_lru_entry)) list_add_tail(&obj_priv->gtt_lru_entry, &dev_priv->mm.gtt_lru); - } else + else list_move_tail(&obj_priv->gtt_lru_entry, &dev_priv->mm.gtt_lru); #if WATCH_LRU && 0 i915_dump_lru (dev, __FUNCTION__); @@ -536,16 +564,44 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, } if (reloc.offset > obj->size - 4) { - DRM_ERROR("Relocation beyond object bounds.\n"); + DRM_ERROR("Relocation beyond object bounds: obj %p target %d offset %d size %d.\n", + obj, reloc.target_handle, (int) reloc.offset, (int) obj->size); drm_gem_object_unreference (target_obj); return -EINVAL; } if (reloc.offset & 3) { - DRM_ERROR("Relocation not 4-byte aligned.\n"); + DRM_ERROR("Relocation not 4-byte aligned: obj %p target %d offset %d.\n", + obj, reloc.target_handle, (int) reloc.offset); drm_gem_object_unreference (target_obj); return -EINVAL; } + if (reloc.write_domain && target_obj->pending_write_domain && + reloc.write_domain != target_obj->pending_write_domain) + { + DRM_ERROR("Write domain conflict: obj %p target %d offset %d new %08x old %08x\n", + obj, reloc.target_handle, (int) reloc.offset, + reloc.write_domain, target_obj->pending_write_domain); + drm_gem_object_unreference (target_obj); + return -EINVAL; + } + +#if WATCH_RELOC + DRM_INFO ("%s: obj %p offset %08x target %d read %08x write %08x gtt %08x presumed %08x delta %08x\n", + __FUNCTION__, + obj, + (int) reloc.offset, + (int) reloc.target_handle, + (int) reloc.read_domains, + (int) reloc.write_domain, + (int) target_obj_priv->gtt_offset, + (int) reloc.presumed_offset, + reloc.delta); +#endif + + target_obj->pending_read_domains |= reloc.read_domains; + target_obj->pending_write_domain |= reloc.write_domain; + /* If the relocation already has the right value in it, no * more work needs to be done. */ @@ -558,6 +614,16 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, */ i915_gem_object_wait_rendering(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) + { + i915_gem_clflush_object (obj); + drm_agp_chipset_flush(dev); + obj->write_domain = 0; + } + /* Map the page containing the relocation we're going to * perform. */ @@ -672,6 +738,19 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, LOCK_TEST_WITH_RETURN(dev, file_priv); +#if 0 + /* + * XXX wait for previous rendering to complete as we otherwise never + * flush the LRU list + */ + { + drm_i915_private_t *dev_priv = dev->dev_private; + + while (!list_empty (&dev_priv->mm.gtt_lru)) + i915_gem_evict_something (dev); + } +#endif + #if WATCH_EXEC DRM_INFO ("buffers_ptr %d buffer_count %d len %08x\n", (int) args->buffers_ptr, args->buffer_count, args->batch_len); @@ -717,6 +796,10 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, } } + /* Set the pending read domains for the batch buffer to COMMAND */ + object_list[args->buffer_count-1]->pending_read_domains = DRM_GEM_DOMAIN_I915_COMMAND; + object_list[args->buffer_count-1]->pending_write_domain = 0; + 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; @@ -730,22 +813,19 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, ret = -ENOMEM; goto err; } - } - if (dev->invalidate_domains | dev->flush_domains) - { -#if WATCH_EXEC - DRM_INFO ("%s: invalidate_domains %08x flush_domains %08x\n", - __FUNCTION__, dev->invalidate_domains, dev->flush_domains); -#endif - i915_gem_flush (dev, dev->invalidate_domains | dev->flush_domains); - dev->invalidate_domains = 0; - dev->flush_domains = 0; + /* make sure all previous memory operations have passed */ + i915_gem_object_set_domain (obj, + obj->pending_read_domains, + obj->pending_write_domain); + obj->pending_read_domains = 0; + obj->pending_write_domain = 0; } - exec_offset = validate_list[args->buffer_count - 1].buffer_offset; + /* Flush/invalidate caches and chipset buffer */ + i915_gem_dev_set_domain (dev); - /* make sure all previous memory operations have passed */ + exec_offset = validate_list[args->buffer_count - 1].buffer_offset; #if WATCH_EXEC i915_gem_dump_object (object_list[args->buffer_count - 1], @@ -773,6 +853,12 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, struct drm_gem_object *obj = object_list[i]; struct drm_i915_gem_object *obj_priv = obj->driver_private; + /* + * Have the cookie hold a reference to this object + * which is freed when the object is waited for + */ + if (obj_priv->last_rendering_cookie == 0) + drm_gem_object_reference (obj); obj_priv->last_rendering_cookie = cookie; } @@ -789,6 +875,13 @@ err: for (i = 0; i < args->buffer_count; i++) drm_gem_object_unreference(object_list[i]); } + + /* XXX kludge for now as we don't clean the exec ring yet */ + if (object_list != NULL) { + for (i = 0; i < args->buffer_count; i++) + i915_gem_object_wait_rendering (object_list[i]); + } + drm_free(object_list, sizeof(*object_list) * args->buffer_count, DRM_MEM_DRIVER); drm_free(validate_list, sizeof(*validate_list) * args->buffer_count, @@ -873,3 +966,13 @@ void i915_gem_free_object(struct drm_gem_object *obj) drm_free(obj->driver_private, 1, DRM_MEM_DRIVER); } + +int +i915_gem_set_domain_ioctl (struct drm_gem_object *obj, + uint32_t read_domains, + uint32_t write_domain) +{ + i915_gem_object_set_domain (obj, read_domains, write_domain); + i915_gem_dev_set_domain (obj->dev); + return 0; +} -- cgit v1.2.3 From 2f573e6df4890784124eea24ce168702574f0152 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 8 May 2008 12:46:02 -0700 Subject: GEM: Fix oops on NULL dereference when we try clflushing when we don't need to. --- linux-core/i915_gem.c | 7 +++++++ 1 file changed, 7 insertions(+) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 861e7bb7..5c58032b 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -415,6 +415,13 @@ i915_gem_clflush_object (struct drm_gem_object *obj) { struct drm_i915_gem_object *obj_priv = obj->driver_private; + /* If we don't have a page list set up, then we're not pinned + * to GPU, and we can ignore the cache flush because it'll happen + * again at bind time. + */ + if (obj_priv->page_list == NULL) + return; + drm_ttm_cache_flush (obj_priv->page_list, obj->size / PAGE_SIZE); } -- cgit v1.2.3 From 07ad5ce1e199ebca1e51a831503f923fa49cc57e Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Thu, 8 May 2008 11:13:29 -0700 Subject: Clean up whinging from checkpatch.pl in drm_gem.c Whitespace changes, a few too-long-lines and some extra braces. --- 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 5c58032b..d25c99a9 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -883,11 +883,13 @@ err: drm_gem_object_unreference(object_list[i]); } +#if 0 /* XXX kludge for now as we don't clean the exec ring yet */ if (object_list != NULL) { for (i = 0; i < args->buffer_count; i++) i915_gem_object_wait_rendering (object_list[i]); } +#endif drm_free(object_list, sizeof(*object_list) * args->buffer_count, DRM_MEM_DRIVER); -- cgit v1.2.3 From ec75369b402235d74b06b08907572050962075a6 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Thu, 8 May 2008 11:45:53 -0700 Subject: [i915] clean up whinging from checkpatch.pl --- linux-core/i915_gem.c | 410 +++++++++++++++++++++++++++----------------------- 1 file changed, 218 insertions(+), 192 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index d25c99a9..ff8e2762 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -63,12 +63,11 @@ i915_gem_object_free_page_list(struct drm_gem_object *obj) if (obj_priv->page_list == NULL) return; - - for (i = 0; i < page_count; i++) { - if (obj_priv->page_list[i] != NULL) { - page_cache_release (obj_priv->page_list[i]); - } - } + + for (i = 0; i < page_count; i++) + if (obj_priv->page_list[i] != NULL) + page_cache_release(obj_priv->page_list[i]); + drm_free(obj_priv->page_list, page_count * sizeof(struct page *), DRM_MEM_DRIVER); @@ -76,56 +75,66 @@ i915_gem_object_free_page_list(struct drm_gem_object *obj) } static void -i915_gem_flush(struct drm_device *dev, uint32_t invalidate_domains, uint32_t flush_domains) +i915_gem_flush(struct drm_device *dev, + uint32_t invalidate_domains, + uint32_t flush_domains) { drm_i915_private_t *dev_priv = dev->dev_private; - uint32_t cmd; + uint32_t cmd; RING_LOCALS; #if WATCH_EXEC - DRM_INFO ("%s: invalidate %08x flush %08x\n", __FUNCTION__, + DRM_INFO("%s: invalidate %08x flush %08x\n", __func__, invalidate_domains, flush_domains); #endif if (flush_domains & DRM_GEM_DOMAIN_CPU) drm_agp_chipset_flush(dev); - - if ((invalidate_domains|flush_domains) & ~DRM_GEM_DOMAIN_CPU) - { - /* read/write caches: + + if ((invalidate_domains|flush_domains) & ~DRM_GEM_DOMAIN_CPU) { + /* + * read/write caches: + * * DRM_GEM_DOMAIN_I915_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. + * 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 MI_READ_FLUSH - * is set, and is always flushed on 965. + * + * DRM_GEM_DOMAIN_I915_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? - * DRM_GEM_DOMAIN_I915_INSTRUCTION, which exists on 965, is invalidated - * when MI_EXE_FLUSH is set. - * DRM_GEM_DOMAIN_I915_VERTEX, which exists on 965, is invalidated with - * every MI_FLUSH. + * + * DRM_GEM_DOMAIN_I915_INSTRUCTION, which exists on 965, is + * invalidated when MI_EXE_FLUSH is set. + * + * DRM_GEM_DOMAIN_I915_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 are - * flushed at any MI_FLUSH. + * + * 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 + * are flushed at any MI_FLUSH. */ - + cmd = CMD_MI_FLUSH | MI_NO_WRITE_FLUSH; - if ((invalidate_domains|flush_domains) & DRM_GEM_DOMAIN_I915_RENDER) + if ((invalidate_domains|flush_domains) & + DRM_GEM_DOMAIN_I915_RENDER) cmd &= ~MI_NO_WRITE_FLUSH; if (!IS_I965G(dev)) { - /* On the 965, the sampler cache always gets flushed and this - * bit is reserved. + /* + * On the 965, the sampler cache always gets flushed + * and this bit is reserved. */ if (invalidate_domains & DRM_GEM_DOMAIN_I915_SAMPLER) cmd |= MI_READ_FLUSH; } if (invalidate_domains & DRM_GEM_DOMAIN_I915_INSTRUCTION) cmd |= MI_EXE_FLUSH; - + BEGIN_LP_RING(2); OUT_RING(cmd); OUT_RING(0); /* noop */ @@ -147,34 +156,36 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) /* If there are writes queued to the buffer, flush and * create a new cookie to wait for. */ - if (obj->write_domain & ~(DRM_GEM_DOMAIN_CPU)) - { + if (obj->write_domain & ~(DRM_GEM_DOMAIN_CPU)) { #if WATCH_BUF - DRM_INFO ("%s: flushing object %p from write domain %08x\n", - __FUNCTION__, obj, obj->write_domain); + DRM_INFO("%s: flushing object %p from write domain %08x\n", + __func__, obj, obj->write_domain); #endif - i915_gem_flush (dev, 0, obj->write_domain); + i915_gem_flush(dev, 0, obj->write_domain); obj->write_domain = 0; if (obj_priv->last_rendering_cookie == 0) - drm_gem_object_reference (obj); - obj_priv->last_rendering_cookie = i915_emit_irq (dev); + drm_gem_object_reference(obj); + obj_priv->last_rendering_cookie = i915_emit_irq(dev); } /* If there is rendering queued on the buffer being evicted, wait for * it. */ if (obj_priv->last_rendering_cookie != 0) { #if WATCH_BUF - DRM_INFO ("%s: object %p wait for cookie %08x\n", - __FUNCTION__, obj, obj_priv->last_rendering_cookie); + DRM_INFO("%s: object %p wait for cookie %08x\n", + __func__, obj, obj_priv->last_rendering_cookie); #endif ret = i915_wait_irq(dev, obj_priv->last_rendering_cookie); if (ret != 0) return ret; /* Clear it now that we know it's passed. */ obj_priv->last_rendering_cookie = 0; - - /* The cookie held a reference to the object, release that now */ - drm_gem_object_unreference (obj); + + /* + * The cookie held a reference to the object, + * release that now + */ + drm_gem_object_unreference(obj); } return 0; @@ -189,8 +200,8 @@ i915_gem_object_unbind(struct drm_gem_object *obj) struct drm_i915_gem_object *obj_priv = obj->driver_private; #if WATCH_BUF - DRM_INFO ("%s:%d %p\n", __FUNCTION__, __LINE__, obj); - DRM_INFO ("gtt_space %p\n", obj_priv->gtt_space); + DRM_INFO("%s:%d %p\n", __func__, __LINE__, obj); + DRM_INFO("gtt_space %p\n", obj_priv->gtt_space); #endif if (obj_priv->gtt_space == NULL) return; @@ -206,34 +217,35 @@ i915_gem_object_unbind(struct drm_gem_object *obj) drm_memrange_put_block(obj_priv->gtt_space); obj_priv->gtt_space = NULL; - if (!list_empty (&obj_priv->gtt_lru_entry)) + if (!list_empty(&obj_priv->gtt_lru_entry)) list_del_init(&obj_priv->gtt_lru_entry); } #if WATCH_BUF | WATCH_EXEC static void -i915_gem_dump_page (struct page *page, uint32_t start, uint32_t end, uint32_t bias, uint32_t mark) +i915_gem_dump_page(struct page *page, uint32_t start, uint32_t end, + uint32_t bias, uint32_t mark) { - uint32_t *mem = kmap_atomic (page, KM_USER0); + uint32_t *mem = kmap_atomic(page, KM_USER0); int i; for (i = start; i < end; i += 4) - DRM_INFO ("%08x: %08x%s\n", + DRM_INFO("%08x: %08x%s\n", (int) (bias + i), mem[i / 4], (bias + i == mark) ? " ********" : ""); - kunmap_atomic (mem, KM_USER0); + kunmap_atomic(mem, KM_USER0); /* give syslog time to catch up */ - msleep (1); + msleep(1); } static void -i915_gem_dump_object (struct drm_gem_object *obj, int len, const char *where, uint32_t mark) +i915_gem_dump_object(struct drm_gem_object *obj, int len, + const char *where, uint32_t mark) { struct drm_i915_gem_object *obj_priv = obj->driver_private; int page; - DRM_INFO ("%s: object at offset %08x\n", where, obj_priv->gtt_offset); - for (page = 0; page < (len + PAGE_SIZE-1) / PAGE_SIZE; page++) - { + DRM_INFO("%s: object at offset %08x\n", where, obj_priv->gtt_offset); + for (page = 0; page < (len + PAGE_SIZE-1) / PAGE_SIZE; page++) { int page_len, chunk, chunk_len; page_len = len - page * PAGE_SIZE; @@ -244,10 +256,11 @@ i915_gem_dump_object (struct drm_gem_object *obj, int len, const char *where, ui chunk_len = page_len - chunk; if (chunk_len > 128) chunk_len = 128; - i915_gem_dump_page (obj_priv->page_list[page], - chunk, chunk + chunk_len, - obj_priv->gtt_offset + page * PAGE_SIZE, - mark); + i915_gem_dump_page(obj_priv->page_list[page], + chunk, chunk + chunk_len, + obj_priv->gtt_offset + + page * PAGE_SIZE, + mark); } } } @@ -255,16 +268,17 @@ i915_gem_dump_object (struct drm_gem_object *obj, int len, const char *where, ui #if WATCH_LRU static void -i915_dump_lru (struct drm_device *dev, const char *where) +i915_dump_lru(struct drm_device *dev, const char *where) { drm_i915_private_t *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj_priv; - - DRM_INFO ("GTT LRU %s {\n", where); - list_for_each_entry (obj_priv, &dev_priv->mm.gtt_lru, gtt_lru_entry) { - DRM_INFO (" %p: %08x\n", obj_priv, obj_priv->last_rendering_cookie); + + DRM_INFO("GTT LRU %s {\n", where); + list_for_each_entry(obj_priv, &dev_priv->mm.gtt_lru, gtt_lru_entry) { + DRM_INFO(" %p: %08x\n", obj_priv, + obj_priv->last_rendering_cookie); } - DRM_INFO ("}\n"); + DRM_INFO("}\n"); } #endif @@ -283,7 +297,7 @@ i915_gem_evict_something(struct drm_device *dev) gtt_lru_entry); obj = obj_priv->obj; #if WATCH_LRU - DRM_INFO ("%s: evicting %p\n", __FUNCTION__, obj); + DRM_INFO("%s: evicting %p\n", __func__, obj); #endif /* Only unpinned buffers should be on this list. */ @@ -300,7 +314,7 @@ i915_gem_evict_something(struct drm_device *dev) /* Wait on the rendering and unbind the buffer. */ i915_gem_object_unbind(obj); #if WATCH_LRU - DRM_INFO ("%s: evicted %p\n", __FUNCTION__, obj); + DRM_INFO("%s: evicted %p\n", __func__, obj); #endif return 0; @@ -343,10 +357,10 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) * fitting our object in, we're out of memory. */ #if WATCH_LRU - DRM_INFO ("%s: GTT full, evicting something\n", __FUNCTION__); + DRM_INFO("%s: GTT full, evicting something\n", __func__); #endif if (list_empty(&dev_priv->mm.gtt_lru)) { - DRM_ERROR ("GTT full, but LRU list empty\n"); + DRM_ERROR("GTT full, but LRU list empty\n"); return -ENOMEM; } @@ -357,7 +371,8 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) } #if WATCH_BUF - DRM_INFO ("Binding object of size %d at 0x%08x\n", obj->size, obj_priv->gtt_offset); + DRM_INFO("Binding object of size %d at 0x%08x\n", + obj->size, obj_priv->gtt_offset); #endif /* Get the list of pages out of our struct file. They'll be pinned @@ -383,9 +398,9 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) obj_priv->gtt_space = NULL; return -ENOMEM; } - unlock_page (obj_priv->page_list[i]); + unlock_page(obj_priv->page_list[i]); } - + /* Create an AGP memory structure pointing at our pages, and bind it * into the GTT. */ @@ -411,7 +426,7 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) } static void -i915_gem_clflush_object (struct drm_gem_object *obj) +i915_gem_clflush_object(struct drm_gem_object *obj) { struct drm_i915_gem_object *obj_priv = obj->driver_private; @@ -422,26 +437,26 @@ i915_gem_clflush_object (struct drm_gem_object *obj) if (obj_priv->page_list == NULL) return; - drm_ttm_cache_flush (obj_priv->page_list, obj->size / PAGE_SIZE); + drm_ttm_cache_flush(obj_priv->page_list, obj->size / PAGE_SIZE); } - + /* * Set the next domain for the specified object. This * may not actually perform the necessary flushing/invaliding though, * as that may want to be batched with other set_domain operations */ static void -i915_gem_object_set_domain (struct drm_gem_object *obj, +i915_gem_object_set_domain(struct drm_gem_object *obj, uint32_t read_domains, uint32_t write_domain) { struct drm_device *dev = obj->dev; uint32_t invalidate_domains = 0; uint32_t flush_domains = 0; - + #if WATCH_BUF - DRM_INFO ("%s: object %p read %08x write %08x\n", - __FUNCTION__, obj, read_domains, write_domain); + DRM_INFO("%s: object %p read %08x write %08x\n", + __func__, obj, read_domains, write_domain); #endif /* * Flush the current write domain if @@ -449,8 +464,7 @@ i915_gem_object_set_domain (struct drm_gem_object *obj, * any read domains which differ from the old * write domain */ - if (obj->write_domain && obj->write_domain != read_domains) - { + if (obj->write_domain && obj->write_domain != read_domains) { flush_domains |= obj->write_domain; invalidate_domains |= read_domains & ~obj->write_domain; } @@ -459,21 +473,20 @@ 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) & DRM_GEM_DOMAIN_CPU) { #if WATCH_BUF - DRM_INFO ("%s: CPU domain flush %08x invalidate %08x\n", - __FUNCTION__, flush_domains, invalidate_domains); + DRM_INFO("%s: CPU domain flush %08x invalidate %08x\n", + __func__, flush_domains, invalidate_domains); #endif /* * If we're invaliding the CPU cache and flushing a GPU cache, - * then pause for rendering so that the GPU caches will be + * 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)) - i915_gem_object_wait_rendering (obj); - i915_gem_clflush_object (obj); + i915_gem_object_wait_rendering(obj); + i915_gem_clflush_object(obj); } obj->write_domain = write_domain; @@ -488,19 +501,22 @@ i915_gem_object_set_domain (struct drm_gem_object *obj, */ static void -i915_gem_dev_set_domain (struct drm_device *dev) +i915_gem_dev_set_domain(struct drm_device *dev) { /* * Now that all the buffers are synced to the proper domains, * flush and invalidate the collected domains */ - if (dev->invalidate_domains | dev->flush_domains) - { + if (dev->invalidate_domains | dev->flush_domains) { #if WATCH_EXEC - DRM_INFO ("%s: invalidate_domains %08x flush_domains %08x\n", - __FUNCTION__, dev->invalidate_domains, dev->flush_domains); + DRM_INFO("%s: invalidate_domains %08x flush_domains %08x\n", + __func__, + dev->invalidate_domains, + dev->flush_domains); #endif - i915_gem_flush (dev, dev->invalidate_domains, dev->flush_domains); + i915_gem_flush(dev, + dev->invalidate_domains, + dev->flush_domains); dev->invalidate_domains = 0; dev->flush_domains = 0; } @@ -531,16 +547,19 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, if (obj_priv->pin_count == 0) { /* Move our buffer to the head of the LRU. */ - if (list_empty (&obj_priv->gtt_lru_entry)) - list_add_tail(&obj_priv->gtt_lru_entry, &dev_priv->mm.gtt_lru); + if (list_empty(&obj_priv->gtt_lru_entry)) + list_add_tail(&obj_priv->gtt_lru_entry, + &dev_priv->mm.gtt_lru); else - list_move_tail(&obj_priv->gtt_lru_entry, &dev_priv->mm.gtt_lru); + list_move_tail(&obj_priv->gtt_lru_entry, + &dev_priv->mm.gtt_lru); #if WATCH_LRU && 0 - i915_dump_lru (dev, __FUNCTION__); + i915_dump_lru(dev, __func__); #endif } - relocs = (struct drm_i915_gem_relocation_entry __user *) (uintptr_t) entry->relocs_ptr; + relocs = (struct drm_i915_gem_relocation_entry __user *) + (uintptr_t) entry->relocs_ptr; /* Apply the relocations, using the GTT aperture to avoid cache * flushing requirements. */ @@ -566,46 +585,55 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, if (target_obj_priv->gtt_space == NULL) { DRM_ERROR("No GTT space found for object %d\n", reloc.target_handle); - drm_gem_object_unreference (target_obj); + drm_gem_object_unreference(target_obj); return -EINVAL; } if (reloc.offset > obj->size - 4) { - DRM_ERROR("Relocation beyond object bounds: obj %p target %d offset %d size %d.\n", - obj, reloc.target_handle, (int) reloc.offset, (int) obj->size); - drm_gem_object_unreference (target_obj); + DRM_ERROR("Relocation beyond object bounds: " + "obj %p target %d offset %d size %d.\n", + obj, reloc.target_handle, + (int) reloc.offset, (int) obj->size); + drm_gem_object_unreference(target_obj); return -EINVAL; } if (reloc.offset & 3) { - DRM_ERROR("Relocation not 4-byte aligned: obj %p target %d offset %d.\n", - obj, reloc.target_handle, (int) reloc.offset); - drm_gem_object_unreference (target_obj); + DRM_ERROR("Relocation not 4-byte aligned: " + "obj %p target %d offset %d.\n", + obj, reloc.target_handle, + (int) reloc.offset); + drm_gem_object_unreference(target_obj); return -EINVAL; } if (reloc.write_domain && target_obj->pending_write_domain && - reloc.write_domain != target_obj->pending_write_domain) - { - DRM_ERROR("Write domain conflict: obj %p target %d offset %d new %08x old %08x\n", - obj, reloc.target_handle, (int) reloc.offset, - reloc.write_domain, target_obj->pending_write_domain); - drm_gem_object_unreference (target_obj); + reloc.write_domain != target_obj->pending_write_domain) { + DRM_ERROR("Write domain conflict: " + "obj %p target %d offset %d " + "new %08x old %08x\n", + obj, reloc.target_handle, + (int) reloc.offset, + reloc.write_domain, + target_obj->pending_write_domain); + drm_gem_object_unreference(target_obj); return -EINVAL; } - + #if WATCH_RELOC - DRM_INFO ("%s: obj %p offset %08x target %d read %08x write %08x gtt %08x presumed %08x delta %08x\n", - __FUNCTION__, - obj, - (int) reloc.offset, - (int) reloc.target_handle, - (int) reloc.read_domains, - (int) reloc.write_domain, - (int) target_obj_priv->gtt_offset, - (int) reloc.presumed_offset, - reloc.delta); + DRM_INFO("%s: obj %p offset %08x target %d " + "read %08x write %08x gtt %08x " + "presumed %08x delta %08x\n", + __func__, + obj, + (int) reloc.offset, + (int) reloc.target_handle, + (int) reloc.read_domains, + (int) reloc.write_domain, + (int) target_obj_priv->gtt_offset, + (int) reloc.presumed_offset, + reloc.delta); #endif - + target_obj->pending_read_domains |= reloc.read_domains; target_obj->pending_write_domain |= reloc.write_domain; @@ -624,9 +652,8 @@ i915_gem_reloc_and_validate_object(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) - { - i915_gem_clflush_object (obj); + if (obj->write_domain & DRM_GEM_DOMAIN_CPU) { + i915_gem_clflush_object(obj); drm_agp_chipset_flush(dev); obj->write_domain = 0; } @@ -646,7 +673,7 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, PAGE_SIZE); last_reloc_offset = reloc_offset; if (reloc_page == NULL) { - drm_gem_object_unreference (target_obj); + drm_gem_object_unreference(target_obj); return -ENOMEM; } } @@ -658,29 +685,30 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, #if WATCH_BUF DRM_INFO("Applied relocation: %p@0x%08x %08x -> %08x\n", obj, (unsigned int) reloc.offset, - readl (reloc_entry), reloc_val); + readl(reloc_entry), reloc_val); #endif - writel (reloc_val, reloc_entry); + writel(reloc_val, reloc_entry); - drm_gem_object_unreference (target_obj); + drm_gem_object_unreference(target_obj); } if (reloc_page != NULL) iounmap(reloc_page); #if WATCH_BUF - i915_gem_dump_object (obj, 128, __FUNCTION__, ~0); + i915_gem_dump_object(obj, 128, __func__, ~0); #endif return 0; } static int -i915_dispatch_gem_execbuffer (struct drm_device * dev, - struct drm_i915_gem_execbuffer * exec, +i915_dispatch_gem_execbuffer(struct drm_device *dev, + struct drm_i915_gem_execbuffer *exec, uint64_t exec_offset) { drm_i915_private_t *dev_priv = dev->dev_private; - struct drm_clip_rect __user *boxes = (struct drm_clip_rect __user *) (uintptr_t) exec->cliprects_ptr; + struct drm_clip_rect __user *boxes = (struct drm_clip_rect __user *) + (uintptr_t) exec->cliprects_ptr; int nbox = exec->num_cliprects; int i = 0, count; uint32_t exec_start, exec_len; @@ -688,7 +716,7 @@ i915_dispatch_gem_execbuffer (struct drm_device * dev, exec_start = (uint32_t) exec_offset + exec->batch_start_offset; exec_len = (uint32_t) exec->batch_len; - + if ((exec_start | exec_len) & 0x7) { DRM_ERROR("alignment\n"); return -EINVAL; @@ -710,10 +738,13 @@ i915_dispatch_gem_execbuffer (struct drm_device * dev, if (dev_priv->use_mi_batchbuffer_start) { BEGIN_LP_RING(2); if (IS_I965G(dev)) { - OUT_RING(MI_BATCH_BUFFER_START | (2 << 6) | MI_BATCH_NON_SECURE_I965); + OUT_RING(MI_BATCH_BUFFER_START | + (2 << 6) | + MI_BATCH_NON_SECURE_I965); OUT_RING(exec_start); } else { - OUT_RING(MI_BATCH_BUFFER_START | (2 << 6)); + OUT_RING(MI_BATCH_BUFFER_START | + (2 << 6)); OUT_RING(exec_start | MI_BATCH_NON_SECURE); } ADVANCE_LP_RING(); @@ -739,27 +770,15 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, struct drm_i915_gem_execbuffer *args = data; struct drm_i915_gem_validate_entry *validate_list; struct drm_gem_object **object_list; + struct drm_gem_object *batch_obj; int ret, i; uint64_t exec_offset; uint32_t cookie; LOCK_TEST_WITH_RETURN(dev, file_priv); -#if 0 - /* - * XXX wait for previous rendering to complete as we otherwise never - * flush the LRU list - */ - { - drm_i915_private_t *dev_priv = dev->dev_private; - - while (!list_empty (&dev_priv->mm.gtt_lru)) - i915_gem_evict_something (dev); - } -#endif - #if WATCH_EXEC - DRM_INFO ("buffers_ptr %d buffer_count %d len %08x\n", + 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); @@ -770,42 +789,47 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, object_list = drm_calloc(sizeof(*object_list), args->buffer_count, DRM_MEM_DRIVER); if (validate_list == NULL || object_list == NULL) { - DRM_ERROR ("Failed to allocate validate or object list for %d buffers\n", - args->buffer_count); + DRM_ERROR("Failed to allocate validate or object list " + "for %d buffers\n", + args->buffer_count); ret = -ENOMEM; goto err; } ret = copy_from_user(validate_list, - (struct drm_i915_relocation_entry __user*)(uintptr_t) - args->buffers_ptr, + (struct drm_i915_relocation_entry __user *) + (uintptr_t) args->buffers_ptr, sizeof(*validate_list) * args->buffer_count); if (ret != 0) { - DRM_ERROR ("copy %d validate entries failed %d\n", args->buffer_count, ret); + DRM_ERROR("copy %d validate entries failed %d\n", + args->buffer_count, ret); goto err; } /* Look up object handles and perform the relocations */ for (i = 0; i < args->buffer_count; i++) { object_list[i] = drm_gem_object_lookup(dev, file_priv, - validate_list[i].buffer_handle); + validate_list[i]. + buffer_handle); if (object_list[i] == NULL) { - DRM_ERROR ("Invalid object handle %d at index %d\n", + DRM_ERROR("Invalid object handle %d at index %d\n", validate_list[i].buffer_handle, i); ret = -EINVAL; goto err; } - ret = i915_gem_reloc_and_validate_object(object_list[i], file_priv, + ret = i915_gem_reloc_and_validate_object(object_list[i], + file_priv, &validate_list[i]); if (ret) { - DRM_ERROR ("reloc and validate failed %d\n", ret); + DRM_ERROR("reloc and validate failed %d\n", ret); goto err; } } /* Set the pending read domains for the batch buffer to COMMAND */ - object_list[args->buffer_count-1]->pending_read_domains = DRM_GEM_DOMAIN_I915_COMMAND; - object_list[args->buffer_count-1]->pending_write_domain = 0; + batch_obj = object_list[args->buffer_count-1]; + batch_obj->pending_read_domains = DRM_GEM_DOMAIN_I915_COMMAND; + batch_obj->pending_write_domain = 0; for (i = 0; i < args->buffer_count; i++) { struct drm_gem_object *obj = object_list[i]; @@ -822,7 +846,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, } /* make sure all previous memory operations have passed */ - i915_gem_object_set_domain (obj, + i915_gem_object_set_domain(obj, obj->pending_read_domains, obj->pending_write_domain); obj->pending_read_domains = 0; @@ -830,30 +854,30 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, } /* Flush/invalidate caches and chipset buffer */ - i915_gem_dev_set_domain (dev); + i915_gem_dev_set_domain(dev); exec_offset = validate_list[args->buffer_count - 1].buffer_offset; #if WATCH_EXEC - i915_gem_dump_object (object_list[args->buffer_count - 1], + i915_gem_dump_object(object_list[args->buffer_count - 1], args->batch_len, - __FUNCTION__, + __func__, ~0); #endif - + /* Exec the batchbuffer */ - ret = i915_dispatch_gem_execbuffer (dev, args, exec_offset); - if (ret) - { - DRM_ERROR ("dispatch failed %d\n", ret); + ret = i915_dispatch_gem_execbuffer(dev, args, exec_offset); + if (ret) { + DRM_ERROR("dispatch failed %d\n", ret); goto err; } - /* Get a cookie representing the execution of the current buffer, which we - * can wait on. We would like to mitigate these interrupts, likely by - * only creating cookies occasionally (so that we have *some* interrupts - * representing completion of buffers that we can wait on when trying - * to clear up gtt space). + /* + * Get a cookie representing the execution of the current buffer, + * which we can wait on. We would like to mitigate these interrupts, + * likely by only creating cookies occasionally (so that we have + * *some* interrupts representing completion of buffers that we can + * wait on when trying to clear up gtt space). */ cookie = i915_emit_irq(dev); for (i = 0; i < args->buffer_count; i++) { @@ -865,17 +889,18 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, * which is freed when the object is waited for */ if (obj_priv->last_rendering_cookie == 0) - drm_gem_object_reference (obj); + drm_gem_object_reference(obj); obj_priv->last_rendering_cookie = cookie; } /* Copy the new buffer offsets back to the user's validate list. */ - ret = copy_to_user((struct drm_i915_relocation_entry __user*)(uintptr_t) - args->buffers_ptr, + ret = copy_to_user((struct drm_i915_relocation_entry __user *) + (uintptr_t) args->buffers_ptr, validate_list, sizeof(*validate_list) * args->buffer_count); if (ret) - DRM_ERROR ("failed to copy %d validate entries back to user (%d)\n", + DRM_ERROR("failed to copy %d validate entries " + "back to user (%d)\n", args->buffer_count, ret); err: if (object_list != NULL) { @@ -883,11 +908,11 @@ err: drm_gem_object_unreference(object_list[i]); } -#if 0 +#if 1 /* XXX kludge for now as we don't clean the exec ring yet */ if (object_list != NULL) { for (i = 0; i < args->buffer_count; i++) - i915_gem_object_wait_rendering (object_list[i]); + i915_gem_object_wait_rendering(object_list[i]); } #endif @@ -916,20 +941,21 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data, } obj_priv = obj->driver_private; - if (obj_priv->gtt_space == NULL) - { - ret = i915_gem_object_bind_to_gtt(obj, (unsigned) args->alignment); + if (obj_priv->gtt_space == NULL) { + ret = i915_gem_object_bind_to_gtt(obj, + (unsigned) args->alignment); if (ret != 0) { - DRM_ERROR("Failure to bind in i915_gem_pin_ioctl(): %d\n", + DRM_ERROR("Failure to bind in " + "i915_gem_pin_ioctl(): %d\n", ret); - drm_gem_object_unreference (obj); + drm_gem_object_unreference(obj); return ret; } } obj_priv->pin_count++; args->offset = obj_priv->gtt_offset; - drm_gem_object_unreference (obj); + drm_gem_object_unreference(obj); return 0; } @@ -951,7 +977,7 @@ i915_gem_unpin_ioctl(struct drm_device *dev, void *data, obj_priv = obj->driver_private; obj_priv->pin_count--; - drm_gem_object_unreference (obj); + drm_gem_object_unreference(obj); return 0; } @@ -977,11 +1003,11 @@ void i915_gem_free_object(struct drm_gem_object *obj) } int -i915_gem_set_domain_ioctl (struct drm_gem_object *obj, +i915_gem_set_domain_ioctl(struct drm_gem_object *obj, uint32_t read_domains, uint32_t write_domain) { - i915_gem_object_set_domain (obj, read_domains, write_domain); - i915_gem_dev_set_domain (obj->dev); + i915_gem_object_set_domain(obj, read_domains, write_domain); + i915_gem_dev_set_domain(obj->dev); return 0; } -- cgit v1.2.3 From f0ae335cd70077043f2f7af39d7edcc529367c61 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 9 May 2008 15:02:50 -0700 Subject: GEM: Avoid leaking refs on target objects on presumed offset success. --- linux-core/i915_gem.c | 4 +++- 1 file changed, 3 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 ff8e2762..be30d6bf 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -640,8 +640,10 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, /* If the relocation already has the right value in it, no * more work needs to be done. */ - if (target_obj_priv->gtt_offset == reloc.presumed_offset) + if (target_obj_priv->gtt_offset == reloc.presumed_offset) { + drm_gem_object_unreference(target_obj); continue; + } /* Now that we're going to actually write some data in, * make sure that any rendering using this buffer's contents -- cgit v1.2.3 From f56f2acb5a3f34ad6916ff315d3d2058bd4b8f9c Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 9 May 2008 15:07:49 -0700 Subject: GEM: Clear obj_priv->agp_mem when we free it. Still managing to get something wrong with this, oopsing down in agp. --- linux-core/i915_gem.c | 1 + 1 file changed, 1 insertion(+) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index be30d6bf..caead91f 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -211,6 +211,7 @@ i915_gem_object_unbind(struct drm_gem_object *obj) if (obj_priv->agp_mem != NULL) { drm_unbind_agp(obj_priv->agp_mem); drm_free_agp(obj_priv->agp_mem, obj->size / PAGE_SIZE); + obj_priv->agp_mem = NULL; } i915_gem_object_free_page_list(obj); -- cgit v1.2.3 From c5c59eab809604e4d0d4d1dc71fc11186d0220f8 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 9 May 2008 14:34:20 -0700 Subject: GEM: Separate the LRU into execution list and LRU list. Now, the LRU list has objects that are completely done rendering and ready to kick out, while the execution list has things with active rendering, which have associated cookies and reference counts on them. --- linux-core/i915_gem.c | 130 +++++++++++++++++++++++++++++++++++++------------- 1 file changed, 97 insertions(+), 33 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index caead91f..ec5a9872 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -150,6 +150,7 @@ static int i915_gem_object_wait_rendering(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; int ret; @@ -163,9 +164,19 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) #endif i915_gem_flush(dev, 0, obj->write_domain); obj->write_domain = 0; + + /* Add a reference since we're gaining a cookie. */ if (obj_priv->last_rendering_cookie == 0) drm_gem_object_reference(obj); + /* Move from whatever list we were on to the tail of execution. + */ + list_move_tail(&obj_priv->gtt_lru_entry, + &dev_priv->mm.execution_list); obj_priv->last_rendering_cookie = i915_emit_irq(dev); + BUG_ON(obj_priv->last_rendering_cookie == 0); +#if WATCH_LRU + DRM_INFO("%s: flush moves to exec list %p\n", __func__, obj); +#endif } /* If there is rendering queued on the buffer being evicted, wait for * it. @@ -178,12 +189,19 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) ret = i915_wait_irq(dev, obj_priv->last_rendering_cookie); if (ret != 0) return ret; + /* Clear it now that we know it's passed. */ obj_priv->last_rendering_cookie = 0; - - /* - * The cookie held a reference to the object, - * release that now + /* We were on the execution list since we had a cookie. + * Move to the tail of the LRU list now since we're done. + */ + list_move_tail(&obj_priv->gtt_lru_entry, + &dev_priv->mm.gtt_lru); +#if WATCH_LRU + DRM_INFO("%s: wait moves to lru list %p\n", __func__, obj); +#endif + /* The cookie held a reference to the object, release that + * now */ drm_gem_object_unreference(obj); } @@ -206,7 +224,11 @@ i915_gem_object_unbind(struct drm_gem_object *obj) if (obj_priv->gtt_space == NULL) return; - i915_gem_object_wait_rendering(obj); + /* Ignore the return value of wait_rendering. If we're here but + * a wait_rendering hasn't completed, we're in the freeing process, + * and we want the buffer to go away even if the command queue is hung. + */ + (void)i915_gem_object_wait_rendering(obj); if (obj_priv->agp_mem != NULL) { drm_unbind_agp(obj_priv->agp_mem); @@ -218,8 +240,17 @@ i915_gem_object_unbind(struct drm_gem_object *obj) drm_memrange_put_block(obj_priv->gtt_space); obj_priv->gtt_space = NULL; - if (!list_empty(&obj_priv->gtt_lru_entry)) + + /* Remove ourselves from the LRU list if present. */ + if (!list_empty(&obj_priv->gtt_lru_entry)) { list_del_init(&obj_priv->gtt_lru_entry); + if (obj_priv->last_rendering_cookie) { + DRM_ERROR("Failed to wait on buffer when unbinding, " + "continued anyway.\n"); + obj_priv->last_rendering_cookie = 0; + drm_gem_object_unreference(obj); + } + } } #if WATCH_BUF | WATCH_EXEC @@ -274,6 +305,13 @@ i915_dump_lru(struct drm_device *dev, const char *where) drm_i915_private_t *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj_priv; + DRM_INFO("GTT execution list %s {\n", where); + list_for_each_entry(obj_priv, &dev_priv->mm.execution_list, + gtt_lru_entry) + { + DRM_INFO(" %p: %08x\n", obj_priv, + obj_priv->last_rendering_cookie); + } DRM_INFO("GTT LRU %s {\n", where); list_for_each_entry(obj_priv, &dev_priv->mm.gtt_lru, gtt_lru_entry) { DRM_INFO(" %p: %08x\n", obj_priv, @@ -292,11 +330,22 @@ i915_gem_evict_something(struct drm_device *dev) int ret; /* Find the LRU buffer. */ - BUG_ON(list_empty(&dev_priv->mm.gtt_lru)); - obj_priv = list_first_entry(&dev_priv->mm.gtt_lru, - struct drm_i915_gem_object, - gtt_lru_entry); + if (!list_empty(&dev_priv->mm.gtt_lru)) { + obj_priv = list_first_entry(&dev_priv->mm.gtt_lru, + struct drm_i915_gem_object, + gtt_lru_entry); + } else if (!list_empty(&dev_priv->mm.execution_list)) { + /* If there's nothing unused and ready, grab the LRU + * from the currently executing list. + */ + obj_priv = list_first_entry(&dev_priv->mm.execution_list, + struct drm_i915_gem_object, + gtt_lru_entry); + } else { + return -ENOMEM; + } obj = obj_priv->obj; + drm_gem_object_reference(obj); #if WATCH_LRU DRM_INFO("%s: evicting %p\n", __func__, obj); #endif @@ -317,6 +366,7 @@ i915_gem_evict_something(struct drm_device *dev) #if WATCH_LRU DRM_INFO("%s: evicted %p\n", __func__, obj); #endif + drm_gem_object_unreference(obj); return 0; } @@ -360,7 +410,8 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) #if WATCH_LRU DRM_INFO("%s: GTT full, evicting something\n", __func__); #endif - if (list_empty(&dev_priv->mm.gtt_lru)) { + if (list_empty(&dev_priv->mm.gtt_lru) && + list_empty(&dev_priv->mm.execution_list)) { DRM_ERROR("GTT full, but LRU list empty\n"); return -ENOMEM; } @@ -529,7 +580,6 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, struct drm_i915_gem_validate_entry *entry) { struct drm_device *dev = obj->dev; - drm_i915_private_t *dev_priv = dev->dev_private; 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; @@ -546,19 +596,6 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, entry->buffer_offset = obj_priv->gtt_offset; - if (obj_priv->pin_count == 0) { - /* Move our buffer to the head of the LRU. */ - if (list_empty(&obj_priv->gtt_lru_entry)) - list_add_tail(&obj_priv->gtt_lru_entry, - &dev_priv->mm.gtt_lru); - else - list_move_tail(&obj_priv->gtt_lru_entry, - &dev_priv->mm.gtt_lru); -#if WATCH_LRU && 0 - i915_dump_lru(dev, __func__); -#endif - } - relocs = (struct drm_i915_gem_relocation_entry __user *) (uintptr_t) entry->relocs_ptr; /* Apply the relocations, using the GTT aperture to avoid cache @@ -770,6 +807,7 @@ int 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_gem_execbuffer *args = data; struct drm_i915_gem_validate_entry *validate_list; struct drm_gem_object **object_list; @@ -894,7 +932,17 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, if (obj_priv->last_rendering_cookie == 0) drm_gem_object_reference(obj); obj_priv->last_rendering_cookie = cookie; + BUG_ON(obj_priv->last_rendering_cookie == 0); + /* Move our buffer to the tail of the execution list. */ + list_move_tail(&obj_priv->gtt_lru_entry, + &dev_priv->mm.execution_list); +#if WATCH_LRU + DRM_INFO("%s: move to exec list %p\n", __func__, obj); +#endif } +#if WATCH_LRU && 0 + i915_dump_lru(dev, __func__); +#endif /* Copy the new buffer offsets back to the user's validate list. */ ret = copy_to_user((struct drm_i915_relocation_entry __user *) @@ -911,14 +959,6 @@ err: drm_gem_object_unreference(object_list[i]); } -#if 1 - /* XXX kludge for now as we don't clean the exec ring yet */ - if (object_list != NULL) { - for (i = 0; i < args->buffer_count; i++) - i915_gem_object_wait_rendering(object_list[i]); - } -#endif - drm_free(object_list, sizeof(*object_list) * args->buffer_count, DRM_MEM_DRIVER); drm_free(validate_list, sizeof(*validate_list) * args->buffer_count, @@ -1014,3 +1054,27 @@ i915_gem_set_domain_ioctl(struct drm_gem_object *obj, i915_gem_dev_set_domain(obj->dev); return 0; } + +void +i915_gem_lastclose(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + + /* Assume that the chip has been idled at this point. Just pull them + * off the execution list and unref them. Since this is the last + * close, this is also the last ref and they'll go away. + */ + + while (!list_empty(&dev_priv->mm.execution_list)) { + struct drm_i915_gem_object *obj_priv; + + obj_priv = list_first_entry(&dev_priv->mm.execution_list, + struct drm_i915_gem_object, + gtt_lru_entry); + + list_del_init(&obj_priv->gtt_lru_entry); + obj_priv->last_rendering_cookie = 0; + obj_priv->obj->write_domain = 0; + drm_gem_object_unreference(obj_priv->obj); + } +} -- cgit v1.2.3 From 48a8531aa403ea250696338aa8717e3e36477370 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 9 May 2008 18:23:51 -0700 Subject: GEM: Fix arguments to drm_memrange_init so we don't exceed our allocation. It takes (offset, size), not (offset, end). --- linux-core/i915_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index ec5a9872..37a4e503 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -48,7 +48,7 @@ i915_gem_init_ioctl(struct drm_device *dev, void *data, return -EINVAL; drm_memrange_init(&dev_priv->mm.gtt_space, args->gtt_start, - args->gtt_end); + args->gtt_end - args->gtt_start); return 0; } -- cgit v1.2.3 From a37ac493da1730436028ecc79a38513380ce15d0 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Sat, 10 May 2008 21:04:18 -0700 Subject: [intel-GEM] Clean up GEM ioctl naming. Rename 'validate_entry' to 'exec_object', then clean up some field names in structures (renaming buffer_offset to just offset, for example). --- linux-core/i915_gem.c | 13 ++++++------- 1 file changed, 6 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 ff8e2762..e91cae30 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -525,7 +525,7 @@ i915_gem_dev_set_domain(struct drm_device *dev) static int i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, struct drm_file *file_priv, - struct drm_i915_gem_validate_entry *entry) + struct drm_i915_gem_exec_object *entry) { struct drm_device *dev = obj->dev; drm_i915_private_t *dev_priv = dev->dev_private; @@ -543,7 +543,7 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, return -ENOMEM; } - entry->buffer_offset = obj_priv->gtt_offset; + entry->offset = obj_priv->gtt_offset; if (obj_priv->pin_count == 0) { /* Move our buffer to the head of the LRU. */ @@ -768,7 +768,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_i915_gem_execbuffer *args = data; - struct drm_i915_gem_validate_entry *validate_list; + struct drm_i915_gem_exec_object *validate_list; struct drm_gem_object **object_list; struct drm_gem_object *batch_obj; int ret, i; @@ -808,11 +808,10 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, /* Look up object handles and perform the relocations */ for (i = 0; i < args->buffer_count; i++) { object_list[i] = drm_gem_object_lookup(dev, file_priv, - validate_list[i]. - buffer_handle); + validate_list[i].handle); if (object_list[i] == NULL) { DRM_ERROR("Invalid object handle %d at index %d\n", - validate_list[i].buffer_handle, i); + validate_list[i].handle, i); ret = -EINVAL; goto err; } @@ -856,7 +855,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, /* Flush/invalidate caches and chipset buffer */ i915_gem_dev_set_domain(dev); - exec_offset = validate_list[args->buffer_count - 1].buffer_offset; + exec_offset = validate_list[args->buffer_count - 1].offset; #if WATCH_EXEC i915_gem_dump_object(object_list[args->buffer_count - 1], -- cgit v1.2.3 From 1b0bf301431e76712de1ee43681bc818383b2e56 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Sat, 10 May 2008 22:04:39 -0700 Subject: [intel-GEM] exec list can contain pinned, lru cannot. The exec list contains all objects, in order of use. The lru list contains only unpinned objects ready to be evicted. This required two changes -- the first was to not migrate pinned objects from exec to lru, the second was to search for the first unpinned object in the exec list when doing eviction. --- linux-core/i915_gem.c | 20 +++++++++++++------- 1 file changed, 13 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 0cacc65b..90d1d52b 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -192,11 +192,14 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) /* Clear it now that we know it's passed. */ obj_priv->last_rendering_cookie = 0; + /* We were on the execution list since we had a cookie. * Move to the tail of the LRU list now since we're done. */ - list_move_tail(&obj_priv->gtt_lru_entry, - &dev_priv->mm.gtt_lru); + if (obj_priv->pin_count == 0) + list_move_tail(&obj_priv->gtt_lru_entry, + &dev_priv->mm.gtt_lru); + #if WATCH_LRU DRM_INFO("%s: wait moves to lru list %p\n", __func__, obj); #endif @@ -335,12 +338,15 @@ i915_gem_evict_something(struct drm_device *dev) struct drm_i915_gem_object, gtt_lru_entry); } else if (!list_empty(&dev_priv->mm.execution_list)) { - /* If there's nothing unused and ready, grab the LRU - * from the currently executing list. + /* If there's nothing unused and ready, grab the first + * unpinned object from the currently executing list. */ - obj_priv = list_first_entry(&dev_priv->mm.execution_list, - struct drm_i915_gem_object, - gtt_lru_entry); + list_for_each_entry(obj_priv, &dev_priv->mm.execution_list, + gtt_lru_entry) + if (obj_priv->pin_count == 0) + break; + if (!obj_priv) + return -ENOMEM; } else { return -ENOMEM; } -- cgit v1.2.3 From ff39db099b9ca6c8feee68101a2269345b7bd798 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Sun, 11 May 2008 00:10:16 -0700 Subject: [GEM] Make pread/pwrite manage memory domains. No luck with movnti though. pread and pwrite must update the memory domains to ensure consistency with the GPU. At some point, it should be possible to avoid clflush through this path, but that isn't working for me. --- linux-core/i915_gem.c | 91 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 64 insertions(+), 27 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 90d1d52b..9d701ba1 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -377,6 +377,37 @@ i915_gem_evict_something(struct drm_device *dev) return 0; } +static int +i915_gem_object_get_page_list(struct drm_gem_object *obj) +{ + struct drm_i915_gem_object *obj_priv = obj->driver_private; + int page_count, i; + if (obj_priv->page_list) + return 0; + + /* Get the list of pages out of our struct file. They'll be pinned + * at this point until we release them. + */ + page_count = obj->size / PAGE_SIZE; + BUG_ON(obj_priv->page_list != NULL); + obj_priv->page_list = drm_calloc(page_count, sizeof(struct page *), + DRM_MEM_DRIVER); + if (obj_priv->page_list == NULL) + return -ENOMEM; + + for (i = 0; i < page_count; i++) { + obj_priv->page_list[i] = + find_or_create_page(obj->filp->f_mapping, i, GFP_HIGHUSER); + + if (obj_priv->page_list[i] == NULL) { + i915_gem_object_free_page_list(obj); + return -ENOMEM; + } + unlock_page(obj_priv->page_list[i]); + } + return 0; +} + /** * Finds free space in the GTT aperture and binds the object there. */ @@ -387,7 +418,7 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) drm_i915_private_t *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj_priv = obj->driver_private; struct drm_memrange_node *free_space; - int page_count, i, ret; + int page_count, ret; if (alignment == 0) alignment = PAGE_SIZE; @@ -432,33 +463,14 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) DRM_INFO("Binding object of size %d at 0x%08x\n", obj->size, obj_priv->gtt_offset); #endif - - /* Get the list of pages out of our struct file. They'll be pinned - * at this point until we release them. - */ - page_count = obj->size / PAGE_SIZE; - BUG_ON(obj_priv->page_list != NULL); - obj_priv->page_list = drm_calloc(page_count, sizeof(struct page *), - DRM_MEM_DRIVER); - if (obj_priv->page_list == NULL) { - drm_memrange_put_block(obj_priv->gtt_space); + ret = i915_gem_object_get_page_list (obj); + if (ret) { + drm_memrange_put_block (obj_priv->gtt_space); obj_priv->gtt_space = NULL; - return -ENOMEM; - } - - for (i = 0; i < page_count; i++) { - obj_priv->page_list[i] = - find_or_create_page(obj->filp->f_mapping, i, GFP_HIGHUSER); - - if (obj_priv->page_list[i] == NULL) { - i915_gem_object_free_page_list(obj); - drm_memrange_put_block(obj_priv->gtt_space); - obj_priv->gtt_space = NULL; - return -ENOMEM; - } - unlock_page(obj_priv->page_list[i]); + return ret; } + page_count = obj->size / PAGE_SIZE; /* Create an AGP memory structure pointing at our pages, and bind it * into the GTT. */ @@ -1052,14 +1064,39 @@ void i915_gem_free_object(struct drm_gem_object *obj) int i915_gem_set_domain_ioctl(struct drm_gem_object *obj, - uint32_t read_domains, - uint32_t write_domain) + uint32_t read_domains, + uint32_t write_domain) { i915_gem_object_set_domain(obj, read_domains, write_domain); i915_gem_dev_set_domain(obj->dev); return 0; } +int +i915_gem_flush_pwrite(struct drm_gem_object *obj, + uint64_t offset, uint64_t size) +{ + struct drm_device *dev = obj->dev; + struct drm_i915_gem_object *obj_priv = obj->driver_private; + /* + * As far as I can tell, writes of > 64 bytes will use non-temporal + * stores which should obviate the need for this clflush. + * It doesn't work for me though... + */ +/* if (size <= 64) */{ + if (obj_priv->gtt_space == NULL) { + int ret = i915_gem_object_get_page_list (obj); + if (ret) + return ret; + } + i915_gem_clflush_object(obj); + if (obj_priv->gtt_space == NULL) + i915_gem_object_free_page_list (obj); + } + drm_agp_chipset_flush(dev); + return 0; +} + void i915_gem_lastclose(struct drm_device *dev) { -- cgit v1.2.3 From 17e8000ac046e912bf02649e67165cafed270e2e Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Mon, 12 May 2008 13:04:18 -0700 Subject: [intel] Minor kludge -- wait for the ring to be nearly empty before queuing No need to fill the ring that much; wait for it to become nearly empty before adding the execbuffer request. A better fix will involve scheduling ring insertion in the irq handler. --- linux-core/i915_gem.c | 67 +++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 49 insertions(+), 18 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 9d701ba1..90332bf0 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -463,9 +463,9 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) DRM_INFO("Binding object of size %d at 0x%08x\n", obj->size, obj_priv->gtt_offset); #endif - ret = i915_gem_object_get_page_list (obj); + ret = i915_gem_object_get_page_list(obj); if (ret) { - drm_memrange_put_block (obj_priv->gtt_space); + drm_memrange_put_block(obj_priv->gtt_space); obj_priv->gtt_space = NULL; return ret; } @@ -821,14 +821,43 @@ i915_dispatch_gem_execbuffer(struct drm_device *dev, return 0; } +/* + * Kludge -- wait for almost all rendering to complete + * before queuing more. This uses interrupts, so the wakeup + * occurs without any delay. + */ +static int +i915_gem_wait_space(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + drm_i915_ring_buffer_t *ring = &(dev_priv->ring); + struct drm_i915_gem_object *obj_priv, *last_priv = NULL; + int ret = 0; + + while (ring->space + 1024 < dev_priv->ring.Size && + !list_empty(&dev_priv->mm.execution_list)) { + obj_priv = list_first_entry(&dev_priv->mm.execution_list, + struct drm_i915_gem_object, + gtt_lru_entry); + if (obj_priv == last_priv) + break; + ret = i915_gem_object_wait_rendering(obj_priv->obj); + if (ret) + break; + last_priv = obj_priv; + i915_kernel_lost_context(dev); + } + return ret; +} + int 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_gem_execbuffer *args = data; - struct drm_i915_gem_exec_object *validate_list; - struct drm_gem_object **object_list; + struct drm_i915_gem_exec_object *validate_list = NULL; + struct drm_gem_object **object_list = NULL; struct drm_gem_object *batch_obj; int ret, i; uint64_t exec_offset; @@ -842,6 +871,10 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, #endif i915_kernel_lost_context(dev); + ret = i915_gem_wait_space(dev); + if (ret) + return ret; + /* Copy in the validate list from userland */ validate_list = drm_calloc(sizeof(*validate_list), args->buffer_count, DRM_MEM_DRIVER); @@ -975,7 +1008,6 @@ err: for (i = 0; i < args->buffer_count; i++) drm_gem_object_unreference(object_list[i]); } - drm_free(object_list, sizeof(*object_list) * args->buffer_count, DRM_MEM_DRIVER); drm_free(validate_list, sizeof(*validate_list) * args->buffer_count, @@ -1078,22 +1110,21 @@ i915_gem_flush_pwrite(struct drm_gem_object *obj, { struct drm_device *dev = obj->dev; struct drm_i915_gem_object *obj_priv = obj->driver_private; + /* - * As far as I can tell, writes of > 64 bytes will use non-temporal - * stores which should obviate the need for this clflush. - * It doesn't work for me though... + * For writes much less than the size of the object and + * which are already pinned in memory, do the flush right now */ -/* if (size <= 64) */{ - if (obj_priv->gtt_space == NULL) { - int ret = i915_gem_object_get_page_list (obj); - if (ret) - return ret; - } - i915_gem_clflush_object(obj); - if (obj_priv->gtt_space == NULL) - i915_gem_object_free_page_list (obj); + + 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); + + drm_ttm_cache_flush(obj_priv->page_list + first_page, + beyond_page - first_page); + drm_agp_chipset_flush(dev); + obj->write_domain = 0; } - drm_agp_chipset_flush(dev); return 0; } -- cgit v1.2.3 From 3ab152da66f6c7bcc68a13efcf4a62800354f13b Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 15 May 2008 09:37:49 -0700 Subject: [gem] Rename the GTT LRU lists to active (executing) and inactive (idle). --- linux-core/i915_gem.c | 54 +++++++++++++++++++++++++-------------------------- 1 file changed, 27 insertions(+), 27 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 90332bf0..00f56013 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -170,8 +170,8 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) drm_gem_object_reference(obj); /* Move from whatever list we were on to the tail of execution. */ - list_move_tail(&obj_priv->gtt_lru_entry, - &dev_priv->mm.execution_list); + list_move_tail(&obj_priv->list, + &dev_priv->mm.active_list); obj_priv->last_rendering_cookie = i915_emit_irq(dev); BUG_ON(obj_priv->last_rendering_cookie == 0); #if WATCH_LRU @@ -197,8 +197,8 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) * Move to the tail of the LRU list now since we're done. */ if (obj_priv->pin_count == 0) - list_move_tail(&obj_priv->gtt_lru_entry, - &dev_priv->mm.gtt_lru); + list_move_tail(&obj_priv->list, + &dev_priv->mm.inactive_list); #if WATCH_LRU DRM_INFO("%s: wait moves to lru list %p\n", __func__, obj); @@ -245,8 +245,8 @@ 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->gtt_lru_entry)) { - list_del_init(&obj_priv->gtt_lru_entry); + if (!list_empty(&obj_priv->list)) { + list_del_init(&obj_priv->list); if (obj_priv->last_rendering_cookie) { DRM_ERROR("Failed to wait on buffer when unbinding, " "continued anyway.\n"); @@ -309,14 +309,14 @@ i915_dump_lru(struct drm_device *dev, const char *where) struct drm_i915_gem_object *obj_priv; DRM_INFO("GTT execution list %s {\n", where); - list_for_each_entry(obj_priv, &dev_priv->mm.execution_list, - gtt_lru_entry) + list_for_each_entry(obj_priv, &dev_priv->mm.active_list, + list) { DRM_INFO(" %p: %08x\n", obj_priv, obj_priv->last_rendering_cookie); } DRM_INFO("GTT LRU %s {\n", where); - list_for_each_entry(obj_priv, &dev_priv->mm.gtt_lru, gtt_lru_entry) { + list_for_each_entry(obj_priv, &dev_priv->mm.inactive_list, list) { DRM_INFO(" %p: %08x\n", obj_priv, obj_priv->last_rendering_cookie); } @@ -333,16 +333,16 @@ i915_gem_evict_something(struct drm_device *dev) int ret; /* Find the LRU buffer. */ - if (!list_empty(&dev_priv->mm.gtt_lru)) { - obj_priv = list_first_entry(&dev_priv->mm.gtt_lru, + if (!list_empty(&dev_priv->mm.inactive_list)) { + obj_priv = list_first_entry(&dev_priv->mm.inactive_list, struct drm_i915_gem_object, - gtt_lru_entry); - } else if (!list_empty(&dev_priv->mm.execution_list)) { + list); + } else if (!list_empty(&dev_priv->mm.active_list)) { /* If there's nothing unused and ready, grab the first * unpinned object from the currently executing list. */ - list_for_each_entry(obj_priv, &dev_priv->mm.execution_list, - gtt_lru_entry) + list_for_each_entry(obj_priv, &dev_priv->mm.active_list, + list) if (obj_priv->pin_count == 0) break; if (!obj_priv) @@ -447,8 +447,8 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) #if WATCH_LRU DRM_INFO("%s: GTT full, evicting something\n", __func__); #endif - if (list_empty(&dev_priv->mm.gtt_lru) && - list_empty(&dev_priv->mm.execution_list)) { + if (list_empty(&dev_priv->mm.inactive_list) && + list_empty(&dev_priv->mm.active_list)) { DRM_ERROR("GTT full, but LRU list empty\n"); return -ENOMEM; } @@ -835,10 +835,10 @@ i915_gem_wait_space(struct drm_device *dev) int ret = 0; while (ring->space + 1024 < dev_priv->ring.Size && - !list_empty(&dev_priv->mm.execution_list)) { - obj_priv = list_first_entry(&dev_priv->mm.execution_list, + !list_empty(&dev_priv->mm.active_list)) { + obj_priv = list_first_entry(&dev_priv->mm.active_list, struct drm_i915_gem_object, - gtt_lru_entry); + list); if (obj_priv == last_priv) break; ret = i915_gem_object_wait_rendering(obj_priv->obj); @@ -984,8 +984,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, obj_priv->last_rendering_cookie = cookie; BUG_ON(obj_priv->last_rendering_cookie == 0); /* Move our buffer to the tail of the execution list. */ - list_move_tail(&obj_priv->gtt_lru_entry, - &dev_priv->mm.execution_list); + list_move_tail(&obj_priv->list, + &dev_priv->mm.active_list); #if WATCH_LRU DRM_INFO("%s: move to exec list %p\n", __func__, obj); #endif @@ -1083,7 +1083,7 @@ int i915_gem_init_object(struct drm_gem_object *obj) obj->driver_private = obj_priv; obj_priv->obj = obj; - INIT_LIST_HEAD(&obj_priv->gtt_lru_entry); + INIT_LIST_HEAD(&obj_priv->list); return 0; } @@ -1138,14 +1138,14 @@ i915_gem_lastclose(struct drm_device *dev) * close, this is also the last ref and they'll go away. */ - while (!list_empty(&dev_priv->mm.execution_list)) { + while (!list_empty(&dev_priv->mm.active_list)) { struct drm_i915_gem_object *obj_priv; - obj_priv = list_first_entry(&dev_priv->mm.execution_list, + obj_priv = list_first_entry(&dev_priv->mm.active_list, struct drm_i915_gem_object, - gtt_lru_entry); + list); - list_del_init(&obj_priv->gtt_lru_entry); + list_del_init(&obj_priv->list); obj_priv->last_rendering_cookie = 0; obj_priv->obj->write_domain = 0; drm_gem_object_unreference(obj_priv->obj); -- cgit v1.2.3 From 7dced2f33a952ad12aafb7a3e34747156020a3ae Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 15 May 2008 11:21:11 -0700 Subject: [gem] Hold dev->struct_mutex to protect structure data. --- linux-core/i915_gem.c | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 00f56013..144667a3 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -42,14 +42,20 @@ i915_gem_init_ioctl(struct drm_device *dev, void *data, drm_i915_private_t *dev_priv = dev->dev_private; struct drm_i915_gem_init *args = data; + mutex_lock(&dev->struct_mutex); + if (args->gtt_start >= args->gtt_end || (args->gtt_start & (PAGE_SIZE - 1)) != 0 || - (args->gtt_end & (PAGE_SIZE - 1)) != 0) + (args->gtt_end & (PAGE_SIZE - 1)) != 0) { + mutex_unlock(&dev->struct_mutex); return -EINVAL; + } drm_memrange_init(&dev_priv->mm.gtt_space, args->gtt_start, args->gtt_end - args->gtt_start); + mutex_unlock(&dev->struct_mutex); + return 0; } @@ -834,6 +840,7 @@ i915_gem_wait_space(struct drm_device *dev) struct drm_i915_gem_object *obj_priv, *last_priv = NULL; int ret = 0; + mutex_lock(&dev->struct_mutex); while (ring->space + 1024 < dev_priv->ring.Size && !list_empty(&dev_priv->mm.active_list)) { obj_priv = list_first_entry(&dev_priv->mm.active_list, @@ -847,6 +854,7 @@ i915_gem_wait_space(struct drm_device *dev) last_priv = obj_priv; i915_kernel_lost_context(dev); } + mutex_unlock(&dev->struct_mutex); return ret; } @@ -897,6 +905,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, goto err; } + mutex_lock(&dev->struct_mutex); /* Look up object handles and perform the relocations */ for (i = 0; i < args->buffer_count; i++) { object_list[i] = drm_gem_object_lookup(dev, file_priv, @@ -1008,6 +1017,8 @@ err: for (i = 0; i < args->buffer_count; i++) drm_gem_object_unreference(object_list[i]); } + mutex_unlock(&dev->struct_mutex); + drm_free(object_list, sizeof(*object_list) * args->buffer_count, DRM_MEM_DRIVER); drm_free(validate_list, sizeof(*validate_list) * args->buffer_count, @@ -1025,10 +1036,13 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data, struct drm_i915_gem_object *obj_priv; int ret; + mutex_lock(&dev->struct_mutex); + obj = drm_gem_object_lookup(dev, file_priv, args->handle); if (obj == NULL) { DRM_ERROR("Bad handle in i915_gem_pin_ioctl(): %d\n", args->handle); + mutex_unlock(&dev->struct_mutex); return -EINVAL; } @@ -1041,6 +1055,7 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data, "i915_gem_pin_ioctl(): %d\n", ret); drm_gem_object_unreference(obj); + mutex_unlock(&dev->struct_mutex); return ret; } } @@ -1048,6 +1063,7 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data, obj_priv->pin_count++; args->offset = obj_priv->gtt_offset; drm_gem_object_unreference(obj); + mutex_unlock(&dev->struct_mutex); return 0; } @@ -1060,16 +1076,19 @@ i915_gem_unpin_ioctl(struct drm_device *dev, void *data, struct drm_gem_object *obj; struct drm_i915_gem_object *obj_priv; + mutex_lock(&dev->struct_mutex); obj = drm_gem_object_lookup(dev, file_priv, args->handle); if (obj == NULL) { DRM_ERROR("Bad handle in i915_gem_unpin_ioctl(): %d\n", args->handle); + mutex_unlock(&dev->struct_mutex); return -EINVAL; } obj_priv = obj->driver_private; obj_priv->pin_count--; drm_gem_object_unreference(obj); + mutex_unlock(&dev->struct_mutex); return 0; } @@ -1095,12 +1114,17 @@ void i915_gem_free_object(struct drm_gem_object *obj) } int -i915_gem_set_domain_ioctl(struct drm_gem_object *obj, - uint32_t read_domains, - uint32_t write_domain) +i915_gem_set_domain(struct drm_gem_object *obj, + uint32_t read_domains, + uint32_t write_domain) { + struct drm_device *dev = obj->dev; + + BUG_ON(!mutex_is_locked(&dev->struct_mutex)); + i915_gem_object_set_domain(obj, read_domains, write_domain); i915_gem_dev_set_domain(obj->dev); + return 0; } @@ -1133,6 +1157,8 @@ i915_gem_lastclose(struct drm_device *dev) { drm_i915_private_t *dev_priv = dev->dev_private; + mutex_lock(&dev->struct_mutex); + /* Assume that the chip has been idled at this point. Just pull them * off the execution list and unref them. Since this is the last * close, this is also the last ref and they'll go away. @@ -1150,4 +1176,6 @@ i915_gem_lastclose(struct drm_device *dev) obj_priv->obj->write_domain = 0; drm_gem_object_unreference(obj_priv->obj); } + + mutex_unlock(&dev->struct_mutex); } -- cgit v1.2.3 From 6c3ac484b049681f9f3e692f9a6238ed122a8191 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Tue, 20 May 2008 10:48:36 -0700 Subject: [gem] Clean up active/inactive list handling using helper functions. Additionally, a boolean active field is added to indicate which list an object is on, rather than smashing last_rendering_cookie to 0 to show inactive. This will help with flush-reduction later on, and makes the code clearer. --- linux-core/i915_gem.c | 80 ++++++++++++++++++++++++++++----------------------- 1 file changed, 44 insertions(+), 36 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 144667a3..1a97fdef 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -80,6 +80,42 @@ i915_gem_object_free_page_list(struct drm_gem_object *obj) obj_priv->page_list = NULL; } +static void +i915_gem_object_move_to_active(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; + + /* Add a reference if we're newly entering the active list. */ + if (!obj_priv->active) { + drm_gem_object_reference(obj); + obj_priv->active = 1; + } + /* Move from whatever list we were on to the tail of execution. */ + list_move_tail(&obj_priv->list, + &dev_priv->mm.active_list); +} + +static void +i915_gem_object_move_to_inactive(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; + + if (obj_priv->pin_count != 0) + list_del_init(&obj_priv->list); + else + list_move_tail(&obj_priv->list, &dev_priv->mm.inactive_list); + + if (obj_priv->active) { + obj_priv->active = 0; + drm_gem_object_unreference(obj); + } +} + + static void i915_gem_flush(struct drm_device *dev, uint32_t invalidate_domains, @@ -156,7 +192,6 @@ static int i915_gem_object_wait_rendering(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; int ret; @@ -171,13 +206,7 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) i915_gem_flush(dev, 0, obj->write_domain); obj->write_domain = 0; - /* Add a reference since we're gaining a cookie. */ - if (obj_priv->last_rendering_cookie == 0) - drm_gem_object_reference(obj); - /* Move from whatever list we were on to the tail of execution. - */ - list_move_tail(&obj_priv->list, - &dev_priv->mm.active_list); + i915_gem_object_move_to_active(obj); obj_priv->last_rendering_cookie = i915_emit_irq(dev); BUG_ON(obj_priv->last_rendering_cookie == 0); #if WATCH_LRU @@ -187,7 +216,7 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) /* If there is rendering queued on the buffer being evicted, wait for * it. */ - if (obj_priv->last_rendering_cookie != 0) { + if (obj_priv->active) { #if WATCH_BUF DRM_INFO("%s: object %p wait for cookie %08x\n", __func__, obj, obj_priv->last_rendering_cookie); @@ -196,23 +225,11 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) if (ret != 0) return ret; - /* Clear it now that we know it's passed. */ - obj_priv->last_rendering_cookie = 0; - - /* We were on the execution list since we had a cookie. - * Move to the tail of the LRU list now since we're done. - */ - if (obj_priv->pin_count == 0) - list_move_tail(&obj_priv->list, - &dev_priv->mm.inactive_list); + i915_gem_object_move_to_inactive(obj); #if WATCH_LRU DRM_INFO("%s: wait moves to lru list %p\n", __func__, obj); #endif - /* The cookie held a reference to the object, release that - * now - */ - drm_gem_object_unreference(obj); } return 0; @@ -253,10 +270,10 @@ i915_gem_object_unbind(struct drm_gem_object *obj) /* Remove ourselves from the LRU list if present. */ if (!list_empty(&obj_priv->list)) { list_del_init(&obj_priv->list); - if (obj_priv->last_rendering_cookie) { + if (obj_priv->active) { DRM_ERROR("Failed to wait on buffer when unbinding, " "continued anyway.\n"); - obj_priv->last_rendering_cookie = 0; + obj_priv->active = 0; drm_gem_object_unreference(obj); } } @@ -862,7 +879,6 @@ int 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_gem_execbuffer *args = data; struct drm_i915_gem_exec_object *validate_list = NULL; struct drm_gem_object **object_list = NULL; @@ -980,21 +996,13 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, * wait on when trying to clear up gtt space). */ cookie = i915_emit_irq(dev); + BUG_ON(cookie == 0); 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; - /* - * Have the cookie hold a reference to this object - * which is freed when the object is waited for - */ - if (obj_priv->last_rendering_cookie == 0) - drm_gem_object_reference(obj); + i915_gem_object_move_to_active(obj); obj_priv->last_rendering_cookie = cookie; - BUG_ON(obj_priv->last_rendering_cookie == 0); - /* Move our buffer to the tail of the execution list. */ - list_move_tail(&obj_priv->list, - &dev_priv->mm.active_list); #if WATCH_LRU DRM_INFO("%s: move to exec list %p\n", __func__, obj); #endif @@ -1172,7 +1180,7 @@ i915_gem_lastclose(struct drm_device *dev) list); list_del_init(&obj_priv->list); - obj_priv->last_rendering_cookie = 0; + obj_priv->active = 0; obj_priv->obj->write_domain = 0; drm_gem_object_unreference(obj_priv->obj); } -- cgit v1.2.3 From ab36a6f983107971890e81473452b3f0313fb692 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Tue, 20 May 2008 10:52:24 -0700 Subject: [gem] Rename sequence numbers from "cookie" to "seqno" --- linux-core/i915_gem.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 1a97fdef..ee8f1023 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -196,7 +196,7 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) int ret; /* If there are writes queued to the buffer, flush and - * create a new cookie to wait for. + * create a new seqno to wait for. */ if (obj->write_domain & ~(DRM_GEM_DOMAIN_CPU)) { #if WATCH_BUF @@ -207,8 +207,8 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) obj->write_domain = 0; i915_gem_object_move_to_active(obj); - obj_priv->last_rendering_cookie = i915_emit_irq(dev); - BUG_ON(obj_priv->last_rendering_cookie == 0); + obj_priv->last_rendering_seqno = i915_emit_irq(dev); + BUG_ON(obj_priv->last_rendering_seqno == 0); #if WATCH_LRU DRM_INFO("%s: flush moves to exec list %p\n", __func__, obj); #endif @@ -218,10 +218,10 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) */ if (obj_priv->active) { #if WATCH_BUF - DRM_INFO("%s: object %p wait for cookie %08x\n", - __func__, obj, obj_priv->last_rendering_cookie); + DRM_INFO("%s: object %p wait for seqno %08x\n", + __func__, obj, obj_priv->last_rendering_seqno); #endif - ret = i915_wait_irq(dev, obj_priv->last_rendering_cookie); + ret = i915_wait_irq(dev, obj_priv->last_rendering_seqno); if (ret != 0) return ret; @@ -336,12 +336,12 @@ i915_dump_lru(struct drm_device *dev, const char *where) list) { DRM_INFO(" %p: %08x\n", obj_priv, - obj_priv->last_rendering_cookie); + obj_priv->last_rendering_seqno); } DRM_INFO("GTT LRU %s {\n", where); list_for_each_entry(obj_priv, &dev_priv->mm.inactive_list, list) { DRM_INFO(" %p: %08x\n", obj_priv, - obj_priv->last_rendering_cookie); + obj_priv->last_rendering_seqno); } DRM_INFO("}\n"); } @@ -885,7 +885,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, struct drm_gem_object *batch_obj; int ret, i; uint64_t exec_offset; - uint32_t cookie; + uint32_t seqno; LOCK_TEST_WITH_RETURN(dev, file_priv); @@ -989,20 +989,20 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, } /* - * Get a cookie representing the execution of the current buffer, + * Get a seqno representing the execution of the current buffer, * which we can wait on. We would like to mitigate these interrupts, - * likely by only creating cookies occasionally (so that we have + * likely by only creating seqnos occasionally (so that we have * *some* interrupts representing completion of buffers that we can * wait on when trying to clear up gtt space). */ - cookie = i915_emit_irq(dev); - BUG_ON(cookie == 0); + seqno = i915_emit_irq(dev); + BUG_ON(seqno == 0); 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; i915_gem_object_move_to_active(obj); - obj_priv->last_rendering_cookie = cookie; + obj_priv->last_rendering_seqno = seqno; #if WATCH_LRU DRM_INFO("%s: move to exec list %p\n", __func__, obj); #endif -- cgit v1.2.3 From af8e087157ef5034fa12d93202037f87da61355d Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Tue, 20 May 2008 14:03:27 -0700 Subject: [gem] Use a separate sequence number field from classic/ttm This lets us get some qualities we desire, such as using the full 32-bit range (except zero), avoiding DRM_WAIT_ON, and a 1:1 mapping of active sequence numbers to request structs, which will be used soon for throttling and interrupt-driven list cleanup. --- linux-core/i915_gem.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 120 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 ee8f1023..3535fae1 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -115,6 +115,123 @@ i915_gem_object_move_to_inactive(struct drm_gem_object *obj) } } +/** + * Creates a new sequence number, emitting a write of it to the status page + * plus an interrupt, which will trigger i915_user_interrupt_handler. + * + * Must be called with struct_lock held. + * + * Returned sequence numbers are nonzero on success. + */ +static uint32_t +i915_add_request(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_gem_request *request; + uint32_t seqno; + RING_LOCALS; + + request = drm_calloc(1, sizeof(*request), DRM_MEM_DRIVER); + if (request == NULL) + return 0; + + /* Grab the seqno we're going to make this request be, and bump the + * next (skipping 0 so it can be the reserved no-seqno value). + */ + seqno = dev_priv->mm.next_gem_seqno; + dev_priv->mm.next_gem_seqno++; + if (dev_priv->mm.next_gem_seqno == 0) + dev_priv->mm.next_gem_seqno++; + + BEGIN_LP_RING(4); + OUT_RING(CMD_STORE_DWORD_IDX); + OUT_RING(I915_GEM_HWS_INDEX << STORE_DWORD_INDEX_SHIFT); + OUT_RING(seqno); + + OUT_RING(GFX_OP_USER_INTERRUPT); + ADVANCE_LP_RING(); + + DRM_DEBUG("%d\n", seqno); + + request->seqno = seqno; + list_add_tail(&request->list, &dev_priv->mm.request_list); + + return seqno; +} + +/** + * Returns true if seq1 is later than seq2. + */ +static int +i915_seqno_passed(uint32_t seq1, uint32_t seq2) +{ + return (int32_t)(seq1 - seq2) >= 0; +} + +static uint32_t +i915_get_gem_seqno(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + + return READ_HWSP(dev_priv, I915_GEM_HWS_INDEX); +} + +/** + * This function clears the request list as sequence numbers are passed. + */ +void +i915_gem_retire_requests(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + uint32_t seqno; + + seqno = i915_get_gem_seqno(dev); + + while (!list_empty(&dev_priv->mm.request_list)) { + struct drm_i915_gem_request *request; + uint32_t retiring_seqno; + + request = list_first_entry(&dev_priv->mm.request_list, + struct drm_i915_gem_request, + list); + retiring_seqno = request->seqno; + + if (i915_seqno_passed(seqno, retiring_seqno)) { + list_del(&request->list); + drm_free(request, sizeof(*request), DRM_MEM_DRIVER); + } else + break; + } +} + +/** + * Waits for a sequence number to be signaled, and cleans up the + * request and object lists appropriately for that event. + */ +int +i915_wait_request(struct drm_device *dev, uint32_t seqno) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + int ret = 0; + + BUG_ON(seqno == 0); + + i915_user_irq_on(dev_priv); + ret = wait_event_interruptible(dev_priv->irq_queue, + i915_seqno_passed(i915_get_gem_seqno(dev), + seqno)); + i915_user_irq_off(dev_priv); + + /* Directly dispatch request retiring. While we have the work queue + * to handle this, the waiter on a request often wants an associated + * buffer to have made it to the inactive list, and we would need + * a separate wait queue to handle that. + */ + if (ret == 0) + i915_gem_retire_requests(dev); + + return ret; +} static void i915_gem_flush(struct drm_device *dev, @@ -207,7 +324,7 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) obj->write_domain = 0; i915_gem_object_move_to_active(obj); - obj_priv->last_rendering_seqno = i915_emit_irq(dev); + obj_priv->last_rendering_seqno = i915_add_request(dev); BUG_ON(obj_priv->last_rendering_seqno == 0); #if WATCH_LRU DRM_INFO("%s: flush moves to exec list %p\n", __func__, obj); @@ -221,7 +338,7 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) DRM_INFO("%s: object %p wait for seqno %08x\n", __func__, obj, obj_priv->last_rendering_seqno); #endif - ret = i915_wait_irq(dev, obj_priv->last_rendering_seqno); + ret = i915_wait_request(dev, obj_priv->last_rendering_seqno); if (ret != 0) return ret; @@ -995,7 +1112,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, * *some* interrupts representing completion of buffers that we can * wait on when trying to clear up gtt space). */ - seqno = i915_emit_irq(dev); + seqno = i915_add_request(dev); BUG_ON(seqno == 0); for (i = 0; i < args->buffer_count; i++) { struct drm_gem_object *obj = object_list[i]; -- cgit v1.2.3 From f8e38e49dd70bf4a5ef97c29f0c405a1fc5023f5 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Wed, 21 May 2008 00:32:02 -0700 Subject: [intel-gem] invalidate ring locals for pin/unpin/set_domain/free functions Ring locals must be reloaded from hardware in case the X server ran. --- linux-core/i915_gem.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 3535fae1..8e5539f5 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1163,6 +1163,7 @@ 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", @@ -1202,6 +1203,8 @@ i915_gem_unpin_ioctl(struct drm_device *dev, void *data, struct drm_i915_gem_object *obj_priv; 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", @@ -1233,6 +1236,7 @@ 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); @@ -1247,6 +1251,7 @@ i915_gem_set_domain(struct drm_gem_object *obj, BUG_ON(!mutex_is_locked(&dev->struct_mutex)); + i915_kernel_lost_context(dev); i915_gem_object_set_domain(obj, read_domains, write_domain); i915_gem_dev_set_domain(obj->dev); -- cgit v1.2.3 From 7078978db0e014a2621984f6c67ca65fa4f23f3a Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Wed, 21 May 2008 15:03:47 -0700 Subject: [gem] Hold a reference on the object in i915_gem_wait_space. Otherwise, in the middle of the function called using it the last ref might disappear. --- 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 8e5539f5..e01823fb 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -982,7 +982,9 @@ i915_gem_wait_space(struct drm_device *dev) list); if (obj_priv == last_priv) break; + drm_gem_object_reference(obj_priv->obj); ret = i915_gem_object_wait_rendering(obj_priv->obj); + drm_gem_object_unreference(obj_priv->obj); if (ret) break; last_priv = obj_priv; -- cgit v1.2.3 From 54fa32cdfe1529023324a0a261ee5d4e033f46ea Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Wed, 21 May 2008 15:15:58 -0700 Subject: [gem] Fix bad test for list_for_each completion. Since it's a circular list, the entry won't be NULL at termination. --- linux-core/i915_gem.c | 11 ++++++++--- 1 file changed, 8 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 e01823fb..94e2e477 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -478,14 +478,19 @@ i915_gem_evict_something(struct drm_device *dev) struct drm_i915_gem_object, list); } else if (!list_empty(&dev_priv->mm.active_list)) { + int found = 0; + /* If there's nothing unused and ready, grab the first * unpinned object from the currently executing list. */ list_for_each_entry(obj_priv, &dev_priv->mm.active_list, - list) - if (obj_priv->pin_count == 0) + list) { + if (obj_priv->pin_count == 0) { + found = 1; break; - if (!obj_priv) + } + } + if (!found) return -ENOMEM; } else { return -ENOMEM; -- cgit v1.2.3 From d6f796857780fc54641047e2aa4e7091376928eb Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Tue, 20 May 2008 16:27:05 -0700 Subject: [gem] Replace ring throttling hack with actual time measurement. --- linux-core/i915_gem.c | 49 +++++++++++++++++++++++++++---------------------- 1 file changed, 27 insertions(+), 22 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 94e2e477..641ca8a3 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -154,6 +154,7 @@ i915_add_request(struct drm_device *dev) DRM_DEBUG("%d\n", seqno); request->seqno = seqno; + request->emitted_jiffies = jiffies; list_add_tail(&request->list, &dev_priv->mm.request_list); return seqno; @@ -966,34 +967,38 @@ i915_dispatch_gem_execbuffer(struct drm_device *dev, return 0; } -/* - * Kludge -- wait for almost all rendering to complete - * before queuing more. This uses interrupts, so the wakeup - * occurs without any delay. +/* Throttle our rendering by waiting until the ring has completed our requests + * emitted over 20 msec ago. + * + * This should get us reasonable parallelism between CPU and GPU but also + * relatively low latency when blocking on a particular request to finish. */ static int -i915_gem_wait_space(struct drm_device *dev) +i915_gem_ring_throttle(struct drm_device *dev) { drm_i915_private_t *dev_priv = dev->dev_private; - drm_i915_ring_buffer_t *ring = &(dev_priv->ring); - struct drm_i915_gem_object *obj_priv, *last_priv = NULL; int ret = 0; mutex_lock(&dev->struct_mutex); - while (ring->space + 1024 < dev_priv->ring.Size && - !list_empty(&dev_priv->mm.active_list)) { - obj_priv = list_first_entry(&dev_priv->mm.active_list, - struct drm_i915_gem_object, - list); - if (obj_priv == last_priv) - break; - drm_gem_object_reference(obj_priv->obj); - ret = i915_gem_object_wait_rendering(obj_priv->obj); - drm_gem_object_unreference(obj_priv->obj); - if (ret) - break; - last_priv = obj_priv; - i915_kernel_lost_context(dev); + 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 (jiffies_to_msecs(jiffies - request->emitted_jiffies) < 20) { + 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; + } } mutex_unlock(&dev->struct_mutex); return ret; @@ -1019,7 +1024,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, #endif i915_kernel_lost_context(dev); - ret = i915_gem_wait_space(dev); + ret = i915_gem_ring_throttle(dev); if (ret) return ret; -- cgit v1.2.3 From 5e662f90d1143de53db866e2b8a94f1bfbe5fc51 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Wed, 14 May 2008 15:13:14 -0700 Subject: [gem] Release GEM buffers from work task scheduled from IRQ. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit There are now 3 lists. Active is buffers currently in the ringbuffer. Flushing is not in the ringbuffer, but needs a flush before unbinding. Inactive is as before. This prevents object_free → unbind → wait_rendering → object_reference and a kernel oops about weird refcounting. This also avoids an synchronous extra flush and wait when freeing a buffer which had a write_domain set (such as a temporary rendered to and then from using the 2d engine). It will sit around on the flushing list until the appropriate flush gets emitted, or we need the GTT space for another operation. --- linux-core/i915_gem.c | 186 ++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 136 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 641ca8a3..c67ce309 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -124,7 +124,7 @@ i915_gem_object_move_to_inactive(struct drm_gem_object *obj) * Returned sequence numbers are nonzero on success. */ static uint32_t -i915_add_request(struct drm_device *dev) +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; @@ -155,11 +155,73 @@ i915_add_request(struct drm_device *dev) request->seqno = seqno; request->emitted_jiffies = jiffies; + request->flush_domains = flush_domains; list_add_tail(&request->list, &dev_priv->mm.request_list); return seqno; } +/** + * Moves buffers associated only with the given active seqno from the active + * to inactive list, potentially freeing them. + */ +static void +i915_gem_retire_request(struct drm_device *dev, + struct drm_i915_gem_request *request) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + + if (request->flush_domains != 0) { + struct drm_i915_gem_object *obj_priv, *next; + + /* First clear any buffers that were only waiting for a flush + * matching the one just retired. + */ + + list_for_each_entry_safe(obj_priv, next, + &dev_priv->mm.flushing_list, list) { + struct drm_gem_object *obj = obj_priv->obj; + + if (obj->write_domain & request->flush_domains) { + obj->write_domain = 0; + i915_gem_object_move_to_inactive(obj); + } + } + + } + + /* Move any buffers on the active list that are no longer referenced + * by the ringbuffer to the flushing/inactive lists as appropriate. + */ + while (!list_empty(&dev_priv->mm.active_list)) { + struct drm_gem_object *obj; + struct drm_i915_gem_object *obj_priv; + + obj_priv = list_first_entry(&dev_priv->mm.active_list, + struct drm_i915_gem_object, + list); + obj = obj_priv->obj; + + /* If the seqno being retired doesn't match the oldest in the + * list, then the oldest in the list must still be newer than + * this seqno. + */ + if (obj_priv->last_rendering_seqno != request->seqno) + return; +#if WATCH_LRU + DRM_INFO("%s: retire %d moves to inactive list %p\n", + __func__, seqno, obj); +#endif + + if (obj->write_domain != 0) { + list_move_tail(&obj_priv->list, + &dev_priv->mm.flushing_list); + } else { + i915_gem_object_move_to_inactive(obj); + } + } +} + /** * Returns true if seq1 is later than seq2. */ @@ -198,6 +260,8 @@ i915_gem_retire_requests(struct drm_device *dev) retiring_seqno = request->seqno; if (i915_seqno_passed(seqno, retiring_seqno)) { + i915_gem_retire_request(dev, request); + list_del(&request->list); drm_free(request, sizeof(*request), DRM_MEM_DRIVER); } else @@ -317,15 +381,17 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) * create a new seqno to wait for. */ if (obj->write_domain & ~(DRM_GEM_DOMAIN_CPU)) { + uint32_t write_domain = obj->write_domain; #if WATCH_BUF DRM_INFO("%s: flushing object %p from write domain %08x\n", - __func__, obj, obj->write_domain); + __func__, obj, write_domain); #endif - i915_gem_flush(dev, 0, obj->write_domain); + i915_gem_flush(dev, 0, write_domain); obj->write_domain = 0; i915_gem_object_move_to_active(obj); - obj_priv->last_rendering_seqno = i915_add_request(dev); + obj_priv->last_rendering_seqno = i915_add_request(dev, + write_domain); BUG_ON(obj_priv->last_rendering_seqno == 0); #if WATCH_LRU DRM_INFO("%s: flush moves to exec list %p\n", __func__, obj); @@ -343,11 +409,7 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) if (ret != 0) return ret; - i915_gem_object_move_to_inactive(obj); - -#if WATCH_LRU - DRM_INFO("%s: wait moves to lru list %p\n", __func__, obj); -#endif + BUG_ON(obj_priv->active); } return 0; @@ -471,54 +533,73 @@ 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; - /* Find the LRU buffer. */ - if (!list_empty(&dev_priv->mm.inactive_list)) { - obj_priv = list_first_entry(&dev_priv->mm.inactive_list, - struct drm_i915_gem_object, - list); - } else if (!list_empty(&dev_priv->mm.active_list)) { - int found = 0; + for (;;) { + /* If there's an inactive buffer available now, grab it + * and be done. + */ + if (!list_empty(&dev_priv->mm.inactive_list)) { + obj_priv = list_first_entry(&dev_priv->mm.inactive_list, + struct drm_i915_gem_object, + list); + obj = obj_priv->obj; + BUG_ON(obj_priv->pin_count != 0); + break; + } - /* If there's nothing unused and ready, grab the first - * unpinned object from the currently executing list. + /* If we didn't get anything, but the ring is still processing + * things, wait for one of those things to finish and hopefully + * leave us a buffer to evict. */ - list_for_each_entry(obj_priv, &dev_priv->mm.active_list, - list) { - if (obj_priv->pin_count == 0) { - found = 1; - break; - } + 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 (!found) - return -ENOMEM; - } else { + + /* If we didn't have anything on the request list but there + * are buffers awaiting a flush, emit one and try again. + * When we wait on it, those buffers waiting for that flush + * will get moved to inactive. + */ + if (!list_empty(&dev_priv->mm.flushing_list)) { + obj_priv = list_first_entry(&dev_priv->mm.flushing_list, + struct drm_i915_gem_object, + list); + obj = obj_priv->obj; + + i915_gem_flush(dev, + obj->write_domain, + obj->write_domain); + i915_add_request(dev, obj->write_domain); + + obj = NULL; + continue; + } + + /* 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; } - obj = obj_priv->obj; - drm_gem_object_reference(obj); + #if WATCH_LRU DRM_INFO("%s: evicting %p\n", __func__, obj); #endif - /* Only unpinned buffers should be on this list. */ - BUG_ON(obj_priv->pin_count != 0); - - /* Do this separately from the wait_rendering in - * i915_gem_object_unbind() because we want to catch interrupts and - * return. - */ - ret = i915_gem_object_wait_rendering(obj); - if (ret != 0) - return ret; + BUG_ON(obj_priv->active); /* Wait on the rendering and unbind the buffer. */ i915_gem_object_unbind(obj); -#if WATCH_LRU - DRM_INFO("%s: evicted %p\n", __func__, obj); -#endif - drm_gem_object_unreference(obj); return 0; } @@ -713,12 +794,15 @@ i915_gem_object_set_domain(struct drm_gem_object *obj, /** * Once all of the objects have been set in the proper domain, - * perform the necessary flush and invalidate operations + * perform the necessary flush and invalidate operations. + * + * Returns the write domains flushed, for use in flush tracking. */ - -static void +static uint32_t i915_gem_dev_set_domain(struct drm_device *dev) { + uint32_t flush_domains = dev->flush_domains; + /* * Now that all the buffers are synced to the proper domains, * flush and invalidate the collected domains @@ -736,6 +820,8 @@ i915_gem_dev_set_domain(struct drm_device *dev) dev->invalidate_domains = 0; dev->flush_domains = 0; } + + return flush_domains; } static int @@ -1014,7 +1100,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, struct drm_gem_object *batch_obj; int ret, i; uint64_t exec_offset; - uint32_t seqno; + uint32_t seqno, flush_domains; LOCK_TEST_WITH_RETURN(dev, file_priv); @@ -1099,7 +1185,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, } /* Flush/invalidate caches and chipset buffer */ - i915_gem_dev_set_domain(dev); + flush_domains = i915_gem_dev_set_domain(dev); exec_offset = validate_list[args->buffer_count - 1].offset; @@ -1124,7 +1210,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, * *some* interrupts representing completion of buffers that we can * wait on when trying to clear up gtt space). */ - seqno = i915_add_request(dev); + seqno = i915_add_request(dev, flush_domains); BUG_ON(seqno == 0); for (i = 0; i < args->buffer_count; i++) { struct drm_gem_object *obj = object_list[i]; -- cgit v1.2.3 From da3f099a7c4a18468ff84819ed39e42bec641e11 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Wed, 21 May 2008 00:32:02 -0700 Subject: [intel-gem] invalidate ring locals for pin/unpin/set_domain/free functions Ring locals must be reloaded from hardware in case the X server ran. --- 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 c67ce309..469b613f 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -408,8 +408,6 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) ret = i915_wait_request(dev, obj_priv->last_rendering_seqno); if (ret != 0) return ret; - - BUG_ON(obj_priv->active); } return 0; -- cgit v1.2.3 From 71b09a5f75c6063a592f7be07465761519839bcd Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Thu, 22 May 2008 10:59:59 -0700 Subject: [intel-gem] Force ring retire by emiting flush before user-interrupt. Commands in the ring are parsed and started when the head pointer passes by them, but they are not necessarily finished until a MI_FLUSH happens. This patch inserts a flush after the execbuffer (the only place a flush wasn't already happening). --- linux-core/i915_gem.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 469b613f..3214707f 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -161,6 +161,31 @@ i915_add_request(struct drm_device *dev, uint32_t flush_domains) return seqno; } +/** + * Command execution barrier + * + * Ensures that all commands in the ring are finished + * before signalling the CPU + */ + +uint32_t +i915_retire_commands(struct drm_device *dev) +{ + drm_i915_private_t *dev_priv = dev->dev_private; + uint32_t cmd = CMD_MI_FLUSH | MI_NO_WRITE_FLUSH; + uint32_t flush_domains = 0; + RING_LOCALS; + + /* The sampler always gets flushed on i965 (sigh) */ + if (IS_I965G(dev)) + flush_domains |= DRM_GEM_DOMAIN_I915_SAMPLER; + BEGIN_LP_RING(2); + OUT_RING(cmd); + OUT_RING(0); /* noop */ + ADVANCE_LP_RING(); + return flush_domains; +} + /** * Moves buffers associated only with the given active seqno from the active * to inactive list, potentially freeing them. @@ -359,6 +384,9 @@ i915_gem_flush(struct drm_device *dev, if (invalidate_domains & DRM_GEM_DOMAIN_I915_INSTRUCTION) cmd |= MI_EXE_FLUSH; +#if WATCH_EXEC + DRM_INFO("%s: queue flush %08x to ring\n", __func__, cmd); +#endif BEGIN_LP_RING(2); OUT_RING(cmd); OUT_RING(0); /* noop */ @@ -1201,6 +1229,12 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, goto err; } + /* + * Ensure that the commands in the batch buffer are + * finished before the interrupt fires + */ + flush_domains |= i915_retire_commands(dev); + /* * Get a seqno representing the execution of the current buffer, * which we can wait on. We would like to mitigate these interrupts, -- cgit v1.2.3 From 44ed693ca6f8d19acb39174c6efada070652a027 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Thu, 22 May 2008 11:34:56 -0700 Subject: [gem] Use CPU domain for new or pageable objects Newly allocated objects need to be in the CPU domain as they've just been cleared by the CPU. Also, unmapping objects from the GTT needs to put them into the CPU domain, both to flush rendering as well as to ensure that any paging action gets flushed before we remap to the GTT. --- linux-core/i915_gem.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 3214707f..60a8fa58 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -35,6 +35,11 @@ #define WATCH_LRU 0 #define WATCH_RELOC 0 +static void +i915_gem_object_set_domain(struct drm_gem_object *obj, + uint32_t read_domains, + uint32_t write_domain); + int i915_gem_init_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) @@ -456,11 +461,14 @@ i915_gem_object_unbind(struct drm_gem_object *obj) if (obj_priv->gtt_space == NULL) return; - /* Ignore the return value of wait_rendering. If we're here but - * a wait_rendering hasn't completed, we're in the freeing process, - * and we want the buffer to go away even if the command queue is hung. + /* 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 + * also ensure that all pending GPU writes are finished + * before we unbind. */ - (void)i915_gem_object_wait_rendering(obj); + i915_gem_object_set_domain (obj, DRM_GEM_DOMAIN_CPU, + DRM_GEM_DOMAIN_CPU); if (obj_priv->agp_mem != NULL) { drm_unbind_agp(obj_priv->agp_mem); -- cgit v1.2.3 From 8c2b207f9b1fb1cf6df23c7ef73ca57dfb5dd459 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Thu, 22 May 2008 23:08:38 -0700 Subject: [intel-gem] Encourage multiple caches to hold read data When reading from multiple domains, allow each cache to continue to hold data until writes occur somewhere. This is done by first leaving the read_domains alone at bind time (presumably the CPU read cache contains valid data still) and then in set_domain, if no write_domain is specified, the new read domains are simply merged into the existing read domains. A huge comment was added above set_domain to explain how things are expected to work. --- linux-core/i915_gem.c | 123 ++++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 118 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 60a8fa58..b3b2dbce 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -746,12 +746,12 @@ i915_gem_object_bind_to_gtt(struct drm_gem_object *obj, unsigned alignment) return -ENOMEM; } - /* When we have just bound an object, we have no valid read - * caches on it, regardless of where it was before. We also need - * an MI_FLUSH to occur so that the render and sampler TLBs - * get flushed and pick up our binding change above. + /* 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 + * a GPU cache */ - obj->read_domains = 0; + BUG_ON(obj->read_domains & ~DRM_GEM_DOMAIN_CPU); + BUG_ON(obj->write_domain & ~DRM_GEM_DOMAIN_CPU); return 0; } @@ -775,6 +775,112 @@ i915_gem_clflush_object(struct drm_gem_object *obj) * Set the next domain for the specified object. This * may not actually perform the necessary flushing/invaliding though, * as that may want to be batched with other set_domain operations + * + * This is (we hope) the only really tricky part of gem. The goal + * is fairly simple -- track which caches hold bits of the object + * and make sure they remain coherent. A few concrete examples may + * help to explain how it works. For shorthand, we use the notation + * (read_domains, write_domain), e.g. (CPU, CPU) to indicate the + * a pair of read and write domain masks. + * + * Case 1: the batch buffer + * + * 1. Allocated + * 2. Written by CPU + * 3. Mapped to GTT + * 4. Read by GPU + * 5. Unmapped from GTT + * 6. Freed + * + * Let's take these a step at a time + * + * 1. Allocated + * Pages allocated from the kernel may still have + * cache contents, so we set them to (CPU, CPU) always. + * 2. Written by CPU (using pwrite) + * The pwrite function calls set_domain (CPU, CPU) and + * this function does nothing (as nothing changes) + * 3. Mapped by GTT + * This function asserts that the object is not + * currently in any GPU-based read or write domains + * 4. Read by GPU + * i915_gem_execbuffer calls set_domain (COMMAND, 0). + * As write_domain is zero, this function adds in the + * current read domains (CPU+COMMAND, 0). + * flush_domains is set to CPU. + * invalidate_domains is set to COMMAND + * clflush is run to get data out of the CPU caches + * then i915_dev_set_domain calls i915_gem_flush to + * emit an MI_FLUSH and drm_agp_chipset_flush + * 5. Unmapped from GTT + * i915_gem_object_unbind calls set_domain (CPU, CPU) + * flush_domains and invalidate_domains end up both zero + * so no flushing/invalidating happens + * 6. Freed + * yay, done + * + * Case 2: The shared render buffer + * + * 1. Allocated + * 2. Mapped to GTT + * 3. Read/written by GPU + * 4. set_domain to (CPU,CPU) + * 5. Read/written by CPU + * 6. Read/written by GPU + * + * 1. Allocated + * Same as last example, (CPU, CPU) + * 2. Mapped to GTT + * Nothing changes (assertions find that it is not in the GPU) + * 3. Read/written by GPU + * execbuffer calls set_domain (RENDER, RENDER) + * flush_domains gets CPU + * invalidate_domains gets GPU + * clflush (obj) + * MI_FLUSH and drm_agp_chipset_flush + * 4. set_domain (CPU, CPU) + * flush_domains gets GPU + * invalidate_domains gets CPU + * wait_rendering (obj) to make sure all drawing is complete. + * This will include an MI_FLUSH to get the data from GPU + * to memory + * clflush (obj) to invalidate the CPU cache + * Another MI_FLUSH in i915_gem_flush (eliminate this somehow?) + * 5. Read/written by CPU + * cache lines are loaded and dirtied + * 6. Read written by GPU + * Same as last GPU access + * + * Case 3: The constant buffer + * + * 1. Allocated + * 2. Written by CPU + * 3. Read by GPU + * 4. Updated (written) by CPU again + * 5. Read by GPU + * + * 1. Allocated + * (CPU, CPU) + * 2. Written by CPU + * (CPU, CPU) + * 3. Read by GPU + * (CPU+RENDER, 0) + * flush_domains = CPU + * invalidate_domains = RENDER + * clflush (obj) + * MI_FLUSH + * drm_agp_chipset_flush + * 4. Updated (written) by CPU again + * (CPU, CPU) + * flush_domains = 0 (no previous write domain) + * invalidate_domains = 0 (no new read domains) + * 5. Read by GPU + * (CPU+RENDER, 0) + * flush_domains = CPU + * invalidate_domains = RENDER + * clflush (obj) + * MI_FLUSH + * drm_agp_chipset_flush */ static void i915_gem_object_set_domain(struct drm_gem_object *obj, @@ -789,6 +895,13 @@ i915_gem_object_set_domain(struct drm_gem_object *obj, DRM_INFO("%s: object %p read %08x write %08x\n", __func__, obj, read_domains, write_domain); #endif + /* + * If the object isn't moving to a new write domain, + * let the object stay in multiple read domains + */ + if (write_domain == 0) + read_domains |= obj->read_domains; + /* * Flush the current write domain if * the new read domains don't match. Invalidate -- cgit v1.2.3 From c69b81df62cb7e04f956f2cf77091216754c3632 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Sun, 25 May 2008 20:41:42 -0700 Subject: [intel-gem] replace call to jiffies_to-msec with simple inline --- linux-core/i915_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index b3b2dbce..c53a39ec 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1221,7 +1221,7 @@ i915_gem_ring_throttle(struct drm_device *dev) list); /* Break out if we're close enough. */ - if (jiffies_to_msecs(jiffies - request->emitted_jiffies) < 20) { + if ((long) (jiffies - request->emitted_jiffies) <= (20 * HZ) / 1000) { mutex_unlock(&dev->struct_mutex); return 0; } -- cgit v1.2.3 From 6d1d11704ab36e4ee50b2c1d3b984ab6bb691417 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Sun, 25 May 2008 20:44:19 -0700 Subject: [intel-gem] Compute npages instead of nbytes in flush_pwrite i915_gem_flush_pwrite optimizes short writes to the buffer by clflushing only the modified pages, but it was miscomputing the number of pages. --- linux-core/i915_gem.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index c53a39ec..b5c87747 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1523,7 +1523,7 @@ i915_gem_flush_pwrite(struct drm_gem_object *obj, 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); + 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); -- cgit v1.2.3 From 7cf3fd29fe058a0bfc2ba7e889d1b360398be161 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Sun, 25 May 2008 20:45:20 -0700 Subject: [intel-gem] Add DRM_I915_GEM_BUSY ioctl to check for idle buffers. This new ioctl returns whether re-using the buffer would force a wait. --- linux-core/i915_gem.c | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index b5c87747..99dc00fc 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1471,6 +1471,31 @@ i915_gem_unpin_ioctl(struct drm_device *dev, void *data, return 0; } +int +i915_gem_busy_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + struct drm_i915_gem_busy *args = data; + struct drm_gem_object *obj; + struct drm_i915_gem_object *obj_priv; + + mutex_lock(&dev->struct_mutex); + obj = drm_gem_object_lookup(dev, file_priv, args->handle); + if (obj == NULL) { + DRM_ERROR("Bad handle in i915_gem_busy_ioctl(): %d\n", + args->handle); + mutex_unlock(&dev->struct_mutex); + return -EINVAL; + } + + obj_priv = obj->driver_private; + args->busy = obj_priv->active; + + drm_gem_object_unreference(obj); + mutex_unlock(&dev->struct_mutex); + return 0; +} + int i915_gem_init_object(struct drm_gem_object *obj) { struct drm_i915_gem_object *obj_priv; -- cgit v1.2.3 From d434b64f6a760d85295e32298a9a1f3624ee1b69 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Mon, 26 May 2008 03:25:16 -0700 Subject: [i915] leave interrupts masked off when not in use. The interrupt enable register cannot be used to temporarily disable interrupts, instead use the interrupt mask register. Note that this change means that a pile of buffers will be left stuck on the chip as the final interrupts will not be recognized to come and drain things. --- linux-core/i915_gem.c | 14 +++++++++----- 1 file changed, 9 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 99dc00fc..3ad3f40c 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -311,11 +311,13 @@ i915_wait_request(struct drm_device *dev, uint32_t seqno) BUG_ON(seqno == 0); - i915_user_irq_on(dev_priv); - ret = wait_event_interruptible(dev_priv->irq_queue, - i915_seqno_passed(i915_get_gem_seqno(dev), - seqno)); - i915_user_irq_off(dev_priv); + if (!i915_seqno_passed(i915_get_gem_seqno(dev), seqno)) { + i915_user_irq_on(dev_priv); + ret = wait_event_interruptible(dev_priv->irq_queue, + i915_seqno_passed(i915_get_gem_seqno(dev), + seqno)); + i915_user_irq_off(dev_priv); + } /* Directly dispatch request retiring. While we have the work queue * to handle this, the waiter on a request often wants an associated @@ -1538,6 +1540,7 @@ 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; @@ -1555,6 +1558,7 @@ i915_gem_flush_pwrite(struct drm_gem_object *obj, drm_agp_chipset_flush(dev); obj->write_domain = 0; } +#endif return 0; } -- cgit v1.2.3 From 1f4e36081bd6ff7d7b53a62e0c8db7c0f82edf99 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Mon, 26 May 2008 17:41:46 -0700 Subject: [intel-gem] Must hold DRM lock while setting object domain Object domain transfer can involve adding flush ops to the request queue, and so the DRM lock must be held to avoid having the X server smash pointers badly. --- 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 3ad3f40c..5eeabdaa 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1529,9 +1529,11 @@ i915_gem_set_domain(struct drm_gem_object *obj, BUG_ON(!mutex_is_locked(&dev->struct_mutex)); + drm_idlelock_take (&dev->lock); i915_kernel_lost_context(dev); i915_gem_object_set_domain(obj, read_domains, write_domain); i915_gem_dev_set_domain(obj->dev); + drm_idlelock_release (&dev->lock); return 0; } -- cgit v1.2.3 From e10502002f0ebb2b56b19384b2f2eae7a7a84512 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Tue, 27 May 2008 17:50:39 -0700 Subject: [intel-gem] Replace idlelock usage with real lock acquisition. --- 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 5eeabdaa..ebe51539 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1522,6 +1522,7 @@ void i915_gem_free_object(struct drm_gem_object *obj) int i915_gem_set_domain(struct drm_gem_object *obj, + struct drm_file *file_priv, uint32_t read_domains, uint32_t write_domain) { @@ -1529,11 +1530,11 @@ i915_gem_set_domain(struct drm_gem_object *obj, BUG_ON(!mutex_is_locked(&dev->struct_mutex)); - drm_idlelock_take (&dev->lock); + drm_client_lock_take(dev, file_priv); i915_kernel_lost_context(dev); i915_gem_object_set_domain(obj, read_domains, write_domain); i915_gem_dev_set_domain(obj->dev); - drm_idlelock_release (&dev->lock); + drm_client_lock_release(dev); return 0; } -- cgit v1.2.3 From 19ff3366e4ed591741af4bcf49991823115bdb17 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Tue, 27 May 2008 16:49:49 -0700 Subject: [intel-gem] Clean up active/inactive/flushing list debugging. --- linux-core/i915_gem.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index ebe51539..1fc48e0c 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -240,7 +240,7 @@ i915_gem_retire_request(struct drm_device *dev, return; #if WATCH_LRU DRM_INFO("%s: retire %d moves to inactive list %p\n", - __func__, seqno, obj); + __func__, request->seqno, obj); #endif if (obj->write_domain != 0) { @@ -547,14 +547,23 @@ i915_dump_lru(struct drm_device *dev, const char *where) drm_i915_private_t *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj_priv; - DRM_INFO("GTT execution list %s {\n", where); + DRM_INFO("active list %s {\n", where); list_for_each_entry(obj_priv, &dev_priv->mm.active_list, list) { DRM_INFO(" %p: %08x\n", obj_priv, obj_priv->last_rendering_seqno); } - DRM_INFO("GTT LRU %s {\n", where); + DRM_INFO("}\n"); + DRM_INFO("flushing list %s {\n", where); + list_for_each_entry(obj_priv, &dev_priv->mm.flushing_list, + list) + { + DRM_INFO(" %p: %08x\n", obj_priv, + obj_priv->last_rendering_seqno); + } + DRM_INFO("}\n"); + DRM_INFO("inactive %s {\n", where); list_for_each_entry(obj_priv, &dev_priv->mm.inactive_list, list) { DRM_INFO(" %p: %08x\n", obj_priv, obj_priv->last_rendering_seqno); @@ -1377,7 +1386,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, DRM_INFO("%s: move to exec list %p\n", __func__, obj); #endif } -#if WATCH_LRU && 0 +#if WATCH_LRU i915_dump_lru(dev, __func__); #endif -- cgit v1.2.3 From 3b1e4e6dc38029e697afb8e6ec81ebbed7adf442 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Thu, 29 May 2008 12:53:13 -0700 Subject: [intel-gem] Write the presumed_offset back out after updating it. Otherwise, 965 constant state buffers get re-relocated every exec. Ouch. --- linux-core/i915_gem.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 1fc48e0c..52a57d5e 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1137,6 +1137,16 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, #endif writel(reloc_val, reloc_entry); + /* Write the updated presumed offset for this entry back out + * to the user. + */ + reloc.presumed_offset = target_obj_priv->gtt_offset; + ret = copy_to_user(relocs + i, &reloc, sizeof(reloc)); + if (ret != 0) { + drm_gem_object_unreference(target_obj); + return ret; + } + drm_gem_object_unreference(target_obj); } -- cgit v1.2.3 From 4f92ed34270ae4afaa0ddba38d227c6e359bcc98 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 30 May 2008 10:04:22 -0700 Subject: [intel-gem] Add an option to check GTT versus CPU coherency at execbuf time. --- linux-core/i915_gem.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 52a57d5e..7c826aeb 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -30,6 +30,7 @@ #include "i915_drm.h" #include "i915_drv.h" +#define WATCH_COHERENCY 0 #define WATCH_BUF 0 #define WATCH_EXEC 0 #define WATCH_LRU 0 @@ -982,6 +983,76 @@ i915_gem_dev_set_domain(struct drm_device *dev) return flush_domains; } +#if WATCH_COHERENCY +static void +i915_gem_object_check_coherency(struct drm_gem_object *obj, int handle) +{ + struct drm_device *dev = obj->dev; + struct drm_i915_gem_object *obj_priv = obj->driver_private; + int page; + uint32_t *gtt_mapping; + uint32_t *backing_map = NULL; + int bad_count = 0; + + DRM_INFO("%s: checking coherency of object %p@0x%08x (%d, %dkb):\n", + __FUNCTION__, obj, obj_priv->gtt_offset, handle, + obj->size / 1024); + + gtt_mapping = ioremap(dev->agp->base + obj_priv->gtt_offset, + obj->size); + if (gtt_mapping == NULL) { + DRM_ERROR("failed to map GTT space\n"); + return; + } + + for (page = 0; page < obj->size / PAGE_SIZE; page++) { + int i; + + backing_map = kmap_atomic(obj_priv->page_list[page], KM_USER0); + + if (backing_map == NULL) { + DRM_ERROR("failed to map backing page\n"); + goto out; + } + + for (i = 0; i < PAGE_SIZE / 4; i++) { + uint32_t cpuval = backing_map[i]; + uint32_t gttval = readl(gtt_mapping + + page * 1024 + i); + + if (cpuval != gttval) { + DRM_INFO("incoherent CPU vs GPU at 0x%08x: " + "0x%08x vs 0x%08x\n", + (int)(obj_priv->gtt_offset + + page * PAGE_SIZE + i * 4), + cpuval, gttval); + if (bad_count++ >= 8) { + DRM_INFO("...\n"); + goto out; + } + } + } + kunmap_atomic(backing_map, KM_USER0); + backing_map = NULL; + } + + out: + if (backing_map != NULL) + kunmap_atomic(backing_map, KM_USER0); + iounmap(gtt_mapping); + + /* give syslog time to catch up */ + msleep(1); + + /* Directly flush the object, since we just loaded values with the CPU + * from thebacking pages and we don't want to disturb the cache + * management that we're trying to observe. + */ + + i915_gem_clflush_object(obj); +} +#endif + static int i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, struct drm_file *file_priv, @@ -1355,6 +1426,13 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, /* Flush/invalidate caches and chipset buffer */ flush_domains = i915_gem_dev_set_domain(dev); +#if WATCH_COHERENCY + for (i = 0; i < args->buffer_count; i++) { + i915_gem_object_check_coherency(object_list[i], + validate_list[i].handle); + } +#endif + exec_offset = validate_list[args->buffer_count - 1].offset; #if WATCH_EXEC -- cgit v1.2.3 From 50bce2bc625deb439dd61f504496dddd0cd4f572 Mon Sep 17 00:00:00 2001 From: Eric Anholt Date: Fri, 30 May 2008 13:47:34 -0700 Subject: [intel-gem] Only update obj->write_domain if we're actually changing it. The problem was revealed where on 965, the display list vertex buffer would see: create -> (CPU, CPU) set_domain (CPU, CPU) -> (CPU, CPU) set_comain (CPU, 0) -> (CPU, 0) (no clflush occurred) execbuf (GPU, 0) -> (CPU+GPU, 0) (still no clflush) instead of: create -> (CPU, CPU) set_domain (CPU, CPU) -> (CPU, CPU) set_comain (CPU, 0) -> (CPU, CPU) execbuf (GPU, 0) -> (CPU+GPU, 0) (clflushed) --- linux-core/i915_gem.c | 6 ++++-- 1 file changed, 4 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 7c826aeb..bcc15dd3 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -945,7 +945,8 @@ i915_gem_object_set_domain(struct drm_gem_object *obj, i915_gem_clflush_object(obj); } - obj->write_domain = write_domain; + if ((write_domain | flush_domains) != 0) + obj->write_domain = write_domain; obj->read_domains = read_domains; dev->invalidate_domains |= invalidate_domains; dev->flush_domains |= flush_domains; @@ -1225,7 +1226,8 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, iounmap(reloc_page); #if WATCH_BUF - i915_gem_dump_object(obj, 128, __func__, ~0); + if (0) + i915_gem_dump_object(obj, 128, __func__, ~0); #endif return 0; } -- cgit v1.2.3 From 1cb2940a252f970bad0f88a5f14b4d39ea53ef1f Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Mon, 2 Jun 2008 10:59:15 -0700 Subject: [intel-gem] Propagate set_domain errors. set_domain can block waiting for rendering to complete. If that process is interrupted by a signal, it can return -EINTR. Catch this error in all callers and correctly deal with the result. --- linux-core/i915_gem.c | 58 ++++++++++++++++++++++++++++++++++++--------------- 1 file changed, 41 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 bcc15dd3..4e01b8ea 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -36,7 +36,7 @@ #define WATCH_LRU 0 #define WATCH_RELOC 0 -static void +static int i915_gem_object_set_domain(struct drm_gem_object *obj, uint32_t read_domains, uint32_t write_domain); @@ -452,17 +452,18 @@ i915_gem_object_wait_rendering(struct drm_gem_object *obj) /** * Unbinds an object from the GTT aperture. */ -static void +static int i915_gem_object_unbind(struct drm_gem_object *obj) { struct drm_i915_gem_object *obj_priv = obj->driver_private; + int ret = 0; #if WATCH_BUF DRM_INFO("%s:%d %p\n", __func__, __LINE__, obj); DRM_INFO("gtt_space %p\n", obj_priv->gtt_space); #endif if (obj_priv->gtt_space == NULL) - return; + return 0; /* Move the object to the CPU domain to ensure that * any possible CPU writes while it's not in the GTT @@ -470,8 +471,10 @@ i915_gem_object_unbind(struct drm_gem_object *obj) * also ensure that all pending GPU writes are finished * before we unbind. */ - i915_gem_object_set_domain (obj, DRM_GEM_DOMAIN_CPU, - DRM_GEM_DOMAIN_CPU); + ret = i915_gem_object_set_domain (obj, DRM_GEM_DOMAIN_CPU, + DRM_GEM_DOMAIN_CPU); + if (ret) + return ret; if (obj_priv->agp_mem != NULL) { drm_unbind_agp(obj_priv->agp_mem); @@ -494,6 +497,7 @@ i915_gem_object_unbind(struct drm_gem_object *obj) drm_gem_object_unreference(obj); } } + return 0; } #if WATCH_BUF | WATCH_EXEC @@ -579,6 +583,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; for (;;) { /* If there's an inactive buffer available now, grab it @@ -645,9 +650,9 @@ i915_gem_evict_something(struct drm_device *dev) BUG_ON(obj_priv->active); /* Wait on the rendering and unbind the buffer. */ - i915_gem_object_unbind(obj); + ret = i915_gem_object_unbind(obj); - return 0; + return ret; } static int @@ -894,7 +899,7 @@ i915_gem_clflush_object(struct drm_gem_object *obj) * MI_FLUSH * drm_agp_chipset_flush */ -static void +static int i915_gem_object_set_domain(struct drm_gem_object *obj, uint32_t read_domains, uint32_t write_domain) @@ -902,6 +907,7 @@ i915_gem_object_set_domain(struct drm_gem_object *obj, struct drm_device *dev = obj->dev; uint32_t invalidate_domains = 0; uint32_t flush_domains = 0; + int ret; #if WATCH_BUF DRM_INFO("%s: object %p read %08x write %08x\n", @@ -940,8 +946,11 @@ i915_gem_object_set_domain(struct drm_gem_object *obj, * flushed before the cpu cache is invalidated */ if ((invalidate_domains & DRM_GEM_DOMAIN_CPU) && - (flush_domains & ~DRM_GEM_DOMAIN_CPU)) - i915_gem_object_wait_rendering(obj); + (flush_domains & ~DRM_GEM_DOMAIN_CPU)) { + ret = i915_gem_object_wait_rendering(obj); + if (ret) + return ret; + } i915_gem_clflush_object(obj); } @@ -950,6 +959,7 @@ i915_gem_object_set_domain(struct drm_gem_object *obj, obj->read_domains = read_domains; dev->invalidate_domains |= invalidate_domains; dev->flush_domains |= flush_domains; + return 0; } /** @@ -1378,6 +1388,14 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, } mutex_lock(&dev->struct_mutex); + + /* Zero the gloabl flush/invalidate flags. These + * will be modified as each object is bound to the + * gtt + */ + dev->invalidate_domains = 0; + dev->flush_domains = 0; + /* Look up object handles and perform the relocations */ for (i = 0; i < args->buffer_count; i++) { object_list[i] = drm_gem_object_lookup(dev, file_priv, @@ -1389,6 +1407,8 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, goto err; } + object_list[i]->pending_read_domains = 0; + object_list[i]->pending_write_domain = 0; ret = i915_gem_reloc_and_validate_object(object_list[i], file_priv, &validate_list[i]); @@ -1418,11 +1438,11 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, } /* make sure all previous memory operations have passed */ - i915_gem_object_set_domain(obj, - obj->pending_read_domains, - obj->pending_write_domain); - obj->pending_read_domains = 0; - obj->pending_write_domain = 0; + ret = i915_gem_object_set_domain(obj, + obj->pending_read_domains, + obj->pending_write_domain); + if (ret) + goto err; } /* Flush/invalidate caches and chipset buffer */ @@ -1626,15 +1646,19 @@ i915_gem_set_domain(struct drm_gem_object *obj, uint32_t write_domain) { struct drm_device *dev = obj->dev; + int ret; BUG_ON(!mutex_is_locked(&dev->struct_mutex)); drm_client_lock_take(dev, file_priv); i915_kernel_lost_context(dev); - i915_gem_object_set_domain(obj, read_domains, write_domain); + ret = i915_gem_object_set_domain(obj, read_domains, write_domain); + if (ret) { + drm_client_lock_release(dev); + return ret; + } i915_gem_dev_set_domain(obj->dev); drm_client_lock_release(dev); - return 0; } -- cgit v1.2.3 From 867c2bb461e4bf7765fdbf502f625b739ceecb96 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Mon, 2 Jun 2008 12:37:10 -0700 Subject: =?UTF-8?q?[intel-gem]=20reloc=5Fand=5Fvalidate=5Fobject=20?= =?UTF-8?q?=E2=86=92=20object=5Fbind=5Fand=5Frelocate?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Just renaming this function and related parameters to match terminology used elsewhere in the driver. --- linux-core/i915_gem.c | 64 +++++++++++++++++++++++++++++---------------------- 1 file changed, 36 insertions(+), 28 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 4e01b8ea..ad73f0a0 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1064,10 +1064,15 @@ 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 + * + * + */ static int -i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, - struct drm_file *file_priv, - struct drm_i915_gem_exec_object *entry) +i915_gem_object_bind_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; @@ -1108,7 +1113,7 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, target_obj_priv = target_obj->driver_private; /* The target buffer should have appeared before us in the - * validate list, so it should have a GTT space bound by now. + * exec_object list, so it should have a GTT space bound by now. */ if (target_obj_priv->gtt_space == NULL) { DRM_ERROR("No GTT space found for object %d\n", @@ -1242,6 +1247,8 @@ i915_gem_reloc_and_validate_object(struct drm_gem_object *obj, return 0; } +/** Dispatch a batchbuffer to the ring + */ static int i915_dispatch_gem_execbuffer(struct drm_device *dev, struct drm_i915_gem_execbuffer *exec, @@ -1346,7 +1353,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, struct drm_file *file_priv) { struct drm_i915_gem_execbuffer *args = data; - struct drm_i915_gem_exec_object *validate_list = NULL; + 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; @@ -1365,26 +1372,26 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, if (ret) return ret; - /* Copy in the validate list from userland */ - validate_list = drm_calloc(sizeof(*validate_list), args->buffer_count, - DRM_MEM_DRIVER); + /* Copy in the exec list from userland */ + exec_list = drm_calloc(sizeof(*exec_list), args->buffer_count, + DRM_MEM_DRIVER); object_list = drm_calloc(sizeof(*object_list), args->buffer_count, DRM_MEM_DRIVER); - if (validate_list == NULL || object_list == NULL) { - DRM_ERROR("Failed to allocate validate or object list " + if (exec_list == NULL || object_list == NULL) { + DRM_ERROR("Failed to allocate exec or object list " "for %d buffers\n", args->buffer_count); ret = -ENOMEM; - goto err; + goto pre_mutex_err; } - ret = copy_from_user(validate_list, + ret = copy_from_user(exec_list, (struct drm_i915_relocation_entry __user *) (uintptr_t) args->buffers_ptr, - sizeof(*validate_list) * args->buffer_count); + sizeof(*exec_list) * args->buffer_count); if (ret != 0) { - DRM_ERROR("copy %d validate entries failed %d\n", + DRM_ERROR("copy %d exec entries failed %d\n", args->buffer_count, ret); - goto err; + goto pre_mutex_err; } mutex_lock(&dev->struct_mutex); @@ -1399,21 +1406,21 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, /* Look up object handles and perform the relocations */ for (i = 0; i < args->buffer_count; i++) { object_list[i] = drm_gem_object_lookup(dev, file_priv, - validate_list[i].handle); + exec_list[i].handle); if (object_list[i] == NULL) { DRM_ERROR("Invalid object handle %d at index %d\n", - validate_list[i].handle, i); + exec_list[i].handle, i); ret = -EINVAL; goto err; } object_list[i]->pending_read_domains = 0; object_list[i]->pending_write_domain = 0; - ret = i915_gem_reloc_and_validate_object(object_list[i], - file_priv, - &validate_list[i]); + ret = i915_gem_object_bind_and_relocate(object_list[i], + file_priv, + &exec_list[i]); if (ret) { - DRM_ERROR("reloc and validate failed %d\n", ret); + DRM_ERROR("object bind and relocate failed %d\n", ret); goto err; } } @@ -1451,11 +1458,11 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, #if WATCH_COHERENCY for (i = 0; i < args->buffer_count; i++) { i915_gem_object_check_coherency(object_list[i], - validate_list[i].handle); + exec_list[i].handle); } #endif - exec_offset = validate_list[args->buffer_count - 1].offset; + exec_offset = exec_list[args->buffer_count - 1].offset; #if WATCH_EXEC i915_gem_dump_object(object_list[args->buffer_count - 1], @@ -1500,13 +1507,13 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, i915_dump_lru(dev, __func__); #endif - /* Copy the new buffer offsets back to the user's validate list. */ + /* 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, - validate_list, - sizeof(*validate_list) * args->buffer_count); + exec_list, + sizeof(*exec_list) * args->buffer_count); if (ret) - DRM_ERROR("failed to copy %d validate entries " + DRM_ERROR("failed to copy %d exec entries " "back to user (%d)\n", args->buffer_count, ret); err: @@ -1516,9 +1523,10 @@ err: } mutex_unlock(&dev->struct_mutex); +pre_mutex_err: drm_free(object_list, sizeof(*object_list) * args->buffer_count, DRM_MEM_DRIVER); - drm_free(validate_list, sizeof(*validate_list) * args->buffer_count, + drm_free(exec_list, sizeof(*exec_list) * args->buffer_count, DRM_MEM_DRIVER); return ret; -- cgit v1.2.3 From 118baeee1820102177f4f5bb48dd2a1e3d95d21e Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Thu, 5 Jun 2008 13:47:41 -0700 Subject: [intel-gem] Dump error status on wait_request failure --- linux-core/i915_gem.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index ad73f0a0..268411e8 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -319,6 +319,9 @@ i915_wait_request(struct drm_device *dev, uint32_t seqno) 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)); /* Directly dispatch request retiring. While we have the work queue * to handle this, the waiter on a request often wants an associated -- cgit v1.2.3 From 56a96841d01d112d7d4adfebb572016398551ba8 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 6 Jun 2008 12:57:01 -0700 Subject: [intel-gem] Add explicit throttle ioctl Instead of throttling and execbuffer time, have the application ask to throttle explicitly. This allows the throttle to happen less often, and without holding the DRM lock. --- linux-core/i915_gem.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 268411e8..14e57b41 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1371,10 +1371,6 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, #endif i915_kernel_lost_context(dev); - ret = i915_gem_ring_throttle(dev); - if (ret) - return ret; - /* Copy in the exec list from userland */ exec_list = drm_calloc(sizeof(*exec_list), args->buffer_count, DRM_MEM_DRIVER); @@ -1628,6 +1624,13 @@ i915_gem_busy_ioctl(struct drm_device *dev, void *data, return 0; } +int +i915_gem_throttle_ioctl(struct drm_device *dev, void *data, + struct drm_file *file_priv) +{ + return i915_gem_ring_throttle(dev); +} + int i915_gem_init_object(struct drm_gem_object *obj) { struct drm_i915_gem_object *obj_priv; -- cgit v1.2.3 From 9f46c6935d154743162c6239903a4a9e443907bc Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 6 Jun 2008 12:59:52 -0700 Subject: [intel-gem] Use timers to retire requests periodically. Without the user IRQ running constantly, there's no wakeup when the ring empties to go retire requests and free buffers. Use a 1 second timer to make that happen more often. --- linux-core/i915_gem.c | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) (limited to 'linux-core/i915_gem.c') diff --git a/linux-core/i915_gem.c b/linux-core/i915_gem.c index 14e57b41..abc929e9 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -162,6 +162,9 @@ 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); + list_add_tail(&request->list, &dev_priv->mm.request_list); return seqno; @@ -300,6 +303,32 @@ 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) +{ + drm_i915_private_t *dev_priv; + struct drm_device *dev; + + dev_priv = container_of(work, drm_i915_private_t, + mm.retire_task); + 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); + mutex_unlock(&dev->struct_mutex); +} + /** * Waits for a sequence number to be signaled, and cleans up the * request and object lists appropriately for that event. -- cgit v1.2.3 From 6cd0ef06a6c2bdcede166d9a2d0434e58e4a01f2 Mon Sep 17 00:00:00 2001 From: Keith Packard Date: Fri, 6 Jun 2008 13:26:03 -0700 Subject: [intel] remove settable use_mi_batchbuffer_start The driver can know what hardware requires MI_BATCH_BUFFER vs MI_BATCH_BUFFER_START; there's no reason to let user mode configure this. --- linux-core/i915_gem.c | 17 ++++++++--------- 1 file changed, 8 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 abc929e9..d60a98f8 100644 --- a/linux-core/i915_gem.c +++ b/linux-core/i915_gem.c @@ -1315,7 +1315,14 @@ i915_dispatch_gem_execbuffer(struct drm_device *dev, return ret; } - if (dev_priv->use_mi_batchbuffer_start) { + if (IS_I830(dev) || IS_845G(dev)) { + BEGIN_LP_RING(4); + OUT_RING(MI_BATCH_BUFFER); + OUT_RING(exec_start | MI_BATCH_NON_SECURE); + OUT_RING(exec_start + exec_len - 4); + OUT_RING(0); + ADVANCE_LP_RING(); + } else { BEGIN_LP_RING(2); if (IS_I965G(dev)) { OUT_RING(MI_BATCH_BUFFER_START | @@ -1328,14 +1335,6 @@ i915_dispatch_gem_execbuffer(struct drm_device *dev, OUT_RING(exec_start | MI_BATCH_NON_SECURE); } ADVANCE_LP_RING(); - - } else { - BEGIN_LP_RING(4); - OUT_RING(MI_BATCH_BUFFER); - OUT_RING(exec_start | MI_BATCH_NON_SECURE); - OUT_RING(exec_start + exec_len - 4); - OUT_RING(0); - ADVANCE_LP_RING(); } } -- cgit v1.2.3