On Tue, Nov 27, 2018 at 10:05:50PM +0200, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx
> Cc: Daniel Vetter <daniel@xxxxxxxx>
> 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@xxxxxxxxxxxxxxx>
>
> fix# _slub_broken.S
^ some remnant
Reviewed-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> 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 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..cb13eff78ad9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -1754,6 +1754,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);
> @@ -1806,7 +1835,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);
> }
>
> @@ -5617,7 +5646,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 +5803,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 +6143,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 +6202,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 +12682,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 +15766,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.18.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
