FAILED: patch "[PATCH] drm/i915: Remove WaDisableLSQCROPERFforOCL KBL workaround." failed to apply to 4.9-stable tree

PATCH In review [email protected] 2 participants 5 messages Jan 30, 2017 – Jan 31, 2017

Subsystems (3)

Stable branch DRM drivers Intel i915

lore

Share & tools

lore

[email protected]

author maintainer

2017-01-30 13:19 UTC #0 lore raw

The patch below does not apply to the 4.9-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

Quoted 1 line

From 4fc020d864647ea3ae8cb8f17d63e48e87ebd0bf Mon Sep 17 00:00:00 2001

From: Francisco Jerez <[email protected]>
Date: Thu, 12 Jan 2017 12:44:54 +0200
Subject: [PATCH] drm/i915: Remove WaDisableLSQCROPERFforOCL KBL workaround.

The WaDisableLSQCROPERFforOCL workaround has the side effect of disabling an L3SQ optimization that has huge performance implications and is unlikely to be necessary for the correct functioning of usual graphic workloads. Userspace is free to re-enable the workaround on demand, and is generally in a better position to determine whether the workaround is necessary than the DRM is (e.g. only during the execution of compute kernels that rely on both L3 fences and HDC R/W requests).

The same workaround seems to apply to BDW (at least to production stepping G1) and SKL as well (the internal workaround database claims that it does for all steppings, while the BSpec workaround table only mentions pre-production steppings), but the DRM doesn't do anything beyond whitelisting the L3SQCREG4 register so userspace can enable it when it sees fit. Do the same on KBL platforms.

Improves performance of the GFXBench4 gl_manhattan31 benchmark by 60%, and gl_4 (AKA car chase) by 14% on a KBL GT2 running Mesa master -- This is followed by a regression of 35% and 10% respectively for the same benchmarks and platform caused by my recent patch series switching userspace to use the dataport constant cache instead of the sampler to implement uniform pull constant loads, which caused us to hit more heavily the L3 cache (and on platforms other than KBL had the opposite effect of improving performance of the same two benchmarks). The overall effect on KBL of this change combined with the recent userspace change is respectively 4.6% and 2.6%. SynMark2 OglShMapPcf was affected by the constant cache changes (though it improved as it did on other platforms rather than regressing), but is not significantly affected by this patch (with statistical significance of 5% and sample size 20).

v2: Drop some more code to avoid unused variable warning.

Fixes 738fa1b3123f ("drm/i915/kbl: Add WaDisableLSQCROPERFforOCL")

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99256

Signed-off-by Francisco Jerez <[email protected]>

Cc Matthew Auld <[email protected]>

Cc Eero Tamminen <[email protected]>

Cc Jani Nikula <[email protected]>

Cc Mika Kuoppala <[email protected]>

Cc [email protected]

Cc <[email protected]> # v4.7+

Reviewed-by Mika Kuoppala <[email protected]>

[Removed double Fixes tag]

Signed-off-by Mika Kuoppala <[email protected]>

Link http://patchwork.freedesktop.org/patch/msgid/[email protected]

(cherry picked from commit 8726f2faa371514fba2f594d799db95203dfeee0)

Signed-off-by Jani Nikula <[email protected]>

drivers/gpu/drm/i915/intel_lrc.c +0 ~~-10~~

index d4961fa20c73..beabc17e7c8a 100644
@@ -979,18 +979,8 @@ static inline int gen8_emit_flush_coherentl3_wa(struct intel_engine_cs *engine,
 						uint32_t *batch,
 						uint32_t index)
 {
-	struct drm_i915_private *dev_priv = engine->i915;
 	uint32_t l3sqc4_flush = (0x40400000 | GEN8_LQSC_FLUSH_COHERENT_LINES);
 
-	/*
-	 * WaDisableLSQCROPERFforOCL:kbl
-	 * This WA is implemented in skl_init_clock_gating() but since
-	 * this batch updates GEN8_L3SQCREG4 with default value we need to
-	 * set this bit here to retain the WA during flush.
-	 */
-	if (IS_KBL_REVID(dev_priv, 0, KBL_REVID_E0))
-		l3sqc4_flush |= GEN8_LQSC_RO_PERF_DIS;
-
 	wa_ctx_emit(batch, index, (MI_STORE_REGISTER_MEM_GEN8 |
 				   MI_SRM_LRM_GLOBAL_GTT));
 	wa_ctx_emit_reg(batch, index, GEN8_L3SQCREG4);

drivers/gpu/drm/i915/intel_ringbuffer.c +0 -8

index aeb637dc1fdf..91cb4c422ad5 100644
@@ -1095,14 +1095,6 @@ static int kbl_init_workarounds(struct intel_engine_cs *engine)
 		WA_SET_BIT_MASKED(HDC_CHICKEN0,
 				  HDC_FENCE_DEST_SLM_DISABLE);
 
-	/* GEN8_L3SQCREG4 has a dependency with WA batch so any new changes
-	 * involving this register should also be added to WA batch as required.
-	 */
-	if (IS_KBL_REVID(dev_priv, 0, KBL_REVID_E0))
-		/* WaDisableLSQCROPERFforOCL:kbl */
-		I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) |
-			   GEN8_LQSC_RO_PERF_DIS);
-
 	/* WaToEnableHwFixForPushConstHWBug:kbl */
 	if (IS_KBL_REVID(dev_priv, KBL_REVID_C0, REVID_FOREVER))
 		WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2,

Francisco Jerez

2017-01-30 20:35 UTC #1 lore raw

PATCH 1/1 drm/i915: Remove WaDisableLSQCROPERFforOCL KBL workaround.

v2: Drop some more code to avoid unused variable warning.

Fixes 738fa1b3123f ("drm/i915/kbl: Add WaDisableLSQCROPERFforOCL")

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99256

Signed-off-by Francisco Jerez <[email protected]>

Cc Matthew Auld <[email protected]>

Cc Eero Tamminen <[email protected]>

Cc Jani Nikula <[email protected]>

Cc Mika Kuoppala <[email protected]>

Cc [email protected]

Cc <[email protected]> # v4.7+

Reviewed-by Mika Kuoppala <[email protected]>

[Removed double Fixes tag]

Signed-off-by Mika Kuoppala <[email protected]>

Link http://patchwork.freedesktop.org/patch/msgid/[email protected]

(cherry picked from commit 8726f2faa371514fba2f594d799db95203dfeee0)

Signed-off-by Jani Nikula <[email protected]>

[ Francisco Jerez: Rebase on v4.9 branch. ]

Signed-off-by Francisco Jerez <[email protected]>

---
 drivers/gpu/drm/i915/intel_lrc.c        | 3 +--
 drivers/gpu/drm/i915/intel_ringbuffer.c | 8 --------
 2 files changed, 1 insertion(+), 10 deletions(-)

drivers/gpu/drm/i915/intel_lrc.c +1 -2

index 0adb879..67db157 100644
@@ -858,8 +858,7 @@ static inline int gen8_emit_flush_coherentl3_wa(struct intel_engine_cs *engine,
 	 * this batch updates GEN8_L3SQCREG4 with default value we need to
 	 * set this bit here to retain the WA during flush.
 	 */
-	if (IS_SKL_REVID(dev_priv, 0, SKL_REVID_E0) ||
-	    IS_KBL_REVID(dev_priv, 0, KBL_REVID_E0))
+	if (IS_SKL_REVID(dev_priv, 0, SKL_REVID_E0))
 		l3sqc4_flush |= GEN8_LQSC_RO_PERF_DIS;
 
 	wa_ctx_emit(batch, index, (MI_STORE_REGISTER_MEM_GEN8 |

drivers/gpu/drm/i915/intel_ringbuffer.c +0 -8

index ed9955d..8babfe0 100644
@@ -1153,14 +1153,6 @@ static int kbl_init_workarounds(struct intel_engine_cs *engine)
 		WA_SET_BIT_MASKED(HDC_CHICKEN0,
 				  HDC_FENCE_DEST_SLM_DISABLE);
 
-	/* GEN8_L3SQCREG4 has a dependency with WA batch so any new changes
-	 * involving this register should also be added to WA batch as required.
-	 */
-	if (IS_KBL_REVID(dev_priv, 0, KBL_REVID_E0))
-		/* WaDisableLSQCROPERFforOCL:kbl */
-		I915_WRITE(GEN8_L3SQCREG4, I915_READ(GEN8_L3SQCREG4) |
-			   GEN8_LQSC_RO_PERF_DIS);
-
 	/* WaToEnableHwFixForPushConstHWBug:kbl */
 	if (IS_KBL_REVID(dev_priv, KBL_REVID_C0, REVID_FOREVER))
 		WA_SET_BIT_MASKED(COMMON_SLICE_CHICKEN2,

Signature · 1 line

2.10.2

Greg KH

author maintainer

2017-01-31 05:11 UTC #2 lore raw

Re: [PATCH] drm/i915: Remove WaDisableLSQCROPERFforOCL KBL workaround.

Quoted 50 lines · Francisco Jerez

The WaDisableLSQCROPERFforOCL workaround has the side effect of disabling an L3SQ optimization that has huge performance implications and is unlikely to be necessary for the correct functioning of usual graphic workloads. Userspace is free to re-enable the workaround on demand, and is generally in a better position to determine whether the workaround is necessary than the DRM is (e.g. only during the execution of compute kernels that rely on both L3 fences and HDC R/W requests).
The same workaround seems to apply to BDW (at least to production stepping G1) and SKL as well (the internal workaround database claims that it does for all steppings, while the BSpec workaround table only mentions pre-production steppings), but the DRM doesn't do anything beyond whitelisting the L3SQCREG4 register so userspace can enable it when it sees fit. Do the same on KBL platforms.
Improves performance of the GFXBench4 gl_manhattan31 benchmark by 60%, and gl_4 (AKA car chase) by 14% on a KBL GT2 running Mesa master -- This is followed by a regression of 35% and 10% respectively for the same benchmarks and platform caused by my recent patch series switching userspace to use the dataport constant cache instead of the sampler to implement uniform pull constant loads, which caused us to hit more heavily the L3 cache (and on platforms other than KBL had the opposite effect of improving performance of the same two benchmarks). The overall effect on KBL of this change combined with the recent userspace change is respectively 4.6% and 2.6%. SynMark2 OglShMapPcf was affected by the constant cache changes (though it improved as it did on other platforms rather than regressing), but is not significantly affected by this patch (with statistical significance of 5% and sample size 20).
v2: Drop some more code to avoid unused variable warning.
Fixes 738fa1b3123f ("drm/i915/kbl: Add WaDisableLSQCROPERFforOCL")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99256
Signed-off-by Francisco Jerez <[email protected]>
Cc Matthew Auld <[email protected]>
Cc Eero Tamminen <[email protected]>
Cc Jani Nikula <[email protected]>
Cc Mika Kuoppala <[email protected]>
Cc [email protected]
Cc <[email protected]> # v4.7+
Reviewed-by Mika Kuoppala <[email protected]>
[Removed double Fixes tag]
Signed-off-by Mika Kuoppala <[email protected]>
Link http://patchwork.freedesktop.org/patch/msgid/[email protected]
(cherry picked from commit 8726f2faa371514fba2f594d799db95203dfeee0)
Signed-off-by Jani Nikula <[email protected]>
[ Francisco Jerez: Rebase on v4.9 branch. ]
Signed-off-by Francisco Jerez <[email protected]>

Quoted 4 lines

---
 drivers/gpu/drm/i915/intel_lrc.c        | 3 +--
 drivers/gpu/drm/i915/intel_ringbuffer.c | 8 --------
 2 files changed, 1 insertion(+), 10 deletions(-)

What is the commit id of this patch in Linus's tree?

thanks,

greg k-h

Greg KH

author maintainer

2017-01-31 05:11 UTC #3 lore raw

Re: [PATCH] drm/i915: Remove WaDisableLSQCROPERFforOCL KBL workaround.

Quoted 56 lines · Greg KH

Quoted 50 lines · Francisco Jerez
The WaDisableLSQCROPERFforOCL workaround has the side effect of disabling an L3SQ optimization that has huge performance implications and is unlikely to be necessary for the correct functioning of usual graphic workloads. Userspace is free to re-enable the workaround on demand, and is generally in a better position to determine whether the workaround is necessary than the DRM is (e.g. only during the execution of compute kernels that rely on both L3 fences and HDC R/W requests).
The same workaround seems to apply to BDW (at least to production stepping G1) and SKL as well (the internal workaround database claims that it does for all steppings, while the BSpec workaround table only mentions pre-production steppings), but the DRM doesn't do anything beyond whitelisting the L3SQCREG4 register so userspace can enable it when it sees fit. Do the same on KBL platforms.
Improves performance of the GFXBench4 gl_manhattan31 benchmark by 60%, and gl_4 (AKA car chase) by 14% on a KBL GT2 running Mesa master -- This is followed by a regression of 35% and 10% respectively for the same benchmarks and platform caused by my recent patch series switching userspace to use the dataport constant cache instead of the sampler to implement uniform pull constant loads, which caused us to hit more heavily the L3 cache (and on platforms other than KBL had the opposite effect of improving performance of the same two benchmarks). The overall effect on KBL of this change combined with the recent userspace change is respectively 4.6% and 2.6%. SynMark2 OglShMapPcf was affected by the constant cache changes (though it improved as it did on other platforms rather than regressing), but is not significantly affected by this patch (with statistical significance of 5% and sample size 20).
v2: Drop some more code to avoid unused variable warning.
Fixes 738fa1b3123f ("drm/i915/kbl: Add WaDisableLSQCROPERFforOCL")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99256
Signed-off-by Francisco Jerez <[email protected]>
Cc Matthew Auld <[email protected]>
Cc Eero Tamminen <[email protected]>
Cc Jani Nikula <[email protected]>
Cc Mika Kuoppala <[email protected]>
Cc [email protected]
Cc <[email protected]> # v4.7+
Reviewed-by Mika Kuoppala <[email protected]>
[Removed double Fixes tag]
Signed-off-by Mika Kuoppala <[email protected]>
Link http://patchwork.freedesktop.org/patch/msgid/[email protected]
(cherry picked from commit 8726f2faa371514fba2f594d799db95203dfeee0)
Signed-off-by Jani Nikula <[email protected]>
[ Francisco Jerez: Rebase on v4.9 branch. ]
Signed-off-by Francisco Jerez <[email protected]>
Quoted 4 lines
---
 drivers/gpu/drm/i915/intel_lrc.c        | 3 +--
 drivers/gpu/drm/i915/intel_ringbuffer.c | 8 --------
 2 files changed, 1 insertion(+), 10 deletions(-)

Quoted 1 line

What is the commit id of this patch in Linus's tree?

Ah, nevermind, it's 4fc020d864647ea3ae8cb8f17d63e48e87ebd0bf, right?

thanks,

greg k-h

Francisco Jerez

2017-01-31 05:18 UTC #4 lore raw

Re: [PATCH] drm/i915: Remove WaDisableLSQCROPERFforOCL KBL workaround.

Quoted 57 lines · Greg KH

Quoted 56 lines · Greg KH
Quoted 54 lines · Francisco Jerez
The WaDisableLSQCROPERFforOCL workaround has the side effect of disabling an L3SQ optimization that has huge performance implications and is unlikely to be necessary for the correct functioning of usual graphic workloads. Userspace is free to re-enable the workaround on demand, and is generally in a better position to determine whether the workaround is necessary than the DRM is (e.g. only during the execution of compute kernels that rely on both L3 fences and HDC R/W requests).
The same workaround seems to apply to BDW (at least to production stepping G1) and SKL as well (the internal workaround database claims that it does for all steppings, while the BSpec workaround table only mentions pre-production steppings), but the DRM doesn't do anything beyond whitelisting the L3SQCREG4 register so userspace can enable it when it sees fit. Do the same on KBL platforms.
Improves performance of the GFXBench4 gl_manhattan31 benchmark by 60%, and gl_4 (AKA car chase) by 14% on a KBL GT2 running Mesa master -- This is followed by a regression of 35% and 10% respectively for the same benchmarks and platform caused by my recent patch series switching userspace to use the dataport constant cache instead of the sampler to implement uniform pull constant loads, which caused us to hit more heavily the L3 cache (and on platforms other than KBL had the opposite effect of improving performance of the same two benchmarks). The overall effect on KBL of this change combined with the recent userspace change is respectively 4.6% and 2.6%. SynMark2 OglShMapPcf was affected by the constant cache changes (though it improved as it did on other platforms rather than regressing), but is not significantly affected by this patch (with statistical significance of 5% and sample size 20).
v2: Drop some more code to avoid unused variable warning.
Fixes 738fa1b3123f ("drm/i915/kbl: Add WaDisableLSQCROPERFforOCL")
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99256
Signed-off-by Francisco Jerez <[email protected]>
Cc Matthew Auld <[email protected]>
Cc Eero Tamminen <[email protected]>
Cc Jani Nikula <[email protected]>
Cc Mika Kuoppala <[email protected]>
Cc [email protected]
Cc <[email protected]> # v4.7+
Reviewed-by Mika Kuoppala <[email protected]>
[Removed double Fixes tag]
Signed-off-by Mika Kuoppala <[email protected]>
Link http://patchwork.freedesktop.org/patch/msgid/[email protected]
(cherry picked from commit 8726f2faa371514fba2f594d799db95203dfeee0)
Signed-off-by Jani Nikula <[email protected]>
[ Francisco Jerez: Rebase on v4.9 branch. ]
Signed-off-by Francisco Jerez <[email protected]>
---
 drivers/gpu/drm/i915/intel_lrc.c        | 3 +--
 drivers/gpu/drm/i915/intel_ringbuffer.c | 8 --------
 2 files changed, 1 insertion(+), 10 deletions(-)

Quoted 4 lines

Quoted 1 line
What is the commit id of this patch in Linus's tree?
Ah, nevermind, it's 4fc020d864647ea3ae8cb8f17d63e48e87ebd0bf, right?

Oops, yes, that's right. Thanks!

Quoted 3 lines

thanks,
greg k-h