Some of our upcoming platforms, including the Xe_HP SDV, support a "multi-tile" design. A multi-tile platform is effectively a platform with multiple GT instances and local memory regions, all behind a single PCI device. From an i915 perspective, this translates to multiple intel_gt structures per drm_i915_private. This series provides the initial refactoring to support multiple independent GTs per card, but further work (especially related to local memory) will be required to fully enable a multi-tile platform.
Note that the presence of multiple GTs is largely transparent to userspace. A multi-tile platform will advertise a larger list of engines to userspace, but the concept of "tile" is not something userspace has to worry about directly. There will be some uapi implications later due to the devices having multiple local memory regions, but that aspect of multi-tile is not covered by this patch series and will show up in future work.
Daniele Ceraolo Spurio (2): drm/i915: split general MMIO setup from per-GT uncore init drm/i915: Initial support for per-tile uncore
Matt Roper (1): drm/i915: Restructure probe to handle multi-tile platforms
Michal Wajdeczko (1): drm/i915/guc: Update CT debug macro for multi-tile
Michał Winiarski (1): drm/i915: Store backpointer to GT in uncore
Paulo Zanoni (3): drm/i915: rework some irq functions to take intel_gt as argument drm/i915/xehp: Determine which tile raised an interrupt drm/i915/xehp: Make IRQ reset and postinstall multi-tile aware
Tvrtko Ursulin (2): drm/i915: Prepare for multiple gts drm/i915/xehpsdv: Initialize multi-tiles
Venkata Sandeep Dhanalakota (1): drm/i915: Release per-gt resources allocated
drivers/gpu/drm/i915/gt/intel_gt.c | 180 +++++++++++++++++- drivers/gpu/drm/i915/gt/intel_gt.h | 11 +- drivers/gpu/drm/i915/gt/intel_gt_pm.c | 9 +- drivers/gpu/drm/i915/gt/intel_gt_types.h | 7 + drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 6 +- drivers/gpu/drm/i915/i915_debugfs.c | 5 +- drivers/gpu/drm/i915/i915_drv.c | 80 ++++++-- drivers/gpu/drm/i915/i915_drv.h | 9 + drivers/gpu/drm/i915/i915_irq.c | 71 ++++--- drivers/gpu/drm/i915/i915_pci.c | 1 + drivers/gpu/drm/i915/i915_reg.h | 4 + drivers/gpu/drm/i915/intel_device_info.h | 1 + drivers/gpu/drm/i915/intel_memory_region.h | 3 + drivers/gpu/drm/i915/intel_uncore.c | 36 ++-- drivers/gpu/drm/i915/intel_uncore.h | 6 +- .../gpu/drm/i915/selftests/mock_gem_device.c | 3 +- drivers/gpu/drm/i915/selftests/mock_uncore.c | 2 +- 17 files changed, 345 insertions(+), 89 deletions(-)
From: Paulo Zanoni paulo.r.zanoni@intel.com
We'll be adding multi-tile support soon; on multi-tile platforms interrupts are per-tile and every tile has the full set of interrupt registers.
In this commit we start passing intel_gt instead of dev_priv for the functions that are related to Xe_HP irq handling. Right now we're still passing tile 0 everywhere, but in later patches we'll start actually passing the correct tile.
Signed-off-by: Paulo Zanoni paulo.r.zanoni@intel.com Co-authored-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Radhakrishna Sripada radhakrishna.sripada@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/i915/i915_irq.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 77680bca46ee..038a9ec563c1 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2772,7 +2772,7 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg) { struct drm_i915_private * const i915 = arg; struct intel_gt *gt = &i915->gt; - void __iomem * const regs = i915->uncore.regs; + void __iomem * const regs = gt->uncore->regs; u32 master_tile_ctl, master_ctl; u32 gu_misc_iir;
@@ -3173,11 +3173,12 @@ static void gen11_display_irq_reset(struct drm_i915_private *dev_priv)
static void gen11_irq_reset(struct drm_i915_private *dev_priv) { - struct intel_uncore *uncore = &dev_priv->uncore; + struct intel_gt *gt = &dev_priv->gt; + struct intel_uncore *uncore = gt->uncore;
gen11_master_intr_disable(dev_priv->uncore.regs);
- gen11_gt_irq_reset(&dev_priv->gt); + gen11_gt_irq_reset(gt); gen11_display_irq_reset(dev_priv);
GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_); @@ -3186,11 +3187,12 @@ static void gen11_irq_reset(struct drm_i915_private *dev_priv)
static void dg1_irq_reset(struct drm_i915_private *dev_priv) { - struct intel_uncore *uncore = &dev_priv->uncore; + struct intel_gt *gt = &dev_priv->gt; + struct intel_uncore *uncore = gt->uncore;
dg1_master_intr_disable(dev_priv->uncore.regs);
- gen11_gt_irq_reset(&dev_priv->gt); + gen11_gt_irq_reset(gt); gen11_display_irq_reset(dev_priv);
GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_); @@ -3869,13 +3871,14 @@ static void gen11_de_irq_postinstall(struct drm_i915_private *dev_priv)
static void gen11_irq_postinstall(struct drm_i915_private *dev_priv) { - struct intel_uncore *uncore = &dev_priv->uncore; + struct intel_gt *gt = &dev_priv->gt; + struct intel_uncore *uncore = gt->uncore; u32 gu_misc_masked = GEN11_GU_MISC_GSE;
if (INTEL_PCH_TYPE(dev_priv) >= PCH_ICP) icp_irq_postinstall(dev_priv);
- gen11_gt_irq_postinstall(&dev_priv->gt); + gen11_gt_irq_postinstall(gt); gen11_de_irq_postinstall(dev_priv);
GEN3_IRQ_INIT(uncore, GEN11_GU_MISC_, ~gu_misc_masked, gu_misc_masked); @@ -3886,10 +3889,11 @@ static void gen11_irq_postinstall(struct drm_i915_private *dev_priv)
static void dg1_irq_postinstall(struct drm_i915_private *dev_priv) { - struct intel_uncore *uncore = &dev_priv->uncore; + struct intel_gt *gt = &dev_priv->gt; + struct intel_uncore *uncore = gt->uncore; u32 gu_misc_masked = GEN11_GU_MISC_GSE;
- gen11_gt_irq_postinstall(&dev_priv->gt); + gen11_gt_irq_postinstall(gt);
GEN3_IRQ_INIT(uncore, GEN11_GU_MISC_, ~gu_misc_masked, gu_misc_masked);
@@ -3900,8 +3904,8 @@ static void dg1_irq_postinstall(struct drm_i915_private *dev_priv) GEN11_DISPLAY_IRQ_ENABLE); }
- dg1_master_intr_enable(dev_priv->uncore.regs); - intel_uncore_posting_read(&dev_priv->uncore, DG1_MSTR_TILE_INTR); + dg1_master_intr_enable(uncore->regs); + intel_uncore_posting_read(uncore, DG1_MSTR_TILE_INTR); }
static void cherryview_irq_postinstall(struct drm_i915_private *dev_priv)
On Fri, Oct 08, 2021 at 02:56:25PM -0700, Matt Roper wrote:
From: Paulo Zanoni paulo.r.zanoni@intel.com
We'll be adding multi-tile support soon; on multi-tile platforms interrupts are per-tile and every tile has the full set of interrupt registers.
In this commit we start passing intel_gt instead of dev_priv for the functions that are related to Xe_HP irq handling. Right now we're still passing tile 0 everywhere, but in later patches we'll start actually passing the correct tile.
Signed-off-by: Paulo Zanoni paulo.r.zanoni@intel.com Co-authored-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Radhakrishna Sripada radhakrishna.sripada@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com
mostly replacing the i915->uncore with the gt->uncore, which right now should be the same. The other changes are just changing
Reviewed-by: Lucas De Marchi lucas.demarchi@intel.com
Lucas De Marchi
Hi Paulo and Matt,
From: Paulo Zanoni paulo.r.zanoni@intel.com
We'll be adding multi-tile support soon; on multi-tile platforms interrupts are per-tile and every tile has the full set of interrupt registers.
In this commit we start passing intel_gt instead of dev_priv for the functions that are related to Xe_HP irq handling. Right now we're still passing tile 0 everywhere, but in later patches we'll start actually passing the correct tile.
Signed-off-by: Paulo Zanoni paulo.r.zanoni@intel.com Co-authored-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Radhakrishna Sripada radhakrishna.sripada@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com
Reviewed-by: Andi Shyti andi.shyti@linux.intel.com
Andi
From: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com
In coming patches we'll be doing the actual tile initialization between these two uncore init phases.
Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 9 ++++++++- drivers/gpu/drm/i915/intel_uncore.c | 17 +++-------------- drivers/gpu/drm/i915/intel_uncore.h | 2 ++ 3 files changed, 13 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index c65c3742887a..7f96d26c012a 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -415,10 +415,14 @@ static int i915_driver_mmio_probe(struct drm_i915_private *dev_priv) if (ret < 0) return ret;
- ret = intel_uncore_init_mmio(&dev_priv->uncore); + ret = intel_uncore_setup_mmio(&dev_priv->uncore); if (ret < 0) goto err_bridge;
+ ret = intel_uncore_init_mmio(&dev_priv->uncore); + if (ret) + goto err_mmio; + /* Try to make sure MCHBAR is enabled before poking at it */ intel_setup_mchbar(dev_priv); intel_device_info_runtime_init(dev_priv); @@ -435,6 +439,8 @@ static int i915_driver_mmio_probe(struct drm_i915_private *dev_priv) err_uncore: intel_teardown_mchbar(dev_priv); intel_uncore_fini_mmio(&dev_priv->uncore); +err_mmio: + intel_uncore_cleanup_mmio(&dev_priv->uncore); err_bridge: pci_dev_put(dev_priv->bridge_dev);
@@ -449,6 +455,7 @@ static void i915_driver_mmio_release(struct drm_i915_private *dev_priv) { intel_teardown_mchbar(dev_priv); intel_uncore_fini_mmio(&dev_priv->uncore); + intel_uncore_cleanup_mmio(&dev_priv->uncore); pci_dev_put(dev_priv->bridge_dev); }
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index e072054adac5..a308e86c9d9f 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -2020,7 +2020,7 @@ static int i915_pmic_bus_access_notifier(struct notifier_block *nb, return NOTIFY_OK; }
-static int uncore_mmio_setup(struct intel_uncore *uncore) +int intel_uncore_setup_mmio(struct intel_uncore *uncore) { struct drm_i915_private *i915 = uncore->i915; struct pci_dev *pdev = to_pci_dev(i915->drm.dev); @@ -2053,7 +2053,7 @@ static int uncore_mmio_setup(struct intel_uncore *uncore) return 0; }
-static void uncore_mmio_cleanup(struct intel_uncore *uncore) +void intel_uncore_cleanup_mmio(struct intel_uncore *uncore) { struct pci_dev *pdev = to_pci_dev(uncore->i915->drm.dev);
@@ -2146,10 +2146,6 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore) struct drm_i915_private *i915 = uncore->i915; int ret;
- ret = uncore_mmio_setup(uncore); - if (ret) - return ret; - /* * The boot firmware initializes local memory and assesses its health. * If memory training fails, the punit will have been instructed to @@ -2170,7 +2166,7 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore) } else { ret = uncore_forcewake_init(uncore); if (ret) - goto out_mmio_cleanup; + return ret; }
/* make sure fw funcs are set if and only if we have fw*/ @@ -2192,11 +2188,6 @@ int intel_uncore_init_mmio(struct intel_uncore *uncore) drm_dbg(&i915->drm, "unclaimed mmio detected on uncore init, clearing\n");
return 0; - -out_mmio_cleanup: - uncore_mmio_cleanup(uncore); - - return ret; }
/* @@ -2261,8 +2252,6 @@ void intel_uncore_fini_mmio(struct intel_uncore *uncore) intel_uncore_fw_domains_fini(uncore); iosf_mbi_punit_release(); } - - uncore_mmio_cleanup(uncore); }
static const struct reg_whitelist { diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h index 3248e4e2c540..d1d17b04e29f 100644 --- a/drivers/gpu/drm/i915/intel_uncore.h +++ b/drivers/gpu/drm/i915/intel_uncore.h @@ -218,11 +218,13 @@ void intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug); void intel_uncore_init_early(struct intel_uncore *uncore, struct drm_i915_private *i915); +int intel_uncore_setup_mmio(struct intel_uncore *uncore); int intel_uncore_init_mmio(struct intel_uncore *uncore); void intel_uncore_prune_engine_fw_domains(struct intel_uncore *uncore, struct intel_gt *gt); bool intel_uncore_unclaimed_mmio(struct intel_uncore *uncore); bool intel_uncore_arm_unclaimed_mmio_detection(struct intel_uncore *uncore); +void intel_uncore_cleanup_mmio(struct intel_uncore *uncore); void intel_uncore_fini_mmio(struct intel_uncore *uncore); void intel_uncore_suspend(struct intel_uncore *uncore); void intel_uncore_resume_early(struct intel_uncore *uncore);
On Fri, Oct 08, 2021 at 02:56:26PM -0700, Matt Roper wrote:
From: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com
In coming patches we'll be doing the actual tile initialization between these two uncore init phases.
Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com
Reviewed-by: Lucas De Marchi lucas.demarchi@intel.com
Lucas De Marchi
Hi Matt and Daniele,
From: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com
In coming patches we'll be doing the actual tile initialization between these two uncore init phases.
Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com
Reviewed-by: Andi Shyti andi.shyti@linux.intel.com
Andi
On a multi-tile platform, each tile has its own registers + GGTT space, and BAR 0 is extended to cover all of them. Upcoming patches will start exposing the tiles as multiple GTs within a single PCI device. In preparation for supporting such setups, restructure the driver's probe code a bit.
Only the primary/root tile is initialized for now; the other tiles will be detected and plugged in by future patches once the necessary infrastructure is in place to handle them.
Original-author: Abdiel Janulgue Cc: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Cc: Matthew Auld matthew.auld@intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/i915/gt/intel_gt.c | 45 ++++++++++++++++++++++++ drivers/gpu/drm/i915/gt/intel_gt.h | 3 ++ drivers/gpu/drm/i915/gt/intel_gt_pm.c | 9 ++++- drivers/gpu/drm/i915/gt/intel_gt_types.h | 5 +++ drivers/gpu/drm/i915/i915_drv.c | 20 +++++------ drivers/gpu/drm/i915/intel_uncore.c | 12 +++---- drivers/gpu/drm/i915/intel_uncore.h | 3 +- 7 files changed, 76 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 1cb1948ac959..f4bea1f1de77 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -900,6 +900,51 @@ u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg) return intel_uncore_read_fw(gt->uncore, reg); }
+static int +tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) +{ + int ret; + + intel_uncore_init_early(gt->uncore, gt->i915); + + ret = intel_uncore_setup_mmio(gt->uncore, phys_addr); + if (ret) + return ret; + + gt->phys_addr = phys_addr; + + return 0; +} + +static void tile_cleanup(struct intel_gt *gt) +{ + intel_uncore_cleanup_mmio(gt->uncore); +} + +int intel_probe_gts(struct drm_i915_private *i915) +{ + struct pci_dev *pdev = to_pci_dev(i915->drm.dev); + phys_addr_t phys_addr; + unsigned int mmio_bar; + int ret; + + mmio_bar = GRAPHICS_VER(i915) == 2 ? 1 : 0; + phys_addr = pci_resource_start(pdev, mmio_bar); + + /* We always have at least one primary GT on any device */ + ret = tile_setup(&i915->gt, 0, phys_addr); + if (ret) + return ret; + + /* TODO: add more tiles */ + return 0; +} + +void intel_gts_release(struct drm_i915_private *i915) +{ + tile_cleanup(&i915->gt); +} + void intel_gt_info_print(const struct intel_gt_info *info, struct drm_printer *p) { diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h index 74e771871a9b..f4f35a70cbe4 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.h +++ b/drivers/gpu/drm/i915/gt/intel_gt.h @@ -85,6 +85,9 @@ static inline bool intel_gt_needs_read_steering(struct intel_gt *gt,
u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg);
+int intel_probe_gts(struct drm_i915_private *i915); +void intel_gts_release(struct drm_i915_private *i915); + void intel_gt_info_print(const struct intel_gt_info *info, struct drm_printer *p);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c index 524eaf678790..76f498edb0d5 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c @@ -126,7 +126,14 @@ static const struct intel_wakeref_ops wf_ops = {
void intel_gt_pm_init_early(struct intel_gt *gt) { - intel_wakeref_init(>->wakeref, gt->uncore->rpm, &wf_ops); + /* + * We access the runtime_pm structure via gt->i915 here rather than + * gt->uncore as we do elsewhere in the file because gt->uncore is not + * yet initialized for all tiles at this point in the driver startup. + * runtime_pm is per-device rather than per-tile, so this is still the + * correct structure. + */ + intel_wakeref_init(>->wakeref, >->i915->runtime_pm, &wf_ops); seqcount_mutex_init(>->stats.lock, >->wakeref.mutex); }
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h index 14216cc471b1..66143316d92e 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h @@ -180,6 +180,11 @@ struct intel_gt {
const struct intel_mmio_range *steering_table[NUM_STEERING_TYPES];
+ /* + * Base of per-tile GTTMMADR where we can derive the MMIO and the GGTT. + */ + phys_addr_t phys_addr; + struct intel_gt_info { intel_engine_mask_t engine_mask;
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 7f96d26c012a..51234fd1349b 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -312,8 +312,8 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv) intel_device_info_subplatform_init(dev_priv); intel_step_init(dev_priv);
+ /* All tiles share a single mmio_debug */ intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug); - intel_uncore_init_early(&dev_priv->uncore, dev_priv);
spin_lock_init(&dev_priv->irq_lock); spin_lock_init(&dev_priv->gpu_error.lock); @@ -415,13 +415,9 @@ static int i915_driver_mmio_probe(struct drm_i915_private *dev_priv) if (ret < 0) return ret;
- ret = intel_uncore_setup_mmio(&dev_priv->uncore); - if (ret < 0) - goto err_bridge; - ret = intel_uncore_init_mmio(&dev_priv->uncore); if (ret) - goto err_mmio; + return ret;
/* Try to make sure MCHBAR is enabled before poking at it */ intel_setup_mchbar(dev_priv); @@ -439,9 +435,6 @@ static int i915_driver_mmio_probe(struct drm_i915_private *dev_priv) err_uncore: intel_teardown_mchbar(dev_priv); intel_uncore_fini_mmio(&dev_priv->uncore); -err_mmio: - intel_uncore_cleanup_mmio(&dev_priv->uncore); -err_bridge: pci_dev_put(dev_priv->bridge_dev);
return ret; @@ -455,7 +448,6 @@ static void i915_driver_mmio_release(struct drm_i915_private *dev_priv) { intel_teardown_mchbar(dev_priv); intel_uncore_fini_mmio(&dev_priv->uncore); - intel_uncore_cleanup_mmio(&dev_priv->uncore); pci_dev_put(dev_priv->bridge_dev); }
@@ -844,10 +836,14 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
intel_vgpu_detect(i915);
- ret = i915_driver_mmio_probe(i915); + ret = intel_probe_gts(i915); if (ret < 0) goto out_runtime_pm_put;
+ ret = i915_driver_mmio_probe(i915); + if (ret < 0) + goto out_tiles_cleanup; + ret = i915_driver_hw_probe(i915); if (ret < 0) goto out_cleanup_mmio; @@ -904,6 +900,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) i915_ggtt_driver_late_release(i915); out_cleanup_mmio: i915_driver_mmio_release(i915); +out_tiles_cleanup: + intel_gts_release(i915); out_runtime_pm_put: enable_rpm_wakeref_asserts(&i915->runtime_pm); i915_driver_late_release(i915); diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index a308e86c9d9f..8a0a0676d67a 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -2020,14 +2020,11 @@ static int i915_pmic_bus_access_notifier(struct notifier_block *nb, return NOTIFY_OK; }
-int intel_uncore_setup_mmio(struct intel_uncore *uncore) +int intel_uncore_setup_mmio(struct intel_uncore *uncore, phys_addr_t phys_addr) { struct drm_i915_private *i915 = uncore->i915; - struct pci_dev *pdev = to_pci_dev(i915->drm.dev); - int mmio_bar; int mmio_size;
- mmio_bar = GRAPHICS_VER(i915) == 2 ? 1 : 0; /* * Before gen4, the registers and the GTT are behind different BARs. * However, from gen4 onwards, the registers and the GTT are shared @@ -2044,7 +2041,7 @@ int intel_uncore_setup_mmio(struct intel_uncore *uncore) else mmio_size = 2 * 1024 * 1024;
- uncore->regs = pci_iomap(pdev, mmio_bar, mmio_size); + uncore->regs = ioremap(phys_addr, mmio_size); if (uncore->regs == NULL) { drm_err(&i915->drm, "failed to map registers\n"); return -EIO; @@ -2055,9 +2052,8 @@ int intel_uncore_setup_mmio(struct intel_uncore *uncore)
void intel_uncore_cleanup_mmio(struct intel_uncore *uncore) { - struct pci_dev *pdev = to_pci_dev(uncore->i915->drm.dev); - - pci_iounmap(pdev, uncore->regs); + if (uncore->regs) + iounmap(uncore->regs); }
void intel_uncore_init_early(struct intel_uncore *uncore, diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h index d1d17b04e29f..83a455aa8374 100644 --- a/drivers/gpu/drm/i915/intel_uncore.h +++ b/drivers/gpu/drm/i915/intel_uncore.h @@ -29,6 +29,7 @@ #include <linux/notifier.h> #include <linux/hrtimer.h> #include <linux/io-64-nonatomic-lo-hi.h> +#include <linux/types.h>
#include "i915_reg.h"
@@ -218,7 +219,7 @@ void intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug); void intel_uncore_init_early(struct intel_uncore *uncore, struct drm_i915_private *i915); -int intel_uncore_setup_mmio(struct intel_uncore *uncore); +int intel_uncore_setup_mmio(struct intel_uncore *uncore, phys_addr_t phys_addr); int intel_uncore_init_mmio(struct intel_uncore *uncore); void intel_uncore_prune_engine_fw_domains(struct intel_uncore *uncore, struct intel_gt *gt);
On Fri, 08 Oct 2021, Matt Roper matthew.d.roper@intel.com wrote:
On a multi-tile platform, each tile has its own registers + GGTT space, and BAR 0 is extended to cover all of them. Upcoming patches will start exposing the tiles as multiple GTs within a single PCI device. In preparation for supporting such setups, restructure the driver's probe code a bit.
Only the primary/root tile is initialized for now; the other tiles will be detected and plugged in by future patches once the necessary infrastructure is in place to handle them.
Original-author: Abdiel Janulgue Cc: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Cc: Matthew Auld matthew.auld@intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com
drivers/gpu/drm/i915/gt/intel_gt.c | 45 ++++++++++++++++++++++++ drivers/gpu/drm/i915/gt/intel_gt.h | 3 ++ drivers/gpu/drm/i915/gt/intel_gt_pm.c | 9 ++++- drivers/gpu/drm/i915/gt/intel_gt_types.h | 5 +++ drivers/gpu/drm/i915/i915_drv.c | 20 +++++------ drivers/gpu/drm/i915/intel_uncore.c | 12 +++---- drivers/gpu/drm/i915/intel_uncore.h | 3 +- 7 files changed, 76 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 1cb1948ac959..f4bea1f1de77 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -900,6 +900,51 @@ u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg) return intel_uncore_read_fw(gt->uncore, reg); }
+static int +tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) +{
- int ret;
- intel_uncore_init_early(gt->uncore, gt->i915);
- ret = intel_uncore_setup_mmio(gt->uncore, phys_addr);
- if (ret)
return ret;- gt->phys_addr = phys_addr;
- return 0;
+}
+static void tile_cleanup(struct intel_gt *gt) +{
- intel_uncore_cleanup_mmio(gt->uncore);
+}
+int intel_probe_gts(struct drm_i915_private *i915) +{
- struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
- phys_addr_t phys_addr;
- unsigned int mmio_bar;
- int ret;
- mmio_bar = GRAPHICS_VER(i915) == 2 ? 1 : 0;
- phys_addr = pci_resource_start(pdev, mmio_bar);
- /* We always have at least one primary GT on any device */
- ret = tile_setup(&i915->gt, 0, phys_addr);
- if (ret)
return ret;- /* TODO: add more tiles */
- return 0;
+}
+void intel_gts_release(struct drm_i915_private *i915) +{
- tile_cleanup(&i915->gt);
+}
Please call the functions intel_gt_*.
BR, Jani.
void intel_gt_info_print(const struct intel_gt_info *info, struct drm_printer *p) { diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h index 74e771871a9b..f4f35a70cbe4 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.h +++ b/drivers/gpu/drm/i915/gt/intel_gt.h @@ -85,6 +85,9 @@ static inline bool intel_gt_needs_read_steering(struct intel_gt *gt,
u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg);
+int intel_probe_gts(struct drm_i915_private *i915); +void intel_gts_release(struct drm_i915_private *i915);
void intel_gt_info_print(const struct intel_gt_info *info, struct drm_printer *p);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c index 524eaf678790..76f498edb0d5 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c @@ -126,7 +126,14 @@ static const struct intel_wakeref_ops wf_ops = {
void intel_gt_pm_init_early(struct intel_gt *gt) {
- intel_wakeref_init(>->wakeref, gt->uncore->rpm, &wf_ops);
- /*
* We access the runtime_pm structure via gt->i915 here rather than* gt->uncore as we do elsewhere in the file because gt->uncore is not* yet initialized for all tiles at this point in the driver startup.* runtime_pm is per-device rather than per-tile, so this is still the* correct structure.*/- intel_wakeref_init(>->wakeref, >->i915->runtime_pm, &wf_ops); seqcount_mutex_init(>->stats.lock, >->wakeref.mutex);
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h index 14216cc471b1..66143316d92e 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h @@ -180,6 +180,11 @@ struct intel_gt {
const struct intel_mmio_range *steering_table[NUM_STEERING_TYPES];
- /*
* Base of per-tile GTTMMADR where we can derive the MMIO and the GGTT.*/- phys_addr_t phys_addr;
- struct intel_gt_info { intel_engine_mask_t engine_mask;
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 7f96d26c012a..51234fd1349b 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -312,8 +312,8 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv) intel_device_info_subplatform_init(dev_priv); intel_step_init(dev_priv);
- /* All tiles share a single mmio_debug */ intel_uncore_mmio_debug_init_early(&dev_priv->mmio_debug);
intel_uncore_init_early(&dev_priv->uncore, dev_priv);
spin_lock_init(&dev_priv->irq_lock); spin_lock_init(&dev_priv->gpu_error.lock);
@@ -415,13 +415,9 @@ static int i915_driver_mmio_probe(struct drm_i915_private *dev_priv) if (ret < 0) return ret;
- ret = intel_uncore_setup_mmio(&dev_priv->uncore);
- if (ret < 0)
goto err_bridge;- ret = intel_uncore_init_mmio(&dev_priv->uncore); if (ret)
goto err_mmio;
return ret;/* Try to make sure MCHBAR is enabled before poking at it */ intel_setup_mchbar(dev_priv);
@@ -439,9 +435,6 @@ static int i915_driver_mmio_probe(struct drm_i915_private *dev_priv) err_uncore: intel_teardown_mchbar(dev_priv); intel_uncore_fini_mmio(&dev_priv->uncore); -err_mmio:
- intel_uncore_cleanup_mmio(&dev_priv->uncore);
-err_bridge: pci_dev_put(dev_priv->bridge_dev);
return ret; @@ -455,7 +448,6 @@ static void i915_driver_mmio_release(struct drm_i915_private *dev_priv) { intel_teardown_mchbar(dev_priv); intel_uncore_fini_mmio(&dev_priv->uncore);
- intel_uncore_cleanup_mmio(&dev_priv->uncore); pci_dev_put(dev_priv->bridge_dev);
}
@@ -844,10 +836,14 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
intel_vgpu_detect(i915);
- ret = i915_driver_mmio_probe(i915);
ret = intel_probe_gts(i915); if (ret < 0) goto out_runtime_pm_put;
ret = i915_driver_mmio_probe(i915);
if (ret < 0)
goto out_tiles_cleanup;ret = i915_driver_hw_probe(i915); if (ret < 0) goto out_cleanup_mmio;
@@ -904,6 +900,8 @@ int i915_driver_probe(struct pci_dev *pdev, const struct pci_device_id *ent) i915_ggtt_driver_late_release(i915); out_cleanup_mmio: i915_driver_mmio_release(i915); +out_tiles_cleanup:
- intel_gts_release(i915);
out_runtime_pm_put: enable_rpm_wakeref_asserts(&i915->runtime_pm); i915_driver_late_release(i915); diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index a308e86c9d9f..8a0a0676d67a 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -2020,14 +2020,11 @@ static int i915_pmic_bus_access_notifier(struct notifier_block *nb, return NOTIFY_OK; }
-int intel_uncore_setup_mmio(struct intel_uncore *uncore) +int intel_uncore_setup_mmio(struct intel_uncore *uncore, phys_addr_t phys_addr) { struct drm_i915_private *i915 = uncore->i915;
struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
int mmio_bar; int mmio_size;
mmio_bar = GRAPHICS_VER(i915) == 2 ? 1 : 0; /*
- Before gen4, the registers and the GTT are behind different BARs.
- However, from gen4 onwards, the registers and the GTT are shared
@@ -2044,7 +2041,7 @@ int intel_uncore_setup_mmio(struct intel_uncore *uncore) else mmio_size = 2 * 1024 * 1024;
- uncore->regs = pci_iomap(pdev, mmio_bar, mmio_size);
- uncore->regs = ioremap(phys_addr, mmio_size); if (uncore->regs == NULL) { drm_err(&i915->drm, "failed to map registers\n"); return -EIO;
@@ -2055,9 +2052,8 @@ int intel_uncore_setup_mmio(struct intel_uncore *uncore)
void intel_uncore_cleanup_mmio(struct intel_uncore *uncore) {
- struct pci_dev *pdev = to_pci_dev(uncore->i915->drm.dev);
- pci_iounmap(pdev, uncore->regs);
- if (uncore->regs)
iounmap(uncore->regs);}
void intel_uncore_init_early(struct intel_uncore *uncore, diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h index d1d17b04e29f..83a455aa8374 100644 --- a/drivers/gpu/drm/i915/intel_uncore.h +++ b/drivers/gpu/drm/i915/intel_uncore.h @@ -29,6 +29,7 @@ #include <linux/notifier.h> #include <linux/hrtimer.h> #include <linux/io-64-nonatomic-lo-hi.h> +#include <linux/types.h>
#include "i915_reg.h"
@@ -218,7 +219,7 @@ void intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug); void intel_uncore_init_early(struct intel_uncore *uncore, struct drm_i915_private *i915); -int intel_uncore_setup_mmio(struct intel_uncore *uncore); +int intel_uncore_setup_mmio(struct intel_uncore *uncore, phys_addr_t phys_addr); int intel_uncore_init_mmio(struct intel_uncore *uncore); void intel_uncore_prune_engine_fw_domains(struct intel_uncore *uncore, struct intel_gt *gt);
On Wed, Oct 13, 2021 at 03:12:55PM +0300, Jani Nikula wrote:
On Fri, 08 Oct 2021, Matt Roper matthew.d.roper@intel.com wrote:
On a multi-tile platform, each tile has its own registers + GGTT space, and BAR 0 is extended to cover all of them. Upcoming patches will start exposing the tiles as multiple GTs within a single PCI device. In preparation for supporting such setups, restructure the driver's probe code a bit.
Only the primary/root tile is initialized for now; the other tiles will be detected and plugged in by future patches once the necessary infrastructure is in place to handle them.
Original-author: Abdiel Janulgue Cc: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Cc: Matthew Auld matthew.auld@intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com
drivers/gpu/drm/i915/gt/intel_gt.c | 45 ++++++++++++++++++++++++ drivers/gpu/drm/i915/gt/intel_gt.h | 3 ++ drivers/gpu/drm/i915/gt/intel_gt_pm.c | 9 ++++- drivers/gpu/drm/i915/gt/intel_gt_types.h | 5 +++ drivers/gpu/drm/i915/i915_drv.c | 20 +++++------ drivers/gpu/drm/i915/intel_uncore.c | 12 +++---- drivers/gpu/drm/i915/intel_uncore.h | 3 +- 7 files changed, 76 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 1cb1948ac959..f4bea1f1de77 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -900,6 +900,51 @@ u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg) return intel_uncore_read_fw(gt->uncore, reg); }
+static int +tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) +{
- int ret;
- intel_uncore_init_early(gt->uncore, gt->i915);
- ret = intel_uncore_setup_mmio(gt->uncore, phys_addr);
- if (ret)
return ret;- gt->phys_addr = phys_addr;
- return 0;
+}
+static void tile_cleanup(struct intel_gt *gt) +{
- intel_uncore_cleanup_mmio(gt->uncore);
+}
+int intel_probe_gts(struct drm_i915_private *i915) +{
- struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
- phys_addr_t phys_addr;
- unsigned int mmio_bar;
- int ret;
- mmio_bar = GRAPHICS_VER(i915) == 2 ? 1 : 0;
- phys_addr = pci_resource_start(pdev, mmio_bar);
- /* We always have at least one primary GT on any device */
- ret = tile_setup(&i915->gt, 0, phys_addr);
- if (ret)
return ret;- /* TODO: add more tiles */
- return 0;
+}
+void intel_gts_release(struct drm_i915_private *i915) +{
- tile_cleanup(&i915->gt);
+}
Please call the functions intel_gt_*.
actually besides the name, the fact that these take i915 as argument seems to suggest they are in the wrong place. Probably this part should remain in i915_drv.c with name i915_setup_gts()?
then we could export tile_setup as intel_gt_setup() or something else (name here is confusing IMO as in some places we have `init(); ...; setup()` and in other this is `setup(); ...; init();` like in patch 1. We already have
- intel_gt_init_early() - intel_gt_init_hw_early() - intel_gt_init_mmio() - intel_gt_init()
given this is just initiliazing mmio for that specific gt (tile), do we actually need a new init/setup entrypoint?
BR, Jani.
void intel_gt_info_print(const struct intel_gt_info *info, struct drm_printer *p) { diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h index 74e771871a9b..f4f35a70cbe4 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.h +++ b/drivers/gpu/drm/i915/gt/intel_gt.h @@ -85,6 +85,9 @@ static inline bool intel_gt_needs_read_steering(struct intel_gt *gt,
u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg);
+int intel_probe_gts(struct drm_i915_private *i915); +void intel_gts_release(struct drm_i915_private *i915);
void intel_gt_info_print(const struct intel_gt_info *info, struct drm_printer *p);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c index 524eaf678790..76f498edb0d5 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c @@ -126,7 +126,14 @@ static const struct intel_wakeref_ops wf_ops = {
void intel_gt_pm_init_early(struct intel_gt *gt) {
- intel_wakeref_init(>->wakeref, gt->uncore->rpm, &wf_ops);
- /*
* We access the runtime_pm structure via gt->i915 here rather than* gt->uncore as we do elsewhere in the file because gt->uncore is not* yet initialized for all tiles at this point in the driver startup.* runtime_pm is per-device rather than per-tile, so this is still the* correct structure.*/- intel_wakeref_init(>->wakeref, >->i915->runtime_pm, &wf_ops); seqcount_mutex_init(>->stats.lock, >->wakeref.mutex);
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h index 14216cc471b1..66143316d92e 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h @@ -180,6 +180,11 @@ struct intel_gt {
const struct intel_mmio_range *steering_table[NUM_STEERING_TYPES];
- /*
* Base of per-tile GTTMMADR where we can derive the MMIO and the GGTT.*/- phys_addr_t phys_addr;
here and in the next patches it doesn't seem like we need to save phys_addr (which should probably be better named as gttmmadr)? Looking at what is coming it seems this will be needed only when initializing ggtt... We could move this to when it's needed or, just to be clear this is indeed desired for a future change, drop a comment in the commit message the address will be needed for the ggtt initialization?
Lucas De Marchi
On Tue, 26 Oct 2021, Lucas De Marchi lucas.demarchi@intel.com wrote:
On Wed, Oct 13, 2021 at 03:12:55PM +0300, Jani Nikula wrote:
On Fri, 08 Oct 2021, Matt Roper matthew.d.roper@intel.com wrote:
On a multi-tile platform, each tile has its own registers + GGTT space, and BAR 0 is extended to cover all of them. Upcoming patches will start exposing the tiles as multiple GTs within a single PCI device. In preparation for supporting such setups, restructure the driver's probe code a bit.
Only the primary/root tile is initialized for now; the other tiles will be detected and plugged in by future patches once the necessary infrastructure is in place to handle them.
Original-author: Abdiel Janulgue Cc: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Cc: Matthew Auld matthew.auld@intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com
drivers/gpu/drm/i915/gt/intel_gt.c | 45 ++++++++++++++++++++++++ drivers/gpu/drm/i915/gt/intel_gt.h | 3 ++ drivers/gpu/drm/i915/gt/intel_gt_pm.c | 9 ++++- drivers/gpu/drm/i915/gt/intel_gt_types.h | 5 +++ drivers/gpu/drm/i915/i915_drv.c | 20 +++++------ drivers/gpu/drm/i915/intel_uncore.c | 12 +++---- drivers/gpu/drm/i915/intel_uncore.h | 3 +- 7 files changed, 76 insertions(+), 21 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 1cb1948ac959..f4bea1f1de77 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -900,6 +900,51 @@ u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg) return intel_uncore_read_fw(gt->uncore, reg); }
+static int +tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) +{
- int ret;
- intel_uncore_init_early(gt->uncore, gt->i915);
- ret = intel_uncore_setup_mmio(gt->uncore, phys_addr);
- if (ret)
return ret;- gt->phys_addr = phys_addr;
- return 0;
+}
+static void tile_cleanup(struct intel_gt *gt) +{
- intel_uncore_cleanup_mmio(gt->uncore);
+}
+int intel_probe_gts(struct drm_i915_private *i915) +{
- struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
- phys_addr_t phys_addr;
- unsigned int mmio_bar;
- int ret;
- mmio_bar = GRAPHICS_VER(i915) == 2 ? 1 : 0;
- phys_addr = pci_resource_start(pdev, mmio_bar);
- /* We always have at least one primary GT on any device */
- ret = tile_setup(&i915->gt, 0, phys_addr);
- if (ret)
return ret;- /* TODO: add more tiles */
- return 0;
+}
+void intel_gts_release(struct drm_i915_private *i915) +{
- tile_cleanup(&i915->gt);
+}
Please call the functions intel_gt_*.
actually besides the name, the fact that these take i915 as argument seems to suggest they are in the wrong place.
As a rule of thumb, anything to do with just display should go under display/, anything to do with just gt should go under gt/, etc. I think we need more separation between the parts of the driver.
Probably this part should remain in i915_drv.c with name i915_setup_gts()?
I dislike the use of plurals like this, because you don't know if it's "a single GTS" or "multiple GT's". I first paused with "what's a GTS". And as we know, new TLAs crop up constantly...
BR, Jani.
then we could export tile_setup as intel_gt_setup() or something else (name here is confusing IMO as in some places we have `init(); ...; setup()` and in other this is `setup(); ...; init();` like in patch 1. We already have
- intel_gt_init_early()
- intel_gt_init_hw_early()
- intel_gt_init_mmio()
- intel_gt_init()
given this is just initiliazing mmio for that specific gt (tile), do we actually need a new init/setup entrypoint?
BR, Jani.
void intel_gt_info_print(const struct intel_gt_info *info, struct drm_printer *p) { diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h index 74e771871a9b..f4f35a70cbe4 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.h +++ b/drivers/gpu/drm/i915/gt/intel_gt.h @@ -85,6 +85,9 @@ static inline bool intel_gt_needs_read_steering(struct intel_gt *gt,
u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg);
+int intel_probe_gts(struct drm_i915_private *i915); +void intel_gts_release(struct drm_i915_private *i915);
void intel_gt_info_print(const struct intel_gt_info *info, struct drm_printer *p);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_pm.c b/drivers/gpu/drm/i915/gt/intel_gt_pm.c index 524eaf678790..76f498edb0d5 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_pm.c +++ b/drivers/gpu/drm/i915/gt/intel_gt_pm.c @@ -126,7 +126,14 @@ static const struct intel_wakeref_ops wf_ops = {
void intel_gt_pm_init_early(struct intel_gt *gt) {
- intel_wakeref_init(>->wakeref, gt->uncore->rpm, &wf_ops);
- /*
* We access the runtime_pm structure via gt->i915 here rather than* gt->uncore as we do elsewhere in the file because gt->uncore is not* yet initialized for all tiles at this point in the driver startup.* runtime_pm is per-device rather than per-tile, so this is still the* correct structure.*/- intel_wakeref_init(>->wakeref, >->i915->runtime_pm, &wf_ops); seqcount_mutex_init(>->stats.lock, >->wakeref.mutex);
}
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h index 14216cc471b1..66143316d92e 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h @@ -180,6 +180,11 @@ struct intel_gt {
const struct intel_mmio_range *steering_table[NUM_STEERING_TYPES];
- /*
* Base of per-tile GTTMMADR where we can derive the MMIO and the GGTT.*/- phys_addr_t phys_addr;
here and in the next patches it doesn't seem like we need to save phys_addr (which should probably be better named as gttmmadr)? Looking at what is coming it seems this will be needed only when initializing ggtt... We could move this to when it's needed or, just to be clear this is indeed desired for a future change, drop a comment in the commit message the address will be needed for the ggtt initialization?
Lucas De Marchi
From: Michał Winiarski michal.winiarski@intel.com
We now support a per-gt uncore, yet we're not able to infer which GT we're operating upon. Let's store a backpointer for now.
Signed-off-by: Michał Winiarski michal.winiarski@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/i915/gt/intel_gt.c | 2 +- drivers/gpu/drm/i915/intel_uncore.c | 9 +++++---- drivers/gpu/drm/i915/intel_uncore.h | 3 ++- drivers/gpu/drm/i915/selftests/mock_gem_device.c | 3 +-- drivers/gpu/drm/i915/selftests/mock_uncore.c | 2 +- 5 files changed, 10 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index f4bea1f1de77..863039d56cba 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -905,7 +905,7 @@ tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) { int ret;
- intel_uncore_init_early(gt->uncore, gt->i915); + intel_uncore_init_early(gt->uncore, gt);
ret = intel_uncore_setup_mmio(gt->uncore, phys_addr); if (ret) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 8a0a0676d67a..2c449836f537 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -2057,12 +2057,13 @@ void intel_uncore_cleanup_mmio(struct intel_uncore *uncore) }
void intel_uncore_init_early(struct intel_uncore *uncore, - struct drm_i915_private *i915) + struct intel_gt *gt) { spin_lock_init(&uncore->lock); - uncore->i915 = i915; - uncore->rpm = &i915->runtime_pm; - uncore->debug = &i915->mmio_debug; + uncore->i915 = gt->i915; + uncore->gt = gt; + uncore->rpm = >->i915->runtime_pm; + uncore->debug = >->i915->mmio_debug; }
static void uncore_raw_init(struct intel_uncore *uncore) diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h index 83a455aa8374..2989032b580b 100644 --- a/drivers/gpu/drm/i915/intel_uncore.h +++ b/drivers/gpu/drm/i915/intel_uncore.h @@ -130,6 +130,7 @@ struct intel_uncore { void __iomem *regs;
struct drm_i915_private *i915; + struct intel_gt *gt; struct intel_runtime_pm *rpm;
spinlock_t lock; /** lock is also taken in irq contexts. */ @@ -218,7 +219,7 @@ u32 intel_uncore_read_with_mcr_steering(struct intel_uncore *uncore, void intel_uncore_mmio_debug_init_early(struct intel_uncore_mmio_debug *mmio_debug); void intel_uncore_init_early(struct intel_uncore *uncore, - struct drm_i915_private *i915); + struct intel_gt *gt); int intel_uncore_setup_mmio(struct intel_uncore *uncore, phys_addr_t phys_addr); int intel_uncore_init_mmio(struct intel_uncore *uncore); void intel_uncore_prune_engine_fw_domains(struct intel_uncore *uncore, diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c index 4f8180146888..bd21bb7d104e 100644 --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c @@ -175,10 +175,9 @@ struct drm_i915_private *mock_gem_device(void) mkwrite_device_info(i915)->memory_regions = REGION_SMEM; intel_memory_regions_hw_probe(i915);
- mock_uncore_init(&i915->uncore, i915); - i915_gem_init__mm(i915); intel_gt_init_early(&i915->gt, i915); + mock_uncore_init(&i915->uncore, i915); atomic_inc(&i915->gt.wakeref.count); /* disable; no hw support */ i915->gt.awake = -ENODEV;
diff --git a/drivers/gpu/drm/i915/selftests/mock_uncore.c b/drivers/gpu/drm/i915/selftests/mock_uncore.c index ca57e4008701..b3790ef137e4 100644 --- a/drivers/gpu/drm/i915/selftests/mock_uncore.c +++ b/drivers/gpu/drm/i915/selftests/mock_uncore.c @@ -42,7 +42,7 @@ __nop_read(64) void mock_uncore_init(struct intel_uncore *uncore, struct drm_i915_private *i915) { - intel_uncore_init_early(uncore, i915); + intel_uncore_init_early(uncore, &i915->gt);
ASSIGN_RAW_WRITE_MMIO_VFUNCS(uncore, nop); ASSIGN_RAW_READ_MMIO_VFUNCS(uncore, nop);
Hi Matt and Michal,
From: Michał Winiarski michal.winiarski@intel.com
We now support a per-gt uncore, yet we're not able to infer which GT we're operating upon. Let's store a backpointer for now.
Signed-off-by: Michał Winiarski michal.winiarski@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com
Reviewed-by: Andi Shyti andi.shyti@linux.intel.com
Andi
From: Tvrtko Ursulin tvrtko.ursulin@intel.com
Add some basic plumbing to support more than one dynamically allocated struct intel_gt. Up to four gts are supported in i915->gts[], with slot zero shadowing the existing i915->gt to enable source compatibility with legacy driver paths. A for_each_gt macro is added to iterate over the GTs and will be used by upcoming patches that convert various parts of the driver to be multi-gt aware.
Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/i915/gt/intel_gt.c | 74 ++++++++++++++++++++-- drivers/gpu/drm/i915/gt/intel_gt.h | 8 ++- drivers/gpu/drm/i915/gt/intel_gt_types.h | 2 + drivers/gpu/drm/i915/i915_drv.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 6 ++ drivers/gpu/drm/i915/intel_memory_region.h | 3 + 6 files changed, 86 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 863039d56cba..736725411f51 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -23,10 +23,13 @@ #include "shmem_utils.h" #include "pxp/intel_pxp.h"
-void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915) +static void +__intel_gt_init_early(struct intel_gt *gt, + struct intel_uncore *uncore, + struct drm_i915_private *i915) { gt->i915 = i915; - gt->uncore = &i915->uncore; + gt->uncore = uncore;
spin_lock_init(>->irq_lock);
@@ -46,13 +49,18 @@ void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915) intel_rps_init_early(>->rps); }
-int intel_gt_probe_lmem(struct intel_gt *gt) +static int intel_gt_probe_lmem(struct intel_gt *gt) { struct drm_i915_private *i915 = gt->i915; + unsigned int instance = gt->info.id; struct intel_memory_region *mem; int id; int err;
+ id = INTEL_REGION_LMEM + instance; + if (drm_WARN_ON(&i915->drm, id >= INTEL_REGION_STOLEN_SMEM)) + return -ENODEV; + mem = intel_gt_setup_lmem(gt); if (mem == ERR_PTR(-ENODEV)) mem = intel_gt_setup_fake_lmem(gt); @@ -67,9 +75,8 @@ int intel_gt_probe_lmem(struct intel_gt *gt) return err; }
- id = INTEL_REGION_LMEM; - mem->id = id; + mem->instance = instance;
intel_memory_region_set_name(mem, "local%u", mem->instance);
@@ -80,6 +87,11 @@ int intel_gt_probe_lmem(struct intel_gt *gt) return 0; }
+void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915) +{ + __intel_gt_init_early(gt, &i915->uncore, i915); +} + void intel_gt_init_hw_early(struct intel_gt *gt, struct i915_ggtt *ggtt) { gt->ggtt = ggtt; @@ -903,9 +915,29 @@ u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg) static int tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) { + struct drm_i915_private *i915 = gt->i915; + struct intel_uncore *uncore; + struct intel_uncore_mmio_debug *mmio_debug; int ret;
- intel_uncore_init_early(gt->uncore, gt); + if (id) { + uncore = kzalloc(sizeof(*uncore), GFP_KERNEL); + if (!uncore) + return -ENOMEM; + + mmio_debug = kzalloc(sizeof(*mmio_debug), GFP_KERNEL); + if (!mmio_debug) { + kfree(uncore); + return -ENOMEM; + } + + __intel_gt_init_early(gt, uncore, i915); + } else { + uncore = &i915->uncore; + mmio_debug = &i915->mmio_debug; + } + + intel_uncore_init_early(uncore, gt);
ret = intel_uncore_setup_mmio(gt->uncore, phys_addr); if (ret) @@ -919,6 +951,11 @@ tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) static void tile_cleanup(struct intel_gt *gt) { intel_uncore_cleanup_mmio(gt->uncore); + + if (gt->info.id) { + kfree(gt->uncore); + kfree(gt); + } }
int intel_probe_gts(struct drm_i915_private *i915) @@ -936,13 +973,36 @@ int intel_probe_gts(struct drm_i915_private *i915) if (ret) return ret;
+ i915->gts[0] = &i915->gt; + /* TODO: add more tiles */ return 0; }
+int intel_gt_tiles_init(struct drm_i915_private *i915) +{ + struct intel_gt *gt; + unsigned int id; + int ret; + + for_each_gt(i915, id, gt) { + ret = intel_gt_probe_lmem(gt); + if (ret) + return ret; + } + + return 0; +} + void intel_gts_release(struct drm_i915_private *i915) { - tile_cleanup(&i915->gt); + struct intel_gt *gt; + unsigned int id; + + for_each_gt(i915, id, gt) { + tile_cleanup(gt); + i915->gts[id] = NULL; + } }
void intel_gt_info_print(const struct intel_gt_info *info, diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h index f4f35a70cbe4..f9dc55adfc4a 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.h +++ b/drivers/gpu/drm/i915/gt/intel_gt.h @@ -36,7 +36,6 @@ static inline struct intel_gt *huc_to_gt(struct intel_huc *huc)
void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915); void intel_gt_init_hw_early(struct intel_gt *gt, struct i915_ggtt *ggtt); -int intel_gt_probe_lmem(struct intel_gt *gt); int intel_gt_init_mmio(struct intel_gt *gt); int __must_check intel_gt_init_hw(struct intel_gt *gt); int intel_gt_init(struct intel_gt *gt); @@ -86,8 +85,15 @@ static inline bool intel_gt_needs_read_steering(struct intel_gt *gt, u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg);
int intel_probe_gts(struct drm_i915_private *i915); +int intel_gt_tiles_init(struct drm_i915_private *i915); void intel_gts_release(struct drm_i915_private *i915);
+#define for_each_gt(i915__, id__, gt__) \ + for ((id__) = 0; \ + (id__) < I915_MAX_TILES; \ + (id__)++) \ + for_each_if (((gt__) = (i915__)->gts[(id__)])) + void intel_gt_info_print(const struct intel_gt_info *info, struct drm_printer *p);
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h index 66143316d92e..7311e485faae 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h @@ -186,6 +186,8 @@ struct intel_gt { phys_addr_t phys_addr;
struct intel_gt_info { + unsigned int id; + intel_engine_mask_t engine_mask;
u32 l3bank_mask; diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 51234fd1349b..44ccf0078ac4 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -578,7 +578,7 @@ static int i915_driver_hw_probe(struct drm_i915_private *dev_priv)
intel_gt_init_hw_early(&dev_priv->gt, &dev_priv->ggtt);
- ret = intel_gt_probe_lmem(&dev_priv->gt); + ret = intel_gt_tiles_init(dev_priv); if (ret) goto err_mem_regions;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 12256218634f..3a26a21ffb3a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1193,6 +1193,12 @@ struct drm_i915_private { /* Abstract the submission mechanism (legacy ringbuffer or execlists) away */ struct intel_gt gt;
+ /* + * i915->gts[0] == &i915->gt + */ +#define I915_MAX_TILES 4 + struct intel_gt *gts[I915_MAX_TILES]; + struct { struct i915_gem_contexts { spinlock_t lock; /* locks list */ diff --git a/drivers/gpu/drm/i915/intel_memory_region.h b/drivers/gpu/drm/i915/intel_memory_region.h index 3feae3353d33..d255e4bffb23 100644 --- a/drivers/gpu/drm/i915/intel_memory_region.h +++ b/drivers/gpu/drm/i915/intel_memory_region.h @@ -31,6 +31,9 @@ enum intel_memory_type { enum intel_region_id { INTEL_REGION_SMEM = 0, INTEL_REGION_LMEM, + INTEL_REGION_LMEM1, + INTEL_REGION_LMEM2, + INTEL_REGION_LMEM3, INTEL_REGION_STOLEN_SMEM, INTEL_REGION_STOLEN_LMEM, INTEL_REGION_UNKNOWN, /* Should be last */
On Fri, Oct 08, 2021 at 02:56:29PM -0700, Matt Roper wrote:
From: Tvrtko Ursulin tvrtko.ursulin@intel.com
Add some basic plumbing to support more than one dynamically allocated struct intel_gt. Up to four gts are supported in i915->gts[], with slot zero shadowing the existing i915->gt to enable source compatibility with legacy driver paths. A for_each_gt macro is added to iterate over the GTs and will be used by upcoming patches that convert various parts of the driver to be multi-gt aware.
Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com
drivers/gpu/drm/i915/gt/intel_gt.c | 74 ++++++++++++++++++++-- drivers/gpu/drm/i915/gt/intel_gt.h | 8 ++- drivers/gpu/drm/i915/gt/intel_gt_types.h | 2 + drivers/gpu/drm/i915/i915_drv.c | 2 +- drivers/gpu/drm/i915/i915_drv.h | 6 ++ drivers/gpu/drm/i915/intel_memory_region.h | 3 + 6 files changed, 86 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 863039d56cba..736725411f51 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -23,10 +23,13 @@ #include "shmem_utils.h" #include "pxp/intel_pxp.h"
-void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915) +static void +__intel_gt_init_early(struct intel_gt *gt,
struct intel_uncore *uncore,struct drm_i915_private *i915){ gt->i915 = i915;
- gt->uncore = &i915->uncore;
gt->uncore = uncore;
spin_lock_init(>->irq_lock);
@@ -46,13 +49,18 @@ void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915) intel_rps_init_early(>->rps); }
-int intel_gt_probe_lmem(struct intel_gt *gt) +static int intel_gt_probe_lmem(struct intel_gt *gt) { struct drm_i915_private *i915 = gt->i915;
unsigned int instance = gt->info.id; struct intel_memory_region *mem; int id; int err;
id = INTEL_REGION_LMEM + instance;
if (drm_WARN_ON(&i915->drm, id >= INTEL_REGION_STOLEN_SMEM))
return -ENODEV;mem = intel_gt_setup_lmem(gt); if (mem == ERR_PTR(-ENODEV)) mem = intel_gt_setup_fake_lmem(gt);
@@ -67,9 +75,8 @@ int intel_gt_probe_lmem(struct intel_gt *gt) return err; }
- id = INTEL_REGION_LMEM;
- mem->id = id;
mem->instance = instance;
intel_memory_region_set_name(mem, "local%u", mem->instance);
@@ -80,6 +87,11 @@ int intel_gt_probe_lmem(struct intel_gt *gt) return 0; }
+void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915) +{
- __intel_gt_init_early(gt, &i915->uncore, i915);
+}
void intel_gt_init_hw_early(struct intel_gt *gt, struct i915_ggtt *ggtt) { gt->ggtt = ggtt; @@ -903,9 +915,29 @@ u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg) static int tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) {
- struct drm_i915_private *i915 = gt->i915;
- struct intel_uncore *uncore;
- struct intel_uncore_mmio_debug *mmio_debug; int ret;
- intel_uncore_init_early(gt->uncore, gt);
if (id) {
uncore = kzalloc(sizeof(*uncore), GFP_KERNEL);if (!uncore)return -ENOMEM;mmio_debug = kzalloc(sizeof(*mmio_debug), GFP_KERNEL);if (!mmio_debug) {kfree(uncore);return -ENOMEM;}__intel_gt_init_early(gt, uncore, i915);} else {
uncore = &i915->uncore;mmio_debug = &i915->mmio_debug;}
intel_uncore_init_early(uncore, gt);
ret = intel_uncore_setup_mmio(gt->uncore, phys_addr); if (ret)
@@ -919,6 +951,11 @@ tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) static void tile_cleanup(struct intel_gt *gt) { intel_uncore_cleanup_mmio(gt->uncore);
- if (gt->info.id) {
kfree(gt->uncore);kfree(gt);- }
}
int intel_probe_gts(struct drm_i915_private *i915) @@ -936,13 +973,36 @@ int intel_probe_gts(struct drm_i915_private *i915) if (ret) return ret;
- i915->gts[0] = &i915->gt;
- /* TODO: add more tiles */ return 0;
}
+int intel_gt_tiles_init(struct drm_i915_private *i915) +{
- struct intel_gt *gt;
- unsigned int id;
- int ret;
- for_each_gt(i915, id, gt) {
ret = intel_gt_probe_lmem(gt);if (ret)return ret;- }
- return 0;
same thing as with previous patches that makes me confused. gt == tile, why do we have such a function in intel_gt.c - intel_gt_tiles_init() that receive the device struct? IMO this belongs to i915_drv.c
+}
void intel_gts_release(struct drm_i915_private *i915) {
- tile_cleanup(&i915->gt);
- struct intel_gt *gt;
- unsigned int id;
- for_each_gt(i915, id, gt) {
tile_cleanup(gt);i915->gts[id] = NULL;
ditto
Lucas De Marchi
From: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com
Initialization and suspend/resume is replicated per-tile.
Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/i915/gt/intel_gt.c | 1 + drivers/gpu/drm/i915/i915_debugfs.c | 5 ++- drivers/gpu/drm/i915/i915_drv.c | 61 ++++++++++++++++++++++------- 3 files changed, 51 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 736725411f51..6528d21e68eb 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -1008,6 +1008,7 @@ void intel_gts_release(struct drm_i915_private *i915) void intel_gt_info_print(const struct intel_gt_info *info, struct drm_printer *p) { + drm_printf(p, "GT %u info:\n", info->id); drm_printf(p, "available engines: %x\n", info->engine_mask);
intel_sseu_dump(&info->sseu, p); diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index fdbd46ff59e0..34fefdfb6661 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -60,12 +60,15 @@ static int i915_capabilities(struct seq_file *m, void *data) { struct drm_i915_private *i915 = node_to_i915(m->private); struct drm_printer p = drm_seq_file_printer(m); + struct intel_gt *gt; + unsigned int id;
seq_printf(m, "pch: %d\n", INTEL_PCH_TYPE(i915));
intel_device_info_print_static(INTEL_INFO(i915), &p); intel_device_info_print_runtime(RUNTIME_INFO(i915), &p); - intel_gt_info_print(&i915->gt.info, &p); + for_each_gt(i915, id, gt) + intel_gt_info_print(>->info, &p); intel_driver_caps_print(&i915->caps, &p);
kernel_param_lock(THIS_MODULE); diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 44ccf0078ac4..36b6e6f2cebf 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -406,6 +406,8 @@ static void i915_driver_late_release(struct drm_i915_private *dev_priv) */ static int i915_driver_mmio_probe(struct drm_i915_private *dev_priv) { + struct intel_gt *gt; + unsigned int i, j; int ret;
if (i915_inject_probe_failure(dev_priv)) @@ -415,26 +417,35 @@ static int i915_driver_mmio_probe(struct drm_i915_private *dev_priv) if (ret < 0) return ret;
- ret = intel_uncore_init_mmio(&dev_priv->uncore); - if (ret) - return ret; + for_each_gt(dev_priv, i, gt) { + ret = intel_uncore_init_mmio(gt->uncore); + if (ret) + goto err_uncore; + }
/* Try to make sure MCHBAR is enabled before poking at it */ intel_setup_mchbar(dev_priv); intel_device_info_runtime_init(dev_priv);
- ret = intel_gt_init_mmio(&dev_priv->gt); - if (ret) - goto err_uncore; + for_each_gt(dev_priv, j, gt) { + ret = intel_gt_init_mmio(gt); + if (ret) + goto err_mchbar; + }
/* As early as possible, scrub existing GPU state before clobbering */ sanitize_gpu(dev_priv);
return 0;
-err_uncore: +err_mchbar: intel_teardown_mchbar(dev_priv); - intel_uncore_fini_mmio(&dev_priv->uncore); +err_uncore: + for_each_gt(dev_priv, j, gt) { + if (j >= i) + break; + intel_uncore_fini_mmio(gt->uncore); + } pci_dev_put(dev_priv->bridge_dev);
return ret; @@ -446,8 +457,12 @@ static int i915_driver_mmio_probe(struct drm_i915_private *dev_priv) */ static void i915_driver_mmio_release(struct drm_i915_private *dev_priv) { + struct intel_gt *gt; + unsigned int i; + intel_teardown_mchbar(dev_priv); - intel_uncore_fini_mmio(&dev_priv->uncore); + for_each_gt(dev_priv, i, gt) + intel_uncore_fini_mmio(gt->uncore); pci_dev_put(dev_priv->bridge_dev); }
@@ -734,6 +749,8 @@ static void i915_welcome_messages(struct drm_i915_private *dev_priv) { if (drm_debug_enabled(DRM_UT_DRIVER)) { struct drm_printer p = drm_debug_printer("i915 device info:"); + struct intel_gt *gt; + unsigned int id;
drm_printf(&p, "pciid=0x%04x rev=0x%02x platform=%s (subplatform=0x%x) gen=%i\n", INTEL_DEVID(dev_priv), @@ -745,7 +762,8 @@ static void i915_welcome_messages(struct drm_i915_private *dev_priv)
intel_device_info_print_static(INTEL_INFO(dev_priv), &p); intel_device_info_print_runtime(RUNTIME_INFO(dev_priv), &p); - intel_gt_info_print(&dev_priv->gt.info, &p); + for_each_gt(dev_priv, id, gt) + intel_gt_info_print(>->info, &p); }
if (IS_ENABLED(CONFIG_DRM_I915_DEBUG)) @@ -1167,13 +1185,16 @@ static int i915_drm_suspend_late(struct drm_device *dev, bool hibernation) struct drm_i915_private *dev_priv = to_i915(dev); struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); struct intel_runtime_pm *rpm = &dev_priv->runtime_pm; + struct intel_gt *gt; + unsigned int i; int ret;
disable_rpm_wakeref_asserts(rpm);
i915_gem_suspend_late(dev_priv);
- intel_uncore_suspend(&dev_priv->uncore); + for_each_gt(dev_priv, i, gt) + intel_uncore_suspend(gt->uncore);
intel_power_domains_suspend(dev_priv, get_suspend_mode(dev_priv, hibernation)); @@ -1302,6 +1323,8 @@ static int i915_drm_resume_early(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); struct pci_dev *pdev = to_pci_dev(dev_priv->drm.dev); + struct intel_gt *gt; + unsigned int i; int ret;
/* @@ -1356,7 +1379,8 @@ static int i915_drm_resume_early(struct drm_device *dev) drm_err(&dev_priv->drm, "Resume prepare failed: %d, continuing anyway\n", ret);
- intel_uncore_resume_early(&dev_priv->uncore); + for_each_gt(dev_priv, i, gt) + intel_uncore_resume_early(gt->uncore);
intel_gt_check_and_clear_faults(&dev_priv->gt);
@@ -1525,6 +1549,8 @@ static int intel_runtime_suspend(struct device *kdev) { struct drm_i915_private *dev_priv = kdev_to_i915(kdev); struct intel_runtime_pm *rpm = &dev_priv->runtime_pm; + struct intel_gt *gt; + unsigned int i; int ret;
if (drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_RUNTIME_PM(dev_priv))) @@ -1544,7 +1570,8 @@ static int intel_runtime_suspend(struct device *kdev)
intel_runtime_pm_disable_interrupts(dev_priv);
- intel_uncore_suspend(&dev_priv->uncore); + for_each_gt(dev_priv, i, gt) + intel_uncore_suspend(gt->uncore);
intel_display_power_suspend(dev_priv);
@@ -1552,7 +1579,8 @@ static int intel_runtime_suspend(struct device *kdev) if (ret) { drm_err(&dev_priv->drm, "Runtime suspend failed, disabling it (%d)\n", ret); - intel_uncore_runtime_resume(&dev_priv->uncore); + for_each_gt(dev_priv, i, gt) + intel_uncore_runtime_resume(gt->uncore);
intel_runtime_pm_enable_interrupts(dev_priv);
@@ -1608,6 +1636,8 @@ static int intel_runtime_resume(struct device *kdev) { struct drm_i915_private *dev_priv = kdev_to_i915(kdev); struct intel_runtime_pm *rpm = &dev_priv->runtime_pm; + struct intel_gt *gt; + unsigned int i; int ret;
if (drm_WARN_ON_ONCE(&dev_priv->drm, !HAS_RUNTIME_PM(dev_priv))) @@ -1628,7 +1658,8 @@ static int intel_runtime_resume(struct device *kdev)
ret = vlv_resume_prepare(dev_priv, true);
- intel_uncore_runtime_resume(&dev_priv->uncore); + for_each_gt(dev_priv, i, gt) + intel_uncore_runtime_resume(gt->uncore);
intel_runtime_pm_enable_interrupts(dev_priv);
Hi Daniele and Matt,
From: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com
Initialization and suspend/resume is replicated per-tile.
Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com
Reviewed-by: Andi Shyti andi.shyti@linux.intel.com
Andi
From: Paulo Zanoni paulo.r.zanoni@intel.com
The first step of interrupt handling is to read a tile0 register that tells us in which tile the interrupt happened; we can then we read the usual interrupt registers from the appropriate tile.
Note that this is just the first step of handling interrupts properly on multi-tile platforms. Subsequent patches will convert other parts of the interrupt handling flow.
Cc: Stuart Summers stuart.summers@intel.com Signed-off-by: Paulo Zanoni paulo.r.zanoni@intel.com Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/i915/i915_irq.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 038a9ec563c1..9f99ad56cde6 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2772,37 +2772,38 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg) { struct drm_i915_private * const i915 = arg; struct intel_gt *gt = &i915->gt; - void __iomem * const regs = gt->uncore->regs; + void __iomem * const t0_regs = gt->uncore->regs; u32 master_tile_ctl, master_ctl; - u32 gu_misc_iir; + u32 gu_misc_iir = 0; + unsigned int i;
if (!intel_irqs_enabled(i915)) return IRQ_NONE;
- master_tile_ctl = dg1_master_intr_disable(regs); + master_tile_ctl = dg1_master_intr_disable(t0_regs); if (!master_tile_ctl) { - dg1_master_intr_enable(regs); + dg1_master_intr_enable(t0_regs); return IRQ_NONE; }
- /* FIXME: we only support tile 0 for now. */ - if (master_tile_ctl & DG1_MSTR_TILE(0)) { + for_each_gt(i915, i, gt) { + void __iomem *const regs = gt->uncore->regs; + + if ((master_tile_ctl & DG1_MSTR_TILE(i)) == 0) + continue; + master_ctl = raw_reg_read(regs, GEN11_GFX_MSTR_IRQ); raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, master_ctl); - } else { - DRM_ERROR("Tile not supported: 0x%08x\n", master_tile_ctl); - dg1_master_intr_enable(regs); - return IRQ_NONE; - }
- gen11_gt_irq_handler(gt, master_ctl); + gen11_gt_irq_handler(gt, master_ctl); + + gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl); + }
if (master_ctl & GEN11_DISPLAY_IRQ) gen11_display_irq_handler(i915);
- gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl); - - dg1_master_intr_enable(regs); + dg1_master_intr_enable(t0_regs);
gen11_gu_misc_irq_handler(gt, gu_misc_iir);
On Fri, Oct 08, 2021 at 02:56:31PM -0700, Matt Roper wrote:
From: Paulo Zanoni paulo.r.zanoni@intel.com
The first step of interrupt handling is to read a tile0 register that tells us in which tile the interrupt happened; we can then we read the usual interrupt registers from the appropriate tile.
Note that this is just the first step of handling interrupts properly on multi-tile platforms. Subsequent patches will convert other parts of the interrupt handling flow.
Cc: Stuart Summers stuart.summers@intel.com Signed-off-by: Paulo Zanoni paulo.r.zanoni@intel.com Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com
drivers/gpu/drm/i915/i915_irq.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 038a9ec563c1..9f99ad56cde6 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2772,37 +2772,38 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg) { struct drm_i915_private * const i915 = arg; struct intel_gt *gt = &i915->gt;
- void __iomem * const regs = gt->uncore->regs;
- void __iomem * const t0_regs = gt->uncore->regs; u32 master_tile_ctl, master_ctl;
- u32 gu_misc_iir;
u32 gu_misc_iir = 0;
unsigned int i;
if (!intel_irqs_enabled(i915)) return IRQ_NONE;
- master_tile_ctl = dg1_master_intr_disable(regs);
- master_tile_ctl = dg1_master_intr_disable(t0_regs); if (!master_tile_ctl) {
dg1_master_intr_enable(regs);
dg1_master_intr_enable(t0_regs);
- /* FIXME: we only support tile 0 for now. */
- if (master_tile_ctl & DG1_MSTR_TILE(0)) {
- for_each_gt(i915, i, gt) {
void __iomem *const regs = gt->uncore->regs;if ((master_tile_ctl & DG1_MSTR_TILE(i)) == 0)continue;- master_ctl = raw_reg_read(regs, GEN11_GFX_MSTR_IRQ); raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, master_ctl);
} else {
DRM_ERROR("Tile not supported: 0x%08x\n", master_tile_ctl);dg1_master_intr_enable(regs);return IRQ_NONE;}
gen11_gt_irq_handler(gt, master_ctl);
gen11_gt_irq_handler(gt, master_ctl);gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl);
Hmm, I missed it before sending the series, but this doesn't look right. We ack every tile's gu_misc_irq separately, but...
}
if (master_ctl & GEN11_DISPLAY_IRQ) gen11_display_irq_handler(i915);
- gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl);
- dg1_master_intr_enable(regs);
dg1_master_intr_enable(t0_regs);
gen11_gu_misc_irq_handler(gt, gu_misc_iir);
...only handle the value from the final tile? Looks like this was intended to move inside the loop as well.
Matt
-- 2.33.0
Hi Matt and Paulo,
The first step of interrupt handling is to read a tile0 register that tells us in which tile the interrupt happened; we can then we read the
^^^^^^^^^^^^^^^^
... we can then read the...
Andi
On Fri, Oct 08, 2021 at 02:56:31PM -0700, Matt Roper wrote:
From: Paulo Zanoni paulo.r.zanoni@intel.com
The first step of interrupt handling is to read a tile0 register that tells us in which tile the interrupt happened; we can then we read the usual interrupt registers from the appropriate tile.
Note that this is just the first step of handling interrupts properly on multi-tile platforms. Subsequent patches will convert other parts of the interrupt handling flow.
Cc: Stuart Summers stuart.summers@intel.com Signed-off-by: Paulo Zanoni paulo.r.zanoni@intel.com Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com
drivers/gpu/drm/i915/i915_irq.c | 31 ++++++++++++++++--------------- 1 file changed, 16 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 038a9ec563c1..9f99ad56cde6 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -2772,37 +2772,38 @@ static irqreturn_t dg1_irq_handler(int irq, void *arg) { struct drm_i915_private * const i915 = arg; struct intel_gt *gt = &i915->gt;
- void __iomem * const regs = gt->uncore->regs;
- void __iomem * const t0_regs = gt->uncore->regs;
given that we later make gt point elsewhere since it's now only used inside the loop, I think this would be clearer with
void __iomem * const t0_regs = i915->gt->uncore->regs; struct intel_gt *gt;
but see below
u32 master_tile_ctl, master_ctl;
- u32 gu_misc_iir;
u32 gu_misc_iir = 0;
unsigned int i;
if (!intel_irqs_enabled(i915)) return IRQ_NONE;
- master_tile_ctl = dg1_master_intr_disable(regs);
- master_tile_ctl = dg1_master_intr_disable(t0_regs); if (!master_tile_ctl) {
dg1_master_intr_enable(regs);
dg1_master_intr_enable(t0_regs);
- /* FIXME: we only support tile 0 for now. */
- if (master_tile_ctl & DG1_MSTR_TILE(0)) {
- for_each_gt(i915, i, gt) {
void __iomem *const regs = gt->uncore->regs;if ((master_tile_ctl & DG1_MSTR_TILE(i)) == 0)continue;- master_ctl = raw_reg_read(regs, GEN11_GFX_MSTR_IRQ); raw_reg_write(regs, GEN11_GFX_MSTR_IRQ, master_ctl);
} else {
DRM_ERROR("Tile not supported: 0x%08x\n", master_tile_ctl);dg1_master_intr_enable(regs);return IRQ_NONE;}
gen11_gt_irq_handler(gt, master_ctl);
gen11_gt_irq_handler(gt, master_ctl);gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl);}
if (master_ctl & GEN11_DISPLAY_IRQ) gen11_display_irq_handler(i915);
- gu_misc_iir = gen11_gu_misc_irq_ack(gt, master_ctl);
- dg1_master_intr_enable(regs);
dg1_master_intr_enable(t0_regs);
gen11_gu_misc_irq_handler(gt, gu_misc_iir);
since we used gt in the for_each_gt() loop it looks like this is not the gt we wanted anymore. Alas gen11_gu_misc_irq_handler() only uses gt to backpoint to i915... so I'm not sure if it should actually be taking a gt as parameter if it is per device rather than per tile/gt.
Lucas De Marchi
-- 2.33.0
From: Paulo Zanoni paulo.r.zanoni@intel.com
Loop through all the tiles when initializing and resetting interrupts.
Signed-off-by: Paulo Zanoni paulo.r.zanoni@intel.com Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/i915/i915_irq.c | 28 ++++++++++++++++++---------- 1 file changed, 18 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 9f99ad56cde6..e788e283d4a8 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -3190,14 +3190,19 @@ static void dg1_irq_reset(struct drm_i915_private *dev_priv) { struct intel_gt *gt = &dev_priv->gt; struct intel_uncore *uncore = gt->uncore; + unsigned int i;
dg1_master_intr_disable(dev_priv->uncore.regs);
- gen11_gt_irq_reset(gt); - gen11_display_irq_reset(dev_priv); + for_each_gt(dev_priv, i, gt) { + gen11_gt_irq_reset(gt);
- GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_); - GEN3_IRQ_RESET(uncore, GEN8_PCU_); + uncore = gt->uncore; + GEN3_IRQ_RESET(uncore, GEN11_GU_MISC_); + GEN3_IRQ_RESET(uncore, GEN8_PCU_); + } + + gen11_display_irq_reset(dev_priv); }
void gen8_irq_power_well_post_enable(struct drm_i915_private *dev_priv, @@ -3890,13 +3895,16 @@ static void gen11_irq_postinstall(struct drm_i915_private *dev_priv)
static void dg1_irq_postinstall(struct drm_i915_private *dev_priv) { - struct intel_gt *gt = &dev_priv->gt; - struct intel_uncore *uncore = gt->uncore; + struct intel_gt *gt; u32 gu_misc_masked = GEN11_GU_MISC_GSE; + unsigned int i;
- gen11_gt_irq_postinstall(gt); + for_each_gt(dev_priv, i, gt) { + gen11_gt_irq_postinstall(gt);
- GEN3_IRQ_INIT(uncore, GEN11_GU_MISC_, ~gu_misc_masked, gu_misc_masked); + GEN3_IRQ_INIT(gt->uncore, GEN11_GU_MISC_, ~gu_misc_masked, + gu_misc_masked); + }
if (HAS_DISPLAY(dev_priv)) { icp_irq_postinstall(dev_priv); @@ -3905,8 +3913,8 @@ static void dg1_irq_postinstall(struct drm_i915_private *dev_priv) GEN11_DISPLAY_IRQ_ENABLE); }
- dg1_master_intr_enable(uncore->regs); - intel_uncore_posting_read(uncore, DG1_MSTR_TILE_INTR); + dg1_master_intr_enable(dev_priv->gt.uncore->regs); + intel_uncore_posting_read(dev_priv->gt.uncore, DG1_MSTR_TILE_INTR); }
static void cherryview_irq_postinstall(struct drm_i915_private *dev_priv)
Hi Paulo and Matt,
[...]
@@ -3190,14 +3190,19 @@ static void dg1_irq_reset(struct drm_i915_private *dev_priv)
mmmhhh... bad naming :/
[...]
- dg1_master_intr_enable(uncore->regs);
- intel_uncore_posting_read(uncore, DG1_MSTR_TILE_INTR);
- dg1_master_intr_enable(dev_priv->gt.uncore->regs);
- intel_uncore_posting_read(dev_priv->gt.uncore, DG1_MSTR_TILE_INTR);
I guess this should also go under a for_each_gt()
Andi
On Thu, Oct 28, 2021 at 06:30:09PM +0200, Andi Shyti wrote:
Hi Paulo and Matt,
[...]
@@ -3190,14 +3190,19 @@ static void dg1_irq_reset(struct drm_i915_private *dev_priv)
mmmhhh... bad naming :/
Even though dg1 wasn't a multi-tile platform, it was the platform that introduced the singleton "master tile interrupt" register that is responsible for telling us which tile(s) had interrupts; we then proceed to read the per-tile master register to find out what those interrupts are. So I think the name is accurate since the hardware introduced the extra level of indirection, and we do need to use this handler on DG1 (we'll just never have more than a single GT to loop over in that case).
[...]
- dg1_master_intr_enable(uncore->regs);
- intel_uncore_posting_read(uncore, DG1_MSTR_TILE_INTR);
- dg1_master_intr_enable(dev_priv->gt.uncore->regs);
- intel_uncore_posting_read(dev_priv->gt.uncore, DG1_MSTR_TILE_INTR);
I guess this should also go under a for_each_gt()
DG1_MSTR_TILE_INTR (0x190008) is the top-level, one-per-PCI device interrupt register; we always access it via tile0's MMIO . So in this case we do want to do this outside the loop since it's not a per-tile operation.
We could probably simplify the dev_priv->gt.uncore parameter to just dev_priv->uncore to make this more obvious.
Matt
Andi
Hi Matt,
- dg1_master_intr_enable(uncore->regs);
- intel_uncore_posting_read(uncore, DG1_MSTR_TILE_INTR);
- dg1_master_intr_enable(dev_priv->gt.uncore->regs);
- intel_uncore_posting_read(dev_priv->gt.uncore, DG1_MSTR_TILE_INTR);
I guess this should also go under a for_each_gt()
DG1_MSTR_TILE_INTR (0x190008) is the top-level, one-per-PCI device interrupt register; we always access it via tile0's MMIO . So in this case we do want to do this outside the loop since it's not a per-tile operation.
yes of course... we are writing to the master interrupt.
We could probably simplify the dev_priv->gt.uncore parameter to just dev_priv->uncore to make this more obvious.
... it would be a nitpick, but nice to have.
Reviewed-by: Andi Shyti andi.shyti@linux.intel.com
Thanks, Andi
From: Michal Wajdeczko michal.wajdeczko@intel.com
Update CT debug macros by including tile ID in all messages.
Cc: Michał Winiarski michal.winiarski@intel.com Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c index 0a3504bc0b61..013ad85cc7f6 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c @@ -33,15 +33,15 @@ static inline struct drm_device *ct_to_drm(struct intel_guc_ct *ct) }
#define CT_ERROR(_ct, _fmt, ...) \ - drm_err(ct_to_drm(_ct), "CT: " _fmt, ##__VA_ARGS__) + drm_err(ct_to_drm(_ct), "CT%u: " _fmt, ct_to_gt(_ct)->info.id, ##__VA_ARGS__) #ifdef CONFIG_DRM_I915_DEBUG_GUC #define CT_DEBUG(_ct, _fmt, ...) \ - drm_dbg(ct_to_drm(_ct), "CT: " _fmt, ##__VA_ARGS__) + drm_dbg(ct_to_drm(_ct), "CT%u: " _fmt, ct_to_gt(_ct)->info.id, ##__VA_ARGS__) #else #define CT_DEBUG(...) do { } while (0) #endif #define CT_PROBE_ERROR(_ct, _fmt, ...) \ - i915_probe_error(ct_to_i915(ct), "CT: " _fmt, ##__VA_ARGS__) + i915_probe_error(ct_to_i915(ct), "CT%u: " _fmt, ct_to_gt(_ct)->info.id, ##__VA_ARGS__)
/** * DOC: CTB Blob
From: Venkata Sandeep Dhanalakota venkata.s.dhanalakota@intel.com
Iterate for_each_gt during release to support multi-tile devices.
Cc: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Venkata Sandeep Dhanalakota venkata.s.dhanalakota@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 36b6e6f2cebf..da574f422084 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -381,10 +381,14 @@ static int i915_driver_early_probe(struct drm_i915_private *dev_priv) */ static void i915_driver_late_release(struct drm_i915_private *dev_priv) { + struct intel_gt *gt; + unsigned int id; + intel_irq_fini(dev_priv); intel_power_domains_cleanup(dev_priv); i915_gem_cleanup_early(dev_priv); - intel_gt_driver_late_release(&dev_priv->gt); + for_each_gt(dev_priv, id, gt) + intel_gt_driver_late_release(gt); intel_region_ttm_device_fini(dev_priv); vlv_suspend_cleanup(dev_priv); i915_workqueues_cleanup(dev_priv);
Hi Matt and Venkata,
On Fri, Oct 08, 2021 at 02:56:34PM -0700, Matt Roper wrote:
From: Venkata Sandeep Dhanalakota venkata.s.dhanalakota@intel.com
Iterate for_each_gt during release to support multi-tile devices.
Cc: Tvrtko Ursulin tvrtko.ursulin@intel.com Signed-off-by: Venkata Sandeep Dhanalakota venkata.s.dhanalakota@intel.com Signed-off-by: Matt Roper matthew.d.roper@intel.com
This patches are quite similar, we could even think of squashing them.
Reviewed-by: Andi Shyti andi.shyti@linux.intel.com
Andi
From: Tvrtko Ursulin tvrtko.ursulin@intel.com
Check how many extra GT tiles are available on the system and setup register access for all of them. We can detect how may GT tiles are available by reading a register on the root tile. The same register returns the tile ID on all tiles.
Bspec: 33407 Original-author: Abdiel Janulgue Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Cc: Matthew Auld matthew.auld@intel.com Cc: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Paulo Zanoni paulo.r.zanoni@intel.com Signed-off-by: Paulo Zanoni paulo.r.zanoni@intel.com Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com --- drivers/gpu/drm/i915/gt/intel_gt.c | 66 +++++++++++++++++++++++- drivers/gpu/drm/i915/i915_drv.h | 3 ++ drivers/gpu/drm/i915/i915_pci.c | 1 + drivers/gpu/drm/i915/i915_reg.h | 4 ++ drivers/gpu/drm/i915/intel_device_info.h | 1 + 5 files changed, 74 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 6528d21e68eb..d7efaef9ade7 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -943,6 +943,17 @@ tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) if (ret) return ret;
+ /* Which tile am I? default to zero on single tile systems */ + if (HAS_REMOTE_TILES(i915)) { + u32 instance = + __raw_uncore_read32(gt->uncore, XEHPSDV_MTCFG_ADDR) & + TILE_NUMBER; + + if (GEM_WARN_ON(instance != id)) + return -ENXIO; + } + + gt->info.id = id; gt->phys_addr = phys_addr;
return 0; @@ -958,11 +969,25 @@ static void tile_cleanup(struct intel_gt *gt) } }
+static unsigned int tile_count(struct drm_i915_private *i915) +{ + u32 mtcfg; + + /* + * We use raw MMIO reads at this point since the + * MMIO vfuncs are not setup yet + */ + mtcfg = __raw_uncore_read32(&i915->uncore, XEHPSDV_MTCFG_ADDR); + return REG_FIELD_GET(TILE_COUNT, mtcfg) + 1; +} + int intel_probe_gts(struct drm_i915_private *i915) { struct pci_dev *pdev = to_pci_dev(i915->drm.dev); + struct intel_gt *gt; phys_addr_t phys_addr; unsigned int mmio_bar; + unsigned int i, tiles; int ret;
mmio_bar = GRAPHICS_VER(i915) == 2 ? 1 : 0; @@ -975,8 +1000,47 @@ int intel_probe_gts(struct drm_i915_private *i915)
i915->gts[0] = &i915->gt;
- /* TODO: add more tiles */ + if (!HAS_REMOTE_TILES(i915)) + return 0; + + /* Setup other tiles */ + tiles = tile_count(i915); + drm_dbg(&i915->drm, "Tile count: %u\n", tiles); + + if (GEM_WARN_ON(tiles > I915_MAX_TILES)) + return -EINVAL; + + /* For multi-tile platforms, size of GTTMMADR is 16MB per tile */ + if (GEM_WARN_ON(pci_resource_len(pdev, 0) / tiles != SZ_16M)) + return -EINVAL; + + for (i = 1; i < tiles; i++) { + gt = kzalloc(sizeof(*gt), GFP_KERNEL); + if (!gt) { + ret = -ENOMEM; + goto err; + } + + ret = tile_setup(gt, i, phys_addr + SZ_16M * i); + if (ret) + goto err; + + i915->gts[i] = gt; + } + + i915->remote_tiles = tiles - 1; + return 0; + +err: + drm_err(&i915->drm, "Failed to initialize tile %u! (%d)\n", i, ret); + + for_each_gt(i915, i, gt) { + tile_cleanup(gt); + i915->gts[i] = NULL; + } + + return ret; }
int intel_gt_tiles_init(struct drm_i915_private *i915) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3a26a21ffb3a..342c42e5aa96 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -865,6 +865,8 @@ struct drm_i915_private { */ resource_size_t stolen_usable_size; /* Total size minus reserved ranges */
+ unsigned int remote_tiles; + struct intel_uncore uncore; struct intel_uncore_mmio_debug mmio_debug;
@@ -1724,6 +1726,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i)) #define HAS_LMEM(i915) HAS_REGION(i915, REGION_LMEM) +#define HAS_REMOTE_TILES(dev_priv) (INTEL_INFO(dev_priv)->has_remote_tiles)
#define HAS_GT_UC(dev_priv) (INTEL_INFO(dev_priv)->has_gt_uc)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 169837de395d..95870c2e366e 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -1015,6 +1015,7 @@ static const struct intel_device_info xehpsdv_info = { DGFX_FEATURES, PLATFORM(INTEL_XEHPSDV), .display = { }, + .has_remote_tiles = 1, .pipe_mask = 0, .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index a897f4abea0c..5d13c19e14aa 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -12477,6 +12477,10 @@ enum skl_power_gate {
#define GEN12_GLOBAL_MOCS(i) _MMIO(0x4000 + (i) * 4) /* Global MOCS regs */
+#define XEHPSDV_MTCFG_ADDR _MMIO(0x101800) +#define TILE_COUNT REG_GENMASK(15, 8) +#define TILE_NUMBER REG_GENMASK(7, 0) + #define GEN12_GSMBASE _MMIO(0x108100) #define GEN12_DSMBASE _MMIO(0x1080C0)
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index 8e6f48d1eb7b..3992b414e21d 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -136,6 +136,7 @@ enum intel_ppgtt_type { func(has_pxp); \ func(has_rc6); \ func(has_rc6p); \ + func(has_remote_tiles); \ func(has_rps); \ func(has_runtime_pm); \ func(has_snoop); \
From: Tvrtko Ursulin tvrtko.ursulin@intel.com
Check how many extra GT tiles are available on the system and setup register access for all of them. We can detect how may GT tiles are available by reading a register on the root tile. The same register returns the tile ID on all tiles.
v2: - Include some additional refactor that didn't get squashed in properly on v1.
Bspec: 33407 Original-author: Abdiel Janulgue Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Cc: Matthew Auld matthew.auld@intel.com Cc: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Paulo Zanoni paulo.r.zanoni@intel.com Cc: Andi Shyti andi.shyti@intel.com Signed-off-by: Paulo Zanoni paulo.r.zanoni@intel.com Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com --- drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- drivers/gpu/drm/i915/gt/intel_gt.c | 83 +++++++++++++++++++++-- drivers/gpu/drm/i915/gt/intel_gt.h | 4 +- drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 + drivers/gpu/drm/i915/i915_drv.h | 7 +- drivers/gpu/drm/i915/i915_pci.c | 40 +++++++++-- drivers/gpu/drm/i915/i915_reg.h | 4 ++ drivers/gpu/drm/i915/intel_device_info.h | 15 ++++ 8 files changed, 145 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 2ae57e4656a3..1d9fcf9572ca 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -525,7 +525,7 @@ static intel_engine_mask_t init_engine_mask(struct intel_gt *gt) u16 vdbox_mask; u16 vebox_mask;
- info->engine_mask = INTEL_INFO(i915)->platform_engine_mask; + GEM_BUG_ON(!info->engine_mask);
if (GRAPHICS_VER(i915) < 11) return info->engine_mask; diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 6528d21e68eb..0879e30ace7c 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -912,14 +912,17 @@ u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg) return intel_uncore_read_fw(gt->uncore, reg); }
-static int -tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) +int intel_tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) { struct drm_i915_private *i915 = gt->i915; struct intel_uncore *uncore; struct intel_uncore_mmio_debug *mmio_debug; int ret;
+ /* For Modern GENs size of GTTMMADR is 16MB (for each tile) */ + if (GEM_WARN_ON(pci_resource_len(to_pci_dev(i915->drm.dev), 0) < (id + 1) * SZ_16M)) + return -EINVAL; + if (id) { uncore = kzalloc(sizeof(*uncore), GFP_KERNEL); if (!uncore) @@ -943,6 +946,16 @@ tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) if (ret) return ret;
+ /* Which tile am I? default to zero on single tile systems */ + if (HAS_REMOTE_TILES(i915)) { + u32 instance = + __raw_uncore_read32(gt->uncore, XEHPSDV_MTCFG_ADDR) & + TILE_NUMBER; + + if (GEM_WARN_ON(instance != id)) + return -ENXIO; + } + gt->phys_addr = phys_addr;
return 0; @@ -958,25 +971,87 @@ static void tile_cleanup(struct intel_gt *gt) } }
+static unsigned int tile_count(struct drm_i915_private *i915) +{ + u32 mtcfg; + + /* + * We use raw MMIO reads at this point since the + * MMIO vfuncs are not setup yet + */ + mtcfg = __raw_uncore_read32(&i915->uncore, XEHPSDV_MTCFG_ADDR); + return REG_FIELD_GET(TILE_COUNT, mtcfg) + 1; +} + int intel_probe_gts(struct drm_i915_private *i915) { struct pci_dev *pdev = to_pci_dev(i915->drm.dev); + const struct intel_gt_definition *gtdef; + struct intel_gt *gt; phys_addr_t phys_addr; unsigned int mmio_bar; + unsigned int i, tiles; int ret;
mmio_bar = GRAPHICS_VER(i915) == 2 ? 1 : 0; phys_addr = pci_resource_start(pdev, mmio_bar);
/* We always have at least one primary GT on any device */ - ret = tile_setup(&i915->gt, 0, phys_addr); + gt = &i915->gt; + gt->name = "Primary GT"; + gt->info.engine_mask = INTEL_INFO(i915)->platform_engine_mask; + + drm_dbg(&i915->drm, "Setting up %s %u\n", gt->name, gt->info.id); + ret = intel_tile_setup(gt, 0, phys_addr); if (ret) return ret;
i915->gts[0] = &i915->gt;
- /* TODO: add more tiles */ + tiles = tile_count(i915); + drm_dbg(&i915->drm, "Tile count: %u\n", tiles); + + for (gtdef = INTEL_INFO(i915)->extra_gts, i = 1; + gtdef && i < tiles; + gtdef++, i++) { + if (GEM_WARN_ON(i >= I915_MAX_GTS)) { + ret = -EINVAL; + goto err; + } + + gt = kzalloc(sizeof(*gt), GFP_KERNEL); + if (!gt) { + ret = -ENOMEM; + goto err; + } + + gt->i915 = i915; + gt->name = gtdef->name; + gt->type = gtdef->type; + gt->info.engine_mask = gtdef->engine_mask; + gt->info.id = i; + + drm_dbg(&i915->drm, "Setting up %s %u\n", gt->name, gt->info.id); + ret = intel_tile_setup(gt, i, phys_addr + gtdef->mapping_base); + if (ret) + goto err; + + i915->gts[i] = gt; + } + + i915->remote_tiles = tiles - 1; + return 0; + +err: + drm_err(&i915->drm, "Failed to initialize %s %u! (%d)\n", gtdef->name, i, ret); + + for_each_gt(i915, i, gt) { + tile_cleanup(gt); + i915->gts[i] = NULL; + } + + return ret; }
int intel_gt_tiles_init(struct drm_i915_private *i915) diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h index f9dc55adfc4a..00d483db8eb1 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.h +++ b/drivers/gpu/drm/i915/gt/intel_gt.h @@ -49,6 +49,8 @@ void intel_gt_driver_late_release(struct intel_gt *gt);
int intel_gt_wait_for_idle(struct intel_gt *gt, long timeout);
+int intel_tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr); + void intel_gt_check_and_clear_faults(struct intel_gt *gt); void intel_gt_clear_error_registers(struct intel_gt *gt, intel_engine_mask_t engine_mask); @@ -90,7 +92,7 @@ void intel_gts_release(struct drm_i915_private *i915);
#define for_each_gt(i915__, id__, gt__) \ for ((id__) = 0; \ - (id__) < I915_MAX_TILES; \ + (id__) < I915_MAX_GTS; \ (id__)++) \ for_each_if (((gt__) = (i915__)->gts[(id__)]))
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h index 7311e485faae..345090e2bafd 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h @@ -68,6 +68,9 @@ enum intel_submission_method {
struct intel_gt { struct drm_i915_private *i915; + const char *name; + enum intel_gt_type type; + struct intel_uncore *uncore; struct i915_ggtt *ggtt;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3a26a21ffb3a..97920340b5e0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -865,6 +865,8 @@ struct drm_i915_private { */ resource_size_t stolen_usable_size; /* Total size minus reserved ranges */
+ unsigned int remote_tiles; + struct intel_uncore uncore; struct intel_uncore_mmio_debug mmio_debug;
@@ -1196,8 +1198,8 @@ struct drm_i915_private { /* * i915->gts[0] == &i915->gt */ -#define I915_MAX_TILES 4 - struct intel_gt *gts[I915_MAX_TILES]; +#define I915_MAX_GTS 4 + struct intel_gt *gts[I915_MAX_GTS];
struct { struct i915_gem_contexts { @@ -1724,6 +1726,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i)) #define HAS_LMEM(i915) HAS_REGION(i915, REGION_LMEM) +#define HAS_REMOTE_TILES(dev_priv) (INTEL_INFO(dev_priv)->has_remote_tiles)
#define HAS_GT_UC(dev_priv) (INTEL_INFO(dev_priv)->has_gt_uc)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 169837de395d..2fd45321318a 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -29,6 +29,7 @@
#include "i915_drv.h" #include "i915_pci.h" +#include "gt/intel_gt.h"
#define PLATFORM(x) .platform = (x) #define GEN(x) \ @@ -1008,6 +1009,37 @@ static const struct intel_device_info adl_p_info = { .media_ver = 12, \ .media_rel = 50
+#define XE_HP_SDV_ENGINES \ + BIT(BCS0) | \ + BIT(VECS0) | BIT(VECS1) | BIT(VECS2) | BIT(VECS3) | \ + BIT(VCS0) | BIT(VCS1) | BIT(VCS2) | BIT(VCS3) | \ + BIT(VCS4) | BIT(VCS5) | BIT(VCS6) | BIT(VCS7) + +static const struct intel_gt_definition xehp_sdv_gts[] = { + { + .type = GT_TILE, + .name = "Remote Tile GT", + .mapping_base = SZ_16M, + .engine_mask = XE_HP_SDV_ENGINES, + + }, + { + .type = GT_TILE, + .name = "Remote Tile GT", + .mapping_base = SZ_16M * 2, + .engine_mask = XE_HP_SDV_ENGINES, + + }, + { + .type = GT_TILE, + .name = "Remote Tile GT", + .mapping_base = SZ_16M * 3, + .engine_mask = XE_HP_SDV_ENGINES, + + }, + {} +}; + __maybe_unused static const struct intel_device_info xehpsdv_info = { XE_HP_FEATURES, @@ -1015,12 +1047,10 @@ static const struct intel_device_info xehpsdv_info = { DGFX_FEATURES, PLATFORM(INTEL_XEHPSDV), .display = { }, + .extra_gts = xehp_sdv_gts, + .has_remote_tiles = 1, .pipe_mask = 0, - .platform_engine_mask = - BIT(RCS0) | BIT(BCS0) | - BIT(VECS0) | BIT(VECS1) | BIT(VECS2) | BIT(VECS3) | - BIT(VCS0) | BIT(VCS1) | BIT(VCS2) | BIT(VCS3) | - BIT(VCS4) | BIT(VCS5) | BIT(VCS6) | BIT(VCS7), + .platform_engine_mask = XE_HP_SDV_ENGINES, .require_force_probe = 1, };
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index a897f4abea0c..5d13c19e14aa 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -12477,6 +12477,10 @@ enum skl_power_gate {
#define GEN12_GLOBAL_MOCS(i) _MMIO(0x4000 + (i) * 4) /* Global MOCS regs */
+#define XEHPSDV_MTCFG_ADDR _MMIO(0x101800) +#define TILE_COUNT REG_GENMASK(15, 8) +#define TILE_NUMBER REG_GENMASK(7, 0) + #define GEN12_GSMBASE _MMIO(0x108100) #define GEN12_DSMBASE _MMIO(0x1080C0)
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index 8e6f48d1eb7b..e0b8758b4085 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -136,6 +136,7 @@ enum intel_ppgtt_type { func(has_pxp); \ func(has_rc6); \ func(has_rc6p); \ + func(has_remote_tiles); \ func(has_rps); \ func(has_runtime_pm); \ func(has_snoop); \ @@ -166,6 +167,18 @@ enum intel_ppgtt_type { func(overlay_needs_physical); \ func(supports_tv);
+enum intel_gt_type { + GT_PRIMARY, + GT_TILE, +}; + +struct intel_gt_definition { + enum intel_gt_type type; + char *name; + u32 mapping_base; + intel_engine_mask_t engine_mask; +}; + struct intel_device_info { u8 graphics_ver; u8 graphics_rel; @@ -185,6 +198,8 @@ struct intel_device_info {
u32 memory_regions; /* regions supported by the HW */
+ const struct intel_gt_definition *extra_gts; + u32 display_mmio_offset;
u8 gt; /* GT number, 0 if undefined */
On 09/10/2021 00:33, Matt Roper wrote:
From: Tvrtko Ursulin tvrtko.ursulin@intel.com
Check how many extra GT tiles are available on the system and setup register access for all of them. We can detect how may GT tiles are available by reading a register on the root tile. The same register returns the tile ID on all tiles.
v2:
- Include some additional refactor that didn't get squashed in properly on v1.
With v2 we should probably add Co-authored-by since, by a quick look, seems half of the patch is no longer mine?
Regards,
Tvrtko
Bspec: 33407 Original-author: Abdiel Janulgue Signed-off-by: Tvrtko Ursulin tvrtko.ursulin@intel.com Cc: Matthew Auld matthew.auld@intel.com Cc: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Paulo Zanoni paulo.r.zanoni@intel.com Cc: Andi Shyti andi.shyti@intel.com Signed-off-by: Paulo Zanoni paulo.r.zanoni@intel.com Signed-off-by: Daniele Ceraolo Spurio daniele.ceraolospurio@intel.com Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 2 +- drivers/gpu/drm/i915/gt/intel_gt.c | 83 +++++++++++++++++++++-- drivers/gpu/drm/i915/gt/intel_gt.h | 4 +- drivers/gpu/drm/i915/gt/intel_gt_types.h | 3 + drivers/gpu/drm/i915/i915_drv.h | 7 +- drivers/gpu/drm/i915/i915_pci.c | 40 +++++++++-- drivers/gpu/drm/i915/i915_reg.h | 4 ++ drivers/gpu/drm/i915/intel_device_info.h | 15 ++++ 8 files changed, 145 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c index 2ae57e4656a3..1d9fcf9572ca 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c @@ -525,7 +525,7 @@ static intel_engine_mask_t init_engine_mask(struct intel_gt *gt) u16 vdbox_mask; u16 vebox_mask;
- info->engine_mask = INTEL_INFO(i915)->platform_engine_mask;
GEM_BUG_ON(!info->engine_mask);
if (GRAPHICS_VER(i915) < 11) return info->engine_mask;
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.c b/drivers/gpu/drm/i915/gt/intel_gt.c index 6528d21e68eb..0879e30ace7c 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.c +++ b/drivers/gpu/drm/i915/gt/intel_gt.c @@ -912,14 +912,17 @@ u32 intel_gt_read_register_fw(struct intel_gt *gt, i915_reg_t reg) return intel_uncore_read_fw(gt->uncore, reg); }
-static int -tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) +int intel_tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) { struct drm_i915_private *i915 = gt->i915; struct intel_uncore *uncore; struct intel_uncore_mmio_debug *mmio_debug; int ret;
- /* For Modern GENs size of GTTMMADR is 16MB (for each tile) */
- if (GEM_WARN_ON(pci_resource_len(to_pci_dev(i915->drm.dev), 0) < (id + 1) * SZ_16M))
return -EINVAL;- if (id) { uncore = kzalloc(sizeof(*uncore), GFP_KERNEL); if (!uncore)
@@ -943,6 +946,16 @@ tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr) if (ret) return ret;
/* Which tile am I? default to zero on single tile systems */
if (HAS_REMOTE_TILES(i915)) {
u32 instance =__raw_uncore_read32(gt->uncore, XEHPSDV_MTCFG_ADDR) &TILE_NUMBER;if (GEM_WARN_ON(instance != id))return -ENXIO;}
gt->phys_addr = phys_addr;
return 0;
@@ -958,25 +971,87 @@ static void tile_cleanup(struct intel_gt *gt) } }
+static unsigned int tile_count(struct drm_i915_private *i915) +{
- u32 mtcfg;
- /*
* We use raw MMIO reads at this point since the* MMIO vfuncs are not setup yet*/- mtcfg = __raw_uncore_read32(&i915->uncore, XEHPSDV_MTCFG_ADDR);
- return REG_FIELD_GET(TILE_COUNT, mtcfg) + 1;
+}
int intel_probe_gts(struct drm_i915_private *i915) { struct pci_dev *pdev = to_pci_dev(i915->drm.dev);
const struct intel_gt_definition *gtdef;
struct intel_gt *gt; phys_addr_t phys_addr; unsigned int mmio_bar;
unsigned int i, tiles; int ret;
mmio_bar = GRAPHICS_VER(i915) == 2 ? 1 : 0; phys_addr = pci_resource_start(pdev, mmio_bar);
/* We always have at least one primary GT on any device */
- ret = tile_setup(&i915->gt, 0, phys_addr);
gt = &i915->gt;
gt->name = "Primary GT";
gt->info.engine_mask = INTEL_INFO(i915)->platform_engine_mask;
drm_dbg(&i915->drm, "Setting up %s %u\n", gt->name, gt->info.id);
ret = intel_tile_setup(gt, 0, phys_addr); if (ret) return ret;
i915->gts[0] = &i915->gt;
- /* TODO: add more tiles */
- tiles = tile_count(i915);
- drm_dbg(&i915->drm, "Tile count: %u\n", tiles);
- for (gtdef = INTEL_INFO(i915)->extra_gts, i = 1;
gtdef && i < tiles;gtdef++, i++) {if (GEM_WARN_ON(i >= I915_MAX_GTS)) {ret = -EINVAL;goto err;}gt = kzalloc(sizeof(*gt), GFP_KERNEL);if (!gt) {ret = -ENOMEM;goto err;}gt->i915 = i915;gt->name = gtdef->name;gt->type = gtdef->type;gt->info.engine_mask = gtdef->engine_mask;gt->info.id = i;drm_dbg(&i915->drm, "Setting up %s %u\n", gt->name, gt->info.id);ret = intel_tile_setup(gt, i, phys_addr + gtdef->mapping_base);if (ret)goto err;i915->gts[i] = gt;- }
- i915->remote_tiles = tiles - 1;
- return 0;
+err:
drm_err(&i915->drm, "Failed to initialize %s %u! (%d)\n", gtdef->name, i, ret);
for_each_gt(i915, i, gt) {
tile_cleanup(gt);i915->gts[i] = NULL;}
return ret; }
int intel_gt_tiles_init(struct drm_i915_private *i915)
diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h index f9dc55adfc4a..00d483db8eb1 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.h +++ b/drivers/gpu/drm/i915/gt/intel_gt.h @@ -49,6 +49,8 @@ void intel_gt_driver_late_release(struct intel_gt *gt);
int intel_gt_wait_for_idle(struct intel_gt *gt, long timeout);
+int intel_tile_setup(struct intel_gt *gt, unsigned int id, phys_addr_t phys_addr);
- void intel_gt_check_and_clear_faults(struct intel_gt *gt); void intel_gt_clear_error_registers(struct intel_gt *gt, intel_engine_mask_t engine_mask);
@@ -90,7 +92,7 @@ void intel_gts_release(struct drm_i915_private *i915);
#define for_each_gt(i915__, id__, gt__) \ for ((id__) = 0; \
(id__) < I915_MAX_TILES; \
(id__) < I915_MAX_GTS; \ (id__)++) \diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h index 7311e485faae..345090e2bafd 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt_types.h +++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h @@ -68,6 +68,9 @@ enum intel_submission_method {
struct intel_gt { struct drm_i915_private *i915;
- const char *name;
- enum intel_gt_type type;
- struct intel_uncore *uncore; struct i915_ggtt *ggtt;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 3a26a21ffb3a..97920340b5e0 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -865,6 +865,8 @@ struct drm_i915_private { */ resource_size_t stolen_usable_size; /* Total size minus reserved ranges */
- unsigned int remote_tiles;
- struct intel_uncore uncore; struct intel_uncore_mmio_debug mmio_debug;
@@ -1196,8 +1198,8 @@ struct drm_i915_private { /* * i915->gts[0] == &i915->gt */ -#define I915_MAX_TILES 4
- struct intel_gt *gts[I915_MAX_TILES];
+#define I915_MAX_GTS 4
struct intel_gt *gts[I915_MAX_GTS];
struct { struct i915_gem_contexts {
@@ -1724,6 +1726,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i)) #define HAS_LMEM(i915) HAS_REGION(i915, REGION_LMEM) +#define HAS_REMOTE_TILES(dev_priv) (INTEL_INFO(dev_priv)->has_remote_tiles)
#define HAS_GT_UC(dev_priv) (INTEL_INFO(dev_priv)->has_gt_uc)
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c index 169837de395d..2fd45321318a 100644 --- a/drivers/gpu/drm/i915/i915_pci.c +++ b/drivers/gpu/drm/i915/i915_pci.c @@ -29,6 +29,7 @@
#include "i915_drv.h" #include "i915_pci.h" +#include "gt/intel_gt.h"
#define PLATFORM(x) .platform = (x) #define GEN(x) \ @@ -1008,6 +1009,37 @@ static const struct intel_device_info adl_p_info = { .media_ver = 12, \ .media_rel = 50
+#define XE_HP_SDV_ENGINES \
- BIT(BCS0) | \
- BIT(VECS0) | BIT(VECS1) | BIT(VECS2) | BIT(VECS3) | \
- BIT(VCS0) | BIT(VCS1) | BIT(VCS2) | BIT(VCS3) | \
- BIT(VCS4) | BIT(VCS5) | BIT(VCS6) | BIT(VCS7)
+static const struct intel_gt_definition xehp_sdv_gts[] = {
- {
.type = GT_TILE,.name = "Remote Tile GT",.mapping_base = SZ_16M,.engine_mask = XE_HP_SDV_ENGINES,- },
- {
.type = GT_TILE,.name = "Remote Tile GT",.mapping_base = SZ_16M * 2,.engine_mask = XE_HP_SDV_ENGINES,- },
- {
.type = GT_TILE,.name = "Remote Tile GT",.mapping_base = SZ_16M * 3,.engine_mask = XE_HP_SDV_ENGINES,- },
- {}
+};
- __maybe_unused static const struct intel_device_info xehpsdv_info = { XE_HP_FEATURES,
@@ -1015,12 +1047,10 @@ static const struct intel_device_info xehpsdv_info = { DGFX_FEATURES, PLATFORM(INTEL_XEHPSDV), .display = { },
- .extra_gts = xehp_sdv_gts,
- .has_remote_tiles = 1, .pipe_mask = 0,
- .platform_engine_mask =
BIT(RCS0) | BIT(BCS0) |BIT(VECS0) | BIT(VECS1) | BIT(VECS2) | BIT(VECS3) |BIT(VCS0) | BIT(VCS1) | BIT(VCS2) | BIT(VCS3) |BIT(VCS4) | BIT(VCS5) | BIT(VCS6) | BIT(VCS7),
- .platform_engine_mask = XE_HP_SDV_ENGINES, .require_force_probe = 1, };
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index a897f4abea0c..5d13c19e14aa 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -12477,6 +12477,10 @@ enum skl_power_gate {
#define GEN12_GLOBAL_MOCS(i) _MMIO(0x4000 + (i) * 4) /* Global MOCS regs */
+#define XEHPSDV_MTCFG_ADDR _MMIO(0x101800) +#define TILE_COUNT REG_GENMASK(15, 8) +#define TILE_NUMBER REG_GENMASK(7, 0)
- #define GEN12_GSMBASE _MMIO(0x108100) #define GEN12_DSMBASE _MMIO(0x1080C0)
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h index 8e6f48d1eb7b..e0b8758b4085 100644 --- a/drivers/gpu/drm/i915/intel_device_info.h +++ b/drivers/gpu/drm/i915/intel_device_info.h @@ -136,6 +136,7 @@ enum intel_ppgtt_type { func(has_pxp); \ func(has_rc6); \ func(has_rc6p); \
- func(has_remote_tiles); \ func(has_rps); \ func(has_runtime_pm); \ func(has_snoop); \
@@ -166,6 +167,18 @@ enum intel_ppgtt_type { func(overlay_needs_physical); \ func(supports_tv);
+enum intel_gt_type {
- GT_PRIMARY,
- GT_TILE,
+};
+struct intel_gt_definition {
- enum intel_gt_type type;
- char *name;
- u32 mapping_base;
- intel_engine_mask_t engine_mask;
+};
- struct intel_device_info { u8 graphics_ver; u8 graphics_rel;
@@ -185,6 +198,8 @@ struct intel_device_info {
u32 memory_regions; /* regions supported by the HW */
const struct intel_gt_definition *extra_gts;
u32 display_mmio_offset;
u8 gt; /* GT number, 0 if undefined */
Hi Tvrtko and Matt,
[...]
-#define I915_MAX_TILES 4
- struct intel_gt *gts[I915_MAX_TILES];
+#define I915_MAX_GTS 4
- struct intel_gt *gts[I915_MAX_GTS];
let's call it MAX_GTS already in patch 5 so that we can avoid a rename.
BTW, out of the scope of this patch but if we can read the number of tiles, why don't we make this dynamic? We already have a "dynamic" version for_each_gt() in probe_gts().
[...]
struct { struct i915_gem_contexts { @@ -1724,6 +1726,7 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define HAS_REGION(i915, i) (INTEL_INFO(i915)->memory_regions & (i)) #define HAS_LMEM(i915) HAS_REGION(i915, REGION_LMEM) +#define HAS_REMOTE_TILES(dev_priv) (INTEL_INFO(dev_priv)->has_remote_tiles)
s/dev_priv/i915
[...]
+static const struct intel_gt_definition xehp_sdv_gts[] = {
- {
.type = GT_TILE,.name = "Remote Tile GT",.mapping_base = SZ_16M,.engine_mask = XE_HP_SDV_ENGINES,- },
- {
.type = GT_TILE,.name = "Remote Tile GT",.mapping_base = SZ_16M * 2,.engine_mask = XE_HP_SDV_ENGINES,- },
- {
.type = GT_TILE,.name = "Remote Tile GT",
why don't we call it "Remote Tile GT <N>" or similar?
[...]
Andi
dri-devel@lists.freedesktop.org
-
Andi Shyti -
Andi Shyti -
Jani Nikula -
Lucas De Marchi -
Matt Roper -
Tvrtko Ursulin