All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/4] drm/i915: some perf cleanups (& fixes!)
@ 2017-11-13 23:34 Lionel Landwerlin
  2017-11-13 23:34 ` [PATCH v3 1/4] drm/i915/perf: replace .reg accesses with i915_mmio_reg_offset Lionel Landwerlin
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Lionel Landwerlin @ 2017-11-13 23:34 UTC (permalink / raw)
  To: intel-gfx

Some sensible changes pointed by Chris, Matt & Ville.

Thanks!

Lionel Landwerlin (4):
  drm/i915/perf: replace .reg accesses with i915_mmio_reg_offset
  drm/i915: fix 64bit divide
  drm/i915/perf: reuse timestamp frequency from device info
  drm/i915/cnl: only divide up base frequency with crystal source

 drivers/gpu/drm/i915/i915_debugfs.c      |  4 +-
 drivers/gpu/drm/i915/i915_drv.c          |  9 ++--
 drivers/gpu/drm/i915/i915_drv.h          |  3 +-
 drivers/gpu/drm/i915/i915_perf.c         | 75 ++++++++++++--------------------
 drivers/gpu/drm/i915/intel_device_info.c | 45 ++++++++++---------
 5 files changed, 59 insertions(+), 77 deletions(-)

--
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] 12+ messages in thread

* [PATCH v3 1/4] drm/i915/perf: replace .reg accesses with i915_mmio_reg_offset
  2017-11-13 23:34 [PATCH v3 0/4] drm/i915: some perf cleanups (& fixes!) Lionel Landwerlin
@ 2017-11-13 23:34 ` Lionel Landwerlin
  2017-11-14  7:26   ` Ewelina Musial
  2017-11-13 23:34 ` [PATCH v3 2/4] drm/i915: fix 64bit divide Lionel Landwerlin
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Lionel Landwerlin @ 2017-11-13 23:34 UTC (permalink / raw)
  To: intel-gfx

This replaces accesses to the reg field of the i915_reg_t structure
with the i915_mmio_reg_offset() inline function.

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/i915_perf.c | 39 ++++++++++++++++++++++++---------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 00be015e01df..0f48e666098d 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -3007,7 +3007,7 @@ static bool gen8_is_valid_flex_addr(struct drm_i915_private *dev_priv, u32 addr)
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(flex_eu_regs); i++) {
-		if (flex_eu_regs[i].reg == addr)
+		if (i915_mmio_reg_offset(flex_eu_regs[i]) == addr)
 			return true;
 	}
 	return false;
@@ -3015,38 +3015,47 @@ static bool gen8_is_valid_flex_addr(struct drm_i915_private *dev_priv, u32 addr)
 
 static bool gen7_is_valid_b_counter_addr(struct drm_i915_private *dev_priv, u32 addr)
 {
-	return (addr >= OASTARTTRIG1.reg && addr <= OASTARTTRIG8.reg) ||
-		(addr >= OAREPORTTRIG1.reg && addr <= OAREPORTTRIG8.reg) ||
-		(addr >= OACEC0_0.reg && addr <= OACEC7_1.reg);
+	return (addr >= i915_mmio_reg_offset(OASTARTTRIG1) &&
+		addr <= i915_mmio_reg_offset(OASTARTTRIG8)) ||
+		(addr >= i915_mmio_reg_offset(OAREPORTTRIG1) &&
+		 addr <= i915_mmio_reg_offset(OAREPORTTRIG8)) ||
+		(addr >= i915_mmio_reg_offset(OACEC0_0) &&
+		 addr <= i915_mmio_reg_offset(OACEC7_1));
 }
 
 static bool gen7_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 addr)
 {
-	return addr == HALF_SLICE_CHICKEN2.reg ||
-		(addr >= MICRO_BP0_0.reg && addr <= NOA_WRITE.reg) ||
-		(addr >= OA_PERFCNT1_LO.reg && addr <= OA_PERFCNT2_HI.reg) ||
-		(addr >= OA_PERFMATRIX_LO.reg && addr <= OA_PERFMATRIX_HI.reg);
+	return addr == i915_mmio_reg_offset(HALF_SLICE_CHICKEN2) ||
+		(addr >= i915_mmio_reg_offset(MICRO_BP0_0) &&
+		 addr <= i915_mmio_reg_offset(NOA_WRITE)) ||
+		(addr >= i915_mmio_reg_offset(OA_PERFCNT1_LO) &&
+		 addr <= i915_mmio_reg_offset(OA_PERFCNT2_HI)) ||
+		(addr >= i915_mmio_reg_offset(OA_PERFMATRIX_LO) &&
+		 addr <= i915_mmio_reg_offset(OA_PERFMATRIX_HI));
 }
 
 static bool gen8_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 addr)
 {
 	return gen7_is_valid_mux_addr(dev_priv, addr) ||
-		addr == WAIT_FOR_RC6_EXIT.reg ||
-		(addr >= RPM_CONFIG0.reg && addr <= NOA_CONFIG(8).reg);
+		addr == i915_mmio_reg_offset(WAIT_FOR_RC6_EXIT) ||
+		(addr >= i915_mmio_reg_offset(RPM_CONFIG0) &&
+		 addr <= i915_mmio_reg_offset(NOA_CONFIG(8)));
 }
 
 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);
+		(addr >= i915_mmio_reg_offset(OA_PERFCNT3_LO) &&
+		 addr <= i915_mmio_reg_offset(OA_PERFCNT4_HI));
 }
 
 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 >= HSW_MBVID2_NOA0.reg && addr <= HSW_MBVID2_NOA9.reg) ||
-		addr == HSW_MBVID2_MISR0.reg;
+		(addr >= i915_mmio_reg_offset(HSW_MBVID2_NOA0) &&
+		 addr <= i915_mmio_reg_offset(HSW_MBVID2_NOA9)) ||
+		addr == i915_mmio_reg_offset(HSW_MBVID2_MISR0);
 }
 
 static bool chv_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 addr)
@@ -3061,14 +3070,14 @@ static uint32_t mask_reg_value(u32 reg, u32 val)
 	 * WaDisableSTUnitPowerOptimization workaround. Make sure the value
 	 * programmed by userspace doesn't change this.
 	 */
-	if (HALF_SLICE_CHICKEN2.reg == reg)
+	if (i915_mmio_reg_offset(HALF_SLICE_CHICKEN2) == reg)
 		val = val & ~_MASKED_BIT_ENABLE(GEN8_ST_PO_DISABLE);
 
 	/* WAIT_FOR_RC6_EXIT has only one bit fullfilling the function
 	 * indicated by its name and a bunch of selection fields used by OA
 	 * configs.
 	 */
-	if (WAIT_FOR_RC6_EXIT.reg == reg)
+	if (i915_mmio_reg_offset(WAIT_FOR_RC6_EXIT) == reg)
 		val = val & ~_MASKED_BIT_ENABLE(HSW_WAIT_FOR_RC6_EXIT_ENABLE);
 
 	return val;
-- 
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] 12+ messages in thread

* [PATCH v3 2/4] drm/i915: fix 64bit divide
  2017-11-13 23:34 [PATCH v3 0/4] drm/i915: some perf cleanups (& fixes!) Lionel Landwerlin
  2017-11-13 23:34 ` [PATCH v3 1/4] drm/i915/perf: replace .reg accesses with i915_mmio_reg_offset Lionel Landwerlin
@ 2017-11-13 23:34 ` Lionel Landwerlin
  2017-11-14  7:30   ` Ewelina Musial
  2017-11-13 23:34 ` [PATCH v3 3/4] drm/i915/perf: reuse timestamp frequency from device info Lionel Landwerlin
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Lionel Landwerlin @ 2017-11-13 23:34 UTC (permalink / raw)
  To: intel-gfx

ERROR: "__udivdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
ERROR: "__divdi3" [drivers/gpu/drm/i915/i915.ko] undefined!

Store the frequency in kHz and drop 64bit divisions.

v2: Use div64_u64 (Matthew)

v3: store frequency in kHz to avoid 64bit divs (Chris/Ville)

Fixes: dab9178333 ("drm/i915: expose command stream timestamp frequency to userspace")
Reported-by: Matthew Auld <matthew.auld@intel.com>
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c      |  4 ++--
 drivers/gpu/drm/i915/i915_drv.c          |  2 +-
 drivers/gpu/drm/i915/i915_drv.h          |  2 +-
 drivers/gpu/drm/i915/intel_device_info.c | 29 ++++++++++++++---------------
 4 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 82ff186108f1..6ba08b0c1c22 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3269,8 +3269,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);
+	seq_printf(m, "CS timestamp frequency: %u kHz\n",
+		   dev_priv->info.cs_timestamp_frequency_khz);
 
 	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 42813f4247e2..171b86f37878 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -420,7 +420,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
 			return -ENODEV;
 		break;
 	case I915_PARAM_CS_TIMESTAMP_FREQUENCY:
-		value = INTEL_INFO(dev_priv)->cs_timestamp_frequency;
+		value = 1000 * INTEL_INFO(dev_priv)->cs_timestamp_frequency_khz;
 		break;
 	default:
 		DRM_DEBUG("Unknown parameter %d\n", param->param);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b538df740ac3..2158a758a17d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -885,7 +885,7 @@ struct intel_device_info {
 	/* Slice/subslice/EU info */
 	struct sseu_dev_info sseu;
 
-	u64 cs_timestamp_frequency;
+	u32 cs_timestamp_frequency_khz;
 
 	struct color_luts {
 		u16 degamma_lut_size;
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 78bf7374fbdd..f3e4940fed49 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -329,29 +329,28 @@ 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)
+static u32 read_reference_ts_freq(struct drm_i915_private *dev_priv)
 {
 	u32 ts_override = I915_READ(GEN9_TIMESTAMP_OVERRIDE);
-	u64 base_freq, frac_freq;
+	u32 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;
+	base_freq *= 1000;
 
 	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);
+	frac_freq = 1000 / (frac_freq + 1);
 
 	return base_freq + frac_freq;
 }
 
-static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
+static u32 read_timestamp_frequency(struct drm_i915_private *dev_priv)
 {
-	u64 f12_5_mhz = 12500000;
-	u64 f19_2_mhz = 19200000;
-	u64 f24_mhz = 24000000;
+	u32 f12_5_mhz = 12500;
+	u32 f19_2_mhz = 19200;
+	u32 f24_mhz = 24000;
 
 	if (INTEL_GEN(dev_priv) <= 4) {
 		/* PRMs say:
@@ -360,7 +359,7 @@ static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
 		 *      hclks." (through the “Clocking Configuration”
 		 *      (“CLKCFG”) MCHBAR register)
 		 */
-		return (dev_priv->rawclk_freq * 1000) / 16;
+		return dev_priv->rawclk_freq / 16;
 	} else if (INTEL_GEN(dev_priv) <= 8) {
 		/* PRMs say:
 		 *
@@ -371,7 +370,7 @@ static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
 		return f12_5_mhz;
 	} else if (INTEL_GEN(dev_priv) <= 9) {
 		u32 ctc_reg = I915_READ(CTC_MODE);
-		u64 freq = 0;
+		u32 freq = 0;
 
 		if ((ctc_reg & CTC_SOURCE_PARAMETER_MASK) == CTC_SOURCE_DIVIDE_LOGIC) {
 			freq = read_reference_ts_freq(dev_priv);
@@ -389,7 +388,7 @@ static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
 		return freq;
 	} else if (INTEL_GEN(dev_priv) <= 10) {
 		u32 ctc_reg = I915_READ(CTC_MODE);
-		u64 freq = 0;
+		u32 freq = 0;
 		u32 rpm_config_reg = 0;
 
 		/* First figure out the reference frequency. There are 2 ways
@@ -553,7 +552,7 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
 		gen10_sseu_info_init(dev_priv);
 
 	/* Initialize command stream timestamp frequency */
-	info->cs_timestamp_frequency = read_timestamp_frequency(dev_priv);
+	info->cs_timestamp_frequency_khz = 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));
@@ -570,6 +569,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);
+	DRM_DEBUG_DRIVER("CS timestamp frequency: %u kHz\n",
+			 info->cs_timestamp_frequency_khz);
 }
-- 
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] 12+ messages in thread

* [PATCH v3 3/4] drm/i915/perf: reuse timestamp frequency from device info
  2017-11-13 23:34 [PATCH v3 0/4] drm/i915: some perf cleanups (& fixes!) Lionel Landwerlin
  2017-11-13 23:34 ` [PATCH v3 1/4] drm/i915/perf: replace .reg accesses with i915_mmio_reg_offset Lionel Landwerlin
  2017-11-13 23:34 ` [PATCH v3 2/4] drm/i915: fix 64bit divide Lionel Landwerlin
@ 2017-11-13 23:34 ` Lionel Landwerlin
  2017-11-13 23:34 ` [PATCH v3 4/4] drm/i915/cnl: only divide up base frequency with crystal source Lionel Landwerlin
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Lionel Landwerlin @ 2017-11-13 23:34 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().

v2: Use div_u64 (Chris)

v3: Drop u64 divs by switching to kHz (Chris/Ville)
    Move i915_perf_fini to i915_driver_cleanup_hw (Matthew)

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

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 171b86f37878..b017bdbd1c21 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -931,8 +931,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:
@@ -949,7 +947,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv,
  */
 static void i915_driver_cleanup_early(struct drm_i915_private *dev_priv)
 {
-	i915_perf_fini(dev_priv);
 	i915_gem_load_cleanup(dev_priv);
 	intel_irq_fini(dev_priv);
 	i915_workqueues_cleanup(dev_priv);
@@ -1096,6 +1093,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;
@@ -1201,6 +1200,8 @@ static void i915_driver_cleanup_hw(struct drm_i915_private *dev_priv)
 {
 	struct pci_dev *pdev = dev_priv->drm.pdev;
 
+	i915_perf_fini(dev_priv);
+
 	if (pdev->msi_enabled)
 		pci_disable_msi(pdev);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2158a758a17d..13bad5bc1b9f 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2619,7 +2619,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 0f48e666098d..fcfd3edb2979 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -2691,8 +2691,8 @@ 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);
+	return div64_u64(1000000000ULL * (2ULL << exponent),
+			 1000ULL * INTEL_INFO(dev_priv)->cs_timestamp_frequency_khz);
 }
 
 /**
@@ -3424,8 +3424,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;
@@ -3441,8 +3439,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
@@ -3486,23 +3482,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;
@@ -3518,15 +3497,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;
@@ -3536,8 +3510,8 @@ void i915_perf_init(struct drm_i915_private *dev_priv)
 		mutex_init(&dev_priv->perf.lock);
 		spin_lock_init(&dev_priv->perf.oa.oa_buffer.ptr_lock);
 
-		oa_sample_rate_hard_limit =
-			dev_priv->perf.oa.timestamp_frequency / 2;
+		oa_sample_rate_hard_limit = 1000 *
+			(INTEL_INFO(dev_priv)->cs_timestamp_frequency_khz / 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] 12+ messages in thread

* [PATCH v3 4/4] drm/i915/cnl: only divide up base frequency with crystal source
  2017-11-13 23:34 [PATCH v3 0/4] drm/i915: some perf cleanups (& fixes!) Lionel Landwerlin
                   ` (2 preceding siblings ...)
  2017-11-13 23:34 ` [PATCH v3 3/4] drm/i915/perf: reuse timestamp frequency from device info Lionel Landwerlin
@ 2017-11-13 23:34 ` Lionel Landwerlin
  2017-12-04 17:50   ` Paulo Zanoni
  2017-11-14  0:10 ` ✓ Fi.CI.BAT: success for drm/i915: some perf cleanups (& fixes!) (rev2) Patchwork
  2017-11-14  0:53 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 1 reply; 12+ messages in thread
From: Lionel Landwerlin @ 2017-11-13 23:34 UTC (permalink / raw)
  To: intel-gfx

We apply this logic to Gen9 as well. We didn't notice this issue as
most part we've encountered so far only use the crystal as source for
their timestamp registers.

Fixes: dab9178333 ("drm/i915: expose command stream timestamp frequency to userspace")
Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
---
 drivers/gpu/drm/i915/intel_device_info.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index f3e4940fed49..039f8ec7ad27 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -413,15 +413,15 @@ static u32 read_timestamp_frequency(struct drm_i915_private *dev_priv)
 				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);
+			/* 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;
 	}
-- 
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] 12+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: some perf cleanups (& fixes!) (rev2)
  2017-11-13 23:34 [PATCH v3 0/4] drm/i915: some perf cleanups (& fixes!) Lionel Landwerlin
                   ` (3 preceding siblings ...)
  2017-11-13 23:34 ` [PATCH v3 4/4] drm/i915/cnl: only divide up base frequency with crystal source Lionel Landwerlin
@ 2017-11-14  0:10 ` Patchwork
  2017-11-14  0:53 ` ✓ Fi.CI.IGT: " Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2017-11-14  0:10 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: some perf cleanups (& fixes!) (rev2)
URL   : https://patchwork.freedesktop.org/series/33736/
State : success

== Summary ==

Series 33736v2 drm/i915: some perf cleanups (& fixes!)
https://patchwork.freedesktop.org/api/1.0/series/33736/revisions/2/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                fail       -> PASS       (fi-kbl-7500u) fdo#102514
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> INCOMPLETE (fi-snb-2520m) fdo#103713
Test vgem_basic:
        Subgroup dmabuf-export:
                pass       -> INCOMPLETE (fi-byt-n2820) fdo#103714
                incomplete -> PASS       (fi-bxt-j4205) fdo#103706 +1
        Subgroup unload:
                notrun     -> INCOMPLETE (fi-bxt-j4205) fdo#103702 +1

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#103713 https://bugs.freedesktop.org/show_bug.cgi?id=103713
fdo#103714 https://bugs.freedesktop.org/show_bug.cgi?id=103714
fdo#103706 https://bugs.freedesktop.org/show_bug.cgi?id=103706
fdo#103702 https://bugs.freedesktop.org/show_bug.cgi?id=103702

fi-bdw-5557u     total:285  pass:263  dwarn:0   dfail:1   fail:0   skip:20 
fi-bdw-gvtdvm    total:285  pass:260  dwarn:0   dfail:1   fail:0   skip:23 
fi-blb-e6850     total:285  pass:218  dwarn:1   dfail:1   fail:0   skip:64 
fi-bsw-n3050     total:285  pass:238  dwarn:0   dfail:1   fail:0   skip:45 
fi-bwr-2160      total:285  pass:178  dwarn:0   dfail:1   fail:0   skip:105
fi-bxt-dsi       total:285  pass:254  dwarn:0   dfail:1   fail:0   skip:29 
fi-bxt-j4205     total:285  pass:255  dwarn:0   dfail:1   fail:0   skip:28 
fi-byt-j1900     total:285  pass:249  dwarn:0   dfail:1   fail:0   skip:34 
fi-byt-n2820     total:278  pass:238  dwarn:0   dfail:1   fail:0   skip:38 
fi-elk-e7500     total:285  pass:224  dwarn:0   dfail:1   fail:0   skip:59 
fi-gdg-551       total:285  pass:174  dwarn:0   dfail:1   fail:1   skip:108
fi-glk-1         total:285  pass:256  dwarn:0   dfail:1   fail:0   skip:27 
fi-hsw-4770      total:285  pass:257  dwarn:0   dfail:1   fail:0   skip:26 
fi-hsw-4770r     total:285  pass:257  dwarn:0   dfail:1   fail:0   skip:26 
fi-ilk-650       total:285  pass:223  dwarn:0   dfail:1   fail:0   skip:60 
fi-ivb-3520m     total:285  pass:255  dwarn:0   dfail:1   fail:0   skip:28 
fi-ivb-3770      total:285  pass:255  dwarn:0   dfail:1   fail:0   skip:28 
fi-kbl-7500u     total:285  pass:259  dwarn:1   dfail:1   fail:0   skip:23 
fi-kbl-7560u     total:285  pass:265  dwarn:0   dfail:1   fail:0   skip:18 
fi-kbl-7567u     total:285  pass:264  dwarn:0   dfail:1   fail:0   skip:19 
fi-kbl-r         total:285  pass:257  dwarn:0   dfail:1   fail:0   skip:26 
fi-pnv-d510      total:285  pass:217  dwarn:1   dfail:1   fail:0   skip:65 
fi-skl-6260u     total:285  pass:264  dwarn:0   dfail:1   fail:0   skip:19 
fi-skl-6600u     total:285  pass:257  dwarn:0   dfail:1   fail:0   skip:26 
fi-skl-6700hq    total:285  pass:258  dwarn:0   dfail:1   fail:0   skip:25 
fi-skl-6700k     total:285  pass:260  dwarn:0   dfail:1   fail:0   skip:23 
fi-skl-6770hq    total:285  pass:264  dwarn:0   dfail:1   fail:0   skip:19 
fi-skl-gvtdvm    total:285  pass:261  dwarn:0   dfail:1   fail:0   skip:22 
fi-snb-2520m     total:246  pass:212  dwarn:0   dfail:0   fail:0   skip:33 
fi-snb-2600      total:285  pass:244  dwarn:0   dfail:1   fail:0   skip:39 
Blacklisted hosts:
fi-cfl-s         total:285  pass:252  dwarn:0   dfail:1   fail:0   skip:31 
fi-cnl-y failed to connect after reboot

0ae838346d57d7767dec9e25f9d534faaa59ea7f drm-tip: 2017y-11m-13d-20h-43m-56s UTC integration manifest
17d6c21606da drm/i915/cnl: only divide up base frequency with crystal source
a8abf6e2fea0 drm/i915/perf: reuse timestamp frequency from device info
6628d32f1594 drm/i915: fix 64bit divide
7276de652763 drm/i915/perf: replace .reg accesses with i915_mmio_reg_offset

== Logs ==

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

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

* ✓ Fi.CI.IGT: success for drm/i915: some perf cleanups (& fixes!) (rev2)
  2017-11-13 23:34 [PATCH v3 0/4] drm/i915: some perf cleanups (& fixes!) Lionel Landwerlin
                   ` (4 preceding siblings ...)
  2017-11-14  0:10 ` ✓ Fi.CI.BAT: success for drm/i915: some perf cleanups (& fixes!) (rev2) Patchwork
@ 2017-11-14  0:53 ` Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2017-11-14  0:53 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: some perf cleanups (& fixes!) (rev2)
URL   : https://patchwork.freedesktop.org/series/33736/
State : success

== Summary ==

Test kms_busy:
        Subgroup extended-modeset-hang-oldfb-with-reset-render-a:
                pass       -> DMESG-WARN (shard-hsw) fdo#102249
Test kms_flip:
        Subgroup plain-flip-ts-check-interruptible:
                fail       -> PASS       (shard-hsw) fdo#100368
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912

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

shard-hsw        total:2550 pass:1457 dwarn:2   dfail:1   fail:10  skip:1079 time:9157s
Blacklisted hosts:
shard-apl        total:2561 pass:1601 dwarn:1   dfail:1   fail:23  skip:933 time:12553s
shard-kbl        total:2565 pass:1685 dwarn:25  dfail:2   fail:25  skip:827 time:10569s
shard-snb        total:2584 pass:1216 dwarn:2   dfail:2   fail:13  skip:1351 time:7716s

== Logs ==

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

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

* Re: [PATCH v3 1/4] drm/i915/perf: replace .reg accesses with i915_mmio_reg_offset
  2017-11-13 23:34 ` [PATCH v3 1/4] drm/i915/perf: replace .reg accesses with i915_mmio_reg_offset Lionel Landwerlin
@ 2017-11-14  7:26   ` Ewelina Musial
  0 siblings, 0 replies; 12+ messages in thread
From: Ewelina Musial @ 2017-11-14  7:26 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

On Mon, Nov 13, 2017 at 11:34:52PM +0000, Lionel Landwerlin wrote:
> This replaces accesses to the reg field of the i915_reg_t structure
> with the i915_mmio_reg_offset() inline function.
> 
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Ewelina Musial <ewelina.musial@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_perf.c | 39 ++++++++++++++++++++++++---------------
>  1 file changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 00be015e01df..0f48e666098d 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -3007,7 +3007,7 @@ static bool gen8_is_valid_flex_addr(struct drm_i915_private *dev_priv, u32 addr)
>  	int i;
>  
>  	for (i = 0; i < ARRAY_SIZE(flex_eu_regs); i++) {
> -		if (flex_eu_regs[i].reg == addr)
> +		if (i915_mmio_reg_offset(flex_eu_regs[i]) == addr)
>  			return true;
>  	}
>  	return false;
> @@ -3015,38 +3015,47 @@ static bool gen8_is_valid_flex_addr(struct drm_i915_private *dev_priv, u32 addr)
>  
>  static bool gen7_is_valid_b_counter_addr(struct drm_i915_private *dev_priv, u32 addr)
>  {
> -	return (addr >= OASTARTTRIG1.reg && addr <= OASTARTTRIG8.reg) ||
> -		(addr >= OAREPORTTRIG1.reg && addr <= OAREPORTTRIG8.reg) ||
> -		(addr >= OACEC0_0.reg && addr <= OACEC7_1.reg);
> +	return (addr >= i915_mmio_reg_offset(OASTARTTRIG1) &&
> +		addr <= i915_mmio_reg_offset(OASTARTTRIG8)) ||
> +		(addr >= i915_mmio_reg_offset(OAREPORTTRIG1) &&
> +		 addr <= i915_mmio_reg_offset(OAREPORTTRIG8)) ||
> +		(addr >= i915_mmio_reg_offset(OACEC0_0) &&
> +		 addr <= i915_mmio_reg_offset(OACEC7_1));
>  }
>  
>  static bool gen7_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 addr)
>  {
> -	return addr == HALF_SLICE_CHICKEN2.reg ||
> -		(addr >= MICRO_BP0_0.reg && addr <= NOA_WRITE.reg) ||
> -		(addr >= OA_PERFCNT1_LO.reg && addr <= OA_PERFCNT2_HI.reg) ||
> -		(addr >= OA_PERFMATRIX_LO.reg && addr <= OA_PERFMATRIX_HI.reg);
> +	return addr == i915_mmio_reg_offset(HALF_SLICE_CHICKEN2) ||
> +		(addr >= i915_mmio_reg_offset(MICRO_BP0_0) &&
> +		 addr <= i915_mmio_reg_offset(NOA_WRITE)) ||
> +		(addr >= i915_mmio_reg_offset(OA_PERFCNT1_LO) &&
> +		 addr <= i915_mmio_reg_offset(OA_PERFCNT2_HI)) ||
> +		(addr >= i915_mmio_reg_offset(OA_PERFMATRIX_LO) &&
> +		 addr <= i915_mmio_reg_offset(OA_PERFMATRIX_HI));
>  }
>  
>  static bool gen8_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 addr)
>  {
>  	return gen7_is_valid_mux_addr(dev_priv, addr) ||
> -		addr == WAIT_FOR_RC6_EXIT.reg ||
> -		(addr >= RPM_CONFIG0.reg && addr <= NOA_CONFIG(8).reg);
> +		addr == i915_mmio_reg_offset(WAIT_FOR_RC6_EXIT) ||
> +		(addr >= i915_mmio_reg_offset(RPM_CONFIG0) &&
> +		 addr <= i915_mmio_reg_offset(NOA_CONFIG(8)));
>  }
>  
>  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);
> +		(addr >= i915_mmio_reg_offset(OA_PERFCNT3_LO) &&
> +		 addr <= i915_mmio_reg_offset(OA_PERFCNT4_HI));
>  }
>  
>  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 >= HSW_MBVID2_NOA0.reg && addr <= HSW_MBVID2_NOA9.reg) ||
> -		addr == HSW_MBVID2_MISR0.reg;
> +		(addr >= i915_mmio_reg_offset(HSW_MBVID2_NOA0) &&
> +		 addr <= i915_mmio_reg_offset(HSW_MBVID2_NOA9)) ||
> +		addr == i915_mmio_reg_offset(HSW_MBVID2_MISR0);
>  }
>  
>  static bool chv_is_valid_mux_addr(struct drm_i915_private *dev_priv, u32 addr)
> @@ -3061,14 +3070,14 @@ static uint32_t mask_reg_value(u32 reg, u32 val)
>  	 * WaDisableSTUnitPowerOptimization workaround. Make sure the value
>  	 * programmed by userspace doesn't change this.
>  	 */
> -	if (HALF_SLICE_CHICKEN2.reg == reg)
> +	if (i915_mmio_reg_offset(HALF_SLICE_CHICKEN2) == reg)
>  		val = val & ~_MASKED_BIT_ENABLE(GEN8_ST_PO_DISABLE);
>  
>  	/* WAIT_FOR_RC6_EXIT has only one bit fullfilling the function
>  	 * indicated by its name and a bunch of selection fields used by OA
>  	 * configs.
>  	 */
> -	if (WAIT_FOR_RC6_EXIT.reg == reg)
> +	if (i915_mmio_reg_offset(WAIT_FOR_RC6_EXIT) == reg)
>  		val = val & ~_MASKED_BIT_ENABLE(HSW_WAIT_FOR_RC6_EXIT_ENABLE);
>  
>  	return val;
> -- 
> 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] 12+ messages in thread

* Re: [PATCH v3 2/4] drm/i915: fix 64bit divide
  2017-11-13 23:34 ` [PATCH v3 2/4] drm/i915: fix 64bit divide Lionel Landwerlin
@ 2017-11-14  7:30   ` Ewelina Musial
  2017-11-14 14:27     ` Ville Syrjälä
  0 siblings, 1 reply; 12+ messages in thread
From: Ewelina Musial @ 2017-11-14  7:30 UTC (permalink / raw)
  To: Lionel Landwerlin; +Cc: intel-gfx

On Mon, Nov 13, 2017 at 11:34:53PM +0000, Lionel Landwerlin wrote:
> ERROR: "__udivdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
> ERROR: "__divdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
> 
> Store the frequency in kHz and drop 64bit divisions.
> 
> v2: Use div64_u64 (Matthew)
> 
> v3: store frequency in kHz to avoid 64bit divs (Chris/Ville)
> 
> Fixes: dab9178333 ("drm/i915: expose command stream timestamp frequency to userspace")
> Reported-by: Matthew Auld <matthew.auld@intel.com>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
Reviewed-by: Ewelina Musial <ewelina.musial@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c      |  4 ++--
>  drivers/gpu/drm/i915/i915_drv.c          |  2 +-
>  drivers/gpu/drm/i915/i915_drv.h          |  2 +-
>  drivers/gpu/drm/i915/intel_device_info.c | 29 ++++++++++++++---------------
>  4 files changed, 18 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 82ff186108f1..6ba08b0c1c22 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3269,8 +3269,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);
> +	seq_printf(m, "CS timestamp frequency: %u kHz\n",
> +		   dev_priv->info.cs_timestamp_frequency_khz);
>  
>  	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 42813f4247e2..171b86f37878 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -420,7 +420,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
>  			return -ENODEV;
>  		break;
>  	case I915_PARAM_CS_TIMESTAMP_FREQUENCY:
> -		value = INTEL_INFO(dev_priv)->cs_timestamp_frequency;
> +		value = 1000 * INTEL_INFO(dev_priv)->cs_timestamp_frequency_khz;
>  		break;
>  	default:
>  		DRM_DEBUG("Unknown parameter %d\n", param->param);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b538df740ac3..2158a758a17d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -885,7 +885,7 @@ struct intel_device_info {
>  	/* Slice/subslice/EU info */
>  	struct sseu_dev_info sseu;
>  
> -	u64 cs_timestamp_frequency;
> +	u32 cs_timestamp_frequency_khz;
>  
>  	struct color_luts {
>  		u16 degamma_lut_size;
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 78bf7374fbdd..f3e4940fed49 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -329,29 +329,28 @@ 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)
> +static u32 read_reference_ts_freq(struct drm_i915_private *dev_priv)
>  {
>  	u32 ts_override = I915_READ(GEN9_TIMESTAMP_OVERRIDE);
> -	u64 base_freq, frac_freq;
> +	u32 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;
> +	base_freq *= 1000;
>  
>  	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);
> +	frac_freq = 1000 / (frac_freq + 1);
>  
>  	return base_freq + frac_freq;
>  }
>  
> -static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
> +static u32 read_timestamp_frequency(struct drm_i915_private *dev_priv)
>  {
> -	u64 f12_5_mhz = 12500000;
> -	u64 f19_2_mhz = 19200000;
> -	u64 f24_mhz = 24000000;
> +	u32 f12_5_mhz = 12500;
> +	u32 f19_2_mhz = 19200;
> +	u32 f24_mhz = 24000;
>  
>  	if (INTEL_GEN(dev_priv) <= 4) {
>  		/* PRMs say:
> @@ -360,7 +359,7 @@ static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
>  		 *      hclks." (through the “Clocking Configuration”
>  		 *      (“CLKCFG”) MCHBAR register)
>  		 */
> -		return (dev_priv->rawclk_freq * 1000) / 16;
> +		return dev_priv->rawclk_freq / 16;
>  	} else if (INTEL_GEN(dev_priv) <= 8) {
>  		/* PRMs say:
>  		 *
> @@ -371,7 +370,7 @@ static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
>  		return f12_5_mhz;
>  	} else if (INTEL_GEN(dev_priv) <= 9) {
>  		u32 ctc_reg = I915_READ(CTC_MODE);
> -		u64 freq = 0;
> +		u32 freq = 0;
>  
>  		if ((ctc_reg & CTC_SOURCE_PARAMETER_MASK) == CTC_SOURCE_DIVIDE_LOGIC) {
>  			freq = read_reference_ts_freq(dev_priv);
> @@ -389,7 +388,7 @@ static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
>  		return freq;
>  	} else if (INTEL_GEN(dev_priv) <= 10) {
>  		u32 ctc_reg = I915_READ(CTC_MODE);
> -		u64 freq = 0;
> +		u32 freq = 0;
>  		u32 rpm_config_reg = 0;
>  
>  		/* First figure out the reference frequency. There are 2 ways
> @@ -553,7 +552,7 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>  		gen10_sseu_info_init(dev_priv);
>  
>  	/* Initialize command stream timestamp frequency */
> -	info->cs_timestamp_frequency = read_timestamp_frequency(dev_priv);
> +	info->cs_timestamp_frequency_khz = 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));
> @@ -570,6 +569,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);
> +	DRM_DEBUG_DRIVER("CS timestamp frequency: %u kHz\n",
> +			 info->cs_timestamp_frequency_khz);
>  }
> -- 
> 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] 12+ messages in thread

* Re: [PATCH v3 2/4] drm/i915: fix 64bit divide
  2017-11-14  7:30   ` Ewelina Musial
@ 2017-11-14 14:27     ` Ville Syrjälä
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2017-11-14 14:27 UTC (permalink / raw)
  To: Ewelina Musial; +Cc: intel-gfx

On Tue, Nov 14, 2017 at 08:30:41AM +0100, Ewelina Musial wrote:
> On Mon, Nov 13, 2017 at 11:34:53PM +0000, Lionel Landwerlin wrote:
> > ERROR: "__udivdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
> > ERROR: "__divdi3" [drivers/gpu/drm/i915/i915.ko] undefined!
> > 
> > Store the frequency in kHz and drop 64bit divisions.
> > 
> > v2: Use div64_u64 (Matthew)
> > 
> > v3: store frequency in kHz to avoid 64bit divs (Chris/Ville)
> > 
> > Fixes: dab9178333 ("drm/i915: expose command stream timestamp frequency to userspace")
> > Reported-by: Matthew Auld <matthew.auld@intel.com>
> > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Reviewed-by: Ewelina Musial <ewelina.musial@intel.com>

Pushed to dinq. Thanks for the patch and review.

> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c      |  4 ++--
> >  drivers/gpu/drm/i915/i915_drv.c          |  2 +-
> >  drivers/gpu/drm/i915/i915_drv.h          |  2 +-
> >  drivers/gpu/drm/i915/intel_device_info.c | 29 ++++++++++++++---------------
> >  4 files changed, 18 insertions(+), 19 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index 82ff186108f1..6ba08b0c1c22 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -3269,8 +3269,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);
> > +	seq_printf(m, "CS timestamp frequency: %u kHz\n",
> > +		   dev_priv->info.cs_timestamp_frequency_khz);
> >  
> >  	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 42813f4247e2..171b86f37878 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -420,7 +420,7 @@ static int i915_getparam(struct drm_device *dev, void *data,
> >  			return -ENODEV;
> >  		break;
> >  	case I915_PARAM_CS_TIMESTAMP_FREQUENCY:
> > -		value = INTEL_INFO(dev_priv)->cs_timestamp_frequency;
> > +		value = 1000 * INTEL_INFO(dev_priv)->cs_timestamp_frequency_khz;
> >  		break;
> >  	default:
> >  		DRM_DEBUG("Unknown parameter %d\n", param->param);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index b538df740ac3..2158a758a17d 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -885,7 +885,7 @@ struct intel_device_info {
> >  	/* Slice/subslice/EU info */
> >  	struct sseu_dev_info sseu;
> >  
> > -	u64 cs_timestamp_frequency;
> > +	u32 cs_timestamp_frequency_khz;
> >  
> >  	struct color_luts {
> >  		u16 degamma_lut_size;
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> > index 78bf7374fbdd..f3e4940fed49 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > @@ -329,29 +329,28 @@ 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)
> > +static u32 read_reference_ts_freq(struct drm_i915_private *dev_priv)
> >  {
> >  	u32 ts_override = I915_READ(GEN9_TIMESTAMP_OVERRIDE);
> > -	u64 base_freq, frac_freq;
> > +	u32 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;
> > +	base_freq *= 1000;
> >  
> >  	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);
> > +	frac_freq = 1000 / (frac_freq + 1);
> >  
> >  	return base_freq + frac_freq;
> >  }
> >  
> > -static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
> > +static u32 read_timestamp_frequency(struct drm_i915_private *dev_priv)
> >  {
> > -	u64 f12_5_mhz = 12500000;
> > -	u64 f19_2_mhz = 19200000;
> > -	u64 f24_mhz = 24000000;
> > +	u32 f12_5_mhz = 12500;
> > +	u32 f19_2_mhz = 19200;
> > +	u32 f24_mhz = 24000;
> >  
> >  	if (INTEL_GEN(dev_priv) <= 4) {
> >  		/* PRMs say:
> > @@ -360,7 +359,7 @@ static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
> >  		 *      hclks." (through the “Clocking Configuration”
> >  		 *      (“CLKCFG”) MCHBAR register)
> >  		 */
> > -		return (dev_priv->rawclk_freq * 1000) / 16;
> > +		return dev_priv->rawclk_freq / 16;
> >  	} else if (INTEL_GEN(dev_priv) <= 8) {
> >  		/* PRMs say:
> >  		 *
> > @@ -371,7 +370,7 @@ static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
> >  		return f12_5_mhz;
> >  	} else if (INTEL_GEN(dev_priv) <= 9) {
> >  		u32 ctc_reg = I915_READ(CTC_MODE);
> > -		u64 freq = 0;
> > +		u32 freq = 0;
> >  
> >  		if ((ctc_reg & CTC_SOURCE_PARAMETER_MASK) == CTC_SOURCE_DIVIDE_LOGIC) {
> >  			freq = read_reference_ts_freq(dev_priv);
> > @@ -389,7 +388,7 @@ static u64 read_timestamp_frequency(struct drm_i915_private *dev_priv)
> >  		return freq;
> >  	} else if (INTEL_GEN(dev_priv) <= 10) {
> >  		u32 ctc_reg = I915_READ(CTC_MODE);
> > -		u64 freq = 0;
> > +		u32 freq = 0;
> >  		u32 rpm_config_reg = 0;
> >  
> >  		/* First figure out the reference frequency. There are 2 ways
> > @@ -553,7 +552,7 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
> >  		gen10_sseu_info_init(dev_priv);
> >  
> >  	/* Initialize command stream timestamp frequency */
> > -	info->cs_timestamp_frequency = read_timestamp_frequency(dev_priv);
> > +	info->cs_timestamp_frequency_khz = 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));
> > @@ -570,6 +569,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);
> > +	DRM_DEBUG_DRIVER("CS timestamp frequency: %u kHz\n",
> > +			 info->cs_timestamp_frequency_khz);
> >  }
> > -- 
> > 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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v3 4/4] drm/i915/cnl: only divide up base frequency with crystal source
  2017-11-13 23:34 ` [PATCH v3 4/4] drm/i915/cnl: only divide up base frequency with crystal source Lionel Landwerlin
@ 2017-12-04 17:50   ` Paulo Zanoni
  2017-12-04 18:41     ` Paulo Zanoni
  0 siblings, 1 reply; 12+ messages in thread
From: Paulo Zanoni @ 2017-12-04 17:50 UTC (permalink / raw)
  To: Lionel Landwerlin, intel-gfx

Em Seg, 2017-11-13 às 23:34 +0000, Lionel Landwerlin escreveu:
> We apply this logic to Gen9 as well. We didn't notice this issue as
> most part we've encountered so far only use the crystal as source for
> their timestamp registers.
> 
> Fixes: dab9178333 ("drm/i915: expose command stream timestamp
> frequency to userspace")
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>

Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_device_info.c | 16 ++++++++--------
>  1 file changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> b/drivers/gpu/drm/i915/intel_device_info.c
> index f3e4940fed49..039f8ec7ad27 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -413,15 +413,15 @@ static u32 read_timestamp_frequency(struct
> drm_i915_private *dev_priv)
>  				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);
> +			/* 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_P
> ARAMETER_MASK) >>
> +				      GEN10_RPM_CONFIG0_CTC_SHIFT_PA
> RAMETER_SHIFT);
> +		}
>  
>  		return freq;
>  	}
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v3 4/4] drm/i915/cnl: only divide up base frequency with crystal source
  2017-12-04 17:50   ` Paulo Zanoni
@ 2017-12-04 18:41     ` Paulo Zanoni
  0 siblings, 0 replies; 12+ messages in thread
From: Paulo Zanoni @ 2017-12-04 18:41 UTC (permalink / raw)
  To: Lionel Landwerlin, intel-gfx

Em Seg, 2017-12-04 às 15:50 -0200, Paulo Zanoni escreveu:
> Em Seg, 2017-11-13 às 23:34 +0000, Lionel Landwerlin escreveu:
> > We apply this logic to Gen9 as well. We didn't notice this issue as
> > most part we've encountered so far only use the crystal as source
> > for
> > their timestamp registers.
> > 
> > Fixes: dab9178333 ("drm/i915: expose command stream timestamp
> > frequency to userspace")
> > Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> 
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

I also merged this since I need it.

Patch 3 still needs a check from the people who were reviewing its
previous versions.

> 
> > ---
> >  drivers/gpu/drm/i915/intel_device_info.c | 16 ++++++++--------
> >  1 file changed, 8 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_device_info.c
> > b/drivers/gpu/drm/i915/intel_device_info.c
> > index f3e4940fed49..039f8ec7ad27 100644
> > --- a/drivers/gpu/drm/i915/intel_device_info.c
> > +++ b/drivers/gpu/drm/i915/intel_device_info.c
> > @@ -413,15 +413,15 @@ static u32 read_timestamp_frequency(struct
> > drm_i915_private *dev_priv)
> >  				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_PARAMET
> > ER
> > _MASK) >>
> > -			      GEN10_RPM_CONFIG0_CTC_SHIFT_PARAMETE
> > R_
> > SHIFT);
> > +			/* 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
> > _P
> > ARAMETER_MASK) >>
> > +				      GEN10_RPM_CONFIG0_CTC_SHIFT_
> > PA
> > RAMETER_SHIFT);
> > +		}
> >  
> >  		return freq;
> >  	}
> 
> _______________________________________________
> 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] 12+ messages in thread

end of thread, other threads:[~2017-12-04 18:41 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-13 23:34 [PATCH v3 0/4] drm/i915: some perf cleanups (& fixes!) Lionel Landwerlin
2017-11-13 23:34 ` [PATCH v3 1/4] drm/i915/perf: replace .reg accesses with i915_mmio_reg_offset Lionel Landwerlin
2017-11-14  7:26   ` Ewelina Musial
2017-11-13 23:34 ` [PATCH v3 2/4] drm/i915: fix 64bit divide Lionel Landwerlin
2017-11-14  7:30   ` Ewelina Musial
2017-11-14 14:27     ` Ville Syrjälä
2017-11-13 23:34 ` [PATCH v3 3/4] drm/i915/perf: reuse timestamp frequency from device info Lionel Landwerlin
2017-11-13 23:34 ` [PATCH v3 4/4] drm/i915/cnl: only divide up base frequency with crystal source Lionel Landwerlin
2017-12-04 17:50   ` Paulo Zanoni
2017-12-04 18:41     ` Paulo Zanoni
2017-11-14  0:10 ` ✓ Fi.CI.BAT: success for drm/i915: some perf cleanups (& fixes!) (rev2) Patchwork
2017-11-14  0:53 ` ✓ 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.