diff mbox

[v12,05/17] drm/i915/guc/slpc: Add SLPC communication interfaces

Message ID 1522398722-12161-6-git-send-email-sagar.a.kamble@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

sagar.a.kamble@intel.com March 30, 2018, 8:31 a.m. UTC
Communication with SLPC is via Host to GuC interrupt through shared data
and parameters. This patch defines the structure of shared data,
parameters, data structure to be passed as input and received as output
from SLPC. This patch also defines the events to be sent as input and
status values output by GuC on processing SLPC events.
SLPC shared data has details of SKU type, Slice count, IA Perf MSR values,
SLPC state, Power source/plan, SLPC tasks status. Parameters allow
overriding task control, frequency range etc.

v1: fix whitespace (Sagar)

v2-v3: Rebase.

v4: Updated with GuC firmware v9.

v5: Added definition of input and output data structures for SLPC
    events. Updated commit message.

v6: Removed definition of host2guc_slpc. Will be added in the next
    patch that uses it. Commit subject update. Rebase.

v7: Added definition of SLPC_RESET_FLAG_TDR_OCCURRED to be sent
    throgh SLPC reset in case of engine reset. Moved all Host/SLPC
    interfaces from later patches to this patch. Commit message update.

v8: Updated value of SLPC_RESET_FLAG_TDR_OCCURRED.

v9: Removed struct slpc_param, slpc_paramlist and corresponding defines.
    Will be added in later patches where they are used.

v10: Rebase. Prepared separate header for SLPC firmware interface.

Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
Cc: Jeff McGee <jeff.mcgee@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_slpc.h      |   2 +
 drivers/gpu/drm/i915/intel_guc_slpc_fwif.h | 211 +++++++++++++++++++++++++++++
 2 files changed, 213 insertions(+)
 create mode 100644 drivers/gpu/drm/i915/intel_guc_slpc_fwif.h

Comments

Michal Wajdeczko March 30, 2018, 1:37 p.m. UTC | #1
On Fri, 30 Mar 2018 10:31:50 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

> Communication with SLPC is via Host to GuC interrupt through shared data
> and parameters. This patch defines the structure of shared data,
> parameters, data structure to be passed as input and received as output
> from SLPC. This patch also defines the events to be sent as input and
> status values output by GuC on processing SLPC events.
> SLPC shared data has details of SKU type, Slice count, IA Perf MSR  
> values,
> SLPC state, Power source/plan, SLPC tasks status. Parameters allow
> overriding task control, frequency range etc.
>
> v1: fix whitespace (Sagar)
>
> v2-v3: Rebase.
>
> v4: Updated with GuC firmware v9.
>
> v5: Added definition of input and output data structures for SLPC
>     events. Updated commit message.
>
> v6: Removed definition of host2guc_slpc. Will be added in the next
>     patch that uses it. Commit subject update. Rebase.
>
> v7: Added definition of SLPC_RESET_FLAG_TDR_OCCURRED to be sent
>     throgh SLPC reset in case of engine reset. Moved all Host/SLPC
>     interfaces from later patches to this patch. Commit message update.
>
> v8: Updated value of SLPC_RESET_FLAG_TDR_OCCURRED.
>
> v9: Removed struct slpc_param, slpc_paramlist and corresponding defines.
>     Will be added in later patches where they are used.
>
> v10: Rebase. Prepared separate header for SLPC firmware interface.
>
> Signed-off-by: Tom O'Rourke <Tom.O'Rourke@intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
> Cc: Jeff McGee <jeff.mcgee@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_slpc.h      |   2 +
>  drivers/gpu/drm/i915/intel_guc_slpc_fwif.h | 211  
> +++++++++++++++++++++++++++++
>  2 files changed, 213 insertions(+)
>  create mode 100644 drivers/gpu/drm/i915/intel_guc_slpc_fwif.h
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_slpc.h  
> b/drivers/gpu/drm/i915/intel_guc_slpc.h
> index 66c76fe..81250c0 100644
> --- a/drivers/gpu/drm/i915/intel_guc_slpc.h
> +++ b/drivers/gpu/drm/i915/intel_guc_slpc.h
> @@ -6,6 +6,8 @@
>  #ifndef _INTEL_GUC_SLPC_H_
>  #define _INTEL_GUC_SLPC_H_
> +#include <intel_guc_slpc_fwif.h>

Please use "" instead of <>

> +
>  struct intel_guc_slpc {
>  };
> diff --git a/drivers/gpu/drm/i915/intel_guc_slpc_fwif.h  
> b/drivers/gpu/drm/i915/intel_guc_slpc_fwif.h
> new file mode 100644
> index 0000000..9400af4
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_guc_slpc_fwif.h
> @@ -0,0 +1,211 @@
> +/*
> + * SPDX-License-Identifier: MIT
> + *
> + * Copyright © 2015-2018 Intel Corporation
> + */
> +#ifndef _INTEL_GUC_SLPC_FWIF_H_
> +#define _INTEL_GUC_SLPC_FWIF_H_
> +
> +#include <linux/types.h>
> +
> +enum slpc_status {

s/slpc_status/intel_guc_slpc_status

> +	SLPC_STATUS_OK = 0,
> +	SLPC_STATUS_ERROR = 1,
> +	SLPC_STATUS_ILLEGAL_COMMAND = 2,
> +	SLPC_STATUS_INVALID_ARGS = 3,
> +	SLPC_STATUS_INVALID_PARAMS = 4,
> +	SLPC_STATUS_INVALID_DATA = 5,
> +	SLPC_STATUS_OUT_OF_RANGE = 6,
> +	SLPC_STATUS_NOT_SUPPORTED = 7,
> +	SLPC_STATUS_NOT_IMPLEMENTED = 8,
> +	SLPC_STATUS_NO_DATA = 9,
> +	SLPC_STATUS_EVENT_NOT_REGISTERED = 10,
> +	SLPC_STATUS_REGISTER_LOCKED = 11,
> +	SLPC_STATUS_TEMPORARILY_UNAVAILABLE = 12,
> +	SLPC_STATUS_VALUE_ALREADY_SET = 13,
> +	SLPC_STATUS_VALUE_ALREADY_UNSET = 14,
> +	SLPC_STATUS_VALUE_NOT_CHANGED = 15,
> +	SLPC_STATUS_MEMIO_ERROR = 16,
> +	SLPC_STATUS_EVENT_QUEUED_REQ_DPC = 17,
> +	SLPC_STATUS_EVENT_QUEUED_NOREQ_DPC = 18,
> +	SLPC_STATUS_NO_EVENT_QUEUED = 19,
> +	SLPC_STATUS_OUT_OF_SPACE = 20,
> +	SLPC_STATUS_TIMEOUT = 21,
> +	SLPC_STATUS_NO_LOCK = 22,
> +	SLPC_STATUS_MAX

s/SLPC_STATUS/INTEL_GUC_SLPC_STATUS

> +};
> +
> +enum slpc_event_id {

s/slpc_event_id/intel_guc_slpc_event

> +	SLPC_EVENT_RESET = 0,
> +	SLPC_EVENT_SHUTDOWN = 1,
> +	SLPC_EVENT_PLATFORM_INFO_CHANGE = 2,
> +	SLPC_EVENT_DISPLAY_MODE_CHANGE = 3,
> +	SLPC_EVENT_FLIP_COMPLETE = 4,
> +	SLPC_EVENT_QUERY_TASK_STATE = 5,
> +	SLPC_EVENT_PARAMETER_SET = 6,
> +	SLPC_EVENT_PARAMETER_UNSET = 7,

s/SLPC_EVENT/INTEL_GUC_SLPC_EVENT

> +};
> +
> +enum slpc_param_id {

s/slpc_param_id/intel_guc_slpc_param

> +	SLPC_PARAM_TASK_ENABLE_GTPERF = 0,
> +	SLPC_PARAM_TASK_DISABLE_GTPERF = 1,
> +	SLPC_PARAM_TASK_ENABLE_BALANCER = 2,
> +	SLPC_PARAM_TASK_DISABLE_BALANCER = 3,
> +	SLPC_PARAM_TASK_ENABLE_DCC = 4,
> +	SLPC_PARAM_TASK_DISABLE_DCC = 5,
> +	SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ = 6,
> +	SLPC_PARAM_GLOBAL_MAX_GT_UNSLICE_FREQ_MHZ = 7,
> +	SLPC_PARAM_GLOBAL_MIN_GT_SLICE_FREQ_MHZ = 8,
> +	SLPC_PARAM_GLOBAL_MAX_GT_SLICE_FREQ_MHZ = 9,
> +	SLPC_PARAM_GTPERF_THRESHOLD_MAX_FPS = 10,
> +	SLPC_PARAM_GLOBAL_DISABLE_GT_FREQ_MANAGEMENT = 11,
> +	SLPC_PARAM_GTPERF_ENABLE_FRAMERATE_STALLING = 12,
> +	SLPC_PARAM_GLOBAL_DISABLE_RC6_MODE_CHANGE = 13,
> +	SLPC_PARAM_GLOBAL_OC_UNSLICE_FREQ_MHZ = 14,
> +	SLPC_PARAM_GLOBAL_OC_SLICE_FREQ_MHZ = 15,
> +	SLPC_PARAM_GLOBAL_ENABLE_IA_GT_BALANCING = 16,
> +	SLPC_PARAM_GLOBAL_ENABLE_ADAPTIVE_BURST_TURBO = 17,
> +	SLPC_PARAM_GLOBAL_ENABLE_EVAL_MODE = 18,
> +	SLPC_PARAM_GLOBAL_ENABLE_BALANCER_IN_NON_GAMING_MODE = 19,

s/SLPC_PARAM/INTEL_GUC_SLPC_PARAM

> +	SLPC_MAX_PARAM,
> +	SLPC_KMD_MAX_PARAM = 32,

hmm, do we really need these two ? please drop

or maybe these are related to SLPC_MAX_OVERRIDE_PARAMETERS ?

> +};
> +
> +enum slpc_global_state {

s/slpc_global_state/intel_guc_slpc_state

> +	SLPC_GLOBAL_STATE_NOT_RUNNING = 0,
> +	SLPC_GLOBAL_STATE_INITIALIZING = 1,
> +	SLPC_GLOBAL_STATE_RESETTING = 2,
> +	SLPC_GLOBAL_STATE_RUNNING = 3,
> +	SLPC_GLOBAL_STATE_SHUTTING_DOWN = 4,
> +	SLPC_GLOBAL_STATE_ERROR = 5

s/SLPC_GLOBAL_STATE/INTEL_GUC_SLPC_STATE

> +};
> +
> +enum slpc_platform_sku {
> +	SLPC_PLATFORM_SKU_UNDEFINED = 0,
> +	SLPC_PLATFORM_SKU_ULX = 1,
> +	SLPC_PLATFORM_SKU_ULT = 2,
> +	SLPC_PLATFORM_SKU_T = 3,
> +	SLPC_PLATFORM_SKU_MOBL = 4,
> +	SLPC_PLATFORM_SKU_DT = 5,
> +	SLPC_PLATFORM_SKU_UNKNOWN = 6,

hmm, this should be already known to GuC/SLPC.
and if still needed, can it be made non-SLPC specific?

> +};
> +
> +enum slpc_power_source {

s/slpc_power_source/intel_power_source

> +	SLPC_POWER_SOURCE_UNDEFINED = 0,
> +	SLPC_POWER_SOURCE_AC = 1,
> +	SLPC_POWER_SOURCE_DC = 2,
> +	SLPC_POWER_SOURCE_UNKNOWN = 3,

s/SLPC_POWER_SOURCE/INTEL_POWER_SOURCE

> +};
> +
> +enum slpc_power_plan {
> +	SLPC_POWER_PLAN_UNDEFINED = 0,
> +	SLPC_POWER_PLAN_BATTERY_SAVER = 1,
> +	SLPC_POWER_PLAN_BALANCED = 2,
> +	SLPC_POWER_PLAN_PERFORMANCE = 3,
> +	SLPC_POWER_PLAN_UNKNOWN = 4,
> +};
> +
> +struct slpc_platform_info {

s/slpc_platform_info/intel_guc_slpc_platform

> +	u8 sku;
> +	u8 slice_count;
> +	u8 reserved;
> +	u8 power_plan_source;

 from macros below, it looks that this is bitfield and
in other struct bitfields are defined explicitly...

what about defining related MASK/SHIFT and MAKE/TO macros here?

> +	u8 p0_freq;
> +	u8 p1_freq;
> +	u8 pe_freq;
> +	u8 pn_freq;
> +	u32 reserved1;
> +	u32 reserved2;
> +} __packed;
> +
> +struct slpc_task_state_data {
> +	union {
> +		u32 bitfield1;
> +		struct {
> +			u32 gtperf_task_active:1;
> +			u32 gtperf_stall_possible:1;
> +			u32 gtperf_gaming_mode:1;
> +			u32 gtperf_target_fps:8;
> +			u32 dcc_task_active:1;
> +			u32 in_dcc:1;
> +			u32 in_dct:1;
> +			u32 freq_switch_active:1;
> +			u32 ibc_enabled:1;
> +			u32 ibc_active:1;
> +			u32 pg1_enabled:1;
> +			u32 pg1_active:1;
> +			u32 reserved:13;

All register bits and most of shared data are defined as
macros, maybe we can do the same for SLPC ?

#define INTEL_GUC_SLPC_TASK_GTPERF_ACTIVE	(1 << 0)
#define INTEL_GUC_SLPC_TASK_GTPERF_STALL	(1 << 1)
...

> +		};
> +	};
> +	union {
> +		u32 bitfield2;
> +		struct {
> +			u32 max_unslice_freq:8;
> +			u32 min_unslice_freq:8;
> +			u32 max_slice_freq:8;
> +			u32 min_slice_freq:8;
> +		};
> +	};
> +} __packed;
> +
> +#define SLPC_MAX_OVERRIDE_PARAMETERS 192
> +#define SLPC_OVERRIDE_BITFIELD_SIZE \
> +		((SLPC_MAX_OVERRIDE_PARAMETERS + 31) / 32)

are there "NON-OVERRIDE" parameters?

> +
> +struct slpc_shared_data {

s/slpc_shared_data/intel_guc_slpc_shared_data
or
s/slpc_shared_data/intel_guc_slpc_data

> +	u32 reserved;

hmm, starting with reserved is odd ;)

> +	u32 shared_data_size;

s/shared_data_size/size

> +	u32 global_state;
> +	struct slpc_platform_info platform_info;
> +	struct slpc_task_state_data task_state_data;
> +	u32 override_params_set_bits[SLPC_OVERRIDE_BITFIELD_SIZE];
> +	u32 override_params_values[SLPC_MAX_OVERRIDE_PARAMETERS];

above two can be promoted to separate struct:

struct intel_guc_slpc_params {
	u32 valid[INTEL_GUC_SLPC_PARAMS_BITFIELD_SIZE];
	u32 values[INTEL_GUC_SLPC_MAX_PARAMS];
};

> +} __packed;
> +
> +enum slpc_reset_flags {

s/slpc_reset_flags/intel_guc_slpc_reset_flags

> +	SLPC_RESET_FLAG_TDR_OCCURRED = (1 << 0)

s/SLPC_RESET_FLAG/INTEL_GUC_SLPC_RESET_FLAG
and to minimize later diffs add trailing ","

> +};
> +
> +#define SLPC_EVENT_MAX_INPUT_ARGS  7
> +#define SLPC_EVENT_MAX_OUTPUT_ARGS 1
> +
> +union slpc_event_input_header {
> +	u32 value;
> +	struct {
> +		u32 num_args:8;
> +		u32 event_id:8;
> +	};

I'm not sure that this struct deserves separate definition

> +};
> +
> +struct slpc_event_input {
> +	u32 h2g_action_id;

what h2g action is this ?
if this event is sent over CTB then probably num_args is
redundant as total CTB message len is already stored in
CT header...

> +	union slpc_event_input_header header;
> +	u32 args[SLPC_EVENT_MAX_INPUT_ARGS];
> +} __packed;
> +
> +union slpc_event_output_header {
> +	u32 value;
> +	struct {
> +		u32 num_args:8;
> +		u32 event_id:8;
> +		u32 status:16;
> +	};
> +};
> +
> +struct slpc_event_output {
> +	u32 reserved;

is this needed ?

> +	union slpc_event_output_header header;

hmm, "header" should be first ;)

> +	u32 args[SLPC_EVENT_MAX_OUTPUT_ARGS];
> +} __packed;
> +
> +#define SLPC_EVENT(id, argc)		 ((u32)(id) << 8 | (argc))
> +#define SLPC_POWER_PLAN_SOURCE(plan, source) ((plan) | ((source) << 6))
> +#define SLPC_POWER_PLAN(plan_source)	 ((plan_source) & 0x3F)
> +#define SLPC_POWER_SOURCE(plan_source)	 ((plan_source) >> 6)

maybe:

	INTEL_GUC_SLPC_MAKE_EVENT
	INTEL_GUC_SLPC_MAKE_POWER_INFO
	INTEL_GUC_SLPC_POWER_INFO_TO_PLAN
	INTEL_GUC_SLPC_POWER_INFO_TO_SOURCE

and use SHIFT/MASK macros

> +
> +#define SLPC_PARAM_TASK_DEFAULT  0
> +#define SLPC_PARAM_TASK_ENABLED  1
> +#define SLPC_PARAM_TASK_DISABLED 2
> +#define SLPC_PARAM_TASK_UNKNOWN  3

in other places you are using enums, why #defines here?

> +
> +#endif
sagar.a.kamble@intel.com March 30, 2018, 3:57 p.m. UTC | #2
On 3/30/2018 7:07 PM, Michal Wajdeczko wrote:
> On Fri, 30 Mar 2018 10:31:50 +0200, Sagar Arun Kamble 
> <sagar.a.kamble@intel.com> wrote:
<snip>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_slpc.h 
>> b/drivers/gpu/drm/i915/intel_guc_slpc.h
>> index 66c76fe..81250c0 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_slpc.h
>> +++ b/drivers/gpu/drm/i915/intel_guc_slpc.h
>> @@ -6,6 +6,8 @@
>>  #ifndef _INTEL_GUC_SLPC_H_
>>  #define _INTEL_GUC_SLPC_H_
>> +#include <intel_guc_slpc_fwif.h>
>
> Please use "" instead of <>
Yes. will hopefully not forget this next time :)
>
>> +
>>  struct intel_guc_slpc {
>>  };
>> diff --git a/drivers/gpu/drm/i915/intel_guc_slpc_fwif.h 
>> b/drivers/gpu/drm/i915/intel_guc_slpc_fwif.h
>> new file mode 100644
>> index 0000000..9400af4
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_guc_slpc_fwif.h
>> @@ -0,0 +1,211 @@
>> +/*
>> + * SPDX-License-Identifier: MIT
>> + *
>> + * Copyright © 2015-2018 Intel Corporation
>> + */
>> +#ifndef _INTEL_GUC_SLPC_FWIF_H_
>> +#define _INTEL_GUC_SLPC_FWIF_H_
>> +
>> +#include <linux/types.h>
>> +
>> +enum slpc_status {
>
> s/slpc_status/intel_guc_slpc_status
>
I have done this because of following reasons:
1. GuC SLPC interface file shared by firmware team names enums/structs 
as SLPM_* or SLPM_KMD_*. i understand that this isn't strict requirement. :)
2. INTEL_GUC_STATUS|EVENT|PARAM_* becomes more than 80 chars for some values
3. I wanted intel_guc_slpc_fwif.h to be included only in 
intel_guc_slpc.c from where we can export functions as intel_guc_slpc_*
     static functions in intel_guc_slpc.c are named as slpc_*.
    In my series, point 3 above is not done but that was intent. There 
is access to SLPC enums from debugfs.c. Would consolidate all in 
intel_guc_slpc.c and export.

Does this plan look good?
>> +    SLPC_STATUS_OK = 0,
>> +    SLPC_STATUS_ERROR = 1,
>> +    SLPC_STATUS_ILLEGAL_COMMAND = 2,
>> +    SLPC_STATUS_INVALID_ARGS = 3,
>> +    SLPC_STATUS_INVALID_PARAMS = 4,
>> +    SLPC_STATUS_INVALID_DATA = 5,
>> +    SLPC_STATUS_OUT_OF_RANGE = 6,
>> +    SLPC_STATUS_NOT_SUPPORTED = 7,
>> +    SLPC_STATUS_NOT_IMPLEMENTED = 8,
>> +    SLPC_STATUS_NO_DATA = 9,
>> +    SLPC_STATUS_EVENT_NOT_REGISTERED = 10,
>> +    SLPC_STATUS_REGISTER_LOCKED = 11,
>> +    SLPC_STATUS_TEMPORARILY_UNAVAILABLE = 12,
>> +    SLPC_STATUS_VALUE_ALREADY_SET = 13,
>> +    SLPC_STATUS_VALUE_ALREADY_UNSET = 14,
>> +    SLPC_STATUS_VALUE_NOT_CHANGED = 15,
>> +    SLPC_STATUS_MEMIO_ERROR = 16,
>> +    SLPC_STATUS_EVENT_QUEUED_REQ_DPC = 17,
>> +    SLPC_STATUS_EVENT_QUEUED_NOREQ_DPC = 18,
>> +    SLPC_STATUS_NO_EVENT_QUEUED = 19,
>> +    SLPC_STATUS_OUT_OF_SPACE = 20,
>> +    SLPC_STATUS_TIMEOUT = 21,
>> +    SLPC_STATUS_NO_LOCK = 22,
>> +    SLPC_STATUS_MAX
>
> s/SLPC_STATUS/INTEL_GUC_SLPC_STATUS
>
>> +};
>> +
>> +enum slpc_event_id {
>
> s/slpc_event_id/intel_guc_slpc_event
>
>> +    SLPC_EVENT_RESET = 0,
>> +    SLPC_EVENT_SHUTDOWN = 1,
>> +    SLPC_EVENT_PLATFORM_INFO_CHANGE = 2,
>> +    SLPC_EVENT_DISPLAY_MODE_CHANGE = 3,
>> +    SLPC_EVENT_FLIP_COMPLETE = 4,
>> +    SLPC_EVENT_QUERY_TASK_STATE = 5,
>> +    SLPC_EVENT_PARAMETER_SET = 6,
>> +    SLPC_EVENT_PARAMETER_UNSET = 7,
>
> s/SLPC_EVENT/INTEL_GUC_SLPC_EVENT
>
>> +};
>> +
>> +enum slpc_param_id {
>
> s/slpc_param_id/intel_guc_slpc_param
>
>> +    SLPC_PARAM_TASK_ENABLE_GTPERF = 0,
>> +    SLPC_PARAM_TASK_DISABLE_GTPERF = 1,
>> +    SLPC_PARAM_TASK_ENABLE_BALANCER = 2,
>> +    SLPC_PARAM_TASK_DISABLE_BALANCER = 3,
>> +    SLPC_PARAM_TASK_ENABLE_DCC = 4,
>> +    SLPC_PARAM_TASK_DISABLE_DCC = 5,
>> +    SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ = 6,
>> +    SLPC_PARAM_GLOBAL_MAX_GT_UNSLICE_FREQ_MHZ = 7,
>> +    SLPC_PARAM_GLOBAL_MIN_GT_SLICE_FREQ_MHZ = 8,
>> +    SLPC_PARAM_GLOBAL_MAX_GT_SLICE_FREQ_MHZ = 9,
>> +    SLPC_PARAM_GTPERF_THRESHOLD_MAX_FPS = 10,
>> +    SLPC_PARAM_GLOBAL_DISABLE_GT_FREQ_MANAGEMENT = 11,
>> +    SLPC_PARAM_GTPERF_ENABLE_FRAMERATE_STALLING = 12,
>> +    SLPC_PARAM_GLOBAL_DISABLE_RC6_MODE_CHANGE = 13,
>> +    SLPC_PARAM_GLOBAL_OC_UNSLICE_FREQ_MHZ = 14,
>> +    SLPC_PARAM_GLOBAL_OC_SLICE_FREQ_MHZ = 15,
>> +    SLPC_PARAM_GLOBAL_ENABLE_IA_GT_BALANCING = 16,
>> +    SLPC_PARAM_GLOBAL_ENABLE_ADAPTIVE_BURST_TURBO = 17,
>> +    SLPC_PARAM_GLOBAL_ENABLE_EVAL_MODE = 18,
>> +    SLPC_PARAM_GLOBAL_ENABLE_BALANCER_IN_NON_GAMING_MODE = 19,
>
> s/SLPC_PARAM/INTEL_GUC_SLPC_PARAM
>
>> +    SLPC_MAX_PARAM,
>> +    SLPC_KMD_MAX_PARAM = 32,
>
> hmm, do we really need these two ? please drop
>
> or maybe these are related to SLPC_MAX_OVERRIDE_PARAMETERS ?
Ok. Will drop SLPC_KMD_MAX_PARAM.
There are total 192 params (MAX_OVERRIDE_PARAMS) out of which 32 
(KMD_MAX_PARAM) params can be exposed
to KMD, but currently only params till MAX_PARAM < 32 are exported.
>
>> +};
>> +
>> +enum slpc_global_state {
>
> s/slpc_global_state/intel_guc_slpc_state
>
>> +    SLPC_GLOBAL_STATE_NOT_RUNNING = 0,
>> +    SLPC_GLOBAL_STATE_INITIALIZING = 1,
>> +    SLPC_GLOBAL_STATE_RESETTING = 2,
>> +    SLPC_GLOBAL_STATE_RUNNING = 3,
>> +    SLPC_GLOBAL_STATE_SHUTTING_DOWN = 4,
>> +    SLPC_GLOBAL_STATE_ERROR = 5
>
> s/SLPC_GLOBAL_STATE/INTEL_GUC_SLPC_STATE
>
>> +};
>> +
>> +enum slpc_platform_sku {
>> +    SLPC_PLATFORM_SKU_UNDEFINED = 0,
>> +    SLPC_PLATFORM_SKU_ULX = 1,
>> +    SLPC_PLATFORM_SKU_ULT = 2,
>> +    SLPC_PLATFORM_SKU_T = 3,
>> +    SLPC_PLATFORM_SKU_MOBL = 4,
>> +    SLPC_PLATFORM_SKU_DT = 5,
>> +    SLPC_PLATFORM_SKU_UNKNOWN = 6,
>
> hmm, this should be already known to GuC/SLPC.
> and if still needed, can it be made non-SLPC specific?
>
In current firmwares this is not possible but certainly can be changed 
in newer firmwares.
Will suggest this to Bob, Daisy.
Will keep it SLPC specific for now as only SLPC uses it.
>> +};
>> +
>> +enum slpc_power_source {
>
> s/slpc_power_source/intel_power_source
>
>> +    SLPC_POWER_SOURCE_UNDEFINED = 0,
>> +    SLPC_POWER_SOURCE_AC = 1,
>> +    SLPC_POWER_SOURCE_DC = 2,
>> +    SLPC_POWER_SOURCE_UNKNOWN = 3,
>
> s/SLPC_POWER_SOURCE/INTEL_POWER_SOURCE
>
>> +};
>> +
>> +enum slpc_power_plan {
>> +    SLPC_POWER_PLAN_UNDEFINED = 0,
>> +    SLPC_POWER_PLAN_BATTERY_SAVER = 1,
>> +    SLPC_POWER_PLAN_BALANCED = 2,
>> +    SLPC_POWER_PLAN_PERFORMANCE = 3,
>> +    SLPC_POWER_PLAN_UNKNOWN = 4,
>> +};
>> +
>> +struct slpc_platform_info {
>
> s/slpc_platform_info/intel_guc_slpc_platform
>
>> +    u8 sku;
>> +    u8 slice_count;
>> +    u8 reserved;
>> +    u8 power_plan_source;
>
> from macros below, it looks that this is bitfield and
> in other struct bitfields are defined explicitly...
>
> what about defining related MASK/SHIFT and MAKE/TO macros here?
>
Yes. Can add those.
>> +    u8 p0_freq;
>> +    u8 p1_freq;
>> +    u8 pe_freq;
>> +    u8 pn_freq;
>> +    u32 reserved1;
>> +    u32 reserved2;
>> +} __packed;
>> +
>> +struct slpc_task_state_data {
>> +    union {
>> +        u32 bitfield1;
>> +        struct {
>> +            u32 gtperf_task_active:1;
>> +            u32 gtperf_stall_possible:1;
>> +            u32 gtperf_gaming_mode:1;
>> +            u32 gtperf_target_fps:8;
>> +            u32 dcc_task_active:1;
>> +            u32 in_dcc:1;
>> +            u32 in_dct:1;
>> +            u32 freq_switch_active:1;
>> +            u32 ibc_enabled:1;
>> +            u32 ibc_active:1;
>> +            u32 pg1_enabled:1;
>> +            u32 pg1_active:1;
>> +            u32 reserved:13;
>
> All register bits and most of shared data are defined as
> macros, maybe we can do the same for SLPC ?
>
> #define INTEL_GUC_SLPC_TASK_GTPERF_ACTIVE    (1 << 0)
> #define INTEL_GUC_SLPC_TASK_GTPERF_STALL    (1 << 1)
> ...
>
>> +        };
>> +    };
>> +    union {
>> +        u32 bitfield2;
>> +        struct {
>> +            u32 max_unslice_freq:8;
>> +            u32 min_unslice_freq:8;
>> +            u32 max_slice_freq:8;
>> +            u32 min_slice_freq:8;
>> +        };
>> +    };
>> +} __packed;
>> +
>> +#define SLPC_MAX_OVERRIDE_PARAMETERS 192
>> +#define SLPC_OVERRIDE_BITFIELD_SIZE \
>> +        ((SLPC_MAX_OVERRIDE_PARAMETERS + 31) / 32)
>
> are there "NON-OVERRIDE" parameters?
>
Yes. Non-accessible ones (>MAX_PARAM)
>> +
>> +struct slpc_shared_data {
>
> s/slpc_shared_data/intel_guc_slpc_shared_data
> or
> s/slpc_shared_data/intel_guc_slpc_data
>
>> +    u32 reserved;
>
> hmm, starting with reserved is odd ;)
Yes.
>
>> +    u32 shared_data_size;
>
> s/shared_data_size/size
>
Will do this.
>> +    u32 global_state;
>> +    struct slpc_platform_info platform_info;
>> +    struct slpc_task_state_data task_state_data;
>> +    u32 override_params_set_bits[SLPC_OVERRIDE_BITFIELD_SIZE];
>> +    u32 override_params_values[SLPC_MAX_OVERRIDE_PARAMETERS];
>
> above two can be promoted to separate struct:
>
> struct intel_guc_slpc_params {
>     u32 valid[INTEL_GUC_SLPC_PARAMS_BITFIELD_SIZE];
>     u32 values[INTEL_GUC_SLPC_MAX_PARAMS];
> };
>
Sure.
>> +} __packed;
>> +
>> +enum slpc_reset_flags {
>
> s/slpc_reset_flags/intel_guc_slpc_reset_flags
>
>> +    SLPC_RESET_FLAG_TDR_OCCURRED = (1 << 0)
>
> s/SLPC_RESET_FLAG/INTEL_GUC_SLPC_RESET_FLAG
> and to minimize later diffs add trailing ","
>
>> +};
>> +
>> +#define SLPC_EVENT_MAX_INPUT_ARGS  7
>> +#define SLPC_EVENT_MAX_OUTPUT_ARGS 1
>> +
>> +union slpc_event_input_header {
>> +    u32 value;
>> +    struct {
>> +        u32 num_args:8;
>> +        u32 event_id:8;
>> +    };
>
> I'm not sure that this struct deserves separate definition
>
>> +};
>> +
>> +struct slpc_event_input {
>> +    u32 h2g_action_id;
>
> what h2g action is this ?
Seems to be not used by even by GuC SLPC. another change for interface 
update.
> if this event is sent over CTB then probably num_args is
> redundant as total CTB message len is already stored in
> CT header...
>
Agree.
>> +    union slpc_event_input_header header;
>> +    u32 args[SLPC_EVENT_MAX_INPUT_ARGS];
>> +} __packed;
>> +
>> +union slpc_event_output_header {
>> +    u32 value;
>> +    struct {
>> +        u32 num_args:8;
>> +        u32 event_id:8;
>> +        u32 status:16;
>> +    };
>> +};
>> +
>> +struct slpc_event_output {
>> +    u32 reserved;
>
> is this needed ?
>
Can skip. What I have done is take entire SLPC interface file from 
firmware source and just rename things.
Agree that such things should not be exported if they are not going to 
be used. Will share with Bob/Daisy.
>> +    union slpc_event_output_header header;
>
> hmm, "header" should be first ;)
>
>> +    u32 args[SLPC_EVENT_MAX_OUTPUT_ARGS];
>> +} __packed;
>> +
>> +#define SLPC_EVENT(id, argc)         ((u32)(id) << 8 | (argc))
>> +#define SLPC_POWER_PLAN_SOURCE(plan, source) ((plan) | ((source) << 6))
>> +#define SLPC_POWER_PLAN(plan_source)     ((plan_source) & 0x3F)
>> +#define SLPC_POWER_SOURCE(plan_source)     ((plan_source) >> 6)
>
> maybe:
>
>     INTEL_GUC_SLPC_MAKE_EVENT
>     INTEL_GUC_SLPC_MAKE_POWER_INFO
>     INTEL_GUC_SLPC_POWER_INFO_TO_PLAN
>     INTEL_GUC_SLPC_POWER_INFO_TO_SOURCE
>
> and use SHIFT/MASK macros
>
Yes but need your thoughts on INTEL_GUC prefix.
>> +
>> +#define SLPC_PARAM_TASK_DEFAULT  0
>> +#define SLPC_PARAM_TASK_ENABLED  1
>> +#define SLPC_PARAM_TASK_DISABLED 2
>> +#define SLPC_PARAM_TASK_UNKNOWN  3
>
> in other places you are using enums, why #defines here?
>
Will unify.

Thanks for the review
>> +
>> +#endif
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_guc_slpc.h b/drivers/gpu/drm/i915/intel_guc_slpc.h
index 66c76fe..81250c0 100644
--- a/drivers/gpu/drm/i915/intel_guc_slpc.h
+++ b/drivers/gpu/drm/i915/intel_guc_slpc.h
@@ -6,6 +6,8 @@ 
 #ifndef _INTEL_GUC_SLPC_H_
 #define _INTEL_GUC_SLPC_H_
 
+#include <intel_guc_slpc_fwif.h>
+
 struct intel_guc_slpc {
 };
 
diff --git a/drivers/gpu/drm/i915/intel_guc_slpc_fwif.h b/drivers/gpu/drm/i915/intel_guc_slpc_fwif.h
new file mode 100644
index 0000000..9400af4
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc_slpc_fwif.h
@@ -0,0 +1,211 @@ 
+/*
+ * SPDX-License-Identifier: MIT
+ *
+ * Copyright © 2015-2018 Intel Corporation
+ */
+#ifndef _INTEL_GUC_SLPC_FWIF_H_
+#define _INTEL_GUC_SLPC_FWIF_H_
+
+#include <linux/types.h>
+
+enum slpc_status {
+	SLPC_STATUS_OK = 0,
+	SLPC_STATUS_ERROR = 1,
+	SLPC_STATUS_ILLEGAL_COMMAND = 2,
+	SLPC_STATUS_INVALID_ARGS = 3,
+	SLPC_STATUS_INVALID_PARAMS = 4,
+	SLPC_STATUS_INVALID_DATA = 5,
+	SLPC_STATUS_OUT_OF_RANGE = 6,
+	SLPC_STATUS_NOT_SUPPORTED = 7,
+	SLPC_STATUS_NOT_IMPLEMENTED = 8,
+	SLPC_STATUS_NO_DATA = 9,
+	SLPC_STATUS_EVENT_NOT_REGISTERED = 10,
+	SLPC_STATUS_REGISTER_LOCKED = 11,
+	SLPC_STATUS_TEMPORARILY_UNAVAILABLE = 12,
+	SLPC_STATUS_VALUE_ALREADY_SET = 13,
+	SLPC_STATUS_VALUE_ALREADY_UNSET = 14,
+	SLPC_STATUS_VALUE_NOT_CHANGED = 15,
+	SLPC_STATUS_MEMIO_ERROR = 16,
+	SLPC_STATUS_EVENT_QUEUED_REQ_DPC = 17,
+	SLPC_STATUS_EVENT_QUEUED_NOREQ_DPC = 18,
+	SLPC_STATUS_NO_EVENT_QUEUED = 19,
+	SLPC_STATUS_OUT_OF_SPACE = 20,
+	SLPC_STATUS_TIMEOUT = 21,
+	SLPC_STATUS_NO_LOCK = 22,
+	SLPC_STATUS_MAX
+};
+
+enum slpc_event_id {
+	SLPC_EVENT_RESET = 0,
+	SLPC_EVENT_SHUTDOWN = 1,
+	SLPC_EVENT_PLATFORM_INFO_CHANGE = 2,
+	SLPC_EVENT_DISPLAY_MODE_CHANGE = 3,
+	SLPC_EVENT_FLIP_COMPLETE = 4,
+	SLPC_EVENT_QUERY_TASK_STATE = 5,
+	SLPC_EVENT_PARAMETER_SET = 6,
+	SLPC_EVENT_PARAMETER_UNSET = 7,
+};
+
+enum slpc_param_id {
+	SLPC_PARAM_TASK_ENABLE_GTPERF = 0,
+	SLPC_PARAM_TASK_DISABLE_GTPERF = 1,
+	SLPC_PARAM_TASK_ENABLE_BALANCER = 2,
+	SLPC_PARAM_TASK_DISABLE_BALANCER = 3,
+	SLPC_PARAM_TASK_ENABLE_DCC = 4,
+	SLPC_PARAM_TASK_DISABLE_DCC = 5,
+	SLPC_PARAM_GLOBAL_MIN_GT_UNSLICE_FREQ_MHZ = 6,
+	SLPC_PARAM_GLOBAL_MAX_GT_UNSLICE_FREQ_MHZ = 7,
+	SLPC_PARAM_GLOBAL_MIN_GT_SLICE_FREQ_MHZ = 8,
+	SLPC_PARAM_GLOBAL_MAX_GT_SLICE_FREQ_MHZ = 9,
+	SLPC_PARAM_GTPERF_THRESHOLD_MAX_FPS = 10,
+	SLPC_PARAM_GLOBAL_DISABLE_GT_FREQ_MANAGEMENT = 11,
+	SLPC_PARAM_GTPERF_ENABLE_FRAMERATE_STALLING = 12,
+	SLPC_PARAM_GLOBAL_DISABLE_RC6_MODE_CHANGE = 13,
+	SLPC_PARAM_GLOBAL_OC_UNSLICE_FREQ_MHZ = 14,
+	SLPC_PARAM_GLOBAL_OC_SLICE_FREQ_MHZ = 15,
+	SLPC_PARAM_GLOBAL_ENABLE_IA_GT_BALANCING = 16,
+	SLPC_PARAM_GLOBAL_ENABLE_ADAPTIVE_BURST_TURBO = 17,
+	SLPC_PARAM_GLOBAL_ENABLE_EVAL_MODE = 18,
+	SLPC_PARAM_GLOBAL_ENABLE_BALANCER_IN_NON_GAMING_MODE = 19,
+	SLPC_MAX_PARAM,
+	SLPC_KMD_MAX_PARAM = 32,
+};
+
+enum slpc_global_state {
+	SLPC_GLOBAL_STATE_NOT_RUNNING = 0,
+	SLPC_GLOBAL_STATE_INITIALIZING = 1,
+	SLPC_GLOBAL_STATE_RESETTING = 2,
+	SLPC_GLOBAL_STATE_RUNNING = 3,
+	SLPC_GLOBAL_STATE_SHUTTING_DOWN = 4,
+	SLPC_GLOBAL_STATE_ERROR = 5
+};
+
+enum slpc_platform_sku {
+	SLPC_PLATFORM_SKU_UNDEFINED = 0,
+	SLPC_PLATFORM_SKU_ULX = 1,
+	SLPC_PLATFORM_SKU_ULT = 2,
+	SLPC_PLATFORM_SKU_T = 3,
+	SLPC_PLATFORM_SKU_MOBL = 4,
+	SLPC_PLATFORM_SKU_DT = 5,
+	SLPC_PLATFORM_SKU_UNKNOWN = 6,
+};
+
+enum slpc_power_source {
+	SLPC_POWER_SOURCE_UNDEFINED = 0,
+	SLPC_POWER_SOURCE_AC = 1,
+	SLPC_POWER_SOURCE_DC = 2,
+	SLPC_POWER_SOURCE_UNKNOWN = 3,
+};
+
+enum slpc_power_plan {
+	SLPC_POWER_PLAN_UNDEFINED = 0,
+	SLPC_POWER_PLAN_BATTERY_SAVER = 1,
+	SLPC_POWER_PLAN_BALANCED = 2,
+	SLPC_POWER_PLAN_PERFORMANCE = 3,
+	SLPC_POWER_PLAN_UNKNOWN = 4,
+};
+
+struct slpc_platform_info {
+	u8 sku;
+	u8 slice_count;
+	u8 reserved;
+	u8 power_plan_source;
+	u8 p0_freq;
+	u8 p1_freq;
+	u8 pe_freq;
+	u8 pn_freq;
+	u32 reserved1;
+	u32 reserved2;
+} __packed;
+
+struct slpc_task_state_data {
+	union {
+		u32 bitfield1;
+		struct {
+			u32 gtperf_task_active:1;
+			u32 gtperf_stall_possible:1;
+			u32 gtperf_gaming_mode:1;
+			u32 gtperf_target_fps:8;
+			u32 dcc_task_active:1;
+			u32 in_dcc:1;
+			u32 in_dct:1;
+			u32 freq_switch_active:1;
+			u32 ibc_enabled:1;
+			u32 ibc_active:1;
+			u32 pg1_enabled:1;
+			u32 pg1_active:1;
+			u32 reserved:13;
+		};
+	};
+	union {
+		u32 bitfield2;
+		struct {
+			u32 max_unslice_freq:8;
+			u32 min_unslice_freq:8;
+			u32 max_slice_freq:8;
+			u32 min_slice_freq:8;
+		};
+	};
+} __packed;
+
+#define SLPC_MAX_OVERRIDE_PARAMETERS 192
+#define SLPC_OVERRIDE_BITFIELD_SIZE \
+		((SLPC_MAX_OVERRIDE_PARAMETERS + 31) / 32)
+
+struct slpc_shared_data {
+	u32 reserved;
+	u32 shared_data_size;
+	u32 global_state;
+	struct slpc_platform_info platform_info;
+	struct slpc_task_state_data task_state_data;
+	u32 override_params_set_bits[SLPC_OVERRIDE_BITFIELD_SIZE];
+	u32 override_params_values[SLPC_MAX_OVERRIDE_PARAMETERS];
+} __packed;
+
+enum slpc_reset_flags {
+	SLPC_RESET_FLAG_TDR_OCCURRED = (1 << 0)
+};
+
+#define SLPC_EVENT_MAX_INPUT_ARGS  7
+#define SLPC_EVENT_MAX_OUTPUT_ARGS 1
+
+union slpc_event_input_header {
+	u32 value;
+	struct {
+		u32 num_args:8;
+		u32 event_id:8;
+	};
+};
+
+struct slpc_event_input {
+	u32 h2g_action_id;
+	union slpc_event_input_header header;
+	u32 args[SLPC_EVENT_MAX_INPUT_ARGS];
+} __packed;
+
+union slpc_event_output_header {
+	u32 value;
+	struct {
+		u32 num_args:8;
+		u32 event_id:8;
+		u32 status:16;
+	};
+};
+
+struct slpc_event_output {
+	u32 reserved;
+	union slpc_event_output_header header;
+	u32 args[SLPC_EVENT_MAX_OUTPUT_ARGS];
+} __packed;
+
+#define SLPC_EVENT(id, argc)		 ((u32)(id) << 8 | (argc))
+#define SLPC_POWER_PLAN_SOURCE(plan, source) ((plan) | ((source) << 6))
+#define SLPC_POWER_PLAN(plan_source)	 ((plan_source) & 0x3F)
+#define SLPC_POWER_SOURCE(plan_source)	 ((plan_source) >> 6)
+
+#define SLPC_PARAM_TASK_DEFAULT  0
+#define SLPC_PARAM_TASK_ENABLED  1
+#define SLPC_PARAM_TASK_DISABLED 2
+#define SLPC_PARAM_TASK_UNKNOWN  3
+
+#endif