[PATCH v2 00/16] powerplay code refactoring.

Christian König deathsimple at vodafone.de
Mon Sep 12 10:52:06 UTC 2016


A few coding style issues, but nothing major.

Taking patch #7 as an example, but most of this applies to the rest as well:
> +/*    sviLoadLIneEn, SviLoadLineVddC, TDC_VDDC_ThrottleReleaseLimitPerc, TDC_MAWt, TdcWaterfallCtl, DTEAmbientTempBase, DisplayCac, BAPM_TEMP_GRADIENT */
That line looks to long, if you can please try to shorten it down to 80 
chars.

If that doesn't work without completely sacrificing readability then 
just leave it as it is.

> +		if (allowed_clock_voltage_table->entries[i].clk >= clock) {
...
> +			if (allowed_clock_voltage_table->entries[i].vddci) {
> +				voltage->Vddci = phm_get_voltage_id(&data->vddci_voltage_table,
> +									allowed_clock_voltage_table->entries[i].vddci);
> +			} else {
> +				voltage->Vddci = phm_get_voltage_id(&data->vddci_voltage_table,
> +									allowed_clock_voltage_table->entries[i].vddc - VDDC_VDDCI_DELTA);
> +			}
Again the line is way to long. What you could do is checking the 
prerequisites if a continue instead of the if.

E.g. instead of:

if (allowed_clock_voltage_table->entries[i].clk >= clock) {
...
}

use this:

if (allowed_clock_voltage_table->entries[i].clk < clock)
     continue;
...

> +
> +			if (allowed_clock_voltage_table->entries[i].mvdd) {
> +				*mvdd = (uint32_t) allowed_clock_voltage_table->entries[i].mvdd;
> +			}
Drop the "{" and "}" if it's just a single line of code.

Regards,
Christian.

Am 12.09.2016 um 10:59 schrieb Rex Zhu:
> v2: 1. move out ppt renamed patches.
>      2. wrap ppt version related functions in smu7_hwmgr.c
>
> implement smu7_hwmgr smu7_smumgr to manager asics with smu version 7.
> add delete duplicated code.
>
> There are subtle differences in firmware image between different asics,
> add asic_name_smc.c under smumgr to deal with fw related jobs.
>
> Rex Zhu (16):
>    drm/amd/powerplay: add common interface in smumgr to help to visit fw
>      image.
>    drm/amd/powerplay: implement smu7 hwmgr to manager asics with smu ip
>      version 7.
>    drm/amd/powerplay: implement fw image related smum interface for
>      Polaris.
>    drm/amd/powerplay: use smu7 hwmgr to manager polaris10/11
>    drm/amd/powerplay: implement fw image related smu interface for Fiji.
>    drm/amd/powerplay: use smu7 hwmgr to manager fiji
>    drm/amd/powerplay: implement fw image related smum interface for
>      tonga.
>    drm/amd/powerplay: use smu7_hwmgr to manager tonga.
>    drm/amd/powerplay: implement smu7_smumgr for asics with smu ip version
>      7.
>    drm/amd/powerplay: use smu7 common functions and data on Tonga.
>    drm/amd/powerplay: use smu7 common functions and data on Polars10.
>    drm/amd/powerplay: use smu7 common functions and data on Fiji.
>    drm/amd/powerplay: use smu7 common functions and data on icelannd.
>    drm/amd/powerplay: implement fw related smu interface for iceland.
>    drm/amd/powerplay: use smu7 hwmgr to manager iceland
>    drm/amd/powerplay: delete useless files.
>
>   drivers/gpu/drm/amd/powerplay/hwmgr/Makefile       |   16 +-
>   .../amd/powerplay/hwmgr/fiji_clockpowergating.c    |  121 -
>   .../amd/powerplay/hwmgr/fiji_clockpowergating.h    |   35 -
>   .../drm/amd/powerplay/hwmgr/fiji_dyn_defaults.h    |  105 -
>   drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c   | 5596 -----------------
>   drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.h   |  350 --
>   .../gpu/drm/amd/powerplay/hwmgr/fiji_powertune.c   |  610 --
>   .../gpu/drm/amd/powerplay/hwmgr/fiji_powertune.h   |   81 -
>   drivers/gpu/drm/amd/powerplay/hwmgr/fiji_thermal.c |  687 ---
>   drivers/gpu/drm/amd/powerplay/hwmgr/fiji_thermal.h |   62 -
>   drivers/gpu/drm/amd/powerplay/hwmgr/hwmgr.c        |  110 +-
>   .../amd/powerplay/hwmgr/iceland_clockpowergating.c |  119 -
>   .../amd/powerplay/hwmgr/iceland_clockpowergating.h |   38 -
>   .../drm/amd/powerplay/hwmgr/iceland_dyn_defaults.h |   41 -
>   .../gpu/drm/amd/powerplay/hwmgr/iceland_hwmgr.c    | 5684 -----------------
>   .../gpu/drm/amd/powerplay/hwmgr/iceland_hwmgr.h    |  424 --
>   .../drm/amd/powerplay/hwmgr/iceland_powertune.c    |  490 --
>   .../drm/amd/powerplay/hwmgr/iceland_powertune.h    |   74 -
>   .../gpu/drm/amd/powerplay/hwmgr/iceland_thermal.c  |  595 --
>   .../gpu/drm/amd/powerplay/hwmgr/iceland_thermal.h  |   58 -
>   .../powerplay/hwmgr/polaris10_clockpowergating.c   |  444 --
>   .../powerplay/hwmgr/polaris10_clockpowergating.h   |   40 -
>   .../amd/powerplay/hwmgr/polaris10_dyn_defaults.h   |   62 -
>   .../gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c  | 5284 ----------------
>   .../gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.h  |  355 --
>   .../drm/amd/powerplay/hwmgr/polaris10_powertune.c  |  988 ---
>   .../drm/amd/powerplay/hwmgr/polaris10_powertune.h  |   81 -
>   .../drm/amd/powerplay/hwmgr/polaris10_thermal.c    |  716 ---
>   .../drm/amd/powerplay/hwmgr/polaris10_thermal.h    |   62 -
>   .../amd/powerplay/hwmgr/smu7_clockpowergating.c    |  491 ++
>   .../amd/powerplay/hwmgr/smu7_clockpowergating.h    |   40 +
>   .../drm/amd/powerplay/hwmgr/smu7_dyn_defaults.h    |   55 +
>   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c   | 4283 +++++++++++++
>   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.h   |  351 ++
>   .../gpu/drm/amd/powerplay/hwmgr/smu7_powertune.c   |  729 +++
>   .../gpu/drm/amd/powerplay/hwmgr/smu7_powertune.h   |   62 +
>   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c |  577 ++
>   drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.h |   62 +
>   .../amd/powerplay/hwmgr/tonga_clockpowergating.c   |  350 --
>   .../amd/powerplay/hwmgr/tonga_clockpowergating.h   |   36 -
>   .../drm/amd/powerplay/hwmgr/tonga_dyn_defaults.h   |  107 -
>   drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c  | 6373 --------------------
>   drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.h  |  402 --
>   .../gpu/drm/amd/powerplay/hwmgr/tonga_powertune.c  |  495 --
>   .../gpu/drm/amd/powerplay/hwmgr/tonga_powertune.h  |   80 -
>   .../gpu/drm/amd/powerplay/hwmgr/tonga_thermal.c    |  590 --
>   .../gpu/drm/amd/powerplay/hwmgr/tonga_thermal.h    |   61 -
>   drivers/gpu/drm/amd/powerplay/inc/hwmgr.h          |    1 +
>   .../gpu/drm/amd/powerplay/inc/polaris10_pwrvirus.h |    3 +-
>   drivers/gpu/drm/amd/powerplay/inc/smu7_common.h    |   58 +
>   drivers/gpu/drm/amd/powerplay/inc/smu7_ppsmc.h     |  412 ++
>   drivers/gpu/drm/amd/powerplay/inc/smumgr.h         |   70 +
>   drivers/gpu/drm/amd/powerplay/smumgr/Makefile      |    5 +-
>   drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c    | 2374 ++++++++
>   drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.h    |   51 +
>   drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.c |  612 +-
>   drivers/gpu/drm/amd/powerplay/smumgr/fiji_smumgr.h |   32 +-
>   drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c | 2577 ++++++++
>   drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.h |   40 +
>   .../gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c  |  613 +-
>   .../gpu/drm/amd/powerplay/smumgr/iceland_smumgr.h  |   63 +-
>   .../gpu/drm/amd/powerplay/smumgr/polaris10_smc.c   | 2287 +++++++
>   .../gpu/drm/amd/powerplay/smumgr/polaris10_smc.h   |   42 +
>   .../drm/amd/powerplay/smumgr/polaris10_smumgr.c    |  703 +--
>   .../drm/amd/powerplay/smumgr/polaris10_smumgr.h    |   41 +-
>   drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c |  589 ++
>   drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.h |   87 +
>   drivers/gpu/drm/amd/powerplay/smumgr/smumgr.c      |  101 +-
>   drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c   | 3092 ++++++++++
>   drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.h   |   60 +
>   .../gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c    |  672 +--
>   .../gpu/drm/amd/powerplay/smumgr/tonga_smumgr.h    |   46 +-
>   72 files changed, 18902 insertions(+), 34201 deletions(-)
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/fiji_clockpowergating.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/fiji_clockpowergating.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/fiji_dyn_defaults.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/fiji_hwmgr.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/fiji_powertune.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/fiji_powertune.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/fiji_thermal.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/fiji_thermal.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/iceland_clockpowergating.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/iceland_clockpowergating.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/iceland_dyn_defaults.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/iceland_hwmgr.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/iceland_hwmgr.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/iceland_powertune.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/iceland_powertune.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/iceland_thermal.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/iceland_thermal.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_clockpowergating.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_clockpowergating.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_dyn_defaults.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_hwmgr.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_powertune.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_powertune.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_thermal.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/polaris10_thermal.h
>   create mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_clockpowergating.c
>   create mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_clockpowergating.h
>   create mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_dyn_defaults.h
>   create mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.c
>   create mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_hwmgr.h
>   create mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_powertune.c
>   create mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_powertune.h
>   create mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.c
>   create mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/smu7_thermal.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/tonga_clockpowergating.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/tonga_clockpowergating.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/tonga_dyn_defaults.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/tonga_hwmgr.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/tonga_powertune.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/tonga_powertune.h
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/tonga_thermal.c
>   delete mode 100644 drivers/gpu/drm/amd/powerplay/hwmgr/tonga_thermal.h
>   create mode 100644 drivers/gpu/drm/amd/powerplay/inc/smu7_common.h
>   create mode 100644 drivers/gpu/drm/amd/powerplay/inc/smu7_ppsmc.h
>   create mode 100644 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.c
>   create mode 100644 drivers/gpu/drm/amd/powerplay/smumgr/fiji_smc.h
>   create mode 100644 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.c
>   create mode 100644 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smc.h
>   create mode 100644 drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smc.c
>   create mode 100644 drivers/gpu/drm/amd/powerplay/smumgr/polaris10_smc.h
>   create mode 100644 drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.c
>   create mode 100644 drivers/gpu/drm/amd/powerplay/smumgr/smu7_smumgr.h
>   create mode 100644 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.c
>   create mode 100644 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smc.h
>



More information about the amd-gfx mailing list