From d7f46fc4e7323887494db13f063a8e59861fefb0 Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Fri, 6 Dec 2013 14:10:55 -0800 Subject: drm/i915: Make pin count per VMA Signed-off-by: Ben Widawsky Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 9282b4c411f6..a2d6eb5336e3 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -566,7 +566,7 @@ i915_gem_execbuffer_unreserve_vma(struct i915_vma *vma) i915_gem_object_unpin_fence(obj); if (entry->flags & __EXEC_OBJECT_HAS_PIN) - i915_gem_object_unpin(obj); + vma->pin_count--; entry->flags &= ~(__EXEC_OBJECT_HAS_FENCE | __EXEC_OBJECT_HAS_PIN); } @@ -923,7 +923,9 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas, if (obj->base.write_domain) { obj->dirty = 1; obj->last_write_seqno = intel_ring_get_seqno(ring); - if (obj->pin_count) /* check for potential scanout */ + /* check for potential scanout */ + if (i915_gem_obj_ggtt_bound(obj) && + i915_gem_obj_to_ggtt(obj)->pin_count) intel_mark_fb_busy(obj, ring); } -- cgit From 6f65e29acad7499920cf1e49b675fac7cde24166 Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Fri, 6 Dec 2013 14:10:56 -0800 Subject: drm/i915: Create bind/unbind abstraction for VMAs To sum up what goes on here, we abstract the vma binding, similarly to the previous object binding. This helps for distinguishing legacy binding, versus modern binding. To keep the code churn as minimal as possible, I am leaving in insert_entries(). It serves as the per platform pte writing basically. bind_vma and insert_entries do share a lot of similarities, and I did have designs to combine the two, but as mentioned already... too much churn in an already massive patchset. What follows are the 3 commits which existed discretely in the original submissions. Upon rebasing on Broadwell support, it became clear that separation was not good, and only made for more error prone code. Below are the 3 commit messages with all their history. drm/i915: Add bind/unbind object functions to VMA drm/i915: Use the new vm [un]bind functions drm/i915: reduce vm->insert_entries() usage drm/i915: Add bind/unbind object functions to VMA As we plumb the code with more VM information, it has become more obvious that the easiest way to deal with bind and unbind is to simply put the function pointers in the vm, and let those choose the correct way to handle the page table updates. This change allows many places in the code to simply be vm->bind, and not have to worry about distinguishing PPGTT vs GGTT. Notice that this patch has no impact on functionality. I've decided to save the actual change until the next patch because I think it's easier to review that way. I'm happy to squash the two, or let Daniel do it on merge. v2: Make ggtt handle the quirky aliasing ppgtt Add flags to bind object to support above Don't ever call bind/unbind directly for PPGTT until we have real, full PPGTT (use NULLs to assert this) Make sure we rebind the ggtt if there already is a ggtt binding. This happens on set cache levels. Use VMA for bind/unbind (Daniel, Ben) v3: Reorganize ggtt_vma_bind to be more concise and easier to read (Ville). Change logic in unbind to only unbind ggtt when there is a global mapping, and to remove a redundant check if the aliasing ppgtt exists. v4: Make the bind function a bit smarter about the cache levels to avoid unnecessary multiple remaps. "I accept it is a wart, I think unifying the pin_vma / bind_vma could be unified later" (Chris) Removed the git notes, and put version info here. (Daniel) v5: Update the comment to not suck (Chris) v6: Move bind/unbind to the VMA. It makes more sense in the VMA structure (always has, but I was previously lazy). With this change, it will allow us to keep a distinct insert_entries. Reviewed-by: Chris Wilson Signed-off-by: Ben Widawsky drm/i915: Use the new vm [un]bind functions Building on the last patch which created the new function pointers in the VM for bind/unbind, here we actually put those new function pointers to use. Split out as a separate patch to aid in review. I'm fine with squashing into the previous patch if people request it. v2: Updated to address the smart ggtt which can do aliasing as needed Make sure we bind to global gtt when mappable and fenceable. I thought we could get away without this initialy, but we cannot. v3: Make the global GTT binding explicitly use the ggtt VM for bind_vma(). While at it, use the new ggtt_vma helper (Chris) At this point the original mailing list thread diverges. ie. v4^: use target_obj instead of obj for gen6 relocate_entry vma->bind_vma() can be called safely during pin. So simply do that instead of the complicated conditionals. Don't restore PPGTT bound objects on resume path Bug fix in resume path for globally bound Bos Properly handle secure dispatch Rebased on vma bind/unbind conversion Signed-off-by: Ben Widawsky drm/i915: reduce vm->insert_entries() usage FKA: drm/i915: eliminate vm->insert_entries() With bind/unbind function pointers in place, we no longer need insert_entries. We could, and want, to remove clear_range, however it's not totally easy at this point. Since it's used in a couple of place still that don't only deal in objects: setup, ppgtt init, and restore gtt mappings. v2: Don't actually remove insert_entries, just limit its usage. It will be useful when we introduce gen8. It will always be called from the vma bind/unbind. Reviewed-by: Chris Wilson (v1) Signed-off-by: Ben Widawsky Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 103 ++++++++-------- drivers/gpu/drm/i915/i915_gem.c | 61 ++------- drivers/gpu/drm/i915/i915_gem_context.c | 8 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 43 ++++--- drivers/gpu/drm/i915/i915_gem_gtt.c | 192 +++++++++++++++++++++++------ 5 files changed, 245 insertions(+), 162 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index bf022c4a5773..9fe078b41674 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -523,6 +523,57 @@ enum i915_cache_level { typedef uint32_t gen6_gtt_pte_t; +/** + * A VMA represents a GEM BO that is bound into an address space. Therefore, a + * VMA's presence cannot be guaranteed before binding, or after unbinding the + * object into/from the address space. + * + * To make things as simple as possible (ie. no refcounting), a VMA's lifetime + * will always be <= an objects lifetime. So object refcounting should cover us. + */ +struct i915_vma { + struct drm_mm_node node; + struct drm_i915_gem_object *obj; + struct i915_address_space *vm; + + /** This object's place on the active/inactive lists */ + struct list_head mm_list; + + struct list_head vma_link; /* Link in the object's VMA list */ + + /** This vma's place in the batchbuffer or on the eviction list */ + struct list_head exec_list; + + /** + * Used for performing relocations during execbuffer insertion. + */ + struct hlist_node exec_node; + unsigned long exec_handle; + struct drm_i915_gem_exec_object2 *exec_entry; + + /** + * How many users have pinned this object in GTT space. The following + * users can each hold at most one reference: pwrite/pread, pin_ioctl + * (via user_pin_count), execbuffer (objects are not allowed multiple + * times for the same batchbuffer), and the framebuffer code. When + * switching/pageflipping, the framebuffer code has at most two buffers + * pinned per crtc. + * + * In the worst case this is 1 + 1 + 1 + 2*2 = 7. That would fit into 3 + * bits with absolutely no headroom. So use 4 bits. */ + unsigned int pin_count:4; +#define DRM_I915_GEM_OBJECT_MAX_PIN_COUNT 0xf + + /** Unmap an object from an address space. This usually consists of + * setting the valid PTE entries to a reserved scratch page. */ + void (*unbind_vma)(struct i915_vma *vma); + /* Map an object into an address space with the given cache flags. */ +#define GLOBAL_BIND (1<<0) + void (*bind_vma)(struct i915_vma *vma, + enum i915_cache_level cache_level, + u32 flags); +}; + struct i915_address_space { struct drm_mm mm; struct drm_device *dev; @@ -623,49 +674,6 @@ struct i915_hw_ppgtt { int (*enable)(struct drm_device *dev); }; -/** - * A VMA represents a GEM BO that is bound into an address space. Therefore, a - * VMA's presence cannot be guaranteed before binding, or after unbinding the - * object into/from the address space. - * - * To make things as simple as possible (ie. no refcounting), a VMA's lifetime - * will always be <= an objects lifetime. So object refcounting should cover us. - */ -struct i915_vma { - struct drm_mm_node node; - struct drm_i915_gem_object *obj; - struct i915_address_space *vm; - - /** This object's place on the active/inactive lists */ - struct list_head mm_list; - - struct list_head vma_link; /* Link in the object's VMA list */ - - /** This vma's place in the batchbuffer or on the eviction list */ - struct list_head exec_list; - - /** - * Used for performing relocations during execbuffer insertion. - */ - struct hlist_node exec_node; - unsigned long exec_handle; - struct drm_i915_gem_exec_object2 *exec_entry; - - /** - * How many users have pinned this object in GTT space. The following - * users can each hold at most one reference: pwrite/pread, pin_ioctl - * (via user_pin_count), execbuffer (objects are not allowed multiple - * times for the same batchbuffer), and the framebuffer code. When - * switching/pageflipping, the framebuffer code has at most two buffers - * pinned per crtc. - * - * In the worst case this is 1 + 1 + 1 + 2*2 = 7. That would fit into 3 - * bits with absolutely no headroom. So use 4 bits. - */ - unsigned int pin_count:4; -#define DRM_I915_GEM_OBJECT_MAX_PIN_COUNT 0xf -}; - struct i915_ctx_hang_stats { /* This context had batch pending when hang was declared */ unsigned batch_pending; @@ -2242,19 +2250,10 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, /* i915_gem_gtt.c */ void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev); -void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt, - struct drm_i915_gem_object *obj, - enum i915_cache_level cache_level); -void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt, - struct drm_i915_gem_object *obj); - void i915_check_and_clear_faults(struct drm_device *dev); void i915_gem_suspend_gtt_mappings(struct drm_device *dev); void i915_gem_restore_gtt_mappings(struct drm_device *dev); int __must_check i915_gem_gtt_prepare_object(struct drm_i915_gem_object *obj); -void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj, - enum i915_cache_level cache_level); -void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj); void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj); void i915_gem_init_global_gtt(struct drm_device *dev); void i915_gem_setup_global_gtt(struct drm_device *dev, unsigned long start, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 6dc96bceed19..2b92e89c5369 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2743,12 +2743,8 @@ int i915_vma_unbind(struct i915_vma *vma) trace_i915_vma_unbind(vma); - if (obj->has_global_gtt_mapping) - i915_gem_gtt_unbind_object(obj); - if (obj->has_aliasing_ppgtt_mapping) { - i915_ppgtt_unbind_object(dev_priv->mm.aliasing_ppgtt, obj); - obj->has_aliasing_ppgtt_mapping = 0; - } + vma->unbind_vma(vma); + i915_gem_gtt_finish_object(obj); list_del(&vma->mm_list); @@ -3479,7 +3475,6 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj, enum i915_cache_level cache_level) { struct drm_device *dev = obj->base.dev; - drm_i915_private_t *dev_priv = dev->dev_private; struct i915_vma *vma; int ret; @@ -3518,11 +3513,8 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj, return ret; } - if (obj->has_global_gtt_mapping) - i915_gem_gtt_bind_object(obj, cache_level); - if (obj->has_aliasing_ppgtt_mapping) - i915_ppgtt_bind_object(dev_priv->mm.aliasing_ppgtt, - obj, cache_level); + list_for_each_entry(vma, &obj->vma_list, vma_link) + vma->bind_vma(vma, cache_level, 0); } list_for_each_entry(vma, &obj->vma_list, vma_link) @@ -3850,6 +3842,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, bool map_and_fenceable, bool nonblocking) { + const u32 flags = map_and_fenceable ? GLOBAL_BIND : 0; struct i915_vma *vma; int ret; @@ -3878,20 +3871,17 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, } if (!i915_gem_obj_bound(obj, vm)) { - struct drm_i915_private *dev_priv = obj->base.dev->dev_private; - ret = i915_gem_object_bind_to_vm(obj, vm, alignment, map_and_fenceable, nonblocking); if (ret) return ret; - if (!dev_priv->mm.aliasing_ppgtt) - i915_gem_gtt_bind_object(obj, obj->cache_level); } - if (!obj->has_global_gtt_mapping && map_and_fenceable) - i915_gem_gtt_bind_object(obj, obj->cache_level); + vma = i915_gem_obj_to_vma(obj, vm); + + vma->bind_vma(vma, obj->cache_level, flags); i915_gem_obj_to_vma(obj, vm)->pin_count++; obj->pin_mappable |= map_and_fenceable; @@ -4235,41 +4225,6 @@ struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj, return NULL; } -static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, - struct i915_address_space *vm) -{ - struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL); - if (vma == NULL) - return ERR_PTR(-ENOMEM); - - INIT_LIST_HEAD(&vma->vma_link); - INIT_LIST_HEAD(&vma->mm_list); - INIT_LIST_HEAD(&vma->exec_list); - vma->vm = vm; - vma->obj = obj; - - /* Keep GGTT vmas first to make debug easier */ - if (i915_is_ggtt(vm)) - list_add(&vma->vma_link, &obj->vma_list); - else - list_add_tail(&vma->vma_link, &obj->vma_list); - - return vma; -} - -struct i915_vma * -i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, - struct i915_address_space *vm) -{ - struct i915_vma *vma; - - vma = i915_gem_obj_to_vma(obj, vm); - if (!vma) - vma = __i915_gem_vma_create(obj, vm); - - return vma; -} - void i915_gem_vma_destroy(struct i915_vma *vma) { WARN_ON(vma->node.allocated); diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index b06199175d41..0640ab8b9958 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -408,6 +408,7 @@ mi_set_context(struct intel_ring_buffer *ring, static int do_switch(struct i915_hw_context *to) { struct intel_ring_buffer *ring = to->ring; + struct drm_i915_private *dev_priv = ring->dev->dev_private; struct i915_hw_context *from = ring->last_context; u32 hw_flags = 0; int ret, i; @@ -432,8 +433,11 @@ static int do_switch(struct i915_hw_context *to) return ret; } - if (!to->obj->has_global_gtt_mapping) - i915_gem_gtt_bind_object(to->obj, to->obj->cache_level); + if (!to->obj->has_global_gtt_mapping) { + struct i915_vma *vma = i915_gem_obj_to_vma(to->obj, + &dev_priv->gtt.base); + vma->bind_vma(vma, to->obj->cache_level, GLOBAL_BIND); + } if (!to->is_initialized || is_default_context(to)) hw_flags |= MI_RESTORE_INHIBIT; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index a2d6eb5336e3..c093a2b6f06d 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -88,6 +88,7 @@ eb_lookup_vmas(struct eb_vmas *eb, struct i915_address_space *vm, struct drm_file *file) { + struct drm_i915_private *dev_priv = vm->dev->dev_private; struct drm_i915_gem_object *obj; struct list_head objects; int i, ret = 0; @@ -122,6 +123,15 @@ eb_lookup_vmas(struct eb_vmas *eb, i = 0; list_for_each_entry(obj, &objects, obj_exec_link) { struct i915_vma *vma; + struct i915_address_space *bind_vm = vm; + + /* If we have secure dispatch, or the userspace assures us that + * they know what they're doing, use the GGTT VM. + */ + if (exec[i].flags & EXEC_OBJECT_NEEDS_GTT || + ((args->flags & I915_EXEC_SECURE) && + (i == (args->buffer_count - 1)))) + bind_vm = &dev_priv->gtt.base; /* * NOTE: We can leak any vmas created here when something fails @@ -131,7 +141,7 @@ eb_lookup_vmas(struct eb_vmas *eb, * from the (obj, vm) we don't run the risk of creating * duplicated vmas for the same vm. */ - vma = i915_gem_obj_lookup_or_create_vma(obj, vm); + vma = i915_gem_obj_lookup_or_create_vma(obj, bind_vm); if (IS_ERR(vma)) { DRM_DEBUG("Failed to lookup VMA\n"); ret = PTR_ERR(vma); @@ -315,8 +325,8 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj, if (unlikely(IS_GEN6(dev) && reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION && !target_i915_obj->has_global_gtt_mapping)) { - i915_gem_gtt_bind_object(target_i915_obj, - target_i915_obj->cache_level); + struct i915_vma *vma = i915_gem_obj_to_vma(target_i915_obj, vm); + vma->bind_vma(vma, target_i915_obj->cache_level, GLOBAL_BIND); } /* Validate that the target is in a valid r/w GPU domain */ @@ -493,11 +503,12 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma, struct intel_ring_buffer *ring, bool *need_reloc) { - struct drm_i915_private *dev_priv = ring->dev->dev_private; + struct drm_i915_gem_object *obj = vma->obj; struct drm_i915_gem_exec_object2 *entry = vma->exec_entry; bool has_fenced_gpu_access = INTEL_INFO(ring->dev)->gen < 4; bool need_fence, need_mappable; - struct drm_i915_gem_object *obj = vma->obj; + u32 flags = (entry->flags & EXEC_OBJECT_NEEDS_GTT) && + !vma->obj->has_global_gtt_mapping ? GLOBAL_BIND : 0; int ret; need_fence = @@ -526,14 +537,6 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma, } } - /* Ensure ppgtt mapping exists if needed */ - if (dev_priv->mm.aliasing_ppgtt && !obj->has_aliasing_ppgtt_mapping) { - i915_ppgtt_bind_object(dev_priv->mm.aliasing_ppgtt, - obj, obj->cache_level); - - obj->has_aliasing_ppgtt_mapping = 1; - } - if (entry->offset != vma->node.start) { entry->offset = vma->node.start; *need_reloc = true; @@ -544,9 +547,7 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma, obj->base.pending_write_domain = I915_GEM_DOMAIN_RENDER; } - if (entry->flags & EXEC_OBJECT_NEEDS_GTT && - !obj->has_global_gtt_mapping) - i915_gem_gtt_bind_object(obj, obj->cache_level); + vma->bind_vma(vma, obj->cache_level, flags); return 0; } @@ -1171,8 +1172,14 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, /* snb/ivb/vlv conflate the "batch in ppgtt" bit with the "non-secure * batch" bit. Hence we need to pin secure batches into the global gtt. * hsw should have this fixed, but bdw mucks it up again. */ - if (flags & I915_DISPATCH_SECURE && !batch_obj->has_global_gtt_mapping) - i915_gem_gtt_bind_object(batch_obj, batch_obj->cache_level); + if (flags & I915_DISPATCH_SECURE && + !batch_obj->has_global_gtt_mapping) { + /* When we have multiple VMs, we'll need to make sure that we + * allocate space first */ + struct i915_vma *vma = i915_gem_obj_to_ggtt(batch_obj); + BUG_ON(!vma); + vma->bind_vma(vma, batch_obj->cache_level, GLOBAL_BIND); + } ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->vmas); if (ret) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 056033791d24..73117ec333cc 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -68,6 +68,11 @@ typedef gen8_gtt_pte_t gen8_ppgtt_pde_t; #define PPAT_CACHED_INDEX _PAGE_PAT /* WB LLCeLLC */ #define PPAT_DISPLAY_ELLC_INDEX _PAGE_PCD /* WT eLLC */ +static void ppgtt_bind_vma(struct i915_vma *vma, + enum i915_cache_level cache_level, + u32 flags); +static void ppgtt_unbind_vma(struct i915_vma *vma); + static inline gen8_gtt_pte_t gen8_pte_encode(dma_addr_t addr, enum i915_cache_level level, bool valid) @@ -746,22 +751,26 @@ void i915_gem_cleanup_aliasing_ppgtt(struct drm_device *dev) dev_priv->mm.aliasing_ppgtt = NULL; } -void i915_ppgtt_bind_object(struct i915_hw_ppgtt *ppgtt, - struct drm_i915_gem_object *obj, - enum i915_cache_level cache_level) +static void __always_unused +ppgtt_bind_vma(struct i915_vma *vma, + enum i915_cache_level cache_level, + u32 flags) { - ppgtt->base.insert_entries(&ppgtt->base, obj->pages, - i915_gem_obj_ggtt_offset(obj) >> PAGE_SHIFT, - cache_level); + const unsigned long entry = vma->node.start >> PAGE_SHIFT; + + WARN_ON(flags); + + vma->vm->insert_entries(vma->vm, vma->obj->pages, entry, cache_level); } -void i915_ppgtt_unbind_object(struct i915_hw_ppgtt *ppgtt, - struct drm_i915_gem_object *obj) +static void __always_unused ppgtt_unbind_vma(struct i915_vma *vma) { - ppgtt->base.clear_range(&ppgtt->base, - i915_gem_obj_ggtt_offset(obj) >> PAGE_SHIFT, - obj->base.size >> PAGE_SHIFT, - true); + const unsigned long entry = vma->node.start >> PAGE_SHIFT; + + vma->vm->clear_range(vma->vm, + entry, + vma->obj->base.size >> PAGE_SHIFT, + true); } extern int intel_iommu_gfx_mapped; @@ -863,8 +872,18 @@ void i915_gem_restore_gtt_mappings(struct drm_device *dev) true); list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) { + struct i915_vma *vma = i915_gem_obj_to_vma(obj, + &dev_priv->gtt.base); + if (!vma) + continue; + i915_gem_clflush_object(obj, obj->pin_display); - i915_gem_gtt_bind_object(obj, obj->cache_level); + /* The bind_vma code tries to be smart about tracking mappings. + * Unfortunately above, we've just wiped out the mappings + * without telling our object about it. So we need to fake it. + */ + obj->has_global_gtt_mapping = 0; + vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND); } i915_gem_chipset_flush(dev); @@ -1023,16 +1042,18 @@ static void gen6_ggtt_clear_range(struct i915_address_space *vm, readl(gtt_base); } -static void i915_ggtt_insert_entries(struct i915_address_space *vm, - struct sg_table *st, - unsigned int pg_start, - enum i915_cache_level cache_level) + +static void i915_ggtt_bind_vma(struct i915_vma *vma, + enum i915_cache_level cache_level, + u32 unused) { + const unsigned long entry = vma->node.start >> PAGE_SHIFT; unsigned int flags = (cache_level == I915_CACHE_NONE) ? AGP_USER_MEMORY : AGP_USER_CACHED_MEMORY; - intel_gtt_insert_sg_entries(st, pg_start, flags); - + BUG_ON(!i915_is_ggtt(vma->vm)); + intel_gtt_insert_sg_entries(vma->obj->pages, entry, flags); + vma->obj->has_global_gtt_mapping = 1; } static void i915_ggtt_clear_range(struct i915_address_space *vm, @@ -1043,33 +1064,77 @@ static void i915_ggtt_clear_range(struct i915_address_space *vm, intel_gtt_clear_range(first_entry, num_entries); } +static void i915_ggtt_unbind_vma(struct i915_vma *vma) +{ + const unsigned int first = vma->node.start >> PAGE_SHIFT; + const unsigned int size = vma->obj->base.size >> PAGE_SHIFT; -void i915_gem_gtt_bind_object(struct drm_i915_gem_object *obj, - enum i915_cache_level cache_level) + BUG_ON(!i915_is_ggtt(vma->vm)); + vma->obj->has_global_gtt_mapping = 0; + intel_gtt_clear_range(first, size); +} + +static void ggtt_bind_vma(struct i915_vma *vma, + enum i915_cache_level cache_level, + u32 flags) { - struct drm_device *dev = obj->base.dev; + struct drm_device *dev = vma->vm->dev; struct drm_i915_private *dev_priv = dev->dev_private; - const unsigned long entry = i915_gem_obj_ggtt_offset(obj) >> PAGE_SHIFT; + struct drm_i915_gem_object *obj = vma->obj; + const unsigned long entry = vma->node.start >> PAGE_SHIFT; - dev_priv->gtt.base.insert_entries(&dev_priv->gtt.base, obj->pages, - entry, - cache_level); + /* If there is no aliasing PPGTT, or the caller needs a global mapping, + * or we have a global mapping already but the cacheability flags have + * changed, set the global PTEs. + * + * If there is an aliasing PPGTT it is anecdotally faster, so use that + * instead if none of the above hold true. + * + * NB: A global mapping should only be needed for special regions like + * "gtt mappable", SNB errata, or if specified via special execbuf + * flags. At all other times, the GPU will use the aliasing PPGTT. + */ + if (!dev_priv->mm.aliasing_ppgtt || flags & GLOBAL_BIND) { + if (!obj->has_global_gtt_mapping || + (cache_level != obj->cache_level)) { + vma->vm->insert_entries(vma->vm, obj->pages, entry, + cache_level); + obj->has_global_gtt_mapping = 1; + } + } - obj->has_global_gtt_mapping = 1; + if (dev_priv->mm.aliasing_ppgtt && + (!obj->has_aliasing_ppgtt_mapping || + (cache_level != obj->cache_level))) { + struct i915_hw_ppgtt *appgtt = dev_priv->mm.aliasing_ppgtt; + appgtt->base.insert_entries(&appgtt->base, + vma->obj->pages, entry, cache_level); + vma->obj->has_aliasing_ppgtt_mapping = 1; + } } -void i915_gem_gtt_unbind_object(struct drm_i915_gem_object *obj) +static void ggtt_unbind_vma(struct i915_vma *vma) { - struct drm_device *dev = obj->base.dev; + struct drm_device *dev = vma->vm->dev; struct drm_i915_private *dev_priv = dev->dev_private; - const unsigned long entry = i915_gem_obj_ggtt_offset(obj) >> PAGE_SHIFT; - - dev_priv->gtt.base.clear_range(&dev_priv->gtt.base, - entry, - obj->base.size >> PAGE_SHIFT, - true); + struct drm_i915_gem_object *obj = vma->obj; + const unsigned long entry = vma->node.start >> PAGE_SHIFT; + + if (obj->has_global_gtt_mapping) { + vma->vm->clear_range(vma->vm, entry, + vma->obj->base.size >> PAGE_SHIFT, + true); + obj->has_global_gtt_mapping = 0; + } - obj->has_global_gtt_mapping = 0; + if (obj->has_aliasing_ppgtt_mapping) { + struct i915_hw_ppgtt *appgtt = dev_priv->mm.aliasing_ppgtt; + appgtt->base.clear_range(&appgtt->base, + entry, + obj->base.size >> PAGE_SHIFT, + true); + obj->has_aliasing_ppgtt_mapping = 0; + } } void i915_gem_gtt_finish_object(struct drm_i915_gem_object *obj) @@ -1444,7 +1509,6 @@ static int i915_gmch_probe(struct drm_device *dev, dev_priv->gtt.do_idle_maps = needs_idle_maps(dev_priv->dev); dev_priv->gtt.base.clear_range = i915_ggtt_clear_range; - dev_priv->gtt.base.insert_entries = i915_ggtt_insert_entries; return 0; } @@ -1496,3 +1560,57 @@ int i915_gem_gtt_init(struct drm_device *dev) return 0; } + +static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, + struct i915_address_space *vm) +{ + struct i915_vma *vma = kzalloc(sizeof(*vma), GFP_KERNEL); + if (vma == NULL) + return ERR_PTR(-ENOMEM); + + INIT_LIST_HEAD(&vma->vma_link); + INIT_LIST_HEAD(&vma->mm_list); + INIT_LIST_HEAD(&vma->exec_list); + vma->vm = vm; + vma->obj = obj; + + switch (INTEL_INFO(vm->dev)->gen) { + case 8: + case 7: + case 6: + vma->unbind_vma = ggtt_unbind_vma; + vma->bind_vma = ggtt_bind_vma; + break; + case 5: + case 4: + case 3: + case 2: + BUG_ON(!i915_is_ggtt(vm)); + vma->unbind_vma = i915_ggtt_unbind_vma; + vma->bind_vma = i915_ggtt_bind_vma; + break; + default: + BUG(); + } + + /* Keep GGTT vmas first to make debug easier */ + if (i915_is_ggtt(vm)) + list_add(&vma->vma_link, &obj->vma_list); + else + list_add_tail(&vma->vma_link, &obj->vma_list); + + return vma; +} + +struct i915_vma * +i915_gem_obj_lookup_or_create_vma(struct drm_i915_gem_object *obj, + struct i915_address_space *vm) +{ + struct i915_vma *vma; + + vma = i915_gem_obj_to_vma(obj, vm); + if (!vma) + vma = __i915_gem_vma_create(obj, vm); + + return vma; +} -- cgit From 3e7a032295f178d1db4e4b9ac25b6d6bc6d5826e Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Fri, 6 Dec 2013 14:10:57 -0800 Subject: drm/i915: Remove vm arg from relocate entry The only place we were using it was for GEN6, which won't have PPGTT support anyway (ie. the VM is always the same). To clear things up, (it only added confusion for me since it doesn't allow us to assert vma->vm is what we always want, when just looking at the code). Signed-off-by: Ben Widawsky Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index c093a2b6f06d..099998156e6c 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -300,8 +300,7 @@ relocate_entry_gtt(struct drm_i915_gem_object *obj, static int i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj, struct eb_vmas *eb, - struct drm_i915_gem_relocation_entry *reloc, - struct i915_address_space *vm) + struct drm_i915_gem_relocation_entry *reloc) { struct drm_device *dev = obj->base.dev; struct drm_gem_object *target_obj; @@ -325,7 +324,9 @@ i915_gem_execbuffer_relocate_entry(struct drm_i915_gem_object *obj, if (unlikely(IS_GEN6(dev) && reloc->write_domain == I915_GEM_DOMAIN_INSTRUCTION && !target_i915_obj->has_global_gtt_mapping)) { - struct i915_vma *vma = i915_gem_obj_to_vma(target_i915_obj, vm); + struct i915_vma *vma = + list_first_entry(&target_i915_obj->vma_list, + typeof(*vma), vma_link); vma->bind_vma(vma, target_i915_obj->cache_level, GLOBAL_BIND); } @@ -424,8 +425,7 @@ i915_gem_execbuffer_relocate_vma(struct i915_vma *vma, do { u64 offset = r->presumed_offset; - ret = i915_gem_execbuffer_relocate_entry(vma->obj, eb, r, - vma->vm); + ret = i915_gem_execbuffer_relocate_entry(vma->obj, eb, r); if (ret) return ret; @@ -454,8 +454,7 @@ i915_gem_execbuffer_relocate_vma_slow(struct i915_vma *vma, int i, ret; for (i = 0; i < entry->relocation_count; i++) { - ret = i915_gem_execbuffer_relocate_entry(vma->obj, eb, &relocs[i], - vma->vm); + ret = i915_gem_execbuffer_relocate_entry(vma->obj, eb, &relocs[i]); if (ret) return ret; } -- cgit From ca01b12b401a0e17a265a5ee18bf33e2cfbd32aa Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Fri, 6 Dec 2013 14:11:00 -0800 Subject: drm/i915: Simplify ring handling in execbuf Signed-off-by: Ben Widawsky Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 41 ++++++++---------------------- 1 file changed, 10 insertions(+), 31 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 099998156e6c..dfe7cb912e29 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1006,41 +1006,20 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, if (args->flags & I915_EXEC_IS_PINNED) flags |= I915_DISPATCH_PINNED; - switch (args->flags & I915_EXEC_RING_MASK) { - case I915_EXEC_DEFAULT: - case I915_EXEC_RENDER: - ring = &dev_priv->ring[RCS]; - break; - case I915_EXEC_BSD: - ring = &dev_priv->ring[VCS]; - if (ctx_id != DEFAULT_CONTEXT_ID) { - DRM_DEBUG("Ring %s doesn't support contexts\n", - ring->name); - return -EPERM; - } - break; - case I915_EXEC_BLT: - ring = &dev_priv->ring[BCS]; - if (ctx_id != DEFAULT_CONTEXT_ID) { - DRM_DEBUG("Ring %s doesn't support contexts\n", - ring->name); - return -EPERM; - } - break; - case I915_EXEC_VEBOX: - ring = &dev_priv->ring[VECS]; - if (ctx_id != DEFAULT_CONTEXT_ID) { - DRM_DEBUG("Ring %s doesn't support contexts\n", - ring->name); - return -EPERM; - } - break; - - default: + if ((args->flags & I915_EXEC_RING_MASK) > I915_NUM_RINGS) { DRM_DEBUG("execbuf with unknown ring: %d\n", (int)(args->flags & I915_EXEC_RING_MASK)); return -EINVAL; } + if (ctx_id != DEFAULT_CONTEXT_ID && + (args->flags & I915_EXEC_RING_MASK) > I915_EXEC_RENDER) + return -EPERM; + + if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_DEFAULT) + ring = &dev_priv->ring[RCS]; + else + ring = &dev_priv->ring[(args->flags & I915_EXEC_RING_MASK) - 1]; + if (!intel_ring_initialized(ring)) { DRM_DEBUG("execbuf with invalid ring: %d\n", (int)(args->flags & I915_EXEC_RING_MASK)); -- cgit From 67e3d2979be1bf42d1818b2961c671eb31e0b4d9 Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Fri, 6 Dec 2013 14:11:01 -0800 Subject: drm/i915: Permit contexts on all rings If we want to use contexts in more abstract terms (specifically with PPGTT in mind), we need to allow them to be specified for any ring. Since the upcoming patches will bring about the use of multiple address spaces, and each ring needs to have an address space programmed (which we intend to do at context switch time), we can no longer only use RCS. With multiple rings having a last context, we must now unreference these contexts. NOTE: This commit requires an update to intel-gpu-tools to make it not fail. v2: Rebased with some logical conflicts. Squashed in the context fini refcount patch Signed-off-by: Ben Widawsky Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_context.c | 54 +++++++++++++++++++++++------- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 -- 2 files changed, 42 insertions(+), 15 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 40413704614e..7a5311c3225a 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -98,7 +98,8 @@ static struct i915_hw_context * i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id); -static int do_switch(struct i915_hw_context *to); +static int do_switch(struct intel_ring_buffer *ring, + struct i915_hw_context *to); static size_t get_context_alignment(struct drm_device *dev) { @@ -240,7 +241,7 @@ static int create_default_context(struct drm_device *dev) goto err_destroy; } - ret = do_switch(ctx); + ret = do_switch(&dev_priv->ring[RCS], ctx); if (ret) { DRM_DEBUG_DRIVER("Switch failed %d\n", ret); goto err_unpin; @@ -261,7 +262,8 @@ err_destroy: int i915_gem_context_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - int ret; + struct intel_ring_buffer *ring; + int i, ret; if (!HAS_HW_CONTEXTS(dev)) return 0; @@ -284,6 +286,16 @@ int i915_gem_context_init(struct drm_device *dev) return ret; } + for (i = RCS + 1; i < I915_NUM_RINGS; i++) { + if (!(INTEL_INFO(dev)->ring_mask & (1<ring[i]; + + /* NB: RCS will hold a ref for all rings */ + ring->default_context = dev_priv->ring[RCS].default_context; + } + DRM_DEBUG_DRIVER("HW context support initialized\n"); return 0; } @@ -292,6 +304,7 @@ void i915_gem_context_fini(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; struct i915_hw_context *dctx = dev_priv->ring[RCS].default_context; + int i; if (!HAS_HW_CONTEXTS(dev)) return; @@ -313,12 +326,22 @@ void i915_gem_context_fini(struct drm_device *dev) WARN_ON(dctx->obj->active); i915_gem_object_ggtt_unpin(dctx->obj); i915_gem_context_unreference(dctx); + dev_priv->ring[RCS].last_context = NULL; + } + + for (i = 0; i < I915_NUM_RINGS; i++) { + struct intel_ring_buffer *ring = &dev_priv->ring[i]; + if (!(INTEL_INFO(dev)->ring_mask & (1<last_context) + i915_gem_context_unreference(ring->last_context); + + ring->default_context = NULL; } i915_gem_object_ggtt_unpin(dctx->obj); i915_gem_context_unreference(dctx); - dev_priv->ring[RCS].default_context = NULL; - dev_priv->ring[RCS].last_context = NULL; } static int context_idr_cleanup(int id, void *p, void *data) @@ -431,19 +454,28 @@ mi_set_context(struct intel_ring_buffer *ring, return ret; } -static int do_switch(struct i915_hw_context *to) +static int do_switch(struct intel_ring_buffer *ring, + struct i915_hw_context *to) { - struct intel_ring_buffer *ring = to->ring; struct drm_i915_private *dev_priv = ring->dev->dev_private; struct i915_hw_context *from = ring->last_context; u32 hw_flags = 0; int ret, i; - BUG_ON(from != NULL && from->obj != NULL && !i915_gem_obj_is_pinned(from->obj)); + if (from != NULL && ring == &dev_priv->ring[RCS]) { + BUG_ON(from->obj == NULL); + BUG_ON(!i915_gem_obj_is_pinned(from->obj)); + } if (from == to && !to->remap_slice) return 0; + if (ring != &dev_priv->ring[RCS]) { + if (from) + i915_gem_context_unreference(from); + goto done; + } + ret = i915_gem_obj_ggtt_pin(to->obj, get_context_alignment(ring->dev), false, false); if (ret) @@ -511,6 +543,7 @@ static int do_switch(struct i915_hw_context *to) i915_gem_context_unreference(from); } +done: i915_gem_context_reference(to); ring->last_context = to; to->is_initialized = true; @@ -541,9 +574,6 @@ int i915_switch_context(struct intel_ring_buffer *ring, WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex)); - if (ring != &dev_priv->ring[RCS]) - return 0; - if (to_id == DEFAULT_CONTEXT_ID) { to = ring->default_context; } else { @@ -555,7 +585,7 @@ int i915_switch_context(struct intel_ring_buffer *ring, return -ENOENT; } - return do_switch(to); + return do_switch(ring, to); } int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index dfe7cb912e29..d608a07f4398 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1011,9 +1011,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, (int)(args->flags & I915_EXEC_RING_MASK)); return -EINVAL; } - if (ctx_id != DEFAULT_CONTEXT_ID && - (args->flags & I915_EXEC_RING_MASK) > I915_EXEC_RENDER) - return -EPERM; if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_DEFAULT) ring = &dev_priv->ring[RCS]; -- cgit From 41bde5535a7d48876095926bb55b1aed5ccd6b2c Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Fri, 6 Dec 2013 14:11:21 -0800 Subject: drm/i915: Get context early in execbuf We need to have the address space when reserving space for the objects. Since the address space and context are tied together, and reserve occurs before context switch (for good reason), we must lookup our context earlier in the process. This leaves some room for optimizations where we no longer need to use ctx_id in certain places. This will be addressed in a subsequent patch. Important tricky bit: Because slow relocations during execbuffer drop struct_mutex Perhaps it would be best to acquire the reference when we get the context, but I'll save that for another day (note I have written the patch before, and I found the changes required to be uglier than this). Note that since we currently access everything via context id, and not the data structure this is fine, though not desirable. The next change attempts to get the context only once via the context ID idr lookup, and as such, the following can happen: CTX-A is created, refcount = 1 CTX-A execbuf, mutex dropped close IOCTL called on CTX-A, refcount = 0 CTX-A resumes in execbuf. v2: Rebased on top of commit b6359918b885da7c7b58c050674278dbd06020ab Author: Mika Kuoppala Date: Wed Oct 30 15:44:16 2013 +0200 drm/i915: add i915_get_reset_stats_ioctl v3: Rebased on top of commit 25b3dfc87bff80317d67ddd2cd4cfb91e6fe7d79 Author: Mika Westerberg Date: Tue Nov 12 11:57:30 2013 +0200 Author: Mika Kuoppala Date: Tue Nov 26 16:14:33 2013 +0200 drm/i915: check context reset stats before relocations Signed-off-by: Ben Widawsky Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 8 ++---- drivers/gpu/drm/i915/i915_gem.c | 2 +- drivers/gpu/drm/i915/i915_gem_context.c | 32 ++++------------------ drivers/gpu/drm/i915/i915_gem_execbuffer.c | 44 ++++++++++++++++++------------ drivers/gpu/drm/i915/intel_uncore.c | 8 ++++-- 5 files changed, 41 insertions(+), 53 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index c1adb8268025..4f0b17b617c6 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2239,7 +2239,9 @@ int i915_gem_context_open(struct drm_device *dev, struct drm_file *file); int i915_gem_context_enable(struct drm_i915_private *dev_priv); void i915_gem_context_close(struct drm_device *dev, struct drm_file *file); int i915_switch_context(struct intel_ring_buffer *ring, - struct drm_file *file, int to_id); + struct drm_file *file, struct i915_hw_context *to); +struct i915_hw_context * +i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id); void i915_gem_context_free(struct kref *ctx_ref); static inline void i915_gem_context_reference(struct i915_hw_context *ctx) { @@ -2253,10 +2255,6 @@ static inline void i915_gem_context_unreference(struct i915_hw_context *ctx) kref_put(&ctx->ref, i915_gem_context_free); } -struct i915_ctx_hang_stats * __must_check -i915_gem_context_get_hang_stats(struct drm_device *dev, - struct drm_file *file, - u32 id); int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, struct drm_file *file); int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 42647c39ddcc..89e2f92e8335 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2799,7 +2799,7 @@ int i915_gpu_idle(struct drm_device *dev) /* Flush everything onto the inactive list. */ for_each_ring(ring, dev_priv, i) { - ret = i915_switch_context(ring, NULL, DEFAULT_CONTEXT_ID); + ret = i915_switch_context(ring, NULL, ring->default_context); if (ret) return ret; diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 192a25978d39..d3a17ef78ba6 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -96,8 +96,6 @@ #define GEN6_CONTEXT_ALIGN (64<<10) #define GEN7_CONTEXT_ALIGN 4096 -static struct i915_hw_context * -i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id); static int do_switch(struct intel_ring_buffer *ring, struct i915_hw_context *to); @@ -485,20 +483,6 @@ static int context_idr_cleanup(int id, void *p, void *data) return 0; } -struct i915_ctx_hang_stats * -i915_gem_context_get_hang_stats(struct drm_device *dev, - struct drm_file *file, - u32 id) -{ - struct i915_hw_context *ctx; - - ctx = i915_gem_context_get(file->driver_priv, id); - if (ctx == NULL) - return ERR_PTR(-ENOENT); - - return &ctx->hang_stats; -} - int i915_gem_context_open(struct drm_device *dev, struct drm_file *file) { struct drm_i915_file_private *file_priv = file->driver_priv; @@ -543,9 +527,12 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file) mutex_unlock(&dev->struct_mutex); } -static struct i915_hw_context * +struct i915_hw_context * i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id) { + if (!HAS_HW_CONTEXTS(file_priv->dev_priv->dev)) + return file_priv->private_default_ctx; + return (struct i915_hw_context *)idr_find(&file_priv->context_idr, id); } @@ -708,20 +695,13 @@ done: */ int i915_switch_context(struct intel_ring_buffer *ring, struct drm_file *file, - int to_id) + struct i915_hw_context *to) { struct drm_i915_private *dev_priv = ring->dev->dev_private; - struct i915_hw_context *to; WARN_ON(!mutex_is_locked(&dev_priv->dev->struct_mutex)); - if (file == NULL) - to = ring->default_context; - else - to = i915_gem_context_get(file->driver_priv, to_id); - - if (to == NULL) - return -ENOENT; + BUG_ON(file && to == NULL); /* We have the fake context, but don't supports switching. */ if (!HAS_HW_CONTEXTS(ring->dev)) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index d608a07f4398..e78c5c0e33d8 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -884,22 +884,24 @@ validate_exec_list(struct drm_i915_gem_exec_object2 *exec, return 0; } -static int +static struct i915_hw_context * i915_gem_validate_context(struct drm_device *dev, struct drm_file *file, const u32 ctx_id) { + struct i915_hw_context *ctx = NULL; struct i915_ctx_hang_stats *hs; - hs = i915_gem_context_get_hang_stats(dev, file, ctx_id); - if (IS_ERR(hs)) - return PTR_ERR(hs); + ctx = i915_gem_context_get(file->driver_priv, ctx_id); + if (IS_ERR_OR_NULL(ctx)) + return ctx; + hs = &ctx->hang_stats; if (hs->banned) { DRM_DEBUG("Context %u tried to submit while banned\n", ctx_id); - return -EIO; + return ERR_PTR(-EIO); } - return 0; + return ctx; } static void @@ -975,14 +977,15 @@ static int i915_gem_do_execbuffer(struct drm_device *dev, void *data, struct drm_file *file, struct drm_i915_gem_execbuffer2 *args, - struct drm_i915_gem_exec_object2 *exec, - struct i915_address_space *vm) + struct drm_i915_gem_exec_object2 *exec) { drm_i915_private_t *dev_priv = dev->dev_private; struct eb_vmas *eb; struct drm_i915_gem_object *batch_obj; struct drm_clip_rect *cliprects = NULL; struct intel_ring_buffer *ring; + struct i915_hw_context *ctx; + struct i915_address_space *vm; const u32 ctx_id = i915_execbuffer2_get_context_id(*args); u32 exec_start, exec_len; u32 mask, flags; @@ -1096,11 +1099,18 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, goto pre_mutex_err; } - ret = i915_gem_validate_context(dev, file, ctx_id); - if (ret) { + ctx = i915_gem_validate_context(dev, file, ctx_id); + if (IS_ERR_OR_NULL(ctx)) { mutex_unlock(&dev->struct_mutex); + ret = PTR_ERR(ctx); goto pre_mutex_err; - } + } + + i915_gem_context_reference(ctx); + + /* HACK until we have full PPGTT */ + /* vm = ctx->vm; */ + vm = &dev_priv->gtt.base; eb = eb_create(args); if (eb == NULL) { @@ -1160,7 +1170,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, if (ret) goto err; - ret = i915_switch_context(ring, file, ctx_id); + ret = i915_switch_context(ring, file, ctx); if (ret) goto err; @@ -1215,6 +1225,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, i915_gem_execbuffer_retire_commands(dev, file, ring, batch_obj); err: + /* the request owns the ref now */ + i915_gem_context_unreference(ctx); eb_destroy(eb); mutex_unlock(&dev->struct_mutex); @@ -1232,7 +1244,6 @@ int i915_gem_execbuffer(struct drm_device *dev, void *data, struct drm_file *file) { - struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_execbuffer *args = data; struct drm_i915_gem_execbuffer2 exec2; struct drm_i915_gem_exec_object *exec_list = NULL; @@ -1288,8 +1299,7 @@ i915_gem_execbuffer(struct drm_device *dev, void *data, exec2.flags = I915_EXEC_RENDER; i915_execbuffer2_set_context_id(exec2, 0); - ret = i915_gem_do_execbuffer(dev, data, file, &exec2, exec2_list, - &dev_priv->gtt.base); + ret = i915_gem_do_execbuffer(dev, data, file, &exec2, exec2_list); if (!ret) { /* Copy the new buffer offsets back to the user's exec list. */ for (i = 0; i < args->buffer_count; i++) @@ -1315,7 +1325,6 @@ int i915_gem_execbuffer2(struct drm_device *dev, void *data, struct drm_file *file) { - struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_execbuffer2 *args = data; struct drm_i915_gem_exec_object2 *exec2_list = NULL; int ret; @@ -1346,8 +1355,7 @@ i915_gem_execbuffer2(struct drm_device *dev, void *data, return -EFAULT; } - ret = i915_gem_do_execbuffer(dev, data, file, args, exec2_list, - &dev_priv->gtt.base); + ret = i915_gem_do_execbuffer(dev, data, file, args, exec2_list); if (!ret) { /* Copy the new buffer offsets back to the user's exec list. */ ret = copy_to_user(to_user_ptr(args->buffers_ptr), diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index feb2d6692544..e52fccecf28f 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -836,6 +836,7 @@ int i915_get_reset_stats_ioctl(struct drm_device *dev, struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_reset_stats *args = data; struct i915_ctx_hang_stats *hs; + struct i915_hw_context *ctx; int ret; if (args->flags || args->pad) @@ -848,11 +849,12 @@ int i915_get_reset_stats_ioctl(struct drm_device *dev, if (ret) return ret; - hs = i915_gem_context_get_hang_stats(dev, file, args->ctx_id); - if (IS_ERR(hs)) { + ctx = i915_gem_context_get(file->driver_priv, args->ctx_id); + if (IS_ERR(ctx)) { mutex_unlock(&dev->struct_mutex); - return PTR_ERR(hs); + return PTR_ERR(ctx); } + hs = &ctx->hang_stats; if (capable(CAP_SYS_ADMIN)) args->reset_count = i915_reset_count(&dev_priv->gpu_error); -- cgit From 7e0d96bc03c140cb8183955ad6f0290caa731e64 Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Fri, 6 Dec 2013 14:11:26 -0800 Subject: drm/i915: Use multiple VMs -- the point of no return As with processes which run on the CPU, the goal of multiple VMs is to provide process isolation. Specific to GEN, there is also the ability to map more objects per process (2GB each instead of 2Gb-2k total). For the most part, all the pipes have been laid, and all we need to do is remove asserts and actually start changing address spaces with the context switch. Since prior to this we've converted the setting of the page tables to a streamed version, this is quite easy. One important thing to point out (since it'd been hotly contested) is that with this patch, every context created will have it's own address space (provided the HW can do it). v2: Disable BDW on rebase NOTE: I tried to make this commit as small as possible. I needed one place where I could "turn everything on" and that is here. It could be split into finer commits, but I didn't really see much point. Cc: Eric Anholt Cc: Daniel Vetter Signed-off-by: Ben Widawsky Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_dma.c | 3 ++ drivers/gpu/drm/i915/i915_drv.c | 3 +- drivers/gpu/drm/i915/i915_drv.h | 12 +++++- drivers/gpu/drm/i915/i915_gem.c | 22 ++++------- drivers/gpu/drm/i915/i915_gem_context.c | 60 +++++++++++++++++------------- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 16 +++++--- drivers/gpu/drm/i915/i915_gem_gtt.c | 22 ++++++++--- drivers/gpu/drm/i915/i915_gpu_error.c | 5 --- include/uapi/drm/i915_drm.h | 1 + 9 files changed, 86 insertions(+), 58 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 89af75a6d9a9..9fedfa04357d 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1011,6 +1011,9 @@ static int i915_getparam(struct drm_device *dev, void *data, case I915_PARAM_HAS_EXEC_HANDLE_LUT: value = 1; break; + case I915_PARAM_HAS_FULL_PPGTT: + value = USES_FULL_PPGTT(dev); + break; default: DRM_DEBUG("Unknown parameter %d\n", param->param); return -EINVAL; diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 65b5c83df3bb..6cdaa78f3a63 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -116,7 +116,8 @@ MODULE_PARM_DESC(enable_hangcheck, int i915_enable_ppgtt __read_mostly = -1; module_param_named(i915_enable_ppgtt, i915_enable_ppgtt, int, 0400); MODULE_PARM_DESC(i915_enable_ppgtt, - "Enable PPGTT (default: true)"); + "Override PPGTT usage. " + "(-1=auto [default], 0=disabled, 1=aliasing, 2=full)"); int i915_enable_psr __read_mostly = 0; module_param_named(enable_psr, i915_enable_psr, int, 0600); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index a70d9c8b5277..8fd99ac96940 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1831,7 +1831,9 @@ struct drm_i915_file_private { #define HAS_HW_CONTEXTS(dev) (INTEL_INFO(dev)->gen >= 6) #define HAS_ALIASING_PPGTT(dev) (INTEL_INFO(dev)->gen >= 6 && !IS_VALLEYVIEW(dev)) +#define HAS_PPGTT(dev) (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev) && !IS_BROADWELL(dev)) #define USES_ALIASING_PPGTT(dev) intel_enable_ppgtt(dev, false) +#define USES_FULL_PPGTT(dev) intel_enable_ppgtt(dev, true) #define HAS_OVERLAY(dev) (INTEL_INFO(dev)->has_overlay) #define OVERLAY_NEEDS_PHYSICAL(dev) (INTEL_INFO(dev)->overlay_needs_physical) @@ -2012,6 +2014,8 @@ void i915_gem_object_init(struct drm_i915_gem_object *obj, const struct drm_i915_gem_object_ops *ops); struct drm_i915_gem_object *i915_gem_alloc_object(struct drm_device *dev, size_t size); +void i915_init_vm(struct drm_i915_private *dev_priv, + struct i915_address_space *vm); void i915_gem_free_object(struct drm_gem_object *obj); void i915_gem_vma_destroy(struct i915_vma *vma); @@ -2290,7 +2294,8 @@ static inline bool intel_enable_ppgtt(struct drm_device *dev, bool full) if (i915_enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev)) return false; - BUG_ON(full); + if (i915_enable_ppgtt == 1 && full) + return false; #ifdef CONFIG_INTEL_IOMMU /* Disable ppgtt on SNB if VT-d is on. */ @@ -2300,7 +2305,10 @@ static inline bool intel_enable_ppgtt(struct drm_device *dev, bool full) } #endif - return HAS_ALIASING_PPGTT(dev); + if (full) + return HAS_PPGTT(dev); + else + return HAS_ALIASING_PPGTT(dev); } static inline void ppgtt_release(struct kref *kref) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index a9cabff5aa37..f3b0025998db 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2247,7 +2247,10 @@ request_to_vm(struct drm_i915_gem_request *request) struct drm_i915_private *dev_priv = request->ring->dev->dev_private; struct i915_address_space *vm; - vm = &dev_priv->gtt.base; + if (request->ctx) + vm = request->ctx->vm; + else + vm = &dev_priv->gtt.base; return vm; } @@ -2718,9 +2721,6 @@ int i915_vma_unbind(struct i915_vma *vma) drm_i915_private_t *dev_priv = obj->base.dev->dev_private; int ret; - /* For now we only ever use 1 vma per object */ - WARN_ON(!list_is_singular(&obj->vma_list)); - if (list_empty(&vma->vma_link)) return 0; @@ -3268,17 +3268,12 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, i915_gem_object_pin_pages(obj); - BUG_ON(!i915_is_ggtt(vm)); - vma = i915_gem_obj_lookup_or_create_vma(obj, vm); if (IS_ERR(vma)) { ret = PTR_ERR(vma); goto err_unpin; } - /* For now we only ever use 1 vma per object */ - WARN_ON(!list_is_singular(&obj->vma_list)); - search_free: /* FIXME: Some tests are failing when they receive a reloc of 0. To * prevent this, we simply don't allow the 0th offset. */ @@ -4182,9 +4177,6 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj) if (obj->phys_obj) i915_gem_detach_phys_object(dev, obj); - /* NB: 0 or 1 elements */ - WARN_ON(!list_empty(&obj->vma_list) && - !list_is_singular(&obj->vma_list)); list_for_each_entry_safe(vma, next, &obj->vma_list, vma_link) { int ret; @@ -4580,9 +4572,11 @@ init_ring_lists(struct intel_ring_buffer *ring) INIT_LIST_HEAD(&ring->request_list); } -static void i915_init_vm(struct drm_i915_private *dev_priv, - struct i915_address_space *vm) +void i915_init_vm(struct drm_i915_private *dev_priv, + struct i915_address_space *vm) { + if (!i915_is_ggtt(vm)) + drm_mm_init(&vm->mm, vm->start, vm->total); vm->dev = dev_priv->dev; INIT_LIST_HEAD(&vm->active_list); INIT_LIST_HEAD(&vm->inactive_list); diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 165a5c7d9424..ebe0f67eac08 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -288,17 +288,15 @@ i915_gem_create_context(struct drm_device *dev, DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret); goto err_destroy; } + + ctx->vm = &dev_priv->mm.aliasing_ppgtt->base; } } else if (USES_ALIASING_PPGTT(dev)) { /* For platforms which only have aliasing PPGTT, we fake the * address space and refcounting. */ - kref_get(&dev_priv->mm.aliasing_ppgtt->ref); - } - - /* TODO: Until full ppgtt... */ - if (USES_ALIASING_PPGTT(dev)) ctx->vm = &dev_priv->mm.aliasing_ppgtt->base; - else + kref_get(&dev_priv->mm.aliasing_ppgtt->ref); + } else ctx->vm = &dev_priv->gtt.base; return ctx; @@ -500,7 +498,7 @@ int i915_gem_context_open(struct drm_device *dev, struct drm_file *file) mutex_lock(&dev->struct_mutex); file_priv->private_default_ctx = - i915_gem_create_context(dev, file_priv, false); + i915_gem_create_context(dev, file_priv, USES_FULL_PPGTT(dev)); mutex_unlock(&dev->struct_mutex); if (IS_ERR(file_priv->private_default_ctx)) { @@ -587,6 +585,7 @@ static int do_switch(struct intel_ring_buffer *ring, { struct drm_i915_private *dev_priv = ring->dev->dev_private; struct i915_hw_context *from = ring->last_context; + struct i915_hw_ppgtt *ppgtt = ctx_to_ppgtt(to); u32 hw_flags = 0; int ret, i; @@ -598,17 +597,15 @@ static int do_switch(struct intel_ring_buffer *ring, if (from == to && from->last_ring == ring && !to->remap_slice) return 0; - if (ring != &dev_priv->ring[RCS]) { - if (from) - i915_gem_context_unreference(from); - goto done; + /* Trying to pin first makes error handling easier. */ + if (ring == &dev_priv->ring[RCS]) { + ret = i915_gem_obj_ggtt_pin(to->obj, + get_context_alignment(ring->dev), + false, false); + if (ret) + return ret; } - ret = i915_gem_obj_ggtt_pin(to->obj, get_context_alignment(ring->dev), - false, false); - if (ret) - return ret; - /* * Pin can switch back to the default context if we end up calling into * evict_everything - as a last ditch gtt defrag effort that also @@ -616,6 +613,18 @@ static int do_switch(struct intel_ring_buffer *ring, */ from = ring->last_context; + if (USES_FULL_PPGTT(ring->dev)) { + ret = ppgtt->switch_mm(ppgtt, ring, false); + if (ret) + goto unpin_out; + } + + if (ring != &dev_priv->ring[RCS]) { + if (from) + i915_gem_context_unreference(from); + goto done; + } + /* * Clear this page out of any CPU caches for coherent swap-in/out. Note * that thanks to write = false in this call and us not setting any gpu @@ -625,10 +634,8 @@ static int do_switch(struct intel_ring_buffer *ring, * XXX: We need a real interface to do this instead of trickery. */ ret = i915_gem_object_set_to_gtt_domain(to->obj, false); - if (ret) { - i915_gem_object_ggtt_unpin(to->obj); - return ret; - } + if (ret) + goto unpin_out; if (!to->obj->has_global_gtt_mapping) { struct i915_vma *vma = i915_gem_obj_to_vma(to->obj, @@ -640,10 +647,8 @@ static int do_switch(struct intel_ring_buffer *ring, hw_flags |= MI_RESTORE_INHIBIT; ret = mi_set_context(ring, to, hw_flags); - if (ret) { - i915_gem_object_ggtt_unpin(to->obj); - return ret; - } + if (ret) + goto unpin_out; for (i = 0; i < MAX_L3_SLICES; i++) { if (!(to->remap_slice & (1<last_ring = ring; return 0; + +unpin_out: + if (ring->id == RCS) + i915_gem_object_ggtt_unpin(to->obj); + return ret; } /** @@ -736,7 +746,7 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data, if (ret) return ret; - ctx = i915_gem_create_context(dev, file_priv, false); + ctx = i915_gem_create_context(dev, file_priv, USES_FULL_PPGTT(dev)); mutex_unlock(&dev->struct_mutex); if (IS_ERR(ctx)) return PTR_ERR(ctx); diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 8779d75bee1f..2e80f8c6d123 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -991,7 +991,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, struct i915_hw_context *ctx; struct i915_address_space *vm; const u32 ctx_id = i915_execbuffer2_get_context_id(*args); - u32 exec_start, exec_len; + u32 exec_start = args->batch_start_offset, exec_len; u32 mask, flags; int ret, mode, i; bool need_relocs; @@ -1112,9 +1112,9 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, i915_gem_context_reference(ctx); - /* HACK until we have full PPGTT */ - /* vm = ctx->vm; */ - vm = &dev_priv->gtt.base; + vm = ctx->vm; + if (!USES_FULL_PPGTT(dev)) + vm = &dev_priv->gtt.base; eb = eb_create(args); if (eb == NULL) { @@ -1170,6 +1170,11 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, vma->bind_vma(vma, batch_obj->cache_level, GLOBAL_BIND); } + if (flags & I915_DISPATCH_SECURE) + exec_start += i915_gem_obj_ggtt_offset(batch_obj); + else + exec_start += i915_gem_obj_offset(batch_obj, vm); + ret = i915_gem_execbuffer_move_to_gpu(ring, &eb->vmas); if (ret) goto err; @@ -1199,8 +1204,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, goto err; } - exec_start = i915_gem_obj_offset(batch_obj, vm) + - args->batch_start_offset; + exec_len = args->batch_len; if (cliprects) { for (i = 0; i < args->num_cliprects; i++) { diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 88e49b19baba..4143efd9c02b 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -324,6 +324,7 @@ static void gen8_ppgtt_cleanup(struct i915_address_space *vm) container_of(vm, struct i915_hw_ppgtt, base); int i, j; + list_del(&vm->global_link); drm_mm_takedown(&vm->mm); for (i = 0; i < ppgtt->num_pd_pages ; i++) { @@ -755,6 +756,7 @@ static void gen6_ppgtt_cleanup(struct i915_address_space *vm) container_of(vm, struct i915_hw_ppgtt, base); int i; + list_del(&vm->global_link); drm_mm_takedown(&ppgtt->base.mm); drm_mm_remove_node(&ppgtt->node); @@ -901,17 +903,22 @@ int i915_gem_init_ppgtt(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt) BUG(); if (!ret) { + struct drm_i915_private *dev_priv = dev->dev_private; kref_init(&ppgtt->ref); drm_mm_init(&ppgtt->base.mm, ppgtt->base.start, ppgtt->base.total); - if (INTEL_INFO(dev)->gen < 8) + i915_init_vm(dev_priv, &ppgtt->base); + if (INTEL_INFO(dev)->gen < 8) { gen6_write_pdes(ppgtt); + DRM_DEBUG("Adding PPGTT at offset %x\n", + ppgtt->pd_offset << 10); + } } return ret; } -static void __always_unused +static void ppgtt_bind_vma(struct i915_vma *vma, enum i915_cache_level cache_level, u32 flags) @@ -923,7 +930,7 @@ ppgtt_bind_vma(struct i915_vma *vma, vma->vm->insert_entries(vma->vm, vma->obj->pages, entry, cache_level); } -static void __always_unused ppgtt_unbind_vma(struct i915_vma *vma) +static void ppgtt_unbind_vma(struct i915_vma *vma) { const unsigned long entry = vma->node.start >> PAGE_SHIFT; @@ -1719,8 +1726,13 @@ static struct i915_vma *__i915_gem_vma_create(struct drm_i915_gem_object *obj, case 8: case 7: case 6: - vma->unbind_vma = ggtt_unbind_vma; - vma->bind_vma = ggtt_bind_vma; + if (i915_is_ggtt(vm)) { + vma->unbind_vma = ggtt_unbind_vma; + vma->bind_vma = ggtt_bind_vma; + } else { + vma->unbind_vma = ppgtt_unbind_vma; + vma->bind_vma = ppgtt_bind_vma; + } break; case 5: case 4: diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 5dede92396c8..80773eca7311 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -909,11 +909,6 @@ static void i915_gem_capture_buffers(struct drm_i915_private *dev_priv, list_for_each_entry(vm, &dev_priv->vm_list, global_link) cnt++; - if (WARN(cnt > 1, "Multiple VMs not yet supported\n")) - cnt = 1; - - vm = &dev_priv->gtt.base; - error->active_bo = kcalloc(cnt, sizeof(*error->active_bo), GFP_ATOMIC); error->pinned_bo = kcalloc(cnt, sizeof(*error->pinned_bo), GFP_ATOMIC); error->active_bo_count = kcalloc(cnt, sizeof(*error->active_bo_count), diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h index 52aed893710a..d5b52846ae8f 100644 --- a/include/uapi/drm/i915_drm.h +++ b/include/uapi/drm/i915_drm.h @@ -337,6 +337,7 @@ typedef struct drm_i915_irq_wait { #define I915_PARAM_HAS_EXEC_NO_RELOC 25 #define I915_PARAM_HAS_EXEC_HANDLE_LUT 26 #define I915_PARAM_HAS_WT 27 +#define I915_PARAM_HAS_FULL_PPGTT 28 typedef struct drm_i915_getparam { int param; -- cgit From 7c9c4b8f5dfe224ce587a470ce8817214c92271e Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 18 Dec 2013 16:37:49 +0100 Subject: drm/i915: Reject non-default contexts on non-render again This reverts the abi-change from commit 67e3d2979be1bf42d1818b2961c671eb31e0b4d9 Author: Ben Widawsky Date: Fri Dec 6 14:11:01 2013 -0800 drm/i915: Permit contexts on all rings We don't actually need this, only the internal changes to allow contexts on all rings for the purpose of ppgtt switching are required. And I'm not sure whether this is the right thing to do given some of the hw features in the pipeline. Also, new abi needs userspace patches as a proof-of-need, which is completely lacking here. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 2e80f8c6d123..f5a1e0c34552 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -890,11 +890,14 @@ validate_exec_list(struct drm_i915_gem_exec_object2 *exec, static struct i915_hw_context * i915_gem_validate_context(struct drm_device *dev, struct drm_file *file, - const u32 ctx_id) + struct intel_ring_buffer *ring, const u32 ctx_id) { struct i915_hw_context *ctx = NULL; struct i915_ctx_hang_stats *hs; + if (ring->id != RCS && ctx_id != DEFAULT_CONTEXT_ID) + return ERR_PTR(-EINVAL); + ctx = i915_gem_context_get(file->driver_priv, ctx_id); if (IS_ERR_OR_NULL(ctx)) return ctx; @@ -1103,7 +1106,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, goto pre_mutex_err; } - ctx = i915_gem_validate_context(dev, file, ctx_id); + ctx = i915_gem_validate_context(dev, file, ring, ctx_id); if (IS_ERR_OR_NULL(ctx)) { mutex_unlock(&dev->struct_mutex); ret = PTR_ERR(ctx); -- cgit From 2c9f8d56a1ccc9064180a95cf22531c4b37154be Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 18 Dec 2013 17:38:53 +0100 Subject: drm/i915: Reject NEEDS_GTT relocations with full ppgtt Doesn't make sense. Spotted while fixing an issue Chris noticed in the same area. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index f5a1e0c34552..277485505bca 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -128,6 +128,12 @@ eb_lookup_vmas(struct eb_vmas *eb, struct i915_vma *vma; struct i915_address_space *bind_vm = vm; + if (exec[i].flags & EXEC_OBJECT_NEEDS_GTT && + USES_FULL_PPGTT(vm->dev)) { + ret = -EINVAL; + goto out; + } + /* If we have secure dispatch, or the userspace assures us that * they know what they're doing, use the GGTT VM. */ -- cgit From a7c1d426ef335ccfb6bd567a3f616fa232418fa2 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 18 Dec 2013 17:46:18 +0100 Subject: drm/i915: Don't check for NEEDS_GTT when deciding the address space This means something different and is only relevant for gen6 and the reason why we cant use anything else than aliasing ppgtt there. Note that the currently implemented logic for secure batches is broken: Userspace wants the buffer both in ppgtt (for self-referencing relocations) and in ggtt (for priveledge operations). This is the same issue the command parser is also facing. Unfortunately our coverage for corner-cases of self-referencing batches is spotty. Note that this will break vsync'ed Xv and DRI2 copies. Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 277485505bca..a36511db1416 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -137,8 +137,7 @@ eb_lookup_vmas(struct eb_vmas *eb, /* If we have secure dispatch, or the userspace assures us that * they know what they're doing, use the GGTT VM. */ - if (exec[i].flags & EXEC_OBJECT_NEEDS_GTT || - ((args->flags & I915_EXEC_SECURE) && + if (((args->flags & I915_EXEC_SECURE) && (i == (args->buffer_count - 1)))) bind_vm = &dev_priv->gtt.base; -- cgit From 72ad5c45f0c9036cbc6d23aeff4e8beb6d8b5e33 Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Thu, 2 Jan 2014 19:50:27 -1000 Subject: drm/i915/ppgtt: Fix ioctl errno for "no such context" Without this fix the ioctls silently succeeded (but actually did nothing). It makes all the code which calls into this function way too confusing. v2: Fix destroy IOCTL as well v3: Clarify the other two callers of i915_gem_context_get() to never check for NULL. (Mika) Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=72903 Signed-off-by: Ben Widawsky Testcase: igt/gem_ctx_exec/basic [danvet: Fix up the commit message and actually bother to mention the testcase this fixes.] Reviewed-by: Mika Kuoppala Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_context.c | 12 +++++++++--- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 4 ++-- 2 files changed, 11 insertions(+), 5 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index ebe0f67eac08..44dddc00ca72 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -526,10 +526,16 @@ void i915_gem_context_close(struct drm_device *dev, struct drm_file *file) struct i915_hw_context * i915_gem_context_get(struct drm_i915_file_private *file_priv, u32 id) { + struct i915_hw_context *ctx; + if (!HAS_HW_CONTEXTS(file_priv->dev_priv->dev)) return file_priv->private_default_ctx; - return (struct i915_hw_context *)idr_find(&file_priv->context_idr, id); + ctx = (struct i915_hw_context *)idr_find(&file_priv->context_idr, id); + if (!ctx) + return ERR_PTR(-ENOENT); + + return ctx; } static inline int @@ -776,9 +782,9 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, return ret; ctx = i915_gem_context_get(file_priv, args->ctx_id); - if (!ctx) { + if (IS_ERR(ctx)) { mutex_unlock(&dev->struct_mutex); - return -ENOENT; + return PTR_ERR(ctx); } idr_remove(&ctx->file_priv->context_idr, ctx->id); diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index a36511db1416..0843e0ec5f1b 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -904,7 +904,7 @@ i915_gem_validate_context(struct drm_device *dev, struct drm_file *file, return ERR_PTR(-EINVAL); ctx = i915_gem_context_get(file->driver_priv, ctx_id); - if (IS_ERR_OR_NULL(ctx)) + if (IS_ERR(ctx)) return ctx; hs = &ctx->hang_stats; @@ -1112,7 +1112,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, } ctx = i915_gem_validate_context(dev, file, ring, ctx_id); - if (IS_ERR_OR_NULL(ctx)) { + if (IS_ERR(ctx)) { mutex_unlock(&dev->struct_mutex); ret = PTR_ERR(ctx); goto pre_mutex_err; -- cgit From a25eebb0afb6d0bebdc86cb1e8e4a6f3dadf266c Mon Sep 17 00:00:00 2001 From: Rodrigo Vivi Date: Tue, 14 Jan 2014 16:21:49 -0200 Subject: drm: dp helper: Add DP test sink CRC definition. This address will be used to verify panel CRC for test and validation purposes. Signed-off-by: Rodrigo Vivi [danvet: Fix whitespace fail.] Acked-by: Dave Airlie Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- include/drm/drm_dp_helper.h | 10 ++++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index bd5f4e756289..0c6bcff740c2 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -131,7 +131,7 @@ eb_lookup_vmas(struct eb_vmas *eb, if (exec[i].flags & EXEC_OBJECT_NEEDS_GTT && USES_FULL_PPGTT(vm->dev)) { ret = -EINVAL; - goto out; + goto err; } /* If we have secure dispatch, or the userspace assures us that diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h index 1d09050a8c00..73c3d1f2b20d 100644 --- a/include/drm/drm_dp_helper.h +++ b/include/drm/drm_dp_helper.h @@ -279,11 +279,21 @@ #define DP_TEST_PATTERN 0x221 +#define DP_TEST_CRC_R_CR 0x240 +#define DP_TEST_CRC_G_Y 0x242 +#define DP_TEST_CRC_B_CB 0x244 + +#define DP_TEST_SINK_MISC 0x246 +#define DP_TEST_CRC_SUPPORTED (1 << 5) + #define DP_TEST_RESPONSE 0x260 # define DP_TEST_ACK (1 << 0) # define DP_TEST_NAK (1 << 1) # define DP_TEST_EDID_CHECKSUM_WRITE (1 << 2) +#define DP_TEST_SINK 0x270 +#define DP_TEST_SINK_START (1 << 0) + #define DP_SOURCE_OUI 0x300 #define DP_SINK_OUI 0x400 #define DP_BRANCH_OUI 0x500 -- cgit From d330a9530c97b8ee4704fdd7f228712029438ea9 Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Tue, 21 Jan 2014 11:24:25 +0200 Subject: drm/i915: move module parameters into a struct, in a new file With 20+ module parameters, I think referring to them via a struct improves clarity over just having a bunch of globals. While at it, move the parameter initialization and definitions into a new file i915_params.c to reduce clutter in i915_drv.c. Apart from the ill-named i915_enable_rc6, i915_enable_fbc and i915_enable_ppgtt parameters, for which we lose the "i915_" prefix internally, the module parameters now look the same both on the kernel command line and in code. For example, "i915.modeset". The downsides of the change are losing static on a couple of variables and not having the initialization and module_param_named() right next to each other. On the other hand, all module parameters are now defined in one place at i915_params.c. Plus you can do this to find all module parameter references: $ git grep "i915\." -- drivers/gpu/drm/i915 v2: - move the definitions into a new file - s/i915_params/i915/ - make i915_try_reset i915.reset, for consistency Signed-off-by: Jani Nikula Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_drv.c | 130 ++---------------------- drivers/gpu/drm/i915/i915_drv.h | 50 ++++++---- drivers/gpu/drm/i915/i915_gem.c | 4 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- drivers/gpu/drm/i915/i915_irq.c | 4 +- drivers/gpu/drm/i915/i915_params.c | 155 +++++++++++++++++++++++++++++ drivers/gpu/drm/i915/intel_bios.c | 4 +- drivers/gpu/drm/i915/intel_display.c | 26 ++--- drivers/gpu/drm/i915/intel_dp.c | 2 +- drivers/gpu/drm/i915/intel_lvds.c | 6 +- drivers/gpu/drm/i915/intel_panel.c | 15 +-- drivers/gpu/drm/i915/intel_pm.c | 12 +-- 14 files changed, 228 insertions(+), 185 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_params.c (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index da682cbcb806..4850494bd548 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -14,6 +14,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \ i915_gem_gtt.o \ i915_gem_stolen.o \ i915_gem_tiling.o \ + i915_params.o \ i915_sysfs.o \ i915_trace_points.o \ i915_ums.o \ diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 82c46050ebee..a071748d301a 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -38,120 +38,6 @@ #include #include -static int i915_modeset __read_mostly = -1; -module_param_named(modeset, i915_modeset, int, 0400); -MODULE_PARM_DESC(modeset, - "Use kernel modesetting [KMS] (0=DRM_I915_KMS from .config, " - "1=on, -1=force vga console preference [default])"); - -int i915_panel_ignore_lid __read_mostly = 1; -module_param_named(panel_ignore_lid, i915_panel_ignore_lid, int, 0600); -MODULE_PARM_DESC(panel_ignore_lid, - "Override lid status (0=autodetect, 1=autodetect disabled [default], " - "-1=force lid closed, -2=force lid open)"); - -unsigned int i915_powersave __read_mostly = 1; -module_param_named(powersave, i915_powersave, int, 0600); -MODULE_PARM_DESC(powersave, - "Enable powersavings, fbc, downclocking, etc. (default: true)"); - -int i915_semaphores __read_mostly = -1; -module_param_named(semaphores, i915_semaphores, int, 0400); -MODULE_PARM_DESC(semaphores, - "Use semaphores for inter-ring sync (default: -1 (use per-chip defaults))"); - -int i915_enable_rc6 __read_mostly = -1; -module_param_named(i915_enable_rc6, i915_enable_rc6, int, 0400); -MODULE_PARM_DESC(i915_enable_rc6, - "Enable power-saving render C-state 6. " - "Different stages can be selected via bitmask values " - "(0 = disable; 1 = enable rc6; 2 = enable deep rc6; 4 = enable deepest rc6). " - "For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. " - "default: -1 (use per-chip default)"); - -int i915_enable_fbc __read_mostly = -1; -module_param_named(i915_enable_fbc, i915_enable_fbc, int, 0600); -MODULE_PARM_DESC(i915_enable_fbc, - "Enable frame buffer compression for power savings " - "(default: -1 (use per-chip default))"); - -unsigned int i915_lvds_downclock __read_mostly = 0; -module_param_named(lvds_downclock, i915_lvds_downclock, int, 0400); -MODULE_PARM_DESC(lvds_downclock, - "Use panel (LVDS/eDP) downclocking for power savings " - "(default: false)"); - -int i915_lvds_channel_mode __read_mostly; -module_param_named(lvds_channel_mode, i915_lvds_channel_mode, int, 0600); -MODULE_PARM_DESC(lvds_channel_mode, - "Specify LVDS channel mode " - "(0=probe BIOS [default], 1=single-channel, 2=dual-channel)"); - -int i915_panel_use_ssc __read_mostly = -1; -module_param_named(lvds_use_ssc, i915_panel_use_ssc, int, 0600); -MODULE_PARM_DESC(lvds_use_ssc, - "Use Spread Spectrum Clock with panels [LVDS/eDP] " - "(default: auto from VBT)"); - -int i915_vbt_sdvo_panel_type __read_mostly = -1; -module_param_named(vbt_sdvo_panel_type, i915_vbt_sdvo_panel_type, int, 0600); -MODULE_PARM_DESC(vbt_sdvo_panel_type, - "Override/Ignore selection of SDVO panel mode in the VBT " - "(-2=ignore, -1=auto [default], index in VBT BIOS table)"); - -static bool i915_try_reset __read_mostly = true; -module_param_named(reset, i915_try_reset, bool, 0600); -MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)"); - -bool i915_enable_hangcheck __read_mostly = true; -module_param_named(enable_hangcheck, i915_enable_hangcheck, bool, 0644); -MODULE_PARM_DESC(enable_hangcheck, - "Periodically check GPU activity for detecting hangs. " - "WARNING: Disabling this can cause system wide hangs. " - "(default: true)"); - -int i915_enable_ppgtt __read_mostly = -1; -module_param_named(i915_enable_ppgtt, i915_enable_ppgtt, int, 0400); -MODULE_PARM_DESC(i915_enable_ppgtt, - "Override PPGTT usage. " - "(-1=auto [default], 0=disabled, 1=aliasing, 2=full)"); - -int i915_enable_psr __read_mostly = 0; -module_param_named(enable_psr, i915_enable_psr, int, 0600); -MODULE_PARM_DESC(enable_psr, "Enable PSR (default: false)"); - -unsigned int i915_preliminary_hw_support __read_mostly = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT); -module_param_named(preliminary_hw_support, i915_preliminary_hw_support, int, 0600); -MODULE_PARM_DESC(preliminary_hw_support, - "Enable preliminary hardware support."); - -int i915_disable_power_well __read_mostly = 1; -module_param_named(disable_power_well, i915_disable_power_well, int, 0600); -MODULE_PARM_DESC(disable_power_well, - "Disable the power well when possible (default: true)"); - -int i915_enable_ips __read_mostly = 1; -module_param_named(enable_ips, i915_enable_ips, int, 0600); -MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)"); - -bool i915_fastboot __read_mostly = 0; -module_param_named(fastboot, i915_fastboot, bool, 0600); -MODULE_PARM_DESC(fastboot, "Try to skip unnecessary mode sets at boot time " - "(default: false)"); - -int i915_enable_pc8 __read_mostly = 1; -module_param_named(enable_pc8, i915_enable_pc8, int, 0600); -MODULE_PARM_DESC(enable_pc8, "Enable support for low power package C states (PC8+) (default: true)"); - -int i915_pc8_timeout __read_mostly = 5000; -module_param_named(pc8_timeout, i915_pc8_timeout, int, 0600); -MODULE_PARM_DESC(pc8_timeout, "Number of msecs of idleness required to enter PC8+ (default: 5000)"); - -bool i915_prefault_disable __read_mostly; -module_param_named(prefault_disable, i915_prefault_disable, bool, 0600); -MODULE_PARM_DESC(prefault_disable, - "Disable page prefaulting for pread/pwrite/reloc (default:false). For developers only."); - static struct drm_driver driver; static const struct intel_device_info intel_i830_info = { @@ -480,12 +366,12 @@ bool i915_semaphore_is_enabled(struct drm_device *dev) /* Until we get further testing... */ if (IS_GEN8(dev)) { - WARN_ON(!i915_preliminary_hw_support); + WARN_ON(!i915.preliminary_hw_support); return false; } - if (i915_semaphores >= 0) - return i915_semaphores; + if (i915.semaphores >= 0) + return i915.semaphores; #ifdef CONFIG_INTEL_IOMMU /* Enable semaphores on SNB when IO remapping is off */ @@ -750,7 +636,7 @@ int i915_reset(struct drm_device *dev) bool simulated; int ret; - if (!i915_try_reset) + if (!i915.reset) return 0; mutex_lock(&dev->struct_mutex); @@ -818,7 +704,7 @@ static int i915_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent) struct intel_device_info *intel_info = (struct intel_device_info *) ent->driver_data; - if (IS_PRELIMINARY_HW(intel_info) && !i915_preliminary_hw_support) { + if (IS_PRELIMINARY_HW(intel_info) && !i915.preliminary_hw_support) { DRM_INFO("This hardware requires preliminary hardware support.\n" "See CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT, and/or modparam preliminary_hw_support\n"); return -ENODEV; @@ -1049,14 +935,14 @@ static int __init i915_init(void) * the default behavior. */ #if defined(CONFIG_DRM_I915_KMS) - if (i915_modeset != 0) + if (i915.modeset != 0) driver.driver_features |= DRIVER_MODESET; #endif - if (i915_modeset == 1) + if (i915.modeset == 1) driver.driver_features |= DRIVER_MODESET; #ifdef CONFIG_VGA_CONSOLE - if (vgacon_text_force() && i915_modeset == -1) + if (vgacon_text_force() && i915.modeset == -1) driver.driver_features &= ~DRIVER_MODESET; #endif diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f57b345f77a8..3971e7c3943d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1891,31 +1891,39 @@ struct drm_i915_file_private { extern const struct drm_ioctl_desc i915_ioctls[]; extern int i915_max_ioctl; -extern int i915_panel_ignore_lid __read_mostly; -extern unsigned int i915_powersave __read_mostly; -extern int i915_semaphores __read_mostly; -extern unsigned int i915_lvds_downclock __read_mostly; -extern int i915_lvds_channel_mode __read_mostly; -extern int i915_panel_use_ssc __read_mostly; -extern int i915_vbt_sdvo_panel_type __read_mostly; -extern int i915_enable_rc6 __read_mostly; -extern int i915_enable_fbc __read_mostly; -extern bool i915_enable_hangcheck __read_mostly; -extern int i915_enable_ppgtt __read_mostly; -extern int i915_enable_psr __read_mostly; -extern unsigned int i915_preliminary_hw_support __read_mostly; -extern int i915_disable_power_well __read_mostly; -extern int i915_enable_ips __read_mostly; -extern bool i915_fastboot __read_mostly; -extern int i915_enable_pc8 __read_mostly; -extern int i915_pc8_timeout __read_mostly; -extern bool i915_prefault_disable __read_mostly; extern int i915_suspend(struct drm_device *dev, pm_message_t state); extern int i915_resume(struct drm_device *dev); extern int i915_master_create(struct drm_device *dev, struct drm_master *master); extern void i915_master_destroy(struct drm_device *dev, struct drm_master *master); +/* i915_params.c */ +struct i915_params { + int modeset; + int panel_ignore_lid; + unsigned int powersave; + int semaphores; + unsigned int lvds_downclock; + int lvds_channel_mode; + int panel_use_ssc; + int vbt_sdvo_panel_type; + int enable_rc6; + int enable_fbc; + bool enable_hangcheck; + int enable_ppgtt; + int enable_psr; + unsigned int preliminary_hw_support; + int disable_power_well; + int enable_ips; + bool fastboot; + int enable_pc8; + int pc8_timeout; + bool prefault_disable; + bool reset; + int invert_brightness; +}; +extern struct i915_params i915 __read_mostly; + /* i915_dma.c */ void i915_update_dri1_breadcrumb(struct drm_device *dev); extern void i915_kernel_lost_context(struct drm_device * dev); @@ -2295,10 +2303,10 @@ static inline void i915_gem_chipset_flush(struct drm_device *dev) int i915_gem_init_ppgtt(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt); static inline bool intel_enable_ppgtt(struct drm_device *dev, bool full) { - if (i915_enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev)) + if (i915.enable_ppgtt == 0 || !HAS_ALIASING_PPGTT(dev)) return false; - if (i915_enable_ppgtt == 1 && full) + if (i915.enable_ppgtt == 1 && full) return false; #ifdef CONFIG_INTEL_IOMMU diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 946a577e6bee..072211bf99e3 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -476,7 +476,7 @@ i915_gem_shmem_pread(struct drm_device *dev, mutex_unlock(&dev->struct_mutex); - if (likely(!i915_prefault_disable) && !prefaulted) { + if (likely(!i915.prefault_disable) && !prefaulted) { ret = fault_in_multipages_writeable(user_data, remain); /* Userspace is tricking us, but we've already clobbered * its pages with the prefault and promised to write the @@ -868,7 +868,7 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, args->size)) return -EFAULT; - if (likely(!i915_prefault_disable)) { + if (likely(!i915.prefault_disable)) { ret = fault_in_multipages_readable(to_user_ptr(args->data_ptr), args->size); if (ret) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 0c6bcff740c2..032def901f98 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -896,7 +896,7 @@ validate_exec_list(struct drm_i915_gem_exec_object2 *exec, if (!access_ok(VERIFY_WRITE, ptr, length)) return -EFAULT; - if (likely(!i915_prefault_disable)) { + if (likely(!i915.prefault_disable)) { if (fault_in_multipages_readable(ptr, length)) return -EFAULT; } diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 3192089cb0bf..6e858e17bb0c 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -1542,7 +1542,7 @@ static inline unsigned int gen8_get_total_gtt_size(u16 bdw_gmch_ctl) if (bdw_gmch_ctl) bdw_gmch_ctl = 1 << bdw_gmch_ctl; if (bdw_gmch_ctl > 4) { - WARN_ON(!i915_preliminary_hw_support); + WARN_ON(!i915.preliminary_hw_support); return 4<<20; } diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 16d7b742f84a..72ade87a7154 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2495,7 +2495,7 @@ static void i915_hangcheck_elapsed(unsigned long data) #define HUNG 20 #define FIRE 30 - if (!i915_enable_hangcheck) + if (!i915.enable_hangcheck) return; for_each_ring(ring, dev_priv, i) { @@ -2597,7 +2597,7 @@ static void i915_hangcheck_elapsed(unsigned long data) void i915_queue_hangcheck(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev->dev_private; - if (!i915_enable_hangcheck) + if (!i915.enable_hangcheck) return; mod_timer(&dev_priv->gpu_error.hangcheck_timer, diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c new file mode 100644 index 000000000000..ee5bbf435005 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_params.c @@ -0,0 +1,155 @@ +/* + * Copyright © 2014 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, sub license, 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. + */ + +#include "i915_drv.h" + +struct i915_params i915 __read_mostly = { + .modeset = -1, + .panel_ignore_lid = 1, + .powersave = 1, + .semaphores = -1, + .lvds_downclock = 0, + .lvds_channel_mode = 0, + .panel_use_ssc = -1, + .vbt_sdvo_panel_type = -1, + .enable_rc6 = -1, + .enable_fbc = -1, + .enable_hangcheck = true, + .enable_ppgtt = -1, + .enable_psr = 0, + .preliminary_hw_support = IS_ENABLED(CONFIG_DRM_I915_PRELIMINARY_HW_SUPPORT), + .disable_power_well = 1, + .enable_ips = 1, + .fastboot = 0, + .enable_pc8 = 1, + .pc8_timeout = 5000, + .prefault_disable = 0, + .reset = true, + .invert_brightness = 0, +}; + +module_param_named(modeset, i915.modeset, int, 0400); +MODULE_PARM_DESC(modeset, + "Use kernel modesetting [KMS] (0=DRM_I915_KMS from .config, " + "1=on, -1=force vga console preference [default])"); + +module_param_named(panel_ignore_lid, i915.panel_ignore_lid, int, 0600); +MODULE_PARM_DESC(panel_ignore_lid, + "Override lid status (0=autodetect, 1=autodetect disabled [default], " + "-1=force lid closed, -2=force lid open)"); + +module_param_named(powersave, i915.powersave, int, 0600); +MODULE_PARM_DESC(powersave, + "Enable powersavings, fbc, downclocking, etc. (default: true)"); + +module_param_named(semaphores, i915.semaphores, int, 0400); +MODULE_PARM_DESC(semaphores, + "Use semaphores for inter-ring sync " + "(default: -1 (use per-chip defaults))"); + +module_param_named(i915_enable_rc6, i915.enable_rc6, int, 0400); +MODULE_PARM_DESC(i915_enable_rc6, + "Enable power-saving render C-state 6. " + "Different stages can be selected via bitmask values " + "(0 = disable; 1 = enable rc6; 2 = enable deep rc6; 4 = enable deepest rc6). " + "For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. " + "default: -1 (use per-chip default)"); + +module_param_named(i915_enable_fbc, i915.enable_fbc, int, 0600); +MODULE_PARM_DESC(i915_enable_fbc, + "Enable frame buffer compression for power savings " + "(default: -1 (use per-chip default))"); + +module_param_named(lvds_downclock, i915.lvds_downclock, int, 0400); +MODULE_PARM_DESC(lvds_downclock, + "Use panel (LVDS/eDP) downclocking for power savings " + "(default: false)"); + +module_param_named(lvds_channel_mode, i915.lvds_channel_mode, int, 0600); +MODULE_PARM_DESC(lvds_channel_mode, + "Specify LVDS channel mode " + "(0=probe BIOS [default], 1=single-channel, 2=dual-channel)"); + +module_param_named(lvds_use_ssc, i915.panel_use_ssc, int, 0600); +MODULE_PARM_DESC(lvds_use_ssc, + "Use Spread Spectrum Clock with panels [LVDS/eDP] " + "(default: auto from VBT)"); + +module_param_named(vbt_sdvo_panel_type, i915.vbt_sdvo_panel_type, int, 0600); +MODULE_PARM_DESC(vbt_sdvo_panel_type, + "Override/Ignore selection of SDVO panel mode in the VBT " + "(-2=ignore, -1=auto [default], index in VBT BIOS table)"); + +module_param_named(reset, i915.reset, bool, 0600); +MODULE_PARM_DESC(reset, "Attempt GPU resets (default: true)"); + +module_param_named(enable_hangcheck, i915.enable_hangcheck, bool, 0644); +MODULE_PARM_DESC(enable_hangcheck, + "Periodically check GPU activity for detecting hangs. " + "WARNING: Disabling this can cause system wide hangs. " + "(default: true)"); + +module_param_named(i915_enable_ppgtt, i915.enable_ppgtt, int, 0400); +MODULE_PARM_DESC(i915_enable_ppgtt, + "Override PPGTT usage. " + "(-1=auto [default], 0=disabled, 1=aliasing, 2=full)"); + +module_param_named(enable_psr, i915.enable_psr, int, 0600); +MODULE_PARM_DESC(enable_psr, "Enable PSR (default: false)"); + +module_param_named(preliminary_hw_support, i915.preliminary_hw_support, int, 0600); +MODULE_PARM_DESC(preliminary_hw_support, + "Enable preliminary hardware support."); + +module_param_named(disable_power_well, i915.disable_power_well, int, 0600); +MODULE_PARM_DESC(disable_power_well, + "Disable the power well when possible (default: true)"); + +module_param_named(enable_ips, i915.enable_ips, int, 0600); +MODULE_PARM_DESC(enable_ips, "Enable IPS (default: true)"); + +module_param_named(fastboot, i915.fastboot, bool, 0600); +MODULE_PARM_DESC(fastboot, + "Try to skip unnecessary mode sets at boot time (default: false)"); + +module_param_named(enable_pc8, i915.enable_pc8, int, 0600); +MODULE_PARM_DESC(enable_pc8, + "Enable support for low power package C states (PC8+) (default: true)"); + +module_param_named(pc8_timeout, i915.pc8_timeout, int, 0600); +MODULE_PARM_DESC(pc8_timeout, + "Number of msecs of idleness required to enter PC8+ (default: 5000)"); + +module_param_named(prefault_disable, i915.prefault_disable, bool, 0600); +MODULE_PARM_DESC(prefault_disable, + "Disable page prefaulting for pread/pwrite/reloc (default:false). " + "For developers only."); + +module_param_named(invert_brightness, i915.invert_brightness, int, 0600); +MODULE_PARM_DESC(invert_brightness, + "Invert backlight brightness " + "(-1 force normal, 0 machine defaults, 1 force inversion), please " + "report PCI device ID, subsystem vendor and subsystem device ID " + "to dri-devel@lists.freedesktop.org, if your machine needs it. " + "It will then be included in an upcoming module version."); diff --git a/drivers/gpu/drm/i915/intel_bios.c b/drivers/gpu/drm/i915/intel_bios.c index f22041973f3a..86b95ca413d1 100644 --- a/drivers/gpu/drm/i915/intel_bios.c +++ b/drivers/gpu/drm/i915/intel_bios.c @@ -259,7 +259,7 @@ parse_lfp_panel_data(struct drm_i915_private *dev_priv, downclock = dvo_timing->clock; } - if (downclock < panel_dvo_timing->clock && i915_lvds_downclock) { + if (downclock < panel_dvo_timing->clock && i915.lvds_downclock) { dev_priv->lvds_downclock_avail = 1; dev_priv->lvds_downclock = downclock * 10; DRM_DEBUG_KMS("LVDS downclock is found in VBT. " @@ -318,7 +318,7 @@ parse_sdvo_panel_data(struct drm_i915_private *dev_priv, struct drm_display_mode *panel_fixed_mode; int index; - index = i915_vbt_sdvo_panel_type; + index = i915.vbt_sdvo_panel_type; if (index == -2) { DRM_DEBUG_KMS("Ignore SDVO panel mode from BIOS VBT tables.\n"); return; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 46b014f6080f..122f87155b8e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -2372,7 +2372,7 @@ intel_pipe_set_base(struct drm_crtc *crtc, int x, int y, * whether the platform allows pfit disable with pipe active, and only * then update the pipesrc and pfit state, even on the flip path. */ - if (i915_fastboot) { + if (i915.fastboot) { const struct drm_display_mode *adjusted_mode = &intel_crtc->config.adjusted_mode; @@ -4580,7 +4580,7 @@ retry: static void hsw_compute_ips_config(struct intel_crtc *crtc, struct intel_crtc_config *pipe_config) { - pipe_config->ips_enabled = i915_enable_ips && + pipe_config->ips_enabled = i915.enable_ips && hsw_crtc_supports_ips(crtc) && pipe_config->pipe_bpp <= 24; } @@ -4781,8 +4781,8 @@ intel_link_compute_m_n(int bits_per_pixel, int nlanes, static inline bool intel_panel_use_ssc(struct drm_i915_private *dev_priv) { - if (i915_panel_use_ssc >= 0) - return i915_panel_use_ssc != 0; + if (i915.panel_use_ssc >= 0) + return i915.panel_use_ssc != 0; return dev_priv->vbt.lvds_use_ssc && !(dev_priv->quirks & QUIRK_LVDS_SSC_DISABLE); } @@ -4841,7 +4841,7 @@ static void i9xx_update_pll_dividers(struct intel_crtc *crtc, crtc->lowfreq_avail = false; if (intel_pipe_has_type(&crtc->base, INTEL_OUTPUT_LVDS) && - reduced_clock && i915_powersave) { + reduced_clock && i915.powersave) { I915_WRITE(FP1(pipe), fp2); crtc->config.dpll_hw_state.fp1 = fp2; crtc->lowfreq_avail = true; @@ -6345,7 +6345,7 @@ static int ironlake_crtc_mode_set(struct drm_crtc *crtc, if (intel_crtc->config.has_dp_encoder) intel_dp_set_m_n(intel_crtc); - if (is_lvds && has_reduced_clock && i915_powersave) + if (is_lvds && has_reduced_clock && i915.powersave) intel_crtc->lowfreq_avail = true; else intel_crtc->lowfreq_avail = false; @@ -6713,7 +6713,7 @@ static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv) return; schedule_delayed_work(&dev_priv->pc8.enable_work, - msecs_to_jiffies(i915_pc8_timeout)); + msecs_to_jiffies(i915.pc8_timeout)); } static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv) @@ -6812,7 +6812,7 @@ static void hsw_update_package_c8(struct drm_device *dev) if (!HAS_PC8(dev_priv->dev)) return; - if (!i915_enable_pc8) + if (!i915.enable_pc8) return; mutex_lock(&dev_priv->pc8.lock); @@ -8210,7 +8210,7 @@ void intel_mark_idle(struct drm_device *dev) hsw_package_c8_gpu_idle(dev_priv); - if (!i915_powersave) + if (!i915.powersave) return; list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { @@ -8230,7 +8230,7 @@ void intel_mark_fb_busy(struct drm_i915_gem_object *obj, struct drm_device *dev = obj->base.dev; struct drm_crtc *crtc; - if (!i915_powersave) + if (!i915.powersave) return; list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) { @@ -9893,7 +9893,7 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set, struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc); - if (intel_crtc->active && i915_fastboot) { + if (intel_crtc->active && i915.fastboot) { DRM_DEBUG_KMS("crtc has no fb, will flip\n"); config->fb_changed = true; } else { @@ -10144,7 +10144,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set) * flipping, so increasing its cost here shouldn't be a big * deal). */ - if (i915_fastboot && ret == 0) + if (i915.fastboot && ret == 0) intel_modeset_check_state(set->crtc->dev); } @@ -11382,7 +11382,7 @@ void intel_modeset_setup_hw_state(struct drm_device *dev, */ list_for_each_entry(crtc, &dev->mode_config.crtc_list, base.head) { - if (crtc->active && i915_fastboot) { + if (crtc->active && i915.fastboot) { intel_crtc_mode_from_pipe_config(crtc, &crtc->config); DRM_DEBUG_KMS("[CRTC:%d] found active mode: ", diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 389eabf898ad..376089052e12 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -1725,7 +1725,7 @@ static bool intel_edp_psr_match_conditions(struct intel_dp *intel_dp) return false; } - if (!i915_enable_psr) { + if (!i915.enable_psr) { DRM_DEBUG_KMS("PSR disable by flag\n"); return false; } diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 8bcb93a2a9f6..3f3043b4ff26 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -848,8 +848,8 @@ static bool compute_is_dual_link_lvds(struct intel_lvds_encoder *lvds_encoder) struct drm_i915_private *dev_priv = dev->dev_private; /* use the module option value if specified */ - if (i915_lvds_channel_mode > 0) - return i915_lvds_channel_mode == 2; + if (i915.lvds_channel_mode > 0) + return i915.lvds_channel_mode == 2; if (dmi_check_system(intel_dual_link_lvds)) return true; @@ -1036,7 +1036,7 @@ void intel_lvds_init(struct drm_device *dev) intel_find_panel_downclock(dev, fixed_mode, connector); if (intel_connector->panel.downclock_mode != - NULL && i915_lvds_downclock) { + NULL && i915.lvds_downclock) { /* We found the downclock for LVDS. */ dev_priv->lvds_downclock_avail = true; dev_priv->lvds_downclock = diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 9f83ab06fb5e..f1ee2c4d282e 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -323,13 +323,6 @@ out: pipe_config->gmch_pfit.lvds_border_bits = border; } -static int i915_panel_invert_brightness; -MODULE_PARM_DESC(invert_brightness, "Invert backlight brightness " - "(-1 force normal, 0 machine defaults, 1 force inversion), please " - "report PCI device ID, subsystem vendor and subsystem device ID " - "to dri-devel@lists.freedesktop.org, if your machine needs it. " - "It will then be included in an upcoming module version."); -module_param_named(invert_brightness, i915_panel_invert_brightness, int, 0600); static u32 intel_panel_compute_brightness(struct intel_connector *connector, u32 val) { @@ -339,10 +332,10 @@ static u32 intel_panel_compute_brightness(struct intel_connector *connector, WARN_ON(panel->backlight.max == 0); - if (i915_panel_invert_brightness < 0) + if (i915.invert_brightness < 0) return val; - if (i915_panel_invert_brightness > 0 || + if (i915.invert_brightness > 0 || dev_priv->quirks & QUIRK_INVERT_BRIGHTNESS) { return panel->backlight.max - val; } @@ -808,13 +801,13 @@ intel_panel_detect(struct drm_device *dev) struct drm_i915_private *dev_priv = dev->dev_private; /* Assume that the BIOS does not lie through the OpRegion... */ - if (!i915_panel_ignore_lid && dev_priv->opregion.lid_state) { + if (!i915.panel_ignore_lid && dev_priv->opregion.lid_state) { return ioread32(dev_priv->opregion.lid_state) & 0x1 ? connector_status_connected : connector_status_disconnected; } - switch (i915_panel_ignore_lid) { + switch (i915.panel_ignore_lid) { case -2: return connector_status_connected; case -1: diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index d91d9acfc3ff..f38470f795d7 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -469,7 +469,7 @@ void intel_update_fbc(struct drm_device *dev) return; } - if (!i915_powersave) { + if (!i915.powersave) { if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM)) DRM_DEBUG_KMS("fbc disabled per module param\n"); return; @@ -508,13 +508,13 @@ void intel_update_fbc(struct drm_device *dev) obj = intel_fb->obj; adjusted_mode = &intel_crtc->config.adjusted_mode; - if (i915_enable_fbc < 0 && + if (i915.enable_fbc < 0 && INTEL_INFO(dev)->gen <= 7 && !IS_HASWELL(dev)) { if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT)) DRM_DEBUG_KMS("disabled per chip default\n"); goto out_disable; } - if (!i915_enable_fbc) { + if (!i915.enable_fbc) { if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM)) DRM_DEBUG_KMS("fbc disabled per module param\n"); goto out_disable; @@ -3154,8 +3154,8 @@ int intel_enable_rc6(const struct drm_device *dev) return 0; /* Respect the kernel parameter if it is set */ - if (i915_enable_rc6 >= 0) - return i915_enable_rc6; + if (i915.enable_rc6 >= 0) + return i915.enable_rc6; /* Disable RC6 on Ironlake */ if (INTEL_INFO(dev)->gen == 5) @@ -5279,7 +5279,7 @@ static void __intel_power_well_put(struct drm_device *dev, WARN_ON(!power_well->count); if (!--power_well->count && power_well->set && - i915_disable_power_well) { + i915.disable_power_well) { power_well->set(dev, power_well, false); hsw_enable_package_c8(dev_priv); } -- cgit From 1ec9e26ddab06459e89a890431b2de064c5d1056 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 14 Feb 2014 14:01:11 +0100 Subject: drm/i915: Consolidate binding parameters into flags Anything more than just one bool parameter is just a pain to read, symbolic constants are much better. Split out from Chris' vma-binding rework patch. v2: Undo the behaviour change in object_pin that Chris spotted. v3: Split out misplaced hunk to handle set_cache_level errors, spotted by Jani. v4: Keep the current over-zealous binding logic in the execbuffer code working with a quick hack while the overall binding code gets shuffled around. v5: Reorder the PIN_ flags for more natural patch splitup. v6: Pull out the PIN_GLOBAL split-up again. Cc: Chris Wilson Cc: Ben Widawsky Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 14 +++---- drivers/gpu/drm/i915/i915_gem.c | 62 ++++++++++++------------------ drivers/gpu/drm/i915/i915_gem_context.c | 9 ++--- drivers/gpu/drm/i915/i915_gem_evict.c | 10 ++--- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 19 ++++++--- drivers/gpu/drm/i915/i915_gem_gtt.c | 2 +- drivers/gpu/drm/i915/i915_trace.h | 20 +++++----- drivers/gpu/drm/i915/intel_overlay.c | 2 +- drivers/gpu/drm/i915/intel_pm.c | 2 +- drivers/gpu/drm/i915/intel_ringbuffer.c | 11 +++--- 10 files changed, 70 insertions(+), 81 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index d33199452c11..8a6db27dd966 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2076,11 +2076,12 @@ void i915_init_vm(struct drm_i915_private *dev_priv, void i915_gem_free_object(struct drm_gem_object *obj); void i915_gem_vma_destroy(struct i915_vma *vma); +#define PIN_MAPPABLE 0x1 +#define PIN_NONBLOCK 0x2 int __must_check i915_gem_object_pin(struct drm_i915_gem_object *obj, struct i915_address_space *vm, uint32_t alignment, - bool map_and_fenceable, - bool nonblocking); + unsigned flags); void i915_gem_object_ggtt_unpin(struct drm_i915_gem_object *obj); int __must_check i915_vma_unbind(struct i915_vma *vma); int __must_check i915_gem_object_ggtt_unbind(struct drm_i915_gem_object *obj); @@ -2283,11 +2284,9 @@ i915_gem_obj_ggtt_size(struct drm_i915_gem_object *obj) static inline int __must_check i915_gem_obj_ggtt_pin(struct drm_i915_gem_object *obj, uint32_t alignment, - bool map_and_fenceable, - bool nonblocking) + unsigned flags) { - return i915_gem_object_pin(obj, obj_to_ggtt(obj), alignment, - map_and_fenceable, nonblocking); + return i915_gem_object_pin(obj, obj_to_ggtt(obj), alignment, flags); } /* i915_gem_context.c */ @@ -2331,8 +2330,7 @@ int __must_check i915_gem_evict_something(struct drm_device *dev, int min_size, unsigned alignment, unsigned cache_level, - bool mappable, - bool nonblock); + unsigned flags); int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle); int i915_gem_evict_everything(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index dee560267b1d..aa263e371ebc 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -43,12 +43,6 @@ static void i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *o static __must_check int i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj, bool readonly); -static __must_check int -i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, - struct i915_address_space *vm, - unsigned alignment, - bool map_and_fenceable, - bool nonblocking); static int i915_gem_phys_pwrite(struct drm_device *dev, struct drm_i915_gem_object *obj, struct drm_i915_gem_pwrite *args, @@ -605,7 +599,7 @@ i915_gem_gtt_pwrite_fast(struct drm_device *dev, char __user *user_data; int page_offset, page_length, ret; - ret = i915_gem_obj_ggtt_pin(obj, 0, true, true); + ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE | PIN_NONBLOCK); if (ret) goto out; @@ -1411,7 +1405,7 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf) } /* Now bind it into the GTT if needed */ - ret = i915_gem_obj_ggtt_pin(obj, 0, true, false); + ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE); if (ret) goto unlock; @@ -2721,7 +2715,6 @@ int i915_vma_unbind(struct i915_vma *vma) if (!drm_mm_node_allocated(&vma->node)) { i915_gem_vma_destroy(vma); - return 0; } @@ -3219,14 +3212,13 @@ static int i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, struct i915_address_space *vm, unsigned alignment, - bool map_and_fenceable, - bool nonblocking) + unsigned flags) { struct drm_device *dev = obj->base.dev; drm_i915_private_t *dev_priv = dev->dev_private; u32 size, fence_size, fence_alignment, unfenced_alignment; size_t gtt_max = - map_and_fenceable ? dev_priv->gtt.mappable_end : vm->total; + flags & PIN_MAPPABLE ? dev_priv->gtt.mappable_end : vm->total; struct i915_vma *vma; int ret; @@ -3238,18 +3230,18 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, obj->tiling_mode, true); unfenced_alignment = i915_gem_get_gtt_alignment(dev, - obj->base.size, - obj->tiling_mode, false); + obj->base.size, + obj->tiling_mode, false); if (alignment == 0) - alignment = map_and_fenceable ? fence_alignment : + alignment = flags & PIN_MAPPABLE ? fence_alignment : unfenced_alignment; - if (map_and_fenceable && alignment & (fence_alignment - 1)) { + if (flags & PIN_MAPPABLE && alignment & (fence_alignment - 1)) { DRM_DEBUG("Invalid object alignment requested %u\n", alignment); return -EINVAL; } - size = map_and_fenceable ? fence_size : obj->base.size; + size = flags & PIN_MAPPABLE ? fence_size : obj->base.size; /* If the object is bigger than the entire aperture, reject it early * before evicting everything in a vain attempt to find space. @@ -3257,7 +3249,7 @@ i915_gem_object_bind_to_vm(struct drm_i915_gem_object *obj, if (obj->base.size > gtt_max) { DRM_DEBUG("Attempting to bind an object larger than the aperture: object=%zd > %s aperture=%zu\n", obj->base.size, - map_and_fenceable ? "mappable" : "total", + flags & PIN_MAPPABLE ? "mappable" : "total", gtt_max); return -E2BIG; } @@ -3281,9 +3273,7 @@ search_free: DRM_MM_SEARCH_DEFAULT); if (ret) { ret = i915_gem_evict_something(dev, vm, size, alignment, - obj->cache_level, - map_and_fenceable, - nonblocking); + obj->cache_level, flags); if (ret == 0) goto search_free; @@ -3314,9 +3304,9 @@ search_free: obj->map_and_fenceable = mappable && fenceable; } - WARN_ON(map_and_fenceable && !obj->map_and_fenceable); + WARN_ON(flags & PIN_MAPPABLE && !obj->map_and_fenceable); - trace_i915_vma_bind(vma, map_and_fenceable); + trace_i915_vma_bind(vma, flags); i915_gem_verify_gtt(dev); return 0; @@ -3687,7 +3677,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, * (e.g. libkms for the bootup splash), we have to ensure that we * always use map_and_fenceable for all scanout buffers. */ - ret = i915_gem_obj_ggtt_pin(obj, alignment, true, false); + ret = i915_gem_obj_ggtt_pin(obj, alignment, PIN_MAPPABLE); if (ret) goto err_unpin_display; @@ -3843,30 +3833,28 @@ int i915_gem_object_pin(struct drm_i915_gem_object *obj, struct i915_address_space *vm, uint32_t alignment, - bool map_and_fenceable, - bool nonblocking) + unsigned flags) { - const u32 flags = map_and_fenceable ? GLOBAL_BIND : 0; struct i915_vma *vma; int ret; - WARN_ON(map_and_fenceable && !i915_is_ggtt(vm)); + if (WARN_ON(flags & PIN_MAPPABLE && !i915_is_ggtt(vm))) + return -EINVAL; vma = i915_gem_obj_to_vma(obj, vm); - if (vma) { if (WARN_ON(vma->pin_count == DRM_I915_GEM_OBJECT_MAX_PIN_COUNT)) return -EBUSY; if ((alignment && vma->node.start & (alignment - 1)) || - (map_and_fenceable && !obj->map_and_fenceable)) { + (flags & PIN_MAPPABLE && !obj->map_and_fenceable)) { WARN(vma->pin_count, "bo is already pinned with incorrect alignment:" " offset=%lx, req.alignment=%x, req.map_and_fenceable=%d," " obj->map_and_fenceable=%d\n", i915_gem_obj_offset(obj, vm), alignment, - map_and_fenceable, + flags & PIN_MAPPABLE, obj->map_and_fenceable); ret = i915_vma_unbind(vma); if (ret) @@ -3875,9 +3863,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, } if (!i915_gem_obj_bound(obj, vm)) { - ret = i915_gem_object_bind_to_vm(obj, vm, alignment, - map_and_fenceable, - nonblocking); + ret = i915_gem_object_bind_to_vm(obj, vm, alignment, flags); if (ret) return ret; @@ -3885,10 +3871,12 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, vma = i915_gem_obj_to_vma(obj, vm); - vma->bind_vma(vma, obj->cache_level, flags); + vma->bind_vma(vma, obj->cache_level, + flags & PIN_MAPPABLE ? GLOBAL_BIND : 0); i915_gem_obj_to_vma(obj, vm)->pin_count++; - obj->pin_mappable |= map_and_fenceable; + if (flags & PIN_MAPPABLE) + obj->pin_mappable |= true; return 0; } @@ -3946,7 +3934,7 @@ i915_gem_pin_ioctl(struct drm_device *dev, void *data, } if (obj->user_pin_count == 0) { - ret = i915_gem_obj_ggtt_pin(obj, args->alignment, true, false); + ret = i915_gem_obj_ggtt_pin(obj, args->alignment, PIN_MAPPABLE); if (ret) goto out; } diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c index 19fd3629795c..f8c21a6dd663 100644 --- a/drivers/gpu/drm/i915/i915_gem_context.c +++ b/drivers/gpu/drm/i915/i915_gem_context.c @@ -258,8 +258,7 @@ i915_gem_create_context(struct drm_device *dev, * context. */ ret = i915_gem_obj_ggtt_pin(ctx->obj, - get_context_alignment(dev), - false, false); + get_context_alignment(dev), 0); if (ret) { DRM_DEBUG_DRIVER("Couldn't pin %d\n", ret); goto err_destroy; @@ -335,8 +334,7 @@ void i915_gem_context_reset(struct drm_device *dev) if (i == RCS) { WARN_ON(i915_gem_obj_ggtt_pin(dctx->obj, - get_context_alignment(dev), - false, false)); + get_context_alignment(dev), 0)); /* Fake a finish/inactive */ dctx->obj->base.write_domain = 0; dctx->obj->active = 0; @@ -612,8 +610,7 @@ static int do_switch(struct intel_ring_buffer *ring, /* Trying to pin first makes error handling easier. */ if (ring == &dev_priv->ring[RCS]) { ret = i915_gem_obj_ggtt_pin(to->obj, - get_context_alignment(ring->dev), - false, false); + get_context_alignment(ring->dev), 0); if (ret) return ret; } diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c index 5168d6a08054..8a78f7885cba 100644 --- a/drivers/gpu/drm/i915/i915_gem_evict.c +++ b/drivers/gpu/drm/i915/i915_gem_evict.c @@ -68,7 +68,7 @@ mark_free(struct i915_vma *vma, struct list_head *unwind) int i915_gem_evict_something(struct drm_device *dev, struct i915_address_space *vm, int min_size, unsigned alignment, unsigned cache_level, - bool mappable, bool nonblocking) + unsigned flags) { drm_i915_private_t *dev_priv = dev->dev_private; struct list_head eviction_list, unwind_list; @@ -76,7 +76,7 @@ i915_gem_evict_something(struct drm_device *dev, struct i915_address_space *vm, int ret = 0; int pass = 0; - trace_i915_gem_evict(dev, min_size, alignment, mappable); + trace_i915_gem_evict(dev, min_size, alignment, flags); /* * The goal is to evict objects and amalgamate space in LRU order. @@ -102,7 +102,7 @@ i915_gem_evict_something(struct drm_device *dev, struct i915_address_space *vm, */ INIT_LIST_HEAD(&unwind_list); - if (mappable) { + if (flags & PIN_MAPPABLE) { BUG_ON(!i915_is_ggtt(vm)); drm_mm_init_scan_with_range(&vm->mm, min_size, alignment, cache_level, 0, @@ -117,7 +117,7 @@ search_again: goto found; } - if (nonblocking) + if (flags & PIN_NONBLOCK) goto none; /* Now merge in the soon-to-be-expired objects... */ @@ -141,7 +141,7 @@ none: /* Can we unpin some objects such as idle hw contents, * or pending flips? */ - if (nonblocking) + if (flags & PIN_NONBLOCK) return -ENOSPC; /* Only idle the GPU and repeat the search once */ diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 032def901f98..013bd5ab3913 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -544,19 +544,23 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma, struct drm_i915_gem_object *obj = vma->obj; struct drm_i915_gem_exec_object2 *entry = vma->exec_entry; bool has_fenced_gpu_access = INTEL_INFO(ring->dev)->gen < 4; - bool need_fence, need_mappable; - u32 flags = (entry->flags & EXEC_OBJECT_NEEDS_GTT) && - !vma->obj->has_global_gtt_mapping ? GLOBAL_BIND : 0; + bool need_fence; + unsigned flags; int ret; + flags = 0; + need_fence = has_fenced_gpu_access && entry->flags & EXEC_OBJECT_NEEDS_FENCE && obj->tiling_mode != I915_TILING_NONE; - need_mappable = need_fence || need_reloc_mappable(vma); + if (need_fence || need_reloc_mappable(vma)) + flags |= PIN_MAPPABLE; - ret = i915_gem_object_pin(obj, vma->vm, entry->alignment, need_mappable, - false); + if (entry->flags & EXEC_OBJECT_NEEDS_GTT) + flags |= PIN_MAPPABLE; + + ret = i915_gem_object_pin(obj, vma->vm, entry->alignment, flags); if (ret) return ret; @@ -585,6 +589,9 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma, obj->base.pending_write_domain = I915_GEM_DOMAIN_RENDER; } + /* Temporary hack while we rework the binding logic. */ + flags = (entry->flags & EXEC_OBJECT_NEEDS_GTT) && + !vma->obj->has_global_gtt_mapping ? GLOBAL_BIND : 0; vma->bind_vma(vma, obj->cache_level, flags); return 0; diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index ee38fafc6917..1dcd50541ae2 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -885,7 +885,7 @@ alloc: if (ret == -ENOSPC && !retried) { ret = i915_gem_evict_something(dev, &dev_priv->gtt.base, GEN6_PD_SIZE, GEN6_PD_ALIGN, - I915_CACHE_NONE, false, true); + I915_CACHE_NONE, PIN_NONBLOCK); if (ret) return ret; diff --git a/drivers/gpu/drm/i915/i915_trace.h b/drivers/gpu/drm/i915/i915_trace.h index 6e580c98dede..b95a380958db 100644 --- a/drivers/gpu/drm/i915/i915_trace.h +++ b/drivers/gpu/drm/i915/i915_trace.h @@ -34,15 +34,15 @@ TRACE_EVENT(i915_gem_object_create, ); TRACE_EVENT(i915_vma_bind, - TP_PROTO(struct i915_vma *vma, bool mappable), - TP_ARGS(vma, mappable), + TP_PROTO(struct i915_vma *vma, unsigned flags), + TP_ARGS(vma, flags), TP_STRUCT__entry( __field(struct drm_i915_gem_object *, obj) __field(struct i915_address_space *, vm) __field(u32, offset) __field(u32, size) - __field(bool, mappable) + __field(unsigned, flags) ), TP_fast_assign( @@ -50,12 +50,12 @@ TRACE_EVENT(i915_vma_bind, __entry->vm = vma->vm; __entry->offset = vma->node.start; __entry->size = vma->node.size; - __entry->mappable = mappable; + __entry->flags = flags; ), TP_printk("obj=%p, offset=%08x size=%x%s vm=%p", __entry->obj, __entry->offset, __entry->size, - __entry->mappable ? ", mappable" : "", + __entry->flags & PIN_MAPPABLE ? ", mappable" : "", __entry->vm) ); @@ -196,26 +196,26 @@ DEFINE_EVENT(i915_gem_object, i915_gem_object_destroy, ); TRACE_EVENT(i915_gem_evict, - TP_PROTO(struct drm_device *dev, u32 size, u32 align, bool mappable), - TP_ARGS(dev, size, align, mappable), + TP_PROTO(struct drm_device *dev, u32 size, u32 align, unsigned flags), + TP_ARGS(dev, size, align, flags), TP_STRUCT__entry( __field(u32, dev) __field(u32, size) __field(u32, align) - __field(bool, mappable) + __field(unsigned, flags) ), TP_fast_assign( __entry->dev = dev->primary->index; __entry->size = size; __entry->align = align; - __entry->mappable = mappable; + __entry->flags = flags; ), TP_printk("dev=%d, size=%d, align=%d %s", __entry->dev, __entry->size, __entry->align, - __entry->mappable ? ", mappable" : "") + __entry->flags & PIN_MAPPABLE ? ", mappable" : "") ); TRACE_EVENT(i915_gem_evict_everything, diff --git a/drivers/gpu/drm/i915/intel_overlay.c b/drivers/gpu/drm/i915/intel_overlay.c index 424f0946d8c4..ac519cb46f22 100644 --- a/drivers/gpu/drm/i915/intel_overlay.c +++ b/drivers/gpu/drm/i915/intel_overlay.c @@ -1349,7 +1349,7 @@ void intel_setup_overlay(struct drm_device *dev) } overlay->flip_addr = reg_bo->phys_obj->handle->busaddr; } else { - ret = i915_gem_obj_ggtt_pin(reg_bo, PAGE_SIZE, true, false); + ret = i915_gem_obj_ggtt_pin(reg_bo, PAGE_SIZE, PIN_MAPPABLE); if (ret) { DRM_ERROR("failed to pin overlay register bo\n"); goto out_free_bo; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index e4a0c9cc226d..136647037244 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2741,7 +2741,7 @@ intel_alloc_context_page(struct drm_device *dev) return NULL; } - ret = i915_gem_obj_ggtt_pin(ctx, 4096, true, false); + ret = i915_gem_obj_ggtt_pin(ctx, 4096, PIN_MAPPABLE); if (ret) { DRM_ERROR("failed to pin power context: %d\n", ret); goto err_unref; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index ae6d234b8c12..f256d5fe46f8 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -533,7 +533,7 @@ init_pipe_control(struct intel_ring_buffer *ring) i915_gem_object_set_cache_level(ring->scratch.obj, I915_CACHE_LLC); - ret = i915_gem_obj_ggtt_pin(ring->scratch.obj, 4096, true, false); + ret = i915_gem_obj_ggtt_pin(ring->scratch.obj, 4096, 0); if (ret) goto err_unref; @@ -1273,10 +1273,9 @@ static int init_status_page(struct intel_ring_buffer *ring) i915_gem_object_set_cache_level(obj, I915_CACHE_LLC); - ret = i915_gem_obj_ggtt_pin(obj, 4096, true, false); - if (ret != 0) { + ret = i915_gem_obj_ggtt_pin(obj, 4096, PIN_MAPPABLE); + if (ret) goto err_unref; - } ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(obj); ring->status_page.page_addr = kmap(sg_page(obj->pages->sgl)); @@ -1356,7 +1355,7 @@ static int intel_init_ring_buffer(struct drm_device *dev, ring->obj = obj; - ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, true, false); + ret = i915_gem_obj_ggtt_pin(obj, PAGE_SIZE, PIN_MAPPABLE); if (ret) goto err_unref; @@ -1919,7 +1918,7 @@ int intel_init_render_ring_buffer(struct drm_device *dev) return -ENOMEM; } - ret = i915_gem_obj_ggtt_pin(obj, 0, true, false); + ret = i915_gem_obj_ggtt_pin(obj, 0, PIN_MAPPABLE); if (ret != 0) { drm_gem_object_unreference(&obj->base); DRM_ERROR("Failed to ping batch bo\n"); -- cgit From bf3d149b25f67f241735b91a56b7f070bc0a5407 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 14 Feb 2014 14:01:12 +0100 Subject: drm/i915: split PIN_GLOBAL out from PIN_MAPPABLE With abitrary pin flags it makes sense to split out a "please bind this into global gtt" from the "please allocate in the mappable range". Use this unconditionally in our global gtt pin helper since this is what its callers want. Later patches will drop PIN_MAPPABLE where it's not strictly needed. Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 3 ++- drivers/gpu/drm/i915/i915_gem.c | 4 ++-- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- 3 files changed, 5 insertions(+), 4 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8a6db27dd966..8300c5b38203 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2078,6 +2078,7 @@ void i915_gem_vma_destroy(struct i915_vma *vma); #define PIN_MAPPABLE 0x1 #define PIN_NONBLOCK 0x2 +#define PIN_GLOBAL 0x4 int __must_check i915_gem_object_pin(struct drm_i915_gem_object *obj, struct i915_address_space *vm, uint32_t alignment, @@ -2286,7 +2287,7 @@ i915_gem_obj_ggtt_pin(struct drm_i915_gem_object *obj, uint32_t alignment, unsigned flags) { - return i915_gem_object_pin(obj, obj_to_ggtt(obj), alignment, flags); + return i915_gem_object_pin(obj, obj_to_ggtt(obj), alignment, flags | PIN_GLOBAL); } /* i915_gem_context.c */ diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index aa263e371ebc..b3a15c9fa034 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3838,7 +3838,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, struct i915_vma *vma; int ret; - if (WARN_ON(flags & PIN_MAPPABLE && !i915_is_ggtt(vm))) + if (WARN_ON(flags & (PIN_GLOBAL | PIN_MAPPABLE) && !i915_is_ggtt(vm))) return -EINVAL; vma = i915_gem_obj_to_vma(obj, vm); @@ -3872,7 +3872,7 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, vma = i915_gem_obj_to_vma(obj, vm); vma->bind_vma(vma, obj->cache_level, - flags & PIN_MAPPABLE ? GLOBAL_BIND : 0); + flags & PIN_GLOBAL ? GLOBAL_BIND : 0); i915_gem_obj_to_vma(obj, vm)->pin_count++; if (flags & PIN_MAPPABLE) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 013bd5ab3913..b35849bbc367 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -558,7 +558,7 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma, flags |= PIN_MAPPABLE; if (entry->flags & EXEC_OBJECT_NEEDS_GTT) - flags |= PIN_MAPPABLE; + flags |= PIN_GLOBAL; ret = i915_gem_object_pin(obj, vma->vm, entry->alignment, flags); if (ret) -- cgit From 8ea99c928787ba1712b7506b4c56c948c45d84b1 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 14 Feb 2014 14:01:21 +0100 Subject: drm/i915: Only bind each object rather than for every execbuffer One side-effect of the introduction of ppgtt was that we needed to rebind the object into the appropriate vm (and global gtt in some peculiar cases). For simplicity this was done twice for every object on every call to execbuffer. However, that adds a tremendous amount of CPU overhead (rewriting all the PTE for all objects into WC memory) per draw. The fix is to push all the decision about which vm to bind into and when down into the low-level bind routines through hints rather than performing the bind unconditionally in the execbuffer routine. Note that this is a regression introduced in the full ppgtt feature branch, before this we've only done re-bound objects when the relevant has_(aliasing_ppgtt|global_gtt)_mapping flag was clear. But since that's per-object and not per-vma that optimization broke. v2: Split out prep work and unrelated changes. v3: Bring back functional change around PIN_GLOBAL that I've accidentally split out. v4: Remove the temporary hack for the old binding logic to avoid bisection issues. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=72906 Tested-by: jianx.zhou@intel.com Signed-off-by: Chris Wilson (v1) Cc: Ben Widawsky Cc: Daniel Vetter Acked-by: Ben Widawsky Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_gem.c | 20 ++++++++++++-------- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 5 ----- 2 files changed, 12 insertions(+), 13 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index ce7064d1ab30..3618bb0cda0a 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3285,6 +3285,9 @@ search_free: WARN_ON(flags & PIN_MAPPABLE && !obj->map_and_fenceable); trace_i915_vma_bind(vma, flags); + vma->bind_vma(vma, obj->cache_level, + flags & (PIN_MAPPABLE | PIN_GLOBAL) ? GLOBAL_BIND : 0); + i915_gem_verify_gtt(dev); return vma; @@ -3487,7 +3490,9 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj, } list_for_each_entry(vma, &obj->vma_list, vma_link) - vma->bind_vma(vma, cache_level, 0); + if (drm_mm_node_allocated(&vma->node)) + vma->bind_vma(vma, cache_level, + obj->has_global_gtt_mapping ? GLOBAL_BIND : 0); } list_for_each_entry(vma, &obj->vma_list, vma_link) @@ -3838,22 +3843,21 @@ i915_gem_object_pin(struct drm_i915_gem_object *obj, ret = i915_vma_unbind(vma); if (ret) return ret; + + vma = NULL; } } - if (!i915_gem_obj_bound(obj, vm)) { - + if (vma == NULL || !drm_mm_node_allocated(&vma->node)) { vma = i915_gem_object_bind_to_vm(obj, vm, alignment, flags); if (IS_ERR(vma)) return PTR_ERR(vma); } - vma = i915_gem_obj_to_vma(obj, vm); - - vma->bind_vma(vma, obj->cache_level, - flags & PIN_GLOBAL ? GLOBAL_BIND : 0); + if (flags & PIN_GLOBAL && !obj->has_global_gtt_mapping) + vma->bind_vma(vma, obj->cache_level, GLOBAL_BIND); - i915_gem_obj_to_vma(obj, vm)->pin_count++; + vma->pin_count++; if (flags & PIN_MAPPABLE) obj->pin_mappable |= true; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index b35849bbc367..d7229ad2bd22 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -589,11 +589,6 @@ i915_gem_execbuffer_reserve_vma(struct i915_vma *vma, obj->base.pending_write_domain = I915_GEM_DOMAIN_RENDER; } - /* Temporary hack while we rework the binding logic. */ - flags = (entry->flags & EXEC_OBJECT_NEEDS_GTT) && - !vma->obj->has_global_gtt_mapping ? GLOBAL_BIND : 0; - vma->bind_vma(vma, obj->cache_level, flags); - return 0; } -- cgit From 351e3db2b3631556607d0d94fa26df8e2e0d0fd8 Mon Sep 17 00:00:00 2001 From: Brad Volkin Date: Tue, 18 Feb 2014 10:15:46 -0800 Subject: drm/i915: Implement command buffer parsing logic The command parser scans batch buffers submitted via execbuffer ioctls before the driver submits them to hardware. At a high level, it looks for several things: 1) Commands which are explicitly defined as privileged or which should only be used by the kernel driver. The parser generally rejects such commands, with the provision that it may allow some from the drm master process. 2) Commands which access registers. To support correct/enhanced userspace functionality, particularly certain OpenGL extensions, the parser provides a whitelist of registers which userspace may safely access (for both normal and drm master processes). 3) Commands which access privileged memory (i.e. GGTT, HWS page, etc). The parser always rejects such commands. See the overview comment in the source for more details. This patch only implements the logic. Subsequent patches will build the tables that drive the parser. v2: Don't set the secure bit if the parser succeeds Fail harder during init Makefile cleanup Kerneldoc cleanup Clarify module param description Convert ints to bools in a few places Move client/subclient defs to i915_reg.h Remove the bits_count field OTC-Tracker: AXIA-4631 Change-Id: I50b98c71c6655893291c78a2d1b8954577b37a30 Signed-off-by: Brad Volkin Reviewed-by: Jani Nikula [danvet: Appease checkpatch.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_cmd_parser.c | 485 +++++++++++++++++++++++++++++ drivers/gpu/drm/i915/i915_drv.h | 93 ++++++ drivers/gpu/drm/i915/i915_gem_execbuffer.c | 18 ++ drivers/gpu/drm/i915/i915_params.c | 5 + drivers/gpu/drm/i915/i915_reg.h | 12 + drivers/gpu/drm/i915/intel_ringbuffer.c | 2 + drivers/gpu/drm/i915/intel_ringbuffer.h | 32 ++ 8 files changed, 648 insertions(+) create mode 100644 drivers/gpu/drm/i915/i915_cmd_parser.c (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 4850494bd548..3569122b1995 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -14,6 +14,7 @@ i915-y := i915_drv.o i915_dma.o i915_irq.o \ i915_gem_gtt.o \ i915_gem_stolen.o \ i915_gem_tiling.o \ + i915_cmd_parser.o \ i915_params.o \ i915_sysfs.o \ i915_trace_points.o \ diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c new file mode 100644 index 000000000000..7a5756e9bb86 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c @@ -0,0 +1,485 @@ +/* + * Copyright © 2013 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: + * Brad Volkin + * + */ + +#include "i915_drv.h" + +/** + * DOC: i915 batch buffer command parser + * + * Motivation: + * Certain OpenGL features (e.g. transform feedback, performance monitoring) + * require userspace code to submit batches containing commands such as + * MI_LOAD_REGISTER_IMM to access various registers. Unfortunately, some + * generations of the hardware will noop these commands in "unsecure" batches + * (which includes all userspace batches submitted via i915) even though the + * commands may be safe and represent the intended programming model of the + * device. + * + * The software command parser is similar in operation to the command parsing + * done in hardware for unsecure batches. However, the software parser allows + * some operations that would be noop'd by hardware, if the parser determines + * the operation is safe, and submits the batch as "secure" to prevent hardware + * parsing. + * + * Threats: + * At a high level, the hardware (and software) checks attempt to prevent + * granting userspace undue privileges. There are three categories of privilege. + * + * First, commands which are explicitly defined as privileged or which should + * only be used by the kernel driver. The parser generally rejects such + * commands, though it may allow some from the drm master process. + * + * Second, commands which access registers. To support correct/enhanced + * userspace functionality, particularly certain OpenGL extensions, the parser + * provides a whitelist of registers which userspace may safely access (for both + * normal and drm master processes). + * + * Third, commands which access privileged memory (i.e. GGTT, HWS page, etc). + * The parser always rejects such commands. + * + * The majority of the problematic commands fall in the MI_* range, with only a + * few specific commands on each ring (e.g. PIPE_CONTROL and MI_FLUSH_DW). + * + * Implementation: + * Each ring maintains tables of commands and registers which the parser uses in + * scanning batch buffers submitted to that ring. + * + * Since the set of commands that the parser must check for is significantly + * smaller than the number of commands supported, the parser tables contain only + * those commands required by the parser. This generally works because command + * opcode ranges have standard command length encodings. So for commands that + * the parser does not need to check, it can easily skip them. This is + * implementated via a per-ring length decoding vfunc. + * + * Unfortunately, there are a number of commands that do not follow the standard + * length encoding for their opcode range, primarily amongst the MI_* commands. + * To handle this, the parser provides a way to define explicit "skip" entries + * in the per-ring command tables. + * + * Other command table entries map fairly directly to high level categories + * mentioned above: rejected, master-only, register whitelist. The parser + * implements a number of checks, including the privileged memory checks, via a + * general bitmasking mechanism. + */ + +static u32 gen7_render_get_cmd_length_mask(u32 cmd_header) +{ + u32 client = (cmd_header & INSTR_CLIENT_MASK) >> INSTR_CLIENT_SHIFT; + u32 subclient = + (cmd_header & INSTR_SUBCLIENT_MASK) >> INSTR_SUBCLIENT_SHIFT; + + if (client == INSTR_MI_CLIENT) + return 0x3F; + else if (client == INSTR_RC_CLIENT) { + if (subclient == INSTR_MEDIA_SUBCLIENT) + return 0xFFFF; + else + return 0xFF; + } + + DRM_DEBUG_DRIVER("CMD: Abnormal rcs cmd length! 0x%08X\n", cmd_header); + return 0; +} + +static u32 gen7_bsd_get_cmd_length_mask(u32 cmd_header) +{ + u32 client = (cmd_header & INSTR_CLIENT_MASK) >> INSTR_CLIENT_SHIFT; + u32 subclient = + (cmd_header & INSTR_SUBCLIENT_MASK) >> INSTR_SUBCLIENT_SHIFT; + + if (client == INSTR_MI_CLIENT) + return 0x3F; + else if (client == INSTR_RC_CLIENT) { + if (subclient == INSTR_MEDIA_SUBCLIENT) + return 0xFFF; + else + return 0xFF; + } + + DRM_DEBUG_DRIVER("CMD: Abnormal bsd cmd length! 0x%08X\n", cmd_header); + return 0; +} + +static u32 gen7_blt_get_cmd_length_mask(u32 cmd_header) +{ + u32 client = (cmd_header & INSTR_CLIENT_MASK) >> INSTR_CLIENT_SHIFT; + + if (client == INSTR_MI_CLIENT) + return 0x3F; + else if (client == INSTR_BC_CLIENT) + return 0xFF; + + DRM_DEBUG_DRIVER("CMD: Abnormal blt cmd length! 0x%08X\n", cmd_header); + return 0; +} + +static void validate_cmds_sorted(struct intel_ring_buffer *ring) +{ + int i; + + if (!ring->cmd_tables || ring->cmd_table_count == 0) + return; + + for (i = 0; i < ring->cmd_table_count; i++) { + const struct drm_i915_cmd_table *table = &ring->cmd_tables[i]; + u32 previous = 0; + int j; + + for (j = 0; j < table->count; j++) { + const struct drm_i915_cmd_descriptor *desc = + &table->table[i]; + u32 curr = desc->cmd.value & desc->cmd.mask; + + if (curr < previous) + DRM_ERROR("CMD: table not sorted ring=%d table=%d entry=%d cmd=0x%08X prev=0x%08X\n", + ring->id, i, j, curr, previous); + + previous = curr; + } + } +} + +static void check_sorted(int ring_id, const u32 *reg_table, int reg_count) +{ + int i; + u32 previous = 0; + + for (i = 0; i < reg_count; i++) { + u32 curr = reg_table[i]; + + if (curr < previous) + DRM_ERROR("CMD: table not sorted ring=%d entry=%d reg=0x%08X prev=0x%08X\n", + ring_id, i, curr, previous); + + previous = curr; + } +} + +static void validate_regs_sorted(struct intel_ring_buffer *ring) +{ + check_sorted(ring->id, ring->reg_table, ring->reg_count); + check_sorted(ring->id, ring->master_reg_table, ring->master_reg_count); +} + +/** + * i915_cmd_parser_init_ring() - set cmd parser related fields for a ringbuffer + * @ring: the ringbuffer to initialize + * + * Optionally initializes fields related to batch buffer command parsing in the + * struct intel_ring_buffer based on whether the platform requires software + * command parsing. + */ +void i915_cmd_parser_init_ring(struct intel_ring_buffer *ring) +{ + if (!IS_GEN7(ring->dev)) + return; + + switch (ring->id) { + case RCS: + ring->get_cmd_length_mask = gen7_render_get_cmd_length_mask; + break; + case VCS: + ring->get_cmd_length_mask = gen7_bsd_get_cmd_length_mask; + break; + case BCS: + ring->get_cmd_length_mask = gen7_blt_get_cmd_length_mask; + break; + case VECS: + /* VECS can use the same length_mask function as VCS */ + ring->get_cmd_length_mask = gen7_bsd_get_cmd_length_mask; + break; + default: + DRM_ERROR("CMD: cmd_parser_init with unknown ring: %d\n", + ring->id); + BUG(); + } + + validate_cmds_sorted(ring); + validate_regs_sorted(ring); +} + +static const struct drm_i915_cmd_descriptor* +find_cmd_in_table(const struct drm_i915_cmd_table *table, + u32 cmd_header) +{ + int i; + + for (i = 0; i < table->count; i++) { + const struct drm_i915_cmd_descriptor *desc = &table->table[i]; + u32 masked_cmd = desc->cmd.mask & cmd_header; + u32 masked_value = desc->cmd.value & desc->cmd.mask; + + if (masked_cmd == masked_value) + return desc; + } + + return NULL; +} + +/* + * Returns a pointer to a descriptor for the command specified by cmd_header. + * + * The caller must supply space for a default descriptor via the default_desc + * parameter. If no descriptor for the specified command exists in the ring's + * command parser tables, this function fills in default_desc based on the + * ring's default length encoding and returns default_desc. + */ +static const struct drm_i915_cmd_descriptor* +find_cmd(struct intel_ring_buffer *ring, + u32 cmd_header, + struct drm_i915_cmd_descriptor *default_desc) +{ + u32 mask; + int i; + + for (i = 0; i < ring->cmd_table_count; i++) { + const struct drm_i915_cmd_descriptor *desc; + + desc = find_cmd_in_table(&ring->cmd_tables[i], cmd_header); + if (desc) + return desc; + } + + mask = ring->get_cmd_length_mask(cmd_header); + if (!mask) + return NULL; + + BUG_ON(!default_desc); + default_desc->flags = CMD_DESC_SKIP; + default_desc->length.mask = mask; + + return default_desc; +} + +static bool valid_reg(const u32 *table, int count, u32 addr) +{ + if (table && count != 0) { + int i; + + for (i = 0; i < count; i++) { + if (table[i] == addr) + return true; + } + } + + return false; +} + +static u32 *vmap_batch(struct drm_i915_gem_object *obj) +{ + int i; + void *addr = NULL; + struct sg_page_iter sg_iter; + struct page **pages; + + pages = drm_malloc_ab(obj->base.size >> PAGE_SHIFT, sizeof(*pages)); + if (pages == NULL) { + DRM_DEBUG_DRIVER("Failed to get space for pages\n"); + goto finish; + } + + i = 0; + for_each_sg_page(obj->pages->sgl, &sg_iter, obj->pages->nents, 0) { + pages[i] = sg_page_iter_page(&sg_iter); + i++; + } + + addr = vmap(pages, i, 0, PAGE_KERNEL); + if (addr == NULL) { + DRM_DEBUG_DRIVER("Failed to vmap pages\n"); + goto finish; + } + +finish: + if (pages) + drm_free_large(pages); + return (u32*)addr; +} + +/** + * i915_needs_cmd_parser() - should a given ring use software command parsing? + * @ring: the ring in question + * + * Only certain platforms require software batch buffer command parsing, and + * only when enabled via module paramter. + * + * Return: true if the ring requires software command parsing + */ +bool i915_needs_cmd_parser(struct intel_ring_buffer *ring) +{ + /* No command tables indicates a platform without parsing */ + if (!ring->cmd_tables) + return false; + + return (i915.enable_cmd_parser == 1); +} + +#define LENGTH_BIAS 2 + +/** + * i915_parse_cmds() - parse a submitted batch buffer for privilege violations + * @ring: the ring on which the batch is to execute + * @batch_obj: the batch buffer in question + * @batch_start_offset: byte offset in the batch at which execution starts + * @is_master: is the submitting process the drm master? + * + * Parses the specified batch buffer looking for privilege violations as + * described in the overview. + * + * Return: non-zero if the parser finds violations or otherwise fails + */ +int i915_parse_cmds(struct intel_ring_buffer *ring, + struct drm_i915_gem_object *batch_obj, + u32 batch_start_offset, + bool is_master) +{ + int ret = 0; + u32 *cmd, *batch_base, *batch_end; + struct drm_i915_cmd_descriptor default_desc = { 0 }; + int needs_clflush = 0; + + ret = i915_gem_obj_prepare_shmem_read(batch_obj, &needs_clflush); + if (ret) { + DRM_DEBUG_DRIVER("CMD: failed to prep read\n"); + return ret; + } + + batch_base = vmap_batch(batch_obj); + if (!batch_base) { + DRM_DEBUG_DRIVER("CMD: Failed to vmap batch\n"); + i915_gem_object_unpin_pages(batch_obj); + return -ENOMEM; + } + + if (needs_clflush) + drm_clflush_virt_range((char *)batch_base, batch_obj->base.size); + + cmd = batch_base + (batch_start_offset / sizeof(*cmd)); + batch_end = cmd + (batch_obj->base.size / sizeof(*batch_end)); + + while (cmd < batch_end) { + const struct drm_i915_cmd_descriptor *desc; + u32 length; + + if (*cmd == MI_BATCH_BUFFER_END) + break; + + desc = find_cmd(ring, *cmd, &default_desc); + if (!desc) { + DRM_DEBUG_DRIVER("CMD: Unrecognized command: 0x%08X\n", + *cmd); + ret = -EINVAL; + break; + } + + if (desc->flags & CMD_DESC_FIXED) + length = desc->length.fixed; + else + length = ((*cmd & desc->length.mask) + LENGTH_BIAS); + + if ((batch_end - cmd) < length) { + DRM_DEBUG_DRIVER("CMD: Command length exceeds batch length: 0x%08X length=%d batchlen=%ld\n", + *cmd, + length, + batch_end - cmd); + ret = -EINVAL; + break; + } + + if (desc->flags & CMD_DESC_REJECT) { + DRM_DEBUG_DRIVER("CMD: Rejected command: 0x%08X\n", *cmd); + ret = -EINVAL; + break; + } + + if ((desc->flags & CMD_DESC_MASTER) && !is_master) { + DRM_DEBUG_DRIVER("CMD: Rejected master-only command: 0x%08X\n", + *cmd); + ret = -EINVAL; + break; + } + + if (desc->flags & CMD_DESC_REGISTER) { + u32 reg_addr = cmd[desc->reg.offset] & desc->reg.mask; + + if (!valid_reg(ring->reg_table, + ring->reg_count, reg_addr)) { + if (!is_master || + !valid_reg(ring->master_reg_table, + ring->master_reg_count, + reg_addr)) { + DRM_DEBUG_DRIVER("CMD: Rejected register 0x%08X in command: 0x%08X (ring=%d)\n", + reg_addr, + *cmd, + ring->id); + ret = -EINVAL; + break; + } + } + } + + if (desc->flags & CMD_DESC_BITMASK) { + int i; + + for (i = 0; i < MAX_CMD_DESC_BITMASKS; i++) { + u32 dword; + + if (desc->bits[i].mask == 0) + break; + + dword = cmd[desc->bits[i].offset] & + desc->bits[i].mask; + + if (dword != desc->bits[i].expected) { + DRM_DEBUG_DRIVER("CMD: Rejected command 0x%08X for bitmask 0x%08X (exp=0x%08X act=0x%08X) (ring=%d)\n", + *cmd, + desc->bits[i].mask, + desc->bits[i].expected, + dword, ring->id); + ret = -EINVAL; + break; + } + } + + if (ret) + break; + } + + cmd += length; + } + + if (cmd >= batch_end) { + DRM_DEBUG_DRIVER("CMD: Got to the end of the buffer w/o a BBE cmd!\n"); + ret = -EINVAL; + } + + vunmap(batch_base); + + i915_gem_object_unpin_pages(batch_obj); + + return ret; +} diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ef386bf9cecd..7b6dfdfb2d8d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1849,6 +1849,90 @@ struct drm_i915_file_private { atomic_t rps_wait_boost; }; +/* + * A command that requires special handling by the command parser. + */ +struct drm_i915_cmd_descriptor { + /* + * Flags describing how the command parser processes the command. + * + * CMD_DESC_FIXED: The command has a fixed length if this is set, + * a length mask if not set + * CMD_DESC_SKIP: The command is allowed but does not follow the + * standard length encoding for the opcode range in + * which it falls + * CMD_DESC_REJECT: The command is never allowed + * CMD_DESC_REGISTER: The command should be checked against the + * register whitelist for the appropriate ring + * CMD_DESC_MASTER: The command is allowed if the submitting process + * is the DRM master + */ + u32 flags; +#define CMD_DESC_FIXED (1<<0) +#define CMD_DESC_SKIP (1<<1) +#define CMD_DESC_REJECT (1<<2) +#define CMD_DESC_REGISTER (1<<3) +#define CMD_DESC_BITMASK (1<<4) +#define CMD_DESC_MASTER (1<<5) + + /* + * The command's unique identification bits and the bitmask to get them. + * This isn't strictly the opcode field as defined in the spec and may + * also include type, subtype, and/or subop fields. + */ + struct { + u32 value; + u32 mask; + } cmd; + + /* + * The command's length. The command is either fixed length (i.e. does + * not include a length field) or has a length field mask. The flag + * CMD_DESC_FIXED indicates a fixed length. Otherwise, the command has + * a length mask. All command entries in a command table must include + * length information. + */ + union { + u32 fixed; + u32 mask; + } length; + + /* + * Describes where to find a register address in the command to check + * against the ring's register whitelist. Only valid if flags has the + * CMD_DESC_REGISTER bit set. + */ + struct { + u32 offset; + u32 mask; + } reg; + +#define MAX_CMD_DESC_BITMASKS 3 + /* + * Describes command checks where a particular dword is masked and + * compared against an expected value. If the command does not match + * the expected value, the parser rejects it. Only valid if flags has + * the CMD_DESC_BITMASK bit set. Only entries where mask is non-zero + * are valid. + */ + struct { + u32 offset; + u32 mask; + u32 expected; + } bits[MAX_CMD_DESC_BITMASKS]; +}; + +/* + * A table of commands requiring special handling by the command parser. + * + * Each ring has an array of tables. Each table consists of an array of command + * descriptors, which must be sorted with command opcodes in ascending order. + */ +struct drm_i915_cmd_table { + const struct drm_i915_cmd_descriptor *table; + int count; +}; + #define INTEL_INFO(dev) (&to_i915(dev)->info) #define IS_I830(dev) ((dev)->pdev->device == 0x3577) @@ -2003,6 +2087,7 @@ struct i915_params { int enable_pc8; int pc8_timeout; int invert_brightness; + int enable_cmd_parser; /* leave bools at the end to not create holes */ bool enable_hangcheck; bool fastboot; @@ -2480,6 +2565,14 @@ void i915_destroy_error_state(struct drm_device *dev); void i915_get_extra_instdone(struct drm_device *dev, uint32_t *instdone); const char *i915_cache_level_str(int type); +/* i915_cmd_parser.c */ +void i915_cmd_parser_init_ring(struct intel_ring_buffer *ring); +bool i915_needs_cmd_parser(struct intel_ring_buffer *ring); +int i915_parse_cmds(struct intel_ring_buffer *ring, + struct drm_i915_gem_object *batch_obj, + u32 batch_start_offset, + bool is_master); + /* i915_suspend.c */ extern int i915_save_state(struct drm_device *dev); extern int i915_restore_state(struct drm_device *dev); diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index d7229ad2bd22..3851a1b1dc88 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1182,6 +1182,24 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, } batch_obj->base.pending_read_domains |= I915_GEM_DOMAIN_COMMAND; + if (i915_needs_cmd_parser(ring)) { + ret = i915_parse_cmds(ring, + batch_obj, + args->batch_start_offset, + file->is_master); + if (ret) + goto err; + + /* + * XXX: Actually do this when enabling batch copy... + * + * Set the DISPATCH_SECURE bit to remove the NON_SECURE bit + * from MI_BATCH_BUFFER_START commands issued in the + * dispatch_execbuffer implementations. We specifically don't + * want that set when the command parser is enabled. + */ + } + /* snb/ivb/vlv conflate the "batch in ppgtt" bit with the "non-secure * batch" bit. Hence we need to pin secure batches into the global gtt. * hsw should have this fixed, but bdw mucks it up again. */ diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c index 3b482585c5ae..a66ffb652bee 100644 --- a/drivers/gpu/drm/i915/i915_params.c +++ b/drivers/gpu/drm/i915/i915_params.c @@ -48,6 +48,7 @@ struct i915_params i915 __read_mostly = { .reset = true, .invert_brightness = 0, .disable_display = 0, + .enable_cmd_parser = 0, }; module_param_named(modeset, i915.modeset, int, 0400); @@ -157,3 +158,7 @@ MODULE_PARM_DESC(invert_brightness, module_param_named(disable_display, i915.disable_display, bool, 0600); MODULE_PARM_DESC(disable_display, "Disable display (default: false)"); + +module_param_named(enable_cmd_parser, i915.enable_cmd_parser, int, 0600); +MODULE_PARM_DESC(enable_cmd_parser, + "Enable command parsing (1=enabled, 0=disabled [default])"); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index b719385c4511..146609ab42bb 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -174,6 +174,18 @@ #define VGA_CR_INDEX_CGA 0x3d4 #define VGA_CR_DATA_CGA 0x3d5 +/* + * Instruction field definitions used by the command parser + */ +#define INSTR_CLIENT_SHIFT 29 +#define INSTR_CLIENT_MASK 0xE0000000 +#define INSTR_MI_CLIENT 0x0 +#define INSTR_BC_CLIENT 0x2 +#define INSTR_RC_CLIENT 0x3 +#define INSTR_SUBCLIENT_SHIFT 27 +#define INSTR_SUBCLIENT_MASK 0x18000000 +#define INSTR_MEDIA_SUBCLIENT 0x2 + /* * Memory interface instructions used by the kernel */ diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index b2d4f4852c98..5629cc7a7732 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1388,6 +1388,8 @@ static int intel_init_ring_buffer(struct drm_device *dev, if (IS_I830(ring->dev) || IS_845G(ring->dev)) ring->effective_size -= 128; + i915_cmd_parser_init_ring(ring); + return 0; err_unmap: diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 38c757e136dc..9d4c3b1182e9 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -164,6 +164,38 @@ struct intel_ring_buffer { u32 gtt_offset; volatile u32 *cpu_page; } scratch; + + /* + * Tables of commands the command parser needs to know about + * for this ring. + */ + const struct drm_i915_cmd_table *cmd_tables; + int cmd_table_count; + + /* + * Table of registers allowed in commands that read/write registers. + */ + const u32 *reg_table; + int reg_count; + + /* + * Table of registers allowed in commands that read/write registers, but + * only from the DRM master. + */ + const u32 *master_reg_table; + int master_reg_count; + + /* + * Returns the bitmask for the length field of the specified command. + * Return 0 for an unrecognized/invalid command. + * + * If the command parser finds an entry for a command in the ring's + * cmd_tables, it gets the command's length based on the table entry. + * If not, it calls this function to determine the per-ring length field + * encoding for the command (i.e. certain opcode ranges use certain bits + * to encode the command length in the header). + */ + u32 (*get_cmd_length_mask)(u32 cmd_header); }; static inline bool -- cgit From 50227e1cae118562b8b6230e31bca84870cad27e Mon Sep 17 00:00:00 2001 From: Jani Nikula Date: Mon, 31 Mar 2014 14:27:21 +0300 Subject: drm/i915: prefer struct drm_i915_private to drm_i915_private_t Remove the rest of the references to drm_i915_private_t. No functional changes. Signed-off-by: Jani Nikula [danvet: Drop hunk in i915_cmd_parser.c] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 6 +++--- drivers/gpu/drm/i915/i915_gem_debug.c | 2 +- drivers/gpu/drm/i915/i915_gem_evict.c | 4 ++-- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 4 ++-- drivers/gpu/drm/i915/i915_gem_gtt.c | 4 ++-- drivers/gpu/drm/i915/i915_gem_tiling.c | 6 +++--- drivers/gpu/drm/i915/i915_gpu_error.c | 2 +- drivers/gpu/drm/i915/intel_pm.c | 4 ++-- 9 files changed, 17 insertions(+), 17 deletions(-) (limited to 'drivers/gpu/drm/i915/i915_gem_execbuffer.c') diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index fa5d0ed76378..11f77a8e4c4d 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -662,7 +662,7 @@ int i915_resume(struct drm_device *dev) */ int i915_reset(struct drm_device *dev) { - drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = dev->dev_private; bool simulated; int ret; diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 8d10f220e044..0905cd915589 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2128,11 +2128,11 @@ extern void intel_uncore_check_errors(struct drm_device *dev); extern void intel_uncore_fini(struct drm_device *dev); void -i915_enable_pipestat(drm_i915_private_t *dev_priv, enum pipe pipe, +i915_enable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe, u32 status_mask); void -i915_disable_pipestat(drm_i915_private_t *dev_priv, enum pipe pipe, +i915_disable_pipestat(struct drm_i915_private *dev_priv, enum pipe pipe, u32 status_mask); void valleyview_enable_display_irqs(struct drm_i915_private *dev_priv); @@ -2502,7 +2502,7 @@ void i915_gem_object_release_stolen(struct drm_i915_gem_object *obj); /* i915_gem_tiling.c */ static inline bool i915_gem_object_needs_bit17_swizzle(struct drm_i915_gem_object *obj) { - drm_i915_private_t *dev_priv = obj->base.dev->dev_private; + struct drm_i915_private *dev_priv = obj->base.dev->dev_private; return dev_priv->mm.bit_6_swizzle_x == I915_BIT_6_SWIZZLE_9_10_17 && obj->tiling_mode != I915_TILING_NONE; diff --git a/drivers/gpu/drm/i915/i915_gem_debug.c b/drivers/gpu/drm/i915/i915_gem_debug.c index 775d506b3208..f462d1b51d97 100644 --- a/drivers/gpu/drm/i915/i915_gem_debug.c +++ b/drivers/gpu/drm/i915/i915_gem_debug.c @@ -34,7 +34,7 @@ int i915_verify_lists(struct drm_device *dev) { static int warned; - drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj; int err = 0; diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c index 8a78f7885cba..75fca63dc8c1 100644 --- a/drivers/gpu/drm/i915/i915_gem_evict.c +++ b/drivers/gpu/drm/i915/i915_gem_evict.c @@ -70,7 +70,7 @@ i915_gem_evict_something(struct drm_device *dev, struct i915_address_space *vm, int min_size, unsigned alignment, unsigned cache_level, unsigned flags) { - drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = dev->dev_private; struct list_head eviction_list, unwind_list; struct i915_vma *vma; int ret = 0; @@ -243,7 +243,7 @@ int i915_gem_evict_vm(struct i915_address_space *vm, bool do_idle) int i915_gem_evict_everything(struct drm_device *dev) { - drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = dev->dev_private; struct i915_address_space *vm; bool lists_empty = true; int ret; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 3851a1b1dc88..7447160155a3 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -978,7 +978,7 @@ static int i915_reset_gen7_sol_offsets(struct drm_device *dev, struct intel_ring_buffer *ring) { - drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = dev->dev_private; int ret, i; if (!IS_GEN7(dev) || ring != &dev_priv->ring[RCS]) @@ -1005,7 +1005,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, struct drm_i915_gem_execbuffer2 *args, struct drm_i915_gem_exec_object2 *exec) { - drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = dev->dev_private; struct eb_vmas *eb; struct drm_i915_gem_object *batch_obj; struct drm_clip_rect *cliprects = NULL; diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index ae6919c8c522..3b1f621f1009 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -888,7 +888,7 @@ err_out: static int gen7_ppgtt_enable(struct i915_hw_ppgtt *ppgtt) { struct drm_device *dev = ppgtt->base.dev; - drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = dev->dev_private; struct intel_ring_buffer *ring; uint32_t ecochk, ecobits; int i; @@ -927,7 +927,7 @@ static int gen7_ppgtt_enable(struct i915_hw_ppgtt *ppgtt) static int gen6_ppgtt_enable(struct i915_hw_ppgtt *ppgtt) { struct drm_device *dev = ppgtt->base.dev; - drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = dev->dev_private; struct intel_ring_buffer *ring; uint32_t ecochk, gab_ctl, ecobits; int i; diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c index eb993584aa6b..cb150e8b4336 100644 --- a/drivers/gpu/drm/i915/i915_gem_tiling.c +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c @@ -87,7 +87,7 @@ void i915_gem_detect_bit_6_swizzle(struct drm_device *dev) { - drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = dev->dev_private; uint32_t swizzle_x = I915_BIT_6_SWIZZLE_UNKNOWN; uint32_t swizzle_y = I915_BIT_6_SWIZZLE_UNKNOWN; @@ -294,7 +294,7 @@ i915_gem_set_tiling(struct drm_device *dev, void *data, struct drm_file *file) { struct drm_i915_gem_set_tiling *args = data; - drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj; int ret = 0; @@ -415,7 +415,7 @@ i915_gem_get_tiling(struct drm_device *dev, void *data, struct drm_file *file) { struct drm_i915_gem_get_tiling *args = data; - drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_gem_object *obj; obj = to_intel_bo(drm_gem_object_lookup(dev, file, args->handle)); diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 7f5dd38eb33d..12f1d43b2d68 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -322,7 +322,7 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m, const struct i915_error_state_file_priv *error_priv) { struct drm_device *dev = error_priv->dev; - drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = dev->dev_private; struct drm_i915_error_state *error = error_priv->error; int i, j, offset, elt; int max_hangcheck_score; diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index e9df08a55441..38a68cea5ed7 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -623,7 +623,7 @@ out_disable: static void i915_pineview_get_mem_freq(struct drm_device *dev) { - drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = dev->dev_private; u32 tmp; tmp = I915_READ(CLKCFG); @@ -662,7 +662,7 @@ static void i915_pineview_get_mem_freq(struct drm_device *dev) static void i915_ironlake_get_mem_freq(struct drm_device *dev) { - drm_i915_private_t *dev_priv = dev->dev_private; + struct drm_i915_private *dev_priv = dev->dev_private; u16 ddrpll, csipll; ddrpll = I915_READ16(DDRMPLL1); -- cgit