On 21/11/2022 18:21, John Harrison wrote:
On 11/18/2022 02:52, Jani Nikula wrote:
On Thu, 17 Nov 2022, John.C.Harrison@xxxxxxxxx wrote:
From: John Harrison <John.C.Harrison@xxxxxxxxx>
When trying to analyse bug reports from CI, customers, etc. it can be
difficult to work out exactly what is happening on which GT in a
multi-GT system. So add GT oriented debug/error message wrappers. If
used instead of the drm_ equivalents, you get the same output but with
a GT# prefix on it.
It was also requested to extend this further to submodules in order to
factor out the repeated structure accessing constructs and common
string prefixes. So, add versions for GuC, HuC and GuC CTB as well.
This patch set updates all the gt/uc files to use the new helpers as a
first step. The intention would be to convert all output messages that
have access to a GT structure.
v2: Go back to using lower case names, add more wrapper sets (combined
review feedback). Also, wrap up probe injection and WARN entries.
Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
For adding the wrappers in general, I'm going to disagree and
commit. I'll leave it up to Tvrtko and Joonas.
Regarding the placement of the macros, I insist you add individual
header files for the wrappers and include them only where needed.
We have a fairly serious problem with everything including everything in
i915 that I've been slowly trying to tackle. Touch one thing, rebuild
everything. About a third of our headers cause the rebuild of the entire
driver when modified. We need to reduce the surface of things that cause
rebuilds.
For example, intel_gt.h is included by 97 files, intel_guc.h by 332
files, and intel_huc.h by 329 files (counting recursively).
There's absolutely no reason any of the display code, for example, needs
to have these logging macros in their build. Long term, the headers
should be reorganized to reduce the interdependencies, and this is what
I've been doing in i915_drv.h and display/ in general. But the least we
can do is not make the problem worse.
@Tvrtko/@Michal W, any other review comments or feedback? I'd rather not
spend time fixing up the header file issue and reposting only to have
someone point out another issue that could have been resolved at the
same time.
I read through the patches when you posted them and it looked nice and
clean to me. I think I spotted one instance of a debug build only
message getting upgraded to production build, and one loss of stack
trace on a warning, but it wasn't a concern to me AFAIR.
Regards,
Tvrtko
John.
BR,
Jani.
John Harrison (5):
drm/i915/gt: Start adding module oriented dmesg output
drm/i915/huc: Add HuC specific debug print wrappers
drm/i915/guc: Add GuC specific debug print wrappers
drm/i915/guc: Add GuC CT specific debug print wrappers
drm/i915/uc: Update the gt/uc code to use gt_err and friends
drivers/gpu/drm/i915/gt/intel_gt.c | 96 ++++----
drivers/gpu/drm/i915/gt/intel_gt.h | 35 +++
drivers/gpu/drm/i915/gt/uc/intel_guc.c | 32 +--
drivers/gpu/drm/i915/gt/uc/intel_guc.h | 35 +++
drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c | 8 +-
.../gpu/drm/i915/gt/uc/intel_guc_capture.c | 48 ++--
drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 222 +++++++++---------
drivers/gpu/drm/i915/gt/uc/intel_guc_fw.c | 19 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_log.c | 37 ++-
drivers/gpu/drm/i915/gt/uc/intel_guc_rc.c | 7 +-
drivers/gpu/drm/i915/gt/uc/intel_guc_slpc.c | 55 ++---
.../gpu/drm/i915/gt/uc/intel_guc_submission.c | 62 +++--
drivers/gpu/drm/i915/gt/uc/intel_huc.c | 31 +--
drivers/gpu/drm/i915/gt/uc/intel_huc.h | 23 ++
drivers/gpu/drm/i915/gt/uc/intel_uc.c | 108 ++++-----
drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c | 98 ++++----
drivers/gpu/drm/i915/gt/uc/selftest_guc.c | 34 +--
.../drm/i915/gt/uc/selftest_guc_hangcheck.c | 22 +-
.../drm/i915/gt/uc/selftest_guc_multi_lrc.c | 10 +-
19 files changed, 507 insertions(+), 475 deletions(-)
