On Mon, Jun 16, 2014 at 02:40:45PM +0300, Mika Kuoppala wrote:
> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> writes:
>
> > 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@xxxxxxxxxxxxxxxxxx>
> > Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> > Cc: Damien Lespiau <damien.lespiau@xxxxxxxxx>
> > Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
>
> Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
Queued for -next, thanks for the patch.
-Daniel
>
> > ---
> > 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@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
