mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Lv Zheng <lv.zheng@intel.com>
To: "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
	Len Brown <len.brown@intel.com>
Cc: Lv Zheng <lv.zheng@intel.com>, Lv Zheng <zetalog@gmail.com>,
	<linux-kernel@vger.kernel.org>,
	linux-acpi@vger.kernel.org
Subject: [PATCH v2 5/6] ACPI / EC: Add module params for polling modes.
Date: Fri, 15 May 2015 14:16:48 +0800	[thread overview]
Message-ID: <8c15c7b45fcdfb05ecf0ffb3d3bd67fff2ec2243.1431670199.git.lv.zheng@intel.com> (raw)
In-Reply-To: <cover.1431670199.git.lv.zheng@intel.com>

We have 2 polling modes in the EC driver:
1. busy polling: originally used for the MSI quirks. udelay() is used to
   perform register access guarding.
2. wait polling: normal code path uses wait_event_timeout() and it can be
   woken up as soon as the transaction is completed in the interrupt mode.
   It also contains the register acces guarding logic in case the interrupt
   doesn't arrive and the EC driver is about to advance the transaction in
   task context (the polling mode).
The wait polling is useful for interrupt mode to allow other tasks to use
the CPU during the wait.
But for the polling mode, the busy polling takes less time than the wait
polling, because if no interrupt arrives, the wait polling has to wait the
minimal HZ interval.

We have a new use case for using the busy polling mode. Some GPIO drivers
initialize PIN configuration which cause a GPIO multiplexed EC GPE to be
disabled out of the GPE register's control. Busy polling mode is useful
here as it takes less time than the wait polling. But the guarding logic
prevents it from responding even faster. We should spinning around the EC
status rather than spinning around the nop execution lasted a determined
period.

This patch introduces 2 module params for the polling mode switch and the
guard time, so that users can use the busy polling mode without the
guarding in case the guarding is not necessary. This is an example to use
the 2 module params for this purpose:
  acpi.ec_busy_polling acpi.ec_polling_guard=0

We've tested the patch on a test platform. The platform suffers from such
kind of the GPIO PIN issue. The GPIO driver resets all PIN configuration
and after that, EC interrupt cannot arrive because of the multiplexing.
Then the platform suffers from a long delay carried out by the
wait_event_timeout() as all further EC transactions will run in the polling
mode. We switched the EC driver to use the busy polling mechanism instead
of the wait timeout polling mechanism and the delay is still high:
[   44.283005] calling  PNP0C0B:00+ @ 1305, parent: platform
[   44.417548] call PNP0C0B:00+ returned 0 after 131323 usecs
And this patch can significantly reduce the delay:
[   44.502625] calling  PNP0C0B:00+ @ 1308, parent: platform
[   44.503760] call PNP0C0B:00+ returned 0 after 1103 usecs

Tested-by: Chen Yu <yu.c.chen@intel.com>
Signed-off-by: Lv Zheng <lv.zheng@intel.com>
---
 drivers/acpi/ec.c |   39 +++++++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 16 deletions(-)

diff --git a/drivers/acpi/ec.c b/drivers/acpi/ec.c
index a521b6b..846e0617 100644
--- a/drivers/acpi/ec.c
+++ b/drivers/acpi/ec.c
@@ -92,6 +92,14 @@ static unsigned int ec_delay __read_mostly = ACPI_EC_DELAY;
 module_param(ec_delay, uint, 0644);
 MODULE_PARM_DESC(ec_delay, "Timeout(ms) waited until an EC command completes");
 
+static bool ec_busy_polling __read_mostly;
+module_param(ec_busy_polling, bool, 0644);
+MODULE_PARM_DESC(ec_busy_polling, "Use busy polling to advance EC transaction");
+
+static unsigned int ec_polling_guard __read_mostly = ACPI_EC_UDELAY_POLL;
+module_param(ec_polling_guard, uint, 0644);
+MODULE_PARM_DESC(ec_polling_guard, "Guard time(us) between EC accesses in polling modes");
+
 /*
  * If the number of false interrupts per one transaction exceeds
  * this threshold, will think there is a GPE storm happened and
@@ -128,7 +136,6 @@ static void advance_transaction(struct acpi_ec *ec);
 struct acpi_ec *boot_ec, *first_ec;
 EXPORT_SYMBOL(first_ec);
 
-static int EC_FLAGS_MSI; /* Out-of-spec MSI controller */
 static int EC_FLAGS_VALIDATE_ECDT; /* ASUStec ECDTs need to be validated */
 static int EC_FLAGS_SKIP_DSDT_SCAN; /* Not all BIOS survive early DSDT scan */
 static int EC_FLAGS_CLEAR_ON_RESUME; /* Needs acpi_ec_clear() on boot/resume */
@@ -504,11 +511,11 @@ static void start_transaction(struct acpi_ec *ec)
 
 static int ec_guard(struct acpi_ec *ec)
 {
-	unsigned long guard = usecs_to_jiffies(ACPI_EC_UDELAY_POLL);
+	unsigned long guard = usecs_to_jiffies(ec_polling_guard);
 	unsigned long timeout = ec->timestamp + guard;
 
 	do {
-		if (EC_FLAGS_MSI) {
+		if (ec_busy_polling) {
 			/* Perform busy polling */
 			if (ec_transaction_completed(ec))
 				return 0;
@@ -985,7 +992,7 @@ acpi_ec_space_handler(u32 function, acpi_physical_address address,
 	if (function != ACPI_READ && function != ACPI_WRITE)
 		return AE_BAD_PARAMETER;
 
-	if (EC_FLAGS_MSI || bits > 8)
+	if (ec_busy_polling || bits > 8)
 		acpi_ec_burst_enable(ec);
 
 	for (i = 0; i < bytes; ++i, ++address, ++value)
@@ -993,7 +1000,7 @@ acpi_ec_space_handler(u32 function, acpi_physical_address address,
 			acpi_ec_read(ec, address, value) :
 			acpi_ec_write(ec, address, *value);
 
-	if (EC_FLAGS_MSI || bits > 8)
+	if (ec_busy_polling || bits > 8)
 		acpi_ec_burst_disable(ec);
 
 	switch (result) {
@@ -1262,11 +1269,11 @@ static int ec_validate_ecdt(const struct dmi_system_id *id)
 	return 0;
 }
 
-/* MSI EC needs special treatment, enable it */
-static int ec_flag_msi(const struct dmi_system_id *id)
+/* EC firmware needs special polling mode, enable it */
+static int ec_use_busy_polling(const struct dmi_system_id *id)
 {
-	pr_debug("Detected MSI hardware, enabling workarounds.\n");
-	EC_FLAGS_MSI = 1;
+	pr_debug("Detected the EC firmware requiring busy polling mode.\n");
+	ec_busy_polling = 1;
 	EC_FLAGS_VALIDATE_ECDT = 1;
 	return 0;
 }
@@ -1313,27 +1320,27 @@ static struct dmi_system_id ec_dmi_table[] __initdata = {
 	DMI_MATCH(DMI_BIOS_VENDOR, "COMPAL"),
 	DMI_MATCH(DMI_BOARD_NAME, "JFL92") }, NULL},
 	{
-	ec_flag_msi, "MSI hardware", {
+	ec_use_busy_polling, "MSI hardware", {
 	DMI_MATCH(DMI_BIOS_VENDOR, "Micro-Star")}, NULL},
 	{
-	ec_flag_msi, "MSI hardware", {
+	ec_use_busy_polling, "MSI hardware", {
 	DMI_MATCH(DMI_SYS_VENDOR, "Micro-Star")}, NULL},
 	{
-	ec_flag_msi, "MSI hardware", {
+	ec_use_busy_polling, "MSI hardware", {
 	DMI_MATCH(DMI_CHASSIS_VENDOR, "MICRO-Star")}, NULL},
 	{
-	ec_flag_msi, "MSI hardware", {
+	ec_use_busy_polling, "MSI hardware", {
 	DMI_MATCH(DMI_CHASSIS_VENDOR, "MICRO-STAR")}, NULL},
 	{
-	ec_flag_msi, "Quanta hardware", {
+	ec_use_busy_polling, "Quanta hardware", {
 	DMI_MATCH(DMI_SYS_VENDOR, "Quanta"),
 	DMI_MATCH(DMI_PRODUCT_NAME, "TW8/SW8/DW8"),}, NULL},
 	{
-	ec_flag_msi, "Quanta hardware", {
+	ec_use_busy_polling, "Quanta hardware", {
 	DMI_MATCH(DMI_SYS_VENDOR, "Quanta"),
 	DMI_MATCH(DMI_PRODUCT_NAME, "TW9/SW9"),}, NULL},
 	{
-	ec_flag_msi, "Clevo W350etq", {
+	ec_use_busy_polling, "Clevo W350etq", {
 	DMI_MATCH(DMI_SYS_VENDOR, "CLEVO CO."),
 	DMI_MATCH(DMI_PRODUCT_NAME, "W35_37ET"),}, NULL},
 	{
-- 
1.7.10


  parent reply	other threads:[~2015-05-15  6:17 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <3b6064b23b148a9c3eed58566e9361a852618556.1430900840.git.lv.zheng@intel.com>
2015-05-15  6:16 ` [PATCH v2 0/6] ACPI / EC: Update due to recent changes Lv Zheng
2015-05-15  6:16   ` [PATCH v2 1/6] ACPI / EC: Update acpi_ec_is_gpe_raised() with new GPE status flag Lv Zheng
2015-05-15  6:16   ` [PATCH v2 2/6] ACPI / EC: Remove storming threashold enlarging quirk Lv Zheng
2015-05-15  6:16   ` [PATCH v2 3/6] ACPI / EC: Remove irqs_disabled() check Lv Zheng
2015-05-15  6:16   ` [PATCH v2 4/6] ACPI / EC: Fix and clean up register access guarding logics Lv Zheng
2015-05-15  6:16   ` Lv Zheng [this message]
2015-05-15  6:16   ` [PATCH v2 6/6] ACPI / EC: Remove non-root-caused busy polling quirks Lv Zheng
2015-05-16  0:22   ` [PATCH v2 0/6] ACPI / EC: Update due to recent changes Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8c15c7b45fcdfb05ecf0ffb3d3bd67fff2ec2243.1431670199.git.lv.zheng@intel.com \
    --to=lv.zheng@intel.com \
    --cc=len.brown@intel.com \
    --cc=linux-acpi@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafael.j.wysocki@intel.com \
    --cc=zetalog@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®