drm: Push latency sensitive bits of vblank scanoutpos timestamping into kms drivers.
authorMario Kleiner <mario.kleiner.de@gmail.com>
Wed, 30 Oct 2013 04:13:06 +0000 (05:13 +0100)
committerDave Airlie <airlied@redhat.com>
Wed, 6 Nov 2013 01:53:41 +0000 (11:53 +1000)
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.de@gmail.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Signed-off-by: Dave Airlie <airlied@redhat.com>
drivers/gpu/drm/drm_irq.c
include/drm/drmP.h

index 33ee515..d80d952 100644 (file)
@@ -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. */
index e6d0cd9..748c729 100644 (file)
@@ -836,12 +836,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
@@ -858,7 +863,8 @@ struct drm_driver {
         *
         */
        int (*get_scanout_position) (struct drm_device *dev, int crtc,
-                                    int *vpos, int *hpos);
+                                    int *vpos, int *hpos, ktime_t *stime,
+                                    ktime_t *etime);
 
        /**
         * Called by \c drm_get_last_vbltimestamp. Should return a precise