[Intel-gfx] [PATCH v2 1/4] drm/i915/perf: reuse timestamp frequency from device info

[Matthew Auld]

On 13 November 2017 at 18:18, Lionel Landwerlin
<lionel.g.landwerlin at intel.com> 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 at 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 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 at intel.com>