All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+
@ 2016-01-13 10:44 Artur Harasimiuk
  2016-01-13 11:49 ` ✓ success: Fi.CI.BAT Patchwork
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Artur Harasimiuk @ 2016-01-13 10:44 UTC (permalink / raw)
  To: intel-gfx

Starting from Gen9 we can use IA-Coherent caches. Coherence may be not
required in certain cases and can be disabled in an easy way. To do this
we can set HDC_FORCE_NON_COHERENT bit in HDC_CHICKEN0 register. This
register is part of HW context, however it is private and cannot be
programmed from non-privileged batch buffer.

New parameter is to override default programming and allow UMD to
decide whether IA-Coherency is not needed for submitted batch buffer.
When flag is set KMD emits commands to disable coherency before batch
buffer execution starts. After execution finished state is restored.

When WaForceEnableNonCoherent is programmed, it does not make sense to
allow for coherency because this can lead to GPU hangs. In such
situation flag is ignored.

Signed-off-by: Artur Harasimiuk <artur.harasimiuk@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c            |  3 +++
 drivers/gpu/drm/i915/i915_drv.h            |  4 ++++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  8 ++++++++
 drivers/gpu/drm/i915/intel_lrc.c           | 28 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.c    |  7 +++++++
 include/uapi/drm/i915_drm.h                |  8 +++++++-
 6 files changed, 57 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 44a896c..79ecf20 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -172,6 +172,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_HAS_EXEC_SOFTPIN:
 		value = 1;
 		break;
+	case I915_PARAM_HAS_EXEC_FORCE_NON_COHERENT:
+		value = INTEL_INFO(dev)->gen >= 9;
+		break;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 104bd18..71d739c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -886,6 +886,10 @@ struct intel_context {
 	} engine[I915_NUM_RINGS];
 
 	struct list_head link;
+
+	struct {
+		unsigned int WaForceEnableNonCoherent:1;
+	} wa;
 };
 
 enum fb_op_origin {
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index d469c47..2997a58 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1400,6 +1400,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	if (!i915_gem_check_execbuffer(args))
 		return -EINVAL;
 
+	if ((args->flags & I915_EXEC_FORCE_NON_COHERENT) &&
+		INTEL_INFO(dev)->gen < 9)
+		return -EINVAL;
+
 	ret = validate_exec_list(dev, exec, args->buffer_count);
 	if (ret)
 		return ret;
@@ -1494,6 +1498,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 
 	i915_gem_context_reference(ctx);
 
+	/* Clear this flag when WA is programmed */
+	if (ctx->wa.WaForceEnableNonCoherent)
+		args->flags &= ~I915_EXEC_FORCE_NON_COHERENT;
+
 	if (ctx->ppgtt)
 		vm = &ctx->ppgtt->base;
 	else
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ab344e0..4482a6a 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -879,6 +879,24 @@ int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request)
 	return intel_logical_ring_begin(request, 0);
 }
 
+static inline void
+intel_lr_emit_force_non_coherent(struct i915_execbuffer_params *params,
+		struct drm_i915_gem_execbuffer2 *args, bool force)
+{
+	if (args->flags & I915_EXEC_FORCE_NON_COHERENT) {
+		struct intel_ringbuffer *ringbuf =
+				params->ctx->engine[params->ring->id].ringbuf;
+
+		intel_logical_ring_emit(ringbuf, MI_NOOP);
+		intel_logical_ring_emit(ringbuf, MI_LOAD_REGISTER_IMM(1));
+		intel_logical_ring_emit(ringbuf, HDC_CHICKEN0.reg);
+		intel_logical_ring_emit(ringbuf, force ?
+				_MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT) :
+				_MASKED_BIT_DISABLE(HDC_FORCE_NON_COHERENT));
+		intel_logical_ring_advance(ringbuf);
+	}
+}
+
 /**
  * execlists_submission() - submit a batchbuffer for execution, Execlists style
  * @dev: DRM device.
@@ -959,6 +977,8 @@ int intel_execlists_submission(struct i915_execbuffer_params *params,
 		dev_priv->relative_constants_mode = instp_mode;
 	}
 
+	intel_lr_emit_force_non_coherent(params, args, true);
+
 	exec_start = params->batch_obj_vm_offset +
 		     args->batch_start_offset;
 
@@ -966,6 +986,8 @@ int intel_execlists_submission(struct i915_execbuffer_params *params,
 	if (ret)
 		return ret;
 
+	intel_lr_emit_force_non_coherent(params, args, false);
+
 	trace_i915_gem_ring_dispatch(params->request, params->dispatch_flags);
 
 	i915_gem_execbuffer_move_to_active(vmas, params->request);
@@ -1112,6 +1134,9 @@ static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct i915_workarounds *w = &dev_priv->workarounds;
+	struct intel_context *ctx = req->ctx;
+
+	ctx->wa.WaForceEnableNonCoherent = 0;
 
 	if (w->count == 0)
 		return 0;
@@ -1129,6 +1154,9 @@ static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
 	for (i = 0; i < w->count; i++) {
 		intel_logical_ring_emit_reg(ringbuf, w->reg[i].addr);
 		intel_logical_ring_emit(ringbuf, w->reg[i].value);
+		ctx->wa.WaForceEnableNonCoherent |=
+				(w->reg[i].addr.reg == HDC_CHICKEN0.reg) &&
+				(w->reg[i].value & HDC_FORCE_NON_COHERENT);
 	}
 	intel_logical_ring_emit(ringbuf, MI_NOOP);
 
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 4060acf..a7e1f24 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -702,6 +702,9 @@ static int intel_ring_workarounds_emit(struct drm_i915_gem_request *req)
 	struct drm_device *dev = ring->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct i915_workarounds *w = &dev_priv->workarounds;
+	struct intel_context    *ctx = req->ctx;
+
+	ctx->wa.WaForceEnableNonCoherent = 0;
 
 	if (w->count == 0)
 		return 0;
@@ -719,6 +722,10 @@ static int intel_ring_workarounds_emit(struct drm_i915_gem_request *req)
 	for (i = 0; i < w->count; i++) {
 		intel_ring_emit_reg(ring, w->reg[i].addr);
 		intel_ring_emit(ring, w->reg[i].value);
+		ctx->wa.WaForceEnableNonCoherent |=
+				(w->reg[i].addr.reg == HDC_CHICKEN0.reg) &&
+				(w->reg[i].value & HDC_FORCE_NON_COHERENT);
+
 	}
 	intel_ring_emit(ring, MI_NOOP);
 
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index acf2102..c425e80 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -357,6 +357,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_GPU_RESET	 35
 #define I915_PARAM_HAS_RESOURCE_STREAMER 36
 #define I915_PARAM_HAS_EXEC_SOFTPIN	 37
+#define I915_PARAM_HAS_EXEC_FORCE_NON_COHERENT 38
 
 typedef struct drm_i915_getparam {
 	__s32 param;
@@ -782,7 +783,12 @@ struct drm_i915_gem_execbuffer2 {
  */
 #define I915_EXEC_RESOURCE_STREAMER     (1<<15)
 
-#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_RESOURCE_STREAMER<<1)
+/**
+ * Tell the kernel that the batch buffer requires to disable IA-Coherency
+ */
+#define I915_EXEC_FORCE_NON_COHERENT    (1<<16)
+
+#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_FORCE_NON_COHERENT<<1)
 
 #define I915_EXEC_CONTEXT_ID_MASK	(0xffffffff)
 #define i915_execbuffer2_set_context_id(eb2, context) \
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* ✓ success: Fi.CI.BAT
  2016-01-13 10:44 [PATCH] drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ Artur Harasimiuk
@ 2016-01-13 11:49 ` Patchwork
  2016-01-13 12:53 ` [PATCH] drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ Chris Wilson
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2016-01-13 11:49 UTC (permalink / raw)
  To: Artur Harasimiuk; +Cc: intel-gfx

== Summary ==

Built on 8da57dfe6c675c35109dac986e3f8b627cffab49 drm-intel-nightly: 2016y-01m-13d-10h-33m-04s UTC integration manifest

Test gem_storedw_loop:
        Subgroup basic-render:
                pass       -> DMESG-WARN (skl-i5k-2) UNSTABLE
                dmesg-warn -> PASS       (bdw-nuci7)
                dmesg-warn -> PASS       (bdw-ultra)
Test kms_flip:
        Subgroup basic-flip-vs-dpms:
                dmesg-warn -> PASS       (ilk-hp8440p)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b:
                dmesg-warn -> PASS       (bdw-ultra)
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (snb-x220t)

bdw-nuci7        total:138  pass:129  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:138  pass:132  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:141  pass:115  dwarn:2   dfail:0   fail:0   skip:24 
hsw-brixbox      total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2          total:141  pass:137  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:141  pass:101  dwarn:3   dfail:0   fail:0   skip:37 
ivb-t430s        total:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2        total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
skl-i7k-2        total:141  pass:131  dwarn:2   dfail:0   fail:0   skip:8  
snb-dellxps      total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220t        total:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1162/

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+
  2016-01-13 10:44 [PATCH] drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ Artur Harasimiuk
  2016-01-13 11:49 ` ✓ success: Fi.CI.BAT Patchwork
