[Intel-gfx] [PATCH 1/3] drm/i915: Invalidate the guc ggtt TLB upon insertion
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Wed Jan 11 15:51:00 UTC 2017
On 11/01/2017 13:13, Chris Wilson wrote:
> Move the GuC invalidation of its ggtt TLB to where we perform the ggtt
> modification rather than proliferate it into all the callers of the
> insert (which may or may not in fact have to do the insertion).
>
> v2: Just do the guc invalidate unconditionally, (afaict) it has no impact
> without the guc loaded on gen8+
> v3: Conditionally invalidate the guc - just in case that register has
> not been validated for other modes.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at linux.intel.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> ---
> drivers/gpu/drm/i915/i915_gem_gtt.c | 78 +++++++++++++++++++-----------
> drivers/gpu/drm/i915/i915_gem_gtt.h | 3 ++
> drivers/gpu/drm/i915/i915_guc_submission.c | 3 --
> drivers/gpu/drm/i915/intel_guc_loader.c | 7 +--
> drivers/gpu/drm/i915/intel_lrc.c | 6 ---
> 5 files changed, 57 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 0ed99adfd0da..ed120a1e7f93 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -110,6 +110,30 @@ const struct i915_ggtt_view i915_ggtt_view_rotated = {
> .type = I915_GGTT_VIEW_ROTATED,
> };
>
> +static void gen6_ggtt_invalidate(struct drm_i915_private *dev_priv)
> +{
> + /* Note that as an uncached mmio write, this should flush the
> + * WCB of the writes into the GGTT before it triggers the invalidate.
> + */
> + I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> +}
> +
> +static void guc_ggtt_invalidate(struct drm_i915_private *dev_priv)
> +{
> + gen6_ggtt_invalidate(dev_priv);
> + I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
> +}
> +
> +static void gmch_ggtt_invalidate(struct drm_i915_private *dev_priv)
> +{
> + intel_gtt_chipset_flush();
> +}
> +
> +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)
> {
> @@ -2307,16 +2331,6 @@ void i915_check_and_clear_faults(struct drm_i915_private *dev_priv)
> POSTING_READ(RING_FAULT_REG(dev_priv->engine[RCS]));
> }
>
> -static void i915_ggtt_flush(struct drm_i915_private *dev_priv)
> -{
> - if (INTEL_INFO(dev_priv)->gen < 6) {
> - intel_gtt_chipset_flush();
> - } else {
> - I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> - POSTING_READ(GFX_FLSH_CNTL_GEN6);
> - }
> -}
> -
> void i915_gem_suspend_gtt_mappings(struct drm_i915_private *dev_priv)
> {
> struct i915_ggtt *ggtt = &dev_priv->ggtt;
> @@ -2331,7 +2345,7 @@ void i915_gem_suspend_gtt_mappings(struct drm_i915_private *dev_priv)
>
> ggtt->base.clear_range(&ggtt->base, ggtt->base.start, ggtt->base.total);
>
> - i915_ggtt_flush(dev_priv);
> + i915_ggtt_invalidate(dev_priv);
> }
>
> int i915_gem_gtt_prepare_pages(struct drm_i915_gem_object *obj,
> @@ -2370,15 +2384,13 @@ static void gen8_ggtt_insert_page(struct i915_address_space *vm,
> enum i915_cache_level level,
> u32 unused)
> {
> - struct drm_i915_private *dev_priv = vm->i915;
> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> gen8_pte_t __iomem *pte =
> - (gen8_pte_t __iomem *)dev_priv->ggtt.gsm +
> - (offset >> PAGE_SHIFT);
> + (gen8_pte_t __iomem *)ggtt->gsm + (offset >> PAGE_SHIFT);
>
> gen8_set_pte(pte, gen8_pte_encode(addr, level));
>
> - I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> - POSTING_READ(GFX_FLSH_CNTL_GEN6);
> + ggtt->invalidate(vm->i915);
> }
>
> static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
> @@ -2386,7 +2398,6 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
> uint64_t start,
> enum i915_cache_level level, u32 unused)
> {
> - struct drm_i915_private *dev_priv = vm->i915;
> struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> struct sgt_iter sgt_iter;
> gen8_pte_t __iomem *gtt_entries;
> @@ -2415,8 +2426,7 @@ static void gen8_ggtt_insert_entries(struct i915_address_space *vm,
> * want to flush the TLBs only after we're certain all the PTE updates
> * have finished.
> */
> - I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> - POSTING_READ(GFX_FLSH_CNTL_GEN6);
> + ggtt->invalidate(vm->i915);
> }
>
> struct insert_entries {
> @@ -2451,15 +2461,13 @@ static void gen6_ggtt_insert_page(struct i915_address_space *vm,
> enum i915_cache_level level,
> u32 flags)
> {
> - struct drm_i915_private *dev_priv = vm->i915;
> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> gen6_pte_t __iomem *pte =
> - (gen6_pte_t __iomem *)dev_priv->ggtt.gsm +
> - (offset >> PAGE_SHIFT);
> + (gen6_pte_t __iomem *)ggtt->gsm + (offset >> PAGE_SHIFT);
>
> iowrite32(vm->pte_encode(addr, level, flags), pte);
>
> - I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> - POSTING_READ(GFX_FLSH_CNTL_GEN6);
> + ggtt->invalidate(vm->i915);
> }
>
> /*
> @@ -2473,7 +2481,6 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
> uint64_t start,
> enum i915_cache_level level, u32 flags)
> {
> - struct drm_i915_private *dev_priv = vm->i915;
> struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> struct sgt_iter sgt_iter;
> gen6_pte_t __iomem *gtt_entries;
> @@ -2501,8 +2508,7 @@ static void gen6_ggtt_insert_entries(struct i915_address_space *vm,
> * want to flush the TLBs only after we're certain all the PTE updates
> * have finished.
> */
> - I915_WRITE(GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
> - POSTING_READ(GFX_FLSH_CNTL_GEN6);
> + ggtt->invalidate(vm->i915);
> }
>
> static void nop_clear_range(struct i915_address_space *vm,
> @@ -3062,6 +3068,8 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
> if (IS_CHERRYVIEW(dev_priv))
> ggtt->base.insert_entries = gen8_ggtt_insert_entries__BKL;
>
> + ggtt->invalidate = gen6_ggtt_invalidate;
> +
> return ggtt_probe_common(ggtt, size);
> }
>
> @@ -3099,6 +3107,8 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt)
> ggtt->base.unbind_vma = ggtt_unbind_vma;
> ggtt->base.cleanup = gen6_gmch_remove;
>
> + ggtt->invalidate = gen6_ggtt_invalidate;
> +
> if (HAS_EDRAM(dev_priv))
> ggtt->base.pte_encode = iris_pte_encode;
> else if (IS_HASWELL(dev_priv))
> @@ -3142,6 +3152,8 @@ static int i915_gmch_probe(struct i915_ggtt *ggtt)
> ggtt->base.unbind_vma = ggtt_unbind_vma;
> ggtt->base.cleanup = i915_gmch_remove;
>
> + ggtt->invalidate = gmch_ggtt_invalidate;
> +
> if (unlikely(ggtt->do_idle_maps))
> DRM_INFO("applying Ironlake quirks for intel_iommu\n");
>
> @@ -3260,6 +3272,16 @@ int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv)
> return 0;
> }
>
> +void i915_ggtt_enable_guc(struct drm_i915_private *i915)
> +{
> + i915->ggtt.invalidate = guc_ggtt_invalidate;
> +}
> +
> +void i915_ggtt_disable_guc(struct drm_i915_private *i915)
> +{
> + i915->ggtt.invalidate = gen6_ggtt_invalidate;
> +}
> +
> void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
> {
> struct i915_ggtt *ggtt = &dev_priv->ggtt;
> @@ -3323,7 +3345,7 @@ void i915_gem_restore_gtt_mappings(struct drm_i915_private *dev_priv)
> }
> }
>
> - i915_ggtt_flush(dev_priv);
> + i915_ggtt_invalidate(dev_priv);
> }
>
> struct i915_vma *
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index 3e031a057f78..6c40088f8cf4 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -336,6 +336,7 @@ struct i915_ggtt {
>
> /** "Graphics Stolen Memory" holds the global PTEs */
> void __iomem *gsm;
> + void (*invalidate)(struct drm_i915_private *dev_priv);
>
> bool do_idle_maps;
>
> @@ -504,6 +505,8 @@ i915_vm_to_ggtt(struct i915_address_space *vm)
> int i915_ggtt_probe_hw(struct drm_i915_private *dev_priv);
> int i915_ggtt_init_hw(struct drm_i915_private *dev_priv);
> int i915_ggtt_enable_hw(struct drm_i915_private *dev_priv);
> +void i915_ggtt_enable_guc(struct drm_i915_private *i915);
> +void i915_ggtt_disable_guc(struct drm_i915_private *i915);
> int i915_gem_init_ggtt(struct drm_i915_private *dev_priv);
> void i915_ggtt_cleanup_hw(struct drm_i915_private *dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 710fbb9fc63f..913d87358972 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -579,9 +579,6 @@ static struct i915_vma *guc_allocate_vma(struct intel_guc *guc, u32 size)
> goto err;
> }
>
> - /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */
> - I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
> -
> return vma;
>
> err:
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index aa2b866474be..4d26965e3f7f 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -366,9 +366,6 @@ static int guc_ucode_xfer(struct drm_i915_private *dev_priv)
> return PTR_ERR(vma);
> }
>
> - /* Invalidate GuC TLB to let GuC take the latest updates to GTT. */
> - I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
> -
> intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
>
> /* init WOPCM */
> @@ -486,6 +483,9 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
> guc_interrupts_release(dev_priv);
> gen9_reset_guc_interrupts(dev_priv);
>
> + /* We need to notify the guc whenever we change the GGTT */
> + i915_ggtt_enable_guc(dev_priv);
> +
> guc_fw->guc_fw_load_status = GUC_FIRMWARE_PENDING;
>
> DRM_DEBUG_DRIVER("GuC fw status: fetch %s, load %s\n",
> @@ -547,6 +547,7 @@ int intel_guc_setup(struct drm_i915_private *dev_priv)
> guc_interrupts_release(dev_priv);
> i915_guc_submission_disable(dev_priv);
> i915_guc_submission_fini(dev_priv);
> + i915_ggtt_disable_guc(dev_priv);
>
> /*
> * We've failed to load the firmware :(
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 81665a9eb43f..d9de455da131 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -811,12 +811,6 @@ static int execlists_context_pin(struct intel_engine_cs *engine,
>
> ce->state->obj->mm.dirty = true;
>
> - /* Invalidate GuC TLB. */
> - if (i915.enable_guc_submission) {
> - struct drm_i915_private *dev_priv = ctx->i915;
> - I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
> - }
> -
> i915_gem_context_get(ctx);
> return 0;
>
>
Looks good to me.
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Regards,
Tvrtko
More information about the Intel-gfx
mailing list