From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH 2/9] drm/i915: Warn if drm_vblank_get() still works after drm_vblank_off() Date: Mon, 26 May 2014 16:36:26 +0300 Message-ID: <20140526133626.GH27580@intel.com> References: <1401104792-26560-1-git-send-email-ville.syrjala@linux.intel.com> <1401104792-26560-3-git-send-email-ville.syrjala@linux.intel.com> <20140526132245.GJ14357@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <20140526132245.GJ14357@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, May 26, 2014 at 03:22:45PM +0200, Daniel Vetter wrote: > On Mon, May 26, 2014 at 02:46:25PM +0300, ville.syrjala@linux.intel.com w= rote: > > From: Ville Syrj=E4l=E4 > > = > > Signed-off-by: Ville Syrj=E4l=E4 > > --- > > drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++++++ > > 1 file changed, 15 insertions(+) > > = > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i91= 5/intel_display.c > > index 3da73ef..da318a7 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -1305,6 +1305,17 @@ static void assert_sprites_disabled(struct drm_i= 915_private *dev_priv, > > } > > } > > = > > +static void assert_vblank_disabled(struct intel_crtc *crtc) > > +{ > > + struct drm_device *dev =3D crtc->base.dev; > > + enum pipe pipe =3D crtc->pipe; > > + > > + if (WARN_ON(drm_vblank_get(dev, pipe) =3D=3D 0)) { > > + drm_vblank_put(dev, pipe); > > + drm_vblank_off(dev, pipe); > = > Imo the _off is too much, since with that it's not just an assert but a > "... and please make it so if not". Imo better to drop that. The idea was that if drm_vblank_get() managed to re-enable vblank interrupts when it wasn't supposed to, we should turn them off again. But the whole thing is a bug, and another drm_vblank_get() might happen just after the drm_vblank_off() anyway, so I guess it's a bit pointless. > -Daniel > = > > + } > > +} > > + > > static void ibx_assert_pch_refclk_enabled(struct drm_i915_private *dev= _priv) > > { > > u32 val; > > @@ -3885,6 +3896,8 @@ static void intel_crtc_enable_planes(struct drm_c= rtc *crtc) > > int pipe =3D intel_crtc->pipe; > > int plane =3D intel_crtc->plane; > > = > > + assert_vblank_disabled(intel_crtc); > > + > > intel_enable_primary_hw_plane(dev_priv, plane, pipe); > > intel_enable_planes(crtc); > > /* The fixup needs to happen before cursor is enabled */ > > @@ -3921,6 +3934,8 @@ static void intel_crtc_disable_planes(struct drm_= crtc *crtc) > > intel_crtc_update_cursor(crtc, false); > > intel_disable_planes(crtc); > > intel_disable_primary_hw_plane(dev_priv, plane, pipe); > > + > > + assert_vblank_disabled(intel_crtc); > > } > > = > > static void ironlake_crtc_enable(struct drm_crtc *crtc) > > -- = > > 1.8.5.5 > > = > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/dri-devel > = > -- = > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch -- = Ville Syrj=E4l=E4 Intel OTC