From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH] drm/i915: Wait for pending flips to complete before tearing down the encoders Date: Thu, 14 Feb 2013 16:18:50 +0200 Message-ID: <20130214141850.GW9135@intel.com> References: <1360779408-21705-1-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 2F445E6957 for ; Thu, 14 Feb 2013 06:18:54 -0800 (PST) Content-Disposition: inline In-Reply-To: <1360779408-21705-1-git-send-email-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Wed, Feb 13, 2013 at 06:16:48PM +0000, Chris Wilson wrote: > If we start disabling the encoders, there is a potential for a pending > flip to never occur - and so we will end up waiting indefinitely. To me that would indicate that the encoder disable hooks either kill the vblank signal or the entire pixel clock, so that the flip never completes. But if that is the case, then shouldn't we also disable all planes and indeed the whole pipe before calling the encoder disabled hooks? > = > v2: Also pre-emptively perform the drm_vblank_off() before switching off > the encoders. > = > References: https://bugs.launchpad.net/ubuntu/+source/xserver-xorg-video-= intel/+bug/1097315 > Cc: Timo Aaltonen > Cc: Daniel Vetter > Signed-off-by: Chris Wilson > --- > drivers/gpu/drm/i915/intel_display.c | 17 +++++++++-------- > 1 file changed, 9 insertions(+), 8 deletions(-) > = > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/= intel_display.c > index da1ad9c..15cc838 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3486,15 +3486,15 @@ static void ironlake_crtc_disable(struct drm_crtc= *crtc) > int plane =3D intel_crtc->plane; > u32 reg, temp; > = > - > if (!intel_crtc->active) > return; > = > + intel_crtc_wait_for_pending_flips(crtc); > + drm_vblank_off(dev, pipe); > + > for_each_encoder_on_crtc(dev, crtc, encoder) > encoder->disable(encoder); > = > - intel_crtc_wait_for_pending_flips(crtc); > - drm_vblank_off(dev, pipe); > intel_crtc_update_cursor(crtc, false); > = > intel_disable_plane(dev_priv, plane, pipe); > @@ -3570,13 +3570,14 @@ static void haswell_crtc_disable(struct drm_crtc = *crtc) > if (!intel_crtc->active) > return; > = > + intel_crtc_wait_for_pending_flips(crtc); > + drm_vblank_off(dev, pipe); > + > is_pch_port =3D haswell_crtc_driving_pch(crtc); > = > for_each_encoder_on_crtc(dev, crtc, encoder) > encoder->disable(encoder); > = > - intel_crtc_wait_for_pending_flips(crtc); > - drm_vblank_off(dev, pipe); > intel_crtc_update_cursor(crtc, false); > = > intel_disable_plane(dev_priv, plane, pipe); > @@ -3687,16 +3688,16 @@ static void i9xx_crtc_disable(struct drm_crtc *cr= tc) > int pipe =3D intel_crtc->pipe; > int plane =3D intel_crtc->plane; > = > - > if (!intel_crtc->active) > return; > = > + intel_crtc_wait_for_pending_flips(crtc); > + drm_vblank_off(dev, pipe); > + > for_each_encoder_on_crtc(dev, crtc, encoder) > encoder->disable(encoder); > = > /* Give the overlay scaler a chance to disable if it's on this pipe */ > - intel_crtc_wait_for_pending_flips(crtc); > - drm_vblank_off(dev, pipe); > intel_crtc_dpms_overlay(intel_crtc, false); > intel_crtc_update_cursor(crtc, false); > = > -- = > 1.7.10.4 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC