On Mon, Oct 30, 2023 at 06:34:48AM +0000, Pawel Laszczak wrote: > > > > >hi Pawel Laszczak > > > >Recently, I met the problem when use uvc. UVC report jpg header error. > > > >Basic reproduce steps. > >Gadget side: > >1 - > > https://urldefense.com/v3/__https://gist.github.com/kbingham/c39c > >4cc7c20882a104c08df5206e2f9f?permalink_comment_id=3270713__;!!EHscm > >S1ygiU1lA!H1h8GlLnbS6vqklXm_2qGyinP638O62Kk2eLB9zeMkjAGXUAPYyPXDL > >FasSqYt16xq0RGT0Ff-cP4A$ > > uvc-gadget.sh start > >2 - > > https://urldefense.com/v3/__https://git.ideasonboard.org/uvc- > >gadget.git__;!!EHscmS1ygiU1lA!H1h8GlLnbS6vqklXm_2qGyinP638O62Kk2eLB9z > >eMkjAGXUAPYyPXDLFasSqYt16xq0RGT1ogOdRQA$ > > uvc-gadget -i test.jpg > > > > > >Host side: > > https://urldefense.com/v3/__https://github.com/thekvs/uvccapture2 > >__;!!EHscmS1ygiU1lA!H1h8GlLnbS6vqklXm_2qGyinP638O62Kk2eLB9zeMkjAGX > >UAPYyPXDLFasSqYt16xq0RGT1MNlKiXA$ > > uvccapture2 --device /dev/video0 --resolution 640x360 --count 1 -- > >result 8qxp.jpeg > > > > It will report jpeg header error. > > > > > >After debugs, I found two problem. > > > >Problem 1, sg is enabled. so uvc driver will use sg. each package include two > >trb, trb0 is 8bytes header, trb1 is 1016bytes. total 1024. > > > >num_trb here is wrong. > >it should be > > num_trb = priv_ep->interval * request->num_mapped_sgs. > > > >because priv_ep->interval is 1, I just simple set to request->num_mapped_sg > >as below patch. USB analyer show one whole 1024 ISO package sent out as > >expectation although document said only support one TD when use ISO > >(Maybe my doc is too old). > > Support for sg in uvc has been added after upstreaming this driver, so the driver > needs some improvement. > > Calculating of num_trb probably will more complicated change. > > You can see how it is implemented in > https://elixir.bootlin.com/linux/latest/source/drivers/usb/gadget/udc/cdns2/cdns2-gadget.c#L412. > > CDNS2 is different controller and support only HS but has borrowed the DMA part from CDNS3. > It was upsteamed after adding sg to UVC. > > Regarding TD, it is true that controller can support only one TD per SOF but this TD can contain many TRBs Okay, great. I can work a patch if I can resolve problem 2. > > > > >diff --git a/drivers/usb/cdns3/cdns3-gadget.c b/drivers/usb/cdns3/cdns3- > >gadget.c > >index 69a44bd7e5d02..8cc99a885883f 100644 > >--- a/drivers/usb/cdns3/cdns3-gadget.c > >+++ b/drivers/usb/cdns3/cdns3-gadget.c > >@@ -1125,10 +1125,7 @@ static int cdns3_ep_run_transfer(struct > >cdns3_endpoint *priv_ep, > > struct scatterlist *s = NULL; > > bool sg_supported = !!(request->num_mapped_sgs); > > > >- if (priv_ep->type == USB_ENDPOINT_XFER_ISOC) > >- num_trb = priv_ep->interval; > >- else > >- num_trb = sg_supported ? request->num_mapped_sgs : 1; > >+ num_trb = sg_supported ? request->num_mapped_sgs : 1; > > > > if (num_trb > priv_ep->free_trbs) { > > priv_ep->flags |= EP_RING_FULL; > > > > > >*** Problem 2 *** > > > >According to doc and my observation, looks like hardware fetch data into FIFO > >when get SOF, then transfer data when get IN token. Each SOF will increase > >TRB regardless it is ready or not. > > Yes, but this fetched data will be sent in next ITP. > > > > >When gadget complete equeue ISO data, so SOF will increase TRB regardless if > >there are IN token. > > > > SOF SOF SOF SOF IN SOF .... > > TRB0 TRB1 TRB2 TRB3 ... > > > > > >Host may start get data at some time after gadget queue data. > > > >So TRB0..2 data will be lost. > > > >If it is audio data, it should be okay. But for uvc, it is jpeg header, so host side > >report error. > > > >I checked dwc gadget driver, which start equeue ISO data only get NYET. > > > >I check cdns spec, there are ISOERR. But it is never happen. According to > >document, ISOERR should issue when IN token and FIFO no data. > > > > Current CDNS3 driver has disabled ISOERR. Did you enable it? Yes, I enabled all IRQ. + if (priv_ep->type == USB_ENDPOINT_XFER_ISOC && priv_ep->dir) { + priv_ep->flags |= EP_QUIRK_ISO_IN_NOST; + reg |= 0xFFFF; + } Supposed ISOERR should happen even DMA is disabled. But I also tried enable DMA, and using lenght 0 TRB and link to loop. Still no ISOERR happen. I can see TRBADDR changed, but still no ISOERR irq/447-5b13000-200 [000] d..1. 78.662729: cdns3_epx_irq: IRQ for ep2in: 00000804 IOC , ep_traddr: c0086018 ep_last_sid: 00000000 use_streams: 0 ^^^^^^^ irq/447-5b13000-200 [000] d..1. 78.662851: cdns3_epx_irq: IRQ for ep2in: 00000804 IOC , ep_traddr: c008600c ep_last_sid: 00000000 use_streams: 0 irq/447-5b13000-200 [000] d..1. 78.662975: cdns3_epx_irq: IRQ for ep2in: 00000804 IOC , ep_traddr: c0086018 ep_last_sid: 00000000 use_streams: 0 STS is 0x804, only IOC set. Frank > > >I tried below method > > 1. Delay queue TRB, but no ISOERR. > > 2. queue a lenght 0 TRB,but no ISOERR > > > >My question is how to delay queue TRB to ISO IN token really happen to avoid > >lost JPEG header. > > > >Frank > > > > > > > > > > >
