All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] vblanke cleanup resend
@ 2017-05-03  7:26 Daniel Vetter
  2017-05-03  7:26 ` [PATCH 1/5] drm/vblank: Switch drm_driver->get_vblank_timestamp to return a bool Daniel Vetter
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-05-03  7:26 UTC (permalink / raw)
  To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development

Hi all,

Very minor cleanups from Neil's review, and a minimal rebase because nouveau
changed something. Otherwise unchanged.

Still would like an ack from Ville/Mario before merging.

Thanks, Daniel

Daniel Vetter (5):
  drm/vblank: Switch drm_driver->get_vblank_timestamp to return a bool
  drm/vblank: Switch to bool in_vblank_irq in get_vblank_timestamp
  drm/vblank: Add FIXME comments about moving the vblank ts hooks
  drm/vblank: drop the mode argument from
    drm_calc_vbltimestamp_from_scanoutpos
  drm/vblank: Lock down vblank->hwmode more

 drivers/gpu/drm/amd/amdgpu/amdgpu.h       |   4 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   |  14 +++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |  41 ----------
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |   3 +
 drivers/gpu/drm/drm_irq.c                 | 131 +++++++++++++++++-------------
 drivers/gpu/drm/i915/i915_irq.c           |  62 +++-----------
 drivers/gpu/drm/i915/intel_display.c      |  11 +--
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c   |  45 ++--------
 drivers/gpu/drm/nouveau/nouveau_display.c |  39 ++-------
 drivers/gpu/drm/nouveau/nouveau_display.h |   8 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c     |   2 +-
 drivers/gpu/drm/radeon/radeon_drv.c       |  18 ++--
 drivers/gpu/drm/radeon/radeon_kms.c       |  37 ---------
 drivers/gpu/drm/radeon/radeon_mode.h      |   3 +
 drivers/gpu/drm/vc4/vc4_crtc.c            |  34 +++-----
 drivers/gpu/drm/vc4/vc4_drv.c             |   2 +-
 drivers/gpu/drm/vc4/vc4_drv.h             |  11 +--
 include/drm/drmP.h                        |   9 --
 include/drm/drm_drv.h                     |  52 ++++++------
 include/drm/drm_irq.h                     |  21 +++--
 20 files changed, 196 insertions(+), 351 deletions(-)

-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 1/5] drm/vblank: Switch drm_driver->get_vblank_timestamp to return a bool
  2017-05-03  7:26 [PATCH 0/5] vblanke cleanup resend Daniel Vetter
@ 2017-05-03  7:26 ` Daniel Vetter
  2017-05-03  7:26 ` [PATCH 2/5] drm/vblank: Switch to bool in_vblank_irq in get_vblank_timestamp Daniel Vetter
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-05-03  7:26 UTC (permalink / raw)
  To: DRI Development
  Cc: Daniel Vetter, Intel Graphics Development, Ben Skeggs,
	Mario Kleiner, linux-arm-msm, Alex Deucher, Daniel Vetter,
	freedreno, Christian König

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 alrready DRM_ERROR.
- Calling this when the crtc is off is either a driver bug (calling
  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
timestamp. Bool is perfectly fine for that.

v2: Also fix up the one caller, I lost that in the shuffling (Jani).

v3: Fixup commit message (Neil).

Cc: Jani Nikula <jani.nikula@linux.intel.com>
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>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  8 ++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   | 14 ++++-----
 drivers/gpu/drm/drm_irq.c                 | 49 ++++++++++++-------------------
 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, 64 insertions(+), 79 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 6a8129949333..7b4f808afff9 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1910,10 +1910,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 832be632478f..a1b97809305e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -945,19 +945,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: */
@@ -966,7 +966,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 8c866cac62dd..677b37b0372b 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -724,43 +724,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:
@@ -768,7 +757,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.
@@ -792,7 +781,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. */
@@ -836,7 +825,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);
 
@@ -872,25 +861,23 @@ static bool
 drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe,
 			  struct timeval *tvblank, unsigned flags)
 {
-	int ret;
+	bool ret = false;
 
 	/* Define requested maximum error on timestamps (nanoseconds). */
 	int max_error = (int) drm_timestamp_precision * 1000;
 
 	/* Query driver if possible and precision timestamping enabled. */
-	if (dev->driver->get_vblank_timestamp && (max_error > 0)) {
+	if (dev->driver->get_vblank_timestamp && (max_error > 0))
 		ret = dev->driver->get_vblank_timestamp(dev, pipe, &max_error,
 							tvblank, flags);
-		if (ret > 0)
-			return true;
-	}
 
 	/* GPU high precision timestamp query unsupported or failed.
 	 * Return current monotonic/gettimeofday timestamp as best estimate.
 	 */
-	*tvblank = get_drm_timestamp();
+	if (!ret)
+		*tvblank = get_drm_timestamp();
 
-	return false;
+	return ret;
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index fb88fe862e12..126e6d903c9b 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 d3d6b4cae1e6..ffeb34bee3af 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -592,23 +592,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 21b10f9840c9..f1e36f70755d 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 e1d772d39488..c6bfe533a641 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 93d45aa5c3d4..caa0b1cc4269 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -115,10 +115,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 e3e7cb1d10a2..535969d74f64 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -869,25 +869,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 d86c8cce3182..f506525a1066 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 b0967e2f7e88..71957936b2f5 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -492,7 +492,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 e1daa4f343cd..a1b19bf45fb3 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

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 2/5] drm/vblank: Switch to bool in_vblank_irq in get_vblank_timestamp
  2017-05-03  7:26 [PATCH 0/5] vblanke cleanup resend Daniel Vetter
  2017-05-03  7:26 ` [PATCH 1/5] drm/vblank: Switch drm_driver->get_vblank_timestamp to return a bool Daniel Vetter
@ 2017-05-03  7:26 ` Daniel Vetter
  2017-05-04 10:08   ` kbuild test robot
  2017-05-03  7:26 ` [PATCH 3/5] drm/vblank: Add FIXME comments about moving the vblank ts hooks Daniel Vetter
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2017-05-03  7:26 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Mario Kleiner,
	Eric Anholt, Rob Clark, linux-arm-msm, freedreno, Alex Deucher,
	Christian König, Ben Skeggs, Daniel Vetter

It's overkill to have a flag parameter which is essentially used just
as a boolean. This takes care of core + adjusting drivers.

Adjusting the scanout position callback is a bit harder, since radeon
also supplies it's own driver-private flags in there.

v2: Fixup misplaced hunks (Neil).

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>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |  6 ++---
 drivers/gpu/drm/drm_irq.c                 | 41 +++++++++++++++++--------------
 drivers/gpu/drm/i915/i915_irq.c           |  4 +--
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c   |  4 +--
 drivers/gpu/drm/nouveau/nouveau_display.c |  5 ++--
 drivers/gpu/drm/nouveau/nouveau_display.h |  2 +-
 drivers/gpu/drm/radeon/radeon_drv.c       |  2 +-
 drivers/gpu/drm/radeon/radeon_kms.c       |  4 +--
 drivers/gpu/drm/vc4/vc4_crtc.c            |  6 ++---
 drivers/gpu/drm/vc4/vc4_drv.h             |  2 +-
 include/drm/drm_drv.h                     | 17 +++++++------
 include/drm/drm_irq.h                     |  2 +-
 13 files changed, 51 insertions(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7b4f808afff9..0ce8292d97c0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1913,7 +1913,7 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe);
 bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
 				     int *max_error,
 				     struct timeval *vblank_time,
-				     unsigned flags);
+				     bool in_vblank_irq);
 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 a1b97809305e..babd969a63d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -941,7 +941,7 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe)
  * @crtc: crtc to get the timestamp for
  * @max_error: max error
  * @vblank_time: time value
- * @flags: flags passed to the driver
+ * @in_vblank_irq: called from drm_handle_vblank()
  *
  * Gets the timestamp on the requested crtc based on the
  * scanout position.  (all asics).
@@ -950,7 +950,7 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe)
 bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
 				     int *max_error,
 				     struct timeval *vblank_time,
-				     unsigned flags)
+				     bool in_vblank_irq)
 {
 	struct drm_crtc *crtc;
 	struct amdgpu_device *adev = dev->dev_private;
@@ -971,7 +971,7 @@ bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
 
 	/* Helper routine in DRM core does all the work: */
 	return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
-						     vblank_time, flags,
+						     vblank_time, in_vblank_irq,
 						     &crtc->hwmode);
 }
 
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 677b37b0372b..fba6a842f4cd 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -54,7 +54,7 @@
 
 static bool
 drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe,
-			  struct timeval *tvblank, unsigned flags);
+			  struct timeval *tvblank, bool in_vblank_irq);
 
 static unsigned int drm_timestamp_precision = 20;  /* Default to 20 usecs. */
 
@@ -138,7 +138,7 @@ static void drm_reset_vblank_timestamp(struct drm_device *dev, unsigned int pipe
 	 */
 	do {
 		cur_vblank = __get_vblank_counter(dev, pipe);
-		rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, 0);
+		rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, false);
 	} while (cur_vblank != __get_vblank_counter(dev, pipe) && --count > 0);
 
 	/*
@@ -171,7 +171,7 @@ static void drm_reset_vblank_timestamp(struct drm_device *dev, unsigned int pipe
  * device vblank fields.
  */
 static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
-				    unsigned long flags)
+				    bool in_vblank_irq)
 {
 	struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
 	u32 cur_vblank, diff;
@@ -194,7 +194,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
 	 */
 	do {
 		cur_vblank = __get_vblank_counter(dev, pipe);
-		rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, flags);
+		rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, in_vblank_irq);
 	} while (cur_vblank != __get_vblank_counter(dev, pipe) && --count > 0);
 
 	if (dev->max_vblank_count != 0) {
@@ -214,13 +214,13 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
 		 */
 		diff = DIV_ROUND_CLOSEST_ULL(diff_ns, framedur_ns);
 
-		if (diff == 0 && flags & DRM_CALLED_FROM_VBLIRQ)
+		if (diff == 0 && in_vblank_irq)
 			DRM_DEBUG_VBL("crtc %u: Redundant vblirq ignored."
 				      " diff_ns = %lld, framedur_ns = %d)\n",
 				      pipe, (long long) diff_ns, framedur_ns);
 	} else {
 		/* some kind of default for drivers w/o accurate vbl timestamping */
-		diff = (flags & DRM_CALLED_FROM_VBLIRQ) != 0;
+		diff = in_vblank_irq ? 1 : 0;
 	}
 
 	/*
@@ -253,7 +253,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
 	 * Otherwise reinitialize delayed at next vblank interrupt and assign 0
 	 * for now, to mark the vblanktimestamp as invalid.
 	 */
-	if (!rc && (flags & DRM_CALLED_FROM_VBLIRQ) == 0)
+	if (!rc && in_vblank_irq)
 		t_vblank = (struct timeval) {0, 0};
 
 	store_vblank(dev, pipe, diff, &t_vblank, cur_vblank);
@@ -291,7 +291,7 @@ u32 drm_accurate_vblank_count(struct drm_crtc *crtc)
 
 	spin_lock_irqsave(&dev->vblank_time_lock, flags);
 
-	drm_update_vblank_count(dev, pipe, 0);
+	drm_update_vblank_count(dev, pipe, false);
 	vblank = drm_vblank_count(dev, pipe);
 
 	spin_unlock_irqrestore(&dev->vblank_time_lock, flags);
@@ -349,7 +349,7 @@ static void vblank_disable_and_save(struct drm_device *dev, unsigned int pipe)
 	 * this time. This makes the count account for the entire time
 	 * between drm_crtc_vblank_on() and drm_crtc_vblank_off().
 	 */
-	drm_update_vblank_count(dev, pipe, 0);
+	drm_update_vblank_count(dev, pipe, false);
 
 	spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
 }
@@ -700,9 +700,10 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants);
  * @max_error: Desired maximum allowable error in timestamps (nanosecs)
  *             On return contains true maximum error of timestamp
  * @vblank_time: Pointer to struct timeval which should receive the timestamp
- * @flags: Flags to pass to driver:
- *         0 = Default,
- *         DRM_CALLED_FROM_VBLIRQ = If function is called from vbl IRQ handler
+ * @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.
  * @mode: mode which defines the scanout timings
  *
  * Implements calculation of exact vblank timestamps from given drm_display_mode
@@ -732,7 +733,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 					   unsigned int pipe,
 					   int *max_error,
 					   struct timeval *vblank_time,
-					   unsigned flags,
+					   bool in_vblank_irq,
 					   const struct drm_display_mode *mode)
 {
 	struct timeval tv_etime;
@@ -740,6 +741,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 	unsigned int vbl_status;
 	int vpos, hpos, i;
 	int delta_ns, duration_ns;
+	unsigned flags = in_vblank_irq ? DRM_CALLED_FROM_VBLIRQ : 0;
 
 	if (pipe >= dev->num_crtcs) {
 		DRM_ERROR("Invalid crtc %u\n", pipe);
@@ -843,9 +845,10 @@ static struct timeval get_drm_timestamp(void)
  * @dev: DRM device
  * @pipe: index of CRTC whose vblank timestamp to retrieve
  * @tvblank: Pointer to target struct timeval which should receive the timestamp
- * @flags: Flags to pass to driver:
- *         0 = Default,
- *         DRM_CALLED_FROM_VBLIRQ = If function is called from vbl IRQ handler
+ * @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.
  *
  * Fetches the system timestamp corresponding to the time of the most recent
  * vblank interval on specified CRTC. May call into kms-driver to
@@ -859,7 +862,7 @@ static struct timeval get_drm_timestamp(void)
  */
 static bool
 drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe,
-			  struct timeval *tvblank, unsigned flags)
+			  struct timeval *tvblank, bool in_vblank_irq)
 {
 	bool ret = false;
 
@@ -869,7 +872,7 @@ drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe,
 	/* Query driver if possible and precision timestamping enabled. */
 	if (dev->driver->get_vblank_timestamp && (max_error > 0))
 		ret = dev->driver->get_vblank_timestamp(dev, pipe, &max_error,
-							tvblank, flags);
+							tvblank, in_vblank_irq);
 
 	/* GPU high precision timestamp query unsupported or failed.
 	 * Return current monotonic/gettimeofday timestamp as best estimate.
@@ -1747,7 +1750,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe)
 		return false;
 	}
 
-	drm_update_vblank_count(dev, pipe, DRM_CALLED_FROM_VBLIRQ);
+	drm_update_vblank_count(dev, pipe, true);
 
 	spin_unlock(&dev->vblank_time_lock);
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 126e6d903c9b..b0e38cc85c98 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -967,7 +967,7 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc)
 static bool i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
 			      int *max_error,
 			      struct timeval *vblank_time,
-			      unsigned flags)
+			      bool in_vblank_irq)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *crtc;
@@ -991,7 +991,7 @@ static bool i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
 
 	/* Helper routine in DRM core does all the work: */
 	return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
-						     vblank_time, flags,
+						     vblank_time, in_vblank_irq,
 						     &crtc->base.hwmode);
 }
 
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index ffeb34bee3af..07e2b1335f65 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -595,7 +595,7 @@ static int mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe,
 static bool mdp5_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
 				      int *max_error,
 				      struct timeval *vblank_time,
-				      unsigned flags)
+				      bool in_vblank_irq)
 {
 	struct msm_drm_private *priv = dev->dev_private;
 	struct drm_crtc *crtc;
@@ -612,7 +612,7 @@ static bool mdp5_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
 	}
 
 	return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
-						     vblank_time, flags,
+						     vblank_time, in_vblank_irq,
 						     &crtc->mode);
 }
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index f1e36f70755d..2d28ef57f2bf 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -158,7 +158,7 @@ nouveau_display_scanoutpos(struct drm_device *dev, unsigned int pipe,
 
 bool
 nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe,
-			 int *max_error, struct timeval *time, unsigned flags)
+			 int *max_error, struct timeval *time, bool in_vblank_irq)
 {
 	struct drm_crtc *crtc;
 
@@ -170,7 +170,8 @@ nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe,
 			else
 				mode = &crtc->hwmode;
 			return drm_calc_vbltimestamp_from_scanoutpos(dev,
-					pipe, max_error, time, flags, mode);
+					pipe, max_error, time, in_vblank_irq,
+					mode);
 		}
 	}
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h
index c6bfe533a641..8ec86259c5ac 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.h
+++ b/drivers/gpu/drm/nouveau/nouveau_display.h
@@ -72,7 +72,7 @@ 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_vblstamp(struct drm_device *, unsigned int, int *,
-			       struct timeval *, unsigned);
+			       struct timeval *, bool);
 
 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 caa0b1cc4269..88fc791ec8fb 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -118,7 +118,7 @@ void radeon_disable_vblank_kms(struct drm_device *dev, unsigned int pipe);
 bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
 				     int *max_error,
 				     struct timeval *vblank_time,
-				     unsigned flags);
+				     bool in_vblank_irq);
 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 535969d74f64..5bccdeae0773 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -874,7 +874,7 @@ void radeon_disable_vblank_kms(struct drm_device *dev, int crtc)
 bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc,
 				     int *max_error,
 				     struct timeval *vblank_time,
-				     unsigned flags)
+				     bool in_vblank_irq)
 {
 	struct drm_crtc *drmcrtc;
 	struct radeon_device *rdev = dev->dev_private;
@@ -891,7 +891,7 @@ bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc,
 
 	/* Helper routine in DRM core does all the work: */
 	return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc, max_error,
-						     vblank_time, flags,
+						     vblank_time, in_vblank_irq,
 						     &drmcrtc->hwmode);
 }
 
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index f506525a1066..310c155e1be0 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -232,7 +232,7 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id,
 	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
@@ -272,14 +272,14 @@ int vc4_crtc_get_scanoutpos(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)
+				  bool in_vblank_irq)
 {
 	struct drm_crtc *crtc = drm_crtc_from_index(dev, crtc_id);
 	struct drm_crtc_state *state = crtc->state;
 
 	/* Helper routine in DRM core does all the work: */
 	return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc_id, max_error,
-						     vblank_time, flags,
+						     vblank_time, in_vblank_irq,
 						     &state->adjusted_mode);
 }
 
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 71957936b2f5..9c86370df0c7 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -494,7 +494,7 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id,
 			    const struct drm_display_mode *mode);
 bool vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id,
 				  int *max_error, struct timeval *vblank_time,
-				  unsigned flags);
+				  bool in_vblank_irq);
 
 /* vc4_debugfs.c */
 int vc4_debugfs_init(struct drm_minor *minor);
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index da78e248d9d8..a97dbc1eeccd 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:
@@ -308,11 +310,10 @@ struct drm_driver {
 	 *     Returns true upper bound on error for timestamp.
 	 * vblank_time:
 	 *     Target location for returned vblank timestamp.
-	 * flags:
-	 *     0 = Defaults, no special treatment needed.
-	 *     DRM_CALLED_FROM_VBLIRQ = Function is called from vblank
-	 *     irq handler. Some drivers need to apply some workarounds
-	 *     for gpu-specific vblank irq quirks if flag is set.
+	 * 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.
 	 *
 	 * Returns:
 	 *
@@ -322,7 +323,7 @@ struct drm_driver {
 	bool (*get_vblank_timestamp) (struct drm_device *dev, unsigned int pipe,
 				     int *max_error,
 				     struct timeval *vblank_time,
-				     unsigned flags);
+				     bool in_vblank_irq);
 
 	/* these have to be filled in */
 
diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h
index f0d5ccf9b282..445406efb8dc 100644
--- a/include/drm/drm_irq.h
+++ b/include/drm/drm_irq.h
@@ -156,7 +156,7 @@ u32 drm_accurate_vblank_count(struct drm_crtc *crtc);
 bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 					   unsigned int pipe, int *max_error,
 					   struct timeval *vblank_time,
-					   unsigned flags,
+					   bool in_vblank_irq,
 					   const struct drm_display_mode *mode);
 void drm_calc_timestamping_constants(struct drm_crtc *crtc,
 				     const struct drm_display_mode *mode);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 3/5] drm/vblank: Add FIXME comments about moving the vblank ts hooks
  2017-05-03  7:26 [PATCH 0/5] vblanke cleanup resend Daniel Vetter
  2017-05-03  7:26 ` [PATCH 1/5] drm/vblank: Switch drm_driver->get_vblank_timestamp to return a bool Daniel Vetter
  2017-05-03  7:26 ` [PATCH 2/5] drm/vblank: Switch to bool in_vblank_irq in get_vblank_timestamp Daniel Vetter
@ 2017-05-03  7:26 ` Daniel Vetter
  2017-05-03  7:26 ` [PATCH 4/5] drm/vblank: drop the mode argument from drm_calc_vbltimestamp_from_scanoutpos Daniel Vetter
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-05-03  7:26 UTC (permalink / raw)
  To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter

This is going to be a bit too much, but good to have at least a small
note about where this should all head towards.

Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 include/drm/drm_drv.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index a97dbc1eeccd..619da98533cd 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -276,6 +276,11 @@ struct drm_driver {
 	 *     constant but unknown small number of scanlines wrt. real scanout
 	 *     position.
 	 *
+	 * FIXME:
+	 *
+	 * Since this is a helper to implement @get_vblank_timestamp, we should
+	 * 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,
@@ -319,6 +324,11 @@ struct drm_driver {
 	 *
 	 * True on success, false on failure, which means the core should
 	 * fallback to a simple timestamp taken in drm_crtc_handle_vblank().
+	 *
+	 * FIXME:
+	 *
+	 * We should move this hook to &struct drm_crtc_funcs like all the other
+	 * vblank hooks.
 	 */
 	bool (*get_vblank_timestamp) (struct drm_device *dev, unsigned int pipe,
 				     int *max_error,
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 4/5] drm/vblank: drop the mode argument from drm_calc_vbltimestamp_from_scanoutpos
  2017-05-03  7:26 [PATCH 0/5] vblanke cleanup resend Daniel Vetter
                   ` (2 preceding siblings ...)
  2017-05-03  7:26 ` [PATCH 3/5] drm/vblank: Add FIXME comments about moving the vblank ts hooks Daniel Vetter
@ 2017-05-03  7:26 ` Daniel Vetter
  2017-05-03  7:26 ` [PATCH 5/5] drm/vblank: Lock down vblank->hwmode more Daniel Vetter
  2017-05-03  7:46 ` ✓ Fi.CI.BAT: success for vblanke cleanup resend Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-05-03  7:26 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Mario Kleiner,
	Eric Anholt, Rob Clark, linux-arm-msm, freedreno, Alex Deucher,
	Christian König, Ben Skeggs, Daniel Vetter

