* Re: [RFC PATCH] usb: optimize acpi companion search for usb port devices [not found] ` <20170524144414.GA13730@kroah.com> @ 2017-05-25 15:24 ` Mathias Nyman 2017-05-31 23:01 ` Rafael J. Wysocki 0 siblings, 1 reply; 3+ messages in thread From: Mathias Nyman @ 2017-05-25 15:24 UTC (permalink / raw) To: Greg KH; +Cc: linux-usb, stern, Rafael J. Wysocki, ACPI Devel Maling List On 24.05.2017 17:44, Greg KH wrote: > On Wed, May 24, 2017 at 04:11:12PM +0300, Mathias Nyman wrote: >> This optimization significantly reduces xhci driver load time. >> >> In ACPI tables the acpi companion port devices are children of >> the hub device. The port devices are identified by their port number >> returned by the ACPI _ADR method. >> _ADR 0 is reserved for the root hub device. >> >> The current implementation to find a acpi companion port device >> loops through all acpi port devices under that parent hub, calling >> their _ADR method each time a new port device is added. >> >> for a xHC controller with 25 ports under its roothub it >> will end up invoking ACPI bytecode 625 times before all ports >> are ready, making it really slow. >> >> The _ADR values are already read and cached earler. So instead of >> running the bytecode again we can check the cached _ADR value first, >> and then fall back to the old way. >> >> As one of the more significant changes, the xhci load time on >> Intel kabylake reduced by 70%, (28ms) from >> initcall xhci_pci_init+0x0/0x49 returned 0 after 39537 usecs >> to >> initcall xhci_pci_init+0x0/0x49 returned 0 after 11270 usecs >> >> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> >> --- >> drivers/usb/core/usb-acpi.c | 26 +++++++++++++++++++++++--- >> 1 file changed, 23 insertions(+), 3 deletions(-) > > Why is this RFC? What's wrong with it as-is? > Last minute doubt, nothing should be wrong, but I started to wonder if there is any particular reason the ACPI part was done the way it was. Or if maybe other drivers could benefit from checking cached _ADR value first as well, and this whole thing should be a part of drivers/acpi/glue.c instead? (adding acpi mailing list, not just Rafael) Thanks -Mathias ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] usb: optimize acpi companion search for usb port devices 2017-05-25 15:24 ` [RFC PATCH] usb: optimize acpi companion search for usb port devices Mathias Nyman @ 2017-05-31 23:01 ` Rafael J. Wysocki 2017-06-02 12:20 ` Mathias Nyman 0 siblings, 1 reply; 3+ messages in thread From: Rafael J. Wysocki @ 2017-05-31 23:01 UTC (permalink / raw) To: Mathias Nyman; +Cc: Greg KH, linux-usb, stern, ACPI Devel Maling List On 5/25/2017 5:24 PM, Mathias Nyman wrote: > On 24.05.2017 17:44, Greg KH wrote: >> On Wed, May 24, 2017 at 04:11:12PM +0300, Mathias Nyman wrote: >>> This optimization significantly reduces xhci driver load time. >>> >>> In ACPI tables the acpi companion port devices are children of >>> the hub device. The port devices are identified by their port number >>> returned by the ACPI _ADR method. >>> _ADR 0 is reserved for the root hub device. >>> >>> The current implementation to find a acpi companion port device >>> loops through all acpi port devices under that parent hub, calling >>> their _ADR method each time a new port device is added. >>> >>> for a xHC controller with 25 ports under its roothub it >>> will end up invoking ACPI bytecode 625 times before all ports >>> are ready, making it really slow. >>> >>> The _ADR values are already read and cached earler. So instead of >>> running the bytecode again we can check the cached _ADR value first, >>> and then fall back to the old way. >>> >>> As one of the more significant changes, the xhci load time on >>> Intel kabylake reduced by 70%, (28ms) from >>> initcall xhci_pci_init+0x0/0x49 returned 0 after 39537 usecs >>> to >>> initcall xhci_pci_init+0x0/0x49 returned 0 after 11270 usecs >>> >>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> >>> --- >>> drivers/usb/core/usb-acpi.c | 26 +++++++++++++++++++++++--- >>> 1 file changed, 23 insertions(+), 3 deletions(-) >> >> Why is this RFC? What's wrong with it as-is? >> > > Last minute doubt, nothing should be wrong, but I started to wonder if > there is > any particular reason the ACPI part was done the way it was. > > Or if maybe other drivers could benefit from checking cached _ADR > value first as > well, and this whole thing should be a part of drivers/acpi/glue.c > instead? > That or we should just evaluate _ADR if present during the very initialization and store the value in a filed under struct acpi_device. Thanks, Rafael ^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [RFC PATCH] usb: optimize acpi companion search for usb port devices 2017-05-31 23:01 ` Rafael J. Wysocki @ 2017-06-02 12:20 ` Mathias Nyman 0 siblings, 0 replies; 3+ messages in thread From: Mathias Nyman @ 2017-06-02 12:20 UTC (permalink / raw) To: Rafael J. Wysocki; +Cc: Greg KH, linux-usb, stern, ACPI Devel Maling List On 01.06.2017 02:01, Rafael J. Wysocki wrote: > On 5/25/2017 5:24 PM, Mathias Nyman wrote: >> On 24.05.2017 17:44, Greg KH wrote: >>> On Wed, May 24, 2017 at 04:11:12PM +0300, Mathias Nyman wrote: >>>> This optimization significantly reduces xhci driver load time. >>>> >>>> In ACPI tables the acpi companion port devices are children of >>>> the hub device. The port devices are identified by their port number >>>> returned by the ACPI _ADR method. >>>> _ADR 0 is reserved for the root hub device. >>>> >>>> The current implementation to find a acpi companion port device >>>> loops through all acpi port devices under that parent hub, calling >>>> their _ADR method each time a new port device is added. >>>> >>>> for a xHC controller with 25 ports under its roothub it >>>> will end up invoking ACPI bytecode 625 times before all ports >>>> are ready, making it really slow. >>>> >>>> The _ADR values are already read and cached earler. So instead of >>>> running the bytecode again we can check the cached _ADR value first, >>>> and then fall back to the old way. >>>> >>>> As one of the more significant changes, the xhci load time on >>>> Intel kabylake reduced by 70%, (28ms) from >>>> initcall xhci_pci_init+0x0/0x49 returned 0 after 39537 usecs >>>> to >>>> initcall xhci_pci_init+0x0/0x49 returned 0 after 11270 usecs >>>> >>>> Signed-off-by: Mathias Nyman <mathias.nyman@linux.intel.com> >>>> --- >>>> drivers/usb/core/usb-acpi.c | 26 +++++++++++++++++++++++--- >>>> 1 file changed, 23 insertions(+), 3 deletions(-) >>> >>> Why is this RFC? What's wrong with it as-is? >>> >> >> Last minute doubt, nothing should be wrong, but I started to wonder if there is >> any particular reason the ACPI part was done the way it was. >> >> Or if maybe other drivers could benefit from checking cached _ADR value first as >> well, and this whole thing should be a part of drivers/acpi/glue.c instead? >> > > That or we should just evaluate _ADR if present during the very initialization and store the value in a filed under struct acpi_device. > Yes, that is pretty much done, acpi_init_device_object() will end up evaluating _ADR and store it in acpi_device.pnp.bus_address Just considering if there should be something like acpi_get_adr(acpi_device *adev) that would first check if adev->pnp.bus_address exists, if not, then it would evaluate _ADR. But really just a thought, I think I'll send this forward as is. Thanks Mathias ^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-06-02 12:17 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <1495631472-3828-1-git-send-email-mathias.nyman@linux.intel.com>
[not found] ` <20170524144414.GA13730@kroah.com>
2017-05-25 15:24 ` [RFC PATCH] usb: optimize acpi companion search for usb port devices Mathias Nyman
2017-05-31 23:01 ` Rafael J. Wysocki
2017-06-02 12:20 ` Mathias Nyman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).