[Intel-gfx] [PATCH v4] drm/i915: Remove i915.enable_ppgtt override

Zhi Wang zhi.a.wang at intel.com
Wed Sep 26 12:29:37 UTC 2018


Sure. Acked-by: Zhi Wang <zhi.a.wang at intel.com>

Meanwhile, I could send a patch to remove 
gen8_preallocate_top_level_pdp() in i915_gem_gtt.c, which is only used 
by vGPU with 32bit PPGTT (no one is going to use them after this patch 
is landed)

Thanks,
Zhi.

On 09/26/18 04:02, Joonas Lahtinen wrote:
> Quoting He, Min (2018-09-26 04:06:25)
>> No. We changed to full 48bit ppgtt long time ago. :)
> 
> So would that mean the change is OK to do?
> 
> Acked-by from you, Zhi, would be good to have for documentation purposes
> in that case :)
> 
> Regards, Joonas
> 
>>
>>> -----Original Message-----
>>> From: Wang, Zhi A
>>> Sent: Wednesday, September 26, 2018 2:01 AM
>>> To: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>; Chris Wilson
>>> <chris at chris-wilson.co.uk>; intel-gfx at lists.freedesktop.org; Zhenyu Wang
>>> <zhenyuw at linux.intel.com>
>>> Cc: Auld, Matthew <matthew.auld at intel.com>; He, Min <min.he at intel.com>
>>> Subject: Re: [PATCH v4] drm/i915: Remove i915.enable_ppgtt override
>>>
>>> Hi Min:
>>>
>>> I remembered the IOTG guest kernel is using aliasing-ppgtt in the last
>>> technical sharing (probably I didn't remember it correctly). Can you
>>> confirm?
>>>
>>> Also, thanks Joonas for the reminding. :)
>>>
>>> thanks,
>>> Zhi.
>>>
>>> On 09/25/18 09:22, Joonas Lahtinen wrote:
>>>> + Zhi and Zhenyu
>>>>
>>>> For me the patch looks ok.
>>>>
>>>> It refuses to load GVT on systems where 48-bit is not supported, for not
>>>> worsening the system security when virtualization is enabled (as anybody
>>>> would probably expect virtualization to improve that). Please see code.
>>>>
>>>> Do we have such systems in the wild? Should we instruct the user to
>>>> updating the hypervisor to specific kernel version when they hit the
>>>> error?
>>>>
>>>> Regards, Joonas
>>>>
>>>> Quoting Chris Wilson (2018-09-25 14:48:20)
>>>>> Now that we are confident in providing full-ppgtt where supported,
>>>>> remove the ability to override the context isolation.
>>>>>
>>>>> v2: Remove faked aliasing-ppgtt for testing as it no longer is accepted.
>>>>> v3: s/USES/HAS/ to match usage and reject attempts to load the module
>>> on
>>>>> old GVT-g setups that do not provide support for full-ppgtt.
>>>>>
>>>>> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
>>>>> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
>>>>> Cc: Matthew Auld <matthew.auld at intel.com>
>>>>> ---
>>>>>    drivers/gpu/drm/i915/i915_drv.c               | 22 +++--
>>>>>    drivers/gpu/drm/i915/i915_drv.h               | 14 +--
>>>>>    drivers/gpu/drm/i915/i915_gem_context.c       |  2 +-
>>>>>    drivers/gpu/drm/i915/i915_gem_gtt.c           | 88 ++-----------------
>>>>>    drivers/gpu/drm/i915/i915_gpu_error.c         |  4 +-
>>>>>    drivers/gpu/drm/i915/i915_params.c            |  4 -
>>>>>    drivers/gpu/drm/i915/i915_params.h            |  1 -
>>>>>    drivers/gpu/drm/i915/i915_pci.c               | 17 ++--
>>>>>    drivers/gpu/drm/i915/intel_device_info.c      |  6 ++
>>>>>    drivers/gpu/drm/i915/intel_device_info.h      | 11 ++-
>>>>>    drivers/gpu/drm/i915/intel_lrc.c              | 13 ++-
>>>>>    drivers/gpu/drm/i915/selftests/huge_pages.c   | 12 +--
>>>>>    .../gpu/drm/i915/selftests/i915_gem_context.c | 59 +------------
>>>>>    .../gpu/drm/i915/selftests/i915_gem_evict.c   |  2 +-
>>>>>    drivers/gpu/drm/i915/selftests/i915_gem_gtt.c |  4 +-
>>>>>    15 files changed, 62 insertions(+), 197 deletions(-)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>>> b/drivers/gpu/drm/i915/i915_drv.c
>>>>> index 44e2c0f5ec50..3efbbc71c3b4 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_drv.c
>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>>>>> @@ -345,7 +345,7 @@ static int i915_getparam_ioctl(struct drm_device
>>> *dev, void *data,
>>>>>                   value = HAS_WT(dev_priv);
>>>>>                   break;
>>>>>           case I915_PARAM_HAS_ALIASING_PPGTT:
>>>>> -               value = USES_PPGTT(dev_priv);
>>>>> +               value = INTEL_PPGTT(dev_priv);
>>>>>                   break;
>>>>>           case I915_PARAM_HAS_SEMAPHORES:
>>>>>                   value = HAS_LEGACY_SEMAPHORES(dev_priv);
>>>>> @@ -1049,17 +1049,6 @@ static void i915_driver_cleanup_mmio(struct
>>> drm_i915_private *dev_priv)
>>>>>
>>>>>    static void intel_sanitize_options(struct drm_i915_private *dev_priv)
>>>>>    {
>>>>> -       /*
>>>>> -        * i915.enable_ppgtt is read-only, so do an early pass to validate the
>>>>> -        * user's requested state against the hardware/driver capabilities.
>>> We
>>>>> -        * do this now so that we can print out any log messages once rather
>>>>> -        * than every time we check intel_enable_ppgtt().
>>>>> -        */
>>>>> -       i915_modparams.enable_ppgtt =
>>>>> -               intel_sanitize_enable_ppgtt(dev_priv,
>>>>> -                                           i915_modparams.enable_ppgtt);
>>>>> -       DRM_DEBUG_DRIVER("ppgtt mode: %i\n",
>>> i915_modparams.enable_ppgtt);
>>>>> -
>>>>>           intel_gvt_sanitize_options(dev_priv);
>>>>>    }
>>>>>
>>>>> @@ -1374,6 +1363,15 @@ static int i915_driver_init_hw(struct
>>> drm_i915_private *dev_priv)
>>>>>
>>>>>           intel_device_info_runtime_init(mkwrite_device_info(dev_priv));
>>>>>
>>>>> +       if (HAS_PPGTT(dev_priv)) {
>>>>> +               if (intel_vgpu_active(dev_priv) &&
>>>>> +                   !intel_vgpu_has_full_48bit_ppgtt(dev_priv)) {
>>>>> +                       i915_report_error(dev_priv,
>>>>> +                                         "incompatible vGPU found, support for isolated
>>> ppGTT required\n");
>>>>> +                       return -ENXIO;
>>>>> +               }
>>>>> +       }
>>>>> +
>>>>>           intel_sanitize_options(dev_priv);
>>>>>
>>>>>           i915_perf_init(dev_priv);
>>>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>> b/drivers/gpu/drm/i915/i915_drv.h
>>>>> index 8624b4bdc242..f28b3c2a3cd2 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>>>> @@ -2593,9 +2593,14 @@ intel_info(const struct drm_i915_private
>>> *dev_priv)
>>>>>
>>>>>    #define HAS_EXECLISTS(dev_priv)
>>> HAS_LOGICAL_RING_CONTEXTS(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 INTEL_PPGTT(dev_priv) (INTEL_INFO(dev_priv)->ppgtt)
>>>>> +#define HAS_PPGTT(dev_priv) \
>>>>> +       (INTEL_PPGTT(dev_priv) != INTEL_PPGTT_NONE)
>>>>> +#define HAS_FULL_PPGTT(dev_priv) \
>>>>> +       (INTEL_PPGTT(dev_priv) >= INTEL_PPGTT_FULL)
>>>>> +#define HAS_FULL_48BIT_PPGTT(dev_priv) \
>>>>> +       (INTEL_PPGTT(dev_priv) >= INTEL_PPGTT_FULL_4LVL)
>>>>> +
>>>>>    #define HAS_PAGE_SIZES(dev_priv, sizes) ({ \
>>>>>           GEM_BUG_ON((sizes) == 0); \
>>>>>           ((sizes) & ~(dev_priv)->info.page_sizes) == 0; \
>>>>> @@ -2743,9 +2748,6 @@ intel_ggtt_update_needs_vtd_wa(struct
>>> drm_i915_private *dev_priv)
>>>>>           return IS_BROXTON(dev_priv) && intel_vtd_active();
>>>>>    }
>>>>>
>>>>> -int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
>>>>> -                               int enable_ppgtt);
>>>>> -
>>>>>    /* i915_drv.c */
>>>>>    void __printf(3, 4)
>>>>>    __i915_printk(struct drm_i915_private *dev_priv, const char *level,
>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c
>>> b/drivers/gpu/drm/i915/i915_gem_context.c
>>>>> index f772593b99ab..15c92f75b1b8 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_gem_context.c
>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
>>>>> @@ -414,7 +414,7 @@ i915_gem_create_context(struct
>>> drm_i915_private *dev_priv,
>>>>>           if (IS_ERR(ctx))
>>>>>                   return ctx;
>>>>>
>>>>> -       if (USES_FULL_PPGTT(dev_priv)) {
>>>>> +       if (HAS_FULL_PPGTT(dev_priv)) {
>>>>>                   struct i915_hw_ppgtt *ppgtt;
>>>>>
>>>>>                   ppgtt = i915_ppgtt_create(dev_priv, file_priv);
>>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>> b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> index f6c7ab413081..c9363504949f 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
>>>>> @@ -133,55 +133,6 @@ static inline void i915_ggtt_invalidate(struct
>>> drm_i915_private *i915)
>>>>>           i915->ggtt.invalidate(i915);
>>>>>    }
>>>>>
>>>>> -int intel_sanitize_enable_ppgtt(struct drm_i915_private *dev_priv,
>>>>> -                               int enable_ppgtt)
>>>>> -{
>>>>> -       bool has_full_ppgtt;
>>>>> -       bool has_full_48bit_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;
>>>>> -
>>>>> -       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);
>>>>> -       }
>>>>> -
>>>>> -       /*
>>>>> -        * We don't allow disabling PPGTT for gen8+ as it's a requirement for
>>>>> -        * execlists, the sole mechanism available to submit work.
>>>>> -        */
>>>>> -       if (enable_ppgtt == 0 && !HAS_EXECLISTS(dev_priv))
>>>>> -               return 0;
>>>>> -
>>>>> -       if (enable_ppgtt == 1)
>>>>> -               return 1;
>>>>> -
>>>>> -       if (enable_ppgtt == 2 && has_full_ppgtt)
>>>>> -               return 2;
>>>>> -
>>>>> -       if (enable_ppgtt == 3 && has_full_48bit_ppgtt)
>>>>> -               return 3;
>>>>> -
>>>>> -       /* Disable ppgtt on SNB if VT-d is on. */
>>>>> -       if (IS_GEN6(dev_priv) && intel_vtd_active()) {
>>>>> -               DRM_INFO("Disabling PPGTT because VT-d is on\n");
>>>>> -               return 0;
>>>>> -       }
>>>>> -
>>>>> -       if (has_full_48bit_ppgtt)
>>>>> -               return 3;
>>>>> -
>>>>> -       if (has_full_ppgtt)
>>>>> -               return 2;
>>>>> -
>>>>> -       return 1;
>>>>> -}
>>>>> -
>>>>>    static int ppgtt_bind_vma(struct i915_vma *vma,
>>>>>                             enum i915_cache_level cache_level,
>>>>>                             u32 unused)
>>>>> @@ -1647,7 +1598,7 @@ 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) ?
>>>>> +       ppgtt->vm.total = HAS_FULL_48BIT_PPGTT(i915) ?
>>>>>                   1ULL << 48 :
>>>>>                   1ULL << 32;
>>>>>
>>>>> @@ -1782,19 +1733,6 @@ static inline void gen6_write_pde(const struct
>>> gen6_hw_ppgtt *ppgtt,
>>>>>                     ppgtt->pd_addr + pde);
>>>>>    }
>>>>>
>>>>> -static void gen8_ppgtt_enable(struct drm_i915_private *dev_priv)
>>>>> -{
>>>>> -       struct intel_engine_cs *engine;
>>>>> -       enum intel_engine_id id;
>>>>> -
>>>>> -       for_each_engine(engine, dev_priv, id) {
>>>>> -               u32 four_level = USES_FULL_48BIT_PPGTT(dev_priv) ?
>>>>> -                                GEN8_GFX_PPGTT_48B : 0;
>>>>> -               I915_WRITE(RING_MODE_GEN7(engine),
>>>>> -                          _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE | four_level));
>>>>> -       }
>>>>> -}
>>>>> -
>>>>>    static void gen7_ppgtt_enable(struct drm_i915_private *dev_priv)
>>>>>    {
>>>>>           struct intel_engine_cs *engine;
>>>>> @@ -1834,7 +1772,8 @@ static void gen6_ppgtt_enable(struct
>>> drm_i915_private *dev_priv)
>>>>>           ecochk = I915_READ(GAM_ECOCHK);
>>>>>           I915_WRITE(GAM_ECOCHK, ecochk | ECOCHK_SNB_BIT |
>>> ECOCHK_PPGTT_CACHE64B);
>>>>>
>>>>> -       I915_WRITE(GFX_MODE,
>>> _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
>>>>> +       if (HAS_PPGTT(dev_priv)) /* may be disabled for VT-d */
>>>>> +               I915_WRITE(GFX_MODE,
>>> _MASKED_BIT_ENABLE(GFX_PPGTT_ENABLE));
>>>>>    }
>>>>>
>>>>>    /* PPGTT support for Sandybdrige/Gen6 and later */
>>>>> @@ -2237,23 +2176,10 @@ int i915_ppgtt_init_hw(struct
>>> drm_i915_private *dev_priv)
>>>>>    {
>>>>>           gtt_write_workarounds(dev_priv);
>>>>>
>>>>> -       /* In the case of execlists, PPGTT is enabled by the context descriptor
>>>>> -        * and the PDPs are contained within the context itself.  We don't
>>>>> -        * need to do anything here. */
>>>>> -       if (HAS_LOGICAL_RING_CONTEXTS(dev_priv))
>>>>> -               return 0;
>>>>> -
>>>>> -       if (!USES_PPGTT(dev_priv))
>>>>> -               return 0;
>>>>> -
>>>>>           if (IS_GEN6(dev_priv))
>>>>>                   gen6_ppgtt_enable(dev_priv);
>>>>>           else if (IS_GEN7(dev_priv))
>>>>>                   gen7_ppgtt_enable(dev_priv);
>>>>> -       else if (INTEL_GEN(dev_priv) >= 8)
>>>>> -               gen8_ppgtt_enable(dev_priv);
>>>>> -       else
>>>>> -               MISSING_CASE(INTEL_GEN(dev_priv));
>>>>>
>>>>>           return 0;
>>>>>    }
>>>>> @@ -2952,7 +2878,7 @@ int i915_gem_init_ggtt(struct drm_i915_private
>>> *dev_priv)
>>>>>           /* And finally clear the reserved guard page */
>>>>>           ggtt->vm.clear_range(&ggtt->vm, ggtt->vm.total - PAGE_SIZE,
>>> PAGE_SIZE);
>>>>>
>>>>> -       if (USES_PPGTT(dev_priv) && !USES_FULL_PPGTT(dev_priv)) {
>>>>> +       if (INTEL_PPGTT(dev_priv) == INTEL_PPGTT_ALIASING) {
>>>>>                   ret = i915_gem_init_aliasing_ppgtt(dev_priv);
>>>>>                   if (ret)
>>>>>                           goto err;
>>>>> @@ -3275,7 +3201,7 @@ static void bdw_setup_private_ppat(struct
>>> intel_ppat *ppat)
>>>>>           ppat->match = bdw_private_pat_match;
>>>>>           ppat->clear_value = GEN8_PPAT_WB | GEN8_PPAT_LLCELLC |
>>> GEN8_PPAT_AGE(3);
>>>>>
>>>>> -       if (!USES_PPGTT(ppat->i915)) {
>>>>> +       if (!HAS_PPGTT(ppat->i915)) {
>>>>>                   /* Spec: "For GGTT, there is NO pat_sel[2:0] from the entry,
>>>>>                    * so RTL will always use the value corresponding to
>>>>>                    * pat_sel = 000".
>>>>> @@ -3402,7 +3328,7 @@ static int gen8_gmch_probe(struct i915_ggtt
>>> *ggtt)
>>>>>           ggtt->vm.cleanup = gen6_gmch_remove;
>>>>>           ggtt->vm.insert_page = gen8_ggtt_insert_page;
>>>>>           ggtt->vm.clear_range = nop_clear_range;
>>>>> -       if (!USES_FULL_PPGTT(dev_priv) ||
>>> intel_scanout_needs_vtd_wa(dev_priv))
>>>>> +       if (intel_scanout_needs_vtd_wa(dev_priv))
>>>>>                   ggtt->vm.clear_range = gen8_ggtt_clear_range;
>>>>>
>>>>>           ggtt->vm.insert_entries = gen8_ggtt_insert_entries;
>>>>> @@ -3609,7 +3535,7 @@ int i915_ggtt_init_hw(struct drm_i915_private
>>> *dev_priv)
>>>>>           /* Only VLV supports read-only GGTT mappings */
>>>>>           ggtt->vm.has_read_only = IS_VALLEYVIEW(dev_priv);
>>>>>
>>>>> -       if (!HAS_LLC(dev_priv) && !USES_PPGTT(dev_priv))
>>>>> +       if (!HAS_LLC(dev_priv) && !HAS_PPGTT(dev_priv))
>>>>>                   ggtt->vm.mm.color_adjust = i915_gtt_color_adjust;
>>>>>           mutex_unlock(&dev_priv->drm.struct_mutex);
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c
>>> b/drivers/gpu/drm/i915/i915_gpu_error.c
>>>>> index 2835cacd0d08..3d5554f14dfd 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
>>>>> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
>>>>> @@ -474,7 +474,7 @@ static void error_print_engine(struct
>>> drm_i915_error_state_buf *m,
>>>>>                           err_printf(m, "  SYNC_2: 0x%08x\n",
>>>>>                                      ee->semaphore_mboxes[2]);
>>>>>           }
>>>>> -       if (USES_PPGTT(m->i915)) {
>>>>> +       if (HAS_PPGTT(m->i915)) {
>>>>>                   err_printf(m, "  GFX_MODE: 0x%08x\n", ee->vm_info.gfx_mode);
>>>>>
>>>>>                   if (INTEL_GEN(m->i915) >= 8) {
>>>>> @@ -1230,7 +1230,7 @@ static void
>>> error_record_engine_registers(struct i915_gpu_state *error,
>>>>>           ee->reset_count = i915_reset_engine_count(&dev_priv->gpu_error,
>>>>>                                                     engine);
>>>>>
>>>>> -       if (USES_PPGTT(dev_priv)) {
>>>>> +       if (HAS_PPGTT(dev_priv)) {
>>>>>                   int i;
>>>>>
>>>>>                   ee->vm_info.gfx_mode =
>>> I915_READ(RING_MODE_GEN7(engine));
>>>>> diff --git a/drivers/gpu/drm/i915/i915_params.c
>>> b/drivers/gpu/drm/i915/i915_params.c
>>>>> index 295e981e4a39..bd6bd8879cab 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_params.c
>>>>> +++ b/drivers/gpu/drm/i915/i915_params.c
>>>>> @@ -82,10 +82,6 @@ i915_param_named_unsafe(enable_hangcheck,
>>> bool, 0644,
>>>>>           "WARNING: Disabling this can cause system wide hangs. "
>>>>>           "(default: true)");
>>>>>
>>>>> -i915_param_named_unsafe(enable_ppgtt, int, 0400,
>>>>> -       "Override PPGTT usage. "
>>>>> -       "(-1=auto [default], 0=disabled, 1=aliasing, 2=full, 3=full with
>>> extended address space)");
>>>>> -
>>>>>    i915_param_named_unsafe(enable_psr, int, 0600,
>>>>>           "Enable PSR "
>>>>>           "(0=disabled, 1=enabled) "
>>>>> diff --git a/drivers/gpu/drm/i915/i915_params.h
>>> b/drivers/gpu/drm/i915/i915_params.h
>>>>> index 6c4d4a21474b..7e56c516c815 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_params.h
>>>>> +++ b/drivers/gpu/drm/i915/i915_params.h
>>>>> @@ -41,7 +41,6 @@ struct drm_printer;
>>>>>           param(int, vbt_sdvo_panel_type, -1) \
>>>>>           param(int, enable_dc, -1) \
>>>>>           param(int, enable_fbc, -1) \
>>>>> -       param(int, enable_ppgtt, -1) \
>>>>>           param(int, enable_psr, -1) \
>>>>>           param(int, disable_power_well, -1) \
>>>>>           param(int, enable_ips, 1) \
>>>>> diff --git a/drivers/gpu/drm/i915/i915_pci.c
>>> b/drivers/gpu/drm/i915/i915_pci.c
>>>>> index d6f7b9fe1d26..eeec902896b9 100644
>>>>> --- a/drivers/gpu/drm/i915/i915_pci.c
>>>>> +++ b/drivers/gpu/drm/i915/i915_pci.c
>>>>> @@ -252,7 +252,7 @@ static const struct intel_device_info
>>> intel_ironlake_m_info = {
>>>>>           .has_llc = 1, \
>>>>>           .has_rc6 = 1, \
>>>>>           .has_rc6p = 1, \
>>>>> -       .has_aliasing_ppgtt = 1, \
>>>>> +       .ppgtt = INTEL_PPGTT_ALIASING, \
>>>>>           GEN_DEFAULT_PIPEOFFSETS, \
>>>>>           GEN_DEFAULT_PAGE_SIZES, \
>>>>>           CURSOR_OFFSETS
>>>>> @@ -297,8 +297,7 @@ static const struct intel_device_info
>>> intel_sandybridge_m_gt2_info = {
>>>>>           .has_llc = 1, \
>>>>>           .has_rc6 = 1, \
>>>>>           .has_rc6p = 1, \
>>>>> -       .has_aliasing_ppgtt = 1, \
>>>>> -       .has_full_ppgtt = 1, \
>>>>> +       .ppgtt = INTEL_PPGTT_FULL, \
>>>>>           GEN_DEFAULT_PIPEOFFSETS, \
>>>>>           GEN_DEFAULT_PAGE_SIZES, \
>>>>>           IVB_CURSOR_OFFSETS
>>>>> @@ -351,8 +350,7 @@ static const struct intel_device_info
>>> intel_valleyview_info = {
>>>>>           .has_rc6 = 1,
>>>>>           .has_gmch_display = 1,
>>>>>           .has_hotplug = 1,
>>>>> -       .has_aliasing_ppgtt = 1,
>>>>> -       .has_full_ppgtt = 1,
>>>>> +       .ppgtt = INTEL_PPGTT_FULL,
>>>>>           .has_snoop = true,
>>>>>           .has_coherent_ggtt = false,
>>>>>           .ring_mask = RENDER_RING | BSD_RING | BLT_RING,
>>>>> @@ -399,7 +397,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, \
>>>>> +       .ppgtt = INTEL_PPGTT_FULL_4LVL, \
>>>>>           .has_64bit_reloc = 1, \
>>>>>           .has_reset_engine = 1
>>>>>
>>>>> @@ -443,8 +441,7 @@ static const struct intel_device_info
>>> intel_cherryview_info = {
>>>>>           .has_rc6 = 1,
>>>>>           .has_logical_ring_contexts = 1,
>>>>>           .has_gmch_display = 1,
>>>>> -       .has_aliasing_ppgtt = 1,
>>>>> -       .has_full_ppgtt = 1,
>>>>> +       .ppgtt = INTEL_PPGTT_FULL,
>>>>>           .has_reset_engine = 1,
>>>>>           .has_snoop = true,
>>>>>           .has_coherent_ggtt = false,
>>>>> @@ -518,9 +515,7 @@ static const struct intel_device_info
>>> intel_skylake_gt4_info = {
>>>>>           .has_logical_ring_contexts = 1, \
>>>>>           .has_logical_ring_preemption = 1, \
>>>>>           .has_guc = 1, \
>>>>> -       .has_aliasing_ppgtt = 1, \
>>>>> -       .has_full_ppgtt = 1, \
>>>>> -       .has_full_48bit_ppgtt = 1, \
>>>>> +       .ppgtt = INTEL_PPGTT_FULL_4LVL, \
>>>>>           .has_reset_engine = 1, \
>>>>>           .has_snoop = true, \
>>>>>           .has_coherent_ggtt = false, \
>>>>> diff --git a/drivers/gpu/drm/i915/intel_device_info.c
>>> b/drivers/gpu/drm/i915/intel_device_info.c
>>>>> index 0ef0c6448d53..a841d181efd3 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_device_info.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_device_info.c
>>>>> @@ -26,6 +26,7 @@
>>>>>
>>>>>    #include "intel_device_info.h"
>>>>>    #include "i915_drv.h"
>>>>> +#include "i915_vgpu.h"
>>>>>
>>>>>    #define PLATFORM_NAME(x) [INTEL_##x] = #x
>>>>>    static const char * const platform_names[] = {
>>>>> @@ -851,6 +852,11 @@ void intel_device_info_runtime_init(struct
>>> intel_device_info *info)
>>>>>           else if (INTEL_GEN(dev_priv) >= 11)
>>>>>                   gen11_sseu_info_init(dev_priv);
>>>>>
>>>>> +       if (IS_GEN6(dev_priv) && intel_vtd_active()) {
>>>>> +               DRM_INFO("Disabling ppGTT for VT-d support\n");
>>>>> +               info->ppgtt = INTEL_PPGTT_NONE;
>>>>> +       }
>>>>> +
>>>>>           /* Initialize command stream timestamp frequency */
>>>>>           info->cs_timestamp_frequency_khz =
>>> read_timestamp_frequency(dev_priv);
>>>>>    }
>>>>> diff --git a/drivers/gpu/drm/i915/intel_device_info.h
>>> b/drivers/gpu/drm/i915/intel_device_info.h
>>>>> index 6eecd64734d5..6bab97687328 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_device_info.h
>>>>> +++ b/drivers/gpu/drm/i915/intel_device_info.h
>>>>> @@ -74,21 +74,25 @@ enum intel_platform {
>>>>>           INTEL_MAX_PLATFORMS
>>>>>    };
>>>>>
>>>>> +enum intel_ppgtt {
>>>>> +       INTEL_PPGTT_NONE = 0,
>>>>> +       INTEL_PPGTT_ALIASING,
>>>>> +       INTEL_PPGTT_FULL,
>>>>> +       INTEL_PPGTT_FULL_4LVL,
>>>>> +};
>>>>> +
>>>>>    #define DEV_INFO_FOR_EACH_FLAG(func) \
>>>>>           func(is_mobile); \
>>>>>           func(is_lp); \
>>>>>           func(is_alpha_support); \
>>>>>           /* Keep has_* in alphabetical order */ \
>>>>>           func(has_64bit_reloc); \
>>>>> -       func(has_aliasing_ppgtt); \
>>>>>           func(has_csr); \
>>>>>           func(has_ddi); \
>>>>>           func(has_dp_mst); \
>>>>>           func(has_reset_engine); \
>>>>>           func(has_fbc); \
>>>>>           func(has_fpga_dbg); \
>>>>> -       func(has_full_ppgtt); \
>>>>> -       func(has_full_48bit_ppgtt); \
>>>>>           func(has_gmch_display); \
>>>>>           func(has_guc); \
>>>>>           func(has_guc_ct); \
>>>>> @@ -154,6 +158,7 @@ struct intel_device_info {
>>>>>           enum intel_platform platform;
>>>>>           u32 platform_mask;
>>>>>
>>>>> +       enum intel_ppgtt ppgtt;
>>>>>           unsigned int page_sizes; /* page sizes supported by the HW */
>>>>>
>>>>>           u32 display_mmio_offset;
>>>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
>>> b/drivers/gpu/drm/i915/intel_lrc.c
>>>>> index 4b28225320ff..3836a8a3761a 100644
>>>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>>>> @@ -418,9 +418,8 @@ execlists_update_context_pdps(struct
>>> i915_hw_ppgtt *ppgtt, u32 *reg_state)
>>>>>
>>>>>    static u64 execlists_update_context(struct i915_request *rq)
>>>>>    {
>>>>> +       struct i915_hw_ppgtt *ppgtt = rq->gem_context->ppgtt;
>>>>>           struct intel_context *ce = rq->hw_context;
>>>>> -       struct i915_hw_ppgtt *ppgtt =
>>>>> -               rq->gem_context->ppgtt ?: rq->i915->mm.aliasing_ppgtt;
>>>>>           u32 *reg_state = ce->lrc_reg_state;
>>>>>
>>>>>           reg_state[CTX_RING_TAIL+1] = intel_ring_set_tail(rq->ring, rq->tail);
>>>>> @@ -1376,7 +1375,7 @@ execlists_context_pin(struct intel_engine_cs
>>> *engine,
>>>>>           struct intel_context *ce = to_intel_context(ctx, engine);
>>>>>
>>>>>           lockdep_assert_held(&ctx->i915->drm.struct_mutex);
>>>>> -       GEM_BUG_ON(!(ctx->ppgtt ?: ctx->i915->mm.aliasing_ppgtt));
>>>>> +       GEM_BUG_ON(!ctx->ppgtt);
>>>>>
>>>>>           if (likely(ce->pin_count++))
>>>>>                   return ce;
>>>>> @@ -2031,8 +2030,7 @@ static int gen8_emit_bb_start(struct
>>> i915_request *rq,
>>>>>            * it is unsafe in case of lite-restore (because the ctx is
>>>>>            * not idle). PML4 is allocated during ppgtt init so this is
>>>>>            * not needed in 48-bit.*/
>>>>> -       if (rq->gem_context->ppgtt &&
>>>>> -           (intel_engine_flag(rq->engine) & rq->gem_context->ppgtt-
>>>> pd_dirty_rings) &&
>>>>> +       if ((intel_engine_flag(rq->engine) & rq->gem_context->ppgtt-
>>>> pd_dirty_rings) &&
>>>>>               !i915_vm_is_48bit(&rq->gem_context->ppgtt->vm) &&
>>>>>               !intel_vgpu_active(rq->i915)) {
>>>>>                   ret = intel_logical_ring_emit_pdps(rq);
>>>>> @@ -2634,7 +2632,6 @@ static void execlists_init_reg_state(u32 *regs,
>>>>>                                        struct intel_ring *ring)
>>>>>    {
>>>>>           struct drm_i915_private *dev_priv = engine->i915;
>>>>> -       struct i915_hw_ppgtt *ppgtt = ctx->ppgtt ?: dev_priv-
>>>> mm.aliasing_ppgtt;
>>>>>           u32 base = engine->mmio_base;
>>>>>           bool rcs = engine->class == RENDER_CLASS;
>>>>>
>>>>> @@ -2706,12 +2703,12 @@ static void execlists_init_reg_state(u32 *regs,
>>>>>           CTX_REG(regs, CTX_PDP0_UDW, GEN8_RING_PDP_UDW(engine, 0),
>>> 0);
>>>>>           CTX_REG(regs, CTX_PDP0_LDW, GEN8_RING_PDP_LDW(engine, 0),
>>> 0);
>>>>>
>>>>> -       if (i915_vm_is_48bit(&ppgtt->vm)) {
>>>>> +       if (i915_vm_is_48bit(&ctx->ppgtt->vm)) {
>>>>>                   /* 64b PPGTT (48bit canonical)
>>>>>                    * PDP0_DESCRIPTOR contains the base address to PML4 and
>>>>>                    * other PDP Descriptors are ignored.
>>>>>                    */
>>>>> -               ASSIGN_CTX_PML4(ppgtt, regs);
>>>>> +               ASSIGN_CTX_PML4(ctx->ppgtt, regs);
>>>>>           }
>>>>>
>>>>>           if (rcs) {
>>>>> diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c
>>> b/drivers/gpu/drm/i915/selftests/huge_pages.c
>>>>> index 8d03f64eabd7..09ea65a29d98 100644
>>>>> --- a/drivers/gpu/drm/i915/selftests/huge_pages.c
>>>>> +++ b/drivers/gpu/drm/i915/selftests/huge_pages.c
>>>>> @@ -1436,7 +1436,7 @@ static int igt_ppgtt_pin_update(void *arg)
>>>>>            * huge-gtt-pages.
>>>>>            */
>>>>>
>>>>> -       if (!USES_FULL_48BIT_PPGTT(dev_priv)) {
>>>>> +       if (!HAS_FULL_48BIT_PPGTT(dev_priv)) {
>>>>>                   pr_info("48b PPGTT not supported, skipping\n");
>>>>>                   return 0;
>>>>>           }
>>>>> @@ -1687,10 +1687,9 @@ int
>>> i915_gem_huge_page_mock_selftests(void)
>>>>>                   SUBTEST(igt_mock_ppgtt_huge_fill),
>>>>>                   SUBTEST(igt_mock_ppgtt_64K),
>>>>>           };
>>>>> -       int saved_ppgtt = i915_modparams.enable_ppgtt;
>>>>>           struct drm_i915_private *dev_priv;
>>>>> -       struct pci_dev *pdev;
>>>>>           struct i915_hw_ppgtt *ppgtt;
>>>>> +       struct pci_dev *pdev;
>>>>>           int err;
>>>>>
>>>>>           dev_priv = mock_gem_device();
>>>>> @@ -1698,7 +1697,7 @@ int i915_gem_huge_page_mock_selftests(void)
>>>>>                   return -ENOMEM;
>>>>>
>>>>>           /* Pretend to be a device which supports the 48b PPGTT */
>>>>> -       i915_modparams.enable_ppgtt = 3;
>>>>> +       mkwrite_device_info(dev_priv)->ppgtt = INTEL_PPGTT_FULL_4LVL;
>>>>>
>>>>>           pdev = dev_priv->drm.pdev;
>>>>>           dma_coerce_mask_and_coherent(&pdev->dev, DMA_BIT_MASK(39));
>>>>> @@ -1731,9 +1730,6 @@ int i915_gem_huge_page_mock_selftests(void)
>>>>>
>>>>>    out_unlock:
>>>>>           mutex_unlock(&dev_priv->drm.struct_mutex);
>>>>> -
>>>>> -       i915_modparams.enable_ppgtt = saved_ppgtt;
>>>>> -
>>>>>           drm_dev_put(&dev_priv->drm);
>>>>>
>>>>>           return err;
>>>>> @@ -1753,7 +1749,7 @@ int i915_gem_huge_page_live_selftests(struct
>>> drm_i915_private *dev_priv)
>>>>>           struct i915_gem_context *ctx;
>>>>>           int err;
>>>>>
>>>>> -       if (!USES_PPGTT(dev_priv)) {
>>>>> +       if (!HAS_PPGTT(dev_priv)) {
>>>>>                   pr_info("PPGTT not supported, skipping live-selftests\n");
>>>>>                   return 0;
>>>>>           }
>>>>> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
>>> b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
>>>>> index 76df25aa90c9..913c0f83f86a 100644
>>>>> --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c
>>>>> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c
>>>>> @@ -535,7 +535,6 @@ static int igt_ctx_exec(void *arg)
>>>>>           IGT_TIMEOUT(end_time);
>>>>>           LIST_HEAD(objects);
>>>>>           unsigned long ncontexts, ndwords, dw;
>>>>> -       bool first_shared_gtt = true;
>>>>>           int err = -ENODEV;
>>>>>
>>>>>           /*
>>>>> @@ -561,12 +560,7 @@ static int igt_ctx_exec(void *arg)
>>>>>                   struct i915_gem_context *ctx;
>>>>>                   unsigned int id;
>>>>>
>>>>> -               if (first_shared_gtt) {
>>>>> -                       ctx = __create_hw_context(i915, file->driver_priv);
>>>>> -                       first_shared_gtt = false;
>>>>> -               } else {
>>>>> -                       ctx = i915_gem_create_context(i915, file->driver_priv);
>>>>> -               }
>>>>> +               ctx = i915_gem_create_context(i915, file->driver_priv);
>>>>>                   if (IS_ERR(ctx)) {
>>>>>                           err = PTR_ERR(ctx);
>>>>>                           goto out_unlock;
>>>>> @@ -865,33 +859,6 @@ static int igt_switch_to_kernel_context(void
>>> *arg)
>>>>>           return err;
>>>>>    }
>>>>>
>>>>> -static int fake_aliasing_ppgtt_enable(struct drm_i915_private *i915)
>>>>> -{
>>>>> -       struct drm_i915_gem_object *obj;
>>>>> -       int err;
>>>>> -
>>>>> -       err = i915_gem_init_aliasing_ppgtt(i915);
>>>>> -       if (err)
>>>>> -               return err;
>>>>> -
>>>>> -       list_for_each_entry(obj, &i915->mm.bound_list, mm.link) {
>>>>> -               struct i915_vma *vma;
>>>>> -
>>>>> -               vma = i915_vma_instance(obj, &i915->ggtt.vm, NULL);
>>>>> -               if (IS_ERR(vma))
>>>>> -                       continue;
>>>>> -
>>>>> -               vma->flags &= ~I915_VMA_LOCAL_BIND;
>>>>> -       }
>>>>> -
>>>>> -       return 0;
>>>>> -}
>>>>> -
>>>>> -static void fake_aliasing_ppgtt_disable(struct drm_i915_private *i915)
>>>>> -{
>>>>> -       i915_gem_fini_aliasing_ppgtt(i915);
>>>>> -}
>>>>> -
>>>>>    int i915_gem_context_mock_selftests(void)
>>>>>    {
>>>>>           static const struct i915_subtest tests[] = {
>>>>> @@ -918,31 +885,9 @@ int i915_gem_context_live_selftests(struct
>>> drm_i915_private *dev_priv)
>>>>>                   SUBTEST(igt_ctx_exec),
>>>>>                   SUBTEST(igt_ctx_readonly),
>>>>>           };
>>>>> -       bool fake_alias = false;
>>>>> -       int err;
>>>>>
>>>>>           if (i915_terminally_wedged(&dev_priv->gpu_error))
>>>>>                   return 0;
>>>>>
>>>>> -       /* Install a fake aliasing gtt for exercise */
>>>>> -       if (USES_PPGTT(dev_priv) && !dev_priv->mm.aliasing_ppgtt) {
>>>>> -               mutex_lock(&dev_priv->drm.struct_mutex);
>>>>> -               err = fake_aliasing_ppgtt_enable(dev_priv);
>>>>> -               mutex_unlock(&dev_priv->drm.struct_mutex);
>>>>> -               if (err)
>>>>> -                       return err;
>>>>> -
>>>>> -               GEM_BUG_ON(!dev_priv->mm.aliasing_ppgtt);
>>>>> -               fake_alias = true;
>>>>> -       }
>>>>> -
>>>>> -       err = i915_subtests(tests, dev_priv);
>>>>> -
>>>>> -       if (fake_alias) {
>>>>> -               mutex_lock(&dev_priv->drm.struct_mutex);
>>>>> -               fake_aliasing_ppgtt_disable(dev_priv);
>>>>> -               mutex_unlock(&dev_priv->drm.struct_mutex);
>>>>> -       }
>>>>> -
>>>>> -       return err;
>>>>> +       return i915_subtests(tests, dev_priv);
>>>>>    }
>>>>> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
>>> b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
>>>>> index 128ad1cf0647..4365979d8222 100644
>>>>> --- a/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
>>>>> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_evict.c
>>>>> @@ -351,7 +351,7 @@ static int igt_evict_contexts(void *arg)
>>>>>            * where the GTT space of the request is separate from the GGTT
>>>>>            * allocation required to build the request.
>>>>>            */
>>>>> -       if (!USES_FULL_PPGTT(i915))
>>>>> +       if (!HAS_FULL_PPGTT(i915))
>>>>>                   return 0;
>>>>>
>>>>>           mutex_lock(&i915->drm.struct_mutex);
>>>>> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
>>> b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
>>>>> index 8e2e269db97e..17b5aaaa7a50 100644
>>>>> --- a/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
>>>>> +++ b/drivers/gpu/drm/i915/selftests/i915_gem_gtt.c
>>>>> @@ -153,7 +153,7 @@ static int igt_ppgtt_alloc(void *arg)
>>>>>
>>>>>           /* Allocate a ppggt and try to fill the entire range */
>>>>>
>>>>> -       if (!USES_PPGTT(dev_priv))
>>>>> +       if (!HAS_PPGTT(dev_priv))
>>>>>                   return 0;
>>>>>
>>>>>           ppgtt = __hw_ppgtt_create(dev_priv);
>>>>> @@ -1001,7 +1001,7 @@ static int exercise_ppgtt(struct
>>> drm_i915_private *dev_priv,
>>>>>           IGT_TIMEOUT(end_time);
>>>>>           int err;
>>>>>
>>>>> -       if (!USES_FULL_PPGTT(dev_priv))
>>>>> +       if (!HAS_FULL_PPGTT(dev_priv))
>>>>>                   return 0;
>>>>>
>>>>>           file = mock_file(dev_priv);
>>>>> --
>>>>> 2.19.0
>>>>>


More information about the Intel-gfx mailing list