From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-8.5 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED, USER_AGENT_MUTT autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id AD7D4C282C4 for ; Tue, 22 Jan 2019 12:51:56 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 720A5217F9 for ; Tue, 22 Jan 2019 12:51:56 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727936AbfAVMvz (ORCPT ); Tue, 22 Jan 2019 07:51:55 -0500 Received: from mga18.intel.com ([134.134.136.126]:22678 "EHLO mga18.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728215AbfAVMvy (ORCPT ); Tue, 22 Jan 2019 07:51:54 -0500 X-Amp-Result: UNSCANNABLE X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by orsmga106.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 22 Jan 2019 04:51:51 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.56,506,1539673200"; d="scan'208";a="136701993" Received: from ideak-desk.fi.intel.com ([10.237.72.62]) by fmsmga002.fm.intel.com with ESMTP; 22 Jan 2019 04:51:50 -0800 Date: Tue, 22 Jan 2019 14:51:49 +0200 From: Imre Deak To: Ville Syrjala Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org, dri-devel@lists.freedesktop.org Subject: Re: [PATCH v3 02/15] drm/i915: Don't try to use the hardware frame counter with i965gm TV output Message-ID: <20190122125149.GE5527@ideak-desk.fi.intel.com> Reply-To: imre.deak@intel.com References: <20181112170000.27531-3-ville.syrjala@linux.intel.com> <20181127200550.19727-1-ville.syrjala@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <20181127200550.19727-1-ville.syrjala@linux.intel.com> User-Agent: Mutt/1.9.4 (2018-02-28) Sender: stable-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: stable@vger.kernel.org On Tue, Nov 27, 2018 at 10:05:50PM +0200, Ville Syrjala wrote: > From: Ville Syrjälä > > 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ä > > fix# _slub_broken.S ^ some remnant 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 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@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel