All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 1/2] i915/perf: Move gen specific OA formats to single array
@ 2020-12-15 21:49 Umesh Nerlige Ramappa
  2020-12-15 21:49 ` [Intel-gfx] [PATCH 2/2] i915/perf: Add removed OA formats back for TGL Umesh Nerlige Ramappa
                   ` (5 more replies)
  0 siblings, 6 replies; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2020-12-15 21:49 UTC (permalink / raw)
  To: intel-gfx, Lionel G Landwerlin

Variations in OA formats in the different gens has led to creation of
several sparse arrays to store the formats.

Move oa formats into a single array and add the supported range of
platforms for the oa formats.

Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c       | 56 ++++++++++++--------------
 drivers/gpu/drm/i915/i915_perf_types.h |  3 ++
 2 files changed, 28 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index f553caf4b06d..afa92cf075c4 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -298,28 +298,27 @@ static u32 i915_oa_max_sample_rate = 100000;
 
 /* XXX: beware if future OA HW adds new report formats that the current
  * code assumes all reports have a power-of-two size and ~(size - 1) can
- * be used as a mask to align the OA tail pointer.
+ * be used as a mask to align the OA tail pointer. Note that the platforms
+ * in this array specify a range (inclusive of start and end).
  */
-static const struct i915_oa_format hsw_oa_formats[I915_OA_FORMAT_MAX] = {
-	[I915_OA_FORMAT_A13]	    = { 0, 64 },
-	[I915_OA_FORMAT_A29]	    = { 1, 128 },
-	[I915_OA_FORMAT_A13_B8_C8]  = { 2, 128 },
-	/* A29_B8_C8 Disallowed as 192 bytes doesn't factor into buffer size */
-	[I915_OA_FORMAT_B4_C8]	    = { 4, 64 },
-	[I915_OA_FORMAT_A45_B8_C8]  = { 5, 256 },
-	[I915_OA_FORMAT_B4_C8_A16]  = { 6, 128 },
-	[I915_OA_FORMAT_C4_B8]	    = { 7, 64 },
-};
-
-static const struct i915_oa_format gen8_plus_oa_formats[I915_OA_FORMAT_MAX] = {
-	[I915_OA_FORMAT_A12]		    = { 0, 64 },
-	[I915_OA_FORMAT_A12_B8_C8]	    = { 2, 128 },
-	[I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
-	[I915_OA_FORMAT_C4_B8]		    = { 7, 64 },
-};
-
-static const struct i915_oa_format gen12_oa_formats[I915_OA_FORMAT_MAX] = {
-	[I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
+static const struct i915_oa_format oa_formats[I915_OA_FORMAT_MAX] = {
+	/* haswell */
+	[I915_OA_FORMAT_A13]                    = { 0, 64, INTEL_IVYBRIDGE, INTEL_HASWELL },
+	[I915_OA_FORMAT_A29]                    = { 1, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
+	[I915_OA_FORMAT_A13_B8_C8]              = { 2, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
+	[I915_OA_FORMAT_B4_C8]                  = { 4, 64, INTEL_IVYBRIDGE, INTEL_HASWELL },
+	[I915_OA_FORMAT_A45_B8_C8]              = { 5, 256, INTEL_IVYBRIDGE, INTEL_HASWELL },
+	[I915_OA_FORMAT_B4_C8_A16]              = { 6, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
+
+	/* haswell+ upto but not including tigerlake */
+	[I915_OA_FORMAT_C4_B8]                  = { 7, 64, INTEL_IVYBRIDGE, INTEL_TIGERLAKE - 1 },
+
+	/* gen8+ upto but not including tigerlake */
+	[I915_OA_FORMAT_A12]                    = { 0, 64, INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
+	[I915_OA_FORMAT_A12_B8_C8]              = { 2, 128, INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
+
+	/* gen8+ */
+	[I915_OA_FORMAT_A32u40_A4u32_B8_C8]     = { 5, 256, INTEL_BROADWELL, INTEL_MAX_PLATFORMS - 1 },
 };
 
 #define SAMPLE_OA_REPORT      (1<<0)
@@ -3575,6 +3574,7 @@ static int read_properties_unlocked(struct i915_perf *perf,
 	for (i = 0; i < n_props; i++) {
 		u64 oa_period, oa_freq_hz;
 		u64 id, value;
+		enum intel_platform p = INTEL_INFO(perf->i915)->platform;
 
 		ret = get_user(id, uprop);
 		if (ret)
@@ -3611,8 +3611,9 @@ static int read_properties_unlocked(struct i915_perf *perf,
 					  value);
 				return -EINVAL;
 			}
-			if (!perf->oa_formats[value].size) {
-				DRM_DEBUG("Unsupported OA report format %llu\n",
+			if (p < perf->oa_formats[value].start ||
+			    p > perf->oa_formats[value].end) {
+				DRM_DEBUG("OA report format not supported %llu\n",
 					  value);
 				return -EINVAL;
 			}
@@ -4270,6 +4271,7 @@ void i915_perf_init(struct drm_i915_private *i915)
 
 	/* XXX const struct i915_perf_ops! */
 
+	perf->oa_formats = oa_formats;
 	if (IS_HASWELL(i915)) {
 		perf->ops.is_valid_b_counter_reg = gen7_is_valid_b_counter_addr;
 		perf->ops.is_valid_mux_reg = hsw_is_valid_mux_addr;
@@ -4280,8 +4282,6 @@ void i915_perf_init(struct drm_i915_private *i915)
 		perf->ops.oa_disable = gen7_oa_disable;
 		perf->ops.read = gen7_oa_read;
 		perf->ops.oa_hw_tail_read = gen7_oa_hw_tail_read;
-
-		perf->oa_formats = hsw_oa_formats;
 	} else if (HAS_LOGICAL_RING_CONTEXTS(i915)) {
 		/* Note: that although we could theoretically also support the
 		 * legacy ringbuffer mode on BDW (and earlier iterations of
@@ -4292,8 +4292,6 @@ void i915_perf_init(struct drm_i915_private *i915)
 		perf->ops.read = gen8_oa_read;
 
 		if (IS_GEN_RANGE(i915, 8, 9)) {
-			perf->oa_formats = gen8_plus_oa_formats;
-
 			perf->ops.is_valid_b_counter_reg =
 				gen7_is_valid_b_counter_addr;
 			perf->ops.is_valid_mux_reg =
@@ -4324,8 +4322,6 @@ void i915_perf_init(struct drm_i915_private *i915)
 				perf->gen8_valid_ctx_bit = BIT(16);
 			}
 		} else if (IS_GEN_RANGE(i915, 10, 11)) {
-			perf->oa_formats = gen8_plus_oa_formats;
-
 			perf->ops.is_valid_b_counter_reg =
 				gen7_is_valid_b_counter_addr;
 			perf->ops.is_valid_mux_reg =
@@ -4348,8 +4344,6 @@ void i915_perf_init(struct drm_i915_private *i915)
 			}
 			perf->gen8_valid_ctx_bit = BIT(16);
 		} else if (IS_GEN(i915, 12)) {
-			perf->oa_formats = gen12_oa_formats;
-
 			perf->ops.is_valid_b_counter_reg =
 				gen12_is_valid_b_counter_addr;
 			perf->ops.is_valid_mux_reg =
diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
index a36a455ae336..84dceb743ebc 100644
--- a/drivers/gpu/drm/i915/i915_perf_types.h
+++ b/drivers/gpu/drm/i915/i915_perf_types.h
@@ -19,6 +19,7 @@
 #include "gt/intel_sseu.h"
 #include "i915_reg.h"
 #include "intel_wakeref.h"
+#include "intel_device_info.h"
 
 struct drm_i915_private;
 struct file;
@@ -32,6 +33,8 @@ struct intel_engine_cs;
 struct i915_oa_format {
 	u32 format;
 	int size;
+	enum intel_platform start;
+	enum intel_platform end;
 };
 
 struct i915_oa_reg {
-- 
2.20.1

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

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

* [Intel-gfx] [PATCH 2/2] i915/perf: Add removed OA formats back for TGL
  2020-12-15 21:49 [Intel-gfx] [PATCH 1/2] i915/perf: Move gen specific OA formats to single array Umesh Nerlige Ramappa
@ 2020-12-15 21:49 ` Umesh Nerlige Ramappa
  2020-12-16  0:16 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] i915/perf: Move gen specific OA formats to single array Patchwork
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2020-12-15 21:49 UTC (permalink / raw)
  To: intel-gfx, Lionel G Landwerlin

When defining OA formats for TGL, some formats were left out. Add them
back and clean up the uapi comments to reflect available formats.

Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c | 12 +++++-------
 include/uapi/drm/i915_drm.h      | 24 ++++++++++++++----------
 2 files changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index afa92cf075c4..659fcc9ae819 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -310,14 +310,12 @@ static const struct i915_oa_format oa_formats[I915_OA_FORMAT_MAX] = {
 	[I915_OA_FORMAT_A45_B8_C8]              = { 5, 256, INTEL_IVYBRIDGE, INTEL_HASWELL },
 	[I915_OA_FORMAT_B4_C8_A16]              = { 6, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
 
-	/* haswell+ upto but not including tigerlake */
-	[I915_OA_FORMAT_C4_B8]                  = { 7, 64, INTEL_IVYBRIDGE, INTEL_TIGERLAKE - 1 },
+	/* haswell+ */
+	[I915_OA_FORMAT_C4_B8]                  = { 7, 64, INTEL_HASWELL, INTEL_MAX_PLATFORMS - 1 },
 
-	/* gen8+ upto but not including tigerlake */
-	[I915_OA_FORMAT_A12]                    = { 0, 64, INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
-	[I915_OA_FORMAT_A12_B8_C8]              = { 2, 128, INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
-
-	/* gen8+ */
+	/* broadwell+ */
+	[I915_OA_FORMAT_A12]                    = { 0, 64, INTEL_BROADWELL, INTEL_MAX_PLATFORMS - 1 },
+	[I915_OA_FORMAT_A12_B8_C8]              = { 2, 128, INTEL_BROADWELL, INTEL_MAX_PLATFORMS - 1 },
 	[I915_OA_FORMAT_A32u40_A4u32_B8_C8]     = { 5, 256, INTEL_BROADWELL, INTEL_MAX_PLATFORMS - 1 },
 };
 
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 6edcb2b6c708..933511c2892e 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -1951,20 +1951,24 @@ struct drm_i915_gem_userptr {
 };
 
 enum drm_i915_oa_format {
-	I915_OA_FORMAT_A13 = 1,	    /* HSW only */
-	I915_OA_FORMAT_A29,	    /* HSW only */
-	I915_OA_FORMAT_A13_B8_C8,   /* HSW only */
-	I915_OA_FORMAT_B4_C8,	    /* HSW only */
-	I915_OA_FORMAT_A45_B8_C8,   /* HSW only */
-	I915_OA_FORMAT_B4_C8_A16,   /* HSW only */
-	I915_OA_FORMAT_C4_B8,	    /* HSW+ */
-
-	/* Gen8+ */
+	/* haswell */
+	I915_OA_FORMAT_A13 = 1,
+	I915_OA_FORMAT_A29,
+	I915_OA_FORMAT_A13_B8_C8,
+	I915_OA_FORMAT_B4_C8,
+	I915_OA_FORMAT_A45_B8_C8,
+	I915_OA_FORMAT_B4_C8_A16,
+
+	/* haswell+ */
+	I915_OA_FORMAT_C4_B8,
+
+	/* broadwell+ */
 	I915_OA_FORMAT_A12,
 	I915_OA_FORMAT_A12_B8_C8,
 	I915_OA_FORMAT_A32u40_A4u32_B8_C8,
 
-	I915_OA_FORMAT_MAX	    /* non-ABI */
+	/* non-ABI */
+	I915_OA_FORMAT_MAX
 };
 
 enum drm_i915_perf_property_id {
-- 
2.20.1

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

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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] i915/perf: Move gen specific OA formats to single array
  2020-12-15 21:49 [Intel-gfx] [PATCH 1/2] i915/perf: Move gen specific OA formats to single array Umesh Nerlige Ramappa
  2020-12-15 21:49 ` [Intel-gfx] [PATCH 2/2] i915/perf: Add removed OA formats back for TGL Umesh Nerlige Ramappa
@ 2020-12-16  0:16 ` Patchwork
  2020-12-16  0:46 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2020-12-16  0:16 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] i915/perf: Move gen specific OA formats to single array
URL   : https://patchwork.freedesktop.org/series/84978/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
60f0353cf872 i915/perf: Move gen specific OA formats to single array
-:63: WARNING:LONG_LINE: line length of 103 exceeds 100 columns
#63: FILE: drivers/gpu/drm/i915/i915_perf.c:321:
+	[I915_OA_FORMAT_A32u40_A4u32_B8_C8]     = { 5, 256, INTEL_BROADWELL, INTEL_MAX_PLATFORMS - 1 },

total: 0 errors, 1 warnings, 0 checks, 120 lines checked
4db0badf5a24 i915/perf: Add removed OA formats back for TGL
-:30: WARNING:LONG_LINE: line length of 102 exceeds 100 columns
#30: FILE: drivers/gpu/drm/i915/i915_perf.c:317:
+	[I915_OA_FORMAT_A12]                    = { 0, 64, INTEL_BROADWELL, INTEL_MAX_PLATFORMS - 1 },

-:31: WARNING:LONG_LINE: line length of 103 exceeds 100 columns
#31: FILE: drivers/gpu/drm/i915/i915_perf.c:318:
+	[I915_OA_FORMAT_A12_B8_C8]              = { 2, 128, INTEL_BROADWELL, INTEL_MAX_PLATFORMS - 1 },

total: 0 errors, 2 warnings, 0 checks, 53 lines checked


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

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

* [Intel-gfx] ✓ Fi.CI.BAT: success for series starting with [1/2] i915/perf: Move gen specific OA formats to single array
  2020-12-15 21:49 [Intel-gfx] [PATCH 1/2] i915/perf: Move gen specific OA formats to single array Umesh Nerlige Ramappa
  2020-12-15 21:49 ` [Intel-gfx] [PATCH 2/2] i915/perf: Add removed OA formats back for TGL Umesh Nerlige Ramappa
  2020-12-16  0:16 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] i915/perf: Move gen specific OA formats to single array Patchwork
@ 2020-12-16  0:46 ` Patchwork
  2020-12-16  7:08 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2020-12-16  0:46 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 4962 bytes --]

== Series Details ==

Series: series starting with [1/2] i915/perf: Move gen specific OA formats to single array
URL   : https://patchwork.freedesktop.org/series/84978/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9489 -> Patchwork_19151
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

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

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

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

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@nop-gfx0:
    - fi-apl-guc:         NOTRUN -> [SKIP][1] ([fdo#109271]) +17 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/fi-apl-guc/igt@amdgpu/amd_cs_nop@nop-gfx0.html

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-tgl-y:           [PASS][2] -> [DMESG-FAIL][3] ([i915#2601] / [i915#541])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/fi-tgl-y/igt@i915_selftest@live@gt_heartbeat.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/fi-tgl-y/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_addfb_basic@addfb25-y-tiled-small-legacy:
    - fi-snb-2600:        NOTRUN -> [SKIP][4] ([fdo#109271]) +30 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/fi-snb-2600/igt@kms_addfb_basic@addfb25-y-tiled-small-legacy.html

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-snb-2600:        NOTRUN -> [SKIP][5] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/fi-snb-2600/igt@kms_chamelium@hdmi-crc-fast.html

  * igt@kms_psr@primary_page_flip:
    - fi-glk-dsi:         NOTRUN -> [SKIP][6] ([fdo#109271]) +21 similar issues
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/fi-glk-dsi/igt@kms_psr@primary_page_flip.html

  * igt@prime_vgem@basic-write:
    - fi-tgl-y:           [PASS][7] -> [DMESG-WARN][8] ([i915#402]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/fi-tgl-y/igt@prime_vgem@basic-write.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/fi-tgl-y/igt@prime_vgem@basic-write.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3:
    - fi-snb-2600:        [DMESG-WARN][9] -> [PASS][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/fi-snb-2600/igt@gem_exec_suspend@basic-s3.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/fi-snb-2600/igt@gem_exec_suspend@basic-s3.html

  * igt@i915_selftest@live@gt_timelines:
    - fi-apl-guc:         [INCOMPLETE][11] ([i915#2750]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/fi-apl-guc/igt@i915_selftest@live@gt_timelines.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/fi-apl-guc/igt@i915_selftest@live@gt_timelines.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-glk-dsi:         [INCOMPLETE][13] ([i915#2377]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/fi-glk-dsi/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/fi-glk-dsi/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@prime_self_import@basic-with_two_bos:
    - fi-tgl-y:           [DMESG-WARN][15] ([i915#402]) -> [PASS][16] +1 similar issue
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/fi-tgl-y/igt@prime_self_import@basic-with_two_bos.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/fi-tgl-y/igt@prime_self_import@basic-with_two_bos.html

  
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#2377]: https://gitlab.freedesktop.org/drm/intel/issues/2377
  [i915#2601]: https://gitlab.freedesktop.org/drm/intel/issues/2601
  [i915#2750]: https://gitlab.freedesktop.org/drm/intel/issues/2750
  [i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
  [i915#541]: https://gitlab.freedesktop.org/drm/intel/issues/541


Participating hosts (43 -> 39)
------------------------------

  Missing    (4): fi-ctg-p8600 fi-bsw-cyan fi-bdw-samus fi-hsw-4200u 


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

  * Linux: CI_DRM_9489 -> Patchwork_19151

  CI-20190529: 20190529
  CI_DRM_9489: bef2104ec6e0aa163b1b01b661e734b08b567aeb @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5902: 1c1fc6c4d506dc69d8e85b09bcb932466712d416 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_19151: 4db0badf5a249da08dd4a6635c22eaef89b407f4 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

4db0badf5a24 i915/perf: Add removed OA formats back for TGL
60f0353cf872 i915/perf: Move gen specific OA formats to single array

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 6083 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

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

* [Intel-gfx] ✓ Fi.CI.IGT: success for series starting with [1/2] i915/perf: Move gen specific OA formats to single array
  2020-12-15 21:49 [Intel-gfx] [PATCH 1/2] i915/perf: Move gen specific OA formats to single array Umesh Nerlige Ramappa
                   ` (2 preceding siblings ...)
  2020-12-16  0:46 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-12-16  7:08 ` Patchwork
  2020-12-16  8:30 ` [Intel-gfx] [PATCH 1/2] " Lionel Landwerlin
  2020-12-16  9:25 ` Tvrtko Ursulin
  5 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2020-12-16  7:08 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: intel-gfx


[-- Attachment #1.1: Type: text/plain, Size: 17311 bytes --]

== Series Details ==

Series: series starting with [1/2] i915/perf: Move gen specific OA formats to single array
URL   : https://patchwork.freedesktop.org/series/84978/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_9489_full -> Patchwork_19151_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_partial_pwrite_pread@writes-after-reads-display:
    - shard-glk:          [PASS][1] -> [DMESG-WARN][2] ([i915#118] / [i915#95]) +1 similar issue
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-glk7/igt@gem_partial_pwrite_pread@writes-after-reads-display.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-glk4/igt@gem_partial_pwrite_pread@writes-after-reads-display.html

  * igt@gem_pread@exhaustion:
    - shard-apl:          NOTRUN -> [WARN][3] ([i915#2658])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-apl3/igt@gem_pread@exhaustion.html

  * igt@gem_render_copy@linear-to-vebox-y-tiled:
    - shard-skl:          NOTRUN -> [SKIP][4] ([fdo#109271]) +11 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl5/igt@gem_render_copy@linear-to-vebox-y-tiled.html

  * igt@kms_big_fb@x-tiled-32bpp-rotate-90:
    - shard-glk:          NOTRUN -> [SKIP][5] ([fdo#109271]) +2 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-glk4/igt@kms_big_fb@x-tiled-32bpp-rotate-90.html

  * igt@kms_color_chamelium@pipe-a-ctm-red-to-blue:
    - shard-apl:          NOTRUN -> [SKIP][6] ([fdo#109271] / [fdo#111827])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-apl3/igt@kms_color_chamelium@pipe-a-ctm-red-to-blue.html

  * igt@kms_cursor_crc@pipe-a-cursor-256x85-sliding:
    - shard-skl:          [PASS][7] -> [FAIL][8] ([i915#54]) +1 similar issue
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-skl3/igt@kms_cursor_crc@pipe-a-cursor-256x85-sliding.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl7/igt@kms_cursor_crc@pipe-a-cursor-256x85-sliding.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1:
    - shard-skl:          [PASS][9] -> [FAIL][10] ([i915#79])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-skl7/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl8/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html
    - shard-tglb:         [PASS][11] -> [FAIL][12] ([i915#2598])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-tglb1/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-tglb3/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html

  * igt@kms_flip@wf_vblank-ts-check-interruptible@b-edp1:
    - shard-skl:          [PASS][13] -> [FAIL][14] ([i915#2122])
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-skl2/igt@kms_flip@wf_vblank-ts-check-interruptible@b-edp1.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl3/igt@kms_flip@wf_vblank-ts-check-interruptible@b-edp1.html

  * igt@kms_hdr@bpc-switch-suspend:
    - shard-skl:          [PASS][15] -> [FAIL][16] ([i915#1188])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-skl2/igt@kms_hdr@bpc-switch-suspend.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl3/igt@kms_hdr@bpc-switch-suspend.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - shard-skl:          [PASS][17] -> [INCOMPLETE][18] ([i915#198])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-skl5/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl5/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          [PASS][19] -> [FAIL][20] ([fdo#108145] / [i915#265])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-skl9/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl4/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html

  * igt@kms_psr@psr2_primary_mmap_cpu:
    - shard-iclb:         [PASS][21] -> [SKIP][22] ([fdo#109441]) +2 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-iclb2/igt@kms_psr@psr2_primary_mmap_cpu.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-iclb7/igt@kms_psr@psr2_primary_mmap_cpu.html

  * igt@prime_vgem@fence-write-hang:
    - shard-apl:          NOTRUN -> [SKIP][23] ([fdo#109271]) +12 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-apl3/igt@prime_vgem@fence-write-hang.html

  * igt@sysfs_heartbeat_interval@mixed@vecs0:
    - shard-kbl:          [PASS][24] -> [INCOMPLETE][25] ([i915#1731])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-kbl1/igt@sysfs_heartbeat_interval@mixed@vecs0.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-kbl3/igt@sysfs_heartbeat_interval@mixed@vecs0.html

  
#### Possible fixes ####

  * igt@drm_mm@all@evict:
    - shard-skl:          [INCOMPLETE][26] ([i915#2295]) -> [PASS][27]
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-skl4/igt@drm_mm@all@evict.html
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl2/igt@drm_mm@all@evict.html

  * igt@gem_exec_capture@pi@rcs0:
    - shard-skl:          [INCOMPLETE][28] ([i915#2369] / [i915#2502]) -> [PASS][29]
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-skl8/igt@gem_exec_capture@pi@rcs0.html
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl1/igt@gem_exec_capture@pi@rcs0.html

  * igt@gem_exec_flush@basic-batch-kernel-default-uc:
    - shard-glk:          [DMESG-WARN][30] ([i915#118] / [i915#95]) -> [PASS][31]
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-glk2/igt@gem_exec_flush@basic-batch-kernel-default-uc.html
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-glk5/igt@gem_exec_flush@basic-batch-kernel-default-uc.html

  * {igt@gem_exec_schedule@u-fairslice@vcs0}:
    - shard-apl:          [DMESG-WARN][32] ([i915#1610]) -> [PASS][33]
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-apl7/igt@gem_exec_schedule@u-fairslice@vcs0.html
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-apl3/igt@gem_exec_schedule@u-fairslice@vcs0.html

  * igt@gem_ppgtt@flink-and-close-vma-leak:
    - shard-glk:          [FAIL][34] ([i915#644]) -> [PASS][35]
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-glk9/igt@gem_ppgtt@flink-and-close-vma-leak.html
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-glk7/igt@gem_ppgtt@flink-and-close-vma-leak.html

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-hsw:          [WARN][36] ([i915#1519]) -> [PASS][37]
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-hsw4/igt@i915_pm_rc6_residency@rc6-idle.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-hsw2/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@i915_selftest@live@active:
    - shard-skl:          [DMESG-FAIL][38] ([i915#2291]) -> [PASS][39]
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-skl9/igt@i915_selftest@live@active.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl10/igt@i915_selftest@live@active.html

  * igt@kms_async_flips@alternate-sync-async-flip:
    - shard-skl:          [FAIL][40] ([i915#2521]) -> [PASS][41]
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-skl8/igt@kms_async_flips@alternate-sync-async-flip.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl1/igt@kms_async_flips@alternate-sync-async-flip.html
    - shard-kbl:          [FAIL][42] ([i915#2521]) -> [PASS][43]
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-kbl7/igt@kms_async_flips@alternate-sync-async-flip.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-kbl1/igt@kms_async_flips@alternate-sync-async-flip.html

  * igt@kms_cursor_crc@pipe-c-cursor-suspend:
    - shard-skl:          [FAIL][44] ([i915#54]) -> [PASS][45] +5 similar issues
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-skl2/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl2/igt@kms_cursor_crc@pipe-c-cursor-suspend.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic:
    - shard-tglb:         [FAIL][46] ([i915#2346]) -> [PASS][47]
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-tglb5/igt@kms_cursor_legacy@flip-vs-cursor-atomic.html
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-tglb2/igt@kms_cursor_legacy@flip-vs-cursor-atomic.html

  * igt@kms_fbcon_fbt@psr-suspend:
    - shard-skl:          [INCOMPLETE][48] ([i915#146] / [i915#198]) -> [PASS][49]
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-skl1/igt@kms_fbcon_fbt@psr-suspend.html
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl10/igt@kms_fbcon_fbt@psr-suspend.html

  * igt@kms_flip@2x-flip-vs-modeset-vs-hang@ab-hdmi-a1-hdmi-a2:
    - shard-glk:          [INCOMPLETE][50] -> [PASS][51]
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-glk7/igt@kms_flip@2x-flip-vs-modeset-vs-hang@ab-hdmi-a1-hdmi-a2.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-glk4/igt@kms_flip@2x-flip-vs-modeset-vs-hang@ab-hdmi-a1-hdmi-a2.html

  * igt@kms_flip@2x-plain-flip-fb-recreate@ac-vga1-hdmi-a1:
    - shard-hsw:          [FAIL][52] ([i915#2122]) -> [PASS][53]
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-hsw6/igt@kms_flip@2x-plain-flip-fb-recreate@ac-vga1-hdmi-a1.html
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-hsw7/igt@kms_flip@2x-plain-flip-fb-recreate@ac-vga1-hdmi-a1.html

  * igt@kms_flip@plain-flip-fb-recreate@b-edp1:
    - shard-skl:          [FAIL][54] ([i915#2122]) -> [PASS][55] +1 similar issue
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-skl7/igt@kms_flip@plain-flip-fb-recreate@b-edp1.html
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl4/igt@kms_flip@plain-flip-fb-recreate@b-edp1.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-indfb-pgflip-blt:
    - shard-glk:          [FAIL][56] ([i915#49]) -> [PASS][57]
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-glk5/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-indfb-pgflip-blt.html
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-glk2/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-indfb-pgflip-blt.html

  * igt@kms_hdr@bpc-switch-dpms:
    - shard-skl:          [FAIL][58] ([i915#1188]) -> [PASS][59] +1 similar issue
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-skl3/igt@kms_hdr@bpc-switch-dpms.html
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl6/igt@kms_hdr@bpc-switch-dpms.html

  * igt@kms_psr@psr2_basic:
    - shard-iclb:         [SKIP][60] ([fdo#109441]) -> [PASS][61]
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-iclb6/igt@kms_psr@psr2_basic.html
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-iclb2/igt@kms_psr@psr2_basic.html

  * {igt@perf@non-zero-reason}:
    - shard-iclb:         [FAIL][62] -> [PASS][63]
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-iclb5/igt@perf@non-zero-reason.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-iclb4/igt@perf@non-zero-reason.html

  
#### Warnings ####

  * igt@i915_pm_rc6_residency@rc6-idle:
    - shard-iclb:         [WARN][64] ([i915#1804] / [i915#2684]) -> [WARN][65] ([i915#2681] / [i915#2684])
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-iclb7/igt@i915_pm_rc6_residency@rc6-idle.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-iclb8/igt@i915_pm_rc6_residency@rc6-idle.html

  * igt@runner@aborted:
    - shard-kbl:          [FAIL][66] ([i915#2295] / [i915#2722] / [i915#483]) -> [FAIL][67] ([i915#2295] / [i915#2722])
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-kbl4/igt@runner@aborted.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-kbl7/igt@runner@aborted.html
    - shard-iclb:         [FAIL][68] ([i915#2295] / [i915#2722] / [i915#2724] / [i915#483]) -> [FAIL][69] ([i915#2295] / [i915#2722] / [i915#2724])
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-iclb6/igt@runner@aborted.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-iclb2/igt@runner@aborted.html
    - shard-apl:          ([FAIL][70], [FAIL][71]) ([i915#1610] / [i915#2295] / [i915#2426] / [i915#2722]) -> [FAIL][72] ([i915#2295] / [i915#2722])
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-apl7/igt@runner@aborted.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-apl8/igt@runner@aborted.html
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-apl1/igt@runner@aborted.html
    - shard-skl:          [FAIL][73] ([i915#2295] / [i915#2722]) -> ([FAIL][74], [FAIL][75]) ([i915#2295] / [i915#2426] / [i915#2722] / [i915#483])
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_9489/shard-skl2/igt@runner@aborted.html
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl3/igt@runner@aborted.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_19151/shard-skl2/igt@runner@aborted.html

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

  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
  [i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
  [i915#146]: https://gitlab.freedesktop.org/drm/intel/issues/146
  [i915#1519]: https://gitlab.freedesktop.org/drm/intel/issues/1519
  [i915#1610]: https://gitlab.freedesktop.org/drm/intel/issues/1610
  [i915#1731]: https://gitlab.freedesktop.org/drm/intel/issues/1731
  [i915#1804]: https://gitlab.freedesktop.org/drm/intel/issues/1804
  [i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
  [i915#2122]: https://gitlab.freedesktop.org/drm/intel/issues/2122
  [i915#2291]: https://gitlab.freedesktop.org/drm/intel/issues/2291
  [i915#2295]: https://gitlab.freedesktop.org/drm/intel/issues/2295
  [i915#2346]: https://gitlab.freedesktop.org/drm/intel/issues/2346
  [i915#2369]: https://gitlab.freedesktop.org/drm/intel/issues/2369
  [i915#2426]: https://gitlab.freedesktop.org/drm/intel/issues/2426
  [i915#2502]: https://gitlab.freedesktop.org/drm/intel/issues/2502
  [i915#2521]: https://gitlab.freedesktop.org/drm/intel/issues/2521
  [i915#2598]: https://gitlab.freedesktop.org/drm/intel/issues/2598
  [i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
  [i915#2658]: https://gitlab.freedesktop.org/drm/intel/issues/2658
  [i915#2681]: https://gitlab.freedesktop.org/drm/intel/issues/2681
  [i915#2684]: https://gitlab.freedesktop.org/drm/intel/issues/2684
  [i915#2722]: https://gitlab.freedesktop.org/drm/intel/issues/2722
  [i915#2724]: https://gitlab.freedesktop.org/drm/intel/issues/2724
  [i915#2795]: https://gitlab.freedesktop.org/drm/intel/issues/2795
  [i915#483]: https://gitlab.freedesktop.org/drm/intel/issues/483
  [i915#49]: https://gitlab.freedesktop.org/drm/intel/issues/49
  [i915#54]: https://gitlab.freedesktop.org/drm/intel/issues/54
  [i915#644]: https://gitlab.freedesktop.org/drm/intel/issues/644
  [i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
  [i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95


Participating hosts (10 -> 10)
------------------------------

  No changes in participating hosts


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

  * Linux: CI_DRM_9489 -> Patchwork_19151

  CI-20190529: 20190529
  CI_DRM_9489: bef2104ec6e0aa163b1b01b661e734b08b567aeb @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5902: 1c1fc6c4d506dc69d8e85b09bcb932466712d416 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_19151: 4db0badf5a249da08dd4a6635c22eaef89b407f4 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

[-- Attachment #1.2: Type: text/html, Size: 20933 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

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

* Re: [Intel-gfx] [PATCH 1/2] i915/perf: Move gen specific OA formats to single array
  2020-12-15 21:49 [Intel-gfx] [PATCH 1/2] i915/perf: Move gen specific OA formats to single array Umesh Nerlige Ramappa
                   ` (3 preceding siblings ...)
  2020-12-16  7:08 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
@ 2020-12-16  8:30 ` Lionel Landwerlin
  2020-12-18  2:08   ` Umesh Nerlige Ramappa
  2020-12-16  9:25 ` Tvrtko Ursulin
  5 siblings, 1 reply; 11+ messages in thread
From: Lionel Landwerlin @ 2020-12-16  8:30 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa, intel-gfx

On 15/12/2020 23:49, Umesh Nerlige Ramappa wrote:
> Variations in OA formats in the different gens has led to creation of
> several sparse arrays to store the formats.
>
> Move oa formats into a single array and add the supported range of
> platforms for the oa formats.
>
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_perf.c       | 56 ++++++++++++--------------
>   drivers/gpu/drm/i915/i915_perf_types.h |  3 ++
>   2 files changed, 28 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index f553caf4b06d..afa92cf075c4 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -298,28 +298,27 @@ static u32 i915_oa_max_sample_rate = 100000;
>   
>   /* XXX: beware if future OA HW adds new report formats that the current
>    * code assumes all reports have a power-of-two size and ~(size - 1) can
> - * be used as a mask to align the OA tail pointer.
> + * be used as a mask to align the OA tail pointer. Note that the platforms
> + * in this array specify a range (inclusive of start and end).
>    */
> -static const struct i915_oa_format hsw_oa_formats[I915_OA_FORMAT_MAX] = {
> -	[I915_OA_FORMAT_A13]	    = { 0, 64 },
> -	[I915_OA_FORMAT_A29]	    = { 1, 128 },
> -	[I915_OA_FORMAT_A13_B8_C8]  = { 2, 128 },
> -	/* A29_B8_C8 Disallowed as 192 bytes doesn't factor into buffer size */
> -	[I915_OA_FORMAT_B4_C8]	    = { 4, 64 },
> -	[I915_OA_FORMAT_A45_B8_C8]  = { 5, 256 },
> -	[I915_OA_FORMAT_B4_C8_A16]  = { 6, 128 },
> -	[I915_OA_FORMAT_C4_B8]	    = { 7, 64 },
> -};
> -
> -static const struct i915_oa_format gen8_plus_oa_formats[I915_OA_FORMAT_MAX] = {
> -	[I915_OA_FORMAT_A12]		    = { 0, 64 },
> -	[I915_OA_FORMAT_A12_B8_C8]	    = { 2, 128 },
> -	[I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
> -	[I915_OA_FORMAT_C4_B8]		    = { 7, 64 },
> -};
> -
> -static const struct i915_oa_format gen12_oa_formats[I915_OA_FORMAT_MAX] = {
> -	[I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
> +static const struct i915_oa_format oa_formats[I915_OA_FORMAT_MAX] = {
> +	/* haswell */
> +	[I915_OA_FORMAT_A13]                    = { 0, 64, INTEL_IVYBRIDGE, INTEL_HASWELL },
> +	[I915_OA_FORMAT_A29]                    = { 1, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
> +	[I915_OA_FORMAT_A13_B8_C8]              = { 2, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
> +	[I915_OA_FORMAT_B4_C8]                  = { 4, 64, INTEL_IVYBRIDGE, INTEL_HASWELL },
> +	[I915_OA_FORMAT_A45_B8_C8]              = { 5, 256, INTEL_IVYBRIDGE, INTEL_HASWELL },
> +	[I915_OA_FORMAT_B4_C8_A16]              = { 6, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
> +
> +	/* haswell+ upto but not including tigerlake */
> +	[I915_OA_FORMAT_C4_B8]                  = { 7, 64, INTEL_IVYBRIDGE, INTEL_TIGERLAKE - 1 },
I don't think we support IVB,.
> +
> +	/* gen8+ upto but not including tigerlake */
> +	[I915_OA_FORMAT_A12]                    = { 0, 64, INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
> +	[I915_OA_FORMAT_A12_B8_C8]              = { 2, 128, INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
> +
> +	/* gen8+ */
> +	[I915_OA_FORMAT_A32u40_A4u32_B8_C8]     = { 5, 256, INTEL_BROADWELL, INTEL_MAX_PLATFORMS - 1 },
>   };
>   


You also need to change i915_oa_stream_init() to use the global array. 
Looks like it will access a NULL pointer atm.


>   #define SAMPLE_OA_REPORT      (1<<0)
> @@ -3575,6 +3574,7 @@ static int read_properties_unlocked(struct i915_perf *perf,
>   	for (i = 0; i < n_props; i++) {
>   		u64 oa_period, oa_freq_hz;
>   		u64 id, value;
> +		enum intel_platform p = INTEL_INFO(perf->i915)->platform;
>   
>   		ret = get_user(id, uprop);
>   		if (ret)
> @@ -3611,8 +3611,9 @@ static int read_properties_unlocked(struct i915_perf *perf,
>   					  value);
>   				return -EINVAL;
>   			}
> -			if (!perf->oa_formats[value].size) {
> -				DRM_DEBUG("Unsupported OA report format %llu\n",
> +			if (p < perf->oa_formats[value].start ||
> +			    p > perf->oa_formats[value].end) {
> +				DRM_DEBUG("OA report format not supported %llu\n",
>   					  value);
>   				return -EINVAL;
>   			}
> @@ -4270,6 +4271,7 @@ void i915_perf_init(struct drm_i915_private *i915)
>   
>   	/* XXX const struct i915_perf_ops! */
>   
> +	perf->oa_formats = oa_formats;
>   	if (IS_HASWELL(i915)) {
>   		perf->ops.is_valid_b_counter_reg = gen7_is_valid_b_counter_addr;
>   		perf->ops.is_valid_mux_reg = hsw_is_valid_mux_addr;
> @@ -4280,8 +4282,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>   		perf->ops.oa_disable = gen7_oa_disable;
>   		perf->ops.read = gen7_oa_read;
>   		perf->ops.oa_hw_tail_read = gen7_oa_hw_tail_read;
> -
> -		perf->oa_formats = hsw_oa_formats;
>   	} else if (HAS_LOGICAL_RING_CONTEXTS(i915)) {
>   		/* Note: that although we could theoretically also support the
>   		 * legacy ringbuffer mode on BDW (and earlier iterations of
> @@ -4292,8 +4292,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>   		perf->ops.read = gen8_oa_read;
>   
>   		if (IS_GEN_RANGE(i915, 8, 9)) {
> -			perf->oa_formats = gen8_plus_oa_formats;
> -
>   			perf->ops.is_valid_b_counter_reg =
>   				gen7_is_valid_b_counter_addr;
>   			perf->ops.is_valid_mux_reg =
> @@ -4324,8 +4322,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>   				perf->gen8_valid_ctx_bit = BIT(16);
>   			}
>   		} else if (IS_GEN_RANGE(i915, 10, 11)) {
> -			perf->oa_formats = gen8_plus_oa_formats;
> -
>   			perf->ops.is_valid_b_counter_reg =
>   				gen7_is_valid_b_counter_addr;
>   			perf->ops.is_valid_mux_reg =
> @@ -4348,8 +4344,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>   			}
>   			perf->gen8_valid_ctx_bit = BIT(16);
>   		} else if (IS_GEN(i915, 12)) {
> -			perf->oa_formats = gen12_oa_formats;
> -
>   			perf->ops.is_valid_b_counter_reg =
>   				gen12_is_valid_b_counter_addr;
>   			perf->ops.is_valid_mux_reg =
> diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
> index a36a455ae336..84dceb743ebc 100644
> --- a/drivers/gpu/drm/i915/i915_perf_types.h
> +++ b/drivers/gpu/drm/i915/i915_perf_types.h
> @@ -19,6 +19,7 @@
>   #include "gt/intel_sseu.h"
>   #include "i915_reg.h"
>   #include "intel_wakeref.h"
> +#include "intel_device_info.h"
>   
>   struct drm_i915_private;
>   struct file;
> @@ -32,6 +33,8 @@ struct intel_engine_cs;
>   struct i915_oa_format {
>   	u32 format;
>   	int size;
> +	enum intel_platform start;
> +	enum intel_platform end;


Also need to drop oa_formats from struct i915_perf


>   };
>   
>   struct i915_oa_reg {


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

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

* Re: [Intel-gfx] [PATCH 1/2] i915/perf: Move gen specific OA formats to single array
  2020-12-15 21:49 [Intel-gfx] [PATCH 1/2] i915/perf: Move gen specific OA formats to single array Umesh Nerlige Ramappa
                   ` (4 preceding siblings ...)
  2020-12-16  8:30 ` [Intel-gfx] [PATCH 1/2] " Lionel Landwerlin
@ 2020-12-16  9:25 ` Tvrtko Ursulin
  2020-12-18  2:32   ` Umesh Nerlige Ramappa
  5 siblings, 1 reply; 11+ messages in thread
From: Tvrtko Ursulin @ 2020-12-16  9:25 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa, intel-gfx, Lionel G Landwerlin


On 15/12/2020 21:49, Umesh Nerlige Ramappa wrote:
> Variations in OA formats in the different gens has led to creation of
> several sparse arrays to store the formats.
> 
> Move oa formats into a single array and add the supported range of
> platforms for the oa formats.
> 
> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_perf.c       | 56 ++++++++++++--------------
>   drivers/gpu/drm/i915/i915_perf_types.h |  3 ++
>   2 files changed, 28 insertions(+), 31 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index f553caf4b06d..afa92cf075c4 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -298,28 +298,27 @@ static u32 i915_oa_max_sample_rate = 100000;
>   
>   /* XXX: beware if future OA HW adds new report formats that the current
>    * code assumes all reports have a power-of-two size and ~(size - 1) can
> - * be used as a mask to align the OA tail pointer.
> + * be used as a mask to align the OA tail pointer. Note that the platforms
> + * in this array specify a range (inclusive of start and end).
>    */
> -static const struct i915_oa_format hsw_oa_formats[I915_OA_FORMAT_MAX] = {
> -	[I915_OA_FORMAT_A13]	    = { 0, 64 },
> -	[I915_OA_FORMAT_A29]	    = { 1, 128 },
> -	[I915_OA_FORMAT_A13_B8_C8]  = { 2, 128 },
> -	/* A29_B8_C8 Disallowed as 192 bytes doesn't factor into buffer size */
> -	[I915_OA_FORMAT_B4_C8]	    = { 4, 64 },
> -	[I915_OA_FORMAT_A45_B8_C8]  = { 5, 256 },
> -	[I915_OA_FORMAT_B4_C8_A16]  = { 6, 128 },
> -	[I915_OA_FORMAT_C4_B8]	    = { 7, 64 },
> -};
> -
> -static const struct i915_oa_format gen8_plus_oa_formats[I915_OA_FORMAT_MAX] = {
> -	[I915_OA_FORMAT_A12]		    = { 0, 64 },
> -	[I915_OA_FORMAT_A12_B8_C8]	    = { 2, 128 },
> -	[I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
> -	[I915_OA_FORMAT_C4_B8]		    = { 7, 64 },
> -};
> -
> -static const struct i915_oa_format gen12_oa_formats[I915_OA_FORMAT_MAX] = {
> -	[I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
> +static const struct i915_oa_format oa_formats[I915_OA_FORMAT_MAX] = {
> +	/* haswell */
> +	[I915_OA_FORMAT_A13]                    = { 0, 64, INTEL_IVYBRIDGE, INTEL_HASWELL },
> +	[I915_OA_FORMAT_A29]                    = { 1, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
> +	[I915_OA_FORMAT_A13_B8_C8]              = { 2, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
> +	[I915_OA_FORMAT_B4_C8]                  = { 4, 64, INTEL_IVYBRIDGE, INTEL_HASWELL },
> +	[I915_OA_FORMAT_A45_B8_C8]              = { 5, 256, INTEL_IVYBRIDGE, INTEL_HASWELL },
> +	[I915_OA_FORMAT_B4_C8_A16]              = { 6, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
> +
> +	/* haswell+ upto but not including tigerlake */
> +	[I915_OA_FORMAT_C4_B8]                  = { 7, 64, INTEL_IVYBRIDGE, INTEL_TIGERLAKE - 1 },
> +
> +	/* gen8+ upto but not including tigerlake */
> +	[I915_OA_FORMAT_A12]                    = { 0, 64, INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
> +	[I915_OA_FORMAT_A12_B8_C8]              = { 2, 128, INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
> +
> +	/* gen8+ */
> +	[I915_OA_FORMAT_A32u40_A4u32_B8_C8]     = { 5, 256, INTEL_BROADWELL, INTEL_MAX_PLATFORMS - 1 },
>   };
>   
>   #define SAMPLE_OA_REPORT      (1<<0)
> @@ -3575,6 +3574,7 @@ static int read_properties_unlocked(struct i915_perf *perf,
>   	for (i = 0; i < n_props; i++) {
>   		u64 oa_period, oa_freq_hz;
>   		u64 id, value;
> +		enum intel_platform p = INTEL_INFO(perf->i915)->platform;
>   
>   		ret = get_user(id, uprop);
>   		if (ret)
> @@ -3611,8 +3611,9 @@ static int read_properties_unlocked(struct i915_perf *perf,
>   					  value);
>   				return -EINVAL;
>   			}
> -			if (!perf->oa_formats[value].size) {
> -				DRM_DEBUG("Unsupported OA report format %llu\n",
> +			if (p < perf->oa_formats[value].start ||
> +			    p > perf->oa_formats[value].end) {
> +				DRM_DEBUG("OA report format not supported %llu\n",
>   					  value);
>   				return -EINVAL;
>   			}
> @@ -4270,6 +4271,7 @@ void i915_perf_init(struct drm_i915_private *i915)
>   
>   	/* XXX const struct i915_perf_ops! */
>   
> +	perf->oa_formats = oa_formats;
>   	if (IS_HASWELL(i915)) {
>   		perf->ops.is_valid_b_counter_reg = gen7_is_valid_b_counter_addr;
>   		perf->ops.is_valid_mux_reg = hsw_is_valid_mux_addr;
> @@ -4280,8 +4282,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>   		perf->ops.oa_disable = gen7_oa_disable;
>   		perf->ops.read = gen7_oa_read;
>   		perf->ops.oa_hw_tail_read = gen7_oa_hw_tail_read;
> -
> -		perf->oa_formats = hsw_oa_formats;
>   	} else if (HAS_LOGICAL_RING_CONTEXTS(i915)) {
>   		/* Note: that although we could theoretically also support the
>   		 * legacy ringbuffer mode on BDW (and earlier iterations of
> @@ -4292,8 +4292,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>   		perf->ops.read = gen8_oa_read;
>   
>   		if (IS_GEN_RANGE(i915, 8, 9)) {
> -			perf->oa_formats = gen8_plus_oa_formats;
> -
>   			perf->ops.is_valid_b_counter_reg =
>   				gen7_is_valid_b_counter_addr;
>   			perf->ops.is_valid_mux_reg =
> @@ -4324,8 +4322,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>   				perf->gen8_valid_ctx_bit = BIT(16);
>   			}
>   		} else if (IS_GEN_RANGE(i915, 10, 11)) {
> -			perf->oa_formats = gen8_plus_oa_formats;
> -
>   			perf->ops.is_valid_b_counter_reg =
>   				gen7_is_valid_b_counter_addr;
>   			perf->ops.is_valid_mux_reg =
> @@ -4348,8 +4344,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>   			}
>   			perf->gen8_valid_ctx_bit = BIT(16);
>   		} else if (IS_GEN(i915, 12)) {
> -			perf->oa_formats = gen12_oa_formats;
> -
>   			perf->ops.is_valid_b_counter_reg =
>   				gen12_is_valid_b_counter_addr;
>   			perf->ops.is_valid_mux_reg =
> diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
> index a36a455ae336..84dceb743ebc 100644
> --- a/drivers/gpu/drm/i915/i915_perf_types.h
> +++ b/drivers/gpu/drm/i915/i915_perf_types.h
> @@ -19,6 +19,7 @@
>   #include "gt/intel_sseu.h"
>   #include "i915_reg.h"
>   #include "intel_wakeref.h"
> +#include "intel_device_info.h"
>   
>   struct drm_i915_private;
>   struct file;
> @@ -32,6 +33,8 @@ struct intel_engine_cs;
>   struct i915_oa_format {
>   	u32 format;
>   	int size;
> +	enum intel_platform start;
> +	enum intel_platform end;

As a drive by only - I question the concept of thinking about platforms 
enums as ordered and having things like INTEL_TIGERLAKE - 1 in the code. 
It's completely novel concept and I think fragile. Order there is purely 
the order of upstreaming and not stable.

Suggestions wise, if the rules are Gen based you could use a mask, but 
if they are truly platform based then it is more tricky. However current 
code does use gen ranges to assign them, so would a genmask really not fit?

Regards,

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

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

* Re: [Intel-gfx] [PATCH 1/2] i915/perf: Move gen specific OA formats to single array
  2020-12-16  8:30 ` [Intel-gfx] [PATCH 1/2] " Lionel Landwerlin
@ 2020-12-18  2:08   ` Umesh Nerlige Ramappa
  2020-12-18  9:47     ` Lionel Landwerlin
  0 siblings, 1 reply; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2020-12-18  2:08 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

On Wed, Dec 16, 2020 at 10:30:24AM +0200, Lionel Landwerlin wrote:
>On 15/12/2020 23:49, Umesh Nerlige Ramappa wrote:
>>Variations in OA formats in the different gens has led to creation of
>>several sparse arrays to store the formats.
>>
>>Move oa formats into a single array and add the supported range of
>>platforms for the oa formats.
>>
>>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>>---
>>  drivers/gpu/drm/i915/i915_perf.c       | 56 ++++++++++++--------------
>>  drivers/gpu/drm/i915/i915_perf_types.h |  3 ++
>>  2 files changed, 28 insertions(+), 31 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>>index f553caf4b06d..afa92cf075c4 100644
>>--- a/drivers/gpu/drm/i915/i915_perf.c
>>+++ b/drivers/gpu/drm/i915/i915_perf.c
>>@@ -298,28 +298,27 @@ static u32 i915_oa_max_sample_rate = 100000;
>>  /* XXX: beware if future OA HW adds new report formats that the current
>>   * code assumes all reports have a power-of-two size and ~(size - 1) can
>>- * be used as a mask to align the OA tail pointer.
>>+ * be used as a mask to align the OA tail pointer. Note that the platforms
>>+ * in this array specify a range (inclusive of start and end).
>>   */
>>-static const struct i915_oa_format hsw_oa_formats[I915_OA_FORMAT_MAX] = {
>>-	[I915_OA_FORMAT_A13]	    = { 0, 64 },
>>-	[I915_OA_FORMAT_A29]	    = { 1, 128 },
>>-	[I915_OA_FORMAT_A13_B8_C8]  = { 2, 128 },
>>-	/* A29_B8_C8 Disallowed as 192 bytes doesn't factor into buffer size */
>>-	[I915_OA_FORMAT_B4_C8]	    = { 4, 64 },
>>-	[I915_OA_FORMAT_A45_B8_C8]  = { 5, 256 },
>>-	[I915_OA_FORMAT_B4_C8_A16]  = { 6, 128 },
>>-	[I915_OA_FORMAT_C4_B8]	    = { 7, 64 },
>>-};
>>-
>>-static const struct i915_oa_format gen8_plus_oa_formats[I915_OA_FORMAT_MAX] = {
>>-	[I915_OA_FORMAT_A12]		    = { 0, 64 },
>>-	[I915_OA_FORMAT_A12_B8_C8]	    = { 2, 128 },
>>-	[I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
>>-	[I915_OA_FORMAT_C4_B8]		    = { 7, 64 },
>>-};
>>-
>>-static const struct i915_oa_format gen12_oa_formats[I915_OA_FORMAT_MAX] = {
>>-	[I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
>>+static const struct i915_oa_format oa_formats[I915_OA_FORMAT_MAX] = {
>>+	/* haswell */
>>+	[I915_OA_FORMAT_A13]                    = { 0, 64, INTEL_IVYBRIDGE, INTEL_HASWELL },
>>+	[I915_OA_FORMAT_A29]                    = { 1, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
>>+	[I915_OA_FORMAT_A13_B8_C8]              = { 2, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
>>+	[I915_OA_FORMAT_B4_C8]                  = { 4, 64, INTEL_IVYBRIDGE, INTEL_HASWELL },
>>+	[I915_OA_FORMAT_A45_B8_C8]              = { 5, 256, INTEL_IVYBRIDGE, INTEL_HASWELL },
>>+	[I915_OA_FORMAT_B4_C8_A16]              = { 6, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
>>+
>>+	/* haswell+ upto but not including tigerlake */
>>+	[I915_OA_FORMAT_C4_B8]                  = { 7, 64, INTEL_IVYBRIDGE, INTEL_TIGERLAKE - 1 },
>I don't think we support IVB,.

So the above formats are only HASWELL then?

>>+
>>+	/* gen8+ upto but not including tigerlake */
>>+	[I915_OA_FORMAT_A12]                    = { 0, 64, INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
>>+	[I915_OA_FORMAT_A12_B8_C8]              = { 2, 128, INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
>>+
>>+	/* gen8+ */
>>+	[I915_OA_FORMAT_A32u40_A4u32_B8_C8]     = { 5, 256, INTEL_BROADWELL, INTEL_MAX_PLATFORMS - 1 },
>>  };
>
>
>You also need to change i915_oa_stream_init() to use the global array. 
>Looks like it will access a NULL pointer atm.
>

yikes, will do.

>
>>  #define SAMPLE_OA_REPORT      (1<<0)
>>@@ -3575,6 +3574,7 @@ static int read_properties_unlocked(struct i915_perf *perf,
>>  	for (i = 0; i < n_props; i++) {
>>  		u64 oa_period, oa_freq_hz;
>>  		u64 id, value;
>>+		enum intel_platform p = INTEL_INFO(perf->i915)->platform;
>>  		ret = get_user(id, uprop);
>>  		if (ret)
>>@@ -3611,8 +3611,9 @@ static int read_properties_unlocked(struct i915_perf *perf,
>>  					  value);
>>  				return -EINVAL;
>>  			}
>>-			if (!perf->oa_formats[value].size) {
>>-				DRM_DEBUG("Unsupported OA report format %llu\n",
>>+			if (p < perf->oa_formats[value].start ||
>>+			    p > perf->oa_formats[value].end) {
>>+				DRM_DEBUG("OA report format not supported %llu\n",
>>  					  value);
>>  				return -EINVAL;
>>  			}
>>@@ -4270,6 +4271,7 @@ void i915_perf_init(struct drm_i915_private *i915)
>>  	/* XXX const struct i915_perf_ops! */
>>+	perf->oa_formats = oa_formats;
>>  	if (IS_HASWELL(i915)) {
>>  		perf->ops.is_valid_b_counter_reg = gen7_is_valid_b_counter_addr;
>>  		perf->ops.is_valid_mux_reg = hsw_is_valid_mux_addr;
>>@@ -4280,8 +4282,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>>  		perf->ops.oa_disable = gen7_oa_disable;
>>  		perf->ops.read = gen7_oa_read;
>>  		perf->ops.oa_hw_tail_read = gen7_oa_hw_tail_read;
>>-
>>-		perf->oa_formats = hsw_oa_formats;
>>  	} else if (HAS_LOGICAL_RING_CONTEXTS(i915)) {
>>  		/* Note: that although we could theoretically also support the
>>  		 * legacy ringbuffer mode on BDW (and earlier iterations of
>>@@ -4292,8 +4292,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>>  		perf->ops.read = gen8_oa_read;
>>  		if (IS_GEN_RANGE(i915, 8, 9)) {
>>-			perf->oa_formats = gen8_plus_oa_formats;
>>-
>>  			perf->ops.is_valid_b_counter_reg =
>>  				gen7_is_valid_b_counter_addr;
>>  			perf->ops.is_valid_mux_reg =
>>@@ -4324,8 +4322,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>>  				perf->gen8_valid_ctx_bit = BIT(16);
>>  			}
>>  		} else if (IS_GEN_RANGE(i915, 10, 11)) {
>>-			perf->oa_formats = gen8_plus_oa_formats;
>>-
>>  			perf->ops.is_valid_b_counter_reg =
>>  				gen7_is_valid_b_counter_addr;
>>  			perf->ops.is_valid_mux_reg =
>>@@ -4348,8 +4344,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>>  			}
>>  			perf->gen8_valid_ctx_bit = BIT(16);
>>  		} else if (IS_GEN(i915, 12)) {
>>-			perf->oa_formats = gen12_oa_formats;
>>-
>>  			perf->ops.is_valid_b_counter_reg =
>>  				gen12_is_valid_b_counter_addr;
>>  			perf->ops.is_valid_mux_reg =
>>diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
>>index a36a455ae336..84dceb743ebc 100644
>>--- a/drivers/gpu/drm/i915/i915_perf_types.h
>>+++ b/drivers/gpu/drm/i915/i915_perf_types.h
>>@@ -19,6 +19,7 @@
>>  #include "gt/intel_sseu.h"
>>  #include "i915_reg.h"
>>  #include "intel_wakeref.h"
>>+#include "intel_device_info.h"
>>  struct drm_i915_private;
>>  struct file;
>>@@ -32,6 +33,8 @@ struct intel_engine_cs;
>>  struct i915_oa_format {
>>  	u32 format;
>>  	int size;
>>+	enum intel_platform start;
>>+	enum intel_platform end;
>
>
>Also need to drop oa_formats from struct i915_perf

oa_formats still remains in i915_perf. It's just that

perf->oa_formats = oa_formats;

for all platforms.

Thanks,
Umesh
>
>
>>  };
>>  struct i915_oa_reg {
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [Intel-gfx] [PATCH 1/2] i915/perf: Move gen specific OA formats to single array
  2020-12-16  9:25 ` Tvrtko Ursulin
@ 2020-12-18  2:32   ` Umesh Nerlige Ramappa
  2020-12-18  9:20     ` Tvrtko Ursulin
  0 siblings, 1 reply; 11+ messages in thread
From: Umesh Nerlige Ramappa @ 2020-12-18  2:32 UTC (permalink / raw)
  To: Tvrtko Ursulin; +Cc: intel-gfx

On Wed, Dec 16, 2020 at 09:25:35AM +0000, Tvrtko Ursulin wrote:
>
>On 15/12/2020 21:49, Umesh Nerlige Ramappa wrote:
>>Variations in OA formats in the different gens has led to creation of
>>several sparse arrays to store the formats.
>>
>>Move oa formats into a single array and add the supported range of
>>platforms for the oa formats.
>>
>>Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>>---
>>  drivers/gpu/drm/i915/i915_perf.c       | 56 ++++++++++++--------------
>>  drivers/gpu/drm/i915/i915_perf_types.h |  3 ++
>>  2 files changed, 28 insertions(+), 31 deletions(-)
>>
>>diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>>index f553caf4b06d..afa92cf075c4 100644
>>--- a/drivers/gpu/drm/i915/i915_perf.c
>>+++ b/drivers/gpu/drm/i915/i915_perf.c
>>@@ -298,28 +298,27 @@ static u32 i915_oa_max_sample_rate = 100000;
>>  /* XXX: beware if future OA HW adds new report formats that the current
>>   * code assumes all reports have a power-of-two size and ~(size - 1) can
>>- * be used as a mask to align the OA tail pointer.
>>+ * be used as a mask to align the OA tail pointer. Note that the platforms
>>+ * in this array specify a range (inclusive of start and end).
>>   */
>>-static const struct i915_oa_format hsw_oa_formats[I915_OA_FORMAT_MAX] = {
>>-	[I915_OA_FORMAT_A13]	    = { 0, 64 },
>>-	[I915_OA_FORMAT_A29]	    = { 1, 128 },
>>-	[I915_OA_FORMAT_A13_B8_C8]  = { 2, 128 },
>>-	/* A29_B8_C8 Disallowed as 192 bytes doesn't factor into buffer size */
>>-	[I915_OA_FORMAT_B4_C8]	    = { 4, 64 },
>>-	[I915_OA_FORMAT_A45_B8_C8]  = { 5, 256 },
>>-	[I915_OA_FORMAT_B4_C8_A16]  = { 6, 128 },
>>-	[I915_OA_FORMAT_C4_B8]	    = { 7, 64 },
>>-};
>>-
>>-static const struct i915_oa_format gen8_plus_oa_formats[I915_OA_FORMAT_MAX] = {
>>-	[I915_OA_FORMAT_A12]		    = { 0, 64 },
>>-	[I915_OA_FORMAT_A12_B8_C8]	    = { 2, 128 },
>>-	[I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
>>-	[I915_OA_FORMAT_C4_B8]		    = { 7, 64 },
>>-};
>>-
>>-static const struct i915_oa_format gen12_oa_formats[I915_OA_FORMAT_MAX] = {
>>-	[I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
>>+static const struct i915_oa_format oa_formats[I915_OA_FORMAT_MAX] = {
>>+	/* haswell */
>>+	[I915_OA_FORMAT_A13]                    = { 0, 64, INTEL_IVYBRIDGE, INTEL_HASWELL },
>>+	[I915_OA_FORMAT_A29]                    = { 1, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
>>+	[I915_OA_FORMAT_A13_B8_C8]              = { 2, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
>>+	[I915_OA_FORMAT_B4_C8]                  = { 4, 64, INTEL_IVYBRIDGE, INTEL_HASWELL },
>>+	[I915_OA_FORMAT_A45_B8_C8]              = { 5, 256, INTEL_IVYBRIDGE, INTEL_HASWELL },
>>+	[I915_OA_FORMAT_B4_C8_A16]              = { 6, 128, INTEL_IVYBRIDGE, INTEL_HASWELL },
>>+
>>+	/* haswell+ upto but not including tigerlake */
>>+	[I915_OA_FORMAT_C4_B8]                  = { 7, 64, INTEL_IVYBRIDGE, INTEL_TIGERLAKE - 1 },
>>+
>>+	/* gen8+ upto but not including tigerlake */
>>+	[I915_OA_FORMAT_A12]                    = { 0, 64, INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
>>+	[I915_OA_FORMAT_A12_B8_C8]              = { 2, 128, INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
>>+
>>+	/* gen8+ */
>>+	[I915_OA_FORMAT_A32u40_A4u32_B8_C8]     = { 5, 256, INTEL_BROADWELL, INTEL_MAX_PLATFORMS - 1 },
>>  };
>>  #define SAMPLE_OA_REPORT      (1<<0)
>>@@ -3575,6 +3574,7 @@ static int read_properties_unlocked(struct i915_perf *perf,
>>  	for (i = 0; i < n_props; i++) {
>>  		u64 oa_period, oa_freq_hz;
>>  		u64 id, value;
>>+		enum intel_platform p = INTEL_INFO(perf->i915)->platform;
>>  		ret = get_user(id, uprop);
>>  		if (ret)
>>@@ -3611,8 +3611,9 @@ static int read_properties_unlocked(struct i915_perf *perf,
>>  					  value);
>>  				return -EINVAL;
>>  			}
>>-			if (!perf->oa_formats[value].size) {
>>-				DRM_DEBUG("Unsupported OA report format %llu\n",
>>+			if (p < perf->oa_formats[value].start ||
>>+			    p > perf->oa_formats[value].end) {
>>+				DRM_DEBUG("OA report format not supported %llu\n",
>>  					  value);
>>  				return -EINVAL;
>>  			}
>>@@ -4270,6 +4271,7 @@ void i915_perf_init(struct drm_i915_private *i915)
>>  	/* XXX const struct i915_perf_ops! */
>>+	perf->oa_formats = oa_formats;
>>  	if (IS_HASWELL(i915)) {
>>  		perf->ops.is_valid_b_counter_reg = gen7_is_valid_b_counter_addr;
>>  		perf->ops.is_valid_mux_reg = hsw_is_valid_mux_addr;
>>@@ -4280,8 +4282,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>>  		perf->ops.oa_disable = gen7_oa_disable;
>>  		perf->ops.read = gen7_oa_read;
>>  		perf->ops.oa_hw_tail_read = gen7_oa_hw_tail_read;
>>-
>>-		perf->oa_formats = hsw_oa_formats;
>>  	} else if (HAS_LOGICAL_RING_CONTEXTS(i915)) {
>>  		/* Note: that although we could theoretically also support the
>>  		 * legacy ringbuffer mode on BDW (and earlier iterations of
>>@@ -4292,8 +4292,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>>  		perf->ops.read = gen8_oa_read;
>>  		if (IS_GEN_RANGE(i915, 8, 9)) {
>>-			perf->oa_formats = gen8_plus_oa_formats;
>>-
>>  			perf->ops.is_valid_b_counter_reg =
>>  				gen7_is_valid_b_counter_addr;
>>  			perf->ops.is_valid_mux_reg =
>>@@ -4324,8 +4322,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>>  				perf->gen8_valid_ctx_bit = BIT(16);
>>  			}
>>  		} else if (IS_GEN_RANGE(i915, 10, 11)) {
>>-			perf->oa_formats = gen8_plus_oa_formats;
>>-
>>  			perf->ops.is_valid_b_counter_reg =
>>  				gen7_is_valid_b_counter_addr;
>>  			perf->ops.is_valid_mux_reg =
>>@@ -4348,8 +4344,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>>  			}
>>  			perf->gen8_valid_ctx_bit = BIT(16);
>>  		} else if (IS_GEN(i915, 12)) {
>>-			perf->oa_formats = gen12_oa_formats;
>>-
>>  			perf->ops.is_valid_b_counter_reg =
>>  				gen12_is_valid_b_counter_addr;
>>  			perf->ops.is_valid_mux_reg =
>>diff --git a/drivers/gpu/drm/i915/i915_perf_types.h b/drivers/gpu/drm/i915/i915_perf_types.h
>>index a36a455ae336..84dceb743ebc 100644
>>--- a/drivers/gpu/drm/i915/i915_perf_types.h
>>+++ b/drivers/gpu/drm/i915/i915_perf_types.h
>>@@ -19,6 +19,7 @@
>>  #include "gt/intel_sseu.h"
>>  #include "i915_reg.h"
>>  #include "intel_wakeref.h"
>>+#include "intel_device_info.h"
>>  struct drm_i915_private;
>>  struct file;
>>@@ -32,6 +33,8 @@ struct intel_engine_cs;
>>  struct i915_oa_format {
>>  	u32 format;
>>  	int size;
>>+	enum intel_platform start;
>>+	enum intel_platform end;
>
>As a drive by only - I question the concept of thinking about 
>platforms enums as ordered and having things like INTEL_TIGERLAKE - 1 
>in the code. It's completely novel concept and I think fragile. Order 
>there is purely the order of upstreaming and not stable.
>
>Suggestions wise, if the rules are Gen based you could use a mask, but 
>if they are truly platform based then it is more tricky. However 
>current code does use gen ranges to assign them, so would a genmask 
>really not fit?

Hmm, the rules are platform-based. A mask would be ideal, but the last I 
thought about it, it felt more complex than this solution. This code 
would have to maintain a mask of supported platforms (and subplatforms) 
for each entry (preferably in the same format as runtime info -> 
platform_mask).  Then we would just AND this mask with the 
platform_mask/s to check for support. Let me know if that's what you 
have in mind, then I can give it a try.

Thanks,
Umesh

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

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

* Re: [Intel-gfx] [PATCH 1/2] i915/perf: Move gen specific OA formats to single array
  2020-12-18  2:32   ` Umesh Nerlige Ramappa
@ 2020-12-18  9:20     ` Tvrtko Ursulin
  0 siblings, 0 replies; 11+ messages in thread
From: Tvrtko Ursulin @ 2020-12-18  9:20 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: intel-gfx


On 18/12/2020 02:32, Umesh Nerlige Ramappa wrote:
> On Wed, Dec 16, 2020 at 09:25:35AM +0000, Tvrtko Ursulin wrote:
>>
>> On 15/12/2020 21:49, Umesh Nerlige Ramappa wrote:
>>> Variations in OA formats in the different gens has led to creation of
>>> several sparse arrays to store the formats.
>>>
>>> Move oa formats into a single array and add the supported range of
>>> platforms for the oa formats.
>>>
>>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/i915_perf.c       | 56 ++++++++++++--------------
>>>  drivers/gpu/drm/i915/i915_perf_types.h |  3 ++
>>>  2 files changed, 28 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_perf.c 
>>> b/drivers/gpu/drm/i915/i915_perf.c
>>> index f553caf4b06d..afa92cf075c4 100644
>>> --- a/drivers/gpu/drm/i915/i915_perf.c
>>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>>> @@ -298,28 +298,27 @@ static u32 i915_oa_max_sample_rate = 100000;
>>>  /* XXX: beware if future OA HW adds new report formats that the current
>>>   * code assumes all reports have a power-of-two size and ~(size - 1) 
>>> can
>>> - * be used as a mask to align the OA tail pointer.
>>> + * be used as a mask to align the OA tail pointer. Note that the 
>>> platforms
>>> + * in this array specify a range (inclusive of start and end).
>>>   */
>>> -static const struct i915_oa_format 
>>> hsw_oa_formats[I915_OA_FORMAT_MAX] = {
>>> -    [I915_OA_FORMAT_A13]        = { 0, 64 },
>>> -    [I915_OA_FORMAT_A29]        = { 1, 128 },
>>> -    [I915_OA_FORMAT_A13_B8_C8]  = { 2, 128 },
>>> -    /* A29_B8_C8 Disallowed as 192 bytes doesn't factor into buffer 
>>> size */
>>> -    [I915_OA_FORMAT_B4_C8]        = { 4, 64 },
>>> -    [I915_OA_FORMAT_A45_B8_C8]  = { 5, 256 },
>>> -    [I915_OA_FORMAT_B4_C8_A16]  = { 6, 128 },
>>> -    [I915_OA_FORMAT_C4_B8]        = { 7, 64 },
>>> -};
>>> -
>>> -static const struct i915_oa_format 
>>> gen8_plus_oa_formats[I915_OA_FORMAT_MAX] = {
>>> -    [I915_OA_FORMAT_A12]            = { 0, 64 },
>>> -    [I915_OA_FORMAT_A12_B8_C8]        = { 2, 128 },
>>> -    [I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
>>> -    [I915_OA_FORMAT_C4_B8]            = { 7, 64 },
>>> -};
>>> -
>>> -static const struct i915_oa_format 
>>> gen12_oa_formats[I915_OA_FORMAT_MAX] = {
>>> -    [I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
>>> +static const struct i915_oa_format oa_formats[I915_OA_FORMAT_MAX] = {
>>> +    /* haswell */
>>> +    [I915_OA_FORMAT_A13]                    = { 0, 64, 
>>> INTEL_IVYBRIDGE, INTEL_HASWELL },
>>> +    [I915_OA_FORMAT_A29]                    = { 1, 128, 
>>> INTEL_IVYBRIDGE, INTEL_HASWELL },
>>> +    [I915_OA_FORMAT_A13_B8_C8]              = { 2, 128, 
>>> INTEL_IVYBRIDGE, INTEL_HASWELL },
>>> +    [I915_OA_FORMAT_B4_C8]                  = { 4, 64, 
>>> INTEL_IVYBRIDGE, INTEL_HASWELL },
>>> +    [I915_OA_FORMAT_A45_B8_C8]              = { 5, 256, 
>>> INTEL_IVYBRIDGE, INTEL_HASWELL },
>>> +    [I915_OA_FORMAT_B4_C8_A16]              = { 6, 128, 
>>> INTEL_IVYBRIDGE, INTEL_HASWELL },
>>> +
>>> +    /* haswell+ upto but not including tigerlake */
>>> +    [I915_OA_FORMAT_C4_B8]                  = { 7, 64, 
>>> INTEL_IVYBRIDGE, INTEL_TIGERLAKE - 1 },
>>> +
>>> +    /* gen8+ upto but not including tigerlake */
>>> +    [I915_OA_FORMAT_A12]                    = { 0, 64, 
>>> INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
>>> +    [I915_OA_FORMAT_A12_B8_C8]              = { 2, 128, 
>>> INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
>>> +
>>> +    /* gen8+ */
>>> +    [I915_OA_FORMAT_A32u40_A4u32_B8_C8]     = { 5, 256, 
>>> INTEL_BROADWELL, INTEL_MAX_PLATFORMS - 1 },
>>>  };
>>>  #define SAMPLE_OA_REPORT      (1<<0)
>>> @@ -3575,6 +3574,7 @@ static int read_properties_unlocked(struct 
>>> i915_perf *perf,
>>>      for (i = 0; i < n_props; i++) {
>>>          u64 oa_period, oa_freq_hz;
>>>          u64 id, value;
>>> +        enum intel_platform p = INTEL_INFO(perf->i915)->platform;
>>>          ret = get_user(id, uprop);
>>>          if (ret)
>>> @@ -3611,8 +3611,9 @@ static int read_properties_unlocked(struct 
>>> i915_perf *perf,
>>>                        value);
>>>                  return -EINVAL;
>>>              }
>>> -            if (!perf->oa_formats[value].size) {
>>> -                DRM_DEBUG("Unsupported OA report format %llu\n",
>>> +            if (p < perf->oa_formats[value].start ||
>>> +                p > perf->oa_formats[value].end) {
>>> +                DRM_DEBUG("OA report format not supported %llu\n",
>>>                        value);
>>>                  return -EINVAL;
>>>              }
>>> @@ -4270,6 +4271,7 @@ void i915_perf_init(struct drm_i915_private *i915)
>>>      /* XXX const struct i915_perf_ops! */
>>> +    perf->oa_formats = oa_formats;
>>>      if (IS_HASWELL(i915)) {
>>>          perf->ops.is_valid_b_counter_reg = 
>>> gen7_is_valid_b_counter_addr;
>>>          perf->ops.is_valid_mux_reg = hsw_is_valid_mux_addr;
>>> @@ -4280,8 +4282,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>>>          perf->ops.oa_disable = gen7_oa_disable;
>>>          perf->ops.read = gen7_oa_read;
>>>          perf->ops.oa_hw_tail_read = gen7_oa_hw_tail_read;
>>> -
>>> -        perf->oa_formats = hsw_oa_formats;
>>>      } else if (HAS_LOGICAL_RING_CONTEXTS(i915)) {
>>>          /* Note: that although we could theoretically also support the
>>>           * legacy ringbuffer mode on BDW (and earlier iterations of
>>> @@ -4292,8 +4292,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>>>          perf->ops.read = gen8_oa_read;
>>>          if (IS_GEN_RANGE(i915, 8, 9)) {
>>> -            perf->oa_formats = gen8_plus_oa_formats;
>>> -
>>>              perf->ops.is_valid_b_counter_reg =
>>>                  gen7_is_valid_b_counter_addr;
>>>              perf->ops.is_valid_mux_reg =
>>> @@ -4324,8 +4322,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>>>                  perf->gen8_valid_ctx_bit = BIT(16);
>>>              }
>>>          } else if (IS_GEN_RANGE(i915, 10, 11)) {
>>> -            perf->oa_formats = gen8_plus_oa_formats;
>>> -
>>>              perf->ops.is_valid_b_counter_reg =
>>>                  gen7_is_valid_b_counter_addr;
>>>              perf->ops.is_valid_mux_reg =
>>> @@ -4348,8 +4344,6 @@ void i915_perf_init(struct drm_i915_private *i915)
>>>              }
>>>              perf->gen8_valid_ctx_bit = BIT(16);
>>>          } else if (IS_GEN(i915, 12)) {
>>> -            perf->oa_formats = gen12_oa_formats;
>>> -
>>>              perf->ops.is_valid_b_counter_reg =
>>>                  gen12_is_valid_b_counter_addr;
>>>              perf->ops.is_valid_mux_reg =
>>> diff --git a/drivers/gpu/drm/i915/i915_perf_types.h 
>>> b/drivers/gpu/drm/i915/i915_perf_types.h
>>> index a36a455ae336..84dceb743ebc 100644
>>> --- a/drivers/gpu/drm/i915/i915_perf_types.h
>>> +++ b/drivers/gpu/drm/i915/i915_perf_types.h
>>> @@ -19,6 +19,7 @@
>>>  #include "gt/intel_sseu.h"
>>>  #include "i915_reg.h"
>>>  #include "intel_wakeref.h"
>>> +#include "intel_device_info.h"
>>>  struct drm_i915_private;
>>>  struct file;
>>> @@ -32,6 +33,8 @@ struct intel_engine_cs;
>>>  struct i915_oa_format {
>>>      u32 format;
>>>      int size;
>>> +    enum intel_platform start;
>>> +    enum intel_platform end;
>>
>> As a drive by only - I question the concept of thinking about 
>> platforms enums as ordered and having things like INTEL_TIGERLAKE - 1 
>> in the code. It's completely novel concept and I think fragile. Order 
>> there is purely the order of upstreaming and not stable.
>>
>> Suggestions wise, if the rules are Gen based you could use a mask, but 
>> if they are truly platform based then it is more tricky. However 
>> current code does use gen ranges to assign them, so would a genmask 
>> really not fit?
> 
> Hmm, the rules are platform-based. A mask would be ideal, but the last I 
> thought about it, it felt more complex than this solution. This code 
> would have to maintain a mask of supported platforms (and subplatforms) 
> for each entry (preferably in the same format as runtime info -> 
> platform_mask).  Then we would just AND this mask with the 
> platform_mask/s to check for support. Let me know if that's what you 
> have in mind, then I can give it a try.

Gen mask would be trivial but platform mask a bit more work because it 
is an array of u32. Two are used at the moment with some low bits for 
subplatform. It might be doable with some macro magic but I haven't 
really thought about it. You can look at the IS_PLATFORM implementation 
and see if you can come up with something neat.

Regards,

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

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

* Re: [Intel-gfx] [PATCH 1/2] i915/perf: Move gen specific OA formats to single array
  2020-12-18  2:08   ` Umesh Nerlige Ramappa
@ 2020-12-18  9:47     ` Lionel Landwerlin
  0 siblings, 0 replies; 11+ messages in thread
From: Lionel Landwerlin @ 2020-12-18  9:47 UTC (permalink / raw)
  To: Umesh Nerlige Ramappa; +Cc: intel-gfx

On 18/12/2020 04:08, Umesh Nerlige Ramappa wrote:
> On Wed, Dec 16, 2020 at 10:30:24AM +0200, Lionel Landwerlin wrote:
>> On 15/12/2020 23:49, Umesh Nerlige Ramappa wrote:
>>> Variations in OA formats in the different gens has led to creation of
>>> several sparse arrays to store the formats.
>>>
>>> Move oa formats into a single array and add the supported range of
>>> platforms for the oa formats.
>>>
>>> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/i915_perf.c       | 56 ++++++++++++--------------
>>>  drivers/gpu/drm/i915/i915_perf_types.h |  3 ++
>>>  2 files changed, 28 insertions(+), 31 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_perf.c 
>>> b/drivers/gpu/drm/i915/i915_perf.c
>>> index f553caf4b06d..afa92cf075c4 100644
>>> --- a/drivers/gpu/drm/i915/i915_perf.c
>>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>>> @@ -298,28 +298,27 @@ static u32 i915_oa_max_sample_rate = 100000;
>>>  /* XXX: beware if future OA HW adds new report formats that the 
>>> current
>>>   * code assumes all reports have a power-of-two size and ~(size - 
>>> 1) can
>>> - * be used as a mask to align the OA tail pointer.
>>> + * be used as a mask to align the OA tail pointer. Note that the 
>>> platforms
>>> + * in this array specify a range (inclusive of start and end).
>>>   */
>>> -static const struct i915_oa_format 
>>> hsw_oa_formats[I915_OA_FORMAT_MAX] = {
>>> -    [I915_OA_FORMAT_A13]        = { 0, 64 },
>>> -    [I915_OA_FORMAT_A29]        = { 1, 128 },
>>> -    [I915_OA_FORMAT_A13_B8_C8]  = { 2, 128 },
>>> -    /* A29_B8_C8 Disallowed as 192 bytes doesn't factor into buffer 
>>> size */
>>> -    [I915_OA_FORMAT_B4_C8]        = { 4, 64 },
>>> -    [I915_OA_FORMAT_A45_B8_C8]  = { 5, 256 },
>>> -    [I915_OA_FORMAT_B4_C8_A16]  = { 6, 128 },
>>> -    [I915_OA_FORMAT_C4_B8]        = { 7, 64 },
>>> -};
>>> -
>>> -static const struct i915_oa_format 
>>> gen8_plus_oa_formats[I915_OA_FORMAT_MAX] = {
>>> -    [I915_OA_FORMAT_A12]            = { 0, 64 },
>>> -    [I915_OA_FORMAT_A12_B8_C8]        = { 2, 128 },
>>> -    [I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
>>> -    [I915_OA_FORMAT_C4_B8]            = { 7, 64 },
>>> -};
>>> -
>>> -static const struct i915_oa_format 
>>> gen12_oa_formats[I915_OA_FORMAT_MAX] = {
>>> -    [I915_OA_FORMAT_A32u40_A4u32_B8_C8] = { 5, 256 },
>>> +static const struct i915_oa_format oa_formats[I915_OA_FORMAT_MAX] = {
>>> +    /* haswell */
>>> +    [I915_OA_FORMAT_A13]                    = { 0, 64, 
>>> INTEL_IVYBRIDGE, INTEL_HASWELL },
>>> +    [I915_OA_FORMAT_A29]                    = { 1, 128, 
>>> INTEL_IVYBRIDGE, INTEL_HASWELL },
>>> +    [I915_OA_FORMAT_A13_B8_C8]              = { 2, 128, 
>>> INTEL_IVYBRIDGE, INTEL_HASWELL },
>>> +    [I915_OA_FORMAT_B4_C8]                  = { 4, 64, 
>>> INTEL_IVYBRIDGE, INTEL_HASWELL },
>>> +    [I915_OA_FORMAT_A45_B8_C8]              = { 5, 256, 
>>> INTEL_IVYBRIDGE, INTEL_HASWELL },
>>> +    [I915_OA_FORMAT_B4_C8_A16]              = { 6, 128, 
>>> INTEL_IVYBRIDGE, INTEL_HASWELL },
>>> +
>>> +    /* haswell+ upto but not including tigerlake */
>>> +    [I915_OA_FORMAT_C4_B8]                  = { 7, 64, 
>>> INTEL_IVYBRIDGE, INTEL_TIGERLAKE - 1 },
>> I don't think we support IVB,.
>
> So the above formats are only HASWELL then?


Correct


>
>>> +
>>> +    /* gen8+ upto but not including tigerlake */
>>> +    [I915_OA_FORMAT_A12]                    = { 0, 64, 
>>> INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
>>> +    [I915_OA_FORMAT_A12_B8_C8]              = { 2, 128, 
>>> INTEL_BROADWELL, INTEL_TIGERLAKE - 1 },
>>> +
>>> +    /* gen8+ */
>>> +    [I915_OA_FORMAT_A32u40_A4u32_B8_C8]     = { 5, 256, 
>>> INTEL_BROADWELL, INTEL_MAX_PLATFORMS - 1 },
>>>  };
>>
>>
>> You also need to change i915_oa_stream_init() to use the global 
>> array. Looks like it will access a NULL pointer atm.
>>
>
> yikes, will do.


Ah... See my comment below.


>
>>
>>>  #define SAMPLE_OA_REPORT (1<<0)
>>> @@ -3575,6 +3574,7 @@ static int read_properties_unlocked(struct 
>>> i915_perf *perf,
>>>      for (i = 0; i < n_props; i++) {
>>>          u64 oa_period, oa_freq_hz;
>>>          u64 id, value;
>>> +        enum intel_platform p = INTEL_INFO(perf->i915)->platform;
>>>          ret = get_user(id, uprop);
>>>          if (ret)
>>> @@ -3611,8 +3611,9 @@ static int read_properties_unlocked(struct 
>>> i915_perf *perf,
>>>                        value);
>>>                  return -EINVAL;
>>>              }
>>> -            if (!perf->oa_formats[value].size) {
>>> -                DRM_DEBUG("Unsupported OA report format %llu\n",
>>> +            if (p < perf->oa_formats[value].start ||
>>> +                p > perf->oa_formats[value].end) {
>>> +                DRM_DEBUG("OA report format not supported %llu\n",
>>>                        value);
>>>                  return -EINVAL;
>>>              }
>>> @@ -4270,6 +4271,7 @@ void i915_perf_init(struct drm_i915_private 
>>> *i915)
>>>      /* XXX const struct i915_perf_ops! */
>>> +    perf->oa_formats = oa_formats;
>>>      if (IS_HASWELL(i915)) {
>>>          perf->ops.is_valid_b_counter_reg = 
>>> gen7_is_valid_b_counter_addr;
>>>          perf->ops.is_valid_mux_reg = hsw_is_valid_mux_addr;
>>> @@ -4280,8 +4282,6 @@ void i915_perf_init(struct drm_i915_private 
>>> *i915)
>>>          perf->ops.oa_disable = gen7_oa_disable;
>>>          perf->ops.read = gen7_oa_read;
>>>          perf->ops.oa_hw_tail_read = gen7_oa_hw_tail_read;
>>> -
>>> -        perf->oa_formats = hsw_oa_formats;
>>>      } else if (HAS_LOGICAL_RING_CONTEXTS(i915)) {
>>>          /* Note: that although we could theoretically also support the
>>>           * legacy ringbuffer mode on BDW (and earlier iterations of
>>> @@ -4292,8 +4292,6 @@ void i915_perf_init(struct drm_i915_private 
>>> *i915)
>>>          perf->ops.read = gen8_oa_read;
>>>          if (IS_GEN_RANGE(i915, 8, 9)) {
>>> -            perf->oa_formats = gen8_plus_oa_formats;
>>> -
>>>              perf->ops.is_valid_b_counter_reg =
>>>                  gen7_is_valid_b_counter_addr;
>>>              perf->ops.is_valid_mux_reg =
>>> @@ -4324,8 +4322,6 @@ void i915_perf_init(struct drm_i915_private 
>>> *i915)
>>>                  perf->gen8_valid_ctx_bit = BIT(16);
>>>              }
>>>          } else if (IS_GEN_RANGE(i915, 10, 11)) {
>>> -            perf->oa_formats = gen8_plus_oa_formats;
>>> -
>>>              perf->ops.is_valid_b_counter_reg =
>>>                  gen7_is_valid_b_counter_addr;
>>>              perf->ops.is_valid_mux_reg =
>>> @@ -4348,8 +4344,6 @@ void i915_perf_init(struct drm_i915_private 
>>> *i915)
>>>              }
>>>              perf->gen8_valid_ctx_bit = BIT(16);
>>>          } else if (IS_GEN(i915, 12)) {
>>> -            perf->oa_formats = gen12_oa_formats;
>>> -
>>>              perf->ops.is_valid_b_counter_reg =
>>>                  gen12_is_valid_b_counter_addr;
>>>              perf->ops.is_valid_mux_reg =
>>> diff --git a/drivers/gpu/drm/i915/i915_perf_types.h 
>>> b/drivers/gpu/drm/i915/i915_perf_types.h
>>> index a36a455ae336..84dceb743ebc 100644
>>> --- a/drivers/gpu/drm/i915/i915_perf_types.h
>>> +++ b/drivers/gpu/drm/i915/i915_perf_types.h
>>> @@ -19,6 +19,7 @@
>>>  #include "gt/intel_sseu.h"
>>>  #include "i915_reg.h"
>>>  #include "intel_wakeref.h"
>>> +#include "intel_device_info.h"
>>>  struct drm_i915_private;
>>>  struct file;
>>> @@ -32,6 +33,8 @@ struct intel_engine_cs;
>>>  struct i915_oa_format {
>>>      u32 format;
>>>      int size;
>>> +    enum intel_platform start;
>>> +    enum intel_platform end;
>>
>>
>> Also need to drop oa_formats from struct i915_perf
>
> oa_formats still remains in i915_perf. It's just that
>
> perf->oa_formats = oa_formats;
>
> for all platforms.
>
> Thanks,
> Umesh


Oops, sorry I missed that.


-Lionel


>>
>>
>>>  };
>>>  struct i915_oa_reg {
>>
>>

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

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

end of thread, other threads:[~2020-12-18  9:47 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-15 21:49 [Intel-gfx] [PATCH 1/2] i915/perf: Move gen specific OA formats to single array Umesh Nerlige Ramappa
2020-12-15 21:49 ` [Intel-gfx] [PATCH 2/2] i915/perf: Add removed OA formats back for TGL Umesh Nerlige Ramappa
2020-12-16  0:16 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/2] i915/perf: Move gen specific OA formats to single array Patchwork
2020-12-16  0:46 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-12-16  7:08 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2020-12-16  8:30 ` [Intel-gfx] [PATCH 1/2] " Lionel Landwerlin
2020-12-18  2:08   ` Umesh Nerlige Ramappa
2020-12-18  9:47     ` Lionel Landwerlin
2020-12-16  9:25 ` Tvrtko Ursulin
2020-12-18  2:32   ` Umesh Nerlige Ramappa
2020-12-18  9:20     ` Tvrtko Ursulin

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.