Re: About XHCI_TD_PAGE_SIZE.

2014-12-30 Thread Hans Petter Selasky 

Hi Kohji!

Here you go:

https://svnweb.freebsd.org/changeset/base/276407

Please verify!

--HPS

___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org

Re: About XHCI_TD_PAGE_SIZE.

2014-12-22 Thread Hans Petter Selasky 

On 12/22/14 02:38, Kohji Okuno wrote:

From: Hans Petter Selasky h...@selasky.org
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 okuno.ko...@jp.panasonic.com 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

___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org

Re: About XHCI_TD_PAGE_SIZE.

2014-12-22 Thread Kohji Okuno 
From: Hans Petter Selasky h...@selasky.org
Subject: Re: About XHCI_TD_PAGE_SIZE.
Date: Mon, 22 Dec 2014 10:10:46 +0100
Message-ID: 5497e016.7020...@selasky.org

 On 12/22/14 02:38, Kohji Okuno wrote:
 From: Hans Petter Selasky h...@selasky.org
 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 okuno.ko...@jp.panasonic.com
 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
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org

Re: About XHCI_TD_PAGE_SIZE.

2014-12-22 Thread Hans Petter Selasky 

On 12/22/14 10:31, Kohji Okuno wrote:

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.


Hi,

One reason for the additional link TRB's is that I think there is 
different behaviour among the XHCI implementations how and when they 
generate a completion event. Any changes we make in that area needs to 
be tested with different XHCI controllers.


Currently only the umass driver will use transfers greater than 64KBytes 
in case of USB 3.0. Else the most common case is transfers below 
64KBytes, except for custom apps.


I have another idea pending, which is related to performance, but not 
the XHCI. I was thinking that the FreeBSD libusb could be extended to 
allocate a data buffer in the kernel which then gets mmapped to 
userspace, to save copying/copyout of USB transfer data. The problem 
about mmap() is that the buffers cannot be freed afterwards, and must 
remain in the kernel.


What do you think?

--HPS
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org

Re: About XHCI_TD_PAGE_SIZE.

2014-12-22 Thread Ian Lepore 
On Mon, 2014-12-22 at 10:48 +0100, Hans Petter Selasky wrote:
 On 12/22/14 10:31, Kohji Okuno wrote:
  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.
 
 Hi,
 
 One reason for the additional link TRB's is that I think there is 
 different behaviour among the XHCI implementations how and when they 
 generate a completion event. Any changes we make in that area needs to 
 be tested with different XHCI controllers.
 
 Currently only the umass driver will use transfers greater than 64KBytes 
 in case of USB 3.0. Else the most common case is transfers below 
 64KBytes, except for custom apps.
 
 I have another idea pending, which is related to performance, but not 
 the XHCI. I was thinking that the FreeBSD libusb could be extended to 
 allocate a data buffer in the kernel which then gets mmapped to 
 userspace, to save copying/copyout of USB transfer data. The problem 
 about mmap() is that the buffers cannot be freed afterwards, and must 
 remain in the kernel.
 
 What do you think?

So you're going to be doing DMA directly in and out of buffers mapped
into userspace?  I think that will fail on ARM and MIPS at least, and
maybe others (any platform that does cache maintenance based on virtual
addresses will be unable to do the maintenance reliably if the DMA
memory is mapped to multiple virtual addresses).  The solution for that
is bounce buffers, which just gets you right back into copying the data.

-- Ian


___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org

Re: About XHCI_TD_PAGE_SIZE.

2014-12-21 Thread Kohji Okuno 
From: Hans Petter Selasky h...@selasky.org
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 okuno.ko...@jp.panasonic.com 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;


Best regards,
 Kohji Okuno
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org

Re: About XHCI_TD_PAGE_SIZE.

2014-12-20 Thread Hans Petter Selasky 

On 12/17/14 05:42, Nidal Khalil wrote:

I agree. Thanks

Nidal
On Dec 16, 2014 6:25 PM, Kohji Okuno okuno.ko...@jp.panasonic.com 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

___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org

About XHCI_TD_PAGE_SIZE.

2014-12-16 Thread Kohji Okuno 
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.

Best regards,
 Kohji Okuno
___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org

Re: About XHCI_TD_PAGE_SIZE.

2014-12-16 Thread Nidal Khalil 
I agree. Thanks

Nidal
On Dec 16, 2014 6:25 PM, Kohji Okuno okuno.ko...@jp.panasonic.com 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.

 Best regards,
  Kohji Okuno
 ___
 freebsd-usb@freebsd.org mailing list
 http://lists.freebsd.org/mailman/listinfo/freebsd-usb
 To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org

___
freebsd-usb@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-usb
To unsubscribe, send any mail to freebsd-usb-unsubscr...@freebsd.org