All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/7] drm/i915: Perf updates
@ 2017-11-10 19:08 Lionel Landwerlin
  2017-11-10 19:08 ` [PATCH 1/7] drm/i915/perf: complete whitelisting for OA programming on HSW Lionel Landwerlin
                   ` (8 more replies)
  0 siblings, 9 replies; 23+ messages in thread
From: Lionel Landwerlin @ 2017-11-10 19:08 UTC (permalink / raw)
  To: intel-gfx

Hi all,

This is a split of a previous series to enable perf on Cannonlake :

https://patchwork.freedesktop.org/series/32762/

The other bit of information needed for perf' userspace on Cannonlake
is to expose precise topology (rather than accumulated numbers). But
this is now in discussion in a different series.

Cheers,

Lionel Landwerlin (7):
  drm/i915/perf: complete whitelisting for OA programming on HSW
  drm/i915/perf: add support for Coffeelake GT3
  drm/i915/perf: refactor perf setup
  drm/i915: fix register naming
  drm/i915/perf: enable perf support on CNL
  drm/i915: expose command stream timestamp frequency to userspace
  drm/i915/perf: reuse timestamp frequency from device info

 drivers/gpu/drm/i915/Makefile            |   4 +-
 drivers/gpu/drm/i915/i915_debugfs.c      |   2 +
 drivers/gpu/drm/i915/i915_drv.c          |   7 +-
 drivers/gpu/drm/i915/i915_drv.h          |   5 +-
 drivers/gpu/drm/i915/i915_oa_cflgt3.c    | 109 ++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_oa_cflgt3.h    |  34 +++++++++
 drivers/gpu/drm/i915/i915_oa_cnl.c       | 121 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_oa_cnl.h       |  34 +++++++++
 drivers/gpu/drm/i915/i915_perf.c         | 105 ++++++++++++++++-----------
 drivers/gpu/drm/i915/i915_reg.h          |  46 +++++++++++-
 drivers/gpu/drm/i915/intel_device_info.c | 107 +++++++++++++++++++++++++++
 include/uapi/drm/i915_drm.h              |   6 ++
 12 files changed, 530 insertions(+), 50 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_oa_cflgt3.c
 create mode 100644 drivers/gpu/drm/i915/i915_oa_cflgt3.h
 create mode 100644 drivers/gpu/drm/i915/i915_oa_cnl.c
 create mode 100644 drivers/gpu/drm/i915/i915_oa_cnl.h

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

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

* [PATCH 1/7] drm/i915/perf: complete whitelisting for OA programming on HSW
  2017-11-10 19:08 [PATCH 0/7] drm/i915: Perf updates Lionel Landwerlin
@ 2017-11-10 19:08 ` Lionel Landwerlin
  2017-11-13  7:15   ` Ewelina Musial
  2017-11-13 11:40   ` Chris Wilson
  2017-11-10 19:08 ` [PATCH 2/7] drm/i915/perf: add support for Coffeelake GT3 Lionel Landwerlin
                   ` (7 subsequent siblings)
  8 siblings, 2 replies; 23+ messages in thread
From: Lionel Landwerlin @ 2017-11-10 19:08 UTC (permalink / raw)
  To: intel-gfx

We were missing some registers and also can name one for which we only had
the offset.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c |  3 ++-
 drivers/gpu/drm/i915/i915_reg.h  | 14 ++++++++++++++
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 59ee808f8fd9..45aef15b9e7c 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -3023,7 +3023,8 @@ static bool hsw_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 addr)
 {
 	return gen7_is_valid_mux_addr(dev_priv, addr) ||
 		(addr >= 0x25100 && addr <= 0x2FF90) ||
-		addr == 0x9ec0;
+		(addr >= HSW_MBVID2_NOA0.reg && addr <= HSW_MBVID2_NOA9.reg) ||
+		addr == HSW_MBVID2_MISR0.reg;
 }
 
 static bool chv_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 addr)
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 6ef33422f762..8f3cf50f04b4 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1120,6 +1120,20 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 /* RPC unit config (Gen8+) */
 #define RPM_CONFIG	    _MMIO(0x0D08)
 
+/* NOA (HSW) */
+#define HSW_MBVID2_NOA0		_MMIO(0x9E80)
+#define HSW_MBVID2_NOA1		_MMIO(0x9E84)
+#define HSW_MBVID2_NOA2		_MMIO(0x9E88)
+#define HSW_MBVID2_NOA3		_MMIO(0x9E8C)
+#define HSW_MBVID2_NOA4		_MMIO(0x9E90)
+#define HSW_MBVID2_NOA5		_MMIO(0x9E94)
+#define HSW_MBVID2_NOA6		_MMIO(0x9E98)
+#define HSW_MBVID2_NOA7		_MMIO(0x9E9C)
+#define HSW_MBVID2_NOA8		_MMIO(0x9EA0)
+#define HSW_MBVID2_NOA9		_MMIO(0x9EA4)
+
+#define HSW_MBVID2_MISR0	_MMIO(0x9EC0)
+
 /* NOA (Gen8+) */
 #define NOA_CONFIG(i)	    _MMIO(0x0D0C + (i) * 4)
 
-- 
2.15.0

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

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

* [PATCH 2/7] drm/i915/perf: add support for Coffeelake GT3
  2017-11-10 19:08 [PATCH 0/7] drm/i915: Perf updates Lionel Landwerlin
  2017-11-10 19:08 ` [PATCH 1/7] drm/i915/perf: complete whitelisting for OA programming on HSW Lionel Landwerlin
@ 2017-11-10 19:08 ` Lionel Landwerlin
  2017-11-13  7:18   ` Ewelina Musial
  2017-11-10 19:08 ` [PATCH 3/7] drm/i915/perf: refactor perf setup Lionel Landwerlin
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 23+ messages in thread
From: Lionel Landwerlin @ 2017-11-10 19:08 UTC (permalink / raw)
  To: intel-gfx

We can enable GT3 as well as GT2.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/i915/Makefile         |   3 +-
 drivers/gpu/drm/i915/i915_drv.h       |   2 +
 drivers/gpu/drm/i915/i915_oa_cflgt3.c | 109 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_oa_cflgt3.h |  34 +++++++++++
 drivers/gpu/drm/i915/i915_perf.c      |   3 +
 5 files changed, 150 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/i915/i915_oa_cflgt3.c
 create mode 100644 drivers/gpu/drm/i915/i915_oa_cflgt3.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 177404462386..58463bd94b4c 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -163,7 +163,8 @@ i915-y += i915_perf.o \
 	  i915_oa_kblgt2.o \
 	  i915_oa_kblgt3.o \
 	  i915_oa_glk.o \
-	  i915_oa_cflgt2.o
+	  i915_oa_cflgt2.o \
+	  i915_oa_cflgt3.o
 
 ifeq ($(CONFIG_DRM_I915_GVT),y)
 i915-y += intel_gvt.o
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 40012b6daea2..9f26c34e0e52 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3058,6 +3058,8 @@ intel_info(const struct drm_i915_private *dev_priv)
 				 (INTEL_DEVID(dev_priv) & 0x00F0) == 0x00A0)
 #define IS_CFL_GT2(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
 				 (dev_priv)->info.gt == 2)
+#define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
+				 (dev_priv)->info.gt == 3)
 
 #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
 
diff --git a/drivers/gpu/drm/i915/i915_oa_cflgt3.c b/drivers/gpu/drm/i915/i915_oa_cflgt3.c
new file mode 100644
index 000000000000..42ff06fe54a3
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_oa_cflgt3.c
@@ -0,0 +1,109 @@
+/*
+ * Autogenerated file by GPU Top : https://github.com/rib/gputop
+ * DO NOT EDIT manually!
+ *
+ *
+ * Copyright (c) 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include <linux/sysfs.h>
+
+#include "i915_drv.h"
+#include "i915_oa_cflgt3.h"
+
+static const struct i915_oa_reg b_counter_config_test_oa[] = {
+	{ _MMIO(0x2740), 0x00000000 },
+	{ _MMIO(0x2744), 0x00800000 },
+	{ _MMIO(0x2714), 0xf0800000 },
+	{ _MMIO(0x2710), 0x00000000 },
+	{ _MMIO(0x2724), 0xf0800000 },
+	{ _MMIO(0x2720), 0x00000000 },
+	{ _MMIO(0x2770), 0x00000004 },
+	{ _MMIO(0x2774), 0x00000000 },
+	{ _MMIO(0x2778), 0x00000003 },
+	{ _MMIO(0x277c), 0x00000000 },
+	{ _MMIO(0x2780), 0x00000007 },
+	{ _MMIO(0x2784), 0x00000000 },
+	{ _MMIO(0x2788), 0x00100002 },
+	{ _MMIO(0x278c), 0x0000fff7 },
+	{ _MMIO(0x2790), 0x00100002 },
+	{ _MMIO(0x2794), 0x0000ffcf },
+	{ _MMIO(0x2798), 0x00100082 },
+	{ _MMIO(0x279c), 0x0000ffef },
+	{ _MMIO(0x27a0), 0x001000c2 },
+	{ _MMIO(0x27a4), 0x0000ffe7 },
+	{ _MMIO(0x27a8), 0x00100001 },
+	{ _MMIO(0x27ac), 0x0000ffe7 },
+};
+
+static const struct i915_oa_reg flex_eu_config_test_oa[] = {
+};
+
+static const struct i915_oa_reg mux_config_test_oa[] = {
+	{ _MMIO(0x9840), 0x00000080 },
+	{ _MMIO(0x9888), 0x11810000 },
+	{ _MMIO(0x9888), 0x07810013 },
+	{ _MMIO(0x9888), 0x1f810000 },
+	{ _MMIO(0x9888), 0x1d810000 },
+	{ _MMIO(0x9888), 0x1b930040 },
+	{ _MMIO(0x9888), 0x07e54000 },
+	{ _MMIO(0x9888), 0x1f908000 },
+	{ _MMIO(0x9888), 0x11900000 },
+	{ _MMIO(0x9888), 0x37900000 },
+	{ _MMIO(0x9888), 0x53900000 },
+	{ _MMIO(0x9888), 0x45900000 },
+	{ _MMIO(0x9888), 0x33900000 },
+};
+
+static ssize_t
+show_test_oa_id(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "1\n");
+}
+
+void
+i915_perf_load_test_config_cflgt3(struct drm_i915_private *dev_priv)
+{
+	strncpy(dev_priv->perf.oa.test_config.uuid,
+		"577e8e2c-3fa0-4875-8743-3538d585e3b0",
+		UUID_STRING_LEN);
+	dev_priv->perf.oa.test_config.id = 1;
+
+	dev_priv->perf.oa.test_config.mux_regs = mux_config_test_oa;
+	dev_priv->perf.oa.test_config.mux_regs_len = ARRAY_SIZE(mux_config_test_oa);
+
+	dev_priv->perf.oa.test_config.b_counter_regs = b_counter_config_test_oa;
+	dev_priv->perf.oa.test_config.b_counter_regs_len = ARRAY_SIZE(b_counter_config_test_oa);
+
+	dev_priv->perf.oa.test_config.flex_regs = flex_eu_config_test_oa;
+	dev_priv->perf.oa.test_config.flex_regs_len = ARRAY_SIZE(flex_eu_config_test_oa);
+
+	dev_priv->perf.oa.test_config.sysfs_metric.name = "577e8e2c-3fa0-4875-8743-3538d585e3b0";
+	dev_priv->perf.oa.test_config.sysfs_metric.attrs = dev_priv->perf.oa.test_config.attrs;
+
+	dev_priv->perf.oa.test_config.attrs[0] = &dev_priv->perf.oa.test_config.sysfs_metric_id.attr;
+
+	dev_priv->perf.oa.test_config.sysfs_metric_id.attr.name = "id";
+	dev_priv->perf.oa.test_config.sysfs_metric_id.attr.mode = 0444;
+	dev_priv->perf.oa.test_config.sysfs_metric_id.show = show_test_oa_id;
+}
diff --git a/drivers/gpu/drm/i915/i915_oa_cflgt3.h b/drivers/gpu/drm/i915/i915_oa_cflgt3.h
new file mode 100644
index 000000000000..c13b5aac01b9
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_oa_cflgt3.h
@@ -0,0 +1,34 @@
+/*
+ * Autogenerated file by GPU Top : https://github.com/rib/gputop
+ * DO NOT EDIT manually!
+ *
+ *
+ * Copyright (c) 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __I915_OA_CFLGT3_H__
+#define __I915_OA_CFLGT3_H__
+
+extern void i915_perf_load_test_config_cflgt3(struct drm_i915_private *dev_priv);
+
+#endif
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 45aef15b9e7c..7271debe0417 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -207,6 +207,7 @@
 #include "i915_oa_kblgt3.h"
 #include "i915_oa_glk.h"
 #include "i915_oa_cflgt2.h"
+#include "i915_oa_cflgt3.h"
 
 /* HW requires this to be a power of two, between 128k and 16M, though driver
  * is currently generally designed assuming the largest 16M size is used such
@@ -2934,6 +2935,8 @@ void i915_perf_register(struct drm_i915_private *dev_priv)
 	} else if (IS_COFFEELAKE(dev_priv)) {
 		if (IS_CFL_GT2(dev_priv))
 			i915_perf_load_test_config_cflgt2(dev_priv);
+		if (IS_CFL_GT3(dev_priv))
+			i915_perf_load_test_config_cflgt3(dev_priv);
 	}
 
 	if (dev_priv->perf.oa.test_config.id == 0)
-- 
2.15.0

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

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

* [PATCH 3/7] drm/i915/perf: refactor perf setup
  2017-11-10 19:08 [PATCH 0/7] drm/i915: Perf updates Lionel Landwerlin
  2017-11-10 19:08 ` [PATCH 1/7] drm/i915/perf: complete whitelisting for OA programming on HSW Lionel Landwerlin
  2017-11-10 19:08 ` [PATCH 2/7] drm/i915/perf: add support for Coffeelake GT3 Lionel Landwerlin
@ 2017-11-10 19:08 ` Lionel Landwerlin
  2017-11-10 19:08 ` [PATCH 4/7] drm/i915: fix register naming Lionel Landwerlin
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 23+ messages in thread
From: Lionel Landwerlin @ 2017-11-10 19:08 UTC (permalink / raw)
  To: intel-gfx

Gen8/9 aren't very different and we can merge some of this code.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c | 48 +++++++++++++++++++++-------------------
 1 file changed, 25 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 7271debe0417..802928c54f06 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -3423,41 +3423,46 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
 		 * worth the complexity to maintain now that BDW+ enable
 		 * execlist mode by default.
 		 */
-		dev_priv->perf.oa.ops.is_valid_b_counter_reg =
-			gen7_is_valid_b_counter_addr;
-		dev_priv->perf.oa.ops.is_valid_mux_reg =
-			gen8_is_valid_mux_addr;
-		dev_priv->perf.oa.ops.is_valid_flex_reg =
-			gen8_is_valid_flex_addr;
+		dev_priv->perf.oa.oa_formats = gen8_plus_oa_formats;
 
 		dev_priv->perf.oa.ops.init_oa_buffer = gen8_init_oa_buffer;
-		dev_priv->perf.oa.ops.enable_metric_set = gen8_enable_metric_set;
-		dev_priv->perf.oa.ops.disable_metric_set = gen8_disable_metric_set;
 		dev_priv->perf.oa.ops.oa_enable = gen8_oa_enable;
 		dev_priv->perf.oa.ops.oa_disable = gen8_oa_disable;
 		dev_priv->perf.oa.ops.read = gen8_oa_read;
 		dev_priv->perf.oa.ops.oa_hw_tail_read = gen8_oa_hw_tail_read;
 
-		dev_priv->perf.oa.oa_formats = gen8_plus_oa_formats;
-
-		if (IS_GEN8(dev_priv)) {
-			dev_priv->perf.oa.ctx_oactxctrl_offset = 0x120;
-			dev_priv->perf.oa.ctx_flexeu0_offset = 0x2ce;
+		if (IS_GEN8(dev_priv) || IS_GEN9(dev_priv)) {
+			dev_priv->perf.oa.ops.is_valid_b_counter_reg =
+				gen7_is_valid_b_counter_addr;
+			dev_priv->perf.oa.ops.is_valid_mux_reg =
+				gen8_is_valid_mux_addr;
+			dev_priv->perf.oa.ops.is_valid_flex_reg =
+				gen8_is_valid_flex_addr;
 
-			dev_priv->perf.oa.timestamp_frequency = 12500000;
-
-			dev_priv->perf.oa.gen8_valid_ctx_bit = (1<<25);
 			if (IS_CHERRYVIEW(dev_priv)) {
 				dev_priv->perf.oa.ops.is_valid_mux_reg =
 					chv_is_valid_mux_addr;
 			}
-		} else if (IS_GEN9(dev_priv)) {
-			dev_priv->perf.oa.ctx_oactxctrl_offset = 0x128;
-			dev_priv->perf.oa.ctx_flexeu0_offset = 0x3de;
 
-			dev_priv->perf.oa.gen8_valid_ctx_bit = (1<<16);
+			dev_priv->perf.oa.ops.enable_metric_set = gen8_enable_metric_set;
+			dev_priv->perf.oa.ops.disable_metric_set = gen8_disable_metric_set;
+
+			if (IS_GEN8(dev_priv)) {
+				dev_priv->perf.oa.ctx_oactxctrl_offset = 0x120;
+				dev_priv->perf.oa.ctx_flexeu0_offset = 0x2ce;
+
+				dev_priv->perf.oa.gen8_valid_ctx_bit = (1<<25);
+			} else {
+				dev_priv->perf.oa.ctx_oactxctrl_offset = 0x128;
+				dev_priv->perf.oa.ctx_flexeu0_offset = 0x3de;
+
+				dev_priv->perf.oa.gen8_valid_ctx_bit = (1<<16);
+			}
 
 			switch (dev_priv->info.platform) {
+			case INTEL_BROADWELL:
+				dev_priv->perf.oa.timestamp_frequency = 12500000;
+				break;
 			case INTEL_BROXTON:
 			case INTEL_GEMINILAKE:
 				dev_priv->perf.oa.timestamp_frequency = 19200000;
@@ -3468,9 +3473,6 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
 				dev_priv->perf.oa.timestamp_frequency = 12000000;
 				break;
 			default:
-				/* Leave timestamp_frequency to 0 so we can
-				 * detect unsupported platforms.
-				 */
 				break;
 			}
 		}
-- 
2.15.0

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

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

* [PATCH 4/7] drm/i915: fix register naming
  2017-11-10 19:08 [PATCH 0/7] drm/i915: Perf updates Lionel Landwerlin
                   ` (2 preceding siblings ...)
  2017-11-10 19:08 ` [PATCH 3/7] drm/i915/perf: refactor perf setup Lionel Landwerlin
@ 2017-11-10 19:08 ` Lionel Landwerlin
  2017-11-10 19:44   ` Matthew Auld
  2017-11-13  7:28   ` Ewelina Musial
  2017-11-10 19:08 ` [PATCH 5/7] drm/i915/perf: enable perf support on CNL Lionel Landwerlin
                   ` (4 subsequent siblings)
  8 siblings, 2 replies; 23+ messages in thread
