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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
