From mboxrd@z Thu Jan 1 00:00:00 1970 From: Matt Roper Subject: Re: [PATCH v5 3/6] drm/i915/skl: Update plane watermarks atomically during plane updates Date: Tue, 2 Aug 2016 14:20:33 -0700 Message-ID: <20160802212033.GY32025@intel.com> References: <1470163975-30467-1-git-send-email-cpaul@redhat.com> <1470163975-30467-4-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: <1470163975-30467-4-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 T24gVHVlLCBBdWcgMDIsIDIwMTYgYXQgMDI6NTI6NTFQTSAtMDQwMCwgTHl1ZGUgd3JvdGU6Cj4g 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 PiAgICB3cml0aW5nIG5ldyBwbGFuZSB3YXRlcm1hcmtzCj4gCj4gRml4ZXM6IDJkNDFjMGI1OWFm YyAoImRybS9pOTE1L3NrbDogU0tMIFdhdGVybWFyayBDb21wdXRhdGlvbiIpCj4gU2lnbmVkLW9m Zi1ieTogTHl1ZGUgPGNwYXVsQHJlZGhhdC5jb20+Cj4gQ2M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5v cmcKPiBDYzogVmlsbGUgU3lyasOkbMOkIDx2aWxsZS5zeXJqYWxhQGxpbnV4LmludGVsLmNvbT4K PiBDYzogRGFuaWVsIFZldHRlciA8ZGFuaWVsLnZldHRlckBpbnRlbC5jb20+Cj4gQ2M6IFJhZGhh a3Jpc2huYSBTcmlwYWRhIDxyYWRoYWtyaXNobmEuc3JpcGFkYUBpbnRlbC5jb20+Cj4gQ2M6IEhh bnMgZGUgR29lZGUgPGhkZWdvZWRlQHJlZGhhdC5jb20+Cj4gQ2M6IE1hdHQgUm9wZXIgPG1hdHRo ZXcuZC5yb3BlckBpbnRlbC5jb20+Cj4gLS0tCj4gIGRyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVs X2Rpc3BsYXkuYyB8ICA4ICsrKysrCj4gIGRyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2Rydi5o ICAgICB8ICA1ICsrKysKPiAgZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfcG0uYyAgICAgIHwg NTggKysrKysrKysrKysrKysrKysrKysrKysrKystLS0tLS0tLS0tCj4gIGRyaXZlcnMvZ3B1L2Ry bS9pOTE1L2ludGVsX3Nwcml0ZS5jICB8ICA2ICsrKysKPiAgNCBmaWxlcyBjaGFuZ2VkLCA2MSBp bnNlcnRpb25zKCspLCAxNiBkZWxldGlvbnMoLSkKPiAKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9n cHUvZHJtL2k5MTUvaW50ZWxfZGlzcGxheS5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxf ZGlzcGxheS5jCj4gaW5kZXggNzZiYTc5Zi4uNzlkMTQ2YyAxMDA2NDQKPiAtLS0gYS9kcml2ZXJz L2dwdS9kcm0vaTkxNS9pbnRlbF9kaXNwbGF5LmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vaTkx NS9pbnRlbF9kaXNwbGF5LmMKPiBAQCAtMjk4MCw2ICsyOTgwLDcgQEAgc3RhdGljIHZvaWQgc2t5 bGFrZV91cGRhdGVfcHJpbWFyeV9wbGFuZShzdHJ1Y3QgZHJtX3BsYW5lICpwbGFuZSwKPiAgCXN0 cnVjdCBpbnRlbF9jcnRjICppbnRlbF9jcnRjID0gdG9faW50ZWxfY3J0YyhjcnRjX3N0YXRlLT5i YXNlLmNydGMpOwo+ICAJc3RydWN0IGRybV9mcmFtZWJ1ZmZlciAqZmIgPSBwbGFuZV9zdGF0ZS0+ YmFzZS5mYjsKPiAgCXN0cnVjdCBkcm1faTkxNV9nZW1fb2JqZWN0ICpvYmogPSBpbnRlbF9mYl9v YmooZmIpOwo+ICsJc3RydWN0IHNrbF93bV92YWx1ZXMgKndtID0gJmRldl9wcml2LT53bS5za2xf cmVzdWx0czsKPiAgCWludCBwaXBlID0gaW50ZWxfY3J0Yy0+cGlwZTsKPiAgCXUzMiBwbGFuZV9j dGwsIHN0cmlkZV9kaXYsIHN0cmlkZTsKPiAgCXUzMiB0aWxlX2hlaWdodCwgcGxhbmVfb2Zmc2V0 LCBwbGFuZV9zaXplOwo+IEBAIC0zMDMxLDYgKzMwMzIsOSBAQCBzdGF0aWMgdm9pZCBza3lsYWtl X3VwZGF0ZV9wcmltYXJ5X3BsYW5lKHN0cnVjdCBkcm1fcGxhbmUgKnBsYW5lLAo+ICAJaW50ZWxf Y3J0Yy0+YWRqdXN0ZWRfeCA9IHhfb2Zmc2V0Owo+ICAJaW50ZWxfY3J0Yy0+YWRqdXN0ZWRfeSA9 IHlfb2Zmc2V0Owo+ICAKPiArCWlmICh3bS0+ZGlydHlfcGlwZXMgJiBkcm1fY3J0Y19tYXNrKCZp bnRlbF9jcnRjLT5iYXNlKSkKPiArCSAgICBza2xfd3JpdGVfcGxhbmVfd20oaW50ZWxfY3J0Yywg d20sIDApOwo+ICsKPiAgCUk5MTVfV1JJVEUoUExBTkVfQ1RMKHBpcGUsIDApLCBwbGFuZV9jdGwp Owo+ICAJSTkxNV9XUklURShQTEFORV9PRkZTRVQocGlwZSwgMCksIHBsYW5lX29mZnNldCk7Cj4g IAlJOTE1X1dSSVRFKFBMQU5FX1NJWkUocGlwZSwgMCksIHBsYW5lX3NpemUpOwo+IEBAIC0xMDI0 Myw5ICsxMDI0NywxMyBAQCBzdGF0aWMgdm9pZCBpOXh4X3VwZGF0ZV9jdXJzb3Ioc3RydWN0IGRy bV9jcnRjICpjcnRjLCB1MzIgYmFzZSwKPiAgCXN0cnVjdCBkcm1fZGV2aWNlICpkZXYgPSBjcnRj LT5kZXY7Cj4gIAlzdHJ1Y3QgZHJtX2k5MTVfcHJpdmF0ZSAqZGV2X3ByaXYgPSB0b19pOTE1KGRl dik7Cj4gIAlzdHJ1Y3QgaW50ZWxfY3J0YyAqaW50ZWxfY3J0YyA9IHRvX2ludGVsX2NydGMoY3J0 Yyk7Cj4gKwlzdHJ1Y3Qgc2tsX3dtX3ZhbHVlcyAqd20gPSAmZGV2X3ByaXYtPndtLnNrbF9yZXN1 bHRzOwo+ICAJaW50IHBpcGUgPSBpbnRlbF9jcnRjLT5waXBlOwo+ICAJdWludDMyX3QgY250bCA9 IDA7Cj4gIAo+ICsJaWYgKElTX0dFTjkoZGV2X3ByaXYpICYmIHdtLT5kaXJ0eV9waXBlcyAmIGRy bV9jcnRjX21hc2soY3J0YykpCj4gKwkJc2tsX3dyaXRlX2N1cnNvcl93bShpbnRlbF9jcnRjLCB3 bSk7Cj4gKwo+ICAJaWYgKHBsYW5lX3N0YXRlICYmIHBsYW5lX3N0YXRlLT52aXNpYmxlKSB7Cj4g IAkJY250bCA9IE1DVVJTT1JfR0FNTUFfRU5BQkxFOwo+ICAJCXN3aXRjaCAocGxhbmVfc3RhdGUt PmJhc2UuY3J0Y193KSB7Cj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVs X2Rydi5oIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHJ2LmgKPiBpbmRleCA2YjA1MzJh Li4xYjQ0NGQzIDEwMDY0NAo+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2Rydi5o Cj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZHJ2LmgKPiBAQCAtMTcxMSw2ICsx NzExLDExIEBAIHZvaWQgc2tsX2RkYl9nZXRfaHdfc3RhdGUoc3RydWN0IGRybV9pOTE1X3ByaXZh dGUgKmRldl9wcml2LAo+ICAJCQkgIHN0cnVjdCBza2xfZGRiX2FsbG9jYXRpb24gKmRkYiAvKiBv dXQgKi8pOwo+ICBpbnQgc2tsX2VuYWJsZV9zYWd2KHN0cnVjdCBkcm1faTkxNV9wcml2YXRlICpk ZXZfcHJpdik7Cj4gIGludCBza2xfZGlzYWJsZV9zYWd2KHN0cnVjdCBkcm1faTkxNV9wcml2YXRl ICpkZXZfcHJpdik7Cj4gK3ZvaWQgc2tsX3dyaXRlX2N1cnNvcl93bShzdHJ1Y3QgaW50ZWxfY3J0 YyAqaW50ZWxfY3J0YywKPiArCQkJIGNvbnN0IHN0cnVjdCBza2xfd21fdmFsdWVzICp3bSk7Cj4g K3ZvaWQgc2tsX3dyaXRlX3BsYW5lX3dtKHN0cnVjdCBpbnRlbF9jcnRjICppbnRlbF9jcnRjLAo+ ICsJCQljb25zdCBzdHJ1Y3Qgc2tsX3dtX3ZhbHVlcyAqd20sCj4gKwkJCWludCBwbGFuZSk7Cj4g IHVpbnQzMl90IGlsa19waXBlX3BpeGVsX3JhdGUoY29uc3Qgc3RydWN0IGludGVsX2NydGNfc3Rh dGUgKnBpcGVfY29uZmlnKTsKPiAgYm9vbCBpbGtfZGlzYWJsZV9scF93bShzdHJ1Y3QgZHJtX2Rl dmljZSAqZGV2KTsKPiAgaW50IHNhbml0aXplX3JjNl9vcHRpb24oc3RydWN0IGRybV9pOTE1X3By aXZhdGUgKmRldl9wcml2LCBpbnQgZW5hYmxlX3JjNik7Cj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMv Z3B1L2RybS9pOTE1L2ludGVsX3BtLmMgYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRlbF9wbS5j Cj4gaW5kZXggN2ZkMjk5ZS4uNTNhZGNiZiAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0v aTkxNS9pbnRlbF9wbS5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfcG0uYwo+ IEBAIC0zNzk4LDYgKzM3OTgsNDcgQEAgc3RhdGljIHZvaWQgc2tsX2RkYl9lbnRyeV93cml0ZShz dHJ1Y3QgZHJtX2k5MTVfcHJpdmF0ZSAqZGV2X3ByaXYsCj4gIAkJSTkxNV9XUklURShyZWcsIDAp Owo+ICB9Cj4gIAo+ICt2b2lkIHNrbF93cml0ZV9wbGFuZV93bShzdHJ1Y3QgaW50ZWxfY3J0YyAq aW50ZWxfY3J0YywKPiArCQkJY29uc3Qgc3RydWN0IHNrbF93bV92YWx1ZXMgKndtLAo+ICsJCQlp bnQgcGxhbmUpCj4gK3sKPiArCXN0cnVjdCBkcm1fY3J0YyAqY3J0YyA9ICZpbnRlbF9jcnRjLT5i YXNlOwo+ICsJc3RydWN0IGRybV9kZXZpY2UgKmRldiA9IGNydGMtPmRldjsKPiArCXN0cnVjdCBk cm1faTkxNV9wcml2YXRlICpkZXZfcHJpdiA9IHRvX2k5MTUoZGV2KTsKPiArCWludCBsZXZlbCwg bWF4X2xldmVsID0gaWxrX3dtX21heF9sZXZlbChkZXYpOwo+ICsJZW51bSBwaXBlIHBpcGUgPSBp bnRlbF9jcnRjLT5waXBlOwo+ICsKPiArCWlmICghKHdtLT5kaXJ0eV9waXBlcyAmIGRybV9jcnRj X21hc2soY3J0YykpKQo+ICsJCXJldHVybjsKPiArCj4gKwlJOTE1X1dSSVRFKFBJUEVfV01fTElO RVRJTUUocGlwZSksIHdtLT53bV9saW5ldGltZVtwaXBlXSk7CgpJIHRoaW5rIEkgbWVudGlvbmVk IHRoaXMgb24gdGhlIHByZXZpb3VzIHZlcnNpb24sIGJ1dCBpZiB3ZSBwcm9ncmFtIHRoaXMKaGVy ZSB0aGVuIGl0IG9ubHkgZ2V0cyBwcm9ncmFtbWVkIHdoZW4gYSBwbGFuZSBpcyBiZWluZyB1cGRh dGVkLiAgSXQncwpwb3NzaWJsZSB0aGF0IHdlIGNvdWxkIGhhdmUgdGhlIGxpbmV0aW1lIHZhbHVl IGNoYW5nZSBkdWUgdG8gYSBzY2FsZXIKdXBkYXRlIG9yIGEgbW9kZXNldCwgYnV0IHdpdGhvdXQg Y29ycmVzcG9uZGluZyBwbGFuZSB1cGRhdGVzIChlLmcuLAptYXliZSBhbGwgcGxhbmVzIGFyZSBv ZmYpLiAgU28gd2UgcHJvYmFibHkgbmVlZCB0byBtYWtlIHN1cmUgdGhpcyB2YWx1ZQpnZXRzIHdy aXR0ZW4gb3V0c2lkZSB0aGUgcGxhbmUgdXBkYXRlcyAoYnV0IHN0aWxsIHVuZGVyIHZibGFuayBl dmFzaW9uKS4KCj4gKwo+ICsJZm9yIChsZXZlbCA9IDA7IGxldmVsIDw9IG1heF9sZXZlbDsgbGV2 ZWwrKykgewo+ICsJCUk5MTVfV1JJVEUoUExBTkVfV00ocGlwZSwgcGxhbmUsIGxldmVsKSwKPiAr CQkJICAgd20tPnBsYW5lW3BpcGVdW3BsYW5lXVtsZXZlbF0pOwo+ICsJfQo+ICsJSTkxNV9XUklU RShQTEFORV9XTV9UUkFOUyhwaXBlLCBwbGFuZSksIHdtLT5wbGFuZV90cmFuc1twaXBlXVtwbGFu ZV0pOwoKSSBub3RpY2UgdGhhdCB5b3UgbW92ZWQgdGhlIEREQiB1cGRhdGUgaW50byBza2xfd3Jp dGVfY3Vyc29yX3dtKCkgZG93bgpiZWxvdywgYnV0IGRpZG4ndCBtb3ZlIHRoZSBwbGFuZSBEREIg dXBkYXRlIGluIGhlcmUuICBJIHJlYWxpemUgeW91CnJlbWVkeSB0aGF0IGluIHBhdGNoICM2IHdo ZXJlIHlvdSBmaW5hbGx5IGZpeCB1cCB0aGUgZmx1c2hpbmcgb3JkZXIsIGJ1dAp0aGUgaW5jb25z aXN0ZW5jeSBiZXR3ZWVuIGhvdyBjdXJzb3IgdnMgIWN1cnNvciBwbGFuZXMgYXJlIGhhbmRsZWQg c2VlbXMKY29uZnVzaW5nLiAgQ2FuIHdlIGVpdGhlciBtb3ZlIHRoZSBwbGFuZSBEREIgdXBkYXRl IGhlcmUgaW4gdGhpcyBwYXRjaAood2UgZG9uJ3QgZmx1c2ggcHJvcGVybHkgeWV0LCBidXQgd2Un cmUgcmVhbGx5IG5vIHdvcnNlIG9mZiB0aGFuIHdlCmFscmVhZHkgYXJlKSwgb3IgY2FuIHdlIGRl ZmVyIHRoZSBtb3ZlIG9mIHRoZSBjdXJzb3IgRERCIHRvIHBhdGNoIDYgdG8Ka2VlcCB0aGVtIGNv bnNpc3RlbnQ/CgoKTWF0dAoKPiArfQo+ICsKPiArdm9pZCBza2xfd3JpdGVfY3Vyc29yX3dtKHN0 cnVjdCBpbnRlbF9jcnRjICppbnRlbF9jcnRjLAo+ICsJCQkgY29uc3Qgc3RydWN0IHNrbF93bV92 YWx1ZXMgKndtKQo+ICt7Cj4gKwlzdHJ1Y3QgZHJtX2NydGMgKmNydGMgPSAmaW50ZWxfY3J0Yy0+ YmFzZTsKPiArCXN0cnVjdCBkcm1fZGV2aWNlICpkZXYgPSBjcnRjLT5kZXY7Cj4gKwlzdHJ1Y3Qg ZHJtX2k5MTVfcHJpdmF0ZSAqZGV2X3ByaXYgPSB0b19pOTE1KGRldik7Cj4gKwlpbnQgbGV2ZWws IG1heF9sZXZlbCA9IGlsa193bV9tYXhfbGV2ZWwoZGV2KTsKPiArCWVudW0gcGlwZSBwaXBlID0g aW50ZWxfY3J0Yy0+cGlwZTsKPiArCj4gKwlmb3IgKGxldmVsID0gMDsgbGV2ZWwgPD0gbWF4X2xl dmVsOyBsZXZlbCsrKSB7Cj4gKwkJSTkxNV9XUklURShDVVJfV00ocGlwZSwgbGV2ZWwpLAo+ICsJ CQkgICB3bS0+cGxhbmVbcGlwZV1bUExBTkVfQ1VSU09SXVtsZXZlbF0pOwo+ICsJfQo+ICsJSTkx NV9XUklURShDVVJfV01fVFJBTlMocGlwZSksIHdtLT5wbGFuZV90cmFuc1twaXBlXVtQTEFORV9D VVJTT1JdKTsKPiArCj4gKwlza2xfZGRiX2VudHJ5X3dyaXRlKGRldl9wcml2LCBDVVJfQlVGX0NG RyhwaXBlKSwKPiArCQkJICAgICZ3bS0+ZGRiLnBsYW5lW3BpcGVdW1BMQU5FX0NVUlNPUl0pOwo+ ICt9Cj4gKwo+ICBzdGF0aWMgdm9pZCBza2xfd3JpdGVfd21fdmFsdWVzKHN0cnVjdCBkcm1faTkx NV9wcml2YXRlICpkZXZfcHJpdiwKPiAgCQkJCWNvbnN0IHN0cnVjdCBza2xfd21fdmFsdWVzICpu ZXcpCj4gIHsKPiBAQCAtMzgwNSw3ICszODQ2LDcgQEAgc3RhdGljIHZvaWQgc2tsX3dyaXRlX3dt X3ZhbHVlcyhzdHJ1Y3QgZHJtX2k5MTVfcHJpdmF0ZSAqZGV2X3ByaXYsCj4gIAlzdHJ1Y3QgaW50 ZWxfY3J0YyAqY3J0YzsKPiAgCj4gIAlmb3JfZWFjaF9pbnRlbF9jcnRjKGRldiwgY3J0Yykgewo+ IC0JCWludCBpLCBsZXZlbCwgbWF4X2xldmVsID0gaWxrX3dtX21heF9sZXZlbChkZXYpOwo+ICsJ CWludCBpOwo+ICAJCWVudW0gcGlwZSBwaXBlID0gY3J0Yy0+cGlwZTsKPiAgCj4gIAkJaWYgKChu ZXctPmRpcnR5X3BpcGVzICYgZHJtX2NydGNfbWFzaygmY3J0Yy0+YmFzZSkpID09IDApCj4gQEAg LTM4MTMsMjEgKzM4NTQsNiBAQCBzdGF0aWMgdm9pZCBza2xfd3JpdGVfd21fdmFsdWVzKHN0cnVj dCBkcm1faTkxNV9wcml2YXRlICpkZXZfcHJpdiwKPiAgCQlpZiAoIWNydGMtPmFjdGl2ZSkKPiAg CQkJY29udGludWU7Cj4gIAo+IC0JCUk5MTVfV1JJVEUoUElQRV9XTV9MSU5FVElNRShwaXBlKSwg bmV3LT53bV9saW5ldGltZVtwaXBlXSk7Cj4gLQo+IC0JCWZvciAobGV2ZWwgPSAwOyBsZXZlbCA8 PSBtYXhfbGV2ZWw7IGxldmVsKyspIHsKPiAtCQkJZm9yIChpID0gMDsgaSA8IGludGVsX251bV9w bGFuZXMoY3J0Yyk7IGkrKykKPiAtCQkJCUk5MTVfV1JJVEUoUExBTkVfV00ocGlwZSwgaSwgbGV2 ZWwpLAo+IC0JCQkJCSAgIG5ldy0+cGxhbmVbcGlwZV1baV1bbGV2ZWxdKTsKPiAtCQkJSTkxNV9X UklURShDVVJfV00ocGlwZSwgbGV2ZWwpLAo+IC0JCQkJICAgbmV3LT5wbGFuZVtwaXBlXVtQTEFO RV9DVVJTT1JdW2xldmVsXSk7Cj4gLQkJfQo+IC0JCWZvciAoaSA9IDA7IGkgPCBpbnRlbF9udW1f cGxhbmVzKGNydGMpOyBpKyspCj4gLQkJCUk5MTVfV1JJVEUoUExBTkVfV01fVFJBTlMocGlwZSwg aSksCj4gLQkJCQkgICBuZXctPnBsYW5lX3RyYW5zW3BpcGVdW2ldKTsKPiAtCQlJOTE1X1dSSVRF KENVUl9XTV9UUkFOUyhwaXBlKSwKPiAtCQkJICAgbmV3LT5wbGFuZV90cmFuc1twaXBlXVtQTEFO RV9DVVJTT1JdKTsKPiAtCj4gIAkJZm9yIChpID0gMDsgaSA8IGludGVsX251bV9wbGFuZXMoY3J0 Yyk7IGkrKykgewo+ICAJCQlza2xfZGRiX2VudHJ5X3dyaXRlKGRldl9wcml2LAo+ICAJCQkJCSAg ICBQTEFORV9CVUZfQ0ZHKHBpcGUsIGkpLAo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0v aTkxNS9pbnRlbF9zcHJpdGUuYyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX3Nwcml0ZS5j Cj4gaW5kZXggMGRlOTM1YS4uNTVkMTczZiAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0v aTkxNS9pbnRlbF9zcHJpdGUuYwo+ICsrKyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX3Nw cml0ZS5jCj4gQEAgLTIwMyw2ICsyMDMsOSBAQCBza2xfdXBkYXRlX3BsYW5lKHN0cnVjdCBkcm1f cGxhbmUgKmRybV9wbGFuZSwKPiAgCXN0cnVjdCBpbnRlbF9wbGFuZSAqaW50ZWxfcGxhbmUgPSB0 b19pbnRlbF9wbGFuZShkcm1fcGxhbmUpOwo+ICAJc3RydWN0IGRybV9mcmFtZWJ1ZmZlciAqZmIg PSBwbGFuZV9zdGF0ZS0+YmFzZS5mYjsKPiAgCXN0cnVjdCBkcm1faTkxNV9nZW1fb2JqZWN0ICpv YmogPSBpbnRlbF9mYl9vYmooZmIpOwo+ICsJc3RydWN0IHNrbF93bV92YWx1ZXMgKndtID0gJmRl dl9wcml2LT53bS5za2xfcmVzdWx0czsKPiArCXN0cnVjdCBkcm1fY3J0YyAqY3J0YyA9IGNydGNf c3RhdGUtPmJhc2UuY3J0YzsKPiArCXN0cnVjdCBpbnRlbF9jcnRjICppbnRlbF9jcnRjID0gdG9f aW50ZWxfY3J0YyhjcnRjKTsKPiAgCWNvbnN0IGludCBwaXBlID0gaW50ZWxfcGxhbmUtPnBpcGU7 Cj4gIAljb25zdCBpbnQgcGxhbmUgPSBpbnRlbF9wbGFuZS0+cGxhbmUgKyAxOwo+ICAJdTMyIHBs YW5lX2N0bCwgc3RyaWRlX2Rpdiwgc3RyaWRlOwo+IEBAIC0yMzgsNiArMjQxLDkgQEAgc2tsX3Vw ZGF0ZV9wbGFuZShzdHJ1Y3QgZHJtX3BsYW5lICpkcm1fcGxhbmUsCj4gIAljcnRjX3ctLTsKPiAg CWNydGNfaC0tOwo+ICAKPiArCWlmICh3bS0+ZGlydHlfcGlwZXMgJiBkcm1fY3J0Y19tYXNrKGNy dGMpKQo+ICsJICAgIHNrbF93cml0ZV9wbGFuZV93bShpbnRlbF9jcnRjLCB3bSwgcGxhbmUpOwo+ ICsKPiAgCWlmIChrZXktPmZsYWdzKSB7Cj4gIAkJSTkxNV9XUklURShQTEFORV9LRVlWQUwocGlw ZSwgcGxhbmUpLCBrZXktPm1pbl92YWx1ZSk7Cj4gIAkJSTkxNV9XUklURShQTEFORV9LRVlNQVgo cGlwZSwgcGxhbmUpLCBrZXktPm1heF92YWx1ZSk7Cj4gLS0gCj4gMi43LjQKPiAKCi0tIApNYXR0 IFJvcGVyCkdyYXBoaWNzIFNvZnR3YXJlIEVuZ2luZWVyCklvVEcgUGxhdGZvcm0gRW5hYmxpbmcg JiBEZXZlbG9wbWVudApJbnRlbCBDb3Jwb3JhdGlvbgooOTE2KSAzNTYtMjc5NQpfX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpkcmktZGV2ZWwgbWFpbGluZyBs aXN0CmRyaS1kZXZlbEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0cy5mcmVlZGVz a3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9kcmktZGV2ZWwK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754994AbcHBVaf (ORCPT ); Tue, 2 Aug 2016 17:30:35 -0400 Received: from mga11.intel.com ([192.55.52.93]:38868 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750705AbcHBVa3 (ORCPT ); Tue, 2 Aug 2016 17:30:29 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.28,462,1464678000"; d="scan'208";a="743306761" Date: Tue, 2 Aug 2016 14:20:33 -0700 From: Matt Roper To: Lyude Cc: intel-gfx@lists.freedesktop.org, Ville =?iso-8859-1?Q?Syrj=E4l=E4?= , Maarten Lankhorst , 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 v5 3/6] drm/i915/skl: Update plane watermarks atomically during plane updates Message-ID: <20160802212033.GY32025@intel.com> References: <1470163975-30467-1-git-send-email-cpaul@redhat.com> <1470163975-30467-4-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: <1470163975-30467-4-git-send-email-cpaul@redhat.com> User-Agent: Mutt/1.5.20 (2009-06-14) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Aug 02, 2016 at 02:52:51PM -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 > > Fixes: 2d41c0b59afc ("drm/i915/skl: SKL Watermark Computation") > Signed-off-by: Lyude > Cc: stable@vger.kernel.org > Cc: Ville Syrjälä > Cc: Daniel Vetter > Cc: Radhakrishna Sripada > Cc: Hans de Goede > Cc: Matt Roper > --- > drivers/gpu/drm/i915/intel_display.c | 8 +++++ > drivers/gpu/drm/i915/intel_drv.h | 5 ++++ > drivers/gpu/drm/i915/intel_pm.c | 58 ++++++++++++++++++++++++++---------- > drivers/gpu/drm/i915/intel_sprite.c | 6 ++++ > 4 files changed, 61 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 76ba79f..79d146c 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2980,6 +2980,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane, > 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); > + struct skl_wm_values *wm = &dev_priv->wm.skl_results; > int pipe = intel_crtc->pipe; > u32 plane_ctl, stride_div, stride; > u32 tile_height, plane_offset, plane_size; > @@ -3031,6 +3032,9 @@ static void skylake_update_primary_plane(struct drm_plane *plane, > intel_crtc->adjusted_x = x_offset; > intel_crtc->adjusted_y = y_offset; > > + if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base)) > + skl_write_plane_wm(intel_crtc, wm, 0); > + > I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl); > I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset); > I915_WRITE(PLANE_SIZE(pipe, 0), plane_size); > @@ -10243,9 +10247,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 (IS_GEN9(dev_priv) && wm->dirty_pipes & drm_crtc_mask(crtc)) > + skl_write_cursor_wm(intel_crtc, wm); > + > if (plane_state && plane_state->visible) { > cntl = MCURSOR_GAMMA_ENABLE; > switch (plane_state->base.crtc_w) { > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 6b0532a..1b444d3 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1711,6 +1711,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 7fd299e..53adcbf 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3798,6 +3798,47 @@ 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; > + > + if (!(wm->dirty_pipes & drm_crtc_mask(crtc))) > + return; > + > + I915_WRITE(PIPE_WM_LINETIME(pipe), wm->wm_linetime[pipe]); I think I mentioned this on the previous version, but if we program this here then it only gets programmed when a plane is being updated. It's possible that we could have the linetime value change due to a scaler update or a modeset, but without corresponding plane updates (e.g., maybe all planes are off). So we probably need to make sure this value gets written outside the plane updates (but still under vblank evasion). > + > + 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]); I notice that you moved the DDB update into skl_write_cursor_wm() down below, but didn't move the plane DDB update in here. I realize you remedy that in patch #6 where you finally fix up the flushing order, but the inconsistency between how cursor vs !cursor planes are handled seems confusing. Can we either move the plane DDB update here in this patch (we don't flush properly yet, but we're really no worse off than we already are), or can we defer the move of the cursor DDB to patch 6 to keep them consistent? Matt > +} > + > +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]); > + > + skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe), > + &wm->ddb.plane[pipe][PLANE_CURSOR]); > +} > + > static void skl_write_wm_values(struct drm_i915_private *dev_priv, > const struct skl_wm_values *new) > { > @@ -3805,7 +3846,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) > @@ -3813,21 +3854,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 0de935a..55d173f 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -203,6 +203,9 @@ skl_update_plane(struct drm_plane *drm_plane, > 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, stride_div, stride; > @@ -238,6 +241,9 @@ skl_update_plane(struct drm_plane *drm_plane, > crtc_w--; > crtc_h--; > > + 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 > -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795 From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com ([192.55.52.93]:62236 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750705AbcHBVXN (ORCPT ); Tue, 2 Aug 2016 17:23:13 -0400 Date: Tue, 2 Aug 2016 14:20:33 -0700 From: Matt Roper To: Lyude Cc: intel-gfx@lists.freedesktop.org, Ville =?iso-8859-1?Q?Syrj=E4l=E4?= , Maarten Lankhorst , 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 v5 3/6] drm/i915/skl: Update plane watermarks atomically during plane updates Message-ID: <20160802212033.GY32025@intel.com> References: <1470163975-30467-1-git-send-email-cpaul@redhat.com> <1470163975-30467-4-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: <1470163975-30467-4-git-send-email-cpaul@redhat.com> Sender: stable-owner@vger.kernel.org List-ID: On Tue, Aug 02, 2016 at 02:52:51PM -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 > > Fixes: 2d41c0b59afc ("drm/i915/skl: SKL Watermark Computation") > Signed-off-by: Lyude > Cc: stable@vger.kernel.org > Cc: Ville Syrj�l� > Cc: Daniel Vetter > Cc: Radhakrishna Sripada > Cc: Hans de Goede > Cc: Matt Roper > --- > drivers/gpu/drm/i915/intel_display.c | 8 +++++ > drivers/gpu/drm/i915/intel_drv.h | 5 ++++ > drivers/gpu/drm/i915/intel_pm.c | 58 ++++++++++++++++++++++++++---------- > drivers/gpu/drm/i915/intel_sprite.c | 6 ++++ > 4 files changed, 61 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 76ba79f..79d146c 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -2980,6 +2980,7 @@ static void skylake_update_primary_plane(struct drm_plane *plane, > 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); > + struct skl_wm_values *wm = &dev_priv->wm.skl_results; > int pipe = intel_crtc->pipe; > u32 plane_ctl, stride_div, stride; > u32 tile_height, plane_offset, plane_size; > @@ -3031,6 +3032,9 @@ static void skylake_update_primary_plane(struct drm_plane *plane, > intel_crtc->adjusted_x = x_offset; > intel_crtc->adjusted_y = y_offset; > > + if (wm->dirty_pipes & drm_crtc_mask(&intel_crtc->base)) > + skl_write_plane_wm(intel_crtc, wm, 0); > + > I915_WRITE(PLANE_CTL(pipe, 0), plane_ctl); > I915_WRITE(PLANE_OFFSET(pipe, 0), plane_offset); > I915_WRITE(PLANE_SIZE(pipe, 0), plane_size); > @@ -10243,9 +10247,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 (IS_GEN9(dev_priv) && wm->dirty_pipes & drm_crtc_mask(crtc)) > + skl_write_cursor_wm(intel_crtc, wm); > + > if (plane_state && plane_state->visible) { > cntl = MCURSOR_GAMMA_ENABLE; > switch (plane_state->base.crtc_w) { > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 6b0532a..1b444d3 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -1711,6 +1711,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 7fd299e..53adcbf 100644 > --- a/drivers/gpu/drm/i915/intel_pm.c > +++ b/drivers/gpu/drm/i915/intel_pm.c > @@ -3798,6 +3798,47 @@ 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; > + > + if (!(wm->dirty_pipes & drm_crtc_mask(crtc))) > + return; > + > + I915_WRITE(PIPE_WM_LINETIME(pipe), wm->wm_linetime[pipe]); I think I mentioned this on the previous version, but if we program this here then it only gets programmed when a plane is being updated. It's possible that we could have the linetime value change due to a scaler update or a modeset, but without corresponding plane updates (e.g., maybe all planes are off). So we probably need to make sure this value gets written outside the plane updates (but still under vblank evasion). > + > + 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]); I notice that you moved the DDB update into skl_write_cursor_wm() down below, but didn't move the plane DDB update in here. I realize you remedy that in patch #6 where you finally fix up the flushing order, but the inconsistency between how cursor vs !cursor planes are handled seems confusing. Can we either move the plane DDB update here in this patch (we don't flush properly yet, but we're really no worse off than we already are), or can we defer the move of the cursor DDB to patch 6 to keep them consistent? Matt > +} > + > +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]); > + > + skl_ddb_entry_write(dev_priv, CUR_BUF_CFG(pipe), > + &wm->ddb.plane[pipe][PLANE_CURSOR]); > +} > + > static void skl_write_wm_values(struct drm_i915_private *dev_priv, > const struct skl_wm_values *new) > { > @@ -3805,7 +3846,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) > @@ -3813,21 +3854,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 0de935a..55d173f 100644 > --- a/drivers/gpu/drm/i915/intel_sprite.c > +++ b/drivers/gpu/drm/i915/intel_sprite.c > @@ -203,6 +203,9 @@ skl_update_plane(struct drm_plane *drm_plane, > 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, stride_div, stride; > @@ -238,6 +241,9 @@ skl_update_plane(struct drm_plane *drm_plane, > crtc_w--; > crtc_h--; > > + 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 > -- Matt Roper Graphics Software Engineer IoTG Platform Enabling & Development Intel Corporation (916) 356-2795