Re: [RFC PATCH 0/2] usb: dwc2: Enable URB giveback in a tasklet context
On Mon, 15 Sep 2014 dingu...@opensource.altera.com wrote: From: Dinh Nguyen dingu...@opensource.altera.com 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. Maybe you don't need it. Did you try testing with only the first patch? Alan Stern -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/2] usb: dwc2: Enable URB giveback in a tasklet context
On 09/15/2014 11:08 AM, Alan Stern wrote: On Mon, 15 Sep 2014 dingu...@opensource.altera.com wrote: From: Dinh Nguyen dingu...@opensource.altera.com 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. Maybe you don't need it. Did you try testing with only the first patch? Yes, I did. 2nd patch was needed. Thanks, Dinh -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [RFC PATCH 0/2] usb: dwc2: Enable URB giveback in a tasklet context
From: dingu...@opensource.altera.com [mailto:dingu...@opensource.altera.com] Sent: Monday, September 15, 2014 8:23 AM 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. 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 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? 2) I don't quite understand why removing local_irq_save/local_irq_restore around the the complete makes the webcam work? Thanks in advance for your comments. Hi Dinh, I'm not Ming, but I can hazard a guess why you are seeing this issue. The webcam driver is likely doing a lot of processing in the -complete callback. With local interrupts disabled, this probably causes too long of a delay in handling other interrupts. Since the SOCFPGA platform is a uniprocessor (I think), there is no other CPU to handle the other interrupts. I think the only solution is to look at whatever driver is handling the webcam, and see if it can re-enable interrupts earlier. I believe this is an OK thing for it to do when the interrupt handler is threaded. Ah, wait. I just looked at the specs, and I see the SOCFPGA is dual-core. So the second core should be available for handling the other interrupts. I wonder if something is wrong there? Is the 2nd CPU actually running? Are interrupts getting disabled on both CPUs somehow? -- Paul -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/2] usb: dwc2: Enable URB giveback in a tasklet context
On Mon, Sep 15, 2014 at 11:22 PM, dingu...@opensource.altera.com wrote: From: Dinh Nguyen dingu...@opensource.altera.com 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=2r=1s=enabling+irq+completeq=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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/2] usb: dwc2: Enable URB giveback in a tasklet context
On Tue, Sep 16, 2014 at 7:16 AM, Paul Zimmerman paul.zimmer...@synopsys.com wrote: From: dingu...@opensource.altera.com [mailto:dingu...@opensource.altera.com] Sent: Monday, September 15, 2014 8:23 AM 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. 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 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? 2) I don't quite understand why removing local_irq_save/local_irq_restore around the the complete makes the webcam work? Thanks in advance for your comments. Hi Dinh, I'm not Ming, but I can hazard a guess why you are seeing this issue. The webcam driver is likely doing a lot of processing in the -complete callback. With local interrupts disabled, this probably causes too long of a delay in handling other interrupts. Since the SOCFPGA platform is a uniprocessor (I think), there is no other CPU to handle the other interrupts. Per my previous observation, on some ARM platforms it is extremely slow to memcpy to DMA coherent buffer, which is used in uvcvideo at default. I remember Pandaboard should be one such platform. You can check if it is your case. I think the only solution is to look at whatever driver is handling the webcam, and see if it can re-enable interrupts earlier. I believe this is an OK thing for it to do when the interrupt handler is threaded. You need to persuade all usb camera drivers' maintainera to agree on this change to these drivers, :-) We discussed to take threaded-irq too, looks which may decrease usb-storage performance a bit. Ah, wait. I just looked at the specs, and I see the SOCFPGA is dual-core. So the second core should be available for handling the other interrupts. I wonder if something is wrong there? Is the 2nd CPU actually running? Are interrupts getting disabled on both CPUs somehow? The same interrupt can't be handled on two CPU cores at the same time. Thanks, -- To unsubscribe from this list: send the line unsubscribe linux-usb in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RFC PATCH 0/2] usb: dwc2: Enable URB giveback in a tasklet context
Hi, On Tue, Sep 16, 2014 at 08:35:17AM +0800, Ming Lei wrote: On Tue, Sep 16, 2014 at 7:16 AM, Paul Zimmerman paul.zimmer...@synopsys.com wrote: From: dingu...@opensource.altera.com [mailto:dingu...@opensource.altera.com] Sent: Monday, September 15, 2014 8:23 AM 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. 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 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? 2) I don't quite understand why removing local_irq_save/local_irq_restore around the the complete makes the webcam work? Thanks in advance for your comments. Hi Dinh, I'm not Ming, but I can hazard a guess why you are seeing this issue. The webcam driver is likely doing a lot of processing in the -complete this is correct, yes. callback. With local interrupts disabled, this probably causes too long of a delay in handling other interrupts. Since the SOCFPGA platform is a uniprocessor (I think), there is no other CPU to handle the other interrupts. Per my previous observation, on some ARM platforms it is extremely slow to memcpy to DMA coherent buffer, which is used in uvcvideo at default. I remember Pandaboard should be one such platform. you remember correctly ;-) -- balbi signature.asc Description: Digital signature
