[PATCH 00/11] drm-intel-collector - update

[PATCH 00/11] drm-intel-collector - update

[Rodrigo Vivi]

This is another drm-intel-collector updated notice:
http://cgit.freedesktop.org/~vivijim/drm-intel/log/?h=drm-intel-collector

Here goes the update list in order for better reviewers assignment:

Patch     drm/i915: Specify bsd rings through exec flag - Reviewer:
Patch     drm/i915: add I915_PARAM_HAS_BSD2 to i915_getparam - Reviewer:
Patch     drm/i915: Move the ban period onto the context - Reviewer:
Patch     drm/i915: Add ioctl to set per-context parameters - Reviewer:
Patch     drm/i915: Put logical pipe_control emission into a helper. - Reviewer:
Patch     drm/i915: Add WaCsStallBeforeStateCacheInvalidate:bdw, chv to logical ring - Reviewer:
Patch     drm/i915: Remove pinned check from madvise_ioctl - Reviewer:
Patch     drm/i915: Extend GET_APERTURE ioctl to report available map space - Reviewer:
Patch     drm/i915: Display current hangcheck status in debugfs - Reviewer:
Patch     drm/i915/vlv: check port in infoframe_enabled v2 - Reviewer:
Patch     drm/i915: vlv: fix save/restore of GFX_MAX_REQ_COUNT reg - Reviewer:

Round from discussions ended between Nov 21 to Dec 05.

Thanks,
Rodrigo.


Chris Wilson (5):
  drm/i915: Move the ban period onto the context
  drm/i915: Add ioctl to set per-context parameters
  drm/i915: Remove pinned check from madvise_ioctl
  drm/i915: Extend GET_APERTURE ioctl to report available map space
  drm/i915: Display current hangcheck status in debugfs

Imre Deak (1):
  drm/i915: vlv: fix save/restore of GFX_MAX_REQ_COUNT reg

Jesse Barnes (1):
  drm/i915/vlv: check port in infoframe_enabled v2

Rodrigo Vivi (2):
  drm/i915: Put logical pipe_control emission into a helper.
  drm/i915: Add WaCsStallBeforeStateCacheInvalidate:bdw, chv to logical
    ring

Zhipeng Gong (2):
  drm/i915: Specify bsd rings through exec flag
  drm/i915: add I915_PARAM_HAS_BSD2 to i915_getparam

 drivers/gpu/drm/i915/i915_debugfs.c        |  62 +++++++++++++++
 drivers/gpu/drm/i915/i915_dma.c            |   5 ++
 drivers/gpu/drm/i915/i915_drv.c            |   4 +-
 drivers/gpu/drm/i915/i915_drv.h            |   9 +++
 drivers/gpu/drm/i915/i915_gem.c            | 119 ++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/i915_gem_context.c    |  71 +++++++++++++++++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  19 ++++-
 drivers/gpu/drm/i915/intel_hdmi.c          |   7 +-
 drivers/gpu/drm/i915/intel_lrc.c           |  41 +++++++---
 include/uapi/drm/i915_drm.h                |  46 ++++++++++-
 10 files changed, 354 insertions(+), 29 deletions(-)

-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 01/11] drm/i915: Specify bsd rings through exec flag

[Rodrigo Vivi]

From: Zhipeng Gong <zhipeng.gong@intel.com>

On Skylake GT3 we have 2 Video Command Streamers (VCS), which is asymmetrical.
For example, HEVC GPU commands can be only dispatched to VCS1 ring.
But userspace has no control when using VCS1 or VCS2. This patch introduces
a mechanism to avoid the default ping-pong mode and use one specific ring
through execution flag. This mechanism is usable for all the platforms
with 2 VCS rings.

v2: fix whitespace (Rodrigo)
v3: remove incorrect chunk that came on -collector rebase. (Rodrigo)
v4: change the comment (Zhipeng)

Signed-off-by: Zhipeng Gong <zhipeng.gong@intel.com>
Reviewed-by-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 19 +++++++++++++++++--
 include/uapi/drm/i915_drm.h                |  8 +++++++-
 2 files changed, 24 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 8330660..a9bcfab 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1386,8 +1386,23 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	else if ((args->flags & I915_EXEC_RING_MASK) == I915_EXEC_BSD) {
 		if (HAS_BSD2(dev)) {
 			int ring_id;
-			ring_id = gen8_dispatch_bsd_ring(dev, file);
-			ring = &dev_priv->ring[ring_id];
+
+			switch (args->flags & I915_EXEC_BSD_MASK) {
+			case I915_EXEC_BSD_DEFAULT:
+				ring_id = gen8_dispatch_bsd_ring(dev, file);
+				ring = &dev_priv->ring[ring_id];
+				break;
+			case I915_EXEC_BSD_RING1:
+				ring = &dev_priv->ring[VCS];
+				break;
+			case I915_EXEC_BSD_RING2:
+				ring = &dev_priv->ring[VCS2];
+				break;
+			default:
+				DRM_DEBUG("execbuf with unknown bsd ring: %d\n",
+					  (int)(args->flags & I915_EXEC_BSD_MASK));
+				return -EINVAL;
+			}
 		} else
 			ring = &dev_priv->ring[VCS];
 	} else
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 2502622..fcb16bf 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -737,7 +737,13 @@ struct drm_i915_gem_execbuffer2 {
  */
 #define I915_EXEC_HANDLE_LUT		(1<<12)
 
-#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_HANDLE_LUT<<1)
+/** Used for switching BSD rings on the platforms with two BSD rings */
+#define I915_EXEC_BSD_MASK		(3<<13)
+#define I915_EXEC_BSD_DEFAULT		(0<<13) /* default ping-pong mode */
+#define I915_EXEC_BSD_RING1		(1<<13)
+#define I915_EXEC_BSD_RING2		(2<<13)
+
+#define __I915_EXEC_UNKNOWN_FLAGS -(1<<15)
 
 #define I915_EXEC_CONTEXT_ID_MASK	(0xffffffff)
 #define i915_execbuffer2_set_context_id(eb2, context) \
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 02/11] drm/i915: add I915_PARAM_HAS_BSD2 to i915_getparam

[Rodrigo Vivi]

From: Zhipeng Gong <zhipeng.gong@intel.com>

This will let userland only try to use the new ring
when the appropriate kernel is present

Signed-off-by: Zhipeng Gong <zhipeng.gong@intel.com>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
Reviewed--by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c | 3 +++
 include/uapi/drm/i915_drm.h     | 1 +
 2 files changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 52730ed..126a36f 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -92,6 +92,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_HAS_VEBOX:
 		value = intel_ring_initialized(&dev_priv->ring[VECS]);
 		break;
+	case I915_PARAM_HAS_BSD2:
+		value = intel_ring_initialized(&dev_priv->ring[VCS2]);
+		break;
 	case I915_PARAM_HAS_RELAXED_FENCING:
 		value = 1;
 		break;
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index fcb16bf..fa99129 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -341,6 +341,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_WT     	 	 27
 #define I915_PARAM_CMD_PARSER_VERSION	 28
 #define I915_PARAM_HAS_COHERENT_PHYS_GTT 29
+#define I915_PARAM_HAS_BSD2		 30
 
 typedef struct drm_i915_getparam {
 	int param;
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 03/11] drm/i915: Move the ban period onto the context

[Rodrigo Vivi]

From: Chris Wilson <chris@chris-wilson.co.uk>

