Re: [PATCH v2 18/43] drm/xe/pci: Convert register access to use xe_mmio

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Sep 06, 2024 at 05:08:07PM -0700, Matt Roper wrote:
> Stop using GT pointers for register access.
> 
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pci.c | 13 ++++++++-----
>  1 file changed, 8 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index aa0c29450b4a..e0188ef9995e 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -473,13 +473,15 @@ enum xe_gmdid_type {
>  
>  static void read_gmdid(struct xe_device *xe, enum xe_gmdid_type type, u32 *ver, u32 *revid)
>  {
> -	struct xe_gt *gt = xe_root_mmio_gt(xe);
> +	struct xe_mmio *mmio = xe_root_tile_mmio(xe);
>  	struct xe_reg gmdid_reg = GMD_ID;
>  	u32 val;
>  
>  	KUNIT_STATIC_STUB_REDIRECT(read_gmdid, xe, type, ver, revid);
>  
>  	if (IS_SRIOV_VF(xe)) {
> +		struct xe_gt *gt = xe_root_mmio_gt(xe);
> +
>  		/*
>  		 * To get the value of the GMDID register, VFs must obtain it
>  		 * from the GuC using MMIO communication.
> @@ -515,14 +517,15 @@ static void read_gmdid(struct xe_device *xe, enum xe_gmdid_type type, u32 *ver,
>  		gt->info.type = XE_GT_TYPE_UNINITIALIZED;
>  	} else {
>  		/*
> -		 * We need to apply the GSI offset explicitly here as at this
> -		 * point the xe_gt is not fully uninitialized and only basic
> -		 * access to MMIO registers is possible.
> +		 * We need to apply the GSI offset explicitly here since we're
> +		 * reading through the tile-level MMIO accessor (at this
> +		 * point the GT is not fully uninitialized and only basic
> +		 * access to MMIO registers is possible).

it is kind of strange to mention GT initialization now that gt struct is
not visible and that we are trying to detach mmio from gt.

But well, perhaps the strange part is this entire manual offset thing and
this change in the comment is needed now that xe_gt struct is not used
and this keep the meaning, so


Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

>  		 */
>  		if (type == GMDID_MEDIA)
>  			gmdid_reg.addr += MEDIA_GT_GSI_OFFSET;
>  
> -		val = xe_mmio_read32(gt, gmdid_reg);
> +		val = xe_mmio_read32(mmio, gmdid_reg);
>  	}
>  
>  	*ver = REG_FIELD_GET(GMD_ID_ARCH_MASK, val) * 100 + REG_FIELD_GET(GMD_ID_RELEASE_MASK, val);
> -- 
> 2.45.2
>