Re: [Intel-gfx] [PATCH v3] drm/i915: Don't show engine information in fdinfo with GuC submission

["Dixit, Ashutosh" <ashutosh.dixit@intel.com>]

On Wed, 27 Apr 2022 10:16:03 -0700, Tvrtko Ursulin wrote:
>
>
> On 27/04/2022 16:43, Dixit, Ashutosh wrote:
> > On Wed, 27 Apr 2022 02:15:35 -0700, Tvrtko Ursulin wrote:
> >>
> >> On 15/04/2022 01:25, Ashutosh Dixit wrote:
> >>> At present i915 does not fetch busyness information from GuC, resulting in
> >>> incorrect busyness values in fdinfo. Because engine information is coupled
> >>> with busyness in fdinfo, skip showing client engine information in fdinfo
> >>> with GuC submission till fetching busyness is supported in the i915 GuC
> >>> submission backend.
> >>>
> >>> v2 (Daniele):
> >>>     Make commit title and description more precise
> >>>     Add FIXME with brief description at code change
> >>>     s/intel_guc_submission_is_used/intel_uc_uses_guc_submission/
> >>>
> >>> v3 (Daniele):
> >>>     Drop FIXME in comment
> >>>
> >>> Bug: https://gitlab.freedesktop.org/drm/intel/-/issues/5564
> >>> Fixes: 055634e4b62f ("drm/i915: Expose client engine utilisation via fdinfo")
> >>> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com
> >>> Cc: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com>
> >>> Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com>
> >>> Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> >>> ---
> >>>    drivers/gpu/drm/i915/i915_drm_client.c | 6 +++++-
> >>>    1 file changed, 5 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_drm_client.c b/drivers/gpu/drm/i915/i915_drm_client.c
> >>> index e539f6b23060..475a6f824cad 100644
> >>> --- a/drivers/gpu/drm/i915/i915_drm_client.c
> >>> +++ b/drivers/gpu/drm/i915/i915_drm_client.c
> >>> @@ -145,7 +145,11 @@ void i915_drm_client_fdinfo(struct seq_file *m, struct file *f)
> >>>		   PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn));
> >>>	seq_printf(m, "drm-client-id:\t%u\n", client->id);
> >>>    -	if (GRAPHICS_VER(i915) < 8)
> >>> +	/*
> >>> +	 * Temporarily skip showing client engine information with GuC submission till
> >>> +	 * fetching engine busyness is implemented in the GuC submission backend
> >>> +	 */
> >>> +	if (GRAPHICS_VER(i915) < 8 || intel_uc_uses_guc_submission(&i915->gt0.uc))
> >>>		return;
> >>>		for (i = 0; i < ARRAY_SIZE(uabi_class_names); i++)
> >>
> >> Thanks for fixing this while I was away. It was a simple miss, nothing
> >> sinister. In terms of mention of "garbage" numbers being reported - were
> >> they actually garbage or simply always zero?
> >
> > Ah, you are referring to what I wrote in the bug. Actually I didn't check
> > the values myself but was told we were displaying "garbage" values (or at
> > least I interpreted it that way, and garbage meaning not just zero). But
> > looking now at IGT outputs from that time appears the values were just zero
> > :/
> >
> > https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11503/re-adlp-pub1/igt@drm_fdinfo@all-busy-idle-check-all.html
> >
> > I think we could even have left zero values as is except that we'd have to
> > fix the IGT failure.
> >
> > Sorry for the confusion.
>
> Np. One could even say that there is little difference between skip and
> fail, given both need manual handling in cibuglog to be marked as known
> until GuC support gets added. At least if I still remember how it works
> and that unexplained skips still count as fails.

True, I just updated
https://gitlab.freedesktop.org/drm/intel/-/issues/5563. I think in
retrospect we should have just changed the IGT check to ignore 0 busyness
values :/