2015-04-13 07:46:26

by Janusz Dziedzic

[permalink] [raw]
Subject: [RFC 1/2] ath10k: refactor ASPM workaround

Disable ASPM only when loading firmware.
After that back to BIOS/system ASMP
settings. When ASPM is enable by system,
this decrease power consumtion.

Signed-off-by: Janusz Dziedzic <[email protected]>
---
drivers/net/wireless/ath/ath10k/pci.c | 17 ++++++++++++-----
drivers/net/wireless/ath/ath10k/pci.h | 3 +++
2 files changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/ath/ath10k/pci.c b/drivers/net/wireless/ath/ath10k/pci.c
index 51e3921..9923c09 100644
--- a/drivers/net/wireless/ath/ath10k/pci.c
+++ b/drivers/net/wireless/ath/ath10k/pci.c
@@ -1227,11 +1227,16 @@ static void ath10k_pci_irq_enable(struct ath10k *ar)

static int ath10k_pci_hif_start(struct ath10k *ar)
{
+ struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif start\n");

ath10k_pci_irq_enable(ar);
ath10k_pci_rx_post(ar);

+ /* Workaround: Back to BIOS/system settings */
+ pcie_capability_write_word(ar_pci->pdev, PCI_EXP_LNKCTL,
+ ar_pci->link_ctl);
+
return 0;
}

@@ -1982,6 +1987,7 @@ static int ath10k_pci_chip_reset(struct ath10k *ar)

static int ath10k_pci_hif_power_up(struct ath10k *ar)
{
+ struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
int ret;

ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot hif power up\n");
@@ -1992,6 +1998,12 @@ static int ath10k_pci_hif_power_up(struct ath10k *ar)
return ret;
}

+ /* Workaround: Disable ASPM */
+ pcie_capability_read_word(ar_pci->pdev, PCI_EXP_LNKCTL,
+ &ar_pci->link_ctl);
+ pcie_capability_write_word(ar_pci->pdev, PCI_EXP_LNKCTL,
+ ar_pci->link_ctl & ~PCI_EXP_LNKCTL_ASPMC);
+
/*
* Bring the target up cleanly.
*
@@ -2515,7 +2527,6 @@ static int ath10k_pci_claim(struct ath10k *ar)
{
struct ath10k_pci *ar_pci = ath10k_pci_priv(ar);
struct pci_dev *pdev = ar_pci->pdev;
- u32 lcr_val;
int ret;

pci_set_drvdata(pdev, ar);
@@ -2549,10 +2560,6 @@ static int ath10k_pci_claim(struct ath10k *ar)

pci_set_master(pdev);

- /* Workaround: Disable ASPM */
- pci_read_config_dword(pdev, 0x80, &lcr_val);
- pci_write_config_dword(pdev, 0x80, (lcr_val & 0xffffff00));
-
/* Arrange for access to Target SoC registers. */
ar_pci->mem = pci_iomap(pdev, BAR_NUM, 0);
if (!ar_pci->mem) {
diff --git a/drivers/net/wireless/ath/ath10k/pci.h b/drivers/net/wireless/ath/ath10k/pci.h
index 56795ed..a6eafec 100644
--- a/drivers/net/wireless/ath/ath10k/pci.h
+++ b/drivers/net/wireless/ath/ath10k/pci.h
@@ -185,6 +185,9 @@ struct ath10k_pci {
/* Map CE id to ce_state */
struct ath10k_ce_pipe ce_states[CE_COUNT_MAX];
struct timer_list rx_post_retry;
+
+ /* Link Control */
+ u16 link_ctl;
};

static inline struct ath10k_pci *ath10k_pci_priv(struct ath10k *ar)
--
1.9.1



2015-04-16 04:35:39

by Janusz Dziedzic

[permalink] [raw]
Subject: Re: [RFC 2/2] ath10k: don't disable PS when not connected

On 15 April 2015 at 19:10, YanBo <[email protected]> wrote:
> On Tue, Apr 14, 2015 at 10:00 PM, Janusz Dziedzic
> <[email protected]> wrote:
>> On 15 April 2015 at 00:45, YanBo <[email protected]> wrote:
>>> On Mon, Apr 13, 2015 at 12:45 AM, Janusz Dziedzic
>>> <[email protected]> wrote:
>>>> Don't disable PS while we are not connected.
>>>> In other case we will get higher power consumption.
>>>>
>>>> Signed-off-by: Janusz Dziedzic <[email protected]>
>>>> ---
>>>> drivers/net/wireless/ath/ath10k/mac.c | 8 +++++++-
>>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
>>>> index 52c5b1f..b896dd4 100644
>>>> --- a/drivers/net/wireless/ath/ath10k/mac.c
>>>> +++ b/drivers/net/wireless/ath/ath10k/mac.c
>>>> @@ -1730,7 +1730,13 @@ static int ath10k_mac_vif_setup_ps(struct ath10k_vif *arvif)
>>>> enable_ps = false;
>>>> }
>>>>
>>>> - if (enable_ps) {
>>>> + if (!arvif->is_started) {
>>>> + /* enable power save mode while not connected,
>>>> + * in other case after iface up we will get
>>>> + * higher power consumption - firmware design
>>>> + */
>>>> + psmode = WMI_STA_PS_MODE_ENABLED;
>>>> + } else if (enable_ps) {
>>>> psmode = WMI_STA_PS_MODE_ENABLED;
>>>> param = WMI_STA_PS_PARAM_INACTIVITY_TIME;
>>>>
>>>> --
>>>
>>> What the expectation behavior after we enable the
>>> WMI_STA_PS_MODE_ENABLED at Idle status?
>>> Is there any effect for TX or RX chain after set it?
>>>
>>
>> First I think that WMI_STA_PS_MODE_ENABLED is important only when we
>> are connected.
>> But, I see current consumption drop in my test environtment from 88mA
>> to 33mA when:
>> 1) load driver, iface up
>> 2) disconnect network, iface down, iface up
>> So, seems WMI_STA_PS_MODE_ENABLED do something more in FW (not only
>> standard PS enable/disable when
>> connected to AP). Probably someone from FW team should answer that, if
>> that is a feature or a bug.
>>
> I guess the chip enter Idle mode power save after set this, did you
> run the the
> test on QCA61XX or QCA 98XX? Can it still auto scan to get all the APs around
> it, cause for IMPS mode it will only looks in its existing profile
> list and sends a probe request,
> with SSID specified in profile for what I know
>
I see iw scan works correctly after iface up (same scan results like ath9k):

In my laptop I have 3 cards
wlan0 - ath9k
wlan1 - QCA9888
wlan4 - QCA6174

janusz@dell6430:~$ sudo iw wlan0 scan dump|grep SSID |wc -l
0
janusz@dell6430:~$ sudo iw wlan0 scan |grep SSID| wc -l
21
janusz@dell6430:~$ sudo ifconfig wlan0 down

janusz@dell6430:~$ sudo iw wlan1 scan dump|grep SSID|wc -l
0
janusz@dell6430:~$ sudo iw wlan1 scan |grep SSID|wc -l
21
janusz@dell6430:~$ sudo ifconfig wlan1 down

janusz@dell6430:~$ sudo iw wlan4 scan dump|grep SSID|wc -l
0
janusz@dell6430:~$ sudo iw wlan4 scan |grep SSID|wc -l
22

Should I run any other scan tests?

BR
Janusz

2015-04-15 17:10:15

by YanBo

[permalink] [raw]
Subject: Re: [RFC 2/2] ath10k: don't disable PS when not connected

On Tue, Apr 14, 2015 at 10:00 PM, Janusz Dziedzic
<[email protected]> wrote:
> On 15 April 2015 at 00:45, YanBo <[email protected]> wrote:
>> On Mon, Apr 13, 2015 at 12:45 AM, Janusz Dziedzic
>> <[email protected]> wrote:
>>> Don't disable PS while we are not connected.
>>> In other case we will get higher power consumption.
>>>
>>> Signed-off-by: Janusz Dziedzic <[email protected]>
>>> ---
>>> drivers/net/wireless/ath/ath10k/mac.c | 8 +++++++-
>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
>>> index 52c5b1f..b896dd4 100644
>>> --- a/drivers/net/wireless/ath/ath10k/mac.c
>>> +++ b/drivers/net/wireless/ath/ath10k/mac.c
>>> @@ -1730,7 +1730,13 @@ static int ath10k_mac_vif_setup_ps(struct ath10k_vif *arvif)
>>> enable_ps = false;
>>> }
>>>
>>> - if (enable_ps) {
>>> + if (!arvif->is_started) {
>>> + /* enable power save mode while not connected,
>>> + * in other case after iface up we will get
>>> + * higher power consumption - firmware design
>>> + */
>>> + psmode = WMI_STA_PS_MODE_ENABLED;
>>> + } else if (enable_ps) {
>>> psmode = WMI_STA_PS_MODE_ENABLED;
>>> param = WMI_STA_PS_PARAM_INACTIVITY_TIME;
>>>
>>> --
>>
>> What the expectation behavior after we enable the
>> WMI_STA_PS_MODE_ENABLED at Idle status?
>> Is there any effect for TX or RX chain after set it?
>>
>
> First I think that WMI_STA_PS_MODE_ENABLED is important only when we
> are connected.
> But, I see current consumption drop in my test environtment from 88mA
> to 33mA when:
> 1) load driver, iface up
> 2) disconnect network, iface down, iface up
> So, seems WMI_STA_PS_MODE_ENABLED do something more in FW (not only
> standard PS enable/disable when
> connected to AP). Probably someone from FW team should answer that, if
> that is a feature or a bug.
>
I guess the chip enter Idle mode power save after set this, did you
run the the
test on QCA61XX or QCA 98XX? Can it still auto scan to get all the APs around
it, cause for IMPS mode it will only looks in its existing profile
list and sends a probe request,
with SSID specified in profile for what I know

