Tighten the timestamp queries before/after the register read so that we
have less uncertainity for when the read actually took place. This is
more apt for the older generations where it is not a simple single
register read. Whether we are able to discern an improvement in our
sampling accuracy remains to be seen.

Signed-off-by: Chris Wilson <ch...@chris-wilson.co.uk>
Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_irq.c | 57 ++++++++++++++++++++++++---------
 1 file changed, 42 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 8e823ba25f5f..9c44df8ecce7 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -713,7 +713,9 @@ u32 g4x_get_vblank_counter(struct drm_crtc *crtc)
  * This function will use Framestamp and current
  * timestamp registers to calculate the scanline.
  */
-static u32 __intel_get_crtc_scanline_from_timestamp(struct intel_crtc *crtc)
+static u32
+__intel_get_crtc_scanline_from_timestamp(struct intel_crtc *crtc,
+                                        ktime_t *stime, ktime_t *etime)
 {
        struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
        struct drm_vblank_crtc *vblank =
@@ -737,6 +739,9 @@ static u32 __intel_get_crtc_scanline_from_timestamp(struct 
intel_crtc *crtc)
                 * pipe frame time stamp. The time stamp value
                 * is sampled at every start of vertical blank.
                 */
+               if (stime)
+                       *stime = ktime_get();
+
                scan_prev_time = intel_de_read_fw(dev_priv,
                                                  PIPE_FRMTMSTMP(crtc->pipe));
 
@@ -746,6 +751,9 @@ static u32 __intel_get_crtc_scanline_from_timestamp(struct 
intel_crtc *crtc)
                 */
                scan_curr_time = intel_de_read_fw(dev_priv, IVB_TIMESTAMP_CTR);
 
+               if (etime)
+                       *etime = ktime_get();
+
                scan_post_time = intel_de_read_fw(dev_priv,
                                                  PIPE_FRMTMSTMP(crtc->pipe));
        } while (scan_post_time != scan_prev_time);
@@ -762,7 +770,8 @@ static u32 __intel_get_crtc_scanline_from_timestamp(struct 
intel_crtc *crtc)
  * intel_de_read_fw(), only for fast reads of display block, no need for
  * forcewake etc.
  */
-static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
+static int __intel_get_crtc_scanline(struct intel_crtc *crtc,
+                                    ktime_t *stime, ktime_t *etime)
 {
        struct drm_device *dev = crtc->base.dev;
        struct drm_i915_private *dev_priv = to_i915(dev);
@@ -771,23 +780,34 @@ static int __intel_get_crtc_scanline(struct intel_crtc 
*crtc)
        enum pipe pipe = crtc->pipe;
        int position, vtotal;
 
-       if (!crtc->active)
+       if (!crtc->active) {
+               if (stime)
+                       *stime = 0;
+               if (etime)
+                       *etime = 0;
                return -1;
+       }
 
        vblank = &crtc->base.dev->vblank[drm_crtc_index(&crtc->base)];
        mode = &vblank->hwmode;
 
        if (crtc->mode_flags & I915_MODE_FLAG_GET_SCANLINE_FROM_TIMESTAMP)
-               return __intel_get_crtc_scanline_from_timestamp(crtc);
+               return __intel_get_crtc_scanline_from_timestamp(crtc,
+                                                               stime,
+                                                               etime);
 
        vtotal = mode->crtc_vtotal;
        if (mode->flags & DRM_MODE_FLAG_INTERLACE)
                vtotal /= 2;
 
+       if (stime)
+               *stime = ktime_get();
        if (IS_GEN(dev_priv, 2))
                position = intel_de_read_fw(dev_priv, PIPEDSL(pipe)) & 
DSL_LINEMASK_GEN2;
        else
                position = intel_de_read_fw(dev_priv, PIPEDSL(pipe)) & 
DSL_LINEMASK_GEN3;
+       if (etime)
+               *etime = ktime_get();
 
        /*
         * On HSW, the DSL reg (0x70000) appears to return 0 if we
@@ -806,7 +826,13 @@ static int __intel_get_crtc_scanline(struct intel_crtc 
*crtc)
 
                for (i = 0; i < 100; i++) {
                        udelay(1);
+
+                       if (stime)
+                               *stime = ktime_get();
                        temp = intel_de_read_fw(dev_priv, PIPEDSL(pipe)) & 
DSL_LINEMASK_GEN3;
+                       if (etime)
+                               *etime = ktime_get();
+
                        if (temp != position) {
                                position = temp;
                                break;
@@ -866,21 +892,25 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc 
*_crtc,
 
        /* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */
 
-       /* Get optional system timestamp before query. */
-       if (stime)
-               *stime = ktime_get();
-
        if (use_scanline_counter) {
                /* No obvious pixelcount register. Only query vertical
                 * scanout position from Display scan line register.
                 */
-               position = __intel_get_crtc_scanline(crtc);
+               position = __intel_get_crtc_scanline(crtc, stime, etime);
        } else {
+               /* Get optional system timestamp before query. */
+               if (stime)
+                       *stime = ktime_get();
+
                /* Have access to pixelcount since start of frame.
                 * We can split this into vertical and horizontal
                 * scanout position.
                 */
-               position = (intel_de_read_fw(dev_priv, PIPEFRAMEPIXEL(pipe)) & 
PIPE_PIXEL_MASK) >> PIPE_PIXEL_SHIFT;
+               position = intel_de_read_fw(dev_priv, PIPEFRAMEPIXEL(pipe));
+
+               /* Get optional system timestamp after query. */
+               if (etime)
+                       *etime = ktime_get();
 
                /* convert to pixel counts */
                vbl_start *= htotal;
@@ -896,6 +926,7 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc *_crtc,
                 * matches how the scanline counter based position works since
                 * the scanline counter doesn't count the two half lines.
                 */
+               position = (position & PIPE_PIXEL_MASK) >> PIPE_PIXEL_SHIFT;
                if (position >= vtotal)
                        position = vtotal - 1;
 
@@ -911,10 +942,6 @@ static bool i915_get_crtc_scanoutpos(struct drm_crtc 
*_crtc,
                position = (position + htotal - hsync_start) % vtotal;
        }
 
-       /* Get optional system timestamp after query. */
-       if (etime)
-               *etime = ktime_get();
-
        /* preempt_enable_rt() should go right here in PREEMPT_RT patchset. */
 
        spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
@@ -956,7 +983,7 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc)
        int position;
 
        spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
-       position = __intel_get_crtc_scanline(crtc);
+       position = __intel_get_crtc_scanline(crtc, NULL, NULL);
        spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
 
        return position;
-- 
2.27.0

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

Reply via email to