[Intel-gfx] [PATCH] drm/i915: Make 48bit full ppgtt configuration generic (v2)

[Bob Paauwe]

On Thu, 6 Sep 2018 21:08:33 +0100
Chris Wilson <chris at chris-wilson.co.uk> wrote:

> Quoting Bob Paauwe (2018-09-06 21:04:09)
> > @@ -1647,9 +1647,10 @@ static struct i915_hw_ppgtt *gen8_ppgtt_create(struct drm_i915_private *i915)
> >         ppgtt->vm.i915 = i915;
> >         ppgtt->vm.dma = &i915->drm.pdev->dev;
> >  
> > -       ppgtt->vm.total = USES_FULL_48BIT_PPGTT(i915) ?
> > -               1ULL << 48 :
> > -               1ULL << 32;
> > +       if ((i915_modparams.enable_ppgtt < 3) && USES_FULL_4LVL_PPGTT(i915))  
> (brackets (because(?))
habit mostly. 

> 
> > +               ppgtt->vm.total = 1ULL << 32;
> > +       else
> > +               ppgtt->vm.total = 1ULL << i915->info.full_ppgtt_bits;  
> 
> How about
> 
> ppgtt->vm.total = BIT_ULL(i915->info.full_ppgtt_bits);
> if (i915_modparams.enable_ppgtt < 3)
> 	ppgtt->vm.total = min(ppgtt->vm.total, SZ_4G);

That looks a bit cleaner, thanks!

> 
> Although let me complain loudly about introducing more modparams.

I didn't introduce a modparam, enable.ppgtt is an existing modparam
and it's currently able to "downgrade" an extended address range device
to use only 32 bits by setting it to 2.  I'm just trying to keep that
existing behavior. 

In the current code, setting enable_ppgtt=2 sets USES_FULL_PPGTT to
true and USES_FULL_48BIT_PPGTT to false.

If that existing behavior is not desired, I can rework this to remove
enable_ppgtt and have this use just the appropriate device info
settings.  It does get a bit more complicated as there are other parts
of the code that rely on the enable_ppgtt value today.

> 
> Please no. If you want to configure it, do so at runtime via context
> parameters or creation.
> -Chris


-- 
--
Bob Paauwe                  
Bob.J.Paauwe at intel.com
IOTG / PED Software Organization
Intel Corp.  Folsom, CA
(916) 356-6193