diff mbox series

[v2] examples/power: add baseline mode to PMD power

Message ID 20210622140750.6208-1-david.hunt@intel.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers
Series [v2] examples/power: add baseline mode to PMD power |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/github-robot success github build: passed
ci/iol-abi-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Functional fail Functional Testing issues

Commit Message

Hunt, David June 22, 2021, 2:07 p.m. UTC 
  The PMD Power Management scheme currently has 3 modes,
scale, monitor and pause. However, it would be nice to
have a baseline mode for easy comparison of power savings
with and without these modes.

This patch adds a 'baseline' mode were the pmd power
management is not enabled. Use --pmg-mgmt=baseline.

Signed-off-by: David Hunt <david.hunt@intel.com>

---
changes in v2
* added a bool for baseline mode rather than abusing enums
---
 examples/l3fwd-power/main.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

Comments

Burakov, Anatoly June 23, 2021, 10:14 a.m. UTC | #1 
On 22-Jun-21 3:07 PM, David Hunt wrote:
> The PMD Power Management scheme currently has 3 modes,
> scale, monitor and pause. However, it would be nice to
> have a baseline mode for easy comparison of power savings
> with and without these modes.
> 
> This patch adds a 'baseline' mode were the pmd power
> management is not enabled. Use --pmg-mgmt=baseline.
> 
> Signed-off-by: David Hunt <david.hunt@intel.com>
> 
> ---
> changes in v2
> * added a bool for baseline mode rather than abusing enums
> ---
>   examples/l3fwd-power/main.c | 14 ++++++++++++--
>   1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
> index f8dfed1634..aeb2411e62 100644
> --- a/examples/l3fwd-power/main.c
> +++ b/examples/l3fwd-power/main.c
> @@ -207,6 +207,7 @@ enum appmode {
>   enum appmode app_mode;
>   
>   static enum rte_power_pmd_mgmt_type pmgmt_type;
> +bool baseline_enabled;
>   
>   enum freq_scale_hint_t
>   {
> @@ -1617,7 +1618,7 @@ print_usage(const char *prgname)
>   		" empty polls, full polls, and core busyness to telemetry\n"
>   		" --interrupt-only: enable interrupt-only mode\n"
>   		" --pmd-mgmt MODE: enable PMD power management mode. "
> -		"Currently supported modes: monitor, pause, scale\n",
> +		"Currently supported modes: baseline, monitor, pause, scale\n",
>   		prgname);
>   }
>   
> @@ -1714,6 +1715,7 @@ parse_pmd_mgmt_config(const char *name)
>   #define PMD_MGMT_MONITOR "monitor"
>   #define PMD_MGMT_PAUSE   "pause"
>   #define PMD_MGMT_SCALE   "scale"
> +#define PMD_MGMT_BASELINE  "baseline"
>   
>   	if (strncmp(PMD_MGMT_MONITOR, name, sizeof(PMD_MGMT_MONITOR)) == 0) {
>   		pmgmt_type = RTE_POWER_MGMT_TYPE_MONITOR;
> @@ -1729,6 +1731,10 @@ parse_pmd_mgmt_config(const char *name)
>   		pmgmt_type = RTE_POWER_MGMT_TYPE_SCALE;
>   		return 0;
>   	}
> +	if (strncmp(PMD_MGMT_BASELINE, name, sizeof(PMD_MGMT_BASELINE)) == 0) {
> +		baseline_enabled = true;
> +		return 0;
> +	}
>   	/* unknown PMD power management mode */
>   	return -1;
>   }
> @@ -2528,6 +2534,9 @@ main(int argc, char **argv)
>   	/* init RTE timer library to be used late */
>   	rte_timer_subsystem_init();
>   
> +	/* if we're running pmd-mgmt mode, don't default to baseline mode */
> +	baseline_enabled = false;
> +
>   	/* parse application arguments (after the EAL ones) */
>   	ret = parse_args(argc, argv);
>   	if (ret < 0)
> @@ -2767,7 +2776,8 @@ main(int argc, char **argv)
>   						 "Fail to add ptype cb\n");
>   			}
>   
> -			if (app_mode == APP_MODE_PMD_MGMT) {
> +			if ((app_mode == APP_MODE_PMD_MGMT) &&
> +					(baseline_enabled == false)) {

Nitpicking, !baseline_enabled

Otherwise,

Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>

>   				ret = rte_power_ethdev_pmgmt_queue_enable(
>   						lcore_id, portid, queueid,
>   						pmgmt_type);
>
Hunt, David June 23, 2021, 12:09 p.m. UTC | #2 
On 23/6/2021 11:14 AM, Burakov, Anatoly wrote:
> On 22-Jun-21 3:07 PM, David Hunt wrote:
>> The PMD Power Management scheme currently has 3 modes,
>> scale, monitor and pause. However, it would be nice to
>> have a baseline mode for easy comparison of power savings
>> with and without these modes.
>>
>> This patch adds a 'baseline' mode were the pmd power
>> management is not enabled. Use --pmg-mgmt=baseline.
>>
>> Signed-off-by: David Hunt <david.hunt@intel.com>
>>
>> ---
>> changes in v2
>> * added a bool for baseline mode rather than abusing enums
>> ---
>>   examples/l3fwd-power/main.c | 14 ++++++++++++--
>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>
>> diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
>> index f8dfed1634..aeb2411e62 100644
>> --- a/examples/l3fwd-power/main.c
>> +++ b/examples/l3fwd-power/main.c
>> @@ -207,6 +207,7 @@ enum appmode {
>>   enum appmode app_mode;
>>     static enum rte_power_pmd_mgmt_type pmgmt_type;
>> +bool baseline_enabled;
>>     enum freq_scale_hint_t
>>   {
>> @@ -1617,7 +1618,7 @@ print_usage(const char *prgname)
>>           " empty polls, full polls, and core busyness to telemetry\n"
>>           " --interrupt-only: enable interrupt-only mode\n"
>>           " --pmd-mgmt MODE: enable PMD power management mode. "
>> -        "Currently supported modes: monitor, pause, scale\n",
>> +        "Currently supported modes: baseline, monitor, pause, scale\n",
>>           prgname);
>>   }
>>   @@ -1714,6 +1715,7 @@ parse_pmd_mgmt_config(const char *name)
>>   #define PMD_MGMT_MONITOR "monitor"
>>   #define PMD_MGMT_PAUSE   "pause"
>>   #define PMD_MGMT_SCALE   "scale"
>> +#define PMD_MGMT_BASELINE  "baseline"
>>         if (strncmp(PMD_MGMT_MONITOR, name, sizeof(PMD_MGMT_MONITOR)) 
>> == 0) {
>>           pmgmt_type = RTE_POWER_MGMT_TYPE_MONITOR;
>> @@ -1729,6 +1731,10 @@ parse_pmd_mgmt_config(const char *name)
>>           pmgmt_type = RTE_POWER_MGMT_TYPE_SCALE;
>>           return 0;
>>       }
>> +    if (strncmp(PMD_MGMT_BASELINE, name, sizeof(PMD_MGMT_BASELINE)) 
>> == 0) {
>> +        baseline_enabled = true;
>> +        return 0;
>> +    }
>>       /* unknown PMD power management mode */
>>       return -1;
>>   }
>> @@ -2528,6 +2534,9 @@ main(int argc, char **argv)
>>       /* init RTE timer library to be used late */
>>       rte_timer_subsystem_init();
>>   +    /* if we're running pmd-mgmt mode, don't default to baseline 
>> mode */
>> +    baseline_enabled = false;
>> +
>>       /* parse application arguments (after the EAL ones) */
>>       ret = parse_args(argc, argv);
>>       if (ret < 0)
>> @@ -2767,7 +2776,8 @@ main(int argc, char **argv)
>>                            "Fail to add ptype cb\n");
>>               }
>>   -            if (app_mode == APP_MODE_PMD_MGMT) {
>> +            if ((app_mode == APP_MODE_PMD_MGMT) &&
>> +                    (baseline_enabled == false)) {
>
> Nitpicking, !baseline_enabled
>
Thanks, Anatoly. I don't feel a re-spin is needed for this, but I'll 
take this format into consideration for future patches, OK?


> Otherwise,
>
> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>
>
Burakov, Anatoly June 23, 2021, 2:14 p.m. UTC | #3 
On 23-Jun-21 1:09 PM, David Hunt wrote:
> 
> On 23/6/2021 11:14 AM, Burakov, Anatoly wrote:
>> On 22-Jun-21 3:07 PM, David Hunt wrote:
>>> The PMD Power Management scheme currently has 3 modes,
>>> scale, monitor and pause. However, it would be nice to
>>> have a baseline mode for easy comparison of power savings
>>> with and without these modes.
>>>
>>> This patch adds a 'baseline' mode were the pmd power
>>> management is not enabled. Use --pmg-mgmt=baseline.
>>>
>>> Signed-off-by: David Hunt <david.hunt@intel.com>
>>>
>>> ---
>>> changes in v2
>>> * added a bool for baseline mode rather than abusing enums
>>> ---
>>>   examples/l3fwd-power/main.c | 14 ++++++++++++--
>>>   1 file changed, 12 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
>>> index f8dfed1634..aeb2411e62 100644
>>> --- a/examples/l3fwd-power/main.c
>>> +++ b/examples/l3fwd-power/main.c
>>> @@ -207,6 +207,7 @@ enum appmode {
>>>   enum appmode app_mode;
>>>     static enum rte_power_pmd_mgmt_type pmgmt_type;
>>> +bool baseline_enabled;
>>>     enum freq_scale_hint_t
>>>   {
>>> @@ -1617,7 +1618,7 @@ print_usage(const char *prgname)
>>>           " empty polls, full polls, and core busyness to telemetry\n"
>>>           " --interrupt-only: enable interrupt-only mode\n"
>>>           " --pmd-mgmt MODE: enable PMD power management mode. "
>>> -        "Currently supported modes: monitor, pause, scale\n",
>>> +        "Currently supported modes: baseline, monitor, pause, scale\n",
>>>           prgname);
>>>   }
>>>   @@ -1714,6 +1715,7 @@ parse_pmd_mgmt_config(const char *name)
>>>   #define PMD_MGMT_MONITOR "monitor"
>>>   #define PMD_MGMT_PAUSE   "pause"
>>>   #define PMD_MGMT_SCALE   "scale"
>>> +#define PMD_MGMT_BASELINE  "baseline"
>>>         if (strncmp(PMD_MGMT_MONITOR, name, sizeof(PMD_MGMT_MONITOR)) 
>>> == 0) {
>>>           pmgmt_type = RTE_POWER_MGMT_TYPE_MONITOR;
>>> @@ -1729,6 +1731,10 @@ parse_pmd_mgmt_config(const char *name)
>>>           pmgmt_type = RTE_POWER_MGMT_TYPE_SCALE;
>>>           return 0;
>>>       }
>>> +    if (strncmp(PMD_MGMT_BASELINE, name, sizeof(PMD_MGMT_BASELINE)) 
>>> == 0) {
>>> +        baseline_enabled = true;
>>> +        return 0;
>>> +    }
>>>       /* unknown PMD power management mode */
>>>       return -1;
>>>   }
>>> @@ -2528,6 +2534,9 @@ main(int argc, char **argv)
>>>       /* init RTE timer library to be used late */
>>>       rte_timer_subsystem_init();
>>>   +    /* if we're running pmd-mgmt mode, don't default to baseline 
>>> mode */
>>> +    baseline_enabled = false;
>>> +
>>>       /* parse application arguments (after the EAL ones) */
>>>       ret = parse_args(argc, argv);
>>>       if (ret < 0)
>>> @@ -2767,7 +2776,8 @@ main(int argc, char **argv)
>>>                            "Fail to add ptype cb\n");
>>>               }
>>>   -            if (app_mode == APP_MODE_PMD_MGMT) {
>>> +            if ((app_mode == APP_MODE_PMD_MGMT) &&
>>> +                    (baseline_enabled == false)) {
>>
>> Nitpicking, !baseline_enabled
>>
> Thanks, Anatoly. I don't feel a re-spin is needed for this, but I'll 
> take this format into consideration for future patches, OK?

Yep, i'm fine either way. Maybe someone can fix it on apply :)

> 
> 
>> Otherwise,
>>
>> Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
>>
>>
Thomas Monjalon July 10, 2021, 6:43 a.m. UTC | #4 
23/06/2021 16:14, Burakov, Anatoly:
> On 23-Jun-21 1:09 PM, David Hunt wrote:
> > On 23/6/2021 11:14 AM, Burakov, Anatoly wrote:
> >> On 22-Jun-21 3:07 PM, David Hunt wrote:
> >>> The PMD Power Management scheme currently has 3 modes,
> >>> scale, monitor and pause. However, it would be nice to
> >>> have a baseline mode for easy comparison of power savings
> >>> with and without these modes.
> >>>
> >>> This patch adds a 'baseline' mode were the pmd power
> >>> management is not enabled. Use --pmg-mgmt=baseline.
> >>>
> >>> Signed-off-by: David Hunt <david.hunt@intel.com>
> >>>
> >>> ---
> >>>   -            if (app_mode == APP_MODE_PMD_MGMT) {
> >>> +            if ((app_mode == APP_MODE_PMD_MGMT) &&
> >>> +                    (baseline_enabled == false)) {
> >>
> >> Nitpicking, !baseline_enabled
> >>
> > Thanks, Anatoly. I don't feel a re-spin is needed for this, but I'll 
> > take this format into consideration for future patches, OK?
> 
> Yep, i'm fine either way. Maybe someone can fix it on apply :)

Applied with the change.
diff mbox series

Patch

diff --git a/examples/l3fwd-power/main.c b/examples/l3fwd-power/main.c
index f8dfed1634..aeb2411e62 100644
--- a/examples/l3fwd-power/main.c
+++ b/examples/l3fwd-power/main.c
@@ -207,6 +207,7 @@  enum appmode {
 enum appmode app_mode;
 
 static enum rte_power_pmd_mgmt_type pmgmt_type;
+bool baseline_enabled;
 
 enum freq_scale_hint_t
 {
@@ -1617,7 +1618,7 @@  print_usage(const char *prgname)
 		" empty polls, full polls, and core busyness to telemetry\n"
 		" --interrupt-only: enable interrupt-only mode\n"
 		" --pmd-mgmt MODE: enable PMD power management mode. "
-		"Currently supported modes: monitor, pause, scale\n",
+		"Currently supported modes: baseline, monitor, pause, scale\n",
 		prgname);
 }
 
@@ -1714,6 +1715,7 @@  parse_pmd_mgmt_config(const char *name)
 #define PMD_MGMT_MONITOR "monitor"
 #define PMD_MGMT_PAUSE   "pause"
 #define PMD_MGMT_SCALE   "scale"
+#define PMD_MGMT_BASELINE  "baseline"
 
 	if (strncmp(PMD_MGMT_MONITOR, name, sizeof(PMD_MGMT_MONITOR)) == 0) {
 		pmgmt_type = RTE_POWER_MGMT_TYPE_MONITOR;
@@ -1729,6 +1731,10 @@  parse_pmd_mgmt_config(const char *name)
 		pmgmt_type = RTE_POWER_MGMT_TYPE_SCALE;
 		return 0;
 	}
+	if (strncmp(PMD_MGMT_BASELINE, name, sizeof(PMD_MGMT_BASELINE)) == 0) {
+		baseline_enabled = true;
+		return 0;
+	}
 	/* unknown PMD power management mode */
 	return -1;
 }
@@ -2528,6 +2534,9 @@  main(int argc, char **argv)
 	/* init RTE timer library to be used late */
 	rte_timer_subsystem_init();
 
+	/* if we're running pmd-mgmt mode, don't default to baseline mode */
+	baseline_enabled = false;
+
 	/* parse application arguments (after the EAL ones) */
 	ret = parse_args(argc, argv);
 	if (ret < 0)
@@ -2767,7 +2776,8 @@  main(int argc, char **argv)
 						 "Fail to add ptype cb\n");
 			}
 
-			if (app_mode == APP_MODE_PMD_MGMT) {
+			if ((app_mode == APP_MODE_PMD_MGMT) &&
+					(baseline_enabled == false)) {
 				ret = rte_power_ethdev_pmgmt_queue_enable(
 						lcore_id, portid, queueid,
 						pmgmt_type);