Re: [PATCH] drm: tweak drm_print_bits()
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.
>
> Also use for_each_set_bit().
>
> Suggested-by: Jani Nikula
> Signed-off-by: Gerd Hoffmann
> ---
> 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(-)
>
> 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
>
> __printf(2, 0)
> /**
> diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c
> b/drivers/gpu/drm/drm_gem_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[] = {
> + static const char * const plname[] = {
> [ TTM_PL_SYSTEM ] = "system",
> [ TTM_PL_TT ] = "tt",
> [ TTM_PL_VRAM ] = "vram",
> @@ -40,7 +40,7 @@ void drm_gem_ttm_print_info(struct drm_printer *p, unsigned
> int indent,
> const struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem);
>
> drm_printf_indent(p, indent, "placement=");
> - 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");
>
> 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 _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 _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 = true;
> unsigned int i;
>
> - for (i = 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 = sizeof(unsigned long) * 8;
> +
> + for_each_set_bit(i, , nbits) {
> + if (WARN_ON_ONCE(!bits[i]))
> continue;
> drm_printf(p, "%s%s", first ? "" : ",",
> -bits[i-from]);
> +bits[i]);
> first = false;
> }
> if (first)
> --
> 2.18.1
>
> ___
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
signature.asc
Description: PGP signature
Re: [PATCH] drm: tweak drm_print_bits()
On Thu, 19 Sep 2019, 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.
>
> Also use for_each_set_bit().
Oh, I don't know why I stumbled upon and reviewed a patch that had
already been merged. Can't keep track of everything it seems...
>
> Suggested-by: Jani Nikula
> Signed-off-by: Gerd Hoffmann
> ---
> 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(-)
>
> 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);
>
> __printf(2, 0)
> /**
> diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c
> b/drivers/gpu/drm/drm_gem_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[] = {
> + static const char * const plname[] = {
> [ TTM_PL_SYSTEM ] = "system",
> [ TTM_PL_TT ] = "tt",
> [ TTM_PL_VRAM ] = "vram",
> @@ -40,7 +40,7 @@ void drm_gem_ttm_print_info(struct drm_printer *p, unsigned
> int indent,
> const struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem);
>
> drm_printf_indent(p, indent, "placement=");
> - 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");
>
> 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 _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 _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 = true;
> unsigned int i;
>
> - for (i = 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 = sizeof(unsigned long) * 8;
Might be neater with BITS_PER_TYPE(value) instead of open coding, but
either way,
Reviewed-by: Jani Nikula
> +
> + for_each_set_bit(i, , nbits) {
> + if (WARN_ON_ONCE(!bits[i]))
> continue;
> drm_printf(p, "%s%s", first ? "" : ",",
> -bits[i-from]);
> +bits[i]);
> first = false;
> }
> if (first)
--
Jani Nikula, Intel Open Source Graphics Center
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm: tweak drm_print_bits()
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.
Also use for_each_set_bit().
Suggested-by: Jani Nikula
Signed-off-by: Gerd Hoffmann
---
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(-)
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);
__printf(2, 0)
/**
diff --git a/drivers/gpu/drm/drm_gem_ttm_helper.c
b/drivers/gpu/drm/drm_gem_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[] = {
+ static const char * const plname[] = {
[ TTM_PL_SYSTEM ] = "system",
[ TTM_PL_TT ] = "tt",
[ TTM_PL_VRAM ] = "vram",
@@ -40,7 +40,7 @@ void drm_gem_ttm_print_info(struct drm_printer *p, unsigned
int indent,
const struct ttm_buffer_object *bo = drm_gem_ttm_of_gem(gem);
drm_printf_indent(p, indent, "placement=");
- 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");
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 _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 _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 = true;
unsigned int i;
- for (i = 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 = sizeof(unsigned long) * 8;
+
+ for_each_set_bit(i, , nbits) {
+ if (WARN_ON_ONCE(!bits[i]))
continue;
drm_printf(p, "%s%s", first ? "" : ",",
- bits[i-from]);
+ bits[i]);
first = false;
}
if (first)
--
2.18.1
