From mboxrd@z Thu Jan 1 00:00:00 1970 From: Joonas Lahtinen Subject: Re: [PATCH] drm/i915: Drop support for I915_EXEC_CONSTANTS_* execbuf parameters. Date: Wed, 15 Feb 2017 14:59:32 +0200 Message-ID: <1487163572.8490.29.camel@linux.intel.com> References: <20170215093446.21291-1-kenneth@whitecape.org> <20170215104007.GU18048@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by gabe.freedesktop.org (Postfix) with ESMTPS id 43A076E914 for ; Wed, 15 Feb 2017 12:59:36 +0000 (UTC) In-Reply-To: <20170215104007.GU18048@nuc-i3427.alporthouse.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Chris Wilson , Kenneth Graunke Cc: daniel.vetter@ffwll.ch, intel-gfx@lists.freedesktop.org, stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24ga2UsIDIwMTctMDItMTUgYXQgMTA6NDAgKzAwMDAsIENocmlzIFdpbHNvbiB3cm90ZToKPiBP biBXZWQsIEZlYiAxNSwgMjAxNyBhdCAwMTozNDo0NkFNIC0wODAwLCBLZW5uZXRoIEdyYXVua2Ug d3JvdGU6Cj4gPiAKPiA+IFRoaXMgcGF0Y2ggbWFrZXMgdGhlIEk5MTVfUEFSQU1fSEFTX0VYRUNf Q09OU1RBTlRTIGdldHBhcmFtIHJldHVybiAwCj4gPiAoaW5kaWNhdGluZyB0aGUgb3B0aW9uYWwg ZmVhdHVyZSBpcyBub3Qgc3VwcG9ydGVkKSwgYW5kIG1ha2VzIGV4ZWNidWYKPiA+IGFsd2F5cyBy ZXR1cm4gLUVJTlZBTCBpZiB0aGUgZmxhZ3MgYXJlIHVzZWQuCj4gPiAKPiA+IEFwcGFyZW50bHks IG5vIHVzZXJzcGFjZSBldmVyIHNoaXBwZWQgd2hpY2ggdXNlZCB0aGlzIG9wdGlvbmFsIGZlYXR1 cmU6Cj4gPiBJIGNoZWNrZWQgdGhlIGdpdCBoaXN0b3J5IG9mIE1lc2EsIHhmODYtdmlkZW8taW50 ZWwsIGxpYnZhLCBhbmQgQmVpZ25ldCwKPiA+IGFuZCB0aGVyZSB3ZXJlIHplcm8gY29tbWl0cyBz aG93aW5nIGEgdXNlIG9mIHRoZXNlIGZsYWdzLsKgwqBLZXJuZWwgY29tbWl0Cj4gPiA3MmJmYTE5 YzhkZWI0IGFwcGFyZW50bHkgaW50cm9kdWNlZCB0aGUgZmVhdHVyZSBwcmVtYXR1cmVseS7CoMKg QWNjb3JkaW5nCj4gPiB0byBDaHJpcywgdGhlIGludGVudGlvbiB3YXMgdG8gdXNlIHRoaXMgaW4g Y2Fpcm8tZHJtLCBidXQgInRoZSB1c2Ugd2FzCj4gPiBicm9rZW4gZm9yIGdlbjYiLCBzbyBJIGRv bid0IHRoaW5rIGl0IGV2ZXIgaGFwcGVuZWQuCj4gPiAKPiA+ICdyZWxhdGl2ZV9jb25zdGFudHNf bW9kZScgaGFzIGFsd2F5cyBiZWVuIHRyYWNrZWQgcGVyLWRldmljZSwgYnV0IHRoaXMKPiA+IGhh cyBhY3R1YWxseSBiZWVuIHdyb25nIGV2ZXIgc2luY2UgaGFyZHdhcmUgY29udGV4dHMgd2VyZSBp bnRyb2R1Y2VkLCBhcwo+ID4gdGhlIElOU1RQTSByZWdpc3RlciBpcyBzYXZlZCAoYW5kIGF1dG9t YXRpY2FsbHkgcmVzdG9yZWQpIGFzIHBhcnQgb2YgdGhlCj4gPiByZW5kZXIgcmluZyBjb250ZXh0 LiBUaGUgc29mdHdhcmUgcGVyLWRldmljZSB2YWx1ZSBjb3VsZCB0aGVyZWZvcmUgZ2V0Cj4gPiBv dXQgb2Ygc3luYyB3aXRoIHRoZSBoYXJkd2FyZSBwZXItY29udGV4dCB2YWx1ZS7CoMKgVGhpcyBt ZWFudCB0aGF0IHVzaW5nCj4gPiB0aGVtIGlzIGFjdHVhbGx5IHVuc2FmZTogYSBjbGllbnQgd2hp Y2ggdHJpZWQgdG8gdXNlIHRoZW0gY291bGQgZGFtYWdlCj4gPiB0aGUgc3RhdGUgb2Ygb3RoZXIg Y2xpZW50cywgY2F1c2luZyB0aGUgR1BVIHRvIGludGVycHJldCB0aGVpciBCTwo+ID4gb2Zmc2V0 cyBhcyBhYnNvbHV0ZSBwb2ludGVycywgbGVhZGluZyB0byBib2d1cyBtZW1vcnkgcmVhZHMuCj4g PiAKPiA+IFRoZXNlIGZsYWdzIHdlcmUgYWxzbyBuZXZlciBwb3J0ZWQgdG8gZXhlY2xpc3QgbW9k ZSwgbWFraW5nIHRoZW0gbm8tb3BzCj4gPiBvbiBHZW45KyAod2hpY2ggcmVxdWlyZXMgZXhlY2xp c3RzKSwgYW5kIEdlbjggaW4gdGhlIGRlZmF1bHQgbW9kZS4KPiA+IAo+ID4gT24gR2VuOCssIHVz ZXJzcGFjZSBjYW4gd3JpdGUgdGhlc2UgcmVnaXN0ZXJzIGRpcmVjdGx5LCBhY2hpZXZpbmcgdGhl Cj4gPiBzYW1lIGVmZmVjdC7CoMKgT24gR2VuNi03LjUsIGl0IGxpa2VseSBtYWtlcyBzZW5zZSB0 byBleHRlbmQgdGhlIGNvbW1hbmQKPiA+IHBhcnNlciB0byBzdXBwb3J0IHRoZW0uwqDCoEkgZG9u J3QgdGhpbmsgYW55b25lIHdhbnRzIHRoaXMgb24gR2VuNC01Lgo+ID4gCj4gPiBCYXNlZCBvbiBh IHBhdGNoIGJ5IERhdmUgR29yZG9uLgo+ID4gCj4gPiB2MzogUmV0dXJuIC1FTk9ERVYgZm9yIHRo ZSBnZXRwYXJhbSwgYXMgdGhpcyBpcyB3aGF0IHdlIGRvIGZvciBvdGhlcgo+ID4gwqDCoMKgwqBv YnNvbGV0ZSBmZWF0dXJlcy7CoMKgU3VnZ2VzdGVkIGJ5IENocmlzIFdpbHNvbi4KPiA+IAo+ID4g Q2M6IHN0YWJsZUB2Z2VyLmtlcm5lbC5vcmcKPiA+IEJ1Z3ppbGxhOiBodHRwczovL2J1Z3MuZnJl ZWRlc2t0b3Aub3JnL3Nob3dfYnVnLmNnaT9pZD05MjQ0OAo+ID4gU2lnbmVkLW9mZi1ieTogS2Vu bmV0aCBHcmF1bmtlIDxrZW5uZXRoQHdoaXRlY2FwZS5vcmc+Cj4gPiBSZXZpZXdlZC1ieTogSm9v bmFzIExhaHRpbmVuIDxqb29uYXMubGFodGluZW5AbGludXguaW50ZWwuY29tPiBbdjJdCj4gPiBS ZXZpZXdlZC1ieTogQ2hyaXMgV2lsc29uIDxjaHJpc0BjaHJpcy13aWxzb24uY28udWs+IFt2Ml0K PiBSZXZpZXdlZC1ieTogQ2hyaXMgV2lsc29uIDxjaHJpc0BjaHJpcy13aWxzb24uY28udWs+Cgo8 U05JUD4KCj4gSnVzdCBnaXZlIERhbmllbCBhIGNoYW5jZSB0byBhY2sgdGhlIEFCSSByZXZlcnQg aWYgaGUgY2FyZXMuLi4KCkknbSBqdXN0IHRoaW5raW5nIHRoaXMgbWlnaHQgYnJlYWsgYW4gYXBw bGljYXRpb24gd2hpY2ggZG9lcyB0aGUKZmVhdHVyZSBkZXRlY3Rpb24gYW5kIG9ubHkgdW5kZXJz dGFuZHMgMCBvciAxLgoKRWl0aGVyIHdheSwgd2l0aCBBLWIgZnJvbSBEYW5pZWwsIHRoaXMgdG9v IGlzOwoKUmV2aWV3ZWQtYnk6IEpvb25hcyBMYWh0aW5lbiA8am9vbmFzLmxhaHRpbmVuQGxpbnV4 LmludGVsLmNvbT4KClJlZ2FyZHMsIEpvb25hcwotLSAKSm9vbmFzIExhaHRpbmVuCk9wZW4gU291 cmNlIFRlY2hub2xvZ3kgQ2VudGVyCkludGVsIENvcnBvcmF0aW9uCl9fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fCkludGVsLWdmeCBtYWlsaW5nIGxpc3QKSW50 ZWwtZ2Z4QGxpc3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9y Zy9tYWlsbWFuL2xpc3RpbmZvL2ludGVsLWdmeAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com ([134.134.136.65]:18480 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751778AbdBOM7r (ORCPT ); Wed, 15 Feb 2017 07:59:47 -0500 Message-ID: <1487163572.8490.29.camel@linux.intel.com> Subject: Re: [PATCH] drm/i915: Drop support for I915_EXEC_CONSTANTS_* execbuf parameters. From: Joonas Lahtinen To: Chris Wilson , Kenneth Graunke Cc: intel-gfx@lists.freedesktop.org, stable@vger.kernel.org, daniel.vetter@ffwll.ch Date: Wed, 15 Feb 2017 14:59:32 +0200 In-Reply-To: <20170215104007.GU18048@nuc-i3427.alporthouse.com> References: <20170215093446.21291-1-kenneth@whitecape.org> <20170215104007.GU18048@nuc-i3427.alporthouse.com> Content-Type: text/plain; charset="UTF-8" Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: stable-owner@vger.kernel.org List-ID: On ke, 2017-02-15 at 10:40 +0000, Chris Wilson wrote: > On Wed, Feb 15, 2017 at 01:34:46AM -0800, Kenneth Graunke wrote: > > > > This patch makes the I915_PARAM_HAS_EXEC_CONSTANTS getparam return 0 > > (indicating the optional feature is not supported), and makes execbuf > > always return -EINVAL if the flags are used. > > > > Apparently, no userspace ever shipped which used this optional feature: > > I checked the git history of Mesa, xf86-video-intel, libva, and Beignet, > > and there were zero commits showing a use of these flags.  Kernel commit > > 72bfa19c8deb4 apparently introduced the feature prematurely.  According > > to Chris, the intention was to use this in cairo-drm, but "the use was > > broken for gen6", so I don't think it ever happened. > > > > 'relative_constants_mode' has always been tracked per-device, but this > > has actually been wrong ever since hardware contexts were introduced, as > > the INSTPM register is saved (and automatically restored) as part of the > > render ring context. The software per-device value could therefore get > > out of sync with the hardware per-context value.  This meant that using > > them is actually unsafe: a client which tried to use them could damage > > the state of other clients, causing the GPU to interpret their BO > > offsets as absolute pointers, leading to bogus memory reads. > > > > These flags were also never ported to execlist mode, making them no-ops > > on Gen9+ (which requires execlists), and Gen8 in the default mode. > > > > On Gen8+, userspace can write these registers directly, achieving the > > same effect.  On Gen6-7.5, it likely makes sense to extend the command > > parser to support them.  I don't think anyone wants this on Gen4-5. > > > > Based on a patch by Dave Gordon. > > > > v3: Return -ENODEV for the getparam, as this is what we do for other > >     obsolete features.  Suggested by Chris Wilson. > > > > Cc: stable@vger.kernel.org > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=92448 > > Signed-off-by: Kenneth Graunke > > Reviewed-by: Joonas Lahtinen [v2] > > Reviewed-by: Chris Wilson [v2] > Reviewed-by: Chris Wilson > Just give Daniel a chance to ack the ABI revert if he cares... I'm just thinking this might break an application which does the feature detection and only understands 0 or 1. Either way, with A-b from Daniel, this too is; Reviewed-by: Joonas Lahtinen Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation