[Intel-gfx] [PATCH v2 1/2] drm/i915/mtl: Extend Wa_16014892111 to MTL A-step

[Intel-gfx] [PATCH v2 1/2] drm/i915/mtl: Extend Wa_16014892111 to MTL A-step

[Radhakrishna Sripada]

The dg2 workaround which is used for performance tuning
is needed for Meteorlake A-step.

v2: Limit the WA for A-step

Bspec: 68331
Cc: Haridhar Kalvala <haridhar.kalvala@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Gustavo Sousa <gustavo.sousa@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
index 81a96c52a92b..9c1007c44298 100644
--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
@@ -1370,7 +1370,9 @@ gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs)
 					      cs, GEN12_GFX_CCS_AUX_NV);
 
 	/* Wa_16014892111 */
-	if (IS_DG2(ce->engine->i915))
+	if (IS_DG2(ce->engine->i915) ||
+	    IS_MTL_GRAPHICS_STEP(ce->engine->i915, M, STEP_A0, STEP_B0) ||
+	    IS_MTL_GRAPHICS_STEP(ce->engine->i915, P, STEP_A0, STEP_B0))
 		cs = dg2_emit_draw_watermark_setting(cs);
 
 	return cs;
-- 
2.34.1

[Intel-gfx] [PATCH v2 2/2] drm/i915/mtl: Add MTL performance tuning changes

[Radhakrishna Sripada]

MTL reuses the tuning parameters for DG2. Extend the dg2
performance tuning parameters to MTL.

v2: Add DRAW_WATERMARK tuning parameter.

Bspec: 68331
Cc: Haridhar Kalvala <haridhar.kalvala@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Gustavo Sousa <gustavo.sousa@intel.com>
Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_workarounds.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
index 786349e95487..72dab970de5b 100644
--- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
+++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
@@ -817,6 +817,10 @@ static void mtl_ctx_workarounds_init(struct intel_engine_cs *engine,
 {
 	struct drm_i915_private *i915 = engine->i915;
 
+	dg2_ctx_gt_tuning_init(engine, wal);
+
+	wa_add(wal, DRAW_WATERMARK, VERT_WM_VAL, 0x3FF, 0, false);
+
 	if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) ||
 	    IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) {
 		/* Wa_14014947963 */
@@ -1754,7 +1758,7 @@ static void gt_tuning_settings(struct intel_gt *gt, struct i915_wa_list *wal)
 		wa_mcr_masked_en(wal, XEHPC_LNCFMISCCFGREG0, XEHPC_HOSTCACHEEN);
 	}
 
