All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
To: Ville Syrjala <ville.syrjala@linux.intel.com>,
	intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH 2/6] drm/i915: Store CS timestamp frequency in Hz
Date: Wed, 13 May 2020 16:04:52 +0100	[thread overview]
Message-ID: <158938229253.29850.18245041240600513891@build.alporthouse.com> (raw)
In-Reply-To: <20200302143943.32676-2-ville.syrjala@linux.intel.com>

Quoting Ville Syrjala (2020-03-02 14:39:39)
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> kHz isn't accurate enough for storing the CS timestamp
> frequency on some of the platforms. Store the value
> in Hz instead.
> 
> Cc: Lionel Landwerlin <lionel.g.landwerlin@intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c        |  6 ++--
>  drivers/gpu/drm/i915/i915_getparam.c       |  2 +-
>  drivers/gpu/drm/i915/i915_perf.c           | 12 ++++----
>  drivers/gpu/drm/i915/intel_device_info.c   | 34 +++++++++++-----------
>  drivers/gpu/drm/i915/intel_device_info.h   |  2 +-
>  drivers/gpu/drm/i915/selftests/i915_perf.c |  4 +--
>  6 files changed, 30 insertions(+), 30 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 8f2525e4ce0f..c0e54c500017 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1790,8 +1790,8 @@ static int i915_engine_info(struct seq_file *m, void *unused)
>         seq_printf(m, "GT awake? %s [%d]\n",
>                    yesno(dev_priv->gt.awake),
>                    atomic_read(&dev_priv->gt.wakeref.count));
> -       seq_printf(m, "CS timestamp frequency: %u kHz\n",
> -                  RUNTIME_INFO(dev_priv)->cs_timestamp_frequency_khz);
> +       seq_printf(m, "CS timestamp frequency: %u Hz\n",
> +                  RUNTIME_INFO(dev_priv)->cs_timestamp_frequency_hz);
>  
>         p = drm_seq_file_printer(m);
>         for_each_uabi_engine(engine, dev_priv)
> @@ -1890,7 +1890,7 @@ static int
>  i915_perf_noa_delay_set(void *data, u64 val)
>  {
>         struct drm_i915_private *i915 = data;
> -       const u32 clk = RUNTIME_INFO(i915)->cs_timestamp_frequency_khz;
> +       const u32 clk = RUNTIME_INFO(i915)->cs_timestamp_frequency_hz / 1000;
>  
>         /*
>          * This would lead to infinite waits as we're doing timestamp
> diff --git a/drivers/gpu/drm/i915/i915_getparam.c b/drivers/gpu/drm/i915/i915_getparam.c
> index 54fce81d5724..d042644b9cd2 100644
> --- a/drivers/gpu/drm/i915/i915_getparam.c
> +++ b/drivers/gpu/drm/i915/i915_getparam.c
> @@ -153,7 +153,7 @@ int i915_getparam_ioctl(struct drm_device *dev, void *data,
>                         return -ENODEV;
>                 break;
>         case I915_PARAM_CS_TIMESTAMP_FREQUENCY:
> -               value = 1000 * RUNTIME_INFO(i915)->cs_timestamp_frequency_khz;
> +               value = RUNTIME_INFO(i915)->cs_timestamp_frequency_hz;
>                 break;
>         case I915_PARAM_MMAP_GTT_COHERENT:
>                 value = INTEL_INFO(i915)->has_coherent_ggtt;
> diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
> index 716fe6e4e56c..a2f98fb08bf1 100644
> --- a/drivers/gpu/drm/i915/i915_perf.c
> +++ b/drivers/gpu/drm/i915/i915_perf.c
> @@ -1668,8 +1668,8 @@ static int alloc_noa_wait(struct i915_perf_stream *stream)
>         struct i915_vma *vma;
>         const u64 delay_ticks = 0xffffffffffffffff -
>                 DIV_ROUND_UP_ULL(atomic64_read(&stream->perf->noa_programming_delay) *
> -                                RUNTIME_INFO(i915)->cs_timestamp_frequency_khz,
> -                                1000000);
> +                                RUNTIME_INFO(i915)->cs_timestamp_frequency_hz,
> +                                1000000000);
>         const u32 base = stream->engine->mmio_base;
>  #define CS_GPR(x) GEN8_RING_CS_GPR(base, x)
>         u32 *batch, *ts0, *cs, *jump;
> @@ -3466,8 +3466,8 @@ i915_perf_open_ioctl_locked(struct i915_perf *perf,
>  
>  static u64 oa_exponent_to_ns(struct i915_perf *perf, int exponent)
>  {
> -       return div_u64(1000000 * (2ULL << exponent),
> -                      RUNTIME_INFO(perf->i915)->cs_timestamp_frequency_khz);
> +       return div_u64(1000000000 * (2ULL << exponent),
> +                      RUNTIME_INFO(perf->i915)->cs_timestamp_frequency_hz);
>  }
>  
>  /**
> @@ -4359,8 +4359,8 @@ void i915_perf_init(struct drm_i915_private *i915)
>         if (perf->ops.enable_metric_set) {
>                 mutex_init(&perf->lock);
>  
> -               oa_sample_rate_hard_limit = 1000 *
> -                       (RUNTIME_INFO(i915)->cs_timestamp_frequency_khz / 2);
> +               oa_sample_rate_hard_limit =
> +                       RUNTIME_INFO(i915)->cs_timestamp_frequency_hz / 2;
>  
>                 mutex_init(&perf->metrics_lock);
>                 idr_init(&perf->metrics_idr);
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index d7fe12734db8..32733535964d 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -135,8 +135,8 @@ void intel_device_info_print_runtime(const struct intel_runtime_info *info,
>         sseu_dump(&info->sseu, p);
>  
>         drm_printf(p, "rawclk rate: %u kHz\n", info->rawclk_freq);
> -       drm_printf(p, "CS timestamp frequency: %u kHz\n",
> -                  info->cs_timestamp_frequency_khz);
> +       drm_printf(p, "CS timestamp frequency: %u Hz\n",
> +                  info->cs_timestamp_frequency_hz);
>  }
>  
>  static int sseu_eu_idx(const struct sseu_dev_info *sseu, int slice,
> @@ -677,12 +677,12 @@ static u32 read_reference_ts_freq(struct drm_i915_private *dev_priv)
>  
>         base_freq = ((ts_override & GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_MASK) >>
>                      GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DIVIDER_SHIFT) + 1;
> -       base_freq *= 1000;
> +       base_freq *= 1000000;
>  
>         frac_freq = ((ts_override &
>                       GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_MASK) >>
>                      GEN9_TIMESTAMP_OVERRIDE_US_COUNTER_DENOMINATOR_SHIFT);
> -       frac_freq = 1000 / (frac_freq + 1);
> +       frac_freq = 1000000 / (frac_freq + 1);
>  
>         return base_freq + frac_freq;
>  }
> @@ -690,8 +690,8 @@ static u32 read_reference_ts_freq(struct drm_i915_private *dev_priv)
>  static u32 gen10_get_crystal_clock_freq(struct drm_i915_private *dev_priv,
>                                         u32 rpm_config_reg)
>  {
> -       u32 f19_2_mhz = 19200;
> -       u32 f24_mhz = 24000;
> +       u32 f19_2_mhz = 19200000;
> +       u32 f24_mhz = 24000000;
>         u32 crystal_clock = (rpm_config_reg &
>                              GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_MASK) >>
>                             GEN9_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT;
> @@ -710,10 +710,10 @@ static u32 gen10_get_crystal_clock_freq(struct drm_i915_private *dev_priv,
>  static u32 gen11_get_crystal_clock_freq(struct drm_i915_private *dev_priv,
>                                         u32 rpm_config_reg)
>  {
> -       u32 f19_2_mhz = 19200;
> -       u32 f24_mhz = 24000;
> -       u32 f25_mhz = 25000;
> -       u32 f38_4_mhz = 38400;
> +       u32 f19_2_mhz = 19200000;
> +       u32 f24_mhz = 24000000;
> +       u32 f25_mhz = 25000000;
> +       u32 f38_4_mhz = 38400000;
>         u32 crystal_clock = (rpm_config_reg &
>                              GEN11_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_MASK) >>
>                             GEN11_RPM_CONFIG0_CRYSTAL_CLOCK_FREQ_SHIFT;
> @@ -735,9 +735,9 @@ static u32 gen11_get_crystal_clock_freq(struct drm_i915_private *dev_priv,
>  
>  static u32 read_timestamp_frequency(struct drm_i915_private *dev_priv)
>  {
> -       u32 f12_5_mhz = 12500;
> -       u32 f19_2_mhz = 19200;
> -       u32 f24_mhz = 24000;
> +       u32 f12_5_mhz = 12500000;
> +       u32 f19_2_mhz = 19200000;
> +       u32 f24_mhz = 24000000;
>  
>         if (INTEL_GEN(dev_priv) <= 4) {
>                 /* PRMs say:
> @@ -746,7 +746,7 @@ static u32 read_timestamp_frequency(struct drm_i915_private *dev_priv)
>                  *      hclks." (through the “Clocking Configuration”
>                  *      (“CLKCFG”) MCHBAR register)
>                  */
> -               return RUNTIME_INFO(dev_priv)->rawclk_freq / 16;
> +               return RUNTIME_INFO(dev_priv)->rawclk_freq * 1000 / 16;
>         } else if (INTEL_GEN(dev_priv) <= 8) {
>                 /* PRMs say:
>                  *
> @@ -1050,11 +1050,11 @@ void intel_device_info_runtime_init(struct drm_i915_private *dev_priv)
>         drm_dbg(&dev_priv->drm, "rawclk rate: %d kHz\n", runtime->rawclk_freq);
>  
>         /* Initialize command stream timestamp frequency */
> -       runtime->cs_timestamp_frequency_khz =
> +       runtime->cs_timestamp_frequency_hz =
>                 read_timestamp_frequency(dev_priv);
> -       if (runtime->cs_timestamp_frequency_khz) {
> +       if (runtime->cs_timestamp_frequency_hz) {
>                 runtime->cs_timestamp_period_ns =
> -                       div_u64(1e6, runtime->cs_timestamp_frequency_khz);
> +                       div_u64(1e9, runtime->cs_timestamp_frequency_hz);
>                 drm_dbg(&dev_priv->drm,
>                         "CS timestamp wraparound in %lldms\n",
>                         div_u64(mul_u32_u32(runtime->cs_timestamp_period_ns,
> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
> index 1ecb9df2de91..432e9c7c0fe7 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.h
> +++ b/drivers/gpu/drm/i915/intel_device_info.h
> @@ -218,7 +218,7 @@ struct intel_runtime_info {
>  
>         u32 rawclk_freq;
>  
> -       u32 cs_timestamp_frequency_khz;
> +       u32 cs_timestamp_frequency_hz;

As far as I'm aware, there are no plans for GHz crystals.

Counted all the zeros, which are a lot :)

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2020-05-13 15:05 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-02 14:39 [Intel-gfx] [PATCH 1/6] drm/i915: Nuke pointless div by 64bit Ville Syrjala
2020-03-02 14:39 ` [Intel-gfx] [PATCH 2/6] drm/i915: Store CS timestamp frequency in Hz Ville Syrjala
2020-05-13 15:04   ` Chris Wilson [this message]
2020-05-13 15:08   ` Lionel Landwerlin
2020-03-02 14:39 ` [Intel-gfx] [PATCH 3/6] drm/i915: Fix cs_timestamp_frequency_hz for ctg/elk/ilk Ville Syrjala
2020-03-02 14:39 ` [Intel-gfx] [PATCH 4/6] drm/i915: Fix cs_timestamp_frequency_hz for cl/bw Ville Syrjala
2020-03-02 14:39 ` [Intel-gfx] [PATCH 5/6] drm/i915: Extract i915_cs_timestamp_{ns_to_ticks, tick_to_ns}() Ville Syrjala
2020-05-13 15:09   ` Chris Wilson
2020-03-02 14:39 ` [Intel-gfx] [PATCH 6/6] drm/i915/selftests: Make the CS timestamp tests work on gen4-snb (sort of) Ville Syrjala
2020-05-17 12:49   ` Chris Wilson
2020-03-02 14:58 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/6] drm/i915: Nuke pointless div by 64bit Patchwork
2020-03-02 15:15 ` [Intel-gfx] ✗ Fi.CI.DOCS: " Patchwork
2020-03-02 15:24 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-03-03  1:51 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
2020-05-13 14:57 ` [Intel-gfx] [PATCH 1/6] " Chris Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=158938229253.29850.18245041240600513891@build.alporthouse.com \
    --to=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=ville.syrjala@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.