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 11:33:11 +0200 Message-ID: <87r359pmeg.fsf@intel.com> References: <1481774609-20998-1-git-send-email-hofrat@osadl.org> <8737hpr32a.fsf@intel.com> <20161215085651.GA25256@osadl.at> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20161215085651.GA25256@osadl.at> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Nicholas Mc Guire Cc: David Airlie , intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Nicholas Mc Guire , Daniel Vetter List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCAxNSBEZWMgMjAxNiwgTmljaG9sYXMgTWMgR3VpcmUgPGRlci5oZXJyQGhvZnIuYXQ+ IHdyb3RlOgo+IE9uIFRodSwgRGVjIDE1LCAyMDE2IGF0IDEwOjQ3OjU3QU0gKzAyMDAsIEphbmkg TmlrdWxhIHdyb3RlOgo+PiBPbiBUaHUsIDE1IERlYyAyMDE2LCBOaWNob2xhcyBNYyBHdWlyZSA8 aG9mcmF0QG9zYWRsLm9yZz4gd3JvdGU6Cj4+ID4gdXNsZWVwX3JhbmdlKCkgaXMgaW50ZW5kZWQg Zm9yIGRlbGF5cyBpbiB0aGUgMTB1cyB0byAxMG1zIHJhbmdlIHRoYXQgbmVlZAo+PiA+IGdvb2Qg cHJlY2lzaW9uLiBhIHVzZWxlZXBfcmFuZ2UoMSwgd2lsbCBlZmZlY3RpdmVseSBiZSBubyBtb3Jl IHRoYW4gYW4KPj4gPiBpbXByZWNpc2UgdWRlbGF5IHdpdGggc29tZSBhZGRlZCBjYWNoZSBkaXNy dXB0aW9uIGFzIGl0IHdpbGwgZmlyZSBtb3JlIG9yCj4+ID4gbGVzcyBpbW1lZGlhdGVseSAtIHVz ZSB1ZGVsYXkoKSBoZXJlLgo+PiA+Cj4+ID4gRml4ZXM6IGNvbW1pdCBiZTRmYzA0NmJlZDMgKCJk cm0vaTkxNTogYWRkIFZMViBEU0kgUExMIENhbGN1bGF0aW9ucyIpCj4+ID4gU2lnbmVkLW9mZi1i eTogTmljaG9sYXMgTWMgR3VpcmUgPGhvZnJhdEBvc2FkbC5vcmc+Cj4+ID4gLS0tCj4+ID4KPj4g PiBQcm9ibGVtIGxvY2F0ZWQgYnkgY29jY2luZWxsZQo+PiA+Cj4+ID4gVGhlIHJlcXVpcmVtZW50 IG9mIHdhaXRpbmcgYXQgbGVhc3QgMC41IHVzIGlzIGFzc3VyZWQgd2l0aCB0aGUgdWRlbGF5KDEp Cj4+ID4gaGVyZSB3aGljaCBzaG91bGQgYmUgbW9yZSBlZmZlY3RpdmUgdGhhbiBhIHVzbGVlcF9y YW5nZSgpIC0gd291bGQgCj4+ID4gbmRlbGF5KDUwMCkgbWFrZSBzZW5zZSBoZXJlID8KPj4gCj4+ IFRoaXMgaXMgaW4gdGhlIG1vZGVzZXQgcGF0aCwgaS5lLiBwcmV0dHkgc2xvdyBhbnl3YXkuIElu IHRoaXMgY2FzZSwgdGhlCj4+IHBvaW50IGlzIG5vdCB0byB0cnkgaGFyZCB0byBtaW5pbWl6ZSB0 aGUgd2FpdCwgdGhlIHBvaW50IGlzIHRvIGd1YXJhbnRlZQo+PiAiYXQgbGVhc3QgMC41IHVzIiBo YXMgcGFzc2VkLiBJZiB0aGUgQ1BVIGNhbiBkbyBzb21ldGhpbmcgZWxzZSwKPj4gaW5jbHVkaW5n IGRvemluZyBvZmYsIGluIHRoZSBtZWFuIHRpbWUsIGdyZWF0LiBJIHRoaW5rIHdlIHNob3VsZCBz dGljawo+PiB3aXRoIHVzbGVlcF9yYW5nZSgpLgo+Cj4gd2VsbCBpbiB0aGF0IGNhc2UgbWF5YmUg YW4gYWNjZXB0YWJsZSBzb2x1dGlvbiB3b3VsZCBiZSB0byBzZXQgaXQgdG8gCj4gc29tZSBzdWl0 YWJsZSByYW5nZSAxMCwyMCB1cyA/IG9yIGlmIG5vdCBjcml0aWNhbCBwcmVmZXJhYmx5IGV2ZW4g d2l0aCBhIGxhcmdlCj4gdXBwZXIgbGltaXQuCgpJJ2QgYmUgZmluZSB3aXRoIDEwLCA1MCBoZXJl LiBQbGVhc2UgZG8gc2VuZCB0aGUgcGF0Y2gsIENjOiBtZS4KCj4+IAo+PiBJIHRoaW5rIHRoZSBx dWVzdGlvbiBpcywgaG93IGRvIHdlIGV4cHJlc3MgdGhpcyBpbiBjb2RlPyBJTU8gdWRlbGF5KCkg aXMKPj4gbm90IHRoZSBhbnN3ZXIuCj4KPiBpZiB0aGUgZGVsYXkgbmVlZCB0byBiZSBrZXB0IHNo b3J0IHRoZW4gbm8gLSB0aGVuIGl0cyBub3QgdGhlIGFuc3dlcgo+IGJ1dCB1c2xlZXBfcmFuZ2Vz KDEsMikgSSB0aGluayBpcyBlZmZlY3RpdmVseSBqdXN0IGFuIGluZWZmaWNpZW50IHZlcnNpb24K PiBvZiB1ZGVsYXkoMSksIGJ5IHRoZSB0aW1lIHRoZSB0aW1lciBpcyBzZXR1cCBhbmQgdGhlIHRh c2sgZ2l2ZXMKPiB1cCB0aGUgY3B1IHRoZSB0aW1lciB3b3VsZCBmaXJlLgo+Cj4+IAo+PiBBbmQg d2h5IGRvZXNuJ3QgdXNsZWVwX3JhbmdlKCkga2VybmVsLWRvYyBtZW50aW9uIGFueXRoaW5nIGFi b3V0IHRoZQo+PiByYW5nZXM/Cj4+IAo+Cj4gaW50ZXJlc3RpbmcgLSB0aGF0IG1pZ2h0IGJlIHBh cnQgb2YgdGhlIHJlYXNvbiB0aGVyZSBhcmUgbWFueSBmaW5kaW5ncwo+IERvY3VtZW50YXRpb24v dGltZXJzL3RpbWVycy1ob3d0by50eHQgZG9lcwo+Cj4gICAgICAgICBTTEVFUElORyBGT1IgflVT RUNTIE9SIFNNQUxMIE1TRUNTICggMTB1cyAtIDIwbXMpOgo+ICAgICAgICAgICAgICAgICAqIFVz ZSB1c2xlZXBfcmFuZ2UKCkknZCBhcHByZWNpYXRlIHNob3J0IGFkZGl0aW9ucyB0byB0aGUga2Vy bmVsLWRvYyBkb2N1bWVudGF0aW9uIG9mIGVhY2gKZnVuY3Rpb24gdG8gZG9jdW1lbnQgdGhlIGFw cHJveGltYXRlIHJhbmdlIGl0J3MgYXBwcm9wcmlhdGUgZm9yLiBQZW9wbGUKd2lsbCBrbm93IHdo ZXJlIHRvIGxvb2sgaWYgdGhlaXIgdXNlIGRvZXNuJ3QgZmFsbCBpbiB0aGF0IHJhbmdlLgoKVGhh bmtzLApKYW5pLgoKLS0gCkphbmkgTmlrdWxhLCBJbnRlbCBPcGVuIFNvdXJjZSBUZWNobm9sb2d5 IENlbnRlcgpfX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fX19fXwpJ bnRlbC1nZnggbWFpbGluZyBsaXN0CkludGVsLWdmeEBsaXN0cy5mcmVlZGVza3RvcC5vcmcKaHR0 cHM6Ly9saXN0cy5mcmVlZGVza3RvcC5vcmcvbWFpbG1hbi9saXN0aW5mby9pbnRlbC1nZngK From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S935930AbcLOJdR (ORCPT ); Thu, 15 Dec 2016 04:33:17 -0500 Received: from mga05.intel.com ([192.55.52.43]:51381 "EHLO mga05.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S935618AbcLOJdO (ORCPT ); Thu, 15 Dec 2016 04:33:14 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,351,1477983600"; d="scan'208";a="798250898" From: Jani Nikula To: Nicholas Mc Guire Cc: Nicholas Mc Guire , Daniel Vetter , ymohanma , David Airlie , intel-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] drm/i915: use udelay for very small delays In-Reply-To: <20161215085651.GA25256@osadl.at> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <1481774609-20998-1-git-send-email-hofrat@osadl.org> <8737hpr32a.fsf@intel.com> <20161215085651.GA25256@osadl.at> Date: Thu, 15 Dec 2016 11:33:11 +0200 Message-ID: <87r359pmeg.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: > On Thu, Dec 15, 2016 at 10:47:57AM +0200, Jani Nikula wrote: >> 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(). > > well in that case maybe an acceptable solution would be to set it to > some suitable range 10,20 us ? or if not critical preferably even with a large > upper limit. I'd be fine with 10, 50 here. Please do send the patch, Cc: me. >> >> I think the question is, how do we express this in code? IMO udelay() is >> not the answer. > > if the delay need to be kept short then no - then its not the answer > but usleep_ranges(1,2) I think is effectively just an inefficient version > of udelay(1), by the time the timer is setup and the task gives > up the cpu the timer would fire. > >> >> And why doesn't usleep_range() kernel-doc mention anything about the >> ranges? >> > > interesting - that might be part of the reason there are many findings > Documentation/timers/timers-howto.txt does > > SLEEPING FOR ~USECS OR SMALL MSECS ( 10us - 20ms): > * Use usleep_range I'd appreciate short additions to the kernel-doc documentation of each function to document the approximate range it's appropriate for. People will know where to look if their use doesn't fall in that range. Thanks, Jani. -- Jani Nikula, Intel Open Source Technology Center