From: Lionel Landwerlin @ 2017-11-10 19:08 UTC (permalink / raw)
  To: intel-gfx

This name was added with the whitelisting of registers for building up OA
configs. It is contained in a range gen8 whitelist :

   addr >= RPM_CONFIG0.reg && addr <= NOA_CONFIG(8).reg

Hence why the name isn't used anywhere.

v2: Fix register name again RPC->RCP (Matthew)

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 8f3cf50f04b4..f812224d1fc8 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1117,8 +1117,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define RPM_CONFIG0	    _MMIO(0x0D00)
 #define RPM_CONFIG1	    _MMIO(0x0D04)
 
-/* RPC unit config (Gen8+) */
-#define RPM_CONFIG	    _MMIO(0x0D08)
+/* RCP unit config (Gen8+) */
+#define RCP_CONFIG	    _MMIO(0x0D08)
 
 /* NOA (HSW) */
 #define HSW_MBVID2_NOA0		_MMIO(0x9E80)
-- 
2.15.0

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

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

* [PATCH 5/7] drm/i915/perf: enable perf support on CNL
  2017-11-10 19:08 [PATCH 0/7] drm/i915: Perf updates Lionel Landwerlin
                   ` (3 preceding siblings ...)
  2017-11-10 19:08 ` [PATCH 4/7] drm/i915: fix register naming Lionel Landwerlin
@ 2017-11-10 19:08 ` Lionel Landwerlin
  2017-11-10 19:08 ` [PATCH 6/7] drm/i915: expose command stream timestamp frequency to userspace Lionel Landwerlin
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 23+ messages in thread
From: Lionel Landwerlin @ 2017-11-10 19:08 UTC (permalink / raw)
  To: intel-gfx

This adds new registers to the whitelist to configs emitted from userspace.

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
---
 drivers/gpu/drm/i915/Makefile      |   3 +-
 drivers/gpu/drm/i915/i915_oa_cnl.c | 121 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_oa_cnl.h |  34 +++++++++++
 drivers/gpu/drm/i915/i915_perf.c   |  41 ++++++++++++-
 drivers/gpu/drm/i915/i915_reg.h    |   5 ++
 5 files changed, 202 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_oa_cnl.c
 create mode 100644 drivers/gpu/drm/i915/i915_oa_cnl.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 58463bd94b4c..4f2c6e302100 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -164,7 +164,8 @@ i915-y += i915_perf.o \
 	  i915_oa_kblgt3.o \
 	  i915_oa_glk.o \
 	  i915_oa_cflgt2.o \
-	  i915_oa_cflgt3.o
+	  i915_oa_cflgt3.o \
+	  i915_oa_cnl.o
 
 ifeq ($(CONFIG_DRM_I915_GVT),y)
 i915-y += intel_gvt.o
diff --git a/drivers/gpu/drm/i915/i915_oa_cnl.c b/drivers/gpu/drm/i915/i915_oa_cnl.c
new file mode 100644
index 000000000000..ff0ac3627cc4
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_oa_cnl.c
@@ -0,0 +1,121 @@
+/*
+ * Autogenerated file by GPU Top : https://github.com/rib/gputop
+ * DO NOT EDIT manually!
+ *
+ *
+ * Copyright (c) 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include <linux/sysfs.h>
+
+#include "i915_drv.h"
+#include "i915_oa_cnl.h"
+
+static const struct i915_oa_reg b_counter_config_test_oa[] = {
+	{ _MMIO(0x2740), 0x00000000 },
+	{ _MMIO(0x2710), 0x00000000 },
+	{ _MMIO(0x2714), 0xf0800000 },
+	{ _MMIO(0x2720), 0x00000000 },
+	{ _MMIO(0x2724), 0xf0800000 },
+	{ _MMIO(0x2770), 0x00000004 },
+	{ _MMIO(0x2774), 0x0000ffff },
+	{ _MMIO(0x2778), 0x00000003 },
+	{ _MMIO(0x277c), 0x0000ffff },
+	{ _MMIO(0x2780), 0x00000007 },
+	{ _MMIO(0x2784), 0x0000ffff },
+	{ _MMIO(0x2788), 0x00100002 },
+	{ _MMIO(0x278c), 0x0000fff7 },
+	{ _MMIO(0x2790), 0x00100002 },
+	{ _MMIO(0x2794), 0x0000ffcf },
+	{ _MMIO(0x2798), 0x00100082 },
+	{ _MMIO(0x279c), 0x0000ffef },
+	{ _MMIO(0x27a0), 0x001000c2 },
+	{ _MMIO(0x27a4), 0x0000ffe7 },
+	{ _MMIO(0x27a8), 0x00100001 },
+	{ _MMIO(0x27ac), 0x0000ffe7 },
+};
+
+static const struct i915_oa_reg flex_eu_config_test_oa[] = {
+};
+
+static const struct i915_oa_reg mux_config_test_oa[] = {
+	{ _MMIO(0xd04), 0x00000200 },
+	{ _MMIO(0x9884), 0x00000007 },
+	{ _MMIO(0x9888), 0x17060000 },
+	{ _MMIO(0x9840), 0x00000000 },
+	{ _MMIO(0x9884), 0x00000007 },
+	{ _MMIO(0x9888), 0x13034000 },
+	{ _MMIO(0x9884), 0x00000007 },
+	{ _MMIO(0x9888), 0x07060066 },
+	{ _MMIO(0x9884), 0x00000007 },
+	{ _MMIO(0x9888), 0x05060000 },
+	{ _MMIO(0x9884), 0x00000007 },
+	{ _MMIO(0x9888), 0x0f080040 },
+	{ _MMIO(0x9884), 0x00000007 },
+	{ _MMIO(0x9888), 0x07091000 },
+	{ _MMIO(0x9884), 0x00000007 },
+	{ _MMIO(0x9888), 0x0f041000 },
+	{ _MMIO(0x9884), 0x00000007 },
+	{ _MMIO(0x9888), 0x1d004000 },
+	{ _MMIO(0x9884), 0x00000007 },
+	{ _MMIO(0x9888), 0x35000000 },
+	{ _MMIO(0x9884), 0x00000007 },
+	{ _MMIO(0x9888), 0x49000000 },
+	{ _MMIO(0x9884), 0x00000007 },
+	{ _MMIO(0x9888), 0x3d000000 },
+	{ _MMIO(0x9884), 0x00000007 },
+	{ _MMIO(0x9888), 0x31000000 },
+};
+
+static ssize_t
+show_test_oa_id(struct device *kdev, struct device_attribute *attr, char *buf)
+{
+	return sprintf(buf, "1\n");
+}
+
+void
+i915_perf_load_test_config_cnl(struct drm_i915_private *dev_priv)
+{
+	strncpy(dev_priv->perf.oa.test_config.uuid,
+		"db41edd4-d8e7-4730-ad11-b9a2d6833503",
+		UUID_STRING_LEN);
+	dev_priv->perf.oa.test_config.id = 1;
+
+	dev_priv->perf.oa.test_config.mux_regs = mux_config_test_oa;
+	dev_priv->perf.oa.test_config.mux_regs_len = ARRAY_SIZE(mux_config_test_oa);
+
+	dev_priv->perf.oa.test_config.b_counter_regs = b_counter_config_test_oa;
+	dev_priv->perf.oa.test_config.b_counter_regs_len = ARRAY_SIZE(b_counter_config_test_oa);
+
+	dev_priv->perf.oa.test_config.flex_regs = flex_eu_config_test_oa;
+	dev_priv->perf.oa.test_config.flex_regs_len = ARRAY_SIZE(flex_eu_config_test_oa);
+
+	dev_priv->perf.oa.test_config.sysfs_metric.name = "db41edd4-d8e7-4730-ad11-b9a2d6833503";
+	dev_priv->perf.oa.test_config.sysfs_metric.attrs = dev_priv->perf.oa.test_config.attrs;
+
+	dev_priv->perf.oa.test_config.attrs[0] = &dev_priv->perf.oa.test_config.sysfs_metric_id.attr;
+
+	dev_priv->perf.oa.test_config.sysfs_metric_id.attr.name = "id";
+	dev_priv->perf.oa.test_config.sysfs_metric_id.attr.mode = 0444;
+	dev_priv->perf.oa.test_config.sysfs_metric_id.show = show_test_oa_id;
+}
diff --git a/drivers/gpu/drm/i915/i915_oa_cnl.h b/drivers/gpu/drm/i915/i915_oa_cnl.h
new file mode 100644
index 000000000000..fb918b131105
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_oa_cnl.h
@@ -0,0 +1,34 @@
+/*
+ * Autogenerated file by GPU Top : https://github.com/rib/gputop
+ * DO NOT EDIT manually!
+ *
+ *
+ * Copyright (c) 2015 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#ifndef __I915_OA_CNL_H__
+#define __I915_OA_CNL_H__
+
+extern void i915_perf_load_test_config_cnl(struct drm_i915_private *dev_priv);
+
+#endif
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 802928c54f06..00be015e01df 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -208,6 +208,7 @@
 #include "i915_oa_glk.h"
 #include "i915_oa_cflgt2.h"
 #include "i915_oa_cflgt3.h"
+#include "i915_oa_cnl.h"
 
 /* HW requires this to be a power of two, between 128k and 16M, though driver
  * is currently generally designed assuming the largest 16M size is used such
@@ -1852,7 +1853,7 @@ static int gen8_enable_metric_set(struct drm_i915_private *dev_priv,
 	 * be read back from automatically triggered reports, as part of the
 	 * RPT_ID field.
 	 */
