From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/i915: use udelay for very small delays Date: Thu, 15 Dec 2016 10:47:57 +0200 Message-ID: <8737hpr32a.fsf@intel.com> References: <1481774609-20998-1-git-send-email-hofrat@osadl.org> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <1481774609-20998-1-git-send-email-hofrat@osadl.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: David Airlie , intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Nicholas Mc Guire List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCAxNSBEZWMgMjAxNiwgTmljaG9sYXMgTWMgR3VpcmUgPGhvZnJhdEBvc2FkbC5vcmc+ IHdyb3RlOgo+IHVzbGVlcF9yYW5nZSgpIGlzIGludGVuZGVkIGZvciBkZWxheXMgaW4gdGhlIDEw dXMgdG8gMTBtcyByYW5nZSB0aGF0IG5lZWQKPiBnb29kIHByZWNpc2lvbi4gYSB1c2VsZWVwX3Jh bmdlKDEsIHdpbGwgZWZmZWN0aXZlbHkgYmUgbm8gbW9yZSB0aGFuIGFuCj4gaW1wcmVjaXNlIHVk ZWxheSB3aXRoIHNvbWUgYWRkZWQgY2FjaGUgZGlzcnVwdGlvbiBhcyBpdCB3aWxsIGZpcmUgbW9y ZSBvcgo+IGxlc3MgaW1tZWRpYXRlbHkgLSB1c2UgdWRlbGF5KCkgaGVyZS4KPgo+IEZpeGVzOiBj b21taXQgYmU0ZmMwNDZiZWQzICgiZHJtL2k5MTU6IGFkZCBWTFYgRFNJIFBMTCBDYWxjdWxhdGlv bnMiKQo+IFNpZ25lZC1vZmYtYnk6IE5pY2hvbGFzIE1jIEd1aXJlIDxob2ZyYXRAb3NhZGwub3Jn Pgo+IC0tLQo+Cj4gUHJvYmxlbSBsb2NhdGVkIGJ5IGNvY2NpbmVsbGUKPgo+IFRoZSByZXF1aXJl bWVudCBvZiB3YWl0aW5nIGF0IGxlYXN0IDAuNSB1cyBpcyBhc3N1cmVkIHdpdGggdGhlIHVkZWxh eSgxKQo+IGhlcmUgd2hpY2ggc2hvdWxkIGJlIG1vcmUgZWZmZWN0aXZlIHRoYW4gYSB1c2xlZXBf cmFuZ2UoKSAtIHdvdWxkIAo+IG5kZWxheSg1MDApIG1ha2Ugc2Vuc2UgaGVyZSA/CgpUaGlzIGlz IGluIHRoZSBtb2Rlc2V0IHBhdGgsIGkuZS4gcHJldHR5IHNsb3cgYW55d2F5LiBJbiB0aGlzIGNh c2UsIHRoZQpwb2ludCBpcyBub3QgdG8gdHJ5IGhhcmQgdG8gbWluaW1pemUgdGhlIHdhaXQsIHRo ZSBwb2ludCBpcyB0byBndWFyYW50ZWUKImF0IGxlYXN0IDAuNSB1cyIgaGFzIHBhc3NlZC4gSWYg dGhlIENQVSBjYW4gZG8gc29tZXRoaW5nIGVsc2UsCmluY2x1ZGluZyBkb3ppbmcgb2ZmLCBpbiB0 aGUgbWVhbiB0aW1lLCBncmVhdC4gSSB0aGluayB3ZSBzaG91bGQgc3RpY2sKd2l0aCB1c2xlZXBf cmFuZ2UoKS4KCkkgdGhpbmsgdGhlIHF1ZXN0aW9uIGlzLCBob3cgZG8gd2UgZXhwcmVzcyB0aGlz IGluIGNvZGU/IElNTyB1ZGVsYXkoKSBpcwpub3QgdGhlIGFuc3dlci4KCkFuZCB3aHkgZG9lc24n dCB1c2xlZXBfcmFuZ2UoKSBrZXJuZWwtZG9jIG1lbnRpb24gYW55dGhpbmcgYWJvdXQgdGhlCnJh bmdlcz8KCgpCUiwKSmFuaS4KCgo+Cj4gUGF0Y2ggd2FzIGNvbXBpbGUgdGVzdGVkIHdpdGg6IHg4 Nl82NF9kZWZjb25maWcgKGltcGxpZXMgQ09ORklHX0RSTV9JOTE1KQo+Cj4gUGF0Y2ggaXMgYWdh aW5zdCA0LjkuMCAobG9jYWx2cnNpb24tbmV4dCBpcyBuZXh0LTIwMTYxMjE0KQo+Cj4gIGRyaXZl cnMvZ3B1L2RybS9pOTE1L2ludGVsX2RzaV9wbGwuYyB8IDIgKy0KPiAgMSBmaWxlIGNoYW5nZWQs IDEgaW5zZXJ0aW9uKCspLCAxIGRlbGV0aW9uKC0pCj4KPiBkaWZmIC0tZ2l0IGEvZHJpdmVycy9n cHUvZHJtL2k5MTUvaW50ZWxfZHNpX3BsbC5jIGIvZHJpdmVycy9ncHUvZHJtL2k5MTUvaW50ZWxf ZHNpX3BsbC5jCj4gaW5kZXggNTZlZmY2MC4uMGVjMDQwZSAxMDA2NDQKPiAtLS0gYS9kcml2ZXJz L2dwdS9kcm0vaTkxNS9pbnRlbF9kc2lfcGxsLmMKPiArKysgYi9kcml2ZXJzL2dwdS9kcm0vaTkx NS9pbnRlbF9kc2lfcGxsLmMKPiBAQCAtMTU3LDcgKzE1Nyw3IEBAIHN0YXRpYyB2b2lkIHZsdl9l bmFibGVfZHNpX3BsbChzdHJ1Y3QgaW50ZWxfZW5jb2RlciAqZW5jb2RlciwKPiAgCQkgICAgICBj b25maWctPmRzaV9wbGwuY3RybCAmIH5EU0lfUExMX1ZDT19FTik7Cj4gIAo+ICAJLyogd2FpdCBh dCBsZWFzdCAwLjUgdXMgYWZ0ZXIgdW5nYXRpbmcgYmVmb3JlIGVuYWJsaW5nIFZDTyAqLwo+IC0J dXNsZWVwX3JhbmdlKDEsIDEwKTsKPiArCXVkZWxheSgxKTsKPiAgCj4gIAl2bHZfY2NrX3dyaXRl KGRldl9wcml2LCBDQ0tfUkVHX0RTSV9QTExfQ09OVFJPTCwgY29uZmlnLT5kc2lfcGxsLmN0cmwp OwoKLS0gCkphbmkgTmlrdWxhLCBJbnRlbCBPcGVuIFNvdXJjZSBUZWNobm9sb2d5IENlbnRlcgpf X19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpJbnRlbC1nZngg bWFpbGluZyBsaXN0CkludGVsLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0cHM6Ly9saXN0 cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757460AbcLOIsr (ORCPT ); Thu, 15 Dec 2016 03:48:47 -0500 Received: from mga05.intel.com ([192.55.52.43]:26826 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751135AbcLOIsq (ORCPT ); Thu, 15 Dec 2016 03:48:46 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,351,1477983600"; d="scan'208";a="798239714" From: Jani Nikula To: Nicholas Mc Guire , Daniel Vetter Cc: ymohanma , David Airlie , intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Nicholas Mc Guire Subject: Re: [PATCH] drm/i915: use udelay for very small delays In-Reply-To: <1481774609-20998-1-git-send-email-hofrat@osadl.org> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <1481774609-20998-1-git-send-email-hofrat@osadl.org> Date: Thu, 15 Dec 2016 10:47:57 +0200 Message-ID: <8737hpr32a.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 15 Dec 2016, Nicholas Mc Guire wrote: > usleep_range() is intended for delays in the 10us to 10ms range that need > good precision. a useleep_range(1, will effectively be no more than an > imprecise udelay with some added cache disruption as it will fire more or > less immediately - use udelay() here. > > Fixes: commit be4fc046bed3 ("drm/i915: add VLV DSI PLL Calculations") > Signed-off-by: Nicholas Mc Guire > --- > > Problem located by coccinelle > > The requirement of waiting at least 0.5 us is assured with the udelay(1) > here which should be more effective than a usleep_range() - would > ndelay(500) make sense here ? This is in the modeset path, i.e. pretty slow anyway. In this case, the point is not to try hard to minimize the wait, the point is to guarantee "at least 0.5 us" has passed. If the CPU can do something else, including dozing off, in the mean time, great. I think we should stick with usleep_range(). I think the question is, how do we express this in code? IMO udelay() is not the answer. And why doesn't usleep_range() kernel-doc mention anything about the ranges? BR, Jani. > > Patch was compile tested with: x86_64_defconfig (implies CONFIG_DRM_I915) > > Patch is against 4.9.0 (localvrsion-next is next-20161214) > > drivers/gpu/drm/i915/intel_dsi_pll.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/intel_dsi_pll.c b/drivers/gpu/drm/i915/intel_dsi_pll.c > index 56eff60..0ec040e 100644 > --- a/drivers/gpu/drm/i915/intel_dsi_pll.c > +++ b/drivers/gpu/drm/i915/intel_dsi_pll.c > @@ -157,7 +157,7 @@ static void vlv_enable_dsi_pll(struct intel_encoder *encoder, > config->dsi_pll.ctrl & ~DSI_PLL_VCO_EN); > > /* wait at least 0.5 us after ungating before enabling VCO */ > - usleep_range(1, 10); > + udelay(1); > > vlv_cck_write(dev_priv, CCK_REG_DSI_PLL_CONTROL, config->dsi_pll.ctrl); -- Jani Nikula, Intel Open Source Technology Center