@ 2016-01-13 12:53 ` Chris Wilson
  2016-01-13 14:11   ` Dave Gordon
  2016-01-15  8:00 ` [PATCH] [v2] drm/i915: Exec flag to force non IA-Coherent cache " Artur Harasimiuk
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Chris Wilson @ 2016-01-13 12:53 UTC (permalink / raw)
  To: Artur Harasimiuk; +Cc: intel-gfx

On Wed, Jan 13, 2016 at 11:44:39AM +0100, Artur Harasimiuk wrote:
> Starting from Gen9 we can use IA-Coherent caches. Coherence may be not
> required in certain cases and can be disabled in an easy way. To do this
> we can set HDC_FORCE_NON_COHERENT bit in HDC_CHICKEN0 register. This
> register is part of HW context, however it is private and cannot be
> programmed from non-privileged batch buffer.
> 
> New parameter is to override default programming and allow UMD to
> decide whether IA-Coherency is not needed for submitted batch buffer.
> When flag is set KMD emits commands to disable coherency before batch
> buffer execution starts. After execution finished state is restored.

What's the usecase for batch vs context flag?
 
> When WaForceEnableNonCoherent is programmed, it does not make sense to
> allow for coherency because this can lead to GPU hangs. In such
> situation flag is ignored.
> 
> Signed-off-by: Artur Harasimiuk <artur.harasimiuk@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_dma.c            |  3 +++
>  drivers/gpu/drm/i915/i915_drv.h            |  4 ++++
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  8 ++++++++
>  drivers/gpu/drm/i915/intel_lrc.c           | 28 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_ringbuffer.c    |  7 +++++++
>  include/uapi/drm/i915_drm.h                |  8 +++++++-
>  6 files changed, 57 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 44a896c..79ecf20 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -172,6 +172,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  	case I915_PARAM_HAS_EXEC_SOFTPIN:
>  		value = 1;
>  		break;
> +	case I915_PARAM_HAS_EXEC_FORCE_NON_COHERENT:
> +		value = INTEL_INFO(dev)->gen >= 9;

This should actually report the w/a status, or else you need to provide
some other means for userspace to detect if it can successfully force
non-coherency.

> +		break;
>  	default:
>  		DRM_DEBUG("Unknown parameter %d\n", param->param);
>  		return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 104bd18..71d739c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -886,6 +886,10 @@ struct intel_context {
>  	} engine[I915_NUM_RINGS];
>  
>  	struct list_head link;
> +
> +	struct {
> +		unsigned int WaForceEnableNonCoherent:1;
> +	} wa;
>  };
>  
>  enum fb_op_origin {
> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> index d469c47..2997a58 100644
> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> @@ -1400,6 +1400,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>  	if (!i915_gem_check_execbuffer(args))
>  		return -EINVAL;
>  
> +	if ((args->flags & I915_EXEC_FORCE_NON_COHERENT) &&
> +		INTEL_INFO(dev)->gen < 9)
> +		return -EINVAL;

It would seem easier to simply ignore this request in all situations
where it doesn't apply, rather than the current select few.

>  	ret = validate_exec_list(dev, exec, args->buffer_count);
>  	if (ret)
>  		return ret;
> @@ -1494,6 +1498,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>  
>  	i915_gem_context_reference(ctx);
>  
> +	/* Clear this flag when WA is programmed */
> +	if (ctx->wa.WaForceEnableNonCoherent)
> +		args->flags &= ~I915_EXEC_FORCE_NON_COHERENT;
> +
>  	if (ctx->ppgtt)
>  		vm = &ctx->ppgtt->base;
>  	else
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index ab344e0..4482a6a 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -879,6 +879,24 @@ int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request)
>  	return intel_logical_ring_begin(request, 0);
>  }
>  
> +static inline void
> +intel_lr_emit_force_non_coherent(struct i915_execbuffer_params *params,
> +		struct drm_i915_gem_execbuffer2 *args, bool force)
> +{
> +	if (args->flags & I915_EXEC_FORCE_NON_COHERENT) {
> +		struct intel_ringbuffer *ringbuf =
> +				params->ctx->engine[params->ring->id].ringbuf;

This should be using the request->ringbuf

> +

Missing ring_begin.

> +		intel_logical_ring_emit(ringbuf, MI_NOOP);
> +		intel_logical_ring_emit(ringbuf, MI_LOAD_REGISTER_IMM(1));
> +		intel_logical_ring_emit(ringbuf, HDC_CHICKEN0.reg);

intel_logical_ring_emit_reg(ringbuf, HDC_CHICKEN0);

> +		intel_logical_ring_emit(ringbuf, force ?
> +				_MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT) :
> +				_MASKED_BIT_DISABLE(HDC_FORCE_NON_COHERENT));
> +		intel_logical_ring_advance(ringbuf);
> +	}
> +}
> +
>  /**
>   * execlists_submission() - submit a batchbuffer for execution, Execlists style
>   * @dev: DRM device.
> @@ -959,6 +977,8 @@ int intel_execlists_submission(struct i915_execbuffer_params *params,
>  		dev_priv->relative_constants_mode = instp_mode;
>  	}
>  
> +	intel_lr_emit_force_non_coherent(params, args, true);

This is after cache invalidation. Do you need to send another cache
invalidate?

> +
>  	exec_start = params->batch_obj_vm_offset +
>  		     args->batch_start_offset;
>  
> @@ -966,6 +986,8 @@ int intel_execlists_submission(struct i915_execbuffer_params *params,
>  	if (ret)
>  		return ret;
>  
> +	intel_lr_emit_force_non_coherent(params, args, false);

You anticipate a context switching between modes between every batch?
This goes back to whether it is a context flag vs batch flag, and
whether we should cache the register value.

> +
>  	trace_i915_gem_ring_dispatch(params->request, params->dispatch_flags);
>  
>  	i915_gem_execbuffer_move_to_active(vmas, params->request);
> @@ -1112,6 +1134,9 @@ static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
>  	struct drm_device *dev = ring->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct i915_workarounds *w = &dev_priv->workarounds;
> +	struct intel_context *ctx = req->ctx;
> +
> +	ctx->wa.WaForceEnableNonCoherent = 0;
>  
>  	if (w->count == 0)
>  		return 0;
> @@ -1129,6 +1154,9 @@ static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
>  	for (i = 0; i < w->count; i++) {
>  		intel_logical_ring_emit_reg(ringbuf, w->reg[i].addr);
>  		intel_logical_ring_emit(ringbuf, w->reg[i].value);
> +		ctx->wa.WaForceEnableNonCoherent |=
> +				(w->reg[i].addr.reg == HDC_CHICKEN0.reg) &&
> +				(w->reg[i].value & HDC_FORCE_NON_COHERENT);

I don't like this second guessing of what w/a are being applied. We
could just as easily explicitly list the w/a on the engine (i.e. a
synopsis of the wa reg list, plus those setup globally) and reference
that instead of copying that flag to every context.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+
  2016-01-13 12:53 ` [PATCH] drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ Chris Wilson
