[PATCH v3 0/9] drm/i915: Guc code reorg

[PATCH v3 0/9] drm/i915: Guc code reorg

[Michal Wajdeczko]

Other pending series will try to fix current GuC code.
Lets move some functions to dedicated files now to
make place for these changes and preserve history.

v2: move guc files in one step (Joonas)
    don't rename dev_priv (Joonas)
    uc_init_mmio (Sagar)
    
v3: more dedicated headers (Joonas)

Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>

Michal Wajdeczko (8):
  drm/i915: Make intel_uncore.h header self-contained
  drm/i915/uc: Drop unnecessary forward declaration
  drm/i915/uc: Move uC fw helper code into dedicated files
  drm/i915/huc: Move HuC declarations into dedicated header
  drm/i915/guc: Move Guc early init into own function
  drm/i915/guc: Move GuC log declarations into dedicated header
  drm/i915/guc: Move GuC submission declarations into dedicated header
  drm/i915/guc: Move GuC core definitions into dedicated files

Sagar Arun Kamble (1):
  drm/i915/uc: Create intel_uc_init_mmio

 drivers/gpu/drm/i915/Makefile              |   2 +
 drivers/gpu/drm/i915/i915_debugfs.c        |   1 +
 drivers/gpu/drm/i915/i915_drv.c            |   2 +
 drivers/gpu/drm/i915/i915_guc_submission.c | 101 +---------
 drivers/gpu/drm/i915/i915_guc_submission.h |  79 ++++++++
 drivers/gpu/drm/i915/intel_guc.c           | 264 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc.h           | 110 ++++++++++
 drivers/gpu/drm/i915/intel_guc_log.c       |   1 +
 drivers/gpu/drm/i915/intel_guc_log.h       |  57 ++++++
 drivers/gpu/drm/i915/intel_huc.h           |  39 ++++
 drivers/gpu/drm/i915/intel_uc.c            | 310 ++---------------------------
 drivers/gpu/drm/i915/intel_uc.h            | 228 +--------------------
 drivers/gpu/drm/i915/intel_uc_fw.c         | 179 +++++++++++++++++
 drivers/gpu/drm/i915/intel_uc_fw.h         |  98 +++++++++
 drivers/gpu/drm/i915/intel_uncore.h        |   6 +
 15 files changed, 861 insertions(+), 616 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_guc_submission.h
 create mode 100644 drivers/gpu/drm/i915/intel_guc.c
 create mode 100644 drivers/gpu/drm/i915/intel_guc.h
 create mode 100644 drivers/gpu/drm/i915/intel_guc_log.h
 create mode 100644 drivers/gpu/drm/i915/intel_huc.h
 create mode 100644 drivers/gpu/drm/i915/intel_uc_fw.c
 create mode 100644 drivers/gpu/drm/i915/intel_uc_fw.h

-- 
2.7.4

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

[PATCH v3 1/9] drm/i915: Make intel_uncore.h header self-contained

[Michal Wajdeczko]

We're trying to resolve inter-header dependencies.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_uncore.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
index 03786f9..66eae2c 100644
--- a/drivers/gpu/drm/i915/intel_uncore.h
+++ b/drivers/gpu/drm/i915/intel_uncore.h
@@ -25,6 +25,12 @@
 #ifndef __INTEL_UNCORE_H__
 #define __INTEL_UNCORE_H__
 
+#include <linux/spinlock.h>
+#include <linux/notifier.h>
+#include <linux/hrtimer.h>
+
+#include "i915_reg.h"
+
 struct drm_i915_private;
 
 enum forcewake_domain_id {
-- 
2.7.4

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

[PATCH v3 2/9] drm/i915/uc: Drop unnecessary forward declaration

[Michal Wajdeczko]

We don't need it here.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_uc.h | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 6966349..38ec880 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -30,8 +30,6 @@
 #include "intel_guc_ct.h"
 #include "i915_vma.h"
 
-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
-- 
2.7.4

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

[PATCH v3 3/9] drm/i915/uc: Create intel_uc_init_mmio

[Michal Wajdeczko]

From: Sagar Arun Kamble <sagar.a.kamble@intel.com>

This patch adds new function intel_uc_init_mmio which will initialize
MMIO access related variables prior to uc load/init.

v2: Removed unnecessary export of guc_send_init_regs. Created
intel_uc_init_mmio that currently wraps guc_init_send_regs. (Michal)

v3 (Michal): add kerneldoc (Joonas)

Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c |  2 ++
 drivers/gpu/drm/i915/intel_uc.c | 15 +++++++++++++--
 drivers/gpu/drm/i915/intel_uc.h |  1 +
 3 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5cc2434..74a456f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1007,6 +1007,8 @@ static int i915_driver_init_mmio(struct drm_i915_private *dev_priv)
 
 	intel_uncore_init(dev_priv);
 
+	intel_uc_init_mmio(dev_priv);
+
 	ret = intel_engines_init_mmio(dev_priv);
 	if (ret)
 		goto err_uncore;
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 2774778..8151a9d 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -288,6 +288,19 @@ static void guc_init_send_regs(struct intel_guc *guc)
 	guc->send_regs.fw_domains = fw_domains;
 }
 
+/**
+ * intel_uc_init_mmio - setup uC MMIO access
+ *
+ * @dev_priv: device private
+ *
+ * Setup minimal state necessary for MMIO accesses later in the
+ * initialization sequence.
+ */
+void intel_uc_init_mmio(struct drm_i915_private *dev_priv)
+{
+	guc_init_send_regs(&dev_priv->guc);
+}
+
 static void guc_capture_load_err_log(struct intel_guc *guc)
 {
 	if (!guc->log.vma || i915_modparams.guc_log_level < 0)
@@ -309,8 +322,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);
 
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 38ec880..388528f 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -202,6 +202,7 @@ struct intel_huc {
 /* 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);
+void intel_uc_init_mmio(struct drm_i915_private *dev_priv);
 void intel_uc_init_fw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
-- 
2.7.4

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

[PATCH v3 4/9] drm/i915/uc: Move uC fw helper code into dedicated files

[Michal Wajdeczko]

This is a prerequisite to unblock next steps.

v2: correct include order (Joonas)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/Makefile      |   1 +
 drivers/gpu/drm/i915/intel_uc.c    | 159 +-------------------------------
 drivers/gpu/drm/i915/intel_uc.h    |  67 +-------------
 drivers/gpu/drm/i915/intel_uc_fw.c | 179 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc_fw.h |  98 ++++++++++++++++++++
 5 files changed, 283 insertions(+), 221 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_uc_fw.c
 create mode 100644 drivers/gpu/drm/i915/intel_uc_fw.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 5182e3d..4850f26 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_uc_fw.o \
 	  intel_guc_ct.o \
 	  intel_guc_log.o \
 	  intel_guc_loader.o \
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 8151a9d..0b6c931 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -26,19 +26,6 @@
 #include "intel_uc.h"
 #include <linux/firmware.h>
 
-/* Cleans up uC firmware by releasing the firmware GEM obj.
- */
-static void __intel_uc_fw_fini(struct intel_uc_fw *uc_fw)
-{
-	struct drm_i915_gem_object *obj;
-
-	obj = fetch_and_zero(&uc_fw->obj);
-	if (obj)
-		i915_gem_object_put(obj);
-
-	uc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE;
-}
-
 /* Reset GuC providing us with fresh state for both GuC and HuC.
  */
 static int __intel_uc_reset_hw(struct drm_i915_private *dev_priv)
@@ -112,154 +99,16 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
 	guc->notify = gen8_guc_raise_irq;
 }
 
