On Thu, Aug 21, 2014 at 11:40:54AM +0100, Thomas Daniel wrote:
> From: Oscar Mateo <oscar.ma...@intel.com>
>
> 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 <oscar.ma...@intel.com>
>
> 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 <thomas.dan...@intel.com>
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 <linux/io-mapping.h>
> #include <linux/i2c.h>
> #include <linux/i2c-algo-bit.h>
> @@ -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/drm/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 != RCS))
> return -ENOENT;
>
> - ret = render_state_init(&so, ring->dev);
> + ret = render_state_init(so, ring->dev);
> if (ret)
> return ret;
>
> - if (so.rodata == NULL)
> + if (so->rodata == NULL)
> return 0;
>
> - ret = render_state_setup(&so);
> + ret = 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 = i915_gem_render_state_prepare(ring, &so);
> if (ret)
> - goto out;
> + return ret;
> +
> + if (so.rodata == NULL)
> + return 0;
>
> ret = ring->dispatch_execbuffer(ring,
> so.ggtt_offset,
> @@ -164,6 +174,6 @@ int i915_gem_render_state_init(struct intel_engine_cs
> *ring)
> ret = __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/drm/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 © 2014 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * 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, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * 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 <linux/types.h>
> +
> +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/intel_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 = ring->default_context;
> - struct drm_i915_gem_object *dctx_obj;
>
> /* Intentionally left blank. */
> ring->buffer = NULL;
> @@ -1232,18 +1230,6 @@ static int logical_ring_init(struct drm_device *dev,
> struct intel_engine_cs *rin
> spin_lock_init(&ring->execlist_lock);
> ring->next_context_status_buffer = 0;
>
> - ret = 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 = dctx->engine[ring->id].state;
> - ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(dctx_obj);
> - ring->status_page.page_addr = kmap(sg_page(dctx_obj->pages->sgl));
> - if (ring->status_page.page_addr == NULL)
> - return -ENOMEM;
> - ring->status_page.obj = dctx_obj;
> -
> ret = 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 = 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 = ctx->engine[ring->id].ringbuf;
> + struct render_state so;
> + struct drm_i915_file_private *file_priv = ctx->file_priv;
> + struct drm_file *file = file_priv ? file_priv->file : NULL;
> + int ret;
> +
> + ret = i915_gem_render_state_prepare(ring, &so);
> + if (ret)
> + return ret;
> +
> + if (so.rodata == NULL)
> + return 0;
> +
> + ret = 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 = __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_object
> *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 = ringbuf;
> ctx->engine[ring->id].state = ctx_obj;
>
> + if (ctx == ring->default_context) {
> + /* The status page is offset 0 from the default context object
> + * in LRC mode. */
> + ring->status_page.gfx_addr = i915_gem_obj_ggtt_offset(ctx_obj);
> + ring->status_page.page_addr =
> + kmap(sg_page(ctx_obj->pages->sgl));
> + if (ring->status_page.page_addr == NULL)
> + return -ENOMEM;
> + ring->status_page.obj = ctx_obj;
> + }
> +
> + if (ring->id == RCS && !ctx->rcs_initialized) {
> + ret = intel_lr_context_render_state_init(ring, ctx);
> + if (ret) {
> + DRM_ERROR("Init render state failed: %d\n", ret);
> + ctx->engine[ring->id].ringbuf = NULL;
> + ctx->engine[ring->id].state = NULL;
> + intel_destroy_ringbuffer_obj(ringbuf);
> + goto error;
> + }
> + ctx->rcs_initialized = true;
> + }
> +
> return 0;
>
> error:
> diff --git a/drivers/gpu/drm/i915/intel_lrc.h
> b/drivers/gpu/drm/i915/intel_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_dwords);
>
> /* 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/i915/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 <linux/types.h>
> -
> -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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
