Re: [PATCH 09/29] drm/i915: thread address space through execbuf

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Jul 31, 2013 at 05:00:02PM -0700, Ben Widawsky wrote:
> This represents the first half of hooking up VMs to execbuf. Here we
> basically pass an address space all around to the different internal
> functions. It should be much more readable, and have less risk than the
> second half, which begins switching over to using VMAs instead of an
> obj,vm.
> 
> The overall series echoes this style of, "add a VM, then make it smart
> later"
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> ---

[snip]

> @@ -477,6 +483,7 @@ i915_gem_execbuffer_unreserve_object(struct drm_i915_gem_object *obj)
>  static int
>  i915_gem_execbuffer_reserve(struct intel_ring_buffer *ring,
>  			    struct list_head *objects,
> +			    struct i915_address_space *vm,
>  			    bool *need_relocs)
>  {
>  	struct drm_i915_gem_object *obj;
> @@ -531,32 +538,37 @@ i915_gem_execbuffer_reserve(struct intel_ring_buffer *ring,
>  		list_for_each_entry(obj, objects, exec_list) {
>  			struct drm_i915_gem_exec_object2 *entry = obj->exec_entry;
>  			bool need_fence, need_mappable;
> +			u32 obj_offset;
>  
> -			if (!i915_gem_obj_ggtt_bound(obj))
> +			if (!i915_gem_obj_bound(obj, vm))
>  				continue;
>  
> +			obj_offset = i915_gem_obj_offset(obj, vm);
>  			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(obj);
>  
> +			BUG_ON((need_mappable || need_fence) &&
> +			       !i915_is_ggtt(vm));

No BUG_ON if the error isn't fatal, please. I've fixed this up to a
WARN_ON while applying.

I know that you prefer BUG_ON for developing, but when we hit one of these
in the wild it's a royal pain to debug. But please either switch to WARNs
when submitting the patches or (imo preferred) get into the habit of
grepping dmesg for backtraces when testing.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch