On Mon, Nov 06, 2023 at 11:12:10AM +0000, Pawel Laszczak wrote: > > > >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 I post a sg ISO fix patch, in case you need it. https://lore.kernel.org/imx/BYAPR07MB538112FE4150BC19696D7613DDAAA@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/#R It fixed my case. but android looks like still issue. Recently quite busy. Frank Li > > > > >> > >> > > >> >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 > >> > > >> > > >> > > >> > > >> > > >>
