All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality
@ 2017-09-17 12:17 Sagar Arun Kamble
  2017-09-17 12:17 ` [PATCH 02/10] drm/i915/guc: Move guc_send_* functions to intel_guc.c Sagar Arun Kamble
                   ` (12 more replies)
  0 siblings, 13 replies; 27+ messages in thread
From: Sagar Arun Kamble @ 2017-09-17 12:17 UTC (permalink / raw)
  To: intel-gfx

Create intel_guc.c and move guc communication init functionality from
intel_uc.c. Prepared new initialization function intel_guc_init_early.
Moved below functions to intel_guc.c.
1. intel_guc_send_nop
2. gen8_guc_raise_irq
And below functions to intel_guc.h.
1. intel_guc_send
2. intel_guc_notify

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/Makefile              |  1 +
 drivers/gpu/drm/i915/i915_guc_submission.c |  1 +
 drivers/gpu/drm/i915/intel_guc.c           | 47 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc.h           | 43 +++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc_ct.c        |  1 +
 drivers/gpu/drm/i915/intel_guc_log.c       |  1 +
 drivers/gpu/drm/i915/intel_huc.c           |  1 +
 drivers/gpu/drm/i915/intel_uc.c            | 22 ++------------
 drivers/gpu/drm/i915/intel_uc.h            | 11 -------
 9 files changed, 97 insertions(+), 31 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_guc.c
 create mode 100644 drivers/gpu/drm/i915/intel_guc.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 1cb8059..e13fc19 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -59,6 +59,7 @@ i915-y += i915_cmd_parser.o \
 
 # general-purpose microcontroller (GuC) support
 i915-y += intel_uc.o \
+	  intel_guc.o \
 	  intel_guc_ct.o \
 	  intel_guc_log.o \
 	  intel_guc_loader.o \
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 3f9d227..f37cd47 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -24,6 +24,7 @@
 #include <linux/circ_buf.h>
 #include "i915_drv.h"
 #include "intel_uc.h"
+#include "intel_guc.h"
 
 #include <trace/events/dma_fence.h>
 
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
new file mode 100644
index 0000000..0c62cc2
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -0,0 +1,47 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "i915_drv.h"
+
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
+{
+	WARN(1, "Unexpected send: action=%#x\n", *action);
+	return -ENODEV;
+}
+
+static void gen8_guc_raise_irq(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+	I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
+}
+
+void intel_guc_init_early(struct intel_guc *guc)
+{
+	intel_guc_ct_init_early(&guc->ct);
+
+	mutex_init(&guc->send_mutex);
+	guc->send = intel_guc_send_nop;
+	guc->notify = gen8_guc_raise_irq;
+}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
new file mode 100644
index 0000000..76b7113
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -0,0 +1,43 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+#ifndef _INTEL_GUC_H_
+#define _INTEL_GUC_H_
+
+static inline int intel_guc_send(struct intel_guc *guc,
+				 const u32 *action,
+				 u32 len)
+{
+	return guc->send(guc, action, len);
+}
+
+static inline void intel_guc_notify(struct intel_guc *guc)
+{
+	guc->notify(guc);
+}
+
+/* intel_guc.c */
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
+void intel_guc_init_early(struct intel_guc *guc);
+
+#endif
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index c4cbec1..87dd12d 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -23,6 +23,7 @@
 
 #include "i915_drv.h"
 #include "intel_guc_ct.h"
+#include "intel_guc.h"
 
 enum { CTB_SEND = 0, CTB_RECV = 1 };
 
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 16d3b87..5c7e0c2 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -24,6 +24,7 @@
 #include <linux/debugfs.h>
 #include <linux/relay.h>
 #include "i915_drv.h"
+#include "intel_guc.h"
 
 static void guc_log_capture_logs(struct intel_guc *guc);
 
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 6145fa0..b81c6af 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -24,6 +24,7 @@
 #include <linux/firmware.h>
 #include "i915_drv.h"
 #include "intel_uc.h"
+#include "intel_guc.h"
 
 /**
  * DOC: HuC Firmware
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 0178ba4..9ff5c97 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -24,6 +24,7 @@
 
 #include "i915_drv.h"
 #include "intel_uc.h"
+#include "intel_guc.h"
 #include <linux/firmware.h>
 
 /* Cleans up uC firmware by releasing the firmware GEM obj.
@@ -94,22 +95,9 @@ void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
 		i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
 }
 
-static void gen8_guc_raise_irq(struct intel_guc *guc)
-{
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-
-	I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
-}
-
 void intel_uc_init_early(struct drm_i915_private *dev_priv)
 {
-	struct intel_guc *guc = &dev_priv->guc;
-
-	intel_guc_ct_init_early(&guc->ct);
-
-	mutex_init(&guc->send_mutex);
-	guc->send = intel_guc_send_nop;
-	guc->notify = gen8_guc_raise_irq;
+	intel_guc_init_early(&dev_priv->guc);
 }
 
 static void fetch_uc_fw(struct drm_i915_private *dev_priv,
@@ -459,12 +447,6 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 	i915_ggtt_disable_guc(dev_priv);
 }
 
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
-{
-	WARN(1, "Unexpected send: action=%#x\n", *action);
-	return -ENODEV;
-}
-
 /*
  * This function implements the MMIO based host to GuC interface.
  */
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 7703c9a..27e30aa 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -209,19 +209,8 @@ struct intel_huc {
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 int intel_guc_sample_forcewake(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);
 
-static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
-{
-	return guc->send(guc, action, len);
-}
-
-static inline void intel_guc_notify(struct intel_guc *guc)
-{
-	guc->notify(guc);
-}
-
 /* intel_guc_loader.c */
 int intel_guc_select_fw(struct intel_guc *guc);
 int intel_guc_init_hw(struct intel_guc *guc);
-- 
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] 27+ messages in thread

* [PATCH 02/10] drm/i915/guc: Move guc_send_* functions to intel_guc.c
  2017-09-17 12:17 [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Sagar Arun Kamble
@ 2017-09-17 12:17 ` Sagar Arun Kamble
  2017-09-17 12:17 ` [PATCH 03/10] drm/i915/guc: Move guc_sample_forcewake " Sagar Arun Kamble
                   ` (11 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Sagar Arun Kamble @ 2017-09-17 12:17 UTC (permalink / raw)
  To: intel-gfx

s/guc_init_send_regs/intel_guc_init_send_regs.
Calling intel_guc_init_send_regs from intel_uc_init_hw.
Moved below functions to intel_guc.c from intel_uc.c
1. guc_send_regs
2. intel_guc_init_send_regs
3. intel_guc_send_mmio

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c | 84 ++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc.h |  2 +
 drivers/gpu/drm/i915/intel_uc.c  | 87 +---------------------------------------
 drivers/gpu/drm/i915/intel_uc.h  |  1 -
 4 files changed, 88 insertions(+), 86 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 0c62cc2..02b8251 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -23,6 +23,7 @@
  */
 
 #include "i915_drv.h"
+#include "intel_guc.h"
 
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
 {
@@ -45,3 +46,86 @@ void intel_guc_init_early(struct intel_guc *guc)
 	guc->send = intel_guc_send_nop;
 	guc->notify = gen8_guc_raise_irq;
 }
+
+static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
+{
+	GEM_BUG_ON(!guc->send_regs.base);
+	GEM_BUG_ON(!guc->send_regs.count);
+	GEM_BUG_ON(i >= guc->send_regs.count);
+
+	return _MMIO(guc->send_regs.base + 4 * i);
+}
+
+void intel_guc_init_send_regs(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	enum forcewake_domains fw_domains = 0;
+	unsigned int i;
+
+	guc->send_regs.base = i915_mmio_reg_offset(SOFT_SCRATCH(0));
+	guc->send_regs.count = SOFT_SCRATCH_COUNT - 1;
+
+	for (i = 0; i < guc->send_regs.count; i++) {
+		fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
+					guc_send_reg(guc, i),
+					FW_REG_READ | FW_REG_WRITE);
+	}
+	guc->send_regs.fw_domains = fw_domains;
+}
+
+/*
+ * This function implements the MMIO based host to GuC interface.
+ */
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	u32 status;
+	int i;
+	int ret;
+
+	GEM_BUG_ON(!len);
+	GEM_BUG_ON(len > guc->send_regs.count);
+
+	/* 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 &&
+		*action != INTEL_GUC_ACTION_DEREGISTER_COMMAND_TRANSPORT_BUFFER);
+
+	mutex_lock(&guc->send_mutex);
+	intel_uncore_forcewake_get(dev_priv, guc->send_regs.fw_domains);
+
+	for (i = 0; i < len; i++)
+		I915_WRITE(guc_send_reg(guc, i), action[i]);
+
+	POSTING_READ(guc_send_reg(guc, i - 1));
+
+	intel_guc_notify(guc);
+
+	/*
+	 * No GuC command should ever take longer than 10ms.
+	 * Fast commands should still complete in 10us.
+	 */
+	ret = __intel_wait_for_register_fw(dev_priv,
+					   guc_send_reg(guc, 0),
+					   INTEL_GUC_RECV_MASK,
+					   INTEL_GUC_RECV_MASK,
+					   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;
+
+		DRM_WARN("INTEL_GUC_SEND: Action 0x%X failed;"
+			 " ret=%d status=0x%08X response=0x%08X\n",
+			 action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
+	}
+
+	intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
+	mutex_unlock(&guc->send_mutex);
+
+	return ret;
+}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 76b7113..7860b8f 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -39,5 +39,7 @@ static inline void intel_guc_notify(struct intel_guc *guc)
 /* intel_guc.c */
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
 void intel_guc_init_early(struct intel_guc *guc);
+void intel_guc_init_send_regs(struct intel_guc *guc);
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
 
 #endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 9ff5c97..700ec70 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -250,32 +250,6 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
 	__intel_uc_fw_fini(&dev_priv->huc.fw);
 }
 
-static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
-{
-	GEM_BUG_ON(!guc->send_regs.base);
-	GEM_BUG_ON(!guc->send_regs.count);
-	GEM_BUG_ON(i >= guc->send_regs.count);
-
-	return _MMIO(guc->send_regs.base + 4 * i);
-}
-
-static void guc_init_send_regs(struct intel_guc *guc)
-{
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	enum forcewake_domains fw_domains = 0;
-	unsigned int i;
-
-	guc->send_regs.base = i915_mmio_reg_offset(SOFT_SCRATCH(0));
-	guc->send_regs.count = SOFT_SCRATCH_COUNT - 1;
-
-	for (i = 0; i < guc->send_regs.count; i++) {
-		fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
-					guc_send_reg(guc, i),
-					FW_REG_READ | FW_REG_WRITE);
-	}
-	guc->send_regs.fw_domains = fw_domains;
-}
-
 static void guc_capture_load_err_log(struct intel_guc *guc)
 {
 	if (!guc->log.vma || i915.guc_log_level < 0)
@@ -297,8 +271,6 @@ static int guc_enable_communication(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 
-	guc_init_send_regs(guc);
-
 	if (HAS_GUC_CT(dev_priv))
 		return intel_guc_enable_ct(guc);
 
@@ -374,6 +346,8 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	if (ret)
 		goto err_log_capture;
 
+	intel_guc_init_send_regs(guc);
+
 	ret = guc_enable_communication(guc);
 	if (ret)
 		goto err_log_capture;
@@ -447,63 +421,6 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 	i915_ggtt_disable_guc(dev_priv);
 }
 
-/*
- * This function implements the MMIO based host to GuC interface.
- */
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
-{
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	u32 status;
-	int i;
-	int ret;
-
-	GEM_BUG_ON(!len);
-	GEM_BUG_ON(len > guc->send_regs.count);
-
-	/* 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 &&
-		*action != INTEL_GUC_ACTION_DEREGISTER_COMMAND_TRANSPORT_BUFFER);
-
-	mutex_lock(&guc->send_mutex);
-	intel_uncore_forcewake_get(dev_priv, guc->send_regs.fw_domains);
-
-	for (i = 0; i < len; i++)
-		I915_WRITE(guc_send_reg(guc, i), action[i]);
-
-	POSTING_READ(guc_send_reg(guc, i - 1));
-
-	intel_guc_notify(guc);
-
-	/*
-	 * No GuC command should ever take longer than 10ms.
-	 * Fast commands should still complete in 10us.
-	 */
-	ret = __intel_wait_for_register_fw(dev_priv,
-					   guc_send_reg(guc, 0),
-					   INTEL_GUC_RECV_MASK,
-					   INTEL_GUC_RECV_MASK,
-					   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;
-
-		DRM_WARN("INTEL_GUC_SEND: Action 0x%X failed;"
-			 " ret=%d status=0x%08X response=0x%08X\n",
-			 action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
-	}
-
-	intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
-	mutex_unlock(&guc->send_mutex);
-
-	return ret;
-}
-
 int intel_guc_sample_forcewake(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 27e30aa..b9724e6 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -209,7 +209,6 @@ struct intel_huc {
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
 
 /* intel_guc_loader.c */
 int intel_guc_select_fw(struct intel_guc *guc);
-- 
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] 27+ messages in thread

* [PATCH 03/10] drm/i915/guc: Move guc_sample_forcewake to intel_guc.c
  2017-09-17 12:17 [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Sagar Arun Kamble
  2017-09-17 12:17 ` [PATCH 02/10] drm/i915/guc: Move guc_send_* functions to intel_guc.c Sagar Arun Kamble
@ 2017-09-17 12:17 ` Sagar Arun Kamble
  2017-09-17 12:17 ` [PATCH 04/10] drm/i915/guc: Move GuC specific declarations from intel_uc.h to intel_guc.h Sagar Arun Kamble
                   ` (10 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Sagar Arun Kamble @ 2017-09-17 12:17 UTC (permalink / raw)
  To: intel-gfx

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c | 16 ++++++++++++++++
 drivers/gpu/drm/i915/intel_guc.h |  1 +
 drivers/gpu/drm/i915/intel_uc.c  | 16 ----------------
 drivers/gpu/drm/i915/intel_uc.h  |  1 -
 4 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 02b8251..0767ec2 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -129,3 +129,19 @@ int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
 
 	return ret;
 }
+
+int intel_guc_sample_forcewake(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	u32 action[2];
+
+	action[0] = INTEL_GUC_ACTION_SAMPLE_FORCEWAKE;
+	/* WaRsDisableCoarsePowerGating:skl,bxt */
+	if (!intel_enable_rc6() || NEEDS_WaRsDisableCoarsePowerGating(dev_priv))
+		action[1] = 0;
+	else
+		/* bit 0 and 1 are for Render and Media domain separately */
+		action[1] = GUC_FORCEWAKE_RENDER | GUC_FORCEWAKE_MEDIA;
+
+	return intel_guc_send(guc, action, ARRAY_SIZE(action));
+}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 7860b8f..06f2d27 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -41,5 +41,6 @@ static inline void intel_guc_notify(struct intel_guc *guc)
 void intel_guc_init_early(struct intel_guc *guc);
 void intel_guc_init_send_regs(struct intel_guc *guc);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
+int intel_guc_sample_forcewake(struct intel_guc *guc);
 
 #endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 700ec70..4f8f0ff 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -420,19 +420,3 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 
 	i915_ggtt_disable_guc(dev_priv);
 }
-
-int intel_guc_sample_forcewake(struct intel_guc *guc)
-{
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	u32 action[2];
-
-	action[0] = INTEL_GUC_ACTION_SAMPLE_FORCEWAKE;
-	/* WaRsDisableCoarsePowerGating:skl,bxt */
-	if (!intel_enable_rc6() || NEEDS_WaRsDisableCoarsePowerGating(dev_priv))
-		action[1] = 0;
-	else
-		/* bit 0 and 1 are for Render and Media domain separately */
-		action[1] = GUC_FORCEWAKE_RENDER | GUC_FORCEWAKE_MEDIA;
-
-	return intel_guc_send(guc, action, ARRAY_SIZE(action));
-}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index b9724e6..2c11d11 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -208,7 +208,6 @@ struct intel_huc {
 void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
-int intel_guc_sample_forcewake(struct intel_guc *guc);
 
 /* intel_guc_loader.c */
 int intel_guc_select_fw(struct intel_guc *guc);
-- 
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] 27+ messages in thread

* [PATCH 04/10] drm/i915/guc: Move GuC specific declarations from intel_uc.h to intel_guc.h
  2017-09-17 12:17 [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Sagar Arun Kamble
  2017-09-17 12:17 ` [PATCH 02/10] drm/i915/guc: Move guc_send_* functions to intel_guc.c Sagar Arun Kamble
  2017-09-17 12:17 ` [PATCH 03/10] drm/i915/guc: Move guc_sample_forcewake " Sagar Arun Kamble
@ 2017-09-17 12:17 ` Sagar Arun Kamble
  2017-09-17 18:24   ` Michal Wajdeczko
  2017-09-17 12:17 ` [PATCH 05/10] drm/i915: Reorganize HuC authentication Sagar Arun Kamble
                   ` (9 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Sagar Arun Kamble @ 2017-09-17 12:17 UTC (permalink / raw)
  To: intel-gfx

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |   1 +
 drivers/gpu/drm/i915/intel_guc.h | 126 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.h  | 125 --------------------------------------
 3 files changed, 127 insertions(+), 125 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5aecbf7..b0f38bb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -59,6 +59,7 @@
 #include "intel_bios.h"
 #include "intel_dpll_mgr.h"
 #include "intel_uc.h"
+#include "intel_guc.h"
 #include "intel_lrc.h"
 #include "intel_ringbuffer.h"
 
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 06f2d27..919b6e1 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -24,6 +24,102 @@
 #ifndef _INTEL_GUC_H_
 #define _INTEL_GUC_H_
 
+/*
+ * This structure primarily describes the GEM object shared with the GuC.
+ * The specs sometimes refer to this object as a "GuC context", but we use
+ * the term "client" to avoid confusion with hardware contexts. This
+ * GEM object is held for the entire lifetime of our interaction with
+ * the GuC, being allocated before the GuC is loaded with its firmware.
+ * Because there's no way to update the address used by the GuC after
+ * initialisation, the shared object must stay pinned into the GGTT as
+ * long as the GuC is in use. We also keep the first page (only) mapped
+ * into kernel address space, as it includes shared data that must be
+ * updated on every request submission.
+ *
+ * The single GEM object described here is actually made up of several
+ * separate areas, as far as the GuC is concerned. The first page (kept
+ * kmap'd) includes the "process descriptor" which holds sequence data for
+ * the doorbell, and one cacheline which actually *is* the doorbell; a
+ * write to this will "ring the doorbell" (i.e. send an interrupt to the
+ * GuC). The subsequent  pages of the client object constitute the work
+ * queue (a circular array of work items), again described in the process
+ * descriptor. Work queue pages are mapped momentarily as required.
+ */
+struct i915_guc_client {
+	struct i915_vma *vma;
+	void *vaddr;
+	struct i915_gem_context *owner;
+	struct intel_guc *guc;
+
+	uint32_t engines;		/* bitmap of (host) engine ids	*/
+	uint32_t priority;
+	u32 stage_id;
+	uint32_t proc_desc_offset;
+
+	u16 doorbell_id;
+	unsigned long doorbell_offset;
+
+	spinlock_t wq_lock;
+	/* Per-engine counts of GuC submissions */
+	uint64_t submissions[I915_NUM_ENGINES];
+};
+
+struct intel_guc_log {
+	uint32_t flags;
+	struct i915_vma *vma;
+	/* The runtime stuff gets created only when GuC logging gets enabled */
+	struct {
+		void *buf_addr;
+		struct workqueue_struct *flush_wq;
+		struct work_struct flush_work;
+		struct rchan *relay_chan;
+	} runtime;
+	/* logging related stats */
+	u32 capture_miss_count;
+	u32 flush_interrupt_count;
+	u32 prev_overflow_count[GUC_MAX_LOG_BUFFER];
+	u32 total_overflow_count[GUC_MAX_LOG_BUFFER];
+	u32 flush_count[GUC_MAX_LOG_BUFFER];
+};
+
+struct intel_guc {
+	struct intel_uc_fw fw;
+	struct intel_guc_log log;
+	struct intel_guc_ct ct;
+
+	/* Log snapshot if GuC errors during load */
+	struct drm_i915_gem_object *load_err_log;
+
+	/* intel_guc_recv interrupt related state */
+	bool interrupts_enabled;
+
+	struct i915_vma *ads_vma;
+	struct i915_vma *stage_desc_pool;
+	void *stage_desc_pool_vaddr;
+	struct ida stage_ids;
+
+	struct i915_guc_client *execbuf_client;
+
+	DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
+	uint32_t db_cacheline;		/* Cyclic counter mod pagesize	*/
+
+	/* GuC's FW specific registers used in MMIO send */
+	struct {
+		u32 base;
+		unsigned int count;
+		enum forcewake_domains fw_domains;
+	} send_regs;
+
+	/* To serialize the intel_guc_send actions */
+	struct mutex send_mutex;
+
+	/* GuC's FW specific send function */
+	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
+
+	/* GuC's FW specific notify function */
+	void (*notify)(struct intel_guc *guc);
+};
+
 static inline int intel_guc_send(struct intel_guc *guc,
 				 const u32 *action,
 				 u32 len)
@@ -36,6 +132,15 @@ static inline void intel_guc_notify(struct intel_guc *guc)
 	guc->notify(guc);
 }
 
+static inline u32 guc_ggtt_offset(struct i915_vma *vma)
+{
+	u32 offset = i915_ggtt_offset(vma);
+
+	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
+	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
+	return offset;
+}
+
 /* intel_guc.c */
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
 void intel_guc_init_early(struct intel_guc *guc);
@@ -43,4 +148,25 @@ static inline void intel_guc_notify(struct intel_guc *guc)
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 
+/* intel_guc_loader.c */
+int intel_guc_select_fw(struct intel_guc *guc);
+int intel_guc_init_hw(struct intel_guc *guc);
+int intel_guc_suspend(struct drm_i915_private *dev_priv);
+int intel_guc_resume(struct drm_i915_private *dev_priv);
+u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
+
+/* i915_guc_submission.c */
+int i915_guc_submission_init(struct drm_i915_private *dev_priv);
+int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
+void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
+void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
+struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
+
+/* intel_guc_log.c */
+int intel_guc_log_create(struct intel_guc *guc);
+void intel_guc_log_destroy(struct intel_guc *guc);
+int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
+void i915_guc_log_register(struct drm_i915_private *dev_priv);
+void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
+
 #endif
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 2c11d11..30902f3 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -32,46 +32,6 @@
 
 struct drm_i915_gem_request;
 
-/*
- * This structure primarily describes the GEM object shared with the GuC.
- * The specs sometimes refer to this object as a "GuC context", but we use
- * the term "client" to avoid confusion with hardware contexts. This
- * GEM object is held for the entire lifetime of our interaction with
- * the GuC, being allocated before the GuC is loaded with its firmware.
- * Because there's no way to update the address used by the GuC after
- * initialisation, the shared object must stay pinned into the GGTT as
- * long as the GuC is in use. We also keep the first page (only) mapped
- * into kernel address space, as it includes shared data that must be
- * updated on every request submission.
- *
- * The single GEM object described here is actually made up of several
- * separate areas, as far as the GuC is concerned. The first page (kept
- * kmap'd) includes the "process descriptor" which holds sequence data for
- * the doorbell, and one cacheline which actually *is* the doorbell; a
- * write to this will "ring the doorbell" (i.e. send an interrupt to the
- * GuC). The subsequent  pages of the client object constitute the work
- * queue (a circular array of work items), again described in the process
- * descriptor. Work queue pages are mapped momentarily as required.
- */
-struct i915_guc_client {
-	struct i915_vma *vma;
-	void *vaddr;
-	struct i915_gem_context *owner;
-	struct intel_guc *guc;
-
-	uint32_t engines;		/* bitmap of (host) engine ids	*/
-	uint32_t priority;
-	u32 stage_id;
-	uint32_t proc_desc_offset;
-
-	u16 doorbell_id;
-	unsigned long doorbell_offset;
-
-	spinlock_t wq_lock;
-	/* Per-engine counts of GuC submissions */
-	uint64_t submissions[I915_NUM_ENGINES];
-};
-
 enum intel_uc_fw_status {
 	INTEL_UC_FIRMWARE_FAIL = -1,
 	INTEL_UC_FIRMWARE_NONE = 0,
@@ -138,62 +98,6 @@ struct intel_uc_fw {
 	uint32_t ucode_offset;
 };
 
-struct intel_guc_log {
-	uint32_t flags;
-	struct i915_vma *vma;
-	/* The runtime stuff gets created only when GuC logging gets enabled */
-	struct {
-		void *buf_addr;
-		struct workqueue_struct *flush_wq;
-		struct work_struct flush_work;
-		struct rchan *relay_chan;
-	} runtime;
-	/* logging related stats */
-	u32 capture_miss_count;
-	u32 flush_interrupt_count;
-	u32 prev_overflow_count[GUC_MAX_LOG_BUFFER];
-	u32 total_overflow_count[GUC_MAX_LOG_BUFFER];
-	u32 flush_count[GUC_MAX_LOG_BUFFER];
-};
-
-struct intel_guc {
-	struct intel_uc_fw fw;
-	struct intel_guc_log log;
-	struct intel_guc_ct ct;
-
-	/* Log snapshot if GuC errors during load */
-	struct drm_i915_gem_object *load_err_log;
-
-	/* intel_guc_recv interrupt related state */
-	bool interrupts_enabled;
-
-	struct i915_vma *ads_vma;
-	struct i915_vma *stage_desc_pool;
-	void *stage_desc_pool_vaddr;
-	struct ida stage_ids;
-
-	struct i915_guc_client *execbuf_client;
-
-	DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
-	uint32_t db_cacheline;		/* Cyclic counter mod pagesize	*/
-
-	/* GuC's FW specific registers used in MMIO send */
-	struct {
-		u32 base;
-		unsigned int count;
-		enum forcewake_domains fw_domains;
-	} send_regs;
-
-	/* To serialize the intel_guc_send actions */
-	struct mutex send_mutex;
-
-	/* GuC's FW specific send function */
-	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
-
-	/* GuC's FW specific notify function */
-	void (*notify)(struct intel_guc *guc);
-};
-
 struct intel_huc {
 	/* Generic uC firmware management */
 	struct intel_uc_fw fw;
@@ -209,35 +113,6 @@ struct intel_huc {
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 
-/* intel_guc_loader.c */
-int intel_guc_select_fw(struct intel_guc *guc);
-int intel_guc_init_hw(struct intel_guc *guc);
-int intel_guc_suspend(struct drm_i915_private *dev_priv);
-int intel_guc_resume(struct drm_i915_private *dev_priv);
-u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
-
-/* i915_guc_submission.c */
-int i915_guc_submission_init(struct drm_i915_private *dev_priv);
-int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
-void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
-void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
-struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
-
-/* intel_guc_log.c */
-int intel_guc_log_create(struct intel_guc *guc);
-void intel_guc_log_destroy(struct intel_guc *guc);
-int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
-void i915_guc_log_register(struct drm_i915_private *dev_priv);
-void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
-
-static inline u32 guc_ggtt_offset(struct i915_vma *vma)
-{
-	u32 offset = i915_ggtt_offset(vma);
-	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
-	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
-	return offset;
-}
-
 /* intel_huc.c */
 void intel_huc_select_fw(struct intel_huc *huc);
 void intel_huc_init_hw(struct intel_huc *huc);
-- 
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] 27+ messages in thread

* [PATCH 05/10] drm/i915: Reorganize HuC authentication
  2017-09-17 12:17 [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Sagar Arun Kamble
                   ` (2 preceding siblings ...)
  2017-09-17 12:17 ` [PATCH 04/10] drm/i915/guc: Move GuC specific declarations from intel_uc.h to intel_guc.h Sagar Arun Kamble
@ 2017-09-17 12:17 ` Sagar Arun Kamble
  2017-09-17 18:41   ` Michal Wajdeczko
  2017-09-17 12:17 ` [PATCH 06/10] drm/i915/huc: Move HuC specific declarations from intel_uc.h to intel_huc.h Sagar Arun Kamble
                   ` (8 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Sagar Arun Kamble @ 2017-09-17 12:17 UTC (permalink / raw)
  To: intel-gfx

Prepared intel_auth_huc to separate HuC specific functionality
from GuC send action. Created new header intel_huc.h to group
HuC specific declarations.

v2: Changed argument preparation for AUTHENTICATE_HUC.
s/intel_auth_huc/intel_huc_auth. Deferred creation of intel_huc.h
to later patch.

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc.h |  1 +
 drivers/gpu/drm/i915/intel_huc.c | 21 ++++++---------------
 drivers/gpu/drm/i915/intel_uc.c  |  2 +-
 drivers/gpu/drm/i915/intel_uc.h  |  2 +-
 5 files changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 0767ec2..b250f39 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -145,3 +145,21 @@ int intel_guc_sample_forcewake(struct intel_guc *guc)
 
 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
+
+/**
+ * intel_guc_auth_huc() - authenticate ucode
+ * @guc: struct intel_guc*
+ * @offset: rsa offset w.r.t GGTT base of huc vma
+ *
+ * Triggers a HuC fw authentication request to the GuC via intel_guc_send
+ * AUTHENTICATE_HUC interface.
+ */
+int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset)
+{
+	u32 action[] = {
+		INTEL_GUC_ACTION_AUTHENTICATE_HUC,
+		rsa_offset
+	};
+
+	return intel_guc_send(guc, action, ARRAY_SIZE(action));
+}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 919b6e1..e7b9a8b 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -147,6 +147,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 void intel_guc_init_send_regs(struct intel_guc *guc);
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
+int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
 
 /* intel_guc_loader.c */
 int intel_guc_select_fw(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index b81c6af..6ecf344 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -226,19 +226,15 @@ void intel_huc_init_hw(struct intel_huc *huc)
 }
 
 /**
- * intel_guc_auth_huc() - authenticate ucode
- * @dev_priv: the drm_i915_device
- *
- * Triggers a HuC fw authentication request to the GuC via intel_guc_action_
- * authenticate_huc interface.
+ * intel_huc_auth() - authenticate ucode
  */
-void intel_guc_auth_huc(struct drm_i915_private *dev_priv)
+void intel_huc_auth(struct intel_huc *huc)
 {
+	struct drm_i915_private *dev_priv = huc_to_i915(huc);
 	struct intel_guc *guc = &dev_priv->guc;
-	struct intel_huc *huc = &dev_priv->huc;
 	struct i915_vma *vma;
+	u32 offset;
 	int ret;
-	u32 data[2];
 
 	if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
 		return;
@@ -251,11 +247,8 @@ void intel_guc_auth_huc(struct drm_i915_private *dev_priv)
 		return;
 	}
 
-	/* Specify auth action and where public signature is. */
-	data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC;
-	data[1] = guc_ggtt_offset(vma) + huc->fw.rsa_offset;
-
-	ret = intel_guc_send(guc, data, ARRAY_SIZE(data));
+	offset = guc_ggtt_offset(vma) + huc->fw.rsa_offset;
+	ret = intel_guc_auth_huc(guc, offset);
 	if (ret) {
 		DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret);
 		goto out;
@@ -267,7 +260,6 @@ void intel_guc_auth_huc(struct drm_i915_private *dev_priv)
 				HUC_FW_VERIFIED,
 				HUC_FW_VERIFIED,
 				50);
-
 	if (ret) {
 		DRM_ERROR("HuC: Authentication failed %d\n", ret);
 		goto out;
@@ -276,4 +268,3 @@ void intel_guc_auth_huc(struct drm_i915_private *dev_priv)
 out:
 	i915_vma_unpin(vma);
 }
-
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 4f8f0ff..9a0006a 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -352,7 +352,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	if (ret)
 		goto err_log_capture;
 
-	intel_guc_auth_huc(dev_priv);
+	intel_huc_auth(&dev_priv->huc);
 	if (i915.enable_guc_submission) {
 		if (i915.guc_log_level >= 0)
 			gen9_enable_guc_interrupts(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 30902f3..aad0b1c 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -116,6 +116,6 @@ struct intel_huc {
 /* intel_huc.c */
 void intel_huc_select_fw(struct intel_huc *huc);
 void intel_huc_init_hw(struct intel_huc *huc);
-void intel_guc_auth_huc(struct drm_i915_private *dev_priv);
+void intel_huc_auth(struct intel_huc *huc);
 
 #endif
-- 
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] 27+ messages in thread

* [PATCH 06/10] drm/i915/huc: Move HuC specific declarations from intel_uc.h to intel_huc.h
  2017-09-17 12:17 [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Sagar Arun Kamble
                   ` (3 preceding siblings ...)
  2017-09-17 12:17 ` [PATCH 05/10] drm/i915: Reorganize HuC authentication Sagar Arun Kamble
@ 2017-09-17 12:17 ` Sagar Arun Kamble
  2017-09-17 20:13   ` Michal Wajdeczko
  2017-09-17 12:17 ` [PATCH 07/10] drm/i915/guc: Fix GuC interaction in reset/suspend scenarios Sagar Arun Kamble
                   ` (7 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Sagar Arun Kamble @ 2017-09-17 12:17 UTC (permalink / raw)
  To: intel-gfx

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_huc.h | 39 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.h  | 12 ------------
 3 files changed, 40 insertions(+), 12 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_huc.h

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b0f38bb..a3b7435 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -60,6 +60,7 @@
 #include "intel_dpll_mgr.h"
 #include "intel_uc.h"
 #include "intel_guc.h"
+#include "intel_huc.h"
 #include "intel_lrc.h"
 #include "intel_ringbuffer.h"
 
diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
new file mode 100644
index 0000000..4604a40
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -0,0 +1,39 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+#ifndef _INTEL_HUC_H_
+#define _INTEL_HUC_H_
+
+struct intel_huc {
+	/* Generic uC firmware management */
+	struct intel_uc_fw fw;
+
+	/* HuC-specific additions */
+};
+
+/* intel_huc.c */
+void intel_huc_select_fw(struct intel_huc *huc);
+void intel_huc_init_hw(struct intel_huc *huc);
+void intel_huc_auth(struct intel_huc *huc);
+
+#endif
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index aad0b1c..0d346ef 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -98,13 +98,6 @@ struct intel_uc_fw {
 	uint32_t ucode_offset;
 };
 
-struct intel_huc {
-	/* Generic uC firmware management */
-	struct intel_uc_fw fw;
-
-	/* HuC-specific additions */
-};
-
 /* intel_uc.c */
 void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
 void intel_uc_init_early(struct drm_i915_private *dev_priv);
@@ -113,9 +106,4 @@ struct intel_huc {
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 
-/* intel_huc.c */
-void intel_huc_select_fw(struct intel_huc *huc);
-void intel_huc_init_hw(struct intel_huc *huc);
-void intel_huc_auth(struct intel_huc *huc);
-
 #endif
-- 
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] 27+ messages in thread

* [PATCH 07/10] drm/i915/guc: Fix GuC interaction in reset/suspend scenarios
  2017-09-17 12:17 [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Sagar Arun Kamble
                   ` (4 preceding siblings ...)
  2017-09-17 12:17 ` [PATCH 06/10] drm/i915/huc: Move HuC specific declarations from intel_uc.h to intel_huc.h Sagar Arun Kamble
@ 2017-09-17 12:17 ` Sagar Arun Kamble
  2017-09-17 12:17 ` [PATCH 08/10] drm/i915/guc: Fix GuC HW/SW state cleanup in unload path Sagar Arun Kamble
                   ` (6 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Sagar Arun Kamble @ 2017-09-17 12:17 UTC (permalink / raw)
  To: intel-gfx

Pass intel_guc to intel_guc_suspend/resume instead of drm_i915_private.
guc_ggtt_invalidate/guc_interrupts should be disabled towards
end of reset/suspend post GuC suspension as these are setup back again
during recovery/resume.

Prepared helpers intel_guc_pause and intel_guc_unpause that will do
teardown/bringup of this setup along with suspension/resumption of GuC if
loaded. These helpers can then be used along the system or runtime
suspend/resume paths as applicable. Currently post system resume, since
GuC is loaded completely system_resume function for GuC is doing
nothing. We rely on the setup happening in intel_uc_init_hw path.
During runtime_suspend we will call intel_guc_pause helper.
Another helper added is intel_guc_reset_prepare, this will make sure that
disabling of ggtt_invalidate and GuC interrupts happens prior to reset and
updates the firmware load status to PENDING.

v2: Updated commit message. Added note about skipped call to
intel_guc_system_resume. guc_enable/disable_communication was moved to
earlier patch so corresponding rebase. (Michal Wajdeczko)

v3: Introduction of intel_uc_runtime/system_suspend/resume. Updated
changes for enable/disable_communication.

v4: Squashed change to update parameters of intel_guc_suspend/resume from
earlier patch.

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c            |  11 ++-
 drivers/gpu/drm/i915/i915_gem.c            |   4 +-
 drivers/gpu/drm/i915/i915_guc_submission.c |  52 ------------
 drivers/gpu/drm/i915/intel_guc.c           | 129 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc.h           |   7 +-
 drivers/gpu/drm/i915/intel_uc.c            |  30 +++++++
 drivers/gpu/drm/i915/intel_uc.h            |   5 ++
 7 files changed, 179 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5c111ea..4751679 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1684,6 +1684,11 @@ static int i915_drm_resume(struct drm_device *dev)
 
 	drm_mode_config_reset(dev);
 
+	/*
+	 * NB: Full gem reinitialization is being done during i915_drm_resume,
+	 * hence intel_guc_system_resume will be of no use. If full
+	 * reinitialization is avoided, need to call intel_guc_system_resume.
+	 */
 	mutex_lock(&dev->struct_mutex);
 	if (i915_gem_init_hw(dev_priv)) {
 		DRM_ERROR("failed to re-initialize GPU, declaring wedged!\n");
@@ -1691,8 +1696,6 @@ static int i915_drm_resume(struct drm_device *dev)
 	}
 	mutex_unlock(&dev->struct_mutex);
 
-	intel_guc_resume(dev_priv);
-
 	intel_modeset_init_hw(dev);
 
 	spin_lock_irq(&dev_priv->irq_lock);
@@ -2493,7 +2496,7 @@ static int intel_runtime_suspend(struct device *kdev)
 	 */
 	i915_gem_runtime_suspend(dev_priv);
 
-	intel_guc_suspend(dev_priv);
+	intel_uc_runtime_suspend(dev_priv);
 
 	intel_runtime_pm_disable_interrupts(dev_priv);
 
@@ -2578,7 +2581,7 @@ static int intel_runtime_resume(struct device *kdev)
 	if (intel_uncore_unclaimed_mmio(dev_priv))
 		DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
 
-	intel_guc_resume(dev_priv);
+	intel_uc_runtime_resume(dev_priv);
 
 	if (IS_GEN9_LP(dev_priv)) {
 		bxt_disable_dc9(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f445587..7354307 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2847,6 +2847,8 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv)
 
 	i915_gem_revoke_fences(dev_priv);
 
+	intel_uc_reset_prepare(dev_priv);
+
 	return err;
 }
 
@@ -4575,7 +4577,7 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	i915_gem_contexts_lost(dev_priv);
 	mutex_unlock(&dev->struct_mutex);
 
-	intel_guc_suspend(dev_priv);
+	intel_uc_system_suspend(dev_priv);
 
 	cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
 	cancel_delayed_work_sync(&dev_priv->gt.retire_work);
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index f37cd47..f0ff874 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1206,55 +1206,3 @@ void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
 	guc_client_free(guc->execbuf_client);
 	guc->execbuf_client = NULL;
 }
-
-/**
- * intel_guc_suspend() - notify GuC entering suspend state
- * @dev_priv:	i915 device private
- */
-int intel_guc_suspend(struct drm_i915_private *dev_priv)
-{
-	struct intel_guc *guc = &dev_priv->guc;
-	struct i915_gem_context *ctx;
-	u32 data[3];
-
-	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
-		return 0;
-
-	gen9_disable_guc_interrupts(dev_priv);
-
-	ctx = dev_priv->kernel_context;
-
-	data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
-	/* any value greater than GUC_POWER_D0 */
-	data[1] = GUC_POWER_D1;
-	/* first page is shared data with GuC */
-	data[2] = guc_ggtt_offset(ctx->engine[RCS].state) + LRC_GUCSHR_PN * PAGE_SIZE;
-
-	return intel_guc_send(guc, data, ARRAY_SIZE(data));
-}
-
-/**
- * intel_guc_resume() - notify GuC resuming from suspend state
- * @dev_priv:	i915 device private
- */
-int intel_guc_resume(struct drm_i915_private *dev_priv)
-{
-	struct intel_guc *guc = &dev_priv->guc;
-	struct i915_gem_context *ctx;
-	u32 data[3];
-
-	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
-		return 0;
-
-	if (i915.guc_log_level >= 0)
-		gen9_enable_guc_interrupts(dev_priv);
-
-	ctx = dev_priv->kernel_context;
-
-	data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
-	data[1] = GUC_POWER_D0;
-	/* first page is shared data with GuC */
-	data[2] = guc_ggtt_offset(ctx->engine[RCS].state) + LRC_GUCSHR_PN * PAGE_SIZE;
-
-	return intel_guc_send(guc, data, ARRAY_SIZE(data));
-}
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index b250f39..904cc58 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -163,3 +163,132 @@ int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset)
 
 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
+
+/**
+ * intel_guc_suspend() - notify GuC entering suspend state
+ */
+static int intel_guc_suspend(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct i915_gem_context *ctx;
+	u32 data[3];
+
+	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+		return 0;
+
+	ctx = dev_priv->kernel_context;
+
+	data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
+	/* any value greater than GUC_POWER_D0 */
+	data[1] = GUC_POWER_D1;
+	/* first page is shared data with GuC */
+	data[2] = guc_ggtt_offset(ctx->engine[RCS].state);
+
+	return intel_guc_send(guc, data, ARRAY_SIZE(data));
+}
+
+/**
+ * intel_guc_resume() - notify GuC resuming from suspend state
+ */
+static int intel_guc_resume(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct i915_gem_context *ctx;
+	u32 data[3];
+
+	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+		return 0;
+
+	ctx = dev_priv->kernel_context;
+
+	data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
+	data[1] = GUC_POWER_D0;
+	/* first page is shared data with GuC */
+	data[2] = guc_ggtt_offset(ctx->engine[RCS].state);
+
+	return intel_guc_send(guc, data, ARRAY_SIZE(data));
+}
+
+static void intel_guc_sanitize(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+	i915_ggtt_disable_guc(dev_priv);
+	gen9_disable_guc_interrupts(dev_priv);
+}
+
+void intel_guc_reset_prepare(struct intel_guc *guc)
+{
+	if (!i915.enable_guc_loading)
+		return;
+
+	intel_guc_sanitize(guc);
+	guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING;
+}
+
+static int intel_guc_pause(struct intel_guc *guc)
+{
+	int ret = 0;
+
+	ret = intel_guc_suspend(guc);
+	intel_guc_sanitize(guc);
+
+	return ret;
+}
+
+static int intel_guc_unpause(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	int ret = 0;
+
+	if (i915.guc_log_level >= 0)
+		gen9_enable_guc_interrupts(dev_priv);
+	i915_ggtt_enable_guc(dev_priv);
+	ret = intel_guc_resume(guc);
+
+	return ret;
+}
+
+int intel_guc_runtime_suspend(struct intel_guc *guc)
+{
+	if (!i915.enable_guc_loading)
+		return 0;
+
+	return intel_guc_pause(guc);
+}
+
+int intel_guc_runtime_resume(struct intel_guc *guc)
+{
+	if (!i915.enable_guc_loading)
+		return 0;
+
+	return intel_guc_unpause(guc);
+}
+
+int intel_guc_system_suspend(struct intel_guc *guc)
+{
+	int ret = 0;
+
+	if (!i915.enable_guc_loading)
+		return ret;
+
+	ret = intel_guc_pause(guc);
+	guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING;
+
+	return ret;
+}
+
+int intel_guc_system_resume(struct intel_guc *guc)
+{
+	int ret = 0;
+
+	if (!i915.enable_guc_loading)
+		return ret;
+
+	/*
+	 * Placeholder for GuC resume from system suspend/freeze states.
+	 * Currently full reinitialization of GEM and GuC happens along
+	 * these paths, Hence this function is doing nothing.
+	 */
+	return ret;
+}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index e7b9a8b..e912667 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -148,12 +148,15 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
 int intel_guc_sample_forcewake(struct intel_guc *guc);
 int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
+void intel_guc_reset_prepare(struct intel_guc *guc);
+int intel_guc_runtime_suspend(struct intel_guc *guc);
+int intel_guc_runtime_resume(struct intel_guc *guc);
+int intel_guc_system_suspend(struct intel_guc *guc);
+int intel_guc_system_resume(struct intel_guc *guc);
 
 /* intel_guc_loader.c */
 int intel_guc_select_fw(struct intel_guc *guc);
 int intel_guc_init_hw(struct intel_guc *guc);
-int intel_guc_suspend(struct drm_i915_private *dev_priv);
-int intel_guc_resume(struct drm_i915_private *dev_priv);
 u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
 
 /* i915_guc_submission.c */
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 9a0006a..759d8f4e 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -420,3 +420,33 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 
 	i915_ggtt_disable_guc(dev_priv);
 }
+
+void intel_uc_reset_prepare(struct drm_i915_private *dev_priv)
+{
+	intel_guc_reset_prepare(&dev_priv->guc);
+	guc_disable_communication(&dev_priv->guc);
+}
+
+void intel_uc_runtime_suspend(struct drm_i915_private *dev_priv)
+{
+	intel_guc_runtime_suspend(&dev_priv->guc);
+	guc_disable_communication(&dev_priv->guc);
+}
+
+int intel_uc_runtime_resume(struct drm_i915_private *dev_priv)
+{
+	intel_guc_runtime_resume(&dev_priv->guc);
+	return guc_enable_communication(&dev_priv->guc);
+}
+
+void intel_uc_system_suspend(struct drm_i915_private *dev_priv)
+{
+	intel_guc_system_suspend(&dev_priv->guc);
+	guc_disable_communication(&dev_priv->guc);
+}
+
+int intel_uc_system_resume(struct drm_i915_private *dev_priv)
+{
+	intel_guc_system_resume(&dev_priv->guc);
+	return guc_enable_communication(&dev_priv->guc);
+}
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 0d346ef..4d49a19 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -105,5 +105,10 @@ struct intel_uc_fw {
 void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
+void intel_uc_reset_prepare(struct drm_i915_private *dev_priv);
+void intel_uc_runtime_suspend(struct drm_i915_private *dev_priv);
+int intel_uc_runtime_resume(struct drm_i915_private *dev_priv);
+void intel_uc_system_suspend(struct drm_i915_private *dev_priv);
+int intel_uc_system_resume(struct drm_i915_private *dev_priv);
 
 #endif
-- 
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] 27+ messages in thread

* [PATCH 08/10] drm/i915/guc: Fix GuC HW/SW state cleanup in unload path
  2017-09-17 12:17 [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Sagar Arun Kamble
                   ` (5 preceding siblings ...)
  2017-09-17 12:17 ` [PATCH 07/10] drm/i915/guc: Fix GuC interaction in reset/suspend scenarios Sagar Arun Kamble
@ 2017-09-17 12:17 ` Sagar Arun Kamble
  2017-09-21 18:33   ` Oscar Mateo
  2017-09-17 12:17 ` [PATCH 09/10] drm/i915/guc: Enable default/critical logging in GuC by default from GuC v9 Sagar Arun Kamble
                   ` (5 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Sagar Arun Kamble @ 2017-09-17 12:17 UTC (permalink / raw)
  To: intel-gfx

Teardown of GuC HW/SW state was not properly done in unload path.
guc_submission_disable was called as part of intel_uc_fini_hw which
happens post gem_unload in the i915_driver_unload path.
s/i915_gem_fini/i915_gem_cleanup as it looks more suitable as that
function does cleanup.
To differentiate the tasks during suspend and unload w.r.t GuC this
patch introduces new function i915_gem_fini which in addition to
disabling GuC interfaces also disables GuC submission during which
communication with GuC is needed for destroying doorbell.
i915_gem_fini is copy of i915_gem_suspend with difference w.r.t
GuC operations. To achieve this, new helpers i915_gem_context_suspend
and i915_gem_suspend_complete are prepared.

This patch updates the functions responsibilities as follows:
1. intel_uc_fini_hw: Disable all things that involve communication with
   GuC and internal operation of GuC firmware, currently submission.
   Post this disable all other state like guc_ggtt_invalidate,
   guc_communication and guc_interrupts.
2. intel_uc_cleanup: Free up all UC related memory and other data.

v2: Prepared i915_gem_unload. (Michal)

v3: Moved guc_free_load_err_log past i915.enable_guc_loading check in
intel_uc_cleanup_hw. Prepared i915_gem_contexts_suspend and
i915_gem_suspend_complete that are used in the two paths
i915_gem_suspend and i915_gem_unload. Unload specific actions like
disabling GuC submission and GuC communication are being done in
i915_gem_unload. Commit message update. (Michał Winiarski)

v4: prepared i915_gem_cleanup and intel_uc_cleanup.

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c  | 12 +++++-----
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/i915_gem.c  | 52 ++++++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_guc.c | 41 +++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc.h |  3 +++
 drivers/gpu/drm/i915/intel_uc.c  | 39 +++++-------------------------
 drivers/gpu/drm/i915/intel_uc.h  |  1 +
 7 files changed, 105 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 4751679..f9622fc 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -596,13 +596,13 @@ static bool i915_switcheroo_can_switch(struct pci_dev *pdev)
 	.can_switch = i915_switcheroo_can_switch,
 };
 
-static void i915_gem_fini(struct drm_i915_private *dev_priv)
+static void i915_gem_cleanup(struct drm_i915_private *dev_priv)
 {
 	/* Flush any outstanding unpin_work. */
 	i915_gem_drain_workqueue(dev_priv);
 
 	mutex_lock(&dev_priv->drm.struct_mutex);
-	intel_uc_fini_hw(dev_priv);
+	intel_uc_cleanup(dev_priv);
 	i915_gem_cleanup_engines(dev_priv);
 	i915_gem_contexts_fini(dev_priv);
 	i915_gem_cleanup_userptr(dev_priv);
@@ -683,9 +683,9 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	return 0;
 
 cleanup_gem:
-	if (i915_gem_suspend(dev_priv))
+	if (i915_gem_fini(dev_priv))
 		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
-	i915_gem_fini(dev_priv);
+	i915_gem_cleanup(dev_priv);
 cleanup_uc:
 	intel_uc_fini_fw(dev_priv);
 cleanup_irq:
@@ -1376,7 +1376,7 @@ void i915_driver_unload(struct drm_device *dev)
 
 	i915_driver_unregister(dev_priv);
 
-	if (i915_gem_suspend(dev_priv))
+	if (i915_gem_fini(dev_priv))
 		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
 
 	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
@@ -1410,7 +1410,7 @@ void i915_driver_unload(struct drm_device *dev)
 	cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
 	i915_reset_error_state(dev_priv);
 
-	i915_gem_fini(dev_priv);
+	i915_gem_cleanup(dev_priv);
 	intel_uc_fini_fw(dev_priv);
 	intel_fbc_cleanup_cfb(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a3b7435..341f2ba 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3671,6 +3671,7 @@ void i915_gem_reset_engine(struct intel_engine_cs *engine,
 int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv,
 			   unsigned int flags);
 int __must_check i915_gem_suspend(struct drm_i915_private *dev_priv);
+int __must_check i915_gem_fini(struct drm_i915_private *dev_priv);
 void i915_gem_resume(struct drm_i915_private *dev_priv);
 int i915_gem_fault(struct vm_fault *vmf);
 int i915_gem_object_wait(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7354307..99f55bd 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4545,12 +4545,11 @@ void i915_gem_sanitize(struct drm_i915_private *i915)
 	}
 }
 
-int i915_gem_suspend(struct drm_i915_private *dev_priv)
+static int i915_gem_contexts_suspend(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = &dev_priv->drm;
 	int ret;
 
-	intel_runtime_pm_get(dev_priv);
 	intel_suspend_gt_powersave(dev_priv);
 
 	mutex_lock(&dev->struct_mutex);
@@ -4577,8 +4576,15 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	i915_gem_contexts_lost(dev_priv);
 	mutex_unlock(&dev->struct_mutex);
 
-	intel_uc_system_suspend(dev_priv);
+	return 0;
+
+err_unlock:
+	mutex_unlock(&dev->struct_mutex);
+	return ret;
+}
 
+static void i915_gem_suspend_complete(struct drm_i915_private *dev_priv)
+{
 	cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
 	cancel_delayed_work_sync(&dev_priv->gt.retire_work);
 
@@ -4615,12 +4621,48 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	 * machine in an unusable condition.
 	 */
 	i915_gem_sanitize(dev_priv);
+}
+
+int i915_gem_suspend(struct drm_i915_private *dev_priv)
+{
+	int ret;
+
+	intel_runtime_pm_get(dev_priv);
+
+	ret = i915_gem_contexts_suspend(dev_priv);
+	if (ret && ret != -EIO)
+		goto err_suspend;
+
+	intel_uc_system_suspend(dev_priv);
+
+	i915_gem_suspend_complete(dev_priv);
 
 	intel_runtime_pm_put(dev_priv);
 	return 0;
 
-err_unlock:
-	mutex_unlock(&dev->struct_mutex);
+err_suspend:
+	intel_runtime_pm_put(dev_priv);
+	return ret;
+}
+
+int i915_gem_fini(struct drm_i915_private *dev_priv)
+{
+	int ret;
+
+	intel_runtime_pm_get(dev_priv);
+
+	ret = i915_gem_contexts_suspend(dev_priv);
+	if (ret && ret != -EIO)
+		goto err_suspend;
+
+	intel_uc_fini_hw(dev_priv);
+
+	i915_gem_suspend_complete(dev_priv);
+
+	intel_runtime_pm_put(dev_priv);
+	return 0;
+
+err_suspend:
 	intel_runtime_pm_put(dev_priv);
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 904cc58..a288c70 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -292,3 +292,44 @@ int intel_guc_system_resume(struct intel_guc *guc)
 	 */
 	return ret;
 }
+
+void intel_guc_fini_hw(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct drm_device *dev = &dev_priv->drm;
+
+	if (i915.enable_guc_submission) {
+		mutex_lock(&dev->struct_mutex);
+		i915_guc_submission_disable(dev_priv);
+		mutex_unlock(&dev->struct_mutex);
+		intel_guc_reset_prepare(guc);
+	}
+}
+
+void intel_guc_capture_load_err_log(struct intel_guc *guc)
+{
+	if (!guc->log.vma || i915.guc_log_level < 0)
+		return;
+
+	if (!guc->load_err_log)
+		guc->load_err_log = i915_gem_object_get(guc->log.vma->obj);
+}
+
+static void guc_free_load_err_log(struct intel_guc *guc)
+{
+	if (guc->load_err_log)
+		i915_gem_object_put(guc->load_err_log);
+}
+
+void intel_guc_cleanup(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+	if (!i915.enable_guc_loading)
+		return;
+
+	guc_free_load_err_log(guc);
+
+	if (i915.enable_guc_submission)
+		i915_guc_submission_fini(dev_priv);
+}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index e912667..4d7561c 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -153,6 +153,9 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 int intel_guc_runtime_resume(struct intel_guc *guc);
 int intel_guc_system_suspend(struct intel_guc *guc);
 int intel_guc_system_resume(struct intel_guc *guc);
+void intel_guc_fini_hw(struct intel_guc *guc);
+void intel_guc_capture_load_err_log(struct intel_guc *guc);
+void intel_guc_cleanup(struct intel_guc *guc);
 
 /* intel_guc_loader.c */
 int intel_guc_select_fw(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 759d8f4e..52e55e1 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -250,23 +250,6 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
 	__intel_uc_fw_fini(&dev_priv->huc.fw);
 }
 
-static void guc_capture_load_err_log(struct intel_guc *guc)
-{
-	if (!guc->log.vma || i915.guc_log_level < 0)
-		return;
-
-	if (!guc->load_err_log)
-		guc->load_err_log = i915_gem_object_get(guc->log.vma->obj);
-
-	return;
-}
-
-static void guc_free_load_err_log(struct intel_guc *guc)
-{
-	if (guc->load_err_log)
-		i915_gem_object_put(guc->load_err_log);
-}
-
 static int guc_enable_communication(struct intel_guc *guc)
 {
 	struct drm_i915_private *dev_priv = guc_to_i915(guc);
@@ -377,7 +360,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	guc_disable_communication(guc);
 	gen9_disable_guc_interrupts(dev_priv);
 err_log_capture:
-	guc_capture_load_err_log(guc);
+	intel_guc_capture_load_err_log(guc);
 err_submission:
 	if (i915.enable_guc_submission)
 		i915_guc_submission_fini(dev_priv);
@@ -403,22 +386,12 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 {
-	guc_free_load_err_log(&dev_priv->guc);
-
-	if (!i915.enable_guc_loading)
-		return;
-
-	if (i915.enable_guc_submission)
-		i915_guc_submission_disable(dev_priv);
-
-	guc_disable_communication(&dev_priv->guc);
-
-	if (i915.enable_guc_submission) {
-		gen9_disable_guc_interrupts(dev_priv);
-		i915_guc_submission_fini(dev_priv);
-	}
+	intel_guc_fini_hw(&dev_priv->guc);
+}
 
-	i915_ggtt_disable_guc(dev_priv);
+void intel_uc_cleanup(struct drm_i915_private *dev_priv)
+{
+	intel_guc_cleanup(&dev_priv->guc);
 }
 
 void intel_uc_reset_prepare(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 4d49a19..9fd8626 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -105,6 +105,7 @@ struct intel_uc_fw {
 void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
+void intel_uc_cleanup(struct drm_i915_private *dev_priv);
 void intel_uc_reset_prepare(struct drm_i915_private *dev_priv);
 void intel_uc_runtime_suspend(struct drm_i915_private *dev_priv);
 int intel_uc_runtime_resume(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] 27+ messages in thread

* [PATCH 09/10] drm/i915/guc: Enable default/critical logging in GuC by default from GuC v9
  2017-09-17 12:17 [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Sagar Arun Kamble
                   ` (6 preceding siblings ...)
  2017-09-17 12:17 ` [PATCH 08/10] drm/i915/guc: Fix GuC HW/SW state cleanup in unload path Sagar Arun Kamble
@ 2017-09-17 12:17 ` Sagar Arun Kamble
  2017-09-17 12:17 ` [PATCH 10/10] drm/i915/guc: Remove i915_guc_log_unregister Sagar Arun Kamble
                   ` (4 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Sagar Arun Kamble @ 2017-09-17 12:17 UTC (permalink / raw)
  To: intel-gfx; +Cc: Chheda Harsh J, Fry Gregory P

With GuC v9, new type of Default/critical logging in GuC to enable
capturing minimal important logs in production systems efficiently.
This patch enables this logging in GuC by default always. It should
be noted that streaming support with half-full interrupt mechanism
that is present for normal logging is not present for this type of
logging.

v2: Emulated GuC critical logging through i915.guc_log_level.

v3: Commit message update. Enable default/critical logging in GuC always.
Fixed RPM wake during guc_log_unregister in the unload path.

v4: Moved RPM wake change to separate patch. Removed GUC_DEBUG_RESERVED
and updated name of new bit to be version agnostic. Updated parameter to
struct intel_guc * and name of macro NEEDS_GUC_CRITICAL_LOGGING.
Removed explicit clearing of GUC_CRITICAL_LOGGING_DISABLED from
params[GUC_CTL_DEBUG] as it is unnecessary. (Michal Wajdeczko)

v5: Removed GUC_CRITICAL_LOGGING_DISABLED. Added HAS_GUC check to
GUC_NEEDS_CRITICAL_LOGGING. (Michal Wajdeczko)

v6: More refined version of GUC_NEEDS_CRITICAL_LOGGING. Commit message
update. (Michal Wajdeczko)

Cc: Chheda Harsh J <harsh.j.chheda@intel.com>
Cc: Fry Gregory P <gregory.p.fry@intel.com>
Cc: Spotswood John A <john.a.spotswood@intel.com>
Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_fwif.h |  4 ++--
 drivers/gpu/drm/i915/intel_guc_log.c  | 13 ++++++++++++-
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 7eb6b4f..fed875a 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -127,7 +127,6 @@
 #define   GUC_PROFILE_ENABLED		(1 << 7)
 #define   GUC_WQ_TRACK_ENABLED		(1 << 8)
 #define   GUC_ADS_ENABLED		(1 << 9)
-#define   GUC_DEBUG_RESERVED		(1 << 10)
 #define   GUC_ADS_ADDR_SHIFT		11
 #define   GUC_ADS_ADDR_MASK		0xfffff800
 
@@ -539,7 +538,8 @@ struct guc_log_buffer_state {
 		u32 logging_enabled:1;
 		u32 reserved1:3;
 		u32 verbosity:4;
-		u32 reserved2:24;
+		u32 critical_logging_enabled:1;
+		u32 reserved2:23;
 	};
 	u32 value;
 } __packed;
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 5c7e0c2..1bd91ec 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -587,10 +587,18 @@ void intel_guc_log_destroy(struct intel_guc *guc)
 	i915_vma_unpin_and_release(&guc->log.vma);
 }
 
+/*
+ * Critical logging in GuC is to be enabled always from GuC v9+.
+ * (for KBL - v9.39+)
+ */
+#define GUC_NEEDS_CRITICAL_LOGGING(guc)	\
+	(HAS_GUC(guc_to_i915(guc)) && \
+	 (guc->fw.major_ver_found >= 9) && \
+	 (guc->fw.minor_ver_found >= (IS_KABYLAKE(guc_to_i915(guc)) ? 39 : 0)))
+
 int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
 {
 	struct intel_guc *guc = &dev_priv->guc;
-
 	union guc_log_control log_param;
 	int ret;
 
@@ -604,6 +612,9 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
 	if (!log_param.logging_enabled && (i915.guc_log_level < 0))
 		return 0;
 
+	if (GUC_NEEDS_CRITICAL_LOGGING(guc))
+		log_param.critical_logging_enabled = 1;
+
 	ret = guc_log_control(guc, log_param.value);
 	if (ret < 0) {
 		DRM_DEBUG_DRIVER("guc_logging_control action failed %d\n", 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] 27+ messages in thread

* [PATCH 10/10] drm/i915/guc: Remove i915_guc_log_unregister
  2017-09-17 12:17 [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Sagar Arun Kamble
                   ` (7 preceding siblings ...)
  2017-09-17 12:17 ` [PATCH 09/10] drm/i915/guc: Enable default/critical logging in GuC by default from GuC v9 Sagar Arun Kamble
@ 2017-09-17 12:17 ` Sagar Arun Kamble
  2017-09-17 19:30 ` [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Michal Wajdeczko
                   ` (3 subsequent siblings)
  12 siblings, 0 replies; 27+ messages in thread
From: Sagar Arun Kamble @ 2017-09-17 12:17 UTC (permalink / raw)
  To: intel-gfx

Functionality needed to disable GuC interrupts and cleanup the
runtime/relay data structures is already covered in the unload path
via intel_guc_fini_hw and intel_guc_cleanup hence remove
i915_guc_log_unregister

v2: Removed the function i915_guc_log_unregister.

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c      |  1 -
 drivers/gpu/drm/i915/intel_guc.h     |  1 -
 drivers/gpu/drm/i915/intel_guc_log.c | 12 ------------
 3 files changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index f9622fc..99d2e27 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1252,7 +1252,6 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 	i915_perf_unregister(dev_priv);
 
 	i915_teardown_sysfs(dev_priv);
-	i915_guc_log_unregister(dev_priv);
 	drm_dev_unregister(&dev_priv->drm);
 
 	i915_gem_shrinker_cleanup(dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 4d7561c..1767059 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -174,6 +174,5 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 void intel_guc_log_destroy(struct intel_guc *guc);
 int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
 void i915_guc_log_register(struct drm_i915_private *dev_priv);
-void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
 
 #endif
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 1bd91ec..b71fe8d 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -660,15 +660,3 @@ void i915_guc_log_register(struct drm_i915_private *dev_priv)
 	guc_log_late_setup(&dev_priv->guc);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 }
-
-void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
-{
-	if (!i915.enable_guc_submission)
-		return;
-
-	mutex_lock(&dev_priv->drm.struct_mutex);
-	/* GuC logging is currently the only user of Guc2Host interrupts */
-	gen9_disable_guc_interrupts(dev_priv);
-	guc_log_runtime_destroy(&dev_priv->guc);
-	mutex_unlock(&dev_priv->drm.struct_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] 27+ messages in thread

* Re: [PATCH 04/10] drm/i915/guc: Move GuC specific declarations from intel_uc.h to intel_guc.h
  2017-09-17 12:17 ` [PATCH 04/10] drm/i915/guc: Move GuC specific declarations from intel_uc.h to intel_guc.h Sagar Arun Kamble
@ 2017-09-17 18:24   ` Michal Wajdeczko
  2017-09-18  6:10     ` Kamble, Sagar A
  0 siblings, 1 reply; 27+ messages in thread
From: Michal Wajdeczko @ 2017-09-17 18:24 UTC (permalink / raw)
  To: intel-gfx, Sagar Arun Kamble

On Sun, 17 Sep 2017 14:17:28 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

Maybe to make this refactoring series much clearer you should start with  
this patch ?
and btw, please don't forget about adding commit description.

> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |   1 +
>  drivers/gpu/drm/i915/intel_guc.h | 126  
> +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_uc.h  | 125  
> --------------------------------------
>  3 files changed, 127 insertions(+), 125 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h  
> b/drivers/gpu/drm/i915/i915_drv.h
> index 5aecbf7..b0f38bb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -59,6 +59,7 @@
>  #include "intel_bios.h"
>  #include "intel_dpll_mgr.h"
>  #include "intel_uc.h"
> +#include "intel_guc.h"

Hmm, the goal of previous (unfinished) refactoring was to
wrap all details about Guc/Huc into generic single "uc"
component. Thus any "guc"/"huc" specific headers shall
be included by master "intel_uc.h". If this is hard to
make, then maybe we should postpone changes in .h and
current limit refactoring only to .c files.

Michal

>  #include "intel_lrc.h"
>  #include "intel_ringbuffer.h"
> diff --git a/drivers/gpu/drm/i915/intel_guc.h  
> b/drivers/gpu/drm/i915/intel_guc.h
> index 06f2d27..919b6e1 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -24,6 +24,102 @@
>  #ifndef _INTEL_GUC_H_
>  #define _INTEL_GUC_H_
> +/*
> + * This structure primarily describes the GEM object shared with the  
> GuC.
> + * The specs sometimes refer to this object as a "GuC context", but we  
> use
> + * the term "client" to avoid confusion with hardware contexts. This
> + * GEM object is held for the entire lifetime of our interaction with
> + * the GuC, being allocated before the GuC is loaded with its firmware.
> + * Because there's no way to update the address used by the GuC after
> + * initialisation, the shared object must stay pinned into the GGTT as
> + * long as the GuC is in use. We also keep the first page (only) mapped
> + * into kernel address space, as it includes shared data that must be
> + * updated on every request submission.
> + *
> + * The single GEM object described here is actually made up of several
> + * separate areas, as far as the GuC is concerned. The first page (kept
> + * kmap'd) includes the "process descriptor" which holds sequence data  
> for
> + * the doorbell, and one cacheline which actually *is* the doorbell; a
> + * write to this will "ring the doorbell" (i.e. send an interrupt to the
> + * GuC). The subsequent  pages of the client object constitute the work
> + * queue (a circular array of work items), again described in the  
> process
> + * descriptor. Work queue pages are mapped momentarily as required.
> + */
> +struct i915_guc_client {
> +	struct i915_vma *vma;
> +	void *vaddr;
> +	struct i915_gem_context *owner;
> +	struct intel_guc *guc;
> +
> +	uint32_t engines;		/* bitmap of (host) engine ids	*/
> +	uint32_t priority;
> +	u32 stage_id;
> +	uint32_t proc_desc_offset;
> +
> +	u16 doorbell_id;
> +	unsigned long doorbell_offset;
> +
> +	spinlock_t wq_lock;
> +	/* Per-engine counts of GuC submissions */
> +	uint64_t submissions[I915_NUM_ENGINES];
> +};
> +
> +struct intel_guc_log {
> +	uint32_t flags;
> +	struct i915_vma *vma;
> +	/* The runtime stuff gets created only when GuC logging gets enabled */
> +	struct {
> +		void *buf_addr;
> +		struct workqueue_struct *flush_wq;
> +		struct work_struct flush_work;
> +		struct rchan *relay_chan;
> +	} runtime;
> +	/* logging related stats */
> +	u32 capture_miss_count;
> +	u32 flush_interrupt_count;
> +	u32 prev_overflow_count[GUC_MAX_LOG_BUFFER];
> +	u32 total_overflow_count[GUC_MAX_LOG_BUFFER];
> +	u32 flush_count[GUC_MAX_LOG_BUFFER];
> +};
> +
> +struct intel_guc {
> +	struct intel_uc_fw fw;
> +	struct intel_guc_log log;
> +	struct intel_guc_ct ct;
> +
> +	/* Log snapshot if GuC errors during load */
> +	struct drm_i915_gem_object *load_err_log;
> +
> +	/* intel_guc_recv interrupt related state */
> +	bool interrupts_enabled;
> +
> +	struct i915_vma *ads_vma;
> +	struct i915_vma *stage_desc_pool;
> +	void *stage_desc_pool_vaddr;
> +	struct ida stage_ids;
> +
> +	struct i915_guc_client *execbuf_client;
> +
> +	DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
> +	uint32_t db_cacheline;		/* Cyclic counter mod pagesize	*/
> +
> +	/* GuC's FW specific registers used in MMIO send */
> +	struct {
> +		u32 base;
> +		unsigned int count;
> +		enum forcewake_domains fw_domains;
> +	} send_regs;
> +
> +	/* To serialize the intel_guc_send actions */
> +	struct mutex send_mutex;
> +
> +	/* GuC's FW specific send function */
> +	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
> +
> +	/* GuC's FW specific notify function */
> +	void (*notify)(struct intel_guc *guc);
> +};
> +
>  static inline int intel_guc_send(struct intel_guc *guc,
>  				 const u32 *action,
>  				 u32 len)
> @@ -36,6 +132,15 @@ static inline void intel_guc_notify(struct intel_guc  
> *guc)
>  	guc->notify(guc);
>  }
> +static inline u32 guc_ggtt_offset(struct i915_vma *vma)
> +{
> +	u32 offset = i915_ggtt_offset(vma);
> +
> +	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
> +	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
> +	return offset;
> +}
> +
>  /* intel_guc.c */
>  int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32  
> len);
>  void intel_guc_init_early(struct intel_guc *guc);
> @@ -43,4 +148,25 @@ static inline void intel_guc_notify(struct intel_guc  
> *guc)
>  int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32  
> len);
>  int intel_guc_sample_forcewake(struct intel_guc *guc);
> +/* intel_guc_loader.c */
> +int intel_guc_select_fw(struct intel_guc *guc);
> +int intel_guc_init_hw(struct intel_guc *guc);
> +int intel_guc_suspend(struct drm_i915_private *dev_priv);
> +int intel_guc_resume(struct drm_i915_private *dev_priv);
> +u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
> +
> +/* i915_guc_submission.c */
> +int i915_guc_submission_init(struct drm_i915_private *dev_priv);
> +int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
> +void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
> +void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
> +struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32  
> size);
> +
> +/* intel_guc_log.c */
> +int intel_guc_log_create(struct intel_guc *guc);
> +void intel_guc_log_destroy(struct intel_guc *guc);
> +int i915_guc_log_control(struct drm_i915_private *dev_priv, u64  
> control_val);
> +void i915_guc_log_register(struct drm_i915_private *dev_priv);
> +void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
> +
>  #endif
> diff --git a/drivers/gpu/drm/i915/intel_uc.h  
> b/drivers/gpu/drm/i915/intel_uc.h
> index 2c11d11..30902f3 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -32,46 +32,6 @@
> struct drm_i915_gem_request;
> -/*
> - * This structure primarily describes the GEM object shared with the  
> GuC.
> - * The specs sometimes refer to this object as a "GuC context", but we  
> use
> - * the term "client" to avoid confusion with hardware contexts. This
> - * GEM object is held for the entire lifetime of our interaction with
> - * the GuC, being allocated before the GuC is loaded with its firmware.
> - * Because there's no way to update the address used by the GuC after
> - * initialisation, the shared object must stay pinned into the GGTT as
> - * long as the GuC is in use. We also keep the first page (only) mapped
> - * into kernel address space, as it includes shared data that must be
> - * updated on every request submission.
> - *
> - * The single GEM object described here is actually made up of several
> - * separate areas, as far as the GuC is concerned. The first page (kept
> - * kmap'd) includes the "process descriptor" which holds sequence data  
> for
> - * the doorbell, and one cacheline which actually *is* the doorbell; a
> - * write to this will "ring the doorbell" (i.e. send an interrupt to the
> - * GuC). The subsequent  pages of the client object constitute the work
> - * queue (a circular array of work items), again described in the  
> process
> - * descriptor. Work queue pages are mapped momentarily as required.
> - */
> -struct i915_guc_client {
> -	struct i915_vma *vma;
> -	void *vaddr;
> -	struct i915_gem_context *owner;
> -	struct intel_guc *guc;
> -
> -	uint32_t engines;		/* bitmap of (host) engine ids	*/
> -	uint32_t priority;
> -	u32 stage_id;
> -	uint32_t proc_desc_offset;
> -
> -	u16 doorbell_id;
> -	unsigned long doorbell_offset;
> -
> -	spinlock_t wq_lock;
> -	/* Per-engine counts of GuC submissions */
> -	uint64_t submissions[I915_NUM_ENGINES];
> -};
> -
>  enum intel_uc_fw_status {
>  	INTEL_UC_FIRMWARE_FAIL = -1,
>  	INTEL_UC_FIRMWARE_NONE = 0,
> @@ -138,62 +98,6 @@ struct intel_uc_fw {
>  	uint32_t ucode_offset;
>  };
> -struct intel_guc_log {
> -	uint32_t flags;
> -	struct i915_vma *vma;
> -	/* The runtime stuff gets created only when GuC logging gets enabled */
> -	struct {
> -		void *buf_addr;
> -		struct workqueue_struct *flush_wq;
> -		struct work_struct flush_work;
> -		struct rchan *relay_chan;
> -	} runtime;
> -	/* logging related stats */
> -	u32 capture_miss_count;
> -	u32 flush_interrupt_count;
> -	u32 prev_overflow_count[GUC_MAX_LOG_BUFFER];
> -	u32 total_overflow_count[GUC_MAX_LOG_BUFFER];
> -	u32 flush_count[GUC_MAX_LOG_BUFFER];
> -};
> -
> -struct intel_guc {
> -	struct intel_uc_fw fw;
> -	struct intel_guc_log log;
> -	struct intel_guc_ct ct;
> -
> -	/* Log snapshot if GuC errors during load */
> -	struct drm_i915_gem_object *load_err_log;
> -
> -	/* intel_guc_recv interrupt related state */
> -	bool interrupts_enabled;
> -
> -	struct i915_vma *ads_vma;
> -	struct i915_vma *stage_desc_pool;
> -	void *stage_desc_pool_vaddr;
> -	struct ida stage_ids;
> -
> -	struct i915_guc_client *execbuf_client;
> -
> -	DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
> -	uint32_t db_cacheline;		/* Cyclic counter mod pagesize	*/
> -
> -	/* GuC's FW specific registers used in MMIO send */
> -	struct {
> -		u32 base;
> -		unsigned int count;
> -		enum forcewake_domains fw_domains;
> -	} send_regs;
> -
> -	/* To serialize the intel_guc_send actions */
> -	struct mutex send_mutex;
> -
> -	/* GuC's FW specific send function */
> -	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
> -
> -	/* GuC's FW specific notify function */
> -	void (*notify)(struct intel_guc *guc);
> -};
> -
>  struct intel_huc {
>  	/* Generic uC firmware management */
>  	struct intel_uc_fw fw;
> @@ -209,35 +113,6 @@ struct intel_huc {
>  int intel_uc_init_hw(struct drm_i915_private *dev_priv);
>  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
> -/* intel_guc_loader.c */
> -int intel_guc_select_fw(struct intel_guc *guc);
> -int intel_guc_init_hw(struct intel_guc *guc);
> -int intel_guc_suspend(struct drm_i915_private *dev_priv);
> -int intel_guc_resume(struct drm_i915_private *dev_priv);
> -u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
> -
> -/* i915_guc_submission.c */
> -int i915_guc_submission_init(struct drm_i915_private *dev_priv);
> -int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
> -void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
> -void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
> -struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32  
> size);
> -
> -/* intel_guc_log.c */
> -int intel_guc_log_create(struct intel_guc *guc);
> -void intel_guc_log_destroy(struct intel_guc *guc);
> -int i915_guc_log_control(struct drm_i915_private *dev_priv, u64  
> control_val);
> -void i915_guc_log_register(struct drm_i915_private *dev_priv);
> -void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
> -
> -static inline u32 guc_ggtt_offset(struct i915_vma *vma)
> -{
> -	u32 offset = i915_ggtt_offset(vma);
> -	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
> -	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
> -	return offset;
> -}
> -
>  /* intel_huc.c */
>  void intel_huc_select_fw(struct intel_huc *huc);
>  void intel_huc_init_hw(struct intel_huc *huc);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 05/10] drm/i915: Reorganize HuC authentication
  2017-09-17 12:17 ` [PATCH 05/10] drm/i915: Reorganize HuC authentication Sagar Arun Kamble
@ 2017-09-17 18:41   ` Michal Wajdeczko
  0 siblings, 0 replies; 27+ messages in thread
From: Michal Wajdeczko @ 2017-09-17 18:41 UTC (permalink / raw)
  To: intel-gfx, Sagar Arun Kamble

On Sun, 17 Sep 2017 14:17:29 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

> Prepared intel_auth_huc to separate HuC specific functionality
> from GuC send action. Created new header intel_huc.h to group
> HuC specific declarations.
>
> v2: Changed argument preparation for AUTHENTICATE_HUC.
> s/intel_auth_huc/intel_huc_auth. Deferred creation of intel_huc.h
> to later patch.
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc.c | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_guc.h |  1 +
>  drivers/gpu/drm/i915/intel_huc.c | 21 ++++++---------------
>  drivers/gpu/drm/i915/intel_uc.c  |  2 +-
>  drivers/gpu/drm/i915/intel_uc.h  |  2 +-
>  5 files changed, 27 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc.c  
> b/drivers/gpu/drm/i915/intel_guc.c
> index 0767ec2..b250f39 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -145,3 +145,21 @@ int intel_guc_sample_forcewake(struct intel_guc  
> *guc)
> 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
>  }
> +
> +/**
> + * intel_guc_auth_huc() - authenticate ucode
> + * @guc: struct intel_guc*
> + * @offset: rsa offset w.r.t GGTT base of huc vma
> + *
> + * Triggers a HuC fw authentication request to the GuC via  
> intel_guc_send
> + * AUTHENTICATE_HUC interface.
> + */
> +int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset)
> +{
> +	u32 action[] = {
> +		INTEL_GUC_ACTION_AUTHENTICATE_HUC,
> +		rsa_offset
> +	};
> +
> +	return intel_guc_send(guc, action, ARRAY_SIZE(action));
> +}
> diff --git a/drivers/gpu/drm/i915/intel_guc.h  
> b/drivers/gpu/drm/i915/intel_guc.h
> index 919b6e1..e7b9a8b 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -147,6 +147,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma  
> *vma)
>  void intel_guc_init_send_regs(struct intel_guc *guc);
>  int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32  
> len);
>  int intel_guc_sample_forcewake(struct intel_guc *guc);
> +int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
> /* intel_guc_loader.c */
>  int intel_guc_select_fw(struct intel_guc *guc);
> diff --git a/drivers/gpu/drm/i915/intel_huc.c  
> b/drivers/gpu/drm/i915/intel_huc.c
> index b81c6af..6ecf344 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -226,19 +226,15 @@ void intel_huc_init_hw(struct intel_huc *huc)
>  }
> /**
> - * intel_guc_auth_huc() - authenticate ucode
> - * @dev_priv: the drm_i915_device
> - *
> - * Triggers a HuC fw authentication request to the GuC via  
> intel_guc_action_
> - * authenticate_huc interface.
> + * intel_huc_auth() - authenticate ucode

Add param description. With that fixed,

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>


>   */
> -void intel_guc_auth_huc(struct drm_i915_private *dev_priv)
> +void intel_huc_auth(struct intel_huc *huc)
>  {
> +	struct drm_i915_private *dev_priv = huc_to_i915(huc);
>  	struct intel_guc *guc = &dev_priv->guc;
> -	struct intel_huc *huc = &dev_priv->huc;
>  	struct i915_vma *vma;
> +	u32 offset;
>  	int ret;
> -	u32 data[2];
> 	if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
>  		return;
> @@ -251,11 +247,8 @@ void intel_guc_auth_huc(struct drm_i915_private  
> *dev_priv)
>  		return;
>  	}
> -	/* Specify auth action and where public signature is. */
> -	data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC;
> -	data[1] = guc_ggtt_offset(vma) + huc->fw.rsa_offset;
> -
> -	ret = intel_guc_send(guc, data, ARRAY_SIZE(data));
> +	offset = guc_ggtt_offset(vma) + huc->fw.rsa_offset;
> +	ret = intel_guc_auth_huc(guc, offset);
>  	if (ret) {
>  		DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret);
>  		goto out;
> @@ -267,7 +260,6 @@ void intel_guc_auth_huc(struct drm_i915_private  
> *dev_priv)
>  				HUC_FW_VERIFIED,
>  				HUC_FW_VERIFIED,
>  				50);
> -
>  	if (ret) {
>  		DRM_ERROR("HuC: Authentication failed %d\n", ret);
>  		goto out;
> @@ -276,4 +268,3 @@ void intel_guc_auth_huc(struct drm_i915_private  
> *dev_priv)
>  out:
>  	i915_vma_unpin(vma);
>  }
> -
> diff --git a/drivers/gpu/drm/i915/intel_uc.c  
> b/drivers/gpu/drm/i915/intel_uc.c
> index 4f8f0ff..9a0006a 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -352,7 +352,7 @@ int intel_uc_init_hw(struct drm_i915_private  
> *dev_priv)
>  	if (ret)
>  		goto err_log_capture;
> -	intel_guc_auth_huc(dev_priv);
> +	intel_huc_auth(&dev_priv->huc);
>  	if (i915.enable_guc_submission) {
>  		if (i915.guc_log_level >= 0)
>  			gen9_enable_guc_interrupts(dev_priv);
> diff --git a/drivers/gpu/drm/i915/intel_uc.h  
> b/drivers/gpu/drm/i915/intel_uc.h
> index 30902f3..aad0b1c 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -116,6 +116,6 @@ struct intel_huc {
>  /* intel_huc.c */
>  void intel_huc_select_fw(struct intel_huc *huc);
>  void intel_huc_init_hw(struct intel_huc *huc);
> -void intel_guc_auth_huc(struct drm_i915_private *dev_priv);
> +void intel_huc_auth(struct intel_huc *huc);
> #endif
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality
  2017-09-17 12:17 [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Sagar Arun Kamble
                   ` (8 preceding siblings ...)
  2017-09-17 12:17 ` [PATCH 10/10] drm/i915/guc: Remove i915_guc_log_unregister Sagar Arun Kamble
@ 2017-09-17 19:30 ` Michal Wajdeczko
  2017-09-18  6:23   ` Kamble, Sagar A
  2017-09-18  6:59 ` ✓ Fi.CI.BAT: success for series starting with [01/10] " Patchwork
                   ` (2 subsequent siblings)
  12 siblings, 1 reply; 27+ messages in thread
From: Michal Wajdeczko @ 2017-09-17 19:30 UTC (permalink / raw)
  To: intel-gfx, Sagar Arun Kamble

On Sun, 17 Sep 2017 14:17:25 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

> Create intel_guc.c and move guc communication init functionality from
> intel_uc.c. Prepared new initialization function intel_guc_init_early.
> Moved below functions to intel_guc.c.
> 1. intel_guc_send_nop
> 2. gen8_guc_raise_irq
> And below functions to intel_guc.h.
> 1. intel_guc_send
> 2. intel_guc_notify
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/Makefile              |  1 +
>  drivers/gpu/drm/i915/i915_guc_submission.c |  1 +
>  drivers/gpu/drm/i915/intel_guc.c           | 47  
> ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_guc.h           | 43  
> +++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_guc_ct.c        |  1 +
>  drivers/gpu/drm/i915/intel_guc_log.c       |  1 +
>  drivers/gpu/drm/i915/intel_huc.c           |  1 +
>  drivers/gpu/drm/i915/intel_uc.c            | 22 ++------------
>  drivers/gpu/drm/i915/intel_uc.h            | 11 -------
>  9 files changed, 97 insertions(+), 31 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_guc.c
>  create mode 100644 drivers/gpu/drm/i915/intel_guc.h
>
> diff --git a/drivers/gpu/drm/i915/Makefile  
> b/drivers/gpu/drm/i915/Makefile
> index 1cb8059..e13fc19 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -59,6 +59,7 @@ i915-y += i915_cmd_parser.o \
> # general-purpose microcontroller (GuC) support
>  i915-y += intel_uc.o \
> +	  intel_guc.o \
>  	  intel_guc_ct.o \
>  	  intel_guc_log.o \
>  	  intel_guc_loader.o \
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c  
> b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 3f9d227..f37cd47 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -24,6 +24,7 @@
>  #include <linux/circ_buf.h>
>  #include "i915_drv.h"
>  #include "intel_uc.h"
> +#include "intel_guc.h"

Please reorder your patches and make sure that no
explicit #include "intel_guc.h" are needed.

> #include <trace/events/dma_fence.h>
> diff --git a/drivers/gpu/drm/i915/intel_guc.c  
> b/drivers/gpu/drm/i915/intel_guc.c
> new file mode 100644
> index 0000000..0c62cc2
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -0,0 +1,47 @@
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person  
> obtaining a
> + * copy of this software and associated documentation files (the  
> "Software"),
> + * to deal in the Software without restriction, including without  
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,  
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the  
> next
> + * paragraph) shall be included in all copies or substantial portions  
> of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,  
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF  
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT  
> SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR  
> OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,  
> ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER  
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +
> +#include "i915_drv.h"
> +
> +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32  
> len)
> +{
> +	WARN(1, "Unexpected send: action=%#x\n", *action);
> +	return -ENODEV;
> +}
> +

hmm, you're moving send functions in patch 002/010

> +static void gen8_guc_raise_irq(struct intel_guc *guc)
> +{
> +	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> +
> +	I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
> +}
> +
> +void intel_guc_init_early(struct intel_guc *guc)
> +{
> +	intel_guc_ct_init_early(&guc->ct);
> +
> +	mutex_init(&guc->send_mutex);
> +	guc->send = intel_guc_send_nop;
> +	guc->notify = gen8_guc_raise_irq;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_guc.h  
> b/drivers/gpu/drm/i915/intel_guc.h
> new file mode 100644
> index 0000000..76b7113
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -0,0 +1,43 @@
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person  
> obtaining a
> + * copy of this software and associated documentation files (the  
> "Software"),
> + * to deal in the Software without restriction, including without  
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,  
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the  
> next
> + * paragraph) shall be included in all copies or substantial portions  
> of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,  
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF  
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT  
> SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR  
> OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,  
> ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER  
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +#ifndef _INTEL_GUC_H_
> +#define _INTEL_GUC_H_
> +

hmm, starting with empty header does not look good.

> +static inline int intel_guc_send(struct intel_guc *guc,
> +				 const u32 *action,
> +				 u32 len)
> +{
> +	return guc->send(guc, action, len);
> +}
> +

hmm, you're moving send functions in patch 002/010

> +static inline void intel_guc_notify(struct intel_guc *guc)
> +{
> +	guc->notify(guc);
> +}
> +
> +/* intel_guc.c */
> +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32  
> len);
> +void intel_guc_init_early(struct intel_guc *guc);
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c  
> b/drivers/gpu/drm/i915/intel_guc_ct.c
> index c4cbec1..87dd12d 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -23,6 +23,7 @@
> #include "i915_drv.h"
>  #include "intel_guc_ct.h"
> +#include "intel_guc.h"

please try to avoid this

> enum { CTB_SEND = 0, CTB_RECV = 1 };
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c  
> b/drivers/gpu/drm/i915/intel_guc_log.c
> index 16d3b87..5c7e0c2 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -24,6 +24,7 @@
>  #include <linux/debugfs.h>
>  #include <linux/relay.h>
>  #include "i915_drv.h"
> +#include "intel_guc.h"

please try to avoid this

> static void guc_log_capture_logs(struct intel_guc *guc);
> diff --git a/drivers/gpu/drm/i915/intel_huc.c  
> b/drivers/gpu/drm/i915/intel_huc.c
> index 6145fa0..b81c6af 100644
> --- a/drivers/gpu/drm/i915/intel_huc.c
> +++ b/drivers/gpu/drm/i915/intel_huc.c
> @@ -24,6 +24,7 @@
>  #include <linux/firmware.h>
>  #include "i915_drv.h"
>  #include "intel_uc.h"
> +#include "intel_guc.h"

please try to avoid this

> /**
>   * DOC: HuC Firmware
> diff --git a/drivers/gpu/drm/i915/intel_uc.c  
> b/drivers/gpu/drm/i915/intel_uc.c
> index 0178ba4..9ff5c97 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -24,6 +24,7 @@
> #include "i915_drv.h"
>  #include "intel_uc.h"
> +#include "intel_guc.h"

please try to avoid this

>  #include <linux/firmware.h>
> /* Cleans up uC firmware by releasing the firmware GEM obj.
> @@ -94,22 +95,9 @@ void intel_uc_sanitize_options(struct  
> drm_i915_private *dev_priv)
>  		i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
>  }
> -static void gen8_guc_raise_irq(struct intel_guc *guc)
> -{
> -	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> -
> -	I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
> -}
> -
>  void intel_uc_init_early(struct drm_i915_private *dev_priv)
>  {
> -	struct intel_guc *guc = &dev_priv->guc;
> -
> -	intel_guc_ct_init_early(&guc->ct);
> -
> -	mutex_init(&guc->send_mutex);
> -	guc->send = intel_guc_send_nop;
> -	guc->notify = gen8_guc_raise_irq;
> +	intel_guc_init_early(&dev_priv->guc);
>  }
> static void fetch_uc_fw(struct drm_i915_private *dev_priv,
> @@ -459,12 +447,6 @@ void intel_uc_fini_hw(struct drm_i915_private  
> *dev_priv)
>  	i915_ggtt_disable_guc(dev_priv);
>  }
> -int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32  
> len)
> -{
> -	WARN(1, "Unexpected send: action=%#x\n", *action);
> -	return -ENODEV;
> -}
> -
>  /*
>   * This function implements the MMIO based host to GuC interface.
>   */
> diff --git a/drivers/gpu/drm/i915/intel_uc.h  
> b/drivers/gpu/drm/i915/intel_uc.h
> index 7703c9a..27e30aa 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -209,19 +209,8 @@ struct intel_huc {
>  int intel_uc_init_hw(struct drm_i915_private *dev_priv);
>  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
>  int intel_guc_sample_forcewake(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);
> -static inline int intel_guc_send(struct intel_guc *guc, const u32  
> *action, u32 len)
> -{
> -	return guc->send(guc, action, len);
> -}
> -
> -static inline void intel_guc_notify(struct intel_guc *guc)
> -{
> -	guc->notify(guc);
> -}
> -
>  /* intel_guc_loader.c */
>  int intel_guc_select_fw(struct intel_guc *guc);
>  int intel_guc_init_hw(struct intel_guc *guc);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 06/10] drm/i915/huc: Move HuC specific declarations from intel_uc.h to intel_huc.h
  2017-09-17 12:17 ` [PATCH 06/10] drm/i915/huc: Move HuC specific declarations from intel_uc.h to intel_huc.h Sagar Arun Kamble
@ 2017-09-17 20:13   ` Michal Wajdeczko
  2017-09-18  6:24     ` Kamble, Sagar A
  0 siblings, 1 reply; 27+ messages in thread
From: Michal Wajdeczko @ 2017-09-17 20:13 UTC (permalink / raw)
  To: intel-gfx, Sagar Arun Kamble

On Sun, 17 Sep 2017 14:17:30 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

Missing commit message

> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_huc.h | 39  
> +++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_uc.h  | 12 ------------
>  3 files changed, 40 insertions(+), 12 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_huc.h
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h  
> b/drivers/gpu/drm/i915/i915_drv.h
> index b0f38bb..a3b7435 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -60,6 +60,7 @@
>  #include "intel_dpll_mgr.h"
>  #include "intel_uc.h"
>  #include "intel_guc.h"
> +#include "intel_huc.h"

Huc/Guc details shall be wrapped into "intel_uc.h"

>  #include "intel_lrc.h"
>  #include "intel_ringbuffer.h"
> diff --git a/drivers/gpu/drm/i915/intel_huc.h  
> b/drivers/gpu/drm/i915/intel_huc.h
> new file mode 100644
> index 0000000..4604a40
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_huc.h
> @@ -0,0 +1,39 @@
> +/*
> + * Copyright © 2017 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person  
> obtaining a
> + * copy of this software and associated documentation files (the  
> "Software"),
> + * to deal in the Software without restriction, including without  
> limitation
> + * the rights to use, copy, modify, merge, publish, distribute,  
> sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the  
> next
> + * paragraph) shall be included in all copies or substantial portions  
> of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,  
> EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF  
> MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT  
> SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR  
> OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,  
> ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER  
> DEALINGS
> + * IN THE SOFTWARE.
> + *
> + */
> +#ifndef _INTEL_HUC_H_
> +#define _INTEL_HUC_H_
> +
> +struct intel_huc {
> +	/* Generic uC firmware management */
> +	struct intel_uc_fw fw;
> +
> +	/* HuC-specific additions */
> +};
> +
> +/* intel_huc.c */
> +void intel_huc_select_fw(struct intel_huc *huc);
> +void intel_huc_init_hw(struct intel_huc *huc);
> +void intel_huc_auth(struct intel_huc *huc);
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/intel_uc.h  
> b/drivers/gpu/drm/i915/intel_uc.h
> index aad0b1c..0d346ef 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -98,13 +98,6 @@ struct intel_uc_fw {
>  	uint32_t ucode_offset;
>  };
> -struct intel_huc {
> -	/* Generic uC firmware management */
> -	struct intel_uc_fw fw;
> -
> -	/* HuC-specific additions */
> -};
> -
>  /* intel_uc.c */
>  void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
>  void intel_uc_init_early(struct drm_i915_private *dev_priv);
> @@ -113,9 +106,4 @@ struct intel_huc {
>  int intel_uc_init_hw(struct drm_i915_private *dev_priv);
>  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
> -/* intel_huc.c */
> -void intel_huc_select_fw(struct intel_huc *huc);
> -void intel_huc_init_hw(struct intel_huc *huc);
> -void intel_huc_auth(struct intel_huc *huc);
> -
>  #endif
 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 04/10] drm/i915/guc: Move GuC specific declarations from intel_uc.h to intel_guc.h
  2017-09-17 18:24   ` Michal Wajdeczko
@ 2017-09-18  6:10     ` Kamble, Sagar A
  0 siblings, 0 replies; 27+ messages in thread
From: Kamble, Sagar A @ 2017-09-18  6:10 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx



On 9/17/2017 11:54 PM, Michal Wajdeczko wrote:
> On Sun, 17 Sep 2017 14:17:28 +0200, Sagar Arun Kamble 
> <sagar.a.kamble@intel.com> wrote:
>
> Maybe to make this refactoring series much clearer you should start 
> with this patch ?
> and btw, please don't forget about adding commit description.
Sorry. Will update the patch and reorder. Tend to think that change 
obvious from commit subject does
not need the commit message. Will add commit message.
>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h  |   1 +
>>  drivers/gpu/drm/i915/intel_guc.h | 126 
>> +++++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_uc.h  | 125 
>> --------------------------------------
>>  3 files changed, 127 insertions(+), 125 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 5aecbf7..b0f38bb 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -59,6 +59,7 @@
>>  #include "intel_bios.h"
>>  #include "intel_dpll_mgr.h"
>>  #include "intel_uc.h"
>> +#include "intel_guc.h"
>
> Hmm, the goal of previous (unfinished) refactoring was to
> wrap all details about Guc/Huc into generic single "uc"
> component. Thus any "guc"/"huc" specific headers shall
> be included by master "intel_uc.h". If this is hard to
> make, then maybe we should postpone changes in .h and
> current limit refactoring only to .c files.
>
> Michal
Sure. I will limit the factoring only to .c files.
>
>>  #include "intel_lrc.h"
>>  #include "intel_ringbuffer.h"
>> diff --git a/drivers/gpu/drm/i915/intel_guc.h 
>> b/drivers/gpu/drm/i915/intel_guc.h
>> index 06f2d27..919b6e1 100644
>> --- a/drivers/gpu/drm/i915/intel_guc.h
>> +++ b/drivers/gpu/drm/i915/intel_guc.h
>> @@ -24,6 +24,102 @@
>>  #ifndef _INTEL_GUC_H_
>>  #define _INTEL_GUC_H_
>> +/*
>> + * This structure primarily describes the GEM object shared with the 
>> GuC.
>> + * The specs sometimes refer to this object as a "GuC context", but 
>> we use
>> + * the term "client" to avoid confusion with hardware contexts. This
>> + * GEM object is held for the entire lifetime of our interaction with
>> + * the GuC, being allocated before the GuC is loaded with its firmware.
>> + * Because there's no way to update the address used by the GuC after
>> + * initialisation, the shared object must stay pinned into the GGTT as
>> + * long as the GuC is in use. We also keep the first page (only) mapped
>> + * into kernel address space, as it includes shared data that must be
>> + * updated on every request submission.
>> + *
>> + * The single GEM object described here is actually made up of several
>> + * separate areas, as far as the GuC is concerned. The first page (kept
>> + * kmap'd) includes the "process descriptor" which holds sequence 
>> data for
>> + * the doorbell, and one cacheline which actually *is* the doorbell; a
>> + * write to this will "ring the doorbell" (i.e. send an interrupt to 
>> the
>> + * GuC). The subsequent  pages of the client object constitute the work
>> + * queue (a circular array of work items), again described in the 
>> process
>> + * descriptor. Work queue pages are mapped momentarily as required.
>> + */
>> +struct i915_guc_client {
>> +    struct i915_vma *vma;
>> +    void *vaddr;
>> +    struct i915_gem_context *owner;
>> +    struct intel_guc *guc;
>> +
>> +    uint32_t engines;        /* bitmap of (host) engine ids */
>> +    uint32_t priority;
>> +    u32 stage_id;
>> +    uint32_t proc_desc_offset;
>> +
>> +    u16 doorbell_id;
>> +    unsigned long doorbell_offset;
>> +
>> +    spinlock_t wq_lock;
>> +    /* Per-engine counts of GuC submissions */
>> +    uint64_t submissions[I915_NUM_ENGINES];
>> +};
>> +
>> +struct intel_guc_log {
>> +    uint32_t flags;
>> +    struct i915_vma *vma;
>> +    /* The runtime stuff gets created only when GuC logging gets 
>> enabled */
>> +    struct {
>> +        void *buf_addr;
>> +        struct workqueue_struct *flush_wq;
>> +        struct work_struct flush_work;
>> +        struct rchan *relay_chan;
>> +    } runtime;
>> +    /* logging related stats */
>> +    u32 capture_miss_count;
>> +    u32 flush_interrupt_count;
>> +    u32 prev_overflow_count[GUC_MAX_LOG_BUFFER];
>> +    u32 total_overflow_count[GUC_MAX_LOG_BUFFER];
>> +    u32 flush_count[GUC_MAX_LOG_BUFFER];
>> +};
>> +
>> +struct intel_guc {
>> +    struct intel_uc_fw fw;
>> +    struct intel_guc_log log;
>> +    struct intel_guc_ct ct;
>> +
>> +    /* Log snapshot if GuC errors during load */
>> +    struct drm_i915_gem_object *load_err_log;
>> +
>> +    /* intel_guc_recv interrupt related state */
>> +    bool interrupts_enabled;
>> +
>> +    struct i915_vma *ads_vma;
>> +    struct i915_vma *stage_desc_pool;
>> +    void *stage_desc_pool_vaddr;
>> +    struct ida stage_ids;
>> +
>> +    struct i915_guc_client *execbuf_client;
>> +
>> +    DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
>> +    uint32_t db_cacheline;        /* Cyclic counter mod pagesize    */
>> +
>> +    /* GuC's FW specific registers used in MMIO send */
>> +    struct {
>> +        u32 base;
>> +        unsigned int count;
>> +        enum forcewake_domains fw_domains;
>> +    } send_regs;
>> +
>> +    /* To serialize the intel_guc_send actions */
>> +    struct mutex send_mutex;
>> +
>> +    /* GuC's FW specific send function */
>> +    int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
>> +
>> +    /* GuC's FW specific notify function */
>> +    void (*notify)(struct intel_guc *guc);
>> +};
>> +
>>  static inline int intel_guc_send(struct intel_guc *guc,
>>                   const u32 *action,
>>                   u32 len)
>> @@ -36,6 +132,15 @@ static inline void intel_guc_notify(struct 
>> intel_guc *guc)
>>      guc->notify(guc);
>>  }
>> +static inline u32 guc_ggtt_offset(struct i915_vma *vma)
>> +{
>> +    u32 offset = i915_ggtt_offset(vma);
>> +
>> +    GEM_BUG_ON(offset < GUC_WOPCM_TOP);
>> +    GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, 
>> GUC_GGTT_TOP));
>> +    return offset;
>> +}
>> +
>>  /* intel_guc.c */
>>  int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 
>> len);
>>  void intel_guc_init_early(struct intel_guc *guc);
>> @@ -43,4 +148,25 @@ static inline void intel_guc_notify(struct 
>> intel_guc *guc)
>>  int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, 
>> u32 len);
>>  int intel_guc_sample_forcewake(struct intel_guc *guc);
>> +/* intel_guc_loader.c */
>> +int intel_guc_select_fw(struct intel_guc *guc);
>> +int intel_guc_init_hw(struct intel_guc *guc);
>> +int intel_guc_suspend(struct drm_i915_private *dev_priv);
>> +int intel_guc_resume(struct drm_i915_private *dev_priv);
>> +u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
>> +
>> +/* i915_guc_submission.c */
>> +int i915_guc_submission_init(struct drm_i915_private *dev_priv);
>> +int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
>> +void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
>> +void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
>> +struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 
>> size);
>> +
>> +/* intel_guc_log.c */
>> +int intel_guc_log_create(struct intel_guc *guc);
>> +void intel_guc_log_destroy(struct intel_guc *guc);
>> +int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 
>> control_val);
>> +void i915_guc_log_register(struct drm_i915_private *dev_priv);
>> +void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
>> +
>>  #endif
>> diff --git a/drivers/gpu/drm/i915/intel_uc.h 
>> b/drivers/gpu/drm/i915/intel_uc.h
>> index 2c11d11..30902f3 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.h
>> +++ b/drivers/gpu/drm/i915/intel_uc.h
>> @@ -32,46 +32,6 @@
>> struct drm_i915_gem_request;
>> -/*
>> - * This structure primarily describes the GEM object shared with the 
>> GuC.
>> - * The specs sometimes refer to this object as a "GuC context", but 
>> we use
>> - * the term "client" to avoid confusion with hardware contexts. This
>> - * GEM object is held for the entire lifetime of our interaction with
>> - * the GuC, being allocated before the GuC is loaded with its firmware.
>> - * Because there's no way to update the address used by the GuC after
>> - * initialisation, the shared object must stay pinned into the GGTT as
>> - * long as the GuC is in use. We also keep the first page (only) mapped
>> - * into kernel address space, as it includes shared data that must be
>> - * updated on every request submission.
>> - *
>> - * The single GEM object described here is actually made up of several
>> - * separate areas, as far as the GuC is concerned. The first page (kept
>> - * kmap'd) includes the "process descriptor" which holds sequence 
>> data for
>> - * the doorbell, and one cacheline which actually *is* the doorbell; a
>> - * write to this will "ring the doorbell" (i.e. send an interrupt to 
>> the
>> - * GuC). The subsequent  pages of the client object constitute the work
>> - * queue (a circular array of work items), again described in the 
>> process
>> - * descriptor. Work queue pages are mapped momentarily as required.
>> - */
>> -struct i915_guc_client {
>> -    struct i915_vma *vma;
>> -    void *vaddr;
>> -    struct i915_gem_context *owner;
>> -    struct intel_guc *guc;
>> -
>> -    uint32_t engines;        /* bitmap of (host) engine ids */
>> -    uint32_t priority;
>> -    u32 stage_id;
>> -    uint32_t proc_desc_offset;
>> -
>> -    u16 doorbell_id;
>> -    unsigned long doorbell_offset;
>> -
>> -    spinlock_t wq_lock;
>> -    /* Per-engine counts of GuC submissions */
>> -    uint64_t submissions[I915_NUM_ENGINES];
>> -};
>> -
>>  enum intel_uc_fw_status {
>>      INTEL_UC_FIRMWARE_FAIL = -1,
>>      INTEL_UC_FIRMWARE_NONE = 0,
>> @@ -138,62 +98,6 @@ struct intel_uc_fw {
>>      uint32_t ucode_offset;
>>  };
>> -struct intel_guc_log {
>> -    uint32_t flags;
>> -    struct i915_vma *vma;
>> -    /* The runtime stuff gets created only when GuC logging gets 
>> enabled */
>> -    struct {
>> -        void *buf_addr;
>> -        struct workqueue_struct *flush_wq;
>> -        struct work_struct flush_work;
>> -        struct rchan *relay_chan;
>> -    } runtime;
>> -    /* logging related stats */
>> -    u32 capture_miss_count;
>> -    u32 flush_interrupt_count;
>> -    u32 prev_overflow_count[GUC_MAX_LOG_BUFFER];
>> -    u32 total_overflow_count[GUC_MAX_LOG_BUFFER];
>> -    u32 flush_count[GUC_MAX_LOG_BUFFER];
>> -};
>> -
>> -struct intel_guc {
>> -    struct intel_uc_fw fw;
>> -    struct intel_guc_log log;
>> -    struct intel_guc_ct ct;
>> -
>> -    /* Log snapshot if GuC errors during load */
>> -    struct drm_i915_gem_object *load_err_log;
>> -
>> -    /* intel_guc_recv interrupt related state */
>> -    bool interrupts_enabled;
>> -
>> -    struct i915_vma *ads_vma;
>> -    struct i915_vma *stage_desc_pool;
>> -    void *stage_desc_pool_vaddr;
>> -    struct ida stage_ids;
>> -
>> -    struct i915_guc_client *execbuf_client;
>> -
>> -    DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
>> -    uint32_t db_cacheline;        /* Cyclic counter mod pagesize    */
>> -
>> -    /* GuC's FW specific registers used in MMIO send */
>> -    struct {
>> -        u32 base;
>> -        unsigned int count;
>> -        enum forcewake_domains fw_domains;
>> -    } send_regs;
>> -
>> -    /* To serialize the intel_guc_send actions */
>> -    struct mutex send_mutex;
>> -
>> -    /* GuC's FW specific send function */
>> -    int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
>> -
>> -    /* GuC's FW specific notify function */
>> -    void (*notify)(struct intel_guc *guc);
>> -};
>> -
>>  struct intel_huc {
>>      /* Generic uC firmware management */
>>      struct intel_uc_fw fw;
>> @@ -209,35 +113,6 @@ struct intel_huc {
>>  int intel_uc_init_hw(struct drm_i915_private *dev_priv);
>>  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
>> -/* intel_guc_loader.c */
>> -int intel_guc_select_fw(struct intel_guc *guc);
>> -int intel_guc_init_hw(struct intel_guc *guc);
>> -int intel_guc_suspend(struct drm_i915_private *dev_priv);
>> -int intel_guc_resume(struct drm_i915_private *dev_priv);
>> -u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
>> -
>> -/* i915_guc_submission.c */
>> -int i915_guc_submission_init(struct drm_i915_private *dev_priv);
>> -int i915_guc_submission_enable(struct drm_i915_private *dev_priv);
>> -void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
>> -void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
>> -struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 
>> size);
>> -
>> -/* intel_guc_log.c */
>> -int intel_guc_log_create(struct intel_guc *guc);
>> -void intel_guc_log_destroy(struct intel_guc *guc);
>> -int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 
>> control_val);
>> -void i915_guc_log_register(struct drm_i915_private *dev_priv);
>> -void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
>> -
>> -static inline u32 guc_ggtt_offset(struct i915_vma *vma)
>> -{
>> -    u32 offset = i915_ggtt_offset(vma);
>> -    GEM_BUG_ON(offset < GUC_WOPCM_TOP);
>> -    GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, 
>> GUC_GGTT_TOP));
>> -    return offset;
>> -}
>> -
>>  /* intel_huc.c */
>>  void intel_huc_select_fw(struct intel_huc *huc);
>>  void intel_huc_init_hw(struct intel_huc *huc);

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

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

* Re: [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality
  2017-09-17 19:30 ` [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Michal Wajdeczko
@ 2017-09-18  6:23   ` Kamble, Sagar A
  0 siblings, 0 replies; 27+ messages in thread
From: Kamble, Sagar A @ 2017-09-18  6:23 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx



On 9/18/2017 1:00 AM, Michal Wajdeczko wrote:
> On Sun, 17 Sep 2017 14:17:25 +0200, Sagar Arun Kamble 
> <sagar.a.kamble@intel.com> wrote:
>
>> Create intel_guc.c and move guc communication init functionality from
>> intel_uc.c. Prepared new initialization function intel_guc_init_early.
>> Moved below functions to intel_guc.c.
>> 1. intel_guc_send_nop
>> 2. gen8_guc_raise_irq
>> And below functions to intel_guc.h.
>> 1. intel_guc_send
>> 2. intel_guc_notify
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>>  drivers/gpu/drm/i915/Makefile              |  1 +
>>  drivers/gpu/drm/i915/i915_guc_submission.c |  1 +
>>  drivers/gpu/drm/i915/intel_guc.c           | 47 
>> ++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_guc.h           | 43 
>> +++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_guc_ct.c        |  1 +
>>  drivers/gpu/drm/i915/intel_guc_log.c       |  1 +
>>  drivers/gpu/drm/i915/intel_huc.c           |  1 +
>>  drivers/gpu/drm/i915/intel_uc.c            | 22 ++------------
>>  drivers/gpu/drm/i915/intel_uc.h            | 11 -------
>>  9 files changed, 97 insertions(+), 31 deletions(-)
>>  create mode 100644 drivers/gpu/drm/i915/intel_guc.c
>>  create mode 100644 drivers/gpu/drm/i915/intel_guc.h
>>
>> diff --git a/drivers/gpu/drm/i915/Makefile 
>> b/drivers/gpu/drm/i915/Makefile
>> index 1cb8059..e13fc19 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -59,6 +59,7 @@ i915-y += i915_cmd_parser.o \
>> # general-purpose microcontroller (GuC) support
>>  i915-y += intel_uc.o \
>> +      intel_guc.o \
>>        intel_guc_ct.o \
>>        intel_guc_log.o \
>>        intel_guc_loader.o \
>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c 
>> b/drivers/gpu/drm/i915/i915_guc_submission.c
>> index 3f9d227..f37cd47 100644
>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>> @@ -24,6 +24,7 @@
>>  #include <linux/circ_buf.h>
>>  #include "i915_drv.h"
>>  #include "intel_uc.h"
>> +#include "intel_guc.h"
>
> Please reorder your patches and make sure that no
> explicit #include "intel_guc.h" are needed.
Sure. Will update.
>
>> #include <trace/events/dma_fence.h>
>> diff --git a/drivers/gpu/drm/i915/intel_guc.c 
>> b/drivers/gpu/drm/i915/intel_guc.c
>> new file mode 100644
>> index 0000000..0c62cc2
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_guc.c
>> @@ -0,0 +1,47 @@
>> +/*
>> + * Copyright © 2017 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person 
>> obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without 
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, 
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom 
>> the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including 
>> the next
>> + * paragraph) shall be included in all copies or substantial 
>> portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO 
>> EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR 
>> OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + */
>> +
>> +#include "i915_drv.h"
>> +
>> +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 
>> len)
>> +{
>> +    WARN(1, "Unexpected send: action=%#x\n", *action);
>> +    return -ENODEV;
>> +}
>> +
>
> hmm, you're moving send functions in patch 002/010
I think I should squash these patches together as I don't see clear 
separation.
Will prepare new patch to take out any delta refactoring done.
>
>> +static void gen8_guc_raise_irq(struct intel_guc *guc)
>> +{
>> +    struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> +
>> +    I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
>> +}
>> +
>> +void intel_guc_init_early(struct intel_guc *guc)
>> +{
>> +    intel_guc_ct_init_early(&guc->ct);
>> +
>> +    mutex_init(&guc->send_mutex);
>> +    guc->send = intel_guc_send_nop;
>> +    guc->notify = gen8_guc_raise_irq;
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_guc.h 
>> b/drivers/gpu/drm/i915/intel_guc.h
>> new file mode 100644
>> index 0000000..76b7113
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_guc.h
>> @@ -0,0 +1,43 @@
>> +/*
>> + * Copyright © 2017 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person 
>> obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without 
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, 
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom 
>> the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including 
>> the next
>> + * paragraph) shall be included in all copies or substantial 
>> portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO 
>> EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR 
>> OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + */
>> +#ifndef _INTEL_GUC_H_
>> +#define _INTEL_GUC_H_
>> +
>
> hmm, starting with empty header does not look good.
>
>> +static inline int intel_guc_send(struct intel_guc *guc,
>> +                 const u32 *action,
>> +                 u32 len)
>> +{
>> +    return guc->send(guc, action, len);
>> +}
>> +
>
> hmm, you're moving send functions in patch 002/010
Yes. Will update.
>
>> +static inline void intel_guc_notify(struct intel_guc *guc)
>> +{
>> +    guc->notify(guc);
>> +}
>> +
>> +/* intel_guc.c */
>> +int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 
>> len);
>> +void intel_guc_init_early(struct intel_guc *guc);
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c 
>> b/drivers/gpu/drm/i915/intel_guc_ct.c
>> index c4cbec1..87dd12d 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
>> @@ -23,6 +23,7 @@
>> #include "i915_drv.h"
>>  #include "intel_guc_ct.h"
>> +#include "intel_guc.h"
>
> please try to avoid this
Yes. Will update at all places.
>
>> enum { CTB_SEND = 0, CTB_RECV = 1 };
>> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c 
>> b/drivers/gpu/drm/i915/intel_guc_log.c
>> index 16d3b87..5c7e0c2 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_log.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
>> @@ -24,6 +24,7 @@
>>  #include <linux/debugfs.h>
>>  #include <linux/relay.h>
>>  #include "i915_drv.h"
>> +#include "intel_guc.h"
>
> please try to avoid this
>
>> static void guc_log_capture_logs(struct intel_guc *guc);
>> diff --git a/drivers/gpu/drm/i915/intel_huc.c 
>> b/drivers/gpu/drm/i915/intel_huc.c
>> index 6145fa0..b81c6af 100644
>> --- a/drivers/gpu/drm/i915/intel_huc.c
>> +++ b/drivers/gpu/drm/i915/intel_huc.c
>> @@ -24,6 +24,7 @@
>>  #include <linux/firmware.h>
>>  #include "i915_drv.h"
>>  #include "intel_uc.h"
>> +#include "intel_guc.h"
>
> please try to avoid this
>
>> /**
>>   * DOC: HuC Firmware
>> diff --git a/drivers/gpu/drm/i915/intel_uc.c 
>> b/drivers/gpu/drm/i915/intel_uc.c
>> index 0178ba4..9ff5c97 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.c
>> +++ b/drivers/gpu/drm/i915/intel_uc.c
>> @@ -24,6 +24,7 @@
>> #include "i915_drv.h"
>>  #include "intel_uc.h"
>> +#include "intel_guc.h"
>
> please try to avoid this
>
>>  #include <linux/firmware.h>
>> /* Cleans up uC firmware by releasing the firmware GEM obj.
>> @@ -94,22 +95,9 @@ void intel_uc_sanitize_options(struct 
>> drm_i915_private *dev_priv)
>>          i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
>>  }
>> -static void gen8_guc_raise_irq(struct intel_guc *guc)
>> -{
>> -    struct drm_i915_private *dev_priv = guc_to_i915(guc);
>> -
>> -    I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
>> -}
>> -
>>  void intel_uc_init_early(struct drm_i915_private *dev_priv)
>>  {
>> -    struct intel_guc *guc = &dev_priv->guc;
>> -
>> -    intel_guc_ct_init_early(&guc->ct);
>> -
>> -    mutex_init(&guc->send_mutex);
>> -    guc->send = intel_guc_send_nop;
>> -    guc->notify = gen8_guc_raise_irq;
>> +    intel_guc_init_early(&dev_priv->guc);
>>  }
>> static void fetch_uc_fw(struct drm_i915_private *dev_priv,
>> @@ -459,12 +447,6 @@ void intel_uc_fini_hw(struct drm_i915_private 
>> *dev_priv)
>>      i915_ggtt_disable_guc(dev_priv);
>>  }
>> -int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 
>> len)
>> -{
>> -    WARN(1, "Unexpected send: action=%#x\n", *action);
>> -    return -ENODEV;
>> -}
>> -
>>  /*
>>   * This function implements the MMIO based host to GuC interface.
>>   */
>> diff --git a/drivers/gpu/drm/i915/intel_uc.h 
>> b/drivers/gpu/drm/i915/intel_uc.h
>> index 7703c9a..27e30aa 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.h
>> +++ b/drivers/gpu/drm/i915/intel_uc.h
>> @@ -209,19 +209,8 @@ struct intel_huc {
>>  int intel_uc_init_hw(struct drm_i915_private *dev_priv);
>>  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
>>  int intel_guc_sample_forcewake(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);
>> -static inline int intel_guc_send(struct intel_guc *guc, const u32 
>> *action, u32 len)
>> -{
>> -    return guc->send(guc, action, len);
>> -}
>> -
>> -static inline void intel_guc_notify(struct intel_guc *guc)
>> -{
>> -    guc->notify(guc);
>> -}
>> -
>>  /* intel_guc_loader.c */
>>  int intel_guc_select_fw(struct intel_guc *guc);
>>  int intel_guc_init_hw(struct intel_guc *guc);

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

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

* Re: [PATCH 06/10] drm/i915/huc: Move HuC specific declarations from intel_uc.h to intel_huc.h
  2017-09-17 20:13   ` Michal Wajdeczko
@ 2017-09-18  6:24     ` Kamble, Sagar A
  0 siblings, 0 replies; 27+ messages in thread
From: Kamble, Sagar A @ 2017-09-18  6:24 UTC (permalink / raw)
  To: Michal Wajdeczko, intel-gfx



On 9/18/2017 1:43 AM, Michal Wajdeczko wrote:
> On Sun, 17 Sep 2017 14:17:30 +0200, Sagar Arun Kamble 
> <sagar.a.kamble@intel.com> wrote:
>
> Missing commit message
Sorry. Will update.
>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>>  drivers/gpu/drm/i915/intel_huc.h | 39 
>> +++++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_uc.h  | 12 ------------
>>  3 files changed, 40 insertions(+), 12 deletions(-)
>>  create mode 100644 drivers/gpu/drm/i915/intel_huc.h
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h 
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index b0f38bb..a3b7435 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -60,6 +60,7 @@
>>  #include "intel_dpll_mgr.h"
>>  #include "intel_uc.h"
>>  #include "intel_guc.h"
>> +#include "intel_huc.h"
>
> Huc/Guc details shall be wrapped into "intel_uc.h"
Yes. Will update.
>
>>  #include "intel_lrc.h"
>>  #include "intel_ringbuffer.h"
>> diff --git a/drivers/gpu/drm/i915/intel_huc.h 
>> b/drivers/gpu/drm/i915/intel_huc.h
>> new file mode 100644
>> index 0000000..4604a40
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_huc.h
>> @@ -0,0 +1,39 @@
>> +/*
>> + * Copyright © 2017 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person 
>> obtaining a
>> + * copy of this software and associated documentation files (the 
>> "Software"),
>> + * to deal in the Software without restriction, including without 
>> limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, 
>> sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom 
>> the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including 
>> the next
>> + * paragraph) shall be included in all copies or substantial 
>> portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
>> EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
>> MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO 
>> EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES 
>> OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, 
>> ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR 
>> OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + */
>> +#ifndef _INTEL_HUC_H_
>> +#define _INTEL_HUC_H_
>> +
>> +struct intel_huc {
>> +    /* Generic uC firmware management */
>> +    struct intel_uc_fw fw;
>> +
>> +    /* HuC-specific additions */
>> +};
>> +
>> +/* intel_huc.c */
>> +void intel_huc_select_fw(struct intel_huc *huc);
>> +void intel_huc_init_hw(struct intel_huc *huc);
>> +void intel_huc_auth(struct intel_huc *huc);
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/i915/intel_uc.h 
>> b/drivers/gpu/drm/i915/intel_uc.h
>> index aad0b1c..0d346ef 100644
>> --- a/drivers/gpu/drm/i915/intel_uc.h
>> +++ b/drivers/gpu/drm/i915/intel_uc.h
>> @@ -98,13 +98,6 @@ struct intel_uc_fw {
>>      uint32_t ucode_offset;
>>  };
>> -struct intel_huc {
>> -    /* Generic uC firmware management */
>> -    struct intel_uc_fw fw;
>> -
>> -    /* HuC-specific additions */
>> -};
>> -
>>  /* intel_uc.c */
>>  void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
>>  void intel_uc_init_early(struct drm_i915_private *dev_priv);
>> @@ -113,9 +106,4 @@ struct intel_huc {
>>  int intel_uc_init_hw(struct drm_i915_private *dev_priv);
>>  void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
>> -/* intel_huc.c */
>> -void intel_huc_select_fw(struct intel_huc *huc);
>> -void intel_huc_init_hw(struct intel_huc *huc);
>> -void intel_huc_auth(struct intel_huc *huc);
>> -
>>  #endif
>

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

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

* ✓ Fi.CI.BAT: success for series starting with [01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality
  2017-09-17 12:17 [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Sagar Arun Kamble
                   ` (9 preceding siblings ...)
  2017-09-17 19:30 ` [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Michal Wajdeczko
@ 2017-09-18  6:59 ` Patchwork
  2017-09-18  7:53 ` ✗ Fi.CI.IGT: warning " Patchwork
  2017-09-22 11:00 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality (rev2) Patchwork
  12 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2017-09-18  6:59 UTC (permalink / raw)
  To: Kamble, Sagar A; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality
URL   : https://patchwork.freedesktop.org/series/30486/
State : success

== Summary ==

Series 30486v1 series starting with [01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality
https://patchwork.freedesktop.org/api/1.0/series/30486/revisions/1/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-legacy:
                fail       -> PASS       (fi-snb-2600) fdo#100215
Test pm_rpm:
        Subgroup basic-rte:
                dmesg-warn -> PASS       (fi-cfl-s) fdo#102294

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

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:441s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:455s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:376s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:536s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:271s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:513s
fi-byt-j1900     total:289  pass:255  dwarn:0   dfail:0   fail:0   skip:34  time:503s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:497s
fi-cfl-s         total:289  pass:223  dwarn:34  dfail:0   fail:0   skip:32  time:548s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:420s
fi-glk-2a        total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:594s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:427s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:406s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:439s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:478s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:468s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:490s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:578s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:585s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:553s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:454s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:521s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:500s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:459s
fi-skl-x1585l    total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:477s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:569s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:421s

7fb202bddcf81e23028f9aadcf82b732699bd527 drm-tip: 2017y-09m-16d-10h-04m-01s UTC integration manifest
344642194d40 drm/i915/guc: Move guc_sample_forcewake to intel_guc.c
b9bce6a2ba7e drm/i915/guc: Move guc_send_* functions to intel_guc.c
cc6c7ba7e98d drm/i915/guc: Create intel_guc.c for defining GuC specific functionality

== Logs ==

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

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

* ✗ Fi.CI.IGT: warning for series starting with [01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality
  2017-09-17 12:17 [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Sagar Arun Kamble
                   ` (10 preceding siblings ...)
  2017-09-18  6:59 ` ✓ Fi.CI.BAT: success for series starting with [01/10] " Patchwork
@ 2017-09-18  7:53 ` Patchwork
  2017-09-22 11:00 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality (rev2) Patchwork
  12 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2017-09-18  7:53 UTC (permalink / raw)
  To: Kamble, Sagar A; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality
URL   : https://patchwork.freedesktop.org/series/30486/
State : warning

== Summary ==

Test kms_frontbuffer_tracking:
        Subgroup fbc-2p-pri-indfb-multidraw:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-scndscrn-spr-indfb-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-spr-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-offscren-pri-shrfb-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-primscrn-pri-shrfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-primscrn-pri-shrfb-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-scndscrn-pri-shrfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-pri-indfb-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-shrfb-plflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-primscrn-pri-indfb-draw-blt:
                skip       -> PASS       (shard-hsw)
        Subgroup psr-1p-rte:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-cur-indfb-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-spr-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-cur-indfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-offscren-pri-shrfb-draw-mmap-gtt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-primscrn-spr-indfb-fullscreen:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-cur-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-indfb-fliptrack:
                skip       -> PASS       (shard-hsw)
        Subgroup psr-rgb101010-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-offscren-pri-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-scndscrn-pri-shrfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-scndscrn-pri-shrfb-draw-mmap-gtt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-primscrn-spr-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-scndscrn-spr-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-scndscrn-spr-indfb-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-primscrn-spr-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-primscrn-pri-shrfb-draw-mmap-cpu:
                skip       -> PASS       (shard-hsw)
        Subgroup psr-2p-primscrn-cur-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-rgb565-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-spr-indfb-onoff:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-primscrn-pri-shrfb-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-pri-indfb-multidraw:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-scndscrn-pri-indfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-primscrn-cur-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-indfb-msflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-cur-indfb-move:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-primscrn-pri-shrfb-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-primscrn-cur-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-pri-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-offscren-pri-shrfb-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-scndscrn-indfb-msflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-rgb565-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-primscrn-cur-indfb-onoff:
                skip       -> PASS       (shard-hsw)
        Subgroup psr-1p-offscren-pri-shrfb-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-rgb565-draw-pwrite:
                skip       -> PASS       (shard-hsw)
        Subgroup fbc-shrfb-scaledprimary:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-primscrn-pri-indfb-draw-render:
                skip       -> PASS       (shard-hsw)
        Subgroup psr-2p-primscrn-pri-shrfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-scndscrn-indfb-pgflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-rgb101010-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-cur-indfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-pri-indfb-multidraw:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-offscren-pri-shrfb-draw-blt:
                skip       -> PASS       (shard-hsw)
        Subgroup psr-rgb101010-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-rte:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-offscren-pri-shrfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-primscrn-spr-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-indfb-scaledprimary:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-scndscrn-pri-shrfb-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-scndscrn-pri-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-primscrn-cur-indfb-draw-blt:
                skip       -> PASS       (shard-hsw)
        Subgroup fbc-1p-primscrn-shrfb-msflip-blt:
                skip       -> PASS       (shard-hsw)
        Subgroup psr-1p-offscren-pri-indfb-draw-mmap-gtt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-scndscrn-spr-indfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-rgb101010-draw-mmap-wc:
                skip       -> PASS       (shard-hsw)
        Subgroup psr-2p-scndscrn-spr-indfb-move:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-primscrn-pri-shrfb-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-pri-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-primscrn-cur-indfb-move:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-modesetfrombusy:
                skip       -> PASS       (shard-hsw)
        Subgroup fbc-1p-primscrn-pri-indfb-draw-mmap-wc:
                skip       -> PASS       (shard-hsw)
        Subgroup fbcpsr-badstride:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-primscrn-cur-indfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-pri-indfb-multidraw:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-offscren-pri-indfb-draw-blt:
                skip       -> PASS       (shard-hsw)
        Subgroup fbcpsr-2p-primscrn-cur-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-scndscrn-cur-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-spr-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-rgb565-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-offscren-pri-shrfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-rgb101010-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-primscrn-pri-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-primscrn-spr-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-primscrn-pri-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-shrfb-fliptrack:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-cur-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-cur-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-primscrn-spr-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-rgb565-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-primscrn-pri-indfb-draw-mmap-gtt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-tilingchange:
                skip       -> PASS       (shard-hsw)
        Subgroup fbc-2p-scndscrn-cur-indfb-draw-mmap-gtt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-primscrn-spr-indfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-primscrn-cur-indfb-move:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-primscrn-spr-indfb-onoff:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-primscrn-shrfb-pgflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-pri-shrfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-shrfb-pgflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-scndscrn-spr-indfb-draw-mmap-gtt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-primscrn-spr-indfb-onoff:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-rgb565-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-primscrn-spr-indfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-pri-shrfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-primscrn-cur-indfb-move:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-primscrn-pri-shrfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-primscrn-pri-shrfb-draw-mmap-gtt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-primscrn-cur-indfb-onoff:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-primscrn-cur-indfb-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-primscrn-shrfb-msflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-scndscrn-shrfb-pgflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-rte:
                skip       -> PASS       (shard-hsw)
        Subgroup psr-1p-primscrn-indfb-pgflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-primscrn-pri-indfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-primscrn-indfb-msflip-blt:
                skip       -> PASS       (shard-hsw)
        Subgroup psr-2p-scndscrn-pri-indfb-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-spr-indfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-suspend:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-scndscrn-cur-indfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-rgb101010-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-cur-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-rgb565-draw-mmap-gtt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-primscrn-pri-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-shrfb-msflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-shrfb-scaledprimary:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-stridechange:
                skip       -> PASS       (shard-hsw)
        Subgroup psr-1p-primscrn-cur-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-offscren-pri-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-primscrn-cur-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-primscrn-cur-indfb-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-primscrn-cur-indfb-onoff:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-primscrn-cur-indfb-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-primscrn-spr-indfb-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-offscren-pri-shrfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-primscrn-cur-indfb-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-scndscrn-spr-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-scndscrn-cur-indfb-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-primscrn-pri-shrfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-pri-shrfb-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-primscrn-spr-indfb-move:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-badstride:
                skip       -> PASS       (shard-hsw)
        Subgroup fbcpsr-2p-scndscrn-spr-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-primscrn-pri-indfb-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-primscrn-cur-indfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-indfb-scaledprimary:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-scndscrn-cur-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-primscrn-cur-indfb-draw-mmap-gtt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-offscren-pri-indfb-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-primscrn-cur-indfb-draw-mmap-cpu:
                skip       -> PASS       (shard-hsw)
        Subgroup fbc-2p-scndscrn-spr-indfb-move:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-primscrn-pri-indfb-draw-mmap-gtt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-spr-indfb-move:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-pri-indfb-draw-mmap-gtt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-pri-indfb-multidraw:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-primscrn-shrfb-pgflip-blt:
                skip       -> PASS       (shard-hsw)
        Subgroup fbc-1p-primscrn-cur-indfb-draw-render:
                skip       -> PASS       (shard-hsw)
        Subgroup psr-1p-primscrn-indfb-msflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-primscrn-shrfb-plflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-farfromfence:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-cur-indfb-draw-mmap-gtt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-pri-shrfb-draw-mmap-gtt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-rgb101010-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-indfb-pgflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-primscrn-indfb-plflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-scndscrn-spr-indfb-move:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-scndscrn-pri-shrfb-draw-mmap-gtt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-pri-shrfb-draw-pwrite:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-primscrn-spr-indfb-onoff:
                skip       -> PASS       (shard-hsw)
        Subgroup fbcpsr-2p-scndscrn-cur-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-scndscrn-pri-indfb-draw-render:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-suspend:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-1p-indfb-fliptrack:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-primscrn-indfb-plflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-pri-indfb-draw-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-spr-indfb-draw-mmap-gtt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-primscrn-pri-indfb-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-primscrn-cur-indfb-move:
                skip       -> PASS       (shard-hsw)
        Subgroup fbc-2p-scndscrn-cur-indfb-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-primscrn-spr-indfb-fullscreen:
                skip       -> PASS       (shard-hsw)
        Subgroup fbc-2p-primscrn-pri-indfb-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-1p-offscren-pri-indfb-draw-pwrite:
                skip       -> PASS       (shard-hsw)
        Subgroup fbc-1p-primscrn-spr-indfb-draw-blt:
                skip       -> PASS       (shard-hsw)
        Subgroup fbc-2p-scndscrn-cur-indfb-onoff:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-primscrn-shrfb-msflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-primscrn-spr-indfb-draw-mmap-wc:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-1p-primscrn-cur-indfb-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbcpsr-2p-scndscrn-shrfb-plflip-blt:
                pass       -> SKIP       (shard-hsw)
        Subgroup psr-2p-scndscrn-spr-indfb-draw-mmap-cpu:
                pass       -> SKIP       (shard-hsw)
        Subgroup fbc-2p-scndscrn-shrfb-pgflip-blt:
                pass       -> SKIP       (shard-hsw)

== Logs ==

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

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

* Re: [PATCH 08/10] drm/i915/guc: Fix GuC HW/SW state cleanup in unload path
  2017-09-17 12:17 ` [PATCH 08/10] drm/i915/guc: Fix GuC HW/SW state cleanup in unload path Sagar Arun Kamble
@ 2017-09-21 18:33   ` Oscar Mateo
  2017-09-21 19:09     ` Sagar Arun Kamble
                       ` (2 more replies)
  0 siblings, 3 replies; 27+ messages in thread
From: Oscar Mateo @ 2017-09-21 18:33 UTC (permalink / raw)
  To: Sagar Arun Kamble, intel-gfx

<SNIP>


On 09/17/2017 05:17 AM, Sagar Arun Kamble wrote:
> Teardown of GuC HW/SW state was not properly done in unload path.
> guc_submission_disable was called as part of intel_uc_fini_hw which
> happens post gem_unload in the i915_driver_unload path.
> s/i915_gem_fini/i915_gem_cleanup as it looks more suitable as that
> function does cleanup.
> To differentiate the tasks during suspend and unload w.r.t GuC this
> patch introduces new function i915_gem_fini which in addition to
> disabling GuC interfaces also disables GuC submission during which
> communication with GuC is needed for destroying doorbell.
> i915_gem_fini is copy of i915_gem_suspend with difference w.r.t
> GuC operations. To achieve this, new helpers i915_gem_context_suspend
> and i915_gem_suspend_complete are prepared.
>

Sagar, I just realized this patch would make this comment in 
guc_client_free superfluous, right?:

     /* FIXME: in many cases, by the time we get here the GuC has been
      * reset, so we cannot destroy the doorbell properly. Ignore the
      * error message for now */

Can you make sure you remove it in this patch?

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

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

* Re: [PATCH 08/10] drm/i915/guc: Fix GuC HW/SW state cleanup in unload path
  2017-09-21 18:33   ` Oscar Mateo
@ 2017-09-21 19:09     ` Sagar Arun Kamble
  2017-09-21 19:49       ` Oscar Mateo
  2017-09-22  4:42     ` Sagar Arun Kamble
  2017-09-22  4:42     ` [PATCH v6] drm/i915/guc: Update GuC suspend functionality in intel_uc_suspend path Sagar Arun Kamble
  2 siblings, 1 reply; 27+ messages in thread
From: Sagar Arun Kamble @ 2017-09-21 19:09 UTC (permalink / raw)
  To: Oscar Mateo, intel-gfx



On 9/22/2017 12:03 AM, Oscar Mateo wrote:
> <SNIP>
>
>
> On 09/17/2017 05:17 AM, Sagar Arun Kamble wrote:
>> Teardown of GuC HW/SW state was not properly done in unload path.
>> guc_submission_disable was called as part of intel_uc_fini_hw which
>> happens post gem_unload in the i915_driver_unload path.
>> s/i915_gem_fini/i915_gem_cleanup as it looks more suitable as that
>> function does cleanup.
>> To differentiate the tasks during suspend and unload w.r.t GuC this
>> patch introduces new function i915_gem_fini which in addition to
>> disabling GuC interfaces also disables GuC submission during which
>> communication with GuC is needed for destroying doorbell.
>> i915_gem_fini is copy of i915_gem_suspend with difference w.r.t
>> GuC operations. To achieve this, new helpers i915_gem_context_suspend
>> and i915_gem_suspend_complete are prepared.
>>
>
> Sagar, I just realized this patch would make this comment in 
> guc_client_free superfluous, right?:
>
>     /* FIXME: in many cases, by the time we get here the GuC has been
>      * reset, so we cannot destroy the doorbell properly. Ignore the
>      * error message for now */
>
> Can you make sure you remove it in this patch?
>
> Thanks!
Yes. Will need to remove this comment :)
Will make this change as part of 
https://patchwork.freedesktop.org/patch/178189/
How about another comment before it? i915_gem_suspend is ensuring no 
outstanding submissions before coming to
uc_suspend. So shall we remove that as well?

         /*
          * XXX: wait for any outstanding submissions before freeing memory.
          * Be sure to drop any locks
          */



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

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

* Re: [PATCH 08/10] drm/i915/guc: Fix GuC HW/SW state cleanup in unload path
  2017-09-21 19:09     ` Sagar Arun Kamble
@ 2017-09-21 19:49       ` Oscar Mateo
  2017-09-22  4:13         ` Sagar Arun Kamble
  0 siblings, 1 reply; 27+ messages in thread
From: Oscar Mateo @ 2017-09-21 19:49 UTC (permalink / raw)
  To: Sagar Arun Kamble, intel-gfx



On 09/21/2017 12:09 PM, Sagar Arun Kamble wrote:
>
>
> On 9/22/2017 12:03 AM, Oscar Mateo wrote:
>> <SNIP>
>>
>>
>> On 09/17/2017 05:17 AM, Sagar Arun Kamble wrote:
>>> Teardown of GuC HW/SW state was not properly done in unload path.
>>> guc_submission_disable was called as part of intel_uc_fini_hw which
>>> happens post gem_unload in the i915_driver_unload path.
>>> s/i915_gem_fini/i915_gem_cleanup as it looks more suitable as that
>>> function does cleanup.
>>> To differentiate the tasks during suspend and unload w.r.t GuC this
>>> patch introduces new function i915_gem_fini which in addition to
>>> disabling GuC interfaces also disables GuC submission during which
>>> communication with GuC is needed for destroying doorbell.
>>> i915_gem_fini is copy of i915_gem_suspend with difference w.r.t
>>> GuC operations. To achieve this, new helpers i915_gem_context_suspend
>>> and i915_gem_suspend_complete are prepared.
>>>
>>
>> Sagar, I just realized this patch would make this comment in 
>> guc_client_free superfluous, right?:
>>
>>     /* FIXME: in many cases, by the time we get here the GuC has been
>>      * reset, so we cannot destroy the doorbell properly. Ignore the
>>      * error message for now */
>>
>> Can you make sure you remove it in this patch?
>>
>> Thanks!
> Yes. Will need to remove this comment :)
> Will make this change as part of 
> https://patchwork.freedesktop.org/patch/178189/
> How about another comment before it? i915_gem_suspend is ensuring no 
> outstanding submissions before coming to
> uc_suspend. So shall we remove that as well?
>
>         /*
>          * XXX: wait for any outstanding submissions before freeing 
> memory.
>          * Be sure to drop any locks
>          */

I have the impression that this comment was here as a future design 
guide for Direct Submission. It seems to be the case for almost all the 
"XXX" comments in that file, like:

     /* XXX: wait for any interrupts */
     /* XXX: wait for workqueue to drain */

Since Direct Submission is not being considered at the moment, maybe we 
can just drop of all them (or transform them into something more 
innocuous than a "XXX").
What do people think?

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

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

* Re: [PATCH 08/10] drm/i915/guc: Fix GuC HW/SW state cleanup in unload path
  2017-09-21 19:49       ` Oscar Mateo
@ 2017-09-22  4:13         ` Sagar Arun Kamble
  0 siblings, 0 replies; 27+ messages in thread
From: Sagar Arun Kamble @ 2017-09-22  4:13 UTC (permalink / raw)
  To: Oscar Mateo, intel-gfx



On 9/22/2017 1:19 AM, Oscar Mateo wrote:
>
>
> On 09/21/2017 12:09 PM, Sagar Arun Kamble wrote:
>>
>>
>> On 9/22/2017 12:03 AM, Oscar Mateo wrote:
>>> <SNIP>
>>>
>>>
>>> On 09/17/2017 05:17 AM, Sagar Arun Kamble wrote:
>>>> Teardown of GuC HW/SW state was not properly done in unload path.
>>>> guc_submission_disable was called as part of intel_uc_fini_hw which
>>>> happens post gem_unload in the i915_driver_unload path.
>>>> s/i915_gem_fini/i915_gem_cleanup as it looks more suitable as that
>>>> function does cleanup.
>>>> To differentiate the tasks during suspend and unload w.r.t GuC this
>>>> patch introduces new function i915_gem_fini which in addition to
>>>> disabling GuC interfaces also disables GuC submission during which
>>>> communication with GuC is needed for destroying doorbell.
>>>> i915_gem_fini is copy of i915_gem_suspend with difference w.r.t
>>>> GuC operations. To achieve this, new helpers i915_gem_context_suspend
>>>> and i915_gem_suspend_complete are prepared.
>>>>
>>>
>>> Sagar, I just realized this patch would make this comment in 
>>> guc_client_free superfluous, right?:
>>>
>>>     /* FIXME: in many cases, by the time we get here the GuC has been
>>>      * reset, so we cannot destroy the doorbell properly. Ignore the
>>>      * error message for now */
>>>
>>> Can you make sure you remove it in this patch?
>>>
>>> Thanks!
>> Yes. Will need to remove this comment :)
>> Will make this change as part of 
>> https://patchwork.freedesktop.org/patch/178189/
>> How about another comment before it? i915_gem_suspend is ensuring no 
>> outstanding submissions before coming to
>> uc_suspend. So shall we remove that as well?
>>
>>         /*
>>          * XXX: wait for any outstanding submissions before freeing 
>> memory.
>>          * Be sure to drop any locks
>>          */
>
> I have the impression that this comment was here as a future design 
> guide for Direct Submission. It seems to be the case for almost all 
> the "XXX" comments in that file, like:
>
>     /* XXX: wait for any interrupts */
>     /* XXX: wait for workqueue to drain */
>
> Since Direct Submission is not being considered at the moment, maybe 
> we can just drop of all them (or transform them into something more 
> innocuous than a "XXX").
> What do people think?
>
I feel it makes sense to keep for future changes then. If needed, let us 
update these in separate patch.
Thanks for the review Oscar.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH 08/10] drm/i915/guc: Fix GuC HW/SW state cleanup in unload path
  2017-09-21 18:33   ` Oscar Mateo
  2017-09-21 19:09     ` Sagar Arun Kamble
@ 2017-09-22  4:42     ` Sagar Arun Kamble
  2017-09-22  4:42     ` [PATCH v6] drm/i915/guc: Update GuC suspend functionality in intel_uc_suspend path Sagar Arun Kamble
  2 siblings, 0 replies; 27+ messages in thread
From: Sagar Arun Kamble @ 2017-09-22  4:42 UTC (permalink / raw)
  To: Oscar Mateo, intel-gfx

This was reply to older series hence I replied patch there. Will update 
w.r.t new series.


On 9/22/2017 12:03 AM, Oscar Mateo wrote:
> <SNIP>
>
>
> On 09/17/2017 05:17 AM, Sagar Arun Kamble wrote:
>> Teardown of GuC HW/SW state was not properly done in unload path.
>> guc_submission_disable was called as part of intel_uc_fini_hw which
>> happens post gem_unload in the i915_driver_unload path.
>> s/i915_gem_fini/i915_gem_cleanup as it looks more suitable as that
>> function does cleanup.
>> To differentiate the tasks during suspend and unload w.r.t GuC this
>> patch introduces new function i915_gem_fini which in addition to
>> disabling GuC interfaces also disables GuC submission during which
>> communication with GuC is needed for destroying doorbell.
>> i915_gem_fini is copy of i915_gem_suspend with difference w.r.t
>> GuC operations. To achieve this, new helpers i915_gem_context_suspend
>> and i915_gem_suspend_complete are prepared.
>>
>
> Sagar, I just realized this patch would make this comment in 
> guc_client_free superfluous, right?:
>
>     /* FIXME: in many cases, by the time we get here the GuC has been
>      * reset, so we cannot destroy the doorbell properly. Ignore the
>      * error message for now */
>
> Can you make sure you remove it in this patch?
>
> Thanks!

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

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

* [PATCH v6] drm/i915/guc: Update GuC suspend functionality in intel_uc_suspend path
  2017-09-21 18:33   ` Oscar Mateo
  2017-09-21 19:09     ` Sagar Arun Kamble
  2017-09-22  4:42     ` Sagar Arun Kamble
@ 2017-09-22  4:42     ` Sagar Arun Kamble
  2 siblings, 0 replies; 27+ messages in thread
From: Sagar Arun Kamble @ 2017-09-22  4:42 UTC (permalink / raw)
  To: intel-gfx

With this patch we disable GuC submission in i915_drm_suspend. This will
destroy the client which will be setup back again. We also reuse the
complete sanitization done via intel_uc_runtime_suspend in this path.
Post drm resume this state is recreated by intel_uc_init_hw hence we need
not have similar reuse for intel_uc_resume.
This also fixes issue where intel_uc_fini_hw was being called after GPU
reset happening in i915_gem_suspend in i915_driver_unload.

v2: Rebase w.r.t removal of GuC code restructuring. Added struct_mutex
protection for i915_guc_submission_disable.

v3: Rebase w.r.t updated GuC suspend function name.

v4: Added lockdep assert in i915_guc_submission_enable/disable. Refined
intel_uc_suspend to remove unnecessary locals and simplify return.
(Michal Winiarski)
Removed comment in guc_client_free about ignoring failure for
destroy_doorbell. (Oscar)

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_guc_submission.c | 15 ++++++++++++---
 drivers/gpu/drm/i915/intel_uc.c            | 11 +++++++----
 2 files changed, 19 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index cd5ef8b..09fb156 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -879,9 +879,6 @@ static void guc_client_free(struct i915_guc_client *client)
 	 * Be sure to drop any locks
 	 */
 
-	/* FIXME: in many cases, by the time we get here the GuC has been
-	 * reset, so we cannot destroy the doorbell properly. Ignore the
-	 * error message for now */
 	destroy_doorbell(client);
 	guc_stage_desc_fini(client->guc, client);
 	i915_gem_object_unpin_map(client->vma->obj);
@@ -1148,6 +1145,12 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
 		     sizeof(struct guc_wq_item) *
 		     I915_NUM_ENGINES > GUC_WQ_SIZE);
 
+	/*
+	 * Assert that drm.struct_mutex is held.
+	 * Needed for GuC client vma binding.
+	 */
+	lockdep_assert_held(&dev_priv->drm.struct_mutex);
+
 	if (!client) {
 		client = guc_client_alloc(dev_priv,
 					  INTEL_INFO(dev_priv)->ring_mask,
@@ -1197,6 +1200,12 @@ void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
 
+	/*
+	 * Assert that drm.struct_mutex is held.
+	 * Needed for GuC client vma unbinding.
+	 */
+	lockdep_assert_held(&dev_priv->drm.struct_mutex);
+
 	guc_interrupts_release(dev_priv);
 
 	/* Revert back to manual ELSP submission */
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 66d2cff..0e971d7 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -446,9 +446,6 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 	if (!i915.enable_guc_loading)
 		return;
 
-	if (i915.enable_guc_submission)
-		i915_guc_submission_disable(dev_priv);
-
 	guc_disable_communication(&dev_priv->guc);
 
 	if (i915.enable_guc_submission) {
@@ -585,7 +582,13 @@ int intel_uc_runtime_resume(struct drm_i915_private *dev_priv)
 
 int intel_uc_suspend(struct drm_i915_private *dev_priv)
 {
-	return intel_guc_suspend(dev_priv);
+	if (i915.enable_guc_submission) {
+		mutex_lock(&dev_priv->drm.struct_mutex);
+		i915_guc_submission_disable(dev_priv);
+		mutex_unlock(&dev_priv->drm.struct_mutex);
+	}
+
+	return intel_uc_runtime_suspend(dev_priv);
 }
 
 int intel_uc_resume(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] 27+ messages in thread

* ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality (rev2)
  2017-09-17 12:17 [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Sagar Arun Kamble
                   ` (11 preceding siblings ...)
  2017-09-18  7:53 ` ✗ Fi.CI.IGT: warning " Patchwork
@ 2017-09-22 11:00 ` Patchwork
  2017-09-22 11:26   ` Kamble, Sagar A
  12 siblings, 1 reply; 27+ messages in thread
From: Patchwork @ 2017-09-22 11:00 UTC (permalink / raw)
  To: Sagar Arun Kamble; +Cc: intel-gfx

== Series Details ==

Series: series starting with [01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality (rev2)
URL   : https://patchwork.freedesktop.org/series/30486/
State : failure

== Summary ==

  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CHK     include/generated/bounds.h
  CHK     include/generated/timeconst.h
  CHK     include/generated/asm-offsets.h
  CALL    scripts/checksyscalls.sh
  CHK     scripts/mod/devicetable-offsets.h
  CHK     include/generated/compile.h
  CHK     kernel/config_data.h
  CC [M]  drivers/gpu/drm/i915/intel_uc.o
drivers/gpu/drm/i915/intel_uc.c: In function ‘intel_uc_system_suspend’:
drivers/gpu/drm/i915/intel_uc.c:441:1: error: version control conflict marker in file
 <<<<<<< HEAD
 ^~~~~~~
drivers/gpu/drm/i915/intel_uc.c:444:1: error: version control conflict marker in file
 =======
 ^~~~~~~
drivers/gpu/drm/i915/intel_uc.c:452:1: error: version control conflict marker in file
 >>>>>>> drm/i915/guc: Update GuC suspend functionality in intel_uc_suspend path
 ^~~~~~~
scripts/Makefile.build:311: recipe for target 'drivers/gpu/drm/i915/intel_uc.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_uc.o] Error 1
scripts/Makefile.build:570: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:570: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:570: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1019: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

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

* Re: ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality (rev2)
  2017-09-22 11:00 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality (rev2) Patchwork
@ 2017-09-22 11:26   ` Kamble, Sagar A
  0 siblings, 0 replies; 27+ messages in thread
From: Kamble, Sagar A @ 2017-09-22 11:26 UTC (permalink / raw)
  To: intel-gfx

Kindly ignore this failure and series. This was old series and due to replying a patch to it, BAT faced issue.

-----Original Message-----
From: Patchwork [mailto:patchwork@emeril.freedesktop.org] 
Sent: Friday, September 22, 2017 4:30 PM
To: Kamble, Sagar A <sagar.a.kamble@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality (rev2)

== Series Details ==

Series: series starting with [01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality (rev2)
URL   : https://patchwork.freedesktop.org/series/30486/
State : failure

== Summary ==

  CHK     include/config/kernel.release
  CHK     include/generated/uapi/linux/version.h
  CHK     include/generated/utsrelease.h
  CHK     include/generated/bounds.h
  CHK     include/generated/timeconst.h
  CHK     include/generated/asm-offsets.h
  CALL    scripts/checksyscalls.sh
  CHK     scripts/mod/devicetable-offsets.h
  CHK     include/generated/compile.h
  CHK     kernel/config_data.h
  CC [M]  drivers/gpu/drm/i915/intel_uc.o
drivers/gpu/drm/i915/intel_uc.c: In function ‘intel_uc_system_suspend’:
drivers/gpu/drm/i915/intel_uc.c:441:1: error: version control conflict marker in file  <<<<<<< HEAD  ^~~~~~~
drivers/gpu/drm/i915/intel_uc.c:444:1: error: version control conflict marker in file  =======  ^~~~~~~
drivers/gpu/drm/i915/intel_uc.c:452:1: error: version control conflict marker in file  >>>>>>> drm/i915/guc: Update GuC suspend functionality in intel_uc_suspend path  ^~~~~~~
scripts/Makefile.build:311: recipe for target 'drivers/gpu/drm/i915/intel_uc.o' failed
make[4]: *** [drivers/gpu/drm/i915/intel_uc.o] Error 1
scripts/Makefile.build:570: recipe for target 'drivers/gpu/drm/i915' failed
make[3]: *** [drivers/gpu/drm/i915] Error 2
scripts/Makefile.build:570: recipe for target 'drivers/gpu/drm' failed
make[2]: *** [drivers/gpu/drm] Error 2
scripts/Makefile.build:570: recipe for target 'drivers/gpu' failed
make[1]: *** [drivers/gpu] Error 2
Makefile:1019: recipe for target 'drivers' failed
make: *** [drivers] Error 2

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

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

end of thread, other threads:[~2017-09-22 11:27 UTC | newest]

Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-17 12:17 [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Sagar Arun Kamble
2017-09-17 12:17 ` [PATCH 02/10] drm/i915/guc: Move guc_send_* functions to intel_guc.c Sagar Arun Kamble
2017-09-17 12:17 ` [PATCH 03/10] drm/i915/guc: Move guc_sample_forcewake " Sagar Arun Kamble
2017-09-17 12:17 ` [PATCH 04/10] drm/i915/guc: Move GuC specific declarations from intel_uc.h to intel_guc.h Sagar Arun Kamble
2017-09-17 18:24   ` Michal Wajdeczko
2017-09-18  6:10     ` Kamble, Sagar A
2017-09-17 12:17 ` [PATCH 05/10] drm/i915: Reorganize HuC authentication Sagar Arun Kamble
2017-09-17 18:41   ` Michal Wajdeczko
2017-09-17 12:17 ` [PATCH 06/10] drm/i915/huc: Move HuC specific declarations from intel_uc.h to intel_huc.h Sagar Arun Kamble
2017-09-17 20:13   ` Michal Wajdeczko
2017-09-18  6:24     ` Kamble, Sagar A
2017-09-17 12:17 ` [PATCH 07/10] drm/i915/guc: Fix GuC interaction in reset/suspend scenarios Sagar Arun Kamble
2017-09-17 12:17 ` [PATCH 08/10] drm/i915/guc: Fix GuC HW/SW state cleanup in unload path Sagar Arun Kamble
2017-09-21 18:33   ` Oscar Mateo
2017-09-21 19:09     ` Sagar Arun Kamble
2017-09-21 19:49       ` Oscar Mateo
2017-09-22  4:13         ` Sagar Arun Kamble
2017-09-22  4:42     ` Sagar Arun Kamble
2017-09-22  4:42     ` [PATCH v6] drm/i915/guc: Update GuC suspend functionality in intel_uc_suspend path Sagar Arun Kamble
2017-09-17 12:17 ` [PATCH 09/10] drm/i915/guc: Enable default/critical logging in GuC by default from GuC v9 Sagar Arun Kamble
2017-09-17 12:17 ` [PATCH 10/10] drm/i915/guc: Remove i915_guc_log_unregister Sagar Arun Kamble
2017-09-17 19:30 ` [PATCH 01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality Michal Wajdeczko
2017-09-18  6:23   ` Kamble, Sagar A
2017-09-18  6:59 ` ✓ Fi.CI.BAT: success for series starting with [01/10] " Patchwork
2017-09-18  7:53 ` ✗ Fi.CI.IGT: warning " Patchwork
2017-09-22 11:00 ` ✗ Fi.CI.BAT: failure for series starting with [01/10] drm/i915/guc: Create intel_guc.c for defining GuC specific functionality (rev2) Patchwork
2017-09-22 11:26   ` Kamble, Sagar A

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.