[PATCH] drm/xe: Refactor mmio setup for multi-tile
Lucas De Marchi
lucas.demarchi at intel.com
Tue Jul 23 04:49:18 UTC 2024
On Mon, Jul 22, 2024 at 08:23:06PM GMT, Gustavo Sousa wrote:
>Quoting Lucas De Marchi (2024-07-16 16:54:28-03:00)
>>Extract functions to setup the multi-tile mmio space and extension
>>space, while better documenting the final memory layout. No change in
>>behavior.
>>
>>Signed-off-by: Lucas De Marchi <lucas.demarchi at intel.com>
>>---
>>
>>tested only on single-tile platforms for now. It shouldn't change the
>>behavior, but it would be good to get some extra testing.
>>
>> drivers/gpu/drm/xe/xe_mmio.c | 99 ++++++++++++++++++++++++++++--------
>> 1 file changed, 77 insertions(+), 22 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
>>index ea3c37d3e13f..31f6703daec2 100644
>>--- a/drivers/gpu/drm/xe/xe_mmio.c
>>+++ b/drivers/gpu/drm/xe/xe_mmio.c
>>@@ -33,29 +33,56 @@ static void tiles_fini(void *arg)
>> tile->mmio.regs = NULL;
>> }
>>
>>-int xe_mmio_probe_tiles(struct xe_device *xe)
>>+/*
>>+ * On multi-tile devices, partition the BAR space for MMIO on each tile,
>>+ * possibly accounting for register override on the number of tiles available.
>>+ * Resulting memory layout is like below:
>>+ *
>>+ * .----------------------. <- tile_count * tile_mmio_size
>>+ * | .... |
>>+ * |----------------------| <- 2 * tile_mmio_size
>>+ * | tile1->mmio.regs |
>>+ * |----------------------| <- 1 * tile_mmio_size
>>+ * | tile0->mmio.regs |
>>+ * '----------------------' <- 0MB
>>+ */
>>+static void mmio_multi_tile_setup(struct xe_device *xe, size_t tile_mmio_size)
>> {
>>- size_t tile_mmio_size = SZ_16M, tile_mmio_ext_size = xe->info.tile_mmio_ext_size;
>>- u8 id, tile_count = xe->info.tile_count;
>>- struct xe_gt *gt = xe_root_mmio_gt(xe);
>> struct xe_tile *tile;
>> void __iomem *regs;
>>- u32 mtcfg;
>>+ u8 id;
>>
>>- if (tile_count == 1)
>>- goto add_mmio_ext;
>>+ /*
>>+ * Nothing to be done as tile 0 has already been setup earlier with the
>>+ * entire BAR mapped - see xe_mmio_init()
>>+ */
>>+ if (xe->info.tile_count == 1)
>>+ return;
>>
>>+ /* Possibly override number of tile based on configuration register */
>> if (!xe->info.skip_mtcfg) {
>>+ struct xe_gt *gt = xe_root_mmio_gt(xe);
>>+ u8 tile_count = xe->info.tile_count;
>
>Useless assignment.
>
>>+ u32 mtcfg;
>>+
>>+ /*
>>+ * Although the per-tile mmio regs are not yet initialized, this
>>+ * is fine as it's going to the root gt, that's guaranteed to be
>>+ * initialized earlier in xe_mmio_init()
>>+ */
>> mtcfg = xe_mmio_read64_2x32(gt, XEHP_MTCFG_ADDR);
>> tile_count = REG_FIELD_GET(TILE_COUNT, mtcfg) + 1;
>>+
>> if (tile_count < xe->info.tile_count) {
>> drm_info(&xe->drm, "tile_count: %d, reduced_tile_count %d\n",
>> xe->info.tile_count, tile_count);
>> xe->info.tile_count = tile_count;
>>
>> /*
>>- * FIXME: Needs some work for standalone media, but should be impossible
>>- * with multi-tile for now.
>>+ * FIXME: Needs some work for standalone media, but
>>+ * should be impossible with multi-tile for now:
>>+ * multi-tile platform with standalone media doesn't
>>+ * exist
>> */
>> xe->info.gt_count = xe->info.tile_count;
>> }
>
>Unrelated to this patch, but I suspect this is an odd place to probe the
>hardware for title count. We should probably move this logic to a more
>suitable place in the future?
yep, I agree.
Actually the separation of the mmio multi-tile setup may not need to be
that late in the init sequence.
maybe the override of xe->info.tile_count and xe->info.gt_count
could be moved to drivers/gpu/drm/xe/xe_device.c:update_device_info(),
just after mmio is initialized. But maybe moving them even
earlier. Once upon a time we had xe_info_init_early() and xe_info_init()
where these things should be happening. Note that SR-IOV breaks the
sequence in a not-so-obvious way since it needs more of the driver
initialized for a simple mmio - see how a read is redirected to
xe_gt_sriov_vf_read32()
>
>With the removal of the useless assignment to tile_count,
>
> Reviewed-by: Gustavo Sousa <gustavo.sousa at intel.com>
thanks
Lucas De Marchi
>
>>@@ -67,23 +94,51 @@ int xe_mmio_probe_tiles(struct xe_device *xe)
>> tile->mmio.regs = regs;
>> regs += tile_mmio_size;
>> }
>>+}
>>
>>-add_mmio_ext:
>>- /*
>>- * By design, there's a contiguous multi-tile MMIO space (16MB hard coded per tile).
>>- * When supported, there could be an additional contiguous multi-tile MMIO extension
>>- * space ON TOP of it, and hence the necessity for distinguished MMIO spaces.
>>- */
>>- if (xe->info.has_mmio_ext) {
>>- regs = xe->mmio.regs + tile_mmio_size * tile_count;
>>+/*
>>+ * On top of all the multi-tile MMIO space there can be a platform-dependent
>>+ * extension for each tile, resulting in a layout like below:
>>+ *
>>+ * .----------------------. <- ext_base + tile_count * tile_mmio_ext_size
>>+ * | .... |
>>+ * |----------------------| <- ext_base + 2 * tile_mmio_ext_size
>>+ * | tile1->mmio_ext.regs |
>>+ * |----------------------| <- ext_base + 1 * tile_mmio_ext_size
>>+ * | tile0->mmio_ext.regs |
>>+ * |======================| <- ext_base = tile_count * tile_mmio_size
>>+ * | |
>>+ * | mmio.regs |
>>+ * | |
>>+ * '----------------------' <- 0MB
>>+ *
>>+ * Set up the tile[]->mmio_ext pointers/sizes.
>>+ */
>>+static void mmio_extension_setup(struct xe_device *xe, size_t tile_mmio_size,
>>+ size_t tile_mmio_ext_size)
>>+{
>>+ struct xe_tile *tile;
>>+ void __iomem *regs;
>>+ u8 id;
>>
>>- for_each_tile(tile, xe, id) {
>>- tile->mmio_ext.size = tile_mmio_ext_size;
>>- tile->mmio_ext.regs = regs;
>>+ if (!xe->info.has_mmio_ext)
>>+ return;
>>
>>- regs += tile_mmio_ext_size;
>>- }
>>+ regs = xe->mmio.regs + tile_mmio_size * xe->info.tile_count;
>>+ for_each_tile(tile, xe, id) {
>>+ tile->mmio_ext.size = tile_mmio_ext_size;
>>+ tile->mmio_ext.regs = regs;
>>+ regs += tile_mmio_ext_size;
>> }
>>+}
>>+
>>+int xe_mmio_probe_tiles(struct xe_device *xe)
>>+{
>>+ size_t tile_mmio_size = SZ_16M;
>>+ size_t tile_mmio_ext_size = xe->info.tile_mmio_ext_size;
>>+
>>+ mmio_multi_tile_setup(xe, tile_mmio_size);
>>+ mmio_extension_setup(xe, tile_mmio_size, tile_mmio_ext_size);
>>
>> return devm_add_action_or_reset(xe->drm.dev, tiles_fini, xe);
>> }
>>--
>>2.43.0
>>
More information about the Intel-xe
mailing list