Re: [PATCH v2 2/5] drm/xe: Add helper to calculate adjusted register offset

["Piotr Piórkowski" <piotr.piorkowski@intel.com>]

Michal Wajdeczko <michal.wajdeczko@intel.com> wrote on wto [2024-kwi-23 20:04:33 +0200]:
> Our MMIO accessing functions automatically adjust addresses for the
> media registers based on mmio.adj_limit and mmio.adj_offset logic.
> Move it to the separate helper to avoid code duplication and to
> allow using it by the upcoming changes to PF driver code.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Piotr Piórkowski <piotr.piorkowski@intel.com>
> ---
> v2: start using helper (Piotr)
>     make gt param const to allow use in xe_mmio_in_range()
>     assert that we adjust both or none addr in xe_mmio_read64_2x32()
> ---
>  drivers/gpu/drm/xe/xe_mmio.c | 38 ++++++++++++++----------------------
>  drivers/gpu/drm/xe/xe_mmio.h |  7 +++++++
>  2 files changed, 22 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_mmio.c b/drivers/gpu/drm/xe/xe_mmio.c
> index 334637511e75..2b18e8149ec3 100644
> --- a/drivers/gpu/drm/xe/xe_mmio.c
> +++ b/drivers/gpu/drm/xe/xe_mmio.c
> @@ -423,41 +423,33 @@ int xe_mmio_init(struct xe_device *xe)
>  u8 xe_mmio_read8(struct xe_gt *gt, struct xe_reg reg)
>  {
>  	struct xe_tile *tile = gt_to_tile(gt);
> +	u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
>  
> -	if (reg.addr < gt->mmio.adj_limit)
> -		reg.addr += gt->mmio.adj_offset;
> -
> -	return readb((reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + reg.addr);
> +	return readb((reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
>  }
>  
>  u16 xe_mmio_read16(struct xe_gt *gt, struct xe_reg reg)
>  {
>  	struct xe_tile *tile = gt_to_tile(gt);
> +	u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
>  
> -	if (reg.addr < gt->mmio.adj_limit)
> -		reg.addr += gt->mmio.adj_offset;
> -
> -	return readw((reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + reg.addr);
> +	return readw((reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
>  }
>  
>  void xe_mmio_write32(struct xe_gt *gt, struct xe_reg reg, u32 val)
>  {
>  	struct xe_tile *tile = gt_to_tile(gt);
> +	u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
>  
> -	if (reg.addr < gt->mmio.adj_limit)
> -		reg.addr += gt->mmio.adj_offset;
> -
> -	writel(val, (reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + reg.addr);
> +	writel(val, (reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
>  }
>  
>  u32 xe_mmio_read32(struct xe_gt *gt, struct xe_reg reg)
>  {
>  	struct xe_tile *tile = gt_to_tile(gt);
> +	u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
>  
> -	if (reg.addr < gt->mmio.adj_limit)
> -		reg.addr += gt->mmio.adj_offset;
> -
> -	return readl((reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + reg.addr);
> +	return readl((reg.ext ? tile->mmio_ext.regs : tile->mmio.regs) + addr);
>  }
>  
>  u32 xe_mmio_rmw32(struct xe_gt *gt, struct xe_reg reg, u32 clr, u32 set)
> @@ -486,10 +478,9 @@ bool xe_mmio_in_range(const struct xe_gt *gt,
>  		      const struct xe_mmio_range *range,
>  		      struct xe_reg reg)
>  {
> -	if (reg.addr < gt->mmio.adj_limit)
> -		reg.addr += gt->mmio.adj_offset;
> +	u32 addr = xe_mmio_adjusted_addr(gt, reg.addr);
>  
> -	return range && reg.addr >= range->start && reg.addr <= range->end;
> +	return range && addr >= range->start && addr <= range->end;
>  }
>  
>  /**
> @@ -519,10 +510,11 @@ u64 xe_mmio_read64_2x32(struct xe_gt *gt, struct xe_reg reg)
>  	struct xe_reg reg_udw = { .addr = reg.addr + 0x4 };
>  	u32 ldw, udw, oldudw, retries;
>  
> -	if (reg.addr < gt->mmio.adj_limit) {
> -		reg.addr += gt->mmio.adj_offset;
> -		reg_udw.addr += gt->mmio.adj_offset;
> -	}
> +	reg.addr = xe_mmio_adjusted_addr(gt, reg.addr);
> +	reg_udw.addr = xe_mmio_adjusted_addr(gt, reg_udw.addr);
> +
> +	/* we shouldn't adjust just one register address */
> +	xe_gt_assert(gt, reg_udw.addr == reg.addr + 0x4);
>  
>  	oldudw = xe_mmio_read32(gt, reg_udw);
>  	for (retries = 5; retries; --retries) {
> diff --git a/drivers/gpu/drm/xe/xe_mmio.h b/drivers/gpu/drm/xe/xe_mmio.h
> index a3cd7b3036c7..445ec6a0753e 100644
> --- a/drivers/gpu/drm/xe/xe_mmio.h
> +++ b/drivers/gpu/drm/xe/xe_mmio.h
> @@ -36,4 +36,11 @@ u64 xe_mmio_read64_2x32(struct xe_gt *gt, struct xe_reg reg);
>  int xe_mmio_wait32(struct xe_gt *gt, struct xe_reg reg, u32 mask, u32 val, u32 timeout_us,
>  		   u32 *out_val, bool atomic);
>  
> +static inline u32 xe_mmio_adjusted_addr(const struct xe_gt *gt, u32 addr)
> +{
> +	if (addr < gt->mmio.adj_limit)
> +		addr += gt->mmio.adj_offset;
> +	return addr;
> +}
> +

LGTM:
Reviewed-by: Piotr Piórkowski <piotr.piorkowski@intel.com>

>  #endif
> -- 
> 2.43.0
> 

--