| Message ID | 1307363962-27223-14-git-send-email-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 06/06/2011 06:39 AM, Gerd Hoffmann wrote: > The state machine doesn't stop in EXECUTING state any more when async > packets are in flight, so the checks are not needed any more and can > be dropped. > > Also kick out the check for the frame timer. As we don't stop & sleep > any more on async packets this is obsolete. > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hw/usb-ehci.c | 32 ++------------------------------ > 1 files changed, 2 insertions(+), 30 deletions(-) > > diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c > index e345a1c..8b1ae3a 100644 > --- a/hw/usb-ehci.c > +++ b/hw/usb-ehci.c > @@ -1437,17 +1437,6 @@ static int ehci_state_fetchentry(EHCIState *ehci, int async) > int again = 0; > uint32_t entry = ehci_get_fetch_addr(ehci, async); > > -#if EHCI_DEBUG == 0 > - if (qemu_get_clock_ns(vm_clock) / 1000 >= ehci->frame_end_usec) { > - if (async) { > - DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state machine\n"); > - goto out; > - } else { > - DPRINTF("FETCHENTRY: WARNING " > - "- frame timer elapsed during periodic\n"); > - } > - } > -#endif > if (entry < 0x1000) { > DPRINTF("fetchentry: entry invalid (0x%08x)\n", entry); > ehci_set_state(ehci, async, EST_ACTIVE); > @@ -1952,12 +1941,6 @@ static void ehci_advance_async_state(EHCIState *ehci) > } > > ehci_set_state(ehci, async, EST_WAITLISTHEAD); > - /* fall through */ > - > - case EST_FETCHENTRY: > - /* fall through */ Why drop this case too? As I recall that is needed for proper execution. David > - > - case EST_EXECUTING: > ehci_advance_state(ehci, async); > break; > > @@ -2010,11 +1993,6 @@ static void ehci_advance_periodic_state(EHCIState *ehci) > ehci_advance_state(ehci, async); > break; > > - case EST_EXECUTING: > - DPRINTF("PERIODIC state adv for executing\n"); > - ehci_advance_state(ehci, async); > - break; > - > default: > /* this should only be due to a developer mistake */ > fprintf(stderr, "ehci: Bad periodic state %d. " > @@ -2063,11 +2041,7 @@ static void ehci_frame_timer(void *opaque) > if (frames - i > 10) { > skipped_frames++; > } else { > - // TODO could this cause periodic frames to get skipped if async > - // active? > - if (ehci_get_state(ehci, 1) != EST_EXECUTING) { > - ehci_advance_periodic_state(ehci); > - } > + ehci_advance_periodic_state(ehci); > } > > ehci->last_run_usec += FRAME_TIMER_USEC; > @@ -2082,9 +2056,7 @@ static void ehci_frame_timer(void *opaque) > /* Async is not inside loop since it executes everything it can once > * called > */ > - if (ehci_get_state(ehci, 0) != EST_EXECUTING) { > - ehci_advance_async_state(ehci); > - } > + ehci_advance_async_state(ehci); > > qemu_mod_timer(ehci->frame_timer, expire_time); > }
On 06/06/2011 07:34 AM, David Ahern wrote: > On 06/06/2011 06:39 AM, Gerd Hoffmann wrote: >> The state machine doesn't stop in EXECUTING state any more when async >> packets are in flight, so the checks are not needed any more and can >> be dropped. >> >> Also kick out the check for the frame timer. As we don't stop & sleep >> any more on async packets this is obsolete. >> >> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> >> --- >> hw/usb-ehci.c | 32 ++------------------------------ >> 1 files changed, 2 insertions(+), 30 deletions(-) >> >> diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c >> index e345a1c..8b1ae3a 100644 >> --- a/hw/usb-ehci.c >> +++ b/hw/usb-ehci.c >> @@ -1437,17 +1437,6 @@ static int ehci_state_fetchentry(EHCIState *ehci, int async) >> int again = 0; >> uint32_t entry = ehci_get_fetch_addr(ehci, async); >> >> -#if EHCI_DEBUG == 0 >> - if (qemu_get_clock_ns(vm_clock) / 1000 >= ehci->frame_end_usec) { >> - if (async) { >> - DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state machine\n"); >> - goto out; >> - } else { >> - DPRINTF("FETCHENTRY: WARNING " >> - "- frame timer elapsed during periodic\n"); >> - } >> - } >> -#endif This check was added to per Section 4 of the EHCI spec -- the HC should not start transactions that will not be completed before the end of the micro-frame. If you remove this what causes the EHCI model to take a breather? David >> if (entry < 0x1000) { >> DPRINTF("fetchentry: entry invalid (0x%08x)\n", entry); >> ehci_set_state(ehci, async, EST_ACTIVE); >> @@ -1952,12 +1941,6 @@ static void ehci_advance_async_state(EHCIState *ehci) >> } >> >> ehci_set_state(ehci, async, EST_WAITLISTHEAD); >> - /* fall through */ >> - >> - case EST_FETCHENTRY: >> - /* fall through */ > > Why drop this case too? As I recall that is needed for proper execution. > > David > >> - >> - case EST_EXECUTING: >> ehci_advance_state(ehci, async); >> break; >> >> @@ -2010,11 +1993,6 @@ static void ehci_advance_periodic_state(EHCIState *ehci) >> ehci_advance_state(ehci, async); >> break; >> >> - case EST_EXECUTING: >> - DPRINTF("PERIODIC state adv for executing\n"); >> - ehci_advance_state(ehci, async); >> - break; >> - >> default: >> /* this should only be due to a developer mistake */ >> fprintf(stderr, "ehci: Bad periodic state %d. " >> @@ -2063,11 +2041,7 @@ static void ehci_frame_timer(void *opaque) >> if (frames - i > 10) { >> skipped_frames++; >> } else { >> - // TODO could this cause periodic frames to get skipped if async >> - // active? >> - if (ehci_get_state(ehci, 1) != EST_EXECUTING) { >> - ehci_advance_periodic_state(ehci); >> - } >> + ehci_advance_periodic_state(ehci); >> } >> >> ehci->last_run_usec += FRAME_TIMER_USEC; >> @@ -2082,9 +2056,7 @@ static void ehci_frame_timer(void *opaque) >> /* Async is not inside loop since it executes everything it can once >> * called >> */ >> - if (ehci_get_state(ehci, 0) != EST_EXECUTING) { >> - ehci_advance_async_state(ehci); >> - } >> + ehci_advance_async_state(ehci); >> >> qemu_mod_timer(ehci->frame_timer, expire_time); >> }
Hi, >>> -#if EHCI_DEBUG == 0 >>> - if (qemu_get_clock_ns(vm_clock) / 1000>= ehci->frame_end_usec) { >>> - if (async) { >>> - DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state machine\n"); >>> - goto out; >>> - } else { >>> - DPRINTF("FETCHENTRY: WARNING " >>> - "- frame timer elapsed during periodic\n"); >>> - } >>> - } >>> -#endif > > This check was added to per Section 4 of the EHCI spec -- the HC should > not start transactions that will not be completed before the end of the > micro-frame. If you remove this what causes the EHCI model to take a > breather? Look at patch #8. That brings a number of state machine changes. One of them is that the async schedule stops as soon as it notices it walks in circles (i.e. sees a QH the second time). >>> - case EST_FETCHENTRY: >>> - /* fall through */ >> >> Why drop this case too? As I recall that is needed for proper execution. The async schedule doesn't pause in fetchentry state any more (also done by patch #8). cheers, Gerd
On 06/06/2011 08:25 AM, Gerd Hoffmann wrote: > > Hi, > >>>> -#if EHCI_DEBUG == 0 >>>> - if (qemu_get_clock_ns(vm_clock) / 1000>= ehci->frame_end_usec) { >>>> - if (async) { >>>> - DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state >>>> machine\n"); >>>> - goto out; >>>> - } else { >>>> - DPRINTF("FETCHENTRY: WARNING " >>>> - "- frame timer elapsed during periodic\n"); >>>> - } >>>> - } >>>> -#endif >> >> This check was added to per Section 4 of the EHCI spec -- the HC should >> not start transactions that will not be completed before the end of the >> micro-frame. If you remove this what causes the EHCI model to take a >> breather? > > Look at patch #8. That brings a number of state machine changes. One > of them is that the async schedule stops as soon as it notices it walks > in circles (i.e. sees a QH the second time). > >>>> - case EST_FETCHENTRY: >>>> - /* fall through */ >>> >>> Why drop this case too? As I recall that is needed for proper execution. > > The async schedule doesn't pause in fetchentry state any more (also done > by patch #8). Ok. see that now. David > > cheers, > Gerd >
diff --git a/hw/usb-ehci.c b/hw/usb-ehci.c index e345a1c..8b1ae3a 100644 --- a/hw/usb-ehci.c +++ b/hw/usb-ehci.c @@ -1437,17 +1437,6 @@ static int ehci_state_fetchentry(EHCIState *ehci, int async) int again = 0; uint32_t entry = ehci_get_fetch_addr(ehci, async); -#if EHCI_DEBUG == 0 - if (qemu_get_clock_ns(vm_clock) / 1000 >= ehci->frame_end_usec) { - if (async) { - DPRINTF("FETCHENTRY: FRAME timer elapsed, exit state machine\n"); - goto out; - } else { - DPRINTF("FETCHENTRY: WARNING " - "- frame timer elapsed during periodic\n"); - } - } -#endif if (entry < 0x1000) { DPRINTF("fetchentry: entry invalid (0x%08x)\n", entry); ehci_set_state(ehci, async, EST_ACTIVE); @@ -1952,12 +1941,6 @@ static void ehci_advance_async_state(EHCIState *ehci) } ehci_set_state(ehci, async, EST_WAITLISTHEAD); - /* fall through */ - - case EST_FETCHENTRY: - /* fall through */ - - case EST_EXECUTING: ehci_advance_state(ehci, async); break; @@ -2010,11 +1993,6 @@ static void ehci_advance_periodic_state(EHCIState *ehci) ehci_advance_state(ehci, async); break; - case EST_EXECUTING: - DPRINTF("PERIODIC state adv for executing\n"); - ehci_advance_state(ehci, async); - break; - default: /* this should only be due to a developer mistake */ fprintf(stderr, "ehci: Bad periodic state %d. " @@ -2063,11 +2041,7 @@ static void ehci_frame_timer(void *opaque) if (frames - i > 10) { skipped_frames++; } else { - // TODO could this cause periodic frames to get skipped if async - // active? - if (ehci_get_state(ehci, 1) != EST_EXECUTING) { - ehci_advance_periodic_state(ehci); - } + ehci_advance_periodic_state(ehci); } ehci->last_run_usec += FRAME_TIMER_USEC; @@ -2082,9 +2056,7 @@ static void ehci_frame_timer(void *opaque) /* Async is not inside loop since it executes everything it can once * called */ - if (ehci_get_state(ehci, 0) != EST_EXECUTING) { - ehci_advance_async_state(ehci); - } + ehci_advance_async_state(ehci); qemu_mod_timer(ehci->frame_timer, expire_time); }
The state machine doesn't stop in EXECUTING state any more when async packets are in flight, so the checks are not needed any more and can be dropped. Also kick out the check for the frame timer. As we don't stop & sleep any more on async packets this is obsolete. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hw/usb-ehci.c | 32 ++------------------------------ 1 files changed, 2 insertions(+), 30 deletions(-)