From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Brezillon Subject: Re: [PATCH 1/5] drm: don't block fb changes for async plane updates Date: Tue, 12 Mar 2019 10:32:09 +0100 Message-ID: <20190312103209.57e05982@collabora.com> References: <20190304144909.6267-1-helen.koike@collabora.com> <20190304144909.6267-2-helen.koike@collabora.com> <20190311110616.6b474865@collabora.com> <01f2b3ba-434a-f61f-e8e8-85f3c9107a5c@amd.com> <20190311152009.7c55b797@collabora.com> <20190311195127.GT2665@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from bhuna.collabora.co.uk (bhuna.collabora.co.uk [IPv6:2a00:1098:0:82:1000:25:2eeb:e3e3]) by gabe.freedesktop.org (Postfix) with ESMTPS id 8BB1C89E50 for ; Tue, 12 Mar 2019 09:32:15 +0000 (UTC) In-Reply-To: <20190311195127.GT2665@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 , Sean Paul , =?UTF-8?B?U3TDqXBoYW5l?= Marchesin , Eric Anholt Cc: David Airlie , "daniel.vetter@ffwll.ch" , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , Tomasz Figa , Helen Koike , "kernel@collabora.com" , "Kazlauskas, Nicholas" List-Id: dri-devel@lists.freedesktop.org T24gTW9uLCAxMSBNYXIgMjAxOSAyMDo1MToyNyArMDEwMApEYW5pZWwgVmV0dGVyIDxkYW5pZWxA ZmZ3bGwuY2g+IHdyb3RlOgoKPiBPbiBNb24sIE1hciAxMSwgMjAxOSBhdCAwMzoyMDowOVBNICsw MTAwLCBCb3JpcyBCcmV6aWxsb24gd3JvdGU6Cj4gPiBPbiBNb24sIDExIE1hciAyMDE5IDEzOjE1 OjIzICswMDAwCj4gPiAiS2F6bGF1c2thcywgTmljaG9sYXMiIDxOaWNob2xhcy5LYXpsYXVza2Fz QGFtZC5jb20+IHdyb3RlOgo+ID4gICAKPiA+ID4gT24gMy8xMS8xOSA2OjA2IEFNLCBCb3JpcyBC cmV6aWxsb24gd3JvdGU6ICAKPiA+ID4gPiBIZWxsbyBOaWNob2xhcywKPiA+ID4gPiAKPiA+ID4g PiBPbiBNb24sIDQgTWFyIDIwMTkgMTU6NDY6NDkgKzAwMDAKPiA+ID4gPiAiS2F6bGF1c2thcywg TmljaG9sYXMiIDxOaWNob2xhcy5LYXpsYXVza2FzQGFtZC5jb20+IHdyb3RlOgo+ID4gPiA+ICAg ICAKPiA+ID4gPj4gT24gMy80LzE5IDk6NDkgQU0sIEhlbGVuIEtvaWtlIHdyb3RlOiAgICAKPiA+ ID4gPj4+IEluIHRoZSBjYXNlIG9mIGEgbm9ybWFsIHN5bmMgdXBkYXRlLCB0aGUgcHJlcGFyYXRp b24gb2YgZnJhbWVidWZmZXJzIChiZQo+ID4gPiA+Pj4gaXQgY2FsbGluZyBkcm1fYXRvbWljX2hl bHBlcl9wcmVwYXJlX3BsYW5lcygpIG9yIGRvaW5nIHNldHVwcyB3aXRoCj4gPiA+ID4+PiBkcm1f ZnJhbWVidWZmZXJfZ2V0KCkpIGFyZSBwZXJmb3JtZWQgaW4gdGhlIG5ld19zdGF0ZSBhbmQgdGhl IHJlc3BlY3RpdmUKPiA+ID4gPj4+IGNsZWFudXBzIGFyZSBwZXJmb3JtZWQgaW4gdGhlIG9sZF9z dGF0ZS4KPiA+ID4gPj4+Cj4gPiA+ID4+PiBJbiB0aGUgY2FzZSBvZiBhc3luYyB1cGRhdGVzLCB0 aGUgcHJlcGFyYXRpb24gaXMgYWxzbyBkb25lIGluIHRoZQo+ID4gPiA+Pj4gbmV3X3N0YXRlIGJ1 dCB0aGUgY2xlYW51cHMgYXJlIGRvbmUgaW4gdGhlIG5ld19zdGF0ZSAoYmVjYXVzZSB1cGRhdGVz Cj4gPiA+ID4+PiBhcmUgcGVyZm9ybWVkIGluIHBsYWNlLCBpLmUuIGluIHRoZSBjdXJyZW50IHN0 YXRlKS4KPiA+ID4gPj4+Cj4gPiA+ID4+PiBUaGUgY3VycmVudCBjb2RlIGJsb2NrcyBhc3luYyB1 ZHBhdGVzIHdoZW4gdGhlIGZiIGlzIGNoYW5nZWQsIHR1cm5pbmcKPiA+ID4gPj4+IGFzeW5jIHVw ZGF0ZXMgaW50byBzeW5jIHVwZGF0ZXMsIHNsb3dpbmcgZG93biBjdXJzb3IgdXBkYXRlcyBhbmQK PiA+ID4gPj4+IGludHJvZHVjaW5nIHJlZ3Jlc3Npb25zIGluIGlndCB0ZXN0cyB3aXRoIGVycm9y cyBvZiB0eXBlOgo+ID4gPiA+Pj4KPiA+ID4gPj4+ICJDUklUSUNBTDogY29tcGxldGVkIDk3IGN1 cnNvciB1cGRhdGVkIGluIGEgcGVyaW9kIG9mIDMwIGZsaXBzLCB3ZQo+ID4gPiA+Pj4gZXhwZWN0 IHRvIGNvbXBsZXRlIGFwcHJveGltYXRlbHkgMTUzNjAgdXBkYXRlcywgd2l0aCB0aGUgdGhyZXNo b2xkIHNldAo+ID4gPiA+Pj4gYXQgNzY4MCIKPiA+ID4gPj4+Cj4gPiA+ID4+PiBGYiBjaGFuZ2Vz IGluIGFzeW5jIHVwZGF0ZXMgd2VyZSBwcmV2ZW50ZWQgdG8gYXZvaWQgdGhlIGZvbGxvd2luZyBz Y2VuYXJpbzoKPiA+ID4gPj4+Cj4gPiA+ID4+PiAtIEFzeW5jIHVwZGF0ZSwgb2xkZmIgPSBOVUxM LCBuZXdmYiA9IGZiMSwgcHJlcGFyZSBmYjEsIGNsZWFudXAgZmIxCj4gPiA+ID4+PiAtIEFzeW5j IHVwZGF0ZSwgb2xkZmIgPSBmYjEsIG5ld2ZiID0gZmIyLCBwcmVwYXJlIGZiMiwgY2xlYW51cCBm YjIKPiA+ID4gPj4+IC0gTm9uLWFzeW5jIGNvbW1pdCwgb2xkZmIgPSBmYjIsIG5ld2ZiID0gZmIx LCBwcmVwYXJlIGZiMSwgY2xlYW51cCBmYjIgKHdyb25nKQo+ID4gPiA+Pj4gV2hlcmUgd2UgaGF2 ZSBhIHNpbmdsZSBjYWxsIHRvIHByZXBhcmUgZmIyIGJ1dCBkb3VibGUgY2xlYW51cCBjYWxsIHRv IGZiMi4KPiA+ID4gPj4+Cj4gPiA+ID4+PiBUbyBzb2x2ZSB0aGUgYWJvdmUgcHJvYmxlbXMsIGlu c3RlYWQgb2YgYmxvY2tpbmcgYXN5bmMgZmIgY2hhbmdlcywgd2UKPiA+ID4gPj4+IHBsYWNlIHRo ZSBvbGQgZnJhbWVidWZmZXIgaW4gdGhlIG5ld19zdGF0ZSBvYmplY3QsIHNvIHdoZW4gdGhlIGNv ZGUKPiA+ID4gPj4+IHBlcmZvcm1zIGNsZWFudXBzIGluIHRoZSBuZXdfc3RhdGUgaXQgd2lsbCBj bGVhbnVwIHRoZSBvbGRfZmIgYW5kIHdlCj4gPiA+ID4+PiB3aWxsIGhhdmUgdGhlIGZvbGxvd2lu ZyBzY2VuYXJpbyBpbnN0ZWFkOgo+ID4gPiA+Pj4KPiA+ID4gPj4+IC0gQXN5bmMgdXBkYXRlLCBv bGRmYiA9IE5VTEwsIG5ld2ZiID0gZmIxLCBwcmVwYXJlIGZiMSwgbm8gY2xlYW51cAo+ID4gPiA+ Pj4gLSBBc3luYyB1cGRhdGUsIG9sZGZiID0gZmIxLCBuZXdmYiA9IGZiMiwgcHJlcGFyZSBmYjIs IGNsZWFudXAgZmIxCj4gPiA+ID4+PiAtIE5vbi1hc3luYyBjb21taXQsIG9sZGZiID0gZmIyLCBu ZXdmYiA9IGZiMSwgcHJlcGFyZSBmYjEsIGNsZWFudXAgZmIyCj4gPiA+ID4+Pgo+ID4gPiA+Pj4g V2hlcmUgY2FsbHMgdG8gcHJlcGFyZS9jbGVhbnVwIGFyZSBiYWxsYW5jZWQuCj4gPiA+ID4+Pgo+ ID4gPiA+Pj4gQ2M6IDxzdGFibGVAdmdlci5rZXJuZWwub3JnPiAjIHY0LjE0KzogMjVkYzE5NGIz NGRkOiBkcm06IEJsb2NrIGZiIGNoYW5nZXMgZm9yIGFzeW5jIHBsYW5lIHVwZGF0ZXMKPiA+ID4g Pj4+IEZpeGVzOiAyNWRjMTk0YjM0ZGQgKCJkcm06IEJsb2NrIGZiIGNoYW5nZXMgZm9yIGFzeW5j IHBsYW5lIHVwZGF0ZXMiKQo+ID4gPiA+Pj4gU3VnZ2VzdGVkLWJ5OiBCb3JpcyBCcmV6aWxsb24g PGJvcmlzLmJyZXppbGxvbkBjb2xsYWJvcmEuY29tPgo+ID4gPiA+Pj4gU2lnbmVkLW9mZi1ieTog SGVsZW4gS29pa2UgPGhlbGVuLmtvaWtlQGNvbGxhYm9yYS5jb20+Cj4gPiA+ID4+Pgo+ID4gPiA+ Pj4gLS0tCj4gPiA+ID4+PiBIZWxsbywKPiA+ID4gPj4+Cj4gPiA+ID4+PiBBcyBtZW50aW9uZWQg aW4gdGhlIGNvdmVyIGxldHRlciwKPiA+ID4gPj4+IEkgdGVzdGVkIG9uIHRoZSByb2NrY2hpcCBh bmQgb24gaTkxNSAod2l0aCBhIHBhdGNoIEkgYW0gc3RpbGwgd29ya2luZyBvbiBmb3IKPiA+ID4g Pj4+IHJlcGxhY2luZyBjdXJzb3JzIGJ5IGFzeW5jIHVwZGF0ZSksIHdpdGggaWd0IHBsYW5lX2N1 cnNvcl9sZWdhY3kgYW5kCj4gPiA+ID4+PiBrbXNfY3Vyc29yX2xlZ2FjeSBhbmQgSSBkaWRuJ3Qg c2VlIGFueSByZWdyZXNzaW9ucy4KPiA+ID4gPj4+IEkgY291bGRuJ3QgdGVzdCBvbiBNU00gYW5k IEFNRCBiZWNhdXNlIEkgZG9uJ3QgaGF2ZSB0aGUgaGFyZHdhcmUgKGFuZCBJIGFtCj4gPiA+ID4+ PiBoYXZpbmcgc29tZSBpc3N1ZXMgdGVzdGluZyBvbiB2YzQpIGFuZCBJIHdvdWxkIGFwcHJlY2lh dGUgaWYgYW55b25lIGNvdWxkIGhlbHAKPiA+ID4gPj4+IG1lIHRlc3RpbmcgdGhvc2UuCj4gPiA+ ID4+Pgo+ID4gPiA+Pj4gSSBhbHNvIHRoaW5rIGl0IHdvdWxkIGJlIGEgYmV0dGVyIHNvbHV0aW9u IGlmLCBpbnN0ZWFkIG9mIGhhdmluZyBhc3luYwo+ID4gPiA+Pj4gdG8gZG8gaW4tcGxhY2UgdXBk YXRlcyBpbiB0aGUgY3VycmVudCBzdGF0ZSwgdGhlIGFzeW5jIHBhdGggc2hvdWxkIGJlCj4gPiA+ ID4+PiBlcXVpdmFsZW50IHRvIGEgc3luY3Jvbm91cyB1cGRhdGUsIGkuZS4sIG1vZGlmeWluZyBu ZXdfc3RhdGUgYW5kCj4gPiA+ID4+PiBwZXJmb3JtaW5nIGEgZmxpcAo+ID4gPiA+Pj4gSU1ITywg dGhlIG9ubHkgZGlmZmVyZW5jZSBiZXR3ZWVuIHN5bmMgYW5kIGFzeW5jIHNob3VsZCBiZSB0aGF0 IGFzeW5jIHVwZGF0ZQo+ID4gPiA+Pj4gZG9lc24ndCB3YWl0IGZvciB2YmxhbmsgYW5kIGFwcGxp ZXMgdGhlIGNoYW5nZXMgaW1tZWRpdGFsbHkgdG8gdGhlIGh3LAo+ID4gPiA+Pj4gYnV0IHRoZSBj b2RlIHBhdGggY291bGQgYmUgYWxtb3N0IHRoZSBzYW1lLgo+ID4gPiA+Pj4gQnV0IGZvciBub3cg SSB0aGluayB0aGlzIHNvbHV0aW9uIGlzIG9rIChzd2FwaW5nIG5ld19mYi9vbGRfZmIpLCBhbmQK PiA+ID4gPj4+IHRoZW4gd2UgY2FuIGFkanVzdCB0aGluZ3MgbGl0dGxlIGJ5IGxpdHRsZSwgd2hh dCBkbyB5b3UgdGhpbms/Cj4gPiA+ID4+Pgo+ID4gPiA+Pj4gVGhhbmtzIQo+ID4gPiA+Pj4gSGVs ZW4KPiA+ID4gPj4+Cj4gPiA+ID4+PiAgICBkcml2ZXJzL2dwdS9kcm0vZHJtX2F0b21pY19oZWxw ZXIuYyB8IDIwICsrKysrKysrKystLS0tLS0tLS0tCj4gPiA+ID4+PiAgICAxIGZpbGUgY2hhbmdl ZCwgMTAgaW5zZXJ0aW9ucygrKSwgMTAgZGVsZXRpb25zKC0pCj4gPiA+ID4+Pgo+ID4gPiA+Pj4g ZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9kcm1fYXRvbWljX2hlbHBlci5jIGIvZHJpdmVy cy9ncHUvZHJtL2RybV9hdG9taWNfaGVscGVyLmMKPiA+ID4gPj4+IGluZGV4IDU0MGE3N2EyYWRl OS4uZTdlYjk2ZjFlZmMyIDEwMDY0NAo+ID4gPiA+Pj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2Ry bV9hdG9taWNfaGVscGVyLmMKPiA+ID4gPj4+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1fYXRv bWljX2hlbHBlci5jCj4gPiA+ID4+PiBAQCAtMTYwOCwxNSArMTYwOCw2IEBAIGludCBkcm1fYXRv bWljX2hlbHBlcl9hc3luY19jaGVjayhzdHJ1Y3QgZHJtX2RldmljZSAqZGV2LAo+ID4gPiA+Pj4g ICAgCSAgICBvbGRfcGxhbmVfc3RhdGUtPmNydGMgIT0gbmV3X3BsYW5lX3N0YXRlLT5jcnRjKQo+ ID4gPiA+Pj4gICAgCQlyZXR1cm4gLUVJTlZBTDsKPiA+ID4gPj4+ICAgIAo+ID4gPiA+Pj4gLQkv Kgo+ID4gPiA+Pj4gLQkgKiBGSVhNRTogU2luY2UgcHJlcGFyZV9mYiBhbmQgY2xlYW51cF9mYiBh cmUgYWx3YXlzIGNhbGxlZCBvbgo+ID4gPiA+Pj4gLQkgKiB0aGUgbmV3X3BsYW5lX3N0YXRlIGZv ciBhc3luYyB1cGRhdGVzIHdlIG5lZWQgdG8gYmxvY2sgZnJhbWVidWZmZXIKPiA+ID4gPj4+IC0J ICogY2hhbmdlcy4gVGhpcyBwcmV2ZW50cyB1c2Ugb2YgYSBmYiB0aGF0J3MgYmVlbiBjbGVhbmVk IHVwIGFuZAo+ID4gPiA+Pj4gLQkgKiBkb3VibGUgY2xlYW51cHMgZnJvbSBvY2N1cmluZy4KPiA+ ID4gPj4+IC0JICovCj4gPiA+ID4+PiAtCWlmIChvbGRfcGxhbmVfc3RhdGUtPmZiICE9IG5ld19w bGFuZV9zdGF0ZS0+ZmIpCj4gPiA+ID4+PiAtCQlyZXR1cm4gLUVJTlZBTDsKPiA+ID4gPj4+IC0K PiA+ID4gPj4+ICAgIAlmdW5jcyA9IHBsYW5lLT5oZWxwZXJfcHJpdmF0ZTsKPiA+ID4gPj4+ICAg IAlpZiAoIWZ1bmNzLT5hdG9taWNfYXN5bmNfdXBkYXRlKQo+ID4gPiA+Pj4gICAgCQlyZXR1cm4g LUVJTlZBTDsKPiA+ID4gPj4+IEBAIC0xNjU3LDYgKzE2NDgsOSBAQCB2b2lkIGRybV9hdG9taWNf aGVscGVyX2FzeW5jX2NvbW1pdChzdHJ1Y3QgZHJtX2RldmljZSAqZGV2LAo+ID4gPiA+Pj4gICAg CWludCBpOwo+ID4gPiA+Pj4gICAgCj4gPiA+ID4+PiAgICAJZm9yX2VhY2hfbmV3X3BsYW5lX2lu X3N0YXRlKHN0YXRlLCBwbGFuZSwgcGxhbmVfc3RhdGUsIGkpIHsKPiA+ID4gPj4+ICsJCXN0cnVj dCBkcm1fZnJhbWVidWZmZXIgKm5ld19mYiA9IHBsYW5lX3N0YXRlLT5mYjsKPiA+ID4gPj4+ICsJ CXN0cnVjdCBkcm1fZnJhbWVidWZmZXIgKm9sZF9mYiA9IHBsYW5lLT5zdGF0ZS0+ZmI7Cj4gPiA+ ID4+PiArCj4gPiA+ID4+PiAgICAJCWZ1bmNzID0gcGxhbmUtPmhlbHBlcl9wcml2YXRlOwo+ID4g PiA+Pj4gICAgCQlmdW5jcy0+YXRvbWljX2FzeW5jX3VwZGF0ZShwbGFuZSwgcGxhbmVfc3RhdGUp Owo+ID4gPiA+Pj4gICAgCj4gPiA+ID4+PiBAQCAtMTY2NSwxMSArMTY1OSwxNyBAQCB2b2lkIGRy bV9hdG9taWNfaGVscGVyX2FzeW5jX2NvbW1pdChzdHJ1Y3QgZHJtX2RldmljZSAqZGV2LAo+ID4g PiA+Pj4gICAgCQkgKiBwbGFuZS0+c3RhdGUgaW4tcGxhY2UsIG1ha2Ugc3VyZSBhdCBsZWFzdCBj b21tb24KPiA+ID4gPj4+ICAgIAkJICogcHJvcGVydGllcyBoYXZlIGJlZW4gcHJvcGVybHkgdXBk YXRlZC4KPiA+ID4gPj4+ICAgIAkJICovCj4gPiA+ID4+PiAtCQlXQVJOX09OX09OQ0UocGxhbmUt PnN0YXRlLT5mYiAhPSBwbGFuZV9zdGF0ZS0+ZmIpOwo+ID4gPiA+Pj4gKwkJV0FSTl9PTl9PTkNF KHBsYW5lLT5zdGF0ZS0+ZmIgIT0gbmV3X2ZiKTsKPiA+ID4gPj4+ICAgIAkJV0FSTl9PTl9PTkNF KHBsYW5lLT5zdGF0ZS0+Y3J0Y194ICE9IHBsYW5lX3N0YXRlLT5jcnRjX3gpOwo+ID4gPiA+Pj4g ICAgCQlXQVJOX09OX09OQ0UocGxhbmUtPnN0YXRlLT5jcnRjX3kgIT0gcGxhbmVfc3RhdGUtPmNy dGNfeSk7Cj4gPiA+ID4+PiAgICAJCVdBUk5fT05fT05DRShwbGFuZS0+c3RhdGUtPnNyY194ICE9 IHBsYW5lX3N0YXRlLT5zcmNfeCk7Cj4gPiA+ID4+PiAgICAJCVdBUk5fT05fT05DRShwbGFuZS0+ c3RhdGUtPnNyY195ICE9IHBsYW5lX3N0YXRlLT5zcmNfeSk7Cj4gPiA+ID4+PiArCj4gPiA+ID4+ PiArCQkvKgo+ID4gPiA+Pj4gKwkJICogTWFrZSBzdXJlIHRoZSBGQnMgaGF2ZSBiZWVuIHN3YXBw ZWQgc28gdGhhdCBjbGVhbnVwcyBpbiB0aGUKPiA+ID4gPj4+ICsJCSAqIG5ld19zdGF0ZSBwZXJm b3JtcyBhIGNsZWFudXAgaW4gdGhlIG9sZCBGQi4KPiA+ID4gPj4+ICsJCSAqLwo+ID4gPiA+Pj4g KwkJV0FSTl9PTl9PTkNFKHBsYW5lX3N0YXRlLT5mYiAhPSBvbGRfZmIpOyAgICAKPiA+ID4gPj4K PiA+ID4gPj4gSSBwZXJzb25hbGx5IHRoaW5rIHRoaXMgYXBwcm9hY2ggaXMgZmluZSBhbmQgdGhl IFdBUk5fT04gcyBhcmUgZ29vZCBmb3IKPiA+ID4gPj4gY2F0Y2hpbmcgZHJpdmVycyB0aGF0IHdh bnQgdG8gdXNlIHRoZXNlIGluIHRoZSBmdXR1cmUuICAgIAo+ID4gPiA+IAo+ID4gPiA+IFdlbGws IEkgYWdyZWUgdGhpcyBjaGFuZ2UgaXMgdGhlIHdheSB0byBnbyBmb3IgYSBzaG9ydC10ZXJtIHNv bHV0aW9uCj4gPiA+ID4gdG8gcmVsYXggdGhlIG9sZF9mYiA9PSBuZXdfZmIgY29uc3RyYWludCwg YnV0IEkga2VlcCB0aGlua2luZyB0aGlzIHdob2xlCj4gPiA+ID4gInVwZGF0ZSBwbGFuZV9zdGF0 ZSBpbiBwbGFjZSIgaXMgYSByZWNpcGUgZm9yIHRyb3VibGUgYW5kIGp1c3QgbWFrZQo+ID4gPiA+ IHRoaW5ncyBtb3JlIGNvbXBsaWNhdGVkIGZvciBkcml2ZXJzIGZvciBubyBvYnZpb3VzIHJlYXNv bnMuIExvb2sgYXQgdGhlCj4gPiA+ID4gVkM0IGltcGxlbSBbMV0gaWYgeW91IG5lZWQgYSBwcm9v ZiB0aGF0IHRoaW5ncyBjYW4gZ2V0IG1lc3N5IHByZXR0eQo+ID4gPiA+IHF1aWNrbHkuCj4gPiA+ ID4gCj4gPiA+ID4gQWxsIHRoaXMgc3RhdGUtZmllbGRzLWNvcHlpbmcgc3RlcHMgY291bGQgYmUg c2tpcHBlZCBpZiB0aGUgY29yZSB3YXMKPiA+ID4gPiBzaW1wbHkgc3dhcHBpbmcgdGhlIG9sZC9u ZXcgc3RhdGVzIGFzIGlzIGRvbmUgaW4gdGhlIHN5bmMgdXBkYXRlIHBhdGguCj4gPiA+ID4gCj4g PiA+ID4gWzFdaHR0cHM6Ly9lbGl4aXIuYm9vdGxpbi5jb20vbGludXgvdjUuMC1yYzcvc291cmNl L2RyaXZlcnMvZ3B1L2RybS92YzQvdmM0X3BsYW5lLmMjTDg3OCAgICAKPiA+ID4gCj4gPiA+IEkg Y29tcGxldGVseSBhZ3JlZSB3aXRoIHRoaXMgdmlldyBGV0lXLiBJIGhhZCBhIGRpc2N1c3Npb24g d2l0aCBEYW5pZWwgCj4gPiA+IGFib3V0IHRoaXMgd2hlbiBJIGhhZCBwb3N0ZWQgdGhlIG9yaWdp bmFsIGJsb2NrIEZCIGNoYW5nZXMgcGF0Y2guCj4gPiA+IAo+ID4gPiAtIFRoZSBwbGFuZSBvYmpl Y3QgbmVlZHMgdG8gYmUgbG9ja2VkIGluIG9yZGVyIGZvciBhc3luYyBzdGF0ZSB0byBiZSB1cGRh dGVkCj4gPiA+IC0gQmxvY2tpbmcgY29tbWl0IHdvcmsgaG9sZHMgdGhlIGxvY2sgZm9yIHRoZSBw bGFuZSwgYXN5bmMgdXBkYXRlIHdvbid0IAo+ID4gPiBoYXBwZW4KPiA+ID4gLSBOb24tYmxvY2tp bmcgY29tbWl0IHdvcmsgdGhhdCdzIHN0aWxsIG9uZ29pbmcgd29uJ3QgaGF2ZSBod19kb25lIAo+ ID4gPiBzaWduYWxlZCBhbmQgZHJtX2F0b21pY19oZWxwZXJfYXN5bmNfY2hlY2sgd2lsbCBibG9j ayB0aGUgYXN5bmMgdXBkYXRlCj4gPiA+IAo+ID4gPiBTbyB0aGlzIGxvb2tzIHNhZmUgaW4gdGhl b3J5LCB3aXRoIHRoZSBleGNlcHRpb24gb2YgdGhlIGNhbGwgdG8gCj4gPiA+IGRybV9hdG9taWNf aGVscGVyX2NsZWFudXBfcGxhbmVzIG9jY3VyaW5nIGFmdGVyIGh3X2RvbmUgaXMgc2lnbmFsZWQu ICAKPiA+IAo+ID4gSXNuJ3QgaXQgYWxzbyB0aGUgY2FzZSBpbiB0aGUgc3luYyB1cGRhdGUgcGF0 aD8KPiA+ICAgCj4gPiA+IAo+ID4gPiBJIGJlbGlldmUgdGhhdCB0aGUgYmVoYXZpb3Igb2YgdGhp cyBmdW5jdGlvbiBzdGlsbCByZW1haW5zIHRoZSBzYW1lIGV2ZW4gCj4gPiA+IGlmIHBsYW5lLT5z dGF0ZSBpcyBzd2FwcGVkIHRvIHNvbWV0aGluZyBlbHNlIGR1cmluZyB0aGUgY2FsbCAoc2luY2Ug Cj4gPiA+IG9sZF9wbGFuZV9zdGF0ZSBzaG91bGQgbmV2ZXIgYmUgZXF1YWwgdG8gcGxhbmUtPnN0 YXRlIGlmIHRoZSBjb21taXQgCj4gPiA+IHN1Y2NlZWRlZCBhbmQgdGhlIHBsYW5lIGlzIGluIHRo ZSBjb21taXQpLCBidXQgSSdtIG5vdCBzdXJlIHRoYXQncyAKPiA+ID4gc29tZXRoaW5nIHdlJ2Qg d2FudCB0byByZWx5IG9uLgo+ID4gPiAKPiA+ID4gSSB0aGluayBvdGhlciB0aGFuIHRoYXQgaXNz dWUsIHlvdSBjb3VsZCBwcm9iYWJseSBqdXN0Ogo+ID4gPiAKPiA+ID4gZHJtX2F0b21pY19oZWxw ZXJfcHJlcGFyZV9wbGFuZXMoLi4uKTsKPiA+ID4gZHJtX2F0b21pY19oZWxwZXJfc3dhcF9zdGF0 ZSguLi4pOwo+ID4gPiBkcm1fYXRvbWljX3N0YXRlX2dldChzdGF0ZSk7ICAKPiA+IAo+ID4gV2h5 IGRvIHdlIG5lZWQgYSBzdGF0ZV9nZXQoKSBoZXJlPyBBRkFJQ1QsIGl0J3MgZG9uZSB0aGlzIHdh eSBpbiB0aGUKPiA+IHN5bmMgdXBkYXRlIHBhdGggYmVjYXVzZSBvZiB0aGUgbm9uLWJsb2NraW5n IHNlbWFudGljIHdoZXJlIHRoZSBzdGF0ZQo+ID4gbWlnaHQgYmUgcmVsZWFzZWQgYnkgdGhlIGNh bGxlciBiZWZvcmUgaXQncyBiZWVuIGFwcGxpZWQgYnkgdGhlIGNvbW1pdAo+ID4gd29ya2VyLgo+ ID4gICAKPiA+ID4gZHJtX2F0b21pY19oZWxwZXJfYXN5bmNfY29tbWl0KC4uLik7Cj4gPiA+IGRy bV9hdG9taWNfaGVscGVyX2NsZWFudXBfcGxhbmVzKGRldiwgc3RhdGUpOwo+ID4gPiAKPiA+ID4g YW5kIGl0IHdvdWxkIHdvcmsgYXMgZXhwZWN0ZWQuIEJ1dCB0aGVyZSBzdGlsbCBtYXkgYmUgb3Ro ZXIgdGhpbmdzIEknbSAKPiA+ID4gbWlzc2luZyBvciBoYXZlbid0IGNvbnNpZGVyZWQgaGVyZS4g IAo+ID4gCj4gPiBBY3R1YWxseSwgd2hlbiBJIHNhaWQgd2UgY291bGQgc3dhcCBzdGF0ZXMsIEkg d2FzIG5vdCBuZWNlc3NhcmlseQo+ID4gdGhpbmtpbmcgYWJvdXQgcmUtdXNpbmcgZHJtX2F0b21p Y19oZWxwZXJfc3dhcF9zdGF0ZSgpLCBidXQgaW5zdGVhZAo+ID4gc3dhcCBzdGF0ZXMgZGlyZWN0 bHkgaW4gZHJtX2F0b21pY19oZWxwZXJfYXN5bmNfY29tbWl0KCk6Cj4gPiAKPiA+IAlmb3JfZWFj aF9vbGRuZXdfcGxhbmVfaW5fc3RhdGUoc3RhdGUsIHBsYW5lLCBvbGRfcGxhbmVfc3RhdGUsCj4g PiAJCQkJICAgICAgIG5ld19wbGFuZV9zdGF0ZSwgaSkgewo+ID4gCQlXQVJOX09OKHBsYW5lLT5z dGF0ZSAhPSBvbGRfcGxhbmVfc3RhdGUpOwo+ID4gCQlvbGRfcGxhbmVfc3RhdGUtPnN0YXRlID0g c3RhdGU7Cj4gPiAJCW5ld19wbGFuZV9zdGF0ZS0+c3RhdGUgPSBOVUxMOwo+ID4gCQlzdGF0ZS0+ cGxhbmVzW2ldLnN0YXRlID0gb2xkX3BsYW5lX3N0YXRlOwo+ID4gCQlwbGFuZS0+c3RhdGUgPSBu ZXdfcGxhbmVfc3RhdGU7Cj4gPiAKPiA+IAkJZnVuY3MgPSBwbGFuZS0+aGVscGVyX3ByaXZhdGU7 Cj4gPiAJCWZ1bmNzLT5hdG9taWNfYXN5bmNfdXBkYXRlKHBsYW5lLCBuZXdfcGxhbmVfc3RhdGUp Owo+ID4gCX0KPiA+IAo+ID4gVGhpcyB3YXkgd2Ugd291bGQgYXZvaWQgdGhlIFdBUk5fT04oKSBs aW5lcyB3ZSBoYXZlIGluCj4gPiBkcm1fYXRvbWljX2hlbHBlcl9hc3luY19jb21taXQoKSB0byBj aGVjayB0aGF0IHRoaW5ncyBoYXZlIGJlZW4KPiA+IHByb3Blcmx5IHVwZGF0ZWQgaW4tcGxhY2Us IGFuZCB3ZSB3b3VsZCBhbHNvIGdldCByaWQgb2YgdGhlIGRyaXZlcgo+ID4gY29kZSBjb3B5aW5n IHRoZSBwbGFuZV9zdGF0ZSBwcm9wZXJ0eSB0aGF0IGNhbiBjaGFuZ2UgZHVyaW5nIGFuIGFzeW5j Cj4gPiB1cGRhdGUuCj4gPiAKPiA+IEJ1dCwgYXMgeW91IHNhaWQsIEkgbWlnaHQgYmUgbWlzc2lu ZyBvdGhlciBwb3RlbnRpYWwgaXNzdWVzLiAgCj4gCj4gT2sgSSBkdWcgYXJvdW5kIGFnYWluLCBh bmQgSSB0aGluayBJIHJlY29uc3RydWN0ZWQgdGhlIHByb2JsZW0gYWdhaW4uCgpHcmVhdCEKCj4g Cj4gVGhlIGlzc3VlIGlzIHRoZSBsaWZldGltZXMgb2Ygc3RhdGUgc3RydWN0cy4gVGhlIG5vbmJs b2NraW5nIGNvbW1pdCB3b3JrZXIKPiBkb2Vzbid0IGhvbGQgYSByZWZlcmVuY2Ugb250byB0aGUg bmV3IHN0YXRlcyBhdCBhbGwuIFRoZSBvbmx5IHJlYXNvbiB0aG9zZQo+IG5ldyBzdGF0ZXMgY2Fu bm90IGRpc2FwcGVhciBpcyB0aGF0IHRoZSBuZXh0IGF0b21pYyBjb21pdCB0b3VjaGluZyB0aGUK PiBzYW1lIHN0YXRlcyB3YWl0cyBmb3IgY3J0Y19jb21taXQuaHdfZG9uZSBiZWZvcmUgaXQgcHVz aGVzIGl0cyBvd24gdXBkYXRlCj4gdGhyb3VnaCAoYW5kIHRoZW4gZ29lcyBhbmQgcmVsZWFzZXMg dGhvc2Ugc3RhdGUgc3RydWN0dXJlcykuCgpCeSBkaXNhcHBlYXIgSSBndWVzcyB5b3UgbWVhbiB3 aGVuIGl0J3MgcmVwbGFjZWQgaW4gcGxhbmUtPnN0YXRlIGJ5IGEKc3Vic2VxdWVudCBhdG9taWMg Y29tbWl0IHRoYXQgcGxhY2VzIHRoZW0gaW4gdGhlIG9sZF9zdGF0ZSBzbG90IGFuZApyZWxlYXNl IHRoZW0gYXMgcGFydCBvZiB0aGUgZHJtX2F0b21pY19zdGF0ZV9wdXQoKSBjYWxsIHdoZW4gcmV0 dXJuaW5nCmZyb20gYSBub24tYmxvY2tpbmcgYXRvbWljIHVwZGF0ZS4gQW55IHJlYXNvbiB3ZSBj b3VsZG4ndCByZXRhaW4KbmV3X3N0YXRlIHJlZnMgdW50aWwgd2UncmUgZG9uZSBtYW5pcHVsYXRp bmcgdGhlbSB0byBvdmVyY29tZSB0aGlzCnByb2JsZW0/Cgo+IAo+IFRoZSBvbGQgc3RhdGUgaGFz IG5vIHN1Y2ggaXNzdWUsIHNpbmNlIGVhY2ggY29tbWl0IHRha2VzIG93bmVyc2hpcCBvZiB0aGUK PiBvbGQgc3RhdGUgYW5kIHRoZW4gcmVsZWFzZXMgaXQuIEFuZCBjYW4gZG8gdGhhdCBhbnkgdGlt ZSBhZnRlciBod19kb25lLgoKSSdkIGV4cGVjdCB0aGUgd2FpdCBvbiBod19kb25lIHRvIGJlIG5l ZWRlZCBhbnl3YXkgZm9yIGFzeW5jIGNvbW1pdHMKZ29pbmcgYWZ0ZXIgc3luYyBvbmVzLiBBcyB0 aGUgY29tbWVudCBzYXlzLCBpZiB3ZSBkb24ndCB3YWl0IGZvcgpod19kb25lLCB0aGUgYXN5bmMg dXBkYXRlIHNldHRpbmdzIG1pZ2h0IGJlIG92ZXJyaWRkZW4gYnkgdGhlIHN5bmMKdXBkYXRlIG9u ZXMuCgo+IAo+IE5vdyB3aXRoIHRoZSBjdXJyZW50IGFzeW5jIGNvZGUgdGhhdCdzIG5vIGlzc3Vl LCBiZWNhdXNlIHdlIGRvIGNoZWNrIGZvcgo+IGh3X2RvbmUuIFRoZSB0cm91YmxlIGlzIHRoYXQg aHdfZG9uZSBpcyBhIGtlcm5lbC1pbnRlcm5hbCBpbXBsZW1lbnRhdGlvbgo+IGRldGFpbC4gVGhl IG9ubHkgdGhpbmsgdXNlcnNwYWNlIGNhbiBvYnNlcnZlIGlzIGZsaXBfZG9uZSwgYW5kIHRoYXQn cwo+IHdoYXQncyB1c2VkIGZvciAtRUJVU1kgZm9yIG5vcm1hbCBwYWdlLWZsaXBzLiBGb3IgY3Vy c29yIHRoaXMga2luZGEKPiBkb2Vzbid0IG1hdHRlciwgYmVjYXVzZSB0aGVzZSB0d28gc2hvdWxk IGJlIGZhaXJseSBjbG9zZSB0b2dldGhlciAoaW4gbW9zdAo+IGNhc2VzIGh3X2RvbmUgZXZlbiBo YXBwZW5zIGJlZm9yZSBmbGlwX2RvbmUsIGJ1dCB0aGF0IGRlcGVuZHMgdXBvbiB0aGUKPiBkcml2 ZXIpLiBTbyB0aGUgb2NjYXNpb25hbCBzaWxlbnQgZmFsbGJhY2sgdG8gYSBzeW5jaHJvbm91cyBj b21taXQgZG9lc24ndAo+IHJlYWxseSBtYXR0ZXIuCj4gCj4gV2hhdCB3ZSBjb3VsZCBkbyBpcyBq dXN0IHdhaXQgZm9yIGh3X2RvbmUgZm9yIGFzeW5jIGNvbW1pdHMsIGJ1dCB0aGF0J3MKPiBraW5k YSBub3QgY29vbCBlaXRoZXIgc2luY2UgaXQgYmxvY2tzIChhZ2FpbiBjdXJzb3IgaXMgaWxsLWRl ZmluZWQgZW5vdWdoCj4gdGhhdCBpdCBkb2Vzbid0IG1hdHRlcikuIEFuZCBwdXNoaW5nIGFzeW5j IHVwZGF0ZXMgdG8gYSB3b3JrZXIgbWVhbnMgd2UKPiBuZWVkIHRvIGdyZWF0bHkgZXh0ZW5kIHRo ZSBjcnRjX2NvbW1pdCB0cmFja2luZyAoYXQgbGVhc3QgdG8gZWFjaCBwbGFuZQo+IHN0YXRlKS4g SSB0aGluayBtb3N0IG9mIHRoYXQgZXhpc3Qgbm93LCBzaW5jZSB3ZSBoYWQgdG8gYWRkIGl0IGFu eXdheSBmb3IKPiBwbGFuZXMgd2hpY2ggY2FuIGJlIHJlYXNzaWduZWQgYmV0d2VlbiBjcnRjLgoK VG8gYmUgaG9uZXN0LCBJIGRvbid0IGtub3cgd2hhdCB0aGUgc2VtYW50aWMgb2YgYXN5bmMgY29t bWl0IHNob3VsZCBiZS4KRG9lcyBhc3luYyAodXBkYXRlIHRoaW5ncyBiZXR3ZWVuIDIgVkJMQU5L UyBhdCB0aGUgcmlzayBvZiBjYXVzaW5nCnRlYXJpbmcpIG5lY2Vzc2FyaWx5IGltcGxpZXMgbm9u LWJsb2NraW5nIChyZXR1cm4gYmVmb3JlIHRoZSB1cGRhdGUgaXMKYWN0dWFsbHkgcHVzaGVkIHRv IHRoZSBIVyk/Cgo+IAo+IHRsZHI7IG1heWJlIHdlIGNhbiBkbyB0aGUgZnVsbCBzd2FwcGluZyBu b3c/Cj4gCj4gSSBhZ3JlZSBpdCBmZWVscyBsaWtlIHRoZSBjbGVhbmVyIHNvbHV0aW9uLCBidXQg ZGVmaW5pdGVseSBuZWVkIGEgcGlsZSBvZgo+IGlndCB0ZXN0cyB0byBtYWtlIHN1cmUgd2UgY2Fu IG1peCZtYXRjaCBiZXR3ZWVuIGFzeW5jIGFuZCBzeW5jIGNvbW1pdHMgYW5kCj4gbm90aGluZyBi bG93cyB1cC4gQW5kIHN5bmMgY29tbWl0cyBuZWVkIHRvIHVzZSByZWFzc2lnbm1lbnQgb2YgcGxh bmVzIHRvCj4gZGlmZmVyZW50IGNydGNzIHBsdXMgbm9uYmxvY2tpbmcgY29tbWl0IChJIHRoaW5r IGFtZCBodyBjYW4gZG8gYWxsIHRoYXQsCj4gb3IgYXQgbGVhc3QgSSd2ZSBzZWVuIHByZXAgcGF0 Y2hlcykuCgpZZXMsIHRoYXQnZCBiZSBncmVhdCB0byBoYXZlIHRoYXQgaW4gcGxhY2UsIGVzcGVj aWFsbHkgaWYgd2Ugd2FudCB0bwpleHBvc2UgYXN5bmMgYXRvbWljIGNvbW1pdHMgdG8gdXNlcnNw YWNlIChyaWdodCBub3cgaXQncyBvbmx5IHVzZWQgZm9yCmxlZ2FjeSBjdXJzb3IgdXBkYXRlcyku Cl9fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCmRyaS1kZXZl bCBtYWlsaW5nIGxpc3QKZHJpLWRldmVsQGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xp c3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2RyaS1kZXZlbA== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 2DB3BC43381 for ; Tue, 12 Mar 2019 09:32:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id E2623214AE for ; Tue, 12 Mar 2019 09:32:18 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1726524AbfCLJcR (ORCPT ); Tue, 12 Mar 2019 05:32:17 -0400 Received: from bhuna.collabora.co.uk ([46.235.227.227]:58436 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725767AbfCLJcQ (ORCPT ); Tue, 12 Mar 2019 05:32:16 -0400 Received: from localhost (unknown [IPv6:2a01:e0a:2c:6930:5cf4:84a1:2763:fe0d]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) (Authenticated sender: bbrezillon) by bhuna.collabora.co.uk (Postfix) with ESMTPSA id 1FF38280747; Tue, 12 Mar 2019 09:32:13 +0000 (GMT) Date: Tue, 12 Mar 2019 10:32:09 +0100 From: Boris Brezillon To: Daniel Vetter , Sean Paul , =?UTF-8?B?U3TDqXBoYW5l?= Marchesin , Eric Anholt Cc: "Kazlauskas, Nicholas" , Helen Koike , "dri-devel@lists.freedesktop.org" , "Grodzovsky, Andrey" , "daniel.vetter@ffwll.ch" , "linux-kernel@vger.kernel.org" , Tomasz Figa , David Airlie , "kernel@collabora.com" , "Wentland, Harry" Subject: Re: [PATCH 1/5] drm: don't block fb changes for async plane updates Message-ID: <20190312103209.57e05982@collabora.com> In-Reply-To: <20190311195127.GT2665@phenom.ffwll.local> References: <20190304144909.6267-1-helen.koike@collabora.com> <20190304144909.6267-2-helen.koike@collabora.com> <20190311110616.6b474865@collabora.com> <01f2b3ba-434a-f61f-e8e8-85f3c9107a5c@amd.com> <20190311152009.7c55b797@collabora.com> <20190311195127.GT2665@phenom.ffwll.local> Organization: Collabora X-Mailer: Claws Mail 3.17.3 (GTK+ 2.24.32; x86_64-redhat-linux-gnu) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Mon, 11 Mar 2019 20:51:27 +0100 Daniel Vetter wrote: > On Mon, Mar 11, 2019 at 03:20:09PM +0100, Boris Brezillon wrote: > > On Mon, 11 Mar 2019 13:15:23 +0000 > > "Kazlauskas, Nicholas" wrote: > > > > > On 3/11/19 6:06 AM, Boris Brezillon wrote: > > > > Hello Nicholas, > > > > > > > > On Mon, 4 Mar 2019 15:46:49 +0000 > > > > "Kazlauskas, Nicholas" wrote: > > > > > > > >> On 3/4/19 9:49 AM, Helen Koike wrote: > > > >>> In the case of a normal sync update, the preparation of framebuffers (be > > > >>> it calling drm_atomic_helper_prepare_planes() or doing setups with > > > >>> drm_framebuffer_get()) are performed in the new_state and the respective > > > >>> cleanups are performed in the old_state. > > > >>> > > > >>> In the case of async updates, the preparation is also done in the > > > >>> new_state but the cleanups are done in the new_state (because updates > > > >>> are performed in place, i.e. in the current state). > > > >>> > > > >>> The current code blocks async udpates when the fb is changed, turning > > > >>> async updates into sync updates, slowing down cursor updates and > > > >>> introducing regressions in igt tests with errors of type: > > > >>> > > > >>> "CRITICAL: completed 97 cursor updated in a period of 30 flips, we > > > >>> expect to complete approximately 15360 updates, with the threshold set > > > >>> at 7680" > > > >>> > > > >>> Fb changes in async updates were prevented to avoid the following scenario: > > > >>> > > > >>> - Async update, oldfb = NULL, newfb = fb1, prepare fb1, cleanup fb1 > > > >>> - Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb2 > > > >>> - Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2 (wrong) > > > >>> Where we have a single call to prepare fb2 but double cleanup call to fb2. > > > >>> > > > >>> To solve the above problems, instead of blocking async fb changes, we > > > >>> place the old framebuffer in the new_state object, so when the code > > > >>> performs cleanups in the new_state it will cleanup the old_fb and we > > > >>> will have the following scenario instead: > > > >>> > > > >>> - Async update, oldfb = NULL, newfb = fb1, prepare fb1, no cleanup > > > >>> - Async update, oldfb = fb1, newfb = fb2, prepare fb2, cleanup fb1 > > > >>> - Non-async commit, oldfb = fb2, newfb = fb1, prepare fb1, cleanup fb2 > > > >>> > > > >>> Where calls to prepare/cleanup are ballanced. > > > >>> > > > >>> Cc: # v4.14+: 25dc194b34dd: drm: Block fb changes for async plane updates > > > >>> Fixes: 25dc194b34dd ("drm: Block fb changes for async plane updates") > > > >>> Suggested-by: Boris Brezillon > > > >>> Signed-off-by: Helen Koike > > > >>> > > > >>> --- > > > >>> Hello, > > > >>> > > > >>> As mentioned in the cover letter, > > > >>> I tested on the rockchip and on i915 (with a patch I am still working on for > > > >>> replacing cursors by async update), with igt plane_cursor_legacy and > > > >>> kms_cursor_legacy and I didn't see any regressions. > > > >>> I couldn't test on MSM and AMD because I don't have the hardware (and I am > > > >>> having some issues testing on vc4) and I would appreciate if anyone could help > > > >>> me testing those. > > > >>> > > > >>> I also think it would be a better solution if, instead of having async > > > >>> to do in-place updates in the current state, the async path should be > > > >>> equivalent to a syncronous update, i.e., modifying new_state and > > > >>> performing a flip > > > >>> IMHO, the only difference between sync and async should be that async update > > > >>> doesn't wait for vblank and applies the changes immeditally to the hw, > > > >>> but the code path could be almost the same. > > > >>> But for now I think this solution is ok (swaping new_fb/old_fb), and > > > >>> then we can adjust things little by little, what do you think? > > > >>> > > > >>> Thanks! > > > >>> Helen > > > >>> > > > >>> drivers/gpu/drm/drm_atomic_helper.c | 20 ++++++++++---------- > > > >>> 1 file changed, 10 insertions(+), 10 deletions(-) > > > >>> > > > >>> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c > > > >>> index 540a77a2ade9..e7eb96f1efc2 100644 > > > >>> --- a/drivers/gpu/drm/drm_atomic_helper.c > > > >>> +++ b/drivers/gpu/drm/drm_atomic_helper.c > > > >>> @@ -1608,15 +1608,6 @@ int drm_atomic_helper_async_check(struct drm_device *dev, > > > >>> old_plane_state->crtc != new_plane_state->crtc) > > > >>> return -EINVAL; > > > >>> > > > >>> - /* > > > >>> - * FIXME: Since prepare_fb and cleanup_fb are always called on > > > >>> - * the new_plane_state for async updates we need to block framebuffer > > > >>> - * changes. This prevents use of a fb that's been cleaned up and > > > >>> - * double cleanups from occuring. > > > >>> - */ > > > >>> - if (old_plane_state->fb != new_plane_state->fb) > > > >>> - return -EINVAL; > > > >>> - > > > >>> funcs = plane->helper_private; > > > >>> if (!funcs->atomic_async_update) > > > >>> return -EINVAL; > > > >>> @@ -1657,6 +1648,9 @@ void drm_atomic_helper_async_commit(struct drm_device *dev, > > > >>> int i; > > > >>> > > > >>> for_each_new_plane_in_state(state, plane, plane_state, i) { > > > >>> + struct drm_framebuffer *new_fb = plane_state->fb; > > > >>> + struct drm_framebuffer *old_fb = plane->state->fb; > > > >>> + > > > >>> funcs = plane->helper_private; > > > >>> funcs->atomic_async_update(plane, plane_state); > > > >>> > > > >>> @@ -1665,11 +1659,17 @@ void drm_atomic_helper_async_commit(struct drm_device *dev, > > > >>> * plane->state in-place, make sure at least common > > > >>> * properties have been properly updated. > > > >>> */ > > > >>> - WARN_ON_ONCE(plane->state->fb != plane_state->fb); > > > >>> + WARN_ON_ONCE(plane->state->fb != new_fb); > > > >>> WARN_ON_ONCE(plane->state->crtc_x != plane_state->crtc_x); > > > >>> WARN_ON_ONCE(plane->state->crtc_y != plane_state->crtc_y); > > > >>> WARN_ON_ONCE(plane->state->src_x != plane_state->src_x); > > > >>> WARN_ON_ONCE(plane->state->src_y != plane_state->src_y); > > > >>> + > > > >>> + /* > > > >>> + * Make sure the FBs have been swapped so that cleanups in the > > > >>> + * new_state performs a cleanup in the old FB. > > > >>> + */ > > > >>> + WARN_ON_ONCE(plane_state->fb != old_fb); > > > >> > > > >> I personally think this approach is fine and the WARN_ON s are good for > > > >> catching drivers that want to use these in the future. > > > > > > > > Well, I agree this change is the way to go for a short-term solution > > > > to relax the old_fb == new_fb constraint, but I keep thinking this whole > > > > "update plane_state in place" is a recipe for trouble and just make > > > > things more complicated for drivers for no obvious reasons. Look at the > > > > VC4 implem [1] if you need a proof that things can get messy pretty > > > > quickly. > > > > > > > > All this state-fields-copying steps could be skipped if the core was > > > > simply swapping the old/new states as is done in the sync update path. > > > > > > > > [1]https://elixir.bootlin.com/linux/v5.0-rc7/source/drivers/gpu/drm/vc4/vc4_plane.c#L878 > > > > > > I completely agree with this view FWIW. I had a discussion with Daniel > > > about this when I had posted the original block FB changes patch. > > > > > > - The plane object needs to be locked in order for async state to be updated > > > - Blocking commit work holds the lock for the plane, async update won't > > > happen > > > - Non-blocking commit work that's still ongoing won't have hw_done > > > signaled and drm_atomic_helper_async_check will block the async update > > > > > > So this looks safe in theory, with the exception of the call to > > > drm_atomic_helper_cleanup_planes occuring after hw_done is signaled. > > > > Isn't it also the case in the sync update path? > > > > > > > > I believe that the behavior of this function still remains the same even > > > if plane->state is swapped to something else during the call (since > > > old_plane_state should never be equal to plane->state if the commit > > > succeeded and the plane is in the commit), but I'm not sure that's > > > something we'd want to rely on. > > > > > > I think other than that issue, you could probably just: > > > > > > drm_atomic_helper_prepare_planes(...); > > > drm_atomic_helper_swap_state(...); > > > drm_atomic_state_get(state); > > > > Why do we need a state_get() here? AFAICT, it's done this way in the > > sync update path because of the non-blocking semantic where the state > > might be released by the caller before it's been applied by the commit > > worker. > > > > > drm_atomic_helper_async_commit(...); > > > drm_atomic_helper_cleanup_planes(dev, state); > > > > > > and it would work as expected. But there still may be other things I'm > > > missing or haven't considered here. > > > > Actually, when I said we could swap states, I was not necessarily > > thinking about re-using drm_atomic_helper_swap_state(), but instead > > swap states directly in drm_atomic_helper_async_commit(): > > > > for_each_oldnew_plane_in_state(state, plane, old_plane_state, > > new_plane_state, i) { > > WARN_ON(plane->state != old_plane_state); > > old_plane_state->state = state; > > new_plane_state->state = NULL; > > state->planes[i].state = old_plane_state; > > plane->state = new_plane_state; > > > > funcs = plane->helper_private; > > funcs->atomic_async_update(plane, new_plane_state); > > } > > > > This way we would avoid the WARN_ON() lines we have in > > drm_atomic_helper_async_commit() to check that things have been > > properly updated in-place, and we would also get rid of the driver > > code copying the plane_state property that can change during an async > > update. > > > > But, as you said, I might be missing other potential issues. > > Ok I dug around again, and I think I reconstructed the problem again. Great! > > The issue is the lifetimes of state structs. The nonblocking commit worker > doesn't hold a reference onto the new states at all. The only reason those > new states cannot disappear is that the next atomic comit touching the > same states waits for crtc_commit.hw_done before it pushes its own update > through (and then goes and releases those state structures). By disappear I guess you mean when it's replaced in plane->state by a subsequent atomic commit that places them in the old_state slot and release them as part of the drm_atomic_state_put() call when returning from a non-blocking atomic update. Any reason we couldn't retain new_state refs until we're done manipulating them to overcome this problem? > > The old state has no such issue, since each commit takes ownership of the > old state and then releases it. And can do that any time after hw_done. I'd expect the wait on hw_done to be needed anyway for async commits going after sync ones. As the comment says, if we don't wait for hw_done, the async update settings might be overridden by the sync update ones. > > Now with the current async code that's no issue, because we do check for > hw_done. The trouble is that hw_done is a kernel-internal implementation > detail. The only think userspace can observe is flip_done, and that's > what's used for -EBUSY for normal page-flips. For cursor this kinda > doesn't matter, because these two should be fairly close together (in most > cases hw_done even happens before flip_done, but that depends upon the > driver). So the occasional silent fallback to a synchronous commit doesn't > really matter. > > What we could do is just wait for hw_done for async commits, but that's > kinda not cool either since it blocks (again cursor is ill-defined enough > that it doesn't matter). And pushing async updates to a worker means we > need to greatly extend the crtc_commit tracking (at least to each plane > state). I think most of that exist now, since we had to add it anyway for > planes which can be reassigned between crtc. To be honest, I don't know what the semantic of async commit should be. Does async (update things between 2 VBLANKS at the risk of causing tearing) necessarily implies non-blocking (return before the update is actually pushed to the HW)? > > tldr; maybe we can do the full swapping now? > > I agree it feels like the cleaner solution, but definitely need a pile of > igt tests to make sure we can mix&match between async and sync commits and > nothing blows up. And sync commits need to use reassignment of planes to > different crtcs plus nonblocking commit (I think amd hw can do all that, > or at least I've seen prep patches). Yes, that'd be great to have that in place, especially if we want to expose async atomic commits to userspace (right now it's only used for legacy cursor updates).