This will allow us to set per-file, or even per-context, periods in the
future.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         | 5 +++++
 drivers/gpu/drm/i915/i915_gem.c         | 3 ++-
 drivers/gpu/drm/i915/i915_gem_context.c | 2 ++
 3 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 921e4c5..3749415 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -678,6 +678,11 @@ struct i915_ctx_hang_stats {
 	/* Time when this context was last blamed for a GPU reset */
 	unsigned long guilty_ts;
 
+	/* If the contexts causes a second GPU hang within this time,
+	 * it is permanently banned from submitting any more work.
+	 */
+	unsigned long ban_period_seconds;
+
 	/* This context is banned to submit more work */
 	bool banned;
 };
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 2b6ecfd..67e9100 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2547,7 +2547,8 @@ static bool i915_context_is_banned(struct drm_i915_private *dev_priv,
 	if (ctx->hang_stats.banned)
 		return true;
 
-	if (elapsed <= DRM_I915_CTX_BAN_PERIOD) {
+	if (ctx->hang_stats.ban_period_seconds &&
+	    elapsed <= ctx->hang_stats.ban_period_seconds) {
 		if (!i915_gem_context_is_default(ctx)) {
 			DRM_DEBUG("context hanging too fast, banning!\n");
 			return true;
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 9b23fb1..bf9778e 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -222,6 +222,8 @@ __create_hw_context(struct drm_device *dev,
 	 * is no remap info, it will be a NOP. */
 	ctx->remap_slice = (1 << NUM_L3_SLICES(dev)) - 1;
 
+	ctx->hang_stats.ban_period_seconds = DRM_I915_CTX_BAN_PERIOD;
+
 	return ctx;
 
 err_out:
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 04/11] drm/i915: Add ioctl to set per-context parameters

[Rodrigo Vivi]

From: Chris Wilson <chris@chris-wilson.co.uk>

Sometimes we wish to tweak how an individual context behaves. Since we
always create a context for every filp, this means that individual
processes can fine tune their behaviour even if they do not explicitly
create a context.

The first example parameter here is to enable multi-process GPU testing,
but the interface should be able to cope with passing arbitrarily complex
parameters.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c         |  2 +
 drivers/gpu/drm/i915/i915_drv.h         |  4 ++
 drivers/gpu/drm/i915/i915_gem_context.c | 69 +++++++++++++++++++++++++++++++++
 include/uapi/drm/i915_drm.h             | 12 ++++++
 4 files changed, 87 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 126a36f..3c3f33f 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1066,6 +1066,8 @@ const struct drm_ioctl_desc i915_ioctls[] = {
 	DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_get_reset_stats_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
 };
 
 int i915_max_ioctl = ARRAY_SIZE(i915_ioctls);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 3749415..9ae515e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2898,6 +2898,10 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
 				  struct drm_file *file);
 int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 				   struct drm_file *file);
+int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
+				    struct drm_file *file_priv);
+int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
+				    struct drm_file *file_priv);
 
 /* i915_gem_evict.c */
 int __must_check i915_gem_evict_something(struct drm_device *dev,
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index bf9778e..8603bf4 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -794,3 +794,72 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 	DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
 	return 0;
 }
+
+int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
+				    struct drm_file *file)
+{
+	struct drm_i915_file_private *file_priv = file->driver_priv;
+	struct drm_i915_gem_context_param *args = data;
+	struct intel_context *ctx;
+	int ret;
+
+	ret = i915_mutex_lock_interruptible(dev);
+	if (ret)
+		return ret;
+
+	ctx = i915_gem_context_get(file_priv, args->ctx_id);
+	if (IS_ERR(ctx)) {
+		mutex_unlock(&dev->struct_mutex);
+		return PTR_ERR(ctx);
+	}
+
+	args->size = 0;
+	switch (args->param) {
+	case I915_CONTEXT_PARAM_BAN_PERIOD:
+		args->value = ctx->hang_stats.ban_period_seconds;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	mutex_unlock(&dev->struct_mutex);
+
+	return ret;
+}
+
+int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
+				    struct drm_file *file)
+{
+	struct drm_i915_file_private *file_priv = file->driver_priv;
+	struct drm_i915_gem_context_param *args = data;
+	struct intel_context *ctx;
+	int ret;
+
+	ret = i915_mutex_lock_interruptible(dev);
+	if (ret)
+		return ret;
+
+	ctx = i915_gem_context_get(file_priv, args->ctx_id);
+	if (IS_ERR(ctx)) {
+		mutex_unlock(&dev->struct_mutex);
+		return PTR_ERR(ctx);
+	}
+
+	switch (args->param) {
+	case I915_CONTEXT_PARAM_BAN_PERIOD:
+		if (args->size)
+			ret = -EINVAL;
+		else if (args->value < ctx->hang_stats.ban_period_seconds &&
+			 !capable(CAP_SYS_ADMIN))
+			ret = -EPERM;
+		else
+			ctx->hang_stats.ban_period_seconds = args->value;
+		break;
+	default:
+		ret = -EINVAL;
+		break;
+	}
+	mutex_unlock(&dev->struct_mutex);
+
+	return ret;
+}
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index fa99129..d253c85 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -224,6 +224,8 @@ typedef struct _drm_i915_sarea {
 #define DRM_I915_REG_READ		0x31
 #define DRM_I915_GET_RESET_STATS	0x32
 #define DRM_I915_GEM_USERPTR		0x33
+#define DRM_I915_GEM_CONTEXT_GETPARAM	0x34
+#define DRM_I915_GEM_CONTEXT_SETPARAM	0x35
 
 #define DRM_IOCTL_I915_INIT		DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
 #define DRM_IOCTL_I915_FLUSH		DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
@@ -275,6 +277,8 @@ typedef struct _drm_i915_sarea {
 #define DRM_IOCTL_I915_REG_READ			DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_REG_READ, struct drm_i915_reg_read)
 #define DRM_IOCTL_I915_GET_RESET_STATS		DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GET_RESET_STATS, struct drm_i915_reset_stats)
 #define DRM_IOCTL_I915_GEM_USERPTR			DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_USERPTR, struct drm_i915_gem_userptr)
+#define DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM	DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_GETPARAM, struct drm_i915_gem_context_param)
+#define DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM	DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_SETPARAM, struct drm_i915_gem_context_param)
 
 /* Allow drivers to submit batchbuffers directly to hardware, relying
  * on the security mechanisms provided by hardware.
@@ -1080,4 +1084,12 @@ struct drm_i915_gem_userptr {
 	__u32 handle;
 };
 
+struct drm_i915_gem_context_param {
+	__u32 ctx_id;
+	__u32 size;
+	__u64 param;
+#define I915_CONTEXT_PARAM_BAN_PERIOD 0x1
+	__u64 value;
+};
+
 #endif /* _UAPI_I915_DRM_H_ */
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 05/11] drm/i915: Put logical pipe_control emission into a helper.

[Rodrigo Vivi]

To be used for a Workaroud. Similar to:

commit 884ceacee308f0e4616d0c933518af2639f7b1d8
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Sat Jun 28 02:04:20 2014 +0300

    drm/i915: Refactor Broadwell PIPE_CONTROL emission into a helper.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 35 +++++++++++++++++++++--------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 7670a0f..240474c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1258,6 +1258,26 @@ static int gen8_emit_flush(struct intel_ringbuffer *ringbuf,
 	return 0;
 }
 
+static int gen8_emit_pipe_control(struct intel_ringbuffer *ringbuf,
+				  u32 flags, u32 scratch_addr)
+{
+	int ret;
+
+	ret = intel_logical_ring_begin(ringbuf, 6);
+	if (ret)
+		return ret;
+
+	intel_logical_ring_emit(ringbuf, GFX_OP_PIPE_CONTROL(6));
+	intel_logical_ring_emit(ringbuf, flags);
+	intel_logical_ring_emit(ringbuf, scratch_addr);
+	intel_logical_ring_emit(ringbuf, 0);
+	intel_logical_ring_emit(ringbuf, 0);
+	intel_logical_ring_emit(ringbuf, 0);
+	intel_logical_ring_advance(ringbuf);
+
+	return 0;
+}
+
 static int gen8_emit_flush_render(struct intel_ringbuffer *ringbuf,
 				  u32 invalidate_domains,
 				  u32 flush_domains)
@@ -1265,7 +1285,6 @@ static int gen8_emit_flush_render(struct intel_ringbuffer *ringbuf,
 	struct intel_engine_cs *ring = ringbuf->ring;
 	u32 scratch_addr = ring->scratch.gtt_offset + 2 * CACHELINE_BYTES;
 	u32 flags = 0;
-	int ret;
 
 	flags |= PIPE_CONTROL_CS_STALL;
 
@@ -1285,19 +1304,7 @@ static int gen8_emit_flush_render(struct intel_ringbuffer *ringbuf,
 		flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
 	}
 
-	ret = intel_logical_ring_begin(ringbuf, 6);
-	if (ret)
-		return ret;
-
-	intel_logical_ring_emit(ringbuf, GFX_OP_PIPE_CONTROL(6));
-	intel_logical_ring_emit(ringbuf, flags);
-	intel_logical_ring_emit(ringbuf, scratch_addr);
-	intel_logical_ring_emit(ringbuf, 0);
-	intel_logical_ring_emit(ringbuf, 0);
-	intel_logical_ring_emit(ringbuf, 0);
-	intel_logical_ring_advance(ringbuf);
-
-	return 0;
+	return gen8_emit_pipe_control(ringbuf, flags, scratch_addr);
 }
 
 static u32 gen8_get_seqno(struct intel_engine_cs *ring, bool lazy_coherency)
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 06/11] drm/i915: Add WaCsStallBeforeStateCacheInvalidate:bdw, chv to logical ring

