2014/1/16 Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>:
> An object can only have an active gtt mapping if it is currently bound
> into the global gtt. Therefore we can simply walk the list of all bound
> objects and check the flag upon those for an active gtt mapping.
>
> From commit 48018a57a8f5900e7e53ffaa0adeb784095accfb
> Author: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> Date: Fri Dec 13 15:22:31 2013 -0200
>
> drm/i915: release the GTT mmaps when going into D3
>
> Also note that the WARN is inappropriate for this function as GPU
> activity is orthogonal to GTT mmap status. Rather it is the caller that
> relies upon this condition and so it should assert that the GPU is idle
> itself.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>
> Cc: Daniel Vetter <daniel.vetter@xxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 25 +++++++++----------------
> 1 file changed, 9 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 39c25ebc36b7..74f583fcab3e 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1562,22 +1562,6 @@ out:
> return ret;
> }
>
> -void i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv)
> -{
> - struct i915_vma *vma;
> -
> - /*
> - * Only the global gtt is relevant for gtt memory mappings, so restrict
> - * list traversal to objects bound into the global address space. Note
> - * that the active list should be empty, but better safe than sorry.
> - */
> - WARN_ON(!list_empty(&dev_priv->gtt.base.active_list));
> - list_for_each_entry(vma, &dev_priv->gtt.base.active_list, mm_list)
> - i915_gem_release_mmap(vma->obj);
> - list_for_each_entry(vma, &dev_priv->gtt.base.inactive_list, mm_list)
> - i915_gem_release_mmap(vma->obj);
> -}
> -
> /**
> * i915_gem_release_mmap - remove physical page mappings
> * @obj: obj in question
> @@ -1602,6 +1586,15 @@ i915_gem_release_mmap(struct drm_i915_gem_object *obj)
> obj->fault_mappable = false;
> }
>
> +void
> +i915_gem_release_all_mmaps(struct drm_i915_private *dev_priv)
> +{
> + struct drm_i915_gem_object *obj;
> +
> + list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list)
> + i915_gem_release_mmap(obj);
> +}
> +
This was exactly what I wrote in the first version:
http://lists.freedesktop.org/archives/intel-gfx/2013-November/036310.html
I had written this based on a description by Daniel. I asked his help
due to my lack of experience with this code.
But then later he changed his mind and guided me to implement what is
currently merged today:
http://lists.freedesktop.org/archives/intel-gfx/2013-December/037263.html
So your patch does work and it does pass the pm_pc8.c test suite,
because I tested it in the past. Based on that, you have my
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
I also counted the amount of times we call i915_gem_release_mmap() on
both versions, and for the pm_pc8.c test suite, as far as I remember,
it was the same. But, of course, pm_pc8.c doesn't really reflect a
real use-case environment.
But I think you and Daniel should discuss and really decide which
version you want merged. I don't have any strong opinions here.
Thanks,
Paulo
> uint32_t
> i915_gem_get_gtt_size(struct drm_device *dev, uint32_t size, int tiling_mode)
> {
> --
> 1.8.5.2
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Paulo Zanoni
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
