From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH v2] drm/i915: Limit the number of loops for reading a split 64bit register Date: Wed, 09 Sep 2015 11:13:10 +0300 Message-ID: <87wpw0m2nd.fsf@intel.com> References: <8737ypnl4f.fsf@intel.com> <1441718233-29112-1-git-send-email-chris@chris-wilson.co.uk> <20150908190059.GG2767@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id 180216EAF2 for ; Wed, 9 Sep 2015 01:10:01 -0700 (PDT) In-Reply-To: <20150908190059.GG2767@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter , Chris Wilson Cc: Daniel Vetter , intel-gfx@lists.freedesktop.org, stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org T24gVHVlLCAwOCBTZXAgMjAxNSwgRGFuaWVsIFZldHRlciA8ZGFuaWVsQGZmd2xsLmNoPiB3cm90 ZToKPiBPbiBUdWUsIFNlcCAwOCwgMjAxNSBhdCAwMjoxNzoxM1BNICswMTAwLCBDaHJpcyBXaWxz b24gd3JvdGU6Cj4+IEluIEk5MTVfUkVBRDY0XzJ4MzIgd2UgYXR0ZW1wdCB0byByZWFkIGEgNjRi aXQgcmVnaXN0ZXIgdXNpbmcgMiAzMmJpdAo+PiByZWFkcy4gRHVlIHRvIHRoZSBuYXR1cmUgb2Yg dGhlIHJlZ2lzdGVycyB3ZSB0cnkgdG8gcmVhZCBpbiB0aGlzIG1hbm5lciwKPj4gdGhleSBtYXkg aW5jcmVtZW50IGJldHdlZW4gdGhlIHR3byBpbnN0cnVjdGlvbiAoZS5nLiBhIHRpbWVzdGFtcAo+ PiBjb3VudGVyKS4gVG8ga2VlcCB0aGUgcmVzdWx0IGFjY3VyYXRlLCB3ZSByZXBlYXQgdGhlIHJl YWQgaWYgd2UgZGV0ZWN0Cj4+IGFuIG92ZXJmbG93IChpLmUuIHRoZSB1cHBlciB2YWx1ZSB2YXJp ZXMpLiBIb3dldmVyLCBzb21lIGhhcndhcmUgaXMganVzdAo+PiBwbGFpbiBmbGFreSBhbmQgbWF5 IGVuZGxlc3MgbG9vcCBhcyB0aGUgdGhlIHVwcGVyIDMyYml0cyBhcmUgbm90IHN0YWJsZS4KPj4g SnVzdCBnaXZlIHVwIGFmdGVyIGEgY291cGxlIG9mIHRyaWVzIGFuZCByZXBvcnQgd2hhdGV2ZXIg d2UgcmVhZCBsYXN0Lgo+PiAKPj4gdjI6IFVzZSB0aGUgbW9zdCByZWNlbnQgdmFsdWVzIHdoZW4g ZXJyaW5nIG91dCBvbiBhbiB1bnN0YWJsZSByZWdpc3Rlci4KPj4gCj4+IFJlcG9ydGVkLWJ5OiBy dXNzaWFubmV1cm9tYW5jZXJAeWEucnUKPj4gQnVnemlsbGE6IGh0dHBzOi8vYnVncy5mcmVlZGVz a3RvcC5vcmcvc2hvd19idWcuY2dpP2lkPTkxOTA2Cj4+IFNpZ25lZC1vZmYtYnk6IENocmlzIFdp bHNvbiA8Y2hyaXNAY2hyaXMtd2lsc29uLmNvLnVrPgo+PiBDYzogTWljaGHFgiBXaW5pYXJza2kg PG1pY2hhbC53aW5pYXJza2lAaW50ZWwuY29tPgo+PiBDYzogRGFuaWVsIFZldHRlciA8ZGFuaWVs LnZldHRlckBmZndsbC5jaD4KPj4gQ2M6IEphbmkgTmlrdWxhIDxqYW5pLm5pa3VsYUBsaW51eC5p bnRlbC5jb20+Cj4+IENjOiBzdGFibGVAdmdlci5rZXJuZWwub3JnCj4KPiBTdGlsbCBSZXZpZXdl ZC1ieTogRGFuaWVsIFZldHRlciA8ZGFuaWVsLnZldHRlckBmZndsbC5jaD4KClB1c2hlZCB0byBk cm0taW50ZWwtbmV4dC1maXhlcywgdGhhbmtzIGZvciB0aGUgcGF0Y2ggYW5kIHJldmlldy4KCkJS LApKYW5pLgoKCj4KPj4gLS0tCj4+ICBkcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X2Rydi5oIHwg MTAgKysrKystLS0tLQo+PiAgMSBmaWxlIGNoYW5nZWQsIDUgaW5zZXJ0aW9ucygrKSwgNSBkZWxl dGlvbnMoLSkKPj4gCj4+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X2Ry di5oIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkxNV9kcnYuaAo+PiBpbmRleCAxMjg3MDA3M2Q1 OGYuLjUxYTg4ZTcwYTZmNyAxMDA2NDQKPj4gLS0tIGEvZHJpdmVycy9ncHUvZHJtL2k5MTUvaTkx NV9kcnYuaAo+PiArKysgYi9kcml2ZXJzL2dwdS9kcm0vaTkxNS9pOTE1X2Rydi5oCj4+IEBAIC0z NDAyLDEzICszNDAyLDEzIEBAIGludCBpbnRlbF9mcmVxX29wY29kZShzdHJ1Y3QgZHJtX2k5MTVf cHJpdmF0ZSAqZGV2X3ByaXYsIGludCB2YWwpOwo+PiAgI2RlZmluZSBJOTE1X1JFQUQ2NChyZWcp CWRldl9wcml2LT51bmNvcmUuZnVuY3MubW1pb19yZWFkcShkZXZfcHJpdiwgKHJlZyksIHRydWUp Cj4+ICAKPj4gICNkZWZpbmUgSTkxNV9SRUFENjRfMngzMihsb3dlcl9yZWcsIHVwcGVyX3JlZykg KHsJCQlcCj4+IC0JdTMyIHVwcGVyLCBsb3dlciwgdG1wOwkJCQkJCVwKPj4gLQl0bXAgPSBJOTE1 X1JFQUQodXBwZXJfcmVnKTsJCQkJCVwKPj4gKwl1MzIgdXBwZXIsIGxvd2VyLCBvbGRfdXBwZXIs IGxvb3AgPSAwOwkJCQlcCj4+ICsJdXBwZXIgPSBJOTE1X1JFQUQodXBwZXJfcmVnKTsJCQkJCVwK Pj4gIAlkbyB7CQkJCQkJCQlcCj4+IC0JCXVwcGVyID0gdG1wOwkJCQkJCVwKPj4gKwkJb2xkX3Vw cGVyID0gdXBwZXI7CQkJCQlcCj4+ICAJCWxvd2VyID0gSTkxNV9SRUFEKGxvd2VyX3JlZyk7CQkJ CVwKPj4gLQkJdG1wID0gSTkxNV9SRUFEKHVwcGVyX3JlZyk7CQkJCVwKPj4gLQl9IHdoaWxlICh1 cHBlciAhPSB0bXApOwkJCQkJCVwKPj4gKwkJdXBwZXIgPSBJOTE1X1JFQUQodXBwZXJfcmVnKTsJ CQkJXAo+PiArCX0gd2hpbGUgKHVwcGVyICE9IG9sZF91cHBlciAmJiBsb29wKysgPCAyKTsJCQlc Cj4+ICAJKHU2NCl1cHBlciA8PCAzMiB8IGxvd2VyOyB9KQo+PiAgCj4+ICAjZGVmaW5lIFBPU1RJ TkdfUkVBRChyZWcpCSh2b2lkKUk5MTVfUkVBRF9OT1RSQUNFKHJlZykKPj4gLS0gCj4+IDIuNS4x Cj4+IAo+Cj4gLS0gCj4gRGFuaWVsIFZldHRlcgo+IFNvZnR3YXJlIEVuZ2luZWVyLCBJbnRlbCBD b3Jwb3JhdGlvbgo+IGh0dHA6Ly9ibG9nLmZmd2xsLmNoCgotLSAKSmFuaSBOaWt1bGEsIEludGVs IE9wZW4gU291cmNlIFRlY2hub2xvZ3kgQ2VudGVyCl9fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fX19fCkludGVsLWdmeCBtYWlsaW5nIGxpc3QKSW50ZWwtZ2Z4QGxp c3RzLmZyZWVkZXNrdG9wLm9yZwpodHRwOi8vbGlzdHMuZnJlZWRlc2t0b3Aub3JnL21haWxtYW4v bGlzdGluZm8vaW50ZWwtZ2Z4Cg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga03.intel.com ([134.134.136.65]:25074 "EHLO mga03.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751323AbbIIIKY convert rfc822-to-8bit (ORCPT ); Wed, 9 Sep 2015 04:10:24 -0400 From: Jani Nikula To: Daniel Vetter , Chris Wilson Cc: intel-gfx@lists.freedesktop.org, =?utf-8?Q?Micha=C5=82?= Winiarski , Daniel Vetter , stable@vger.kernel.org Subject: Re: [PATCH v2] drm/i915: Limit the number of loops for reading a split 64bit register In-Reply-To: <20150908190059.GG2767@phenom.ffwll.local> References: <8737ypnl4f.fsf@intel.com> <1441718233-29112-1-git-send-email-chris@chris-wilson.co.uk> <20150908190059.GG2767@phenom.ffwll.local> Date: Wed, 09 Sep 2015 11:13:10 +0300 Message-ID: <87wpw0m2nd.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: stable-owner@vger.kernel.org List-ID: On Tue, 08 Sep 2015, Daniel Vetter wrote: > On Tue, Sep 08, 2015 at 02:17:13PM +0100, Chris Wilson wrote: >> In I915_READ64_2x32 we attempt to read a 64bit register using 2 32bit >> reads. Due to the nature of the registers we try to read in this manner, >> they may increment between the two instruction (e.g. a timestamp >> counter). To keep the result accurate, we repeat the read if we detect >> an overflow (i.e. the upper value varies). However, some harware is just >> plain flaky and may endless loop as the the upper 32bits are not stable. >> Just give up after a couple of tries and report whatever we read last. >> >> v2: Use the most recent values when erring out on an unstable register. >> >> Reported-by: russianneuromancer@ya.ru >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=91906 >> Signed-off-by: Chris Wilson >> Cc: MichaƂ Winiarski >> Cc: Daniel Vetter >> Cc: Jani Nikula >> Cc: stable@vger.kernel.org > > Still Reviewed-by: Daniel Vetter Pushed to drm-intel-next-fixes, thanks for the patch and review. BR, Jani. > >> --- >> drivers/gpu/drm/i915/i915_drv.h | 10 +++++----- >> 1 file changed, 5 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 12870073d58f..51a88e70a6f7 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -3402,13 +3402,13 @@ int intel_freq_opcode(struct drm_i915_private *dev_priv, int val); >> #define I915_READ64(reg) dev_priv->uncore.funcs.mmio_readq(dev_priv, (reg), true) >> >> #define I915_READ64_2x32(lower_reg, upper_reg) ({ \ >> - u32 upper, lower, tmp; \ >> - tmp = I915_READ(upper_reg); \ >> + u32 upper, lower, old_upper, loop = 0; \ >> + upper = I915_READ(upper_reg); \ >> do { \ >> - upper = tmp; \ >> + old_upper = upper; \ >> lower = I915_READ(lower_reg); \ >> - tmp = I915_READ(upper_reg); \ >> - } while (upper != tmp); \ >> + upper = I915_READ(upper_reg); \ >> + } while (upper != old_upper && loop++ < 2); \ >> (u64)upper << 32 | lower; }) >> >> #define POSTING_READ(reg) (void)I915_READ_NOTRACE(reg) >> -- >> 2.5.1 >> > > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- Jani Nikula, Intel Open Source Technology Center