[PATCH] drm/i915: Simplify processing of the golden render context state

* [PATCH] drm/i915: Simplify processing of the golden render context state
@ 2014-06-10 10:23 Chris Wilson
  2014-06-16 11:40 ` Mika Kuoppala
  0 siblings, 1 reply; 3+ messages in thread
From: Chris Wilson @ 2014-06-10 10:23 UTC (permalink / raw)
  To: intel-gfx; +Cc: Mika Kuoppala

Rewrite i915_gem_render_state.c for the purposes of clarity and
compactness, in the process we can eliminate some dodgy math that did
not handle 64bit addresses correctly.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Damien Lespiau <damien.lespiau@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_render_state.c  | 161 +++++++++++---------------
 drivers/gpu/drm/i915/intel_renderstate.h      |   2 -
 drivers/gpu/drm/i915/intel_renderstate_gen6.c |   1 +
 drivers/gpu/drm/i915/intel_renderstate_gen7.c |   1 +
 drivers/gpu/drm/i915/intel_renderstate_gen8.c |   1 +
 5 files changed, 69 insertions(+), 97 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 3521f998a178..e60be3f552a6 100644
--- a/drivers/gpu/drm/i915/i915_gem_render_state.c
+++ b/drivers/gpu/drm/i915/i915_gem_render_state.c
@@ -28,64 +28,13 @@
 #include "i915_drv.h"
 #include "intel_renderstate.h"
 
-struct i915_render_state {
+struct render_state {
+	const struct intel_renderstate_rodata *rodata;
 	struct drm_i915_gem_object *obj;
-	unsigned long ggtt_offset;
-	void *batch;
-	u32 size;
-	u32 len;
+	u64 ggtt_offset;
+	int gen;
 };
 
-static struct i915_render_state *render_state_alloc(struct drm_device *dev)
-{
-	struct i915_render_state *so;
-	struct page *page;
-	int ret;
-
-	so = kzalloc(sizeof(*so), GFP_KERNEL);
-	if (!so)
-		return ERR_PTR(-ENOMEM);
-
-	so->obj = i915_gem_alloc_object(dev, 4096);
-	if (so->obj == NULL) {
-		ret = -ENOMEM;
-		goto free;
-	}
-	so->size = 4096;
-
-	ret = i915_gem_obj_ggtt_pin(so->obj, 4096, 0);
-	if (ret)
-		goto free_gem;
-
-	BUG_ON(so->obj->pages->nents != 1);
-	page = sg_page(so->obj->pages->sgl);
-
-	so->batch = kmap(page);
-	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)
-{
-	kunmap(so->batch);
-	i915_gem_object_ggtt_unpin(so->obj);
-	drm_gem_object_unreference(&so->obj->base);
-	kfree(so);
-}
-
 static const struct intel_renderstate_rodata *
 render_state_get_rodata(struct drm_device *dev, const int gen)
 {
@@ -101,98 +50,120 @@ render_state_get_rodata(struct drm_device *dev, const int gen)
 	return NULL;
 }
 
-static int render_state_setup(const int gen,
-			      const struct intel_renderstate_rodata *rodata,
-			      struct i915_render_state *so)
+static int render_state_init(struct render_state *so, struct drm_device *dev)
 {
-	const u64 goffset = i915_gem_obj_ggtt_offset(so->obj);
-	u32 reloc_index = 0;
-	u32 * const d = so->batch;
-	unsigned int i = 0;
 	int ret;
 
-	if (!rodata || rodata->batch_items * 4 > so->size)
+	so->gen = INTEL_INFO(dev)->gen;
+	so->rodata = render_state_get_rodata(dev, so->gen);
+	if (so->rodata == NULL)
+		return 0;
+
+	if (so->rodata->batch_items * 4 > 4096)
 		return -EINVAL;
 
+	so->obj = i915_gem_alloc_object(dev, 4096);
+	if (so->obj == NULL)
+		return -ENOMEM;
+
+	ret = i915_gem_obj_ggtt_pin(so->obj, 4096, 0);
+	if (ret)
+		goto free_gem;
+
+	so->ggtt_offset = i915_gem_obj_ggtt_offset(so->obj);
+	return 0;
+
+free_gem:
+	drm_gem_object_unreference(&so->obj->base);
+	return ret;
+}
+
+static int render_state_setup(struct render_state *so)
+{
+	const struct intel_renderstate_rodata *rodata = so->rodata;
+	unsigned int i = 0, reloc_index = 0;
+	struct page *page;
+	u32 *d;
+	int ret;
+
 	ret = i915_gem_object_set_to_cpu_domain(so->obj, true);
 	if (ret)
 		return ret;
 
+	page = sg_page(so->obj->pages->sgl);
+	d = kmap(page);
+
 	while (i < rodata->batch_items) {
 		u32 s = rodata->batch[i];
 
-		if (reloc_index < rodata->reloc_items &&
-		    i * 4  == rodata->reloc[reloc_index]) {
-
-			s += goffset & 0xffffffff;
-
-			/* We keep batch offsets max 32bit */
-			if (gen >= 8) {
+		if (i * 4  == rodata->reloc[reloc_index]) {
+			u64 r = s + so->ggtt_offset;
+			s = lower_32_bits(r);
+			if (so->gen >= 8) {
 				if (i + 1 >= rodata->batch_items ||
 				    rodata->batch[i + 1] != 0)
 					return -EINVAL;
 
-				d[i] = s;
-				i++;
-				s = (goffset & 0xffffffff00000000ull) >> 32;
+				d[i++] = s;
+				s = upper_32_bits(r);
 			}
 
 			reloc_index++;
 		}
 
-		d[i] = s;
-		i++;
+		d[i++] = s;
 	}
+	kunmap(page);
 
 	ret = i915_gem_object_set_to_gtt_domain(so->obj, false);
 	if (ret)
 		return ret;
 
-	if (rodata->reloc_items != reloc_index) {
-		DRM_ERROR("not all relocs resolved, %d out of %d\n",
-			  reloc_index, rodata->reloc_items);
+	if (rodata->reloc[reloc_index] != -1) {
+		DRM_ERROR("only %d relocs resolved\n", reloc_index);
 		return -EINVAL;
 	}
 
-	so->len = rodata->batch_items * 4;
-
 	return 0;
 }
 
+static void render_state_fini(struct render_state *so)
+{
+	i915_gem_object_ggtt_unpin(so->obj);
+	drm_gem_object_unreference(&so->obj->base);
+}
+
 int i915_gem_render_state_init(struct intel_engine_cs *ring)
 {
-	const int gen = INTEL_INFO(ring->dev)->gen;
-	struct i915_render_state *so;
-	const struct intel_renderstate_rodata *rodata;
+	struct render_state so;
 	int ret;
 
 	if (WARN_ON(ring->id != RCS))
 		return -ENOENT;
 
-	rodata = render_state_get_rodata(ring->dev, gen);
-	if (rodata == NULL)
-		return 0;
+	ret = render_state_init(&so, ring->dev);
+	if (ret)
+		return ret;
 
-	so = render_state_alloc(ring->dev);
-	if (IS_ERR(so))
-		return PTR_ERR(so);
+	if (so.rodata == NULL)
+		return 0;
 
-	ret = render_state_setup(gen, rodata, so);
+	ret = render_state_setup(&so);
 	if (ret)
 		goto out;
 
 	ret = ring->dispatch_execbuffer(ring,
-					i915_gem_obj_ggtt_offset(so->obj),
-					so->len,
+					so.ggtt_offset,
+					so.rodata->batch_items * 4,
 					I915_DISPATCH_SECURE);
 	if (ret)
 		goto out;
 
-	i915_vma_move_to_active(i915_gem_obj_to_ggtt(so->obj), ring);
+	i915_vma_move_to_active(i915_gem_obj_to_ggtt(so.obj), ring);
 
-	ret = __i915_add_request(ring, NULL, so->obj, NULL);
+	ret = __i915_add_request(ring, NULL, so.obj, NULL);
 	/* __i915_add_request moves object to inactive if it fails */
 out:
-	render_state_free(so);
+	render_state_fini(&so);
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/intel_renderstate.h b/drivers/gpu/drm/i915/intel_renderstate.h
index a5e783a9928a..fd4f66231d30 100644
--- a/drivers/gpu/drm/i915/intel_renderstate.h
+++ b/drivers/gpu/drm/i915/intel_renderstate.h
@@ -28,7 +28,6 @@
 
 struct intel_renderstate_rodata {
 	const u32 *reloc;
-	const u32 reloc_items;
 	const u32 *batch;
 	const u32 batch_items;
 };
@@ -40,7 +39,6 @@ extern const struct intel_renderstate_rodata gen8_null_state;
 #define RO_RENDERSTATE(_g)						\
 	const struct intel_renderstate_rodata gen ## _g ## _null_state = { \
 		.reloc = gen ## _g ## _null_state_relocs,		\
-		.reloc_items = sizeof(gen ## _g ## _null_state_relocs)/4, \
 		.batch = gen ## _g ## _null_state_batch,		\
 		.batch_items = sizeof(gen ## _g ## _null_state_batch)/4, \
 	}
diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen6.c b/drivers/gpu/drm/i915/intel_renderstate_gen6.c
index 740538ad0977..56c1429d8a60 100644
--- a/drivers/gpu/drm/i915/intel_renderstate_gen6.c
+++ b/drivers/gpu/drm/i915/intel_renderstate_gen6.c
@@ -6,6 +6,7 @@ static const u32 gen6_null_state_relocs[] = {
 	0x0000002c,
 	0x000001e0,
 	0x000001e4,
+	-1,
 };
 
 static const u32 gen6_null_state_batch[] = {
diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen7.c b/drivers/gpu/drm/i915/intel_renderstate_gen7.c
index 6fa7ff2a1298..419e35a7b0ff 100644
--- a/drivers/gpu/drm/i915/intel_renderstate_gen7.c
+++ b/drivers/gpu/drm/i915/intel_renderstate_gen7.c
@@ -5,6 +5,7 @@ static const u32 gen7_null_state_relocs[] = {
 	0x00000010,
 	0x00000018,
 	0x000001ec,
+	-1,
 };
 
 static const u32 gen7_null_state_batch[] = {
diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen8.c b/drivers/gpu/drm/i915/intel_renderstate_gen8.c
index 5c875615d42a..75ef1b5de45c 100644
--- a/drivers/gpu/drm/i915/intel_renderstate_gen8.c
+++ b/drivers/gpu/drm/i915/intel_renderstate_gen8.c
@@ -5,6 +5,7 @@ static const u32 gen8_null_state_relocs[] = {
 	0x00000050,
 	0x00000060,
 	0x000003ec,
+	-1,
 };
 
 static const u32 gen8_null_state_batch[] = {
-- 
2.0.0

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

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