-static void fetch_uc_fw(struct drm_i915_private *dev_priv,
-			struct intel_uc_fw *uc_fw)
-{
-	struct pci_dev *pdev = dev_priv->drm.pdev;
-	struct drm_i915_gem_object *obj;
-	const struct firmware *fw = NULL;
-	struct uc_css_header *css;
-	size_t size;
-	int err;
-
-	if (!uc_fw->path)
-		return;
-
-	uc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING;
-
-	DRM_DEBUG_DRIVER("before requesting firmware: uC fw fetch status %s\n",
-			 intel_uc_fw_status_repr(uc_fw->fetch_status));
-
-	err = request_firmware(&fw, uc_fw->path, &pdev->dev);
-	if (err)
-		goto fail;
-	if (!fw)
-		goto fail;
-
-	DRM_DEBUG_DRIVER("fetch uC fw from %s succeeded, fw %p\n",
-			 uc_fw->path, fw);
-
-	/* Check the size of the blob before examining buffer contents */
-	if (fw->size < sizeof(struct uc_css_header)) {
-		DRM_NOTE("Firmware header is missing\n");
-		goto fail;
-	}
-
-	css = (struct uc_css_header *)fw->data;
-
-	/* Firmware bits always start from header */
-	uc_fw->header_offset = 0;
-	uc_fw->header_size = (css->header_size_dw - css->modulus_size_dw -
-			      css->key_size_dw - css->exponent_size_dw) * sizeof(u32);
-
-	if (uc_fw->header_size != sizeof(struct uc_css_header)) {
-		DRM_NOTE("CSS header definition mismatch\n");
-		goto fail;
-	}
-
-	/* then, uCode */
-	uc_fw->ucode_offset = uc_fw->header_offset + uc_fw->header_size;
-	uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
-
-	/* now RSA */
-	if (css->key_size_dw != UOS_RSA_SCRATCH_MAX_COUNT) {
-		DRM_NOTE("RSA key size is bad\n");
-		goto fail;
-	}
-	uc_fw->rsa_offset = uc_fw->ucode_offset + uc_fw->ucode_size;
-	uc_fw->rsa_size = css->key_size_dw * sizeof(u32);
-
-	/* At least, it should have header, uCode and RSA. Size of all three. */
-	size = uc_fw->header_size + uc_fw->ucode_size + uc_fw->rsa_size;
-	if (fw->size < size) {
-		DRM_NOTE("Missing firmware components\n");
-		goto fail;
-	}
-
-	/*
-	 * The GuC firmware image has the version number embedded at a
-	 * well-known offset within the firmware blob; note that major / minor
-	 * version are TWO bytes each (i.e. u16), although all pointers and
-	 * offsets are defined in terms of bytes (u8).
-	 */
-	switch (uc_fw->type) {
-	case INTEL_UC_FW_TYPE_GUC:
-		/* Header and uCode will be loaded to WOPCM. Size of the two. */
-		size = uc_fw->header_size + uc_fw->ucode_size;
-
-		/* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */
-		if (size > intel_guc_wopcm_size(dev_priv)) {
-			DRM_ERROR("Firmware is too large to fit in WOPCM\n");
-			goto fail;
-		}
-		uc_fw->major_ver_found = css->guc.sw_version >> 16;
-		uc_fw->minor_ver_found = css->guc.sw_version & 0xFFFF;
-		break;
-
-	case INTEL_UC_FW_TYPE_HUC:
-		uc_fw->major_ver_found = css->huc.sw_version >> 16;
-		uc_fw->minor_ver_found = css->huc.sw_version & 0xFFFF;
-		break;
-
-	default:
-		DRM_ERROR("Unknown firmware type %d\n", uc_fw->type);
-		err = -ENOEXEC;
-		goto fail;
-	}
-
-	if (uc_fw->major_ver_wanted == 0 && uc_fw->minor_ver_wanted == 0) {
-		DRM_NOTE("Skipping %s firmware version check\n",
-			 intel_uc_fw_type_repr(uc_fw->type));
-	} else if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
-		   uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
-		DRM_NOTE("%s firmware version %d.%d, required %d.%d\n",
-			 intel_uc_fw_type_repr(uc_fw->type),
-			 uc_fw->major_ver_found, uc_fw->minor_ver_found,
-			 uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
-		err = -ENOEXEC;
-		goto fail;
-	}
-
-	DRM_DEBUG_DRIVER("firmware version %d.%d OK (minimum %d.%d)\n",
-			 uc_fw->major_ver_found, uc_fw->minor_ver_found,
-			 uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
-
-	obj = i915_gem_object_create_from_data(dev_priv, fw->data, fw->size);
-	if (IS_ERR(obj)) {
-		err = PTR_ERR(obj);
-		goto fail;
-	}
-
-	uc_fw->obj = obj;
-	uc_fw->size = fw->size;
-
-	DRM_DEBUG_DRIVER("uC fw fetch status SUCCESS, obj %p\n",
-			 uc_fw->obj);
-
-	release_firmware(fw);
-	uc_fw->fetch_status = INTEL_UC_FIRMWARE_SUCCESS;
-	return;
-
-fail:
-	DRM_WARN("Failed to fetch valid uC firmware from %s (error %d)\n",
-		 uc_fw->path, err);
-	DRM_DEBUG_DRIVER("uC fw fetch status FAIL; err %d, fw %p, obj %p\n",
-			 err, fw, uc_fw->obj);
-
-	release_firmware(fw);		/* OK even if fw is NULL */
-	uc_fw->fetch_status = INTEL_UC_FIRMWARE_FAIL;
-}
-
 void intel_uc_init_fw(struct drm_i915_private *dev_priv)
 {
-	fetch_uc_fw(dev_priv, &dev_priv->huc.fw);
-	fetch_uc_fw(dev_priv, &dev_priv->guc.fw);
+	i915_fetch_uc_fw(dev_priv, &dev_priv->huc.fw);
+	i915_fetch_uc_fw(dev_priv, &dev_priv->guc.fw);
 }
 
 void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
 {
-	__intel_uc_fw_fini(&dev_priv->guc.fw);
-	__intel_uc_fw_fini(&dev_priv->huc.fw);
+	intel_uc_fw_fini(&dev_priv->guc.fw);
+	intel_uc_fw_fini(&dev_priv->huc.fw);
 }
 
 static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 388528f..f0d5a3f 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -24,6 +24,7 @@
 #ifndef _INTEL_UC_H_
 #define _INTEL_UC_H_
 
+#include "intel_uc_fw.h"
 #include "intel_guc_fwif.h"
 #include "i915_guc_reg.h"
 #include "intel_ringbuffer.h"
@@ -70,72 +71,6 @@ struct i915_guc_client {
 	uint64_t submissions[I915_NUM_ENGINES];
 };
 
-enum intel_uc_fw_status {
-	INTEL_UC_FIRMWARE_FAIL = -1,
-	INTEL_UC_FIRMWARE_NONE = 0,
-	INTEL_UC_FIRMWARE_PENDING,
-	INTEL_UC_FIRMWARE_SUCCESS
-};
-
-/* User-friendly representation of an enum */
-static inline
-const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
-{
-	switch (status) {
-	case INTEL_UC_FIRMWARE_FAIL:
-		return "FAIL";
-	case INTEL_UC_FIRMWARE_NONE:
-		return "NONE";
-	case INTEL_UC_FIRMWARE_PENDING:
-		return "PENDING";
-	case INTEL_UC_FIRMWARE_SUCCESS:
-		return "SUCCESS";
-	}
-	return "<invalid>";
-}
-
-enum intel_uc_fw_type {
-	INTEL_UC_FW_TYPE_GUC,
-	INTEL_UC_FW_TYPE_HUC
-};
-
-/* User-friendly representation of an enum */
-static inline const char *intel_uc_fw_type_repr(enum intel_uc_fw_type type)
-{
-	switch (type) {
-	case INTEL_UC_FW_TYPE_GUC:
-		return "GuC";
-	case INTEL_UC_FW_TYPE_HUC:
-		return "HuC";
-	}
-	return "uC";
-}
-
-/*
- * This structure encapsulates all the data needed during the process
- * of fetching, caching, and loading the firmware image into the GuC.
- */
-struct intel_uc_fw {
-	const char *path;
-	size_t size;
-	struct drm_i915_gem_object *obj;
-	enum intel_uc_fw_status fetch_status;
-	enum intel_uc_fw_status load_status;
-
-	uint16_t major_ver_wanted;
-	uint16_t minor_ver_wanted;
-	uint16_t major_ver_found;
-	uint16_t minor_ver_found;
-
-	enum intel_uc_fw_type type;
-	uint32_t header_size;
-	uint32_t header_offset;
-	uint32_t rsa_size;
-	uint32_t rsa_offset;
-	uint32_t ucode_size;
-	uint32_t ucode_offset;
-};
-
 struct intel_guc_log {
 	uint32_t flags;
 	struct i915_vma *vma;
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.c b/drivers/gpu/drm/i915/intel_uc_fw.c
new file mode 100644
index 0000000..0c9284f
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_uc_fw.c
@@ -0,0 +1,179 @@
+/*
+ * Copyright © 2016-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 <linux/firmware.h>
+#include "intel_uc_fw.h"
+#include "i915_drv.h"
+
+/* Cleans up uC firmware by releasing the firmware GEM obj.
+ */
+void intel_uc_fw_fini(struct intel_uc_fw *uc_fw)
+{
+	struct drm_i915_gem_object *obj;
+
+	obj = fetch_and_zero(&uc_fw->obj);
+	if (obj)
+		i915_gem_object_put(obj);
+
+	uc_fw->fetch_status = INTEL_UC_FIRMWARE_NONE;
+}
+
+void i915_fetch_uc_fw(struct drm_i915_private *dev_priv,
+		      struct intel_uc_fw *uc_fw)
+{
+	struct pci_dev *pdev = dev_priv->drm.pdev;
+	struct drm_i915_gem_object *obj;
+	const struct firmware *fw = NULL;
+	struct uc_css_header *css;
+	size_t size;
+	int err;
+
+	if (!uc_fw->path)
+		return;
+
+	uc_fw->fetch_status = INTEL_UC_FIRMWARE_PENDING;
+
+	DRM_DEBUG_DRIVER("before requesting firmware: uC fw fetch status %s\n",
+			 intel_uc_fw_status_repr(uc_fw->fetch_status));
+
+	err = request_firmware(&fw, uc_fw->path, &pdev->dev);
+	if (err)
+		goto fail;
+	if (!fw)
+		goto fail;
+
+	DRM_DEBUG_DRIVER("fetch uC fw from %s succeeded, fw %p\n",
+			 uc_fw->path, fw);
+
+	/* Check the size of the blob before examining buffer contents */
+	if (fw->size < sizeof(struct uc_css_header)) {
+		DRM_NOTE("Firmware header is missing\n");
+		goto fail;
+	}
+
+	css = (struct uc_css_header *)fw->data;
+
+	/* Firmware bits always start from header */
+	uc_fw->header_offset = 0;
+	uc_fw->header_size = (css->header_size_dw - css->modulus_size_dw -
+			      css->key_size_dw - css->exponent_size_dw) *
+			     sizeof(u32);
+
+	if (uc_fw->header_size != sizeof(struct uc_css_header)) {
+		DRM_NOTE("CSS header definition mismatch\n");
+		goto fail;
+	}
+
+	/* then, uCode */
+	uc_fw->ucode_offset = uc_fw->header_offset + uc_fw->header_size;
+	uc_fw->ucode_size = (css->size_dw - css->header_size_dw) * sizeof(u32);
+
+	/* now RSA */
+	if (css->key_size_dw != UOS_RSA_SCRATCH_MAX_COUNT) {
+		DRM_NOTE("RSA key size is bad\n");
+		goto fail;
+	}
+	uc_fw->rsa_offset = uc_fw->ucode_offset + uc_fw->ucode_size;
+	uc_fw->rsa_size = css->key_size_dw * sizeof(u32);
+
+	/* At least, it should have header, uCode and RSA. Size of all three. */
+	size = uc_fw->header_size + uc_fw->ucode_size + uc_fw->rsa_size;
+	if (fw->size < size) {
+		DRM_NOTE("Missing firmware components\n");
+		goto fail;
+	}
+
+	/*
+	 * The GuC firmware image has the version number embedded at a
+	 * well-known offset within the firmware blob; note that major / minor
+	 * version are TWO bytes each (i.e. u16), although all pointers and
+	 * offsets are defined in terms of bytes (u8).
+	 */
+	switch (uc_fw->type) {
+	case INTEL_UC_FW_TYPE_GUC:
+		/* Header and uCode will be loaded to WOPCM. Size of the two. */
+		size = uc_fw->header_size + uc_fw->ucode_size;
+
+		/* Top 32k of WOPCM is reserved (8K stack + 24k RC6 context). */
+		if (size > intel_guc_wopcm_size(dev_priv)) {
+			DRM_ERROR("Firmware is too large to fit in WOPCM\n");
+			goto fail;
+		}
+		uc_fw->major_ver_found = css->guc.sw_version >> 16;
+		uc_fw->minor_ver_found = css->guc.sw_version & 0xFFFF;
+		break;
+
+	case INTEL_UC_FW_TYPE_HUC:
+		uc_fw->major_ver_found = css->huc.sw_version >> 16;
+		uc_fw->minor_ver_found = css->huc.sw_version & 0xFFFF;
+		break;
+
+	default:
+		DRM_ERROR("Unknown firmware type %d\n", uc_fw->type);
+		err = -ENOEXEC;
+		goto fail;
+	}
+
+	if (uc_fw->major_ver_wanted == 0 && uc_fw->minor_ver_wanted == 0) {
+		DRM_NOTE("Skipping %s firmware version check\n",
+			 intel_uc_fw_type_repr(uc_fw->type));
+	} else if (uc_fw->major_ver_found != uc_fw->major_ver_wanted ||
+		   uc_fw->minor_ver_found < uc_fw->minor_ver_wanted) {
+		DRM_NOTE("%s firmware version %d.%d, required %d.%d\n",
+			 intel_uc_fw_type_repr(uc_fw->type),
+			 uc_fw->major_ver_found, uc_fw->minor_ver_found,
+			 uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
+		err = -ENOEXEC;
+		goto fail;
+	}
+
+	DRM_DEBUG_DRIVER("firmware version %d.%d OK (minimum %d.%d)\n",
+			 uc_fw->major_ver_found, uc_fw->minor_ver_found,
+			 uc_fw->major_ver_wanted, uc_fw->minor_ver_wanted);
+
+	obj = i915_gem_object_create_from_data(dev_priv, fw->data, fw->size);
+	if (IS_ERR(obj)) {
+		err = PTR_ERR(obj);
+		goto fail;
+	}
+
+	uc_fw->obj = obj;
+	uc_fw->size = fw->size;
+
+	DRM_DEBUG_DRIVER("uC fw fetch status SUCCESS, obj %p\n",
+			 uc_fw->obj);
+
+	release_firmware(fw);
+	uc_fw->fetch_status = INTEL_UC_FIRMWARE_SUCCESS;
+	return;
+
+fail:
+	DRM_WARN("Failed to fetch valid uC firmware from %s (error %d)\n",
+		 uc_fw->path, err);
+	DRM_DEBUG_DRIVER("uC fw fetch status FAIL; err %d, fw %p, obj %p\n",
+			 err, fw, uc_fw->obj);
+
+	release_firmware(fw);		/* OK even if fw is NULL */
+	uc_fw->fetch_status = INTEL_UC_FIRMWARE_FAIL;
+}
diff --git a/drivers/gpu/drm/i915/intel_uc_fw.h b/drivers/gpu/drm/i915/intel_uc_fw.h
new file mode 100644
index 0000000..1450ad3
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_uc_fw.h
@@ -0,0 +1,98 @@
+/*
+ * Copyright © 2014-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_UC_FW_H_
+#define _INTEL_UC_FW_H_
+
+struct drm_i915_private;
+
+enum intel_uc_fw_status {
+	INTEL_UC_FIRMWARE_FAIL = -1,
+	INTEL_UC_FIRMWARE_NONE = 0,
+	INTEL_UC_FIRMWARE_PENDING,
+	INTEL_UC_FIRMWARE_SUCCESS
+};
+
+enum intel_uc_fw_type {
+	INTEL_UC_FW_TYPE_GUC,
+	INTEL_UC_FW_TYPE_HUC
+};
+
+/*
+ * This structure encapsulates all the data needed during the process
+ * of fetching, caching, and loading the firmware image into the uC.
+ */
+struct intel_uc_fw {
+	const char *path;
+	size_t size;
+	struct drm_i915_gem_object *obj;
+	enum intel_uc_fw_status fetch_status;
+	enum intel_uc_fw_status load_status;
+
+	uint16_t major_ver_wanted;
+	uint16_t minor_ver_wanted;
+	uint16_t major_ver_found;
+	uint16_t minor_ver_found;
+
+	enum intel_uc_fw_type type;
+	uint32_t header_size;
+	uint32_t header_offset;
+	uint32_t rsa_size;
+	uint32_t rsa_offset;
+	uint32_t ucode_size;
+	uint32_t ucode_offset;
+};
+
+static inline
+const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
+{
+	switch (status) {
+	case INTEL_UC_FIRMWARE_FAIL:
+		return "FAIL";
+	case INTEL_UC_FIRMWARE_NONE:
+		return "NONE";
+	case INTEL_UC_FIRMWARE_PENDING:
+		return "PENDING";
+	case INTEL_UC_FIRMWARE_SUCCESS:
+		return "SUCCESS";
+	}
+	return "<invalid>";
+}
+
+static inline const char *intel_uc_fw_type_repr(enum intel_uc_fw_type type)
+{
+	switch (type) {
+	case INTEL_UC_FW_TYPE_GUC:
+		return "GuC";
+	case INTEL_UC_FW_TYPE_HUC:
+		return "HuC";
+	}
+	return "uC";
+}
+
+void intel_uc_fw_fini(struct intel_uc_fw *uc_fw);
+void i915_fetch_uc_fw(struct drm_i915_private *dev_priv,
+		      struct intel_uc_fw *uc_fw);
+
+#endif
-- 
2.7.4

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

[PATCH v3 5/9] drm/i915/huc: Move HuC declarations into dedicated header

[Michal Wajdeczko]

We want to keep each uC specific code in separate files.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_huc.h | 39 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.h  | 13 +------------
 2 files changed, 40 insertions(+), 12 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_huc.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..d58422b
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -0,0 +1,39 @@
+/*
+ * Copyright © 2014-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 */
+};
+
+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 f0d5a3f..4fa091e 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -30,6 +30,7 @@
 #include "intel_ringbuffer.h"
 #include "intel_guc_ct.h"
 #include "i915_vma.h"
+#include "intel_huc.h"
 
 /*
  * This structure primarily describes the GEM object shared with the GuC.
@@ -127,13 +128,6 @@ struct intel_guc {
 	void (*notify)(struct intel_guc *guc);
 };
 
-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);
@@ -186,9 +180,4 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 	return offset;
 }
 
-/* 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
-- 
2.7.4

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

[PATCH v3 6/9] drm/i915/guc: Move Guc early init into own function

[Michal Wajdeczko]

We don't want to make aggregate uc functions to be too detailed.
This will also make future patch easier.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_uc.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 0b6c931..b1bd4d3 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -88,10 +88,8 @@ static void gen8_guc_raise_irq(struct intel_guc *guc)
 	I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
 }
 
-void intel_uc_init_early(struct drm_i915_private *dev_priv)
+static void guc_init_early(struct intel_guc *guc)
 {
-	struct intel_guc *guc = &dev_priv->guc;
-
 	intel_guc_ct_init_early(&guc->ct);
 
 	mutex_init(&guc->send_mutex);
@@ -99,6 +97,11 @@ void intel_uc_init_early(struct drm_i915_private *dev_priv)
 	guc->notify = gen8_guc_raise_irq;
 }
 
+void intel_uc_init_early(struct drm_i915_private *dev_priv)
+{
+	guc_init_early(&dev_priv->guc);
+}
+
 void intel_uc_init_fw(struct drm_i915_private *dev_priv)
 {
 	i915_fetch_uc_fw(dev_priv, &dev_priv->huc.fw);
-- 
2.7.4

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

[PATCH v3 7/9] drm/i915/guc: Move GuC log declarations into dedicated header

[Michal Wajdeczko]

We want to keep component specific code in separate files.

Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_log.c |  1 +
 drivers/gpu/drm/i915/intel_guc_log.h | 57 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.h      | 26 +---------------
 3 files changed, 59 insertions(+), 25 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_guc_log.h

diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 6571d96..bd9b02e 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -23,6 +23,7 @@
  */
 #include <linux/debugfs.h>
 #include <linux/relay.h>
+#include "intel_guc_log.h"
 #include "i915_drv.h"
 
 static void guc_log_capture_logs(struct intel_guc *guc);
diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h
new file mode 100644
index 0000000..099a0c8
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc_log.h
@@ -0,0 +1,57 @@
+/*
+ * Copyright © 2014-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_LOG_H_
+#define _INTEL_GUC_LOG_H_
+
+#include <linux/workqueue.h>
+#include "intel_guc_fwif.h"
+
+struct drm_i915_private;
+struct intel_guc;
+
+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];
+};
+
+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 4fa091e..86ae507 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -29,6 +29,7 @@
 #include "i915_guc_reg.h"
 #include "intel_ringbuffer.h"
 #include "intel_guc_ct.h"
+#include "intel_guc_log.h"
 #include "i915_vma.h"
 #include "intel_huc.h"
 
@@ -72,24 +73,6 @@ struct i915_guc_client {
 	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;
@@ -165,13 +148,6 @@ 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);
-- 
2.7.4

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

[PATCH v3 8/9] drm/i915/guc: Move GuC submission declarations into dedicated header

[Michal Wajdeczko]

We want to keep uC specific code in separate files.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: MichaĹ Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c        |  1 +
 drivers/gpu/drm/i915/i915_guc_submission.c |  7 +--
 drivers/gpu/drm/i915/i915_guc_submission.h | 79 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_uc.c            |  1 +
 drivers/gpu/drm/i915/intel_uc.h            | 45 -----------------
 5 files changed, 85 insertions(+), 48 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_guc_submission.h

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b4a6ac6..44aae25 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -30,6 +30,7 @@
 #include <linux/sort.h>
 #include <linux/sched/mm.h>
 #include "intel_drv.h"
+#include "i915_guc_submission.h"
 
 static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
 {
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 04f1281..97dfe96 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -21,12 +21,13 @@
  * IN THE SOFTWARE.
  *
  */
-#include <linux/circ_buf.h>
-#include "i915_drv.h"
-#include "intel_uc.h"
 
+#include <linux/circ_buf.h>
 #include <trace/events/dma_fence.h>
 
+#include "i915_guc_submission.h"
+#include "i915_drv.h"
+
 /**
  * DOC: GuC-based command submission
  *
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.h b/drivers/gpu/drm/i915/i915_guc_submission.h
new file mode 100644
index 0000000..b43af42
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_guc_submission.h
@@ -0,0 +1,79 @@
+/*
+ * Copyright © 2014-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 _I915_GUC_SUBMISSION_H_
+#define _I915_GUC_SUBMISSION_H_
+
+#include <linux/spinlock.h>
+#include "i915_gem.h"
+
+struct drm_i915_private;
+
+/*
+ * 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;
+
+	/* bitmap of (host) engine ids */
+	uint32_t engines;
+	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];
+};
+
+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);
+
+#endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index b1bd4d3..2ccf39c 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 "i915_guc_submission.h"
 #include <linux/firmware.h>
 
 /* Reset GuC providing us with fresh state for both GuC and HuC.
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 86ae507..2f741a9 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -33,46 +33,6 @@
 #include "i915_vma.h"
 #include "intel_huc.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 {
 	struct intel_uc_fw fw;
 	struct intel_guc_log log;
@@ -141,11 +101,6 @@ 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);
 
 static inline u32 guc_ggtt_offset(struct i915_vma *vma)
-- 
2.7.4

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

[PATCH v3 9/9] drm/i915/guc: Move GuC core definitions into dedicated files

[Michal Wajdeczko]

We want to keep GuC specific code in separated files.

v2: move all functions in single patch (Joonas)
    fix old checkpatch issues (Sagar)

v3: rebased

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> #1
---
 drivers/gpu/drm/i915/Makefile              |   1 +
 drivers/gpu/drm/i915/i915_guc_submission.c |  94 ----------
 drivers/gpu/drm/i915/intel_guc.c           | 264 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc.h           | 110 ++++++++++++
 drivers/gpu/drm/i915/intel_uc.c            | 146 +---------------
 drivers/gpu/drm/i915/intel_uc.h            |  78 +--------
 6 files changed, 378 insertions(+), 315 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 4850f26..51d0d29 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -60,6 +60,7 @@ i915-y += i915_cmd_parser.o \
 # general-purpose microcontroller (GuC) support
 i915-y += intel_uc.o \
 	  intel_uc_fw.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 97dfe96..7460ab4 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -644,48 +644,6 @@ static void i915_guc_irq_handler(unsigned long data)
  * path of i915_guc_submit() above.
  */
 
-/**
- * intel_guc_allocate_vma() - Allocate a GGTT VMA for GuC usage
- * @guc:	the guc
- * @size:	size of area to allocate (both virtual space and memory)
- *
- * This is a wrapper to create an object for use with the GuC. In order to
- * use it inside the GuC, an object needs to be pinned lifetime, so we allocate
- * both some backing storage and a range inside the Global GTT. We must pin
- * it in the GGTT somewhere other than than [0, GUC_WOPCM_TOP) because that
- * range is reserved inside GuC.
- *
- * Return:	A i915_vma if successful, otherwise an ERR_PTR.
- */
-struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
-{
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	struct drm_i915_gem_object *obj;
-	struct i915_vma *vma;
-	int ret;
-
-	obj = i915_gem_object_create(dev_priv, size);
-	if (IS_ERR(obj))
-		return ERR_CAST(obj);
-
-	vma = i915_vma_instance(obj, &dev_priv->ggtt.base, NULL);
-	if (IS_ERR(vma))
-		goto err;
-
-	ret = i915_vma_pin(vma, 0, PAGE_SIZE,
-			   PIN_GLOBAL | PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
-	if (ret) {
-		vma = ERR_PTR(ret);
-		goto err;
-	}
-
-	return vma;
-
-err:
-	i915_gem_object_put(obj);
-	return vma;
-}
-
 /* Check that a doorbell register is in the expected state */
 static bool doorbell_ok(struct intel_guc *guc, u16 db_id)
 {
@@ -1213,55 +1171,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_modparams.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
new file mode 100644
index 0000000..bbe4c32
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -0,0 +1,264 @@
+/*
+ * Copyright © 2014-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 "intel_guc.h"
+#include "i915_drv.h"
+
+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);
+}
+
+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;
+}
+
+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;
+}
+
+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.
+ */
+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);
+	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));
+}
+
+/**
+ * intel_guc_auth_huc() - Send action to GuC to authenticate HuC ucode
+ * @guc: intel_guc structure
+ * @rsa_offset: rsa offset w.r.t ggtt base of huc vma
+ *
+ * Triggers a HuC firmware authentication request to the GuC via intel_guc_send
+ * INTEL_GUC_ACTION_AUTHENTICATE_HUC interface. This function is invoked by
+ * intel_huc_auth().
+ *
+ * Return:	non-zero code on error
+ */
+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));
+}
+
+/**
+ * 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_modparams.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));
+}
+
+/**
+ * intel_guc_allocate_vma() - Allocate a GGTT VMA for GuC usage
+ * @guc:	the guc
+ * @size:	size of area to allocate (both virtual space and memory)
+ *
+ * This is a wrapper to create an object for use with the GuC. In order to
+ * use it inside the GuC, an object needs to be pinned lifetime, so we allocate
+ * both some backing storage and a range inside the Global GTT. We must pin
+ * it in the GGTT somewhere other than than [0, GUC_WOPCM_TOP) because that
+ * range is reserved inside GuC.
+ *
+ * Return:	A i915_vma if successful, otherwise an ERR_PTR.
+ */
+struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct drm_i915_gem_object *obj;
+	struct i915_vma *vma;
+	int ret;
+
+	obj = i915_gem_object_create(dev_priv, size);
+	if (IS_ERR(obj))
+		return ERR_CAST(obj);
+
+	vma = i915_vma_instance(obj, &dev_priv->ggtt.base, NULL);
+	if (IS_ERR(vma))
+		goto err;
+
+	ret = i915_vma_pin(vma, 0, PAGE_SIZE,
+			   PIN_GLOBAL | PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+	if (ret) {
+		vma = ERR_PTR(ret);
+		goto err;
+	}
+
+	return vma;
+
+err:
+	i915_gem_object_put(obj);
+	return vma;
+}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
new file mode 100644
index 0000000..bd5a223
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -0,0 +1,110 @@
+/*
+ * Copyright © 2014-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_
+
+#include "intel_uncore.h"
+#include "intel_guc_fwif.h"
+#include "intel_guc_ct.h"
+#include "intel_guc_log.h"
+#include "intel_uc_fw.h"
+#include "i915_guc_reg.h"
+#include "i915_vma.h"
+
+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)
+{
+	return guc->send(guc, action, len);
+}
+
+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;
+}
+
+void intel_guc_init_early(struct intel_guc *guc);
+void intel_guc_init_send_regs(struct intel_guc *guc);
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
+int intel_guc_sample_forcewake(struct intel_guc *guc);
+int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
+int intel_guc_suspend(struct drm_i915_private *dev_priv);
+int intel_guc_resume(struct drm_i915_private *dev_priv);
+struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
+
+/* intel_guc_loader.c */
+int intel_guc_select_fw(struct intel_guc *guc);
+int intel_guc_init_hw(struct intel_guc *guc);
+u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
+
+#endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 2ccf39c..0a128e7 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -82,25 +82,9 @@ void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
 		i915_modparams.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);
-}
-
-static void 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;
-}
-
 void intel_uc_init_early(struct drm_i915_private *dev_priv)
 {
-	guc_init_early(&dev_priv->guc);
+	intel_guc_init_early(&dev_priv->guc);
 }
 
 void intel_uc_init_fw(struct drm_i915_private *dev_priv)
@@ -115,32 +99,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;
-}
-
 /**
  * intel_uc_init_mmio - setup uC MMIO access
  *
@@ -151,7 +109,7 @@ static void guc_init_send_regs(struct intel_guc *guc)
  */
 void intel_uc_init_mmio(struct drm_i915_private *dev_priv)
 {
-	guc_init_send_regs(&dev_priv->guc);
+	intel_guc_init_send_regs(&dev_priv->guc);
 }
 
 static void guc_capture_load_err_log(struct intel_guc *guc)
@@ -192,27 +150,6 @@ static void guc_disable_communication(struct intel_guc *guc)
 	guc->send = intel_guc_send_nop;
 }
 
-/**
- * intel_guc_auth_huc() - Send action to GuC to authenticate HuC ucode
- * @guc: intel_guc structure
- * @rsa_offset: rsa offset w.r.t ggtt base of huc vma
- *
- * Triggers a HuC firmware authentication request to the GuC via intel_guc_send
- * INTEL_GUC_ACTION_AUTHENTICATE_HUC interface. This function is invoked by
- * intel_huc_auth().
- *
- * Return:	non-zero code on error
- */
-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));
-}
-
 int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
@@ -344,82 +281,3 @@ 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.
- */
-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);
-	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 2f741a9..e18d3bb 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -24,54 +24,9 @@
 #ifndef _INTEL_UC_H_
 #define _INTEL_UC_H_
 
-#include "intel_uc_fw.h"
-#include "intel_guc_fwif.h"
-#include "i915_guc_reg.h"
-#include "intel_ringbuffer.h"
-#include "intel_guc_ct.h"
-#include "intel_guc_log.h"
-#include "i915_vma.h"
+#include "intel_guc.h"
 #include "intel_huc.h"
 
-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);
-};
-
-/* 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);
 void intel_uc_init_mmio(struct drm_i915_private *dev_priv);
@@ -79,36 +34,5 @@ void intel_uc_init_fw(struct drm_i915_private *dev_priv);
 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);
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
-int intel_guc_auth_huc(struct intel_guc *guc, u32 rsa_offset);
-
-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);
-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);
-
-struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
-
-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;
-}
 
 #endif
-- 
2.7.4

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

Re: [PATCH v3 1/9] drm/i915: Make intel_uncore.h header self-contained

[Chris Wilson]

Quoting Michal Wajdeczko (2017-10-03 17:35:59)
> We're trying to resolve inter-header dependencies.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_uncore.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_uncore.h b/drivers/gpu/drm/i915/intel_uncore.h
> index 03786f9..66eae2c 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.h
> +++ b/drivers/gpu/drm/i915/intel_uncore.h
> @@ -25,6 +25,12 @@
>  #ifndef __INTEL_UNCORE_H__
>  #define __INTEL_UNCORE_H__
>  
> +#include <linux/spinlock.h>
> +#include <linux/notifier.h>
> +#include <linux/hrtimer.h>
> +
> +#include "i915_reg.h"

Hmm, what do we pull in directly? Something like i915_mmio_offset()?

In my head I have i915_reg.h == unwieldy list of register defines and
nothing more.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

Looks sensible but if it's only one small thing for i915_reg.h can you
add a comment and we can try and split it up.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 7/9] drm/i915/guc: Move GuC log declarations into dedicated header

[Chris Wilson]

Quoting Michal Wajdeczko (2017-10-03 17:36:05)
> We want to keep component specific code in separate files.
> 
> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_log.c |  1 +
>  drivers/gpu/drm/i915/intel_guc_log.h | 57 ++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_uc.h      | 26 +---------------
>  3 files changed, 59 insertions(+), 25 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/intel_guc_log.h
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index 6571d96..bd9b02e 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -23,6 +23,7 @@
>   */
>  #include <linux/debugfs.h>
>  #include <linux/relay.h>
> +#include "intel_guc_log.h"
>  #include "i915_drv.h"

Can we keep a newline between <> and "", and aim for alphabetical order.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Guc code reorg

[Patchwork]

== Series Details ==

Series: drm/i915: Guc code reorg
URL   : https://patchwork.freedesktop.org/series/31340/
State : success

== Summary ==

Series 31340v1 drm/i915: Guc code reorg
https://patchwork.freedesktop.org/api/1.0/series/31340/revisions/1/mbox/

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:457s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:472s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:392s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:569s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:286s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:526s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:534s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:540s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:536s
fi-cfl-s         total:289  pass:256  dwarn:1   dfail:0   fail:0   skip:32  time:564s
fi-cnl-y         total:289  pass:261  dwarn:1   dfail:0   fail:0   skip:27  time:618s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:439s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:591s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:439s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:417s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:467s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:518s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:478s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:503s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:580s
fi-kbl-7567u     total:289  pass:265  dwarn:4   dfail:0   fail:0   skip:20  time:492s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:589s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:657s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:477s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:534s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:523s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:476s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:584s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:432s

e7187e87bb3b5ca43194f86d096b9cf974cc54d7 drm-tip: 2017y-10m-03d-16h-09m-59s UTC integration manifest
075e3af7041e drm/i915/guc: Move GuC core definitions into dedicated files
6376585ec9d0 drm/i915/guc: Move GuC submission declarations into dedicated header
17911cbed040 drm/i915/guc: Move GuC log declarations into dedicated header
4d9fdaddcdeb drm/i915/guc: Move Guc early init into own function
410d58f48040 drm/i915/huc: Move HuC declarations into dedicated header
81ea7a4979de drm/i915/uc: Move uC fw helper code into dedicated files
efcc84c34937 drm/i915/uc: Create intel_uc_init_mmio
897166bdaffc drm/i915/uc: Drop unnecessary forward declaration
927791b05913 drm/i915: Make intel_uncore.h header self-contained

== Logs ==

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

Re: [PATCH v3 1/9] drm/i915: Make intel_uncore.h header self-contained

[Michal Wajdeczko]

On Tue, 03 Oct 2017 19:02:42 +0200, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> Quoting Michal Wajdeczko (2017-10-03 17:35:59)
>> We're trying to resolve inter-header dependencies.
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> ---
>>  drivers/gpu/drm/i915/intel_uncore.h | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.h  
>> b/drivers/gpu/drm/i915/intel_uncore.h
>> index 03786f9..66eae2c 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.h
>> +++ b/drivers/gpu/drm/i915/intel_uncore.h
>> @@ -25,6 +25,12 @@
>>  #ifndef __INTEL_UNCORE_H__
>>  #define __INTEL_UNCORE_H__
>>
>> +#include <linux/spinlock.h>
>> +#include <linux/notifier.h>
>> +#include <linux/hrtimer.h>
>> +
>> +#include "i915_reg.h"
>
> Hmm, what do we pull in directly? Something like i915_mmio_offset()?
>
> In my head I have i915_reg.h == unwieldy list of register defines and
> nothing more.

We need this:

typedef struct {
	uint32_t reg;
} i915_reg_t;

>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> Looks sensible but if it's only one small thing for i915_reg.h can you
> add a comment and we can try and split it up.
> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 7/9] drm/i915/guc: Move GuC log declarations into dedicated header

[Michal Wajdeczko]

On Tue, 03 Oct 2017 19:03:58 +0200, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> Quoting Michal Wajdeczko (2017-10-03 17:36:05)
>> We want to keep component specific code in separate files.
>>
>> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_guc_log.c |  1 +
>>  drivers/gpu/drm/i915/intel_guc_log.h | 57  
>> ++++++++++++++++++++++++++++++++++++
>>  drivers/gpu/drm/i915/intel_uc.h      | 26 +---------------
>>  3 files changed, 59 insertions(+), 25 deletions(-)
>>  create mode 100644 drivers/gpu/drm/i915/intel_guc_log.h
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c  
>> b/drivers/gpu/drm/i915/intel_guc_log.c
>> index 6571d96..bd9b02e 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_log.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
>> @@ -23,6 +23,7 @@
>>   */
>>  #include <linux/debugfs.h>
>>  #include <linux/relay.h>
>> +#include "intel_guc_log.h"
>>  #include "i915_drv.h"
>
> Can we keep a newline between <> and "", and aim for alphabetical order.

I'll add newline in next spin, but I'm not sure about alphabetical order.

Note that Joonas wants all foo.c files to start with "foo.h" (after <>)
and then followed by other headers to make dependencies more explicit.

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

✓ Fi.CI.IGT: success for drm/i915: Guc code reorg

[Patchwork]

== Series Details ==

Series: drm/i915: Guc code reorg
URL   : https://patchwork.freedesktop.org/series/31340/
State : success

== Summary ==

Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test perf:
        Subgroup blocking:
                pass       -> FAIL       (shard-hsw) fdo#102252

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

shard-hsw        total:2429 pass:1315 dwarn:7   dfail:0   fail:24  skip:1083 time:10047s

== Logs ==

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

Re: [PATCH v3 7/9] drm/i915/guc: Move GuC log declarations into dedicated header

[Chris Wilson]

Quoting Michal Wajdeczko (2017-10-03 19:28:02)
> On Tue, 03 Oct 2017 19:03:58 +0200, Chris Wilson  
> <chris@chris-wilson.co.uk> wrote:
> 
> > Quoting Michal Wajdeczko (2017-10-03 17:36:05)
> >> We want to keep component specific code in separate files.
> >>
> >> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_guc_log.c |  1 +
> >>  drivers/gpu/drm/i915/intel_guc_log.h | 57  
> >> ++++++++++++++++++++++++++++++++++++
> >>  drivers/gpu/drm/i915/intel_uc.h      | 26 +---------------
> >>  3 files changed, 59 insertions(+), 25 deletions(-)
> >>  create mode 100644 drivers/gpu/drm/i915/intel_guc_log.h
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c  
> >> b/drivers/gpu/drm/i915/intel_guc_log.c
> >> index 6571d96..bd9b02e 100644
> >> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> >> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> >> @@ -23,6 +23,7 @@
> >>   */
> >>  #include <linux/debugfs.h>
> >>  #include <linux/relay.h>
> >> +#include "intel_guc_log.h"
> >>  #include "i915_drv.h"
> >
> > Can we keep a newline between <> and "", and aim for alphabetical order.
> 
> I'll add newline in next spin, but I'm not sure about alphabetical order.
> 
> Note that Joonas wants all foo.c files to start with "foo.h" (after <>)
> and then followed by other headers to make dependencies more explicit.

I'm not fussed, though tbh I am more likely to forget which file I'm in
and so which special case to apply. 2 blocks of alphabetical includes is
simple enough for me to not muck up :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 4/9] drm/i915/uc: Move uC fw helper code into dedicated files

[Joonas Lahtinen]

On Tue, 2017-10-03 at 16:36 +0000, Michal Wajdeczko wrote:
> This is a prerequisite to unblock next steps.
> 
> v2: correct include order (Joonas)
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>

s/i915_fetch_uc_fw/intel_uc_fw_init/

And this is;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Can you then re-submit these first four patches for CI results and I'll
merge them.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 6/9] drm/i915/guc: Move Guc early init into own function

[Joonas Lahtinen]

On Tue, 2017-10-03 at 16:36 +0000, Michal Wajdeczko wrote:
> We don't want to make aggregate uc functions to be too detailed.
> This will also make future patch easier.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 4/9] drm/i915/uc: Move uC fw helper code into dedicated files

[Michal Wajdeczko]

On Wed, 04 Oct 2017 16:23:50 +0200, Joonas Lahtinen  
<joonas.lahtinen@linux.intel.com> wrote:

> On Tue, 2017-10-03 at 16:36 +0000, Michal Wajdeczko wrote:
>> This is a prerequisite to unblock next steps.
>>
>> v2: correct include order (Joonas)
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>
> s/i915_fetch_uc_fw/intel_uc_fw_init/

Actually, "fetch_uc" function is not symmetrical to "fw_init" so
above rename will be misleading. I'm planning to add below init
function after we are done with all moves:

intel_uc_fw_init(struct intel_uc_fw *fw, enum intel_uc_fw_type type);

Given above, can I still keep your r-b?

Michal

>
> And this is;
>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>
> Can you then re-submit these first four patches for CI results and I'll
> merge them.
>
> Regards, Joonas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: ✓ Fi.CI.IGT: success for drm/i915: Guc code reorg

[Joonas Lahtinen]

On Tue, 2017-10-03 at 18:33 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: drm/i915: Guc code reorg
> URL   : https://patchwork.freedesktop.org/series/31340/
> State : success

I've merged the series, thanks for the patches and review.

Regards, Joonas
-- 
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 7/9] drm/i915/guc: Move GuC log declarations into dedicated header

[Chris Wilson]

Quoting Michal Wajdeczko (2017-10-03 19:28:02)
> On Tue, 03 Oct 2017 19:03:58 +0200, Chris Wilson  
> <chris@chris-wilson.co.uk> wrote:
> 
> > Quoting Michal Wajdeczko (2017-10-03 17:36:05)
> >> We want to keep component specific code in separate files.
> >>
> >> Suggested-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> >> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> >> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_guc_log.c |  1 +
> >>  drivers/gpu/drm/i915/intel_guc_log.h | 57  
> >> ++++++++++++++++++++++++++++++++++++
> >>  drivers/gpu/drm/i915/intel_uc.h      | 26 +---------------
> >>  3 files changed, 59 insertions(+), 25 deletions(-)
> >>  create mode 100644 drivers/gpu/drm/i915/intel_guc_log.h
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c  
> >> b/drivers/gpu/drm/i915/intel_guc_log.c
> >> index 6571d96..bd9b02e 100644
> >> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> >> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> >> @@ -23,6 +23,7 @@
> >>   */
> >>  #include <linux/debugfs.h>
> >>  #include <linux/relay.h>
> >> +#include "intel_guc_log.h"
> >>  #include "i915_drv.h"
> >
> > Can we keep a newline between <> and "", and aim for alphabetical order.
> 
> I'll add newline in next spin, but I'm not sure about alphabetical order.
> 
> Note that Joonas wants all foo.c files to start with "foo.h" (after <>)
> and then followed by other headers to make dependencies more explicit.

Anyway, the change is trivial and clearly correct, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 8/9] drm/i915/guc: Move GuC submission declarations into dedicated header

[Chris Wilson]

Quoting Michal Wajdeczko (2017-10-03 17:36:06)
> We want to keep uC specific code in separate files.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: MichaĹ Winiarski <michal.winiarski@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c        |  1 +
>  drivers/gpu/drm/i915/i915_guc_submission.c |  7 +--
>  drivers/gpu/drm/i915/i915_guc_submission.h | 79 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_uc.c            |  1 +
>  drivers/gpu/drm/i915/intel_uc.h            | 45 -----------------
>  5 files changed, 85 insertions(+), 48 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/i915_guc_submission.h
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index b4a6ac6..44aae25 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -30,6 +30,7 @@
>  #include <linux/sort.h>
>  #include <linux/sched/mm.h>
>  #include "intel_drv.h"
> +#include "i915_guc_submission.h"
>  
>  static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
>  {
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> index 04f1281..97dfe96 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -21,12 +21,13 @@
>   * IN THE SOFTWARE.
>   *
>   */
> -#include <linux/circ_buf.h>
> -#include "i915_drv.h"
> -#include "intel_uc.h"
>  
> +#include <linux/circ_buf.h>
>  #include <trace/events/dma_fence.h>
>  
> +#include "i915_guc_submission.h"
> +#include "i915_drv.h"
> +
>  /**
>   * DOC: GuC-based command submission
>   *
> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.h b/drivers/gpu/drm/i915/i915_guc_submission.h
> new file mode 100644
> index 0000000..b43af42
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.h
> @@ -0,0 +1,79 @@
> +/*
> + * Copyright © 2014-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 _I915_GUC_SUBMISSION_H_
> +#define _I915_GUC_SUBMISSION_H_
> +
> +#include <linux/spinlock.h>

\n

> +#include "i915_gem.h"
> +
> +struct drm_i915_private;
> +
> +/*
> + * 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;
> +
> +       /* bitmap of (host) engine ids */
> +       uint32_t engines;
> +       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];
> +};
> +
> +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);
> +
> +#endif
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index b1bd4d3..2ccf39c 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 "i915_guc_submission.h"
>  #include <linux/firmware.h>

Pencil in a fix for the ordering here.

The mechanical changes look mechanical, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 9/9] drm/i915/guc: Move GuC core definitions into dedicated files

[Chris Wilson]

Quoting Michal Wajdeczko (2017-10-03 17:36:07)
> We want to keep GuC specific code in separated files.
> 
> v2: move all functions in single patch (Joonas)
>     fix old checkpatch issues (Sagar)
> 
> v3: rebased
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> #1
> ---
>  drivers/gpu/drm/i915/Makefile              |   1 +
>  drivers/gpu/drm/i915/i915_guc_submission.c |  94 ----------
>  drivers/gpu/drm/i915/intel_guc.c           | 264 +++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_guc.h           | 110 ++++++++++++
>  drivers/gpu/drm/i915/intel_uc.c            | 146 +---------------
>  drivers/gpu/drm/i915/intel_uc.h            |  78 +--------
>  6 files changed, 378 insertions(+), 315 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 4850f26..51d0d29 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -60,6 +60,7 @@ i915-y += i915_cmd_parser.o \
>  # general-purpose microcontroller (GuC) support
>  i915-y += intel_uc.o \
>           intel_uc_fw.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 97dfe96..7460ab4 100644
> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> @@ -644,48 +644,6 @@ static void i915_guc_irq_handler(unsigned long data)
>   * path of i915_guc_submit() above.
>   */
>  
> -/**
> - * intel_guc_allocate_vma() - Allocate a GGTT VMA for GuC usage
> - * @guc:       the guc
> - * @size:      size of area to allocate (both virtual space and memory)
> - *
> - * This is a wrapper to create an object for use with the GuC. In order to
> - * use it inside the GuC, an object needs to be pinned lifetime, so we allocate
> - * both some backing storage and a range inside the Global GTT. We must pin
> - * it in the GGTT somewhere other than than [0, GUC_WOPCM_TOP) because that
> - * range is reserved inside GuC.
> - *
> - * Return:     A i915_vma if successful, otherwise an ERR_PTR.
> - */
> -struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
> -{
> -       struct drm_i915_private *dev_priv = guc_to_i915(guc);
> -       struct drm_i915_gem_object *obj;
> -       struct i915_vma *vma;
> -       int ret;
> -
> -       obj = i915_gem_object_create(dev_priv, size);
> -       if (IS_ERR(obj))
> -               return ERR_CAST(obj);
> -
> -       vma = i915_vma_instance(obj, &dev_priv->ggtt.base, NULL);
> -       if (IS_ERR(vma))
> -               goto err;
> -
> -       ret = i915_vma_pin(vma, 0, PAGE_SIZE,
> -                          PIN_GLOBAL | PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
> -       if (ret) {
> -               vma = ERR_PTR(ret);
> -               goto err;
> -       }
> -
> -       return vma;
> -
> -err:
> -       i915_gem_object_put(obj);
> -       return vma;
> -}
> -
>  /* Check that a doorbell register is in the expected state */
>  static bool doorbell_ok(struct intel_guc *guc, u16 db_id)
>  {
> @@ -1213,55 +1171,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_modparams.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
> new file mode 100644
> index 0000000..bbe4c32
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -0,0 +1,264 @@
> +/*
> + * Copyright © 2014-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 "intel_guc.h"
> +#include "i915_drv.h"

Hmm, if some of these functions aren't in intel_uncore.h they should be.

> +
> +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);
> +}
> +
> +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;
> +}
> +
> +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;
> +}
> +
> +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.
> + */
> +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);
> +       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));
> +}
> +
> +/**
> + * intel_guc_auth_huc() - Send action to GuC to authenticate HuC ucode
> + * @guc: intel_guc structure
> + * @rsa_offset: rsa offset w.r.t ggtt base of huc vma
> + *
> + * Triggers a HuC firmware authentication request to the GuC via intel_guc_send
> + * INTEL_GUC_ACTION_AUTHENTICATE_HUC interface. This function is invoked by
> + * intel_huc_auth().
> + *
> + * Return:     non-zero code on error
> + */
> +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));
> +}
> +
> +/**
> + * 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_modparams.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));
> +}
> +
> +/**
> + * intel_guc_allocate_vma() - Allocate a GGTT VMA for GuC usage
> + * @guc:       the guc
> + * @size:      size of area to allocate (both virtual space and memory)
> + *
> + * This is a wrapper to create an object for use with the GuC. In order to
> + * use it inside the GuC, an object needs to be pinned lifetime, so we allocate
> + * both some backing storage and a range inside the Global GTT. We must pin
> + * it in the GGTT somewhere other than than [0, GUC_WOPCM_TOP) because that
> + * range is reserved inside GuC.
> + *
> + * Return:     A i915_vma if successful, otherwise an ERR_PTR.
> + */
> +struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size)
> +{
> +       struct drm_i915_private *dev_priv = guc_to_i915(guc);
> +       struct drm_i915_gem_object *obj;
> +       struct i915_vma *vma;
> +       int ret;
> +
> +       obj = i915_gem_object_create(dev_priv, size);
> +       if (IS_ERR(obj))
> +               return ERR_CAST(obj);
> +
> +       vma = i915_vma_instance(obj, &dev_priv->ggtt.base, NULL);
> +       if (IS_ERR(vma))
> +               goto err;
> +
> +       ret = i915_vma_pin(vma, 0, PAGE_SIZE,
> +                          PIN_GLOBAL | PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
> +       if (ret) {
> +               vma = ERR_PTR(ret);
> +               goto err;
> +       }
> +
> +       return vma;
> +
> +err:
> +       i915_gem_object_put(obj);
> +       return vma;
> +}
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> new file mode 100644
> index 0000000..bd5a223
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -0,0 +1,110 @@
> +/*
> + * Copyright © 2014-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_
> +
> +#include "intel_uncore.h"
> +#include "intel_guc_fwif.h"
> +#include "intel_guc_ct.h"
> +#include "intel_guc_log.h"
> +#include "intel_uc_fw.h"
> +#include "i915_guc_reg.h"
> +#include "i915_vma.h"

Hmm. Are they required for the header? Or for the C body?

> +
> +struct intel_guc {
> +       struct intel_uc_fw fw;
> +       struct intel_guc_log log;
> +       struct intel_guc_ct ct;

Most of the embedded types ^^ will not require all of these headers.

> +
> +       /* 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);

But we do need <linux/bitmap.h>

> +       uint32_t db_cacheline;          /* Cyclic counter mod pagesize  */

Too soon to s/uint32_t/u32/ ?

> +
> +       /* 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)

Unusual line splitting.

> +{
> +       return guc->send(guc, action, len);
> +}
> +
> +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);

So here we required i915_vma.h, ok.

It's pretty much a carbon copy, so
Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

However, there's still plenty to polish.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH v3 9/9] drm/i915/guc: Move GuC core definitions into dedicated files

[Michal Wajdeczko]

On Wed, 04 Oct 2017 19:17:21 +0200, Chris Wilson  
<chris@chris-wilson.co.uk> wrote:

> Quoting Michal Wajdeczko (2017-10-03 17:36:07)
>> We want to keep GuC specific code in separated files.
>>
>> v2: move all functions in single patch (Joonas)
>>     fix old checkpatch issues (Sagar)
>>
>> v3: rebased
>>
>> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com> #1
>> ---

<snip>

>> +#ifndef _INTEL_GUC_H_
>> +#define _INTEL_GUC_H_
>> +
>> +#include "intel_uncore.h"
>> +#include "intel_guc_fwif.h"
>> +#include "intel_guc_ct.h"
>> +#include "intel_guc_log.h"
>> +#include "intel_uc_fw.h"
>> +#include "i915_guc_reg.h"
>> +#include "i915_vma.h"
>
> Hmm. Are they required for the header? Or for the C body?

For the header.

#include "intel_uncore.h" --> enum forcewake_domains
#include "intel_guc_fwif.h" --> GUC_NUM_DOORBELLS
#include "intel_guc_ct.h" --> struct intel_guc_ct
#include "intel_guc_log.h" --> struct intel_guc_log
#include "intel_uc_fw.h" --> struct intel_uc_fw
#include "i915_guc_reg.h" --> GUC_WOPCM_TOP
#include "i915_vma.h" --> i915_ggtt_offset

>
>> +
>> +struct intel_guc {
>> +       struct intel_uc_fw fw;
>> +       struct intel_guc_log log;
>> +       struct intel_guc_ct ct;
>
> Most of the embedded types ^^ will not require all of these headers.

Their requirements are handled in their own .h

>
>> +
>> +       /* 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);
>
> But we do need <linux/bitmap.h>

No, only <linux/types.h>

https://cgit.freedesktop.org/drm-tip/tree/include/linux/types.h#n9

>
>> +       uint32_t db_cacheline;          /* Cyclic counter mod pagesize   
>> */
>
> Too soon to s/uint32_t/u32/ ?

Yes.

>
>> +
>> +       /* 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)
>
> Unusual line splitting.

Will fix.

>
>> +{
>> +       return guc->send(guc, action, len);
>> +}
>> +
>> +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);
>
> So here we required i915_vma.h, ok.
>
> It's pretty much a carbon copy, so
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
>
> However, there's still plenty to polish.
> -Chris

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