David Brownell wrote: > On Tuesday 20 February 2007 12:27 pm, [EMAIL PROTECTED] wrote: >> >> This patch turns off periodic list processing in the EHCI controller >> for the duration of processor frequency changes. > > Did you test this with any kind of isochronous device active, like a > set of USB speakers playing? ISO transfers work differently than the > interrupt transfers you tested (with a HID device). > > Also ... what kind of hardware did you test this with? I wonder how > much of the (potential) 1.125 msec it was spinning while waiting for > the periodic schedule to actually turn off. That seems like it > should be modifying the latency metrics used by cpufreq... is there > even a feedback mechanism whereby the system can say that _right now_ > it would take this much more time? Last time I remember looking at > cpufreq, it had static metrics basically relating only to the CPUs; > so costs related to components like EHCI were hidden. (Much less > costs associated with re-clocking other peripherals, which is a big > issue with embedded SOC chips!) > > > The patch looks mostly OK, but the issue with isochronous transfers > is a difference in how transfer completion is handled in the > hardware. > > Rather than scanning a queue head node in the schedule tree, which > will be re-scanned after the periodic list is re-activated. Instead, > isochronous transfers use an entirely different hardware mechanism. > Their ITDs go before that schedule tree, and won't get re-scanned > after the relevant frame passes. > > I'd not be sure that the current iso scanning logic would behave > correctly with this kind of on/off mechanism. In fact I'd kind of > expect it to break. > > That's in addition to the issue that it might not be a Good Thing to > let cpufreq create audio (or video etc) dropouts ... > > Maybe the best solution to this issue would be to reject the cpufreq > change if ISO transfers are active on EHCI. > > - Dave
OK, I've made an attempt to solve this issue without using such a large hammer. Instead of shutting off the entire periodic schedule during cpufreq changes, this patch will just inactivate (using the "I" bit in qh->hw_info1, currently unused in the EHCI driver) any queue heads that are to full/low speed interrupt endpoints, during the cpufreq changes. I have found that the ICH7 works without this patch, because it caches 7 uframes of the periodic schedule, so it never has to read main memory in order to complete a split transaction in time. The Broadcom/Serverworks HT1000 only caches 1 uframe, so it fails without this patch. I have tested this code on both the HT1000 and ICH7 (I tested the patch on the ICH7 with the cpufreq notifier registered, too, even though the patch won't even register it, just to make sure it would work correctly). This patch adds more code, but it avoids a couple of the issues that were brought up with the previous patch: 1--This patch doesn't mess with isochronous transfers (or high speed interrupt transfers) at all. 2--The notifier will take very little time unless the cpufreq change happens while a split transaction is in progress (or is already cached in the controller). I can repost it with a more concise summarization of the problem/fix and a signed-off-by if it looks ok. Thanks! Stuart
take7_i_bit.patch
Description: take7_i_bit.patch
------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV
_______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
