On Tue, Aug 03, 2021 at 03:29:22PM -0700, Matthew Brost wrote:
> Display the workqueue status in debugfs for GuC contexts that are in
> parent-child relationship.
>
> Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx>
> ---
> .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 56 +++++++++++++------
> 1 file changed, 39 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> index 30df1c8db491..44a7582c9aed 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
> @@ -4527,31 +4527,53 @@ void intel_guc_submission_print_info(struct intel_guc *guc,
> gse_log_submission_info(guc->gse[i], p, i);
> }
>
> +static inline void guc_log_context(struct drm_printer *p,
> + struct intel_context *ce)
> +{
> + drm_printf(p, "GuC lrc descriptor %u:\n", ce->guc_id);
> + drm_printf(p, "\tHW Context Desc: 0x%08x\n", ce->lrc.lrca);
> + drm_printf(p, "\t\tLRC Head: Internal %u, Memory %u\n",
> + ce->ring->head,
> + ce->lrc_reg_state[CTX_RING_HEAD]);
> + drm_printf(p, "\t\tLRC Tail: Internal %u, Memory %u\n",
> + ce->ring->tail,
> + ce->lrc_reg_state[CTX_RING_TAIL]);
> + drm_printf(p, "\t\tContext Pin Count: %u\n",
> + atomic_read(&ce->pin_count));
> + drm_printf(p, "\t\tGuC ID Ref Count: %u\n",
> + atomic_read(&ce->guc_id_ref));
> + drm_printf(p, "\t\tNumber Requests Not Ready: %u\n",
> + atomic_read(&ce->guc_num_rq_not_ready));
> + drm_printf(p, "\t\tSchedule State: 0x%x, 0x%x\n\n",
> + ce->guc_state.sched_state,
> + atomic_read(&ce->guc_sched_state_no_lock));
It's all debugfs, but I think proper locking even there is good. It at
least reduces the confusion when the locking scheme is largely
undocumented. Also given how much we have rcu for everything would be good
to double-check all pointer dererences are properly protected.
> +}
> +
> void intel_guc_submission_print_context_info(struct intel_guc *guc,
> struct drm_printer *p)
> {
> struct intel_context *ce;
> unsigned long index;
> xa_for_each(&guc->context_lookup, index, ce) {
xa_for_each doesn't provide any guarantees, so doesn't protect against
concurrent removeal or anything like that. We need to do better than that.
-Daniel
> - drm_printf(p, "GuC lrc descriptor %u:\n", ce->guc_id);
> - drm_printf(p, "\tHW Context Desc: 0x%08x\n", ce->lrc.lrca);
> - drm_printf(p, "\t\tLRC Head: Internal %u, Memory %u\n",
> - ce->ring->head,
> - ce->lrc_reg_state[CTX_RING_HEAD]);
> - drm_printf(p, "\t\tLRC Tail: Internal %u, Memory %u\n",
> - ce->ring->tail,
> - ce->lrc_reg_state[CTX_RING_TAIL]);
> - drm_printf(p, "\t\tContext Pin Count: %u\n",
> - atomic_read(&ce->pin_count));
> - drm_printf(p, "\t\tGuC ID Ref Count: %u\n",
> - atomic_read(&ce->guc_id_ref));
> - drm_printf(p, "\t\tNumber Requests Not Ready: %u\n",
> - atomic_read(&ce->guc_num_rq_not_ready));
> - drm_printf(p, "\t\tSchedule State: 0x%x, 0x%x\n\n",
> - ce->guc_state.sched_state,
> - atomic_read(&ce->guc_sched_state_no_lock));
> + GEM_BUG_ON(intel_context_is_child(ce));
>
> + guc_log_context(p, ce);
> guc_log_context_priority(p, ce);
> +
> + if (intel_context_is_parent(ce)) {
> + struct guc_process_desc *desc = __get_process_desc(ce);
> + struct intel_context *child;
> +
> + drm_printf(p, "\t\tWQI Head: %u\n",
> + READ_ONCE(desc->head));
> + drm_printf(p, "\t\tWQI Tail: %u\n",
> + READ_ONCE(desc->tail));
> + drm_printf(p, "\t\tWQI Status: %u\n\n",
> + READ_ONCE(desc->wq_status));
> +
> + for_each_child(ce, child)
> + guc_log_context(p, child);
> + }
> }
> }
>
> --
> 2.28.0
>
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
