From: Ville Syrjälä ville.syrjala@linux.intel.com

On i965gm the hardware frame counter does not work when the TV encoder is active. So let's not try to consult the hardware frame counter in that case. Instead we'll fall back to the timestamp based guesstimation method used on gen2.

Note that the pipe timings generated by the TV encoder are also rather peculiar. Apparently the pipe wants to run at a much higher speed (related to the oversample clock somehow it seems) but during the vertical active period the TV encoder stalls the pipe every few lines to keep its speed in check. But once the vertical blanking period is reached the pipe gets to run at full speed. This means our vblank timestamp estimates are suspect. Fixing all that would require quite a bit more work. This simple fix at least avoids the nasty vblank timeouts that are happening currently.

Curiously the frame counter works just fine on i945gm and gm45. I don't really understand what kind of mishap occurred with the hardware design on i965gm. Sadly I wasn't able to find any chicken bits etc. that would fix the frame counter :(

v2: Move the zero vs. non-zero hw counter value handling into i915_get_vblank_counter() (Daniel) Use the per-crtc maximum exclusively, leaving the per-device maximum at zero

Cc: stable@vger.kernel.org Cc: Daniel Vetter daniel@ffwll.ch Fixes: 51e31d49c890 ("drm/i915: Use generic vblank wait") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93782 Signed-off-by: Ville Syrjälä ville.syrjala@linux.intel.com --- drivers/gpu/drm/i915/i915_irq.c | 27 ++++++++++----- drivers/gpu/drm/i915/intel_display.c | 49 +++++++++++++++++++++++----- 2 files changed, 59 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index d447d7d508f4..ab2d4eefef18 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -822,11 +822,26 @@ static void i915_enable_asle_pipestat(struct drm_i915_private *dev_priv) static u32 i915_get_vblank_counter(struct drm_device *dev, unsigned int pipe) { struct drm_i915_private *dev_priv = to_i915(dev); + struct drm_vblank_crtc *vblank = &dev->vblank[pipe]; + const struct drm_display_mode *mode = &vblank->hwmode; i915_reg_t high_frame, low_frame; u32 high1, high2, low, pixel, vbl_start, hsync_start, htotal; - const struct drm_display_mode *mode = &dev->vblank[pipe].hwmode; unsigned long irqflags;

+ /* + * On i965gm TV output the frame counter only works up to + * the point when we enable the TV encoder. After that the + * frame counter ceases to work and reads zero. We need a + * vblank wait before enabling the TV encoder and so we + * have to enable vblank interrupts while the frame counter + * is still in a working state. However the core vblank code + * does not like us returning non-zero frame counter values + * when we've told it that we don't have a working frame + * counter. Thus we must stop non-zero values leaking out. + */ + if (!vblank->max_vblank_count) + return 0; + htotal = mode->crtc_htotal; hsync_start = mode->crtc_hsync_start; vbl_start = mode->crtc_vblank_start; @@ -4836,16 +4851,10 @@ void intel_irq_init(struct drm_i915_private *dev_priv) if (INTEL_GEN(dev_priv) >= 8) rps->pm_intrmsk_mbz |= GEN8_PMINTR_DISABLE_REDIRECT_TO_GUC;

- if (IS_GEN2(dev_priv)) { - /* Gen2 doesn't have a hardware frame counter */ - dev->max_vblank_count = 0; - } else if (IS_G4X(dev_priv) || INTEL_GEN(dev_priv) >= 5) { - dev->max_vblank_count = 0xffffffff; /* full 32 bit counter */ + if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)) dev->driver->get_vblank_counter = g4x_get_vblank_counter; - } else { + else if (INTEL_GEN(dev_priv) >= 3) dev->driver->get_vblank_counter = i915_get_vblank_counter; - dev->max_vblank_count = 0xffffff; /* only 24 bits of frame count */ - }

/* * Opt out of the vblank disable timer on everything except gen2. diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e9f4e22b2a4e..3eafea4e598f 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1758,6 +1758,7 @@ static void intel_enable_pipe(const struct intel_crtc_state *new_crtc_state) { struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->base.crtc); struct drm_i915_private *dev_priv = to_i915(crtc->base.dev); + struct drm_vblank_crtc *vblank = &dev_priv->drm.vblank[drm_crtc_index(&crtc->base)]; enum transcoder cpu_transcoder = new_crtc_state->cpu_transcoder; enum pipe pipe = crtc->pipe; i915_reg_t reg; @@ -1806,7 +1807,7 @@ static void intel_enable_pipe(const struct intel_crtc_state *new_crtc_state) * when it's derived from the timestamps. So let's wait for the * pipe to start properly before we call drm_crtc_vblank_on() */ - if (dev_priv->drm.max_vblank_count == 0) + if (vblank->max_vblank_count == 0) intel_wait_for_pipe_scanline_moving(crtc); }

@@ -5544,6 +5545,35 @@ static void intel_encoders_post_pll_disable(struct drm_crtc *crtc, } }

+static u32 intel_crtc_max_vblank_count(const struct intel_crtc_state *crtc_state) +{ + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev); + + /* + * On i965gm the hardware frame counter reads + * zero when the TV encoder is enabled :( + */ + if (IS_I965GM(dev_priv) && + (crtc_state->output_types & BIT(INTEL_OUTPUT_TVOUT))) + return 0; + + if (INTEL_GEN(dev_priv) >= 5 || IS_G4X(dev_priv)) + return 0xffffffff; /* full 32 bit counter */ + else if (INTEL_GEN(dev_priv) >= 3) + return 0xffffff; /* only 24 bits of frame count */ + else + return 0; /* Gen2 doesn't have a hardware frame counter */ +} + +static void intel_crtc_vblank_on(const struct intel_crtc_state *crtc_state) +{ + struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc); + + drm_crtc_set_max_vblank_count(&crtc->base, + intel_crtc_max_vblank_count(crtc_state)); + drm_crtc_vblank_on(&crtc->base); +} + static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config, struct drm_atomic_state *old_state) { @@ -5617,7 +5647,7 @@ static void ironlake_crtc_enable(struct intel_crtc_state *pipe_config, ironlake_pch_enable(old_intel_state, pipe_config);

assert_vblank_disabled(crtc); - drm_crtc_vblank_on(crtc); + intel_crtc_vblank_on(pipe_config);

intel_encoders_enable(crtc, pipe_config, old_state);

@@ -5774,7 +5804,7 @@ static void haswell_crtc_enable(struct intel_crtc_state *pipe_config, intel_ddi_set_vc_payload_alloc(pipe_config, true);

assert_vblank_disabled(crtc); - drm_crtc_vblank_on(crtc); + intel_crtc_vblank_on(pipe_config);

intel_encoders_enable(crtc, pipe_config, old_state);

@@ -6114,7 +6144,7 @@ static void valleyview_crtc_enable(struct intel_crtc_state *pipe_config, intel_enable_pipe(pipe_config);

assert_vblank_disabled(crtc); - drm_crtc_vblank_on(crtc); + intel_crtc_vblank_on(pipe_config);

intel_encoders_enable(crtc, pipe_config, old_state); } @@ -6173,7 +6203,7 @@ static void i9xx_crtc_enable(struct intel_crtc_state *pipe_config, intel_enable_pipe(pipe_config);

assert_vblank_disabled(crtc); - drm_crtc_vblank_on(crtc); + intel_crtc_vblank_on(pipe_config);

intel_encoders_enable(crtc, pipe_config, old_state); } @@ -12653,8 +12683,9 @@ static int intel_atomic_prepare_commit(struct drm_device *dev, u32 intel_crtc_get_vblank_counter(struct intel_crtc *crtc) { struct drm_device *dev = crtc->base.dev; + struct drm_vblank_crtc *vblank = &dev->vblank[drm_crtc_index(&crtc->base)];

- if (!dev->max_vblank_count) + if (!vblank->max_vblank_count) return (u32)drm_crtc_accurate_vblank_count(&crtc->base);

return dev->driver->get_vblank_counter(dev, crtc->pipe); @@ -15736,10 +15767,12 @@ intel_modeset_setup_hw_state(struct drm_device *dev, * waits, so we need vblank interrupts restored beforehand. */ for_each_intel_crtc(&dev_priv->drm, crtc) { + crtc_state = to_intel_crtc_state(crtc->base.state); + drm_crtc_vblank_reset(&crtc->base);

- if (crtc->base.state->active) - drm_crtc_vblank_on(&crtc->base); + if (crtc_state->base.active) + intel_crtc_vblank_on(crtc_state); }

intel_sanitize_plane_mapping(dev_priv);