-	if (IS_GEN9(dev_priv)) {
+	if (IS_GEN9(dev_priv) || IS_GEN10(dev_priv)) {
 		I915_WRITE(GEN8_OA_DEBUG,
 			   _MASKED_BIT_ENABLE(GEN9_OA_DEBUG_DISABLE_CLK_RATIO_REPORTS |
 					      GEN9_OA_DEBUG_INCLUDE_CLK_RATIO));
@@ -1885,6 +1886,16 @@ static void gen8_disable_metric_set(struct drm_i915_private *dev_priv)
 
 }
 
+static void gen10_disable_metric_set(struct drm_i915_private *dev_priv)
+{
+	/* Reset all contexts' slices/subslices configurations. */
+	gen8_configure_all_contexts(dev_priv, NULL, false);
+
+	/* Make sure we disable noa to save power. */
+	I915_WRITE(RPM_CONFIG1,
+		   I915_READ(RPM_CONFIG1) & ~GEN10_GT_NOA_ENABLE);
+}
+
 static void gen7_oa_enable(struct drm_i915_private *dev_priv)
 {
 	/*
@@ -2937,6 +2948,8 @@ void i915_perf_register(struct drm_i915_private *dev_priv)
 			i915_perf_load_test_config_cflgt2(dev_priv);
 		if (IS_CFL_GT3(dev_priv))
 			i915_perf_load_test_config_cflgt3(dev_priv);
+	} else if (IS_CANNONLAKE(dev_priv)) {
+		i915_perf_load_test_config_cnl(dev_priv);
 	}
 
 	if (dev_priv->perf.oa.test_config.id == 0)
@@ -3022,6 +3035,12 @@ static bool gen8_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 addr)
 		(addr >= RPM_CONFIG0.reg && addr <= NOA_CONFIG(8).reg);
 }
 
+static bool gen10_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 addr)
+{
+	return gen8_is_valid_mux_addr(dev_priv, addr) ||
+		(addr >= OA_PERFCNT3_LO.reg && addr <= OA_PERFCNT4_HI.reg);
+}
+
 static bool hsw_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 addr)
 {
 	return gen7_is_valid_mux_addr(dev_priv, addr) ||
@@ -3475,6 +3494,26 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
 			default:
 				break;
 			}
+		} else if (IS_GEN10(dev_priv)) {
+			dev_priv->perf.oa.ops.is_valid_b_counter_reg =
+				gen7_is_valid_b_counter_addr;
+			dev_priv->perf.oa.ops.is_valid_mux_reg =
+				gen10_is_valid_mux_addr;
+			dev_priv->perf.oa.ops.is_valid_flex_reg =
+				gen8_is_valid_flex_addr;
+
+			dev_priv->perf.oa.ops.enable_metric_set = gen8_enable_metric_set;
+			dev_priv->perf.oa.ops.disable_metric_set = gen10_disable_metric_set;
+
+			dev_priv->perf.oa.ctx_oactxctrl_offset = 0x128;
+			dev_priv->perf.oa.ctx_flexeu0_offset = 0x3de;
+
+			dev_priv->perf.oa.gen8_valid_ctx_bit = (1<<16);
+
+			/* Default frequency, although we need to read it from
+			 * the register as it might vary between parts.
+			 */
+			dev_priv->perf.oa.timestamp_frequency = 12000000;
 		}
 	}
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index f812224d1fc8..7aa7dc0c336b 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1109,6 +1109,10 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 #define OA_PERFCNT1_HI      _MMIO(0x91BC)
 #define OA_PERFCNT2_LO      _MMIO(0x91C0)
 #define OA_PERFCNT2_HI      _MMIO(0x91C4)
+#define OA_PERFCNT3_LO      _MMIO(0x91C8)
+#define OA_PERFCNT3_HI      _MMIO(0x91CC)
+#define OA_PERFCNT4_LO      _MMIO(0x91D8)
+#define OA_PERFCNT4_HI      _MMIO(0x91DC)
 
 #define OA_PERFMATRIX_LO    _MMIO(0x91C8)
 #define OA_PERFMATRIX_HI    _MMIO(0x91CC)
@@ -1116,6 +1120,7 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 /* RPM unit config (Gen8+) */
 #define RPM_CONFIG0	    _MMIO(0x0D00)
 #define RPM_CONFIG1	    _MMIO(0x0D04)
+#define  GEN10_GT_NOA_ENABLE  (1 << 9)
 
 /* RCP unit config (Gen8+) */
 #define RCP_CONFIG	    _MMIO(0x0D08)
-- 
2.15.0

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

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

* [PATCH 6/7] drm/i915: expose command stream timestamp frequency to userspace
  2017-11-10 19:08 [PATCH 0/7] drm/i915: Perf updates Lionel Landwerlin
                   ` (4 preceding siblings ...)
  2017-11-10 19:08 ` [PATCH 5/7] drm/i915/perf: enable perf support on CNL Lionel Landwerlin
@ 2017-11-10 19:08 ` Lionel Landwerlin
  2017-11-10 20:58   ` Chris Wilson
                     ` (2 more replies)
  2017-11-10 19:08 ` [PATCH 7/7] drm/i915/perf: reuse timestamp frequency from device info Lionel Landwerlin
                   ` (2 subsequent siblings)
  8 siblings, 3 replies; 23+ messages in thread
From: Lionel Landwerlin @ 2017-11-10 19:08 UTC (permalink / raw)
  To: intel-gfx

We use to have this fixed per generation, but starting with CNL userspace
cannot tell just off the PCI ID. Let's make this information available. This
is particularly useful for performance monitoring where much of the
normalization work is done using those timestamps (this include pipeline
statistics in both GL & Vulkan as well as OA reports).

v2: Use variables for 24MHz/19.2MHz values (Ewelina)
    Renamed function & coding style (Sagar)

v3: Fix frequency read on Broadwell (Sagar)
    Fix missing divide by 4 on <= gen4 (Sagar)

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Tested-by: Rafael Antognolli <rafael.antognolli@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c      |   2 +
 drivers/gpu/drm/i915/i915_drv.c          |   3 +
 drivers/gpu/drm/i915/i915_drv.h          |   2 +
 drivers/gpu/drm/i915/i915_reg.h          |  21 ++++++
 drivers/gpu/drm/i915/intel_device_info.c | 107 +++++++++++++++++++++++++++++++
 include/uapi/drm/i915_drm.h              |   6 ++
 6 files changed, 141 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 533ba096b9a6..57485f29d7c9 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3245,6 +3245,8 @@ static int i915_engine_info(struct seq_file *m, void *unused)
 		   yesno(dev_priv->gt.awake));
 	seq_printf(m, "Global active requests: %d\n",
 		   dev_priv->gt.active_requests);
+	seq_printf(m, "CS timestamp frequency: %llu Hz\n",
+		   dev_priv->info.cs_timestamp_frequency);
 
 	p = drm_seq_file_printer(m);
 	for_each_engine(engine, dev_priv, id)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d97fe9c9439a..dbd04d5f75ee 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -419,6 +419,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
 		if (!value)
 			return -ENODEV;
 		break;
+	case I915_PARAM_CS_TIMESTAMP_FREQUENCY:
+		value = INTEL_INFO(dev_priv)->cs_timestamp_frequency;
+		break;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
 		return -EINVAL;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9f26c34e0e52..ebc49ca58eb7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -885,6 +885,8 @@ struct intel_device_info {
 	/* Slice/subslice/EU info */
 	struct sseu_dev_info sseu;
 
+	u64 cs_timestamp_frequency;
+
 	struct color_luts {
 		u16 degamma_lut_size;
 		u16 gamma_lut_size;
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7aa7dc0c336b..ec9faa11e305 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -1119,9 +1119,24 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
 
 /* RPM unit config (Gen8+) */
 #define RPM_CONFIG0	    _MMIO(0x0D00)
+#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT	3
+#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_MASK	(1 << GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT)
+#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_19_2_MHZ	0
+#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_24_MHZ	1
+#define  GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_SHIFT	1
+#define  GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_MASK	(0x3 << GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_SHIFT)
+
 #define RPM_CONFIG1	    _MMIO(0x0D04)
 #define  GEN10_GT_NOA_ENABLE  (1 << 9)
 
+/* GPM unit config (Gen9+) */
+#define CTC_MODE			_MMIO(0xA26C)
+#define  CTC_SOURCE_PARAMETER_MASK 1
+#define  CTC_SOURCE_CRYSTAL_CLOCK	0
+#define  CTC_SOURCE_DIVIDE_LOGIC	1
+#define  CTC_SHIFT_PARAMETER_SHIFT	1
+#define  CTC_SHIFT_PARAMETER_MASK	(0x3 << CTC_SHIFT_PARAMETER_SHIFT)
+
 /* RCP unit config (Gen8+) */
 #define RCP_CONFIG	    _MMIO(0x0D08)
 
@@ -8866,6 +8881,12 @@ enum skl_power_gate {
 #define ILK_TIMESTAMP_HI	_MMIO(0x70070)
 #define IVB_TIMESTAMP_CTR	_MMIO(0x44070)
 
+#define GEN9_TIMESTAMP_OVERRIDE				_MMIO(0x44074)
+#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_SHIFT	0
+#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_MASK	0x3ff
+#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_SHIFT	12
+#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_MASK	(0xf << 12)
+
 #define _PIPE_FRMTMSTMP_A		0x70048
 #define PIPE_FRMTMSTMP(pipe)		\
 			_MMIO_PIPE2(pipe, _PIPE_FRMTMSTMP_A)
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index db03d179fc85..78bf7374fbdd 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -329,6 +329,108 @@ static void broadwell_sseu_info_init(struct drm_i915_private *dev_priv)
 	sseu->has_eu_pg = 0;
 }
 
+static u64 read_reference_ts_freq(struct drm_i915_private *dev_priv)
+{
+	u32 ts_override = I915_READ(GEN9_TIMESTAMP_OVERRIDE);
+	u64 base_freq, frac_freq;
+
+	base_freq = ((ts_override & GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_MASK) >>
+		     GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_SHIFT) + 1;
+	base_freq *= 1000000;
+
+	frac_freq = ((ts_override &
+		      GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_MASK) >>
+		     GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_SHIFT);
+	if (frac_freq != 0)
+		frac_freq = 1000000 / (frac_freq + 1);
+
+	return base_freq + frac_freq;
+}
+
+static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
+{
+	u64 f12_5_mhz = 12500000;
+	u64 f19_2_mhz = 19200000;
+	u64 f24_mhz = 24000000;
+
+	if (INTEL_GEN(dev_priv) <= 4) {
+		/* PRMs say:
+		 *
+		 *     "The value in this register increments once every 16
+		 *      hclks." (through the “Clocking Configuration”
+		 *      (“CLKCFG”) MCHBAR register)
+		 */
+		return (dev_priv->rawclk_freq * 1000) / 16;
+	} else if (INTEL_GEN(dev_priv) <= 8) {
+		/* PRMs say:
+		 *
+		 *     "The PCU TSC counts 10ns increments; this timestamp
+		 *      reflects bits 38:3 of the TSC (i.e. 80ns granularity,
+		 *      rolling over every 1.5 hours).
+		 */
+		return f12_5_mhz;
+	} else if (INTEL_GEN(dev_priv) <= 9) {
+		u32 ctc_reg = I915_READ(CTC_MODE);
+		u64 freq = 0;
+
+		if ((ctc_reg & CTC_SOURCE_PARAMETER_MASK) == CTC_SOURCE_DIVIDE_LOGIC) {
+			freq = read_reference_ts_freq(dev_priv);
+		} else {
+			freq = IS_GEN9_LP(dev_priv) ? f19_2_mhz : f24_mhz;
+
+			/* Now figure out how the command stream's timestamp
+			 * register increments from this frequency (it might
+			 * increment only every few clock cycle).
+			 */
+			freq >>= 3 - ((ctc_reg & CTC_SHIFT_PARAMETER_MASK) >>
+				      CTC_SHIFT_PARAMETER_SHIFT);
+		}
+
+		return freq;
+	} else if (INTEL_GEN(dev_priv) <= 10) {
+		u32 ctc_reg = I915_READ(CTC_MODE);
+		u64 freq = 0;
+		u32 rpm_config_reg = 0;
+
+		/* First figure out the reference frequency. There are 2 ways
+		 * we can compute the frequency, either through the
+		 * TIMESTAMP_OVERRIDE register or through RPM_CONFIG. CTC_MODE
+		 * tells us which one we should use.
+		 */
+		if ((ctc_reg & CTC_SOURCE_PARAMETER_MASK) == CTC_SOURCE_DIVIDE_LOGIC) {
+			freq = read_reference_ts_freq(dev_priv);
+		} else {
+			u32 crystal_clock;
+
+			rpm_config_reg = I915_READ(RPM_CONFIG0);
+			crystal_clock = (rpm_config_reg &
+					 GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_MASK) >>
+				GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT;
+			switch (crystal_clock) {
+			case GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_19_2_MHZ:
+				freq = f19_2_mhz;
+				break;
+			case GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_24_MHZ:
+				freq = f24_mhz;
+				break;
+			}
+		}
+
+		/* Now figure out how the command stream's timestamp register
+		 * increments from this frequency (it might increment only
+		 * every few clock cycle).
+		 */
+		freq >>= 3 - ((rpm_config_reg &
+			       GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_MASK) >>
+			      GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_SHIFT);
+
+		return freq;
+	}
+
+	DRM_ERROR("Unknown gen, unable to compute command stream timestamp frequency\n");
+	return 0;
+}
+
 /*
  * Determine various intel_device_info fields at runtime.
  *
@@ -450,6 +552,9 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
 	else if (INTEL_GEN(dev_priv) >= 10)
 		gen10_sseu_info_init(dev_priv);
 
+	/* Initialize command stream timestamp frequency */
+	info->cs_timestamp_frequency = read_timestamp_frequency(dev_priv);
+
 	DRM_DEBUG_DRIVER("slice mask: %04x\n", info->sseu.slice_mask);
 	DRM_DEBUG_DRIVER("slice total: %u\n", hweight8(info->sseu.slice_mask));
 	DRM_DEBUG_DRIVER("subslice total: %u\n",
@@ -465,4 +570,6 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
 			 info->sseu.has_subslice_pg ? "y" : "n");
 	DRM_DEBUG_DRIVER("has EU power gating: %s\n",
 			 info->sseu.has_eu_pg ? "y" : "n");
+	DRM_DEBUG_DRIVER("CS timestamp frequency: %llu\n",
+			 info->cs_timestamp_frequency);
 }
diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
index 6c02ced663f8..b57985929553 100644
--- a/include/uapi/drm/i915_drm.h
+++ b/include/uapi/drm/i915_drm.h
@@ -481,6 +481,12 @@ typedef struct drm_i915_irq_wait {
  */
 #define I915_PARAM_HAS_CONTEXT_ISOLATION 50
 
+/* Frequency of the command streamer timestamps given by the *_TIMESTAMP
+ * registers. This used to be fixed per platform but from CNL onwards, this
+ * might vary depending on the parts.
+ */
+#define I915_PARAM_CS_TIMESTAMP_FREQUENCY 51
+
 typedef struct drm_i915_getparam {
 	__s32 param;
 	/*
-- 
2.15.0

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

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

* [PATCH 7/7] drm/i915/perf: reuse timestamp frequency from device info
  2017-11-10 19:08 [PATCH 0/7] drm/i915: Perf updates Lionel Landwerlin
                   ` (5 preceding siblings ...)
  2017-11-10 19:08 ` [PATCH 6/7] drm/i915: expose command stream timestamp frequency to userspace Lionel Landwerlin
@ 2017-11-10 19:08 ` Lionel Landwerlin
  2017-11-10 21:00   ` Chris Wilson
  2017-11-10 19:57 ` ✓ Fi.CI.BAT: success for drm/i915: Perf updates Patchwork
  2017-11-10 21:35 ` ✓ Fi.CI.IGT: " Patchwork
  8 siblings, 1 reply; 23+ messages in thread
From: Lionel Landwerlin @ 2017-11-10 19:08 UTC (permalink / raw)
  To: intel-gfx

Now that we have this stored in the device info, we can drop it from perf
part of the driver.

Note that this requires to init perf after we've computed the frequency,
hence why we move i915_perf_init() from i915_driver_init_early() to after
intel_device_info_runtime_init().

Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c  |  4 ++--
 drivers/gpu/drm/i915/i915_drv.h  |  1 -
 drivers/gpu/drm/i915/i915_perf.c | 32 +++-----------------------------
 3 files changed, 5 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index dbd04d5f75ee..0aa1867da784 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -932,8 +932,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
 
 	intel_detect_preproduction_hw(dev_priv);
 
-	i915_perf_init(dev_priv);
-
 	return 0;
 
 err_irq:
@@ -1097,6 +1095,8 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 
 	intel_sanitize_options(dev_priv);
 
+	i915_perf_init(dev_priv);
+
 	ret = i915_ggtt_probe_hw(dev_priv);
 	if (ret)
 		return ret;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ebc49ca58eb7..5274e8261059 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2620,7 +2620,6 @@ struct drm_i915_private {
 
 			bool periodic;
 			int period_exponent;
-			int timestamp_frequency;
 
 			struct i915_oa_config test_config;
 
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 00be015e01df..1f9d86b5cad4 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -2692,7 +2692,7 @@ i915_perf_open_ioctl_locked(struct drm_i915_private *dev_priv,
 static u64 oa_exponent_to_ns(struct drm_i915_private *dev_priv, int exponent)
 {
 	return div_u64(1000000000ULL * (2ULL << exponent),
-		       dev_priv->perf.oa.timestamp_frequency);
+		       INTEL_INFO(dev_priv)->cs_timestamp_frequency);
 }
 
 /**
@@ -3415,8 +3415,6 @@ static struct ctl_table dev_root[] = {
  */
 void i915_perf_init(struct drm_i915_private *dev_priv)
 {
-	dev_priv->perf.oa.timestamp_frequency = 0;
-
 	if (IS_HASWELL(dev_priv)) {
 		dev_priv->perf.oa.ops.is_valid_b_counter_reg =
 			gen7_is_valid_b_counter_addr;
@@ -3432,8 +3430,6 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
 		dev_priv->perf.oa.ops.oa_hw_tail_read =
 			gen7_oa_hw_tail_read;
 
-		dev_priv->perf.oa.timestamp_frequency = 12500000;
-
 		dev_priv->perf.oa.oa_formats = hsw_oa_formats;
 	} else if (i915_modparams.enable_execlists) {
 		/* Note: that although we could theoretically also support the
@@ -3477,23 +3473,6 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
 
 				dev_priv->perf.oa.gen8_valid_ctx_bit = (1<<16);
 			}
-
-			switch (dev_priv->info.platform) {
-			case INTEL_BROADWELL:
-				dev_priv->perf.oa.timestamp_frequency = 12500000;
-				break;
-			case INTEL_BROXTON:
-			case INTEL_GEMINILAKE:
-				dev_priv->perf.oa.timestamp_frequency = 19200000;
-				break;
-			case INTEL_SKYLAKE:
-			case INTEL_KABYLAKE:
-			case INTEL_COFFEELAKE:
-				dev_priv->perf.oa.timestamp_frequency = 12000000;
-				break;
-			default:
-				break;
-			}
 		} else if (IS_GEN10(dev_priv)) {
 			dev_priv->perf.oa.ops.is_valid_b_counter_reg =
 				gen7_is_valid_b_counter_addr;
@@ -3509,15 +3488,10 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
 			dev_priv->perf.oa.ctx_flexeu0_offset = 0x3de;
 
 			dev_priv->perf.oa.gen8_valid_ctx_bit = (1<<16);
-
-			/* Default frequency, although we need to read it from
-			 * the register as it might vary between parts.
-			 */
-			dev_priv->perf.oa.timestamp_frequency = 12000000;
 		}
 	}
 
-	if (dev_priv->perf.oa.timestamp_frequency) {
+	if (dev_priv->perf.oa.ops.enable_metric_set) {
 		hrtimer_init(&dev_priv->perf.oa.poll_check_timer,
 				CLOCK_MONOTONIC, HRTIMER_MODE_REL);
 		dev_priv->perf.oa.poll_check_timer.function = oa_poll_check_timer_cb;
@@ -3528,7 +3502,7 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
 		spin_lock_init(&dev_priv->perf.oa.oa_buffer.ptr_lock);
 
 		oa_sample_rate_hard_limit =
-			dev_priv->perf.oa.timestamp_frequency / 2;
+			INTEL_INFO(dev_priv)->cs_timestamp_frequency / 2;
 		dev_priv->perf.sysctl_header = register_sysctl_table(dev_root);
 
 		mutex_init(&dev_priv->perf.metrics_lock);
-- 
2.15.0

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

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

* Re: [PATCH 4/7] drm/i915: fix register naming
  2017-11-10 19:08 ` [PATCH 4/7] drm/i915: fix register naming Lionel Landwerlin
@ 2017-11-10 19:44   ` Matthew Auld
  2017-11-13  7:28   ` Ewelina Musial
  1 sibling, 0 replies; 23+ messages in thread
From: Matthew Auld @ 2017-11-10 19:44 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: Intel Graphics Development

On 10 November 2017 at 19:08, Lionel Landwerlin
<lionel.g.landwerlin@intel.com> wrote:
> This name was added with the whitelisting of registers for building up OA
> configs. It is contained in a range gen8 whitelist :
>
>    addr >= RPM_CONFIG0.reg && addr <= NOA_CONFIG(8).reg
>
> Hence why the name isn't used anywhere.
>
> v2: Fix register name again RPC->RCP (Matthew)
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.BAT: success for drm/i915: Perf updates
  2017-11-10 19:08 [PATCH 0/7] drm/i915: Perf updates Lionel Landwerlin
                   ` (6 preceding siblings ...)
  2017-11-10 19:08 ` [PATCH 7/7] drm/i915/perf: reuse timestamp frequency from device info Lionel Landwerlin
@ 2017-11-10 19:57 ` Patchwork
  2017-11-10 21:35 ` ✓ Fi.CI.IGT: " Patchwork
  8 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2017-11-10 19:57 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Perf updates
URL   : https://patchwork.freedesktop.org/series/33631/
State : success

== Summary ==

Series 33631v1 drm/i915: Perf updates
https://patchwork.freedesktop.org/api/1.0/series/33631/revisions/1/mbox/

Test gem_exec_suspend:
        Subgroup basic-s3:
                dmesg-fail -> PASS       (fi-kbl-7560u) fdo#103039
        Subgroup basic-s4-devices:
                dmesg-fail -> PASS       (fi-kbl-7560u) fdo#102846
Test gem_flink_basic:
        Subgroup bad-flink:
                dmesg-warn -> PASS       (fi-kbl-7560u) fdo#103049 +4
Test gem_linear_blits:
        Subgroup basic:
                incomplete -> PASS       (fi-kbl-7560u) fdo#103163
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                fail       -> PASS       (fi-gdg-551) fdo#102618

fdo#103039 https://bugs.freedesktop.org/show_bug.cgi?id=103039
fdo#102846 https://bugs.freedesktop.org/show_bug.cgi?id=102846
fdo#103049 https://bugs.freedesktop.org/show_bug.cgi?id=103049
fdo#103163 https://bugs.freedesktop.org/show_bug.cgi?id=103163
fdo#102618 https://bugs.freedesktop.org/show_bug.cgi?id=102618

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:439s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:454s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:383s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:537s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:276s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:503s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:503s
fi-byt-j1900     total:289  pass:254  dwarn:0   dfail:0   fail:0   skip:35  time:490s
fi-byt-n2820     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:488s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:427s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:262s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:539s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:428s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:447s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:424s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:481s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:460s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:484s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:521s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:473s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:534s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:569s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:454s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:545s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:564s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:519s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:491s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:460s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:562s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:419s
Blacklisted hosts:
fi-cfl-s         total:289  pass:254  dwarn:3   dfail:0   fail:0   skip:32  time:538s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:571s
fi-glk-dsi       total:289  pass:258  dwarn:0   dfail:0   fail:1   skip:30  time:510s

6f3dca38f7a939db9658bd11d9c37357089cabef drm-tip: 2017y-11m-10d-12h-56m-44s UTC integration manifest
0ecc5c8be360 drm/i915/perf: enable perf support on CNL
46323c64afa0 drm/i915: fix register naming
4450774c309c drm/i915/perf: refactor perf setup
7a8f1169c793 drm/i915/perf: add support for Coffeelake GT3
5e6410bf5bf9 drm/i915/perf: complete whitelisting for OA programming on HSW

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7069/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 6/7] drm/i915: expose command stream timestamp frequency to userspace
  2017-11-10 19:08 ` [PATCH 6/7] drm/i915: expose command stream timestamp frequency to userspace Lionel Landwerlin
@ 2017-11-10 20:58   ` Chris Wilson
  2017-11-13  9:15   ` Sagar Arun Kamble
  2017-12-01 20:02   ` Paulo Zanoni
  2 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2017-11-10 20:58 UTC (permalink / raw)
  To: Lionel Landwerlin, intel-gfx

Quoting Lionel Landwerlin (2017-11-10 19:08:44)
> +static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
> +{
> +       u64 f12_5_mhz = 12500000;
> +       u64 f19_2_mhz = 19200000;
> +       u64 f24_mhz = 24000000;
> +
> +       if (INTEL_GEN(dev_priv) <= 4) {
> +               /* PRMs say:
> +                *
> +                *     "The value in this register increments once every 16
> +                *      hclks." (through the “Clocking Configuration”
> +                *      (“CLKCFG”) MCHBAR register)
> +                */
> +               return (dev_priv->rawclk_freq * 1000) / 16;
> +       } else if (INTEL_GEN(dev_priv) <= 8) {
> +               /* PRMs say:
> +                *
> +                *     "The PCU TSC counts 10ns increments; this timestamp
> +                *      reflects bits 38:3 of the TSC (i.e. 80ns granularity,
> +                *      rolling over every 1.5 hours).
> +                */
> +               return f12_5_mhz;
> +       } else if (INTEL_GEN(dev_priv) <= 9) {
> +               u32 ctc_reg = I915_READ(CTC_MODE);
> +               u64 freq = 0;
> +
> +               if ((ctc_reg & CTC_SOURCE_PARAMETER_MASK) == CTC_SOURCE_DIVIDE_LOGIC) {
> +                       freq = read_reference_ts_freq(dev_priv);
> +               } else {
> +                       freq = IS_GEN9_LP(dev_priv) ? f19_2_mhz : f24_mhz;
> +
> +                       /* Now figure out how the command stream's timestamp
> +                        * register increments from this frequency (it might
> +                        * increment only every few clock cycle).
> +                        */
> +                       freq >>= 3 - ((ctc_reg & CTC_SHIFT_PARAMETER_MASK) >>
> +                                     CTC_SHIFT_PARAMETER_SHIFT);
> +               }
> +
> +               return freq;
> +       } else if (INTEL_GEN(dev_priv) <= 10) {
> +               u32 ctc_reg = I915_READ(CTC_MODE);
> +               u64 freq = 0;
> +               u32 rpm_config_reg = 0;
> +
> +               /* First figure out the reference frequency. There are 2 ways
> +                * we can compute the frequency, either through the
> +                * TIMESTAMP_OVERRIDE register or through RPM_CONFIG. CTC_MODE
> +                * tells us which one we should use.
> +                */
> +               if ((ctc_reg & CTC_SOURCE_PARAMETER_MASK) == CTC_SOURCE_DIVIDE_LOGIC) {
> +                       freq = read_reference_ts_freq(dev_priv);
> +               } else {
> +                       u32 crystal_clock;
> +
> +                       rpm_config_reg = I915_READ(RPM_CONFIG0);
> +                       crystal_clock = (rpm_config_reg &
> +                                        GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_MASK) >>
> +                               GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT;
> +                       switch (crystal_clock) {
> +                       case GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_19_2_MHZ:
> +                               freq = f19_2_mhz;
> +                               break;
> +                       case GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_24_MHZ:
> +                               freq = f24_mhz;
> +                               break;
> +                       }
> +               }
> +
> +               /* Now figure out how the command stream's timestamp register
> +                * increments from this frequency (it might increment only
> +                * every few clock cycle).
> +                */
> +               freq >>= 3 - ((rpm_config_reg &
> +                              GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_MASK) >>
> +                             GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_SHIFT);
> +
> +               return freq;
> +       }
> +
> +       DRM_ERROR("Unknown gen, unable to compute command stream timestamp frequency\n");

Typically this would be a MISSING_CASE(). It gives something to grep for
when adding new gen.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 7/7] drm/i915/perf: reuse timestamp frequency from device info
  2017-11-10 19:08 ` [PATCH 7/7] drm/i915/perf: reuse timestamp frequency from device info Lionel Landwerlin
@ 2017-11-10 21:00   ` Chris Wilson
  2017-11-13 15:19     ` Lionel Landwerlin
  0 siblings, 1 reply; 23+ messages in thread
From: Chris Wilson @ 2017-11-10 21:00 UTC (permalink / raw)
  To: Lionel Landwerlin, intel-gfx

Quoting Lionel Landwerlin (2017-11-10 19:08:45)
> @@ -3528,7 +3502,7 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
>                 spin_lock_init(&dev_priv->perf.oa.oa_buffer.ptr_lock);
>  
>                 oa_sample_rate_hard_limit =
> -                       dev_priv->perf.oa.timestamp_frequency / 2;
> +                       INTEL_INFO(dev_priv)->cs_timestamp_frequency / 2;

32-bit builds may complain, as this is ostensibly now a 64b division.
Right?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* ✓ Fi.CI.IGT: success for drm/i915: Perf updates
  2017-11-10 19:08 [PATCH 0/7] drm/i915: Perf updates Lionel Landwerlin
                   ` (7 preceding siblings ...)
  2017-11-10 19:57 ` ✓ Fi.CI.BAT: success for drm/i915: Perf updates Patchwork
@ 2017-11-10 21:35 ` Patchwork
  8 siblings, 0 replies; 23+ messages in thread
From: Patchwork @ 2017-11-10 21:35 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Perf updates
URL   : https://patchwork.freedesktop.org/series/33631/
State : success

== Summary ==

Test kms_busy:
        Subgroup extended-modeset-hang-oldfb-with-reset-render-c:
                pass       -> DMESG-WARN (shard-hsw) fdo#102249
Test kms_flip:
        Subgroup modeset-vs-vblank-race-interruptible:
                fail       -> PASS       (shard-hsw) fdo#103060
Test kms_frontbuffer_tracking:
        Subgroup fbc-farfromfence:
                skip       -> PASS       (shard-hsw)

fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060

shard-hsw        total:2584 pass:1470 dwarn:3   dfail:1   fail:11  skip:1099 time:9474s
Blacklisted hosts:
shard-apl        total:2584 pass:1621 dwarn:4   dfail:1   fail:22  skip:936 time:13157s
shard-kbl        total:2484 pass:1641 dwarn:10  dfail:0   fail:26  skip:804 time:9720s
shard-snb        total:2584 pass:1213 dwarn:3   dfail:1   fail:10  skip:1357 time:7820s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_7069/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/7] drm/i915/perf: complete whitelisting for OA programming on HSW
  2017-11-10 19:08 ` [PATCH 1/7] drm/i915/perf: complete whitelisting for OA programming on HSW Lionel Landwerlin
@ 2017-11-13  7:15   ` Ewelina Musial
  2017-11-13 11:40   ` Chris Wilson
  1 sibling, 0 replies; 23+ messages in thread
From: Ewelina Musial @ 2017-11-13  7:15 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

On Fri, Nov 10, 2017 at 07:08:39PM +0000, Lionel Landwerlin wrote:
> We were missing some registers and also can name one for which we only had
> the offset.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_perf.c |  3 ++-
>  drivers/gpu/drm/i915/i915_reg.h  | 14 ++++++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 59ee808f8fd9..45aef15b9e7c 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -3023,7 +3023,8 @@ static bool hsw_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 addr)
>  {
>  	return gen7_is_valid_mux_addr(dev_priv, addr) ||
>  		(addr >= 0x25100 && addr <= 0x2FF90) ||
Maybe some define here also? 
- Ewelina
> -		addr == 0x9ec0;
> +		(addr >= HSW_MBVID2_NOA0.reg && addr <= HSW_MBVID2_NOA9.reg) ||
> +		addr == HSW_MBVID2_MISR0.reg;
>  }
>  
>  static bool chv_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 addr)
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 6ef33422f762..8f3cf50f04b4 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1120,6 +1120,20 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  /* RPC unit config (Gen8+) */
>  #define RPM_CONFIG	    _MMIO(0x0D08)
>  
> +/* NOA (HSW) */
> +#define HSW_MBVID2_NOA0		_MMIO(0x9E80)
> +#define HSW_MBVID2_NOA1		_MMIO(0x9E84)
> +#define HSW_MBVID2_NOA2		_MMIO(0x9E88)
> +#define HSW_MBVID2_NOA3		_MMIO(0x9E8C)
> +#define HSW_MBVID2_NOA4		_MMIO(0x9E90)
> +#define HSW_MBVID2_NOA5		_MMIO(0x9E94)
> +#define HSW_MBVID2_NOA6		_MMIO(0x9E98)
> +#define HSW_MBVID2_NOA7		_MMIO(0x9E9C)
> +#define HSW_MBVID2_NOA8		_MMIO(0x9EA0)
> +#define HSW_MBVID2_NOA9		_MMIO(0x9EA4)
> +
> +#define HSW_MBVID2_MISR0	_MMIO(0x9EC0)
> +
>  /* NOA (Gen8+) */
>  #define NOA_CONFIG(i)	    _MMIO(0x0D0C + (i) * 4)
>  
> -- 
> 2.15.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 2/7] drm/i915/perf: add support for Coffeelake GT3
  2017-11-10 19:08 ` [PATCH 2/7] drm/i915/perf: add support for Coffeelake GT3 Lionel Landwerlin
@ 2017-11-13  7:18   ` Ewelina Musial
  0 siblings, 0 replies; 23+ messages in thread
From: Ewelina Musial @ 2017-11-13  7:18 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

On Fri, Nov 10, 2017 at 07:08:40PM +0000, Lionel Landwerlin wrote:
> We can enable GT3 as well as GT2.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
Reviewed-by: Ewelina Musial <ewelina.musial@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile         |   3 +-
>  drivers/gpu/drm/i915/i915_drv.h       |   2 +
>  drivers/gpu/drm/i915/i915_oa_cflgt3.c | 109 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_oa_cflgt3.h |  34 +++++++++++
>  drivers/gpu/drm/i915/i915_perf.c      |   3 +
>  5 files changed, 150 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/i915/i915_oa_cflgt3.c
>  create mode 100644 drivers/gpu/drm/i915/i915_oa_cflgt3.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index 177404462386..58463bd94b4c 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -163,7 +163,8 @@ i915-y += i915_perf.o \
>  	  i915_oa_kblgt2.o \
>  	  i915_oa_kblgt3.o \
>  	  i915_oa_glk.o \
> -	  i915_oa_cflgt2.o
> +	  i915_oa_cflgt2.o \
> +	  i915_oa_cflgt3.o
>  
>  ifeq ($(CONFIG_DRM_I915_GVT),y)
>  i915-y += intel_gvt.o
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 40012b6daea2..9f26c34e0e52 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3058,6 +3058,8 @@ intel_info(const struct drm_i915_private *dev_priv)
>  				 (INTEL_DEVID(dev_priv) & 0x00F0) == 0x00A0)
>  #define IS_CFL_GT2(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
>  				 (dev_priv)->info.gt == 2)
> +#define IS_CFL_GT3(dev_priv)	(IS_COFFEELAKE(dev_priv) && \
> +				 (dev_priv)->info.gt == 3)
>  
>  #define IS_ALPHA_SUPPORT(intel_info) ((intel_info)->is_alpha_support)
>  
> diff --git a/drivers/gpu/drm/i915/i915_oa_cflgt3.c b/drivers/gpu/drm/i915/i915_oa_cflgt3.c
> new file mode 100644
> index 000000000000..42ff06fe54a3
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_oa_cflgt3.c
> @@ -0,0 +1,109 @@
> +/*
> + * Autogenerated file by GPU Top : https://github.com/rib/gputop
> + * DO NOT EDIT manually!
> + *
> + *
> + * Copyright (c) 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include <linux/sysfs.h>
> +
> +#include "i915_drv.h"
> +#include "i915_oa_cflgt3.h"
> +
> +static const struct i915_oa_reg b_counter_config_test_oa[] = {
> +	{ _MMIO(0x2740), 0x00000000 },
> +	{ _MMIO(0x2744), 0x00800000 },
> +	{ _MMIO(0x2714), 0xf0800000 },
> +	{ _MMIO(0x2710), 0x00000000 },
> +	{ _MMIO(0x2724), 0xf0800000 },
> +	{ _MMIO(0x2720), 0x00000000 },
> +	{ _MMIO(0x2770), 0x00000004 },
> +	{ _MMIO(0x2774), 0x00000000 },
> +	{ _MMIO(0x2778), 0x00000003 },
> +	{ _MMIO(0x277c), 0x00000000 },
> +	{ _MMIO(0x2780), 0x00000007 },
> +	{ _MMIO(0x2784), 0x00000000 },
> +	{ _MMIO(0x2788), 0x00100002 },
> +	{ _MMIO(0x278c), 0x0000fff7 },
> +	{ _MMIO(0x2790), 0x00100002 },
> +	{ _MMIO(0x2794), 0x0000ffcf },
> +	{ _MMIO(0x2798), 0x00100082 },
> +	{ _MMIO(0x279c), 0x0000ffef },
> +	{ _MMIO(0x27a0), 0x001000c2 },
> +	{ _MMIO(0x27a4), 0x0000ffe7 },
> +	{ _MMIO(0x27a8), 0x00100001 },
> +	{ _MMIO(0x27ac), 0x0000ffe7 },
> +};
> +
> +static const struct i915_oa_reg flex_eu_config_test_oa[] = {
> +};
> +
> +static const struct i915_oa_reg mux_config_test_oa[] = {
> +	{ _MMIO(0x9840), 0x00000080 },
> +	{ _MMIO(0x9888), 0x11810000 },
> +	{ _MMIO(0x9888), 0x07810013 },
> +	{ _MMIO(0x9888), 0x1f810000 },
> +	{ _MMIO(0x9888), 0x1d810000 },
> +	{ _MMIO(0x9888), 0x1b930040 },
> +	{ _MMIO(0x9888), 0x07e54000 },
> +	{ _MMIO(0x9888), 0x1f908000 },
> +	{ _MMIO(0x9888), 0x11900000 },
> +	{ _MMIO(0x9888), 0x37900000 },
> +	{ _MMIO(0x9888), 0x53900000 },
> +	{ _MMIO(0x9888), 0x45900000 },
> +	{ _MMIO(0x9888), 0x33900000 },
> +};
> +
> +static ssize_t
> +show_test_oa_id(struct device *kdev, struct device_attribute *attr, char *buf)
> +{
> +	return sprintf(buf, "1\n");
> +}
> +
> +void
> +i915_perf_load_test_config_cflgt3(struct drm_i915_private *dev_priv)
> +{
> +	strncpy(dev_priv->perf.oa.test_config.uuid,
> +		"577e8e2c-3fa0-4875-8743-3538d585e3b0",
> +		UUID_STRING_LEN);
> +	dev_priv->perf.oa.test_config.id = 1;
> +
> +	dev_priv->perf.oa.test_config.mux_regs = mux_config_test_oa;
> +	dev_priv->perf.oa.test_config.mux_regs_len = ARRAY_SIZE(mux_config_test_oa);
> +
> +	dev_priv->perf.oa.test_config.b_counter_regs = b_counter_config_test_oa;
> +	dev_priv->perf.oa.test_config.b_counter_regs_len = ARRAY_SIZE(b_counter_config_test_oa);
> +
> +	dev_priv->perf.oa.test_config.flex_regs = flex_eu_config_test_oa;
> +	dev_priv->perf.oa.test_config.flex_regs_len = ARRAY_SIZE(flex_eu_config_test_oa);
> +
> +	dev_priv->perf.oa.test_config.sysfs_metric.name = "577e8e2c-3fa0-4875-8743-3538d585e3b0";
> +	dev_priv->perf.oa.test_config.sysfs_metric.attrs = dev_priv->perf.oa.test_config.attrs;
> +
> +	dev_priv->perf.oa.test_config.attrs[0] = &dev_priv->perf.oa.test_config.sysfs_metric_id.attr;
> +
> +	dev_priv->perf.oa.test_config.sysfs_metric_id.attr.name = "id";
> +	dev_priv->perf.oa.test_config.sysfs_metric_id.attr.mode = 0444;
> +	dev_priv->perf.oa.test_config.sysfs_metric_id.show = show_test_oa_id;
> +}
> diff --git a/drivers/gpu/drm/i915/i915_oa_cflgt3.h b/drivers/gpu/drm/i915/i915_oa_cflgt3.h
> new file mode 100644
> index 000000000000..c13b5aac01b9
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_oa_cflgt3.h
> @@ -0,0 +1,34 @@
> +/*
> + * Autogenerated file by GPU Top : https://github.com/rib/gputop
> + * DO NOT EDIT manually!
> + *
> + *
> + * Copyright (c) 2015 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef __I915_OA_CFLGT3_H__
> +#define __I915_OA_CFLGT3_H__
> +
> +extern void i915_perf_load_test_config_cflgt3(struct drm_i915_private *dev_priv);
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 45aef15b9e7c..7271debe0417 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -207,6 +207,7 @@
>  #include "i915_oa_kblgt3.h"
>  #include "i915_oa_glk.h"
>  #include "i915_oa_cflgt2.h"
> +#include "i915_oa_cflgt3.h"
>  
>  /* HW requires this to be a power of two, between 128k and 16M, though driver
>   * is currently generally designed assuming the largest 16M size is used such
> @@ -2934,6 +2935,8 @@ void i915_perf_register(struct drm_i915_private *dev_priv)
>  	} else if (IS_COFFEELAKE(dev_priv)) {
>  		if (IS_CFL_GT2(dev_priv))
>  			i915_perf_load_test_config_cflgt2(dev_priv);
> +		if (IS_CFL_GT3(dev_priv))
> +			i915_perf_load_test_config_cflgt3(dev_priv);
>  	}
>  
>  	if (dev_priv->perf.oa.test_config.id == 0)
> -- 
> 2.15.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 4/7] drm/i915: fix register naming
  2017-11-10 19:08 ` [PATCH 4/7] drm/i915: fix register naming Lionel Landwerlin
  2017-11-10 19:44   ` Matthew Auld
@ 2017-11-13  7:28   ` Ewelina Musial
  1 sibling, 0 replies; 23+ messages in thread
From: Ewelina Musial @ 2017-11-13  7:28 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

On Fri, Nov 10, 2017 at 07:08:42PM +0000, Lionel Landwerlin wrote:
> This name was added with the whitelisting of registers for building up OA
> configs. It is contained in a range gen8 whitelist :
> 
>    addr >= RPM_CONFIG0.reg && addr <= NOA_CONFIG(8).reg
> 
> Hence why the name isn't used anywhere.
> 
> v2: Fix register name again RPC->RCP (Matthew)
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Ewelina Musial <ewelina.musial@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 8f3cf50f04b4..f812224d1fc8 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1117,8 +1117,8 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>  #define RPM_CONFIG0	    _MMIO(0x0D00)
>  #define RPM_CONFIG1	    _MMIO(0x0D04)
>  
> -/* RPC unit config (Gen8+) */
> -#define RPM_CONFIG	    _MMIO(0x0D08)
> +/* RCP unit config (Gen8+) */
> +#define RCP_CONFIG	    _MMIO(0x0D08)
>  
>  /* NOA (HSW) */
>  #define HSW_MBVID2_NOA0		_MMIO(0x9E80)
> -- 
> 2.15.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 6/7] drm/i915: expose command stream timestamp frequency to userspace
  2017-11-10 19:08 ` [PATCH 6/7] drm/i915: expose command stream timestamp frequency to userspace Lionel Landwerlin
  2017-11-10 20:58   ` Chris Wilson
@ 2017-11-13  9:15   ` Sagar Arun Kamble
  2017-12-01 20:02   ` Paulo Zanoni
  2 siblings, 0 replies; 23+ messages in thread
From: Sagar Arun Kamble @ 2017-11-13  9:15 UTC (permalink / raw)
  To: Lionel Landwerlin, intel-gfx