BR /Yanbo

2015-04-16 17:02:06

by YanBo

[permalink] [raw]
Subject: Re: [RFC 2/2] ath10k: don't disable PS when not connected

On Wed, Apr 15, 2015 at 9:35 PM, Janusz Dziedzic
<[email protected]> wrote:
> On 15 April 2015 at 19:10, YanBo <[email protected]> wrote:
>> On Tue, Apr 14, 2015 at 10:00 PM, Janusz Dziedzic
>> <[email protected]> wrote:
>>> On 15 April 2015 at 00:45, YanBo <[email protected]> wrote:
>>>> On Mon, Apr 13, 2015 at 12:45 AM, Janusz Dziedzic
>>>> <[email protected]> wrote:
>>>>> Don't disable PS while we are not connected.
>>>>> In other case we will get higher power consumption.
>>>>>
>>>>> Signed-off-by: Janusz Dziedzic <[email protected]>
>>>>> ---
>>>>> drivers/net/wireless/ath/ath10k/mac.c | 8 +++++++-
>>>>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
>>>>> index 52c5b1f..b896dd4 100644
>>>>> --- a/drivers/net/wireless/ath/ath10k/mac.c
>>>>> +++ b/drivers/net/wireless/ath/ath10k/mac.c
>>>>> @@ -1730,7 +1730,13 @@ static int ath10k_mac_vif_setup_ps(struct ath10k_vif *arvif)
>>>>> enable_ps = false;
>>>>> }
>>>>>
>>>>> - if (enable_ps) {
>>>>> + if (!arvif->is_started) {
>>>>> + /* enable power save mode while not connected,
>>>>> + * in other case after iface up we will get
>>>>> + * higher power consumption - firmware design
>>>>> + */
>>>>> + psmode = WMI_STA_PS_MODE_ENABLED;
>>>>> + } else if (enable_ps) {
>>>>> psmode = WMI_STA_PS_MODE_ENABLED;
>>>>> param = WMI_STA_PS_PARAM_INACTIVITY_TIME;
>>>>>
>>>>> --
>>>>
>>>> What the expectation behavior after we enable the
>>>> WMI_STA_PS_MODE_ENABLED at Idle status?
>>>> Is there any effect for TX or RX chain after set it?
>>>>
>>>
>>> First I think that WMI_STA_PS_MODE_ENABLED is important only when we
>>> are connected.
>>> But, I see current consumption drop in my test environtment from 88mA
>>> to 33mA when:
>>> 1) load driver, iface up
>>> 2) disconnect network, iface down, iface up
>>> So, seems WMI_STA_PS_MODE_ENABLED do something more in FW (not only
>>> standard PS enable/disable when
>>> connected to AP). Probably someone from FW team should answer that, if
>>> that is a feature or a bug.
>>>
>> I guess the chip enter Idle mode power save after set this, did you
>> run the the
>> test on QCA61XX or QCA 98XX? Can it still auto scan to get all the APs around
>> it, cause for IMPS mode it will only looks in its existing profile
>> list and sends a probe request,
>> with SSID specified in profile for what I know
>>
> I see iw scan works correctly after iface up (same scan results like ath9k):
>
> In my laptop I have 3 cards
> wlan0 - ath9k
> wlan1 - QCA9888
> wlan4 - QCA6174
>
> janusz@dell6430:~$ sudo iw wlan0 scan dump|grep SSID |wc -l
> 0
> janusz@dell6430:~$ sudo iw wlan0 scan |grep SSID| wc -l
> 21
> janusz@dell6430:~$ sudo ifconfig wlan0 down
>
> janusz@dell6430:~$ sudo iw wlan1 scan dump|grep SSID|wc -l
> 0
> janusz@dell6430:~$ sudo iw wlan1 scan |grep SSID|wc -l
> 21
> janusz@dell6430:~$ sudo ifconfig wlan1 down
>
> janusz@dell6430:~$ sudo iw wlan4 scan dump|grep SSID|wc -l
> 0
> janusz@dell6430:~$ sudo iw wlan4 scan |grep SSID|wc -l
> 22
>
> Should I run any other scan tests?
>

Thanks for the verification, I also get the confirmation that the card will
leave IMPS mode when scanning, so it works as expected.

BR /Yanbo

2015-04-15 05:00:33

