[PATCH v2 00/16] powerplay code refactoring.
Zhu, Rex
Rex.Zhu at amd.com
Tue Sep 13 12:12:42 UTC 2016
Hi Christian,
Thanks.
I will refine those patches.
Best Regards
Rex
-----Original Message-----
From: Christian König [mailto:deathsimple at vodafone.de]
Sent: Monday, September 12, 2016 6:52 PM
To: Zhu, Rex; amd-gfx at lists.freedesktop.org
Subject: Re: [PATCH v2 00/16] powerplay code refactoring.
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