On 8/4/2022 17:40, Teres Alexis, Alan Previn wrote:
I have a question on below code. Everything else looked good.
Will r-b as soon as we can close on below question
...alan
On Wed, 2022-07-27 at 19:20 -0700, John.C.Harrison@xxxxxxxxx wrote:
From: John Harrison <John.C.Harrison@xxxxxxxxx>
It is useful to be able to match GuC events to kernel events when
looking at the GuC log. That requires being able to convert GuC
timestamps to kernel time. So, when dumping error captures and/or GuC
logs, include a stamp in both time zones plus the clock frequency.
Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -1675,6 +1678,13 @@ gt_record_uc(struct intel_gt_coredump *gt,
*/
error_uc->guc_fw.path = kstrdup(uc->guc.fw.path, ALLOW_FAIL);
error_uc->huc_fw.path = kstrdup(uc->huc.fw.path, ALLOW_FAIL);
+
+ /*
+ * Save the GuC log and include a timestamp reference for converting the
+ * log times to system times (in conjunction with the error->boottime and
+ * gt->clock_frequency fields saved elsewhere).
+ */
+ error_uc->timestamp = intel_uncore_read(gt->_gt->uncore, GUCPMTIMESTAMP);
Alan:this register is in the GUC-SHIM domain and so unless i am mistaken u might need to ensure we hold a wakeref so
that are getting a live value of the real timestamp register that this register is mirror-ing and not a stale snapshot.
Or was this already done farther up the stack? Or are we doing the opposite - in which case we should ensure we drop al
wakeref prior to this point. (which i am not sure is a reliable method since we wouldnt know what GuC ref was at).
The intel_uncore_read() does a forcewake acquire implicitly.
Not sure what you mean about dropping all wakerefs prior to this point?
John.
error_uc->guc_log = create_vma_coredump(gt->_gt, uc->guc.log.vma,
"GuC log buffer", compress);
