Re: [RFC 1/4] drm/i915/perf: Add support to correlate GPU timestamp with system time

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 11/15/2017 5:55 PM, Chris Wilson wrote:
Quoting Sagar Arun Kamble (2017-11-15 12:13:51)
  #include <drm/drmP.h>
  #include <drm/intel-gtt.h>
@@ -2149,6 +2150,14 @@ struct i915_perf_stream {
          * @oa_config: The OA configuration used by the stream.
          */
         struct i915_oa_config *oa_config;
+
+       /**
+        * System time correlation variables.
+        */
+       struct cyclecounter cc;
+       spinlock_t systime_lock;
+       struct timespec64 start_systime;
+       struct timecounter tc;
This pattern is repeated a lot by struct timecounter users. (I'm still
trying to understand why the common case is not catered for by a
convenience timecounter api.)
Yes. Looks good case to change the cyclecounter* member in timecounter to cyclecounter.
And then we can avoid separate cyclecounter.

  };
/**
diff --git a/drivers/gpu/drm/i915/i915_perf.c b/drivers/gpu/drm/i915/i915_perf.c
index 00be015..72ddc34 100644
--- a/drivers/gpu/drm/i915/i915_perf.c
+++ b/drivers/gpu/drm/i915/i915_perf.c
@@ -192,6 +192,7 @@
   */
#include <linux/anon_inodes.h>
+#include <linux/clocksource.h>
  #include <linux/sizes.h>
  #include <linux/uuid.h>
@@ -2391,6 +2392,56 @@ static unsigned int i915_perf_poll(struct file *file, poll_table *wait)
  }
/**
+ * i915_cyclecounter_read - read raw cycle/timestamp counter
+ * @cc: cyclecounter structure
+ */
+static u64 i915_cyclecounter_read(const struct cyclecounter *cc)
+{
+       struct i915_perf_stream *stream = container_of(cc, typeof(*stream), cc);
+       struct drm_i915_private *dev_priv = stream->dev_priv;
+       u64 ts_count;
+
+       intel_runtime_pm_get(dev_priv);
+       ts_count = I915_READ64_2x32(GEN4_TIMESTAMP,
+                                   GEN7_TIMESTAMP_UDW);
+       intel_runtime_pm_put(dev_priv);
+
+       return ts_count;
+}
+
+static void i915_perf_init_cyclecounter(struct i915_perf_stream *stream)
+{
+       struct drm_i915_private *dev_priv = stream->dev_priv;
+       int cs_ts_freq = dev_priv->perf.oa.timestamp_frequency;
+       struct cyclecounter *cc = &stream->cc;
+       u32 maxsec;
+
+       cc->read = i915_cyclecounter_read;
+       cc->mask = CYCLECOUNTER_MASK(CS_TIMESTAMP_WIDTH(dev_priv));
+       maxsec = cc->mask / cs_ts_freq;
+
+       clocks_calc_mult_shift(&cc->mult, &cc->shift, cs_ts_freq,
+                              NSEC_PER_SEC, maxsec);
+}
+
+static void i915_perf_init_timecounter(struct i915_perf_stream *stream)
+{
+#define SYSTIME_START_OFFSET   350000 /* Counter read takes about 350us */
+       unsigned long flags;
+       u64 ns;
+
+       i915_perf_init_cyclecounter(stream);
+       spin_lock_init(&stream->systime_lock);
+
+       getnstimeofday64(&stream->start_systime);
+       ns = timespec64_to_ns(&stream->start_systime) + SYSTIME_START_OFFSET;
Use ktime directly. Or else Arnd will be back with a patch to fix it.
(All non-ktime interfaces are effectively deprecated; obsolete for
drivers.)
Ok. Will update.

+       spin_lock_irqsave(&stream->systime_lock, flags);
+       timecounter_init(&stream->tc, &stream->cc, ns);
+       spin_unlock_irqrestore(&stream->systime_lock, flags);
+}
+
+/**
   * i915_perf_enable_locked - handle `I915_PERF_IOCTL_ENABLE` ioctl
   * @stream: A disabled i915 perf stream
   *
@@ -2408,6 +2459,8 @@ static void i915_perf_enable_locked(struct i915_perf_stream *stream)
         /* Allow stream->ops->enable() to refer to this */
         stream->enabled = true;
+ i915_perf_init_timecounter(stream);
+
         if (stream->ops->enable)
                 stream->ops->enable(stream);
  }
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index cfdf4f8..e7e6966 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8882,6 +8882,12 @@ enum skl_power_gate {
/* Gen4+ Timestamp and Pipe Frame time stamp registers */
  #define GEN4_TIMESTAMP         _MMIO(0x2358)
+#define GEN7_TIMESTAMP_UDW     _MMIO(0x235C)
+#define PRE_GEN7_TIMESTAMP_WIDTH       32
+#define GEN7_TIMESTAMP_WIDTH           36
+#define CS_TIMESTAMP_WIDTH(dev_priv) \
+       (INTEL_GEN(dev_priv) < 7 ? PRE_GEN7_TIMESTAMP_WIDTH : \
+                                  GEN7_TIMESTAMP_WIDTH)
s/PRE_GEN7/GEN4/ would be consistent.
If you really want to add support for earlier, I9XX_.
Yes.

Ok. I can accept the justification, and we are not the only ones who do
the cyclecounter -> timecounter correction like this.
-Chris

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux