From 3bb403bf421b5b00366a9041a7edc0a1f6494f5e Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Mon, 14 Sep 2015 22:43:44 +0300 Subject: [PATCH] drm: Stop using linedur_ns and pixeldur_ns for vblank timestamps MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit linedur_ns, and especially pixeldur_ns are becoming rather inaccurate to be used for the vblank timestamp correction. With 4k@60 the pixel duration is already below 2ns, so the amount of error due to the truncation to nanoseconds is introducing quite a bit of error. We can avoid such problems if we instead calculate the timestamp delta_ns directly from the dislay timings, avoiding the use of these intermediate truncated values. Signed-off-by: Ville Syrjälä Reviewed-by: Maarten Lankhorst [danvet: Squash in fixup from Thierry Reding for amdgpu.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/amd/amdgpu/amdgpu_display.c | 11 ++++++----- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 3 ++- drivers/gpu/drm/drm_irq.c | 19 ++++++++----------- drivers/gpu/drm/i915/i915_irq.c | 4 ++-- drivers/gpu/drm/nouveau/nouveau_display.c | 3 ++- drivers/gpu/drm/nouveau/nouveau_display.h | 3 ++- drivers/gpu/drm/radeon/radeon_display.c | 14 ++++++++------ drivers/gpu/drm/radeon/radeon_drv.c | 5 +++-- drivers/gpu/drm/radeon/radeon_mode.h | 5 +++-- drivers/gpu/drm/radeon/radeon_pm.c | 4 +++- include/drm/drmP.h | 6 ++++-- 11 files changed, 43 insertions(+), 34 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c index e3d7077..9b34a34 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_display.c @@ -745,7 +745,8 @@ bool amdgpu_crtc_scaling_mode_fixup(struct drm_crtc *crtc, * */ int amdgpu_get_crtc_scanoutpos(struct drm_device *dev, int crtc, unsigned int flags, - int *vpos, int *hpos, ktime_t *stime, ktime_t *etime) + int *vpos, int *hpos, ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode) { u32 vbl = 0, position = 0; int vbl_start, vbl_end, vtotal, ret = 0; @@ -781,7 +782,7 @@ int amdgpu_get_crtc_scanoutpos(struct drm_device *dev, int crtc, unsigned int fl } else { /* No: Fake something reasonable which gives at least ok results. */ - vbl_start = adev->mode_info.crtcs[crtc]->base.hwmode.crtc_vdisplay; + vbl_start = mode->crtc_vdisplay; vbl_end = 0; } @@ -797,7 +798,7 @@ int amdgpu_get_crtc_scanoutpos(struct drm_device *dev, int crtc, unsigned int fl /* Inside "upper part" of vblank area? Apply corrective offset if so: */ if (in_vbl && (*vpos >= vbl_start)) { - vtotal = adev->mode_info.crtcs[crtc]->base.hwmode.crtc_vtotal; + vtotal = mode->crtc_vtotal; *vpos = *vpos - vtotal; } @@ -819,8 +820,8 @@ int amdgpu_get_crtc_scanoutpos(struct drm_device *dev, int crtc, unsigned int fl * We only do this if DRM_CALLED_FROM_VBLIRQ. */ if ((flags & DRM_CALLED_FROM_VBLIRQ) && !in_vbl) { - vbl_start = adev->mode_info.crtcs[crtc]->base.hwmode.crtc_vdisplay; - vtotal = adev->mode_info.crtcs[crtc]->base.hwmode.crtc_vtotal; + vbl_start = mode->crtc_vdisplay; + vtotal = mode->crtc_vtotal; if (vbl_start - *vpos < vtotal / 100) { *vpos -= vtotal; diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h index 64efe5b..2b03425 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h @@ -543,7 +543,8 @@ void amdgpu_encoder_set_active_device(struct drm_encoder *encoder); int amdgpu_get_crtc_scanoutpos(struct drm_device *dev, int crtc, unsigned int flags, int *vpos, int *hpos, ktime_t *stime, - ktime_t *etime); + ktime_t *etime, + const struct drm_display_mode *mode); int amdgpu_framebuffer_init(struct drm_device *dev, struct amdgpu_framebuffer *rfb, diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 6b2fefd..9fab333 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -694,12 +694,11 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, unsigned flags, const struct drm_display_mode *mode) { - struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; struct timeval tv_etime; ktime_t stime, etime; int vbl_status; int vpos, hpos, i; - int framedur_ns, linedur_ns, pixeldur_ns, delta_ns, duration_ns; + int delta_ns, duration_ns; bool invbl; if (pipe >= dev->num_crtcs) { @@ -713,15 +712,10 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, return -EIO; } - /* Durations of frames, lines, pixels in nanoseconds. */ - framedur_ns = vblank->framedur_ns; - linedur_ns = vblank->linedur_ns; - pixeldur_ns = vblank->pixeldur_ns; - /* If mode timing undefined, just return as no-op: * Happens during initial modesetting of a crtc. */ - if (framedur_ns == 0) { + if (mode->crtc_clock == 0) { DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n", pipe); return -EAGAIN; } @@ -738,8 +732,10 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, * Get vertical and horizontal scanout position vpos, hpos, * and bounding timestamps stime, etime, pre/post query. */ - vbl_status = dev->driver->get_scanout_position(dev, pipe, flags, &vpos, - &hpos, &stime, &etime); + vbl_status = dev->driver->get_scanout_position(dev, pipe, flags, + &vpos, &hpos, + &stime, &etime, + mode); /* Return as no-op if scanout query unsupported or failed. */ if (!(vbl_status & DRM_SCANOUTPOS_VALID)) { @@ -776,7 +772,8 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, * since start of scanout at first display scanline. delta_ns * can be negative if start of scanout hasn't happened yet. */ - delta_ns = vpos * linedur_ns + hpos * pixeldur_ns; + delta_ns = div_s64(1000000LL * (vpos * mode->crtc_htotal + hpos), + mode->crtc_clock); if (!drm_timestamp_monotonic) etime = ktime_mono_to_real(etime); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 4cbc722..ce1e0f5 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -648,12 +648,12 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc) static int i915_get_crtc_scanoutpos(struct drm_device *dev, int pipe, unsigned int flags, int *vpos, int *hpos, - ktime_t *stime, ktime_t *etime) + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode) { struct drm_i915_private *dev_priv = dev->dev_private; struct drm_crtc *crtc = dev_priv->pipe_to_crtc_mapping[pipe]; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); - const struct drm_display_mode *mode = &intel_crtc->base.hwmode; int position; int vbl_start, vbl_end, hsync_start, htotal, vtotal; bool in_vbl = true; diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index 425515f..a82c3cb 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -133,7 +133,8 @@ nouveau_display_scanoutpos_head(struct drm_crtc *crtc, int *vpos, int *hpos, int nouveau_display_scanoutpos(struct drm_device *dev, int head, unsigned int flags, - int *vpos, int *hpos, ktime_t *stime, ktime_t *etime) + int *vpos, int *hpos, ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode) { struct drm_crtc *crtc; diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h index a6213e2..4182d21 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.h +++ b/drivers/gpu/drm/nouveau/nouveau_display.h @@ -68,7 +68,8 @@ void nouveau_display_resume(struct drm_device *dev, bool runtime); int nouveau_display_vblank_enable(struct drm_device *, int); void nouveau_display_vblank_disable(struct drm_device *, int); int nouveau_display_scanoutpos(struct drm_device *, int, unsigned int, - int *, int *, ktime_t *, ktime_t *); + int *, int *, ktime_t *, ktime_t *, + const struct drm_display_mode *); int nouveau_display_vblstamp(struct drm_device *, int, int *, struct timeval *, unsigned); diff --git a/drivers/gpu/drm/radeon/radeon_display.c b/drivers/gpu/drm/radeon/radeon_display.c index d2e9e9e..0503af7 100644 --- a/drivers/gpu/drm/radeon/radeon_display.c +++ b/drivers/gpu/drm/radeon/radeon_display.c @@ -323,7 +323,8 @@ void radeon_crtc_handle_vblank(struct radeon_device *rdev, int crtc_id) */ if (update_pending && (DRM_SCANOUTPOS_VALID & radeon_get_crtc_scanoutpos(rdev->ddev, crtc_id, 0, - &vpos, &hpos, NULL, NULL)) && + &vpos, &hpos, NULL, NULL, + &rdev->mode_info.crtcs[crtc_id]->base.hwmode)) && ((vpos >= (99 * rdev->mode_info.crtcs[crtc_id]->base.hwmode.crtc_vdisplay)/100) || (vpos < 0 && !ASIC_IS_AVIVO(rdev)))) { /* crtc didn't flip in this target vblank interval, @@ -1799,7 +1800,8 @@ bool radeon_crtc_scaling_mode_fixup(struct drm_crtc *crtc, * */ int radeon_get_crtc_scanoutpos(struct drm_device *dev, int crtc, unsigned int flags, - int *vpos, int *hpos, ktime_t *stime, ktime_t *etime) + int *vpos, int *hpos, ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode) { u32 stat_crtc = 0, vbl = 0, position = 0; int vbl_start, vbl_end, vtotal, ret = 0; @@ -1914,7 +1916,7 @@ int radeon_get_crtc_scanoutpos(struct drm_device *dev, int crtc, unsigned int fl } else { /* No: Fake something reasonable which gives at least ok results. */ - vbl_start = rdev->mode_info.crtcs[crtc]->base.hwmode.crtc_vdisplay; + vbl_start = mode->crtc_vdisplay; vbl_end = 0; } @@ -1930,7 +1932,7 @@ int radeon_get_crtc_scanoutpos(struct drm_device *dev, int crtc, unsigned int fl /* Inside "upper part" of vblank area? Apply corrective offset if so: */ if (in_vbl && (*vpos >= vbl_start)) { - vtotal = rdev->mode_info.crtcs[crtc]->base.hwmode.crtc_vtotal; + vtotal = mode->crtc_vtotal; *vpos = *vpos - vtotal; } @@ -1952,8 +1954,8 @@ int radeon_get_crtc_scanoutpos(struct drm_device *dev, int crtc, unsigned int fl * We only do this if DRM_CALLED_FROM_VBLIRQ. */ if ((flags & DRM_CALLED_FROM_VBLIRQ) && !in_vbl) { - vbl_start = rdev->mode_info.crtcs[crtc]->base.hwmode.crtc_vdisplay; - vtotal = rdev->mode_info.crtcs[crtc]->base.hwmode.crtc_vtotal; + vbl_start = mode->crtc_vdisplay; + vtotal = mode->crtc_vtotal; if (vbl_start - *vpos < vtotal / 100) { *vpos -= vtotal; diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index 5751446..e30c1d7 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -126,8 +126,9 @@ struct dma_buf *radeon_gem_prime_export(struct drm_device *dev, int flags); extern int radeon_get_crtc_scanoutpos(struct drm_device *dev, int crtc, unsigned int flags, - int *vpos, int *hpos, ktime_t *stime, - ktime_t *etime); + int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode); extern bool radeon_is_px(struct drm_device *dev); extern const struct drm_ioctl_desc radeon_ioctls_kms[]; extern int radeon_max_kms_ioctl; diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h index aecc3e3..2317d04 100644 --- a/drivers/gpu/drm/radeon/radeon_mode.h +++ b/drivers/gpu/drm/radeon/radeon_mode.h @@ -876,8 +876,9 @@ extern void radeon_cursor_reset(struct drm_crtc *crtc); extern int radeon_get_crtc_scanoutpos(struct drm_device *dev, int crtc, unsigned int flags, - int *vpos, int *hpos, ktime_t *stime, - ktime_t *etime); + int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode); extern bool radeon_combios_check_hardcoded_edid(struct radeon_device *rdev); extern struct edid * diff --git a/drivers/gpu/drm/radeon/radeon_pm.c b/drivers/gpu/drm/radeon/radeon_pm.c index 05751f3..10f4c12 100644 --- a/drivers/gpu/drm/radeon/radeon_pm.c +++ b/drivers/gpu/drm/radeon/radeon_pm.c @@ -1733,7 +1733,9 @@ static bool radeon_pm_in_vbl(struct radeon_device *rdev) */ for (crtc = 0; (crtc < rdev->num_crtc) && in_vbl; crtc++) { if (rdev->pm.active_crtcs & (1 << crtc)) { - vbl_status = radeon_get_crtc_scanoutpos(rdev->ddev, crtc, 0, &vpos, &hpos, NULL, NULL); + vbl_status = radeon_get_crtc_scanoutpos(rdev->ddev, crtc, 0, + &vpos, &hpos, NULL, NULL, + &rdev->mode_info.crtcs[crtc]->base.hwmode); if ((vbl_status & DRM_SCANOUTPOS_VALID) && !(vbl_status & DRM_SCANOUTPOS_IN_VBLANK)) in_vbl = false; diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 2998867..b2a95e7 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -482,6 +482,7 @@ struct drm_driver { * scanout position query. Can be NULL to skip timestamp. * \param *etime Target location for timestamp taken immediately after * scanout position query. Can be NULL to skip timestamp. + * \param mode Current display timings. * * Returns vpos as a positive number while in active scanout area. * Returns vpos as a negative number inside vblank, counting the number @@ -499,8 +500,9 @@ struct drm_driver { */ int (*get_scanout_position) (struct drm_device *dev, int crtc, unsigned int flags, - int *vpos, int *hpos, ktime_t *stime, - ktime_t *etime); + int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode); /** * Called by \c drm_get_last_vbltimestamp. Should return a precise -- 2.7.4