On Thu, Mar 06, 2014 at 05:17:59AM -0800, Jani Nikula wrote:
> On Tue, 18 Feb 2014, bradley.d.volkin@xxxxxxxxx wrote:
> > From: Brad Volkin <bradley.d.volkin@xxxxxxxxx>
> >
> > Various commands that access memory have a bit to determine whether
> > the graphics address specified in the command should use the GGTT or
> > PPGTT for translation. These checks ensure that the bit indicates
> > PPGTT translation.
> >
> > Most of these checks use the existing bit-checking infrastructure.
> > The PIPE_CONTROL and MI_FLUSH_DW commands, however, are multi-function
> > commands. The GGTT/PPGTT bit is only relevant for certain uses of the
> > command. As such, this change also extends the bit-checking code to
> > include a "condition" mask and offset. If the condition mask is non-zero
> > then the parser only performs the bit check when the bits specified by
> > the condition mask/offset are also non-zero.
> >
> > NOTE: At this point in the series PPGTT must be enabled for the parser
> > to work correctly. If it's not enabled, userspace will not be setting
> > the PPGTT bits the way the parser requires. VLV is the only platform
> > where this is a problem, so at this point, we disable parsing for VLV.
> >
> > v2: whitespace and trailing commas fixes, rebased
> >
> > OTC-Tracker: AXIA-4631
> > Change-Id: I3f4c76b6734f1956ec47e698230f97d0998ff92b
> > Signed-off-by: Brad Volkin <bradley.d.volkin@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/i915_cmd_parser.c | 128 ++++++++++++++++++++++++++++++---
> > drivers/gpu/drm/i915/i915_drv.h | 6 ++
> > drivers/gpu/drm/i915/i915_reg.h | 6 ++
> > 3 files changed, 129 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > index 0351df1..1528549 100644
> > --- a/drivers/gpu/drm/i915/i915_cmd_parser.c
> > +++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
> > @@ -124,10 +124,20 @@ static const struct drm_i915_cmd_descriptor common_cmds[] = {
> > CMD( MI_STORE_DWORD_INDEX, SMI, !F, 0xFF, R ),
> > CMD( MI_LOAD_REGISTER_IMM(1), SMI, !F, 0xFF, W,
> > .reg = { .offset = 1, .mask = 0x007FFFFC } ),
> > - CMD( MI_STORE_REGISTER_MEM(1), SMI, !F, 0xFF, W,
> > - .reg = { .offset = 1, .mask = 0x007FFFFC } ),
> > - CMD( MI_LOAD_REGISTER_MEM, SMI, !F, 0xFF, W,
> > - .reg = { .offset = 1, .mask = 0x007FFFFC } ),
> > + CMD( MI_STORE_REGISTER_MEM(1), SMI, !F, 0xFF, W | B,
> > + .reg = { .offset = 1, .mask = 0x007FFFFC },
> > + .bits = {{
> > + .offset = 0,
> > + .mask = MI_GLOBAL_GTT,
> > + .expected = 0,
> > + }}, ),
> > + CMD( MI_LOAD_REGISTER_MEM, SMI, !F, 0xFF, W | B,
> > + .reg = { .offset = 1, .mask = 0x007FFFFC },
> > + .bits = {{
> > + .offset = 0,
> > + .mask = MI_GLOBAL_GTT,
> > + .expected = 0,
> > + }}, ),
> > CMD( MI_BATCH_BUFFER_START, SMI, !F, 0xFF, S ),
> > };
> >
> > @@ -139,9 +149,31 @@ static const struct drm_i915_cmd_descriptor render_cmds[] = {
> > CMD( MI_DISPLAY_FLIP, SMI, !F, 0xFF, R ),
> > CMD( MI_SET_CONTEXT, SMI, !F, 0xFF, R ),
> > CMD( MI_URB_CLEAR, SMI, !F, 0xFF, S ),
> > + CMD( MI_STORE_DWORD_IMM, SMI, !F, 0x3F, B,
> > + .bits = {{
> > + .offset = 0,
> > + .mask = MI_GLOBAL_GTT,
> > + .expected = 0,
> > + }}, ),
> > CMD( MI_UPDATE_GTT, SMI, !F, 0xFF, R ),
> > - CMD( MI_CLFLUSH, SMI, !F, 0x3FF, S ),
> > - CMD( MI_CONDITIONAL_BATCH_BUFFER_END, SMI, !F, 0xFF, S ),
> > + CMD( MI_CLFLUSH, SMI, !F, 0x3FF, B,
> > + .bits = {{
> > + .offset = 0,
> > + .mask = MI_GLOBAL_GTT,
> > + .expected = 0,
> > + }}, ),
> > + CMD( MI_REPORT_PERF_COUNT, SMI, !F, 0x3F, B,
> > + .bits = {{
> > + .offset = 1,
> > + .mask = MI_REPORT_PERF_COUNT_GGTT,
> > + .expected = 0,
> > + }}, ),
> > + CMD( MI_CONDITIONAL_BATCH_BUFFER_END, SMI, !F, 0xFF, B,
> > + .bits = {{
> > + .offset = 0,
> > + .mask = MI_GLOBAL_GTT,
> > + .expected = 0,
> > + }}, ),
> > CMD( GFX_OP_3DSTATE_VF_STATISTICS, S3D, F, 1, S ),
> > CMD( PIPELINE_SELECT, S3D, F, 1, S ),
> > CMD( MEDIA_VFE_STATE, S3D, !F, 0xFFFF, B,
> > @@ -158,6 +190,13 @@ static const struct drm_i915_cmd_descriptor render_cmds[] = {
> > .offset = 1,
> > .mask = (PIPE_CONTROL_MMIO_WRITE | PIPE_CONTROL_NOTIFY),
> > .expected = 0,
> > + },
> > + {
> > + .offset = 1,
> > + .mask = PIPE_CONTROL_GLOBAL_GTT_IVB,
> > + .expected = 0,
> > + .condition_offset = 1,
> > + .condition_mask = PIPE_CONTROL_POST_SYNC_OP_MASK,
> > }}, ),
> > };
> >
> > @@ -184,15 +223,32 @@ static const struct drm_i915_cmd_descriptor hsw_render_cmds[] = {
> >
> > static const struct drm_i915_cmd_descriptor video_cmds[] = {
> > CMD( MI_ARB_ON_OFF, SMI, F, 1, R ),
> > - CMD( MI_STORE_DWORD_IMM, SMI, !F, 0xFF, S ),
> > + CMD( MI_STORE_DWORD_IMM, SMI, !F, 0xFF, B,
> > + .bits = {{
> > + .offset = 0,
> > + .mask = MI_GLOBAL_GTT,
> > + .expected = 0,
> > + }}, ),
> > CMD( MI_UPDATE_GTT, SMI, !F, 0x3F, R ),
> > CMD( MI_FLUSH_DW, SMI, !F, 0x3F, B,
> > .bits = {{
> > .offset = 0,
> > .mask = MI_FLUSH_DW_NOTIFY,
> > .expected = 0,
> > + },
> > + {
> > + .offset = 1,
> > + .mask = MI_FLUSH_DW_USE_GTT,
> > + .expected = 0,
> > + .condition_offset = 0,
> > + .condition_mask = MI_FLUSH_DW_OP_MASK,
> > + }}, ),
> > + CMD( MI_CONDITIONAL_BATCH_BUFFER_END, SMI, !F, 0xFF, B,
> > + .bits = {{
> > + .offset = 0,
> > + .mask = MI_GLOBAL_GTT,
> > + .expected = 0,
> > }}, ),
> > - CMD( MI_CONDITIONAL_BATCH_BUFFER_END, SMI, !F, 0xFF, S ),
> > /*
> > * MFX_WAIT doesn't fit the way we handle length for most commands.
> > * It has a length field but it uses a non-standard length bias.
> > @@ -203,26 +259,55 @@ static const struct drm_i915_cmd_descriptor video_cmds[] = {
> >
> > static const struct drm_i915_cmd_descriptor vecs_cmds[] = {
> > CMD( MI_ARB_ON_OFF, SMI, F, 1, R ),
> > - CMD( MI_STORE_DWORD_IMM, SMI, !F, 0xFF, S ),
> > + CMD( MI_STORE_DWORD_IMM, SMI, !F, 0xFF, B,
> > + .bits = {{
> > + .offset = 0,
> > + .mask = MI_GLOBAL_GTT,
> > + .expected = 0,
> > + }}, ),
> > CMD( MI_UPDATE_GTT, SMI, !F, 0x3F, R ),
> > CMD( MI_FLUSH_DW, SMI, !F, 0x3F, B,
> > .bits = {{
> > .offset = 0,
> > .mask = MI_FLUSH_DW_NOTIFY,
> > .expected = 0,
> > + },
> > + {
> > + .offset = 1,
> > + .mask = MI_FLUSH_DW_USE_GTT,
> > + .expected = 0,
> > + .condition_offset = 0,
> > + .condition_mask = MI_FLUSH_DW_OP_MASK,
> > + }}, ),
> > + CMD( MI_CONDITIONAL_BATCH_BUFFER_END, SMI, !F, 0xFF, B,
> > + .bits = {{
> > + .offset = 0,
> > + .mask = MI_GLOBAL_GTT,
> > + .expected = 0,
> > }}, ),
> > - CMD( MI_CONDITIONAL_BATCH_BUFFER_END, SMI, !F, 0xFF, S ),
> > };
> >
> > static const struct drm_i915_cmd_descriptor blt_cmds[] = {
> > CMD( MI_DISPLAY_FLIP, SMI, !F, 0xFF, R ),
> > - CMD( MI_STORE_DWORD_IMM, SMI, !F, 0x3FF, S ),
> > + CMD( MI_STORE_DWORD_IMM, SMI, !F, 0x3FF, B,
> > + .bits = {{
> > + .offset = 0,
> > + .mask = MI_GLOBAL_GTT,
> > + .expected = 0,
> > + }}, ),
> > CMD( MI_UPDATE_GTT, SMI, !F, 0x3F, R ),
> > CMD( MI_FLUSH_DW, SMI, !F, 0x3F, B,
> > .bits = {{
> > .offset = 0,
> > .mask = MI_FLUSH_DW_NOTIFY,
> > .expected = 0,
> > + },
> > + {
> > + .offset = 1,
> > + .mask = MI_FLUSH_DW_USE_GTT,
> > + .expected = 0,
> > + .condition_offset = 0,
> > + .condition_mask = MI_FLUSH_DW_OP_MASK,
> > }}, ),
> > CMD( COLOR_BLT, S2D, !F, 0x3F, S ),
> > CMD( SRC_COPY_BLT, S2D, !F, 0x3F, S ),
> > @@ -617,10 +702,21 @@ finish:
> > */
> > bool i915_needs_cmd_parser(struct intel_ring_buffer *ring)
> > {
> > + drm_i915_private_t *dev_priv =
> > + (drm_i915_private_t *)ring->dev->dev_private;
>
> Make that:
>
> struct drm_i915_private *dev_priv = ring->dev->dev_private;
Is this something that can be fixed up when merging the patch, or
should I fix it up and resend just this one?
Brad
>
>
> > +
> > /* No command tables indicates a platform without parsing */
> > if (!ring->cmd_tables)
> > return false;
> >
> > + /*
> > + * XXX: VLV is Gen7 and therefore has cmd_tables, but has PPGTT
> > + * disabled. That will cause all of the parser's PPGTT checks to
> > + * fail. For now, disable parsing when PPGTT is off.
> > + */
> > + if (!dev_priv->mm.aliasing_ppgtt)
> > + return false;
> > +
> > return (i915.enable_cmd_parser == 1);
> > }
> >
> > @@ -737,6 +833,16 @@ int i915_parse_cmds(struct intel_ring_buffer *ring,
> > if (desc->bits[i].mask == 0)
> > break;
> >
> > + if (desc->bits[i].condition_mask != 0) {
> > + u32 offset =
> > + desc->bits[i].condition_offset;
> > + u32 condition = cmd[offset] &
> > + desc->bits[i].condition_mask;
> > +
> > + if (condition == 0)
> > + continue;
> > + }
> > +
> > dword = cmd[desc->bits[i].offset] &
> > desc->bits[i].mask;
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 376472f..27a48d9 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -1868,11 +1868,17 @@ struct drm_i915_cmd_descriptor {
> > * the expected value, the parser rejects it. Only valid if flags has
> > * the CMD_DESC_BITMASK bit set. Only entries where mask is non-zero
> > * are valid.
> > + *
> > + * If the check specifies a non-zero condition_mask then the parser
> > + * only performs the check when the bits specified by condition_mask
> > + * are non-zero.
> > */
> > struct {
> > u32 offset;
> > u32 mask;
> > u32 expected;
> > + u32 condition_offset;
> > + u32 condition_mask;
> > } bits[MAX_CMD_DESC_BITMASKS];
> > };
> >
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index e6dd7e9..e683b31 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -190,6 +190,8 @@
> > * Memory interface instructions used by the kernel
> > */
> > #define MI_INSTR(opcode, flags) (((opcode) << 23) | (flags))
> > +/* Many MI commands use bit 22 of the header dword for GGTT vs PPGTT */
> > +#define MI_GLOBAL_GTT (1<<22)
> >
> > #define MI_NOOP MI_INSTR(0, 0)
> > #define MI_USER_INTERRUPT MI_INSTR(0x02, 0)
> > @@ -269,6 +271,7 @@
> > #define MI_FLUSH_DW_STORE_INDEX (1<<21)
> > #define MI_INVALIDATE_TLB (1<<18)
> > #define MI_FLUSH_DW_OP_STOREDW (1<<14)
> > +#define MI_FLUSH_DW_OP_MASK (3<<14)
> > #define MI_FLUSH_DW_NOTIFY (1<<8)
> > #define MI_INVALIDATE_BSD (1<<7)
> > #define MI_FLUSH_DW_USE_GTT (1<<2)
> > @@ -335,6 +338,7 @@
> > #define PIPE_CONTROL_CS_STALL (1<<20)
> > #define PIPE_CONTROL_TLB_INVALIDATE (1<<18)
> > #define PIPE_CONTROL_QW_WRITE (1<<14)
> > +#define PIPE_CONTROL_POST_SYNC_OP_MASK (3<<14)
> > #define PIPE_CONTROL_DEPTH_STALL (1<<13)
> > #define PIPE_CONTROL_WRITE_FLUSH (1<<12)
> > #define PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH (1<<12) /* gen6+ */
> > @@ -363,6 +367,8 @@
> > #define MI_URB_CLEAR MI_INSTR(0x19, 0)
> > #define MI_UPDATE_GTT MI_INSTR(0x23, 0)
> > #define MI_CLFLUSH MI_INSTR(0x27, 0)
> > +#define MI_REPORT_PERF_COUNT MI_INSTR(0x28, 0)
> > +#define MI_REPORT_PERF_COUNT_GGTT (1<<0)
> > #define MI_LOAD_REGISTER_MEM MI_INSTR(0x29, 0)
> > #define MI_LOAD_REGISTER_REG MI_INSTR(0x2A, 0)
> > #define MI_RS_STORE_DATA_IMM MI_INSTR(0x2B, 0)
> > --
> > 1.8.3.2
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
