2011-02-15 15:59:40

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states

From: Mohammed Shafi Shajakhan <[email protected]>

The DMA latency issue is observed only in Intel pinetrail platforms but
in the driver we had a default PM-QOS value of 55. This caused
unnecessary power consumption and battery drain in other platforms.
Remove the pm-qos thing in the driver code and address the throughput issue in
Intel pinetrail platfroms in user space using any one of the scripts in below links:
http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
http://johannes.sipsolutions.net/files/netlatency.c.txt
More details can be found in the following bugzilla link:
https://bugzilla.kernel.org/show_bug.cgi?id=27532

Signed-off-by: Mohammed Shafi Shajakhan <[email protected]>
---
drivers/net/wireless/ath/ath9k/ath9k.h | 5 -----
drivers/net/wireless/ath/ath9k/init.c | 7 -------
drivers/net/wireless/ath/ath9k/main.c | 8 --------
3 files changed, 0 insertions(+), 20 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index ba436cd..0052f64 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -21,7 +21,6 @@
#include <linux/device.h>
#include <linux/leds.h>
#include <linux/completion.h>
-#include <linux/pm_qos_params.h>

#include "debug.h"
#include "common.h"
@@ -57,8 +56,6 @@ struct ath_node;

#define A_MAX(a, b) ((a) > (b) ? (a) : (b))

-#define ATH9K_PM_QOS_DEFAULT_VALUE 55
-
#define TSF_TO_TU(_h,_l) \
((((u32)(_h)) << 22) | (((u32)(_l)) >> 10))

@@ -650,7 +647,6 @@ struct ath_softc {

struct ath_ant_comb ant_comb;

- struct pm_qos_request_list pm_qos_req;
};

void ath9k_tasklet(unsigned long data);
@@ -665,7 +661,6 @@ static inline void ath_read_cachesize(struct ath_common *common, int *csz)
extern struct ieee80211_ops ath9k_ops;
extern int ath9k_modparam_nohwcrypt;
extern int led_blink;
-extern int ath9k_pm_qos_value;
extern bool is_ath9k_unloaded;

irqreturn_t ath_isr(int irq, void *dev);
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index e5c1eea..8fed4e4 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -41,9 +41,6 @@ static int ath9k_btcoex_enable;
module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");

-int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
-module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR | S_IRGRP | S_IROTH);
-MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");

bool is_ath9k_unloaded;
/* We use the hw_value as an index into our private channel structure */
@@ -760,9 +757,6 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc, u16 subsysid,
ath_init_leds(sc);
ath_start_rfkill_poll(sc);

- pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
- PM_QOS_DEFAULT_VALUE);
-
return 0;

error_world:
@@ -819,7 +813,6 @@ void ath9k_deinit_device(struct ath_softc *sc)
ath9k_ps_restore(sc);

ieee80211_unregister_hw(hw);
- pm_qos_remove_request(&sc->pm_qos_req);
ath_rx_cleanup(sc);
ath_tx_cleanup(sc);
ath9k_deinit_softc(sc);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index 4f568b8..1d2c7c3 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1117,12 +1117,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
ath9k_btcoex_timer_resume(sc);
}

- /* User has the option to provide pm-qos value as a module
- * parameter rather than using the default value of
- * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
- */
- pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
-
if (ah->caps.pcie_lcr_extsync_en && common->bus_ops->extn_synch_en)
common->bus_ops->extn_synch_en(common);

@@ -1267,8 +1261,6 @@ static void ath9k_stop(struct ieee80211_hw *hw)

sc->sc_flags |= SC_OP_INVALID;

- pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
-
mutex_unlock(&sc->mutex);

ath_dbg(common, ATH_DBG_CONFIG, "Driver halt\n");
--
1.7.0.4



2011-02-10 17:44:42

by Richard Schütz

[permalink] [raw]
Subject: Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states

> Richard, can you test the userspace app as a replacement? Or did your board not
> exerpience the issue?

I was unaffected. Therefore I was unhappy with losing my C-states for no
good reason.

--
Regards,
Richard Sch?tz

2011-02-09 14:50:14

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states

On Wednesday 09 February 2011 08:12 PM, John W. Linville wrote:
> On Wed, Feb 09, 2011 at 08:03:27PM +0530, Mohammed Shafi Shajakhan wrote:
>
>> From: Mohammed Shafi Shajakhan<[email protected]>
>>
>> The DMA latency issue is observed only in Intel pinetrail platforms but
>> in the driver we had a default PM-QOS value of 55. This caused
>> unnecessary power consumption and battery drain in other platforms.
>> Address this issue by disabling PM-QOS by default by setting it's value
>> as '0' and making code changes appropriately.This addresses the bug:
>> https://bugzilla.kernel.org/show_bug.cgi?id=27532
>> If the user sees some DMA latency issue he can still use the pmqos as a
>> module parameter to trade power for throughput as below:
>> sudo modprobe ath9k pmqos=55
>>
>> Signed-off-by: Mohammed Shafi Shajakhan<[email protected]>
>>
> Seems a bit unclear to me -- if you have ath9k_pm_qos_value then why
> use ATH9K_PM_QOS_DEFAULT_VALUE at all?
>
Yes thanks, I think that macro is not needed at all with the current
approach
> John
>

2011-02-10 17:41:28

by Luis R. Rodriguez

[permalink] [raw]
Subject: Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states

On Thu, Feb 10, 2011 at 06:19:10AM -0800, Mohammed Shajakhan wrote:
> On Wednesday 09 February 2011 10:18 PM, Richard Sch?tz wrote:
> >> From: Mohammed Shafi Shajakhan<[email protected]>
> >>
> >> The DMA latency issue is observed only in Intel pinetrail platforms but
> >> in the driver we had a default PM-QOS value of 55. This caused
> >> unnecessary power consumption and battery drain in other platforms.
> >> Address this issue by disabling PM-QOS by default by setting it's value
> >> as '0' and making code changes appropriately.This addresses the bug:
> >> https://bugzilla.kernel.org/show_bug.cgi?id=27532
> >> If the user sees some DMA latency issue he can still use the pmqos as a
> >> module parameter to trade power for throughput as below:
> >> sudo modprobe ath9k pmqos=55
> >>
> >> Signed-off-by: Mohammed Shafi Shajakhan<[email protected]>
> >>
> > Acked-By: Richard Sch?tz<[email protected]>
> >
> Thanks, but we got to consider user space approach which Luis had mentioned.

Richard, can you test the userspace app as a replacement? Or did your board not
exerpience the issue?

Luis

2011-02-09 14:45:26

by John W. Linville

[permalink] [raw]
Subject: Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states

On Wed, Feb 09, 2011 at 08:03:27PM +0530, Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan <[email protected]>
>
> The DMA latency issue is observed only in Intel pinetrail platforms but
> in the driver we had a default PM-QOS value of 55. This caused
> unnecessary power consumption and battery drain in other platforms.
> Address this issue by disabling PM-QOS by default by setting it's value
> as '0' and making code changes appropriately.This addresses the bug:
> https://bugzilla.kernel.org/show_bug.cgi?id=27532
> If the user sees some DMA latency issue he can still use the pmqos as a
> module parameter to trade power for throughput as below:
> sudo modprobe ath9k pmqos=55
>
> Signed-off-by: Mohammed Shafi Shajakhan <[email protected]>

Seems a bit unclear to me -- if you have ath9k_pm_qos_value then why
use ATH9K_PM_QOS_DEFAULT_VALUE at all?

John
--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2011-02-09 16:48:25

by Richard Schütz

[permalink] [raw]
Subject: Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states

> From: Mohammed Shafi Shajakhan<[email protected]>
>
> The DMA latency issue is observed only in Intel pinetrail platforms but
> in the driver we had a default PM-QOS value of 55. This caused
> unnecessary power consumption and battery drain in other platforms.
> Address this issue by disabling PM-QOS by default by setting it's value
> as '0' and making code changes appropriately.This addresses the bug:
> https://bugzilla.kernel.org/show_bug.cgi?id=27532
> If the user sees some DMA latency issue he can still use the pmqos as a
> module parameter to trade power for throughput as below:
> sudo modprobe ath9k pmqos=55
>
> Signed-off-by: Mohammed Shafi Shajakhan<[email protected]>
Acked-By: Richard Sch?tz <[email protected]>

Looks good to me with the exception of ATH9K_PM_QOS_DEFAULT_VALUE. But
John already mentioned that.

--
Regards,
Richard Sch?tz

2011-02-16 05:05:12

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states

On Tuesday 15 February 2011 09:42 PM, John W. Linville wrote:
> Despite the [RFC], I am applying this. I should have reverted all
> of this three weeks ago...
>
Thanks John.
> John
>
> On Tue, Feb 15, 2011 at 09:29:32PM +0530, Mohammed Shafi Shajakhan wrote:
>
>> From: Mohammed Shafi Shajakhan<[email protected]>
>>
>> The DMA latency issue is observed only in Intel pinetrail platforms but
>> in the driver we had a default PM-QOS value of 55. This caused
>> unnecessary power consumption and battery drain in other platforms.
>> Remove the pm-qos thing in the driver code and address the throughput issue in
>> Intel pinetrail platfroms in user space using any one of the scripts in below links:
>> http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
>> http://johannes.sipsolutions.net/files/netlatency.c.txt
>> More details can be found in the following bugzilla link:
>> https://bugzilla.kernel.org/show_bug.cgi?id=27532
>>
>> Signed-off-by: Mohammed Shafi Shajakhan<[email protected]>
>> ---
>> drivers/net/wireless/ath/ath9k/ath9k.h | 5 -----
>> drivers/net/wireless/ath/ath9k/init.c | 7 -------
>> drivers/net/wireless/ath/ath9k/main.c | 8 --------
>> 3 files changed, 0 insertions(+), 20 deletions(-)
>>
>> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
>> index ba436cd..0052f64 100644
>> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>> @@ -21,7 +21,6 @@
>> #include<linux/device.h>
>> #include<linux/leds.h>
>> #include<linux/completion.h>
>> -#include<linux/pm_qos_params.h>
>>
>> #include "debug.h"
>> #include "common.h"
>> @@ -57,8 +56,6 @@ struct ath_node;
>>
>> #define A_MAX(a, b) ((a)> (b) ? (a) : (b))
>>
>> -#define ATH9K_PM_QOS_DEFAULT_VALUE 55
>> -
>> #define TSF_TO_TU(_h,_l) \
>> ((((u32)(_h))<< 22) | (((u32)(_l))>> 10))
>>
>> @@ -650,7 +647,6 @@ struct ath_softc {
>>
>> struct ath_ant_comb ant_comb;
>>
>> - struct pm_qos_request_list pm_qos_req;
>> };
>>
>> void ath9k_tasklet(unsigned long data);
>> @@ -665,7 +661,6 @@ static inline void ath_read_cachesize(struct ath_common *common, int *csz)
>> extern struct ieee80211_ops ath9k_ops;
>> extern int ath9k_modparam_nohwcrypt;
>> extern int led_blink;
>> -extern int ath9k_pm_qos_value;
>> extern bool is_ath9k_unloaded;
>>
>> irqreturn_t ath_isr(int irq, void *dev);
>> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
>> index e5c1eea..8fed4e4 100644
>> --- a/drivers/net/wireless/ath/ath9k/init.c
>> +++ b/drivers/net/wireless/ath/ath9k/init.c
>> @@ -41,9 +41,6 @@ static int ath9k_btcoex_enable;
>> module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
>> MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
>>
>> -int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
>> -module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR | S_IRGRP | S_IROTH);
>> -MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
>>
>> bool is_ath9k_unloaded;
>> /* We use the hw_value as an index into our private channel structure */
>> @@ -760,9 +757,6 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc, u16 subsysid,
>> ath_init_leds(sc);
>> ath_start_rfkill_poll(sc);
>>
>> - pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
>> - PM_QOS_DEFAULT_VALUE);
>> -
>> return 0;
>>
>> error_world:
>> @@ -819,7 +813,6 @@ void ath9k_deinit_device(struct ath_softc *sc)
>> ath9k_ps_restore(sc);
>>
>> ieee80211_unregister_hw(hw);
>> - pm_qos_remove_request(&sc->pm_qos_req);
>> ath_rx_cleanup(sc);
>> ath_tx_cleanup(sc);
>> ath9k_deinit_softc(sc);
>> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
>> index 4f568b8..1d2c7c3 100644
>> --- a/drivers/net/wireless/ath/ath9k/main.c
>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>> @@ -1117,12 +1117,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
>> ath9k_btcoex_timer_resume(sc);
>> }
>>
>> - /* User has the option to provide pm-qos value as a module
>> - * parameter rather than using the default value of
>> - * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
>> - */
>> - pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
>> -
>> if (ah->caps.pcie_lcr_extsync_en&& common->bus_ops->extn_synch_en)
>> common->bus_ops->extn_synch_en(common);
>>
>> @@ -1267,8 +1261,6 @@ static void ath9k_stop(struct ieee80211_hw *hw)
>>
>> sc->sc_flags |= SC_OP_INVALID;
>>
>> - pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
>> -
>> mutex_unlock(&sc->mutex);
>>
>> ath_dbg(common, ATH_DBG_CONFIG, "Driver halt\n");
>> --
>> 1.7.0.4
>>
>>
>>
>

2011-02-15 16:15:31

by John W. Linville

[permalink] [raw]
Subject: Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states

Despite the [RFC], I am applying this. I should have reverted all
of this three weeks ago...

John

On Tue, Feb 15, 2011 at 09:29:32PM +0530, Mohammed Shafi Shajakhan wrote:
> From: Mohammed Shafi Shajakhan <[email protected]>
>
> The DMA latency issue is observed only in Intel pinetrail platforms but
> in the driver we had a default PM-QOS value of 55. This caused
> unnecessary power consumption and battery drain in other platforms.
> Remove the pm-qos thing in the driver code and address the throughput issue in
> Intel pinetrail platfroms in user space using any one of the scripts in below links:
> http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
> http://johannes.sipsolutions.net/files/netlatency.c.txt
> More details can be found in the following bugzilla link:
> https://bugzilla.kernel.org/show_bug.cgi?id=27532
>
> Signed-off-by: Mohammed Shafi Shajakhan <[email protected]>
> ---
> drivers/net/wireless/ath/ath9k/ath9k.h | 5 -----
> drivers/net/wireless/ath/ath9k/init.c | 7 -------
> drivers/net/wireless/ath/ath9k/main.c | 8 --------
> 3 files changed, 0 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index ba436cd..0052f64 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> @@ -21,7 +21,6 @@
> #include <linux/device.h>
> #include <linux/leds.h>
> #include <linux/completion.h>
> -#include <linux/pm_qos_params.h>
>
> #include "debug.h"
> #include "common.h"
> @@ -57,8 +56,6 @@ struct ath_node;
>
> #define A_MAX(a, b) ((a) > (b) ? (a) : (b))
>
> -#define ATH9K_PM_QOS_DEFAULT_VALUE 55
> -
> #define TSF_TO_TU(_h,_l) \
> ((((u32)(_h)) << 22) | (((u32)(_l)) >> 10))
>
> @@ -650,7 +647,6 @@ struct ath_softc {
>
> struct ath_ant_comb ant_comb;
>
> - struct pm_qos_request_list pm_qos_req;
> };
>
> void ath9k_tasklet(unsigned long data);
> @@ -665,7 +661,6 @@ static inline void ath_read_cachesize(struct ath_common *common, int *csz)
> extern struct ieee80211_ops ath9k_ops;
> extern int ath9k_modparam_nohwcrypt;
> extern int led_blink;
> -extern int ath9k_pm_qos_value;
> extern bool is_ath9k_unloaded;
>
> irqreturn_t ath_isr(int irq, void *dev);
> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
> index e5c1eea..8fed4e4 100644
> --- a/drivers/net/wireless/ath/ath9k/init.c
> +++ b/drivers/net/wireless/ath/ath9k/init.c
> @@ -41,9 +41,6 @@ static int ath9k_btcoex_enable;
> module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
> MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
>
> -int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
> -module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR | S_IRGRP | S_IROTH);
> -MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
>
> bool is_ath9k_unloaded;
> /* We use the hw_value as an index into our private channel structure */
> @@ -760,9 +757,6 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc, u16 subsysid,
> ath_init_leds(sc);
> ath_start_rfkill_poll(sc);
>
> - pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
> - PM_QOS_DEFAULT_VALUE);
> -
> return 0;
>
> error_world:
> @@ -819,7 +813,6 @@ void ath9k_deinit_device(struct ath_softc *sc)
> ath9k_ps_restore(sc);
>
> ieee80211_unregister_hw(hw);
> - pm_qos_remove_request(&sc->pm_qos_req);
> ath_rx_cleanup(sc);
> ath_tx_cleanup(sc);
> ath9k_deinit_softc(sc);
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index 4f568b8..1d2c7c3 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -1117,12 +1117,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
> ath9k_btcoex_timer_resume(sc);
> }
>
> - /* User has the option to provide pm-qos value as a module
> - * parameter rather than using the default value of
> - * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
> - */
> - pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
> -
> if (ah->caps.pcie_lcr_extsync_en && common->bus_ops->extn_synch_en)
> common->bus_ops->extn_synch_en(common);
>
> @@ -1267,8 +1261,6 @@ static void ath9k_stop(struct ieee80211_hw *hw)
>
> sc->sc_flags |= SC_OP_INVALID;
>
> - pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
> -
> mutex_unlock(&sc->mutex);
>
> ath_dbg(common, ATH_DBG_CONFIG, "Driver halt\n");
> --
> 1.7.0.4
>
>

--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2011-02-10 14:19:16

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states

On Wednesday 09 February 2011 10:18 PM, Richard Sch?tz wrote:
>> From: Mohammed Shafi Shajakhan<[email protected]>
>>
>> The DMA latency issue is observed only in Intel pinetrail platforms but
>> in the driver we had a default PM-QOS value of 55. This caused
>> unnecessary power consumption and battery drain in other platforms.
>> Address this issue by disabling PM-QOS by default by setting it's value
>> as '0' and making code changes appropriately.This addresses the bug:
>> https://bugzilla.kernel.org/show_bug.cgi?id=27532
>> If the user sees some DMA latency issue he can still use the pmqos as a
>> module parameter to trade power for throughput as below:
>> sudo modprobe ath9k pmqos=55
>>
>> Signed-off-by: Mohammed Shafi Shajakhan<[email protected]>
>>
> Acked-By: Richard Sch?tz<[email protected]>
>
Thanks, but we got to consider user space approach which Luis had mentioned.
> Looks good to me with the exception of ATH9K_PM_QOS_DEFAULT_VALUE. But
> John already mentioned that.
>
>

2011-02-11 13:31:51

by Richard Schütz

[permalink] [raw]
Subject: Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states

>> Richard, can you test the userspace app as a replacement? Or did your board not
>> exerpience the issue?
>
> I was unaffected. Therefore I was unhappy with losing my C-states for no good reason.
>

Nevertheless I tested the userspace approach now. It disables the C4-state on my machine,
so it should also help those who really have problems with the performance of their
wireless connection.

--
Regards,
Richard Sch?tz

2011-03-04 06:06:44

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states

On Wednesday 16 February 2011 10:35 AM, Mohammed Shafi wrote:
> On Tuesday 15 February 2011 09:42 PM, John W. Linville wrote:
>> Despite the [RFC], I am applying this. I should have reverted all
>> of this three weeks ago...
John can you please push this to stable kernel ?
> Thanks John.
>> John
>>
>> On Tue, Feb 15, 2011 at 09:29:32PM +0530, Mohammed Shafi Shajakhan
>> wrote:
>>> From: Mohammed Shafi Shajakhan<[email protected]>
>>>
>>> The DMA latency issue is observed only in Intel pinetrail platforms but
>>> in the driver we had a default PM-QOS value of 55. This caused
>>> unnecessary power consumption and battery drain in other platforms.
>>> Remove the pm-qos thing in the driver code and address the
>>> throughput issue in
>>> Intel pinetrail platfroms in user space using any one of the scripts
>>> in below links:
>>> http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
>>>
>>> http://johannes.sipsolutions.net/files/netlatency.c.txt
>>> More details can be found in the following bugzilla link:
>>> https://bugzilla.kernel.org/show_bug.cgi?id=27532
>>>
>>> Signed-off-by: Mohammed Shafi Shajakhan<[email protected]>
>>> ---
>>> drivers/net/wireless/ath/ath9k/ath9k.h | 5 -----
>>> drivers/net/wireless/ath/ath9k/init.c | 7 -------
>>> drivers/net/wireless/ath/ath9k/main.c | 8 --------
>>> 3 files changed, 0 insertions(+), 20 deletions(-)
>>>
>>> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h
>>> b/drivers/net/wireless/ath/ath9k/ath9k.h
>>> index ba436cd..0052f64 100644
>>> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>>> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>>> @@ -21,7 +21,6 @@
>>> #include<linux/device.h>
>>> #include<linux/leds.h>
>>> #include<linux/completion.h>
>>> -#include<linux/pm_qos_params.h>
>>>
>>> #include "debug.h"
>>> #include "common.h"
>>> @@ -57,8 +56,6 @@ struct ath_node;
>>>
>>> #define A_MAX(a, b) ((a)> (b) ? (a) : (b))
>>>
>>> -#define ATH9K_PM_QOS_DEFAULT_VALUE 55
>>> -
>>> #define TSF_TO_TU(_h,_l) \
>>> ((((u32)(_h))<< 22) | (((u32)(_l))>> 10))
>>>
>>> @@ -650,7 +647,6 @@ struct ath_softc {
>>>
>>> struct ath_ant_comb ant_comb;
>>>
>>> - struct pm_qos_request_list pm_qos_req;
>>> };
>>>
>>> void ath9k_tasklet(unsigned long data);
>>> @@ -665,7 +661,6 @@ static inline void ath_read_cachesize(struct
>>> ath_common *common, int *csz)
>>> extern struct ieee80211_ops ath9k_ops;
>>> extern int ath9k_modparam_nohwcrypt;
>>> extern int led_blink;
>>> -extern int ath9k_pm_qos_value;
>>> extern bool is_ath9k_unloaded;
>>>
>>> irqreturn_t ath_isr(int irq, void *dev);
>>> diff --git a/drivers/net/wireless/ath/ath9k/init.c
>>> b/drivers/net/wireless/ath/ath9k/init.c
>>> index e5c1eea..8fed4e4 100644
>>> --- a/drivers/net/wireless/ath/ath9k/init.c
>>> +++ b/drivers/net/wireless/ath/ath9k/init.c
>>> @@ -41,9 +41,6 @@ static int ath9k_btcoex_enable;
>>> module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
>>> MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
>>>
>>> -int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
>>> -module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR |
>>> S_IRGRP | S_IROTH);
>>> -MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
>>>
>>> bool is_ath9k_unloaded;
>>> /* We use the hw_value as an index into our private channel
>>> structure */
>>> @@ -760,9 +757,6 @@ int ath9k_init_device(u16 devid, struct
>>> ath_softc *sc, u16 subsysid,
>>> ath_init_leds(sc);
>>> ath_start_rfkill_poll(sc);
>>>
>>> - pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
>>> - PM_QOS_DEFAULT_VALUE);
>>> -
>>> return 0;
>>>
>>> error_world:
>>> @@ -819,7 +813,6 @@ void ath9k_deinit_device(struct ath_softc *sc)
>>> ath9k_ps_restore(sc);
>>>
>>> ieee80211_unregister_hw(hw);
>>> - pm_qos_remove_request(&sc->pm_qos_req);
>>> ath_rx_cleanup(sc);
>>> ath_tx_cleanup(sc);
>>> ath9k_deinit_softc(sc);
>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c
>>> b/drivers/net/wireless/ath/ath9k/main.c
>>> index 4f568b8..1d2c7c3 100644
>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>> @@ -1117,12 +1117,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
>>> ath9k_btcoex_timer_resume(sc);
>>> }
>>>
>>> - /* User has the option to provide pm-qos value as a module
>>> - * parameter rather than using the default value of
>>> - * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
>>> - */
>>> - pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
>>> -
>>> if (ah->caps.pcie_lcr_extsync_en&&
>>> common->bus_ops->extn_synch_en)
>>> common->bus_ops->extn_synch_en(common);
>>>
>>> @@ -1267,8 +1261,6 @@ static void ath9k_stop(struct ieee80211_hw *hw)
>>>
>>> sc->sc_flags |= SC_OP_INVALID;
>>>
>>> - pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
>>> -
>>> mutex_unlock(&sc->mutex);
>>>
>>> ath_dbg(common, ATH_DBG_CONFIG, "Driver halt\n");
>>> --
>>> 1.7.0.4
>>>
>>>

2011-03-04 22:07:26

by Thomas Bächler

[permalink] [raw]
Subject: [PATCH] ath9k: Fix ath9k prevents CPU to enter C3 states

This is a backport of upstream commit 0f5cd45960173ba5b36727decbb4a241cbd35ef9.

The DMA latency issue is observed only in Intel pinetrail platforms
but in the driver we had a default PM-QOS value of 55. This caused
unnecessary power consumption and battery drain in other platforms.

Remove the pm-qos thing in the driver code and address the throughput
issue in Intel pinetrail platfroms in user space using any one of
the scripts in below links:

http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
http://johannes.sipsolutions.net/files/netlatency.c.txt

More details can be found in the following bugzilla link:

https://bugzilla.kernel.org/show_bug.cgi?id=27532

Signed-off-by: Thomas Bächler <[email protected]>
---

I hope this is right, it seems to work for me.
Luis or John, can you sign off on this just
to be sure?

drivers/net/wireless/ath/ath9k/ath9k.h | 3 ---
drivers/net/wireless/ath/ath9k/init.c | 4 ----
drivers/net/wireless/ath/ath9k/main.c | 4 ----
3 files changed, 0 insertions(+), 11 deletions(-)

diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
index c0b60ce..94bd9bc 100644
--- a/drivers/net/wireless/ath/ath9k/ath9k.h
+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
@@ -21,7 +21,6 @@
#include <linux/device.h>
#include <linux/leds.h>
#include <linux/completion.h>
-#include <linux/pm_qos_params.h>

#include "debug.h"
#include "common.h"
@@ -647,8 +646,6 @@ struct ath_softc {
struct ath_descdma txsdma;

struct ath_ant_comb ant_comb;
-
- struct pm_qos_request_list pm_qos_req;
};

struct ath_wiphy {
diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
index 14b8ab3..91d9b2a 100644
--- a/drivers/net/wireless/ath/ath9k/init.c
+++ b/drivers/net/wireless/ath/ath9k/init.c
@@ -758,9 +758,6 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc, u16 subsysid,
ath_init_leds(sc);
ath_start_rfkill_poll(sc);

- pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
- PM_QOS_DEFAULT_VALUE);
-
return 0;

error_world:
@@ -829,7 +826,6 @@ void ath9k_deinit_device(struct ath_softc *sc)
}

ieee80211_unregister_hw(hw);
- pm_qos_remove_request(&sc->pm_qos_req);
ath_rx_cleanup(sc);
ath_tx_cleanup(sc);
ath9k_deinit_softc(sc);
diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
index d1b0db4..cb0b2b9 100644
--- a/drivers/net/wireless/ath/ath9k/main.c
+++ b/drivers/net/wireless/ath/ath9k/main.c
@@ -1245,8 +1245,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
ath9k_btcoex_timer_resume(sc);
}

- pm_qos_update_request(&sc->pm_qos_req, 55);
-
mutex_unlock:
mutex_unlock(&sc->mutex);

@@ -1425,8 +1423,6 @@ static void ath9k_stop(struct ieee80211_hw *hw)

sc->sc_flags |= SC_OP_INVALID;

- pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
-
mutex_unlock(&sc->mutex);

ath_print(common, ATH_DBG_CONFIG, "Driver halt\n");
--
1.7.4.1


2011-03-04 13:45:43

by John W. Linville

[permalink] [raw]
Subject: Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states

On Fri, Mar 04, 2011 at 11:36:37AM +0530, Mohammed Shafi wrote:
> On Wednesday 16 February 2011 10:35 AM, Mohammed Shafi wrote:
> >On Tuesday 15 February 2011 09:42 PM, John W. Linville wrote:
> >>Despite the [RFC], I am applying this. I should have reverted all
> >>of this three weeks ago...
> John can you please push this to stable kernel ?

Stable folks, this is the commit in Linus's tree;

commit 0f5cd45960173ba5b36727decbb4a241cbd35ef9
Author: Mohammed Shafi Shajakhan <[email protected]>
Date: Tue Feb 15 21:29:32 2011 +0530

ath9k: Fix ath9k prevents CPU to enter C3 states

The DMA latency issue is observed only in Intel pinetrail platforms
but in the driver we had a default PM-QOS value of 55. This caused
unnecessary power consumption and battery drain in other platforms.

Remove the pm-qos thing in the driver code and address the throughput
issue in Intel pinetrail platfroms in user space using any one of
the scripts in below links:

http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
http://johannes.sipsolutions.net/files/netlatency.c.txt

More details can be found in the following bugzilla link:

https://bugzilla.kernel.org/show_bug.cgi?id=27532

This reverts the following commits:

98c316e348bedffa730e6f1e4baeb8a3c3e0f28b
4dc3530df7c0428b41c00399a7ee8c929406d181
10598c124ecabbbfd7522f74de19b8f7d52a1bee

Signed-off-by: Mohammed Shafi Shajakhan <[email protected]>
Signed-off-by: John W. Linville <[email protected]>

> >Thanks John.
> >>John
> >>
> >>On Tue, Feb 15, 2011 at 09:29:32PM +0530, Mohammed Shafi
> >>Shajakhan wrote:
> >>>From: Mohammed Shafi Shajakhan<[email protected]>
> >>>
> >>>The DMA latency issue is observed only in Intel pinetrail platforms but
> >>>in the driver we had a default PM-QOS value of 55. This caused
> >>>unnecessary power consumption and battery drain in other platforms.
> >>> Remove the pm-qos thing in the driver code and address the
> >>>throughput issue in
> >>>Intel pinetrail platfroms in user space using any one of the
> >>>scripts in below links:
> >>>http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
> >>>
> >>>http://johannes.sipsolutions.net/files/netlatency.c.txt
> >>>More details can be found in the following bugzilla link:
> >>>https://bugzilla.kernel.org/show_bug.cgi?id=27532
> >>>
> >>>Signed-off-by: Mohammed Shafi Shajakhan<[email protected]>
> >>>---
> >>> drivers/net/wireless/ath/ath9k/ath9k.h | 5 -----
> >>> drivers/net/wireless/ath/ath9k/init.c | 7 -------
> >>> drivers/net/wireless/ath/ath9k/main.c | 8 --------
> >>> 3 files changed, 0 insertions(+), 20 deletions(-)
> >>>
> >>>diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h
> >>>b/drivers/net/wireless/ath/ath9k/ath9k.h
> >>>index ba436cd..0052f64 100644
> >>>--- a/drivers/net/wireless/ath/ath9k/ath9k.h
> >>>+++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> >>>@@ -21,7 +21,6 @@
> >>> #include<linux/device.h>
> >>> #include<linux/leds.h>
> >>> #include<linux/completion.h>
> >>>-#include<linux/pm_qos_params.h>
> >>>
> >>> #include "debug.h"
> >>> #include "common.h"
> >>>@@ -57,8 +56,6 @@ struct ath_node;
> >>>
> >>> #define A_MAX(a, b) ((a)> (b) ? (a) : (b))
> >>>
> >>>-#define ATH9K_PM_QOS_DEFAULT_VALUE 55
> >>>-
> >>> #define TSF_TO_TU(_h,_l) \
> >>> ((((u32)(_h))<< 22) | (((u32)(_l))>> 10))
> >>>
> >>>@@ -650,7 +647,6 @@ struct ath_softc {
> >>>
> >>> struct ath_ant_comb ant_comb;
> >>>
> >>>- struct pm_qos_request_list pm_qos_req;
> >>> };
> >>>
> >>> void ath9k_tasklet(unsigned long data);
> >>>@@ -665,7 +661,6 @@ static inline void
> >>>ath_read_cachesize(struct ath_common *common, int *csz)
> >>> extern struct ieee80211_ops ath9k_ops;
> >>> extern int ath9k_modparam_nohwcrypt;
> >>> extern int led_blink;
> >>>-extern int ath9k_pm_qos_value;
> >>> extern bool is_ath9k_unloaded;
> >>>
> >>> irqreturn_t ath_isr(int irq, void *dev);
> >>>diff --git a/drivers/net/wireless/ath/ath9k/init.c
> >>>b/drivers/net/wireless/ath/ath9k/init.c
> >>>index e5c1eea..8fed4e4 100644
> >>>--- a/drivers/net/wireless/ath/ath9k/init.c
> >>>+++ b/drivers/net/wireless/ath/ath9k/init.c
> >>>@@ -41,9 +41,6 @@ static int ath9k_btcoex_enable;
> >>> module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
> >>> MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
> >>>
> >>>-int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
> >>>-module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR |
> >>>S_IRGRP | S_IROTH);
> >>>-MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
> >>>
> >>> bool is_ath9k_unloaded;
> >>> /* We use the hw_value as an index into our private channel
> >>>structure */
> >>>@@ -760,9 +757,6 @@ int ath9k_init_device(u16 devid, struct
> >>>ath_softc *sc, u16 subsysid,
> >>> ath_init_leds(sc);
> >>> ath_start_rfkill_poll(sc);
> >>>
> >>>- pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
> >>>- PM_QOS_DEFAULT_VALUE);
> >>>-
> >>> return 0;
> >>>
> >>> error_world:
> >>>@@ -819,7 +813,6 @@ void ath9k_deinit_device(struct ath_softc *sc)
> >>> ath9k_ps_restore(sc);
> >>>
> >>> ieee80211_unregister_hw(hw);
> >>>- pm_qos_remove_request(&sc->pm_qos_req);
> >>> ath_rx_cleanup(sc);
> >>> ath_tx_cleanup(sc);
> >>> ath9k_deinit_softc(sc);
> >>>diff --git a/drivers/net/wireless/ath/ath9k/main.c
> >>>b/drivers/net/wireless/ath/ath9k/main.c
> >>>index 4f568b8..1d2c7c3 100644
> >>>--- a/drivers/net/wireless/ath/ath9k/main.c
> >>>+++ b/drivers/net/wireless/ath/ath9k/main.c
> >>>@@ -1117,12 +1117,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
> >>> ath9k_btcoex_timer_resume(sc);
> >>> }
> >>>
> >>>- /* User has the option to provide pm-qos value as a module
> >>>- * parameter rather than using the default value of
> >>>- * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
> >>>- */
> >>>- pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
> >>>-
> >>> if (ah->caps.pcie_lcr_extsync_en&&
> >>>common->bus_ops->extn_synch_en)
> >>> common->bus_ops->extn_synch_en(common);
> >>>
> >>>@@ -1267,8 +1261,6 @@ static void ath9k_stop(struct ieee80211_hw *hw)
> >>>
> >>> sc->sc_flags |= SC_OP_INVALID;
> >>>
> >>>- pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
> >>>-
> >>> mutex_unlock(&sc->mutex);
> >>>
> >>> ath_dbg(common, ATH_DBG_CONFIG, "Driver halt\n");
> >>>--
> >>>1.7.0.4
> >>>
> >>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

--
John W. Linville Someday the world will need a hero, and you
[email protected] might be all we have. Be ready.

2011-03-04 14:26:59

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states

On Friday 04 March 2011 07:07 PM, John W. Linville wrote:
> On Fri, Mar 04, 2011 at 11:36:37AM +0530, Mohammed Shafi wrote:
>
>> On Wednesday 16 February 2011 10:35 AM, Mohammed Shafi wrote:
>>
>>> On Tuesday 15 February 2011 09:42 PM, John W. Linville wrote:
>>>
>>>> Despite the [RFC], I am applying this. I should have reverted all
>>>> of this three weeks ago...
>>>>
>> John can you please push this to stable kernel ?
>>
> Stable folks, this is the commit in Linus's tree;
>
Thanks a lot John.
> commit 0f5cd45960173ba5b36727decbb4a241cbd35ef9
> Author: Mohammed Shafi Shajakhan<[email protected]>
> Date: Tue Feb 15 21:29:32 2011 +0530
>
> ath9k: Fix ath9k prevents CPU to enter C3 states
>
> The DMA latency issue is observed only in Intel pinetrail platforms
> but in the driver we had a default PM-QOS value of 55. This caused
> unnecessary power consumption and battery drain in other platforms.
>
> Remove the pm-qos thing in the driver code and address the throughput
> issue in Intel pinetrail platfroms in user space using any one of
> the scripts in below links:
>
> http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
> http://johannes.sipsolutions.net/files/netlatency.c.txt
>
> More details can be found in the following bugzilla link:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=27532
>
> This reverts the following commits:
>
> 98c316e348bedffa730e6f1e4baeb8a3c3e0f28b
> 4dc3530df7c0428b41c00399a7ee8c929406d181
> 10598c124ecabbbfd7522f74de19b8f7d52a1bee
>
> Signed-off-by: Mohammed Shafi Shajakhan<[email protected]>
> Signed-off-by: John W. Linville<[email protected]>
>
>
>>> Thanks John.
>>>
>>>> John
>>>>
>>>> On Tue, Feb 15, 2011 at 09:29:32PM +0530, Mohammed Shafi
>>>> Shajakhan wrote:
>>>>
>>>>> From: Mohammed Shafi Shajakhan<[email protected]>
>>>>>
>>>>> The DMA latency issue is observed only in Intel pinetrail platforms but
>>>>> in the driver we had a default PM-QOS value of 55. This caused
>>>>> unnecessary power consumption and battery drain in other platforms.
>>>>> Remove the pm-qos thing in the driver code and address the
>>>>> throughput issue in
>>>>> Intel pinetrail platfroms in user space using any one of the
>>>>> scripts in below links:
>>>>> http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
>>>>>
>>>>> http://johannes.sipsolutions.net/files/netlatency.c.txt
>>>>> More details can be found in the following bugzilla link:
>>>>> https://bugzilla.kernel.org/show_bug.cgi?id=27532
>>>>>
>>>>> Signed-off-by: Mohammed Shafi Shajakhan<[email protected]>
>>>>> ---
>>>>> drivers/net/wireless/ath/ath9k/ath9k.h | 5 -----
>>>>> drivers/net/wireless/ath/ath9k/init.c | 7 -------
>>>>> drivers/net/wireless/ath/ath9k/main.c | 8 --------
>>>>> 3 files changed, 0 insertions(+), 20 deletions(-)
>>>>>
>>>>> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h
>>>>> b/drivers/net/wireless/ath/ath9k/ath9k.h
>>>>> index ba436cd..0052f64 100644
>>>>> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
>>>>> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
>>>>> @@ -21,7 +21,6 @@
>>>>> #include<linux/device.h>
>>>>> #include<linux/leds.h>
>>>>> #include<linux/completion.h>
>>>>> -#include<linux/pm_qos_params.h>
>>>>>
>>>>> #include "debug.h"
>>>>> #include "common.h"
>>>>> @@ -57,8 +56,6 @@ struct ath_node;
>>>>>
>>>>> #define A_MAX(a, b) ((a)> (b) ? (a) : (b))
>>>>>
>>>>> -#define ATH9K_PM_QOS_DEFAULT_VALUE 55
>>>>> -
>>>>> #define TSF_TO_TU(_h,_l) \
>>>>> ((((u32)(_h))<< 22) | (((u32)(_l))>> 10))
>>>>>
>>>>> @@ -650,7 +647,6 @@ struct ath_softc {
>>>>>
>>>>> struct ath_ant_comb ant_comb;
>>>>>
>>>>> - struct pm_qos_request_list pm_qos_req;
>>>>> };
>>>>>
>>>>> void ath9k_tasklet(unsigned long data);
>>>>> @@ -665,7 +661,6 @@ static inline void
>>>>> ath_read_cachesize(struct ath_common *common, int *csz)
>>>>> extern struct ieee80211_ops ath9k_ops;
>>>>> extern int ath9k_modparam_nohwcrypt;
>>>>> extern int led_blink;
>>>>> -extern int ath9k_pm_qos_value;
>>>>> extern bool is_ath9k_unloaded;
>>>>>
>>>>> irqreturn_t ath_isr(int irq, void *dev);
>>>>> diff --git a/drivers/net/wireless/ath/ath9k/init.c
>>>>> b/drivers/net/wireless/ath/ath9k/init.c
>>>>> index e5c1eea..8fed4e4 100644
>>>>> --- a/drivers/net/wireless/ath/ath9k/init.c
>>>>> +++ b/drivers/net/wireless/ath/ath9k/init.c
>>>>> @@ -41,9 +41,6 @@ static int ath9k_btcoex_enable;
>>>>> module_param_named(btcoex_enable, ath9k_btcoex_enable, int, 0444);
>>>>> MODULE_PARM_DESC(btcoex_enable, "Enable wifi-BT coexistence");
>>>>>
>>>>> -int ath9k_pm_qos_value = ATH9K_PM_QOS_DEFAULT_VALUE;
>>>>> -module_param_named(pmqos, ath9k_pm_qos_value, int, S_IRUSR |
>>>>> S_IRGRP | S_IROTH);
>>>>> -MODULE_PARM_DESC(pmqos, "User specified PM-QOS value");
>>>>>
>>>>> bool is_ath9k_unloaded;
>>>>> /* We use the hw_value as an index into our private channel
>>>>> structure */
>>>>> @@ -760,9 +757,6 @@ int ath9k_init_device(u16 devid, struct
>>>>> ath_softc *sc, u16 subsysid,
>>>>> ath_init_leds(sc);
>>>>> ath_start_rfkill_poll(sc);
>>>>>
>>>>> - pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
>>>>> - PM_QOS_DEFAULT_VALUE);
>>>>> -
>>>>> return 0;
>>>>>
>>>>> error_world:
>>>>> @@ -819,7 +813,6 @@ void ath9k_deinit_device(struct ath_softc *sc)
>>>>> ath9k_ps_restore(sc);
>>>>>
>>>>> ieee80211_unregister_hw(hw);
>>>>> - pm_qos_remove_request(&sc->pm_qos_req);
>>>>> ath_rx_cleanup(sc);
>>>>> ath_tx_cleanup(sc);
>>>>> ath9k_deinit_softc(sc);
>>>>> diff --git a/drivers/net/wireless/ath/ath9k/main.c
>>>>> b/drivers/net/wireless/ath/ath9k/main.c
>>>>> index 4f568b8..1d2c7c3 100644
>>>>> --- a/drivers/net/wireless/ath/ath9k/main.c
>>>>> +++ b/drivers/net/wireless/ath/ath9k/main.c
>>>>> @@ -1117,12 +1117,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
>>>>> ath9k_btcoex_timer_resume(sc);
>>>>> }
>>>>>
>>>>> - /* User has the option to provide pm-qos value as a module
>>>>> - * parameter rather than using the default value of
>>>>> - * 'ATH9K_PM_QOS_DEFAULT_VALUE'.
>>>>> - */
>>>>> - pm_qos_update_request(&sc->pm_qos_req, ath9k_pm_qos_value);
>>>>> -
>>>>> if (ah->caps.pcie_lcr_extsync_en&&
>>>>> common->bus_ops->extn_synch_en)
>>>>> common->bus_ops->extn_synch_en(common);
>>>>>
>>>>> @@ -1267,8 +1261,6 @@ static void ath9k_stop(struct ieee80211_hw *hw)
>>>>>
>>>>> sc->sc_flags |= SC_OP_INVALID;
>>>>>
>>>>> - pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
>>>>> -
>>>>> mutex_unlock(&sc->mutex);
>>>>>
>>>>> ath_dbg(common, ATH_DBG_CONFIG, "Driver halt\n");
>>>>> --
>>>>> 1.7.0.4
>>>>>
>>>>>
>>>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>

2011-03-07 05:40:06

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [PATCH] ath9k: Fix ath9k prevents CPU to enter C3 states

On Saturday 05 March 2011 03:30 AM, Thomas Bächler wrote:
> This is a backport of upstream commit 0f5cd45960173ba5b36727decbb4a241cbd35ef9.
>
> The DMA latency issue is observed only in Intel pinetrail platforms
> but in the driver we had a default PM-QOS value of 55. This caused
> unnecessary power consumption and battery drain in other platforms.
>
> Remove the pm-qos thing in the driver code and address the throughput
> issue in Intel pinetrail platfroms in user space using any one of
> the scripts in below links:
>
> http://www.kernel.org/pub/linux/kernel/people/mcgrof/scripts/cpudmalatency.c
> http://johannes.sipsolutions.net/files/netlatency.c.txt
>
> More details can be found in the following bugzilla link:
>
> https://bugzilla.kernel.org/show_bug.cgi?id=27532
>
> Signed-off-by: Thomas Bächler<[email protected]>
> ---
>
> I hope this is right, it seems to work for me.
> Luis or John, can you sign off on this just
> to be sure?
>
> drivers/net/wireless/ath/ath9k/ath9k.h | 3 ---
> drivers/net/wireless/ath/ath9k/init.c | 4 ----
> drivers/net/wireless/ath/ath9k/main.c | 4 ----
> 3 files changed, 0 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/net/wireless/ath/ath9k/ath9k.h b/drivers/net/wireless/ath/ath9k/ath9k.h
> index c0b60ce..94bd9bc 100644
> --- a/drivers/net/wireless/ath/ath9k/ath9k.h
> +++ b/drivers/net/wireless/ath/ath9k/ath9k.h
> @@ -21,7 +21,6 @@
> #include<linux/device.h>
> #include<linux/leds.h>
> #include<linux/completion.h>
> -#include<linux/pm_qos_params.h>
>
> #include "debug.h"
> #include "common.h"
> @@ -647,8 +646,6 @@ struct ath_softc {
> struct ath_descdma txsdma;
>
> struct ath_ant_comb ant_comb;
> -
> - struct pm_qos_request_list pm_qos_req;
> };
>
> struct ath_wiphy {
> diff --git a/drivers/net/wireless/ath/ath9k/init.c b/drivers/net/wireless/ath/ath9k/init.c
> index 14b8ab3..91d9b2a 100644
> --- a/drivers/net/wireless/ath/ath9k/init.c
> +++ b/drivers/net/wireless/ath/ath9k/init.c
> @@ -758,9 +758,6 @@ int ath9k_init_device(u16 devid, struct ath_softc *sc, u16 subsysid,
> ath_init_leds(sc);
> ath_start_rfkill_poll(sc);
>
> - pm_qos_add_request(&sc->pm_qos_req, PM_QOS_CPU_DMA_LATENCY,
> - PM_QOS_DEFAULT_VALUE);
> -
> return 0;
>
> error_world:
> @@ -829,7 +826,6 @@ void ath9k_deinit_device(struct ath_softc *sc)
> }
>
> ieee80211_unregister_hw(hw);
> - pm_qos_remove_request(&sc->pm_qos_req);
> ath_rx_cleanup(sc);
> ath_tx_cleanup(sc);
> ath9k_deinit_softc(sc);
> diff --git a/drivers/net/wireless/ath/ath9k/main.c b/drivers/net/wireless/ath/ath9k/main.c
> index d1b0db4..cb0b2b9 100644
> --- a/drivers/net/wireless/ath/ath9k/main.c
> +++ b/drivers/net/wireless/ath/ath9k/main.c
> @@ -1245,8 +1245,6 @@ static int ath9k_start(struct ieee80211_hw *hw)
> ath9k_btcoex_timer_resume(sc);
> }
>
> - pm_qos_update_request(&sc->pm_qos_req, 55);
> -
> mutex_unlock:
> mutex_unlock(&sc->mutex);
>
> @@ -1425,8 +1423,6 @@ static void ath9k_stop(struct ieee80211_hw *hw)
>
> sc->sc_flags |= SC_OP_INVALID;
>
> - pm_qos_update_request(&sc->pm_qos_req, PM_QOS_DEFAULT_VALUE);
> -
> mutex_unlock(&sc->mutex);
>
> ath_print(common, ATH_DBG_CONFIG, "Driver halt\n");
>
Thomas, thanks !

2011-03-04 21:39:50

by Greg KH

[permalink] [raw]
Subject: Re: [stable] [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states

On Fri, Mar 04, 2011 at 08:37:23AM -0500, John W. Linville wrote:
> On Fri, Mar 04, 2011 at 11:36:37AM +0530, Mohammed Shafi wrote:
> > On Wednesday 16 February 2011 10:35 AM, Mohammed Shafi wrote:
> > >On Tuesday 15 February 2011 09:42 PM, John W. Linville wrote:
> > >>Despite the [RFC], I am applying this. I should have reverted all
> > >>of this three weeks ago...
> > John can you please push this to stable kernel ?
>
> Stable folks, this is the commit in Linus's tree;
>
> commit 0f5cd45960173ba5b36727decbb4a241cbd35ef9

Too bad this commit doesn't apply to the .37-stable tree :(

Can someone provide [email protected] a backported version of this?

thanks,

greg k-h

2011-03-07 05:44:20

by Mohammed Shafi Shajakhan

[permalink] [raw]
Subject: Re: [stable] [RFC] ath9k: Fix ath9k prevents CPU to enter C3 states

On Saturday 05 March 2011 03:07 AM, Greg KH wrote:
> On Fri, Mar 04, 2011 at 08:37:23AM -0500, John W. Linville wrote:
>
>> On Fri, Mar 04, 2011 at 11:36:37AM +0530, Mohammed Shafi wrote:
>>
>>> On Wednesday 16 February 2011 10:35 AM, Mohammed Shafi wrote:
>>>
>>>> On Tuesday 15 February 2011 09:42 PM, John W. Linville wrote:
>>>>
>>>>> Despite the [RFC], I am applying this. I should have reverted all
>>>>> of this three weeks ago...
>>>>>
>>> John can you please push this to stable kernel ?
>>>
>> Stable folks, this is the commit in Linus's tree;
>>
>> commit 0f5cd45960173ba5b36727decbb4a241cbd35ef9
>>
> Too bad this commit doesn't apply to the .37-stable tree :(
>
This is because of the commit 4dc3530df7c0428b41c00399a7ee8c929406d181.
And this is handled by the backport provided by Thomas.

thanks,
shafi
> Can someone provide [email protected] a backported version of this?
>
> thanks,
>
> greg k-h
> .
>
>