Re: [PATCH v2 3/4] drm/i915: Introduce i915_vm_to_ggtt()

Chris Wilson
2016-04-18T14:02:18Z

On Mon, Apr 18, 2016 at 04:37:12PM +0300, Joonas Lahtinen wrote:
> On ma, 2016-04-18 at 12:14 +0100, Chris Wilson wrote:
> > In a couple of places, we have an i915_address_space that we know is
> > really an i915_ggtt that we want to use. Create an inline helper to
> > convert from the i915_address_space subclass into its container.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>
> Comment below.
>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>
> > Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_gtt.c | 15 +++++++++++----
> >  1 file changed, 11 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 9f165feb54ae..b3af2e808b49 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -93,6 +93,13 @@
> >   *
> >   */
> >
> > +static inline struct i915_ggtt *
> > +i915_vm_to_ggtt(struct i915_address_space *vm)
> > +{
> > + GEM_BUG_ON(!i915_is_ggtt(vm));
> > + return container_of(vm, struct i915_ggtt, base);
> > +}
> > +
> >  static int
> >  i915_get_ggtt_vma_pages(struct i915_vma *vma);
> >
> > @@ -2359,7 +2366,7 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
> >        enum i915_cache_level level, u32 unused)
> >  {
> >   struct drm_i915_private *dev_priv = to_i915(vm->dev);
> > - struct i915_ggtt *ggtt = &dev_priv->ggtt;
> > + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>
> I do not see this as a very necessary change, because we are going to
> have only one Global XYZ per device by definiton of "global".

I ultimately want to remove the dev_priv usage here. But it is also
useful for consistency with a couple of other ggtt PTE routines that do
not need the drm_i915_private pointer at all, that aren't yet upstream.
-Chris

--
Chris Wilson, Intel Open Source Technology Centre