From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mark yao Subject: Re: [RFC/PATCH] drm/rockchip: don't wait for vblank if fb hasn't changed Date: Thu, 14 Jan 2016 16:46:37 +0800 Message-ID: <5697606D.50603@rock-chips.com> References: <1452689614-6570-1-git-send-email-john@metanate.com> <20160113142320.GQ19130@phenom.ffwll.local> <20160113143425.363ce07a.john@metanate.com> <20160113154005.GT19130@phenom.ffwll.local> <20160113155529.6048b2c1.john@metanate.com> <20160113162156.GA19130@phenom.ffwll.local> <20160113164038.5fcf670f.john@metanate.com> <20160113171917.GC19130@phenom.ffwll.local> <20160113173954.52e688fc.john@metanate.com> <5696F6F4.5060908@rock-chips.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: 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: "linux-arm-kernel@lists.infradead.org" , "open list:ARM/Rockchip SoC..." , Linux Kernel Mailing List , dri-devel List-Id: linux-rockchip.vger.kernel.org T24gMjAxNuW5tDAx5pyIMTTml6UgMTY6MzIsIERhbmllbCBWZXR0ZXIgd3JvdGU6Cj4gT24gVGh1 LCBKYW4gMTQsIDIwMTYgYXQgMjoxNiBBTSwgTWFyayB5YW8gPG1hcmsueWFvQHJvY2stY2hpcHMu Y29tPiB3cm90ZToKPj4gT24gMjAxNuW5tDAx5pyIMTTml6UgMDE6MzksIEpvaG4gS2VlcGluZyB3 cm90ZToKPj4+IE9uIFdlZCwgMTMgSmFuIDIwMTYgMTg6MTk6MTcgKzAxMDAsIERhbmllbCBWZXR0 ZXIgd3JvdGU6Cj4+Pgo+Pj4+IE9uIFdlZCwgSmFuIDEzLCAyMDE2IGF0IDA0OjQwOjM4UE0gKzAw MDAsIEpvaG4gS2VlcGluZyB3cm90ZToKPj4+Pj4gT24gV2VkLCAxMyBKYW4gMjAxNiAxNzoyMTo1 NiArMDEwMCwgRGFuaWVsIFZldHRlciB3cm90ZToKPj4+Pj4KPj4+Pj4+IE9uIFdlZCwgSmFuIDEz LCAyMDE2IGF0IDAzOjU1OjI5UE0gKzAwMDAsIEpvaG4gS2VlcGluZyB3cm90ZToKPj4+Pj4+PiBP biBXZWQsIDEzIEphbiAyMDE2IDE2OjQwOjA1ICswMTAwLCBEYW5pZWwgVmV0dGVyIHdyb3RlOgo+ Pj4+Pj4+Cj4+Pj4+Pj4+IE9uIFdlZCwgSmFuIDEzLCAyMDE2IGF0IDAyOjM0OjI1UE0gKzAwMDAs IEpvaG4gS2VlcGluZyB3cm90ZToKPj4+Pj4+Pj4+IE9uIFdlZCwgMTMgSmFuIDIwMTYgMTU6MjM6 MjAgKzAxMDAsIERhbmllbCBWZXR0ZXIgd3JvdGU6Cj4+Pj4+Pj4+Pgo+Pj4+Pj4+Pj4+IE9uIFdl ZCwgSmFuIDEzLCAyMDE2IGF0IDEyOjUzOjM0UE0gKzAwMDAsIEpvaG4gS2VlcGluZyB3cm90ZToK Pj4+Pj4+Pj4+Pj4gQXMgY29tbWVudGVkIGluIGRybV9hdG9taWNfaGVscGVyX3dhaXRfZm9yX3Zi bGFua3MoKSwgdXNlcnNwYWNlCj4+Pj4+Pj4+Pj4+IHJlbGllcyBvbiBjdXJzb3IgaW9jdGxzIGJl aW5nIHVuc3luY2VkLiAgQ29udmVydGluZyB0aGUgcm9ja2NoaXAKPj4+Pj4+Pj4+Pj4gZHJpdmVy IHRvIGF0b21pYyBoYXMgc2lnbmlmaWNhbnRseSBpbXBhY3RlZCBjdXJzb3IgcGVyZm9ybWFuY2Ug YnkKPj4+Pj4+Pj4+Pj4gbWFraW5nIGV2ZXJ5IGN1cnNvciB1cGRhdGUgd2FpdCBmb3IgdmJsYW5r Lgo+Pj4+Pj4+Pj4+Pgo+Pj4+Pj4+Pj4+PiBCeSBza2lwcGluZyB0aGUgdmJsYW5rIHN5bmMgd2hl biB0aGUgZnJhbWVidWZmZXIgaGFzIG5vdCBjaGFuZ2VkCj4+Pj4+Pj4+Pj4+IChhcyBpcyBkb25l IGluIGRybV9hdG9taWNfaGVscGVyX3dhaXRfZm9yX3ZibGFua3MoKSkgd2UgY2FuIGF2b2lkCj4+ Pj4+Pj4+Pj4+IHRoaXMgZm9yIHRoZSBjb21tb24gY2FzZSBvZiBtb3ZpbmcgdGhlIGN1cnNvciBh bmQgb25seSBuZWVkIHRvCj4+Pj4+Pj4+Pj4+IGRlbGF5IHRoZSBjdXJzb3IgaW9jdGwgd2hlbiB0 aGUgY3Vyc29yIGljb24gY2hhbmdlcy4KPj4+Pj4+Pj4+Pj4KPj4+Pj4+Pj4+Pj4gSSBvcmlnaW5h bGx5IGluc2VydGVkIGEgY2hlY2sgb24gbGVnYWN5X2N1cnNvcl91cGRhdGUgYXMgd2VsbCwgYnV0 Cj4+Pj4+Pj4+Pj4+IHRoYXQgY2F1c2VkIGEgc3Rvcm0gb2YgaW9tbXUgcGFnZSBmYXVsdHMuICBJ IGRpZG4ndCBpbnZlc3RpZ2F0ZQo+Pj4+Pj4+Pj4+PiB0aGUKPj4+Pj4+Pj4+Pj4gY2F1c2Ugb2Yg dGhvc2Ugc2luY2UgdGhpcyBjaGFuZ2UgZ2l2ZXMgZW5vdWdoIG9mIGEgcGVyZm9ybWFuY2UKPj4+ Pj4+Pj4+Pj4gaW1wcm92ZW1lbnQgZm9yIG15IHVzZSBjYXNlLgo+Pj4+Pj4+Pj4+Pgo+Pj4+Pj4+ Pj4+PiBUaGlzIGlzIFJGQyBiZWNhdXNlIG9mIHRoYXQgYW5kIGJlY2F1c2UgdGhlIGZyYW1lYnVm ZmVyX2NoYW5nZWQoKQo+Pj4+Pj4+Pj4+PiBmdW5jdGlvbiBpcyBjb3BpZWQgZnJvbSBkcm1fYXRv bWljX2hlbHBlci5jIGFzIGEgcXVpY2sgd2F5IHRvIHRlc3QKPj4+Pj4+Pj4+Pj4gdGhlIHJlc3Vs dC4KPj4+Pj4+Pj4+Pj4KPj4+Pj4+Pj4+Pj4gU2lnbmVkLW9mZi1ieTogSm9obiBLZWVwaW5nIDxq b2huQG1ldGFuYXRlLmNvbT4KPj4+Pj4+Pj4+Pj4gLS0tCj4+Pj4+Pj4+Pj4+ICAgIGRyaXZlcnMv Z3B1L2RybS9yb2NrY2hpcC9yb2NrY2hpcF9kcm1fZmIuYyB8IDI3Cj4+Pj4+Pj4+Pj4+ICsrKysr KysrKysrKysrKysrKysrKysrKystLSAxIGZpbGUgY2hhbmdlZCwgMjUgaW5zZXJ0aW9ucygrKSwg Mgo+Pj4+Pj4+Pj4+PiBkZWxldGlvbnMoLSkKPj4+Pj4+Pj4+Pj4KPj4+Pj4+Pj4+Pj4gZGlmZiAt LWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9yb2NrY2hpcC9yb2NrY2hpcF9kcm1fZmIuYwo+Pj4+Pj4+ Pj4+PiBiL2RyaXZlcnMvZ3B1L2RybS9yb2NrY2hpcC9yb2NrY2hpcF9kcm1fZmIuYyBpbmRleAo+ Pj4+Pj4+Pj4+PiBmNzg0NDg4Li44ZmQ5ODIxCj4+Pj4+Pj4+Pj4+IDEwMDY0NCAtLS0gYS9kcml2 ZXJzL2dwdS9kcm0vcm9ja2NoaXAvcm9ja2NoaXBfZHJtX2ZiLmMKPj4+Pj4+Pj4+Pj4gKysrIGIv ZHJpdmVycy9ncHUvZHJtL3JvY2tjaGlwL3JvY2tjaGlwX2RybV9mYi5jCj4+Pj4+Pj4+Pj4+IEBA IC0xNzcsOCArMTc3LDI4IEBAIHN0YXRpYyB2b2lkCj4+Pj4+Pj4+Pj4+IHJvY2tjaGlwX2NydGNf d2FpdF9mb3JfdXBkYXRlKHN0cnVjdCBkcm1fY3J0YyAqY3J0YykKPj4+Pj4+Pj4+Pj4gY3J0Y19m dW5jcy0+d2FpdF9mb3JfdXBkYXRlKGNydGMpOyB9Cj4+Pj4+Pj4+Pj4+ICAgICtzdGF0aWMgYm9v bCBmcmFtZWJ1ZmZlcl9jaGFuZ2VkKHN0cnVjdCBkcm1fZGV2aWNlICpkZXYsCj4+Pj4+Pj4+Pj4+ ICsgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgc3RydWN0IGRybV9hdG9taWNfc3RhdGUK Pj4+Pj4+Pj4+Pj4gKm9sZF9zdGF0ZSwKPj4+Pj4+Pj4+Pj4gKyAgICAgICAgICAgICAgICAgICAg ICAgICAgICAgICBzdHJ1Y3QgZHJtX2NydGMgKmNydGMpCj4+Pj4+Pj4+Pj4+ICt7Cj4+Pj4+Pj4+ Pj4+ICsgICAgICAgc3RydWN0IGRybV9wbGFuZSAqcGxhbmU7Cj4+Pj4+Pj4+Pj4+ICsgICAgICAg c3RydWN0IGRybV9wbGFuZV9zdGF0ZSAqb2xkX3BsYW5lX3N0YXRlOwo+Pj4+Pj4+Pj4+PiArICAg ICAgIGludCBpOwo+Pj4+Pj4+Pj4+PiArCj4+Pj4+Pj4+Pj4+ICsgICAgICAgZm9yX2VhY2hfcGxh bmVfaW5fc3RhdGUob2xkX3N0YXRlLCBwbGFuZSwgb2xkX3BsYW5lX3N0YXRlLAo+Pj4+Pj4+Pj4+ PiBpKSB7Cj4+Pj4+Pj4+Pj4+ICsgICAgICAgICAgICAgICBpZiAocGxhbmUtPnN0YXRlLT5jcnRj ICE9IGNydGMgJiYKPj4+Pj4+Pj4+Pj4gKyAgICAgICAgICAgICAgICAgICBvbGRfcGxhbmVfc3Rh dGUtPmNydGMgIT0gY3J0YykKPj4+Pj4+Pj4+Pj4gKyAgICAgICAgICAgICAgICAgICAgICAgY29u dGludWU7Cj4+Pj4+Pj4+Pj4+ICsKPj4+Pj4+Pj4+Pj4gKyAgICAgICAgICAgICAgIGlmIChwbGFu ZS0+c3RhdGUtPmZiICE9IG9sZF9wbGFuZV9zdGF0ZS0+ZmIpCj4+Pj4+Pj4+Pj4+ICsgICAgICAg ICAgICAgICAgICAgICAgIHJldHVybiB0cnVlOwo+Pj4+Pj4+Pj4+PiArICAgICAgIH0KPj4+Pj4+ Pj4+Pj4gKwo+Pj4+Pj4+Pj4+PiArICAgICAgIHJldHVybiBmYWxzZTsKPj4+Pj4+Pj4+Pj4gK30K Pj4+Pj4+Pj4+PiBQbGVhc2UgZG9uJ3QgaGFuZC1yb2xsIGxvZ2ljIHRoYXQgYWZmZWN0cyBzZW1h bnRpY3MgbGlrZSB0aGlzLgo+Pj4+Pj4+Pj4+IEluc3RlYWQKPj4+Pj4+Pj4+PiBwbGVhc2UgdXNl IGRybV9hdG9taWNfaGVscGVyX3dhaXRfZm9yX3ZibGFua3MoKSwgd2hpY2ggc2hvdWxkIGRvCj4+ Pj4+Pj4+Pj4gdGhpcwo+Pj4+Pj4+Pj4+IGNvcnJlY3RseSBmb3IgeW91Lgo+Pj4+Pj4+Pj4+Cj4+ Pj4+Pj4+Pj4gSWYgdGhhdCdzIG5vdCB0aGUgY2FzZSB0aGVuIHdlIG5lZWQgdG8gaW1wcm92ZSB0 aGUgZ2VuZXJpYyBoZWxwZXIsCj4+Pj4+Pj4+Pj4gb3IKPj4+Pj4+Pj4+PiBmaWd1cmUgb3V0IHdo YXQncyBkaWZmZXJlbnQgd2l0aCByb2NraGlwLgo+Pj4+Pj4+Pj4gQWNjb3JkaW5nIHRvIGNvbW1p dCA2M2ViYjlmIChkcm0vcm9ja2NoaXA6IENvbnZlcnQgdG8gc3VwcG9ydCBhdG9taWMKPj4+Pj4+ Pj4+IEFQSSkgaXQncyBiZWNhdXNlIHJvY2tjaGlwIGRvZXNuJ3QgaGF2ZSBhIGhhcmR3YXJlIHZi bGFuayBjb3VudGVyLgo+Pj4+Pj4+Pj4KPj4+Pj4+Pj4+IEknbSBub3QgZW50aXJlbHkgY2xlYXIg b24gd2h5IHRoaXMgcHJldmVudHMgdGhlIHVzZSBvZgo+Pj4+Pj4+Pj4gZHJtX2F0b21pY19oZWxw ZXJfd2FpdF9mb3JfdmJsYW5rcygpLgo+Pj4+Pj4+PiBIbSwgdGhhdCBjb21taXQgaXNuJ3QgdGVy cmlibHkgaGVscGZ1bC4gSWYgdGhhdCdzIHJlYWxseSBuZWVkZWQgdGhlbgo+Pj4+Pj4+PiBpbW8g SQo+Pj4+Pj4+PiB0aGluayB3ZSBzaG91bGQgZXh0cmFjdCBhCj4+Pj4+Pj4+ICJkcm1fYXRvbWlj X2hlbHBlcl9wbGFuZV9uZWVkc192Ymxhbmtfd2FpdCgpIgo+Pj4+Pj4+PiBoZWxwZXIgdGhhdCdz IHVzZWQgYnkgYm90aC4gQnV0IHNpbmNlIHJvY2tjaGlwIGRvZXMgdmJsYW5rX2dldC9wdXQKPj4+ Pj4+Pj4gY2FsbHMKPj4+Pj4+Pj4gSSdkIGhvcGUgdmJsYW5rcyBhY3R1YWxseSB3b3JrIGNvcnJl Y3RseS4gQW5kIHRoZW4gdGhlIGhlbHBlciBzaG91bGQKPj4+Pj4+Pj4gd29yawo+Pj4+Pj4+PiB0 b28uCj4+Pj4+Pj4gSSB0cmllZCBzd2l0Y2hpbmcgdGhlIGNhbGwgdG8gcm9ja2NoaXBfY3J0Y193 YWl0X2Zvcl91cGRhdGUoKSB0bwo+Pj4+Pj4+IGRybV9hdG9taWNfaGVscGVyX3dhaXRfZm9yX3Zi bGFua3MoKSBhbmQgaXQgd29ya3MgZmluZSB1bnRpbCBJIHN3aXRjaAo+Pj4+Pj4+IHRoZSBidWZm ZXIgYXNzb2NpYXRlZCB3aXRoIGEgY3Vyc29yLCBhdCB3aGljaCBwb2ludCBJIGdldCBpb21tdSBw YWdlCj4+Pj4+Pj4gZmF1bHRzLCBwcmVzdW1hYmx5IGJlY2F1c2UgdGhlIEdFTSBidWZmZXIgaXMg dW5yZWZlcmVuY2VkIHRvbyBlYXJseS4KPj4+Pj4+Pgo+Pj4+Pj4+IEFGQUlDVCB0aGUgYnVmZmVy IHdpbGwgYmUgcmVsZWFzZWQgdmlhIGRybV9hdG9taWNfc3RhdGVfZnJlZSgpCj4+Pj4+Pj4gdW5j b25kaXRpb25hbGx5LCBidXQgSSBzdXNwZWN0IEknbSBtaXNzaW5nIHNvbWV0aGluZyBzaW5jZSB0 aGF0IHdvdWxkCj4+Pj4+Pj4gbWVhbiBldmVyeSBkcml2ZXIgd291bGQgaGl0IGEgc2ltaWxhciBw cm9ibGVtLgo+Pj4+Pj4gWWVhaCwgd2l0aCB0aGUgaGVscGVyIHdlIGFsd2F5cyBza2lwLCB3aGlj aCBtZWFucyB3aGVuIHRoZSBjdXJzb3IgYm8KPj4+Pj4+IGNoYW5nZXMgeW91IGluZGVlZCB1bm1h cCB0b28gZWFybHkuIFNvIGNhbid0IGV2ZW4gc2hhcmUgdGhlIG92ZXJhbGwKPj4+Pj4+IGNvbmRp dGlvbiwgYnV0IHdlIGNvdWxkIGRlZmluaXRlbHkgc2hhcmUgdGhlIGxpdHRsZSBmcmFtZWJ1ZmZl cl9jaGFuZ2VkCj4+Pj4+PiBoZWxwZXIuCj4+Pj4+IFRoYXQgbGVhdmVzIG1lIHdpdGggdGhlIHF1 ZXN0aW9uOiB3aHkgZG8gb3RoZXIgYXRvbWljIGRyaXZlcnMgd29yaz8KPj4+Pj4KPj4+Pj4gSWYg ZHJtX2F0b21pY19oZWxwZXJfd2FpdF9mb3JfdmJsYW5rcygpIHNraXBwaW5nIHZibGFua3MgcmVz dWx0cyBpbiB0aGUKPj4+Pj4gY3Vyc29yIGJvIGJlaW5nIHVubWFwcGVkIHRvbyBlYXJseSBmb3Ig cm9ja2NoaXAsIHdoeSBpcyBpdCBub3QgdW5tYXBwZWQKPj4+Pj4gdG9vIGVhcmx5IGZvciBhbGwg b2YgdGhlIG90aGVyIGRyaXZlcnMgdXNpbmcgdGhhdCBoZWxwZXI/Cj4+Pj4gSXQncyB1bm1hcHBl ZCB0b28gZWFybHkgZm9yIGV2ZXJ5b25lLCBpdCdzIGp1c3QgdGhhdCBub3JtYWxseSB0aGF0Cj4+ Pj4gZG9lc24ndAo+Pj4+IHJlc3VsdCBpbiBhIGZpcmV3b3JrcyBzaG93LiBXaGF0IHdlIG1heWJl IGNvdWxkL3Nob3VsZCBkbyBpcyBkbyB0aGUKPj4+PiB1bm1hcHBpbmcgYXN5bmNocm9ub3VzbHks IGJ1dCB0aGF0IHJ1bnMgaW50byB0aGUgb3ZlcmFsbCAiY3VycmVudCBhdG9taWMKPj4+PiBoZWxw ZXJzIGRvbid0IGRvIGFzeW5jIHlldCIgcHJvYmxlbS4gTWlnaHQgYmUgYSBnb29kIHBvaW50IHRv IHN0YXJ0Cj4+Pj4gZml4aW5nCj4+Pj4gdGhpcyB1cCB0aG91Z2guCj4+PiBPSywgdGhhbmtzLCBJ IHRoaW5rIEknbSBiZWdpbm5pbmcgdG8gdW5kZXJzdGFuZCBob3cgdGhpcyBhbGwgZml0cwo+Pj4g dG9nZXRoZXIuCj4+Pgo+Pj4gSXQgbG9va3MgbGlrZSB0aGVyZSBhcmUgdHdvIG9wdGlvbnMgZm9y IG1lIHRvIGdldCByZWFzb25hYmxlIGN1cnNvcgo+Pj4gcGVyZm9ybWFuY2Ugb24gcm9ja2NoaXAg aW4gdGhlIHNob3J0IHRlcm06Cj4+Pgo+Pj4gMSkgRXhwb3J0IHRoZSBjdXJyZW50IGZyYW1lYnVm ZmVyX2NoYW5nZWQoKSBmdW5jdGlvbiBhcwo+Pj4gICAgICBkcm1fYXRvbWljX2hlbHBlcl9mcmFt ZWJ1ZmZlcl9jaGFuZ2VkKCkgYW5kIHVzZSBpdCBpbgo+Pj4gICAgICByb2NrY2hpcF9jcnRjX3dh aXRfZm9yX3VwZGF0ZSgpLgo+Pj4KPj4+IDIpIEFkZCBhIG1lY2hhbmlzbSB0byBzdXBwcmVzcyB0 aGUgbGVnYWN5X2N1cnNvcl91cGRhdGUgY2hlY2sgaW4KPj4+ICAgICAgZHJtX2F0b21pY19oZWxw ZXJfd2FpdF9mb3JfdmJsYW5rcygpIGFuZCBzd2l0Y2ggdGhlIHJvY2tjaGlwIGRyaXZlcgo+Pj4g ICAgICBvdmVyIHRvIGl0Lgo+Pj4KPj4+IEluIGJvdGggb2YgdGhlc2UgY2FzZXMgd2UncmUgb25s eSByZXN0b3JpbmcgdGhlIHVuc3luY2VkIGN1cnNvciBpb2N0bHMKPj4+IGJlaGF2aW91ciB3aGVu IHRoZSBjdXJzb3IgaXMgbW92ZWQgYnV0IGl0IHdpbGwgc3RpbGwgYmUgZXhwZW5zaXZlIHdoZW4K Pj4+IHRoZSBjdXJzb3IgYm8gY2hhbmdlcy4gIFRoYXQgZ2l2ZXMgc3VmZmljaWVudCBwZXJmb3Jt YW5jZSBpbiBteSB0ZXN0aW5nLgo+Pj4KPj4+Cj4+Pgo+PiBUaGFua3MgZm9yIHBvaW50IHRoYXQu Cj4+Cj4+IGJlY2F1c2Ugcm9ja2NoaXAgbm90IHN1cHBvcnQgaGFyZHdhcmUgdmJsYW5rIGNvdW50 ZXIsIHVzZQo+PiBkcm1fYXRvbWljX2hlbHBlcl93YWl0X2Zvcl92YmxhbmtzIGhhdmUgdW5kZXIg aXNzdWVzOgo+Pgo+PiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAg ICAgfCA8LS0gSFcgdnN5bmMgaXJxIGFuZCByZWcgdGFrZQo+PiBlZmZlY3QKPj4gICAgICAgICAg ICAgIHBsYW5lX2NvbW1pdCAgLS0tPiAgfAo+PiAgICAgICBnZXRfdmJsYW5rIGFuZCB3YWl0IC0+ ICAgfAo+PiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgfCA8 LS0gaGFuZGxlX3ZibGFuaywKPj4gdmJsYW5rLT5jb3VudCArIDEKPj4gICAgICAgICAgICAgICAg ICAgY2xlYW51cF9mYiAgIC0tLT4gfAo+PiAgICAgICAgICAgICAgICBpb21tdSBjcmFzaCAgLS0t PiAgfAo+PiAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgICAgfCA8 LS0gSFcgdnN5bmMgaXJxIGFuZCByZWcgdGFrZQo+PiBlZmZlY3QKPj4gdGhlcmUgaXMgbm8gaGFy ZHdhcmUgdmJsYW5rIGNvdW50ZXIgb24gcm9ja2NoaXAgdm9wLCB3ZSBjYW4ndCBlbnN1cmUgdGhl Cj4+IGNvbnNpc3RlbmN5IG9mIHJlZyB0YWtlIGVmZmVjdCBhbmQgdmJsYW5rLT5jb3VudCwKPj4g aWYgcGxhbmUgY29tbWl0IGhpdCBpbnRvIHRoZSBwZXJpb2Qgb2YgIHJlZyB0YWtlIGVmZmVjdCBh bmQgdmJsYW5rLT5jb3VudCwKPj4gY2xlYW51cF9mYiBoYXBwZW4gYmVmb3JlIG9sZF9mYiBzd2Fw IG91dCBmcm9tIHZvcCwKPj4gdGhlbiBpb21tdSBjcmFzaC4KPj4KPj4gVGhhdCBpcyB3aHkgSSBz cGVjaWFsIHRoZSB3YWl0X2Zvcl92YmxhbmtzLCB3ZSBuZWVkIGNoZWNrIHRoZSByZWcgcmVhbGx5 Cj4+IHRha2UgZWZmZWN0IGJlZm9yZSBjbGVhbiB1cCBvbGQgZmIuCj4+IGF0IHZvcF93aW5fcGVu ZGluZ19pc19jb21wbGV0ZSBmdW5jdGlvbiwgY2hlY2sgd2luIGVuYWJsZSBhbmQgd2luIGFkZHJl c3MsCj4+IHRvIGVuc3VyZSB0aGF0Lgo+Pgo+PiBOb3Qgb25seSByb2NrY2hpcCBkcm0gZG8gdGhh dCB0aGluZzoKPj4KPj4gZXh5bm9zIGFsc28gY2hlY2sgYWRkcmVzcyBiZWZvcmUgY2xlYW51cCBm Ygo+PiAgICAgICAgICBpZiAoc3RhcnQgPT0gc3RhcnRfcykKPj4gICAgICAgICAgICAgIGV4eW5v c19kcm1fY3J0Y19maW5pc2hfdXBkYXRlKGN0eC0+Y3J0YywgcGxhbmUpOwo+Pgo+PiBUaGFua3Mu Cj4gRG8geW91IGhhdmUgYSBzY2FubGluZSBjb3VudGVyIG9yIHNvbWV0aGluZyBzaW1pbGFyIGF0 IGxlYXN0PyBBbnkKPiBvdGhlciBpbmRpY2F0aW9uIGFib3V0IGhvdyBmYXIgYWxvbmcgdGhlIGNo aXAgaXMgd2l0aCBzY2FubmluZyBvdXQ/IFdlCj4gdXNlIHRoYXQgaW4gaTkxNSB0byBhdm9pZCBy YWNlcyB3aXRoIHRoZSBpbnRlcnJ1cHQgaGFuZGxlciBhbmQgZGV0ZWN0Cj4gdGhpcyB3L2Egc2Nl bmFyaW8uCj4KPiBJIHRoaW5rIGlmIHlvdSBoYXZlIGEgc2NhbmxpbmUgY291bnRlciB0aGVuIGl0 IHNob3VsZCBtYWdpY2FsbHkgd29yaywKPiBzaW5jZSB0aGUgdmJsYW5rIGNvZGUgd2lsbCByZWFs aXplIHRoYXQgeW91J3JlIGFscmVhZHkgcGFzdCB0aGUgbGFzdAo+IHZibGFuayBpbnRlcnJ1cHQg YW5kIC9zaG91bGQvIGhhdmUgaW5jcmVtZW50ZWQgYWxyZWFkeS4gT3Igc29tZXRoaW5nCj4gbGlr ZSB0aGF0Lgo+Cj4gT3RoZXJ3aXNlIGlmIHRoaXMgaXMgY29tbW9uIHdlIG1pZ2h0IHdhbnQgdG8g ZmlndXJlIG91dCBob3cgdG8gc29sdmUKPiB0aGlzIGluIGEgZ2VuZXJpYyB3YXkuIEl0J3Mgb25l IG9mIHRoZXNlIHByb2JsZW1zIHRoYXQgd2lsbCBtYWtlCj4gZ2VuZXJpYyBhc3luYyBzdXBwb3J0 IGFsbW9zdCBpbXBvc3NpYmxlLgo+IC1EYW5pZWwKCk5vLCBib3RoIHJrMzI4OCBvciByazMwMzYg bm90IHN1cHBvcnQgaGFyZHdhcmUgdmJsYW5rIGNvdW50ZXIgYW5kIApzY2FubGluZSBjb3VudGVy LgoKQXQgYW5kcm9pZCBzaWRlLCB3ZSB1c2Ugc2FtZSB3YXksIGNoZWNrIGFkZHJlc3MgYW5kIGVu YWJsZSBiaXQgdG8gZW5zdXJlIApyZWdpc3RlciB0YWtlIGVmZmVjdC4KCk9uIGZ1dHVyZSBjaGlw cywgIHNjYW5saW5lIGNvdW50ZXIgYW5kIGhhcmR3YXJlIGNvdW50ZXIgd291bGQgYmUgCnN1cHBv cnQsIGJ1dCBub3Qgbm93LgoKLS0gCu+8rWFyayBZYW8KCgpfX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZl bEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cDovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWls bWFuL2xpc3RpbmZvL2RyaS1kZXZlbAo= From mboxrd@z Thu Jan 1 00:00:00 1970 From: mark.yao@rock-chips.com (Mark yao) Date: Thu, 14 Jan 2016 16:46:37 +0800 Subject: [RFC/PATCH] drm/rockchip: don't wait for vblank if fb hasn't changed In-Reply-To: References: <1452689614-6570-1-git-send-email-john@metanate.com> <20160113142320.GQ19130@phenom.ffwll.local> <20160113143425.363ce07a.john@metanate.com> <20160113154005.GT19130@phenom.ffwll.local> <20160113155529.6048b2c1.john@metanate.com> <20160113162156.GA19130@phenom.ffwll.local> <20160113164038.5fcf670f.john@metanate.com> <20160113171917.GC19130@phenom.ffwll.local> <20160113173954.52e688fc.john@metanate.com> <5696F6F4.5060908@rock-chips.com> Message-ID: <5697606D.50603@rock-chips.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 2016?01?14? 16:32, Daniel Vetter wrote: > On Thu, Jan 14, 2016 at 2:16 AM, Mark yao wrote: >> On 2016?01?14? 01:39, John Keeping wrote: >>> On Wed, 13 Jan 2016 18:19:17 +0100, Daniel Vetter wrote: >>> >>>> On Wed, Jan 13, 2016 at 04:40:38PM +0000, John Keeping wrote: >>>>> On Wed, 13 Jan 2016 17:21:56 +0100, Daniel Vetter wrote: >>>>> >>>>>> On Wed, Jan 13, 2016 at 03:55:29PM +0000, John Keeping wrote: >>>>>>> On Wed, 13 Jan 2016 16:40:05 +0100, Daniel Vetter wrote: >>>>>>> >>>>>>>> On Wed, Jan 13, 2016 at 02:34:25PM +0000, John Keeping wrote: >>>>>>>>> On Wed, 13 Jan 2016 15:23:20 +0100, Daniel Vetter wrote: >>>>>>>>> >>>>>>>>>> On Wed, Jan 13, 2016 at 12:53:34PM +0000, John Keeping wrote: >>>>>>>>>>> As commented in drm_atomic_helper_wait_for_vblanks(), userspace >>>>>>>>>>> relies on cursor ioctls being unsynced. Converting the rockchip >>>>>>>>>>> driver to atomic has significantly impacted cursor performance by >>>>>>>>>>> making every cursor update wait for vblank. >>>>>>>>>>> >>>>>>>>>>> By skipping the vblank sync when the framebuffer has not changed >>>>>>>>>>> (as is done in drm_atomic_helper_wait_for_vblanks()) we can avoid >>>>>>>>>>> this for the common case of moving the cursor and only need to >>>>>>>>>>> delay the cursor ioctl when the cursor icon changes. >>>>>>>>>>> >>>>>>>>>>> I originally inserted a check on legacy_cursor_update as well, but >>>>>>>>>>> that caused a storm of iommu page faults. I didn't investigate >>>>>>>>>>> the >>>>>>>>>>> cause of those since this change gives enough of a performance >>>>>>>>>>> improvement for my use case. >>>>>>>>>>> >>>>>>>>>>> This is RFC because of that and because the framebuffer_changed() >>>>>>>>>>> function is copied from drm_atomic_helper.c as a quick way to test >>>>>>>>>>> the result. >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: John Keeping >>>>>>>>>>> --- >>>>>>>>>>> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 27 >>>>>>>>>>> +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 >>>>>>>>>>> deletions(-) >>>>>>>>>>> >>>>>>>>>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c >>>>>>>>>>> b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c index >>>>>>>>>>> f784488..8fd9821 >>>>>>>>>>> 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c >>>>>>>>>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c >>>>>>>>>>> @@ -177,8 +177,28 @@ static void >>>>>>>>>>> rockchip_crtc_wait_for_update(struct drm_crtc *crtc) >>>>>>>>>>> crtc_funcs->wait_for_update(crtc); } >>>>>>>>>>> +static bool framebuffer_changed(struct drm_device *dev, >>>>>>>>>>> + struct drm_atomic_state >>>>>>>>>>> *old_state, >>>>>>>>>>> + struct drm_crtc *crtc) >>>>>>>>>>> +{ >>>>>>>>>>> + struct drm_plane *plane; >>>>>>>>>>> + struct drm_plane_state *old_plane_state; >>>>>>>>>>> + int i; >>>>>>>>>>> + >>>>>>>>>>> + for_each_plane_in_state(old_state, plane, old_plane_state, >>>>>>>>>>> i) { >>>>>>>>>>> + if (plane->state->crtc != crtc && >>>>>>>>>>> + old_plane_state->crtc != crtc) >>>>>>>>>>> + continue; >>>>>>>>>>> + >>>>>>>>>>> + if (plane->state->fb != old_plane_state->fb) >>>>>>>>>>> + return true; >>>>>>>>>>> + } >>>>>>>>>>> + >>>>>>>>>>> + return false; >>>>>>>>>>> +} >>>>>>>>>> Please don't hand-roll logic that affects semantics like this. >>>>>>>>>> Instead >>>>>>>>>> please use drm_atomic_helper_wait_for_vblanks(), which should do >>>>>>>>>> this >>>>>>>>>> correctly for you. >>>>>>>>>> >>>>>>>>>> If that's not the case then we need to improve the generic helper, >>>>>>>>>> or >>>>>>>>>> figure out what's different with rockhip. >>>>>>>>> According to commit 63ebb9f (drm/rockchip: Convert to support atomic >>>>>>>>> API) it's because rockchip doesn't have a hardware vblank counter. >>>>>>>>> >>>>>>>>> I'm not entirely clear on why this prevents the use of >>>>>>>>> drm_atomic_helper_wait_for_vblanks(). >>>>>>>> Hm, that commit isn't terribly helpful. If that's really needed then >>>>>>>> imo I >>>>>>>> think we should extract a >>>>>>>> "drm_atomic_helper_plane_needs_vblank_wait()" >>>>>>>> helper that's used by both. But since rockchip does vblank_get/put >>>>>>>> calls >>>>>>>> I'd hope vblanks actually work correctly. And then the helper should >>>>>>>> work >>>>>>>> too. >>>>>>> I tried switching the call to rockchip_crtc_wait_for_update() to >>>>>>> drm_atomic_helper_wait_for_vblanks() and it works fine until I switch >>>>>>> the buffer associated with a cursor, at which point I get iommu page >>>>>>> faults, presumably because the GEM buffer is unreferenced too early. >>>>>>> >>>>>>> AFAICT the buffer will be released via drm_atomic_state_free() >>>>>>> unconditionally, but I suspect I'm missing something since that would >>>>>>> mean every driver would hit a similar problem. >>>>>> Yeah, with the helper we always skip, which means when the cursor bo >>>>>> changes you indeed unmap too early. So can't even share the overall >>>>>> condition, but we could definitely share the little framebuffer_changed >>>>>> helper. >>>>> That leaves me with the question: why do other atomic drivers work? >>>>> >>>>> If drm_atomic_helper_wait_for_vblanks() skipping vblanks results in the >>>>> cursor bo being unmapped too early for rockchip, why is it not unmapped >>>>> too early for all of the other drivers using that helper? >>>> It's unmapped too early for everyone, it's just that normally that >>>> doesn't >>>> result in a fireworks show. What we maybe could/should do is do the >>>> unmapping asynchronously, but that runs into the overall "current atomic >>>> helpers don't do async yet" problem. Might be a good point to start >>>> fixing >>>> this up though. >>> OK, thanks, I think I'm beginning to understand how this all fits >>> together. >>> >>> It looks like there are two options for me to get reasonable cursor >>> performance on rockchip in the short term: >>> >>> 1) Export the current framebuffer_changed() function as >>> drm_atomic_helper_framebuffer_changed() and use it in >>> rockchip_crtc_wait_for_update(). >>> >>> 2) Add a mechanism to suppress the legacy_cursor_update check in >>> drm_atomic_helper_wait_for_vblanks() and switch the rockchip driver >>> over to it. >>> >>> In both of these cases we're only restoring the unsynced cursor ioctls >>> behaviour when the cursor is moved but it will still be expensive when >>> the cursor bo changes. That gives sufficient performance in my testing. >>> >>> >>> >> Thanks for point that. >> >> because rockchip not support hardware vblank counter, use >> drm_atomic_helper_wait_for_vblanks have under issues: >> >> | <-- HW vsync irq and reg take >> effect >> plane_commit ---> | >> get_vblank and wait -> | >> | <-- handle_vblank, >> vblank->count + 1 >> cleanup_fb ---> | >> iommu crash ---> | >> | <-- HW vsync irq and reg take >> effect >> there is no hardware vblank counter on rockchip vop, we can't ensure the >> consistency of reg take effect and vblank->count, >> if plane commit hit into the period of reg take effect and vblank->count, >> cleanup_fb happen before old_fb swap out from vop, >> then iommu crash. >> >> That is why I special the wait_for_vblanks, we need check the reg really >> take effect before clean up old fb. >> at vop_win_pending_is_complete function, check win enable and win address, >> to ensure that. >> >> Not only rockchip drm do that thing: >> >> exynos also check address before cleanup fb >> if (start == start_s) >> exynos_drm_crtc_finish_update(ctx->crtc, plane); >> >> Thanks. > Do you have a scanline counter or something similar at least? Any > other indication about how far along the chip is with scanning out? We > use that in i915 to avoid races with the interrupt handler and detect > this w/a scenario. > > I think if you have a scanline counter then it should magically work, > since the vblank code will realize that you're already past the last > vblank interrupt and /should/ have incremented already. Or something > like that. > > Otherwise if this is common we might want to figure out how to solve > this in a generic way. It's one of these problems that will make > generic async support almost impossible. > -Daniel No, both rk3288 or rk3036 not support hardware vblank counter and scanline counter. At android side, we use same way, check address and enable bit to ensure register take effect. On future chips, scanline counter and hardware counter would be support, but not now. -- ?ark Yao From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752382AbcANIq7 (ORCPT ); Thu, 14 Jan 2016 03:46:59 -0500 Received: from regular1.263xmail.com ([211.150.99.140]:41605 "EHLO regular1.263xmail.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750920AbcANIq6 (ORCPT ); Thu, 14 Jan 2016 03:46:58 -0500 X-263anti-spam: KSV:0;BIG:0;ABS:1;DNS:0;ATT:0;SPF:S; X-MAIL-GRAY: 0 X-MAIL-DELIVERY: 1 X-KSVirus-check: 0 X-ABS-CHECKED: 1 X-SKE-CHECKED: 1 X-ADDR-CHECKED: 0 X-RL-SENDER: mark.yao@rock-chips.com X-FST-TO: linux-arm-kernel@lists.infradead.org X-SENDER-IP: 58.22.7.114 X-LOGIN-NAME: mark.yao@rock-chips.com X-UNIQUE-TAG: <5e0e245da0a7412df2f8208c4a93e748> X-ATTACHMENT-NUM: 0 X-DNS-TYPE: 0 Message-ID: <5697606D.50603@rock-chips.com> Date: Thu, 14 Jan 2016 16:46:37 +0800 From: Mark yao User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.8.0 MIME-Version: 1.0 To: Daniel Vetter CC: John Keeping , Linux Kernel Mailing List , dri-devel , "open list:ARM/Rockchip SoC..." , "linux-arm-kernel@lists.infradead.org" Subject: Re: [RFC/PATCH] drm/rockchip: don't wait for vblank if fb hasn't changed References: <1452689614-6570-1-git-send-email-john@metanate.com> <20160113142320.GQ19130@phenom.ffwll.local> <20160113143425.363ce07a.john@metanate.com> <20160113154005.GT19130@phenom.ffwll.local> <20160113155529.6048b2c1.john@metanate.com> <20160113162156.GA19130@phenom.ffwll.local> <20160113164038.5fcf670f.john@metanate.com> <20160113171917.GC19130@phenom.ffwll.local> <20160113173954.52e688fc.john@metanate.com> <5696F6F4.5060908@rock-chips.com> In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2016年01月14日 16:32, Daniel Vetter wrote: > On Thu, Jan 14, 2016 at 2:16 AM, Mark yao wrote: >> On 2016年01月14日 01:39, John Keeping wrote: >>> On Wed, 13 Jan 2016 18:19:17 +0100, Daniel Vetter wrote: >>> >>>> On Wed, Jan 13, 2016 at 04:40:38PM +0000, John Keeping wrote: >>>>> On Wed, 13 Jan 2016 17:21:56 +0100, Daniel Vetter wrote: >>>>> >>>>>> On Wed, Jan 13, 2016 at 03:55:29PM +0000, John Keeping wrote: >>>>>>> On Wed, 13 Jan 2016 16:40:05 +0100, Daniel Vetter wrote: >>>>>>> >>>>>>>> On Wed, Jan 13, 2016 at 02:34:25PM +0000, John Keeping wrote: >>>>>>>>> On Wed, 13 Jan 2016 15:23:20 +0100, Daniel Vetter wrote: >>>>>>>>> >>>>>>>>>> On Wed, Jan 13, 2016 at 12:53:34PM +0000, John Keeping wrote: >>>>>>>>>>> As commented in drm_atomic_helper_wait_for_vblanks(), userspace >>>>>>>>>>> relies on cursor ioctls being unsynced. Converting the rockchip >>>>>>>>>>> driver to atomic has significantly impacted cursor performance by >>>>>>>>>>> making every cursor update wait for vblank. >>>>>>>>>>> >>>>>>>>>>> By skipping the vblank sync when the framebuffer has not changed >>>>>>>>>>> (as is done in drm_atomic_helper_wait_for_vblanks()) we can avoid >>>>>>>>>>> this for the common case of moving the cursor and only need to >>>>>>>>>>> delay the cursor ioctl when the cursor icon changes. >>>>>>>>>>> >>>>>>>>>>> I originally inserted a check on legacy_cursor_update as well, but >>>>>>>>>>> that caused a storm of iommu page faults. I didn't investigate >>>>>>>>>>> the >>>>>>>>>>> cause of those since this change gives enough of a performance >>>>>>>>>>> improvement for my use case. >>>>>>>>>>> >>>>>>>>>>> This is RFC because of that and because the framebuffer_changed() >>>>>>>>>>> function is copied from drm_atomic_helper.c as a quick way to test >>>>>>>>>>> the result. >>>>>>>>>>> >>>>>>>>>>> Signed-off-by: John Keeping >>>>>>>>>>> --- >>>>>>>>>>> drivers/gpu/drm/rockchip/rockchip_drm_fb.c | 27 >>>>>>>>>>> +++++++++++++++++++++++++-- 1 file changed, 25 insertions(+), 2 >>>>>>>>>>> deletions(-) >>>>>>>>>>> >>>>>>>>>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c >>>>>>>>>>> b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c index >>>>>>>>>>> f784488..8fd9821 >>>>>>>>>>> 100644 --- a/drivers/gpu/drm/rockchip/rockchip_drm_fb.c >>>>>>>>>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_fb.c >>>>>>>>>>> @@ -177,8 +177,28 @@ static void >>>>>>>>>>> rockchip_crtc_wait_for_update(struct drm_crtc *crtc) >>>>>>>>>>> crtc_funcs->wait_for_update(crtc); } >>>>>>>>>>> +static bool framebuffer_changed(struct drm_device *dev, >>>>>>>>>>> + struct drm_atomic_state >>>>>>>>>>> *old_state, >>>>>>>>>>> + struct drm_crtc *crtc) >>>>>>>>>>> +{ >>>>>>>>>>> + struct drm_plane *plane; >>>>>>>>>>> + struct drm_plane_state *old_plane_state; >>>>>>>>>>> + int i; >>>>>>>>>>> + >>>>>>>>>>> + for_each_plane_in_state(old_state, plane, old_plane_state, >>>>>>>>>>> i) { >>>>>>>>>>> + if (plane->state->crtc != crtc && >>>>>>>>>>> + old_plane_state->crtc != crtc) >>>>>>>>>>> + continue; >>>>>>>>>>> + >>>>>>>>>>> + if (plane->state->fb != old_plane_state->fb) >>>>>>>>>>> + return true; >>>>>>>>>>> + } >>>>>>>>>>> + >>>>>>>>>>> + return false; >>>>>>>>>>> +} >>>>>>>>>> Please don't hand-roll logic that affects semantics like this. >>>>>>>>>> Instead >>>>>>>>>> please use drm_atomic_helper_wait_for_vblanks(), which should do >>>>>>>>>> this >>>>>>>>>> correctly for you. >>>>>>>>>> >>>>>>>>>> If that's not the case then we need to improve the generic helper, >>>>>>>>>> or >>>>>>>>>> figure out what's different with rockhip. >>>>>>>>> According to commit 63ebb9f (drm/rockchip: Convert to support atomic >>>>>>>>> API) it's because rockchip doesn't have a hardware vblank counter. >>>>>>>>> >>>>>>>>> I'm not entirely clear on why this prevents the use of >>>>>>>>> drm_atomic_helper_wait_for_vblanks(). >>>>>>>> Hm, that commit isn't terribly helpful. If that's really needed then >>>>>>>> imo I >>>>>>>> think we should extract a >>>>>>>> "drm_atomic_helper_plane_needs_vblank_wait()" >>>>>>>> helper that's used by both. But since rockchip does vblank_get/put >>>>>>>> calls >>>>>>>> I'd hope vblanks actually work correctly. And then the helper should >>>>>>>> work >>>>>>>> too. >>>>>>> I tried switching the call to rockchip_crtc_wait_for_update() to >>>>>>> drm_atomic_helper_wait_for_vblanks() and it works fine until I switch >>>>>>> the buffer associated with a cursor, at which point I get iommu page >>>>>>> faults, presumably because the GEM buffer is unreferenced too early. >>>>>>> >>>>>>> AFAICT the buffer will be released via drm_atomic_state_free() >>>>>>> unconditionally, but I suspect I'm missing something since that would >>>>>>> mean every driver would hit a similar problem. >>>>>> Yeah, with the helper we always skip, which means when the cursor bo >>>>>> changes you indeed unmap too early. So can't even share the overall >>>>>> condition, but we could definitely share the little framebuffer_changed >>>>>> helper. >>>>> That leaves me with the question: why do other atomic drivers work? >>>>> >>>>> If drm_atomic_helper_wait_for_vblanks() skipping vblanks results in the >>>>> cursor bo being unmapped too early for rockchip, why is it not unmapped >>>>> too early for all of the other drivers using that helper? >>>> It's unmapped too early for everyone, it's just that normally that >>>> doesn't >>>> result in a fireworks show. What we maybe could/should do is do the >>>> unmapping asynchronously, but that runs into the overall "current atomic >>>> helpers don't do async yet" problem. Might be a good point to start >>>> fixing >>>> this up though. >>> OK, thanks, I think I'm beginning to understand how this all fits >>> together. >>> >>> It looks like there are two options for me to get reasonable cursor >>> performance on rockchip in the short term: >>> >>> 1) Export the current framebuffer_changed() function as >>> drm_atomic_helper_framebuffer_changed() and use it in >>> rockchip_crtc_wait_for_update(). >>> >>> 2) Add a mechanism to suppress the legacy_cursor_update check in >>> drm_atomic_helper_wait_for_vblanks() and switch the rockchip driver >>> over to it. >>> >>> In both of these cases we're only restoring the unsynced cursor ioctls >>> behaviour when the cursor is moved but it will still be expensive when >>> the cursor bo changes. That gives sufficient performance in my testing. >>> >>> >>> >> Thanks for point that. >> >> because rockchip not support hardware vblank counter, use >> drm_atomic_helper_wait_for_vblanks have under issues: >> >> | <-- HW vsync irq and reg take >> effect >> plane_commit ---> | >> get_vblank and wait -> | >> | <-- handle_vblank, >> vblank->count + 1 >> cleanup_fb ---> | >> iommu crash ---> | >> | <-- HW vsync irq and reg take >> effect >> there is no hardware vblank counter on rockchip vop, we can't ensure the >> consistency of reg take effect and vblank->count, >> if plane commit hit into the period of reg take effect and vblank->count, >> cleanup_fb happen before old_fb swap out from vop, >> then iommu crash. >> >> That is why I special the wait_for_vblanks, we need check the reg really >> take effect before clean up old fb. >> at vop_win_pending_is_complete function, check win enable and win address, >> to ensure that. >> >> Not only rockchip drm do that thing: >> >> exynos also check address before cleanup fb >> if (start == start_s) >> exynos_drm_crtc_finish_update(ctx->crtc, plane); >> >> Thanks. > Do you have a scanline counter or something similar at least? Any > other indication about how far along the chip is with scanning out? We > use that in i915 to avoid races with the interrupt handler and detect > this w/a scenario. > > I think if you have a scanline counter then it should magically work, > since the vblank code will realize that you're already past the last > vblank interrupt and /should/ have incremented already. Or something > like that. > > Otherwise if this is common we might want to figure out how to solve > this in a generic way. It's one of these problems that will make > generic async support almost impossible. > -Daniel No, both rk3288 or rk3036 not support hardware vblank counter and scanline counter. At android side, we use same way, check address and enable bit to ensure register take effect. On future chips, scanline counter and hardware counter would be support, but not now. -- Ｍark Yao