@ 2016-01-13 14:11   ` Dave Gordon
  2016-01-13 16:09     ` Arun Siluvery
  0 siblings, 1 reply; 9+ messages in thread
From: Dave Gordon @ 2016-01-13 14:11 UTC (permalink / raw)
  To: intel-gfx

On 13/01/16 12:53, Chris Wilson wrote:
> On Wed, Jan 13, 2016 at 11:44:39AM +0100, Artur Harasimiuk wrote:
>> Starting from Gen9 we can use IA-Coherent caches. Coherence may be not
>> required in certain cases and can be disabled in an easy way. To do this
>> we can set HDC_FORCE_NON_COHERENT bit in HDC_CHICKEN0 register. This
>> register is part of HW context, however it is private and cannot be
>> programmed from non-privileged batch buffer.
>>
>> New parameter is to override default programming and allow UMD to
>> decide whether IA-Coherency is not needed for submitted batch buffer.
>> When flag is set KMD emits commands to disable coherency before batch
>> buffer execution starts. After execution finished state is restored.
>
> What's the usecase for batch vs context flag?
>
>> When WaForceEnableNonCoherent is programmed, it does not make sense to
>> allow for coherency because this can lead to GPU hangs. In such
>> situation flag is ignored.
>>
>> Signed-off-by: Artur Harasimiuk <artur.harasimiuk@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_dma.c            |  3 +++
>>   drivers/gpu/drm/i915/i915_drv.h            |  4 ++++
>>   drivers/gpu/drm/i915/i915_gem_execbuffer.c |  8 ++++++++
>>   drivers/gpu/drm/i915/intel_lrc.c           | 28 ++++++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/intel_ringbuffer.c    |  7 +++++++
>>   include/uapi/drm/i915_drm.h                |  8 +++++++-
>>   6 files changed, 57 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index 44a896c..79ecf20 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -172,6 +172,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
>>   	case I915_PARAM_HAS_EXEC_SOFTPIN:
>>   		value = 1;
>>   		break;
>> +	case I915_PARAM_HAS_EXEC_FORCE_NON_COHERENT:
>> +		value = INTEL_INFO(dev)->gen >= 9;
>
> This should actually report the w/a status, or else you need to provide
> some other means for userspace to detect if it can successfully force
> non-coherency.

Agreed; especially with the naming used, something called 
*Force*Something shouldn't be reported as available but then silently 
ignored on use because some workaround is in effect. If user code knows 
that it wants to *force* the use of a particular mode then it has to 
know if it doesn't take effect.

OTOH is it really "Force", or is it "Allow"? Or "Request", or "Prefer"?

>> +		break;
>>   	default:
>>   		DRM_DEBUG("Unknown parameter %d\n", param->param);
>>   		return -EINVAL;
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 104bd18..71d739c 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -886,6 +886,10 @@ struct intel_context {
>>   	} engine[I915_NUM_RINGS];
>>
>>   	struct list_head link;
>> +
>> +	struct {
>> +		unsigned int WaForceEnableNonCoherent:1;
>> +	} wa;
>>   };
>>
>>   enum fb_op_origin {
>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> index d469c47..2997a58 100644
>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>> @@ -1400,6 +1400,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>>   	if (!i915_gem_check_execbuffer(args))
>>   		return -EINVAL;
>>
>> +	if ((args->flags & I915_EXEC_FORCE_NON_COHERENT) &&
>> +		INTEL_INFO(dev)->gen < 9)
>> +		return -EINVAL;
>
> It would seem easier to simply ignore this request in all situations
> where it doesn't apply, rather than the current select few.
>
>>   	ret = validate_exec_list(dev, exec, args->buffer_count);
>>   	if (ret)
>>   		return ret;
>> @@ -1494,6 +1498,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
>>
>>   	i915_gem_context_reference(ctx);
>>
>> +	/* Clear this flag when WA is programmed */
>> +	if (ctx->wa.WaForceEnableNonCoherent)
>> +		args->flags &= ~I915_EXEC_FORCE_NON_COHERENT;
>> +

This seems back-to-front (I'm only looking at the names, without 
checking what this bit really does), but surely if the 
"ForceEnableNonCoherent" workaround is in effect it should force the 
"FORCE_NON_COHERENT" flag ON, not OFF?

Also, the use of a per-batch bool doesn't seem to leave any don't-care 
option. Is there any use case where the user code absolutely *doesn't* 
want this non-coherency? Or which of these are useful:

1.	Require non-coherent, fail if not possible
2.	Request non-coherent, warn me but continue if not possible
3.	Prefer non-coherent, ignore failure (don't even tell me)
4.	Don't care, use system default
5.	Prefer coherent, ignore failure (don't tell me)
6.	Request coherent, warn me (but continue) if not possible
7.	Require coherent, fail if not possible

Then we just need to make sure the names mean what they would appear to.

.Dave.

>>   	if (ctx->ppgtt)
>>   		vm = &ctx->ppgtt->base;
>>   	else
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> index ab344e0..4482a6a 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -879,6 +879,24 @@ int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request)
>>   	return intel_logical_ring_begin(request, 0);
>>   }
>>
>> +static inline void
>> +intel_lr_emit_force_non_coherent(struct i915_execbuffer_params *params,
>> +		struct drm_i915_gem_execbuffer2 *args, bool force)
>> +{
>> +	if (args->flags & I915_EXEC_FORCE_NON_COHERENT) {
>> +		struct intel_ringbuffer *ringbuf =
>> +				params->ctx->engine[params->ring->id].ringbuf;
>
> This should be using the request->ringbuf
>
>> +
>
> Missing ring_begin.
>
>> +		intel_logical_ring_emit(ringbuf, MI_NOOP);
>> +		intel_logical_ring_emit(ringbuf, MI_LOAD_REGISTER_IMM(1));
>> +		intel_logical_ring_emit(ringbuf, HDC_CHICKEN0.reg);
>
> intel_logical_ring_emit_reg(ringbuf, HDC_CHICKEN0);
>
>> +		intel_logical_ring_emit(ringbuf, force ?
>> +				_MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT) :
>> +				_MASKED_BIT_DISABLE(HDC_FORCE_NON_COHERENT));
>> +		intel_logical_ring_advance(ringbuf);
>> +	}
>> +}
>> +
>>   /**
>>    * execlists_submission() - submit a batchbuffer for execution, Execlists style
>>    * @dev: DRM device.
>> @@ -959,6 +977,8 @@ int intel_execlists_submission(struct i915_execbuffer_params *params,
>>   		dev_priv->relative_constants_mode = instp_mode;
>>   	}
>>
>> +	intel_lr_emit_force_non_coherent(params, args, true);
>
> This is after cache invalidation. Do you need to send another cache
> invalidate?
>
>> +
>>   	exec_start = params->batch_obj_vm_offset +
>>   		     args->batch_start_offset;
>>
>> @@ -966,6 +986,8 @@ int intel_execlists_submission(struct i915_execbuffer_params *params,
>>   	if (ret)
>>   		return ret;
>>
>> +	intel_lr_emit_force_non_coherent(params, args, false);
>
> You anticipate a context switching between modes between every batch?
> This goes back to whether it is a context flag vs batch flag, and
> whether we should cache the register value.
>
>> +
>>   	trace_i915_gem_ring_dispatch(params->request, params->dispatch_flags);
>>
>>   	i915_gem_execbuffer_move_to_active(vmas, params->request);
>> @@ -1112,6 +1134,9 @@ static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
>>   	struct drm_device *dev = ring->dev;
>>   	struct drm_i915_private *dev_priv = dev->dev_private;
>>   	struct i915_workarounds *w = &dev_priv->workarounds;
>> +	struct intel_context *ctx = req->ctx;
>> +
>> +	ctx->wa.WaForceEnableNonCoherent = 0;
>>
>>   	if (w->count == 0)
>>   		return 0;
>> @@ -1129,6 +1154,9 @@ static int intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
>>   	for (i = 0; i < w->count; i++) {
>>   		intel_logical_ring_emit_reg(ringbuf, w->reg[i].addr);
>>   		intel_logical_ring_emit(ringbuf, w->reg[i].value);
>> +		ctx->wa.WaForceEnableNonCoherent |=
>> +				(w->reg[i].addr.reg == HDC_CHICKEN0.reg) &&
>> +				(w->reg[i].value & HDC_FORCE_NON_COHERENT);
>
> I don't like this second guessing of what w/a are being applied. We
> could just as easily explicitly list the w/a on the engine (i.e. a
> synopsis of the wa reg list, plus those setup globally) and reference
> that instead of copying that flag to every context.
> -Chris
>

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+
  2016-01-13 14:11   ` Dave Gordon
@ 2016-01-13 16:09     ` Arun Siluvery
  2016-01-13 17:41       ` Harasimiuk, Artur
  0 siblings, 1 reply; 9+ messages in thread
From: Arun Siluvery @ 2016-01-13 16:09 UTC (permalink / raw)
  To: Dave Gordon, intel-gfx

On 13/01/2016 14:11, Dave Gordon wrote:
> On 13/01/16 12:53, Chris Wilson wrote:
>> On Wed, Jan 13, 2016 at 11:44:39AM +0100, Artur Harasimiuk wrote:
>>> Starting from Gen9 we can use IA-Coherent caches. Coherence may be not
>>> required in certain cases and can be disabled in an easy way. To do this
>>> we can set HDC_FORCE_NON_COHERENT bit in HDC_CHICKEN0 register. This
>>> register is part of HW context, however it is private and cannot be
>>> programmed from non-privileged batch buffer.
>>>
Looks like what you need is to add this register to HW whitelist so that 
UMD can program required behaviour between batches but you need strong 
justification and approval to whitelist a register. Is there a real 
usecase or this is only for debug?

regards
Arun

>>> New parameter is to override default programming and allow UMD to
>>> decide whether IA-Coherency is not needed for submitted batch buffer.
>>> When flag is set KMD emits commands to disable coherency before batch
>>> buffer execution starts. After execution finished state is restored.
>>
>> What's the usecase for batch vs context flag?
>>
>>> When WaForceEnableNonCoherent is programmed, it does not make sense to
>>> allow for coherency because this can lead to GPU hangs. In such
>>> situation flag is ignored.
>>>
>>> Signed-off-by: Artur Harasimiuk <artur.harasimiuk@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_dma.c            |  3 +++
>>>   drivers/gpu/drm/i915/i915_drv.h            |  4 ++++
>>>   drivers/gpu/drm/i915/i915_gem_execbuffer.c |  8 ++++++++
>>>   drivers/gpu/drm/i915/intel_lrc.c           | 28
>>> ++++++++++++++++++++++++++++
>>>   drivers/gpu/drm/i915/intel_ringbuffer.c    |  7 +++++++
>>>   include/uapi/drm/i915_drm.h                |  8 +++++++-
>>>   6 files changed, 57 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_dma.c
>>> b/drivers/gpu/drm/i915/i915_dma.c
>>> index 44a896c..79ecf20 100644
>>> --- a/drivers/gpu/drm/i915/i915_dma.c
>>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>>> @@ -172,6 +172,9 @@ static int i915_getparam(struct drm_device *dev,
>>> void *data,
>>>       case I915_PARAM_HAS_EXEC_SOFTPIN:
>>>           value = 1;
>>>           break;
>>> +    case I915_PARAM_HAS_EXEC_FORCE_NON_COHERENT:
>>> +        value = INTEL_INFO(dev)->gen >= 9;
>>
>> This should actually report the w/a status, or else you need to provide
>> some other means for userspace to detect if it can successfully force
>> non-coherency.
>
> Agreed; especially with the naming used, something called
> *Force*Something shouldn't be reported as available but then silently
> ignored on use because some workaround is in effect. If user code knows
> that it wants to *force* the use of a particular mode then it has to
> know if it doesn't take effect.
>
> OTOH is it really "Force", or is it "Allow"? Or "Request", or "Prefer"?
>
>>> +        break;
>>>       default:
>>>           DRM_DEBUG("Unknown parameter %d\n", param->param);
>>>           return -EINVAL;
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>>> b/drivers/gpu/drm/i915/i915_drv.h
>>> index 104bd18..71d739c 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -886,6 +886,10 @@ struct intel_context {
>>>       } engine[I915_NUM_RINGS];
>>>
>>>       struct list_head link;
>>> +
>>> +    struct {
>>> +        unsigned int WaForceEnableNonCoherent:1;
>>> +    } wa;
>>>   };
>>>
>>>   enum fb_op_origin {
>>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>> index d469c47..2997a58 100644
>>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
>>> @@ -1400,6 +1400,10 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>>> void *data,
>>>       if (!i915_gem_check_execbuffer(args))
>>>           return -EINVAL;
>>>
>>> +    if ((args->flags & I915_EXEC_FORCE_NON_COHERENT) &&
>>> +        INTEL_INFO(dev)->gen < 9)
>>> +        return -EINVAL;
>>
>> It would seem easier to simply ignore this request in all situations
>> where it doesn't apply, rather than the current select few.
>>
>>>       ret = validate_exec_list(dev, exec, args->buffer_count);
>>>       if (ret)
>>>           return ret;
>>> @@ -1494,6 +1498,10 @@ i915_gem_do_execbuffer(struct drm_device *dev,
>>> void *data,
>>>
>>>       i915_gem_context_reference(ctx);
>>>
>>> +    /* Clear this flag when WA is programmed */
>>> +    if (ctx->wa.WaForceEnableNonCoherent)
>>> +        args->flags &= ~I915_EXEC_FORCE_NON_COHERENT;
>>> +
>
> This seems back-to-front (I'm only looking at the names, without
> checking what this bit really does), but surely if the
> "ForceEnableNonCoherent" workaround is in effect it should force the
> "FORCE_NON_COHERENT" flag ON, not OFF?
>
> Also, the use of a per-batch bool doesn't seem to leave any don't-care
> option. Is there any use case where the user code absolutely *doesn't*
> want this non-coherency? Or which of these are useful:
>
> 1.    Require non-coherent, fail if not possible
> 2.    Request non-coherent, warn me but continue if not possible
> 3.    Prefer non-coherent, ignore failure (don't even tell me)
> 4.    Don't care, use system default
> 5.    Prefer coherent, ignore failure (don't tell me)
> 6.    Request coherent, warn me (but continue) if not possible
> 7.    Require coherent, fail if not possible
>
> Then we just need to make sure the names mean what they would appear to.
>
> .Dave.
>
>>>       if (ctx->ppgtt)
>>>           vm = &ctx->ppgtt->base;
>>>       else
>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
>>> b/drivers/gpu/drm/i915/intel_lrc.c
>>> index ab344e0..4482a6a 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -879,6 +879,24 @@ int intel_logical_ring_reserve_space(struct
>>> drm_i915_gem_request *request)
>>>       return intel_logical_ring_begin(request, 0);
>>>   }
>>>
>>> +static inline void
>>> +intel_lr_emit_force_non_coherent(struct i915_execbuffer_params *params,
>>> +        struct drm_i915_gem_execbuffer2 *args, bool force)
>>> +{
>>> +    if (args->flags & I915_EXEC_FORCE_NON_COHERENT) {
>>> +        struct intel_ringbuffer *ringbuf =
>>> +                params->ctx->engine[params->ring->id].ringbuf;
>>
>> This should be using the request->ringbuf
>>
>>> +
>>
>> Missing ring_begin.
>>
>>> +        intel_logical_ring_emit(ringbuf, MI_NOOP);
>>> +        intel_logical_ring_emit(ringbuf, MI_LOAD_REGISTER_IMM(1));
>>> +        intel_logical_ring_emit(ringbuf, HDC_CHICKEN0.reg);
>>
>> intel_logical_ring_emit_reg(ringbuf, HDC_CHICKEN0);
>>
>>> +        intel_logical_ring_emit(ringbuf, force ?
>>> +                _MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT) :
>>> +                _MASKED_BIT_DISABLE(HDC_FORCE_NON_COHERENT));
>>> +        intel_logical_ring_advance(ringbuf);
>>> +    }
>>> +}
>>> +
>>>   /**
>>>    * execlists_submission() - submit a batchbuffer for execution,
>>> Execlists style
>>>    * @dev: DRM device.
>>> @@ -959,6 +977,8 @@ int intel_execlists_submission(struct
>>> i915_execbuffer_params *params,
>>>           dev_priv->relative_constants_mode = instp_mode;
>>>       }
>>>
>>> +    intel_lr_emit_force_non_coherent(params, args, true);
>>
>> This is after cache invalidation. Do you need to send another cache
>> invalidate?
>>
>>> +
>>>       exec_start = params->batch_obj_vm_offset +
>>>                args->batch_start_offset;
>>>
>>> @@ -966,6 +986,8 @@ int intel_execlists_submission(struct
>>> i915_execbuffer_params *params,
>>>       if (ret)
>>>           return ret;
>>>
>>> +    intel_lr_emit_force_non_coherent(params, args, false);
>>
>> You anticipate a context switching between modes between every batch?
>> This goes back to whether it is a context flag vs batch flag, and
>> whether we should cache the register value.
>>
>>> +
>>>       trace_i915_gem_ring_dispatch(params->request,
>>> params->dispatch_flags);
>>>
>>>       i915_gem_execbuffer_move_to_active(vmas, params->request);
>>> @@ -1112,6 +1134,9 @@ static int
>>> intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
>>>       struct drm_device *dev = ring->dev;
>>>       struct drm_i915_private *dev_priv = dev->dev_private;
>>>       struct i915_workarounds *w = &dev_priv->workarounds;
>>> +    struct intel_context *ctx = req->ctx;
>>> +
>>> +    ctx->wa.WaForceEnableNonCoherent = 0;
>>>
>>>       if (w->count == 0)
>>>           return 0;
>>> @@ -1129,6 +1154,9 @@ static int
>>> intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
>>>       for (i = 0; i < w->count; i++) {
>>>           intel_logical_ring_emit_reg(ringbuf, w->reg[i].addr);
>>>           intel_logical_ring_emit(ringbuf, w->reg[i].value);
>>> +        ctx->wa.WaForceEnableNonCoherent |=
>>> +                (w->reg[i].addr.reg == HDC_CHICKEN0.reg) &&
>>> +                (w->reg[i].value & HDC_FORCE_NON_COHERENT);
>>
>> I don't like this second guessing of what w/a are being applied. We
>> could just as easily explicitly list the w/a on the engine (i.e. a
>> synopsis of the wa reg list, plus those setup globally) and reference
>> that instead of copying that flag to every context.
>> -Chris
>>
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+
  2016-01-13 16:09     ` Arun Siluvery
@ 2016-01-13 17:41       ` Harasimiuk, Artur
  0 siblings, 0 replies; 9+ messages in thread
From: Harasimiuk, Artur @ 2016-01-13 17:41 UTC (permalink / raw)
  To: intel-gfx

> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Arun Siluvery
> Sent: Wednesday, January 13, 2016 5:09 PM
> To: Gordon, David S <david.s.gordon@intel.com>; intel-
> gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: Expose exec parameter to force
> non IA-Coherent for Gen9+
> 
> On 13/01/2016 14:11, Dave Gordon wrote:
> > On 13/01/16 12:53, Chris Wilson wrote:
> >> On Wed, Jan 13, 2016 at 11:44:39AM +0100, Artur Harasimiuk wrote:
> >>> Starting from Gen9 we can use IA-Coherent caches. Coherence may be
> >>> not required in certain cases and can be disabled in an easy way. To
> >>> do this we can set HDC_FORCE_NON_COHERENT bit in HDC_CHICKEN0
> >>> register. This register is part of HW context, however it is private
> >>> and cannot be programmed from non-privileged batch buffer.
> >>>
> Looks like what you need is to add this register to HW whitelist so that
> UMD can program required behaviour between batches but you need strong
> justification and approval to whitelist a register. Is there a real
> usecase or this is only for debug?
> 
  Whitelisting was already refused.
 
> regards
> Arun
> 
> >>> New parameter is to override default programming and allow UMD to
> >>> decide whether IA-Coherency is not needed for submitted batch buffer.
> >>> When flag is set KMD emits commands to disable coherency before
> >>> batch buffer execution starts. After execution finished state is
> restored.
> >>
> >> What's the usecase for batch vs context flag?
>

In overall coherency can be programmed using RENDER_SURFACE_STATE (for statefull memory accesses) or using BTI.255 for stateless (see PRM: Data Cache Coherency). When HDC_FORCE_NON_COHERENT bit is clear, then all accesses are processed as described. By setting HDC_FORCE_NON_COHERENT to one we *Force* data port to ignore these attributes and caches are GPU-Coherent. 
However with stateless access mode IA-Coherency control is complicated. It would require to have ISA for each case, depending on required coherency for different surfaces. This requires ISA 'send' instruction with BTI.253 and BTI.255 mixed. 
Moreover, IA-Coherency requires additional effort from HW. Increased LLC usage or extra snooping for non-LLC. This costs additional power and causes performance penalties. So there is no reason to be always IA-Coherent.

Per batch flags allows user space to select if single batch requires caches to be non-IA-Coherent. In effect, in single HW context we can work in both modes and have performance gain. 
Setting flag for context would mean that user space want to be non-IA-Coherent. For single batch which may require IA-Coherency, we need switch context back to IA-Coherent mode. 
User space can have two contexts, for GPU-Coherent and IA-Coherent, and use one of them during batch buffer submission. But this means context switching which is overkill in this case, I think.

I did per-batch flag, because it seems to be simpler and minimizes risk of making bug. This allow to keep state of workaround.

> >>
> >>> When WaForceEnableNonCoherent is programmed, it does not make sense
> >>> to allow for coherency because this can lead to GPU hangs. In such
> >>> situation flag is ignored.
> >>>
> >>> Signed-off-by: Artur Harasimiuk <artur.harasimiuk@intel.com>
> >>> ---
> >>>   drivers/gpu/drm/i915/i915_dma.c            |  3 +++
> >>>   drivers/gpu/drm/i915/i915_drv.h            |  4 ++++
> >>>   drivers/gpu/drm/i915/i915_gem_execbuffer.c |  8 ++++++++
> >>>   drivers/gpu/drm/i915/intel_lrc.c           | 28
> >>> ++++++++++++++++++++++++++++
> >>>   drivers/gpu/drm/i915/intel_ringbuffer.c    |  7 +++++++
> >>>   include/uapi/drm/i915_drm.h                |  8 +++++++-
> >>>   6 files changed, 57 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_dma.c
> >>> b/drivers/gpu/drm/i915/i915_dma.c index 44a896c..79ecf20 100644
> >>> --- a/drivers/gpu/drm/i915/i915_dma.c
> >>> +++ b/drivers/gpu/drm/i915/i915_dma.c
> >>> @@ -172,6 +172,9 @@ static int i915_getparam(struct drm_device *dev,
> >>> void *data,
> >>>       case I915_PARAM_HAS_EXEC_SOFTPIN:
> >>>           value = 1;
> >>>           break;
> >>> +    case I915_PARAM_HAS_EXEC_FORCE_NON_COHERENT:
> >>> +        value = INTEL_INFO(dev)->gen >= 9;
> >>
> >> This should actually report the w/a status, or else you need to
> >> provide some other means for userspace to detect if it can
> >> successfully force non-coherency.
> >

It sounds like, we need to have way to pass w/a status from kernel to user Space?

This parameter is to tell user space if it can request to disable IA-Coherency (Force). It tells user space about kernel capability.
 
> > Agreed; especially with the naming used, something called
> > *Force*Something shouldn't be reported as available but then silently
> > ignored on use because some workaround is in effect. If user code
> > knows that it wants to *force* the use of a particular mode then it
> > has to know if it doesn't take effect.
> >
> > OTOH is it really "Force", or is it "Allow"? Or "Request", or "Prefer"?
> >

This means exactly "Force". It is equal to setting HDC_FORCE_NON_COHERENT bit.

> >>> +        break;
> >>>       default:
> >>>           DRM_DEBUG("Unknown parameter %d\n", param->param);
> >>>           return -EINVAL;
> >>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> >>> b/drivers/gpu/drm/i915/i915_drv.h index 104bd18..71d739c 100644
> >>> --- a/drivers/gpu/drm/i915/i915_drv.h
> >>> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >>> @@ -886,6 +886,10 @@ struct intel_context {
> >>>       } engine[I915_NUM_RINGS];
> >>>
> >>>       struct list_head link;
> >>> +
> >>> +    struct {
> >>> +        unsigned int WaForceEnableNonCoherent:1;
> >>> +    } wa;
> >>>   };
> >>>
> >>>   enum fb_op_origin {
> >>> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >>> b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >>> index d469c47..2997a58 100644
> >>> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >>> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
> >>> @@ -1400,6 +1400,10 @@ i915_gem_do_execbuffer(struct drm_device
> >>> *dev, void *data,
> >>>       if (!i915_gem_check_execbuffer(args))
> >>>           return -EINVAL;
> >>>
> >>> +    if ((args->flags & I915_EXEC_FORCE_NON_COHERENT) &&
> >>> +        INTEL_INFO(dev)->gen < 9)
> >>> +        return -EINVAL;
> >>
> >> It would seem easier to simply ignore this request in all situations
> >> where it doesn't apply, rather than the current select few.

I'm not sure. Request to disable coherency for non gen9+ would mean:
1. bad user mode programming (did not checked I915_PARAM_HAS_EXEC_FORCE_NON_COHERENT). In this case we can skip flag.
2. out of sync with numbers - user mode thinks this is different flag.
3. ...

> >>
> >>>       ret = validate_exec_list(dev, exec, args->buffer_count);
> >>>       if (ret)
> >>>           return ret;
> >>> @@ -1494,6 +1498,10 @@ i915_gem_do_execbuffer(struct drm_device
> >>> *dev, void *data,
> >>>
> >>>       i915_gem_context_reference(ctx);
> >>>
> >>> +    /* Clear this flag when WA is programmed */
> >>> +    if (ctx->wa.WaForceEnableNonCoherent)
> >>> +        args->flags &= ~I915_EXEC_FORCE_NON_COHERENT;
> >>> +
> >
> > This seems back-to-front (I'm only looking at the names, without
> > checking what this bit really does), but surely if the
> > "ForceEnableNonCoherent" workaround is in effect it should force the
> > "FORCE_NON_COHERENT" flag ON, not OFF?
> >

When WaForceEnableNonCoherent is in action, this means HDC_FORCE_NON_COHERENT bit is set to one. This workaround is for possible GPU hangs. So, for devices requiring this workaround, we should keep this always set - do not allow to be IA-Coherent. By clearing this flag we keep code consistent.

> > Also, the use of a per-batch bool doesn't seem to leave any don't-care
> > option. Is there any use case where the user code absolutely *doesn't*
> > want this non-coherency? Or which of these are useful:
> >
> > 1.    Require non-coherent, fail if not possible
> > 2.    Request non-coherent, warn me but continue if not possible
> > 3.    Prefer non-coherent, ignore failure (don't even tell me)
> > 4.    Don't care, use system default
> > 5.    Prefer coherent, ignore failure (don't tell me)
> > 6.    Request coherent, warn me (but continue) if not possible
> > 7.    Require coherent, fail if not possible
> >

We don't need don't-care scenario, IMO. I think flags meaning is quite simple:
1. When flag is not set, this means use system default. No behavior change.
2. When set 
   - disable IA-Coherency when enabled (wa not programmed)
   - do nothing, because coherency is already disabled (wa programmed)
Looks like this should be in commit message.

> > Then we just need to make sure the names mean what they would appear to.

I agree. I will update commit message to better describe things depending on further input.

> >
> > .Dave.
> >
> >>>       if (ctx->ppgtt)
> >>>           vm = &ctx->ppgtt->base;
> >>>       else
> >>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c
> >>> b/drivers/gpu/drm/i915/intel_lrc.c
> >>> index ab344e0..4482a6a 100644
> >>> --- a/drivers/gpu/drm/i915/intel_lrc.c
> >>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> >>> @@ -879,6 +879,24 @@ int intel_logical_ring_reserve_space(struct
> >>> drm_i915_gem_request *request)
> >>>       return intel_logical_ring_begin(request, 0);
> >>>   }
> >>>
> >>> +static inline void
> >>> +intel_lr_emit_force_non_coherent(struct i915_execbuffer_params
> *params,
> >>> +        struct drm_i915_gem_execbuffer2 *args, bool force) {
> >>> +    if (args->flags & I915_EXEC_FORCE_NON_COHERENT) {
> >>> +        struct intel_ringbuffer *ringbuf =
> >>> +                params->ctx->engine[params->ring->id].ringbuf;
> >>
> >> This should be using the request->ringbuf
> >>
> >>> +
> >>
> >> Missing ring_begin.
> >>
> >>> +        intel_logical_ring_emit(ringbuf, MI_NOOP);
> >>> +        intel_logical_ring_emit(ringbuf, MI_LOAD_REGISTER_IMM(1));
> >>> +        intel_logical_ring_emit(ringbuf, HDC_CHICKEN0.reg);
> >>
> >> intel_logical_ring_emit_reg(ringbuf, HDC_CHICKEN0);
> >>
> >>> +        intel_logical_ring_emit(ringbuf, force ?
> >>> +                _MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT) :
> >>> +                _MASKED_BIT_DISABLE(HDC_FORCE_NON_COHERENT));
> >>> +        intel_logical_ring_advance(ringbuf);
> >>> +    }
> >>> +}
> >>> +
> >>>   /**
> >>>    * execlists_submission() - submit a batchbuffer for execution,
> >>> Execlists style
> >>>    * @dev: DRM device.
> >>> @@ -959,6 +977,8 @@ int intel_execlists_submission(struct
> >>> i915_execbuffer_params *params,
> >>>           dev_priv->relative_constants_mode = instp_mode;
> >>>       }
> >>>
> >>> +    intel_lr_emit_force_non_coherent(params, args, true);
> >>
> >> This is after cache invalidation. Do you need to send another cache
> >> invalidate?
> >>

We do not need this. 

> >>> +
> >>>       exec_start = params->batch_obj_vm_offset +
> >>>                args->batch_start_offset;
> >>>
> >>> @@ -966,6 +986,8 @@ int intel_execlists_submission(struct
> >>> i915_execbuffer_params *params,
> >>>       if (ret)
> >>>           return ret;
> >>>
> >>> +    intel_lr_emit_force_non_coherent(params, args, false);
> >>
> >> You anticipate a context switching between modes between every batch?
> >> This goes back to whether it is a context flag vs batch flag, and
> >> whether we should cache the register value.
> >>

Because this is batch flag, we do not need any context switching. 
We don't need to cache register value because flag is cleared if workaround is programmed (@@ -1494,6 +1498,10 @@). This way we do not allow to clear HDC_FORCE_NON_COHERENT bit when workaround is required.

> >>> +
> >>>       trace_i915_gem_ring_dispatch(params->request,
> >>> params->dispatch_flags);
> >>>
> >>>       i915_gem_execbuffer_move_to_active(vmas, params->request); @@
> >>> -1112,6 +1134,9 @@ static int
> >>> intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
> >>>       struct drm_device *dev = ring->dev;
> >>>       struct drm_i915_private *dev_priv = dev->dev_private;
> >>>       struct i915_workarounds *w = &dev_priv->workarounds;
> >>> +    struct intel_context *ctx = req->ctx;
> >>> +
> >>> +    ctx->wa.WaForceEnableNonCoherent = 0;
> >>>
> >>>       if (w->count == 0)
> >>>           return 0;
> >>> @@ -1129,6 +1154,9 @@ static int
> >>> intel_logical_ring_workarounds_emit(struct drm_i915_gem_request *req)
> >>>       for (i = 0; i < w->count; i++) {
> >>>           intel_logical_ring_emit_reg(ringbuf, w->reg[i].addr);
> >>>           intel_logical_ring_emit(ringbuf, w->reg[i].value);
> >>> +        ctx->wa.WaForceEnableNonCoherent |=
> >>> +                (w->reg[i].addr.reg == HDC_CHICKEN0.reg) &&
> >>> +                (w->reg[i].value & HDC_FORCE_NON_COHERENT);
> >>
> >> I don't like this second guessing of what w/a are being applied. We
> >> could just as easily explicitly list the w/a on the engine (i.e. a
> >> synopsis of the wa reg list, plus those setup globally) and reference
> >> that instead of copying that flag to every context.
> >> -Chris
> >>
>

Right. Will re-work patch to keep information in one place.


Thanks,
Artur

> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] [v2] drm/i915: Exec flag to force non IA-Coherent cache for Gen9+
  2016-01-13 10:44 [PATCH] drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ Artur Harasimiuk
  2016-01-13 11:49 ` ✓ success: Fi.CI.BAT Patchwork
  2016-01-13 12:53 ` [PATCH] drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ Chris Wilson
@ 2016-01-15  8:00 ` Artur Harasimiuk
  2016-01-15  8:20 ` ✗ Fi.CI.BAT: failure for drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ (rev2) Patchwork
  2020-02-06  1:10 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ Patchwork
  4 siblings, 0 replies; 9+ messages in thread
From: Artur Harasimiuk @ 2016-01-15  8:00 UTC (permalink / raw)
  To: intel-gfx

Starting from Gen9 we can use IA-Coherent caches. Generally, coherency
can be programmed using RENDER_SURFACE_STATE or BTI 255, depending if
surface state model or stateless model is used. It is important to control
whether IA or GPU cache coherency should be used, especially for non-LLC
devices. However this control is complicated when stateless memory access
model is in action. It would require dedicated ISA code depending on
coherency requirement.

By setting HDC_FORCE_NON_COHERENT we *Force* data port to ignore these
attributes and all caches are GPU-Coherent. This register is part of HW
context, however it is private and cannot be programmed from
non-privileged batch buffer.

Default operation mode is as programmed by workaround. When
WaForceEnableNonCoherent is in place caches are GPU-Coherent and we
should not change it back to IA-Coherent because this can lead to GPU
hangs (as workaround description says).

A new device parameter is to inform user space about kernel capability.
It tells if can request to disable IA-Coherency.

Exec flag is to allow UMD to decide whether IA-Coherency is not needed
for submitted batch buffer. Exec flag behavior:
    1. flag is not set - use system default
    2. flag is set but WaForceEnableNonCoherent is
       a) not programmed - *Force* GPU-Coherent cache by setting
          HDC_FORCE_NON_COHERENT prior to bb_start and clearing after
       b) programmed - do nothing, GPU-Coherent is already in place

v2: Ringbufer handling fixes (Chris)
    Moved workarounds to common place (Chris)
    Removed flag cleanup (Dave)
    Updated commit message to reflect comments (Chris,Dave)

Signed-off-by: Artur Harasimiuk <artur.harasimiuk@intel.com>
---
 drivers/gpu/drm/i915/i915_dma.c            |  4 ++++
 drivers/gpu/drm/i915/i915_drv.h            |  4 ++++
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  4 ++++
 drivers/gpu/drm/i915/intel_lrc.c           | 38 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.c    |  2 ++
 include/uapi/drm/i915_drm.h                |  8 ++++++-
 6 files changed, 59 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 44a896c..f735e56 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -172,6 +172,10 @@ static int i915_getparam(struct drm_device *dev, void *data,
 	case I915_PARAM_HAS_EXEC_SOFTPIN:
 		value = 1;
 		break;
+	case I915_PARAM_HAS_EXEC_FORCE_NON_COHERENT:
+		value = !dev_priv->workarounds.WaForceEnableNonCoherent &&
+			INTEL_INFO(dev)->gen >= 9;
+		break;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 104bd18..793be854 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1658,6 +1658,10 @@ struct i915_wa_reg {
 struct i915_workarounds {
 	struct i915_wa_reg reg[I915_MAX_WA_REGS];
 	u32 count;
+
+	struct {
+		unsigned int WaForceEnableNonCoherent:1;
+	};
 };
 
 struct i915_virtual_gpu {
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index d469c47..5db3806 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1400,6 +1400,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
 	if (!i915_gem_check_execbuffer(args))
 		return -EINVAL;
 
+	if ((args->flags & I915_EXEC_FORCE_NON_COHERENT) &&
+		INTEL_INFO(dev)->gen < 9)
+		return -EINVAL;
+
 	ret = validate_exec_list(dev, exec, args->buffer_count);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ab344e0..f37d12f 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -879,6 +879,36 @@ int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request)
 	return intel_logical_ring_begin(request, 0);
 }
 
+static inline int
+intel_lr_emit_force_non_coherent(struct i915_execbuffer_params *params,
+		struct drm_i915_gem_execbuffer2 *args, bool force)
+{
+	struct drm_device       *dev = params->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	int ret;
+
+	if (dev_priv->workarounds.WaForceEnableNonCoherent)
+		return 0;
+
+	if (args->flags & I915_EXEC_FORCE_NON_COHERENT) {
+		struct intel_ringbuffer *ringbuf = params->request->ringbuf;
+
+		ret = intel_logical_ring_begin(params->request, 4);
+		if (ret)
+			return ret;
+
+		intel_logical_ring_emit(ringbuf, MI_NOOP);
+		intel_logical_ring_emit(ringbuf, MI_LOAD_REGISTER_IMM(1));
+		intel_logical_ring_emit(ringbuf, HDC_CHICKEN0.reg);
+		intel_logical_ring_emit(ringbuf, force ?
+				_MASKED_BIT_ENABLE(HDC_FORCE_NON_COHERENT) :
+				_MASKED_BIT_DISABLE(HDC_FORCE_NON_COHERENT));
+		intel_logical_ring_advance(ringbuf);
+	}
+
+	return 0;
+}
+
 /**
  * execlists_submission() - submit a batchbuffer for execution, Execlists style
  * @dev: DRM device.
@@ -959,6 +989,10 @@ int intel_execlists_submission(struct i915_execbuffer_params *params,
 		dev_priv->relative_constants_mode = instp_mode;
 	}
 
+	ret = intel_lr_emit_force_non_coherent(params, args, true);
+	if (ret)
+		return ret;
+
 	exec_start = params->batch_obj_vm_offset +
 		     args->batch_start_offset;
 
@@ -966,6 +1000,10 @@ int intel_execlists_submission(struct i915_execbuffer_params *params,
 	if (ret)
 		return ret;
 
+	ret = intel_lr_emit_force_non_coherent(params, args, false);
+	if (ret)
+		return ret;
+
 	trace_i915_gem_ring_dispatch(params->request, params->dispatch_flags);
 
 	i915_gem_execbuffer_move_to_active(vmas, params->request);
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 4060acf..456421e 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -806,6 +806,7 @@ static int gen8_init_workarounds(struct intel_engine_cs *ring)
 	 * invalidation occurs during a PSD flush.
 	 */
 	/* WaForceEnableNonCoherent:bdw,chv */
+	dev_priv->workarounds.WaForceEnableNonCoherent = 1;
 	/* WaHdcDisableFetchWhenMasked:bdw,chv */
 	WA_SET_BIT_MASKED(HDC_CHICKEN0,
 			  HDC_DONOT_FETCH_MEM_WHEN_MASKED |
@@ -1049,6 +1050,7 @@ static int skl_init_workarounds(struct intel_engine_cs *ring)
 		 * a TLB invalidation occurs during a PSD flush.
 		 */
 		/* WaForceEnableNonCoherent:skl */
+		dev_priv->workarounds.WaForceEnableNonCoherent = 1;
 		WA_SET_BIT_MASKED(HDC_CHICKEN0,
 				  HDC_FORCE_NON_COHERENT);
 
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index acf2102..c425e80 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -357,6 +357,7 @@ typedef struct drm_i915_irq_wait {
 #define I915_PARAM_HAS_GPU_RESET	 35
 #define I915_PARAM_HAS_RESOURCE_STREAMER 36
 #define I915_PARAM_HAS_EXEC_SOFTPIN	 37
+#define I915_PARAM_HAS_EXEC_FORCE_NON_COHERENT 38
 
 typedef struct drm_i915_getparam {
 	__s32 param;
@@ -782,7 +783,12 @@ struct drm_i915_gem_execbuffer2 {
  */
 #define I915_EXEC_RESOURCE_STREAMER     (1<<15)
 
-#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_RESOURCE_STREAMER<<1)
+/**
+ * Tell the kernel that the batch buffer requires to disable IA-Coherency
+ */
+#define I915_EXEC_FORCE_NON_COHERENT    (1<<16)
+
+#define __I915_EXEC_UNKNOWN_FLAGS -(I915_EXEC_FORCE_NON_COHERENT<<1)
 
 #define I915_EXEC_CONTEXT_ID_MASK	(0xffffffff)
 #define i915_execbuffer2_set_context_id(eb2, context) \
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ (rev2)
  2016-01-13 10:44 [PATCH] drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ Artur Harasimiuk
                   ` (2 preceding siblings ...)
  2016-01-15  8:00 ` [PATCH] [v2] drm/i915: Exec flag to force non IA-Coherent cache " Artur Harasimiuk
@ 2016-01-15  8:20 ` Patchwork
  2020-02-06  1:10 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ Patchwork
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2016-01-15  8:20 UTC (permalink / raw)
  To: Artur Harasimiuk; +Cc: intel-gfx

== Summary ==

Built on 4efb7ea717f8478860c4cb829fc0956f195c0b50 drm-intel-nightly: 2016y-01m-15d-02h-35m-27s UTC integration manifest

Test gem_ctx_basic:
                pass       -> FAIL       (bdw-ultra)
Test gem_storedw_loop:
        Subgroup basic-render:
                dmesg-warn -> PASS       (skl-i5k-2) UNSTABLE
                pass       -> DMESG-WARN (bdw-nuci7)
                dmesg-warn -> PASS       (skl-i7k-2) UNSTABLE
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (byt-nuc)

bdw-nuci7        total:138  pass:128  dwarn:1   dfail:0   fail:0   skip:9  
bdw-ultra        total:138  pass:131  dwarn:0   dfail:0   fail:1   skip:6  
byt-nuc          total:141  pass:121  dwarn:5   dfail:0   fail:0   skip:15 
hsw-brixbox      total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2          total:141  pass:137  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:141  pass:101  dwarn:3   dfail:0   fail:0   skip:37 
ivb-t430s        total:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2        total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
skl-i7k-2        total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
snb-dellxps      total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220t        total:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1193/

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+
  2016-01-13 10:44 [PATCH] drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ Artur Harasimiuk
                   ` (3 preceding siblings ...)
  2016-01-15  8:20 ` ✗ Fi.CI.BAT: failure for drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ (rev2) Patchwork
@ 2020-02-06  1:10 ` Patchwork
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2020-02-06  1:10 UTC (permalink / raw)
  To: Artur Harasimiuk; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+
URL   : https://patchwork.freedesktop.org/series/2428/
State : failure

== Summary ==

CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_dpp_cm.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_hubp.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_mpc.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_opp.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_hubbub.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_optc.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_mmhubbub.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_stream_encoder.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_link_encoder.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_dccg.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_vmid.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_dwb.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_dwb_scl.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn20/dcn20_dsc.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dsc/dc_dsc.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dsc/rc_calc.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dsc/rc_calc_dpi.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_init.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_resource.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_ipp.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hw_sequencer_debug.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_dpp.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_opp.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_optc.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hubp.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_mpc.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_dpp_dscl.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_dpp_cm.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_cm_common.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_hubbub.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_stream_encoder.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn10/dcn10_link_encoder.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_lib.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_rq_dlg_helpers.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dml1_display_rq_dlg_calc.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dml_common_defs.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dml/display_mode_vba.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_rq_dlg_calc_20.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_rq_dlg_calc_20v2.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn20/display_mode_vba_20v2.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_rq_dlg_calc_21.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dml/dcn21/display_mode_vba_21.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_init.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_hubp.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_hubbub.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_resource.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_hwseq.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dcn21/dcn21_link_encoder.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce120/dce120_resource.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce120/dce120_timing_generator.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce120/dce120_hw_sequencer.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce112/dce112_compressor.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce112/dce112_hw_sequencer.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce112/dce112_resource.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_timing_generator.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_compressor.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_hw_sequencer.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_resource.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_opp_regamma_v.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_opp_csc_v.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_timing_generator_v.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_mem_input_v.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_opp_v.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce110/dce110_transform_v.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce100/dce100_resource.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce100/dce100_hw_sequencer.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce80/dce80_timing_generator.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce80/dce80_hw_sequencer.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dce80/dce80_resource.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_resource.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_hw_sequencer.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_sink.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_surface.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_hwss.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_dp.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_link_ddc.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_debug.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_stream.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/core/dc_vm_helper.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dc_helper.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dc/dc_dmub_srv.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/modules/freesync/freesync.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/modules/color/color_gamma.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/modules/info_packet/info_packet.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/modules/power/power_helpers.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_srv.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_reg.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_dcn20.o
  CC [M]  drivers/gpu/drm/amd/amdgpu/../display/dmub/src/dmub_dcn21.o
  LD [M]  drivers/gpu/drm/amd/amdgpu/amdgpu.o
scripts/Makefile.build:503: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:503: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1693: recipe for target 'drivers' failed
make: *** [drivers] Error 2

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2020-02-06  1:10 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-13 10:44 [PATCH] drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ Artur Harasimiuk
2016-01-13 11:49 ` ✓ success: Fi.CI.BAT Patchwork
2016-01-13 12:53 ` [PATCH] drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ Chris Wilson
2016-01-13 14:11   ` Dave Gordon
2016-01-13 16:09     ` Arun Siluvery
2016-01-13 17:41       ` Harasimiuk, Artur
2016-01-15  8:00 ` [PATCH] [v2] drm/i915: Exec flag to force non IA-Coherent cache " Artur Harasimiuk
2016-01-15  8:20 ` ✗ Fi.CI.BAT: failure for drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ (rev2) Patchwork
2020-02-06  1:10 ` [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: Expose exec parameter to force non IA-Coherent for Gen9+ Patchwork

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.