diff mbox

[3/4] mailbox: pcc: optimized pcc_send_data

Message ID 1452883415-24452-4-git-send-email-pprakash@codeaurora.org (mailing list archive)
State Superseded, archived
Headers show

Commit Message

Prakash, Prashanth Jan. 15, 2016, 6:43 p.m. UTC
pcc_send_data can be invoked during the execution of performance
critical code as in cppc_cpufreq driver. With acpi_* APIs, the
doorbell register accessed in pcc_send_data if present in system
memory will be searched (in cached virt to phys addr mapping),
mapped, read/written and then unmapped. These operations take
significant amount of time.

This patch maps the performance critical doorbell register
during init and then reads/writes to it directly using the
mapped virtual address. This patch + similar changes to CPPC
acpi driver reduce the time per freq. transition from around
200us to about 20us for cppc cpufreq driver

Signed-off-by: Prashanth Prakash <pprakash@codeaurora.org>
---
 drivers/mailbox/pcc.c | 110 ++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 103 insertions(+), 7 deletions(-)

Comments

kernel test robot Jan. 15, 2016, 7:33 p.m. UTC | #1
Hi Prashanth,

[auto build test ERROR on pm/linux-next]
[also build test ERROR on v4.4 next-20160115]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Prashanth-Prakash/acpi-cppc-optimization-patches/20160116-024434
base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: i386-allmodconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/mailbox/pcc.c: In function 'read_register':
>> drivers/mailbox/pcc.c:195:10: error: implicit declaration of function 'readq' [-Werror=implicit-function-declaration]
      *val = readq(vaddr);
             ^
   drivers/mailbox/pcc.c: In function 'write_register':
>> drivers/mailbox/pcc.c:221:3: error: implicit declaration of function 'writeq' [-Werror=implicit-function-declaration]
      writeq(val, vaddr);
      ^
   cc1: some warnings being treated as errors

vim +/readq +195 drivers/mailbox/pcc.c

   189			*val = readw(vaddr);
   190			break;
   191		case 32:
   192			*val = readl(vaddr);
   193			break;
   194		case 64:
 > 195			*val = readq(vaddr);
   196			break;
   197		default:
   198			pr_debug("Error: Cannot read register of %u bit width",
   199				bit_width);
   200			ret_val = -EFAULT;
   201			break;
   202		}
   203		return ret_val;
   204	}
   205	
   206	static int write_register(void *vaddr, u64 val, unsigned int bit_width)
   207	{
   208		int ret_val = 0;
   209	
   210		switch (bit_width) {
   211		case 8:
   212			writeb(val, vaddr);
   213			break;
   214		case 16:
   215			writew(val, vaddr);
   216			break;
   217		case 32:
   218			writel(val, vaddr);
   219			break;
   220		case 64:
 > 221			writeq(val, vaddr);
   222			break;
   223		default:
   224			pr_debug("Error: Cannot write register of %u bit width",

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Ashwin Chaugule Jan. 20, 2016, 10:29 p.m. UTC | #2
On 15 January 2016 at 14:33, kbuild test robot <lkp@intel.com> wrote:
> Hi Prashanth,
>
> [auto build test ERROR on pm/linux-next]
> [also build test ERROR on v4.4 next-20160115]
> [if your patch is applied to the wrong git tree, please drop us a note to help improving the system]
>
> url:    https://github.com/0day-ci/linux/commits/Prashanth-Prakash/acpi-cppc-optimization-patches/20160116-024434
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
> config: i386-allmodconfig (attached as .config)
> reproduce:
>         # save the attached .config to linux build tree
>         make ARCH=i386
>
> All errors (new ones prefixed by >>):
>
>    drivers/mailbox/pcc.c: In function 'read_register':
>>> drivers/mailbox/pcc.c:195:10: error: implicit declaration of function 'readq' [-Werror=implicit-function-declaration]
>       *val = readq(vaddr);
>              ^
>    drivers/mailbox/pcc.c: In function 'write_register':
>>> drivers/mailbox/pcc.c:221:3: error: implicit declaration of function 'writeq' [-Werror=implicit-function-declaration]
>       writeq(val, vaddr);
>       ^
>    cc1: some warnings being treated as errors

You probably need to include asm-generic/io.h

Thanks,
Ashwin.

>
> vim +/readq +195 drivers/mailbox/pcc.c
>
>    189                  *val = readw(vaddr);
>    190                  break;
>    191          case 32:
>    192                  *val = readl(vaddr);
>    193                  break;
>    194          case 64:
>  > 195                  *val = readq(vaddr);
>    196                  break;
>    197          default:
>    198                  pr_debug("Error: Cannot read register of %u bit width",
>    199                          bit_width);
>    200                  ret_val = -EFAULT;
>    201                  break;
>    202          }
>    203          return ret_val;
>    204  }
>    205
>    206  static int write_register(void *vaddr, u64 val, unsigned int bit_width)
>    207  {
>    208          int ret_val = 0;
>    209
>    210          switch (bit_width) {
>    211          case 8:
>    212                  writeb(val, vaddr);
>    213                  break;
>    214          case 16:
>    215                  writew(val, vaddr);
>    216                  break;
>    217          case 32:
>    218                  writel(val, vaddr);
>    219                  break;
>    220          case 64:
>  > 221                  writeq(val, vaddr);
>    222                  break;
>    223          default:
>    224                  pr_debug("Error: Cannot write register of %u bit width",
>
> ---
> 0-DAY kernel test infrastructure                Open Source Technology Center
> https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Prakash, Prashanth Jan. 21, 2016, 12:05 a.m. UTC | #3
On 1/20/2016 3:29 PM, Ashwin Chaugule wrote:
>>>> drivers/mailbox/pcc.c:221:3: error: implicit declaration of function 'writeq' [-Werror=implicit-function-declaration]
>>       writeq(val, vaddr);
>>       ^
>>    cc1: some warnings being treated as errors
> You probably need to include asm-generic/io.h
>
Thanks! I will update, test and re-post the patch set.

-Prashanth
--
To unsubscribe from this list: send the line "unsubscribe linux-acpi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/mailbox/pcc.c b/drivers/mailbox/pcc.c
index 45d85ae..150e18e 100644
--- a/drivers/mailbox/pcc.c
+++ b/drivers/mailbox/pcc.c
@@ -70,6 +70,9 @@ 
 
 static struct mbox_chan *pcc_mbox_channels;
 
+/*Array of cached virtual address for doorbell registers*/
+static void **pcc_doorbell_vaddr;
+
 static struct mbox_controller pcc_mbox_ctrl = {};
 /**
  * get_pcc_channel - Given a PCC subspace idx, get
@@ -166,6 +169,66 @@  void pcc_mbox_free_channel(struct mbox_chan *chan)
 }
 EXPORT_SYMBOL_GPL(pcc_mbox_free_channel);
 
+/*
+ * PCC can be used with perf critical drivers such as CPPC
+ * So it makes sense to locally cache the virtual address and
+ * use it to read/write to PCC registers such as doorbell register
+ *
+ * The below read_register and write_registers are used to read and
+ * write from perf critical registers such as PCC doorbell register
+ */
+static int read_register(void *vaddr, u64 *val, unsigned int bit_width)
+{
+	int ret_val = 0;
+
+	switch (bit_width) {
+	case 8:
+		*val = readb(vaddr);
+		break;
+	case 16:
+		*val = readw(vaddr);
+		break;
+	case 32:
+		*val = readl(vaddr);
+		break;
+	case 64:
+		*val = readq(vaddr);
+		break;
+	default:
+		pr_debug("Error: Cannot read register of %u bit width",
+			bit_width);
+		ret_val = -EFAULT;
+		break;
+	}
+	return ret_val;
+}
+
+static int write_register(void *vaddr, u64 val, unsigned int bit_width)
+{
+	int ret_val = 0;
+
+	switch (bit_width) {
+	case 8:
+		writeb(val, vaddr);
+		break;
+	case 16:
+		writew(val, vaddr);
+		break;
+	case 32:
+		writel(val, vaddr);
+		break;
+	case 64:
+		writeq(val, vaddr);
+		break;
+	default:
+		pr_debug("Error: Cannot write register of %u bit width",
+			bit_width);
+		ret_val = -EFAULT;
+		break;
+	}
+	return ret_val;
+}
+
 /**
  * pcc_send_data - Called from Mailbox Controller code. Used
  *		here only to ring the channel doorbell. The PCC client
@@ -181,21 +244,39 @@  EXPORT_SYMBOL_GPL(pcc_mbox_free_channel);
 static int pcc_send_data(struct mbox_chan *chan, void *data)
 {
 	struct acpi_pcct_hw_reduced *pcct_ss = chan->con_priv;
-	struct acpi_generic_address doorbell;
+	struct acpi_generic_address *doorbell;
 	u64 doorbell_preserve;
 	u64 doorbell_val;
 	u64 doorbell_write;
+	u32 id = chan - pcc_mbox_channels;
+	int ret = 0;
+
+	if (id >= pcc_mbox_ctrl.num_chans) {
+		pr_debug("pcc_send_data: Invalid mbox_chan passed\n");
+		return -ENOENT;
+	}
 
-	doorbell = pcct_ss->doorbell_register;
+	doorbell = &pcct_ss->doorbell_register;
 	doorbell_preserve = pcct_ss->preserve_mask;
 	doorbell_write = pcct_ss->write_mask;
 
 	/* Sync notification from OS to Platform. */
-	acpi_read(&doorbell_val, &doorbell);
-	acpi_write((doorbell_val & doorbell_preserve) | doorbell_write,
-			&doorbell);
-
-	return 0;
+	if (pcc_doorbell_vaddr[id]) {
+		ret = read_register(pcc_doorbell_vaddr[id], &doorbell_val,
+			doorbell->bit_width);
+		if (ret)
+			return ret;
+		ret = write_register(pcc_doorbell_vaddr[id],
+			(doorbell_val & doorbell_preserve) | doorbell_write,
+			doorbell->bit_width);
+	} else {
+		ret = acpi_read(&doorbell_val, doorbell);
+		if (ret)
+			return ret;
+		ret = acpi_write((doorbell_val & doorbell_preserve) | doorbell_write,
+			doorbell);
+	}
+	return ret;
 }
 
 static const struct mbox_chan_ops pcc_chan_ops = {
@@ -271,14 +352,29 @@  static int __init acpi_pcc_probe(void)
 		return -ENOMEM;
 	}
 
+	pcc_doorbell_vaddr = kcalloc(count, sizeof(void *), GFP_KERNEL);
+	if (!pcc_doorbell_vaddr) {
+		kfree(pcc_mbox_channels);
+		return -ENOMEM;
+	}
+
 	/* Point to the first PCC subspace entry */
 	pcct_entry = (struct acpi_subtable_header *) (
 		(unsigned long) pcct_tbl + sizeof(struct acpi_table_pcct));
 
 	for (i = 0; i < count; i++) {
+		struct acpi_generic_address *db_reg;
+		struct acpi_pcct_hw_reduced *pcct_ss;
 		pcc_mbox_channels[i].con_priv = pcct_entry;
 		pcct_entry = (struct acpi_subtable_header *)
 			((unsigned long) pcct_entry + pcct_entry->length);
+
+		/* If doorbell is in system memory cache the virt address */
+		pcct_ss = (struct acpi_pcct_hw_reduced *)pcct_entry;
+		db_reg = &pcct_ss->doorbell_register;
+		if (db_reg->space_id == ACPI_ADR_SPACE_SYSTEM_MEMORY)
+			pcc_doorbell_vaddr[i] = acpi_os_ioremap(db_reg->address,
+							db_reg->bit_width/8);
 	}
 
 	pcc_mbox_ctrl.num_chans = count;