Re: [PATCH 01/10] drm/i915: Move map-and-fenceable tracking to the VMA

Chris Wilson
2016-08-15T08:14:35Z

On Mon, Aug 15, 2016 at 11:03:32AM +0300, Joonas Lahtinen wrote:
> On pe, 2016-08-12 at 11:28 +0100, Chris Wilson wrote:
> Not sure if there should be a comment that for 1:1 mappings vma->pages
> is just obj->pages so it should not be freed. Or maybe you could even
> make the test if vma->pages != vma->obj->pages? More self-documenting.

I contemplated making this vma->pages != vma->obj->pages as well in
light of the recent changes, will do.
>
> > + if (vma->ggtt_view.type != I915_GGTT_VIEW_NORMAL) {
> > + sg_free_table(vma->pages);
> > + kfree(vma->pages);
> >   }
> >   vma->pages = NULL;
>
> <SNIP>
>
> > @@ -3693,7 +3687,10 @@ void __i915_vma_set_map_and_fenceable(struct i915_vma *vma)
>
> This might also clear, so function name should be
> update_map_and_fenceable, really.

update/compute either is a fine TODO ;)

> > @@ -2262,11 +2262,11 @@ void intel_unpin_fb_obj(struct drm_framebuffer *fb, unsigned int rotation)
> >   WARN_ON(!mutex_is_locked(&obj->base.dev->struct_mutex));
> >
> >   intel_fill_fb_ggtt_view(&view, fb, rotation);
> > + vma = i915_gem_object_to_ggtt(obj, &view);
> >
> > - if (view.type == I915_GGTT_VIEW_NORMAL)
> > + if (i915_vma_is_map_and_fenceable(vma))
> >   i915_gem_object_unpin_fence(obj);
> >
> > - vma = i915_gem_object_to_ggtt(obj, &view);
> >   i915_gem_object_unpin_from_display_plane(vma);
>
> This did not have NULL protection previously either, so should be OK.

Yup, the long goal here is to pass in the vma.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre