[Intel-gfx] [PATCH 2/3] drm/i915/selftests: Use full release for local ppgtt allocation

Chris Wilson chris at chris-wilson.co.uk
Thu Jul 5 10:02:29 UTC 2018


Quoting Tvrtko Ursulin (2018-07-05 10:55:23)
> 
> On 05/07/2018 10:50, Tvrtko Ursulin wrote:
> > 
> > On 05/07/2018 07:56, Chris Wilson wrote:
> >> We can now use the full release mechanism (i915_ppgtt_put) for our local
> >> ppgtt allocation in igt_ppgtt_alloc.
> >>
> >> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> >> ---
> >>   drivers/gpu/drm/i915/selftests/i915_gem_gtt.c | 3 +--
> >>   1 file changed, 1 insertion(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c 
> >> b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> >> index 4bfb0537f9be..e108fe4e0fd9 100644
> >> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> >> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
> >> @@ -202,9 +202,8 @@ static int igt_ppgtt_alloc(void *arg)
> >>   err_ppgtt_cleanup:
> >>       mutex_lock(&dev_priv->drm.struct_mutex);
> >> -    ppgtt->vm.cleanup(&ppgtt->vm);
> >> +    i915_ppgtt_put(ppgtt);
> >>       mutex_unlock(&dev_priv->drm.struct_mutex);
> >> -    kfree(ppgtt);
> >>       return err;
> >>   }
> >>
> > 
> > Hm, it's not obvious why this works, if it does. This ppgtt it creates 
> > does not call kref_init on ppgtt->ref, or i915_address_space_init. 
> > Colour me confused.
> 
> I was looking at the too old tree, in the meantime you sneaked in v4 of 
> the wc stash patch which changes these things.

I thought it was clean and rushed before checking the selftests.
 
> Slightly unfortunate that common parts of the init are now duplicated..

I think we should just pull the kref into i915_address_space, then all
the common parts are inited together. The only reason it is not there is
to avoid refcounting ggtt, which I'm not that concerned about as we
should only be tweaking ppgtt krefs on the fly.
-Chris


More information about the Intel-gfx mailing list