From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mario Kleiner Subject: Re: [PATCH 5/6] drm: Prevent vblank counter jumps with timestamp based update method. Date: Wed, 10 Feb 2016 17:28:26 +0100 Message-ID: <56BB652A.1010107@gmail.com> References: <1454894009-15466-1-git-send-email-mario.kleiner.de@gmail.com> <1454894009-15466-6-git-send-email-mario.kleiner.de@gmail.com> <20160209100917.GP11240@phenom.ffwll.local> <56B9EF5A.6050902@gmail.com> <20160209141149.GP23290@intel.com> <20160209150347.GZ11240@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com [74.125.82.66]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9AC056E723 for ; Wed, 10 Feb 2016 08:28:31 -0800 (PST) Received: by mail-wm0-f66.google.com with SMTP id p63so5281942wmp.1 for ; Wed, 10 Feb 2016 08:28:31 -0800 (PST) In-Reply-To: <20160209150347.GZ11240@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 , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= Cc: daniel.vetter@ffwll.ch, michel@daenzer.net, linux@bernd-steinhauser.de, stable@vger.kernel.org, dri-devel@lists.freedesktop.org, alexander.deucher@amd.com, christian.koenig@amd.com, vbabka@suse.cz List-Id: dri-devel@lists.freedesktop.org T24gMDIvMDkvMjAxNiAwNDowMyBQTSwgRGFuaWVsIFZldHRlciB3cm90ZToKPiBPbiBUdWUsIEZl YiAwOSwgMjAxNiBhdCAwNDoxMTo0OVBNICswMjAwLCBWaWxsZSBTeXJqw6Rsw6Qgd3JvdGU6Cj4+ IE9uIFR1ZSwgRmViIDA5LCAyMDE2IGF0IDAyOjUzOjMwUE0gKzAxMDAsIE1hcmlvIEtsZWluZXIg d3JvdGU6Cj4+PiBPbiAwMi8wOS8yMDE2IDExOjA5IEFNLCBEYW5pZWwgVmV0dGVyIHdyb3RlOgo+ Pj4+IE9uIE1vbiwgRmViIDA4LCAyMDE2IGF0IDAyOjEzOjI4QU0gKzAxMDAsIE1hcmlvIEtsZWlu ZXIgd3JvdGU6Cj4+Pj4+IFRoZSBjaGFuZ2VzIHRvIGRybV91cGRhdGVfdmJsYW5rX2NvdW50KCkg aW4gTGludXggNC40IGFkZGVkIGEKPj4+Pj4gbWV0aG9kIHRvIGVtdWxhdGUgYSBoYXJkd2FyZSB2 YmxhbmsgY291bnRlciBieSB1c2Ugb2YgaGlnaAo+Pj4+PiBwcmVjaXNpb24gdmJsYW5rIHRpbWVz dGFtcHMgaWYgYSBrbXMgZHJpdmVyIHN1cHBvcnRzIHRob3NlLAo+Pj4+PiBidXQgZG9lc24ndCBz dXBwcG9ydCBodyB2YmxhbmsgY291bnRlcnMuCj4+Pj4+Cj4+Pj4+IFRoYXQgbWV0aG9kIGFzc3Vt ZXMgdGhhdCB0aGUgb2xkIHRpbWVzdGFtcCBmcm9tIGEgcHJldmlvdXMKPj4+Pj4gaW52b2NhdGlv biBpcyB2YWxpZCwgYnV0IHRoYXQgaXMgbm90IGFsd2F5cyB0aGUgY2FzZS4gRS5nLiwKPj4+Pj4g aWYgZHJtX3Jlc2V0X3ZibGFua190aW1lc3RhbXAoKSBnZXRzIGNhbGxlZCBkdXJpbmcgZHJtX3Zi bGFua19vbigpCj4+Pj4+IG9yIGRybV91cGRhdGVfdmJsYW5rX2NvdW50KCkgZ2V0cyBjYWxsZWQg b3V0c2lkZSB2YmxhbmsgaXJxIGFuZAo+Pj4+PiB0aGUgaGlnaCBwcmVjaXNpb24gdGltZXN0YW1w aW5nIGNhbid0IGRlbGl2ZXIgYSBwcmVjaXNlIHRpbWVzdGFtcCwKPj4+Pj4gaWUuIGRybV9nZXRf bGFzdF92Ymx0aW1lc3RhbXAoKSBkZWxpdmVycyBhIHJldHVybiB2YWx1ZSBvZiBmYWxzZSwKPj4+ Pj4gdGhlbiB0aG9zZSBmdW5jdGlvbnMgd2lsbCBpbml0aWFsaXplIHRoZSBvbGQgdGltZXN0YW1w IHRvIHplcm8KPj4+Pj4gdG8gbWFyayBpdCBhcyBpbnZhbGlkLgo+Pj4+Pgo+Pj4+PiBBIGZvbGxv d2luZyBjYWxsIHRvIGRybV91cGRhdGVfdmJsYW5rX2NvdW50KCkgd291bGQgdGhlbiBjYWxjdWxh dGUKPj4+Pj4gZWxhcHNlZCB0aW1lIHdpdGggdmJsYW5rIGlycXMgb2ZmIGFzIGN1cnJlbnQgdmJs YW5rIHRpbWVzdGFtcCBtaW51cwo+Pj4+PiB0aGUgemVybyBvbGQgdGltZXN0YW1wIGFuZCBjb21w dXRlIGEgc29mdHdhcmUgdmJsYW5rIGNvdW50ZXIgaW5jcmVtZW50Cj4+Pj4+IHRoYXQgY29ycmVz cG9uZHMgdG8gc3lzdGVtIHVwdGltZSwgY2F1c2luZyBhIGxhcmdlIGZvcndhcmQganVtcCBvZiB0 aGUKPj4+Pj4gc29mdHdhcmUgdmJsYW5rIGNvdW50ZXIuIFRoYXQganVtcCBpbiB0dXJuIGNhbiBj YXVzZSB0b28gbG9uZyB3YWl0cwo+Pj4+PiBpbiBkcm1XYWl0VmJsYW5rIGFuZCB2ZXJ5IGxvbmcg ZGVsYXlzIGluIGRlbGl2ZXJ5IG9mIHZibGFuayBldmVudHMsCj4+Pj4+IHJlc3VsdGluZyBpbiBo YW5ncyBvZiB1c2Vyc3BhY2UgY2xpZW50cy4KPj4+Pj4KPj4+Pj4gVGhpcyBwcm9ibGVtIGNhbiBi ZSBvYnNlcnZlZCBvbiBub3V2ZWF1LWttcyBkdXJpbmcgbWFjaGluZQo+Pj4+PiBzdXNwZW5kLT5y ZXN1bWUgY3ljbGVzLCB3aGVyZSBkcm1fdmJsYW5rX29mZiBpcyBjYWxsZWQgZHVyaW5nCj4+Pj4+ IHN1c3BlbmQsIGRybV92Ymxhbmtfb24gaXMgY2FsbGVkIGR1cmluZyByZXN1bWUgYW5kIHRoZSBm aXJzdAo+Pj4+PiBxdWVyaWVzIHRvIGRybV9nZXRfbGFzdF92Ymx0aW1lc3RhbXAoKSBkb24ndCBk ZWxpdmVyIGhpZ2gKPj4+Pj4gcHJlY2lzaW9uIHRpbWVzdGFtcHMsIHJlc3VsdGluZyBpbiBhIGxh cmdlIGhhcm1mdWwgY291bnRlciBqdW1wLgo+Pj4+Cj4+Pj4gV2h5IGRvZXMgbm91dmVhdSBlbmFi bGUgdmJsYW5rIGludGVycnVwdHMgYmVmb3JlIGl0IGNhbiBnZXQgdmFsaWQKPj4+PiB0aW1lc3Rh bXBzPyBUaGF0IHNvdW5kcyBsaWtlIHRoZSBjb3JlIGJ1ZyBoZXJlLCBhbmQgcGFwZXJpbmcgb3Zl ciB0aGF0IGluCj4+Pj4gdGhlIHZibGFuayBjb2RlIGZlZWxzIHZlcnkgd3JvbmcgdG8gbWUuIE1h eWJlIHdlIHNob3VsZCBpbnN0ZWFkIGp1c3Qgbm90Cj4+Pj4gc2FtcGxlIHRoZSB2YmxhbmsgYXQg YWxsIGlmIHRoZSB0aW1lc3RhbXAgZmFpbHMgYW5kIHNwbGF0IGEgYmlnIFdBUk5fT04KPj4+PiBh Ym91dCB0aGlzLCB0byBnaXZlIGRyaXZlciB3cml0ZXJzIGEgY2hhbmNlIHRvIGZpeCB1cCB0aGVp ciBtZXNzPwo+Pj4+IC1EYW5pZWwKPj4+Pgo+Pj4KPj4+IFRoZSBoaWdoIHByZWNpc2lvbiB0aW1l c3RhbXBpbmcgaXMgYWxsb3dlZCB0byBmYWlsIGZvciBhIGttcyBkcml2ZXIKPj4+IHVuZGVyIHNv bWUgY29uZGl0aW9ucyB3aGljaCBhcmUgbm90IGltcGxlbWVudGF0aW9uIGVycm9ycyBvZiB0aGUg ZHJpdmVyLAo+Pj4gb3IgZ2V0dGluZyBkaXNhYmxlZCBieSB1c2VyIG92ZXJyaWRlLCBzbyB3ZSBz aG91bGQgaGFuZGxlIHRoYXQgcm9idXN0bHkuCj4+PiBXZSBoYW5kbGUgaXQgcm9idXN0bHkgZXZl cnl3aGVyZSBlbHNlIGluIHRoZSBkcm0sIHNvIHdlIHNob3VsZCBkbyBpdAo+Pj4gaGVyZSBhcyB3 ZWxsLgo+Pj4KPj4+IEUuZy4sIG5vdXZlYXUgZ2VuZXJhbGx5IHN1cHBvcnRzIHRpbWVzdGFtcGlu Zy9zY2Fub3V0IHBvc2l0aW9uIHF1ZXJpZXMsCj4+PiBidXQgY2FuJ3Qgc3VwcG9ydCBpdCBvbiBv bGQgcHJlIE5WLTUwIGhhcmR3YXJlIHdpdGggc29tZSBvdXRwdXQgdHlwZQo+Pj4gKGVpdGhlciBv biBhbmFsb2cgVkdBLCBvciBEVkktRCwgY2FuJ3QgcmVtZW1iZXIgYXRtLiB3aGljaCBvbmUgaXMK Pj4+IHVuc3VwcG9ydGVkKS4KPj4KPj4gSSB0aGluayB0aGUgc3VycHJpc2luZyB0aGluZyBoZXJl IGlzIHRoYXQgaXQgZmFpbHMgZmlyc3QgYW5kIHRoZW4KPj4gc3VjY2VlZHMgb24gdGhlIHNhbWUg Y3J0Yy9vdXRwdXQgY29tYm8gcHJlc3VtYWJseS4KPgo+IFllYWggZXhhY3RseSB0aGlzLiBGYWls aW5nIGNvbnNpc3RlbnRseSBpcyBvayBpbW8gKGFuZCBwcm9iYWJseSBzaG91bGQgYmUKPiBoYW5k bGVkKS4gRmFpbGluZyBmaXJzdCBhbmQgdGhlbiBsYXRlciBvbiB3b3JraW5nICh3aXRob3V0IGNo YW5naW5nIHRoZQo+IG1vZGUgY29uZmlnIGluIGJldHdlZW4pIHNlZW1zIHN1c3BpY291cy4gVGhh dCBzaG91bGRuJ3QgZXZlciBoYXBwZW4gcmVhbGx5Cj4gYW5kIHNlZW1zIGxpa2UgYSBkcml2ZXIg YnVnIChsaWtlIGVuYWJsaW5nIHZibGFua3MgdG9vIGVhcmx5LCBiZWZvcmUgdGhlCj4gcGlwZSBp cyBmdWxseSB1cCZydW5uaW5nKS4KPiAtRGFuaWVsCj4KPj4KPj4gSSBndWVzcyBpbiB0aGVvcnkg dGhlIGRyaXZlciBjb3VsZCBmYWlsIGR1cmluZyByYW5kb20gdGltZXMgZm9yIHdoYXRldmVyCj4+ IHJlYXNvbiwgdGhvdWdoIEkgdGVuZCB0byB0aGluayB0aGF0IHRoZSBkcml2ZXIgc2hvdWxkIGVp dGhlciBtYWtlIGl0Cj4+IHJvYnVzdCBvciBub3QgZXZlbiBwcmV0ZW5kIHRvIHN1cHBvcnQgaXQu Cj4+Cj4+IEkgc3VwcG9zZSBvbmUgZmFpbHVyZSBtb2RlIHRoZSBkcml2ZXIgY2FuJ3QgcmVhbGx5 IGRvIGFueXRoaW5nIGFib3V0IGlzCj4+IHNvbWUgcmFuZG9tIFNNSSBjcmFwIG9yIHNvbWV0aGlu ZyBzdGFsbGluZyB0aGUgdGltZXN0YW1wIHF1ZXJpZXMgZW5vdWdoCj4+IHRoYXQgd2UgZmFpbCB0 aGUgcHJlY2lzaW9ucyB0ZXN0cyBhbmQgZXhoYXVzdCB0aGUgcmV0cnkgbGltaXRzLiBTbyB5ZWFo LAo+PiBtYWtpbmcgaXQgcm9idXN0IGFnYWluc3QgdGhhdCBraW5kIG9mIG5hc3R5bmVzcyBzb3Vu ZHMgbGluZSBpdCBtaWdodCBiZQo+PiBhIGdvb2QgaWRlYS4gVGhvdWdoIHBlcmhhcHMgaXQgc2hv dWxkIGJlIHNvbWV0aGluZyBhIGJpdCBtb3JlIHNldmVyZQo+PiB0aGFuIGEgRFJNX0RFQlVHIHNp bmNlIEkgdGhpbmsgaXQgcmVhbGx5IHNob3VsZG4ndCBoYXBwZW4gd2hlbiB0aGUKPj4gZHJpdmVy IGFuZCBzeXN0ZW0gYXJlIG90aGVyd2lzZSBzYW5lLiBPZiBjb3Vyc2UgaWYgaXQgcm91dGluZWx5 IGZhaWxzCj4+IHdpdGggc29tZSBkcml2ZXIgdGhlbiBzaW1wbHkgbWFraW5nIGl0IHNwZXcgZXJy b3JzIGludG8gZG1lc2cgaXNuJ3QKPj4gc28gbmljZSwgdW5sZXNzIHRoZSBkcml2ZXIgYWxzbyBn ZXRzIGZpeGVkLgo+PgoKSSB0aGluayBpIGhhdmUgYW4gaWRlYSB3aGF0IG1pZ2h0IGdvIHdyb25n IHdpdGggbm91dmVhdSwgc28gaSdsbCBzZWUgaWYgCmkgY2FuIGFkZCBhIGZpeHVwIHBhdGNoLgoK VGhlcmUncyBhbm90aGVyIHNjZW5hcmlvIHdoZXJlIHRoaXMgemVyby10cyBjYXNlIGNhbiBiZSBo aXQuIElmIHRoZSAKZHJpdmVyIGRybV92YmxhbmtfaW5pdCgpJ3MgLSBzZXR0aW5nIGFsbCB0aW1l c3RhbXBzIHRvIHplcm8gLSBhbmQgdGhlbiAKY29kZSBzdGFydHMgdXNpbmcgdmJsYW5rcyAoZHJt X3ZibGFua19nZXQoKSkgd2l0aG91dCBkcm1fdmJsYW5rX29uIApiZWZvcmVoYW5kLCB3aGljaCBp cyBhZmFpY3MgdGhlIGNhc2Ugd2l0aCBub3V2ZWF1LiBVbmxlc3MgdGhhdCdzIApjb25zaWRlcmVk IGFuIGVycm9yIGFzIHdlbGwsIHdlJ2QgbmVlZCB0byBpbml0IHRoZSB0aW1lc3RhbXBzIHRvIApz b21ldGhpbmcgbm9uLXplcm8gYnV0IGhhcm1sZXNzIGxpa2UgMSB1c2VjcyBhdCBkcm1fdmJsYW5r X2luaXQoKSB0aW1lPwpXaGF0IG1ha2VzIHNlbnNlIGFzIG91dHB1dCBoZXJlPyBEUk1fV0FSTl9P TkNFPwoKLW1hcmlvCgo+Pj4KPj4+IFRoZXJlIGFyZSBhbHNvIG5ldyBTb2MgZHJpdmVycyBzaG93 aW5nIHVwIHdoaWNoIHN1cHBvcnQgdGhvc2UgbWV0aG9kcywKPj4+IGJ1dCBhdCBsZWFzdCBpIGRp ZG4ndCB2ZXJpZnkgb3IgdGVzdCBpZiB0aGVpciBpbXBsZW1lbnRhdGlvbnMgYXJlIGdvb2QKPj4+ IGVub3VnaCBmb3IgdGhlIG5lZWRzIG9mIHRoZSBuZXcgZHJtX3VkcGF0ZV92YmxhbmtfY291bnQo KSB3aGljaCBpcyBtb3JlCj4+PiBzZW5zaXRpdmUgdG8ga21zIGRyaXZlcnMgYmVpbmcgZXZlbiBz bGlnaHRseSBvZmYuCj4+Pgo+Pj4gLW1hcmlvCj4+Pgo+Pj4+Pgo+Pj4+PiBGaXggdGhpcyBieSBj aGVja2luZyBpZiB0aGUgb2xkIHRpbWVzdGFtcCB1c2VkIGZvciB0aGlzIGNhbGN1bGF0aW9ucwo+ Pj4+PiBpcyB6ZXJvID09IGludmFsaWQuIElmIHNvLCBwZXJmb3JtIGEgY291bnRlciBpbmNyZW1l bnQgb2YgKzEgdG8KPj4+Pj4gcHJldmVudCBsYXJnZSBjb3VudGVyIGp1bXBzIGFuZCByZWluaXRp YWxpemUgdGhlIHRpbWVzdGFtcHMgdG8KPj4+Pj4gc2FuZSB2YWx1ZXMuCj4+Pj4+Cj4+Pj4+IFNp Z25lZC1vZmYtYnk6IE1hcmlvIEtsZWluZXIgPG1hcmlvLmtsZWluZXIuZGVAZ21haWwuY29tPgo+ Pj4+PiBDYzogPHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmc+ICMgNC40Kwo+Pj4+PiBDYzogbWljaGVs QGRhZW56ZXIubmV0Cj4+Pj4+IENjOiB2YmFia2FAc3VzZS5jego+Pj4+PiBDYzogdmlsbGUuc3ly amFsYUBsaW51eC5pbnRlbC5jb20KPj4+Pj4gQ2M6IGRhbmllbC52ZXR0ZXJAZmZ3bGwuY2gKPj4+ Pj4gQ2M6IGRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKPj4+Pj4gQ2M6IGFsZXhhbmRl ci5kZXVjaGVyQGFtZC5jb20KPj4+Pj4gQ2M6IGNocmlzdGlhbi5rb2VuaWdAYW1kLmNvbQo+Pj4+ PiAtLS0KPj4+Pj4gICAgZHJpdmVycy9ncHUvZHJtL2RybV9pcnEuYyB8IDcgKysrKysrKwo+Pj4+ PiAgICAxIGZpbGUgY2hhbmdlZCwgNyBpbnNlcnRpb25zKCspCj4+Pj4+Cj4+Pj4+IGRpZmYgLS1n aXQgYS9kcml2ZXJzL2dwdS9kcm0vZHJtX2lycS5jIGIvZHJpdmVycy9ncHUvZHJtL2RybV9pcnEu Ywo+Pj4+PiBpbmRleCBmYjE3YzQ1Li44OGJkZjE5IDEwMDY0NAo+Pj4+PiAtLS0gYS9kcml2ZXJz L2dwdS9kcm0vZHJtX2lycS5jCj4+Pj4+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1faXJxLmMK Pj4+Pj4gQEAgLTIxNiw2ICsyMTYsMTMgQEAgc3RhdGljIHZvaWQgZHJtX3VwZGF0ZV92Ymxhbmtf Y291bnQoc3RydWN0IGRybV9kZXZpY2UgKmRldiwgdW5zaWduZWQgaW50IHBpcGUsCj4+Pj4+ICAg IAkJCURSTV9ERUJVR19WQkwoImNydGMgJXU6IFJlZHVuZGFudCB2YmxpcnEgaWdub3JlZC4iCj4+ Pj4+ICAgIAkJCQkgICAgICAiIGRpZmZfbnMgPSAlbGxkLCBmcmFtZWR1cl9ucyA9ICVkKVxuIiwK Pj4+Pj4gICAgCQkJCSAgICAgIHBpcGUsIChsb25nIGxvbmcpIGRpZmZfbnMsIGZyYW1lZHVyX25z KTsKPj4+Pj4gKwo+Pj4+PiArCQkvKiBObyB2YWxpZCB0X29sZCB0byBjYWxjdWxhdGUgZGlmZj8g QnVtcCArMSB0byBmb3JjZSByZWluaXQuICovCj4+Pj4+ICsJCWlmICh0X29sZC0+dHZfc2VjID09 IDAgJiYgdF9vbGQtPnR2X3VzZWMgPT0gMCkgewo+Pj4+PiArCQkJRFJNX0RFQlVHX1ZCTCgiY3J0 YyAldTogTm8gYmFzZWxpbmUgdHMuIEJ1bXAgKzEuXG4iLAo+Pj4+PiArCQkJCSAgICAgIHBpcGUp Owo+Pj4+PiArCQkJZGlmZiA9IDE7Cj4+Pj4+ICsJCX0KPj4+Pj4gICAgCX0gZWxzZSB7Cj4+Pj4+ ICAgIAkJLyogc29tZSBraW5kIG9mIGRlZmF1bHQgZm9yIGRyaXZlcnMgdy9vIGFjY3VyYXRlIHZi bCB0aW1lc3RhbXBpbmcgKi8KPj4+Pj4gICAgCQlkaWZmID0gKGZsYWdzICYgRFJNX0NBTExFRF9G Uk9NX1ZCTElSUSkgIT0gMDsKPj4+Pj4gLS0KPj4+Pj4gMS45LjEKPj4+Pj4KPj4+Pgo+Pgo+PiAt LQo+PiBWaWxsZSBTeXJqw6Rsw6QKPj4gSW50ZWwgT1RDCj4KX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2 ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21h aWxtYW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f67.google.com ([74.125.82.67]:36594 "EHLO mail-wm0-f67.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752906AbcBJQ2b (ORCPT ); Wed, 10 Feb 2016 11:28:31 -0500 Received: by mail-wm0-f67.google.com with SMTP id 128so5263884wmz.3 for ; Wed, 10 Feb 2016 08:28:30 -0800 (PST) Subject: Re: [PATCH 5/6] drm: Prevent vblank counter jumps with timestamp based update method. To: Daniel Vetter , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= References: <1454894009-15466-1-git-send-email-mario.kleiner.de@gmail.com> <1454894009-15466-6-git-send-email-mario.kleiner.de@gmail.com> <20160209100917.GP11240@phenom.ffwll.local> <56B9EF5A.6050902@gmail.com> <20160209141149.GP23290@intel.com> <20160209150347.GZ11240@phenom.ffwll.local> Cc: dri-devel@lists.freedesktop.org, linux@bernd-steinhauser.de, stable@vger.kernel.org, michel@daenzer.net, vbabka@suse.cz, daniel.vetter@ffwll.ch, alexander.deucher@amd.com, christian.koenig@amd.com From: Mario Kleiner Message-ID: <56BB652A.1010107@gmail.com> Date: Wed, 10 Feb 2016 17:28:26 +0100 MIME-Version: 1.0 In-Reply-To: <20160209150347.GZ11240@phenom.ffwll.local> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: On 02/09/2016 04:03 PM, Daniel Vetter wrote: > On Tue, Feb 09, 2016 at 04:11:49PM +0200, Ville Syrj�l� wrote: >> On Tue, Feb 09, 2016 at 02:53:30PM +0100, Mario Kleiner wrote: >>> On 02/09/2016 11:09 AM, Daniel Vetter wrote: >>>> On Mon, Feb 08, 2016 at 02:13:28AM +0100, Mario Kleiner wrote: >>>>> The changes to drm_update_vblank_count() in Linux 4.4 added a >>>>> method to emulate a hardware vblank counter by use of high >>>>> precision vblank timestamps if a kms driver supports those, >>>>> but doesn't suppport hw vblank counters. >>>>> >>>>> That method assumes that the old timestamp from a previous >>>>> invocation is valid, but that is not always the case. E.g., >>>>> if drm_reset_vblank_timestamp() gets called during drm_vblank_on() >>>>> or drm_update_vblank_count() gets called outside vblank irq and >>>>> the high precision timestamping can't deliver a precise timestamp, >>>>> ie. drm_get_last_vbltimestamp() delivers a return value of false, >>>>> then those functions will initialize the old timestamp to zero >>>>> to mark it as invalid. >>>>> >>>>> A following call to drm_update_vblank_count() would then calculate >>>>> elapsed time with vblank irqs off as current vblank timestamp minus >>>>> the zero old timestamp and compute a software vblank counter increment >>>>> that corresponds to system uptime, causing a large forward jump of the >>>>> software vblank counter. That jump in turn can cause too long waits >>>>> in drmWaitVblank and very long delays in delivery of vblank events, >>>>> resulting in hangs of userspace clients. >>>>> >>>>> This problem can be observed on nouveau-kms during machine >>>>> suspend->resume cycles, where drm_vblank_off is called during >>>>> suspend, drm_vblank_on is called during resume and the first >>>>> queries to drm_get_last_vbltimestamp() don't deliver high >>>>> precision timestamps, resulting in a large harmful counter jump. >>>> >>>> Why does nouveau enable vblank interrupts before it can get valid >>>> timestamps? That sounds like the core bug here, and papering over that in >>>> the vblank code feels very wrong to me. Maybe we should instead just not >>>> sample the vblank at all if the timestamp fails and splat a big WARN_ON >>>> about this, to give driver writers a chance to fix up their mess? >>>> -Daniel >>>> >>> >>> The high precision timestamping is allowed to fail for a kms driver >>> under some conditions which are not implementation errors of the driver, >>> or getting disabled by user override, so we should handle that robustly. >>> We handle it robustly everywhere else in the drm, so we should do it >>> here as well. >>> >>> E.g., nouveau generally supports timestamping/scanout position queries, >>> but can't support it on old pre NV-50 hardware with some output type >>> (either on analog VGA, or DVI-D, can't remember atm. which one is >>> unsupported). >> >> I think the surprising thing here is that it fails first and then >> succeeds on the same crtc/output combo presumably. > > Yeah exactly this. Failing consistently is ok imo (and probably should be > handled). Failing first and then later on working (without changing the > mode config in between) seems suspicous. That shouldn't ever happen really > and seems like a driver bug (like enabling vblanks too early, before the > pipe is fully up&running). > -Daniel > >> >> I guess in theory the driver could fail during random times for whatever >> reason, though I tend to think that the driver should either make it >> robust or not even pretend to support it. >> >> I suppose one failure mode the driver can't really do anything about is >> some random SMI crap or something stalling the timestamp queries enough >> that we fail the precisions tests and exhaust the retry limits. So yeah, >> making it robust against that kind of nastyness sounds line it might be >> a good idea. Though perhaps it should be something a bit more severe >> than a DRM_DEBUG since I think it really shouldn't happen when the >> driver and system are otherwise sane. Of course if it routinely fails >> with some driver then simply making it spew errors into dmesg isn't >> so nice, unless the driver also gets fixed. >> I think i have an idea what might go wrong with nouveau, so i'll see if i can add a fixup patch. There's another scenario where this zero-ts case can be hit. If the driver drm_vblank_init()'s - setting all timestamps to zero - and then code starts using vblanks (drm_vblank_get()) without drm_vblank_on beforehand, which is afaics the case with nouveau. Unless that's considered an error as well, we'd need to init the timestamps to something non-zero but harmless like 1 usecs at drm_vblank_init() time? What makes sense as output here? DRM_WARN_ONCE? -mario >>> >>> There are also new Soc drivers showing up which support those methods, >>> but at least i didn't verify or test if their implementations are good >>> enough for the needs of the new drm_udpate_vblank_count() which is more >>> sensitive to kms drivers being even slightly off. >>> >>> -mario >>> >>>>> >>>>> Fix this by checking if the old timestamp used for this calculations >>>>> is zero == invalid. If so, perform a counter increment of +1 to >>>>> prevent large counter jumps and reinitialize the timestamps to >>>>> sane values. >>>>> >>>>> 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 | 7 +++++++ >>>>> 1 file changed, 7 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c >>>>> index fb17c45..88bdf19 100644 >>>>> --- a/drivers/gpu/drm/drm_irq.c >>>>> +++ b/drivers/gpu/drm/drm_irq.c >>>>> @@ -216,6 +216,13 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, >>>>> DRM_DEBUG_VBL("crtc %u: Redundant vblirq ignored." >>>>> " diff_ns = %lld, framedur_ns = %d)\n", >>>>> pipe, (long long) diff_ns, framedur_ns); >>>>> + >>>>> + /* No valid t_old to calculate diff? Bump +1 to force reinit. */ >>>>> + if (t_old->tv_sec == 0 && t_old->tv_usec == 0) { >>>>> + DRM_DEBUG_VBL("crtc %u: No baseline ts. Bump +1.\n", >>>>> + pipe); >>>>> + diff = 1; >>>>> + } >>>>> } else { >>>>> /* some kind of default for drivers w/o accurate vbl timestamping */ >>>>> diff = (flags & DRM_CALLED_FROM_VBLIRQ) != 0; >>>>> -- >>>>> 1.9.1 >>>>> >>>> >> >> -- >> Ville Syrj�l� >> Intel OTC >