[RFC 0/3] render state initialization (bdw rc6)

[RFC 0/3] render state initialization (bdw rc6)

[Mika Kuoppala]

Hi,

Here are patches to initialize first render context to a hopefully, 
valid state. If pipeline/context is not initialized and we enter rc6 on bdw,
the render ring can hung on the first batch submitted. That is atleast
the hypothesis this work is based on.

The states are stripped from rendercopy_genX's from i-g-t/lib and
the state generators are part of i-g-t also. The states are
propably overshoot from what can be consider the minimal valid
(null) state on the pipeline. I just initialized everything rendercopy
does and haven't really put effort on optimizing the state until
I get some test results that this indeed solves anything.

The state generators can be found here but they are not needed for testing.
http://cgit.freedesktop.org/~miku/intel-gpu-tools/log/?h=null_state_gen

Gen7 and Gen8 seems to atleast survive the boot but Gen6 is totally
untested.

Here is the branch for testing:
http://cgit.freedesktop.org/~miku/drm-intel/log/?h=render_state

I am interested to know if these patches make matters better/worse for those
who have issues with first batch hanging on bdw, but as always, any feedback
is greatly appreciated.

Mika Kuoppala (3):
  drm/i915: export vmap_batch from command parser
  drm/i915: add render state initialization
  drm/i915: add null render states for gen6, gen7 and gen8

 drivers/gpu/drm/i915/Makefile                 |    1 +
 drivers/gpu/drm/i915/i915_cmd_parser.c        |    4 +-
 drivers/gpu/drm/i915/i915_drv.h               |    3 +
 drivers/gpu/drm/i915/i915_gem_context.c       |    7 +
 drivers/gpu/drm/i915/i915_gem_render_state.c  |  222 ++++++++++++
 drivers/gpu/drm/i915/intel_renderstate_gen6.h |  290 +++++++++++++++
 drivers/gpu/drm/i915/intel_renderstate_gen7.h |  254 +++++++++++++
 drivers/gpu/drm/i915/intel_renderstate_gen8.h |  480 +++++++++++++++++++++++++
 8 files changed, 1259 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_gem_render_state.c
 create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen6.h
 create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen7.h
 create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen8.h

-- 
1.7.9.5

[PATCH 1/3] drm/i915: export vmap_batch from command parser

[Mika Kuoppala]

as need it for generating batch commands and state
in subsequent commit

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_cmd_parser.c |    4 ++--
 drivers/gpu/drm/i915/i915_drv.h        |    1 +
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 9bac097..5fab893 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -704,7 +704,7 @@ static bool valid_reg(const u32 *table, int count, u32 addr)
 	return false;
 }
 
-static u32 *vmap_batch(struct drm_i915_gem_object *obj)
+void *i915_gem_vmap_obj(struct drm_i915_gem_object *obj)
 {
 	int i;
 	void *addr = NULL;
@@ -882,7 +882,7 @@ int i915_parse_cmds(struct intel_ring_buffer *ring,
 		return ret;
 	}
 
-	batch_base = vmap_batch(batch_obj);
+	batch_base = i915_gem_vmap_obj(batch_obj);
 	if (!batch_base) {
 		DRM_DEBUG_DRIVER("CMD: Failed to vmap batch\n");
 		i915_gem_object_unpin_pages(batch_obj);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 92c3095..43b022c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2413,6 +2413,7 @@ void i915_get_extra_instdone(struct drm_device *dev, uint32_t *instdone);
 const char *i915_cache_level_str(int type);
 
 /* i915_cmd_parser.c */
+void __must_check *i915_gem_vmap_obj(struct drm_i915_gem_object *obj);
 int i915_cmd_parser_get_version(void);
 void i915_cmd_parser_init_ring(struct intel_ring_buffer *ring);
 bool i915_needs_cmd_parser(struct intel_ring_buffer *ring);
-- 
1.7.9.5

[PATCH 2/3] drm/i915: add render state initialization

[Mika Kuoppala]

HW guys say that it is not a cool idea to let device
go into rc6 without proper 3d pipeline state.

For each new uninitialized context, generate a
valid null render state to be run on context
creation.

This patch introduces a skeleton with emty states.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/Makefile                 |    1 +
 drivers/gpu/drm/i915/i915_drv.h               |    2 +
 drivers/gpu/drm/i915/i915_gem_context.c       |    7 +
 drivers/gpu/drm/i915/i915_gem_render_state.c  |  222 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_renderstate_gen6.h |   11 ++
 drivers/gpu/drm/i915/intel_renderstate_gen7.h |   11 ++
 drivers/gpu/drm/i915/intel_renderstate_gen8.h |   11 ++
 7 files changed, 265 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/i915_gem_render_state.c
 create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen6.h
 create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen7.h
 create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen8.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index b1445b7..b5d4029 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -18,6 +18,7 @@ i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
 # GEM code
 i915-y += i915_cmd_parser.o \
 	  i915_gem_context.o \
+	  i915_gem_render_state.o \
 	  i915_gem_debug.o \
 	  i915_gem_dmabuf.o \
 	  i915_gem_evict.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 43b022c..f6ae2ee 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2330,6 +2330,8 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
 int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
 				   struct drm_file *file);
 
+/* i915_gem_render_state.c */
+int i915_gem_init_render_state(struct intel_ring_buffer *ring);
 /* i915_gem_evict.c */
 int __must_check i915_gem_evict_something(struct drm_device *dev,
 					  struct i915_address_space *vm,
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 30b355a..d648d4d 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -746,6 +746,13 @@ static int do_switch(struct intel_ring_buffer *ring,
 		/* obj is kept alive until the next request by its active ref */
 		i915_gem_object_ggtt_unpin(from->obj);
 		i915_gem_context_unreference(from);
+	} else {
+		if (ring->id == RCS && to->is_initialized == false) {
+
+			ret = i915_gem_init_render_state(ring);
+			if (ret)
+				DRM_ERROR("init render state: %d\n", ret);
+		}
 	}
 
 	to->is_initialized = true;
diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
new file mode 100644
index 0000000..409f99b
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -0,0 +1,222 @@
+/*
+ * Copyright © 2014 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ *    Mika Kuoppala <mika.kuoppala@intel.com>
+ *
+ */
+
+#include "i915_drv.h"
+
+#include "intel_renderstate_gen6.h"
+#include "intel_renderstate_gen7.h"
+#include "intel_renderstate_gen8.h"
+
+#define BATCH_MAX_SIZE 4096
+
+struct i915_render_state {
+	struct drm_i915_gem_object *obj;
+	unsigned long ggtt_offset;
+	void *batch;
+	u32 size;
+	u32 len;
+};
+
+static struct i915_render_state *
+render_state_alloc(struct drm_device *dev, unsigned int size)
+{
+	struct i915_render_state *so;
+	int ret;
+
+	so = kzalloc(sizeof(*so), GFP_KERNEL);
+	if (!so)
+		return ERR_PTR(-ENOMEM);
+
+	so->size = ALIGN(size, 4096);
+
+	so->obj = i915_gem_alloc_object(dev, so->size);
+	if (so->obj == NULL) {
+		ret = -ENOMEM;
+		goto free;
+	}
+
+	ret = i915_gem_obj_ggtt_pin(so->obj, 4096, 0);
+	if (ret)
+		goto free_gem;
+
+	so->batch = i915_gem_vmap_obj(so->obj);
+	if (!so->batch) {
+		ret = -ENOMEM;
+		goto unpin;
+	}
+
+	so->ggtt_offset = i915_gem_obj_ggtt_offset(so->obj);
+
+	return so;
+unpin:
+	i915_gem_object_ggtt_unpin(so->obj);
+free_gem:
+	drm_gem_object_unreference(&so->obj->base);
+free:
+	kfree(so);
+	return ERR_PTR(ret);
+}
+
+static void render_state_free(struct i915_render_state *so)
+{
+	vunmap(so->batch);
+	i915_gem_object_ggtt_unpin(so->obj);
+	drm_gem_object_unreference(&so->obj->base);
+	kfree(so);
+}
+
+struct render_state_ro_data {
+	const u32 *batch;
+	unsigned int batch_items;
+	const u32 *reloc;
+	unsigned int reloc_items;
+	bool reloc_64bit;
+};
+
+static int render_state_copy(struct i915_render_state *so,
+			     const struct render_state_ro_data *source)
+{
+	const u64 goffset = i915_gem_obj_ggtt_offset(so->obj);
+	const unsigned int num_relocs = source->reloc_items;
+	int reloc_index = 0;
+	u32 *d = (uint32_t *)so->batch;
+	unsigned int i = 0;
+
+	if (source->batch_items * sizeof(*source->batch) > so->size)
+		return -EINVAL;
+
+	while (i < source->batch_items) {
+		u32 s = source->batch[i];
+
+		if (reloc_index < num_relocs &&
+		    i * 4  == source->reloc[reloc_index]) {
+
+			s += goffset & 0xffffffff;
+
+			/* We keep batch offsets max 32bit */
+			if (source->reloc_64bit) {
+				if (i + 1 >= source->batch_items ||
+				    source->batch[i + 1] != 0)
+					return -EINVAL;
+
+				d[i] = s;
+				i++;
+				s = (goffset & 0xffffffff00000000ull) >> 32;
+			}
+
+			reloc_index++;
+		}
+
+		d[i] = s;
+		i++;
+	}
+
+	if (num_relocs != reloc_index) {
+		DRM_ERROR("not all relocs resolved, %d out of %d\n",
+			  reloc_index, num_relocs);
+		return -EINVAL;
+	}
+
+	so->len = source->batch_items * sizeof(*source->batch);
+
+	return 0;
+}
+
+static int render_state_get(struct i915_render_state *so, int gen)
+{
+	struct render_state_ro_data ro_data;
+
+#define RS_SETUP(_d, _g, _r) {						\
+		if (!sizeof(gen ## _g ## _null_state_batch))		\
+			return -EINVAL;					\
+		if (sizeof(gen ## _g ## _null_state_batch) & 0x3)	\
+			return -EINVAL;					\
+		if (sizeof(gen ## _g ## _null_state_relocs) & 0x3)	\
+			return -EINVAL;					\
+		_d.batch = gen ## _g ## _null_state_batch;		\
+		_d.batch_items = sizeof(gen ##_g ## _null_state_batch) / 4; \
+		_d.reloc = gen ## _g ## _null_state_relocs;		\
+		_d.reloc_items = sizeof(gen ## _g ## _null_state_relocs) / 4; \
+		_d.reloc_64bit = _r;					\
+	}
+
+	switch (gen) {
+	case 6:
+		RS_SETUP(ro_data, 6, false);
+		break;
+	case 7:
+		RS_SETUP(ro_data, 7, false);
+		break;
+	case 8:
+		RS_SETUP(ro_data, 8, true);
+		break;
+	default:
+		return -ENOENT;
+	}
+#undef RS_SETUP
+
+	return render_state_copy(so, &ro_data);
+}
+
+int i915_gem_init_render_state(struct intel_ring_buffer *ring)
+{
+	const int gen = INTEL_INFO(ring->dev)->gen;
+	struct i915_render_state *so;
+	u32 seqno;
+	int ret;
+
+	if (gen < 6)
+		return 0;
+
+	so = render_state_alloc(ring->dev, BATCH_MAX_SIZE);
+	if (IS_ERR(so))
+		return PTR_ERR(so);
+
+	ret = render_state_get(so, gen);
+	if (ret)
+		goto out;
+
+	ret = ring->dispatch_execbuffer(ring,
+					i915_gem_obj_ggtt_offset(so->obj),
+					so->len,
+					I915_DISPATCH_SECURE);
+	if (ret)
+		goto out;
+
+	ret = intel_ring_flush_all_caches(ring);
+	if (ret)
+		goto out;
+
+	ret = i915_add_request(ring, &seqno);
+	if (ret)
+		goto out;
+
+	ret = i915_wait_seqno(ring, seqno);
+out:
+	render_state_free(so);
+	return ret;
+}
diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen6.h b/drivers/gpu/drm/i915/intel_renderstate_gen6.h
new file mode 100644
index 0000000..4809f2f
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_renderstate_gen6.h
@@ -0,0 +1,11 @@
+#ifndef __INTEL_RENDERSTATE_GEN6
+#define __INTEL_RENDERSTATE_GEN6
+
+static const uint32_t gen6_null_state_relocs[] = {
+};
+
+static const uint32_t gen6_null_state_batch[] = {
+	MI_BATCH_BUFFER_END,
+};
+
+#endif /* __INTEL_RENDERSTATE_GEN6 */
diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen7.h b/drivers/gpu/drm/i915/intel_renderstate_gen7.h
new file mode 100644
index 0000000..9b1420b
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_renderstate_gen7.h
@@ -0,0 +1,11 @@
+#ifndef __INTEL_RENDERSTATE_GEN7
+#define __INTEL_RENDERSTATE_GEN7
+
+static const uint32_t gen7_null_state_relocs[] = {
+};
+
+static const uint32_t gen7_null_state_batch[] = {
+	MI_BATCH_BUFFER_END,
+};
+
+#endif /* __INTEL_RENDERSTATE_GEN7 */
diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen8.h b/drivers/gpu/drm/i915/intel_renderstate_gen8.h
new file mode 100644
index 0000000..d349dda
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_renderstate_gen8.h
@@ -0,0 +1,11 @@
+#ifndef __INTEL_RENDERSTATE_GEN8
+#define __INTEL_RENDERSTATE_GEN8
+
+static const uint32_t gen8_null_state_relocs[] = {
+};
+
+static const uint32_t gen8_null_state_batch[] = {
+	MI_BATCH_BUFFER_END,
+};
+
+#endif /* __INTEL_RENDERSTATE_GEN8 */
-- 
1.7.9.5

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

[PATCH 3/3] drm/i915: add null render states for gen6, gen7 and gen8

[Mika Kuoppala]

These are generated with intel-gpu-tools/tools/null_state_gen.

Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/intel_renderstate_gen6.h |  281 ++++++++++++++-
 drivers/gpu/drm/i915/intel_renderstate_gen7.h |  245 ++++++++++++-
 drivers/gpu/drm/i915/intel_renderstate_gen8.h |  471 ++++++++++++++++++++++++-
 3 files changed, 994 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen6.h b/drivers/gpu/drm/i915/intel_renderstate_gen6.h
index 4809f2f..5dea85c 100644
--- a/drivers/gpu/drm/i915/intel_renderstate_gen6.h
+++ b/drivers/gpu/drm/i915/intel_renderstate_gen6.h
@@ -2,10 +2,289 @@
 #define __INTEL_RENDERSTATE_GEN6
 
 static const uint32_t gen6_null_state_relocs[] = {
+	0x00000020,
+	0x00000024,
+	0x0000002c,
+	0x000001e0,
+	0x000001e4,
 };
 
 static const uint32_t gen6_null_state_batch[] = {
-	MI_BATCH_BUFFER_END,
+	0x69040000,
+	0x790d0001,
+	0x00000000,
+	0x00000000,
+	0x78180000,
+	0x00000001,
+	0x61010008,
+	0x00000000,
+	0x00000001,	 /* reloc */
+	0x00000001,	 /* reloc */
+	0x00000000,
+	0x00000001,	 /* reloc */
+	0x00000000,
+	0x00000001,
+	0x00000000,
+	0x00000001,
+	0x61020000,
+	0x00000000,
+	0x78050001,
+	0x00000018,
+	0x00000000,
+	0x780d1002,
+	0x00000000,
+	0x00000000,
+	0x00000420,
+	0x78150003,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78100004,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78160003,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78110005,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78120002,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78170003,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x79050005,
+	0xe0040000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x79100000,
+	0x00000000,
+	0x79000002,
+	0xffffffff,
+	0x00000000,
+	0x00000000,
+	0x780e0002,
+	0x00000441,
+	0x00000401,
+	0x00000401,
+	0x78021002,
+	0x00000000,
+	0x00000000,
+	0x00000400,
+	0x78130012,
+	0x00400810,
+	0x00000000,
+	0x20000000,
+	0x04000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78140007,
+	0x00000280,
+	0x08080000,
+	0x00000000,
+	0x00060000,
+	0x4e080002,
+	0x00100400,
+	0x00000000,
+	0x00000000,
+	0x78090005,
+	0x02000000,
+	0x22220000,
+	0x02f60000,
+	0x11330000,
+	0x02850004,
+	0x11220000,
+	0x78011002,
+	0x00000000,
+	0x00000000,
+	0x00000200,
+	0x78080003,
+	0x00002000,
+	0x00000448,	 /* reloc */
+	0x00000448,	 /* reloc */
+	0x00000000,
+	0x05000000,	 /* cmds end */
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000220,	 /* state start */
+	0x00000240,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x0060005a,
+	0x204077be,
+	0x000000c0,
+	0x008d0040,
+	0x0060005a,
+	0x206077be,
+	0x000000c0,
+	0x008d0080,
+	0x0060005a,
+	0x208077be,
+	0x000000d0,
+	0x008d0040,
+	0x0060005a,
+	0x20a077be,
+	0x000000d0,
+	0x008d0080,
+	0x00000201,
+	0x20080061,
+	0x00000000,
+	0x00000000,
+	0x00600001,
+	0x20200022,
+	0x008d0000,
+	0x00000000,
+	0x02800031,
+	0x21c01cc9,
+	0x00000020,
+	0x0a8a0001,
+	0x00600001,
+	0x204003be,
+	0x008d01c0,
+	0x00000000,
+	0x00600001,
+	0x206003be,
+	0x008d01e0,
+	0x00000000,
+	0x00600001,
+	0x208003be,
+	0x008d0200,
+	0x00000000,
+	0x00600001,
+	0x20a003be,
+	0x008d0220,
+	0x00000000,
+	0x00600001,
+	0x20c003be,
+	0x008d0240,
+	0x00000000,
+	0x00600001,
+	0x20e003be,
+	0x008d0260,
+	0x00000000,
+	0x00600001,
+	0x210003be,
+	0x008d0280,
+	0x00000000,
+	0x00600001,
+	0x212003be,
+	0x008d02a0,
+	0x00000000,
+	0x05800031,
+	0x24001cc8,
+	0x00000040,
+	0x90019000,
+	0x0000007e,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x0000007e,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x0000007e,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x0000007e,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x0000007e,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x0000007e,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x0000007e,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x0000007e,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x30000000,
+	0x00000124,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0xf99a130c,
+	0x799a130c,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x80000031,
+	0x00000003,
+	0x00000000,	 /* state end */
 };
 
 #endif /* __INTEL_RENDERSTATE_GEN6 */
diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen7.h b/drivers/gpu/drm/i915/intel_renderstate_gen7.h
index 9b1420b..0b0ca64 100644
--- a/drivers/gpu/drm/i915/intel_renderstate_gen7.h
+++ b/drivers/gpu/drm/i915/intel_renderstate_gen7.h
@@ -2,10 +2,253 @@
 #define __INTEL_RENDERSTATE_GEN7
 
 static const uint32_t gen7_null_state_relocs[] = {
+	0x0000000c,
+	0x00000010,
+	0x00000018,
+	0x000001ec,
 };
 
 static const uint32_t gen7_null_state_batch[] = {
-	MI_BATCH_BUFFER_END,
+	0x69040000,
+	0x61010008,
+	0x00000000,
+	0x00000001,	 /* reloc */
+	0x00000001,	 /* reloc */
+	0x00000000,
+	0x00000001,	 /* reloc */
+	0x00000000,
+	0x00000001,
+	0x00000000,
+	0x00000001,
+	0x790d0002,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78180000,
+	0x00000001,
+	0x79160000,
+	0x00000008,
+	0x78300000,
+	0x02010040,
+	0x78310000,
+	0x04000000,
+	0x78320000,
+	0x04000000,
+	0x78330000,
+	0x02000000,
+	0x78100004,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x781b0005,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x781c0002,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x781d0004,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78110005,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78120002,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78210000,
+	0x00000000,
+	0x78130005,
+	0x00000000,
+	0x20000000,
+	0x04000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78140001,
+	0x20000800,
+	0x00000000,
+	0x781e0001,
+	0x00000000,
+	0x00000000,
+	0x78050005,
+	0xe0040000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78040001,
+	0x00000000,
+	0x00000000,
+	0x78240000,
+	0x00000240,
+	0x78230000,
+	0x00000260,
+	0x782f0000,
+	0x00000280,
+	0x781f000c,
+	0x00400810,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78200006,
+	0x000002c0,
+	0x08080000,
+	0x00000000,
+	0x28000402,
+	0x00060000,
+	0x00000000,
+	0x00000000,
+	0x78090005,
+	0x02000000,
+	0x22220000,
+	0x02f60000,
+	0x11230000,
+	0x02f60004,
+	0x11230000,
+	0x78080003,
+	0x00006008,
+	0x00000340,	 /* reloc */
+	0xffffffff,
+	0x00000000,
+	0x782a0000,
+	0x00000360,
+	0x79000002,
+	0xffffffff,
+	0x00000000,
+	0x00000000,
+	0x7b000005,
+	0x0000000f,
+	0x00000003,
+	0x00000000,
+	0x00000001,
+	0x00000000,
+	0x00000000,
+	0x05000000,	 /* cmds end */
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000031,	 /* state start */
+	0x00000003,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0xf99a130c,
+	0x799a130c,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000492,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x0080005a,
+	0x2e2077bd,
+	0x000000c0,
+	0x008d0040,
+	0x0080005a,
+	0x2e6077bd,
+	0x000000d0,
+	0x008d0040,
+	0x02800031,
+	0x21801fa9,
+	0x008d0e20,
+	0x08840001,
+	0x00800001,
+	0x2e2003bd,
+	0x008d0180,
+	0x00000000,
+	0x00800001,
+	0x2e6003bd,
+	0x008d01c0,
+	0x00000000,
+	0x00800001,
+	0x2ea003bd,
+	0x008d0200,
+	0x00000000,
+	0x00800001,
+	0x2ee003bd,
+	0x008d0240,
+	0x00000000,
+	0x05800031,
+	0x20001fa8,
+	0x008d0e20,
+	0x90031000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000380,
+	0x000003a0,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,	 /* state end */
 };
 
 #endif /* __INTEL_RENDERSTATE_GEN7 */
diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen8.h b/drivers/gpu/drm/i915/intel_renderstate_gen8.h
index d349dda..1e04d25 100644
--- a/drivers/gpu/drm/i915/intel_renderstate_gen8.h
+++ b/drivers/gpu/drm/i915/intel_renderstate_gen8.h
@@ -2,10 +2,479 @@
 #define __INTEL_RENDERSTATE_GEN8
 
 static const uint32_t gen8_null_state_relocs[] = {
+	0x00000048,
+	0x00000050,
+	0x00000060,
+	0x000003ec,
 };
 
 static const uint32_t gen8_null_state_batch[] = {
-	MI_BATCH_BUFFER_END,
+	0x69040000,
+	0x61020001,
+	0x00000000,
+	0x00000000,
+	0x79120000,
+	0x00000000,
+	0x79130000,
+	0x00000000,
+	0x79140000,
+	0x00000000,
+	0x79150000,
+	0x00000000,
+	0x79160000,
+	0x00000000,
+	0x6101000e,
+	0x00000001,
+	0x00000000,
+	0x00000001,
+	0x00000001,	 /* reloc */
+	0x00000000,
+	0x00000001,	 /* reloc */
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000001,	 /* reloc */
+	0x00000000,
+	0xfffff001,
+	0x00001001,
+	0xfffff001,
+	0x00001001,
+	0x78230000,
+	0x000006e0,
+	0x78210000,
+	0x00000700,
+	0x78300000,
+	0x04010040,
+	0x78330000,
+	0x04000000,
+	0x78310000,
+	0x04000000,
+	0x78320000,
+	0x04000000,
+	0x78240000,
+	0x00000641,
+	0x780e0000,
+	0x00000601,
+	0x780d0000,
+	0x00000000,
+	0x78180000,
+	0x00000001,
+	0x78520003,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78190009,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x781b0007,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78270000,
+	0x00000000,
+	0x782c0000,
+	0x00000000,
+	0x781c0002,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78160009,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78110008,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78290000,
+	0x00000000,
+	0x782e0000,
+	0x00000000,
+	0x781a0009,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x781d0007,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78280000,
+	0x00000000,
+	0x782d0000,
+	0x00000000,
+	0x78260000,
+	0x00000000,
+	0x782b0000,
+	0x00000000,
+	0x78150009,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78100007,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x781e0003,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78120002,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x781f0002,
+	0x30400820,
+	0x00000000,
+	0x00000000,
+	0x78510009,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78500003,
+	0x00210000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78130002,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x782a0000,
+	0x00000480,
+	0x782f0000,
+	0x00000540,
+	0x78140000,
+	0x00000800,
+	0x78170009,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x7820000a,
+	0x00000580,
+	0x00000000,
+	0x08080000,
+	0x00000000,
+	0x00000000,
+	0x1f000002,
+	0x00060000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x784d0000,
+	0x40000000,
+	0x784f0000,
+	0x80000100,
+	0x780f0000,
+	0x00000740,
+	0x78050006,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78070003,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78060003,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x78040001,
+	0x00000000,
+	0x00000001,
+	0x79000002,
+	0xffffffff,
+	0x00000000,
+	0x00000000,
+	0x78080003,
+	0x00006000,
+	0x000005e0,	 /* reloc */
+	0x00000000,
+	0x00000000,
+	0x78090005,
+	0x02000000,
+	0x22220000,
+	0x02f60000,
+	0x11230000,
+	0x02850004,
+	0x11230000,
+	0x784b0000,
+	0x0000000f,
+	0x78490001,
+	0x00000000,
+	0x00000000,
+	0x7b000005,
+	0x00000000,
+	0x00000003,
+	0x00000000,
+	0x00000001,
+	0x00000000,
+	0x00000000,
+	0x05000000,	 /* cmds end */
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x000004c0,	 /* state start */
+	0x00000500,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000092,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x0060005a,
+	0x21403ae8,
+	0x3a0000c0,
+	0x008d0040,
+	0x0060005a,
+	0x21603ae8,
+	0x3a0000c0,
+	0x008d0080,
+	0x0060005a,
+	0x21803ae8,
+	0x3a0000d0,
+	0x008d0040,
+	0x0060005a,
+	0x21a03ae8,
+	0x3a0000d0,
+	0x008d0080,
+	0x02800031,
+	0x2e0022e8,
+	0x0e000140,
+	0x08840001,
+	0x05800031,
+	0x200022e0,
+	0x0e000e00,
+	0x90031000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x06200000,
+	0x00000002,
+	0x06200000,
+	0x00000002,
+	0x06200000,
+	0x00000002,
+	0x06200000,
+	0x00000002,
+	0x06200000,
+	0x00000002,
+	0x06200000,
+	0x00000002,
+	0x06200000,
+	0x00000002,
+	0x06200000,
+	0x00000002,
+	0x06200000,
+	0x00000002,
+	0x06200000,
+	0x00000002,
+	0x06200000,
+	0x00000002,
+	0x06200000,
+	0x00000002,
+	0x06200000,
+	0x00000002,
+	0x06200000,
+	0x00000002,
+	0x06200000,
+	0x00000002,
+	0x06200000,
+	0x00000002,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0xf99a130c,
+	0x799a130c,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x3f800000,
+	0x00000000,
+	0x3f800000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,
+	0x00000000,	 /* state end */
 };
 
 #endif /* __INTEL_RENDERSTATE_GEN8 */
-- 
1.7.9.5

Re: [PATCH 2/3] drm/i915: add render state initialization

[Daniel Vetter]

On Tue, Apr 22, 2014 at 08:19:43PM +0300, Mika Kuoppala wrote:
> HW guys say that it is not a cool idea to let device
> go into rc6 without proper 3d pipeline state.
> 
> For each new uninitialized context, generate a
> valid null render state to be run on context
> creation.
> 
> This patch introduces a skeleton with emty states.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile                 |    1 +
>  drivers/gpu/drm/i915/i915_drv.h               |    2 +
>  drivers/gpu/drm/i915/i915_gem_context.c       |    7 +
>  drivers/gpu/drm/i915/i915_gem_render_state.c  |  222 +++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_renderstate_gen6.h |   11 ++
>  drivers/gpu/drm/i915/intel_renderstate_gen7.h |   11 ++
>  drivers/gpu/drm/i915/intel_renderstate_gen8.h |   11 ++

Imo static const arrays in headers look ugly ... I prefer we just add
normal C files with forward declarations in i915_gem_render_state.c.

In the i915/Makefile you could add a new section for all these golden
renderstate files. I know this is a bikeshed, but ;-)
-Daniel

>  7 files changed, 265 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/i915_gem_render_state.c
>  create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen6.h
>  create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen7.h
>  create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen8.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index b1445b7..b5d4029 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -18,6 +18,7 @@ i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
>  # GEM code
>  i915-y += i915_cmd_parser.o \
>  	  i915_gem_context.o \
> +	  i915_gem_render_state.o \
>  	  i915_gem_debug.o \
>  	  i915_gem_dmabuf.o \
>  	  i915_gem_evict.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 43b022c..f6ae2ee 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2330,6 +2330,8 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
>  int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
>  				   struct drm_file *file);
>  
> +/* i915_gem_render_state.c */
> +int i915_gem_init_render_state(struct intel_ring_buffer *ring);
>  /* i915_gem_evict.c */
>  int __must_check i915_gem_evict_something(struct drm_device *dev,
>  					  struct i915_address_space *vm,
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 30b355a..d648d4d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -746,6 +746,13 @@ static int do_switch(struct intel_ring_buffer *ring,
>  		/* obj is kept alive until the next request by its active ref */
>  		i915_gem_object_ggtt_unpin(from->obj);
>  		i915_gem_context_unreference(from);
> +	} else {
> +		if (ring->id == RCS && to->is_initialized == false) {
> +
> +			ret = i915_gem_init_render_state(ring);
> +			if (ret)
> +				DRM_ERROR("init render state: %d\n", ret);
> +		}
>  	}
>  
>  	to->is_initialized = true;
> diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
> new file mode 100644
> index 0000000..409f99b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
> @@ -0,0 +1,222 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *    Mika Kuoppala <mika.kuoppala@intel.com>
> + *
> + */
> +
> +#include "i915_drv.h"
> +
> +#include "intel_renderstate_gen6.h"
> +#include "intel_renderstate_gen7.h"
> +#include "intel_renderstate_gen8.h"
> +
> +#define BATCH_MAX_SIZE 4096
> +
> +struct i915_render_state {
> +	struct drm_i915_gem_object *obj;
> +	unsigned long ggtt_offset;
> +	void *batch;
> +	u32 size;
> +	u32 len;
> +};
> +
> +static struct i915_render_state *
> +render_state_alloc(struct drm_device *dev, unsigned int size)
> +{
> +	struct i915_render_state *so;
> +	int ret;
> +
> +	so = kzalloc(sizeof(*so), GFP_KERNEL);
> +	if (!so)
> +		return ERR_PTR(-ENOMEM);
> +
> +	so->size = ALIGN(size, 4096);
> +
> +	so->obj = i915_gem_alloc_object(dev, so->size);
> +	if (so->obj == NULL) {
> +		ret = -ENOMEM;
> +		goto free;
> +	}
> +
> +	ret = i915_gem_obj_ggtt_pin(so->obj, 4096, 0);
> +	if (ret)
> +		goto free_gem;
> +
> +	so->batch = i915_gem_vmap_obj(so->obj);
> +	if (!so->batch) {
> +		ret = -ENOMEM;
> +		goto unpin;
> +	}
> +
> +	so->ggtt_offset = i915_gem_obj_ggtt_offset(so->obj);
> +
> +	return so;
> +unpin:
> +	i915_gem_object_ggtt_unpin(so->obj);
> +free_gem:
> +	drm_gem_object_unreference(&so->obj->base);
> +free:
> +	kfree(so);
> +	return ERR_PTR(ret);
> +}
> +
> +static void render_state_free(struct i915_render_state *so)
> +{
> +	vunmap(so->batch);
> +	i915_gem_object_ggtt_unpin(so->obj);
> +	drm_gem_object_unreference(&so->obj->base);
> +	kfree(so);
> +}
> +
> +struct render_state_ro_data {
> +	const u32 *batch;
> +	unsigned int batch_items;
> +	const u32 *reloc;
> +	unsigned int reloc_items;
> +	bool reloc_64bit;
> +};
> +
> +static int render_state_copy(struct i915_render_state *so,
> +			     const struct render_state_ro_data *source)
> +{
> +	const u64 goffset = i915_gem_obj_ggtt_offset(so->obj);
> +	const unsigned int num_relocs = source->reloc_items;
> +	int reloc_index = 0;
> +	u32 *d = (uint32_t *)so->batch;
> +	unsigned int i = 0;
> +
> +	if (source->batch_items * sizeof(*source->batch) > so->size)
> +		return -EINVAL;
> +
> +	while (i < source->batch_items) {
> +		u32 s = source->batch[i];
> +
> +		if (reloc_index < num_relocs &&
> +		    i * 4  == source->reloc[reloc_index]) {
> +
> +			s += goffset & 0xffffffff;
> +
> +			/* We keep batch offsets max 32bit */
> +			if (source->reloc_64bit) {
> +				if (i + 1 >= source->batch_items ||
> +				    source->batch[i + 1] != 0)
> +					return -EINVAL;
> +
> +				d[i] = s;
> +				i++;
> +				s = (goffset & 0xffffffff00000000ull) >> 32;
> +			}
> +
> +			reloc_index++;
> +		}
> +
> +		d[i] = s;
> +		i++;
> +	}
> +
> +	if (num_relocs != reloc_index) {
> +		DRM_ERROR("not all relocs resolved, %d out of %d\n",
> +			  reloc_index, num_relocs);
> +		return -EINVAL;
> +	}
> +
> +	so->len = source->batch_items * sizeof(*source->batch);
> +
> +	return 0;
> +}
> +
> +static int render_state_get(struct i915_render_state *so, int gen)
> +{
> +	struct render_state_ro_data ro_data;
> +
> +#define RS_SETUP(_d, _g, _r) {						\
> +		if (!sizeof(gen ## _g ## _null_state_batch))		\
> +			return -EINVAL;					\
> +		if (sizeof(gen ## _g ## _null_state_batch) & 0x3)	\
> +			return -EINVAL;					\
> +		if (sizeof(gen ## _g ## _null_state_relocs) & 0x3)	\
> +			return -EINVAL;					\
> +		_d.batch = gen ## _g ## _null_state_batch;		\
> +		_d.batch_items = sizeof(gen ##_g ## _null_state_batch) / 4; \
> +		_d.reloc = gen ## _g ## _null_state_relocs;		\
> +		_d.reloc_items = sizeof(gen ## _g ## _null_state_relocs) / 4; \
> +		_d.reloc_64bit = _r;					\
> +	}
> +
> +	switch (gen) {
> +	case 6:
> +		RS_SETUP(ro_data, 6, false);
> +		break;
> +	case 7:
> +		RS_SETUP(ro_data, 7, false);
> +		break;
> +	case 8:
> +		RS_SETUP(ro_data, 8, true);
> +		break;
> +	default:
> +		return -ENOENT;
> +	}
> +#undef RS_SETUP
> +
> +	return render_state_copy(so, &ro_data);
> +}
> +
> +int i915_gem_init_render_state(struct intel_ring_buffer *ring)
> +{
> +	const int gen = INTEL_INFO(ring->dev)->gen;
> +	struct i915_render_state *so;
> +	u32 seqno;
> +	int ret;
> +
> +	if (gen < 6)
> +		return 0;
> +
> +	so = render_state_alloc(ring->dev, BATCH_MAX_SIZE);
> +	if (IS_ERR(so))
> +		return PTR_ERR(so);
> +
> +	ret = render_state_get(so, gen);
> +	if (ret)
> +		goto out;
> +
> +	ret = ring->dispatch_execbuffer(ring,
> +					i915_gem_obj_ggtt_offset(so->obj),
> +					so->len,
> +					I915_DISPATCH_SECURE);
> +	if (ret)
> +		goto out;
> +
> +	ret = intel_ring_flush_all_caches(ring);
> +	if (ret)
> +		goto out;
> +
> +	ret = i915_add_request(ring, &seqno);
> +	if (ret)
> +		goto out;
> +
> +	ret = i915_wait_seqno(ring, seqno);
> +out:
> +	render_state_free(so);
> +	return ret;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen6.h b/drivers/gpu/drm/i915/intel_renderstate_gen6.h
> new file mode 100644
> index 0000000..4809f2f
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_renderstate_gen6.h
> @@ -0,0 +1,11 @@
> +#ifndef __INTEL_RENDERSTATE_GEN6
> +#define __INTEL_RENDERSTATE_GEN6
> +
> +static const uint32_t gen6_null_state_relocs[] = {
> +};
> +
> +static const uint32_t gen6_null_state_batch[] = {
> +	MI_BATCH_BUFFER_END,
> +};
> +
> +#endif /* __INTEL_RENDERSTATE_GEN6 */
> diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen7.h b/drivers/gpu/drm/i915/intel_renderstate_gen7.h
> new file mode 100644
> index 0000000..9b1420b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_renderstate_gen7.h
> @@ -0,0 +1,11 @@
> +#ifndef __INTEL_RENDERSTATE_GEN7
> +#define __INTEL_RENDERSTATE_GEN7
> +
> +static const uint32_t gen7_null_state_relocs[] = {
> +};
> +
> +static const uint32_t gen7_null_state_batch[] = {
> +	MI_BATCH_BUFFER_END,
> +};
> +
> +#endif /* __INTEL_RENDERSTATE_GEN7 */
> diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen8.h b/drivers/gpu/drm/i915/intel_renderstate_gen8.h
> new file mode 100644
> index 0000000..d349dda
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_renderstate_gen8.h
> @@ -0,0 +1,11 @@
> +#ifndef __INTEL_RENDERSTATE_GEN8
> +#define __INTEL_RENDERSTATE_GEN8
> +
> +static const uint32_t gen8_null_state_relocs[] = {
> +};
> +
> +static const uint32_t gen8_null_state_batch[] = {
> +	MI_BATCH_BUFFER_END,
> +};
> +
> +#endif /* __INTEL_RENDERSTATE_GEN8 */
> -- 
> 1.7.9.5
> 
> _______________________________________________
> 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

Re: [RFC 0/3] render state initialization (bdw rc6)

[Chris Wilson]

On Tue, Apr 22, 2014 at 08:19:41PM +0300, Mika Kuoppala wrote:
> Hi,
> 
> Here are patches to initialize first render context to a hopefully, 
> valid state. If pipeline/context is not initialized and we enter rc6 on bdw,
> the render ring can hung on the first batch submitted. That is atleast
> the hypothesis this work is based on.
> 
> The states are stripped from rendercopy_genX's from i-g-t/lib and
> the state generators are part of i-g-t also. The states are
> propably overshoot from what can be consider the minimal valid
> (null) state on the pipeline. I just initialized everything rendercopy
> does and haven't really put effort on optimizing the state until
> I get some test results that this indeed solves anything.
> 
> The state generators can be found here but they are not needed for testing.
> http://cgit.freedesktop.org/~miku/intel-gpu-tools/log/?h=null_state_gen
> 
> Gen7 and Gen8 seems to atleast survive the boot but Gen6 is totally
> untested.
> 
> Here is the branch for testing:
> http://cgit.freedesktop.org/~miku/drm-intel/log/?h=render_state
> 
> I am interested to know if these patches make matters better/worse for those
> who have issues with first batch hanging on bdw, but as always, any feedback
> is greatly appreciated.
> 
> Mika Kuoppala (3):
>   drm/i915: export vmap_batch from command parser
It's only a single page, it does not need to be vmapped.

>   drm/i915: add render state initialization
>   drm/i915: add null render states for gen6, gen7 and gen8

I still don't buy that this is anything more than papering over a
problem. The state you load into the context is invalid as soon as it is
executed, which may lead to problems, we don't know since the problem is
not being discussed, and it will certainly be more explicit if the right
bits are poked into the context directly to keep the hw from falling over.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [RFC 0/3] render state initialization (bdw rc6)

[Ben Widawsky]

On Tue, Apr 22, 2014 at 09:51:56PM +0100, Chris Wilson wrote:
> On Tue, Apr 22, 2014 at 08:19:41PM +0300, Mika Kuoppala wrote:
> > Hi,
> > 
> > Here are patches to initialize first render context to a hopefully, 
> > valid state. If pipeline/context is not initialized and we enter rc6 on bdw,
> > the render ring can hung on the first batch submitted. That is atleast
> > the hypothesis this work is based on.
> > 
> > The states are stripped from rendercopy_genX's from i-g-t/lib and
> > the state generators are part of i-g-t also. The states are
> > propably overshoot from what can be consider the minimal valid
> > (null) state on the pipeline. I just initialized everything rendercopy
> > does and haven't really put effort on optimizing the state until
> > I get some test results that this indeed solves anything.
> > 
> > The state generators can be found here but they are not needed for testing.
> > http://cgit.freedesktop.org/~miku/intel-gpu-tools/log/?h=null_state_gen
> > 
> > Gen7 and Gen8 seems to atleast survive the boot but Gen6 is totally
> > untested.
> > 
> > Here is the branch for testing:
> > http://cgit.freedesktop.org/~miku/drm-intel/log/?h=render_state
> > 
> > I am interested to know if these patches make matters better/worse for those
> > who have issues with first batch hanging on bdw, but as always, any feedback
> > is greatly appreciated.
> > 
> > Mika Kuoppala (3):
> >   drm/i915: export vmap_batch from command parser
> It's only a single page, it does not need to be vmapped.
> 
> >   drm/i915: add render state initialization
> >   drm/i915: add null render states for gen6, gen7 and gen8
> 
> I still don't buy that this is anything more than papering over a
> problem. The state you load into the context is invalid as soon as it is
> executed, which may lead to problems, we don't know since the problem is
> not being discussed, and it will certainly be more explicit if the right
> bits are poked into the context directly to keep the hw from falling over.
> -Chris
> 

Paper is better than no paper. Anyway there are a couple of units where
we know NULL is better than not NULL (VFE is one). I have been unable to
get an exact reason why this is needed so that we know exactly what to
fix. It has been a very frustrating experience.

We could try to get info on what Windows does, but we may not be able to
get a, "why."

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [RFC 0/3] render state initialization (bdw rc6)

[Kristen Carlson Accardi]

On Tue, 22 Apr 2014 20:19:41 +0300
Mika Kuoppala <mika.kuoppala@linux.intel.com> wrote:

> Hi,
> 
> Here are patches to initialize first render context to a hopefully, 
> valid state. If pipeline/context is not initialized and we enter rc6 on bdw,
> the render ring can hung on the first batch submitted. That is atleast
> the hypothesis this work is based on.
> 
> The states are stripped from rendercopy_genX's from i-g-t/lib and
> the state generators are part of i-g-t also. The states are
> propably overshoot from what can be consider the minimal valid
> (null) state on the pipeline. I just initialized everything rendercopy
> does and haven't really put effort on optimizing the state until
> I get some test results that this indeed solves anything.
> 
> The state generators can be found here but they are not needed for testing.
> http://cgit.freedesktop.org/~miku/intel-gpu-tools/log/?h=null_state_gen
> 
> Gen7 and Gen8 seems to atleast survive the boot but Gen6 is totally
> untested.
> 
> Here is the branch for testing:
> http://cgit.freedesktop.org/~miku/drm-intel/log/?h=render_state
> 
> I am interested to know if these patches make matters better/worse for those
> who have issues with first batch hanging on bdw, but as always, any feedback
> is greatly appreciated.

I tested these on a system which has experienced the hang, and they
worked fine for me.

Tested-by:  Kristen Carlson Accardi <kristen@linux.intel.com>

> 
> Mika Kuoppala (3):
>   drm/i915: export vmap_batch from command parser
>   drm/i915: add render state initialization
>   drm/i915: add null render states for gen6, gen7 and gen8
> 
>  drivers/gpu/drm/i915/Makefile                 |    1 +
>  drivers/gpu/drm/i915/i915_cmd_parser.c        |    4 +-
>  drivers/gpu/drm/i915/i915_drv.h               |    3 +
>  drivers/gpu/drm/i915/i915_gem_context.c       |    7 +
>  drivers/gpu/drm/i915/i915_gem_render_state.c  |  222 ++++++++++++
>  drivers/gpu/drm/i915/intel_renderstate_gen6.h |  290 +++++++++++++++
>  drivers/gpu/drm/i915/intel_renderstate_gen7.h |  254 +++++++++++++
>  drivers/gpu/drm/i915/intel_renderstate_gen8.h |  480 +++++++++++++++++++++++++
>  8 files changed, 1259 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/i915_gem_render_state.c
>  create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen6.h
>  create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen7.h
>  create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen8.h
> 

Re: [PATCH 3/3] drm/i915: add null render states for gen6, gen7 and gen8

[Damien Lespiau]

On Tue, Apr 22, 2014 at 08:19:44PM +0300, Mika Kuoppala wrote:
> These are generated with intel-gpu-tools/tools/null_state_gen.

I couldn't find the patches adding this to intel-gpu-tools. Mind posting
them here (or probably just push them to i-g-t?)

Thanks,

-- 
Damien

Re: [PATCH 3/3] drm/i915: add null render states for gen6, gen7 and gen8

[Damien Lespiau]

On Fri, May 02, 2014 at 09:43:29PM +0100, Damien Lespiau wrote:
> On Tue, Apr 22, 2014 at 08:19:44PM +0300, Mika Kuoppala wrote:
> > These are generated with intel-gpu-tools/tools/null_state_gen.
> 
> I couldn't find the patches adding this to intel-gpu-tools. Mind posting
> them here (or probably just push them to i-g-t?)

Ah, nm, the branch was mentionned in your cover letter.

-- 
Damien

Re: [PATCH 2/3] drm/i915: add render state initialization

[Ben Widawsky]

On Tue, Apr 22, 2014 at 08:19:43PM +0300, Mika Kuoppala wrote:
> HW guys say that it is not a cool idea to let device
> go into rc6 without proper 3d pipeline state.
> 
> For each new uninitialized context, generate a
> valid null render state to be run on context
> creation.
> 
> This patch introduces a skeleton with emty states.
> 
> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile                 |    1 +
>  drivers/gpu/drm/i915/i915_drv.h               |    2 +
>  drivers/gpu/drm/i915/i915_gem_context.c       |    7 +
>  drivers/gpu/drm/i915/i915_gem_render_state.c  |  222 +++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_renderstate_gen6.h |   11 ++
>  drivers/gpu/drm/i915/intel_renderstate_gen7.h |   11 ++
>  drivers/gpu/drm/i915/intel_renderstate_gen8.h |   11 ++
>  7 files changed, 265 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/i915_gem_render_state.c
>  create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen6.h
>  create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen7.h
>  create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen8.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index b1445b7..b5d4029 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -18,6 +18,7 @@ i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
>  # GEM code
>  i915-y += i915_cmd_parser.o \
>  	  i915_gem_context.o \
> +	  i915_gem_render_state.o \
>  	  i915_gem_debug.o \
>  	  i915_gem_dmabuf.o \
>  	  i915_gem_evict.o \
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 43b022c..f6ae2ee 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2330,6 +2330,8 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
>  int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
>  				   struct drm_file *file);
>  
> +/* i915_gem_render_state.c */
> +int i915_gem_init_render_state(struct intel_ring_buffer *ring);
>  /* i915_gem_evict.c */
>  int __must_check i915_gem_evict_something(struct drm_device *dev,
>  					  struct i915_address_space *vm,
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 30b355a..d648d4d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -746,6 +746,13 @@ static int do_switch(struct intel_ring_buffer *ring,
>  		/* obj is kept alive until the next request by its active ref */
>  		i915_gem_object_ggtt_unpin(from->obj);
>  		i915_gem_context_unreference(from);
> +	} else {
> +		if (ring->id == RCS && to->is_initialized == false) {
> +
> +			ret = i915_gem_init_render_state(ring);
> +			if (ret)
> +				DRM_ERROR("init render state: %d\n", ret);
> +		}

The way I thought we'd do this was to emit the state once for the
default context, and copy those pages to the BO of the new context.

>  	}
>  
>  	to->is_initialized = true;
> diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
> new file mode 100644
> index 0000000..409f99b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
> @@ -0,0 +1,222 @@
> +/*
> + * Copyright © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + *    Mika Kuoppala <mika.kuoppala@intel.com>
> + *
> + */
> +
> +#include "i915_drv.h"
> +
> +#include "intel_renderstate_gen6.h"
> +#include "intel_renderstate_gen7.h"
> +#include "intel_renderstate_gen8.h"
> +
> +#define BATCH_MAX_SIZE 4096
> +
> +struct i915_render_state {
> +	struct drm_i915_gem_object *obj;
> +	unsigned long ggtt_offset;
> +	void *batch;
> +	u32 size;
> +	u32 len;
> +};
> +
> +static struct i915_render_state *
> +render_state_alloc(struct drm_device *dev, unsigned int size)
> +{
> +	struct i915_render_state *so;
> +	int ret;
> +
> +	so = kzalloc(sizeof(*so), GFP_KERNEL);
> +	if (!so)
> +		return ERR_PTR(-ENOMEM);
> +
> +	so->size = ALIGN(size, 4096);
> +
> +	so->obj = i915_gem_alloc_object(dev, so->size);
> +	if (so->obj == NULL) {
> +		ret = -ENOMEM;
> +		goto free;
> +	}
> +
> +	ret = i915_gem_obj_ggtt_pin(so->obj, 4096, 0);
> +	if (ret)
> +		goto free_gem;

Using dispatch the way you do below, we'll actually be executing out of
the PPGTT, and not the global. While this makes no difference on the
current code, it does make a difference for some of my plans. I don't
see a clean way to fix this with the current code base, I am just
complaining.

> +
> +	so->batch = i915_gem_vmap_obj(so->obj);
> +	if (!so->batch) {
> +		ret = -ENOMEM;
> +		goto unpin;
> +	}
> +
> +	so->ggtt_offset = i915_gem_obj_ggtt_offset(so->obj);
> +
> +	return so;
> +unpin:
> +	i915_gem_object_ggtt_unpin(so->obj);
> +free_gem:
> +	drm_gem_object_unreference(&so->obj->base);
> +free:
> +	kfree(so);
> +	return ERR_PTR(ret);
> +}
> +
> +static void render_state_free(struct i915_render_state *so)
> +{
> +	vunmap(so->batch);
> +	i915_gem_object_ggtt_unpin(so->obj);
> +	drm_gem_object_unreference(&so->obj->base);
> +	kfree(so);
> +}
> +
> +struct render_state_ro_data {
> +	const u32 *batch;
> +	unsigned int batch_items;
> +	const u32 *reloc;
> +	unsigned int reloc_items;
> +	bool reloc_64bit;
> +};
> +
> +static int render_state_copy(struct i915_render_state *so,
> +			     const struct render_state_ro_data *source)
> +{
> +	const u64 goffset = i915_gem_obj_ggtt_offset(so->obj);
> +	const unsigned int num_relocs = source->reloc_items;
> +	int reloc_index = 0;
> +	u32 *d = (uint32_t *)so->batch;
> +	unsigned int i = 0;
> +
> +	if (source->batch_items * sizeof(*source->batch) > so->size)
> +		return -EINVAL;
> +
> +	while (i < source->batch_items) {
> +		u32 s = source->batch[i];
> +
> +		if (reloc_index < num_relocs &&
> +		    i * 4  == source->reloc[reloc_index]) {
> +
> +			s += goffset & 0xffffffff;
> +
> +			/* We keep batch offsets max 32bit */
> +			if (source->reloc_64bit) {
> +				if (i + 1 >= source->batch_items ||
> +				    source->batch[i + 1] != 0)
> +					return -EINVAL;
> +
> +				d[i] = s;
> +				i++;
> +				s = (goffset & 0xffffffff00000000ull) >> 32;
> +			}
> +
> +			reloc_index++;
> +		}
> +
> +		d[i] = s;
> +		i++;
> +	}
> +
> +	if (num_relocs != reloc_index) {
> +		DRM_ERROR("not all relocs resolved, %d out of %d\n",
> +			  reloc_index, num_relocs);
> +		return -EINVAL;
> +	}
> +
> +	so->len = source->batch_items * sizeof(*source->batch);
> +
> +	return 0;
> +}
> +
> +static int render_state_get(struct i915_render_state *so, int gen)
> +{
> +	struct render_state_ro_data ro_data;
> +
> +#define RS_SETUP(_d, _g, _r) {						\
> +		if (!sizeof(gen ## _g ## _null_state_batch))		\
> +			return -EINVAL;					\
> +		if (sizeof(gen ## _g ## _null_state_batch) & 0x3)	\
> +			return -EINVAL;					\
> +		if (sizeof(gen ## _g ## _null_state_relocs) & 0x3)	\
> +			return -EINVAL;					\
> +		_d.batch = gen ## _g ## _null_state_batch;		\
> +		_d.batch_items = sizeof(gen ##_g ## _null_state_batch) / 4; \
> +		_d.reloc = gen ## _g ## _null_state_relocs;		\
> +		_d.reloc_items = sizeof(gen ## _g ## _null_state_relocs) / 4; \
> +		_d.reloc_64bit = _r;					\
> +	}
> +
> +	switch (gen) {
> +	case 6:
> +		RS_SETUP(ro_data, 6, false);
> +		break;
> +	case 7:
> +		RS_SETUP(ro_data, 7, false);
> +		break;

I'm a bit surprised we have no differences for HSW/BYT/IVB.

> +	case 8:
> +		RS_SETUP(ro_data, 8, true);
> +		break;
> +	default:
> +		return -ENOENT;
> +	}
> +#undef RS_SETUP
> +
> +	return render_state_copy(so, &ro_data);
> +}
> +
> +int i915_gem_init_render_state(struct intel_ring_buffer *ring)
> +{
> +	const int gen = INTEL_INFO(ring->dev)->gen;
> +	struct i915_render_state *so;
> +	u32 seqno;
> +	int ret;
> +
> +	if (gen < 6)
> +		return 0;

HAS_HW_CONTEXTS
ickle: "Never forget ILK contexts"

> +
> +	so = render_state_alloc(ring->dev, BATCH_MAX_SIZE);
> +	if (IS_ERR(so))
> +		return PTR_ERR(so);
> +
> +	ret = render_state_get(so, gen);
> +	if (ret)
> +		goto out;
> +
> +	ret = ring->dispatch_execbuffer(ring,
> +					i915_gem_obj_ggtt_offset(so->obj),
> +					so->len,
> +					I915_DISPATCH_SECURE);
> +	if (ret)
> +		goto out;
> +
> +	ret = intel_ring_flush_all_caches(ring);
> +	if (ret)
> +		goto out;
> +
> +	ret = i915_add_request(ring, &seqno);
> +	if (ret)
> +		goto out;
> +
> +	ret = i915_wait_seqno(ring, seqno);

This wait is going to hurt. Not seeing why it's needed (plus I still
recommend my idea on the top).

> +out:
> +	render_state_free(so);
> +	return ret;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen6.h b/drivers/gpu/drm/i915/intel_renderstate_gen6.h
> new file mode 100644
> index 0000000..4809f2f
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_renderstate_gen6.h
> @@ -0,0 +1,11 @@
> +#ifndef __INTEL_RENDERSTATE_GEN6
> +#define __INTEL_RENDERSTATE_GEN6
> +
> +static const uint32_t gen6_null_state_relocs[] = {
> +};
> +
> +static const uint32_t gen6_null_state_batch[] = {
> +	MI_BATCH_BUFFER_END,
> +};
> +
> +#endif /* __INTEL_RENDERSTATE_GEN6 */
> diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen7.h b/drivers/gpu/drm/i915/intel_renderstate_gen7.h
> new file mode 100644
> index 0000000..9b1420b
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_renderstate_gen7.h
> @@ -0,0 +1,11 @@
> +#ifndef __INTEL_RENDERSTATE_GEN7
> +#define __INTEL_RENDERSTATE_GEN7
> +
> +static const uint32_t gen7_null_state_relocs[] = {
> +};
> +
> +static const uint32_t gen7_null_state_batch[] = {
> +	MI_BATCH_BUFFER_END,
> +};
> +
> +#endif /* __INTEL_RENDERSTATE_GEN7 */
> diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen8.h b/drivers/gpu/drm/i915/intel_renderstate_gen8.h
> new file mode 100644
> index 0000000..d349dda
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_renderstate_gen8.h
> @@ -0,0 +1,11 @@
> +#ifndef __INTEL_RENDERSTATE_GEN8
> +#define __INTEL_RENDERSTATE_GEN8
> +
> +static const uint32_t gen8_null_state_relocs[] = {
> +};
> +
> +static const uint32_t gen8_null_state_batch[] = {
> +	MI_BATCH_BUFFER_END,
> +};
> +
> +#endif /* __INTEL_RENDERSTATE_GEN8 */
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: add render state initialization

[Mika Kuoppala]

Ben Widawsky <ben@bwidawsk.net> writes:

> On Tue, Apr 22, 2014 at 08:19:43PM +0300, Mika Kuoppala wrote:
>> HW guys say that it is not a cool idea to let device
>> go into rc6 without proper 3d pipeline state.
>> 
>> For each new uninitialized context, generate a
>> valid null render state to be run on context
>> creation.
>> 
>> This patch introduces a skeleton with emty states.
>> 
>> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com>
>> ---
>>  drivers/gpu/drm/i915/Makefile                 |    1 +
>>  drivers/gpu/drm/i915/i915_drv.h               |    2 +
>>  drivers/gpu/drm/i915/i915_gem_context.c       |    7 +
>>  drivers/gpu/drm/i915/i915_gem_render_state.c  |  222 +++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_renderstate_gen6.h |   11 ++
>>  drivers/gpu/drm/i915/intel_renderstate_gen7.h |   11 ++
>>  drivers/gpu/drm/i915/intel_renderstate_gen8.h |   11 ++
>>  7 files changed, 265 insertions(+)
>>  create mode 100644 drivers/gpu/drm/i915/i915_gem_render_state.c
>>  create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen6.h
>>  create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen7.h
>>  create mode 100644 drivers/gpu/drm/i915/intel_renderstate_gen8.h
>> 
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index b1445b7..b5d4029 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -18,6 +18,7 @@ i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o
>>  # GEM code
>>  i915-y += i915_cmd_parser.o \
>>  	  i915_gem_context.o \
>> +	  i915_gem_render_state.o \
>>  	  i915_gem_debug.o \
>>  	  i915_gem_dmabuf.o \
>>  	  i915_gem_evict.o \
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 43b022c..f6ae2ee 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -2330,6 +2330,8 @@ int i915_gem_context_create_ioctl(struct drm_device *dev, void *data,
>>  int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data,
>>  				   struct drm_file *file);
>>  
>> +/* i915_gem_render_state.c */
>> +int i915_gem_init_render_state(struct intel_ring_buffer *ring);
>>  /* i915_gem_evict.c */
>>  int __must_check i915_gem_evict_something(struct drm_device *dev,
>>  					  struct i915_address_space *vm,
>> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
>> index 30b355a..d648d4d 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_context.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
>> @@ -746,6 +746,13 @@ static int do_switch(struct intel_ring_buffer *ring,
>>  		/* obj is kept alive until the next request by its active ref */
>>  		i915_gem_object_ggtt_unpin(from->obj);
>>  		i915_gem_context_unreference(from);
>> +	} else {
>> +		if (ring->id == RCS && to->is_initialized == false) {
>> +
>> +			ret = i915_gem_init_render_state(ring);
>> +			if (ret)
>> +				DRM_ERROR("init render state: %d\n", ret);
>> +		}
>
> The way I thought we'd do this was to emit the state once for the
> default context, and copy those pages to the BO of the new context.

The state is big (on bdw), 18 pages. More on cover letter of v2 series:
1399382766-25116-1-git-send-email-mika.kuoppala@intel.com

>>  	}
>>  
>>  	to->is_initialized = true;
>> diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
>> new file mode 100644
>> index 0000000..409f99b
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
>> @@ -0,0 +1,222 @@
>> +/*
>> + * Copyright © 2014 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + * Authors:
>> + *    Mika Kuoppala <mika.kuoppala@intel.com>
>> + *
>> + */
>> +
>> +#include "i915_drv.h"
>> +
>> +#include "intel_renderstate_gen6.h"
>> +#include "intel_renderstate_gen7.h"
>> +#include "intel_renderstate_gen8.h"
>> +
>> +#define BATCH_MAX_SIZE 4096
>> +
>> +struct i915_render_state {
>> +	struct drm_i915_gem_object *obj;
>> +	unsigned long ggtt_offset;
>> +	void *batch;
>> +	u32 size;
>> +	u32 len;
>> +};
>> +
>> +static struct i915_render_state *
>> +render_state_alloc(struct drm_device *dev, unsigned int size)
>> +{
>> +	struct i915_render_state *so;
>> +	int ret;
>> +
>> +	so = kzalloc(sizeof(*so), GFP_KERNEL);
>> +	if (!so)
>> +		return ERR_PTR(-ENOMEM);
>> +
>> +	so->size = ALIGN(size, 4096);
>> +
>> +	so->obj = i915_gem_alloc_object(dev, so->size);
>> +	if (so->obj == NULL) {
>> +		ret = -ENOMEM;
>> +		goto free;
>> +	}
>> +
>> +	ret = i915_gem_obj_ggtt_pin(so->obj, 4096, 0);
>> +	if (ret)
>> +		goto free_gem;
>
> Using dispatch the way you do below, we'll actually be executing out of
> the PPGTT, and not the global. While this makes no difference on the
> current code, it does make a difference for some of my plans. I don't
> see a clean way to fix this with the current code base, I am just
> complaining.
>
>> +
>> +	so->batch = i915_gem_vmap_obj(so->obj);
>> +	if (!so->batch) {
>> +		ret = -ENOMEM;
>> +		goto unpin;
>> +	}
>> +
>> +	so->ggtt_offset = i915_gem_obj_ggtt_offset(so->obj);
>> +
>> +	return so;
>> +unpin:
>> +	i915_gem_object_ggtt_unpin(so->obj);
>> +free_gem:
>> +	drm_gem_object_unreference(&so->obj->base);
>> +free:
>> +	kfree(so);
>> +	return ERR_PTR(ret);
>> +}
>> +
>> +static void render_state_free(struct i915_render_state *so)
>> +{
>> +	vunmap(so->batch);
>> +	i915_gem_object_ggtt_unpin(so->obj);
>> +	drm_gem_object_unreference(&so->obj->base);
>> +	kfree(so);
>> +}
>> +
>> +struct render_state_ro_data {
>> +	const u32 *batch;
>> +	unsigned int batch_items;
>> +	const u32 *reloc;
>> +	unsigned int reloc_items;
>> +	bool reloc_64bit;
>> +};
>> +
>> +static int render_state_copy(struct i915_render_state *so,
>> +			     const struct render_state_ro_data *source)
>> +{
>> +	const u64 goffset = i915_gem_obj_ggtt_offset(so->obj);
>> +	const unsigned int num_relocs = source->reloc_items;
>> +	int reloc_index = 0;
>> +	u32 *d = (uint32_t *)so->batch;
>> +	unsigned int i = 0;
>> +
>> +	if (source->batch_items * sizeof(*source->batch) > so->size)
>> +		return -EINVAL;
>> +
>> +	while (i < source->batch_items) {
>> +		u32 s = source->batch[i];
>> +
>> +		if (reloc_index < num_relocs &&
>> +		    i * 4  == source->reloc[reloc_index]) {
>> +
>> +			s += goffset & 0xffffffff;
>> +
>> +			/* We keep batch offsets max 32bit */
>> +			if (source->reloc_64bit) {
>> +				if (i + 1 >= source->batch_items ||
>> +				    source->batch[i + 1] != 0)
>> +					return -EINVAL;
>> +
>> +				d[i] = s;
>> +				i++;
>> +				s = (goffset & 0xffffffff00000000ull) >> 32;
>> +			}
>> +
>> +			reloc_index++;
>> +		}
>> +
>> +		d[i] = s;
>> +		i++;
>> +	}
>> +
>> +	if (num_relocs != reloc_index) {
>> +		DRM_ERROR("not all relocs resolved, %d out of %d\n",
>> +			  reloc_index, num_relocs);
>> +		return -EINVAL;
>> +	}
>> +
>> +	so->len = source->batch_items * sizeof(*source->batch);
>> +
>> +	return 0;
>> +}
>> +
>> +static int render_state_get(struct i915_render_state *so, int gen)
>> +{
>> +	struct render_state_ro_data ro_data;
>> +
>> +#define RS_SETUP(_d, _g, _r) {						\
>> +		if (!sizeof(gen ## _g ## _null_state_batch))		\
>> +			return -EINVAL;					\
>> +		if (sizeof(gen ## _g ## _null_state_batch) & 0x3)	\
>> +			return -EINVAL;					\
>> +		if (sizeof(gen ## _g ## _null_state_relocs) & 0x3)	\
>> +			return -EINVAL;					\
>> +		_d.batch = gen ## _g ## _null_state_batch;		\
>> +		_d.batch_items = sizeof(gen ##_g ## _null_state_batch) / 4; \
>> +		_d.reloc = gen ## _g ## _null_state_relocs;		\
>> +		_d.reloc_items = sizeof(gen ## _g ## _null_state_relocs) / 4; \
>> +		_d.reloc_64bit = _r;					\
>> +	}
>> +
>> +	switch (gen) {
>> +	case 6:
>> +		RS_SETUP(ro_data, 6, false);
>> +		break;
>> +	case 7:
>> +		RS_SETUP(ro_data, 7, false);
>> +		break;
>
> I'm a bit surprised we have no differences for HSW/BYT/IVB.

The state generators need eyes on them for sure. There was some difference
between ivb/hsw in rendercopy. But for the null state, I gathered it
doesnt matter.

>> +	case 8:
>> +		RS_SETUP(ro_data, 8, true);
>> +		break;
>> +	default:
>> +		return -ENOENT;
>> +	}
>> +#undef RS_SETUP
>> +
>> +	return render_state_copy(so, &ro_data);
>> +}
>> +
>> +int i915_gem_init_render_state(struct intel_ring_buffer *ring)
>> +{
>> +	const int gen = INTEL_INFO(ring->dev)->gen;
>> +	struct i915_render_state *so;
>> +	u32 seqno;
>> +	int ret;
>> +
>> +	if (gen < 6)
>> +		return 0;
>
> HAS_HW_CONTEXTS
> ickle: "Never forget ILK contexts"

We have states only for >= snb. So thus the bailout.
And do_switch is not called with fake contexts.

>> +
>> +	so = render_state_alloc(ring->dev, BATCH_MAX_SIZE);
>> +	if (IS_ERR(so))
>> +		return PTR_ERR(so);
>> +
>> +	ret = render_state_get(so, gen);
>> +	if (ret)
>> +		goto out;
>> +
>> +	ret = ring->dispatch_execbuffer(ring,
>> +					i915_gem_obj_ggtt_offset(so->obj),
>> +					so->len,
>> +					I915_DISPATCH_SECURE);
>> +	if (ret)
>> +		goto out;
>> +
>> +	ret = intel_ring_flush_all_caches(ring);
>> +	if (ret)
>> +		goto out;
>> +
>> +	ret = i915_add_request(ring, &seqno);
>> +	if (ret)
>> +		goto out;
>> +
>> +	ret = i915_wait_seqno(ring, seqno);
>
> This wait is going to hurt. Not seeing why it's needed (plus I still
> recommend my idea on the top).

I couldn't come up with any explanations either. Removed in v2

Thanks,
-Mika

>> +out:
>> +	render_state_free(so);
>> +	return ret;
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen6.h b/drivers/gpu/drm/i915/intel_renderstate_gen6.h
>> new file mode 100644
>> index 0000000..4809f2f
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_renderstate_gen6.h
>> @@ -0,0 +1,11 @@
>> +#ifndef __INTEL_RENDERSTATE_GEN6
>> +#define __INTEL_RENDERSTATE_GEN6
>> +
>> +static const uint32_t gen6_null_state_relocs[] = {
>> +};
>> +
>> +static const uint32_t gen6_null_state_batch[] = {
>> +	MI_BATCH_BUFFER_END,
>> +};
>> +
>> +#endif /* __INTEL_RENDERSTATE_GEN6 */
>> diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen7.h b/drivers/gpu/drm/i915/intel_renderstate_gen7.h
>> new file mode 100644
>> index 0000000..9b1420b
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_renderstate_gen7.h
>> @@ -0,0 +1,11 @@
>> +#ifndef __INTEL_RENDERSTATE_GEN7
>> +#define __INTEL_RENDERSTATE_GEN7
>> +
>> +static const uint32_t gen7_null_state_relocs[] = {
>> +};
>> +
>> +static const uint32_t gen7_null_state_batch[] = {
>> +	MI_BATCH_BUFFER_END,
>> +};
>> +
>> +#endif /* __INTEL_RENDERSTATE_GEN7 */
>> diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen8.h b/drivers/gpu/drm/i915/intel_renderstate_gen8.h
>> new file mode 100644
>> index 0000000..d349dda
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_renderstate_gen8.h
>> @@ -0,0 +1,11 @@
>> +#ifndef __INTEL_RENDERSTATE_GEN8
>> +#define __INTEL_RENDERSTATE_GEN8
>> +
>> +static const uint32_t gen8_null_state_relocs[] = {
>> +};
>> +
>> +static const uint32_t gen8_null_state_batch[] = {
>> +	MI_BATCH_BUFFER_END,
>> +};
>> +
>> +#endif /* __INTEL_RENDERSTATE_GEN8 */
>> -- 
>> 1.7.9.5
>> 
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> -- 
> Ben Widawsky, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx