diff mbox

[7/7] drm/i915/perf: reuse timestamp frequency from device info

Message ID 20171110190845.32574-8-lionel.g.landwerlin@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Lionel Landwerlin Nov. 10, 2017, 7:08 p.m. UTC 
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(-)

Comments

Chris Wilson Nov. 10, 2017, 9 p.m. UTC | #1 
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
Lionel Landwerlin Nov. 13, 2017, 3:19 p.m. UTC | #2 
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
Chris Wilson Nov. 13, 2017, 3:28 p.m. UTC | #3 
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
diff mbox

Patch

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);