All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/6] drm/i915: Missed interrupt simulation is no more, tell the world
@ 2018-09-07 11:28 Chris Wilson
  2018-09-07 11:28 ` [PATCH 2/6] drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G Chris Wilson
                   ` (8 more replies)
  0 siblings, 9 replies; 12+ messages in thread
From: Chris Wilson @ 2018-09-07 11:28 UTC (permalink / raw)
  To: intel-gfx

Using the guc, we cannot disable the user interrupt generation as we use
it for driving submission. And from Icelake, we no longer have the
ability to individually mask interrupt generation from each engine,
disabling our ability to fake missed interrupts.

In both cases, report back to userspace that the missed interrupt
generator is no longer available.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 1f7051e97afb..b4744a68cd88 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -4117,6 +4117,17 @@ i915_ring_test_irq_set(void *data, u64 val)
 {
 	struct drm_i915_private *i915 = data;
 
+	/* GuC keeps the user interrupt permanently enabled for submission */
+	if (USES_GUC_SUBMISSION(i915))
+		return -ENODEV;
+
+	/*
+	 * From icl, we can no longer individually mask interrupt generation
+	 * from each engine.
+	 */
+	if (INTEL_GEN(i915) >= 11)
+		return -ENODEV;
+
 	val &= INTEL_INFO(i915)->ring_mask;
 	DRM_DEBUG_DRIVER("Masking interrupts on rings 0x%08llx\n", val);
 
-- 
2.19.0.rc2

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/6] drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G
  2018-09-07 11:28 [PATCH 1/6] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
@ 2018-09-07 11:28 ` Chris Wilson
  2018-09-07 11:28 ` [PATCH 3/6] drm/i915: Limit number of capture objects Chris Wilson
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2018-09-07 11:28 UTC (permalink / raw)
  To: intel-gfx

If the caller supplies more than 4G of objects and than one that has to
be in the low 4G, it is possible for the low 4G to be full before we
attempt to find room for the last object that must be there. As we don't
reorder the two types, every pass hits the same problem and we fail with
ENOSPC. However, if we impose a little bit of ordering between the two
classes of objects, on the second pass we will be able to fit the
special object as we do it first. For setups that only use !48b objects,
we now reverse the order between passes, hopefully making the subsequent
passes more likely to succeed given that we are trying a different
order (rather than repeating the previous pass!)

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 22b4cb775576..d70d142f5338 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -696,6 +696,8 @@ static int eb_reserve(struct i915_execbuffer *eb)
 				list_add(&vma->exec_link, &eb->unbound);
 			else if (flags & __EXEC_OBJECT_NEEDS_MAP)
 				list_add_tail(&vma->exec_link, &eb->unbound);
+			else if (!(flags & EXEC_OBJECT_SUPPORTS_48B_ADDRESS))
+				list_add(&vma->exec_link, &last);
 			else
 				list_add_tail(&vma->exec_link, &last);
 		}
-- 
2.19.0.rc2

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/6] drm/i915: Limit number of capture objects
  2018-09-07 11:28 [PATCH 1/6] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
  2018-09-07 11:28 ` [PATCH 2/6] drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G Chris Wilson
@ 2018-09-07 11:28 ` Chris Wilson
  2018-09-10 13:14   ` Mika Kuoppala
  2018-09-07 11:28 ` [PATCH 4/6] drm/i915: Handle incomplete Z_FINISH for compressed error states Chris Wilson
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 12+ messages in thread
From: Chris Wilson @ 2018-09-07 11:28 UTC (permalink / raw)
  To: intel-gfx

If we fail to allocate an array for a large number of user requested
capture objects, reduce the array size and try to grab at least some of
the objects!

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 20 +++++++++++++-------
 1 file changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index f7f2aa71d8d9..2835cacd0d08 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1365,15 +1365,20 @@ static void request_record_user_bo(struct i915_request *request,
 {
 	struct i915_capture_list *c;
 	struct drm_i915_error_object **bo;
-	long count;
+	long count, max;
 
-	count = 0;
+	max = 0;
 	for (c = request->capture_list; c; c = c->next)
-		count++;
+		max++;
+	if (!max)
+		return;
 
-	bo = NULL;
-	if (count)
-		bo = kcalloc(count, sizeof(*bo), GFP_ATOMIC);
+	bo = kmalloc_array(max, sizeof(*bo), GFP_ATOMIC);
+	if (!bo) {
+		/* If we can't capture everything, try to capture something. */
+		max = min_t(long, max, PAGE_SIZE / sizeof(*bo));
+		bo = kmalloc_array(max, sizeof(*bo), GFP_ATOMIC);
+	}
 	if (!bo)
 		return;
 
@@ -1382,7 +1387,8 @@ static void request_record_user_bo(struct i915_request *request,
 		bo[count] = i915_error_object_create(request->i915, c->vma);
 		if (!bo[count])
 			break;
-		count++;
+		if (++count == max)
+			break;
 	}
 
 	ee->user_bo = bo;
-- 
2.19.0.rc2

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/6] drm/i915: Handle incomplete Z_FINISH for compressed error states
  2018-09-07 11:28 [PATCH 1/6] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
  2018-09-07 11:28 ` [PATCH 2/6] drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G Chris Wilson
  2018-09-07 11:28 ` [PATCH 3/6] drm/i915: Limit number of capture objects Chris Wilson
@ 2018-09-07 11:28 ` Chris Wilson
  2018-09-07 11:28 ` [PATCH 5/6] drm/i915: Clear the error PTE just once on finish Chris Wilson
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2018-09-07 11:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: mika.kuoppala, Chris Wilson, Joonas Lahtinen, stable

The final call to zlib_deflate(Z_FINISH) may require more output
space to be allocated and so needs to re-invoked. Failure to do so in
the current code leads to incomplete zlib streams (albeit intact due to
the use of Z_SYNC_FLUSH) resulting in the occasional short object
capture.

Testcase: igt/i915-error-capture.js
Fixes: 0a97015d45ee ("drm/i915: Compress GPU objects in error state")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: <stable@vger.kernel.org> # v4.10+
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 60 +++++++++++++++++++++------
 1 file changed, 47 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 2835cacd0d08..9789f4ff8c32 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -237,6 +237,7 @@ static int compress_page(struct compress *c,
 			 struct drm_i915_error_object *dst)
 {
 	struct z_stream_s *zstream = &c->zstream;
+	int flush = Z_NO_FLUSH;
 
 	zstream->next_in = src;
 	if (c->tmp && i915_memcpy_from_wc(c->tmp, src, PAGE_SIZE))
@@ -257,8 +258,11 @@ static int compress_page(struct compress *c,
 			zstream->avail_out = PAGE_SIZE;
 		}
 
-		if (zlib_deflate(zstream, Z_SYNC_FLUSH) != Z_OK)
+		if (zlib_deflate(zstream, flush) != Z_OK)
 			return -EIO;
+
+		if (zstream->avail_out)
+			flush = Z_SYNC_FLUSH;
 	} while (zstream->avail_in);
 
 	/* Fallback to uncompressed if we increase size? */
@@ -268,19 +272,43 @@ static int compress_page(struct compress *c,
 	return 0;
 }
 
-static void compress_fini(struct compress *c,
+static int compress_flush(struct compress *c,
 			  struct drm_i915_error_object *dst)
 {
 	struct z_stream_s *zstream = &c->zstream;
+	unsigned long page;
 
-	if (dst) {
-		zlib_deflate(zstream, Z_FINISH);
-		dst->unused = zstream->avail_out;
-	}
+	do {
+		switch (zlib_deflate(zstream, Z_FINISH)) {
+		case Z_OK: /* more space requested */
+			page = __get_free_page(GFP_ATOMIC | __GFP_NOWARN);
+			if (!page)
+				return -ENOMEM;
+
+			dst->pages[dst->page_count++] = (void *)page;
+			zstream->next_out = (void *)page;
+			zstream->avail_out = PAGE_SIZE;
+			break;
+		case Z_STREAM_END:
+			goto end;
+		default: /* any error */
+			return -EIO;
+		}
+	} while (1);
+
+end:
+	memset(zstream->next_out, 0, zstream->avail_out);
+	dst->unused = zstream->avail_out;
+	return 0;
+}
+
+static void compress_fini(struct compress *c,
+			  struct drm_i915_error_object *dst)
+{
+	struct z_stream_s *zstream = &c->zstream;
 
 	zlib_deflateEnd(zstream);
 	kfree(zstream->workspace);
-
 	if (c->tmp)
 		free_page((unsigned long)c->tmp);
 }
@@ -319,6 +347,12 @@ static int compress_page(struct compress *c,
 	return 0;
 }
 
+static int compress_flush(struct compress *c,
+			  struct drm_i915_error_object *dst)
+{
+	return 0;
+}
+
 static void compress_fini(struct compress *c,
 			  struct drm_i915_error_object *dst)
 {
@@ -951,15 +985,15 @@ i915_error_object_create(struct drm_i915_private *i915,
 		if (ret)
 			goto unwind;
 	}
-	goto out;
 
+	if (compress_flush(&compress, dst)) {
 unwind:
-	while (dst->page_count--)
-		free_page((unsigned long)dst->pages[dst->page_count]);
-	kfree(dst);
-	dst = NULL;
+		while (dst->page_count--)
+			free_page((unsigned long)dst->pages[dst->page_count]);
+		kfree(dst);
+		dst = NULL;
+	}
 
-out:
 	compress_fini(&compress, dst);
 	ggtt->vm.clear_range(&ggtt->vm, slot, PAGE_SIZE);
 	return dst;
-- 
2.19.0.rc2

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/6] drm/i915: Clear the error PTE just once on finish
  2018-09-07 11:28 [PATCH 1/6] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
                   ` (2 preceding siblings ...)
  2018-09-07 11:28 ` [PATCH 4/6] drm/i915: Handle incomplete Z_FINISH for compressed error states Chris Wilson
@ 2018-09-07 11:28 ` Chris Wilson
  2018-09-07 11:28 ` [PATCH 6/6] drm/i915: Cache the error string Chris Wilson
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2018-09-07 11:28 UTC (permalink / raw)
  To: intel-gfx

We do not need to continually clear our dedicated PTE for error capture
as it will be updated and invalidated to the next object. Only at the
end do we wish to be sure that the PTE doesn't point back to any buffer.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_gpu_error.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 9789f4ff8c32..08fdf4dc200f 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -995,7 +995,6 @@ i915_error_object_create(struct drm_i915_private *i915,
 	}
 
 	compress_fini(&compress, dst);
-	ggtt->vm.clear_range(&ggtt->vm, slot, PAGE_SIZE);
 	return dst;
 }
 
@@ -1781,6 +1780,14 @@ static unsigned long capture_find_epoch(const struct i915_gpu_state *error)
 	return epoch;
 }
 
+static void capture_finish(struct i915_gpu_state *error)
+{
+	struct i915_ggtt *ggtt = &error->i915->ggtt;
+	const u64 slot = ggtt->error_capture.start;
+
+	ggtt->vm.clear_range(&ggtt->vm, slot, PAGE_SIZE);
+}
+
 static int capture(void *data)
 {
 	struct i915_gpu_state *error = data;
@@ -1805,6 +1812,7 @@ static int capture(void *data)
 
 	error->epoch = capture_find_epoch(error);
 
+	capture_finish(error);
 	return 0;
 }
 
-- 
2.19.0.rc2

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 6/6] drm/i915: Cache the error string
  2018-09-07 11:28 [PATCH 1/6] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
                   ` (3 preceding siblings ...)
  2018-09-07 11:28 ` [PATCH 5/6] drm/i915: Clear the error PTE just once on finish Chris Wilson
@ 2018-09-07 11:28 ` Chris Wilson
  2018-09-07 11:38 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/6] drm/i915: Missed interrupt simulation is no more, tell the world Patchwork
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2018-09-07 11:28 UTC (permalink / raw)
  To: intel-gfx

Currently, we convert the error state into a string every time we read
from sysfs (and sysfs reads in page size (4KiB) chunks). We do try to
window the string and only capture the portion that is being read, but
that means that we must always convert up to the window to find the
start. For a very large error state bordering on EXEC_OBJECT_CAPTURE
abuse, this is noticeable as it degrades to O(N^2)!

As we do not have a convenient hook for sysfs open(), and we would like
to keep the lazy conversion into a string, do the conversion of the
whole string on the first read and keep the string until the error state
is freed.

v2: Don't double advance simple_read_from_buffer
v3: Due to extreme pain of lack of vrealloc, use a scatterlist
v4: Keep the forward iterator loosely cached

Reported-by: Jason Ekstrand <jason@jlekstrand.net>
Testcase: igt/gem_exec_capture/many*
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Cc: Jason Ekstrand <jason@jlekstrand.net>
---
 drivers/gpu/drm/i915/i915_debugfs.c   |  32 +-
 drivers/gpu/drm/i915/i915_gpu_error.c | 402 +++++++++++++++-----------
 drivers/gpu/drm/i915/i915_gpu_error.h |  28 +-
 drivers/gpu/drm/i915/i915_sysfs.c     |  27 +-
 4 files changed, 273 insertions(+), 216 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b4744a68cd88..2ac75bc10afa 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -943,30 +943,28 @@ static int i915_gem_fence_regs_info(struct seq_file *m, void *data)
 static ssize_t gpu_state_read(struct file *file, char __user *ubuf,
 			      size_t count, loff_t *pos)
 {
-	struct i915_gpu_state *error = file->private_data;
-	struct drm_i915_error_state_buf str;
+	struct i915_gpu_state *error;
 	ssize_t ret;
-	loff_t tmp;
+	void *buf;
 
+	error = file->private_data;
 	if (!error)
 		return 0;
 
-	ret = i915_error_state_buf_init(&str, error->i915, count, *pos);
-	if (ret)
-		return ret;
-
-	ret = i915_error_state_to_str(&str, error);
-	if (ret)
-		goto out;
+	/* Bounce buffer required because of kernfs __user API convenience. */
+	buf = kmalloc(count, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
 
-	tmp = 0;
-	ret = simple_read_from_buffer(ubuf, count, &tmp, str.buf, str.bytes);
-	if (ret < 0)
-		goto out;
+	ret = i915_gpu_state_copy_to_buffer(error, buf, *pos, count);
+	if (ret > 0) {
+		if (!copy_to_user(ubuf, buf, ret))
+			*pos += ret;
+		else
+			ret = -EFAULT;
+	}
 
-	*pos = str.start + ret;
-out:
-	i915_error_state_buf_release(&str);
+	kfree(buf);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 08fdf4dc200f..4d0090328c12 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -28,6 +28,8 @@
  */
 
 #include <generated/utsrelease.h>
+#include <linux/nmi.h>
+#include <linux/scatterlist.h>
 #include <linux/stop_machine.h>
 #include <linux/zlib.h>
 #include <drm/drm_print.h>
@@ -77,112 +79,110 @@ static const char *purgeable_flag(int purgeable)
 	return purgeable ? " purgeable" : "";
 }
 
-static bool __i915_error_ok(struct drm_i915_error_state_buf *e)
+static void __sg_set_buf(struct scatterlist *sg,
+			 void *addr, unsigned int len, loff_t it)
 {
-
-	if (!e->err && WARN(e->bytes > (e->size - 1), "overflow")) {
-		e->err = -ENOSPC;
-		return false;
-	}
-
-	if (e->bytes == e->size - 1 || e->err)
-		return false;
-
-	return true;
+	sg->page_link = (unsigned long)virt_to_page(addr);
+	sg->offset = offset_in_page(addr);
+	sg->length = len;
+	sg->dma_address = it;
 }
 
-static bool __i915_error_seek(struct drm_i915_error_state_buf *e,
-			      unsigned len)
+static bool __i915_error_grow(struct drm_i915_error_state_buf *e, size_t len)
 {
-	if (e->pos + len <= e->start) {
-		e->pos += len;
+	if (!len)
 		return false;
-	}
 
-	/* First vsnprintf needs to fit in its entirety for memmove */
-	if (len >= e->size) {
-		e->err = -EIO;
-		return false;
-	}
+	if (e->bytes + len + 1 > e->size) {
+		if (e->bytes) {
+			__sg_set_buf(e->cur++, e->buf, e->bytes, e->iter);
+			e->iter += e->bytes;
+			e->buf = NULL;
+			e->bytes = 0;
+		}
 
-	return true;
-}
+		if (e->cur == e->end) {
+			struct scatterlist *sgl;
 
-static void __i915_error_advance(struct drm_i915_error_state_buf *e,
-				 unsigned len)
-{
-	/* If this is first printf in this window, adjust it so that
-	 * start position matches start of the buffer
-	 */
+			sgl = (typeof(sgl))__get_free_page(GFP_KERNEL);
+			if (!sgl) {
+				e->err = -ENOMEM;
+				return false;
+			}
 
-	if (e->pos < e->start) {
-		const size_t off = e->start - e->pos;
+			if (e->cur) {
+				e->cur->offset = 0;
+				e->cur->length = 0;
+				e->cur->page_link =
+					(unsigned long)sgl | SG_CHAIN;
+			} else {
+				e->sgl = sgl;
+			}
 
-		/* Should not happen but be paranoid */
-		if (off > len || e->bytes) {
-			e->err = -EIO;
-			return;
+			e->cur = sgl;
+			e->end = sgl + SG_MAX_SINGLE_ALLOC - 1;
 		}
 
-		memmove(e->buf, e->buf + off, len - off);
-		e->bytes = len - off;
-		e->pos = e->start;
-		return;
+		e->size = ALIGN(len + 1, SZ_64K);
+		e->buf = kmalloc(e->size,
+				 GFP_KERNEL | __GFP_NOWARN | __GFP_NORETRY);
+		if (!e->buf) {
+			e->size = PAGE_ALIGN(len + 1);
+			e->buf = kmalloc(e->size, GFP_KERNEL);
+		}
+		if (!e->buf) {
+			e->err = -ENOMEM;
+			return false;
+		}
 	}
 
-	e->bytes += len;
-	e->pos += len;
+	return true;
 }
 
 __printf(2, 0)
 static void i915_error_vprintf(struct drm_i915_error_state_buf *e,
-			       const char *f, va_list args)
+			       const char *fmt, va_list args)
 {
-	unsigned len;
+	va_list ap;
+	int len;
 
-	if (!__i915_error_ok(e))
+	if (e->err)
 		return;
 
-	/* Seek the first printf which is hits start position */
-	if (e->pos < e->start) {
-		va_list tmp;
-
-		va_copy(tmp, args);
-		len = vsnprintf(NULL, 0, f, tmp);
-		va_end(tmp);
-
-		if (!__i915_error_seek(e, len))
-			return;
+	va_copy(ap, args);
+	len = vsnprintf(NULL, 0, fmt, ap);
+	va_end(ap);
+	if (len <= 0) {
+		e->err = len;
+		return;
 	}
 
-	len = vsnprintf(e->buf + e->bytes, e->size - e->bytes, f, args);
-	if (len >= e->size - e->bytes)
-		len = e->size - e->bytes - 1;
+	if (!__i915_error_grow(e, len))
+		return;
 
-	__i915_error_advance(e, len);
+	GEM_BUG_ON(e->bytes >= e->size);
+	len = vscnprintf(e->buf + e->bytes, e->size - e->bytes, fmt, args);
+	if (len < 0) {
+		e->err = len;
+		return;
+	}
+	e->bytes += len;
 }
 
-static void i915_error_puts(struct drm_i915_error_state_buf *e,
-			    const char *str)
+static void i915_error_puts(struct drm_i915_error_state_buf *e, const char *str)
 {
 	unsigned len;
 
-	if (!__i915_error_ok(e))
+	if (e->err || !str)
 		return;
 
 	len = strlen(str);
+	if (!__i915_error_grow(e, len))
+		return;
 
-	/* Seek the first printf which is hits start position */
-	if (e->pos < e->start) {
-		if (!__i915_error_seek(e, len))
-			return;
-	}
-
-	if (len >= e->size - e->bytes)
-		len = e->size - e->bytes - 1;
+	GEM_BUG_ON(e->bytes + len > e->size);
 	memcpy(e->buf + e->bytes, str, len);
-
-	__i915_error_advance(e, len);
+	e->bytes += len;
 }
 
 #define err_printf(e, ...) i915_error_printf(e, __VA_ARGS__)
@@ -263,6 +263,8 @@ static int compress_page(struct compress *c,
 
 		if (zstream->avail_out)
 			flush = Z_SYNC_FLUSH;
+
+		touch_nmi_watchdog();
 	} while (zstream->avail_in);
 
 	/* Fallback to uncompressed if we increase size? */
@@ -631,33 +633,50 @@ static void err_print_uc(struct drm_i915_error_state_buf *m,
 	print_error_obj(m, NULL, "GuC log buffer", error_uc->guc_log);
 }
 
-int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
-			    const struct i915_gpu_state *error)
+static void err_free_sgl(struct scatterlist *sgl)
 {
-	struct drm_i915_private *dev_priv = m->i915;
+	while (sgl) {
+		struct scatterlist *sg;
+
+		for (sg = sgl; !sg_is_chain(sg); sg++) {
+			kfree(sg_virt(sg));
+			if (sg_is_last(sg))
+				break;
+		}
+
+		sg = sg_is_last(sg) ? NULL : sg_chain_ptr(sg);
+		free_page((unsigned long)sgl);
+		sgl = sg;
+	}
+}
+
+static int err_print_to_sgl(struct i915_gpu_state *error)
+{
+	struct drm_i915_error_state_buf m;
 	struct drm_i915_error_object *obj;
 	struct timespec64 ts;
 	int i, j;
 
-	if (!error) {
-		err_printf(m, "No error state collected\n");
+	if (READ_ONCE(error->sgl))
 		return 0;
-	}
+
+	memset(&m, 0, sizeof(m));
+	m.i915 = error->i915;
 
 	if (*error->error_msg)
-		err_printf(m, "%s\n", error->error_msg);
-	err_printf(m, "Kernel: " UTS_RELEASE "\n");
+		err_printf(&m, "%s\n", error->error_msg);
+	err_printf(&m, "Kernel: " UTS_RELEASE "\n");
 	ts = ktime_to_timespec64(error->time);
-	err_printf(m, "Time: %lld s %ld us\n",
+	err_printf(&m, "Time: %lld s %ld us\n",
 		   (s64)ts.tv_sec, ts.tv_nsec / NSEC_PER_USEC);
 	ts = ktime_to_timespec64(error->boottime);
-	err_printf(m, "Boottime: %lld s %ld us\n",
+	err_printf(&m, "Boottime: %lld s %ld us\n",
 		   (s64)ts.tv_sec, ts.tv_nsec / NSEC_PER_USEC);
 	ts = ktime_to_timespec64(error->uptime);
-	err_printf(m, "Uptime: %lld s %ld us\n",
+	err_printf(&m, "Uptime: %lld s %ld us\n",
 		   (s64)ts.tv_sec, ts.tv_nsec / NSEC_PER_USEC);
-	err_printf(m, "Epoch: %lu jiffies (%u HZ)\n", error->epoch, HZ);
-	err_printf(m, "Capture: %lu jiffies; %d ms ago, %d ms after epoch\n",
+	err_printf(&m, "Epoch: %lu jiffies (%u HZ)\n", error->epoch, HZ);
+	err_printf(&m, "Capture: %lu jiffies; %d ms ago, %d ms after epoch\n",
 		   error->capture,
 		   jiffies_to_msecs(jiffies - error->capture),
 		   jiffies_to_msecs(error->capture - error->epoch));
@@ -665,63 +684,63 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 	for (i = 0; i < ARRAY_SIZE(error->engine); i++) {
 		if (error->engine[i].hangcheck_stalled &&
 		    error->engine[i].context.pid) {
-			err_printf(m, "Active process (on ring %s): %s [%d], score %d%s\n",
-				   engine_name(m->i915, i),
+			err_printf(&m, "Active process (on ring %s): %s [%d], score %d%s\n",
+				   engine_name(m.i915, i),
 				   error->engine[i].context.comm,
 				   error->engine[i].context.pid,
 				   error->engine[i].context.ban_score,
 				   bannable(&error->engine[i].context));
 		}
 	}
-	err_printf(m, "Reset count: %u\n", error->reset_count);
-	err_printf(m, "Suspend count: %u\n", error->suspend_count);
-	err_printf(m, "Platform: %s\n", intel_platform_name(error->device_info.platform));
-	err_print_pciid(m, error->i915);
+	err_printf(&m, "Reset count: %u\n", error->reset_count);
+	err_printf(&m, "Suspend count: %u\n", error->suspend_count);
+	err_printf(&m, "Platform: %s\n", intel_platform_name(error->device_info.platform));
+	err_print_pciid(&m, m.i915);
 
-	err_printf(m, "IOMMU enabled?: %d\n", error->iommu);
+	err_printf(&m, "IOMMU enabled?: %d\n", error->iommu);
 
-	if (HAS_CSR(dev_priv)) {
-		struct intel_csr *csr = &dev_priv->csr;
+	if (HAS_CSR(m.i915)) {
+		struct intel_csr *csr = &m.i915->csr;
 
-		err_printf(m, "DMC loaded: %s\n",
+		err_printf(&m, "DMC loaded: %s\n",
 			   yesno(csr->dmc_payload != NULL));
-		err_printf(m, "DMC fw version: %d.%d\n",
+		err_printf(&m, "DMC fw version: %d.%d\n",
 			   CSR_VERSION_MAJOR(csr->version),
 			   CSR_VERSION_MINOR(csr->version));
 	}
 
-	err_printf(m, "GT awake: %s\n", yesno(error->awake));
-	err_printf(m, "RPM wakelock: %s\n", yesno(error->wakelock));
-	err_printf(m, "PM suspended: %s\n", yesno(error->suspended));
-	err_printf(m, "EIR: 0x%08x\n", error->eir);
-	err_printf(m, "IER: 0x%08x\n", error->ier);
+	err_printf(&m, "GT awake: %s\n", yesno(error->awake));
+	err_printf(&m, "RPM wakelock: %s\n", yesno(error->wakelock));
+	err_printf(&m, "PM suspended: %s\n", yesno(error->suspended));
+	err_printf(&m, "EIR: 0x%08x\n", error->eir);
+	err_printf(&m, "IER: 0x%08x\n", error->ier);
 	for (i = 0; i < error->ngtier; i++)
-		err_printf(m, "GTIER[%d]: 0x%08x\n", i, error->gtier[i]);
-	err_printf(m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
-	err_printf(m, "FORCEWAKE: 0x%08x\n", error->forcewake);
-	err_printf(m, "DERRMR: 0x%08x\n", error->derrmr);
-	err_printf(m, "CCID: 0x%08x\n", error->ccid);
-	err_printf(m, "Missed interrupts: 0x%08lx\n", dev_priv->gpu_error.missed_irq_rings);
+		err_printf(&m, "GTIER[%d]: 0x%08x\n", i, error->gtier[i]);
+	err_printf(&m, "PGTBL_ER: 0x%08x\n", error->pgtbl_er);
+	err_printf(&m, "FORCEWAKE: 0x%08x\n", error->forcewake);
+	err_printf(&m, "DERRMR: 0x%08x\n", error->derrmr);
+	err_printf(&m, "CCID: 0x%08x\n", error->ccid);
+	err_printf(&m, "Missed interrupts: 0x%08lx\n", m.i915->gpu_error.missed_irq_rings);
 
 	for (i = 0; i < error->nfence; i++)
-		err_printf(m, "  fence[%d] = %08llx\n", i, error->fence[i]);
+		err_printf(&m, "  fence[%d] = %08llx\n", i, error->fence[i]);
 
-	if (INTEL_GEN(dev_priv) >= 6) {
-		err_printf(m, "ERROR: 0x%08x\n", error->error);
+	if (INTEL_GEN(m.i915) >= 6) {
+		err_printf(&m, "ERROR: 0x%08x\n", error->error);
 
-		if (INTEL_GEN(dev_priv) >= 8)
-			err_printf(m, "FAULT_TLB_DATA: 0x%08x 0x%08x\n",
+		if (INTEL_GEN(m.i915) >= 8)
+			err_printf(&m, "FAULT_TLB_DATA: 0x%08x 0x%08x\n",
 				   error->fault_data1, error->fault_data0);
 
-		err_printf(m, "DONE_REG: 0x%08x\n", error->done_reg);
+		err_printf(&m, "DONE_REG: 0x%08x\n", error->done_reg);
 	}
 
-	if (IS_GEN7(dev_priv))
-		err_printf(m, "ERR_INT: 0x%08x\n", error->err_int);
+	if (IS_GEN7(m.i915))
+		err_printf(&m, "ERR_INT: 0x%08x\n", error->err_int);
 
 	for (i = 0; i < ARRAY_SIZE(error->engine); i++) {
 		if (error->engine[i].engine_id != -1)
-			error_print_engine(m, &error->engine[i], error->epoch);
+			error_print_engine(&m, &error->engine[i], error->epoch);
 	}
 
 	for (i = 0; i < ARRAY_SIZE(error->active_vm); i++) {
@@ -738,16 +757,16 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 
 			len += scnprintf(buf + len, sizeof(buf), "%s%s",
 					 first ? "" : ", ",
-					 dev_priv->engine[j]->name);
+					 m.i915->engine[j]->name);
 			first = 0;
 		}
 		scnprintf(buf + len, sizeof(buf), ")");
-		print_error_buffers(m, buf,
+		print_error_buffers(&m, buf,
 				    error->active_bo[i],
 				    error->active_bo_count[i]);
 	}
 
-	print_error_buffers(m, "Pinned (global)",
+	print_error_buffers(&m, "Pinned (global)",
 			    error->pinned_bo,
 			    error->pinned_bo_count);
 
@@ -756,115 +775,163 @@ int i915_error_state_to_str(struct drm_i915_error_state_buf *m,
 
 		obj = ee->batchbuffer;
 		if (obj) {
-			err_puts(m, dev_priv->engine[i]->name);
+			err_puts(&m, m.i915->engine[i]->name);
 			if (ee->context.pid)
-				err_printf(m, " (submitted by %s [%d], ctx %d [%d], score %d%s)",
+				err_printf(&m, " (submitted by %s [%d], ctx %d [%d], score %d%s)",
 					   ee->context.comm,
 					   ee->context.pid,
 					   ee->context.handle,
 					   ee->context.hw_id,
 					   ee->context.ban_score,
 					   bannable(&ee->context));
-			err_printf(m, " --- gtt_offset = 0x%08x %08x\n",
+			err_printf(&m, " --- gtt_offset = 0x%08x %08x\n",
 				   upper_32_bits(obj->gtt_offset),
 				   lower_32_bits(obj->gtt_offset));
-			print_error_obj(m, dev_priv->engine[i], NULL, obj);
+			print_error_obj(&m, m.i915->engine[i], NULL, obj);
 		}
 
 		for (j = 0; j < ee->user_bo_count; j++)
-			print_error_obj(m, dev_priv->engine[i],
+			print_error_obj(&m, m.i915->engine[i],
 					"user", ee->user_bo[j]);
 
 		if (ee->num_requests) {
-			err_printf(m, "%s --- %d requests\n",
-				   dev_priv->engine[i]->name,
+			err_printf(&m, "%s --- %d requests\n",
+				   m.i915->engine[i]->name,
 				   ee->num_requests);
 			for (j = 0; j < ee->num_requests; j++)
-				error_print_request(m, " ",
+				error_print_request(&m, " ",
 						    &ee->requests[j],
 						    error->epoch);
 		}
 
 		if (IS_ERR(ee->waiters)) {
-			err_printf(m, "%s --- ? waiters [unable to acquire spinlock]\n",
-				   dev_priv->engine[i]->name);
+			err_printf(&m, "%s --- ? waiters [unable to acquire spinlock]\n",
+				   m.i915->engine[i]->name);
 		} else if (ee->num_waiters) {
-			err_printf(m, "%s --- %d waiters\n",
-				   dev_priv->engine[i]->name,
+			err_printf(&m, "%s --- %d waiters\n",
+				   m.i915->engine[i]->name,
 				   ee->num_waiters);
 			for (j = 0; j < ee->num_waiters; j++) {
-				err_printf(m, " seqno 0x%08x for %s [%d]\n",
+				err_printf(&m, " seqno 0x%08x for %s [%d]\n",
 					   ee->waiters[j].seqno,
 					   ee->waiters[j].comm,
 					   ee->waiters[j].pid);
 			}
 		}
 
-		print_error_obj(m, dev_priv->engine[i],
+		print_error_obj(&m, m.i915->engine[i],
 				"ringbuffer", ee->ringbuffer);
 
-		print_error_obj(m, dev_priv->engine[i],
+		print_error_obj(&m, m.i915->engine[i],
 				"HW Status", ee->hws_page);
 
-		print_error_obj(m, dev_priv->engine[i],
+		print_error_obj(&m, m.i915->engine[i],
 				"HW context", ee->ctx);
 
-		print_error_obj(m, dev_priv->engine[i],
+		print_error_obj(&m, m.i915->engine[i],
 				"WA context", ee->wa_ctx);
 
-		print_error_obj(m, dev_priv->engine[i],
+		print_error_obj(&m, m.i915->engine[i],
 				"WA batchbuffer", ee->wa_batchbuffer);
 
-		print_error_obj(m, dev_priv->engine[i],
+		print_error_obj(&m, m.i915->engine[i],
 				"NULL context", ee->default_state);
 	}
 
 	if (error->overlay)
-		intel_overlay_print_error_state(m, error->overlay);
+		intel_overlay_print_error_state(&m, error->overlay);
 
 	if (error->display)
-		intel_display_print_error_state(m, error->display);
+		intel_display_print_error_state(&m, error->display);
 
-	err_print_capabilities(m, &error->device_info, &error->driver_caps);
-	err_print_params(m, &error->params);
-	err_print_uc(m, &error->uc);
+	err_print_capabilities(&m, &error->device_info, &error->driver_caps);
+	err_print_params(&m, &error->params);
+	err_print_uc(&m, &error->uc);
 
-	if (m->bytes == 0 && m->err)
-		return m->err;
+	if (m.buf) {
+		__sg_set_buf(m.cur++, m.buf, m.bytes, m.iter);
+		m.bytes = 0;
+		m.buf = 0;
+	}
+	if (m.cur) {
+		GEM_BUG_ON(m.end < m.cur);
+		sg_mark_end(m.cur - 1);
+	}
+	GEM_BUG_ON(m.sgl && !m.cur);
+
+	if (m.err) {
+		err_free_sgl(m.sgl);
+		return m.err;
+	}
+
+	if (cmpxchg(&error->sgl, NULL, m.sgl))
+		err_free_sgl(m.sgl);
 
 	return 0;
 }
 
-int i915_error_state_buf_init(struct drm_i915_error_state_buf *ebuf,
-			      struct drm_i915_private *i915,
-			      size_t count, loff_t pos)
+ssize_t i915_gpu_state_copy_to_buffer(struct i915_gpu_state *error,
+				      char *buf, loff_t off, size_t rem)
 {
-	memset(ebuf, 0, sizeof(*ebuf));
-	ebuf->i915 = i915;
+	struct scatterlist *sg;
+	size_t count;
+	loff_t pos;
+	int err;
 
-	/* We need to have enough room to store any i915_error_state printf
-	 * so that we can move it to start position.
-	 */
-	ebuf->size = count + 1 > PAGE_SIZE ? count + 1 : PAGE_SIZE;
-	ebuf->buf = kmalloc(ebuf->size,
-				GFP_KERNEL | __GFP_NORETRY | __GFP_NOWARN);
+	if (!error || !rem)
+		return 0;
 
-	if (ebuf->buf == NULL) {
-		ebuf->size = PAGE_SIZE;
-		ebuf->buf = kmalloc(ebuf->size, GFP_KERNEL);
-	}
+	err = err_print_to_sgl(error);
+	if (err)
+		return err;
 
-	if (ebuf->buf == NULL) {
-		ebuf->size = 128;
-		ebuf->buf = kmalloc(ebuf->size, GFP_KERNEL);
-	}
+	sg = READ_ONCE(error->fit);
+	if (!sg || off < sg->dma_address)
+		sg = error->sgl;
+	if (!sg)
+		return 0;
 
-	if (ebuf->buf == NULL)
-		return -ENOMEM;
+	pos = sg->dma_address;
+	count = 0;
+	do {
+		size_t len, start;
 
-	ebuf->start = pos;
+		if (sg_is_chain(sg)) {
+			sg = sg_chain_ptr(sg);
+			GEM_BUG_ON(sg_is_chain(sg));
+		}
 
-	return 0;
+		len = sg->length;
+		if (pos + len <= off) {
+			pos += len;
+			continue;
+		}
+
+		start = sg->offset;
+		if (pos < off) {
+			GEM_BUG_ON(off - pos > len);
+			len -= off - pos;
+			start += off - pos;
+			pos = off;
+		}
+
+		len = min(len, rem);
+		GEM_BUG_ON(!len || len > sg->length);
+
+		memcpy(buf, page_address(sg_page(sg)) + start, len);
+
+		count += len;
+		pos += len;
+
+		buf += len;
+		rem -= len;
+		if (!rem) {
+			WRITE_ONCE(error->fit, sg);
+			break;
+		}
+	} while (!sg_is_last(sg++));
+
+	return count;
 }
 
 static void i915_error_object_free(struct drm_i915_error_object *obj)
@@ -937,6 +1004,7 @@ void __i915_gpu_state_free(struct kref *error_ref)
 	cleanup_params(error);
 	cleanup_uc_state(error);
 
+	err_free_sgl(error->sgl);
 	kfree(error);
 }
 
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.h b/drivers/gpu/drm/i915/i915_gpu_error.h
index f893a4e8b783..1c1bc0c23468 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.h
+++ b/drivers/gpu/drm/i915/i915_gpu_error.h
@@ -191,6 +191,8 @@ struct i915_gpu_state {
 	} *active_bo[I915_NUM_ENGINES], *pinned_bo;
 	u32 active_bo_count[I915_NUM_ENGINES], pinned_bo_count;
 	struct i915_address_space *active_vm[I915_NUM_ENGINES];
+
+	struct scatterlist *sgl, *fit;
 };
 
 struct i915_gpu_error {
@@ -297,29 +299,20 @@ struct i915_gpu_error {
 
 struct drm_i915_error_state_buf {
 	struct drm_i915_private *i915;
-	unsigned int bytes;
-	unsigned int size;
+	struct scatterlist *sgl, *cur, *end;
+
+	char *buf;
+	size_t bytes;
+	size_t size;
+	loff_t iter;
+
 	int err;
-	u8 *buf;
-	loff_t start;
-	loff_t pos;
 };
 
 #if IS_ENABLED(CONFIG_DRM_I915_CAPTURE_ERROR)
 
 __printf(2, 3)
 void i915_error_printf(struct drm_i915_error_state_buf *e, const char *f, ...);
-int i915_error_state_to_str(struct drm_i915_error_state_buf *estr,
-			    const struct i915_gpu_state *gpu);
-int i915_error_state_buf_init(struct drm_i915_error_state_buf *eb,
-			      struct drm_i915_private *i915,
-			      size_t count, loff_t pos);
-
-static inline void
-i915_error_state_buf_release(struct drm_i915_error_state_buf *eb)
-{
-	kfree(eb->buf);
-}
 
 struct i915_gpu_state *i915_capture_gpu_state(struct drm_i915_private *i915);
 void i915_capture_error_state(struct drm_i915_private *dev_priv,
@@ -333,6 +326,9 @@ i915_gpu_state_get(struct i915_gpu_state *gpu)
 	return gpu;
 }
 
+ssize_t i915_gpu_state_copy_to_buffer(struct i915_gpu_state *error,
+				      char *buf, loff_t offset, size_t count);
+
 void __i915_gpu_state_free(struct kref *kref);
 static inline void i915_gpu_state_put(struct i915_gpu_state *gpu)
 {
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index e5e6f6bb2b05..ae63a7d0f51d 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -516,26 +516,21 @@ static ssize_t error_state_read(struct file *filp, struct kobject *kobj,
 {
 
 	struct device *kdev = kobj_to_dev(kobj);
-	struct drm_i915_private *dev_priv = kdev_minor_to_i915(kdev);
-	struct drm_i915_error_state_buf error_str;
+	struct drm_i915_private *i915 = kdev_minor_to_i915(kdev);
 	struct i915_gpu_state *gpu;
 	ssize_t ret;
 
-	ret = i915_error_state_buf_init(&error_str, dev_priv, count, off);
-	if (ret)
-		return ret;
-
-	gpu = i915_first_error_state(dev_priv);
-	ret = i915_error_state_to_str(&error_str, gpu);
-	if (ret)
-		goto out;
-
-	ret = count < error_str.bytes ? count : error_str.bytes;
-	memcpy(buf, error_str.buf, ret);
+	gpu = i915_first_error_state(i915);
+	if (gpu) {
+		ret = i915_gpu_state_copy_to_buffer(gpu, buf, off, count);
+		i915_gpu_state_put(gpu);
+	} else {
+		const char *str = "No error state collected\n";
+		size_t len = strlen(str);
 
-out:
-	i915_gpu_state_put(gpu);
-	i915_error_state_buf_release(&error_str);
+		ret = min_t(size_t, count, len - off);
+		memcpy(buf, str + off, ret);
+	}
 
 	return ret;
 }
-- 
2.19.0.rc2

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* ✗ Fi.CI.SPARSE: warning for series starting with [1/6] drm/i915: Missed interrupt simulation is no more, tell the world
  2018-09-07 11:28 [PATCH 1/6] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
                   ` (4 preceding siblings ...)
  2018-09-07 11:28 ` [PATCH 6/6] drm/i915: Cache the error string Chris Wilson
@ 2018-09-07 11:38 ` Patchwork
  2018-09-07 11:54 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-09-07 11:38 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/6] drm/i915: Missed interrupt simulation is no more, tell the world
