[Intel-gfx] [PATCH 2/4] drm: Push latency sensitive bits of vblank scanoutpos timestamping into kms drivers.

2013-10-29 Thread Mario Kleiner
A change in locking of some kms drivers (currently intel-kms) make
the old approach too inaccurate and also incompatible with the
PREEMPT_RT realtime kernel patchset.

The driver-get_scanout_position() method of intel-kms now needs
to aquire a spinlock, which clashes badly with the former
preempt_disable() calls in the drm, and it also introduces larger
delays and timing uncertainty on a contended lock than acceptable.

This patch changes the prototype of driver-get_scanout_position()
to require/allow kms drivers to perform the ktime_get() system time
queries which go along with actual scanout position readout in a way
that provides maximum precision and to return those timestamps to
the drm. kms drivers implementations of get_scanout_position() are
asked to implement timestamping and scanoutpos readout in a way
that is as precise as possible and compatible with preempt_disable()
on a PREMPT_RT kernel. A driver should follow this pattern in
get_scanout_position() for precision and compatibility:

spin_lock...(...);
preempt_disable_rt(); // On a PREEMPT_RT kernel, otherwise omit.
if (stime) *stime = ktime_get();
... Minimum amount of MMIO register reads to get scanout position ...
... no taking of locks allowed here! ...
if (etime) *etime = ktime_get();
preempt_enable_rt(); // On PREEMPT_RT kernel, otherwise omit.
spin_unlock...(...);

v2: Fix formatting of new multi-line code comments.

Signed-off-by: Mario Kleiner mario.kleiner...@gmail.com
Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com
Reviewed-by: Alex Deucher alexander.deuc...@amd.com
---
 drivers/gpu/drm/drm_irq.c |   20 
 include/drm/drmP.h|   10 --
 2 files changed, 20 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 33ee515..d80d952 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -219,7 +219,7 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs)
for (i = 0; i  num_crtcs; i++)
init_waitqueue_head(dev-vblank[i].queue);
 
-   DRM_INFO(Supports vblank timestamp caching Rev 1 (10.10.2010).\n);
+   DRM_INFO(Supports vblank timestamp caching Rev 2 (21.10.2013).\n);
 
/* Driver specific high-precision vblank timestamping supported? */
if (dev-driver-get_vblank_timestamp)
@@ -586,14 +586,17 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct 
drm_device *dev, int crtc,
 * code gets preempted or delayed for some reason.
 */
for (i = 0; i  DRM_TIMESTAMP_MAXRETRIES; i++) {
-   /* Get system timestamp before query. */
-   stime = ktime_get();
-
-   /* Get vertical and horizontal scanout pos. vpos, hpos. */
-   vbl_status = dev-driver-get_scanout_position(dev, crtc, 
vpos, hpos);
+   /*
+* Get vertical and horizontal scanout position vpos, hpos,
+* and bounding timestamps stime, etime, pre/post query.
+*/
+   vbl_status = dev-driver-get_scanout_position(dev, crtc, vpos,
+  hpos, stime, 
etime);
 
-   /* Get system timestamp after query. */
-   etime = ktime_get();
+   /*
+* Get correction for CLOCK_MONOTONIC - CLOCK_REALTIME if
+* CLOCK_REALTIME is requested.
+*/
if (!drm_timestamp_monotonic)
mono_time_offset = ktime_get_monotonic_offset();
 
@@ -604,6 +607,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device 
*dev, int crtc,
return -EIO;
}
 
+   /* Compute uncertainty in timestamp of scanout position query. 
*/
duration_ns = ktime_to_ns(etime) - ktime_to_ns(stime);
 
