From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f47.google.com ([74.125.82.47]:34291 "EHLO mail-wm0-f47.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751861AbcBIKA3 (ORCPT ); Tue, 9 Feb 2016 05:00:29 -0500 Received: by mail-wm0-f47.google.com with SMTP id 128so189401610wmz.1 for ; Tue, 09 Feb 2016 02:00:29 -0800 (PST) Date: Tue, 9 Feb 2016 11:00:51 +0100 From: Daniel Vetter To: Mario Kleiner Cc: dri-devel@lists.freedesktop.org, linux@bernd-steinhauser.de, stable@vger.kernel.org, michel@daenzer.net, vbabka@suse.cz, ville.syrjala@linux.intel.com, daniel.vetter@ffwll.ch, alexander.deucher@amd.com, christian.koenig@amd.com Subject: Re: [PATCH 3/6] drm: Fix drm_vblank_pre/post_modeset regression from Linux 4.4 Message-ID: <20160209100051.GN11240@phenom.ffwll.local> References: <1454894009-15466-1-git-send-email-mario.kleiner.de@gmail.com> <1454894009-15466-4-git-send-email-mario.kleiner.de@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1454894009-15466-4-git-send-email-mario.kleiner.de@gmail.com> Sender: stable-owner@vger.kernel.org List-ID: On Mon, Feb 08, 2016 at 02:13:26AM +0100, Mario Kleiner wrote: > Changes to drm_update_vblank_count() in Linux 4.4 broke the > behaviour of the pre/post modeset functions as the new update > code doesn't deal with hw vblank counter resets inbetween calls > to drm_vblank_pre_modeset an drm_vblank_post_modeset, as it > should. > > This causes mistreatment of such hw counter resets as counter > wraparound, and thereby large forward jumps of the software > vblank counter which in turn cause vblank event dispatching > and vblank waits to fail/hang --> userspace clients hang. > > This symptom was reported on radeon-kms to cause a infinite > hang of KDE Plasma 5 shell's login procedure, preventing users > from logging in. > > Fix this by detecting when drm_update_vblank_count() is called > inside a pre->post modeset interval. If so, clamp valid vblank > increments to the safe values 0 and 1, pretty much restoring > the update behavior of the old update code of Linux 4.3 and > earlier. Also reset the last recorded hw vblank count at call > to drm_vblank_post_modeset() to be safe against hw that after > modesetting, dpms on etc. only fires its first vblank irq after > drm_vblank_post_modeset() was already called. > > Reported-by: Vlastimil Babka > Signed-off-by: Mario Kleiner > Cc: # 4.4+ > Cc: michel@daenzer.net > Cc: vbabka@suse.cz > Cc: ville.syrjala@linux.intel.com > Cc: daniel.vetter@ffwll.ch > Cc: dri-devel@lists.freedesktop.org > Cc: alexander.deucher@amd.com > Cc: christian.koenig@amd.com We need to untangle the new vblank stuff that assumes solide (atomic) drivers and all the old stuff much more. But that can be done later on. Reviewed-by: Daniel Vetter > --- > drivers/gpu/drm/drm_irq.c | 16 ++++++++++++++++ > 1 file changed, 16 insertions(+) > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index aa2c74b..5c27ad3 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -222,6 +222,21 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, > } > > /* > + * Within a drm_vblank_pre_modeset - drm_vblank_post_modeset > + * interval? If so then vblank irqs keep running and it will likely > + * happen that the hardware vblank counter is not trustworthy as it > + * might reset at some point in that interval and vblank timestamps > + * are not trustworthy either in that interval. Iow. this can result > + * in a bogus diff >> 1 which must be avoided as it would cause > + * random large forward jumps of the software vblank counter. > + */ > + if (diff > 1 && (vblank->inmodeset & 0x2)) { > + DRM_DEBUG_VBL("clamping vblank bump to 1 on crtc %u: diffr=%u" > + " due to pre-modeset.\n", pipe, diff); > + diff = 1; > + } > + > + /* > * Restrict the bump of the software vblank counter to a safe maximum > * value of +1 whenever there is the possibility that concurrent readers > * of vblank timestamps could be active at the moment, as the current > @@ -1573,6 +1588,7 @@ void drm_vblank_post_modeset(struct drm_device *dev, unsigned int pipe) > if (vblank->inmodeset) { > spin_lock_irqsave(&dev->vbl_lock, irqflags); > dev->vblank_disable_allowed = true; > + drm_reset_vblank_timestamp(dev, pipe); > spin_unlock_irqrestore(&dev->vbl_lock, irqflags); > > if (vblank->inmodeset & 0x2) > -- > 1.9.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch