[Intel-gfx] [PATCH 01/11] drm/i915: Move gtt and ppgtt under address space umbrella

[Ben Widawsky]

On Thu, Jul 11, 2013 at 04:57:30PM -0700, Ben Widawsky wrote:
> On Thu, Jul 11, 2013 at 02:14:06PM +0300, Imre Deak wrote:
> > On Mon, 2013-07-08 at 23:08 -0700, Ben Widawsky wrote:
> > > The GTT and PPGTT can be thought of more generally as GPU address
> > > spaces. Many of their actions (insert entries), state (LRU lists) and
> > > many of their characteristics (size), can be shared. Do that.
> > > 
> > > The change itself doesn't actually impact most of the VMA/VM rework
> > > coming up, it just fits in with the grand scheme. GGTT will usually be a
> > > special case where we either know an object must be in the GGTT (dislay
> > > engine, workarounds, etc.).
> > > 
> > > v2: Drop usage of i915_gtt_vm (Daniel)
> > > Make cleanup also part of the parent class (Ben)
> > > Modified commit msg
> > > Rebased
> > > 
> > > Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> > > ---
> > >  drivers/gpu/drm/i915/i915_debugfs.c |   4 +-
> > >  drivers/gpu/drm/i915/i915_dma.c     |   4 +-
> > >  drivers/gpu/drm/i915/i915_drv.h     |  57 ++++++-------
> > >  drivers/gpu/drm/i915/i915_gem.c     |   4 +-
> > >  drivers/gpu/drm/i915/i915_gem_gtt.c | 162 ++++++++++++++++++++----------------
> > >  5 files changed, 121 insertions(+), 110 deletions(-)
> > > 
> > >[...]
> > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > index 242d0f9..693115a 100644
> > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > > @@ -102,7 +102,7 @@ static gen6_gtt_pte_t hsw_pte_encode(dma_addr_t addr,
> > >  
> > >  static void gen6_write_pdes(struct i915_hw_ppgtt *ppgtt)
> > >  {
> > > -	struct drm_i915_private *dev_priv = ppgtt->dev->dev_private;
> > > +	struct drm_i915_private *dev_priv = ppgtt->base.dev->dev_private;
> > >  	gen6_gtt_pte_t __iomem *pd_addr;
> > >  	uint32_t pd_entry;
> > >  	int i;
> > > @@ -181,18 +181,18 @@ static int gen6_ppgtt_enable(struct drm_device *dev)
> > >  }
> > >  
> > >  /* PPGTT support for Sandybdrige/Gen6 and later */
> > > -static void gen6_ppgtt_clear_range(struct i915_hw_ppgtt *ppgtt,
> > > +static void gen6_ppgtt_clear_range(struct i915_address_space *vm,
> > >  				   unsigned first_entry,
> > >  				   unsigned num_entries)
> > >  {
> > > -	struct drm_i915_private *dev_priv = ppgtt->dev->dev_private;
> > > +	struct i915_hw_ppgtt *ppgtt =
> > > +		container_of(vm, struct i915_hw_ppgtt, base);
> > >  	gen6_gtt_pte_t *pt_vaddr, scratch_pte;
> > >  	unsigned act_pt = first_entry / I915_PPGTT_PT_ENTRIES;
> > >  	unsigned first_pte = first_entry % I915_PPGTT_PT_ENTRIES;
> > >  	unsigned last_pte, i;
> > >  
> > > -	scratch_pte = ppgtt->pte_encode(dev_priv->gtt.scratch.addr,
> > > -					I915_CACHE_LLC);
> > > +	scratch_pte = vm->pte_encode(vm->scratch.addr, I915_CACHE_LLC);
> > 
> > I only see ggtt's scratch page being initialized, but can't find the
> > corresponding init/teardown for ppgtt. Btw, why do we need separate
> > global/per-process scratch pages? (would be nice to add it to the commit
> > message)
> > 
> > --Imre
> > 
> 
> There is indeed a bug here, it existed somewhere, so I've mistakenly dropped
> it. Here is my local fix, which is what I had done previously.
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 552e4cb..c8130db 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -295,6 +295,7 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
>         ppgtt->base.clear_range = gen6_ppgtt_clear_range;
>         ppgtt->base.bind_object = gen6_ppgtt_bind_object;
>         ppgtt->base.cleanup = gen6_ppgtt_cleanup;
> +       ppgtt->base.scratch = dev_priv->gtt.base.scratch;
>         ppgtt->pt_pages = kzalloc(sizeof(struct page *)*ppgtt->num_pd_entries,
>                                   GFP_KERNEL);
>         if (!ppgtt->pt_pages)
> 
> 
> Not sure what you mean, there should be only 1 scratch page now.
>
I've updated my commit message to address what we discussed on IRC. The
VM has the scratch structure because I intend to have a scratch page per
PPGTT when we have full PPGTT.

Thanks for the insightful question ;-)
>
> 
> -- 
> Ben Widawsky, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center