* [Intel-gfx] [PATCH 0/2] drm/i915/guc: Update to GuC v45
@ 2020-08-07 17:46 John.C.Harrison
2020-08-07 17:46 ` [Intel-gfx] [PATCH 1/2] drm/i915/guc: Update to GuC v45.0.0 John.C.Harrison
` (5 more replies)
0 siblings, 6 replies; 10+ messages in thread
From: John.C.Harrison @ 2020-08-07 17:46 UTC (permalink / raw)
To: Intel-GFX
From: John Harrison <John.C.Harrison@Intel.com>
Update to the latest GuC firmware and enable by default.
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Daniele Ceraolo Spurio (1):
drm/i915/uc: turn on GuC/HuC auto mode by default
John Harrison (1):
drm/i915/guc: Update to GuC v45.0.0
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 3 +-
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 125 +++++++----
drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h | 215 +++++++++++++++++++
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 116 ++++++++--
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 21 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 110 ++++------
drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 5 +
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 27 ++-
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 2 +
drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h | 6 +-
drivers/gpu/drm/i915/i915_params.h | 2 +-
11 files changed, 486 insertions(+), 146 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h
--
2.25.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Intel-gfx] [PATCH 1/2] drm/i915/guc: Update to GuC v45.0.0
2020-08-07 17:46 [Intel-gfx] [PATCH 0/2] drm/i915/guc: Update to GuC v45 John.C.Harrison
@ 2020-08-07 17:46 ` John.C.Harrison
2020-08-11 1:04 ` Daniele Ceraolo Spurio
2020-08-07 17:46 ` [Intel-gfx] [PATCH 2/2] drm/i915/uc: turn on GuC/HuC auto mode by default John.C.Harrison
` (4 subsequent siblings)
5 siblings, 1 reply; 10+ messages in thread
From: John.C.Harrison @ 2020-08-07 17:46 UTC (permalink / raw)
To: Intel-GFX
From: John Harrison <John.C.Harrison@Intel.com>
Update to the latest GuC firmware. This includes some significant
changes to the interface.
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Author: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Author: John Harrison <John.C.Harrison@Intel.com>
Author: Matthew Brost <matthew.brost@intel.com>
Author: Michal Wajdeczko <michal.wajdeczko@intel.com>
Author: Michel Thierry <michel.thierry@intel.com>
Author: Oscar Mateo <oscar.mateo@intel.com>
Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
---
drivers/gpu/drm/i915/gt/intel_engine_cs.c | 3 +-
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 125 +++++++----
drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h | 215 +++++++++++++++++++
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 116 ++++++++--
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 21 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 110 ++++------
drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 5 +
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 27 ++-
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 2 +
drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h | 6 +-
10 files changed, 485 insertions(+), 145 deletions(-)
create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
index ea4ba2afe9f9..9cd62d68abc6 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
@@ -305,8 +305,9 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
engine->i915 = i915;
engine->gt = gt;
engine->uncore = gt->uncore;
- engine->hw_id = engine->guc_id = info->hw_id;
engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
+ engine->hw_id = info->hw_id;
+ engine->guc_id = MAKE_GUC_ID(info->class, info->instance);
engine->class = info->class;
engine->instance = info->instance;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 861657897c0f..8d30e1d7d8a6 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -7,6 +7,7 @@
#include "gt/intel_gt_irq.h"
#include "gt/intel_gt_pm_irq.h"
#include "intel_guc.h"
+#include "intel_guc_abi.h"
#include "intel_guc_ads.h"
#include "intel_guc_submission.h"
#include "i915_drv.h"
@@ -213,23 +214,6 @@ static u32 guc_ctl_feature_flags(struct intel_guc *guc)
return flags;
}
-static u32 guc_ctl_ctxinfo_flags(struct intel_guc *guc)
-{
- u32 flags = 0;
-
- if (intel_guc_submission_is_used(guc)) {
- u32 ctxnum, base;
-
- base = intel_guc_ggtt_offset(guc, guc->stage_desc_pool);
- ctxnum = GUC_MAX_STAGE_DESCRIPTORS / 16;
-
- base >>= PAGE_SHIFT;
- flags |= (base << GUC_CTL_BASE_ADDR_SHIFT) |
- (ctxnum << GUC_CTL_CTXNUM_IN16_SHIFT);
- }
- return flags;
-}
-
static u32 guc_ctl_log_params_flags(struct intel_guc *guc)
{
u32 offset = intel_guc_ggtt_offset(guc, guc->log.vma) >> PAGE_SHIFT;
@@ -291,7 +275,6 @@ static void guc_init_params(struct intel_guc *guc)
BUILD_BUG_ON(sizeof(guc->params) != GUC_CTL_MAX_DWORDS * sizeof(u32));
- params[GUC_CTL_CTXINFO] = guc_ctl_ctxinfo_flags(guc);
params[GUC_CTL_LOG_PARAMS] = guc_ctl_log_params_flags(guc);
params[GUC_CTL_FEATURE] = guc_ctl_feature_flags(guc);
params[GUC_CTL_DEBUG] = guc_ctl_debug_flags(guc);
@@ -400,32 +383,65 @@ void intel_guc_fini(struct intel_guc *guc)
intel_uc_fw_fini(&guc->fw);
}
+static bool is_valid_mmio_action(u32 action)
+{
+ return action == GUC_ACTION_REGISTER_CTB ||
+ action == GUC_ACTION_DEREGISTER_CTB;
+}
+
+static int guc_status_to_errno(u32 status)
+{
+ switch (status) {
+ case GUC_STATUS_UNKNOWN_ACTION:
+ return -EOPNOTSUPP;
+ case GUC_STATUS_INVALID_PARAMS:
+ return -EINVAL;
+ case GUC_STATUS_INVALID_ADDR:
+ return -EFAULT;
+ case GUC_STATUS_CTX_NOT_REGISTERED:
+ return -ESRCH;
+ case GUC_STATUS_CTB_FULL:
+ return -ENOSPC;
+ case GUC_STATUS_UNAUTHORIZED_REQUEST:
+ return -EACCES;
+ case GUC_STATUS_GENERIC_FAIL:
+ default:
+ return -ENXIO;
+ }
+}
+
/*
* This function implements the MMIO based host to GuC interface.
*/
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *request, u32 len,
u32 *response_buf, u32 response_buf_size)
{
+ struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
struct intel_uncore *uncore = guc_to_gt(guc)->uncore;
- u32 status;
+ u32 action = FIELD_GET(GUC_MMIO_REQUEST_ACTION, *request);
+ u32 subcode = FIELD_GET(GUC_MMIO_REQUEST_SUBCODE, *request);
+ u32 magic = GUC_MMIO_MSG_MAGIC_DEFAULT;
+ u32 header;
int i;
int ret;
GEM_BUG_ON(!len);
GEM_BUG_ON(len > guc->send_regs.count);
- /* We expect only action code */
- GEM_BUG_ON(*action & ~INTEL_GUC_MSG_CODE_MASK);
+ /* We expect to use MMIO only for special actions */
+ GEM_BUG_ON(!is_valid_mmio_action(action));
- /* If CT is available, we expect to use MMIO only during init/fini */
- GEM_BUG_ON(*action != INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER &&
- *action != INTEL_GUC_ACTION_DEREGISTER_COMMAND_TRANSPORT_BUFFER);
+ header = FIELD_PREP(GUC_MMIO_MSG_TYPE, GUC_MMIO_MSG_TYPE_REQUEST) |
+ FIELD_PREP(GUC_MMIO_MSG_MAGIC, magic) |
+ FIELD_PREP(GUC_MMIO_REQUEST_SUBCODE, subcode) |
+ FIELD_PREP(GUC_MMIO_REQUEST_ACTION, action);
mutex_lock(&guc->send_mutex);
intel_uncore_forcewake_get(uncore, guc->send_regs.fw_domains);
- for (i = 0; i < len; i++)
- intel_uncore_write(uncore, guc_send_reg(guc, i), action[i]);
+ intel_uncore_write(uncore, guc_send_reg(guc, 0), header);
+ for (i = 1; i < len; i++)
+ intel_uncore_write(uncore, guc_send_reg(guc, i), request[i]);
intel_uncore_posting_read(uncore, guc_send_reg(guc, i - 1));
@@ -437,17 +453,45 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
*/
ret = __intel_wait_for_register_fw(uncore,
guc_send_reg(guc, 0),
- INTEL_GUC_MSG_TYPE_MASK,
- INTEL_GUC_MSG_TYPE_RESPONSE <<
- INTEL_GUC_MSG_TYPE_SHIFT,
- 10, 10, &status);
- /* If GuC explicitly returned an error, convert it to -EIO */
- if (!ret && !INTEL_GUC_MSG_IS_RESPONSE_SUCCESS(status))
- ret = -EIO;
+ GUC_MMIO_MSG_ORIGIN,
+ FIELD_PREP(GUC_MMIO_MSG_ORIGIN,
+ GUC_MMIO_MSG_ORIGIN_GUC),
+ 10, 10, &header);
+ if (unlikely(ret))
+ goto out;
- if (ret) {
- DRM_ERROR("MMIO: GuC action %#x failed with error %d %#x\n",
- action[0], ret, status);
+ if (unlikely(FIELD_GET(GUC_MMIO_MSG_MAGIC, header) != magic)) {
+ ret = -ESTALE;
+ goto out;
+ }
+
+ if (FIELD_GET(GUC_MMIO_MSG_TYPE, header) == GUC_MMIO_MSG_TYPE_BUSY) {
+#define done ({ header = intel_uncore_read(uncore, guc_send_reg(guc, 0)); \
+ FIELD_GET(GUC_MMIO_MSG_TYPE, header) != GUC_MMIO_MSG_TYPE_BUSY; })
+ ret = wait_for(done, 1000);
+ if (unlikely(ret))
+ goto out;
+ if (unlikely(FIELD_GET(GUC_MMIO_MSG_ORIGIN, header) !=
+ GUC_MMIO_MSG_ORIGIN_GUC)) {
+ ret = -EPROTO;
+ goto out;
+ }
+ if (unlikely(FIELD_GET(GUC_MMIO_MSG_MAGIC, header) != magic)) {
+ ret = -ESTALE;
+ goto out;
+ }
+#undef done
+ }
+
+ if (FIELD_GET(GUC_MMIO_MSG_TYPE, header) == GUC_MMIO_MSG_TYPE_ERROR) {
+ u32 status = FIELD_GET(GUC_MMIO_ERROR_STATUS, header);
+
+ ret = guc_status_to_errno(status);
+ goto out;
+ }
+
+ if (FIELD_GET(GUC_MMIO_MSG_TYPE, header) != GUC_MMIO_MSG_TYPE_RESPONSE) {
+ ret = -EPROTO;
goto out;
}
@@ -460,12 +504,17 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
}
/* Use data from the GuC response as our return value */
- ret = INTEL_GUC_MSG_TO_DATA(status);
+ ret = FIELD_GET(GUC_MMIO_RESPONSE_DATA0, header);
out:
intel_uncore_forcewake_put(uncore, guc->send_regs.fw_domains);
mutex_unlock(&guc->send_mutex);
+ if (unlikely(ret < 0))
+ drm_err(&i915->drm,
+ "GuC MMIO action %#06x failed with error %d %#x\n",
+ *request, ret, header);
+
return ret;
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h
new file mode 100644
index 000000000000..95043788e0ad
--- /dev/null
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h
@@ -0,0 +1,215 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2020 Intel Corporation
+ */
+
+#ifndef __INTEL_GUC_ABI_H__
+#define __INTEL_GUC_ABI_H__
+
+/**
+ * DOC: GuC MMIO based communication
+ *
+ * The MMIO based communication between Host and GuC relies on special
+ * hardware registers which format could be defined by the software
+ * (so called scratch registers).
+ *
+ * Each MMIO based message, both Host to GuC (H2G) and GuC to Host (G2H)
+ * messages, which maximum length depends on number of available scratch
+ * registers, is directly written into those scratch registers.
+ *
+ * For Gen9+, there are 16 software scratch registers 0xC180-0xC1B8,
+ * but no H2G command takes more than 8 parameters and the GuC firmware
+ * itself uses an 8-element array to store the H2G message.
+ *
+ * For Gen11+, there are additional 4 registers 0x190240-0x19024C, which
+ * are, regardless on lower count, preffered over legacy ones.
+ *
+ * The MMIO based communication is mainly used during driver initialization
+ * phase to setup the CTB based communication that will be used afterwards.
+ *
+ * Details of the messages formats depend on GuC firmware version being used
+ * by the host driver. Documented here messages are for GuC 45.0 and newer.
+ */
+
+#define GUC_MAX_MMIO_MSG_LEN 8
+
+/**
+ * DOC: GuC MMIO message format
+ *
+ * Bits of the first scratch register are treated as a message header,
+ * and other registers values are used to hold message payload (data)::
+ *
+ * +=======================================================+
+ * | SCRATCH | |
+ * +-----------+-------------------------------------------+
+ * | 0 | 31:28 | message ORIGIN/TYPE |
+ * | | 27:24 | message MAGIC |
+ * | | 23:0 | message DATA0 (depends on TYPE) |
+ * +---+-------+-------------------------------------------+
+ * | 1 | 31:0 | message DATA1 (depends on TYPE) |
+ * |...| | message DATA2 (depends on TYPE) |
+ * | n | | message DATA3 (depends on TYPE) |
+ * +=======================================================+
+ *
+ * Where:
+ * - **TYPE** is a 4b message type identifier.
+ * - **MAGIC** is a 4b message sequence number.
+ * The same sequence number must be included in all related messages.
+ * This field is used for tracking and debug purposes.
+ * - **DATA0..3** bits represents message payload.
+ * Definitions of these bits depends on message **TYPE**.
+ *
+ * The MSB of the header and **TYPE** indicates origin of the message:
+ * - 0 - message from the Host
+ * - 1 - message from the GuC
+ *
+ * Currently supported message types are:
+ * - 0x0 - **REQUEST** - represents Host request message to the GuC
+ * - 0xF - **SUCCESS RESPONSE** - GuC reply for the earlier request
+ * - 0xE - **ERROR RESPONSE** - GuC failure reply for the earlier request
+ * - 0xB - **BUSY** - GuC will be processing request for the longer time
+ */
+
+#define GUC_MMIO_MSG_ORIGIN (0x1 << 31)
+#define GUC_MMIO_MSG_ORIGIN_HOST 0u
+#define GUC_MMIO_MSG_ORIGIN_GUC 1u
+
+#define GUC_MMIO_MSG_TYPE (0xF << 28)
+#define GUC_MMIO_MSG_TYPE_REQUEST 0x0
+#define GUC_MMIO_MSG_TYPE_RESPONSE 0xF
+#define GUC_MMIO_MSG_TYPE_ERROR 0xE
+#define GUC_MMIO_MSG_TYPE_BUSY 0xB
+
+#define GUC_MMIO_MSG_MAGIC (0xF << 24)
+#define GUC_MMIO_MSG_MAGIC_DEFAULT 0u
+
+/**
+ * DOC: GuC MMIO H2G REQUEST
+ *
+ * The MMIO H2G REQUEST message is used by the Host to request some action
+ * to be performed by the GuC::
+ *
+ * +=======================================================+
+ * | SCRATCH | |
+ * +=======================================================+
+ * | 0 | 31:28 | message TYPE = 0x0 = REQUEST |
+ * | | 27:24 | message MAGIC |
+ * | | 23:16 | request SUBCODE (depends on ACTION) |
+ * | | 15:0 | request ACTION code |
+ * +-------------------------------------------------------+
+ * | 1 | 31:0 | request DATA1 (depends on ACTION/SUBCODE) |
+ * |...| | request DATA2 (depends on ACTION/SUBCODE) |
+ * | n | 31:0 | request DATA3 (depends on ACTION/SUBCODE) |
+ * +=======================================================+
+ *
+ * Where:
+ * - **TYPE** must be set to value 0x0.
+ * - **MAGIC** message sequence number is generated by the host.
+ * - **ACTION** represents 16b request action code that defines both
+ * purpose of the request and format of the passed payload data.
+ * List of supported codes depends on GuC version and plaform.
+ * Action code can't be zero.
+ * - **SUBCODE** is optional 8b data related to the **ACTION**.
+ * MBZ if not explicitly defined by given **ACTION**.
+ * - **DATA1..3** are optional 32b payload dwords related to **ACTION**.
+ * Format of each dword is defined by specific **ACTION**.
+ */
+
+#define GUC_MMIO_REQUEST_SUBCODE (0xFF << 16)
+#define GUC_MMIO_REQUEST_ACTION (0xFFFF << 0)
+#define GUC_ACTION_INVALID 0x0000
+#define GUC_ACTION_REGISTER_CTB 0x4505
+#define GUC_ACTION_DEREGISTER_CTB 0x4506
+
+/**
+ * DOC: GuC MMIO G2H SUCCESS RESPONSE
+ *
+ * The MMIO G2H SUCCESS RESPONSE message is used by the GuC to reply to
+ * the earlier H2G request message. This message is used if no errors
+ * were encountered by the GuC during processing of the request::
+ *
+ * +=======================================================+
+ * | SCRATCH | |
+ * +=======================================================+
+ * | 0 | 31:28 | message TYPE = 0xF = SUCCESS RESPONSE |
+ * | | 27:24 | message MAGIC |
+ * | | 23:0 | response DATA0 (depends on ACTION/SUBCODE)|
+ * +-------------------------------------------------------+
+ * | 1 | 31:0 | response DATA1 (depends on ACTION/SUBCODE)|
+ * |...| | response DATA2 (depends on ACTION/SUBCODE)|
+ * | n | 31:0 | response DATA3 (depends on ACTION/SUBCODE)|
+ * +=======================================================+
+ *
+ * Where:
+ * - **TYPE** must be set to value 0xF.
+ * - **MAGIC** must match value used by the host in **REQUEST** message.
+ * - **DATA** is optional payload data related to **ACTION**.
+ * Format is defined by each **ACTION** separately.
+ */
+
+#define GUC_MMIO_RESPONSE_DATA0 (0xFFFFFF << 0)
+
+/**
+ * DOC: GuC MMIO G2H ERROR RESPONSE
+ *
+ * The MMIO G2H ERROR RESPONSE message is used by the GuC to reply to
+ * the earlier H2G request message. This message is used if some errors
+ * were encountered by the GuC during processing of the request::
+ *
+ * +=======================================================+
+ * | SCRATCH | |
+ * +=======================================================+
+ * | 0 | 31:28 | message TYPE = 0xE = ERROR RESPONSE |
+ * | | 27:24 | message MAGIC |
+ * | | 23:16 | reserved MBZ |
+ * | | 15:0 | response STATUS failure code |
+ * +-------------------------------------------------------+
+ * | 1 | 31:0 | reserved MBZ |
+ * |...| | reserved MBZ |
+ * | n | 31:0 | reserved MBZ |
+ * +=======================================================+
+ *
+ * Where:
+ * - **TYPE** must be set to value 0xE.
+ * - **MAGIC** must match value used by the host in **REQUEST** message.
+ * - **STATUS** represents non-zero failure code.
+ */
+
+#define GUC_MMIO_ERROR_STATUS (0xFFFF << 0)
+#define GUC_STATUS_UNKNOWN_ACTION 0x30
+#define GUC_STATUS_INVALID_PARAMS 0x60
+#define GUC_STATUS_INVALID_ADDR 0x80
+#define GUC_STATUS_CTX_NOT_REGISTERED 0x100
+#define GUC_STATUS_NO_LOCAL_MEMORY 0x200
+#define GUC_STATUS_NO_VIRTUALIZATION 0x300
+#define GUC_STATUS_CTB_FULL 0x301
+#define GUC_STATUS_UNAUTHORIZED_REQUEST 0x302
+#define GUC_STATUS_GENERIC_FAIL 0xF000
+
+/**
+ * DOC: MMIO G2H BUSY RESPONSE
+ *
+ * The MMIO G2H BUSY RESPONSE message is used by the GuC to reply to
+ * the earlier H2G request message. This message is used if processing
+ * of the request will take longer time and final SUCCESS/ERROR response
+ * message will be delivered later::
+ *
+ * +=======================================================+
+ * | SCRATCH | |
+ * +=======================================================+
+ * | 0 | 31:28 | message TYPE = 0xB = BUSY RESPONSE |
+ * | | 27:24 | message MAGIC |
+ * | | 23:0 | reserved MBZ |
+ * +-------------------------------------------------------+
+ * | 1 | 31:0 | reserved MBZ |
+ * |...| | reserved MBZ |
+ * | n | 31:0 | reserved MBZ |
+ * +=======================================================+
+ *
+ * Where:
+ * - **TYPE** must be set to value 0xB.
+ * - **MAGIC** must match value used by the host in **REQUEST** message.
+ * - all other bits are reserved as MBZ.
+ */
+
+#endif /* __INTEL_GUC_ABI_H__ */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
index d44061033f23..e66cbf1be320 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
@@ -10,11 +10,52 @@
/*
* The Additional Data Struct (ADS) has pointers for different buffers used by
- * the GuC. One single gem object contains the ADS struct itself (guc_ads), the
- * scheduling policies (guc_policies), a structure describing a collection of
- * register sets (guc_mmio_reg_state) and some extra pages for the GuC to save
- * its internal state for sleep.
+ * the GuC. One single gem object contains the ADS struct itself (guc_ads) and
+ * all the extra buffers indirectly linked via the ADS struct's entires.
+ *
+ * Layout of the ADS blob allocated for the GuC:
+ *
+ * +---------------------------------------+ <== base
+ * | guc_ads |
+ * +---------------------------------------+
+ * | guc_policies |
+ * +---------------------------------------+
+ * | guc_gt_system_info |
+ * +---------------------------------------+
+ * | guc_clients_info |
+ * +---------------------------------------+
+ * | guc_ct_pool_entry[size] |
+ * +---------------------------------------+
+ * | padding |
+ * +---------------------------------------+ <== 4K aligned
+ * | private data |
+ * +---------------------------------------+
+ * | padding |
+ * +---------------------------------------+ <== 4K aligned
*/
+struct __guc_ads_blob {
+ struct guc_ads ads;
+ struct guc_policies policies;
+ struct guc_gt_system_info system_info;
+ struct guc_clients_info clients_info;
+ struct guc_ct_pool_entry ct_pool[GUC_CT_POOL_SIZE];
+} __packed;
+
+static u32 guc_ads_private_data_size(struct intel_guc *guc)
+{
+ return PAGE_ALIGN(guc->fw.private_data_size);
+}
+
+static u32 guc_ads_private_data_offset(struct intel_guc *guc)
+{
+ return PAGE_ALIGN(sizeof(struct __guc_ads_blob));
+}
+
+static u32 guc_ads_blob_size(struct intel_guc *guc)
+{
+ return guc_ads_private_data_offset(guc) +
+ guc_ads_private_data_size(guc);
+}
static void guc_policy_init(struct guc_policy *policy)
{
@@ -48,23 +89,33 @@ static void guc_ct_pool_entries_init(struct guc_ct_pool_entry *pool, u32 num)
memset(pool, 0, num * sizeof(*pool));
}
+static void guc_mapping_table_init(struct intel_gt *gt,
+ struct guc_gt_system_info *system_info)
+{
+ unsigned int i, j;
+ struct intel_engine_cs *engine;
+ enum intel_engine_id id;
+
+ /* Table must be set to invalid values for entries not used */
+ for (i = 0; i < GUC_MAX_ENGINE_CLASSES; ++i)
+ for (j = 0; j < GUC_MAX_INSTANCES_PER_CLASS; ++j)
+ system_info->mapping_table[i][j] =
+ GUC_MAX_INSTANCES_PER_CLASS;
+
+ for_each_engine(engine, gt, id) {
+ u8 guc_class = engine->class;
+
+ system_info->mapping_table[guc_class][engine->instance]
+ = engine->instance;
+ }
+}
+
/*
* The first 80 dwords of the register state context, containing the
* execlists and ppgtt registers.
*/
#define LR_HW_CONTEXT_SIZE (80 * sizeof(u32))
-/* The ads obj includes the struct itself and buffers passed to GuC */
-struct __guc_ads_blob {
- struct guc_ads ads;
- struct guc_policies policies;
- struct guc_mmio_reg_state reg_state;
- struct guc_gt_system_info system_info;
- struct guc_clients_info clients_info;
- struct guc_ct_pool_entry ct_pool[GUC_CT_POOL_SIZE];
- u8 reg_state_buffer[GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE];
-} __packed;
-
static void __guc_ads_init(struct intel_guc *guc)
{
struct intel_gt *gt = guc_to_gt(guc);
@@ -107,6 +158,16 @@ static void __guc_ads_init(struct intel_guc *guc)
blob->system_info.vebox_enable_mask = VEBOX_MASK(gt);
blob->system_info.vdbox_sfc_support_mask = gt->info.vdbox_sfc_access;
+ if (INTEL_GEN(gt->i915) >= 12 && !IS_DGFX(gt->i915)) {
+ u32 distdbreg = intel_uncore_read(gt->uncore,
+ GEN12_DIST_DBS_POPULATED);
+ blob->system_info.num_of_doorbells_per_sqidi =
+ ((distdbreg >> GEN12_DOORBELLS_PER_SQIDI_SHIFT) &
+ GEN12_DOORBELLS_PER_SQIDI) + 1;
+ }
+
+ guc_mapping_table_init(guc_to_gt(guc), &blob->system_info);
+
base = intel_guc_ggtt_offset(guc, guc->ads_vma);
/* Clients info */
@@ -118,11 +179,12 @@ static void __guc_ads_init(struct intel_guc *guc)
/* ADS */
blob->ads.scheduler_policies = base + ptr_offset(blob, policies);
- blob->ads.reg_state_buffer = base + ptr_offset(blob, reg_state_buffer);
- blob->ads.reg_state_addr = base + ptr_offset(blob, reg_state);
blob->ads.gt_system_info = base + ptr_offset(blob, system_info);
blob->ads.clients_info = base + ptr_offset(blob, clients_info);
+ /* Private Data */
+ blob->ads.private_data = base + guc_ads_private_data_offset(guc);
+
i915_gem_object_flush_map(guc->ads_vma->obj);
}
@@ -135,14 +197,15 @@ static void __guc_ads_init(struct intel_guc *guc)
*/
int intel_guc_ads_create(struct intel_guc *guc)
{
- const u32 size = PAGE_ALIGN(sizeof(struct __guc_ads_blob));
+ u32 size;
int ret;
GEM_BUG_ON(guc->ads_vma);
+ size = guc_ads_blob_size(guc);
+
ret = intel_guc_allocate_and_map_vma(guc, size, &guc->ads_vma,
(void **)&guc->ads_blob);
-
if (ret)
return ret;
@@ -156,6 +219,18 @@ void intel_guc_ads_destroy(struct intel_guc *guc)
i915_vma_unpin_and_release(&guc->ads_vma, I915_VMA_RELEASE_MAP);
}
+void guc_ads_private_data_reset(struct intel_guc *guc)
+{
+ u32 size;
+
+ size = guc_ads_private_data_size(guc);
+ if (!size)
+ return;
+
+ memset((void *)guc->ads_blob + guc_ads_private_data_offset(guc), 0,
+ size);
+}
+
/**
* intel_guc_ads_reset() - prepares GuC Additional Data Struct for reuse
* @guc: intel_guc struct
@@ -168,5 +243,8 @@ void intel_guc_ads_reset(struct intel_guc *guc)
{
if (!guc->ads_vma)
return;
+
__guc_ads_init(guc);
+
+ guc_ads_private_data_reset(guc);
}
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
index d4a87f4c9421..212de0a939bf 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
@@ -74,8 +74,9 @@ static inline bool guc_ready(struct intel_uncore *uncore, u32 *status)
((val & GS_MIA_CORE_STATE) && (uk_val == GS_UKERNEL_LAPIC_DONE));
}
-static int guc_wait_ucode(struct intel_uncore *uncore)
+static int guc_wait_ucode(struct intel_gt *gt)
{
+ struct intel_uncore *uncore = gt->uncore;
u32 status;
int ret;
@@ -93,12 +94,24 @@ static int guc_wait_ucode(struct intel_uncore *uncore)
if ((status & GS_BOOTROM_MASK) == GS_BOOTROM_RSA_FAILED) {
DRM_ERROR("GuC firmware signature verification failed\n");
ret = -ENOEXEC;
- }
-
+ } else
if ((status & GS_UKERNEL_MASK) == GS_UKERNEL_EXCEPTION) {
DRM_ERROR("GuC firmware exception. EIP: %#x\n",
intel_uncore_read(uncore, SOFT_SCRATCH(13)));
ret = -ENXIO;
+ } else
+ if (ret) {
+ i915_probe_error(gt->i915, "GuC load failed: status = 0x%08X\n",
+ status);
+ i915_probe_error(gt->i915, "GuC status: Reset = %d, "
+ "BootROM = 0x%02X, UKernel = 0x%02X, "
+ "MIA = 0x%02X, Auth = 0x%02X\n",
+ (status >> GS_RESET_SHIFT) & 1,
+ (status & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT,
+ (status & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT,
+ (status & GS_MIA_MASK) >> GS_MIA_SHIFT,
+ (status & GS_AUTH_STATUS_MASK) >>
+ GS_AUTH_STATUS_SHIFT);
}
return ret;
@@ -139,7 +152,7 @@ int intel_guc_fw_upload(struct intel_guc *guc)
if (ret)
goto out;
- ret = guc_wait_ucode(uncore);
+ ret = guc_wait_ucode(gt);
if (ret)
goto out;
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
index a6b733c146c9..77e49d53bb5c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
@@ -27,7 +27,7 @@
#define GUC_MAX_ENGINES_NUM (GUC_VIDEO_ENGINE2 + 1)
#define GUC_MAX_ENGINE_CLASSES 5
-#define GUC_MAX_INSTANCES_PER_CLASS 16
+#define GUC_MAX_INSTANCES_PER_CLASS 32
#define GUC_DOORBELL_INVALID 256
@@ -62,12 +62,7 @@
#define GUC_STAGE_DESC_ATTR_PCH BIT(6)
#define GUC_STAGE_DESC_ATTR_TERMINATED BIT(7)
-/* New GuC control data */
-#define GUC_CTL_CTXINFO 0
-#define GUC_CTL_CTXNUM_IN16_SHIFT 0
-#define GUC_CTL_BASE_ADDR_SHIFT 12
-
-#define GUC_CTL_LOG_PARAMS 1
+#define GUC_CTL_LOG_PARAMS 0
#define GUC_LOG_VALID (1 << 0)
#define GUC_LOG_NOTIFY_ON_HALF_FULL (1 << 1)
#define GUC_LOG_ALLOC_IN_MEGABYTE (1 << 3)
@@ -79,11 +74,11 @@
#define GUC_LOG_ISR_MASK (0x7 << GUC_LOG_ISR_SHIFT)
#define GUC_LOG_BUF_ADDR_SHIFT 12
-#define GUC_CTL_WA 2
-#define GUC_CTL_FEATURE 3
+#define GUC_CTL_WA 1
+#define GUC_CTL_FEATURE 2
#define GUC_CTL_DISABLE_SCHEDULER (1 << 14)
-#define GUC_CTL_DEBUG 4
+#define GUC_CTL_DEBUG 3
#define GUC_LOG_VERBOSITY_SHIFT 0
#define GUC_LOG_VERBOSITY_LOW (0 << GUC_LOG_VERBOSITY_SHIFT)
#define GUC_LOG_VERBOSITY_MED (1 << GUC_LOG_VERBOSITY_SHIFT)
@@ -97,12 +92,31 @@
#define GUC_LOG_DISABLED (1 << 6)
#define GUC_PROFILE_ENABLED (1 << 7)
-#define GUC_CTL_ADS 5
+#define GUC_CTL_ADS 4
#define GUC_ADS_ADDR_SHIFT 1
#define GUC_ADS_ADDR_MASK (0xFFFFF << GUC_ADS_ADDR_SHIFT)
#define GUC_CTL_MAX_DWORDS (SOFT_SCRATCH_COUNT - 2) /* [1..14] */
+/*
+ * The class goes in bits [0..2] of the GuC ID, the instance in bits [3..6].
+ * Bit 7 can be used for operations that apply to all engine classes&instances.
+ */
+#define GUC_ENGINE_CLASS_SHIFT 0
+#define GUC_ENGINE_CLASS_MASK (0x7 << GUC_ENGINE_CLASS_SHIFT)
+#define GUC_ENGINE_INSTANCE_SHIFT 3
+#define GUC_ENGINE_INSTANCE_MASK (0xf << GUC_ENGINE_INSTANCE_SHIFT)
+#define GUC_ENGINE_ALL_INSTANCES (1 << 7)
+
+#define MAKE_GUC_ID(class, instance) \
+ (((class) << GUC_ENGINE_CLASS_SHIFT) | \
+ ((instance) << GUC_ENGINE_INSTANCE_SHIFT))
+
+#define GUC_ID_TO_ENGINE_CLASS(guc_id) \
+ (((guc_id) & GUC_ENGINE_CLASS_MASK) >> GUC_ENGINE_CLASS_SHIFT)
+#define GUC_ID_TO_ENGINE_INSTANCE(guc_id) \
+ (((guc_id) & GUC_ENGINE_INSTANCE_MASK) >> GUC_ENGINE_INSTANCE_SHIFT)
+
/* Work item for submitting workloads into work queue of GuC. */
struct guc_wq_item {
u32 header;
@@ -338,7 +352,6 @@ struct guc_policies {
/* GuC MMIO reg state struct */
-#define GUC_REGSET_MAX_REGISTERS 64
#define GUC_S3_SAVE_SPACE_PAGES 10
struct guc_mmio_reg {
@@ -348,16 +361,17 @@ struct guc_mmio_reg {
#define GUC_REGSET_MASKED (1 << 0)
} __packed;
-struct guc_mmio_regset {
- struct guc_mmio_reg registers[GUC_REGSET_MAX_REGISTERS];
- u32 values_valid;
- u32 number_of_registers;
-} __packed;
-
/* GuC register sets */
-struct guc_mmio_reg_state {
- struct guc_mmio_regset engine_reg[GUC_MAX_ENGINE_CLASSES][GUC_MAX_INSTANCES_PER_CLASS];
- u32 reserved[98];
+struct guc_mmio_reg_set {
+ u32 address;
+ union {
+ struct {
+ u32 count:16;
+ u32 reserved1:12;
+ u32 reserved2:4;
+ };
+ u32 count_u32;
+ };
} __packed;
/* HW info */
@@ -369,7 +383,9 @@ struct guc_gt_system_info {
u32 vdbox_enable_mask;
u32 vdbox_sfc_support_mask;
u32 vebox_enable_mask;
- u32 reserved[9];
+ u32 num_of_doorbells_per_sqidi;
+ u8 mapping_table[GUC_MAX_ENGINE_CLASSES][GUC_MAX_INSTANCES_PER_CLASS];
+ u32 reserved2[8];
} __packed;
/* Clients info */
@@ -390,15 +406,16 @@ struct guc_clients_info {
/* GuC Additional Data Struct */
struct guc_ads {
- u32 reg_state_addr;
- u32 reg_state_buffer;
+ struct guc_mmio_reg_set reg_state_list[GUC_MAX_ENGINE_CLASSES][GUC_MAX_INSTANCES_PER_CLASS];
+ u32 reserved0;
u32 scheduler_policies;
u32 gt_system_info;
u32 clients_info;
u32 control_data;
u32 golden_context_lrca[GUC_MAX_ENGINE_CLASSES];
u32 eng_state_size[GUC_MAX_ENGINE_CLASSES];
- u32 reserved[16];
+ u32 private_data;
+ u32 reserved[15];
} __packed;
/* GuC logging structures */
@@ -474,49 +491,6 @@ struct guc_shared_ctx_data {
struct guc_ctx_report preempt_ctx_report[GUC_MAX_ENGINES_NUM];
} __packed;
-/**
- * DOC: MMIO based communication
- *
- * The MMIO based communication between Host and GuC uses software scratch
- * registers, where first register holds data treated as message header,
- * and other registers are used to hold message payload.
- *
- * For Gen9+, GuC uses software scratch registers 0xC180-0xC1B8,
- * but no H2G command takes more than 8 parameters and the GuC FW
- * itself uses an 8-element array to store the H2G message.
- *
- * +-----------+---------+---------+---------+
- * | MMIO[0] | MMIO[1] | ... | MMIO[n] |
- * +-----------+---------+---------+---------+
- * | header | optional payload |
- * +======+====+=========+=========+=========+
- * | 31:28|type| | | |
- * +------+----+ | | |
- * | 27:16|data| | | |
- * +------+----+ | | |
- * | 15:0|code| | | |
- * +------+----+---------+---------+---------+
- *
- * The message header consists of:
- *
- * - **type**, indicates message type
- * - **code**, indicates message code, is specific for **type**
- * - **data**, indicates message data, optional, depends on **code**
- *
- * The following message **types** are supported:
- *
- * - **REQUEST**, indicates Host-to-GuC request, requested GuC action code
- * must be priovided in **code** field. Optional action specific parameters
- * can be provided in remaining payload registers or **data** field.
- *
- * - **RESPONSE**, indicates GuC-to-Host response from earlier GuC request,
- * action response status will be provided in **code** field. Optional
- * response data can be returned in remaining payload registers or **data**
- * field.
- */
-
-#define GUC_MAX_MMIO_MSG_LEN 8
-
#define INTEL_GUC_MSG_TYPE_SHIFT 28
#define INTEL_GUC_MSG_TYPE_MASK (0xF << INTEL_GUC_MSG_TYPE_SHIFT)
#define INTEL_GUC_MSG_DATA_SHIFT 16
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
index 1949346e714e..b37fc2ffaef2 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
@@ -118,6 +118,11 @@ struct guc_doorbell_info {
#define GEN8_DRB_VALID (1<<0)
#define GEN8_DRBREGU(x) _MMIO(0x1000 + (x) * 8 + 4)
+#define GEN12_DIST_DBS_POPULATED _MMIO(0xd08)
+#define GEN12_DOORBELLS_PER_SQIDI_SHIFT 16
+#define GEN12_DOORBELLS_PER_SQIDI (0xff)
+#define GEN12_SQIDIS_DOORBELL_EXIST (0xffff)
+
#define DE_GUCRMR _MMIO(0x44054)
#define GUC_BCS_RCS_IER _MMIO(0xC550)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 59b27aba15c6..f7cb0b1f1ba5 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -44,23 +44,19 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
* List of required GuC and HuC binaries per-platform.
* Must be ordered based on platform + revid, from newer to older.
*
- * TGL 35.2 is interface-compatible with 33.0 for previous Gens. The deltas
- * between 33.0 and 35.2 are only related to new additions to support new Gen12
- * features.
- *
* Note that RKL uses the same firmware as TGL.
*/
#define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \
- fw_def(ROCKETLAKE, 0, guc_def(tgl, 35, 2, 0), huc_def(tgl, 7, 0, 12)) \
- fw_def(TIGERLAKE, 0, guc_def(tgl, 35, 2, 0), huc_def(tgl, 7, 0, 12)) \
- fw_def(ELKHARTLAKE, 0, guc_def(ehl, 33, 0, 4), huc_def(ehl, 9, 0, 0)) \
- fw_def(ICELAKE, 0, guc_def(icl, 33, 0, 0), huc_def(icl, 9, 0, 0)) \
- fw_def(COMETLAKE, 5, guc_def(cml, 33, 0, 0), huc_def(cml, 4, 0, 0)) \
- fw_def(COFFEELAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 4, 0, 0)) \
- fw_def(GEMINILAKE, 0, guc_def(glk, 33, 0, 0), huc_def(glk, 4, 0, 0)) \
- fw_def(KABYLAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 4, 0, 0)) \
- fw_def(BROXTON, 0, guc_def(bxt, 33, 0, 0), huc_def(bxt, 2, 0, 0)) \
- fw_def(SKYLAKE, 0, guc_def(skl, 33, 0, 0), huc_def(skl, 2, 0, 0))
+ fw_def(ROCKETLAKE, 0, guc_def(tgl, 45, 0, 0), huc_def(tgl, 7, 0, 12)) \
+ fw_def(TIGERLAKE, 0, guc_def(tgl, 45, 0, 0), huc_def(tgl, 7, 0, 12)) \
+ fw_def(ELKHARTLAKE, 0, guc_def(ehl, 45, 0, 0), huc_def(ehl, 9, 0, 0)) \
+ fw_def(ICELAKE, 0, guc_def(icl, 45, 0, 0), huc_def(icl, 9, 0, 0)) \
+ fw_def(COMETLAKE, 5, guc_def(cml, 45, 0, 0), huc_def(cml, 4, 0, 0)) \
+ fw_def(COFFEELAKE, 0, guc_def(kbl, 45, 0, 0), huc_def(kbl, 4, 0, 0)) \
+ fw_def(GEMINILAKE, 0, guc_def(glk, 45, 0, 0), huc_def(glk, 4, 0, 0)) \
+ fw_def(KABYLAKE, 0, guc_def(kbl, 45, 0, 0), huc_def(kbl, 4, 0, 0)) \
+ fw_def(BROXTON, 0, guc_def(bxt, 45, 0, 0), huc_def(bxt, 2, 0, 0)) \
+ fw_def(SKYLAKE, 0, guc_def(skl, 45, 0, 0), huc_def(skl, 2, 0, 0))
#define __MAKE_UC_FW_PATH(prefix_, name_, major_, minor_, patch_) \
"i915/" \
@@ -371,6 +367,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
}
}
+ if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
+ uc_fw->private_data_size = css->private_data_size;
+
obj = i915_gem_object_create_shmem_from_data(i915, fw->data, fw->size);
if (IS_ERR(obj)) {
err = PTR_ERR(obj);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index 23d3a423ac0f..99bb1fe1af66 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -88,6 +88,8 @@ struct intel_uc_fw {
u32 rsa_size;
u32 ucode_size;
+
+ u32 private_data_size;
};
#ifdef CONFIG_DRM_I915_DEBUG_GUC
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
index 029214cdedd5..e41ffc7a7fbc 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
@@ -69,7 +69,11 @@ struct uc_css_header {
#define CSS_SW_VERSION_UC_MAJOR (0xFF << 16)
#define CSS_SW_VERSION_UC_MINOR (0xFF << 8)
#define CSS_SW_VERSION_UC_PATCH (0xFF << 0)
- u32 reserved[14];
+ u32 reserved0[13];
+ union {
+ u32 private_data_size; /* only applies to GuC */
+ u32 reserved1;
+ };
u32 header_info;
} __packed;
static_assert(sizeof(struct uc_css_header) == 128);
--
2.25.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Intel-gfx] [PATCH 2/2] drm/i915/uc: turn on GuC/HuC auto mode by default
2020-08-07 17:46 [Intel-gfx] [PATCH 0/2] drm/i915/guc: Update to GuC v45 John.C.Harrison
2020-08-07 17:46 ` [Intel-gfx] [PATCH 1/2] drm/i915/guc: Update to GuC v45.0.0 John.C.Harrison
@ 2020-08-07 17:46 ` John.C.Harrison
2020-08-07 18:33 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/guc: Update to GuC v45 Patchwork
` (3 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: John.C.Harrison @ 2020-08-07 17:46 UTC (permalink / raw)
To: Intel-GFX
From: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
This will enable HuC loading for Gen11+ by default if the binaries
are available on the system. GuC submission still requires explicit
enabling by the user.
Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
drivers/gpu/drm/i915/i915_params.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 53fb5ba8fbed..5b7644770e9b 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -57,7 +57,7 @@ struct drm_printer;
param(int, disable_power_well, -1, 0400) \
param(int, enable_ips, 1, 0600) \
param(int, invert_brightness, 0, 0600) \
- param(int, enable_guc, 0, 0400) \
+ param(int, enable_guc, -1, 0400) \
param(int, guc_log_level, -1, 0400) \
param(char *, guc_firmware_path, NULL, 0400) \
param(char *, huc_firmware_path, NULL, 0400) \
--
2.25.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/guc: Update to GuC v45
2020-08-07 17:46 [Intel-gfx] [PATCH 0/2] drm/i915/guc: Update to GuC v45 John.C.Harrison
2020-08-07 17:46 ` [Intel-gfx] [PATCH 1/2] drm/i915/guc: Update to GuC v45.0.0 John.C.Harrison
2020-08-07 17:46 ` [Intel-gfx] [PATCH 2/2] drm/i915/uc: turn on GuC/HuC auto mode by default John.C.Harrison
@ 2020-08-07 18:33 ` Patchwork
2020-08-07 18:34 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
` (2 subsequent siblings)
5 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2020-08-07 18:33 UTC (permalink / raw)
To: John.C.Harrison; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/guc: Update to GuC v45
URL : https://patchwork.freedesktop.org/series/80402/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
711a49fe9138 drm/i915/guc: Update to GuC v45.0.0
-:228: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#228:
new file mode 100644
-:461: WARNING:TYPO_SPELLING: 'entires' may be misspelled - perhaps 'entries'?
#461: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c:14:
+ * all the extra buffers indirectly linked via the ADS struct's entires.
-:530: CHECK:ASSIGNMENT_CONTINUATIONS: Assignment operator '=' should be on the previous line
#530: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c:109:
+ system_info->mapping_table[guc_class][engine->instance]
+ = engine->instance;
-:653: WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (8, 8)
#653: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c:97:
+ } else
if ((status & GS_UKERNEL_MASK) == GS_UKERNEL_EXCEPTION) {
-:653: CHECK:BRACES: Unbalanced braces around else statement
#653: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c:97:
+ } else
-:658: WARNING:SUSPECT_CODE_INDENT: suspect code indent for conditional statements (8, 8)
#658: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c:102:
+ } else
+ if (ret) {
-:658: CHECK:BRACES: Unbalanced braces around else statement
#658: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c:102:
+ } else
total: 0 errors, 4 warnings, 3 checks, 877 lines checked
5a3a8dce1672 drm/i915/uc: turn on GuC/HuC auto mode by default
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Update to GuC v45
2020-08-07 17:46 [Intel-gfx] [PATCH 0/2] drm/i915/guc: Update to GuC v45 John.C.Harrison
` (2 preceding siblings ...)
2020-08-07 18:33 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/guc: Update to GuC v45 Patchwork
@ 2020-08-07 18:34 ` Patchwork
2020-08-07 18:56 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-08-08 1:32 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
5 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2020-08-07 18:34 UTC (permalink / raw)
To: John.C.Harrison; +Cc: intel-gfx
== Series Details ==
Series: drm/i915/guc: Update to GuC v45
URL : https://patchwork.freedesktop.org/series/80402/
State : warning
== Summary ==
$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.0
Fast mode used, each commit won't be checked separately.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/guc: Update to GuC v45
2020-08-07 17:46 [Intel-gfx] [PATCH 0/2] drm/i915/guc: Update to GuC v45 John.C.Harrison
` (3 preceding siblings ...)
2020-08-07 18:34 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2020-08-07 18:56 ` Patchwork
2020-08-08 1:32 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
5 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2020-08-07 18:56 UTC (permalink / raw)
To: John.C.Harrison; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 11900 bytes --]
== Series Details ==
Series: drm/i915/guc: Update to GuC v45
URL : https://patchwork.freedesktop.org/series/80402/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_8859 -> Patchwork_18325
====================================================
Summary
-------
**WARNING**
Minor unknown changes coming with Patchwork_18325 need to be verified
manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_18325, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/index.html
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_18325:
### IGT changes ###
#### Warnings ####
* igt@gem_huc_copy@huc-copy:
- fi-cml-u2: [SKIP][1] ([i915#2190]) -> [SKIP][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-cml-u2/igt@gem_huc_copy@huc-copy.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-cml-u2/igt@gem_huc_copy@huc-copy.html
- fi-cml-s: [SKIP][3] ([i915#2190]) -> [SKIP][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-cml-s/igt@gem_huc_copy@huc-copy.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-cml-s/igt@gem_huc_copy@huc-copy.html
Known issues
------------
Here are the changes found in Patchwork_18325 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_selftest@live@gem_contexts:
- fi-tgl-u2: [PASS][5] -> [INCOMPLETE][6] ([i915#2045])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-tgl-u2/igt@i915_selftest@live@gem_contexts.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-tgl-u2/igt@i915_selftest@live@gem_contexts.html
* igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
- fi-bsw-n3050: [PASS][7] -> [DMESG-WARN][8] ([i915#1982])
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-bsw-n3050/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html
* igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2:
- fi-skl-guc: [PASS][9] -> [DMESG-WARN][10] ([i915#2203])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-skl-guc/igt@kms_flip@basic-flip-vs-wf_vblank@c-hdmi-a2.html
#### Possible fixes ####
* igt@gem_huc_copy@huc-copy:
- fi-tgl-u2: [SKIP][11] ([i915#2190]) -> [PASS][12]
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-tgl-u2/igt@gem_huc_copy@huc-copy.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-tgl-u2/igt@gem_huc_copy@huc-copy.html
- {fi-ehl-1}: [SKIP][13] ([i915#2190]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-ehl-1/igt@gem_huc_copy@huc-copy.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-ehl-1/igt@gem_huc_copy@huc-copy.html
- fi-icl-y: [SKIP][15] ([i915#2190]) -> [PASS][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-icl-y/igt@gem_huc_copy@huc-copy.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-icl-y/igt@gem_huc_copy@huc-copy.html
- {fi-tgl-dsi}: [SKIP][17] ([i915#2190]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-tgl-dsi/igt@gem_huc_copy@huc-copy.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-tgl-dsi/igt@gem_huc_copy@huc-copy.html
- fi-icl-u2: [SKIP][19] ([i915#2190]) -> [PASS][20]
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-icl-u2/igt@gem_huc_copy@huc-copy.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-icl-u2/igt@gem_huc_copy@huc-copy.html
* igt@i915_pm_rpm@basic-pci-d3-state:
- fi-bsw-kefka: [DMESG-WARN][21] ([i915#1982]) -> [PASS][22]
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-bsw-kefka/igt@i915_pm_rpm@basic-pci-d3-state.html
* igt@i915_pm_rpm@module-reload:
- {fi-kbl-7560u}: [DMESG-WARN][23] -> [PASS][24]
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-kbl-7560u/igt@i915_pm_rpm@module-reload.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-kbl-7560u/igt@i915_pm_rpm@module-reload.html
- fi-apl-guc: [DMESG-WARN][25] ([i915#1635] / [i915#1982]) -> [PASS][26]
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-apl-guc/igt@i915_pm_rpm@module-reload.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-apl-guc/igt@i915_pm_rpm@module-reload.html
* igt@i915_selftest@live@execlists:
- {fi-tgl-dsi}: [INCOMPLETE][27] ([i915#2268]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-tgl-dsi/igt@i915_selftest@live@execlists.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-tgl-dsi/igt@i915_selftest@live@execlists.html
* igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1:
- fi-icl-u2: [DMESG-WARN][29] ([i915#1982]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-icl-u2/igt@kms_flip@basic-flip-vs-wf_vblank@b-edp1.html
#### Warnings ####
* igt@gem_exec_suspend@basic-s0:
- fi-kbl-x1275: [DMESG-WARN][31] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][32] ([i915#62] / [i915#92]) +1 similar issue
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-kbl-x1275/igt@gem_exec_suspend@basic-s0.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-kbl-x1275/igt@gem_exec_suspend@basic-s0.html
* igt@gem_huc_copy@huc-copy:
- fi-skl-6600u: [SKIP][33] ([fdo#109271] / [i915#2190]) -> [SKIP][34] ([fdo#109271])
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-skl-6600u/igt@gem_huc_copy@huc-copy.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-skl-6600u/igt@gem_huc_copy@huc-copy.html
- fi-skl-6700k2: [SKIP][35] ([fdo#109271] / [i915#2190]) -> [SKIP][36] ([fdo#109271])
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-skl-6700k2/igt@gem_huc_copy@huc-copy.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-skl-6700k2/igt@gem_huc_copy@huc-copy.html
- fi-kbl-soraka: [SKIP][37] ([fdo#109271] / [i915#2190]) -> [SKIP][38] ([fdo#109271])
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-kbl-soraka/igt@gem_huc_copy@huc-copy.html
- fi-skl-lmem: [SKIP][39] ([fdo#109271] / [i915#2190]) -> [SKIP][40] ([fdo#109271])
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-skl-lmem/igt@gem_huc_copy@huc-copy.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-skl-lmem/igt@gem_huc_copy@huc-copy.html
- fi-kbl-x1275: [SKIP][41] ([fdo#109271] / [i915#2190]) -> [SKIP][42] ([fdo#109271])
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-kbl-x1275/igt@gem_huc_copy@huc-copy.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-kbl-x1275/igt@gem_huc_copy@huc-copy.html
- fi-kbl-7500u: [SKIP][43] ([fdo#109271] / [i915#2190]) -> [SKIP][44] ([fdo#109271])
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-kbl-7500u/igt@gem_huc_copy@huc-copy.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-kbl-7500u/igt@gem_huc_copy@huc-copy.html
- fi-kbl-r: [SKIP][45] ([fdo#109271] / [i915#2190]) -> [SKIP][46] ([fdo#109271])
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-kbl-r/igt@gem_huc_copy@huc-copy.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-kbl-r/igt@gem_huc_copy@huc-copy.html
- fi-cfl-8109u: [SKIP][47] ([fdo#109271] / [i915#2190]) -> [SKIP][48] ([fdo#109271])
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-cfl-8109u/igt@gem_huc_copy@huc-copy.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-cfl-8109u/igt@gem_huc_copy@huc-copy.html
- fi-bxt-dsi: [SKIP][49] ([fdo#109271] / [i915#1635] / [i915#2190]) -> [SKIP][50] ([fdo#109271] / [i915#1635])
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-bxt-dsi/igt@gem_huc_copy@huc-copy.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-bxt-dsi/igt@gem_huc_copy@huc-copy.html
- fi-cfl-8700k: [SKIP][51] ([fdo#109271] / [i915#2190]) -> [SKIP][52] ([fdo#109271])
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-cfl-8700k/igt@gem_huc_copy@huc-copy.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-cfl-8700k/igt@gem_huc_copy@huc-copy.html
* igt@i915_module_load@reload:
- fi-icl-u2: [DMESG-WARN][53] ([i915#289]) -> [DMESG-WARN][54] ([i915#1982] / [i915#289])
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-icl-u2/igt@i915_module_load@reload.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-icl-u2/igt@i915_module_load@reload.html
* igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
- fi-kbl-x1275: [DMESG-WARN][55] ([i915#62] / [i915#92]) -> [DMESG-WARN][56] ([i915#62] / [i915#92] / [i915#95]) +1 similar issue
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/fi-kbl-x1275/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/fi-kbl-x1275/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2045]: https://gitlab.freedesktop.org/drm/intel/issues/2045
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#2203]: https://gitlab.freedesktop.org/drm/intel/issues/2203
[i915#2268]: https://gitlab.freedesktop.org/drm/intel/issues/2268
[i915#289]: https://gitlab.freedesktop.org/drm/intel/issues/289
[i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
[i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
[i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
Participating hosts (44 -> 38)
------------------------------
Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_8859 -> Patchwork_18325
CI-20190529: 20190529
CI_DRM_8859: 5384db4f16cc5e32864bb76f06cd67463be21023 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5765: 9f0977284d54ed37496260988dfcd6d2ad72dd1e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_18325: 5a3a8dce1672e306a359a6639d93cade01a01fed @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
5a3a8dce1672 drm/i915/uc: turn on GuC/HuC auto mode by default
711a49fe9138 drm/i915/guc: Update to GuC v45.0.0
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/index.html
[-- Attachment #1.2: Type: text/html, Size: 16255 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* [Intel-gfx] ✗ Fi.CI.IGT: failure for drm/i915/guc: Update to GuC v45
2020-08-07 17:46 [Intel-gfx] [PATCH 0/2] drm/i915/guc: Update to GuC v45 John.C.Harrison
` (4 preceding siblings ...)
2020-08-07 18:56 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2020-08-08 1:32 ` Patchwork
5 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2020-08-08 1:32 UTC (permalink / raw)
To: John.C.Harrison; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 16949 bytes --]
== Series Details ==
Series: drm/i915/guc: Update to GuC v45
URL : https://patchwork.freedesktop.org/series/80402/
State : failure
== Summary ==
CI Bug Log - changes from CI_DRM_8859_full -> Patchwork_18325_full
====================================================
Summary
-------
**FAILURE**
Serious unknown changes coming with Patchwork_18325_full absolutely need to be
verified manually.
If you think the reported changes have nothing to do with the changes
introduced in Patchwork_18325_full, please notify your bug team to allow them
to document this new failure mode, which will reduce false positives in CI.
Possible new issues
-------------------
Here are the unknown changes that may have been introduced in Patchwork_18325_full:
### IGT changes ###
#### Possible regressions ####
* igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
- shard-skl: [PASS][1] -> [INCOMPLETE][2]
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-skl10/igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-skl7/igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend.html
#### Warnings ####
* igt@gem_ctx_shared@disjoint-timelines:
- shard-hsw: [SKIP][3] ([fdo#109271]) -> [FAIL][4]
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-hsw2/igt@gem_ctx_shared@disjoint-timelines.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-hsw1/igt@gem_ctx_shared@disjoint-timelines.html
- shard-snb: [SKIP][5] ([fdo#109271]) -> [FAIL][6]
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-snb5/igt@gem_ctx_shared@disjoint-timelines.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-snb6/igt@gem_ctx_shared@disjoint-timelines.html
Known issues
------------
Here are the changes found in Patchwork_18325_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_exec_gttfill@engines@rcs0:
- shard-glk: [PASS][7] -> [DMESG-WARN][8] ([i915#118] / [i915#95]) +1 similar issue
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-glk4/igt@gem_exec_gttfill@engines@rcs0.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-glk7/igt@gem_exec_gttfill@engines@rcs0.html
* igt@i915_selftest@mock@contexts:
- shard-apl: [PASS][9] -> [INCOMPLETE][10] ([i915#1635])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-apl1/igt@i915_selftest@mock@contexts.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-apl6/igt@i915_selftest@mock@contexts.html
* igt@kms_cursor_legacy@cursora-vs-flipb-legacy:
- shard-glk: [PASS][11] -> [DMESG-WARN][12] ([i915#1982]) +1 similar issue
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-glk7/igt@kms_cursor_legacy@cursora-vs-flipb-legacy.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-glk3/igt@kms_cursor_legacy@cursora-vs-flipb-legacy.html
* igt@kms_flip@2x-flip-vs-suspend@ab-vga1-hdmi-a1:
- shard-hsw: [PASS][13] -> [DMESG-WARN][14] ([i915#1982])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-hsw4/igt@kms_flip@2x-flip-vs-suspend@ab-vga1-hdmi-a1.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-hsw6/igt@kms_flip@2x-flip-vs-suspend@ab-vga1-hdmi-a1.html
* igt@kms_flip@flip-vs-expired-vblank@c-edp1:
- shard-skl: [PASS][15] -> [DMESG-FAIL][16] ([i915#1982] / [i915#79])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-skl7/igt@kms_flip@flip-vs-expired-vblank@c-edp1.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-skl2/igt@kms_flip@flip-vs-expired-vblank@c-edp1.html
* igt@kms_flip@flip-vs-suspend-interruptible@c-hdmi-a1:
- shard-hsw: [PASS][17] -> [INCOMPLETE][18] ([i915#2055])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-hsw1/igt@kms_flip@flip-vs-suspend-interruptible@c-hdmi-a1.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-hsw2/igt@kms_flip@flip-vs-suspend-interruptible@c-hdmi-a1.html
* igt@kms_flip@flip-vs-suspend@c-dp1:
- shard-kbl: [PASS][19] -> [DMESG-WARN][20] ([i915#180]) +4 similar issues
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-kbl4/igt@kms_flip@flip-vs-suspend@c-dp1.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-kbl7/igt@kms_flip@flip-vs-suspend@c-dp1.html
* igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt:
- shard-kbl: [PASS][21] -> [DMESG-WARN][22] ([i915#1982])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-kbl4/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-indfb-draw-blt.html
* igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-wc:
- shard-tglb: [PASS][23] -> [DMESG-WARN][24] ([i915#1982]) +1 similar issue
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-tglb1/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-wc.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-tglb2/igt@kms_frontbuffer_tracking@fbc-1p-offscren-pri-shrfb-draw-mmap-wc.html
* igt@kms_hdr@bpc-switch-suspend:
- shard-skl: [PASS][25] -> [FAIL][26] ([i915#1188])
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-skl8/igt@kms_hdr@bpc-switch-suspend.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-skl9/igt@kms_hdr@bpc-switch-suspend.html
* igt@kms_plane@plane-position-covered-pipe-b-planes:
- shard-skl: [PASS][27] -> [DMESG-WARN][28] ([i915#1982]) +13 similar issues
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-skl7/igt@kms_plane@plane-position-covered-pipe-b-planes.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-skl2/igt@kms_plane@plane-position-covered-pipe-b-planes.html
* igt@kms_psr@psr2_sprite_plane_move:
- shard-iclb: [PASS][29] -> [SKIP][30] ([fdo#109441]) +2 similar issues
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-iclb8/igt@kms_psr@psr2_sprite_plane_move.html
* igt@perf@polling-parameterized:
- shard-iclb: [PASS][31] -> [FAIL][32] ([i915#1542])
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-iclb4/igt@perf@polling-parameterized.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-iclb3/igt@perf@polling-parameterized.html
#### Possible fixes ####
* igt@gem_ctx_shared@q-smoketest-all:
- shard-glk: [DMESG-WARN][33] ([i915#118] / [i915#95]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-glk4/igt@gem_ctx_shared@q-smoketest-all.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-glk6/igt@gem_ctx_shared@q-smoketest-all.html
* igt@gem_huc_copy@huc-copy:
- shard-iclb: [SKIP][35] ([i915#2190]) -> [PASS][36]
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-iclb7/igt@gem_huc_copy@huc-copy.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-iclb3/igt@gem_huc_copy@huc-copy.html
* igt@kms_draw_crc@draw-method-xrgb2101010-mmap-cpu-ytiled:
- shard-glk: [DMESG-WARN][37] ([i915#1982]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-glk9/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-cpu-ytiled.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-glk5/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-cpu-ytiled.html
* igt@kms_flip@2x-flip-vs-fences-interruptible@ab-vga1-hdmi-a1:
- shard-hsw: [DMESG-WARN][39] ([i915#1982]) -> [PASS][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-hsw1/igt@kms_flip@2x-flip-vs-fences-interruptible@ab-vga1-hdmi-a1.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-hsw2/igt@kms_flip@2x-flip-vs-fences-interruptible@ab-vga1-hdmi-a1.html
* igt@kms_flip@flip-vs-suspend@b-edp1:
- shard-iclb: [INCOMPLETE][41] ([i915#1373]) -> [PASS][42]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-iclb3/igt@kms_flip@flip-vs-suspend@b-edp1.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-iclb4/igt@kms_flip@flip-vs-suspend@b-edp1.html
* igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-mmap-cpu:
- shard-kbl: [DMESG-WARN][43] ([i915#1982]) -> [PASS][44]
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-kbl4/igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-mmap-cpu.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-kbl7/igt@kms_frontbuffer_tracking@fbc-rgb101010-draw-mmap-cpu.html
* igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-mmap-cpu:
- shard-tglb: [DMESG-WARN][45] ([i915#1982]) -> [PASS][46] +1 similar issue
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-tglb5/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-mmap-cpu.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-tglb1/igt@kms_frontbuffer_tracking@psr-1p-primscrn-pri-shrfb-draw-mmap-cpu.html
* igt@kms_frontbuffer_tracking@psr-rgb101010-draw-pwrite:
- shard-skl: [DMESG-WARN][47] ([i915#1982]) -> [PASS][48] +7 similar issues
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-skl3/igt@kms_frontbuffer_tracking@psr-rgb101010-draw-pwrite.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-skl5/igt@kms_frontbuffer_tracking@psr-rgb101010-draw-pwrite.html
* igt@kms_hdr@bpc-switch:
- shard-skl: [FAIL][49] ([i915#1188]) -> [PASS][50]
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-skl10/igt@kms_hdr@bpc-switch.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-skl7/igt@kms_hdr@bpc-switch.html
* igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min:
- shard-skl: [FAIL][51] ([fdo#108145] / [i915#265]) -> [PASS][52]
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-skl10/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-skl7/igt@kms_plane_alpha_blend@pipe-c-constant-alpha-min.html
* igt@kms_plane_scaling@pipe-c-scaler-with-clipping-clamping:
- shard-iclb: [DMESG-WARN][53] ([i915#1982]) -> [PASS][54]
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-iclb3/igt@kms_plane_scaling@pipe-c-scaler-with-clipping-clamping.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-iclb1/igt@kms_plane_scaling@pipe-c-scaler-with-clipping-clamping.html
* igt@kms_psr@psr2_cursor_render:
- shard-iclb: [SKIP][55] ([fdo#109441]) -> [PASS][56] +2 similar issues
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-iclb4/igt@kms_psr@psr2_cursor_render.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-iclb2/igt@kms_psr@psr2_cursor_render.html
* igt@kms_vblank@pipe-a-ts-continuation-suspend:
- shard-kbl: [DMESG-WARN][57] ([i915#180]) -> [PASS][58] +8 similar issues
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-kbl2/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-kbl3/igt@kms_vblank@pipe-a-ts-continuation-suspend.html
* igt@perf@blocking-parameterized:
- shard-iclb: [FAIL][59] ([i915#1542]) -> [PASS][60]
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-iclb1/igt@perf@blocking-parameterized.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-iclb4/igt@perf@blocking-parameterized.html
#### Warnings ####
* igt@gem_huc_copy@huc-copy:
- shard-apl: [SKIP][61] ([fdo#109271] / [i915#1635] / [i915#2190]) -> [SKIP][62] ([fdo#109271] / [i915#1635])
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-apl1/igt@gem_huc_copy@huc-copy.html
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-apl8/igt@gem_huc_copy@huc-copy.html
- shard-glk: [SKIP][63] ([fdo#109271] / [i915#2190]) -> [SKIP][64] ([fdo#109271])
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-glk1/igt@gem_huc_copy@huc-copy.html
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-glk4/igt@gem_huc_copy@huc-copy.html
- shard-kbl: [SKIP][65] ([fdo#109271] / [i915#2190]) -> [SKIP][66] ([fdo#109271])
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-kbl7/igt@gem_huc_copy@huc-copy.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-kbl6/igt@gem_huc_copy@huc-copy.html
- shard-skl: [SKIP][67] ([fdo#109271] / [i915#2190]) -> [SKIP][68] ([fdo#109271])
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-skl1/igt@gem_huc_copy@huc-copy.html
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-skl1/igt@gem_huc_copy@huc-copy.html
* igt@kms_content_protection@atomic-dpms:
- shard-kbl: [TIMEOUT][69] ([i915#1319] / [i915#1958]) -> [TIMEOUT][70] ([i915#1319])
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-kbl3/igt@kms_content_protection@atomic-dpms.html
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-kbl3/igt@kms_content_protection@atomic-dpms.html
* igt@kms_dp_dsc@basic-dsc-enable-edp:
- shard-iclb: [SKIP][71] ([fdo#109349]) -> [DMESG-WARN][72] ([i915#1226])
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-iclb6/igt@kms_dp_dsc@basic-dsc-enable-edp.html
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-iclb2/igt@kms_dp_dsc@basic-dsc-enable-edp.html
* igt@kms_flip@plain-flip-ts-check-interruptible@a-edp1:
- shard-skl: [DMESG-FAIL][73] ([i915#1982]) -> [DMESG-WARN][74] ([i915#1982])
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8859/shard-skl8/igt@kms_flip@plain-flip-ts-check-interruptible@a-edp1.html
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/shard-skl9/igt@kms_flip@plain-flip-ts-check-interruptible@a-edp1.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[fdo#109349]: https://bugs.freedesktop.org/show_bug.cgi?id=109349
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
[i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
[i915#1226]: https://gitlab.freedesktop.org/drm/intel/issues/1226
[i915#1319]: https://gitlab.freedesktop.org/drm/intel/issues/1319
[i915#1373]: https://gitlab.freedesktop.org/drm/intel/issues/1373
[i915#1542]: https://gitlab.freedesktop.org/drm/intel/issues/1542
[i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#1958]: https://gitlab.freedesktop.org/drm/intel/issues/1958
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2055]: https://gitlab.freedesktop.org/drm/intel/issues/2055
[i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
[i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
[i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
[i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
[i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
Participating hosts (11 -> 11)
------------------------------
No changes in participating hosts
Build changes
-------------
* Linux: CI_DRM_8859 -> Patchwork_18325
CI-20190529: 20190529
CI_DRM_8859: 5384db4f16cc5e32864bb76f06cd67463be21023 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5765: 9f0977284d54ed37496260988dfcd6d2ad72dd1e @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_18325: 5a3a8dce1672e306a359a6639d93cade01a01fed @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18325/index.html
[-- Attachment #1.2: Type: text/html, Size: 20698 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915/guc: Update to GuC v45.0.0
2020-08-07 17:46 ` [Intel-gfx] [PATCH 1/2] drm/i915/guc: Update to GuC v45.0.0 John.C.Harrison
@ 2020-08-11 1:04 ` Daniele Ceraolo Spurio
2020-08-19 21:14 ` Michal Wajdeczko
0 siblings, 1 reply; 10+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-08-11 1:04 UTC (permalink / raw)
To: John.C.Harrison, Intel-GFX, Wajdeczko, Michal
On 8/7/2020 10:46 AM, John.C.Harrison@Intel.com wrote:
> From: John Harrison <John.C.Harrison@Intel.com>
>
> Update to the latest GuC firmware. This includes some significant
> changes to the interface.
A high level overview of the changes would be nice for extra clarity. A
couple of example:
- GuC now requires a private memory area
- you're dropping the programming of the reg_state struct, but that's
actually still defined in the structure, so IMO it's worth mentioning
that it will come back with GuC submission.
>
> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
> Author: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Author: John Harrison <John.C.Harrison@Intel.com>
> Author: Matthew Brost <matthew.brost@intel.com>
> Author: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Author: Michel Thierry <michel.thierry@intel.com>
> Author: Oscar Mateo <oscar.mateo@intel.com>
> Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
> ---
> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 3 +-
> drivers/gpu/drm/i915/gt/uc/intel_guc.c | 125 +++++++----
> drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h | 215 +++++++++++++++++++
> drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 116 ++++++++--
> drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 21 +-
> drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 110 ++++------
> drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 5 +
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 27 ++-
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 2 +
> drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h | 6 +-
> 10 files changed, 485 insertions(+), 145 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index ea4ba2afe9f9..9cd62d68abc6 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -305,8 +305,9 @@ static int intel_engine_setup(struct intel_gt *gt, enum intel_engine_id id)
> engine->i915 = i915;
> engine->gt = gt;
> engine->uncore = gt->uncore;
> - engine->hw_id = engine->guc_id = info->hw_id;
> engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
> + engine->hw_id = info->hw_id;
> + engine->guc_id = MAKE_GUC_ID(info->class, info->instance);
>
> engine->class = info->class;
> engine->instance = info->instance;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 861657897c0f..8d30e1d7d8a6 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -7,6 +7,7 @@
> #include "gt/intel_gt_irq.h"
> #include "gt/intel_gt_pm_irq.h"
> #include "intel_guc.h"
> +#include "intel_guc_abi.h"
> #include "intel_guc_ads.h"
> #include "intel_guc_submission.h"
> #include "i915_drv.h"
> @@ -213,23 +214,6 @@ static u32 guc_ctl_feature_flags(struct intel_guc *guc)
> return flags;
> }
>
> -static u32 guc_ctl_ctxinfo_flags(struct intel_guc *guc)
> -{
> - u32 flags = 0;
> -
> - if (intel_guc_submission_is_used(guc)) {
> - u32 ctxnum, base;
> -
> - base = intel_guc_ggtt_offset(guc, guc->stage_desc_pool);
> - ctxnum = GUC_MAX_STAGE_DESCRIPTORS / 16;
> -
> - base >>= PAGE_SHIFT;
> - flags |= (base << GUC_CTL_BASE_ADDR_SHIFT) |
> - (ctxnum << GUC_CTL_CTXNUM_IN16_SHIFT);
> - }
> - return flags;
> -}
> -
> static u32 guc_ctl_log_params_flags(struct intel_guc *guc)
> {
> u32 offset = intel_guc_ggtt_offset(guc, guc->log.vma) >> PAGE_SHIFT;
> @@ -291,7 +275,6 @@ static void guc_init_params(struct intel_guc *guc)
>
> BUILD_BUG_ON(sizeof(guc->params) != GUC_CTL_MAX_DWORDS * sizeof(u32));
>
> - params[GUC_CTL_CTXINFO] = guc_ctl_ctxinfo_flags(guc);
> params[GUC_CTL_LOG_PARAMS] = guc_ctl_log_params_flags(guc);
> params[GUC_CTL_FEATURE] = guc_ctl_feature_flags(guc);
> params[GUC_CTL_DEBUG] = guc_ctl_debug_flags(guc);
> @@ -400,32 +383,65 @@ void intel_guc_fini(struct intel_guc *guc)
> intel_uc_fw_fini(&guc->fw);
> }
>
> +static bool is_valid_mmio_action(u32 action)
> +{
> + return action == GUC_ACTION_REGISTER_CTB ||
> + action == GUC_ACTION_DEREGISTER_CTB;
> +}
> +
> +static int guc_status_to_errno(u32 status)
> +{
> + switch (status) {
> + case GUC_STATUS_UNKNOWN_ACTION:
> + return -EOPNOTSUPP;
> + case GUC_STATUS_INVALID_PARAMS:
> + return -EINVAL;
> + case GUC_STATUS_INVALID_ADDR:
> + return -EFAULT;
> + case GUC_STATUS_CTX_NOT_REGISTERED:
> + return -ESRCH;
> + case GUC_STATUS_CTB_FULL:
> + return -ENOSPC;
> + case GUC_STATUS_UNAUTHORIZED_REQUEST:
> + return -EACCES;
> + case GUC_STATUS_GENERIC_FAIL:
> + default:
> + return -ENXIO;
> + }
> +}
> +
> /*
> * This function implements the MMIO based host to GuC interface.
> */
> -int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
> +int intel_guc_send_mmio(struct intel_guc *guc, const u32 *request, u32 len,
> u32 *response_buf, u32 response_buf_size)
> {
> + struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
> struct intel_uncore *uncore = guc_to_gt(guc)->uncore;
> - u32 status;
> + u32 action = FIELD_GET(GUC_MMIO_REQUEST_ACTION, *request);
> + u32 subcode = FIELD_GET(GUC_MMIO_REQUEST_SUBCODE, *request);
> + u32 magic = GUC_MMIO_MSG_MAGIC_DEFAULT;
> + u32 header;
> int i;
> int ret;
>
> GEM_BUG_ON(!len);
> GEM_BUG_ON(len > guc->send_regs.count);
>
> - /* We expect only action code */
> - GEM_BUG_ON(*action & ~INTEL_GUC_MSG_CODE_MASK);
> + /* We expect to use MMIO only for special actions */
> + GEM_BUG_ON(!is_valid_mmio_action(action));
>
> - /* If CT is available, we expect to use MMIO only during init/fini */
> - GEM_BUG_ON(*action != INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER &&
> - *action != INTEL_GUC_ACTION_DEREGISTER_COMMAND_TRANSPORT_BUFFER);
> + header = FIELD_PREP(GUC_MMIO_MSG_TYPE, GUC_MMIO_MSG_TYPE_REQUEST) |
> + FIELD_PREP(GUC_MMIO_MSG_MAGIC, magic) |
> + FIELD_PREP(GUC_MMIO_REQUEST_SUBCODE, subcode) |
> + FIELD_PREP(GUC_MMIO_REQUEST_ACTION, action);
>
> mutex_lock(&guc->send_mutex);
> intel_uncore_forcewake_get(uncore, guc->send_regs.fw_domains);
>
> - for (i = 0; i < len; i++)
> - intel_uncore_write(uncore, guc_send_reg(guc, i), action[i]);
> + intel_uncore_write(uncore, guc_send_reg(guc, 0), header);
> + for (i = 1; i < len; i++)
> + intel_uncore_write(uncore, guc_send_reg(guc, i), request[i]);
>
> intel_uncore_posting_read(uncore, guc_send_reg(guc, i - 1));
>
> @@ -437,17 +453,45 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
> */
> ret = __intel_wait_for_register_fw(uncore,
> guc_send_reg(guc, 0),
> - INTEL_GUC_MSG_TYPE_MASK,
> - INTEL_GUC_MSG_TYPE_RESPONSE <<
> - INTEL_GUC_MSG_TYPE_SHIFT,
> - 10, 10, &status);
> - /* If GuC explicitly returned an error, convert it to -EIO */
> - if (!ret && !INTEL_GUC_MSG_IS_RESPONSE_SUCCESS(status))
> - ret = -EIO;
> + GUC_MMIO_MSG_ORIGIN,
> + FIELD_PREP(GUC_MMIO_MSG_ORIGIN,
> + GUC_MMIO_MSG_ORIGIN_GUC),
> + 10, 10, &header);
> + if (unlikely(ret))
> + goto out;
>
> - if (ret) {
> - DRM_ERROR("MMIO: GuC action %#x failed with error %d %#x\n",
> - action[0], ret, status);
> + if (unlikely(FIELD_GET(GUC_MMIO_MSG_MAGIC, header) != magic)) {
> + ret = -ESTALE;
> + goto out;
> + }
I think we should move this after the check for busyness. IIRC the
protocol says we need to wait for the GuC to stop being busy before
sending more messages. That way you can also unify this with the other
similar check below.
> +
> + if (FIELD_GET(GUC_MMIO_MSG_TYPE, header) == GUC_MMIO_MSG_TYPE_BUSY) {
> +#define done ({ header = intel_uncore_read(uncore, guc_send_reg(guc, 0)); \
> + FIELD_GET(GUC_MMIO_MSG_TYPE, header) != GUC_MMIO_MSG_TYPE_BUSY; })
> + ret = wait_for(done, 1000);
> + if (unlikely(ret))
> + goto out;
> + if (unlikely(FIELD_GET(GUC_MMIO_MSG_ORIGIN, header) !=
> + GUC_MMIO_MSG_ORIGIN_GUC)) {
This check on GUC_MMIO_MSG_ORIGIN is redundant. We already waited for it
to be GUC_MMIO_MSG_ORIGIN_GUC during the wait above and we're not
sending other message from i915 while we're waiting for this one, so
that field can't change.
> + ret = -EPROTO;
> + goto out;
> + }
> + if (unlikely(FIELD_GET(GUC_MMIO_MSG_MAGIC, header) != magic)) {
> + ret = -ESTALE;
> + goto out;
> + }
> +#undef done
> + }
> +
> + if (FIELD_GET(GUC_MMIO_MSG_TYPE, header) == GUC_MMIO_MSG_TYPE_ERROR) {
> + u32 status = FIELD_GET(GUC_MMIO_ERROR_STATUS, header);
> +
> + ret = guc_status_to_errno(status);
> + goto out;
> + }
> +
> + if (FIELD_GET(GUC_MMIO_MSG_TYPE, header) != GUC_MMIO_MSG_TYPE_RESPONSE) {
> + ret = -EPROTO;
> goto out;
> }
>
> @@ -460,12 +504,17 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
> }
>
> /* Use data from the GuC response as our return value */
> - ret = INTEL_GUC_MSG_TO_DATA(status);
INTEL_GUC_MSG_TO_DATA is still used in intel_guc_ct.c. Either stick to
that or update the other use-case to the new define.
> + ret = FIELD_GET(GUC_MMIO_RESPONSE_DATA0, header);
>
> out:
> intel_uncore_forcewake_put(uncore, guc->send_regs.fw_domains);
> mutex_unlock(&guc->send_mutex);
>
> + if (unlikely(ret < 0))
> + drm_err(&i915->drm,
> + "GuC MMIO action %#06x failed with error %d %#x\n",
> + *request, ret, header);
> +
> return ret;
> }
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h
> new file mode 100644
> index 000000000000..95043788e0ad
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h
> @@ -0,0 +1,215 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2020 Intel Corporation
> + */
> +
> +#ifndef __INTEL_GUC_ABI_H__
> +#define __INTEL_GUC_ABI_H__
> +
> +/**
> + * DOC: GuC MMIO based communication
> + *
> + * The MMIO based communication between Host and GuC relies on special
> + * hardware registers which format could be defined by the software
> + * (so called scratch registers).
> + *
> + * Each MMIO based message, both Host to GuC (H2G) and GuC to Host (G2H)
> + * messages, which maximum length depends on number of available scratch
> + * registers, is directly written into those scratch registers.
> + *
> + * For Gen9+, there are 16 software scratch registers 0xC180-0xC1B8,
> + * but no H2G command takes more than 8 parameters and the GuC firmware
> + * itself uses an 8-element array to store the H2G message.
> + *
> + * For Gen11+, there are additional 4 registers 0x190240-0x19024C, which
> + * are, regardless on lower count, preffered over legacy ones.
> + *
> + * The MMIO based communication is mainly used during driver initialization
> + * phase to setup the CTB based communication that will be used afterwards.
> + *
> + * Details of the messages formats depend on GuC firmware version being used
> + * by the host driver. Documented here messages are for GuC 45.0 and newer.
> + */
> +
> +#define GUC_MAX_MMIO_MSG_LEN 8
Are we ok with not having an INTEL_ prefix to all the defines? I'm not
complaining, we usually stick with that prefix but I'm not sure if it is
an iron rule or not.
> +
> +/**
> + * DOC: GuC MMIO message format
> + *
> + * Bits of the first scratch register are treated as a message header,
> + * and other registers values are used to hold message payload (data)::
> + *
> + * +=======================================================+
> + * | SCRATCH | |
> + * +-----------+-------------------------------------------+
> + * | 0 | 31:28 | message ORIGIN/TYPE |
> + * | | 27:24 | message MAGIC |
> + * | | 23:0 | message DATA0 (depends on TYPE) |
> + * +---+-------+-------------------------------------------+
> + * | 1 | 31:0 | message DATA1 (depends on TYPE) |
> + * |...| | message DATA2 (depends on TYPE) |
> + * | n | | message DATA3 (depends on TYPE) |
> + * +=======================================================+
> + *
> + * Where:
> + * - **TYPE** is a 4b message type identifier.
> + * - **MAGIC** is a 4b message sequence number.
> + * The same sequence number must be included in all related messages.
> + * This field is used for tracking and debug purposes.
I don't think "magic" is a clear enough name for the role of the field.
"Request id" or something like that would be more appropriate IMO.
> + * - **DATA0..3** bits represents message payload.
> + * Definitions of these bits depends on message **TYPE**.
> + *
> + * The MSB of the header and **TYPE** indicates origin of the message:
> + * - 0 - message from the Host
> + * - 1 - message from the GuC
> + *
> + * Currently supported message types are:
> + * - 0x0 - **REQUEST** - represents Host request message to the GuC
> + * - 0xF - **SUCCESS RESPONSE** - GuC reply for the earlier request
> + * - 0xE - **ERROR RESPONSE** - GuC failure reply for the earlier request
> + * - 0xB - **BUSY** - GuC will be processing request for the longer time
> + */
> +
> +#define GUC_MMIO_MSG_ORIGIN (0x1 << 31)
> +#define GUC_MMIO_MSG_ORIGIN_HOST 0u
> +#define GUC_MMIO_MSG_ORIGIN_GUC 1u
> +
> +#define GUC_MMIO_MSG_TYPE (0xF << 28)
> +#define GUC_MMIO_MSG_TYPE_REQUEST 0x0
> +#define GUC_MMIO_MSG_TYPE_RESPONSE 0xF
> +#define GUC_MMIO_MSG_TYPE_ERROR 0xE
> +#define GUC_MMIO_MSG_TYPE_BUSY 0xB
> +
> +#define GUC_MMIO_MSG_MAGIC (0xF << 24)
> +#define GUC_MMIO_MSG_MAGIC_DEFAULT 0u
> +
> +/**
> + * DOC: GuC MMIO H2G REQUEST
> + *
> + * The MMIO H2G REQUEST message is used by the Host to request some action
> + * to be performed by the GuC::
> + *
> + * +=======================================================+
> + * | SCRATCH | |
> + * +=======================================================+
> + * | 0 | 31:28 | message TYPE = 0x0 = REQUEST |
> + * | | 27:24 | message MAGIC |
> + * | | 23:16 | request SUBCODE (depends on ACTION) |
> + * | | 15:0 | request ACTION code |
> + * +-------------------------------------------------------+
> + * | 1 | 31:0 | request DATA1 (depends on ACTION/SUBCODE) |
> + * |...| | request DATA2 (depends on ACTION/SUBCODE) |
> + * | n | 31:0 | request DATA3 (depends on ACTION/SUBCODE) |
> + * +=======================================================+
> + *
> + * Where:
> + * - **TYPE** must be set to value 0x0.
> + * - **MAGIC** message sequence number is generated by the host.
> + * - **ACTION** represents 16b request action code that defines both
> + * purpose of the request and format of the passed payload data.
> + * List of supported codes depends on GuC version and plaform.
> + * Action code can't be zero.
> + * - **SUBCODE** is optional 8b data related to the **ACTION**.
> + * MBZ if not explicitly defined by given **ACTION**.
> + * - **DATA1..3** are optional 32b payload dwords related to **ACTION**.
> + * Format of each dword is defined by specific **ACTION**.
> + */
> +
> +#define GUC_MMIO_REQUEST_SUBCODE (0xFF << 16)
> +#define GUC_MMIO_REQUEST_ACTION (0xFFFF << 0)
> +#define GUC_ACTION_INVALID 0x0000
> +#define GUC_ACTION_REGISTER_CTB 0x4505
> +#define GUC_ACTION_DEREGISTER_CTB 0x4506
These are now defined twice (once here and once in the enum). We should
pick one for consistency.
> +
> +/**
> + * DOC: GuC MMIO G2H SUCCESS RESPONSE
> + *
> + * The MMIO G2H SUCCESS RESPONSE message is used by the GuC to reply to
> + * the earlier H2G request message. This message is used if no errors
> + * were encountered by the GuC during processing of the request::
> + *
> + * +=======================================================+
> + * | SCRATCH | |
> + * +=======================================================+
> + * | 0 | 31:28 | message TYPE = 0xF = SUCCESS RESPONSE |
> + * | | 27:24 | message MAGIC |
> + * | | 23:0 | response DATA0 (depends on ACTION/SUBCODE)|
> + * +-------------------------------------------------------+
> + * | 1 | 31:0 | response DATA1 (depends on ACTION/SUBCODE)|
> + * |...| | response DATA2 (depends on ACTION/SUBCODE)|
> + * | n | 31:0 | response DATA3 (depends on ACTION/SUBCODE)|
> + * +=======================================================+
> + *
> + * Where:
> + * - **TYPE** must be set to value 0xF.
> + * - **MAGIC** must match value used by the host in **REQUEST** message.
> + * - **DATA** is optional payload data related to **ACTION**.
> + * Format is defined by each **ACTION** separately.
> + */
> +
> +#define GUC_MMIO_RESPONSE_DATA0 (0xFFFFFF << 0)
> +
> +/**
> + * DOC: GuC MMIO G2H ERROR RESPONSE
> + *
> + * The MMIO G2H ERROR RESPONSE message is used by the GuC to reply to
> + * the earlier H2G request message. This message is used if some errors
> + * were encountered by the GuC during processing of the request::
> + *
> + * +=======================================================+
> + * | SCRATCH | |
> + * +=======================================================+
> + * | 0 | 31:28 | message TYPE = 0xE = ERROR RESPONSE |
> + * | | 27:24 | message MAGIC |
> + * | | 23:16 | reserved MBZ |
> + * | | 15:0 | response STATUS failure code |
> + * +-------------------------------------------------------+
> + * | 1 | 31:0 | reserved MBZ |
> + * |...| | reserved MBZ |
> + * | n | 31:0 | reserved MBZ |
> + * +=======================================================+
> + *
> + * Where:
> + * - **TYPE** must be set to value 0xE.
> + * - **MAGIC** must match value used by the host in **REQUEST** message.
> + * - **STATUS** represents non-zero failure code.
> + */
> +
> +#define GUC_MMIO_ERROR_STATUS (0xFFFF << 0)
> +#define GUC_STATUS_UNKNOWN_ACTION 0x30
> +#define GUC_STATUS_INVALID_PARAMS 0x60
> +#define GUC_STATUS_INVALID_ADDR 0x80
> +#define GUC_STATUS_CTX_NOT_REGISTERED 0x100
> +#define GUC_STATUS_NO_LOCAL_MEMORY 0x200
> +#define GUC_STATUS_NO_VIRTUALIZATION 0x300
> +#define GUC_STATUS_CTB_FULL 0x301
> +#define GUC_STATUS_UNAUTHORIZED_REQUEST 0x302
> +#define GUC_STATUS_GENERIC_FAIL 0xF000
> +
> +/**
> + * DOC: MMIO G2H BUSY RESPONSE
> + *
> + * The MMIO G2H BUSY RESPONSE message is used by the GuC to reply to
> + * the earlier H2G request message. This message is used if processing
> + * of the request will take longer time and final SUCCESS/ERROR response
> + * message will be delivered later::
> + *
> + * +=======================================================+
> + * | SCRATCH | |
> + * +=======================================================+
> + * | 0 | 31:28 | message TYPE = 0xB = BUSY RESPONSE |
> + * | | 27:24 | message MAGIC |
> + * | | 23:0 | reserved MBZ |
> + * +-------------------------------------------------------+
> + * | 1 | 31:0 | reserved MBZ |
> + * |...| | reserved MBZ |
> + * | n | 31:0 | reserved MBZ |
> + * +=======================================================+
> + *
> + * Where:
> + * - **TYPE** must be set to value 0xB.
> + * - **MAGIC** must match value used by the host in **REQUEST** message.
> + * - all other bits are reserved as MBZ.
> + */
> +
> +#endif /* __INTEL_GUC_ABI_H__ */
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> index d44061033f23..e66cbf1be320 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
> @@ -10,11 +10,52 @@
>
> /*
> * The Additional Data Struct (ADS) has pointers for different buffers used by
> - * the GuC. One single gem object contains the ADS struct itself (guc_ads), the
> - * scheduling policies (guc_policies), a structure describing a collection of
> - * register sets (guc_mmio_reg_state) and some extra pages for the GuC to save
> - * its internal state for sleep.
> + * the GuC. One single gem object contains the ADS struct itself (guc_ads) and
> + * all the extra buffers indirectly linked via the ADS struct's entires.
> + *
> + * Layout of the ADS blob allocated for the GuC:
> + *
> + * +---------------------------------------+ <== base
> + * | guc_ads |
> + * +---------------------------------------+
> + * | guc_policies |
> + * +---------------------------------------+
> + * | guc_gt_system_info |
> + * +---------------------------------------+
> + * | guc_clients_info |
> + * +---------------------------------------+
> + * | guc_ct_pool_entry[size] |
> + * +---------------------------------------+
> + * | padding |
> + * +---------------------------------------+ <== 4K aligned
> + * | private data |
> + * +---------------------------------------+
> + * | padding |
> + * +---------------------------------------+ <== 4K aligned
> */
I think it we could expand the comment (or add a new one) to make clear
that the private data is not included in the __guc_ads_blob to keep
things simple since it is variable in size, but it is still part of the
ads_blobpasses to GuC.
> +struct __guc_ads_blob {
> + struct guc_ads ads;
> + struct guc_policies policies;
> + struct guc_gt_system_info system_info;
> + struct guc_clients_info clients_info;
> + struct guc_ct_pool_entry ct_pool[GUC_CT_POOL_SIZE];
> +} __packed;
> +
> +static u32 guc_ads_private_data_size(struct intel_guc *guc)
> +{
> + return PAGE_ALIGN(guc->fw.private_data_size);
> +}
> +
> +static u32 guc_ads_private_data_offset(struct intel_guc *guc)
> +{
> + return PAGE_ALIGN(sizeof(struct __guc_ads_blob));
> +}
> +
> +static u32 guc_ads_blob_size(struct intel_guc *guc)
> +{
> + return guc_ads_private_data_offset(guc) +
> + guc_ads_private_data_size(guc);
> +}
>
> static void guc_policy_init(struct guc_policy *policy)
> {
> @@ -48,23 +89,33 @@ static void guc_ct_pool_entries_init(struct guc_ct_pool_entry *pool, u32 num)
> memset(pool, 0, num * sizeof(*pool));
> }
>
> +static void guc_mapping_table_init(struct intel_gt *gt,
> + struct guc_gt_system_info *system_info)
> +{
> + unsigned int i, j;
> + struct intel_engine_cs *engine;
> + enum intel_engine_id id;
> +
> + /* Table must be set to invalid values for entries not used */
> + for (i = 0; i < GUC_MAX_ENGINE_CLASSES; ++i)
> + for (j = 0; j < GUC_MAX_INSTANCES_PER_CLASS; ++j)
> + system_info->mapping_table[i][j] =
> + GUC_MAX_INSTANCES_PER_CLASS;
> +
> + for_each_engine(engine, gt, id) {
> + u8 guc_class = engine->class;
> +
> + system_info->mapping_table[guc_class][engine->instance]
> + = engine->instance;
> + }
> +}
> +
> /*
> * The first 80 dwords of the register state context, containing the
> * execlists and ppgtt registers.
> */
> #define LR_HW_CONTEXT_SIZE (80 * sizeof(u32))
>
> -/* The ads obj includes the struct itself and buffers passed to GuC */
> -struct __guc_ads_blob {
> - struct guc_ads ads;
> - struct guc_policies policies;
> - struct guc_mmio_reg_state reg_state;
> - struct guc_gt_system_info system_info;
> - struct guc_clients_info clients_info;
> - struct guc_ct_pool_entry ct_pool[GUC_CT_POOL_SIZE];
> - u8 reg_state_buffer[GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE];
> -} __packed;
> -
> static void __guc_ads_init(struct intel_guc *guc)
> {
> struct intel_gt *gt = guc_to_gt(guc);
> @@ -107,6 +158,16 @@ static void __guc_ads_init(struct intel_guc *guc)
> blob->system_info.vebox_enable_mask = VEBOX_MASK(gt);
> blob->system_info.vdbox_sfc_support_mask = gt->info.vdbox_sfc_access;
>
> + if (INTEL_GEN(gt->i915) >= 12 && !IS_DGFX(gt->i915)) {
> + u32 distdbreg = intel_uncore_read(gt->uncore,
> + GEN12_DIST_DBS_POPULATED);
> + blob->system_info.num_of_doorbells_per_sqidi =
> + ((distdbreg >> GEN12_DOORBELLS_PER_SQIDI_SHIFT) &
> + GEN12_DOORBELLS_PER_SQIDI) + 1;
> + }
> +
> + guc_mapping_table_init(guc_to_gt(guc), &blob->system_info);
> +
> base = intel_guc_ggtt_offset(guc, guc->ads_vma);
>
> /* Clients info */
> @@ -118,11 +179,12 @@ static void __guc_ads_init(struct intel_guc *guc)
>
> /* ADS */
> blob->ads.scheduler_policies = base + ptr_offset(blob, policies);
> - blob->ads.reg_state_buffer = base + ptr_offset(blob, reg_state_buffer);
> - blob->ads.reg_state_addr = base + ptr_offset(blob, reg_state);
> blob->ads.gt_system_info = base + ptr_offset(blob, system_info);
> blob->ads.clients_info = base + ptr_offset(blob, clients_info);
>
> + /* Private Data */
> + blob->ads.private_data = base + guc_ads_private_data_offset(guc);
> +
> i915_gem_object_flush_map(guc->ads_vma->obj);
> }
>
> @@ -135,14 +197,15 @@ static void __guc_ads_init(struct intel_guc *guc)
> */
> int intel_guc_ads_create(struct intel_guc *guc)
> {
> - const u32 size = PAGE_ALIGN(sizeof(struct __guc_ads_blob));
> + u32 size;
> int ret;
>
> GEM_BUG_ON(guc->ads_vma);
>
> + size = guc_ads_blob_size(guc);
> +
> ret = intel_guc_allocate_and_map_vma(guc, size, &guc->ads_vma,
> (void **)&guc->ads_blob);
> -
> if (ret)
> return ret;
>
> @@ -156,6 +219,18 @@ void intel_guc_ads_destroy(struct intel_guc *guc)
> i915_vma_unpin_and_release(&guc->ads_vma, I915_VMA_RELEASE_MAP);
> }
>
> +void guc_ads_private_data_reset(struct intel_guc *guc)
> +{
> + u32 size;
> +
> + size = guc_ads_private_data_size(guc);
> + if (!size)
> + return;
> +
> + memset((void *)guc->ads_blob + guc_ads_private_data_offset(guc), 0,
> + size);
> +}
> +
> /**
> * intel_guc_ads_reset() - prepares GuC Additional Data Struct for reuse
> * @guc: intel_guc struct
> @@ -168,5 +243,8 @@ void intel_guc_ads_reset(struct intel_guc *guc)
> {
> if (!guc->ads_vma)
> return;
> +
> __guc_ads_init(guc);
> +
> + guc_ads_private_data_reset(guc);
> }
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> index d4a87f4c9421..212de0a939bf 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
> @@ -74,8 +74,9 @@ static inline bool guc_ready(struct intel_uncore *uncore, u32 *status)
> ((val & GS_MIA_CORE_STATE) && (uk_val == GS_UKERNEL_LAPIC_DONE));
> }
>
> -static int guc_wait_ucode(struct intel_uncore *uncore)
> +static int guc_wait_ucode(struct intel_gt *gt)
> {
> + struct intel_uncore *uncore = gt->uncore;
The gt seems to be used only for gt->i915, why not just use uncore->i915?
> u32 status;
> int ret;
>
> @@ -93,12 +94,24 @@ static int guc_wait_ucode(struct intel_uncore *uncore)
> if ((status & GS_BOOTROM_MASK) == GS_BOOTROM_RSA_FAILED) {
> DRM_ERROR("GuC firmware signature verification failed\n");
> ret = -ENOEXEC;
> - }
> -
> + } else
> if ((status & GS_UKERNEL_MASK) == GS_UKERNEL_EXCEPTION) {
> DRM_ERROR("GuC firmware exception. EIP: %#x\n",
> intel_uncore_read(uncore, SOFT_SCRATCH(13)));
> ret = -ENXIO;
> + } else
> + if (ret) {
> + i915_probe_error(gt->i915, "GuC load failed: status = 0x%08X\n",
> + status);
> + i915_probe_error(gt->i915, "GuC status: Reset = %d, "
> + "BootROM = 0x%02X, UKernel = 0x%02X, "
> + "MIA = 0x%02X, Auth = 0x%02X\n",
> + (status >> GS_RESET_SHIFT) & 1,
> + (status & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT,
> + (status & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT,
> + (status & GS_MIA_MASK) >> GS_MIA_SHIFT,
> + (status & GS_AUTH_STATUS_MASK) >>
> + GS_AUTH_STATUS_SHIFT);
nit: I think using the FIELD_GET macro here as well could make things
cleaner
> }
>
> return ret;
> @@ -139,7 +152,7 @@ int intel_guc_fw_upload(struct intel_guc *guc)
> if (ret)
> goto out;
>
> - ret = guc_wait_ucode(uncore);
> + ret = guc_wait_ucode(gt);
> if (ret)
> goto out;
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> index a6b733c146c9..77e49d53bb5c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> @@ -27,7 +27,7 @@
> #define GUC_MAX_ENGINES_NUM (GUC_VIDEO_ENGINE2 + 1)
>
> #define GUC_MAX_ENGINE_CLASSES 5
> -#define GUC_MAX_INSTANCES_PER_CLASS 16
> +#define GUC_MAX_INSTANCES_PER_CLASS 32
>
> #define GUC_DOORBELL_INVALID 256
>
> @@ -62,12 +62,7 @@
> #define GUC_STAGE_DESC_ATTR_PCH BIT(6)
> #define GUC_STAGE_DESC_ATTR_TERMINATED BIT(7)
>
> -/* New GuC control data */
> -#define GUC_CTL_CTXINFO 0
> -#define GUC_CTL_CTXNUM_IN16_SHIFT 0
> -#define GUC_CTL_BASE_ADDR_SHIFT 12
> -
> -#define GUC_CTL_LOG_PARAMS 1
> +#define GUC_CTL_LOG_PARAMS 0
> #define GUC_LOG_VALID (1 << 0)
> #define GUC_LOG_NOTIFY_ON_HALF_FULL (1 << 1)
> #define GUC_LOG_ALLOC_IN_MEGABYTE (1 << 3)
> @@ -79,11 +74,11 @@
> #define GUC_LOG_ISR_MASK (0x7 << GUC_LOG_ISR_SHIFT)
> #define GUC_LOG_BUF_ADDR_SHIFT 12
>
> -#define GUC_CTL_WA 2
> -#define GUC_CTL_FEATURE 3
> +#define GUC_CTL_WA 1
> +#define GUC_CTL_FEATURE 2
> #define GUC_CTL_DISABLE_SCHEDULER (1 << 14)
>
> -#define GUC_CTL_DEBUG 4
> +#define GUC_CTL_DEBUG 3
> #define GUC_LOG_VERBOSITY_SHIFT 0
> #define GUC_LOG_VERBOSITY_LOW (0 << GUC_LOG_VERBOSITY_SHIFT)
> #define GUC_LOG_VERBOSITY_MED (1 << GUC_LOG_VERBOSITY_SHIFT)
> @@ -97,12 +92,31 @@
> #define GUC_LOG_DISABLED (1 << 6)
> #define GUC_PROFILE_ENABLED (1 << 7)
>
> -#define GUC_CTL_ADS 5
> +#define GUC_CTL_ADS 4
> #define GUC_ADS_ADDR_SHIFT 1
> #define GUC_ADS_ADDR_MASK (0xFFFFF << GUC_ADS_ADDR_SHIFT)
>
> #define GUC_CTL_MAX_DWORDS (SOFT_SCRATCH_COUNT - 2) /* [1..14] */
>
> +/*
> + * The class goes in bits [0..2] of the GuC ID, the instance in bits [3..6].
> + * Bit 7 can be used for operations that apply to all engine classes&instances.
> + */
> +#define GUC_ENGINE_CLASS_SHIFT 0
> +#define GUC_ENGINE_CLASS_MASK (0x7 << GUC_ENGINE_CLASS_SHIFT)
> +#define GUC_ENGINE_INSTANCE_SHIFT 3
> +#define GUC_ENGINE_INSTANCE_MASK (0xf << GUC_ENGINE_INSTANCE_SHIFT)
> +#define GUC_ENGINE_ALL_INSTANCES (1 << 7)
> +
> +#define MAKE_GUC_ID(class, instance) \
> + (((class) << GUC_ENGINE_CLASS_SHIFT) | \
> + ((instance) << GUC_ENGINE_INSTANCE_SHIFT))
> +
> +#define GUC_ID_TO_ENGINE_CLASS(guc_id) \
> + (((guc_id) & GUC_ENGINE_CLASS_MASK) >> GUC_ENGINE_CLASS_SHIFT)
> +#define GUC_ID_TO_ENGINE_INSTANCE(guc_id) \
> + (((guc_id) & GUC_ENGINE_INSTANCE_MASK) >> GUC_ENGINE_INSTANCE_SHIFT)
> +
> /* Work item for submitting workloads into work queue of GuC. */
> struct guc_wq_item {
> u32 header;
> @@ -338,7 +352,6 @@ struct guc_policies {
> /* GuC MMIO reg state struct */
>
>
> -#define GUC_REGSET_MAX_REGISTERS 64
> #define GUC_S3_SAVE_SPACE_PAGES 10
>
> struct guc_mmio_reg {
> @@ -348,16 +361,17 @@ struct guc_mmio_reg {
> #define GUC_REGSET_MASKED (1 << 0)
> } __packed;
>
> -struct guc_mmio_regset {
> - struct guc_mmio_reg registers[GUC_REGSET_MAX_REGISTERS];
> - u32 values_valid;
> - u32 number_of_registers;
> -} __packed;
> -
> /* GuC register sets */
> -struct guc_mmio_reg_state {
> - struct guc_mmio_regset engine_reg[GUC_MAX_ENGINE_CLASSES][GUC_MAX_INSTANCES_PER_CLASS];
> - u32 reserved[98];
> +struct guc_mmio_reg_set {
> + u32 address;
> + union {
> + struct {
> + u32 count:16;
> + u32 reserved1:12;
> + u32 reserved2:4;
> + };
> + u32 count_u32;
> + };
> } __packed;
>
> /* HW info */
> @@ -369,7 +383,9 @@ struct guc_gt_system_info {
> u32 vdbox_enable_mask;
> u32 vdbox_sfc_support_mask;
> u32 vebox_enable_mask;
> - u32 reserved[9];
> + u32 num_of_doorbells_per_sqidi;
> + u8 mapping_table[GUC_MAX_ENGINE_CLASSES][GUC_MAX_INSTANCES_PER_CLASS];
> + u32 reserved2[8];
> } __packed;
>
> /* Clients info */
> @@ -390,15 +406,16 @@ struct guc_clients_info {
>
> /* GuC Additional Data Struct */
> struct guc_ads {
> - u32 reg_state_addr;
> - u32 reg_state_buffer;
> + struct guc_mmio_reg_set reg_state_list[GUC_MAX_ENGINE_CLASSES][GUC_MAX_INSTANCES_PER_CLASS];
> + u32 reserved0;
> u32 scheduler_policies;
> u32 gt_system_info;
> u32 clients_info;
> u32 control_data;
> u32 golden_context_lrca[GUC_MAX_ENGINE_CLASSES];
> u32 eng_state_size[GUC_MAX_ENGINE_CLASSES];
> - u32 reserved[16];
> + u32 private_data;
> + u32 reserved[15];
> } __packed;
>
> /* GuC logging structures */
> @@ -474,49 +491,6 @@ struct guc_shared_ctx_data {
> struct guc_ctx_report preempt_ctx_report[GUC_MAX_ENGINES_NUM];
> } __packed;
>
> -/**
> - * DOC: MMIO based communication
> - *
> - * The MMIO based communication between Host and GuC uses software scratch
> - * registers, where first register holds data treated as message header,
> - * and other registers are used to hold message payload.
> - *
> - * For Gen9+, GuC uses software scratch registers 0xC180-0xC1B8,
> - * but no H2G command takes more than 8 parameters and the GuC FW
> - * itself uses an 8-element array to store the H2G message.
> - *
> - * +-----------+---------+---------+---------+
> - * | MMIO[0] | MMIO[1] | ... | MMIO[n] |
> - * +-----------+---------+---------+---------+
> - * | header | optional payload |
> - * +======+====+=========+=========+=========+
> - * | 31:28|type| | | |
> - * +------+----+ | | |
> - * | 27:16|data| | | |
> - * +------+----+ | | |
> - * | 15:0|code| | | |
> - * +------+----+---------+---------+---------+
> - *
> - * The message header consists of:
> - *
> - * - **type**, indicates message type
> - * - **code**, indicates message code, is specific for **type**
> - * - **data**, indicates message data, optional, depends on **code**
> - *
> - * The following message **types** are supported:
> - *
> - * - **REQUEST**, indicates Host-to-GuC request, requested GuC action code
> - * must be priovided in **code** field. Optional action specific parameters
> - * can be provided in remaining payload registers or **data** field.
> - *
> - * - **RESPONSE**, indicates GuC-to-Host response from earlier GuC request,
> - * action response status will be provided in **code** field. Optional
> - * response data can be returned in remaining payload registers or **data**
> - * field.
> - */
> -
> -#define GUC_MAX_MMIO_MSG_LEN 8
> -
> #define INTEL_GUC_MSG_TYPE_SHIFT 28
> #define INTEL_GUC_MSG_TYPE_MASK (0xF << INTEL_GUC_MSG_TYPE_SHIFT)
> #define INTEL_GUC_MSG_DATA_SHIFT 16
These GUC_MSG defines are also duplicated
Daniele
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> index 1949346e714e..b37fc2ffaef2 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
> @@ -118,6 +118,11 @@ struct guc_doorbell_info {
> #define GEN8_DRB_VALID (1<<0)
> #define GEN8_DRBREGU(x) _MMIO(0x1000 + (x) * 8 + 4)
>
> +#define GEN12_DIST_DBS_POPULATED _MMIO(0xd08)
> +#define GEN12_DOORBELLS_PER_SQIDI_SHIFT 16
> +#define GEN12_DOORBELLS_PER_SQIDI (0xff)
> +#define GEN12_SQIDIS_DOORBELL_EXIST (0xffff)
> +
> #define DE_GUCRMR _MMIO(0x44054)
>
> #define GUC_BCS_RCS_IER _MMIO(0xC550)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> index 59b27aba15c6..f7cb0b1f1ba5 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
> @@ -44,23 +44,19 @@ void intel_uc_fw_change_status(struct intel_uc_fw *uc_fw,
> * List of required GuC and HuC binaries per-platform.
> * Must be ordered based on platform + revid, from newer to older.
> *
> - * TGL 35.2 is interface-compatible with 33.0 for previous Gens. The deltas
> - * between 33.0 and 35.2 are only related to new additions to support new Gen12
> - * features.
> - *
> * Note that RKL uses the same firmware as TGL.
> */
> #define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \
> - fw_def(ROCKETLAKE, 0, guc_def(tgl, 35, 2, 0), huc_def(tgl, 7, 0, 12)) \
> - fw_def(TIGERLAKE, 0, guc_def(tgl, 35, 2, 0), huc_def(tgl, 7, 0, 12)) \
> - fw_def(ELKHARTLAKE, 0, guc_def(ehl, 33, 0, 4), huc_def(ehl, 9, 0, 0)) \
> - fw_def(ICELAKE, 0, guc_def(icl, 33, 0, 0), huc_def(icl, 9, 0, 0)) \
> - fw_def(COMETLAKE, 5, guc_def(cml, 33, 0, 0), huc_def(cml, 4, 0, 0)) \
> - fw_def(COFFEELAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 4, 0, 0)) \
> - fw_def(GEMINILAKE, 0, guc_def(glk, 33, 0, 0), huc_def(glk, 4, 0, 0)) \
> - fw_def(KABYLAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 4, 0, 0)) \
> - fw_def(BROXTON, 0, guc_def(bxt, 33, 0, 0), huc_def(bxt, 2, 0, 0)) \
> - fw_def(SKYLAKE, 0, guc_def(skl, 33, 0, 0), huc_def(skl, 2, 0, 0))
> + fw_def(ROCKETLAKE, 0, guc_def(tgl, 45, 0, 0), huc_def(tgl, 7, 0, 12)) \
> + fw_def(TIGERLAKE, 0, guc_def(tgl, 45, 0, 0), huc_def(tgl, 7, 0, 12)) \
> + fw_def(ELKHARTLAKE, 0, guc_def(ehl, 45, 0, 0), huc_def(ehl, 9, 0, 0)) \
> + fw_def(ICELAKE, 0, guc_def(icl, 45, 0, 0), huc_def(icl, 9, 0, 0)) \
> + fw_def(COMETLAKE, 5, guc_def(cml, 45, 0, 0), huc_def(cml, 4, 0, 0)) \
> + fw_def(COFFEELAKE, 0, guc_def(kbl, 45, 0, 0), huc_def(kbl, 4, 0, 0)) \
> + fw_def(GEMINILAKE, 0, guc_def(glk, 45, 0, 0), huc_def(glk, 4, 0, 0)) \
> + fw_def(KABYLAKE, 0, guc_def(kbl, 45, 0, 0), huc_def(kbl, 4, 0, 0)) \
> + fw_def(BROXTON, 0, guc_def(bxt, 45, 0, 0), huc_def(bxt, 2, 0, 0)) \
> + fw_def(SKYLAKE, 0, guc_def(skl, 45, 0, 0), huc_def(skl, 2, 0, 0))
>
> #define __MAKE_UC_FW_PATH(prefix_, name_, major_, minor_, patch_) \
> "i915/" \
> @@ -371,6 +367,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
> }
> }
>
> + if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
> + uc_fw->private_data_size = css->private_data_size;
> +
> obj = i915_gem_object_create_shmem_from_data(i915, fw->data, fw->size);
> if (IS_ERR(obj)) {
> err = PTR_ERR(obj);
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> index 23d3a423ac0f..99bb1fe1af66 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
> @@ -88,6 +88,8 @@ struct intel_uc_fw {
>
> u32 rsa_size;
> u32 ucode_size;
> +
> + u32 private_data_size;
> };
>
> #ifdef CONFIG_DRM_I915_DEBUG_GUC
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> index 029214cdedd5..e41ffc7a7fbc 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
> @@ -69,7 +69,11 @@ struct uc_css_header {
> #define CSS_SW_VERSION_UC_MAJOR (0xFF << 16)
> #define CSS_SW_VERSION_UC_MINOR (0xFF << 8)
> #define CSS_SW_VERSION_UC_PATCH (0xFF << 0)
> - u32 reserved[14];
> + u32 reserved0[13];
> + union {
> + u32 private_data_size; /* only applies to GuC */
> + u32 reserved1;
> + };
> u32 header_info;
> } __packed;
> static_assert(sizeof(struct uc_css_header) == 128);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915/guc: Update to GuC v45.0.0
2020-08-11 1:04 ` Daniele Ceraolo Spurio
@ 2020-08-19 21:14 ` Michal Wajdeczko
2020-08-20 21:29 ` Daniele Ceraolo Spurio
0 siblings, 1 reply; 10+ messages in thread
From: Michal Wajdeczko @ 2020-08-19 21:14 UTC (permalink / raw)
To: Daniele Ceraolo Spurio, John.C.Harrison, Intel-GFX
On 11.08.2020 03:04, Daniele Ceraolo Spurio wrote:
>
>
> On 8/7/2020 10:46 AM, John.C.Harrison@Intel.com wrote:
>> From: John Harrison <John.C.Harrison@Intel.com>
>>
>> Update to the latest GuC firmware. This includes some significant
>> changes to the interface.
>
> A high level overview of the changes would be nice for extra clarity. A
> couple of example:
>
> - GuC now requires a private memory area
> - you're dropping the programming of the reg_state struct, but that's
> actually still defined in the structure, so IMO it's worth mentioning
> that it will come back with GuC submission.
>
>>
>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>> Author: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Author: John Harrison <John.C.Harrison@Intel.com>
>> Author: Matthew Brost <matthew.brost@intel.com>
>> Author: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Author: Michel Thierry <michel.thierry@intel.com>
>> Author: Oscar Mateo <oscar.mateo@intel.com>
>> Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
For the review process it would be better to have individual patches
from each author. We can squash them just before merge.
>> ---
>> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 3 +-
>> drivers/gpu/drm/i915/gt/uc/intel_guc.c | 125 +++++++----
>> drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h | 215 +++++++++++++++++++
>> drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 116 ++++++++--
>> drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 21 +-
>> drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 110 ++++------
>> drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 5 +
>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 27 ++-
>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 2 +
>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h | 6 +-
>> 10 files changed, 485 insertions(+), 145 deletions(-)
>> create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h
>>
>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> index ea4ba2afe9f9..9cd62d68abc6 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>> @@ -305,8 +305,9 @@ static int intel_engine_setup(struct intel_gt *gt,
>> enum intel_engine_id id)
>> engine->i915 = i915;
>> engine->gt = gt;
>> engine->uncore = gt->uncore;
>> - engine->hw_id = engine->guc_id = info->hw_id;
>> engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
>> + engine->hw_id = info->hw_id;
>> + engine->guc_id = MAKE_GUC_ID(info->class, info->instance);
>> engine->class = info->class;
>> engine->instance = info->instance;
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> index 861657897c0f..8d30e1d7d8a6 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> @@ -7,6 +7,7 @@
>> #include "gt/intel_gt_irq.h"
>> #include "gt/intel_gt_pm_irq.h"
>> #include "intel_guc.h"
>> +#include "intel_guc_abi.h"
>> #include "intel_guc_ads.h"
>> #include "intel_guc_submission.h"
>> #include "i915_drv.h"
>> @@ -213,23 +214,6 @@ static u32 guc_ctl_feature_flags(struct intel_guc
>> *guc)
>> return flags;
>> }
>> -static u32 guc_ctl_ctxinfo_flags(struct intel_guc *guc)
>> -{
>> - u32 flags = 0;
>> -
>> - if (intel_guc_submission_is_used(guc)) {
>> - u32 ctxnum, base;
>> -
>> - base = intel_guc_ggtt_offset(guc, guc->stage_desc_pool);
>> - ctxnum = GUC_MAX_STAGE_DESCRIPTORS / 16;
>> -
>> - base >>= PAGE_SHIFT;
>> - flags |= (base << GUC_CTL_BASE_ADDR_SHIFT) |
>> - (ctxnum << GUC_CTL_CTXNUM_IN16_SHIFT);
>> - }
>> - return flags;
>> -}
>> -
>> static u32 guc_ctl_log_params_flags(struct intel_guc *guc)
>> {
>> u32 offset = intel_guc_ggtt_offset(guc, guc->log.vma) >>
>> PAGE_SHIFT;
>> @@ -291,7 +275,6 @@ static void guc_init_params(struct intel_guc *guc)
>> BUILD_BUG_ON(sizeof(guc->params) != GUC_CTL_MAX_DWORDS *
>> sizeof(u32));
>> - params[GUC_CTL_CTXINFO] = guc_ctl_ctxinfo_flags(guc);
>> params[GUC_CTL_LOG_PARAMS] = guc_ctl_log_params_flags(guc);
>> params[GUC_CTL_FEATURE] = guc_ctl_feature_flags(guc);
>> params[GUC_CTL_DEBUG] = guc_ctl_debug_flags(guc);
>> @@ -400,32 +383,65 @@ void intel_guc_fini(struct intel_guc *guc)
>> intel_uc_fw_fini(&guc->fw);
>> }
>> +static bool is_valid_mmio_action(u32 action)
>> +{
>> + return action == GUC_ACTION_REGISTER_CTB ||
>> + action == GUC_ACTION_DEREGISTER_CTB;
>> +}
>> +
>> +static int guc_status_to_errno(u32 status)
>> +{
>> + switch (status) {
>> + case GUC_STATUS_UNKNOWN_ACTION:
>> + return -EOPNOTSUPP;
>> + case GUC_STATUS_INVALID_PARAMS:
>> + return -EINVAL;
>> + case GUC_STATUS_INVALID_ADDR:
>> + return -EFAULT;
>> + case GUC_STATUS_CTX_NOT_REGISTERED:
>> + return -ESRCH;
>> + case GUC_STATUS_CTB_FULL:
>> + return -ENOSPC;
>> + case GUC_STATUS_UNAUTHORIZED_REQUEST:
>> + return -EACCES;
>> + case GUC_STATUS_GENERIC_FAIL:
>> + default:
>> + return -ENXIO;
>> + }
>> +}
>> +
>> /*
>> * This function implements the MMIO based host to GuC interface.
>> */
>> -int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32
>> len,
>> +int intel_guc_send_mmio(struct intel_guc *guc, const u32 *request,
>> u32 len,
>> u32 *response_buf, u32 response_buf_size)
>> {
>> + struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
>> struct intel_uncore *uncore = guc_to_gt(guc)->uncore;
>> - u32 status;
>> + u32 action = FIELD_GET(GUC_MMIO_REQUEST_ACTION, *request);
>> + u32 subcode = FIELD_GET(GUC_MMIO_REQUEST_SUBCODE, *request);
>> + u32 magic = GUC_MMIO_MSG_MAGIC_DEFAULT;
>> + u32 header;
>> int i;
>> int ret;
>> GEM_BUG_ON(!len);
>> GEM_BUG_ON(len > guc->send_regs.count);
>> - /* We expect only action code */
>> - GEM_BUG_ON(*action & ~INTEL_GUC_MSG_CODE_MASK);
>> + /* We expect to use MMIO only for special actions */
>> + GEM_BUG_ON(!is_valid_mmio_action(action));
>> - /* If CT is available, we expect to use MMIO only during
>> init/fini */
>> - GEM_BUG_ON(*action !=
>> INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER &&
>> - *action !=
>> INTEL_GUC_ACTION_DEREGISTER_COMMAND_TRANSPORT_BUFFER);
>> + header = FIELD_PREP(GUC_MMIO_MSG_TYPE, GUC_MMIO_MSG_TYPE_REQUEST) |
>> + FIELD_PREP(GUC_MMIO_MSG_MAGIC, magic) |
>> + FIELD_PREP(GUC_MMIO_REQUEST_SUBCODE, subcode) |
>> + FIELD_PREP(GUC_MMIO_REQUEST_ACTION, action);
>> mutex_lock(&guc->send_mutex);
>> intel_uncore_forcewake_get(uncore, guc->send_regs.fw_domains);
>> - for (i = 0; i < len; i++)
>> - intel_uncore_write(uncore, guc_send_reg(guc, i), action[i]);
>> + intel_uncore_write(uncore, guc_send_reg(guc, 0), header);
>> + for (i = 1; i < len; i++)
>> + intel_uncore_write(uncore, guc_send_reg(guc, i), request[i]);
>> intel_uncore_posting_read(uncore, guc_send_reg(guc, i - 1));
>> @@ -437,17 +453,45 @@ int intel_guc_send_mmio(struct intel_guc *guc,
>> const u32 *action, u32 len,
>> */
>> ret = __intel_wait_for_register_fw(uncore,
>> guc_send_reg(guc, 0),
>> - INTEL_GUC_MSG_TYPE_MASK,
>> - INTEL_GUC_MSG_TYPE_RESPONSE <<
>> - INTEL_GUC_MSG_TYPE_SHIFT,
>> - 10, 10, &status);
>> - /* If GuC explicitly returned an error, convert it to -EIO */
>> - if (!ret && !INTEL_GUC_MSG_IS_RESPONSE_SUCCESS(status))
>> - ret = -EIO;
>> + GUC_MMIO_MSG_ORIGIN,
>> + FIELD_PREP(GUC_MMIO_MSG_ORIGIN,
>> + GUC_MMIO_MSG_ORIGIN_GUC),
>> + 10, 10, &header);
>> + if (unlikely(ret))
>> + goto out;
>> - if (ret) {
>> - DRM_ERROR("MMIO: GuC action %#x failed with error %d %#x\n",
>> - action[0], ret, status);
>> + if (unlikely(FIELD_GET(GUC_MMIO_MSG_MAGIC, header) != magic)) {
>> + ret = -ESTALE;
>> + goto out;
>> + }
>
> I think we should move this after the check for busyness. IIRC the
> protocol says we need to wait for the GuC to stop being busy before
> sending more messages. That way you can also unify this with the other
> similar check below.
but each MMIO message should be handled separately and part of that
handling is a check of the 'magic' field, so we can't skip it here
>
>> +
>> + if (FIELD_GET(GUC_MMIO_MSG_TYPE, header) ==
>> GUC_MMIO_MSG_TYPE_BUSY) {
>> +#define done ({ header = intel_uncore_read(uncore, guc_send_reg(guc,
>> 0)); \
>> + FIELD_GET(GUC_MMIO_MSG_TYPE, header) !=
>> GUC_MMIO_MSG_TYPE_BUSY; })
>> + ret = wait_for(done, 1000);
>> + if (unlikely(ret))
>> + goto out;
>> + if (unlikely(FIELD_GET(GUC_MMIO_MSG_ORIGIN, header) !=
>> + GUC_MMIO_MSG_ORIGIN_GUC)) {
>
> This check on GUC_MMIO_MSG_ORIGIN is redundant. We already waited for it
> to be GUC_MMIO_MSG_ORIGIN_GUC during the wait above and we're not
> sending other message from i915 while we're waiting for this one, so
> that field can't change.
in the wait above we are waiting for message different than BUSY and we
expect that this other message will be from the GuC (as this is part of
the protocol) but we should still check that GuC is compliant
>
>> + ret = -EPROTO;
>> + goto out;
>> + }
>> + if (unlikely(FIELD_GET(GUC_MMIO_MSG_MAGIC, header) != magic)) {
>> + ret = -ESTALE;
>> + goto out;
>> + }
>> +#undef done
>> + }
>> +
>> + if (FIELD_GET(GUC_MMIO_MSG_TYPE, header) ==
>> GUC_MMIO_MSG_TYPE_ERROR) {
>> + u32 status = FIELD_GET(GUC_MMIO_ERROR_STATUS, header);
>> +
>> + ret = guc_status_to_errno(status);
>> + goto out;
>> + }
>> +
>> + if (FIELD_GET(GUC_MMIO_MSG_TYPE, header) !=
>> GUC_MMIO_MSG_TYPE_RESPONSE) {
>> + ret = -EPROTO;
>> goto out;
>> }
>> @@ -460,12 +504,17 @@ int intel_guc_send_mmio(struct intel_guc *guc,
>> const u32 *action, u32 len,
>> }
>> /* Use data from the GuC response as our return value */
>> - ret = INTEL_GUC_MSG_TO_DATA(status);
>
> INTEL_GUC_MSG_TO_DATA is still used in intel_guc_ct.c. Either stick to
> that or update the other use-case to the new define.
this new define is for MMIO messages, changes to CTB messages will
follow soon
>
>> + ret = FIELD_GET(GUC_MMIO_RESPONSE_DATA0, header);
>> out:
>> intel_uncore_forcewake_put(uncore, guc->send_regs.fw_domains);
>> mutex_unlock(&guc->send_mutex);
>> + if (unlikely(ret < 0))
>> + drm_err(&i915->drm,
>> + "GuC MMIO action %#06x failed with error %d %#x\n",
>> + *request, ret, header);
>> +
>> return ret;
>> }
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h
>> new file mode 100644
>> index 000000000000..95043788e0ad
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h
>> @@ -0,0 +1,215 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2020 Intel Corporation
>> + */
>> +
>> +#ifndef __INTEL_GUC_ABI_H__
>> +#define __INTEL_GUC_ABI_H__
>> +
>> +/**
>> + * DOC: GuC MMIO based communication
>> + *
>> + * The MMIO based communication between Host and GuC relies on special
>> + * hardware registers which format could be defined by the software
>> + * (so called scratch registers).
>> + *
>> + * Each MMIO based message, both Host to GuC (H2G) and GuC to Host (G2H)
>> + * messages, which maximum length depends on number of available scratch
>> + * registers, is directly written into those scratch registers.
>> + *
>> + * For Gen9+, there are 16 software scratch registers 0xC180-0xC1B8,
>> + * but no H2G command takes more than 8 parameters and the GuC firmware
>> + * itself uses an 8-element array to store the H2G message.
>> + *
>> + * For Gen11+, there are additional 4 registers 0x190240-0x19024C, which
>> + * are, regardless on lower count, preffered over legacy ones.
>> + *
>> + * The MMIO based communication is mainly used during driver
>> initialization
>> + * phase to setup the CTB based communication that will be used
>> afterwards.
>> + *
>> + * Details of the messages formats depend on GuC firmware version
>> being used
>> + * by the host driver. Documented here messages are for GuC 45.0 and
>> newer.
>> + */
>> +
>> +#define GUC_MAX_MMIO_MSG_LEN 8
>
> Are we ok with not having an INTEL_ prefix to all the defines? I'm not
> complaining, we usually stick with that prefix but I'm not sure if it is
> an iron rule or not.
existing defines in intel_guc_fwif.h have "GUC" and "INTEL_GUC" (or
none) prefixes and at the same time almost all structs have "guc"
prefix, so the best option IMHO is to stick with just guc/GUC prefix.
>
>> +
>> +/**
>> + * DOC: GuC MMIO message format
>> + *
>> + * Bits of the first scratch register are treated as a message header,
>> + * and other registers values are used to hold message payload (data)::
>> + *
>> + * +=======================================================+
>> + * | SCRATCH | |
>> + * +-----------+-------------------------------------------+
>> + * | 0 | 31:28 | message ORIGIN/TYPE |
>> + * | | 27:24 | message MAGIC |
>> + * | | 23:0 | message DATA0 (depends on TYPE) |
>> + * +---+-------+-------------------------------------------+
>> + * | 1 | 31:0 | message DATA1 (depends on TYPE) |
>> + * |...| | message DATA2 (depends on TYPE) |
>> + * | n | | message DATA3 (depends on TYPE) |
>> + * +=======================================================+
>> + *
>> + * Where:
>> + * - **TYPE** is a 4b message type identifier.
>> + * - **MAGIC** is a 4b message sequence number.
>> + * The same sequence number must be included in all related messages.
>> + * This field is used for tracking and debug purposes.
>
> I don't think "magic" is a clear enough name for the role of the field.
> "Request id" or something like that would be more appropriate IMO.
we are following naming from GuC spec
>
>> + * - **DATA0..3** bits represents message payload.
>> + * Definitions of these bits depends on message **TYPE**.
>> + *
>> + * The MSB of the header and **TYPE** indicates origin of the message:
>> + * - 0 - message from the Host
>> + * - 1 - message from the GuC
>> + *
>> + * Currently supported message types are:
>> + * - 0x0 - **REQUEST** - represents Host request message to the GuC
>> + * - 0xF - **SUCCESS RESPONSE** - GuC reply for the earlier request
>> + * - 0xE - **ERROR RESPONSE** - GuC failure reply for the earlier
>> request
>> + * - 0xB - **BUSY** - GuC will be processing request for the longer
>> time
>> + */
>> +
>> +#define GUC_MMIO_MSG_ORIGIN (0x1 << 31)
>> +#define GUC_MMIO_MSG_ORIGIN_HOST 0u
>> +#define GUC_MMIO_MSG_ORIGIN_GUC 1u
>> +
>> +#define GUC_MMIO_MSG_TYPE (0xF << 28)
>> +#define GUC_MMIO_MSG_TYPE_REQUEST 0x0
>> +#define GUC_MMIO_MSG_TYPE_RESPONSE 0xF
>> +#define GUC_MMIO_MSG_TYPE_ERROR 0xE
>> +#define GUC_MMIO_MSG_TYPE_BUSY 0xB
>> +
>> +#define GUC_MMIO_MSG_MAGIC (0xF << 24)
>> +#define GUC_MMIO_MSG_MAGIC_DEFAULT 0u
>> +
>> +/**
>> + * DOC: GuC MMIO H2G REQUEST
>> + *
>> + * The MMIO H2G REQUEST message is used by the Host to request some
>> action
>> + * to be performed by the GuC::
>> + *
>> + * +=======================================================+
>> + * | SCRATCH | |
>> + * +=======================================================+
>> + * | 0 | 31:28 | message TYPE = 0x0 = REQUEST |
>> + * | | 27:24 | message MAGIC |
>> + * | | 23:16 | request SUBCODE (depends on ACTION) |
>> + * | | 15:0 | request ACTION code |
>> + * +-------------------------------------------------------+
>> + * | 1 | 31:0 | request DATA1 (depends on ACTION/SUBCODE) |
>> + * |...| | request DATA2 (depends on ACTION/SUBCODE) |
>> + * | n | 31:0 | request DATA3 (depends on ACTION/SUBCODE) |
>> + * +=======================================================+
>> + *
>> + * Where:
>> + * - **TYPE** must be set to value 0x0.
>> + * - **MAGIC** message sequence number is generated by the host.
>> + * - **ACTION** represents 16b request action code that defines both
>> + * purpose of the request and format of the passed payload data.
>> + * List of supported codes depends on GuC version and plaform.
>> + * Action code can't be zero.
>> + * - **SUBCODE** is optional 8b data related to the **ACTION**.
>> + * MBZ if not explicitly defined by given **ACTION**.
>> + * - **DATA1..3** are optional 32b payload dwords related to
>> **ACTION**.
>> + * Format of each dword is defined by specific **ACTION**.
>> + */
>> +
>> +#define GUC_MMIO_REQUEST_SUBCODE (0xFF << 16)
>> +#define GUC_MMIO_REQUEST_ACTION (0xFFFF << 0)
>> +#define GUC_ACTION_INVALID 0x0000
>> +#define GUC_ACTION_REGISTER_CTB 0x4505
>> +#define GUC_ACTION_DEREGISTER_CTB 0x4506
>
> These are now defined twice (once here and once in the enum). We should
> pick one for consistency.
we can drop old ones from intel_guc_fwif.h
>
>> +
>> +/**
>> + * DOC: GuC MMIO G2H SUCCESS RESPONSE
>> + *
>> + * The MMIO G2H SUCCESS RESPONSE message is used by the GuC to reply to
>> + * the earlier H2G request message. This message is used if no errors
>> + * were encountered by the GuC during processing of the request::
>> + *
>> + * +=======================================================+
>> + * | SCRATCH | |
>> + * +=======================================================+
>> + * | 0 | 31:28 | message TYPE = 0xF = SUCCESS RESPONSE |
>> + * | | 27:24 | message MAGIC |
>> + * | | 23:0 | response DATA0 (depends on ACTION/SUBCODE)|
>> + * +-------------------------------------------------------+
>> + * | 1 | 31:0 | response DATA1 (depends on ACTION/SUBCODE)|
>> + * |...| | response DATA2 (depends on ACTION/SUBCODE)|
>> + * | n | 31:0 | response DATA3 (depends on ACTION/SUBCODE)|
>> + * +=======================================================+
>> + *
>> + * Where:
>> + * - **TYPE** must be set to value 0xF.
>> + * - **MAGIC** must match value used by the host in **REQUEST**
>> message.
>> + * - **DATA** is optional payload data related to **ACTION**.
>> + * Format is defined by each **ACTION** separately.
>> + */
>> +
>> +#define GUC_MMIO_RESPONSE_DATA0 (0xFFFFFF << 0)
>> +
>> +/**
>> + * DOC: GuC MMIO G2H ERROR RESPONSE
>> + *
>> + * The MMIO G2H ERROR RESPONSE message is used by the GuC to reply to
>> + * the earlier H2G request message. This message is used if some errors
>> + * were encountered by the GuC during processing of the request::
>> + *
>> + * +=======================================================+
>> + * | SCRATCH | |
>> + * +=======================================================+
>> + * | 0 | 31:28 | message TYPE = 0xE = ERROR RESPONSE |
>> + * | | 27:24 | message MAGIC |
>> + * | | 23:16 | reserved MBZ |
>> + * | | 15:0 | response STATUS failure code |
>> + * +-------------------------------------------------------+
>> + * | 1 | 31:0 | reserved MBZ |
>> + * |...| | reserved MBZ |
>> + * | n | 31:0 | reserved MBZ |
>> + * +=======================================================+
>> + *
>> + * Where:
>> + * - **TYPE** must be set to value 0xE.
>> + * - **MAGIC** must match value used by the host in **REQUEST**
>> message.
>> + * - **STATUS** represents non-zero failure code.
>> + */
>> +
>> +#define GUC_MMIO_ERROR_STATUS (0xFFFF << 0)
>> +#define GUC_STATUS_UNKNOWN_ACTION 0x30
>> +#define GUC_STATUS_INVALID_PARAMS 0x60
>> +#define GUC_STATUS_INVALID_ADDR 0x80
>> +#define GUC_STATUS_CTX_NOT_REGISTERED 0x100
>> +#define GUC_STATUS_NO_LOCAL_MEMORY 0x200
>> +#define GUC_STATUS_NO_VIRTUALIZATION 0x300
>> +#define GUC_STATUS_CTB_FULL 0x301
>> +#define GUC_STATUS_UNAUTHORIZED_REQUEST 0x302
>> +#define GUC_STATUS_GENERIC_FAIL 0xF000
>> +
>> +/**
>> + * DOC: MMIO G2H BUSY RESPONSE
>> + *
>> + * The MMIO G2H BUSY RESPONSE message is used by the GuC to reply to
>> + * the earlier H2G request message. This message is used if processing
>> + * of the request will take longer time and final SUCCESS/ERROR response
>> + * message will be delivered later::
>> + *
>> + * +=======================================================+
>> + * | SCRATCH | |
>> + * +=======================================================+
>> + * | 0 | 31:28 | message TYPE = 0xB = BUSY RESPONSE |
>> + * | | 27:24 | message MAGIC |
>> + * | | 23:0 | reserved MBZ |
>> + * +-------------------------------------------------------+
>> + * | 1 | 31:0 | reserved MBZ |
>> + * |...| | reserved MBZ |
>> + * | n | 31:0 | reserved MBZ |
>> + * +=======================================================+
>> + *
>> + * Where:
>> + * - **TYPE** must be set to value 0xB.
>> + * - **MAGIC** must match value used by the host in **REQUEST**
>> message.
>> + * - all other bits are reserved as MBZ.
>> + */
>> +
>> +#endif /* __INTEL_GUC_ABI_H__ */
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
>> index d44061033f23..e66cbf1be320 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
>> @@ -10,11 +10,52 @@
>> /*
>> * The Additional Data Struct (ADS) has pointers for different
>> buffers used by
>> - * the GuC. One single gem object contains the ADS struct itself
>> (guc_ads), the
>> - * scheduling policies (guc_policies), a structure describing a
>> collection of
>> - * register sets (guc_mmio_reg_state) and some extra pages for the
>> GuC to save
>> - * its internal state for sleep.
>> + * the GuC. One single gem object contains the ADS struct itself
>> (guc_ads) and
>> + * all the extra buffers indirectly linked via the ADS struct's entires.
>> + *
>> + * Layout of the ADS blob allocated for the GuC:
>> + *
>> + * +---------------------------------------+ <== base
>> + * | guc_ads |
>> + * +---------------------------------------+
>> + * | guc_policies |
>> + * +---------------------------------------+
>> + * | guc_gt_system_info |
>> + * +---------------------------------------+
>> + * | guc_clients_info |
>> + * +---------------------------------------+
>> + * | guc_ct_pool_entry[size] |
>> + * +---------------------------------------+
>> + * | padding |
>> + * +---------------------------------------+ <== 4K aligned
>> + * | private data |
>> + * +---------------------------------------+
>> + * | padding |
>> + * +---------------------------------------+ <== 4K aligned
>> */
>
> I think it we could expand the comment (or add a new one) to make clear
> that the private data is not included in the __guc_ads_blob to keep
> things simple since it is variable in size, but it is still part of the
> ads_blobpasses to GuC.
>
>> +struct __guc_ads_blob {
>> + struct guc_ads ads;
>> + struct guc_policies policies;
>> + struct guc_gt_system_info system_info;
>> + struct guc_clients_info clients_info;
>> + struct guc_ct_pool_entry ct_pool[GUC_CT_POOL_SIZE];
>> +} __packed;
>> +
>> +static u32 guc_ads_private_data_size(struct intel_guc *guc)
>> +{
>> + return PAGE_ALIGN(guc->fw.private_data_size);
>> +}
>> +
>> +static u32 guc_ads_private_data_offset(struct intel_guc *guc)
>> +{
>> + return PAGE_ALIGN(sizeof(struct __guc_ads_blob));
>> +}
>> +
>> +static u32 guc_ads_blob_size(struct intel_guc *guc)
>> +{
>> + return guc_ads_private_data_offset(guc) +
>> + guc_ads_private_data_size(guc);
>> +}
>> static void guc_policy_init(struct guc_policy *policy)
>> {
>> @@ -48,23 +89,33 @@ static void guc_ct_pool_entries_init(struct
>> guc_ct_pool_entry *pool, u32 num)
>> memset(pool, 0, num * sizeof(*pool));
>> }
>> +static void guc_mapping_table_init(struct intel_gt *gt,
>> + struct guc_gt_system_info *system_info)
>> +{
>> + unsigned int i, j;
>> + struct intel_engine_cs *engine;
>> + enum intel_engine_id id;
>> +
>> + /* Table must be set to invalid values for entries not used */
>> + for (i = 0; i < GUC_MAX_ENGINE_CLASSES; ++i)
>> + for (j = 0; j < GUC_MAX_INSTANCES_PER_CLASS; ++j)
>> + system_info->mapping_table[i][j] =
>> + GUC_MAX_INSTANCES_PER_CLASS;
>> +
>> + for_each_engine(engine, gt, id) {
>> + u8 guc_class = engine->class;
>> +
>> + system_info->mapping_table[guc_class][engine->instance]
>> + = engine->instance;
>> + }
>> +}
>> +
>> /*
>> * The first 80 dwords of the register state context, containing the
>> * execlists and ppgtt registers.
>> */
>> #define LR_HW_CONTEXT_SIZE (80 * sizeof(u32))
>> -/* The ads obj includes the struct itself and buffers passed to GuC */
>> -struct __guc_ads_blob {
>> - struct guc_ads ads;
>> - struct guc_policies policies;
>> - struct guc_mmio_reg_state reg_state;
>> - struct guc_gt_system_info system_info;
>> - struct guc_clients_info clients_info;
>> - struct guc_ct_pool_entry ct_pool[GUC_CT_POOL_SIZE];
>> - u8 reg_state_buffer[GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE];
>> -} __packed;
>> -
>> static void __guc_ads_init(struct intel_guc *guc)
>> {
>> struct intel_gt *gt = guc_to_gt(guc);
>> @@ -107,6 +158,16 @@ static void __guc_ads_init(struct intel_guc *guc)
>> blob->system_info.vebox_enable_mask = VEBOX_MASK(gt);
>> blob->system_info.vdbox_sfc_support_mask =
>> gt->info.vdbox_sfc_access;
>> + if (INTEL_GEN(gt->i915) >= 12 && !IS_DGFX(gt->i915)) {
>> + u32 distdbreg = intel_uncore_read(gt->uncore,
>> + GEN12_DIST_DBS_POPULATED);
>> + blob->system_info.num_of_doorbells_per_sqidi =
>> + ((distdbreg >> GEN12_DOORBELLS_PER_SQIDI_SHIFT) &
>> + GEN12_DOORBELLS_PER_SQIDI) + 1;
>> + }
>> +
>> + guc_mapping_table_init(guc_to_gt(guc), &blob->system_info);
>> +
>> base = intel_guc_ggtt_offset(guc, guc->ads_vma);
>> /* Clients info */
>> @@ -118,11 +179,12 @@ static void __guc_ads_init(struct intel_guc *guc)
>> /* ADS */
>> blob->ads.scheduler_policies = base + ptr_offset(blob, policies);
>> - blob->ads.reg_state_buffer = base + ptr_offset(blob,
>> reg_state_buffer);
>> - blob->ads.reg_state_addr = base + ptr_offset(blob, reg_state);
>> blob->ads.gt_system_info = base + ptr_offset(blob, system_info);
>> blob->ads.clients_info = base + ptr_offset(blob, clients_info);
>> + /* Private Data */
>> + blob->ads.private_data = base + guc_ads_private_data_offset(guc);
>> +
>> i915_gem_object_flush_map(guc->ads_vma->obj);
>> }
>> @@ -135,14 +197,15 @@ static void __guc_ads_init(struct intel_guc *guc)
>> */
>> int intel_guc_ads_create(struct intel_guc *guc)
>> {
>> - const u32 size = PAGE_ALIGN(sizeof(struct __guc_ads_blob));
>> + u32 size;
>> int ret;
>> GEM_BUG_ON(guc->ads_vma);
>> + size = guc_ads_blob_size(guc);
>> +
>> ret = intel_guc_allocate_and_map_vma(guc, size, &guc->ads_vma,
>> (void **)&guc->ads_blob);
>> -
>> if (ret)
>> return ret;
>> @@ -156,6 +219,18 @@ void intel_guc_ads_destroy(struct intel_guc *guc)
>> i915_vma_unpin_and_release(&guc->ads_vma, I915_VMA_RELEASE_MAP);
>> }
>> +void guc_ads_private_data_reset(struct intel_guc *guc)
>> +{
>> + u32 size;
>> +
>> + size = guc_ads_private_data_size(guc);
>> + if (!size)
>> + return;
>> +
>> + memset((void *)guc->ads_blob + guc_ads_private_data_offset(guc), 0,
>> + size);
>> +}
>> +
>> /**
>> * intel_guc_ads_reset() - prepares GuC Additional Data Struct for
>> reuse
>> * @guc: intel_guc struct
>> @@ -168,5 +243,8 @@ void intel_guc_ads_reset(struct intel_guc *guc)
>> {
>> if (!guc->ads_vma)
>> return;
>> +
>> __guc_ads_init(guc);
>> +
>> + guc_ads_private_data_reset(guc);
>> }
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
>> index d4a87f4c9421..212de0a939bf 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
>> @@ -74,8 +74,9 @@ static inline bool guc_ready(struct intel_uncore
>> *uncore, u32 *status)
>> ((val & GS_MIA_CORE_STATE) && (uk_val ==
>> GS_UKERNEL_LAPIC_DONE));
>> }
>> -static int guc_wait_ucode(struct intel_uncore *uncore)
>> +static int guc_wait_ucode(struct intel_gt *gt)
>> {
>> + struct intel_uncore *uncore = gt->uncore;
>
> The gt seems to be used only for gt->i915, why not just use uncore->i915?
>
>> u32 status;
>> int ret;
>> @@ -93,12 +94,24 @@ static int guc_wait_ucode(struct intel_uncore
>> *uncore)
>> if ((status & GS_BOOTROM_MASK) == GS_BOOTROM_RSA_FAILED) {
>> DRM_ERROR("GuC firmware signature verification failed\n");
>> ret = -ENOEXEC;
>> - }
>> -
>> + } else
>> if ((status & GS_UKERNEL_MASK) == GS_UKERNEL_EXCEPTION) {
>> DRM_ERROR("GuC firmware exception. EIP: %#x\n",
>> intel_uncore_read(uncore, SOFT_SCRATCH(13)));
>> ret = -ENXIO;
>> + } else
>> + if (ret) {
>> + i915_probe_error(gt->i915, "GuC load failed: status = 0x%08X\n",
>> + status);
>> + i915_probe_error(gt->i915, "GuC status: Reset = %d, "
>> + "BootROM = 0x%02X, UKernel = 0x%02X, "
>> + "MIA = 0x%02X, Auth = 0x%02X\n",
>> + (status >> GS_RESET_SHIFT) & 1,
>> + (status & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT,
>> + (status & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT,
>> + (status & GS_MIA_MASK) >> GS_MIA_SHIFT,
>> + (status & GS_AUTH_STATUS_MASK) >>
>> + GS_AUTH_STATUS_SHIFT);
>
> nit: I think using the FIELD_GET macro here as well could make things
> cleaner
>
>> }
>> return ret;
>> @@ -139,7 +152,7 @@ int intel_guc_fw_upload(struct intel_guc *guc)
>> if (ret)
>> goto out;
>> - ret = guc_wait_ucode(uncore);
>> + ret = guc_wait_ucode(gt);
>> if (ret)
>> goto out;
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>> index a6b733c146c9..77e49d53bb5c 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>> @@ -27,7 +27,7 @@
>> #define GUC_MAX_ENGINES_NUM (GUC_VIDEO_ENGINE2 + 1)
>> #define GUC_MAX_ENGINE_CLASSES 5
>> -#define GUC_MAX_INSTANCES_PER_CLASS 16
>> +#define GUC_MAX_INSTANCES_PER_CLASS 32
>> #define GUC_DOORBELL_INVALID 256
>> @@ -62,12 +62,7 @@
>> #define GUC_STAGE_DESC_ATTR_PCH BIT(6)
>> #define GUC_STAGE_DESC_ATTR_TERMINATED BIT(7)
>> -/* New GuC control data */
>> -#define GUC_CTL_CTXINFO 0
>> -#define GUC_CTL_CTXNUM_IN16_SHIFT 0
>> -#define GUC_CTL_BASE_ADDR_SHIFT 12
>> -
>> -#define GUC_CTL_LOG_PARAMS 1
>> +#define GUC_CTL_LOG_PARAMS 0
>> #define GUC_LOG_VALID (1 << 0)
>> #define GUC_LOG_NOTIFY_ON_HALF_FULL (1 << 1)
>> #define GUC_LOG_ALLOC_IN_MEGABYTE (1 << 3)
>> @@ -79,11 +74,11 @@
>> #define GUC_LOG_ISR_MASK (0x7 << GUC_LOG_ISR_SHIFT)
>> #define GUC_LOG_BUF_ADDR_SHIFT 12
>> -#define GUC_CTL_WA 2
>> -#define GUC_CTL_FEATURE 3
>> +#define GUC_CTL_WA 1
>> +#define GUC_CTL_FEATURE 2
>> #define GUC_CTL_DISABLE_SCHEDULER (1 << 14)
>> -#define GUC_CTL_DEBUG 4
>> +#define GUC_CTL_DEBUG 3
>> #define GUC_LOG_VERBOSITY_SHIFT 0
>> #define GUC_LOG_VERBOSITY_LOW (0 << GUC_LOG_VERBOSITY_SHIFT)
>> #define GUC_LOG_VERBOSITY_MED (1 << GUC_LOG_VERBOSITY_SHIFT)
>> @@ -97,12 +92,31 @@
>> #define GUC_LOG_DISABLED (1 << 6)
>> #define GUC_PROFILE_ENABLED (1 << 7)
>> -#define GUC_CTL_ADS 5
>> +#define GUC_CTL_ADS 4
>> #define GUC_ADS_ADDR_SHIFT 1
>> #define GUC_ADS_ADDR_MASK (0xFFFFF << GUC_ADS_ADDR_SHIFT)
>> #define GUC_CTL_MAX_DWORDS (SOFT_SCRATCH_COUNT - 2) /*
>> [1..14] */
>> +/*
>> + * The class goes in bits [0..2] of the GuC ID, the instance in bits
>> [3..6].
>> + * Bit 7 can be used for operations that apply to all engine
>> classes&instances.
>> + */
>> +#define GUC_ENGINE_CLASS_SHIFT 0
>> +#define GUC_ENGINE_CLASS_MASK (0x7 << GUC_ENGINE_CLASS_SHIFT)
>> +#define GUC_ENGINE_INSTANCE_SHIFT 3
>> +#define GUC_ENGINE_INSTANCE_MASK (0xf << GUC_ENGINE_INSTANCE_SHIFT)
>> +#define GUC_ENGINE_ALL_INSTANCES (1 << 7)
>> +
>> +#define MAKE_GUC_ID(class, instance) \
>> + (((class) << GUC_ENGINE_CLASS_SHIFT) | \
>> + ((instance) << GUC_ENGINE_INSTANCE_SHIFT))
>> +
>> +#define GUC_ID_TO_ENGINE_CLASS(guc_id) \
>> + (((guc_id) & GUC_ENGINE_CLASS_MASK) >> GUC_ENGINE_CLASS_SHIFT)
>> +#define GUC_ID_TO_ENGINE_INSTANCE(guc_id) \
>> + (((guc_id) & GUC_ENGINE_INSTANCE_MASK) >> GUC_ENGINE_INSTANCE_SHIFT)
>> +
>> /* Work item for submitting workloads into work queue of GuC. */
>> struct guc_wq_item {
>> u32 header;
>> @@ -338,7 +352,6 @@ struct guc_policies {
>> /* GuC MMIO reg state struct */
>> -#define GUC_REGSET_MAX_REGISTERS 64
>> #define GUC_S3_SAVE_SPACE_PAGES 10
>> struct guc_mmio_reg {
>> @@ -348,16 +361,17 @@ struct guc_mmio_reg {
>> #define GUC_REGSET_MASKED (1 << 0)
>> } __packed;
>> -struct guc_mmio_regset {
>> - struct guc_mmio_reg registers[GUC_REGSET_MAX_REGISTERS];
>> - u32 values_valid;
>> - u32 number_of_registers;
>> -} __packed;
>> -
>> /* GuC register sets */
>> -struct guc_mmio_reg_state {
>> - struct guc_mmio_regset
>> engine_reg[GUC_MAX_ENGINE_CLASSES][GUC_MAX_INSTANCES_PER_CLASS];
>> - u32 reserved[98];
>> +struct guc_mmio_reg_set {
>> + u32 address;
>> + union {
>> + struct {
>> + u32 count:16;
>> + u32 reserved1:12;
>> + u32 reserved2:4;
>> + };
>> + u32 count_u32;
>> + };
>> } __packed;
>> /* HW info */
>> @@ -369,7 +383,9 @@ struct guc_gt_system_info {
>> u32 vdbox_enable_mask;
>> u32 vdbox_sfc_support_mask;
>> u32 vebox_enable_mask;
>> - u32 reserved[9];
>> + u32 num_of_doorbells_per_sqidi;
>> + u8
>> mapping_table[GUC_MAX_ENGINE_CLASSES][GUC_MAX_INSTANCES_PER_CLASS];
>> + u32 reserved2[8];
>> } __packed;
>> /* Clients info */
>> @@ -390,15 +406,16 @@ struct guc_clients_info {
>> /* GuC Additional Data Struct */
>> struct guc_ads {
>> - u32 reg_state_addr;
>> - u32 reg_state_buffer;
>> + struct guc_mmio_reg_set
>> reg_state_list[GUC_MAX_ENGINE_CLASSES][GUC_MAX_INSTANCES_PER_CLASS];
>> + u32 reserved0;
>> u32 scheduler_policies;
>> u32 gt_system_info;
>> u32 clients_info;
>> u32 control_data;
>> u32 golden_context_lrca[GUC_MAX_ENGINE_CLASSES];
>> u32 eng_state_size[GUC_MAX_ENGINE_CLASSES];
>> - u32 reserved[16];
>> + u32 private_data;
>> + u32 reserved[15];
>> } __packed;
>> /* GuC logging structures */
>> @@ -474,49 +491,6 @@ struct guc_shared_ctx_data {
>> struct guc_ctx_report preempt_ctx_report[GUC_MAX_ENGINES_NUM];
>> } __packed;
>> -/**
>> - * DOC: MMIO based communication
>> - *
>> - * The MMIO based communication between Host and GuC uses software
>> scratch
>> - * registers, where first register holds data treated as message header,
>> - * and other registers are used to hold message payload.
>> - *
>> - * For Gen9+, GuC uses software scratch registers 0xC180-0xC1B8,
>> - * but no H2G command takes more than 8 parameters and the GuC FW
>> - * itself uses an 8-element array to store the H2G message.
>> - *
>> - * +-----------+---------+---------+---------+
>> - * | MMIO[0] | MMIO[1] | ... | MMIO[n] |
>> - * +-----------+---------+---------+---------+
>> - * | header | optional payload |
>> - * +======+====+=========+=========+=========+
>> - * | 31:28|type| | | |
>> - * +------+----+ | | |
>> - * | 27:16|data| | | |
>> - * +------+----+ | | |
>> - * | 15:0|code| | | |
>> - * +------+----+---------+---------+---------+
>> - *
>> - * The message header consists of:
>> - *
>> - * - **type**, indicates message type
>> - * - **code**, indicates message code, is specific for **type**
>> - * - **data**, indicates message data, optional, depends on **code**
>> - *
>> - * The following message **types** are supported:
>> - *
>> - * - **REQUEST**, indicates Host-to-GuC request, requested GuC action
>> code
>> - * must be priovided in **code** field. Optional action specific
>> parameters
>> - * can be provided in remaining payload registers or **data** field.
>> - *
>> - * - **RESPONSE**, indicates GuC-to-Host response from earlier GuC
>> request,
>> - * action response status will be provided in **code** field. Optional
>> - * response data can be returned in remaining payload registers or
>> **data**
>> - * field.
>> - */
>> -
>> -#define GUC_MAX_MMIO_MSG_LEN 8
>> -
>> #define INTEL_GUC_MSG_TYPE_SHIFT 28
>> #define INTEL_GUC_MSG_TYPE_MASK (0xF <<
>> INTEL_GUC_MSG_TYPE_SHIFT)
>> #define INTEL_GUC_MSG_DATA_SHIFT 16
>
> These GUC_MSG defines are also duplicated
These are still used by the CTB and we want to keep CTB definitions
separate as we will do some refactoring there too.
Michal
>
> Daniele
>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
>> index 1949346e714e..b37fc2ffaef2 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
>> @@ -118,6 +118,11 @@ struct guc_doorbell_info {
>> #define GEN8_DRB_VALID (1<<0)
>> #define GEN8_DRBREGU(x) _MMIO(0x1000 + (x) * 8 + 4)
>> +#define GEN12_DIST_DBS_POPULATED _MMIO(0xd08)
>> +#define GEN12_DOORBELLS_PER_SQIDI_SHIFT 16
>> +#define GEN12_DOORBELLS_PER_SQIDI (0xff)
>> +#define GEN12_SQIDIS_DOORBELL_EXIST (0xffff)
>> +
>> #define DE_GUCRMR _MMIO(0x44054)
>> #define GUC_BCS_RCS_IER _MMIO(0xC550)
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> index 59b27aba15c6..f7cb0b1f1ba5 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>> @@ -44,23 +44,19 @@ void intel_uc_fw_change_status(struct intel_uc_fw
>> *uc_fw,
>> * List of required GuC and HuC binaries per-platform.
>> * Must be ordered based on platform + revid, from newer to older.
>> *
>> - * TGL 35.2 is interface-compatible with 33.0 for previous Gens. The
>> deltas
>> - * between 33.0 and 35.2 are only related to new additions to support
>> new Gen12
>> - * features.
>> - *
>> * Note that RKL uses the same firmware as TGL.
>> */
>> #define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \
>> - fw_def(ROCKETLAKE, 0, guc_def(tgl, 35, 2, 0), huc_def(tgl, 7,
>> 0, 12)) \
>> - fw_def(TIGERLAKE, 0, guc_def(tgl, 35, 2, 0), huc_def(tgl, 7,
>> 0, 12)) \
>> - fw_def(ELKHARTLAKE, 0, guc_def(ehl, 33, 0, 4), huc_def(ehl, 9,
>> 0, 0)) \
>> - fw_def(ICELAKE, 0, guc_def(icl, 33, 0, 0), huc_def(icl, 9,
>> 0, 0)) \
>> - fw_def(COMETLAKE, 5, guc_def(cml, 33, 0, 0), huc_def(cml, 4,
>> 0, 0)) \
>> - fw_def(COFFEELAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 4,
>> 0, 0)) \
>> - fw_def(GEMINILAKE, 0, guc_def(glk, 33, 0, 0), huc_def(glk, 4,
>> 0, 0)) \
>> - fw_def(KABYLAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 4,
>> 0, 0)) \
>> - fw_def(BROXTON, 0, guc_def(bxt, 33, 0, 0), huc_def(bxt, 2,
>> 0, 0)) \
>> - fw_def(SKYLAKE, 0, guc_def(skl, 33, 0, 0), huc_def(skl, 2,
>> 0, 0))
>> + fw_def(ROCKETLAKE, 0, guc_def(tgl, 45, 0, 0), huc_def(tgl, 7,
>> 0, 12)) \
>> + fw_def(TIGERLAKE, 0, guc_def(tgl, 45, 0, 0), huc_def(tgl, 7,
>> 0, 12)) \
>> + fw_def(ELKHARTLAKE, 0, guc_def(ehl, 45, 0, 0), huc_def(ehl, 9,
>> 0, 0)) \
>> + fw_def(ICELAKE, 0, guc_def(icl, 45, 0, 0), huc_def(icl, 9,
>> 0, 0)) \
>> + fw_def(COMETLAKE, 5, guc_def(cml, 45, 0, 0), huc_def(cml, 4,
>> 0, 0)) \
>> + fw_def(COFFEELAKE, 0, guc_def(kbl, 45, 0, 0), huc_def(kbl, 4,
>> 0, 0)) \
>> + fw_def(GEMINILAKE, 0, guc_def(glk, 45, 0, 0), huc_def(glk, 4,
>> 0, 0)) \
>> + fw_def(KABYLAKE, 0, guc_def(kbl, 45, 0, 0), huc_def(kbl, 4,
>> 0, 0)) \
>> + fw_def(BROXTON, 0, guc_def(bxt, 45, 0, 0), huc_def(bxt, 2,
>> 0, 0)) \
>> + fw_def(SKYLAKE, 0, guc_def(skl, 45, 0, 0), huc_def(skl, 2,
>> 0, 0))
>> #define __MAKE_UC_FW_PATH(prefix_, name_, major_, minor_, patch_) \
>> "i915/" \
>> @@ -371,6 +367,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>> }
>> }
>> + if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
>> + uc_fw->private_data_size = css->private_data_size;
>> +
>> obj = i915_gem_object_create_shmem_from_data(i915, fw->data,
>> fw->size);
>> if (IS_ERR(obj)) {
>> err = PTR_ERR(obj);
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
>> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
>> index 23d3a423ac0f..99bb1fe1af66 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
>> @@ -88,6 +88,8 @@ struct intel_uc_fw {
>> u32 rsa_size;
>> u32 ucode_size;
>> +
>> + u32 private_data_size;
>> };
>> #ifdef CONFIG_DRM_I915_DEBUG_GUC
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>> index 029214cdedd5..e41ffc7a7fbc 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>> @@ -69,7 +69,11 @@ struct uc_css_header {
>> #define CSS_SW_VERSION_UC_MAJOR (0xFF << 16)
>> #define CSS_SW_VERSION_UC_MINOR (0xFF << 8)
>> #define CSS_SW_VERSION_UC_PATCH (0xFF << 0)
>> - u32 reserved[14];
>> + u32 reserved0[13];
>> + union {
>> + u32 private_data_size; /* only applies to GuC */
>> + u32 reserved1;
>> + };
>> u32 header_info;
>> } __packed;
>> static_assert(sizeof(struct uc_css_header) == 128);
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Intel-gfx] [PATCH 1/2] drm/i915/guc: Update to GuC v45.0.0
2020-08-19 21:14 ` Michal Wajdeczko
@ 2020-08-20 21:29 ` Daniele Ceraolo Spurio
0 siblings, 0 replies; 10+ messages in thread
From: Daniele Ceraolo Spurio @ 2020-08-20 21:29 UTC (permalink / raw)
To: Michal Wajdeczko, John.C.Harrison, Intel-GFX
On 8/19/2020 2:14 PM, Michal Wajdeczko wrote:
>
> On 11.08.2020 03:04, Daniele Ceraolo Spurio wrote:
>>
>> On 8/7/2020 10:46 AM, John.C.Harrison@Intel.com wrote:
>>> From: John Harrison <John.C.Harrison@Intel.com>
>>>
>>> Update to the latest GuC firmware. This includes some significant
>>> changes to the interface.
>> A high level overview of the changes would be nice for extra clarity. A
>> couple of example:
>>
>> - GuC now requires a private memory area
>> - you're dropping the programming of the reg_state struct, but that's
>> actually still defined in the structure, so IMO it's worth mentioning
>> that it will come back with GuC submission.
>>
>>> Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
>>> Author: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>>> Author: John Harrison <John.C.Harrison@Intel.com>
>>> Author: Matthew Brost <matthew.brost@intel.com>
>>> Author: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Author: Michel Thierry <michel.thierry@intel.com>
>>> Author: Oscar Mateo <oscar.mateo@intel.com>
>>> Author: Rodrigo Vivi <rodrigo.vivi@intel.com>
> For the review process it would be better to have individual patches
> from each author. We can squash them just before merge.
>
>>> ---
>>> drivers/gpu/drm/i915/gt/intel_engine_cs.c | 3 +-
>>> drivers/gpu/drm/i915/gt/uc/intel_guc.c | 125 +++++++----
>>> drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h | 215 +++++++++++++++++++
>>> drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 116 ++++++++--
>>> drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 21 +-
>>> drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 110 ++++------
>>> drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h | 5 +
>>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 27 ++-
>>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h | 2 +
>>> drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h | 6 +-
>>> 10 files changed, 485 insertions(+), 145 deletions(-)
>>> create mode 100644 drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> index ea4ba2afe9f9..9cd62d68abc6 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
>>> @@ -305,8 +305,9 @@ static int intel_engine_setup(struct intel_gt *gt,
>>> enum intel_engine_id id)
>>> engine->i915 = i915;
>>> engine->gt = gt;
>>> engine->uncore = gt->uncore;
>>> - engine->hw_id = engine->guc_id = info->hw_id;
>>> engine->mmio_base = __engine_mmio_base(i915, info->mmio_bases);
>>> + engine->hw_id = info->hw_id;
>>> + engine->guc_id = MAKE_GUC_ID(info->class, info->instance);
>>> engine->class = info->class;
>>> engine->instance = info->instance;
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>>> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>>> index 861657897c0f..8d30e1d7d8a6 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>>> @@ -7,6 +7,7 @@
>>> #include "gt/intel_gt_irq.h"
>>> #include "gt/intel_gt_pm_irq.h"
>>> #include "intel_guc.h"
>>> +#include "intel_guc_abi.h"
>>> #include "intel_guc_ads.h"
>>> #include "intel_guc_submission.h"
>>> #include "i915_drv.h"
>>> @@ -213,23 +214,6 @@ static u32 guc_ctl_feature_flags(struct intel_guc
>>> *guc)
>>> return flags;
>>> }
>>> -static u32 guc_ctl_ctxinfo_flags(struct intel_guc *guc)
>>> -{
>>> - u32 flags = 0;
>>> -
>>> - if (intel_guc_submission_is_used(guc)) {
>>> - u32 ctxnum, base;
>>> -
>>> - base = intel_guc_ggtt_offset(guc, guc->stage_desc_pool);
>>> - ctxnum = GUC_MAX_STAGE_DESCRIPTORS / 16;
>>> -
>>> - base >>= PAGE_SHIFT;
>>> - flags |= (base << GUC_CTL_BASE_ADDR_SHIFT) |
>>> - (ctxnum << GUC_CTL_CTXNUM_IN16_SHIFT);
>>> - }
>>> - return flags;
>>> -}
>>> -
>>> static u32 guc_ctl_log_params_flags(struct intel_guc *guc)
>>> {
>>> u32 offset = intel_guc_ggtt_offset(guc, guc->log.vma) >>
>>> PAGE_SHIFT;
>>> @@ -291,7 +275,6 @@ static void guc_init_params(struct intel_guc *guc)
>>> BUILD_BUG_ON(sizeof(guc->params) != GUC_CTL_MAX_DWORDS *
>>> sizeof(u32));
>>> - params[GUC_CTL_CTXINFO] = guc_ctl_ctxinfo_flags(guc);
>>> params[GUC_CTL_LOG_PARAMS] = guc_ctl_log_params_flags(guc);
>>> params[GUC_CTL_FEATURE] = guc_ctl_feature_flags(guc);
>>> params[GUC_CTL_DEBUG] = guc_ctl_debug_flags(guc);
>>> @@ -400,32 +383,65 @@ void intel_guc_fini(struct intel_guc *guc)
>>> intel_uc_fw_fini(&guc->fw);
>>> }
>>> +static bool is_valid_mmio_action(u32 action)
>>> +{
>>> + return action == GUC_ACTION_REGISTER_CTB ||
>>> + action == GUC_ACTION_DEREGISTER_CTB;
>>> +}
>>> +
>>> +static int guc_status_to_errno(u32 status)
>>> +{
>>> + switch (status) {
>>> + case GUC_STATUS_UNKNOWN_ACTION:
>>> + return -EOPNOTSUPP;
>>> + case GUC_STATUS_INVALID_PARAMS:
>>> + return -EINVAL;
>>> + case GUC_STATUS_INVALID_ADDR:
>>> + return -EFAULT;
>>> + case GUC_STATUS_CTX_NOT_REGISTERED:
>>> + return -ESRCH;
>>> + case GUC_STATUS_CTB_FULL:
>>> + return -ENOSPC;
>>> + case GUC_STATUS_UNAUTHORIZED_REQUEST:
>>> + return -EACCES;
>>> + case GUC_STATUS_GENERIC_FAIL:
>>> + default:
>>> + return -ENXIO;
>>> + }
>>> +}
>>> +
>>> /*
>>> * This function implements the MMIO based host to GuC interface.
>>> */
>>> -int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32
>>> len,
>>> +int intel_guc_send_mmio(struct intel_guc *guc, const u32 *request,
>>> u32 len,
>>> u32 *response_buf, u32 response_buf_size)
>>> {
>>> + struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
>>> struct intel_uncore *uncore = guc_to_gt(guc)->uncore;
>>> - u32 status;
>>> + u32 action = FIELD_GET(GUC_MMIO_REQUEST_ACTION, *request);
>>> + u32 subcode = FIELD_GET(GUC_MMIO_REQUEST_SUBCODE, *request);
>>> + u32 magic = GUC_MMIO_MSG_MAGIC_DEFAULT;
>>> + u32 header;
>>> int i;
>>> int ret;
>>> GEM_BUG_ON(!len);
>>> GEM_BUG_ON(len > guc->send_regs.count);
>>> - /* We expect only action code */
>>> - GEM_BUG_ON(*action & ~INTEL_GUC_MSG_CODE_MASK);
>>> + /* We expect to use MMIO only for special actions */
>>> + GEM_BUG_ON(!is_valid_mmio_action(action));
>>> - /* If CT is available, we expect to use MMIO only during
>>> init/fini */
>>> - GEM_BUG_ON(*action !=
>>> INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER &&
>>> - *action !=
>>> INTEL_GUC_ACTION_DEREGISTER_COMMAND_TRANSPORT_BUFFER);
>>> + header = FIELD_PREP(GUC_MMIO_MSG_TYPE, GUC_MMIO_MSG_TYPE_REQUEST) |
>>> + FIELD_PREP(GUC_MMIO_MSG_MAGIC, magic) |
>>> + FIELD_PREP(GUC_MMIO_REQUEST_SUBCODE, subcode) |
>>> + FIELD_PREP(GUC_MMIO_REQUEST_ACTION, action);
>>> mutex_lock(&guc->send_mutex);
>>> intel_uncore_forcewake_get(uncore, guc->send_regs.fw_domains);
>>> - for (i = 0; i < len; i++)
>>> - intel_uncore_write(uncore, guc_send_reg(guc, i), action[i]);
>>> + intel_uncore_write(uncore, guc_send_reg(guc, 0), header);
>>> + for (i = 1; i < len; i++)
>>> + intel_uncore_write(uncore, guc_send_reg(guc, i), request[i]);
>>> intel_uncore_posting_read(uncore, guc_send_reg(guc, i - 1));
>>> @@ -437,17 +453,45 @@ int intel_guc_send_mmio(struct intel_guc *guc,
>>> const u32 *action, u32 len,
>>> */
>>> ret = __intel_wait_for_register_fw(uncore,
>>> guc_send_reg(guc, 0),
>>> - INTEL_GUC_MSG_TYPE_MASK,
>>> - INTEL_GUC_MSG_TYPE_RESPONSE <<
>>> - INTEL_GUC_MSG_TYPE_SHIFT,
>>> - 10, 10, &status);
>>> - /* If GuC explicitly returned an error, convert it to -EIO */
>>> - if (!ret && !INTEL_GUC_MSG_IS_RESPONSE_SUCCESS(status))
>>> - ret = -EIO;
>>> + GUC_MMIO_MSG_ORIGIN,
>>> + FIELD_PREP(GUC_MMIO_MSG_ORIGIN,
>>> + GUC_MMIO_MSG_ORIGIN_GUC),
>>> + 10, 10, &header);
>>> + if (unlikely(ret))
>>> + goto out;
>>> - if (ret) {
>>> - DRM_ERROR("MMIO: GuC action %#x failed with error %d %#x\n",
>>> - action[0], ret, status);
>>> + if (unlikely(FIELD_GET(GUC_MMIO_MSG_MAGIC, header) != magic)) {
>>> + ret = -ESTALE;
>>> + goto out;
>>> + }
>> I think we should move this after the check for busyness. IIRC the
>> protocol says we need to wait for the GuC to stop being busy before
>> sending more messages. That way you can also unify this with the other
>> similar check below.
> but each MMIO message should be handled separately and part of that
> handling is a check of the 'magic' field, so we can't skip it here
I wasn't suggesting to skip the magic check, just to do it after the
busyness check. If we exit this function while there is a busy message
there is a risk we could send another message while the GuC is still
busy with this message, which would likely lead to undefined behavior.
My suggestion is to do as follow:
wait_for(message is response);
if (MSG_TYPE == BUSY)
wait_for (MSG_TYPE != BUSY);
if (MSG_MAGIC != expected magic)
return error;
>>> +
>>> + if (FIELD_GET(GUC_MMIO_MSG_TYPE, header) ==
>>> GUC_MMIO_MSG_TYPE_BUSY) {
>>> +#define done ({ header = intel_uncore_read(uncore, guc_send_reg(guc,
>>> 0)); \
>>> + FIELD_GET(GUC_MMIO_MSG_TYPE, header) !=
>>> GUC_MMIO_MSG_TYPE_BUSY; })
>>> + ret = wait_for(done, 1000);
>>> + if (unlikely(ret))
>>> + goto out;
>>> + if (unlikely(FIELD_GET(GUC_MMIO_MSG_ORIGIN, header) !=
>>> + GUC_MMIO_MSG_ORIGIN_GUC)) {
>> This check on GUC_MMIO_MSG_ORIGIN is redundant. We already waited for it
>> to be GUC_MMIO_MSG_ORIGIN_GUC during the wait above and we're not
>> sending other message from i915 while we're waiting for this one, so
>> that field can't change.
> in the wait above we are waiting for message different than BUSY and we
> expect that this other message will be from the GuC (as this is part of
> the protocol) but we should still check that GuC is compliant
I think this is overly paranoid. Even in the very unlikely case that the
GuC marked its own messages as coming from the host by mistake, why
would we care? We are already sure this message is coming from the GuC,
no matter what the bit says. IMO if you want this kind of compliance
coverage it should be in a test, preferably on the GuC side of things.
>>> + ret = -EPROTO;
>>> + goto out;
>>> + }
>>> + if (unlikely(FIELD_GET(GUC_MMIO_MSG_MAGIC, header) != magic)) {
>>> + ret = -ESTALE;
>>> + goto out;
>>> + }
>>> +#undef done
>>> + }
>>> +
>>> + if (FIELD_GET(GUC_MMIO_MSG_TYPE, header) ==
>>> GUC_MMIO_MSG_TYPE_ERROR) {
>>> + u32 status = FIELD_GET(GUC_MMIO_ERROR_STATUS, header);
>>> +
>>> + ret = guc_status_to_errno(status);
>>> + goto out;
>>> + }
>>> +
>>> + if (FIELD_GET(GUC_MMIO_MSG_TYPE, header) !=
>>> GUC_MMIO_MSG_TYPE_RESPONSE) {
>>> + ret = -EPROTO;
>>> goto out;
>>> }
>>> @@ -460,12 +504,17 @@ int intel_guc_send_mmio(struct intel_guc *guc,
>>> const u32 *action, u32 len,
>>> }
>>> /* Use data from the GuC response as our return value */
>>> - ret = INTEL_GUC_MSG_TO_DATA(status);
>> INTEL_GUC_MSG_TO_DATA is still used in intel_guc_ct.c. Either stick to
>> that or update the other use-case to the new define.
> this new define is for MMIO messages, changes to CTB messages will
> follow soon
>
>>> + ret = FIELD_GET(GUC_MMIO_RESPONSE_DATA0, header);
>>> out:
>>> intel_uncore_forcewake_put(uncore, guc->send_regs.fw_domains);
>>> mutex_unlock(&guc->send_mutex);
>>> + if (unlikely(ret < 0))
>>> + drm_err(&i915->drm,
>>> + "GuC MMIO action %#06x failed with error %d %#x\n",
>>> + *request, ret, header);
>>> +
>>> return ret;
>>> }
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h
>>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h
>>> new file mode 100644
>>> index 000000000000..95043788e0ad
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_abi.h
>>> @@ -0,0 +1,215 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +/*
>>> + * Copyright © 2020 Intel Corporation
>>> + */
>>> +
>>> +#ifndef __INTEL_GUC_ABI_H__
>>> +#define __INTEL_GUC_ABI_H__
>>> +
>>> +/**
>>> + * DOC: GuC MMIO based communication
>>> + *
>>> + * The MMIO based communication between Host and GuC relies on special
>>> + * hardware registers which format could be defined by the software
>>> + * (so called scratch registers).
>>> + *
>>> + * Each MMIO based message, both Host to GuC (H2G) and GuC to Host (G2H)
>>> + * messages, which maximum length depends on number of available scratch
>>> + * registers, is directly written into those scratch registers.
>>> + *
>>> + * For Gen9+, there are 16 software scratch registers 0xC180-0xC1B8,
>>> + * but no H2G command takes more than 8 parameters and the GuC firmware
>>> + * itself uses an 8-element array to store the H2G message.
>>> + *
>>> + * For Gen11+, there are additional 4 registers 0x190240-0x19024C, which
>>> + * are, regardless on lower count, preffered over legacy ones.
>>> + *
>>> + * The MMIO based communication is mainly used during driver
>>> initialization
>>> + * phase to setup the CTB based communication that will be used
>>> afterwards.
>>> + *
>>> + * Details of the messages formats depend on GuC firmware version
>>> being used
>>> + * by the host driver. Documented here messages are for GuC 45.0 and
>>> newer.
>>> + */
>>> +
>>> +#define GUC_MAX_MMIO_MSG_LEN 8
>> Are we ok with not having an INTEL_ prefix to all the defines? I'm not
>> complaining, we usually stick with that prefix but I'm not sure if it is
>> an iron rule or not.
> existing defines in intel_guc_fwif.h have "GUC" and "INTEL_GUC" (or
> none) prefixes and at the same time almost all structs have "guc"
> prefix, so the best option IMHO is to stick with just guc/GUC prefix.
Ok for me, but we'll need to migrate the other defines as well so we're
consistent.
>>> +
>>> +/**
>>> + * DOC: GuC MMIO message format
>>> + *
>>> + * Bits of the first scratch register are treated as a message header,
>>> + * and other registers values are used to hold message payload (data)::
>>> + *
>>> + * +=======================================================+
>>> + * | SCRATCH | |
>>> + * +-----------+-------------------------------------------+
>>> + * | 0 | 31:28 | message ORIGIN/TYPE |
>>> + * | | 27:24 | message MAGIC |
>>> + * | | 23:0 | message DATA0 (depends on TYPE) |
>>> + * +---+-------+-------------------------------------------+
>>> + * | 1 | 31:0 | message DATA1 (depends on TYPE) |
>>> + * |...| | message DATA2 (depends on TYPE) |
>>> + * | n | | message DATA3 (depends on TYPE) |
>>> + * +=======================================================+
>>> + *
>>> + * Where:
>>> + * - **TYPE** is a 4b message type identifier.
>>> + * - **MAGIC** is a 4b message sequence number.
>>> + * The same sequence number must be included in all related messages.
>>> + * This field is used for tracking and debug purposes.
>> I don't think "magic" is a clear enough name for the role of the field.
>> "Request id" or something like that would be more appropriate IMO.
> we are following naming from GuC spec
>
>>> + * - **DATA0..3** bits represents message payload.
>>> + * Definitions of these bits depends on message **TYPE**.
>>> + *
>>> + * The MSB of the header and **TYPE** indicates origin of the message:
>>> + * - 0 - message from the Host
>>> + * - 1 - message from the GuC
>>> + *
>>> + * Currently supported message types are:
>>> + * - 0x0 - **REQUEST** - represents Host request message to the GuC
>>> + * - 0xF - **SUCCESS RESPONSE** - GuC reply for the earlier request
>>> + * - 0xE - **ERROR RESPONSE** - GuC failure reply for the earlier
>>> request
>>> + * - 0xB - **BUSY** - GuC will be processing request for the longer
>>> time
>>> + */
>>> +
>>> +#define GUC_MMIO_MSG_ORIGIN (0x1 << 31)
>>> +#define GUC_MMIO_MSG_ORIGIN_HOST 0u
>>> +#define GUC_MMIO_MSG_ORIGIN_GUC 1u
>>> +
>>> +#define GUC_MMIO_MSG_TYPE (0xF << 28)
>>> +#define GUC_MMIO_MSG_TYPE_REQUEST 0x0
>>> +#define GUC_MMIO_MSG_TYPE_RESPONSE 0xF
>>> +#define GUC_MMIO_MSG_TYPE_ERROR 0xE
>>> +#define GUC_MMIO_MSG_TYPE_BUSY 0xB
>>> +
>>> +#define GUC_MMIO_MSG_MAGIC (0xF << 24)
>>> +#define GUC_MMIO_MSG_MAGIC_DEFAULT 0u
>>> +
>>> +/**
>>> + * DOC: GuC MMIO H2G REQUEST
>>> + *
>>> + * The MMIO H2G REQUEST message is used by the Host to request some
>>> action
>>> + * to be performed by the GuC::
>>> + *
>>> + * +=======================================================+
>>> + * | SCRATCH | |
>>> + * +=======================================================+
>>> + * | 0 | 31:28 | message TYPE = 0x0 = REQUEST |
>>> + * | | 27:24 | message MAGIC |
>>> + * | | 23:16 | request SUBCODE (depends on ACTION) |
>>> + * | | 15:0 | request ACTION code |
>>> + * +-------------------------------------------------------+
>>> + * | 1 | 31:0 | request DATA1 (depends on ACTION/SUBCODE) |
>>> + * |...| | request DATA2 (depends on ACTION/SUBCODE) |
>>> + * | n | 31:0 | request DATA3 (depends on ACTION/SUBCODE) |
>>> + * +=======================================================+
>>> + *
>>> + * Where:
>>> + * - **TYPE** must be set to value 0x0.
>>> + * - **MAGIC** message sequence number is generated by the host.
>>> + * - **ACTION** represents 16b request action code that defines both
>>> + * purpose of the request and format of the passed payload data.
>>> + * List of supported codes depends on GuC version and plaform.
>>> + * Action code can't be zero.
>>> + * - **SUBCODE** is optional 8b data related to the **ACTION**.
>>> + * MBZ if not explicitly defined by given **ACTION**.
>>> + * - **DATA1..3** are optional 32b payload dwords related to
>>> **ACTION**.
>>> + * Format of each dword is defined by specific **ACTION**.
>>> + */
>>> +
>>> +#define GUC_MMIO_REQUEST_SUBCODE (0xFF << 16)
>>> +#define GUC_MMIO_REQUEST_ACTION (0xFFFF << 0)
>>> +#define GUC_ACTION_INVALID 0x0000
>>> +#define GUC_ACTION_REGISTER_CTB 0x4505
>>> +#define GUC_ACTION_DEREGISTER_CTB 0x4506
>> These are now defined twice (once here and once in the enum). We should
>> pick one for consistency.
> we can drop old ones from intel_guc_fwif.h
>
>>> +
>>> +/**
>>> + * DOC: GuC MMIO G2H SUCCESS RESPONSE
>>> + *
>>> + * The MMIO G2H SUCCESS RESPONSE message is used by the GuC to reply to
>>> + * the earlier H2G request message. This message is used if no errors
>>> + * were encountered by the GuC during processing of the request::
>>> + *
>>> + * +=======================================================+
>>> + * | SCRATCH | |
>>> + * +=======================================================+
>>> + * | 0 | 31:28 | message TYPE = 0xF = SUCCESS RESPONSE |
>>> + * | | 27:24 | message MAGIC |
>>> + * | | 23:0 | response DATA0 (depends on ACTION/SUBCODE)|
>>> + * +-------------------------------------------------------+
>>> + * | 1 | 31:0 | response DATA1 (depends on ACTION/SUBCODE)|
>>> + * |...| | response DATA2 (depends on ACTION/SUBCODE)|
>>> + * | n | 31:0 | response DATA3 (depends on ACTION/SUBCODE)|
>>> + * +=======================================================+
>>> + *
>>> + * Where:
>>> + * - **TYPE** must be set to value 0xF.
>>> + * - **MAGIC** must match value used by the host in **REQUEST**
>>> message.
>>> + * - **DATA** is optional payload data related to **ACTION**.
>>> + * Format is defined by each **ACTION** separately.
>>> + */
>>> +
>>> +#define GUC_MMIO_RESPONSE_DATA0 (0xFFFFFF << 0)
>>> +
>>> +/**
>>> + * DOC: GuC MMIO G2H ERROR RESPONSE
>>> + *
>>> + * The MMIO G2H ERROR RESPONSE message is used by the GuC to reply to
>>> + * the earlier H2G request message. This message is used if some errors
>>> + * were encountered by the GuC during processing of the request::
>>> + *
>>> + * +=======================================================+
>>> + * | SCRATCH | |
>>> + * +=======================================================+
>>> + * | 0 | 31:28 | message TYPE = 0xE = ERROR RESPONSE |
>>> + * | | 27:24 | message MAGIC |
>>> + * | | 23:16 | reserved MBZ |
>>> + * | | 15:0 | response STATUS failure code |
>>> + * +-------------------------------------------------------+
>>> + * | 1 | 31:0 | reserved MBZ |
>>> + * |...| | reserved MBZ |
>>> + * | n | 31:0 | reserved MBZ |
>>> + * +=======================================================+
>>> + *
>>> + * Where:
>>> + * - **TYPE** must be set to value 0xE.
>>> + * - **MAGIC** must match value used by the host in **REQUEST**
>>> message.
>>> + * - **STATUS** represents non-zero failure code.
>>> + */
>>> +
>>> +#define GUC_MMIO_ERROR_STATUS (0xFFFF << 0)
>>> +#define GUC_STATUS_UNKNOWN_ACTION 0x30
>>> +#define GUC_STATUS_INVALID_PARAMS 0x60
>>> +#define GUC_STATUS_INVALID_ADDR 0x80
>>> +#define GUC_STATUS_CTX_NOT_REGISTERED 0x100
>>> +#define GUC_STATUS_NO_LOCAL_MEMORY 0x200
>>> +#define GUC_STATUS_NO_VIRTUALIZATION 0x300
>>> +#define GUC_STATUS_CTB_FULL 0x301
>>> +#define GUC_STATUS_UNAUTHORIZED_REQUEST 0x302
>>> +#define GUC_STATUS_GENERIC_FAIL 0xF000
>>> +
>>> +/**
>>> + * DOC: MMIO G2H BUSY RESPONSE
>>> + *
>>> + * The MMIO G2H BUSY RESPONSE message is used by the GuC to reply to
>>> + * the earlier H2G request message. This message is used if processing
>>> + * of the request will take longer time and final SUCCESS/ERROR response
>>> + * message will be delivered later::
>>> + *
>>> + * +=======================================================+
>>> + * | SCRATCH | |
>>> + * +=======================================================+
>>> + * | 0 | 31:28 | message TYPE = 0xB = BUSY RESPONSE |
>>> + * | | 27:24 | message MAGIC |
>>> + * | | 23:0 | reserved MBZ |
>>> + * +-------------------------------------------------------+
>>> + * | 1 | 31:0 | reserved MBZ |
>>> + * |...| | reserved MBZ |
>>> + * | n | 31:0 | reserved MBZ |
>>> + * +=======================================================+
>>> + *
>>> + * Where:
>>> + * - **TYPE** must be set to value 0xB.
>>> + * - **MAGIC** must match value used by the host in **REQUEST**
>>> message.
>>> + * - all other bits are reserved as MBZ.
>>> + */
>>> +
>>> +#endif /* __INTEL_GUC_ABI_H__ */
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
>>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
>>> index d44061033f23..e66cbf1be320 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
>>> @@ -10,11 +10,52 @@
>>> /*
>>> * The Additional Data Struct (ADS) has pointers for different
>>> buffers used by
>>> - * the GuC. One single gem object contains the ADS struct itself
>>> (guc_ads), the
>>> - * scheduling policies (guc_policies), a structure describing a
>>> collection of
>>> - * register sets (guc_mmio_reg_state) and some extra pages for the
>>> GuC to save
>>> - * its internal state for sleep.
>>> + * the GuC. One single gem object contains the ADS struct itself
>>> (guc_ads) and
>>> + * all the extra buffers indirectly linked via the ADS struct's entires.
>>> + *
>>> + * Layout of the ADS blob allocated for the GuC:
>>> + *
>>> + * +---------------------------------------+ <== base
>>> + * | guc_ads |
>>> + * +---------------------------------------+
>>> + * | guc_policies |
>>> + * +---------------------------------------+
>>> + * | guc_gt_system_info |
>>> + * +---------------------------------------+
>>> + * | guc_clients_info |
>>> + * +---------------------------------------+
>>> + * | guc_ct_pool_entry[size] |
>>> + * +---------------------------------------+
>>> + * | padding |
>>> + * +---------------------------------------+ <== 4K aligned
>>> + * | private data |
>>> + * +---------------------------------------+
>>> + * | padding |
>>> + * +---------------------------------------+ <== 4K aligned
>>> */
>> I think it we could expand the comment (or add a new one) to make clear
>> that the private data is not included in the __guc_ads_blob to keep
>> things simple since it is variable in size, but it is still part of the
>> ads_blobpasses to GuC.
>>
>>> +struct __guc_ads_blob {
>>> + struct guc_ads ads;
>>> + struct guc_policies policies;
>>> + struct guc_gt_system_info system_info;
>>> + struct guc_clients_info clients_info;
>>> + struct guc_ct_pool_entry ct_pool[GUC_CT_POOL_SIZE];
>>> +} __packed;
>>> +
>>> +static u32 guc_ads_private_data_size(struct intel_guc *guc)
>>> +{
>>> + return PAGE_ALIGN(guc->fw.private_data_size);
>>> +}
>>> +
>>> +static u32 guc_ads_private_data_offset(struct intel_guc *guc)
>>> +{
>>> + return PAGE_ALIGN(sizeof(struct __guc_ads_blob));
>>> +}
>>> +
>>> +static u32 guc_ads_blob_size(struct intel_guc *guc)
>>> +{
>>> + return guc_ads_private_data_offset(guc) +
>>> + guc_ads_private_data_size(guc);
>>> +}
>>> static void guc_policy_init(struct guc_policy *policy)
>>> {
>>> @@ -48,23 +89,33 @@ static void guc_ct_pool_entries_init(struct
>>> guc_ct_pool_entry *pool, u32 num)
>>> memset(pool, 0, num * sizeof(*pool));
>>> }
>>> +static void guc_mapping_table_init(struct intel_gt *gt,
>>> + struct guc_gt_system_info *system_info)
>>> +{
>>> + unsigned int i, j;
>>> + struct intel_engine_cs *engine;
>>> + enum intel_engine_id id;
>>> +
>>> + /* Table must be set to invalid values for entries not used */
>>> + for (i = 0; i < GUC_MAX_ENGINE_CLASSES; ++i)
>>> + for (j = 0; j < GUC_MAX_INSTANCES_PER_CLASS; ++j)
>>> + system_info->mapping_table[i][j] =
>>> + GUC_MAX_INSTANCES_PER_CLASS;
>>> +
>>> + for_each_engine(engine, gt, id) {
>>> + u8 guc_class = engine->class;
>>> +
>>> + system_info->mapping_table[guc_class][engine->instance]
>>> + = engine->instance;
>>> + }
>>> +}
>>> +
>>> /*
>>> * The first 80 dwords of the register state context, containing the
>>> * execlists and ppgtt registers.
>>> */
>>> #define LR_HW_CONTEXT_SIZE (80 * sizeof(u32))
>>> -/* The ads obj includes the struct itself and buffers passed to GuC */
>>> -struct __guc_ads_blob {
>>> - struct guc_ads ads;
>>> - struct guc_policies policies;
>>> - struct guc_mmio_reg_state reg_state;
>>> - struct guc_gt_system_info system_info;
>>> - struct guc_clients_info clients_info;
>>> - struct guc_ct_pool_entry ct_pool[GUC_CT_POOL_SIZE];
>>> - u8 reg_state_buffer[GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE];
>>> -} __packed;
>>> -
>>> static void __guc_ads_init(struct intel_guc *guc)
>>> {
>>> struct intel_gt *gt = guc_to_gt(guc);
>>> @@ -107,6 +158,16 @@ static void __guc_ads_init(struct intel_guc *guc)
>>> blob->system_info.vebox_enable_mask = VEBOX_MASK(gt);
>>> blob->system_info.vdbox_sfc_support_mask =
>>> gt->info.vdbox_sfc_access;
>>> + if (INTEL_GEN(gt->i915) >= 12 && !IS_DGFX(gt->i915)) {
>>> + u32 distdbreg = intel_uncore_read(gt->uncore,
>>> + GEN12_DIST_DBS_POPULATED);
>>> + blob->system_info.num_of_doorbells_per_sqidi =
>>> + ((distdbreg >> GEN12_DOORBELLS_PER_SQIDI_SHIFT) &
>>> + GEN12_DOORBELLS_PER_SQIDI) + 1;
>>> + }
>>> +
>>> + guc_mapping_table_init(guc_to_gt(guc), &blob->system_info);
>>> +
>>> base = intel_guc_ggtt_offset(guc, guc->ads_vma);
>>> /* Clients info */
>>> @@ -118,11 +179,12 @@ static void __guc_ads_init(struct intel_guc *guc)
>>> /* ADS */
>>> blob->ads.scheduler_policies = base + ptr_offset(blob, policies);
>>> - blob->ads.reg_state_buffer = base + ptr_offset(blob,
>>> reg_state_buffer);
>>> - blob->ads.reg_state_addr = base + ptr_offset(blob, reg_state);
>>> blob->ads.gt_system_info = base + ptr_offset(blob, system_info);
>>> blob->ads.clients_info = base + ptr_offset(blob, clients_info);
>>> + /* Private Data */
>>> + blob->ads.private_data = base + guc_ads_private_data_offset(guc);
>>> +
>>> i915_gem_object_flush_map(guc->ads_vma->obj);
>>> }
>>> @@ -135,14 +197,15 @@ static void __guc_ads_init(struct intel_guc *guc)
>>> */
>>> int intel_guc_ads_create(struct intel_guc *guc)
>>> {
>>> - const u32 size = PAGE_ALIGN(sizeof(struct __guc_ads_blob));
>>> + u32 size;
>>> int ret;
>>> GEM_BUG_ON(guc->ads_vma);
>>> + size = guc_ads_blob_size(guc);
>>> +
>>> ret = intel_guc_allocate_and_map_vma(guc, size, &guc->ads_vma,
>>> (void **)&guc->ads_blob);
>>> -
>>> if (ret)
>>> return ret;
>>> @@ -156,6 +219,18 @@ void intel_guc_ads_destroy(struct intel_guc *guc)
>>> i915_vma_unpin_and_release(&guc->ads_vma, I915_VMA_RELEASE_MAP);
>>> }
>>> +void guc_ads_private_data_reset(struct intel_guc *guc)
>>> +{
>>> + u32 size;
>>> +
>>> + size = guc_ads_private_data_size(guc);
>>> + if (!size)
>>> + return;
>>> +
>>> + memset((void *)guc->ads_blob + guc_ads_private_data_offset(guc), 0,
>>> + size);
>>> +}
>>> +
>>> /**
>>> * intel_guc_ads_reset() - prepares GuC Additional Data Struct for
>>> reuse
>>> * @guc: intel_guc struct
>>> @@ -168,5 +243,8 @@ void intel_guc_ads_reset(struct intel_guc *guc)
>>> {
>>> if (!guc->ads_vma)
>>> return;
>>> +
>>> __guc_ads_init(guc);
>>> +
>>> + guc_ads_private_data_reset(guc);
>>> }
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
>>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
>>> index d4a87f4c9421..212de0a939bf 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c
>>> @@ -74,8 +74,9 @@ static inline bool guc_ready(struct intel_uncore
>>> *uncore, u32 *status)
>>> ((val & GS_MIA_CORE_STATE) && (uk_val ==
>>> GS_UKERNEL_LAPIC_DONE));
>>> }
>>> -static int guc_wait_ucode(struct intel_uncore *uncore)
>>> +static int guc_wait_ucode(struct intel_gt *gt)
>>> {
>>> + struct intel_uncore *uncore = gt->uncore;
>> The gt seems to be used only for gt->i915, why not just use uncore->i915?
>>
>>> u32 status;
>>> int ret;
>>> @@ -93,12 +94,24 @@ static int guc_wait_ucode(struct intel_uncore
>>> *uncore)
>>> if ((status & GS_BOOTROM_MASK) == GS_BOOTROM_RSA_FAILED) {
>>> DRM_ERROR("GuC firmware signature verification failed\n");
>>> ret = -ENOEXEC;
>>> - }
>>> -
>>> + } else
>>> if ((status & GS_UKERNEL_MASK) == GS_UKERNEL_EXCEPTION) {
>>> DRM_ERROR("GuC firmware exception. EIP: %#x\n",
>>> intel_uncore_read(uncore, SOFT_SCRATCH(13)));
>>> ret = -ENXIO;
>>> + } else
>>> + if (ret) {
>>> + i915_probe_error(gt->i915, "GuC load failed: status = 0x%08X\n",
>>> + status);
>>> + i915_probe_error(gt->i915, "GuC status: Reset = %d, "
>>> + "BootROM = 0x%02X, UKernel = 0x%02X, "
>>> + "MIA = 0x%02X, Auth = 0x%02X\n",
>>> + (status >> GS_RESET_SHIFT) & 1,
>>> + (status & GS_BOOTROM_MASK) >> GS_BOOTROM_SHIFT,
>>> + (status & GS_UKERNEL_MASK) >> GS_UKERNEL_SHIFT,
>>> + (status & GS_MIA_MASK) >> GS_MIA_SHIFT,
>>> + (status & GS_AUTH_STATUS_MASK) >>
>>> + GS_AUTH_STATUS_SHIFT);
>> nit: I think using the FIELD_GET macro here as well could make things
>> cleaner
>>
>>> }
>>> return ret;
>>> @@ -139,7 +152,7 @@ int intel_guc_fw_upload(struct intel_guc *guc)
>>> if (ret)
>>> goto out;
>>> - ret = guc_wait_ucode(uncore);
>>> + ret = guc_wait_ucode(gt);
>>> if (ret)
>>> goto out;
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>>> index a6b733c146c9..77e49d53bb5c 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
>>> @@ -27,7 +27,7 @@
>>> #define GUC_MAX_ENGINES_NUM (GUC_VIDEO_ENGINE2 + 1)
>>> #define GUC_MAX_ENGINE_CLASSES 5
>>> -#define GUC_MAX_INSTANCES_PER_CLASS 16
>>> +#define GUC_MAX_INSTANCES_PER_CLASS 32
>>> #define GUC_DOORBELL_INVALID 256
>>> @@ -62,12 +62,7 @@
>>> #define GUC_STAGE_DESC_ATTR_PCH BIT(6)
>>> #define GUC_STAGE_DESC_ATTR_TERMINATED BIT(7)
>>> -/* New GuC control data */
>>> -#define GUC_CTL_CTXINFO 0
>>> -#define GUC_CTL_CTXNUM_IN16_SHIFT 0
>>> -#define GUC_CTL_BASE_ADDR_SHIFT 12
>>> -
>>> -#define GUC_CTL_LOG_PARAMS 1
>>> +#define GUC_CTL_LOG_PARAMS 0
>>> #define GUC_LOG_VALID (1 << 0)
>>> #define GUC_LOG_NOTIFY_ON_HALF_FULL (1 << 1)
>>> #define GUC_LOG_ALLOC_IN_MEGABYTE (1 << 3)
>>> @@ -79,11 +74,11 @@
>>> #define GUC_LOG_ISR_MASK (0x7 << GUC_LOG_ISR_SHIFT)
>>> #define GUC_LOG_BUF_ADDR_SHIFT 12
>>> -#define GUC_CTL_WA 2
>>> -#define GUC_CTL_FEATURE 3
>>> +#define GUC_CTL_WA 1
>>> +#define GUC_CTL_FEATURE 2
>>> #define GUC_CTL_DISABLE_SCHEDULER (1 << 14)
>>> -#define GUC_CTL_DEBUG 4
>>> +#define GUC_CTL_DEBUG 3
>>> #define GUC_LOG_VERBOSITY_SHIFT 0
>>> #define GUC_LOG_VERBOSITY_LOW (0 << GUC_LOG_VERBOSITY_SHIFT)
>>> #define GUC_LOG_VERBOSITY_MED (1 << GUC_LOG_VERBOSITY_SHIFT)
>>> @@ -97,12 +92,31 @@
>>> #define GUC_LOG_DISABLED (1 << 6)
>>> #define GUC_PROFILE_ENABLED (1 << 7)
>>> -#define GUC_CTL_ADS 5
>>> +#define GUC_CTL_ADS 4
>>> #define GUC_ADS_ADDR_SHIFT 1
>>> #define GUC_ADS_ADDR_MASK (0xFFFFF << GUC_ADS_ADDR_SHIFT)
>>> #define GUC_CTL_MAX_DWORDS (SOFT_SCRATCH_COUNT - 2) /*
>>> [1..14] */
>>> +/*
>>> + * The class goes in bits [0..2] of the GuC ID, the instance in bits
>>> [3..6].
>>> + * Bit 7 can be used for operations that apply to all engine
>>> classes&instances.
>>> + */
>>> +#define GUC_ENGINE_CLASS_SHIFT 0
>>> +#define GUC_ENGINE_CLASS_MASK (0x7 << GUC_ENGINE_CLASS_SHIFT)
>>> +#define GUC_ENGINE_INSTANCE_SHIFT 3
>>> +#define GUC_ENGINE_INSTANCE_MASK (0xf << GUC_ENGINE_INSTANCE_SHIFT)
>>> +#define GUC_ENGINE_ALL_INSTANCES (1 << 7)
>>> +
>>> +#define MAKE_GUC_ID(class, instance) \
>>> + (((class) << GUC_ENGINE_CLASS_SHIFT) | \
>>> + ((instance) << GUC_ENGINE_INSTANCE_SHIFT))
>>> +
>>> +#define GUC_ID_TO_ENGINE_CLASS(guc_id) \
>>> + (((guc_id) & GUC_ENGINE_CLASS_MASK) >> GUC_ENGINE_CLASS_SHIFT)
>>> +#define GUC_ID_TO_ENGINE_INSTANCE(guc_id) \
>>> + (((guc_id) & GUC_ENGINE_INSTANCE_MASK) >> GUC_ENGINE_INSTANCE_SHIFT)
>>> +
>>> /* Work item for submitting workloads into work queue of GuC. */
>>> struct guc_wq_item {
>>> u32 header;
>>> @@ -338,7 +352,6 @@ struct guc_policies {
>>> /* GuC MMIO reg state struct */
>>> -#define GUC_REGSET_MAX_REGISTERS 64
>>> #define GUC_S3_SAVE_SPACE_PAGES 10
>>> struct guc_mmio_reg {
>>> @@ -348,16 +361,17 @@ struct guc_mmio_reg {
>>> #define GUC_REGSET_MASKED (1 << 0)
>>> } __packed;
>>> -struct guc_mmio_regset {
>>> - struct guc_mmio_reg registers[GUC_REGSET_MAX_REGISTERS];
>>> - u32 values_valid;
>>> - u32 number_of_registers;
>>> -} __packed;
>>> -
>>> /* GuC register sets */
>>> -struct guc_mmio_reg_state {
>>> - struct guc_mmio_regset
>>> engine_reg[GUC_MAX_ENGINE_CLASSES][GUC_MAX_INSTANCES_PER_CLASS];
>>> - u32 reserved[98];
>>> +struct guc_mmio_reg_set {
>>> + u32 address;
>>> + union {
>>> + struct {
>>> + u32 count:16;
>>> + u32 reserved1:12;
>>> + u32 reserved2:4;
>>> + };
>>> + u32 count_u32;
>>> + };
>>> } __packed;
>>> /* HW info */
>>> @@ -369,7 +383,9 @@ struct guc_gt_system_info {
>>> u32 vdbox_enable_mask;
>>> u32 vdbox_sfc_support_mask;
>>> u32 vebox_enable_mask;
>>> - u32 reserved[9];
>>> + u32 num_of_doorbells_per_sqidi;
>>> + u8
>>> mapping_table[GUC_MAX_ENGINE_CLASSES][GUC_MAX_INSTANCES_PER_CLASS];
>>> + u32 reserved2[8];
>>> } __packed;
>>> /* Clients info */
>>> @@ -390,15 +406,16 @@ struct guc_clients_info {
>>> /* GuC Additional Data Struct */
>>> struct guc_ads {
>>> - u32 reg_state_addr;
>>> - u32 reg_state_buffer;
>>> + struct guc_mmio_reg_set
>>> reg_state_list[GUC_MAX_ENGINE_CLASSES][GUC_MAX_INSTANCES_PER_CLASS];
>>> + u32 reserved0;
>>> u32 scheduler_policies;
>>> u32 gt_system_info;
>>> u32 clients_info;
>>> u32 control_data;
>>> u32 golden_context_lrca[GUC_MAX_ENGINE_CLASSES];
>>> u32 eng_state_size[GUC_MAX_ENGINE_CLASSES];
>>> - u32 reserved[16];
>>> + u32 private_data;
>>> + u32 reserved[15];
>>> } __packed;
>>> /* GuC logging structures */
>>> @@ -474,49 +491,6 @@ struct guc_shared_ctx_data {
>>> struct guc_ctx_report preempt_ctx_report[GUC_MAX_ENGINES_NUM];
>>> } __packed;
>>> -/**
>>> - * DOC: MMIO based communication
>>> - *
>>> - * The MMIO based communication between Host and GuC uses software
>>> scratch
>>> - * registers, where first register holds data treated as message header,
>>> - * and other registers are used to hold message payload.
>>> - *
>>> - * For Gen9+, GuC uses software scratch registers 0xC180-0xC1B8,
>>> - * but no H2G command takes more than 8 parameters and the GuC FW
>>> - * itself uses an 8-element array to store the H2G message.
>>> - *
>>> - * +-----------+---------+---------+---------+
>>> - * | MMIO[0] | MMIO[1] | ... | MMIO[n] |
>>> - * +-----------+---------+---------+---------+
>>> - * | header | optional payload |
>>> - * +======+====+=========+=========+=========+
>>> - * | 31:28|type| | | |
>>> - * +------+----+ | | |
>>> - * | 27:16|data| | | |
>>> - * +------+----+ | | |
>>> - * | 15:0|code| | | |
>>> - * +------+----+---------+---------+---------+
>>> - *
>>> - * The message header consists of:
>>> - *
>>> - * - **type**, indicates message type
>>> - * - **code**, indicates message code, is specific for **type**
>>> - * - **data**, indicates message data, optional, depends on **code**
>>> - *
>>> - * The following message **types** are supported:
>>> - *
>>> - * - **REQUEST**, indicates Host-to-GuC request, requested GuC action
>>> code
>>> - * must be priovided in **code** field. Optional action specific
>>> parameters
>>> - * can be provided in remaining payload registers or **data** field.
>>> - *
>>> - * - **RESPONSE**, indicates GuC-to-Host response from earlier GuC
>>> request,
>>> - * action response status will be provided in **code** field. Optional
>>> - * response data can be returned in remaining payload registers or
>>> **data**
>>> - * field.
>>> - */
>>> -
>>> -#define GUC_MAX_MMIO_MSG_LEN 8
>>> -
>>> #define INTEL_GUC_MSG_TYPE_SHIFT 28
>>> #define INTEL_GUC_MSG_TYPE_MASK (0xF <<
>>> INTEL_GUC_MSG_TYPE_SHIFT)
>>> #define INTEL_GUC_MSG_DATA_SHIFT 16
>> These GUC_MSG defines are also duplicated
> These are still used by the CTB and we want to keep CTB definitions
> separate as we will do some refactoring there too.
Is the GuC changing those defines for the CTB in a future release, or is
the refactoring purely on the i915 side? in the latter case I'd prefer
for all the duplication to be removed now until the CTB refactoring comes.
Daniele
> Michal
>
>> Daniele
>>
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
>>> b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
>>> index 1949346e714e..b37fc2ffaef2 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_reg.h
>>> @@ -118,6 +118,11 @@ struct guc_doorbell_info {
>>> #define GEN8_DRB_VALID (1<<0)
>>> #define GEN8_DRBREGU(x) _MMIO(0x1000 + (x) * 8 + 4)
>>> +#define GEN12_DIST_DBS_POPULATED _MMIO(0xd08)
>>> +#define GEN12_DOORBELLS_PER_SQIDI_SHIFT 16
>>> +#define GEN12_DOORBELLS_PER_SQIDI (0xff)
>>> +#define GEN12_SQIDIS_DOORBELL_EXIST (0xffff)
>>> +
>>> #define DE_GUCRMR _MMIO(0x44054)
>>> #define GUC_BCS_RCS_IER _MMIO(0xC550)
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> index 59b27aba15c6..f7cb0b1f1ba5 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
>>> @@ -44,23 +44,19 @@ void intel_uc_fw_change_status(struct intel_uc_fw
>>> *uc_fw,
>>> * List of required GuC and HuC binaries per-platform.
>>> * Must be ordered based on platform + revid, from newer to older.
>>> *
>>> - * TGL 35.2 is interface-compatible with 33.0 for previous Gens. The
>>> deltas
>>> - * between 33.0 and 35.2 are only related to new additions to support
>>> new Gen12
>>> - * features.
>>> - *
>>> * Note that RKL uses the same firmware as TGL.
>>> */
>>> #define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \
>>> - fw_def(ROCKETLAKE, 0, guc_def(tgl, 35, 2, 0), huc_def(tgl, 7,
>>> 0, 12)) \
>>> - fw_def(TIGERLAKE, 0, guc_def(tgl, 35, 2, 0), huc_def(tgl, 7,
>>> 0, 12)) \
>>> - fw_def(ELKHARTLAKE, 0, guc_def(ehl, 33, 0, 4), huc_def(ehl, 9,
>>> 0, 0)) \
>>> - fw_def(ICELAKE, 0, guc_def(icl, 33, 0, 0), huc_def(icl, 9,
>>> 0, 0)) \
>>> - fw_def(COMETLAKE, 5, guc_def(cml, 33, 0, 0), huc_def(cml, 4,
>>> 0, 0)) \
>>> - fw_def(COFFEELAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 4,
>>> 0, 0)) \
>>> - fw_def(GEMINILAKE, 0, guc_def(glk, 33, 0, 0), huc_def(glk, 4,
>>> 0, 0)) \
>>> - fw_def(KABYLAKE, 0, guc_def(kbl, 33, 0, 0), huc_def(kbl, 4,
>>> 0, 0)) \
>>> - fw_def(BROXTON, 0, guc_def(bxt, 33, 0, 0), huc_def(bxt, 2,
>>> 0, 0)) \
>>> - fw_def(SKYLAKE, 0, guc_def(skl, 33, 0, 0), huc_def(skl, 2,
>>> 0, 0))
>>> + fw_def(ROCKETLAKE, 0, guc_def(tgl, 45, 0, 0), huc_def(tgl, 7,
>>> 0, 12)) \
>>> + fw_def(TIGERLAKE, 0, guc_def(tgl, 45, 0, 0), huc_def(tgl, 7,
>>> 0, 12)) \
>>> + fw_def(ELKHARTLAKE, 0, guc_def(ehl, 45, 0, 0), huc_def(ehl, 9,
>>> 0, 0)) \
>>> + fw_def(ICELAKE, 0, guc_def(icl, 45, 0, 0), huc_def(icl, 9,
>>> 0, 0)) \
>>> + fw_def(COMETLAKE, 5, guc_def(cml, 45, 0, 0), huc_def(cml, 4,
>>> 0, 0)) \
>>> + fw_def(COFFEELAKE, 0, guc_def(kbl, 45, 0, 0), huc_def(kbl, 4,
>>> 0, 0)) \
>>> + fw_def(GEMINILAKE, 0, guc_def(glk, 45, 0, 0), huc_def(glk, 4,
>>> 0, 0)) \
>>> + fw_def(KABYLAKE, 0, guc_def(kbl, 45, 0, 0), huc_def(kbl, 4,
>>> 0, 0)) \
>>> + fw_def(BROXTON, 0, guc_def(bxt, 45, 0, 0), huc_def(bxt, 2,
>>> 0, 0)) \
>>> + fw_def(SKYLAKE, 0, guc_def(skl, 45, 0, 0), huc_def(skl, 2,
>>> 0, 0))
>>> #define __MAKE_UC_FW_PATH(prefix_, name_, major_, minor_, patch_) \
>>> "i915/" \
>>> @@ -371,6 +367,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
>>> }
>>> }
>>> + if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
>>> + uc_fw->private_data_size = css->private_data_size;
>>> +
>>> obj = i915_gem_object_create_shmem_from_data(i915, fw->data,
>>> fw->size);
>>> if (IS_ERR(obj)) {
>>> err = PTR_ERR(obj);
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
>>> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
>>> index 23d3a423ac0f..99bb1fe1af66 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
>>> @@ -88,6 +88,8 @@ struct intel_uc_fw {
>>> u32 rsa_size;
>>> u32 ucode_size;
>>> +
>>> + u32 private_data_size;
>>> };
>>> #ifdef CONFIG_DRM_I915_DEBUG_GUC
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>>> b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>>> index 029214cdedd5..e41ffc7a7fbc 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
>>> @@ -69,7 +69,11 @@ struct uc_css_header {
>>> #define CSS_SW_VERSION_UC_MAJOR (0xFF << 16)
>>> #define CSS_SW_VERSION_UC_MINOR (0xFF << 8)
>>> #define CSS_SW_VERSION_UC_PATCH (0xFF << 0)
>>> - u32 reserved[14];
>>> + u32 reserved0[13];
>>> + union {
>>> + u32 private_data_size; /* only applies to GuC */
>>> + u32 reserved1;
>>> + };
>>> u32 header_info;
>>> } __packed;
>>> static_assert(sizeof(struct uc_css_header) == 128);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2020-08-20 21:29 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-08-07 17:46 [Intel-gfx] [PATCH 0/2] drm/i915/guc: Update to GuC v45 John.C.Harrison
2020-08-07 17:46 ` [Intel-gfx] [PATCH 1/2] drm/i915/guc: Update to GuC v45.0.0 John.C.Harrison
2020-08-11 1:04 ` Daniele Ceraolo Spurio
2020-08-19 21:14 ` Michal Wajdeczko
2020-08-20 21:29 ` Daniele Ceraolo Spurio
2020-08-07 17:46 ` [Intel-gfx] [PATCH 2/2] drm/i915/uc: turn on GuC/HuC auto mode by default John.C.Harrison
2020-08-07 18:33 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915/guc: Update to GuC v45 Patchwork
2020-08-07 18:34 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2020-08-07 18:56 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2020-08-08 1:32 ` [Intel-gfx] ✗ Fi.CI.IGT: failure " Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).