Re: [Intel-gfx] [PATCH 05/11] drm/i915: Prepare for multiple gts

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(&gt->irq_lock);

@@ -46,13 +49,18 @@ void intel_gt_init_early(struct intel_gt *gt, struct drm_i915_private *i915) intel_rps_init_early(&gt->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