From 32db0b6501d97b09e92e70caefc74fa35aa9a8d6 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Ville=20Syrj=C3=A4l=C3=A4?= Date: Tue, 27 Nov 2018 22:05:50 +0200 Subject: [PATCH] drm/i915: Don't try to use the hardware frame counter with i965gm TV output MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 v3: max_vblank_count not populated yet in intel_enable_pipe() use intel_crtc_max_vblank_count() instead Cc: stable@vger.kernel.org Cc: Daniel Vetter Fixes: 51e31d49c890 ("drm/i915: Use generic vblank wait") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93782 Signed-off-by: Ville Syrjälä Link: https://patchwork.freedesktop.org/patch/msgid/20190122125149.GE5527@ideak-desk.fi.intel.com Reviewed-by: Imre Deak --- drivers/gpu/drm/i915/i915_irq.c | 27 ++++++++++------ drivers/gpu/drm/i915/intel_display.c | 48 +++++++++++++++++++++++----- 2 files changed, 58 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 7056ae2d1e0e..926e05f0db24 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -821,11 +821,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; @@ -4579,16 +4594,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_GEN(dev_priv, 2)) { - /* 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 36c1126cbc85..67a64cbbe851 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1758,6 +1758,35 @@ enum pipe intel_crtc_pch_transcoder(struct intel_crtc *crtc) return crtc->pipe; } +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 intel_enable_pipe(const struct intel_crtc_state *new_crtc_state) { struct intel_crtc *crtc = to_intel_crtc(new_crtc_state->base.crtc); @@ -1810,7 +1839,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 (intel_crtc_max_vblank_count(new_crtc_state) == 0) intel_wait_for_pipe_scanline_moving(crtc); } @@ -5678,7 +5707,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); @@ -5832,7 +5861,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); @@ -6171,7 +6200,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); } @@ -6230,7 +6259,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); } @@ -12778,8 +12807,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); @@ -15894,10 +15924,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); -- 2.30.2