[Rodrigo Vivi]

Similar to:

commit 02c9f7e3cfe76a7f54ef03438c36aade86cc1c8b
Author: Kenneth Graunke <kenneth@whitecape.org>
Date:   Mon Jan 27 14:20:16 2014 -0800

    drm/i915: Add the WaCsStallBeforeStateCacheInvalidate:bdw workaround.

    On Broadwell, any PIPE_CONTROL with the "State Cache Invalidate" bit set
    must be preceded by a PIPE_CONTROL with the "CS Stall" bit set.

    Documented on the BSpec 3D workarounds page.

Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 240474c..53d6bcd 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1285,6 +1285,7 @@ static int gen8_emit_flush_render(struct intel_ringbuffer *ringbuf,
 	struct intel_engine_cs *ring = ringbuf->ring;
 	u32 scratch_addr = ring->scratch.gtt_offset + 2 * CACHELINE_BYTES;
 	u32 flags = 0;
+	int ret;
 
 	flags |= PIPE_CONTROL_CS_STALL;
 
@@ -1302,6 +1303,15 @@ static int gen8_emit_flush_render(struct intel_ringbuffer *ringbuf,
 		flags |= PIPE_CONTROL_STATE_CACHE_INVALIDATE;
 		flags |= PIPE_CONTROL_QW_WRITE;
 		flags |= PIPE_CONTROL_GLOBAL_GTT_IVB;
+
+
+		/* WaCsStallBeforeStateCacheInvalidate:bdw,chv */
+		ret = gen8_emit_pipe_control(ring,
+					     PIPE_CONTROL_CS_STALL |
+					     PIPE_CONTROL_STALL_AT_SCOREBOARD,
+					     0);
+		if (ret)
+			return ret;
 	}
 
 	return gen8_emit_pipe_control(ringbuf, flags, scratch_addr);
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 07/11] drm/i915: Remove pinned check from madvise_ioctl

[Rodrigo Vivi]

From: Chris Wilson <chris@chris-wilson.co.uk>

We don't need to incur the overhead of checking whether the object is
pinned prior to changing its madvise. If the object is pinned, the
madvise will not take effect until it is unpinned and so we cannot free
the pages being pointed at by hardware. Marking a pinned object with
allocated pages as DONTNEED will not trigger any undue warnings. The check
is therefore superfluous, and by removing it we can remove a linear walk
over all the vma the object has.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 67e9100..940fd24 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4369,11 +4369,6 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 		goto unlock;
 	}
 
-	if (i915_gem_obj_is_pinned(obj)) {
-		ret = -EINVAL;
-		goto out;
-	}
-
 	if (obj->pages &&
 	    obj->tiling_mode != I915_TILING_NONE &&
 	    dev_priv->quirks & QUIRK_PIN_SWIZZLED_PAGES) {
@@ -4392,7 +4387,6 @@ i915_gem_madvise_ioctl(struct drm_device *dev, void *data,
 
 	args->retained = obj->madv != __I915_MADV_PURGED;
 
-out:
 	drm_gem_object_unreference(&obj->base);
 unlock:
 	mutex_unlock(&dev->struct_mutex);
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 08/11] drm/i915: Extend GET_APERTURE ioctl to report available map space

[Rodrigo Vivi]

From: Chris Wilson <chris@chris-wilson.co.uk>

When constructing a batchbuffer, it is sometimes crucial to know the
largest hole into which we can fit a fenceable buffer (for example when
handling very large objects on gen2 and gen3). This depends on the
fragmentation of pinned buffers inside the aperture, a question only the
kernel can easily answer.

This patch extends the current DRM_I915_GEM_GET_APERTURE ioctl to
include a couple of new fields in its reply to userspace - the total
amount of space available in the mappable region of the aperture and
also the single largest block available.

This is not quite what userspace wants to answer the question of whether
this batch will fit as fences are also required to meet severe alignment
constraints within the batch. For this purpose, a third conservative
estimate of largest fence available is also provided. For when userspace
needs more than one batch, we also provide the culmulative space
available for fences such that it has some additional guidance to how
much space it could allocate to fences. Conservatism still wins.

The patch also adds a debugfs file for convenient testing and reporting.

v2: The first object cannot end at offset 0, so we can use last==0 to
detect the empty list.

v3: Expand all values to 64bit, just in case.
    Report total mappable aperture size for userspace that cannot easily
    determine it by inspecting the PCI device.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c |  27 +++++++++
 drivers/gpu/drm/i915/i915_gem.c     | 110 ++++++++++++++++++++++++++++++++++--
 include/uapi/drm/i915_drm.h         |  25 ++++++++
 3 files changed, 158 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index e515aad..9619270 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -498,6 +498,32 @@ static int i915_gem_object_info(struct seq_file *m, void* data)
 	return 0;
 }
 
+static int i915_gem_aperture_info(struct seq_file *m, void *data)
+{
+	struct drm_info_node *node = m->private;
+	struct drm_i915_gem_get_aperture arg;
+	int ret;
+
+	ret = i915_gem_get_aperture_ioctl(node->minor->dev, &arg, NULL);
+	if (ret)
+		return ret;
+
+	seq_printf(m, "Total size of the GTT: %llu bytes\n",
+		   arg.aper_size);
+	seq_printf(m, "Available space in the GTT: %llu bytes\n",
+		   arg.aper_available_size);
+	seq_printf(m, "Available space in the mappable aperture: %llu bytes\n",
+		   arg.map_available_size);
+	seq_printf(m, "Single largest space in the mappable aperture: %llu bytes\n",
+		   arg.map_largest_size);
+	seq_printf(m, "Available space for fences: %llu bytes\n",
+		   arg.fence_available_size);
+	seq_printf(m, "Single largest fence available: %llu bytes\n",
+		   arg.fence_largest_size);
+
+	return 0;
+}
+
 static int i915_gem_gtt_info(struct seq_file *m, void *data)
 {
 	struct drm_info_node *node = m->private;
@@ -4395,6 +4421,7 @@ static int i915_debugfs_create(struct dentry *root,
 static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_capabilities", i915_capabilities, 0},
 	{"i915_gem_objects", i915_gem_object_info, 0},
+	{"i915_gem_aperture", i915_gem_aperture_info, 0},
 	{"i915_gem_gtt", i915_gem_gtt_info, 0},
 	{"i915_gem_pinned", i915_gem_gtt_info, 0, (void *) PINNED_LIST},
 	{"i915_gem_active", i915_gem_object_list_info, 0, (void *) ACTIVE_LIST},
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 940fd24..8c4a6e1 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -31,6 +31,7 @@
 #include "i915_drv.h"
 #include "i915_trace.h"
 #include "intel_drv.h"
+#include <linux/list_sort.h>
 #include <linux/oom.h>
 #include <linux/shmem_fs.h>
 #include <linux/slab.h>
@@ -159,6 +160,49 @@ i915_gem_object_is_inactive(struct drm_i915_gem_object *obj)
 	return i915_gem_obj_bound_any(obj) && !obj->active;
 }
 
+static int obj_rank_by_ggtt(void *priv,
+			    struct list_head *A,
+			    struct list_head *B)
+{
+	struct drm_i915_gem_object *a = list_entry(A,typeof(*a), obj_exec_link);
+	struct drm_i915_gem_object *b = list_entry(B,typeof(*b), obj_exec_link);
+
+	return i915_gem_obj_ggtt_offset(a) - i915_gem_obj_ggtt_offset(b);
+}
+
+static u32 __fence_size(struct drm_i915_private *dev_priv, u32 start, u32 end)
+{
+	u32 size = end - start;
+	u32 fence_size;
+
+	if (INTEL_INFO(dev_priv)->gen < 4) {
+		u32 fence_max;
+		u32 fence_next;
+
+		if (IS_GEN3(dev_priv)) {
+			fence_max = I830_FENCE_MAX_SIZE_VAL << 20;
+			fence_next = 1024*1024;
+		} else {
+			fence_max = I830_FENCE_MAX_SIZE_VAL << 19;
+			fence_next = 512*1024;
+		}
+
+		fence_max = min(fence_max, size);
+		fence_size = 0;
+		while (fence_next <= fence_max) {
+			u32 base = ALIGN(start, fence_next);
+			if (base + fence_next > end)
+				break;
+
+			fence_size = fence_next;
+			fence_next <<= 1;
+		}
+	} else
+		fence_size = size;
+
+	return fence_size;
+}
+
 int
 i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
 			    struct drm_file *file)
@@ -166,17 +210,75 @@ i915_gem_get_aperture_ioctl(struct drm_device *dev, void *data,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_i915_gem_get_aperture *args = data;
 	struct drm_i915_gem_object *obj;
-	size_t pinned;
+	struct list_head map_list;
+	const u32 map_limit = dev_priv->gtt.mappable_end;
+	size_t pinned, map_space, map_largest, fence_space, fence_largest;
+	u32 last, size;
+
+	INIT_LIST_HEAD(&map_list);
 
 	pinned = 0;
+	map_space = map_largest = 0;
+	fence_space = fence_largest = 0;
+
 	mutex_lock(&dev->struct_mutex);
-	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list)
-		if (i915_gem_obj_is_pinned(obj))
-			pinned += i915_gem_obj_ggtt_size(obj);
+	list_for_each_entry(obj, &dev_priv->mm.bound_list, global_list) {
+		struct i915_vma *vma = i915_gem_obj_to_ggtt(obj);
+
+		if (vma == NULL || !vma->pin_count)
+			continue;
+
+		pinned += vma->node.size;
+
+		if (vma->node.start < map_limit)
+			list_add(&obj->obj_exec_link, &map_list);
+	}
+
+	last = 0;
+	list_sort(NULL, &map_list, obj_rank_by_ggtt);
+	while (!list_empty(&map_list)) {
+		struct i915_vma *vma;
+
+		obj = list_first_entry(&map_list, typeof(*obj), obj_exec_link);
+		list_del_init(&obj->obj_exec_link);
+
+		vma = i915_gem_obj_to_ggtt(obj);
+		if (last == 0)
+			goto skip_first;
+
+		size = vma->node.start - last;
+		if (size > map_largest)
+			map_largest = size;
+		map_space += size;
+
+		size = __fence_size(dev_priv, last, vma->node.start);
+		if (size > fence_largest)
+			fence_largest = size;
+		fence_space += size;
+
+skip_first:
+		last = vma->node.start + vma->node.size;
+	}
+	if (last < map_limit) {
+		size = map_limit - last;
+		if (size > map_largest)
+			map_largest = size;
+		map_space += size;
+
+		size = __fence_size(dev_priv, last, map_limit);
+		if (size > fence_largest)
+			fence_largest = size;
+		fence_space += size;
+	}
 	mutex_unlock(&dev->struct_mutex);
 
 	args->aper_size = dev_priv->gtt.base.total;
 	args->aper_available_size = args->aper_size - pinned;
+	args->map_available_size = map_space;
+	args->map_largest_size = map_largest;
+	args->map_total_size = dev_priv->gtt.mappable_end;
+	args->fence_available_size = fence_space;
+	args->fence_largest_size = fence_largest;
 
 	return 0;
 }
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index d253c85..7972a60 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -905,6 +905,31 @@ struct drm_i915_gem_get_aperture {
 	 * bytes
 	 */
 	__u64 aper_available_size;
+
+	/**
+	 * Total space in the mappable region of the aperture, in bytes
+	 */
+	__u64 map_total_size;
+
+	/**
+	 * Available space in the mappable region of the aperture, in bytes
+	 */
+	__u64 map_available_size;
+
+	/**
+	 * Single largest available region inside the mappable region, in bytes.
+	 */
+	__u64 map_largest_size;
+
+	/**
+	 * Culmulative space available for fences, in bytes
+	 */
+	__u64 fence_available_size;
+
+	/**
+	 * Single largest fenceable region, in bytes.
+	 */
+	__u64 fence_largest_size;
 };
 
 struct drm_i915_get_pipe_from_crtc_id {
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 09/11] drm/i915: Display current hangcheck status in debugfs

[Rodrigo Vivi]

From: Chris Wilson <chris@chris-wilson.co.uk>

For example,

/sys/kernel/debug/dri/0/i915_hangcheck_info:

Hangcheck active, fires in 15887800ms
render ring:
        seqno = -4059 [current -583]
        action = 2
        score = 0
        ACTHD = 1ee8 [current 21f980]
        max ACTHD = 0

v2: Include expiration ETA. Can anyone spot a problem?

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 9619270..5691594 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1245,6 +1245,40 @@ out:
 	return ret;
 }
 
+static int i915_hangcheck_info(struct seq_file *m, void *unused)
+{
+	struct drm_info_node *node = m->private;
+	struct drm_i915_private *dev_priv = to_i915(node->minor->dev);
+	struct intel_engine_cs *ring;
+	int i;
+
+	if (!i915.enable_hangcheck) {
+		seq_printf(m, "Hangcheck disabled\n");
+		return 0;
+	}
+
+	if (timer_pending(&dev_priv->gpu_error.hangcheck_timer)) {
+		seq_printf(m, "Hangcheck active, fires in %dms\n",
+			   jiffies_to_msecs(dev_priv->gpu_error.hangcheck_timer.expires - jiffies));
+	} else
+		seq_printf(m, "Hangcheck inactive\n");
+
+	for_each_ring(ring, dev_priv, i) {
+		seq_printf(m, "%s:\n", ring->name);
+		seq_printf(m, "\tseqno = %d [current %d]\n",
+			   ring->hangcheck.seqno, ring->get_seqno(ring, false));
+		seq_printf(m, "\taction = %d\n", ring->hangcheck.action);
+		seq_printf(m, "\tscore = %d\n", ring->hangcheck.score);
+		seq_printf(m, "\tACTHD = 0x%08llx [current 0x%08llx]\n",
+			   (long long)ring->hangcheck.acthd,
+			   (long long)intel_ring_get_active_head(ring));
+		seq_printf(m, "\tmax ACTHD = 0x%08llx\n",
+			   (long long)ring->hangcheck.max_acthd);
+	}
+
+	return 0;
+}
+
 static int ironlake_drpc_info(struct seq_file *m)
 {
 	struct drm_info_node *node = m->private;
@@ -4438,6 +4472,7 @@ static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_gem_hws_vebox", i915_hws_info, 0, (void *)VECS},
 	{"i915_gem_batch_pool", i915_gem_batch_pool_info, 0},
 	{"i915_frequency_info", i915_frequency_info, 0},
+	{"i915_hangcheck_info", i915_hangcheck_info, 0},
 	{"i915_drpc_info", i915_drpc_info, 0},
 	{"i915_emon_status", i915_emon_status, 0},
 	{"i915_ring_freq_table", i915_ring_freq_table, 0},
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 10/11] drm/i915/vlv: check port in infoframe_enabled v2

[Rodrigo Vivi]

From: Jesse Barnes <jbarnes@virtuousgeek.org>

Same as IBX and G4x, they all share the same genetic material.

v2: we all need a bit more port in our lives

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/intel_hdmi.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 3abc200..62606e6 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -323,10 +323,15 @@ static bool vlv_infoframe_enabled(struct drm_encoder *encoder)
 	struct drm_device *dev = encoder->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->crtc);
+	struct intel_digital_port *intel_dig_port = enc_to_dig_port(encoder);
 	int reg = VLV_TVIDEO_DIP_CTL(intel_crtc->pipe);
 	u32 val = I915_READ(reg);
+	u32 port = intel_dig_port->port;
 
-	return val & VIDEO_DIP_ENABLE;
+	if (port == (val & VIDEO_DIP_PORT_MASK))
+		return val & VIDEO_DIP_ENABLE;
+
+	return false;
 }
 
 static void hsw_write_infoframe(struct drm_encoder *encoder,
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 11/11] drm/i915: vlv: fix save/restore of GFX_MAX_REQ_COUNT reg

[Rodrigo Vivi]

From: Imre Deak <imre.deak@intel.com>

Due this typo we don't save/restore the GFX_MAX_REQ_COUNT register across
suspend/resume, so fix this.

This was introduced in

commit ddeea5b0c36f3665446518c609be91f9336ef674
Author: Imre Deak <imre.deak@intel.com>
Date:   Mon May 5 15:19:56 2014 +0300

    drm/i915: vlv: add runtime PM support

I noticed this only by reading the code. To my knowledge it shouldn't
cause any real problems at the moment, since the power well backing this
register remains on across a runtime s/r. This may change once
system-wide s0ix functionality is enabled in the kernel.

v2:
- resend after a missing git add -u :/

Signed-off-by: Imre Deak <imre.deak@intel.com>
Tested-By: PRC QA PRTS (Patch Regression Test System Contact: shuang.he@intel.com)
Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f702921..7dc8dae 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1031,7 +1031,7 @@ static void vlv_save_gunit_s0ix_state(struct drm_i915_private *dev_priv)
 		s->lra_limits[i] = I915_READ(GEN7_LRA_LIMITS_BASE + i * 4);
 
 	s->media_max_req_count	= I915_READ(GEN7_MEDIA_MAX_REQ_COUNT);
-	s->gfx_max_req_count	= I915_READ(GEN7_MEDIA_MAX_REQ_COUNT);
+	s->gfx_max_req_count	= I915_READ(GEN7_GFX_MAX_REQ_COUNT);
 
 	s->render_hwsp		= I915_READ(RENDER_HWS_PGA_GEN7);
 	s->ecochk		= I915_READ(GAM_ECOCHK);
@@ -1112,7 +1112,7 @@ static void vlv_restore_gunit_s0ix_state(struct drm_i915_private *dev_priv)
 		I915_WRITE(GEN7_LRA_LIMITS_BASE + i * 4, s->lra_limits[i]);
 
 	I915_WRITE(GEN7_MEDIA_MAX_REQ_COUNT, s->media_max_req_count);
-	I915_WRITE(GEN7_MEDIA_MAX_REQ_COUNT, s->gfx_max_req_count);
+	I915_WRITE(GEN7_GFX_MAX_REQ_COUNT, s->gfx_max_req_count);
 
 	I915_WRITE(RENDER_HWS_PGA_GEN7,	s->render_hwsp);
 	I915_WRITE(GAM_ECOCHK,		s->ecochk);
-- 
1.9.3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 03/11] drm/i915: Move the ban period onto the context

[Mika Kuoppala]

Rodrigo Vivi <rodrigo.vivi@intel.com> writes:

> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> This will allow us to set per-file, or even per-context, periods in the
> future.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h         | 5 +++++
>  drivers/gpu/drm/i915/i915_gem.c         | 3 ++-
>  drivers/gpu/drm/i915/i915_gem_context.c | 2 ++
>  3 files changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 921e4c5..3749415 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -678,6 +678,11 @@ struct i915_ctx_hang_stats {
>  	/* Time when this context was last blamed for a GPU reset */
>  	unsigned long guilty_ts;
>  
> +	/* If the contexts causes a second GPU hang within this time,
> +	 * it is permanently banned from submitting any more work.
> +	 */
> +	unsigned long ban_period_seconds;
> +
>  	/* This context is banned to submit more work */
>  	bool banned;
>  };
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 2b6ecfd..67e9100 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -2547,7 +2547,8 @@ static bool i915_context_is_banned(struct drm_i915_private *dev_priv,
>  	if (ctx->hang_stats.banned)
>  		return true;
>  
> -	if (elapsed <= DRM_I915_CTX_BAN_PERIOD) {
> +	if (ctx->hang_stats.ban_period_seconds &&
> +	    elapsed <= ctx->hang_stats.ban_period_seconds) {
>  		if (!i915_gem_context_is_default(ctx)) {
>  			DRM_DEBUG("context hanging too fast, banning!\n");
>  			return true;
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 9b23fb1..bf9778e 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -222,6 +222,8 @@ __create_hw_context(struct drm_device *dev,
>  	 * is no remap info, it will be a NOP. */
>  	ctx->remap_slice = (1 << NUM_L3_SLICES(dev)) - 1;
>  
> +	ctx->hang_stats.ban_period_seconds = DRM_I915_CTX_BAN_PERIOD;
> +
>  	return ctx;
>  
>  err_out:
> -- 
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/11] drm/i915: Add ioctl to set per-context parameters

[Mika Kuoppala]

Rodrigo Vivi <rodrigo.vivi@intel.com> writes:

> From: Chris Wilson <chris@chris-wilson.co.uk>
>
> Sometimes we wish to tweak how an individual context behaves. Since we
> always create a context for every filp, this means that individual
> processes can fine tune their behaviour even if they do not explicitly
> create a context.
>
> The first example parameter here is to enable multi-process GPU testing,
> but the interface should be able to cope with passing arbitrarily complex
> parameters.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_dma.c         |  2 +
>  drivers/gpu/drm/i915/i915_drv.h         |  4 ++
>  drivers/gpu/drm/i915/i915_gem_context.c | 69 +++++++++++++++++++++++++++++++++
>  include/uapi/drm/i915_drm.h             | 12 ++++++
>  4 files changed, 87 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 126a36f..3c3f33f 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1066,6 +1066,8 @@ const struct drm_ioctl_desc i915_ioctls[] = {
>  	DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_get_reset_stats_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
>  	DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
> +	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
> +	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
>  };
>  
>  int i915_max_ioctl = ARRAY_SIZE(i915_ioctls);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 3749415..9ae515e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2898,6 +2898,10 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
>  				  struct drm_file *file);
>  int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
>  				   struct drm_file *file);
> +int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
> +				    struct drm_file *file_priv);
> +int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
> +				    struct drm_file *file_priv);
>  
>  /* i915_gem_evict.c */
>  int __must_check i915_gem_evict_something(struct drm_device *dev,
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index bf9778e..8603bf4 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -794,3 +794,72 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
>  	DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
>  	return 0;
>  }
> +
> +int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
> +				    struct drm_file *file)
> +{
> +	struct drm_i915_file_private *file_priv = file->driver_priv;
> +	struct drm_i915_gem_context_param *args = data;
> +	struct intel_context *ctx;
> +	int ret;
> +
> +	ret = i915_mutex_lock_interruptible(dev);
> +	if (ret)
> +		return ret;
> +
> +	ctx = i915_gem_context_get(file_priv, args->ctx_id);
> +	if (IS_ERR(ctx)) {
> +		mutex_unlock(&dev->struct_mutex);
> +		return PTR_ERR(ctx);
> +	}
> +
> +	args->size = 0;
> +	switch (args->param) {
> +	case I915_CONTEXT_PARAM_BAN_PERIOD:
> +		args->value = ctx->hang_stats.ban_period_seconds;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +	mutex_unlock(&dev->struct_mutex);
> +
> +	return ret;
> +}
> +
> +int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
> +				    struct drm_file *file)
> +{
> +	struct drm_i915_file_private *file_priv = file->driver_priv;
> +	struct drm_i915_gem_context_param *args = data;
> +	struct intel_context *ctx;
> +	int ret;
> +
> +	ret = i915_mutex_lock_interruptible(dev);
> +	if (ret)
> +		return ret;
> +
> +	ctx = i915_gem_context_get(file_priv, args->ctx_id);
> +	if (IS_ERR(ctx)) {
> +		mutex_unlock(&dev->struct_mutex);
> +		return PTR_ERR(ctx);
> +	}
> +
> +	switch (args->param) {
> +	case I915_CONTEXT_PARAM_BAN_PERIOD:
> +		if (args->size)
> +			ret = -EINVAL;
> +		else if (args->value < ctx->hang_stats.ban_period_seconds &&
> +			 !capable(CAP_SYS_ADMIN))
> +			ret = -EPERM;
> +		else
> +			ctx->hang_stats.ban_period_seconds = args->value;
> +		break;
> +	default:
> +		ret = -EINVAL;
> +		break;
> +	}
> +	mutex_unlock(&dev->struct_mutex);
> +
> +	return ret;
> +}
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index fa99129..d253c85 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -224,6 +224,8 @@ typedef struct _drm_i915_sarea {
>  #define DRM_I915_REG_READ		0x31
>  #define DRM_I915_GET_RESET_STATS	0x32
>  #define DRM_I915_GEM_USERPTR		0x33
> +#define DRM_I915_GEM_CONTEXT_GETPARAM	0x34
> +#define DRM_I915_GEM_CONTEXT_SETPARAM	0x35
>  
>  #define DRM_IOCTL_I915_INIT		DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
>  #define DRM_IOCTL_I915_FLUSH		DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
> @@ -275,6 +277,8 @@ typedef struct _drm_i915_sarea {
>  #define DRM_IOCTL_I915_REG_READ			DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_REG_READ, struct drm_i915_reg_read)
>  #define DRM_IOCTL_I915_GET_RESET_STATS		DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GET_RESET_STATS, struct drm_i915_reset_stats)
>  #define DRM_IOCTL_I915_GEM_USERPTR			DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_USERPTR, struct drm_i915_gem_userptr)
> +#define DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM	DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_GETPARAM, struct drm_i915_gem_context_param)
> +#define DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM	DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_SETPARAM, struct drm_i915_gem_context_param)
>  
>  /* Allow drivers to submit batchbuffers directly to hardware, relying
>   * on the security mechanisms provided by hardware.
> @@ -1080,4 +1084,12 @@ struct drm_i915_gem_userptr {
>  	__u32 handle;
>  };
>  
> +struct drm_i915_gem_context_param {
> +	__u32 ctx_id;
> +	__u32 size;
> +	__u64 param;
> +#define I915_CONTEXT_PARAM_BAN_PERIOD 0x1
> +	__u64 value;
> +};
> +
>  #endif /* _UAPI_I915_DRM_H_ */
> -- 
> 1.9.3
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/11] drm/i915: Add ioctl to set per-context parameters

[Daniel Vetter]

On Wed, Jan 07, 2015 at 01:57:04PM +0200, Mika Kuoppala wrote:
> Rodrigo Vivi <rodrigo.vivi@intel.com> writes:
> 
> > From: Chris Wilson <chris@chris-wilson.co.uk>
> >
> > Sometimes we wish to tweak how an individual context behaves. Since we
> > always create a context for every filp, this means that individual
> > processes can fine tune their behaviour even if they do not explicitly
> > create a context.
> >
> > The first example parameter here is to enable multi-process GPU testing,
> > but the interface should be able to cope with passing arbitrarily complex
> > parameters.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

Do we have the igt for this already somewhere? I've merged the prep patch
meanwhile.

Thanks, Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/i915_dma.c         |  2 +
> >  drivers/gpu/drm/i915/i915_drv.h         |  4 ++
> >  drivers/gpu/drm/i915/i915_gem_context.c | 69 +++++++++++++++++++++++++++++++++
> >  include/uapi/drm/i915_drm.h             | 12 ++++++
> >  4 files changed, 87 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index 126a36f..3c3f33f 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1066,6 +1066,8 @@ const struct drm_ioctl_desc i915_ioctls[] = {
> >  	DRM_IOCTL_DEF_DRV(I915_REG_READ, i915_reg_read_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
> >  	DRM_IOCTL_DEF_DRV(I915_GET_RESET_STATS, i915_get_reset_stats_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
> >  	DRM_IOCTL_DEF_DRV(I915_GEM_USERPTR, i915_gem_userptr_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
> > +	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_GETPARAM, i915_gem_context_getparam_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
> > +	DRM_IOCTL_DEF_DRV(I915_GEM_CONTEXT_SETPARAM, i915_gem_context_setparam_ioctl, DRM_UNLOCKED|DRM_RENDER_ALLOW),
> >  };
> >  
> >  int i915_max_ioctl = ARRAY_SIZE(i915_ioctls);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 3749415..9ae515e 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2898,6 +2898,10 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
> >  				  struct drm_file *file);
> >  int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
> >  				   struct drm_file *file);
> > +int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
> > +				    struct drm_file *file_priv);
> > +int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
> > +				    struct drm_file *file_priv);
> >  
> >  /* i915_gem_evict.c */
> >  int __must_check i915_gem_evict_something(struct drm_device *dev,
> > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> > index bf9778e..8603bf4 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_context.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> > @@ -794,3 +794,72 @@ int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
> >  	DRM_DEBUG_DRIVER("HW context %d destroyed\n", args->ctx_id);
> >  	return 0;
> >  }
> > +
> > +int i915_gem_context_getparam_ioctl(struct drm_device *dev, void *data,
> > +				    struct drm_file *file)
> > +{
> > +	struct drm_i915_file_private *file_priv = file->driver_priv;
> > +	struct drm_i915_gem_context_param *args = data;
> > +	struct intel_context *ctx;
> > +	int ret;
> > +
> > +	ret = i915_mutex_lock_interruptible(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ctx = i915_gem_context_get(file_priv, args->ctx_id);
> > +	if (IS_ERR(ctx)) {
> > +		mutex_unlock(&dev->struct_mutex);
> > +		return PTR_ERR(ctx);
> > +	}
> > +
> > +	args->size = 0;
> > +	switch (args->param) {
> > +	case I915_CONTEXT_PARAM_BAN_PERIOD:
> > +		args->value = ctx->hang_stats.ban_period_seconds;
> > +		break;
> > +	default:
> > +		ret = -EINVAL;
> > +		break;
> > +	}
> > +	mutex_unlock(&dev->struct_mutex);
> > +
> > +	return ret;
> > +}
> > +
> > +int i915_gem_context_setparam_ioctl(struct drm_device *dev, void *data,
> > +				    struct drm_file *file)
> > +{
> > +	struct drm_i915_file_private *file_priv = file->driver_priv;
> > +	struct drm_i915_gem_context_param *args = data;
> > +	struct intel_context *ctx;
> > +	int ret;
> > +
> > +	ret = i915_mutex_lock_interruptible(dev);
> > +	if (ret)
> > +		return ret;
> > +
> > +	ctx = i915_gem_context_get(file_priv, args->ctx_id);
> > +	if (IS_ERR(ctx)) {
> > +		mutex_unlock(&dev->struct_mutex);
> > +		return PTR_ERR(ctx);
> > +	}
> > +
> > +	switch (args->param) {
> > +	case I915_CONTEXT_PARAM_BAN_PERIOD:
> > +		if (args->size)
> > +			ret = -EINVAL;
> > +		else if (args->value < ctx->hang_stats.ban_period_seconds &&
> > +			 !capable(CAP_SYS_ADMIN))
> > +			ret = -EPERM;
> > +		else
> > +			ctx->hang_stats.ban_period_seconds = args->value;
> > +		break;
> > +	default:
> > +		ret = -EINVAL;
> > +		break;
> > +	}
> > +	mutex_unlock(&dev->struct_mutex);
> > +
> > +	return ret;
> > +}
> > diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> > index fa99129..d253c85 100644
> > --- a/include/uapi/drm/i915_drm.h
> > +++ b/include/uapi/drm/i915_drm.h
> > @@ -224,6 +224,8 @@ typedef struct _drm_i915_sarea {
> >  #define DRM_I915_REG_READ		0x31
> >  #define DRM_I915_GET_RESET_STATS	0x32
> >  #define DRM_I915_GEM_USERPTR		0x33
> > +#define DRM_I915_GEM_CONTEXT_GETPARAM	0x34
> > +#define DRM_I915_GEM_CONTEXT_SETPARAM	0x35
> >  
> >  #define DRM_IOCTL_I915_INIT		DRM_IOW( DRM_COMMAND_BASE + DRM_I915_INIT, drm_i915_init_t)
> >  #define DRM_IOCTL_I915_FLUSH		DRM_IO ( DRM_COMMAND_BASE + DRM_I915_FLUSH)
> > @@ -275,6 +277,8 @@ typedef struct _drm_i915_sarea {
> >  #define DRM_IOCTL_I915_REG_READ			DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_REG_READ, struct drm_i915_reg_read)
> >  #define DRM_IOCTL_I915_GET_RESET_STATS		DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GET_RESET_STATS, struct drm_i915_reset_stats)
> >  #define DRM_IOCTL_I915_GEM_USERPTR			DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_USERPTR, struct drm_i915_gem_userptr)
> > +#define DRM_IOCTL_I915_GEM_CONTEXT_GETPARAM	DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_GETPARAM, struct drm_i915_gem_context_param)
> > +#define DRM_IOCTL_I915_GEM_CONTEXT_SETPARAM	DRM_IOWR (DRM_COMMAND_BASE + DRM_I915_GEM_CONTEXT_SETPARAM, struct drm_i915_gem_context_param)
> >  
> >  /* Allow drivers to submit batchbuffers directly to hardware, relying
> >   * on the security mechanisms provided by hardware.
> > @@ -1080,4 +1084,12 @@ struct drm_i915_gem_userptr {
> >  	__u32 handle;
> >  };
> >  
> > +struct drm_i915_gem_context_param {
> > +	__u32 ctx_id;
> > +	__u32 size;
> > +	__u64 param;
> > +#define I915_CONTEXT_PARAM_BAN_PERIOD 0x1
> > +	__u64 value;
> > +};
> > +
> >  #endif /* _UAPI_I915_DRM_H_ */
> > -- 
> > 1.9.3
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/11] drm/i915: Add ioctl to set per-context parameters

[Chris Wilson]

On Wed, Jan 07, 2015 at 02:08:46PM +0100, Daniel Vetter wrote:
> On Wed, Jan 07, 2015 at 01:57:04PM +0200, Mika Kuoppala wrote:
> > Rodrigo Vivi <rodrigo.vivi@intel.com> writes:
> > 
> > > From: Chris Wilson <chris@chris-wilson.co.uk>
> > >
> > > Sometimes we wish to tweak how an individual context behaves. Since we
> > > always create a context for every filp, this means that individual
> > > processes can fine tune their behaviour even if they do not explicitly
> > > create a context.
> > >
> > > The first example parameter here is to enable multi-process GPU testing,
> > > but the interface should be able to cope with passing arbitrarily complex
> > > parameters.
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
> > 
> > Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> Do we have the igt for this already somewhere? I've merged the prep patch
> meanwhile.

I have a bunch of igt that use this for hangcheck races. Patches were
posted around the same time and you were happy enough with them.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 04/11] drm/i915: Add ioctl to set per-context parameters

[Mika Kuoppala]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> On Wed, Jan 07, 2015 at 02:08:46PM +0100, Daniel Vetter wrote:
>> On Wed, Jan 07, 2015 at 01:57:04PM +0200, Mika Kuoppala wrote:
>> > Rodrigo Vivi <rodrigo.vivi@intel.com> writes:
>> > 
>> > > From: Chris Wilson <chris@chris-wilson.co.uk>
>> > >
>> > > Sometimes we wish to tweak how an individual context behaves. Since we
>> > > always create a context for every filp, this means that individual
>> > > processes can fine tune their behaviour even if they do not explicitly
>> > > create a context.
>> > >
>> > > The first example parameter here is to enable multi-process GPU testing,
>> > > but the interface should be able to cope with passing arbitrarily complex
>> > > parameters.
>> > >
>> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > > Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> > 
>> > Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
>> 
>> Do we have the igt for this already somewhere? I've merged the prep patch
>> meanwhile.
>
> I have a bunch of igt that use this for hangcheck races. Patches were
> posted around the same time and you were happy enough with them.

On top of those I have additional stuff for tests/gem_reset_stats.

-Mika
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] tests/gem_reset_stats: add tests for ban period ioctl

[Mika Kuoppala]

Test parameter set/get for ban periods.
Test actual impact on banning.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 tests/gem_reset_stats.c | 179 ++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 179 insertions(+)

diff --git a/tests/gem_reset_stats.c b/tests/gem_reset_stats.c
index ab8728a..fdbc84b 100644
--- a/tests/gem_reset_stats.c
+++ b/tests/gem_reset_stats.c
@@ -1058,6 +1058,174 @@ static void defer_hangcheck(int ring_num)
 	close(fd);
 }
 
+static bool was_banned_in_period(int fd, int ctx, int seconds)
+{
+	int h1,h2,h3,h4;
+	bool banned;
+
+	h1 = inject_hang_no_ban_error(fd, ctx);
+	igt_assert(h1 >= 0);
+
+	sleep(seconds);
+
+	h2 = exec_valid(fd, ctx);
+	igt_assert(h2 >= 0);
+
+	h3 = inject_hang_no_ban_error(fd, ctx);
+	igt_assert(h3 >= 0);
+
+	gem_sync(fd, h3);
+
+	h4 = exec_valid(fd, ctx);
+	banned = (h4 == -EIO);
+
+	gem_close(fd, h1);
+	gem_close(fd, h2);
+	gem_close(fd, h3);
+	if (h4 >= 0)
+		gem_close(fd, h4);
+
+	return banned;
+}
+
+static int get_ban_period(int fd, int ctx)
+{
+	struct local_i915_gem_context_param p;
+
+	p.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
+	p.size = rand();
+	p.context = rand();
+	if (p.context == ctx)
+		p.context = ctx + 1;
+	p.value = ((uint64_t)rand() << 32) | rand();
+
+	igt_assert(gem_context_get_param(fd, &p) == -1);
+	igt_assert(errno == ENOENT);
+
+	p.context = ctx;
+	p.param = 0xdeadf00d;
+
+	igt_assert(gem_context_get_param(fd, &p) == -1);
+	igt_assert(errno == EINVAL);
+
+	p.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
+	igt_assert(gem_context_get_param(fd, &p) == 0);
+
+	return p.value;
+}
+
+static int _set_ban_period(int fd, struct local_i915_gem_context_param *p)
+{
+	int r;
+
+	r = gem_context_set_param(fd, p);
+
+	if (r == -1)
+		return errno;
+
+	return 0;
+}
+
+static int set_ban_period(int fd, int ctx, int period)
+{
+	struct local_i915_gem_context_param p;
+
+	p.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
+	p.size = 0;
+	p.context = ctx;
+	p.value = period;
+	return _set_ban_period(fd, &p);
+}
+
+static void test_ban_period_params(bool new_ctx)
+{
+	struct local_i915_gem_context_param p;
+	int fd, ctx, period;
+
+	fd = drm_open_any();
+	igt_assert(fd >= 0);
+
+	igt_skip_on(gem_context_has_param(fd, LOCAL_CONTEXT_PARAM_BAN_PERIOD)
+		    == 0);
+
+	if (new_ctx)
+		ctx = context_create(fd);
+	else
+		ctx = 0;
+
+	period = get_ban_period(fd, ctx);
+	igt_assert(period > 2);
+
+	p.param = LOCAL_CONTEXT_PARAM_BAN_PERIOD;
+	p.size = 0xdeadf00d;
+	p.context = ctx;
+	p.value = ((uint64_t)rand() << 32) | rand();
+
+	igt_assert(_set_ban_period(fd, &p) == EINVAL);
+
+	p.size = 0;
+	p.context = 0xdeadf00d;
+
+	igt_assert(_set_ban_period(fd, &p) == ENOENT);
+
+	p.size = 0;
+	p.context = ctx;
+	p.value = period;
+
+	igt_fork(child, 1) {
+		igt_drop_root();
+		p.value -= 2;
+
+		igt_assert(_set_ban_period(fd, &p) == EPERM);
+	}
+
+	igt_assert(_set_ban_period(fd, &p) == 0);
+
+	p.size = 0;
+	p.context = ctx;
+	p.value = period + 1;
+
+	igt_assert(_set_ban_period(fd, &p) == 0);
+}
+
+static void test_ban_period(bool new_ctx)
+{
+	int fd, ctx, period;
+
+	fd = drm_open_any();
+	igt_assert(fd >= 0);
+
+	igt_skip_on(gem_context_has_param(fd, LOCAL_CONTEXT_PARAM_BAN_PERIOD)
+		    == 0);
+
+	if (new_ctx)
+		ctx = context_create(fd);
+	else
+		ctx = 0;
+
+	period = get_ban_period(fd, ctx);
+	igt_assert(period > 2);
+
+	period += 2;
+
+	igt_assert(set_ban_period(fd, ctx, period) == 0);
+
+	igt_assert(was_banned_in_period(fd, ctx, period + 2) == false);
+
+	igt_assert(set_ban_period(fd, ctx, 0) == 0);
+
+	igt_assert(was_banned_in_period(fd, ctx, 0) == false);
+
+	/* We just hanged, wait for a while */
+	sleep(period + 2);
+
+	igt_assert(set_ban_period(fd, ctx, period) == 0);
+
+	igt_assert(was_banned_in_period(fd, ctx, period  / 4) == true);
+
+	close(fd);
+}
+
 static bool gem_has_hw_contexts(int fd)
 {
 	struct local_drm_i915_gem_context_create create;
@@ -1202,5 +1370,16 @@ igt_main
 		igt_subtest_f("defer-hangcheck-%s", name)
 			RUN_TEST(defer_hangcheck(i));
 
+		igt_subtest_f("ban-period-params-%s", name)
+			RUN_TEST(test_ban_period_params(false));
+
+		igt_subtest_f("ban-period-params-ctx-%s", name)
+			RUN_CTX_TEST(test_ban_period_params(true));
+
+		igt_subtest_f("ban-period-%s", name)
+			RUN_TEST(test_ban_period(false));
+
+		igt_subtest_f("ban-period-ctx-%s", name)
+			RUN_CTX_TEST(test_ban_period(true));
 	}
 }
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] tests/gem_reset_stats: add tests for ban period ioctl

[Chris Wilson]

On Wed, Jan 07, 2015 at 04:21:24PM +0200, Mika Kuoppala wrote:
> Test parameter set/get for ban periods.
> Test actual impact on banning.

Nice. These are more like the tests Daniel was thinking of when he asked
his question. Could you please separate out the negative parameter
testing to a new gem_ctx_param set of tests? Testing whether changing
the ban period has any effect seems sensible to keep in gem_reset_stats.

> +	igt_skip_on(gem_context_has_param(fd, LOCAL_CONTEXT_PARAM_BAN_PERIOD)
> +		    == 0);

is better as

igt_require(gem_context_has_param(fd, LOCAL_CONTEXT_PARAM_BAN_PERIOD);

as that produces a saner error message.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] tests/gem_reset_stats: add tests for ban period ioctl

[Daniel Vetter]

On Wed, Jan 07, 2015 at 02:21:54PM +0000, Chris Wilson wrote:
> On Wed, Jan 07, 2015 at 04:21:24PM +0200, Mika Kuoppala wrote:
> > Test parameter set/get for ban periods.
> > Test actual impact on banning.
> 
> Nice. These are more like the tests Daniel was thinking of when he asked
> his question. Could you please separate out the negative parameter
> testing to a new gem_ctx_param set of tests? Testing whether changing
> the ban period has any effect seems sensible to keep in gem_reset_stats.

Imo the negative parameter tests are fine as-is, I've used the current
naming at least for the testcase tag. Usuaully I put an -invalid-
somewhere in the name though, but that's optional. Anyway patch merged,
thanks for patch, testcase& review.
-Daniel

> 
> > +	igt_skip_on(gem_context_has_param(fd, LOCAL_CONTEXT_PARAM_BAN_PERIOD)
> > +		    == 0);
> 
> is better as
> 
> igt_require(gem_context_has_param(fd, LOCAL_CONTEXT_PARAM_BAN_PERIOD);
> 
> as that produces a saner error message.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx