On Thu, 6 Sep 2018 21:08:33 +0100 Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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@xxxxxxxxx IOTG / PED Software Organization Intel Corp. Folsom, CA (916) 356-6193 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
