From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915/bdw: Render state init for Execlists Date: Thu, 28 Aug 2014 11:40:29 +0200 Message-ID: <20140828094029.GA15520@phenom.ffwll.local> References: <1406217891-8912-28-git-send-email-thomas.daniel@intel.com> <1408617654-2608-1-git-send-email-thomas.daniel@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-f181.google.com (mail-wi0-f181.google.com [209.85.212.181]) by gabe.freedesktop.org (Postfix) with ESMTP id 37C276E123 for ; Thu, 28 Aug 2014 02:40:10 -0700 (PDT) Received: by mail-wi0-f181.google.com with SMTP id e4so546416wiv.2 for ; Thu, 28 Aug 2014 02:40:09 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1408617654-2608-1-git-send-email-thomas.daniel@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Thomas Daniel Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Thu, Aug 21, 2014 at 11:40:54AM +0100, Thomas Daniel wrote: > From: Oscar Mateo > = > The batchbuffer that sets the render context state is submitted > in a different way, and from different places. > = > We needed to make both the render state preparation and free functions > outside accesible, and namespace accordingly. This mess is so that all > LR, LRC and Execlists functionality can go together in intel_lrc.c: we > can fix all of this later on, once the interfaces are clear. > = > v2: Create a separate ctx->rcs_initialized for the Execlists case, as > suggested by Chris Wilson. > = > Signed-off-by: Oscar Mateo > = > v3: Setup ring status page in lr_context_deferred_create when the > default context is being created. This means that the render state > init for the default context is no longer a special case. Execute > deferred creation of the default context at the end of > logical_ring_init to allow the render state commands to be submitted. > Fix style errors reported by checkpatch. Rebased. > = > Signed-off-by: Thomas Daniel Queued for -next, thanks for the patch. -Daniel > --- > drivers/gpu/drm/i915/i915_drv.h | 4 +- > drivers/gpu/drm/i915/i915_gem_render_state.c | 40 ++++++++------ > drivers/gpu/drm/i915/i915_gem_render_state.h | 47 +++++++++++++++++ > drivers/gpu/drm/i915/intel_lrc.c | 73 ++++++++++++++++++++= ------ > drivers/gpu/drm/i915/intel_lrc.h | 2 + > drivers/gpu/drm/i915/intel_renderstate.h | 8 +-- > 6 files changed, 135 insertions(+), 39 deletions(-) > create mode 100644 drivers/gpu/drm/i915/i915_gem_render_state.h > = > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_= drv.h > index e449f81..f416e341 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -37,6 +37,7 @@ > #include "intel_ringbuffer.h" > #include "intel_lrc.h" > #include "i915_gem_gtt.h" > +#include "i915_gem_render_state.h" > #include > #include > #include > @@ -635,6 +636,7 @@ struct intel_context { > } legacy_hw_ctx; > = > /* Execlists */ > + bool rcs_initialized; > struct { > struct drm_i915_gem_object *state; > struct intel_ringbuffer *ringbuf; > @@ -2596,8 +2598,6 @@ int i915_gem_context_create_ioctl(struct drm_device= *dev, void *data, > int i915_gem_context_destroy_ioctl(struct drm_device *dev, void *data, > struct drm_file *file); > = > -/* i915_gem_render_state.c */ > -int i915_gem_render_state_init(struct intel_engine_cs *ring); > /* i915_gem_evict.c */ > int __must_check i915_gem_evict_something(struct drm_device *dev, > struct i915_address_space *vm, > diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.c b/drivers/gpu/d= rm/i915/i915_gem_render_state.c > index e60be3f..a9a62d7 100644 > --- a/drivers/gpu/drm/i915/i915_gem_render_state.c > +++ b/drivers/gpu/drm/i915/i915_gem_render_state.c > @@ -28,13 +28,6 @@ > #include "i915_drv.h" > #include "intel_renderstate.h" > = > -struct render_state { > - const struct intel_renderstate_rodata *rodata; > - struct drm_i915_gem_object *obj; > - u64 ggtt_offset; > - int gen; > -}; > - > static const struct intel_renderstate_rodata * > render_state_get_rodata(struct drm_device *dev, const int gen) > { > @@ -127,30 +120,47 @@ static int render_state_setup(struct render_state *= so) > return 0; > } > = > -static void render_state_fini(struct render_state *so) > +void i915_gem_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) > +int i915_gem_render_state_prepare(struct intel_engine_cs *ring, > + struct render_state *so) > { > - struct render_state so; > int ret; > = > if (WARN_ON(ring->id !=3D RCS)) > return -ENOENT; > = > - ret =3D render_state_init(&so, ring->dev); > + ret =3D render_state_init(so, ring->dev); > if (ret) > return ret; > = > - if (so.rodata =3D=3D NULL) > + if (so->rodata =3D=3D NULL) > return 0; > = > - ret =3D render_state_setup(&so); > + ret =3D render_state_setup(so); > + if (ret) { > + i915_gem_render_state_fini(so); > + return ret; > + } > + > + return 0; > +} > + > +int i915_gem_render_state_init(struct intel_engine_cs *ring) > +{ > + struct render_state so; > + int ret; > + > + ret =3D i915_gem_render_state_prepare(ring, &so); > if (ret) > - goto out; > + return ret; > + > + if (so.rodata =3D=3D NULL) > + return 0; > = > ret =3D ring->dispatch_execbuffer(ring, > so.ggtt_offset, > @@ -164,6 +174,6 @@ int i915_gem_render_state_init(struct intel_engine_cs= *ring) > ret =3D __i915_add_request(ring, NULL, so.obj, NULL); > /* __i915_add_request moves object to inactive if it fails */ > out: > - render_state_fini(&so); > + i915_gem_render_state_fini(&so); > return ret; > } > diff --git a/drivers/gpu/drm/i915/i915_gem_render_state.h b/drivers/gpu/d= rm/i915/i915_gem_render_state.h > new file mode 100644 > index 0000000..c44961e > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_gem_render_state.h > @@ -0,0 +1,47 @@ > +/* > + * Copyright =A9 2014 Intel Corporation > + * > + * Permission is hereby granted, free of charge, to any person obtaining= a > + * copy of this software and associated documentation files (the "Softwa= re"), > + * to deal in the Software without restriction, including without limita= tion > + * the rights to use, copy, modify, merge, publish, distribute, sublicen= se, > + * and/or sell copies of the Software, and to permit persons to whom the > + * Software is furnished to do so, subject to the following conditions: > + * > + * The above copyright notice and this permission notice (including the = next > + * paragraph) shall be included in all copies or substantial portions of= the > + * Software. > + * > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRE= SS OR > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILI= TY, > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SH= ALL > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR = OTHER > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISI= NG > + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER > + * DEALINGS IN THE SOFTWARE. > + */ > + > +#ifndef _I915_GEM_RENDER_STATE_H_ > +#define _I915_GEM_RENDER_STATE_H_ > + > +#include > + > +struct intel_renderstate_rodata { > + const u32 *reloc; > + const u32 *batch; > + const u32 batch_items; > +}; > + > +struct render_state { > + const struct intel_renderstate_rodata *rodata; > + struct drm_i915_gem_object *obj; > + u64 ggtt_offset; > + int gen; > +}; > + > +int i915_gem_render_state_init(struct intel_engine_cs *ring); > +void i915_gem_render_state_fini(struct render_state *so); > +int i915_gem_render_state_prepare(struct intel_engine_cs *ring, > + struct render_state *so); > + > +#endif /* _I915_GEM_RENDER_STATE_H_ */ > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/inte= l_lrc.c > index c096b9b..8e51fd0 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -1217,8 +1217,6 @@ void intel_logical_ring_cleanup(struct intel_engine= _cs *ring) > static int logical_ring_init(struct drm_device *dev, struct intel_engine= _cs *ring) > { > int ret; > - struct intel_context *dctx =3D ring->default_context; > - struct drm_i915_gem_object *dctx_obj; > = > /* Intentionally left blank. */ > ring->buffer =3D NULL; > @@ -1232,18 +1230,6 @@ static int logical_ring_init(struct drm_device *de= v, struct intel_engine_cs *rin > spin_lock_init(&ring->execlist_lock); > ring->next_context_status_buffer =3D 0; > = > - ret =3D intel_lr_context_deferred_create(dctx, ring); > - if (ret) > - return ret; > - > - /* The status page is offset 0 from the context object in LRCs. */ > - dctx_obj =3D dctx->engine[ring->id].state; > - ring->status_page.gfx_addr =3D i915_gem_obj_ggtt_offset(dctx_obj); > - ring->status_page.page_addr =3D kmap(sg_page(dctx_obj->pages->sgl)); > - if (ring->status_page.page_addr =3D=3D NULL) > - return -ENOMEM; > - ring->status_page.obj =3D dctx_obj; > - > ret =3D i915_cmd_parser_init_ring(ring); > if (ret) > return ret; > @@ -1254,7 +1240,9 @@ static int logical_ring_init(struct drm_device *dev= , struct intel_engine_cs *rin > return ret; > } > = > - return 0; > + ret =3D intel_lr_context_deferred_create(ring->default_context, ring); > + > + return ret; > } > = > static int logical_render_ring_init(struct drm_device *dev) > @@ -1448,6 +1436,38 @@ cleanup_render_ring: > return ret; > } > = > +int intel_lr_context_render_state_init(struct intel_engine_cs *ring, > + struct intel_context *ctx) > +{ > + struct intel_ringbuffer *ringbuf =3D ctx->engine[ring->id].ringbuf; > + struct render_state so; > + struct drm_i915_file_private *file_priv =3D ctx->file_priv; > + struct drm_file *file =3D file_priv ? file_priv->file : NULL; > + int ret; > + > + ret =3D i915_gem_render_state_prepare(ring, &so); > + if (ret) > + return ret; > + > + if (so.rodata =3D=3D NULL) > + return 0; > + > + ret =3D ring->emit_bb_start(ringbuf, > + so.ggtt_offset, > + I915_DISPATCH_SECURE); > + if (ret) > + goto out; > + > + i915_vma_move_to_active(i915_gem_obj_to_ggtt(so.obj), ring); > + > + ret =3D __i915_add_request(ring, file, so.obj, NULL); > + /* intel_logical_ring_add_request moves object to inactive if it > + * fails */ > +out: > + i915_gem_render_state_fini(&so); > + return ret; > +} > + > static int > populate_lr_context(struct intel_context *ctx, struct drm_i915_gem_objec= t *ctx_obj, > struct intel_engine_cs *ring, struct intel_ringbuffer *ringbuf) > @@ -1687,6 +1707,29 @@ int intel_lr_context_deferred_create(struct intel_= context *ctx, > ctx->engine[ring->id].ringbuf =3D ringbuf; > ctx->engine[ring->id].state =3D ctx_obj; > = > + if (ctx =3D=3D ring->default_context) { > + /* The status page is offset 0 from the default context object > + * in LRC mode. */ > + ring->status_page.gfx_addr =3D i915_gem_obj_ggtt_offset(ctx_obj); > + ring->status_page.page_addr =3D > + kmap(sg_page(ctx_obj->pages->sgl)); > + if (ring->status_page.page_addr =3D=3D NULL) > + return -ENOMEM; > + ring->status_page.obj =3D ctx_obj; > + } > + > + if (ring->id =3D=3D RCS && !ctx->rcs_initialized) { > + ret =3D intel_lr_context_render_state_init(ring, ctx); > + if (ret) { > + DRM_ERROR("Init render state failed: %d\n", ret); > + ctx->engine[ring->id].ringbuf =3D NULL; > + ctx->engine[ring->id].state =3D NULL; > + intel_destroy_ringbuffer_obj(ringbuf); > + goto error; > + } > + ctx->rcs_initialized =3D true; > + } > + > return 0; > = > error: > diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/inte= l_lrc.h > index 991d449..33c3b4b 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.h > +++ b/drivers/gpu/drm/i915/intel_lrc.h > @@ -62,6 +62,8 @@ static inline void intel_logical_ring_emit(struct intel= _ringbuffer *ringbuf, > int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, int num_d= words); > = > /* Logical Ring Contexts */ > +int intel_lr_context_render_state_init(struct intel_engine_cs *ring, > + struct intel_context *ctx); > void intel_lr_context_free(struct intel_context *ctx); > int intel_lr_context_deferred_create(struct intel_context *ctx, > struct intel_engine_cs *ring); > diff --git a/drivers/gpu/drm/i915/intel_renderstate.h b/drivers/gpu/drm/i= 915/intel_renderstate.h > index fd4f662..6c792d3 100644 > --- a/drivers/gpu/drm/i915/intel_renderstate.h > +++ b/drivers/gpu/drm/i915/intel_renderstate.h > @@ -24,13 +24,7 @@ > #ifndef _INTEL_RENDERSTATE_H > #define _INTEL_RENDERSTATE_H > = > -#include > - > -struct intel_renderstate_rodata { > - const u32 *reloc; > - const u32 *batch; > - const u32 batch_items; > -}; > +#include "i915_drv.h" > = > extern const struct intel_renderstate_rodata gen6_null_state; > extern const struct intel_renderstate_rodata gen7_null_state; > -- = > 1.7.9.5 > = > _______________________________________________ > 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