On Fri, May 20, 2022 at 10:15:32AM +0100, Tvrtko Ursulin wrote: > > On 17/05/2022 04: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@xxxxxxxxx> > > --- > > 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))); > > Just in case lets run this by Jordan and Lionel since it affects DG2. Anyone > else on the userspace side who might be affected? When I grep'd Mesa, I found two uses of I915_PARAM_SLICE_MASK and I915_PARAM_SUBSLICE_MASK: * oa_metrics_kernel_support: The topology query is used on gen10+ so the getparam code is only called on gen9 and below * getparam_topology: Invoked via intel_get_device_info_from_fd(). The topology query is attempted first. Only if that fails _and_ we're on a pre-gen10 platform does it fall back to GETPARAM. I also checked https://github.com/intel/compute-runtime and only see these being issued in one place: * HwInfoConfig::configureHwInfoDrm: Only used if drm->queryTopology() returns a failure first. I think those are the only relevant userspace for SSEU topology, so as far as I can tell nobody is still relying on the legacy getparams by the time we get to Xe_HP hardware. Matt > > Regards, > > Tvrtko -- Matt Roper Graphics Software Engineer VTT-OSGC Platform Enablement Intel Corporation
