| Message ID | 20180227125230.13000-12-michal.winiarski@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 2/27/2018 6:22 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 reason for this? I think this was needed for verifying log flushes. > and tidy the debugfs formatting. > > 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 | 47 ++++++++++++++++++++++++------------ > drivers/gpu/drm/i915/intel_guc.c | 5 +--- > drivers/gpu/drm/i915/intel_guc_log.c | 24 +++++++++--------- > drivers/gpu/drm/i915/intel_guc_log.h | 12 +++++---- > 4 files changed, 51 insertions(+), 37 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 866d44a091b3..65f758f7c425 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]); > + if (!intel_guc_log_relay_enabled(guc)) { > + seq_puts(m, "GuC log relay disabled\n"); > + return; > + } > > - 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]); > + seq_puts(m, "GuC logging stats:\n"); > > - seq_printf(m, "\tTotal flush interrupt count: %u\n", > - guc->log.flush_interrupt_count); > + seq_printf(m, "\tRelay full count: %u\n", > + guc->log.relay_full_count); > > - seq_printf(m, "\tCapture miss count: %u\n", > - guc->log.capture_miss_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 e3b6ae158a12..0500b4164254 100644 > --- a/drivers/gpu/drm/i915/intel_guc.c > +++ b/drivers/gpu/drm/i915/intel_guc.c > @@ -388,12 +388,9 @@ void intel_guc_notification_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.flush_wq, > &guc->log.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 f95e18be1c5f..bdf6b3178488 100644 > --- a/drivers/gpu/drm/i915/intel_guc_log.c > +++ b/drivers/gpu/drm/i915/intel_guc_log.c > @@ -176,22 +176,22 @@ static void *guc_get_write_buffer(struct intel_guc *guc) > return relay_reserve(guc->log.relay_chan, 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].overflow += 16; Should be log->stats[type].sampled_overflow += 16; > } > DRM_ERROR_RATELIMITED("GuC log buffer overflow\n"); > } > @@ -224,7 +224,7 @@ static void guc_read_update_log_buffer(struct intel_guc *guc) > void *src_data, *dst_data; > bool new_overflow; > > - if (WARN_ON(!guc->log.buf_addr)) > + if (WARN_ON(!intel_guc_log_relay_enabled(guc))) > return; > > /* Get the pointer to shared GuC log buffer */ > @@ -239,7 +239,7 @@ static void guc_read_update_log_buffer(struct intel_guc *guc) > * getting consumed by User at a slow rate. > */ > DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n"); > - guc->log.capture_miss_count++; > + guc->log.relay_full_count++; > > return; > } > @@ -262,8 +262,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; > @@ -323,7 +323,7 @@ static void capture_logs_work(struct work_struct *work) > mutex_unlock(&guc->log.lock); > } > > -static bool guc_log_relay_enabled(struct intel_guc *guc) > +bool intel_guc_log_relay_enabled(struct intel_guc *guc) > { > return guc->log.buf_addr != NULL; > } > @@ -553,7 +553,7 @@ int intel_guc_log_relay_open(struct intel_guc *guc) > > mutex_lock(&guc->log.lock); > > - if (guc_log_relay_enabled(guc)) { > + if (intel_guc_log_relay_enabled(guc)) { > ret = -EEXIST; > goto out_unlock; > } > @@ -619,7 +619,7 @@ void intel_guc_log_relay_flush(struct intel_guc *guc) > > void intel_guc_log_relay_close(struct intel_guc *guc) > { > - GEM_BUG_ON(!guc_log_relay_enabled(guc)); > + GEM_BUG_ON(!intel_guc_log_relay_enabled(guc)); > > guc_log_flush_irq_disable(guc); > flush_work(&guc->log.flush_work); > diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h > index 11f49ef41f01..a8ccb04e2b0a 100644 > --- a/drivers/gpu/drm/i915/intel_guc_log.h > +++ b/drivers/gpu/drm/i915/intel_guc_log.h > @@ -48,11 +48,12 @@ struct intel_guc_log { > struct rchan *relay_chan; > struct mutex lock; > /* 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]; > + u32 relay_full_count; > + struct { > + u32 sampled_overflow; > + u32 overflow; > + u32 flush; > + } stats[GUC_MAX_LOG_BUFFER]; > }; > > int intel_guc_log_create(struct intel_guc *guc); > @@ -63,5 +64,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
On Tue, Mar 06, 2018 at 04:51:27PM +0530, Sagar Arun Kamble wrote: > > > On 2/27/2018 6:22 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 > reason for this? I think this was needed for verifying log flushes. I'm simply not a fan of random debugfs counters. Is someone really using this? We do have individual flush counts, do we really need to count the interrupts? > > and tidy the debugfs formatting. > > > > 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 | 47 ++++++++++++++++++++++++------------ > > drivers/gpu/drm/i915/intel_guc.c | 5 +--- > > drivers/gpu/drm/i915/intel_guc_log.c | 24 +++++++++--------- > > drivers/gpu/drm/i915/intel_guc_log.h | 12 +++++---- > > 4 files changed, 51 insertions(+), 37 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > > index 866d44a091b3..65f758f7c425 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]); > > + if (!intel_guc_log_relay_enabled(guc)) { > > + seq_puts(m, "GuC log relay disabled\n"); > > + return; > > + } > > - 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]); > > + seq_puts(m, "GuC logging stats:\n"); > > - seq_printf(m, "\tTotal flush interrupt count: %u\n", > > - guc->log.flush_interrupt_count); > > + seq_printf(m, "\tRelay full count: %u\n", > > + guc->log.relay_full_count); > > - seq_printf(m, "\tCapture miss count: %u\n", > > - guc->log.capture_miss_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 e3b6ae158a12..0500b4164254 100644 > > --- a/drivers/gpu/drm/i915/intel_guc.c > > +++ b/drivers/gpu/drm/i915/intel_guc.c > > @@ -388,12 +388,9 @@ void intel_guc_notification_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.flush_wq, > > &guc->log.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 f95e18be1c5f..bdf6b3178488 100644 > > --- a/drivers/gpu/drm/i915/intel_guc_log.c > > +++ b/drivers/gpu/drm/i915/intel_guc_log.c > > @@ -176,22 +176,22 @@ static void *guc_get_write_buffer(struct intel_guc *guc) > > return relay_reserve(guc->log.relay_chan, 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].overflow += 16; > Should be > log->stats[type].sampled_overflow += 16; Oops. -Michał > > } > > DRM_ERROR_RATELIMITED("GuC log buffer overflow\n"); > > } > > @@ -224,7 +224,7 @@ static void guc_read_update_log_buffer(struct intel_guc *guc) > > void *src_data, *dst_data; > > bool new_overflow; > > - if (WARN_ON(!guc->log.buf_addr)) > > + if (WARN_ON(!intel_guc_log_relay_enabled(guc))) > > return; > > /* Get the pointer to shared GuC log buffer */ > > @@ -239,7 +239,7 @@ static void guc_read_update_log_buffer(struct intel_guc *guc) > > * getting consumed by User at a slow rate. > > */ > > DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n"); > > - guc->log.capture_miss_count++; > > + guc->log.relay_full_count++; > > return; > > } > > @@ -262,8 +262,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; > > @@ -323,7 +323,7 @@ static void capture_logs_work(struct work_struct *work) > > mutex_unlock(&guc->log.lock); > > } > > -static bool guc_log_relay_enabled(struct intel_guc *guc) > > +bool intel_guc_log_relay_enabled(struct intel_guc *guc) > > { > > return guc->log.buf_addr != NULL; > > } > > @@ -553,7 +553,7 @@ int intel_guc_log_relay_open(struct intel_guc *guc) > > mutex_lock(&guc->log.lock); > > - if (guc_log_relay_enabled(guc)) { > > + if (intel_guc_log_relay_enabled(guc)) { > > ret = -EEXIST; > > goto out_unlock; > > } > > @@ -619,7 +619,7 @@ void intel_guc_log_relay_flush(struct intel_guc *guc) > > void intel_guc_log_relay_close(struct intel_guc *guc) > > { > > - GEM_BUG_ON(!guc_log_relay_enabled(guc)); > > + GEM_BUG_ON(!intel_guc_log_relay_enabled(guc)); > > guc_log_flush_irq_disable(guc); > > flush_work(&guc->log.flush_work); > > diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h > > index 11f49ef41f01..a8ccb04e2b0a 100644 > > --- a/drivers/gpu/drm/i915/intel_guc_log.h > > +++ b/drivers/gpu/drm/i915/intel_guc_log.h > > @@ -48,11 +48,12 @@ struct intel_guc_log { > > struct rchan *relay_chan; > > struct mutex lock; > > /* 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]; > > + u32 relay_full_count; > > + struct { > > + u32 sampled_overflow; > > + u32 overflow; > > + u32 flush; > > + } stats[GUC_MAX_LOG_BUFFER]; > > }; > > int intel_guc_log_create(struct intel_guc *guc); > > @@ -63,5 +64,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 >
I think we should also clear the stats.count and relay_full_count on relay_open. Thanks Sagar On 2/27/2018 6:22 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. > > 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 | 47 ++++++++++++++++++++++++------------ > drivers/gpu/drm/i915/intel_guc.c | 5 +--- > drivers/gpu/drm/i915/intel_guc_log.c | 24 +++++++++--------- > drivers/gpu/drm/i915/intel_guc_log.h | 12 +++++---- > 4 files changed, 51 insertions(+), 37 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c > index 866d44a091b3..65f758f7c425 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]); > + if (!intel_guc_log_relay_enabled(guc)) { > + seq_puts(m, "GuC log relay disabled\n"); > + return; > + } > > - 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]); > + seq_puts(m, "GuC logging stats:\n"); > > - seq_printf(m, "\tTotal flush interrupt count: %u\n", > - guc->log.flush_interrupt_count); > + seq_printf(m, "\tRelay full count: %u\n", > + guc->log.relay_full_count); > > - seq_printf(m, "\tCapture miss count: %u\n", > - guc->log.capture_miss_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 e3b6ae158a12..0500b4164254 100644 > --- a/drivers/gpu/drm/i915/intel_guc.c > +++ b/drivers/gpu/drm/i915/intel_guc.c > @@ -388,12 +388,9 @@ void intel_guc_notification_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.flush_wq, > &guc->log.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 f95e18be1c5f..bdf6b3178488 100644 > --- a/drivers/gpu/drm/i915/intel_guc_log.c > +++ b/drivers/gpu/drm/i915/intel_guc_log.c > @@ -176,22 +176,22 @@ static void *guc_get_write_buffer(struct intel_guc *guc) > return relay_reserve(guc->log.relay_chan, 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].overflow += 16; > } > DRM_ERROR_RATELIMITED("GuC log buffer overflow\n"); > } > @@ -224,7 +224,7 @@ static void guc_read_update_log_buffer(struct intel_guc *guc) > void *src_data, *dst_data; > bool new_overflow; > > - if (WARN_ON(!guc->log.buf_addr)) > + if (WARN_ON(!intel_guc_log_relay_enabled(guc))) > return; > > /* Get the pointer to shared GuC log buffer */ > @@ -239,7 +239,7 @@ static void guc_read_update_log_buffer(struct intel_guc *guc) > * getting consumed by User at a slow rate. > */ > DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n"); > - guc->log.capture_miss_count++; > + guc->log.relay_full_count++; > > return; > } > @@ -262,8 +262,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; > @@ -323,7 +323,7 @@ static void capture_logs_work(struct work_struct *work) > mutex_unlock(&guc->log.lock); > } > > -static bool guc_log_relay_enabled(struct intel_guc *guc) > +bool intel_guc_log_relay_enabled(struct intel_guc *guc) > { > return guc->log.buf_addr != NULL; > } > @@ -553,7 +553,7 @@ int intel_guc_log_relay_open(struct intel_guc *guc) > > mutex_lock(&guc->log.lock); > > - if (guc_log_relay_enabled(guc)) { > + if (intel_guc_log_relay_enabled(guc)) { > ret = -EEXIST; > goto out_unlock; > } > @@ -619,7 +619,7 @@ void intel_guc_log_relay_flush(struct intel_guc *guc) > > void intel_guc_log_relay_close(struct intel_guc *guc) > { > - GEM_BUG_ON(!guc_log_relay_enabled(guc)); > + GEM_BUG_ON(!intel_guc_log_relay_enabled(guc)); > > guc_log_flush_irq_disable(guc); > flush_work(&guc->log.flush_work); > diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h > index 11f49ef41f01..a8ccb04e2b0a 100644 > --- a/drivers/gpu/drm/i915/intel_guc_log.h > +++ b/drivers/gpu/drm/i915/intel_guc_log.h > @@ -48,11 +48,12 @@ struct intel_guc_log { > struct rchan *relay_chan; > struct mutex lock; > /* 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]; > + u32 relay_full_count; > + struct { > + u32 sampled_overflow; > + u32 overflow; > + u32 flush; > + } stats[GUC_MAX_LOG_BUFFER]; > }; > > int intel_guc_log_create(struct intel_guc *guc); > @@ -63,5 +64,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 --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 866d44a091b3..65f758f7c425 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]); + if (!intel_guc_log_relay_enabled(guc)) { + seq_puts(m, "GuC log relay disabled\n"); + return; + } - 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]); + seq_puts(m, "GuC logging stats:\n"); - seq_printf(m, "\tTotal flush interrupt count: %u\n", - guc->log.flush_interrupt_count); + seq_printf(m, "\tRelay full count: %u\n", + guc->log.relay_full_count); - seq_printf(m, "\tCapture miss count: %u\n", - guc->log.capture_miss_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 e3b6ae158a12..0500b4164254 100644 --- a/drivers/gpu/drm/i915/intel_guc.c +++ b/drivers/gpu/drm/i915/intel_guc.c @@ -388,12 +388,9 @@ void intel_guc_notification_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.flush_wq, &guc->log.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 f95e18be1c5f..bdf6b3178488 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.c +++ b/drivers/gpu/drm/i915/intel_guc_log.c @@ -176,22 +176,22 @@ static void *guc_get_write_buffer(struct intel_guc *guc) return relay_reserve(guc->log.relay_chan, 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].overflow += 16; } DRM_ERROR_RATELIMITED("GuC log buffer overflow\n"); } @@ -224,7 +224,7 @@ static void guc_read_update_log_buffer(struct intel_guc *guc) void *src_data, *dst_data; bool new_overflow; - if (WARN_ON(!guc->log.buf_addr)) + if (WARN_ON(!intel_guc_log_relay_enabled(guc))) return; /* Get the pointer to shared GuC log buffer */ @@ -239,7 +239,7 @@ static void guc_read_update_log_buffer(struct intel_guc *guc) * getting consumed by User at a slow rate. */ DRM_ERROR_RATELIMITED("no sub-buffer to capture logs\n"); - guc->log.capture_miss_count++; + guc->log.relay_full_count++; return; } @@ -262,8 +262,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; @@ -323,7 +323,7 @@ static void capture_logs_work(struct work_struct *work) mutex_unlock(&guc->log.lock); } -static bool guc_log_relay_enabled(struct intel_guc *guc) +bool intel_guc_log_relay_enabled(struct intel_guc *guc) { return guc->log.buf_addr != NULL; } @@ -553,7 +553,7 @@ int intel_guc_log_relay_open(struct intel_guc *guc) mutex_lock(&guc->log.lock); - if (guc_log_relay_enabled(guc)) { + if (intel_guc_log_relay_enabled(guc)) { ret = -EEXIST; goto out_unlock; } @@ -619,7 +619,7 @@ void intel_guc_log_relay_flush(struct intel_guc *guc) void intel_guc_log_relay_close(struct intel_guc *guc) { - GEM_BUG_ON(!guc_log_relay_enabled(guc)); + GEM_BUG_ON(!intel_guc_log_relay_enabled(guc)); guc_log_flush_irq_disable(guc); flush_work(&guc->log.flush_work); diff --git a/drivers/gpu/drm/i915/intel_guc_log.h b/drivers/gpu/drm/i915/intel_guc_log.h index 11f49ef41f01..a8ccb04e2b0a 100644 --- a/drivers/gpu/drm/i915/intel_guc_log.h +++ b/drivers/gpu/drm/i915/intel_guc_log.h @@ -48,11 +48,12 @@ struct intel_guc_log { struct rchan *relay_chan; struct mutex lock; /* 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]; + u32 relay_full_count; + struct { + u32 sampled_overflow; + u32 overflow; + u32 flush; + } stats[GUC_MAX_LOG_BUFFER]; }; int intel_guc_log_create(struct intel_guc *guc); @@ -63,5 +64,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
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. 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 | 47 ++++++++++++++++++++++++------------ drivers/gpu/drm/i915/intel_guc.c | 5 +--- drivers/gpu/drm/i915/intel_guc_log.c | 24 +++++++++--------- drivers/gpu/drm/i915/intel_guc_log.h | 12 +++++---- 4 files changed, 51 insertions(+), 37 deletions(-)