| Message ID | 20180906200409.4069910-1-bob.j.paauwe@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Make 48bit full ppgtt configuration generic (v2) | expand |
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. -Chris
On Thu, 6 Sep 2018 21:08:33 +0100 Chris Wilson <chris@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
On Thu, Sep 06, 2018 at 01:04:09PM -0700, Bob Paauwe wrote: > 48 bit ppgtt device configuration is really just extended address > range full ppgtt and may actually be something other than 48 bits. > > Change USES_FULL_48BIT_PPGTT() to USES_FULL_4LVL_PPGTT() to better > describe that a 4 level walk table extended range PPGTT is being > used. Add a new device info field that specifies the number of > bits to prepare for cases where the range is not 32 or 48 bits. > > v2: keep USES_FULL_PPGTT() unchanged (Chris) > > Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> > CC: Rodrigo Vivi <rodrigo.vivi@intel.com> > CC: Michel Thierry <michel.thierry@intel.com> > CC: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/i915_gem_gtt.c | 19 ++++++++++--------- > drivers/gpu/drm/i915/i915_pci.c | 7 +++++-- > drivers/gpu/drm/i915/intel_device_info.h | 3 +++ > drivers/gpu/drm/i915/selftests/huge_pages.c | 2 +- > drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 ++ > 6 files changed, 22 insertions(+), 13 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 5a4da5b723fd..a367686fd735 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2569,7 +2569,7 @@ intel_info(const struct drm_i915_private *dev_priv) > > #define USES_PPGTT(dev_priv) (i915_modparams.enable_ppgtt) > #define USES_FULL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2) > -#define USES_FULL_48BIT_PPGTT(dev_priv) (i915_modparams.enable_ppgtt == 3) > +#define USES_FULL_4LVL_PPGTT(dev_priv) ((dev_priv)->info.full_ppgtt_bits > 32) > #define HAS_PAGE_SIZES(dev_priv, sizes) ({ \ > GEM_BUG_ON((sizes) == 0); \ > ((sizes) & ~(dev_priv)->info.page_sizes) == 0; \ > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > index eb0e446d6482..530a4c1452b3 100644 > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > @@ -137,18 +137,18 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, > int enable_ppgtt) > { > bool has_full_ppgtt; > - bool has_full_48bit_ppgtt; > + bool has_full_4lvl_ppgtt; > > if (!dev_priv->info.has_aliasing_ppgtt) > return 0; > > has_full_ppgtt = dev_priv->info.has_full_ppgtt; > - has_full_48bit_ppgtt = dev_priv->info.has_full_48bit_ppgtt; > + has_full_4lvl_ppgtt = USES_FULL_4LVL_PPGTT(dev_priv); > > if (intel_vgpu_active(dev_priv)) { > /* GVT-g has no support for 32bit ppgtt */ > has_full_ppgtt = false; > - has_full_48bit_ppgtt = intel_vgpu_has_full_48bit_ppgtt(dev_priv); > + has_full_4lvl_ppgtt = intel_vgpu_has_full_48bit_ppgtt(dev_priv); I wonder if we should already rename this function to match 4lvl instead of full and/or 48bit on it. > } > > /* > @@ -164,7 +164,7 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, > if (enable_ppgtt == 2 && has_full_ppgtt) > return 2; > > - if (enable_ppgtt == 3 && has_full_48bit_ppgtt) > + if (enable_ppgtt == 3 && has_full_4lvl_ppgtt) > return 3; > > /* Disable ppgtt on SNB if VT-d is on. */ > @@ -173,7 +173,7 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, > return 0; > } > > - if (has_full_48bit_ppgtt) > + if (has_full_4lvl_ppgtt) > return 3; > > if (has_full_ppgtt) > @@ -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)) > + ppgtt->vm.total = 1ULL << 32; > + else > + ppgtt->vm.total = 1ULL << i915->info.full_ppgtt_bits; > > /* > * From bdw, there is support for read-only pages in the PPGTT. > @@ -1788,7 +1789,7 @@ static void gen8_ppgtt_enable(struct drm_i915_private *dev_priv) > enum intel_engine_id id; > > for_each_engine(engine, dev_priv, id) { > - u32 four_level = USES_FULL_48BIT_PPGTT(dev_priv) ? > + u32 four_level = USES_FULL_4LVL_PPGTT(dev_priv) ? > GEN8_GFX_PPGTT_48B : 0; > I915_WRITE(RING_MODE_GEN7(engine), > _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE | four_level)); > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index d6f7b9fe1d26..a99c1f6de64e 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -299,6 +299,7 @@ static const struct intel_device_info intel_sandybridge_m_gt2_info = { > .has_rc6p = 1, \ > .has_aliasing_ppgtt = 1, \ > .has_full_ppgtt = 1, \ > + .full_ppgtt_bits = 32, \ > GEN_DEFAULT_PIPEOFFSETS, \ > GEN_DEFAULT_PAGE_SIZES, \ > IVB_CURSOR_OFFSETS > @@ -353,6 +354,7 @@ static const struct intel_device_info intel_valleyview_info = { > .has_hotplug = 1, > .has_aliasing_ppgtt = 1, > .has_full_ppgtt = 1, > + .full_ppgtt_bits = 32, \ > .has_snoop = true, > .has_coherent_ggtt = false, > .ring_mask = RENDER_RING | BSD_RING | BLT_RING, > @@ -399,7 +401,7 @@ static const struct intel_device_info intel_haswell_gt3_info = { > .page_sizes = I915_GTT_PAGE_SIZE_4K | \ > I915_GTT_PAGE_SIZE_2M, \ > .has_logical_ring_contexts = 1, \ > - .has_full_48bit_ppgtt = 1, \ > + .full_ppgtt_bits = 48, \ > .has_64bit_reloc = 1, \ > .has_reset_engine = 1 > > @@ -445,6 +447,7 @@ static const struct intel_device_info intel_cherryview_info = { > .has_gmch_display = 1, > .has_aliasing_ppgtt = 1, > .has_full_ppgtt = 1, > + .full_ppgtt_bits = 32, \ > .has_reset_engine = 1, > .has_snoop = true, > .has_coherent_ggtt = false, > @@ -520,7 +523,7 @@ static const struct intel_device_info intel_skylake_gt4_info = { > .has_guc = 1, \ > .has_aliasing_ppgtt = 1, \ > .has_full_ppgtt = 1, \ > - .has_full_48bit_ppgtt = 1, \ > + .full_ppgtt_bits = 48, \ > .has_reset_engine = 1, \ > .has_snoop = true, \ > .has_coherent_ggtt = false, \ > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h > index 6eecd64734d5..083590e98195 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.h > +++ b/drivers/gpu/drm/i915/intel_device_info.h > @@ -182,6 +182,9 @@ struct intel_device_info { > u16 degamma_lut_size; > u16 gamma_lut_size; > } color; > + > + /* PPGTT bit size */ > + int full_ppgtt_bits; > }; > > struct intel_driver_caps { > diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c > index e272127783fe..9f74244ef3e1 100644 > --- a/drivers/gpu/drm/i915/selftests/huge_pages.c > +++ b/drivers/gpu/drm/i915/selftests/huge_pages.c > @@ -1434,7 +1434,7 @@ static int igt_ppgtt_pin_update(void *arg) > * huge-gtt-pages. > */ > > - if (!USES_FULL_48BIT_PPGTT(dev_priv)) { > + if (!USES_FULL_4LVL_PPGTT(dev_priv)) { > pr_info("48b PPGTT not supported, skipping\n"); > return 0; > } > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > index 43ed8b28aeaa..33d7225edbbb 100644 > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > @@ -181,6 +181,8 @@ struct drm_i915_private *mock_gem_device(void) > I915_GTT_PAGE_SIZE_64K | > I915_GTT_PAGE_SIZE_2M; > > + mkwrite_device_info(i915)->full_ppgtt_bits = 48; > + > mock_uncore_init(i915); > i915_gem_init__mm(i915); > > -- > 2.14.4 >
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
On Thu, 6 Sep 2018 14:10:35 -0700 Rodrigo Vivi <rodrigo.vivi@intel.com> wrote: > On Thu, Sep 06, 2018 at 01:04:09PM -0700, Bob Paauwe wrote: > > 48 bit ppgtt device configuration is really just extended address > > range full ppgtt and may actually be something other than 48 bits. > > > > Change USES_FULL_48BIT_PPGTT() to USES_FULL_4LVL_PPGTT() to better > > describe that a 4 level walk table extended range PPGTT is being > > used. Add a new device info field that specifies the number of > > bits to prepare for cases where the range is not 32 or 48 bits. > > > > v2: keep USES_FULL_PPGTT() unchanged (Chris) > > > > Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> > > CC: Rodrigo Vivi <rodrigo.vivi@intel.com> > > CC: Michel Thierry <michel.thierry@intel.com> > > CC: Chris Wilson <chris@chris-wilson.co.uk> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 2 +- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 19 ++++++++++--------- > > drivers/gpu/drm/i915/i915_pci.c | 7 +++++-- > > drivers/gpu/drm/i915/intel_device_info.h | 3 +++ > > drivers/gpu/drm/i915/selftests/huge_pages.c | 2 +- > > drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 ++ > > 6 files changed, 22 insertions(+), 13 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 5a4da5b723fd..a367686fd735 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -2569,7 +2569,7 @@ intel_info(const struct drm_i915_private *dev_priv) > > > > #define USES_PPGTT(dev_priv) (i915_modparams.enable_ppgtt) > > #define USES_FULL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2) > > -#define USES_FULL_48BIT_PPGTT(dev_priv) (i915_modparams.enable_ppgtt == 3) > > +#define USES_FULL_4LVL_PPGTT(dev_priv) ((dev_priv)->info.full_ppgtt_bits > 32) > > #define HAS_PAGE_SIZES(dev_priv, sizes) ({ \ > > GEM_BUG_ON((sizes) == 0); \ > > ((sizes) & ~(dev_priv)->info.page_sizes) == 0; \ > > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c > > index eb0e446d6482..530a4c1452b3 100644 > > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c > > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c > > @@ -137,18 +137,18 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, > > int enable_ppgtt) > > { > > bool has_full_ppgtt; > > - bool has_full_48bit_ppgtt; > > + bool has_full_4lvl_ppgtt; > > > > if (!dev_priv->info.has_aliasing_ppgtt) > > return 0; > > > > has_full_ppgtt = dev_priv->info.has_full_ppgtt; > > - has_full_48bit_ppgtt = dev_priv->info.has_full_48bit_ppgtt; > > + has_full_4lvl_ppgtt = USES_FULL_4LVL_PPGTT(dev_priv); > > > > if (intel_vgpu_active(dev_priv)) { > > /* GVT-g has no support for 32bit ppgtt */ > > has_full_ppgtt = false; > > - has_full_48bit_ppgtt = intel_vgpu_has_full_48bit_ppgtt(dev_priv); > > + has_full_4lvl_ppgtt = intel_vgpu_has_full_48bit_ppgtt(dev_priv); > > I wonder if we should already rename this function to match 4lvl instead > of full and/or 48bit on it. If I start down that path, do I rename i915_vm_is_48bit() too and go through comments that use 48b/48bit? Or is it better to split this up into multiple patches? > > > } > > > > /* > > @@ -164,7 +164,7 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, > > if (enable_ppgtt == 2 && has_full_ppgtt) > > return 2; > > > > - if (enable_ppgtt == 3 && has_full_48bit_ppgtt) > > + if (enable_ppgtt == 3 && has_full_4lvl_ppgtt) > > return 3; > > > > /* Disable ppgtt on SNB if VT-d is on. */ > > @@ -173,7 +173,7 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, > > return 0; > > } > > > > - if (has_full_48bit_ppgtt) > > + if (has_full_4lvl_ppgtt) > > return 3; > > > > if (has_full_ppgtt) > > @@ -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)) > > + ppgtt->vm.total = 1ULL << 32; > > + else > > + ppgtt->vm.total = 1ULL << i915->info.full_ppgtt_bits; > > > > /* > > * From bdw, there is support for read-only pages in the PPGTT. > > @@ -1788,7 +1789,7 @@ static void gen8_ppgtt_enable(struct drm_i915_private *dev_priv) > > enum intel_engine_id id; > > > > for_each_engine(engine, dev_priv, id) { > > - u32 four_level = USES_FULL_48BIT_PPGTT(dev_priv) ? > > + u32 four_level = USES_FULL_4LVL_PPGTT(dev_priv) ? > > GEN8_GFX_PPGTT_48B : 0; > > I915_WRITE(RING_MODE_GEN7(engine), > > _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE | four_level)); > > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > > index d6f7b9fe1d26..a99c1f6de64e 100644 > > --- a/drivers/gpu/drm/i915/i915_pci.c > > +++ b/drivers/gpu/drm/i915/i915_pci.c > > @@ -299,6 +299,7 @@ static const struct intel_device_info intel_sandybridge_m_gt2_info = { > > .has_rc6p = 1, \ > > .has_aliasing_ppgtt = 1, \ > > .has_full_ppgtt = 1, \ > > + .full_ppgtt_bits = 32, \ > > GEN_DEFAULT_PIPEOFFSETS, \ > > GEN_DEFAULT_PAGE_SIZES, \ > > IVB_CURSOR_OFFSETS > > @@ -353,6 +354,7 @@ static const struct intel_device_info intel_valleyview_info = { > > .has_hotplug = 1, > > .has_aliasing_ppgtt = 1, > > .has_full_ppgtt = 1, > > + .full_ppgtt_bits = 32, \ > > .has_snoop = true, > > .has_coherent_ggtt = false, > > .ring_mask = RENDER_RING | BSD_RING | BLT_RING, > > @@ -399,7 +401,7 @@ static const struct intel_device_info intel_haswell_gt3_info = { > > .page_sizes = I915_GTT_PAGE_SIZE_4K | \ > > I915_GTT_PAGE_SIZE_2M, \ > > .has_logical_ring_contexts = 1, \ > > - .has_full_48bit_ppgtt = 1, \ > > + .full_ppgtt_bits = 48, \ > > .has_64bit_reloc = 1, \ > > .has_reset_engine = 1 > > > > @@ -445,6 +447,7 @@ static const struct intel_device_info intel_cherryview_info = { > > .has_gmch_display = 1, > > .has_aliasing_ppgtt = 1, > > .has_full_ppgtt = 1, > > + .full_ppgtt_bits = 32, \ > > .has_reset_engine = 1, > > .has_snoop = true, > > .has_coherent_ggtt = false, > > @@ -520,7 +523,7 @@ static const struct intel_device_info intel_skylake_gt4_info = { > > .has_guc = 1, \ > > .has_aliasing_ppgtt = 1, \ > > .has_full_ppgtt = 1, \ > > - .has_full_48bit_ppgtt = 1, \ > > + .full_ppgtt_bits = 48, \ > > .has_reset_engine = 1, \ > > .has_snoop = true, \ > > .has_coherent_ggtt = false, \ > > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h > > index 6eecd64734d5..083590e98195 100644 > > --- a/drivers/gpu/drm/i915/intel_device_info.h > > +++ b/drivers/gpu/drm/i915/intel_device_info.h > > @@ -182,6 +182,9 @@ struct intel_device_info { > > u16 degamma_lut_size; > > u16 gamma_lut_size; > > } color; > > + > > + /* PPGTT bit size */ > > + int full_ppgtt_bits; > > }; > > > > struct intel_driver_caps { > > diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c > > index e272127783fe..9f74244ef3e1 100644 > > --- a/drivers/gpu/drm/i915/selftests/huge_pages.c > > +++ b/drivers/gpu/drm/i915/selftests/huge_pages.c > > @@ -1434,7 +1434,7 @@ static int igt_ppgtt_pin_update(void *arg) > > * huge-gtt-pages. > > */ > > > > - if (!USES_FULL_48BIT_PPGTT(dev_priv)) { > > + if (!USES_FULL_4LVL_PPGTT(dev_priv)) { > > pr_info("48b PPGTT not supported, skipping\n"); > > return 0; > > } > > diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > > index 43ed8b28aeaa..33d7225edbbb 100644 > > --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c > > +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c > > @@ -181,6 +181,8 @@ struct drm_i915_private *mock_gem_device(void) > > I915_GTT_PAGE_SIZE_64K | > > I915_GTT_PAGE_SIZE_2M; > > > > + mkwrite_device_info(i915)->full_ppgtt_bits = 48; > > + > > mock_uncore_init(i915); > > i915_gem_init__mm(i915); > > > > -- > > 2.14.4 > >
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 5a4da5b723fd..a367686fd735 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2569,7 +2569,7 @@ intel_info(const struct drm_i915_private *dev_priv) #define USES_PPGTT(dev_priv) (i915_modparams.enable_ppgtt) #define USES_FULL_PPGTT(dev_priv) (i915_modparams.enable_ppgtt >= 2) -#define USES_FULL_48BIT_PPGTT(dev_priv) (i915_modparams.enable_ppgtt == 3) +#define USES_FULL_4LVL_PPGTT(dev_priv) ((dev_priv)->info.full_ppgtt_bits > 32) #define HAS_PAGE_SIZES(dev_priv, sizes) ({ \ GEM_BUG_ON((sizes) == 0); \ ((sizes) & ~(dev_priv)->info.page_sizes) == 0; \ diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index eb0e446d6482..530a4c1452b3 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -137,18 +137,18 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, int enable_ppgtt) { bool has_full_ppgtt; - bool has_full_48bit_ppgtt; + bool has_full_4lvl_ppgtt; if (!dev_priv->info.has_aliasing_ppgtt) return 0; has_full_ppgtt = dev_priv->info.has_full_ppgtt; - has_full_48bit_ppgtt = dev_priv->info.has_full_48bit_ppgtt; + has_full_4lvl_ppgtt = USES_FULL_4LVL_PPGTT(dev_priv); if (intel_vgpu_active(dev_priv)) { /* GVT-g has no support for 32bit ppgtt */ has_full_ppgtt = false; - has_full_48bit_ppgtt = intel_vgpu_has_full_48bit_ppgtt(dev_priv); + has_full_4lvl_ppgtt = intel_vgpu_has_full_48bit_ppgtt(dev_priv); } /* @@ -164,7 +164,7 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, if (enable_ppgtt == 2 && has_full_ppgtt) return 2; - if (enable_ppgtt == 3 && has_full_48bit_ppgtt) + if (enable_ppgtt == 3 && has_full_4lvl_ppgtt) return 3; /* Disable ppgtt on SNB if VT-d is on. */ @@ -173,7 +173,7 @@ int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv, return 0; } - if (has_full_48bit_ppgtt) + if (has_full_4lvl_ppgtt) return 3; if (has_full_ppgtt) @@ -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)) + ppgtt->vm.total = 1ULL << 32; + else + ppgtt->vm.total = 1ULL << i915->info.full_ppgtt_bits; /* * From bdw, there is support for read-only pages in the PPGTT. @@ -1788,7 +1789,7 @@ static void gen8_ppgtt_enable(struct drm_i915_private *dev_priv) enum intel_engine_id id; for_each_engine(engine, dev_priv, id) { - u32 four_level = USES_FULL_48BIT_PPGTT(dev_priv) ? + u32 four_level = USES_FULL_4LVL_PPGTT(dev_priv) ? GEN8_GFX_PPGTT_48B : 0; I915_WRITE(RING_MODE_GEN7(engine), _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE | four_level)); diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index d6f7b9fe1d26..a99c1f6de64e 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -299,6 +299,7 @@ static const struct intel_device_info intel_sandybridge_m_gt2_info = { .has_rc6p = 1, \ .has_aliasing_ppgtt = 1, \ .has_full_ppgtt = 1, \ + .full_ppgtt_bits = 32, \ GEN_DEFAULT_PIPEOFFSETS, \ GEN_DEFAULT_PAGE_SIZES, \ IVB_CURSOR_OFFSETS @@ -353,6 +354,7 @@ static const struct intel_device_info intel_valleyview_info = { .has_hotplug = 1, .has_aliasing_ppgtt = 1, .has_full_ppgtt = 1, + .full_ppgtt_bits = 32, \ .has_snoop = true, .has_coherent_ggtt = false, .ring_mask = RENDER_RING | BSD_RING | BLT_RING, @@ -399,7 +401,7 @@ static const struct intel_device_info intel_haswell_gt3_info = { .page_sizes = I915_GTT_PAGE_SIZE_4K | \ I915_GTT_PAGE_SIZE_2M, \ .has_logical_ring_contexts = 1, \ - .has_full_48bit_ppgtt = 1, \ + .full_ppgtt_bits = 48, \ .has_64bit_reloc = 1, \ .has_reset_engine = 1 @@ -445,6 +447,7 @@ static const struct intel_device_info intel_cherryview_info = { .has_gmch_display = 1, .has_aliasing_ppgtt = 1, .has_full_ppgtt = 1, + .full_ppgtt_bits = 32, \ .has_reset_engine = 1, .has_snoop = true, .has_coherent_ggtt = false, @@ -520,7 +523,7 @@ static const struct intel_device_info intel_skylake_gt4_info = { .has_guc = 1, \ .has_aliasing_ppgtt = 1, \ .has_full_ppgtt = 1, \ - .has_full_48bit_ppgtt = 1, \ + .full_ppgtt_bits = 48, \ .has_reset_engine = 1, \ .has_snoop = true, \ .has_coherent_ggtt = false, \ diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index 6eecd64734d5..083590e98195 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -182,6 +182,9 @@ struct intel_device_info { u16 degamma_lut_size; u16 gamma_lut_size; } color; + + /* PPGTT bit size */ + int full_ppgtt_bits; }; struct intel_driver_caps { diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c index e272127783fe..9f74244ef3e1 100644 --- a/drivers/gpu/drm/i915/selftests/huge_pages.c +++ b/drivers/gpu/drm/i915/selftests/huge_pages.c @@ -1434,7 +1434,7 @@ static int igt_ppgtt_pin_update(void *arg) * huge-gtt-pages. */ - if (!USES_FULL_48BIT_PPGTT(dev_priv)) { + if (!USES_FULL_4LVL_PPGTT(dev_priv)) { pr_info("48b PPGTT not supported, skipping\n"); return 0; } diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c index 43ed8b28aeaa..33d7225edbbb 100644 --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c @@ -181,6 +181,8 @@ struct drm_i915_private *mock_gem_device(void) I915_GTT_PAGE_SIZE_64K | I915_GTT_PAGE_SIZE_2M; + mkwrite_device_info(i915)->full_ppgtt_bits = 48; + mock_uncore_init(i915); i915_gem_init__mm(i915);
48 bit ppgtt device configuration is really just extended address range full ppgtt and may actually be something other than 48 bits. Change USES_FULL_48BIT_PPGTT() to USES_FULL_4LVL_PPGTT() to better describe that a 4 level walk table extended range PPGTT is being used. Add a new device info field that specifies the number of bits to prepare for cases where the range is not 32 or 48 bits. v2: keep USES_FULL_PPGTT() unchanged (Chris) Signed-off-by: Bob Paauwe <bob.j.paauwe@intel.com> CC: Rodrigo Vivi <rodrigo.vivi@intel.com> CC: Michel Thierry <michel.thierry@intel.com> CC: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/i915_gem_gtt.c | 19 ++++++++++--------- drivers/gpu/drm/i915/i915_pci.c | 7 +++++-- drivers/gpu/drm/i915/intel_device_info.h | 3 +++ drivers/gpu/drm/i915/selftests/huge_pages.c | 2 +- drivers/gpu/drm/i915/selftests/mock_gem_device.c | 2 ++ 6 files changed, 22 insertions(+), 13 deletions(-)