From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: Simplify processing of the golden render context state Date: Mon, 16 Jun 2014 19:53:28 +0200 Message-ID: <20140616175328.GZ5821@phenom.ffwll.local> References: <1402395813-6568-1-git-send-email-chris@chris-wilson.co.uk> <87fvj5ozeq.fsf@gaia.fi.intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-wi0-f174.google.com (mail-wi0-f174.google.com [209.85.212.174]) by gabe.freedesktop.org (Postfix) with ESMTP id 5443A6E17D for ; Mon, 16 Jun 2014 10:53:35 -0700 (PDT) Received: by mail-wi0-f174.google.com with SMTP id bs8so4530444wib.1 for ; Mon, 16 Jun 2014 10:53:34 -0700 (PDT) Content-Disposition: inline In-Reply-To: <87fvj5ozeq.fsf@gaia.fi.intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Mika Kuoppala Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Jun 16, 2014 at 02:40:45PM +0300, Mika Kuoppala wrote: > Chris Wilson 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 > > Cc: Ville Syrj=E4l=E4 > > Cc: Damien Lespiau > > Cc: Mika Kuoppala > = > Reviewed-by: Mika Kuoppala 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 =3D kzalloc(sizeof(*so), GFP_KERNEL); > > - if (!so) > > - return ERR_PTR(-ENOMEM); > > - > > - so->obj =3D i915_gem_alloc_object(dev, 4096); > > - if (so->obj =3D=3D NULL) { > > - ret =3D -ENOMEM; > > - goto free; > > - } > > - so->size =3D 4096; > > - > > - ret =3D i915_gem_obj_ggtt_pin(so->obj, 4096, 0); > > - if (ret) > > - goto free_gem; > > - > > - BUG_ON(so->obj->pages->nents !=3D 1); > > - page =3D sg_page(so->obj->pages->sgl); > > - > > - so->batch =3D kmap(page); > > - if (!so->batch) { > > - ret =3D -ENOMEM; > > - goto unpin; > > - } > > - > > - so->ggtt_offset =3D 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, c= onst 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_devic= e *dev) > > { > > - const u64 goffset =3D i915_gem_obj_ggtt_offset(so->obj); > > - u32 reloc_index =3D 0; > > - u32 * const d =3D so->batch; > > - unsigned int i =3D 0; > > int ret; > > = > > - if (!rodata || rodata->batch_items * 4 > so->size) > > + so->gen =3D INTEL_INFO(dev)->gen; > > + so->rodata =3D render_state_get_rodata(dev, so->gen); > > + if (so->rodata =3D=3D NULL) > > + return 0; > > + > > + if (so->rodata->batch_items * 4 > 4096) > > return -EINVAL; > > = > > + so->obj =3D i915_gem_alloc_object(dev, 4096); > > + if (so->obj =3D=3D NULL) > > + return -ENOMEM; > > + > > + ret =3D i915_gem_obj_ggtt_pin(so->obj, 4096, 0); > > + if (ret) > > + goto free_gem; > > + > > + so->ggtt_offset =3D 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 =3D so->rodata; > > + unsigned int i =3D 0, reloc_index =3D 0; > > + struct page *page; > > + u32 *d; > > + int ret; > > + > > ret =3D i915_gem_object_set_to_cpu_domain(so->obj, true); > > if (ret) > > return ret; > > = > > + page =3D sg_page(so->obj->pages->sgl); > > + d =3D kmap(page); > > + > > while (i < rodata->batch_items) { > > u32 s =3D rodata->batch[i]; > > = > > - if (reloc_index < rodata->reloc_items && > > - i * 4 =3D=3D rodata->reloc[reloc_index]) { > > - > > - s +=3D goffset & 0xffffffff; > > - > > - /* We keep batch offsets max 32bit */ > > - if (gen >=3D 8) { > > + if (i * 4 =3D=3D rodata->reloc[reloc_index]) { > > + u64 r =3D s + so->ggtt_offset; > > + s =3D lower_32_bits(r); > > + if (so->gen >=3D 8) { > > if (i + 1 >=3D rodata->batch_items || > > rodata->batch[i + 1] !=3D 0) > > return -EINVAL; > > = > > - d[i] =3D s; > > - i++; > > - s =3D (goffset & 0xffffffff00000000ull) >> 32; > > + d[i++] =3D s; > > + s =3D upper_32_bits(r); > > } > > = > > reloc_index++; > > } > > = > > - d[i] =3D s; > > - i++; > > + d[i++] =3D s; > > } > > + kunmap(page); > > = > > ret =3D i915_gem_object_set_to_gtt_domain(so->obj, false); > > if (ret) > > return ret; > > = > > - if (rodata->reloc_items !=3D reloc_index) { > > - DRM_ERROR("not all relocs resolved, %d out of %d\n", > > - reloc_index, rodata->reloc_items); > > + if (rodata->reloc[reloc_index] !=3D -1) { > > + DRM_ERROR("only %d relocs resolved\n", reloc_index); > > return -EINVAL; > > } > > = > > - so->len =3D 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 =3D 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 !=3D RCS)) > > return -ENOENT; > > = > > - rodata =3D render_state_get_rodata(ring->dev, gen); > > - if (rodata =3D=3D NULL) > > - return 0; > > + ret =3D render_state_init(&so, ring->dev); > > + if (ret) > > + return ret; > > = > > - so =3D render_state_alloc(ring->dev); > > - if (IS_ERR(so)) > > - return PTR_ERR(so); > > + if (so.rodata =3D=3D NULL) > > + return 0; > > = > > - ret =3D render_state_setup(gen, rodata, so); > > + ret =3D render_state_setup(&so); > > if (ret) > > goto out; > > = > > ret =3D 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 =3D __i915_add_request(ring, NULL, so->obj, NULL); > > + ret =3D __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_nul= l_state; > > #define RO_RENDERSTATE(_g) \ > > const struct intel_renderstate_rodata gen ## _g ## _null_state =3D { \ > > .reloc =3D gen ## _g ## _null_state_relocs, \ > > - .reloc_items =3D sizeof(gen ## _g ## _null_state_relocs)/4, \ > > .batch =3D gen ## _g ## _null_state_batch, \ > > .batch_items =3D sizeof(gen ## _g ## _null_state_batch)/4, \ > > } > > diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen6.c b/drivers/gp= u/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[] =3D { > > 0x0000002c, > > 0x000001e0, > > 0x000001e4, > > + -1, > > }; > > = > > static const u32 gen6_null_state_batch[] =3D { > > diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen7.c b/drivers/gp= u/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[] =3D { > > 0x00000010, > > 0x00000018, > > 0x000001ec, > > + -1, > > }; > > = > > static const u32 gen7_null_state_batch[] =3D { > > diff --git a/drivers/gpu/drm/i915/intel_renderstate_gen8.c b/drivers/gp= u/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[] =3D { > > 0x00000050, > > 0x00000060, > > 0x000003ec, > > + -1, > > }; > > = > > static const u32 gen8_null_state_batch[] =3D { > > -- = > > 2.0.0 > _______________________________________________ > 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