[Intel-gfx] [PATCH v2] drm/i915/guc: Enable guc logging on guc log relay write

[Matthew Brost]

On Mon, Sep 16, 2019 at 03:50:18PM -0700, Robert M. Fosha wrote:
>Creating and opening the GuC log relay file enables and starts
>the relay potentially before the caller is ready to consume logs.
>Change the behavior so that relay starts only on an explicit call
>to the write function (with a value of '1'). Other values flush
>the log relay as before.
>
>v2: Style changes and fix typos. Add guc_log_relay_stop()
>function. (Daniele)
>
>Cc: Matthew Brost <matthew.brost at intel.com>
>Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
>Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
>Signed-off-by: Robert M. Fosha <robert.m.fosha at intel.com>
>---
> drivers/gpu/drm/i915/gt/uc/intel_guc_log.c | 53 +++++++++++++++++-----
> drivers/gpu/drm/i915/gt/uc/intel_guc_log.h |  4 +-
> drivers/gpu/drm/i915/i915_debugfs.c        | 22 +++++++--
> 3 files changed, 62 insertions(+), 17 deletions(-)
>

Reviewed-by: Matthew Brost <matthew.brost at intel.com>

>diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
>index 36332064de9c..e26c7748358b 100644
>--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
>+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.c
>@@ -226,7 +226,7 @@ static void guc_read_update_log_buffer(struct intel_guc_log *log)
>
> 	mutex_lock(&log->relay.lock);
>
>-	if (WARN_ON(!intel_guc_log_relay_enabled(log)))
>+	if (WARN_ON(!intel_guc_log_relay_created(log)))
> 		goto out_unlock;
>
> 	/* Get the pointer to shared GuC log buffer */
>@@ -361,6 +361,7 @@ void intel_guc_log_init_early(struct intel_guc_log *log)
> {
> 	mutex_init(&log->relay.lock);
> 	INIT_WORK(&log->relay.flush_work, capture_logs_work);
>+	log->relay.started = false;
> }
>
> static int guc_log_relay_create(struct intel_guc_log *log)
>@@ -546,7 +547,7 @@ int intel_guc_log_set_level(struct intel_guc_log *log, u32 level)
> 	return ret;
> }
>
>-bool intel_guc_log_relay_enabled(const struct intel_guc_log *log)
>+bool intel_guc_log_relay_created(const struct intel_guc_log *log)
> {
> 	return log->relay.buf_addr;
> }
>@@ -560,7 +561,7 @@ int intel_guc_log_relay_open(struct intel_guc_log *log)
>
> 	mutex_lock(&log->relay.lock);
>
>-	if (intel_guc_log_relay_enabled(log)) {
>+	if (intel_guc_log_relay_created(log)) {
> 		ret = -EEXIST;
> 		goto out_unlock;
> 	}
>@@ -585,6 +586,21 @@ int intel_guc_log_relay_open(struct intel_guc_log *log)
>
> 	mutex_unlock(&log->relay.lock);
>
>+	return 0;
>+
>+out_relay:
>+	guc_log_relay_destroy(log);
>+out_unlock:
>+	mutex_unlock(&log->relay.lock);
>+
>+	return ret;
>+}
>+
>+int intel_guc_log_relay_start(struct intel_guc_log *log)
>+{
>+	if (log->relay.started)
>+		return -EEXIST;
>+
> 	guc_log_enable_flush_events(log);
>
> 	/*
>@@ -594,14 +610,9 @@ int intel_guc_log_relay_open(struct intel_guc_log *log)
> 	 */
> 	queue_work(system_highpri_wq, &log->relay.flush_work);
>
>-	return 0;
>-
>-out_relay:
>-	guc_log_relay_destroy(log);
>-out_unlock:
>-	mutex_unlock(&log->relay.lock);
>+	log->relay.started = true;
>
>-	return ret;
>+	return 0;
> }
>
> void intel_guc_log_relay_flush(struct intel_guc_log *log)
>@@ -610,6 +621,9 @@ void intel_guc_log_relay_flush(struct intel_guc_log *log)
> 	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
> 	intel_wakeref_t wakeref;
>
>+	if (!log->relay.started)
>+		return;
>+
> 	/*
> 	 * Before initiating the forceful flush, wait for any pending/ongoing
> 	 * flush to complete otherwise forceful flush may not actually happen.
>@@ -623,18 +637,33 @@ void intel_guc_log_relay_flush(struct intel_guc_log *log)
> 	guc_log_capture_logs(log);
> }
>
>-void intel_guc_log_relay_close(struct intel_guc_log *log)
>+/*
>+ * Stops the relay log. Called from intel_guc_log_relay_close(), so no
>+ * possibility of race with start/flush since relay_write cannot race
>+ * relay_close.
>+ */
>+static void guc_log_relay_stop(struct intel_guc_log *log)
> {
> 	struct intel_guc *guc = log_to_guc(log);
> 	struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
>
>+	if (!log->relay.started)
>+		return;
>+
> 	guc_log_disable_flush_events(log);
> 	intel_synchronize_irq(i915);
>
> 	flush_work(&log->relay.flush_work);
>
>+	log->relay.started = false;
>+}
>+
>+void intel_guc_log_relay_close(struct intel_guc_log *log)
>+{
>+	guc_log_relay_stop(log);
>+
> 	mutex_lock(&log->relay.lock);
>-	GEM_BUG_ON(!intel_guc_log_relay_enabled(log));
>+	GEM_BUG_ON(!intel_guc_log_relay_created(log));
> 	guc_log_unmap(log);
> 	guc_log_relay_destroy(log);
> 	mutex_unlock(&log->relay.lock);
>diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
>index 6f764879acb1..c252c022c5fc 100644
>--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
>+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_log.h
>@@ -47,6 +47,7 @@ struct intel_guc_log {
> 	struct i915_vma *vma;
> 	struct {
> 		void *buf_addr;
>+		bool started;
> 		struct work_struct flush_work;
> 		struct rchan *channel;
> 		struct mutex lock;
>@@ -65,8 +66,9 @@ int intel_guc_log_create(struct intel_guc_log *log);
> void intel_guc_log_destroy(struct intel_guc_log *log);
>
> int intel_guc_log_set_level(struct intel_guc_log *log, u32 level);
>-bool intel_guc_log_relay_enabled(const struct intel_guc_log *log);
>+bool intel_guc_log_relay_created(const struct intel_guc_log *log);
> int intel_guc_log_relay_open(struct intel_guc_log *log);
>+int intel_guc_log_relay_start(struct intel_guc_log *log);
> void intel_guc_log_relay_flush(struct intel_guc_log *log);
> void intel_guc_log_relay_close(struct intel_guc_log *log);
>
>diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>index 43db50095257..3f86f2b60b92 100644
>--- a/drivers/gpu/drm/i915/i915_debugfs.c
>+++ b/drivers/gpu/drm/i915/i915_debugfs.c
>@@ -1871,8 +1871,8 @@ static void i915_guc_log_info(struct seq_file *m,
> 	struct intel_guc_log *log = &dev_priv->gt.uc.guc.log;
> 	enum guc_log_buffer_type type;
>
>-	if (!intel_guc_log_relay_enabled(log)) {
>-		seq_puts(m, "GuC log relay disabled\n");
>+	if (!intel_guc_log_relay_created(log)) {
>+		seq_puts(m, "GuC log relay not created\n");
> 		return;
> 	}
>
>@@ -2059,9 +2059,23 @@ i915_guc_log_relay_write(struct file *filp,
> 			 loff_t *ppos)
> {
> 	struct intel_guc_log *log = filp->private_data;
>+	int val;
>+	int ret;
>
>-	intel_guc_log_relay_flush(log);
>-	return cnt;
>+	ret = kstrtoint_from_user(ubuf, cnt, 0, &val);
>+	if (ret < 0)
>+		return ret;
>+
>+	/*
>+	 * Enable and start the guc log relay on value of 1.
>+	 * Flush log relay for any other value.
>+	 */
>+	if (val == 1)
>+		ret = intel_guc_log_relay_start(log);
>+	else
>+		intel_guc_log_relay_flush(log);
>+
>+	return ret ?: cnt;
> }
>
> static int i915_guc_log_relay_release(struct inode *inode, struct file *file)
>-- 
>2.21.0.5.gaeb582a983
>