* [Intel-gfx] [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
@ 2022-11-22 7:01 ` Aravind Iddamsetty
0 siblings, 0 replies; 14+ messages in thread
From: Aravind Iddamsetty @ 2022-11-22 7:01 UTC (permalink / raw)
To: intel-gfx; +Cc: dri-devel
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 <matthew.d.roper@intel.com>
Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
---
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 <linux/types.h>
#include <linux/stop_machine.h>
+#include <drm/drm_managed.h>
#include <drm/i915_drm.h>
#include <drm/intel-gtt.h>
@@ -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 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 b5ad9caa5537..b03788d7674e 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt.c
@@ -110,9 +110,18 @@ 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 = i915_ggtt_create(gt->i915);
+ if (IS_ERR(gt->ggtt))
+ return PTR_ERR(gt->ggtt);
+ }
- return gt->ggtt ? 0 : -ENOMEM;
+ list_add_tail(>->ggtt_link, >->ggtt->gt_list);
+
+ return 0;
}
int intel_gt_init_mmio(struct intel_gt *gt)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index c1d9cd255e06..8d915640914b 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -277,6 +277,9 @@ struct intel_gt {
struct kobject *sysfs_defaults;
struct i915_perf_gt perf;
+
+ /** link: &ggtt.gt_list */
+ struct list_head ggtt_link;
};
struct intel_gt_definition {
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
index 4d75ba4bb41d..d1900fec6cd1 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.h
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
@@ -390,6 +390,9 @@ struct i915_ggtt {
struct mutex error_mutex;
struct drm_mm_node error_capture;
struct drm_mm_node uc_fw;
+
+ /** List of GTs mapping this GGTT */
+ struct list_head gt_list;
};
struct i915_ppgtt {
@@ -584,6 +587,7 @@ void i915_ggtt_disable_guc(struct i915_ggtt *ggtt);
int i915_init_ggtt(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);
+struct i915_ggtt *i915_ggtt_create(struct drm_i915_private *i915);
static inline bool i915_ggtt_has_aperture(const struct i915_ggtt *ggtt)
{
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 69103ae37779..4e1bb3c23c63 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -612,10 +612,6 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
i915_perf_init(dev_priv);
- ret = intel_gt_assign_ggtt(to_gt(dev_priv));
- if (ret)
- goto err_perf;
-
ret = i915_ggtt_probe_hw(dev_priv);
if (ret)
goto err_perf;
@@ -1316,7 +1312,8 @@ int i915_driver_suspend_switcheroo(struct drm_i915_private *i915,
static int i915_drm_resume(struct drm_device *dev)
{
struct drm_i915_private *dev_priv = to_i915(dev);
- int ret;
+ struct intel_gt *gt;
+ int ret, i;
disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
@@ -1331,6 +1328,11 @@ static int i915_drm_resume(struct drm_device *dev)
drm_err(&dev_priv->drm, "failed to re-enable GGTT\n");
i915_ggtt_resume(to_gt(dev_priv)->ggtt);
+
+ for_each_gt(gt, dev_priv, i)
+ if (GRAPHICS_VER(gt->i915) >= 8)
+ setup_private_pat(gt);
+
/* Must be called after GGTT is resumed. */
intel_dpt_resume(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 8468ca9885fd..086c4702e1bf 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1143,6 +1143,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
for_each_gt(gt, dev_priv, i) {
intel_uc_fetch_firmwares(>->uc);
intel_wopcm_init(>->wopcm);
+ if (GRAPHICS_VER(dev_priv) >= 8)
+ setup_private_pat(gt);
}
ret = i915_init_ggtt(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
index f025ee4fa526..4cfe36b0366b 100644
--- a/drivers/gpu/drm/i915/i915_gem_evict.c
+++ b/drivers/gpu/drm/i915/i915_gem_evict.c
@@ -43,16 +43,25 @@ static bool dying_vma(struct i915_vma *vma)
return !kref_read(&vma->obj->base.refcount);
}
-static int ggtt_flush(struct intel_gt *gt)
+static int ggtt_flush(struct i915_address_space *vm)
{
- /*
- * Not everything in the GGTT is tracked via vma (otherwise we
- * could evict as required with minimal stalling) so we are forced
- * to idle the GPU and explicitly retire outstanding requests in
- * the hopes that we can then remove contexts and the like only
- * bound by their active reference.
- */
- return intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
+ struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
+ struct intel_gt *gt;
+ int ret = 0;
+
+ list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) {
+ /*
+ * Not everything in the GGTT is tracked via vma (otherwise we
+ * could evict as required with minimal stalling) so we are forced
+ * to idle the GPU and explicitly retire outstanding requests in
+ * the hopes that we can then remove contexts and the like only
+ * bound by their active reference.
+ */
+ ret = intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
+ if (ret)
+ return ret;
+ }
+ return ret;
}
static bool grab_vma(struct i915_vma *vma, struct i915_gem_ww_ctx *ww)
@@ -149,6 +158,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
struct drm_mm_node *node;
enum drm_mm_insert_mode mode;
struct i915_vma *active;
+ struct intel_gt *gt;
int ret;
lockdep_assert_held(&vm->mutex);
@@ -174,7 +184,14 @@ i915_gem_evict_something(struct i915_address_space *vm,
min_size, alignment, color,
start, end, mode);
- intel_gt_retire_requests(vm->gt);
+ if (i915_is_ggtt(vm)) {
+ struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
+
+ list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
+ intel_gt_retire_requests(gt);
+ } else {
+ intel_gt_retire_requests(vm->gt);
+ }
search_again:
active = NULL;
@@ -246,7 +263,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
if (I915_SELFTEST_ONLY(igt_evict_ctl.fail_if_busy))
return -EBUSY;
- ret = ggtt_flush(vm->gt);
+ ret = ggtt_flush(vm);
if (ret)
return ret;
@@ -332,7 +349,15 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
* a stray pin (preventing eviction) that can only be resolved by
* retiring.
*/
- intel_gt_retire_requests(vm->gt);
+ if (i915_is_ggtt(vm)) {
+ struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
+ struct intel_gt *gt;
+
+ list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
+ intel_gt_retire_requests(gt);
+ } else {
+ intel_gt_retire_requests(vm->gt);
+ }
if (i915_vm_has_cache_coloring(vm)) {
/* Expand search to cover neighbouring guard pages (or lack!) */
@@ -438,7 +463,7 @@ int i915_gem_evict_vm(struct i915_address_space *vm, struct i915_gem_ww_ctx *ww)
* switch otherwise is ineffective.
*/
if (i915_is_ggtt(vm)) {
- ret = ggtt_flush(vm->gt);
+ ret = ggtt_flush(vm);
if (ret)
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
index 703fee6b5f75..726705b10637 100644
--- a/drivers/gpu/drm/i915/i915_vma.c
+++ b/drivers/gpu/drm/i915/i915_vma.c
@@ -1544,6 +1544,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
u32 align, unsigned int flags)
{
struct i915_address_space *vm = vma->vm;
+ struct intel_gt *gt;
+ struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
int err;
do {
@@ -1559,7 +1561,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
}
/* Unlike i915_vma_pin, we don't take no for an answer! */
- flush_idle_contexts(vm->gt);
+ list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
+ flush_idle_contexts(gt);
if (mutex_lock_interruptible(&vm->mutex) == 0) {
/*
* We pass NULL ww here, as we don't want to unbind
diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
index e5dd82e7e480..2535b9684bd1 100644
--- a/drivers/gpu/drm/i915/selftests/i915_gem.c
+++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
@@ -127,6 +127,8 @@ static void igt_pm_resume(struct drm_i915_private *i915)
*/
with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
i915_ggtt_resume(to_gt(i915)->ggtt);
+ if (GRAPHICS_VER(i915) >= 8)
+ setup_private_pat(to_gt(i915));
i915_gem_resume(i915);
}
}
--
2.25.1
^ permalink raw reply related [flat|nested] 14+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/mtl: Media GT and Render GT share common GGTT (rev5)
2022-11-22 7:01 ` [Intel-gfx] " Aravind Iddamsetty
(?)
@ 2022-11-22 7:39 ` Patchwork
-1 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2022-11-22 7:39 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/mtl: Media GT and Render GT share common GGTT (rev5)
URL : https://patchwork.freedesktop.org/series/110321/
State : warning
== Summary ==
Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/mtl: Media GT and Render GT share common GGTT (rev5)
2022-11-22 7:01 ` [Intel-gfx] " Aravind Iddamsetty
(?)
(?)
@ 2022-11-22 8:04 ` Patchwork
-1 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2022-11-22 8:04 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 5361 bytes --]
== Series Details ==
Series: drm/i915/mtl: Media GT and Render GT share common GGTT (rev5)
URL : https://patchwork.freedesktop.org/series/110321/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12413 -> Patchwork_110321v5
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/index.html
Participating hosts (36 -> 23)
------------------------------
Additional (2): fi-kbl-soraka fi-tgl-dsi
Missing (15): bat-dg1-7 bat-kbl-2 bat-dg1-6 bat-dg1-5 bat-dg2-8 bat-adlm-1 bat-dg2-9 bat-adlp-6 bat-adlp-4 bat-adln-1 bat-jsl-3 bat-rplp-1 bat-rpls-2 bat-dg2-11 bat-jsl-1
Known issues
------------
Here are the changes found in Patchwork_110321v5 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_gttfill@basic:
- fi-kbl-soraka: NOTRUN -> [SKIP][1] ([fdo#109271]) +7 similar issues
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/fi-kbl-soraka/igt@gem_exec_gttfill@basic.html
* igt@gem_huc_copy@huc-copy:
- fi-kbl-soraka: NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#2190])
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html
* igt@gem_lmem_swapping@basic:
- fi-kbl-soraka: NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#4613]) +3 similar issues
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html
* igt@i915_selftest@live@gem_contexts:
- fi-kbl-soraka: NOTRUN -> [INCOMPLETE][4] ([i915#7099])
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/fi-kbl-soraka/igt@i915_selftest@live@gem_contexts.html
* igt@i915_selftest@live@gt_pm:
- fi-kbl-soraka: NOTRUN -> [DMESG-FAIL][5] ([i915#1886])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html
* igt@kms_chamelium@common-hpd-after-suspend:
- fi-hsw-4770: NOTRUN -> [SKIP][6] ([fdo#109271] / [fdo#111827])
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/fi-hsw-4770/igt@kms_chamelium@common-hpd-after-suspend.html
* igt@kms_chamelium@hdmi-hpd-fast:
- fi-kbl-soraka: NOTRUN -> [SKIP][7] ([fdo#109271] / [fdo#111827]) +7 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/fi-kbl-soraka/igt@kms_chamelium@hdmi-hpd-fast.html
#### Possible fixes ####
* igt@i915_selftest@live@gt_heartbeat:
- fi-apl-guc: [DMESG-FAIL][8] ([i915#5334]) -> [PASS][9]
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
* igt@i915_selftest@live@hangcheck:
- fi-hsw-4770: [INCOMPLETE][10] ([i915#4785]) -> [PASS][11]
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/fi-hsw-4770/igt@i915_selftest@live@hangcheck.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109284]: https://bugs.freedesktop.org/show_bug.cgi?id=109284
[fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
[fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
[fdo#110189]: https://bugs.freedesktop.org/show_bug.cgi?id=110189
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#4103]: https://gitlab.freedesktop.org/drm/intel/issues/4103
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4785]: https://gitlab.freedesktop.org/drm/intel/issues/4785
[i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
[i915#6856]: https://gitlab.freedesktop.org/drm/intel/issues/6856
[i915#7099]: https://gitlab.freedesktop.org/drm/intel/issues/7099
[i915#7125]: https://gitlab.freedesktop.org/drm/intel/issues/7125
[i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
[i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
Build changes
-------------
* Linux: CI_DRM_12413 -> Patchwork_110321v5
CI-20190529: 20190529
CI_DRM_12413: 6d95d2370020769c71fa36b4bc1884ef62e14a14 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7069: 40a2de5cc6a6b43af7da7905bfe1ede9d9a3200c @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_110321v5: 6d95d2370020769c71fa36b4bc1884ef62e14a14 @ git://anongit.freedesktop.org/gfx-ci/linux
### Linux commits
000d9b98f9f8 drm/i915/mtl: Media GT and Render GT share common GGTT
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/index.html
[-- Attachment #2: Type: text/html, Size: 5713 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/mtl: Media GT and Render GT share common GGTT (rev5)
2022-11-22 7:01 ` [Intel-gfx] " Aravind Iddamsetty
` (2 preceding siblings ...)
(?)
@ 2022-11-22 9:58 ` Patchwork
-1 siblings, 0 replies; 14+ messages in thread
From: Patchwork @ 2022-11-22 9:58 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx
[-- Attachment #1: Type: text/plain, Size: 29687 bytes --]
== Series Details ==
Series: drm/i915/mtl: Media GT and Render GT share common GGTT (rev5)
URL : https://patchwork.freedesktop.org/series/110321/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_12413_full -> Patchwork_110321v5_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Participating hosts (9 -> 9)
------------------------------
No changes in participating hosts
Known issues
------------
Here are the changes found in Patchwork_110321v5_full that come from known issues:
### CI changes ###
#### Possible fixes ####
* boot:
- shard-skl: ([PASS][1], [PASS][2], [PASS][3], [PASS][4], [PASS][5], [PASS][6], [PASS][7], [PASS][8], [PASS][9], [PASS][10], [FAIL][11], [PASS][12], [PASS][13], [PASS][14], [PASS][15], [PASS][16], [PASS][17], [PASS][18], [PASS][19], [PASS][20], [PASS][21], [PASS][22], [PASS][23], [PASS][24]) ([i915#5032]) -> ([PASS][25], [PASS][26], [PASS][27], [PASS][28], [PASS][29], [PASS][30], [PASS][31], [PASS][32], [PASS][33], [PASS][34], [PASS][35], [PASS][36], [PASS][37], [PASS][38], [PASS][39], [PASS][40], [PASS][41], [PASS][42], [PASS][43], [PASS][44], [PASS][45], [PASS][46], [PASS][47])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl9/boot.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl9/boot.html
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl9/boot.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl7/boot.html
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl7/boot.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl7/boot.html
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl6/boot.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl6/boot.html
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl6/boot.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl5/boot.html
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl5/boot.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl5/boot.html
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl5/boot.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl4/boot.html
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl4/boot.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl4/boot.html
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl4/boot.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl1/boot.html
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl1/boot.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl1/boot.html
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl1/boot.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl10/boot.html
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl10/boot.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl10/boot.html
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl9/boot.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl9/boot.html
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl9/boot.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl7/boot.html
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl7/boot.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl7/boot.html
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl6/boot.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl6/boot.html
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl6/boot.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl6/boot.html
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl5/boot.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl5/boot.html
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl5/boot.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl5/boot.html
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl4/boot.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl4/boot.html
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl4/boot.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl1/boot.html
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl1/boot.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl10/boot.html
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl10/boot.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl10/boot.html
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl10/boot.html
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_isolation@preservation-s3@vecs0:
- shard-apl: [PASS][48] -> [DMESG-WARN][49] ([i915#180])
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-apl3/igt@gem_ctx_isolation@preservation-s3@vecs0.html
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-apl8/igt@gem_ctx_isolation@preservation-s3@vecs0.html
* igt@gem_exec_balancer@parallel-out-fence:
- shard-iclb: [PASS][50] -> [SKIP][51] ([i915#4525])
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-iclb2/igt@gem_exec_balancer@parallel-out-fence.html
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb3/igt@gem_exec_balancer@parallel-out-fence.html
* igt@gem_exec_fair@basic-flow@rcs0:
- shard-tglb: [PASS][52] -> [FAIL][53] ([i915#2842])
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-tglb5/igt@gem_exec_fair@basic-flow@rcs0.html
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-tglb2/igt@gem_exec_fair@basic-flow@rcs0.html
* igt@gem_exec_fair@basic-none-solo@rcs0:
- shard-apl: [PASS][54] -> [FAIL][55] ([i915#2842])
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-apl7/igt@gem_exec_fair@basic-none-solo@rcs0.html
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-apl2/igt@gem_exec_fair@basic-none-solo@rcs0.html
* igt@gem_lmem_swapping@heavy-verify-random:
- shard-skl: NOTRUN -> [SKIP][56] ([fdo#109271] / [i915#4613]) +1 similar issue
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl1/igt@gem_lmem_swapping@heavy-verify-random.html
* igt@gem_pread@exhaustion:
- shard-apl: NOTRUN -> [INCOMPLETE][57] ([i915#7248])
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-apl2/igt@gem_pread@exhaustion.html
* igt@gem_pxp@verify-pxp-stale-buf-execution:
- shard-skl: NOTRUN -> [SKIP][58] ([fdo#109271]) +102 similar issues
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl1/igt@gem_pxp@verify-pxp-stale-buf-execution.html
* igt@i915_module_load@reload:
- shard-skl: [PASS][59] -> [DMESG-WARN][60] ([i915#1982])
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl6/igt@i915_module_load@reload.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl4/igt@i915_module_load@reload.html
* igt@i915_pm_dc@dc6-dpms:
- shard-skl: NOTRUN -> [FAIL][61] ([i915#3989] / [i915#454])
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl1/igt@i915_pm_dc@dc6-dpms.html
- shard-iclb: [PASS][62] -> [FAIL][63] ([i915#3989] / [i915#454]) +1 similar issue
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-iclb1/igt@i915_pm_dc@dc6-dpms.html
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb3/igt@i915_pm_dc@dc6-dpms.html
* igt@i915_pm_dc@dc9-dpms:
- shard-iclb: [PASS][64] -> [SKIP][65] ([i915#4281])
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-iclb1/igt@i915_pm_dc@dc9-dpms.html
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb3/igt@i915_pm_dc@dc9-dpms.html
* igt@i915_selftest@live@hangcheck:
- shard-tglb: [PASS][66] -> [DMESG-WARN][67] ([i915#5591])
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-tglb7/igt@i915_selftest@live@hangcheck.html
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-tglb3/igt@i915_selftest@live@hangcheck.html
* igt@kms_async_flips@alternate-sync-async-flip@pipe-a-edp-1:
- shard-skl: [PASS][68] -> [FAIL][69] ([i915#2521])
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl9/igt@kms_async_flips@alternate-sync-async-flip@pipe-a-edp-1.html
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl9/igt@kms_async_flips@alternate-sync-async-flip@pipe-a-edp-1.html
* igt@kms_atomic_transition@plane-all-modeset-transition@pipe-b-hdmi-a-1:
- shard-glk: [PASS][70] -> [INCOMPLETE][71] ([i915#5584])
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-glk8/igt@kms_atomic_transition@plane-all-modeset-transition@pipe-b-hdmi-a-1.html
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-glk7/igt@kms_atomic_transition@plane-all-modeset-transition@pipe-b-hdmi-a-1.html
* igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_mc_ccs:
- shard-skl: NOTRUN -> [SKIP][72] ([fdo#109271] / [i915#3886]) +6 similar issues
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl6/igt@kms_ccs@pipe-a-missing-ccs-buffer-y_tiled_gen12_mc_ccs.html
* igt@kms_ccs@pipe-c-bad-rotation-90-y_tiled_gen12_rc_ccs_cc:
- shard-apl: NOTRUN -> [SKIP][73] ([fdo#109271] / [i915#3886]) +2 similar issues
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-apl2/igt@kms_ccs@pipe-c-bad-rotation-90-y_tiled_gen12_rc_ccs_cc.html
* igt@kms_chamelium@dp-mode-timings:
- shard-apl: NOTRUN -> [SKIP][74] ([fdo#109271] / [fdo#111827]) +1 similar issue
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-apl2/igt@kms_chamelium@dp-mode-timings.html
* igt@kms_color_chamelium@ctm-limited-range:
- shard-skl: NOTRUN -> [SKIP][75] ([fdo#109271] / [fdo#111827]) +4 similar issues
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl1/igt@kms_color_chamelium@ctm-limited-range.html
* igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions:
- shard-skl: NOTRUN -> [INCOMPLETE][76] ([i915#7152])
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl7/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions.html
* igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions-varying-size:
- shard-glk: [PASS][77] -> [FAIL][78] ([i915#2346])
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-glk9/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions-varying-size.html
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-glk9/igt@kms_cursor_legacy@flip-vs-cursor@atomic-transitions-varying-size.html
* igt@kms_cursor_legacy@flip-vs-cursor@varying-size:
- shard-iclb: [PASS][79] -> [FAIL][80] ([i915#2346]) +2 similar issues
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-iclb2/igt@kms_cursor_legacy@flip-vs-cursor@varying-size.html
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb7/igt@kms_cursor_legacy@flip-vs-cursor@varying-size.html
* igt@kms_flip@flip-vs-expired-vblank@b-edp1:
- shard-skl: [PASS][81] -> [FAIL][82] ([i915#79])
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl6/igt@kms_flip@flip-vs-expired-vblank@b-edp1.html
[82]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl7/igt@kms_flip@flip-vs-expired-vblank@b-edp1.html
* igt@kms_flip@flip-vs-suspend@a-edp1:
- shard-skl: [PASS][83] -> [INCOMPLETE][84] ([i915#4839])
[83]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl4/igt@kms_flip@flip-vs-suspend@a-edp1.html
[84]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl10/igt@kms_flip@flip-vs-suspend@a-edp1.html
* igt@kms_flip@plain-flip-fb-recreate@c-edp1:
- shard-skl: [PASS][85] -> [FAIL][86] ([i915#2122]) +1 similar issue
[85]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl6/igt@kms_flip@plain-flip-fb-recreate@c-edp1.html
[86]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl4/igt@kms_flip@plain-flip-fb-recreate@c-edp1.html
* igt@kms_flip_scaled_crc@flip-64bpp-4tile-to-16bpp-4tile-downscaling@pipe-a-valid-mode:
- shard-iclb: NOTRUN -> [SKIP][87] ([i915#2587] / [i915#2672]) +2 similar issues
[87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb7/igt@kms_flip_scaled_crc@flip-64bpp-4tile-to-16bpp-4tile-downscaling@pipe-a-valid-mode.html
* igt@kms_flip_scaled_crc@flip-64bpp-xtile-to-16bpp-xtile-downscaling@pipe-a-default-mode:
- shard-iclb: NOTRUN -> [SKIP][88] ([i915#3555]) +3 similar issues
[88]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb2/igt@kms_flip_scaled_crc@flip-64bpp-xtile-to-16bpp-xtile-downscaling@pipe-a-default-mode.html
* igt@kms_flip_scaled_crc@flip-64bpp-yftile-to-32bpp-yftile-upscaling@pipe-a-default-mode:
- shard-iclb: NOTRUN -> [SKIP][89] ([i915#2672]) +4 similar issues
[89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb3/igt@kms_flip_scaled_crc@flip-64bpp-yftile-to-32bpp-yftile-upscaling@pipe-a-default-mode.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-wc:
- shard-apl: NOTRUN -> [SKIP][90] ([fdo#109271]) +27 similar issues
[90]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-apl2/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-spr-indfb-draw-mmap-wc.html
* igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-draw-blt:
- shard-iclb: NOTRUN -> [SKIP][91] ([fdo#109280]) +1 similar issue
[91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb7/igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-draw-blt.html
* igt@kms_plane_alpha_blend@alpha-basic@pipe-a-edp-1:
- shard-skl: NOTRUN -> [FAIL][92] ([i915#4573]) +1 similar issue
[92]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl6/igt@kms_plane_alpha_blend@alpha-basic@pipe-a-edp-1.html
* igt@kms_plane_alpha_blend@alpha-basic@pipe-c-edp-1:
- shard-skl: NOTRUN -> [DMESG-FAIL][93] ([IGT#6])
[93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl6/igt@kms_plane_alpha_blend@alpha-basic@pipe-c-edp-1.html
* igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-5@pipe-c-edp-1:
- shard-iclb: [PASS][94] -> [SKIP][95] ([i915#5235]) +2 similar issues
[94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-iclb1/igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-5@pipe-c-edp-1.html
[95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb2/igt@kms_plane_scaling@planes-unity-scaling-downscale-factor-0-5@pipe-c-edp-1.html
* igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area:
- shard-apl: NOTRUN -> [SKIP][96] ([fdo#109271] / [i915#658])
[96]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-apl2/igt@kms_psr2_sf@overlay-primary-update-sf-dmg-area.html
* igt@kms_psr2_sf@plane-move-sf-dmg-area:
- shard-skl: NOTRUN -> [SKIP][97] ([fdo#109271] / [i915#658])
[97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl1/igt@kms_psr2_sf@plane-move-sf-dmg-area.html
* igt@kms_psr2_su@page_flip-nv12@pipe-b-edp-1:
- shard-iclb: NOTRUN -> [FAIL][98] ([i915#5939]) +2 similar issues
[98]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb2/igt@kms_psr2_su@page_flip-nv12@pipe-b-edp-1.html
* igt@kms_psr@psr2_sprite_plane_onoff:
- shard-iclb: [PASS][99] -> [SKIP][100] ([fdo#109441]) +2 similar issues
[99]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-iclb2/igt@kms_psr@psr2_sprite_plane_onoff.html
[100]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb6/igt@kms_psr@psr2_sprite_plane_onoff.html
* igt@kms_psr_stress_test@flip-primary-invalidate-overlay:
- shard-tglb: [PASS][101] -> [SKIP][102] ([i915#5519])
[101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-tglb8/igt@kms_psr_stress_test@flip-primary-invalidate-overlay.html
[102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-tglb6/igt@kms_psr_stress_test@flip-primary-invalidate-overlay.html
- shard-iclb: [PASS][103] -> [SKIP][104] ([i915#5519])
[103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-iclb7/igt@kms_psr_stress_test@flip-primary-invalidate-overlay.html
[104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb6/igt@kms_psr_stress_test@flip-primary-invalidate-overlay.html
* igt@kms_vblank@pipe-d-ts-continuation-dpms-rpm:
- shard-iclb: NOTRUN -> [SKIP][105] ([fdo#109278])
[105]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb7/igt@kms_vblank@pipe-d-ts-continuation-dpms-rpm.html
* igt@perf_pmu@interrupts:
- shard-skl: [PASS][106] -> [FAIL][107] ([i915#7318])
[106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl1/igt@perf_pmu@interrupts.html
[107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl1/igt@perf_pmu@interrupts.html
* igt@sysfs_clients@split-50:
- shard-skl: NOTRUN -> [SKIP][108] ([fdo#109271] / [i915#2994]) +1 similar issue
[108]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl1/igt@sysfs_clients@split-50.html
#### Possible fixes ####
* igt@gem_exec_balancer@parallel:
- shard-iclb: [SKIP][109] ([i915#4525]) -> [PASS][110]
[109]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-iclb7/igt@gem_exec_balancer@parallel.html
[110]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb1/igt@gem_exec_balancer@parallel.html
* igt@gem_exec_capture@pi@rcs0:
- shard-skl: [INCOMPLETE][111] ([i915#3371] / [i915#7557]) -> [PASS][112]
[111]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl1/igt@gem_exec_capture@pi@rcs0.html
[112]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl1/igt@gem_exec_capture@pi@rcs0.html
* igt@gem_exec_endless@dispatch@vcs0:
- shard-iclb: [TIMEOUT][113] ([i915#3778]) -> [PASS][114]
[113]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-iclb2/igt@gem_exec_endless@dispatch@vcs0.html
[114]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb7/igt@gem_exec_endless@dispatch@vcs0.html
* igt@gem_exec_fair@basic-none-share@rcs0:
- shard-glk: [FAIL][115] ([i915#2842]) -> [PASS][116]
[115]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-glk8/igt@gem_exec_fair@basic-none-share@rcs0.html
[116]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-glk5/igt@gem_exec_fair@basic-none-share@rcs0.html
* igt@kms_cursor_legacy@flip-vs-cursor@atomic:
- shard-skl: [INCOMPLETE][117] -> [PASS][118]
[117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl7/igt@kms_cursor_legacy@flip-vs-cursor@atomic.html
[118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl7/igt@kms_cursor_legacy@flip-vs-cursor@atomic.html
* igt@kms_cursor_legacy@flip-vs-cursor@toggle:
- shard-skl: [FAIL][119] ([i915#2346]) -> [PASS][120]
[119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl7/igt@kms_cursor_legacy@flip-vs-cursor@toggle.html
[120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl7/igt@kms_cursor_legacy@flip-vs-cursor@toggle.html
* igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1:
- shard-skl: [FAIL][121] ([i915#79]) -> [PASS][122]
[121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl9/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1.html
[122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl10/igt@kms_flip@flip-vs-expired-vblank-interruptible@b-edp1.html
* igt@kms_flip@plain-flip-ts-check@b-edp1:
- shard-skl: [FAIL][123] ([i915#2122]) -> [PASS][124]
[123]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl7/igt@kms_flip@plain-flip-ts-check@b-edp1.html
[124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl7/igt@kms_flip@plain-flip-ts-check@b-edp1.html
* igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-5@pipe-b-edp-1:
- shard-iclb: [SKIP][125] ([i915#5176]) -> [PASS][126] +2 similar issues
[125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-iclb2/igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-5@pipe-b-edp-1.html
[126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb7/igt@kms_plane_scaling@plane-downscale-with-pixel-format-factor-0-5@pipe-b-edp-1.html
* igt@kms_psr@psr2_primary_blt:
- shard-iclb: [SKIP][127] ([fdo#109441]) -> [PASS][128]
[127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-iclb6/igt@kms_psr@psr2_primary_blt.html
[128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb2/igt@kms_psr@psr2_primary_blt.html
* igt@kms_vblank@pipe-c-ts-continuation-suspend:
- shard-apl: [DMESG-WARN][129] ([i915#180]) -> [PASS][130]
[129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-apl7/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
[130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-apl2/igt@kms_vblank@pipe-c-ts-continuation-suspend.html
* igt@perf@polling:
- shard-skl: [FAIL][131] ([i915#1542]) -> [PASS][132]
[131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-skl7/igt@perf@polling.html
[132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-skl7/igt@perf@polling.html
#### Warnings ####
* igt@gem_pwrite@basic-exhaustion:
- shard-tglb: [INCOMPLETE][133] ([i915#7248]) -> [WARN][134] ([i915#2658])
[133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-tglb1/igt@gem_pwrite@basic-exhaustion.html
[134]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-tglb3/igt@gem_pwrite@basic-exhaustion.html
* igt@i915_pm_dc@dc3co-vpb-simulation:
- shard-iclb: [SKIP][135] ([i915#658]) -> [SKIP][136] ([i915#588])
[135]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-iclb1/igt@i915_pm_dc@dc3co-vpb-simulation.html
[136]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html
* igt@kms_psr2_sf@cursor-plane-move-continuous-sf:
- shard-iclb: [SKIP][137] ([i915#658]) -> [SKIP][138] ([i915#2920])
[137]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-iclb6/igt@kms_psr2_sf@cursor-plane-move-continuous-sf.html
[138]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb2/igt@kms_psr2_sf@cursor-plane-move-continuous-sf.html
* igt@kms_psr2_sf@cursor-plane-update-sf:
- shard-iclb: [SKIP][139] ([fdo#111068] / [i915#658]) -> [SKIP][140] ([i915#2920])
[139]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-iclb6/igt@kms_psr2_sf@cursor-plane-update-sf.html
[140]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb2/igt@kms_psr2_sf@cursor-plane-update-sf.html
* igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb:
- shard-iclb: [SKIP][141] ([i915#2920]) -> [SKIP][142] ([i915#658]) +1 similar issue
[141]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-iclb2/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb.html
[142]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-iclb6/igt@kms_psr2_sf@primary-plane-update-sf-dmg-area-big-fb.html
* igt@runner@aborted:
- shard-apl: ([FAIL][143], [FAIL][144], [FAIL][145]) ([i915#3002] / [i915#4312]) -> ([FAIL][146], [FAIL][147], [FAIL][148]) ([i915#180] / [i915#3002] / [i915#4312])
[143]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-apl7/igt@runner@aborted.html
[144]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-apl8/igt@runner@aborted.html
[145]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12413/shard-apl7/igt@runner@aborted.html
[146]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-apl3/igt@runner@aborted.html
[147]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-apl8/igt@runner@aborted.html
[148]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/shard-apl1/igt@runner@aborted.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[IGT#6]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/6
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
[fdo#109280]: https://bugs.freedesktop.org/show_bug.cgi?id=109280
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[fdo#111068]: https://bugs.freedesktop.org/show_bug.cgi?id=111068
[fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
[i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
[i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
[i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521
[i915#2587]: https://gitlab.freedesktop.org/drm/intel/issues/2587
[i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
[i915#2672]: https://gitlab.freedesktop.org/drm/intel/issues/2672
[i915#2842]: https://gitlab.freedesktop.org/drm/intel/issues/2842
[i915#2920]: https://gitlab.freedesktop.org/drm/intel/issues/2920
[i915#2994]: https://gitlab.freedesktop.org/drm/intel/issues/2994
[i915#3002]: https://gitlab.freedesktop.org/drm/intel/issues/3002
[i915#3371]: https://gitlab.freedesktop.org/drm/intel/issues/3371
[i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
[i915#3778]: https://gitlab.freedesktop.org/drm/intel/issues/3778
[i915#3886]: https://gitlab.freedesktop.org/drm/intel/issues/3886
[i915#3989]: https://gitlab.freedesktop.org/drm/intel/issues/3989
[i915#4281]: https://gitlab.freedesktop.org/drm/intel/issues/4281
[i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
[i915#4525]: https://gitlab.freedesktop.org/drm/intel/issues/4525
[i915#454]: https://gitlab.freedesktop.org/drm/intel/issues/454
[i915#4573]: https://gitlab.freedesktop.org/drm/intel/issues/4573
[i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
[i915#4839]: https://gitlab.freedesktop.org/drm/intel/issues/4839
[i915#5032]: https://gitlab.freedesktop.org/drm/intel/issues/5032
[i915#5176]: https://gitlab.freedesktop.org/drm/intel/issues/5176
[i915#5235]: https://gitlab.freedesktop.org/drm/intel/issues/5235
[i915#5519]: https://gitlab.freedesktop.org/drm/intel/issues/5519
[i915#5584]: https://gitlab.freedesktop.org/drm/intel/issues/5584
[i915#5591]: https://gitlab.freedesktop.org/drm/intel/issues/5591
[i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
[i915#5939]: https://gitlab.freedesktop.org/drm/intel/issues/5939
[i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
[i915#7152]: https://gitlab.freedesktop.org/drm/intel/issues/7152
[i915#7248]: https://gitlab.freedesktop.org/drm/intel/issues/7248
[i915#7318]: https://gitlab.freedesktop.org/drm/intel/issues/7318
[i915#7557]: https://gitlab.freedesktop.org/drm/intel/issues/7557
[i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
Build changes
-------------
* Linux: CI_DRM_12413 -> Patchwork_110321v5
CI-20190529: 20190529
CI_DRM_12413: 6d95d2370020769c71fa36b4bc1884ef62e14a14 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_7069: 40a2de5cc6a6b43af7da7905bfe1ede9d9a3200c @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
Patchwork_110321v5: 6d95d2370020769c71fa36b4bc1884ef62e14a14 @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_110321v5/index.html
[-- Attachment #2: Type: text/html, Size: 34365 bytes --]
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
2022-11-22 7:01 ` [Intel-gfx] " Aravind Iddamsetty
@ 2022-11-22 23:59 ` Matt Roper
-1 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2022-11-22 23:59 UTC (permalink / raw)
To: Aravind Iddamsetty; +Cc: intel-gfx, dri-devel
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 <matthew.d.roper@intel.com>
> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> 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 <linux/types.h>
> #include <linux/stop_machine.h>
>
> +#include <drm/drm_managed.h>
> #include <drm/i915_drm.h>
> #include <drm/intel-gtt.h>
>
> @@ -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 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 b5ad9caa5537..b03788d7674e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -110,9 +110,18 @@ 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 = i915_ggtt_create(gt->i915);
> + if (IS_ERR(gt->ggtt))
> + return PTR_ERR(gt->ggtt);
> + }
>
> - return gt->ggtt ? 0 : -ENOMEM;
> + list_add_tail(>->ggtt_link, >->ggtt->gt_list);
> +
> + return 0;
> }
>
> int intel_gt_init_mmio(struct intel_gt *gt)
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> index c1d9cd255e06..8d915640914b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> @@ -277,6 +277,9 @@ struct intel_gt {
> struct kobject *sysfs_defaults;
>
> struct i915_perf_gt perf;
> +
> + /** link: &ggtt.gt_list */
> + struct list_head ggtt_link;
> };
>
> struct intel_gt_definition {
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
> index 4d75ba4bb41d..d1900fec6cd1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
> @@ -390,6 +390,9 @@ struct i915_ggtt {
> struct mutex error_mutex;
> struct drm_mm_node error_capture;
> struct drm_mm_node uc_fw;
> +
> + /** List of GTs mapping this GGTT */
> + struct list_head gt_list;
> };
>
> struct i915_ppgtt {
> @@ -584,6 +587,7 @@ void i915_ggtt_disable_guc(struct i915_ggtt *ggtt);
> int i915_init_ggtt(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);
> +struct i915_ggtt *i915_ggtt_create(struct drm_i915_private *i915);
>
> static inline bool i915_ggtt_has_aperture(const struct i915_ggtt *ggtt)
> {
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 69103ae37779..4e1bb3c23c63 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -612,10 +612,6 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
>
> i915_perf_init(dev_priv);
>
> - ret = intel_gt_assign_ggtt(to_gt(dev_priv));
> - if (ret)
> - goto err_perf;
> -
> ret = i915_ggtt_probe_hw(dev_priv);
> if (ret)
> goto err_perf;
> @@ -1316,7 +1312,8 @@ int i915_driver_suspend_switcheroo(struct drm_i915_private *i915,
> static int i915_drm_resume(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> - int ret;
> + struct intel_gt *gt;
> + int ret, i;
>
> disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>
> @@ -1331,6 +1328,11 @@ static int i915_drm_resume(struct drm_device *dev)
> drm_err(&dev_priv->drm, "failed to re-enable GGTT\n");
>
> i915_ggtt_resume(to_gt(dev_priv)->ggtt);
> +
> + for_each_gt(gt, dev_priv, i)
> + if (GRAPHICS_VER(gt->i915) >= 8)
> + setup_private_pat(gt);
> +
> /* Must be called after GGTT is resumed. */
> intel_dpt_resume(dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8468ca9885fd..086c4702e1bf 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1143,6 +1143,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
> for_each_gt(gt, dev_priv, i) {
> intel_uc_fetch_firmwares(>->uc);
> intel_wopcm_init(>->wopcm);
> + if (GRAPHICS_VER(dev_priv) >= 8)
> + setup_private_pat(gt);
> }
>
> ret = i915_init_ggtt(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> index f025ee4fa526..4cfe36b0366b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -43,16 +43,25 @@ static bool dying_vma(struct i915_vma *vma)
> return !kref_read(&vma->obj->base.refcount);
> }
>
> -static int ggtt_flush(struct intel_gt *gt)
> +static int ggtt_flush(struct i915_address_space *vm)
> {
> - /*
> - * Not everything in the GGTT is tracked via vma (otherwise we
> - * could evict as required with minimal stalling) so we are forced
> - * to idle the GPU and explicitly retire outstanding requests in
> - * the hopes that we can then remove contexts and the like only
> - * bound by their active reference.
> - */
> - return intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> + struct intel_gt *gt;
> + int ret = 0;
> +
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) {
> + /*
> + * Not everything in the GGTT is tracked via vma (otherwise we
> + * could evict as required with minimal stalling) so we are forced
> + * to idle the GPU and explicitly retire outstanding requests in
> + * the hopes that we can then remove contexts and the like only
> + * bound by their active reference.
> + */
> + ret = intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
> + if (ret)
> + return ret;
> + }
> + return ret;
> }
>
> static bool grab_vma(struct i915_vma *vma, struct i915_gem_ww_ctx *ww)
> @@ -149,6 +158,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
> struct drm_mm_node *node;
> enum drm_mm_insert_mode mode;
> struct i915_vma *active;
> + struct intel_gt *gt;
> int ret;
>
> lockdep_assert_held(&vm->mutex);
> @@ -174,7 +184,14 @@ i915_gem_evict_something(struct i915_address_space *vm,
> min_size, alignment, color,
> start, end, mode);
>
> - intel_gt_retire_requests(vm->gt);
> + if (i915_is_ggtt(vm)) {
> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> +
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
> + intel_gt_retire_requests(gt);
> + } else {
> + intel_gt_retire_requests(vm->gt);
> + }
>
> search_again:
> active = NULL;
> @@ -246,7 +263,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
> if (I915_SELFTEST_ONLY(igt_evict_ctl.fail_if_busy))
> return -EBUSY;
>
> - ret = ggtt_flush(vm->gt);
> + ret = ggtt_flush(vm);
> if (ret)
> return ret;
>
> @@ -332,7 +349,15 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
> * a stray pin (preventing eviction) that can only be resolved by
> * retiring.
> */
> - intel_gt_retire_requests(vm->gt);
> + if (i915_is_ggtt(vm)) {
> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> + struct intel_gt *gt;
> +
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
> + intel_gt_retire_requests(gt);
> + } else {
> + intel_gt_retire_requests(vm->gt);
> + }
>
> if (i915_vm_has_cache_coloring(vm)) {
> /* Expand search to cover neighbouring guard pages (or lack!) */
> @@ -438,7 +463,7 @@ int i915_gem_evict_vm(struct i915_address_space *vm, struct i915_gem_ww_ctx *ww)
> * switch otherwise is ineffective.
> */
> if (i915_is_ggtt(vm)) {
> - ret = ggtt_flush(vm->gt);
> + ret = ggtt_flush(vm);
> if (ret)
> return ret;
> }
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 703fee6b5f75..726705b10637 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -1544,6 +1544,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
> u32 align, unsigned int flags)
> {
> struct i915_address_space *vm = vma->vm;
> + struct intel_gt *gt;
> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> int err;
>
> do {
> @@ -1559,7 +1561,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
> }
>
> /* Unlike i915_vma_pin, we don't take no for an answer! */
> - flush_idle_contexts(vm->gt);
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
> + flush_idle_contexts(gt);
> if (mutex_lock_interruptible(&vm->mutex) == 0) {
> /*
> * We pass NULL ww here, as we don't want to unbind
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
> index e5dd82e7e480..2535b9684bd1 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
> @@ -127,6 +127,8 @@ static void igt_pm_resume(struct drm_i915_private *i915)
> */
> with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
> i915_ggtt_resume(to_gt(i915)->ggtt);
> + if (GRAPHICS_VER(i915) >= 8)
> + setup_private_pat(to_gt(i915));
> i915_gem_resume(i915);
> }
> }
> --
> 2.25.1
>
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
@ 2022-11-22 23:59 ` Matt Roper
0 siblings, 0 replies; 14+ messages in thread
From: Matt Roper @ 2022-11-22 23:59 UTC (permalink / raw)
To: Aravind Iddamsetty; +Cc: intel-gfx, dri-devel
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 <matthew.d.roper@intel.com>
> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
> ---
> 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 <linux/types.h>
> #include <linux/stop_machine.h>
>
> +#include <drm/drm_managed.h>
> #include <drm/i915_drm.h>
> #include <drm/intel-gtt.h>
>
> @@ -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 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 b5ad9caa5537..b03788d7674e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -110,9 +110,18 @@ 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 = i915_ggtt_create(gt->i915);
> + if (IS_ERR(gt->ggtt))
> + return PTR_ERR(gt->ggtt);
> + }
>
> - return gt->ggtt ? 0 : -ENOMEM;
> + list_add_tail(>->ggtt_link, >->ggtt->gt_list);
> +
> + return 0;
> }
>
> int intel_gt_init_mmio(struct intel_gt *gt)
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> index c1d9cd255e06..8d915640914b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> @@ -277,6 +277,9 @@ struct intel_gt {
> struct kobject *sysfs_defaults;
>
> struct i915_perf_gt perf;
> +
> + /** link: &ggtt.gt_list */
> + struct list_head ggtt_link;
> };
>
> struct intel_gt_definition {
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
> index 4d75ba4bb41d..d1900fec6cd1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
> @@ -390,6 +390,9 @@ struct i915_ggtt {
> struct mutex error_mutex;
> struct drm_mm_node error_capture;
> struct drm_mm_node uc_fw;
> +
> + /** List of GTs mapping this GGTT */
> + struct list_head gt_list;
> };
>
> struct i915_ppgtt {
> @@ -584,6 +587,7 @@ void i915_ggtt_disable_guc(struct i915_ggtt *ggtt);
> int i915_init_ggtt(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);
> +struct i915_ggtt *i915_ggtt_create(struct drm_i915_private *i915);
>
> static inline bool i915_ggtt_has_aperture(const struct i915_ggtt *ggtt)
> {
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 69103ae37779..4e1bb3c23c63 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -612,10 +612,6 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
>
> i915_perf_init(dev_priv);
>
> - ret = intel_gt_assign_ggtt(to_gt(dev_priv));
> - if (ret)
> - goto err_perf;
> -
> ret = i915_ggtt_probe_hw(dev_priv);
> if (ret)
> goto err_perf;
> @@ -1316,7 +1312,8 @@ int i915_driver_suspend_switcheroo(struct drm_i915_private *i915,
> static int i915_drm_resume(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> - int ret;
> + struct intel_gt *gt;
> + int ret, i;
>
> disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>
> @@ -1331,6 +1328,11 @@ static int i915_drm_resume(struct drm_device *dev)
> drm_err(&dev_priv->drm, "failed to re-enable GGTT\n");
>
> i915_ggtt_resume(to_gt(dev_priv)->ggtt);
> +
> + for_each_gt(gt, dev_priv, i)
> + if (GRAPHICS_VER(gt->i915) >= 8)
> + setup_private_pat(gt);
> +
> /* Must be called after GGTT is resumed. */
> intel_dpt_resume(dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8468ca9885fd..086c4702e1bf 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1143,6 +1143,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
> for_each_gt(gt, dev_priv, i) {
> intel_uc_fetch_firmwares(>->uc);
> intel_wopcm_init(>->wopcm);
> + if (GRAPHICS_VER(dev_priv) >= 8)
> + setup_private_pat(gt);
> }
>
> ret = i915_init_ggtt(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> index f025ee4fa526..4cfe36b0366b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -43,16 +43,25 @@ static bool dying_vma(struct i915_vma *vma)
> return !kref_read(&vma->obj->base.refcount);
> }
>
> -static int ggtt_flush(struct intel_gt *gt)
> +static int ggtt_flush(struct i915_address_space *vm)
> {
> - /*
> - * Not everything in the GGTT is tracked via vma (otherwise we
> - * could evict as required with minimal stalling) so we are forced
> - * to idle the GPU and explicitly retire outstanding requests in
> - * the hopes that we can then remove contexts and the like only
> - * bound by their active reference.
> - */
> - return intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> + struct intel_gt *gt;
> + int ret = 0;
> +
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) {
> + /*
> + * Not everything in the GGTT is tracked via vma (otherwise we
> + * could evict as required with minimal stalling) so we are forced
> + * to idle the GPU and explicitly retire outstanding requests in
> + * the hopes that we can then remove contexts and the like only
> + * bound by their active reference.
> + */
> + ret = intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
> + if (ret)
> + return ret;
> + }
> + return ret;
> }
>
> static bool grab_vma(struct i915_vma *vma, struct i915_gem_ww_ctx *ww)
> @@ -149,6 +158,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
> struct drm_mm_node *node;
> enum drm_mm_insert_mode mode;
> struct i915_vma *active;
> + struct intel_gt *gt;
> int ret;
>
> lockdep_assert_held(&vm->mutex);
> @@ -174,7 +184,14 @@ i915_gem_evict_something(struct i915_address_space *vm,
> min_size, alignment, color,
> start, end, mode);
>
> - intel_gt_retire_requests(vm->gt);
> + if (i915_is_ggtt(vm)) {
> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> +
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
> + intel_gt_retire_requests(gt);
> + } else {
> + intel_gt_retire_requests(vm->gt);
> + }
>
> search_again:
> active = NULL;
> @@ -246,7 +263,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
> if (I915_SELFTEST_ONLY(igt_evict_ctl.fail_if_busy))
> return -EBUSY;
>
> - ret = ggtt_flush(vm->gt);
> + ret = ggtt_flush(vm);
> if (ret)
> return ret;
>
> @@ -332,7 +349,15 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
> * a stray pin (preventing eviction) that can only be resolved by
> * retiring.
> */
> - intel_gt_retire_requests(vm->gt);
> + if (i915_is_ggtt(vm)) {
> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> + struct intel_gt *gt;
> +
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
> + intel_gt_retire_requests(gt);
> + } else {
> + intel_gt_retire_requests(vm->gt);
> + }
>
> if (i915_vm_has_cache_coloring(vm)) {
> /* Expand search to cover neighbouring guard pages (or lack!) */
> @@ -438,7 +463,7 @@ int i915_gem_evict_vm(struct i915_address_space *vm, struct i915_gem_ww_ctx *ww)
> * switch otherwise is ineffective.
> */
> if (i915_is_ggtt(vm)) {
> - ret = ggtt_flush(vm->gt);
> + ret = ggtt_flush(vm);
> if (ret)
> return ret;
> }
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 703fee6b5f75..726705b10637 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -1544,6 +1544,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
> u32 align, unsigned int flags)
> {
> struct i915_address_space *vm = vma->vm;
> + struct intel_gt *gt;
> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> int err;
>
> do {
> @@ -1559,7 +1561,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
> }
>
> /* Unlike i915_vma_pin, we don't take no for an answer! */
> - flush_idle_contexts(vm->gt);
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
> + flush_idle_contexts(gt);
> if (mutex_lock_interruptible(&vm->mutex) == 0) {
> /*
> * We pass NULL ww here, as we don't want to unbind
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
> index e5dd82e7e480..2535b9684bd1 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
> @@ -127,6 +127,8 @@ static void igt_pm_resume(struct drm_i915_private *i915)
> */
> with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
> i915_ggtt_resume(to_gt(i915)->ggtt);
> + if (GRAPHICS_VER(i915) >= 8)
> + setup_private_pat(to_gt(i915));
> i915_gem_resume(i915);
> }
> }
> --
> 2.25.1
>
--
Matt Roper
Graphics Software Engineer
VTT-OSGC Platform Enablement
Intel Corporation
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
2022-11-22 23:59 ` [Intel-gfx] " Matt Roper
@ 2022-11-23 4:17 ` Iddamsetty, Aravind
-1 siblings, 0 replies; 14+ messages in thread
From: Iddamsetty, Aravind @ 2022-11-23 4:17 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx, dri-devel
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 <matthew.d.roper@intel.com>
>> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
>
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
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 <linux/types.h>
>> #include <linux/stop_machine.h>
>>
>> +#include <drm/drm_managed.h>
>> #include <drm/i915_drm.h>
>> #include <drm/intel-gtt.h>
>>
>> @@ -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 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 b5ad9caa5537..b03788d7674e 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
>> @@ -110,9 +110,18 @@ 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 = i915_ggtt_create(gt->i915);
>> + if (IS_ERR(gt->ggtt))
>> + return PTR_ERR(gt->ggtt);
>> + }
>>
>> - return gt->ggtt ? 0 : -ENOMEM;
>> + list_add_tail(>->ggtt_link, >->ggtt->gt_list);
>> +
>> + return 0;
>> }
>>
>> int intel_gt_init_mmio(struct intel_gt *gt)
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> index c1d9cd255e06..8d915640914b 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> @@ -277,6 +277,9 @@ struct intel_gt {
>> struct kobject *sysfs_defaults;
>>
>> struct i915_perf_gt perf;
>> +
>> + /** link: &ggtt.gt_list */
>> + struct list_head ggtt_link;
>> };
>>
>> struct intel_gt_definition {
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
>> index 4d75ba4bb41d..d1900fec6cd1 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
>> @@ -390,6 +390,9 @@ struct i915_ggtt {
>> struct mutex error_mutex;
>> struct drm_mm_node error_capture;
>> struct drm_mm_node uc_fw;
>> +
>> + /** List of GTs mapping this GGTT */
>> + struct list_head gt_list;
>> };
>>
>> struct i915_ppgtt {
>> @@ -584,6 +587,7 @@ void i915_ggtt_disable_guc(struct i915_ggtt *ggtt);
>> int i915_init_ggtt(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);
>> +struct i915_ggtt *i915_ggtt_create(struct drm_i915_private *i915);
>>
>> static inline bool i915_ggtt_has_aperture(const struct i915_ggtt *ggtt)
>> {
>> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>> index 69103ae37779..4e1bb3c23c63 100644
>> --- a/drivers/gpu/drm/i915/i915_driver.c
>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>> @@ -612,10 +612,6 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
>>
>> i915_perf_init(dev_priv);
>>
>> - ret = intel_gt_assign_ggtt(to_gt(dev_priv));
>> - if (ret)
>> - goto err_perf;
>> -
>> ret = i915_ggtt_probe_hw(dev_priv);
>> if (ret)
>> goto err_perf;
>> @@ -1316,7 +1312,8 @@ int i915_driver_suspend_switcheroo(struct drm_i915_private *i915,
>> static int i915_drm_resume(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = to_i915(dev);
>> - int ret;
>> + struct intel_gt *gt;
>> + int ret, i;
>>
>> disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>>
>> @@ -1331,6 +1328,11 @@ static int i915_drm_resume(struct drm_device *dev)
>> drm_err(&dev_priv->drm, "failed to re-enable GGTT\n");
>>
>> i915_ggtt_resume(to_gt(dev_priv)->ggtt);
>> +
>> + for_each_gt(gt, dev_priv, i)
>> + if (GRAPHICS_VER(gt->i915) >= 8)
>> + setup_private_pat(gt);
>> +
>> /* Must be called after GGTT is resumed. */
>> intel_dpt_resume(dev_priv);
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 8468ca9885fd..086c4702e1bf 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -1143,6 +1143,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>> for_each_gt(gt, dev_priv, i) {
>> intel_uc_fetch_firmwares(>->uc);
>> intel_wopcm_init(>->wopcm);
>> + if (GRAPHICS_VER(dev_priv) >= 8)
>> + setup_private_pat(gt);
>> }
>>
>> ret = i915_init_ggtt(dev_priv);
>> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
>> index f025ee4fa526..4cfe36b0366b 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
>> @@ -43,16 +43,25 @@ static bool dying_vma(struct i915_vma *vma)
>> return !kref_read(&vma->obj->base.refcount);
>> }
>>
>> -static int ggtt_flush(struct intel_gt *gt)
>> +static int ggtt_flush(struct i915_address_space *vm)
>> {
>> - /*
>> - * Not everything in the GGTT is tracked via vma (otherwise we
>> - * could evict as required with minimal stalling) so we are forced
>> - * to idle the GPU and explicitly retire outstanding requests in
>> - * the hopes that we can then remove contexts and the like only
>> - * bound by their active reference.
>> - */
>> - return intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> + struct intel_gt *gt;
>> + int ret = 0;
>> +
>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) {
>> + /*
>> + * Not everything in the GGTT is tracked via vma (otherwise we
>> + * could evict as required with minimal stalling) so we are forced
>> + * to idle the GPU and explicitly retire outstanding requests in
>> + * the hopes that we can then remove contexts and the like only
>> + * bound by their active reference.
>> + */
>> + ret = intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
>> + if (ret)
>> + return ret;
>> + }
>> + return ret;
>> }
>>
>> static bool grab_vma(struct i915_vma *vma, struct i915_gem_ww_ctx *ww)
>> @@ -149,6 +158,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
>> struct drm_mm_node *node;
>> enum drm_mm_insert_mode mode;
>> struct i915_vma *active;
>> + struct intel_gt *gt;
>> int ret;
>>
>> lockdep_assert_held(&vm->mutex);
>> @@ -174,7 +184,14 @@ i915_gem_evict_something(struct i915_address_space *vm,
>> min_size, alignment, color,
>> start, end, mode);
>>
>> - intel_gt_retire_requests(vm->gt);
>> + if (i915_is_ggtt(vm)) {
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> +
>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
>> + intel_gt_retire_requests(gt);
>> + } else {
>> + intel_gt_retire_requests(vm->gt);
>> + }
>>
>> search_again:
>> active = NULL;
>> @@ -246,7 +263,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
>> if (I915_SELFTEST_ONLY(igt_evict_ctl.fail_if_busy))
>> return -EBUSY;
>>
>> - ret = ggtt_flush(vm->gt);
>> + ret = ggtt_flush(vm);
>> if (ret)
>> return ret;
>>
>> @@ -332,7 +349,15 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
>> * a stray pin (preventing eviction) that can only be resolved by
>> * retiring.
>> */
>> - intel_gt_retire_requests(vm->gt);
>> + if (i915_is_ggtt(vm)) {
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> + struct intel_gt *gt;
>> +
>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
>> + intel_gt_retire_requests(gt);
>> + } else {
>> + intel_gt_retire_requests(vm->gt);
>> + }
>>
>> if (i915_vm_has_cache_coloring(vm)) {
>> /* Expand search to cover neighbouring guard pages (or lack!) */
>> @@ -438,7 +463,7 @@ int i915_gem_evict_vm(struct i915_address_space *vm, struct i915_gem_ww_ctx *ww)
>> * switch otherwise is ineffective.
>> */
>> if (i915_is_ggtt(vm)) {
>> - ret = ggtt_flush(vm->gt);
>> + ret = ggtt_flush(vm);
>> if (ret)
>> return ret;
>> }
>> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
>> index 703fee6b5f75..726705b10637 100644
>> --- a/drivers/gpu/drm/i915/i915_vma.c
>> +++ b/drivers/gpu/drm/i915/i915_vma.c
>> @@ -1544,6 +1544,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
>> u32 align, unsigned int flags)
>> {
>> struct i915_address_space *vm = vma->vm;
>> + struct intel_gt *gt;
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> int err;
>>
>> do {
>> @@ -1559,7 +1561,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
>> }
>>
>> /* Unlike i915_vma_pin, we don't take no for an answer! */
>> - flush_idle_contexts(vm->gt);
>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
>> + flush_idle_contexts(gt);
>> if (mutex_lock_interruptible(&vm->mutex) == 0) {
>> /*
>> * We pass NULL ww here, as we don't want to unbind
>> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
>> index e5dd82e7e480..2535b9684bd1 100644
>> --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
>> @@ -127,6 +127,8 @@ static void igt_pm_resume(struct drm_i915_private *i915)
>> */
>> with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
>> i915_ggtt_resume(to_gt(i915)->ggtt);
>> + if (GRAPHICS_VER(i915) >= 8)
>> + setup_private_pat(to_gt(i915));
>> i915_gem_resume(i915);
>> }
>> }
>> --
>> 2.25.1
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
@ 2022-11-23 4:17 ` Iddamsetty, Aravind
0 siblings, 0 replies; 14+ messages in thread
From: Iddamsetty, Aravind @ 2022-11-23 4:17 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx, dri-devel
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 <matthew.d.roper@intel.com>
>> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
>
> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
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 <linux/types.h>
>> #include <linux/stop_machine.h>
>>
>> +#include <drm/drm_managed.h>
>> #include <drm/i915_drm.h>
>> #include <drm/intel-gtt.h>
>>
>> @@ -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 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 b5ad9caa5537..b03788d7674e 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
>> @@ -110,9 +110,18 @@ 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 = i915_ggtt_create(gt->i915);
>> + if (IS_ERR(gt->ggtt))
>> + return PTR_ERR(gt->ggtt);
>> + }
>>
>> - return gt->ggtt ? 0 : -ENOMEM;
>> + list_add_tail(>->ggtt_link, >->ggtt->gt_list);
>> +
>> + return 0;
>> }
>>
>> int intel_gt_init_mmio(struct intel_gt *gt)
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> index c1d9cd255e06..8d915640914b 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> @@ -277,6 +277,9 @@ struct intel_gt {
>> struct kobject *sysfs_defaults;
>>
>> struct i915_perf_gt perf;
>> +
>> + /** link: &ggtt.gt_list */
>> + struct list_head ggtt_link;
>> };
>>
>> struct intel_gt_definition {
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
>> index 4d75ba4bb41d..d1900fec6cd1 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
>> @@ -390,6 +390,9 @@ struct i915_ggtt {
>> struct mutex error_mutex;
>> struct drm_mm_node error_capture;
>> struct drm_mm_node uc_fw;
>> +
>> + /** List of GTs mapping this GGTT */
>> + struct list_head gt_list;
>> };
>>
>> struct i915_ppgtt {
>> @@ -584,6 +587,7 @@ void i915_ggtt_disable_guc(struct i915_ggtt *ggtt);
>> int i915_init_ggtt(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);
>> +struct i915_ggtt *i915_ggtt_create(struct drm_i915_private *i915);
>>
>> static inline bool i915_ggtt_has_aperture(const struct i915_ggtt *ggtt)
>> {
>> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>> index 69103ae37779..4e1bb3c23c63 100644
>> --- a/drivers/gpu/drm/i915/i915_driver.c
>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>> @@ -612,10 +612,6 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
>>
>> i915_perf_init(dev_priv);
>>
>> - ret = intel_gt_assign_ggtt(to_gt(dev_priv));
>> - if (ret)
>> - goto err_perf;
>> -
>> ret = i915_ggtt_probe_hw(dev_priv);
>> if (ret)
>> goto err_perf;
>> @@ -1316,7 +1312,8 @@ int i915_driver_suspend_switcheroo(struct drm_i915_private *i915,
>> static int i915_drm_resume(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = to_i915(dev);
>> - int ret;
>> + struct intel_gt *gt;
>> + int ret, i;
>>
>> disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>>
>> @@ -1331,6 +1328,11 @@ static int i915_drm_resume(struct drm_device *dev)
>> drm_err(&dev_priv->drm, "failed to re-enable GGTT\n");
>>
>> i915_ggtt_resume(to_gt(dev_priv)->ggtt);
>> +
>> + for_each_gt(gt, dev_priv, i)
>> + if (GRAPHICS_VER(gt->i915) >= 8)
>> + setup_private_pat(gt);
>> +
>> /* Must be called after GGTT is resumed. */
>> intel_dpt_resume(dev_priv);
>>
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>> index 8468ca9885fd..086c4702e1bf 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -1143,6 +1143,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>> for_each_gt(gt, dev_priv, i) {
>> intel_uc_fetch_firmwares(>->uc);
>> intel_wopcm_init(>->wopcm);
>> + if (GRAPHICS_VER(dev_priv) >= 8)
>> + setup_private_pat(gt);
>> }
>>
>> ret = i915_init_ggtt(dev_priv);
>> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
>> index f025ee4fa526..4cfe36b0366b 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
>> @@ -43,16 +43,25 @@ static bool dying_vma(struct i915_vma *vma)
>> return !kref_read(&vma->obj->base.refcount);
>> }
>>
>> -static int ggtt_flush(struct intel_gt *gt)
>> +static int ggtt_flush(struct i915_address_space *vm)
>> {
>> - /*
>> - * Not everything in the GGTT is tracked via vma (otherwise we
>> - * could evict as required with minimal stalling) so we are forced
>> - * to idle the GPU and explicitly retire outstanding requests in
>> - * the hopes that we can then remove contexts and the like only
>> - * bound by their active reference.
>> - */
>> - return intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> + struct intel_gt *gt;
>> + int ret = 0;
>> +
>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) {
>> + /*
>> + * Not everything in the GGTT is tracked via vma (otherwise we
>> + * could evict as required with minimal stalling) so we are forced
>> + * to idle the GPU and explicitly retire outstanding requests in
>> + * the hopes that we can then remove contexts and the like only
>> + * bound by their active reference.
>> + */
>> + ret = intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
>> + if (ret)
>> + return ret;
>> + }
>> + return ret;
>> }
>>
>> static bool grab_vma(struct i915_vma *vma, struct i915_gem_ww_ctx *ww)
>> @@ -149,6 +158,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
>> struct drm_mm_node *node;
>> enum drm_mm_insert_mode mode;
>> struct i915_vma *active;
>> + struct intel_gt *gt;
>> int ret;
>>
>> lockdep_assert_held(&vm->mutex);
>> @@ -174,7 +184,14 @@ i915_gem_evict_something(struct i915_address_space *vm,
>> min_size, alignment, color,
>> start, end, mode);
>>
>> - intel_gt_retire_requests(vm->gt);
>> + if (i915_is_ggtt(vm)) {
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> +
>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
>> + intel_gt_retire_requests(gt);
>> + } else {
>> + intel_gt_retire_requests(vm->gt);
>> + }
>>
>> search_again:
>> active = NULL;
>> @@ -246,7 +263,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
>> if (I915_SELFTEST_ONLY(igt_evict_ctl.fail_if_busy))
>> return -EBUSY;
>>
>> - ret = ggtt_flush(vm->gt);
>> + ret = ggtt_flush(vm);
>> if (ret)
>> return ret;
>>
>> @@ -332,7 +349,15 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
>> * a stray pin (preventing eviction) that can only be resolved by
>> * retiring.
>> */
>> - intel_gt_retire_requests(vm->gt);
>> + if (i915_is_ggtt(vm)) {
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> + struct intel_gt *gt;
>> +
>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
>> + intel_gt_retire_requests(gt);
>> + } else {
>> + intel_gt_retire_requests(vm->gt);
>> + }
>>
>> if (i915_vm_has_cache_coloring(vm)) {
>> /* Expand search to cover neighbouring guard pages (or lack!) */
>> @@ -438,7 +463,7 @@ int i915_gem_evict_vm(struct i915_address_space *vm, struct i915_gem_ww_ctx *ww)
>> * switch otherwise is ineffective.
>> */
>> if (i915_is_ggtt(vm)) {
>> - ret = ggtt_flush(vm->gt);
>> + ret = ggtt_flush(vm);
>> if (ret)
>> return ret;
>> }
>> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
>> index 703fee6b5f75..726705b10637 100644
>> --- a/drivers/gpu/drm/i915/i915_vma.c
>> +++ b/drivers/gpu/drm/i915/i915_vma.c
>> @@ -1544,6 +1544,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
>> u32 align, unsigned int flags)
>> {
>> struct i915_address_space *vm = vma->vm;
>> + struct intel_gt *gt;
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> int err;
>>
>> do {
>> @@ -1559,7 +1561,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
>> }
>>
>> /* Unlike i915_vma_pin, we don't take no for an answer! */
>> - flush_idle_contexts(vm->gt);
>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
>> + flush_idle_contexts(gt);
>> if (mutex_lock_interruptible(&vm->mutex) == 0) {
>> /*
>> * We pass NULL ww here, as we don't want to unbind
>> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
>> index e5dd82e7e480..2535b9684bd1 100644
>> --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
>> @@ -127,6 +127,8 @@ static void igt_pm_resume(struct drm_i915_private *i915)
>> */
>> with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
>> i915_ggtt_resume(to_gt(i915)->ggtt);
>> + if (GRAPHICS_VER(i915) >= 8)
>> + setup_private_pat(to_gt(i915));
>> i915_gem_resume(i915);
>> }
>> }
>> --
>> 2.25.1
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
2022-11-23 4:17 ` [Intel-gfx] " Iddamsetty, Aravind
(?)
@ 2022-11-29 5:54 ` Lucas De Marchi
2022-11-29 6:03 ` Iddamsetty, Aravind
-1 siblings, 1 reply; 14+ messages in thread
From: Lucas De Marchi @ 2022-11-29 5:54 UTC (permalink / raw)
To: Iddamsetty, Aravind; +Cc: intel-gfx, dri-devel
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 <matthew.d.roper@intel.com>
>>> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
>>
>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>
>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 <linux/types.h>
>>> #include <linux/stop_machine.h>
>>>
>>> +#include <drm/drm_managed.h>
>>> #include <drm/i915_drm.h>
>>> #include <drm/intel-gtt.h>
>>>
>>> @@ -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(-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 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 b5ad9caa5537..b03788d7674e 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
>>> @@ -110,9 +110,18 @@ 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 = i915_ggtt_create(gt->i915);
>>> + if (IS_ERR(gt->ggtt))
>>> + return PTR_ERR(gt->ggtt);
>>> + }
>>>
>>> - return gt->ggtt ? 0 : -ENOMEM;
>>> + list_add_tail(>->ggtt_link, >->ggtt->gt_list);
>>> +
>>> + return 0;
>>> }
>>>
>>> int intel_gt_init_mmio(struct intel_gt *gt)
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>>> index c1d9cd255e06..8d915640914b 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>>> @@ -277,6 +277,9 @@ struct intel_gt {
>>> struct kobject *sysfs_defaults;
>>>
>>> struct i915_perf_gt perf;
>>> +
>>> + /** link: &ggtt.gt_list */
>>> + struct list_head ggtt_link;
>>> };
>>>
>>> struct intel_gt_definition {
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
>>> index 4d75ba4bb41d..d1900fec6cd1 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
>>> @@ -390,6 +390,9 @@ struct i915_ggtt {
>>> struct mutex error_mutex;
>>> struct drm_mm_node error_capture;
>>> struct drm_mm_node uc_fw;
>>> +
>>> + /** List of GTs mapping this GGTT */
>>> + struct list_head gt_list;
>>> };
>>>
>>> struct i915_ppgtt {
>>> @@ -584,6 +587,7 @@ void i915_ggtt_disable_guc(struct i915_ggtt *ggtt);
>>> int i915_init_ggtt(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);
>>> +struct i915_ggtt *i915_ggtt_create(struct drm_i915_private *i915);
>>>
>>> static inline bool i915_ggtt_has_aperture(const struct i915_ggtt *ggtt)
>>> {
>>> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>>> index 69103ae37779..4e1bb3c23c63 100644
>>> --- a/drivers/gpu/drm/i915/i915_driver.c
>>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>>> @@ -612,10 +612,6 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
>>>
>>> i915_perf_init(dev_priv);
>>>
>>> - ret = intel_gt_assign_ggtt(to_gt(dev_priv));
>>> - if (ret)
>>> - goto err_perf;
>>> -
>>> ret = i915_ggtt_probe_hw(dev_priv);
>>> if (ret)
>>> goto err_perf;
>>> @@ -1316,7 +1312,8 @@ int i915_driver_suspend_switcheroo(struct drm_i915_private *i915,
>>> static int i915_drm_resume(struct drm_device *dev)
>>> {
>>> struct drm_i915_private *dev_priv = to_i915(dev);
>>> - int ret;
>>> + struct intel_gt *gt;
>>> + int ret, i;
>>>
>>> disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>>>
>>> @@ -1331,6 +1328,11 @@ static int i915_drm_resume(struct drm_device *dev)
>>> drm_err(&dev_priv->drm, "failed to re-enable GGTT\n");
>>>
>>> i915_ggtt_resume(to_gt(dev_priv)->ggtt);
>>> +
>>> + for_each_gt(gt, dev_priv, i)
>>> + if (GRAPHICS_VER(gt->i915) >= 8)
>>> + setup_private_pat(gt);
>>> +
>>> /* Must be called after GGTT is resumed. */
>>> intel_dpt_resume(dev_priv);
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
>>> index 8468ca9885fd..086c4702e1bf 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>>> @@ -1143,6 +1143,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>>> for_each_gt(gt, dev_priv, i) {
>>> intel_uc_fetch_firmwares(>->uc);
>>> intel_wopcm_init(>->wopcm);
>>> + if (GRAPHICS_VER(dev_priv) >= 8)
>>> + setup_private_pat(gt);
>>> }
>>>
>>> ret = i915_init_ggtt(dev_priv);
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
>>> index f025ee4fa526..4cfe36b0366b 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
>>> @@ -43,16 +43,25 @@ static bool dying_vma(struct i915_vma *vma)
>>> return !kref_read(&vma->obj->base.refcount);
>>> }
>>>
>>> -static int ggtt_flush(struct intel_gt *gt)
>>> +static int ggtt_flush(struct i915_address_space *vm)
>>> {
>>> - /*
>>> - * Not everything in the GGTT is tracked via vma (otherwise we
>>> - * could evict as required with minimal stalling) so we are forced
>>> - * to idle the GPU and explicitly retire outstanding requests in
>>> - * the hopes that we can then remove contexts and the like only
>>> - * bound by their active reference.
>>> - */
>>> - return intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
>>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>>> + struct intel_gt *gt;
>>> + int ret = 0;
>>> +
>>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) {
>>> + /*
>>> + * Not everything in the GGTT is tracked via vma (otherwise we
>>> + * could evict as required with minimal stalling) so we are forced
>>> + * to idle the GPU and explicitly retire outstanding requests in
>>> + * the hopes that we can then remove contexts and the like only
>>> + * bound by their active reference.
>>> + */
>>> + ret = intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
>>> + if (ret)
>>> + return ret;
>>> + }
>>> + return ret;
>>> }
>>>
>>> static bool grab_vma(struct i915_vma *vma, struct i915_gem_ww_ctx *ww)
>>> @@ -149,6 +158,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
>>> struct drm_mm_node *node;
>>> enum drm_mm_insert_mode mode;
>>> struct i915_vma *active;
>>> + struct intel_gt *gt;
>>> int ret;
>>>
>>> lockdep_assert_held(&vm->mutex);
>>> @@ -174,7 +184,14 @@ i915_gem_evict_something(struct i915_address_space *vm,
>>> min_size, alignment, color,
>>> start, end, mode);
>>>
>>> - intel_gt_retire_requests(vm->gt);
>>> + if (i915_is_ggtt(vm)) {
>>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>>> +
>>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
>>> + intel_gt_retire_requests(gt);
>>> + } else {
>>> + intel_gt_retire_requests(vm->gt);
>>> + }
>>>
>>> search_again:
>>> active = NULL;
>>> @@ -246,7 +263,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
>>> if (I915_SELFTEST_ONLY(igt_evict_ctl.fail_if_busy))
>>> return -EBUSY;
>>>
>>> - ret = ggtt_flush(vm->gt);
>>> + ret = ggtt_flush(vm);
>>> if (ret)
>>> return ret;
>>>
>>> @@ -332,7 +349,15 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
>>> * a stray pin (preventing eviction) that can only be resolved by
>>> * retiring.
>>> */
>>> - intel_gt_retire_requests(vm->gt);
>>> + if (i915_is_ggtt(vm)) {
>>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>>> + struct intel_gt *gt;
>>> +
>>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
>>> + intel_gt_retire_requests(gt);
>>> + } else {
>>> + intel_gt_retire_requests(vm->gt);
>>> + }
>>>
>>> if (i915_vm_has_cache_coloring(vm)) {
>>> /* Expand search to cover neighbouring guard pages (or lack!) */
>>> @@ -438,7 +463,7 @@ int i915_gem_evict_vm(struct i915_address_space *vm, struct i915_gem_ww_ctx *ww)
>>> * switch otherwise is ineffective.
>>> */
>>> if (i915_is_ggtt(vm)) {
>>> - ret = ggtt_flush(vm->gt);
>>> + ret = ggtt_flush(vm);
>>> if (ret)
>>> return ret;
>>> }
>>> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
>>> index 703fee6b5f75..726705b10637 100644
>>> --- a/drivers/gpu/drm/i915/i915_vma.c
>>> +++ b/drivers/gpu/drm/i915/i915_vma.c
>>> @@ -1544,6 +1544,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
>>> u32 align, unsigned int flags)
>>> {
>>> struct i915_address_space *vm = vma->vm;
>>> + struct intel_gt *gt;
>>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>>> int err;
>>>
>>> do {
>>> @@ -1559,7 +1561,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
>>> }
>>>
>>> /* Unlike i915_vma_pin, we don't take no for an answer! */
>>> - flush_idle_contexts(vm->gt);
>>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
>>> + flush_idle_contexts(gt);
>>> if (mutex_lock_interruptible(&vm->mutex) == 0) {
>>> /*
>>> * We pass NULL ww here, as we don't want to unbind
>>> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
>>> index e5dd82e7e480..2535b9684bd1 100644
>>> --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
>>> +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
>>> @@ -127,6 +127,8 @@ static void igt_pm_resume(struct drm_i915_private *i915)
>>> */
>>> with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
>>> i915_ggtt_resume(to_gt(i915)->ggtt);
>>> + if (GRAPHICS_VER(i915) >= 8)
>>> + setup_private_pat(to_gt(i915));
>>> i915_gem_resume(i915);
>>> }
>>> }
>>> --
>>> 2.25.1
>>>
>>
>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
2022-11-29 5:54 ` Lucas De Marchi
@ 2022-11-29 6:03 ` Iddamsetty, Aravind
2022-11-29 6:45 ` Lucas De Marchi
0 siblings, 1 reply; 14+ messages in thread
From: Iddamsetty, Aravind @ 2022-11-29 6:03 UTC (permalink / raw)
To: Lucas De Marchi; +Cc: intel-gfx, dri-devel
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 <matthew.d.roper@intel.com>
>>>> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
>>>
>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>>
>> 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 <linux/types.h>
>>>> #include <linux/stop_machine.h>
>>>>
>>>> +#include <drm/drm_managed.h>
>>>> #include <drm/i915_drm.h>
>>>> #include <drm/intel-gtt.h>
>>>>
>>>> @@ -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.
>
> pushed, thanks
Thanks a lot for the help.
Regards,
Aravind.
>
> 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(-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 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 b5ad9caa5537..b03788d7674e 100644
>>>> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
>>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
>>>> @@ -110,9 +110,18 @@ 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 = i915_ggtt_create(gt->i915);
>>>> + if (IS_ERR(gt->ggtt))
>>>> + return PTR_ERR(gt->ggtt);
>>>> + }
>>>>
>>>> - return gt->ggtt ? 0 : -ENOMEM;
>>>> + list_add_tail(>->ggtt_link, >->ggtt->gt_list);
>>>> +
>>>> + return 0;
>>>> }
>>>>
>>>> int intel_gt_init_mmio(struct intel_gt *gt)
>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>>>> b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>>>> index c1d9cd255e06..8d915640914b 100644
>>>> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>>>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>>>> @@ -277,6 +277,9 @@ struct intel_gt {
>>>> struct kobject *sysfs_defaults;
>>>>
>>>> struct i915_perf_gt perf;
>>>> +
>>>> + /** link: &ggtt.gt_list */
>>>> + struct list_head ggtt_link;
>>>> };
>>>>
>>>> struct intel_gt_definition {
>>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h
>>>> b/drivers/gpu/drm/i915/gt/intel_gtt.h
>>>> index 4d75ba4bb41d..d1900fec6cd1 100644
>>>> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
>>>> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
>>>> @@ -390,6 +390,9 @@ struct i915_ggtt {
>>>> struct mutex error_mutex;
>>>> struct drm_mm_node error_capture;
>>>> struct drm_mm_node uc_fw;
>>>> +
>>>> + /** List of GTs mapping this GGTT */
>>>> + struct list_head gt_list;
>>>> };
>>>>
>>>> struct i915_ppgtt {
>>>> @@ -584,6 +587,7 @@ void i915_ggtt_disable_guc(struct i915_ggtt *ggtt);
>>>> int i915_init_ggtt(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);
>>>> +struct i915_ggtt *i915_ggtt_create(struct drm_i915_private *i915);
>>>>
>>>> static inline bool i915_ggtt_has_aperture(const struct i915_ggtt
>>>> *ggtt)
>>>> {
>>>> diff --git a/drivers/gpu/drm/i915/i915_driver.c
>>>> b/drivers/gpu/drm/i915/i915_driver.c
>>>> index 69103ae37779..4e1bb3c23c63 100644
>>>> --- a/drivers/gpu/drm/i915/i915_driver.c
>>>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>>>> @@ -612,10 +612,6 @@ static int i915_driver_hw_probe(struct
>>>> drm_i915_private *dev_priv)
>>>>
>>>> i915_perf_init(dev_priv);
>>>>
>>>> - ret = intel_gt_assign_ggtt(to_gt(dev_priv));
>>>> - if (ret)
>>>> - goto err_perf;
>>>> -
>>>> ret = i915_ggtt_probe_hw(dev_priv);
>>>> if (ret)
>>>> goto err_perf;
>>>> @@ -1316,7 +1312,8 @@ int i915_driver_suspend_switcheroo(struct
>>>> drm_i915_private *i915,
>>>> static int i915_drm_resume(struct drm_device *dev)
>>>> {
>>>> struct drm_i915_private *dev_priv = to_i915(dev);
>>>> - int ret;
>>>> + struct intel_gt *gt;
>>>> + int ret, i;
>>>>
>>>> disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>>>>
>>>> @@ -1331,6 +1328,11 @@ static int i915_drm_resume(struct drm_device
>>>> *dev)
>>>> drm_err(&dev_priv->drm, "failed to re-enable GGTT\n");
>>>>
>>>> i915_ggtt_resume(to_gt(dev_priv)->ggtt);
>>>> +
>>>> + for_each_gt(gt, dev_priv, i)
>>>> + if (GRAPHICS_VER(gt->i915) >= 8)
>>>> + setup_private_pat(gt);
>>>> +
>>>> /* Must be called after GGTT is resumed. */
>>>> intel_dpt_resume(dev_priv);
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/i915_gem.c
>>>> b/drivers/gpu/drm/i915/i915_gem.c
>>>> index 8468ca9885fd..086c4702e1bf 100644
>>>> --- a/drivers/gpu/drm/i915/i915_gem.c
>>>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>>>> @@ -1143,6 +1143,8 @@ int i915_gem_init(struct drm_i915_private
>>>> *dev_priv)
>>>> for_each_gt(gt, dev_priv, i) {
>>>> intel_uc_fetch_firmwares(>->uc);
>>>> intel_wopcm_init(>->wopcm);
>>>> + if (GRAPHICS_VER(dev_priv) >= 8)
>>>> + setup_private_pat(gt);
>>>> }
>>>>
>>>> ret = i915_init_ggtt(dev_priv);
>>>> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c
>>>> b/drivers/gpu/drm/i915/i915_gem_evict.c
>>>> index f025ee4fa526..4cfe36b0366b 100644
>>>> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
>>>> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
>>>> @@ -43,16 +43,25 @@ static bool dying_vma(struct i915_vma *vma)
>>>> return !kref_read(&vma->obj->base.refcount);
>>>> }
>>>>
>>>> -static int ggtt_flush(struct intel_gt *gt)
>>>> +static int ggtt_flush(struct i915_address_space *vm)
>>>> {
>>>> - /*
>>>> - * Not everything in the GGTT is tracked via vma (otherwise we
>>>> - * could evict as required with minimal stalling) so we are forced
>>>> - * to idle the GPU and explicitly retire outstanding requests in
>>>> - * the hopes that we can then remove contexts and the like only
>>>> - * bound by their active reference.
>>>> - */
>>>> - return intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
>>>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>>>> + struct intel_gt *gt;
>>>> + int ret = 0;
>>>> +
>>>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) {
>>>> + /*
>>>> + * Not everything in the GGTT is tracked via vma (otherwise we
>>>> + * could evict as required with minimal stalling) so we are
>>>> forced
>>>> + * to idle the GPU and explicitly retire outstanding
>>>> requests in
>>>> + * the hopes that we can then remove contexts and the like
>>>> only
>>>> + * bound by their active reference.
>>>> + */
>>>> + ret = intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
>>>> + if (ret)
>>>> + return ret;
>>>> + }
>>>> + return ret;
>>>> }
>>>>
>>>> static bool grab_vma(struct i915_vma *vma, struct i915_gem_ww_ctx *ww)
>>>> @@ -149,6 +158,7 @@ i915_gem_evict_something(struct
>>>> i915_address_space *vm,
>>>> struct drm_mm_node *node;
>>>> enum drm_mm_insert_mode mode;
>>>> struct i915_vma *active;
>>>> + struct intel_gt *gt;
>>>> int ret;
>>>>
>>>> lockdep_assert_held(&vm->mutex);
>>>> @@ -174,7 +184,14 @@ i915_gem_evict_something(struct
>>>> i915_address_space *vm,
>>>> min_size, alignment, color,
>>>> start, end, mode);
>>>>
>>>> - intel_gt_retire_requests(vm->gt);
>>>> + if (i915_is_ggtt(vm)) {
>>>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>>>> +
>>>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
>>>> + intel_gt_retire_requests(gt);
>>>> + } else {
>>>> + intel_gt_retire_requests(vm->gt);
>>>> + }
>>>>
>>>> search_again:
>>>> active = NULL;
>>>> @@ -246,7 +263,7 @@ i915_gem_evict_something(struct
>>>> i915_address_space *vm,
>>>> if (I915_SELFTEST_ONLY(igt_evict_ctl.fail_if_busy))
>>>> return -EBUSY;
>>>>
>>>> - ret = ggtt_flush(vm->gt);
>>>> + ret = ggtt_flush(vm);
>>>> if (ret)
>>>> return ret;
>>>>
>>>> @@ -332,7 +349,15 @@ int i915_gem_evict_for_node(struct
>>>> i915_address_space *vm,
>>>> * a stray pin (preventing eviction) that can only be resolved by
>>>> * retiring.
>>>> */
>>>> - intel_gt_retire_requests(vm->gt);
>>>> + if (i915_is_ggtt(vm)) {
>>>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>>>> + struct intel_gt *gt;
>>>> +
>>>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
>>>> + intel_gt_retire_requests(gt);
>>>> + } else {
>>>> + intel_gt_retire_requests(vm->gt);
>>>> + }
>>>>
>>>> if (i915_vm_has_cache_coloring(vm)) {
>>>> /* Expand search to cover neighbouring guard pages (or
>>>> lack!) */
>>>> @@ -438,7 +463,7 @@ int i915_gem_evict_vm(struct i915_address_space
>>>> *vm, struct i915_gem_ww_ctx *ww)
>>>> * switch otherwise is ineffective.
>>>> */
>>>> if (i915_is_ggtt(vm)) {
>>>> - ret = ggtt_flush(vm->gt);
>>>> + ret = ggtt_flush(vm);
>>>> if (ret)
>>>> return ret;
>>>> }
>>>> diff --git a/drivers/gpu/drm/i915/i915_vma.c
>>>> b/drivers/gpu/drm/i915/i915_vma.c
>>>> index 703fee6b5f75..726705b10637 100644
>>>> --- a/drivers/gpu/drm/i915/i915_vma.c
>>>> +++ b/drivers/gpu/drm/i915/i915_vma.c
>>>> @@ -1544,6 +1544,8 @@ static int __i915_ggtt_pin(struct i915_vma
>>>> *vma, struct i915_gem_ww_ctx *ww,
>>>> u32 align, unsigned int flags)
>>>> {
>>>> struct i915_address_space *vm = vma->vm;
>>>> + struct intel_gt *gt;
>>>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>>>> int err;
>>>>
>>>> do {
>>>> @@ -1559,7 +1561,8 @@ static int __i915_ggtt_pin(struct i915_vma
>>>> *vma, struct i915_gem_ww_ctx *ww,
>>>> }
>>>>
>>>> /* Unlike i915_vma_pin, we don't take no for an answer! */
>>>> - flush_idle_contexts(vm->gt);
>>>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
>>>> + flush_idle_contexts(gt);
>>>> if (mutex_lock_interruptible(&vm->mutex) == 0) {
>>>> /*
>>>> * We pass NULL ww here, as we don't want to unbind
>>>> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c
>>>> b/drivers/gpu/drm/i915/selftests/i915_gem.c
>>>> index e5dd82e7e480..2535b9684bd1 100644
>>>> --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
>>>> +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
>>>> @@ -127,6 +127,8 @@ static void igt_pm_resume(struct
>>>> drm_i915_private *i915)
>>>> */
>>>> with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
>>>> i915_ggtt_resume(to_gt(i915)->ggtt);
>>>> + if (GRAPHICS_VER(i915) >= 8)
>>>> + setup_private_pat(to_gt(i915));
>>>> i915_gem_resume(i915);
>>>> }
>>>> }
>>>> --
>>>> 2.25.1
>>>>
>>>
>>
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
2022-11-29 6:03 ` Iddamsetty, Aravind
@ 2022-11-29 6:45 ` Lucas De Marchi
0 siblings, 0 replies; 14+ messages in thread
From: Lucas De Marchi @ 2022-11-29 6:45 UTC (permalink / raw)
To: Iddamsetty, Aravind; +Cc: intel-gfx, dri-devel
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 <matthew.d.roper@intel.com>
>>>>> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
>>>>
>>>> Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
>>>
>>> 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 <linux/types.h>
>>>>> #include <linux/stop_machine.h>
>>>>>
>>>>> +#include <drm/drm_managed.h>
>>>>> #include <drm/i915_drm.h>
>>>>> #include <drm/intel-gtt.h>
>>>>>
>>>>> @@ -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
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
2022-11-22 7:01 ` [Intel-gfx] " Aravind Iddamsetty
` (4 preceding siblings ...)
(?)
@ 2022-11-29 10:11 ` Tvrtko Ursulin
2022-11-29 11:25 ` Iddamsetty, Aravind
-1 siblings, 1 reply; 14+ messages in thread
From: Tvrtko Ursulin @ 2022-11-29 10:11 UTC (permalink / raw)
To: Aravind Iddamsetty, intel-gfx; +Cc: dri-devel
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 <matthew.d.roper@intel.com>
> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
> ---
> 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 <linux/types.h>
> #include <linux/stop_machine.h>
>
> +#include <drm/drm_managed.h>
> #include <drm/i915_drm.h>
> #include <drm/intel-gtt.h>
>
> @@ -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 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);
Moving this really should have been a separate patch.
> -
> 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 b5ad9caa5537..b03788d7674e 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
> @@ -110,9 +110,18 @@ 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;
AFAICT this creates two implicit assumptions: 1) That for_each_gt
iterates in a certain order (primary always first), when it is calling
in here; and 2) That the primary tile is not media. Ideally a
GEM_BUG_ON(!gt->ggtt) would cover for it, since I am not sure the
list_add_tail below is guaranteed to explode or not.
> + } else {
> + gt->ggtt = i915_ggtt_create(gt->i915);
> + if (IS_ERR(gt->ggtt))
> + return PTR_ERR(gt->ggtt);
> + }
>
> - return gt->ggtt ? 0 : -ENOMEM;
> + list_add_tail(>->ggtt_link, >->ggtt->gt_list);
> +
> + return 0;
> }
>
> int intel_gt_init_mmio(struct intel_gt *gt)
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> index c1d9cd255e06..8d915640914b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
> @@ -277,6 +277,9 @@ struct intel_gt {
> struct kobject *sysfs_defaults;
>
> struct i915_perf_gt perf;
> +
> + /** link: &ggtt.gt_list */
> + struct list_head ggtt_link;
> };
>
> struct intel_gt_definition {
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h b/drivers/gpu/drm/i915/gt/intel_gtt.h
> index 4d75ba4bb41d..d1900fec6cd1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
> @@ -390,6 +390,9 @@ struct i915_ggtt {
> struct mutex error_mutex;
> struct drm_mm_node error_capture;
> struct drm_mm_node uc_fw;
> +
> + /** List of GTs mapping this GGTT */
> + struct list_head gt_list;
> };
>
> struct i915_ppgtt {
> @@ -584,6 +587,7 @@ void i915_ggtt_disable_guc(struct i915_ggtt *ggtt);
> int i915_init_ggtt(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);
> +struct i915_ggtt *i915_ggtt_create(struct drm_i915_private *i915);
>
> static inline bool i915_ggtt_has_aperture(const struct i915_ggtt *ggtt)
> {
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 69103ae37779..4e1bb3c23c63 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -612,10 +612,6 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
>
> i915_perf_init(dev_priv);
>
> - ret = intel_gt_assign_ggtt(to_gt(dev_priv));
> - if (ret)
> - goto err_perf;
> -
> ret = i915_ggtt_probe_hw(dev_priv);
> if (ret)
> goto err_perf;
> @@ -1316,7 +1312,8 @@ int i915_driver_suspend_switcheroo(struct drm_i915_private *i915,
> static int i915_drm_resume(struct drm_device *dev)
> {
> struct drm_i915_private *dev_priv = to_i915(dev);
> - int ret;
> + struct intel_gt *gt;
> + int ret, i;
>
> disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>
> @@ -1331,6 +1328,11 @@ static int i915_drm_resume(struct drm_device *dev)
> drm_err(&dev_priv->drm, "failed to re-enable GGTT\n");
>
> i915_ggtt_resume(to_gt(dev_priv)->ggtt);
> +
> + for_each_gt(gt, dev_priv, i)
> + if (GRAPHICS_VER(gt->i915) >= 8)
> + setup_private_pat(gt);
If this is now called from i915_driver.c, the >= 8 check should probably
go into setup_private_pat. And exported function renamed to
intel_gt_setup_private_pat. Otherwise it feels like top level code has a
little bit of too much deep knowledge of things.
> +
> /* Must be called after GGTT is resumed. */
> intel_dpt_resume(dev_priv);
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 8468ca9885fd..086c4702e1bf 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1143,6 +1143,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
> for_each_gt(gt, dev_priv, i) {
> intel_uc_fetch_firmwares(>->uc);
> intel_wopcm_init(>->wopcm);
> + if (GRAPHICS_VER(dev_priv) >= 8)
> + setup_private_pat(gt);
> }
>
> ret = i915_init_ggtt(dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c b/drivers/gpu/drm/i915/i915_gem_evict.c
> index f025ee4fa526..4cfe36b0366b 100644
> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
> @@ -43,16 +43,25 @@ static bool dying_vma(struct i915_vma *vma)
> return !kref_read(&vma->obj->base.refcount);
> }
>
> -static int ggtt_flush(struct intel_gt *gt)
> +static int ggtt_flush(struct i915_address_space *vm)
> {
> - /*
> - * Not everything in the GGTT is tracked via vma (otherwise we
> - * could evict as required with minimal stalling) so we are forced
> - * to idle the GPU and explicitly retire outstanding requests in
> - * the hopes that we can then remove contexts and the like only
> - * bound by their active reference.
> - */
> - return intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> + struct intel_gt *gt;
> + int ret = 0;
> +
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) {
> + /*
> + * Not everything in the GGTT is tracked via vma (otherwise we
> + * could evict as required with minimal stalling) so we are forced
> + * to idle the GPU and explicitly retire outstanding requests in
> + * the hopes that we can then remove contexts and the like only
> + * bound by their active reference.
> + */
> + ret = intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
> + if (ret)
> + return ret;
> + }
> + return ret;
> }
>
> static bool grab_vma(struct i915_vma *vma, struct i915_gem_ww_ctx *ww)
> @@ -149,6 +158,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
> struct drm_mm_node *node;
> enum drm_mm_insert_mode mode;
> struct i915_vma *active;
> + struct intel_gt *gt;
Declare where it is used, like in i915_gem_evict_for_node? Or maybe add
a local helper like vm_retire_requests, not sure?
Regards,
Tvrtko
> int ret;
>
> lockdep_assert_held(&vm->mutex);
> @@ -174,7 +184,14 @@ i915_gem_evict_something(struct i915_address_space *vm,
> min_size, alignment, color,
> start, end, mode);
>
> - intel_gt_retire_requests(vm->gt);
> + if (i915_is_ggtt(vm)) {
> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> +
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
> + intel_gt_retire_requests(gt);
> + } else {
> + intel_gt_retire_requests(vm->gt);
> + }
>
> search_again:
> active = NULL;
> @@ -246,7 +263,7 @@ i915_gem_evict_something(struct i915_address_space *vm,
> if (I915_SELFTEST_ONLY(igt_evict_ctl.fail_if_busy))
> return -EBUSY;
>
> - ret = ggtt_flush(vm->gt);
> + ret = ggtt_flush(vm);
> if (ret)
> return ret;
>
> @@ -332,7 +349,15 @@ int i915_gem_evict_for_node(struct i915_address_space *vm,
> * a stray pin (preventing eviction) that can only be resolved by
> * retiring.
> */
> - intel_gt_retire_requests(vm->gt);
> + if (i915_is_ggtt(vm)) {
> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> + struct intel_gt *gt;
> +
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
> + intel_gt_retire_requests(gt);
> + } else {
> + intel_gt_retire_requests(vm->gt);
> + }
>
> if (i915_vm_has_cache_coloring(vm)) {
> /* Expand search to cover neighbouring guard pages (or lack!) */
> @@ -438,7 +463,7 @@ int i915_gem_evict_vm(struct i915_address_space *vm, struct i915_gem_ww_ctx *ww)
> * switch otherwise is ineffective.
> */
> if (i915_is_ggtt(vm)) {
> - ret = ggtt_flush(vm->gt);
> + ret = ggtt_flush(vm);
> if (ret)
> return ret;
> }
> diff --git a/drivers/gpu/drm/i915/i915_vma.c b/drivers/gpu/drm/i915/i915_vma.c
> index 703fee6b5f75..726705b10637 100644
> --- a/drivers/gpu/drm/i915/i915_vma.c
> +++ b/drivers/gpu/drm/i915/i915_vma.c
> @@ -1544,6 +1544,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
> u32 align, unsigned int flags)
> {
> struct i915_address_space *vm = vma->vm;
> + struct intel_gt *gt;
> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
> int err;
>
> do {
> @@ -1559,7 +1561,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma, struct i915_gem_ww_ctx *ww,
> }
>
> /* Unlike i915_vma_pin, we don't take no for an answer! */
> - flush_idle_contexts(vm->gt);
> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
> + flush_idle_contexts(gt);
> if (mutex_lock_interruptible(&vm->mutex) == 0) {
> /*
> * We pass NULL ww here, as we don't want to unbind
> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c b/drivers/gpu/drm/i915/selftests/i915_gem.c
> index e5dd82e7e480..2535b9684bd1 100644
> --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
> +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
> @@ -127,6 +127,8 @@ static void igt_pm_resume(struct drm_i915_private *i915)
> */
> with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
> i915_ggtt_resume(to_gt(i915)->ggtt);
> + if (GRAPHICS_VER(i915) >= 8)
> + setup_private_pat(to_gt(i915));
> i915_gem_resume(i915);
> }
> }
^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [Intel-gfx] [PATCH v4] drm/i915/mtl: Media GT and Render GT share common GGTT
2022-11-29 10:11 ` Tvrtko Ursulin
@ 2022-11-29 11:25 ` Iddamsetty, Aravind
0 siblings, 0 replies; 14+ messages in thread
From: Iddamsetty, Aravind @ 2022-11-29 11:25 UTC (permalink / raw)
To: Tvrtko Ursulin, intel-gfx; +Cc: dri-devel
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 <matthew.d.roper@intel.com>
>> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
>> ---
>> 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 <linux/types.h>
>> #include <linux/stop_machine.h>
>> +#include <drm/drm_managed.h>
>> #include <drm/i915_drm.h>
>> #include <drm/intel-gtt.h>
>> @@ -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 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);
>
> Moving this really should have been a separate patch.
>
>> -
>> 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 b5ad9caa5537..b03788d7674e 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt.c
>> @@ -110,9 +110,18 @@ 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;
>
> AFAICT this creates two implicit assumptions: 1) That for_each_gt
> iterates in a certain order (primary always first), when it is calling
> in here; and 2) That the primary tile is not media. Ideally a
> GEM_BUG_ON(!gt->ggtt) would cover for it, since I am not sure the
> list_add_tail below is guaranteed to explode or not.
Ok makes sense.
>
>> + } else {
>> + gt->ggtt = i915_ggtt_create(gt->i915);
>> + if (IS_ERR(gt->ggtt))
>> + return PTR_ERR(gt->ggtt);
>> + }
>> - return gt->ggtt ? 0 : -ENOMEM;
>> + list_add_tail(>->ggtt_link, >->ggtt->gt_list);
>> +
>> + return 0;
>> }
>> int intel_gt_init_mmio(struct intel_gt *gt)
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> index c1d9cd255e06..8d915640914b 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>> @@ -277,6 +277,9 @@ struct intel_gt {
>> struct kobject *sysfs_defaults;
>> struct i915_perf_gt perf;
>> +
>> + /** link: &ggtt.gt_list */
>> + struct list_head ggtt_link;
>> };
>> struct intel_gt_definition {
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.h
>> b/drivers/gpu/drm/i915/gt/intel_gtt.h
>> index 4d75ba4bb41d..d1900fec6cd1 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gtt.h
>> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.h
>> @@ -390,6 +390,9 @@ struct i915_ggtt {
>> struct mutex error_mutex;
>> struct drm_mm_node error_capture;
>> struct drm_mm_node uc_fw;
>> +
>> + /** List of GTs mapping this GGTT */
>> + struct list_head gt_list;
>> };
>> struct i915_ppgtt {
>> @@ -584,6 +587,7 @@ void i915_ggtt_disable_guc(struct i915_ggtt *ggtt);
>> int i915_init_ggtt(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);
>> +struct i915_ggtt *i915_ggtt_create(struct drm_i915_private *i915);
>> static inline bool i915_ggtt_has_aperture(const struct i915_ggtt
>> *ggtt)
>> {
>> diff --git a/drivers/gpu/drm/i915/i915_driver.c
>> b/drivers/gpu/drm/i915/i915_driver.c
>> index 69103ae37779..4e1bb3c23c63 100644
>> --- a/drivers/gpu/drm/i915/i915_driver.c
>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>> @@ -612,10 +612,6 @@ static int i915_driver_hw_probe(struct
>> drm_i915_private *dev_priv)
>> i915_perf_init(dev_priv);
>> - ret = intel_gt_assign_ggtt(to_gt(dev_priv));
>> - if (ret)
>> - goto err_perf;
>> -
>> ret = i915_ggtt_probe_hw(dev_priv);
>> if (ret)
>> goto err_perf;
>> @@ -1316,7 +1312,8 @@ int i915_driver_suspend_switcheroo(struct
>> drm_i915_private *i915,
>> static int i915_drm_resume(struct drm_device *dev)
>> {
>> struct drm_i915_private *dev_priv = to_i915(dev);
>> - int ret;
>> + struct intel_gt *gt;
>> + int ret, i;
>> disable_rpm_wakeref_asserts(&dev_priv->runtime_pm);
>> @@ -1331,6 +1328,11 @@ static int i915_drm_resume(struct drm_device
>> *dev)
>> drm_err(&dev_priv->drm, "failed to re-enable GGTT\n");
>> i915_ggtt_resume(to_gt(dev_priv)->ggtt);
>> +
>> + for_each_gt(gt, dev_priv, i)
>> + if (GRAPHICS_VER(gt->i915) >= 8)
>> + setup_private_pat(gt);
>
> If this is now called from i915_driver.c, the >= 8 check should probably
> go into setup_private_pat. And exported function renamed to
> intel_gt_setup_private_pat. Otherwise it feels like top level code has a
> little bit of too much deep knowledge of things.
got it.
>
>> +
>> /* Must be called after GGTT is resumed. */
>> intel_dpt_resume(dev_priv);
>> diff --git a/drivers/gpu/drm/i915/i915_gem.c
>> b/drivers/gpu/drm/i915/i915_gem.c
>> index 8468ca9885fd..086c4702e1bf 100644
>> --- a/drivers/gpu/drm/i915/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/i915_gem.c
>> @@ -1143,6 +1143,8 @@ int i915_gem_init(struct drm_i915_private
>> *dev_priv)
>> for_each_gt(gt, dev_priv, i) {
>> intel_uc_fetch_firmwares(>->uc);
>> intel_wopcm_init(>->wopcm);
>> + if (GRAPHICS_VER(dev_priv) >= 8)
>> + setup_private_pat(gt);
>> }
>> ret = i915_init_ggtt(dev_priv);
>> diff --git a/drivers/gpu/drm/i915/i915_gem_evict.c
>> b/drivers/gpu/drm/i915/i915_gem_evict.c
>> index f025ee4fa526..4cfe36b0366b 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_evict.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_evict.c
>> @@ -43,16 +43,25 @@ static bool dying_vma(struct i915_vma *vma)
>> return !kref_read(&vma->obj->base.refcount);
>> }
>> -static int ggtt_flush(struct intel_gt *gt)
>> +static int ggtt_flush(struct i915_address_space *vm)
>> {
>> - /*
>> - * Not everything in the GGTT is tracked via vma (otherwise we
>> - * could evict as required with minimal stalling) so we are forced
>> - * to idle the GPU and explicitly retire outstanding requests in
>> - * the hopes that we can then remove contexts and the like only
>> - * bound by their active reference.
>> - */
>> - return intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> + struct intel_gt *gt;
>> + int ret = 0;
>> +
>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) {
>> + /*
>> + * Not everything in the GGTT is tracked via vma (otherwise we
>> + * could evict as required with minimal stalling) so we are
>> forced
>> + * to idle the GPU and explicitly retire outstanding requests in
>> + * the hopes that we can then remove contexts and the like only
>> + * bound by their active reference.
>> + */
>> + ret = intel_gt_wait_for_idle(gt, MAX_SCHEDULE_TIMEOUT);
>> + if (ret)
>> + return ret;
>> + }
>> + return ret;
>> }
>> static bool grab_vma(struct i915_vma *vma, struct i915_gem_ww_ctx
>> *ww)
>> @@ -149,6 +158,7 @@ i915_gem_evict_something(struct i915_address_space
>> *vm,
>> struct drm_mm_node *node;
>> enum drm_mm_insert_mode mode;
>> struct i915_vma *active;
>> + struct intel_gt *gt;
>
> Declare where it is used, like in i915_gem_evict_for_node? Or maybe add
> a local helper like vm_retire_requests, not sure?
sure.
Thanks for your review Tvrtko, I would be doing these changes a little
later. Hope it should be fine.
Thanks,
Aravind.
>
> Regards,
>
> Tvrtko
>
>> int ret;
>> lockdep_assert_held(&vm->mutex);
>> @@ -174,7 +184,14 @@ i915_gem_evict_something(struct
>> i915_address_space *vm,
>> min_size, alignment, color,
>> start, end, mode);
>> - intel_gt_retire_requests(vm->gt);
>> + if (i915_is_ggtt(vm)) {
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> +
>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
>> + intel_gt_retire_requests(gt);
>> + } else {
>> + intel_gt_retire_requests(vm->gt);
>> + }
>> search_again:
>> active = NULL;
>> @@ -246,7 +263,7 @@ i915_gem_evict_something(struct i915_address_space
>> *vm,
>> if (I915_SELFTEST_ONLY(igt_evict_ctl.fail_if_busy))
>> return -EBUSY;
>> - ret = ggtt_flush(vm->gt);
>> + ret = ggtt_flush(vm);
>> if (ret)
>> return ret;
>> @@ -332,7 +349,15 @@ int i915_gem_evict_for_node(struct
>> i915_address_space *vm,
>> * a stray pin (preventing eviction) that can only be resolved by
>> * retiring.
>> */
>> - intel_gt_retire_requests(vm->gt);
>> + if (i915_is_ggtt(vm)) {
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> + struct intel_gt *gt;
>> +
>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
>> + intel_gt_retire_requests(gt);
>> + } else {
>> + intel_gt_retire_requests(vm->gt);
>> + }
>> if (i915_vm_has_cache_coloring(vm)) {
>> /* Expand search to cover neighbouring guard pages (or
>> lack!) */
>> @@ -438,7 +463,7 @@ int i915_gem_evict_vm(struct i915_address_space
>> *vm, struct i915_gem_ww_ctx *ww)
>> * switch otherwise is ineffective.
>> */
>> if (i915_is_ggtt(vm)) {
>> - ret = ggtt_flush(vm->gt);
>> + ret = ggtt_flush(vm);
>> if (ret)
>> return ret;
>> }
>> diff --git a/drivers/gpu/drm/i915/i915_vma.c
>> b/drivers/gpu/drm/i915/i915_vma.c
>> index 703fee6b5f75..726705b10637 100644
>> --- a/drivers/gpu/drm/i915/i915_vma.c
>> +++ b/drivers/gpu/drm/i915/i915_vma.c
>> @@ -1544,6 +1544,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma,
>> struct i915_gem_ww_ctx *ww,
>> u32 align, unsigned int flags)
>> {
>> struct i915_address_space *vm = vma->vm;
>> + struct intel_gt *gt;
>> + struct i915_ggtt *ggtt = i915_vm_to_ggtt(vm);
>> int err;
>> do {
>> @@ -1559,7 +1561,8 @@ static int __i915_ggtt_pin(struct i915_vma *vma,
>> struct i915_gem_ww_ctx *ww,
>> }
>> /* Unlike i915_vma_pin, we don't take no for an answer! */
>> - flush_idle_contexts(vm->gt);
>> + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link)
>> + flush_idle_contexts(gt);
>> if (mutex_lock_interruptible(&vm->mutex) == 0) {
>> /*
>> * We pass NULL ww here, as we don't want to unbind
>> diff --git a/drivers/gpu/drm/i915/selftests/i915_gem.c
>> b/drivers/gpu/drm/i915/selftests/i915_gem.c
>> index e5dd82e7e480..2535b9684bd1 100644
>> --- a/drivers/gpu/drm/i915/selftests/i915_gem.c
>> +++ b/drivers/gpu/drm/i915/selftests/i915_gem.c
>> @@ -127,6 +127,8 @@ static void igt_pm_resume(struct drm_i915_private
>> *i915)
>> */
>> with_intel_runtime_pm(&i915->runtime_pm, wakeref) {
>> i915_ggtt_resume(to_gt(i915)->ggtt);
>> + if (GRAPHICS_VER(i915) >= 8)
>> + setup_private_pat(to_gt(i915));
>> i915_gem_resume(i915);
>> }
>> }
^ permalink raw reply [flat|nested] 14+ messages in thread