Re: [PATCH 04/29] drm/i915: Dump 'output_types' in crtc state dump

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Sep 18, 2017 at 07:43:43PM +0100, Chris Wilson wrote:
> Quoting Ville Syrjala (2017-09-18 19:25:39)
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > To make it easier to debug things let's dump the output types bitmask in
> > the crtc state dump. And to make life that much better, let's pretty
> > print it as a a human reaadable string as well.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_display.c | 50 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 50 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 8599e425abb1..192b07a92e76 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10683,6 +10683,53 @@ intel_dump_m_n_config(struct intel_crtc_state *pipe_config, char *id,
> >                       m_n->link_m, m_n->link_n, m_n->tu);
> >  }
> >  
> > +#define OUTPUT_TYPE(x) [INTEL_OUTPUT_ ## x] = #x
> > +
> > +static const char * const output_type_str[] = {
> > +       OUTPUT_TYPE(UNUSED),
> > +       OUTPUT_TYPE(ANALOG),
> > +       OUTPUT_TYPE(DVO),
> > +       OUTPUT_TYPE(SDVO),
> > +       OUTPUT_TYPE(LVDS),
> > +       OUTPUT_TYPE(TVOUT),
> > +       OUTPUT_TYPE(HDMI),
> > +       OUTPUT_TYPE(DP),
> > +       OUTPUT_TYPE(EDP),
> > +       OUTPUT_TYPE(DSI),
> > +       OUTPUT_TYPE(UNKNOWN),
> > +       OUTPUT_TYPE(DP_MST),
> > +};
> > +
> > +static const char *output_types_str(unsigned int output_types)
> > +{
> > +       static char buf[64];
> 
> Bleh. Push to caller?

It's a debug thing so I wasn't too worried about making it entirely
robust. But I guess these things tend to come back and bite us
later, so with that in mind I can respin this to use a caller
provided buffer.

> > +       size_t len = sizeof(buf);
> > +       char *str = buf;
> > +       int i;
> > +
> > +       str[0] = '\0';
> > +
> > +       for (i = 0; i < ARRAY_SIZE(output_type_str); i++) {
> > +               int r;
> > +
> > +               if ((output_types & BIT(i)) == 0)
> > +                       continue;
> > +
> > +               r = snprintf(str, len, "%s%s",
> > +                            str != buf ? "," : "", output_type_str[i]);
> 
> No space after the comma? Heathen.

I had it there originally, but it looked a bit weird to me when I had
more than one bit set. I think the problem is that we alrady use a
comma for separating different pieces of state when we print them on
the same line. And so not having a space in there makes it more
clear that these things are in fact part of the same piece of state.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx