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

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>
Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

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[] = {