Re: [Intel-gfx] [PATCH v2] drm/i915/mtl: Media GT and Render GT share common GGTT
On Wed, 09 Nov 2022, Aravind Iddamsetty wrote: > On XE_LPM+ platforms the media engines are carved out into a separate > GT but have a common GGTMMADR address range which essentially makes > the GGTT address space to be shared between media and render GT. As a > result any updates in GGTT shall invalidate TLB of GTs sharing it and > similarly any operation on GGTT requiring an action on a GT will have to > involve all GTs sharing it. setup_private_pat was being done on a per > GGTT based as that doesn't touch any GGTT structures moved it to per GT > based. > > BSPEC: 63834 > > v2: > 1. Add details to commit msg > 2. includes fix for failure to add item to ggtt->gt_list, as suggested > by Lucas > 3. as ggtt_flush() is used only for ggtt drop i915_is_ggtt check within > it. > 4. setup_private_pat moved out of intel_gt_tiles_init > > Cc: Matt Roper > Signed-off-by: Aravind Iddamsetty > --- > drivers/gpu/drm/i915/gt/intel_ggtt.c | 47 ++--- > drivers/gpu/drm/i915/gt/intel_gt.c| 13 +- > drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 ++ > drivers/gpu/drm/i915/gt/intel_gtt.h | 4 ++ > drivers/gpu/drm/i915/i915_driver.c| 22 +++--- > drivers/gpu/drm/i915/i915_gem_evict.c | 51 +-- > drivers/gpu/drm/i915/i915_vma.c | 5 ++- > drivers/gpu/drm/i915/selftests/i915_gem.c | 2 + > 8 files changed, 112 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c > b/drivers/gpu/drm/i915/gt/intel_ggtt.c > index 2518cebbf931..6ba7e9e8e2ca 100644 > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c > @@ -8,6 +8,7 @@ > #include > #include > > +#include > #include > #include > > @@ -196,10 +197,13 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm) > > void i915_ggtt_suspend(struct i915_ggtt *ggtt) > { > + struct intel_gt *gt; > + > i915_ggtt_suspend_vm(&ggtt->vm); > ggtt->invalidate(ggtt); > > - intel_gt_check_and_clear_faults(ggtt->vm.gt); > + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) > + intel_gt_check_and_clear_faults(gt); > } > > void gen6_ggtt_invalidate(struct i915_ggtt *ggtt) > @@ -225,16 +229,21 @@ static void gen8_ggtt_invalidate(struct i915_ggtt *ggtt) > > static void guc_ggtt_invalidate(struct i915_ggtt *ggtt) > { > - struct intel_uncore *uncore = ggtt->vm.gt->uncore; > struct drm_i915_private *i915 = ggtt->vm.i915; > > gen8_ggtt_invalidate(ggtt); > > - if (GRAPHICS_VER(i915) >= 12) > - intel_uncore_write_fw(uncore, GEN12_GUC_TLB_INV_CR, > - GEN12_GUC_TLB_INV_CR_INVALIDATE); > - else > - intel_uncore_write_fw(uncore, GEN8_GTCR, GEN8_GTCR_INVALIDATE); > + if (GRAPHICS_VER(i915) >= 12) { > + struct intel_gt *gt; > + > + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) > + intel_uncore_write_fw(gt->uncore, > + GEN12_GUC_TLB_INV_CR, > + GEN12_GUC_TLB_INV_CR_INVALIDATE); > + } else { > + intel_uncore_write_fw(ggtt->vm.gt->uncore, > + GEN8_GTCR, GEN8_GTCR_INVALIDATE); > + } > } > > u64 gen8_ggtt_pte_encode(dma_addr_t addr, > @@ -986,8 +995,6 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt) > > ggtt->vm.pte_encode = gen8_ggtt_pte_encode; > > - setup_private_pat(ggtt->vm.gt); > - > return ggtt_probe_common(ggtt, size); > } > > @@ -1186,7 +1193,7 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct > intel_gt *gt) > (u64)ggtt->mappable_end >> 20); > drm_dbg(&i915->drm, "DSM size = %lluM\n", > (u64)resource_size(&intel_graphics_stolen_res) >> 20); > - > + INIT_LIST_HEAD(&ggtt->gt_list); > return 0; > } > > @@ -1208,6 +1215,19 @@ int i915_ggtt_probe_hw(struct drm_i915_private *i915) > return 0; > } > > +struct i915_ggtt *i915_ggtt_create(struct drm_i915_private *i915) > +{ > + struct i915_ggtt *ggtt; > + > + ggtt = drmm_kzalloc(&i915->drm, sizeof(*ggtt), GFP_KERNEL); > + if (!ggtt) > + return ERR_PTR(-ENOMEM); > + > + INIT_LIST_HEAD(&ggtt->gt_list); > + > + return ggtt; > +} > + > int i915_ggtt_enable_hw(struct drm_i915_private *i915) > { > if (GRAPHICS_VER(i915) < 6) > @@ -1296,9 +1316,11 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm) > > void i915_ggtt_resume(struct i915_ggtt *ggtt) > { > + struct intel_gt *gt; > bool flush; > > - intel_gt_check_and_clear_faults(ggtt->vm.gt); > + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) > + intel_gt_check_and_clear_faults(gt); > > flush = i915_ggtt_resume_vm(&ggtt->vm); > > @@ -1307,9 +1329,6 @@ void i915_ggtt_resume(struct i915_ggtt *ggtt) > if (flush) > wbinvd_on_all_c
[PATCH v2] drm/i915/mtl: Media GT and Render GT share common GGTT
On XE_LPM+ platforms the media engines are carved out into a separate GT but have a common GGTMMADR address range which essentially makes the GGTT address space to be shared between media and render GT. As a result any updates in GGTT shall invalidate TLB of GTs sharing it and similarly any operation on GGTT requiring an action on a GT will have to involve all GTs sharing it. setup_private_pat was being done on a per GGTT based as that doesn't touch any GGTT structures moved it to per GT based. BSPEC: 63834 v2: 1. Add details to commit msg 2. includes fix for failure to add item to ggtt->gt_list, as suggested by Lucas 3. as ggtt_flush() is used only for ggtt drop i915_is_ggtt check within it. 4. setup_private_pat moved out of intel_gt_tiles_init Cc: Matt Roper Signed-off-by: Aravind Iddamsetty --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 47 ++--- drivers/gpu/drm/i915/gt/intel_gt.c| 13 +- drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 ++ drivers/gpu/drm/i915/gt/intel_gtt.h | 4 ++ drivers/gpu/drm/i915/i915_driver.c| 22 +++--- drivers/gpu/drm/i915/i915_gem_evict.c | 51 +-- drivers/gpu/drm/i915/i915_vma.c | 5 ++- drivers/gpu/drm/i915/selftests/i915_gem.c | 2 + 8 files changed, 112 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 2518cebbf931..6ba7e9e8e2ca 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -196,10 +197,13 @@ void i915_ggtt_suspend_vm(struct i915_address_space *vm) void i915_ggtt_suspend(struct i915_ggtt *ggtt) { + struct intel_gt *gt; + i915_ggtt_suspend_vm(&ggtt->vm); ggtt->invalidate(ggtt); - intel_gt_check_and_clear_faults(ggtt->vm.gt); + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) + intel_gt_check_and_clear_faults(gt); } void gen6_ggtt_invalidate(struct i915_ggtt *ggtt) @@ -225,16 +229,21 @@ static void gen8_ggtt_invalidate(struct i915_ggtt *ggtt) static void guc_ggtt_invalidate(struct i915_ggtt *ggtt) { - struct intel_uncore *uncore = ggtt->vm.gt->uncore; struct drm_i915_private *i915 = ggtt->vm.i915; gen8_ggtt_invalidate(ggtt); - if (GRAPHICS_VER(i915) >= 12) - intel_uncore_write_fw(uncore, GEN12_GUC_TLB_INV_CR, - GEN12_GUC_TLB_INV_CR_INVALIDATE); - else - intel_uncore_write_fw(uncore, GEN8_GTCR, GEN8_GTCR_INVALIDATE); + if (GRAPHICS_VER(i915) >= 12) { + struct intel_gt *gt; + + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) + intel_uncore_write_fw(gt->uncore, + GEN12_GUC_TLB_INV_CR, + GEN12_GUC_TLB_INV_CR_INVALIDATE); + } else { + intel_uncore_write_fw(ggtt->vm.gt->uncore, + GEN8_GTCR, GEN8_GTCR_INVALIDATE); + } } u64 gen8_ggtt_pte_encode(dma_addr_t addr, @@ -986,8 +995,6 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt) ggtt->vm.pte_encode = gen8_ggtt_pte_encode; - setup_private_pat(ggtt->vm.gt); - return ggtt_probe_common(ggtt, size); } @@ -1186,7 +1193,7 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct intel_gt *gt) (u64)ggtt->mappable_end >> 20); drm_dbg(&i915->drm, "DSM size = %lluM\n", (u64)resource_size(&intel_graphics_stolen_res) >> 20); - + INIT_LIST_HEAD(&ggtt->gt_list); return 0; } @@ -1208,6 +1215,19 @@ int i915_ggtt_probe_hw(struct drm_i915_private *i915) return 0; } +struct i915_ggtt *i915_ggtt_create(struct drm_i915_private *i915) +{ + struct i915_ggtt *ggtt; + + ggtt = drmm_kzalloc(&i915->drm, sizeof(*ggtt), GFP_KERNEL); + if (!ggtt) + return ERR_PTR(-ENOMEM); + + INIT_LIST_HEAD(&ggtt->gt_list); + + return ggtt; +} + int i915_ggtt_enable_hw(struct drm_i915_private *i915) { if (GRAPHICS_VER(i915) < 6) @@ -1296,9 +1316,11 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm) void i915_ggtt_resume(struct i915_ggtt *ggtt) { + struct intel_gt *gt; bool flush; - intel_gt_check_and_clear_faults(ggtt->vm.gt); + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) + intel_gt_check_and_clear_faults(gt); flush = i915_ggtt_resume_vm(&ggtt->vm); @@ -1307,9 +1329,6 @@ void i915_ggtt_resume(struct i915_ggtt *ggtt) if (flush) wbinvd_on_all_cpus(); - if (GRAPHICS_VER(ggtt->vm.i915) >= 8) - setup_private_pat(ggtt->vm.gt); - intel_ggtt_restore_fences(ggtt); } diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt