On Tue, Oct 13, 2015 at 02:20:05PM +0100, Dave Gordon wrote: > 'relative_constants_mode' has always been tracked per-device, but this > is wrong in execlists (or GuC) mode, as INSTPM is saved and restored > with the logical context, and the per-context value could therefore get > out of sync with the tracked value. This patch moves the tracking > element from the dev_priv structure into the intel_context structure, > with corresponding adjustments to the code that initialises and uses it. > > Test case (if anyone wants to write it) would be to create two contexts, > submit a batch with a non-default mode in the first context, submit a > batch with the default mode in the other context, submit another batch > in the first context, but this time in default mode. The driver will > fail to insert the instructions to reset INSTPM into the first context's > ringbuffer. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92448 > > Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx> Doesn't this break pre-gen8? Or maybe legacy mode instead of execlist, dunno. If it's pre-gen8 we need a check, if it's execlist vs. legacy then we could just move this into the ring instead. -Daniel > --- > drivers/gpu/drm/i915/i915_drv.h | 4 ++-- > drivers/gpu/drm/i915/i915_gem.c | 2 -- > drivers/gpu/drm/i915/i915_gem_context.c | 3 +++ > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 +++--- > drivers/gpu/drm/i915/intel_lrc.c | 6 +++--- > 5 files changed, 11 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 51eea29..2917370 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -872,6 +872,8 @@ struct intel_context { > struct i915_ctx_hang_stats hang_stats; > struct i915_hw_ppgtt *ppgtt; > > + int relative_constants_mode; > + > /* Legacy ring buffer submission */ > struct { > struct drm_i915_gem_object *rcs_state; > @@ -1707,8 +1709,6 @@ struct drm_i915_private { > > const struct intel_device_info info; > > - int relative_constants_mode; > - > void __iomem *regs; > > struct intel_uncore uncore; > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index f0cfbb9..374af2d 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -4894,8 +4894,6 @@ i915_gem_load(struct drm_device *dev) > i915_gem_idle_work_handler); > init_waitqueue_head(&dev_priv->gpu_error.reset_queue); > > - dev_priv->relative_constants_mode = I915_EXEC_CONSTANTS_REL_GENERAL; > - > if (INTEL_INFO(dev)->gen >= 7 && !IS_VALLEYVIEW(dev)) > dev_priv->num_fence_regs = 32; > else if (INTEL_INFO(dev)->gen >= 4 || IS_I945G(dev) || IS_I945GM(dev) || IS_G33(dev)) > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index 74aa0c9..465e3e0 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -222,6 +222,9 @@ __create_hw_context(struct drm_device *dev, > * is no remap info, it will be a NOP. */ > ctx->remap_slice = (1 << NUM_L3_SLICES(dev)) - 1; > > + /* First execbuffer will override this */ > + ctx->relative_constants_mode = -1; > + > ctx->hang_stats.ban_period_seconds = DRM_I915_CTX_BAN_PERIOD; > > return ctx; > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index edc17be..9833c8a 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1283,7 +1283,7 @@ i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params, > goto error; > } > > - if (instp_mode != dev_priv->relative_constants_mode) { > + if (instp_mode != params->ctx->relative_constants_mode) { > if (INTEL_INFO(dev)->gen < 4) { > DRM_DEBUG("no rel constants on pre-gen4\n"); > ret = -EINVAL; > @@ -1309,7 +1309,7 @@ i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params, > } > > if (ring == &dev_priv->ring[RCS] && > - instp_mode != dev_priv->relative_constants_mode) { > + instp_mode != params->ctx->relative_constants_mode) { > ret = intel_ring_begin(params->request, 4); > if (ret) > goto error; > @@ -1320,7 +1320,7 @@ i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params, > intel_ring_emit(ring, instp_mask << 16 | instp_mode); > intel_ring_advance(ring); > > - dev_priv->relative_constants_mode = instp_mode; > + params->ctx->relative_constants_mode = instp_mode; > } > > if (args->flags & I915_EXEC_GEN7_SOL_RESET) { > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 825fa7a..9ff409d 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -889,7 +889,7 @@ int intel_execlists_submission(struct i915_execbuffer_params *params, > return -EINVAL; > } > > - if (instp_mode != dev_priv->relative_constants_mode) { > + if (instp_mode != params->ctx->relative_constants_mode) { > if (instp_mode == I915_EXEC_CONSTANTS_REL_SURFACE) { > DRM_DEBUG("rel surface constants mode invalid on gen5+\n"); > return -EINVAL; > @@ -929,7 +929,7 @@ int intel_execlists_submission(struct i915_execbuffer_params *params, > return ret; > > if (ring == &dev_priv->ring[RCS] && > - instp_mode != dev_priv->relative_constants_mode) { > + instp_mode != params->ctx->relative_constants_mode) { > ret = intel_logical_ring_begin(params->request, 4); > if (ret) > return ret; > @@ -940,7 +940,7 @@ int intel_execlists_submission(struct i915_execbuffer_params *params, > intel_logical_ring_emit(ringbuf, instp_mask << 16 | instp_mode); > intel_logical_ring_advance(ringbuf); > > - dev_priv->relative_constants_mode = instp_mode; > + params->ctx->relative_constants_mode = instp_mode; > } > > exec_start = params->batch_obj_vm_offset + > -- > 1.9.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
