[PATCH v2 1/2] drm/i915/uc: Move uC related types into dedicated header

[PATCH v2 1/2] drm/i915/uc: Move uC related types into dedicated header

[Michal Wajdeczko]

In old header structure we were mixing type definitions and
declarations that prevent us from exposing some functions
as inline. Lets try to fix that.

v2: keep old order of the structs, pull fwif.h (Sagar)

Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   |   1 +
 drivers/gpu/drm/i915/i915_drv.h       |   2 +-
 drivers/gpu/drm/i915/i915_gem.c       |   1 +
 drivers/gpu/drm/i915/intel_guc_ct.c   |   2 +-
 drivers/gpu/drm/i915/intel_guc_ct.h   |   3 -
 drivers/gpu/drm/i915/intel_guc_log.c  |   1 +
 drivers/gpu/drm/i915/intel_uc.h       | 178 +++-------------------------------
 drivers/gpu/drm/i915/intel_uc_types.h | 171 ++++++++++++++++++++++++++++++++
 8 files changed, 192 insertions(+), 167 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_uc_types.h

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index b4a6ac6..8f85add 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 "intel_uc.h"
 
 static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
 {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b7cba89..521af91 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -58,7 +58,6 @@
 #include "intel_uncore.h"
 #include "intel_bios.h"
 #include "intel_dpll_mgr.h"
-#include "intel_uc.h"
 #include "intel_lrc.h"
 #include "intel_ringbuffer.h"
 
@@ -73,6 +72,7 @@
 
 #include "i915_vma.h"
 
+#include "intel_uc_types.h"
 #include "intel_gvt.h"
 
 /* General customization:
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 73eeb6b..fa70b7c 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -35,6 +35,7 @@
 #include "intel_drv.h"
 #include "intel_frontbuffer.h"
 #include "intel_mocs.h"
+#include "intel_uc.h"
 #include <linux/dma-fence-array.h>
 #include <linux/kthread.h>
 #include <linux/reservation.h>
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
index c4cbec1..722e671 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/intel_guc_ct.c
@@ -22,7 +22,7 @@
  */
 
 #include "i915_drv.h"
-#include "intel_guc_ct.h"
+#include "intel_uc.h"
 
 enum { CTB_SEND = 0, CTB_RECV = 1 };
 
diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
index 6d97f36..49c8d94 100644
--- a/drivers/gpu/drm/i915/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/intel_guc_ct.h
@@ -25,9 +25,6 @@
 #define _INTEL_GUC_CT_H_
 
 struct intel_guc;
-struct i915_vma;
-
-#include "intel_guc_fwif.h"
 
 /**
  * DOC: Command Transport (CT).
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 6571d96..024e3b1 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -24,6 +24,7 @@
 #include <linux/debugfs.h>
 #include <linux/relay.h>
 #include "i915_drv.h"
+#include "intel_uc.h"
 
 static void guc_log_capture_logs(struct intel_guc *guc);
 
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 6966349..296c52d 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -24,62 +24,8 @@
 #ifndef _INTEL_UC_H_
 #define _INTEL_UC_H_
 
-#include "intel_guc_fwif.h"
-#include "i915_guc_reg.h"
-#include "intel_ringbuffer.h"
-#include "intel_guc_ct.h"
-#include "i915_vma.h"
+#include "intel_uc_types.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
- * the term "client" to avoid confusion with hardware contexts. This
- * GEM object is held for the entire lifetime of our interaction with
- * the GuC, being allocated before the GuC is loaded with its firmware.
- * Because there's no way to update the address used by the GuC after
- * initialisation, the shared object must stay pinned into the GGTT as
- * long as the GuC is in use. We also keep the first page (only) mapped
- * into kernel address space, as it includes shared data that must be
- * updated on every request submission.
- *
- * The single GEM object described here is actually made up of several
- * separate areas, as far as the GuC is concerned. The first page (kept
- * kmap'd) includes the "process descriptor" which holds sequence data for
- * the doorbell, and one cacheline which actually *is* the doorbell; a
- * write to this will "ring the doorbell" (i.e. send an interrupt to the
- * GuC). The subsequent  pages of the client object constitute the work
- * queue (a circular array of work items), again described in the process
- * descriptor. Work queue pages are mapped momentarily as required.
- */
-struct i915_guc_client {
-	struct i915_vma *vma;
-	void *vaddr;
-	struct i915_gem_context *owner;
-	struct intel_guc *guc;
-
-	uint32_t engines;		/* bitmap of (host) engine ids	*/
-	uint32_t priority;
-	u32 stage_id;
-	uint32_t proc_desc_offset;
-
-	u16 doorbell_id;
-	unsigned long doorbell_offset;
-
-	spinlock_t wq_lock;
-	/* Per-engine counts of GuC submissions */
-	uint64_t submissions[I915_NUM_ENGINES];
-};
-
-enum intel_uc_fw_status {
-	INTEL_UC_FIRMWARE_FAIL = -1,
-	INTEL_UC_FIRMWARE_NONE = 0,
-	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)
 {
@@ -96,12 +42,6 @@ const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
 	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) {
@@ -113,93 +53,25 @@ static inline const char *intel_uc_fw_type_repr(enum intel_uc_fw_type type)
 	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;
-	/* The runtime stuff gets created only when GuC logging gets enabled */
-	struct {
-		void *buf_addr;
-		struct workqueue_struct *flush_wq;
-		struct work_struct flush_work;
-		struct rchan *relay_chan;
-	} runtime;
-	/* logging related stats */
-	u32 capture_miss_count;
-	u32 flush_interrupt_count;
-	u32 prev_overflow_count[GUC_MAX_LOG_BUFFER];
-	u32 total_overflow_count[GUC_MAX_LOG_BUFFER];
-	u32 flush_count[GUC_MAX_LOG_BUFFER];
-};
-
-struct intel_guc {
-	struct intel_uc_fw fw;
-	struct intel_guc_log log;
-	struct intel_guc_ct ct;
-
-	/* Log snapshot if GuC errors during load */
-	struct drm_i915_gem_object *load_err_log;
-
-	/* intel_guc_recv interrupt related state */
-	bool interrupts_enabled;
-
-	struct i915_vma *ads_vma;
-	struct i915_vma *stage_desc_pool;
-	void *stage_desc_pool_vaddr;
-	struct ida stage_ids;
-
-	struct i915_guc_client *execbuf_client;
-
-	DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
-	uint32_t db_cacheline;		/* Cyclic counter mod pagesize	*/
-
-	/* GuC's FW specific registers used in MMIO send */
-	struct {
-		u32 base;
-		unsigned int count;
-		enum forcewake_domains fw_domains;
-	} send_regs;
-
-	/* To serialize the intel_guc_send actions */
-	struct mutex send_mutex;
+static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
+{
+	return guc->send(guc, action, len);
+}
 
-	/* GuC's FW specific send function */
-	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
+static inline void intel_guc_notify(struct intel_guc *guc)
+{
+	guc->notify(guc);
+}
 
-	/* GuC's FW specific notify function */
-	void (*notify)(struct intel_guc *guc);
-};
+static inline u32 guc_ggtt_offset(struct i915_vma *vma)
+{
+	u32 offset = i915_ggtt_offset(vma);
 
-struct intel_huc {
-	/* Generic uC firmware management */
-	struct intel_uc_fw fw;
+	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
+	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
 
-	/* HuC-specific additions */
-};
+	return offset;
+}
 
 /* intel_uc.c */
 void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
@@ -213,16 +85,6 @@ 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);
@@ -244,14 +106,6 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
 void i915_guc_log_register(struct drm_i915_private *dev_priv);
 void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
 
-static inline u32 guc_ggtt_offset(struct i915_vma *vma)
-{
-	u32 offset = i915_ggtt_offset(vma);
-	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
-	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
-	return offset;
-}
-
 /* intel_huc.c */
 void intel_huc_select_fw(struct intel_huc *huc);
 void intel_huc_init_hw(struct intel_huc *huc);
diff --git a/drivers/gpu/drm/i915/intel_uc_types.h b/drivers/gpu/drm/i915/intel_uc_types.h
new file mode 100644
index 0000000..8f8bf4e
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_uc_types.h
@@ -0,0 +1,171 @@
+/*
+ * 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_TYPES_H_
+#define _INTEL_UC_TYPES_H_
+
+#include "intel_guc_fwif.h"
+#include "intel_guc_ct.h"
+#include "i915_guc_reg.h"
+
+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 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;
+	/* 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];
+};
+
+/*
+ * 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;
+	struct intel_guc_ct ct;
+
+	/* Log snapshot if GuC errors during load */
+	struct drm_i915_gem_object *load_err_log;
+
+	/* intel_guc_recv interrupt related state */
+	bool interrupts_enabled;
+
+	struct i915_vma *ads_vma;
+	struct i915_vma *stage_desc_pool;
+	void *stage_desc_pool_vaddr;
+	struct ida stage_ids;
+
+	struct i915_guc_client *execbuf_client;
+
+	DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
+	uint32_t db_cacheline;		/* Cyclic counter mod pagesize	*/
+
+	/* GuC's FW specific registers used in MMIO send */
+	struct {
+		u32 base;
+		unsigned int count;
+		enum forcewake_domains fw_domains;
+	} send_regs;
+
+	/* To serialize the intel_guc_send actions */
+	struct mutex send_mutex;
+
+	/* GuC's FW specific send function */
+	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
+
+	/* GuC's FW specific notify function */
+	void (*notify)(struct intel_guc *guc);
+};
+
+struct intel_huc {
+	/* Generic uC firmware management */
+	struct intel_uc_fw fw;
+
+	/* HuC-specific additions */
+};
+
+#endif
-- 
2.7.4

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

[PATCH v2 2/2] drm/i915: Make intel_guc_wopcm_size() inline

[Michal Wajdeczko]

While here fix param and var types to match new naming rules.

v2: update names and commit message (Sagar)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_loader.c | 11 -----------
 drivers/gpu/drm/i915/intel_uc.c         |  4 ++--
 drivers/gpu/drm/i915/intel_uc.h         | 13 ++++++++++++-
 3 files changed, 14 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index c9e25be..387d105a 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -250,17 +250,6 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
 	return ret;
 }
 
-u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv)
-{
-	u32 wopcm_size = GUC_WOPCM_TOP;
-
-	/* On BXT, the top of WOPCM is reserved for RC6 context */
-	if (IS_GEN9_LP(dev_priv))
-		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
-
-	return wopcm_size;
-}
-
 /*
  * Load the GuC firmware blob into the MinuteIA.
  */
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 2774778..1ef2de6 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -188,7 +188,7 @@ static void fetch_uc_fw(struct drm_i915_private *dev_priv,
 		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)) {
+		if (size > intel_guc_wopcm_size(&dev_priv->guc)) {
 			DRM_ERROR("Firmware is too large to fit in WOPCM\n");
 			goto fail;
 		}
@@ -374,7 +374,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	}
 
 	/* init WOPCM */
-	I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(dev_priv));
+	I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(guc));
 	I915_WRITE(DMA_GUC_WOPCM_OFFSET,
 		   GUC_WOPCM_OFFSET_VALUE | HUC_LOADING_AGENT_GUC);
 
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 296c52d..f43bb43 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -63,6 +63,18 @@ static inline void intel_guc_notify(struct intel_guc *guc)
 	guc->notify(guc);
 }
 
+static inline u32 intel_guc_wopcm_size(struct intel_guc *guc)
+{
+	struct drm_i915_private *i915 = guc_to_i915(guc);
+	u32 wopcm_size = GUC_WOPCM_TOP;
+
+	/* On BXT, the top of WOPCM is reserved for RC6 context */
+	if (IS_GEN9_LP(i915))
+		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
+
+	return wopcm_size;
+}
+
 static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 {
 	u32 offset = i915_ggtt_offset(vma);
@@ -90,7 +102,6 @@ int intel_guc_select_fw(struct intel_guc *guc);
 int intel_guc_init_hw(struct intel_guc *guc);
 int intel_guc_suspend(struct drm_i915_private *dev_priv);
 int intel_guc_resume(struct drm_i915_private *dev_priv);
-u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
 
 /* i915_guc_submission.c */
 int i915_guc_submission_init(struct drm_i915_private *dev_priv);
-- 
2.7.4

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

Re: [PATCH v2 1/2] drm/i915/uc: Move uC related types into dedicated header

[Joonas Lahtinen]

On Wed, 2017-09-27 at 11:22 +0000, Michal Wajdeczko wrote:
> In old header structure we were mixing type definitions and
> declarations that prevent us from exposing some functions
> as inline. Lets try to fix that.
> 
> v2: keep old order of the structs, pull fwif.h (Sagar)
> 
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Like I mentioned in the other patch, lets make future ourselves life
easier and take this cleanup further;

intel_uc.{c,h}
intel_guc.{c,h} .h includes "intel_uc.h"
intel_huc.{c,h} ditto

intel_guc_ct.h includes "intel_guc.h"

That way we're not driving ourselves towards the spaghetti insanity
that i915_drv.h is currently. Aka. move a declaration, spend hours
fixing the ordering of things.

Regards, Joonas

PS. Free beer offered for the one to first remove all "filename.c"
sections from i915_drv.h
-- 
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

✓ Fi.CI.BAT: success for series starting with [v2,1/2] drm/i915/uc: Move uC related types into dedicated header

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/uc: Move uC related types into dedicated header
URL   : https://patchwork.freedesktop.org/series/30957/
State : success

== Summary ==

Series 30957v1 series starting with [v2,1/2] drm/i915/uc: Move uC related types into dedicated header
https://patchwork.freedesktop.org/api/1.0/series/30957/revisions/1/mbox/

Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-c:
                pass       -> INCOMPLETE (fi-kbl-7560u) fdo#102846
Test pm_rpm:
        Subgroup basic-rte:
                pass       -> DMESG-WARN (fi-cfl-s) fdo#102294
Test drv_module_reload:
        Subgroup basic-no-display:
                dmesg-warn -> PASS       (fi-glk-1) fdo#102777 +1

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

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:440s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:469s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:420s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:517s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:500s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:499s
fi-cfl-s         total:289  pass:222  dwarn:35  dfail:0   fail:0   skip:32  time:548s
fi-cnl-y         total:289  pass:258  dwarn:0   dfail:0   fail:4   skip:27  time:648s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:425s
fi-glk-1         total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:563s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:428s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:406s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:430s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:487s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:468s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:471s
fi-kbl-7560u     total:247  pass:230  dwarn:0   dfail:0   fail:0   skip:16 
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:587s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:547s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:445s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:756s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:489s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:472s
fi-snb-2600      total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:417s
fi-snb-2520m failed to connect after reboot

da14aaa14e584a49e94bffd3c7af6e659745bae6 drm-tip: 2017y-09m-27d-09h-50m-55s UTC integration manifest
e2176e717363 drm/i915: Make intel_guc_wopcm_size() inline
9a669b51a4a0 drm/i915/uc: Move uC related types into dedicated header

== Logs ==

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

Re: [PATCH v2 2/2] drm/i915: Make intel_guc_wopcm_size() inline

[Sagar Arun Kamble]

[-- Attachment #1.1: Type: text/plain, Size: 3363 bytes --]


On 9/27/2017 4:52 PM, Michal Wajdeczko wrote:
> While here fix param and var types to match new naming rules.
>
> v2: update names and commit message (Sagar)
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc_loader.c | 11 -----------
>   drivers/gpu/drm/i915/intel_uc.c         |  4 ++--
>   drivers/gpu/drm/i915/intel_uc.h         | 13 ++++++++++++-
>   3 files changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
> index c9e25be..387d105a 100644
> --- a/drivers/gpu/drm/i915/intel_guc_loader.c
> +++ b/drivers/gpu/drm/i915/intel_guc_loader.c
> @@ -250,17 +250,6 @@ static int guc_ucode_xfer_dma(struct drm_i915_private *dev_priv,
>   	return ret;
>   }
>   
> -u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv)
> -{
> -	u32 wopcm_size = GUC_WOPCM_TOP;
> -
> -	/* On BXT, the top of WOPCM is reserved for RC6 context */
> -	if (IS_GEN9_LP(dev_priv))
> -		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
> -
> -	return wopcm_size;
> -}
> -
>   /*
>    * Load the GuC firmware blob into the MinuteIA.
>    */
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 2774778..1ef2de6 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -188,7 +188,7 @@ static void fetch_uc_fw(struct drm_i915_private *dev_priv,
>   		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)) {
> +		if (size > intel_guc_wopcm_size(&dev_priv->guc)) {
>   			DRM_ERROR("Firmware is too large to fit in WOPCM\n");
>   			goto fail;
>   		}
> @@ -374,7 +374,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
>   	}
>   
>   	/* init WOPCM */
> -	I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(dev_priv));
> +	I915_WRITE(GUC_WOPCM_SIZE, intel_guc_wopcm_size(guc));
>   	I915_WRITE(DMA_GUC_WOPCM_OFFSET,
>   		   GUC_WOPCM_OFFSET_VALUE | HUC_LOADING_AGENT_GUC);
>   
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 296c52d..f43bb43 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -63,6 +63,18 @@ static inline void intel_guc_notify(struct intel_guc *guc)
>   	guc->notify(guc);
>   }
>   
> +static inline u32 intel_guc_wopcm_size(struct intel_guc *guc)
> +{
> +	struct drm_i915_private *i915 = guc_to_i915(guc);
> +	u32 wopcm_size = GUC_WOPCM_TOP;
> +
> +	/* On BXT, the top of WOPCM is reserved for RC6 context */
> +	if (IS_GEN9_LP(i915))
> +		wopcm_size -= BXT_GUC_WOPCM_RC6_RESERVED;
> +
> +	return wopcm_size;
> +}
> +
>   static inline u32 guc_ggtt_offset(struct i915_vma *vma)
>   {
>   	u32 offset = i915_ggtt_offset(vma);
> @@ -90,7 +102,6 @@ int intel_guc_select_fw(struct intel_guc *guc);
>   int intel_guc_init_hw(struct intel_guc *guc);
>   int intel_guc_suspend(struct drm_i915_private *dev_priv);
>   int intel_guc_resume(struct drm_i915_private *dev_priv);
> -u32 intel_guc_wopcm_size(struct drm_i915_private *dev_priv);
>   
>   /* i915_guc_submission.c */
>   int i915_guc_submission_init(struct drm_i915_private *dev_priv);


[-- Attachment #1.2: Type: text/html, Size: 3988 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

Re: [PATCH v2 1/2] drm/i915/uc: Move uC related types into dedicated header

[Sagar Arun Kamble]

[-- Attachment #1.1: Type: text/plain, Size: 17198 bytes --]



On 9/27/2017 4:52 PM, Michal Wajdeczko wrote:
> In old header structure we were mixing type definitions and
> declarations that prevent us from exposing some functions
> as inline. Lets try to fix that.
>
> v2: keep old order of the structs, pull fwif.h (Sagar)
>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c   |   1 +
>   drivers/gpu/drm/i915/i915_drv.h       |   2 +-
>   drivers/gpu/drm/i915/i915_gem.c       |   1 +
>   drivers/gpu/drm/i915/intel_guc_ct.c   |   2 +-
>   drivers/gpu/drm/i915/intel_guc_ct.h   |   3 -
>   drivers/gpu/drm/i915/intel_guc_log.c  |   1 +
>   drivers/gpu/drm/i915/intel_uc.h       | 178 +++-------------------------------
>   drivers/gpu/drm/i915/intel_uc_types.h | 171 ++++++++++++++++++++++++++++++++
>   8 files changed, 192 insertions(+), 167 deletions(-)
>   create mode 100644 drivers/gpu/drm/i915/intel_uc_types.h
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index b4a6ac6..8f85add 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 "intel_uc.h"
>   
>   static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
>   {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b7cba89..521af91 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -58,7 +58,6 @@
>   #include "intel_uncore.h"
>   #include "intel_bios.h"
>   #include "intel_dpll_mgr.h"
> -#include "intel_uc.h"
>   #include "intel_lrc.h"
>   #include "intel_ringbuffer.h"
>   
> @@ -73,6 +72,7 @@
>   
>   #include "i915_vma.h"
>   
> +#include "intel_uc_types.h"
>   #include "intel_gvt.h"
>   
>   /* General customization:
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 73eeb6b..fa70b7c 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -35,6 +35,7 @@
>   #include "intel_drv.h"
>   #include "intel_frontbuffer.h"
>   #include "intel_mocs.h"
> +#include "intel_uc.h"
>   #include <linux/dma-fence-array.h>
>   #include <linux/kthread.h>
>   #include <linux/reservation.h>
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
> index c4cbec1..722e671 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -22,7 +22,7 @@
>    */
>   
>   #include "i915_drv.h"
> -#include "intel_guc_ct.h"
> +#include "intel_uc.h"
>   
>   enum { CTB_SEND = 0, CTB_RECV = 1 };
>   
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
> index 6d97f36..49c8d94 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.h
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.h
> @@ -25,9 +25,6 @@
>   #define _INTEL_GUC_CT_H_
>   
>   struct intel_guc;
> -struct i915_vma;
> -
> -#include "intel_guc_fwif.h"
>   
>   /**
>    * DOC: Command Transport (CT).
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index 6571d96..024e3b1 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -24,6 +24,7 @@
>   #include <linux/debugfs.h>
>   #include <linux/relay.h>
>   #include "i915_drv.h"
> +#include "intel_uc.h"
>   
>   static void guc_log_capture_logs(struct intel_guc *guc);
>   
> diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
> index 6966349..296c52d 100644
> --- a/drivers/gpu/drm/i915/intel_uc.h
> +++ b/drivers/gpu/drm/i915/intel_uc.h
> @@ -24,62 +24,8 @@
>   #ifndef _INTEL_UC_H_
>   #define _INTEL_UC_H_
>   
> -#include "intel_guc_fwif.h"
> -#include "i915_guc_reg.h"
> -#include "intel_ringbuffer.h"
> -#include "intel_guc_ct.h"
> -#include "i915_vma.h"
> +#include "intel_uc_types.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
> - * the term "client" to avoid confusion with hardware contexts. This
> - * GEM object is held for the entire lifetime of our interaction with
> - * the GuC, being allocated before the GuC is loaded with its firmware.
> - * Because there's no way to update the address used by the GuC after
> - * initialisation, the shared object must stay pinned into the GGTT as
> - * long as the GuC is in use. We also keep the first page (only) mapped
> - * into kernel address space, as it includes shared data that must be
> - * updated on every request submission.
> - *
> - * The single GEM object described here is actually made up of several
> - * separate areas, as far as the GuC is concerned. The first page (kept
> - * kmap'd) includes the "process descriptor" which holds sequence data for
> - * the doorbell, and one cacheline which actually *is* the doorbell; a
> - * write to this will "ring the doorbell" (i.e. send an interrupt to the
> - * GuC). The subsequent  pages of the client object constitute the work
> - * queue (a circular array of work items), again described in the process
> - * descriptor. Work queue pages are mapped momentarily as required.
> - */
> -struct i915_guc_client {
> -	struct i915_vma *vma;
> -	void *vaddr;
> -	struct i915_gem_context *owner;
> -	struct intel_guc *guc;
> -
> -	uint32_t engines;		/* bitmap of (host) engine ids	*/
> -	uint32_t priority;
> -	u32 stage_id;
> -	uint32_t proc_desc_offset;
> -
> -	u16 doorbell_id;
> -	unsigned long doorbell_offset;
> -
> -	spinlock_t wq_lock;
> -	/* Per-engine counts of GuC submissions */
> -	uint64_t submissions[I915_NUM_ENGINES];
> -};
> -
> -enum intel_uc_fw_status {
> -	INTEL_UC_FIRMWARE_FAIL = -1,
> -	INTEL_UC_FIRMWARE_NONE = 0,
> -	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)
>   {
> @@ -96,12 +42,6 @@ const char *intel_uc_fw_status_repr(enum intel_uc_fw_status status)
>   	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) {
> @@ -113,93 +53,25 @@ static inline const char *intel_uc_fw_type_repr(enum intel_uc_fw_type type)
>   	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;
> -	/* The runtime stuff gets created only when GuC logging gets enabled */
> -	struct {
> -		void *buf_addr;
> -		struct workqueue_struct *flush_wq;
> -		struct work_struct flush_work;
> -		struct rchan *relay_chan;
> -	} runtime;
> -	/* logging related stats */
> -	u32 capture_miss_count;
> -	u32 flush_interrupt_count;
> -	u32 prev_overflow_count[GUC_MAX_LOG_BUFFER];
> -	u32 total_overflow_count[GUC_MAX_LOG_BUFFER];
> -	u32 flush_count[GUC_MAX_LOG_BUFFER];
> -};
> -
> -struct intel_guc {
> -	struct intel_uc_fw fw;
> -	struct intel_guc_log log;
> -	struct intel_guc_ct ct;
> -
> -	/* Log snapshot if GuC errors during load */
> -	struct drm_i915_gem_object *load_err_log;
> -
> -	/* intel_guc_recv interrupt related state */
> -	bool interrupts_enabled;
> -
> -	struct i915_vma *ads_vma;
> -	struct i915_vma *stage_desc_pool;
> -	void *stage_desc_pool_vaddr;
> -	struct ida stage_ids;
> -
> -	struct i915_guc_client *execbuf_client;
> -
> -	DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
> -	uint32_t db_cacheline;		/* Cyclic counter mod pagesize	*/
> -
> -	/* GuC's FW specific registers used in MMIO send */
> -	struct {
> -		u32 base;
> -		unsigned int count;
> -		enum forcewake_domains fw_domains;
> -	} send_regs;
> -
> -	/* To serialize the intel_guc_send actions */
> -	struct mutex send_mutex;
> +static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
> +{
> +	return guc->send(guc, action, len);
> +}
>   
> -	/* GuC's FW specific send function */
> -	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
> +static inline void intel_guc_notify(struct intel_guc *guc)
> +{
> +	guc->notify(guc);
> +}
>   
> -	/* GuC's FW specific notify function */
> -	void (*notify)(struct intel_guc *guc);
> -};
> +static inline u32 guc_ggtt_offset(struct i915_vma *vma)
> +{
> +	u32 offset = i915_ggtt_offset(vma);
>   
> -struct intel_huc {
> -	/* Generic uC firmware management */
> -	struct intel_uc_fw fw;
> +	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
> +	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
>   
> -	/* HuC-specific additions */
> -};
> +	return offset;
> +}
>   
>   /* intel_uc.c */
>   void intel_uc_sanitize_options(struct drm_i915_private *dev_priv);
> @@ -213,16 +85,6 @@ 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);
> @@ -244,14 +106,6 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
>   void i915_guc_log_register(struct drm_i915_private *dev_priv);
>   void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
>   
> -static inline u32 guc_ggtt_offset(struct i915_vma *vma)
> -{
> -	u32 offset = i915_ggtt_offset(vma);
> -	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
> -	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
> -	return offset;
> -}
> -
>   /* intel_huc.c */
>   void intel_huc_select_fw(struct intel_huc *huc);
>   void intel_huc_init_hw(struct intel_huc *huc);
> diff --git a/drivers/gpu/drm/i915/intel_uc_types.h b/drivers/gpu/drm/i915/intel_uc_types.h
> new file mode 100644
> index 0000000..8f8bf4e
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_uc_types.h
> @@ -0,0 +1,171 @@
> +/*
> + * 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_TYPES_H_
> +#define _INTEL_UC_TYPES_H_
> +
> +#include "intel_guc_fwif.h"
> +#include "intel_guc_ct.h"
> +#include "i915_guc_reg.h"
> +
> +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 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;
> +	/* 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];
> +};
> +
> +/*
> + * 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;
> +	struct intel_guc_ct ct;
> +
> +	/* Log snapshot if GuC errors during load */
> +	struct drm_i915_gem_object *load_err_log;
> +
> +	/* intel_guc_recv interrupt related state */
> +	bool interrupts_enabled;
> +
> +	struct i915_vma *ads_vma;
> +	struct i915_vma *stage_desc_pool;
> +	void *stage_desc_pool_vaddr;
> +	struct ida stage_ids;
> +
> +	struct i915_guc_client *execbuf_client;
> +
> +	DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
> +	uint32_t db_cacheline;		/* Cyclic counter mod pagesize	*/
> +
> +	/* GuC's FW specific registers used in MMIO send */
> +	struct {
> +		u32 base;
> +		unsigned int count;
> +		enum forcewake_domains fw_domains;
> +	} send_regs;
> +
> +	/* To serialize the intel_guc_send actions */
> +	struct mutex send_mutex;
> +
> +	/* GuC's FW specific send function */
> +	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
> +
> +	/* GuC's FW specific notify function */
> +	void (*notify)(struct intel_guc *guc);
> +};
> +
> +struct intel_huc {
> +	/* Generic uC firmware management */
> +	struct intel_uc_fw fw;
> +
> +	/* HuC-specific additions */
> +};
> +
> +#endif


[-- Attachment #1.2: Type: text/html, Size: 17339 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

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

✓ Fi.CI.IGT: success for series starting with [v2,1/2] drm/i915/uc: Move uC related types into dedicated header

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/2] drm/i915/uc: Move uC related types into dedicated header
URL   : https://patchwork.freedesktop.org/series/30957/
State : success

== Summary ==

Test kms_setmode:
        Subgroup basic:
                fail       -> PASS       (shard-hsw) fdo#99912
Test perf:
        Subgroup polling:
                pass       -> FAIL       (shard-hsw) fdo#102252
Test kms_flip:
        Subgroup plain-flip-fb-recreate-interruptible:
                pass       -> FAIL       (shard-hsw) fdo#100368

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

shard-hsw        total:2381 pass:1304 dwarn:1   dfail:0   fail:10  skip:1066 time:9811s

== Logs ==

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

Re: [PATCH v2 2/2] drm/i915: Make intel_guc_wopcm_size() inline

[Joonas Lahtinen]

On Wed, 2017-09-27 at 11:22 +0000, Michal Wajdeczko wrote:
> While here fix param and var types to match new naming rules.
> 
> v2: update names and commit message (Sagar)
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>

Generally, when the function ends up being used only from single .c
file, you may drop the intel_ prefix and just have it as a private
inline.

So in intel_uc.c as "guc_wopcm_size".

We should try to have the related code in same files and avoid having
functions in the .h files to the extent we can. For example here this
is very much an implementation detail.

When moved there with the rename, this is;

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 v2 1/2] drm/i915/uc: Move uC related types into dedicated header

[Joonas Lahtinen]

On Wed, 2017-09-27 at 14:36 +0300, Joonas Lahtinen wrote:
> On Wed, 2017-09-27 at 11:22 +0000, Michal Wajdeczko wrote:
> > In old header structure we were mixing type definitions and
> > declarations that prevent us from exposing some functions
> > as inline. Lets try to fix that.
> > 
> > v2: keep old order of the structs, pull fwif.h (Sagar)
> > 
> > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> > Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

Ping, does this sound like a solid idea to you, Michal?

Regards, Joonas

> Like I mentioned in the other patch, lets make future ourselves life
> easier and take this cleanup further;
> 
> intel_uc.{c,h}
> intel_guc.{c,h} .h includes "intel_uc.h"
> intel_huc.{c,h} ditto
> 
> intel_guc_ct.h includes "intel_guc.h"
> 
> That way we're not driving ourselves towards the spaghetti insanity
> that i915_drv.h is currently. Aka. move a declaration, spend hours
> fixing the ordering of things.
> 
> Regards, Joonas
> 
> PS. Free beer offered for the one to first remove all "filename.c"
> sections from i915_drv.h
-- 
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 v2 1/2] drm/i915/uc: Move uC related types into dedicated header

[Michal Wajdeczko]

On Fri, 29 Sep 2017 12:37:17 +0200, Joonas Lahtinen  
<joonas.lahtinen@linux.intel.com> wrote:

> On Wed, 2017-09-27 at 14:36 +0300, Joonas Lahtinen wrote:
>> On Wed, 2017-09-27 at 11:22 +0000, Michal Wajdeczko wrote:
>> > In old header structure we were mixing type definitions and
>> > declarations that prevent us from exposing some functions
>> > as inline. Lets try to fix that.
>> >
>> > v2: keep old order of the structs, pull fwif.h (Sagar)
>> >
>> > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
>> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> > Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>> > Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> > Acked-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
>
> Ping, does this sound like a solid idea to you, Michal?

Idea is fine (I was the one who vote for intel_huc.c) but this
patch was to solve different problem: we can't define inline
functions in private headers if they depend on INTEL_INFO/GEN
macros as these macros requires drm_i915_private definition
that is defined later in #include chain.

But maybe you're right and we should start to clean our stuff
first and then later focus on inline implementations. @Chris?

Regards,
Michal

>
> Regards, Joonas
>
>> Like I mentioned in the other patch, lets make future ourselves life
>> easier and take this cleanup further;
>>
>> intel_uc.{c,h}
>> intel_guc.{c,h} .h includes "intel_uc.h"
>> intel_huc.{c,h} ditto
>>
>> intel_guc_ct.h includes "intel_guc.h"
>>
>> That way we're not driving ourselves towards the spaghetti insanity
>> that i915_drv.h is currently. Aka. move a declaration, spend hours
>> fixing the ordering of things.
>>
>> Regards, Joonas
>>
>> PS. Free beer offered for the one to first remove all "filename.c"
>> sections from i915_drv.h
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx