Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.33 as permitted sender) client-ip=23.128.96.33;
Date:   Mon, 6 Nov 2023 10:38:24 -0500
From:   Frank Li <Frank.li@nxp.com>
To:     Pawel Laszczak <pawell@cadence.com>
Cc:     "peter.chen@kernel.org" <peter.chen@kernel.org>,
        "rogerq@kernel.org" <rogerq@kernel.org>,
        "a-govindraju@ti.com" <a-govindraju@ti.com>,
        "gregkh@linuxfoundation.org" <gregkh@linuxfoundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-usb@vger.kernel.org" <linux-usb@vger.kernel.org>,
        "imx@lists.linux.dev" <imx@lists.linux.dev>
Subject: Re: cdns3 uvc first ISO parkage lost problem
Message-ID: <ZUkIcCAdDQS0n8A5@lizhi-Precision-Tower-5810>
References: <ZTvhEl+JcnhJXcrl@lizhi-Precision-Tower-5810>
 <BYAPR07MB5381F7224612F0C0793B6B29DDA1A@BYAPR07MB5381.namprd07.prod.outlook.com>
 <ZT/XkhOvbucxA90V@lizhi-Precision-Tower-5810>
 <BYAPR07MB538112FE4150BC19696D7613DDAAA@BYAPR07MB5381.namprd07.prod.outlook.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <BYAPR07MB538112FE4150BC19696D7613DDAAA@BYAPR07MB5381.namprd07.prod.outlook.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Entdm5YBa7naOmzIdolxvpFPrFZEyM8TJPR1E7ME+DbqlwVU+zEmLM/qClMc?=
 =?us-ascii?Q?Wc6Qtb4sJ3lPOhfSiepqb9D84iDnUik/u7LJd+chQf8ZVJGkXwxXLl7zkeiT?=
 =?us-ascii?Q?UK8ilT7DQ9+/T9COLfNLrMZHtFAs7COi02TNnzJtLpag4Zs8+8CMrEh91pOd?=
 =?us-ascii?Q?25DhC3E3zMWTS7esOhOWYPQbstQ4LSY6bJ4Rlf3eSzU50oMk3ejxq+E6ixen?=
 =?us-ascii?Q?UJxOuDGleQbx/z5mh2skS3uk6wBVfmHRbrHW58X3AVN6XaENlBAVyFLc7pNU?=
 =?us-ascii?Q?hW9+n/rV9zFvWCEORfdHXIEIsRCuLztZ/RUhlMcz8gIRUvtrXSmUnt0bCMen?=
 =?us-ascii?Q?qhptyNEzJ3U///NyS4wPaglgMp+zSB01AOog8MQ4J/2v2N61BzkjY7f885WF?=
 =?us-ascii?Q?wO3bU/hQWABBLIofeDz4paICfmUl3qtYjjxbQNp8WIGYs5E//32BbVRRACBy?=
 =?us-ascii?Q?4C1bVgnv7Zke6FyCPusw7b7iTQ3j0uQeiE7zSOEeN0uQKyC1IN5P5qhcdtDD?=
 =?us-ascii?Q?HHcnyDAap2POTkOG6KaE6o5+JWpv9OqA3TgCb+sIzUVtmUed1xb2gXaPklU0?=
 =?us-ascii?Q?InAxHd6Kenf1aY2Z31aZodLXY2GjUpBlm+pxmYGRNdjyE67Jdw5mTNRpplW2?=
 =?us-ascii?Q?ot7aJSXJuF3v8ngrkyrVX5zbSWANusN0a5jOUZyNYAL9b2vCT+yfEopSfUm/?=
 =?us-ascii?Q?A5dywIhJPKJkD9j/3fnI9aBWowS8HhLTFWvREgLad6BpyP7RYn4aIuT4aPlq?=
 =?us-ascii?Q?R64T7w+dZoVYJuRuzXdZJF3DHOVkkXEncb85OtTWu23HwqsdJpVPjJNykGsc?=
 =?us-ascii?Q?sS6llpix6ym6gzQ9eCaZu1IC5dvAI5gvgMOCbB9kR049/PaJWKxrFXqZLqGv?=
 =?us-ascii?Q?nnnN5J8RocAtRJ5p9AKqXfTS5G+8Jz0BiV1TJPyUtuherqrYYLSw63q3F1rD?=
 =?us-ascii?Q?pfGS6fiXLtKa5xyjW3F3sIU2a7d/yetlRXsUqxapukVckrWVjp/EeyP63Hin?=
 =?us-ascii?Q?fwj8GK4kJIOh6H9FaBbMPj9XOUFIcLa4YOdjl+Kvx+9tU5gw0ZAzmGS9YTXa?=
 =?us-ascii?Q?ozg5Z3t6GR34UscdNVrEUeDs4XM9im813rUruXLpwXLkeZLuvOoZGw4tztpq?=
 =?us-ascii?Q?wl3PWmlQqBo+TbTgZx3nvSspLpmC6yTcLoJCmdEhmGYl7sOqYYqmDpoDT7Pa?=
 =?us-ascii?Q?y2hpbWhDoYuSZu0r6c3kqEay8mPxqXFvSvyE5UkIYNZFcgTPfYV105w7DPcB?=
 =?us-ascii?Q?kljamf8hb8FdTPSPxgyHVRWtmjodk60woClpsGTh91iHVoaoOZWnuT76b3OT?=
 =?us-ascii?Q?XUl0DOtWSXS3JYiGmiL2u70cDOT0j9nJaMry6IbHeCUwkeuKE6nsTb8SlOw8?=
 =?us-ascii?Q?OqsACJYxCgga7+jIAbSO9B9SDLHE084Ewx/N/jW9dmtmwdWeOHpOgoTBcxL/?=
 =?us-ascii?Q?3pKqJHxJBORF++PmqKVRTns+oDDsKayNs9RZmyA4TCot0S+Am1nlkI4xFBaU?=
 =?us-ascii?Q?fOpWzTQqKVCe5JxezhSeyTAoRMO/ZlOGc/pfIVhIb84hly9it6hnkp0/5JDQ?=
 =?us-ascii?Q?0rc8uqFjRkAeo5MWtbw=3D?=
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9c844697-9477-4bdd-2d7e-08dbdede6e83
X-MS-Exchange-CrossTenant-AuthSource: AM6PR04MB4838.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 06 Nov 2023 15:38:32.3003
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: RQPS3BNqQq+La8jXiu3vpXLLdYA5RZuSp6fkXi9BLHjBmn3vV3JW345wsnWTki686nykzPyDFLzYhbGYnHWtcg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AS8PR04MB8513
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lipwig.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (lipwig.vger.email [0.0.0.0]); Mon, 06 Nov 2023 07:38:49 -0800 (PST)

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@BYAPR07MB5381.namprd07.prod.outlook.com/#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
> >> >
> >> >
> >> >
> >> >
> >> >
> >>