On Thu, Mar 06, 2014 at 01:58:09PM -0800, Daniel Vetter wrote:
> 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@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?
>
> 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
Apologies. I think the flaw is that some extra spaces in the tables and log messages
making lines longer than 80 characters drive checkpatch crazy. I'm so used to skimming
over those that I've probably ignored other issues too.
Brad
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
