powercap: occ: Fix the powercapping range allowed for user

OCC provides two levels of minimum powercap which are hard min
powercap and soft min powercap. Soft min powercap is lower than the
hard min powercap and users should be allowed to request till this
limit.

CC: stable # v5.8+
Fixes: c6aabe3f2eb5("powercap: occ: Add a generic powercap framework")
Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
---
 hw/occ.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

* Shilpa Bhat <shilpa.bhat@linux.vnet.ibm.com> [2018-08-16 11:12:22]:

> OCC provides two levels of minimum powercap which are hard min
> powercap and soft min powercap. Soft min powercap is lower than the
> hard min powercap and users should be allowed to request till this
> limit.
> 
> CC: stable # v5.8+
> Fixes: c6aabe3f2eb5("powercap: occ: Add a generic powercap framework")
> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>

Reviewed-by: Vaidyanathan Srinivasan <svaidy@linux.vnet.ibm.com>

> ---
>  hw/occ.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/occ.c b/hw/occ.c
> index a55bf8e..1140ccd 100644
> --- a/hw/occ.c
> +++ b/hw/occ.c
> @@ -225,9 +225,12 @@ struct occ_response_buffer {
>   *				power to maintain a power cap. Value of 100
>   *				means take all power from CPU.
>   * @pwr_cap_type:		Indicates type of power cap in effect
> - * @min_pwr_cap:		Minimum allowed system power cap in Watts
> + * @hard_min_pwr_cap:		Hard minimum system power cap in Watts.
> + *				Guaranteed unless hardware failure
>   * @max_pwr_cap:		Maximum allowed system power cap in Watts
>   * @cur_pwr_cap:		Current system power cap
> + * @soft_min_pwr_cap:		Soft powercap minimum. OCC may or may not be
> + *				able to maintain this
>   * @spare/reserved:		Unused data
>   * @cmd:			Opal Command Buffer
>   * @rsp:			OCC Response Buffer
> @@ -243,10 +246,11 @@ struct occ_dynamic_data {
>  	u8 quick_pwr_drop;
>  	u8 pwr_shifting_ratio;
>  	u8 pwr_cap_type;
> -	u16 min_pwr_cap;
> +	u16 hard_min_pwr_cap;
>  	u16 max_pwr_cap;
>  	u16 cur_pwr_cap;
> -	u8 pad[112];
> +	u16 soft_min_pwr_cap;
> +	u8 pad[110];
>  	struct opal_command_buffer cmd;
>  	struct occ_response_buffer rsp;
>  } __packed;
> @@ -1362,7 +1366,7 @@ int occ_get_powercap(u32 handle, u32 *pcap)
> 
>  	switch (powercap_get_attr(handle)) {
>  	case POWERCAP_OCC_MIN:
> -		*pcap = ddata->min_pwr_cap;
> +		*pcap = ddata->soft_min_pwr_cap;
>  		break;
>  	case POWERCAP_OCC_MAX:
>  		*pcap = ddata->max_pwr_cap;
> @@ -1410,7 +1414,7 @@ int occ_set_powercap(u32 handle, int token, u32 pcap)
>  		return OPAL_SUCCESS;
> 
>  	if (pcap && (pcap > ddata->max_pwr_cap ||
> -	    pcap < ddata->min_pwr_cap))
> +	    pcap < ddata->soft_min_pwr_cap))
>  		return OPAL_PARAMETER;

This fix allows a larger range of power-cap to get set from Linux inband.

--Vaidy

Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com> writes:
> OCC provides two levels of minimum powercap which are hard min
> powercap and soft min powercap. Soft min powercap is lower than the
> hard min powercap and users should be allowed to request till this
> limit.
>
> CC: stable # v5.8+
> Fixes: c6aabe3f2eb5("powercap: occ: Add a generic powercap framework")
> Signed-off-by: Shilpasri G Bhat <shilpa.bhat@linux.vnet.ibm.com>
> ---
>  hw/occ.c | 14 +++++++++-----
>  1 file changed, 9 insertions(+), 5 deletions(-)

This seems related to https://github.com/open-power/skiboot/issues/195
and should possibly have a Fixes: associated with it.

I made some improvements to the op-test testcase (now upstream
https://github.com/open-power/op-test-framework/blob/adeaa20/testcases/OpalSysfsTests.py#L107
)

and stock upstream on a DD2.1 witherspoon gets:
======================================================================
FAIL [114.099s]: test_opal_powercap (testcases.OpalSysfsTests.Skiroot)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/stewart/op-test-framework/testcases/OpalSysfsTests.py", line 120, in test_opal_powercap
    self.set_power_cap(min_powercap)
  File "/home/stewart/op-test-framework/testcases/OpalSysfsTests.py", line 98, in set_power_cap
    repr(valid_powercap_values)))
AssertionError: Retrieved powercap was not either the previous one (3050) or the one we're trying
 to set (1700). Got 1550, expected in [3050, 1700]

----------------------------------------------------------------------
Ran 1 test in 114.099s

and then, if you run it again without an IPL:

======================================================================
FAIL [58.049s]: test_opal_powercap (testcases.OpalSysfsTests.Skiroot)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/stewart/op-test-framework/testcases/OpalSysfsTests.py", line 119, in test_opal_powercap
    self.set_power_cap(max_powercap)
  File "/home/stewart/op-test-framework/testcases/OpalSysfsTests.py", line 105, in set_power_cap
    cur_powercap, value))
AssertionError: OPAL failed to set power cap value. Got 1550 when trying to set 3050.

----------------------------------------------------------------------
Ran 1 test in 58.049s

Which all looks pretty suspicious, as in - something is fundamentally
broken here.

I'm not convinced that this patch is the correct way to go, as it
changes current semantics. As such, I don't think I want to take as-is,
especially as after it there's no way to work out what the hard minimum
is.

I think the correct solution here is going to be twofold:

  1. add a soft min powercap to the device tree bindings.
  2. add support for this to the kernel.

and I'd be happy to take that kind of patch.