[Intel-gfx] [PATCH] drm/i915: update last_vblank when disabling the power well

[Daniel Vetter]

On Fri, Aug 02, 2013 at 04:56:44PM -0300, Paulo Zanoni wrote:
> 2013/7/23 Paulo Zanoni <przanoni at gmail.com>:
> > 2013/7/23 Chris Wilson <chris at chris-wilson.co.uk>:
> >> On Tue, Jul 23, 2013 at 10:48:11AM -0300, Paulo Zanoni wrote:
> >>> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> >>>
> >>> The DRM layer keeps track of our vblanks and it assumes our vblank
> >>> counters only go back to zero when they overflow. The problem is that
> >>> when we disable the power well our counters also go to zero, but it
> >>> doesn't mean they did overflow. So on this patch we grab the lock and
> >>> update last_vblank so the DRM layer won't think our counters
> >>> overflowed.
> >>>
> >>> This patch fixes the following intel-gpu-tools test:
> >>> ./kms_flip --run-subtest blocking-absolute-wf_vblank
> >>>
> >>> Regression introduced by the following commit:
> >>>
> >>> commit bf51d5e2cda5d36d98e4b46ac7fca9461e512c41
> >>> Author: Paulo Zanoni <paulo.r.zanoni at intel.com>
> >>> Date:   Wed Jul 3 17:12:13 2013 -0300
> >>>     drm/i915: switch disable_power_well default value to 1
> >>>
> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=66808
> >>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> >>> ---
> >>>  drivers/gpu/drm/i915/intel_pm.c | 13 +++++++++++++
> >>>  1 file changed, 13 insertions(+)
> >>>
> >>> Tested on -nightly, but applies cleanly to -fixes.
> >>>
> >>> I recognize this patch is not really beautiful, I'm open to suggestions.
> >>
> >> Saving and restoring each enabled pipes' framecounter across the powerwell
> >> would look neater than messing around with the drm core structs.
> >
> > The registers are read-only, so we can't save/restore them.
> 
> Ping? This is for -fixes.

Oops, I've thought I've dropped my bikeshed on this but seems I've gone on
vacation before doing that ;-)

I think we should do this vblank counter resetting in general in the drm
vblank core in drm_vblank_post_modeset, instead of frobbing around in the
implementation details from our driver code. But for -fixes this much more
minimal approach looks better. I've added a FIXME comment and merged the
patch, but looking into this for -next might be good.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch