From owner-freebsd-usb@FreeBSD.ORG Mon Dec 22 09:32:01 2014 Return-Path: Delivered-To: freebsd-usb@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 6EA1C6DA for ; Mon, 22 Dec 2014 09:32:01 +0000 (UTC) Received: from smtp.mei.co.jp (smtp.mei.co.jp [133.183.100.20]) by mx1.freebsd.org (Postfix) with ESMTP id 25A683FA5 for ; Mon, 22 Dec 2014 09:32:00 +0000 (UTC) Received: from mail-gw.jp.panasonic.com ([157.8.1.157]) by smtp.mei.co.jp (8.12.11.20060614/3.7W/kc-maile12) with ESMTP id sBM9VwlY001594; Mon, 22 Dec 2014 18:31:58 +0900 (JST) Received: from epochmail.jp.panasonic.com ([157.8.1.130]) by mail.jp.panasonic.com (8.11.6p2/3.7W/kc-maili12) with ESMTP id sBM9Vwo19015; Mon, 22 Dec 2014 18:31:58 +0900 Received: by epochmail.jp.panasonic.com (8.12.11.20060308/3.7W/lomi13) id sBM9VwIs022642; Mon, 22 Dec 2014 18:31:58 +0900 Received: from localhost by lomi13.jp.panasonic.com (8.12.11.20060308/3.7W) with ESMTP id sBM9VwhC022612; Mon, 22 Dec 2014 18:31:58 +0900 Date: Mon, 22 Dec 2014 18:31:57 +0900 (JST) Message-Id: <20141222.183157.253796126986510571.okuno.kohji@jp.panasonic.com> To: hps@selasky.org Subject: Re: About XHCI_TD_PAGE_SIZE. From: Kohji Okuno In-Reply-To: <5497E016.7020809@selasky.org> References: <549541BC.6070505@selasky.org> <20141222.103833.2105768702793613386.okuno.kohji@jp.panasonic.com> <5497E016.7020809@selasky.org> Organization: Panasonic Corporation X-Mailer: Mew version 6.6 on Emacs 24.4 / Mule 6.0 (HANACHIRUSATO) Mime-Version: 1.0 Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Cc: freebsd-usb@freebsd.org X-BeenThere: freebsd-usb@freebsd.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: FreeBSD support for USB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 22 Dec 2014 09:32:01 -0000 From: Hans Petter Selasky Subject: Re: About XHCI_TD_PAGE_SIZE. Date: Mon, 22 Dec 2014 10:10:46 +0100 Message-ID: <5497E016.7020809@selasky.org> > On 12/22/14 02:38, Kohji Okuno wrote: >> From: Hans Petter Selasky >> Subject: Re: About XHCI_TD_PAGE_SIZE. >> Date: Sat, 20 Dec 2014 10:30:36 +0100 >> >>> On 12/17/14 05:42, Nidal Khalil wrote: >>>> I agree. Thanks >>>> >>>> Nidal >>>> On Dec 16, 2014 6:25 PM, "Kohji Okuno" >>>> wrote: >>>> >>>>> Hi Hans, >>>>> >>>>> If we use PAGE_SIZE as USB_PAGE_SIZE, we should use PAGE_SIZE as >>>>> XHCI_TD_PAGE_SIZE, too. I think. >>>>> >>>>> As you know, one TRB can use 1~64kB for the transfer length. >>>>> >>> >>> Hi, >>> >>> We currently only check if 4K pages are supported by the hardware. If you >>> change the value of XHCI_TD_PAGE_SIZE, you will also need to change the >>> checks >>> other places. You know that PAGE_SIZE is not a constant? >>> >>> Do you have a complete patch? >>> >>> --HPS >>> >> >> Hi, >> >> XHCI_TD_PAGE_SIZE is used at only 2-points. >> >> 1. use as XHCI_TD_PAYLOAD_MAX (XHCI_TD_PAGE_NBUF) in xhci.h >> We sholud change as the following, I think. >> >> - #define XHCI_TD_PAGE_NBUF 17 /* units, room enough for 64Kbytes */ >> - #define XHCI_TD_PAGE_SIZE 4096 /* bytes */ >> - #define XHCI_TD_PAYLOAD_MAX (XHCI_TD_PAGE_SIZE * (XHCI_TD_PAGE_NBUF - 1)) >> + #define XHCI_TD_PAYLOAD_MAX (64*1024) /* bytes */ >> + #define XHCI_TD_PAGE_SIZE PAGE_SIZE /* bytes */ >> + /* units, room enough for 64Kbytes */ >> + #define XHCI_TD_PAGE_NBUF (XHCI_TD_PAYLOAD_MAX/XHCI_TD_PAGE_SIZE + 1) >> >> 2. use as the maximum length of TRB. >> If PAGE_SIZE is 8kB, buf_res.length may be 8kB. >> But, we can set 1B~64kB for length of TRB. This is the spcification >> of xHCI. So, we don't need change this point. >> >> xhci.c >> 1807 /* check for maximum length */ >> 1808 if (buf_res.length > XHCI_TD_PAGE_SIZE) >> 1809 buf_res.length = XHCI_TD_PAGE_SIZE; >> > > Hi Kohji, > > I see your points. By doing this you save some memory in the descriptor layout > for 8K page size - right? > > BTW: Do you think the following check is OK, or should it be extended to check > also for 8K? > > if (!(XREAD4(sc, oper, XHCI_PAGESIZE) & XHCI_PAGESIZE_4K)) { > device_printf(sc->sc_bus.parent, "Controller does " > "not support 4K page size.\n"); > return (USB_ERR_IOERROR); > } > > I guess your patch is more in the direction of optimisation. Is it very > urgent? Or is it fine if I handle it the beginning of January? > > Thank you for your input and review! > > --HPS > Hi HPS, > I see your points. By doing this you save some memory in the > descriptor layout for 8K page size - right? Yes. In addition, we may reduce the size of data which a xhci controller fetches. > I guess your patch is more in the direction of optimisation. Is it very > urgent? Or is it fine if I handle it the beginning of January? No, it isn't urgent. It's OK in the next month. In an optimisation, we should reduce the number of LINK_TRB, too. I heard from a LSI engineer that, Generally xhci controler has the cache of TRB array. But, LINK_TRB may make the cache miss. Unfortunately, I don't know about XHCI_PAGESIZE. If I can hear from a LSI engineer, I will inform you. Best regards, Kohji Okuno