On 6/25/19 11:47 AM, Chris Wilson wrote:
Quoting Michal Wajdeczko (2019-06-25 19:30:18)
On Tue, 25 Jun 2019 19:45:47 +0200, Chris Wilson
<chris@xxxxxxxxxxxxxxxxxx> wrote:
Use hex_dump_to_buffer() to compress the parameter debug into a single
line for less verbose debug logs.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_guc.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_guc.c
b/drivers/gpu/drm/i915/intel_guc.c
index c40a6efdd33a..447f1de15289 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -367,6 +367,7 @@ static u32 guc_ctl_ads_flags(struct intel_guc *guc)
void intel_guc_init_params(struct intel_guc *guc)
{
struct drm_i915_private *dev_priv = guc_to_i915(guc);
+ char buf[GUC_CTL_MAX_DWORDS * 10];
u32 params[GUC_CTL_MAX_DWORDS];
int i;
@@ -378,8 +379,9 @@ void intel_guc_init_params(struct intel_guc *guc)
params[GUC_CTL_DEBUG] = guc_ctl_debug_flags(guc);
params[GUC_CTL_ADS] = guc_ctl_ads_flags(guc);
- for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
- DRM_DEBUG_DRIVER("param[%2d] = %#x\n", i, params[i]);
+ hex_dump_to_buffer(params, sizeof(params),
+ 32, 4, buf, sizeof(buf), false);
hmm, GUC_CTL_MAX_DWORDS is 14, so it will be 56 bytes in total,
but hex_dump_to_buffer will dump only 32 bytes ... unless
we explicitly limit our dump to currently used just 5 entries
To be pedantic we use 6 entries (0..5), but doesn't really change the
point :)
(20 bytes) but then this might be not future proof if new fw
will require/use more then 8 parameters
Ah, it only does one line. I completely forgot about that.
+ DRM_DEBUG_DRIVER("params[%s]\n", buf);
use of [%s] may make this less readable, so maybe:
DRM_DEBUG_DRIVER("GuC params %s\n", buf);
but I'm still not sure if we should go that partial way, Daniele ?
How about,
if (drm_debug & DRM_UT_DRIVER)
print_hex_dump(KERN_DEBUG, "guc init params: ", 0, 16, 4,
params, sizeof(params), false);
ps. we can aslo use two lines or two bufs for 0..7 and 8..13 params
Would also be an improvement over 14 :)
Do we even need to dump them? They are almost all static, with the
exception of debug level and ads address? Is it useful?
In my experience it can be useful when we get a bug report where guc
failed to load or when we're testing an interface change to double-check
that the parameters are set as expected. But I agree there is no need to
dump all the dwords we don't set. Maybe we can reduce GUC_CTL_MAX_DWORDS
to the number of used dwords, or add a new define set to that and use it
for buf size?
Daniele
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
