Doug,
On 01/29/2016 10:20 AM, Douglas Anderson wrote:
> When setting up ISO and INT transfers dwc2 needs to specify whether the
> transfer is for an even or an odd frame (or microframe if the controller
> is running in high speed mode).
>
> The controller appears to use this as a simple way to figure out if a
> transfer should happen right away (in the current microframe) or should
> happen at the start of the next microframe. Said another way:
>
> - If you set "odd" and the current frame number is odd it appears that
> the controller will try to transfer right away. Same thing if you set
> "even" and the current frame number is even.
> - If the oddness you set and the oddness of the frame number are
> _different_, the transfer will be delayed until the frame number
> changes.
>
> As I understand it, the above technique allows you to plan ahead of time
> where possible by always working on the next frame. ...but it still
> allows you to properly respond immediately to things that happened in
> the previous frame.
>
> The old dwc2_hc_set_even_odd_frame() didn't really handle this concept.
> It always looked at the frame number and setup the transfer to happen in
> the next frame. In some cases that meant that certain transactions
> would be transferred in the wrong frame.
>
> We'll try our best to set the even / odd to do the transfer in the
> scheduled frame. If that fails then we'll do an ugly "schedule ASAP".
> We'll also modify the scheduler code to handle this and not try to
> schedule a second transfer for the same frame.
>
> Note that this change relies on the work to redo the microframe
> scheduler. It can work atop ("usb: dwc2: host: Manage frame nums better
> in scheduler") but it works even better after ("usb: dwc2: host: Totally
> redo the microframe scheduler").
>
> With this change my stressful USB test (USB webcam + USB audio +
> keyboards) has less audio crackling than before.
Seems this really help for your case?
Do you check if the transfer can happen right in the current frame? I
know it's
quite difficult to check it, but this changes what I know for the dwc core
schedule the transaction.
In dwc_otgbook, Interrupt OUT Transactions(also similar for Int IN, Iso
IN/OUT)
in DMA Mode, the normal Interrupt OUT operation says:
The DWC_otg host attempts to send out the OUT token in the beginning of next
odd frame/microframe.
So I'm confuse about if the dwc core can do the transaction at the same
frame
of host channel initialized or not.
Thanks,
- Kever
> Signed-off-by: Douglas Anderson <dianders at chromium.org>
> Tested-by: Heiko Stuebner <heiko at sntech.de>
> Tested-by: Stefan Wahren <stefan.wahren at i2se.com>
> ---
> Changes in v6:
> - Add Heiko's Tested-by.
> - Add Stefan's Tested-by.
>
> Changes in v5: None
> Changes in v4:
> - Properly set even/odd frame new for v4.
>
> Changes in v3: None
> Changes in v2: None
>
> drivers/usb/dwc2/core.c | 92 +++++++++++++++++++++++++++++++++++++++++++-
> drivers/usb/dwc2/hcd_queue.c | 11 +++++-
> 2 files changed, 100 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/usb/dwc2/core.c b/drivers/usb/dwc2/core.c
> index a5db20f12ee4..c143f26bd9d9 100644
> --- a/drivers/usb/dwc2/core.c
> +++ b/drivers/usb/dwc2/core.c
> @@ -1703,9 +1703,97 @@ static void dwc2_hc_set_even_odd_frame(struct dwc2_hsotg *hsotg,
> {
> if (chan->ep_type == USB_ENDPOINT_XFER_INT ||
> chan->ep_type == USB_ENDPOINT_XFER_ISOC) {
> - /* 1 if _next_ frame is odd, 0 if it's even */
> - if (!(dwc2_hcd_get_frame_number(hsotg) & 0x1))
> + int host_speed;
> + int xfer_ns;
> + int xfer_us;
> + int bytes_in_fifo;
> + u16 fifo_space;
> + u16 frame_number;
> + u16 wire_frame;
> +
> + /*
> + * Try to figure out if we're an even or odd frame. If we set
> + * even and the current frame number is even the the transfer
> + * will happen immediately. Similar if both are odd. If one is
> + * even and the other is odd then the transfer will happen when
> + * the frame number ticks.
> + *
> + * There's a bit of a balancing act to get this right.
> + * Sometimes we may want to send data in the current frame (AK
> + * right away). We might want to do this if the frame number
> + * _just_ ticked, but we might also want to do this in order
> + * to continue a split transaction that happened late in a
> + * microframe (so we didn't know to queue the next transfer
> + * until the frame number had ticked). The problem is that we
> + * need a lot of knowledge to know if there's actually still
> + * time to send things or if it would be better to wait until
> + * the next frame.
> + *
> + * We can look at how much time is left in the current frame
> + * and make a guess about whether we'll have time to transfer.
> + * We'll do that.
> + */
> +
> + /* Get speed host is running at */
> + host_speed = (chan->speed != USB_SPEED_HIGH &&
> + !chan->do_split) ? chan->speed : USB_SPEED_HIGH;
> +
> + /* See how many bytes are in the periodic FIFO right now */
> + fifo_space = (dwc2_readl(hsotg->regs + HPTXSTS) &
> + TXSTS_FSPCAVAIL_MASK) >> TXSTS_FSPCAVAIL_SHIFT;
> + bytes_in_fifo = sizeof(u32) *
> + (hsotg->core_params->host_perio_tx_fifo_size -
> + fifo_space);
> +
> + /*
> + * Roughly estimate bus time for everything in the periodic
> + * queue + our new transfer. This is "rough" because we're
> + * using a function that makes takes into account IN/OUT
> + * and INT/ISO and we're just slamming in one value for all
> + * transfers. This should be an over-estimate and that should
> + * be OK, but we can probably tighten it.
> + */
> + xfer_ns = usb_calc_bus_time(host_speed, false, false,
> + chan->xfer_len + bytes_in_fifo);
> + xfer_us = NS_TO_US(xfer_ns);
> +
> + /* See what frame number we'll be at by the time we finish */
> + frame_number = dwc2_hcd_get_future_frame_number(hsotg, xfer_us);
> +
> + /* This is when we were scheduled to be on the wire */
> + wire_frame = dwc2_frame_num_inc(chan->qh->next_active_frame, 1);
> +
> + /*
> + * If we'd finish _after_ the frame we're scheduled in then
> + * it's hopeless. Just schedule right away and hope for the
> + * best. Note that it _might_ be wise to call back into the
> + * scheduler to pick a better frame, but this is better than
> + * nothing.
> + */
> + if (dwc2_frame_num_gt(frame_number, wire_frame)) {
> + dwc2_sch_vdbg(hsotg,
> + "QH=%p EO MISS fr=%04x=>%04x (%+d)\n",
> + chan->qh, wire_frame, frame_number,
> + dwc2_frame_num_dec(frame_number,
> + wire_frame));
> + wire_frame = frame_number;
> +
> + /*
> + * We picked a different frame number; communicate this
> + * back to the scheduler so it doesn't try to schedule
> + * another in the same frame.
> + *
> + * Remember that next_active_frame is 1 before the wire
> + * frame.
> + */
> + chan->qh->next_active_frame =
> + dwc2_frame_num_dec(frame_number, 1);
> + }
> +
> + if (wire_frame & 1)
> *hcchar |= HCCHAR_ODDFRM;
> + else
> + *hcchar &= ~HCCHAR_ODDFRM;
> }
> }
>
> diff --git a/drivers/usb/dwc2/hcd_queue.c b/drivers/usb/dwc2/hcd_queue.c
> index 3abb34a5fc5b..5f909747b5a4 100644
> --- a/drivers/usb/dwc2/hcd_queue.c
> +++ b/drivers/usb/dwc2/hcd_queue.c
> @@ -985,6 +985,14 @@ static int dwc2_next_periodic_start(struct dwc2_hsotg *hsotg,
> * and next_active_frame are always 1 frame before we want things
> * to be active and we assume we can still get scheduled in the
> * current frame number.
> + * - It's possible for start_active_frame (now incremented) to be
> + * next_active_frame if we got an EO MISS (even_odd miss) which
> + * basically means that we detected there wasn't enough time for
> + * the last packet and dwc2_hc_set_even_odd_frame() rescheduled us
> + * at the last second. We want to make sure we don't schedule
> + * another transfer for the same frame. My test webcam doesn't seem
> + * terribly upset by missing a transfer but really doesn't like when
> + * we do two transfers in the same frame.
> * - Some misses are expected. Specifically, in order to work
> * perfectly dwc2 really needs quite spectacular interrupt latency
> * requirements. It needs to be able to handle its interrupts
> @@ -995,7 +1003,8 @@ static int dwc2_next_periodic_start(struct dwc2_hsotg *hsotg,
> * guarantee that a system will have interrupt latency < 125 us, so
> * we have to be robust to some misses.
> */
> - if (dwc2_frame_num_gt(prev_frame_number, qh->start_active_frame)) {
> + if (qh->start_active_frame == qh->next_active_frame ||
> + dwc2_frame_num_gt(prev_frame_number, qh->start_active_frame)) {
> u16 ideal_start = qh->start_active_frame;
>
> /* Adjust interval as per gcd with plan length. */
