From: Daniel Vetter <daniel@ffwll.ch>
To: "Volkin, Bradley D" <bradley.d.volkin@intel.com>
Cc: "intel-gfx@lists.freedesktop.org" <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 10/13] drm/i915: Enable PPGTT command parser checks
Date: Thu, 6 Mar 2014 22:58:09 +0100 [thread overview]
Message-ID: <20140306215809.GL17001@phenom.ffwll.local> (raw)
In-Reply-To: <20140306213248.GA5700@bdvolkin-ubuntu-desktop>
On Thu, Mar 06, 2014 at 01:32:48PM -0800, Volkin, Bradley D wrote:
> On Thu, Mar 06, 2014 at 05:17:59AM -0800, Jani Nikula wrote:
> > 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;
>
> Is this something that can be fixed up when merging the patch, or
> should I fix it up and resend just this one?
I usually do that when merging, at least if it's not too much. Already
applied a small checkpatch appeasement change for your 2nd patch ;-)
BKM is to run scripts/checkpatch.pl before hitting send and working down
the list. Presuming your editor is configured correctly not upsetting
checkpatch should become natural after a few patches though. If not you
need a better editor ;-)
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
next prev parent reply other threads:[~2014-03-06 21:58 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
2014-03-06 21:32 ` Volkin, Bradley D
2014-03-06 21:58 ` Daniel Vetter [this message]
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=20140306215809.GL17001@phenom.ffwll.local \
--to=daniel@ffwll.ch \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox