[Intel-gfx] [PATCH] drm/i915: Only check PPGTT bits when using PPGTT
bradley.d.volkin at intel.com
bradley.d.volkin at intel.com
Wed May 28 22:47:59 CEST 2014
From: Brad Volkin <bradley.d.volkin at intel.com>
This extends use of the command parser to VLV.
Note that the patch checks that the PPGTT bit is set appropriately when
PPGTT is enabled but ignores it when PPGTT is disabled. It would be
awkward to correctly invert the expected value to check that the bit is
set appropriately in that case, and of limited value anyhow.
Signed-off-by: Brad Volkin <bradley.d.volkin at intel.com>
---
I've confirmed that the shmem pread setup stuff we added does fix the
caching issues I saw previously. I've done some basic testing with this
on both IVB and VLV and don't see regressions. I don't have any data on
the VLV perf impact though.
Also, I considered splitting the patch up a bit differently but decided
that a single patch seemed ok. I'm happy to split it up a bit if that's
what people prefer.
drivers/gpu/drm/i915/i915_cmd_parser.c | 187 +++++++++++++++++----------------
drivers/gpu/drm/i915/i915_drv.h | 8 +-
2 files changed, 104 insertions(+), 91 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_cmd_parser.c b/drivers/gpu/drm/i915/i915_cmd_parser.c
index 9d79543..fd35900 100644
--- a/drivers/gpu/drm/i915/i915_cmd_parser.c
+++ b/drivers/gpu/drm/i915/i915_cmd_parser.c
@@ -110,6 +110,7 @@
#define W CMD_DESC_REGISTER
#define B CMD_DESC_BITMASK
#define M CMD_DESC_MASTER
+#define P CMD_DESC_PPGTT
/* Command Mask Fixed Len Action
---------------------------------------------------------- */
@@ -124,20 +125,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 | B,
+ CMD( MI_STORE_REGISTER_MEM(1), SMI, !F, 0xFF, W | P,
.reg = { .offset = 1, .mask = 0x007FFFFC },
- .bits = {{
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
- CMD( MI_LOAD_REGISTER_MEM, SMI, !F, 0xFF, W | B,
+ }, ),
+ CMD( MI_LOAD_REGISTER_MEM, SMI, !F, 0xFF, W | P,
.reg = { .offset = 1, .mask = 0x007FFFFC },
- .bits = {{
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
+ }, ),
CMD( MI_BATCH_BUFFER_START, SMI, !F, 0xFF, S ),
};
@@ -149,31 +150,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 = {{
+ CMD( MI_STORE_DWORD_IMM, SMI, !F, 0x3F, P,
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
+ }, ),
CMD( MI_UPDATE_GTT, SMI, !F, 0xFF, R ),
- CMD( MI_CLFLUSH, SMI, !F, 0x3FF, B,
- .bits = {{
+ CMD( MI_CLFLUSH, SMI, !F, 0x3FF, P,
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
- CMD( MI_REPORT_PERF_COUNT, SMI, !F, 0x3F, B,
- .bits = {{
+ }, ),
+ CMD( MI_REPORT_PERF_COUNT, SMI, !F, 0x3F, P,
+ .ppgtt = {
.offset = 1,
.mask = MI_REPORT_PERF_COUNT_GGTT,
.expected = 0,
- }}, ),
- CMD( MI_CONDITIONAL_BATCH_BUFFER_END, SMI, !F, 0xFF, B,
- .bits = {{
+ }, ),
+ CMD( MI_CONDITIONAL_BATCH_BUFFER_END, SMI, !F, 0xFF, P,
+ .ppgtt = {
.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,
@@ -185,7 +186,14 @@ static const struct drm_i915_cmd_descriptor render_cmds[] = {
CMD( GPGPU_OBJECT, S3D, !F, 0xFF, S ),
CMD( GPGPU_WALKER, S3D, !F, 0xFF, S ),
CMD( GFX_OP_3DSTATE_SO_DECL_LIST, S3D, !F, 0x1FF, S ),
- CMD( GFX_OP_PIPE_CONTROL(5), S3D, !F, 0xFF, B,
+ CMD( GFX_OP_PIPE_CONTROL(5), S3D, !F, 0xFF, B | P,
+ .ppgtt = {
+ .offset = 1,
+ .mask = PIPE_CONTROL_GLOBAL_GTT_IVB,
+ .expected = 0,
+ .condition_offset = 1,
+ .condition_mask = PIPE_CONTROL_POST_SYNC_OP_MASK,
+ },
.bits = {{
.offset = 1,
.mask = (PIPE_CONTROL_MMIO_WRITE | PIPE_CONTROL_NOTIFY),
@@ -193,8 +201,7 @@ static const struct drm_i915_cmd_descriptor render_cmds[] = {
},
{
.offset = 1,
- .mask = (PIPE_CONTROL_GLOBAL_GTT_IVB |
- PIPE_CONTROL_STORE_DATA_INDEX),
+ .mask = PIPE_CONTROL_STORE_DATA_INDEX,
.expected = 0,
.condition_offset = 1,
.condition_mask = PIPE_CONTROL_POST_SYNC_OP_MASK,
@@ -224,26 +231,26 @@ 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, B,
- .bits = {{
+ CMD( MI_STORE_DWORD_IMM, SMI, !F, 0xFF, P,
+ .ppgtt = {
.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,
- },
- {
+ CMD( MI_FLUSH_DW, SMI, !F, 0x3F, B | P,
+ .ppgtt = {
.offset = 1,
.mask = MI_FLUSH_DW_USE_GTT,
.expected = 0,
.condition_offset = 0,
.condition_mask = MI_FLUSH_DW_OP_MASK,
},
+ .bits = {{
+ .offset = 0,
+ .mask = MI_FLUSH_DW_NOTIFY,
+ .expected = 0,
+ },
{
.offset = 0,
.mask = MI_FLUSH_DW_STORE_INDEX,
@@ -251,12 +258,12 @@ static const struct drm_i915_cmd_descriptor video_cmds[] = {
.condition_offset = 0,
.condition_mask = MI_FLUSH_DW_OP_MASK,
}}, ),
- CMD( MI_CONDITIONAL_BATCH_BUFFER_END, SMI, !F, 0xFF, B,
- .bits = {{
+ CMD( MI_CONDITIONAL_BATCH_BUFFER_END, SMI, !F, 0xFF, P,
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
+ }, ),
/*
* 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.
@@ -267,26 +274,26 @@ 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, B,
- .bits = {{
+ CMD( MI_STORE_DWORD_IMM, SMI, !F, 0xFF, B | P,
+ .ppgtt = {
.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,
- },
- {
+ CMD( MI_FLUSH_DW, SMI, !F, 0x3F, B | P,
+ .ppgtt = {
.offset = 1,
.mask = MI_FLUSH_DW_USE_GTT,
.expected = 0,
.condition_offset = 0,
.condition_mask = MI_FLUSH_DW_OP_MASK,
},
+ .bits = {{
+ .offset = 0,
+ .mask = MI_FLUSH_DW_NOTIFY,
+ .expected = 0,
+ },
{
.offset = 0,
.mask = MI_FLUSH_DW_STORE_INDEX,
@@ -294,36 +301,36 @@ static const struct drm_i915_cmd_descriptor vecs_cmds[] = {
.condition_offset = 0,
.condition_mask = MI_FLUSH_DW_OP_MASK,
}}, ),
- CMD( MI_CONDITIONAL_BATCH_BUFFER_END, SMI, !F, 0xFF, B,
- .bits = {{
+ CMD( MI_CONDITIONAL_BATCH_BUFFER_END, SMI, !F, 0xFF, P,
+ .ppgtt = {
.offset = 0,
.mask = MI_GLOBAL_GTT,
.expected = 0,
- }}, ),
+ }, ),
};
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, B,
- .bits = {{
+ CMD( MI_STORE_DWORD_IMM, SMI, !F, 0x3FF, P,
+ .ppgtt = {
.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,
- },
- {
+ CMD( MI_FLUSH_DW, SMI, !F, 0x3F, B | P,
+ .ppgtt = {
.offset = 1,
.mask = MI_FLUSH_DW_USE_GTT,
.expected = 0,
.condition_offset = 0,
.condition_mask = MI_FLUSH_DW_OP_MASK,
},
+ .bits = {{
+ .offset = 0,
+ .mask = MI_FLUSH_DW_NOTIFY,
+ .expected = 0,
+ },
{
.offset = 0,
.mask = MI_FLUSH_DW_STORE_INDEX,
@@ -351,6 +358,7 @@ static const struct drm_i915_cmd_descriptor hsw_blt_cmds[] = {
#undef W
#undef B
#undef M
+#undef P
static const struct drm_i915_cmd_table gen7_render_cmds[] = {
{ common_cmds, ARRAY_SIZE(common_cmds) },
@@ -797,6 +805,34 @@ static bool valid_reg(const u32 *table, int count, u32 addr)
return false;
}
+static bool valid_bits(const int ring_id,
+ const struct drm_i915_cmd_bits *bits,
+ const u32 *cmd)
+{
+ u32 dword;
+
+ if (bits->condition_mask != 0) {
+ u32 offset = bits->condition_offset;
+ u32 condition = cmd[offset] & bits->condition_mask;
+
+ if (condition == 0)
+ return true;
+ }
+
+ dword = cmd[bits->offset] & bits->mask;
+
+ if (dword != bits->expected) {
+ DRM_DEBUG_DRIVER("CMD: Rejected command 0x%08X for bitmask 0x%08X (exp=0x%08X act=0x%08X) (ring=%d)\n",
+ *cmd,
+ bits->mask,
+ bits->expected,
+ dword, ring_id);
+ return false;
+ }
+
+ return true;
+}
+
static u32 *vmap_batch(struct drm_i915_gem_object *obj)
{
int i;
@@ -839,19 +875,9 @@ finish:
*/
bool i915_needs_cmd_parser(struct intel_engine_cs *ring)
{
- struct drm_i915_private *dev_priv = ring->dev->dev_private;
-
if (!ring->needs_cmd_parser)
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);
}
@@ -907,36 +933,19 @@ static bool check_cmd(const struct intel_engine_cs *ring,
}
}
+ if ((desc->flags & CMD_DESC_PPGTT) && USES_PPGTT(ring->dev))
+ if (!valid_bits(ring->id, &desc->ppgtt, cmd))
+ return false;
+
if (desc->flags & CMD_DESC_BITMASK) {
int i;
for (i = 0; i < MAX_CMD_DESC_BITMASKS; i++) {
- u32 dword;
-
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;
-
- if (dword != desc->bits[i].expected) {
- DRM_DEBUG_DRIVER("CMD: Rejected command 0x%08X for bitmask 0x%08X (exp=0x%08X act=0x%08X) (ring=%d)\n",
- *cmd,
- desc->bits[i].mask,
- desc->bits[i].expected,
- dword, ring->id);
+ if (!valid_bits(ring->id, &desc->bits[i], cmd))
return false;
- }
}
}
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index bea9ab40..6eead7a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1787,6 +1787,9 @@ struct drm_i915_cmd_descriptor {
* register whitelist for the appropriate ring
* CMD_DESC_MASTER: The command is allowed if the submitting process
* is the DRM master
+ * CMD_DESC_PPGTT: The command contains a bit that indicates GGTT or
+ * PPGTT access, which must be checked only when
+ * PPGTT is enabled
*/
u32 flags;
#define CMD_DESC_FIXED (1<<0)
@@ -1795,6 +1798,7 @@ struct drm_i915_cmd_descriptor {
#define CMD_DESC_REGISTER (1<<3)
#define CMD_DESC_BITMASK (1<<4)
#define CMD_DESC_MASTER (1<<5)
+#define CMD_DESC_PPGTT (1<<6)
/*
* The command's unique identification bits and the bitmask to get them.
@@ -1840,13 +1844,13 @@ struct drm_i915_cmd_descriptor {
* only performs the check when the bits specified by condition_mask
* are non-zero.
*/
- struct {
+ struct drm_i915_cmd_bits {
u32 offset;
u32 mask;
u32 expected;
u32 condition_offset;
u32 condition_mask;
- } bits[MAX_CMD_DESC_BITMASKS];
+ } bits[MAX_CMD_DESC_BITMASKS], ppgtt;
};
/*
--
1.8.3.2
More information about the Intel-gfx
mailing list