From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mario Kleiner Subject: Re: [PATCH 2/6] drm: Prevent vblank counter bumps > 1 with active vblank clients. Date: Tue, 9 Feb 2016 17:18:54 +0100 Message-ID: <56BA116E.4010406@gmail.com> References: <1454894009-15466-1-git-send-email-mario.kleiner.de@gmail.com> <1454894009-15466-3-git-send-email-mario.kleiner.de@gmail.com> <20160209095638.GM11240@phenom.ffwll.local> <20160209100727.GG23290@intel.com> <20160209102302.GT11240@phenom.ffwll.local> <56B9EC20.9050400@gmail.com> <20160209142957.GX11240@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-f52.google.com (mail-wm0-f52.google.com [74.125.82.52]) by gabe.freedesktop.org (Postfix) with ESMTPS id 87EDC6E131 for ; Tue, 9 Feb 2016 08:18:58 -0800 (PST) Received: by mail-wm0-f52.google.com with SMTP id c200so67673760wme.0 for ; Tue, 09 Feb 2016 08:18:58 -0800 (PST) In-Reply-To: <20160209142957.GX11240@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: 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 CgpPbiAwMi8wOS8yMDE2IDAzOjI5IFBNLCBEYW5pZWwgVmV0dGVyIHdyb3RlOgo+IE9uIFR1ZSwg RmViIDA5LCAyMDE2IGF0IDAyOjM5OjQ0UE0gKzAxMDAsIE1hcmlvIEtsZWluZXIgd3JvdGU6Cj4+ IE9uIDAyLzA5LzIwMTYgMTE6MjMgQU0sIERhbmllbCBWZXR0ZXIgd3JvdGU6Cj4+PiBPbiBUdWUs IEZlYiAwOSwgMjAxNiBhdCAxMjowNzoyN1BNICswMjAwLCBWaWxsZSBTeXJqw6Rsw6Qgd3JvdGU6 Cj4+Pj4gT24gVHVlLCBGZWIgMDksIDIwMTYgYXQgMTA6NTY6MzhBTSArMDEwMCwgRGFuaWVsIFZl dHRlciB3cm90ZToKPj4+Pj4gT24gTW9uLCBGZWIgMDgsIDIwMTYgYXQgMDI6MTM6MjVBTSArMDEw MCwgTWFyaW8gS2xlaW5lciB3cm90ZToKPj4+Pj4+IFRoaXMgZml4ZXMgYSByZWdyZXNzaW9uIGlu dHJvZHVjZWQgYnkgdGhlIG5ldyBkcm1fdXBkYXRlX3ZibGFua19jb3VudCgpCj4+Pj4+PiBpbXBs ZW1lbnRhdGlvbiBpbiBMaW51eCA0LjQ6Cj4+Pj4+Pgo+Pj4+Pj4gUmVzdHJpY3QgdGhlIGJ1bXAg b2YgdGhlIHNvZnR3YXJlIHZibGFuayBjb3VudGVyIGluIGRybV91cGRhdGVfdmJsYW5rX2NvdW50 KCkKPj4+Pj4+IHRvIGEgc2FmZSBtYXhpbXVtIHZhbHVlIG9mICsxIHdoZW5ldmVyIHRoZXJlIGlz IHRoZSBwb3NzaWJpbGl0eSB0aGF0Cj4+Pj4+PiBjb25jdXJyZW50IHJlYWRlcnMgb2YgdmJsYW5r IHRpbWVzdGFtcHMgY291bGQgYmUgYWN0aXZlIGF0IHRoZSBtb21lbnQsCj4+Pj4+PiBhcyB0aGUg Y3VycmVudCBpbXBsZW1lbnRhdGlvbiBvZiB0aGUgdGltZXN0YW1wIGNhY2hpbmcgYW5kIHVwZGF0 aW5nIGlzCj4+Pj4+PiBub3Qgc2FmZSBhZ2FpbnN0IGNvbmN1cnJlbnQgcmVhZGVycyBmb3IgY2Fs bHMgdG8gc3RvcmVfdmJsYW5rKCkgd2l0aCBhCj4+Pj4+PiBidW1wIG9mIGFueXRoaW5nIGJ1dCAr MS4gQSBidW1wICE9IDEgd291bGQgdmVyeSBsaWtlbHkgcmV0dXJuIGNvcnJ1cHRlZAo+Pj4+Pj4g dGltZXN0YW1wcyB0byB1c2Vyc3BhY2UsIGJlY2F1c2UgdGhlIHNhbWUgc2xvdCBpbiB0aGUgY2Fj aGUgY291bGQKPj4+Pj4+IGJlIGNvbmN1cnJlbnRseSB3cml0dGVuIGJ5IHN0b3JlX3ZibGFuaygp IGFuZCByZWFkIGJ5IG9uZSBvZiB0aG9zZQo+Pj4+Pj4gcmVhZGVycyBpbiBhIG5vbi1hdG9taWMg ZmFzaGlvbiBhbmQgd2l0aG91dCB0aGUgcmVhZC1yZXRyeSBsb2dpYwo+Pj4+Pj4gZGV0ZWN0aW5n IHRoaXMgY29sbGlzaW9uLgo+Pj4+Pj4KPj4+Pj4+IENvbmN1cnJlbnQgcmVhZGVycyBjYW4gZXhp c3Qgd2hpbGUgZHJtX3VwZGF0ZV92YmxhbmtfY291bnQoKSBpcyBjYWxsZWQKPj4+Pj4gPmZyb20g dGhlIGRybV92Ymxhbmtfb2ZmKCkgb3IgZHJtX3ZibGFua19vbigpIGZ1bmN0aW9ucyBvciBvdGhl ciBub24tdmJsYW5rLQo+Pj4+Pj4gaXJxIGNhbGxlcnMuIEhvd2V2ZXIsIGFsbCB0aG9zZSBjYWxs cyBhcmUgaGFwcGVuaW5nIHdpdGggdGhlIHZibF9sb2NrCj4+Pj4+PiBsb2NrZWQgdGhlcmVieSBw cmV2ZW50aW5nIGEgZHJtX3ZibGFua19nZXQoKSwgc28gdGhlIHZibGFuayByZWZjb3VudAo+Pj4+ Pj4gY2FuJ3QgaW5jcmVhc2Ugd2hpbGUgZHJtX3VwZGF0ZV92YmxhbmtfY291bnQoKSBpcyBleGVj dXRpbmcuIFRoZXJlZm9yZQo+Pj4+Pj4gYSB6ZXJvIHZibGFuayByZWZjb3VudCBkdXJpbmcgZXhl Y3V0aW9uIG9mIHRoYXQgZnVuY3Rpb24gc2lnbmFscyB0aGF0Cj4+Pj4+PiBpcyBzYWZlIGZvciBh cmJpdHJhcnkgY291bnRlciBidW1wcyBpZiBjYWxsZWQgZnJvbSBvdXRzaWRlIHZibGFuayBpcnEs Cj4+Pj4+PiB3aGVyZWFzIGEgbm9uLXplcm8gY291bnQgaXMgbm90IHNhZmUuCj4+Pj4+Pgo+Pj4+ Pj4gV2hlbmV2ZXIgdGhlIGZ1bmN0aW9uIGlzIGNhbGxlZCBmcm9tIHZibGFuayBpcnEsIHdlIGhh dmUgdG8gYXNzdW1lIGNvbmN1cnJlbnQKPj4+Pj4+IHJlYWRlcnMgY291bGQgc2hvdyB1cCBhbnkg dGltZSBkdXJpbmcgaXRzIGV4ZWN1dGlvbiwgZXZlbiBpZiB0aGUgcmVmY291bnQKPj4+Pj4+IGlz IGN1cnJlbnRseSB6ZXJvLCBhcyB2YmxhbmsgaXJxcyBhcmUgdXN1YWxseSBvbmx5IGVuYWJsZWQg ZHVlIHRvIHRoZQo+Pj4+Pj4gcHJlc2VuY2Ugb2YgcmVhZGVycywgYW5kIGJlY2F1c2Ugd2hlbiBp dCBpcyBjYWxsZWQgZnJvbSB2YmxhbmsgaXJxIGl0Cj4+Pj4+PiBjYW4ndCBob2xkIHRoZSB2Ymxf bG9jayB0byBwcm90ZWN0IGl0IGZyb20gc3VkZGVuIGJ1bXBzIGluIHZibGFuayByZWZjb3VudC4K Pj4+Pj4+IFRoZXJlZm9yZSBhbHNvIHJlc3RyaWN0IGJ1bXBzIHRvICsxIHdoZW4gdGhlIGZ1bmN0 aW9uIGlzIGNhbGxlZCBmcm9tIHZibGFuawo+Pj4+Pj4gaXJxLgo+Pj4+Pj4KPj4+Pj4+IFN1Y2gg YnVtcHMgb2YgbW9yZSB0aGFuICsxIGNhbiBoYXBwZW4gYXQgb3RoZXIgdGltZXMgdGhhbiByZWVu YWJsaW5nCj4+Pj4+PiB2YmxhbmsgaXJxcywgZS5nLiwgd2hlbiByZWd1bGFyIHZibGFuayBpbnRl cnJ1cHRzIGdldCBkZWxheWVkIGJ5IG1vcmUKPj4+Pj4+IHRoYW4gMSBmcmFtZSBkdWUgdG8gbG9u ZyBoZWxkIGxvY2tzLCBsb25nIGlycSBvZmYgcGVyaW9kcywgcmVhbHRpbWUKPj4+Pj4+IHByZWVt cHRpb24gb24gUlQga2VybmVscywgb3Igc3lzdGVtIG1hbmFnZW1lbnQgaW50ZXJydXB0cy4KPj4+ Pj4+Cj4+Pj4+PiBTaWduZWQtb2ZmLWJ5OiBNYXJpbyBLbGVpbmVyIDxtYXJpby5rbGVpbmVyLmRl QGdtYWlsLmNvbT4KPj4+Pj4+IENjOiA8c3RhYmxlQHZnZXIua2VybmVsLm9yZz4gIyA0LjQrCj4+ Pj4+PiBDYzogbWljaGVsQGRhZW56ZXIubmV0Cj4+Pj4+PiBDYzogdmJhYmthQHN1c2UuY3oKPj4+ Pj4+IENjOiB2aWxsZS5zeXJqYWxhQGxpbnV4LmludGVsLmNvbQo+Pj4+Pj4gQ2M6IGRhbmllbC52 ZXR0ZXJAZmZ3bGwuY2gKPj4+Pj4+IENjOiBkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0b3Aub3Jn Cj4+Pj4+PiBDYzogYWxleGFuZGVyLmRldWNoZXJAYW1kLmNvbQo+Pj4+Pj4gQ2M6IGNocmlzdGlh bi5rb2VuaWdAYW1kLmNvbQo+Pj4+Pgo+Pj4+PiBJbW8gdGhpcyBpcyBkdWN0LXRhcGUuIElmIHdl IHdhbnQgdG8gZml4IHRoaXMgdXAgcHJvcGVybHkgSSB0aGluayB3ZQo+Pj4+PiBzaG91bGQganVz dCB1c2UgYSBmdWxsLWJsb3duIHNlcWxvY2sgaW5zdGVhZCBvZiBvdXIgaGFuZC1yb2xsZWQgb25l LiBBbmQKPj4+Pj4gdGhhdCBjb3VsZCBoYW5kbGUgYW55IGluY3JlbWVudCBhdCBhbGwuCj4+Pj4K Pj4+PiBBbmQgSSBldmVuIGZpeGVkIHRoaXMgWzFdIGFsbW9zdCBhIGhhbGYgYSB5ZWFyIGFnbyB3 aGVuIEkgc2VudCB0aGUKPj4+PiBvcmlnaW5hbCBzZXJpZXMsIGJ1dCB0aGF0IHBhcnQgZ290IGhl bGQgaG9zdGFnZSB0byB0aGUgc2FtZSBzZXFsb2NrCj4+Pj4gYXJndW1lbnQuIFBlcmZlY3QgaXMg dGhlIGVuZW15IG9mIGdvb2QuCj4+Pj4KPj4+PiBbMV0gaHR0cHM6Ly9saXN0cy5mcmVlZGVza3Rv cC5vcmcvYXJjaGl2ZXMvaW50ZWwtZ2Z4LzIwMTUtU2VwdGVtYmVyLzA3NTg3OS5odG1sCj4+Pgo+ Pj4gSG0geWVhaCwgdGhhdCBkb2VzIHN1ZmZlciBmcm9tIHJlaW52ZW50aW5nIHNlcWxvY2tzLiBC dXQgSSdkIHByZWZlciB5b3VyCj4+PiBwYXRjaCBvdmVyIE1hcmlvJ3MgaGFjayBoZXJlIHRiaC4g WW91ciBwYXRjaCB3aXRoIHNlcWxvY2sgd291bGQgYmUgZXZlbgo+Pj4gbW9yZSBhd2Vzb21lLgo+ Pj4gLURhbmllbAo+Pj4KPj4KPj4gSSBhZ3JlZSB0aGF0IG15IGhhY2sgaXMgb25seSBkdWN0LXRh cGUuIFRoYXQncyB3aHkgdGhlIGxvbmcgY29kZSBjb21tZW50IHRvCj4+IGxldCBwZW9wbGUga25v dyB1bmRlciB3aGljaCBjb25kaXRpb24gdGhleSBjb3VsZCByZW1vdmUgaXQuCj4+Cj4+IFVzaW5n IHNlcWxvY2tzIHdvdWxkIGJlIHRoZSByb2J1c3QgbG9uZyB0ZXJtIHNvbHV0aW9uLiBCdXQgYXMg dGhpcyBpcwo+PiBzdXBwb3NlZCB0byBiZSBhIGZpeCBmb3IgYm90aCA0LjQgYW5kIDQuNSBpIHRo b3VnaHQgdGhhdCBzdWNoIGEgcmV3cml0ZQo+PiB3b3VsZCBiZSB0b28gaW50cnVzaXZlIGFzIGEg Y2hhbmdlLCBjb21wYXJlZCB0byB0aGlzIG9uZS1saW5lcj8KPj4KPj4gVGhlIG9yaWdpbmFsICJy b2xsIG91ciBvd24iIHNlcWxvY2sgbG9vayBhbGlrZSBpbXBsZW1lbnRhdGlvbiB3YXMgbWVhbnQg dG8KPj4gYXZvaWQvbWluaW1pemUgdGFraW5nIGxvY2tzLCBlc3AuIHdpdGggX2lycXNhdmUgdGhh dCBhcmUgdGFrZW4gYnkgYm90aAo+PiB1c2Vyc3BhY2UgYW5kIHRpbWluZyBzZW5zaXRpdmUgdmJs YW5rIGlycSBoYW5kbGluZyBjb2RlLiBUaGVyZSB3ZXJlIHZhcmlvdXMKPj4gbG9ja2luZyBjaGFu Z2VzIHNpbmNlIHRoZW4gYW5kIHRoYXQgYWR2YW50YWdlIG1pZ2h0IGhhdmUgYmVlbiBsb3N0IGFs cmVhZHkKPj4gcXVpdGUgYSBsb25nIHRpbWUgYWdvLCBzbyBtYXliZSBzd2l0Y2hpbmcgdG8gZnVs bCBzZXFsb2NrcyB3b3VsZG4ndCBwb3NlCj4+IHNvbWUgbmV3IHBlcmZvcm1hbmNlIHByb2JsZW1z IHRoZXJlLCBidXQgaSBoYXZlbid0IGxvb2tlZCBpbnRvIHRoaXMuCj4KPiBMYXN0IHRpbWUgSSd2 ZSBjaGVja2VkIHdlJ3ZlIGFscmVhZHkgcmVpbnZlbnRlZCBzZXFsb2NrcyBjb21wbGV0ZWx5LAo+ IGV4Y2VwdCBidWdneSBzaW5jZSBvdXJzIGNhbid0IHRha2UgYW4gaW5jcmVtZW50ID4gMS4gSSBk b24ndCBleHBlY3QgeW91J2xsCj4gYmUgYWJsZSB0byBtZWFzdXJlIGFueXRoaW5nIGlmIHdlIHN3 aXRjaC4KPgo+IEFncmVlIHRoYXQgaXQgbWlnaHQgYmUgYmV0dGVyIHRvIGRlbGF5IHRoaXMgZm9y IDQuNi4gU28gaWYgeW91IGFkZCBhIGJpZwo+ICJGSU1YRTogTmVlZCB0byByZXBsYWNlIHRoaXMg aGFjayB3aXRoIHByb3BlciBzZXFsb2Nrcy4iIGEgdGhlIHRvcCBvZiB5b3VyCj4gYmlnIGNvbW1l bnQgKG9yIGp1c3QgYXMgYSByZXBsYWNlbWVudCBmb3IgaXQpLCB0aGVuCj4KPiBSZXZpZXdlZC1i eTogRGFuaWVsIFZldHRlciA8ZGFuaWVsLnZldHRlckBmZndsbC5jaD4KPgo+IEJ1dCBjdXJyZW50 bHkgaXQgbG9va3MgbGlrZSB0aGlzIGlzIGEgcHJvcGVyIGxvbmctdGVybSBzb2x1dGlvbiwgd2hp Y2ggaXQKPiBpbW8gaXNuJ3QuCj4gLURhbmllbAo+CgpPaywgd2lsbCBkby4KLW1hcmlvCgo+Cj4+ Cj4+IC1tYXJpbwo+Pgo+Pj4+Cj4+Pj4+IC1EYW5pZWwKPj4+Pj4KPj4+Pj4+IC0tLQo+Pj4+Pj4g ICBkcml2ZXJzL2dwdS9kcm0vZHJtX2lycS5jIHwgNDEgKysrKysrKysrKysrKysrKysrKysrKysr KysrKysrKysrKysrKysrKysKPj4+Pj4+ICAgMSBmaWxlIGNoYW5nZWQsIDQxIGluc2VydGlvbnMo KykKPj4+Pj4+Cj4+Pj4+PiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2RybV9pcnEuYyBi L2RyaXZlcnMvZ3B1L2RybS9kcm1faXJxLmMKPj4+Pj4+IGluZGV4IGJjYjg1MjguLmFhMmM3NGIg MTAwNjQ0Cj4+Pj4+PiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vZHJtX2lycS5jCj4+Pj4+PiArKysg Yi9kcml2ZXJzL2dwdS9kcm0vZHJtX2lycS5jCj4+Pj4+PiBAQCAtMjIxLDYgKzIyMSw0NyBAQCBz dGF0aWMgdm9pZCBkcm1fdXBkYXRlX3ZibGFua19jb3VudChzdHJ1Y3QgZHJtX2RldmljZSAqZGV2 LCB1bnNpZ25lZCBpbnQgcGlwZSwKPj4+Pj4+ICAgCQlkaWZmID0gKGZsYWdzICYgRFJNX0NBTExF RF9GUk9NX1ZCTElSUSkgIT0gMDsKPj4+Pj4+ICAgCX0KPj4+Pj4+Cj4+Pj4+PiArCS8qCj4+Pj4+ PiArCSAqIFJlc3RyaWN0IHRoZSBidW1wIG9mIHRoZSBzb2Z0d2FyZSB2YmxhbmsgY291bnRlciB0 byBhIHNhZmUgbWF4aW11bQo+Pj4+Pj4gKwkgKiB2YWx1ZSBvZiArMSB3aGVuZXZlciB0aGVyZSBp cyB0aGUgcG9zc2liaWxpdHkgdGhhdCBjb25jdXJyZW50IHJlYWRlcnMKPj4+Pj4+ICsJICogb2Yg dmJsYW5rIHRpbWVzdGFtcHMgY291bGQgYmUgYWN0aXZlIGF0IHRoZSBtb21lbnQsIGFzIHRoZSBj dXJyZW50Cj4+Pj4+PiArCSAqIGltcGxlbWVudGF0aW9uIG9mIHRoZSB0aW1lc3RhbXAgY2FjaGlu ZyBhbmQgdXBkYXRpbmcgaXMgbm90IHNhZmUKPj4+Pj4+ICsJICogYWdhaW5zdCBjb25jdXJyZW50 IHJlYWRlcnMgZm9yIGNhbGxzIHRvIHN0b3JlX3ZibGFuaygpIHdpdGggYSBidW1wCj4+Pj4+PiAr CSAqIG9mIGFueXRoaW5nIGJ1dCArMS4gQSBidW1wICE9IDEgd291bGQgdmVyeSBsaWtlbHkgcmV0 dXJuIGNvcnJ1cHRlZAo+Pj4+Pj4gKwkgKiB0aW1lc3RhbXBzIHRvIHVzZXJzcGFjZSwgYmVjYXVz ZSB0aGUgc2FtZSBzbG90IGluIHRoZSBjYWNoZSBjb3VsZAo+Pj4+Pj4gKwkgKiBiZSBjb25jdXJy ZW50bHkgd3JpdHRlbiBieSBzdG9yZV92YmxhbmsoKSBhbmQgcmVhZCBieSBvbmUgb2YgdGhvc2UK Pj4+Pj4+ICsJICogcmVhZGVycyB3aXRob3V0IHRoZSByZWFkLXJldHJ5IGxvZ2ljIGRldGVjdGlu ZyB0aGUgY29sbGlzaW9uLgo+Pj4+Pj4gKwkgKgo+Pj4+Pj4gKwkgKiBDb25jdXJyZW50IHJlYWRl cnMgY2FuIGV4aXN0IHdoZW4gd2UgYXJlIGNhbGxlZCBmcm9tIHRoZQo+Pj4+Pj4gKwkgKiBkcm1f dmJsYW5rX29mZigpIG9yIGRybV92Ymxhbmtfb24oKSBmdW5jdGlvbnMgYW5kIG90aGVyIG5vbi12 YmxhbmstCj4+Pj4+PiArCSAqIGlycSBjYWxsZXJzLiBIb3dldmVyLCBhbGwgdGhvc2UgY2FsbHMg dG8gdXMgYXJlIGhhcHBlbmluZyB3aXRoIHRoZQo+Pj4+Pj4gKwkgKiB2YmxfbG9jayBsb2NrZWQg dG8gcHJldmVudCBkcm1fdmJsYW5rX2dldCgpLCBzbyB0aGUgdmJsYW5rIHJlZmNvdW50Cj4+Pj4+ PiArCSAqIGNhbid0IGluY3JlYXNlIHdoaWxlIHdlIGFyZSBleGVjdXRpbmcuIFRoZXJlZm9yZSBh IHplcm8gcmVmY291bnQgYXQKPj4+Pj4+ICsJICogdGhpcyBwb2ludCBpcyBzYWZlIGZvciBhcmJp dHJhcnkgY291bnRlciBidW1wcyBpZiB3ZSBhcmUgY2FsbGVkCj4+Pj4+PiArCSAqIG91dHNpZGUg dmJsYW5rIGlycSwgYSBub24temVybyBjb3VudCBpcyBub3QgMTAwJSBzYWZlLiBVbmZvcnR1bmF0 ZWx5Cj4+Pj4+PiArCSAqIHdlIG11c3QgYWxzbyBhY2NlcHQgYSByZWZjb3VudCBvZiAxLCBhcyB3 aGVuZXZlciB3ZSBhcmUgY2FsbGVkIGZyb20KPj4+Pj4+ICsJICogZHJtX3ZibGFua19nZXQoKSAt PiBkcm1fdmJsYW5rX2VuYWJsZSgpIHRoZSByZWZjb3VudCB3aWxsIGJlIDEgYW5kCj4+Pj4+PiAr CSAqIHdlIG11c3QgbGV0IHRoYXQgb25lIHBhc3MgdGhyb3VnaCBpbiBvcmRlciB0byBub3QgbG9z ZSB2YmxhbmsgY291bnRzCj4+Pj4+PiArCSAqIGR1cmluZyB2YmxhbmsgaXJxIG9mZiAtIHdoaWNo IHdvdWxkIGNvbXBsZXRlbHkgZGVmZWF0IHRoZSB3aG9sZQo+Pj4+Pj4gKwkgKiBwb2ludCBvZiB0 aGlzIHJvdXRpbmUuCj4+Pj4+PiArCSAqCj4+Pj4+PiArCSAqIFdoZW5ldmVyIHdlIGFyZSBjYWxs ZWQgZnJvbSB2YmxhbmsgaXJxLCB3ZSBoYXZlIHRvIGFzc3VtZSBjb25jdXJyZW50Cj4+Pj4+PiAr CSAqIHJlYWRlcnMgZXhpc3Qgb3IgY2FuIHNob3cgdXAgYW55IHRpbWUgZHVyaW5nIG91ciBleGVj dXRpb24sIGV2ZW4gaWYKPj4+Pj4+ICsJICogdGhlIHJlZmNvdW50IGlzIGN1cnJlbnRseSB6ZXJv LCBhcyB2YmxhbmsgaXJxcyBhcmUgdXN1YWxseSBvbmx5Cj4+Pj4+PiArCSAqIGVuYWJsZWQgZHVl IHRvIHRoZSBwcmVzZW5jZSBvZiByZWFkZXJzLCBhbmQgYmVjYXVzZSB3aGVuIHdlIGFyZSBjYWxs ZWQKPj4+Pj4+ICsJICogZnJvbSB2YmxhbmsgaXJxIHdlIGNhbid0IGhvbGQgdGhlIHZibF9sb2Nr IHRvIHByb3RlY3QgdXMgZnJvbSBzdWRkZW4KPj4+Pj4+ICsJICogYnVtcHMgaW4gdmJsYW5rIHJl ZmNvdW50LiBUaGVyZWZvcmUgYWxzbyByZXN0cmljdCBidW1wcyB0byArMSB3aGVuCj4+Pj4+PiAr CSAqIGNhbGxlZCBmcm9tIHZibGFuayBpcnEuCj4+Pj4+PiArCSAqLwo+Pj4+Pj4gKwlpZiAoKGRp ZmYgPiAxKSAmJiAoYXRvbWljX3JlYWQoJnZibGFuay0+cmVmY291bnQpID4gMSB8fAo+Pj4+Pj4g KwkgICAgKGZsYWdzICYgRFJNX0NBTExFRF9GUk9NX1ZCTElSUSkpKSB7Cj4+Pj4+PiArCQlEUk1f REVCVUdfVkJMKCJjbGFtcGluZyB2YmxhbmsgYnVtcCB0byAxIG9uIGNydGMgJXU6IGRpZmZyPSV1 ICIKPj4+Pj4+ICsJCQkgICAgICAicmVmY291bnQgJXUsIHZibGlycSAldVxuIiwgcGlwZSwgZGlm ZiwKPj4+Pj4+ICsJCQkgICAgICBhdG9taWNfcmVhZCgmdmJsYW5rLT5yZWZjb3VudCksCj4+Pj4+ PiArCQkJICAgICAgKGZsYWdzICYgRFJNX0NBTExFRF9GUk9NX1ZCTElSUSkgIT0gMCk7Cj4+Pj4+ PiArCQlkaWZmID0gMTsKPj4+Pj4+ICsJfQo+Pj4+Pj4gKwo+Pj4+Pj4gICAJRFJNX0RFQlVHX1ZC TCgidXBkYXRpbmcgdmJsYW5rIGNvdW50IG9uIGNydGMgJXU6Igo+Pj4+Pj4gICAJCSAgICAgICIg Y3VycmVudD0ldSwgZGlmZj0ldSwgaHc9JXUgaHdfbGFzdD0ldVxuIiwKPj4+Pj4+ICAgCQkgICAg ICBwaXBlLCB2YmxhbmstPmNvdW50LCBkaWZmLCBjdXJfdmJsYW5rLCB2YmxhbmstPmxhc3QpOwo+ Pj4+Pj4gLS0KPj4+Pj4+IDEuOS4xCj4+Pj4+Pgo+Pj4+Pgo+Pj4+PiAtLQo+Pj4+PiBEYW5pZWwg VmV0dGVyCj4+Pj4+IFNvZnR3YXJlIEVuZ2luZWVyLCBJbnRlbCBDb3Jwb3JhdGlvbgo+Pj4+PiBo dHRwOi8vYmxvZy5mZndsbC5jaAo+Pj4+Cj4+Pj4gLS0KPj4+PiBWaWxsZSBTeXJqw6Rsw6QKPj4+ PiBJbnRlbCBPVEMKPj4+Cj4KX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxAbGlzdHMuZnJlZWRlc2t0 b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4vbGlzdGluZm8vZHJp LWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f54.google.com ([74.125.82.54]:34907 "EHLO mail-wm0-f54.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S964804AbcBIQS6 (ORCPT ); Tue, 9 Feb 2016 11:18:58 -0500 Received: by mail-wm0-f54.google.com with SMTP id c200so67673753wme.0 for ; Tue, 09 Feb 2016 08:18:57 -0800 (PST) Subject: Re: [PATCH 2/6] drm: Prevent vblank counter bumps > 1 with active vblank clients. To: Daniel Vetter References: <1454894009-15466-1-git-send-email-mario.kleiner.de@gmail.com> <1454894009-15466-3-git-send-email-mario.kleiner.de@gmail.com> <20160209095638.GM11240@phenom.ffwll.local> <20160209100727.GG23290@intel.com> <20160209102302.GT11240@phenom.ffwll.local> <56B9EC20.9050400@gmail.com> <20160209142957.GX11240@phenom.ffwll.local> Cc: =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , 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: <56BA116E.4010406@gmail.com> Date: Tue, 9 Feb 2016 17:18:54 +0100 MIME-Version: 1.0 In-Reply-To: <20160209142957.GX11240@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 03:29 PM, Daniel Vetter wrote: > On Tue, Feb 09, 2016 at 02:39:44PM +0100, Mario Kleiner wrote: >> On 02/09/2016 11:23 AM, Daniel Vetter wrote: >>> On Tue, Feb 09, 2016 at 12:07:27PM +0200, Ville Syrj�l� wrote: >>>> On Tue, Feb 09, 2016 at 10:56:38AM +0100, Daniel Vetter wrote: >>>>> On Mon, Feb 08, 2016 at 02:13:25AM +0100, Mario Kleiner wrote: >>>>>> This fixes a regression introduced by the new drm_update_vblank_count() >>>>>> implementation in Linux 4.4: >>>>>> >>>>>> Restrict the bump of the software vblank counter in drm_update_vblank_count() >>>>>> 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 implementation of the timestamp caching and updating is >>>>>> not safe against concurrent readers for calls to store_vblank() with a >>>>>> bump of anything but +1. A bump != 1 would very likely return corrupted >>>>>> timestamps to userspace, because the same slot in the cache could >>>>>> be concurrently written by store_vblank() and read by one of those >>>>>> readers in a non-atomic fashion and without the read-retry logic >>>>>> detecting this collision. >>>>>> >>>>>> Concurrent readers can exist while drm_update_vblank_count() is called >>>>> >from the drm_vblank_off() or drm_vblank_on() functions or other non-vblank- >>>>>> irq callers. However, all those calls are happening with the vbl_lock >>>>>> locked thereby preventing a drm_vblank_get(), so the vblank refcount >>>>>> can't increase while drm_update_vblank_count() is executing. Therefore >>>>>> a zero vblank refcount during execution of that function signals that >>>>>> is safe for arbitrary counter bumps if called from outside vblank irq, >>>>>> whereas a non-zero count is not safe. >>>>>> >>>>>> Whenever the function is called from vblank irq, we have to assume concurrent >>>>>> readers could show up any time during its execution, even if the refcount >>>>>> is currently zero, as vblank irqs are usually only enabled due to the >>>>>> presence of readers, and because when it is called from vblank irq it >>>>>> can't hold the vbl_lock to protect it from sudden bumps in vblank refcount. >>>>>> Therefore also restrict bumps to +1 when the function is called from vblank >>>>>> irq. >>>>>> >>>>>> Such bumps of more than +1 can happen at other times than reenabling >>>>>> vblank irqs, e.g., when regular vblank interrupts get delayed by more >>>>>> than 1 frame due to long held locks, long irq off periods, realtime >>>>>> preemption on RT kernels, or system management interrupts. >>>>>> >>>>>> 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 >>>>> >>>>> Imo this is duct-tape. If we want to fix this up properly I think we >>>>> should just use a full-blown seqlock instead of our hand-rolled one. And >>>>> that could handle any increment at all. >>>> >>>> And I even fixed this [1] almost a half a year ago when I sent the >>>> original series, but that part got held hostage to the same seqlock >>>> argument. Perfect is the enemy of good. >>>> >>>> [1] https://lists.freedesktop.org/archives/intel-gfx/2015-September/075879.html >>> >>> Hm yeah, that does suffer from reinventing seqlocks. But I'd prefer your >>> patch over Mario's hack here tbh. Your patch with seqlock would be even >>> more awesome. >>> -Daniel >>> >> >> I agree that my hack is only duct-tape. That's why the long code comment to >> let people know under which condition they could remove it. >> >> Using seqlocks would be the robust long term solution. But as this is >> supposed to be a fix for both 4.4 and 4.5 i thought that such a rewrite >> would be too intrusive as a change, compared to this one-liner? >> >> The original "roll our own" seqlock look alike implementation was meant to >> avoid/minimize taking locks, esp. with _irqsave that are taken by both >> userspace and timing sensitive vblank irq handling code. There were various >> locking changes since then and that advantage might have been lost already >> quite a long time ago, so maybe switching to full seqlocks wouldn't pose >> some new performance problems there, but i haven't looked into this. > > Last time I've checked we've already reinvented seqlocks completely, > except buggy since ours can't take an increment > 1. I don't expect you'll > be able to measure anything if we switch. > > Agree that it might be better to delay this for 4.6. So if you add a big > "FIMXE: Need to replace this hack with proper seqlocks." a the top of your > big comment (or just as a replacement for it), then > > Reviewed-by: Daniel Vetter > > But currently it looks like this is a proper long-term solution, which it > imo isn't. > -Daniel > Ok, will do. -mario > >> >> -mario >> >>>> >>>>> -Daniel >>>>> >>>>>> --- >>>>>> drivers/gpu/drm/drm_irq.c | 41 +++++++++++++++++++++++++++++++++++++++++ >>>>>> 1 file changed, 41 insertions(+) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c >>>>>> index bcb8528..aa2c74b 100644 >>>>>> --- a/drivers/gpu/drm/drm_irq.c >>>>>> +++ b/drivers/gpu/drm/drm_irq.c >>>>>> @@ -221,6 +221,47 @@ static void drm_update_vblank_count(struct drm_device *dev, unsigned int pipe, >>>>>> diff = (flags & DRM_CALLED_FROM_VBLIRQ) != 0; >>>>>> } >>>>>> >>>>>> + /* >>>>>> + * 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 >>>>>> + * implementation of the timestamp caching and updating is not safe >>>>>> + * against concurrent readers for calls to store_vblank() with a bump >>>>>> + * of anything but +1. A bump != 1 would very likely return corrupted >>>>>> + * timestamps to userspace, because the same slot in the cache could >>>>>> + * be concurrently written by store_vblank() and read by one of those >>>>>> + * readers without the read-retry logic detecting the collision. >>>>>> + * >>>>>> + * Concurrent readers can exist when we are called from the >>>>>> + * drm_vblank_off() or drm_vblank_on() functions and other non-vblank- >>>>>> + * irq callers. However, all those calls to us are happening with the >>>>>> + * vbl_lock locked to prevent drm_vblank_get(), so the vblank refcount >>>>>> + * can't increase while we are executing. Therefore a zero refcount at >>>>>> + * this point is safe for arbitrary counter bumps if we are called >>>>>> + * outside vblank irq, a non-zero count is not 100% safe. Unfortunately >>>>>> + * we must also accept a refcount of 1, as whenever we are called from >>>>>> + * drm_vblank_get() -> drm_vblank_enable() the refcount will be 1 and >>>>>> + * we must let that one pass through in order to not lose vblank counts >>>>>> + * during vblank irq off - which would completely defeat the whole >>>>>> + * point of this routine. >>>>>> + * >>>>>> + * Whenever we are called from vblank irq, we have to assume concurrent >>>>>> + * readers exist or can show up any time during our execution, even if >>>>>> + * the refcount is currently zero, as vblank irqs are usually only >>>>>> + * enabled due to the presence of readers, and because when we are called >>>>>> + * from vblank irq we can't hold the vbl_lock to protect us from sudden >>>>>> + * bumps in vblank refcount. Therefore also restrict bumps to +1 when >>>>>> + * called from vblank irq. >>>>>> + */ >>>>>> + if ((diff > 1) && (atomic_read(&vblank->refcount) > 1 || >>>>>> + (flags & DRM_CALLED_FROM_VBLIRQ))) { >>>>>> + DRM_DEBUG_VBL("clamping vblank bump to 1 on crtc %u: diffr=%u " >>>>>> + "refcount %u, vblirq %u\n", pipe, diff, >>>>>> + atomic_read(&vblank->refcount), >>>>>> + (flags & DRM_CALLED_FROM_VBLIRQ) != 0); >>>>>> + diff = 1; >>>>>> + } >>>>>> + >>>>>> DRM_DEBUG_VBL("updating vblank count on crtc %u:" >>>>>> " current=%u, diff=%u, hw=%u hw_last=%u\n", >>>>>> pipe, vblank->count, diff, cur_vblank, vblank->last); >>>>>> -- >>>>>> 1.9.1 >>>>>> >>>>> >>>>> -- >>>>> Daniel Vetter >>>>> Software Engineer, Intel Corporation >>>>> http://blog.ffwll.ch >>>> >>>> -- >>>> Ville Syrj�l� >>>> Intel OTC >>> >