Re: [Intel-gfx] [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
On 29-11-2022 15:41, Tvrtko Ursulin wrote: > > On 22/11/2022 07:01, 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 >> >> v3: >> 1. Move out for_each_gt from i915_driver.c (Jani Nikula) >> >> v4: drop using RCU primitives on ggtt->gt_list as it is not an RCU list >> (Matt Roper) >> >> Cc: Matt Roper >> Signed-off-by: Aravind Iddamsetty >> --- >> drivers/gpu/drm/i915/gt/intel_ggtt.c | 54 +-- >> 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 | 12 ++--- >> drivers/gpu/drm/i915/i915_gem.c | 2 + >> 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 + >> 9 files changed, 111 insertions(+), 35 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c >> b/drivers/gpu/drm/i915/gt/intel_ggtt.c >> index 8145851ad23d..7644738b9cdb 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); >> } >> @@ -1196,7 +1203,14 @@ static int ggtt_probe_hw(struct i915_ggtt >> *ggtt, struct intel_gt *gt) >> */ >> int i915_ggtt_probe_hw(struct drm_i915_private *i915) >> { >> - int ret; >> + struct intel_gt *gt; >> + int ret, i; >> + >> + for_each_gt(gt, i915, i) { >> + ret = intel_gt_assign_ggtt(gt); >> + if (ret) >> + return ret; >> + } >> ret = ggtt_probe_hw(to_gt(i915)->ggtt, to_gt(i915)); >> if (ret) >> @@ -1208,6 +1222,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 +1323,11 @@ bool i915_ggtt_resume_vm(struct >> i915_address_space *vm) >> void i915_ggtt_resume(struct i915_ggtt *ggtt) >> { >> +
Re: [Intel-gfx] [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
On 22/11/2022 07:01, 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 v3: 1. Move out for_each_gt from i915_driver.c (Jani Nikula) v4: drop using RCU primitives on ggtt->gt_list as it is not an RCU list (Matt Roper) Cc: Matt Roper Signed-off-by: Aravind Iddamsetty --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 54 +-- 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| 12 ++--- drivers/gpu/drm/i915/i915_gem.c | 2 + 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 + 9 files changed, 111 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 8145851ad23d..7644738b9cdb 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); } @@ -1196,7 +1203,14 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct intel_gt *gt) */ int i915_ggtt_probe_hw(struct drm_i915_private *i915) { - int ret; + struct intel_gt *gt; + int ret, i; + + for_each_gt(gt, i915, i) { + ret = intel_gt_assign_ggtt(gt); + if (ret) + return ret; + } ret = ggtt_probe_hw(to_gt(i915)->ggtt, to_gt(i915)); if (ret) @@ -1208,6 +1222,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 +1323,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); fl
Re: [Intel-gfx] [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
On Tue, Nov 29, 2022 at 11:33:15AM +0530, Iddamsetty, Aravind wrote: On 29-11-2022 11:24, Lucas De Marchi wrote: On Wed, Nov 23, 2022 at 09:47:03AM +0530, Iddamsetty, Aravind wrote: On 23-11-2022 05:29, Matt Roper wrote: On Tue, Nov 22, 2022 at 12:31:26PM +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. 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 v3: 1. Move out for_each_gt from i915_driver.c (Jani Nikula) v4: drop using RCU primitives on ggtt->gt_list as it is not an RCU list (Matt Roper) Cc: Matt Roper Signed-off-by: Aravind Iddamsetty Reviewed-by: Matt Roper Thanks Matt, could you also help with merging the change. Regards, Aravind. --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 54 +-- 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 | 12 ++--- drivers/gpu/drm/i915/i915_gem.c | 2 + 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 + 9 files changed, 111 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 8145851ad23d..7644738b9cdb 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); } @@ -1196,7 +1203,14 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct intel_gt *gt) */ int i915_ggtt_probe_hw(struct drm_i915_private *i915) { - int ret; + struct intel_gt *gt; + int ret, i; + + for_each_gt(gt, i915, i) { + ret = intel_gt_assign_ggtt(gt); in v3 the intel_gt_assign_ggtt() call is not in i915_driver.c anymore but rather moved here. We could make i915_ggtt_create() static, doing the allocation here and intel_gt_assign_ggtt() would be in charge of just assigning the ggtt. Not very important though and can be done later. well we call intel_gt_assign_ggtt in i915_gem_gtt_mock_selftests but not i915_ggtt_probe_hw. makes sense, let's leave it as is. Lucas De Marchi
Re: [Intel-gfx] [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
On 29-11-2022 11:24, Lucas De Marchi wrote: > On Wed, Nov 23, 2022 at 09:47:03AM +0530, Iddamsetty, Aravind wrote: >> >> >> On 23-11-2022 05:29, Matt Roper wrote: >>> On Tue, Nov 22, 2022 at 12:31:26PM +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. 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 v3: 1. Move out for_each_gt from i915_driver.c (Jani Nikula) v4: drop using RCU primitives on ggtt->gt_list as it is not an RCU list (Matt Roper) Cc: Matt Roper Signed-off-by: Aravind Iddamsetty >>> >>> Reviewed-by: Matt Roper >> >> Thanks Matt, could you also help with merging the change. >> >> Regards, >> Aravind. >>> --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 54 +-- 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 | 12 ++--- drivers/gpu/drm/i915/i915_gem.c | 2 + 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 + 9 files changed, 111 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 8145851ad23d..7644738b9cdb 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); } @@ -1196,7 +1203,14 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct intel_gt *gt) */ int i915_ggtt_probe_hw(struct drm_i915_private *i915) { - int ret; + struct intel_gt *gt; + int ret, i; + + for_each_gt(gt, i915, i) { + ret = intel_gt_assign_ggtt(gt); > > in v3 the intel_gt_assign_ggtt() call is not in i915_driver.c anymore but > rather moved here. We could make i915_ggtt_create() static, doing the > allocation here and intel_gt_assign_ggtt() would be in charge of just > assigning the ggtt. Not very important though and can be done later. well we call intel_gt_assign_ggtt in i9
Re: [Intel-gfx] [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
On Wed, Nov 23, 2022 at 09:47:03AM +0530, Iddamsetty, Aravind wrote: On 23-11-2022 05:29, Matt Roper wrote: On Tue, Nov 22, 2022 at 12:31:26PM +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. 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 v3: 1. Move out for_each_gt from i915_driver.c (Jani Nikula) v4: drop using RCU primitives on ggtt->gt_list as it is not an RCU list (Matt Roper) Cc: Matt Roper Signed-off-by: Aravind Iddamsetty Reviewed-by: Matt Roper Thanks Matt, could you also help with merging the change. Regards, Aravind. --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 54 +-- 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| 12 ++--- drivers/gpu/drm/i915/i915_gem.c | 2 + 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 + 9 files changed, 111 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 8145851ad23d..7644738b9cdb 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); } @@ -1196,7 +1203,14 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct intel_gt *gt) */ int i915_ggtt_probe_hw(struct drm_i915_private *i915) { - int ret; + struct intel_gt *gt; + int ret, i; + + for_each_gt(gt, i915, i) { + ret = intel_gt_assign_ggtt(gt); in v3 the intel_gt_assign_ggtt() call is not in i915_driver.c anymore but rather moved here. We could make i915_ggtt_create() static, doing the allocation here and intel_gt_assign_ggtt() would be in charge of just assigning the ggtt. Not very important though and can be done later. pushed, thanks Lucas De Marchi + if (ret) + return ret; + } ret = ggtt_probe_hw(to_gt(i915)->ggtt, to_gt(i915)); if (ret) @@ -1208,6 +1222,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(-ENOM
Re: [Intel-gfx] [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
On 23-11-2022 05:29, Matt Roper wrote: > On Tue, Nov 22, 2022 at 12:31:26PM +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. 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 >> >> v3: >> 1. Move out for_each_gt from i915_driver.c (Jani Nikula) >> >> v4: drop using RCU primitives on ggtt->gt_list as it is not an RCU list >> (Matt Roper) >> >> Cc: Matt Roper >> Signed-off-by: Aravind Iddamsetty > > Reviewed-by: Matt Roper Thanks Matt, could you also help with merging the change. Regards, Aravind. > >> --- >> drivers/gpu/drm/i915/gt/intel_ggtt.c | 54 +-- >> 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| 12 ++--- >> drivers/gpu/drm/i915/i915_gem.c | 2 + >> 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 + >> 9 files changed, 111 insertions(+), 35 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c >> b/drivers/gpu/drm/i915/gt/intel_ggtt.c >> index 8145851ad23d..7644738b9cdb 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); >> } >> >> @@ -1196,7 +1203,14 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, >> struct intel_gt *gt) >> */ >> int i915_ggtt_probe_hw(struct drm_i915_private *i915) >> { >> -int ret; >> +struct intel_gt *gt; >> +int ret, i; >> + >> +for_each_gt(gt, i915, i) { >> +ret = intel_gt_assign_ggtt(gt); >> +if (ret) >> +return ret; >> +} >> >> ret = ggtt_probe_hw(to_gt(i915)->ggtt, to_gt(i915)); >> if (ret) >> @@ -1208,6 +1222,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_gg
Re: [Intel-gfx] [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
On Tue, Nov 22, 2022 at 12:31:26PM +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. 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 > > v3: > 1. Move out for_each_gt from i915_driver.c (Jani Nikula) > > v4: drop using RCU primitives on ggtt->gt_list as it is not an RCU list > (Matt Roper) > > Cc: Matt Roper > Signed-off-by: Aravind Iddamsetty Reviewed-by: Matt Roper > --- > drivers/gpu/drm/i915/gt/intel_ggtt.c | 54 +-- > 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| 12 ++--- > drivers/gpu/drm/i915/i915_gem.c | 2 + > 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 + > 9 files changed, 111 insertions(+), 35 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c > b/drivers/gpu/drm/i915/gt/intel_ggtt.c > index 8145851ad23d..7644738b9cdb 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); > } > > @@ -1196,7 +1203,14 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, > struct intel_gt *gt) > */ > int i915_ggtt_probe_hw(struct drm_i915_private *i915) > { > - int ret; > + struct intel_gt *gt; > + int ret, i; > + > + for_each_gt(gt, i915, i) { > + ret = intel_gt_assign_ggtt(gt); > + if (ret) > + return ret; > + } > > ret = ggtt_probe_hw(to_gt(i915)->ggtt, to_gt(i915)); > if (ret) > @@ -1208,6 +1222,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 +1323,11 @@ bool i915_ggtt_resume_vm(struct i915_address_space *vm) > > void i915_ggtt_resume(struct i915_ggtt *ggtt) > { >
[Intel-gfx] [PATCH v4] 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 v3: 1. Move out for_each_gt from i915_driver.c (Jani Nikula) v4: drop using RCU primitives on ggtt->gt_list as it is not an RCU list (Matt Roper) Cc: Matt Roper Signed-off-by: Aravind Iddamsetty --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 54 +-- 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| 12 ++--- drivers/gpu/drm/i915/i915_gem.c | 2 + 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 + 9 files changed, 111 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 8145851ad23d..7644738b9cdb 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); } @@ -1196,7 +1203,14 @@ static int ggtt_probe_hw(struct i915_ggtt *ggtt, struct intel_gt *gt) */ int i915_ggtt_probe_hw(struct drm_i915_private *i915) { - int ret; + struct intel_gt *gt; + int ret, i; + + for_each_gt(gt, i915, i) { + ret = intel_gt_assign_ggtt(gt); + if (ret) + return ret; + } ret = ggtt_probe_hw(to_gt(i915)->ggtt, to_gt(i915)); if (ret) @@ -1208,6 +1222,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 +1323,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 +1336,6 @@ void