Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1322821

Re: [PATCH v6 18/22] usb: dwc2: host: Schedule periodic right away if it's time

Show all headers | View raw

Kever,

On Sun, Jan 31, 2016 at 7:32 PM, Kever Yang <kever.yang@rock-chips.com> wrote:
> Doug,
>
>
> On 02/01/2016 06:09 AM, Doug Anderson wrote:
>>
>> Kever,
>>
>> On Sun, Jan 31, 2016 at 1:36 AM, Kever Yang <kever.yang@rock-chips.com>
>> wrote:
>>>
>>> Doug,
>>>
>>>
>>> On 01/29/2016 10:20 AM, Douglas Anderson wrote:
>>>>
>>>> In dwc2_hcd_qh_deactivate() we will put some things on the
>>>> periodic_sched_ready list.  These things won't be taken off the ready
>>>> list until the next SOF, which might be a little late.  Let's put them
>>>> on right away.
>>>>
>>>> Signed-off-by: Douglas Anderson <dianders@chromium.org>
>>>> Tested-by: Heiko Stuebner <heiko@sntech.de>
>>>> Tested-by: Stefan Wahren <stefan.wahren@i2se.com>
>>>> ---
>>>> Changes in v6:
>>>> - Add Heiko's Tested-by.
>>>> - Add Stefan's Tested-by.
>>>>
>>>> Changes in v5: None
>>>> Changes in v4:
>>>> - Schedule periodic right away if it's time new for v4.
>>>>
>>>> Changes in v3: None
>>>> Changes in v2: None
>>>>
>>>>    drivers/usb/dwc2/hcd_queue.c | 18 ++++++++++++++++--
>>>>    1 file changed, 16 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/usb/dwc2/hcd_queue.c b/drivers/usb/dwc2/hcd_queue.c
>>>> index 9b3c435339ee..3abb34a5fc5b 100644
>>>> --- a/drivers/usb/dwc2/hcd_queue.c
>>>> +++ b/drivers/usb/dwc2/hcd_queue.c
>>>> @@ -1080,12 +1080,26 @@ void dwc2_hcd_qh_deactivate(struct dwc2_hsotg
>>>> *hsotg, struct dwc2_qh *qh,
>>>>           * Note: we purposely use the frame_number from the "hsotg"
>>>> structure
>>>>           * since we know SOF interrupt will handle future frames.
>>>>           */
>>>> -       if (dwc2_frame_num_le(qh->next_active_frame,
>>>> hsotg->frame_number))
>>>> +       if (dwc2_frame_num_le(qh->next_active_frame,
>>>> hsotg->frame_number))
>>>> {
>>>> +               enum dwc2_transaction_type tr_type;
>>>> +
>>>> +               /*
>>>> +                * We're bypassing the SOF handler which is normally
>>>> what
>>>> puts
>>>> +                * us on the ready list because we're in a hurry and
>>>> need
>>>> to
>>>> +                * try to catch up.
>>>> +                */
>>>> +               dwc2_sch_vdbg(hsotg, "QH=%p IMM ready fn=%04x,
>>>> nxt=%04x\n",
>>>> +                             qh, frame_number, qh->next_active_frame);
>>>>                  list_move_tail(&qh->qh_list_entry,
>>>>                                 &hsotg->periodic_sched_ready);
>>>> -       else
>>>> +
>>>> +               tr_type = dwc2_hcd_select_transactions(hsotg);
>>>
>>> Do we need to add select_transactions call here? If we get into this
>>> function in interrupt
>>> and once we put the qh in ready queue, the qh can be handled in this
>>> frame
>>> again by the
>>> later function call of dwc_hcd_select_transactions, so what we need to to
>>> here is put
>>> it in ready list instead of inactive queue, and wait for the schedule.
>>
>> I'm not sure I understand.  Can you restate?
>>
>>
>> I'll try to explain more in the meantime...
>>
>> Both before and after my change, this function would place something
>> on the ready queue if the next_active_frame <= the frame number as of
>> last SOF interrupt (aka hsotg->frame_number).  Otherwise it goes on
>> the inactive queue.  Assuming that the previous change ("usb: dwc2:
>> host: Manage frame nums better in scheduler") worked properly then
>> next_active_frame shouldn't be less than (hsotg->frame_number - 1).
>> Remember that next_active_frame is always 1 before the wire frame, so
>> if "next_active_frame == hsotg->frame_number - 1" it means that we
>> need to get the transfer on the wire _right away_.  If
>> "next_active_frame == hsotg->frame_number" the transfer doesn't need
>> to go on the wire right away, but since dwc2 can be prepped one frame
>> in advance it doesn't hurt to give it to the hardware right away if
>> there's space.
>>
>> As I understand it, if we stick something on the ready queue it won't
>> generally get looked at until the next SOF interrupt.  That means
>> we'll be too late if "next_active_frame == hsotg->frame_number - 1"
>> and we'll possibly be too late (depending on interrupt latency) if
>> "next_active_frame == hsotg->frame_number"
>>
> I understand this patch and agree with your point of schedule the
> periodic right away instead of at least next frame.
> My point is, there are only two call to dwc2_hcd_qh_deactivate(), from
> dwc2_hcd_urb_dequeue() and dwc2_release_channel(), we don't need
> to do the schedule for dequeue, and there is one
> dwc2_hcd_select_transactions() call at the end of dwc2_release_channel(),
> maybe we don't need another dwc2_hcd_select_transactions() here.
>
> I think the duration from this point to the function call of
> dwc2_hcd_select_transactions()
> in dwc2_release_channel() will be the main factor for us to decide if
> we need to add a function call of  dwc2_hcd_select_transactions() here.

Oh, now I get what you're saying!

A) You've got dwc2_release_channel() -> dwc2_deactivate_qh() ->
dwc2_hcd_qh_deactivate()
...and always in that case we'll do a select / queue, so we don't need it there.

B) You've got dwc2_hcd_urb_dequeue() -> dwc2_hcd_qh_deactivate()

...but why don't we need it for dwc2_hcd_urb_dequeue()?  Yes, you're
not continuing a split so timing isn't quite as urgent, but you still
might have an INT or ISOC packet that's scheduled with an interval of
1.  We still might want to schedule right away if there are remaining
QTDs, right?

-Doug

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH v6 0/22] usb: dwc2: host: Fix and speed up all the stuff, especially with splits Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 09/22] usb: dwc2: host: Giveback URB in tasklet context Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 06/22] usb: dwc2: host: fix split transfer schedule sequence Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 15/22] usb: dwc2: host: Split code out to make dwc2_do_reserve() Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 22/22] usb: dwc2: host: If using uframe scheduler, end splits better Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 11/22] usb: dwc2: host: There's not really a TT for the root hub Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
    Re: [PATCH v6 11/22] usb: dwc2: host: There's not really a TT for  the root hub Kever Yang <kever.yang@rock-chips.com> - 2016-01-31 10:30 +0100
  [PATCH v6 02/22] usb: dwc2: host: Get aligned DMA in a more supported way Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 10/22] usb: dwc2: host: Properly set the HFIR Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
    Re: [PATCH v6 10/22] usb: dwc2: host: Properly set the HFIR Kever Yang <kever.yang@rock-chips.com> - 2016-01-31 10:30 +0100
      Re: [PATCH v6 10/22] usb: dwc2: host: Properly set the HFIR Doug Anderson <dianders@chromium.org> - 2016-01-31 23:20 +0100
  [PATCH v6 13/22] usb: dwc2: host: Rename some fields in struct dwc2_qh Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 04/22] usb: dwc2: host: Avoid use of chan->qh after qh freed Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 16/22] usb: dwc2: host: Add scheduler logging for missed SOFs Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 01/22] usb: dwc2: rockchip: Make the max_transfer_size automatic Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 17/22] usb: dwc2: host: Manage frame nums better in scheduler Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
    Re: [PATCH v6 17/22] usb: dwc2: host: Manage frame nums better in scheduler Doug Anderson <dianders@chromium.org> - 2016-02-03 21:40 +0100
  [PATCH v6 05/22] usb: dwc2: host: Always add to the tail of queues Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 19/22] usb: dwc2: host: Add dwc2_hcd_get_future_frame_number() call Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 03/22] usb: dwc2: host: Set host_rx_fifo_size to 525 for rk3066 Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 07/22] usb: dwc2: host: Add scheduler tracing Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 08/22] usb: dwc2: host: Add a delay before releasing periodic bandwidth Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 20/22] usb: dwc2: host: Properly set even/odd frame Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
    Re: [PATCH v6 20/22] usb: dwc2: host: Properly set even/odd frame Kever Yang <kever.yang@rock-chips.com> - 2016-02-02 08:50 +0100
      Re: [PATCH v6 20/22] usb: dwc2: host: Properly set even/odd frame Doug Anderson <dianders@chromium.org> - 2016-02-02 23:50 +0100
        Re: [PATCH v6 20/22] usb: dwc2: host: Properly set even/odd frame Kever Yang <kever.yang@rock-chips.com> - 2016-02-03 08:50 +0100
  [PATCH v6 18/22] usb: dwc2: host: Schedule periodic right away if it's time Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
    Re: [PATCH v6 18/22] usb: dwc2: host: Schedule periodic right away  if it's time Kever Yang <kever.yang@rock-chips.com> - 2016-01-31 10:40 +0100
      Re: [PATCH v6 18/22] usb: dwc2: host: Schedule periodic right away if  it's time Doug Anderson <dianders@chromium.org> - 2016-01-31 23:10 +0100
        Re: [PATCH v6 18/22] usb: dwc2: host: Schedule periodic right away  if it's time Kever Yang <kever.yang@rock-chips.com> - 2016-02-01 04:40 +0100
          Re: [PATCH v6 18/22] usb: dwc2: host: Schedule periodic right away if  it's time Doug Anderson <dianders@chromium.org> - 2016-02-01 05:40 +0100
            Re: [PATCH v6 18/22] usb: dwc2: host: Schedule periodic right away if  it's time Doug Anderson <dianders@chromium.org> - 2016-02-02 01:40 +0100
              Re: [PATCH v6 18/22] usb: dwc2: host: Schedule periodic right away  if it's time Kever Yang <kever.yang@rock-chips.com> - 2016-02-02 08:10 +0100
                Re: [PATCH v6 18/22] usb: dwc2: host: Schedule periodic right away if  it's time Doug Anderson <dianders@chromium.org> - 2016-02-03 00:40 +0100
  [PATCH v6 14/22] usb: dwc2: host: Reorder things in hcd_queue.c Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  [PATCH v6 12/22] usb: dwc2: host: Use periodic interrupt even with DMA Douglas Anderson <dianders@chromium.org> - 2016-01-29 03:30 +0100
  Re: [PATCH v6 0/22] usb: dwc2: host: Fix and speed up all the  stuff, especially with splits John Youn <John.Youn@synopsys.com> - 2016-02-03 01:00 +0100
    Re: [PATCH v6 0/22] usb: dwc2: host: Fix and speed up all the stuff,  especially with splits Doug Anderson <dianders@chromium.org> - 2016-02-03 19:30 +0100

csiph-web