On Tue, Oct 06, 2015 at 11:39:56AM +0100, Chris Wilson wrote:
> Since the remove of the pin-ioctl, we only care about not changing the
> cache level on buffers pinned to the hardware as indicated by
> obj->pin_display. So we can safely replace i915_gem_object_is_pinned()
> here with a plain obj->pin_display check. During rebinding, we will check
> sanity checks in case vma->pin_count is erroneously set.
>
> At the same time, we can micro-optimise GTT mmap() behaviour since we
> only need to relinquish the mmaps before Sandybridge.
Actual condition is !LLC so would need to be updated (and split out imo).
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 40 ++++++++++++++++++++++++----------------
> 1 file changed, 24 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index d4a3bdf0c5b6..2b8ed7a2faab 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3629,31 +3629,34 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
> {
> struct drm_device *dev = obj->base.dev;
> struct i915_vma *vma, *next;
> + bool bound = false;
> int ret = 0;
>
> if (obj->cache_level == cache_level)
> goto out;
>
> - if (i915_gem_obj_is_pinned(obj)) {
> - DRM_DEBUG("can not change the cache level of pinned objects\n");
> - return -EBUSY;
> - }
> -
> list_for_each_entry_safe(vma, next, &obj->vma_list, vma_link) {
> + if (!drm_mm_node_allocated(&vma->node))
> + continue;
> +
> + if (vma->pin_count) {
> + DRM_DEBUG("can not change the cache level of pinned objects\n");
> + return -EBUSY;
> + }
> +
> if (!i915_gem_valid_gtt_space(vma, cache_level)) {
> ret = i915_vma_unbind(vma);
> if (ret)
> return ret;
> - }
> + } else
> + bound = true;
> }
>
> - if (i915_gem_obj_bound_any(obj)) {
> + if (bound) {
> ret = i915_gem_object_wait_rendering(obj, false);
> if (ret)
> return ret;
Shouldn't the below be split out into a separate patch? And maybe for
paranoia keep calling finish_gtt but restrict it to !LLC && snooped like
you do below.
-Daniel
>
> - i915_gem_object_finish_gtt(obj);
> -
> /* Before SandyBridge, you could not use tiling or fence
> * registers with snooped memory, so relinquish any fences
> * currently pointing to our region in the aperture.
> @@ -3664,13 +3667,18 @@ int i915_gem_object_set_cache_level(struct drm_i915_gem_object *obj,
> return ret;
> }
>
> - list_for_each_entry(vma, &obj->vma_list, vma_link)
> - if (drm_mm_node_allocated(&vma->node)) {
> - ret = i915_vma_bind(vma, cache_level,
> - PIN_UPDATE);
> - if (ret)
> - return ret;
> - }
> + /* Access to snoopable pages through the GTT is incoherent. */
> + if (cache_level != I915_CACHE_NONE && !HAS_LLC(dev))
> + i915_gem_release_mmap(obj);
> +
> + list_for_each_entry(vma, &obj->vma_list, vma_link) {
> + if (!drm_mm_node_allocated(&vma->node))
> + continue;
> +
> + ret = i915_vma_bind(vma, cache_level, PIN_UPDATE);
> + if (ret)
> + return ret;
> + }
> }
>
> list_for_each_entry(vma, &obj->vma_list, vma_link)
> --
> 2.6.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
