From mboxrd@z Thu Jan 1 00:00:00 1970 From: Jani Nikula Subject: Re: [PATCH] drm/i915: use udelay for very short delays Date: Thu, 15 Dec 2016 12:20:01 +0200 Message-ID: <87k2b1pk8e.fsf@intel.com> References: <1481776147-23041-1-git-send-email-hofrat@osadl.org> <87wpf1pnj2.fsf@intel.com> <20161215092830.GA25458@osadl.at> <87mvfxpli5.fsf@intel.com> <20161215101036.GW31595@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Return-path: In-Reply-To: <20161215101036.GW31595@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Ville =?utf-8?B?U3lyasOkbMOk?= Cc: intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Nicholas Mc Guire , Nicholas Mc Guire , Daniel Vetter List-Id: dri-devel@lists.freedesktop.org T24gVGh1LCAxNSBEZWMgMjAxNiwgVmlsbGUgU3lyasOkbMOkIDx2aWxsZS5zeXJqYWxhQGxpbnV4 LmludGVsLmNvbT4gd3JvdGU6Cj4gT24gVGh1LCBEZWMgMTUsIDIwMTYgYXQgMTE6NTI6MzRBTSAr MDIwMCwgSmFuaSBOaWt1bGEgd3JvdGU6Cj4+IE9uIFRodSwgMTUgRGVjIDIwMTYsIE5pY2hvbGFz IE1jIEd1aXJlIDxkZXIuaGVyckBob2ZyLmF0PiB3cm90ZToKPj4gPiBPbiBUaHUsIERlYyAxNSwg MjAxNiBhdCAxMTowODo0OUFNICswMjAwLCBKYW5pIE5pa3VsYSB3cm90ZToKPj4gPj4gT24gVGh1 LCAxNSBEZWMgMjAxNiwgTmljaG9sYXMgTWMgR3VpcmUgPGhvZnJhdEBvc2FkbC5vcmc+IHdyb3Rl Ogo+PiA+PiA+IEV2ZW4gb24gZmFzdCBzeXN0ZW1zIGEgMiBtaWNyb3NlY29uZCBkZWxheSBpcyBt b3N0IGxpa2VseSBtb3JlIGVmZmljaWVudAo+PiA+PiA+IGFzIGEgYnVzeS13YWl0IGxvb3AuIFRo ZSBvdmVyaGVhZCBvZiBhIGhydGltZXIgZG9lcyBub3Qgc2VlbSB3YXJyYW50ZWQgLQo+PiA+PiA+ IGNoYW5nZSB0aGlzIHRvIGEgdWRlbGF5KDIpLgo+PiA+PiAKPj4gPj4gU2ltaWxhciBjb25jZXJu cyBhcyBpbiBbMV0uIFdlIGRvbid0IG5lZWQgdGhlIGFjY3VyYWN5IG9mIHVkZWxheSgpIGhlcmUs Cj4+ID4+IHNvIHRoaXMgYm9pbHMgZG93biB0byB3aGljaCBpcyB0aGUgYmV0dGVyIHVzZSBvZiBD UFUuIFdlIGNvdWxkIHByb2JhYmx5Cj4+ID4+IHJlbGF4IHRoZSBtYXggZGVsYXkgbW9yZSBpZiB0 aGF0IHdhcyBoZWxwZnVsLiBCdXQgSSdtIG5vdCBpbW1lZGlhdGVseQo+PiA+PiBzb2xkIG9uICJp cyBtb3N0IGxpa2VseSBtb3JlIGVmZmljaWVudCIgd2hpY2ggc291bmRzIGxpa2UgYSBndXQgZmVl bGluZy4KPj4gPj4gCj4+ID4+IEknbSBzb3JyeSBpdCdzIG5vdCBjbGVhciBpbiBteSBvdGhlciBy ZXBseSB0aGF0IEkgZG8gYXBwcmVjaWF0ZQo+PiA+PiBhZGRyZXNzaW5nIGluY29ycmVjdC9zaWxs eSB1c2Ugb2YgdXNsZWVwX3JhbmdlKCk7IEknbSBqdXN0IG5vdCAoeWV0KQo+PiA+PiBjb252aW5j ZWQgdWRlbGF5KCkgaXMgdGhlIGFuc3dlci4KPj4gPgo+PiA+IGlmIHRoZSBkZWxheSBpcyBub3Qg Y3JpdGljYWwgYW5kIGFsbCB0aGF0IGlzIG5lZWRlZCAKPj4gPiBpcyBhbiBhc3N1cmFuY2UgdGhh dCBpdCBpcyBncmVhdGVyIHRoYW4gWCB1cyB0aGVuIAo+PiA+IHVzbGVlcF9yYW5nZSBpcyBmaW5l IHdpdGggYSByZWxheGVkIGxpbWl0LiAKPj4gPiBTbyBmcm9tIHdoYXQgeW91IHdyb3RlIG15IHBh dGNoIHByb3Bvc2FsIGlzIHdyb25nIC0gdGhlCj4+ID4gdWRlbGF5KCkgaXMgbm90IHRoZSB3YXkg dG8gZ290Lgo+PiA+IE15IGludGVudCBpcyB0byBnZXQgcmlkIG9mIHZlcnkgc21hbGwgdXNsZWVw X3JhbmdlKCkgY2FzZXMKPj4gPiBzbyBpZiB1c2xlZXBfcmFuZ2UoMjAsNTApIGNhdXNlcyBubyBp c3N1ZXMgd2l0aCB0aGlzIGRyaXZlcgo+PiA+IGFuZCBkb2VzIG5vdCBpbmR1Y2UgYW55IHBlcmZv cm1hbmNlIHBlbmFsdHkgdGhlbiB0aGF0IHdvdWxkIAo+PiA+IGJlIHRoZSB3YXkgdG8gZ28gSSB0 aGluay4KPj4gCj4+IE9rYXksIHNvIEkgbG9va2VkIGF0IHRoZSBjb2RlLCBhbmQgSSBsb29rZWQg YXQgb3VyIHNwZWMsIGFuZCBJIGxvb2tlZCBhdAo+PiB0aGUgTUlQSSBELVBIWSBzcGVjLCBhbmQg SSBjcmllZCBhIGxpdHRsZS4KPj4gCj4+IExvbmcgc3Rvcnkgc2hvcnQsIEkgdGhpbmsgdXNsZWVw X3JhbmdlKDEwLCA1MCkgd2lsbCBiZSBmaW5lLgo+Cj4gTm90ZSB0aGF0IEkgcmVhbGx5IHdhbnQg dG8gc2VlIGEgY29tbWVudCBuZXh0IHRvIGV2ZXJ5IGRlbGF5IGxpa2UgdGhpcwo+IGRvY3VtZW50 aW5nIHRoZSBhY3R1YWwgaGFyZHdhcmUgcmVxdWlyZW1lbnQsIGlmIHRoZSBkZWxheSB1c2VkIGJ5 IHRoZQo+IGNvZGUgZG9lc24ndCAxMDAlIG1hdGNoIGl0LgoKT3VyIHNwZWMgc2F5cywgIldhaXQg Zm9yIDJ1cyBmb3IgVUxQUyB0byBjb21wbGV0ZSIuIFRoYXQncyBhIHNpbXBsaXN0aWMKdmlldyB3 cnQgRC1QSFksIGFuZCBvdXIgY29kZSBkb2Vzbid0IGV2ZW4gbWF0Y2ggdGhlIHNwZWMuIEhlbmNl IHRoZQp0ZWFycy4gV2FudCB0byBwcm9wb3NlIGEgd29yZGluZyBmb3IgdGhlIGNvbW1lbnQgc28g d2UgY2FuIGFwcGx5IHRoaXMKY2hhbmdlLCB3aXRob3V0IGdvaW5nIGZvciBhIGZ1bGwgcmV3cml0 ZSBvZiB0aGUgc2VxdWVuY2U/CgoKQlIsCkphbmkuCgotLSAKSmFuaSBOaWt1bGEsIEludGVsIE9w ZW4gU291cmNlIFRlY2hub2xvZ3kgQ2VudGVyCl9fX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fCkludGVsLWdmeCBtYWlsaW5nIGxpc3QKSW50ZWwtZ2Z4QGxpc3Rz LmZyZWVkZXNrdG9wLm9yZwpodHRwczovL2xpc3RzLmZyZWVkZXNrdG9wLm9yZy9tYWlsbWFuL2xp c3RpbmZvL2ludGVsLWdmeAo= From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1757809AbcLOKU6 convert rfc822-to-8bit (ORCPT ); Thu, 15 Dec 2016 05:20:58 -0500 Received: from mga11.intel.com ([192.55.52.93]:11627 "EHLO mga11.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751633AbcLOKU4 (ORCPT ); Thu, 15 Dec 2016 05:20:56 -0500 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.33,351,1477983600"; d="scan'208";a="798264932" From: Jani Nikula To: Ville =?utf-8?B?U3lyasOkbMOk?= Cc: Nicholas Mc Guire , intel-gfx@lists.freedesktop.org, linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Nicholas Mc Guire , Daniel Vetter Subject: Re: [PATCH] drm/i915: use udelay for very short delays In-Reply-To: <20161215101036.GW31595@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo References: <1481776147-23041-1-git-send-email-hofrat@osadl.org> <87wpf1pnj2.fsf@intel.com> <20161215092830.GA25458@osadl.at> <87mvfxpli5.fsf@intel.com> <20161215101036.GW31595@intel.com> Date: Thu, 15 Dec 2016 12:20:01 +0200 Message-ID: <87k2b1pk8e.fsf@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 15 Dec 2016, Ville Syrjälä wrote: > On Thu, Dec 15, 2016 at 11:52:34AM +0200, Jani Nikula wrote: >> On Thu, 15 Dec 2016, Nicholas Mc Guire wrote: >> > On Thu, Dec 15, 2016 at 11:08:49AM +0200, Jani Nikula wrote: >> >> On Thu, 15 Dec 2016, Nicholas Mc Guire wrote: >> >> > Even on fast systems a 2 microsecond delay is most likely more efficient >> >> > as a busy-wait loop. The overhead of a hrtimer does not seem warranted - >> >> > change this to a udelay(2). >> >> >> >> Similar concerns as in [1]. We don't need the accuracy of udelay() here, >> >> so this boils down to which is the better use of CPU. We could probably >> >> relax the max delay more if that was helpful. But I'm not immediately >> >> sold on "is most likely more efficient" which sounds like a gut feeling. >> >> >> >> I'm sorry it's not clear in my other reply that I do appreciate >> >> addressing incorrect/silly use of usleep_range(); I'm just not (yet) >> >> convinced udelay() is the answer. >> > >> > if the delay is not critical and all that is needed >> > is an assurance that it is greater than X us then >> > usleep_range is fine with a relaxed limit. >> > So from what you wrote my patch proposal is wrong - the >> > udelay() is not the way to got. >> > My intent is to get rid of very small usleep_range() cases >> > so if usleep_range(20,50) causes no issues with this driver >> > and does not induce any performance penalty then that would >> > be the way to go I think. >> >> Okay, so I looked at the code, and I looked at our spec, and I looked at >> the MIPI D-PHY spec, and I cried a little. >> >> Long story short, I think usleep_range(10, 50) will be fine. > > Note that I really want to see a comment next to every delay like this > documenting the actual hardware requirement, if the delay used by the > code doesn't 100% match it. Our spec says, "Wait for 2us for ULPS to complete". That's a simplistic view wrt D-PHY, and our code doesn't even match the spec. Hence the tears. Want to propose a wording for the comment so we can apply this change, without going for a full rewrite of the sequence? BR, Jani. -- Jani Nikula, Intel Open Source Technology Center