URL   : https://patchwork.freedesktop.org/series/49339/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Missed interrupt simulation is no more, tell the world
Okay!

Commit: drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G
Okay!

Commit: drm/i915: Limit number of capture objects
+./include/linux/slab.h:631:13: error: not a function <noident>

Commit: drm/i915: Handle incomplete Z_FINISH for compressed error states
Okay!

Commit: drm/i915: Clear the error PTE just once on finish
Okay!

Commit: drm/i915: Cache the error string
+drivers/gpu/drm/i915/i915_gpu_error.c:854:25: warning: Using plain integer as NULL pointer
+drivers/gpu/drm/i915/i915_gpu_error.c:918:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_gpu_error.c:918:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_sysfs.c:531:23: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/i915_sysfs.c:531:23: warning: expression using sizeof(void)

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/6] drm/i915: Missed interrupt simulation is no more, tell the world
  2018-09-07 11:28 [PATCH 1/6] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
                   ` (5 preceding siblings ...)
  2018-09-07 11:38 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/6] drm/i915: Missed interrupt simulation is no more, tell the world Patchwork
@ 2018-09-07 11:54 ` Patchwork
  2018-09-07 12:30 ` [PATCH 1/6] " Mika Kuoppala
  2018-09-07 14:26 ` ✓ Fi.CI.IGT: success for series starting with [1/6] " Patchwork
  8 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-09-07 11:54 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/6] drm/i915: Missed interrupt simulation is no more, tell the world
URL   : https://patchwork.freedesktop.org/series/49339/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4786 -> Patchwork_10124 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/49339/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_10124 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@amdgpu/amd_basic@userptr:
      fi-kbl-8809g:       PASS -> INCOMPLETE (fdo#107402)

    igt@gem_exec_suspend@basic-s4-devices:
      fi-blb-e6850:       PASS -> INCOMPLETE (fdo#107718)

    
    ==== Possible fixes ====

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-byt-clapper:     FAIL (fdo#107362, fdo#103191) -> PASS

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
      fi-snb-2520m:       INCOMPLETE (fdo#103713) -> PASS

    igt@kms_psr@primary_page_flip:
      fi-kbl-r:           FAIL (fdo#107336) -> PASS

    
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
  fdo#107336 https://bugs.freedesktop.org/show_bug.cgi?id=107336
  fdo#107362 https://bugs.freedesktop.org/show_bug.cgi?id=107362
  fdo#107402 https://bugs.freedesktop.org/show_bug.cgi?id=107402
  fdo#107718 https://bugs.freedesktop.org/show_bug.cgi?id=107718


== Participating hosts (54 -> 48) ==

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-hsw-peppy fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 


== Build changes ==

    * Linux: CI_DRM_4786 -> Patchwork_10124

  CI_DRM_4786: d28dcb05a3c33df6fcb0828d81fa0bb6591a97be @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4634: 7d89cc39dde3b4881d85ace45d504cc098fa3684 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10124: 5de2740fde67d33154ac9b7752baea78251620bd @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

5de2740fde67 drm/i915: Cache the error string
46c071956049 drm/i915: Clear the error PTE just once on finish
4c0be5efc2e9 drm/i915: Handle incomplete Z_FINISH for compressed error states
d226d4702b0f drm/i915: Limit number of capture objects
7c19ceb5f20f drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G
718d1d1b171a drm/i915: Missed interrupt simulation is no more, tell the world

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10124/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 1/6] drm/i915: Missed interrupt simulation is no more, tell the world
  2018-09-07 11:28 [PATCH 1/6] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
                   ` (6 preceding siblings ...)
  2018-09-07 11:54 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-09-07 12:30 ` Mika Kuoppala
  2018-09-07 14:26 ` ✓ Fi.CI.IGT: success for series starting with [1/6] " Patchwork
  8 siblings, 0 replies; 12+ messages in thread
From: Mika Kuoppala @ 2018-09-07 12:30 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

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

> Using the guc, we cannot disable the user interrupt generation as we use
> it for driving submission. And from Icelake, we no longer have the
> ability to individually mask interrupt generation from each engine,
> disabling our ability to fake missed interrupts.
>
> In both cases, report back to userspace that the missed interrupt
> generator is no longer available.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 1f7051e97afb..b4744a68cd88 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -4117,6 +4117,17 @@ i915_ring_test_irq_set(void *data, u64 val)
>  {
>  	struct drm_i915_private *i915 = data;
>  
> +	/* GuC keeps the user interrupt permanently enabled for submission */
> +	if (USES_GUC_SUBMISSION(i915))
> +		return -ENODEV;
> +
> +	/*
> +	 * From icl, we can no longer individually mask interrupt generation
> +	 * from each engine.
> +	 */
> +	if (INTEL_GEN(i915) >= 11)
> +		return -ENODEV;

Perhaps we can go with sw masks in future.

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


> +
>  	val &= INTEL_INFO(i915)->ring_mask;
>  	DRM_DEBUG_DRIVER("Masking interrupts on rings 0x%08llx\n", val);
>  
> -- 
> 2.19.0.rc2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* ✓ Fi.CI.IGT: success for series starting with [1/6] drm/i915: Missed interrupt simulation is no more, tell the world
  2018-09-07 11:28 [PATCH 1/6] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
                   ` (7 preceding siblings ...)
  2018-09-07 12:30 ` [PATCH 1/6] " Mika Kuoppala
@ 2018-09-07 14:26 ` Patchwork
  8 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2018-09-07 14:26 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/6] drm/i915: Missed interrupt simulation is no more, tell the world
URL   : https://patchwork.freedesktop.org/series/49339/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4786_full -> Patchwork_10124_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_10124_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_10124_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_10124_full:

  === IGT changes ===

    ==== Warnings ====

    igt@kms_cursor_legacy@cursora-vs-flipb-toggle:
      shard-hsw:          SKIP -> PASS

    igt@kms_vblank@pipe-b-wait-forked:
      shard-snb:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in Patchwork_10124_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-apl:          PASS -> FAIL (fdo#106886)

    igt@gem_workarounds@suspend-resume-context:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665)

    
    ==== Possible fixes ====

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc:
      shard-apl:          FAIL (fdo#103167) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4786 -> Patchwork_10124

  CI_DRM_4786: d28dcb05a3c33df6fcb0828d81fa0bb6591a97be @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4634: 7d89cc39dde3b4881d85ace45d504cc098fa3684 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_10124: 5de2740fde67d33154ac9b7752baea78251620bd @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_10124/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/6] drm/i915: Limit number of capture objects
  2018-09-07 11:28 ` [PATCH 3/6] drm/i915: Limit number of capture objects Chris Wilson
@ 2018-09-10 13:14   ` Mika Kuoppala
  2018-09-10 13:25     ` Chris Wilson
  0 siblings, 1 reply; 12+ messages in thread
From: Mika Kuoppala @ 2018-09-10 13:14 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

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

> If we fail to allocate an array for a large number of user requested
> capture objects, reduce the array size and try to grab at least some of
> the objects!
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_gpu_error.c | 20 +++++++++++++-------
>  1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index f7f2aa71d8d9..2835cacd0d08 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -1365,15 +1365,20 @@ static void request_record_user_bo(struct i915_request *request,
>  {
>  	struct i915_capture_list *c;
>  	struct drm_i915_error_object **bo;
> -	long count;
> +	long count, max;
>  
> -	count = 0;
> +	max = 0;
>  	for (c = request->capture_list; c; c = c->next)
> -		count++;
> +		max++;
> +	if (!max)
> +		return;
>  
> -	bo = NULL;
> -	if (count)
> -		bo = kcalloc(count, sizeof(*bo), GFP_ATOMIC);
> +	bo = kmalloc_array(max, sizeof(*bo), GFP_ATOMIC);

There seems to be no need to zero the array.

> +	if (!bo) {
> +		/* If we can't capture everything, try to capture something. */
> +		max = min_t(long, max, PAGE_SIZE / sizeof(*bo));

Perhaps there is some bookkeeping in slab so this would spill to 2
pages eventually?

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

> +		bo = kmalloc_array(max, sizeof(*bo), GFP_ATOMIC);
> +	}
>  	if (!bo)
>  		return;
>  
> @@ -1382,7 +1387,8 @@ static void request_record_user_bo(struct i915_request *request,
>  		bo[count] = i915_error_object_create(request->i915, c->vma);
>  		if (!bo[count])
>  			break;
> -		count++;
> +		if (++count == max)
> +			break;
>  	}
>  
>  	ee->user_bo = bo;
> -- 
> 2.19.0.rc2
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 3/6] drm/i915: Limit number of capture objects
  2018-09-10 13:14   ` Mika Kuoppala
@ 2018-09-10 13:25     ` Chris Wilson
  0 siblings, 0 replies; 12+ messages in thread
From: Chris Wilson @ 2018-09-10 13:25 UTC (permalink / raw)
  To: Mika Kuoppala, intel-gfx

Quoting Mika Kuoppala (2018-09-10 14:14:56)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > If we fail to allocate an array for a large number of user requested
> > capture objects, reduce the array size and try to grab at least some of
> > the objects!
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/i915_gpu_error.c | 20 +++++++++++++-------
> >  1 file changed, 13 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> > index f7f2aa71d8d9..2835cacd0d08 100644
> > --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> > +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> > @@ -1365,15 +1365,20 @@ static void request_record_user_bo(struct i915_request *request,
> >  {
> >       struct i915_capture_list *c;
> >       struct drm_i915_error_object **bo;
> > -     long count;
> > +     long count, max;
> >  
> > -     count = 0;
> > +     max = 0;
> >       for (c = request->capture_list; c; c = c->next)
> > -             count++;
> > +             max++;
> > +     if (!max)
> > +             return;
> >  
> > -     bo = NULL;
> > -     if (count)
> > -             bo = kcalloc(count, sizeof(*bo), GFP_ATOMIC);
> > +     bo = kmalloc_array(max, sizeof(*bo), GFP_ATOMIC);
> 
> There seems to be no need to zero the array.
> 
> > +     if (!bo) {
> > +             /* If we can't capture everything, try to capture something. */
> > +             max = min_t(long, max, PAGE_SIZE / sizeof(*bo));
> 
> Perhaps there is some bookkeeping in slab so this would spill to 2
> pages eventually?

Perhaps, my understanding is that kmalloc < 8192 is efficient. It was
more about defining an arbitrary number to try in case the user is
asking for several million captures like the igt did.

We could say halve the number until it allocates, but then we'll
probably just fail latter in capturing pages.

So a page worth of pointers seemed like as good a number as any to use.
We could just capture 1 user buffer, so long as it was the right one ;)
0 just meant the test was a flop (didn't actually test anything other
than the capture malloc failure). What we really need on top of this is
to record when we didn't capture everything :(
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2018-09-10 13:25 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-09-07 11:28 [PATCH 1/6] drm/i915: Missed interrupt simulation is no more, tell the world Chris Wilson
2018-09-07 11:28 ` [PATCH 2/6] drm/i915: Reorder execobject[] to insert non-48b objects into the low 4G Chris Wilson
2018-09-07 11:28 ` [PATCH 3/6] drm/i915: Limit number of capture objects Chris Wilson
2018-09-10 13:14   ` Mika Kuoppala
2018-09-10 13:25     ` Chris Wilson
2018-09-07 11:28 ` [PATCH 4/6] drm/i915: Handle incomplete Z_FINISH for compressed error states Chris Wilson
2018-09-07 11:28 ` [PATCH 5/6] drm/i915: Clear the error PTE just once on finish Chris Wilson
2018-09-07 11:28 ` [PATCH 6/6] drm/i915: Cache the error string Chris Wilson
2018-09-07 11:38 ` ✗ Fi.CI.SPARSE: warning for series starting with [1/6] drm/i915: Missed interrupt simulation is no more, tell the world Patchwork
2018-09-07 11:54 ` ✓ Fi.CI.BAT: success " Patchwork
2018-09-07 12:30 ` [PATCH 1/6] " Mika Kuoppala
2018-09-07 14:26 ` ✓ Fi.CI.IGT: success for series starting with [1/6] " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.