On Mon, Sep 15, 2014 at 11:22 PM, <dinguyen@xxxxxxxxxxxxxxxxxxxxx> wrote: > From: Dinh Nguyen <dinguyen@xxxxxxxxxxxxxxxxxxxxx> > > Hi Ming-Lei, > > Thanks for your patch to enable the URB giveback in a tasklet context for > the EHCI driver. I found your patch to fix a USB webcam timeout/stutter > issue on the DWC2 HCD in the SOCFPGA platform. > > However, I need your help trying to figure out why I need the 2nd patch to > get your URB giveback patch to fully work. Without your second patch, local interrupt is still disabled even the completion handler is run from tasklet context. Generally speaking, what we want is to decrease the interrupt disable period from USB. But this change can make the same host controller interrupt handled on other CPUs at the same time too. > > I enable tracepoints in the dwc2 dwc2_handle_hcd_intr for irq:irq_handler_entry > and irq:irq_handler_exit. However, I did not see any measurable differences > between enabling HCD_BH and no enabling HCD_BH. But I am able to get a full Which platform are you running? There should have been big difference between the two setting. Per my experiences, some ARM platform uses a low frequency clock source(such as 32K on pandaboard, or omap5), so the time stamp in trace isn't accurate enough, but high resolution timer can be passed from kernel parameter too. > image from a webcam with the following 2 patches, and timeouts/green screen > without the 2 patches. > > I am using a Logitech HD C270 webcam. > > Opening video decoder: [raw] RAW Uncompressed Video > Movie-Aspect is undefined - no prescaling applied. > VO: [sdl] 640x480 => 640x480 Packed YUY2 > Selected video codec: [rawyuy2] vfm: raw (RAW YUY2) > > Can you provide any comments on the following issues: > > 1) Am I placing the tracepoints in the right place for this non-ehci hcd? I think it is right if URB completion is run from the only interrupt handler of DWC2. Maybe you needn't put it by yourself, and the built-in irq_handler_entry and irq_handler_exit events should be enough for your test. > 2) I don't quite understand why removing local_irq_save/local_irq_restore > around the the complete makes the webcam work? The two helpers around the ->complete() should have been removed but it isn't mature since we need to make all current drivers safe with the conversion. See below link: http://marc.info/?w=2&r=1&s=enabling+irq+complete&q=t If you plan to use the feature, the above changes have to be merged first. Thanks, -- 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
