diff mbox

[v2,12/15] drm/i915/guc: Don't print out relay statistics when relay is disabled

Message ID 20180308154707.21716-12-michal.winiarski@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Michał Winiarski March 8, 2018, 3:47 p.m. UTC 
If nobody has enabled the relay, we're not comunicating with GuC, which
means that the stats don't have any meaning. Let's also remove interrupt
counter and tidy the debugfs formatting.

v2: Correct stats accounting (Sagar)

Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  | 45 ++++++++++++++++++++++++------------
 drivers/gpu/drm/i915/intel_guc.c     |  5 +---
 drivers/gpu/drm/i915/intel_guc_log.c | 22 +++++++++---------
 drivers/gpu/drm/i915/intel_guc_log.h | 10 ++++----
 4 files changed, 48 insertions(+), 34 deletions(-)

Comments

sagar.a.kamble@intel.com March 9, 2018, 11:16 a.m. UTC | #1 
On 3/8/2018 9:17 PM, Michał Winiarski wrote:
> If nobody has enabled the relay, we're not comunicating with GuC, which
> means that the stats don't have any meaning. Let's also remove interrupt
> counter and tidy the debugfs formatting.
>
> v2: Correct stats accounting (Sagar)
>
> Signed-off-by: Michał Winiarski <michal.winiarski@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> ---
>   drivers/gpu/drm/i915/i915_debugfs.c  | 45 ++++++++++++++++++++++++------------
>   drivers/gpu/drm/i915/intel_guc.c     |  5 +---
>   drivers/gpu/drm/i915/intel_guc_log.c | 22 +++++++++---------
>   drivers/gpu/drm/i915/intel_guc_log.h | 10 ++++----
>   4 files changed, 48 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index d29bacb1f308..94516ab4eaed 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2326,30 +2326,45 @@ static int i915_guc_load_status_info(struct seq_file *m, void *data)
>   	return 0;
>   }
>   
> +static const char *
> +stringify_guc_log_type(enum guc_log_buffer_type type)
> +{
> +	switch (type) {
> +	case GUC_ISR_LOG_BUFFER:
> +		return "ISR";
> +	case GUC_DPC_LOG_BUFFER:
> +		return "DPC";
> +	case GUC_CRASH_DUMP_LOG_BUFFER:
> +		return "CRASH";
> +	default:
> +		MISSING_CASE(type);
> +	}
> +
> +	return "";
> +}
> +
>   static void i915_guc_log_info(struct seq_file *m,
>   			      struct drm_i915_private *dev_priv)
>   {
>   	struct intel_guc *guc = &dev_priv->guc;
> +	enum guc_log_buffer_type type;
>   
> -	seq_puts(m, "GuC logging stats:\n");
> -
> -	seq_printf(m, "\tISR:   flush count %10u, overflow count %10u\n",
> -		   guc->log.flush_count[GUC_ISR_LOG_BUFFER],
> -		   guc->log.total_overflow_count[GUC_ISR_LOG_BUFFER]);
> -
> -	seq_printf(m, "\tDPC:   flush count %10u, overflow count %10u\n",
> -		   guc->log.flush_count[GUC_DPC_LOG_BUFFER],
> -		   guc->log.total_overflow_count[GUC_DPC_LOG_BUFFER]);
> -
> -	seq_printf(m, "\tCRASH: flush count %10u, overflow count %10u\n",
> -		   guc->log.flush_count[GUC_CRASH_DUMP_LOG_BUFFER],
> -		   guc->log.total_overflow_count[GUC_CRASH_DUMP_LOG_BUFFER]);
> +	if (!intel_guc_log_relay_enabled(guc)) {
> +		seq_puts(m, "GuC log relay disabled\n");
> +		return;
> +	}
>   
> -	seq_printf(m, "\tTotal flush interrupt count: %u\n",
> -		   guc->log.flush_interrupt_count);
> +	seq_puts(m, "GuC logging stats:\n");
>   
>   	seq_printf(m, "\tRelay full count: %u\n",
>   		   guc->log.relay.full_count);
> +
> +	for (type = GUC_ISR_LOG_BUFFER; type < GUC_MAX_LOG_BUFFER; type++) {
> +		seq_printf(m, "\t%s:\tflush count %10u, overflow count %10u\n",
> +			   stringify_guc_log_type(type),
> +			   guc->log.stats[type].flush,
> +			   guc->log.stats[type].overflow);
Didn't see this earlier. Should be sampled_overflow.
And ordering of intel_guc_log_relay_enabled() declaration w.r.t other 
declarations needs to be fixed.
With that:
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble@intel.com>
> +	}
>   }
>   
>   static void i915_guc_client_info(struct seq_file *m,
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index cab158e42577..3e2f0f8503ed 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -388,12 +388,9 @@ void intel_guc_to_host_event_handler(struct intel_guc *guc)
>   	spin_unlock(&guc->irq_lock);
>   
>   	if (msg & (INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
> -		   INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED)) {
> +		   INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED))
>   		queue_work(guc->log.relay.flush_wq,
>   			   &guc->log.relay.flush_work);
> -
> -		guc->log.flush_interrupt_count++;
> -	}
>   }
>   
>   int intel_guc_sample_forcewake(struct intel_guc *guc)
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
> index 7c4339dae534..a72fe4a369f4 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.c
> +++ b/drivers/gpu/drm/i915/intel_guc_log.c
> @@ -171,22 +171,22 @@ static void *guc_get_write_buffer(struct intel_guc *guc)
>   	return relay_reserve(guc->log.relay.channel, 0);
>   }
>   
> -static bool guc_check_log_buf_overflow(struct intel_guc *guc,
> +static bool guc_check_log_buf_overflow(struct intel_guc_log *log,
>   				       enum guc_log_buffer_type type,
>   				       unsigned int full_cnt)
>   {
> -	unsigned int prev_full_cnt = guc->log.prev_overflow_count[type];
> +	unsigned int prev_full_cnt = log->stats[type].sampled_overflow;
>   	bool overflow = false;
>   
>   	if (full_cnt != prev_full_cnt) {
>   		overflow = true;
>   
> -		guc->log.prev_overflow_count[type] = full_cnt;
> -		guc->log.total_overflow_count[type] += full_cnt - prev_full_cnt;
> +		log->stats[type].overflow = full_cnt;
> +		log->stats[type].sampled_overflow += full_cnt - prev_full_cnt;
>   
>   		if (full_cnt < prev_full_cnt) {
>   			/* buffer_full_cnt is a 4 bit counter */
> -			guc->log.total_overflow_count[type] += 16;
> +			log->stats[type].sampled_overflow += 16;
>   		}
>   		DRM_ERROR_RATELIMITED("GuC log buffer overflow\n");
>   	}
> @@ -221,7 +221,7 @@ static void guc_read_update_log_buffer(struct intel_guc *guc)
>   
>   	mutex_lock(&guc->log.relay.lock);
>   
> -	if (WARN_ON(!guc->log.relay.buf_addr))
> +	if (WARN_ON(!intel_guc_log_relay_enabled(guc)))
>   		goto out_unlock;
>   
>   	/* Get the pointer to shared GuC log buffer */
> @@ -259,8 +259,8 @@ static void guc_read_update_log_buffer(struct intel_guc *guc)
>   		full_cnt = log_buf_state_local.buffer_full_cnt;
>   
>   		/* Bookkeeping stuff */
> -		guc->log.flush_count[type] += log_buf_state_local.flush_to_file;
> -		new_overflow = guc_check_log_buf_overflow(guc, type, full_cnt);
> +		guc->log.stats[type].flush += log_buf_state_local.flush_to_file;
> +		new_overflow = guc_check_log_buf_overflow(&guc->log, type, full_cnt);
>   
>   		/* Update the state of shared log buffer */
>   		log_buf_state->read_ptr = write_offset;
> @@ -321,7 +321,7 @@ static void capture_logs_work(struct work_struct *work)
>   	guc_log_capture_logs(guc);
>   }
>   
> -static bool guc_log_relay_enabled(struct intel_guc *guc)
> +bool intel_guc_log_relay_enabled(struct intel_guc *guc)
>   {
>   	return guc->log.relay.buf_addr != NULL;
>   }
> @@ -555,7 +555,7 @@ int intel_guc_log_relay_open(struct intel_guc *guc)
>   
>   	mutex_lock(&guc->log.relay.lock);
>   
> -	if (guc_log_relay_enabled(guc)) {
> +	if (intel_guc_log_relay_enabled(guc)) {
>   		ret = -EEXIST;
>   		goto out_unlock;
>   	}
> @@ -623,7 +623,7 @@ void intel_guc_log_relay_close(struct intel_guc *guc)
>   	flush_work(&guc->log.relay.flush_work);
>   
>   	mutex_lock(&guc->log.relay.lock);
> -	GEM_BUG_ON(!guc_log_relay_enabled(guc));
> +	GEM_BUG_ON(!intel_guc_log_relay_enabled(guc));
>   	guc_log_unmap(guc);
>   	guc_log_relay_destroy(guc);
>   	mutex_unlock(&guc->log.relay.lock);
> diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h
> index 9b1257ea2673..8804c5c27409 100644
> --- a/drivers/gpu/drm/i915/intel_guc_log.h
> +++ b/drivers/gpu/drm/i915/intel_guc_log.h
> @@ -51,10 +51,11 @@ struct intel_guc_log {
>   		u32 full_count;
>   	} relay;
>   	/* logging related stats */
> -	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 {
> +		u32 sampled_overflow;
> +		u32 overflow;
> +		u32 flush;
> +	} stats[GUC_MAX_LOG_BUFFER];
>   };
>   
>   int intel_guc_log_create(struct intel_guc *guc);
> @@ -65,5 +66,6 @@ int intel_guc_log_level_set(struct intel_guc *guc, u64 control_val);
>   int intel_guc_log_relay_open(struct intel_guc *guc);
>   void intel_guc_log_relay_close(struct intel_guc *guc);
>   void intel_guc_log_relay_flush(struct intel_guc *guc);
> +bool intel_guc_log_relay_enabled(struct intel_guc *guc);
>   
>   #endif
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index d29bacb1f308..94516ab4eaed 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2326,30 +2326,45 @@  static int i915_guc_load_status_info(struct seq_file *m, void *data)
 	return 0;
 }
 
