[Intel-xe] [PATCH 05/26] drm/xe: Move register MMIO into xe_tile

Lucas De Marchi lucas.demarchi at intel.com
Thu May 11 22:01:55 UTC 2023


On Thu, May 11, 2023 at 03:20:35PM +0300, Jani Nikula wrote:
>On Wed, 10 May 2023, Matt Roper <matthew.d.roper at intel.com> wrote:
>> Each tile has its own register region in the BAR, containing instances
>> of all registers for the platform.  In contrast, the multiple GTs within
>> a tile share the same MMIO space; there's just a small subset of
>> registers (the GSI registers) which have multiple copies at different
>> offsets (0x0 for primary GT, 0x380000 for media GT).  Move the register
>> MMIO region size/pointers to the tile structure, leaving just the GSI
>> offset information in the GT structure.
>>
>> Signed-off-by: Matt Roper <matthew.d.roper at intel.com>
>> ---
>>  drivers/gpu/drm/xe/display/ext/i915_irq.c |  2 +-
>>  drivers/gpu/drm/xe/xe_device_types.h      | 16 ++++++++++++++
>>  drivers/gpu/drm/xe/xe_ggtt.c              |  3 ++-
>>  drivers/gpu/drm/xe/xe_gt_types.h          |  9 +++-----
>>  drivers/gpu/drm/xe/xe_mmio.c              | 26 ++++++++++++-----------
>>  drivers/gpu/drm/xe/xe_mmio.h              | 21 +++++++++++++-----
>>  6 files changed, 52 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xe/display/ext/i915_irq.c b/drivers/gpu/drm/xe/display/ext/i915_irq.c
>> index afde97b6faa6..a9cbd7b59360 100644
>> --- a/drivers/gpu/drm/xe/display/ext/i915_irq.c
>> +++ b/drivers/gpu/drm/xe/display/ext/i915_irq.c
>> @@ -920,7 +920,7 @@ gen8_de_irq_handler(struct drm_i915_private *dev_priv, u32 master_ctl)
>>
>>  void gen11_display_irq_handler(struct drm_i915_private *i915)
>>  {
>> -	void __iomem * const regs = to_gt(i915)->mmio.regs;
>> +	void __iomem * const regs = xe_device_get_root_tile(i915)->mmio.regs;
>
>Side note, I'm hoping to merge [1] into i915, backport (or rebase) that
>into xe, nuking ext/i915_irq.c completely.
>
>IDK if that means adding new ifdefs in the display irq file, or how we
>could abstract this in a way that doesn't require changes in i915
>display code.
>
>BR,
>Jani.
>
>
>[1] https://patchwork.freedesktop.org/series/117344/

looking at drivers/gpu/drm/i915/display/intel_display_irq.h, it seems
that would be the interface to xe, so we'd go:

xe_irq.c -> xe_display.c -> i915-display,

with xe_display.c probably having an ops style. On init:

	display_irq->enable = ...
	display_irq->disable = ...
	display_irq->reset = ...
	display_irq->handler = ...


so on xe_display.c we'd have:


void xe_display_irq_handler(struct xe_device *xe, u32 master_ctl)
{
         if (!xe->info.enable_display)
                 return;

         if (master_ctl & DISPLAY_IRQ)
                 xe->display_irq
}

... and so on


Or even keeping the same xe_display.c we have today, but pointing the
functions to the right place.

Lucas De Marchi

>
>
>
>>  	const u32 disp_ctl = raw_reg_read(regs, GEN11_DISPLAY_INT_CTL);
>>
>>  	/*
>> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
>> index 5dcf1695925f..2481b2045284 100644
>> --- a/drivers/gpu/drm/xe/xe_device_types.h
>> +++ b/drivers/gpu/drm/xe/xe_device_types.h
>> @@ -80,6 +80,22 @@ struct xe_tile {
>>  	struct xe_gt primary_gt;
>>
>>  	/* TODO: Add media GT here */
>> +
>> +	/**
>> +	 * @mmio: MMIO info for a tile.
>> +	 *
>> +	 * Each tile has its own 16MB space in BAR0, laid out as:
>> +	 * * 0-4MB: registers
>> +	 * * 4MB-8MB: reserved
>> +	 * * 8MB-16MB: global GTT
>> +	 */
>> +	struct {
>> +		/** @size: size of tile's MMIO space */
>> +		size_t size;
>> +
>> +		/** @regs: pointer to tile's MMIO space (starting with registers) */
>> +		void *regs;
>> +	} mmio;
>>  };
>>
>>  /**
>> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
>> index 546240261e0a..200976da3dc1 100644
>> --- a/drivers/gpu/drm/xe/xe_ggtt.c
>> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
>> @@ -93,6 +93,7 @@ static void ggtt_fini_noalloc(struct drm_device *drm, void *arg)
>>  int xe_ggtt_init_noalloc(struct xe_gt *gt, struct xe_ggtt *ggtt)
>>  {
>>  	struct xe_device *xe = gt_to_xe(gt);
>> +	struct xe_tile *tile = gt_to_tile(gt);
>>  	struct pci_dev *pdev = to_pci_dev(xe->drm.dev);
>>  	unsigned int gsm_size;
>>
>> @@ -106,7 +107,7 @@ int xe_ggtt_init_noalloc(struct xe_gt *gt, struct xe_ggtt *ggtt)
>>  		return -ENOMEM;
>>  	}
>>
>> -	ggtt->gsm = gt->mmio.regs + SZ_8M;
>> +	ggtt->gsm = tile->mmio.regs + SZ_8M;
>>  	ggtt->size = (gsm_size / 8) * (u64) XE_PAGE_SIZE;
>>
>>  	if (IS_DGFX(xe) && xe->info.vram_flags & XE_VRAM_FLAGS_NEED64K)
>> diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
>> index c4376d50786b..03dd625b2781 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_types.h
>> +++ b/drivers/gpu/drm/xe/xe_gt_types.h
>> @@ -124,14 +124,11 @@ struct xe_gt {
>>  	} info;
>>
>>  	/**
>> -	 * @mmio: mmio info for GT, can be subset of the global device mmio
>> -	 * space
>> +	 * @mmio: mmio info for GT.  All GTs within a tile share the same
>> +	 * register space, but have their own copy of GSI registers at a
>> +	 * specific offset, as well as their own forcewake handling.
>>  	 */
>>  	struct {
>> -		/** @size: size of MMIO space on GT */
>> -		size_t size;
>> -		/** @regs: pointer to MMIO space on GT */
>> -		void *regs;
>>  		/** @fw: force wake for GT */
>>  		struct xe_force_wake fw;
>>  		/**
>> diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
>> index 254b4a63d901..54fa1212fcd9 100644
>> --- a/drivers/gpu/drm/xe/xe_mmio.c
>> +++ b/drivers/gpu/drm/xe/xe_mmio.c
>> @@ -307,6 +307,7 @@ static void xe_mmio_probe_tiles(struct xe_device *xe)
>>
>>  	if (xe->info.tile_count > 1) {
>>  		const int mmio_bar = 0;
>> +		struct xe_tile *tile;
>>  		size_t size;
>>  		void *regs;
>>
>> @@ -320,11 +321,11 @@ static void xe_mmio_probe_tiles(struct xe_device *xe)
>>  		size = xe->mmio.size / adj_tile_count;
>>  		regs = xe->mmio.regs;
>>
>> -		for_each_gt(gt, xe, id) {
>> -			if (id && !xe_gt_is_media_type(gt))
>> -				regs += size;
>> -			gt->mmio.size = size;
>> -			gt->mmio.regs = regs;
>> +		for_each_tile(tile, xe, id) {
>> +			tile->mmio.size = size;
>> +			tile->mmio.regs = regs;
>> +
>> +			regs += size;
>>  		}
>>  	}
>>  }
>> @@ -340,15 +341,16 @@ static void mmio_fini(struct drm_device *drm, void *arg)
>>
>>  int xe_mmio_init(struct xe_device *xe)
>>  {
>> +	struct xe_tile *root_tile = xe_device_get_root_tile(xe);
>>  	struct xe_gt *gt = xe_device_get_gt(xe, 0);
>>  	const int mmio_bar = 0;
>>  	int err;
>>
>>  	/*
>> -	 * Map the entire BAR, which includes registers (0-4MB), reserved space
>> -	 * (4MB-8MB), and GGTT (8MB-16MB). Other parts of the driver (GTs,
>> -	 * GGTTs) will derive the pointers they need from the mapping in the
>> -	 * device structure.
>> +	 * Map the first 16MB of th BAR, which includes the registers (0-4MB),
>> +	 * reserved space (4MB-8MB), and GGTT (8MB-16MB) for a single tile.
>> +	 * This will get remapped later if we determine that we're running
>> +	 * on a multi-tile system.
>>  	 */
>>  	xe->mmio.size = SZ_16M;
>>  	xe->mmio.regs = pci_iomap(to_pci_dev(xe->drm.dev), mmio_bar,
>> @@ -362,9 +364,9 @@ int xe_mmio_init(struct xe_device *xe)
>>  	if (err)
>>  		return err;
>>
>> -	/* 1 GT for now, 1 to 1 mapping, may change on multi-GT devices */
>> -	gt->mmio.size = xe->mmio.size;
>> -	gt->mmio.regs = xe->mmio.regs;
>> +	/* Setup first tile; other tiles (if present) will be setup later. */
>> +	root_tile->mmio.size = xe->mmio.size;
>> +	root_tile->mmio.regs = xe->mmio.regs;
>>
>>  	/*
>>  	 * The boot firmware initializes local memory and assesses its health.
>> diff --git a/drivers/gpu/drm/xe/xe_mmio.h b/drivers/gpu/drm/xe/xe_mmio.h
>> index 1407f1189b0d..acf0b18f3111 100644
>> --- a/drivers/gpu/drm/xe/xe_mmio.h
>> +++ b/drivers/gpu/drm/xe/xe_mmio.h
>> @@ -10,6 +10,7 @@
>>  #include <linux/io-64-nonatomic-lo-hi.h>
>>
>>  #include "regs/xe_reg_defs.h"
>> +#include "xe_device_types.h"
>>  #include "xe_gt_types.h"
>>
>>  struct drm_device;
>> @@ -20,27 +21,33 @@ int xe_mmio_init(struct xe_device *xe);
>>
>>  static inline u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)
>>  {
>> +	struct xe_tile *tile = gt_to_tile(gt);
>> +
>>  	if (reg.addr < gt->mmio.adj_limit)
>>  		reg.addr += gt->mmio.adj_offset;
>>
>> -	return readb(gt->mmio.regs + reg.addr);
>> +	return readb(tile->mmio.regs + reg.addr);
>>  }
>>
>>  static inline void xe_mmio_write32(struct xe_gt *gt,
>>  				   struct xe_reg reg, u32 val)
>>  {
>> +	struct xe_tile *tile = gt_to_tile(gt);
>> +
>>  	if (reg.addr < gt->mmio.adj_limit)
>>  		reg.addr += gt->mmio.adj_offset;
>>
>> -	writel(val, gt->mmio.regs + reg.addr);
>> +	writel(val, tile->mmio.regs + reg.addr);
>>  }
>>
>>  static inline u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg)
>>  {
>> +	struct xe_tile *tile = gt_to_tile(gt);
>> +
>>  	if (reg.addr < gt->mmio.adj_limit)
>>  		reg.addr += gt->mmio.adj_offset;
>>
>> -	return readl(gt->mmio.regs + reg.addr);
>> +	return readl(tile->mmio.regs + reg.addr);
>>  }
>>
>>  static inline u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr,
>> @@ -58,18 +65,22 @@ static inline u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr,
>>  static inline void xe_mmio_write64(struct xe_gt *gt,
>>  				   struct xe_reg reg, u64 val)
>>  {
>> +	struct xe_tile *tile = gt_to_tile(gt);
>> +
>>  	if (reg.addr < gt->mmio.adj_limit)
>>  		reg.addr += gt->mmio.adj_offset;
>>
>> -	writeq(val, gt->mmio.regs + reg.addr);
>> +	writeq(val, tile->mmio.regs + reg.addr);
>>  }
>>
>>  static inline u64 xe_mmio_read64(struct xe_gt *gt, struct xe_reg reg)
>>  {
>> +	struct xe_tile *tile = gt_to_tile(gt);
>> +
>>  	if (reg.addr < gt->mmio.adj_limit)
>>  		reg.addr += gt->mmio.adj_offset;
>>
>> -	return readq(gt->mmio.regs + reg.addr);
>> +	return readq(tile->mmio.regs + reg.addr);
>>  }
>>
>>  static inline int xe_mmio_write32_and_verify(struct xe_gt *gt,
>
>-- 
>Jani Nikula, Intel Open Source Graphics Center


More information about the Intel-xe mailing list