Re: [dpdk-dev] [PATCH v2] examples/power: add baseline mode to PMD power

[David Hunt <david.hunt@intel.com>]

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>
>
>