by Janusz Dziedzic

[permalink] [raw]
Subject: Re: [RFC 2/2] ath10k: don't disable PS when not connected

On 15 April 2015 at 00:45, YanBo <[email protected]> wrote:
> On Mon, Apr 13, 2015 at 12:45 AM, Janusz Dziedzic
> <[email protected]> wrote:
>> Don't disable PS while we are not connected.
>> In other case we will get higher power consumption.
>>
>> Signed-off-by: Janusz Dziedzic <[email protected]>
>> ---
>> drivers/net/wireless/ath/ath10k/mac.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
>> index 52c5b1f..b896dd4 100644
>> --- a/drivers/net/wireless/ath/ath10k/mac.c
>> +++ b/drivers/net/wireless/ath/ath10k/mac.c
>> @@ -1730,7 +1730,13 @@ static int ath10k_mac_vif_setup_ps(struct ath10k_vif *arvif)
>> enable_ps = false;
>> }
>>
>> - if (enable_ps) {
>> + if (!arvif->is_started) {
>> + /* enable power save mode while not connected,
>> + * in other case after iface up we will get
>> + * higher power consumption - firmware design
>> + */
>> + psmode = WMI_STA_PS_MODE_ENABLED;
>> + } else if (enable_ps) {
>> psmode = WMI_STA_PS_MODE_ENABLED;
>> param = WMI_STA_PS_PARAM_INACTIVITY_TIME;
>>
>> --
>
> What the expectation behavior after we enable the
> WMI_STA_PS_MODE_ENABLED at Idle status?
> Is there any effect for TX or RX chain after set it?
>

First I think that WMI_STA_PS_MODE_ENABLED is important only when we
are connected.
But, I see current consumption drop in my test environtment from 88mA
to 33mA when:
1) load driver, iface up
2) disconnect network, iface down, iface up
So, seems WMI_STA_PS_MODE_ENABLED do something more in FW (not only
standard PS enable/disable when
connected to AP). Probably someone from FW team should answer that, if
that is a feature or a bug.

BTW.
Patch don't change behavior we had before, when we are connected. We
can enable/disable PS using iw.

BR
Janusz

2015-04-14 22:45:39

by YanBo

[permalink] [raw]
Subject: Re: [RFC 2/2] ath10k: don't disable PS when not connected

On Mon, Apr 13, 2015 at 12:45 AM, Janusz Dziedzic
<[email protected]> wrote:
> Don't disable PS while we are not connected.
> In other case we will get higher power consumption.
>
> Signed-off-by: Janusz Dziedzic <[email protected]>
> ---
> drivers/net/wireless/ath/ath10k/mac.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
> index 52c5b1f..b896dd4 100644
> --- a/drivers/net/wireless/ath/ath10k/mac.c
> +++ b/drivers/net/wireless/ath/ath10k/mac.c
> @@ -1730,7 +1730,13 @@ static int ath10k_mac_vif_setup_ps(struct ath10k_vif *arvif)
> enable_ps = false;
> }
>
> - if (enable_ps) {
> + if (!arvif->is_started) {
> + /* enable power save mode while not connected,
> + * in other case after iface up we will get
> + * higher power consumption - firmware design
> + */
> + psmode = WMI_STA_PS_MODE_ENABLED;
> + } else if (enable_ps) {
> psmode = WMI_STA_PS_MODE_ENABLED;
> param = WMI_STA_PS_PARAM_INACTIVITY_TIME;
>
> --

What the expectation behavior after we enable the
WMI_STA_PS_MODE_ENABLED at Idle status?
Is there any effect for TX or RX chain after set it?

BR /Yanbo

2015-04-13 07:46:28

by Janusz Dziedzic

[permalink] [raw]
Subject: [RFC 2/2] ath10k: don't disable PS when not connected

Don't disable PS while we are not connected.
In other case we will get higher power consumption.

Signed-off-by: Janusz Dziedzic <[email protected]>
---
drivers/net/wireless/ath/ath10k/mac.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c
index 52c5b1f..b896dd4 100644
--- a/drivers/net/wireless/ath/ath10k/mac.c
+++ b/drivers/net/wireless/ath/ath10k/mac.c
@@ -1730,7 +1730,13 @@ static int ath10k_mac_vif_setup_ps(struct ath10k_vif *arvif)
enable_ps = false;
}

- if (enable_ps) {
+ if (!arvif->is_started) {
+ /* enable power save mode while not connected,
+ * in other case after iface up we will get
+ * higher power consumption - firmware design
+ */
+ psmode = WMI_STA_PS_MODE_ENABLED;
+ } else if (enable_ps) {
psmode = WMI_STA_PS_MODE_ENABLED;
param = WMI_STA_PS_PARAM_INACTIVITY_TIME;

--
1.9.1