> -----Original Message----- > From: Sarah Sharp [mailto:sarah.a.sharp@xxxxxxxxxxxxxxx] > Sent: Wednesday, June 15, 2011 11:32 AM > To: Xu, Andiry > Cc: linux-usb@xxxxxxxxxxxxxxx; matthew.r.wilcox@xxxxxxxxx > Subject: Re: [PATCH 5/5] xHCI: assign devices to different interrupters > > On Fri, Jun 10, 2011 at 04:51:43PM +0800, Andiry Xu wrote: > > This patch assign devices to different interrupters based on its > slot_id. > > However, it's not mandatory that a device should use the same > interrupter. > > A device can assign its control transfer to one interrupter and > bulk/intr/ > > isoc transfer to another interrupter. The interrupter usage policy is > > flexible. > > > > The policy in this patch may be replaced by better approach in the > future. > > I would like to get a more flexible policy, or at least the APIs to > allow drivers or the USB core flexibility in directing URB traffic to > different interrupters before this gets merged. I'm most concerned > about allowing drivers or the USB core to change interrupt moderation > rate the rings should use. Either that, or the xHCI driver needs to > choose the moderation interval based on the endpoint type and polling > rate. I think we need that, because I have doubts that these patches > alone will improve performance. I don't really want to add this much > optional code without some indication that it will help performance. > Agreed. But does the IMODI field can be modified during running? The spec only says the IMODC field may be directly written by software at any time to alter the interrupt rate. > What about adding a field to the struct urb to allow the driver > to set the interrupter field? We might have to add some USB core API > to > allow the driver to figure out how many interrupters are available. If > the driver doesn't set the field, we can just use the per-device > basis. > Adding a intr_target field to the URB? Then to set the field, usb core not only need to know the number of interrupters enabled, but also needs to find out the most suitable interrupter. Does every urb need to calculate the field before being submitted to the host driver? If so, I think it's another performance lost. > I think we'd have to restrict the driver to only using one interrupter > per endpoint, unless streams are enabled. Otherwise I think it breaks > the xhci_td list handling assumption that events are handed back in the > order they are placed on the ring, because we could have a race > condition between two CPUs that receive interrupts for the same > endpoint. You could set an initial interrupter used in the > xhci_virt_ep > structure, and then double check it whenever a new URB is queued for > that endpoint. > I think different kinds of eps can be assigned to different interrupters. For example, bulk ep can use an interrupter with lower interrupt rate. Maybe xHCI driver can simply initialize interrupters for different kinds of eps, and assign urbs to them. > Then if each URB could have a different interrupter, the UAS driver > could > direct different stream transfers to different interrupters. > Does it require to modify UAS driver? I'm not familiar with it. BTW, I found that current xHCI driver initialize the interrupter's IMODI Field with value 160. The default value is 4000. With the algorithm in 5.5.2.2, we know it means 1000 interrupts per second. However, 160 means 25000 interrupts per second. xHCI spec says, the maximum observable interrupt rate from the xHC should never exceed 8000 interrupts/sec. Especially, 4.17.2 says, an initial suggested range for the moderation Interval is 651-5580 (28Bh - 15CCh). So I wonder why the value 160 is chose, is there any special reason? Thanks, Andiry -- 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
