[PATCH v2 0/4] Gen9 WA Batch patches

[PATCH v2 0/4] Gen9 WA Batch patches

[Arun Siluvery]

Compared to v1 changes are only in Patch 1, these changes are to group
all infrastructure bits in a single patch. Patch 2 wasn't getting applied
cleanly because of this hence sending all patches.

Arun Siluvery (4):
  drm/i915: Enable WA batch buffers for Gen9
  drm/i915/gen9: Add WaDisableCtxRestoreArbitration workaround
  drm/i915/gen9: Add WaFlushCoherentL3CacheLinesAtContextSwitch
    workaround
  drm/i915/gen9: Add
    WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken

 drivers/gpu/drm/i915/intel_lrc.c        | 73 +++++++++++++++++++++++++++++++--
 drivers/gpu/drm/i915/intel_ringbuffer.c |  7 +++-
 2 files changed, 75 insertions(+), 5 deletions(-)

-- 
1.9.1

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

[PATCH v2 1/4] drm/i915: Enable WA batch buffers for Gen9

[Arun Siluvery]

This patch only enables support for Gen9, the actual WA will be
initialized in subsequent patches.

The WARN that we use to warn user if WA batch support is not available
for a particular Gen is replaced with DRM_ERROR as warning here doesn't
really add much value.

v2: include all infrastructure bits in this patch so that subsequent
changes only correspond the WA added (Chris)

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 50 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 47 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 23ff018..1e88b3b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1269,6 +1269,35 @@ static int gen8_init_perctx_bb(struct intel_engine_cs *ring,
 	return wa_ctx_end(wa_ctx, *offset = index, 1);
 }
 
+static int gen9_init_indirectctx_bb(struct intel_engine_cs *ring,
+				    struct i915_wa_ctx_bb *wa_ctx,
+				    uint32_t *const batch,
+				    uint32_t *offset)
+{
+	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
+
+	/* FIXME: Replace me with WA */
+	wa_ctx_emit(batch, MI_NOOP);
+
+	/* Pad to end of cacheline */
+	while (index % CACHELINE_DWORDS)
+		wa_ctx_emit(batch, MI_NOOP);
+
+	return wa_ctx_end(wa_ctx, *offset = index, CACHELINE_DWORDS);
+}
+
+static int gen9_init_perctx_bb(struct intel_engine_cs *ring,
+			       struct i915_wa_ctx_bb *wa_ctx,
+			       uint32_t *const batch,
+			       uint32_t *offset)
+{
+	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
+
+	wa_ctx_emit(batch, MI_BATCH_BUFFER_END);
+
+	return wa_ctx_end(wa_ctx, *offset = index, 1);
+}
+
 static int lrc_setup_wa_ctx_obj(struct intel_engine_cs *ring, u32 size)
 {
 	int ret;
@@ -1310,10 +1339,11 @@ static int intel_init_workaround_bb(struct intel_engine_cs *ring)
 	WARN_ON(ring->id != RCS);
 
 	/* update this when WA for higher Gen are added */
-	if (WARN(INTEL_INFO(ring->dev)->gen > 8,
-		 "WA batch buffer is not initialized for Gen%d\n",
-		 INTEL_INFO(ring->dev)->gen))
+	if (INTEL_INFO(ring->dev)->gen > 9) {
+		DRM_ERROR("WA batch buffer is not initialized for Gen%d\n",
+			  INTEL_INFO(ring->dev)->gen);
 		return 0;
+	}
 
 	/* some WA perform writes to scratch page, ensure it is valid */
 	if (ring->scratch.obj == NULL) {
@@ -1345,6 +1375,20 @@ static int intel_init_workaround_bb(struct intel_engine_cs *ring)
 					  &offset);
 		if (ret)
 			goto out;
+	} else if (INTEL_INFO(ring->dev)->gen == 9) {
+		ret = gen9_init_indirectctx_bb(ring,
+					       &wa_ctx->indirect_ctx,
+					       batch,
+					       &offset);
+		if (ret)
+			goto out;
+
+		ret = gen9_init_perctx_bb(ring,
+					  &wa_ctx->per_ctx,
+					  batch,
+					  &offset);
+		if (ret)
+			goto out;
 	}
 
 out:
-- 
1.9.1

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

[PATCH v2 2/4] drm/i915/gen9: Add WaDisableCtxRestoreArbitration workaround

[Arun Siluvery]

In Indirect and Per context w/a batch buffer,
+WaDisableCtxRestoreArbitration

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 1e88b3b..152b4f6 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1274,10 +1274,13 @@ static int gen9_init_indirectctx_bb(struct intel_engine_cs *ring,
 				    uint32_t *const batch,
 				    uint32_t *offset)
 {
+	struct drm_device *dev = ring->dev;
 	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
 
-	/* FIXME: Replace me with WA */
-	wa_ctx_emit(batch, MI_NOOP);
+	/* WaDisableCtxRestoreArbitration:skl,bxt */
+	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
+	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == SKL_REVID_A0)))
+		wa_ctx_emit(batch, MI_ARB_ON_OFF | MI_ARB_DISABLE);
 
 	/* Pad to end of cacheline */
 	while (index % CACHELINE_DWORDS)
@@ -1291,8 +1294,14 @@ static int gen9_init_perctx_bb(struct intel_engine_cs *ring,
 			       uint32_t *const batch,
 			       uint32_t *offset)
 {
+	struct drm_device *dev = ring->dev;
 	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
 
+	/* WaDisableCtxRestoreArbitration:skl,bxt */
+	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
+	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == SKL_REVID_A0)))
+		wa_ctx_emit(batch, MI_ARB_ON_OFF | MI_ARB_ENABLE);
+
 	wa_ctx_emit(batch, MI_BATCH_BUFFER_END);
 
 	return wa_ctx_end(wa_ctx, *offset = index, 1);
-- 
1.9.1

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

[PATCH v2 3/4] drm/i915/gen9: Add WaFlushCoherentL3CacheLinesAtContextSwitch workaround

[Arun Siluvery]

In Indirect context w/a batch buffer,
+WaFlushCoherentL3CacheLinesAtContextSwitch:bdw

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 152b4f6..c4cac4d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1282,6 +1282,11 @@ static int gen9_init_indirectctx_bb(struct intel_engine_cs *ring,
 	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == SKL_REVID_A0)))
 		wa_ctx_emit(batch, MI_ARB_ON_OFF | MI_ARB_DISABLE);
 
+	/* WaFlushCoherentL3CacheLinesAtContextSwitch:skl,bxt */
+	index = gen8_emit_flush_coherentl3_wa(ring, batch, index);
+	if (index < 0)
+		return index;
+
 	/* Pad to end of cacheline */
 	while (index % CACHELINE_DWORDS)
 		wa_ctx_emit(batch, MI_NOOP);
-- 
1.9.1

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

[PATCH v2 4/4] drm/i915/gen9: Add WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken

[Arun Siluvery]

In Indirect context w/a batch buffer,
+WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c        | 9 +++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index c4cac4d..beb1cb3 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1302,6 +1302,15 @@ static int gen9_init_perctx_bb(struct intel_engine_cs *ring,
 	struct drm_device *dev = ring->dev;
 	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
 
+	/* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
+	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_B0)) ||
+	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == SKL_REVID_A0))) {
+		wa_ctx_emit(batch, MI_LOAD_REGISTER_IMM(1));
+		wa_ctx_emit(batch, GEN9_SLICE_COMMON_ECO_CHICKEN0);
+		wa_ctx_emit(batch, _MASKED_BIT_ENABLE(DISABLE_PIXEL_MASK_CAMMING));
+		wa_ctx_emit(batch, MI_NOOP);
+	}
+
 	/* WaDisableCtxRestoreArbitration:skl,bxt */
 	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
 	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == SKL_REVID_A0)))
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index af7c12e..66dde7f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -946,8 +946,11 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring)
 		/* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
 		WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1,
 				  GEN9_RHWO_OPTIMIZATION_DISABLE);
-		WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN0,
-				  DISABLE_PIXEL_MASK_CAMMING);
+		/*
+		 * WA also requires GEN9_SLICE_COMMON_ECO_CHICKEN0[14] to be set,
+		 * but that register is write only hence it is set
+		 * in per ctx batch buffer
+		 */
 	}
 
 	if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) >= SKL_REVID_C0) ||
-- 
1.9.1

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

[PATCH v2 2/4] drm/i915/gen9: Add WaDisableCtxRestoreArbitration workaround

[Arun Siluvery]

In Indirect and Per context w/a batch buffer,
+WaDisableCtxRestoreArbitration

v2: SKL revision id was used for BXT, copy paste error found during
internal review (Bob Beckett).

Cc: Robert Beckett <robert.beckett@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 1e88b3b..e84fc52 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1274,10 +1274,13 @@ static int gen9_init_indirectctx_bb(struct intel_engine_cs *ring,
 				    uint32_t *const batch,
 				    uint32_t *offset)
 {
+	struct drm_device *dev = ring->dev;
 	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
 
-	/* FIXME: Replace me with WA */
-	wa_ctx_emit(batch, MI_NOOP);
+	/* WaDisableCtxRestoreArbitration:skl,bxt */
+	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
+	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
+		wa_ctx_emit(batch, MI_ARB_ON_OFF | MI_ARB_DISABLE);
 
 	/* Pad to end of cacheline */
 	while (index % CACHELINE_DWORDS)
@@ -1291,8 +1294,14 @@ static int gen9_init_perctx_bb(struct intel_engine_cs *ring,
 			       uint32_t *const batch,
 			       uint32_t *offset)
 {
+	struct drm_device *dev = ring->dev;
 	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
 
+	/* WaDisableCtxRestoreArbitration:skl,bxt */
+	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
+	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
+		wa_ctx_emit(batch, MI_ARB_ON_OFF | MI_ARB_ENABLE);
+
 	wa_ctx_emit(batch, MI_BATCH_BUFFER_END);
 
 	return wa_ctx_end(wa_ctx, *offset = index, 1);
-- 
1.9.1

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

[PATCH v2 4/4] drm/i915/gen9: Add WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken

[Arun Siluvery]

In Indirect context w/a batch buffer,
+WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken

v2: SKL revision id was used for BXT, copy paste error found during
internal review (Bob Beckett).

Cc: Robert Beckett <robert.beckett@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c        | 9 +++++++++
 drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++++--
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 58247f0..61ed92b 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1302,6 +1302,15 @@ static int gen9_init_perctx_bb(struct intel_engine_cs *ring,
 	struct drm_device *dev = ring->dev;
 	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
 
+	/* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
+	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_B0)) ||
+	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0))) {
+		wa_ctx_emit(batch, MI_LOAD_REGISTER_IMM(1));
+		wa_ctx_emit(batch, GEN9_SLICE_COMMON_ECO_CHICKEN0);
+		wa_ctx_emit(batch, _MASKED_BIT_ENABLE(DISABLE_PIXEL_MASK_CAMMING));
+		wa_ctx_emit(batch, MI_NOOP);
+	}
+
 	/* WaDisableCtxRestoreArbitration:skl,bxt */
 	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
 	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index af7c12e..66dde7f 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -946,8 +946,11 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring)
 		/* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
 		WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1,
 				  GEN9_RHWO_OPTIMIZATION_DISABLE);
-		WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN0,
-				  DISABLE_PIXEL_MASK_CAMMING);
+		/*
+		 * WA also requires GEN9_SLICE_COMMON_ECO_CHICKEN0[14] to be set,
+		 * but that register is write only hence it is set
+		 * in per ctx batch buffer
+		 */
 	}
 
 	if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) >= SKL_REVID_C0) ||
-- 
1.9.1

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

Re: [PATCH v2 1/4] drm/i915: Enable WA batch buffers for Gen9

[Mika Kuoppala]

Arun Siluvery <arun.siluvery@linux.intel.com> writes:

> This patch only enables support for Gen9, the actual WA will be
> initialized in subsequent patches.
>
> The WARN that we use to warn user if WA batch support is not available
> for a particular Gen is replaced with DRM_ERROR as warning here doesn't
> really add much value.
>
> v2: include all infrastructure bits in this patch so that subsequent
> changes only correspond the WA added (Chris)
>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>

The wa_ctx_emits need index as second param now. With those,

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 50 +++++++++++++++++++++++++++++++++++++---
>  1 file changed, 47 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 23ff018..1e88b3b 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1269,6 +1269,35 @@ static int gen8_init_perctx_bb(struct intel_engine_cs *ring,
>  	return wa_ctx_end(wa_ctx, *offset = index, 1);
>  }
>  
> +static int gen9_init_indirectctx_bb(struct intel_engine_cs *ring,
> +				    struct i915_wa_ctx_bb *wa_ctx,
> +				    uint32_t *const batch,
> +				    uint32_t *offset)
> +{
> +	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
> +
> +	/* FIXME: Replace me with WA */
> +	wa_ctx_emit(batch, MI_NOOP);
> +
> +	/* Pad to end of cacheline */
> +	while (index % CACHELINE_DWORDS)
> +		wa_ctx_emit(batch, MI_NOOP);
> +
> +	return wa_ctx_end(wa_ctx, *offset = index, CACHELINE_DWORDS);
> +}
> +
> +static int gen9_init_perctx_bb(struct intel_engine_cs *ring,
> +			       struct i915_wa_ctx_bb *wa_ctx,
> +			       uint32_t *const batch,
> +			       uint32_t *offset)
> +{
> +	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
> +
> +	wa_ctx_emit(batch, MI_BATCH_BUFFER_END);
> +
> +	return wa_ctx_end(wa_ctx, *offset = index, 1);
> +}
> +
>  static int lrc_setup_wa_ctx_obj(struct intel_engine_cs *ring, u32 size)
>  {
>  	int ret;
> @@ -1310,10 +1339,11 @@ static int intel_init_workaround_bb(struct intel_engine_cs *ring)
>  	WARN_ON(ring->id != RCS);
>  
>  	/* update this when WA for higher Gen are added */
> -	if (WARN(INTEL_INFO(ring->dev)->gen > 8,
> -		 "WA batch buffer is not initialized for Gen%d\n",
> -		 INTEL_INFO(ring->dev)->gen))
> +	if (INTEL_INFO(ring->dev)->gen > 9) {
> +		DRM_ERROR("WA batch buffer is not initialized for Gen%d\n",
> +			  INTEL_INFO(ring->dev)->gen);
>  		return 0;
> +	}
>  
>  	/* some WA perform writes to scratch page, ensure it is valid */
>  	if (ring->scratch.obj == NULL) {
> @@ -1345,6 +1375,20 @@ static int intel_init_workaround_bb(struct intel_engine_cs *ring)
>  					  &offset);
>  		if (ret)
>  			goto out;
> +	} else if (INTEL_INFO(ring->dev)->gen == 9) {
> +		ret = gen9_init_indirectctx_bb(ring,
> +					       &wa_ctx->indirect_ctx,
> +					       batch,
> +					       &offset);
> +		if (ret)
> +			goto out;
> +
> +		ret = gen9_init_perctx_bb(ring,
> +					  &wa_ctx->per_ctx,
> +					  batch,
> +					  &offset);
> +		if (ret)
> +			goto out;
>  	}
>  
>  out:
> -- 
> 1.9.1
>
> _______________________________________________
> 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

Re: [PATCH v2 1/4] drm/i915: Enable WA batch buffers for Gen9

[Siluvery, Arun]

On 10/07/2015 16:52, Mika Kuoppala wrote:
> Arun Siluvery <arun.siluvery@linux.intel.com> writes:
>
>> This patch only enables support for Gen9, the actual WA will be
>> initialized in subsequent patches.
>>
>> The WARN that we use to warn user if WA batch support is not available
>> for a particular Gen is replaced with DRM_ERROR as warning here doesn't
>> really add much value.
>>
>> v2: include all infrastructure bits in this patch so that subsequent
>> changes only correspond the WA added (Chris)
>>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
>
> The wa_ctx_emits need index as second param now. With those,
>
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
>
Thanks Mika, I will send the updated patches.
Your r-b tag is for all patches correct?
Should I include the tag while sending the patches?

regards
Arun

>> ---
>>   drivers/gpu/drm/i915/intel_lrc.c | 50 +++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 47 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> index 23ff018..1e88b3b 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -1269,6 +1269,35 @@ static int gen8_init_perctx_bb(struct intel_engine_cs *ring,
>>   	return wa_ctx_end(wa_ctx, *offset = index, 1);
>>   }
>>
>> +static int gen9_init_indirectctx_bb(struct intel_engine_cs *ring,
>> +				    struct i915_wa_ctx_bb *wa_ctx,
>> +				    uint32_t *const batch,
>> +				    uint32_t *offset)
>> +{
>> +	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
>> +
>> +	/* FIXME: Replace me with WA */
>> +	wa_ctx_emit(batch, MI_NOOP);
>> +
>> +	/* Pad to end of cacheline */
>> +	while (index % CACHELINE_DWORDS)
>> +		wa_ctx_emit(batch, MI_NOOP);
>> +
>> +	return wa_ctx_end(wa_ctx, *offset = index, CACHELINE_DWORDS);
>> +}
>> +
>> +static int gen9_init_perctx_bb(struct intel_engine_cs *ring,
>> +			       struct i915_wa_ctx_bb *wa_ctx,
>> +			       uint32_t *const batch,
>> +			       uint32_t *offset)
>> +{
>> +	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
>> +
>> +	wa_ctx_emit(batch, MI_BATCH_BUFFER_END);
>> +
>> +	return wa_ctx_end(wa_ctx, *offset = index, 1);
>> +}
>> +
>>   static int lrc_setup_wa_ctx_obj(struct intel_engine_cs *ring, u32 size)
>>   {
>>   	int ret;
>> @@ -1310,10 +1339,11 @@ static int intel_init_workaround_bb(struct intel_engine_cs *ring)
>>   	WARN_ON(ring->id != RCS);
>>
>>   	/* update this when WA for higher Gen are added */
>> -	if (WARN(INTEL_INFO(ring->dev)->gen > 8,
>> -		 "WA batch buffer is not initialized for Gen%d\n",
>> -		 INTEL_INFO(ring->dev)->gen))
>> +	if (INTEL_INFO(ring->dev)->gen > 9) {
>> +		DRM_ERROR("WA batch buffer is not initialized for Gen%d\n",
>> +			  INTEL_INFO(ring->dev)->gen);
>>   		return 0;
>> +	}
>>
>>   	/* some WA perform writes to scratch page, ensure it is valid */
>>   	if (ring->scratch.obj == NULL) {
>> @@ -1345,6 +1375,20 @@ static int intel_init_workaround_bb(struct intel_engine_cs *ring)
>>   					  &offset);
>>   		if (ret)
>>   			goto out;
>> +	} else if (INTEL_INFO(ring->dev)->gen == 9) {
>> +		ret = gen9_init_indirectctx_bb(ring,
>> +					       &wa_ctx->indirect_ctx,
>> +					       batch,
>> +					       &offset);
>> +		if (ret)
>> +			goto out;
>> +
>> +		ret = gen9_init_perctx_bb(ring,
>> +					  &wa_ctx->per_ctx,
>> +					  batch,
>> +					  &offset);
>> +		if (ret)
>> +			goto out;
>>   	}
>>
>>   out:
>> --
>> 1.9.1
>>
>> _______________________________________________
>> 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

