[PATCH 1/6] drm/i915: Separate GuC/HuC specific functionality from intel_uc

[PATCH 1/6] drm/i915: Separate GuC/HuC specific functionality from intel_uc

[Sagar Arun Kamble]

From: "Kamble, Sagar A" <sagar.a.kamble@intel.com>

Removed unnecessary intel_uc.h includes as it is present in i915_drv.h.
Created intel_guc.c and intel_guc.h for placing GuC specific code.
Created intel_huc.h to refer to HuC specific functions.

v2: Prepared intel_uc_common.h. Moved enable/disable_communication to
intel_uc.c. huc_auth code declaration adjusted.
Restored static of guc_send_reg and kept along with other send functions
s/guc_init_send_regs/intel_guc_init_send_regs and move the call to
intel_uc_init_hw. Moved intel_guc_sample_forcewake moved to the end
Moved intel_guc_init_early to the top. Moved guc_ggtt_offset after
definition of intel_guc. Changed intel_guc_auth_huc prototype to accept
guc struct and huc firmware struct. (Michal Wajdeczko)

v3: Removed intel_uc_common.h to associate similar includes
for intel_guc.h and intel_huc.h through intel_uc.h itself. Updated
intel_guc.h accordingly. In i915_drv.h now including intel_uc.h,
intel_guc.h, intel_huc.h. Updated commit message. (Michal Winiarski)
Changed definition of intel_guc_auth_huc to pass struct intel_huc in
order to separate the wait for authentication status which is now prepared
as separate function intel_huc_check_auth_status exported from
intel_huc.c. Updated definition of intel_guc_suspend and
intel_guc_resume to take struct intel_guc as parameter and move the
declaration corresponding to those listed for i915_guc_submission.c
declarations. (Michal Wajdeczko)
intel_guc_suspend and intel_guc_resume are moved to intel_guc.c in the
upcoming patches. Not sure if we should move intel_guc_allocate_vma also
there. Currently functions are put in i915_guc_submission.c,
intel_guc_ct.c, intel_guc_loader.c, intel_guc_log.c and intel_guc.c.

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/Makefile              |   1 +
 drivers/gpu/drm/i915/i915_drv.c            |   7 +-
 drivers/gpu/drm/i915/i915_drv.h            |   2 +
 drivers/gpu/drm/i915/i915_gem.c            |   2 +-
 drivers/gpu/drm/i915/i915_guc_submission.c |  11 +-
 drivers/gpu/drm/i915/intel_guc.c           | 183 +++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc.h           | 195 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc_loader.c    |   1 -
 drivers/gpu/drm/i915/intel_huc.c           |  56 +++------
 drivers/gpu/drm/i915/intel_huc.h           |  41 ++++++
 drivers/gpu/drm/i915/intel_uc.c            | 128 +------------------
 drivers/gpu/drm/i915/intel_uc.h            | 170 -------------------------
 12 files changed, 451 insertions(+), 346 deletions(-)
 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_huc.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 1cb8059..e13fc19 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -59,6 +59,7 @@ i915-y += i915_cmd_parser.o \
 
 # general-purpose microcontroller (GuC) support
 i915-y += intel_uc.o \
+	  intel_guc.o \
 	  intel_guc_ct.o \
 	  intel_guc_log.o \
 	  intel_guc_loader.o \
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 5c111ea..b825024 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -50,7 +50,6 @@
 #include "i915_trace.h"
 #include "i915_vgpu.h"
 #include "intel_drv.h"
-#include "intel_uc.h"
 
 static struct drm_driver driver;
 
@@ -1691,7 +1690,7 @@ static int i915_drm_resume(struct drm_device *dev)
 	}
 	mutex_unlock(&dev->struct_mutex);
 
-	intel_guc_resume(dev_priv);
+	intel_guc_resume(&dev_priv->guc);
 
 	intel_modeset_init_hw(dev);
 
@@ -2493,7 +2492,7 @@ static int intel_runtime_suspend(struct device *kdev)
 	 */
 	i915_gem_runtime_suspend(dev_priv);
 
-	intel_guc_suspend(dev_priv);
+	intel_guc_suspend(&dev_priv->guc);
 
 	intel_runtime_pm_disable_interrupts(dev_priv);
 
@@ -2578,7 +2577,7 @@ static int intel_runtime_resume(struct device *kdev)
 	if (intel_uncore_unclaimed_mmio(dev_priv))
 		DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
 
-	intel_guc_resume(dev_priv);
+	intel_guc_resume(&dev_priv->guc);
 
 	if (IS_GEN9_LP(dev_priv)) {
 		bxt_disable_dc9(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d07d110..d0a9c18 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -59,6 +59,8 @@
 #include "intel_bios.h"
 #include "intel_dpll_mgr.h"
 #include "intel_uc.h"
+#include "intel_guc.h"
+#include "intel_huc.h"
 #include "intel_lrc.h"
 #include "intel_ringbuffer.h"
 
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index f445587..eb20e73 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4575,7 +4575,7 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	i915_gem_contexts_lost(dev_priv);
 	mutex_unlock(&dev->struct_mutex);
 
-	intel_guc_suspend(dev_priv);
+	intel_guc_suspend(&dev_priv->guc);
 
 	cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
 	cancel_delayed_work_sync(&dev_priv->gt.retire_work);
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 48a1e93..4b7afba 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -23,7 +23,6 @@
  */
 #include <linux/circ_buf.h>
 #include "i915_drv.h"
-#include "intel_uc.h"
 
 #include <trace/events/dma_fence.h>
 
@@ -1291,11 +1290,10 @@ void i915_guc_submission_disable(struct drm_i915_private *dev_priv)
 
 /**
  * intel_guc_suspend() - notify GuC entering suspend state
- * @dev_priv:	i915 device private
  */
-int intel_guc_suspend(struct drm_i915_private *dev_priv)
+int intel_guc_suspend(struct intel_guc *guc)
 {
-	struct intel_guc *guc = &dev_priv->guc;
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	struct i915_gem_context *ctx;
 	u32 data[3];
 
@@ -1317,11 +1315,10 @@ int intel_guc_suspend(struct drm_i915_private *dev_priv)
 
 /**
  * intel_guc_resume() - notify GuC resuming from suspend state
- * @dev_priv:	i915 device private
  */
-int intel_guc_resume(struct drm_i915_private *dev_priv)
+int intel_guc_resume(struct intel_guc *guc)
 {
-	struct intel_guc *guc = &dev_priv->guc;
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
 	struct i915_gem_context *ctx;
 	u32 data[3];
 
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
new file mode 100644
index 0000000..5559e00
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -0,0 +1,183 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+
+#include "i915_drv.h"
+
+int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len)
+{
+	WARN(1, "Unexpected send: action=%#x\n", *action);
+	return -ENODEV;
+}
+
+static void gen8_guc_raise_irq(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+	I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
+}
+
+void intel_guc_init_early(struct intel_guc *guc)
+{
+	intel_guc_ct_init_early(&guc->ct);
+
+	mutex_init(&guc->send_mutex);
+	guc->send = intel_guc_send_nop;
+	guc->notify = gen8_guc_raise_irq;
+}
+
+static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
+{
+	GEM_BUG_ON(!guc->send_regs.base);
+	GEM_BUG_ON(!guc->send_regs.count);
+	GEM_BUG_ON(i >= guc->send_regs.count);
+
+	return _MMIO(guc->send_regs.base + 4 * i);
+}
+
+void intel_guc_init_send_regs(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	enum forcewake_domains fw_domains = 0;
+	unsigned int i;
+
+	guc->send_regs.base = i915_mmio_reg_offset(SOFT_SCRATCH(0));
+	guc->send_regs.count = SOFT_SCRATCH_COUNT - 1;
+
+	for (i = 0; i < guc->send_regs.count; i++) {
+		fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
+					guc_send_reg(guc, i),
+					FW_REG_READ | FW_REG_WRITE);
+	}
+	guc->send_regs.fw_domains = fw_domains;
+}
+
+/*
+ * This function implements the MMIO based host to GuC interface.
+ */
+int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	u32 status;
+	int i;
+	int ret;
+
+	GEM_BUG_ON(!len);
+	GEM_BUG_ON(len > guc->send_regs.count);
+
+	/* If CT is available, we expect to use MMIO only during init/fini */
+	GEM_BUG_ON(HAS_GUC_CT(dev_priv) &&
+		*action != INTEL_GUC_ACTION_REGISTER_COMMAND_TRANSPORT_BUFFER &&
+		*action != INTEL_GUC_ACTION_DEREGISTER_COMMAND_TRANSPORT_BUFFER);
+
+	mutex_lock(&guc->send_mutex);
+	intel_uncore_forcewake_get(dev_priv, guc->send_regs.fw_domains);
+
+	for (i = 0; i < len; i++)
+		I915_WRITE(guc_send_reg(guc, i), action[i]);
+
+	POSTING_READ(guc_send_reg(guc, i - 1));
+
+	intel_guc_notify(guc);
+
+	/*
+	 * No GuC command should ever take longer than 10ms.
+	 * Fast commands should still complete in 10us.
+	 */
+	ret = __intel_wait_for_register_fw(dev_priv,
+					   guc_send_reg(guc, 0),
+					   INTEL_GUC_RECV_MASK,
+					   INTEL_GUC_RECV_MASK,
+					   10, 10, &status);
+	if (status != INTEL_GUC_STATUS_SUCCESS) {
+		/*
+		 * Either the GuC explicitly returned an error (which
+		 * we convert to -EIO here) or no response at all was
+		 * received within the timeout limit (-ETIMEDOUT)
+		 */
+		if (ret != -ETIMEDOUT)
+			ret = -EIO;
+
+		DRM_WARN("INTEL_GUC_SEND: Action 0x%X failed;"
+			 " ret=%d status=0x%08X response=0x%08X\n",
+			 action[0], ret, status, I915_READ(SOFT_SCRATCH(15)));
+	}
+
+	intel_uncore_forcewake_put(dev_priv, guc->send_regs.fw_domains);
+	mutex_unlock(&guc->send_mutex);
+
+	return ret;
+}
+
+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() - authenticate HuC
+ *
+ * Triggers a HuC fw authentication request to the GuC via intel_guc_action_
+ * authenticate_huc interface.
+ */
+void intel_guc_auth_huc(struct intel_guc *guc, struct intel_huc *huc)
+{
+	struct intel_uc_fw *huc_fw = &huc->fw;
+	struct i915_vma *vma;
+	int ret;
+	u32 data[2];
+
+	vma = i915_gem_object_ggtt_pin(huc_fw->obj, NULL, 0, 0,
+				PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
+	if (IS_ERR(vma)) {
+		DRM_ERROR("failed to pin huc fw object %d\n",
+				(int)PTR_ERR(vma));
+		return;
+	}
+
+	/* Specify auth action and where public signature is. */
+	data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC;
+	data[1] = guc_ggtt_offset(vma) + huc_fw->rsa_offset;
+
+	ret = intel_guc_send(guc, data, ARRAY_SIZE(data));
+	if (ret) {
+		DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret);
+		goto out;
+	}
+
+	ret = intel_huc_check_auth_status(huc);
+
+out:
+	i915_vma_unpin(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..234d9bf
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -0,0 +1,195 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+#ifndef _INTEL_GUC_H_
+#define _INTEL_GUC_H_
+
+#include "intel_uc.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.
+ *
+ * We also keep a few statistics on failures. Ideally, these should all
+ * be zero!
+ *   no_wq_space: times that the submission pre-check found no space was
+ *                available in the work queue (note, the queue is shared,
+ *                not per-engine). It is OK for this to be nonzero, but
+ *                it should not be huge!
+ *   b_fail: failed to ring the doorbell. This should never happen, unless
+ *           somehow the hardware misbehaves, or maybe if the GuC firmware
+ *           crashes? We probably need to reset the GPU to recover.
+ *   retcode: errno from last guc_submit()
+ */
+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;
+	u32 doorbell_cookie;
+
+	spinlock_t wq_lock;
+	uint32_t wq_offset;
+	uint32_t wq_size;
+	uint32_t wq_tail;
+	uint32_t wq_rsvd;
+	uint32_t no_wq_space;
+
+	/* Per-engine counts of GuC submissions */
+	uint64_t submissions[I915_NUM_ENGINES];
+};
+
+struct intel_guc_log {
+	uint32_t flags;
+	struct i915_vma *vma;
+	/* The runtime stuff gets created only when GuC logging gets enabled */
+	struct {
+		void *buf_addr;
+		struct workqueue_struct *flush_wq;
+		struct work_struct flush_work;
+		struct rchan *relay_chan;
+	} runtime;
+	/* logging related stats */
+	u32 capture_miss_count;
+	u32 flush_interrupt_count;
+	u32 prev_overflow_count[GUC_MAX_LOG_BUFFER];
+	u32 total_overflow_count[GUC_MAX_LOG_BUFFER];
+	u32 flush_count[GUC_MAX_LOG_BUFFER];
+};
+
+struct intel_guc {
+	struct intel_uc_fw fw;
+	struct intel_guc_log log;
+	struct intel_guc_ct ct;
+
+	/* Log snapshot if GuC errors during load */
+	struct drm_i915_gem_object *load_err_log;
+
+	/* intel_guc_recv interrupt related state */
+	bool interrupts_enabled;
+
+	struct i915_vma *ads_vma;
+	struct i915_vma *stage_desc_pool;
+	void *stage_desc_pool_vaddr;
+	struct ida stage_ids;
+
+	struct i915_guc_client *execbuf_client;
+
+	DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
+	uint32_t db_cacheline;		/* Cyclic counter mod pagesize	*/
+
+	/* GuC's FW specific registers used in MMIO send */
+	struct {
+		u32 base;
+		unsigned int count;
+		enum forcewake_domains fw_domains;
+	} send_regs;
+
+	/* To serialize the intel_guc_send actions */
+	struct mutex send_mutex;
+
+	/* GuC's FW specific send function */
+	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
+
+	/* GuC's FW specific notify function */
+	void (*notify)(struct intel_guc *guc);
+};
+
+static inline u32 guc_ggtt_offset(struct i915_vma *vma)
+{
+	u32 offset = i915_ggtt_offset(vma);
+
+	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
+	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
+	return offset;
+}
+
+/* intel_guc.c */
+void intel_guc_init_early(struct intel_guc *guc);
+int intel_guc_sample_forcewake(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);
+void intel_guc_auth_huc(struct intel_guc *guc, struct intel_huc *huc);
+
+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);
+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);
+int i915_guc_wq_reserve(struct drm_i915_gem_request *rq);
+void i915_guc_wq_unreserve(struct drm_i915_gem_request *request);
+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);
+int intel_guc_suspend(struct intel_guc *guc);
+int intel_guc_resume(struct intel_guc *guc);
+
+/* intel_guc_log.c */
+int intel_guc_log_create(struct intel_guc *guc);
+void intel_guc_log_destroy(struct intel_guc *guc);
+int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
+void i915_guc_log_register(struct drm_i915_private *dev_priv);
+void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
+
+#endif
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c b/drivers/gpu/drm/i915/intel_guc_loader.c
index 8b0ae7f..81e03a6 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -27,7 +27,6 @@
  *    Alex Dai <yu.dai@intel.com>
  */
 #include "i915_drv.h"
-#include "intel_uc.h"
 
 /**
  * DOC: GuC-specific firmware loader
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 6145fa0..f40089b 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -21,9 +21,7 @@
  * IN THE SOFTWARE.
  *
  */
-#include <linux/firmware.h>
 #include "i915_drv.h"
-#include "intel_uc.h"
 
 /**
  * DOC: HuC Firmware
@@ -224,41 +222,10 @@ void intel_huc_init_hw(struct intel_huc *huc)
 	return;
 }
 
-/**
- * intel_guc_auth_huc() - authenticate ucode
- * @dev_priv: the drm_i915_device
- *
- * Triggers a HuC fw authentication request to the GuC via intel_guc_action_
- * authenticate_huc interface.
- */
-void intel_guc_auth_huc(struct drm_i915_private *dev_priv)
+int intel_huc_check_auth_status(struct intel_huc *huc)
 {
-	struct intel_guc *guc = &dev_priv->guc;
-	struct intel_huc *huc = &dev_priv->huc;
-	struct i915_vma *vma;
+	struct drm_i915_private *dev_priv = huc_to_i915(huc);
 	int ret;
-	u32 data[2];
-
-	if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
-		return;
-
-	vma = i915_gem_object_ggtt_pin(huc->fw.obj, NULL, 0, 0,
-				PIN_OFFSET_BIAS | GUC_WOPCM_TOP);
-	if (IS_ERR(vma)) {
-		DRM_ERROR("failed to pin huc fw object %d\n",
-				(int)PTR_ERR(vma));
-		return;
-	}
-
-	/* Specify auth action and where public signature is. */
-	data[0] = INTEL_GUC_ACTION_AUTHENTICATE_HUC;
-	data[1] = guc_ggtt_offset(vma) + huc->fw.rsa_offset;
-
-	ret = intel_guc_send(guc, data, ARRAY_SIZE(data));
-	if (ret) {
-		DRM_ERROR("HuC: GuC did not ack Auth request %d\n", ret);
-		goto out;
-	}
 
 	/* Check authentication status, it should be done by now */
 	ret = intel_wait_for_register(dev_priv,
@@ -267,12 +234,21 @@ void intel_guc_auth_huc(struct drm_i915_private *dev_priv)
 				HUC_FW_VERIFIED,
 				50);
 
-	if (ret) {
+	if (ret)
 		DRM_ERROR("HuC: Authentication failed %d\n", ret);
-		goto out;
-	}
 
-out:
-	i915_vma_unpin(vma);
+	return ret;
 }
 
+/**
+ * intel_auth_huc() - authenticate HuC ucode
+ */
+void intel_huc_auth(struct intel_huc *huc)
+{
+	struct drm_i915_private *dev_priv = huc_to_i915(huc);
+
+	if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+		return;
+
+	intel_guc_auth_huc(&dev_priv->guc, huc);
+}
diff --git a/drivers/gpu/drm/i915/intel_huc.h b/drivers/gpu/drm/i915/intel_huc.h
new file mode 100644
index 0000000..9a9e23c
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_huc.h
@@ -0,0 +1,41 @@
+/*
+ * Copyright © 2017 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ */
+#ifndef _INTEL_HUC_H_
+#define _INTEL_HUC_H_
+
+#include "intel_uc.h"
+
+struct intel_huc {
+	/* Generic uC firmware management */
+	struct intel_uc_fw fw;
+
+	/* HuC-specific additions */
+};
+
+/* intel_huc.c */
+void intel_huc_select_fw(struct intel_huc *huc);
+void intel_huc_init_hw(struct intel_huc *huc);
+int intel_huc_check_auth_status(struct intel_huc *huc);
+void intel_huc_auth(struct intel_huc *huc);
+#endif
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 0178ba4..a3fc4c8 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -23,7 +23,6 @@
  */
 
 #include "i915_drv.h"
-#include "intel_uc.h"
 #include <linux/firmware.h>
 
 /* Cleans up uC firmware by releasing the firmware GEM obj.
@@ -94,22 +93,11 @@ void intel_uc_sanitize_options(struct drm_i915_private *dev_priv)
 		i915.enable_guc_submission = HAS_GUC_SCHED(dev_priv);
 }
 
-static void gen8_guc_raise_irq(struct intel_guc *guc)
-{
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-
-	I915_WRITE(GUC_SEND_INTERRUPT, GUC_SEND_TRIGGER);
-}
-
 void intel_uc_init_early(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
 
-	intel_guc_ct_init_early(&guc->ct);
-
-	mutex_init(&guc->send_mutex);
-	guc->send = intel_guc_send_nop;
-	guc->notify = gen8_guc_raise_irq;
+	intel_guc_init_early(guc);
 }
 
 static void fetch_uc_fw(struct drm_i915_private *dev_priv,
@@ -262,32 +250,6 @@ void intel_uc_fini_fw(struct drm_i915_private *dev_priv)
 	__intel_uc_fw_fini(&dev_priv->huc.fw);
 }
 
-static inline i915_reg_t guc_send_reg(struct intel_guc *guc, u32 i)
-{
-	GEM_BUG_ON(!guc->send_regs.base);
-	GEM_BUG_ON(!guc->send_regs.count);
-	GEM_BUG_ON(i >= guc->send_regs.count);
-
-	return _MMIO(guc->send_regs.base + 4 * i);
-}
-
-static void guc_init_send_regs(struct intel_guc *guc)
-{
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	enum forcewake_domains fw_domains = 0;
-	unsigned int i;
-
-	guc->send_regs.base = i915_mmio_reg_offset(SOFT_SCRATCH(0));
-	guc->send_regs.count = SOFT_SCRATCH_COUNT - 1;
-
-	for (i = 0; i < guc->send_regs.count; i++) {
-		fw_domains |= intel_uncore_forcewake_for_reg(dev_priv,
-					guc_send_reg(guc, i),
-					FW_REG_READ | FW_REG_WRITE);
-	}
-	guc->send_regs.fw_domains = fw_domains;
-}
-
 static void guc_capture_load_err_log(struct intel_guc *guc)
 {
 	if (!guc->log.vma || i915.guc_log_level < 0)
@@ -295,8 +257,6 @@ static void guc_capture_load_err_log(struct intel_guc *guc)
 
 	if (!guc->load_err_log)
 		guc->load_err_log = i915_gem_object_get(guc->log.vma->obj);
-
-	return;
 }
 
 static void guc_free_load_err_log(struct intel_guc *guc)
@@ -309,8 +269,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);
 
@@ -331,6 +289,7 @@ static void guc_disable_communication(struct intel_guc *guc)
 int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
+	struct intel_huc *huc = &dev_priv->huc;
 	int ret, attempts;
 
 	if (!i915.enable_guc_loading)
@@ -386,11 +345,13 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	if (ret)
 		goto err_log_capture;
 
+	intel_guc_init_send_regs(guc);
+
 	ret = guc_enable_communication(guc);
 	if (ret)
 		goto err_log_capture;
 
-	intel_guc_auth_huc(dev_priv);
+	intel_huc_auth(huc);
 	if (i915.enable_guc_submission) {
 		if (i915.guc_log_level >= 0)
 			gen9_enable_guc_interrupts(dev_priv);
@@ -458,82 +419,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 22ae52b..0d346ef 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -32,64 +32,6 @@
 
 struct drm_i915_gem_request;
 
-/*
- * This structure primarily describes the GEM object shared with the GuC.
- * The specs sometimes refer to this object as a "GuC context", but we use
- * the term "client" to avoid confusion with hardware contexts. This
- * GEM object is held for the entire lifetime of our interaction with
- * the GuC, being allocated before the GuC is loaded with its firmware.
- * Because there's no way to update the address used by the GuC after
- * initialisation, the shared object must stay pinned into the GGTT as
- * long as the GuC is in use. We also keep the first page (only) mapped
- * into kernel address space, as it includes shared data that must be
- * updated on every request submission.
- *
- * The single GEM object described here is actually made up of several
- * separate areas, as far as the GuC is concerned. The first page (kept
- * kmap'd) includes the "process descriptor" which holds sequence data for
- * the doorbell, and one cacheline which actually *is* the doorbell; a
- * write to this will "ring the doorbell" (i.e. send an interrupt to the
- * GuC). The subsequent  pages of the client object constitute the work
- * queue (a circular array of work items), again described in the process
- * descriptor. Work queue pages are mapped momentarily as required.
- *
- * We also keep a few statistics on failures. Ideally, these should all
- * be zero!
- *   no_wq_space: times that the submission pre-check found no space was
- *                available in the work queue (note, the queue is shared,
- *                not per-engine). It is OK for this to be nonzero, but
- *                it should not be huge!
- *   b_fail: failed to ring the doorbell. This should never happen, unless
- *           somehow the hardware misbehaves, or maybe if the GuC firmware
- *           crashes? We probably need to reset the GPU to recover.
- *   retcode: errno from last guc_submit()
- */
-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;
-	u32 doorbell_cookie;
-
-	spinlock_t wq_lock;
-	uint32_t wq_offset;
-	uint32_t wq_size;
-	uint32_t wq_tail;
-	uint32_t wq_rsvd;
-	uint32_t no_wq_space;
-
-	/* 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,
@@ -156,69 +98,6 @@ struct intel_uc_fw {
 	uint32_t ucode_offset;
 };
 
-struct intel_guc_log {
-	uint32_t flags;
-	struct i915_vma *vma;
-	/* The runtime stuff gets created only when GuC logging gets enabled */
-	struct {
-		void *buf_addr;
-		struct workqueue_struct *flush_wq;
-		struct work_struct flush_work;
-		struct rchan *relay_chan;
-	} runtime;
-	/* logging related stats */
-	u32 capture_miss_count;
-	u32 flush_interrupt_count;
-	u32 prev_overflow_count[GUC_MAX_LOG_BUFFER];
-	u32 total_overflow_count[GUC_MAX_LOG_BUFFER];
-	u32 flush_count[GUC_MAX_LOG_BUFFER];
-};
-
-struct intel_guc {
-	struct intel_uc_fw fw;
-	struct intel_guc_log log;
-	struct intel_guc_ct ct;
-
-	/* Log snapshot if GuC errors during load */
-	struct drm_i915_gem_object *load_err_log;
-
-	/* intel_guc_recv interrupt related state */
-	bool interrupts_enabled;
-
-	struct i915_vma *ads_vma;
-	struct i915_vma *stage_desc_pool;
-	void *stage_desc_pool_vaddr;
-	struct ida stage_ids;
-
-	struct i915_guc_client *execbuf_client;
-
-	DECLARE_BITMAP(doorbell_bitmap, GUC_NUM_DOORBELLS);
-	uint32_t db_cacheline;		/* Cyclic counter mod pagesize	*/
-
-	/* GuC's FW specific registers used in MMIO send */
-	struct {
-		u32 base;
-		unsigned int count;
-		enum forcewake_domains fw_domains;
-	} send_regs;
-
-	/* To serialize the intel_guc_send actions */
-	struct mutex send_mutex;
-
-	/* GuC's FW specific send function */
-	int (*send)(struct intel_guc *guc, const u32 *data, u32 len);
-
-	/* GuC's FW specific notify function */
-	void (*notify)(struct intel_guc *guc);
-};
-
-struct intel_huc {
-	/* Generic uC firmware management */
-	struct intel_uc_fw fw;
-
-	/* 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);
@@ -226,54 +105,5 @@ struct intel_huc {
 void intel_uc_fini_fw(struct drm_i915_private *dev_priv);
 int intel_uc_init_hw(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
-int intel_guc_sample_forcewake(struct intel_guc *guc);
-int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len);
-int intel_guc_send_mmio(struct intel_guc *guc, const u32 *action, u32 len);
-
-static inline int intel_guc_send(struct intel_guc *guc, const u32 *action, u32 len)
-{
-	return guc->send(guc, action, len);
-}
-
-static inline void intel_guc_notify(struct intel_guc *guc)
-{
-	guc->notify(guc);
-}
-
-/* intel_guc_loader.c */
-int intel_guc_select_fw(struct intel_guc *guc);
-int intel_guc_init_hw(struct intel_guc *guc);
-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);
-int i915_guc_wq_reserve(struct drm_i915_gem_request *rq);
-void i915_guc_wq_unreserve(struct drm_i915_gem_request *request);
-void i915_guc_submission_disable(struct drm_i915_private *dev_priv);
-void i915_guc_submission_fini(struct drm_i915_private *dev_priv);
-struct i915_vma *intel_guc_allocate_vma(struct intel_guc *guc, u32 size);
-
-/* intel_guc_log.c */
-int intel_guc_log_create(struct intel_guc *guc);
-void intel_guc_log_destroy(struct intel_guc *guc);
-int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val);
-void i915_guc_log_register(struct drm_i915_private *dev_priv);
-void i915_guc_log_unregister(struct drm_i915_private *dev_priv);
-
-static inline u32 guc_ggtt_offset(struct i915_vma *vma)
-{
-	u32 offset = i915_ggtt_offset(vma);
-	GEM_BUG_ON(offset < GUC_WOPCM_TOP);
-	GEM_BUG_ON(range_overflows_t(u64, offset, vma->size, GUC_GGTT_TOP));
-	return offset;
-}
-
-/* intel_huc.c */
-void intel_huc_select_fw(struct intel_huc *huc);
-void intel_huc_init_hw(struct intel_huc *huc);
-void intel_guc_auth_huc(struct drm_i915_private *dev_priv);
 
 #endif
-- 
1.9.1

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

[PATCH 2/6] drm/i915/guc: Make guc_enable/disable_communication functions public

[Sagar Arun Kamble]

This patch is moving guc_enable_communication and
guc_disable_communication to intel_guc.c and making it available for
use through intel_guc.h.
Intent is to reuse this function for calling from intel_uc_init_hw
and also as part of intel_uc_fini_hw where it will be coupled with
other teardown related to GuC in the upcoming patch.

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

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 5559e00..75bb830 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -181,3 +181,24 @@ void intel_guc_auth_huc(struct intel_guc *guc, struct intel_huc *huc)
 out:
 	i915_vma_unpin(vma);
 }
+
+int intel_guc_enable_communication(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+	if (HAS_GUC_CT(dev_priv))
+		return intel_guc_enable_ct(guc);
+
+	guc->send = intel_guc_send_mmio;
+	return 0;
+}
+
+void intel_guc_disable_communication(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+	if (HAS_GUC_CT(dev_priv))
+		intel_guc_disable_ct(guc);
+
+	guc->send = intel_guc_send_nop;
+}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 234d9bf..c6abf93 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -157,6 +157,8 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 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);
 void intel_guc_auth_huc(struct intel_guc *guc, struct intel_huc *huc);
+int intel_guc_enable_communication(struct intel_guc *guc);
+void intel_guc_disable_communication(struct intel_guc *guc);
 
 static inline int intel_guc_send(struct intel_guc *guc, const u32 *action,
 				 u32 len)
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index a3fc4c8..30c004c 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -265,27 +265,6 @@ static void guc_free_load_err_log(struct intel_guc *guc)
 		i915_gem_object_put(guc->load_err_log);
 }
 
-static int guc_enable_communication(struct intel_guc *guc)
-{
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-
-	if (HAS_GUC_CT(dev_priv))
-		return intel_guc_enable_ct(guc);
-
-	guc->send = intel_guc_send_mmio;
-	return 0;
-}
-
-static void guc_disable_communication(struct intel_guc *guc)
-{
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-
-	if (HAS_GUC_CT(dev_priv))
-		intel_guc_disable_ct(guc);
-
-	guc->send = intel_guc_send_nop;
-}
-
 int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
@@ -295,7 +274,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	if (!i915.enable_guc_loading)
 		return 0;
 
-	guc_disable_communication(guc);
+	intel_guc_disable_communication(guc);
 	gen9_reset_guc_interrupts(dev_priv);
 
 	/* We need to notify the guc whenever we change the GGTT */
@@ -347,7 +326,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 
 	intel_guc_init_send_regs(guc);
 
-	ret = guc_enable_communication(guc);
+	ret = intel_guc_enable_communication(guc);
 	if (ret)
 		goto err_log_capture;
 
@@ -373,7 +352,7 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	 * marks the GPU as wedged until reset).
 	 */
 err_interrupts:
-	guc_disable_communication(guc);
+	intel_guc_disable_communication(guc);
 	gen9_disable_guc_interrupts(dev_priv);
 err_log_capture:
 	guc_capture_load_err_log(guc);
@@ -410,7 +389,7 @@ void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
 	if (i915.enable_guc_submission)
 		i915_guc_submission_disable(dev_priv);
 
-	guc_disable_communication(&dev_priv->guc);
+	intel_guc_disable_communication(&dev_priv->guc);
 
 	if (i915.enable_guc_submission) {
 		gen9_disable_guc_interrupts(dev_priv);
-- 
1.9.1

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

[PATCH 3/6] drm/i915/guc: Fix GuC interaction in reset/suspend scenarios

[Sagar Arun Kamble]

From: "Kamble, Sagar A" <sagar.a.kamble@intel.com>

guc_ggtt_invalidate/guc_interrupts/guc_communication should be
disabled towards end of reset/suspend post GuC suspension as these
are setup back again during recovery/resume.

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

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

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c            |  11 ++-
 drivers/gpu/drm/i915/i915_gem.c            |   4 +-
 drivers/gpu/drm/i915/i915_guc_submission.c |  50 -----------
 drivers/gpu/drm/i915/intel_guc.c           | 131 +++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_guc.h           |   7 +-
 5 files changed, 146 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index b825024..d70a160 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1683,6 +1683,11 @@ static int i915_drm_resume(struct drm_device *dev)
 
 	drm_mode_config_reset(dev);
 
+	/*
+	 * NB: Full gem reinitialization is being done during i915_drm_resume,
+	 * hence intel_guc_system_resume will be of no use. If full
+	 * reinitialization is avoided, need to call intel_guc_system_resume.
+	 */
 	mutex_lock(&dev->struct_mutex);
 	if (i915_gem_init_hw(dev_priv)) {
 		DRM_ERROR("failed to re-initialize GPU, declaring wedged!\n");
@@ -1690,8 +1695,6 @@ static int i915_drm_resume(struct drm_device *dev)
 	}
 	mutex_unlock(&dev->struct_mutex);
 
-	intel_guc_resume(&dev_priv->guc);
-
 	intel_modeset_init_hw(dev);
 
 	spin_lock_irq(&dev_priv->irq_lock);
@@ -2492,7 +2495,7 @@ static int intel_runtime_suspend(struct device *kdev)
 	 */
 	i915_gem_runtime_suspend(dev_priv);
 
-	intel_guc_suspend(&dev_priv->guc);
+	intel_guc_runtime_suspend(&dev_priv->guc);
 
 	intel_runtime_pm_disable_interrupts(dev_priv);
 
@@ -2577,7 +2580,7 @@ static int intel_runtime_resume(struct device *kdev)
 	if (intel_uncore_unclaimed_mmio(dev_priv))
 		DRM_DEBUG_DRIVER("Unclaimed access during suspend, bios?\n");
 
-	intel_guc_resume(&dev_priv->guc);
+	intel_guc_runtime_resume(&dev_priv->guc);
 
 	if (IS_GEN9_LP(dev_priv)) {
 		bxt_disable_dc9(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index eb20e73..306e42a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2847,6 +2847,8 @@ int i915_gem_reset_prepare(struct drm_i915_private *dev_priv)
 
 	i915_gem_revoke_fences(dev_priv);
 
+	intel_guc_reset_prepare(&dev_priv->guc);
+
 	return err;
 }
 
@@ -4575,7 +4577,7 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	i915_gem_contexts_lost(dev_priv);
 	mutex_unlock(&dev->struct_mutex);
 
-	intel_guc_suspend(&dev_priv->guc);
+	intel_guc_system_suspend(&dev_priv->guc);
 
 	cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
 	cancel_delayed_work_sync(&dev_priv->gt.retire_work);
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
index 4b7afba..2f977ab 100644
--- a/drivers/gpu/drm/i915/i915_guc_submission.c
+++ b/drivers/gpu/drm/i915/i915_guc_submission.c
@@ -1287,53 +1287,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
- */
-int intel_guc_suspend(struct intel_guc *guc)
-{
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	struct i915_gem_context *ctx;
-	u32 data[3];
-
-	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
-		return 0;
-
-	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);
-
-	return intel_guc_send(guc, data, ARRAY_SIZE(data));
-}
-
-/**
- * intel_guc_resume() - notify GuC resuming from suspend state
- */
-int intel_guc_resume(struct intel_guc *guc)
-{
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	struct i915_gem_context *ctx;
-	u32 data[3];
-
-	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
-		return 0;
-
-	if (i915.guc_log_level >= 0)
-		gen9_enable_guc_interrupts(dev_priv);
-
-	ctx = dev_priv->kernel_context;
-
-	data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
-	data[1] = GUC_POWER_D0;
-	/* first page is shared data with GuC */
-	data[2] = guc_ggtt_offset(ctx->engine[RCS].state);
-
-	return intel_guc_send(guc, data, ARRAY_SIZE(data));
-}
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 75bb830..b957dab 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -202,3 +202,134 @@ void intel_guc_disable_communication(struct intel_guc *guc)
 
 	guc->send = intel_guc_send_nop;
 }
+
+/**
+ * intel_guc_suspend() - notify GuC entering suspend state
+ */
+static int intel_guc_suspend(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct i915_gem_context *ctx;
+	u32 data[3];
+
+	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+		return 0;
+
+	ctx = dev_priv->kernel_context;
+
+	data[0] = INTEL_GUC_ACTION_ENTER_S_STATE;
+	/* any value greater than GUC_POWER_D0 */
+	data[1] = GUC_POWER_D1;
+	/* first page is shared data with GuC */
+	data[2] = guc_ggtt_offset(ctx->engine[RCS].state);
+
+	return intel_guc_send(guc, data, ARRAY_SIZE(data));
+}
+
+/**
+ * intel_guc_resume() - notify GuC resuming from suspend state
+ */
+static int intel_guc_resume(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct i915_gem_context *ctx;
+	u32 data[3];
+
+	if (guc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
+		return 0;
+
+	ctx = dev_priv->kernel_context;
+
+	data[0] = INTEL_GUC_ACTION_EXIT_S_STATE;
+	data[1] = GUC_POWER_D0;
+	/* first page is shared data with GuC */
+	data[2] = guc_ggtt_offset(ctx->engine[RCS].state);
+
+	return intel_guc_send(guc, data, ARRAY_SIZE(data));
+}
+
+static void intel_guc_sanitize(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+
+	i915_ggtt_disable_guc(dev_priv);
+	intel_guc_disable_communication(guc);
+	gen9_disable_guc_interrupts(dev_priv);
+}
+
+void intel_guc_reset_prepare(struct intel_guc *guc)
+{
+	if (!i915.enable_guc_loading)
+		return;
+
+	intel_guc_sanitize(guc);
+	guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING;
+}
+
+static int intel_guc_pause(struct intel_guc *guc)
+{
+	int ret = 0;
+
+	ret = intel_guc_suspend(guc);
+	intel_guc_sanitize(guc);
+
+	return ret;
+}
+
+static int intel_guc_unpause(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	int ret = 0;
+
+	if (i915.guc_log_level >= 0)
+		gen9_enable_guc_interrupts(dev_priv);
+	intel_guc_enable_communication(guc);
+	i915_ggtt_enable_guc(dev_priv);
+	ret = intel_guc_resume(guc);
+
+	return ret;
+}
+
+int intel_guc_runtime_suspend(struct intel_guc *guc)
+{
+	if (!i915.enable_guc_loading)
+		return 0;
+
+	return intel_guc_pause(guc);
+}
+
+int intel_guc_runtime_resume(struct intel_guc *guc)
+{
+	if (!i915.enable_guc_loading)
+		return 0;
+
+	return intel_guc_unpause(guc);
+}
+
+int intel_guc_system_suspend(struct intel_guc *guc)
+{
+	int ret = 0;
+
+	if (!i915.enable_guc_loading)
+		return ret;
+
+	ret = intel_guc_pause(guc);
+	guc->fw.load_status = INTEL_UC_FIRMWARE_PENDING;
+
+	return ret;
+}
+
+int intel_guc_system_resume(struct intel_guc *guc)
+{
+	int ret = 0;
+
+	if (!i915.enable_guc_loading)
+		return ret;
+
+	/*
+	 * Placeholder for GuC resume from system suspend/freeze states.
+	 * Currently full reinitialization of GEM and GuC happens along
+	 * these paths, Hence this function is doing nothing.
+	 */
+	return ret;
+}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index c6abf93..49bab33 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -159,6 +159,11 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 void intel_guc_auth_huc(struct intel_guc *guc, struct intel_huc *huc);
 int intel_guc_enable_communication(struct intel_guc *guc);
 void intel_guc_disable_communication(struct intel_guc *guc);
+void intel_guc_reset_prepare(struct intel_guc *guc);
+int intel_guc_runtime_suspend(struct intel_guc *guc);
+int intel_guc_runtime_resume(struct intel_guc *guc);
+int intel_guc_system_suspend(struct intel_guc *guc);
+int intel_guc_system_resume(struct intel_guc *guc);
 
 static inline int intel_guc_send(struct intel_guc *guc, const u32 *action,
 				 u32 len)
@@ -184,8 +189,6 @@ static inline void intel_guc_notify(struct intel_guc *guc)
 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);
-int intel_guc_suspend(struct intel_guc *guc);
-int intel_guc_resume(struct intel_guc *guc);
 
 /* intel_guc_log.c */
 int intel_guc_log_create(struct intel_guc *guc);
-- 
1.9.1

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

[PATCH 4/6] drm/i915/guc: Fix GuC HW/SW state cleanup in unload path

[Sagar Arun Kamble]

From: "Kamble, Sagar A" <sagar.a.kamble@intel.com>

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

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

v2: Prepared i915_gem_unload. (Michal)

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

Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Michał Winiarski <michal.winiarski@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c  |  4 ++--
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/i915_gem.c  | 52 ++++++++++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_guc.c | 13 ++++++++++
 drivers/gpu/drm/i915/intel_guc.h |  1 +
 drivers/gpu/drm/i915/intel_uc.c  | 18 ++++++--------
 drivers/gpu/drm/i915/intel_uc.h  |  1 +
 7 files changed, 72 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index d70a160..001bff2 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -682,7 +682,7 @@ static int i915_load_modeset_init(struct drm_device *dev)
 	return 0;
 
 cleanup_gem:
-	if (i915_gem_suspend(dev_priv))
+	if (i915_gem_unload(dev_priv))
 		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
 	i915_gem_fini(dev_priv);
 cleanup_uc:
@@ -1375,7 +1375,7 @@ void i915_driver_unload(struct drm_device *dev)
 
 	i915_driver_unregister(dev_priv);
 
-	if (i915_gem_suspend(dev_priv))
+	if (i915_gem_unload(dev_priv))
 		DRM_ERROR("failed to idle hardware; continuing to unload!\n");
 
 	intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d0a9c18..9333e1c 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3661,6 +3661,7 @@ void i915_gem_reset_engine(struct intel_engine_cs *engine,
 int i915_gem_wait_for_idle(struct drm_i915_private *dev_priv,
 			   unsigned int flags);
 int __must_check i915_gem_suspend(struct drm_i915_private *dev_priv);
+int __must_check i915_gem_unload(struct drm_i915_private *dev_priv);
 void i915_gem_resume(struct drm_i915_private *dev_priv);
 int i915_gem_fault(struct vm_fault *vmf);
 int i915_gem_object_wait(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 306e42a..29509d0 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4545,12 +4545,11 @@ void i915_gem_sanitize(struct drm_i915_private *i915)
 	}
 }
 
-int i915_gem_suspend(struct drm_i915_private *dev_priv)
+static int i915_gem_contexts_suspend(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = &dev_priv->drm;
 	int ret;
 
-	intel_runtime_pm_get(dev_priv);
 	intel_suspend_gt_powersave(dev_priv);
 
 	mutex_lock(&dev->struct_mutex);
@@ -4577,8 +4576,15 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	i915_gem_contexts_lost(dev_priv);
 	mutex_unlock(&dev->struct_mutex);
 
-	intel_guc_system_suspend(&dev_priv->guc);
+	return 0;
+
+err_unlock:
+	mutex_unlock(&dev->struct_mutex);
+	return ret;
+}
 
+static void i915_gem_suspend_complete(struct drm_i915_private *dev_priv)
+{
 	cancel_delayed_work_sync(&dev_priv->gpu_error.hangcheck_work);
 	cancel_delayed_work_sync(&dev_priv->gt.retire_work);
 
@@ -4615,12 +4621,48 @@ int i915_gem_suspend(struct drm_i915_private *dev_priv)
 	 * machine in an unusable condition.
 	 */
 	i915_gem_sanitize(dev_priv);
+}
+
+int i915_gem_suspend(struct drm_i915_private *dev_priv)
+{
+	int ret;
+
+	intel_runtime_pm_get(dev_priv);
+
+	ret = i915_gem_contexts_suspend(dev_priv);
+	if (ret && ret != -EIO)
+		goto err_suspend;
+
+	intel_guc_system_suspend(&dev_priv->guc);
+
+	i915_gem_suspend_complete(dev_priv);
 
 	intel_runtime_pm_put(dev_priv);
 	return 0;
 
-err_unlock:
-	mutex_unlock(&dev->struct_mutex);
+err_suspend:
+	intel_runtime_pm_put(dev_priv);
+	return ret;
+}
+
+int i915_gem_unload(struct drm_i915_private *dev_priv)
+{
+	int ret;
+
+	intel_runtime_pm_get(dev_priv);
+
+	ret = i915_gem_contexts_suspend(dev_priv);
+	if (ret && ret != -EIO)
+		goto err_suspend;
+
+	intel_uc_unload(dev_priv);
+
+	i915_gem_suspend_complete(dev_priv);
+
+	intel_runtime_pm_put(dev_priv);
+	return 0;
+
+err_suspend:
 	intel_runtime_pm_put(dev_priv);
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index b957dab..178d8e4 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -333,3 +333,16 @@ int intel_guc_system_resume(struct intel_guc *guc)
 	 */
 	return ret;
 }
+
+void intel_guc_unload(struct intel_guc *guc)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	struct drm_device *dev = &dev_priv->drm;
+
+	if (i915.enable_guc_submission) {
+		mutex_lock(&dev->struct_mutex);
+		i915_guc_submission_disable(dev_priv);
+		mutex_unlock(&dev->struct_mutex);
+		intel_guc_reset_prepare(guc);
+	}
+}
diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
index 49bab33..d99a2cd 100644
--- a/drivers/gpu/drm/i915/intel_guc.h
+++ b/drivers/gpu/drm/i915/intel_guc.h
@@ -164,6 +164,7 @@ static inline u32 guc_ggtt_offset(struct i915_vma *vma)
 int intel_guc_runtime_resume(struct intel_guc *guc);
 int intel_guc_system_suspend(struct intel_guc *guc);
 int intel_guc_system_resume(struct intel_guc *guc);
+void intel_guc_unload(struct intel_guc *guc);
 
 static inline int intel_guc_send(struct intel_guc *guc, const u32 *action,
 				 u32 len)
diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index 30c004c..65561b4 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -379,22 +379,18 @@ int intel_uc_init_hw(struct drm_i915_private *dev_priv)
 	return ret;
 }
 
-void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
+void intel_uc_unload(struct drm_i915_private *dev_priv)
 {
-	guc_free_load_err_log(&dev_priv->guc);
+	intel_guc_unload(&dev_priv->guc);
+}
 
+void intel_uc_fini_hw(struct drm_i915_private *dev_priv)
+{
 	if (!i915.enable_guc_loading)
 		return;
 
-	if (i915.enable_guc_submission)
-		i915_guc_submission_disable(dev_priv);
-
-	intel_guc_disable_communication(&dev_priv->guc);
+	guc_free_load_err_log(&dev_priv->guc);
 
-	if (i915.enable_guc_submission) {
-		gen9_disable_guc_interrupts(dev_priv);
+	if (i915.enable_guc_submission)
 		i915_guc_submission_fini(dev_priv);
-	}
-
-	i915_ggtt_disable_guc(dev_priv);
 }
diff --git a/drivers/gpu/drm/i915/intel_uc.h b/drivers/gpu/drm/i915/intel_uc.h
index 0d346ef..0413a66 100644
--- a/drivers/gpu/drm/i915/intel_uc.h
+++ b/drivers/gpu/drm/i915/intel_uc.h
@@ -104,6 +104,7 @@ struct intel_uc_fw {
 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_unload(struct drm_i915_private *dev_priv);
 void intel_uc_fini_hw(struct drm_i915_private *dev_priv);
 
 #endif
-- 
1.9.1

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

[PATCH 5/6] drm/i915/guc: Enable default/critical logging in GuC by default from GuC v9

[Sagar Arun Kamble]

From: "Kamble, Sagar A" <sagar.a.kamble@intel.com>

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

v2: Emulating GuC critical logging through i915.guc_log_level. Setting
this to 0 will make GuC critical logging ON and setting it to 1-4 will
communicate log level of 0-3 to GuC.

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

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

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

diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
index 5fa2860..3ef228b 100644
--- a/drivers/gpu/drm/i915/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
@@ -131,7 +131,7 @@
 #define   GUC_PROFILE_ENABLED		(1 << 7)
 #define   GUC_WQ_TRACK_ENABLED		(1 << 8)
 #define   GUC_ADS_ENABLED		(1 << 9)
-#define   GUC_DEBUG_RESERVED		(1 << 10)
+#define   GUC_CRITICAL_LOGGING_DISABLED	(1 << 10)
 #define   GUC_ADS_ADDR_SHIFT		11
 #define   GUC_ADS_ADDR_MASK		0xfffff800
 
@@ -139,6 +139,16 @@
 
 #define GUC_CTL_MAX_DWORDS		(SOFT_SCRATCH_COUNT - 2) /* [1..14] */
 
+/*
+ * Critical logging in GuC is to be enabled always from GuC v9+.
+ * (for KBL - v9.39+)
+ */
+#define GUC_NEEDS_CRITICAL_LOGGING(guc)	\
+	(((IS_SKYLAKE(guc_to_i915(guc)) || IS_BROXTON(guc_to_i915(guc))) && \
+				    guc->fw.major_ver_found >= 9) || \
+	  (IS_KABYLAKE(guc_to_i915(guc)) && guc->fw.major_ver_found >= 9 && \
+				    guc->fw.minor_ver_found >= 39))
+
 /**
  * DOC: GuC Firmware Layout
  *
@@ -539,7 +549,8 @@ struct guc_log_buffer_state {
 		u32 logging_enabled:1;
 		u32 reserved1:3;
 		u32 verbosity:4;
-		u32 reserved2:24;
+		u32 critical_logging_enabled:1;
+		u32 reserved2:23;
 	};
 	u32 value;
 } __packed;
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 16d3b87..ba36162 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -589,7 +589,6 @@ void intel_guc_log_destroy(struct intel_guc *guc)
 int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
 {
 	struct intel_guc *guc = &dev_priv->guc;
-
 	union guc_log_control log_param;
 	int ret;
 
@@ -603,6 +602,9 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
 	if (!log_param.logging_enabled && (i915.guc_log_level < 0))
 		return 0;
 
+	if (GUC_NEEDS_CRITICAL_LOGGING(guc))
+		log_param.critical_logging_enabled = 1;
+
 	ret = guc_log_control(guc, log_param.value);
 	if (ret < 0) {
 		DRM_DEBUG_DRIVER("guc_logging_control action failed %d\n", ret);
-- 
1.9.1

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

[PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts

[Sagar Arun Kamble]

Disabling GuC interrupts involves access to GuC IRQ control registers
hence ensure device is RPM awake.

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_log.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index ba36162..d7557b5 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
 		return;
 
 	mutex_lock(&dev_priv->drm.struct_mutex);
+	intel_runtime_pm_get(dev_priv);
 	/* GuC logging is currently the only user of Guc2Host interrupts */
 	gen9_disable_guc_interrupts(dev_priv);
+	intel_runtime_pm_put(dev_priv);
 	guc_log_runtime_destroy(&dev_priv->guc);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 }
-- 
1.9.1

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

✓ Fi.CI.BAT: success for series starting with [1/6] drm/i915: Separate GuC/HuC specific functionality from intel_uc

[Patchwork]

== Series Details ==

Series: series starting with [1/6] drm/i915: Separate GuC/HuC specific functionality from intel_uc
URL   : https://patchwork.freedesktop.org/series/30097/
State : success

== Summary ==

Series 30097v1 series starting with [1/6] drm/i915: Separate GuC/HuC specific functionality from intel_uc
https://patchwork.freedesktop.org/api/1.0/series/30097/revisions/1/mbox/

Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-snb-2600) fdo#100215 +1

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

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:444s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:462s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:378s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:532s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:268s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:508s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:508s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:496s
fi-cfl-s         total:289  pass:250  dwarn:4   dfail:0   fail:0   skip:35  time:449s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:448s
fi-glk-2a        total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:593s
fi-hsw-4770      total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:429s
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:437s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:475s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:458s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:494s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:580s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:582s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:550s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:461s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:517s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:499s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:462s
fi-skl-x1585l    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:495s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:569s
fi-snb-2600      total:289  pass:248  dwarn:0   dfail:0   fail:2   skip:39  time:420s

9ef5732ddf6924e98a5c5d034a3535cff14f72bf drm-tip: 2017y-09m-10d-21h-59m-53s UTC integration manifest
dbb7123f6120 drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts
9b8949afbec2 drm/i915/guc: Enable default/critical logging in GuC by default from GuC v9
4333e8d73e37 drm/i915/guc: Fix GuC HW/SW state cleanup in unload path
c1bfd3d0850b drm/i915/guc: Fix GuC interaction in reset/suspend scenarios
1f0379ed2964 drm/i915/guc: Make guc_enable/disable_communication functions public
cc87ca394b76 drm/i915: Separate GuC/HuC specific functionality from intel_uc

== Logs ==

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

✗ Fi.CI.IGT: failure for series starting with [1/6] drm/i915: Separate GuC/HuC specific functionality from intel_uc

[Patchwork]

== Series Details ==

Series: series starting with [1/6] drm/i915: Separate GuC/HuC specific functionality from intel_uc
URL   : https://patchwork.freedesktop.org/series/30097/
State : failure

== Summary ==

Test kms_sysfs_edid_timing:
                pass       -> WARN       (shard-hsw) fdo#100047
Test kms_setmode:
        Subgroup basic:
                pass       -> FAIL       (shard-hsw) fdo#99912
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-C-frame-sequence:
                pass       -> FAIL       (shard-hsw)
Test perf:
        Subgroup polling:
                pass       -> FAIL       (shard-hsw) fdo#102252

fdo#100047 https://bugs.freedesktop.org/show_bug.cgi?id=100047
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:2302 pass:1235 dwarn:0   dfail:0   fail:14  skip:1052 time:9469s

== Logs ==

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

Re: [PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts

[Michal Wajdeczko]

On Mon, Sep 11, 2017 at 11:32:24AM +0530, Sagar Arun Kamble wrote:
> Disabling GuC interrupts involves access to GuC IRQ control registers
> hence ensure device is RPM awake.
> 
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_log.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index ba36162..d7557b5 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
>  		return;
>  
>  	mutex_lock(&dev_priv->drm.struct_mutex);
> +	intel_runtime_pm_get(dev_priv);
>  	/* GuC logging is currently the only user of Guc2Host interrupts */
>  	gen9_disable_guc_interrupts(dev_priv);
> +	intel_runtime_pm_put(dev_priv);

There are other places in this file where guc interrupts are enabled/disabled.
Shouldn't we do the same there ?

Regards,
Michal

>  	guc_log_runtime_destroy(&dev_priv->guc);
>  	mutex_unlock(&dev_priv->drm.struct_mutex);
>  }
> -- 
> 1.9.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 5/6] drm/i915/guc: Enable default/critical logging in GuC by default from GuC v9

[Michal Wajdeczko]

On Mon, Sep 11, 2017 at 11:32:23AM +0530, Sagar Arun Kamble wrote:
> From: "Kamble, Sagar A" <sagar.a.kamble@intel.com>
> 
> With GuC v9, new type of Default/critical logging in GuC to enable
> capturing minimal important logs in production systems efficiently.
> This patch enables this logging in GuC by default always. It should
> be noted that streaming support with half-full interrupt mechanism
> that is present for normal logging is not present for this type of
> logging.
> 
> v2: Emulating GuC critical logging through i915.guc_log_level. Setting
> this to 0 will make GuC critical logging ON and setting it to 1-4 will
> communicate log level of 0-3 to GuC.
> 
> v3: Commit message update. Enable default/critical logging in GuC always.
> Fixed RPM wake during guc_log_unregister in the unload path.
> 
> v4: Moved RPM wake change to separate patch. Removed GUC_DEBUG_RESERVED
> and updated name of new bit to be version agnostic. Updated parameter to
> struct intel_guc * and name of macro NEEDS_GUC_CRITICAL_LOGGING.
> Removed explicit clearing of GUC_CRITICAL_LOGGING_DISABLED from
> params[GUC_CTL_DEBUG] as it is unnecessary. (Michal Wajdeczko)
> 
> Cc: Chheda Harsh J <harsh.j.chheda@intel.com>
> Cc: Fry Gregory P <gregory.p.fry@intel.com>
> Cc: Spotswood John A <john.a.spotswood@intel.com>
> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Michał Winiarski <michal.winiarski@intel.com>
> Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_fwif.h | 15 +++++++++++++--
>  drivers/gpu/drm/i915/intel_guc_log.c  |  4 +++-
>  2 files changed, 16 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
> index 5fa2860..3ef228b 100644
> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
> @@ -131,7 +131,7 @@
>  #define   GUC_PROFILE_ENABLED		(1 << 7)
>  #define   GUC_WQ_TRACK_ENABLED		(1 << 8)
>  #define   GUC_ADS_ENABLED		(1 << 9)
> -#define   GUC_DEBUG_RESERVED		(1 << 10)
> +#define   GUC_CRITICAL_LOGGING_DISABLED	(1 << 10)

Looks like we don't need this at all


>  #define   GUC_ADS_ADDR_SHIFT		11
>  #define   GUC_ADS_ADDR_MASK		0xfffff800
>  
> @@ -139,6 +139,16 @@
>  
>  #define GUC_CTL_MAX_DWORDS		(SOFT_SCRATCH_COUNT - 2) /* [1..14] */
>  
> +/*
> + * Critical logging in GuC is to be enabled always from GuC v9+.
> + * (for KBL - v9.39+)
> + */
> +#define GUC_NEEDS_CRITICAL_LOGGING(guc)	\
> +	(((IS_SKYLAKE(guc_to_i915(guc)) || IS_BROXTON(guc_to_i915(guc))) && \

Can we use here HAS_GUC() ? Comment says that this is for all Guc

Michal


> +				    guc->fw.major_ver_found >= 9) || \
> +	  (IS_KABYLAKE(guc_to_i915(guc)) && guc->fw.major_ver_found >= 9 && \
> +				    guc->fw.minor_ver_found >= 39))
> +
>  /**
>   * DOC: GuC Firmware Layout
>   *
> @@ -539,7 +549,8 @@ struct guc_log_buffer_state {
>  		u32 logging_enabled:1;
>  		u32 reserved1:3;
>  		u32 verbosity:4;
> -		u32 reserved2:24;
> +		u32 critical_logging_enabled:1;
> +		u32 reserved2:23;
>  	};
>  	u32 value;
>  } __packed;
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index 16d3b87..ba36162 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -589,7 +589,6 @@ void intel_guc_log_destroy(struct intel_guc *guc)
>  int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
>  {
>  	struct intel_guc *guc = &dev_priv->guc;
> -
>  	union guc_log_control log_param;
>  	int ret;
>  
> @@ -603,6 +602,9 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
>  	if (!log_param.logging_enabled && (i915.guc_log_level < 0))
>  		return 0;
>  
> +	if (GUC_NEEDS_CRITICAL_LOGGING(guc))
> +		log_param.critical_logging_enabled = 1;
> +
>  	ret = guc_log_control(guc, log_param.value);
>  	if (ret < 0) {
>  		DRM_DEBUG_DRIVER("guc_logging_control action failed %d\n", ret);
> -- 
> 1.9.1
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts

[Kamble, Sagar A]

On 9/11/2017 11:04 PM, Michal Wajdeczko wrote:
> On Mon, Sep 11, 2017 at 11:32:24AM +0530, Sagar Arun Kamble wrote:
>> Disabling GuC interrupts involves access to GuC IRQ control registers
>> hence ensure device is RPM awake.
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_guc_log.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
>> index ba36162..d7557b5 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_log.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
>> @@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
>>   		return;
>>   
>>   	mutex_lock(&dev_priv->drm.struct_mutex);
>> +	intel_runtime_pm_get(dev_priv);
>>   	/* GuC logging is currently the only user of Guc2Host interrupts */
>>   	gen9_disable_guc_interrupts(dev_priv);
>> +	intel_runtime_pm_put(dev_priv);
> There are other places in this file where guc interrupts are enabled/disabled.
> Shouldn't we do the same there ?
>
> Regards,
> Michal
Those are already covered by the RPM locks along the paths 
i915_gem_init_hw, i915_guc_log_control, guc_unpause, guc_pause caller in 
system suspend, i915_reset.
>
>>   	guc_log_runtime_destroy(&dev_priv->guc);
>>   	mutex_unlock(&dev_priv->drm.struct_mutex);
>>   }
>> -- 
>> 1.9.1
>>

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

Re: [PATCH 5/6] drm/i915/guc: Enable default/critical logging in GuC by default from GuC v9

[Kamble, Sagar A]

On 9/11/2017 11:34 PM, Michal Wajdeczko wrote:
> On Mon, Sep 11, 2017 at 11:32:23AM +0530, Sagar Arun Kamble wrote:
>> From: "Kamble, Sagar A" <sagar.a.kamble@intel.com>
>>
>> With GuC v9, new type of Default/critical logging in GuC to enable
>> capturing minimal important logs in production systems efficiently.
>> This patch enables this logging in GuC by default always. It should
>> be noted that streaming support with half-full interrupt mechanism
>> that is present for normal logging is not present for this type of
>> logging.
>>
>> v2: Emulating GuC critical logging through i915.guc_log_level. Setting
>> this to 0 will make GuC critical logging ON and setting it to 1-4 will
>> communicate log level of 0-3 to GuC.
>>
>> v3: Commit message update. Enable default/critical logging in GuC always.
>> Fixed RPM wake during guc_log_unregister in the unload path.
>>
>> v4: Moved RPM wake change to separate patch. Removed GUC_DEBUG_RESERVED
>> and updated name of new bit to be version agnostic. Updated parameter to
>> struct intel_guc * and name of macro NEEDS_GUC_CRITICAL_LOGGING.
>> Removed explicit clearing of GUC_CRITICAL_LOGGING_DISABLED from
>> params[GUC_CTL_DEBUG] as it is unnecessary. (Michal Wajdeczko)
>>
>> Cc: Chheda Harsh J <harsh.j.chheda@intel.com>
>> Cc: Fry Gregory P <gregory.p.fry@intel.com>
>> Cc: Spotswood John A <john.a.spotswood@intel.com>
>> Cc: Anusha Srivatsa <anusha.srivatsa@intel.com>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Cc: Michał Winiarski <michal.winiarski@intel.com>
>> Signed-off-by: Jeff McGee <jeff.mcgee@intel.com>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_guc_fwif.h | 15 +++++++++++++--
>>   drivers/gpu/drm/i915/intel_guc_log.c  |  4 +++-
>>   2 files changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_fwif.h b/drivers/gpu/drm/i915/intel_guc_fwif.h
>> index 5fa2860..3ef228b 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_fwif.h
>> +++ b/drivers/gpu/drm/i915/intel_guc_fwif.h
>> @@ -131,7 +131,7 @@
>>   #define   GUC_PROFILE_ENABLED		(1 << 7)
>>   #define   GUC_WQ_TRACK_ENABLED		(1 << 8)
>>   #define   GUC_ADS_ENABLED		(1 << 9)
>> -#define   GUC_DEBUG_RESERVED		(1 << 10)
>> +#define   GUC_CRITICAL_LOGGING_DISABLED	(1 << 10)
> Looks like we don't need this at all
I had disabled this logging in the initial version of patch to reduce 
the impact.
So earlier I had to explicitly declare this bit and set in the 
guc_params[CTL_DEBUG].
But recommendation from GuC team is to keep this always enabled hence we 
can do away with not touching
guc_params[CTL_DEBUG] at all but then in order to make the users aware 
that bit 10 being unset has some meaning I was
clearing it explicitly. Wont removing this bit definition now eliminate 
the meaning for bit 10 while loading GuC all together?

>
>
>>   #define   GUC_ADS_ADDR_SHIFT		11
>>   #define   GUC_ADS_ADDR_MASK		0xfffff800
>>   
>> @@ -139,6 +139,16 @@
>>   
>>   #define GUC_CTL_MAX_DWORDS		(SOFT_SCRATCH_COUNT - 2) /* [1..14] */
>>   
>> +/*
>> + * Critical logging in GuC is to be enabled always from GuC v9+.
>> + * (for KBL - v9.39+)
>> + */
>> +#define GUC_NEEDS_CRITICAL_LOGGING(guc)	\
>> +	(((IS_SKYLAKE(guc_to_i915(guc)) || IS_BROXTON(guc_to_i915(guc))) && \
> Can we use here HAS_GUC() ? Comment says that this is for all Guc
>
> Michal
Yes. Will add this.
>
>
>> +				    guc->fw.major_ver_found >= 9) || \
>> +	  (IS_KABYLAKE(guc_to_i915(guc)) && guc->fw.major_ver_found >= 9 && \
>> +				    guc->fw.minor_ver_found >= 39))
>> +
>>   /**
>>    * DOC: GuC Firmware Layout
>>    *
>> @@ -539,7 +549,8 @@ struct guc_log_buffer_state {
>>   		u32 logging_enabled:1;
>>   		u32 reserved1:3;
>>   		u32 verbosity:4;
>> -		u32 reserved2:24;
>> +		u32 critical_logging_enabled:1;
>> +		u32 reserved2:23;
>>   	};
>>   	u32 value;
>>   } __packed;
>> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
>> index 16d3b87..ba36162 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_log.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
>> @@ -589,7 +589,6 @@ void intel_guc_log_destroy(struct intel_guc *guc)
>>   int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
>>   {
>>   	struct intel_guc *guc = &dev_priv->guc;
>> -
>>   	union guc_log_control log_param;
>>   	int ret;
>>   
>> @@ -603,6 +602,9 @@ int i915_guc_log_control(struct drm_i915_private *dev_priv, u64 control_val)
>>   	if (!log_param.logging_enabled && (i915.guc_log_level < 0))
>>   		return 0;
>>   
>> +	if (GUC_NEEDS_CRITICAL_LOGGING(guc))
>> +		log_param.critical_logging_enabled = 1;
>> +
>>   	ret = guc_log_control(guc, log_param.value);
>>   	if (ret < 0) {
>>   		DRM_DEBUG_DRIVER("guc_logging_control action failed %d\n", ret);
>> -- 
>> 1.9.1
>>

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

[PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts

[Sagar Arun Kamble]

From: "Kamble, Sagar A" <sagar.a.kamble@intel.com>

Disabling GuC interrupts involves access to GuC IRQ control registers
hence ensure device is RPM awake.

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_log.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index ba36162..d7557b5 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
 		return;
 
 	mutex_lock(&dev_priv->drm.struct_mutex);
+	intel_runtime_pm_get(dev_priv);
 	/* GuC logging is currently the only user of Guc2Host interrupts */
 	gen9_disable_guc_interrupts(dev_priv);
+	intel_runtime_pm_put(dev_priv);
 	guc_log_runtime_destroy(&dev_priv->guc);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 }
-- 
1.9.1

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

[PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts

[Sagar Arun Kamble]

From: "Kamble, Sagar A" <sagar.a.kamble@intel.com>

Disabling GuC interrupts involves access to GuC IRQ control registers
hence ensure device is RPM awake.

Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
---
 drivers/gpu/drm/i915/intel_guc_log.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index ba36162..d7557b5 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct drm_i915_private *dev_priv)
 		return;
 
 	mutex_lock(&dev_priv->drm.struct_mutex);
+	intel_runtime_pm_get(dev_priv);
 	/* GuC logging is currently the only user of Guc2Host interrupts */
 	gen9_disable_guc_interrupts(dev_priv);
+	intel_runtime_pm_put(dev_priv);
 	guc_log_runtime_destroy(&dev_priv->guc);
 	mutex_unlock(&dev_priv->drm.struct_mutex);
 }
-- 
1.9.1

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

Re: [PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts

[Michal Wajdeczko]

On Thu, 14 Sep 2017 11:55:08 +0200, Sagar Arun Kamble  
<sagar.a.kamble@intel.com> wrote:

> From: "Kamble, Sagar A" <sagar.a.kamble@intel.com>
>
> Disabling GuC interrupts involves access to GuC IRQ control registers
> hence ensure device is RPM awake.
>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_guc_log.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c  
> b/drivers/gpu/drm/i915/intel_guc_log.c
> index ba36162..d7557b5 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct  
> drm_i915_private *dev_priv)
>  		return;
> 	mutex_lock(&dev_priv->drm.struct_mutex);
> +	intel_runtime_pm_get(dev_priv);
>  	/* GuC logging is currently the only user of Guc2Host interrupts */
>  	gen9_disable_guc_interrupts(dev_priv);
> +	intel_runtime_pm_put(dev_priv);
>  	guc_log_runtime_destroy(&dev_priv->guc);

Maybe we should just destroy runtime here, and allow irq to be disabled
by intel_uc_fini_hw() which will be called right after. This will also
fix the upcoming case when log will not be the only user of Guc irqs.

See https://patchwork.freedesktop.org/patch/170390/

Michal

>  	mutex_unlock(&dev_priv->drm.struct_mutex);
>  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts

[Kamble, Sagar A]

On 9/14/2017 6:07 PM, Michal Wajdeczko wrote:
> On Thu, 14 Sep 2017 11:55:08 +0200, Sagar Arun Kamble 
> <sagar.a.kamble@intel.com> wrote:
>
>> From: "Kamble, Sagar A" <sagar.a.kamble@intel.com>
>>
>> Disabling GuC interrupts involves access to GuC IRQ control registers
>> hence ensure device is RPM awake.
>>
>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_guc_log.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c 
>> b/drivers/gpu/drm/i915/intel_guc_log.c
>> index ba36162..d7557b5 100644
>> --- a/drivers/gpu/drm/i915/intel_guc_log.c
>> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
>> @@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct 
>> drm_i915_private *dev_priv)
>>          return;
>>     mutex_lock(&dev_priv->drm.struct_mutex);
>> +    intel_runtime_pm_get(dev_priv);
>>      /* GuC logging is currently the only user of Guc2Host interrupts */
>>      gen9_disable_guc_interrupts(dev_priv);
>> +    intel_runtime_pm_put(dev_priv);
>>      guc_log_runtime_destroy(&dev_priv->guc);
>
> Maybe we should just destroy runtime here, and allow irq to be disabled
> by intel_uc_fini_hw() which will be called right after. This will also
> fix the upcoming case when log will not be the only user of Guc irqs.
>
> See https://patchwork.freedesktop.org/patch/170390/
>
> Michal
Destroying runtime here may create issues as enabled GuC interrupts may 
be causing the use of the guc_log_runtime.
Should we move the i915_driver_unregister post i915_gem_unload?
>
>> mutex_unlock(&dev_priv->drm.struct_mutex);
>>  }

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

Re: [PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts

[Michal Wajdeczko]

On Thu, 14 Sep 2017 18:04:27 +0200, Kamble, Sagar A  
<sagar.a.kamble@intel.com> wrote:

>
>
> On 9/14/2017 6:07 PM, Michal Wajdeczko wrote:
>> On Thu, 14 Sep 2017 11:55:08 +0200, Sagar Arun Kamble  
>> <sagar.a.kamble@intel.com> wrote:
>>
>>> From: "Kamble, Sagar A" <sagar.a.kamble@intel.com>
>>>
>>> Disabling GuC interrupts involves access to GuC IRQ control registers
>>> hence ensure device is RPM awake.
>>>
>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/intel_guc_log.c | 2 ++
>>>  1 file changed, 2 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c  
>>> b/drivers/gpu/drm/i915/intel_guc_log.c
>>> index ba36162..d7557b5 100644
>>> --- a/drivers/gpu/drm/i915/intel_guc_log.c
>>> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
>>> @@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct  
>>> drm_i915_private *dev_priv)
>>>          return;
>>>     mutex_lock(&dev_priv->drm.struct_mutex);
>>> +    intel_runtime_pm_get(dev_priv);
>>>      /* GuC logging is currently the only user of Guc2Host interrupts  
>>> */
>>>      gen9_disable_guc_interrupts(dev_priv);
>>> +    intel_runtime_pm_put(dev_priv);
>>>      guc_log_runtime_destroy(&dev_priv->guc);
>>
>> Maybe we should just destroy runtime here, and allow irq to be disabled
>> by intel_uc_fini_hw() which will be called right after. This will also
>> fix the upcoming case when log will not be the only user of Guc irqs.
>>
>> See https://patchwork.freedesktop.org/patch/170390/
>>
>> Michal
> Destroying runtime here may create issues as enabled GuC interrupts may  
> be causing the use of the guc_log_runtime.

Yes, but this should be easy to fix.

> Should we move the i915_driver_unregister post i915_gem_unload?

But then we will have to handle the case when gem was unloaded but
driver will still be in registered state.

Note that as guc log will not be the only user of the guc irqs,
code that disables irq will be removed from this function anyway.

Michal

>>
>>> mutex_unlock(&dev_priv->drm.struct_mutex);
>>>  }
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/6] drm/i915/guc: Grab RPM wakelock while disabling GuC interrupts

[Kamble, Sagar A]

On 9/14/2017 9:41 PM, Michal Wajdeczko wrote:
> On Thu, 14 Sep 2017 18:04:27 +0200, Kamble, Sagar A 
> <sagar.a.kamble@intel.com> wrote:
>
>>
>>
>> On 9/14/2017 6:07 PM, Michal Wajdeczko wrote:
>>> On Thu, 14 Sep 2017 11:55:08 +0200, Sagar Arun Kamble 
>>> <sagar.a.kamble@intel.com> wrote:
>>>
>>>> From: "Kamble, Sagar A" <sagar.a.kamble@intel.com>
>>>>
>>>> Disabling GuC interrupts involves access to GuC IRQ control registers
>>>> hence ensure device is RPM awake.
>>>>
>>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>>> Signed-off-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
>>>> ---
>>>>  drivers/gpu/drm/i915/intel_guc_log.c | 2 ++
>>>>  1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c 
>>>> b/drivers/gpu/drm/i915/intel_guc_log.c
>>>> index ba36162..d7557b5 100644
>>>> --- a/drivers/gpu/drm/i915/intel_guc_log.c
>>>> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
>>>> @@ -657,8 +657,10 @@ void i915_guc_log_unregister(struct 
>>>> drm_i915_private *dev_priv)
>>>>          return;
>>>>     mutex_lock(&dev_priv->drm.struct_mutex);
>>>> +    intel_runtime_pm_get(dev_priv);
>>>>      /* GuC logging is currently the only user of Guc2Host 
>>>> interrupts */
>>>>      gen9_disable_guc_interrupts(dev_priv);
>>>> +    intel_runtime_pm_put(dev_priv);
>>>>      guc_log_runtime_destroy(&dev_priv->guc);
>>>
>>> Maybe we should just destroy runtime here, and allow irq to be disabled
>>> by intel_uc_fini_hw() which will be called right after. This will also
>>> fix the upcoming case when log will not be the only user of Guc irqs.
>>>
>>> See https://patchwork.freedesktop.org/patch/170390/
>>>
>>> Michal
>> Destroying runtime here may create issues as enabled GuC interrupts 
>> may be causing the use of the guc_log_runtime.
>
> Yes, but this should be easy to fix.
>
>> Should we move the i915_driver_unregister post i915_gem_unload?
>
> But then we will have to handle the case when gem was unloaded but
> driver will still be in registered state.
>
> Note that as guc log will not be the only user of the guc irqs,
> code that disables irq will be removed from this function anyway.
>
> Michal
I see now that guc_log_runtime_destroy already happen along 
intel_uc_fini_hw->i915_guc_submission_fini->intel_guc_log_destroy path too.
So we can remove guc_log_unregister completely with irq_disable 
happening as part of intel_guc_unload and guc_log_runtime_destroy too 
handled.
>
>>>
>>>> mutex_unlock(&dev_priv->drm.struct_mutex);
>>>>  }

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