Re: [Intel-gfx] [PATCH v5 01/12] drm/i915/guc: Add documentation for MMIO based communication
On Tue, 27 Mar 2018 12:05:21 +0200, Sagar Arun Kamble wrote: On 3/27/2018 1:18 AM, Michal Wajdeczko wrote: As we are going to extend our use of MMIO based communication, try to explain its mechanics and update corresponding definitions. v2: fix checkpatch MACRO_ARG_REUSE Signed-off-by: Michal Wajdeczko Cc: Daniele Ceraolo Spurio Cc: Sagar Arun Kamble Cc: Kelvin Gardiner Reviewed-by: Michel Thierry #1 --- } diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h index 72941bd..83143e8 100644 --- a/drivers/gpu/drm/i915/intel_guc_fwif.h +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h @@ -560,7 +560,68 @@ struct guc_shared_ctx_data { struct guc_ctx_report preempt_ctx_report[GUC_MAX_ENGINES_NUM]; } __packed; -/* This Action will be programmed in C180 - SOFT_SCRATCH_O_REG */ +/** + * 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 diagram below should be marked as reST literal block with "::" Diagram below is in grid-table format [1], not literal block. [1] http://docutils.sourceforge.net/docs/ref/rst/restructuredtext.html#grid-tables + * + * +---+-+-+-+ + * | 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 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 +#define INTEL_GUC_MSG_DATA_MASK(0xFFF << INTEL_GUC_MSG_DATA_SHIFT) +#define INTEL_GUC_MSG_CODE_SHIFT 0 +#define INTEL_GUC_MSG_CODE_MASK(0x << INTEL_GUC_MSG_CODE_SHIFT) + +#define __INTEL_GUC_MSG_GET(T, m) \ + (((m) & INTEL_GUC_MSG_ ## T ## _MASK) >> INTEL_GUC_MSG_ ## T ## _SHIFT) +#define INTEL_GUC_MSG_TO_TYPE(m) __INTEL_GUC_MSG_GET(TYPE, m) +#define INTEL_GUC_MSG_TO_DATA(m) __INTEL_GUC_MSG_GET(DATA, m) +#define INTEL_GUC_MSG_TO_CODE(m) __INTEL_GUC_MSG_GET(CODE, m) + +enum intel_guc_msg_type { + INTEL_GUC_MSG_TYPE_REQUEST = 0x0, + INTEL_GUC_MSG_TYPE_RESPONSE = 0xF, +}; + +#define __INTEL_GUC_MSG_TYPE_IS(T, m) \ + (INTEL_GUC_MSG_TO_TYPE(m) == INTEL_GUC_MSG_TYPE_ ## T) +#define INTEL_GUC_MSG_IS_REQUEST(m)__INTEL_GUC_MSG_TYPE_IS(REQUEST, m) *_MSG_IS_REQUEST isn't used. Do we plan to use this? now it is added just for completeness, but yes, we plan to use it. +#define INTEL_GUC_MSG_IS_RESPONSE(m) __INTEL_GUC_MSG_TYPE_IS(RESPONSE, m) + enum intel_guc_action { r INTEL_GUC_ACTION_DEFAULT = 0x0, INTEL_GUC_ACTION_REQUEST_PREEMPTION = 0x2, @@ -597,24 +658,17 @@ enum intel_guc_report_status { #define GUC_LOG_CONTROL_VERBOSITY_MASK (0xF << GUC_LOG_CONTROL_VERBOSITY_SHIFT) #define GUC_LOG_CONTROL_DEFAULT_LOGGING (1 << 8) -/* - * The GuC sends its response to a command by overwriting the - * command in SS0. The response is distinguishable from a command - * by the fact that all the MASK bits are set. The remaining bits - * give more detail. - */ -#defineINTEL_GUC_RECV_MASK ((u32)0xF000) -#defineINTEL_GUC_RECV_IS_RESPONSE(x) ((u32)(x) >= INTEL_GUC_RECV_MASK) -#defineINTEL_GUC_RECV_STATUS(x)(INTEL_GUC_RECV_MASK | (x)) - -/* GUC will return status back to SOFT_SCRATCH_O_REG */ -enum intel_guc_status { - INTEL_GUC_STATUS_SUCCESS = INTEL_GUC_RECV_STATUS(0x0), - INTEL_GUC_STATUS_ALLOCATE_DOORBELL_FAIL = INTEL_GUC_RECV_STATUS(0x10), - INTEL_GUC_STATUS_DEALLOCATE_DOORBELL_FAIL = INTEL_GUC_
Re: [Intel-gfx] [PATCH v5 01/12] drm/i915/guc: Add documentation for MMIO based communication
On 3/27/2018 1:18 AM, Michal Wajdeczko wrote: As we are going to extend our use of MMIO based communication, try to explain its mechanics and update corresponding definitions. v2: fix checkpatch MACRO_ARG_REUSE Signed-off-by: Michal Wajdeczko Cc: Daniele Ceraolo Spurio Cc: Sagar Arun Kamble Cc: Kelvin Gardiner Reviewed-by: Michel Thierry #1 --- } diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h index 72941bd..83143e8 100644 --- a/drivers/gpu/drm/i915/intel_guc_fwif.h +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h @@ -560,7 +560,68 @@ struct guc_shared_ctx_data { struct guc_ctx_report preempt_ctx_report[GUC_MAX_ENGINES_NUM]; } __packed; -/* This Action will be programmed in C180 - SOFT_SCRATCH_O_REG */ +/** + * 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 diagram below should be marked as reST literal block with "::" + * + * +---+-+-+-+ + * | 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 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 +#define INTEL_GUC_MSG_DATA_MASK(0xFFF << INTEL_GUC_MSG_DATA_SHIFT) +#define INTEL_GUC_MSG_CODE_SHIFT 0 +#define INTEL_GUC_MSG_CODE_MASK(0x << INTEL_GUC_MSG_CODE_SHIFT) + +#define __INTEL_GUC_MSG_GET(T, m) \ + (((m) & INTEL_GUC_MSG_ ## T ## _MASK) >> INTEL_GUC_MSG_ ## T ## _SHIFT) +#define INTEL_GUC_MSG_TO_TYPE(m) __INTEL_GUC_MSG_GET(TYPE, m) +#define INTEL_GUC_MSG_TO_DATA(m) __INTEL_GUC_MSG_GET(DATA, m) +#define INTEL_GUC_MSG_TO_CODE(m) __INTEL_GUC_MSG_GET(CODE, m) + +enum intel_guc_msg_type { + INTEL_GUC_MSG_TYPE_REQUEST = 0x0, + INTEL_GUC_MSG_TYPE_RESPONSE = 0xF, +}; + +#define __INTEL_GUC_MSG_TYPE_IS(T, m) \ + (INTEL_GUC_MSG_TO_TYPE(m) == INTEL_GUC_MSG_TYPE_ ## T) +#define INTEL_GUC_MSG_IS_REQUEST(m)__INTEL_GUC_MSG_TYPE_IS(REQUEST, m) *_MSG_IS_REQUEST isn't used. Do we plan to use this? +#define INTEL_GUC_MSG_IS_RESPONSE(m) __INTEL_GUC_MSG_TYPE_IS(RESPONSE, m) + enum intel_guc_action { r INTEL_GUC_ACTION_DEFAULT = 0x0, INTEL_GUC_ACTION_REQUEST_PREEMPTION = 0x2, @@ -597,24 +658,17 @@ enum intel_guc_report_status { #define GUC_LOG_CONTROL_VERBOSITY_MASK(0xF << GUC_LOG_CONTROL_VERBOSITY_SHIFT) #define GUC_LOG_CONTROL_DEFAULT_LOGGING (1 << 8) -/* - * The GuC sends its response to a command by overwriting the - * command in SS0. The response is distinguishable from a command - * by the fact that all the MASK bits are set. The remaining bits - * give more detail. - */ -#defineINTEL_GUC_RECV_MASK ((u32)0xF000) -#defineINTEL_GUC_RECV_IS_RESPONSE(x) ((u32)(x) >= INTEL_GUC_RECV_MASK) -#defineINTEL_GUC_RECV_STATUS(x)(INTEL_GUC_RECV_MASK | (x)) - -/* GUC will return status back to SOFT_SCRATCH_O_REG */ -enum intel_guc_status { - INTEL_GUC_STATUS_SUCCESS = INTEL_GUC_RECV_STATUS(0x0), - INTEL_GUC_STATUS_ALLOCATE_DOORBELL_FAIL = INTEL_GUC_RECV_STATUS(0x10), - INTEL_GUC_STATUS_DEALLOCATE_DOORBELL_FAIL = INTEL_GUC_RECV_STATUS(0x20), - INTEL_GUC_STATUS_GENERIC_FAIL = INTEL_GUC_RECV_STATUS(0xF000) +enum intel_guc_response_status { + INTEL_GUC_RESPONSE_STATUS_SUCCESS = 0x0, + INTEL_GUC_RESPONSE_STATUS_GENERIC_FAIL = 0xF000, }; +#define INTEL_GUC_MSG_IS_RESPONSE_SUCCESS(m) \ +
[Intel-gfx] [PATCH v5 01/12] drm/i915/guc: Add documentation for MMIO based communication
As we are going to extend our use of MMIO based communication, try to explain its mechanics and update corresponding definitions. v2: fix checkpatch MACRO_ARG_REUSE Signed-off-by: Michal Wajdeczko Cc: Daniele Ceraolo Spurio Cc: Sagar Arun Kamble Cc: Kelvin Gardiner Reviewed-by: Michel Thierry #1 --- drivers/gpu/drm/i915/intel_guc.c | 20 drivers/gpu/drm/i915/intel_guc_ct.c | 2 +- drivers/gpu/drm/i915/intel_guc_fwif.h | 88 --- 3 files changed, 82 insertions(+), 28 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c index 8f93f5b..28075e6 100644 --- a/drivers/gpu/drm/i915/intel_guc.c +++ b/drivers/gpu/drm/i915/intel_guc.c @@ -329,6 +329,9 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len) 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); + /* If CT is available, we expect to use MMIO only during init/fini */ GEM_BUG_ON(HAS_GUC_CT(dev_priv) && *action != INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER && @@ -350,18 +353,15 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len) */ ret = __intel_wait_for_register_fw(dev_priv, guc_send_reg(guc, 0), - INTEL_GUC_RECV_MASK, - INTEL_GUC_RECV_MASK, + INTEL_GUC_MSG_TYPE_MASK, + INTEL_GUC_MSG_TYPE_RESPONSE << + INTEL_GUC_MSG_TYPE_SHIFT, 10, 10, &status); - if (status != INTEL_GUC_STATUS_SUCCESS) { - /* -* Either the GuC explicitly returned an error (which -* we convert to -EIO here) or no response at all was -* received within the timeout limit (-ETIMEDOUT) -*/ - if (ret != -ETIMEDOUT) - ret = -EIO; + /* If GuC explicitly returned an error, convert it to -EIO */ + if (!ret && !INTEL_GUC_MSG_IS_RESPONSE_SUCCESS(status)) + ret = -EIO; + if (ret) { DRM_DEBUG_DRIVER("INTEL_GUC_SEND: Action 0x%X failed;" " ret=%d status=0x%08X response=0x%08X\n", action[0], ret, status, diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c index a726283..1dafa7a 100644 --- a/drivers/gpu/drm/i915/intel_guc_ct.c +++ b/drivers/gpu/drm/i915/intel_guc_ct.c @@ -398,7 +398,7 @@ static int ctch_send(struct intel_guc *guc, err = wait_for_response(desc, fence, status); if (unlikely(err)) return err; - if (*status != INTEL_GUC_STATUS_SUCCESS) + if (!INTEL_GUC_MSG_IS_RESPONSE_SUCCESS(*status)) return -EIO; return 0; } diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h index 72941bd..83143e8 100644 --- a/drivers/gpu/drm/i915/intel_guc_fwif.h +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h @@ -560,7 +560,68 @@ struct guc_shared_ctx_data { struct guc_ctx_report preempt_ctx_report[GUC_MAX_ENGINES_NUM]; } __packed; -/* This Action will be programmed in C180 - SOFT_SCRATCH_O_REG */ +/** + * 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 + * + * +---+-+-+-+ + * | 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 f