[PATCH 10/11] drm: Use vblank timestamps to guesstimate how many vblanks were missed

2015-09-30 Thread Thierry Reding 
On Mon, Sep 14, 2015 at 10:43:51PM +0300, ville.syrjala at linux.intel.com 
wrote:
[...]
> @@ -167,7 +238,7 @@ static void drm_update_vblank_count(struct drm_device 
> *dev, unsigned int pipe,
>   if (!rc)
>   t_vblank = (struct timeval) {0, 0};
>  
> - store_vblank(dev, pipe, diff, _vblank);
> + store_vblank(dev, pipe, diff, _vblank, cur_vblank);

I think clearing the t_vblank is now wrong here. This breaks weston on
Tegra (and I think all other drivers that don't implement hardware
VBLANK timestamps). The reason is that if ->get_vblank_timestamp() isn't
implemented, rc will always be false at this point, hence resulting in a
zero VBLANK timestamp being reported to userspace.

In fact the comment, reproduced here because it's not in the patch
context:

/*
 * Only reinitialize corresponding vblank timestamp if high-precision 
query
 * available and didn't fail. Otherwise reinitialize delayed at next 
vblank
 * interrupt and assign 0 for now, to mark the vblanktimestamp as 
invalid.
 */

is no longer accurate because this function is now called at the VBLANK
interrupt. If you want to keep invalidating timestamps for drivers that
implement VBLANK timestamp queries I think the condition needs to be
changed to something like:

if (dev->get_vblank_timestamp && !rc)

and the comment updated to reflect the truth. Alternatively this code
can be removed, though I guess having the monotonic timestamp fallback
would be kind of missing the point.

I've tested either change and they both restore weston on Tegra.

Thierry
-- next part --
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL:

[PATCH 10/11] drm: Use vblank timestamps to guesstimate how many vblanks were missed

2015-09-28 Thread Michel Dänzer 
On 15.09.2015 04:43, ville.syrjala at linux.intel.com wrote:
> From: Ville Syrjälä 
> 
> When lacking am accurate hardware frame counter, we can fall back to
> using the vblank timestamps to guesstimagte how many vblanks have
> elapsed since the last time the vblank counter was updated.
> 
> Take the oppostunity to unify the vblank_disable_and_save() and
> drm_handle_vblank_events() to call the same function
> (drm_update_vblank_count()) to perform the vblank updates.

It would be nice to keep the drm_update_vblank_count unification
separate. As it is, it's very hard to keep track of which parts of the
patch are for each logical change.


BTW, I think the fact that I was hitting the problem fixed by 209e4dbc
("drm/vblank: Use u32 consistently for vblank counters") within a few
days indicates that there's another bug which causes the counter to jump
forward with drm_vblank_on/off(). It may not manifest itself with
current Intel hardware because that has a full 32-bit hardware frame
counter, turning the related calculations into no-ops. I haven't had
time to investigate this further yet.


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer

[PATCH 10/11] drm: Use vblank timestamps to guesstimate how many vblanks were missed

2015-09-14 Thread ville.syrj...@linux.intel.com 
From: Ville Syrjälä 

When lacking am accurate hardware frame counter, we can fall back to
using the vblank timestamps to guesstimagte how many vblanks have
elapsed since the last time the vblank counter was updated.

Take the oppostunity to unify the vblank_disable_and_save() and
drm_handle_vblank_events() to call the same function
(drm_update_vblank_count()) to perform the vblank updates.

If the hardware/driver has an accurate frame counter use it instead of
the timestamp based guesstimate. If the hardware/driver has neither
a frame counter nor acurate vblank timestamps, we fall back to assuming
that each drm_handle_vblank_events() should increment the vblank count
by one.

Signed-off-by: Ville Syrjälä 
---
 drivers/gpu/drm/drm_irq.c | 206 --
 1 file changed, 91 insertions(+), 115 deletions(-)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 07b0cb1..88fbee4 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -76,13 +76,15 @@ module_param_named(timestamp_monotonic, 
drm_timestamp_monotonic, int, 0600);

 static void store_vblank(struct drm_device *dev, unsigned int pipe,
 u32 vblank_count_inc,
-struct timeval *t_vblank)
+struct timeval *t_vblank, u32 last)
 {
struct drm_vblank_crtc *vblank = >vblank[pipe];
u32 tslot;

assert_spin_locked(>vblank_time_lock);

+   vblank->last = last;
+
/* All writers hold the spinlock, but readers are serialized by
 * the latching of vblank->count below.
 */
@@ -103,6 +105,54 @@ static void store_vblank(struct drm_device *dev, unsigned 
int pipe,
 }

 /**
+ * drm_reset_vblank_timestamp - reset the last timestamp to the last vblank
+ * @dev: DRM device
+ * @pipe: index of CRTC for which to reset the timestamp
+ *
+ * Reset the stored timestamp for the current vblank count to correspond
+ * to the last vblank occurred.
+ *
+ * Only to be called from drm_vblank_on().
+ *
+ * Note: caller must hold dev->vbl_lock since this reads & writes
+ * device vblank fields.
+ */
+static void drm_reset_vblank_timestamp(struct drm_device *dev, unsigned int 
pipe)
+{
+   u32 cur_vblank;
+   bool rc;
+   struct timeval t_vblank;
+   int count = DRM_TIMESTAMP_MAXRETRIES;
+
+   spin_lock(>vblank_time_lock);
+
+   /*
+* sample the current counter to avoid random jumps
+* when drm_vblank_enable() applies the diff
+*/
+   do {
+   cur_vblank = dev->driver->get_vblank_counter(dev, pipe);
+   rc = drm_get_last_vbltimestamp(dev, pipe, _vblank, 0);
+   } while (cur_vblank != dev->driver->get_vblank_counter(dev, pipe) && 
--count > 0);
+
+   /*
+* Only reinitialize corresponding vblank timestamp if high-precision 
query
+* available and didn't fail. Otherwise reinitialize delayed at next 
vblank
+* interrupt and assign 0 for now, to mark the vblanktimestamp as 
invalid.
+*/
+   if (!rc)
+   t_vblank = (struct timeval) {0, 0};
+
+   /*
+* +1 to make sure user will never see the same
+* vblank counter value before and after a modeset
+*/
+   store_vblank(dev, pipe, 1, _vblank, cur_vblank);
+
+   spin_unlock(>vblank_time_lock);
+}
+
+/**
  * drm_update_vblank_count - update the master vblank counter
  * @dev: DRM device
  * @pipe: counter to update
@@ -126,6 +176,7 @@ static void drm_update_vblank_count(struct drm_device *dev, 
unsigned int pipe,
bool rc;
struct timeval t_vblank;
int count = DRM_TIMESTAMP_MAXRETRIES;
+   int framedur_ns = vblank->framedur_ns;

/*
 * Interrupts were disabled prior to this call, so deal with counter
@@ -144,20 +195,40 @@ static void drm_update_vblank_count(struct drm_device 
*dev, unsigned int pipe,
rc = drm_get_last_vbltimestamp(dev, pipe, _vblank, flags);
} while (cur_vblank != dev->driver->get_vblank_counter(dev, pipe) && 
--count > 0);

-   /* Deal with counter wrap */
-   diff = cur_vblank - vblank->last;
-   if (cur_vblank < vblank->last) {
-   diff += dev->max_vblank_count + 1;
+   if (dev->max_vblank_count != 0) {
+   /* trust the hw counter when it's around */
+   diff = (cur_vblank - vblank->last) & dev->max_vblank_count;
+   } else if (rc && framedur_ns) {
+   const struct timeval *t_old;
+   u64 diff_ns;
+
+   t_old = (dev, pipe, vblank->count);
+   diff_ns = timeval_to_ns(_vblank) - timeval_to_ns(t_old);

-   DRM_DEBUG("last_vblank[%u]=0x%x, cur_vblank=0x%x => 
diff=0x%x\n",
- pipe, vblank->last, cur_vblank, diff);
+   /*
+* Figure out how many vblanks we've missed based
+* on the difference in the