-	if (IS_DG2(gt->i915)) {
+	if (IS_DG2(gt->i915) || IS_METEORLAKE(gt->i915)) {
 		wa_mcr_write_or(wal, XEHP_L3SCQREG7, BLEND_FILL_CACHING_OPT_DIS);
 		wa_mcr_write_or(wal, XEHP_SQCM, EN_32B_ACCESS);
 	}
@@ -2944,7 +2948,7 @@ static void
 add_render_compute_tuning_settings(struct drm_i915_private *i915,
 				   struct i915_wa_list *wal)
 {
-	if (IS_DG2(i915))
+	if (IS_DG2(i915) || IS_METEORLAKE(i915))
 		wa_mcr_write_clr_set(wal, RT_CTRL, STACKID_CTRL, STACKID_CTRL_512);
 
 	/*
-- 
2.34.1

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [v2,1/2] drm/i915/mtl: Extend Wa_16014892111 to MTL A-step

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/mtl: Extend Wa_16014892111 to MTL A-step
URL   : https://patchwork.freedesktop.org/series/117717/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.
+./arch/x86/include/asm/bitops.h:117:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:148:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:150:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:154:26: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:156:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:174:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:176:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:180:35: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:182:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:182:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:186:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:188:9: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:192:35: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:195:16: warning: unreplaced symbol 'oldbit'
+./arch/x86/include/asm/bitops.h:195:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:237:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:239:9: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:66:1: warning: unreplaced symbol 'return'
+./arch/x86/include/asm/bitops.h:92:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:100:17: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:100:23: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:100:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:105:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:107:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:108:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:109:9: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:111:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:111:14: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:111:20: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:112:17: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:112:23: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:112:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:121:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:128:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:166:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:168:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:169:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:170:9: warning: unreplaced symbol 'val'
+./include/asm-generic/bitops/generic-non-atomic.h:172:19: warning: unreplaced symbol 'val'
+./include/asm-generic/bitops/generic-non-atomic.h:172:25: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:172:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:28:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:30:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:31:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:33:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:33:16: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:37:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:39:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:40:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:42:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:42:16: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:55:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:57:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:58:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:60:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:60:15: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:73:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:75:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:76:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:77:9: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:79:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:79:14: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:79:20: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:80:17: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:80:23: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:80:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:93:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/generic-non-atomic.h:95:9: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/generic-non-atomic.h:96:9: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:97:9: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:99:10: warning: unreplaced symbol 'p'
+./include/asm-generic/bitops/generic-non-atomic.h:99:14: warning: unreplaced symbol 'old'
+./include/asm-generic/bitops/generic-non-atomic.h:99:21: warning: unreplaced symbol 'mask'
+./include/asm-generic/bitops/instrumented-non-atomic.h:100:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:112:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:115:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:127:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:130:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:139:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:142:9: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:26:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:42:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:58:1: warning: unreplaced symbol 'return'
+./include/asm-generic/bitops/instrumented-non-atomic.h:97:1: warning: unreplaced symbol 'return'

[Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [v2,1/2] drm/i915/mtl: Extend Wa_16014892111 to MTL A-step

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 7555 bytes --]

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/mtl: Extend Wa_16014892111 to MTL A-step
URL   : https://patchwork.freedesktop.org/series/117717/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13143 -> Patchwork_117717v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_117717v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_117717v1, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/index.html

Participating hosts (38 -> 38)
------------------------------

  Additional (1): fi-kbl-soraka 
  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_117717v1:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@load:
    - bat-adls-5:         [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13143/bat-adls-5/igt@i915_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/bat-adls-5/igt@i915_module_load@load.html

  
Known issues
------------

  Here are the changes found in Patchwork_117717v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_huc_copy@huc-copy:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][3] ([fdo#109271] / [i915#2190])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@basic:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][4] ([fdo#109271] / [i915#4613]) +3 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/fi-kbl-soraka/igt@gem_lmem_swapping@basic.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][5] ([i915#5334] / [i915#7872])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/fi-kbl-soraka/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@gt_pm:
    - fi-kbl-soraka:      NOTRUN -> [DMESG-FAIL][6] ([i915#1886] / [i915#7913])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/fi-kbl-soraka/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@mman:
    - bat-rpls-2:         [PASS][7] -> [TIMEOUT][8] ([i915#6794] / [i915#7392])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13143/bat-rpls-2/igt@i915_selftest@live@mman.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/bat-rpls-2/igt@i915_selftest@live@mman.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - bat-rpls-2:         NOTRUN -> [ABORT][9] ([i915#6687])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/bat-rpls-2/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@i915_suspend@basic-s3-without-i915:
    - bat-rpls-1:         NOTRUN -> [ABORT][10] ([i915#6687] / [i915#7953] / [i915#7978])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/bat-rpls-1/igt@i915_suspend@basic-s3-without-i915.html

  * igt@kms_chamelium_frames@hdmi-crc-fast:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][11] ([fdo#109271]) +15 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/fi-kbl-soraka/igt@kms_chamelium_frames@hdmi-crc-fast.html

  * igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1:
    - bat-dg2-8:          [PASS][12] -> [FAIL][13] ([i915#7932])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13143/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/bat-dg2-8/igt@kms_pipe_crc_basic@nonblocking-crc@pipe-c-dp-1.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - fi-kbl-soraka:      NOTRUN -> [SKIP][14] ([fdo#109271] / [i915#4579])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/fi-kbl-soraka/igt@kms_setmode@basic-clone-single-crtc.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-apl-guc:         [DMESG-FAIL][15] ([i915#5334]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13143/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html

  * igt@i915_selftest@live@reset:
    - bat-rpls-1:         [ABORT][17] ([i915#4983] / [i915#7461] / [i915#7953] / [i915#8347] / [i915#8384]) -> [PASS][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13143/bat-rpls-1/igt@i915_selftest@live@reset.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/bat-rpls-1/igt@i915_selftest@live@reset.html

  
#### Warnings ####

  * igt@kms_psr@sprite_plane_onoff:
    - bat-rplp-1:         [SKIP][19] ([i915#1072]) -> [ABORT][20] ([i915#8442])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13143/bat-rplp-1/igt@kms_psr@sprite_plane_onoff.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/bat-rplp-1/igt@kms_psr@sprite_plane_onoff.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1886]: https://gitlab.freedesktop.org/drm/intel/issues/1886
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334
  [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
  [i915#6794]: https://gitlab.freedesktop.org/drm/intel/issues/6794
  [i915#7392]: https://gitlab.freedesktop.org/drm/intel/issues/7392
  [i915#7461]: https://gitlab.freedesktop.org/drm/intel/issues/7461
  [i915#7872]: https://gitlab.freedesktop.org/drm/intel/issues/7872
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913
  [i915#7920]: https://gitlab.freedesktop.org/drm/intel/issues/7920
  [i915#7932]: https://gitlab.freedesktop.org/drm/intel/issues/7932
  [i915#7953]: https://gitlab.freedesktop.org/drm/intel/issues/7953
  [i915#7978]: https://gitlab.freedesktop.org/drm/intel/issues/7978
  [i915#8347]: https://gitlab.freedesktop.org/drm/intel/issues/8347
  [i915#8384]: https://gitlab.freedesktop.org/drm/intel/issues/8384
  [i915#8442]: https://gitlab.freedesktop.org/drm/intel/issues/8442


Build changes
-------------

  * Linux: CI_DRM_13143 -> Patchwork_117717v1

  CI-20190529: 20190529
  CI_DRM_13143: 222ff19f23b0bd6aca0b52001d69699f78f5a206 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7286: a482779488f11c432d7ddcb1980691ab1603f356 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_117717v1: 222ff19f23b0bd6aca0b52001d69699f78f5a206 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

4de5bb0eeb47 drm/i915/mtl: Add MTL performance tuning changes
79c0e1695e64 drm/i915/mtl: Extend Wa_16014892111 to MTL A-step

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_117717v1/index.html

[-- Attachment #2: Type: text/html, Size: 8840 bytes --]

Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/mtl: Extend Wa_16014892111 to MTL A-step

[Kalvala, Haridhar]

On 5/13/2023 7:44 AM, Radhakrishna Sripada wrote:
> The dg2 workaround which is used for performance tuning
> is needed for Meteorlake A-step.
>
> v2: Limit the WA for A-step
>
> Bspec: 68331
> Cc: Haridhar Kalvala <haridhar.kalvala@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Gustavo Sousa <gustavo.sousa@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
> index 81a96c52a92b..9c1007c44298 100644
> --- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> @@ -1370,7 +1370,9 @@ gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs)
>   					      cs, GEN12_GFX_CCS_AUX_NV);
>   
>   	/* Wa_16014892111 */
> -	if (IS_DG2(ce->engine->i915))
> +	if (IS_DG2(ce->engine->i915) ||
> +	    IS_MTL_GRAPHICS_STEP(ce->engine->i915, M, STEP_A0, STEP_B0) ||
> +	    IS_MTL_GRAPHICS_STEP(ce->engine->i915, P, STEP_A0, STEP_B0))
>   		cs = dg2_emit_draw_watermark_setting(cs);
>   

looks good to me.

Acked-by: Haridhar Kalvala <haridhar.kalvala@intel.com>

>   	return cs;

-- 
Regards,
Haridhar Kalvala

Re: [Intel-gfx] [PATCH v2 2/2] drm/i915/mtl: Add MTL performance tuning changes

[Kalvala, Haridhar]

On 5/13/2023 7:44 AM, Radhakrishna Sripada wrote:
> MTL reuses the tuning parameters for DG2. Extend the dg2
> performance tuning parameters to MTL.
>
> v2: Add DRAW_WATERMARK tuning parameter.
>
> Bspec: 68331
> Cc: Haridhar Kalvala <haridhar.kalvala@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Gustavo Sousa <gustavo.sousa@intel.com>
> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/intel_workarounds.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 786349e95487..72dab970de5b 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -817,6 +817,10 @@ static void mtl_ctx_workarounds_init(struct intel_engine_cs *engine,
>   {
>   	struct drm_i915_private *i915 = engine->i915;
>   
> +	dg2_ctx_gt_tuning_init(engine, wal);
> +
> +	wa_add(wal, DRAW_WATERMARK, VERT_WM_VAL, 0x3FF, 0, false);

Hi RK,

I am not sure but have doubt w.r.t DRAW_WATERMARK are we not doing same 
in [ Patch V2 1/2].

Thank you,

Haridhar Kalvala

> +
>   	if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) ||
>   	    IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) {
>   		/* Wa_14014947963 */
> @@ -1754,7 +1758,7 @@ static void gt_tuning_settings(struct intel_gt *gt, struct i915_wa_list *wal)
>   		wa_mcr_masked_en(wal, XEHPC_LNCFMISCCFGREG0, XEHPC_HOSTCACHEEN);
>   	}
>   
> -	if (IS_DG2(gt->i915)) {
> +	if (IS_DG2(gt->i915) || IS_METEORLAKE(gt->i915)) {
>   		wa_mcr_write_or(wal, XEHP_L3SCQREG7, BLEND_FILL_CACHING_OPT_DIS);
>   		wa_mcr_write_or(wal, XEHP_SQCM, EN_32B_ACCESS);
>   	}
> @@ -2944,7 +2948,7 @@ static void
>   add_render_compute_tuning_settings(struct drm_i915_private *i915,
>   				   struct i915_wa_list *wal)
>   {
> -	if (IS_DG2(i915))
> +	if (IS_DG2(i915) || IS_METEORLAKE(i915))
>   		wa_mcr_write_clr_set(wal, RT_CTRL, STACKID_CTRL, STACKID_CTRL_512);
>   
>   	/*

-- 
Regards,
Haridhar Kalvala

Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/mtl: Extend Wa_16014892111 to MTL A-step

[Gustavo Sousa]

Quoting Radhakrishna Sripada (2023-05-12 23:14:37)
>The dg2 workaround which is used for performance tuning
>is needed for Meteorlake A-step.
>
>v2: Limit the WA for A-step

I think what Matt meant in the review for v1 was that this commit should
be rather about the tuning setting rather than the workaround itself. As
such, maybe we should change the commit message so that it focus on the
recommended tuning setting, i.e., instead of "Extend Wa_16014892111 to
MTL A-step" as subject, we should write something like "Apply
recommended tuning setting for ..." and give details.

That said, since we are focusing on the tuning settings here, I guess
this could be squashed with the second patch and we could add a note
about DRAW_WATERMARK needing Wa_16014892111 for A steps of MTL.

--
Gustavo Sousa

>
>Bspec: 68331
>Cc: Haridhar Kalvala <haridhar.kalvala@intel.com>
>Cc: Matt Roper <matthew.d.roper@intel.com>
>Cc: Gustavo Sousa <gustavo.sousa@intel.com>
>Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
>---
> drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c b/drivers/gpu/drm/i915/gt/intel_lrc.c
>index 81a96c52a92b..9c1007c44298 100644
>--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
>+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
>@@ -1370,7 +1370,9 @@ gen12_emit_indirect_ctx_rcs(const struct intel_context *ce, u32 *cs)
>                                               cs, GEN12_GFX_CCS_AUX_NV);
> 
>         /* Wa_16014892111 */
>-        if (IS_DG2(ce->engine->i915))
>+        if (IS_DG2(ce->engine->i915) ||
>+            IS_MTL_GRAPHICS_STEP(ce->engine->i915, M, STEP_A0, STEP_B0) ||
>+            IS_MTL_GRAPHICS_STEP(ce->engine->i915, P, STEP_A0, STEP_B0))
>                 cs = dg2_emit_draw_watermark_setting(cs);
> 
>         return cs;
>-- 
>2.34.1
>

Re: [Intel-gfx] [PATCH v2 2/2] drm/i915/mtl: Add MTL performance tuning changes

[Gustavo Sousa]

Quoting Kalvala, Haridhar (2023-05-14 08:13:10)
>
>On 5/13/2023 7:44 AM, Radhakrishna Sripada wrote:
>> MTL reuses the tuning parameters for DG2. Extend the dg2
>> performance tuning parameters to MTL.
>>
>> v2: Add DRAW_WATERMARK tuning parameter.
>>
>> Bspec: 68331
>> Cc: Haridhar Kalvala <haridhar.kalvala@intel.com>
>> Cc: Matt Roper <matthew.d.roper@intel.com>
>> Cc: Gustavo Sousa <gustavo.sousa@intel.com>
>> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/intel_workarounds.c | 8 ++++++--
>>   1 file changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> index 786349e95487..72dab970de5b 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
>> @@ -817,6 +817,10 @@ static void mtl_ctx_workarounds_init(struct intel_engine_cs *engine,
>>   {
>>           struct drm_i915_private *i915 = engine->i915;
>>   
>> +        dg2_ctx_gt_tuning_init(engine, wal);
>> +
>> +        wa_add(wal, DRAW_WATERMARK, VERT_WM_VAL, 0x3FF, 0, false);
>
>Hi RK,
>
>I am not sure but have doubt w.r.t DRAW_WATERMARK are we not doing same 
>in [ Patch V2 1/2].
>
>Thank you,
>
>Haridhar Kalvala

Yeah, I think this should be executed only for B0+ steps.

--
Gustavo Sousa

>
>> +
>>           if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) ||
>>               IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) {
>>                   /* Wa_14014947963 */
>> @@ -1754,7 +1758,7 @@ static void gt_tuning_settings(struct intel_gt *gt, struct i915_wa_list *wal)
>>                   wa_mcr_masked_en(wal, XEHPC_LNCFMISCCFGREG0, XEHPC_HOSTCACHEEN);
>>           }
>>   
>> -        if (IS_DG2(gt->i915)) {
>> +        if (IS_DG2(gt->i915) || IS_METEORLAKE(gt->i915)) {
>>                   wa_mcr_write_or(wal, XEHP_L3SCQREG7, BLEND_FILL_CACHING_OPT_DIS);
>>                   wa_mcr_write_or(wal, XEHP_SQCM, EN_32B_ACCESS);
>>           }
>> @@ -2944,7 +2948,7 @@ static void
>>   add_render_compute_tuning_settings(struct drm_i915_private *i915,
>>                                      struct i915_wa_list *wal)
>>   {
>> -        if (IS_DG2(i915))
>> +        if (IS_DG2(i915) || IS_METEORLAKE(i915))
>>                   wa_mcr_write_clr_set(wal, RT_CTRL, STACKID_CTRL, STACKID_CTRL_512);
>>   
>>           /*
>
>-- 
>Regards,
>Haridhar Kalvala
>

Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/mtl: Extend Wa_16014892111 to MTL A-step

[Sripada, Radhakrishna]

Hi Gustavo,

> -----Original Message-----
> From: Sousa, Gustavo <gustavo.sousa@intel.com>
> Sent: Monday, May 15, 2023 7:45 AM
> To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; intel-
> gfx@lists.freedesktop.org
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Sripada, Radhakrishna
> <radhakrishna.sripada@intel.com>; Kalvala, Haridhar
> <haridhar.kalvala@intel.com>; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: Re: [PATCH v2 1/2] drm/i915/mtl: Extend Wa_16014892111 to MTL A-
> step
> 
> Quoting Radhakrishna Sripada (2023-05-12 23:14:37)
> >The dg2 workaround which is used for performance tuning
> >is needed for Meteorlake A-step.
> >
> >v2: Limit the WA for A-step
> 
> I think what Matt meant in the review for v1 was that this commit should
> be rather about the tuning setting rather than the workaround itself. As
> such, maybe we should change the commit message so that it focus on the
> recommended tuning setting, i.e., instead of "Extend Wa_16014892111 to
> MTL A-step" as subject, we should write something like "Apply
> recommended tuning setting for ..." and give details.
> 
> That said, since we are focusing on the tuning settings here, I guess
> this could be squashed with the second patch and we could add a note
> about DRAW_WATERMARK needing Wa_16014892111 for A steps of MTL.

There are 2 aspects wrt. DRAW_WATERMARK. One that is a workaround which is applied
on each context switch and is only applicable for DG2 and MTL-A step which is what this patch does.

The other is the tuning parameter setting which is applicable for all of MTL which is a onetime configuration
Handled by the next patch during ctx_workarounds_init.

- Radhakrishna Sripada


> 
> --
> Gustavo Sousa
> 
> >
> >Bspec: 68331
> >Cc: Haridhar Kalvala <haridhar.kalvala@intel.com>
> >Cc: Matt Roper <matthew.d.roper@intel.com>
> >Cc: Gustavo Sousa <gustavo.sousa@intel.com>
> >Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> >---
> > drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> >diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> b/drivers/gpu/drm/i915/gt/intel_lrc.c
> >index 81a96c52a92b..9c1007c44298 100644
> >--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> >+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> >@@ -1370,7 +1370,9 @@ gen12_emit_indirect_ctx_rcs(const struct
> intel_context *ce, u32 *cs)
> >                                               cs, GEN12_GFX_CCS_AUX_NV);
> >
> >         /* Wa_16014892111 */
> >-        if (IS_DG2(ce->engine->i915))
> >+        if (IS_DG2(ce->engine->i915) ||
> >+            IS_MTL_GRAPHICS_STEP(ce->engine->i915, M, STEP_A0, STEP_B0) ||
> >+            IS_MTL_GRAPHICS_STEP(ce->engine->i915, P, STEP_A0, STEP_B0))
> >                 cs = dg2_emit_draw_watermark_setting(cs);
> >
> >         return cs;
> >--
> >2.34.1
> >

Re: [Intel-gfx] [PATCH v2 2/2] drm/i915/mtl: Add MTL performance tuning changes

[Sripada, Radhakrishna]

Hi Haridar/Gustavo,

> -----Original Message-----
> From: Sousa, Gustavo <gustavo.sousa@intel.com>
> Sent: Monday, May 15, 2023 7:47 AM
> To: Kalvala, Haridhar <haridhar.kalvala@intel.com>; Sripada, Radhakrishna
> <radhakrishna.sripada@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Roper, Matthew D
> <matthew.d.roper@intel.com>
> Subject: Re: [PATCH v2 2/2] drm/i915/mtl: Add MTL performance tuning
> changes
> 
> Quoting Kalvala, Haridhar (2023-05-14 08:13:10)
> >
> >On 5/13/2023 7:44 AM, Radhakrishna Sripada wrote:
> >> MTL reuses the tuning parameters for DG2. Extend the dg2
> >> performance tuning parameters to MTL.
> >>
> >> v2: Add DRAW_WATERMARK tuning parameter.
> >>
> >> Bspec: 68331
> >> Cc: Haridhar Kalvala <haridhar.kalvala@intel.com>
> >> Cc: Matt Roper <matthew.d.roper@intel.com>
> >> Cc: Gustavo Sousa <gustavo.sousa@intel.com>
> >> Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/gt/intel_workarounds.c | 8 ++++++--
> >>   1 file changed, 6 insertions(+), 2 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> >> index 786349e95487..72dab970de5b 100644
> >> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> >> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> >> @@ -817,6 +817,10 @@ static void mtl_ctx_workarounds_init(struct
> intel_engine_cs *engine,
> >>   {
> >>           struct drm_i915_private *i915 = engine->i915;
> >>
> >> +        dg2_ctx_gt_tuning_init(engine, wal);
> >> +
> >> +        wa_add(wal, DRAW_WATERMARK, VERT_WM_VAL, 0x3FF, 0, false);
> >
> >Hi RK,
> >
> >I am not sure but have doubt w.r.t DRAW_WATERMARK are we not doing same
> >in [ Patch V2 1/2].
> >
> >Thank you,
> >
> >Haridhar Kalvala
> 
> Yeah, I think this should be executed only for B0+ steps.

This is a onetime tuning parameter configuration applicable across all MTL.
The wa limitation is independent of the tuning parameter configuration.

- Radhakrishna Sripada
> 
> --
> Gustavo Sousa
> 
> >
> >> +
> >>           if (IS_MTL_GRAPHICS_STEP(i915, M, STEP_A0, STEP_B0) ||
> >>               IS_MTL_GRAPHICS_STEP(i915, P, STEP_A0, STEP_B0)) {
> >>                   /* Wa_14014947963 */
> >> @@ -1754,7 +1758,7 @@ static void gt_tuning_settings(struct intel_gt *gt,
> struct i915_wa_list *wal)
> >>                   wa_mcr_masked_en(wal, XEHPC_LNCFMISCCFGREG0,
> XEHPC_HOSTCACHEEN);
> >>           }
> >>
> >> -        if (IS_DG2(gt->i915)) {
> >> +        if (IS_DG2(gt->i915) || IS_METEORLAKE(gt->i915)) {
> >>                   wa_mcr_write_or(wal, XEHP_L3SCQREG7,
> BLEND_FILL_CACHING_OPT_DIS);
> >>                   wa_mcr_write_or(wal, XEHP_SQCM, EN_32B_ACCESS);
> >>           }
> >> @@ -2944,7 +2948,7 @@ static void
> >>   add_render_compute_tuning_settings(struct drm_i915_private *i915,
> >>                                      struct i915_wa_list *wal)
> >>   {
> >> -        if (IS_DG2(i915))
> >> +        if (IS_DG2(i915) || IS_METEORLAKE(i915))
> >>                   wa_mcr_write_clr_set(wal, RT_CTRL, STACKID_CTRL,
> STACKID_CTRL_512);
> >>
> >>           /*
> >
> >--
> >Regards,
> >Haridhar Kalvala
> >

Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/mtl: Extend Wa_16014892111 to MTL A-step

[Matt Roper]

On Mon, May 15, 2023 at 08:42:25AM -0700, Sripada, Radhakrishna wrote:
> Hi Gustavo,
> 
> > -----Original Message-----
> > From: Sousa, Gustavo <gustavo.sousa@intel.com>
> > Sent: Monday, May 15, 2023 7:45 AM
> > To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; intel-
> > gfx@lists.freedesktop.org
> > Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Sripada, Radhakrishna
> > <radhakrishna.sripada@intel.com>; Kalvala, Haridhar
> > <haridhar.kalvala@intel.com>; Roper, Matthew D
> > <matthew.d.roper@intel.com>
> > Subject: Re: [PATCH v2 1/2] drm/i915/mtl: Extend Wa_16014892111 to MTL A-
> > step
> > 
> > Quoting Radhakrishna Sripada (2023-05-12 23:14:37)
> > >The dg2 workaround which is used for performance tuning
> > >is needed for Meteorlake A-step.
> > >
> > >v2: Limit the WA for A-step
> > 
> > I think what Matt meant in the review for v1 was that this commit should
> > be rather about the tuning setting rather than the workaround itself. As
> > such, maybe we should change the commit message so that it focus on the
> > recommended tuning setting, i.e., instead of "Extend Wa_16014892111 to
> > MTL A-step" as subject, we should write something like "Apply
> > recommended tuning setting for ..." and give details.
> > 
> > That said, since we are focusing on the tuning settings here, I guess
> > this could be squashed with the second patch and we could add a note
> > about DRAW_WATERMARK needing Wa_16014892111 for A steps of MTL.
> 
> There are 2 aspects wrt. DRAW_WATERMARK. One that is a workaround
> which is applied on each context switch and is only applicable for DG2
> and MTL-A step which is what this patch does.

So just to be clear --- the workaround doesn't directly ask us to do
anything specific with DRAW_WATERMARK.  What the workaround says is that
*if* we wind up needing to change the value of DRAW_WATERMARK away from
the hardware default, then we need to handle the save/restore on each
context switch ourselves since the hardware doesn't process this
register properly on context switch and will lose its value.

It turns out that MTL has a tuning setting that suggests changing
DRAW_WATERMARK to a non-default value.  Since the tuning setting is
constant (i.e., it doesn't change at runtime based on other factors), we
can ignore the "save" part of the workaround and just hardcode the
"restore" part to the value specified by the tuning setting.  But what
we're programming here is still the tuning setting, it's just that the
way we implement the tuning is adjusted by the workaround's guidance.

It might make sense to swap the order of these patches --- make the
first patch add the tuning setting (in the normal manner) for all
steppings not impacted by the workaround.  Then come back and apply the
tuning setting in the special way on the A-step hardware to satisfy the
guidance of Wa_16014892111.  Or maybe it's simpler to just ignore the
tuning setting on A-step entirely; that's a pre-production stepping of
the platform, so it's not really going to get used for performance work
anyway.  If we don't bother programming the tuning on A-step, then we
also don't need to worry about the workaround either.


Matt

> 
> The other is the tuning parameter setting which is applicable for all
> of MTL which is a onetime configuration Handled by the next patch
> during ctx_workarounds_init.
> 
> - Radhakrishna Sripada
> 
> 
> > 
> > --
> > Gustavo Sousa
> > 
> > >
> > >Bspec: 68331
> > >Cc: Haridhar Kalvala <haridhar.kalvala@intel.com>
> > >Cc: Matt Roper <matthew.d.roper@intel.com>
> > >Cc: Gustavo Sousa <gustavo.sousa@intel.com>
> > >Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > >---
> > > drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +++-
> > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > >
> > >diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > >index 81a96c52a92b..9c1007c44298 100644
> > >--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > >+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > >@@ -1370,7 +1370,9 @@ gen12_emit_indirect_ctx_rcs(const struct
> > intel_context *ce, u32 *cs)
> > >                                               cs, GEN12_GFX_CCS_AUX_NV);
> > >
> > >         /* Wa_16014892111 */
> > >-        if (IS_DG2(ce->engine->i915))
> > >+        if (IS_DG2(ce->engine->i915) ||
> > >+            IS_MTL_GRAPHICS_STEP(ce->engine->i915, M, STEP_A0, STEP_B0) ||
> > >+            IS_MTL_GRAPHICS_STEP(ce->engine->i915, P, STEP_A0, STEP_B0))
> > >                 cs = dg2_emit_draw_watermark_setting(cs);
> > >
> > >         return cs;
> > >--
> > >2.34.1
> > >

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

Re: [Intel-gfx] [PATCH v2 1/2] drm/i915/mtl: Extend Wa_16014892111 to MTL A-step

[Sripada, Radhakrishna]

Hi Matt,

> -----Original Message-----
> From: Roper, Matthew D <matthew.d.roper@intel.com>
> Sent: Monday, May 15, 2023 10:28 AM
> To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>
> Cc: Sousa, Gustavo <gustavo.sousa@intel.com>; intel-
> gfx@lists.freedesktop.org; Justen, Jordan L <jordan.l.justen@intel.com>;
> Kalvala, Haridhar <haridhar.kalvala@intel.com>
> Subject: Re: [PATCH v2 1/2] drm/i915/mtl: Extend Wa_16014892111 to MTL A-
> step
> 
> On Mon, May 15, 2023 at 08:42:25AM -0700, Sripada, Radhakrishna wrote:
> > Hi Gustavo,
> >
> > > -----Original Message-----
> > > From: Sousa, Gustavo <gustavo.sousa@intel.com>
> > > Sent: Monday, May 15, 2023 7:45 AM
> > > To: Sripada, Radhakrishna <radhakrishna.sripada@intel.com>; intel-
> > > gfx@lists.freedesktop.org
> > > Cc: Justen, Jordan L <jordan.l.justen@intel.com>; Sripada, Radhakrishna
> > > <radhakrishna.sripada@intel.com>; Kalvala, Haridhar
> > > <haridhar.kalvala@intel.com>; Roper, Matthew D
> > > <matthew.d.roper@intel.com>
> > > Subject: Re: [PATCH v2 1/2] drm/i915/mtl: Extend Wa_16014892111 to MTL
> A-
> > > step
> > >
> > > Quoting Radhakrishna Sripada (2023-05-12 23:14:37)
> > > >The dg2 workaround which is used for performance tuning
> > > >is needed for Meteorlake A-step.
> > > >
> > > >v2: Limit the WA for A-step
> > >
> > > I think what Matt meant in the review for v1 was that this commit should
> > > be rather about the tuning setting rather than the workaround itself. As
> > > such, maybe we should change the commit message so that it focus on the
> > > recommended tuning setting, i.e., instead of "Extend Wa_16014892111 to
> > > MTL A-step" as subject, we should write something like "Apply
> > > recommended tuning setting for ..." and give details.
> > >
> > > That said, since we are focusing on the tuning settings here, I guess
> > > this could be squashed with the second patch and we could add a note
> > > about DRAW_WATERMARK needing Wa_16014892111 for A steps of MTL.
> >
> > There are 2 aspects wrt. DRAW_WATERMARK. One that is a workaround
> > which is applied on each context switch and is only applicable for DG2
> > and MTL-A step which is what this patch does.
> 
> So just to be clear --- the workaround doesn't directly ask us to do
> anything specific with DRAW_WATERMARK.  What the workaround says is that
> *if* we wind up needing to change the value of DRAW_WATERMARK away from
> the hardware default, then we need to handle the save/restore on each
> context switch ourselves since the hardware doesn't process this
> register properly on context switch and will lose its value.
> 
> It turns out that MTL has a tuning setting that suggests changing
> DRAW_WATERMARK to a non-default value.  Since the tuning setting is
> constant (i.e., it doesn't change at runtime based on other factors), we
> can ignore the "save" part of the workaround and just hardcode the
> "restore" part to the value specified by the tuning setting.  But what
> we're programming here is still the tuning setting, it's just that the
> way we implement the tuning is adjusted by the workaround's guidance.
> 
> It might make sense to swap the order of these patches --- make the
> first patch add the tuning setting (in the normal manner) for all
> steppings not impacted by the workaround.  Then come back and apply the
> tuning setting in the special way on the A-step hardware to satisfy the
> guidance of Wa_16014892111.  Or maybe it's simpler to just ignore the
> tuning setting on A-step entirely; that's a pre-production stepping of
> the platform, so it's not really going to get used for performance work
> anyway.  If we don't bother programming the tuning on A-step, then we
> also don't need to worry about the workaround either.

Thank you for the explanation. I was inclined to drop the WA but we do have
B-step parts in CI which have A-step GT hence trying to keep this around.

I resend the patches swapping the order and adding better explanation.

- Radhakrishna Sripada
> 
> 
> Matt
> 
> >
> > The other is the tuning parameter setting which is applicable for all
> > of MTL which is a onetime configuration Handled by the next patch
> > during ctx_workarounds_init.
> >
> > - Radhakrishna Sripada
> >
> >
> > >
> > > --
> > > Gustavo Sousa
> > >
> > > >
> > > >Bspec: 68331
> > > >Cc: Haridhar Kalvala <haridhar.kalvala@intel.com>
> > > >Cc: Matt Roper <matthew.d.roper@intel.com>
> > > >Cc: Gustavo Sousa <gustavo.sousa@intel.com>
> > > >Signed-off-by: Radhakrishna Sripada <radhakrishna.sripada@intel.com>
> > > >---
> > > > drivers/gpu/drm/i915/gt/intel_lrc.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > >diff --git a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > > b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > > >index 81a96c52a92b..9c1007c44298 100644
> > > >--- a/drivers/gpu/drm/i915/gt/intel_lrc.c
> > > >+++ b/drivers/gpu/drm/i915/gt/intel_lrc.c
> > > >@@ -1370,7 +1370,9 @@ gen12_emit_indirect_ctx_rcs(const struct
> > > intel_context *ce, u32 *cs)
> > > >                                               cs, GEN12_GFX_CCS_AUX_NV);
> > > >
> > > >         /* Wa_16014892111 */
> > > >-        if (IS_DG2(ce->engine->i915))
> > > >+        if (IS_DG2(ce->engine->i915) ||
> > > >+            IS_MTL_GRAPHICS_STEP(ce->engine->i915, M, STEP_A0, STEP_B0)
> ||
> > > >+            IS_MTL_GRAPHICS_STEP(ce->engine->i915, P, STEP_A0, STEP_B0))
> > > >                 cs = dg2_emit_draw_watermark_setting(cs);
> > > >
> > > >         return cs;
> > > >--
> > > >2.34.1
> > > >
> 
> --
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation