Re: [PATCH 05/11] drm/xe: Stop assuming the root tile has a primary GT

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [Medium] drm/xe/oa: incorrect GRAPHICS_VER check in
  xe_oa_enable_metric_set() [1]
--

commit 60c4b3494beccd5ac07590503398e4dda529a384
Author: Stuart Summers <stuart.summers@intel.com>

drm/xe: Stop assuming the root tile has a primary GT

This commit introduces xe_device_any_gt() and xe_oa_first_unit() so that
device-level consumers and OA stream open do not assume the root tile has
a primary GT.

> diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c
> index 838802482adaf..05beb156c941a 100644
> --- a/drivers/gpu/drm/xe/xe_oa.c
> +++ b/drivers/gpu/drm/xe/xe_oa.c
> @@ -2041,6 +2041,23 @@ static int xe_oa_assign_hwe(struct xe_oa *oa, struct xe_oa_open_param *param)
>  	return ret;
>  }
>  
> +/*
> + * OA unit IDs are handed out in GT iteration order, so the first unit of the
> + * first GT that has one is OA unit 0. The primary GT may be absent, in which
> + * case unit 0 lives on the media GT.
> + */
> +static struct xe_oa_unit *xe_oa_first_unit(struct xe_device *xe)
> +{
> +	struct xe_gt *gt;
> +	u8 id;
> +
> +	for_each_gt(gt, xe, id)
> +		if (gt->oa.num_oa_units)
> +			return &gt->oa.oa_unit[0];
> +
> +	return NULL;
> +}
> +
>  /**
>   * xe_oa_stream_open_ioctl - Opens an OA stream
>   * @dev: @drm_device
> @@ -2073,8 +2090,11 @@ int xe_oa_stream_open_ioctl(struct drm_device *dev, u64 data, struct drm_file *f
>  		return ret;
>  
>  	/* If not provided, OA unit defaults to OA unit 0 as per uapi */
> -	if (!param.oa_unit)
> -		param.oa_unit = &xe_root_mmio_gt(oa->xe)->oa.oa_unit[0];
> +	if (!param.oa_unit) {
> +		param.oa_unit = xe_oa_first_unit(oa->xe);
> +		if (XE_IOCTL_DBG(xe, !param.oa_unit))
> +			return -ENODEV;
> +	}

[Severity: Medium]
This problem wasn't introduced by this patch, but should
xe_oa_enable_metric_set() and xe_oa_disable_metric_set() check MEDIA_VER()
instead of GRAPHICS_VER() [1] when programming
OAM_COMPRESSION_T3_CONTROL?

drivers/gpu/drm/xe/xe_oa.c:xe_oa_enable_metric_set() {
    ...
	if ((stream->oa_unit->type == DRM_XE_OA_UNIT_TYPE_OAM ||
	     stream->oa_unit->type == DRM_XE_OA_UNIT_TYPE_OAM_SAG) &&
	    GRAPHICS_VER(stream->oa->xe) >= 30)
		xe_mmio_rmw32(mmio, OAM_COMPRESSION_T3_CONTROL, 0, OAM_LAT_MEASURE_ENABLE);
    ...
}

OAM units belong to the media IP block, so checking GRAPHICS_VER() skips
enabling latency measurement when the graphics IP is absent or older than
version 30 while media IP is >= 30, and issues unsupported MMIO writes to
OAM_COMPRESSION_T3_CONTROL when graphics IP is >= 30 but media IP is < 30.

[1] https://sashiko.dev/#/bug/linux-23224ef1-a940-4408-9697-0c35c66f6046

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261110231442.218721-13-stuart.summers@intel.com?part=5