[Intel-gfx] [PATCH 22/48] drm/i915: Use drm_mm for PPGTT PDEs

[Daniel Vetter]

On Tue, Mar 25, 2014 at 01:41:25PM +0000, Chris Wilson wrote:
> On Mon, Mar 24, 2014 at 01:02:57PM -0700, Ben Widawsky wrote:
> > On Mon, Mar 24, 2014 at 07:45:56PM +0000, Chris Wilson wrote:
> > > On Mon, Mar 24, 2014 at 12:36:23PM -0700, Ben Widawsky wrote:
> > > > On Thu, Mar 20, 2014 at 11:10:13AM +0000, Chris Wilson wrote:
> > > > > On Fri, Dec 06, 2013 at 02:11:55PM -0800, Ben Widawsky wrote:
> > > > > >  static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt)
> > > > > >  {
> > > > > > +#define GEN6_PD_ALIGN (PAGE_SIZE * 16)
> > > > > > +#define GEN6_PD_SIZE (GEN6_PPGTT_PD_ENTRIES * PAGE_SIZE)
> > > > > >  	struct drm_device *dev = ppgtt->base.dev;
> > > > > >  	struct drm_i915_private *dev_priv = dev->dev_private;
> > > > > > -	unsigned first_pd_entry_in_global_pt;
> > > > > > -	int i;
> > > > > > -	int ret = -ENOMEM;
> > > > > > +	int i, ret;
> > > > > >  
> > > > > > -	/* ppgtt PDEs reside in the global gtt pagetable, which has 512*1024
> > > > > > -	 * entries. For aliasing ppgtt support we just steal them at the end for
> > > > > > -	 * now. */
> > > > > > -	first_pd_entry_in_global_pt = gtt_total_entries(dev_priv->gtt);
> > > > > > +	/* PPGTT PDEs reside in the GGTT and consists of 512 entries. The
> > > > > > +	 * allocator works in address space sizes, so it's multiplied by page
> > > > > > +	 * size. We allocate at the top of the GTT to avoid fragmentation.
> > > > > > +	 */
> > > > > > +	BUG_ON(!drm_mm_initialized(&dev_priv->gtt.base.mm));
> > > > > > +	ret = drm_mm_insert_node_in_range_generic(&dev_priv->gtt.base.mm,
> > > > > > +						  &ppgtt->node, GEN6_PD_SIZE,
> > > > > > +						  GEN6_PD_ALIGN, 0,
> > > > > > +						  0, dev_priv->gtt.base.total,
> > > > > > +						  DRM_MM_SEARCH_DEFAULT);
> > > > > This could use the simpler drm_mm_insert_node_generic().
> > > > > -Chris
> > > > > 
> > > > 
> > > > Not with my [simple] workaround to not use offset 0, which Daniel
> > > > reverted. I think he has some hope that we'll actually be able to figure
> > > > out why we can't use offset 0 instead of just using the workaround.
> > > 
> > > You can simply reduce the drm_mm range...
> > > -Chris
> > > 
> > 
> > Yeah, that's a better solution. Patches welcome?
> 
> I thought I had mentioned this earlier, but alas couldn't find it in
> email. So here goes,
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index ee53551..0124e2c 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -1227,7 +1227,9 @@ int i915_gem_init_ppgtt(struct drm_device *dev, struct i915_hw_ppgtt *ppgtt)
>         if (!ret) {
>                 struct drm_i915_private *dev_priv = dev->dev_private;
>                 kref_init(&ppgtt->ref);
> -               drm_mm_init(&ppgtt->base.mm, ppgtt->base.start,
> +               /* Magic offset because we have no idea what's going on */
> +               drm_mm_init(&ppgtt->base.mm,
> +                           ppgtt->base.start + 4096,

Do we have bugzillas and stuff like backtraces, just for the record?

I'd still like to know what's going on here and what exactly blows up.
Iirc you've fixed some of the bogus assumptions in some of the igts ...
-Daniel

>                             ppgtt->base.total);
>                 i915_init_vm(dev_priv, &ppgtt->base);
>                 if (INTEL_INFO(dev)->gen < 8) {
> 
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch