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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to