On 13 November 2017 at 18:18, Lionel Landwerlin
<lionel.g.landwerlin@xxxxxxxxx> wrote:
> 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 udiv_u64 (Chris)
>
> Signed-off-by: Lionel Landwerlin <lionel.g.landwerlin@xxxxxxxxx>
> ---
> 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 42813f4247e2..8ce95fd9d313 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:
> @@ -1096,6 +1094,8 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
>
> intel_sanitize_options(dev_priv);
>
> + i915_perf_init(dev_priv);
> +
If we are moving this to init_hw, we should move i915_perf_fini to
i915_driver_cleanup_hw, to keep the symmetry.
> 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 b538df740ac3..036e197f6824 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 00be015e01df..292ad3e2c307 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);
s/div_u64/div64_u64/
> }
>
> /**
> @@ -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) {
Maybe sprinkle a GEM_BUG_ON(!dev_priv->perf.oa.timestamp_frequency) or
something?
Otherwise:
Reviewed-by: Matthew Auld <matthew.auld@xxxxxxxxx>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
