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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