If we restrict this helper to only kms drivers (which is the case) we
can look up the correct mode easily ourselves. But it's a bit tricky:

- All legacy drivers look at crtc->hwmode. But that is updated already
  at the beginning of the modeset helper, which means when we disable
  a pipe. Hence the final timestamps might be a bit off. But since
  this is an existing bug I'm not going to change it, but just try to
  be bug-for-bug compatible with the current code. This only applies
  to radeon&amdgpu.

- i915 tries to get it perfect by updating crtc->hwmode when the pipe
  is off (i.e. vblank->enabled = false).

- All other atomic drivers look at crtc->state->adjusted_mode. Those
  that look at state->requested_mode simply don't adjust their mode,
  so it's the same. That has two problems: Accessing crtc->state from
  interrupt handling code is unsafe, and it's updated before we shut
  down the pipe. For nonblocking modesets it's even worse.

For atomic drivers try to implement what i915 does. To do that we add
a new hwmode field to the vblank structure, and update it from
drm_calc_timestamping_constants(). For atomic drivers that's called
from the right spot by the helper library already, so all fine. But
for safety let's enforce that.

For legacy driver this function is only called at the end (oh the
fun), which is broken, so again let's not bother and just stay
bug-for-bug compatible.

The  benefit is that we can use drm_calc_vbltimestamp_from_scanoutpos
directly to implement ->get_vblank_timestamp in every driver, deleting
a lot of code.

v2: Completely new approach, trying to mimick the i915 solution.

v3: Fixup kerneldoc.

v4: Drop the WARN_ON to check that the vblank is off, atomic helpers
currently unconditionally call this. Recomputing the same stuff should
be harmless.

v5: Fix typos and move misplaced hunks to the right patches (Neil).

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>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  4 ---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c   | 14 +++++++--
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   | 41 ------------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h  |  3 ++
 drivers/gpu/drm/drm_irq.c                 | 43 ++++++++++++++++++-------
 drivers/gpu/drm/i915/i915_irq.c           | 52 +++++--------------------------
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c   | 45 +++++---------------------
 drivers/gpu/drm/nouveau/nouveau_display.c | 38 +++++-----------------
 drivers/gpu/drm/nouveau/nouveau_display.h |  8 ++---
 drivers/gpu/drm/nouveau/nouveau_drm.c     |  2 +-
 drivers/gpu/drm/radeon/radeon_drv.c       | 18 +++++++----
 drivers/gpu/drm/radeon/radeon_kms.c       | 37 ----------------------
 drivers/gpu/drm/radeon/radeon_mode.h      |  3 ++
 drivers/gpu/drm/vc4/vc4_crtc.c            | 32 ++++++-------------
 drivers/gpu/drm/vc4/vc4_drv.c             |  2 +-
 drivers/gpu/drm/vc4/vc4_drv.h             | 11 +++----
 include/drm/drmP.h                        |  8 -----
 include/drm/drm_drv.h                     | 20 ++++--------
 include/drm/drm_irq.h                     | 15 +++++++--
 19 files changed, 120 insertions(+), 276 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 0ce8292d97c0..9de615bb0c2e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1910,10 +1910,6 @@ 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);
-bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
-				     int *max_error,
-				     struct timeval *vblank_time,
-				     bool in_vblank_irq);
 long amdgpu_kms_compat_ioctl(struct file *filp, unsigned int cmd,
 			     unsigned long arg);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 4e0f7d2d87f1..73e982cd6136 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -711,6 +711,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 |
@@ -725,8 +735,8 @@ static struct drm_driver kms_driver = {
 	.get_vblank_counter = amdgpu_get_vblank_counter_kms,
 	.enable_vblank = amdgpu_enable_vblank_kms,
 	.disable_vblank = amdgpu_disable_vblank_kms,
-	.get_vblank_timestamp = amdgpu_get_vblank_timestamp_kms,
-	.get_scanout_position = amdgpu_get_crtc_scanoutpos,
+	.get_vblank_timestamp = drm_calc_vbltimestamp_from_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_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index babd969a63d1..40f45ba71b86 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -934,47 +934,6 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe)
 	amdgpu_irq_put(adev, &adev->crtc_irq, idx);
 }
 
-/**
- * amdgpu_get_vblank_timestamp_kms - get vblank timestamp
- *
- * @dev: drm dev pointer
- * @crtc: crtc to get the timestamp for
- * @max_error: max error
- * @vblank_time: time value
- * @in_vblank_irq: called from drm_handle_vblank()
- *
- * Gets the timestamp on the requested crtc based on the
- * scanout position.  (all asics).
- * Returns true on success, false on failure.
- */
-bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
-				     int *max_error,
-				     struct timeval *vblank_time,
-				     bool in_vblank_irq)
-{
-	struct drm_crtc *crtc;
-	struct amdgpu_device *adev = dev->dev_private;
-
-	if (pipe >= dev->num_crtcs) {
-		DRM_ERROR("Invalid crtc %u\n", pipe);
-		return false;
-	}
-
-	/* Get associated drm_crtc: */
-	crtc = &adev->mode_info.crtcs[pipe]->base;
-	if (!crtc) {
-		/* 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 false;
-	}
-
-	/* Helper routine in DRM core does all the work: */
-	return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
-						     vblank_time, in_vblank_irq,
-						     &crtc->hwmode);
-}
-
 const struct drm_ioctl_desc amdgpu_ioctls_kms[] = {
 	DRM_IOCTL_DEF_DRV(AMDGPU_GEM_CREATE, amdgpu_gem_create_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
 	DRM_IOCTL_DEF_DRV(AMDGPU_CTX, amdgpu_ctx_ioctl, DRM_AUTH|DRM_RENDER_ALLOW),
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index db8f8dda209c..20d6522fd7b4 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 fba6a842f4cd..89f0928b042a 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -684,6 +684,7 @@ void drm_calc_timestamping_constants(struct drm_crtc *crtc,
 
 	vblank->linedur_ns  = linedur_ns;
 	vblank->framedur_ns = framedur_ns;
+	vblank->hwmode = *mode;
 
 	DRM_DEBUG("crtc %u: hwmode: htotal %d, vtotal %d, vdisplay %d\n",
 		  crtc->base.id, mode->crtc_htotal,
@@ -704,7 +705,6 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants);
  *     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.
- * @mode: mode which defines the scanout timings
  *
  * Implements calculation of exact vblank timestamps from given drm_display_mode
  * timings and current video scanout position of a CRTC. This can be called from
@@ -724,6 +724,13 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants);
  * returns as no operation if a doublescan or interlaced video mode is
  * active. Higher level code is expected to handle this.
  *
+ * This function can be used to implement the &drm_driver.get_vblank_timestamp
+ * directly, if the driver implements the &drm_driver.get_scanout_position hook.
+ *
+ * Note that atomic drivers must call drm_calc_timestamping_constants() before
+ * enabling a CRTC. The atomic helpers already take care of that in
+ * drm_atomic_helper_update_legacy_modeset_state().
+ *
  * Returns:
  *
  * Returns true on success, and false on failure, i.e. when no accurate
@@ -733,17 +740,23 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 					   unsigned int pipe,
 					   int *max_error,
 					   struct timeval *vblank_time,
-					   bool in_vblank_irq,
-					   const struct drm_display_mode *mode)
+					   bool in_vblank_irq)
 {
 	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 (pipe >= dev->num_crtcs) {
+	if (!drm_core_check_feature(dev, DRIVER_MODESET))
+		return false;
+
+	crtc = drm_crtc_from_index(dev, pipe);
+
+	if (pipe >= dev->num_crtcs || !crtc) {
 		DRM_ERROR("Invalid crtc %u\n", pipe);
 		return false;
 	}
@@ -754,6 +767,11 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 		return false;
 	}
 
+	if (drm_drv_uses_atomic_modeset(dev))
+		mode = &vblank->hwmode;
+	else
+		mode = &crtc->hwmode;
+
 	/* If mode timing undefined, just return as no-op:
 	 * Happens during initial modesetting of a crtc.
 	 */
@@ -774,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;
 		}
 
@@ -821,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 b0e38cc85c98..837e5bc2019a 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)
@@ -964,37 +957,6 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc)
 	return position;
 }
 
-static bool i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
-			      int *max_error,
-			      struct timeval *vblank_time,
-			      bool in_vblank_irq)
-{
-	struct drm_i915_private *dev_priv = to_i915(dev);
-	struct intel_crtc *crtc;
-
-	if (pipe >= INTEL_INFO(dev_priv)->num_pipes) {
-		DRM_ERROR("Invalid crtc %u\n", pipe);
-		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 false;
-	}
-
-	if (!crtc->base.hwmode.crtc_clock) {
-		DRM_DEBUG_KMS("crtc %u is disabled\n", pipe);
-		return false;
-	}
-
-	/* Helper routine in DRM core does all the work: */
-	return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
-						     vblank_time, in_vblank_irq,
-						     &crtc->base.hwmode);
-}
-
 static void ironlake_rps_change_irq_handler(struct drm_i915_private *dev_priv)
 {
 	u32 busy_up, busy_down, max_avg, min_avg;
@@ -4329,7 +4291,7 @@ void intel_irq_init(struct drm_i915_private *dev_priv)
 
 	dev_priv->hotplug.hpd_storm_threshold = HPD_STORM_DEFAULT_THRESHOLD;
 
-	dev->driver->get_vblank_timestamp = i915_get_vblank_timestamp;
+	dev->driver->get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos;
 	dev->driver->get_scanout_position = i915_get_crtc_scanoutpos;
 
 	if (IS_CHERRYVIEW(dev_priv)) {
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index 07e2b1335f65..e2b3346ead48 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -527,31 +527,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;
 
@@ -575,10 +572,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;
 	}
@@ -589,31 +584,7 @@ static int mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe,
 	if (etime)
 		*etime = ktime_get();
 
-	return ret;
-}
-
-static bool mdp5_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
-				      int *max_error,
-				      struct timeval *vblank_time,
-				      bool in_vblank_irq)
-{
-	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 false;
-	}
-
-	crtc = priv->crtcs[pipe];
-	if (!crtc) {
-		DRM_ERROR("Invalid crtc %d\n", pipe);
-		return false;
-	}
-
-	return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
-						     vblank_time, in_vblank_irq,
-						     &crtc->mode);
+	return true;
 }
 
 static u32 mdp5_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
@@ -725,7 +696,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev)
 	dev->mode_config.max_width = 0xffff;
 	dev->mode_config.max_height = 0xffff;
 
-	dev->driver->get_vblank_timestamp = mdp5_get_vblank_timestamp;
+	dev->driver->get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos;
 	dev->driver->get_scanout_position = mdp5_get_scanoutpos;
 	dev->driver->get_vblank_counter = mdp5_get_vblank_counter;
 	dev->max_vblank_count = 0xffffffff;
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index 2d28ef57f2bf..6718c84fb862 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 = 20;
+	int retry = 20;
+	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)
 {
@@ -153,28 +151,6 @@ nouveau_display_scanoutpos(struct drm_device *dev, unsigned int pipe,
 		}
 	}
 
-	return 0;
-}
-
-bool
-nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe,
-			 int *max_error, struct timeval *time, bool in_vblank_irq)
-{
-	struct drm_crtc *crtc;
-
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
-		if (nouveau_crtc(crtc)->index == pipe) {
-			struct drm_display_mode *mode;
-			if (drm_drv_uses_atomic_modeset(dev))
-				mode = &crtc->state->adjusted_mode;
-			else
-				mode = &crtc->hwmode;
-			return drm_calc_vbltimestamp_from_scanoutpos(dev,
-					pipe, max_error, time, in_vblank_irq,
-					mode);
-		}
-	}
-
 	return false;
 }
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h
index 8ec86259c5ac..201aec2ea5b8 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.h
+++ b/drivers/gpu/drm/nouveau/nouveau_display.h
@@ -68,11 +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_vblstamp(struct drm_device *, unsigned int, int *,
-			       struct timeval *, bool);
+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/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index ec719df619a6..1f751a3f570c 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -978,7 +978,7 @@ driver_stub = {
 	.enable_vblank = nouveau_display_vblank_enable,
 	.disable_vblank = nouveau_display_vblank_disable,
 	.get_scanout_position = nouveau_display_scanoutpos,
-	.get_vblank_timestamp = nouveau_display_vblstamp,
+	.get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
 
 	.ioctls = nouveau_ioctls,
 	.num_ioctls = ARRAY_SIZE(nouveau_ioctls),
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 88fc791ec8fb..ef8a75940980 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -115,10 +115,6 @@ 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);
-bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
-				     int *max_error,
-				     struct timeval *vblank_time,
-				     bool in_vblank_irq);
 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);
@@ -530,6 +526,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 |
@@ -544,8 +550,8 @@ static struct drm_driver kms_driver = {
 	.get_vblank_counter = radeon_get_vblank_counter_kms,
 	.enable_vblank = radeon_enable_vblank_kms,
 	.disable_vblank = radeon_disable_vblank_kms,
-	.get_vblank_timestamp = radeon_get_vblank_timestamp_kms,
-	.get_scanout_position = radeon_get_crtc_scanoutpos,
+	.get_vblank_timestamp = drm_calc_vbltimestamp_from_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_kms.c b/drivers/gpu/drm/radeon/radeon_kms.c
index 5bccdeae0773..6a68d440bc44 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -858,43 +858,6 @@ void radeon_disable_vblank_kms(struct drm_device *dev, int crtc)
 	spin_unlock_irqrestore(&rdev->irq.lock, irqflags);
 }
 
