Re: [PATCH 19/49] drm/i915/bdw: Populate LR contexts (somewhat)

Jeff McGee
2014-04-15T20:35:03Z

On Tue, Apr 15, 2014 at 11:10:34AM -0500, Jeff McGee wrote:
> On Tue, Apr 15, 2014 at 11:00:33AM -0500, Jeff McGee wrote:
> > On Thu, Mar 27, 2014 at 05:59:48PM +0000, oscar.mateo@intel.com wrote:
> > > From: Ben Widawsky <benjamin.widawsky@intel.com>
> > >
> > > For the most part, logical rinf context objects are similar to hardware
> > > contexts in that the backing object is meant to be opaque. There are
> > > some exceptions where we need to poke certain offsets of the object for
> > > initialization, updating the tail pointer or updating the PDPs.
> > >
> > > For our basic execlist implementation we'll only need our PPGTT PDs,
> > > and ringbuffer addresses in order to set up the context. With previous
> > > patches, we have both, so start prepping the context to be load.
> > >
> > > Before running a context for the first time you must populate some
> > > fields in the context object. These fields begin 1 PAGE + LRCA, ie. the
> > > first page (in 0 based counting) of the context image. These same
> > > fields will be read and written to as contexts are saved and restored
> > > once the system is up and running.
> > >
> > > Many of these fields are completely reused from previous global
> > > registers: ringbuffer head/tail/control, context control matches some
> > > previous MI_SET_CONTEXT flags, and page directories. There are other
> > > fields which we don't touch which we may want in the future.
> > >
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > >
> > > v2: CTX_LRI_HEADER_0 is MI_LOAD_REGISTER_IMM(14) for render and (11)
> > > for other engines.
> > >
> > > Signed-off-by: Rafael Barbalho <rafael.barbalho@intel.com>
> > >
> > > v3: Several rebases and general changes to the code.
> > >
> > > Signed-off-by: Oscar Mateo <oscar.mateo@intel.com>
> > > ---
> > > drivers/gpu/drm/i915/i915_lrc.c | 145 ++++++++++++++++++++++++++++++++++++++--
> > > 1 file changed, 138 insertions(+), 7 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/i915_lrc.c b/drivers/gpu/drm/i915/i915_lrc.c
> > > index 40dfa95..f0176ff 100644
> > > --- a/drivers/gpu/drm/i915/i915_lrc.c
> > > +++ b/drivers/gpu/drm/i915/i915_lrc.c
> > > @@ -43,6 +43,38 @@
> > >
> > > #define GEN8_LR_CONTEXT_SIZE (21 * PAGE_SIZE)
> > >
> > > +#define RING_ELSP(ring) ((ring)->mmio_base+0x230)
> > > +#define RING_CONTEXT_CONTROL(ring) ((ring)->mmio_base+0x244)
> > > +
> > > +#define CTX_LRI_HEADER_0 0x01
> > > +#define CTX_CONTEXT_CONTROL 0x02
> > > +#define CTX_RING_HEAD 0x04
> > > +#define CTX_RING_TAIL 0x06
> > > +#define CTX_RING_BUFFER_START 0x08
> > > +#define CTX_RING_BUFFER_CONTROL 0x0a
> > > +#define CTX_BB_HEAD_U 0x0c
> > > +#define CTX_BB_HEAD_L 0x0e
> > > +#define CTX_BB_STATE 0x10
> > > +#define CTX_SECOND_BB_HEAD_U 0x12
> > > +#define CTX_SECOND_BB_HEAD_L 0x14
> > > +#define CTX_SECOND_BB_STATE 0x16
> > > +#define CTX_BB_PER_CTX_PTR 0x18
> > > +#define CTX_RCS_INDIRECT_CTX 0x1a
> > > +#define CTX_RCS_INDIRECT_CTX_OFFSET 0x1c
> > > +#define CTX_LRI_HEADER_1 0x21
> > > +#define CTX_CTX_TIMESTAMP 0x22
> > > +#define CTX_PDP3_UDW 0x24
> > > +#define CTX_PDP3_LDW 0x26
> > > +#define CTX_PDP2_UDW 0x28
> > > +#define CTX_PDP2_LDW 0x2a
> > > +#define CTX_PDP1_UDW 0x2c
> > > +#define CTX_PDP1_LDW 0x2e
> > > +#define CTX_PDP0_UDW 0x30
> > > +#define CTX_PDP0_LDW 0x32
> > > +#define CTX_LRI_HEADER_2 0x41
> > > +#define CTX_R_PWR_CLK_STATE 0x42
> > > +#define CTX_GPGPU_CSR_BASE_ADDRESS 0x44
> > > +
> > > struct i915_hw_context *
> > > gen8_gem_create_context(struct drm_device *dev,
> > > struct intel_engine *ring,
> > > @@ -51,6 +83,9 @@ gen8_gem_create_context(struct drm_device *dev,
> > > {
> > > struct i915_hw_context *ctx = NULL;
> > > struct drm_i915_gem_object *ring_obj = NULL;
> > > + struct i915_hw_ppgtt *ppgtt = NULL;
> > > + struct page *page;
> > > + uint32_t *reg_state;
> > > int ret;
> > >
> > > ctx = i915_gem_create_context(dev, file_priv, create_vm);
> > > @@ -79,18 +114,114 @@ gen8_gem_create_context(struct drm_device *dev,
> > >
> > > /* Failure at this point is almost impossible */
> > > ret = i915_gem_object_set_to_gtt_domain(ring_obj, true);
> > > - if (ret) {
> > > - i915_gem_object_ggtt_unpin(ring_obj);
> > > - drm_gem_object_unreference(&ring_obj->base);
> > > - i915_gem_object_ggtt_unpin(ctx->obj);
> > > - i915_gem_context_unreference(ctx);
> > > - return ERR_PTR(ret);
> > > - }
> > > + if (ret)
> > > + goto destroy_ring_obj;
> > >
> > > ctx->ringbuf = &ring->default_ringbuf;
> > > ctx->ringbuf->obj = ring_obj;
> > >
> > > + ppgtt = ctx_to_ppgtt(ctx);
> > > +
> > > + ret = i915_gem_object_set_to_cpu_domain(ctx->obj, true);
> > > + if (ret)
> > > + goto destroy_ring_obj;
> > > +
> > > + ret = i915_gem_object_get_pages(ctx->obj);
> > > + if (ret)
> > > + goto destroy_ring_obj;
> > > +
> > > + i915_gem_object_pin_pages(ctx->obj);
> > > +
> > > + /* The second page of the context object contains some fields which must
> > > + * be set up prior to the first execution.
> > > + */
> > > + page = i915_gem_object_get_page(ctx->obj, 1);
> > > + reg_state = kmap_atomic(page);
> > > +
> > > + if (ring->id == RCS)
> > > + reg_state[CTX_LRI_HEADER_0] = MI_LOAD_REGISTER_IMM(14);
> > > + else
> > > + reg_state[CTX_LRI_HEADER_0] = MI_LOAD_REGISTER_IMM(11);
> > > + reg_state[CTX_CONTEXT_CONTROL] = RING_CONTEXT_CONTROL(ring);
> > > + reg_state[CTX_CONTEXT_CONTROL+1] = (1<<3) | MI_RESTORE_INHIBIT;
> > > + reg_state[CTX_CONTEXT_CONTROL+1] |= reg_state[CTX_CONTEXT_CONTROL+1] << 16;
> > > + reg_state[CTX_RING_HEAD] = RING_HEAD(ring->mmio_base);
> > > + reg_state[CTX_RING_HEAD+1] = 0;
> > > + reg_state[CTX_RING_TAIL] = RING_TAIL(ring->mmio_base);
> > > + reg_state[CTX_RING_TAIL+1] = 0;
> > > + reg_state[CTX_RING_BUFFER_START] = RING_START(ring->mmio_base);
> > > + reg_state[CTX_RING_BUFFER_START+1] = i915_gem_obj_ggtt_offset(ring_obj);
> > > + reg_state[CTX_RING_BUFFER_CONTROL] = RING_CTL(ring->mmio_base);
> > > + reg_state[CTX_RING_BUFFER_CONTROL+1] = (31 * PAGE_SIZE) | RING_VALID;
> > > + reg_state[CTX_BB_HEAD_U] = ring->mmio_base + 0x168;
> > > + reg_state[CTX_BB_HEAD_U+1] = 0;
> > > + reg_state[CTX_BB_HEAD_L] = ring->mmio_base + 0x140;
> > > + reg_state[CTX_BB_HEAD_L+1] = 0;
> > > + reg_state[CTX_BB_STATE] = ring->mmio_base + 0x110;
> > > + reg_state[CTX_BB_STATE+1] = (1<<5);
> > > + reg_state[CTX_SECOND_BB_HEAD_U] = ring->mmio_base + 0x11c;
> > > + reg_state[CTX_SECOND_BB_HEAD_U+1] = 0;
> > > + reg_state[CTX_SECOND_BB_HEAD_L] = ring->mmio_base + 0x114;
> > > + reg_state[CTX_SECOND_BB_HEAD_L+1] = 0;
> > > + reg_state[CTX_SECOND_BB_STATE] = ring->mmio_base + 0x118;
> > > + reg_state[CTX_SECOND_BB_STATE+1] = 0;
> > > + if (ring->id == RCS) {
> > > + reg_state[CTX_BB_PER_CTX_PTR] = ring->mmio_base + 0x1c0;
> > > + reg_state[CTX_BB_PER_CTX_PTR+1] = 0;
> > > + reg_state[CTX_RCS_INDIRECT_CTX] = ring->mmio_base + 0x1c4;
> > > + reg_state[CTX_RCS_INDIRECT_CTX+1] = 0;
> > > + reg_state[CTX_RCS_INDIRECT_CTX_OFFSET] = ring->mmio_base + 0x1c8;
> > > + reg_state[CTX_RCS_INDIRECT_CTX_OFFSET+1] = 0;
> > > + }
> > > +
> > > + reg_state[CTX_LRI_HEADER_1] = MI_LOAD_REGISTER_IMM(9);
> > > + reg_state[CTX_CTX_TIMESTAMP] = ring->mmio_base + 0x3a8;
> > > + reg_state[CTX_CTX_TIMESTAMP+1] = 0;
> > > + reg_state[CTX_PDP3_UDW] = GEN8_RING_PDP_UDW(ring, 3);
> > > + reg_state[CTX_PDP3_LDW] = GEN8_RING_PDP_LDW(ring, 3);
> > > + reg_state[CTX_PDP2_UDW] = GEN8_RING_PDP_UDW(ring, 2);
> > > + reg_state[CTX_PDP2_LDW] = GEN8_RING_PDP_LDW(ring, 2);
> > > + reg_state[CTX_PDP1_UDW] = GEN8_RING_PDP_UDW(ring, 1);
> > > + reg_state[CTX_PDP1_LDW] = GEN8_RING_PDP_LDW(ring, 1);
> > > + reg_state[CTX_PDP0_UDW] = GEN8_RING_PDP_UDW(ring, 0);
> > > + reg_state[CTX_PDP0_LDW] = GEN8_RING_PDP_LDW(ring, 0);
> > > + reg_state[CTX_PDP3_UDW+1] = ppgtt->pd_dma_addr[3] >> 32;
> > > + reg_state[CTX_PDP3_LDW+1] = ppgtt->pd_dma_addr[3];
> > > + reg_state[CTX_PDP2_UDW+1] = ppgtt->pd_dma_addr[2] >> 32;
> > > + reg_state[CTX_PDP2_LDW+1] = ppgtt->pd_dma_addr[2];
> > > + reg_state[CTX_PDP1_UDW+1] = ppgtt->pd_dma_addr[1] >> 32;
> > > + reg_state[CTX_PDP1_LDW+1] = ppgtt->pd_dma_addr[1];
> > > + reg_state[CTX_PDP0_UDW+1] = ppgtt->pd_dma_addr[0] >> 32;
> > > + reg_state[CTX_PDP0_LDW+1] = ppgtt->pd_dma_addr[0];
> > > + if (ring->id == RCS) {
> > > + reg_state[CTX_LRI_HEADER_2] = MI_LOAD_REGISTER_IMM(1);
> > > + reg_state[CTX_R_PWR_CLK_STATE] = 0x20c8;
> >
> > You're writing the MMIO address for the R_PWR_CLK_STATE register to this
> > field. Shouldn't this receive the value we want programmed to the register?
> >
>
> Oh, nevermind. I understand now.
> -Jeff
>
To clarify my comments...I was at first confused by the need to specify the
R_PWR_CLK_STATE register address in the logical context, thinking that only
the desired value needed to be specified. But I see now that the programming
model is to specify the MI_LOAD_REGISTER_IMM command, followed by the address
at which to load, followed by the value to load.

Reflecting on my initial confusion, would it be clearer to provide names for
each dword position in the context image, rather than using an unnamed offset
like CTX_R_PWR_CLK_STATE+1? Example:

reg_state[CTX_R_PWR_CLK_STATE_ADDR] = 0x20c8
reg_state[CTX_R_PWR_CLK_STATE_DATA] = 0;

Jeff
> > > + reg_state[CTX_R_PWR_CLK_STATE+1] = 0;
> > > + }
> > > +
> > > +#if 0
> > > + /* Offsets not yet defined for these */
> > > + reg_state[CTX_GPGPU_CSR_BASE_ADDRESS[] = ;
> > > + reg_state[CTX_GPGPU_CSR_BASE_ADDRESS+1] = 0;
> > > +#endif
> > > +
> > > + kunmap_atomic(reg_state);
> > > +
> > > + ctx->obj->dirty = 1;
> > > + set_page_dirty(page);
> > > + i915_gem_object_unpin_pages(ctx->obj);
> > > +
> > > return ctx;
> > > +
> > > +destroy_ring_obj:
> > > + i915_gem_object_ggtt_unpin(ring_obj);
> > > + drm_gem_object_unreference(&ring_obj->base);
> > > + ctx->ringbuf->obj = NULL;
> > > + ctx->ringbuf = NULL;
> > > + i915_gem_object_ggtt_unpin(ctx->obj);
> > > + i915_gem_context_unreference(ctx);
> > > +
> > > + return ERR_PTR(ret);
> > > }
> > >
> > > void gen8_gem_context_fini(struct drm_device *dev)
> > > --
> > > 1.9.0
> > >
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx