[Intel-gfx] [PATCH] drm/i915/mtl: Media GT and Render GT share common GGTT
Lucas De Marchi
lucas.demarchi at intel.com
Fri Nov 4 07:05:42 UTC 2022
On Mon, Oct 31, 2022 at 06:01:11PM +0530, 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.
>
>BSPEC: 63834
>
>Cc: Matt Roper <matthew.d.roper at intel.com>
>Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty at intel.com>
>---
> drivers/gpu/drm/i915/gt/intel_ggtt.c | 49 +++++++++++-------
> drivers/gpu/drm/i915/gt/intel_gt.c | 15 +++++-
> drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 ++
> drivers/gpu/drm/i915/gt/intel_gtt.h | 3 ++
> drivers/gpu/drm/i915/i915_driver.c | 19 +++++--
> drivers/gpu/drm/i915/i915_gem_evict.c | 63 +++++++++++++++++------
> drivers/gpu/drm/i915/i915_vma.c | 5 +-
> drivers/gpu/drm/i915/selftests/i915_gem.c | 2 +
> drivers/gpu/drm/i915/selftests/mock_gtt.c | 1 +
> 9 files changed, 115 insertions(+), 45 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
>index 2518cebbf931..f5c2f3c58627 100644
>--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
>+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
>@@ -196,10 +196,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)
>@@ -214,27 +217,36 @@ void gen6_ggtt_invalidate(struct i915_ggtt *ggtt)
>
> static void gen8_ggtt_invalidate(struct i915_ggtt *ggtt)
> {
>- struct intel_uncore *uncore = ggtt->vm.gt->uncore;
>+ struct intel_uncore *uncore;
>+ struct intel_gt *gt;
>
>- /*
>- * Note that as an uncached mmio write, this will flush the
>- * WCB of the writes into the GGTT before it triggers the invalidate.
>- */
>- intel_uncore_write_fw(uncore, GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
>+ list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) {
>+ uncore = gt->uncore;
>+ /*
>+ * Note that as an uncached mmio write, this will flush the
>+ * WCB of the writes into the GGTT before it triggers the invalidate.
>+ */
>+ intel_uncore_write_fw(uncore, GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN);
>+ }
> }
>
> 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 +998,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 +1196,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;
> }
>
>@@ -1296,9 +1306,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 +1319,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.c
>index 2e796ffad911..d72efb74563a 100644
>--- a/drivers/gpu/drm/i915/gt/intel_gt.c
>+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
>@@ -110,9 +110,17 @@ static int intel_gt_probe_lmem(struct intel_gt *gt)
>
> int intel_gt_assign_ggtt(struct intel_gt *gt)
> {
>- gt->ggtt = drmm_kzalloc(>->i915->drm, sizeof(*gt->ggtt), GFP_KERNEL);
>+ /* Media GT shares primary GT's GGTT */
>+ if (gt->type == GT_MEDIA) {
>+ gt->ggtt = to_gt(gt->i915)->ggtt;
>+ } else {
>+ gt->ggtt = drmm_kzalloc(>->i915->drm, sizeof(*gt->ggtt), GFP_KERNEL);
>+ if (!gt->ggtt)
>+ return -ENOMEM;
>+ }
>
>- return gt->ggtt ? 0 : -ENOMEM;
>+ list_add_tail(>->ggtt_link, >->ggtt->gt_list);
I don't think this really works. You're adding to >->ggtt->gt_list
that you just allocated above. I had this patch applied and noticed
this:
[ 151.417250] ------------[ cut here ]------------
[ 151.417251] list_add corruption. prev is NULL.
[ 151.417254] WARNING: CPU: 10 PID: 1780 at lib/list_debug.c:23 __list_add_valid+0x3e/0xb0
[ 151.417259] Modules linked in: i915(+) prime_numbers drm_buddy drm_display_helper drm_kms_helper syscopyarea sysfillrect sysimgblt fb_sys_fops ttm overlay fuse x86_pkg_temp_thermal coretemp crct10dif_pclmul crc32_pclmul wmi_bmof ghash_clmulni_intel kvm_intel video wmi ip_tables x_tables e1000e igb dca ptp i2c_i801 i2c_smbus pps_core
[ 151.417299] CPU: 10 PID: 1780 Comm: modprobe Tainted: G W 6.0.0 #382
[ 151.417302] XXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXXX
[ 151.417304] RIP: 0010:__list_add_valid+0x3e/0xb0
[ 151.417307] Code: 75 46 4c 8b 0a 4d 39 c1 75 56 48 39 fa 74 6f 49 39 f9 74 6a b8 01 00 00 00 c3 cc cc cc cc 48 c7 c7 c0 57 5f 82 e8 96 9c 61 00 <0f> 0b 31 c0 c3 cc cc cc cc 48 c7 c7 e8 57 5f 82 e8 81 9c 61 00 0f
[ 151.417309] RSP: 0018:ffffc90005787b80 EFLAGS: 00010282
[ 151.417313] RAX: 0000000000000000 RBX: ffff8881428fb778 RCX: 0000000000000000
[ 151.417315] RDX: 0000000000000001 RSI: ffffffff825e0c4c RDI: 00000000ffffffff
[ 151.417316] RBP: ffff88813f6c3828 R08: ffff88849effffe8 R09: 00000000fffdffff
[ 151.417318] R10: ffff88849e200000 R11: ffff88849ed00000 R12: 0000000000000000
[ 151.417320] R13: ffff8881428fdaa0 R14: ffff88813f6c3f50 R15: ffff8881428fdab0
[ 151.417322] FS: 00007fc71052d540(0000) GS:ffff88849f900000(0000) knlGS:0000000000000000
[ 151.417325] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 151.417327] CR2: 0000555a8fad6858 CR3: 000000013b7f4006 CR4: 0000000000770ee0
[ 151.417329] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 151.417331] DR3: 0000000000000000 DR6: 00000000ffff07f0 DR7: 0000000000000400
[ 151.417332] PKRU: 55555554
[ 151.417333] Call Trace:
[ 151.417335] <TASK>
[ 151.417336] intel_gt_assign_ggtt+0x42/0xa0 [i915]
I think something like below would do it (untested):
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index c1e23b4be8ed6..454b668108457 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -8,6 +8,7 @@
#include <linux/types.h>
#include <linux/stop_machine.h>
+#include <drm/drm_managed.h>
#include <drm/i915_drm.h>
#include <drm/intel-gtt.h>
@@ -1196,7 +1197,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;
}
@@ -1218,6 +1219,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)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c
index 143359acde5a7..ea3b895dbe6b7 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -115,12 +115,13 @@ int intel_gt_assign_ggtt(struct intel_gt *gt)
if (gt->type == GT_MEDIA) {
gt->ggtt = to_gt(gt->i915)->ggtt;
} else {
- gt->ggtt = drmm_kzalloc(>->i915->drm, sizeof(*gt->ggtt), GFP_KERNEL);
- if (!gt->ggtt)
- return -ENOMEM;
+ gt->ggtt = i915_ggtt_create(gt->i915);
+ if (IS_ERR(gt->ggtt))
+ return PTR_ERR(gt->ggtt);
}
list_add_tail(>->ggtt_link, >->ggtt->gt_list);
+
return 0;
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
index cb1272702a1a1..f2a608c182c82 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -585,6 +585,7 @@ int i915_ggtt_enable_hw(struct drm_i915_private *i915);
void i915_ggtt_enable_guc(struct i915_ggtt *ggtt);
void i915_ggtt_disable_guc(struct i915_ggtt *ggtt);
int i915_init_ggtt(struct drm_i915_private *i915);
+struct i915_ggtt *i915_ggtt_create(struct drm_i915_private *i915);
void i915_ggtt_driver_release(struct drm_i915_private *i915);
void i915_ggtt_driver_late_release(struct drm_i915_private *i915);
--
2.38.1
More information about the Intel-gfx
mailing list