[Intel-gfx] [PATCH 06/11] drm/i915: Store owning file on the i915_address_space

[Chris Wilson]

On Thu, Dec 17, 2015 at 11:52:06AM +0000, Tvrtko Ursulin wrote:
> >-	ret = __hw_ppgtt_init(dev, ppgtt);
> >+	ret = __hw_ppgtt_init(ppgtt, dev_priv);
> >  	if (ret == 0) {
> >  		kref_init(&ppgtt->ref);
> >  		i915_address_space_init(&ppgtt->base, dev_priv);
> >+		ppgtt->base.file = file_priv;
> 
> I would keep using file_priv since that's what's it's called all
> over the place but whatever.

Personally I have been working towards dropping the _priv from our
nomenclature inside our driver (only on the boundaries from drm do we
care about translating from drm objects to ours) - in a parallel fashion
to using crtc and crtc->base instead of intel_crtc / crtc.

> >diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> >index bae005a62cfc..4e9553ace33f 100644
> >--- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> >+++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> >@@ -273,12 +273,11 @@ struct i915_pml4 {
> >  struct i915_address_space {
> >  	struct drm_mm mm;
> >  	struct drm_device *dev;
> >+	struct drm_i915_file_private *file;
> 
> Suggest putting a comment documenting when it is NULL and when it is
> valid. Commit says so, but I think comment is also needed.

+       /* Every address space belongs to a struct file - except for the global
+        * GTT that is owned by the driver (and so @file is set to NULL). In
+        * principle, no information should leak from one context to another
+        * (or between files/processes etc) unless explicitly shared by the
+        * owner. Tracking the owner is important in order to free up per-file
+        * objects along with the file, to aide resource tracking, and to
+        * assign blame.
+        */
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre