[Intel-gfx] [PATCH] drm/i915: Build request info on stack before printk

[Chris Wilson]

Quoting Tvrtko Ursulin (2018-04-24 12:57:41)
> 
> On 24/04/2018 02:08, Chris Wilson wrote:
> > printk unhelpfully inserts a '\n' between consecutive calls, and since
> > our drm_printf wrapper may be emitting info a seq_file instead,
> > KERN_CONT is not an option. To work with any drm_printf destination, we
> > need to build up the output into a temporary buf on the stack and then
> > feed the complete line in a single call to printk.
> > 
> > Fixes: b7268c5eed0a ("drm/i915: Pack params to engine->schedule() into a struct")
> > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> > ---
> >   drivers/gpu/drm/i915/intel_engine_cs.c | 24 +++++++++++++++---------
> >   1 file changed, 15 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
> > index be608f7111f5..66cddd059666 100644
> > --- a/drivers/gpu/drm/i915/intel_engine_cs.c
> > +++ b/drivers/gpu/drm/i915/intel_engine_cs.c
> > @@ -1113,14 +1113,17 @@ unsigned int intel_engines_has_context_isolation(struct drm_i915_private *i915)
> >       return which;
> >   }
> >   
> > -static void print_sched_attr(struct drm_printer *m,
> > -                          const struct drm_i915_private *i915,
> > -                          const struct i915_sched_attr *attr)
> > +static int print_sched_attr(struct drm_i915_private *i915,
> > +                         const struct i915_sched_attr *attr,
> > +                         char *buf, int x, int len)
> >   {
> >       if (attr->priority == I915_PRIORITY_INVALID)
> > -             return;
> > +             return x;
> > +
> > +     x += snprintf(buf + x, len - x,
> > +                   " prio=%d", attr->priority);
> >   
> > -     drm_printf(m, "prio=%d", attr->priority);
> > +     return x;
> >   }
> >   
> >   static void print_request(struct drm_printer *m,
> > @@ -1128,14 +1131,17 @@ static void print_request(struct drm_printer *m,
> >                         const char *prefix)
> >   {
> >       const char *name = rq->fence.ops->get_timeline_name(&rq->fence);
> > +     char buf[80];
> 
> Worth using less stack space? 6 chars plus max negative int (12) - 18 
> should be enough?
> 
> > +     int x = 0;
> > +
> > +     x = print_sched_attr(rq->i915, &rq->sched.attr, buf, x, sizeof(buf));
> 
> x is effectively unused. Drop it and simplify the helper and all?

It felt like a common enough idiom to allow for future expansion. It's
going to be required at some point in the near future, I'm sure.
-Chris