All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/4] Support for STA idle mode power save(IMPS)
@ 2018-04-17 12:06 ` pillair
  0 siblings, 0 replies; 44+ messages in thread
From: pillair @ 2018-04-17 12:06 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Rakesh Pillai

From: Rakesh Pillai <pillair@codeaurora.org>

Enable STA idle mode power save(IMPS) for WCN3990 TLV target.

In-order to support STA idle mode ps direct access to CE registers are
protected via 2 mechanism. As target can power collapse based on idle
inactivity and any target register access during that state can lead to
un-clocked access.

2) Caching SRRI/DRRI in DDR.

WN3990 has a shadow block in HW which is always on domain and allows HOST/APPS
access irrespective of power state of the target(Common subsystem(includes CE block)
might be down due to idle/inactivity).
Any operation on the shadow registers are directly reflected back in the actual
CE registers( SRWI/DRWI) once common subsystem gets up.
The shadow registers configuration is supplied via QMI message.

WC3990 has 24 shadow registers and mapping of shadow register(0-23) to CE registers(0-12)
is as following.

               -----------------------------------------------------------
               Shadow Register      |     CE   |    src/dst write index
               -----------------------------------------------------------
                              0            |     0    |           src
                              3            |     3    |           src
                              4            |     4    |           src
                              5            |     5    |           src
                              7            |     7    |           src
                              13           |     1    |           dst
                              14           |     2    |           dst
                              19           |     7    |           dst
                              20           |     8    |           dst
                              21           |     9    |           dst
                              22           |     10   |           dst
                              23           |     11   |           dst


1) Caching SRWI/SRWI in HW shadow block.

Since shadow block allows only WRITE access, for read access(SRRI/DRRI) driver allocates
region in DDR(12CE*half WORD) which is being configured in CE UPD control register.
CE SRRI/DRRI are restored and replayed on the configured region(DDR) on each update.
HOST/APPS reads SRRI/DRRI from the DDR to make the access independent of target power state.


Changes in v2:
	- Disabled the shadow_reg_support and rri_on_ddr hw params for QCA988X_HW_2_0_VERSION
	- Fixed checkpatch warnings

Govind Singh (2):
  ath10k: Enable SRRI/DRRI support on ddr for WCN3990
  ath10k: Enable sta idle power save

Rakesh Pillai (2):
  ath10k: Add hw params for shadow register support
  ath10k: Add support for shadow register for WNC3990

 drivers/net/wireless/ath/ath10k/ce.c   | 245 +++++++++++++++++++++++++++++++--
 drivers/net/wireless/ath/ath10k/ce.h   |  14 ++
 drivers/net/wireless/ath/ath10k/core.c |  28 ++++
 drivers/net/wireless/ath/ath10k/hw.c   |   9 +-
 drivers/net/wireless/ath/ath10k/hw.h   |  17 ++-
 drivers/net/wireless/ath/ath10k/mac.c  |   7 +
 drivers/net/wireless/ath/ath10k/snoc.c |   3 +
 7 files changed, 312 insertions(+), 11 deletions(-)

-- 
2.14.1

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH v2 0/4] Support for STA idle mode power save(IMPS)
@ 2018-04-17 12:06 ` pillair
  0 siblings, 0 replies; 44+ messages in thread
From: pillair @ 2018-04-17 12:06 UTC (permalink / raw)
  To: ath10k; +Cc: Rakesh Pillai, linux-wireless

From: Rakesh Pillai <pillair@codeaurora.org>

Enable STA idle mode power save(IMPS) for WCN3990 TLV target.

In-order to support STA idle mode ps direct access to CE registers are
protected via 2 mechanism. As target can power collapse based on idle
inactivity and any target register access during that state can lead to
un-clocked access.

2) Caching SRRI/DRRI in DDR.

WN3990 has a shadow block in HW which is always on domain and allows HOST/APPS
access irrespective of power state of the target(Common subsystem(includes CE block)
might be down due to idle/inactivity).
Any operation on the shadow registers are directly reflected back in the actual
CE registers( SRWI/DRWI) once common subsystem gets up.
The shadow registers configuration is supplied via QMI message.

WC3990 has 24 shadow registers and mapping of shadow register(0-23) to CE registers(0-12)
is as following.

               -----------------------------------------------------------
               Shadow Register      |     CE   |    src/dst write index
               -----------------------------------------------------------
                              0            |     0    |           src
                              3            |     3    |           src
                              4            |     4    |           src
                              5            |     5    |           src
                              7            |     7    |           src
                              13           |     1    |           dst
                              14           |     2    |           dst
                              19           |     7    |           dst
                              20           |     8    |           dst
                              21           |     9    |           dst
                              22           |     10   |           dst
                              23           |     11   |           dst


1) Caching SRWI/SRWI in HW shadow block.

Since shadow block allows only WRITE access, for read access(SRRI/DRRI) driver allocates
region in DDR(12CE*half WORD) which is being configured in CE UPD control register.
CE SRRI/DRRI are restored and replayed on the configured region(DDR) on each update.
HOST/APPS reads SRRI/DRRI from the DDR to make the access independent of target power state.


Changes in v2:
	- Disabled the shadow_reg_support and rri_on_ddr hw params for QCA988X_HW_2_0_VERSION
	- Fixed checkpatch warnings

Govind Singh (2):
  ath10k: Enable SRRI/DRRI support on ddr for WCN3990
  ath10k: Enable sta idle power save

Rakesh Pillai (2):
  ath10k: Add hw params for shadow register support
  ath10k: Add support for shadow register for WNC3990

 drivers/net/wireless/ath/ath10k/ce.c   | 245 +++++++++++++++++++++++++++++++--
 drivers/net/wireless/ath/ath10k/ce.h   |  14 ++
 drivers/net/wireless/ath/ath10k/core.c |  28 ++++
 drivers/net/wireless/ath/ath10k/hw.c   |   9 +-
 drivers/net/wireless/ath/ath10k/hw.h   |  17 ++-
 drivers/net/wireless/ath/ath10k/mac.c  |   7 +
 drivers/net/wireless/ath/ath10k/snoc.c |   3 +
 7 files changed, 312 insertions(+), 11 deletions(-)

-- 
2.14.1


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* [PATCH v2 1/4] ath10k: Add hw params for shadow register support
  2018-04-17 12:06 ` pillair
@ 2018-04-17 12:06   ` pillair
  -1 siblings, 0 replies; 44+ messages in thread
From: pillair @ 2018-04-17 12:06 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Rakesh Pillai

From: Rakesh Pillai <pillair@codeaurora.org>

wcn3990 supports shadow register for ce write.

Add a hw param for shadow register support.

Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/core.c | 14 ++++++++++++++
 drivers/net/wireless/ath/ath10k/hw.h   |  4 ++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 64479acd9dc5..5a9d222acfe6 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -90,6 +90,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.num_wds_entries = 0x20,
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA988X_HW_2_0_VERSION,
@@ -120,6 +121,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA9887_HW_1_0_VERSION,
@@ -150,6 +152,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA6174_HW_2_1_VERSION,
@@ -179,6 +182,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA6174_HW_2_1_VERSION,
@@ -208,6 +212,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA6174_HW_3_0_VERSION,
@@ -237,6 +242,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA6174_HW_3_2_VERSION,
@@ -269,6 +275,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA99X0_HW_2_0_DEV_VERSION,
@@ -304,6 +311,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA9984_HW_1_0_DEV_VERSION,
@@ -344,6 +352,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA9888_HW_2_0_DEV_VERSION,
@@ -383,6 +392,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA9377_HW_1_0_DEV_VERSION,
@@ -412,6 +422,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA9377_HW_1_1_DEV_VERSION,
@@ -443,6 +454,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA4019_HW_1_0_DEV_VERSION,
@@ -479,6 +491,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = WCN3990_HW_1_0_DEV_VERSION,
@@ -500,6 +513,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = true,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL_DUAL_MAC,
 		.per_ce_irq = true,
+		.shadow_reg_support = true,
 	},
 };
 
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 3041eba61e54..74faee5a2578 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2005-2011 Atheros Communications Inc.
  * Copyright (c) 2011-2017 Qualcomm Atheros, Inc.
+ * Copyright (c) 2018 The Linux Foundation. All rights reserved.
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -571,6 +572,9 @@ struct ath10k_hw_params {
 
 	/* target supporting per ce IRQ */
 	bool per_ce_irq;
+
+	/* target supporting shadow register for ce write */
+	bool shadow_reg_support;
 };
 
 struct htt_rx_desc;
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v2 1/4] ath10k: Add hw params for shadow register support
@ 2018-04-17 12:06   ` pillair
  0 siblings, 0 replies; 44+ messages in thread
From: pillair @ 2018-04-17 12:06 UTC (permalink / raw)
  To: ath10k; +Cc: Rakesh Pillai, linux-wireless

From: Rakesh Pillai <pillair@codeaurora.org>

wcn3990 supports shadow register for ce write.

Add a hw param for shadow register support.

Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/core.c | 14 ++++++++++++++
 drivers/net/wireless/ath/ath10k/hw.h   |  4 ++++
 2 files changed, 18 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 64479acd9dc5..5a9d222acfe6 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -90,6 +90,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.num_wds_entries = 0x20,
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA988X_HW_2_0_VERSION,
@@ -120,6 +121,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA9887_HW_1_0_VERSION,
@@ -150,6 +152,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA6174_HW_2_1_VERSION,
@@ -179,6 +182,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA6174_HW_2_1_VERSION,
@@ -208,6 +212,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA6174_HW_3_0_VERSION,
@@ -237,6 +242,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA6174_HW_3_2_VERSION,
@@ -269,6 +275,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA99X0_HW_2_0_DEV_VERSION,
@@ -304,6 +311,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA9984_HW_1_0_DEV_VERSION,
@@ -344,6 +352,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA9888_HW_2_0_DEV_VERSION,
@@ -383,6 +392,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA9377_HW_1_0_DEV_VERSION,
@@ -412,6 +422,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA9377_HW_1_1_DEV_VERSION,
@@ -443,6 +454,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = QCA4019_HW_1_0_DEV_VERSION,
@@ -479,6 +491,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
+		.shadow_reg_support = false,
 	},
 	{
 		.id = WCN3990_HW_1_0_DEV_VERSION,
@@ -500,6 +513,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = true,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL_DUAL_MAC,
 		.per_ce_irq = true,
+		.shadow_reg_support = true,
 	},
 };
 
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 3041eba61e54..74faee5a2578 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2005-2011 Atheros Communications Inc.
  * Copyright (c) 2011-2017 Qualcomm Atheros, Inc.
+ * Copyright (c) 2018 The Linux Foundation. All rights reserved.
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -571,6 +572,9 @@ struct ath10k_hw_params {
 
 	/* target supporting per ce IRQ */
 	bool per_ce_irq;
+
+	/* target supporting shadow register for ce write */
+	bool shadow_reg_support;
 };
 
 struct htt_rx_desc;
-- 
2.14.1


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v2 2/4] ath10k: Add support for shadow register for WNC3990
  2018-04-17 12:06 ` pillair
@ 2018-04-17 12:06   ` pillair
  -1 siblings, 0 replies; 44+ messages in thread
From: pillair @ 2018-04-17 12:06 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Rakesh Pillai

From: Rakesh Pillai <pillair@codeaurora.org>

WCN3990 needs shadow register write operation support
for copy engine for regular operation in powersave mode.
Add support for copy engine shadow register write in
datapath tx for WCN3990

Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/ce.c | 143 ++++++++++++++++++++++++++++++++++-
 drivers/net/wireless/ath/ath10k/ce.h |   4 +
 2 files changed, 145 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index e7e7b342e5b8..5053dd92bf01 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2005-2011 Atheros Communications Inc.
  * Copyright (c) 2011-2017 Qualcomm Atheros, Inc.
+ * Copyright (c) 2018 The Linux Foundation. All rights reserved.
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -58,6 +59,74 @@
  * the buffer is sent/received.
  */
 
+static inline u32 shadow_sr_wr_ind_addr(struct ath10k *ar,
+					struct ath10k_ce_pipe *ce_state)
+{
+	u32 ce_id = ce_state->id;
+	u32 addr = 0;
+
+	switch (ce_id) {
+	case 0:
+		addr = 0x00032000;
+		break;
+	case 3:
+		addr = 0x0003200C;
+		break;
+	case 4:
+		addr = 0x00032010;
+		break;
+	case 5:
+		addr = 0x00032014;
+		break;
+	case 7:
+		addr = 0x0003201C;
+		break;
+	default:
+		ath10k_warn(ar, "invalid CE id: %d", ce_id);
+		break;
+	}
+	return addr;
+}
+
+static inline u32 shadow_dst_wr_ind_addr(struct ath10k *ar,
+					 struct ath10k_ce_pipe *ce_state)
+{
+	u32 ce_id = ce_state->id;
+	u32 addr = 0;
+
+	switch (ce_id) {
+	case 1:
+		addr = 0x00032034;
+		break;
+	case 2:
+		addr = 0x00032038;
+		break;
+	case 5:
+		addr = 0x00032044;
+		break;
+	case 7:
+		addr = 0x0003204C;
+		break;
+	case 8:
+		addr = 0x00032050;
+		break;
+	case 9:
+		addr = 0x00032054;
+		break;
+	case 10:
+		addr = 0x00032058;
+		break;
+	case 11:
+		addr = 0x0003205C;
+		break;
+	default:
+		ath10k_warn(ar, "invalid CE id: %d", ce_id);
+		break;
+	}
+
+	return addr;
+}
+
 static inline unsigned int
 ath10k_set_ring_byte(unsigned int offset,
 		     struct ath10k_hw_ce_regs_addr_map *addr_map)
@@ -123,6 +192,22 @@ static inline u32 ath10k_ce_src_ring_read_index_get(struct ath10k *ar,
 				ar->hw_ce_regs->current_srri_addr);
 }
 
+static inline void
+ath10k_ce_shadow_src_ring_write_index_set(struct ath10k *ar,
+					  struct ath10k_ce_pipe *ce_state,
+					  unsigned int value)
+{
+	ath10k_ce_write32(ar, shadow_sr_wr_ind_addr(ar, ce_state), value);
+}
+
+static inline void
+ath10k_ce_shadow_dest_ring_write_index_set(struct ath10k *ar,
+					   struct ath10k_ce_pipe *ce_state,
+					   unsigned int value)
+{
+	ath10k_ce_write32(ar, shadow_dst_wr_ind_addr(ar, ce_state), value);
+}
+
 static inline void ath10k_ce_src_ring_base_addr_set(struct ath10k *ar,
 						    u32 ce_ctrl_addr,
 						    unsigned int addr)
@@ -376,8 +461,14 @@ static int _ath10k_ce_send_nolock(struct ath10k_ce_pipe *ce_state,
 	write_index = CE_RING_IDX_INCR(nentries_mask, write_index);
 
 	/* WORKAROUND */
-	if (!(flags & CE_SEND_FLAG_GATHER))
-		ath10k_ce_src_ring_write_index_set(ar, ctrl_addr, write_index);
+	if (!(flags & CE_SEND_FLAG_GATHER)) {
+		if (ar->hw_params.shadow_reg_support)
+			ath10k_ce_shadow_src_ring_write_index_set(ar, ce_state,
+								  write_index);
+		else
+			ath10k_ce_src_ring_write_index_set(ar, ctrl_addr,
+							   write_index);
+	}
 
 	src_ring->write_index = write_index;
 exit:
@@ -1251,6 +1342,22 @@ static int ath10k_ce_init_dest_ring(struct ath10k *ar,
 	return 0;
 }
 
+static int ath10k_ce_alloc_shadow_base(struct ath10k *ar,
+				       struct ath10k_ce_ring *src_ring,
+				       u32 nentries)
+{
+	src_ring->shadow_base_unaligned = kcalloc(nentries,
+						  sizeof(struct ce_desc),
+						  GFP_KERNEL);
+	if (!src_ring->shadow_base_unaligned)
+		return -ENOMEM;
+
+	src_ring->shadow_base = (struct ce_desc *)
+			PTR_ALIGN(src_ring->shadow_base_unaligned,
+				  CE_DESC_RING_ALIGN);
+	return 0;
+}
+
 static struct ath10k_ce_ring *
 ath10k_ce_alloc_src_ring(struct ath10k *ar, unsigned int ce_id,
 			 const struct ce_attr *attr)
@@ -1258,6 +1365,7 @@ ath10k_ce_alloc_src_ring(struct ath10k *ar, unsigned int ce_id,
 	struct ath10k_ce_ring *src_ring;
 	u32 nentries = attr->src_nentries;
 	dma_addr_t base_addr;
+	int ret;
 
 	nentries = roundup_pow_of_two(nentries);
 
@@ -1294,6 +1402,19 @@ ath10k_ce_alloc_src_ring(struct ath10k *ar, unsigned int ce_id,
 			ALIGN(src_ring->base_addr_ce_space_unaligned,
 			      CE_DESC_RING_ALIGN);
 
+	if (ar->hw_params.shadow_reg_support) {
+		ret = ath10k_ce_alloc_shadow_base(ar, src_ring, nentries);
+		if (ret) {
+			dma_free_coherent(ar->dev,
+					  (nentries * sizeof(struct ce_desc) +
+					   CE_DESC_RING_ALIGN),
+					  src_ring->base_addr_owner_space_unaligned,
+					  base_addr);
+			kfree(src_ring);
+			return ERR_PTR(ret);
+		}
+	}
+
 	return src_ring;
 }
 
@@ -1304,6 +1425,7 @@ ath10k_ce_alloc_src_ring_64(struct ath10k *ar, unsigned int ce_id,
 	struct ath10k_ce_ring *src_ring;
 	u32 nentries = attr->src_nentries;
 	dma_addr_t base_addr;
+	int ret;
 
 	nentries = roundup_pow_of_two(nentries);
 
@@ -1339,6 +1461,19 @@ ath10k_ce_alloc_src_ring_64(struct ath10k *ar, unsigned int ce_id,
 			ALIGN(src_ring->base_addr_ce_space_unaligned,
 			      CE_DESC_RING_ALIGN);
 
+	if (ar->hw_params.shadow_reg_support) {
+		ret = ath10k_ce_alloc_shadow_base(ar, src_ring, nentries);
+		if (ret) {
+			dma_free_coherent(ar->dev,
+					  (nentries * sizeof(struct ce_desc) +
+					   CE_DESC_RING_ALIGN),
+					  src_ring->base_addr_owner_space_unaligned,
+					  base_addr);
+			kfree(src_ring);
+			return ERR_PTR(ret);
+		}
+	}
+
 	return src_ring;
 }
 
@@ -1505,6 +1640,8 @@ static void _ath10k_ce_free_pipe(struct ath10k *ar, int ce_id)
 	struct ath10k_ce_pipe *ce_state = &ce->ce_states[ce_id];
 
 	if (ce_state->src_ring) {
+		if (ar->hw_params.shadow_reg_support)
+			kfree(ce_state->src_ring->shadow_base_unaligned);
 		dma_free_coherent(ar->dev,
 				  (ce_state->src_ring->nentries *
 				   sizeof(struct ce_desc) +
@@ -1534,6 +1671,8 @@ static void _ath10k_ce_free_pipe_64(struct ath10k *ar, int ce_id)
 	struct ath10k_ce_pipe *ce_state = &ce->ce_states[ce_id];
 
 	if (ce_state->src_ring) {
+		if (ar->hw_params.shadow_reg_support)
+			kfree(ce_state->src_ring->shadow_base_unaligned);
 		dma_free_coherent(ar->dev,
 				  (ce_state->src_ring->nentries *
 				   sizeof(struct ce_desc_64) +
diff --git a/drivers/net/wireless/ath/ath10k/ce.h b/drivers/net/wireless/ath/ath10k/ce.h
index d8f9da334529..9aea89133209 100644
--- a/drivers/net/wireless/ath/ath10k/ce.h
+++ b/drivers/net/wireless/ath/ath10k/ce.h
@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2005-2011 Atheros Communications Inc.
  * Copyright (c) 2011-2017 Qualcomm Atheros, Inc.
+ * Copyright (c) 2018 The Linux Foundation. All rights reserved.
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -113,6 +114,9 @@ struct ath10k_ce_ring {
 	/* CE address space */
 	u32 base_addr_ce_space;
 
+	char *shadow_base_unaligned;
+	struct ce_desc *shadow_base;
+
 	/* keep last */
 	void *per_transfer_context[0];
 };
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v2 2/4] ath10k: Add support for shadow register for WNC3990
@ 2018-04-17 12:06   ` pillair
  0 siblings, 0 replies; 44+ messages in thread
From: pillair @ 2018-04-17 12:06 UTC (permalink / raw)
  To: ath10k; +Cc: Rakesh Pillai, linux-wireless

From: Rakesh Pillai <pillair@codeaurora.org>

WCN3990 needs shadow register write operation support
for copy engine for regular operation in powersave mode.
Add support for copy engine shadow register write in
datapath tx for WCN3990

Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/ce.c | 143 ++++++++++++++++++++++++++++++++++-
 drivers/net/wireless/ath/ath10k/ce.h |   4 +
 2 files changed, 145 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index e7e7b342e5b8..5053dd92bf01 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2005-2011 Atheros Communications Inc.
  * Copyright (c) 2011-2017 Qualcomm Atheros, Inc.
+ * Copyright (c) 2018 The Linux Foundation. All rights reserved.
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -58,6 +59,74 @@
  * the buffer is sent/received.
  */
 
+static inline u32 shadow_sr_wr_ind_addr(struct ath10k *ar,
+					struct ath10k_ce_pipe *ce_state)
+{
+	u32 ce_id = ce_state->id;
+	u32 addr = 0;
+
+	switch (ce_id) {
+	case 0:
+		addr = 0x00032000;
+		break;
+	case 3:
+		addr = 0x0003200C;
+		break;
+	case 4:
+		addr = 0x00032010;
+		break;
+	case 5:
+		addr = 0x00032014;
+		break;
+	case 7:
+		addr = 0x0003201C;
+		break;
+	default:
+		ath10k_warn(ar, "invalid CE id: %d", ce_id);
+		break;
+	}
+	return addr;
+}
+
+static inline u32 shadow_dst_wr_ind_addr(struct ath10k *ar,
+					 struct ath10k_ce_pipe *ce_state)
+{
+	u32 ce_id = ce_state->id;
+	u32 addr = 0;
+
+	switch (ce_id) {
+	case 1:
+		addr = 0x00032034;
+		break;
+	case 2:
+		addr = 0x00032038;
+		break;
+	case 5:
+		addr = 0x00032044;
+		break;
+	case 7:
+		addr = 0x0003204C;
+		break;
+	case 8:
+		addr = 0x00032050;
+		break;
+	case 9:
+		addr = 0x00032054;
+		break;
+	case 10:
+		addr = 0x00032058;
+		break;
+	case 11:
+		addr = 0x0003205C;
+		break;
+	default:
+		ath10k_warn(ar, "invalid CE id: %d", ce_id);
+		break;
+	}
+
+	return addr;
+}
+
 static inline unsigned int
 ath10k_set_ring_byte(unsigned int offset,
 		     struct ath10k_hw_ce_regs_addr_map *addr_map)
@@ -123,6 +192,22 @@ static inline u32 ath10k_ce_src_ring_read_index_get(struct ath10k *ar,
 				ar->hw_ce_regs->current_srri_addr);
 }
 
+static inline void
+ath10k_ce_shadow_src_ring_write_index_set(struct ath10k *ar,
+					  struct ath10k_ce_pipe *ce_state,
+					  unsigned int value)
+{
+	ath10k_ce_write32(ar, shadow_sr_wr_ind_addr(ar, ce_state), value);
+}
+
+static inline void
+ath10k_ce_shadow_dest_ring_write_index_set(struct ath10k *ar,
+					   struct ath10k_ce_pipe *ce_state,
+					   unsigned int value)
+{
+	ath10k_ce_write32(ar, shadow_dst_wr_ind_addr(ar, ce_state), value);
+}
+
 static inline void ath10k_ce_src_ring_base_addr_set(struct ath10k *ar,
 						    u32 ce_ctrl_addr,
 						    unsigned int addr)
@@ -376,8 +461,14 @@ static int _ath10k_ce_send_nolock(struct ath10k_ce_pipe *ce_state,
 	write_index = CE_RING_IDX_INCR(nentries_mask, write_index);
 
 	/* WORKAROUND */
-	if (!(flags & CE_SEND_FLAG_GATHER))
-		ath10k_ce_src_ring_write_index_set(ar, ctrl_addr, write_index);
+	if (!(flags & CE_SEND_FLAG_GATHER)) {
+		if (ar->hw_params.shadow_reg_support)
+			ath10k_ce_shadow_src_ring_write_index_set(ar, ce_state,
+								  write_index);
+		else
+			ath10k_ce_src_ring_write_index_set(ar, ctrl_addr,
+							   write_index);
+	}
 
 	src_ring->write_index = write_index;
 exit:
@@ -1251,6 +1342,22 @@ static int ath10k_ce_init_dest_ring(struct ath10k *ar,
 	return 0;
 }
 
+static int ath10k_ce_alloc_shadow_base(struct ath10k *ar,
+				       struct ath10k_ce_ring *src_ring,
+				       u32 nentries)
+{
+	src_ring->shadow_base_unaligned = kcalloc(nentries,
+						  sizeof(struct ce_desc),
+						  GFP_KERNEL);
+	if (!src_ring->shadow_base_unaligned)
+		return -ENOMEM;
+
+	src_ring->shadow_base = (struct ce_desc *)
+			PTR_ALIGN(src_ring->shadow_base_unaligned,
+				  CE_DESC_RING_ALIGN);
+	return 0;
+}
+
 static struct ath10k_ce_ring *
 ath10k_ce_alloc_src_ring(struct ath10k *ar, unsigned int ce_id,
 			 const struct ce_attr *attr)
@@ -1258,6 +1365,7 @@ ath10k_ce_alloc_src_ring(struct ath10k *ar, unsigned int ce_id,
 	struct ath10k_ce_ring *src_ring;
 	u32 nentries = attr->src_nentries;
 	dma_addr_t base_addr;
+	int ret;
 
 	nentries = roundup_pow_of_two(nentries);
 
@@ -1294,6 +1402,19 @@ ath10k_ce_alloc_src_ring(struct ath10k *ar, unsigned int ce_id,
 			ALIGN(src_ring->base_addr_ce_space_unaligned,
 			      CE_DESC_RING_ALIGN);
 
+	if (ar->hw_params.shadow_reg_support) {
+		ret = ath10k_ce_alloc_shadow_base(ar, src_ring, nentries);
+		if (ret) {
+			dma_free_coherent(ar->dev,
+					  (nentries * sizeof(struct ce_desc) +
+					   CE_DESC_RING_ALIGN),
+					  src_ring->base_addr_owner_space_unaligned,
+					  base_addr);
+			kfree(src_ring);
+			return ERR_PTR(ret);
+		}
+	}
+
 	return src_ring;
 }
 
@@ -1304,6 +1425,7 @@ ath10k_ce_alloc_src_ring_64(struct ath10k *ar, unsigned int ce_id,
 	struct ath10k_ce_ring *src_ring;
 	u32 nentries = attr->src_nentries;
 	dma_addr_t base_addr;
+	int ret;
 
 	nentries = roundup_pow_of_two(nentries);
 
@@ -1339,6 +1461,19 @@ ath10k_ce_alloc_src_ring_64(struct ath10k *ar, unsigned int ce_id,
 			ALIGN(src_ring->base_addr_ce_space_unaligned,
 			      CE_DESC_RING_ALIGN);
 
+	if (ar->hw_params.shadow_reg_support) {
+		ret = ath10k_ce_alloc_shadow_base(ar, src_ring, nentries);
+		if (ret) {
+			dma_free_coherent(ar->dev,
+					  (nentries * sizeof(struct ce_desc) +
+					   CE_DESC_RING_ALIGN),
+					  src_ring->base_addr_owner_space_unaligned,
+					  base_addr);
+			kfree(src_ring);
+			return ERR_PTR(ret);
+		}
+	}
+
 	return src_ring;
 }
 
@@ -1505,6 +1640,8 @@ static void _ath10k_ce_free_pipe(struct ath10k *ar, int ce_id)
 	struct ath10k_ce_pipe *ce_state = &ce->ce_states[ce_id];
 
 	if (ce_state->src_ring) {
+		if (ar->hw_params.shadow_reg_support)
+			kfree(ce_state->src_ring->shadow_base_unaligned);
 		dma_free_coherent(ar->dev,
 				  (ce_state->src_ring->nentries *
 				   sizeof(struct ce_desc) +
@@ -1534,6 +1671,8 @@ static void _ath10k_ce_free_pipe_64(struct ath10k *ar, int ce_id)
 	struct ath10k_ce_pipe *ce_state = &ce->ce_states[ce_id];
 
 	if (ce_state->src_ring) {
+		if (ar->hw_params.shadow_reg_support)
+			kfree(ce_state->src_ring->shadow_base_unaligned);
 		dma_free_coherent(ar->dev,
 				  (ce_state->src_ring->nentries *
 				   sizeof(struct ce_desc_64) +
diff --git a/drivers/net/wireless/ath/ath10k/ce.h b/drivers/net/wireless/ath/ath10k/ce.h
index d8f9da334529..9aea89133209 100644
--- a/drivers/net/wireless/ath/ath10k/ce.h
+++ b/drivers/net/wireless/ath/ath10k/ce.h
@@ -1,6 +1,7 @@
 /*
  * Copyright (c) 2005-2011 Atheros Communications Inc.
  * Copyright (c) 2011-2017 Qualcomm Atheros, Inc.
+ * Copyright (c) 2018 The Linux Foundation. All rights reserved.
  *
  * Permission to use, copy, modify, and/or distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -113,6 +114,9 @@ struct ath10k_ce_ring {
 	/* CE address space */
 	u32 base_addr_ce_space;
 
+	char *shadow_base_unaligned;
+	struct ce_desc *shadow_base;
+
 	/* keep last */
 	void *per_transfer_context[0];
 };
-- 
2.14.1


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v2 3/4] ath10k: Enable SRRI/DRRI support on ddr for WCN3990
  2018-04-17 12:06 ` pillair
@ 2018-04-17 12:07   ` pillair
  -1 siblings, 0 replies; 44+ messages in thread
From: pillair @ 2018-04-17 12:07 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Govind Singh, Rakesh Pillai

From: Govind Singh <govinds@codeaurora.org>

SRRI/DRRI are not mapped in the HW Shadow block and can lead
to un-clocked access if common subsystem in the target is
powered down due to idle mode.

To mitigate this problem SRRI/DRRI can be read from
DDR instead of doing an actual hardware read.
Host allocates non cached memory on ddr and configures
the physical address of this memory to the CE hardware.
The hardware updates the RRI on this particular location.
Read SRRI/DRRI from DDR location instead of
direct target read.

Enable retention restore on ddr using hw params to enable
in specific targets.

Signed-off-by: Govind Singh <govinds@codeaurora.org>
Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/ce.c   | 102 +++++++++++++++++++++++++++++++--
 drivers/net/wireless/ath/ath10k/ce.h   |  10 ++++
 drivers/net/wireless/ath/ath10k/core.c |  14 +++++
 drivers/net/wireless/ath/ath10k/hw.c   |   9 ++-
 drivers/net/wireless/ath/ath10k/hw.h   |  13 ++++-
 drivers/net/wireless/ath/ath10k/snoc.c |   3 +
 6 files changed, 142 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 5053dd92bf01..f6eee5f30a8f 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -185,11 +185,30 @@ static inline u32 ath10k_ce_src_ring_write_index_get(struct ath10k *ar,
 				ar->hw_ce_regs->sr_wr_index_addr);
 }
 
+static inline u32 ath10k_ce_src_ring_read_index_from_ddr(struct ath10k *ar,
+							 u32 ce_id)
+{
+	struct ath10k_ce *ce = ath10k_ce_priv(ar);
+
+	return ce->vaddr_rri[ce_id] & CE_DDR_RRI_MASK;
+}
+
 static inline u32 ath10k_ce_src_ring_read_index_get(struct ath10k *ar,
 						    u32 ce_ctrl_addr)
 {
-	return ath10k_ce_read32(ar, ce_ctrl_addr +
-				ar->hw_ce_regs->current_srri_addr);
+	struct ath10k_ce *ce = ath10k_ce_priv(ar);
+	u32 ce_id = COPY_ENGINE_ID(ce_ctrl_addr);
+	struct ath10k_ce_pipe *ce_state = &ce->ce_states[ce_id];
+	u32 index;
+
+	if (ar->hw_params.rri_on_ddr &&
+	    (ce_state->attr_flags & CE_ATTR_DIS_INTR))
+		index = ath10k_ce_src_ring_read_index_from_ddr(ar, ce_id);
+	else
+		index = ath10k_ce_read32(ar, ce_ctrl_addr +
+					 ar->hw_ce_regs->current_srri_addr);
+
+	return index;
 }
 
 static inline void
@@ -266,11 +285,31 @@ static inline void ath10k_ce_dest_ring_byte_swap_set(struct ath10k *ar,
 			  ath10k_set_ring_byte(n, ctrl_regs->dst_ring));
 }
 
+static inline
+	u32 ath10k_ce_dest_ring_read_index_from_ddr(struct ath10k *ar, u32 ce_id)
+{
+	struct ath10k_ce *ce = ath10k_ce_priv(ar);
+
+	return (ce->vaddr_rri[ce_id] >> CE_DDR_DRRI_SHIFT) &
+		CE_DDR_RRI_MASK;
+}
+
 static inline u32 ath10k_ce_dest_ring_read_index_get(struct ath10k *ar,
 						     u32 ce_ctrl_addr)
 {
-	return ath10k_ce_read32(ar, ce_ctrl_addr +
-				ar->hw_ce_regs->current_drri_addr);
+	struct ath10k_ce *ce = ath10k_ce_priv(ar);
+	u32 ce_id = COPY_ENGINE_ID(ce_ctrl_addr);
+	struct ath10k_ce_pipe *ce_state = &ce->ce_states[ce_id];
+	u32 index;
+
+	if (ar->hw_params.rri_on_ddr &&
+	    (ce_state->attr_flags & CE_ATTR_DIS_INTR))
+		index = ath10k_ce_dest_ring_read_index_from_ddr(ar, ce_id);
+	else
+		index = ath10k_ce_read32(ar, ce_ctrl_addr +
+					 ar->hw_ce_regs->current_drri_addr);
+
+	return index;
 }
 
 static inline void ath10k_ce_dest_ring_base_addr_set(struct ath10k *ar,
@@ -486,7 +525,7 @@ static int _ath10k_ce_send_nolock_64(struct ath10k_ce_pipe *ce_state,
 	struct ath10k_ce_ring *src_ring = ce_state->src_ring;
 	struct ce_desc_64 *desc, sdesc;
 	unsigned int nentries_mask = src_ring->nentries_mask;
-	unsigned int sw_index = src_ring->sw_index;
+	unsigned int sw_index;
 	unsigned int write_index = src_ring->write_index;
 	u32 ctrl_addr = ce_state->ctrl_addr;
 	__le32 *addr;
@@ -500,6 +539,11 @@ static int _ath10k_ce_send_nolock_64(struct ath10k_ce_pipe *ce_state,
 		ath10k_warn(ar, "%s: send more we can (nbytes: %d, max: %d)\n",
 			    __func__, nbytes, ce_state->src_sz_max);
 
+	if (ar->hw_params.rri_on_ddr)
+		sw_index = ath10k_ce_src_ring_read_index_from_ddr(ar, ce_state->id);
+	else
+		sw_index = src_ring->sw_index;
+
 	if (unlikely(CE_RING_DELTA(nentries_mask,
 				   write_index, sw_index - 1) <= 0)) {
 		ret = -ENOSR;
@@ -1016,7 +1060,10 @@ int ath10k_ce_completed_send_next_nolock(struct ath10k_ce_pipe *ce_state,
 		src_ring->hw_index = read_index;
 	}
 
-	read_index = src_ring->hw_index;
+	if (ar->hw_params.rri_on_ddr)
+		read_index = ath10k_ce_src_ring_read_index_get(ar, ctrl_addr);
+	else
+		read_index = src_ring->hw_index;
 
 	if (read_index == sw_index)
 		return -EIO;
@@ -1841,3 +1888,46 @@ int ath10k_ce_alloc_pipe(struct ath10k *ar, int ce_id,
 	return 0;
 }
 EXPORT_SYMBOL(ath10k_ce_alloc_pipe);
+
+void ath10k_ce_alloc_rri(struct ath10k *ar)
+{
+	int i;
+	u32 value;
+	u32 ctrl1_regs;
+	u32 ce_base_addr;
+	struct ath10k_ce *ce = ath10k_ce_priv(ar);
+
+	ce->vaddr_rri = dma_alloc_coherent(ar->dev,
+					   (CE_COUNT * sizeof(u32)),
+					   &ce->paddr_rri, GFP_KERNEL);
+
+	if (!ce->vaddr_rri)
+		return;
+
+	ath10k_ce_write32(ar, ar->hw_ce_regs->ce_rri_low,
+			  lower_32_bits(ce->paddr_rri));
+	ath10k_ce_write32(ar, ar->hw_ce_regs->ce_rri_high,
+			  (upper_32_bits(ce->paddr_rri) &
+			  CE_DESC_FLAGS_GET_MASK));
+
+	for (i = 0; i < CE_COUNT; i++) {
+		ctrl1_regs = ar->hw_ce_regs->ctrl1_regs->addr;
+		ce_base_addr = ath10k_ce_base_address(ar, i);
+		value = ath10k_ce_read32(ar, ce_base_addr + ctrl1_regs);
+		value |= ar->hw_ce_regs->upd->mask;
+		ath10k_ce_write32(ar, ce_base_addr + ctrl1_regs, value);
+	}
+
+	memset(ce->vaddr_rri, 0, CE_COUNT * sizeof(u32));
+}
+EXPORT_SYMBOL(ath10k_ce_alloc_rri);
+
+void ath10k_ce_free_rri(struct ath10k *ar)
+{
+	struct ath10k_ce *ce = ath10k_ce_priv(ar);
+
+	dma_free_coherent(ar->dev, (CE_COUNT * sizeof(u32)),
+			  ce->vaddr_rri,
+			  ce->paddr_rri);
+}
+EXPORT_SYMBOL(ath10k_ce_free_rri);
diff --git a/drivers/net/wireless/ath/ath10k/ce.h b/drivers/net/wireless/ath/ath10k/ce.h
index 9aea89133209..dbeffaef6024 100644
--- a/drivers/net/wireless/ath/ath10k/ce.h
+++ b/drivers/net/wireless/ath/ath10k/ce.h
@@ -49,6 +49,9 @@ struct ath10k_ce_pipe;
 #define CE_DESC_FLAGS_META_DATA_MASK ar->hw_values->ce_desc_meta_data_mask
 #define CE_DESC_FLAGS_META_DATA_LSB  ar->hw_values->ce_desc_meta_data_lsb
 
+#define CE_DDR_RRI_MASK			GENMASK(15, 0)
+#define CE_DDR_DRRI_SHIFT		16
+
 struct ce_desc {
 	__le32 addr;
 	__le16 nbytes;
@@ -157,6 +160,8 @@ struct ath10k_ce {
 	spinlock_t ce_lock;
 	const struct ath10k_bus_ops *bus_ops;
 	struct ath10k_ce_pipe ce_states[CE_COUNT_MAX];
+	u32 *vaddr_rri;
+	dma_addr_t paddr_rri;
 };
 
 /*==================Send====================*/
@@ -265,6 +270,8 @@ int ath10k_ce_disable_interrupts(struct ath10k *ar);
 void ath10k_ce_enable_interrupts(struct ath10k *ar);
 void ath10k_ce_dump_registers(struct ath10k *ar,
 			      struct ath10k_fw_crash_data *crash_data);
+void ath10k_ce_alloc_rri(struct ath10k *ar);
+void ath10k_ce_free_rri(struct ath10k *ar);
 
 /* ce_attr.flags values */
 /* Use NonSnooping PCIe accesses? */
@@ -331,6 +338,9 @@ static inline u32 ath10k_ce_base_address(struct ath10k *ar, unsigned int ce_id)
 	return CE0_BASE_ADDRESS + (CE1_BASE_ADDRESS - CE0_BASE_ADDRESS) * ce_id;
 }
 
+#define COPY_ENGINE_ID(COPY_ENGINE_BASE_ADDRESS) (((COPY_ENGINE_BASE_ADDRESS) \
+		- CE0_BASE_ADDRESS) / (CE1_BASE_ADDRESS - CE0_BASE_ADDRESS))
+
 #define CE_SRC_RING_TO_DESC(baddr, idx) \
 	(&(((struct ce_desc *)baddr)[idx]))
 
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 5a9d222acfe6..6da6b8c6ef6a 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -91,6 +91,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA988X_HW_2_0_VERSION,
@@ -122,6 +123,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA9887_HW_1_0_VERSION,
@@ -153,6 +155,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA6174_HW_2_1_VERSION,
@@ -183,6 +186,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA6174_HW_2_1_VERSION,
@@ -213,6 +217,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA6174_HW_3_0_VERSION,
@@ -243,6 +248,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA6174_HW_3_2_VERSION,
@@ -276,6 +282,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA99X0_HW_2_0_DEV_VERSION,
@@ -312,6 +319,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA9984_HW_1_0_DEV_VERSION,
@@ -353,6 +361,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA9888_HW_2_0_DEV_VERSION,
@@ -393,6 +402,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA9377_HW_1_0_DEV_VERSION,
@@ -423,6 +433,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA9377_HW_1_1_DEV_VERSION,
@@ -455,6 +466,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA4019_HW_1_0_DEV_VERSION,
@@ -492,6 +504,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = WCN3990_HW_1_0_DEV_VERSION,
@@ -514,6 +527,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL_DUAL_MAC,
 		.per_ce_irq = true,
 		.shadow_reg_support = true,
+		.rri_on_ddr = true,
 	},
 };
 
diff --git a/drivers/net/wireless/ath/ath10k/hw.c b/drivers/net/wireless/ath/ath10k/hw.c
index 497ac33e0fbf..677535b3d207 100644
--- a/drivers/net/wireless/ath/ath10k/hw.c
+++ b/drivers/net/wireless/ath/ath10k/hw.c
@@ -310,6 +310,12 @@ static struct ath10k_hw_ce_dst_src_wm_regs wcn3990_wm_dst_ring = {
 	.wm_high	= &wcn3990_dst_wm_high,
 };
 
+static struct ath10k_hw_ce_ctrl1_upd wcn3990_ctrl1_upd = {
+	.shift = 19,
+	.mask = 0x00080000,
+	.enable = 0x00000000,
+};
+
 const struct ath10k_hw_ce_regs wcn3990_ce_regs = {
 	.sr_base_addr		= 0x00000000,
 	.sr_size_addr		= 0x00000008,
@@ -320,8 +326,6 @@ const struct ath10k_hw_ce_regs wcn3990_ce_regs = {
 	.dst_wr_index_addr	= 0x00000040,
 	.current_srri_addr	= 0x00000044,
 	.current_drri_addr	= 0x00000048,
-	.ddr_addr_for_rri_low	= 0x00000004,
-	.ddr_addr_for_rri_high	= 0x00000008,
 	.ce_rri_low		= 0x0024C004,
 	.ce_rri_high		= 0x0024C008,
 	.host_ie_addr		= 0x0000002c,
@@ -331,6 +335,7 @@ const struct ath10k_hw_ce_regs wcn3990_ce_regs = {
 	.misc_regs		= &wcn3990_misc_reg,
 	.wm_srcr		= &wcn3990_wm_src_ring,
 	.wm_dstr		= &wcn3990_wm_dst_ring,
+	.upd			= &wcn3990_ctrl1_upd,
 };
 
 const struct ath10k_hw_values wcn3990_values = {
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 74faee5a2578..c6240a28c026 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -336,6 +336,12 @@ struct ath10k_hw_ce_dst_src_wm_regs {
 	struct ath10k_hw_ce_regs_addr_map *wm_low;
 	struct ath10k_hw_ce_regs_addr_map *wm_high; };
 
+struct ath10k_hw_ce_ctrl1_upd {
+	u32 shift;
+	u32 mask;
+	u32 enable;
+};
+
 struct ath10k_hw_ce_regs {
 	u32 sr_base_addr;
 	u32 sr_size_addr;
@@ -358,7 +364,9 @@ struct ath10k_hw_ce_regs {
 	struct ath10k_hw_ce_cmd_halt *cmd_halt;
 	struct ath10k_hw_ce_host_ie *host_ie;
 	struct ath10k_hw_ce_dst_src_wm_regs *wm_srcr;
-	struct ath10k_hw_ce_dst_src_wm_regs *wm_dstr; };
+	struct ath10k_hw_ce_dst_src_wm_regs *wm_dstr;
+	struct ath10k_hw_ce_ctrl1_upd *upd;
+};
 
 struct ath10k_hw_values {
 	u32 rtc_state_val_on;
@@ -575,6 +583,9 @@ struct ath10k_hw_params {
 
 	/* target supporting shadow register for ce write */
 	bool shadow_reg_support;
+
+	/* target supporting retention restore on ddr */
+	bool rri_on_ddr;
 };
 
 struct htt_rx_desc;
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index 2e490ff124f1..47a4d2a5bd4c 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -767,6 +767,7 @@ static void ath10k_snoc_hif_power_down(struct ath10k *ar)
 	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif power down\n");
 
 	ath10k_snoc_wlan_disable(ar);
+	ath10k_ce_free_rri(ar);
 }
 
 static int ath10k_snoc_hif_power_up(struct ath10k *ar)
@@ -782,6 +783,8 @@ static int ath10k_snoc_hif_power_up(struct ath10k *ar)
 		return ret;
 	}
 
+	ath10k_ce_alloc_rri(ar);
+
 	ret = ath10k_snoc_init_pipes(ar);
 	if (ret) {
 		ath10k_err(ar, "failed to initialize CE: %d\n", ret);
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v2 3/4] ath10k: Enable SRRI/DRRI support on ddr for WCN3990
@ 2018-04-17 12:07   ` pillair
  0 siblings, 0 replies; 44+ messages in thread
From: pillair @ 2018-04-17 12:07 UTC (permalink / raw)
  To: ath10k; +Cc: Rakesh Pillai, Govind Singh, linux-wireless

From: Govind Singh <govinds@codeaurora.org>

SRRI/DRRI are not mapped in the HW Shadow block and can lead
to un-clocked access if common subsystem in the target is
powered down due to idle mode.

To mitigate this problem SRRI/DRRI can be read from
DDR instead of doing an actual hardware read.
Host allocates non cached memory on ddr and configures
the physical address of this memory to the CE hardware.
The hardware updates the RRI on this particular location.
Read SRRI/DRRI from DDR location instead of
direct target read.

Enable retention restore on ddr using hw params to enable
in specific targets.

Signed-off-by: Govind Singh <govinds@codeaurora.org>
Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/ce.c   | 102 +++++++++++++++++++++++++++++++--
 drivers/net/wireless/ath/ath10k/ce.h   |  10 ++++
 drivers/net/wireless/ath/ath10k/core.c |  14 +++++
 drivers/net/wireless/ath/ath10k/hw.c   |   9 ++-
 drivers/net/wireless/ath/ath10k/hw.h   |  13 ++++-
 drivers/net/wireless/ath/ath10k/snoc.c |   3 +
 6 files changed, 142 insertions(+), 9 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/ce.c b/drivers/net/wireless/ath/ath10k/ce.c
index 5053dd92bf01..f6eee5f30a8f 100644
--- a/drivers/net/wireless/ath/ath10k/ce.c
+++ b/drivers/net/wireless/ath/ath10k/ce.c
@@ -185,11 +185,30 @@ static inline u32 ath10k_ce_src_ring_write_index_get(struct ath10k *ar,
 				ar->hw_ce_regs->sr_wr_index_addr);
 }
 
+static inline u32 ath10k_ce_src_ring_read_index_from_ddr(struct ath10k *ar,
+							 u32 ce_id)
+{
+	struct ath10k_ce *ce = ath10k_ce_priv(ar);
+
+	return ce->vaddr_rri[ce_id] & CE_DDR_RRI_MASK;
+}
+
 static inline u32 ath10k_ce_src_ring_read_index_get(struct ath10k *ar,
 						    u32 ce_ctrl_addr)
 {
-	return ath10k_ce_read32(ar, ce_ctrl_addr +
-				ar->hw_ce_regs->current_srri_addr);
+	struct ath10k_ce *ce = ath10k_ce_priv(ar);
+	u32 ce_id = COPY_ENGINE_ID(ce_ctrl_addr);
+	struct ath10k_ce_pipe *ce_state = &ce->ce_states[ce_id];
+	u32 index;
+
+	if (ar->hw_params.rri_on_ddr &&
+	    (ce_state->attr_flags & CE_ATTR_DIS_INTR))
+		index = ath10k_ce_src_ring_read_index_from_ddr(ar, ce_id);
+	else
+		index = ath10k_ce_read32(ar, ce_ctrl_addr +
+					 ar->hw_ce_regs->current_srri_addr);
+
+	return index;
 }
 
 static inline void
@@ -266,11 +285,31 @@ static inline void ath10k_ce_dest_ring_byte_swap_set(struct ath10k *ar,
 			  ath10k_set_ring_byte(n, ctrl_regs->dst_ring));
 }
 
+static inline
+	u32 ath10k_ce_dest_ring_read_index_from_ddr(struct ath10k *ar, u32 ce_id)
+{
+	struct ath10k_ce *ce = ath10k_ce_priv(ar);
+
+	return (ce->vaddr_rri[ce_id] >> CE_DDR_DRRI_SHIFT) &
+		CE_DDR_RRI_MASK;
+}
+
 static inline u32 ath10k_ce_dest_ring_read_index_get(struct ath10k *ar,
 						     u32 ce_ctrl_addr)
 {
-	return ath10k_ce_read32(ar, ce_ctrl_addr +
-				ar->hw_ce_regs->current_drri_addr);
+	struct ath10k_ce *ce = ath10k_ce_priv(ar);
+	u32 ce_id = COPY_ENGINE_ID(ce_ctrl_addr);
+	struct ath10k_ce_pipe *ce_state = &ce->ce_states[ce_id];
+	u32 index;
+
+	if (ar->hw_params.rri_on_ddr &&
+	    (ce_state->attr_flags & CE_ATTR_DIS_INTR))
+		index = ath10k_ce_dest_ring_read_index_from_ddr(ar, ce_id);
+	else
+		index = ath10k_ce_read32(ar, ce_ctrl_addr +
+					 ar->hw_ce_regs->current_drri_addr);
+
+	return index;
 }
 
 static inline void ath10k_ce_dest_ring_base_addr_set(struct ath10k *ar,
@@ -486,7 +525,7 @@ static int _ath10k_ce_send_nolock_64(struct ath10k_ce_pipe *ce_state,
 	struct ath10k_ce_ring *src_ring = ce_state->src_ring;
 	struct ce_desc_64 *desc, sdesc;
 	unsigned int nentries_mask = src_ring->nentries_mask;
-	unsigned int sw_index = src_ring->sw_index;
+	unsigned int sw_index;
 	unsigned int write_index = src_ring->write_index;
 	u32 ctrl_addr = ce_state->ctrl_addr;
 	__le32 *addr;
@@ -500,6 +539,11 @@ static int _ath10k_ce_send_nolock_64(struct ath10k_ce_pipe *ce_state,
 		ath10k_warn(ar, "%s: send more we can (nbytes: %d, max: %d)\n",
 			    __func__, nbytes, ce_state->src_sz_max);
 
+	if (ar->hw_params.rri_on_ddr)
+		sw_index = ath10k_ce_src_ring_read_index_from_ddr(ar, ce_state->id);
+	else
+		sw_index = src_ring->sw_index;
+
 	if (unlikely(CE_RING_DELTA(nentries_mask,
 				   write_index, sw_index - 1) <= 0)) {
 		ret = -ENOSR;
@@ -1016,7 +1060,10 @@ int ath10k_ce_completed_send_next_nolock(struct ath10k_ce_pipe *ce_state,
 		src_ring->hw_index = read_index;
 	}
 
-	read_index = src_ring->hw_index;
+	if (ar->hw_params.rri_on_ddr)
+		read_index = ath10k_ce_src_ring_read_index_get(ar, ctrl_addr);
+	else
+		read_index = src_ring->hw_index;
 
 	if (read_index == sw_index)
 		return -EIO;
@@ -1841,3 +1888,46 @@ int ath10k_ce_alloc_pipe(struct ath10k *ar, int ce_id,
 	return 0;
 }
 EXPORT_SYMBOL(ath10k_ce_alloc_pipe);
+
+void ath10k_ce_alloc_rri(struct ath10k *ar)
+{
+	int i;
+	u32 value;
+	u32 ctrl1_regs;
+	u32 ce_base_addr;
+	struct ath10k_ce *ce = ath10k_ce_priv(ar);
+
+	ce->vaddr_rri = dma_alloc_coherent(ar->dev,
+					   (CE_COUNT * sizeof(u32)),
+					   &ce->paddr_rri, GFP_KERNEL);
+
+	if (!ce->vaddr_rri)
+		return;
+
+	ath10k_ce_write32(ar, ar->hw_ce_regs->ce_rri_low,
+			  lower_32_bits(ce->paddr_rri));
+	ath10k_ce_write32(ar, ar->hw_ce_regs->ce_rri_high,
+			  (upper_32_bits(ce->paddr_rri) &
+			  CE_DESC_FLAGS_GET_MASK));
+
+	for (i = 0; i < CE_COUNT; i++) {
+		ctrl1_regs = ar->hw_ce_regs->ctrl1_regs->addr;
+		ce_base_addr = ath10k_ce_base_address(ar, i);
+		value = ath10k_ce_read32(ar, ce_base_addr + ctrl1_regs);
+		value |= ar->hw_ce_regs->upd->mask;
+		ath10k_ce_write32(ar, ce_base_addr + ctrl1_regs, value);
+	}
+
+	memset(ce->vaddr_rri, 0, CE_COUNT * sizeof(u32));
+}
+EXPORT_SYMBOL(ath10k_ce_alloc_rri);
+
+void ath10k_ce_free_rri(struct ath10k *ar)
+{
+	struct ath10k_ce *ce = ath10k_ce_priv(ar);
+
+	dma_free_coherent(ar->dev, (CE_COUNT * sizeof(u32)),
+			  ce->vaddr_rri,
+			  ce->paddr_rri);
+}
+EXPORT_SYMBOL(ath10k_ce_free_rri);
diff --git a/drivers/net/wireless/ath/ath10k/ce.h b/drivers/net/wireless/ath/ath10k/ce.h
index 9aea89133209..dbeffaef6024 100644
--- a/drivers/net/wireless/ath/ath10k/ce.h
+++ b/drivers/net/wireless/ath/ath10k/ce.h
@@ -49,6 +49,9 @@ struct ath10k_ce_pipe;
 #define CE_DESC_FLAGS_META_DATA_MASK ar->hw_values->ce_desc_meta_data_mask
 #define CE_DESC_FLAGS_META_DATA_LSB  ar->hw_values->ce_desc_meta_data_lsb
 
+#define CE_DDR_RRI_MASK			GENMASK(15, 0)
+#define CE_DDR_DRRI_SHIFT		16
+
 struct ce_desc {
 	__le32 addr;
 	__le16 nbytes;
@@ -157,6 +160,8 @@ struct ath10k_ce {
 	spinlock_t ce_lock;
 	const struct ath10k_bus_ops *bus_ops;
 	struct ath10k_ce_pipe ce_states[CE_COUNT_MAX];
+	u32 *vaddr_rri;
+	dma_addr_t paddr_rri;
 };
 
 /*==================Send====================*/
@@ -265,6 +270,8 @@ int ath10k_ce_disable_interrupts(struct ath10k *ar);
 void ath10k_ce_enable_interrupts(struct ath10k *ar);
 void ath10k_ce_dump_registers(struct ath10k *ar,
 			      struct ath10k_fw_crash_data *crash_data);
+void ath10k_ce_alloc_rri(struct ath10k *ar);
+void ath10k_ce_free_rri(struct ath10k *ar);
 
 /* ce_attr.flags values */
 /* Use NonSnooping PCIe accesses? */
@@ -331,6 +338,9 @@ static inline u32 ath10k_ce_base_address(struct ath10k *ar, unsigned int ce_id)
 	return CE0_BASE_ADDRESS + (CE1_BASE_ADDRESS - CE0_BASE_ADDRESS) * ce_id;
 }
 
+#define COPY_ENGINE_ID(COPY_ENGINE_BASE_ADDRESS) (((COPY_ENGINE_BASE_ADDRESS) \
+		- CE0_BASE_ADDRESS) / (CE1_BASE_ADDRESS - CE0_BASE_ADDRESS))
+
 #define CE_SRC_RING_TO_DESC(baddr, idx) \
 	(&(((struct ce_desc *)baddr)[idx]))
 
diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c
index 5a9d222acfe6..6da6b8c6ef6a 100644
--- a/drivers/net/wireless/ath/ath10k/core.c
+++ b/drivers/net/wireless/ath/ath10k/core.c
@@ -91,6 +91,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.target_64bit = false,
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA988X_HW_2_0_VERSION,
@@ -122,6 +123,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA9887_HW_1_0_VERSION,
@@ -153,6 +155,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA6174_HW_2_1_VERSION,
@@ -183,6 +186,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA6174_HW_2_1_VERSION,
@@ -213,6 +217,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA6174_HW_3_0_VERSION,
@@ -243,6 +248,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA6174_HW_3_2_VERSION,
@@ -276,6 +282,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA99X0_HW_2_0_DEV_VERSION,
@@ -312,6 +319,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA9984_HW_1_0_DEV_VERSION,
@@ -353,6 +361,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA9888_HW_2_0_DEV_VERSION,
@@ -393,6 +402,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA9377_HW_1_0_DEV_VERSION,
@@ -423,6 +433,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA9377_HW_1_1_DEV_VERSION,
@@ -455,6 +466,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = QCA4019_HW_1_0_DEV_VERSION,
@@ -492,6 +504,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL,
 		.per_ce_irq = false,
 		.shadow_reg_support = false,
+		.rri_on_ddr = false,
 	},
 	{
 		.id = WCN3990_HW_1_0_DEV_VERSION,
@@ -514,6 +527,7 @@ static const struct ath10k_hw_params ath10k_hw_params_list[] = {
 		.rx_ring_fill_level = HTT_RX_RING_FILL_LEVEL_DUAL_MAC,
 		.per_ce_irq = true,
 		.shadow_reg_support = true,
+		.rri_on_ddr = true,
 	},
 };
 
diff --git a/drivers/net/wireless/ath/ath10k/hw.c b/drivers/net/wireless/ath/ath10k/hw.c
index 497ac33e0fbf..677535b3d207 100644
--- a/drivers/net/wireless/ath/ath10k/hw.c
+++ b/drivers/net/wireless/ath/ath10k/hw.c
@@ -310,6 +310,12 @@ static struct ath10k_hw_ce_dst_src_wm_regs wcn3990_wm_dst_ring = {
 	.wm_high	= &wcn3990_dst_wm_high,
 };
 
+static struct ath10k_hw_ce_ctrl1_upd wcn3990_ctrl1_upd = {
+	.shift = 19,
+	.mask = 0x00080000,
+	.enable = 0x00000000,
+};
+
 const struct ath10k_hw_ce_regs wcn3990_ce_regs = {
 	.sr_base_addr		= 0x00000000,
 	.sr_size_addr		= 0x00000008,
@@ -320,8 +326,6 @@ const struct ath10k_hw_ce_regs wcn3990_ce_regs = {
 	.dst_wr_index_addr	= 0x00000040,
 	.current_srri_addr	= 0x00000044,
 	.current_drri_addr	= 0x00000048,
-	.ddr_addr_for_rri_low	= 0x00000004,
-	.ddr_addr_for_rri_high	= 0x00000008,
 	.ce_rri_low		= 0x0024C004,
 	.ce_rri_high		= 0x0024C008,
 	.host_ie_addr		= 0x0000002c,
@@ -331,6 +335,7 @@ const struct ath10k_hw_ce_regs wcn3990_ce_regs = {
 	.misc_regs		= &wcn3990_misc_reg,
 	.wm_srcr		= &wcn3990_wm_src_ring,
 	.wm_dstr		= &wcn3990_wm_dst_ring,
+	.upd			= &wcn3990_ctrl1_upd,
 };
 
 const struct ath10k_hw_values wcn3990_values = {
diff --git a/drivers/net/wireless/ath/ath10k/hw.h b/drivers/net/wireless/ath/ath10k/hw.h
index 74faee5a2578..c6240a28c026 100644
--- a/drivers/net/wireless/ath/ath10k/hw.h
+++ b/drivers/net/wireless/ath/ath10k/hw.h
@@ -336,6 +336,12 @@ struct ath10k_hw_ce_dst_src_wm_regs {
 	struct ath10k_hw_ce_regs_addr_map *wm_low;
 	struct ath10k_hw_ce_regs_addr_map *wm_high; };
 
+struct ath10k_hw_ce_ctrl1_upd {
+	u32 shift;
+	u32 mask;
+	u32 enable;
+};
+
 struct ath10k_hw_ce_regs {
 	u32 sr_base_addr;
 	u32 sr_size_addr;
@@ -358,7 +364,9 @@ struct ath10k_hw_ce_regs {
 	struct ath10k_hw_ce_cmd_halt *cmd_halt;
 	struct ath10k_hw_ce_host_ie *host_ie;
 	struct ath10k_hw_ce_dst_src_wm_regs *wm_srcr;
-	struct ath10k_hw_ce_dst_src_wm_regs *wm_dstr; };
+	struct ath10k_hw_ce_dst_src_wm_regs *wm_dstr;
+	struct ath10k_hw_ce_ctrl1_upd *upd;
+};
 
 struct ath10k_hw_values {
 	u32 rtc_state_val_on;
@@ -575,6 +583,9 @@ struct ath10k_hw_params {
 
 	/* target supporting shadow register for ce write */
 	bool shadow_reg_support;
+
+	/* target supporting retention restore on ddr */
+	bool rri_on_ddr;
 };
 
 struct htt_rx_desc;
diff --git a/drivers/net/wireless/ath/ath10k/snoc.c b/drivers/net/wireless/ath/ath10k/snoc.c
index 2e490ff124f1..47a4d2a5bd4c 100644
--- a/drivers/net/wireless/ath/ath10k/snoc.c
+++ b/drivers/net/wireless/ath/ath10k/snoc.c
@@ -767,6 +767,7 @@ static void ath10k_snoc_hif_power_down(struct ath10k *ar)
 	ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif power down\n");
 
 	ath10k_snoc_wlan_disable(ar);
+	ath10k_ce_free_rri(ar);
 }
 
 static int ath10k_snoc_hif_power_up(struct ath10k *ar)
@@ -782,6 +783,8 @@ static int ath10k_snoc_hif_power_up(struct ath10k *ar)
 		return ret;
 	}
 
+	ath10k_ce_alloc_rri(ar);
+
 	ret = ath10k_snoc_init_pipes(ar);
 	if (ret) {
 		ath10k_err(ar, "failed to initialize CE: %d\n", ret);
-- 
2.14.1


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v2 4/4] ath10k: Enable sta idle power save
  2018-04-17 12:06 ` pillair
@ 2018-04-17 12:07   ` pillair
  -1 siblings, 0 replies; 44+ messages in thread
From: pillair @ 2018-04-17 12:07 UTC (permalink / raw)
  To: ath10k; +Cc: linux-wireless, Govind Singh, Rakesh Pillai

From: Govind Singh <govinds@codeaurora.org>

Enable sta power save in fw for the targets that
supports idle power save. The idle ps enable command
will be ignored by the firmware which does not support
this feature.

Signed-off-by: Govind Singh <govinds@codeaurora.org>
Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/mac.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 7e02ca02b28e..1d9222af1bb2 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4679,6 +4679,13 @@ static int ath10k_start(struct ieee80211_hw *hw)
 		}
 	}
 
+	param = ar->wmi.pdev_param->idle_ps_config;
+	ret = ath10k_wmi_pdev_set_param(ar, param, 1);
+	if (ret && ret != -EOPNOTSUPP) {
+		ath10k_warn(ar, "failed to enable idle_ps_config: %d\n", ret);
+		goto err_core_stop;
+	}
+
 	__ath10k_set_antenna(ar, ar->cfg_tx_chainmask, ar->cfg_rx_chainmask);
 
 	/*
-- 
2.14.1

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* [PATCH v2 4/4] ath10k: Enable sta idle power save
@ 2018-04-17 12:07   ` pillair
  0 siblings, 0 replies; 44+ messages in thread
From: pillair @ 2018-04-17 12:07 UTC (permalink / raw)
  To: ath10k; +Cc: Rakesh Pillai, Govind Singh, linux-wireless

From: Govind Singh <govinds@codeaurora.org>

Enable sta power save in fw for the targets that
supports idle power save. The idle ps enable command
will be ignored by the firmware which does not support
this feature.

Signed-off-by: Govind Singh <govinds@codeaurora.org>
Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
---
 drivers/net/wireless/ath/ath10k/mac.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 7e02ca02b28e..1d9222af1bb2 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -4679,6 +4679,13 @@ static int ath10k_start(struct ieee80211_hw *hw)
 		}
 	}
 
+	param = ar->wmi.pdev_param->idle_ps_config;
+	ret = ath10k_wmi_pdev_set_param(ar, param, 1);
+	if (ret && ret != -EOPNOTSUPP) {
+		ath10k_warn(ar, "failed to enable idle_ps_config: %d\n", ret);
+		goto err_core_stop;
+	}
+
 	__ath10k_set_antenna(ar, ar->cfg_tx_chainmask, ar->cfg_rx_chainmask);
 
 	/*
-- 
2.14.1


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply related	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
  2018-04-17 12:07   ` pillair
@ 2018-04-18  7:06     ` Sebastian Gottschall
  -1 siblings, 0 replies; 44+ messages in thread
From: Sebastian Gottschall @ 2018-04-18  7:06 UTC (permalink / raw)
  To: pillair, ath10k; +Cc: linux-wireless, Govind Singh

from my point of view powersave should be optional and not forced.

consider :
iw dev <devname> set power_save on/off

so there is already a config option made for that purpose,

Sebastian

Am 17.04.2018 um 14:07 schrieb pillair@codeaurora.org:
> From: Govind Singh <govinds@codeaurora.org>
>
> Enable sta power save in fw for the targets that
> supports idle power save. The idle ps enable command
> will be ignored by the firmware which does not support
> this feature.
>
> Signed-off-by: Govind Singh <govinds@codeaurora.org>
> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
> ---
>   drivers/net/wireless/ath/ath10k/mac.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 7e02ca02b28e..1d9222af1bb2 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -4679,6 +4679,13 @@ static int ath10k_start(struct ieee80211_hw *hw)
>   		}
>   	}
>   
> +	param = ar->wmi.pdev_param->idle_ps_config;
> +	ret = ath10k_wmi_pdev_set_param(ar, param, 1);
> +	if (ret && ret != -EOPNOTSUPP) {
> +		ath10k_warn(ar, "failed to enable idle_ps_config: %d\n", ret);
> +		goto err_core_stop;
> +	}
> +
>   	__ath10k_set_antenna(ar, ar->cfg_tx_chainmask, ar->cfg_rx_chainmask);
>   
>   	/*


-- 
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
@ 2018-04-18  7:06     ` Sebastian Gottschall
  0 siblings, 0 replies; 44+ messages in thread
From: Sebastian Gottschall @ 2018-04-18  7:06 UTC (permalink / raw)
  To: pillair, ath10k; +Cc: Govind Singh, linux-wireless

from my point of view powersave should be optional and not forced.

consider :
iw dev <devname> set power_save on/off

so there is already a config option made for that purpose,

Sebastian

Am 17.04.2018 um 14:07 schrieb pillair@codeaurora.org:
> From: Govind Singh <govinds@codeaurora.org>
>
> Enable sta power save in fw for the targets that
> supports idle power save. The idle ps enable command
> will be ignored by the firmware which does not support
> this feature.
>
> Signed-off-by: Govind Singh <govinds@codeaurora.org>
> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
> ---
>   drivers/net/wireless/ath/ath10k/mac.c | 7 +++++++
>   1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 7e02ca02b28e..1d9222af1bb2 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -4679,6 +4679,13 @@ static int ath10k_start(struct ieee80211_hw *hw)
>   		}
>   	}
>   
> +	param = ar->wmi.pdev_param->idle_ps_config;
> +	ret = ath10k_wmi_pdev_set_param(ar, param, 1);
> +	if (ret && ret != -EOPNOTSUPP) {
> +		ath10k_warn(ar, "failed to enable idle_ps_config: %d\n", ret);
> +		goto err_core_stop;
> +	}
> +
>   	__ath10k_set_antenna(ar, ar->cfg_tx_chainmask, ar->cfg_rx_chainmask);
>   
>   	/*


-- 
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
  2018-04-18  7:06     ` Sebastian Gottschall
@ 2018-04-18 13:07       ` govinds
  -1 siblings, 0 replies; 44+ messages in thread
From: govinds @ 2018-04-18 13:07 UTC (permalink / raw)
  To: Sebastian Gottschall; +Cc: pillair, ath10k, linux-wireless

On 2018-04-18 12:36, Sebastian Gottschall wrote:
> from my point of view powersave should be optional and not forced.
> 
> consider :
> iw dev <devname> set power_save on/off
> 
> so there is already a config option made for that purpose,
> 
> Sebastian
> 
> Am 17.04.2018 um 14:07 schrieb pillair@codeaurora.org:
>> From: Govind Singh <govinds@codeaurora.org>
>> 
>> Enable sta power save in fw for the targets that
>> supports idle power save. The idle ps enable command
>> will be ignored by the firmware which does not support
>> this feature.
>> 
>> Signed-off-by: Govind Singh <govinds@codeaurora.org>
>> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
>> ---
>>   drivers/net/wireless/ath/ath10k/mac.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>> 
>> diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
>> b/drivers/net/wireless/ath/ath10k/mac.c
>> index 7e02ca02b28e..1d9222af1bb2 100644
>> --- a/drivers/net/wireless/ath/ath10k/mac.c
>> +++ b/drivers/net/wireless/ath/ath10k/mac.c
>> @@ -4679,6 +4679,13 @@ static int ath10k_start(struct ieee80211_hw 
>> *hw)
>>   		}
>>   	}
>>   +	param = ar->wmi.pdev_param->idle_ps_config;
>> +	ret = ath10k_wmi_pdev_set_param(ar, param, 1);
>> +	if (ret && ret != -EOPNOTSUPP) {
>> +		ath10k_warn(ar, "failed to enable idle_ps_config: %d\n", ret);
>> +		goto err_core_stop;
>> +	}
>> +
>>   	__ath10k_set_antenna(ar, ar->cfg_tx_chainmask, 
>> ar->cfg_rx_chainmask);
>>     	/*

Thanks Sebastian for your review. Agree this should not be forced with 
in driver.

I will check if i can set the idle ps at the end of 
ath10k_mac_vif_setup_ps by checking arvif->ps.
I will update the patch accordingly.


BR,
Govind

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
@ 2018-04-18 13:07       ` govinds
  0 siblings, 0 replies; 44+ messages in thread
From: govinds @ 2018-04-18 13:07 UTC (permalink / raw)
  To: Sebastian Gottschall; +Cc: linux-wireless, pillair, ath10k

On 2018-04-18 12:36, Sebastian Gottschall wrote:
> from my point of view powersave should be optional and not forced.
> 
> consider :
> iw dev <devname> set power_save on/off
> 
> so there is already a config option made for that purpose,
> 
> Sebastian
> 
> Am 17.04.2018 um 14:07 schrieb pillair@codeaurora.org:
>> From: Govind Singh <govinds@codeaurora.org>
>> 
>> Enable sta power save in fw for the targets that
>> supports idle power save. The idle ps enable command
>> will be ignored by the firmware which does not support
>> this feature.
>> 
>> Signed-off-by: Govind Singh <govinds@codeaurora.org>
>> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
>> ---
>>   drivers/net/wireless/ath/ath10k/mac.c | 7 +++++++
>>   1 file changed, 7 insertions(+)
>> 
>> diff --git a/drivers/net/wireless/ath/ath10k/mac.c 
>> b/drivers/net/wireless/ath/ath10k/mac.c
>> index 7e02ca02b28e..1d9222af1bb2 100644
>> --- a/drivers/net/wireless/ath/ath10k/mac.c
>> +++ b/drivers/net/wireless/ath/ath10k/mac.c
>> @@ -4679,6 +4679,13 @@ static int ath10k_start(struct ieee80211_hw 
>> *hw)
>>   		}
>>   	}
>>   +	param = ar->wmi.pdev_param->idle_ps_config;
>> +	ret = ath10k_wmi_pdev_set_param(ar, param, 1);
>> +	if (ret && ret != -EOPNOTSUPP) {
>> +		ath10k_warn(ar, "failed to enable idle_ps_config: %d\n", ret);
>> +		goto err_core_stop;
>> +	}
>> +
>>   	__ath10k_set_antenna(ar, ar->cfg_tx_chainmask, 
>> ar->cfg_rx_chainmask);
>>     	/*

Thanks Sebastian for your review. Agree this should not be forced with 
in driver.

I will check if i can set the idle ps at the end of 
ath10k_mac_vif_setup_ps by checking arvif->ps.
I will update the patch accordingly.


BR,
Govind

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
  2018-04-18 13:07       ` govinds
@ 2018-04-18 13:16         ` Kalle Valo
  -1 siblings, 0 replies; 44+ messages in thread
From: Kalle Valo @ 2018-04-18 13:16 UTC (permalink / raw)
  To: govinds; +Cc: Sebastian Gottschall, linux-wireless, pillair, ath10k

(fixing top posting)

govinds@codeaurora.org writes:

> On 2018-04-18 12:36, Sebastian Gottschall wrote:
>> from my point of view powersave should be optional and not forced.
>>
>> consider :
>> iw dev <devname> set power_save on/off
>>
>> so there is already a config option made for that purpose,
>
> Thanks Sebastian for your review. Agree this should not be forced with
> in driver.
>
> I will check if i can set the idle ps at the end of
> ath10k_mac_vif_setup_ps by checking arvif->ps.
> I will update the patch accordingly.

So what power save is this exactly? I assumed it's some bus level power
save (turning of clocks etc) but is it actually 802.11 power save?

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
@ 2018-04-18 13:16         ` Kalle Valo
  0 siblings, 0 replies; 44+ messages in thread
From: Kalle Valo @ 2018-04-18 13:16 UTC (permalink / raw)
  To: govinds; +Cc: pillair, linux-wireless, ath10k, Sebastian Gottschall

(fixing top posting)

govinds@codeaurora.org writes:

> On 2018-04-18 12:36, Sebastian Gottschall wrote:
>> from my point of view powersave should be optional and not forced.
>>
>> consider :
>> iw dev <devname> set power_save on/off
>>
>> so there is already a config option made for that purpose,
>
> Thanks Sebastian for your review. Agree this should not be forced with
> in driver.
>
> I will check if i can set the idle ps at the end of
> ath10k_mac_vif_setup_ps by checking arvif->ps.
> I will update the patch accordingly.

So what power save is this exactly? I assumed it's some bus level power
save (turning of clocks etc) but is it actually 802.11 power save?

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
  2018-04-18 13:16         ` Kalle Valo
@ 2018-04-18 14:44           ` Arend van Spriel
  -1 siblings, 0 replies; 44+ messages in thread
From: Arend van Spriel @ 2018-04-18 14:44 UTC (permalink / raw)
  To: Kalle Valo, govinds; +Cc: Sebastian Gottschall, linux-wireless, pillair, ath10k

On 4/18/2018 3:16 PM, Kalle Valo wrote:
> (fixing top posting)
>
> govinds@codeaurora.org writes:
>
>> On 2018-04-18 12:36, Sebastian Gottschall wrote:
>>> from my point of view powersave should be optional and not forced.
>>>
>>> consider :
>>> iw dev <devname> set power_save on/off
>>>
>>> so there is already a config option made for that purpose,
>>
>> Thanks Sebastian for your review. Agree this should not be forced with
>> in driver.
>>
>> I will check if i can set the idle ps at the end of
>> ath10k_mac_vif_setup_ps by checking arvif->ps.
>> I will update the patch accordingly.
>
> So what power save is this exactly? I assumed it's some bus level power
> save (turning of clocks etc) but is it actually 802.11 power save?

I also suspected bus level power save and I would say runtime-pm 
framework should be considered for that.

Regards,
Arend

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
@ 2018-04-18 14:44           ` Arend van Spriel
  0 siblings, 0 replies; 44+ messages in thread
From: Arend van Spriel @ 2018-04-18 14:44 UTC (permalink / raw)
  To: Kalle Valo, govinds; +Cc: pillair, linux-wireless, ath10k, Sebastian Gottschall

On 4/18/2018 3:16 PM, Kalle Valo wrote:
> (fixing top posting)
>
> govinds@codeaurora.org writes:
>
>> On 2018-04-18 12:36, Sebastian Gottschall wrote:
>>> from my point of view powersave should be optional and not forced.
>>>
>>> consider :
>>> iw dev <devname> set power_save on/off
>>>
>>> so there is already a config option made for that purpose,
>>
>> Thanks Sebastian for your review. Agree this should not be forced with
>> in driver.
>>
>> I will check if i can set the idle ps at the end of
>> ath10k_mac_vif_setup_ps by checking arvif->ps.
>> I will update the patch accordingly.
>
> So what power save is this exactly? I assumed it's some bus level power
> save (turning of clocks etc) but is it actually 802.11 power save?

I also suspected bus level power save and I would say runtime-pm 
framework should be considered for that.

Regards,
Arend



_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
  2018-04-18 13:16         ` Kalle Valo
@ 2018-04-19  4:32           ` govinds
  -1 siblings, 0 replies; 44+ messages in thread
From: govinds @ 2018-04-19  4:32 UTC (permalink / raw)
  To: Kalle Valo; +Cc: Sebastian Gottschall, linux-wireless, pillair, ath10k

On 2018-04-18 18:46, Kalle Valo wrote:
> (fixing top posting)
> 
> govinds@codeaurora.org writes:
> 
>> On 2018-04-18 12:36, Sebastian Gottschall wrote:
>>> from my point of view powersave should be optional and not forced.
>>> 
>>> consider :
>>> iw dev <devname> set power_save on/off
>>> 
>>> so there is already a config option made for that purpose,
>> 
>> Thanks Sebastian for your review. Agree this should not be forced with
>> in driver.
>> 
>> I will check if i can set the idle ps at the end of
>> ath10k_mac_vif_setup_ps by checking arvif->ps.
>> I will update the patch accordingly.
> 
> So what power save is this exactly? I assumed it's some bus level power
> save (turning of clocks etc) but is it actually 802.11 power save?

Yes this is WIFI chip set level power-save(based on idleness) and not 
related to protocol power save.
FW will turn off/scale down the resources(clock/rails) based on 
opportunity(when ever idle mode is detected).
This power save is mostly done in disconnected state. I am not really 
sure when framework/user-space
triggers power-save config(iw dev <devname> set power_save on/off).
Then doing this from user-space will be unnecessarily toggling this 
config or may not be setting at disconnected state.

May be you can suggest better here.

BR,
Govind

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
@ 2018-04-19  4:32           ` govinds
  0 siblings, 0 replies; 44+ messages in thread
From: govinds @ 2018-04-19  4:32 UTC (permalink / raw)
  To: Kalle Valo; +Cc: pillair, linux-wireless, ath10k, Sebastian Gottschall

On 2018-04-18 18:46, Kalle Valo wrote:
> (fixing top posting)
> 
> govinds@codeaurora.org writes:
> 
>> On 2018-04-18 12:36, Sebastian Gottschall wrote:
>>> from my point of view powersave should be optional and not forced.
>>> 
>>> consider :
>>> iw dev <devname> set power_save on/off
>>> 
>>> so there is already a config option made for that purpose,
>> 
>> Thanks Sebastian for your review. Agree this should not be forced with
>> in driver.
>> 
>> I will check if i can set the idle ps at the end of
>> ath10k_mac_vif_setup_ps by checking arvif->ps.
>> I will update the patch accordingly.
> 
> So what power save is this exactly? I assumed it's some bus level power
> save (turning of clocks etc) but is it actually 802.11 power save?

Yes this is WIFI chip set level power-save(based on idleness) and not 
related to protocol power save.
FW will turn off/scale down the resources(clock/rails) based on 
opportunity(when ever idle mode is detected).
This power save is mostly done in disconnected state. I am not really 
sure when framework/user-space
triggers power-save config(iw dev <devname> set power_save on/off).
Then doing this from user-space will be unnecessarily toggling this 
config or may not be setting at disconnected state.

May be you can suggest better here.

BR,
Govind

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
  2018-04-19  4:32           ` govinds
@ 2018-04-19 16:36             ` Kalle Valo
  -1 siblings, 0 replies; 44+ messages in thread
From: Kalle Valo @ 2018-04-19 16:36 UTC (permalink / raw)
  To: govinds
  Cc: pillair, linux-wireless, ath10k, Sebastian Gottschall, arend.vanspriel

govinds@codeaurora.org writes:

> On 2018-04-18 18:46, Kalle Valo wrote:
>> (fixing top posting)
>>
>> govinds@codeaurora.org writes:
>>
>>> On 2018-04-18 12:36, Sebastian Gottschall wrote:
>>>> from my point of view powersave should be optional and not forced.
>>>>
>>>> consider :
>>>> iw dev <devname> set power_save on/off
>>>>
>>>> so there is already a config option made for that purpose,
>>>
>>> Thanks Sebastian for your review. Agree this should not be forced with
>>> in driver.
>>>
>>> I will check if i can set the idle ps at the end of
>>> ath10k_mac_vif_setup_ps by checking arvif->ps.
>>> I will update the patch accordingly.
>>
>> So what power save is this exactly? I assumed it's some bus level power
>> save (turning of clocks etc) but is it actually 802.11 power save?
>
> Yes this is WIFI chip set level power-save(based on idleness) and not
> related to protocol power save. FW will turn off/scale down the
> resources(clock/rails) based on opportunity(when ever idle mode is
> detected). This power save is mostly done in disconnected state. I am
> not really sure when framework/user-space triggers power-save
> config(iw dev <devname> set power_save on/off). Then doing this from
> user-space will be unnecessarily toggling this config or may not be
> setting at disconnected state.

I think that 'set power_save' commands affects struct
ieee80211_bss_conf::ps parameter and I don't think it should be used in
this case. We already have ath10k_config_ps() for 802.11 level of power
saving.

Arend again proposed runtime-pm with which I'm not very familiar. But
why would we want to disable this? Doesn't it make sense to have this
feature always enabled to save power? wcn3990 is a chip for a mobile
device anyway.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
@ 2018-04-19 16:36             ` Kalle Valo
  0 siblings, 0 replies; 44+ messages in thread
From: Kalle Valo @ 2018-04-19 16:36 UTC (permalink / raw)
  To: govinds
  Cc: linux-wireless, arend.vanspriel, pillair, Sebastian Gottschall, ath10k

govinds@codeaurora.org writes:

> On 2018-04-18 18:46, Kalle Valo wrote:
>> (fixing top posting)
>>
>> govinds@codeaurora.org writes:
>>
>>> On 2018-04-18 12:36, Sebastian Gottschall wrote:
>>>> from my point of view powersave should be optional and not forced.
>>>>
>>>> consider :
>>>> iw dev <devname> set power_save on/off
>>>>
>>>> so there is already a config option made for that purpose,
>>>
>>> Thanks Sebastian for your review. Agree this should not be forced with
>>> in driver.
>>>
>>> I will check if i can set the idle ps at the end of
>>> ath10k_mac_vif_setup_ps by checking arvif->ps.
>>> I will update the patch accordingly.
>>
>> So what power save is this exactly? I assumed it's some bus level power
>> save (turning of clocks etc) but is it actually 802.11 power save?
>
> Yes this is WIFI chip set level power-save(based on idleness) and not
> related to protocol power save. FW will turn off/scale down the
> resources(clock/rails) based on opportunity(when ever idle mode is
> detected). This power save is mostly done in disconnected state. I am
> not really sure when framework/user-space triggers power-save
> config(iw dev <devname> set power_save on/off). Then doing this from
> user-space will be unnecessarily toggling this config or may not be
> setting at disconnected state.

I think that 'set power_save' commands affects struct
ieee80211_bss_conf::ps parameter and I don't think it should be used in
this case. We already have ath10k_config_ps() for 802.11 level of power
saving.

Arend again proposed runtime-pm with which I'm not very familiar. But
why would we want to disable this? Doesn't it make sense to have this
feature always enabled to save power? wcn3990 is a chip for a mobile
device anyway.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
  2018-04-19 16:36             ` Kalle Valo
@ 2018-04-19 17:00               ` Adrian Chadd
  -1 siblings, 0 replies; 44+ messages in thread
From: Adrian Chadd @ 2018-04-19 17:00 UTC (permalink / raw)
  To: Kalle Valo
  Cc: govinds, pillair, linux-wireless, ath10k, Sebastian Gottschall,
	Arend Van Spriel

Hi,

My 2c here.

As much as I like power save stuff, I've been bitten enough times by
these wifi chips kinda implementing
mostly-ok-but-not-that-particular-time power savings stuff and so have
had to make it configurable chip by chip.

A good example is active state power management in various runtime and
idle modes on different chips, how it intersects with things like
bluetooth, btcoex, is the bios buggy, etc, etc.

So I'd at least like a way to control that feature as a module load
and/or debugfs entry to aide users trying to figure out why chips
disappear or behave erratically. I'm ok with it being turned on but
I'm not ok with a debugging step being "oh yeah, so just go comment
this out and recompile", especially if people are debugging on an
embedded target where it's not easily self hosted or easy to rebuild
things.




-adrian

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
@ 2018-04-19 17:00               ` Adrian Chadd
  0 siblings, 0 replies; 44+ messages in thread
From: Adrian Chadd @ 2018-04-19 17:00 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Arend Van Spriel, govinds, linux-wireless, Sebastian Gottschall,
	pillair, ath10k

Hi,

My 2c here.

As much as I like power save stuff, I've been bitten enough times by
these wifi chips kinda implementing
mostly-ok-but-not-that-particular-time power savings stuff and so have
had to make it configurable chip by chip.

A good example is active state power management in various runtime and
idle modes on different chips, how it intersects with things like
bluetooth, btcoex, is the bios buggy, etc, etc.

So I'd at least like a way to control that feature as a module load
and/or debugfs entry to aide users trying to figure out why chips
disappear or behave erratically. I'm ok with it being turned on but
I'm not ok with a debugging step being "oh yeah, so just go comment
this out and recompile", especially if people are debugging on an
embedded target where it's not easily self hosted or easy to rebuild
things.




-adrian

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
  2018-04-19 16:36             ` Kalle Valo
@ 2018-04-20  7:21               ` Sebastian Gottschall
  -1 siblings, 0 replies; 44+ messages in thread
From: Sebastian Gottschall @ 2018-04-20  7:21 UTC (permalink / raw)
  To: Kalle Valo, govinds; +Cc: pillair, linux-wireless, ath10k, arend.vanspriel


>> Yes this is WIFI chip set level power-save(based on idleness) and not
>> related to protocol power save. FW will turn off/scale down the
>> resources(clock/rails) based on opportunity(when ever idle mode is
>> detected). This power save is mostly done in disconnected state. I am
>> not really sure when framework/user-space triggers power-save
>> config(iw dev <devname> set power_save on/off). Then doing this from
>> user-space will be unnecessarily toggling this config or may not be
>> setting at disconnected state.
> I think that 'set power_save' commands affects struct
> ieee80211_bss_conf::ps parameter and I don't think it should be used in
> this case. We already have ath10k_config_ps() for 802.11 level of power
> saving.
>
> Arend again proposed runtime-pm with which I'm not very familiar. But
> why would we want to disable this? Doesn't it make sense to have this
> feature always enabled to save power? wcn3990 is a chip for a mobile
> device anyway.
it might be made for mobile devices but who knows how it is used by the 
market.

Sebastian
>

-- 
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
@ 2018-04-20  7:21               ` Sebastian Gottschall
  0 siblings, 0 replies; 44+ messages in thread
From: Sebastian Gottschall @ 2018-04-20  7:21 UTC (permalink / raw)
  To: Kalle Valo, govinds; +Cc: linux-wireless, arend.vanspriel, pillair, ath10k


>> Yes this is WIFI chip set level power-save(based on idleness) and not
>> related to protocol power save. FW will turn off/scale down the
>> resources(clock/rails) based on opportunity(when ever idle mode is
>> detected). This power save is mostly done in disconnected state. I am
>> not really sure when framework/user-space triggers power-save
>> config(iw dev <devname> set power_save on/off). Then doing this from
>> user-space will be unnecessarily toggling this config or may not be
>> setting at disconnected state.
> I think that 'set power_save' commands affects struct
> ieee80211_bss_conf::ps parameter and I don't think it should be used in
> this case. We already have ath10k_config_ps() for 802.11 level of power
> saving.
>
> Arend again proposed runtime-pm with which I'm not very familiar. But
> why would we want to disable this? Doesn't it make sense to have this
> feature always enabled to save power? wcn3990 is a chip for a mobile
> device anyway.
it might be made for mobile devices but who knows how it is used by the 
market.

Sebastian
>

-- 
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
  2018-04-20  7:21               ` Sebastian Gottschall
@ 2018-04-20  8:16                 ` Arend van Spriel
  -1 siblings, 0 replies; 44+ messages in thread
From: Arend van Spriel @ 2018-04-20  8:16 UTC (permalink / raw)
  To: Sebastian Gottschall, Kalle Valo, govinds; +Cc: pillair, linux-wireless, ath10k

On 4/20/2018 9:21 AM, Sebastian Gottschall wrote:
>
>>> Yes this is WIFI chip set level power-save(based on idleness) and not
>>> related to protocol power save. FW will turn off/scale down the
>>> resources(clock/rails) based on opportunity(when ever idle mode is
>>> detected). This power save is mostly done in disconnected state. I am
>>> not really sure when framework/user-space triggers power-save
>>> config(iw dev <devname> set power_save on/off). Then doing this from
>>> user-space will be unnecessarily toggling this config or may not be
>>> setting at disconnected state.
>> I think that 'set power_save' commands affects struct
>> ieee80211_bss_conf::ps parameter and I don't think it should be used in
>> this case. We already have ath10k_config_ps() for 802.11 level of power
>> saving.
>>
>> Arend again proposed runtime-pm with which I'm not very familiar. But
>> why would we want to disable this? Doesn't it make sense to have this
>> feature always enabled to save power? wcn3990 is a chip for a mobile
>> device anyway.
> it might be made for mobile devices but who knows how it is used by the
> market.

Reading the explanation above it does not make sense to use runtime-pm. 
That would only come into play if the host driver would actually control 
the resources being turned off/scaled down.

So this is purely reducing power-consumption of the chip. However, it 
would be good to know the consequences in terms of responsiveness to 
firmware commands for instance when requesting a scan operation. Another 
thing to consider is to provide user-space with possibility to change 
this configuration (maybe through debugfs?).

Regards,
Arend

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
@ 2018-04-20  8:16                 ` Arend van Spriel
  0 siblings, 0 replies; 44+ messages in thread
From: Arend van Spriel @ 2018-04-20  8:16 UTC (permalink / raw)
  To: Sebastian Gottschall, Kalle Valo, govinds; +Cc: linux-wireless, pillair, ath10k

On 4/20/2018 9:21 AM, Sebastian Gottschall wrote:
>
>>> Yes this is WIFI chip set level power-save(based on idleness) and not
>>> related to protocol power save. FW will turn off/scale down the
>>> resources(clock/rails) based on opportunity(when ever idle mode is
>>> detected). This power save is mostly done in disconnected state. I am
>>> not really sure when framework/user-space triggers power-save
>>> config(iw dev <devname> set power_save on/off). Then doing this from
>>> user-space will be unnecessarily toggling this config or may not be
>>> setting at disconnected state.
>> I think that 'set power_save' commands affects struct
>> ieee80211_bss_conf::ps parameter and I don't think it should be used in
>> this case. We already have ath10k_config_ps() for 802.11 level of power
>> saving.
>>
>> Arend again proposed runtime-pm with which I'm not very familiar. But
>> why would we want to disable this? Doesn't it make sense to have this
>> feature always enabled to save power? wcn3990 is a chip for a mobile
>> device anyway.
> it might be made for mobile devices but who knows how it is used by the
> market.

Reading the explanation above it does not make sense to use runtime-pm. 
That would only come into play if the host driver would actually control 
the resources being turned off/scaled down.

So this is purely reducing power-consumption of the chip. However, it 
would be good to know the consequences in terms of responsiveness to 
firmware commands for instance when requesting a scan operation. Another 
thing to consider is to provide user-space with possibility to change 
this configuration (maybe through debugfs?).

Regards,
Arend

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
  2018-04-20  8:16                 ` Arend van Spriel
@ 2018-04-20 12:43                   ` govinds
  -1 siblings, 0 replies; 44+ messages in thread
From: govinds @ 2018-04-20 12:43 UTC (permalink / raw)
  To: Arend van Spriel
  Cc: Sebastian Gottschall, Kalle Valo, pillair, linux-wireless, ath10k

On 2018-04-20 13:46, Arend van Spriel wrote:
> On 4/20/2018 9:21 AM, Sebastian Gottschall wrote:
>> 
>>>> Yes this is WIFI chip set level power-save(based on idleness) and 
>>>> not
>>>> related to protocol power save. FW will turn off/scale down the
>>>> resources(clock/rails) based on opportunity(when ever idle mode is
>>>> detected). This power save is mostly done in disconnected state. I 
>>>> am
>>>> not really sure when framework/user-space triggers power-save
>>>> config(iw dev <devname> set power_save on/off). Then doing this from
>>>> user-space will be unnecessarily toggling this config or may not be
>>>> setting at disconnected state.
>>> I think that 'set power_save' commands affects struct
>>> ieee80211_bss_conf::ps parameter and I don't think it should be used 
>>> in
>>> this case. We already have ath10k_config_ps() for 802.11 level of 
>>> power
>>> saving.
>>> 
>>> Arend again proposed runtime-pm with which I'm not very familiar. But
>>> why would we want to disable this? Doesn't it make sense to have this
>>> feature always enabled to save power? wcn3990 is a chip for a mobile
>>> device anyway.
>> it might be made for mobile devices but who knows how it is used by 
>> the
>> market.

I guess having this enabled by default is safe as 
WMI_PDEV_PARAM_UNSUPPORTED
protects for those version which does not support this pdev param.


> Reading the explanation above it does not make sense to use
> runtime-pm. That would only come into play if the host driver would
> actually control the resources being turned off/scaled down.
> 
> So this is purely reducing power-consumption of the chip. However, it
> would be good to know the consequences in terms of responsiveness to
> firmware commands for instance when requesting a scan operation.

Exit latency is around 8-10 ms, so i guess this delta should be ok.

> Another thing to consider is to provide user-space with possibility to
> change this configuration (maybe through debugfs?).


In case any one wants to measure power no's with/without this config, 
just giving
provision to disable may be useful.


BR,
Govind

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
@ 2018-04-20 12:43                   ` govinds
  0 siblings, 0 replies; 44+ messages in thread
From: govinds @ 2018-04-20 12:43 UTC (permalink / raw)
  To: Arend van Spriel
  Cc: linux-wireless, ath10k, pillair, Kalle Valo, Sebastian Gottschall

On 2018-04-20 13:46, Arend van Spriel wrote:
> On 4/20/2018 9:21 AM, Sebastian Gottschall wrote:
>> 
>>>> Yes this is WIFI chip set level power-save(based on idleness) and 
>>>> not
>>>> related to protocol power save. FW will turn off/scale down the
>>>> resources(clock/rails) based on opportunity(when ever idle mode is
>>>> detected). This power save is mostly done in disconnected state. I 
>>>> am
>>>> not really sure when framework/user-space triggers power-save
>>>> config(iw dev <devname> set power_save on/off). Then doing this from
>>>> user-space will be unnecessarily toggling this config or may not be
>>>> setting at disconnected state.
>>> I think that 'set power_save' commands affects struct
>>> ieee80211_bss_conf::ps parameter and I don't think it should be used 
>>> in
>>> this case. We already have ath10k_config_ps() for 802.11 level of 
>>> power
>>> saving.
>>> 
>>> Arend again proposed runtime-pm with which I'm not very familiar. But
>>> why would we want to disable this? Doesn't it make sense to have this
>>> feature always enabled to save power? wcn3990 is a chip for a mobile
>>> device anyway.
>> it might be made for mobile devices but who knows how it is used by 
>> the
>> market.

I guess having this enabled by default is safe as 
WMI_PDEV_PARAM_UNSUPPORTED
protects for those version which does not support this pdev param.


> Reading the explanation above it does not make sense to use
> runtime-pm. That would only come into play if the host driver would
> actually control the resources being turned off/scaled down.
> 
> So this is purely reducing power-consumption of the chip. However, it
> would be good to know the consequences in terms of responsiveness to
> firmware commands for instance when requesting a scan operation.

Exit latency is around 8-10 ms, so i guess this delta should be ok.

> Another thing to consider is to provide user-space with possibility to
> change this configuration (maybe through debugfs?).


In case any one wants to measure power no's with/without this config, 
just giving
provision to disable may be useful.


BR,
Govind

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
  2018-04-20  8:16                 ` Arend van Spriel
@ 2018-04-23 15:50                   ` Kalle Valo
  -1 siblings, 0 replies; 44+ messages in thread
From: Kalle Valo @ 2018-04-23 15:50 UTC (permalink / raw)
  To: Arend van Spriel
  Cc: Sebastian Gottschall, govinds, pillair, linux-wireless, ath10k

Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 4/20/2018 9:21 AM, Sebastian Gottschall wrote:
>>
>>>> Yes this is WIFI chip set level power-save(based on idleness) and not
>>>> related to protocol power save. FW will turn off/scale down the
>>>> resources(clock/rails) based on opportunity(when ever idle mode is
>>>> detected). This power save is mostly done in disconnected state. I am
>>>> not really sure when framework/user-space triggers power-save
>>>> config(iw dev <devname> set power_save on/off). Then doing this from
>>>> user-space will be unnecessarily toggling this config or may not be
>>>> setting at disconnected state.
>>> I think that 'set power_save' commands affects struct
>>> ieee80211_bss_conf::ps parameter and I don't think it should be used in
>>> this case. We already have ath10k_config_ps() for 802.11 level of power
>>> saving.
>>>
>>> Arend again proposed runtime-pm with which I'm not very familiar. But
>>> why would we want to disable this? Doesn't it make sense to have this
>>> feature always enabled to save power? wcn3990 is a chip for a mobile
>>> device anyway.
>>
>> it might be made for mobile devices but who knows how it is used by the
>> market.

Sure, and this does parameter prevent that. You can still connect
wcn3990 to any power source you want, but the driver is just optimised
power consumption in focus.

> Reading the explanation above it does not make sense to use
> runtime-pm. That would only come into play if the host driver would
> actually control the resources being turned off/scaled down.

Exactly.

> So this is purely reducing power-consumption of the chip. However, it
> would be good to know the consequences in terms of responsiveness to
> firmware commands for instance when requesting a scan operation.
> Another thing to consider is to provide user-space with possibility to
> change this configuration (maybe through debugfs?).

Adrian was also commenting something similar about adding a debugfs
interface but I don't really see the point right now while we are adding
initial wcn3990 support. If someone wants to run measurements with and
without this parameter it's very easy to edit it out from the sources.
Later on we can consider making it dynamic if there's really the need
for that, but I'm very skeptic that anyone would even need that.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
@ 2018-04-23 15:50                   ` Kalle Valo
  0 siblings, 0 replies; 44+ messages in thread
From: Kalle Valo @ 2018-04-23 15:50 UTC (permalink / raw)
  To: Arend van Spriel
  Cc: linux-wireless, govinds, pillair, ath10k, Sebastian Gottschall

Arend van Spriel <arend.vanspriel@broadcom.com> writes:

> On 4/20/2018 9:21 AM, Sebastian Gottschall wrote:
>>
>>>> Yes this is WIFI chip set level power-save(based on idleness) and not
>>>> related to protocol power save. FW will turn off/scale down the
>>>> resources(clock/rails) based on opportunity(when ever idle mode is
>>>> detected). This power save is mostly done in disconnected state. I am
>>>> not really sure when framework/user-space triggers power-save
>>>> config(iw dev <devname> set power_save on/off). Then doing this from
>>>> user-space will be unnecessarily toggling this config or may not be
>>>> setting at disconnected state.
>>> I think that 'set power_save' commands affects struct
>>> ieee80211_bss_conf::ps parameter and I don't think it should be used in
>>> this case. We already have ath10k_config_ps() for 802.11 level of power
>>> saving.
>>>
>>> Arend again proposed runtime-pm with which I'm not very familiar. But
>>> why would we want to disable this? Doesn't it make sense to have this
>>> feature always enabled to save power? wcn3990 is a chip for a mobile
>>> device anyway.
>>
>> it might be made for mobile devices but who knows how it is used by the
>> market.

Sure, and this does parameter prevent that. You can still connect
wcn3990 to any power source you want, but the driver is just optimised
power consumption in focus.

> Reading the explanation above it does not make sense to use
> runtime-pm. That would only come into play if the host driver would
> actually control the resources being turned off/scaled down.

Exactly.

> So this is purely reducing power-consumption of the chip. However, it
> would be good to know the consequences in terms of responsiveness to
> firmware commands for instance when requesting a scan operation.
> Another thing to consider is to provide user-space with possibility to
> change this configuration (maybe through debugfs?).

Adrian was also commenting something similar about adding a debugfs
interface but I don't really see the point right now while we are adding
initial wcn3990 support. If someone wants to run measurements with and
without this parameter it's very easy to edit it out from the sources.
Later on we can consider making it dynamic if there's really the need
for that, but I'm very skeptic that anyone would even need that.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* 9984 with 10.4.3.5.3-0057 crashes
  2018-04-23 15:50                   ` Kalle Valo
  (?)
@ 2018-04-23 16:17                   ` Sebastian Gottschall
  2018-04-24  5:27                     ` Kalle Valo
  -1 siblings, 1 reply; 44+ messages in thread
From: Sebastian Gottschall @ 2018-04-23 16:17 UTC (permalink / raw)
  To: Kalle Valo, ath10k

just for your notice. 10.4.3.5.3-0057 on 9984 which was just released 
crashes in vht160 operation mode immediatly after first station associates
last known working stable fw so far is 10.4-3.4-00104. the whole 
10.4.3.5.3 series seem to be seriously broken or the api has been 
changed in a way
which is unsupported by ath10k (which i think is the cause of the problem)
it would be good to know what has been changed.

[   54.170242] ath10k_pci 0001:03:00.0: firmware crashed! (uuid n/a)
[   54.176337] ath10k_pci 0001:03:00.0: qca9984/qca9994 hw1.0 target 
0x01000000 chip_id 0x00000000 sub 168c:cafe
[   54.186237] ath10k_pci 0001:03:00.0: kconfig debug 1 debugfs 1 
tracing 0 dfs 0 testmode 0
[   54.195264] ath10k_pci 0001:03:00.0: firmware ver 10.4-3.5.3-00057 
api 5 features 
no-p2p,mfp,peer-flow-ctrl,btcoex-param,allows-mesh-bcast,no-ps crc32 
f10fd4d3
[   54.209521] ath10k_pci 0001:03:00.0: board_file api 1 bmi_id N/A 
crc32 31ab2fd1
[   54.216823] ath10k_pci 0001:03:00.0: htt-ver 2.2 wmi-op 6 htt-op 4 
cal otp max-sta 512 raw 0 hwcrypto 1
[   54.228223] ath10k_pci 0001:03:00.0: firmware register dump:
[   54.233879] ath10k_pci 0001:03:00.0: [00]: 0x0000000A 0x000015B3 
0x0A0014F9 0x00975B31
[   54.241778] ath10k_pci 0001:03:00.0: [04]: 0x0A0014F9 0x00060B30 
0x0000001D 0x004461F0
[   54.249685] ath10k_pci 0001:03:00.0: [08]: 0x0042E980 0x0045E028 
0x000EE6CC 0x00000001
[   54.257591] ath10k_pci 0001:03:00.0: [12]: 0x00000009 0x00000000 
0x00973ABC 0x00973AD2
[   54.265498] ath10k_pci 0001:03:00.0: [16]: 0x00973AB0 0x009C378C 
0x009606CA 0x00000000
[   54.273406] ath10k_pci 0001:03:00.0: [20]: 0x409C9609 0x0040673C 
0x00400000 0x0000000E
[   54.281304] ath10k_pci 0001:03:00.0: [24]: 0x809805BF 0x0040679C 
0x00000095 0xC09C9609
[   54.289210] ath10k_pci 0001:03:00.0: [28]: 0x809CBCC5 0x004068BC 
0x00000000 0x00009026
[   54.297115] ath10k_pci 0001:03:00.0: [32]: 0x809B7303 0x004068EC 
0x000EE6CC 0x0042B988
[   54.305025] ath10k_pci 0001:03:00.0: [36]: 0x809B3A46 0x0040692C 
0x00000002 0x0042B988
[   54.312935] ath10k_pci 0001:03:00.0: [40]: 0x809B3123 0x0040695C 
0x00406980 0x00423B7C
[   54.320834] ath10k_pci 0001:03:00.0: [44]: 0x8096EE0E 0x0040697C 
0x000EE6C0 0x00000001
[   54.328740] ath10k_pci 0001:03:00.0: [48]: 0x8096F883 0x00406A2C 
0x0042D570 0x004241A4
[   54.336646] ath10k_pci 0001:03:00.0: [52]: 0x80963A26 0x00406A4C 
0x0042D570 0x00405528
[   54.344552] ath10k_pci 0001:03:00.0: [56]: 0x80960E80 0x00406A9C 
0x0000000D 0x00400000
[   54.352458] ath10k_pci 0001:03:00.0: Copy Engine register dump:
[   54.358371] ath10k_pci 0001:03:00.0: [00]: 0x0004a000  15  15 3   3
[   54.364809] ath10k_pci 0001:03:00.0: [01]: 0x0004a400  23  23 90  91
[   54.371240] ath10k_pci 0001:03:00.0: [02]: 0x0004a800   0   0 63   0
[   54.377679] ath10k_pci 0001:03:00.0: [03]: 0x0004ac00  12  12 13  12
[   54.384118] ath10k_pci 0001:03:00.0: [04]: 0x0004b000  19  19 59  19
[   54.390548] ath10k_pci 0001:03:00.0: [05]: 0x0004b400  24  24 55  56
[   54.396986] ath10k_pci 0001:03:00.0: [06]: 0x0004b800  23  23 23  23
[   54.403425] ath10k_pci 0001:03:00.0: [07]: 0x0004bc00   1   1 1   1
[   54.409857] ath10k_pci 0001:03:00.0: [08]: 0x0004c000   0   0 127   0
[   54.416295] ath10k_pci 0001:03:00.0: [09]: 0x0004c400   1   1 1   1
[   54.422736] ath10k_pci 0001:03:00.0: [10]: 0x0004c800   0   0 0   0
[   54.429171] ath10k_pci 0001:03:00.0: [11]: 0x0004cc00   0   0 0   0
[   57.191977] ath10k_pci 0001:03:00.0: failed to setup peer SMPS for 
vdev 0: -11
[   57.199187] ath10k_pci 0001:03:00.0: failed to associate station 
14:91:82:b9:16:d4 for vdev 0: -11
[   60.231976] ath10k_pci 0001:03:00.0: failed to delete peer 
14:91:82:b9:16:d4 for vdev 0: -11
[   60.240441] ath10k_pci 0001:03:00.0: found sta peer 14:91:82:b9:16:d4 
(ptr ede98200 id 3) entry on vdev 0 after it was supposedly removed
[   60.252989] ath10k_pci 0001:03:00.0: failed to read temperature -11


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
  2018-04-23 15:50                   ` Kalle Valo
@ 2018-04-23 16:41                     ` Adrian Chadd
  -1 siblings, 0 replies; 44+ messages in thread
From: Adrian Chadd @ 2018-04-23 16:41 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Arend van Spriel, Sebastian Gottschall, govinds, pillair,
	linux-wireless, ath10k

On 23 April 2018 at 08:50, Kalle Valo <kvalo@codeaurora.org> wrote:

>
> Adrian was also commenting something similar about adding a debugfs
> interface but I don't really see the point right now while we are adding
> initial wcn3990 support. If someone wants to run measurements with and
> without this parameter it's very easy to edit it out from the sources.
> Later on we can consider making it dynamic if there's really the need
> for that, but I'm very skeptic that anyone would even need that.

Hi!

We have at ${WORK}, because we've found various devices (atheros and
qualcomm and QCA and otherwise) have this nasty habit of trying to
guess when a good time to clock-gate down is, but we're busy doing low
latency audio/video.

10, 20ms may not be a lot of latency for bulk data but that's a half
to one of a video frame in my world. :-)



-adrian

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
@ 2018-04-23 16:41                     ` Adrian Chadd
  0 siblings, 0 replies; 44+ messages in thread
From: Adrian Chadd @ 2018-04-23 16:41 UTC (permalink / raw)
  To: Kalle Valo
  Cc: Arend van Spriel, govinds, linux-wireless, ath10k, pillair,
	Sebastian Gottschall

On 23 April 2018 at 08:50, Kalle Valo <kvalo@codeaurora.org> wrote:

>
> Adrian was also commenting something similar about adding a debugfs
> interface but I don't really see the point right now while we are adding
> initial wcn3990 support. If someone wants to run measurements with and
> without this parameter it's very easy to edit it out from the sources.
> Later on we can consider making it dynamic if there's really the need
> for that, but I'm very skeptic that anyone would even need that.

Hi!

We have at ${WORK}, because we've found various devices (atheros and
qualcomm and QCA and otherwise) have this nasty habit of trying to
guess when a good time to clock-gate down is, but we're busy doing low
latency audio/video.

10, 20ms may not be a lot of latency for bulk data but that's a half
to one of a video frame in my world. :-)



-adrian

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: 9984 with 10.4.3.5.3-0057 crashes
  2018-04-23 16:17                   ` 9984 with 10.4.3.5.3-0057 crashes Sebastian Gottschall
@ 2018-04-24  5:27                     ` Kalle Valo
  2018-04-24  6:08                       ` Sebastian Gottschall
  0 siblings, 1 reply; 44+ messages in thread
From: Kalle Valo @ 2018-04-24  5:27 UTC (permalink / raw)
  To: Sebastian Gottschall; +Cc: ath10k

Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:

> just for your notice. 10.4.3.5.3-0057 on 9984 which was just released
> crashes in vht160 operation mode immediatly after first station
> associates
> last known working stable fw so far is 10.4-3.4-00104. the whole
> 10.4.3.5.3 series seem to be seriously broken or the api has been
> changed in a way
> which is unsupported by ath10k (which i think is the cause of the problem)
> it would be good to know what has been changed.

At least I was not informed any changes in the firmware interface. What
version of ath10k are you using? Do you have any custom patches on
ath10k?

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
  2018-04-23 16:41                     ` Adrian Chadd
@ 2018-04-24  5:35                       ` Kalle Valo
  -1 siblings, 0 replies; 44+ messages in thread
From: Kalle Valo @ 2018-04-24  5:35 UTC (permalink / raw)
  To: Adrian Chadd
  Cc: Arend van Spriel, Sebastian Gottschall, govinds, pillair,
	linux-wireless, ath10k

Adrian Chadd <adrian@freebsd.org> writes:

> On 23 April 2018 at 08:50, Kalle Valo <kvalo@codeaurora.org> wrote:
>
>>
>> Adrian was also commenting something similar about adding a debugfs
>> interface but I don't really see the point right now while we are adding
>> initial wcn3990 support. If someone wants to run measurements with and
>> without this parameter it's very easy to edit it out from the sources.
>> Later on we can consider making it dynamic if there's really the need
>> for that, but I'm very skeptic that anyone would even need that.
>
> Hi!
>
> We have at ${WORK}, because we've found various devices (atheros and
> qualcomm and QCA and otherwise) have this nasty habit of trying to
> guess when a good time to clock-gate down is, but we're busy doing low
> latency audio/video.
>
> 10, 20ms may not be a lot of latency for bulk data but that's a half
> to one of a video frame in my world. :-)

Sure, I get that. But we should discuss making a setting like this
dynamic once we have a test case and numbers showing that it's needed,
otherwise it's just unnecessary cruft.

-- 
Kalle Valo

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [PATCH v2 4/4] ath10k: Enable sta idle power save
@ 2018-04-24  5:35                       ` Kalle Valo
  0 siblings, 0 replies; 44+ messages in thread
From: Kalle Valo @ 2018-04-24  5:35 UTC (permalink / raw)
  To: Adrian Chadd
  Cc: Arend van Spriel, govinds, linux-wireless, ath10k, pillair,
	Sebastian Gottschall

Adrian Chadd <adrian@freebsd.org> writes:

> On 23 April 2018 at 08:50, Kalle Valo <kvalo@codeaurora.org> wrote:
>
>>
>> Adrian was also commenting something similar about adding a debugfs
>> interface but I don't really see the point right now while we are adding
>> initial wcn3990 support. If someone wants to run measurements with and
>> without this parameter it's very easy to edit it out from the sources.
>> Later on we can consider making it dynamic if there's really the need
>> for that, but I'm very skeptic that anyone would even need that.
>
> Hi!
>
> We have at ${WORK}, because we've found various devices (atheros and
> qualcomm and QCA and otherwise) have this nasty habit of trying to
> guess when a good time to clock-gate down is, but we're busy doing low
> latency audio/video.
>
> 10, 20ms may not be a lot of latency for bulk data but that's a half
> to one of a video frame in my world. :-)

Sure, I get that. But we should discuss making a setting like this
dynamic once we have a test case and numbers showing that it's needed,
otherwise it's just unnecessary cruft.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [v2,1/4] ath10k: Add hw params for shadow register support
  2018-04-17 12:06   ` pillair
  (?)
  (?)
@ 2018-04-24  6:05   ` Kalle Valo
  -1 siblings, 0 replies; 44+ messages in thread
From: Kalle Valo @ 2018-04-24  6:05 UTC (permalink / raw)
  To: Rakesh Pillai; +Cc: ath10k, linux-wireless, Rakesh Pillai

Rakesh Pillai <pillair@codeaurora.org> wrote:

> wcn3990 supports shadow register for ce write.
> 
> Add a hw param for shadow register support.
> 
> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

4 patches applied to ath-next branch of ath.git, thanks.

b2e40d7ab8e2 ath10k: add hw params for shadow register support
b7ba83f7c414 ath10k: add support for shadow register for WNC3990
4945af5b264f ath10k: enable SRRI/DRRI support on ddr for WCN3990
d5cded16fdc0 ath10k: enable sta idle power save

-- 
https://patchwork.kernel.org/patch/10344857/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: [v2,1/4] ath10k: Add hw params for shadow register support
  2018-04-17 12:06   ` pillair
  (?)
@ 2018-04-24  6:05   ` Kalle Valo
  -1 siblings, 0 replies; 44+ messages in thread
From: Kalle Valo @ 2018-04-24  6:05 UTC (permalink / raw)
  To: Rakesh Pillai; +Cc: linux-wireless, ath10k

Rakesh Pillai <pillair@codeaurora.org> wrote:

> wcn3990 supports shadow register for ce write.
> 
> Add a hw param for shadow register support.
> 
> Signed-off-by: Rakesh Pillai <pillair@codeaurora.org>
> Signed-off-by: Kalle Valo <kvalo@codeaurora.org>

4 patches applied to ath-next branch of ath.git, thanks.

b2e40d7ab8e2 ath10k: add hw params for shadow register support
b7ba83f7c414 ath10k: add support for shadow register for WNC3990
4945af5b264f ath10k: enable SRRI/DRRI support on ddr for WCN3990
d5cded16fdc0 ath10k: enable sta idle power save

-- 
https://patchwork.kernel.org/patch/10344857/

https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: 9984 with 10.4.3.5.3-0057 crashes
  2018-04-24  5:27                     ` Kalle Valo
@ 2018-04-24  6:08                       ` Sebastian Gottschall
  2018-04-25 15:23                         ` Kalle Valo
  0 siblings, 1 reply; 44+ messages in thread
From: Sebastian Gottschall @ 2018-04-24  6:08 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath10k

Am 24.04.2018 um 07:27 schrieb Kalle Valo:
> Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:
>
>> just for your notice. 10.4.3.5.3-0057 on 9984 which was just released
>> crashes in vht160 operation mode immediatly after first station
>> associates
>> last known working stable fw so far is 10.4-3.4-00104. the whole
>> 10.4.3.5.3 series seem to be seriously broken or the api has been
>> changed in a way
>> which is unsupported by ath10k (which i think is the cause of the 
>> problem)
>> it would be good to know what has been changed.
> At least I was not informed any changes in the firmware interface. What
> version of ath10k are you using? Do you have any custom patches on
> ath10k?
i'm using latest git version for testing here
but there is one relevant patch required for more recent firmwares like 
3.4 series (see below)
i posted already a long time ago, that vht160 will not work anymore 
without that patch in newer firmwares since
qca changed the api for channel mapping in vht160. (i took this 
information from the propertiery qca driver sourcecodes)
see our discussion on 2.feb 2017 about that required change. it seems 
that you lost the path on that issue.
as far as i know this change was required because of compatiblity issues 
with vht80 clients from some other vendors.
i have seen similar changes in hostapd for mwlwifi driver

--- wmi.c.old   2018-02-16 11:25:13.023117123 +0100
+++ wmi.c       2018-04-24 07:47:12.642395002 +0200
@@ -1661,13 +1661,18 @@ void ath10k_wmi_put_wmi_channel(struct w
                 flags |= WMI_CHAN_FLAG_HT40_PLUS;
         if (arg->chan_radar)
                 flags |= WMI_CHAN_FLAG_DFS;
-
+       ch->band_center_freq2 = 0;
         ch->mhz = __cpu_to_le32(arg->freq);
         ch->band_center_freq1 = __cpu_to_le32(arg->band_center_freq1);
         if (arg->mode == MODE_11AC_VHT80_80)
                 ch->band_center_freq2 = 
__cpu_to_le32(arg->band_center_freq2);
-       else
-               ch->band_center_freq2 = 0;
+       if (arg->mode == MODE_11AC_VHT160)  {
+               if (arg->freq < arg->band_center_freq1)
+                       ch->band_center_freq1 = 
__cpu_to_le32(arg->band_center_freq1 - 40);
+               else
+                       ch->band_center_freq1 = 
__cpu_to_le32(arg->band_center_freq1 + 40);
+               ch->band_center_freq2 = 
__cpu_to_le32(arg->band_center_freq1);
+       }
         ch->min_power = arg->min_power;
         ch->max_power = arg->max_power;
         ch->reg_power = arg->max_reg_power;

>

-- 
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: 9984 with 10.4.3.5.3-0057 crashes
  2018-04-24  6:08                       ` Sebastian Gottschall
@ 2018-04-25 15:23                         ` Kalle Valo
  2018-04-26  6:23                           ` Sebastian Gottschall
  2018-04-26  7:36                           ` Sebastian Gottschall
  0 siblings, 2 replies; 44+ messages in thread
From: Kalle Valo @ 2018-04-25 15:23 UTC (permalink / raw)
  To: Sebastian Gottschall; +Cc: ath10k

Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:

> Am 24.04.2018 um 07:27 schrieb Kalle Valo:
>> Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:
>>
>>> just for your notice. 10.4.3.5.3-0057 on 9984 which was just released
>>> crashes in vht160 operation mode immediatly after first station
>>> associates
>>> last known working stable fw so far is 10.4-3.4-00104. the whole
>>> 10.4.3.5.3 series seem to be seriously broken or the api has been
>>> changed in a way
>>> which is unsupported by ath10k (which i think is the cause of the
>>> problem)
>>> it would be good to know what has been changed.
>>
>> At least I was not informed any changes in the firmware interface. What
>> version of ath10k are you using? Do you have any custom patches on
>> ath10k?
>
> i'm using latest git version for testing here but there is one
> relevant patch required for more recent firmwares like 3.4 series (see
> below) i posted already a long time ago, that vht160 will not work
> anymore without that patch in newer firmwares since qca changed the api
> for channel mapping in vht160.

I don't remember what happened to that patch, but please resubmit it
with a proper commit log and S-o-b line.

I was told that the firmware crash is an assert triggered by host not
populating the parameter 'peer_bw_rxnss_override' in
WMI_PEER_ASSOC_CMDID which is needed for 160/80+80 operation.

-- 
Kalle Valo

_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: 9984 with 10.4.3.5.3-0057 crashes
  2018-04-25 15:23                         ` Kalle Valo
@ 2018-04-26  6:23                           ` Sebastian Gottschall
  2018-04-26  7:36                           ` Sebastian Gottschall
  1 sibling, 0 replies; 44+ messages in thread
From: Sebastian Gottschall @ 2018-04-26  6:23 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath10k

Am 25.04.2018 um 17:23 schrieb Kalle Valo:
> Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:
>
>> Am 24.04.2018 um 07:27 schrieb Kalle Valo:
>>> Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:
>>>
>>>> just for your notice. 10.4.3.5.3-0057 on 9984 which was just released
>>>> crashes in vht160 operation mode immediatly after first station
>>>> associates
>>>> last known working stable fw so far is 10.4-3.4-00104. the whole
>>>> 10.4.3.5.3 series seem to be seriously broken or the api has been
>>>> changed in a way
>>>> which is unsupported by ath10k (which i think is the cause of the
>>>> problem)
>>>> it would be good to know what has been changed.
>>> At least I was not informed any changes in the firmware interface. What
>>> version of ath10k are you using? Do you have any custom patches on
>>> ath10k?
>> i'm using latest git version for testing here but there is one
>> relevant patch required for more recent firmwares like 3.4 series (see
>> below) i posted already a long time ago, that vht160 will not work
>> anymore without that patch in newer firmwares since qca changed the api
>> for channel mapping in vht160.
> I don't remember what happened to that patch, but please resubmit it
> with a proper commit log and S-o-b line.
will do asap.
>
> I was told that the firmware crash is an assert triggered by host not
> populating the parameter 'peer_bw_rxnss_override' in
> WMI_PEER_ASSOC_CMDID which is needed for 160/80+80 operation.
peer_bw_rxnss_override is calculated by taking peers rx_max_rate
if its 1560 peer_bw_rxnss_override will be 2
if its 780 peer_bw_rxnss_override will be 1

so for me this all looks wrong. die parameter for vht160 looks guessed 
based on the rx rate and may result in wrong values.
since the peer is also supplying the bandwidth itself, we dont need to 
guess vht160 operation.
i will try to fix this and will post a fix once its solved

Sebastian
>

-- 
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

* Re: 9984 with 10.4.3.5.3-0057 crashes
  2018-04-25 15:23                         ` Kalle Valo
  2018-04-26  6:23                           ` Sebastian Gottschall
@ 2018-04-26  7:36                           ` Sebastian Gottschall
  1 sibling, 0 replies; 44+ messages in thread
From: Sebastian Gottschall @ 2018-04-26  7:36 UTC (permalink / raw)
  To: Kalle Valo; +Cc: ath10k

Am 25.04.2018 um 17:23 schrieb Kalle Valo:
> Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:
>
>> Am 24.04.2018 um 07:27 schrieb Kalle Valo:
>>> Sebastian Gottschall <s.gottschall@dd-wrt.com> writes:
>>>
>>>> just for your notice. 10.4.3.5.3-0057 on 9984 which was just released
>>>> crashes in vht160 operation mode immediatly after first station
>>>> associates
>>>> last known working stable fw so far is 10.4-3.4-00104. the whole
>>>> 10.4.3.5.3 series seem to be seriously broken or the api has been
>>>> changed in a way
>>>> which is unsupported by ath10k (which i think is the cause of the
>>>> problem)
>>>> it would be good to know what has been changed.
>>> At least I was not informed any changes in the firmware interface. What
>>> version of ath10k are you using? Do you have any custom patches on
>>> ath10k?
>> i'm using latest git version for testing here but there is one
>> relevant patch required for more recent firmwares like 3.4 series (see
>> below) i posted already a long time ago, that vht160 will not work
>> anymore without that patch in newer firmwares since qca changed the api
>> for channel mapping in vht160.
> I don't remember what happened to that patch, but please resubmit it
> with a proper commit log and S-o-b line.
>
> I was told that the firmware crash is an assert triggered by host not
> populating the parameter 'peer_bw_rxnss_override' in
> WMI_PEER_ASSOC_CMDID which is needed for 160/80+80 operation.
good news. your hint helped me to develop a fix. will post the patch 
soon here


-- 
Mit freundlichen Grüssen / Regards

Sebastian Gottschall / CTO

NewMedia-NET GmbH - DD-WRT
Firmensitz:  Stubenwaldallee 21a, 64625 Bensheim
Registergericht: Amtsgericht Darmstadt, HRB 25473
Geschäftsführer: Peter Steinhäuser, Christian Scheele
http://www.dd-wrt.com
email: s.gottschall@dd-wrt.com
Tel.: +496251-582650 / Fax: +496251-5826565


_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k

^ permalink raw reply	[flat|nested] 44+ messages in thread

end of thread, other threads:[~2018-04-26  7:37 UTC | newest]

Thread overview: 44+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-17 12:06 [PATCH v2 0/4] Support for STA idle mode power save(IMPS) pillair
2018-04-17 12:06 ` pillair
2018-04-17 12:06 ` [PATCH v2 1/4] ath10k: Add hw params for shadow register support pillair
2018-04-17 12:06   ` pillair
2018-04-24  6:05   ` [v2,1/4] " Kalle Valo
2018-04-24  6:05   ` Kalle Valo
2018-04-17 12:06 ` [PATCH v2 2/4] ath10k: Add support for shadow register for WNC3990 pillair
2018-04-17 12:06   ` pillair
2018-04-17 12:07 ` [PATCH v2 3/4] ath10k: Enable SRRI/DRRI support on ddr for WCN3990 pillair
2018-04-17 12:07   ` pillair
2018-04-17 12:07 ` [PATCH v2 4/4] ath10k: Enable sta idle power save pillair
2018-04-17 12:07   ` pillair
2018-04-18  7:06   ` Sebastian Gottschall
2018-04-18  7:06     ` Sebastian Gottschall
2018-04-18 13:07     ` govinds
2018-04-18 13:07       ` govinds
2018-04-18 13:16       ` Kalle Valo
2018-04-18 13:16         ` Kalle Valo
2018-04-18 14:44         ` Arend van Spriel
2018-04-18 14:44           ` Arend van Spriel
2018-04-19  4:32         ` govinds
2018-04-19  4:32           ` govinds
2018-04-19 16:36           ` Kalle Valo
2018-04-19 16:36             ` Kalle Valo
2018-04-19 17:00             ` Adrian Chadd
2018-04-19 17:00               ` Adrian Chadd
2018-04-20  7:21             ` Sebastian Gottschall
2018-04-20  7:21               ` Sebastian Gottschall
2018-04-20  8:16               ` Arend van Spriel
2018-04-20  8:16                 ` Arend van Spriel
2018-04-20 12:43                 ` govinds
2018-04-20 12:43                   ` govinds
2018-04-23 15:50                 ` Kalle Valo
2018-04-23 15:50                   ` Kalle Valo
2018-04-23 16:17                   ` 9984 with 10.4.3.5.3-0057 crashes Sebastian Gottschall
2018-04-24  5:27                     ` Kalle Valo
2018-04-24  6:08                       ` Sebastian Gottschall
2018-04-25 15:23                         ` Kalle Valo
2018-04-26  6:23                           ` Sebastian Gottschall
2018-04-26  7:36                           ` Sebastian Gottschall
2018-04-23 16:41                   ` [PATCH v2 4/4] ath10k: Enable sta idle power save Adrian Chadd
2018-04-23 16:41                     ` Adrian Chadd
2018-04-24  5:35                     ` Kalle Valo
2018-04-24  5:35                       ` Kalle Valo

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.