Hi, On Thu, Mar 31, 2016 at 09:16:19AM +0300, Felipe Balbi wrote: > > Hi, > > Bin Liu <b-liu@xxxxxx> writes: > > [ text/plain ] > > Hi, > > > > On Wed, Mar 30, 2016 at 09:14:18AM +0300, Felipe Balbi wrote: > >> > >> Hi again, > >> > >> Felipe Balbi <felipe.balbi@xxxxxxxxxxxxxxx> writes: > >> > Bin Liu <b-liu@xxxxxx> writes: > >> >> On Fri, Mar 18, 2016 at 08:59:48AM +0200, Felipe Balbi wrote: > >> >>> > >> >>> Hi, > >> >>> > >> >>> Bin Liu <binmlist@xxxxxxxxx> writes: > >> >>> > [ text/plain ] > >> >>> > Hi, > >> >>> > > >> >>> > On Fri, Mar 11, 2016 at 6:54 AM, Felipe Balbi > >> >>> > <felipe.balbi@xxxxxxxxxxxxxxx> wrote: > >> >>> >> previously we were using a maximum of 32 TRBs per > >> >>> >> endpoint. With each TRB being 16 bytes long, we were > >> >>> >> using 512 bytes of memory for each endpoint. > >> >>> >> > >> >>> >> However, SLAB/SLUB will always allocate PAGE_SIZE > >> >>> >> chunks. In order to better utilize the memory we > >> >>> >> allocate and to allow deeper queues for gadgets > >> >>> >> which would benefit from it (g_ether comes to mind), > >> >>> >> let's increase the maximum to 256 TRBs which rounds > >> >>> >> up to 4096 bytes for each endpoint. > >> >>> > > >> >>> > Do we want to increase the same for event ring buffers as > >> >>> > while, which is allocated by dma_alloc_coherent(), which > >> >>> > is also at PAGE_SIZE chunks, right? > >> >>> > >> >>> I could, but that's much less important. Currently we have up to 2 > >> >> > >> >> I agree it is less important, the only issue I see is wasting of memory. > >> >> The device I am working on right now has two dwc3 controllers, each > >> >> allocates 16 event buffers, so for the total of 128KB allocated pages, > >> >> only 8KB is really used, 120KB is wasted. > >> >> > >> >> Seems dma pool makes more sense in here? > >> > > >> > I don't know. I think the real thing is that I really need to revisit > >> > that part of the code/databook. The whole "multiple interrupters" seems > >> > like it's only really necessary for host side. Which means that we could > >> > drop all the loops for multiple event buffers and always use a single > >> > one. > >> > > >> > Do you wanna test the following ? > >> > > >> > diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c > >> > index 17fd81447c9f..ebb3ee9c06f1 100644 > >> > --- a/drivers/usb/dwc3/core.c > >> > +++ b/drivers/usb/dwc3/core.c > >> > @@ -237,8 +237,7 @@ static int dwc3_alloc_event_buffers(struct dwc3 *dwc, unsigned length) > >> > int num; > >> > int i; > >> > > >> > - num = DWC3_NUM_INT(dwc->hwparams.hwparams1); > >> > - dwc->num_event_buffers = num; > >> > + dwc->num_event_buffers = 1; > >> > > >> > dwc->ev_buffs = devm_kzalloc(dwc->dev, sizeof(*dwc->ev_buffs) * num, > >> > GFP_KERNEL); > >> > > >> > I'll re-read what these bits actually mean. I have a strong feeling we > >> > could (should?) be ignoring them for the peripheral side. > >> > >> Okay, so when we're configuring the endpoints, we could route endpoint > >> interrupts to different event buffers. I really think that's really > >> unimportant for us, specially since we end up using a single IRQ line. > >> > >> I guess I'll just go ahead and remove that code. If it turns out we > >> decide to use it, we shouldn't really be using a loop in the hardirq > >> handler anyway. > > > > Sounds good to me, I only see one evt buffer is used in all my devices, > > even thought multi buffers are allocated based on hwparams1. > > I sent some patches yesterday. You might wanna give it a review ;-) The 3 patches are all look good to me. I bet you already tested it, so I didn't do so. ;) Regards, -Bin. -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
