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>

diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/drm/i915/i915_gem_render_state.c
index 3521f998a1788488b8c396860f6433ffacb81ae2..e60be3f552a6b1aed14770bef853a1eab2ddf5aa 100644 (file)
--- 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 a5e783a9928a40fff80297cf71725151fbdae30f..fd4f66231d30edec7d96236fac145e2b00b7b0c5 100644 (file)
--- 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 740538ad09771638b6a184331a0c357cdaebc8ec..56c1429d8a60d2dadaaf3b95c2e7d74b13fc1cb4 100644 (file)
--- 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 6fa7ff2a12983d847c17380f55c22bb9fb87d477..419e35a7b0ff391289c1dff980a90ea7468b7010 100644 (file)
--- 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 5c875615d42acc58deba2bf75d02a92bff8292ef..75ef1b5de45c12c33337b00db36b30d199d9eb20 100644 (file)
--- 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[] = {