Re: [Intel-gfx] [PATCH v2 2/6] drm/i915/xehp: Drop GETPARAM lookups of I915_PARAM_[SUB]SLICE_MASK

[Lionel Landwerlin <lionel.g.landwerlin@intel.com>]

On 17/05/2022 06:20, Matt Roper wrote:
> Slice/subslice/EU information should be obtained via the topology
> queries provided by the I915_QUERY interface; let's turn off support for
> the old GETPARAM lookups on Xe_HP and beyond where we can't return
> meaningful values.
>
> The slice mask lookup is meaningless since Xe_HP doesn't support
> traditional slices (and we make no attempt to return the various new
> units like gslices, cslices, mslices, etc.) here.
>
> The subslice mask lookup is even more problematic; given the distinct
> masks for geometry vs compute purposes, the combined mask returned here
> is likely not what userspace would want to act upon anyway.  The value
> is also limited to 32-bits by the nature of the GETPARAM ioctl which is
> sufficient for the initial Xe_HP platforms, but is unable to convey the
> larger masks that will be needed on other upcoming platforms.  Finally,
> the value returned here becomes even less meaningful when used on
> multi-tile platforms where each tile will have its own masks.
>
> Signed-off-by: Matt Roper <matthew.d.roper@intel.com>


Sounds fair. We've been relying on the topology query in Mesa since it's 
available and it's a requirement for Gfx10+.

FYI, we're also not using I915_PARAM_EU_TOTAL on Gfx10+ for the same reason.


Acked-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>


> ---
>   drivers/gpu/drm/i915/i915_getparam.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
> index c12a0adefda5..ac9767c56619 100644
> --- a/drivers/gpu/drm/i915/i915_getparam.c
> +++ b/drivers/gpu/drm/i915/i915_getparam.c
> @@ -148,11 +148,19 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
>   		value = intel_engines_has_context_isolation(i915);
>   		break;
>   	case I915_PARAM_SLICE_MASK:
> +		/* Not supported from Xe_HP onward; use topology queries */
> +		if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))
> +			return -EINVAL;
> +
>   		value = sseu->slice_mask;
>   		if (!value)
>   			return -ENODEV;
>   		break;
>   	case I915_PARAM_SUBSLICE_MASK:
> +		/* Not supported from Xe_HP onward; use topology queries */
> +		if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50))
> +			return -EINVAL;
> +
>   		/* Only copy bits from the first slice */
>   		memcpy(&value, sseu->subslice_mask,
>   		       min(sseu->ss_stride, (u8)sizeof(value)));