Re: [PATCH] drm/i915: Infrastructure for supporting different GGTT views per object

From: Daniel Vetter <daniel@ffwll.ch>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Infrastructure for supporting different GGTT views per object
Date: Mon, 1 Dec 2014 17:01:42 +0100	[thread overview]
Message-ID: <20141201160142.GV32117@phenom.ffwll.local> (raw)
In-Reply-To: <547C7F45.2030001@linux.intel.com>

On Mon, Dec 01, 2014 at 02:46:29PM +0000, Tvrtko Ursulin wrote:
> 
> On 12/01/2014 11:32 AM, Tvrtko Ursulin wrote:
> >>>@@ -5430,9 +5434,12 @@ struct i915_vma *i915_gem_obj_to_ggtt(struct
> >>>drm_i915_gem_object *obj)
> >>>  {
> >>>      struct i915_vma *vma;
> >>>
> >>>-    vma = list_first_entry(&obj->vma_list, typeof(*vma), vma_link);
> >>>-    if (vma->vm != i915_obj_to_ggtt(obj))
> >>>-        return NULL;
> >>>+    list_for_each_entry(vma, &obj->vma_list, vma_link) {
> >>>+        if (vma->vm != i915_obj_to_ggtt(obj))
> >>>+            continue;
> >>>+        if (vma->ggtt_view.type == I915_GGTT_VIEW_NORMAL)
> >>>+            return vma;
> >>>+    }
> >>
> >>We fairly put the ggtt vma into the head of the list. Imo better to keep
> >>the head slot reserved for the ggtt normal view, might be some random
> >>code
> >>relying upon this.
> >
> >Ok.
> 
> Although on a second thought - I am not sure this makes sense since
> alternative views can exist without the normal one. Thoughts?

Yeah, hence we need to put the normal ggtt view at the front and
everything else at the back. I'm just somewhat afraid of something
expecting the normal ggtt view to be the first one and which then
accidentally breaks.

But if you think this is too much fuzz then please split this change out
into a separate patch (i.e. the change to the lookup loop + no longer
inserting ggtt vmas a the front). That way when any regression bisects to
this patch it's clear what's going on.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx