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

[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, &t_vblank);
> +	store_vblank(dev, pipe, diff, &t_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: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20150930/18a7f916/attachment.sig>