On ke, 2017-02-15 at 10:40 +0000, Chris Wilson wrote: > On Wed, Feb 15, 2017 at 01:34:46AM -0800, Kenneth Graunke wrote: > > > > This patch makes the I915_PARAM_HAS_EXEC_CONSTANTS getparam return 0 > > (indicating the optional feature is not supported), and makes execbuf > > always return -EINVAL if the flags are used. > > > > Apparently, no userspace ever shipped which used this optional feature: > > I checked the git history of Mesa, xf86-video-intel, libva, and Beignet, > > and there were zero commits showing a use of these flags. Kernel commit > > 72bfa19c8deb4 apparently introduced the feature prematurely. According > > to Chris, the intention was to use this in cairo-drm, but "the use was > > broken for gen6", so I don't think it ever happened. > > > > 'relative_constants_mode' has always been tracked per-device, but this > > has actually been wrong ever since hardware contexts were introduced, as > > the INSTPM register is saved (and automatically restored) as part of the > > render ring context. The software per-device value could therefore get > > out of sync with the hardware per-context value. This meant that using > > them is actually unsafe: a client which tried to use them could damage > > the state of other clients, causing the GPU to interpret their BO > > offsets as absolute pointers, leading to bogus memory reads. > > > > These flags were also never ported to execlist mode, making them no-ops > > on Gen9+ (which requires execlists), and Gen8 in the default mode. > > > > On Gen8+, userspace can write these registers directly, achieving the > > same effect. On Gen6-7.5, it likely makes sense to extend the command > > parser to support them. I don't think anyone wants this on Gen4-5. > > > > Based on a patch by Dave Gordon. > > > > v3: Return -ENODEV for the getparam, as this is what we do for other > > obsolete features. Suggested by Chris Wilson. > > > > Cc: stable@xxxxxxxxxxxxxxx > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92448 > > Signed-off-by: Kenneth Graunke <kenneth@xxxxxxxxxxxxx> > > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> [v2] > > Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> [v2] > Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> <SNIP> > Just give Daniel a chance to ack the ABI revert if he cares... I'm just thinking this might break an application which does the feature detection and only understands 0 or 1. Either way, with A-b from Daniel, this too is; Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx> Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation