From: Jani Nikula <jani.nikula@linux.intel.com>
To: bradley.d.volkin@intel.com, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 10/13] drm/i915: Enable PPGTT command parser checks
Date: Thu, 06 Mar 2014 15:17:59 +0200 [thread overview]
Message-ID: <87ha7b31q0.fsf@intel.com> (raw)
In-Reply-To: <1392747357-25703-11-git-send-email-bradley.d.volkin@intel.com>
On Tue, 18 Feb 2014, bradley.d.volkin@intel.com wrote:
> From: Brad Volkin <bradley.d.volkin@intel.com>
>
> 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@intel.com>
> ---
> 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;
> +
> /* 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@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Jani Nikula, Intel Open Source Technology Center
next prev parent reply other threads:[~2014-03-06 13:18 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-02-18 18:15 [PATCH 00/13] Gen7 batch buffer command parser bradley.d.volkin
2014-02-18 18:15 ` [PATCH 01/13] drm/i915: Refactor shmem pread setup bradley.d.volkin
2014-03-06 12:13 ` Jani Nikula
2014-02-18 18:15 ` [PATCH 02/13] drm/i915: Implement command buffer parsing logic bradley.d.volkin
2014-03-06 13:10 ` Jani Nikula
2014-03-06 21:07 ` Daniel Vetter
2014-03-20 12:40 ` Jani Nikula
2014-02-18 18:15 ` [PATCH 03/13] drm/i915: Initial command parser table definitions bradley.d.volkin
2014-03-06 13:12 ` Jani Nikula
2014-02-18 18:15 ` [PATCH 04/13] drm/i915: Reject privileged commands bradley.d.volkin
2014-02-18 18:15 ` [PATCH 05/13] drm/i915: Allow some privileged commands from master bradley.d.volkin
2014-02-18 18:15 ` [PATCH 06/13] drm/i915: Add register whitelists for mesa bradley.d.volkin
2014-02-18 18:15 ` [PATCH 07/13] drm/i915: Add register whitelist for DRM master bradley.d.volkin
2014-02-18 18:15 ` [PATCH 08/13] drm/i915: Enable register whitelist checks bradley.d.volkin
2014-02-18 18:15 ` [PATCH 09/13] drm/i915: Reject commands that explicitly generate interrupts bradley.d.volkin
2014-02-18 18:15 ` [PATCH 10/13] drm/i915: Enable PPGTT command parser checks bradley.d.volkin
2014-03-06 13:17 ` Jani Nikula [this message]
2014-03-06 21:32 ` Volkin, Bradley D
2014-03-06 21:58 ` Daniel Vetter
2014-03-06 22:13 ` Volkin, Bradley D
2014-02-18 18:15 ` [PATCH 11/13] drm/i915: Reject commands that would store to global HWS page bradley.d.volkin
2014-02-18 18:15 ` [PATCH 12/13] drm/i915: Add a CMD_PARSER_VERSION getparam bradley.d.volkin
2014-02-18 18:15 ` [PATCH 13/13] drm/i915: Enable command parsing by default bradley.d.volkin
2014-03-04 17:21 ` [PATCH 00/13] Gen7 batch buffer command parser Volkin, Bradley D
2014-03-05 10:46 ` Daniel Vetter
2014-03-05 16:59 ` Volkin, Bradley D
2014-03-05 17:14 ` Daniel Vetter
2014-03-05 17:45 ` Volkin, Bradley D
2014-03-11 12:41 ` Jani Nikula
2014-03-12 18:02 ` Volkin, Bradley D
2014-03-20 14:43 ` Jani Nikula
2014-03-25 13:15 ` Daniel Vetter
2014-03-25 13:21 ` Jani Nikula
2014-03-25 19:46 ` Volkin, Bradley D
2014-03-25 19:53 ` Daniel Vetter
-- strict thread matches above, loose matches on Subject: below --
2013-11-26 16:51 [RFC 00/22] " bradley.d.volkin
2014-01-29 21:55 ` [PATCH 00/13] " bradley.d.volkin
2014-01-29 21:55 ` [PATCH 10/13] drm/i915: Enable PPGTT command parser checks bradley.d.volkin
2014-01-29 22:33 ` Chris Wilson
2014-01-29 23:00 ` Volkin, Bradley D
2014-01-29 23:08 ` Chris Wilson
2014-02-05 15:37 ` Jani Nikula
2014-02-05 18:54 ` Volkin, Bradley D
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87ha7b31q0.fsf@intel.com \
--to=jani.nikula@linux.intel.com \
--cc=bradley.d.volkin@intel.com \
--cc=intel-gfx@lists.freedesktop.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.