From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tvrtko Ursulin Subject: Re: [PATCH 2/3] drm/i915: Fix some invalid requests cancellations Date: Thu, 11 Feb 2016 13:01:34 +0000 Message-ID: <56BC862E.2020305@linux.intel.com> References: <1454086145-16160-1-git-send-email-chris@chris-wilson.co.uk> <1454086145-16160-2-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8"; Format="flowed" Content-Transfer-Encoding: base64 Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id 62A546E860 for ; Thu, 11 Feb 2016 05:01:59 -0800 (PST) In-Reply-To: <1454086145-16160-2-git-send-email-chris@chris-wilson.co.uk> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , intel-gfx@lists.freedesktop.org Cc: Daniel Vetter , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org CgpPbiAyOS8wMS8xNiAxNjo0OSwgQ2hyaXMgV2lsc29uIHdyb3RlOgo+IEFzIHdlIGFkZCB0aGUg Vk1BIHRvIHRoZSByZXF1ZXN0IGVhcmx5LCBpdCBtYXkgYmUgY2FuY2VsbGVkIGR1cmluZwo+IGV4 ZWNidWYgcmVzZXJ2YXRpb24uIFRoaXMgd2lsbCBsZWF2ZSB0aGUgY29udGV4dCBvYmplY3QgcG9p bnRpbmcgdG8gYQoKSSBkb24ndCBnZXQgaXQsIHJlcXVlc3QgaXMgY3JlYXRlZCBhZnRlciB0aGUg cmVzZXJ2YXRpb24uCgo+IGRhbmdsaW5nIHJlcXVlc3Q7IGk5MTVfd2FpdF9yZXF1ZXN0KCkgc2lt cGx5IHNraXBzIHRoZSB3YWl0IGFuZCBzbyB3ZQo+IG1heSB1bmJpbmQgdGhlIG9iamVjdCB3aGls c3QgaXQgaXMgc3RpbGwgYWN0aXZlLgo+Cj4gSG93ZXZlciwgaWYgYXQgYW55IHBvaW50IHdlIG1h a2UgYSBjaGFuZ2UgdG8gdGhlIGhhcmR3YXJlIChhbmQgZXF1YWxseQo+IGltcG9ydGFudGx5IG91 ciBib29ra2VlcGluZyBpbiB0aGUgZHJpdmVyKSwgd2UgY2Fubm90IGNhbmNlbCB0aGUgcmVxdWVz dAo+IGFzIHdoYXQgaGFzIGFscmVhZHkgYmVlbiB3cml0dGVuIG11c3QgYmUgc3VibWl0dGVkLiBT dWJtaXR0aW5nIGEgcGFydGlhbAo+IHJlcXVlc3QgaXMgZmFyIGVhc2llciB0aGFuIHRyeWluZyB0 byB1bndpbmQgdGhlIGluY29tcGxldGUgY2hhbmdlLgo+Cj4gVW5mb3J0dW5hdGVseSB0aGlzIHBh dGNoIHVuZG9lcyB0aGUgZXhjZXNzIGJyZWFkY3J1bWIgdXNhZ2UgdGhhdCBvbHIKPiBwcmV2ZW50 ZWQsIGUuZy4gaWYgd2UgaW50ZXJydXB0IGJhdGNoYnVmZmVyIHN1Ym1pc3Npb24gdGhlbiB3ZSBz dWJtaXQKPiB0aGUgcmVxdWVzdHMgYWxvbmcgd2l0aCB0aGUgbWVtb3J5IHdyaXRlcyBhbmQgaW50 ZXJydXB0IChldmVuIHRob3VnaCB3ZQo+IGRvIG5vIHJlYWwgd29yaykuIERpc2Fzc29jaWF0aW5n IHJlcXVlc3RzIGZyb20gYnJlYWRjcnVtYnMgKGFuZAo+IHNlbWFwaG9yZXMpIGlzIGEgdG9waWMg Zm9yIGEgcGFzdC9mdXR1cmUgc2VyaWVzLCBidXQgbm93IG11Y2ggbW9yZQo+IGltcG9ydGFudC4K ID4KPiB2MjogUmViYXNlCj4KPiBOb3RlIHRoYXQgaWd0L2dlbV9jb25jdXJyZW50X2JsaXQgdGln Z2VycyBib3RoIG1pc3JlbmRlcmluZyBhbmQgYSBHUEYKPiB0aGF0IGlzIGZpeGVkIGJ5IHRoaXMg cGF0Y2guCj4KPiBCdWd6aWxsYTogaHR0cHM6Ly9idWdzLmZyZWVkZXNrdG9wLm9yZy9zaG93X2J1 Zy5jZ2k/aWQ9OTM5MDcKPiBUZXN0Y2FzZTogaWd0L2dlbV9jb25jdXJyZW50X2JsaXQKPiBTaWdu ZWQtb2ZmLWJ5OiBDaHJpcyBXaWxzb24gPGNocmlzQGNocmlzLXdpbHNvbi5jby51az4KPiBDYzog RGFuaWVsIFZldHRlciA8ZGFuaWVsLnZldHRlckBmZndsbC5jaD4KPiBDYzogc3RhYmxlQHZnZXIu a2VybmVsLm9yZwo+IC0tLQo+ICAgZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9kcnYuaCAgICAg ICAgICAgIHwgIDEgLQo+ICAgZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9nZW0uYyAgICAgICAg ICAgIHwgIDcgKystLS0tLQo+ICAgZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9nZW1fY29udGV4 dC5jICAgIHwgMjEgKysrKysrKysrLS0tLS0tLS0tLS0tCj4gICBkcml2ZXJzL2dwdS9kcm0vaTkx NS9pOTE1X2dlbV9leGVjYnVmZmVyLmMgfCAxNiArKysrKy0tLS0tLS0tLS0tCj4gICBkcml2ZXJz L2dwdS9kcm0vaTkxNS9pbnRlbF9kaXNwbGF5LmMgICAgICAgfCAgMiArLQo+ICAgZHJpdmVycy9n cHUvZHJtL2k5MTUvaW50ZWxfbHJjLmMgICAgICAgICAgIHwgIDEgLQo+ICAgNiBmaWxlcyBjaGFu Z2VkLCAxNyBpbnNlcnRpb25zKCspLCAzMSBkZWxldGlvbnMoLSkKPgo+IGRpZmYgLS1naXQgYS9k cml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X2Rydi5oIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkx NV9kcnYuaAo+IGluZGV4IGEyZDJmMDhiNzUxNS4uZjgyOGE3ZmZlZDM3IDEwMDY0NAo+IC0tLSBh L2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZHJ2LmgKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0v aTkxNS9pOTE1X2Rydi5oCj4gQEAgLTI4MjMsNyArMjgyMyw2IEBAIGludCBpOTE1X2dlbV9zd19m aW5pc2hfaW9jdGwoc3RydWN0IGRybV9kZXZpY2UgKmRldiwgdm9pZCAqZGF0YSwKPiAgIAkJCSAg ICAgc3RydWN0IGRybV9maWxlICpmaWxlX3ByaXYpOwo+ICAgdm9pZCBpOTE1X2dlbV9leGVjYnVm ZmVyX21vdmVfdG9fYWN0aXZlKHN0cnVjdCBsaXN0X2hlYWQgKnZtYXMsCj4gICAJCQkJCXN0cnVj dCBkcm1faTkxNV9nZW1fcmVxdWVzdCAqcmVxKTsKPiAtdm9pZCBpOTE1X2dlbV9leGVjYnVmZmVy X3JldGlyZV9jb21tYW5kcyhzdHJ1Y3QgaTkxNV9leGVjYnVmZmVyX3BhcmFtcyAqcGFyYW1zKTsK PiAgIGludCBpOTE1X2dlbV9yaW5nYnVmZmVyX3N1Ym1pc3Npb24oc3RydWN0IGk5MTVfZXhlY2J1 ZmZlcl9wYXJhbXMgKnBhcmFtcywKPiAgIAkJCQkgICBzdHJ1Y3QgZHJtX2k5MTVfZ2VtX2V4ZWNi dWZmZXIyICphcmdzLAo+ICAgCQkJCSAgIHN0cnVjdCBsaXN0X2hlYWQgKnZtYXMpOwo+IGRpZmYg LS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X2dlbS5jIGIvZHJpdmVycy9ncHUvZHJt L2k5MTUvaTkxNV9nZW0uYwo+IGluZGV4IGU5YjE5YmNhMTM4My4uZjc2NGYzMzU4MGZjIDEwMDY0 NAo+IC0tLSBhL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZ2VtLmMKPiArKysgYi9kcml2ZXJz L2dwdS9kcm0vaTkxNS9pOTE1X2dlbS5jCj4gQEAgLTM0MDcsMTIgKzM0MDcsOSBAQCBpbnQgaTkx NV9ncHVfaWRsZShzdHJ1Y3QgZHJtX2RldmljZSAqZGV2KQo+ICAgCQkJCXJldHVybiBQVFJfRVJS KHJlcSk7Cj4KPiAgIAkJCXJldCA9IGk5MTVfc3dpdGNoX2NvbnRleHQocmVxKTsKPiAtCQkJaWYg KHJldCkgewo+IC0JCQkJaTkxNV9nZW1fcmVxdWVzdF9jYW5jZWwocmVxKTsKPiAtCQkJCXJldHVy biByZXQ7Cj4gLQkJCX0KPiAtCj4gICAJCQlpOTE1X2FkZF9yZXF1ZXN0X25vX2ZsdXNoKHJlcSk7 Cj4gKwkJCWlmIChyZXQpCj4gKwkJCQlyZXR1cm4gcmV0Owo+ICAgCQl9Cj4KPiAgIAkJcmV0ID0g aW50ZWxfcmluZ19pZGxlKHJpbmcpOwo+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vaTkx NS9pOTE1X2dlbV9jb250ZXh0LmMgYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X2dlbV9jb250 ZXh0LmMKPiBpbmRleCA4M2EwOTdjOTQ5MTEuLjVkYTdhZGMzZjdiMiAxMDA2NDQKPiAtLS0gYS9k cml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X2dlbV9jb250ZXh0LmMKPiArKysgYi9kcml2ZXJzL2dw dS9kcm0vaTkxNS9pOTE1X2dlbV9jb250ZXh0LmMKPiBAQCAtNjUyLDcgKzY1Miw2IEBAIHN0YXRp YyBpbnQgZG9fc3dpdGNoKHN0cnVjdCBkcm1faTkxNV9nZW1fcmVxdWVzdCAqcmVxKQo+ICAgCXN0 cnVjdCBkcm1faTkxNV9wcml2YXRlICpkZXZfcHJpdiA9IHJpbmctPmRldi0+ZGV2X3ByaXZhdGU7 Cj4gICAJc3RydWN0IGludGVsX2NvbnRleHQgKmZyb20gPSByaW5nLT5sYXN0X2NvbnRleHQ7Cj4g ICAJdTMyIGh3X2ZsYWdzID0gMDsKPiAtCWJvb2wgdW5pbml0aWFsaXplZCA9IGZhbHNlOwo+ICAg CWludCByZXQsIGk7Cj4KPiAgIAlpZiAoZnJvbSAhPSBOVUxMICYmIHJpbmcgPT0gJmRldl9wcml2 LT5yaW5nW1JDU10pIHsKPiBAQCAtNzU5LDYgKzc1OCwxNSBAQCBzdGF0aWMgaW50IGRvX3N3aXRj aChzdHJ1Y3QgZHJtX2k5MTVfZ2VtX3JlcXVlc3QgKnJlcSkKPiAgIAkJCXRvLT5yZW1hcF9zbGlj ZSAmPSB+KDE8PGkpOwo+ICAgCX0KPgo+ICsJaWYgKCF0by0+bGVnYWN5X2h3X2N0eC5pbml0aWFs aXplZCkgewo+ICsJCWlmIChyaW5nLT5pbml0X2NvbnRleHQpIHsKPiArCQkJcmV0ID0gcmluZy0+ aW5pdF9jb250ZXh0KHJlcSk7Cj4gKwkJCWlmIChyZXQpCj4gKwkJCQlnb3RvIHVucGluX291dDsK PiArCQl9Cj4gKwkJdG8tPmxlZ2FjeV9od19jdHguaW5pdGlhbGl6ZWQgPSB0cnVlOwo+ICsJfQo+ ICsKPiAgIAkvKiBUaGUgYmFja2luZyBvYmplY3QgZm9yIHRoZSBjb250ZXh0IGlzIGRvbmUgYWZ0 ZXIgc3dpdGNoaW5nIHRvIHRoZQo+ICAgCSAqICpuZXh0KiBjb250ZXh0LiBUaGVyZWZvcmUgd2Ug Y2Fubm90IHJldGlyZSB0aGUgcHJldmlvdXMgY29udGV4dCB1bnRpbAo+ICAgCSAqIHRoZSBuZXh0 IGNvbnRleHQgaGFzIGFscmVhZHkgc3RhcnRlZCBydW5uaW5nLiBJbiBmYWN0LCB0aGUgYmVsb3cg Y29kZQo+IEBAIC03ODIsMjEgKzc5MCwxMCBAQCBzdGF0aWMgaW50IGRvX3N3aXRjaChzdHJ1Y3Qg ZHJtX2k5MTVfZ2VtX3JlcXVlc3QgKnJlcSkKPiAgIAkJaTkxNV9nZW1fY29udGV4dF91bnJlZmVy ZW5jZShmcm9tKTsKPiAgIAl9Cj4KPiAtCXVuaW5pdGlhbGl6ZWQgPSAhdG8tPmxlZ2FjeV9od19j dHguaW5pdGlhbGl6ZWQ7Cj4gLQl0by0+bGVnYWN5X2h3X2N0eC5pbml0aWFsaXplZCA9IHRydWU7 Cj4gLQo+ICAgZG9uZToKPiAgIAlpOTE1X2dlbV9jb250ZXh0X3JlZmVyZW5jZSh0byk7Cj4gICAJ cmluZy0+bGFzdF9jb250ZXh0ID0gdG87Cj4KPiAtCWlmICh1bmluaXRpYWxpemVkKSB7Cj4gLQkJ aWYgKHJpbmctPmluaXRfY29udGV4dCkgewo+IC0JCQlyZXQgPSByaW5nLT5pbml0X2NvbnRleHQo cmVxKTsKPiAtCQkJaWYgKHJldCkKPiAtCQkJCURSTV9FUlJPUigicmluZyBpbml0IGNvbnRleHQ6 ICVkXG4iLCByZXQpOwo+IC0JCX0KPiAtCX0KPiAtCj4gICAJcmV0dXJuIDA7Cj4KPiAgIHVucGlu X291dDoKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9nZW1fZXhlY2J1 ZmZlci5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9nZW1fZXhlY2J1ZmZlci5jCj4gaW5k ZXggOGZkMDBkMjc5NDQ3Li42MWJiMTU1MDdiMzAgMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUv ZHJtL2k5MTUvaTkxNV9nZW1fZXhlY2J1ZmZlci5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2k5 MTUvaTkxNV9nZW1fZXhlY2J1ZmZlci5jCj4gQEAgLTExMzMsNyArMTEzMyw3IEBAIGk5MTVfZ2Vt X2V4ZWNidWZmZXJfbW92ZV90b19hY3RpdmUoc3RydWN0IGxpc3RfaGVhZCAqdm1hcywKPiAgIAl9 Cj4gICB9Cj4KPiAtdm9pZAo+ICtzdGF0aWMgdm9pZAo+ICAgaTkxNV9nZW1fZXhlY2J1ZmZlcl9y ZXRpcmVfY29tbWFuZHMoc3RydWN0IGk5MTVfZXhlY2J1ZmZlcl9wYXJhbXMgKnBhcmFtcykKPiAg IHsKPiAgIAkvKiBVbmNvbmRpdGlvbmFsbHkgZm9yY2UgYWRkX3JlcXVlc3QgdG8gZW1pdCBhIGZ1 bGwgZmx1c2guICovCj4gQEAgLTEzMTgsNyArMTMxOCw2IEBAIGk5MTVfZ2VtX3JpbmdidWZmZXJf c3VibWlzc2lvbihzdHJ1Y3QgaTkxNV9leGVjYnVmZmVyX3BhcmFtcyAqcGFyYW1zLAo+ICAgCXRy YWNlX2k5MTVfZ2VtX3JpbmdfZGlzcGF0Y2gocGFyYW1zLT5yZXF1ZXN0LCBwYXJhbXMtPmRpc3Bh dGNoX2ZsYWdzKTsKPgo+ICAgCWk5MTVfZ2VtX2V4ZWNidWZmZXJfbW92ZV90b19hY3RpdmUodm1h cywgcGFyYW1zLT5yZXF1ZXN0KTsKPiAtCWk5MTVfZ2VtX2V4ZWNidWZmZXJfcmV0aXJlX2NvbW1h bmRzKHBhcmFtcyk7CgpIbSB0aGlzIHdob2xlIGJsb2NrIGJsb2NrIGZyb20gdHJhY2VfLi4uIHRv IAppOTE1X2dlbV9leGVjYnVmZmVyX3JldGlyZV9jb21tYW5kcyBjb3VsZCBiZSBtb3ZlZCB0byBk b19leGVjYnVmZmVyIGFzIAphbmQgaW5kZXBlbmRlbnQgY2xlYW51cCBwYXRjaCBpZiB1bmRlciAi aWYgKHJldCA9PSAwKSIuCgpTbyB0aGUgY29uY2VybiBpcyB0aGF0IHNvbWV0aGluZyBoYXMgYmVl biB3cml0dGVuIHRvIHRoZSByaW5nYnVmZmVyIGJ1dCAKbm90IGFsbCBvZiBpdD8KCldoeSBkbyB3 ZSBoYXZlIHRvIHN1Ym1pdCB0aGF0LCBJIGFtIGFzc3VtaW5nIHdoYXRldmVyIHdhcyB3cml0dGVu IGlzIG5vdCAKaW50ZXJlc3RpbmcgdG8gYmUgZXhlY3V0ZWQgdW5sZXNzIHRoZSB3aG9sZSBiYiB3 ZW50IGluLgoKU28gY291bGQgd2UganVzdCByZXdpbmQgdGhlIHBvaW50ZXJzPyBTdG9yZSB0aGVt IGF0IHRoZSBiZWdpbm5pbmcgYW5kIApyZXdpbmQgaWYgc29tZXRoaW5nIGZhaWxlZC4KCj4KPiAg IAlyZXR1cm4gMDsKPiAgIH0KPiBAQCAtMTYxNiw4ICsxNjE1LDEwIEBAIGk5MTVfZ2VtX2RvX2V4 ZWNidWZmZXIoc3RydWN0IGRybV9kZXZpY2UgKmRldiwgdm9pZCAqZGF0YSwKPiAgIAl9Cj4KPiAg IAlyZXQgPSBpOTE1X2dlbV9yZXF1ZXN0X2FkZF90b19jbGllbnQocmVxLCBmaWxlKTsKPiAtCWlm IChyZXQpCj4gKwlpZiAocmV0KSB7Cj4gKwkJaTkxNV9nZW1fcmVxdWVzdF9jYW5jZWwocmVxKTsK PiAgIAkJZ290byBlcnJfYmF0Y2hfdW5waW47Cj4gKwl9CgpEb2Vzbid0IGxvb2sgbGlrZSB0aGlz IGNhbiBmYWlsIHNvIGEgc2lkZSBjbGVhbnVwIG9ubHk/CgpSZWdhcmRzLAoKVHZydGtvCl9fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCkludGVsLWdmeCBtYWls aW5nIGxpc3QKSW50ZWwtZ2Z4QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZy ZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xpc3RpbmZvL2ludGVsLWdmeAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga09.intel.com ([134.134.136.24]:29564 "EHLO mga09.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751974AbcBKNB7 (ORCPT ); Thu, 11 Feb 2016 08:01:59 -0500 Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915: Fix some invalid requests cancellations To: Chris Wilson , intel-gfx@lists.freedesktop.org References: <1454086145-16160-1-git-send-email-chris@chris-wilson.co.uk> <1454086145-16160-2-git-send-email-chris@chris-wilson.co.uk> Cc: Daniel Vetter , stable@vger.kernel.org From: Tvrtko Ursulin Message-ID: <56BC862E.2020305@linux.intel.com> Date: Thu, 11 Feb 2016 13:01:34 +0000 MIME-Version: 1.0 In-Reply-To: <1454086145-16160-2-git-send-email-chris@chris-wilson.co.uk> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: On 29/01/16 16:49, Chris Wilson wrote: > As we add the VMA to the request early, it may be cancelled during > execbuf reservation. This will leave the context object pointing to a I don't get it, request is created after the reservation. > dangling request; i915_wait_request() simply skips the wait and so we > may unbind the object whilst it is still active. > > However, if at any point we make a change to the hardware (and equally > importantly our bookkeeping in the driver), we cannot cancel the request > as what has already been written must be submitted. Submitting a partial > request is far easier than trying to unwind the incomplete change. > > Unfortunately this patch undoes the excess breadcrumb usage that olr > prevented, e.g. if we interrupt batchbuffer submission then we submit > the requests along with the memory writes and interrupt (even though we > do no real work). Disassociating requests from breadcrumbs (and > semaphores) is a topic for a past/future series, but now much more > important. > > v2: Rebase > > Note that igt/gem_concurrent_blit tiggers both misrendering and a GPF > that is fixed by this patch. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93907 > Testcase: igt/gem_concurrent_blit > Signed-off-by: Chris Wilson > Cc: Daniel Vetter > Cc: stable@vger.kernel.org > --- > drivers/gpu/drm/i915/i915_drv.h | 1 - > drivers/gpu/drm/i915/i915_gem.c | 7 ++----- > drivers/gpu/drm/i915/i915_gem_context.c | 21 +++++++++------------ > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 16 +++++----------- > drivers/gpu/drm/i915/intel_display.c | 2 +- > drivers/gpu/drm/i915/intel_lrc.c | 1 - > 6 files changed, 17 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index a2d2f08b7515..f828a7ffed37 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2823,7 +2823,6 @@ int i915_gem_sw_finish_ioctl(struct drm_device *dev, void *data, > struct drm_file *file_priv); > void i915_gem_execbuffer_move_to_active(struct list_head *vmas, > struct drm_i915_gem_request *req); > -void i915_gem_execbuffer_retire_commands(struct i915_execbuffer_params *params); > int i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params, > struct drm_i915_gem_execbuffer2 *args, > struct list_head *vmas); > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index e9b19bca1383..f764f33580fc 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3407,12 +3407,9 @@ int i915_gpu_idle(struct drm_device *dev) > return PTR_ERR(req); > > ret = i915_switch_context(req); > - if (ret) { > - i915_gem_request_cancel(req); > - return ret; > - } > - > i915_add_request_no_flush(req); > + if (ret) > + return ret; > } > > ret = intel_ring_idle(ring); > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index 83a097c94911..5da7adc3f7b2 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -652,7 +652,6 @@ static int do_switch(struct drm_i915_gem_request *req) > struct drm_i915_private *dev_priv = ring->dev->dev_private; > struct intel_context *from = ring->last_context; > u32 hw_flags = 0; > - bool uninitialized = false; > int ret, i; > > if (from != NULL && ring == &dev_priv->ring[RCS]) { > @@ -759,6 +758,15 @@ static int do_switch(struct drm_i915_gem_request *req) > to->remap_slice &= ~(1< } > > + if (!to->legacy_hw_ctx.initialized) { > + if (ring->init_context) { > + ret = ring->init_context(req); > + if (ret) > + goto unpin_out; > + } > + to->legacy_hw_ctx.initialized = true; > + } > + > /* The backing object for the context is done after switching to the > * *next* context. Therefore we cannot retire the previous context until > * the next context has already started running. In fact, the below code > @@ -782,21 +790,10 @@ static int do_switch(struct drm_i915_gem_request *req) > i915_gem_context_unreference(from); > } > > - uninitialized = !to->legacy_hw_ctx.initialized; > - to->legacy_hw_ctx.initialized = true; > - > done: > i915_gem_context_reference(to); > ring->last_context = to; > > - if (uninitialized) { > - if (ring->init_context) { > - ret = ring->init_context(req); > - if (ret) > - DRM_ERROR("ring init context: %d\n", ret); > - } > - } > - > return 0; > > unpin_out: > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 8fd00d279447..61bb15507b30 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1133,7 +1133,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas, > } > } > > -void > +static void > i915_gem_execbuffer_retire_commands(struct i915_execbuffer_params *params) > { > /* Unconditionally force add_request to emit a full flush. */ > @@ -1318,7 +1318,6 @@ i915_gem_ringbuffer_submission(struct i915_execbuffer_params *params, > trace_i915_gem_ring_dispatch(params->request, params->dispatch_flags); > > i915_gem_execbuffer_move_to_active(vmas, params->request); > - i915_gem_execbuffer_retire_commands(params); Hm this whole block block from trace_... to i915_gem_execbuffer_retire_commands could be moved to do_execbuffer as and independent cleanup patch if under "if (ret == 0)". So the concern is that something has been written to the ringbuffer but not all of it? Why do we have to submit that, I am assuming whatever was written is not interesting to be executed unless the whole bb went in. So could we just rewind the pointers? Store them at the beginning and rewind if something failed. > > return 0; > } > @@ -1616,8 +1615,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > } > > ret = i915_gem_request_add_to_client(req, file); > - if (ret) > + if (ret) { > + i915_gem_request_cancel(req); > goto err_batch_unpin; > + } Doesn't look like this can fail so a side cleanup only? Regards, Tvrtko