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

Wang, Zhi A zhi.a.wang at intel.com
Mon Oct 8 13:58:25 UTC 2018


Thanks for pointing this. My bad.

I take a look on the code and it looks like the GVT-g context is now quite similar with the kernel context except the force single submission and ring buffer size. (When we upstream the code, there was no kernel context at that time. :/) Now there is already one API for single submission. If we can introduce another one to configure the ring buffer size. Then maybe we can remove the *create_gvt() in i915_gem_context.c and used kernel context instead.

Feel free to drop comments. :)

Thanks,
Zhi.

-----Original Message-----
From: Zhenyu Wang [mailto:zhenyuw at linux.intel.com] 
Sent: Monday, October 8, 2018 12:54 PM
To: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
Cc: He, Min <min.he at intel.com>; Wang, Zhi A <zhi.a.wang at intel.com>; intel-gfx at lists.freedesktop.org; Chris Wilson <chris at chris-wilson.co.uk>; Zhenyu Wang <zhenyuw at linux.intel.com>; Auld, Matthew <matthew.auld at intel.com>
Subject: Re: [PATCH v4] drm/i915: Remove i915.enable_ppgtt override

On 2018.09.26 11:02:15 +0300, 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?

Looks that one unfortunately caused gvt regression, gvt context has no i915 managed i915_hw_ppgtt but only shadowed one, so need to check if ppgtt is valid before access.

> 
> 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
> > > >>

--
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827


More information about the Intel-gfx mailing list