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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, 30 Mar 2018 10:31:50 +0200, Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx> 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@xxxxxxxxx>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Cc: Radoslaw Szwichtenberg <radoslaw.szwichtenberg@xxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Sujaritha Sundaresan <sujaritha.sundaresan@xxxxxxxxx>
Cc: Jeff McGee <jeff.mcgee@xxxxxxxxx>
---
 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux