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 14:39:44 +0100 Message-ID: <56B9EC20.9050400@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> 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 2654B6E57A for ; Tue, 9 Feb 2016 05:39:49 -0800 (PST) Received: by mail-wm0-f66.google.com with SMTP id g62so3495447wme.2 for ; Tue, 09 Feb 2016 05:39:49 -0800 (PST) In-Reply-To: <20160209102302.GT11240@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 T24gMDIvMDkvMjAxNiAxMToyMyBBTSwgRGFuaWVsIFZldHRlciB3cm90ZToKPiBPbiBUdWUsIEZl YiAwOSwgMjAxNiBhdCAxMjowNzoyN1BNICswMjAwLCBWaWxsZSBTeXJqw6Rsw6Qgd3JvdGU6Cj4+ IE9uIFR1ZSwgRmViIDA5LCAyMDE2IGF0IDEwOjU2OjM4QU0gKzAxMDAsIERhbmllbCBWZXR0ZXIg d3JvdGU6Cj4+PiBPbiBNb24sIEZlYiAwOCwgMjAxNiBhdCAwMjoxMzoyNUFNICswMTAwLCBNYXJp byBLbGVpbmVyIHdyb3RlOgo+Pj4+IFRoaXMgZml4ZXMgYSByZWdyZXNzaW9uIGludHJvZHVjZWQg YnkgdGhlIG5ldyBkcm1fdXBkYXRlX3ZibGFua19jb3VudCgpCj4+Pj4gaW1wbGVtZW50YXRpb24g aW4gTGludXggNC40Ogo+Pj4+Cj4+Pj4gUmVzdHJpY3QgdGhlIGJ1bXAgb2YgdGhlIHNvZnR3YXJl IHZibGFuayBjb3VudGVyIGluIGRybV91cGRhdGVfdmJsYW5rX2NvdW50KCkKPj4+PiB0byBhIHNh ZmUgbWF4aW11bSB2YWx1ZSBvZiArMSB3aGVuZXZlciB0aGVyZSBpcyB0aGUgcG9zc2liaWxpdHkg dGhhdAo+Pj4+IGNvbmN1cnJlbnQgcmVhZGVycyBvZiB2YmxhbmsgdGltZXN0YW1wcyBjb3VsZCBi ZSBhY3RpdmUgYXQgdGhlIG1vbWVudCwKPj4+PiBhcyB0aGUgY3VycmVudCBpbXBsZW1lbnRhdGlv biBvZiB0aGUgdGltZXN0YW1wIGNhY2hpbmcgYW5kIHVwZGF0aW5nIGlzCj4+Pj4gbm90IHNhZmUg YWdhaW5zdCBjb25jdXJyZW50IHJlYWRlcnMgZm9yIGNhbGxzIHRvIHN0b3JlX3ZibGFuaygpIHdp dGggYQo+Pj4+IGJ1bXAgb2YgYW55dGhpbmcgYnV0ICsxLiBBIGJ1bXAgIT0gMSB3b3VsZCB2ZXJ5 IGxpa2VseSByZXR1cm4gY29ycnVwdGVkCj4+Pj4gdGltZXN0YW1wcyB0byB1c2Vyc3BhY2UsIGJl Y2F1c2UgdGhlIHNhbWUgc2xvdCBpbiB0aGUgY2FjaGUgY291bGQKPj4+PiBiZSBjb25jdXJyZW50 bHkgd3JpdHRlbiBieSBzdG9yZV92YmxhbmsoKSBhbmQgcmVhZCBieSBvbmUgb2YgdGhvc2UKPj4+ PiByZWFkZXJzIGluIGEgbm9uLWF0b21pYyBmYXNoaW9uIGFuZCB3aXRob3V0IHRoZSByZWFkLXJl dHJ5IGxvZ2ljCj4+Pj4gZGV0ZWN0aW5nIHRoaXMgY29sbGlzaW9uLgo+Pj4+Cj4+Pj4gQ29uY3Vy cmVudCByZWFkZXJzIGNhbiBleGlzdCB3aGlsZSBkcm1fdXBkYXRlX3ZibGFua19jb3VudCgpIGlz IGNhbGxlZAo+Pj4+IGZyb20gdGhlIGRybV92Ymxhbmtfb2ZmKCkgb3IgZHJtX3ZibGFua19vbigp IGZ1bmN0aW9ucyBvciBvdGhlciBub24tdmJsYW5rLQo+Pj4+IGlycSBjYWxsZXJzLiBIb3dldmVy LCBhbGwgdGhvc2UgY2FsbHMgYXJlIGhhcHBlbmluZyB3aXRoIHRoZSB2YmxfbG9jawo+Pj4+IGxv Y2tlZCB0aGVyZWJ5IHByZXZlbnRpbmcgYSBkcm1fdmJsYW5rX2dldCgpLCBzbyB0aGUgdmJsYW5r IHJlZmNvdW50Cj4+Pj4gY2FuJ3QgaW5jcmVhc2Ugd2hpbGUgZHJtX3VwZGF0ZV92YmxhbmtfY291 bnQoKSBpcyBleGVjdXRpbmcuIFRoZXJlZm9yZQo+Pj4+IGEgemVybyB2YmxhbmsgcmVmY291bnQg ZHVyaW5nIGV4ZWN1dGlvbiBvZiB0aGF0IGZ1bmN0aW9uIHNpZ25hbHMgdGhhdAo+Pj4+IGlzIHNh ZmUgZm9yIGFyYml0cmFyeSBjb3VudGVyIGJ1bXBzIGlmIGNhbGxlZCBmcm9tIG91dHNpZGUgdmJs YW5rIGlycSwKPj4+PiB3aGVyZWFzIGEgbm9uLXplcm8gY291bnQgaXMgbm90IHNhZmUuCj4+Pj4K Pj4+PiBXaGVuZXZlciB0aGUgZnVuY3Rpb24gaXMgY2FsbGVkIGZyb20gdmJsYW5rIGlycSwgd2Ug aGF2ZSB0byBhc3N1bWUgY29uY3VycmVudAo+Pj4+IHJlYWRlcnMgY291bGQgc2hvdyB1cCBhbnkg dGltZSBkdXJpbmcgaXRzIGV4ZWN1dGlvbiwgZXZlbiBpZiB0aGUgcmVmY291bnQKPj4+PiBpcyBj dXJyZW50bHkgemVybywgYXMgdmJsYW5rIGlycXMgYXJlIHVzdWFsbHkgb25seSBlbmFibGVkIGR1 ZSB0byB0aGUKPj4+PiBwcmVzZW5jZSBvZiByZWFkZXJzLCBhbmQgYmVjYXVzZSB3aGVuIGl0IGlz IGNhbGxlZCBmcm9tIHZibGFuayBpcnEgaXQKPj4+PiBjYW4ndCBob2xkIHRoZSB2YmxfbG9jayB0 byBwcm90ZWN0IGl0IGZyb20gc3VkZGVuIGJ1bXBzIGluIHZibGFuayByZWZjb3VudC4KPj4+PiBU aGVyZWZvcmUgYWxzbyByZXN0cmljdCBidW1wcyB0byArMSB3aGVuIHRoZSBmdW5jdGlvbiBpcyBj YWxsZWQgZnJvbSB2YmxhbmsKPj4+PiBpcnEuCj4+Pj4KPj4+PiBTdWNoIGJ1bXBzIG9mIG1vcmUg dGhhbiArMSBjYW4gaGFwcGVuIGF0IG90aGVyIHRpbWVzIHRoYW4gcmVlbmFibGluZwo+Pj4+IHZi bGFuayBpcnFzLCBlLmcuLCB3aGVuIHJlZ3VsYXIgdmJsYW5rIGludGVycnVwdHMgZ2V0IGRlbGF5 ZWQgYnkgbW9yZQo+Pj4+IHRoYW4gMSBmcmFtZSBkdWUgdG8gbG9uZyBoZWxkIGxvY2tzLCBsb25n IGlycSBvZmYgcGVyaW9kcywgcmVhbHRpbWUKPj4+PiBwcmVlbXB0aW9uIG9uIFJUIGtlcm5lbHMs IG9yIHN5c3RlbSBtYW5hZ2VtZW50IGludGVycnVwdHMuCj4+Pj4KPj4+PiBTaWduZWQtb2ZmLWJ5 OiBNYXJpbyBLbGVpbmVyIDxtYXJpby5rbGVpbmVyLmRlQGdtYWlsLmNvbT4KPj4+PiBDYzogPHN0 YWJsZUB2Z2VyLmtlcm5lbC5vcmc+ICMgNC40Kwo+Pj4+IENjOiBtaWNoZWxAZGFlbnplci5uZXQK Pj4+PiBDYzogdmJhYmthQHN1c2UuY3oKPj4+PiBDYzogdmlsbGUuc3lyamFsYUBsaW51eC5pbnRl bC5jb20KPj4+PiBDYzogZGFuaWVsLnZldHRlckBmZndsbC5jaAo+Pj4+IENjOiBkcmktZGV2ZWxA bGlzdHMuZnJlZWRlc2t0b3Aub3JnCj4+Pj4gQ2M6IGFsZXhhbmRlci5kZXVjaGVyQGFtZC5jb20K Pj4+PiBDYzogY2hyaXN0aWFuLmtvZW5pZ0BhbWQuY29tCj4+Pgo+Pj4gSW1vIHRoaXMgaXMgZHVj dC10YXBlLiBJZiB3ZSB3YW50IHRvIGZpeCB0aGlzIHVwIHByb3Blcmx5IEkgdGhpbmsgd2UKPj4+ IHNob3VsZCBqdXN0IHVzZSBhIGZ1bGwtYmxvd24gc2VxbG9jayBpbnN0ZWFkIG9mIG91ciBoYW5k LXJvbGxlZCBvbmUuIEFuZAo+Pj4gdGhhdCBjb3VsZCBoYW5kbGUgYW55IGluY3JlbWVudCBhdCBh bGwuCj4+Cj4+IEFuZCBJIGV2ZW4gZml4ZWQgdGhpcyBbMV0gYWxtb3N0IGEgaGFsZiBhIHllYXIg YWdvIHdoZW4gSSBzZW50IHRoZQo+PiBvcmlnaW5hbCBzZXJpZXMsIGJ1dCB0aGF0IHBhcnQgZ290 IGhlbGQgaG9zdGFnZSB0byB0aGUgc2FtZSBzZXFsb2NrCj4+IGFyZ3VtZW50LiBQZXJmZWN0IGlz IHRoZSBlbmVteSBvZiBnb29kLgo+Pgo+PiBbMV0gaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5v cmcvYXJjaGl2ZXMvaW50ZWwtZ2Z4LzIwMTUtU2VwdGVtYmVyLzA3NTg3OS5odG1sCj4KPiBIbSB5 ZWFoLCB0aGF0IGRvZXMgc3VmZmVyIGZyb20gcmVpbnZlbnRpbmcgc2VxbG9ja3MuIEJ1dCBJJ2Qg cHJlZmVyIHlvdXIKPiBwYXRjaCBvdmVyIE1hcmlvJ3MgaGFjayBoZXJlIHRiaC4gWW91ciBwYXRj aCB3aXRoIHNlcWxvY2sgd291bGQgYmUgZXZlbgo+IG1vcmUgYXdlc29tZS4KPiAtRGFuaWVsCj4K CkkgYWdyZWUgdGhhdCBteSBoYWNrIGlzIG9ubHkgZHVjdC10YXBlLiBUaGF0J3Mgd2h5IHRoZSBs b25nIGNvZGUgY29tbWVudCAKdG8gbGV0IHBlb3BsZSBrbm93IHVuZGVyIHdoaWNoIGNvbmRpdGlv biB0aGV5IGNvdWxkIHJlbW92ZSBpdC4KClVzaW5nIHNlcWxvY2tzIHdvdWxkIGJlIHRoZSByb2J1 c3QgbG9uZyB0ZXJtIHNvbHV0aW9uLiBCdXQgYXMgdGhpcyBpcyAKc3VwcG9zZWQgdG8gYmUgYSBm aXggZm9yIGJvdGggNC40IGFuZCA0LjUgaSB0aG91Z2h0IHRoYXQgc3VjaCBhIHJld3JpdGUgCndv dWxkIGJlIHRvbyBpbnRydXNpdmUgYXMgYSBjaGFuZ2UsIGNvbXBhcmVkIHRvIHRoaXMgb25lLWxp bmVyPwoKVGhlIG9yaWdpbmFsICJyb2xsIG91ciBvd24iIHNlcWxvY2sgbG9vayBhbGlrZSBpbXBs ZW1lbnRhdGlvbiB3YXMgbWVhbnQgCnRvIGF2b2lkL21pbmltaXplIHRha2luZyBsb2NrcywgZXNw LiB3aXRoIF9pcnFzYXZlIHRoYXQgYXJlIHRha2VuIGJ5IApib3RoIHVzZXJzcGFjZSBhbmQgdGlt aW5nIHNlbnNpdGl2ZSB2YmxhbmsgaXJxIGhhbmRsaW5nIGNvZGUuIFRoZXJlIHdlcmUgCnZhcmlv dXMgbG9ja2luZyBjaGFuZ2VzIHNpbmNlIHRoZW4gYW5kIHRoYXQgYWR2YW50YWdlIG1pZ2h0IGhh dmUgYmVlbiAKbG9zdCBhbHJlYWR5IHF1aXRlIGEgbG9uZyB0aW1lIGFnbywgc28gbWF5YmUgc3dp dGNoaW5nIHRvIGZ1bGwgc2VxbG9ja3MgCndvdWxkbid0IHBvc2Ugc29tZSBuZXcgcGVyZm9ybWFu Y2UgcHJvYmxlbXMgdGhlcmUsIGJ1dCBpIGhhdmVuJ3QgbG9va2VkIAppbnRvIHRoaXMuCgotbWFy aW8KCj4+Cj4+PiAtRGFuaWVsCj4+Pgo+Pj4+IC0tLQo+Pj4+ICAgZHJpdmVycy9ncHUvZHJtL2Ry bV9pcnEuYyB8IDQxICsrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrCj4+ Pj4gICAxIGZpbGUgY2hhbmdlZCwgNDEgaW5zZXJ0aW9ucygrKQo+Pj4+Cj4+Pj4gZGlmZiAtLWdp dCBhL2RyaXZlcnMvZ3B1L2RybS9kcm1faXJxLmMgYi9kcml2ZXJzL2dwdS9kcm0vZHJtX2lycS5j Cj4+Pj4gaW5kZXggYmNiODUyOC4uYWEyYzc0YiAxMDA2NDQKPj4+PiAtLS0gYS9kcml2ZXJzL2dw dS9kcm0vZHJtX2lycS5jCj4+Pj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2RybV9pcnEuYwo+Pj4+ IEBAIC0yMjEsNiArMjIxLDQ3IEBAIHN0YXRpYyB2b2lkIGRybV91cGRhdGVfdmJsYW5rX2NvdW50 KHN0cnVjdCBkcm1fZGV2aWNlICpkZXYsIHVuc2lnbmVkIGludCBwaXBlLAo+Pj4+ICAgCQlkaWZm ID0gKGZsYWdzICYgRFJNX0NBTExFRF9GUk9NX1ZCTElSUSkgIT0gMDsKPj4+PiAgIAl9Cj4+Pj4K Pj4+PiArCS8qCj4+Pj4gKwkgKiBSZXN0cmljdCB0aGUgYnVtcCBvZiB0aGUgc29mdHdhcmUgdmJs YW5rIGNvdW50ZXIgdG8gYSBzYWZlIG1heGltdW0KPj4+PiArCSAqIHZhbHVlIG9mICsxIHdoZW5l dmVyIHRoZXJlIGlzIHRoZSBwb3NzaWJpbGl0eSB0aGF0IGNvbmN1cnJlbnQgcmVhZGVycwo+Pj4+ ICsJICogb2YgdmJsYW5rIHRpbWVzdGFtcHMgY291bGQgYmUgYWN0aXZlIGF0IHRoZSBtb21lbnQs IGFzIHRoZSBjdXJyZW50Cj4+Pj4gKwkgKiBpbXBsZW1lbnRhdGlvbiBvZiB0aGUgdGltZXN0YW1w IGNhY2hpbmcgYW5kIHVwZGF0aW5nIGlzIG5vdCBzYWZlCj4+Pj4gKwkgKiBhZ2FpbnN0IGNvbmN1 cnJlbnQgcmVhZGVycyBmb3IgY2FsbHMgdG8gc3RvcmVfdmJsYW5rKCkgd2l0aCBhIGJ1bXAKPj4+ PiArCSAqIG9mIGFueXRoaW5nIGJ1dCArMS4gQSBidW1wICE9IDEgd291bGQgdmVyeSBsaWtlbHkg cmV0dXJuIGNvcnJ1cHRlZAo+Pj4+ICsJICogdGltZXN0YW1wcyB0byB1c2Vyc3BhY2UsIGJlY2F1 c2UgdGhlIHNhbWUgc2xvdCBpbiB0aGUgY2FjaGUgY291bGQKPj4+PiArCSAqIGJlIGNvbmN1cnJl bnRseSB3cml0dGVuIGJ5IHN0b3JlX3ZibGFuaygpIGFuZCByZWFkIGJ5IG9uZSBvZiB0aG9zZQo+ Pj4+ICsJICogcmVhZGVycyB3aXRob3V0IHRoZSByZWFkLXJldHJ5IGxvZ2ljIGRldGVjdGluZyB0 aGUgY29sbGlzaW9uLgo+Pj4+ICsJICoKPj4+PiArCSAqIENvbmN1cnJlbnQgcmVhZGVycyBjYW4g ZXhpc3Qgd2hlbiB3ZSBhcmUgY2FsbGVkIGZyb20gdGhlCj4+Pj4gKwkgKiBkcm1fdmJsYW5rX29m ZigpIG9yIGRybV92Ymxhbmtfb24oKSBmdW5jdGlvbnMgYW5kIG90aGVyIG5vbi12YmxhbmstCj4+ Pj4gKwkgKiBpcnEgY2FsbGVycy4gSG93ZXZlciwgYWxsIHRob3NlIGNhbGxzIHRvIHVzIGFyZSBo YXBwZW5pbmcgd2l0aCB0aGUKPj4+PiArCSAqIHZibF9sb2NrIGxvY2tlZCB0byBwcmV2ZW50IGRy bV92YmxhbmtfZ2V0KCksIHNvIHRoZSB2YmxhbmsgcmVmY291bnQKPj4+PiArCSAqIGNhbid0IGlu Y3JlYXNlIHdoaWxlIHdlIGFyZSBleGVjdXRpbmcuIFRoZXJlZm9yZSBhIHplcm8gcmVmY291bnQg YXQKPj4+PiArCSAqIHRoaXMgcG9pbnQgaXMgc2FmZSBmb3IgYXJiaXRyYXJ5IGNvdW50ZXIgYnVt cHMgaWYgd2UgYXJlIGNhbGxlZAo+Pj4+ICsJICogb3V0c2lkZSB2YmxhbmsgaXJxLCBhIG5vbi16 ZXJvIGNvdW50IGlzIG5vdCAxMDAlIHNhZmUuIFVuZm9ydHVuYXRlbHkKPj4+PiArCSAqIHdlIG11 c3QgYWxzbyBhY2NlcHQgYSByZWZjb3VudCBvZiAxLCBhcyB3aGVuZXZlciB3ZSBhcmUgY2FsbGVk IGZyb20KPj4+PiArCSAqIGRybV92YmxhbmtfZ2V0KCkgLT4gZHJtX3ZibGFua19lbmFibGUoKSB0 aGUgcmVmY291bnQgd2lsbCBiZSAxIGFuZAo+Pj4+ICsJICogd2UgbXVzdCBsZXQgdGhhdCBvbmUg cGFzcyB0aHJvdWdoIGluIG9yZGVyIHRvIG5vdCBsb3NlIHZibGFuayBjb3VudHMKPj4+PiArCSAq IGR1cmluZyB2YmxhbmsgaXJxIG9mZiAtIHdoaWNoIHdvdWxkIGNvbXBsZXRlbHkgZGVmZWF0IHRo ZSB3aG9sZQo+Pj4+ICsJICogcG9pbnQgb2YgdGhpcyByb3V0aW5lLgo+Pj4+ICsJICoKPj4+PiAr CSAqIFdoZW5ldmVyIHdlIGFyZSBjYWxsZWQgZnJvbSB2YmxhbmsgaXJxLCB3ZSBoYXZlIHRvIGFz c3VtZSBjb25jdXJyZW50Cj4+Pj4gKwkgKiByZWFkZXJzIGV4aXN0IG9yIGNhbiBzaG93IHVwIGFu eSB0aW1lIGR1cmluZyBvdXIgZXhlY3V0aW9uLCBldmVuIGlmCj4+Pj4gKwkgKiB0aGUgcmVmY291 bnQgaXMgY3VycmVudGx5IHplcm8sIGFzIHZibGFuayBpcnFzIGFyZSB1c3VhbGx5IG9ubHkKPj4+ PiArCSAqIGVuYWJsZWQgZHVlIHRvIHRoZSBwcmVzZW5jZSBvZiByZWFkZXJzLCBhbmQgYmVjYXVz ZSB3aGVuIHdlIGFyZSBjYWxsZWQKPj4+PiArCSAqIGZyb20gdmJsYW5rIGlycSB3ZSBjYW4ndCBo b2xkIHRoZSB2YmxfbG9jayB0byBwcm90ZWN0IHVzIGZyb20gc3VkZGVuCj4+Pj4gKwkgKiBidW1w cyBpbiB2YmxhbmsgcmVmY291bnQuIFRoZXJlZm9yZSBhbHNvIHJlc3RyaWN0IGJ1bXBzIHRvICsx IHdoZW4KPj4+PiArCSAqIGNhbGxlZCBmcm9tIHZibGFuayBpcnEuCj4+Pj4gKwkgKi8KPj4+PiAr CWlmICgoZGlmZiA+IDEpICYmIChhdG9taWNfcmVhZCgmdmJsYW5rLT5yZWZjb3VudCkgPiAxIHx8 Cj4+Pj4gKwkgICAgKGZsYWdzICYgRFJNX0NBTExFRF9GUk9NX1ZCTElSUSkpKSB7Cj4+Pj4gKwkJ RFJNX0RFQlVHX1ZCTCgiY2xhbXBpbmcgdmJsYW5rIGJ1bXAgdG8gMSBvbiBjcnRjICV1OiBkaWZm cj0ldSAiCj4+Pj4gKwkJCSAgICAgICJyZWZjb3VudCAldSwgdmJsaXJxICV1XG4iLCBwaXBlLCBk aWZmLAo+Pj4+ICsJCQkgICAgICBhdG9taWNfcmVhZCgmdmJsYW5rLT5yZWZjb3VudCksCj4+Pj4g KwkJCSAgICAgIChmbGFncyAmIERSTV9DQUxMRURfRlJPTV9WQkxJUlEpICE9IDApOwo+Pj4+ICsJ CWRpZmYgPSAxOwo+Pj4+ICsJfQo+Pj4+ICsKPj4+PiAgIAlEUk1fREVCVUdfVkJMKCJ1cGRhdGlu ZyB2YmxhbmsgY291bnQgb24gY3J0YyAldToiCj4+Pj4gICAJCSAgICAgICIgY3VycmVudD0ldSwg ZGlmZj0ldSwgaHc9JXUgaHdfbGFzdD0ldVxuIiwKPj4+PiAgIAkJICAgICAgcGlwZSwgdmJsYW5r LT5jb3VudCwgZGlmZiwgY3VyX3ZibGFuaywgdmJsYW5rLT5sYXN0KTsKPj4+PiAtLQo+Pj4+IDEu OS4xCj4+Pj4KPj4+Cj4+PiAtLQo+Pj4gRGFuaWVsIFZldHRlcgo+Pj4gU29mdHdhcmUgRW5naW5l ZXIsIEludGVsIENvcnBvcmF0aW9uCj4+PiBodHRwOi8vYmxvZy5mZndsbC5jaAo+Pgo+PiAtLQo+ PiBWaWxsZSBTeXJqw6Rsw6QKPj4gSW50ZWwgT1RDCj4KX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX18KZHJpLWRldmVsIG1haWxpbmcgbGlzdApkcmktZGV2ZWxA bGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHBzOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxt YW4vbGlzdGluZm8vZHJpLWRldmVsCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f65.google.com ([74.125.82.65]:36201 "EHLO mail-wm0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755146AbcBINjt (ORCPT ); Tue, 9 Feb 2016 08:39:49 -0500 Received: by mail-wm0-f65.google.com with SMTP id 128so3477894wmz.3 for ; Tue, 09 Feb 2016 05:39:48 -0800 (PST) Subject: Re: [PATCH 2/6] drm: Prevent vblank counter bumps > 1 with active vblank clients. To: Daniel Vetter , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= 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> 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: <56B9EC20.9050400@gmail.com> Date: Tue, 9 Feb 2016 14:39:44 +0100 MIME-Version: 1.0 In-Reply-To: <20160209102302.GT11240@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 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. -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 >