Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Date:   Tue, 3 Sep 2019 12:54:36 +0200
From:   Daniel Vetter <daniel@ffwll.ch>
To:     Gerd Hoffmann <kraxel@redhat.com>
Cc:     dri-devel@lists.freedesktop.org,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <maxime.ripard@bootlin.com>,
        Sean Paul <sean@poorly.run>, David Airlie <airlied@linux.ie>,
        Daniel Vetter <daniel@ffwll.ch>,
        open list <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH v2 1/6] drm: add drm_print_bits
Message-ID: <20190903105436.GU2112@phenom.ffwll.local>
Mail-Followup-To: Gerd Hoffmann <kraxel@redhat.com>,
        dri-devel@lists.freedesktop.org,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Maxime Ripard <maxime.ripard@bootlin.com>,
        Sean Paul <sean@poorly.run>, David Airlie <airlied@linux.ie>,
        open list <linux-kernel@vger.kernel.org>
References: <20190903101248.12879-1-kraxel@redhat.com>
 <20190903101248.12879-2-kraxel@redhat.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20190903101248.12879-2-kraxel@redhat.com>
User-Agent: Mutt/1.10.1 (2018-07-13)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Tue, Sep 03, 2019 at 12:12:43PM +0200, Gerd Hoffmann wrote:
> New helper to print named bits of some value (think flags fields).
> 
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/drm/drm_print.h     |  3 +++
>  drivers/gpu/drm/drm_print.c | 36 ++++++++++++++++++++++++++++++++++++
>  2 files changed, 39 insertions(+)
> 
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index a5d6f2f3e430..8658c1da1c7d 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -88,6 +88,9 @@ __printf(2, 3)
>  void drm_printf(struct drm_printer *p, const char *f, ...);
>  void drm_puts(struct drm_printer *p, const char *str);
>  void drm_print_regset32(struct drm_printer *p, struct debugfs_regset32 *regset);
> +void drm_print_bits(struct drm_printer *p, unsigned int indent,
> +		    const char *label, unsigned int value,
> +		    const char *bits[], unsigned int nbits);
>  
>  __printf(2, 0)
>  /**
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index a17c8a14dba4..7f7aba920f51 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -179,6 +179,42 @@ void drm_printf(struct drm_printer *p, const char *f, ...)
>  }
>  EXPORT_SYMBOL(drm_printf);
>  
> +/**
> + * drm_print_bits - print bits to a &drm_printer stream
> + *
> + * Print bits (in flag fields for example) in human readable form.
> + *
> + * @p: the &drm_printer
> + * @indent: Tab indentation level (max 5)
> + * @label: field label.
> + * @value: field value.
> + * @bits: Array with bit names.
> + * @nbits: bit name array size.
> + */
> +void drm_print_bits(struct drm_printer *p, unsigned int indent,
> +		    const char *label, unsigned int value,
> +		    const char *bits[], unsigned int nbits)
> +{
> +	bool first = true;
> +	unsigned int i;
> +
> +	for (i = 0; i < nbits; i++) {
> +		if (!(value & (1 << i)))
> +			continue;
> +		if (!bits[i])

I think this should be a WARN_ON, indicates a programming error?

> +			continue;
> +		if (first) {
> +			first = false;
> +			drm_printf_indent(p, indent, "%s=%s",
> +					  label, bits[i]);

Hm, to make this a bit more flexible to use I'd drop the label= printing
...

> +		} else
> +			drm_printf(p, ",%s", bits[i]);
> +	}
> +	if (!first)
> +		drm_printf(p, "\n");

... and also the newline. Then you could also use this for bit-fields
which just a few bits. Also, should we print anything if no bit is set?

If you prefer the label= + \n then pls add that to the kerneldoc, that it
prints this as a line of its own.
-Daniel

> +}
> +EXPORT_SYMBOL(drm_print_bits);
> +
>  void drm_dev_printk(const struct device *dev, const char *level,
>  		    const char *format, ...)
>  {
> -- 
> 2.18.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch