On 18.11.2022 11: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. do you mean: intel_gt_print.h intel_guc_print.h intel_huc_print.h with just macros or also with all functions that work with drm_printer? > > 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. to solve this we should really consider splitting out GuC and HuC definitions to dedicated _types.h files and only include them in i915_drv.h (and print macros are orthogonal for this problem) Michal > > 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(-) >