Patchwork [2/2] support for UHCI suspend / remote wake up

login
register
mail settings
Submitter Marcelo Tosatti
Date Nov. 25, 2010, 5:04 p.m.
Message ID <20101125170602.676792336@redhat.com>
Download mbox | patch
Permalink /patch/73100/
State New
Headers show

Comments

Marcelo Tosatti - Nov. 25, 2010, 5:04 p.m.
This patch enables USB UHCI global suspend/resume feature. The OS will
stop the HC once all ports are suspended. If there is activity on the
port(s), an interrupt signalling remote wakeup will be triggered.

To enable autosuspend for the USB tablet on Linux guests:

echo auto > /sys/devices/pci0000:00/0000:00:01.2/usb1/1-1/power/level

It reduces CPU consumption of an idle FC12 guest from 2.7% to 0.3%.

Signed-off-by: Marcelo Tosatti <mtosatti@redhat.com>
Gerd Hoffmann - Dec. 1, 2010, 3:12 p.m.
On 11/25/10 18:04, Marcelo Tosatti wrote:
> This patch enables USB UHCI global suspend/resume feature. The OS will
> stop the HC once all ports are suspended. If there is activity on the
> port(s), an interrupt signalling remote wakeup will be triggered.
>
> To enable autosuspend for the USB tablet on Linux guests:
>
> echo auto>  /sys/devices/pci0000:00/0000:00:01.2/usb1/1-1/power/level

Hmm, did you ever got this working sanely?

/me sees bus disconnects in the guest ...

>               port->ctrl&= ~(val&  0x000a);
> +            port->ctrl&= ~(port->ctrl&  0x0040); /* clear port resume detected */
>           }

This chunk looks suspicious ...

I suspect the port suspend/resume emulation isn't complete.

/me goes debugging,
   Gerd
Marcelo Tosatti - Dec. 1, 2010, 4:58 p.m.
On Wed, Dec 01, 2010 at 04:12:14PM +0100, Gerd Hoffmann wrote:
> On 11/25/10 18:04, Marcelo Tosatti wrote:
> >This patch enables USB UHCI global suspend/resume feature. The OS will
> >stop the HC once all ports are suspended. If there is activity on the
> >port(s), an interrupt signalling remote wakeup will be triggered.
> >
> >To enable autosuspend for the USB tablet on Linux guests:
> >
> >echo auto>  /sys/devices/pci0000:00/0000:00:01.2/usb1/1-1/power/level
> 
> Hmm, did you ever got this working sanely?

Yes. Linux and Windows.

> /me sees bus disconnects in the guest ...

I was seeing bus disconnects when not clearing port resume bit properly.

> >              port->ctrl&= ~(val&  0x000a);
> >+            port->ctrl&= ~(port->ctrl&  0x0040); /* clear port resume detected */
> >          }
> 
> This chunk looks suspicious ...
> 
> I suspect the port suspend/resume emulation isn't complete.
> 
> /me goes debugging,
>   Gerd

CONFIG_USB_DEBUG helps.
Gerd Hoffmann - Dec. 1, 2010, 5:17 p.m.
> I was seeing bus disconnects when not clearing port resume bit properly.
>
>>>               port->ctrl&= ~(val&   0x000a);
>>> +            port->ctrl&= ~(port->ctrl&   0x0040); /* clear port resume detected */
>>>           }
>>
>> This chunk looks suspicious ...
>>
>> I suspect the port suspend/resume emulation isn't complete.

The bug is that the port resume bit is masked out from guest writes, so 
the guest hasn't a chance to clear it ...

cheers,
   Gerd

PS: http://cgit.freedesktop.org/spice/qemu/log/?h=usb.1

Patch

Index: qemu-kvm/hw/usb-hid.c
===================================================================
--- qemu-kvm.orig/hw/usb-hid.c
+++ qemu-kvm/hw/usb-hid.c
@@ -412,6 +412,9 @@  static void usb_hid_changed(USBHIDState 
 
     if (hs->datain)
         hs->datain(hs->datain_opaque);
+
+    if (hs->dev.remote_wakeup)
+        usb_remote_wakeup(&hs->dev);
 }
 
 static void usb_mouse_event(void *opaque,
Index: qemu-kvm/hw/usb-uhci.c
===================================================================
--- qemu-kvm.orig/hw/usb-uhci.c
+++ qemu-kvm/hw/usb-uhci.c
@@ -59,6 +59,7 @@ 
 
 #define UHCI_PORT_RESET (1 << 9)
 #define UHCI_PORT_LSDA  (1 << 8)
+#define UHCI_PORT_RD    (1 << 6)
 #define UHCI_PORT_ENC   (1 << 3)
 #define UHCI_PORT_EN    (1 << 2)
 #define UHCI_PORT_CSC   (1 << 1)
@@ -501,6 +502,7 @@  static void uhci_ioport_writew(void *opa
             port->ctrl = (port->ctrl & 0x01fb) | (val & ~0x01fb);
             /* some bits are reset when a '1' is written to them */
             port->ctrl &= ~(val & 0x000a);
+            port->ctrl &= ~(port->ctrl & 0x0040); /* clear port resume detected */
         }
         break;
     }
@@ -593,6 +595,41 @@  static void uhci_resume (void *opaque)
     }
 }
 
+static UHCIPort *find_port(UHCIState *s, USBDevice *d)
+{
+    int i;
+
+    for (i = 0; i < NB_PORTS; i++) {
+        UHCIPort *port = &s->ports[i];
+
+        if (d == port->port.dev) {
+            return port;
+        }
+    }
+
+    return NULL;
+}
+
+static void uhci_event(USBBus *bus, USBDevice *dev)
+{
+    UHCIState *s = container_of(bus, UHCIState, bus);
+
+    if (s->cmd & UHCI_CMD_EGSM) {
+        UHCIPort *port = find_port(s, dev);
+
+        if (!port) {
+            return;
+        }
+
+        if (port->ctrl & UHCI_PORT_RD) {
+            return;
+        }
+
+        port->ctrl |= UHCI_PORT_RD;
+        uhci_resume(s);
+    }
+}
+
 static void uhci_attach(USBPort *port1, USBDevice *dev)
 {
     UHCIState *s = port1->opaque;
@@ -1101,6 +1138,10 @@  static void uhci_map(PCIDevice *pci_dev,
     register_ioport_read(addr, 32, 1, uhci_ioport_readb, s);
 }
 
+static USBBusOps uhci_bus_ops = {
+    .remote_wakeup_cb = uhci_event,
+};
+
 static int usb_uhci_common_initfn(UHCIState *s)
 {
     uint8_t *pci_conf = s->dev.config;
@@ -1113,7 +1154,7 @@  static int usb_uhci_common_initfn(UHCISt
     pci_conf[PCI_INTERRUPT_PIN] = 4; // interrupt pin 3
     pci_conf[0x60] = 0x10; // release number
 
-    usb_bus_new(&s->bus, &s->dev.qdev, NULL);
+    usb_bus_new(&s->bus, &s->dev.qdev, &uhci_bus_ops);
     for(i = 0; i < NB_PORTS; i++) {
         usb_register_port(&s->bus, &s->ports[i].port, s, i, uhci_attach);
     }