On Tue, Feb 14, 2017 at 08:17:51PM -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. It was actually cairo where we had patches to use it first. And we then realised the use was broken for gen6. > '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. > > Cc: Dave Gordon <david.s.gordon@xxxxxxxxx> > Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92448 > Signed-off-by: Kenneth Graunke <kenneth@xxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/i915_drv.c | 2 +- > drivers/gpu/drm/i915/i915_drv.h | 2 -- > drivers/gpu/drm/i915/i915_gem.c | 2 -- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 50 ++---------------------------- > 4 files changed, 3 insertions(+), 53 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 309c29c84c54..1a53a4bb09c8 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -276,7 +276,7 @@ static int i915_getparam(struct drm_device *dev, void *data, > value = !!dev_priv->engine[VCS2]; > break; > case I915_PARAM_HAS_EXEC_CONSTANTS: > - value = INTEL_GEN(dev_priv) >= 4; > + value = 0; The style for obsolete param is to return -ENODEV. If that makes sense we can move this to the start of this block with the other dead params. Reviewed-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> -Chris -- Chris Wilson, Intel Open Source Technology Centre