On 12/01/2014 04:01 PM, Daniel Vetter wrote:
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.
No, my point was that we can't guarantee that since the first entry will
be an alternative view when it is the only item on the list. So in the
light of that does it make sense to bother with this special casing at all?
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.
To double check if I understand what you mean, you lost me with "fuzz":
If you agree with my comment above, then the recommendation is for
another prep patch to do what you say here? Ie. remove the special case
for GGTT VMA list_add ?
i915_gem_obj_ti_ggtt can remain untouched in that prep patch, in my
view, since it will be the only GGTT VMA, no?
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
