From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail.toke.dk ([52.28.52.200]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1fwp0n-0004FM-FX for ath10k@lists.infradead.org; Mon, 03 Sep 2018 13:35:27 +0000 From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= Subject: Re: [PATCH v2 2/2] ath10k: Set sk_pacing_shift to 6 for 11AC WiFi chips In-Reply-To: <1535975240.3437.61.camel@sipsolutions.net> References: <1533724802-30944-1-git-send-email-wgong@codeaurora.org> <1533724802-30944-3-git-send-email-wgong@codeaurora.org> <87sh3pdtpg.fsf@toke.dk> <87mutue4y8.fsf@toke.dk> <1535967508.3437.31.camel@sipsolutions.net> <87in3m25uu.fsf@toke.dk> <1535975240.3437.61.camel@sipsolutions.net> Date: Mon, 03 Sep 2018 15:35:11 +0200 Message-ID: <878t4i1z74.fsf@toke.dk> MIME-Version: 1.0 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: base64 Sender: "ath10k" Errors-To: ath10k-bounces+kvalo=adurom.com@lists.infradead.org To: Johannes Berg , Grant Grundler , wgong@qti.qualcomm.com Cc: linux-wireless@vger.kernel.org, ath10k@lists.infradead.org, wgong@codeaurora.org Sm9oYW5uZXMgQmVyZyA8am9oYW5uZXNAc2lwc29sdXRpb25zLm5ldD4gd3JpdGVzOgoKPiBPbiBN b24sIDIwMTgtMDktMDMgYXQgMTM6MTEgKzAyMDAsIFRva2UgSMO4aWxhbmQtSsO4cmdlbnNlbiB3 cm90ZToKPgo+PiA+IDYgdnMuIDgsIEkgdGhpbms/IEJ1dCBJIGRpZG4ndCBmb2xsb3cgdGhlIGZ1 bGwgZGlzY3Vzc2lvbi4KPgo+IEVyciwgSSBqdXN0IHJlYWxpemVkIHRoYXQgSSB3YXMgY29tcGxl dGVseSB3cm9uZyAtIHRoZSBkZWZhdWx0LCBvZgo+IGNvdXJzZSwgaXMgMTAuIFNvIHNtYWxsZXIg dmFsdWVzIG1lYW4gbW9yZSBidWZmZXJpbmcuCj4KPiBNb3N0IG9mIG15IGFyZ3VtZW50YXRpb24g d2FzIHRodXMgdXR0ZXIgZ2FyYmFnZSA6LSkKCldlbGwsIEkgZ290IHRoZSBnaXN0LCBldmVuIGlm IHRoZSBzaWduIGJpdCB3YXMgd3JvbmcgOykKCj4+ID4gSSBhbHNvIHRoaW5rIHRoYXQgd2Ugc2hv dWxkbid0IG5lY2Vzc2FyaWx5IHJlc3RyaWN0IHRoaXMgdG8gImZvciB0aGUKPj4gPiBhdGgxMGsi LiBJcyB0aGVyZSBhbnkgcmVhc29uIHRvIHRoaW5rIHRoYXQgdGhpcyB3b3VsZCBiZSBkaWZmZXJl bnQgZm9yCj4+ID4gZGlmZmVyZW50IGNoaXBzPwo+PiAKPj4gTm8sIEkgYWxzbyB0aGluayBpdCBz aG91bGQgYmUgcG9zc2libGUgdG8gc2VsZWN0IGEgdmFsdWUgdGhhdCB3aWxsIHdvcmsKPj4gZm9y IGFsbCBkcml2ZXJzLiBUaGVyZSdzIGEgc3Ryb25nIGRpbWluaXNoaW5nIHJldHVybnMgZWZmZWN0 IGhlcmUgYWZ0ZXIKPj4gYSBjZXJ0YWluIHBvaW50LCBhbmQgSSBiZWxpZXZlIHdlIHNob3VsZCBz dHJvbmdseSBwdXNoIGJhY2sgYWdhaW5zdCBqdXN0Cj4+IGFkZGluZyBtb3JlIGJ1ZmZlcmluZyB0 byBjaGFzZSAoc2luZ2xlLWZsb3cpIHRocm91Z2hwdXQgbnVtYmVyczsgYW5kIEknbQo+PiBub3Qg Y29udmluY2VkIHRoYXQgaGF2aW5nIGRpZmZlcmVudCB2YWx1ZXMgZm9yIGRpZmZlcmVudCBkcml2 ZXJzIGlzCj4+IHdvcnRoIHRoZSBjb21wbGV4aXR5Lgo+Cj4gSSB0aGluayBJIGNhbiBzZWUgc29t ZSBwb2ludCBpbiBpdCBpZiB0aGUgZHJpdmVyIHJlcXVpcmVzIHNvbWUKPiBidWZmZXJpbmcgZm9y IHNvbWUgc3BlY2lmaWMgcmVhc29uPyBCdXQgeW91J2QgaGF2ZSB0byBiZSBhYmxlIHRvIHN0YXRl Cj4gdGhhdCByZWFzb24sIEkgZ3Vlc3MsIEkgY291bGQgaW1hZ2luZSBoYXZpbmcgYSBmaXJtd2Fy ZSBsaW1pdGF0aW9uIHRvCj4gbmVlZCB0byBidWlsZCB0d28gYWdncmVnYXRlcywgb3Igc29tZXRo aW5nIGFyb3VuZCBNVS1NSU1PLCBldGMuCgpSaWdodCwgSSdtIG5vdCBydWxpbmcgb3V0IHRoYXQg dGhlcmUgY2FuIGJlIGxlZ2l0aW1hdGUgcmVhc29ucyB0byBhZGQKZXh0cmEgYnVmZmVyaW5nOyBi dXQgYSBsb3Qgb2YgdGltZXMgaXQncyBqdXN0IHVzZWQgdG8gcGFwZXIgb3ZlciBvdGhlcgppc3N1 ZXMsIHNvIGEgZ29vZCBleHBsYW5hdGlvbiBpcyBkZWZpbml0ZWx5IG5lZWRlZC4uLgoKPj4gQXMg ZmFyIGFzIHRoZSBhY3R1YWwgdmFsdWUsIEkgKnRoaW5rKiBpdCBtYXkgYmUgdGhhdCB0aGUgZGVm YXVsdCBzaGlmdAo+PiBzaG91bGQgYmUgNyAoOCBtcykgcmF0aGVyIHRoYW4gOCAoNCBtcykgYXMg aXQgY3VycmVudGx5IGlzLiBHb2luZyBiYWNrCj4+IGFuZCBsb29raW5nIGF0IG15IGRhdGEgZnJv bSB3aGVuIEkgc3VibWl0dGVkIHRoZSBvcmlnaW5hbCBwYXRjaCwgaXQKPj4gbG9va3MgbGlrZSB0 aGUgcG9pbnQgb2YgZGltaW5pc2hpbmcgcmV0dXJucyBpcyBzb21ld2hlcmUgYmV0d2VlbiB0aG9z ZQo+PiB0d28gd2l0aCBhdGg5ayAod2hlcmUgSSBkaWQgbW9zdCBvZiBteSB0ZXN0aW5nKSwgYW5k IGl0IHNlZW1zIHJlYXNvbmFibGUKPj4gdGhhdCBpdCBjb3VsZCBiZSBzbGlnaHRseSBoaWdoZXIg KGkuZS4sIG1vcmUgYnVmZmVyaW5nKSBmb3IgYXRoMTBrLgo+Cj4gR3JhbnQncyBkYXRhIHNob3dz IGEgc2lnbmlmaWNhbnQgZGlmZmVyZW5jZSBiZXR3ZWVuIDYgYW5kIDcgZm9yIGJvdGgKPiBsYXRl bmN5IGFuZCB0aHJvdWdocHV0Ogo+Cj4gICogbWVkaWFuIHRwdAo+ICAgIC0gfjI0MSB2cyB+MjAx IChib3RoIDEgYW5kIDUgc3RyZWFtcykKPiAgKiBtZWRpYW4gbGF0ZW5jeQo+ICAgIC0gNy41IHZz IDYgKDEgc3RyZWFtKQo+ICAgIC0gMTcuMyB2cy4gMTYuNiAoNSBzdHJlYW1zKQo+Cj4gQSAyMCUg dGhyb3VnaHB1dCBpbXByb3ZlbWVudCBhdCA8PSAxLjVtcyBsYXRlbmN5IGNvc3Qgc2VlbXMgbGlr ZSBhCj4gcHJldHR5IHJlYXNvbmFibGUgdHJhZGUtb2ZmPwoKWWVhaCwgb24gaXQncyBmYWNlLiBX aGF0IEknbSBib3RoZXJlZCBhYm91dCBpcyB0aGF0IGl0IGlzIHRoZSBleGFjdApvcHBvc2l0ZSBy ZXN1bHRzIHRoYXQgSSBnb3QgZnJvbSBteSBhdGgxMGsgdGVzdHMgKHRoZXJlLCB0aHJvdWdocHV0 Cipkcm9wcGVkKiBhbmQgbGF0ZW5jeSBkb3VibGVkIHdoZW4gZ29pbmcgdG8gZnJvbSA0IHRvIDE2 IG1zIG9mCmJ1ZmZlcmluZykuIEFuZCwgd2VsbCwgR3JhbnQncyBkYXRhIGlzIGZyb20gYSBzaW5n bGUgdGVzdCBpbiBhIG5vaXN5CmVudmlyb25tZW50IHdoZXJlIHRoZSB0aW1lc2VyaWVzIGdyYXBo IHNob3dzIHRoYXQgdGhyb3VnaHB1dCBpcyBhbGwgb3Zlcgp0aGUgcGxhY2UgZm9yIHRoZSBkdXJh dGlvbiBvZiB0aGUgdGVzdDsgc28gaXQncyBoYXJkIHRvIGRyYXcgc29saWQKY29uY2x1c2lvbnMg ZnJvbSAoZm9yIGluc3RhbmNlLCBmb3IgdGhlIDUtc3RyZWFtIHRlc3QsIHRoZSBhdmVyYWdlCnRo cm91Z2hwdXQgZm9yIDYgaXMgMzMxIGFuZCAzNzkgTWJwcyBmb3IgdGhlIHR3byByZXBldGl0aW9u cywgYW5kIGZvciA3Cml0J3MgMzI2IGFuZCAzNzEgTWJwcykgLiBVbmZvcnR1bmF0ZWx5IEkgZG9u J3QgaGF2ZSB0aGUgc2FtZSBoYXJkd2FyZQp1c2VkIGluIHRoaXMgdGVzdCwgc28gSSBjYW4ndCBn byB2ZXJpZnkgaXQgbXlzZWxmOyBzbyB0aGUgb25seSB0aGluZyBJCmNhbiBkbyBpcyBncnVtYmxl IGFib3V0IGl0IGhlcmUuLi4gOikKCi1Ub2tlCgpfX19fX19fX19fX19fX19fX19fX19fX19fX19f X19fX19fX19fX19fX19fX19fXwphdGgxMGsgbWFpbGluZyBsaXN0CmF0aDEwa0BsaXN0cy5pbmZy YWRlYWQub3JnCmh0dHA6Ly9saXN0cy5pbmZyYWRlYWQub3JnL21haWxtYW4vbGlzdGluZm8vYXRo MTBrCg== From mboxrd@z Thu Jan 1 00:00:00 1970 Return-path: Received: from mail.toke.dk ([52.28.52.200]:60825 "EHLO mail.toke.dk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725794AbeICRzZ (ORCPT ); Mon, 3 Sep 2018 13:55:25 -0400 From: Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= To: Johannes Berg , Grant Grundler , wgong@qti.qualcomm.com Cc: wgong@codeaurora.org, ath10k@lists.infradead.org, linux-wireless@vger.kernel.org Subject: Re: [PATCH v2 2/2] ath10k: Set sk_pacing_shift to 6 for 11AC WiFi chips In-Reply-To: <1535975240.3437.61.camel@sipsolutions.net> References: <1533724802-30944-1-git-send-email-wgong@codeaurora.org> <1533724802-30944-3-git-send-email-wgong@codeaurora.org> <87sh3pdtpg.fsf@toke.dk> <87mutue4y8.fsf@toke.dk> <1535967508.3437.31.camel@sipsolutions.net> <87in3m25uu.fsf@toke.dk> <1535975240.3437.61.camel@sipsolutions.net> Date: Mon, 03 Sep 2018 15:35:11 +0200 Message-ID: <878t4i1z74.fsf@toke.dk> (sfid-20180903_153519_172012_BA3B8E6B) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Sender: linux-wireless-owner@vger.kernel.org List-ID: Johannes Berg writes: > On Mon, 2018-09-03 at 13:11 +0200, Toke H=C3=B8iland-J=C3=B8rgensen wrote: > >> > 6 vs. 8, I think? But I didn't follow the full discussion. > > Err, I just realized that I was completely wrong - the default, of > course, is 10. So smaller values mean more buffering. > > Most of my argumentation was thus utter garbage :-) Well, I got the gist, even if the sign bit was wrong ;) >> > I also think that we shouldn't necessarily restrict this to "for the >> > ath10k". Is there any reason to think that this would be different for >> > different chips? >>=20 >> No, I also think it should be possible to select a value that will work >> for all drivers. There's a strong diminishing returns effect here after >> a certain point, and I believe we should strongly push back against just >> adding more buffering to chase (single-flow) throughput numbers; and I'm >> not convinced that having different values for different drivers is >> worth the complexity. > > I think I can see some point in it if the driver requires some > buffering for some specific reason? But you'd have to be able to state > that reason, I guess, I could imagine having a firmware limitation to > need to build two aggregates, or something around MU-MIMO, etc. Right, I'm not ruling out that there can be legitimate reasons to add extra buffering; but a lot of times it's just used to paper over other issues, so a good explanation is definitely needed... >> As far as the actual value, I *think* it may be that the default shift >> should be 7 (8 ms) rather than 8 (4 ms) as it currently is. Going back >> and looking at my data from when I submitted the original patch, it >> looks like the point of diminishing returns is somewhere between those >> two with ath9k (where I did most of my testing), and it seems reasonable >> that it could be slightly higher (i.e., more buffering) for ath10k. > > Grant's data shows a significant difference between 6 and 7 for both > latency and throughput: > > * median tpt > - ~241 vs ~201 (both 1 and 5 streams) > * median latency > - 7.5 vs 6 (1 stream) > - 17.3 vs. 16.6 (5 streams) > > A 20% throughput improvement at <=3D 1.5ms latency cost seems like a > pretty reasonable trade-off? Yeah, on it's face. What I'm bothered about is that it is the exact opposite results that I got from my ath10k tests (there, throughput *dropped* and latency doubled when going to from 4 to 16 ms of buffering). And, well, Grant's data is from a single test in a noisy environment where the timeseries graph shows that throughput is all over the place for the duration of the test; so it's hard to draw solid conclusions from (for instance, for the 5-stream test, the average throughput for 6 is 331 and 379 Mbps for the two repetitions, and for 7 it's 326 and 371 Mbps) . Unfortunately I don't have the same hardware used in this test, so I can't go verify it myself; so the only thing I can do is grumble about it here... :) -Toke