[Intel-gfx] [PATCH] drm/i915/guc: Compact init params debug to a single line

[Chris Wilson]

Quoting Michal Wajdeczko (2019-06-25 19:30:18)
> On Tue, 25 Jun 2019 19:45:47 +0200, Chris Wilson  
> <chris at chris-wilson.co.uk> 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 at chris-wilson.co.uk>
> > Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> > ---
> >  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
> (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?
-Chris