[PATCH v2 3/4] drm/i915/gen9: Add WaFlushCoherentL3CacheLinesAtContextSwitch workaround

[Arun Siluvery]

In Indirect context w/a batch buffer,
+WaFlushCoherentL3CacheLinesAtContextSwitch:bdw

v2: address static checker warning where unsigned value was checked for
less than zero which is never true.

Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_lrc.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 6a0b128..7536682 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -1253,6 +1253,7 @@ static int gen9_init_indirectctx_bb(struct intel_engine_cs *ring,
 				    uint32_t *const batch,
 				    uint32_t *offset)
 {
+	int ret;
 	struct drm_device *dev = ring->dev;
 	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
 
@@ -1261,6 +1262,12 @@ static int gen9_init_indirectctx_bb(struct intel_engine_cs *ring,
 	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
 		wa_ctx_emit(batch, MI_ARB_ON_OFF | MI_ARB_DISABLE);
 
+	/* WaFlushCoherentL3CacheLinesAtContextSwitch:skl,bxt */
+	ret = gen8_emit_flush_coherentl3_wa(ring, batch, index);
+	if (ret < 0)
+		return ret;
+	index = ret;
+
 	/* Pad to end of cacheline */
 	while (index % CACHELINE_DWORDS)
 		wa_ctx_emit(batch, MI_NOOP);
-- 
1.9.1

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

Re: [PATCH v2 2/4] drm/i915/gen9: Add WaDisableCtxRestoreArbitration workaround

[Mika Kuoppala]

Arun Siluvery <arun.siluvery@linux.intel.com> writes:

> In Indirect and Per context w/a batch buffer,
> +WaDisableCtxRestoreArbitration
>
> v2: SKL revision id was used for BXT, copy paste error found during
> internal review (Bob Beckett).
>
> Cc: Robert Beckett <robert.beckett@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 1e88b3b..e84fc52 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1274,10 +1274,13 @@ static int gen9_init_indirectctx_bb(struct intel_engine_cs *ring,
>  				    uint32_t *const batch,
>  				    uint32_t *offset)
>  {
> +	struct drm_device *dev = ring->dev;
>  	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
>  
> -	/* FIXME: Replace me with WA */
> -	wa_ctx_emit(batch, MI_NOOP);
> +	/* WaDisableCtxRestoreArbitration:skl,bxt */
> +	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
> +	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
> +		wa_ctx_emit(batch, MI_ARB_ON_OFF | MI_ARB_DISABLE);
>  
>  	/* Pad to end of cacheline */
>  	while (index % CACHELINE_DWORDS)
> @@ -1291,8 +1294,14 @@ static int gen9_init_perctx_bb(struct intel_engine_cs *ring,
>  			       uint32_t *const batch,
>  			       uint32_t *offset)
>  {
> +	struct drm_device *dev = ring->dev;
>  	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
>  
> +	/* WaDisableCtxRestoreArbitration:skl,bxt */
> +	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
> +	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
> +		wa_ctx_emit(batch, MI_ARB_ON_OFF | MI_ARB_ENABLE);
> +
>  	wa_ctx_emit(batch, MI_BATCH_BUFFER_END);
>  
>  	return wa_ctx_end(wa_ctx, *offset = index, 1);
> -- 
> 1.9.1
>
> _______________________________________________
> 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

Re: [PATCH v2 3/4] drm/i915/gen9: Add WaFlushCoherentL3CacheLinesAtContextSwitch workaround

[Mika Kuoppala]

Arun Siluvery <arun.siluvery@linux.intel.com> writes:

> In Indirect context w/a batch buffer,
> +WaFlushCoherentL3CacheLinesAtContextSwitch:bdw

s/bdw/skl ?

>
> v2: address static checker warning where unsigned value was checked for
> less than zero which is never true.
>
Add                                   ^^ (Dan Carpenter)

> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

And remove this line as this would mean the workaround/bug
in question would be reported by Dan.

> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c | 7 +++++++
>  1 file changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 6a0b128..7536682 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1253,6 +1253,7 @@ static int gen9_init_indirectctx_bb(struct intel_engine_cs *ring,
>  				    uint32_t *const batch,
>  				    uint32_t *offset)
>  {
> +	int ret;
>  	struct drm_device *dev = ring->dev;
>  	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
>  
> @@ -1261,6 +1262,12 @@ static int gen9_init_indirectctx_bb(struct intel_engine_cs *ring,
>  	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
>  		wa_ctx_emit(batch, MI_ARB_ON_OFF | MI_ARB_DISABLE);
>  
> +	/* WaFlushCoherentL3CacheLinesAtContextSwitch:skl,bxt */
> +	ret = gen8_emit_flush_coherentl3_wa(ring, batch, index);

Not shown in this patch but the above function assumes default
value for GEN8_L3SQCREG4 which doesn't match what we have by
default.

This is due to  skl_init_clock_gating() setting up one bit
in this register.

I think the proper way to fix this would be remove the
write from skl_init_clock_gating() and setup all
the bits in this register, even the default ones with
WA_SET_BIT() in gen9_init_workarounds(). 

And then search the default value out from the wa list,
when you build the batch.

But if you choose to go with default skl value of
0x48400000, make a comment to intel_pm.c and also
the gen8_emit_flush_coherentl3_wa() that you
have a dependency.

-Mika

> +	if (ret < 0)
> +		return ret;
> +	index = ret;
> +


>  	/* Pad to end of cacheline */
>  	while (index % CACHELINE_DWORDS)
>  		wa_ctx_emit(batch, MI_NOOP);
> -- 
> 1.9.1
>
> _______________________________________________
> 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

Re: [PATCH v2 4/4] drm/i915/gen9: Add WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken

[Mika Kuoppala]

Arun Siluvery <arun.siluvery@linux.intel.com> writes:

> In Indirect context w/a batch buffer,
> +WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken
>
> v2: SKL revision id was used for BXT, copy paste error found during
> internal review (Bob Beckett).
>
> Cc: Robert Beckett <robert.beckett@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_lrc.c        | 9 +++++++++
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++++--
>  2 files changed, 14 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 58247f0..61ed92b 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -1302,6 +1302,15 @@ static int gen9_init_perctx_bb(struct intel_engine_cs *ring,
>  	struct drm_device *dev = ring->dev;
>  	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
>  
> +	/* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
> +	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_B0)) ||
> +	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0))) {
> +		wa_ctx_emit(batch, MI_LOAD_REGISTER_IMM(1));
> +		wa_ctx_emit(batch, GEN9_SLICE_COMMON_ECO_CHICKEN0);
> +		wa_ctx_emit(batch, _MASKED_BIT_ENABLE(DISABLE_PIXEL_MASK_CAMMING));
> +		wa_ctx_emit(batch, MI_NOOP);
> +	}
> +
>  	/* WaDisableCtxRestoreArbitration:skl,bxt */
>  	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
>  	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index af7c12e..66dde7f 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -946,8 +946,11 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring)
>  		/* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
>  		WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1,
>  				  GEN9_RHWO_OPTIMIZATION_DISABLE);
> -		WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN0,
> -				  DISABLE_PIXEL_MASK_CAMMING);
> +		/*
> +		 * WA also requires GEN9_SLICE_COMMON_ECO_CHICKEN0[14] to be set,
> +		 * but that register is write only hence it is set
> +		 * in per ctx batch buffer

Why the need to move to per ctx bb? Why the write would not stick
with this? Is the rationale that we get fails with the gem_workarounds?

If so, perhaps we need a write_only marker for workaround list?

-Mika


> +		 */
>  	}
>  
>  	if ((IS_SKYLAKE(dev) && INTEL_REVID(dev) >= SKL_REVID_C0) ||
> -- 
> 1.9.1
>
> _______________________________________________
> 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

Re: [PATCH v2 2/4] drm/i915/gen9: Add WaDisableCtxRestoreArbitration workaround

[Daniel Vetter]

On Mon, Jul 13, 2015 at 03:31:45PM +0300, Mika Kuoppala wrote:
> Arun Siluvery <arun.siluvery@linux.intel.com> writes:
> 
> > In Indirect and Per context w/a batch buffer,
> > +WaDisableCtxRestoreArbitration
> >
> > v2: SKL revision id was used for BXT, copy paste error found during
> > internal review (Bob Beckett).
> >
> > Cc: Robert Beckett <robert.beckett@intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

Merged first two patches of this series to dinq, thanks.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/i915/intel_lrc.c | 13 +++++++++++--
> >  1 file changed, 11 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index 1e88b3b..e84fc52 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -1274,10 +1274,13 @@ static int gen9_init_indirectctx_bb(struct intel_engine_cs *ring,
> >  				    uint32_t *const batch,
> >  				    uint32_t *offset)
> >  {
> > +	struct drm_device *dev = ring->dev;
> >  	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
> >  
> > -	/* FIXME: Replace me with WA */
> > -	wa_ctx_emit(batch, MI_NOOP);
> > +	/* WaDisableCtxRestoreArbitration:skl,bxt */
> > +	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
> > +	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
> > +		wa_ctx_emit(batch, MI_ARB_ON_OFF | MI_ARB_DISABLE);
> >  
> >  	/* Pad to end of cacheline */
> >  	while (index % CACHELINE_DWORDS)
> > @@ -1291,8 +1294,14 @@ static int gen9_init_perctx_bb(struct intel_engine_cs *ring,
> >  			       uint32_t *const batch,
> >  			       uint32_t *offset)
> >  {
> > +	struct drm_device *dev = ring->dev;
> >  	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
> >  
> > +	/* WaDisableCtxRestoreArbitration:skl,bxt */
> > +	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
> > +	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
> > +		wa_ctx_emit(batch, MI_ARB_ON_OFF | MI_ARB_ENABLE);
> > +
> >  	wa_ctx_emit(batch, MI_BATCH_BUFFER_END);
> >  
> >  	return wa_ctx_end(wa_ctx, *offset = index, 1);
> > -- 
> > 1.9.1
> >
> > _______________________________________________
> > 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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 2/4] drm/i915/gen9: Add WaDisableCtxRestoreArbitration workaround

[Daniel Vetter]

On Mon, Jul 13, 2015 at 04:49:37PM +0200, Daniel Vetter wrote:
> On Mon, Jul 13, 2015 at 03:31:45PM +0300, Mika Kuoppala wrote:
> > Arun Siluvery <arun.siluvery@linux.intel.com> writes:
> > 
> > > In Indirect and Per context w/a batch buffer,
> > > +WaDisableCtxRestoreArbitration
> > >
> > > v2: SKL revision id was used for BXT, copy paste error found during
> > > internal review (Bob Beckett).
> > >
> > > Cc: Robert Beckett <robert.beckett@intel.com>
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
> > 
> > Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>
> 
> Merged first two patches of this series to dinq, thanks.

Actually strike that, these all need rebasing. Dropped again.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v2 4/4] drm/i915/gen9: Add WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken

[Dave Gordon]

On 13/07/15 14:16, Mika Kuoppala wrote:
> Arun Siluvery <arun.siluvery@linux.intel.com> writes:
>
>> In Indirect context w/a batch buffer,
>> +WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken
>>
>> v2: SKL revision id was used for BXT, copy paste error found during
>> internal review (Bob Beckett).
>>
>> Cc: Robert Beckett <robert.beckett@intel.com>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_lrc.c        | 9 +++++++++
>>   drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++++--
>>   2 files changed, 14 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>> index 58247f0..61ed92b 100644
>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>> @@ -1302,6 +1302,15 @@ static int gen9_init_perctx_bb(struct intel_engine_cs *ring,
>>   	struct drm_device *dev = ring->dev;
>>   	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
>>
>> +	/* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
>> +	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_B0)) ||
>> +	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0))) {
>> +		wa_ctx_emit(batch, MI_LOAD_REGISTER_IMM(1));
>> +		wa_ctx_emit(batch, GEN9_SLICE_COMMON_ECO_CHICKEN0);
>> +		wa_ctx_emit(batch, _MASKED_BIT_ENABLE(DISABLE_PIXEL_MASK_CAMMING));
>> +		wa_ctx_emit(batch, MI_NOOP);
>> +	}
>> +
>>   	/* WaDisableCtxRestoreArbitration:skl,bxt */
>>   	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
>>   	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> index af7c12e..66dde7f 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> @@ -946,8 +946,11 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring)
>>   		/* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
>>   		WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1,
>>   				  GEN9_RHWO_OPTIMIZATION_DISABLE);
>> -		WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN0,
>> -				  DISABLE_PIXEL_MASK_CAMMING);
>> +		/*
>> +		 * WA also requires GEN9_SLICE_COMMON_ECO_CHICKEN0[14] to be set,
>> +		 * but that register is write only hence it is set
>> +		 * in per ctx batch buffer
>
> Why the need to move to per ctx bb? Why the write would not stick
> with this? Is the rationale that we get fails with the gem_workarounds?
>
> If so, perhaps we need a write_only marker for workaround list?
>
> -Mika

Surely it's the fact that it's a context-saved register that matters, 
rather than it being write-only (generally we treat masked registers as 
write-only anyway, since the masking means that you can set or clear a 
bit without having to read the other bits first).

Whereas AFAICS GEN7_COMMON_SLICE_CHICKEN1 is a global non-context 
register, so it can be set directly rather than having to be done in a 
batchbuffer.

.Dave.

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

Re: [PATCH v2 4/4] drm/i915/gen9: Add WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken

[Mika Kuoppala]

Hi Dave,

Dave Gordon <david.s.gordon@intel.com> writes:

> On 13/07/15 14:16, Mika Kuoppala wrote:
>> Arun Siluvery <arun.siluvery@linux.intel.com> writes:
>>
>>> In Indirect context w/a batch buffer,
>>> +WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken
>>>
>>> v2: SKL revision id was used for BXT, copy paste error found during
>>> internal review (Bob Beckett).
>>>
>>> Cc: Robert Beckett <robert.beckett@intel.com>
>>> Cc: Imre Deak <imre.deak@intel.com>
>>> Signed-off-by: Arun Siluvery <arun.siluvery@linux.intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_lrc.c        | 9 +++++++++
>>>   drivers/gpu/drm/i915/intel_ringbuffer.c | 7 +++++--
>>>   2 files changed, 14 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
>>> index 58247f0..61ed92b 100644
>>> --- a/drivers/gpu/drm/i915/intel_lrc.c
>>> +++ b/drivers/gpu/drm/i915/intel_lrc.c
>>> @@ -1302,6 +1302,15 @@ static int gen9_init_perctx_bb(struct intel_engine_cs *ring,
>>>   	struct drm_device *dev = ring->dev;
>>>   	uint32_t index = wa_ctx_start(wa_ctx, *offset, CACHELINE_DWORDS);
>>>
>>> +	/* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
>>> +	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_B0)) ||
>>> +	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0))) {
>>> +		wa_ctx_emit(batch, MI_LOAD_REGISTER_IMM(1));
>>> +		wa_ctx_emit(batch, GEN9_SLICE_COMMON_ECO_CHICKEN0);
>>> +		wa_ctx_emit(batch, _MASKED_BIT_ENABLE(DISABLE_PIXEL_MASK_CAMMING));
>>> +		wa_ctx_emit(batch, MI_NOOP);
>>> +	}
>>> +
>>>   	/* WaDisableCtxRestoreArbitration:skl,bxt */
>>>   	if ((IS_SKYLAKE(dev) && (INTEL_REVID(dev) <= SKL_REVID_D0)) ||
>>>   	    (IS_BROXTON(dev) && (INTEL_REVID(dev) == BXT_REVID_A0)))
>>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> index af7c12e..66dde7f 100644
>>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>>> @@ -946,8 +946,11 @@ static int gen9_init_workarounds(struct intel_engine_cs *ring)
>>>   		/* WaSetDisablePixMaskCammingAndRhwoInCommonSliceChicken:skl,bxt */
>>>   		WA_SET_BIT_MASKED(GEN7_COMMON_SLICE_CHICKEN1,
>>>   				  GEN9_RHWO_OPTIMIZATION_DISABLE);
>>> -		WA_SET_BIT_MASKED(GEN9_SLICE_COMMON_ECO_CHICKEN0,
>>> -				  DISABLE_PIXEL_MASK_CAMMING);
>>> +		/*
>>> +		 * WA also requires GEN9_SLICE_COMMON_ECO_CHICKEN0[14] to be set,
>>> +		 * but that register is write only hence it is set
>>> +		 * in per ctx batch buffer
>>
>> Why the need to move to per ctx bb? Why the write would not stick
>> with this? Is the rationale that we get fails with the gem_workarounds?
>>
>> If so, perhaps we need a write_only marker for workaround list?
>>
>> -Mika
>
> Surely it's the fact that it's a context-saved register that matters, 
> rather than it being write-only (generally we treat masked registers as 
> write-only anyway, since the masking means that you can set or clear a 
> bit without having to read the other bits first).
>

But with workaround list, we push these writes into the ringbuffer
of the context being created. So I still fail to see how they
would end up being out of context.

The per ctx bb would make sense if this register was not part of
contexts and thus would not be saved. Is this the case here?

-Mika

> Whereas AFAICS GEN7_COMMON_SLICE_CHICKEN1 is a global non-context 
> register, so it can be set directly rather than having to be done in a 
> batchbuffer.
>
> .Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx