There's really no reason for anything more:
- Calling this while the crtc vblank stuff isn't set up is a driver
bug. Those places arlready DRM_ERROR.
- Calling this when the crtc is off is either a driver bug (callin
drm_crtc_handle_vblank at the wrong time) or a core bug (for
anything else). Again, we DRM_ERROR.
- EINVAL is checked at higher levels already, and if we'd use struct
drm_crtc * instead of (dev, pipe) it would be real obvious that
those are again core bugs.
The only valid failure mode is crap hardware that couldn't sample a
useful timestamp, to ask the core to just grab a not-so-accurate
timestampe. Bool is perfectly fine for that.
Cc: Mario Kleiner <mario.kleiner@xxxxxxxxxxxxxxxx>
Cc: Eric Anholt <eric@xxxxxxxxxx>
Cc: Rob Clark <robdclark@xxxxxxxxx>
Cc: linux-arm-msm@xxxxxxxxxxxxxxx
Cc: freedreno@xxxxxxxxxxxxxxxxxxxxx
Cc: Alex Deucher <alexander.deucher@xxxxxxx>
Cc: Christian König <christian.koenig@xxxxxxx>
Cc: Ben Skeggs <bskeggs@xxxxxxxxxx>
Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
---
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 8 +++----
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 14 ++++++------
drivers/gpu/drm/drm_irq.c | 37 +++++++++++--------------------
drivers/gpu/drm/i915/i915_irq.c | 8 +++----
drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c | 12 +++++-----
drivers/gpu/drm/nouveau/nouveau_display.c | 4 ++--
drivers/gpu/drm/nouveau/nouveau_display.h | 4 ++--
drivers/gpu/drm/radeon/radeon_drv.c | 8 +++----
drivers/gpu/drm/radeon/radeon_kms.c | 14 ++++++------
drivers/gpu/drm/vc4/vc4_crtc.c | 2 +-
drivers/gpu/drm/vc4/vc4_drv.h | 2 +-
include/drm/drmP.h | 1 -
include/drm/drm_drv.h | 7 +++---
include/drm/drm_irq.h | 10 ++++-----
14 files changed, 59 insertions(+), 72 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index c8860f05bfb9..acd8631d8024 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1768,10 +1768,10 @@ int amdgpu_device_resume(struct drm_device *dev, bool resume, bool fbcon);
u32 amdgpu_get_vblank_counter_kms(struct drm_device *dev, unsigned int pipe);
int amdgpu_enable_vblank_kms(struct drm_device *dev, unsigned int pipe);
void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe);
-int amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
- int *max_error,
- struct timeval *vblank_time,
- unsigned flags);
+bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
+ int *max_error,
+ struct timeval *vblank_time,
+ unsigned flags);
long amdgpu_kms_compat_ioctl(struct file *filp, unsigned int cmd,
unsigned long arg);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 55f951bcc91f..ac42f707c046 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -838,19 +838,19 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe)
*
* Gets the timestamp on the requested crtc based on the
* scanout position. (all asics).
- * Returns postive status flags on success, negative error on failure.
+ * Returns true on success, false on failure.
*/
-int amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
- int *max_error,
- struct timeval *vblank_time,
- unsigned flags)
+bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
+ int *max_error,
+ struct timeval *vblank_time,
+ unsigned flags)
{
struct drm_crtc *crtc;
struct amdgpu_device *adev = dev->dev_private;
if (pipe >= dev->num_crtcs) {
DRM_ERROR("Invalid crtc %u\n", pipe);
- return -EINVAL;
+ return false;
}
/* Get associated drm_crtc: */
@@ -859,7 +859,7 @@ int amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
/* This can occur on driver load if some component fails to
* initialize completely and driver is unloaded */
DRM_ERROR("Uninitialized crtc %d\n", pipe);
- return -EINVAL;
+ return false;
}
/* Helper routine in DRM core does all the work: */
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 4b0c7475ed13..2121ea29e1b2 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -722,43 +722,32 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants);
* active. Higher level code is expected to handle this.
*
* Returns:
- * Negative value on error, failure or if not supported in current
- * video mode:
- *
- * -EINVAL Invalid CRTC.
- * -EAGAIN Temporary unavailable, e.g., called before initial modeset.
- * -ENOTSUPP Function not supported in current display mode.
- * -EIO Failed, e.g., due to failed scanout position query.
- *
- * Returns or'ed positive status flags on success:
- *
- * DRM_VBLANKTIME_SCANOUTPOS_METHOD - Signal this method used for timestamping.
- * DRM_VBLANKTIME_INVBL - Timestamp taken while scanout was in vblank interval.
*
+ * Returns true on success, and false on failure, i.e. when no accurate
+ * timestamp could be acquired.
*/
-int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
- unsigned int pipe,
- int *max_error,
- struct timeval *vblank_time,
- unsigned flags,
- const struct drm_display_mode *mode)
+bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
+ unsigned int pipe,
+ int *max_error,
+ struct timeval *vblank_time,
+ unsigned flags,
+ const struct drm_display_mode *mode)
{
struct timeval tv_etime;
ktime_t stime, etime;
unsigned int vbl_status;
- int ret = DRM_VBLANKTIME_SCANOUTPOS_METHOD;
int vpos, hpos, i;
int delta_ns, duration_ns;
if (pipe >= dev->num_crtcs) {
DRM_ERROR("Invalid crtc %u\n", pipe);
- return -EINVAL;
+ return false;
}
/* Scanout position query not supported? Should not happen. */
if (!dev->driver->get_scanout_position) {
DRM_ERROR("Called from driver w/o get_scanout_position()!?\n");
- return -EIO;
+ return false;
}
/* If mode timing undefined, just return as no-op:
@@ -766,7 +755,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
*/
if (mode->crtc_clock == 0) {
DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n", pipe);
- return -EAGAIN;
+ return false;
}
/* Get current scanout position with system timestamp.
@@ -790,7 +779,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
if (!(vbl_status & DRM_SCANOUTPOS_VALID)) {
DRM_DEBUG("crtc %u : scanoutpos query failed [0x%x].\n",
pipe, vbl_status);
- return -EIO;
+ return false;
}
/* Compute uncertainty in timestamp of scanout position query. */
@@ -834,7 +823,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
(long)vblank_time->tv_sec, (long)vblank_time->tv_usec,
duration_ns/1000, i);
- return ret;
+ return true;
}
EXPORT_SYMBOL(drm_calc_vbltimestamp_from_scanoutpos);
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 26aa1c170b43..df149d159ce7 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -964,7 +964,7 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc)
return position;
}
-static int i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
+static bool i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
int *max_error,
struct timeval *vblank_time,
unsigned flags)
@@ -974,19 +974,19 @@ static int i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
if (pipe >= INTEL_INFO(dev_priv)->num_pipes) {
DRM_ERROR("Invalid crtc %u\n", pipe);
- return -EINVAL;
+ return false;
}
/* Get drm_crtc to timestamp: */
crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
if (crtc == NULL) {
DRM_ERROR("Invalid crtc %u\n", pipe);
- return -EINVAL;
+ return false;
}
if (!crtc->base.hwmode.crtc_clock) {
DRM_DEBUG_KMS("crtc %u is disabled\n", pipe);
- return -EBUSY;
+ return false;
}
/* Helper routine in DRM core does all the work: */
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index 41ccd2a15d3c..655700eb42ba 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -595,23 +595,23 @@ static int mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe,
return ret;
}
-static int mdp5_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
- int *max_error,
- struct timeval *vblank_time,
- unsigned flags)
+static bool mdp5_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
+ int *max_error,
+ struct timeval *vblank_time,
+ unsigned flags)
{
struct msm_drm_private *priv = dev->dev_private;
struct drm_crtc *crtc;
if (pipe < 0 || pipe >= priv->num_crtcs) {
DRM_ERROR("Invalid crtc %d\n", pipe);
- return -EINVAL;
+ return false;
}
crtc = priv->crtcs[pipe];
if (!crtc) {
DRM_ERROR("Invalid crtc %d\n", pipe);
- return -EINVAL;
+ return false;
}
return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 33269c7df30f..42f18b0b9c43 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -156,7 +156,7 @@ nouveau_display_scanoutpos(struct drm_device *dev, unsigned int pipe,
return 0;
}
-int
+bool
nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe,
int *max_error, struct timeval *time, unsigned flags)
{
@@ -174,7 +174,7 @@ nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe,
}
}
- return -EINVAL;
+ return false;
}
static void
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h
index 4a75df06c139..bc9d1e7b0117 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.h
+++ b/drivers/gpu/drm/nouveau/nouveau_display.h
@@ -71,8 +71,8 @@ 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 *);
-int nouveau_display_vblstamp(struct drm_device *, unsigned int, int *,
- struct timeval *, unsigned);
+bool nouveau_display_vblstamp(struct drm_device *, unsigned int, int *,
+ struct timeval *, unsigned);
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 312436a8d9e5..6d79b5c2805b 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -114,10 +114,10 @@ int radeon_resume_kms(struct drm_device *dev, bool resume, bool fbcon);
u32 radeon_get_vblank_counter_kms(struct drm_device *dev, unsigned int pipe);
int radeon_enable_vblank_kms(struct drm_device *dev, unsigned int pipe);
void radeon_disable_vblank_kms(struct drm_device *dev, unsigned int pipe);
-int radeon_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
- int *max_error,
- struct timeval *vblank_time,
- unsigned flags);
+bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
+ int *max_error,
+ struct timeval *vblank_time,
+ unsigned flags);
void radeon_driver_irq_preinstall_kms(struct drm_device *dev);
int radeon_driver_irq_postinstall_kms(struct drm_device *dev);
void radeon_driver_irq_uninstall_kms(struct drm_device *dev);
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index a32a62e03a44..9afe72010c64 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -868,25 +868,25 @@ void radeon_disable_vblank_kms(struct drm_device *dev, int crtc)
*
* Gets the timestamp on the requested crtc based on the
* scanout position. (all asics).
- * Returns postive status flags on success, negative error on failure.
+ * Returns true on success, false on failure.
*/
-int radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc,
- int *max_error,
- struct timeval *vblank_time,
- unsigned flags)
+bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc,
+ int *max_error,
+ struct timeval *vblank_time,
+ unsigned flags)
{
struct drm_crtc *drmcrtc;
struct radeon_device *rdev = dev->dev_private;
if (crtc < 0 || crtc >= dev->num_crtcs) {
DRM_ERROR("Invalid crtc %d\n", crtc);
- return -EINVAL;
+ return false;
}
/* Get associated drm_crtc: */
drmcrtc = &rdev->mode_info.crtcs[crtc]->base;
if (!drmcrtc)
- return -EINVAL;
+ return false;
/* Helper routine in DRM core does all the work: */
return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc, max_error,
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index 24edd0c22cc9..74260990b6bf 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -270,7 +270,7 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id,
return ret;
}
-int vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id,
+bool vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id,
int *max_error, struct timeval *vblank_time,
unsigned flags)
{
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index dffce6293d87..8a5d0e12ee02 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -450,7 +450,7 @@ 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);
-int vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id,
+bool vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id,
int *max_error, struct timeval *vblank_time,
unsigned flags);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 27ecfcdbf752..6304de7e041e 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -322,7 +322,6 @@ struct pci_controller;
/* Flags and return codes for get_vblank_timestamp() driver function. */
#define DRM_CALLED_FROM_VBLIRQ 1
-#define DRM_VBLANKTIME_SCANOUTPOS_METHOD (1 << 0)
/* get_scanout_position() return flags */
#define DRM_SCANOUTPOS_VALID (1 << 0)
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index d0b5f363bfa1..da78e248d9d8 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -316,11 +316,10 @@ struct drm_driver {
*
* Returns:
*
- * Zero if timestamping isn't supported in current display mode or a
- * negative number on failure. A positive status code on success,
- * which describes how the vblank_time timestamp was computed.
+ * True on success, false on failure, which means the core should
+ * fallback to a simple timestamp taken in drm_crtc_handle_vblank().
*/
- int (*get_vblank_timestamp) (struct drm_device *dev, unsigned int pipe,
+ bool (*get_vblank_timestamp) (struct drm_device *dev, unsigned int pipe,
int *max_error,
struct timeval *vblank_time,
unsigned flags);
diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h
index cf0be6594c8c..f0d5ccf9b282 100644
--- a/include/drm/drm_irq.h
+++ b/include/drm/drm_irq.h
@@ -153,11 +153,11 @@ void drm_crtc_vblank_on(struct drm_crtc *crtc);
void drm_vblank_cleanup(struct drm_device *dev);
u32 drm_accurate_vblank_count(struct drm_crtc *crtc);
-int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
- unsigned int pipe, int *max_error,
- struct timeval *vblank_time,
- unsigned flags,
- const struct drm_display_mode *mode);
+bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
+ unsigned int pipe, int *max_error,
+ struct timeval *vblank_time,
+ unsigned flags,
+ const struct drm_display_mode *mode);
void drm_calc_timestamping_constants(struct drm_crtc *crtc,
const struct drm_display_mode *mode);
--
2.11.0
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
