intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH] drm/i915/mtl: Add support for 32 bit OAG formats in MTL
@ 2022-11-29  1:21 Umesh Nerlige Ramappa
  2022-11-29  2:20 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2022-11-29  1:21 UTC (permalink / raw)
  To: intel-gfx

As part of OA support for MTL,

- Enable 32 bit OAG formats for MTL.
- 0x200c is repurposed on MTL. Use a separate mux table to verify oa
  configs passed by user.
- Similar to ACM, OA/CS timestamp is mismatched on MTL. Add MTL to the
  WA.
- On MTL, gt->scratch was using stolen lmem. An MI_SRM to stolen lmem is
  hanging. Add a page in noa_wait BO to save/restore GPR registers for
  the noa_wait logic.

Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_gt_types.h |  6 ---
 drivers/gpu/drm/i915/i915_perf.c         | 49 ++++++++++++++++++------
 2 files changed, 38 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
index c1d9cd255e06..13dffe0a3d20 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
@@ -296,12 +296,6 @@ enum intel_gt_scratch_field {
 
 	/* 8 bytes */
 	INTEL_GT_SCRATCH_FIELD_COHERENTL3_WA = 256,
-
-	/* 6 * 8 bytes */
-	INTEL_GT_SCRATCH_FIELD_PERF_CS_GPR = 2048,
-
-	/* 4 bytes */
-	INTEL_GT_SCRATCH_FIELD_PERF_PREDICATE_RESULT_1 = 2096,
 };
 
 #endif /* __INTEL_GT_TYPES_H__ */
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 00e09bb18b13..a735b9540113 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -1842,8 +1842,7 @@ static u32 *save_restore_register(struct i915_perf_stream *stream, u32 *cs,
 	for (d = 0; d < dword_count; d++) {
 		*cs++ = cmd;
 		*cs++ = i915_mmio_reg_offset(reg) + 4 * d;
-		*cs++ = intel_gt_scratch_offset(stream->engine->gt,
-						offset) + 4 * d;
+		*cs++ = i915_ggtt_offset(stream->noa_wait) + offset + 4 * d;
 		*cs++ = 0;
 	}
 
@@ -1876,7 +1875,13 @@ static int alloc_noa_wait(struct i915_perf_stream *stream)
 					  MI_PREDICATE_RESULT_2_ENGINE(base) :
 					  MI_PREDICATE_RESULT_1(RENDER_RING_BASE);
 
-	bo = i915_gem_object_create_internal(i915, 4096);
+	/*
+	 * gt->scratch was being used to save/restore the GPR registers, but on
+	 * MTL the scratch uses stolen lmem. An MI_SRM to this memory region
+	 * causes an engine hang. Instead allocate an additional page here to
+	 * save/restore GPR registers
+	 */
+	bo = i915_gem_object_create_internal(i915, 8192);
 	if (IS_ERR(bo)) {
 		drm_err(&i915->drm,
 			"Failed to allocate NOA wait batchbuffer\n");
@@ -1910,14 +1915,19 @@ static int alloc_noa_wait(struct i915_perf_stream *stream)
 		goto err_unpin;
 	}
 
+	stream->noa_wait = vma;
+
+#define GPR_SAVE_OFFSET 4096
+#define PREDICATE_SAVE_OFFSET 4160
+
 	/* Save registers. */
 	for (i = 0; i < N_CS_GPR; i++)
 		cs = save_restore_register(
 			stream, cs, true /* save */, CS_GPR(i),
-			INTEL_GT_SCRATCH_FIELD_PERF_CS_GPR + 8 * i, 2);
+			GPR_SAVE_OFFSET + 8 * i, 2);
 	cs = save_restore_register(
 		stream, cs, true /* save */, mi_predicate_result,
-		INTEL_GT_SCRATCH_FIELD_PERF_PREDICATE_RESULT_1, 1);
+		PREDICATE_SAVE_OFFSET, 1);
 
 	/* First timestamp snapshot location. */
 	ts0 = cs;
@@ -2033,10 +2043,10 @@ static int alloc_noa_wait(struct i915_perf_stream *stream)
 	for (i = 0; i < N_CS_GPR; i++)
 		cs = save_restore_register(
 			stream, cs, false /* restore */, CS_GPR(i),
-			INTEL_GT_SCRATCH_FIELD_PERF_CS_GPR + 8 * i, 2);
+			GPR_SAVE_OFFSET + 8 * i, 2);
 	cs = save_restore_register(
 		stream, cs, false /* restore */, mi_predicate_result,
-		INTEL_GT_SCRATCH_FIELD_PERF_PREDICATE_RESULT_1, 1);
+		PREDICATE_SAVE_OFFSET, 1);
 
 	/* And return to the ring. */
 	*cs++ = MI_BATCH_BUFFER_END;
@@ -2046,7 +2056,6 @@ static int alloc_noa_wait(struct i915_perf_stream *stream)
 	i915_gem_object_flush_map(bo);
 	__i915_gem_object_release_map(bo);
 
-	stream->noa_wait = vma;
 	goto out_ww;
 
 err_unpin:
@@ -3127,8 +3136,11 @@ get_sseu_config(struct intel_sseu *out_sseu,
  */
 u32 i915_perf_oa_timestamp_frequency(struct drm_i915_private *i915)
 {
-	/* Wa_18013179988:dg2 */
-	if (IS_DG2(i915)) {
+	/*
+	 * Wa_18013179988:dg2
+	 * Wa_14015846243:mtl
+	 */
+	if (IS_DG2(i915) || IS_METEORLAKE(i915)) {
 		intel_wakeref_t wakeref;
 		u32 reg, shift;
 
@@ -4306,6 +4318,17 @@ static const struct i915_range gen12_oa_mux_regs[] = {
 	{}
 };
 
+/*
+ * Ref: 14010536224:
+ * 0x20cc is repurposed on MTL, so use a separate array for MTL.
+ */
+static const struct i915_range mtl_oa_mux_regs[] = {
+	{ .start = 0x0d00, .end = 0x0d04 },	/* RPM_CONFIG[0-1] */
+	{ .start = 0x0d0c, .end = 0x0d2c },	/* NOA_CONFIG[0-8] */
+	{ .start = 0x9840, .end = 0x9840 },	/* GDT_CHICKEN_BITS */
+	{ .start = 0x9884, .end = 0x9888 },	/* NOA_WRITE */
+};
+
 static bool gen7_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
 {
 	return reg_in_range_table(addr, gen7_oa_b_counters);
@@ -4349,7 +4372,10 @@ static bool xehp_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
 
 static bool gen12_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
 {
-	return reg_in_range_table(addr, gen12_oa_mux_regs);
+	if (IS_METEORLAKE(perf->i915))
+		return reg_in_range_table(addr, mtl_oa_mux_regs);
+	else
+		return reg_in_range_table(addr, gen12_oa_mux_regs);
 }
 
 static u32 mask_reg_value(u32 reg, u32 val)
@@ -4746,6 +4772,7 @@ static void oa_init_supported_formats(struct i915_perf *perf)
 		break;
 
 	case INTEL_DG2:
+	case INTEL_METEORLAKE:
 		oa_format_add(perf, I915_OAR_FORMAT_A32u40_A4u32_B8_C8);
 		oa_format_add(perf, I915_OA_FORMAT_A24u40_A14u32_B8_C8);
 		break;
-- 
2.36.1


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

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/mtl: Add support for 32 bit OAG formats in MTL
  2022-11-29  1:21 [Intel-gfx] [PATCH] drm/i915/mtl: Add support for 32 bit OAG formats in MTL Umesh Nerlige Ramappa
@ 2022-11-29  2:20 ` Patchwork
  2022-11-29  2:45 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
                   ` (3 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2022-11-29  2:20 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/mtl: Add support for 32 bit OAG formats in MTL
URL   : https://patchwork.freedesktop.org/series/111411/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.



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

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/mtl: Add support for 32 bit OAG formats in MTL
  2022-11-29  1:21 [Intel-gfx] [PATCH] drm/i915/mtl: Add support for 32 bit OAG formats in MTL Umesh Nerlige Ramappa
  2022-11-29  2:20 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork
@ 2022-11-29  2:45 ` Patchwork
  2022-11-29  7:15 ` [Intel-gfx] [PATCH] " Lucas De Marchi
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2022-11-29  2:45 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: intel-gfx

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

== Series Details ==

Series: drm/i915/mtl: Add support for 32 bit OAG formats in MTL
URL   : https://patchwork.freedesktop.org/series/111411/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_12440 -> Patchwork_111411v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_111411v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_111411v1, 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_111411v1/index.html

Participating hosts (34 -> 31)
------------------------------

  Missing    (3): bat-kbl-2 bat-adlp-4 bat-jsl-3 

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

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

### CI changes ###

#### Possible regressions ####

  * boot:
    - fi-ilk-650:         [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-ilk-650/boot.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111411v1/fi-ilk-650/boot.html

  

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@gem_exec_suspend@basic-s0@smem:
    - {bat-rpls-1}:       NOTRUN -> [DMESG-WARN][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111411v1/bat-rpls-1/igt@gem_exec_suspend@basic-s0@smem.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@core_hotunplug@unbind-rebind:
    - fi-apl-guc:         [PASS][4] -> [INCOMPLETE][5] ([i915#7073])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-apl-guc/igt@core_hotunplug@unbind-rebind.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111411v1/fi-apl-guc/igt@core_hotunplug@unbind-rebind.html

  
#### Possible fixes ####

  * igt@gem_tiled_blits@basic:
    - fi-pnv-d510:        [SKIP][6] ([fdo#109271]) -> [PASS][7] +1 similar issue
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/fi-pnv-d510/igt@gem_tiled_blits@basic.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111411v1/fi-pnv-d510/igt@gem_tiled_blits@basic.html

  * igt@i915_module_load@reload:
    - {bat-rpls-2}:       [INCOMPLETE][8] ([i915#6434]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-rpls-2/igt@i915_module_load@reload.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111411v1/bat-rpls-2/igt@i915_module_load@reload.html

  * igt@i915_selftest@live@reset:
    - {bat-rpls-1}:       [DMESG-FAIL][10] ([i915#4983]) -> [PASS][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_12440/bat-rpls-1/igt@i915_selftest@live@reset.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_111411v1/bat-rpls-1/igt@i915_selftest@live@reset.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
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#4258]: https://gitlab.freedesktop.org/drm/intel/issues/4258
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4983]: https://gitlab.freedesktop.org/drm/intel/issues/4983
  [i915#6434]: https://gitlab.freedesktop.org/drm/intel/issues/6434
  [i915#6559]: https://gitlab.freedesktop.org/drm/intel/issues/6559
  [i915#6997]: https://gitlab.freedesktop.org/drm/intel/issues/6997
  [i915#7073]: https://gitlab.freedesktop.org/drm/intel/issues/7073
  [i915#7077]: https://gitlab.freedesktop.org/drm/intel/issues/7077
  [i915#7346]: https://gitlab.freedesktop.org/drm/intel/issues/7346


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

  * Linux: CI_DRM_12440 -> Patchwork_111411v1

  CI-20190529: 20190529
  CI_DRM_12440: d21d6474a37e5d43075a24668807ea40a7ee9fc1 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7073: d021d66e389f4a759dc749b5f74f278ecd2e6cbf @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_111411v1: d21d6474a37e5d43075a24668807ea40a7ee9fc1 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

1127d659634b drm/i915/mtl: Add support for 32 bit OAG formats in MTL

== Logs ==

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

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

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

* Re: [Intel-gfx] [PATCH] drm/i915/mtl: Add support for 32 bit OAG formats in MTL
  2022-11-29  1:21 [Intel-gfx] [PATCH] drm/i915/mtl: Add support for 32 bit OAG formats in MTL Umesh Nerlige Ramappa
  2022-11-29  2:20 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork
  2022-11-29  2:45 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
@ 2022-11-29  7:15 ` Lucas De Marchi
  2022-11-29 18:40   ` Umesh Nerlige Ramappa
  2022-11-30  1:17 ` Dixit, Ashutosh
  2022-11-30  1:51 ` Dixit, Ashutosh
  4 siblings, 1 reply; 11+ messages in thread
From: Lucas De Marchi @ 2022-11-29  7:15 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: intel-gfx

On Mon, Nov 28, 2022 at 05:21:46PM -0800, Umesh Nerlige Ramappa wrote:
>As part of OA support for MTL,
>
>- Enable 32 bit OAG formats for MTL.
>- 0x200c is repurposed on MTL. Use a separate mux table to verify oa
>  configs passed by user.
>- Similar to ACM, OA/CS timestamp is mismatched on MTL. Add MTL to the
>  WA.
>- On MTL, gt->scratch was using stolen lmem. An MI_SRM to stolen lmem is
>  hanging. Add a page in noa_wait BO to save/restore GPR registers for
>  the noa_wait logic.

why are all these changes squashed in a single patch?

>
>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>---
> drivers/gpu/drm/i915/gt/intel_gt_types.h |  6 ---
> drivers/gpu/drm/i915/i915_perf.c         | 49 ++++++++++++++++++------
> 2 files changed, 38 insertions(+), 17 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>index c1d9cd255e06..13dffe0a3d20 100644
>--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>@@ -296,12 +296,6 @@ enum intel_gt_scratch_field {
>
> 	/* 8 bytes */
> 	INTEL_GT_SCRATCH_FIELD_COHERENTL3_WA = 256,
>-
>-	/* 6 * 8 bytes */
>-	INTEL_GT_SCRATCH_FIELD_PERF_CS_GPR = 2048,
>-
>-	/* 4 bytes */
>-	INTEL_GT_SCRATCH_FIELD_PERF_PREDICATE_RESULT_1 = 2096,
> };
>
> #endif /* __INTEL_GT_TYPES_H__ */
>diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>index 00e09bb18b13..a735b9540113 100644
>--- a/drivers/gpu/drm/i915/i915_perf.c
>+++ b/drivers/gpu/drm/i915/i915_perf.c
>@@ -1842,8 +1842,7 @@ static u32 *save_restore_register(struct i915_perf_stream *stream, u32 *cs,
> 	for (d = 0; d < dword_count; d++) {
> 		*cs++ = cmd;
> 		*cs++ = i915_mmio_reg_offset(reg) + 4 * d;
>-		*cs++ = intel_gt_scratch_offset(stream->engine->gt,
>-						offset) + 4 * d;
>+		*cs++ = i915_ggtt_offset(stream->noa_wait) + offset + 4 * d;
> 		*cs++ = 0;
> 	}
>
>@@ -1876,7 +1875,13 @@ static int alloc_noa_wait(struct i915_perf_stream *stream)
> 					  MI_PREDICATE_RESULT_2_ENGINE(base) :
> 					  MI_PREDICATE_RESULT_1(RENDER_RING_BASE);
>
>-	bo = i915_gem_object_create_internal(i915, 4096);
>+	/*
>+	 * gt->scratch was being used to save/restore the GPR registers, but on
>+	 * MTL the scratch uses stolen lmem. An MI_SRM to this memory region
>+	 * causes an engine hang. Instead allocate an additional page here to

humn.. is it because of the pte being wrong?  "stolen lmem" in mtl is
still system memory... do we know why we'd need this change?

Lucas De Marchi


>+	 * save/restore GPR registers
>+	 */
>+	bo = i915_gem_object_create_internal(i915, 8192);
> 	if (IS_ERR(bo)) {
> 		drm_err(&i915->drm,
> 			"Failed to allocate NOA wait batchbuffer\n");
>@@ -1910,14 +1915,19 @@ static int alloc_noa_wait(struct i915_perf_stream *stream)
> 		goto err_unpin;
> 	}
>
>+	stream->noa_wait = vma;
>+
>+#define GPR_SAVE_OFFSET 4096
>+#define PREDICATE_SAVE_OFFSET 4160
>+
> 	/* Save registers. */
> 	for (i = 0; i < N_CS_GPR; i++)
> 		cs = save_restore_register(
> 			stream, cs, true /* save */, CS_GPR(i),
>-			INTEL_GT_SCRATCH_FIELD_PERF_CS_GPR + 8 * i, 2);
>+			GPR_SAVE_OFFSET + 8 * i, 2);
> 	cs = save_restore_register(
> 		stream, cs, true /* save */, mi_predicate_result,
>-		INTEL_GT_SCRATCH_FIELD_PERF_PREDICATE_RESULT_1, 1);
>+		PREDICATE_SAVE_OFFSET, 1);
>
> 	/* First timestamp snapshot location. */
> 	ts0 = cs;
>@@ -2033,10 +2043,10 @@ static int alloc_noa_wait(struct i915_perf_stream *stream)
> 	for (i = 0; i < N_CS_GPR; i++)
> 		cs = save_restore_register(
> 			stream, cs, false /* restore */, CS_GPR(i),
>-			INTEL_GT_SCRATCH_FIELD_PERF_CS_GPR + 8 * i, 2);
>+			GPR_SAVE_OFFSET + 8 * i, 2);
> 	cs = save_restore_register(
> 		stream, cs, false /* restore */, mi_predicate_result,
>-		INTEL_GT_SCRATCH_FIELD_PERF_PREDICATE_RESULT_1, 1);
>+		PREDICATE_SAVE_OFFSET, 1);
>
> 	/* And return to the ring. */
> 	*cs++ = MI_BATCH_BUFFER_END;
>@@ -2046,7 +2056,6 @@ static int alloc_noa_wait(struct i915_perf_stream *stream)
> 	i915_gem_object_flush_map(bo);
> 	__i915_gem_object_release_map(bo);
>
>-	stream->noa_wait = vma;
> 	goto out_ww;
>
> err_unpin:
>@@ -3127,8 +3136,11 @@ get_sseu_config(struct intel_sseu *out_sseu,
>  */
> u32 i915_perf_oa_timestamp_frequency(struct drm_i915_private *i915)
> {
>-	/* Wa_18013179988:dg2 */
>-	if (IS_DG2(i915)) {
>+	/*
>+	 * Wa_18013179988:dg2
>+	 * Wa_14015846243:mtl
>+	 */
>+	if (IS_DG2(i915) || IS_METEORLAKE(i915)) {
> 		intel_wakeref_t wakeref;
> 		u32 reg, shift;
>
>@@ -4306,6 +4318,17 @@ static const struct i915_range gen12_oa_mux_regs[] = {
> 	{}
> };
>
>+/*
>+ * Ref: 14010536224:
>+ * 0x20cc is repurposed on MTL, so use a separate array for MTL.
>+ */
>+static const struct i915_range mtl_oa_mux_regs[] = {
>+	{ .start = 0x0d00, .end = 0x0d04 },	/* RPM_CONFIG[0-1] */
>+	{ .start = 0x0d0c, .end = 0x0d2c },	/* NOA_CONFIG[0-8] */
>+	{ .start = 0x9840, .end = 0x9840 },	/* GDT_CHICKEN_BITS */
>+	{ .start = 0x9884, .end = 0x9888 },	/* NOA_WRITE */
>+};
>+
> static bool gen7_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
> {
> 	return reg_in_range_table(addr, gen7_oa_b_counters);
>@@ -4349,7 +4372,10 @@ static bool xehp_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
>
> static bool gen12_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
> {
>-	return reg_in_range_table(addr, gen12_oa_mux_regs);
>+	if (IS_METEORLAKE(perf->i915))
>+		return reg_in_range_table(addr, mtl_oa_mux_regs);
>+	else
>+		return reg_in_range_table(addr, gen12_oa_mux_regs);
> }
>
> static u32 mask_reg_value(u32 reg, u32 val)
>@@ -4746,6 +4772,7 @@ static void oa_init_supported_formats(struct i915_perf *perf)
> 		break;
>
> 	case INTEL_DG2:
>+	case INTEL_METEORLAKE:
> 		oa_format_add(perf, I915_OAR_FORMAT_A32u40_A4u32_B8_C8);
> 		oa_format_add(perf, I915_OA_FORMAT_A24u40_A14u32_B8_C8);
> 		break;
>-- 
>2.36.1
>

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

* Re: [Intel-gfx] [PATCH] drm/i915/mtl: Add support for 32 bit OAG formats in MTL
  2022-11-29  7:15 ` [Intel-gfx] [PATCH] " Lucas De Marchi
@ 2022-11-29 18:40   ` Umesh Nerlige Ramappa
  0 siblings, 0 replies; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2022-11-29 18:40 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx

On Mon, Nov 28, 2022 at 11:15:52PM -0800, Lucas De Marchi wrote:
>On Mon, Nov 28, 2022 at 05:21:46PM -0800, Umesh Nerlige Ramappa wrote:
>>As part of OA support for MTL,
>>
>>- Enable 32 bit OAG formats for MTL.
>>- 0x200c is repurposed on MTL. Use a separate mux table to verify oa
>> configs passed by user.
>>- Similar to ACM, OA/CS timestamp is mismatched on MTL. Add MTL to the
>> WA.
>>- On MTL, gt->scratch was using stolen lmem. An MI_SRM to stolen lmem is
>> hanging. Add a page in noa_wait BO to save/restore GPR registers for
>> the noa_wait logic.
>
>why are all these changes squashed in a single patch?

I don't have a good reason other than we had it this way to enable OA 
for MTL. I can break them into separate patches in the next revision.

>
>>
>>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>>---
>>drivers/gpu/drm/i915/gt/intel_gt_types.h |  6 ---
>>drivers/gpu/drm/i915/i915_perf.c         | 49 ++++++++++++++++++------
>>2 files changed, 38 insertions(+), 17 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/gt/intel_gt_types.h b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>>index c1d9cd255e06..13dffe0a3d20 100644
>>--- a/drivers/gpu/drm/i915/gt/intel_gt_types.h
>>+++ b/drivers/gpu/drm/i915/gt/intel_gt_types.h
>>@@ -296,12 +296,6 @@ enum intel_gt_scratch_field {
>>
>>	/* 8 bytes */
>>	INTEL_GT_SCRATCH_FIELD_COHERENTL3_WA = 256,
>>-
>>-	/* 6 * 8 bytes */
>>-	INTEL_GT_SCRATCH_FIELD_PERF_CS_GPR = 2048,
>>-
>>-	/* 4 bytes */
>>-	INTEL_GT_SCRATCH_FIELD_PERF_PREDICATE_RESULT_1 = 2096,
>>};
>>
>>#endif /* __INTEL_GT_TYPES_H__ */
>>diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>>index 00e09bb18b13..a735b9540113 100644
>>--- a/drivers/gpu/drm/i915/i915_perf.c
>>+++ b/drivers/gpu/drm/i915/i915_perf.c
>>@@ -1842,8 +1842,7 @@ static u32 *save_restore_register(struct i915_perf_stream *stream, u32 *cs,
>>	for (d = 0; d < dword_count; d++) {
>>		*cs++ = cmd;
>>		*cs++ = i915_mmio_reg_offset(reg) + 4 * d;
>>-		*cs++ = intel_gt_scratch_offset(stream->engine->gt,
>>-						offset) + 4 * d;
>>+		*cs++ = i915_ggtt_offset(stream->noa_wait) + offset + 4 * d;
>>		*cs++ = 0;
>>	}
>>
>>@@ -1876,7 +1875,13 @@ static int alloc_noa_wait(struct i915_perf_stream *stream)
>>					  MI_PREDICATE_RESULT_2_ENGINE(base) :
>>					  MI_PREDICATE_RESULT_1(RENDER_RING_BASE);
>>
>>-	bo = i915_gem_object_create_internal(i915, 4096);
>>+	/*
>>+	 * gt->scratch was being used to save/restore the GPR registers, but on
>>+	 * MTL the scratch uses stolen lmem. An MI_SRM to this memory region
>>+	 * causes an engine hang. Instead allocate an additional page here to
>
>humn.. is it because of the pte being wrong?"stolen lmem" in mtl is
>still system memory... do we know why we'd need this change?

This memory is accessed by the CS to save/restore some registers and 
that was causing a hang, so I guess mapping this memory to ggtt did not 
work at the time. I am not sure if this was fixed later, but using a 
memory that's owned by OA seems to work fine.

Thanks,
Umesh
>
>Lucas De Marchi
>
>
>>+	 * save/restore GPR registers
>>+	 */
>>+	bo = i915_gem_object_create_internal(i915, 8192);
>>	if (IS_ERR(bo)) {
>>		drm_err(&i915->drm,
>>			"Failed to allocate NOA wait batchbuffer\n");
>>@@ -1910,14 +1915,19 @@ static int alloc_noa_wait(struct i915_perf_stream *stream)
>>		goto err_unpin;
>>	}
>>
>>+	stream->noa_wait = vma;
>>+
>>+#define GPR_SAVE_OFFSET 4096
>>+#define PREDICATE_SAVE_OFFSET 4160
>>+
>>	/* Save registers. */
>>	for (i = 0; i < N_CS_GPR; i++)
>>		cs = save_restore_register(
>>			stream, cs, true /* save */, CS_GPR(i),
>>-			INTEL_GT_SCRATCH_FIELD_PERF_CS_GPR + 8 * i, 2);
>>+			GPR_SAVE_OFFSET + 8 * i, 2);
>>	cs = save_restore_register(
>>		stream, cs, true /* save */, mi_predicate_result,
>>-		INTEL_GT_SCRATCH_FIELD_PERF_PREDICATE_RESULT_1, 1);
>>+		PREDICATE_SAVE_OFFSET, 1);
>>
>>	/* First timestamp snapshot location. */
>>	ts0 = cs;
>>@@ -2033,10 +2043,10 @@ static int alloc_noa_wait(struct i915_perf_stream *stream)
>>	for (i = 0; i < N_CS_GPR; i++)
>>		cs = save_restore_register(
>>			stream, cs, false /* restore */, CS_GPR(i),
>>-			INTEL_GT_SCRATCH_FIELD_PERF_CS_GPR + 8 * i, 2);
>>+			GPR_SAVE_OFFSET + 8 * i, 2);
>>	cs = save_restore_register(
>>		stream, cs, false /* restore */, mi_predicate_result,
>>-		INTEL_GT_SCRATCH_FIELD_PERF_PREDICATE_RESULT_1, 1);
>>+		PREDICATE_SAVE_OFFSET, 1);
>>
>>	/* And return to the ring. */
>>	*cs++ = MI_BATCH_BUFFER_END;
>>@@ -2046,7 +2056,6 @@ static int alloc_noa_wait(struct i915_perf_stream *stream)
>>	i915_gem_object_flush_map(bo);
>>	__i915_gem_object_release_map(bo);
>>
>>-	stream->noa_wait = vma;
>>	goto out_ww;
>>
>>err_unpin:
>>@@ -3127,8 +3136,11 @@ get_sseu_config(struct intel_sseu *out_sseu,
>> */
>>u32 i915_perf_oa_timestamp_frequency(struct drm_i915_private *i915)
>>{
>>-	/* Wa_18013179988:dg2 */
>>-	if (IS_DG2(i915)) {
>>+	/*
>>+	 * Wa_18013179988:dg2
>>+	 * Wa_14015846243:mtl
>>+	 */
>>+	if (IS_DG2(i915) || IS_METEORLAKE(i915)) {
>>		intel_wakeref_t wakeref;
>>		u32 reg, shift;
>>
>>@@ -4306,6 +4318,17 @@ static const struct i915_range gen12_oa_mux_regs[] = {
>>	{}
>>};
>>
>>+/*
>>+ * Ref: 14010536224:
>>+ * 0x20cc is repurposed on MTL, so use a separate array for MTL.
>>+ */
>>+static const struct i915_range mtl_oa_mux_regs[] = {
>>+	{ .start = 0x0d00, .end = 0x0d04 },	/* RPM_CONFIG[0-1] */
>>+	{ .start = 0x0d0c, .end = 0x0d2c },	/* NOA_CONFIG[0-8] */
>>+	{ .start = 0x9840, .end = 0x9840 },	/* GDT_CHICKEN_BITS */
>>+	{ .start = 0x9884, .end = 0x9888 },	/* NOA_WRITE */
>>+};
>>+
>>static bool gen7_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
>>{
>>	return reg_in_range_table(addr, gen7_oa_b_counters);
>>@@ -4349,7 +4372,10 @@ static bool xehp_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
>>
>>static bool gen12_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
>>{
>>-	return reg_in_range_table(addr, gen12_oa_mux_regs);
>>+	if (IS_METEORLAKE(perf->i915))
>>+		return reg_in_range_table(addr, mtl_oa_mux_regs);
>>+	else
>>+		return reg_in_range_table(addr, gen12_oa_mux_regs);
>>}
>>
>>static u32 mask_reg_value(u32 reg, u32 val)
>>@@ -4746,6 +4772,7 @@ static void oa_init_supported_formats(struct i915_perf *perf)
>>		break;
>>
>>	case INTEL_DG2:
>>+	case INTEL_METEORLAKE:
>>		oa_format_add(perf, I915_OAR_FORMAT_A32u40_A4u32_B8_C8);
>>		oa_format_add(perf, I915_OA_FORMAT_A24u40_A14u32_B8_C8);
>>		break;
>>-- 
>>2.36.1
>>

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

* Re: [Intel-gfx] [PATCH] drm/i915/mtl: Add support for 32 bit OAG formats in MTL
  2022-11-29  1:21 [Intel-gfx] [PATCH] drm/i915/mtl: Add support for 32 bit OAG formats in MTL Umesh Nerlige Ramappa
                   ` (2 preceding siblings ...)
  2022-11-29  7:15 ` [Intel-gfx] [PATCH] " Lucas De Marchi
@ 2022-11-30  1:17 ` Dixit, Ashutosh
  2022-11-30  1:47   ` Dixit, Ashutosh
  2022-11-30  1:51 ` Dixit, Ashutosh
  4 siblings, 1 reply; 11+ messages in thread
From: Dixit, Ashutosh @ 2022-11-30  1:17 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: intel-gfx

On Mon, 28 Nov 2022 17:21:46 -0800, Umesh Nerlige Ramappa wrote:
>

Hi Umesh,

Overall looks ok, just a couple of questions below. Splitting the patches
would be nice and easier to review, but I'm almost done with this one ;-)

> @@ -1876,7 +1875,13 @@ static int alloc_noa_wait(struct i915_perf_stream *stream)
>					  MI_PREDICATE_RESULT_2_ENGINE(base) :
>					  MI_PREDICATE_RESULT_1(RENDER_RING_BASE);
>
> -	bo = i915_gem_object_create_internal(i915, 4096);
> +	/*
> +	 * gt->scratch was being used to save/restore the GPR registers, but on
> +	 * MTL the scratch uses stolen lmem. An MI_SRM to this memory region
> +	 * causes an engine hang. Instead allocate an additional page here to
> +	 * save/restore GPR registers
> +	 */
> +	bo = i915_gem_object_create_internal(i915, 8192);

Do we know how much space was used in stream->noa_wait originally? Anyway
allocating an additional page is not an issue so ok to skip the question.

> @@ -4746,6 +4772,7 @@ static void oa_init_supported_formats(struct i915_perf *perf)
>		break;
>
>	case INTEL_DG2:
> +	case INTEL_METEORLAKE:
>		oa_format_add(perf, I915_OAR_FORMAT_A32u40_A4u32_B8_C8);
>		oa_format_add(perf, I915_OA_FORMAT_A24u40_A14u32_B8_C8);

Do these formats also need to be added?

		oa_format_add(perf, I915_OAR_FORMAT_A32u40_A4u32_B8_C8);
		oa_format_add(perf, I915_OA_FORMAT_A24u40_A14u32_B8_C8);
		oa_format_add(perf, I915_OAR_FORMAT_A36u64_B8_C8);
		oa_format_add(perf, I915_OA_FORMAT_A38u64_R2u64_B8_C8);

Or these are not considered OAG formats?

>		break;

Thanks.
--
Ashutosh

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

* Re: [Intel-gfx] [PATCH] drm/i915/mtl: Add support for 32 bit OAG formats in MTL
  2022-11-30  1:17 ` Dixit, Ashutosh
@ 2022-11-30  1:47   ` Dixit, Ashutosh
  0 siblings, 0 replies; 11+ messages in thread
From: Dixit, Ashutosh @ 2022-11-30  1:47 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: intel-gfx

On Tue, 29 Nov 2022 17:17:13 -0800, Dixit, Ashutosh wrote:
>
> > @@ -4746,6 +4772,7 @@ static void oa_init_supported_formats(struct i915_perf *perf)
> >		break;
> >
> >	case INTEL_DG2:
> > +	case INTEL_METEORLAKE:
> >		oa_format_add(perf, I915_OAR_FORMAT_A32u40_A4u32_B8_C8);
> >		oa_format_add(perf, I915_OA_FORMAT_A24u40_A14u32_B8_C8);
>
> Do these formats also need to be added?

Sorry!

>		oa_format_add(perf, I915_OAR_FORMAT_A32u40_A4u32_B8_C8);
>		oa_format_add(perf, I915_OA_FORMAT_A24u40_A14u32_B8_C8);

These two are the same as above.

>		oa_format_add(perf, I915_OAR_FORMAT_A36u64_B8_C8);
>		oa_format_add(perf, I915_OA_FORMAT_A38u64_R2u64_B8_C8);

And it seems these are not 32 bit (where 32 bit refers to the header
size).

> Or these are not considered OAG formats?

So it's ok, no changes needed here.

>
> >		break;

> Thanks.
> --
> Ashutosh

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

* Re: [Intel-gfx] [PATCH] drm/i915/mtl: Add support for 32 bit OAG formats in MTL
  2022-11-29  1:21 [Intel-gfx] [PATCH] drm/i915/mtl: Add support for 32 bit OAG formats in MTL Umesh Nerlige Ramappa
                   ` (3 preceding siblings ...)
  2022-11-30  1:17 ` Dixit, Ashutosh
@ 2022-11-30  1:51 ` Dixit, Ashutosh
  2022-11-30 20:00   ` Umesh Nerlige Ramappa
  4 siblings, 1 reply; 11+ messages in thread
From: Dixit, Ashutosh @ 2022-11-30  1:51 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: intel-gfx

On Mon, 28 Nov 2022 17:21:46 -0800, Umesh Nerlige Ramappa wrote:
>
> +/*
> + * Ref: 14010536224:
> + * 0x20cc is repurposed on MTL, so use a separate array for MTL.

Wondering if it was WAIT_FOR_RC6_EXIT (seen in gen12_oa_mux_regs) which
moved elsewhere and if that needs to be added to the array below too?

> + */
> +static const struct i915_range mtl_oa_mux_regs[] = {
> +	{ .start = 0x0d00, .end = 0x0d04 },	/* RPM_CONFIG[0-1] */
> +	{ .start = 0x0d0c, .end = 0x0d2c },	/* NOA_CONFIG[0-8] */
> +	{ .start = 0x9840, .end = 0x9840 },	/* GDT_CHICKEN_BITS */
> +	{ .start = 0x9884, .end = 0x9888 },	/* NOA_WRITE */
> +};
> +
>  static bool gen7_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
>  {
>	return reg_in_range_table(addr, gen7_oa_b_counters);
> @@ -4349,7 +4372,10 @@ static bool xehp_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
>
>  static bool gen12_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
>  {
> -	return reg_in_range_table(addr, gen12_oa_mux_regs);
> +	if (IS_METEORLAKE(perf->i915))
> +		return reg_in_range_table(addr, mtl_oa_mux_regs);
> +	else
> +		return reg_in_range_table(addr, gen12_oa_mux_regs);

But otherwise this is:

Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

If you decide to split the patches, please add my R-b on all the split patches.

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

* Re: [Intel-gfx] [PATCH] drm/i915/mtl: Add support for 32 bit OAG formats in MTL
  2022-11-30  1:51 ` Dixit, Ashutosh
@ 2022-11-30 20:00   ` Umesh Nerlige Ramappa
  2022-11-30 20:14     ` Dixit, Ashutosh
  0 siblings, 1 reply; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2022-11-30 20:00 UTC (permalink / raw)
  To: Dixit, Ashutosh; +Cc: intel-gfx

On Tue, Nov 29, 2022 at 05:51:13PM -0800, Dixit, Ashutosh wrote:
>On Mon, 28 Nov 2022 17:21:46 -0800, Umesh Nerlige Ramappa wrote:
>>
>> +/*
>> + * Ref: 14010536224:
>> + * 0x20cc is repurposed on MTL, so use a separate array for MTL.
>
>Wondering if it was WAIT_FOR_RC6_EXIT (seen in gen12_oa_mux_regs) which
>moved elsewhere and if that needs to be added to the array below too?

WAIT_FOR_RC6_EXIT (0x20cc) moved elsewhere so it should be "removed" 
from mtl oa mux array.

>
>> + */
>> +static const struct i915_range mtl_oa_mux_regs[] = {
>> +	{ .start = 0x0d00, .end = 0x0d04 },	/* RPM_CONFIG[0-1] */
>> +	{ .start = 0x0d0c, .end = 0x0d2c },	/* NOA_CONFIG[0-8] */
>> +	{ .start = 0x9840, .end = 0x9840 },	/* GDT_CHICKEN_BITS */
>> +	{ .start = 0x9884, .end = 0x9888 },	/* NOA_WRITE */
>> +};
>> +
>>  static bool gen7_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
>>  {
>>	return reg_in_range_table(addr, gen7_oa_b_counters);
>> @@ -4349,7 +4372,10 @@ static bool xehp_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
>>
>>  static bool gen12_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
>>  {
>> -	return reg_in_range_table(addr, gen12_oa_mux_regs);
>> +	if (IS_METEORLAKE(perf->i915))
>> +		return reg_in_range_table(addr, mtl_oa_mux_regs);
>> +	else
>> +		return reg_in_range_table(addr, gen12_oa_mux_regs);
>
>But otherwise this is:
>
>Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>

I will break them into separate patches though. If the diff is 
identical, I will carry over your R-b on the split patches. Please let 
me know if that's a concern.

Thanks,
Umesh
>
>If you decide to split the patches, please add my R-b on all the split patches.

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

* Re: [Intel-gfx] [PATCH] drm/i915/mtl: Add support for 32 bit OAG formats in MTL
  2022-11-30 20:00   ` Umesh Nerlige Ramappa
@ 2022-11-30 20:14     ` Dixit, Ashutosh
  2022-11-30 23:39       ` Umesh Nerlige Ramappa
  0 siblings, 1 reply; 11+ messages in thread
From: Dixit, Ashutosh @ 2022-11-30 20:14 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: intel-gfx

On Wed, 30 Nov 2022 12:00:57 -0800, Umesh Nerlige Ramappa wrote:
>
> On Tue, Nov 29, 2022 at 05:51:13PM -0800, Dixit, Ashutosh wrote:
> > On Mon, 28 Nov 2022 17:21:46 -0800, Umesh Nerlige Ramappa wrote:
> >>
> >> +/*
> >> + * Ref: 14010536224:
> >> + * 0x20cc is repurposed on MTL, so use a separate array for MTL.
> >
> > Wondering if it was WAIT_FOR_RC6_EXIT (seen in gen12_oa_mux_regs) which
> > moved elsewhere and if that needs to be added to the array below too?
>
> WAIT_FOR_RC6_EXIT (0x20cc) moved elsewhere so it should be "removed" from
> mtl oa mux array.

What I was saying was let's say WAIT_FOR_RC6_EXIT moved to 0xc0ffee so now
should 0xc0ffee be added to mtl_oa_mux_regs?

>
> >
> >> + */
> >> +static const struct i915_range mtl_oa_mux_regs[] = {
> >> +	{ .start = 0x0d00, .end = 0x0d04 },	/* RPM_CONFIG[0-1] */
> >> +	{ .start = 0x0d0c, .end = 0x0d2c },	/* NOA_CONFIG[0-8] */
> >> +	{ .start = 0x9840, .end = 0x9840 },	/* GDT_CHICKEN_BITS */
> >> +	{ .start = 0x9884, .end = 0x9888 },	/* NOA_WRITE */
> >> +};
> >> +
> >>  static bool gen7_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
> >>  {
> >>	return reg_in_range_table(addr, gen7_oa_b_counters);
> >> @@ -4349,7 +4372,10 @@ static bool xehp_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
> >>
> >>  static bool gen12_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
> >>  {
> >> -	return reg_in_range_table(addr, gen12_oa_mux_regs);
> >> +	if (IS_METEORLAKE(perf->i915))
> >> +		return reg_in_range_table(addr, mtl_oa_mux_regs);
> >> +	else
> >> +		return reg_in_range_table(addr, gen12_oa_mux_regs);
> >
> > But otherwise this is:
> >
> > Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
>
> I will break them into separate patches though. If the diff is identical, I
> will carry over your R-b on the split patches. Please let me know if that's
> a concern.

No I can quickly review again anyway.

> > If you decide to split the patches, please add my R-b on all the split patches.

Thanks.
--
Ashutosh

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

* Re: [Intel-gfx] [PATCH] drm/i915/mtl: Add support for 32 bit OAG formats in MTL
  2022-11-30 20:14     ` Dixit, Ashutosh
@ 2022-11-30 23:39       ` Umesh Nerlige Ramappa
  0 siblings, 0 replies; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2022-11-30 23:39 UTC (permalink / raw)
  To: Dixit, Ashutosh; +Cc: intel-gfx

On Wed, Nov 30, 2022 at 12:14:20PM -0800, Dixit, Ashutosh wrote:
>On Wed, 30 Nov 2022 12:00:57 -0800, Umesh Nerlige Ramappa wrote:
>>
>> On Tue, Nov 29, 2022 at 05:51:13PM -0800, Dixit, Ashutosh wrote:
>> > On Mon, 28 Nov 2022 17:21:46 -0800, Umesh Nerlige Ramappa wrote:
>> >>
>> >> +/*
>> >> + * Ref: 14010536224:
>> >> + * 0x20cc is repurposed on MTL, so use a separate array for MTL.
>> >
>> > Wondering if it was WAIT_FOR_RC6_EXIT (seen in gen12_oa_mux_regs) which
>> > moved elsewhere and if that needs to be added to the array below too?
>>
>> WAIT_FOR_RC6_EXIT (0x20cc) moved elsewhere so it should be "removed" from
>> mtl oa mux array.
>
>What I was saying was let's say WAIT_FOR_RC6_EXIT moved to 0xc0ffee so now
>should 0xc0ffee be added to mtl_oa_mux_regs?

oh, sorry, I misread that.

I looked for WAIT_FOR_RC6_EXIT in the bspec and did not find it defined 
for MTL, so it's dropped completely. If you could confirm, that would be 
great.

>
>>
>> >
>> >> + */
>> >> +static const struct i915_range mtl_oa_mux_regs[] = {
>> >> +	{ .start = 0x0d00, .end = 0x0d04 },	/* RPM_CONFIG[0-1] */
>> >> +	{ .start = 0x0d0c, .end = 0x0d2c },	/* NOA_CONFIG[0-8] */
>> >> +	{ .start = 0x9840, .end = 0x9840 },	/* GDT_CHICKEN_BITS */
>> >> +	{ .start = 0x9884, .end = 0x9888 },	/* NOA_WRITE */
>> >> +};
>> >> +
>> >>  static bool gen7_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
>> >>  {
>> >>	return reg_in_range_table(addr, gen7_oa_b_counters);
>> >> @@ -4349,7 +4372,10 @@ static bool xehp_is_valid_b_counter_addr(struct i915_perf *perf, u32 addr)
>> >>
>> >>  static bool gen12_is_valid_mux_addr(struct i915_perf *perf, u32 addr)
>> >>  {
>> >> -	return reg_in_range_table(addr, gen12_oa_mux_regs);
>> >> +	if (IS_METEORLAKE(perf->i915))
>> >> +		return reg_in_range_table(addr, mtl_oa_mux_regs);
>> >> +	else
>> >> +		return reg_in_range_table(addr, gen12_oa_mux_regs);
>> >
>> > But otherwise this is:
>> >
>> > Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
>>
>> I will break them into separate patches though. If the diff is identical, I
>> will carry over your R-b on the split patches. Please let me know if that's
>> a concern.
>
>No I can quickly review again anyway.

got it, will not add the R-bs.

Thanks,
Umesh

>
>> > If you decide to split the patches, please add my R-b on all the split patches.
>
>Thanks.
>--
>Ashutosh

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

end of thread, other threads:[~2022-11-30 23:39 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-11-29  1:21 [Intel-gfx] [PATCH] drm/i915/mtl: Add support for 32 bit OAG formats in MTL Umesh Nerlige Ramappa
2022-11-29  2:20 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for " Patchwork
2022-11-29  2:45 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
2022-11-29  7:15 ` [Intel-gfx] [PATCH] " Lucas De Marchi
2022-11-29 18:40   ` Umesh Nerlige Ramappa
2022-11-30  1:17 ` Dixit, Ashutosh
2022-11-30  1:47   ` Dixit, Ashutosh
2022-11-30  1:51 ` Dixit, Ashutosh
2022-11-30 20:00   ` Umesh Nerlige Ramappa
2022-11-30 20:14     ` Dixit, Ashutosh
2022-11-30 23:39       ` Umesh Nerlige Ramappa

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).