Re: [PATCH v4] usb: xhci: quirk for data loss in ISOC transfers

["Rangoju, Raju" <raju.rangoju@amd.com>]

On 1/29/2025 6:08 PM, Mathias Nyman wrote:

Hi Mathias,

Thanks for reviewing the patch and apologies for delayed response.

> On 27.1.2025 14.06, Raju Rangoju wrote:
>> During the High-Speed Isochronous Audio transfers, xHCI
>> controller on certain AMD platforms experiences momentary data
>> loss. This results in Missed Service Errors (MSE) being
>> generated by the xHCI.
>>
>> The root cause of the MSE is attributed to the ISOC OUT endpoint
>> being omitted from scheduling. This can happen either when an IN
>> endpoint with a 64ms service interval is pre-scheduled prior to
>> the ISOC OUT endpoint or when the interval of the ISOC OUT
>> endpoint is shorter than that of the IN endpoint. Consequently,
>> the OUT service is neglected when an IN endpoint with a service
>> interval exceeding 32ms is scheduled concurrently (every 64ms in
>> this scenario).
>>
>> This issue is particularly seen on certain older AMD platforms.
>> To mitigate this problem, it is recommended to adjust the service
>> interval of the IN endpoint to not exceed 32ms (interval 8). This
>> adjustment ensures that the OUT endpoint will not be bypassed,
>> even if a smaller interval value is utilized.
>>
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Raju Rangoju <Raju.Rangoju@amd.com>
>> ---
>> Changes since v3:
>>   - Bump up the enum number XHCI_LIMIT_ENDPOINT_INTERVAL_9
>>
>> Changes since v2:
>>   - added stable tag to backport to all stable kernels
>>
>> Changes since v1:
>>   - replaced hex values with pci device names
>>   - corrected the commit message
>>
>>   drivers/usb/host/xhci-mem.c |  5 +++++
>>   drivers/usb/host/xhci-pci.c | 25 +++++++++++++++++++++++++
>>   drivers/usb/host/xhci.h     |  1 +
>>   3 files changed, 31 insertions(+)
>>
>> diff --git a/drivers/usb/host/xhci-mem.c b/drivers/usb/host/xhci-mem.c
>> index 92703efda1f7..d3182ba98788 100644
>> --- a/drivers/usb/host/xhci-mem.c
>> +++ b/drivers/usb/host/xhci-mem.c
>> @@ -1420,6 +1420,11 @@ int xhci_endpoint_init(struct xhci_hcd *xhci,
>>       /* Periodic endpoint bInterval limit quirk */
>>       if (usb_endpoint_xfer_int(&ep->desc) ||
>>           usb_endpoint_xfer_isoc(&ep->desc)) {
>> +        if ((xhci->quirks & XHCI_LIMIT_ENDPOINT_INTERVAL_9) &&
>> +            usb_endpoint_xfer_int(&ep->desc) &&
>> +            interval >= 9) {
>> +            interval = 8;
> 
> Commit message describes this as an issue triggered by High-Speed Isoc In
> endpoints that have interval larger than 32ms.
>
> This code limits interval to 32ms for Interrupt endpoints (any speed),
> should it be isoc instead?

The affected transfer is ISOC. However, due to INT EP service interval 
of 64ms causing the ISO EP to be skipped, the WA is to reduce the INT EP 
service to be less than 64ms (32ms).
> 
> Are Full-/Low-speed devices really also affected?
> 
No, Full-/Low-speed devices are not affected. Thanks for reviewing, I'll 
re-spin the patch adding this change.

> Thanks
> Mathias
>