[PATCH] drm/xe: Remove unused "mmio_ext" code
Summers, Stuart
stuart.summers at intel.com
Thu Jan 9 16:13:45 UTC 2025
On Mon, 2025-01-06 at 15:43 -0800, Matt Roper wrote:
> The "mmio_ext" and 'REG_EXT" code is currently unused on any existing
> platform. Going forward, this also isn't the design we want to use
> for
> any future platforms/features either, so we should just go ahead and
> remove the dead code to avoid confusion.
>
> mmio_ext was originally added in an attempt to hack around the early
> (mis)design of the Xe driver, which used xe_gt as the target for all
> register MMIO access, even those completely unrelated to the GT
> subunit
> of the hardware. With the introduction of commit 34953ee349dd
> ("drm/xe:
> Create dedicated xe_mmio structure") and its follow-up patches, that
> misdesign has been corrected and access to register MMIO regions
> specific to hardware units is now done through xe_mmio structures
> which
> encapsulate an iomap, region size, and some other metadata.
>
> Although all of the registers used by the driver today happen to fall
> within one specific PCI BAR region, and thus re-use a single device-
> wide
> iomap, there's no requirement that this stay true for future
> platforms
> or features. I.e., if a future platform adds a new 'foo' hardware
> unit
> that exists at a different area in the BAR, or even in a completely
> different BAR, then that would be handled by doing a separate iomap
> of
> that unit's register region and wrapping it in its own 'struct
> xe_mmio
> foo_regs' structure. The pointer to the new 'foo_regs' could be
> placed
> within the xe_device, xe_tile, xe_gt, etc., according to where the
> new
> hardware unit falls within the current hardware hierarchy.
>
> This effectively reverts the following commits, although parts of
> these
> commits had already vanished or changed with the earlier xe_mmio
> refactor work:
>
> - commit 399a13323f0d ("drm/xe: add 28-bit address support in struct
> xe_reg")
> - commit fdef72e02e20 ("drm/xe: add a flag to bypass multi-tile
> config
> from MTCFG reg")
> - commit 866b2b176434 ("drm/xe: add MMIO extension support flags")
> - commit ef29b390c734 ("drm/xe: map MMIO BAR according to the num of
> tiles in device desc")
> - commit a4e2f3a299ea ("drm/xe: refactor xe_mmio_probe_tiles to
> support
> MMIO extension")
>
> Cc: Lucas De Marchi <lucas.demarchi at intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Koby Elbaz <kelbaz at habana.ai>
> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
lgtm, would be great to get some feedback from Koby here too.
Reviewed-by: Stuart Summers <stuart.summers at intel.com>
> ---
> drivers/gpu/drm/xe/regs/xe_reg_defs.h | 16 +----------
> drivers/gpu/drm/xe/xe_device_types.h | 11 --------
> drivers/gpu/drm/xe/xe_mmio.c | 39 -------------------------
> --
> drivers/gpu/drm/xe/xe_pci.c | 3 ---
> drivers/gpu/drm/xe/xe_pci_types.h | 2 --
> 5 files changed, 1 insertion(+), 70 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/regs/xe_reg_defs.h
> b/drivers/gpu/drm/xe/regs/xe_reg_defs.h
> index 51fd40ffafcb..0234d1e84139 100644
> --- a/drivers/gpu/drm/xe/regs/xe_reg_defs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_reg_defs.h
> @@ -21,7 +21,7 @@ struct xe_reg {
> union {
> struct {
> /** @addr: address */
> - u32 addr:28;
> + u32 addr:22;
> /**
> * @masked: register is "masked", with upper
> 16bits used
> * to identify the bits that are updated on
> the lower
> @@ -41,10 +41,6 @@ struct xe_reg {
> * @vf: register is accessible from the
> Virtual Function.
> */
> u32 vf:1;
> - /**
> - * @ext: access MMIO extension space for
> current register.
> - */
> - u32 ext:1;
> };
> /** @raw: Raw value with both address and options */
> u32 raw;
> @@ -111,16 +107,6 @@ struct xe_reg_mcr {
> */
> #define XE_REG(r_, ...) ((const struct
> xe_reg)XE_REG_INITIALIZER(r_, ##__VA_ARGS__))
>
> -/**
> - * XE_REG_EXT - Create a struct xe_reg from extension offset and
> additional
> - * flags
> - * @r_: Register extension offset
> - * @...: Additional options like access mode. See struct xe_reg for
> available
> - * options.
> - */
> -#define XE_REG_EXT(r_, ...) \
> - ((const struct xe_reg)XE_REG_INITIALIZER(r_, ##__VA_ARGS__,
> .ext = 1))
> -
> /**
> * XE_REG_MCR - Create a struct xe_reg_mcr from offset and
> additional flags
> * @r_: Register offset
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h
> b/drivers/gpu/drm/xe/xe_device_types.h
> index 8a7b15972413..16ebb2859877 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -186,13 +186,6 @@ struct xe_tile {
> */
> struct xe_mmio mmio;
>
> - /**
> - * @mmio_ext: MMIO-extension info for a tile.
> - *
> - * Each tile has its own additional 256MB (28-bit) MMIO-
> extension space.
> - */
> - struct xe_mmio mmio_ext;
> -
> /** @mem: memory management info for tile */
> struct {
> /**
> @@ -263,8 +256,6 @@ struct xe_device {
> const char *graphics_name;
> /** @info.media_name: media IP name */
> const char *media_name;
> - /** @info.tile_mmio_ext_size: size of MMIO extension
> space, per-tile */
> - u32 tile_mmio_ext_size;
> /** @info.graphics_verx100: graphics IP version */
> u32 graphics_verx100;
> /** @info.media_verx100: media IP version */
> @@ -314,8 +305,6 @@ struct xe_device {
> u8 has_heci_gscfi:1;
> /** @info.has_llc: Device has a shared CPU+GPU last
> level cache */
> u8 has_llc:1;
> - /** @info.has_mmio_ext: Device has extra MMIO address
> range */
> - u8 has_mmio_ext:1;
> /** @info.has_range_tlb_invalidation: Has range based
> TLB invalidations */
> u8 has_range_tlb_invalidation:1;
> /** @info.has_sriov: Supports SR-IOV */
> diff --git a/drivers/gpu/drm/xe/xe_mmio.c
> b/drivers/gpu/drm/xe/xe_mmio.c
> index a48f239cad1c..d321a21aacf0 100644
> --- a/drivers/gpu/drm/xe/xe_mmio.c
> +++ b/drivers/gpu/drm/xe/xe_mmio.c
> @@ -103,50 +103,11 @@ static void mmio_multi_tile_setup(struct
> xe_device *xe, size_t tile_mmio_size)
> }
> }
>
> -/*
> - * 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;
> -
> - if (!xe->info.has_mmio_ext)
> - return;
> -
> - regs = xe->mmio.regs + tile_mmio_size * xe->info.tile_count;
> - for_each_tile(tile, xe, id) {
> - tile->mmio_ext.regs_size = tile_mmio_ext_size;
> - tile->mmio_ext.regs = regs;
> - tile->mmio_ext.tile = tile;
> - 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);
> }
> diff --git a/drivers/gpu/drm/xe/xe_pci.c
> b/drivers/gpu/drm/xe/xe_pci.c
> index 7d146e3e8e21..90fcfafd362f 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -61,7 +61,6 @@ struct xe_device_desc {
> u8 has_heci_gscfi:1;
> u8 has_heci_cscfi:1;
> u8 has_llc:1;
> - u8 has_mmio_ext:1;
> u8 has_sriov:1;
> u8 skip_guc_pc:1;
> u8 skip_mtcfg:1;
> @@ -622,7 +621,6 @@ static int xe_info_init_early(struct xe_device
> *xe,
> xe->info.has_heci_gscfi = desc->has_heci_gscfi;
> xe->info.has_heci_cscfi = desc->has_heci_cscfi;
> xe->info.has_llc = desc->has_llc;
> - xe->info.has_mmio_ext = desc->has_mmio_ext;
> xe->info.has_sriov = desc->has_sriov;
> xe->info.skip_guc_pc = desc->skip_guc_pc;
> xe->info.skip_mtcfg = desc->skip_mtcfg;
> @@ -682,7 +680,6 @@ static int xe_info_init(struct xe_device *xe,
>
> xe->info.graphics_name = graphics_desc->name;
> xe->info.media_name = media_desc ? media_desc->name : "none";
> - xe->info.tile_mmio_ext_size = graphics_desc-
> >tile_mmio_ext_size;
>
> xe->info.dma_mask_size = graphics_desc->dma_mask_size;
> xe->info.vram_flags = graphics_desc->vram_flags;
> diff --git a/drivers/gpu/drm/xe/xe_pci_types.h
> b/drivers/gpu/drm/xe/xe_pci_types.h
> index 79b0f80376a4..873efec5cdee 100644
> --- a/drivers/gpu/drm/xe/xe_pci_types.h
> +++ b/drivers/gpu/drm/xe/xe_pci_types.h
> @@ -20,8 +20,6 @@ struct xe_graphics_desc {
>
> u64 hw_engine_mask; /* hardware engines provided by
> graphics IP */
>
> - u32 tile_mmio_ext_size; /* size of MMIO extension space, per-
> tile */
> -
> u8 max_remote_tiles:2;
>
> u8 has_asid:1;
More information about the Intel-xe
mailing list