+static const char *
+stringify_guc_log_type(enum guc_log_buffer_type type)
+{
+	switch (type) {
+	case GUC_ISR_LOG_BUFFER:
+		return "ISR";
+	case GUC_DPC_LOG_BUFFER:
+		return "DPC";
+	case GUC_CRASH_DUMP_LOG_BUFFER:
+		return "CRASH";
+	default:
+		MISSING_CASE(type);
+	}
+
+	return "";
+}
+
 static void i915_guc_log_info(struct seq_file *m,
 			      struct drm_i915_private *dev_priv)
 {
 	struct intel_guc *guc = &dev_priv->guc;
+	enum guc_log_buffer_type type;
 
-	seq_puts(m, "GuC logging stats:\n");
-
-	seq_printf(m, "\tISR:   flush count %10u, overflow count %10u\n",
-		   guc->log.flush_count[GUC_ISR_LOG_BUFFER],
-		   guc->log.total_overflow_count[GUC_ISR_LOG_BUFFER]);
-
-	seq_printf(m, "\tDPC:   flush count %10u, overflow count %10u\n",
-		   guc->log.flush_count[GUC_DPC_LOG_BUFFER],
-		   guc->log.total_overflow_count[GUC_DPC_LOG_BUFFER]);
-
-	seq_printf(m, "\tCRASH: flush count %10u, overflow count %10u\n",
-		   guc->log.flush_count[GUC_CRASH_DUMP_LOG_BUFFER],
-		   guc->log.total_overflow_count[GUC_CRASH_DUMP_LOG_BUFFER]);
+	if (!intel_guc_log_relay_enabled(guc)) {
+		seq_puts(m, "GuC log relay disabled\n");
+		return;
+	}
 
-	seq_printf(m, "\tTotal flush interrupt count: %u\n",
-		   guc->log.flush_interrupt_count);
+	seq_puts(m, "GuC logging stats:\n");
 
 	seq_printf(m, "\tRelay full count: %u\n",
 		   guc->log.relay.full_count);
+
+	for (type = GUC_ISR_LOG_BUFFER; type < GUC_MAX_LOG_BUFFER; type++) {
+		seq_printf(m, "\t%s:\tflush count %10u, overflow count %10u\n",
+			   stringify_guc_log_type(type),
+			   guc->log.stats[type].flush,
+			   guc->log.stats[type].overflow);
+	}
 }
 
 static void i915_guc_client_info(struct seq_file *m,
diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index cab158e42577..3e2f0f8503ed 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -388,12 +388,9 @@  void intel_guc_to_host_event_handler(struct intel_guc *guc)
 	spin_unlock(&guc->irq_lock);
 
 	if (msg & (INTEL_GUC_RECV_MSG_FLUSH_LOG_BUFFER |
-		   INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED)) {
+		   INTEL_GUC_RECV_MSG_CRASH_DUMP_POSTED))
 		queue_work(guc->log.relay.flush_wq,
 			   &guc->log.relay.flush_work);
-
-		guc->log.flush_interrupt_count++;
-	}
 }
 
 int intel_guc_sample_forcewake(struct intel_guc *guc)
diff --git a/drivers/gpu/drm/i915/intel_guc_log.c b/drivers/gpu/drm/i915/intel_guc_log.c
index 7c4339dae534..a72fe4a369f4 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.c
+++ b/drivers/gpu/drm/i915/intel_guc_log.c
@@ -171,22 +171,22 @@  static void *guc_get_write_buffer(struct intel_guc *guc)
 	return relay_reserve(guc->log.relay.channel, 0);
 }
 
-static bool guc_check_log_buf_overflow(struct intel_guc *guc,
+static bool guc_check_log_buf_overflow(struct intel_guc_log *log,
 				       enum guc_log_buffer_type type,
 				       unsigned int full_cnt)
 {
-	unsigned int prev_full_cnt = guc->log.prev_overflow_count[type];
+	unsigned int prev_full_cnt = log->stats[type].sampled_overflow;
 	bool overflow = false;
 
 	if (full_cnt != prev_full_cnt) {
 		overflow = true;
 
-		guc->log.prev_overflow_count[type] = full_cnt;
-		guc->log.total_overflow_count[type] += full_cnt - prev_full_cnt;
+		log->stats[type].overflow = full_cnt;
+		log->stats[type].sampled_overflow += full_cnt - prev_full_cnt;
 
 		if (full_cnt < prev_full_cnt) {
 			/* buffer_full_cnt is a 4 bit counter */
-			guc->log.total_overflow_count[type] += 16;
+			log->stats[type].sampled_overflow += 16;
 		}
 		DRM_ERROR_RATELIMITED("GuC log buffer overflow\n");
 	}
@@ -221,7 +221,7 @@  static void guc_read_update_log_buffer(struct intel_guc *guc)
 
 	mutex_lock(&guc->log.relay.lock);
 
-	if (WARN_ON(!guc->log.relay.buf_addr))
+	if (WARN_ON(!intel_guc_log_relay_enabled(guc)))
 		goto out_unlock;
 
 	/* Get the pointer to shared GuC log buffer */
@@ -259,8 +259,8 @@  static void guc_read_update_log_buffer(struct intel_guc *guc)
 		full_cnt = log_buf_state_local.buffer_full_cnt;
 
 		/* Bookkeeping stuff */
-		guc->log.flush_count[type] += log_buf_state_local.flush_to_file;
-		new_overflow = guc_check_log_buf_overflow(guc, type, full_cnt);
+		guc->log.stats[type].flush += log_buf_state_local.flush_to_file;
+		new_overflow = guc_check_log_buf_overflow(&guc->log, type, full_cnt);
 
 		/* Update the state of shared log buffer */
 		log_buf_state->read_ptr = write_offset;
@@ -321,7 +321,7 @@  static void capture_logs_work(struct work_struct *work)
 	guc_log_capture_logs(guc);
 }
 
-static bool guc_log_relay_enabled(struct intel_guc *guc)
+bool intel_guc_log_relay_enabled(struct intel_guc *guc)
 {
 	return guc->log.relay.buf_addr != NULL;
 }
@@ -555,7 +555,7 @@  int intel_guc_log_relay_open(struct intel_guc *guc)
 
 	mutex_lock(&guc->log.relay.lock);
 
-	if (guc_log_relay_enabled(guc)) {
+	if (intel_guc_log_relay_enabled(guc)) {
 		ret = -EEXIST;
 		goto out_unlock;
 	}
@@ -623,7 +623,7 @@  void intel_guc_log_relay_close(struct intel_guc *guc)
 	flush_work(&guc->log.relay.flush_work);
 
 	mutex_lock(&guc->log.relay.lock);
-	GEM_BUG_ON(!guc_log_relay_enabled(guc));
+	GEM_BUG_ON(!intel_guc_log_relay_enabled(guc));
 	guc_log_unmap(guc);
 	guc_log_relay_destroy(guc);
 	mutex_unlock(&guc->log.relay.lock);
diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h
index 9b1257ea2673..8804c5c27409 100644
--- a/drivers/gpu/drm/i915/intel_guc_log.h
+++ b/drivers/gpu/drm/i915/intel_guc_log.h
@@ -51,10 +51,11 @@  struct intel_guc_log {
 		u32 full_count;
 	} relay;
 	/* logging related stats */
-	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 {
+		u32 sampled_overflow;
+		u32 overflow;
+		u32 flush;
+	} stats[GUC_MAX_LOG_BUFFER];
 };
 
 int intel_guc_log_create(struct intel_guc *guc);
@@ -65,5 +66,6 @@  int intel_guc_log_level_set(struct intel_guc *guc, u64 control_val);
 int intel_guc_log_relay_open(struct intel_guc *guc);
 void intel_guc_log_relay_close(struct intel_guc *guc);
 void intel_guc_log_relay_flush(struct intel_guc *guc);
+bool intel_guc_log_relay_enabled(struct intel_guc *guc);
 
 #endif