> >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__;!!EHsc >m >> >>S1ygiU1lA!H1h8GlLnbS6vqklXm_2qGyinP638O62Kk2eLB9zeMkjAGXUAPYyPXD >L >> >FasSqYt16xq0RGT0Ff-cP4A$ >> > uvc-gadget.sh start >> >2 - >> > https://urldefense.com/v3/__https://git.ideasonboard.org/uvc- >> >>gadget.git__;!!EHscmS1ygiU1lA!H1h8GlLnbS6vqklXm_2qGyinP638O62Kk2eLB >9z >> >eMkjAGXUAPYyPXDLFasSqYt16xq0RGT1ogOdRQA$ >> > uvc-gadget -i test.jpg >> > >> > >> >Host side: >> > https://urldefense.com/v3/__https://github.com/thekvs/uvccapture2 >> >>__;!!EHscmS1ygiU1lA!H1h8GlLnbS6vqklXm_2qGyinP638O62Kk2eLB9zeMkjAG >X >> >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://urldefense.com/v3/__https://elixir.bootlin.com/linux/latest/source/d >rivers/usb/gadget/udc/cdns2/cdns2- >gadget.c*L412__;Iw!!EHscmS1ygiU1lA!EZS2StTKnSzdCT7N5B1- >l8nGXQgS63KwgcGINcpBC8rnRJu2u8ryV1UjwQb6YfwYLPq9T_115KC5qQ$ . >> >> 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. At this moment I don't know how to resolve the problem 2. I'm going to recreate this case on my side and try to find some solution. Pawel > >> >> > >> >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 >> > >> > >> > >> > >> > >>
