From patchwork Wed May 28 20:47:59 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: bradley.d.volkin@intel.com X-Patchwork-Id: 4257281 Return-Path: X-Original-To: patchwork-intel-gfx@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 583B2BF90B for ; Wed, 28 May 2014 20:47:42 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 3897A202BE for ; Wed, 28 May 2014 20:47:41 +0000 (UTC) Received: from gabe.freedesktop.org (gabe.freedesktop.org [131.252.210.177]) by mail.kernel.org (Postfix) with ESMTP id E42BE202E5 for ; Wed, 28 May 2014 20:47:39 +0000 (UTC) Received: from gabe.freedesktop.org (localhost [127.0.0.1]) by gabe.freedesktop.org (Postfix) with ESMTP id 4104D6E5D8; Wed, 28 May 2014 13:47:39 -0700 (PDT) X-Original-To: intel-gfx@lists.freedesktop.org Delivered-To: intel-gfx@lists.freedesktop.org Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id A17556E5D8 for ; Wed, 28 May 2014 13:47:37 -0700 (PDT) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga102.fm.intel.com with ESMTP; 28 May 2014 13:47:35 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.98,930,1392192000"; d="scan'208";a="546369047" Received: from bdvolkin-cube.ra.intel.com (HELO bdvolkin-ubuntu-desktop.ra.intel.com) ([10.10.34.187]) by fmsmga002.fm.intel.com with ESMTP; 28 May 2014 13:47:34 -0700 From: bradley.d.volkin@intel.com To: intel-gfx@lists.freedesktop.org Date: Wed, 28 May 2014 13:47:59 -0700 Message-Id: <1401310079-22771-1-git-send-email-bradley.d.volkin@intel.com> X-Mailer: git-send-email 1.8.3.2 Subject: [Intel-gfx] [PATCH] drm/i915: Only check PPGTT bits when using PPGTT X-BeenThere: intel-gfx@lists.freedesktop.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: Intel graphics driver community testing & development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Brad Volkin 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 --- 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; }; /*