From mboxrd@z Thu Jan 1 00:00:00 1970 From: Dave Gordon Subject: Re: [PATCH 004/190] drm/i915: Fix some invalid requests cancellations Date: Tue, 12 Jan 2016 18:16:21 +0000 Message-ID: <569542F5.7040704@intel.com> References: <1452503961-14837-1-git-send-email-chris@chris-wilson.co.uk> <1452503961-14837-4-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 mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 5C8766E8B6 for ; Tue, 12 Jan 2016 10:16:24 -0800 (PST) In-Reply-To: <1452503961-14837-4-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 T24gMTEvMDEvMTYgMDk6MTYsIENocmlzIFdpbHNvbiB3cm90ZToKPiBBcyB3ZSBhZGQgdGhlIFZN QSB0byB0aGUgcmVxdWVzdCBlYXJseSwgaXQgbWF5IGJlIGNhbmNlbGxlZCBkdXJpbmcKPiBleGVj YnVmIHJlc2VydmF0aW9uLiBUaGlzIHdpbGwgbGVhdmUgdGhlIGNvbnRleHQgb2JqZWN0IHBvaW50 aW5nIHRvIGEKPiBkYW5nbGluZyByZXF1ZXN0OyBpOTE1X3dhaXRfcmVxdWVzdCgpIHNpbXBseSBz a2lwcyB0aGUgd2FpdCBhbmQgc28gd2UKPiBtYXkgdW5iaW5kIHRoZSBvYmplY3Qgd2hpbHN0IGl0 IGlzIHN0aWxsIGFjdGl2ZS4KCkkgZG9uJ3QgdW5kZXJzdGFuZCB0aGlzOyBjb250ZXh0IG9iamVj dHMgZG9uJ3QgcG9pbnQgdG8gcmVxdWVzdHMsIGl0J3MgCnZpY2UgdmVyc2EuIFRoZSByZXF1ZXN0 IGhhcyBhIHBvaW50ZXIgdG8gdGhlIGVuZ2luZSwgYW5kIHRvIHRoZSBjb250ZXh0LCAKYW5kIGFk ZHMgKzEgdG8gdGhlIHJlZmNvdW50IG9uIHRoZSBsYXR0ZXIuCgo+IEhvd2V2ZXIsIGlmIGF0IGFu eSBwb2ludCB3ZSBtYWtlIGEgY2hhbmdlIHRvIHRoZSBoYXJkd2FyZSAoYW5kIGVxdWFsbHkKPiBp bXBvcnRhbnRseSBvdXIgYm9va2tlZXBpbmcgaW4gdGhlIGRyaXZlciksIHdlIGNhbm5vdCBjYW5j ZWwgdGhlIHJlcXVlc3QKPiBhcyB3aGF0IGhhcyBhbHJlYWR5IGJlZW4gd3JpdHRlbiBtdXN0IGJl IHN1Ym1pdHRlZC4gU3VibWl0dGluZyBhIHBhcnRpYWwKPiByZXF1ZXN0IGlzIGZhciBlYXNpZXIg dGhhbiB0cnlpbmcgdG8gdW53aW5kIHRoZSBpbmNvbXBsZXRlIGNoYW5nZS4KCldoYXQgY2hhbmdl IGNvdWxkIGJlIG1hZGUgdG8gdGhlIGhhcmR3YXJlPyBFbmdpbmUgcmVzZXQsIHBlcmhhcHMsIGJ1 dCAKZXZlbiB0aGF0IGRvZXNuJ3QgbmVjZXNzYXJpbHkgaW52YWxpZGF0ZSBhIHJlcXVlc3QgaW4g cHJlcGFyYXRpb24gZm9yIApzZW5kaW5nIHRvIHRoZSBoYXJkd2FyZS4KClN1Ym1pdHRpbmcgYSBw YXJ0aWFsIGNoYW5nZSBzZWVtcyBsaWtlbHkgdG8gbGVhdmUgdGhlIGgvdyBpbiBhbiAKdW5kZWZp bmVkIHN0YXRlLiBDYW5jZWxsaW5nIGEgcmVxdWVzdCBpcyAob3Igb3VnaHQgdG8gYmUpIHRyaXZp YWw7IGp1c3QgCnJlc2V0IHRoZSByaW5nYnVmZmVyJ3MgdGFpbCBwb2ludGVyIHRvIHdoZXJlIGl0 IHdhcyB3aGVuIHRoZSByZXF1ZXN0IHdhcyAKYWxsb2NhdGVkLiBUaGUgZW5naW5lIGRvZXNuJ3Qg cmVhZCBhbnl0aGluZyBwYXN0IHRoZSB0YWlsIG9mIHRoZSAKcHJldmlvdXMgcmVxdWVzdCB1bnRp bCB0aGUgbmV3IHJlcXVlc3QgaXMgc3VibWl0dGVkLgoKPiBVbmZvcnR1bmF0ZWx5IHRoaXMgcGF0 Y2ggdW5kb2VzIHRoZSBleGNlc3MgYnJlYWRjcnVtYiB1c2FnZSB0aGF0IG9scgo+IHByZXZlbnRl ZCwgZS5nLiBpZiB3ZSBpbnRlcnJ1cHQgYmF0Y2hidWZmZXIgc3VibWlzc2lvbiB0aGVuIHdlIHN1 Ym1pdAo+IHRoZSByZXF1ZXN0cyBhbG9uZyB3aXRoIHRoZSBtZW1vcnkgd3JpdGVzIGFuZCBpbnRl cnJ1cHQgKGV2ZW4gdGhvdWdoIHdlCj4gZG8gbm8gcmVhbCB3b3JrKS4gRGlzYXNzb2NpYXRpbmcg cmVxdWVzdHMgZnJvbSBicmVhZGNydW1icyAoYW5kCj4gc2VtYXBob3JlcykgaXMgYSB0b3BpYyBm b3IgYSBwYXN0L2Z1dHVyZSBzZXJpZXMsIGJ1dCBub3cgbXVjaCBtb3JlCj4gaW1wb3J0YW50LgoK QW5vdGhlciBpbmNvbXByZWhlbnNpYmxlIGNvbW1lbnQ/IE9MUiB3ZW50IGF3YXkgYSBsb25nIHRp bWUgYWdvIG5vdy4gQW5kIApBRkFJSyBiYXRjaGJ1ZmZlciBzdWJtaXNzaW9uIGNhbm5vdCBiZSBp bnRlcnJ1cHRlZCBieSBhbnl0aGluZyAob3IgbW9yZSAKYWNjdXJhdGVseSwgYW55dGhpbmcgdGhh dCBpbnRlcnJ1cHRzIHN1Ym1pc3Npb24gd29uJ3QgYmUgYWJsZSB0byB3cml0ZSAKdG8gdGhlIHJp bmdidWZmZXIgb3Igc3VibWl0IG5ldyB3b3JrKS4KCj4gU2lnbmVkLW9mZi1ieTogQ2hyaXMgV2ls c29uIDxjaHJpc0BjaHJpcy13aWxzb24uY28udWs+Cj4gQ2M6IERhbmllbCBWZXR0ZXIgPGRhbmll bC52ZXR0ZXJAZmZ3bGwuY2g+Cj4gQ2M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmcKPiAtLS0KPiAg IGRyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZHJ2LmggICAgICAgICAgICB8ICAxIC0KPiAgIGRy aXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZ2VtLmMgICAgICAgICAgICB8ICA3ICsrLS0tLS0KPiAg IGRyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZ2VtX2NvbnRleHQuYyAgICB8IDIxICsrKysrKysr Ky0tLS0tLS0tLS0tLQo+ICAgZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9nZW1fZXhlY2J1ZmZl ci5jIHwgMTYgKysrKystLS0tLS0tLS0tLQo+ICAgZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxf ZGlzcGxheS5jICAgICAgIHwgIDIgKy0KPiAgIGRyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2xy Yy5jICAgICAgICAgICB8ICAxIC0KPiAgIDYgZmlsZXMgY2hhbmdlZCwgMTcgaW5zZXJ0aW9ucygr KSwgMzEgZGVsZXRpb25zKC0pCj4KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUv aTkxNV9kcnYuaCBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZHJ2LmgKPiBpbmRleCA3NDdk MmQ4NGExOGMuLmVjMjA4MTRhZGIwYyAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dwdS9kcm0vaTkx NS9pOTE1X2Rydi5oCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9kcnYuaAo+IEBA IC0yODEzLDcgKzI4MTMsNiBAQCBpbnQgaTkxNV9nZW1fc3dfZmluaXNoX2lvY3RsKHN0cnVjdCBk cm1fZGV2aWNlICpkZXYsIHZvaWQgKmRhdGEsCj4gICAJCQkgICAgIHN0cnVjdCBkcm1fZmlsZSAq ZmlsZV9wcml2KTsKPiAgIHZvaWQgaTkxNV9nZW1fZXhlY2J1ZmZlcl9tb3ZlX3RvX2FjdGl2ZShz dHJ1Y3QgbGlzdF9oZWFkICp2bWFzLAo+ICAgCQkJCQlzdHJ1Y3QgZHJtX2k5MTVfZ2VtX3JlcXVl c3QgKnJlcSk7Cj4gLXZvaWQgaTkxNV9nZW1fZXhlY2J1ZmZlcl9yZXRpcmVfY29tbWFuZHMoc3Ry dWN0IGk5MTVfZXhlY2J1ZmZlcl9wYXJhbXMgKnBhcmFtcyk7Cj4gICBpbnQgaTkxNV9nZW1fcmlu Z2J1ZmZlcl9zdWJtaXNzaW9uKHN0cnVjdCBpOTE1X2V4ZWNidWZmZXJfcGFyYW1zICpwYXJhbXMs Cj4gICAJCQkJICAgc3RydWN0IGRybV9pOTE1X2dlbV9leGVjYnVmZmVyMiAqYXJncywKPiAgIAkJ CQkgICBzdHJ1Y3QgbGlzdF9oZWFkICp2bWFzKTsKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUv ZHJtL2k5MTUvaTkxNV9nZW0uYyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZ2VtLmMKPiBp bmRleCAzYWI1Mjk2Njk0NDguLmZkMjQ4NzdlYjBhMCAxMDA2NDQKPiAtLS0gYS9kcml2ZXJzL2dw dS9kcm0vaTkxNS9pOTE1X2dlbS5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9n ZW0uYwo+IEBAIC0zMzg0LDEyICszMzg0LDkgQEAgaW50IGk5MTVfZ3B1X2lkbGUoc3RydWN0IGRy bV9kZXZpY2UgKmRldikKPiAgIAkJCQlyZXR1cm4gcmV0Owo+Cj4gICAJCQlyZXQgPSBpOTE1X3N3 aXRjaF9jb250ZXh0KHJlcSk7Cj4gLQkJCWlmIChyZXQpIHsKPiAtCQkJCWk5MTVfZ2VtX3JlcXVl c3RfY2FuY2VsKHJlcSk7Cj4gLQkJCQlyZXR1cm4gcmV0Owo+IC0JCQl9Cj4gLQo+ICAgCQkJaTkx NV9hZGRfcmVxdWVzdF9ub19mbHVzaChyZXEpOwo+ICsJCQlpZiAocmV0KQo+ICsJCQkJcmV0dXJu IHJldDsKClRoaXMgc2VlbXMgbGlrZSBhIGJhZCBpZGVhLiBMb29raW5nIGF0IGhvdyB3ZSBjb3Vs ZCBnZXQgaGVyZSAoaS5lLiBob3cgCmNvdWxkIHN3aXRjaF9jb250ZXh0KCkgcmV0dXJuIGFuIGVy cm9yKSwgd2Ugc2VlIHRoaW5ncyBzdWNoIGFzICJmYWlsZWQgCnRvIHBpbiIgb3IgaTkxNV9nZW1f b2JqZWN0X2dldF9wYWdlcygpIGZhaWxlZC4KCldpdGggbm8gcmVhbCBpZGVhIG9mIHdoYXQgdGhl IEdQVSBhbmQvb3IgQ1BVIGFkZHJlc3Mgc3BhY2VzIGNvbnRhaW4gYXQgCnRoaXMgcG9pbnQsIGl0 IHNlZW1zIHVud2lzZSB0byBjaGFyZ2UgYWhlYWQgcmVnYXJkbGVzcy4KCi5EYXZlLgoKPiAgIAkJ fQo+Cj4gICAJCXJldCA9IGludGVsX3JpbmdfaWRsZShyaW5nKTsKPiBkaWZmIC0tZ2l0IGEvZHJp dmVycy9ncHUvZHJtL2k5MTUvaTkxNV9nZW1fY29udGV4dC5jIGIvZHJpdmVycy9ncHUvZHJtL2k5 MTUvaTkxNV9nZW1fY29udGV4dC5jCj4gaW5kZXggYzI1MDgzYzc4YmE3Li5lNWU5YTg5MThmMTkg MTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9nZW1fY29udGV4dC5jCj4g KysrIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9nZW1fY29udGV4dC5jCj4gQEAgLTY2MSw3 ICs2NjEsNiBAQCBzdGF0aWMgaW50IGRvX3N3aXRjaChzdHJ1Y3QgZHJtX2k5MTVfZ2VtX3JlcXVl c3QgKnJlcSkKPiAgIAlzdHJ1Y3QgZHJtX2k5MTVfcHJpdmF0ZSAqZGV2X3ByaXYgPSByaW5nLT5k ZXYtPmRldl9wcml2YXRlOwo+ICAgCXN0cnVjdCBpbnRlbF9jb250ZXh0ICpmcm9tID0gcmluZy0+ bGFzdF9jb250ZXh0Owo+ICAgCXUzMiBod19mbGFncyA9IDA7Cj4gLQlib29sIHVuaW5pdGlhbGl6 ZWQgPSBmYWxzZTsKPiAgIAlpbnQgcmV0LCBpOwo+Cj4gICAJaWYgKGZyb20gIT0gTlVMTCAmJiBy aW5nID09ICZkZXZfcHJpdi0+cmluZ1tSQ1NdKSB7Cj4gQEAgLTc2OCw2ICs3NjcsMTUgQEAgc3Rh dGljIGludCBkb19zd2l0Y2goc3RydWN0IGRybV9pOTE1X2dlbV9yZXF1ZXN0ICpyZXEpCj4gICAJ CQl0by0+cmVtYXBfc2xpY2UgJj0gfigxPDxpKTsKPiAgIAl9Cj4KPiArCWlmICghdG8tPmxlZ2Fj eV9od19jdHguaW5pdGlhbGl6ZWQpIHsKPiArCQlpZiAocmluZy0+aW5pdF9jb250ZXh0KSB7Cj4g KwkJCXJldCA9IHJpbmctPmluaXRfY29udGV4dChyZXEpOwo+ICsJCQlpZiAocmV0KQo+ICsJCQkJ Z290byB1bnBpbl9vdXQ7Cj4gKwkJfQo+ICsJCXRvLT5sZWdhY3lfaHdfY3R4LmluaXRpYWxpemVk ID0gdHJ1ZTsKPiArCX0KPiArCj4gICAJLyogVGhlIGJhY2tpbmcgb2JqZWN0IGZvciB0aGUgY29u dGV4dCBpcyBkb25lIGFmdGVyIHN3aXRjaGluZyB0byB0aGUKPiAgIAkgKiAqbmV4dCogY29udGV4 dC4gVGhlcmVmb3JlIHdlIGNhbm5vdCByZXRpcmUgdGhlIHByZXZpb3VzIGNvbnRleHQgdW50aWwK PiAgIAkgKiB0aGUgbmV4dCBjb250ZXh0IGhhcyBhbHJlYWR5IHN0YXJ0ZWQgcnVubmluZy4gSW4g ZmFjdCwgdGhlIGJlbG93IGNvZGUKPiBAQCAtNzkxLDIxICs3OTksMTAgQEAgc3RhdGljIGludCBk b19zd2l0Y2goc3RydWN0IGRybV9pOTE1X2dlbV9yZXF1ZXN0ICpyZXEpCj4gICAJCWk5MTVfZ2Vt X2NvbnRleHRfdW5yZWZlcmVuY2UoZnJvbSk7Cj4gICAJfQo+Cj4gLQl1bmluaXRpYWxpemVkID0g IXRvLT5sZWdhY3lfaHdfY3R4LmluaXRpYWxpemVkOwo+IC0JdG8tPmxlZ2FjeV9od19jdHguaW5p dGlhbGl6ZWQgPSB0cnVlOwo+IC0KPiAgIGRvbmU6Cj4gICAJaTkxNV9nZW1fY29udGV4dF9yZWZl cmVuY2UodG8pOwo+ICAgCXJpbmctPmxhc3RfY29udGV4dCA9IHRvOwo+Cj4gLQlpZiAodW5pbml0 aWFsaXplZCkgewo+IC0JCWlmIChyaW5nLT5pbml0X2NvbnRleHQpIHsKPiAtCQkJcmV0ID0gcmlu Zy0+aW5pdF9jb250ZXh0KHJlcSk7Cj4gLQkJCWlmIChyZXQpCj4gLQkJCQlEUk1fRVJST1IoInJp bmcgaW5pdCBjb250ZXh0OiAlZFxuIiwgcmV0KTsKPiAtCQl9Cj4gLQl9Cj4gLQo+ICAgCXJldHVy biAwOwo+Cj4gICB1bnBpbl9vdXQ6Cj4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvZ3B1L2RybS9pOTE1 L2k5MTVfZ2VtX2V4ZWNidWZmZXIuYyBiL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZ2VtX2V4 ZWNidWZmZXIuYwo+IGluZGV4IGRjY2I1MTczNjFiMy4uYjgxODZiZDA2MWMxIDEwMDY0NAo+IC0t LSBhL2RyaXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZ2VtX2V4ZWNidWZmZXIuYwo+ICsrKyBiL2Ry aXZlcnMvZ3B1L2RybS9pOTE1L2k5MTVfZ2VtX2V4ZWNidWZmZXIuYwo+IEBAIC0xMTM2LDcgKzEx MzYsNyBAQCBpOTE1X2dlbV9leGVjYnVmZmVyX21vdmVfdG9fYWN0aXZlKHN0cnVjdCBsaXN0X2hl YWQgKnZtYXMsCj4gICAJfQo+ICAgfQo+Cj4gLXZvaWQKPiArc3RhdGljIHZvaWQKPiAgIGk5MTVf Z2VtX2V4ZWNidWZmZXJfcmV0aXJlX2NvbW1hbmRzKHN0cnVjdCBpOTE1X2V4ZWNidWZmZXJfcGFy YW1zICpwYXJhbXMpCj4gICB7Cj4gICAJLyogVW5jb25kaXRpb25hbGx5IGZvcmNlIGFkZF9yZXF1 ZXN0IHRvIGVtaXQgYSBmdWxsIGZsdXNoLiAqLwo+IEBAIC0xMzE4LDcgKzEzMTgsNiBAQCBpOTE1 X2dlbV9yaW5nYnVmZmVyX3N1Ym1pc3Npb24oc3RydWN0IGk5MTVfZXhlY2J1ZmZlcl9wYXJhbXMg KnBhcmFtcywKPiAgIAl0cmFjZV9pOTE1X2dlbV9yaW5nX2Rpc3BhdGNoKHBhcmFtcy0+cmVxdWVz dCwgcGFyYW1zLT5kaXNwYXRjaF9mbGFncyk7Cj4KPiAgIAlpOTE1X2dlbV9leGVjYnVmZmVyX21v dmVfdG9fYWN0aXZlKHZtYXMsIHBhcmFtcy0+cmVxdWVzdCk7Cj4gLQlpOTE1X2dlbV9leGVjYnVm ZmVyX3JldGlyZV9jb21tYW5kcyhwYXJhbXMpOwo+Cj4gICAJcmV0dXJuIDA7Cj4gICB9Cj4gQEAg LTE2MDcsOCArMTYwNiwxMCBAQCBpOTE1X2dlbV9kb19leGVjYnVmZmVyKHN0cnVjdCBkcm1fZGV2 aWNlICpkZXYsIHZvaWQgKmRhdGEsCj4gICAJCWdvdG8gZXJyX2JhdGNoX3VucGluOwo+Cj4gICAJ cmV0ID0gaTkxNV9nZW1fcmVxdWVzdF9hZGRfdG9fY2xpZW50KHBhcmFtcy0+cmVxdWVzdCwgZmls ZSk7Cj4gLQlpZiAocmV0KQo+ICsJaWYgKHJldCkgewo+ICsJCWk5MTVfZ2VtX3JlcXVlc3RfY2Fu Y2VsKHBhcmFtcy0+cmVxdWVzdCk7Cj4gICAJCWdvdG8gZXJyX2JhdGNoX3VucGluOwo+ICsJfQo+ Cj4gICAJLyoKPiAgIAkgKiBTYXZlIGFzc29ydGVkIHN0dWZmIGF3YXkgdG8gcGFzcyB0aHJvdWdo IHRvICpfc3VibWlzc2lvbigpLgo+IEBAIC0xNjI0LDYgKzE2MjUsNyBAQCBpOTE1X2dlbV9kb19l eGVjYnVmZmVyKHN0cnVjdCBkcm1fZGV2aWNlICpkZXYsIHZvaWQgKmRhdGEsCj4gICAJcGFyYW1z LT5jdHggICAgICAgICAgICAgICAgICAgICA9IGN0eDsKPgo+ICAgCXJldCA9IGRldl9wcml2LT5n dC5leGVjYnVmX3N1Ym1pdChwYXJhbXMsIGFyZ3MsICZlYi0+dm1hcyk7Cj4gKwlpOTE1X2dlbV9l eGVjYnVmZmVyX3JldGlyZV9jb21tYW5kcyhwYXJhbXMpOwo+Cj4gICBlcnJfYmF0Y2hfdW5waW46 Cj4gICAJLyoKPiBAQCAtMTY0MCwxNCArMTY0Miw2IEBAIGVycjoKPiAgIAlpOTE1X2dlbV9jb250 ZXh0X3VucmVmZXJlbmNlKGN0eCk7Cj4gICAJZWJfZGVzdHJveShlYik7Cj4KPiAtCS8qCj4gLQkg KiBJZiB0aGUgcmVxdWVzdCB3YXMgY3JlYXRlZCBidXQgbm90IHN1Y2Nlc3NmdWxseSBzdWJtaXR0 ZWQgdGhlbiBpdAo+IC0JICogbXVzdCBiZSBmcmVlZCBhZ2Fpbi4gSWYgaXQgd2FzIHN1Ym1pdHRl ZCB0aGVuIGl0IGlzIGJlaW5nIHRyYWNrZWQKPiAtCSAqIG9uIHRoZSBhY3RpdmUgcmVxdWVzdCBs aXN0IGFuZCBubyBjbGVhbiB1cCBpcyByZXF1aXJlZCBoZXJlLgo+IC0JICovCj4gLQlpZiAocmV0 ICYmIHBhcmFtcy0+cmVxdWVzdCkKPiAtCQlpOTE1X2dlbV9yZXF1ZXN0X2NhbmNlbChwYXJhbXMt PnJlcXVlc3QpOwo+IC0KPiAgIAltdXRleF91bmxvY2soJmRldi0+c3RydWN0X211dGV4KTsKPgo+ ICAgcHJlX211dGV4X2VycjoKPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50 ZWxfZGlzcGxheS5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfZGlzcGxheS5jCj4gaW5k ZXggYjRjZjljZTE2MTU1Li45NTk4NjhjNDAwMTggMTAwNjQ0Cj4gLS0tIGEvZHJpdmVycy9ncHUv ZHJtL2k5MTUvaW50ZWxfZGlzcGxheS5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50 ZWxfZGlzcGxheS5jCj4gQEAgLTExNzUxLDcgKzExNzUxLDcgQEAgY2xlYW51cF91bnBpbjoKPiAg IAlpbnRlbF91bnBpbl9mYl9vYmooZmIsIGNydGMtPnByaW1hcnktPnN0YXRlKTsKPiAgIGNsZWFu dXBfcGVuZGluZzoKPiAgIAlpZiAocmVxdWVzdCkKPiAtCQlpOTE1X2dlbV9yZXF1ZXN0X2NhbmNl bChyZXF1ZXN0KTsKPiArCQlpOTE1X2FkZF9yZXF1ZXN0X25vX2ZsdXNoKHJlcXVlc3QpOwo+ICAg CWF0b21pY19kZWMoJmludGVsX2NydGMtPnVucGluX3dvcmtfY291bnQpOwo+ICAgCW11dGV4X3Vu bG9jaygmZGV2LT5zdHJ1Y3RfbXV0ZXgpOwo+ICAgY2xlYW51cDoKPiBkaWZmIC0tZ2l0IGEvZHJp dmVycy9ncHUvZHJtL2k5MTUvaW50ZWxfbHJjLmMgYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pbnRl bF9scmMuYwo+IGluZGV4IGY3ZmFjNWYzYjVjZS4uN2YxN2JhODUyYjhhIDEwMDY0NAo+IC0tLSBh L2RyaXZlcnMvZ3B1L2RybS9pOTE1L2ludGVsX2xyYy5jCj4gKysrIGIvZHJpdmVycy9ncHUvZHJt L2k5MTUvaW50ZWxfbHJjLmMKPiBAQCAtOTcyLDcgKzk3Miw2IEBAIGludCBpbnRlbF9leGVjbGlz dHNfc3VibWlzc2lvbihzdHJ1Y3QgaTkxNV9leGVjYnVmZmVyX3BhcmFtcyAqcGFyYW1zLAo+ICAg CXRyYWNlX2k5MTVfZ2VtX3JpbmdfZGlzcGF0Y2gocGFyYW1zLT5yZXF1ZXN0LCBwYXJhbXMtPmRp c3BhdGNoX2ZsYWdzKTsKPgo+ICAgCWk5MTVfZ2VtX2V4ZWNidWZmZXJfbW92ZV90b19hY3RpdmUo dm1hcywgcGFyYW1zLT5yZXF1ZXN0KTsKPiAtCWk5MTVfZ2VtX2V4ZWNidWZmZXJfcmV0aXJlX2Nv bW1hbmRzKHBhcmFtcyk7Cj4KPiAgIAlyZXR1cm4gMDsKPiAgIH0KPgoKX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX18KSW50ZWwtZ2Z4IG1haWxpbmcgbGlzdApJ bnRlbC1nZnhAbGlzdHMuZnJlZWRlc2t0b3Aub3JnCmh0dHA6Ly9saXN0cy5mcmVlZGVza3RvcC5v cmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga11.intel.com ([192.55.52.93]:20508 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1763366AbcALSQY (ORCPT ); Tue, 12 Jan 2016 13:16:24 -0500 Subject: Re: [Intel-gfx] [PATCH 004/190] drm/i915: Fix some invalid requests cancellations To: Chris Wilson , intel-gfx@lists.freedesktop.org References: <1452503961-14837-1-git-send-email-chris@chris-wilson.co.uk> <1452503961-14837-4-git-send-email-chris@chris-wilson.co.uk> Cc: Daniel Vetter , stable@vger.kernel.org From: Dave Gordon Message-ID: <569542F5.7040704@intel.com> Date: Tue, 12 Jan 2016 18:16:21 +0000 MIME-Version: 1.0 In-Reply-To: <1452503961-14837-4-git-send-email-chris@chris-wilson.co.uk> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Sender: stable-owner@vger.kernel.org List-ID: On 11/01/16 09:16, 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 > dangling request; i915_wait_request() simply skips the wait and so we > may unbind the object whilst it is still active. I don't understand this; context objects don't point to requests, it's vice versa. The request has a pointer to the engine, and to the context, and adds +1 to the refcount on the latter. > 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. What change could be made to the hardware? Engine reset, perhaps, but even that doesn't necessarily invalidate a request in preparation for sending to the hardware. Submitting a partial change seems likely to leave the h/w in an undefined state. Cancelling a request is (or ought to be) trivial; just reset the ringbuffer's tail pointer to where it was when the request was allocated. The engine doesn't read anything past the tail of the previous request until the new request is submitted. > 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. Another incomprehensible comment? OLR went away a long time ago now. And AFAIK batchbuffer submission cannot be interrupted by anything (or more accurately, anything that interrupts submission won't be able to write to the ringbuffer or submit new work). > 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 747d2d84a18c..ec20814adb0c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2813,7 +2813,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 3ab529669448..fd24877eb0a0 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3384,12 +3384,9 @@ int i915_gpu_idle(struct drm_device *dev) > return ret; > > ret = i915_switch_context(req); > - if (ret) { > - i915_gem_request_cancel(req); > - return ret; > - } > - > i915_add_request_no_flush(req); > + if (ret) > + return ret; This seems like a bad idea. Looking at how we could get here (i.e. how could switch_context() return an error), we see things such as "failed to pin" or i915_gem_object_get_pages() failed. With no real idea of what the GPU and/or CPU address spaces contain at this point, it seems unwise to charge ahead regardless. .Dave. > } > > 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 c25083c78ba7..e5e9a8918f19 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -661,7 +661,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]) { > @@ -768,6 +767,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 > @@ -791,21 +799,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 dccb517361b3..b8186bd061c1 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1136,7 +1136,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); > > return 0; > } > @@ -1607,8 +1606,10 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > goto err_batch_unpin; > > ret = i915_gem_request_add_to_client(params->request, file); > - if (ret) > + if (ret) { > + i915_gem_request_cancel(params->request); > goto err_batch_unpin; > + } > > /* > * Save assorted stuff away to pass through to *_submission(). > @@ -1624,6 +1625,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > params->ctx = ctx; > > ret = dev_priv->gt.execbuf_submit(params, args, &eb->vmas); > + i915_gem_execbuffer_retire_commands(params); > > err_batch_unpin: > /* > @@ -1640,14 +1642,6 @@ err: > i915_gem_context_unreference(ctx); > eb_destroy(eb); > > - /* > - * If the request was created but not successfully submitted then it > - * must be freed again. If it was submitted then it is being tracked > - * on the active request list and no clean up is required here. > - */ > - if (ret && params->request) > - i915_gem_request_cancel(params->request); > - > mutex_unlock(&dev->struct_mutex); > > pre_mutex_err: > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index b4cf9ce16155..959868c40018 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -11751,7 +11751,7 @@ cleanup_unpin: > intel_unpin_fb_obj(fb, crtc->primary->state); > cleanup_pending: > if (request) > - i915_gem_request_cancel(request); > + i915_add_request_no_flush(request); > atomic_dec(&intel_crtc->unpin_work_count); > mutex_unlock(&dev->struct_mutex); > cleanup: > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index f7fac5f3b5ce..7f17ba852b8a 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -972,7 +972,6 @@ int intel_execlists_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); > > return 0; > } >