Re: [RFC 1/1] usb: host: xhci-plat: add enable XHCI-AVOID-BEI quirk by dts

[Mathias Nyman]

On 11.6.2024 17.45, joswang wrote:
> On Tue, Jun 11, 2024 at 6:01 PM Mathias Nyman
> <mathias.nyman@xxxxxxxxxxxxxxx> wrote:
> > On 6.6.2024 17.08, joswang wrote:
> > > On Wed, Jun 5, 2024 at 6:31 PM Mathias Nyman
> > > <mathias.nyman@xxxxxxxxxxxxxxx> wrote:
> > ...
> > > > I was thinking of turning XHCI_AVOID_BEI behavior into the new default, so no
> > > > quirk flag would be needed:
> > > >
> > > > Currently without the quirk flag:
> > > >
> > > > - ISOC TRBs trigger interrupt if TRB is the last in the TD
> > > >
> > > > Currently with XHCI_AVOID_BEI quirk flag:
> > > >
> > > > - ISOC TRBs trigger interrupt if TRB is the last in the TD
> > > > - Interrupt is additionally triggered every 32 isoc TRB (initially).
> > > > - if more than 128 events are processed in one interrupt then the
> > > >      32 is halved, and we trigger an interrupts every 16th isoc TRB, and so
> > > >      on, 16 -> 8...
> > > >
> > > > I would remove the quirk flag, and make all controllers interrupt
> > > > behave as if it was set. i.e. interrupt at least every 32 isoc TRB
> > >
> > > Thank you for your detailed analysis.
> > > Excuse me, I have a question, do you mean to set "Currently with
> > > XHCI_AVOID_BEI quirk flag" as the default behavior?
> >
> > Yes, unless it causes some issues or there are strong objections
> >
> > > > Is there an actual real world case where interrupting every 32nd ISOC TRB is
> > > > too often?
> > >
> > > I mean that if the XHCI_AVOID_BEI quirk flag is set, an interrupt will
> > > be triggered every 8 TRBs, which makes the interrupts seem to be quite
> > > frequent.
> > > Thanks
> > > Jos
> >
> > It should start with interrupting every 32nd isoc TD, not 8th
> >
> > #define AVOID_BEI_INTERVAL_MAX   32
> >
> > ir->isoc_bei_interval = AVOID_BEI_INTERVAL_MAX;
> >
> > Thanks
> > Mathias
>
> As you mentioned，if more than 128 events are processed in one
> interrupt then the 32 is halved, and we trigger an interrupts every
> 16th isoc TRB, and so on, 16 -> 8...
> xhci_handle_events()
>      ......
>      if (event_loop++ > TRBS_PER_SEGMENT / 2) {
>           if (ir->isoc_bei_interval > AVOID_BEI_INTERVAL_MIN)
>                ir->isoc_bei_interval = ir->isoc_bei_interval / 2;
>      }
>      .......

Yes, and to me this seems reasonable .
If there are over 128 events per interrupt then we should interrupt more often.

Would this work as the default for the Synopsys hosts you are working with?

Thanks
Mathias