From owner-svn-src-all@FreeBSD.ORG Tue Apr 6 18:45:09 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id A4FBB1065674; Tue, 6 Apr 2010 18:45:09 +0000 (UTC) (envelope-from pyunyh@gmail.com) Received: from mail-bw0-f216.google.com (mail-bw0-f216.google.com [209.85.218.216]) by mx1.freebsd.org (Postfix) with ESMTP id C497C8FC16; Tue, 6 Apr 2010 18:45:08 +0000 (UTC) Received: by bwz8 with SMTP id 8so195158bwz.3 for ; Tue, 06 Apr 2010 11:45:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:received:received:received:from:date:to:cc :subject:message-id:reply-to:references:mime-version:content-type :content-disposition:in-reply-to:user-agent; bh=/akSMq50vORihjBykRF6VEluCTlLTzdgS/6EZKY9o7g=; b=nM3pEEsdtNCrossJqq1JTloWZTcgFBVm2UeMnlY4o3svHl3UUtWvILz8fym/izD4vD 5PayI1vNytmxeICjYnYeJPVDHVdkla7p+FkjEkECPZQIYc+HIUGO0cLHyKiEpB9MGuUE x5FtizHinuhScYTu55LGftUBkJQV3CKmSl8As= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=from:date:to:cc:subject:message-id:reply-to:references:mime-version :content-type:content-disposition:in-reply-to:user-agent; b=tUTrZXMEBTwzicdbdKwUAyS+ms1oGkfZJ7semeHprn7nQ5xbE48VekzR+iEsE7/0TR cBud5hwzvT5aUJ7x5YlBPj2E8WhaCKGuqYpnjr2HQIVKblGVuhEWeLeO+mICQxt617N3 Q7/FDgpNScMOydrDze79xyWKacyR0ZibuH9wA= Received: by 10.204.33.206 with SMTP id i14mr1894663bkd.52.1270579507240; Tue, 06 Apr 2010 11:45:07 -0700 (PDT) Received: from pyunyh@gmail.com ([174.35.1.224]) by mx.google.com with ESMTPS id 13sm6364060bwz.3.2010.04.06.11.45.02 (version=TLSv1/SSLv3 cipher=RC4-MD5); Tue, 06 Apr 2010 11:45:05 -0700 (PDT) Received: by pyunyh@gmail.com (sSMTP sendmail emulation); Tue, 6 Apr 2010 11:44:56 -0700 From: Pyun YongHyeon Date: Tue, 6 Apr 2010 11:44:56 -0700 To: Andre Albsmeier Message-ID: <20100406184456.GA1087@michelle.cdnetworks.com> References: <201003241721.o2OHL5K9063538@svn.freebsd.org> <20100405145937.GA78871@curry.mchp.siemens.de> <20100405180642.GD1225@michelle.cdnetworks.com> <20100406134626.GA1727@curry.mchp.siemens.de> <20100406180027.GA3724@curry.mchp.siemens.de> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="45Z9DzgjV8m4Oswq" Content-Disposition: inline In-Reply-To: <20100406180027.GA3724@curry.mchp.siemens.de> User-Agent: Mutt/1.4.2.3i Cc: "svn-src-stable@freebsd.org" , "svn-src-all@freebsd.org" , Pyun YongHyeon Subject: Re: svn commit: r205614 - stable/7/sys/dev/msk X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: pyunyh@gmail.com List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 06 Apr 2010 18:45:09 -0000 --45Z9DzgjV8m4Oswq Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Tue, Apr 06, 2010 at 08:00:27PM +0200, Andre Albsmeier wrote: > On Tue, 06-Apr-2010 at 15:46:26 +0200, Andre Albsmeier wrote: > > On Mon, 05-Apr-2010 at 20:06:42 +0200, Pyun YongHyeon wrote: [...] > > > As you know 1.18.2.38 removed taskqueue based interrupt handling so > > > it could be culprit of the issue. But that revision also removed > > > two register accesses in TX path so I'd like to know which one > > > caused the issue. > > > > I have now tried rev. 1.18.2.38 with this patch (no idea if > > this is right ;-)): > > > > --- if_msk.c.1.18.2.38 2010-04-06 15:09:19.000000000 +0200 > > +++ if_msk.c.1.18.2.38.TRY 2010-04-06 15:38:13.000000000 +0200 > > @@ -3327,6 +3327,11 @@ > > uint32_t control, status; > > int cons, len, port, rxprog; > > > > + int idx; > > + idx = CSR_READ_2(sc, STAT_PUT_IDX); > > + if (idx == sc->msk_stat_cons) > > + return (0); > > + > > /* Sync status LEs. */ > > bus_dmamap_sync(sc->msk_stat_tag, sc->msk_stat_map, > > BUS_DMASYNC_POSTREAD | BUS_DMASYNC_POSTWRITE); > > @@ -3407,7 +3412,7 @@ > > if (rxput[MSK_PORT_B] > 0) > > msk_rxput(sc->msk_if[MSK_PORT_B]); > > > > - return (rxprog > sc->msk_process_limit ? EAGAIN : 0); > > + return (sc->msk_stat_cons != CSR_READ_2(sc, STAT_PUT_IDX)); > > } > > > > static void > > > > Now performance seems to be the same as with the older > > driver (at least here at work) and in both directions! > > Some numbers: > > > > em0 writes to rev. 1.18.2.36: 20 seconds > > em0 writes to rev. 1.18.2.38: 50 seconds > > em0 writes to rev. 1.18.2.38 with patch from above: 23 seconds > > same as before but with int_holdoff: 100 -> 1: 20 seconds > > > > rev. 1.18.2.36 writes to em0: 22 seconds > > rev. 1.18.2.38 writes to em0: 40 seconds > > rev. 1.18.2.38 with patch from above writes to em0: 21 seconds > > same as before but with int_holdoff: 100 -> 1: 20 seconds > > > > It seems that these two CSR_READ_2s really help ;-). > > > > As I said, this is at work and with slightly different machines. > > I will try things at home later but I am rather confident of > > receiving good results there as well... > > OK, tests at home show similar good results with the > above patch. When setting int_holdoff to 3, performance > seems equal to the older versions. > Thanks a lot for narrowing down the issue. Because the msk_handle_events() are called in interrupt handler I'd like to remove the two register accesses in fast path. It seems accessing STAT_PUT_IDX triggers status updates. Would you try attached patch and let me know whether the patch makes any difference? --45Z9DzgjV8m4Oswq Content-Type: text/x-diff; charset=us-ascii Content-Disposition: attachment; filename="msk.status.update.patch" Index: sys/dev/msk/if_msk.c =================================================================== --- sys/dev/msk/if_msk.c (revision 206204) +++ sys/dev/msk/if_msk.c (working copy) @@ -1470,6 +1470,7 @@ /* WA for dev. #4.18 */ CSR_WRITE_1(sc, STAT_FIFO_WM, 0x21); CSR_WRITE_1(sc, STAT_FIFO_ISR_WM, 0x07); + CSR_WRITE_4(sc, STAT_TX_TIMER_INI, MSK_USECS(sc, 10)); } else { CSR_WRITE_2(sc, STAT_TX_IDX_TH, 0x0a); CSR_WRITE_1(sc, STAT_FIFO_WM, 0x10); @@ -1481,9 +1482,16 @@ CSR_WRITE_4(sc, STAT_ISR_TIMER_INI, 0x0190); } /* - * Use default value for STAT_ISR_TIMER_INI, STAT_LEV_TIMER_INI. + * STAT_TX_TIMER_INI, STAT_ISR_TIMER_INI and STAT_LEV_TIMER_INI + * as well as various water mark registers seem to control when + * controller initiates status LE update. + * It's not clear how these registers interact with interrupt + * state but STAT_TX_TIMER_INI seems to control status update + * time after crossing a threshold(STAT_TX_IDX_TH) value. Due to + * the complexity of various parameters that may affect status + * update just use hardware default until we know better + * the internal things of status updates. */ - CSR_WRITE_4(sc, STAT_TX_TIMER_INI, MSK_USECS(sc, 1000)); /* Enable status unit. */ CSR_WRITE_4(sc, STAT_CTRL, SC_STAT_OP_ON); --45Z9DzgjV8m4Oswq--