[Intel-gfx] [PATCH v12 05/17] drm/i915/guc/slpc: Add SLPC communication interfaces
Michal Wajdeczko
michal.wajdeczko at intel.com
Fri Mar 30 13:37:29 UTC 2018
On Fri, 30 Mar 2018 10:31:50 +0200, Sagar Arun Kamble
<sagar.a.kamble at 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 at intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Sujaritha Sundaresan <sujaritha.sundaresan at intel.com>
> Cc: Jeff McGee <jeff.mcgee at 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
More information about the Intel-gfx
mailing list