All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests
@ 2018-03-26 19:48 Michal Wajdeczko
  2018-03-26 19:48 ` [PATCH v5 01/12] drm/i915/guc: Add documentation for MMIO based communication Michal Wajdeczko
                   ` (21 more replies)
  0 siblings, 22 replies; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-26 19:48 UTC (permalink / raw)
  To: intel-gfx

With this series we will be able to receive more data from the Guc.
New Guc firmwares will be required to actually use that feature.

v4: respin series after 1/2 year break 
v5: updated after review comments

Michal Wajdeczko (12):
  drm/i915/guc: Add documentation for MMIO based communication
  drm/i915/guc: Add support for data reporting in GuC responses
  drm/i915/guc: Prepare send() function to accept bigger response
  drm/i915/guc: Implement response handling in send_mmio()
  drm/i915/guc: Make event handler a virtual function
  drm/i915/guc: Prepare to handle messages from CT RECV buffer
  drm/i915/guc: Use better name for helper wait function
  drm/i915/guc: Implement response handling in send_ct()
  drm/i915/guc: Prepare to process incoming requests from CT
  drm/i915/guc: Handle default action received over CT
  drm/i915/guc: Trace messages from CT while in debug
  HAX: Enable GuC for CI

 drivers/gpu/drm/i915/Kconfig.debug    |  12 +
 drivers/gpu/drm/i915/i915_params.h    |   2 +-
 drivers/gpu/drm/i915/intel_guc.c      |  51 +++-
 drivers/gpu/drm/i915/intel_guc.h      |  29 +-
 drivers/gpu/drm/i915/intel_guc_ct.c   | 503 +++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_guc_ct.h   |  12 +
 drivers/gpu/drm/i915/intel_guc_fwif.h | 139 ++++++++--
 drivers/gpu/drm/i915/intel_uc.c       |   2 +
 8 files changed, 678 insertions(+), 72 deletions(-)

-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v5 01/12] drm/i915/guc: Add documentation for MMIO based communication
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
@ 2018-03-26 19:48 ` Michal Wajdeczko
  2018-03-27 10:05   ` Sagar Arun Kamble
  2018-03-26 19:48 ` [PATCH v5 02/12] drm/i915/guc: Add support for data reporting in GuC responses Michal Wajdeczko
                   ` (20 subsequent siblings)
  21 siblings, 1 reply; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-26 19:48 UTC (permalink / raw)
  To: intel-gfx

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 <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Kelvin Gardiner <kelvin.gardiner@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com> #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 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		(0xFFFF << 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)
+#define INTEL_GUC_MSG_IS_RESPONSE(m)	__INTEL_GUC_MSG_TYPE_IS(RESPONSE, m)
+
 enum intel_guc_action {
 	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.
- */
-#define	INTEL_GUC_RECV_MASK	((u32)0xF0000000)
-#define	INTEL_GUC_RECV_IS_RESPONSE(x)	((u32)(x) >= INTEL_GUC_RECV_MASK)
-#define	INTEL_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(0x0000F000)
+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) \
+	 (typecheck(u32, (m)) && \
+	  ((m) & (INTEL_GUC_MSG_TYPE_MASK | INTEL_GUC_MSG_CODE_MASK)) == \
+	  ((INTEL_GUC_MSG_TYPE_RESPONSE << INTEL_GUC_MSG_TYPE_SHIFT) | \
+	   (INTEL_GUC_RESPONSE_STATUS_SUCCESS << INTEL_GUC_MSG_CODE_SHIFT)))
+
 /* This action will be programmed in C1BC - SOFT_SCRATCH_15_REG */
 enum intel_guc_recv_message {
 	INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED = BIT(1),
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 02/12] drm/i915/guc: Add support for data reporting in GuC responses
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
  2018-03-26 19:48 ` [PATCH v5 01/12] drm/i915/guc: Add documentation for MMIO based communication Michal Wajdeczko
@ 2018-03-26 19:48 ` Michal Wajdeczko
  2018-03-26 19:48 ` [PATCH v5 03/12] drm/i915/guc: Prepare send() function to accept bigger response Michal Wajdeczko
                   ` (19 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-26 19:48 UTC (permalink / raw)
  To: intel-gfx

GuC may return additional data in the response message.
Format and meaning of this data is action specific. We will
use this non-negative data as a new success return value.
Currently used actions don't return data that way yet.

v2: fix prohibited space after '~' (Michel)
    update commit message (Daniele)
v3: rebase

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c    |  3 +++
 drivers/gpu/drm/i915/intel_guc_ct.c | 14 ++++++++------
 2 files changed, 11 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 28075e6..77bf4e6 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -366,6 +366,9 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
 				 " ret=%d status=0x%08X response=0x%08X\n",
 				 action[0], ret, status,
 				 I915_READ(SOFT_SCRATCH(15)));
+	} else {
+		/* Use data from the GuC response as our return value */
+		ret = INTEL_GUC_MSG_TO_DATA(status);
 	}
 
 	intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 1dafa7a..fa52259 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -400,7 +400,9 @@ static int ctch_send(struct intel_guc *guc,
 		return err;
 	if (!INTEL_GUC_MSG_IS_RESPONSE_SUCCESS(*status))
 		return -EIO;
-	return 0;
+
+	/* Use data from the GuC status as our return value */
+	return INTEL_GUC_MSG_TO_DATA(*status);
 }
 
 /*
@@ -410,18 +412,18 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
 {
 	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
 	u32 status = ~0; /* undefined */
-	int err;
+	int ret;
 
 	mutex_lock(&guc->send_mutex);
 
-	err = ctch_send(guc, ctch, action, len, &status);
-	if (unlikely(err)) {
+	ret = ctch_send(guc, ctch, action, len, &status);
+	if (unlikely(ret < 0)) {
 		DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
-			  action[0], err, status);
+			  action[0], ret, status);
 	}
 
 	mutex_unlock(&guc->send_mutex);
-	return err;
+	return ret;
 }
 
 /**
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 03/12] drm/i915/guc: Prepare send() function to accept bigger response
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
  2018-03-26 19:48 ` [PATCH v5 01/12] drm/i915/guc: Add documentation for MMIO based communication Michal Wajdeczko
  2018-03-26 19:48 ` [PATCH v5 02/12] drm/i915/guc: Add support for data reporting in GuC responses Michal Wajdeczko
@ 2018-03-26 19:48 ` Michal Wajdeczko
  2018-03-26 19:48 ` [PATCH v5 04/12] drm/i915/guc: Implement response handling in send_mmio() Michal Wajdeczko
                   ` (18 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-26 19:48 UTC (permalink / raw)
  To: intel-gfx

This is a preparation step for the upcoming patches.
We already can return some small data decoded from the command
status, but we will need more in the future.

v2: add explicit response buf size
v3: squash with helper patch

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c    |  6 ++++--
 drivers/gpu/drm/i915/intel_guc.h    | 18 ++++++++++++++----
 drivers/gpu/drm/i915/intel_guc_ct.c |  7 ++++---
 3 files changed, 22 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 77bf4e6..a533ff8 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -310,7 +310,8 @@ void intel_guc_init_params(struct intel_guc *guc)
 	intel_uncore_forcewake_put(dev_priv, FORCEWAKE_BLITTER);
 }
 
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
+		       u32 *response_buf, u32 response_buf_size)
 {
 	WARN(1, "Unexpected send: action=%#x\n", *action);
 	return -ENODEV;
@@ -319,7 +320,8 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
 /*
  * 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 *action, u32 len,
+			u32 *response_buf, u32 response_buf_size)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	u32 status;
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 13f3d1d..7ee0732 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -88,7 +88,8 @@ struct intel_guc {
 	struct mutex send_mutex;
 
 	/* GuC's FW specific send function */
-	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
+	int (*send)(struct intel_guc *guc, const u32 *data, u32 len,
+		    u32 *response_buf, u32 response_buf_size);
 
 	/* GuC's FW specific notify function */
 	void (*notify)(struct intel_guc *guc);
@@ -97,7 +98,14 @@ struct intel_guc {
 static
 inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
 {
-	return guc->send(guc, action, len);
+	return guc->send(guc, action, len, NULL, 0);
+}
+
+static inline int
+intel_guc_send_and_receive(struct intel_guc *guc, const u32 *action, u32 len,
+			   u32 *response_buf, u32 response_buf_size)
+{
+	return guc->send(guc, action, len, response_buf, response_buf_size);
 }
 
 static inline void intel_guc_notify(struct intel_guc *guc)
@@ -140,8 +148,10 @@ static inline u32 intel_guc_ggtt_offset(struct intel_guc *guc,
 void intel_guc_fini_wq(struct intel_guc *guc);
 int intel_guc_init(struct intel_guc *guc);
 void intel_guc_fini(struct intel_guc *guc);
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
+		       u32 *response_buf, u32 response_buf_size);
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
+			u32 *response_buf, u32 response_buf_size);
 void intel_guc_to_host_event_handler(struct intel_guc *guc);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index fa52259..a54bf58 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -88,7 +88,7 @@ static int guc_action_register_ct_buffer(struct intel_guc *guc,
 	int err;
 
 	/* Can't use generic send(), CT registration must go over MMIO */
-	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
+	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
 	if (err)
 		DRM_ERROR("CT: register %s buffer failed; err=%d\n",
 			  guc_ct_buffer_type_to_str(type), err);
@@ -107,7 +107,7 @@ static int guc_action_deregister_ct_buffer(struct intel_guc *guc,
 	int err;
 
 	/* Can't use generic send(), CT deregistration must go over MMIO */
-	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action));
+	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
 	if (err)
 		DRM_ERROR("CT: deregister %s buffer failed; owner=%d err=%d\n",
 			  guc_ct_buffer_type_to_str(type), owner, err);
@@ -408,7 +408,8 @@ static int ctch_send(struct intel_guc *guc,
 /*
  * Command Transport (CT) buffer based GuC send function.
  */
-static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
+static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
+			     u32 *response_buf, u32 response_buf_size)
 {
 	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
 	u32 status = ~0; /* undefined */
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 04/12] drm/i915/guc: Implement response handling in send_mmio()
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (2 preceding siblings ...)
  2018-03-26 19:48 ` [PATCH v5 03/12] drm/i915/guc: Prepare send() function to accept bigger response Michal Wajdeczko
@ 2018-03-26 19:48 ` Michal Wajdeczko
  2018-03-26 19:48 ` [PATCH v5 05/12] drm/i915/guc: Make event handler a virtual function Michal Wajdeczko
                   ` (17 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-26 19:48 UTC (permalink / raw)
  To: intel-gfx

We're using data encoded in the status MMIO as return value from send
function, but GuC may also write more data in remaining MMIO regs.
Let's copy content of these registers to the buffer provided by caller.

v2: new line (Michel)
v3: updated commit message

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index a533ff8..9ce01e5 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -368,11 +368,20 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
 				 " ret=%d status=0x%08X response=0x%08X\n",
 				 action[0], ret, status,
 				 I915_READ(SOFT_SCRATCH(15)));
-	} else {
-		/* Use data from the GuC response as our return value */
-		ret = INTEL_GUC_MSG_TO_DATA(status);
+		goto out;
 	}
 
+	if (response_buf) {
+		int count = min(response_buf_size, guc->send_regs.count - 1);
+
+		for (i = 0; i < count; i++)
+			response_buf[i] = I915_READ(guc_send_reg(guc, i + 1));
+	}
+
+	/* Use data from the GuC response as our return value */
+	ret = INTEL_GUC_MSG_TO_DATA(status);
+
+out:
 	intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
 	mutex_unlock(&guc->send_mutex);
 
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 05/12] drm/i915/guc: Make event handler a virtual function
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (3 preceding siblings ...)
  2018-03-26 19:48 ` [PATCH v5 04/12] drm/i915/guc: Implement response handling in send_mmio() Michal Wajdeczko
@ 2018-03-26 19:48 ` Michal Wajdeczko
  2018-03-26 19:48 ` [PATCH v5 06/12] drm/i915/guc: Prepare to handle messages from CT RECV buffer Michal Wajdeczko
                   ` (16 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-26 19:48 UTC (permalink / raw)
  To: intel-gfx

On platforms with CTB based GuC communications, we will handle
GuC events in a different way. Let's make event handler a virtual
function to allow easy switch between those variants.

Credits-to: Oscar Mateo <oscar.mateo@intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c |  8 +++++++-
 drivers/gpu/drm/i915/intel_guc.h | 10 ++++++++++
 drivers/gpu/drm/i915/intel_uc.c  |  2 ++
 3 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 9ce01e5..118db81 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -69,6 +69,7 @@ void intel_guc_init_early(struct intel_guc *guc)
 	mutex_init(&guc->send_mutex);
 	spin_lock_init(&guc->irq_lock);
 	guc->send = intel_guc_send_nop;
+	guc->handler = intel_guc_to_host_event_handler_nop;
 	guc->notify = gen8_guc_raise_irq;
 }
 
@@ -317,6 +318,11 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
 	return -ENODEV;
 }
 
+void intel_guc_to_host_event_handler_nop(struct intel_guc *guc)
+{
+	WARN(1, "Unexpected event: no suitable handler\n");
+}
+
 /*
  * This function implements the MMIO based host to GuC interface.
  */
@@ -388,7 +394,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
 	return ret;
 }
 
-void intel_guc_to_host_event_handler(struct intel_guc *guc)
+void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	u32 msg, val;
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 7ee0732..6dc109a 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -91,6 +91,9 @@ struct intel_guc {
 	int (*send)(struct intel_guc *guc, const u32 *data, u32 len,
 		    u32 *response_buf, u32 response_buf_size);
 
+	/* GuC's FW specific event handler function */
+	void (*handler)(struct intel_guc *guc);
+
 	/* GuC's FW specific notify function */
 	void (*notify)(struct intel_guc *guc);
 };
@@ -113,6 +116,11 @@ static inline void intel_guc_notify(struct intel_guc *guc)
 	guc->notify(guc);
 }
 
+static inline void intel_guc_to_host_event_handler(struct intel_guc *guc)
+{
+	guc->handler(guc);
+}
+
 /* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
 #define GUC_GGTT_TOP	0xFEE00000
 
@@ -153,6 +161,8 @@ int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
 			u32 *response_buf, u32 response_buf_size);
 void intel_guc_to_host_event_handler(struct intel_guc *guc);
+void intel_guc_to_host_event_handler_nop(struct intel_guc *guc);
+void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
 int intel_guc_suspend(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 4aad844..081e424 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -233,6 +233,7 @@ static int guc_enable_communication(struct intel_guc *guc)
 		return intel_guc_ct_enable(&guc->ct);
 
 	guc->send = intel_guc_send_mmio;
+	guc->handler = intel_guc_to_host_event_handler_mmio;
 	return 0;
 }
 
@@ -246,6 +247,7 @@ static void guc_disable_communication(struct intel_guc *guc)
 	gen9_disable_guc_interrupts(dev_priv);
 
 	guc->send = intel_guc_send_nop;
+	guc->handler = intel_guc_to_host_event_handler_nop;
 }
 
 int intel_uc_init_misc(struct drm_i915_private *dev_priv)
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 06/12] drm/i915/guc: Prepare to handle messages from CT RECV buffer
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (4 preceding siblings ...)
  2018-03-26 19:48 ` [PATCH v5 05/12] drm/i915/guc: Make event handler a virtual function Michal Wajdeczko
@ 2018-03-26 19:48 ` Michal Wajdeczko
  2018-03-26 19:48 ` [PATCH v5 07/12] drm/i915/guc: Use better name for helper wait function Michal Wajdeczko
                   ` (15 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-26 19:48 UTC (permalink / raw)
  To: intel-gfx

GuC can respond to our commands not only by updating SEND buffer
descriptor, but can also send a response message over RECV buffer.
Guc can also send unsolicited request messages over RECV buffer.
Let's start reading those messages and make placeholders
for actual response/request handlers.

v2: misc improvements (Michal)
v3: change response detection (Michal)
    invalid status is protocol error (Michal)
v4: rebase
v5: fix checkpatch (Michel)
    don't use fields before check (Jani)
    add some documentation (Michal)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com> # 4.5
Cc: Jani Nikula <jani.nikula@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 184 +++++++++++++++++++++++++++++++++++-
 1 file changed, 183 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index a54bf58..14f55de 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -273,6 +273,24 @@ static u32 ctch_get_next_fence(struct intel_guc_ct_channel *ctch)
 	return ++ctch->next_fence;
 }
 
+/**
+ * DOC: CTB Host to GuC request
+ *
+ * Format of the CTB Host to GuC request message is as follows::
+ *
+ *      +------------+---------+---------+---------+---------+
+ *      |   msg[0]   |   [1]   |   [2]   |   ...   |  [n-1]  |
+ *      +------------+---------+---------+---------+---------+
+ *      |   MESSAGE  |       MESSAGE PAYLOAD                 |
+ *      +   HEADER   +---------+---------+---------+---------+
+ *      |            |    0    |    1    |   ...   |    n    |
+ *      +============+=========+=========+=========+=========+
+ *      |  len >= 1  |  FENCE  |     request specific data   |
+ *      +------+-----+---------+---------+---------+---------+
+ *
+ *                   ^-----------------len-------------------^
+ */
+
 static int ctb_write(struct intel_guc_ct_buffer *ctb,
 		     const u32 *action,
 		     u32 len /* in dwords */,
@@ -305,7 +323,8 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 	if (unlikely(used + len + 1 >= size))
 		return -ENOSPC;
 
-	/* Write the message. The format is the following:
+	/*
+	 * Write the message. The format is the following:
 	 * DW0: header (including action code)
 	 * DW1: fence
 	 * DW2+: action data
@@ -427,6 +446,167 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
 	return ret;
 }
 
+static inline unsigned int ct_header_get_len(u32 header)
+{
+	return (header >> GUC_CT_MSG_LEN_SHIFT) & GUC_CT_MSG_LEN_MASK;
+}
+
+static inline unsigned int ct_header_get_action(u32 header)
+{
+	return (header >> GUC_CT_MSG_ACTION_SHIFT) & GUC_CT_MSG_ACTION_MASK;
+}
+
+static inline bool ct_header_is_response(u32 header)
+{
+	return ct_header_get_action(header) == INTEL_GUC_ACTION_DEFAULT;
+}
+
+static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
+{
+	struct guc_ct_buffer_desc *desc = ctb->desc;
+	u32 head = desc->head / 4;	/* in dwords */
+	u32 tail = desc->tail / 4;	/* in dwords */
+	u32 size = desc->size / 4;	/* in dwords */
+	u32 *cmds = ctb->cmds;
+	s32 available;			/* in dwords */
+	unsigned int len;
+	unsigned int i;
+
+	GEM_BUG_ON(desc->size % 4);
+	GEM_BUG_ON(desc->head % 4);
+	GEM_BUG_ON(desc->tail % 4);
+	GEM_BUG_ON(tail >= size);
+	GEM_BUG_ON(head >= size);
+
+	/* tail == head condition indicates empty */
+	available = tail - head;
+	if (unlikely(available == 0))
+		return -ENODATA;
+
+	/* beware of buffer wrap case */
+	if (unlikely(available < 0))
+		available += size;
+	GEM_BUG_ON(available < 0);
+
+	data[0] = cmds[head];
+	head = (head + 1) % size;
+
+	/* message len with header */
+	len = ct_header_get_len(data[0]) + 1;
+	if (unlikely(len > (u32)available)) {
+		DRM_ERROR("CT: incomplete message %*phn %*phn %*phn\n",
+			  4, data,
+			  4 * (head + available - 1 > size ?
+			       size - head : available - 1), &cmds[head],
+			  4 * (head + available - 1 > size ?
+			       available - 1 - size + head : 0), &cmds[0]);
+		return -EPROTO;
+	}
+
+	for (i = 1; i < len; i++) {
+		data[i] = cmds[head];
+		head = (head + 1) % size;
+	}
+
+	desc->head = head * 4;
+	return 0;
+}
+
+/**
+ * DOC: CTB GuC to Host response
+ *
+ * Format of the CTB GuC to Host response message is as follows::
+ *
+ *      +------------+---------+---------+---------+---------+---------+
+ *      |   msg[0]   |   [1]   |   [2]   |   [3]   |   ...   |  [n-1]  |
+ *      +------------+---------+---------+---------+---------+---------+
+ *      |   MESSAGE  |       MESSAGE PAYLOAD                           |
+ *      +   HEADER   +---------+---------+---------+---------+---------+
+ *      |            |    0    |    1    |    2    |   ...   |    n    |
+ *      +============+=========+=========+=========+=========+=========+
+ *      |  len >= 2  |  FENCE  |  STATUS |   response specific data    |
+ *      +------+-----+---------+---------+---------+---------+---------+
+ *
+ *                   ^-----------------------len-----------------------^
+ */
+
+static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
+{
+	u32 header = msg[0];
+	u32 len = ct_header_get_len(header);
+	u32 msglen = len + 1; /* total message length including header */
+	u32 fence;
+	u32 status;
+
+	GEM_BUG_ON(!ct_header_is_response(header));
+
+	/* Response payload shall at least include fence and status */
+	if (unlikely(len < 2)) {
+		DRM_ERROR("CT: corrupted response %*phn\n", 4 * msglen, msg);
+		return -EPROTO;
+	}
+
+	fence = msg[1];
+	status = msg[2];
+
+	/* Format of the status follows RESPONSE message */
+	if (unlikely(!INTEL_GUC_MSG_IS_RESPONSE(status))) {
+		DRM_ERROR("CT: corrupted response %*phn\n", 4 * msglen, msg);
+		return -EPROTO;
+	}
+
+	/* XXX */
+	return 0;
+}
+
+static int ct_handle_request(struct intel_guc_ct *ct, const u32 *msg)
+{
+	u32 header = msg[0];
+
+	GEM_BUG_ON(ct_header_is_response(header));
+
+	/* XXX */
+	return 0;
+}
+
+static void ct_process_host_channel(struct intel_guc_ct *ct)
+{
+	struct intel_guc_ct_channel *ctch = &ct->host_channel;
+	struct intel_guc_ct_buffer *ctb = &ctch->ctbs[CTB_RECV];
+	u32 msg[GUC_CT_MSG_LEN_MASK + 1]; /* one extra dw for the header */
+	int err = 0;
+
+	if (!ctch_is_open(ctch))
+		return;
+
+	do {
+		err = ctb_read(ctb, msg);
+		if (err)
+			break;
+
+		if (ct_header_is_response(msg[0]))
+			err = ct_handle_response(ct, msg);
+		else
+			err = ct_handle_request(ct, msg);
+	} while (!err);
+
+	if (GEM_WARN_ON(err == -EPROTO)) {
+		DRM_ERROR("CT: corrupted message detected!\n");
+		ctb->desc->is_in_error = 1;
+	}
+}
+
+/*
+ * When we're communicating with the GuC over CT, GuC uses events
+ * to notify us about new messages being posted on the RECV buffer.
+ */
+static void intel_guc_to_host_event_handler_ct(struct intel_guc *guc)
+{
+	struct intel_guc_ct *ct = &guc->ct;
+
+	ct_process_host_channel(ct);
+}
+
 /**
  * intel_guc_ct_enable - Enable buffer based command transport.
  * @ct: pointer to CT struct
@@ -450,6 +630,7 @@ int intel_guc_ct_enable(struct intel_guc_ct *ct)
 
 	/* Switch into cmd transport buffer based send() */
 	guc->send = intel_guc_send_ct;
+	guc->handler = intel_guc_to_host_event_handler_ct;
 	DRM_INFO("CT: %s\n", enableddisabled(true));
 	return 0;
 }
@@ -475,5 +656,6 @@ void intel_guc_ct_disable(struct intel_guc_ct *ct)
 
 	/* Disable send */
 	guc->send = intel_guc_send_nop;
+	guc->handler = intel_guc_to_host_event_handler_nop;
 	DRM_INFO("CT: %s\n", enableddisabled(false));
 }
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 07/12] drm/i915/guc: Use better name for helper wait function
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (5 preceding siblings ...)
  2018-03-26 19:48 ` [PATCH v5 06/12] drm/i915/guc: Prepare to handle messages from CT RECV buffer Michal Wajdeczko
@ 2018-03-26 19:48 ` Michal Wajdeczko
  2018-03-26 19:48 ` [PATCH v5 08/12] drm/i915/guc: Implement response handling in send_ct() Michal Wajdeczko
                   ` (14 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-26 19:48 UTC (permalink / raw)
  To: intel-gfx

In next patch we will introduce another way of waiting for the response
that will use RECV buffer. To avoid misleading names, rename old wait
function to reflect the fact that it is based on descriptor update.

v2: fix comment style (Michal)
v3: use more specific name (Michel)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 25 +++++++++++++++++--------
 1 file changed, 17 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 14f55de..2d805a2 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -351,16 +351,25 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 	return 0;
 }
 
-/* Wait for the response from the GuC.
+/**
+ * wait_for_ctb_desc_update - Wait for the CT buffer descriptor update.
+ * @desc:	buffer descriptor
  * @fence:	response fence
  * @status:	placeholder for status
- * return:	0 response received (status is valid)
- *		-ETIMEDOUT no response within hardcoded timeout
- *		-EPROTO no response, ct buffer was in error
+ *
+ * Guc will update CT buffer descriptor with new fence and status
+ * after processing the command identified by the fence. Wait for
+ * specified fence and then read from the descriptor status of the
+ * command.
+ *
+ * Return:
+ * *	0 response received (status is valid)
+ * *	-ETIMEDOUT no response within hardcoded timeout
+ * *	-EPROTO no response, CT buffer is in error
  */
-static int wait_for_response(struct guc_ct_buffer_desc *desc,
-			     u32 fence,
-			     u32 *status)
+static int wait_for_ctb_desc_update(struct guc_ct_buffer_desc *desc,
+				    u32 fence,
+				    u32 *status)
 {
 	int err;
 
@@ -414,7 +423,7 @@ static int ctch_send(struct intel_guc *guc,
 
 	intel_guc_notify(guc);
 
-	err = wait_for_response(desc, fence, status);
+	err = wait_for_ctb_desc_update(desc, fence, status);
 	if (unlikely(err))
 		return err;
 	if (!INTEL_GUC_MSG_IS_RESPONSE_SUCCESS(*status))
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 08/12] drm/i915/guc: Implement response handling in send_ct()
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (6 preceding siblings ...)
  2018-03-26 19:48 ` [PATCH v5 07/12] drm/i915/guc: Use better name for helper wait function Michal Wajdeczko
@ 2018-03-26 19:48 ` Michal Wajdeczko
  2018-03-27 12:14   ` [PATCH v6 " Michal Wajdeczko
  2018-03-26 19:48 ` [PATCH v5 09/12] drm/i915/guc: Prepare to process incoming requests from CT Michal Wajdeczko
                   ` (13 subsequent siblings)
  21 siblings, 1 reply; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-26 19:48 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

Instead of returning small data in response status dword,
GuC may append longer data as response message payload.
If caller provides response buffer, we will copy received
data and use number of received data dwords as new success
return value. We will WARN if response from GuC does not
match caller expectation.

v2: fix timeout and checkpatch warnings (Michal)
v3: fix checkpatch again (Michel)
    update wait function name (Michal)
    no need for spinlock_irqsave (MichalWi)
    no magic numbers (MichalWi)
    must check before use (Jani)
    add some more documentation (Michal)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com> #2.5
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c   | 142 ++++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_guc_ct.h   |   6 ++
 drivers/gpu/drm/i915/intel_guc_fwif.h |  51 ++++++++++++
 3 files changed, 185 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 2d805a2..41b071c 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -24,6 +24,14 @@
 #include "i915_drv.h"
 #include "intel_guc_ct.h"
 
+struct ct_request {
+	struct list_head link;
+	u32 fence;
+	u32 status;
+	u32 response_len;
+	u32 *response_buf;
+};
+
 enum { CTB_SEND = 0, CTB_RECV = 1 };
 
 enum { CTB_OWNER_HOST = 0 };
@@ -36,6 +44,9 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 {
 	/* we're using static channel owners */
 	ct->host_channel.owner = CTB_OWNER_HOST;
+
+	spin_lock_init(&ct->lock);
+	INIT_LIST_HEAD(&ct->pending_requests);
 }
 
 static inline struct intel_guc *ct_to_guc(struct intel_guc_ct *ct)
@@ -294,7 +305,8 @@ static u32 ctch_get_next_fence(struct intel_guc_ct_channel *ctch)
 static int ctb_write(struct intel_guc_ct_buffer *ctb,
 		     const u32 *action,
 		     u32 len /* in dwords */,
-		     u32 fence)
+		     u32 fence,
+		     bool want_response)
 {
 	struct guc_ct_buffer_desc *desc = ctb->desc;
 	u32 head = desc->head / 4;	/* in dwords */
@@ -331,6 +343,7 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 	 */
 	header = (len << GUC_CT_MSG_LEN_SHIFT) |
 		 (GUC_CT_MSG_WRITE_FENCE_TO_DESC) |
+		 (want_response ? GUC_CT_MSG_SEND_STATUS : 0) |
 		 (action[0] << GUC_CT_MSG_ACTION_SHIFT);
 
 	cmds[tail] = header;
@@ -401,36 +414,108 @@ static int wait_for_ctb_desc_update(struct guc_ct_buffer_desc *desc,
 	return err;
 }
 
-static int ctch_send(struct intel_guc *guc,
+/**
+ * wait_for_ct_request_update - Wait for CT request state update.
+ * @req:	pointer to pending request
+ * @status:	placeholder for status
+ *
+ * For each sent request, Guc shall send bac CT response message.
+ * Our message handler will update status of tracked request once
+ * response message with given fence is received. Wait here and
+ * check for valid response status value.
+ *
+ * Return:
+ * *	0 response received (status is valid)
+ * *	-ETIMEDOUT no response within hardcoded timeout
+ */
+static int wait_for_ct_request_update(struct ct_request *req, u32 *status)
+{
+	int err;
+
+	/*
+	 * Fast commands should complete in less than 10us, so sample quickly
+	 * up to that length of time, then switch to a slower sleep-wait loop.
+	 * No GuC command should ever take longer than 10ms.
+	 */
+#define done INTEL_GUC_MSG_IS_RESPONSE(READ_ONCE(req->status))
+	err = wait_for_us(done, 10);
+	if (err)
+		err = wait_for(done, 10);
+#undef done
+
+	if (unlikely(err))
+		DRM_ERROR("CT: fence %u err %d\n", req->fence, err);
+
+	*status = req->status;
+	return err;
+}
+
+static int ctch_send(struct intel_guc_ct *ct,
 		     struct intel_guc_ct_channel *ctch,
 		     const u32 *action,
 		     u32 len,
+		     u32 *response_buf,
+		     u32 response_buf_size,
 		     u32 *status)
 {
 	struct intel_guc_ct_buffer *ctb = &ctch->ctbs[CTB_SEND];
 	struct guc_ct_buffer_desc *desc = ctb->desc;
+	struct ct_request request;
+	unsigned long flags;
 	u32 fence;
 	int err;
 
 	GEM_BUG_ON(!ctch_is_open(ctch));
 	GEM_BUG_ON(!len);
 	GEM_BUG_ON(len & ~GUC_CT_MSG_LEN_MASK);
+	GEM_BUG_ON(!response_buf && response_buf_size);
 
 	fence = ctch_get_next_fence(ctch);
-	err = ctb_write(ctb, action, len, fence);
+	request.fence = fence;
+	request.status = 0;
+	request.response_len = response_buf_size;
+	request.response_buf = response_buf;
+
+	spin_lock_irqsave(&ct->lock, flags);
+	list_add_tail(&request.link, &ct->pending_requests);
+	spin_unlock_irqrestore(&ct->lock, flags);
+
+	err = ctb_write(ctb, action, len, fence, !!response_buf);
 	if (unlikely(err))
-		return err;
+		goto unlink;
 
-	intel_guc_notify(guc);
+	intel_guc_notify(ct_to_guc(ct));
 
-	err = wait_for_ctb_desc_update(desc, fence, status);
+	if (response_buf)
+		err = wait_for_ct_request_update(&request, status);
+	else
+		err = wait_for_ctb_desc_update(desc, fence, status);
 	if (unlikely(err))
-		return err;
-	if (!INTEL_GUC_MSG_IS_RESPONSE_SUCCESS(*status))
-		return -EIO;
+		goto unlink;
+
+	if (!INTEL_GUC_MSG_IS_RESPONSE_SUCCESS(*status)) {
+		err = -EIO;
+		goto unlink;
+	}
+
+	if (response_buf) {
+		/* There shall be no data in the status */
+		WARN_ON(INTEL_GUC_MSG_TO_DATA(request.status));
+		/* Return actual response len */
+		err = request.response_len;
+	} else {
+		/* There shall be no response payload */
+		WARN_ON(request.response_len);
+		/* Return data decoded from the status dword */
+		err = INTEL_GUC_MSG_TO_DATA(*status);
+	}
 
-	/* Use data from the GuC status as our return value */
-	return INTEL_GUC_MSG_TO_DATA(*status);
+unlink:
+	spin_lock_irqsave(&ct->lock, flags);
+	list_del(&request.link);
+	spin_unlock_irqrestore(&ct->lock, flags);
+
+	return err;
 }
 
 /*
@@ -439,13 +524,15 @@ static int ctch_send(struct intel_guc *guc,
 static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
 			     u32 *response_buf, u32 response_buf_size)
 {
-	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
+	struct intel_guc_ct *ct = &guc->ct;
+	struct intel_guc_ct_channel *ctch = &ct->host_channel;
 	u32 status = ~0; /* undefined */
 	int ret;
 
 	mutex_lock(&guc->send_mutex);
 
-	ret = ctch_send(guc, ctch, action, len, &status);
+	ret = ctch_send(ct, ctch, action, len, response_buf, response_buf_size,
+			&status);
 	if (unlikely(ret < 0)) {
 		DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
 			  action[0], ret, status);
@@ -546,8 +633,12 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
 	u32 msglen = len + 1; /* total message length including header */
 	u32 fence;
 	u32 status;
+	u32 datalen;
+	struct ct_request *req;
+	bool found = false;
 
 	GEM_BUG_ON(!ct_header_is_response(header));
+	GEM_BUG_ON(!in_irq());
 
 	/* Response payload shall at least include fence and status */
 	if (unlikely(len < 2)) {
@@ -557,6 +648,7 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
 
 	fence = msg[1];
 	status = msg[2];
+	datalen = len - 2;
 
 	/* Format of the status follows RESPONSE message */
 	if (unlikely(!INTEL_GUC_MSG_IS_RESPONSE(status))) {
@@ -564,7 +656,29 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
 		return -EPROTO;
 	}
 
-	/* XXX */
+	spin_lock(&ct->lock);
+	list_for_each_entry(req, &ct->pending_requests, link) {
+		if (unlikely(fence != req->fence)) {
+			DRM_DEBUG_DRIVER("CT: request %u awaits response\n",
+					 req->fence);
+			continue;
+		}
+		if (unlikely(datalen > req->response_len)) {
+			DRM_ERROR("CT: response %u too long %*phn\n",
+				  req->fence, 4 * msglen, msg);
+			datalen = 0;
+		}
+		if (datalen)
+			memcpy(req->response_buf, msg + 3, 4 * datalen);
+		req->response_len = datalen;
+		WRITE_ONCE(req->status, status);
+		found = true;
+		break;
+	}
+	spin_unlock(&ct->lock);
+
+	if (!found)
+		DRM_ERROR("CT: unsolicited response %*phn\n", 4 * msglen, msg);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
index 595c8ad..fac6e53 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/intel_guc_ct.h
@@ -75,6 +75,12 @@ struct intel_guc_ct_channel {
 struct intel_guc_ct {
 	struct intel_guc_ct_channel host_channel;
 	/* other channels are tbd */
+
+	/** @lock: protects pending requests list */
+	spinlock_t lock;
+
+	/** @pending_requests: list of requests waiting for response */
+	struct list_head pending_requests;
 };
 
 void intel_guc_ct_init_early(struct intel_guc_ct *ct);
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 83143e8..337abc5 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -327,6 +327,57 @@ struct guc_stage_desc {
 	u64 desc_private;
 } __packed;
 
+/**
+ * DOC: CTB based communication
+ *
+ * The CTB (command transport buffer) communication between Host and GuC
+ * is based on u32 data stream written to the shared buffer. One buffer can
+ * be used to transmit data only in one direction (one-directional channel).
+ *
+ * Current status of the each buffer is stored in the buffer descriptor.
+ * Buffer descriptor holds tail and head fields that represents active data
+ * stream. The tail field is updated by the data producer (sender), and head
+ * field is updated by the data consumer (receiver)::
+ *
+ *      +------------+
+ *      | DESCRIPTOR |          +=================+============+========+
+ *      +============+          |                 | MESSAGE(s) |        |
+ *      | address    |--------->+=================+============+========+
+ *      +------------+
+ *      | head       |          ^-----head--------^
+ *      +------------+
+ *      | tail       |          ^---------tail-----------------^
+ *      +------------+
+ *      | size       |          ^---------------size--------------------^
+ *      +------------+
+ *
+ * Each message in data stream starts with the single u32 treated as a header
+ * that specifies message code, flags and length of the whole message,
+ * including message specific payload that follows header::
+ *
+ *      +------------+---------+---------+---------+
+ *      |   msg[0]   |   [1]   |   ...   |  [n-1]  |
+ *      +------------+---------+---------+---------+
+ *      |   MESSAGE  |       MESSAGE PAYLOAD       |
+ *      +   HEADER   +---------+---------+---------+
+ *      |            |    0    |   ...   |    n    |
+ *      +======+=====+=========+=========+=========+
+ *      | 31:16| code|         |         |         |
+ *      +------+-----+         |         |         |
+ *      |  15:5|flags|         |         |         |
+ *      +------+-----+         |         |         |
+ *      |   4:0|  len|         |         |         |
+ *      +------+-----+---------+---------+---------+
+ *
+ *                   ^-------------len-------------^
+ *
+ * The message header consists of:
+ *
+ * - **len**, indicates length of the message payload (in u32)
+ * - **code**, indicates message code
+ * - **flags**, holds various bits to control message handling
+ */
+
 /*
  * Describes single command transport buffer.
  * Used by both guc-master and clients.
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 09/12] drm/i915/guc: Prepare to process incoming requests from CT
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (7 preceding siblings ...)
  2018-03-26 19:48 ` [PATCH v5 08/12] drm/i915/guc: Implement response handling in send_ct() Michal Wajdeczko
@ 2018-03-26 19:48 ` Michal Wajdeczko
  2018-03-27 20:07   ` Michel Thierry
  2018-03-26 19:48 ` [PATCH v5 10/12] drm/i915/guc: Handle default action received over CT Michal Wajdeczko
                   ` (12 subsequent siblings)
  21 siblings, 1 reply; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-26 19:48 UTC (permalink / raw)
  To: intel-gfx

Requests are read from CT in the irq handler, but actual processing
will be done in the work thread. Processing of specific actions will
be added in the upcoming patches.

v2: don't use GEM_BUG_ON (Chris)
    don't kmalloc too large buffer (Michal)
v3: rebased
v4: don't name it 'dispatch' (Michel) and fix checkpatch
    add some documentation (Michal)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c | 95 ++++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/i915/intel_guc_ct.h |  6 +++
 2 files changed, 100 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 41b071c..aa810ad 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -32,10 +32,17 @@ struct ct_request {
 	u32 *response_buf;
 };
 
+struct ct_incoming_request {
+	struct list_head link;
+	u32 msg[];
+};
+
 enum { CTB_SEND = 0, CTB_RECV = 1 };
 
 enum { CTB_OWNER_HOST = 0 };
 
+static void ct_incoming_request_worker_func(struct work_struct *w);
+
 /**
  * intel_guc_ct_init_early - Initialize CT state without requiring device access
  * @ct: pointer to CT struct
@@ -47,6 +54,8 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 
 	spin_lock_init(&ct->lock);
 	INIT_LIST_HEAD(&ct->pending_requests);
+	INIT_LIST_HEAD(&ct->incoming_requests);
+	INIT_WORK(&ct->worker, ct_incoming_request_worker_func);
 }
 
 static inline struct intel_guc *ct_to_guc(struct intel_guc_ct *ct)
@@ -682,13 +691,97 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
 	return 0;
 }
 
+static void ct_process_request(struct intel_guc_ct *ct,
+			       u32 action, u32 len, const u32 *payload)
+{
+	switch (action) {
+	default:
+		DRM_ERROR("CT: unexpected request %x %*phn\n",
+			  action, 4 * len, payload);
+		break;
+	}
+}
+
+static bool ct_process_incoming_requests(struct intel_guc_ct *ct)
+{
+	unsigned long flags;
+	struct ct_incoming_request *request;
+	u32 header;
+	u32 *payload;
+	bool done;
+
+	spin_lock_irqsave(&ct->lock, flags);
+	request = list_first_entry_or_null(&ct->incoming_requests,
+					   struct ct_incoming_request, link);
+	if (request)
+		list_del(&request->link);
+	done = !!list_empty(&ct->incoming_requests);
+	spin_unlock_irqrestore(&ct->lock, flags);
+
+	if (!request)
+		return true;
+
+	header = request->msg[0];
+	payload = &request->msg[1];
+	ct_process_request(ct,
+			   ct_header_get_action(header),
+			   ct_header_get_len(header),
+			   payload);
+
+	kfree(request);
+	return done;
+}
+
+static void ct_incoming_request_worker_func(struct work_struct *w)
+{
+	struct intel_guc_ct *ct = container_of(w, struct intel_guc_ct, worker);
+	bool done;
+
+	done = ct_process_incoming_requests(ct);
+	if (!done)
+		queue_work(system_unbound_wq, &ct->worker);
+}
+
+/**
+ * DOC: CTB GuC to Host request
+ *
+ * Format of the CTB GuC to Host request message is as follows::
+ *
+ *      +------------+---------+---------+---------+---------+---------+
+ *      |   msg[0]   |   [1]   |   [2]   |   [3]   |   ...   |  [n-1]  |
+ *      +------------+---------+---------+---------+---------+---------+
+ *      |   MESSAGE  |       MESSAGE PAYLOAD                           |
+ *      +   HEADER   +---------+---------+---------+---------+---------+
+ *      |            |    0    |    1    |    2    |   ...   |    n    |
+ *      +============+=========+=========+=========+=========+=========+
+ *      |     len    |            request specific data                |
+ *      +------+-----+---------+---------+---------+---------+---------+
+ *
+ *                   ^-----------------------len-----------------------^
+ */
+
 static int ct_handle_request(struct intel_guc_ct *ct, const u32 *msg)
 {
 	u32 header = msg[0];
+	u32 len = ct_header_get_len(header);
+	u32 msglen = len + 1; /* total message length including header */
+	struct ct_incoming_request *request;
+	unsigned long flags;
 
 	GEM_BUG_ON(ct_header_is_response(header));
 
-	/* XXX */
+	request = kmalloc(sizeof(*request) + 4 * msglen, GFP_ATOMIC);
+	if (unlikely(!request)) {
+		DRM_ERROR("CT: dropping request %*phn\n", 4 * msglen, msg);
+		return 0; /* XXX: -ENOMEM ? */
+	}
+	memcpy(request->msg, msg, 4 * msglen);
+
+	spin_lock_irqsave(&ct->lock, flags);
+	list_add_tail(&request->link, &ct->incoming_requests);
+	spin_unlock_irqrestore(&ct->lock, flags);
+
+	queue_work(system_unbound_wq, &ct->worker);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
index fac6e53..d774895a 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/intel_guc_ct.h
@@ -81,6 +81,12 @@ struct intel_guc_ct {
 
 	/** @pending_requests: list of requests waiting for response */
 	struct list_head pending_requests;
+
+	/** @incoming_requests: list of incoming requests */
+	struct list_head incoming_requests;
+
+	/** @worker: worker for handling incoming requests */
+	struct work_struct worker;
 };
 
 void intel_guc_ct_init_early(struct intel_guc_ct *ct);
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 10/12] drm/i915/guc: Handle default action received over CT
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (8 preceding siblings ...)
  2018-03-26 19:48 ` [PATCH v5 09/12] drm/i915/guc: Prepare to process incoming requests from CT Michal Wajdeczko
@ 2018-03-26 19:48 ` Michal Wajdeczko
  2018-03-27 18:25   ` Daniele Ceraolo Spurio
  2018-03-27 21:41   ` [PATCH v7 " Michal Wajdeczko
  2018-03-26 19:48 ` [PATCH v5 11/12] drm/i915/guc: Trace messages from CT while in debug Michal Wajdeczko
                   ` (11 subsequent siblings)
  21 siblings, 2 replies; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-26 19:48 UTC (permalink / raw)
  To: intel-gfx

When running on platform with CTB based GuC communication enabled,
GuC to Host event data will be delivered as CT request message.
However, content of the data[1] of this CT message follows format
of the scratch register used in MMIO based communication, so some
code reuse is still possible.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c    | 5 +++++
 drivers/gpu/drm/i915/intel_guc.h    | 1 +
 drivers/gpu/drm/i915/intel_guc_ct.c | 9 +++++++++
 3 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 118db81..b6d2778 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -416,6 +416,11 @@ void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc)
 	I915_WRITE(SOFT_SCRATCH(15), val & ~msg);
 	spin_unlock(&guc->irq_lock);
 
+	intel_guc_to_host_process_recv_msg(guc, msg);
+}
+
+void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 msg)
+{
 	if (msg & (INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
 		   INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED))
 		intel_guc_log_handle_flush_event(&guc->log);
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 6dc109a..f1265e1 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -163,6 +163,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
 void intel_guc_to_host_event_handler(struct intel_guc *guc);
 void intel_guc_to_host_event_handler_nop(struct intel_guc *guc);
 void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc);
+void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 msg);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
 int intel_guc_suspend(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index aa810ad..e837084 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -694,8 +694,17 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
 static void ct_process_request(struct intel_guc_ct *ct,
 			       u32 action, u32 len, const u32 *payload)
 {
+	struct intel_guc *guc = ct_to_guc(ct);
+
 	switch (action) {
+	case INTEL_GUC_ACTION_DEFAULT:
+		if (unlikely(len < 1))
+			goto fail_unexpected;
+		intel_guc_to_host_process_recv_msg(guc, *payload);
+		break;
+
 	default:
+fail_unexpected:
 		DRM_ERROR("CT: unexpected request %x %*phn\n",
 			  action, 4 * len, payload);
 		break;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 11/12] drm/i915/guc: Trace messages from CT while in debug
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (9 preceding siblings ...)
  2018-03-26 19:48 ` [PATCH v5 10/12] drm/i915/guc: Handle default action received over CT Michal Wajdeczko
@ 2018-03-26 19:48 ` Michal Wajdeczko
  2018-03-26 19:48 ` [PATCH v5 12/12] HAX: Enable GuC for CI Michal Wajdeczko
                   ` (10 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-26 19:48 UTC (permalink / raw)
  To: intel-gfx

During debug we may want to investigate all communication
from the Guc. Add proper tracing macros in debug config.

v2: convert remaining DRM_DEBUG into new CT_DEBUG (Michal)
v3: use dedicated Kconfig (Daniele)
v4: checkpatch

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/Kconfig.debug  | 12 +++++++++++
 drivers/gpu/drm/i915/intel_guc_ct.c | 43 ++++++++++++++++++++++++++-----------
 2 files changed, 43 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/Kconfig.debug b/drivers/gpu/drm/i915/Kconfig.debug
index dd5bf63..80efee1 100644
--- a/drivers/gpu/drm/i915/Kconfig.debug
+++ b/drivers/gpu/drm/i915/Kconfig.debug
@@ -90,6 +90,18 @@ config DRM_I915_SW_FENCE_CHECK_DAG
 
           If in doubt, say "N".
 
+config DRM_I915_DEBUG_GUC
+        bool "Enable additional driver debugging for GuC"
+        depends on DRM_I915
+        default n
+        help
+          Choose this option to turn on extra driver debugging that may affect
+          performance but will help resolve GuC related issues.
+
+          Recommended for driver developers only.
+
+          If in doubt, say "N".
+
 config DRM_I915_SELFTEST
 	bool "Enable selftests upon driver load"
 	depends on DRM_I915
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index e837084..1271221 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -24,6 +24,12 @@
 #include "i915_drv.h"
 #include "intel_guc_ct.h"
 
+#ifdef CONFIG_DRM_I915_DEBUG_GUC
+#define CT_DEBUG_DRIVER(...)	DRM_DEBUG_DRIVER(__VA_ARGS__)
+#else
+#define CT_DEBUG_DRIVER(...)
+#endif
+
 struct ct_request {
 	struct list_head link;
 	u32 fence;
@@ -78,8 +84,8 @@ static inline const char *guc_ct_buffer_type_to_str(u32 type)
 static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc,
 				    u32 cmds_addr, u32 size, u32 owner)
 {
-	DRM_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
-			 desc, cmds_addr, size, owner);
+	CT_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
+			desc, cmds_addr, size, owner);
 	memset(desc, 0, sizeof(*desc));
 	desc->addr = cmds_addr;
 	desc->size = size;
@@ -88,8 +94,8 @@ static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc,
 
 static void guc_ct_buffer_desc_reset(struct guc_ct_buffer_desc *desc)
 {
-	DRM_DEBUG_DRIVER("CT: desc %p reset head=%u tail=%u\n",
-			 desc, desc->head, desc->tail);
+	CT_DEBUG_DRIVER("CT: desc %p reset head=%u tail=%u\n",
+			desc, desc->head, desc->tail);
 	desc->head = 0;
 	desc->tail = 0;
 	desc->is_in_error = 0;
@@ -185,8 +191,8 @@ static int ctch_init(struct intel_guc *guc,
 		err = PTR_ERR(blob);
 		goto err_vma;
 	}
-	DRM_DEBUG_DRIVER("CT: vma base=%#x\n",
-			 intel_guc_ggtt_offset(guc, ctch->vma));
+	CT_DEBUG_DRIVER("CT: vma base=%#x\n",
+			intel_guc_ggtt_offset(guc, ctch->vma));
 
 	/* store pointers to desc and cmds */
 	for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
@@ -200,8 +206,8 @@ static int ctch_init(struct intel_guc *guc,
 err_vma:
 	i915_vma_unpin_and_release(&ctch->vma);
 err_out:
-	DRM_DEBUG_DRIVER("CT: channel %d initialization failed; err=%d\n",
-			 ctch->owner, err);
+	CT_DEBUG_DRIVER("CT: channel %d initialization failed; err=%d\n",
+			ctch->owner, err);
 	return err;
 }
 
@@ -221,8 +227,8 @@ static int ctch_open(struct intel_guc *guc,
 	int err;
 	int i;
 
-	DRM_DEBUG_DRIVER("CT: channel %d reopen=%s\n",
-			 ctch->owner, yesno(ctch_is_open(ctch)));
+	CT_DEBUG_DRIVER("CT: channel %d reopen=%s\n",
+			ctch->owner, yesno(ctch_is_open(ctch)));
 
 	if (!ctch->vma) {
 		err = ctch_init(guc, ctch);
@@ -355,6 +361,10 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 		 (want_response ? GUC_CT_MSG_SEND_STATUS : 0) |
 		 (action[0] << GUC_CT_MSG_ACTION_SHIFT);
 
+	CT_DEBUG_DRIVER("CT: writing %*phn %*phn %*phn\n",
+			4, &header, 4, &fence,
+			4 * (len - 1), &action[1]);
+
 	cmds[tail] = header;
 	tail = (tail + 1) % size;
 
@@ -545,6 +555,9 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
 	if (unlikely(ret < 0)) {
 		DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
 			  action[0], ret, status);
+	} else if (unlikely(ret)) {
+		CT_DEBUG_DRIVER("CT: send action %#x returned %d (%#x)\n",
+				action[0], ret, ret);
 	}
 
 	mutex_unlock(&guc->send_mutex);
@@ -591,6 +604,7 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
 	/* beware of buffer wrap case */
 	if (unlikely(available < 0))
 		available += size;
+	CT_DEBUG_DRIVER("CT: available %d (%u:%u)\n", available, head, tail);
 	GEM_BUG_ON(available < 0);
 
 	data[0] = cmds[head];
@@ -612,6 +626,7 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
 		data[i] = cmds[head];
 		head = (head + 1) % size;
 	}
+	CT_DEBUG_DRIVER("CT: received %*phn\n", 4 * len, data);
 
 	desc->head = head * 4;
 	return 0;
@@ -665,11 +680,13 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
 		return -EPROTO;
 	}
 
+	CT_DEBUG_DRIVER("CT: response fence %u status %#x\n", fence, status);
+
 	spin_lock(&ct->lock);
 	list_for_each_entry(req, &ct->pending_requests, link) {
 		if (unlikely(fence != req->fence)) {
-			DRM_DEBUG_DRIVER("CT: request %u awaits response\n",
-					 req->fence);
+			CT_DEBUG_DRIVER("CT: request %u awaits response\n",
+					req->fence);
 			continue;
 		}
 		if (unlikely(datalen > req->response_len)) {
@@ -696,6 +713,8 @@ static void ct_process_request(struct intel_guc_ct *ct,
 {
 	struct intel_guc *guc = ct_to_guc(ct);
 
+	CT_DEBUG_DRIVER("CT: request %x %*phn\n", action, 4 * len, payload);
+
 	switch (action) {
 	case INTEL_GUC_ACTION_DEFAULT:
 		if (unlikely(len < 1))
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* [PATCH v5 12/12] HAX: Enable GuC for CI
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (10 preceding siblings ...)
  2018-03-26 19:48 ` [PATCH v5 11/12] drm/i915/guc: Trace messages from CT while in debug Michal Wajdeczko
@ 2018-03-26 19:48 ` Michal Wajdeczko
  2018-03-26 20:23 ` ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Support for Guc responses and requests (rev3) Patchwork
                   ` (9 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-26 19:48 UTC (permalink / raw)
  To: intel-gfx

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@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 c963603..53037b5 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -47,7 +47,7 @@
 	param(int, disable_power_well, -1) \
 	param(int, enable_ips, 1) \
 	param(int, invert_brightness, 0) \
-	param(int, enable_guc, 0) \
+	param(int, enable_guc, -1) \
 	param(int, guc_log_level, -1) \
 	param(char *, guc_firmware_path, NULL) \
 	param(char *, huc_firmware_path, NULL) \
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Support for Guc responses and requests (rev3)
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (11 preceding siblings ...)
  2018-03-26 19:48 ` [PATCH v5 12/12] HAX: Enable GuC for CI Michal Wajdeczko
@ 2018-03-26 20:23 ` Patchwork
  2018-03-26 20:36 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (8 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2018-03-26 20:23 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc: Support for Guc responses and requests (rev3)
URL   : https://patchwork.freedesktop.org/series/28393/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915/guc: Add documentation for MMIO based communication
Okay!

Commit: drm/i915/guc: Add support for data reporting in GuC responses
Okay!

Commit: drm/i915/guc: Prepare send() function to accept bigger response
Okay!

Commit: drm/i915/guc: Implement response handling in send_mmio()
Okay!

Commit: drm/i915/guc: Make event handler a virtual function
Okay!

Commit: drm/i915/guc: Prepare to handle messages from CT RECV buffer
Okay!

Commit: drm/i915/guc: Use better name for helper wait function
Okay!

Commit: drm/i915/guc: Implement response handling in send_ct()
Okay!

Commit: drm/i915/guc: Prepare to process incoming requests from CT
Okay!

Commit: drm/i915/guc: Handle default action received over CT
Okay!

Commit: drm/i915/guc: Trace messages from CT while in debug
+Error in reading or end of file.

Commit: HAX: Enable GuC for CI
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/guc: Support for Guc responses and requests (rev3)
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (12 preceding siblings ...)
  2018-03-26 20:23 ` ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Support for Guc responses and requests (rev3) Patchwork
@ 2018-03-26 20:36 ` Patchwork
  2018-03-26 22:11 ` ✗ Fi.CI.IGT: failure " Patchwork
                   ` (7 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2018-03-26 20:36 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc: Support for Guc responses and requests (rev3)
URL   : https://patchwork.freedesktop.org/series/28393/
State : success

== Summary ==

Series 28393v3 drm/i915/guc: Support for Guc responses and requests
https://patchwork.freedesktop.org/api/1.0/series/28393/revisions/3/mbox/

---- Known issues:

Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                pass       -> FAIL       (fi-gdg-551) fdo#102575
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                dmesg-warn -> PASS       (fi-cnl-y3) fdo#103191

fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575
fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191

fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:443s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:382s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:541s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:298s
fi-bxt-dsi       total:285  pass:255  dwarn:0   dfail:0   fail:0   skip:30  time:514s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:516s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:520s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:509s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:414s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:512s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:584s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:431s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:322s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:538s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:406s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:430s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:470s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:428s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:476s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:468s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:513s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:665s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:444s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:536s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:511s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:493s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:432s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:448s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:590s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:402s
Blacklisted hosts:
fi-cfl-s3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:570s
fi-cnl-psr       total:224  pass:198  dwarn:0   dfail:0   fail:1   skip:24 
fi-glk-j4005     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:485s

89eddd507c0ef264c37110f485c78cf9138c8be0 drm-tip: 2018y-03m-26d-19h-50m-03s UTC integration manifest
f032db5c2fe5 HAX: Enable GuC for CI
5cb4377932b0 drm/i915/guc: Trace messages from CT while in debug
ce08f71b0a81 drm/i915/guc: Handle default action received over CT
6ee0ce3642da drm/i915/guc: Prepare to process incoming requests from CT
851d45c0d1ac drm/i915/guc: Implement response handling in send_ct()
3ce83cb4aec2 drm/i915/guc: Use better name for helper wait function
9a9a5c5fc140 drm/i915/guc: Prepare to handle messages from CT RECV buffer
70aeb9589bb1 drm/i915/guc: Make event handler a virtual function
d17930ce159d drm/i915/guc: Implement response handling in send_mmio()
a14df2315765 drm/i915/guc: Prepare send() function to accept bigger response
d81e70189827 drm/i915/guc: Add support for data reporting in GuC responses
89147dab30ef drm/i915/guc: Add documentation for MMIO based communication

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8495/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* ✗ Fi.CI.IGT: failure for drm/i915/guc: Support for Guc responses and requests (rev3)
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (13 preceding siblings ...)
  2018-03-26 20:36 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-03-26 22:11 ` Patchwork
  2018-03-27 13:36 ` ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Support for Guc responses and requests (rev4) Patchwork
                   ` (6 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2018-03-26 22:11 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc: Support for Guc responses and requests (rev3)
URL   : https://patchwork.freedesktop.org/series/28393/
State : failure

== Summary ==

---- Possible new issues:

Test gem_eio:
        Subgroup execbuf:
                pass       -> INCOMPLETE (shard-apl)
        Subgroup throttle:
                pass       -> INCOMPLETE (shard-apl)
Test perf:
        Subgroup gen8-unprivileged-single-ctx-counters:
                pass       -> FAIL       (shard-apl)

---- Known issues:

Test drv_missed_irq:
                pass       -> SKIP       (shard-apl) fdo#103199
Test kms_cursor_legacy:
        Subgroup 2x-long-flip-vs-cursor-atomic:
                fail       -> PASS       (shard-hsw) fdo#104873
Test kms_flip:
        Subgroup plain-flip-ts-check:
                pass       -> FAIL       (shard-hsw) fdo#100368 +3

fdo#103199 https://bugs.freedesktop.org/show_bug.cgi?id=103199
fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368

shard-apl        total:3483 pass:1818 dwarn:1   dfail:0   fail:11  skip:1650 time:12621s
shard-hsw        total:3495 pass:1782 dwarn:1   dfail:0   fail:2   skip:1709 time:11735s
shard-snb        total:3495 pass:1374 dwarn:1   dfail:0   fail:3   skip:2117 time:6976s
Blacklisted hosts:
shard-kbl        total:3165 pass:1704 dwarn:22  dfail:0   fail:13  skip:1418 time:7205s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8495/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 01/12] drm/i915/guc: Add documentation for MMIO based communication
  2018-03-26 19:48 ` [PATCH v5 01/12] drm/i915/guc: Add documentation for MMIO based communication Michal Wajdeczko
@ 2018-03-27 10:05   ` Sagar Arun Kamble
  2018-03-27 11:37     ` Michal Wajdeczko
  0 siblings, 1 reply; 35+ messages in thread
From: Sagar Arun Kamble @ 2018-03-27 10:05 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx



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 <michal.wajdeczko@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Kelvin Gardiner <kelvin.gardiner@intel.com>
> Reviewed-by: Michel Thierry <michel.thierry@intel.com> #1
> ---
>
<snip>
>   }
> 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		(0xFFFF << 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.
> - */
> -#define	INTEL_GUC_RECV_MASK	((u32)0xF0000000)
> -#define	INTEL_GUC_RECV_IS_RESPONSE(x)	((u32)(x) >= INTEL_GUC_RECV_MASK)
> -#define	INTEL_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(0x0000F000)
> +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) \
> +	 (typecheck(u32, (m)) && \
> +	  ((m) & (INTEL_GUC_MSG_TYPE_MASK | INTEL_GUC_MSG_CODE_MASK)) == \
> +	  ((INTEL_GUC_MSG_TYPE_RESPONSE << INTEL_GUC_MSG_TYPE_SHIFT) | \
> +	   (INTEL_GUC_RESPONSE_STATUS_SUCCESS << INTEL_GUC_MSG_CODE_SHIFT)))
> +
>   /* This action will be programmed in C1BC - SOFT_SCRATCH_15_REG */
>   enum intel_guc_recv_message {
>   	INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED = BIT(1),

-- 
Thanks,
Sagar

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 01/12] drm/i915/guc: Add documentation for MMIO based communication
  2018-03-27 10:05   ` Sagar Arun Kamble
@ 2018-03-27 11:37     ` Michal Wajdeczko
  0 siblings, 0 replies; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-27 11:37 UTC (permalink / raw)
  To: intel-gfx, Sagar Arun Kamble

On Tue, 27 Mar 2018 12:05:21 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> 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 <michal.wajdeczko@intel.com>
>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Cc: Kelvin Gardiner <kelvin.gardiner@intel.com>
>> Reviewed-by: Michel Thierry <michel.thierry@intel.com> #1
>> ---
>>
> <snip>
>>   }
>> 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		(0xFFFF << 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.
>> - */
>> -#define	INTEL_GUC_RECV_MASK	((u32)0xF0000000)
>> -#define	INTEL_GUC_RECV_IS_RESPONSE(x)	((u32)(x) >= INTEL_GUC_RECV_MASK)
>> -#define	INTEL_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(0x0000F000)
>> +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) \
>> +	 (typecheck(u32, (m)) && \
>> +	  ((m) & (INTEL_GUC_MSG_TYPE_MASK | INTEL_GUC_MSG_CODE_MASK)) == \
>> +	  ((INTEL_GUC_MSG_TYPE_RESPONSE << INTEL_GUC_MSG_TYPE_SHIFT) | \
>> +	   (INTEL_GUC_RESPONSE_STATUS_SUCCESS << INTEL_GUC_MSG_CODE_SHIFT)))
>> +
>>   /* This action will be programmed in C1BC - SOFT_SCRATCH_15_REG */
>>   enum intel_guc_recv_message {
>>   	INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED = BIT(1),
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v6 08/12] drm/i915/guc: Implement response handling in send_ct()
  2018-03-26 19:48 ` [PATCH v5 08/12] drm/i915/guc: Implement response handling in send_ct() Michal Wajdeczko
@ 2018-03-27 12:14   ` Michal Wajdeczko
  0 siblings, 0 replies; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-27 12:14 UTC (permalink / raw)
  To: intel-gfx; +Cc: Jani Nikula

Instead of returning small data in response status dword,
GuC may append longer data as response message payload.
If caller provides response buffer, we will copy received
data and use number of received data dwords as new success
return value. We will WARN if response from GuC does not
match caller expectation.

v2: fix timeout and checkpatch warnings (Michal)
v3: fix checkpatch again (Michel)
    update wait function name (Michal)
    no need for spinlock_irqsave (MichalWi)
    no magic numbers (MichalWi)
    must check before use (Jani)
    add some more documentation (Michal)
v4: update documentation (Michal)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Cc: Michel Thierry <michel.thierry@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com> #2.5
Cc: Michal Winiarski <michal.winiarski@intel.com>
Cc: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_ct.c   | 142 ++++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_guc_ct.h   |   6 ++
 drivers/gpu/drm/i915/intel_guc_fwif.h |  52 +++++++++++++
 3 files changed, 186 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index 2d805a2..41b071c 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -24,6 +24,14 @@
 #include "i915_drv.h"
 #include "intel_guc_ct.h"
 
+struct ct_request {
+	struct list_head link;
+	u32 fence;
+	u32 status;
+	u32 response_len;
+	u32 *response_buf;
+};
+
 enum { CTB_SEND = 0, CTB_RECV = 1 };
 
 enum { CTB_OWNER_HOST = 0 };
@@ -36,6 +44,9 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 {
 	/* we're using static channel owners */
 	ct->host_channel.owner = CTB_OWNER_HOST;
+
+	spin_lock_init(&ct->lock);
+	INIT_LIST_HEAD(&ct->pending_requests);
 }
 
 static inline struct intel_guc *ct_to_guc(struct intel_guc_ct *ct)
@@ -294,7 +305,8 @@ static u32 ctch_get_next_fence(struct intel_guc_ct_channel *ctch)
 static int ctb_write(struct intel_guc_ct_buffer *ctb,
 		     const u32 *action,
 		     u32 len /* in dwords */,
-		     u32 fence)
+		     u32 fence,
+		     bool want_response)
 {
 	struct guc_ct_buffer_desc *desc = ctb->desc;
 	u32 head = desc->head / 4;	/* in dwords */
@@ -331,6 +343,7 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 	 */
 	header = (len << GUC_CT_MSG_LEN_SHIFT) |
 		 (GUC_CT_MSG_WRITE_FENCE_TO_DESC) |
+		 (want_response ? GUC_CT_MSG_SEND_STATUS : 0) |
 		 (action[0] << GUC_CT_MSG_ACTION_SHIFT);
 
 	cmds[tail] = header;
@@ -401,36 +414,108 @@ static int wait_for_ctb_desc_update(struct guc_ct_buffer_desc *desc,
 	return err;
 }
 
-static int ctch_send(struct intel_guc *guc,
+/**
+ * wait_for_ct_request_update - Wait for CT request state update.
+ * @req:	pointer to pending request
+ * @status:	placeholder for status
+ *
+ * For each sent request, Guc shall send bac CT response message.
+ * Our message handler will update status of tracked request once
+ * response message with given fence is received. Wait here and
+ * check for valid response status value.
+ *
+ * Return:
+ * *	0 response received (status is valid)
+ * *	-ETIMEDOUT no response within hardcoded timeout
+ */
+static int wait_for_ct_request_update(struct ct_request *req, u32 *status)
+{
+	int err;
+
+	/*
+	 * Fast commands should complete in less than 10us, so sample quickly
+	 * up to that length of time, then switch to a slower sleep-wait loop.
+	 * No GuC command should ever take longer than 10ms.
+	 */
+#define done INTEL_GUC_MSG_IS_RESPONSE(READ_ONCE(req->status))
+	err = wait_for_us(done, 10);
+	if (err)
+		err = wait_for(done, 10);
+#undef done
+
+	if (unlikely(err))
+		DRM_ERROR("CT: fence %u err %d\n", req->fence, err);
+
+	*status = req->status;
+	return err;
+}
+
+static int ctch_send(struct intel_guc_ct *ct,
 		     struct intel_guc_ct_channel *ctch,
 		     const u32 *action,
 		     u32 len,
+		     u32 *response_buf,
+		     u32 response_buf_size,
 		     u32 *status)
 {
 	struct intel_guc_ct_buffer *ctb = &ctch->ctbs[CTB_SEND];
 	struct guc_ct_buffer_desc *desc = ctb->desc;
+	struct ct_request request;
+	unsigned long flags;
 	u32 fence;
 	int err;
 
 	GEM_BUG_ON(!ctch_is_open(ctch));
 	GEM_BUG_ON(!len);
 	GEM_BUG_ON(len & ~GUC_CT_MSG_LEN_MASK);
+	GEM_BUG_ON(!response_buf && response_buf_size);
 
 	fence = ctch_get_next_fence(ctch);
-	err = ctb_write(ctb, action, len, fence);
+	request.fence = fence;
+	request.status = 0;
+	request.response_len = response_buf_size;
+	request.response_buf = response_buf;
+
+	spin_lock_irqsave(&ct->lock, flags);
+	list_add_tail(&request.link, &ct->pending_requests);
+	spin_unlock_irqrestore(&ct->lock, flags);
+
+	err = ctb_write(ctb, action, len, fence, !!response_buf);
 	if (unlikely(err))
-		return err;
+		goto unlink;
 
-	intel_guc_notify(guc);
+	intel_guc_notify(ct_to_guc(ct));
 
-	err = wait_for_ctb_desc_update(desc, fence, status);
+	if (response_buf)
+		err = wait_for_ct_request_update(&request, status);
+	else
+		err = wait_for_ctb_desc_update(desc, fence, status);
 	if (unlikely(err))
-		return err;
-	if (!INTEL_GUC_MSG_IS_RESPONSE_SUCCESS(*status))
-		return -EIO;
+		goto unlink;
+
+	if (!INTEL_GUC_MSG_IS_RESPONSE_SUCCESS(*status)) {
+		err = -EIO;
+		goto unlink;
+	}
+
+	if (response_buf) {
+		/* There shall be no data in the status */
+		WARN_ON(INTEL_GUC_MSG_TO_DATA(request.status));
+		/* Return actual response len */
+		err = request.response_len;
+	} else {
+		/* There shall be no response payload */
+		WARN_ON(request.response_len);
+		/* Return data decoded from the status dword */
+		err = INTEL_GUC_MSG_TO_DATA(*status);
+	}
 
-	/* Use data from the GuC status as our return value */
-	return INTEL_GUC_MSG_TO_DATA(*status);
+unlink:
+	spin_lock_irqsave(&ct->lock, flags);
+	list_del(&request.link);
+	spin_unlock_irqrestore(&ct->lock, flags);
+
+	return err;
 }
 
 /*
@@ -439,13 +524,15 @@ static int ctch_send(struct intel_guc *guc,
 static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len,
 			     u32 *response_buf, u32 response_buf_size)
 {
-	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
+	struct intel_guc_ct *ct = &guc->ct;
+	struct intel_guc_ct_channel *ctch = &ct->host_channel;
 	u32 status = ~0; /* undefined */
 	int ret;
 
 	mutex_lock(&guc->send_mutex);
 
-	ret = ctch_send(guc, ctch, action, len, &status);
+	ret = ctch_send(ct, ctch, action, len, response_buf, response_buf_size,
+			&status);
 	if (unlikely(ret < 0)) {
 		DRM_ERROR("CT: send action %#X failed; err=%d status=%#X\n",
 			  action[0], ret, status);
@@ -546,8 +633,12 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
 	u32 msglen = len + 1; /* total message length including header */
 	u32 fence;
 	u32 status;
+	u32 datalen;
+	struct ct_request *req;
+	bool found = false;
 
 	GEM_BUG_ON(!ct_header_is_response(header));
+	GEM_BUG_ON(!in_irq());
 
 	/* Response payload shall at least include fence and status */
 	if (unlikely(len < 2)) {
@@ -557,6 +648,7 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
 
 	fence = msg[1];
 	status = msg[2];
+	datalen = len - 2;
 
 	/* Format of the status follows RESPONSE message */
 	if (unlikely(!INTEL_GUC_MSG_IS_RESPONSE(status))) {
@@ -564,7 +656,29 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
 		return -EPROTO;
 	}
 
-	/* XXX */
+	spin_lock(&ct->lock);
+	list_for_each_entry(req, &ct->pending_requests, link) {
+		if (unlikely(fence != req->fence)) {
+			DRM_DEBUG_DRIVER("CT: request %u awaits response\n",
+					 req->fence);
+			continue;
+		}
+		if (unlikely(datalen > req->response_len)) {
+			DRM_ERROR("CT: response %u too long %*phn\n",
+				  req->fence, 4 * msglen, msg);
+			datalen = 0;
+		}
+		if (datalen)
+			memcpy(req->response_buf, msg + 3, 4 * datalen);
+		req->response_len = datalen;
+		WRITE_ONCE(req->status, status);
+		found = true;
+		break;
+	}
+	spin_unlock(&ct->lock);
+
+	if (!found)
+		DRM_ERROR("CT: unsolicited response %*phn\n", 4 * msglen, msg);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
index 595c8ad..fac6e53 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/intel_guc_ct.h
@@ -75,6 +75,12 @@ struct intel_guc_ct_channel {
 struct intel_guc_ct {
 	struct intel_guc_ct_channel host_channel;
 	/* other channels are tbd */
+
+	/** @lock: protects pending requests list */
+	spinlock_t lock;
+
+	/** @pending_requests: list of requests waiting for response */
+	struct list_head pending_requests;
 };
 
 void intel_guc_ct_init_early(struct intel_guc_ct *ct);
diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 83143e8..d73673f 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -327,6 +327,58 @@ struct guc_stage_desc {
 	u64 desc_private;
 } __packed;
 
+/**
+ * DOC: CTB based communication
+ *
+ * The CTB (command transport buffer) communication between Host and GuC
+ * is based on u32 data stream written to the shared buffer. One buffer can
+ * be used to transmit data only in one direction (one-directional channel).
+ *
+ * Current status of the each buffer is stored in the buffer descriptor.
+ * Buffer descriptor holds tail and head fields that represents active data
+ * stream. The tail field is updated by the data producer (sender), and head
+ * field is updated by the data consumer (receiver)::
+ *
+ *      +------------+
+ *      | DESCRIPTOR |          +=================+============+========+
+ *      +============+          |                 | MESSAGE(s) |        |
+ *      | address    |--------->+=================+============+========+
+ *      +------------+
+ *      | head       |          ^-----head--------^
+ *      +------------+
+ *      | tail       |          ^---------tail-----------------^
+ *      +------------+
+ *      | size       |          ^---------------size--------------------^
+ *      +------------+
+ *
+ * Each message in data stream starts with the single u32 treated as a header,
+ * followed by optional set of u32 data that makes message specific payload::
+ *
+ *      +------------+---------+---------+---------+
+ *      |         MESSAGE                          |
+ *      +------------+---------+---------+---------+
+ *      |   msg[0]   |   [1]   |   ...   |  [n-1]  |
+ *      +------------+---------+---------+---------+
+ *      |   MESSAGE  |       MESSAGE PAYLOAD       |
+ *      +   HEADER   +---------+---------+---------+
+ *      |            |    0    |   ...   |    n    |
+ *      +======+=====+=========+=========+=========+
+ *      | 31:16| code|         |         |         |
+ *      +------+-----+         |         |         |
+ *      |  15:5|flags|         |         |         |
+ *      +------+-----+         |         |         |
+ *      |   4:0|  len|         |         |         |
+ *      +------+-----+---------+---------+---------+
+ *
+ *                   ^-------------len-------------^
+ *
+ * The message header consists of:
+ *
+ * - **len**, indicates length of the message payload (in u32)
+ * - **code**, indicates message code
+ * - **flags**, holds various bits to control message handling
+ */
+
 /*
  * Describes single command transport buffer.
  * Used by both guc-master and clients.
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Support for Guc responses and requests (rev4)
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (14 preceding siblings ...)
  2018-03-26 22:11 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2018-03-27 13:36 ` Patchwork
  2018-03-27 13:48 ` ✓ Fi.CI.BAT: success " Patchwork
                   ` (5 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2018-03-27 13:36 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc: Support for Guc responses and requests (rev4)
URL   : https://patchwork.freedesktop.org/series/28393/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915/guc: Add documentation for MMIO based communication
Okay!

Commit: drm/i915/guc: Add support for data reporting in GuC responses
Okay!

Commit: drm/i915/guc: Prepare send() function to accept bigger response
Okay!

Commit: drm/i915/guc: Implement response handling in send_mmio()
Okay!

Commit: drm/i915/guc: Make event handler a virtual function
Okay!

Commit: drm/i915/guc: Prepare to handle messages from CT RECV buffer
Okay!

Commit: drm/i915/guc: Use better name for helper wait function
Okay!

Commit: drm/i915/guc: Implement response handling in send_ct()
Okay!

Commit: drm/i915/guc: Prepare to process incoming requests from CT
Okay!

Commit: drm/i915/guc: Handle default action received over CT
Okay!

Commit: drm/i915/guc: Trace messages from CT while in debug
+Error in reading or end of file.

Commit: HAX: Enable GuC for CI
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/guc: Support for Guc responses and requests (rev4)
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (15 preceding siblings ...)
  2018-03-27 13:36 ` ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Support for Guc responses and requests (rev4) Patchwork
@ 2018-03-27 13:48 ` Patchwork
  2018-03-27 17:35 ` ✗ Fi.CI.IGT: failure " Patchwork
                   ` (4 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2018-03-27 13:48 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc: Support for Guc responses and requests (rev4)
URL   : https://patchwork.freedesktop.org/series/28393/
State : success

== Summary ==

Series 28393v4 drm/i915/guc: Support for Guc responses and requests
https://patchwork.freedesktop.org/api/1.0/series/28393/revisions/4/mbox/

---- Known issues:

Test gem_exec_suspend:
        Subgroup basic-s3:
                dmesg-warn -> PASS       (fi-skl-6700k2) fdo#104108
Test gem_mmap_gtt:
        Subgroup basic-small-bo-tiledx:
                fail       -> PASS       (fi-gdg-551) fdo#102575

fdo#104108 https://bugs.freedesktop.org/show_bug.cgi?id=104108
fdo#102575 https://bugs.freedesktop.org/show_bug.cgi?id=102575

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:432s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:441s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:380s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:539s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:298s
fi-bxt-dsi       total:285  pass:255  dwarn:0   dfail:0   fail:0   skip:30  time:516s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:521s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:523s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:511s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:416s
fi-cfl-s3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:571s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:513s
fi-cnl-y3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:591s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:428s
fi-gdg-551       total:285  pass:177  dwarn:0   dfail:0   fail:0   skip:108 time:322s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:539s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:402s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:421s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:476s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:432s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:476s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:468s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:515s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:665s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:447s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:532s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:508s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:501s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:428s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:444s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:589s
fi-snb-2600      total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:401s
Blacklisted hosts:
fi-cnl-psr       total:285  pass:256  dwarn:3   dfail:0   fail:0   skip:26  time:519s
fi-glk-j4005     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:485s

1a95bcd16cb9db1eca19346e911fea27ab5d1dc4 drm-tip: 2018y-03m-27d-12h-36m-37s UTC integration manifest
1ebea50f3714 HAX: Enable GuC for CI
6cf9f23d08da drm/i915/guc: Trace messages from CT while in debug
e483ca83c16f drm/i915/guc: Handle default action received over CT
9929b8a0a514 drm/i915/guc: Prepare to process incoming requests from CT
76c7ae511fbb drm/i915/guc: Implement response handling in send_ct()
8f0de3072e13 drm/i915/guc: Use better name for helper wait function
6a848e5c75d4 drm/i915/guc: Prepare to handle messages from CT RECV buffer
68a6483fd201 drm/i915/guc: Make event handler a virtual function
689b8276f19b drm/i915/guc: Implement response handling in send_mmio()
6526f39007de drm/i915/guc: Prepare send() function to accept bigger response
d9409b06a463 drm/i915/guc: Add support for data reporting in GuC responses
6df3882e6146 drm/i915/guc: Add documentation for MMIO based communication

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8504/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* ✗ Fi.CI.IGT: failure for drm/i915/guc: Support for Guc responses and requests (rev4)
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (16 preceding siblings ...)
  2018-03-27 13:48 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-03-27 17:35 ` Patchwork
  2018-03-27 20:54 ` [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michel Thierry
                   ` (3 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2018-03-27 17:35 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc: Support for Guc responses and requests (rev4)
URL   : https://patchwork.freedesktop.org/series/28393/
State : failure

== Summary ==

---- Possible new issues:

Test gem_eio:
        Subgroup execbuf:
                pass       -> INCOMPLETE (shard-apl)
        Subgroup throttle:
                pass       -> INCOMPLETE (shard-apl)
Test kms_frontbuffer_tracking:
        Subgroup fbc-1p-primscrn-spr-indfb-draw-pwrite:
                fail       -> PASS       (shard-apl)
Test perf:
        Subgroup gen8-unprivileged-single-ctx-counters:
                pass       -> FAIL       (shard-apl)

---- Known issues:

Test drv_missed_irq:
                pass       -> SKIP       (shard-apl) fdo#103199
Test kms_flip:
        Subgroup flip-vs-expired-vblank-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#102887
Test kms_sysfs_edid_timing:
                warn       -> PASS       (shard-apl) fdo#100047

fdo#103199 https://bugs.freedesktop.org/show_bug.cgi?id=103199
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047

shard-apl        total:3483 pass:1819 dwarn:1   dfail:0   fail:11  skip:1650 time:12588s
shard-hsw        total:3495 pass:1782 dwarn:1   dfail:0   fail:2   skip:1709 time:11678s
shard-snb        total:3495 pass:1374 dwarn:1   dfail:0   fail:3   skip:2117 time:6962s
Blacklisted hosts:
shard-kbl        total:3143 pass:1719 dwarn:1   dfail:0   fail:11  skip:1404 time:7156s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8504/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 10/12] drm/i915/guc: Handle default action received over CT
  2018-03-26 19:48 ` [PATCH v5 10/12] drm/i915/guc: Handle default action received over CT Michal Wajdeczko
@ 2018-03-27 18:25   ` Daniele Ceraolo Spurio
  2018-03-27 20:03     ` Michel Thierry
  2018-03-27 21:41   ` [PATCH v7 " Michal Wajdeczko
  1 sibling, 1 reply; 35+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-03-27 18:25 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx



On 26/03/18 12:48, Michal Wajdeczko wrote:
> When running on platform with CTB based GuC communication enabled,
> GuC to Host event data will be delivered as CT request message.
> However, content of the data[1] of this CT message follows format
> of the scratch register used in MMIO based communication, so some
> code reuse is still possible.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Reviewed-by: Michel Thierry <michel.thierry@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc.c    | 5 +++++
>   drivers/gpu/drm/i915/intel_guc.h    | 1 +
>   drivers/gpu/drm/i915/intel_guc_ct.c | 9 +++++++++
>   3 files changed, 15 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index 118db81..b6d2778 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -416,6 +416,11 @@ void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc)
>   	I915_WRITE(SOFT_SCRATCH(15), val & ~msg);
>   	spin_unlock(&guc->irq_lock);
>   
> +	intel_guc_to_host_process_recv_msg(guc, msg);
> +}
> +
> +void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 msg)
> +{
>   	if (msg & (INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
>   		   INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED))
>   		intel_guc_log_handle_flush_event(&guc->log);
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 6dc109a..f1265e1 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -163,6 +163,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
>   void intel_guc_to_host_event_handler(struct intel_guc *guc);
>   void intel_guc_to_host_event_handler_nop(struct intel_guc *guc);
>   void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc);
> +void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 msg);
>   int intel_guc_sample_forcewake(struct intel_guc *guc);
>   int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
>   int intel_guc_suspend(struct intel_guc *guc);
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
> index aa810ad..e837084 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -694,8 +694,17 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
>   static void ct_process_request(struct intel_guc_ct *ct,
>   			       u32 action, u32 len, const u32 *payload)
>   {
> +	struct intel_guc *guc = ct_to_guc(ct);
> +
>   	switch (action) {
> +	case INTEL_GUC_ACTION_DEFAULT:
> +		if (unlikely(len < 1))
> +			goto fail_unexpected;

Don't we need to remove the non-enabled messages here? i.e. like we do 
for the mmio receive:

	msg = *payload & guc->msg_enabled_mask;

otherwise I think we'll try to process log interrupts even if the relay 
is not enabled.

Daniele

> +		intel_guc_to_host_process_recv_msg(guc, *payload);
> +		break;
> +
>   	default:
> +fail_unexpected:
>   		DRM_ERROR("CT: unexpected request %x %*phn\n",
>   			  action, 4 * len, payload);
>   		break;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 10/12] drm/i915/guc: Handle default action received over CT
  2018-03-27 18:25   ` Daniele Ceraolo Spurio
@ 2018-03-27 20:03     ` Michel Thierry
  2018-03-27 21:05       ` Michal Wajdeczko
  0 siblings, 1 reply; 35+ messages in thread
From: Michel Thierry @ 2018-03-27 20:03 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio, Michal Wajdeczko, intel-gfx

On 3/27/2018 11:25 AM, Daniele Ceraolo Spurio wrote:
> 
> 
> On 26/03/18 12:48, Michal Wajdeczko wrote:
>> When running on platform with CTB based GuC communication enabled,
>> GuC to Host event data will be delivered as CT request message.
>> However, content of the data[1] of this CT message follows format
>> of the scratch register used in MMIO based communication, so some
>> code reuse is still possible.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>> Reviewed-by: Michel Thierry <michel.thierry@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_guc.c    | 5 +++++
>>   drivers/gpu/drm/i915/intel_guc.h    | 1 +
>>   drivers/gpu/drm/i915/intel_guc_ct.c | 9 +++++++++
>>   3 files changed, 15 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc.c 
>> b/drivers/gpu/drm/i915/intel_guc.c
>> index 118db81..b6d2778 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.c
>> +++ b/drivers/gpu/drm/i915/intel_guc.c
>> @@ -416,6 +416,11 @@ void intel_guc_to_host_event_handler_mmio(struct 
>> intel_guc *guc)
>>       I915_WRITE(SOFT_SCRATCH(15), val & ~msg);
>>       spin_unlock(&guc->irq_lock);
>> +    intel_guc_to_host_process_recv_msg(guc, msg);
>> +}
>> +
>> +void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 msg)
>> +{
>>       if (msg & (INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
>>              INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED))
>>           intel_guc_log_handle_flush_event(&guc->log);
>> diff --git a/drivers/gpu/drm/i915/intel_guc.h 
>> b/drivers/gpu/drm/i915/intel_guc.h
>> index 6dc109a..f1265e1 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.h
>> +++ b/drivers/gpu/drm/i915/intel_guc.h
>> @@ -163,6 +163,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, 
>> const u32 *action, u32 len,
>>   void intel_guc_to_host_event_handler(struct intel_guc *guc);
>>   void intel_guc_to_host_event_handler_nop(struct intel_guc *guc);
>>   void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc);
>> +void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 msg);
>>   int intel_guc_sample_forcewake(struct intel_guc *guc);
>>   int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
>>   int intel_guc_suspend(struct intel_guc *guc);
>> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
>> b/drivers/gpu/drm/i915/intel_guc_ct.c
>> index aa810ad..e837084 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
>> @@ -694,8 +694,17 @@ static int ct_handle_response(struct intel_guc_ct 
>> *ct, const u32 *msg)
>>   static void ct_process_request(struct intel_guc_ct *ct,
>>                      u32 action, u32 len, const u32 *payload)
>>   {
>> +    struct intel_guc *guc = ct_to_guc(ct);
>> +
>>       switch (action) {
>> +    case INTEL_GUC_ACTION_DEFAULT:
>> +        if (unlikely(len < 1))
>> +            goto fail_unexpected;
> 
> Don't we need to remove the non-enabled messages here? i.e. like we do 
> for the mmio receive:
> 
>      msg = *payload & guc->msg_enabled_mask;
> 
> otherwise I think we'll try to process log interrupts even if the relay 
> is not enabled.

In Daniele's specific example, I still think guc_log_enable_flush_events 
should have been called (and don't do anything inside 
handle_flush_event, the goal was to not serve it).

But it's true that we should filter any unexpected incoming request (& 
guc->msg_enabled_mask), and any new user should be calling 
intel_guc_enable_msg during its setup.

Thanks,
> 
> Daniele
> 
>> +        intel_guc_to_host_process_recv_msg(guc, *payload);
>> +        break;
>> +
>>       default:
>> +fail_unexpected:
>>           DRM_ERROR("CT: unexpected request %x %*phn\n",
>>                 action, 4 * len, payload);
>>           break;
>>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 09/12] drm/i915/guc: Prepare to process incoming requests from CT
  2018-03-26 19:48 ` [PATCH v5 09/12] drm/i915/guc: Prepare to process incoming requests from CT Michal Wajdeczko
@ 2018-03-27 20:07   ` Michel Thierry
  0 siblings, 0 replies; 35+ messages in thread
From: Michel Thierry @ 2018-03-27 20:07 UTC (permalink / raw)
  To: Wajdeczko, Michal, intel-gfx

On 3/26/2018 12:48 PM, Wajdeczko, Michal wrote:
> Requests are read from CT in the irq handler, but actual processing
> will be done in the work thread. Processing of specific actions will
> be added in the upcoming patches.
> 
> v2: don't use GEM_BUG_ON (Chris)
>      don't kmalloc too large buffer (Michal)
> v3: rebased
> v4: don't name it 'dispatch' (Michel) and fix checkpatch
>      add some documentation (Michal)
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Cc: Michel Thierry <michel.thierry@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc_ct.c | 95 ++++++++++++++++++++++++++++++++++++-
>   drivers/gpu/drm/i915/intel_guc_ct.h |  6 +++
>   2 files changed, 100 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
> index 41b071c..aa810ad 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -32,10 +32,17 @@ struct ct_request {
>          u32 *response_buf;
>   };
> 
> +struct ct_incoming_request {
> +       struct list_head link;
> +       u32 msg[];
> +};
> +
>   enum { CTB_SEND = 0, CTB_RECV = 1 };
> 
>   enum { CTB_OWNER_HOST = 0 };
> 
> +static void ct_incoming_request_worker_func(struct work_struct *w);
> +
>   /**
>    * intel_guc_ct_init_early - Initialize CT state without requiring device access
>    * @ct: pointer to CT struct
> @@ -47,6 +54,8 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
> 
>          spin_lock_init(&ct->lock);
>          INIT_LIST_HEAD(&ct->pending_requests);
> +       INIT_LIST_HEAD(&ct->incoming_requests);
> +       INIT_WORK(&ct->worker, ct_incoming_request_worker_func);
>   }
> 
>   static inline struct intel_guc *ct_to_guc(struct intel_guc_ct *ct)
> @@ -682,13 +691,97 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
>          return 0;
>   }
> 
> +static void ct_process_request(struct intel_guc_ct *ct,
> +                              u32 action, u32 len, const u32 *payload)
> +{
> +       switch (action) {
> +       default:
> +               DRM_ERROR("CT: unexpected request %x %*phn\n",
> +                         action, 4 * len, payload);
> +               break;
> +       }
> +}
> +
> +static bool ct_process_incoming_requests(struct intel_guc_ct *ct)
> +{
> +       unsigned long flags;
> +       struct ct_incoming_request *request;
> +       u32 header;
> +       u32 *payload;
> +       bool done;
> +
> +       spin_lock_irqsave(&ct->lock, flags);
> +       request = list_first_entry_or_null(&ct->incoming_requests,
> +                                          struct ct_incoming_request, link);
> +       if (request)
> +               list_del(&request->link);
> +       done = !!list_empty(&ct->incoming_requests);
> +       spin_unlock_irqrestore(&ct->lock, flags);
> +
> +       if (!request)
> +               return true;
> +
> +       header = request->msg[0];
> +       payload = &request->msg[1];
> +       ct_process_request(ct,
> +                          ct_header_get_action(header),
> +                          ct_header_get_len(header),
> +                          payload);
> +
> +       kfree(request);
> +       return done;
> +}
> +
> +static void ct_incoming_request_worker_func(struct work_struct *w)
> +{
> +       struct intel_guc_ct *ct = container_of(w, struct intel_guc_ct, worker);
> +       bool done;
> +
> +       done = ct_process_incoming_requests(ct);
> +       if (!done)
> +               queue_work(system_unbound_wq, &ct->worker);
> +}
> +
> +/**
> + * DOC: CTB GuC to Host request
> + *
> + * Format of the CTB GuC to Host request message is as follows::
> + *
> + *      +------------+---------+---------+---------+---------+---------+
> + *      |   msg[0]   |   [1]   |   [2]   |   [3]   |   ...   |  [n-1]  |
> + *      +------------+---------+---------+---------+---------+---------+
> + *      |   MESSAGE  |       MESSAGE PAYLOAD                           |
> + *      +   HEADER   +---------+---------+---------+---------+---------+
> + *      |            |    0    |    1    |    2    |   ...   |    n    |
> + *      +============+=========+=========+=========+=========+=========+
> + *      |     len    |            request specific data                |
> + *      +------+-----+---------+---------+---------+---------+---------+
> + *
> + *                   ^-----------------------len-----------------------^
> + */
> +
>   static int ct_handle_request(struct intel_guc_ct *ct, const u32 *msg)
>   {
>          u32 header = msg[0];
> +       u32 len = ct_header_get_len(header);
> +       u32 msglen = len + 1; /* total message length including header */
> +       struct ct_incoming_request *request;
> +       unsigned long flags;
> 
>          GEM_BUG_ON(ct_header_is_response(header));
> 
> -       /* XXX */
> +       request = kmalloc(sizeof(*request) + 4 * msglen, GFP_ATOMIC);
> +       if (unlikely(!request)) {
> +               DRM_ERROR("CT: dropping request %*phn\n", 4 * msglen, msg);
> +               return 0; /* XXX: -ENOMEM ? */
> +       }
> +       memcpy(request->msg, msg, 4 * msglen);
> +
> +       spin_lock_irqsave(&ct->lock, flags);
> +       list_add_tail(&request->link, &ct->incoming_requests);
> +       spin_unlock_irqrestore(&ct->lock, flags);
> +
> +       queue_work(system_unbound_wq, &ct->worker);
>          return 0;
>   }
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
> index fac6e53..d774895a 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.h
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.h
> @@ -81,6 +81,12 @@ struct intel_guc_ct {
> 
>          /** @pending_requests: list of requests waiting for response */
>          struct list_head pending_requests;
> +
> +       /** @incoming_requests: list of incoming requests */
> +       struct list_head incoming_requests;
> +
> +       /** @worker: worker for handling incoming requests */
> +       struct work_struct worker;
>   };
> 
>   void intel_guc_ct_init_early(struct intel_guc_ct *ct);
> --
> 1.9.1
> 

Reviewed-by: Michel Thierry <michel.thierry@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (17 preceding siblings ...)
  2018-03-27 17:35 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2018-03-27 20:54 ` Michel Thierry
  2018-03-27 23:01 ` ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Support for Guc responses and requests (rev5) Patchwork
                   ` (2 subsequent siblings)
  21 siblings, 0 replies; 35+ messages in thread
From: Michel Thierry @ 2018-03-27 20:54 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx

On 3/26/2018 12:48 PM, Michal Wajdeczko wrote:
> With this series we will be able to receive more data from the Guc.
> New Guc firmwares will be required to actually use that feature.
> 
> v4: respin series after 1/2 year break
> v5: updated after review comments
> 
> Michal Wajdeczko (12):
>    drm/i915/guc: Add documentation for MMIO based communication
>    drm/i915/guc: Add support for data reporting in GuC responses
>    drm/i915/guc: Prepare send() function to accept bigger response
>    drm/i915/guc: Implement response handling in send_mmio()
>    drm/i915/guc: Make event handler a virtual function
>    drm/i915/guc: Prepare to handle messages from CT RECV buffer
>    drm/i915/guc: Use better name for helper wait function
>    drm/i915/guc: Implement response handling in send_ct()
>    drm/i915/guc: Prepare to process incoming requests from CT
>    drm/i915/guc: Handle default action received over CT
>    drm/i915/guc: Trace messages from CT while in debug
>    HAX: Enable GuC for CI
> 
>   drivers/gpu/drm/i915/Kconfig.debug    |  12 +
>   drivers/gpu/drm/i915/i915_params.h    |   2 +-
>   drivers/gpu/drm/i915/intel_guc.c      |  51 +++-
>   drivers/gpu/drm/i915/intel_guc.h      |  29 +-
>   drivers/gpu/drm/i915/intel_guc_ct.c   | 503 +++++++++++++++++++++++++++++++---
>   drivers/gpu/drm/i915/intel_guc_ct.h   |  12 +
>   drivers/gpu/drm/i915/intel_guc_fwif.h | 139 ++++++++--
>   drivers/gpu/drm/i915/intel_uc.c       |   2 +
>   8 files changed, 678 insertions(+), 72 deletions(-)
> 

Hi,

My r-b's still apply after addressing Daniele's comment in [v5,10/12].
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 10/12] drm/i915/guc: Handle default action received over CT
  2018-03-27 20:03     ` Michel Thierry
@ 2018-03-27 21:05       ` Michal Wajdeczko
  2018-03-27 21:17         ` Daniele Ceraolo Spurio
  0 siblings, 1 reply; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-27 21:05 UTC (permalink / raw)
  To: Daniele Ceraolo Spurio, intel-gfx, Michel Thierry

On Tue, 27 Mar 2018 22:03:23 +0200, Michel Thierry  
<michel.thierry@intel.com> wrote:

> On 3/27/2018 11:25 AM, Daniele Ceraolo Spurio wrote:
>>   On 26/03/18 12:48, Michal Wajdeczko wrote:
>>> When running on platform with CTB based GuC communication enabled,
>>> GuC to Host event data will be delivered as CT request message.
>>> However, content of the data[1] of this CT message follows format
>>> of the scratch register used in MMIO based communication, so some
>>> code reuse is still possible.
>>>
>>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>>> Reviewed-by: Michel Thierry <michel.thierry@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/intel_guc.c    | 5 +++++
>>>   drivers/gpu/drm/i915/intel_guc.h    | 1 +
>>>   drivers/gpu/drm/i915/intel_guc_ct.c | 9 +++++++++
>>>   3 files changed, 15 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_guc.c  
>>> b/drivers/gpu/drm/i915/intel_guc.c
>>> index 118db81..b6d2778 100644
>>> --- a/drivers/gpu/drm/i915/intel_guc.c
>>> +++ b/drivers/gpu/drm/i915/intel_guc.c
>>> @@ -416,6 +416,11 @@ void intel_guc_to_host_event_handler_mmio(struct  
>>> intel_guc *guc)
>>>       I915_WRITE(SOFT_SCRATCH(15), val & ~msg);
>>>       spin_unlock(&guc->irq_lock);
>>> +    intel_guc_to_host_process_recv_msg(guc, msg);
>>> +}
>>> +
>>> +void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32  
>>> msg)
>>> +{
>>>       if (msg & (INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
>>>              INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED))
>>>           intel_guc_log_handle_flush_event(&guc->log);
>>> diff --git a/drivers/gpu/drm/i915/intel_guc.h  
>>> b/drivers/gpu/drm/i915/intel_guc.h
>>> index 6dc109a..f1265e1 100644
>>> --- a/drivers/gpu/drm/i915/intel_guc.h
>>> +++ b/drivers/gpu/drm/i915/intel_guc.h
>>> @@ -163,6 +163,7 @@ int intel_guc_send_mmio(struct intel_guc *guc,  
>>> const u32 *action, u32 len,
>>>   void intel_guc_to_host_event_handler(struct intel_guc *guc);
>>>   void intel_guc_to_host_event_handler_nop(struct intel_guc *guc);
>>>   void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc);
>>> +void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32  
>>> msg);
>>>   int intel_guc_sample_forcewake(struct intel_guc *guc);
>>>   int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
>>>   int intel_guc_suspend(struct intel_guc *guc);
>>> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c  
>>> b/drivers/gpu/drm/i915/intel_guc_ct.c
>>> index aa810ad..e837084 100644
>>> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
>>> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
>>> @@ -694,8 +694,17 @@ static int ct_handle_response(struct intel_guc_ct  
>>> *ct, const u32 *msg)
>>>   static void ct_process_request(struct intel_guc_ct *ct,
>>>                      u32 action, u32 len, const u32 *payload)
>>>   {
>>> +    struct intel_guc *guc = ct_to_guc(ct);
>>> +
>>>       switch (action) {
>>> +    case INTEL_GUC_ACTION_DEFAULT:
>>> +        if (unlikely(len < 1))
>>> +            goto fail_unexpected;
>>  Don't we need to remove the non-enabled messages here? i.e. like we do  
>> for the mmio receive:
>>       msg = *payload & guc->msg_enabled_mask;
>>  otherwise I think we'll try to process log interrupts even if the  
>> relay is not enabled.
>
> In Daniele's specific example, I still think guc_log_enable_flush_events  
> should have been called (and don't do anything inside  
> handle_flush_event, the goal was to not serve it).
>
> But it's true that we should filter any unexpected incoming request (&  
> guc->msg_enabled_mask), and any new user should be calling  
> intel_guc_enable_msg during its setup.

hmm, my understanding was that purpose of msg_enabled_mask was to:

	 * Sample the log buffer flush related bits & clear them out now
	 * itself from the message identity register to minimize the
	 * probability of losing a flush interrupt, when there are back
	 * to back flush interrupts.

and is not applicable to messages sent over CTB, as we can't miss msg.

if you still believe that filtering is required, it should not be done
here, as purpose of this function is to verify message completeness and
call actual handler, but in intel_guc_to_host_process_recv_msg(), where
format of received data is known.

/m

>
> Thanks,
>>  Daniele
>>
>>> +        intel_guc_to_host_process_recv_msg(guc, *payload);
>>> +        break;
>>> +
>>>       default:
>>> +fail_unexpected:
>>>           DRM_ERROR("CT: unexpected request %x %*phn\n",
>>>                 action, 4 * len, payload);
>>>           break;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v5 10/12] drm/i915/guc: Handle default action received over CT
  2018-03-27 21:05       ` Michal Wajdeczko
@ 2018-03-27 21:17         ` Daniele Ceraolo Spurio
  0 siblings, 0 replies; 35+ messages in thread
From: Daniele Ceraolo Spurio @ 2018-03-27 21:17 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx, Michel Thierry



On 27/03/18 14:05, Michal Wajdeczko wrote:
> On Tue, 27 Mar 2018 22:03:23 +0200, Michel Thierry 
> <michel.thierry@intel.com> wrote:
> 
>> On 3/27/2018 11:25 AM, Daniele Ceraolo Spurio wrote:
>>>   On 26/03/18 12:48, Michal Wajdeczko wrote:
>>>> When running on platform with CTB based GuC communication enabled,
>>>> GuC to Host event data will be delivered as CT request message.
>>>> However, content of the data[1] of this CT message follows format
>>>> of the scratch register used in MMIO based communication, so some
>>>> code reuse is still possible.
>>>>
>>>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>>>> Reviewed-by: Michel Thierry <michel.thierry@intel.com>
>>>> ---
>>>>   drivers/gpu/drm/i915/intel_guc.c    | 5 +++++
>>>>   drivers/gpu/drm/i915/intel_guc.h    | 1 +
>>>>   drivers/gpu/drm/i915/intel_guc_ct.c | 9 +++++++++
>>>>   3 files changed, 15 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_guc.c 
>>>> b/drivers/gpu/drm/i915/intel_guc.c
>>>> index 118db81..b6d2778 100644
>>>> --- a/drivers/gpu/drm/i915/intel_guc.c
>>>> +++ b/drivers/gpu/drm/i915/intel_guc.c
>>>> @@ -416,6 +416,11 @@ void 
>>>> intel_guc_to_host_event_handler_mmio(struct intel_guc *guc)
>>>>       I915_WRITE(SOFT_SCRATCH(15), val & ~msg);
>>>>       spin_unlock(&guc->irq_lock);
>>>> +    intel_guc_to_host_process_recv_msg(guc, msg);
>>>> +}
>>>> +
>>>> +void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 
>>>> msg)
>>>> +{
>>>>       if (msg & (INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
>>>>              INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED))
>>>>           intel_guc_log_handle_flush_event(&guc->log);
>>>> diff --git a/drivers/gpu/drm/i915/intel_guc.h 
>>>> b/drivers/gpu/drm/i915/intel_guc.h
>>>> index 6dc109a..f1265e1 100644
>>>> --- a/drivers/gpu/drm/i915/intel_guc.h
>>>> +++ b/drivers/gpu/drm/i915/intel_guc.h
>>>> @@ -163,6 +163,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, 
>>>> const u32 *action, u32 len,
>>>>   void intel_guc_to_host_event_handler(struct intel_guc *guc);
>>>>   void intel_guc_to_host_event_handler_nop(struct intel_guc *guc);
>>>>   void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc);
>>>> +void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 
>>>> msg);
>>>>   int intel_guc_sample_forcewake(struct intel_guc *guc);
>>>>   int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
>>>>   int intel_guc_suspend(struct intel_guc *guc);
>>>> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
>>>> b/drivers/gpu/drm/i915/intel_guc_ct.c
>>>> index aa810ad..e837084 100644
>>>> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
>>>> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
>>>> @@ -694,8 +694,17 @@ static int ct_handle_response(struct 
>>>> intel_guc_ct *ct, const u32 *msg)
>>>>   static void ct_process_request(struct intel_guc_ct *ct,
>>>>                      u32 action, u32 len, const u32 *payload)
>>>>   {
>>>> +    struct intel_guc *guc = ct_to_guc(ct);
>>>> +
>>>>       switch (action) {
>>>> +    case INTEL_GUC_ACTION_DEFAULT:
>>>> +        if (unlikely(len < 1))
>>>> +            goto fail_unexpected;
>>>  Don't we need to remove the non-enabled messages here? i.e. like we 
>>> do for the mmio receive:
>>>       msg = *payload & guc->msg_enabled_mask;
>>>  otherwise I think we'll try to process log interrupts even if the 
>>> relay is not enabled.
>>
>> In Daniele's specific example, I still think 
>> guc_log_enable_flush_events should have been called (and don't do 
>> anything inside handle_flush_event, the goal was to not serve it).
>>
>> But it's true that we should filter any unexpected incoming request (& 
>> guc->msg_enabled_mask), and any new user should be calling 
>> intel_guc_enable_msg during its setup.
> 
> hmm, my understanding was that purpose of msg_enabled_mask was to:
> 
>       * Sample the log buffer flush related bits & clear them out now
>       * itself from the message identity register to minimize the
>       * probability of losing a flush interrupt, when there are back
>       * to back flush interrupts.
> 
> and is not applicable to messages sent over CTB, as we can't miss msg.
> 
> if you still believe that filtering is required, it should not be done
> here, as purpose of this function is to verify message completeness and
> call actual handler, but in intel_guc_to_host_process_recv_msg(), where
> format of received data is known.
> 
> /m
> 

The problem is that now we keep the interrupts enabled even if the log 
relay is not enabled. If we handle the interrupt anyway and queue the 
flush_work (from intel_guc_log_handle_flush_event), we end up hitting:

	if (WARN_ON(!intel_guc_log_relay_enabled(log)))
		goto out_unlock;

inside guc_read_update_log_buffer.

I don't think that the comment you added above referred to the specific 
usage of msg_enabled_mask but more to the fact that we clean the bits 
related to the messages we're going to handle.

Daniele

>>
>> Thanks,
>>>  Daniele
>>>
>>>> +        intel_guc_to_host_process_recv_msg(guc, *payload);
>>>> +        break;
>>>> +
>>>>       default:
>>>> +fail_unexpected:
>>>>           DRM_ERROR("CT: unexpected request %x %*phn\n",
>>>>                 action, 4 * len, payload);
>>>>           break;
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* [PATCH v7 10/12] drm/i915/guc: Handle default action received over CT
  2018-03-26 19:48 ` [PATCH v5 10/12] drm/i915/guc: Handle default action received over CT Michal Wajdeczko
  2018-03-27 18:25   ` Daniele Ceraolo Spurio
@ 2018-03-27 21:41   ` Michal Wajdeczko
  2018-03-27 22:49     ` Michel Thierry
  1 sibling, 1 reply; 35+ messages in thread
From: Michal Wajdeczko @ 2018-03-27 21:41 UTC (permalink / raw)
  To: intel-gfx

When running on platform with CTB based GuC communication enabled,
GuC to Host event data will be delivered as CT request message.
However, content of the data[1] of this CT message follows format
of the scratch register used in MMIO based communication, so some
code reuse is still possible.

v2:  filter disabled messages (Daniele)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Oscar Mateo <oscar.mateo@intel.com>
Reviewed-by: Michel Thierry <michel.thierry@intel.com> #1
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c    | 8 ++++++++
 drivers/gpu/drm/i915/intel_guc.h    | 1 +
 drivers/gpu/drm/i915/intel_guc_ct.c | 9 +++++++++
 3 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 118db81..d77dde7 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -416,6 +416,14 @@ void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc)
 	I915_WRITE(SOFT_SCRATCH(15), val & ~msg);
 	spin_unlock(&guc->irq_lock);
 
+	intel_guc_to_host_process_recv_msg(guc, msg);
+}
+
+void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 msg)
+{
+	/* Make sure to handle only enabled messages */
+	msg &= guc->msg_enabled_mask;
+
 	if (msg & (INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
 		   INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED))
 		intel_guc_log_handle_flush_event(&guc->log);
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 6dc109a..f1265e1 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -163,6 +163,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
 void intel_guc_to_host_event_handler(struct intel_guc *guc);
 void intel_guc_to_host_event_handler_nop(struct intel_guc *guc);
 void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc);
+void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 msg);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
 int intel_guc_suspend(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index aa810ad..e837084 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -694,8 +694,17 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
 static void ct_process_request(struct intel_guc_ct *ct,
 			       u32 action, u32 len, const u32 *payload)
 {
+	struct intel_guc *guc = ct_to_guc(ct);
+
 	switch (action) {
+	case INTEL_GUC_ACTION_DEFAULT:
+		if (unlikely(len < 1))
+			goto fail_unexpected;
+		intel_guc_to_host_process_recv_msg(guc, *payload);
+		break;
+
 	default:
+fail_unexpected:
 		DRM_ERROR("CT: unexpected request %x %*phn\n",
 			  action, 4 * len, payload);
 		break;
-- 
1.9.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 35+ messages in thread

* Re: [PATCH v7 10/12] drm/i915/guc: Handle default action received over CT
  2018-03-27 21:41   ` [PATCH v7 " Michal Wajdeczko
@ 2018-03-27 22:49     ` Michel Thierry
  2018-03-28 16:05       ` Ceraolo Spurio, Daniele
  0 siblings, 1 reply; 35+ messages in thread
From: Michel Thierry @ 2018-03-27 22:49 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx

On 3/27/2018 2:41 PM, Michal Wajdeczko wrote:
> When running on platform with CTB based GuC communication enabled,
> GuC to Host event data will be delivered as CT request message.
> However, content of the data[1] of this CT message follows format
> of the scratch register used in MMIO based communication, so some
> code reuse is still possible.
> 
> v2:  filter disabled messages (Daniele)
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Oscar Mateo <oscar.mateo@intel.com>
> Reviewed-by: Michel Thierry <michel.thierry@intel.com> #1
   ^ still applies for v2, but I would wait for Daniele's blessing

> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc.c    | 8 ++++++++
>   drivers/gpu/drm/i915/intel_guc.h    | 1 +
>   drivers/gpu/drm/i915/intel_guc_ct.c | 9 +++++++++
>   3 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index 118db81..d77dde7 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -416,6 +416,14 @@ void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc)
>   	I915_WRITE(SOFT_SCRATCH(15), val & ~msg);
>   	spin_unlock(&guc->irq_lock);
>   
> +	intel_guc_to_host_process_recv_msg(guc, msg);
> +}
> +
> +void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 msg)
> +{

(as I said before, this will need to change later on to receive more 
than jut data[1]/payload anyway)

> +	/* Make sure to handle only enabled messages */
> +	msg &= guc->msg_enabled_mask;
> +
>   	if (msg & (INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
>   		   INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED))
>   		intel_guc_log_handle_flush_event(&guc->log);
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 6dc109a..f1265e1 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -163,6 +163,7 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len,
>   void intel_guc_to_host_event_handler(struct intel_guc *guc);
>   void intel_guc_to_host_event_handler_nop(struct intel_guc *guc);
>   void intel_guc_to_host_event_handler_mmio(struct intel_guc *guc);
> +void intel_guc_to_host_process_recv_msg(struct intel_guc *guc, u32 msg);
>   int intel_guc_sample_forcewake(struct intel_guc *guc);
>   int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
>   int intel_guc_suspend(struct intel_guc *guc);
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
> index aa810ad..e837084 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -694,8 +694,17 @@ static int ct_handle_response(struct intel_guc_ct *ct, const u32 *msg)
>   static void ct_process_request(struct intel_guc_ct *ct,
>   			       u32 action, u32 len, const u32 *payload)
>   {
> +	struct intel_guc *guc = ct_to_guc(ct);
> +
>   	switch (action) {
> +	case INTEL_GUC_ACTION_DEFAULT:
> +		if (unlikely(len < 1))
> +			goto fail_unexpected;
> +		intel_guc_to_host_process_recv_msg(guc, *payload);
> +		break;
> +
>   	default:
> +fail_unexpected:
>   		DRM_ERROR("CT: unexpected request %x %*phn\n",
>   			  action, 4 * len, payload);
>   		break;
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Support for Guc responses and requests (rev5)
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (18 preceding siblings ...)
  2018-03-27 20:54 ` [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michel Thierry
@ 2018-03-27 23:01 ` Patchwork
  2018-03-27 23:14 ` ✓ Fi.CI.BAT: success " Patchwork
  2018-03-28  8:22 ` ✗ Fi.CI.IGT: failure " Patchwork
  21 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2018-03-27 23:01 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc: Support for Guc responses and requests (rev5)
URL   : https://patchwork.freedesktop.org/series/28393/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915/guc: Add documentation for MMIO based communication
Okay!

Commit: drm/i915/guc: Add support for data reporting in GuC responses
Okay!

Commit: drm/i915/guc: Prepare send() function to accept bigger response
Okay!

Commit: drm/i915/guc: Implement response handling in send_mmio()
Okay!

Commit: drm/i915/guc: Make event handler a virtual function
Okay!

Commit: drm/i915/guc: Prepare to handle messages from CT RECV buffer
Okay!

Commit: drm/i915/guc: Use better name for helper wait function
Okay!

Commit: drm/i915/guc: Implement response handling in send_ct()
Okay!

Commit: drm/i915/guc: Prepare to process incoming requests from CT
Okay!

Commit: drm/i915/guc: Handle default action received over CT
Okay!

Commit: drm/i915/guc: Trace messages from CT while in debug
+Error in reading or end of file.

Commit: HAX: Enable GuC for CI
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915/guc: Support for Guc responses and requests (rev5)
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (19 preceding siblings ...)
  2018-03-27 23:01 ` ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Support for Guc responses and requests (rev5) Patchwork
@ 2018-03-27 23:14 ` Patchwork
  2018-03-28  8:22 ` ✗ Fi.CI.IGT: failure " Patchwork
  21 siblings, 0 replies; 35+ messages in thread
From: Patchwork @ 2018-03-27 23:14 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc: Support for Guc responses and requests (rev5)
URL   : https://patchwork.freedesktop.org/series/28393/
State : success

== Summary ==

Series 28393v5 drm/i915/guc: Support for Guc responses and requests
https://patchwork.freedesktop.org/api/1.0/series/28393/revisions/5/mbox/

---- Known issues:

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                pass       -> DMESG-WARN (fi-cnl-y3) fdo#104951

fdo#104951 https://bugs.freedesktop.org/show_bug.cgi?id=104951

fi-bdw-5557u     total:285  pass:264  dwarn:0   dfail:0   fail:0   skip:21  time:439s
fi-bdw-gvtdvm    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:444s
fi-blb-e6850     total:285  pass:220  dwarn:1   dfail:0   fail:0   skip:64  time:381s
fi-bsw-n3050     total:285  pass:239  dwarn:0   dfail:0   fail:0   skip:46  time:543s
fi-bwr-2160      total:285  pass:180  dwarn:0   dfail:0   fail:0   skip:105 time:305s
fi-bxt-j4205     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:517s
fi-byt-j1900     total:285  pass:250  dwarn:0   dfail:0   fail:0   skip:35  time:523s
fi-byt-n2820     total:285  pass:246  dwarn:0   dfail:0   fail:0   skip:39  time:510s
fi-cfl-8700k     total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:413s
fi-cfl-s3        total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:572s
fi-cfl-u         total:285  pass:259  dwarn:0   dfail:0   fail:0   skip:26  time:512s
fi-cnl-y3        total:285  pass:258  dwarn:1   dfail:0   fail:0   skip:26  time:586s
fi-elk-e7500     total:285  pass:225  dwarn:1   dfail:0   fail:0   skip:59  time:428s
fi-gdg-551       total:285  pass:176  dwarn:0   dfail:0   fail:1   skip:108 time:321s
fi-glk-1         total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:539s
fi-hsw-4770      total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:412s
fi-ilk-650       total:285  pass:225  dwarn:0   dfail:0   fail:0   skip:60  time:420s
fi-ivb-3520m     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:475s
fi-ivb-3770      total:285  pass:252  dwarn:0   dfail:0   fail:0   skip:33  time:437s
fi-kbl-7500u     total:285  pass:260  dwarn:1   dfail:0   fail:0   skip:24  time:476s
fi-kbl-7567u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:464s
fi-kbl-r         total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:513s
fi-pnv-d510      total:285  pass:219  dwarn:1   dfail:0   fail:0   skip:65  time:657s
fi-skl-6260u     total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:442s
fi-skl-6600u     total:285  pass:258  dwarn:0   dfail:0   fail:0   skip:27  time:533s
fi-skl-6700k2    total:285  pass:261  dwarn:0   dfail:0   fail:0   skip:24  time:508s
fi-skl-6770hq    total:285  pass:265  dwarn:0   dfail:0   fail:0   skip:20  time:504s
fi-skl-guc       total:285  pass:257  dwarn:0   dfail:0   fail:0   skip:28  time:432s
fi-skl-gvtdvm    total:285  pass:262  dwarn:0   dfail:0   fail:0   skip:23  time:448s
fi-snb-2520m     total:285  pass:245  dwarn:0   dfail:0   fail:0   skip:40  time:584s
Blacklisted hosts:
fi-cnl-psr       total:285  pass:256  dwarn:3   dfail:0   fail:0   skip:26  time:513s
fi-glk-j4005     total:285  pass:256  dwarn:0   dfail:0   fail:0   skip:29  time:487s

0539b52e05cd0abe697d45f2a2373ec42af7ebcb drm-tip: 2018y-03m-27d-18h-45m-40s UTC integration manifest
a5e34053ad2d HAX: Enable GuC for CI
a87384b1c925 drm/i915/guc: Trace messages from CT while in debug
fc751997667a drm/i915/guc: Handle default action received over CT
5f5795f24ab3 drm/i915/guc: Prepare to process incoming requests from CT
f631277cfde8 drm/i915/guc: Implement response handling in send_ct()
afa15bb29b63 drm/i915/guc: Use better name for helper wait function
b7159298221d drm/i915/guc: Prepare to handle messages from CT RECV buffer
58acb5af66e4 drm/i915/guc: Make event handler a virtual function
55dddb9679fe drm/i915/guc: Implement response handling in send_mmio()
e0bdc40b09d3 drm/i915/guc: Prepare send() function to accept bigger response
02a3b07b6d97 drm/i915/guc: Add support for data reporting in GuC responses
8da80e5819e2 drm/i915/guc: Add documentation for MMIO based communication

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8510/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* ✗ Fi.CI.IGT: failure for drm/i915/guc: Support for Guc responses and requests (rev5)
  2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
                   ` (20 preceding siblings ...)
  2018-03-27 23:14 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2018-03-28  8:22 ` Patchwork
  2018-03-28 19:52   ` Chris Wilson
  21 siblings, 1 reply; 35+ messages in thread
From: Patchwork @ 2018-03-28  8:22 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc: Support for Guc responses and requests (rev5)
URL   : https://patchwork.freedesktop.org/series/28393/
State : failure

== Summary ==

---- Possible new issues:

Test debugfs_test:
        Subgroup read_all_entries_display_off:
                pass       -> DMESG-WARN (shard-apl)
Test gem_eio:
        Subgroup execbuf:
                pass       -> INCOMPLETE (shard-apl)
        Subgroup throttle:
                pass       -> INCOMPLETE (shard-apl)
Test perf:
        Subgroup gen8-unprivileged-single-ctx-counters:
                pass       -> FAIL       (shard-apl)

---- Known issues:

Test drv_missed_irq:
                pass       -> SKIP       (shard-apl) fdo#103199
Test kms_cursor_crc:
        Subgroup cursor-128x128-suspend:
                dmesg-warn -> PASS       (shard-snb) fdo#102365
Test kms_cursor_legacy:
        Subgroup 2x-long-flip-vs-cursor-atomic:
                pass       -> FAIL       (shard-hsw) fdo#104873
Test kms_flip:
        Subgroup 2x-dpms-vs-vblank-race-interruptible:
                fail       -> PASS       (shard-hsw) fdo#103060
        Subgroup 2x-flip-vs-expired-vblank-interruptible:
                fail       -> PASS       (shard-hsw) fdo#102887 +1
        Subgroup plain-flip-fb-recreate-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#100368 +1
Test kms_plane_multiple:
        Subgroup atomic-pipe-a-tiling-x:
                pass       -> FAIL       (shard-snb) fdo#103166
Test kms_vblank:
        Subgroup pipe-b-accuracy-idle:
                pass       -> FAIL       (shard-hsw) fdo#102583
Test perf:
        Subgroup polling:
                fail       -> PASS       (shard-hsw) fdo#102252

fdo#103199 https://bugs.freedesktop.org/show_bug.cgi?id=103199
fdo#102365 https://bugs.freedesktop.org/show_bug.cgi?id=102365
fdo#104873 https://bugs.freedesktop.org/show_bug.cgi?id=104873
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#102887 https://bugs.freedesktop.org/show_bug.cgi?id=102887
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
fdo#102583 https://bugs.freedesktop.org/show_bug.cgi?id=102583
fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252

shard-apl        total:3383 pass:1764 dwarn:2   dfail:0   fail:10  skip:1603 time:12404s
shard-hsw        total:3495 pass:1777 dwarn:1   dfail:0   fail:7   skip:1709 time:11579s
shard-snb        total:3495 pass:1373 dwarn:1   dfail:0   fail:4   skip:2117 time:7049s
Blacklisted hosts:
shard-kbl        total:3345 pass:1826 dwarn:3   dfail:1   fail:11  skip:1497 time:7899s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8510/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: [PATCH v7 10/12] drm/i915/guc: Handle default action received over CT
  2018-03-27 22:49     ` Michel Thierry
@ 2018-03-28 16:05       ` Ceraolo Spurio, Daniele
  0 siblings, 0 replies; 35+ messages in thread
From: Ceraolo Spurio, Daniele @ 2018-03-28 16:05 UTC (permalink / raw)
  To: Michel Thierry, Michal Wajdeczko, intel-gfx



On 3/27/2018 3:49 PM, Michel Thierry wrote:
> On 3/27/2018 2:41 PM, Michal Wajdeczko wrote:
>> When running on platform with CTB based GuC communication enabled,
>> GuC to Host event data will be delivered as CT request message.
>> However, content of the data[1] of this CT message follows format
>> of the scratch register used in MMIO based communication, so some
>> code reuse is still possible.
>>
>> v2:  filter disabled messages (Daniele)
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Oscar Mateo <oscar.mateo@intel.com>
>> Reviewed-by: Michel Thierry <michel.thierry@intel.com> #1
>    ^ still applies for v2, but I would wait for Daniele's blessing
> 

Acked-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>

A minor comment below.

Daniele

<snip>

>> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
>> @@ -694,8 +694,17 @@ static int ct_handle_response(struct intel_guc_ct 
>> *ct, const u32 *msg)
>>   static void ct_process_request(struct intel_guc_ct *ct,
>>                      u32 action, u32 len, const u32 *payload)
>>   {
>> +    struct intel_guc *guc = ct_to_guc(ct);
>> +
>>       switch (action) {
>> +    case INTEL_GUC_ACTION_DEFAULT:
>> +        if (unlikely(len < 1))
>> +            goto fail_unexpected;
>> +        intel_guc_to_host_process_recv_msg(guc, *payload);
>> +        break;
>> +
>>       default:
>> +fail_unexpected:
>>           DRM_ERROR("CT: unexpected request %x %*phn\n",
>>                 action, 4 * len, payload);

if we end up here from the goto with len == 0 this print will probably 
not output a fully clear message (error with a correct action number and 
no data). Not a blocker because we can still infer it was a length issue.

>>           break;
>>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

* Re: ✗ Fi.CI.IGT: failure for drm/i915/guc: Support for Guc responses and requests (rev5)
  2018-03-28  8:22 ` ✗ Fi.CI.IGT: failure " Patchwork
@ 2018-03-28 19:52   ` Chris Wilson
  0 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2018-03-28 19:52 UTC (permalink / raw)
  To: Patchwork, Michal Wajdeczko; +Cc: intel-gfx

Quoting Patchwork (2018-03-28 09:22:48)
> == Series Details ==
> 
> Series: drm/i915/guc: Support for Guc responses and requests (rev5)
> URL   : https://patchwork.freedesktop.org/series/28393/
> State : failure
> 
> == Summary ==
> 
> ---- Possible new issues:
> 
> Test debugfs_test:
>         Subgroup read_all_entries_display_off:
>                 pass       -> DMESG-WARN (shard-apl)
> Test gem_eio:
>         Subgroup execbuf:
>                 pass       -> INCOMPLETE (shard-apl)
>         Subgroup throttle:
>                 pass       -> INCOMPLETE (shard-apl)

intel_engines_reset_default_submission() being called from an
inopportune time.

Applied the series, thanks for the patches and review.

I did notice a residual XXX, some potential use of IS_ALIGNED() might be
clearer than x % y, and maybe using circ_buffer.h.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 35+ messages in thread

end of thread, other threads:[~2018-03-28 19:52 UTC | newest]

Thread overview: 35+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-26 19:48 [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michal Wajdeczko
2018-03-26 19:48 ` [PATCH v5 01/12] drm/i915/guc: Add documentation for MMIO based communication Michal Wajdeczko
2018-03-27 10:05   ` Sagar Arun Kamble
2018-03-27 11:37     ` Michal Wajdeczko
2018-03-26 19:48 ` [PATCH v5 02/12] drm/i915/guc: Add support for data reporting in GuC responses Michal Wajdeczko
2018-03-26 19:48 ` [PATCH v5 03/12] drm/i915/guc: Prepare send() function to accept bigger response Michal Wajdeczko
2018-03-26 19:48 ` [PATCH v5 04/12] drm/i915/guc: Implement response handling in send_mmio() Michal Wajdeczko
2018-03-26 19:48 ` [PATCH v5 05/12] drm/i915/guc: Make event handler a virtual function Michal Wajdeczko
2018-03-26 19:48 ` [PATCH v5 06/12] drm/i915/guc: Prepare to handle messages from CT RECV buffer Michal Wajdeczko
2018-03-26 19:48 ` [PATCH v5 07/12] drm/i915/guc: Use better name for helper wait function Michal Wajdeczko
2018-03-26 19:48 ` [PATCH v5 08/12] drm/i915/guc: Implement response handling in send_ct() Michal Wajdeczko
2018-03-27 12:14   ` [PATCH v6 " Michal Wajdeczko
2018-03-26 19:48 ` [PATCH v5 09/12] drm/i915/guc: Prepare to process incoming requests from CT Michal Wajdeczko
2018-03-27 20:07   ` Michel Thierry
2018-03-26 19:48 ` [PATCH v5 10/12] drm/i915/guc: Handle default action received over CT Michal Wajdeczko
2018-03-27 18:25   ` Daniele Ceraolo Spurio
2018-03-27 20:03     ` Michel Thierry
2018-03-27 21:05       ` Michal Wajdeczko
2018-03-27 21:17         ` Daniele Ceraolo Spurio
2018-03-27 21:41   ` [PATCH v7 " Michal Wajdeczko
2018-03-27 22:49     ` Michel Thierry
2018-03-28 16:05       ` Ceraolo Spurio, Daniele
2018-03-26 19:48 ` [PATCH v5 11/12] drm/i915/guc: Trace messages from CT while in debug Michal Wajdeczko
2018-03-26 19:48 ` [PATCH v5 12/12] HAX: Enable GuC for CI Michal Wajdeczko
2018-03-26 20:23 ` ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Support for Guc responses and requests (rev3) Patchwork
2018-03-26 20:36 ` ✓ Fi.CI.BAT: success " Patchwork
2018-03-26 22:11 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-03-27 13:36 ` ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Support for Guc responses and requests (rev4) Patchwork
2018-03-27 13:48 ` ✓ Fi.CI.BAT: success " Patchwork
2018-03-27 17:35 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-03-27 20:54 ` [PATCH v5 00/12] drm/i915/guc: Support for Guc responses and requests Michel Thierry
2018-03-27 23:01 ` ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Support for Guc responses and requests (rev5) Patchwork
2018-03-27 23:14 ` ✓ Fi.CI.BAT: success " Patchwork
2018-03-28  8:22 ` ✗ Fi.CI.IGT: failure " Patchwork
2018-03-28 19:52   ` Chris Wilson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.