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:   Fri, 20 Sep 2019 17:37:09 +0200
From:   Thierry Reding <thierry.reding@gmail.com>
To:     Gerd Hoffmann <kraxel@redhat.com>
Cc:     dri-devel@lists.freedesktop.org,
        Maxime Ripard <maxime.ripard@bootlin.com>,
        open list <linux-kernel@vger.kernel.org>,
        David Airlie <airlied@linux.ie>, Sean Paul <sean@poorly.run>
Subject: Re: [PATCH] drm: tweak drm_print_bits()
Message-ID: <20190920153709.GC10973@ulmo>
References: <20190919131412.25602-1-kraxel@redhat.com>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha256;
        protocol="application/pgp-signature"; boundary="KN5l+BnMqAQyZLvT"
Content-Disposition: inline
In-Reply-To: <20190919131412.25602-1-kraxel@redhat.com>
User-Agent: Mutt/1.12.1 (2019-06-15)
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk


--KN5l+BnMqAQyZLvT
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Thu, Sep 19, 2019 at 03:14:11PM +0200, Gerd Hoffmann wrote:
> There is little reason for the from/to logic, printing a subset of
> the bits can be done by simply shifting/masking value if needed.
>=20
> Also use for_each_set_bit().
>=20
> Suggested-by: Jani Nikula <jani.nikula@linux.intel.com>
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
>  include/drm/drm_print.h              |  5 ++---
>  drivers/gpu/drm/drm_gem_ttm_helper.c |  4 ++--
>  drivers/gpu/drm/drm_print.c          | 20 +++++++++-----------
>  3 files changed, 13 insertions(+), 16 deletions(-)
>=20
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 12d4916254b4..5b9947d157f4 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -89,9 +89,8 @@ __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 long value, const char *bits[],
> -		    unsigned int from, unsigned int to);
> +void drm_print_bits(struct drm_printer *p, unsigned long value,
> +		    const char * const bits[], int nbits);

bitmap_*() APIs that this calls underneath all use unsigned int for
nbits, so might be good to stick with that. After all, this better never
be negative.

Thierry

> =20
>  __printf(2, 0)
>  /**
> diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c b/drivers/gpu/drm/drm_g=
em_ttm_helper.c
> index 9a4bafcf20df..a534104d8bee 100644
> --- a/drivers/gpu/drm/drm_gem_ttm_helper.c
> +++ b/drivers/gpu/drm/drm_gem_ttm_helper.c
> @@ -23,7 +23,7 @@
>  void drm_gem_ttm_print_info(struct drm_printer *p, unsigned int indent,
>  			    const struct drm_gem_object *gem)
>  {
> -	static const char const *plname[] =3D {
> +	static const char * const plname[] =3D {
>  		[ TTM_PL_SYSTEM ] =3D "system",
>  		[ TTM_PL_TT     ] =3D "tt",
>  		[ TTM_PL_VRAM   ] =3D "vram",
> @@ -40,7 +40,7 @@ void drm_gem_ttm_print_info(struct drm_printer *p, unsi=
gned int indent,
>  	const struct ttm_buffer_object *bo =3D drm_gem_ttm_of_gem(gem);
> =20
>  	drm_printf_indent(p, indent, "placement=3D");
> -	drm_print_bits(p, bo->mem.placement, plname, 0, ARRAY_SIZE(plname));
> +	drm_print_bits(p, bo->mem.placement, plname, ARRAY_SIZE(plname));
>  	drm_printf(p, "\n");
> =20
>  	if (bo->mem.bus.is_iomem) {
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index dfa27367ebb8..a495fe3ad909 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -189,28 +189,26 @@ 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.
> - * The first name in the @bits array is for the bit indexed by @from.
>   *
>   * @p: the &drm_printer
>   * @value: field value.
>   * @bits: Array with bit names.
> - * @from: start of bit range to print (inclusive).
> - * @to: end of bit range to print (exclusive).
> + * @nbits: Size of bit names array.
>   */
> -void drm_print_bits(struct drm_printer *p,
> -		    unsigned long value, const char *bits[],
> -		    unsigned int from, unsigned int to)
> +void drm_print_bits(struct drm_printer *p, unsigned long value,
> +		    const char * const bits[], int nbits)
>  {
>  	bool first =3D true;
>  	unsigned int i;
> =20
> -	for (i =3D from; i < to; i++) {
> -		if (!(value & (1 << i)))
> -			continue;
> -		if (WARN_ON_ONCE(!bits[i-from]))
> +	if (WARN_ON_ONCE(nbits > sizeof(unsigned long) * 8))
> +		nbits =3D sizeof(unsigned long) * 8;
> +
> +	for_each_set_bit(i, &value, nbits) {
> +		if (WARN_ON_ONCE(!bits[i]))
>  			continue;
>  		drm_printf(p, "%s%s", first ? "" : ",",
> -			   bits[i-from]);
> +			   bits[i]);
>  		first =3D false;
>  	}
>  	if (first)
> --=20
> 2.18.1
>=20
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

--KN5l+BnMqAQyZLvT
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEEiOrDCAFJzPfAjcif3SOs138+s6EFAl2E8iMACgkQ3SOs138+
s6Eb3g//RVAd8nEd/vTjAihUPyEF8QrbURk9POsCWgjOGQ2nkplz2Pxi3dN2pk/S
ZI9Oz/GhRVoLttOiUbSjsPKx0uym8PK8sVoYnvSl6o3c0WtJZ5ZOJpo2TBwgBJ/L
0XbxLsVbCzph4dIH9BX3+v+XZUddW8CRni5dYyKR3CBrFNyVggt5Nutr0+27wWHl
xdfJqO576b2LJTIltJdreeq5YTeclT6S+ouCrcosoPhZstpOPcStv/Kjb4TLycLG
WOR3aD3OOIWuXpiA64xh/gLhEQs2wJ7f/5c24gWFfSbmmlojOMpBBGMaimupjWOx
bWeDvVSX1SXpThLY8P5EpEKiS7jHXnTm7Vty1V0QC8eYFgcBG1IzDGS63ml2Ffys
EhP18L7klydCxnLsOlHBZe88jiEN7npAZir2XIuZZEFq2sNKoPBCRxPFl4n9JxV0
fpok74P06DED5ytnoSz0qHY9Sb+AkJyZk+RCbfb5pARMg/GxXFVOpZ2NC6z7AP1k
xbCWItj+qajELNwxu7FNPB2PxaUE9qDTP5JgqsI7KVOlTCwkJezx+bIyEoTNEVzu
RUHN8obICzV8uxYtLhjUI4703lR/Xe/DeAgvQVk3vVH+CJzJXcA36WQarWnciPb6
SeSLQydLBzEm5dc2O5F2IIJArvcxnxYXMRvMY/xud9uvpvtPF5E=
=YQP7
-----END PGP SIGNATURE-----

--KN5l+BnMqAQyZLvT--