<!DOCTYPE html><html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
</head>
<body>
<div class="moz-cite-prefix">On 12/24/2024 11:10, Daniele Ceraolo
Spurio wrote:<br>
</div>
<blockquote type="cite" cite="mid:5c2c8e5c-c16d-41c1-90e7-07b1ddf0f923@intel.com">
On 12/24/2024 10:13 AM, John Harrison wrote:<br>
<blockquote type="cite" cite="mid:16414d38-e5ae-4bc5-9b22-a2a7a928264b@intel.com">
<div class="moz-cite-prefix">On 12/23/2024 15:20, Daniele
Ceraolo Spurio wrote:<br>
</div>
<blockquote type="cite" cite="mid:27c27339-cde3-4028-99e5-deb574c091ec@intel.com"> On
12/20/2024 5:19 PM, <a class="moz-txt-link-abbreviated moz-txt-link-freetext" href="mailto:John.C.Harrison@Intel.com" moz-do-not-send="true">John.C.Harrison@Intel.com</a> wrote:<br>
<blockquote type="cite" cite="mid:20241221011925.3944625-1-John.C.Harrison@Intel.com">
<pre wrap="" class="moz-quote-pre">From: John Harrison <a class="moz-txt-link-rfc2396E" href="mailto:John.C.Harrison@Intel.com" moz-do-not-send="true"><John.C.Harrison@Intel.com></a>
Add debug info to help investigate a very rare bug:
<a class="moz-txt-link-freetext" href="https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13385" moz-do-not-send="true">https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13385</a>
Signed-off-by: John Harrison <a class="moz-txt-link-rfc2396E" href="mailto:John.C.Harrison@Intel.com" moz-do-not-send="true"><John.C.Harrison@Intel.com></a>
---
drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
index b67a15f742762..868195c33f5b3 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_hwconfig.c
@@ -7,6 +7,7 @@
#include "gt/intel_hwconfig.h"
#include "i915_drv.h"
#include "i915_memcpy.h"
+#include "intel_guc_print.h"
/*
* GuC has a blob containing hardware configuration information (HWConfig).
@@ -42,6 +43,8 @@ static int __guc_action_get_hwconfig(struct intel_guc *guc,
};
int ret;
+ guc_dbg(guc, "Querying HW config table: size = %d, offset = 0x%08X\n",
+ ggtt_size, ggtt_offset);</pre>
</blockquote>
<br>
This seems to result in a double-log where the first print has
no useful information, e.g.:<br>
<br>
<div style="font-family: Roboto, Oxygen-Sans, Ubuntu, Cantarell, sans-serif; color: rgb(0, 0, 0); font-size: medium; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: nowrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;"><span id="dmesg-normal420" class="dmesg dmesg-normal" style="font-family: monospace; white-space: pre; overflow-wrap: normal;">[drm:__guc_action_get_hwconfig [i915]] GT0: GUC: Querying HW config table: size = 0, offset = 0x00000000</span></div>
<div style="font-family: Roboto, Oxygen-Sans, Ubuntu, Cantarell, sans-serif; color: rgb(0, 0, 0); font-size: medium; font-style: normal; font-variant-ligatures: normal; font-variant-caps: normal; font-weight: 400; letter-spacing: normal; orphans: 2; text-align: start; text-indent: 0px; text-transform: none; widows: 2; word-spacing: 0px; -webkit-text-stroke-width: 0px; white-space: nowrap; text-decoration-thickness: initial; text-decoration-style: initial; text-decoration-color: initial;"><span id="dmesg-normal421" class="dmesg dmesg-normal" style="font-family: monospace; white-space: pre; overflow-wrap: normal;">[drm:__guc_action_get_hwconfig [i915]] GT0: GUC: Querying HW config table: size = 752, offset = 0x00D05000</span></div>
<br>
Given that only the second log is useful, IMO better to move
the guc_dbg to guc_hwconfig_fill_buffer(), because the info
needed for the second print is available there and it is only
called once.<br>
</blockquote>
I disagree that the first print has no useful information. It
tells us that a call is being made and these are the parameters.
We do not know what the failure is. It seems highly unlikely
that the size changes from query to the next given that the
table is a fixed entity. It is much more likely to be a caching
type issue with GuC reading data the KMD did not write. If so,
GuC could potentially read non-zero data for the initial size
query and complain that data is invalid.<br>
<br>
The intention is to report all calls with their parameters to
try to narrow down exactly what calls are not working.<br>
</blockquote>
<br>
But we don't need both prints to know which of the 2 calls has
failed, if the error comes before we get the second print then we
know the failure was in the first call, otherwise it was in the
second.<br>
</blockquote>
Is it really such a problem to have two lines of debug print instead
of one? Given that this is a 'this cannot possibly happen' type bug,
I would much rather have totally explicit debug at the point of
operation rather than something further back that requires making
assumptions about what is or is not happening.<br>
<br>
John.<br>
<br>
<blockquote type="cite" cite="mid:5c2c8e5c-c16d-41c1-90e7-07b1ddf0f923@intel.com"> <br>
Daniele<br>
<br>
<blockquote type="cite" cite="mid:16414d38-e5ae-4bc5-9b22-a2a7a928264b@intel.com"> <br>
John.<br>
<br>
<br>
<blockquote type="cite" cite="mid:27c27339-cde3-4028-99e5-deb574c091ec@intel.com"> <br>
Daniele<br>
<br>
<blockquote type="cite" cite="mid:20241221011925.3944625-1-John.C.Harrison@Intel.com">
<pre wrap="" class="moz-quote-pre"> ret = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
if (ret == -ENXIO)
return -ENOENT;
</pre>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</blockquote>
<br>
</body>
</html>