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: Mon, 11 Mar 2019 15:20:09 +0100 Message-ID: <20190311152009.7c55b797@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> 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 [46.235.227.227]) by gabe.freedesktop.org (Postfix) with ESMTPS id 9363D89728 for ; Mon, 11 Mar 2019 14:20:14 +0000 (UTC) In-Reply-To: <01f2b3ba-434a-f61f-e8e8-85f3c9107a5c@amd.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: "Kazlauskas, Nicholas" Cc: =?UTF-8?B?U3TDqXBoYW5l?= Marchesin , Sean Paul , David Airlie , "daniel.vetter@ffwll.ch" , "linux-kernel@vger.kernel.org" , "dri-devel@lists.freedesktop.org" , Tomasz Figa , Helen Koike , "kernel@collabora.com" List-Id: dri-devel@lists.freedesktop.org T24gTW9uLCAxMSBNYXIgMjAxOSAxMzoxNToyMyArMDAwMAoiS2F6bGF1c2thcywgTmljaG9sYXMi IDxOaWNob2xhcy5LYXpsYXVza2FzQGFtZC5jb20+IHdyb3RlOgoKPiBPbiAzLzExLzE5IDY6MDYg QU0sIEJvcmlzIEJyZXppbGxvbiB3cm90ZToKPiA+IEhlbGxvIE5pY2hvbGFzLAo+ID4gCj4gPiBP biBNb24sIDQgTWFyIDIwMTkgMTU6NDY6NDkgKzAwMDAKPiA+ICJLYXpsYXVza2FzLCBOaWNob2xh cyIgPE5pY2hvbGFzLkthemxhdXNrYXNAYW1kLmNvbT4gd3JvdGU6Cj4gPiAgIAo+ID4+IE9uIDMv NC8xOSA5OjQ5IEFNLCBIZWxlbiBLb2lrZSB3cm90ZTogIAo+ID4+PiBJbiB0aGUgY2FzZSBvZiBh IG5vcm1hbCBzeW5jIHVwZGF0ZSwgdGhlIHByZXBhcmF0aW9uIG9mIGZyYW1lYnVmZmVycyAoYmUK PiA+Pj4gaXQgY2FsbGluZyBkcm1fYXRvbWljX2hlbHBlcl9wcmVwYXJlX3BsYW5lcygpIG9yIGRv aW5nIHNldHVwcyB3aXRoCj4gPj4+IGRybV9mcmFtZWJ1ZmZlcl9nZXQoKSkgYXJlIHBlcmZvcm1l ZCBpbiB0aGUgbmV3X3N0YXRlIGFuZCB0aGUgcmVzcGVjdGl2ZQo+ID4+PiBjbGVhbnVwcyBhcmUg cGVyZm9ybWVkIGluIHRoZSBvbGRfc3RhdGUuCj4gPj4+Cj4gPj4+IEluIHRoZSBjYXNlIG9mIGFz eW5jIHVwZGF0ZXMsIHRoZSBwcmVwYXJhdGlvbiBpcyBhbHNvIGRvbmUgaW4gdGhlCj4gPj4+IG5l d19zdGF0ZSBidXQgdGhlIGNsZWFudXBzIGFyZSBkb25lIGluIHRoZSBuZXdfc3RhdGUgKGJlY2F1 c2UgdXBkYXRlcwo+ID4+PiBhcmUgcGVyZm9ybWVkIGluIHBsYWNlLCBpLmUuIGluIHRoZSBjdXJy ZW50IHN0YXRlKS4KPiA+Pj4KPiA+Pj4gVGhlIGN1cnJlbnQgY29kZSBibG9ja3MgYXN5bmMgdWRw YXRlcyB3aGVuIHRoZSBmYiBpcyBjaGFuZ2VkLCB0dXJuaW5nCj4gPj4+IGFzeW5jIHVwZGF0ZXMg aW50byBzeW5jIHVwZGF0ZXMsIHNsb3dpbmcgZG93biBjdXJzb3IgdXBkYXRlcyBhbmQKPiA+Pj4g aW50cm9kdWNpbmcgcmVncmVzc2lvbnMgaW4gaWd0IHRlc3RzIHdpdGggZXJyb3JzIG9mIHR5cGU6 Cj4gPj4+Cj4gPj4+ICJDUklUSUNBTDogY29tcGxldGVkIDk3IGN1cnNvciB1cGRhdGVkIGluIGEg cGVyaW9kIG9mIDMwIGZsaXBzLCB3ZQo+ID4+PiBleHBlY3QgdG8gY29tcGxldGUgYXBwcm94aW1h dGVseSAxNTM2MCB1cGRhdGVzLCB3aXRoIHRoZSB0aHJlc2hvbGQgc2V0Cj4gPj4+IGF0IDc2ODAi Cj4gPj4+Cj4gPj4+IEZiIGNoYW5nZXMgaW4gYXN5bmMgdXBkYXRlcyB3ZXJlIHByZXZlbnRlZCB0 byBhdm9pZCB0aGUgZm9sbG93aW5nIHNjZW5hcmlvOgo+ID4+Pgo+ID4+PiAtIEFzeW5jIHVwZGF0 ZSwgb2xkZmIgPSBOVUxMLCBuZXdmYiA9IGZiMSwgcHJlcGFyZSBmYjEsIGNsZWFudXAgZmIxCj4g Pj4+IC0gQXN5bmMgdXBkYXRlLCBvbGRmYiA9IGZiMSwgbmV3ZmIgPSBmYjIsIHByZXBhcmUgZmIy LCBjbGVhbnVwIGZiMgo+ID4+PiAtIE5vbi1hc3luYyBjb21taXQsIG9sZGZiID0gZmIyLCBuZXdm YiA9IGZiMSwgcHJlcGFyZSBmYjEsIGNsZWFudXAgZmIyICh3cm9uZykKPiA+Pj4gV2hlcmUgd2Ug aGF2ZSBhIHNpbmdsZSBjYWxsIHRvIHByZXBhcmUgZmIyIGJ1dCBkb3VibGUgY2xlYW51cCBjYWxs IHRvIGZiMi4KPiA+Pj4KPiA+Pj4gVG8gc29sdmUgdGhlIGFib3ZlIHByb2JsZW1zLCBpbnN0ZWFk IG9mIGJsb2NraW5nIGFzeW5jIGZiIGNoYW5nZXMsIHdlCj4gPj4+IHBsYWNlIHRoZSBvbGQgZnJh bWVidWZmZXIgaW4gdGhlIG5ld19zdGF0ZSBvYmplY3QsIHNvIHdoZW4gdGhlIGNvZGUKPiA+Pj4g cGVyZm9ybXMgY2xlYW51cHMgaW4gdGhlIG5ld19zdGF0ZSBpdCB3aWxsIGNsZWFudXAgdGhlIG9s ZF9mYiBhbmQgd2UKPiA+Pj4gd2lsbCBoYXZlIHRoZSBmb2xsb3dpbmcgc2NlbmFyaW8gaW5zdGVh ZDoKPiA+Pj4KPiA+Pj4gLSBBc3luYyB1cGRhdGUsIG9sZGZiID0gTlVMTCwgbmV3ZmIgPSBmYjEs IHByZXBhcmUgZmIxLCBubyBjbGVhbnVwCj4gPj4+IC0gQXN5bmMgdXBkYXRlLCBvbGRmYiA9IGZi MSwgbmV3ZmIgPSBmYjIsIHByZXBhcmUgZmIyLCBjbGVhbnVwIGZiMQo+ID4+PiAtIE5vbi1hc3lu YyBjb21taXQsIG9sZGZiID0gZmIyLCBuZXdmYiA9IGZiMSwgcHJlcGFyZSBmYjEsIGNsZWFudXAg ZmIyCj4gPj4+Cj4gPj4+IFdoZXJlIGNhbGxzIHRvIHByZXBhcmUvY2xlYW51cCBhcmUgYmFsbGFu Y2VkLgo+ID4+Pgo+ID4+PiBDYzogPHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmc+ICMgdjQuMTQrOiAy NWRjMTk0YjM0ZGQ6IGRybTogQmxvY2sgZmIgY2hhbmdlcyBmb3IgYXN5bmMgcGxhbmUgdXBkYXRl cwo+ID4+PiBGaXhlczogMjVkYzE5NGIzNGRkICgiZHJtOiBCbG9jayBmYiBjaGFuZ2VzIGZvciBh c3luYyBwbGFuZSB1cGRhdGVzIikKPiA+Pj4gU3VnZ2VzdGVkLWJ5OiBCb3JpcyBCcmV6aWxsb24g PGJvcmlzLmJyZXppbGxvbkBjb2xsYWJvcmEuY29tPgo+ID4+PiBTaWduZWQtb2ZmLWJ5OiBIZWxl biBLb2lrZSA8aGVsZW4ua29pa2VAY29sbGFib3JhLmNvbT4KPiA+Pj4KPiA+Pj4gLS0tCj4gPj4+ IEhlbGxvLAo+ID4+Pgo+ID4+PiBBcyBtZW50aW9uZWQgaW4gdGhlIGNvdmVyIGxldHRlciwKPiA+ Pj4gSSB0ZXN0ZWQgb24gdGhlIHJvY2tjaGlwIGFuZCBvbiBpOTE1ICh3aXRoIGEgcGF0Y2ggSSBh bSBzdGlsbCB3b3JraW5nIG9uIGZvcgo+ID4+PiByZXBsYWNpbmcgY3Vyc29ycyBieSBhc3luYyB1 cGRhdGUpLCB3aXRoIGlndCBwbGFuZV9jdXJzb3JfbGVnYWN5IGFuZAo+ID4+PiBrbXNfY3Vyc29y X2xlZ2FjeSBhbmQgSSBkaWRuJ3Qgc2VlIGFueSByZWdyZXNzaW9ucy4KPiA+Pj4gSSBjb3VsZG4n dCB0ZXN0IG9uIE1TTSBhbmQgQU1EIGJlY2F1c2UgSSBkb24ndCBoYXZlIHRoZSBoYXJkd2FyZSAo YW5kIEkgYW0KPiA+Pj4gaGF2aW5nIHNvbWUgaXNzdWVzIHRlc3Rpbmcgb24gdmM0KSBhbmQgSSB3 b3VsZCBhcHByZWNpYXRlIGlmIGFueW9uZSBjb3VsZCBoZWxwCj4gPj4+IG1lIHRlc3RpbmcgdGhv c2UuCj4gPj4+Cj4gPj4+IEkgYWxzbyB0aGluayBpdCB3b3VsZCBiZSBhIGJldHRlciBzb2x1dGlv biBpZiwgaW5zdGVhZCBvZiBoYXZpbmcgYXN5bmMKPiA+Pj4gdG8gZG8gaW4tcGxhY2UgdXBkYXRl cyBpbiB0aGUgY3VycmVudCBzdGF0ZSwgdGhlIGFzeW5jIHBhdGggc2hvdWxkIGJlCj4gPj4+IGVx dWl2YWxlbnQgdG8gYSBzeW5jcm9ub3VzIHVwZGF0ZSwgaS5lLiwgbW9kaWZ5aW5nIG5ld19zdGF0 ZSBhbmQKPiA+Pj4gcGVyZm9ybWluZyBhIGZsaXAKPiA+Pj4gSU1ITywgdGhlIG9ubHkgZGlmZmVy ZW5jZSBiZXR3ZWVuIHN5bmMgYW5kIGFzeW5jIHNob3VsZCBiZSB0aGF0IGFzeW5jIHVwZGF0ZQo+ ID4+PiBkb2Vzbid0IHdhaXQgZm9yIHZibGFuayBhbmQgYXBwbGllcyB0aGUgY2hhbmdlcyBpbW1l ZGl0YWxseSB0byB0aGUgaHcsCj4gPj4+IGJ1dCB0aGUgY29kZSBwYXRoIGNvdWxkIGJlIGFsbW9z dCB0aGUgc2FtZS4KPiA+Pj4gQnV0IGZvciBub3cgSSB0aGluayB0aGlzIHNvbHV0aW9uIGlzIG9r IChzd2FwaW5nIG5ld19mYi9vbGRfZmIpLCBhbmQKPiA+Pj4gdGhlbiB3ZSBjYW4gYWRqdXN0IHRo aW5ncyBsaXR0bGUgYnkgbGl0dGxlLCB3aGF0IGRvIHlvdSB0aGluaz8KPiA+Pj4KPiA+Pj4gVGhh bmtzIQo+ID4+PiBIZWxlbgo+ID4+Pgo+ID4+PiAgICBkcml2ZXJzL2dwdS9kcm0vZHJtX2F0b21p Y19oZWxwZXIuYyB8IDIwICsrKysrKysrKystLS0tLS0tLS0tCj4gPj4+ICAgIDEgZmlsZSBjaGFu Z2VkLCAxMCBpbnNlcnRpb25zKCspLCAxMCBkZWxldGlvbnMoLSkKPiA+Pj4KPiA+Pj4gZGlmZiAt LWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9kcm1fYXRvbWljX2hlbHBlci5jIGIvZHJpdmVycy9ncHUv ZHJtL2RybV9hdG9taWNfaGVscGVyLmMKPiA+Pj4gaW5kZXggNTQwYTc3YTJhZGU5Li5lN2ViOTZm MWVmYzIgMTAwNjQ0Cj4gPj4+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9kcm1fYXRvbWljX2hlbHBl ci5jCj4gPj4+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9kcm1fYXRvbWljX2hlbHBlci5jCj4gPj4+ IEBAIC0xNjA4LDE1ICsxNjA4LDYgQEAgaW50IGRybV9hdG9taWNfaGVscGVyX2FzeW5jX2NoZWNr KHN0cnVjdCBkcm1fZGV2aWNlICpkZXYsCj4gPj4+ICAgIAkgICAgb2xkX3BsYW5lX3N0YXRlLT5j cnRjICE9IG5ld19wbGFuZV9zdGF0ZS0+Y3J0YykKPiA+Pj4gICAgCQlyZXR1cm4gLUVJTlZBTDsK PiA+Pj4gICAgCj4gPj4+IC0JLyoKPiA+Pj4gLQkgKiBGSVhNRTogU2luY2UgcHJlcGFyZV9mYiBh bmQgY2xlYW51cF9mYiBhcmUgYWx3YXlzIGNhbGxlZCBvbgo+ID4+PiAtCSAqIHRoZSBuZXdfcGxh bmVfc3RhdGUgZm9yIGFzeW5jIHVwZGF0ZXMgd2UgbmVlZCB0byBibG9jayBmcmFtZWJ1ZmZlcgo+ ID4+PiAtCSAqIGNoYW5nZXMuIFRoaXMgcHJldmVudHMgdXNlIG9mIGEgZmIgdGhhdCdzIGJlZW4g Y2xlYW5lZCB1cCBhbmQKPiA+Pj4gLQkgKiBkb3VibGUgY2xlYW51cHMgZnJvbSBvY2N1cmluZy4K PiA+Pj4gLQkgKi8KPiA+Pj4gLQlpZiAob2xkX3BsYW5lX3N0YXRlLT5mYiAhPSBuZXdfcGxhbmVf c3RhdGUtPmZiKQo+ID4+PiAtCQlyZXR1cm4gLUVJTlZBTDsKPiA+Pj4gLQo+ID4+PiAgICAJZnVu Y3MgPSBwbGFuZS0+aGVscGVyX3ByaXZhdGU7Cj4gPj4+ICAgIAlpZiAoIWZ1bmNzLT5hdG9taWNf YXN5bmNfdXBkYXRlKQo+ID4+PiAgICAJCXJldHVybiAtRUlOVkFMOwo+ID4+PiBAQCAtMTY1Nyw2 ICsxNjQ4LDkgQEAgdm9pZCBkcm1fYXRvbWljX2hlbHBlcl9hc3luY19jb21taXQoc3RydWN0IGRy bV9kZXZpY2UgKmRldiwKPiA+Pj4gICAgCWludCBpOwo+ID4+PiAgICAKPiA+Pj4gICAgCWZvcl9l YWNoX25ld19wbGFuZV9pbl9zdGF0ZShzdGF0ZSwgcGxhbmUsIHBsYW5lX3N0YXRlLCBpKSB7Cj4g Pj4+ICsJCXN0cnVjdCBkcm1fZnJhbWVidWZmZXIgKm5ld19mYiA9IHBsYW5lX3N0YXRlLT5mYjsK PiA+Pj4gKwkJc3RydWN0IGRybV9mcmFtZWJ1ZmZlciAqb2xkX2ZiID0gcGxhbmUtPnN0YXRlLT5m YjsKPiA+Pj4gKwo+ID4+PiAgICAJCWZ1bmNzID0gcGxhbmUtPmhlbHBlcl9wcml2YXRlOwo+ID4+ PiAgICAJCWZ1bmNzLT5hdG9taWNfYXN5bmNfdXBkYXRlKHBsYW5lLCBwbGFuZV9zdGF0ZSk7Cj4g Pj4+ICAgIAo+ID4+PiBAQCAtMTY2NSwxMSArMTY1OSwxNyBAQCB2b2lkIGRybV9hdG9taWNfaGVs cGVyX2FzeW5jX2NvbW1pdChzdHJ1Y3QgZHJtX2RldmljZSAqZGV2LAo+ID4+PiAgICAJCSAqIHBs YW5lLT5zdGF0ZSBpbi1wbGFjZSwgbWFrZSBzdXJlIGF0IGxlYXN0IGNvbW1vbgo+ID4+PiAgICAJ CSAqIHByb3BlcnRpZXMgaGF2ZSBiZWVuIHByb3Blcmx5IHVwZGF0ZWQuCj4gPj4+ICAgIAkJICov Cj4gPj4+IC0JCVdBUk5fT05fT05DRShwbGFuZS0+c3RhdGUtPmZiICE9IHBsYW5lX3N0YXRlLT5m Yik7Cj4gPj4+ICsJCVdBUk5fT05fT05DRShwbGFuZS0+c3RhdGUtPmZiICE9IG5ld19mYik7Cj4g Pj4+ICAgIAkJV0FSTl9PTl9PTkNFKHBsYW5lLT5zdGF0ZS0+Y3J0Y194ICE9IHBsYW5lX3N0YXRl LT5jcnRjX3gpOwo+ID4+PiAgICAJCVdBUk5fT05fT05DRShwbGFuZS0+c3RhdGUtPmNydGNfeSAh PSBwbGFuZV9zdGF0ZS0+Y3J0Y195KTsKPiA+Pj4gICAgCQlXQVJOX09OX09OQ0UocGxhbmUtPnN0 YXRlLT5zcmNfeCAhPSBwbGFuZV9zdGF0ZS0+c3JjX3gpOwo+ID4+PiAgICAJCVdBUk5fT05fT05D RShwbGFuZS0+c3RhdGUtPnNyY195ICE9IHBsYW5lX3N0YXRlLT5zcmNfeSk7Cj4gPj4+ICsKPiA+ Pj4gKwkJLyoKPiA+Pj4gKwkJICogTWFrZSBzdXJlIHRoZSBGQnMgaGF2ZSBiZWVuIHN3YXBwZWQg c28gdGhhdCBjbGVhbnVwcyBpbiB0aGUKPiA+Pj4gKwkJICogbmV3X3N0YXRlIHBlcmZvcm1zIGEg Y2xlYW51cCBpbiB0aGUgb2xkIEZCLgo+ID4+PiArCQkgKi8KPiA+Pj4gKwkJV0FSTl9PTl9PTkNF KHBsYW5lX3N0YXRlLT5mYiAhPSBvbGRfZmIpOyAgCj4gPj4KPiA+PiBJIHBlcnNvbmFsbHkgdGhp bmsgdGhpcyBhcHByb2FjaCBpcyBmaW5lIGFuZCB0aGUgV0FSTl9PTiBzIGFyZSBnb29kIGZvcgo+ ID4+IGNhdGNoaW5nIGRyaXZlcnMgdGhhdCB3YW50IHRvIHVzZSB0aGVzZSBpbiB0aGUgZnV0dXJl LiAgCj4gPiAKPiA+IFdlbGwsIEkgYWdyZWUgdGhpcyBjaGFuZ2UgaXMgdGhlIHdheSB0byBnbyBm b3IgYSBzaG9ydC10ZXJtIHNvbHV0aW9uCj4gPiB0byByZWxheCB0aGUgb2xkX2ZiID09IG5ld19m YiBjb25zdHJhaW50LCBidXQgSSBrZWVwIHRoaW5raW5nIHRoaXMgd2hvbGUKPiA+ICJ1cGRhdGUg cGxhbmVfc3RhdGUgaW4gcGxhY2UiIGlzIGEgcmVjaXBlIGZvciB0cm91YmxlIGFuZCBqdXN0IG1h a2UKPiA+IHRoaW5ncyBtb3JlIGNvbXBsaWNhdGVkIGZvciBkcml2ZXJzIGZvciBubyBvYnZpb3Vz IHJlYXNvbnMuIExvb2sgYXQgdGhlCj4gPiBWQzQgaW1wbGVtIFsxXSBpZiB5b3UgbmVlZCBhIHBy b29mIHRoYXQgdGhpbmdzIGNhbiBnZXQgbWVzc3kgcHJldHR5Cj4gPiBxdWlja2x5Lgo+ID4gCj4g PiBBbGwgdGhpcyBzdGF0ZS1maWVsZHMtY29weWluZyBzdGVwcyBjb3VsZCBiZSBza2lwcGVkIGlm IHRoZSBjb3JlIHdhcwo+ID4gc2ltcGx5IHN3YXBwaW5nIHRoZSBvbGQvbmV3IHN0YXRlcyBhcyBp cyBkb25lIGluIHRoZSBzeW5jIHVwZGF0ZSBwYXRoLgo+ID4gCj4gPiBbMV1odHRwczovL2VsaXhp ci5ib290bGluLmNvbS9saW51eC92NS4wLXJjNy9zb3VyY2UvZHJpdmVycy9ncHUvZHJtL3ZjNC92 YzRfcGxhbmUuYyNMODc4ICAKPiAKPiBJIGNvbXBsZXRlbHkgYWdyZWUgd2l0aCB0aGlzIHZpZXcg RldJVy4gSSBoYWQgYSBkaXNjdXNzaW9uIHdpdGggRGFuaWVsIAo+IGFib3V0IHRoaXMgd2hlbiBJ IGhhZCBwb3N0ZWQgdGhlIG9yaWdpbmFsIGJsb2NrIEZCIGNoYW5nZXMgcGF0Y2guCj4gCj4gLSBU aGUgcGxhbmUgb2JqZWN0IG5lZWRzIHRvIGJlIGxvY2tlZCBpbiBvcmRlciBmb3IgYXN5bmMgc3Rh dGUgdG8gYmUgdXBkYXRlZAo+IC0gQmxvY2tpbmcgY29tbWl0IHdvcmsgaG9sZHMgdGhlIGxvY2sg Zm9yIHRoZSBwbGFuZSwgYXN5bmMgdXBkYXRlIHdvbid0IAo+IGhhcHBlbgo+IC0gTm9uLWJsb2Nr aW5nIGNvbW1pdCB3b3JrIHRoYXQncyBzdGlsbCBvbmdvaW5nIHdvbid0IGhhdmUgaHdfZG9uZSAK PiBzaWduYWxlZCBhbmQgZHJtX2F0b21pY19oZWxwZXJfYXN5bmNfY2hlY2sgd2lsbCBibG9jayB0 aGUgYXN5bmMgdXBkYXRlCj4gCj4gU28gdGhpcyBsb29rcyBzYWZlIGluIHRoZW9yeSwgd2l0aCB0 aGUgZXhjZXB0aW9uIG9mIHRoZSBjYWxsIHRvIAo+IGRybV9hdG9taWNfaGVscGVyX2NsZWFudXBf cGxhbmVzIG9jY3VyaW5nIGFmdGVyIGh3X2RvbmUgaXMgc2lnbmFsZWQuCgpJc24ndCBpdCBhbHNv IHRoZSBjYXNlIGluIHRoZSBzeW5jIHVwZGF0ZSBwYXRoPwoKPiAKPiBJIGJlbGlldmUgdGhhdCB0 aGUgYmVoYXZpb3Igb2YgdGhpcyBmdW5jdGlvbiBzdGlsbCByZW1haW5zIHRoZSBzYW1lIGV2ZW4g Cj4gaWYgcGxhbmUtPnN0YXRlIGlzIHN3YXBwZWQgdG8gc29tZXRoaW5nIGVsc2UgZHVyaW5nIHRo ZSBjYWxsIChzaW5jZSAKPiBvbGRfcGxhbmVfc3RhdGUgc2hvdWxkIG5ldmVyIGJlIGVxdWFsIHRv IHBsYW5lLT5zdGF0ZSBpZiB0aGUgY29tbWl0IAo+IHN1Y2NlZWRlZCBhbmQgdGhlIHBsYW5lIGlz IGluIHRoZSBjb21taXQpLCBidXQgSSdtIG5vdCBzdXJlIHRoYXQncyAKPiBzb21ldGhpbmcgd2Un ZCB3YW50IHRvIHJlbHkgb24uCj4gCj4gSSB0aGluayBvdGhlciB0aGFuIHRoYXQgaXNzdWUsIHlv dSBjb3VsZCBwcm9iYWJseSBqdXN0Ogo+IAo+IGRybV9hdG9taWNfaGVscGVyX3ByZXBhcmVfcGxh bmVzKC4uLik7Cj4gZHJtX2F0b21pY19oZWxwZXJfc3dhcF9zdGF0ZSguLi4pOwo+IGRybV9hdG9t aWNfc3RhdGVfZ2V0KHN0YXRlKTsKCldoeSBkbyB3ZSBuZWVkIGEgc3RhdGVfZ2V0KCkgaGVyZT8g QUZBSUNULCBpdCdzIGRvbmUgdGhpcyB3YXkgaW4gdGhlCnN5bmMgdXBkYXRlIHBhdGggYmVjYXVz ZSBvZiB0aGUgbm9uLWJsb2NraW5nIHNlbWFudGljIHdoZXJlIHRoZSBzdGF0ZQptaWdodCBiZSBy ZWxlYXNlZCBieSB0aGUgY2FsbGVyIGJlZm9yZSBpdCdzIGJlZW4gYXBwbGllZCBieSB0aGUgY29t bWl0Cndvcmtlci4KCj4gZHJtX2F0b21pY19oZWxwZXJfYXN5bmNfY29tbWl0KC4uLik7Cj4gZHJt X2F0b21pY19oZWxwZXJfY2xlYW51cF9wbGFuZXMoZGV2LCBzdGF0ZSk7Cj4gCj4gYW5kIGl0IHdv dWxkIHdvcmsgYXMgZXhwZWN0ZWQuIEJ1dCB0aGVyZSBzdGlsbCBtYXkgYmUgb3RoZXIgdGhpbmdz IEknbSAKPiBtaXNzaW5nIG9yIGhhdmVuJ3QgY29uc2lkZXJlZCBoZXJlLgoKQWN0dWFsbHksIHdo ZW4gSSBzYWlkIHdlIGNvdWxkIHN3YXAgc3RhdGVzLCBJIHdhcyBub3QgbmVjZXNzYXJpbHkKdGhp bmtpbmcgYWJvdXQgcmUtdXNpbmcgZHJtX2F0b21pY19oZWxwZXJfc3dhcF9zdGF0ZSgpLCBidXQg aW5zdGVhZApzd2FwIHN0YXRlcyBkaXJlY3RseSBpbiBkcm1fYXRvbWljX2hlbHBlcl9hc3luY19j b21taXQoKToKCglmb3JfZWFjaF9vbGRuZXdfcGxhbmVfaW5fc3RhdGUoc3RhdGUsIHBsYW5lLCBv bGRfcGxhbmVfc3RhdGUsCgkJCQkgICAgICAgbmV3X3BsYW5lX3N0YXRlLCBpKSB7CgkJV0FSTl9P TihwbGFuZS0+c3RhdGUgIT0gb2xkX3BsYW5lX3N0YXRlKTsKCQlvbGRfcGxhbmVfc3RhdGUtPnN0 YXRlID0gc3RhdGU7CgkJbmV3X3BsYW5lX3N0YXRlLT5zdGF0ZSA9IE5VTEw7CgkJc3RhdGUtPnBs YW5lc1tpXS5zdGF0ZSA9IG9sZF9wbGFuZV9zdGF0ZTsKCQlwbGFuZS0+c3RhdGUgPSBuZXdfcGxh bmVfc3RhdGU7CgoJCWZ1bmNzID0gcGxhbmUtPmhlbHBlcl9wcml2YXRlOwoJCWZ1bmNzLT5hdG9t aWNfYXN5bmNfdXBkYXRlKHBsYW5lLCBuZXdfcGxhbmVfc3RhdGUpOwoJfQoKVGhpcyB3YXkgd2Ug d291bGQgYXZvaWQgdGhlIFdBUk5fT04oKSBsaW5lcyB3ZSBoYXZlIGluCmRybV9hdG9taWNfaGVs cGVyX2FzeW5jX2NvbW1pdCgpIHRvIGNoZWNrIHRoYXQgdGhpbmdzIGhhdmUgYmVlbgpwcm9wZXJs eSB1cGRhdGVkIGluLXBsYWNlLCBhbmQgd2Ugd291bGQgYWxzbyBnZXQgcmlkIG9mIHRoZSBkcml2 ZXIKY29kZSBjb3B5aW5nIHRoZSBwbGFuZV9zdGF0ZSBwcm9wZXJ0eSB0aGF0IGNhbiBjaGFuZ2Ug ZHVyaW5nIGFuIGFzeW5jCnVwZGF0ZS4KCkJ1dCwgYXMgeW91IHNhaWQsIEkgbWlnaHQgYmUgbWlz c2luZyBvdGhlciBwb3RlbnRpYWwgaXNzdWVzLgpfX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0 cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9s aXN0aW5mby9kcmktZGV2ZWw= 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 76541C43381 for ; Mon, 11 Mar 2019 14:20:16 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 432C820857 for ; Mon, 11 Mar 2019 14:20:16 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727593AbfCKOUP (ORCPT ); Mon, 11 Mar 2019 10:20:15 -0400 Received: from bhuna.collabora.co.uk ([46.235.227.227]:54624 "EHLO bhuna.collabora.co.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727219AbfCKOUO (ORCPT ); Mon, 11 Mar 2019 10:20:14 -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 3EFC427D615; Mon, 11 Mar 2019 14:20:12 +0000 (GMT) Date: Mon, 11 Mar 2019 15:20:09 +0100 From: Boris Brezillon To: "Kazlauskas, Nicholas" Cc: Helen Koike , "dri-devel@lists.freedesktop.org" , "Grodzovsky, Andrey" , "daniel.vetter@ffwll.ch" , "linux-kernel@vger.kernel.org" , Tomasz Figa , David Airlie , Sean Paul , "kernel@collabora.com" , "Wentland, Harry" , =?UTF-8?B?U3TDqXBoYW5l?= Marchesin , Eric Anholt Subject: Re: [PATCH 1/5] drm: don't block fb changes for async plane updates Message-ID: <20190311152009.7c55b797@collabora.com> In-Reply-To: <01f2b3ba-434a-f61f-e8e8-85f3c9107a5c@amd.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> 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 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.