Re: [PATCH 04/16] drm/i915: Sample the frame counter instead of a timestamp for CRCs

Ville Syrjälä
2013-10-16T13:29:35Z

On Tue, Oct 15, 2013 at 06:55:30PM +0100, Damien Lespiau wrote:
> Signed-off-by: Damien Lespiau <damien.lespiau@intel.com>
> ---
> drivers/gpu/drm/i915/i915_debugfs.c | 4 ++--
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/i915_irq.c | 8 ++------
> 3 files changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 991abff..58c6fd4 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1748,14 +1748,14 @@ static int i915_pipe_crc(struct seq_file *m, void *data)
> return 0;
> }
>
> - seq_puts(m, " timestamp CRC1 CRC2 CRC3 CRC4 CRC5\n");
> + seq_puts(m, " frame CRC1 CRC2 CRC3 CRC4 CRC5\n");
> head = atomic_read(&pipe_crc->head);
> tail = atomic_read(&pipe_crc->tail);
>
> while (CIRC_CNT(head, tail, INTEL_PIPE_CRC_ENTRIES_NR) >= 1) {
> struct intel_pipe_crc_entry *entry = &pipe_crc->entries[tail];
>
> - seq_printf(m, "%12u %8x %8x %8x %8x %8x\n", entry->timestamp,
> + seq_printf(m, "%8u %8x %8x %8x %8x %8x\n", entry->frame,
> entry->crc[0], entry->crc[1], entry->crc[2],
> entry->crc[3], entry->crc[4]);
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7a1ed3a..cd87919 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1228,7 +1228,7 @@ enum intel_pipe_crc_source {
> };
>
> struct intel_pipe_crc_entry {
> - uint32_t timestamp;
> + uint32_t frame;
> uint32_t crc[5];
> };
>
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 73d76af..0b21828 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -1195,8 +1195,7 @@ static void ivb_pipe_crc_update(struct drm_device *dev, enum pipe pipe)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct intel_pipe_crc *pipe_crc = &dev_priv->pipe_crc[pipe];
> struct intel_pipe_crc_entry *entry;
> - ktime_t now;
> - int ts, head, tail;
> + int head, tail;
>
> head = atomic_read(&pipe_crc->head);
> tail = atomic_read(&pipe_crc->tail);
> @@ -1208,10 +1207,7 @@ static void ivb_pipe_crc_update(struct drm_device *dev, enum pipe pipe)
>
> entry = &pipe_crc->entries[head];
>
> - now = ktime_get();
> - ts = ktime_to_us(now);
> -
> - entry->timestamp = ts;
> + entry->frame = I915_READ(PIPEFRAME(pipe));

BTW that's the wrong register for ctg+. It should be PIPE_FRMCOUNT_GM45.
But it would be better if you just call
dev->driver->get_vblank_counter(). Then you get the correct answer for
everything except gen2, and for gen2 we could implement a software
frame counter if need be.

> entry->crc[0] = I915_READ(PIPE_CRC_RES_1_IVB(pipe));
> entry->crc[1] = I915_READ(PIPE_CRC_RES_2_IVB(pipe));
> entry->crc[2] = I915_READ(PIPE_CRC_RES_3_IVB(pipe));
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Ville Syrjälä
Intel OTC