Linux USB
 help / color / mirror / Atom feed
From: Michal Pecio <michal.pecio@gmail.com>
To: Dylan Robinson <dylan_robinson@motu.com>
Cc: Alan Stern <stern@rowland.harvard.edu>,
	Mathias Nyman <mathias.nyman@linux.intel.com>,
	linux-usb@vger.kernel.org, mathias.nyman@intel.com
Subject: Re: [RFT PATCHv3 1/3] xhci: fix frame id calculation and checks for isoc URBs
Date: Fri, 7 Aug 2026 23:29:52 +0200	[thread overview]
Message-ID: <20260807232952.015d380b.michal.pecio@gmail.com> (raw)
In-Reply-To: <CA+Df+jdCbY23h+Bq+ehKbJWMvTtwQDAOLk+hw=Y8Ne9zhJ_GOw@mail.gmail.com>

On Fri, 7 Aug 2026 13:30:40 -0400, Dylan Robinson wrote:
> On Thu, Aug 6, 2026 at 8:07 PM Mathias Nyman
> <mathias.nyman@linux.intel.com> wrote:
> > With the hardware ep ctx check we will set the frame_id of the late
> > URB to point to the past, to the real time it was supposed to be
> > played back. xHC hardware will then trigger a 'missed service
> > event' for that TD as it was incapable of playing it at the correct
> > time, and then fast forward with MSE until it finds a TD with a
> > frame_id it can play, and play at the correct timeslot. Here we
> > lost the late data but playback stays in sync, as isoc transfer are
> > intended to work.  
> 
> I want to make sure I'm following along correctly. This is only true
> for host controllers that support CFC. On controllers without CFC,
> continuations are always submitted with SIA, so the controller can't
> reject expired TDs based on their Frame ID.

Yes, we are talking about the patch fixing CFC logic. You are right,
and you could only be more right with "because" in place of "so";
I tried treating old chips as if they had CFC, outcomes were weird.

> On Fri, Aug 7, 2026 at 12:48 PM Alan Stern
> <stern@rowland.harvard.edu> wrote:
> > The point under discussion is really about what happens when an
> > underrun occurs by accident -- for example, if the system is
> > temporarily overloaded.  In this situation there are two logical
> > choices of action if the class driver wants to keep the queue
> > running: Keep the alignment of iso_packet_descriptors and
> > (micro)frames as it was, which means assigning some new packets to
> > times in the past, or forget about the old alignment and start
> > fresh, which means assigning the next packet to the next available
> > future time slot.  The URB_ISO_ASAP flag is how the driver
> > communicates its choice to the HCD.  
> 
> How is the first choice realized when the host controller doesn't
> support CFC?

Currently it isn't, everything goes "ASAP" on xHCI 1.0 hardware and
underrun creates a gap. Another patch that Mathias prepared for 7.3
would align the first URB of each new stream, but nothing more.

We get the Ring Underrun event, so we know it happened. Drivers don't
know and they seem to have no trivial and robust way to detect it. To
them, URBs are completing, submissions are succeeding, all looks fine.

Potential solutions I could think of:

1. Somehow let drivers know that a fresh start is their only choice
   (some completion status, bogus start_frame, usb_submit_urb() error,
   callback, ...) and have them ready to deal with this (hard part).

2. Try to resync in xhci-hcd: quickly return newly submitted frames
   as -EXDEV until one is far enough into the future to reliably meet
   the IST and start a new "isoc data flow" from HW's point of view.
   
   Complicated by the fact that new URBs may be submitted after the
   underrun occurs but before our IRQ handler learns about it. Those
   are doomed to execute with some lag (unless unlinked). We could
   mark them -EXDEV or some such so that drivers don't trust them.

Not sure if this is desired by drivers, particularly the potential
inefficiency of the "complicated" part. I wouldn't be shocked if audio
(even specifically pro audio) was the only application in the world
that truly cares about exact scheduling.

I wonder if it should be recommended that drivers which don't care
always set URB_ISO_ASAP to opt out of any such insanity just in case?
I know that uvcvideo uses ASAP, for example.

Regards,
Michal

  reply	other threads:[~2026-08-07 21:30 UTC|newest]

Thread overview: 74+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-11-04 21:25 [Bug 220748] New: usb: xhci_queue_isoc_tx_prepare ignore start_frame and always assumes URB_ISO_ASAP is set bugzilla-daemon
2025-11-05  8:40 ` [Bug 220748] " bugzilla-daemon
2025-11-05  8:48 ` bugzilla-daemon
2025-11-05  8:53 ` bugzilla-daemon
2025-11-05  9:06 ` bugzilla-daemon
2025-11-05 15:47 ` bugzilla-daemon
2025-11-05 18:28 ` bugzilla-daemon
2025-11-05 18:30 ` bugzilla-daemon
2025-11-05 20:16 ` bugzilla-daemon
2025-11-05 21:18 ` bugzilla-daemon
2025-11-06  3:52 ` bugzilla-daemon
2025-11-06  8:27 ` bugzilla-daemon
2025-11-06  8:35 ` bugzilla-daemon
2025-11-06 15:03 ` bugzilla-daemon
2025-11-08 10:33 ` bugzilla-daemon
2025-11-08 16:09 ` bugzilla-daemon
2025-11-10 10:23 ` bugzilla-daemon
2025-11-10 10:42 ` bugzilla-daemon
2026-05-04 23:53 ` bugzilla-daemon
2026-05-04 23:54 ` bugzilla-daemon
2026-05-05  1:14 ` bugzilla-daemon
2026-05-05  9:59 ` bugzilla-daemon
2026-05-05 17:09 ` bugzilla-daemon
2026-05-05 17:10 ` bugzilla-daemon
2026-05-05 17:10 ` bugzilla-daemon
2026-05-05 17:13 ` bugzilla-daemon
2026-05-06 13:32 ` bugzilla-daemon
2026-05-06 15:03 ` bugzilla-daemon
2026-05-07  2:38   ` Alan Stern
2026-05-07 16:17     ` Dylan Robinson
2026-05-07 17:24       ` Alan Stern
2026-05-07 21:16         ` Dylan Robinson
2026-05-08  3:02           ` Alan Stern
2026-05-08 17:20             ` Dylan Robinson
2026-05-09  1:25               ` Alan Stern
2026-05-09 22:12               ` Michal Pecio
2026-05-10 12:39                 ` Dylan Robinson
2026-05-11 19:21                   ` [RFT PATCH] xhci: fix frame id calculation for isoc transfer Mathias Nyman
2026-05-11 19:36                     ` Mathias Nyman
2026-05-12  9:08                     ` Michal Pecio
2026-05-13 14:30                       ` Mathias Nyman
2026-05-13 14:35                         ` [RFT PATCHv2 1/2] xhci: fix frame id calculation and checks for isoc URBs Mathias Nyman
2026-05-13 14:35                           ` [RFT PATCHv2 2/2] xhci: Set frame ID field of isoc TRB when starting an isoch stream Mathias Nyman
2026-05-14 21:16                         ` [RFT PATCH] xhci: fix frame id calculation for isoc transfer Dylan Robinson
2026-05-14 22:10                           ` Dylan Robinson
2026-05-15  4:32                           ` Michal Pecio
2026-05-15 18:13                             ` Dylan Robinson
2026-05-18  7:13                               ` Michal Pecio
2026-05-21 14:20                                 ` Dylan Robinson
2026-05-21 15:24                                   ` Mathias Nyman
2026-05-21 15:27                                     ` [RFT PATCHv3 1/3] xhci: fix frame id calculation and checks for isoc URBs Mathias Nyman
2026-05-21 15:27                                       ` [RFT PATCHv3 2/3] xhci: Set frame ID field of isoc TRB when starting an isoch stream Mathias Nyman
2026-05-21 15:27                                       ` [RFT PATCHv3 3/3] xhci: tune urb->start_frame in ring overrun and underrun cases Mathias Nyman
2026-05-26  0:49                                         ` Dylan Robinson
2026-05-26 12:15                                           ` Michal Pecio
2026-05-27 20:20                                             ` Dylan Robinson
2026-08-06  8:17                                       ` [RFT PATCHv3 1/3] xhci: fix frame id calculation and checks for isoc URBs Michal Pecio
2026-08-06 13:25                                         ` Mathias Nyman
2026-08-06 22:01                                           ` Michal Pecio
2026-08-07  0:07                                             ` Mathias Nyman
2026-08-07  8:26                                               ` Michal Pecio
2026-08-07 11:10                                                 ` Mathias Nyman
2026-08-07 16:48                                                   ` Alan Stern
2026-08-07 17:30                                                     ` Dylan Robinson
2026-08-07 21:29                                                       ` Michal Pecio [this message]
2026-08-10 14:12                                                         ` Mathias Nyman
2026-05-07 21:54       ` [Bug 220748] usb: xhci_queue_isoc_tx_prepare ignore start_frame and always assumes URB_ISO_ASAP is set Michal Pecio
2026-05-08  3:09         ` Alan Stern
2026-05-08  9:41           ` Michal Pecio
2026-05-08 14:54             ` Alan Stern
2026-05-08 21:39         ` Dylan Robinson
2026-05-09 11:10           ` Michal Pecio
2026-05-09 20:18             ` Dylan Robinson
2026-05-11 19:15 ` bugzilla-daemon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260807232952.015d380b.michal.pecio@gmail.com \
    --to=michal.pecio@gmail.com \
    --cc=dylan_robinson@motu.com \
    --cc=linux-usb@vger.kernel.org \
    --cc=mathias.nyman@intel.com \
    --cc=mathias.nyman@linux.intel.com \
    --cc=stern@rowland.harvard.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox