Re: [PATCH 14/17] drm/i915: Add stats for GuC log buffer flush interrupts

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 15/07/16 16:58, Goel, Akash wrote:
> On 7/15/2016 5:21 PM, Tvrtko Ursulin wrote:
>> On 10/07/16 14:41, akash.goel@intel.com wrote:
>>> From: Akash Goel <akash.goel@intel.com>
>>>
>>> GuC firmware sends an interrupt to flush the log buffer when it
>>> becomes half full. GuC firmware also tracks how many times the
>>> buffer overflowed.
>>> It would be useful to maintain a statistics of how many flush
>>> interrupts were received and for which type of log buffer,
>>> along with the overflow count of each buffer type.
>>> Augmented i915_log_info debugfs to report back these statistics.
>>>
>>> Signed-off-by: Akash Goel <akash.goel@intel.com>
>>> ---
>>>   drivers/gpu/drm/i915/i915_debugfs.c        | 26
>>> ++++++++++++++++++++++++++
>>>   drivers/gpu/drm/i915/i915_guc_submission.c |  8 ++++++++
>>>   drivers/gpu/drm/i915/i915_irq.c            |  1 +
>>>   drivers/gpu/drm/i915/intel_guc.h           |  6 ++++++
>>>   4 files changed, 41 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c
>>> b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index 3c9c7f7..888a18a 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -2538,6 +2538,30 @@ static int i915_guc_load_status_info(struct
>>> seq_file *m, void *data)
>>>       return 0;
>>>   }
>>>
>>> +static void i915_guc_log_info(struct seq_file *m,
>>> +                 struct drm_i915_private *dev_priv)
>>> +{
>>> +    struct intel_guc *guc = &dev_priv->guc;
>>> +
>>> +    seq_printf(m, "\nGuC logging stats:\n");
>>> +
>>> +    seq_printf(m, "\tISR:   flush count %10u, overflow count %8u\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 %8u\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 %8u\n",
>>> +        guc->log.flush_count[GUC_CRASH_DUMP_LOG_BUFFER],
>>> +        guc->log.total_overflow_count[GUC_CRASH_DUMP_LOG_BUFFER]);
>>> +
>>> +    seq_printf(m, "\tTotal flush interrupt count: %u\n",
>>> +            guc->log.flush_interrupt_count);
>>> +
>>> +}
>>> +
>>>   static void i915_guc_client_info(struct seq_file *m,
>>>                    struct drm_i915_private *dev_priv,
>>>                    struct i915_guc_client *client)
>>> @@ -2611,6 +2635,8 @@ static int i915_guc_info(struct seq_file *m,
>>> void *data)
>>>       seq_printf(m, "\nGuC execbuf client @ %p:\n", guc.execbuf_client);
>>>       i915_guc_client_info(m, dev_priv, &client);
>>>
>>> +    i915_guc_log_info(m, dev_priv);
>>> +
>>>       /* Add more as required ... */
>>>
>>>       return 0;
>>> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c
>>> b/drivers/gpu/drm/i915/i915_guc_submission.c
>>> index c1e637f..9c94a43 100644
>>> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
>>> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
>>> @@ -914,6 +914,14 @@ static void guc_read_update_log_buffer(struct
>>> drm_device *dev)
>>>           log_buffer_state_local = *log_buffer_state;
>>>           buffer_size = log_buffer_state_local.size;
>>>
>>> +        guc->log.flush_count[i] +=
>>> log_buffer_state_local.flush_to_file;
>>> +        if (log_buffer_state_local.buffer_full_cnt !=
>>> +                    guc->log.prev_overflow_count[i]) {
>>> +            guc->log.prev_overflow_count[i] =
>>> +                    log_buffer_state_local.buffer_full_cnt;
>>> +            guc->log.total_overflow_count[i]++;
>>
>> Is log_buffer_state_local.buffer_full_cnt guaranteed to be one here? Or
>> you would need to increase total_overflow_count by its value?
>>
>
> buffer_full_cnt will not remain as one. Its a 4 bit counter, will be
> incremented monotonically by GuC firmware on every new detection of
> overflow, so will increase from 0 to 15 & then wrap around.
> Hence have to use '!=' in the condition instead of '>'.

But can it happen that it jumps by more than one between being sampled 
here? In which case you would need to replace:

guc->log.total_overflow_count[i]++;

by something like:


guc->log.total_overflow_count[i] += 
log_buffer_state_local.buffer_full_cnt - guc->log.prev_overflow_count[i];

(Doesn't handle the wrap though, just to illustrate my point.)

Regards,

Tvrtko




_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx