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

Rodrigo Vivi
2018-09-06T21:12:38Z

On Thu, Sep 06, 2018 at 09:08:33PM +0100, Chris Wilson 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(?))
>
> > + 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);
>
> Although let me complain loudly about introducing more modparams.
>
> Please no. If you want to configure it, do so at runtime via context
> parameters or creation.

I agree with you, as well as Bob's approach apparently. His patch
is one step further to reduce the use of this parameter.

All the other work to kill it for good could come in follow-up patches imo.

> -Chris