/* Accept result with   max_error nsecs timing uncertainty. */
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 2b954ad..48d15f0 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -835,12 +835,17 @@ struct drm_driver {
/**
 * Called by vblank timestamping code.
 *
-* Return the current display scanout position from a crtc.
+* Return the current display scanout position from a crtc, and an
+* optional accurate ktime_get timestamp of when position was measured.
 *
 * \param dev  DRM device.
 * \param crtc Id of the crtc to query.
 * \param *vpos Target location for current vertical scanout position.
 * \param *hpos Target location for current horizontal scanout position.
+* \param *stime Target location for timestamp taken immediately before
+*   scanout position query. Can be NULL to skip timestamp.
+* \param *etime Target location for timestamp taken immediately after
+*   scanout position query. Can be NULL to skip timestamp.
 *
   

[Intel-gfx] [PATCH 2/4] drm: Push latency sensitive bits of vblank scanoutpos timestamping into kms drivers.

2013-10-26 Thread Mario Kleiner
A change in locking of some kms drivers (currently intel-kms) make
the old approach too inaccurate and also incompatible with the
PREEMPT_RT realtime kernel patchset.

The driver-get_scanout_position() method of intel-kms now needs
to aquire a spinlock, which clashes badly with the former
preempt_disable() calls in the drm, and it also introduces larger
delays and timing uncertainty on a contended lock than acceptable.

This patch changes the prototype of driver-get_scanout_position()
to require/allow kms drivers to perform the ktime_get() system time
queries which go along with actual scanout position readout in a way
that provides maximum precision and to return those timestamps to
the drm. kms drivers implementations of get_scanout_position() are
asked to implement timestamping and scanoutpos readout in a way
that is as precise as possible and compatible with preempt_disable()
on a PREMPT_RT kernel. A driver should follow this pattern in
get_scanout_position() for precision and compatibility:

spin_lock...(...);
preempt_disable_rt(); // On a PREEMPT_RT kernel, otherwise omit.
if (stime) *stime = ktime_get();
... Minimum amount of MMIO register reads to get scanout position ...
... no taking of locks allowed here! ...
if (etime) *etime = ktime_get();
preempt_enable_rt(); // On PREEMPT_RT kernel, otherwise omit.
spin_unlock...(...);

Signed-off-by: Mario Kleiner mario.kleiner...@gmail.com
---
 drivers/gpu/drm/drm_irq.c |   18 ++
 include/drm/drmP.h|   10 --
 2 files changed, 18 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 33ee515..2250724 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -219,7 +219,7 @@ int drm_vblank_init(struct drm_device *dev, int num_crtcs)
for (i = 0; i  num_crtcs; i++)
init_waitqueue_head(dev-vblank[i].queue);
 
-   DRM_INFO(Supports vblank timestamp caching Rev 1 (10.10.2010).\n);
+   DRM_INFO(Supports vblank timestamp caching Rev 2 (21.10.2013).\n);
 
/* Driver specific high-precision vblank timestamping supported? */
if (dev-driver-get_vblank_timestamp)
@@ -586,14 +586,15 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct 
drm_device *dev, int crtc,
 * code gets preempted or delayed for some reason.
 */
for (i = 0; i  DRM_TIMESTAMP_MAXRETRIES; i++) {
-   /* Get system timestamp before query. */
-   stime = ktime_get();
-
-   /* Get vertical and horizontal scanout pos. vpos, hpos. */
-   vbl_status = dev-driver-get_scanout_position(dev, crtc, 
vpos, hpos);
+   /* Get vertical and horizontal scanout position vpos, hpos,
+* and bounding timestamps stime, etime, pre/post query.
+*/
+   vbl_status = dev-driver-get_scanout_position(dev, crtc, vpos,
+  hpos, stime, 
etime);
 
-   /* Get system timestamp after query. */
-   etime = ktime_get();
+   /* Get correction for CLOCK_MONOTONIC - CLOCK_REALTIME if
+* CLOCK_REALTIME is requested.
+*/
if (!drm_timestamp_monotonic)
mono_time_offset = ktime_get_monotonic_offset();
 
@@ -604,6 +605,7 @@ int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device 
*dev, int crtc,
return -EIO;
}
 
+   /* Compute uncertainty in timestamp of scanout position query. 
*/
duration_ns = ktime_to_ns(etime) - ktime_to_ns(stime);
 
/* Accept result with   max_error nsecs timing uncertainty. */
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 2b954ad..48d15f0 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -835,12 +835,17 @@ struct drm_driver {
/**
 * Called by vblank timestamping code.
 *
-* Return the current display scanout position from a crtc.
+* Return the current display scanout position from a crtc, and an
+* optional accurate ktime_get timestamp of when position was measured.
 *
 * \param dev  DRM device.
 * \param crtc Id of the crtc to query.
 * \param *vpos Target location for current vertical scanout position.
 * \param *hpos Target location for current horizontal scanout position.
+* \param *stime Target location for timestamp taken immediately before
+*   scanout position query. Can be NULL to skip timestamp.
+* \param *etime Target location for timestamp taken immediately after
+*   scanout position query. Can be NULL to skip timestamp.
 *
 * Returns vpos as a positive number while in active scanout area.
 * Returns vpos as a negative number inside vblank, counting the number
@@ -857,7 +862,8 @@ struct drm_driver {