From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v11 4/7] drm/i915/skl: Update plane watermarks atomically during plane updates Date: Fri, 12 Aug 2016 15:42:28 +0300 Message-ID: <20160812124228.GA4329@intel.com> References: <1470945277-7973-1-git-send-email-cpaul@redhat.com> <1470945277-7973-5-git-send-email-cpaul@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Content-Disposition: inline In-Reply-To: <1470945277-7973-5-git-send-email-cpaul@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dri-devel-bounces@lists.freedesktop.org Sender: "dri-devel" To: Lyude Cc: dri-devel@lists.freedesktop.org, Radhakrishna Sripada , intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org, Hans de Goede , Daniel Vetter List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCBBdWcgMTEsIDIwMTYgYXQgMDM6NTQ6MzNQTSAtMDQwMCwgTHl1ZGUgd3JvdGU6Cj4g VGhhbmtzIHRvIFZpbGxlIGZvciBzdWdnZXN0aW5nIHRoaXMgYXMgYSBwb3RlbnRpYWwgc29sdXRp b24gdG8gcGlwZQo+IHVuZGVycnVucyBvbiBTa3lsYWtlLgo+IAo+IE9uIFNreWxha2UgYWxsIG9m IHRoZSByZWdpc3RlcnMgZm9yIGNvbmZpZ3VyaW5nIHBsYW5lcywgaW5jbHVkaW5nIHRoZQo+IHJl Z2lzdGVycyBmb3IgY29uZmlndXJpbmcgdGhlaXIgd2F0ZXJtYXJrcywgYXJlIGRvdWJsZSBidWZm ZXJlZC4gTmV3Cj4gdmFsdWVzIHdyaXR0ZW4gdG8gdGhlbSB3b24ndCB0YWtlIGVmZmVjdCB1bnRp bCBzYWlkIHJlZ2lzdGVycyBhcmUKPiAiYXJtZWQiLCB3aGljaCBpcyBkb25lIGJ5IHdyaXRpbmcg dG8gdGhlIFBMQU5FX1NVUkYgKG9yIGluIHRoZSBjYXNlIG9mCj4gY3Vyc29yIHBsYW5lcywgdGhl IENVUkJBU0UgcmVnaXN0ZXIpIHJlZ2lzdGVyLgo+IAo+IFdpdGggdGhpcyBpbiBtaW5kLCB1cCB1 bnRpbCBub3cgd2UndmUgYmVlbiB1cGRhdGluZyB3YXRlcm1hcmtzIG9uIHNrbAo+IGxpa2UgdGhp czoKPiAKPiAgIG5vbi1tb2Rlc2V0IHsKPiAgICAtIGNhbGN1bGF0ZSAoZHVyaW5nIGF0b21pYyBj aGVjayBwaGFzZSkKPiAgICAtIGZpbmlzaF9hdG9taWNfY29tbWl0Ogo+ICAgICAgLSBpbnRlbF9w cmVfcGxhbmVfdXBkYXRlOgo+ICAgICAgICAgLSBpbnRlbF91cGRhdGVfd2F0ZXJtYXJrcygpCj4g ICAgICAtIHt2YmxhbmsgaGFwcGVuczsgbmV3IHdhdGVybWFya3MgKyBvbGQgcGxhbmUgdmFsdWVz ID0+IHVuZGVycnVuIH0KPiAgICAgIC0gZHJtX2F0b21pY19oZWxwZXJfY29tbWl0X3BsYW5lc19v bl9jcnRjOgo+ICAgICAgICAgLSBzdGFydCB2YmxhbmsgZXZhc2lvbgo+ICAgICAgICAgLSB3cml0 ZSBuZXcgcGxhbmUgcmVnaXN0ZXJzCj4gICAgICAgICAtIGVuZCB2YmxhbmsgZXZhc2lvbgo+ICAg fQo+IAo+ICAgb3IKPiAKPiAgIG1vZGVzZXQgewo+ICAgIC0gY2FsY3VsYXRlIChkdXJpbmcgYXRv bWljIGNoZWNrIHBoYXNlKQo+ICAgIC0gZmluaXNoX2F0b21pY19jb21taXQ6Cj4gICAgICAtIGNy dGNfZW5hYmxlOgo+ICAgICAgICAgLSBpbnRlbF91cGRhdGVfd2F0ZXJtYXJrcygpCj4gICAgICAt IHt2YmxhbmsgaGFwcGVuczsgbmV3IHdhdGVybWFya3MgKyBvbGQgcGxhbmUgdmFsdWVzID0+IHVu ZGVycnVuIH0KPiAgICAgIC0gZHJtX2F0b21pY19oZWxwZXJfY29tbWl0X3BsYW5lc19vbl9jcnRj Ogo+ICAgICAgICAgLSBzdGFydCB2YmxhbmsgZXZhc2lvbgo+ICAgICAgICAgLSB3cml0ZSBuZXcg cGxhbmUgcmVnaXN0ZXJzCj4gICAgICAgICAtIGVuZCB2YmxhbmsgZXZhc2lvbgo+ICAgfQo+IAo+ IE5vdyB3ZSB1cGRhdGUgd2F0ZXJtYXJrcyBhdG9taWNhbGx5IGxpa2UgdGhpczoKPiAKPiAgIG5v bi1tb2Rlc2V0IHsKPiAgICAtIGNhbGN1bGF0ZSAoZHVyaW5nIGF0b21pYyBjaGVjayBwaGFzZSkK PiAgICAtIGZpbmlzaF9hdG9taWNfY29tbWl0Ogo+ICAgICAgLSBpbnRlbF9wcmVfcGxhbmVfdXBk YXRlOgo+ICAgICAgICAgLSBpbnRlbF91cGRhdGVfd2F0ZXJtYXJrcygpICh3bSB2YWx1ZXMgYXJl bid0IHdyaXR0ZW4geWV0KQo+ICAgICAgLSBkcm1fYXRvbWljX2hlbHBlcl9jb21taXRfcGxhbmVz X29uX2NydGM6Cj4gICAgICAgICAtIHN0YXJ0IHZibGFuayBldmFzaW9uCj4gICAgICAgICAtIHdy aXRlIG5ldyBwbGFuZSByZWdpc3RlcnMKPiAgICAgICAgIC0gd3JpdGUgbmV3IHdtIHZhbHVlcwo+ ICAgICAgICAgLSBlbmQgdmJsYW5rIGV2YXNpb24KPiAgIH0KPiAKPiAgIG1vZGVzZXQgewo+ICAg IC0gY2FsY3VsYXRlIChkdXJpbmcgYXRvbWljIGNoZWNrIHBoYXNlKQo+ICAgIC0gZmluaXNoX2F0 b21pY19jb21taXQ6Cj4gICAgICAtIGNydGNfZW5hYmxlOgo+ICAgICAgICAgLSBpbnRlbF91cGRh dGVfd2F0ZXJtYXJrcygpIChhY3R1YWwgd20gdmFsdWVzIGFyZW4ndCB3cml0dGVuCj4gICAgICAg ICAgIHlldCkKPiAgICAgIC0gZHJtX2F0b21pY19oZWxwZXJfY29tbWl0X3BsYW5lc19vbl9jcnRj Ogo+ICAgICAgICAgLSBzdGFydCB2YmxhbmsgZXZhc2lvbgo+ICAgICAgICAgLSB3cml0ZSBuZXcg cGxhbmUgcmVnaXN0ZXJzCj4gCS0gd3JpdGUgbmV3IHdtIHZhbHVlcwo+ICAgICAgICAgLSBlbmQg dmJsYW5rIGV2YXNpb24KPiAgIH0KPiAKPiBTbyB0aGlzIHBhdGNoIG1vdmVzIGFsbCBvZiB0aGUg d2F0ZXJtYXJrIHdyaXRlcyBpbnRvIHRoZSByaWdodCBwbGFjZTsKPiBpbnNpZGUgb2YgdGhlIHZi bGFuayBldmFzaW9uIHdoZXJlIHdlIHVwZGF0ZSBhbGwgb2YgdGhlIHJlZ2lzdGVycyBmb3IKPiBl YWNoIHBsYW5lLiBXaGlsZSB0aGlzIHBhdGNoIGRvZXNuJ3QgZml4IGV2ZXJ5dGhpbmcsIGl0IGRv ZXMgYWxsb3cgdXMgdG8KPiB1cGRhdGUgdGhlIHdhdGVybWFyayB2YWx1ZXMgaW4gdGhlIHdheSB0 aGUgaGFyZHdhcmUgZXhwZWN0cyB1cyB0by4KPiAKPiBDaGFuZ2VzIHNpbmNlIG9yaWdpbmFsIHBh dGNoIHNlcmllczoKPiAgLSBSZW1vdmUgbXV0ZXhfbG9jay9tdXRleF91bmxvY2sgc2luY2UgdGhl eSBkb24ndCBkbyBhbnl0aGluZyBhbmQgd2UncmUKPiAgICBub3QgdG91Y2hpbmcgZ2xvYmFsIHN0 YXRlCj4gIC0gTW92ZSBza2xfd3JpdGVfY3Vyc29yX3dtL3NrbF93cml0ZV9wbGFuZV93bSBmdW5j dGlvbnMgaW50bwo+ICAgIGludGVsX3BtLmMsIG1ha2UgZXh0ZXJuYWxseSB2aXNpYmxlCj4gIC0g QWRkIHNrbF93cml0ZV9wbGFuZV93bSBjYWxscyB0byBza2xfdXBkYXRlX3BsYW5lCj4gIC0gRml4 IGNvbmRpdGlvbmFsIGZvciBmb3IgbG9vcCBpbiBza2xfd3JpdGVfcGxhbmVfd20gKGxldmVsIDwg bWF4X2xldmVsCj4gICAgc2hvdWxkIGJlIGxldmVsIDw9IG1heF9sZXZlbCkKPiAgLSBNYWtlIGRp YWdyYW0gaW4gY29tbWl0IG1vcmUgYWNjdXJhdGUgdG8gd2hhdCdzIGFjdHVhbGx5IGhhcHBlbmlu Zwo+ICAtIEFkZCBGaXhlczoKPiAKPiBDaGFuZ2VzIHNpbmNlIHYxOgo+ICAtIFVzZSBJU19HRU45 KCkgaW5zdGVhZCBvZiBJU19TS1lMQUtFKCkgc2luY2UgdGhlc2UgZml4ZXMgYXBwbHkgdG8gbW9y ZQo+ICAgIHRoZW4ganVzdCBTa3lsYWtlCj4gIC0gVXBkYXRlIGRlc2NyaXB0aW9uIHRvIG1ha2Ug aXQgY2xlYXIgdGhpcyBwYXRjaCBkb2Vzbid0IGZpeCBldmVyeXRoaW5nCj4gIC0gQ2hlY2sgaWYg cGlwZXMgd2VyZSBhY3R1YWxseSBjaGFuZ2VkIGJlZm9yZSB3cml0aW5nIHdhdGVybWFya3MKPiAK PiBDaGFuZ2VzIHNpbmNlIHYyOgo+ICAtIFdyaXRlIFBJUEVfV01fTElORVRJTUUgZHVyaW5nIHZi bGFuayBldmFzaW9uCj4gCj4gQ2hhbmdlcyBzaW5jZSB2MzoKPiAgLSBSZWJhc2UgYWdhaW5zdCBu ZXcgU0FHViBwYXRjaCBjaGFuZ2VzCj4gCj4gQ2hhbmdlcyBzaW5jZSB2NDoKPiAgLSBBZGQgYSBw YXJhbWV0ZXIgdG8gY2hvb3NlIHdoYXQgc2tsX3dtX3ZhbHVlcyBzdHJ1Y3QgdG8gdXNlIHdoZW4K PiAgICB3cml0aW5nIG5ldyBwbGFuZSB3YXRlcm1hcmtzCj4gCj4gQ2hhbmdlcyBzaW5jZSB2NToK PiAgLSBSZW1vdmUgY3Vyc29yIGRkYiBlbnRyeSB3cml0ZSBpbiBza2xfd3JpdGVfY3Vyc29yX3dt KCksIGRlZmVyIHVudGlsCj4gICAgcGF0Y2ggNgo+ICAtIFdyaXRlIFdNX0xJTkVUSU1FIGluIGlu dGVsX2JlZ2luX2NydGNfY29tbWl0KCkKPiAKPiBDaGFuZ2VzIHNpbmNlIHY2Ogo+ICAtIFJlbW92 ZSByZWR1bmRhbnQgZGlydHlfcGlwZXMgY2hlY2sgaW4gc2tsX3dyaXRlX3BsYW5lX3dtICh3ZSBj aGVjawo+ICAgIHRoaXMgaW4gYWxsIHBsYWNlcyB3aGVyZSB3ZSBjYWxsIHRoaXMgZnVuY3Rpb24s IGFuZCBpdCB3YXMgc3VwcG9zZWQKPiAgICB0byBoYXZlIGJlZW4gcmVtb3ZlZCBlYXJsaWVyIGFu eXdheSkKPiAgLSBJbiBpOXh4X3VwZGF0ZV9jdXJzb3IoKSwgdXNlIGRldl9wcml2LT5pbmZvLmdl biA+PSA5IGluc3RlYWQgb2YKPiAgICBJU19HRU45KGRldl9wcml2KS4gV2UgZG8gdGhpcyBldmVy eXdoZXJlIGVsc2UgYW5kIEknZCBpbWFnaW5lIHRoaXMKPiAgICBuZWVkcyB0byBiZSBkb25lIGZv ciBnZW4xMCBhcyB3ZWxsCj4gCj4gRml4ZXM6IDJkNDFjMGI1OWFmYyAoImRybS9pOTE1L3NrbDog U0tMIFdhdGVybWFyayBDb21wdXRhdGlvbiIpCj4gU2lnbmVkLW9mZi1ieTogTHl1ZGUgPGNwYXVs QHJlZGhhdC5jb20+Cj4gUmV2aWV3ZWQtYnk6IE1hdHQgUm9wZXIgPG1hdHRoZXcuZC5yb3BlckBp bnRlbC5jb20+Cj4gQ2M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmcKPiBDYzogVmlsbGUgU3lyasOk bMOkIDx2aWxsZS5zeXJqYWxhQGxpbnV4LmludGVsLmNvbT4KPiBDYzogRGFuaWVsIFZldHRlciA8 ZGFuaWVsLnZldHRlckBpbnRlbC5jb20+Cj4gQ2M6IFJhZGhha3Jpc2huYSBTcmlwYWRhIDxyYWRo YWtyaXNobmEuc3JpcGFkYUBpbnRlbC5jb20+Cj4gQ2M6IEhhbnMgZGUgR29lZGUgPGhkZWdvZWRl QHJlZGhhdC5jb20+Cj4gLS0tCj4gIGRyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2Rpc3BsYXku YyB8IDE3ICsrKysrKysrKysrLQo+ICBkcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kcnYuaCAg ICAgfCAgNSArKysrCj4gIGRyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX3BtLmMgICAgICB8IDUw ICsrKysrKysrKysrKysrKysrKysrKysrKy0tLS0tLS0tLS0tLQo+ICBkcml2ZXJzL2dwdS9kcm0v aTkxNS9pbnRlbF9zcHJpdGUuYyAgfCAgNyArKysrKwo+ICA0IGZpbGVzIGNoYW5nZWQsIDYyIGlu c2VydGlvbnMoKyksIDE3IGRlbGV0aW9ucygtKQo+IAo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dw dS9kcm0vaTkxNS9pbnRlbF9kaXNwbGF5LmMgYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9k aXNwbGF5LmMKPiBpbmRleCAzNWJkZDY3Li5iMmY4ZTI0IDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMv Z3B1L2RybS9pOTE1L2ludGVsX2Rpc3BsYXkuYwo+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1 L2ludGVsX2Rpc3BsYXkuYwo+IEBAIC0zMzg2LDYgKzMzODYsOCBAQCBzdGF0aWMgdm9pZCBza3ls YWtlX3VwZGF0ZV9wcmltYXJ5X3BsYW5lKHN0cnVjdCBkcm1fcGxhbmUgKnBsYW5lLAo+ICAJc3Ry dWN0IGRybV9pOTE1X3ByaXZhdGUgKmRldl9wcml2ID0gdG9faTkxNShkZXYpOwo+ICAJc3RydWN0 IGludGVsX2NydGMgKmludGVsX2NydGMgPSB0b19pbnRlbF9jcnRjKGNydGNfc3RhdGUtPmJhc2Uu Y3J0Yyk7Cj4gIAlzdHJ1Y3QgZHJtX2ZyYW1lYnVmZmVyICpmYiA9IHBsYW5lX3N0YXRlLT5iYXNl LmZiOwo+ICsJc3RydWN0IGRybV9pOTE1X2dlbV9vYmplY3QgKm9iaiA9IGludGVsX2ZiX29iaihm Yik7CgpyZWJhc2UgZmFpbD8KClBsZWFzZSBkb3VibGUgY2hlY2sgd2l0aCBnY2Mvc3BhcnNlIHRo YXQgbmV3IHdhcm5pbmdzIGFyZSBrZXB0IHRvIGEKbWluaW11bS4KCj4gKwlzdHJ1Y3Qgc2tsX3dt X3ZhbHVlcyAqd20gPSAmZGV2X3ByaXYtPndtLnNrbF9yZXN1bHRzOwoKY29uc3Q/Cgo+ICAJaW50 IHBpcGUgPSBpbnRlbF9jcnRjLT5waXBlOwo+ICAJdTMyIHBsYW5lX2N0bDsKPiAgCXVuc2lnbmVk IGludCByb3RhdGlvbiA9IHBsYW5lX3N0YXRlLT5iYXNlLnJvdGF0aW9uOwo+IEBAIC0zNDE5LDYg KzM0MjEsOSBAQCBzdGF0aWMgdm9pZCBza3lsYWtlX3VwZGF0ZV9wcmltYXJ5X3BsYW5lKHN0cnVj dCBkcm1fcGxhbmUgKnBsYW5lLAo+ICAJaW50ZWxfY3J0Yy0+YWRqdXN0ZWRfeCA9IHNyY194Owo+ ICAJaW50ZWxfY3J0Yy0+YWRqdXN0ZWRfeSA9IHNyY195Owo+ICAKPiArCWlmICh3bS0+ZGlydHlf cGlwZXMgJiBkcm1fY3J0Y19tYXNrKCZpbnRlbF9jcnRjLT5iYXNlKSkKPiArCSAgICBza2xfd3Jp dGVfcGxhbmVfd20oaW50ZWxfY3J0Yywgd20sIDApOwoKSW5kZW50IGZhaWwuCgo+ICsKPiAgCUk5 MTVfV1JJVEUoUExBTkVfQ1RMKHBpcGUsIDApLCBwbGFuZV9jdGwpOwo+ICAJSTkxNV9XUklURShQ TEFORV9PRkZTRVQocGlwZSwgMCksIChzcmNfeSA8PCAxNikgfCBzcmNfeCk7Cj4gIAlJOTE1X1dS SVRFKFBMQU5FX1NUUklERShwaXBlLCAwKSwgc3RyaWRlKTsKPiBAQCAtMTA2OTksOSArMTA3MDQs MTMgQEAgc3RhdGljIHZvaWQgaTl4eF91cGRhdGVfY3Vyc29yKHN0cnVjdCBkcm1fY3J0YyAqY3J0 YywgdTMyIGJhc2UsCj4gIAlzdHJ1Y3QgZHJtX2RldmljZSAqZGV2ID0gY3J0Yy0+ZGV2Owo+ICAJ c3RydWN0IGRybV9pOTE1X3ByaXZhdGUgKmRldl9wcml2ID0gdG9faTkxNShkZXYpOwo+ICAJc3Ry dWN0IGludGVsX2NydGMgKmludGVsX2NydGMgPSB0b19pbnRlbF9jcnRjKGNydGMpOwo+ICsJc3Ry dWN0IHNrbF93bV92YWx1ZXMgKndtID0gJmRldl9wcml2LT53bS5za2xfcmVzdWx0czsKPiAgCWlu dCBwaXBlID0gaW50ZWxfY3J0Yy0+cGlwZTsKPiAgCXVpbnQzMl90IGNudGwgPSAwOwo+ICAKPiAr CWlmIChkZXZfcHJpdi0+aW5mby5nZW4gPj0gOSAmJiB3bS0+ZGlydHlfcGlwZXMgJiBkcm1fY3J0 Y19tYXNrKGNydGMpCgpJTlRFTF9HRU4oKQoKPiArCQlza2xfd3JpdGVfY3Vyc29yX3dtKGludGVs X2NydGMsIHdtKTsKPiArCj4gIAlpZiAocGxhbmVfc3RhdGUgJiYgcGxhbmVfc3RhdGUtPmJhc2Uu dmlzaWJsZSkgewo+ICAJCWNudGwgPSBNQ1VSU09SX0dBTU1BX0VOQUJMRTsKPiAgCQlzd2l0Y2gg KHBsYW5lX3N0YXRlLT5iYXNlLmNydGNfdykgewo+IEBAIC0xNDYxMCwxMCArMTQ2MTksMTIgQEAg c3RhdGljIHZvaWQgaW50ZWxfYmVnaW5fY3J0Y19jb21taXQoc3RydWN0IGRybV9jcnRjICpjcnRj LAo+ICAJCQkJICAgIHN0cnVjdCBkcm1fY3J0Y19zdGF0ZSAqb2xkX2NydGNfc3RhdGUpCj4gIHsK PiAgCXN0cnVjdCBkcm1fZGV2aWNlICpkZXYgPSBjcnRjLT5kZXY7Cj4gKwlzdHJ1Y3QgZHJtX2k5 MTVfcHJpdmF0ZSAqZGV2X3ByaXYgPSB0b19pOTE1KGRldik7Cj4gIAlzdHJ1Y3QgaW50ZWxfY3J0 YyAqaW50ZWxfY3J0YyA9IHRvX2ludGVsX2NydGMoY3J0Yyk7Cj4gIAlzdHJ1Y3QgaW50ZWxfY3J0 Y19zdGF0ZSAqb2xkX2ludGVsX3N0YXRlID0KPiAgCQl0b19pbnRlbF9jcnRjX3N0YXRlKG9sZF9j cnRjX3N0YXRlKTsKPiAgCWJvb2wgbW9kZXNldCA9IG5lZWRzX21vZGVzZXQoY3J0Yy0+c3RhdGUp Owo+ICsJZW51bSBwaXBlIHBpcGUgPSBpbnRlbF9jcnRjLT5waXBlOwo+ICAKPiAgCS8qIFBlcmZv cm0gdmJsYW5rIGV2YXNpb24gYXJvdW5kIGNvbW1pdCBvcGVyYXRpb24gKi8KPiAgCWludGVsX3Bp cGVfdXBkYXRlX3N0YXJ0KGludGVsX2NydGMpOwo+IEBAIC0xNDYyOCw4ICsxNDYzOSwxMiBAQCBz dGF0aWMgdm9pZCBpbnRlbF9iZWdpbl9jcnRjX2NvbW1pdChzdHJ1Y3QgZHJtX2NydGMgKmNydGMs Cj4gIAo+ICAJaWYgKHRvX2ludGVsX2NydGNfc3RhdGUoY3J0Yy0+c3RhdGUpLT51cGRhdGVfcGlw ZSkKPiAgCQlpbnRlbF91cGRhdGVfcGlwZV9jb25maWcoaW50ZWxfY3J0Yywgb2xkX2ludGVsX3N0 YXRlKTsKPiAtCWVsc2UgaWYgKElOVEVMX0lORk8oZGV2KS0+Z2VuID49IDkpCj4gKwllbHNlIGlm IChJTlRFTF9JTkZPKGRldiktPmdlbiA+PSA5KSB7Cj4gIAkJc2tsX2RldGFjaF9zY2FsZXJzKGlu dGVsX2NydGMpOwo+ICsKPiArCQlJOTE1X1dSSVRFKFBJUEVfV01fTElORVRJTUUocGlwZSksCj4g KwkJCSAgIGRldl9wcml2LT53bS5za2xfaHcud21fbGluZXRpbWVbcGlwZV0pOwo+ICsJfQo+ICB9 Cj4gIAo+ICBzdGF0aWMgdm9pZCBpbnRlbF9maW5pc2hfY3J0Y19jb21taXQoc3RydWN0IGRybV9j cnRjICpjcnRjLAo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kcnYu aCBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2Rydi5oCj4gaW5kZXggNzZlNzhiOC4uODgw ODhjMyAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9kcnYuaAo+ICsr KyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2Rydi5oCj4gQEAgLTE3MjMsNiArMTcyMywx MSBAQCB2b2lkIHNrbF9kZGJfZ2V0X2h3X3N0YXRlKHN0cnVjdCBkcm1faTkxNV9wcml2YXRlICpk ZXZfcHJpdiwKPiAgCQkJICBzdHJ1Y3Qgc2tsX2RkYl9hbGxvY2F0aW9uICpkZGIgLyogb3V0ICov KTsKPiAgaW50IHNrbF9lbmFibGVfc2FndihzdHJ1Y3QgZHJtX2k5MTVfcHJpdmF0ZSAqZGV2X3By aXYpOwo+ICBpbnQgc2tsX2Rpc2FibGVfc2FndihzdHJ1Y3QgZHJtX2k5MTVfcHJpdmF0ZSAqZGV2 X3ByaXYpOwo+ICt2b2lkIHNrbF93cml0ZV9jdXJzb3Jfd20oc3RydWN0IGludGVsX2NydGMgKmlu dGVsX2NydGMsCj4gKwkJCSBjb25zdCBzdHJ1Y3Qgc2tsX3dtX3ZhbHVlcyAqd20pOwo+ICt2b2lk IHNrbF93cml0ZV9wbGFuZV93bShzdHJ1Y3QgaW50ZWxfY3J0YyAqaW50ZWxfY3J0YywKPiArCQkJ Y29uc3Qgc3RydWN0IHNrbF93bV92YWx1ZXMgKndtLAo+ICsJCQlpbnQgcGxhbmUpOwo+ICB1aW50 MzJfdCBpbGtfcGlwZV9waXhlbF9yYXRlKGNvbnN0IHN0cnVjdCBpbnRlbF9jcnRjX3N0YXRlICpw aXBlX2NvbmZpZyk7Cj4gIGJvb2wgaWxrX2Rpc2FibGVfbHBfd20oc3RydWN0IGRybV9kZXZpY2Ug KmRldik7Cj4gIGludCBzYW5pdGl6ZV9yYzZfb3B0aW9uKHN0cnVjdCBkcm1faTkxNV9wcml2YXRl ICpkZXZfcHJpdiwgaW50IGVuYWJsZV9yYzYpOwo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9k cm0vaTkxNS9pbnRlbF9wbS5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfcG0uYwo+IGlu ZGV4IDc3MTY2YzYuLmU4NjUzZDggMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2k5MTUv aW50ZWxfcG0uYwo+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX3BtLmMKPiBAQCAt Mzc3OSw2ICszNzc5LDM5IEBAIHN0YXRpYyB2b2lkIHNrbF9kZGJfZW50cnlfd3JpdGUoc3RydWN0 IGRybV9pOTE1X3ByaXZhdGUgKmRldl9wcml2LAo+ICAJCUk5MTVfV1JJVEUocmVnLCAwKTsKPiAg fQo+ICAKPiArdm9pZCBza2xfd3JpdGVfcGxhbmVfd20oc3RydWN0IGludGVsX2NydGMgKmludGVs X2NydGMsCj4gKwkJCWNvbnN0IHN0cnVjdCBza2xfd21fdmFsdWVzICp3bSwKPiArCQkJaW50IHBs YW5lKQo+ICt7Cj4gKwlzdHJ1Y3QgZHJtX2NydGMgKmNydGMgPSAmaW50ZWxfY3J0Yy0+YmFzZTsK PiArCXN0cnVjdCBkcm1fZGV2aWNlICpkZXYgPSBjcnRjLT5kZXY7Cj4gKwlzdHJ1Y3QgZHJtX2k5 MTVfcHJpdmF0ZSAqZGV2X3ByaXYgPSB0b19pOTE1KGRldik7Cj4gKwlpbnQgbGV2ZWwsIG1heF9s ZXZlbCA9IGlsa193bV9tYXhfbGV2ZWwoZGV2KTsKPiArCWVudW0gcGlwZSBwaXBlID0gaW50ZWxf Y3J0Yy0+cGlwZTsKPiArCj4gKwlmb3IgKGxldmVsID0gMDsgbGV2ZWwgPD0gbWF4X2xldmVsOyBs ZXZlbCsrKSB7Cj4gKwkJSTkxNV9XUklURShQTEFORV9XTShwaXBlLCBwbGFuZSwgbGV2ZWwpLAo+ ICsJCQkgICB3bS0+cGxhbmVbcGlwZV1bcGxhbmVdW2xldmVsXSk7Cj4gKwl9Cj4gKwlJOTE1X1dS SVRFKFBMQU5FX1dNX1RSQU5TKHBpcGUsIHBsYW5lKSwgd20tPnBsYW5lX3RyYW5zW3BpcGVdW3Bs YW5lXSk7Cj4gK30KPiArCj4gK3ZvaWQgc2tsX3dyaXRlX2N1cnNvcl93bShzdHJ1Y3QgaW50ZWxf Y3J0YyAqaW50ZWxfY3J0YywKPiArCQkJIGNvbnN0IHN0cnVjdCBza2xfd21fdmFsdWVzICp3bSkK PiArewo+ICsJc3RydWN0IGRybV9jcnRjICpjcnRjID0gJmludGVsX2NydGMtPmJhc2U7Cj4gKwlz dHJ1Y3QgZHJtX2RldmljZSAqZGV2ID0gY3J0Yy0+ZGV2Owo+ICsJc3RydWN0IGRybV9pOTE1X3By aXZhdGUgKmRldl9wcml2ID0gdG9faTkxNShkZXYpOwo+ICsJaW50IGxldmVsLCBtYXhfbGV2ZWwg PSBpbGtfd21fbWF4X2xldmVsKGRldik7Cj4gKwllbnVtIHBpcGUgcGlwZSA9IGludGVsX2NydGMt PnBpcGU7Cj4gKwo+ICsJZm9yIChsZXZlbCA9IDA7IGxldmVsIDw9IG1heF9sZXZlbDsgbGV2ZWwr Kykgewo+ICsJCUk5MTVfV1JJVEUoQ1VSX1dNKHBpcGUsIGxldmVsKSwKPiArCQkJICAgd20tPnBs YW5lW3BpcGVdW1BMQU5FX0NVUlNPUl1bbGV2ZWxdKTsKPiArCX0KPiArCUk5MTVfV1JJVEUoQ1VS X1dNX1RSQU5TKHBpcGUpLCB3bS0+cGxhbmVfdHJhbnNbcGlwZV1bUExBTkVfQ1VSU09SXSk7Cj4g K30KPiArCj4gIHN0YXRpYyB2b2lkIHNrbF93cml0ZV93bV92YWx1ZXMoc3RydWN0IGRybV9pOTE1 X3ByaXZhdGUgKmRldl9wcml2LAo+ICAJCQkJY29uc3Qgc3RydWN0IHNrbF93bV92YWx1ZXMgKm5l dykKPiAgewo+IEBAIC0zNzg2LDcgKzM4MTksNyBAQCBzdGF0aWMgdm9pZCBza2xfd3JpdGVfd21f dmFsdWVzKHN0cnVjdCBkcm1faTkxNV9wcml2YXRlICpkZXZfcHJpdiwKPiAgCXN0cnVjdCBpbnRl bF9jcnRjICpjcnRjOwo+ICAKPiAgCWZvcl9lYWNoX2ludGVsX2NydGMoZGV2LCBjcnRjKSB7Cj4g LQkJaW50IGksIGxldmVsLCBtYXhfbGV2ZWwgPSBpbGtfd21fbWF4X2xldmVsKGRldik7Cj4gKwkJ aW50IGk7Cj4gIAkJZW51bSBwaXBlIHBpcGUgPSBjcnRjLT5waXBlOwo+ICAKPiAgCQlpZiAoKG5l dy0+ZGlydHlfcGlwZXMgJiBkcm1fY3J0Y19tYXNrKCZjcnRjLT5iYXNlKSkgPT0gMCkKPiBAQCAt Mzc5NCwyMSArMzgyNyw2IEBAIHN0YXRpYyB2b2lkIHNrbF93cml0ZV93bV92YWx1ZXMoc3RydWN0 IGRybV9pOTE1X3ByaXZhdGUgKmRldl9wcml2LAo+ICAJCWlmICghY3J0Yy0+YWN0aXZlKQo+ICAJ CQljb250aW51ZTsKPiAgCj4gLQkJSTkxNV9XUklURShQSVBFX1dNX0xJTkVUSU1FKHBpcGUpLCBu ZXctPndtX2xpbmV0aW1lW3BpcGVdKTsKPiAtCj4gLQkJZm9yIChsZXZlbCA9IDA7IGxldmVsIDw9 IG1heF9sZXZlbDsgbGV2ZWwrKykgewo+IC0JCQlmb3IgKGkgPSAwOyBpIDwgaW50ZWxfbnVtX3Bs YW5lcyhjcnRjKTsgaSsrKQo+IC0JCQkJSTkxNV9XUklURShQTEFORV9XTShwaXBlLCBpLCBsZXZl bCksCj4gLQkJCQkJICAgbmV3LT5wbGFuZVtwaXBlXVtpXVtsZXZlbF0pOwo+IC0JCQlJOTE1X1dS SVRFKENVUl9XTShwaXBlLCBsZXZlbCksCj4gLQkJCQkgICBuZXctPnBsYW5lW3BpcGVdW1BMQU5F X0NVUlNPUl1bbGV2ZWxdKTsKPiAtCQl9Cj4gLQkJZm9yIChpID0gMDsgaSA8IGludGVsX251bV9w bGFuZXMoY3J0Yyk7IGkrKykKPiAtCQkJSTkxNV9XUklURShQTEFORV9XTV9UUkFOUyhwaXBlLCBp KSwKPiAtCQkJCSAgIG5ldy0+cGxhbmVfdHJhbnNbcGlwZV1baV0pOwo+IC0JCUk5MTVfV1JJVEUo Q1VSX1dNX1RSQU5TKHBpcGUpLAo+IC0JCQkgICBuZXctPnBsYW5lX3RyYW5zW3BpcGVdW1BMQU5F X0NVUlNPUl0pOwo+IC0KPiAgCQlmb3IgKGkgPSAwOyBpIDwgaW50ZWxfbnVtX3BsYW5lcyhjcnRj KTsgaSsrKSB7Cj4gIAkJCXNrbF9kZGJfZW50cnlfd3JpdGUoZGV2X3ByaXYsCj4gIAkJCQkJICAg IFBMQU5FX0JVRl9DRkcocGlwZSwgaSksCj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9p OTE1L2ludGVsX3Nwcml0ZS5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfc3ByaXRlLmMK PiBpbmRleCAzNjY5MDBkLi4zYzU2MGY5IDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9p OTE1L2ludGVsX3Nwcml0ZS5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfc3By aXRlLmMKPiBAQCAtMjAzLDYgKzIwMywxMCBAQCBza2xfdXBkYXRlX3BsYW5lKHN0cnVjdCBkcm1f cGxhbmUgKmRybV9wbGFuZSwKPiAgCXN0cnVjdCBkcm1faTkxNV9wcml2YXRlICpkZXZfcHJpdiA9 IHRvX2k5MTUoZGV2KTsKPiAgCXN0cnVjdCBpbnRlbF9wbGFuZSAqaW50ZWxfcGxhbmUgPSB0b19p bnRlbF9wbGFuZShkcm1fcGxhbmUpOwo+ICAJc3RydWN0IGRybV9mcmFtZWJ1ZmZlciAqZmIgPSBw bGFuZV9zdGF0ZS0+YmFzZS5mYjsKPiArCXN0cnVjdCBkcm1faTkxNV9nZW1fb2JqZWN0ICpvYmog PSBpbnRlbF9mYl9vYmooZmIpOwo+ICsJc3RydWN0IHNrbF93bV92YWx1ZXMgKndtID0gJmRldl9w cml2LT53bS5za2xfcmVzdWx0czsKPiArCXN0cnVjdCBkcm1fY3J0YyAqY3J0YyA9IGNydGNfc3Rh dGUtPmJhc2UuY3J0YzsKPiArCXN0cnVjdCBpbnRlbF9jcnRjICppbnRlbF9jcnRjID0gdG9faW50 ZWxfY3J0YyhjcnRjKTsKPiAgCWNvbnN0IGludCBwaXBlID0gaW50ZWxfcGxhbmUtPnBpcGU7Cj4g IAljb25zdCBpbnQgcGxhbmUgPSBpbnRlbF9wbGFuZS0+cGxhbmUgKyAxOwo+ICAJdTMyIHBsYW5l X2N0bDsKPiBAQCAtMjI4LDYgKzIzMiw5IEBAIHNrbF91cGRhdGVfcGxhbmUoc3RydWN0IGRybV9w bGFuZSAqZHJtX3BsYW5lLAo+ICAKPiAgCXBsYW5lX2N0bCB8PSBza2xfcGxhbmVfY3RsX3JvdGF0 aW9uKHJvdGF0aW9uKTsKPiAgCj4gKwlpZiAod20tPmRpcnR5X3BpcGVzICYgZHJtX2NydGNfbWFz ayhjcnRjKSkKPiArCSAgICBza2xfd3JpdGVfcGxhbmVfd20oaW50ZWxfY3J0Yywgd20sIHBsYW5l KTsKPiArCj4gIAlpZiAoa2V5LT5mbGFncykgewo+ICAJCUk5MTVfV1JJVEUoUExBTkVfS0VZVkFM KHBpcGUsIHBsYW5lKSwga2V5LT5taW5fdmFsdWUpOwo+ICAJCUk5MTVfV1JJVEUoUExBTkVfS0VZ TUFYKHBpcGUsIHBsYW5lKSwga2V5LT5tYXhfdmFsdWUpOwo+IC0tIAo+IDIuNy40CgotLSAKVmls bGUgU3lyasOkbMOkCkludGVsIE9UQwpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBsaXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVl ZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5m by9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752881AbcHLMnD (ORCPT ); Fri, 12 Aug 2016 08:43:03 -0400 Received: from mga03.intel.com ([134.134.136.65]:47987 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752621AbcHLMmu (ORCPT ); Fri, 12 Aug 2016 08:42:50 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,510,1464678000"; d="scan'208";a="1013222231" Date: Fri, 12 Aug 2016 15:42:28 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Lyude Cc: intel-gfx@lists.freedesktop.org, Maarten Lankhorst , Matt Roper , stable@vger.kernel.org, Daniel Vetter , Radhakrishna Sripada , Hans de Goede , Jani Nikula , David Airlie , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v11 4/7] drm/i915/skl: Update plane watermarks atomically during plane updates Message-ID: <20160812124228.GA4329@intel.com> References: <1470945277-7973-1-git-send-email-cpaul@redhat.com> <1470945277-7973-5-git-send-email-cpaul@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1470945277-7973-5-git-send-email-cpaul@redhat.com> User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, Aug 11, 2016 at 03:54:33PM -0400, Lyude wrote: > Thanks to Ville for suggesting this as a potential solution to pipe > underruns on Skylake. > > On Skylake all of the registers for configuring planes, including the > registers for configuring their watermarks, are double buffered. New > values written to them won't take effect until said registers are > "armed", which is done by writing to the PLANE_SURF (or in the case of > cursor planes, the CURBASE register) register. > > With this in mind, up until now we've been updating watermarks on skl > like this: > > non-modeset { > - calculate (during atomic check phase) > - finish_atomic_commit: > - intel_pre_plane_update: > - intel_update_watermarks() > - {vblank happens; new watermarks + old plane values => underrun } > - drm_atomic_helper_commit_planes_on_crtc: > - start vblank evasion > - write new plane registers > - end vblank evasion > } > > or > > modeset { > - calculate (during atomic check phase) > - finish_atomic_commit: > - crtc_enable: > - intel_update_watermarks() > - {vblank happens; new watermarks + old plane values => underrun } > - drm_atomic_helper_commit_planes_on_crtc: > - start vblank evasion > - write new plane registers > - end vblank evasion > } > > Now we update watermarks atomically like this: > > non-modeset { > - calculate (during atomic check phase) > - finish_atomic_commit: > - intel_pre_plane_update: > - intel_update_watermarks() (wm values aren't written yet) > - drm_atomic_helper_commit_planes_on_crtc: > - start vblank evasion > - write new plane registers > - write new wm values > - end vblank evasion > } > > modeset { > - calculate (during atomic check phase) > - finish_atomic_commit: > - crtc_enable: > - intel_update_watermarks() (actual wm values aren't written > yet) > - drm_atomic_helper_commit_planes_on_crtc: > - start vblank evasion > - write new plane registers > - write new wm values > - end vblank evasion > } > > So this patch moves all of the watermark writes into the right place; > inside of the vblank evasion where we update all of the registers for > each plane. While this patch doesn't fix everything, it does allow us to > update the watermark values in the way the hardware expects us to. > > Changes since original patch series: > - Remove mutex_lock/mutex_unlock since they don't do anything and we're > not touching global state > - Move skl_write_cursor_wm/skl_write_plane_wm functions into > intel_pm.c, make externally visible > - Add skl_write_plane_wm calls to skl_update_plane > - Fix conditional for for loop in skl_write_plane_wm (level < max_level > should be level <= max_level) > - Make diagram in commit more accurate to what's actually happening > - Add Fixes: > > Changes since v1: > - Use IS_GEN9() instead of IS_SKYLAKE() since these fixes apply to more > then just Skylake > - Update description to make it clear this patch doesn't fix everything > - Check if pipes were actually changed before writing watermarks > > Changes since v2: > - Write PIPE_WM_LINETIME during vblank evasion > > Changes since v3: > - Rebase against new SAGV patch changes > > Changes since v4: > - Add a parameter to choose what skl_wm_values struct to use when > writing new plane watermarks > > Changes since v5: > - Remove cursor ddb entry write in skl_write_cursor_wm(), defer until > patch 6 > - Write WM_LINETIME in intel_begin_crtc_commit() > > Changes since v6: > - Remove redundant dirty_pipes check in skl_write_plane_wm (we check > this in all places where we call this function, and it was supposed > to have been removed earlier anyway) > - In i9xx_update_cursor(), use dev_priv->info.gen >= 9 instead of > IS_GEN9(dev_priv). We do this everywhere else and I'd imagine this > needs to be done for gen10 as well > > Fixes: 2d41c0b59afc ("drm/i915/skl: SKL Watermark Computation") > Signed-off-by: Lyude > Reviewed-by: Matt Roper > Cc: stable@vger.kernel.org > Cc: Ville Syrjälä > Cc: Daniel Vetter > Cc: Radhakrishna Sripada > Cc: Hans de Goede > --- > drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++- > drivers/gpu/drm/i915/intel_drv.h | 5 ++++ > drivers/gpu/drm/i915/intel_pm.c | 50 ++++++++++++++++++++++++------------ > drivers/gpu/drm/i915/intel_sprite.c | 7 +++++ > 4 files changed, 62 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 35bdd67..b2f8e24 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3386,6 +3386,8 @@ static void skylake_update_primary_plane(struct drm_plane *plane, > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc); > struct drm_framebuffer *fb = plane_state->base.fb; > + struct drm_i915_gem_object *obj = intel_fb_obj(fb); rebase fail? Please double check with gcc/sparse that new warnings are kept to a minimum. > + struct skl_wm_values *wm = &dev_priv->wm.skl_results; const? > int pipe = intel_crtc->pipe; > u32 plane_ctl; > unsigned int rotation = plane_state->base.rotation; > @@ -3419,6 +3421,9 @@ static void skylake_update_primary_plane(struct drm_plane *plane, > intel_crtc->adjusted_x = src_x; > intel_crtc->adjusted_y = src_y; > > + if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base)) > + skl_write_plane_wm(intel_crtc, wm, 0); Indent fail. > + > I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl); > I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x); > I915_WRITE(PLANE_STRIDE(pipe, 0), stride); > @@ -10699,9 +10704,13 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct skl_wm_values *wm = &dev_priv->wm.skl_results; > int pipe = intel_crtc->pipe; > uint32_t cntl = 0; > > + if (dev_priv->info.gen >= 9 && wm->dirty_pipes & drm_crtc_mask(crtc) INTEL_GEN() > + skl_write_cursor_wm(intel_crtc, wm); > + > if (plane_state && plane_state->base.visible) { > cntl = MCURSOR_GAMMA_ENABLE; > switch (plane_state->base.crtc_w) { > @@ -14610,10 +14619,12 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc, > struct drm_crtc_state *old_crtc_state) > { > struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_crtc_state *old_intel_state = > to_intel_crtc_state(old_crtc_state); > bool modeset = needs_modeset(crtc->state); > + enum pipe pipe = intel_crtc->pipe; > > /* Perform vblank evasion around commit operation */ > intel_pipe_update_start(intel_crtc); > @@ -14628,8 +14639,12 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc, > > if (to_intel_crtc_state(crtc->state)->update_pipe) > intel_update_pipe_config(intel_crtc, old_intel_state); > - else if (INTEL_INFO(dev)->gen >= 9) > + else if (INTEL_INFO(dev)->gen >= 9) { > skl_detach_scalers(intel_crtc); > + > + I915_WRITE(PIPE_WM_LINETIME(pipe), > + dev_priv->wm.skl_hw.wm_linetime[pipe]); > + } > } > > static void intel_finish_crtc_commit(struct drm_crtc *crtc, > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 76e78b8..88088c3 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1723,6 +1723,11 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv, > struct skl_ddb_allocation *ddb /* out */); > int skl_enable_sagv(struct drm_i915_private *dev_priv); > int skl_disable_sagv(struct drm_i915_private *dev_priv); > +void skl_write_cursor_wm(struct intel_crtc *intel_crtc, > + const struct skl_wm_values *wm); > +void skl_write_plane_wm(struct intel_crtc *intel_crtc, > + const struct skl_wm_values *wm, > + int plane); > uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config); > bool ilk_disable_lp_wm(struct drm_device *dev); > int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 77166c6..e8653d8 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3779,6 +3779,39 @@ static void skl_ddb_entry_write(struct drm_i915_private *dev_priv, > I915_WRITE(reg, 0); > } > > +void skl_write_plane_wm(struct intel_crtc *intel_crtc, > + const struct skl_wm_values *wm, > + int plane) > +{ > + struct drm_crtc *crtc = &intel_crtc->base; > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > + int level, max_level = ilk_wm_max_level(dev); > + enum pipe pipe = intel_crtc->pipe; > + > + for (level = 0; level <= max_level; level++) { > + I915_WRITE(PLANE_WM(pipe, plane, level), > + wm->plane[pipe][plane][level]); > + } > + I915_WRITE(PLANE_WM_TRANS(pipe, plane), wm->plane_trans[pipe][plane]); > +} > + > +void skl_write_cursor_wm(struct intel_crtc *intel_crtc, > + const struct skl_wm_values *wm) > +{ > + struct drm_crtc *crtc = &intel_crtc->base; > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > + int level, max_level = ilk_wm_max_level(dev); > + enum pipe pipe = intel_crtc->pipe; > + > + for (level = 0; level <= max_level; level++) { > + I915_WRITE(CUR_WM(pipe, level), > + wm->plane[pipe][PLANE_CURSOR][level]); > + } > + I915_WRITE(CUR_WM_TRANS(pipe), wm->plane_trans[pipe][PLANE_CURSOR]); > +} > + > static void skl_write_wm_values(struct drm_i915_private *dev_priv, > const struct skl_wm_values *new) > { > @@ -3786,7 +3819,7 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv, > struct intel_crtc *crtc; > > for_each_intel_crtc(dev, crtc) { > - int i, level, max_level = ilk_wm_max_level(dev); > + int i; > enum pipe pipe = crtc->pipe; > > if ((new->dirty_pipes & drm_crtc_mask(&crtc->base)) == 0) > @@ -3794,21 +3827,6 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv, > if (!crtc->active) > continue; > > - I915_WRITE(PIPE_WM_LINETIME(pipe), new->wm_linetime[pipe]); > - > - for (level = 0; level <= max_level; level++) { > - for (i = 0; i < intel_num_planes(crtc); i++) > - I915_WRITE(PLANE_WM(pipe, i, level), > - new->plane[pipe][i][level]); > - I915_WRITE(CUR_WM(pipe, level), > - new->plane[pipe][PLANE_CURSOR][level]); > - } > - for (i = 0; i < intel_num_planes(crtc); i++) > - I915_WRITE(PLANE_WM_TRANS(pipe, i), > - new->plane_trans[pipe][i]); > - I915_WRITE(CUR_WM_TRANS(pipe), > - new->plane_trans[pipe][PLANE_CURSOR]); > - > for (i = 0; i < intel_num_planes(crtc); i++) { > skl_ddb_entry_write(dev_priv, > PLANE_BUF_CFG(pipe, i), > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 366900d..3c560f9 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -203,6 +203,10 @@ skl_update_plane(struct drm_plane *drm_plane, > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_plane *intel_plane = to_intel_plane(drm_plane); > struct drm_framebuffer *fb = plane_state->base.fb; > + struct drm_i915_gem_object *obj = intel_fb_obj(fb); > + struct skl_wm_values *wm = &dev_priv->wm.skl_results; > + struct drm_crtc *crtc = crtc_state->base.crtc; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > const int pipe = intel_plane->pipe; > const int plane = intel_plane->plane + 1; > u32 plane_ctl; > @@ -228,6 +232,9 @@ skl_update_plane(struct drm_plane *drm_plane, > > plane_ctl |= skl_plane_ctl_rotation(rotation); > > + if (wm->dirty_pipes & drm_crtc_mask(crtc)) > + skl_write_plane_wm(intel_crtc, wm, plane); > + > if (key->flags) { > I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value); > I915_WRITE(PLANE_KEYMAX(pipe, plane), key->max_value); > -- > 2.7.4 -- Ville Syrjälä Intel OTC From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com ([134.134.136.65]:47987 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752621AbcHLMmu (ORCPT ); Fri, 12 Aug 2016 08:42:50 -0400 Date: Fri, 12 Aug 2016 15:42:28 +0300 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= To: Lyude Cc: intel-gfx@lists.freedesktop.org, Maarten Lankhorst , Matt Roper , stable@vger.kernel.org, Daniel Vetter , Radhakrishna Sripada , Hans de Goede , Jani Nikula , David Airlie , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH v11 4/7] drm/i915/skl: Update plane watermarks atomically during plane updates Message-ID: <20160812124228.GA4329@intel.com> References: <1470945277-7973-1-git-send-email-cpaul@redhat.com> <1470945277-7973-5-git-send-email-cpaul@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1470945277-7973-5-git-send-email-cpaul@redhat.com> Sender: stable-owner@vger.kernel.org List-ID: On Thu, Aug 11, 2016 at 03:54:33PM -0400, Lyude wrote: > Thanks to Ville for suggesting this as a potential solution to pipe > underruns on Skylake. > > On Skylake all of the registers for configuring planes, including the > registers for configuring their watermarks, are double buffered. New > values written to them won't take effect until said registers are > "armed", which is done by writing to the PLANE_SURF (or in the case of > cursor planes, the CURBASE register) register. > > With this in mind, up until now we've been updating watermarks on skl > like this: > > non-modeset { > - calculate (during atomic check phase) > - finish_atomic_commit: > - intel_pre_plane_update: > - intel_update_watermarks() > - {vblank happens; new watermarks + old plane values => underrun } > - drm_atomic_helper_commit_planes_on_crtc: > - start vblank evasion > - write new plane registers > - end vblank evasion > } > > or > > modeset { > - calculate (during atomic check phase) > - finish_atomic_commit: > - crtc_enable: > - intel_update_watermarks() > - {vblank happens; new watermarks + old plane values => underrun } > - drm_atomic_helper_commit_planes_on_crtc: > - start vblank evasion > - write new plane registers > - end vblank evasion > } > > Now we update watermarks atomically like this: > > non-modeset { > - calculate (during atomic check phase) > - finish_atomic_commit: > - intel_pre_plane_update: > - intel_update_watermarks() (wm values aren't written yet) > - drm_atomic_helper_commit_planes_on_crtc: > - start vblank evasion > - write new plane registers > - write new wm values > - end vblank evasion > } > > modeset { > - calculate (during atomic check phase) > - finish_atomic_commit: > - crtc_enable: > - intel_update_watermarks() (actual wm values aren't written > yet) > - drm_atomic_helper_commit_planes_on_crtc: > - start vblank evasion > - write new plane registers > - write new wm values > - end vblank evasion > } > > So this patch moves all of the watermark writes into the right place; > inside of the vblank evasion where we update all of the registers for > each plane. While this patch doesn't fix everything, it does allow us to > update the watermark values in the way the hardware expects us to. > > Changes since original patch series: > - Remove mutex_lock/mutex_unlock since they don't do anything and we're > not touching global state > - Move skl_write_cursor_wm/skl_write_plane_wm functions into > intel_pm.c, make externally visible > - Add skl_write_plane_wm calls to skl_update_plane > - Fix conditional for for loop in skl_write_plane_wm (level < max_level > should be level <= max_level) > - Make diagram in commit more accurate to what's actually happening > - Add Fixes: > > Changes since v1: > - Use IS_GEN9() instead of IS_SKYLAKE() since these fixes apply to more > then just Skylake > - Update description to make it clear this patch doesn't fix everything > - Check if pipes were actually changed before writing watermarks > > Changes since v2: > - Write PIPE_WM_LINETIME during vblank evasion > > Changes since v3: > - Rebase against new SAGV patch changes > > Changes since v4: > - Add a parameter to choose what skl_wm_values struct to use when > writing new plane watermarks > > Changes since v5: > - Remove cursor ddb entry write in skl_write_cursor_wm(), defer until > patch 6 > - Write WM_LINETIME in intel_begin_crtc_commit() > > Changes since v6: > - Remove redundant dirty_pipes check in skl_write_plane_wm (we check > this in all places where we call this function, and it was supposed > to have been removed earlier anyway) > - In i9xx_update_cursor(), use dev_priv->info.gen >= 9 instead of > IS_GEN9(dev_priv). We do this everywhere else and I'd imagine this > needs to be done for gen10 as well > > Fixes: 2d41c0b59afc ("drm/i915/skl: SKL Watermark Computation") > Signed-off-by: Lyude > Reviewed-by: Matt Roper > Cc: stable@vger.kernel.org > Cc: Ville Syrj�l� > Cc: Daniel Vetter > Cc: Radhakrishna Sripada > Cc: Hans de Goede > --- > drivers/gpu/drm/i915/intel_display.c | 17 +++++++++++- > drivers/gpu/drm/i915/intel_drv.h | 5 ++++ > drivers/gpu/drm/i915/intel_pm.c | 50 ++++++++++++++++++++++++------------ > drivers/gpu/drm/i915/intel_sprite.c | 7 +++++ > 4 files changed, 62 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 35bdd67..b2f8e24 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -3386,6 +3386,8 @@ static void skylake_update_primary_plane(struct drm_plane *plane, > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *intel_crtc = to_intel_crtc(crtc_state->base.crtc); > struct drm_framebuffer *fb = plane_state->base.fb; > + struct drm_i915_gem_object *obj = intel_fb_obj(fb); rebase fail? Please double check with gcc/sparse that new warnings are kept to a minimum. > + struct skl_wm_values *wm = &dev_priv->wm.skl_results; const? > int pipe = intel_crtc->pipe; > u32 plane_ctl; > unsigned int rotation = plane_state->base.rotation; > @@ -3419,6 +3421,9 @@ static void skylake_update_primary_plane(struct drm_plane *plane, > intel_crtc->adjusted_x = src_x; > intel_crtc->adjusted_y = src_y; > > + if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base)) > + skl_write_plane_wm(intel_crtc, wm, 0); Indent fail. > + > I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl); > I915_WRITE(PLANE_OFFSET(pipe, 0), (src_y << 16) | src_x); > I915_WRITE(PLANE_STRIDE(pipe, 0), stride); > @@ -10699,9 +10704,13 @@ static void i9xx_update_cursor(struct drm_crtc *crtc, u32 base, > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > + struct skl_wm_values *wm = &dev_priv->wm.skl_results; > int pipe = intel_crtc->pipe; > uint32_t cntl = 0; > > + if (dev_priv->info.gen >= 9 && wm->dirty_pipes & drm_crtc_mask(crtc) INTEL_GEN() > + skl_write_cursor_wm(intel_crtc, wm); > + > if (plane_state && plane_state->base.visible) { > cntl = MCURSOR_GAMMA_ENABLE; > switch (plane_state->base.crtc_w) { > @@ -14610,10 +14619,12 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc, > struct drm_crtc_state *old_crtc_state) > { > struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_crtc_state *old_intel_state = > to_intel_crtc_state(old_crtc_state); > bool modeset = needs_modeset(crtc->state); > + enum pipe pipe = intel_crtc->pipe; > > /* Perform vblank evasion around commit operation */ > intel_pipe_update_start(intel_crtc); > @@ -14628,8 +14639,12 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc, > > if (to_intel_crtc_state(crtc->state)->update_pipe) > intel_update_pipe_config(intel_crtc, old_intel_state); > - else if (INTEL_INFO(dev)->gen >= 9) > + else if (INTEL_INFO(dev)->gen >= 9) { > skl_detach_scalers(intel_crtc); > + > + I915_WRITE(PIPE_WM_LINETIME(pipe), > + dev_priv->wm.skl_hw.wm_linetime[pipe]); > + } > } > > static void intel_finish_crtc_commit(struct drm_crtc *crtc, > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 76e78b8..88088c3 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1723,6 +1723,11 @@ void skl_ddb_get_hw_state(struct drm_i915_private *dev_priv, > struct skl_ddb_allocation *ddb /* out */); > int skl_enable_sagv(struct drm_i915_private *dev_priv); > int skl_disable_sagv(struct drm_i915_private *dev_priv); > +void skl_write_cursor_wm(struct intel_crtc *intel_crtc, > + const struct skl_wm_values *wm); > +void skl_write_plane_wm(struct intel_crtc *intel_crtc, > + const struct skl_wm_values *wm, > + int plane); > uint32_t ilk_pipe_pixel_rate(const struct intel_crtc_state *pipe_config); > bool ilk_disable_lp_wm(struct drm_device *dev); > int sanitize_rc6_option(struct drm_i915_private *dev_priv, int enable_rc6); > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c > index 77166c6..e8653d8 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3779,6 +3779,39 @@ static void skl_ddb_entry_write(struct drm_i915_private *dev_priv, > I915_WRITE(reg, 0); > } > > +void skl_write_plane_wm(struct intel_crtc *intel_crtc, > + const struct skl_wm_values *wm, > + int plane) > +{ > + struct drm_crtc *crtc = &intel_crtc->base; > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > + int level, max_level = ilk_wm_max_level(dev); > + enum pipe pipe = intel_crtc->pipe; > + > + for (level = 0; level <= max_level; level++) { > + I915_WRITE(PLANE_WM(pipe, plane, level), > + wm->plane[pipe][plane][level]); > + } > + I915_WRITE(PLANE_WM_TRANS(pipe, plane), wm->plane_trans[pipe][plane]); > +} > + > +void skl_write_cursor_wm(struct intel_crtc *intel_crtc, > + const struct skl_wm_values *wm) > +{ > + struct drm_crtc *crtc = &intel_crtc->base; > + struct drm_device *dev = crtc->dev; > + struct drm_i915_private *dev_priv = to_i915(dev); > + int level, max_level = ilk_wm_max_level(dev); > + enum pipe pipe = intel_crtc->pipe; > + > + for (level = 0; level <= max_level; level++) { > + I915_WRITE(CUR_WM(pipe, level), > + wm->plane[pipe][PLANE_CURSOR][level]); > + } > + I915_WRITE(CUR_WM_TRANS(pipe), wm->plane_trans[pipe][PLANE_CURSOR]); > +} > + > static void skl_write_wm_values(struct drm_i915_private *dev_priv, > const struct skl_wm_values *new) > { > @@ -3786,7 +3819,7 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv, > struct intel_crtc *crtc; > > for_each_intel_crtc(dev, crtc) { > - int i, level, max_level = ilk_wm_max_level(dev); > + int i; > enum pipe pipe = crtc->pipe; > > if ((new->dirty_pipes & drm_crtc_mask(&crtc->base)) == 0) > @@ -3794,21 +3827,6 @@ static void skl_write_wm_values(struct drm_i915_private *dev_priv, > if (!crtc->active) > continue; > > - I915_WRITE(PIPE_WM_LINETIME(pipe), new->wm_linetime[pipe]); > - > - for (level = 0; level <= max_level; level++) { > - for (i = 0; i < intel_num_planes(crtc); i++) > - I915_WRITE(PLANE_WM(pipe, i, level), > - new->plane[pipe][i][level]); > - I915_WRITE(CUR_WM(pipe, level), > - new->plane[pipe][PLANE_CURSOR][level]); > - } > - for (i = 0; i < intel_num_planes(crtc); i++) > - I915_WRITE(PLANE_WM_TRANS(pipe, i), > - new->plane_trans[pipe][i]); > - I915_WRITE(CUR_WM_TRANS(pipe), > - new->plane_trans[pipe][PLANE_CURSOR]); > - > for (i = 0; i < intel_num_planes(crtc); i++) { > skl_ddb_entry_write(dev_priv, > PLANE_BUF_CFG(pipe, i), > diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c > index 366900d..3c560f9 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -203,6 +203,10 @@ skl_update_plane(struct drm_plane *drm_plane, > struct drm_i915_private *dev_priv = to_i915(dev); > struct intel_plane *intel_plane = to_intel_plane(drm_plane); > struct drm_framebuffer *fb = plane_state->base.fb; > + struct drm_i915_gem_object *obj = intel_fb_obj(fb); > + struct skl_wm_values *wm = &dev_priv->wm.skl_results; > + struct drm_crtc *crtc = crtc_state->base.crtc; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > const int pipe = intel_plane->pipe; > const int plane = intel_plane->plane + 1; > u32 plane_ctl; > @@ -228,6 +232,9 @@ skl_update_plane(struct drm_plane *drm_plane, > > plane_ctl |= skl_plane_ctl_rotation(rotation); > > + if (wm->dirty_pipes & drm_crtc_mask(crtc)) > + skl_write_plane_wm(intel_crtc, wm, plane); > + > if (key->flags) { > I915_WRITE(PLANE_KEYVAL(pipe, plane), key->min_value); > I915_WRITE(PLANE_KEYMAX(pipe, plane), key->max_value); > -- > 2.7.4 -- Ville Syrj�l� Intel OTC