On 11/11/2017 12:38 AM, Lionel Landwerlin wrote:
> We use to have this fixed per generation, but starting with CNL userspace
> cannot tell just off the PCI ID. Let's make this information available. This
> is particularly useful for performance monitoring where much of the
> normalization work is done using those timestamps (this include pipeline
> statistics in both GL & Vulkan as well as OA reports).
>
> v2: Use variables for 24MHz/19.2MHz values (Ewelina)
>      Renamed function & coding style (Sagar)
>
> v3: Fix frequency read on Broadwell (Sagar)
>      Fix missing divide by 4 on <= gen4 (Sagar)
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Tested-by: Rafael Antognolli <rafael.antognolli@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c      |   2 +
>   drivers/gpu/drm/i915/i915_drv.c          |   3 +
>   drivers/gpu/drm/i915/i915_drv.h          |   2 +
>   drivers/gpu/drm/i915/i915_reg.h          |  21 ++++++
>   drivers/gpu/drm/i915/intel_device_info.c | 107 +++++++++++++++++++++++++++++++
>   include/uapi/drm/i915_drm.h              |   6 ++
>   6 files changed, 141 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 533ba096b9a6..57485f29d7c9 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3245,6 +3245,8 @@ static int i915_engine_info(struct seq_file *m, void *unused)
>   		   yesno(dev_priv->gt.awake));
>   	seq_printf(m, "Global active requests: %d\n",
>   		   dev_priv->gt.active_requests);
> +	seq_printf(m, "CS timestamp frequency: %llu Hz\n",
> +		   dev_priv->info.cs_timestamp_frequency);
>   
>   	p = drm_seq_file_printer(m);
>   	for_each_engine(engine, dev_priv, id)
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index d97fe9c9439a..dbd04d5f75ee 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -419,6 +419,9 @@ static int i915_getparam(struct drm_device *dev, void *data,
>   		if (!value)
>   			return -ENODEV;
>   		break;
> +	case I915_PARAM_CS_TIMESTAMP_FREQUENCY:
> +		value = INTEL_INFO(dev_priv)->cs_timestamp_frequency;
> +		break;
>   	default:
>   		DRM_DEBUG("Unknown parameter %d\n", param->param);
>   		return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9f26c34e0e52..ebc49ca58eb7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -885,6 +885,8 @@ struct intel_device_info {
>   	/* Slice/subslice/EU info */
>   	struct sseu_dev_info sseu;
>   
> +	u64 cs_timestamp_frequency;
> +
>   	struct color_luts {
>   		u16 degamma_lut_size;
>   		u16 gamma_lut_size;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7aa7dc0c336b..ec9faa11e305 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1119,9 +1119,24 @@ static inline bool i915_mmio_reg_valid(i915_reg_t reg)
>   
>   /* RPM unit config (Gen8+) */
>   #define RPM_CONFIG0	    _MMIO(0x0D00)
> +#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT	3
> +#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_MASK	(1 << GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT)
> +#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_19_2_MHZ	0
> +#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_24_MHZ	1
> +#define  GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_SHIFT	1
> +#define  GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_MASK	(0x3 << GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_SHIFT)
> +
>   #define RPM_CONFIG1	    _MMIO(0x0D04)
>   #define  GEN10_GT_NOA_ENABLE  (1 << 9)
>   
> +/* GPM unit config (Gen9+) */
> +#define CTC_MODE			_MMIO(0xA26C)
> +#define  CTC_SOURCE_PARAMETER_MASK 1
> +#define  CTC_SOURCE_CRYSTAL_CLOCK	0
> +#define  CTC_SOURCE_DIVIDE_LOGIC	1
> +#define  CTC_SHIFT_PARAMETER_SHIFT	1
> +#define  CTC_SHIFT_PARAMETER_MASK	(0x3 << CTC_SHIFT_PARAMETER_SHIFT)
> +
>   /* RCP unit config (Gen8+) */
>   #define RCP_CONFIG	    _MMIO(0x0D08)
>   
> @@ -8866,6 +8881,12 @@ enum skl_power_gate {
>   #define ILK_TIMESTAMP_HI	_MMIO(0x70070)
>   #define IVB_TIMESTAMP_CTR	_MMIO(0x44070)
>   
> +#define GEN9_TIMESTAMP_OVERRIDE				_MMIO(0x44074)
> +#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_SHIFT	0
> +#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_MASK	0x3ff
> +#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_SHIFT	12
> +#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_MASK	(0xf << 12)
> +
>   #define _PIPE_FRMTMSTMP_A		0x70048
>   #define PIPE_FRMTMSTMP(pipe)		\
>   			_MMIO_PIPE2(pipe, _PIPE_FRMTMSTMP_A)
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index db03d179fc85..78bf7374fbdd 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -329,6 +329,108 @@ static void broadwell_sseu_info_init(struct drm_i915_private *dev_priv)
>   	sseu->has_eu_pg = 0;
>   }
>   
> +static u64 read_reference_ts_freq(struct drm_i915_private *dev_priv)
> +{
> +	u32 ts_override = I915_READ(GEN9_TIMESTAMP_OVERRIDE);
> +	u64 base_freq, frac_freq;
> +
> +	base_freq = ((ts_override & GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_MASK) >>
> +		     GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_SHIFT) + 1;
> +	base_freq *= 1000000;
> +
> +	frac_freq = ((ts_override &
> +		      GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_MASK) >>
> +		     GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_SHIFT);
> +	if (frac_freq != 0)
> +		frac_freq = 1000000 / (frac_freq + 1);
> +
> +	return base_freq + frac_freq;
> +}
> +
> +static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
> +{
> +	u64 f12_5_mhz = 12500000;
> +	u64 f19_2_mhz = 19200000;
> +	u64 f24_mhz = 24000000;
> +
> +	if (INTEL_GEN(dev_priv) <= 4) {
> +		/* PRMs say:
> +		 *
> +		 *     "The value in this register increments once every 16
> +		 *      hclks." (through the “Clocking Configuration”
> +		 *      (“CLKCFG”) MCHBAR register)
> +		 */
> +		return (dev_priv->rawclk_freq * 1000) / 16;
> +	} else if (INTEL_GEN(dev_priv) <= 8) {
> +		/* PRMs say:
> +		 *
> +		 *     "The PCU TSC counts 10ns increments; this timestamp
> +		 *      reflects bits 38:3 of the TSC (i.e. 80ns granularity,
> +		 *      rolling over every 1.5 hours).
> +		 */
> +		return f12_5_mhz;
> +	} else if (INTEL_GEN(dev_priv) <= 9) {
> +		u32 ctc_reg = I915_READ(CTC_MODE);
> +		u64 freq = 0;
> +
> +		if ((ctc_reg & CTC_SOURCE_PARAMETER_MASK) == CTC_SOURCE_DIVIDE_LOGIC) {
> +			freq = read_reference_ts_freq(dev_priv);
> +		} else {
> +			freq = IS_GEN9_LP(dev_priv) ? f19_2_mhz : f24_mhz;
> +
> +			/* Now figure out how the command stream's timestamp
> +			 * register increments from this frequency (it might
> +			 * increment only every few clock cycle).
> +			 */
> +			freq >>= 3 - ((ctc_reg & CTC_SHIFT_PARAMETER_MASK) >>
> +				      CTC_SHIFT_PARAMETER_SHIFT);
> +		}
> +
> +		return freq;
> +	} else if (INTEL_GEN(dev_priv) <= 10) {
> +		u32 ctc_reg = I915_READ(CTC_MODE);
> +		u64 freq = 0;
> +		u32 rpm_config_reg = 0;
> +
> +		/* First figure out the reference frequency. There are 2 ways
> +		 * we can compute the frequency, either through the
> +		 * TIMESTAMP_OVERRIDE register or through RPM_CONFIG. CTC_MODE
> +		 * tells us which one we should use.
> +		 */
> +		if ((ctc_reg & CTC_SOURCE_PARAMETER_MASK) == CTC_SOURCE_DIVIDE_LOGIC) {
> +			freq = read_reference_ts_freq(dev_priv);
> +		} else {
> +			u32 crystal_clock;
> +
> +			rpm_config_reg = I915_READ(RPM_CONFIG0);
> +			crystal_clock = (rpm_config_reg &
> +					 GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_MASK) >>
> +				GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT;
> +			switch (crystal_clock) {
> +			case GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_19_2_MHZ:
> +				freq = f19_2_mhz;
> +				break;
> +			case GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_24_MHZ:
> +				freq = f24_mhz;
> +				break;
> +			}
> +		}
> +
> +		/* Now figure out how the command stream's timestamp register
> +		 * increments from this frequency (it might increment only
> +		 * every few clock cycle).
> +		 */
> +		freq >>= 3 - ((rpm_config_reg &
> +			       GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_MASK) >>
> +			      GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_SHIFT);
> +
> +		return freq;
> +	}
> +
> +	DRM_ERROR("Unknown gen, unable to compute command stream timestamp frequency\n");
> +	return 0;
> +}
> +
>   /*
>    * Determine various intel_device_info fields at runtime.
>    *
> @@ -450,6 +552,9 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>   	else if (INTEL_GEN(dev_priv) >= 10)
>   		gen10_sseu_info_init(dev_priv);
>   
> +	/* Initialize command stream timestamp frequency */
> +	info->cs_timestamp_frequency = read_timestamp_frequency(dev_priv);
> +
>   	DRM_DEBUG_DRIVER("slice mask: %04x\n", info->sseu.slice_mask);
>   	DRM_DEBUG_DRIVER("slice total: %u\n", hweight8(info->sseu.slice_mask));
>   	DRM_DEBUG_DRIVER("subslice total: %u\n",
> @@ -465,4 +570,6 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>   			 info->sseu.has_subslice_pg ? "y" : "n");
>   	DRM_DEBUG_DRIVER("has EU power gating: %s\n",
>   			 info->sseu.has_eu_pg ? "y" : "n");
> +	DRM_DEBUG_DRIVER("CS timestamp frequency: %llu\n",
> +			 info->cs_timestamp_frequency);
>   }
> diff --git a/include/uapi/drm/i915_drm.h b/include/uapi/drm/i915_drm.h
> index 6c02ced663f8..b57985929553 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -481,6 +481,12 @@ typedef struct drm_i915_irq_wait {
>    */
>   #define I915_PARAM_HAS_CONTEXT_ISOLATION 50
>   
> +/* Frequency of the command streamer timestamps given by the *_TIMESTAMP
> + * registers. This used to be fixed per platform but from CNL onwards, this
> + * might vary depending on the parts.
> + */
> +#define I915_PARAM_CS_TIMESTAMP_FREQUENCY 51
> +
>   typedef struct drm_i915_getparam {
>   	__s32 param;
>   	/*

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

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

* Re: [PATCH 1/7] drm/i915/perf: complete whitelisting for OA programming on HSW
  2017-11-10 19:08 ` [PATCH 1/7] drm/i915/perf: complete whitelisting for OA programming on HSW Lionel Landwerlin
  2017-11-13  7:15   ` Ewelina Musial
@ 2017-11-13 11:40   ` Chris Wilson
  2017-11-13 11:43     ` Lionel Landwerlin
  1 sibling, 1 reply; 23+ messages in thread
From: Chris Wilson @ 2017-11-13 11:40 UTC (permalink / raw)
  To: Lionel Landwerlin, intel-gfx

Quoting Lionel Landwerlin (2017-11-10 19:08:39)
> We were missing some registers and also can name one for which we only had
> the offset.
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_perf.c |  3 ++-
>  drivers/gpu/drm/i915/i915_reg.h  | 14 ++++++++++++++
>  2 files changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 59ee808f8fd9..45aef15b9e7c 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -3023,7 +3023,8 @@ static bool hsw_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 addr)
>  {
>         return gen7_is_valid_mux_addr(dev_priv, addr) ||
>                 (addr >= 0x25100 && addr <= 0x2FF90) ||
> -               addr == 0x9ec0;
> +               (addr >= HSW_MBVID2_NOA0.reg && addr <= HSW_MBVID2_NOA9.reg) ||
> +               addr == HSW_MBVID2_MISR0.reg;

i915_mmio_reg_offset(HSW_MBVID2_NOA0) etc
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 1/7] drm/i915/perf: complete whitelisting for OA programming on HSW
  2017-11-13 11:40   ` Chris Wilson
@ 2017-11-13 11:43     ` Lionel Landwerlin
  0 siblings, 0 replies; 23+ messages in thread
From: Lionel Landwerlin @ 2017-11-13 11:43 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On 13/11/17 11:40, Chris Wilson wrote:
> Quoting Lionel Landwerlin (2017-11-10 19:08:39)
>> We were missing some registers and also can name one for which we only had
>> the offset.
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> Reviewed-by: Matthew Auld <matthew.auld@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_perf.c |  3 ++-
>>   drivers/gpu/drm/i915/i915_reg.h  | 14 ++++++++++++++
>>   2 files changed, 16 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
>> index 59ee808f8fd9..45aef15b9e7c 100644
>> --- a/drivers/gpu/drm/i915/i915_perf.c
>> +++ b/drivers/gpu/drm/i915/i915_perf.c
>> @@ -3023,7 +3023,8 @@ static bool hsw_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 addr)
>>   {
>>          return gen7_is_valid_mux_addr(dev_priv, addr) ||
>>                  (addr >= 0x25100 && addr <= 0x2FF90) ||
>> -               addr == 0x9ec0;
>> +               (addr >= HSW_MBVID2_NOA0.reg && addr <= HSW_MBVID2_NOA9.reg) ||
>> +               addr == HSW_MBVID2_MISR0.reg;
> i915_mmio_reg_offset(HSW_MBVID2_NOA0) etc
> -Chris
>
Oh thanks, will send a follow up patch to clean that up.

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

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

* Re: [PATCH 7/7] drm/i915/perf: reuse timestamp frequency from device info
  2017-11-10 21:00   ` Chris Wilson
@ 2017-11-13 15:19     ` Lionel Landwerlin
  2017-11-13 15:28       ` Chris Wilson
  0 siblings, 1 reply; 23+ messages in thread
From: Lionel Landwerlin @ 2017-11-13 15:19 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

On 10/11/17 21:00, Chris Wilson wrote:
> Quoting Lionel Landwerlin (2017-11-10 19:08:45)
>> @@ -3528,7 +3502,7 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
>>                  spin_lock_init(&dev_priv->perf.oa.oa_buffer.ptr_lock);
>>   
>>                  oa_sample_rate_hard_limit =
>> -                       dev_priv->perf.oa.timestamp_frequency / 2;
>> +                       INTEL_INFO(dev_priv)->cs_timestamp_frequency / 2;
> 32-bit builds may complain, as this is ostensibly now a 64b division.
> Right?
> -Chris
>
Well, we already store cs_timestamp_frequency in the getparam value.
The assumption is that we'll stay well below the INT_MAX.
So that should be fine.

The following didn't raise a warning on the division with 
gcc7.2.0/clang3.8.1 -m32 -Wall -Wextra :

int
main(int argc, char *argv[])
{
   uint64_t plop = ~0ULL;
   int ret = plop / 2;

   return ret;
}


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

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

* Re: [PATCH 7/7] drm/i915/perf: reuse timestamp frequency from device info
  2017-11-13 15:19     ` Lionel Landwerlin
@ 2017-11-13 15:28       ` Chris Wilson
  0 siblings, 0 replies; 23+ messages in thread
From: Chris Wilson @ 2017-11-13 15:28 UTC (permalink / raw)
  To: Lionel Landwerlin, intel-gfx

Quoting Lionel Landwerlin (2017-11-13 15:19:28)
> On 10/11/17 21:00, Chris Wilson wrote:
> > Quoting Lionel Landwerlin (2017-11-10 19:08:45)
> >> @@ -3528,7 +3502,7 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
> >>                  spin_lock_init(&dev_priv->perf.oa.oa_buffer.ptr_lock);
> >>   
> >>                  oa_sample_rate_hard_limit =
> >> -                       dev_priv->perf.oa.timestamp_frequency / 2;
> >> +                       INTEL_INFO(dev_priv)->cs_timestamp_frequency / 2;
> > 32-bit builds may complain, as this is ostensibly now a 64b division.
> > Right?
> > -Chris
> >
> Well, we already store cs_timestamp_frequency in the getparam value.
> The assumption is that we'll stay well below the INT_MAX.
> So that should be fine.

Do you want to risk kbuild complaining? :)

> The following didn't raise a warning on the division with 
> gcc7.2.0/clang3.8.1 -m32 -Wall -Wextra :

You have to also use -standalone to avoid the libgcc fixups, iirc.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 6/7] drm/i915: expose command stream timestamp frequency to userspace
  2017-11-10 19:08 ` [PATCH 6/7] drm/i915: expose command stream timestamp frequency to userspace Lionel Landwerlin
  2017-11-10 20:58   ` Chris Wilson
  2017-11-13  9:15   ` Sagar Arun Kamble
@ 2017-12-01 20:02   ` Paulo Zanoni
  2017-12-01 20:43     ` Lionel Landwerlin
  2 siblings, 1 reply; 23+ messages in thread
From: Paulo Zanoni @ 2017-12-01 20:02 UTC (permalink / raw)
  To: Lionel Landwerlin, intel-gfx

Em Sex, 2017-11-10 às 19:08 +0000, Lionel Landwerlin escreveu:
> We use to have this fixed per generation, but starting with CNL
> userspace
> cannot tell just off the PCI ID. Let's make this information
> available. This
> is particularly useful for performance monitoring where much of the
> normalization work is done using those timestamps (this include
> pipeline
> statistics in both GL & Vulkan as well as OA reports).
> 
> v2: Use variables for 24MHz/19.2MHz values (Ewelina)
>     Renamed function & coding style (Sagar)
> 
> v3: Fix frequency read on Broadwell (Sagar)
>     Fix missing divide by 4 on <= gen4 (Sagar)
> 
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Tested-by: Rafael Antognolli <rafael.antognolli@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c      |   2 +
>  drivers/gpu/drm/i915/i915_drv.c          |   3 +
>  drivers/gpu/drm/i915/i915_drv.h          |   2 +
>  drivers/gpu/drm/i915/i915_reg.h          |  21 ++++++
>  drivers/gpu/drm/i915/intel_device_info.c | 107
> +++++++++++++++++++++++++++++++
>  include/uapi/drm/i915_drm.h              |   6 ++
>  6 files changed, 141 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
> b/drivers/gpu/drm/i915/i915_debugfs.c
> index 533ba096b9a6..57485f29d7c9 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3245,6 +3245,8 @@ static int i915_engine_info(struct seq_file *m,
> void *unused)
>  		   yesno(dev_priv->gt.awake));
>  	seq_printf(m, "Global active requests: %d\n",
>  		   dev_priv->gt.active_requests);
> +	seq_printf(m, "CS timestamp frequency: %llu Hz\n",
> +		   dev_priv->info.cs_timestamp_frequency);
>  
>  	p = drm_seq_file_printer(m);
>  	for_each_engine(engine, dev_priv, id)
> diff --git a/drivers/gpu/drm/i915/i915_drv.c
> b/drivers/gpu/drm/i915/i915_drv.c
> index d97fe9c9439a..dbd04d5f75ee 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -419,6 +419,9 @@ static int i915_getparam(struct drm_device *dev,
> void *data,
>  		if (!value)
>  			return -ENODEV;
>  		break;
> +	case I915_PARAM_CS_TIMESTAMP_FREQUENCY:
> +		value = INTEL_INFO(dev_priv)-
> >cs_timestamp_frequency;
> +		break;
>  	default:
>  		DRM_DEBUG("Unknown parameter %d\n", param->param);
>  		return -EINVAL;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> index 9f26c34e0e52..ebc49ca58eb7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -885,6 +885,8 @@ struct intel_device_info {
>  	/* Slice/subslice/EU info */
>  	struct sseu_dev_info sseu;
>  
> +	u64 cs_timestamp_frequency;
> +
>  	struct color_luts {
>  		u16 degamma_lut_size;
>  		u16 gamma_lut_size;
> diff --git a/drivers/gpu/drm/i915/i915_reg.h
> b/drivers/gpu/drm/i915/i915_reg.h
> index 7aa7dc0c336b..ec9faa11e305 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -1119,9 +1119,24 @@ static inline bool
> i915_mmio_reg_valid(i915_reg_t reg)
>  
>  /* RPM unit config (Gen8+) */
>  #define RPM_CONFIG0	    _MMIO(0x0D00)
> +#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT	3
> +#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_MASK	(1 <<
> GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT)
> +#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_19_2_MHZ	0
> +#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_24_MHZ	1
> +#define  GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_SHIFT	1
> +#define  GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_MASK	(0x3 <<
> GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_SHIFT)
> +
>  #define RPM_CONFIG1	    _MMIO(0x0D04)
>  #define  GEN10_GT_NOA_ENABLE  (1 << 9)
>  
> +/* GPM unit config (Gen9+) */
> +#define CTC_MODE			_MMIO(0xA26C)
> +#define  CTC_SOURCE_PARAMETER_MASK 1
> +#define  CTC_SOURCE_CRYSTAL_CLOCK	0
> +#define  CTC_SOURCE_DIVIDE_LOGIC	1
> +#define  CTC_SHIFT_PARAMETER_SHIFT	1
> +#define  CTC_SHIFT_PARAMETER_MASK	(0x3 <<
> CTC_SHIFT_PARAMETER_SHIFT)
> +
>  /* RCP unit config (Gen8+) */
>  #define RCP_CONFIG	    _MMIO(0x0D08)
>  
> @@ -8866,6 +8881,12 @@ enum skl_power_gate {
>  #define ILK_TIMESTAMP_HI	_MMIO(0x70070)
>  #define IVB_TIMESTAMP_CTR	_MMIO(0x44070)
>  
> +#define GEN9_TIMESTAMP_OVERRIDE				_MMIO
> (0x44074)
> +#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_SHIFT	0
> +#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_MASK	0x3f
> f
> +#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_SHIFT	
> 12
> +#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_MASK	
> (0xf << 12)
> +
>  #define _PIPE_FRMTMSTMP_A		0x70048
>  #define PIPE_FRMTMSTMP(pipe)		\
>  			_MMIO_PIPE2(pipe, _PIPE_FRMTMSTMP_A)
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> b/drivers/gpu/drm/i915/intel_device_info.c
> index db03d179fc85..78bf7374fbdd 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -329,6 +329,108 @@ static void broadwell_sseu_info_init(struct
> drm_i915_private *dev_priv)
>  	sseu->has_eu_pg = 0;
>  }
>  
> +static u64 read_reference_ts_freq(struct drm_i915_private *dev_priv)
> +{
> +	u32 ts_override = I915_READ(GEN9_TIMESTAMP_OVERRIDE);
> +	u64 base_freq, frac_freq;
> +
> +	base_freq = ((ts_override &
> GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_MASK) >>
> +		     GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_SHIF
> T) + 1;
> +	base_freq *= 1000000;
> +
> +	frac_freq = ((ts_override &
> +		      GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR
> _MASK) >>
> +		     GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_
> SHIFT);
> +	if (frac_freq != 0)
> +		frac_freq = 1000000 / (frac_freq + 1);
> +
> +	return base_freq + frac_freq;
> +}
> +
> +static u64 read_timestamp_frequency(struct drm_i915_private
> *dev_priv)
> +{
> +	u64 f12_5_mhz = 12500000;
> +	u64 f19_2_mhz = 19200000;
> +	u64 f24_mhz = 24000000;
> +
> +	if (INTEL_GEN(dev_priv) <= 4) {
> +		/* PRMs say:
> +		 *
> +		 *     "The value in this register increments once
> every 16
> +		 *      hclks." (through the “Clocking
> Configuration”
> +		 *      (“CLKCFG”) MCHBAR register)
> +		 */
> +		return (dev_priv->rawclk_freq * 1000) / 16;
> +	} else if (INTEL_GEN(dev_priv) <= 8) {
> +		/* PRMs say:
> +		 *
> +		 *     "The PCU TSC counts 10ns increments; this
> timestamp
> +		 *      reflects bits 38:3 of the TSC (i.e. 80ns
> granularity,
> +		 *      rolling over every 1.5 hours).
> +		 */
> +		return f12_5_mhz;
> +	} else if (INTEL_GEN(dev_priv) <= 9) {
> +		u32 ctc_reg = I915_READ(CTC_MODE);
> +		u64 freq = 0;
> +
> +		if ((ctc_reg & CTC_SOURCE_PARAMETER_MASK) ==
> CTC_SOURCE_DIVIDE_LOGIC) {
> +			freq = read_reference_ts_freq(dev_priv);
> +		} else {
> +			freq = IS_GEN9_LP(dev_priv) ? f19_2_mhz :
> f24_mhz;
> +
> +			/* Now figure out how the command stream's
> timestamp
> +			 * register increments from this frequency
> (it might
> +			 * increment only every few clock cycle).
> +			 */
> +			freq >>= 3 - ((ctc_reg &
> CTC_SHIFT_PARAMETER_MASK) >>
> +				      CTC_SHIFT_PARAMETER_SHIFT);
> +		}
> +
> +		return freq;
> +	} else if (INTEL_GEN(dev_priv) <= 10) {
> +		u32 ctc_reg = I915_READ(CTC_MODE);
> +		u64 freq = 0;
> +		u32 rpm_config_reg = 0;
> +
> +		/* First figure out the reference frequency. There
> are 2 ways
> +		 * we can compute the frequency, either through the
> +		 * TIMESTAMP_OVERRIDE register or through
> RPM_CONFIG. CTC_MODE
> +		 * tells us which one we should use.
> +		 */
> +		if ((ctc_reg & CTC_SOURCE_PARAMETER_MASK) ==
> CTC_SOURCE_DIVIDE_LOGIC) {
> +			freq = read_reference_ts_freq(dev_priv);
> +		} else {
> +			u32 crystal_clock;
> +
> +			rpm_config_reg = I915_READ(RPM_CONFIG0);
> +			crystal_clock = (rpm_config_reg &
> +					 GEN9_RPM_CONFIG0_CRYSTAL_CL
> OCK_FREQ_MASK) >>
> +				GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_
> SHIFT;
> +			switch (crystal_clock) {
> +			case
> GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_19_2_MHZ:
> +				freq = f19_2_mhz;
> +				break;
> +			case
> GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_24_MHZ:
> +				freq = f24_mhz;
> +				break;
> +			}
> +		}
> +
> +		/* Now figure out how the command stream's timestamp
> register
> +		 * increments from this frequency (it might
> increment only
> +		 * every few clock cycle).
> +		 */
> +		freq >>= 3 - ((rpm_config_reg &
> +			       GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER
> _MASK) >>
> +			      GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_
> SHIFT);

I've been trying to understand this code. What's confusing to me is
that on gen 9 the frequency shifting only happens for the non-
DIVIDER_LOGIC case, while here the frequency shifting (apparently)
applies to both DIVIDER_LOGIC cases since it is outside the if
statement.

On the other hand, this code here uses "rpm_config_reg" which is 0 for
the non-DIVIDER_LOGIC case, since we only I915_READ rpm_config_reg
inside the "else" statement. Either this is a bug, or it's a really
non-trivial way of writing code and we should just do "freq >>= 3" in
the first part of the if statement and do the appropriate division in
the else case.

As a note, in our driver we generally don't zero-initialize variables
when we don't need. I know that doing zero initialization has its
advantages, but the great advantage of not zero initializing things is
that it allows GCC to catch bugs such as the one that's apparently
happening here. Another thing which we try to do is to limit variables
to the narrowest scope where they are needed, this way they can't even
be accessed outside their scope: doing this would also have helped
preventing the apparent problem we have here.

So: if this code is has a bug, we need to fix it (possibly by reading
RPM_CONFIG0 outside the if statement). If this code is actually working
as intended, we need to make it a little less contrived. I spent a long
time staring at our spec and I'm not sure what should be done.

As an additional suggestion, you could also extract the code that deals
with the "else" statement to its own function, just like you did with
read_reference_ts_freq().

Thanks,
Paulo

> +
> +		return freq;
> +	}
> +
> +	DRM_ERROR("Unknown gen, unable to compute command stream
> timestamp frequency\n");
> +	return 0;
> +}
> +
>  /*
>   * Determine various intel_device_info fields at runtime.
>   *
> @@ -450,6 +552,9 @@ void intel_device_info_runtime_init(struct
> drm_i915_private *dev_priv)
>  	else if (INTEL_GEN(dev_priv) >= 10)
>  		gen10_sseu_info_init(dev_priv);
>  
> +	/* Initialize command stream timestamp frequency */
> +	info->cs_timestamp_frequency =
> read_timestamp_frequency(dev_priv);
> +
>  	DRM_DEBUG_DRIVER("slice mask: %04x\n", info-
> >sseu.slice_mask);
>  	DRM_DEBUG_DRIVER("slice total: %u\n", hweight8(info-
> >sseu.slice_mask));
>  	DRM_DEBUG_DRIVER("subslice total: %u\n",
> @@ -465,4 +570,6 @@ void intel_device_info_runtime_init(struct
> drm_i915_private *dev_priv)
>  			 info->sseu.has_subslice_pg ? "y" : "n");
>  	DRM_DEBUG_DRIVER("has EU power gating: %s\n",
>  			 info->sseu.has_eu_pg ? "y" : "n");
> +	DRM_DEBUG_DRIVER("CS timestamp frequency: %llu\n",
> +			 info->cs_timestamp_frequency);
>  }
> diff --git a/include/uapi/drm/i915_drm.h
> b/include/uapi/drm/i915_drm.h
> index 6c02ced663f8..b57985929553 100644
> --- a/include/uapi/drm/i915_drm.h
> +++ b/include/uapi/drm/i915_drm.h
> @@ -481,6 +481,12 @@ typedef struct drm_i915_irq_wait {
>   */
>  #define I915_PARAM_HAS_CONTEXT_ISOLATION 50
>  
> +/* Frequency of the command streamer timestamps given by the
> *_TIMESTAMP
> + * registers. This used to be fixed per platform but from CNL
> onwards, this
> + * might vary depending on the parts.
> + */
> +#define I915_PARAM_CS_TIMESTAMP_FREQUENCY 51
> +
>  typedef struct drm_i915_getparam {
>  	__s32 param;
>  	/*
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 6/7] drm/i915: expose command stream timestamp frequency to userspace
  2017-12-01 20:02   ` Paulo Zanoni
@ 2017-12-01 20:43     ` Lionel Landwerlin
  0 siblings, 0 replies; 23+ messages in thread
From: Lionel Landwerlin @ 2017-12-01 20:43 UTC (permalink / raw)
  To: Paulo Zanoni, intel-gfx

On 01/12/17 20:02, Paulo Zanoni wrote:
> Em Sex, 2017-11-10 às 19:08 +0000, Lionel Landwerlin escreveu:
>> We use to have this fixed per generation, but starting with CNL
>> userspace
>> cannot tell just off the PCI ID. Let's make this information
>> available. This
>> is particularly useful for performance monitoring where much of the
>> normalization work is done using those timestamps (this include
>> pipeline
>> statistics in both GL & Vulkan as well as OA reports).
>>
>> v2: Use variables for 24MHz/19.2MHz values (Ewelina)
>>      Renamed function & coding style (Sagar)
>>
>> v3: Fix frequency read on Broadwell (Sagar)
>>      Fix missing divide by 4 on <= gen4 (Sagar)
>>
>> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
>> Tested-by: Rafael Antognolli <rafael.antognolli@intel.com>
>> ---
>>   drivers/gpu/drm/i915/i915_debugfs.c      |   2 +
>>   drivers/gpu/drm/i915/i915_drv.c          |   3 +
>>   drivers/gpu/drm/i915/i915_drv.h          |   2 +
>>   drivers/gpu/drm/i915/i915_reg.h          |  21 ++++++
>>   drivers/gpu/drm/i915/intel_device_info.c | 107
>> +++++++++++++++++++++++++++++++
>>   include/uapi/drm/i915_drm.h              |   6 ++
>>   6 files changed, 141 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>> b/drivers/gpu/drm/i915/i915_debugfs.c
>> index 533ba096b9a6..57485f29d7c9 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -3245,6 +3245,8 @@ static int i915_engine_info(struct seq_file *m,
>> void *unused)
>>   		   yesno(dev_priv->gt.awake));
>>   	seq_printf(m, "Global active requests: %d\n",
>>   		   dev_priv->gt.active_requests);
>> +	seq_printf(m, "CS timestamp frequency: %llu Hz\n",
>> +		   dev_priv->info.cs_timestamp_frequency);
>>   
>>   	p = drm_seq_file_printer(m);
>>   	for_each_engine(engine, dev_priv, id)
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c
>> b/drivers/gpu/drm/i915/i915_drv.c
>> index d97fe9c9439a..dbd04d5f75ee 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -419,6 +419,9 @@ static int i915_getparam(struct drm_device *dev,
>> void *data,
>>   		if (!value)
>>   			return -ENODEV;
>>   		break;
>> +	case I915_PARAM_CS_TIMESTAMP_FREQUENCY:
>> +		value = INTEL_INFO(dev_priv)-
>>> cs_timestamp_frequency;
>> +		break;
>>   	default:
>>   		DRM_DEBUG("Unknown parameter %d\n", param->param);
>>   		return -EINVAL;
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 9f26c34e0e52..ebc49ca58eb7 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -885,6 +885,8 @@ struct intel_device_info {
>>   	/* Slice/subslice/EU info */
>>   	struct sseu_dev_info sseu;
>>   
>> +	u64 cs_timestamp_frequency;
>> +
>>   	struct color_luts {
>>   		u16 degamma_lut_size;
>>   		u16 gamma_lut_size;
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h
>> b/drivers/gpu/drm/i915/i915_reg.h
>> index 7aa7dc0c336b..ec9faa11e305 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -1119,9 +1119,24 @@ static inline bool
>> i915_mmio_reg_valid(i915_reg_t reg)
>>   
>>   /* RPM unit config (Gen8+) */
>>   #define RPM_CONFIG0	    _MMIO(0x0D00)
>> +#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT	3
>> +#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_MASK	(1 <<
>> GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT)
>> +#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_19_2_MHZ	0
>> +#define  GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_24_MHZ	1
>> +#define  GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_SHIFT	1
>> +#define  GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_MASK	(0x3 <<
>> GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_SHIFT)
>> +
>>   #define RPM_CONFIG1	    _MMIO(0x0D04)
>>   #define  GEN10_GT_NOA_ENABLE  (1 << 9)
>>   
>> +/* GPM unit config (Gen9+) */
>> +#define CTC_MODE			_MMIO(0xA26C)
>> +#define  CTC_SOURCE_PARAMETER_MASK 1
>> +#define  CTC_SOURCE_CRYSTAL_CLOCK	0
>> +#define  CTC_SOURCE_DIVIDE_LOGIC	1
>> +#define  CTC_SHIFT_PARAMETER_SHIFT	1
>> +#define  CTC_SHIFT_PARAMETER_MASK	(0x3 <<
>> CTC_SHIFT_PARAMETER_SHIFT)
>> +
>>   /* RCP unit config (Gen8+) */
>>   #define RCP_CONFIG	    _MMIO(0x0D08)
>>   
>> @@ -8866,6 +8881,12 @@ enum skl_power_gate {
>>   #define ILK_TIMESTAMP_HI	_MMIO(0x70070)
>>   #define IVB_TIMESTAMP_CTR	_MMIO(0x44070)
>>   
>> +#define GEN9_TIMESTAMP_OVERRIDE				_MMIO
>> (0x44074)
>> +#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_SHIFT	0
>> +#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_MASK	0x3f
>> f
>> +#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_SHIFT	
>> 12
>> +#define  GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_MASK	
>> (0xf << 12)
>> +
>>   #define _PIPE_FRMTMSTMP_A		0x70048
>>   #define PIPE_FRMTMSTMP(pipe)		\
>>   			_MMIO_PIPE2(pipe, _PIPE_FRMTMSTMP_A)
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.c
>> b/drivers/gpu/drm/i915/intel_device_info.c
>> index db03d179fc85..78bf7374fbdd 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.c
>> +++ b/drivers/gpu/drm/i915/intel_device_info.c
>> @@ -329,6 +329,108 @@ static void broadwell_sseu_info_init(struct
>> drm_i915_private *dev_priv)
>>   	sseu->has_eu_pg = 0;
>>   }
>>   
>> +static u64 read_reference_ts_freq(struct drm_i915_private *dev_priv)
>> +{
>> +	u32 ts_override = I915_READ(GEN9_TIMESTAMP_OVERRIDE);
>> +	u64 base_freq, frac_freq;
>> +
>> +	base_freq = ((ts_override &
>> GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_MASK) >>
>> +		     GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_SHIF
>> T) + 1;
>> +	base_freq *= 1000000;
>> +
>> +	frac_freq = ((ts_override &
>> +		      GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR
>> _MASK) >>
>> +		     GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_
>> SHIFT);
>> +	if (frac_freq != 0)
>> +		frac_freq = 1000000 / (frac_freq + 1);
>> +
>> +	return base_freq + frac_freq;
>> +}
>> +
>> +static u64 read_timestamp_frequency(struct drm_i915_private
>> *dev_priv)
>> +{
>> +	u64 f12_5_mhz = 12500000;
>> +	u64 f19_2_mhz = 19200000;
>> +	u64 f24_mhz = 24000000;
>> +
>> +	if (INTEL_GEN(dev_priv) <= 4) {
>> +		/* PRMs say:
>> +		 *
>> +		 *     "The value in this register increments once
>> every 16
>> +		 *      hclks." (through the “Clocking
>> Configuration”
>> +		 *      (“CLKCFG”) MCHBAR register)
>> +		 */
>> +		return (dev_priv->rawclk_freq * 1000) / 16;
>> +	} else if (INTEL_GEN(dev_priv) <= 8) {
>> +		/* PRMs say:
>> +		 *
>> +		 *     "The PCU TSC counts 10ns increments; this
>> timestamp
>> +		 *      reflects bits 38:3 of the TSC (i.e. 80ns
>> granularity,
>> +		 *      rolling over every 1.5 hours).
>> +		 */
>> +		return f12_5_mhz;
>> +	} else if (INTEL_GEN(dev_priv) <= 9) {
>> +		u32 ctc_reg = I915_READ(CTC_MODE);
>> +		u64 freq = 0;
>> +
>> +		if ((ctc_reg & CTC_SOURCE_PARAMETER_MASK) ==
>> CTC_SOURCE_DIVIDE_LOGIC) {
>> +			freq = read_reference_ts_freq(dev_priv);
>> +		} else {
>> +			freq = IS_GEN9_LP(dev_priv) ? f19_2_mhz :
>> f24_mhz;
>> +
>> +			/* Now figure out how the command stream's
>> timestamp
>> +			 * register increments from this frequency
>> (it might
>> +			 * increment only every few clock cycle).
>> +			 */
>> +			freq >>= 3 - ((ctc_reg &
>> CTC_SHIFT_PARAMETER_MASK) >>
>> +				      CTC_SHIFT_PARAMETER_SHIFT);
>> +		}
>> +
>> +		return freq;
>> +	} else if (INTEL_GEN(dev_priv) <= 10) {
>> +		u32 ctc_reg = I915_READ(CTC_MODE);
>> +		u64 freq = 0;
>> +		u32 rpm_config_reg = 0;
>> +
>> +		/* First figure out the reference frequency. There
>> are 2 ways
>> +		 * we can compute the frequency, either through the
>> +		 * TIMESTAMP_OVERRIDE register or through
>> RPM_CONFIG. CTC_MODE
>> +		 * tells us which one we should use.
>> +		 */
>> +		if ((ctc_reg & CTC_SOURCE_PARAMETER_MASK) ==
>> CTC_SOURCE_DIVIDE_LOGIC) {
>> +			freq = read_reference_ts_freq(dev_priv);
>> +		} else {
>> +			u32 crystal_clock;
>> +
>> +			rpm_config_reg = I915_READ(RPM_CONFIG0);
>> +			crystal_clock = (rpm_config_reg &
>> +					 GEN9_RPM_CONFIG0_CRYSTAL_CL
>> OCK_FREQ_MASK) >>
>> +				GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_
>> SHIFT;
>> +			switch (crystal_clock) {
>> +			case
>> GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_19_2_MHZ:
>> +				freq = f19_2_mhz;
>> +				break;
>> +			case
>> GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_24_MHZ:
>> +				freq = f24_mhz;
>> +				break;
>> +			}
>> +		}
>> +
>> +		/* Now figure out how the command stream's timestamp
>> register
>> +		 * increments from this frequency (it might
>> increment only
>> +		 * every few clock cycle).
>> +		 */
>> +		freq >>= 3 - ((rpm_config_reg &
>> +			       GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER
>> _MASK) >>
>> +			      GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETER_
>> SHIFT);
> I've been trying to understand this code. What's confusing to me is
> that on gen 9 the frequency shifting only happens for the non-
> DIVIDER_LOGIC case, while here the frequency shifting (apparently)
> applies to both DIVIDER_LOGIC cases since it is outside the if
> statement.
>
> On the other hand, this code here uses "rpm_config_reg" which is 0 for
> the non-DIVIDER_LOGIC case, since we only I915_READ rpm_config_reg
> inside the "else" statement. Either this is a bug, or it's a really
> non-trivial way of writing code and we should just do "freq >>= 3" in
> the first part of the if statement and do the appropriate division in
> the else case.

Hi Paulo,

You're right, this is wrong.
The fix is here : https://patchwork.freedesktop.org/patch/188096/

>
> As a note, in our driver we generally don't zero-initialize variables
> when we don't need. I know that doing zero initialization has its
> advantages, but the great advantage of not zero initializing things is
> that it allows GCC to catch bugs such as the one that's apparently
> happening here. Another thing which we try to do is to limit variables
> to the narrowest scope where they are needed, this way they can't even
> be accessed outside their scope: doing this would also have helped
> preventing the apparent problem we have here.
>
> So: if this code is has a bug, we need to fix it (possibly by reading
> RPM_CONFIG0 outside the if statement). If this code is actually working
> as intended, we need to make it a little less contrived. I spent a long
> time staring at our spec and I'm not sure what should be done.
>
> As an additional suggestion, you could also extract the code that deals
> with the "else" statement to its own function, just like you did with
> read_reference_ts_freq().
>
> Thanks,
> Paulo
>
>> +
>> +		return freq;
>> +	}
>> +
>> +	DRM_ERROR("Unknown gen, unable to compute command stream
>> timestamp frequency\n");
>> +	return 0;
>> +}
>> +
>>   /*
>>    * Determine various intel_device_info fields at runtime.
>>    *
>> @@ -450,6 +552,9 @@ void intel_device_info_runtime_init(struct
>> drm_i915_private *dev_priv)
>>   	else if (INTEL_GEN(dev_priv) >= 10)
>>   		gen10_sseu_info_init(dev_priv);
>>   
>> +	/* Initialize command stream timestamp frequency */
>> +	info->cs_timestamp_frequency =
>> read_timestamp_frequency(dev_priv);
>> +
>>   	DRM_DEBUG_DRIVER("slice mask: %04x\n", info-
>>> sseu.slice_mask);
>>   	DRM_DEBUG_DRIVER("slice total: %u\n", hweight8(info-
>>> sseu.slice_mask));
>>   	DRM_DEBUG_DRIVER("subslice total: %u\n",
>> @@ -465,4 +570,6 @@ void intel_device_info_runtime_init(struct
>> drm_i915_private *dev_priv)
>>   			 info->sseu.has_subslice_pg ? "y" : "n");
>>   	DRM_DEBUG_DRIVER("has EU power gating: %s\n",
>>   			 info->sseu.has_eu_pg ? "y" : "n");
>> +	DRM_DEBUG_DRIVER("CS timestamp frequency: %llu\n",
>> +			 info->cs_timestamp_frequency);
>>   }
>> diff --git a/include/uapi/drm/i915_drm.h
>> b/include/uapi/drm/i915_drm.h
>> index 6c02ced663f8..b57985929553 100644
>> --- a/include/uapi/drm/i915_drm.h
>> +++ b/include/uapi/drm/i915_drm.h
>> @@ -481,6 +481,12 @@ typedef struct drm_i915_irq_wait {
>>    */
>>   #define I915_PARAM_HAS_CONTEXT_ISOLATION 50
>>   
>> +/* Frequency of the command streamer timestamps given by the
>> *_TIMESTAMP
>> + * registers. This used to be fixed per platform but from CNL
>> onwards, this
>> + * might vary depending on the parts.
>> + */
>> +#define I915_PARAM_CS_TIMESTAMP_FREQUENCY 51
>> +
>>   typedef struct drm_i915_getparam {
>>   	__s32 param;
>>   	/*


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

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

end of thread, other threads:[~2017-12-01 20:43 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-10 19:08 [PATCH 0/7] drm/i915: Perf updates Lionel Landwerlin
2017-11-10 19:08 ` [PATCH 1/7] drm/i915/perf: complete whitelisting for OA programming on HSW Lionel Landwerlin
2017-11-13  7:15   ` Ewelina Musial
2017-11-13 11:40   ` Chris Wilson
2017-11-13 11:43     ` Lionel Landwerlin
2017-11-10 19:08 ` [PATCH 2/7] drm/i915/perf: add support for Coffeelake GT3 Lionel Landwerlin
2017-11-13  7:18   ` Ewelina Musial
2017-11-10 19:08 ` [PATCH 3/7] drm/i915/perf: refactor perf setup Lionel Landwerlin
2017-11-10 19:08 ` [PATCH 4/7] drm/i915: fix register naming Lionel Landwerlin
2017-11-10 19:44   ` Matthew Auld
2017-11-13  7:28   ` Ewelina Musial
2017-11-10 19:08 ` [PATCH 5/7] drm/i915/perf: enable perf support on CNL Lionel Landwerlin
2017-11-10 19:08 ` [PATCH 6/7] drm/i915: expose command stream timestamp frequency to userspace Lionel Landwerlin
2017-11-10 20:58   ` Chris Wilson
2017-11-13  9:15   ` Sagar Arun Kamble
2017-12-01 20:02   ` Paulo Zanoni
2017-12-01 20:43     ` Lionel Landwerlin
2017-11-10 19:08 ` [PATCH 7/7] drm/i915/perf: reuse timestamp frequency from device info Lionel Landwerlin
2017-11-10 21:00   ` Chris Wilson
2017-11-13 15:19     ` Lionel Landwerlin
2017-11-13 15:28       ` Chris Wilson
2017-11-10 19:57 ` ✓ Fi.CI.BAT: success for drm/i915: Perf updates Patchwork
2017-11-10 21:35 ` ✓ Fi.CI.IGT: " Patchwork

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.