- We can drop the different return value flags, the only caller only cares about whether the scanout position is valid or not. Also, it's entirely undefined what "accurate" means, if we'd really care we should probably wire the max_error through. But since we never even report this to userspace it's kinda moot.
- Drop all the fancy input flags, there's really only the "called from vblank irq" one. Well except for radeon/amdgpu, which added their own private flags.
Since amdgpu/radoen also use the scanoutposition function internally I just gave them a tiny wrapper, plus copies of all the old #defines they need. Everyone else gets simplified code.
Note how we could remove a lot of error conditions if we'd move this helper hook to drm_crtc_helper_funcs and would pass it a crtc directly.
v2: Make it compile on arm.
Cc: Mario Kleiner mario.kleiner@tuebingen.mpg.de Cc: Eric Anholt eric@anholt.net Cc: Rob Clark robdclark@gmail.com Cc: linux-arm-msm@vger.kernel.org Cc: freedreno@lists.freedesktop.org Cc: Alex Deucher alexander.deucher@amd.com Cc: Christian König christian.koenig@amd.com Cc: Ben Skeggs bskeggs@redhat.com Signed-off-by: Daniel Vetter daniel.vetter@intel.com --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 12 +++++++++++- drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h | 3 +++ drivers/gpu/drm/drm_irq.c | 16 ++++++++-------- drivers/gpu/drm/i915/i915_irq.c | 19 ++++++------------- drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 19 +++++++------------ drivers/gpu/drm/nouveau/nouveau_display.c | 16 +++++++--------- drivers/gpu/drm/nouveau/nouveau_display.h | 6 +++--- drivers/gpu/drm/radeon/radeon_drv.c | 12 +++++++++++- drivers/gpu/drm/radeon/radeon_mode.h | 3 +++ drivers/gpu/drm/vc4/vc4_crtc.c | 21 ++++++++++----------- drivers/gpu/drm/vc4/vc4_drv.h | 8 ++++---- include/drm/drmP.h | 8 -------- include/drm/drm_drv.h | 26 ++++++++++---------------- 13 files changed, 83 insertions(+), 86 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c index ba169a0699d5..87910850e5dc 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c @@ -678,6 +678,16 @@ static const struct file_operations amdgpu_driver_kms_fops = { #endif };
+static bool +amdgpu_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe, + bool in_vblank_irq, int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode) +{ + return amdgpu_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos, + stime, etime, mode); +} + static struct drm_driver kms_driver = { .driver_features = DRIVER_USE_AGP | @@ -693,7 +703,7 @@ static struct drm_driver kms_driver = { .enable_vblank = amdgpu_enable_vblank_kms, .disable_vblank = amdgpu_disable_vblank_kms, .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos, - .get_scanout_position = amdgpu_get_crtc_scanoutpos, + .get_scanout_position = amdgpu_get_crtc_scanout_position, #if defined(CONFIG_DEBUG_FS) .debugfs_init = amdgpu_debugfs_init, #endif diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h index c12497bd3889..6b8f766a6a35 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h @@ -534,6 +534,9 @@ struct amdgpu_framebuffer { ((em) == ATOM_ENCODER_MODE_DP_MST))
/* Driver internal use only flags of amdgpu_get_crtc_scanoutpos() */ +#define DRM_SCANOUTPOS_VALID (1 << 0) +#define DRM_SCANOUTPOS_IN_VBLANK (1 << 1) +#define DRM_SCANOUTPOS_ACCURATE (1 << 2) #define USE_REAL_VBLANKSTART (1 << 30) #define GET_DISTANCE_TO_VBLANKSTART (1 << 31)
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c index 61d726b1f810..00e26b4312cc 100644 --- a/drivers/gpu/drm/drm_irq.c +++ b/drivers/gpu/drm/drm_irq.c @@ -744,13 +744,12 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, { struct timeval tv_etime; ktime_t stime, etime; - unsigned int vbl_status; + bool vbl_status; struct drm_crtc *crtc; const struct drm_display_mode *mode; struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; int vpos, hpos, i; int delta_ns, duration_ns; - unsigned flags = in_vblank_irq ? DRM_CALLED_FROM_VBLIRQ : 0;
if (!drm_core_check_feature(dev, DRIVER_MODESET)) return false; @@ -793,15 +792,16 @@ bool 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, + vbl_status = dev->driver->get_scanout_position(dev, pipe, + in_vblank_irq, &vpos, &hpos, &stime, &etime, mode);
/* Return as no-op if scanout query unsupported or failed. */ - if (!(vbl_status & DRM_SCANOUTPOS_VALID)) { - DRM_DEBUG("crtc %u : scanoutpos query failed [0x%x].\n", - pipe, vbl_status); + if (!vbl_status) { + DRM_DEBUG("crtc %u : scanoutpos query failed.\n", + pipe); return false; }
@@ -840,8 +840,8 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev, etime = ktime_sub_ns(etime, delta_ns); *vblank_time = ktime_to_timeval(etime);
- DRM_DEBUG_VBL("crtc %u : v 0x%x p(%d,%d)@ %ld.%ld -> %ld.%ld [e %d us, %d rep]\n", - pipe, vbl_status, hpos, vpos, + DRM_DEBUG_VBL("crtc %u : v p(%d,%d)@ %ld.%ld -> %ld.%ld [e %d us, %d rep]\n", + pipe, hpos, vpos, (long)tv_etime.tv_sec, (long)tv_etime.tv_usec, (long)vblank_time->tv_sec, (long)vblank_time->tv_usec, duration_ns/1000, i); diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index feadfea77354..a211f9d2e6ca 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -827,10 +827,10 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc) return (position + crtc->scanline_offset) % vtotal; }
-static int i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe, - unsigned int flags, int *vpos, int *hpos, - ktime_t *stime, ktime_t *etime, - const struct drm_display_mode *mode) +static bool i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe, + bool in_vblank_irq, int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode) { struct drm_i915_private *dev_priv = to_i915(dev); struct intel_crtc *intel_crtc = intel_get_crtc_for_pipe(dev_priv, @@ -838,13 +838,12 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe, int position; int vbl_start, vbl_end, hsync_start, htotal, vtotal; bool in_vbl = true; - int ret = 0; unsigned long irqflags;
if (WARN_ON(!mode->crtc_clock)) { DRM_DEBUG_DRIVER("trying to get scanoutpos for disabled " "pipe %c\n", pipe_name(pipe)); - return 0; + return false; }
htotal = mode->crtc_htotal; @@ -859,8 +858,6 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe, vtotal /= 2; }
- ret |= DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE; - /* * Lock uncore.lock, as we will do multiple timing critical raw * register reads, potentially with preemption disabled, so the @@ -944,11 +941,7 @@ static int i915_get_crtc_scanoutpos(struct drm_device *dev, unsigned int pipe, *hpos = position - (*vpos * htotal); }
- /* In vblank? */ - if (in_vbl) - ret |= DRM_SCANOUTPOS_IN_VBLANK; - - return ret; + return true; }
int intel_get_crtc_scanline(struct intel_crtc *crtc) diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c index 6ba216b8bba9..959b826123a2 100644 --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c @@ -530,31 +530,28 @@ static struct drm_encoder *get_encoder_from_crtc(struct drm_crtc *crtc) return NULL; }
-static int mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe, - unsigned int flags, int *vpos, int *hpos, - ktime_t *stime, ktime_t *etime, - const struct drm_display_mode *mode) +static bool mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe, + bool in_vblank_irq, int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode) { struct msm_drm_private *priv = dev->dev_private; struct drm_crtc *crtc; struct drm_encoder *encoder; int line, vsw, vbp, vactive_start, vactive_end, vfp_end; - int ret = 0;
crtc = priv->crtcs[pipe]; if (!crtc) { DRM_ERROR("Invalid crtc %d\n", pipe); - return 0; + return false; }
encoder = get_encoder_from_crtc(crtc); if (!encoder) { DRM_ERROR("no encoder found for crtc %d\n", pipe); - return 0; + return false; }
- ret |= DRM_SCANOUTPOS_VALID | DRM_SCANOUTPOS_ACCURATE; - vsw = mode->crtc_vsync_end - mode->crtc_vsync_start; vbp = mode->crtc_vtotal - mode->crtc_vsync_end;
@@ -578,10 +575,8 @@ static int mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe,
if (line < vactive_start) { line -= vactive_start; - ret |= DRM_SCANOUTPOS_IN_VBLANK; } else if (line > vactive_end) { line = line - vfp_end - vactive_start; - ret |= DRM_SCANOUTPOS_IN_VBLANK; } else { line -= vactive_start; } @@ -592,7 +587,7 @@ static int mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe, if (etime) *etime = ktime_get();
- return ret; + return true; }
static u32 mdp5_get_vblank_counter(struct drm_device *dev, unsigned int pipe) diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c index e4bdac13d4e9..8b3622afa530 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.c +++ b/drivers/gpu/drm/nouveau/nouveau_display.c @@ -98,7 +98,7 @@ calc(int blanks, int blanke, int total, int line) return line; }
-static int +static bool nouveau_display_scanoutpos_head(struct drm_crtc *crtc, int *vpos, int *hpos, ktime_t *stime, ktime_t *etime) { @@ -111,16 +111,16 @@ nouveau_display_scanoutpos_head(struct drm_crtc *crtc, int *vpos, int *hpos, }; struct nouveau_display *disp = nouveau_display(crtc->dev); struct drm_vblank_crtc *vblank = &crtc->dev->vblank[drm_crtc_index(crtc)]; - int ret, retry = 1; + int retry = 1; + bool ret = false;
do { ret = nvif_mthd(&disp->disp, 0, &args, sizeof(args)); if (ret != 0) - return 0; + return false;
if (args.scan.vline) { - ret |= DRM_SCANOUTPOS_ACCURATE; - ret |= DRM_SCANOUTPOS_VALID; + ret = true; break; }
@@ -133,14 +133,12 @@ nouveau_display_scanoutpos_head(struct drm_crtc *crtc, int *vpos, int *hpos, if (stime) *stime = ns_to_ktime(args.scan.time[0]); if (etime) *etime = ns_to_ktime(args.scan.time[1]);
- if (*vpos < 0) - ret |= DRM_SCANOUTPOS_IN_VBLANK; return ret; }
-int +bool nouveau_display_scanoutpos(struct drm_device *dev, unsigned int pipe, - unsigned int flags, int *vpos, int *hpos, + bool in_vblank_irq, int *vpos, int *hpos, ktime_t *stime, ktime_t *etime, const struct drm_display_mode *mode) { diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h index 8360a85ed5ef..b76b65c7c8da 100644 --- a/drivers/gpu/drm/nouveau/nouveau_display.h +++ b/drivers/gpu/drm/nouveau/nouveau_display.h @@ -68,9 +68,9 @@ int nouveau_display_suspend(struct drm_device *dev, bool runtime); void nouveau_display_resume(struct drm_device *dev, bool runtime); int nouveau_display_vblank_enable(struct drm_device *, unsigned int); void nouveau_display_vblank_disable(struct drm_device *, unsigned int); -int nouveau_display_scanoutpos(struct drm_device *, unsigned int, - unsigned int, int *, int *, ktime_t *, - ktime_t *, const struct drm_display_mode *); +bool nouveau_display_scanoutpos(struct drm_device *, unsigned int, + bool, int *, int *, ktime_t *, + ktime_t *, const struct drm_display_mode *);
int nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb, struct drm_pending_vblank_event *event, diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c index a4bf09cd33f7..669ebfda8038 100644 --- a/drivers/gpu/drm/radeon/radeon_drv.c +++ b/drivers/gpu/drm/radeon/radeon_drv.c @@ -525,6 +525,16 @@ static const struct file_operations radeon_driver_kms_fops = { #endif };
+static bool +radeon_get_crtc_scanout_position(struct drm_device *dev, unsigned int pipe, + bool in_vblank_irq, int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode) +{ + return radeon_get_crtc_scanoutpos(dev, pipe, 0, vpos, hpos, + stime, etime, mode); +} + static struct drm_driver kms_driver = { .driver_features = DRIVER_USE_AGP | @@ -540,7 +550,7 @@ static struct drm_driver kms_driver = { .enable_vblank = radeon_enable_vblank_kms, .disable_vblank = radeon_disable_vblank_kms, .get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos, - .get_scanout_position = radeon_get_crtc_scanoutpos, + .get_scanout_position = radeon_get_crtc_scanout_position, .irq_preinstall = radeon_driver_irq_preinstall_kms, .irq_postinstall = radeon_driver_irq_postinstall_kms, .irq_uninstall = radeon_driver_irq_uninstall_kms, diff --git a/drivers/gpu/drm/radeon/radeon_mode.h b/drivers/gpu/drm/radeon/radeon_mode.h index ad282648fc8b..00f5ec5c12c7 100644 --- a/drivers/gpu/drm/radeon/radeon_mode.h +++ b/drivers/gpu/drm/radeon/radeon_mode.h @@ -691,6 +691,9 @@ struct atom_voltage_table };
/* Driver internal use only flags of radeon_get_crtc_scanoutpos() */ +#define DRM_SCANOUTPOS_VALID (1 << 0) +#define DRM_SCANOUTPOS_IN_VBLANK (1 << 1) +#define DRM_SCANOUTPOS_ACCURATE (1 << 2) #define USE_REAL_VBLANKSTART (1 << 30) #define GET_DISTANCE_TO_VBLANKSTART (1 << 31)
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c index 2567d6c9a4ee..e871862a21b8 100644 --- a/drivers/gpu/drm/vc4/vc4_crtc.c +++ b/drivers/gpu/drm/vc4/vc4_crtc.c @@ -151,10 +151,10 @@ int vc4_crtc_debugfs_regs(struct seq_file *m, void *unused) } #endif
-int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, - unsigned int flags, int *vpos, int *hpos, - ktime_t *stime, ktime_t *etime, - const struct drm_display_mode *mode) +bool vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, + bool in_vblank_irq, int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode) { struct vc4_dev *vc4 = to_vc4_dev(dev); struct drm_crtc *crtc = drm_crtc_from_index(dev, crtc_id); @@ -162,7 +162,7 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, u32 val; int fifo_lines; int vblank_lines; - int ret = 0; + bool ret = false;
/* preempt_disable_rt() should go right here in PREEMPT_RT patchset. */
@@ -198,7 +198,7 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, fifo_lines = vc4_crtc->cob_size / mode->crtc_hdisplay;
if (fifo_lines > 0) - ret |= DRM_SCANOUTPOS_VALID; + ret = true;
/* HVS more than fifo_lines into frame for compositing? */ if (*vpos > fifo_lines) { @@ -216,7 +216,6 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, */ *vpos -= fifo_lines + 1;
- ret |= DRM_SCANOUTPOS_ACCURATE; return ret; }
@@ -229,10 +228,9 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, * We can't get meaningful readings wrt. scanline position of the PV * and need to make things up in a approximative but consistent way. */ - ret |= DRM_SCANOUTPOS_IN_VBLANK; vblank_lines = mode->vtotal - mode->vdisplay;
- if (flags & DRM_CALLED_FROM_VBLIRQ) { + if (in_vblank_irq) { /* * Assume the irq handler got called close to first * line of vblank, so PV has about a full vblank @@ -254,9 +252,10 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, * we are at the very beginning of vblank, as the hvs just * started refilling, and the stime and etime timestamps * truly correspond to start of vblank. + * + * Unfortunately there's no way to report this to upper levels + * and make it more useful. */ - if ((val & SCALER_DISPSTATX_FULL) != SCALER_DISPSTATX_FULL) - ret |= DRM_SCANOUTPOS_ACCURATE; } else { /* * No clue where we are inside vblank. Return a vpos of zero, diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h index 64c92e0eb8f7..c34a0915e49d 100644 --- a/drivers/gpu/drm/vc4/vc4_drv.h +++ b/drivers/gpu/drm/vc4/vc4_drv.h @@ -446,10 +446,10 @@ int vc4_bo_stats_debugfs(struct seq_file *m, void *arg); extern struct platform_driver vc4_crtc_driver; bool vc4_event_pending(struct drm_crtc *crtc); int vc4_crtc_debugfs_regs(struct seq_file *m, void *arg); -int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, - unsigned int flags, int *vpos, int *hpos, - ktime_t *stime, ktime_t *etime, - const struct drm_display_mode *mode); +bool vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id, + bool in_vblank_irq, int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode);
/* vc4_debugfs.c */ int vc4_debugfs_init(struct drm_minor *minor); diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 6304de7e041e..952c080e6b28 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -320,14 +320,6 @@ struct pci_controller; #define DRM_IF_VERSION(maj, min) (maj << 16 | min)
-/* Flags and return codes for get_vblank_timestamp() driver function. */ -#define DRM_CALLED_FROM_VBLIRQ 1 - -/* get_scanout_position() return flags */ -#define DRM_SCANOUTPOS_VALID (1 << 0) -#define DRM_SCANOUTPOS_IN_VBLANK (1 << 1) -#define DRM_SCANOUTPOS_ACCURATE (1 << 2) - /** * DRM device structure. This structure represent a complete card that * may contain multiple heads. diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h index 0a367cf5d8d5..e64e33b9dd26 100644 --- a/include/drm/drm_drv.h +++ b/include/drm/drm_drv.h @@ -241,8 +241,10 @@ struct drm_driver { * DRM device. * pipe: * Id of the crtc to query. - * flags: - * Flags from the caller (DRM_CALLED_FROM_VBLIRQ or 0). + * in_vblank_irq: + * True when called from drm_crtc_handle_vblank(). Some drivers + * need to apply some workarounds for gpu-specific vblank irq quirks + * if flag is set. * vpos: * Target location for current vertical scanout position. * hpos: @@ -263,16 +265,8 @@ struct drm_driver { * * Returns: * - * Flags, or'ed together as follows: - * - * DRM_SCANOUTPOS_VALID: - * Query successful. - * DRM_SCANOUTPOS_INVBL: - * Inside vblank. - * DRM_SCANOUTPOS_ACCURATE: Returned position is accurate. A lack of - * this flag means that returned position may be offset by a - * constant but unknown small number of scanlines wrt. real scanout - * position. + * True on success, false if a reliable scanout position counter could + * not be read out. * * FIXME: * @@ -280,10 +274,10 @@ struct drm_driver { * move it to &struct drm_crtc_helper_funcs, like all the other * helper-internal hooks. */ - int (*get_scanout_position) (struct drm_device *dev, unsigned int pipe, - unsigned int flags, int *vpos, int *hpos, - ktime_t *stime, ktime_t *etime, - const struct drm_display_mode *mode); + bool (*get_scanout_position) (struct drm_device *dev, unsigned int pipe, + bool in_vblank_irq, int *vpos, int *hpos, + ktime_t *stime, ktime_t *etime, + const struct drm_display_mode *mode);
/** * @get_vblank_timestamp: