[Skiboot] [PATCH v4 2/2] occ: Add support to read V2 format of OCC-OPAL shared memory region
Joel Stanley
joel at jms.id.au
Fri Feb 12 10:51:55 AEDT 2016
On Thu, Feb 11, 2016 at 6:01 PM, Shilpasri G Bhat
<shilpa.bhat at linux.vnet.ibm.com> wrote:
> Add support to read OCC-OPAL shared memory region version2 to parse
> ultra-turbo pstates and core-to-max-pstate-allowed array and append
> it to device tree. Each element of core-to-max-pstate-allowed indicates
> the maximum pstate sustained with 'n' online cores.
>
> Signed-off-by: Shilpasri G Bhat <shilpa.bhat at linux.vnet.ibm.com>
Looks good! Really good work on addressing the issues found by review.
Reviewed-by: Joel Stanley <joel at jms.id.au>
> ---
> Changes from v3:
> - Added a link to github source code of OCC which defines the OCC-OPAL
> interface.
> - Moved nr_cores computation inside if (ultra_turbo_en) and initialized
> it to avoid gcc complaints
> - Removed type-casting malloc() returned pointer
> - s/printf/prerror
> - Added documentation.
> - Modified commit log
>
> Changes from v2:
> - Got rid of 'next' label.
> - Removed 'ultra_turbo_en' property from DT. The sole purpose of this
> property was to indicate if WOF is enabled. Host can use
> 'ibm,pstate-ultra-turbo' property instead which gets added to DT only
> when WOF is enabled.
> - Added __packed to struct occ_pstate_table which represents the
> memory layout of the OCC data copied to OCC-OPAL shared memory
> region in HOMER. OCC copies a packed structure(pstate table) here.
> So I am adding __packed while mapping the region in skiboot. There is
> nothing breaking with/without this attribute, but adding it just to make
> the structure future-proof. I can remove this if you feel it is not
> required.
>
> Changes from v1:
> - Allocate dt_core_max to number of cores present in the chip.
> - Replace '4' with sizeof(u32).
> - Remove label 'populate'
> - 'dt_core_max' is initialized to NULL
>
> doc/device-tree/ibm,opal/power-mgt.txt | 11 ++++++
> hw/occ.c | 63 ++++++++++++++++++++++++++++------
> 2 files changed, 63 insertions(+), 11 deletions(-)
>
> diff --git a/doc/device-tree/ibm,opal/power-mgt.txt b/doc/device-tree/ibm,opal/power-mgt.txt
> index 0852650..d9cadb8 100644
> --- a/doc/device-tree/ibm,opal/power-mgt.txt
> +++ b/doc/device-tree/ibm,opal/power-mgt.txt
> @@ -55,6 +55,17 @@ These properties list a voltage-identifier of each of the pstates listed in
> ibm,pstate-ids for the Vcs and Vdd values used for that pstate. Each VID is a
> single byte.
>
> +ibm,pstate-ultra-turbo ibm,pstate-turbo
> +---------------------------------------
> +
> +These properties are added when ultra-turbo(WOF) is enabled. These properties
> +give the max turbo and max ultra-turbo pstate.
> +
> +ibm,pstate-core-max
> +-------------------
> +
> +This property is added when ultra_turbo(WOF) is enabled. This property gives
> +the list of max pstate for each 'n' number of active cores in the chip.
>
> FIXME: document these:
> ibm,cpu-idle-state-flags
> diff --git a/hw/occ.c b/hw/occ.c
> index 0e3d953..987d007 100644
> --- a/hw/occ.c
> +++ b/hw/occ.c
> @@ -49,18 +49,24 @@ struct occ_pstate_entry {
> u32 freq_khz;
> };
>
> +/*
> + * OCC-OPAL Shared Memory Region Version 2
> + * https://github.com/open-power/occ/blob/master/src/occ/proc/proc_pstate.h
> + * Interface defined in 'sapphire_table_t'
> + */
> struct occ_pstate_table {
> u8 valid;
> u8 version;
> u8 throttle;
> s8 pstate_min;
> s8 pstate_nom;
> - s8 pstate_max;
> - u8 spare1;
> - u8 spare2;
> + s8 pstate_turbo;
> + s8 pstate_ultra_turbo;
> + u8 spare;
> u64 reserved;
> struct occ_pstate_entry pstates[MAX_PSTATES];
> -};
> + s8 core_max[16];
> +} __packed;
>
> DEFINE_LOG_ENTRY(OPAL_RC_OCC_LOAD, OPAL_PLATFORM_ERR_EVT, OPAL_OCC,
> OPAL_CEC_HARDWARE, OPAL_PREDICTIVE_ERR_GENERAL,
> @@ -142,11 +148,13 @@ static bool add_cpu_pstate_properties(s8 *pstate_nom)
> uint64_t occ_data_area;
> struct occ_pstate_table *occ_data;
> struct dt_node *power_mgt;
> - u8 nr_pstates;
> + u8 nr_pstates, nr_cores = 0;
> + s8 pmax;
> /* Arrays for device tree */
> u32 *dt_id, *dt_freq;
> u8 *dt_vdd, *dt_vcs;
> - bool rc;
> + s8 *dt_core_max = NULL;
> + bool rc, ultra_turbo_en;
> int i;
>
> prlog(PR_DEBUG, "OCC: CPU pstate state device tree init\n");
> @@ -171,10 +179,18 @@ static bool add_cpu_pstate_properties(s8 *pstate_nom)
> return false;
> }
>
> - nr_pstates = occ_data->pstate_max - occ_data->pstate_min + 1;
> + if (occ_data->version > 1 &&
> + occ_data->pstate_ultra_turbo > occ_data->pstate_turbo)
> + ultra_turbo_en = true;
> + else
> + ultra_turbo_en = false;
> +
> + pmax = ultra_turbo_en ? occ_data->pstate_ultra_turbo :
> + occ_data->pstate_turbo;
> + nr_pstates = pmax - occ_data->pstate_min + 1;
> prlog(PR_DEBUG, "OCC: Min %d Nom %d Max %d Nr States %d\n",
> occ_data->pstate_min, occ_data->pstate_nom,
> - occ_data->pstate_max, nr_pstates);
> + pmax, nr_pstates);
>
> if (nr_pstates <= 1 || nr_pstates > 128) {
> prerror("OCC: OCC range is not valid\n");
> @@ -215,6 +231,18 @@ static bool add_cpu_pstate_properties(s8 *pstate_nom)
> goto out_free_vdd;
> }
>
> + if (ultra_turbo_en) {
> + nr_cores = get_available_nr_cores_in_chip(chip->id);
> + dt_core_max = malloc(nr_cores * sizeof(s8));
> + if (!dt_core_max) {
> + prerror("OCC: dt_core_max alloc failure\n");
> + goto out_free_vcs;
> + }
> +
> + for (i = 0; i < nr_cores; i++)
> + dt_core_max[i] = occ_data->core_max[i];
> + }
> +
> for( i=0; i < nr_pstates; i++) {
> dt_id[i] = occ_data->pstates[i].id;
> dt_freq[i] = occ_data->pstates[i].freq_khz/1000;
> @@ -223,18 +251,31 @@ static bool add_cpu_pstate_properties(s8 *pstate_nom)
> }
>
> /* Add the device-tree entries */
> - dt_add_property(power_mgt, "ibm,pstate-ids", dt_id, nr_pstates * 4);
> - dt_add_property(power_mgt, "ibm,pstate-frequencies-mhz", dt_freq, nr_pstates * 4);
> + dt_add_property(power_mgt, "ibm,pstate-ids", dt_id,
> + nr_pstates * sizeof(u32));
> + dt_add_property(power_mgt, "ibm,pstate-frequencies-mhz", dt_freq,
> + nr_pstates * sizeof(u32));
> dt_add_property(power_mgt, "ibm,pstate-vdds", dt_vdd, nr_pstates);
> dt_add_property(power_mgt, "ibm,pstate-vcss", dt_vcs, nr_pstates);
> dt_add_property_cells(power_mgt, "ibm,pstate-min", occ_data->pstate_min);
> dt_add_property_cells(power_mgt, "ibm,pstate-nominal", occ_data->pstate_nom);
> - dt_add_property_cells(power_mgt, "ibm,pstate-max", occ_data->pstate_max);
> + dt_add_property_cells(power_mgt, "ibm,pstate-max", pmax);
> +
> + if (ultra_turbo_en) {
> + dt_add_property_cells(power_mgt, "ibm,pstate-turbo",
> + occ_data->pstate_turbo);
> + dt_add_property_cells(power_mgt, "ibm,pstate-ultra-turbo",
> + occ_data->pstate_ultra_turbo);
> + dt_add_property(power_mgt, "ibm,pstate-core-max", dt_core_max,
> + nr_cores);
> + free(dt_core_max);
> + }
>
> /* Return pstate to set for each core */
> *pstate_nom = occ_data->pstate_nom;
> rc = true;
>
> +out_free_vcs:
> free(dt_vcs);
> out_free_vdd:
> free(dt_vdd);
> --
> 1.9.3
>
More information about the Skiboot
mailing list