Skip to contentSkip to conversation

Patch "drm/i915: Drop support for I915_EXEC_CONSTANTS_* execbuf parameters." has been added to the 4.10-stable tree

PATCH [email protected] 1 participant 1 message Apr 12, 2017

Subsystems (3)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

drm/i915: Drop support for I915_EXEC_CONSTANTS_* execbuf parameters.

to the 4.10-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
drm-i915-drop-support-for-i915_exec_constants_-execbuf-parameters.patch
and it can be found in the queue-4.10 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.

Quoted 1 line

From 0f5418e564ac6452b9086295646e602a9addc4bf Mon Sep 17 00:00:00 2001

From: Kenneth Graunke <[email protected]>
Date: Mon, 13 Mar 2017 17:04:33 +0000
Subject: drm/i915: Drop support for I915_EXEC_CONSTANTS_* execbuf parameters.

From: Kenneth Graunke <[email protected]>

commit 0f5418e564ac6452b9086295646e602a9addc4bf upstream.

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.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92448

---
 drivers/gpu/drm/i915/i915_drv.c            |    4 --
 drivers/gpu/drm/i915/i915_drv.h            |    2 -
 drivers/gpu/drm/i915/i915_gem.c            |    2 -
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |   52 +----------------------------
 4 files changed, 3 insertions(+), 57 deletions(-)
drivers/gpu/drm/i915/i915_drv.c +1 -3
@@ -248,6 +248,7 @@ static int i915_getparam(struct drm_devi
 	case I915_PARAM_IRQ_ACTIVE:
 	case I915_PARAM_ALLOW_BATCHBUFFER:
 	case I915_PARAM_LAST_DISPATCH:
+	case I915_PARAM_HAS_EXEC_CONSTANTS:
 		/* Reject all old ums/dri params. */
 		return -ENODEV;
 	case I915_PARAM_CHIPSET_ID:
@@ -274,9 +275,6 @@ static int i915_getparam(struct drm_devi
 	case I915_PARAM_HAS_BSD2:
 		value = !!dev_priv->engine[VCS2];
 		break;
-	case I915_PARAM_HAS_EXEC_CONSTANTS:
-		value = INTEL_GEN(dev_priv) >= 4;
-		break;
 	case I915_PARAM_HAS_LLC:
 		value = HAS_LLC(dev_priv);
 		break;
drivers/gpu/drm/i915/i915_drv.h +0 -2
@@ -1805,8 +1805,6 @@ struct drm_i915_private {
 
 	const struct intel_device_info info;
 
-	int relative_constants_mode;
-
 	void __iomem *regs;
 
 	struct intel_uncore uncore;
drivers/gpu/drm/i915/i915_gem.c +0 -2
@@ -4567,8 +4567,6 @@ i915_gem_load_init(struct drm_device *de
 	init_waitqueue_head(&dev_priv->gpu_error.wait_queue);
 	init_waitqueue_head(&dev_priv->gpu_error.reset_queue);
 
-	dev_priv->relative_constants_mode = I915_EXEC_CONSTANTS_REL_GENERAL;
-
 	init_waitqueue_head(&dev_priv->pending_flip_queue);
 
 	dev_priv->mm.interruptible = true;
drivers/gpu/drm/i915/i915_gem_execbuffer.c +2 -50

68 lines not on this page · raw message

Patches currently in stable-queue which might be from [email protected] are

queue-4.10/drm-i915-drop-support-for-i915_exec_constants_-execbuf-parameters.patch