* [PATCH] drm/i915: Fix pre-CTG vblank counter
@ 2013-09-25 16:55 ville.syrjala
2013-09-25 20:55 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: ville.syrjala @ 2013-09-25 16:55 UTC (permalink / raw)
To: intel-gfx; +Cc: Mario Kleiner
From: Ville Syrjälä <ville.syrjala@linux.intel.com>
The old style frame counter increments at the start of active video.
However for i915_get_vblank_counter() we want a counter that increments
at the start of vblank.
Fortunately the low frame counter register also contains the pixel
counter for the current frame. We can can compare that against the
vblank start pixel count to determine if we need to increment the
frame counter by 1 to get the correct answer.
Also reorganize the function pointer assignments in intel_irq_init() a
bit to avoid confusing people.
Cc: Mario Kleiner <mario.kleiner@tuebingen.mpg.de>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
Just another small vblank related gem I forgot to polish up and send
out until Imre started asking me questions about the vblank counter
functions.
drivers/gpu/drm/i915/i915_irq.c | 36 +++++++++++++++++++++++++++++++-----
1 file changed, 31 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 4b519c8..8a5eb9d 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -526,7 +526,7 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, int pipe)
drm_i915_private_t *dev_priv = (drm_i915_private_t *) dev->dev_private;
unsigned long high_frame;
unsigned long low_frame;
- u32 high1, high2, low;
+ u32 high1, high2, low, pixel, vbl_start;
if (!i915_pipe_enabled(dev, pipe)) {
DRM_DEBUG_DRIVER("trying to get vblank count for disabled "
@@ -534,6 +534,24 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, int pipe)
return 0;
}
+ if (drm_core_check_feature(dev, DRIVER_MODESET)) {
+ struct intel_crtc *intel_crtc =
+ to_intel_crtc(dev_priv->pipe_to_crtc_mapping[pipe]);
+ const struct drm_display_mode *mode =
+ &intel_crtc->config.adjusted_mode;
+
+ vbl_start = mode->crtc_vblank_start * mode->crtc_htotal;
+ } else {
+ enum transcoder cpu_transcoder =
+ intel_pipe_to_cpu_transcoder(dev_priv, pipe);
+ u32 htotal;
+
+ htotal = ((I915_READ(HTOTAL(cpu_transcoder)) >> 16) & 0x1fff) + 1;
+ vbl_start = (I915_READ(VBLANK(cpu_transcoder)) & 0x1fff) + 1;
+
+ vbl_start *= htotal;
+ }
+
high_frame = PIPEFRAME(pipe);
low_frame = PIPEFRAMEPIXEL(pipe);
@@ -544,13 +562,20 @@ static u32 i915_get_vblank_counter(struct drm_device *dev, int pipe)
*/
do {
high1 = I915_READ(high_frame) & PIPE_FRAME_HIGH_MASK;
- low = I915_READ(low_frame) & PIPE_FRAME_LOW_MASK;
+ low = I915_READ(low_frame);
high2 = I915_READ(high_frame) & PIPE_FRAME_HIGH_MASK;
} while (high1 != high2);
high1 >>= PIPE_FRAME_HIGH_SHIFT;
+ pixel = low & PIPE_PIXEL_MASK;
low >>= PIPE_FRAME_LOW_SHIFT;
- return (high1 << 8) | low;
+
+ /*
+ * The frame counter increments at beginning of active.
+ * Cook up a vblank counter by also checking the pixel
+ * counter against vblank start.
+ */
+ return ((high1 << 8) | low) + (pixel >= vbl_start);
}
static u32 gm45_get_vblank_counter(struct drm_device *dev, int pipe)
@@ -3197,11 +3222,12 @@ void intel_irq_init(struct drm_device *dev)
pm_qos_add_request(&dev_priv->pm_qos, PM_QOS_CPU_DMA_LATENCY, PM_QOS_DEFAULT_VALUE);
- dev->driver->get_vblank_counter = i915_get_vblank_counter;
- dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */
if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
dev->max_vblank_count = 0xffffffff; /* full 32 bit counter */
dev->driver->get_vblank_counter = gm45_get_vblank_counter;
+ } else {
+ dev->driver->get_vblank_counter = i915_get_vblank_counter;
+ dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */
}
if (drm_core_check_feature(dev, DRIVER_MODESET)) {
--
1.8.1.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Fix pre-CTG vblank counter
2013-09-25 16:55 [PATCH] drm/i915: Fix pre-CTG vblank counter ville.syrjala
@ 2013-09-25 20:55 ` Daniel Vetter
2013-09-26 17:32 ` Mario Kleiner
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2013-09-25 20:55 UTC (permalink / raw)
To: ville.syrjala; +Cc: intel-gfx, Mario Kleiner
On Wed, Sep 25, 2013 at 07:55:26PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The old style frame counter increments at the start of active video.
> However for i915_get_vblank_counter() we want a counter that increments
> at the start of vblank.
>
> Fortunately the low frame counter register also contains the pixel
> counter for the current frame. We can can compare that against the
> vblank start pixel count to determine if we need to increment the
> frame counter by 1 to get the correct answer.
>
> Also reorganize the function pointer assignments in intel_irq_init() a
> bit to avoid confusing people.
>
> Cc: Mario Kleiner <mario.kleiner@tuebingen.mpg.de>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>
> Just another small vblank related gem I forgot to polish up and send
> out until Imre started asking me questions about the vblank counter
> functions.
Hm, I've thought the magic fixup code does take care of that for us? But I
agree that we should do this explicitly ...
This could explain some of the strange vblank timestamp off failures QA
has reported (if there's too much delay and the fixup doesn't fire any
more), care to attach this patch to the relevant bug reports? Searching
for ts jitter + pre-gen5 should be good enough.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Fix pre-CTG vblank counter
2013-09-25 20:55 ` Daniel Vetter
@ 2013-09-26 17:32 ` Mario Kleiner
2013-10-11 21:42 ` Daniel Vetter
0 siblings, 1 reply; 4+ messages in thread
From: Mario Kleiner @ 2013-09-26 17:32 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, Mario Kleiner
On 25.09.13 22:55, Daniel Vetter wrote:
> On Wed, Sep 25, 2013 at 07:55:26PM +0300, ville.syrjala@linux.intel.com wrote:
>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>
>> The old style frame counter increments at the start of active video.
>> However for i915_get_vblank_counter() we want a counter that increments
>> at the start of vblank.
>>
>> Fortunately the low frame counter register also contains the pixel
>> counter for the current frame. We can can compare that against the
>> vblank start pixel count to determine if we need to increment the
>> frame counter by 1 to get the correct answer.
>>
>> Also reorganize the function pointer assignments in intel_irq_init() a
>> bit to avoid confusing people.
>>
>> Cc: Mario Kleiner <mario.kleiner@tuebingen.mpg.de>
>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> ---
>>
>> Just another small vblank related gem I forgot to polish up and send
>> out until Imre started asking me questions about the vblank counter
>> functions.
>
> Hm, I've thought the magic fixup code does take care of that for us? But I
> agree that we should do this explicitly ...
>
> This could explain some of the strange vblank timestamp off failures QA
> has reported (if there's too much delay and the fixup doesn't fire any
> more), care to attach this patch to the relevant bug reports? Searching
> for ts jitter + pre-gen5 should be good enough.
> -Daniel
>
The vblank off code has one known race in that if
dev->driver->get_vblank_counter does not increment its counter at start
of vblank, it can have off-by-one vblank. There's a comment in that
function about that, e.g.,
http://lxr.free-electrons.com/source/drivers/gpu/drm/drm_irq.c#L102
The magic code there only fixes races between the vblank off code and
the vblank irq handler. Normally that race would not really affect
applications as long as the vblank_off_delay is as large (5 secs) as it
is now. But in QA testing, if you exercise that code not the way a
normal app would do it, i'd expect you to see that error about 4% of the
time.
The only fix is to fixup the vblank counter in the kms driver, so this
patch looks like a perfect solution to that. You can add my reviewed-by
if you wish.
-mario
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: Fix pre-CTG vblank counter
2013-09-26 17:32 ` Mario Kleiner
@ 2013-10-11 21:42 ` Daniel Vetter
0 siblings, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2013-10-11 21:42 UTC (permalink / raw)
To: Mario Kleiner; +Cc: intel-gfx, Mario Kleiner
On Thu, Sep 26, 2013 at 07:32:58PM +0200, Mario Kleiner wrote:
> On 25.09.13 22:55, Daniel Vetter wrote:
> >On Wed, Sep 25, 2013 at 07:55:26PM +0300, ville.syrjala@linux.intel.com wrote:
> >>From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>
> >>The old style frame counter increments at the start of active video.
> >>However for i915_get_vblank_counter() we want a counter that increments
> >>at the start of vblank.
> >>
> >>Fortunately the low frame counter register also contains the pixel
> >>counter for the current frame. We can can compare that against the
> >>vblank start pixel count to determine if we need to increment the
> >>frame counter by 1 to get the correct answer.
> >>
> >>Also reorganize the function pointer assignments in intel_irq_init() a
> >>bit to avoid confusing people.
> >>
> >>Cc: Mario Kleiner <mario.kleiner@tuebingen.mpg.de>
> >>Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>---
> >>
> >>Just another small vblank related gem I forgot to polish up and send
> >>out until Imre started asking me questions about the vblank counter
> >>functions.
> >
> >Hm, I've thought the magic fixup code does take care of that for us? But I
> >agree that we should do this explicitly ...
> >
> >This could explain some of the strange vblank timestamp off failures QA
> >has reported (if there's too much delay and the fixup doesn't fire any
> >more), care to attach this patch to the relevant bug reports? Searching
> >for ts jitter + pre-gen5 should be good enough.
> >-Daniel
> >
>
> The vblank off code has one known race in that if
> dev->driver->get_vblank_counter does not increment its counter at
> start of vblank, it can have off-by-one vblank. There's a comment in
> that function about that, e.g.,
>
> http://lxr.free-electrons.com/source/drivers/gpu/drm/drm_irq.c#L102
>
> The magic code there only fixes races between the vblank off code
> and the vblank irq handler. Normally that race would not really
> affect applications as long as the vblank_off_delay is as large (5
> secs) as it is now. But in QA testing, if you exercise that code not
> the way a normal app would do it, i'd expect you to see that error
> about 4% of the time.
>
> The only fix is to fixup the vblank counter in the kms driver, so
> this patch looks like a perfect solution to that. You can add my
> reviewed-by if you wish.
r-b added and patch merged. I think we need to repoke qa to test stuff
since our kms_flip testcase has been a bit broken the last few days (and
they've failed to correctly untangle the mess a bit).
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2013-10-11 21:42 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-25 16:55 [PATCH] drm/i915: Fix pre-CTG vblank counter ville.syrjala
2013-09-25 20:55 ` Daniel Vetter
2013-09-26 17:32 ` Mario Kleiner
2013-10-11 21:42 ` Daniel Vetter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox