On ti, 2016-08-16 at 11:42 +0100, Chris Wilson wrote: > @@ -360,8 +361,18 @@ static void reloc_cache_fini(struct reloc_cache *cache) > kunmap_atomic(vaddr); > i915_gem_object_unpin_pages((struct drm_i915_gem_object *)cache->node.mm); > } else { > + wmb(); Why wmb() here? In other spot it's inside if (cache->node.allocated)? This alters the original code path, too. > io_mapping_unmap_atomic(vaddr); > - i915_vma_unpin((struct i915_vma *)cache->node.mm); > + if (cache->node.allocated) { > + struct i915_ggtt *ggtt = &cache->i915->ggtt; > + > + ggtt->base.clear_range(&ggtt->base, > + cache->node.start, > + cache->node.size, > + true); > + drm_mm_remove_node(&cache->node); > + } else > + i915_vma_unpin((struct i915_vma *)cache->node.mm); Pretty sure this needs braces too; "This does not apply if only one branch of a conditional statement is a single statement; in the latter case use braces in both branches:" > @@ -415,21 +437,38 @@ static void *reloc_iomap(struct drm_i915_gem_object *obj, > > vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, > PIN_MAPPABLE | PIN_NONBLOCK); > - if (IS_ERR(vma)) > - return NULL; > + if (IS_ERR(vma)) { > + memset(&cache->node, 0, sizeof(cache->node)); > + ret = drm_mm_insert_node_in_range_generic > + (&ggtt->base.mm, &cache->node, That brace sure needs to be on the previous line. With above fixes; Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx