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

2019-09-16 Thread Robert M. Fosha
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 
Cc: Daniele Ceraolo Spurio 
Cc: Michal Wajdeczko 
Signed-off-by: Robert M. Fosha 
---
 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(-)

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 

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

2019-09-17 Thread 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 
Cc: Daniele Ceraolo Spurio 
Cc: Michal Wajdeczko 
Signed-off-by: Robert M. Fosha 
---
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 


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_w