-/**
- * radeon_get_vblank_timestamp_kms - get vblank timestamp
- *
- * @dev: drm dev pointer
- * @crtc: crtc to get the timestamp for
- * @max_error: max error
- * @vblank_time: time value
- * @flags: flags passed to the driver
- *
- * Gets the timestamp on the requested crtc based on the
- * scanout position.  (all asics).
- * Returns true on success, false on failure.
- */
-bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc,
-				     int *max_error,
-				     struct timeval *vblank_time,
-				     bool in_vblank_irq)
-{
-	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 false;
-	}
-
-	/* Get associated drm_crtc: */
-	drmcrtc = &rdev->mode_info.crtcs[crtc]->base;
-	if (!drmcrtc)
-		return false;
-
-	/* Helper routine in DRM core does all the work: */
-	return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc, max_error,
-						     vblank_time, in_vblank_irq,
-						     &drmcrtc->hwmode);
-}
-
 const struct drm_ioctl_desc radeon_ioctls_kms[] = {
 	DRM_IOCTL_DEF_DRV(RADEON_CP_INIT, drm_invalid_op, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
 	DRM_IOCTL_DEF_DRV(RADEON_CP_START, drm_invalid_op, DRM_AUTH|DRM_MASTER|DRM_ROOT_ONLY),
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 310c155e1be0..1b4dbe9e1c6d 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,7 +228,6 @@ 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 (in_vblank_irq) {
@@ -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,
@@ -270,19 +269,6 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id,
 	return ret;
 }
 
-bool vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id,
-				  int *max_error, struct timeval *vblank_time,
-				  bool in_vblank_irq)
-{
-	struct drm_crtc *crtc = drm_crtc_from_index(dev, crtc_id);
-	struct drm_crtc_state *state = crtc->state;
-
-	/* Helper routine in DRM core does all the work: */
-	return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc_id, max_error,
-						     vblank_time, in_vblank_irq,
-						     &state->adjusted_mode);
-}
-
 static void vc4_crtc_destroy(struct drm_crtc *crtc)
 {
 	drm_crtc_cleanup(crtc);
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 92fb9a41fe7c..5af4670f258b 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -154,7 +154,7 @@ static struct drm_driver vc4_drm_driver = {
 	.irq_uninstall = vc4_irq_uninstall,
 
 	.get_scanout_position = vc4_crtc_get_scanoutpos,
-	.get_vblank_timestamp = vc4_crtc_get_vblank_timestamp,
+	.get_vblank_timestamp = drm_calc_vbltimestamp_from_scanoutpos,
 
 #if defined(CONFIG_DEBUG_FS)
 	.debugfs_init = vc4_debugfs_init,
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index 9c86370df0c7..f19d51f5a8ef 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -488,13 +488,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_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id,
-				  int *max_error, struct timeval *vblank_time,
-				  bool in_vblank_irq);
+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 a1b19bf45fb3..52085832f711 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 619da98533cd..e64e33b9dd26 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -265,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:
 	 *
@@ -282,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:
diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h
index 445406efb8dc..569ca86d4e1f 100644
--- a/include/drm/drm_irq.h
+++ b/include/drm/drm_irq.h
@@ -121,6 +121,18 @@ struct drm_vblank_crtc {
 	 * drm_calc_timestamping_constants().
 	 */
 	int linedur_ns;
+
+	/**
+	 * @hwmode:
+	 *
+	 * Cache of the current hardware display mode. Only valid when @enabled
+	 * is set. This is used by helpers like
+	 * drm_calc_vbltimestamp_from_scanoutpos(). We can't just access the
+	 * hardware mode by e.g. looking at &drm_crtc_state.adjusted_mode,
+	 * because that one is really hard to get from interrupt context.
+	 */
+	struct drm_display_mode hwmode;
+
 	/**
 	 * @enabled: Tracks the enabling state of the corresponding &drm_crtc to
 	 * avoid double-disabling and hence corrupting saved state. Needed by
@@ -156,8 +168,7 @@ u32 drm_accurate_vblank_count(struct drm_crtc *crtc);
 bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 					   unsigned int pipe, int *max_error,
 					   struct timeval *vblank_time,
-					   bool in_vblank_irq,
-					   const struct drm_display_mode *mode);
+					   bool in_vblank_irq);
 void drm_calc_timestamping_constants(struct drm_crtc *crtc,
 				     const struct drm_display_mode *mode);
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH 5/5] drm/vblank: Lock down vblank->hwmode more
  2017-05-03  7:26 [PATCH 0/5] vblanke cleanup resend Daniel Vetter
                   ` (3 preceding siblings ...)
  2017-05-03  7:26 ` [PATCH 4/5] drm/vblank: drop the mode argument from drm_calc_vbltimestamp_from_scanoutpos Daniel Vetter
@ 2017-05-03  7:26 ` Daniel Vetter
  2017-05-03 14:09   ` Ville Syrjälä
  2017-05-03  7:46 ` ✓ Fi.CI.BAT: success for vblanke cleanup resend Patchwork
  5 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2017-05-03  7:26 UTC (permalink / raw)
  To: DRI Development; +Cc: Daniel Vetter, Intel Graphics Development, Daniel Vetter

In the previous patch we've implemented hwmode tracking a la i915 for
the vblank timestamp calculations. But that was just the basic
semantics, i915 has some nice sanity checks to make sure we keep
getting this right. Move them over too.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_irq.c            |  8 +++++++-
 drivers/gpu/drm/i915/i915_irq.c      | 10 ++++++----
 drivers/gpu/drm/i915/intel_display.c | 11 ++---------
 3 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 89f0928b042a..942183a2aa3c 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -775,8 +775,10 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 	/* If mode timing undefined, just return as no-op:
 	 * Happens during initial modesetting of a crtc.
 	 */
-	if (mode->crtc_clock == 0) {
+	if (WARN_ON(mode->crtc_clock == 0)) {
 		DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n", pipe);
+		WARN_ON(drm_drv_uses_atomic_modeset(dev));
+
 		return false;
 	}
 
@@ -1338,6 +1340,10 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
 		send_vblank_event(dev, e, seq, &now);
 	}
 	spin_unlock_irqrestore(&dev->event_lock, irqflags);
+
+	/* Will be reset by the modeset helpers when re-enabling the crtc by
+	 * calling drm_calc_timestamping_constants(). */
+	vblank->hwmode.crtc_clock = 0;
 }
 EXPORT_SYMBOL(drm_crtc_vblank_off);
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 837e5bc2019a..ff18b3fc00d2 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -720,9 +720,7 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	i915_reg_t high_frame, low_frame;
 	u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal;
-	struct intel_crtc *intel_crtc = intel_get_crtc_for_pipe(dev_priv,
-								pipe);
-	const struct drm_display_mode *mode = &intel_crtc->base.hwmode;
+	const struct drm_display_mode *mode = &dev->vblank[pipe].hwmode;
 	unsigned long irqflags;
 
 	htotal = mode->crtc_htotal;
@@ -779,13 +777,17 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	const struct drm_display_mode *mode = &crtc->base.hwmode;
+	const struct drm_display_mode *mode;
+	struct drm_vblank_crtc *vblank;
 	enum pipe pipe = crtc->pipe;
 	int position, vtotal;
 
 	if (!crtc->active)
 		return -1;
 
+	vblank = &crtc->base.dev->vblank[drm_crtc_index(&crtc->base)];
+	mode = &vblank->hwmode;
+
 	vtotal = mode->crtc_vtotal;
 	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
 		vtotal /= 2;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 85b9e2f521a0..a190973daeee 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11443,12 +11443,6 @@ intel_modeset_update_crtc_state(struct drm_atomic_state *state)
 	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
 		to_intel_crtc(crtc)->config = to_intel_crtc_state(new_crtc_state);
 
-		/* Update hwmode for vblank functions */
-		if (new_crtc_state->active)
-			crtc->hwmode = new_crtc_state->adjusted_mode;
-		else
-			crtc->hwmode.crtc_clock = 0;
-
 		/*
 		 * Update legacy state to satisfy fbc code. This can
 		 * be removed when fbc uses the atomic state.
@@ -15425,8 +15419,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			to_intel_crtc_state(crtc->base.state);
 		int pixclk = 0;
 
-		crtc->base.hwmode = crtc_state->base.adjusted_mode;
-
 		memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
 		if (crtc_state->base.active) {
 			intel_mode_from_pipe_config(&crtc->base.mode, crtc_state);
@@ -15456,7 +15448,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
 				pixclk = DIV_ROUND_UP(pixclk * 100, 95);
 
-			drm_calc_timestamping_constants(&crtc->base, &crtc->base.hwmode);
+			drm_calc_timestamping_constants(&crtc->base,
+							&crtc_state->base.adjusted_mode);
 			update_scanline_offset(crtc);
 		}
 
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread

* ✓ Fi.CI.BAT: success for vblanke cleanup resend
  2017-05-03  7:26 [PATCH 0/5] vblanke cleanup resend Daniel Vetter
                   ` (4 preceding siblings ...)
  2017-05-03  7:26 ` [PATCH 5/5] drm/vblank: Lock down vblank->hwmode more Daniel Vetter
@ 2017-05-03  7:46 ` Patchwork
  5 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2017-05-03  7:46 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

== Series Details ==

Series: vblanke cleanup resend
URL   : https://patchwork.freedesktop.org/series/23865/
State : success

== Summary ==

Series 23865v1 vblanke cleanup resend
https://patchwork.freedesktop.org/api/1.0/series/23865/revisions/1/mbox/

Test drv_module_reload:
        Subgroup basic-reload-final:
                dmesg-warn -> PASS       (fi-skl-6770hq) fdo#100248
Test gem_exec_suspend:
        Subgroup basic-s4-devices:
                pass       -> DMESG-WARN (fi-kbl-7560u) fdo#100125
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (fi-byt-j1900) fdo#100652

fdo#100248 https://bugs.freedesktop.org/show_bug.cgi?id=100248
fdo#100125 https://bugs.freedesktop.org/show_bug.cgi?id=100125
fdo#100652 https://bugs.freedesktop.org/show_bug.cgi?id=100652

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time:431s
fi-bdw-gvtdvm    total:278  pass:256  dwarn:8   dfail:0   fail:0   skip:14  time:425s
fi-bsw-n3050     total:278  pass:242  dwarn:0   dfail:0   fail:0   skip:36  time:574s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time:508s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time:547s
fi-byt-j1900     total:278  pass:253  dwarn:1   dfail:0   fail:0   skip:24  time:490s
fi-byt-n2820     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:486s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:411s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time:410s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time:422s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:493s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:475s
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time:459s
fi-kbl-7560u     total:278  pass:267  dwarn:1   dfail:0   fail:0   skip:10  time:569s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:451s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time:578s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time:458s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time:496s
fi-skl-gvtdvm    total:278  pass:265  dwarn:0   dfail:0   fail:0   skip:13  time:429s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time:526s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time:406s

7f027554339633cb4e6a2d3974510aaef1a9e24c drm-tip: 2017y-05m-02d-18h-48m-28s UTC integration manifest
64f37f3 drm/vblank: Lock down vblank->hwmode more
49a1ddb drm/vblank: drop the mode argument from drm_calc_vbltimestamp_from_scanoutpos
65e66d9 drm/vblank: Add FIXME comments about moving the vblank ts hooks
23feacb drm/vblank: Switch to bool in_vblank_irq in get_vblank_timestamp
5a7d301 drm/vblank: Switch drm_driver->get_vblank_timestamp to return a bool

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4606/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] drm/vblank: Lock down vblank->hwmode more
  2017-05-03  7:26 ` [PATCH 5/5] drm/vblank: Lock down vblank->hwmode more Daniel Vetter
@ 2017-05-03 14:09   ` Ville Syrjälä
  2017-05-04 13:20     ` Daniel Vetter
  0 siblings, 1 reply; 12+ messages in thread
From: Ville Syrjälä @ 2017-05-03 14:09 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: Daniel Vetter, Intel Graphics Development, DRI Development

On Wed, May 03, 2017 at 09:26:38AM +0200, Daniel Vetter wrote:
> In the previous patch we've implemented hwmode tracking a la i915 for
> the vblank timestamp calculations. But that was just the basic
> semantics, i915 has some nice sanity checks to make sure we keep
> getting this right. Move them over too.
> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_irq.c            |  8 +++++++-
>  drivers/gpu/drm/i915/i915_irq.c      | 10 ++++++----
>  drivers/gpu/drm/i915/intel_display.c | 11 ++---------
>  3 files changed, 15 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> index 89f0928b042a..942183a2aa3c 100644
> --- a/drivers/gpu/drm/drm_irq.c
> +++ b/drivers/gpu/drm/drm_irq.c
> @@ -775,8 +775,10 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
>  	/* If mode timing undefined, just return as no-op:
>  	 * Happens during initial modesetting of a crtc.
>  	 */
> -	if (mode->crtc_clock == 0) {
> +	if (WARN_ON(mode->crtc_clock == 0)) {
>  		DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n", pipe);
> +		WARN_ON(drm_drv_uses_atomic_modeset(dev));

I would make these _ONCE() otherwise the machine might end up
practically dead.

> +
>  		return false;
>  	}
>  
> @@ -1338,6 +1340,10 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
>  		send_vblank_event(dev, e, seq, &now);
>  	}
>  	spin_unlock_irqrestore(&dev->event_lock, irqflags);
> +
> +	/* Will be reset by the modeset helpers when re-enabling the crtc by
> +	 * calling drm_calc_timestamping_constants(). */
> +	vblank->hwmode.crtc_clock = 0;
>  }
>  EXPORT_SYMBOL(drm_crtc_vblank_off);

Shouldn't we do this in drm_crtc_vblank_reset() as well?

Hmm. Except we call that after drm_calc_timestamping_constants(). I
guess we should be able to move the reset() into
intel_modeset_readout_hw_state(). And possibly move the vblank_on()
call as well?

>  
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 837e5bc2019a..ff18b3fc00d2 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -720,9 +720,7 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	i915_reg_t high_frame, low_frame;
>  	u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal;
> -	struct intel_crtc *intel_crtc = intel_get_crtc_for_pipe(dev_priv,
> -								pipe);
> -	const struct drm_display_mode *mode = &intel_crtc->base.hwmode;
> +	const struct drm_display_mode *mode = &dev->vblank[pipe].hwmode;
>  	unsigned long irqflags;
>  
>  	htotal = mode->crtc_htotal;
> @@ -779,13 +777,17 @@ static int __intel_get_crtc_scanline(struct intel_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	const struct drm_display_mode *mode = &crtc->base.hwmode;
> +	const struct drm_display_mode *mode;
> +	struct drm_vblank_crtc *vblank;
>  	enum pipe pipe = crtc->pipe;
>  	int position, vtotal;
>  
>  	if (!crtc->active)
>  		return -1;
>  
> +	vblank = &crtc->base.dev->vblank[drm_crtc_index(&crtc->base)];
> +	mode = &vblank->hwmode;
> +
>  	vtotal = mode->crtc_vtotal;
>  	if (mode->flags & DRM_MODE_FLAG_INTERLACE)
>  		vtotal /= 2;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 85b9e2f521a0..a190973daeee 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11443,12 +11443,6 @@ intel_modeset_update_crtc_state(struct drm_atomic_state *state)
>  	for_each_new_crtc_in_state(state, crtc, new_crtc_state, i) {
>  		to_intel_crtc(crtc)->config = to_intel_crtc_state(new_crtc_state);
>  
> -		/* Update hwmode for vblank functions */
> -		if (new_crtc_state->active)
> -			crtc->hwmode = new_crtc_state->adjusted_mode;
> -		else
> -			crtc->hwmode.crtc_clock = 0;
> -
>  		/*
>  		 * Update legacy state to satisfy fbc code. This can
>  		 * be removed when fbc uses the atomic state.
> @@ -15425,8 +15419,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  			to_intel_crtc_state(crtc->base.state);
>  		int pixclk = 0;
>  
> -		crtc->base.hwmode = crtc_state->base.adjusted_mode;
> -
>  		memset(&crtc->base.mode, 0, sizeof(crtc->base.mode));
>  		if (crtc_state->base.active) {
>  			intel_mode_from_pipe_config(&crtc->base.mode, crtc_state);
> @@ -15456,7 +15448,8 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  			if (IS_BROADWELL(dev_priv) && crtc_state->ips_enabled)
>  				pixclk = DIV_ROUND_UP(pixclk * 100, 95);
>  
> -			drm_calc_timestamping_constants(&crtc->base, &crtc->base.hwmode);
> +			drm_calc_timestamping_constants(&crtc->base,
> +							&crtc_state->base.adjusted_mode);
>  			update_scanline_offset(crtc);
>  		}
>  
> -- 
> 2.11.0

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 2/5] drm/vblank: Switch to bool in_vblank_irq in get_vblank_timestamp
  2017-05-03  7:26 ` [PATCH 2/5] drm/vblank: Switch to bool in_vblank_irq in get_vblank_timestamp Daniel Vetter
@ 2017-05-04 10:08   ` kbuild test robot
  0 siblings, 0 replies; 12+ messages in thread
From: kbuild test robot @ 2017-05-04 10:08 UTC (permalink / raw)
  Cc: kbuild-all, DRI Development, linux-arm-msm,
	Intel Graphics Development, Ben Skeggs, Mario Kleiner,
	Daniel Vetter, Alex Deucher, Daniel Vetter, freedreno,
	Christian König

[-- Attachment #1: Type: text/plain, Size: 1949 bytes --]

Hi Daniel,

[auto build test ERROR on drm/drm-next]
[also build test ERROR on next-20170503]
[cannot apply to v4.11]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Daniel-Vetter/vblanke-cleanup-resend/20170504-003948
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 6.1.1-9) 6.1.1 20160705
reproduce:
        wget https://raw.githubusercontent.com/01org/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

Note: the linux-review/Daniel-Vetter/vblanke-cleanup-resend/20170504-003948 HEAD 7d42e23d7949707be44be8720a9eb260534aa4dc builds fine.
      It only hurts bisectibility.

All errors (new ones prefixed by >>):

   drivers/gpu//drm/vc4/vc4_crtc.c: In function 'vc4_crtc_get_scanoutpos':
>> drivers/gpu//drm/vc4/vc4_crtc.c:235:6: error: 'in_vblank_irq' undeclared (first use in this function)
     if (in_vblank_irq) {
         ^~~~~~~~~~~~~
   drivers/gpu//drm/vc4/vc4_crtc.c:235:6: note: each undeclared identifier is reported only once for each function it appears in

vim +/in_vblank_irq +235 drivers/gpu//drm/vc4/vc4_crtc.c

   229		 * We can't get meaningful readings wrt. scanline position of the PV
   230		 * and need to make things up in a approximative but consistent way.
   231		 */
   232		ret |= DRM_SCANOUTPOS_IN_VBLANK;
   233		vblank_lines = mode->vtotal - mode->vdisplay;
   234	
 > 235		if (in_vblank_irq) {
   236			/*
   237			 * Assume the irq handler got called close to first
   238			 * line of vblank, so PV has about a full vblank

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 61404 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] drm/vblank: Lock down vblank->hwmode more
  2017-05-03 14:09   ` Ville Syrjälä
@ 2017-05-04 13:20     ` Daniel Vetter
  2017-05-08 11:33       ` Ville Syrjälä
  0 siblings, 1 reply; 12+ messages in thread
From: Daniel Vetter @ 2017-05-04 13:20 UTC (permalink / raw)
  To: Ville Syrjälä
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	Daniel Vetter

On Wed, May 03, 2017 at 05:09:08PM +0300, Ville Syrjälä wrote:
> On Wed, May 03, 2017 at 09:26:38AM +0200, Daniel Vetter wrote:
> > In the previous patch we've implemented hwmode tracking a la i915 for
> > the vblank timestamp calculations. But that was just the basic
> > semantics, i915 has some nice sanity checks to make sure we keep
> > getting this right. Move them over too.
> > 
> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/drm_irq.c            |  8 +++++++-
> >  drivers/gpu/drm/i915/i915_irq.c      | 10 ++++++----
> >  drivers/gpu/drm/i915/intel_display.c | 11 ++---------
> >  3 files changed, 15 insertions(+), 14 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > index 89f0928b042a..942183a2aa3c 100644
> > --- a/drivers/gpu/drm/drm_irq.c
> > +++ b/drivers/gpu/drm/drm_irq.c
> > @@ -775,8 +775,10 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
> >  	/* If mode timing undefined, just return as no-op:
> >  	 * Happens during initial modesetting of a crtc.
> >  	 */
> > -	if (mode->crtc_clock == 0) {
> > +	if (WARN_ON(mode->crtc_clock == 0)) {
> >  		DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n", pipe);
> > +		WARN_ON(drm_drv_uses_atomic_modeset(dev));
> 
> I would make these _ONCE() otherwise the machine might end up
> practically dead.

Will do.

> > +
> >  		return false;
> >  	}
> >  
> > @@ -1338,6 +1340,10 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
> >  		send_vblank_event(dev, e, seq, &now);
> >  	}
> >  	spin_unlock_irqrestore(&dev->event_lock, irqflags);
> > +
> > +	/* Will be reset by the modeset helpers when re-enabling the crtc by
> > +	 * calling drm_calc_timestamping_constants(). */
> > +	vblank->hwmode.crtc_clock = 0;
> >  }
> >  EXPORT_SYMBOL(drm_crtc_vblank_off);
> 
> Shouldn't we do this in drm_crtc_vblank_reset() as well?
> 
> Hmm. Except we call that after drm_calc_timestamping_constants(). I
> guess we should be able to move the reset() into
> intel_modeset_readout_hw_state(). And possibly move the vblank_on()
> call as well?

Yeah, it'd be nice to clean this stuff up some more, but there's also the
problem that legacy and new drivers callc drm_calc_timestamping_constants
at opposite ends of the modeset sequence. Doing more here is a bunch more
work, maybe for the next patche series ...

I don't think we need to call it in _reset, at least at boot-up it should
be 0 already. And for s/r we already shut down the pipe on suspend, so
it's gone through this here.

With the _ONCE nit address (and the build breakage I've introduced in this
version fixed), ack from you on the entire series?

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH 5/5] drm/vblank: Lock down vblank->hwmode more
  2017-05-04 13:20     ` Daniel Vetter
@ 2017-05-08 11:33       ` Ville Syrjälä
  0 siblings, 0 replies; 12+ messages in thread
From: Ville Syrjälä @ 2017-05-08 11:33 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Daniel Vetter, Intel Graphics Development, DRI Development,
	Daniel Vetter

On Thu, May 04, 2017 at 03:20:22PM +0200, Daniel Vetter wrote:
> On Wed, May 03, 2017 at 05:09:08PM +0300, Ville Syrjälä wrote:
> > On Wed, May 03, 2017 at 09:26:38AM +0200, Daniel Vetter wrote:
> > > In the previous patch we've implemented hwmode tracking a la i915 for
> > > the vblank timestamp calculations. But that was just the basic
> > > semantics, i915 has some nice sanity checks to make sure we keep
> > > getting this right. Move them over too.
> > > 
> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_irq.c            |  8 +++++++-
> > >  drivers/gpu/drm/i915/i915_irq.c      | 10 ++++++----
> > >  drivers/gpu/drm/i915/intel_display.c | 11 ++---------
> > >  3 files changed, 15 insertions(+), 14 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
> > > index 89f0928b042a..942183a2aa3c 100644
> > > --- a/drivers/gpu/drm/drm_irq.c
> > > +++ b/drivers/gpu/drm/drm_irq.c
> > > @@ -775,8 +775,10 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
> > >  	/* If mode timing undefined, just return as no-op:
> > >  	 * Happens during initial modesetting of a crtc.
> > >  	 */
> > > -	if (mode->crtc_clock == 0) {
> > > +	if (WARN_ON(mode->crtc_clock == 0)) {
> > >  		DRM_DEBUG("crtc %u: Noop due to uninitialized mode.\n", pipe);
> > > +		WARN_ON(drm_drv_uses_atomic_modeset(dev));
> > 
> > I would make these _ONCE() otherwise the machine might end up
> > practically dead.
> 
> Will do.
> 
> > > +
> > >  		return false;
> > >  	}
> > >  
> > > @@ -1338,6 +1340,10 @@ void drm_crtc_vblank_off(struct drm_crtc *crtc)
> > >  		send_vblank_event(dev, e, seq, &now);
> > >  	}
> > >  	spin_unlock_irqrestore(&dev->event_lock, irqflags);
> > > +
> > > +	/* Will be reset by the modeset helpers when re-enabling the crtc by
> > > +	 * calling drm_calc_timestamping_constants(). */
> > > +	vblank->hwmode.crtc_clock = 0;
> > >  }
> > >  EXPORT_SYMBOL(drm_crtc_vblank_off);
> > 
> > Shouldn't we do this in drm_crtc_vblank_reset() as well?
> > 
> > Hmm. Except we call that after drm_calc_timestamping_constants(). I
> > guess we should be able to move the reset() into
> > intel_modeset_readout_hw_state(). And possibly move the vblank_on()
> > call as well?
> 
> Yeah, it'd be nice to clean this stuff up some more, but there's also the
> problem that legacy and new drivers callc drm_calc_timestamping_constants
> at opposite ends of the modeset sequence. Doing more here is a bunch more
> work, maybe for the next patche series ...
> 
> I don't think we need to call it in _reset, at least at boot-up it should
> be 0 already. And for s/r we already shut down the pipe on suspend, so
> it's gone through this here.

Hmm. Indeed that seems like it should cover it.

> 
> With the _ONCE nit address (and the build breakage I've introduced in this
> version fixed), ack from you on the entire series?

Sure, for the series
Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH 2/5] drm/vblank: Switch to bool in_vblank_irq in get_vblank_timestamp
  2017-05-09 14:03 [PATCH 1/5] drm/vblank: Switch drm_driver->get_vblank_timestamp to return a bool Daniel Vetter
@ 2017-05-09 14:03 ` Daniel Vetter
  0 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2017-05-09 14:03 UTC (permalink / raw)
  To: DRI Development
  Cc: Intel Graphics Development, Daniel Vetter, Mario Kleiner,
	Eric Anholt, Rob Clark, linux-arm-msm, freedreno, Alex Deucher,
	Christian König, Ben Skeggs, Daniel Vetter

It's overkill to have a flag parameter which is essentially used just
as a boolean. This takes care of core + adjusting drivers.

Adjusting the scanout position callback is a bit harder, since radeon
also supplies it's own driver-private flags in there.

v2: Fixup misplaced hunks (Neil).

v3: kbuild says v1 was better ...

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>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h       |  2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c   |  6 ++---
 drivers/gpu/drm/drm_irq.c                 | 41 +++++++++++++++++--------------
 drivers/gpu/drm/i915/i915_irq.c           |  4 +--
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c   |  4 +--
 drivers/gpu/drm/nouveau/nouveau_display.c |  5 ++--
 drivers/gpu/drm/nouveau/nouveau_display.h |  2 +-
 drivers/gpu/drm/radeon/radeon_drv.c       |  2 +-
 drivers/gpu/drm/radeon/radeon_kms.c       |  4 +--
 drivers/gpu/drm/vc4/vc4_crtc.c            |  4 +--
 drivers/gpu/drm/vc4/vc4_drv.h             |  2 +-
 include/drm/drm_drv.h                     | 17 +++++++------
 include/drm/drm_irq.h                     |  2 +-
 13 files changed, 50 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 7b4f808afff9..0ce8292d97c0 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1913,7 +1913,7 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe);
 bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
 				     int *max_error,
 				     struct timeval *vblank_time,
-				     unsigned flags);
+				     bool in_vblank_irq);
 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 a1b97809305e..babd969a63d1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -941,7 +941,7 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe)
  * @crtc: crtc to get the timestamp for
  * @max_error: max error
  * @vblank_time: time value
- * @flags: flags passed to the driver
+ * @in_vblank_irq: called from drm_handle_vblank()
  *
  * Gets the timestamp on the requested crtc based on the
  * scanout position.  (all asics).
@@ -950,7 +950,7 @@ void amdgpu_disable_vblank_kms(struct drm_device *dev, unsigned int pipe)
 bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
 				     int *max_error,
 				     struct timeval *vblank_time,
-				     unsigned flags)
+				     bool in_vblank_irq)
 {
 	struct drm_crtc *crtc;
 	struct amdgpu_device *adev = dev->dev_private;
@@ -971,7 +971,7 @@ bool amdgpu_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
 
 	/* Helper routine in DRM core does all the work: */
 	return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
-						     vblank_time, flags,
+						     vblank_time, in_vblank_irq,
 						     &crtc->hwmode);
 }
 
diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 677b37b0372b..fba6a842f4cd 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -54,7 +54,7 @@
 
 static bool
 drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe,
-			  struct timeval *tvblank, unsigned flags);
+			  struct timeval *tvblank, bool in_vblank_irq);
 
 static unsigned int drm_timestamp_precision = 20;  /* Default to 20 usecs. */
 
@@ -138,7 +138,7 @@ static void drm_reset_vblank_timestamp(struct drm_device *dev, unsigned int pipe
 	 */
 	do {
 		cur_vblank = __get_vblank_counter(dev, pipe);
-		rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, 0);
+		rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, false);
 	} while (cur_vblank != __get_vblank_counter(dev, pipe) && --count > 0);
 
 	/*
@@ -171,7 +171,7 @@ static void drm_reset_vblank_timestamp(struct drm_device *dev, unsigned int pipe
  * device vblank fields.
  */
 static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
-				    unsigned long flags)
+				    bool in_vblank_irq)
 {
 	struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
 	u32 cur_vblank, diff;
@@ -194,7 +194,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
 	 */
 	do {
 		cur_vblank = __get_vblank_counter(dev, pipe);
-		rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, flags);
+		rc = drm_get_last_vbltimestamp(dev, pipe, &t_vblank, in_vblank_irq);
 	} while (cur_vblank != __get_vblank_counter(dev, pipe) && --count > 0);
 
 	if (dev->max_vblank_count != 0) {
@@ -214,13 +214,13 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
 		 */
 		diff = DIV_ROUND_CLOSEST_ULL(diff_ns, framedur_ns);
 
-		if (diff == 0 && flags & DRM_CALLED_FROM_VBLIRQ)
+		if (diff == 0 && in_vblank_irq)
 			DRM_DEBUG_VBL("crtc %u: Redundant vblirq ignored."
 				      " diff_ns = %lld, framedur_ns = %d)\n",
 				      pipe, (long long) diff_ns, framedur_ns);
 	} else {
 		/* some kind of default for drivers w/o accurate vbl timestamping */
-		diff = (flags & DRM_CALLED_FROM_VBLIRQ) != 0;
+		diff = in_vblank_irq ? 1 : 0;
 	}
 
 	/*
@@ -253,7 +253,7 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe,
 	 * Otherwise reinitialize delayed at next vblank interrupt and assign 0
 	 * for now, to mark the vblanktimestamp as invalid.
 	 */
-	if (!rc && (flags & DRM_CALLED_FROM_VBLIRQ) == 0)
+	if (!rc && in_vblank_irq)
 		t_vblank = (struct timeval) {0, 0};
 
 	store_vblank(dev, pipe, diff, &t_vblank, cur_vblank);
@@ -291,7 +291,7 @@ u32 drm_accurate_vblank_count(struct drm_crtc *crtc)
 
 	spin_lock_irqsave(&dev->vblank_time_lock, flags);
 
-	drm_update_vblank_count(dev, pipe, 0);
+	drm_update_vblank_count(dev, pipe, false);
 	vblank = drm_vblank_count(dev, pipe);
 
 	spin_unlock_irqrestore(&dev->vblank_time_lock, flags);
@@ -349,7 +349,7 @@ static void vblank_disable_and_save(struct drm_device *dev, unsigned int pipe)
 	 * this time. This makes the count account for the entire time
 	 * between drm_crtc_vblank_on() and drm_crtc_vblank_off().
 	 */
-	drm_update_vblank_count(dev, pipe, 0);
+	drm_update_vblank_count(dev, pipe, false);
 
 	spin_unlock_irqrestore(&dev->vblank_time_lock, irqflags);
 }
@@ -700,9 +700,10 @@ EXPORT_SYMBOL(drm_calc_timestamping_constants);
  * @max_error: Desired maximum allowable error in timestamps (nanosecs)
  *             On return contains true maximum error of timestamp
  * @vblank_time: Pointer to struct timeval which should receive the timestamp
- * @flags: Flags to pass to driver:
- *         0 = Default,
- *         DRM_CALLED_FROM_VBLIRQ = If function is called from vbl IRQ handler
+ * @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.
  * @mode: mode which defines the scanout timings
  *
  * Implements calculation of exact vblank timestamps from given drm_display_mode
@@ -732,7 +733,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 					   unsigned int pipe,
 					   int *max_error,
 					   struct timeval *vblank_time,
-					   unsigned flags,
+					   bool in_vblank_irq,
 					   const struct drm_display_mode *mode)
 {
 	struct timeval tv_etime;
@@ -740,6 +741,7 @@ bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 	unsigned int vbl_status;
 	int vpos, hpos, i;
 	int delta_ns, duration_ns;
+	unsigned flags = in_vblank_irq ? DRM_CALLED_FROM_VBLIRQ : 0;
 
 	if (pipe >= dev->num_crtcs) {
 		DRM_ERROR("Invalid crtc %u\n", pipe);
@@ -843,9 +845,10 @@ static struct timeval get_drm_timestamp(void)
  * @dev: DRM device
  * @pipe: index of CRTC whose vblank timestamp to retrieve
  * @tvblank: Pointer to target struct timeval which should receive the timestamp
- * @flags: Flags to pass to driver:
- *         0 = Default,
- *         DRM_CALLED_FROM_VBLIRQ = If function is called from vbl IRQ handler
+ * @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.
  *
  * Fetches the system timestamp corresponding to the time of the most recent
  * vblank interval on specified CRTC. May call into kms-driver to
@@ -859,7 +862,7 @@ static struct timeval get_drm_timestamp(void)
  */
 static bool
 drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe,
-			  struct timeval *tvblank, unsigned flags)
+			  struct timeval *tvblank, bool in_vblank_irq)
 {
 	bool ret = false;
 
@@ -869,7 +872,7 @@ drm_get_last_vbltimestamp(struct drm_device *dev, unsigned int pipe,
 	/* Query driver if possible and precision timestamping enabled. */
 	if (dev->driver->get_vblank_timestamp && (max_error > 0))
 		ret = dev->driver->get_vblank_timestamp(dev, pipe, &max_error,
-							tvblank, flags);
+							tvblank, in_vblank_irq);
 
 	/* GPU high precision timestamp query unsupported or failed.
 	 * Return current monotonic/gettimeofday timestamp as best estimate.
@@ -1747,7 +1750,7 @@ bool drm_handle_vblank(struct drm_device *dev, unsigned int pipe)
 		return false;
 	}
 
-	drm_update_vblank_count(dev, pipe, DRM_CALLED_FROM_VBLIRQ);
+	drm_update_vblank_count(dev, pipe, true);
 
 	spin_unlock(&dev->vblank_time_lock);
 
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 34cb05a59742..66e3e2ac7453 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -967,7 +967,7 @@ int intel_get_crtc_scanline(struct intel_crtc *crtc)
 static bool i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
 			      int *max_error,
 			      struct timeval *vblank_time,
-			      unsigned flags)
+			      bool in_vblank_irq)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crtc *crtc;
@@ -991,7 +991,7 @@ static bool i915_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
 
 	/* Helper routine in DRM core does all the work: */
 	return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
-						     vblank_time, flags,
+						     vblank_time, in_vblank_irq,
 						     &crtc->base.hwmode);
 }
 
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index ffeb34bee3af..07e2b1335f65 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -595,7 +595,7 @@ static int mdp5_get_scanoutpos(struct drm_device *dev, unsigned int pipe,
 static bool mdp5_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
 				      int *max_error,
 				      struct timeval *vblank_time,
-				      unsigned flags)
+				      bool in_vblank_irq)
 {
 	struct msm_drm_private *priv = dev->dev_private;
 	struct drm_crtc *crtc;
@@ -612,7 +612,7 @@ static bool mdp5_get_vblank_timestamp(struct drm_device *dev, unsigned int pipe,
 	}
 
 	return drm_calc_vbltimestamp_from_scanoutpos(dev, pipe, max_error,
-						     vblank_time, flags,
+						     vblank_time, in_vblank_irq,
 						     &crtc->mode);
 }
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.c b/drivers/gpu/drm/nouveau/nouveau_display.c
index f1e36f70755d..2d28ef57f2bf 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.c
+++ b/drivers/gpu/drm/nouveau/nouveau_display.c
@@ -158,7 +158,7 @@ nouveau_display_scanoutpos(struct drm_device *dev, unsigned int pipe,
 
 bool
 nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe,
-			 int *max_error, struct timeval *time, unsigned flags)
+			 int *max_error, struct timeval *time, bool in_vblank_irq)
 {
 	struct drm_crtc *crtc;
 
@@ -170,7 +170,8 @@ nouveau_display_vblstamp(struct drm_device *dev, unsigned int pipe,
 			else
 				mode = &crtc->hwmode;
 			return drm_calc_vbltimestamp_from_scanoutpos(dev,
-					pipe, max_error, time, flags, mode);
+					pipe, max_error, time, in_vblank_irq,
+					mode);
 		}
 	}
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h b/drivers/gpu/drm/nouveau/nouveau_display.h
index c6bfe533a641..8ec86259c5ac 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.h
+++ b/drivers/gpu/drm/nouveau/nouveau_display.h
@@ -72,7 +72,7 @@ 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_vblstamp(struct drm_device *, unsigned int, int *,
-			       struct timeval *, unsigned);
+			       struct timeval *, bool);
 
 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 caa0b1cc4269..88fc791ec8fb 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -118,7 +118,7 @@ void radeon_disable_vblank_kms(struct drm_device *dev, unsigned int pipe);
 bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, unsigned int pipe,
 				     int *max_error,
 				     struct timeval *vblank_time,
-				     unsigned flags);
+				     bool in_vblank_irq);
 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 535969d74f64..5bccdeae0773 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -874,7 +874,7 @@ void radeon_disable_vblank_kms(struct drm_device *dev, int crtc)
 bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc,
 				     int *max_error,
 				     struct timeval *vblank_time,
-				     unsigned flags)
+				     bool in_vblank_irq)
 {
 	struct drm_crtc *drmcrtc;
 	struct radeon_device *rdev = dev->dev_private;
@@ -891,7 +891,7 @@ bool radeon_get_vblank_timestamp_kms(struct drm_device *dev, int crtc,
 
 	/* Helper routine in DRM core does all the work: */
 	return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc, max_error,
-						     vblank_time, flags,
+						     vblank_time, in_vblank_irq,
 						     &drmcrtc->hwmode);
 }
 
diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c
index f506525a1066..6ed53d2867c4 100644
--- a/drivers/gpu/drm/vc4/vc4_crtc.c
+++ b/drivers/gpu/drm/vc4/vc4_crtc.c
@@ -272,14 +272,14 @@ int vc4_crtc_get_scanoutpos(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)
+				  bool in_vblank_irq)
 {
 	struct drm_crtc *crtc = drm_crtc_from_index(dev, crtc_id);
 	struct drm_crtc_state *state = crtc->state;
 
 	/* Helper routine in DRM core does all the work: */
 	return drm_calc_vbltimestamp_from_scanoutpos(dev, crtc_id, max_error,
-						     vblank_time, flags,
+						     vblank_time, in_vblank_irq,
 						     &state->adjusted_mode);
 }
 
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index d1c53a5d0923..d192f7e5c1eb 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -495,7 +495,7 @@ int vc4_crtc_get_scanoutpos(struct drm_device *dev, unsigned int crtc_id,
 			    const struct drm_display_mode *mode);
 bool vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id,
 				  int *max_error, struct timeval *vblank_time,
-				  unsigned flags);
+				  bool in_vblank_irq);
 
 /* vc4_debugfs.c */
 int vc4_debugfs_init(struct drm_minor *minor);
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index da78e248d9d8..a97dbc1eeccd 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:
@@ -308,11 +310,10 @@ struct drm_driver {
 	 *     Returns true upper bound on error for timestamp.
 	 * vblank_time:
 	 *     Target location for returned vblank timestamp.
-	 * flags:
-	 *     0 = Defaults, no special treatment needed.
-	 *     DRM_CALLED_FROM_VBLIRQ = Function is called from vblank
-	 *     irq handler. Some drivers need to apply some workarounds
-	 *     for gpu-specific vblank irq quirks if flag is set.
+	 * 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.
 	 *
 	 * Returns:
 	 *
@@ -322,7 +323,7 @@ struct drm_driver {
 	bool (*get_vblank_timestamp) (struct drm_device *dev, unsigned int pipe,
 				     int *max_error,
 				     struct timeval *vblank_time,
-				     unsigned flags);
+				     bool in_vblank_irq);
 
 	/* these have to be filled in */
 
diff --git a/include/drm/drm_irq.h b/include/drm/drm_irq.h
index f0d5ccf9b282..445406efb8dc 100644
--- a/include/drm/drm_irq.h
+++ b/include/drm/drm_irq.h
@@ -156,7 +156,7 @@ u32 drm_accurate_vblank_count(struct drm_crtc *crtc);
 bool drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 					   unsigned int pipe, int *max_error,
 					   struct timeval *vblank_time,
-					   unsigned flags,
+					   bool in_vblank_irq,
 					   const struct drm_display_mode *mode);
 void drm_calc_timestamping_constants(struct drm_crtc *crtc,
 				     const struct drm_display_mode *mode);
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2017-05-09 14:03 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-03  7:26 [PATCH 0/5] vblanke cleanup resend Daniel Vetter
2017-05-03  7:26 ` [PATCH 1/5] drm/vblank: Switch drm_driver->get_vblank_timestamp to return a bool Daniel Vetter
2017-05-03  7:26 ` [PATCH 2/5] drm/vblank: Switch to bool in_vblank_irq in get_vblank_timestamp Daniel Vetter
2017-05-04 10:08   ` kbuild test robot
2017-05-03  7:26 ` [PATCH 3/5] drm/vblank: Add FIXME comments about moving the vblank ts hooks Daniel Vetter
2017-05-03  7:26 ` [PATCH 4/5] drm/vblank: drop the mode argument from drm_calc_vbltimestamp_from_scanoutpos Daniel Vetter
2017-05-03  7:26 ` [PATCH 5/5] drm/vblank: Lock down vblank->hwmode more Daniel Vetter
2017-05-03 14:09   ` Ville Syrjälä
2017-05-04 13:20     ` Daniel Vetter
2017-05-08 11:33       ` Ville Syrjälä
2017-05-03  7:46 ` ✓ Fi.CI.BAT: success for vblanke cleanup resend Patchwork
2017-05-09 14:03 [PATCH 1/5] drm/vblank: Switch drm_driver->get_vblank_timestamp to return a bool Daniel Vetter
2017-05-09 14:03 ` [PATCH 2/5] drm/vblank: Switch to bool in_vblank_irq in get_vblank_timestamp Daniel Vetter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.