[Intel-gfx] [PATCH] drm/i915: track relative-constants-mode per-context not per-device
Daniel Vetter
daniel at ffwll.ch
Tue Oct 13 08:28:23 PDT 2015
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 at intel.com>
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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
More information about the Intel-gfx
mailing list