From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 1/6] drm: No-Op redundant calls to drm_vblank_off() Date: Tue, 9 Feb 2016 10:54:55 +0100 Message-ID: <20160209095455.GL11240@phenom.ffwll.local> References: <1454894009-15466-1-git-send-email-mario.kleiner.de@gmail.com> <1454894009-15466-2-git-send-email-mario.kleiner.de@gmail.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wm0-f67.google.com (mail-wm0-f67.google.com [74.125.82.67]) by gabe.freedesktop.org (Postfix) with ESMTPS id 651236E530 for ; Tue, 9 Feb 2016 01:54:33 -0800 (PST) Received: by mail-wm0-f67.google.com with SMTP id c200so2379241wme.0 for ; Tue, 09 Feb 2016 01:54:33 -0800 (PST) Content-Disposition: inline In-Reply-To: <1454894009-15466-2-git-send-email-mario.kleiner.de@gmail.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Mario Kleiner Cc: dri-devel@lists.freedesktop.org, daniel.vetter@ffwll.ch, michel@daenzer.net, linux@bernd-steinhauser.de, stable@vger.kernel.org, alexander.deucher@amd.com, christian.koenig@amd.com, vbabka@suse.cz List-Id: dri-devel@lists.freedesktop.org T24gTW9uLCBGZWIgMDgsIDIwMTYgYXQgMDI6MTM6MjRBTSArMDEwMCwgTWFyaW8gS2xlaW5lciB3 cm90ZToKPiBPdGhlcndpc2UgaWYgYSBrbXMgZHJpdmVyIGNhbGxzIGludG8gZHJtX3ZibGFua19v ZmYoKSBtb3JlIHRoYW4gb25jZQo+IGJlZm9yZSBjYWxsaW5nIGRybV92Ymxhbmtfb24oKSBhZ2Fp biwgdGhlIHJlZHVuZGFudCBjYWxscyB0bwo+IHZibGFua19kaXNhYmxlX2FuZF9zYXZlKCkgd2ls bCBjYWxsIGRybV91cGRhdGVfdmJsYW5rX2NvdW50KCkKPiB3aGlsZSBodyB2YmxhbmsgY291bnRl cnMgYW5kIHZibGFuayB0aW1lc3RhbXBpbmcgYXJlIGluIGEgdW5kZWZpbmVkCj4gc3RhdGUgZHVy aW5nIG1vZGVzZXRzLCBkcG1zIG9mZiBldGMuCj4gCj4gQXQgbGVhc3Qgd2l0aCB0aGUgbGVnYWN5 IGRybSBoZWxwZXJzIGl0IGlzIG5vdCB1bnVzdWFsIHRvCj4gZ2V0IG11bHRpcGxlIGNhbGxzIHRv IGRybV92Ymxhbmtfb2ZmIGFuZCBkcm1fdmJsYW5rX29uLCBlLmcuLAo+IGhhbGYgYSBkb3plbiBj YWxscyB0byBkcm1fdmJsYW5rX29mZiBhbmQgdHdvIGNhbGxzIHRvIGRybV92Ymxhbmtfb24KPiB3 ZXJlIG9ic2VydmVkIG9uIHJhZGVvbi1rbXMgZHVyaW5nIGRwbXMtb2ZmIC0+IGRwbXMtb24gdHJh bnNpdGlvbi4KPiAKPiBGaXhlcyBsYXJnZSBqdW1wcyBvZiB0aGUgc29mdHdhcmUgbWFpbnRhaW5l ZCB2YmxhbmsgY291bnRlciBkdWUgdG8KPiB0aGUgaGFyZHdhcmUgdmJsYW5rIGNvdW50ZXIgcmVz ZXR0aW5nIHRvIHplcm8gZHVyaW5nIGRwbXMgb2ZmIG9yCj4gbW9kZXNldCwgZS5nLiwgaWYgcmFk ZW9uLWttcyBpcyBtb2RpZmllZCB0byB1c2UgZHJtX3ZibGFua19vZmYvb24KPiBpbnN0ZWFkIG9m IGRybV92YmxhbmtfcHJlL3Bvc3RfbW9kZXNldCgpLgo+IAo+IFRoaXMgZml4ZXMgYSByZWdyZXNz aW9uIGNhdXNlZCBieSB0aGUgY2hhbmdlcyBtYWRlIHRvCj4gZHJtX3VwZGF0ZV92YmxhbmtfY291 bnQoKSBpbiBMaW51eCA0LjQuCj4gCj4gU2lnbmVkLW9mZi1ieTogTWFyaW8gS2xlaW5lciA8bWFy aW8ua2xlaW5lci5kZUBnbWFpbC5jb20+Cj4gQ2M6IDxzdGFibGVAdmdlci5rZXJuZWwub3JnPiAj IDQuNCsKPiBDYzogbWljaGVsQGRhZW56ZXIubmV0Cj4gQ2M6IHZiYWJrYUBzdXNlLmN6Cj4gQ2M6 IHZpbGxlLnN5cmphbGFAbGludXguaW50ZWwuY29tCj4gQ2M6IGRhbmllbC52ZXR0ZXJAZmZ3bGwu Y2gKPiBDYzogZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwo+IENjOiBhbGV4YW5kZXIu ZGV1Y2hlckBhbWQuY29tCj4gQ2M6IGNocmlzdGlhbi5rb2VuaWdAYW1kLmNvbQo+IC0tLQo+ICBk cml2ZXJzL2dwdS9kcm0vZHJtX2lycS5jIHwgMTEgKysrKysrKysrKy0KPiAgMSBmaWxlIGNoYW5n ZWQsIDEwIGluc2VydGlvbnMoKyksIDEgZGVsZXRpb24oLSkKPiAKPiBkaWZmIC0tZ2l0IGEvZHJp dmVycy9ncHUvZHJtL2RybV9pcnEuYyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1faXJxLmMKPiBpbmRl eCA2MDdmNDkzLi5iY2I4NTI4IDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9kcm1faXJx LmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vZHJtX2lycS5jCj4gQEAgLTEzMTMsNyArMTMxMywx MyBAQCB2b2lkIGRybV92Ymxhbmtfb2ZmKHN0cnVjdCBkcm1fZGV2aWNlICpkZXYsIHVuc2lnbmVk IGludCBwaXBlKQo+ICAJc3Bpbl9sb2NrX2lycXNhdmUoJmRldi0+ZXZlbnRfbG9jaywgaXJxZmxh Z3MpOwo+ICAKPiAgCXNwaW5fbG9jaygmZGV2LT52YmxfbG9jayk7Cj4gLQl2YmxhbmtfZGlzYWJs ZV9hbmRfc2F2ZShkZXYsIHBpcGUpOwo+ICsJRFJNX0RFQlVHX1ZCTCgiY3J0YyAlZCwgdmJsYW5r IGVuYWJsZWQgJWQsIGlubW9kZXNldCAlZFxuIiwKPiArCQkgICAgICBwaXBlLCB2YmxhbmstPmVu YWJsZWQsIHZibGFuay0+aW5tb2Rlc2V0KTsKPiArCj4gKwkvKiBBdm9pZCByZWR1bmRhbnQgdmJs YW5rIGRpc2FibGVzIHdpdGhvdXQgcHJldmlvdXMgZHJtX3ZibGFua19vbigpLiAqLwo+ICsJaWYg KCF2YmxhbmstPmlubW9kZXNldCkKClNpbmNlIGF0b21pYyBkcml2ZXJzIHNob3VsZG4ndCBzdWNr IHNvIGJhZGx5IGF0IHRoaXMsIG1heWJlCgoJaWYgKERSSVZFUl9BVE9NSUMgfHwgIXZibGFuay0+ aW5tb2Rlc2V0KQoKaW5zdGVhZD8gVGhhdCB3YXkgdGhlIGZsZXhpYmlsaXR5IHdvdWxkIGJlIGNv bnN0cmFpbnQgdG8gb2xkIGRyaXZlcnMgdGhhdApuZWVkIGl0IGJlY2F1c2UgbGVnYWN5IGNydGMg aGVscGVycyBzdWNrLiBXaXRoIHRoYXQgY2hhbmdlOgoKUmV2aWV3ZWQtYnk6IERhbmllbCBWZXR0 ZXIgPGRhbmllbC52ZXR0ZXJAZmZ3bGwuY2g+Cgo+ICsJCXZibGFua19kaXNhYmxlX2FuZF9zYXZl KGRldiwgcGlwZSk7Cj4gKwo+ICAJd2FrZV91cCgmdmJsYW5rLT5xdWV1ZSk7Cj4gIAo+ICAJLyoK PiBAQCAtMTQxNSw2ICsxNDIxLDkgQEAgdm9pZCBkcm1fdmJsYW5rX29uKHN0cnVjdCBkcm1fZGV2 aWNlICpkZXYsIHVuc2lnbmVkIGludCBwaXBlKQo+ICAJCXJldHVybjsKPiAgCj4gIAlzcGluX2xv Y2tfaXJxc2F2ZSgmZGV2LT52YmxfbG9jaywgaXJxZmxhZ3MpOwo+ICsJRFJNX0RFQlVHX1ZCTCgi Y3J0YyAlZCwgdmJsYW5rIGVuYWJsZWQgJWQsIGlubW9kZXNldCAlZFxuIiwKPiArCQkgICAgICBw aXBlLCB2YmxhbmstPmVuYWJsZWQsIHZibGFuay0+aW5tb2Rlc2V0KTsKPiArCj4gIAkvKiBEcm9w IG91ciBwcml2YXRlICJwcmV2ZW50IGRybV92YmxhbmtfZ2V0IiByZWZjb3VudCAqLwo+ICAJaWYg KHZibGFuay0+aW5tb2Rlc2V0KSB7Cj4gIAkJYXRvbWljX2RlYygmdmJsYW5rLT5yZWZjb3VudCk7 Cj4gLS0gCj4gMS45LjEKPiAKCi0tIApEYW5pZWwgVmV0dGVyClNvZnR3YXJlIEVuZ2luZWVyLCBJ bnRlbCBDb3Jwb3JhdGlvbgpodHRwOi8vYmxvZy5mZndsbC5jaApfX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1k ZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcv bWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f65.google.com ([74.125.82.65]:35950 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756074AbcBIJyd (ORCPT ); Tue, 9 Feb 2016 04:54:33 -0500 Received: by mail-wm0-f65.google.com with SMTP id 128so2360502wmz.3 for ; Tue, 09 Feb 2016 01:54:32 -0800 (PST) Date: Tue, 9 Feb 2016 10:54:55 +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 1/6] drm: No-Op redundant calls to drm_vblank_off() Message-ID: <20160209095455.GL11240@phenom.ffwll.local> References: <1454894009-15466-1-git-send-email-mario.kleiner.de@gmail.com> <1454894009-15466-2-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-2-git-send-email-mario.kleiner.de@gmail.com> Sender: stable-owner@vger.kernel.org List-ID: On Mon, Feb 08, 2016 at 02:13:24AM +0100, Mario Kleiner wrote: > Otherwise if a kms driver calls into drm_vblank_off() more than once > before calling drm_vblank_on() again, the redundant calls to > vblank_disable_and_save() will call drm_update_vblank_count() > while hw vblank counters and vblank timestamping are in a undefined > state during modesets, dpms off etc. > > At least with the legacy drm helpers it is not unusual to > get multiple calls to drm_vblank_off and drm_vblank_on, e.g., > half a dozen calls to drm_vblank_off and two calls to drm_vblank_on > were observed on radeon-kms during dpms-off -> dpms-on transition. > > Fixes large jumps of the software maintained vblank counter due to > the hardware vblank counter resetting to zero during dpms off or > modeset, e.g., if radeon-kms is modified to use drm_vblank_off/on > instead of drm_vblank_pre/post_modeset(). > > This fixes a regression caused by the changes made to > drm_update_vblank_count() in Linux 4.4. > > 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 > --- > drivers/gpu/drm/drm_irq.c | 11 ++++++++++- > 1 file changed, 10 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c > index 607f493..bcb8528 100644 > --- a/drivers/gpu/drm/drm_irq.c > +++ b/drivers/gpu/drm/drm_irq.c > @@ -1313,7 +1313,13 @@ void drm_vblank_off(struct drm_device *dev, unsigned int pipe) > spin_lock_irqsave(&dev->event_lock, irqflags); > > spin_lock(&dev->vbl_lock); > - vblank_disable_and_save(dev, pipe); > + DRM_DEBUG_VBL("crtc %d, vblank enabled %d, inmodeset %d\n", > + pipe, vblank->enabled, vblank->inmodeset); > + > + /* Avoid redundant vblank disables without previous drm_vblank_on(). */ > + if (!vblank->inmodeset) Since atomic drivers shouldn't suck so badly at this, maybe if (DRIVER_ATOMIC || !vblank->inmodeset) instead? That way the flexibility would be constraint to old drivers that need it because legacy crtc helpers suck. With that change: Reviewed-by: Daniel Vetter > + vblank_disable_and_save(dev, pipe); > + > wake_up(&vblank->queue); > > /* > @@ -1415,6 +1421,9 @@ void drm_vblank_on(struct drm_device *dev, unsigned int pipe) > return; > > spin_lock_irqsave(&dev->vbl_lock, irqflags); > + DRM_DEBUG_VBL("crtc %d, vblank enabled %d, inmodeset %d\n", > + pipe, vblank->enabled, vblank->inmodeset); > + > /* Drop our private "prevent drm_vblank_get" refcount */ > if (vblank->inmodeset) { > atomic_dec(&vblank->refcount); > -- > 1.9.1 > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch