[Intel-gfx] [PATCH v2 12/15] drm/i915/guc: Don't print out relay statistics when relay is disabled
Sagar Arun Kamble
sagar.a.kamble at intel.com
Fri Mar 9 11:16:09 UTC 2018
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 at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko at 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 at 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
--
Thanks,
Sagar
More information about the Intel-gfx
mailing list