* [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
@ 2018-03-13 22:02 Joe Perches
2018-03-13 23:33 ` ✓ Fi.CI.BAT: success for " Patchwork
` (4 more replies)
0 siblings, 5 replies; 27+ messages in thread
From: Joe Perches @ 2018-03-13 22:02 UTC (permalink / raw)
To: Gustavo Padovan, Maarten Lankhorst, Sean Paul, David Airlie,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
Cc: dri-devel, linux-kernel, intel-gfx
drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
arguments that can be removed by creating separate functins.
Create specific functions for these calls to reduce x86/64 defconfig
size by ~20k.
Modify the existing macros to use the specific calls.
new:
$ size -t drivers/gpu/drm/built-in.a | tail -1
1876562 44542 995 1922099 1d5433 (TOTALS)
old:
$ size -t drivers/gpu/drm/built-in.a | tail -1
1897565 44542 995 1943102 1da63e (TOTALS)
Miscellanea:
o intel_display requires a change to use the specific calls.
Signed-off-by: Joe Perches <joe@perches.com>
---
drivers/gpu/drm/drm_print.c | 28 +++++++++++++++++++++-------
drivers/gpu/drm/i915/intel_display.c | 15 ++++-----------
include/drm/drm_print.h | 27 ++++++++++++++-------------
3 files changed, 39 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 781518fd88e3..79abf6d5b4db 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -89,23 +89,37 @@ void drm_dev_printk(const struct device *dev, const char *level,
}
EXPORT_SYMBOL(drm_dev_printk);
-void drm_printk(const char *level, unsigned int category,
- const char *format, ...)
+void drm_dbg(unsigned int category, const char *format, ...)
{
struct va_format vaf;
va_list args;
- if (category != DRM_UT_NONE && !(drm_debug & category))
+ if (!(drm_debug & category))
return;
va_start(args, format);
vaf.fmt = format;
vaf.va = &args;
- printk("%s" "[" DRM_NAME ":%ps]%s %pV",
- level, __builtin_return_address(0),
- strcmp(level, KERN_ERR) == 0 ? " *ERROR*" : "", &vaf);
+ printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
+ __builtin_return_address(0), &vaf);
+
+ va_end(args);
+}
+EXPORT_SYMBOL(drm_dbg);
+
+void drm_err(const char *format, ...)
+{
+ struct va_format vaf;
+ va_list args;
+
+ va_start(args, format);
+ vaf.fmt = format;
+ vaf.va = &args;
+
+ printk(KERN_ERR "[" DRM_NAME ":%ps] *ERROR* %pV",
+ __builtin_return_address(0), &vaf);
va_end(args);
}
-EXPORT_SYMBOL(drm_printk);
+EXPORT_SYMBOL(drm_err);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 2933ad38094f..d8e522e3cd39 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11059,24 +11059,17 @@ intel_compare_link_m_n(const struct intel_link_m_n *m_n,
static void __printf(3, 4)
pipe_config_err(bool adjust, const char *name, const char *format, ...)
{
- char *level;
- unsigned int category;
struct va_format vaf;
va_list args;
- if (adjust) {
- level = KERN_DEBUG;
- category = DRM_UT_KMS;
- } else {
- level = KERN_ERR;
- category = DRM_UT_NONE;
- }
-
va_start(args, format);
vaf.fmt = format;
vaf.va = &args;
- drm_printk(level, category, "mismatch in %s %pV", name, &vaf);
+ if (adjust)
+ drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, &vaf);
+ else
+ drm_err("mismatch in %s %pV", name, &vaf);
va_end(args);
}
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 2a4a42e59a47..3a40c5a3a5fa 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -200,9 +200,10 @@ __printf(6, 7)
void drm_dev_printk(const struct device *dev, const char *level,
unsigned int category, const char *function_name,
const char *prefix, const char *format, ...);
-__printf(3, 4)
-void drm_printk(const char *level, unsigned int category,
- const char *format, ...);
+__printf(2, 3)
+void drm_dbg(unsigned int category, const char *format, ...);
+__printf(1, 2)
+void drm_err(const char *format, ...);
/* Macros to make printk easier */
@@ -236,7 +237,7 @@ void drm_printk(const char *level, unsigned int category,
drm_dev_printk(dev, KERN_ERR, DRM_UT_NONE, __func__, " *ERROR*",\
fmt, ##__VA_ARGS__)
#define DRM_ERROR(fmt, ...) \
- drm_printk(KERN_ERR, DRM_UT_NONE, fmt, ##__VA_ARGS__)
+ drm_err(fmt, ##__VA_ARGS__)
/**
* Rate limited error output. Like DRM_ERROR() but won't flood the log.
@@ -279,40 +280,40 @@ void drm_printk(const char *level, unsigned int category,
drm_dev_printk(dev, KERN_DEBUG, DRM_UT_CORE, __func__, "", fmt, \
##args)
#define DRM_DEBUG(fmt, ...) \
- drm_printk(KERN_DEBUG, DRM_UT_CORE, fmt, ##__VA_ARGS__)
+ drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__)
#define DRM_DEV_DEBUG_DRIVER(dev, fmt, args...) \
drm_dev_printk(dev, KERN_DEBUG, DRM_UT_DRIVER, __func__, "", \
fmt, ##args)
#define DRM_DEBUG_DRIVER(fmt, ...) \
- drm_printk(KERN_DEBUG, DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
+ drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
#define DRM_DEV_DEBUG_KMS(dev, fmt, args...) \
drm_dev_printk(dev, KERN_DEBUG, DRM_UT_KMS, __func__, "", fmt, \
##args)
-#define DRM_DEBUG_KMS(fmt, ...) \
- drm_printk(KERN_DEBUG, DRM_UT_KMS, fmt, ##__VA_ARGS__)
+#define DRM_DEBUG_KMS(fmt, ...) \
+ drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__)
#define DRM_DEV_DEBUG_PRIME(dev, fmt, args...) \
drm_dev_printk(dev, KERN_DEBUG, DRM_UT_PRIME, __func__, "", \
fmt, ##args)
#define DRM_DEBUG_PRIME(fmt, ...) \
- drm_printk(KERN_DEBUG, DRM_UT_PRIME, fmt, ##__VA_ARGS__)
+ drm_dbg(DRM_UT_PRIME, fmt, ##__VA_ARGS__)
#define DRM_DEV_DEBUG_ATOMIC(dev, fmt, args...) \
drm_dev_printk(dev, KERN_DEBUG, DRM_UT_ATOMIC, __func__, "", \
fmt, ##args)
#define DRM_DEBUG_ATOMIC(fmt, ...) \
- drm_printk(KERN_DEBUG, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
+ drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
#define DRM_DEV_DEBUG_VBL(dev, fmt, args...) \
drm_dev_printk(dev, KERN_DEBUG, DRM_UT_VBL, __func__, "", fmt, \
##args)
-#define DRM_DEBUG_VBL(fmt, ...) \
- drm_printk(KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__)
+#define DRM_DEBUG_VBL(fmt, ...) \
+ drm_dbg(DRM_UT_VBL, fmt, ##__VA_ARGS__)
#define DRM_DEBUG_LEASE(fmt, ...) \
- drm_printk(KERN_DEBUG, DRM_UT_LEASE, fmt, ##__VA_ARGS__)
+ drm_dbg(DRM_UT_LEASE, fmt, ##__VA_ARGS__)
#define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...) \
({ \
--
2.15.0
^ permalink raw reply related [flat|nested] 27+ messages in thread
* ✓ Fi.CI.BAT: success for drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
2018-03-13 22:02 [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses Joe Perches
@ 2018-03-13 23:33 ` Patchwork
2018-03-14 2:16 ` ✓ Fi.CI.IGT: " Patchwork
` (3 subsequent siblings)
4 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-03-13 23:33 UTC (permalink / raw)
To: Joe Perches; +Cc: intel-gfx
== Series Details ==
Series: drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
URL : https://patchwork.freedesktop.org/series/39911/
State : success
== Summary ==
Series 39911v1 drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
https://patchwork.freedesktop.org/api/1.0/series/39911/revisions/1/mbox/
fi-bdw-5557u total:288 pass:267 dwarn:0 dfail:0 fail:0 skip:21 time:440s
fi-bdw-gvtdvm total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:440s
fi-blb-e6850 total:288 pass:223 dwarn:1 dfail:0 fail:0 skip:64 time:382s
fi-bsw-n3050 total:288 pass:242 dwarn:0 dfail:0 fail:0 skip:46 time:526s
fi-bwr-2160 total:288 pass:183 dwarn:0 dfail:0 fail:0 skip:105 time:299s
fi-bxt-dsi total:288 pass:258 dwarn:0 dfail:0 fail:0 skip:30 time:508s
fi-bxt-j4205 total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:509s
fi-byt-j1900 total:288 pass:253 dwarn:0 dfail:0 fail:0 skip:35 time:507s
fi-byt-n2820 total:288 pass:249 dwarn:0 dfail:0 fail:0 skip:39 time:504s
fi-cfl-8700k total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:410s
fi-cfl-s2 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:581s
fi-cnl-y3 total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:587s
fi-elk-e7500 total:288 pass:229 dwarn:0 dfail:0 fail:0 skip:59 time:425s
fi-gdg-551 total:288 pass:179 dwarn:0 dfail:0 fail:1 skip:108 time:316s
fi-glk-1 total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:530s
fi-hsw-4770 total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:402s
fi-ilk-650 total:288 pass:228 dwarn:0 dfail:0 fail:0 skip:60 time:418s
fi-ivb-3520m total:288 pass:259 dwarn:0 dfail:0 fail:0 skip:29 time:471s
fi-ivb-3770 total:288 pass:255 dwarn:0 dfail:0 fail:0 skip:33 time:434s
fi-kbl-7500u total:288 pass:263 dwarn:1 dfail:0 fail:0 skip:24 time:474s
fi-kbl-7567u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:472s
fi-kbl-r total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:515s
fi-skl-6260u total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:441s
fi-skl-6600u total:288 pass:261 dwarn:0 dfail:0 fail:0 skip:27 time:530s
fi-skl-6700hq total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:540s
fi-skl-6700k2 total:288 pass:264 dwarn:0 dfail:0 fail:0 skip:24 time:507s
fi-skl-6770hq total:288 pass:268 dwarn:0 dfail:0 fail:0 skip:20 time:495s
fi-skl-guc total:288 pass:260 dwarn:0 dfail:0 fail:0 skip:28 time:430s
fi-skl-gvtdvm total:288 pass:265 dwarn:0 dfail:0 fail:0 skip:23 time:438s
fi-snb-2520m total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:535s
fi-snb-2600 total:288 pass:248 dwarn:0 dfail:0 fail:0 skip:40 time:398s
Blacklisted hosts:
fi-cfl-u total:288 pass:262 dwarn:0 dfail:0 fail:0 skip:26 time:510s
fi-cnl-drrs total:288 pass:257 dwarn:3 dfail:0 fail:0 skip:28 time:526s
c30d96215a171b633dae0098d1e62a2556c8bf90 drm-tip: 2018y-03m-13d-22h-35m-59s UTC integration manifest
490bd7abc493 drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8334/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* ✓ Fi.CI.IGT: success for drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
2018-03-13 22:02 [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses Joe Perches
2018-03-13 23:33 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-03-14 2:16 ` Patchwork
2018-03-15 13:22 ` Maarten Lankhorst
` (2 subsequent siblings)
4 siblings, 0 replies; 27+ messages in thread
From: Patchwork @ 2018-03-14 2:16 UTC (permalink / raw)
To: Joe Perches; +Cc: intel-gfx
== Series Details ==
Series: drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
URL : https://patchwork.freedesktop.org/series/39911/
State : success
== Summary ==
---- Known issues:
Test gem_eio:
Subgroup in-flight-contexts:
pass -> INCOMPLETE (shard-apl) fdo#105341
Test gem_softpin:
Subgroup noreloc-s3:
skip -> PASS (shard-snb) fdo#103375 +1
Test kms_cursor_crc:
Subgroup cursor-128x128-suspend:
pass -> INCOMPLETE (shard-hsw) fdo#103540
Test kms_flip:
Subgroup dpms-vs-vblank-race:
pass -> FAIL (shard-hsw) fdo#103060
Test kms_vblank:
Subgroup pipe-b-ts-continuation-suspend:
skip -> PASS (shard-snb) fdo#105411
fdo#105341 https://bugs.freedesktop.org/show_bug.cgi?id=105341
fdo#103375 https://bugs.freedesktop.org/show_bug.cgi?id=103375
fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
shard-apl total:3360 pass:1777 dwarn:1 dfail:0 fail:6 skip:1574 time:12727s
shard-hsw total:3357 pass:1720 dwarn:1 dfail:0 fail:2 skip:1632 time:11180s
shard-snb total:3444 pass:1360 dwarn:1 dfail:0 fail:2 skip:2081 time:7233s
Blacklisted hosts:
shard-kbl total:3234 pass:1828 dwarn:1 dfail:0 fail:9 skip:1393 time:9129s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8334/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
2018-03-13 22:02 [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses Joe Perches
@ 2018-03-15 13:22 ` Maarten Lankhorst
2018-03-14 2:16 ` ✓ Fi.CI.IGT: " Patchwork
` (3 subsequent siblings)
4 siblings, 0 replies; 27+ messages in thread
From: Maarten Lankhorst @ 2018-03-15 13:22 UTC (permalink / raw)
To: Joe Perches, Gustavo Padovan, Sean Paul, David Airlie,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
Cc: dri-devel, linux-kernel, intel-gfx
Op 13-03-18 om 23:02 schreef Joe Perches:
> drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> arguments that can be removed by creating separate functins.
>
> Create specific functions for these calls to reduce x86/64 defconfig
> size by ~20k.
>
> Modify the existing macros to use the specific calls.
>
> new:
> $ size -t drivers/gpu/drm/built-in.a | tail -1
> 1876562 44542 995 1922099 1d5433 (TOTALS)
>
> old:
> $ size -t drivers/gpu/drm/built-in.a | tail -1
> 1897565 44542 995 1943102 1da63e (TOTALS)
>
> Miscellanea:
>
> o intel_display requires a change to use the specific calls.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
I guess this adds up. Nice reduction. :)
> drivers/gpu/drm/drm_print.c | 28 +++++++++++++++++++++-------
> drivers/gpu/drm/i915/intel_display.c | 15 ++++-----------
> include/drm/drm_print.h | 27 ++++++++++++++-------------
> 3 files changed, 39 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 781518fd88e3..79abf6d5b4db 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -89,23 +89,37 @@ void drm_dev_printk(const struct device *dev, const char *level,
> }
> EXPORT_SYMBOL(drm_dev_printk);
>
> -void drm_printk(const char *level, unsigned int category,
> - const char *format, ...)
> +void drm_dbg(unsigned int category, const char *format, ...)
> {
> struct va_format vaf;
> va_list args;
>
> - if (category != DRM_UT_NONE && !(drm_debug & category))
> + if (!(drm_debug & category))
> return;
>
> va_start(args, format);
> vaf.fmt = format;
> vaf.va = &args;
>
> - printk("%s" "[" DRM_NAME ":%ps]%s %pV",
> - level, __builtin_return_address(0),
> - strcmp(level, KERN_ERR) == 0 ? " *ERROR*" : "", &vaf);
> + printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
> + __builtin_return_address(0), &vaf);
> +
> + va_end(args);
> +}
> +EXPORT_SYMBOL(drm_dbg);
> +
> +void drm_err(const char *format, ...)
> +{
> + struct va_format vaf;
> + va_list args;
> +
> + va_start(args, format);
> + vaf.fmt = format;
> + vaf.va = &args;
> +
> + printk(KERN_ERR "[" DRM_NAME ":%ps] *ERROR* %pV",
> + __builtin_return_address(0), &vaf);
>
> va_end(args);
> }
> -EXPORT_SYMBOL(drm_printk);
> +EXPORT_SYMBOL(drm_err);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2933ad38094f..d8e522e3cd39 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11059,24 +11059,17 @@ intel_compare_link_m_n(const struct intel_link_m_n *m_n,
> static void __printf(3, 4)
> pipe_config_err(bool adjust, const char *name, const char *format, ...)
> {
> - char *level;
> - unsigned int category;
> struct va_format vaf;
> va_list args;
>
> - if (adjust) {
> - level = KERN_DEBUG;
> - category = DRM_UT_KMS;
> - } else {
> - level = KERN_ERR;
> - category = DRM_UT_NONE;
> - }
> -
> va_start(args, format);
> vaf.fmt = format;
> vaf.va = &args;
>
> - drm_printk(level, category, "mismatch in %s %pV", name, &vaf);
> + if (adjust)
> + drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, &vaf);
> + else
> + drm_err("mismatch in %s %pV", name, &vaf);
Could this use DRM_DEBUG_KMS/DRM_ERROR?
Rest looks good, so I can fix up if you want.
~Maarten
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
@ 2018-03-15 13:22 ` Maarten Lankhorst
0 siblings, 0 replies; 27+ messages in thread
From: Maarten Lankhorst @ 2018-03-15 13:22 UTC (permalink / raw)
To: Joe Perches, Gustavo Padovan, Sean Paul, David Airlie,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
Cc: intel-gfx, linux-kernel, dri-devel
Op 13-03-18 om 23:02 schreef Joe Perches:
> drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> arguments that can be removed by creating separate functins.
>
> Create specific functions for these calls to reduce x86/64 defconfig
> size by ~20k.
>
> Modify the existing macros to use the specific calls.
>
> new:
> $ size -t drivers/gpu/drm/built-in.a | tail -1
> 1876562 44542 995 1922099 1d5433 (TOTALS)
>
> old:
> $ size -t drivers/gpu/drm/built-in.a | tail -1
> 1897565 44542 995 1943102 1da63e (TOTALS)
>
> Miscellanea:
>
> o intel_display requires a change to use the specific calls.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
I guess this adds up. Nice reduction. :)
> drivers/gpu/drm/drm_print.c | 28 +++++++++++++++++++++-------
> drivers/gpu/drm/i915/intel_display.c | 15 ++++-----------
> include/drm/drm_print.h | 27 ++++++++++++++-------------
> 3 files changed, 39 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 781518fd88e3..79abf6d5b4db 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -89,23 +89,37 @@ void drm_dev_printk(const struct device *dev, const char *level,
> }
> EXPORT_SYMBOL(drm_dev_printk);
>
> -void drm_printk(const char *level, unsigned int category,
> - const char *format, ...)
> +void drm_dbg(unsigned int category, const char *format, ...)
> {
> struct va_format vaf;
> va_list args;
>
> - if (category != DRM_UT_NONE && !(drm_debug & category))
> + if (!(drm_debug & category))
> return;
>
> va_start(args, format);
> vaf.fmt = format;
> vaf.va = &args;
>
> - printk("%s" "[" DRM_NAME ":%ps]%s %pV",
> - level, __builtin_return_address(0),
> - strcmp(level, KERN_ERR) == 0 ? " *ERROR*" : "", &vaf);
> + printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
> + __builtin_return_address(0), &vaf);
> +
> + va_end(args);
> +}
> +EXPORT_SYMBOL(drm_dbg);
> +
> +void drm_err(const char *format, ...)
> +{
> + struct va_format vaf;
> + va_list args;
> +
> + va_start(args, format);
> + vaf.fmt = format;
> + vaf.va = &args;
> +
> + printk(KERN_ERR "[" DRM_NAME ":%ps] *ERROR* %pV",
> + __builtin_return_address(0), &vaf);
>
> va_end(args);
> }
> -EXPORT_SYMBOL(drm_printk);
> +EXPORT_SYMBOL(drm_err);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2933ad38094f..d8e522e3cd39 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11059,24 +11059,17 @@ intel_compare_link_m_n(const struct intel_link_m_n *m_n,
> static void __printf(3, 4)
> pipe_config_err(bool adjust, const char *name, const char *format, ...)
> {
> - char *level;
> - unsigned int category;
> struct va_format vaf;
> va_list args;
>
> - if (adjust) {
> - level = KERN_DEBUG;
> - category = DRM_UT_KMS;
> - } else {
> - level = KERN_ERR;
> - category = DRM_UT_NONE;
> - }
> -
> va_start(args, format);
> vaf.fmt = format;
> vaf.va = &args;
>
> - drm_printk(level, category, "mismatch in %s %pV", name, &vaf);
> + if (adjust)
> + drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, &vaf);
> + else
> + drm_err("mismatch in %s %pV", name, &vaf);
Could this use DRM_DEBUG_KMS/DRM_ERROR?
Rest looks good, so I can fix up if you want.
~Maarten
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
2018-03-13 22:02 [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses Joe Perches
@ 2018-03-15 13:30 ` Ville Syrjälä
2018-03-14 2:16 ` ✓ Fi.CI.IGT: " Patchwork
` (3 subsequent siblings)
4 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2018-03-15 13:30 UTC (permalink / raw)
To: Joe Perches
Cc: Gustavo Padovan, Maarten Lankhorst, Sean Paul, David Airlie,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, intel-gfx,
linux-kernel, dri-devel
On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> arguments that can be removed by creating separate functins.
>
> Create specific functions for these calls to reduce x86/64 defconfig
> size by ~20k.
>
> Modify the existing macros to use the specific calls.
>
> new:
> $ size -t drivers/gpu/drm/built-in.a | tail -1
> 1876562 44542 995 1922099 1d5433 (TOTALS)
>
> old:
> $ size -t drivers/gpu/drm/built-in.a | tail -1
> 1897565 44542 995 1943102 1da63e (TOTALS)
>
> Miscellanea:
>
> o intel_display requires a change to use the specific calls.
How much would we lose if we move the (drm_debug&FOO) outside the
functions again? I'm somewhat concerned about all the function call
overhead when debugs aren't even enabled.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> drivers/gpu/drm/drm_print.c | 28 +++++++++++++++++++++-------
> drivers/gpu/drm/i915/intel_display.c | 15 ++++-----------
> include/drm/drm_print.h | 27 ++++++++++++++-------------
> 3 files changed, 39 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 781518fd88e3..79abf6d5b4db 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -89,23 +89,37 @@ void drm_dev_printk(const struct device *dev, const char *level,
> }
> EXPORT_SYMBOL(drm_dev_printk);
>
> -void drm_printk(const char *level, unsigned int category,
> - const char *format, ...)
> +void drm_dbg(unsigned int category, const char *format, ...)
> {
> struct va_format vaf;
> va_list args;
>
> - if (category != DRM_UT_NONE && !(drm_debug & category))
> + if (!(drm_debug & category))
> return;
>
> va_start(args, format);
> vaf.fmt = format;
> vaf.va = &args;
>
> - printk("%s" "[" DRM_NAME ":%ps]%s %pV",
> - level, __builtin_return_address(0),
> - strcmp(level, KERN_ERR) == 0 ? " *ERROR*" : "", &vaf);
> + printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
> + __builtin_return_address(0), &vaf);
> +
> + va_end(args);
> +}
> +EXPORT_SYMBOL(drm_dbg);
> +
> +void drm_err(const char *format, ...)
> +{
> + struct va_format vaf;
> + va_list args;
> +
> + va_start(args, format);
> + vaf.fmt = format;
> + vaf.va = &args;
> +
> + printk(KERN_ERR "[" DRM_NAME ":%ps] *ERROR* %pV",
> + __builtin_return_address(0), &vaf);
>
> va_end(args);
> }
> -EXPORT_SYMBOL(drm_printk);
> +EXPORT_SYMBOL(drm_err);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2933ad38094f..d8e522e3cd39 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11059,24 +11059,17 @@ intel_compare_link_m_n(const struct intel_link_m_n *m_n,
> static void __printf(3, 4)
> pipe_config_err(bool adjust, const char *name, const char *format, ...)
> {
> - char *level;
> - unsigned int category;
> struct va_format vaf;
> va_list args;
>
> - if (adjust) {
> - level = KERN_DEBUG;
> - category = DRM_UT_KMS;
> - } else {
> - level = KERN_ERR;
> - category = DRM_UT_NONE;
> - }
> -
> va_start(args, format);
> vaf.fmt = format;
> vaf.va = &args;
>
> - drm_printk(level, category, "mismatch in %s %pV", name, &vaf);
> + if (adjust)
> + drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, &vaf);
> + else
> + drm_err("mismatch in %s %pV", name, &vaf);
>
> va_end(args);
> }
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 2a4a42e59a47..3a40c5a3a5fa 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -200,9 +200,10 @@ __printf(6, 7)
> void drm_dev_printk(const struct device *dev, const char *level,
> unsigned int category, const char *function_name,
> const char *prefix, const char *format, ...);
> -__printf(3, 4)
> -void drm_printk(const char *level, unsigned int category,
> - const char *format, ...);
> +__printf(2, 3)
> +void drm_dbg(unsigned int category, const char *format, ...);
> +__printf(1, 2)
> +void drm_err(const char *format, ...);
>
> /* Macros to make printk easier */
>
> @@ -236,7 +237,7 @@ void drm_printk(const char *level, unsigned int category,
> drm_dev_printk(dev, KERN_ERR, DRM_UT_NONE, __func__, " *ERROR*",\
> fmt, ##__VA_ARGS__)
> #define DRM_ERROR(fmt, ...) \
> - drm_printk(KERN_ERR, DRM_UT_NONE, fmt, ##__VA_ARGS__)
> + drm_err(fmt, ##__VA_ARGS__)
>
> /**
> * Rate limited error output. Like DRM_ERROR() but won't flood the log.
> @@ -279,40 +280,40 @@ void drm_printk(const char *level, unsigned int category,
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_CORE, __func__, "", fmt, \
> ##args)
> #define DRM_DEBUG(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_CORE, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_DRIVER(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_DRIVER, __func__, "", \
> fmt, ##args)
> #define DRM_DEBUG_DRIVER(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_KMS(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_KMS, __func__, "", fmt, \
> ##args)
> -#define DRM_DEBUG_KMS(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_KMS, fmt, ##__VA_ARGS__)
> +#define DRM_DEBUG_KMS(fmt, ...) \
> + drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_PRIME(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_PRIME, __func__, "", \
> fmt, ##args)
> #define DRM_DEBUG_PRIME(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_PRIME, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_PRIME, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_ATOMIC(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_ATOMIC, __func__, "", \
> fmt, ##args)
> #define DRM_DEBUG_ATOMIC(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_VBL(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_VBL, __func__, "", fmt, \
> ##args)
> -#define DRM_DEBUG_VBL(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__)
> +#define DRM_DEBUG_VBL(fmt, ...) \
> + drm_dbg(DRM_UT_VBL, fmt, ##__VA_ARGS__)
>
> #define DRM_DEBUG_LEASE(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_LEASE, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_LEASE, fmt, ##__VA_ARGS__)
>
> #define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...) \
> ({ \
> --
> 2.15.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
@ 2018-03-15 13:30 ` Ville Syrjälä
0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2018-03-15 13:30 UTC (permalink / raw)
To: Joe Perches
Cc: David Airlie, intel-gfx, linux-kernel, dri-devel, Rodrigo Vivi
On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> arguments that can be removed by creating separate functins.
>
> Create specific functions for these calls to reduce x86/64 defconfig
> size by ~20k.
>
> Modify the existing macros to use the specific calls.
>
> new:
> $ size -t drivers/gpu/drm/built-in.a | tail -1
> 1876562 44542 995 1922099 1d5433 (TOTALS)
>
> old:
> $ size -t drivers/gpu/drm/built-in.a | tail -1
> 1897565 44542 995 1943102 1da63e (TOTALS)
>
> Miscellanea:
>
> o intel_display requires a change to use the specific calls.
How much would we lose if we move the (drm_debug&FOO) outside the
functions again? I'm somewhat concerned about all the function call
overhead when debugs aren't even enabled.
>
> Signed-off-by: Joe Perches <joe@perches.com>
> ---
> drivers/gpu/drm/drm_print.c | 28 +++++++++++++++++++++-------
> drivers/gpu/drm/i915/intel_display.c | 15 ++++-----------
> include/drm/drm_print.h | 27 ++++++++++++++-------------
> 3 files changed, 39 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 781518fd88e3..79abf6d5b4db 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -89,23 +89,37 @@ void drm_dev_printk(const struct device *dev, const char *level,
> }
> EXPORT_SYMBOL(drm_dev_printk);
>
> -void drm_printk(const char *level, unsigned int category,
> - const char *format, ...)
> +void drm_dbg(unsigned int category, const char *format, ...)
> {
> struct va_format vaf;
> va_list args;
>
> - if (category != DRM_UT_NONE && !(drm_debug & category))
> + if (!(drm_debug & category))
> return;
>
> va_start(args, format);
> vaf.fmt = format;
> vaf.va = &args;
>
> - printk("%s" "[" DRM_NAME ":%ps]%s %pV",
> - level, __builtin_return_address(0),
> - strcmp(level, KERN_ERR) == 0 ? " *ERROR*" : "", &vaf);
> + printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
> + __builtin_return_address(0), &vaf);
> +
> + va_end(args);
> +}
> +EXPORT_SYMBOL(drm_dbg);
> +
> +void drm_err(const char *format, ...)
> +{
> + struct va_format vaf;
> + va_list args;
> +
> + va_start(args, format);
> + vaf.fmt = format;
> + vaf.va = &args;
> +
> + printk(KERN_ERR "[" DRM_NAME ":%ps] *ERROR* %pV",
> + __builtin_return_address(0), &vaf);
>
> va_end(args);
> }
> -EXPORT_SYMBOL(drm_printk);
> +EXPORT_SYMBOL(drm_err);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2933ad38094f..d8e522e3cd39 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11059,24 +11059,17 @@ intel_compare_link_m_n(const struct intel_link_m_n *m_n,
> static void __printf(3, 4)
> pipe_config_err(bool adjust, const char *name, const char *format, ...)
> {
> - char *level;
> - unsigned int category;
> struct va_format vaf;
> va_list args;
>
> - if (adjust) {
> - level = KERN_DEBUG;
> - category = DRM_UT_KMS;
> - } else {
> - level = KERN_ERR;
> - category = DRM_UT_NONE;
> - }
> -
> va_start(args, format);
> vaf.fmt = format;
> vaf.va = &args;
>
> - drm_printk(level, category, "mismatch in %s %pV", name, &vaf);
> + if (adjust)
> + drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, &vaf);
> + else
> + drm_err("mismatch in %s %pV", name, &vaf);
>
> va_end(args);
> }
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 2a4a42e59a47..3a40c5a3a5fa 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -200,9 +200,10 @@ __printf(6, 7)
> void drm_dev_printk(const struct device *dev, const char *level,
> unsigned int category, const char *function_name,
> const char *prefix, const char *format, ...);
> -__printf(3, 4)
> -void drm_printk(const char *level, unsigned int category,
> - const char *format, ...);
> +__printf(2, 3)
> +void drm_dbg(unsigned int category, const char *format, ...);
> +__printf(1, 2)
> +void drm_err(const char *format, ...);
>
> /* Macros to make printk easier */
>
> @@ -236,7 +237,7 @@ void drm_printk(const char *level, unsigned int category,
> drm_dev_printk(dev, KERN_ERR, DRM_UT_NONE, __func__, " *ERROR*",\
> fmt, ##__VA_ARGS__)
> #define DRM_ERROR(fmt, ...) \
> - drm_printk(KERN_ERR, DRM_UT_NONE, fmt, ##__VA_ARGS__)
> + drm_err(fmt, ##__VA_ARGS__)
>
> /**
> * Rate limited error output. Like DRM_ERROR() but won't flood the log.
> @@ -279,40 +280,40 @@ void drm_printk(const char *level, unsigned int category,
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_CORE, __func__, "", fmt, \
> ##args)
> #define DRM_DEBUG(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_CORE, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_DRIVER(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_DRIVER, __func__, "", \
> fmt, ##args)
> #define DRM_DEBUG_DRIVER(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_KMS(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_KMS, __func__, "", fmt, \
> ##args)
> -#define DRM_DEBUG_KMS(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_KMS, fmt, ##__VA_ARGS__)
> +#define DRM_DEBUG_KMS(fmt, ...) \
> + drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_PRIME(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_PRIME, __func__, "", \
> fmt, ##args)
> #define DRM_DEBUG_PRIME(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_PRIME, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_PRIME, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_ATOMIC(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_ATOMIC, __func__, "", \
> fmt, ##args)
> #define DRM_DEBUG_ATOMIC(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_VBL(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_VBL, __func__, "", fmt, \
> ##args)
> -#define DRM_DEBUG_VBL(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__)
> +#define DRM_DEBUG_VBL(fmt, ...) \
> + drm_dbg(DRM_UT_VBL, fmt, ##__VA_ARGS__)
>
> #define DRM_DEBUG_LEASE(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_LEASE, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_LEASE, fmt, ##__VA_ARGS__)
>
> #define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...) \
> ({ \
> --
> 2.15.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
2018-03-15 13:30 ` Ville Syrjälä
@ 2018-03-15 14:04 ` Maarten Lankhorst
-1 siblings, 0 replies; 27+ messages in thread
From: Maarten Lankhorst @ 2018-03-15 14:04 UTC (permalink / raw)
To: Ville Syrjälä, Joe Perches
Cc: Gustavo Padovan, Sean Paul, David Airlie, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, intel-gfx, linux-kernel,
dri-devel
Op 15-03-18 om 14:30 schreef Ville Syrjälä:
> On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
>> drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
>> arguments that can be removed by creating separate functins.
>>
>> Create specific functions for these calls to reduce x86/64 defconfig
>> size by ~20k.
>>
>> Modify the existing macros to use the specific calls.
>>
>> new:
>> $ size -t drivers/gpu/drm/built-in.a | tail -1
>> 1876562 44542 995 1922099 1d5433 (TOTALS)
>>
>> old:
>> $ size -t drivers/gpu/drm/built-in.a | tail -1
>> 1897565 44542 995 1943102 1da63e (TOTALS)
>>
>> Miscellanea:
>>
>> o intel_display requires a change to use the specific calls.
> How much would we lose if we move the (drm_debug&FOO) outside the
> functions again? I'm somewhat concerned about all the function call
> overhead when debugs aren't even enabled.
Upstream:
text data bss dec hex filename
377143 5689 4352 387184 5e870 drivers/gpu/drm/drm.ko
With this patch:
373831 5689 4352 383872 5db80 drivers/gpu/drm/drm.ko
Moving the if outside (below):
377629 5689 4352 387670 5ea56 drivers/gpu/drm/drm.ko
Bye savings..
I don't think there are any places in which the debug output is performance sensitive,
so I'm ok with not inlining.
---
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 79abf6d5b4db..928822403a59 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -89,14 +89,11 @@ void drm_dev_printk(const struct device *dev, const char *level,
}
EXPORT_SYMBOL(drm_dev_printk);
-void drm_dbg(unsigned int category, const char *format, ...)
+void __drm_dbg(const char *format, ...)
{
struct va_format vaf;
va_list args;
- if (!(drm_debug & category))
- return;
-
va_start(args, format);
vaf.fmt = format;
vaf.va = &args;
@@ -106,7 +103,7 @@ void drm_dbg(unsigned int category, const char *format, ...)
va_end(args);
}
-EXPORT_SYMBOL(drm_dbg);
+EXPORT_SYMBOL(__drm_dbg);
void drm_err(const char *format, ...)
{
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 3a40c5a3a5fa..2a145b97bdfc 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -200,8 +200,17 @@ __printf(6, 7)
void drm_dev_printk(const struct device *dev, const char *level,
unsigned int category, const char *function_name,
const char *prefix, const char *format, ...);
-__printf(2, 3)
-void drm_dbg(unsigned int category, const char *format, ...);
+
+__printf(1, 2)
+void __drm_dbg(const char *format, ...);
+
+
+#define drm_dbg(category, format, ...) \
+ do { \
+ if (drm_debug & category) \
+ __drm_dbg(format, ## __VA_ARGS__); \
+ } while (0)
+
__printf(1, 2)
void drm_err(const char *format, ...);
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
@ 2018-03-15 14:04 ` Maarten Lankhorst
0 siblings, 0 replies; 27+ messages in thread
From: Maarten Lankhorst @ 2018-03-15 14:04 UTC (permalink / raw)
To: Ville Syrjälä, Joe Perches
Cc: David Airlie, intel-gfx, linux-kernel, dri-devel, Rodrigo Vivi
Op 15-03-18 om 14:30 schreef Ville Syrjälä:
> On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
>> drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
>> arguments that can be removed by creating separate functins.
>>
>> Create specific functions for these calls to reduce x86/64 defconfig
>> size by ~20k.
>>
>> Modify the existing macros to use the specific calls.
>>
>> new:
>> $ size -t drivers/gpu/drm/built-in.a | tail -1
>> 1876562 44542 995 1922099 1d5433 (TOTALS)
>>
>> old:
>> $ size -t drivers/gpu/drm/built-in.a | tail -1
>> 1897565 44542 995 1943102 1da63e (TOTALS)
>>
>> Miscellanea:
>>
>> o intel_display requires a change to use the specific calls.
> How much would we lose if we move the (drm_debug&FOO) outside the
> functions again? I'm somewhat concerned about all the function call
> overhead when debugs aren't even enabled.
Upstream:
text data bss dec hex filename
377143 5689 4352 387184 5e870 drivers/gpu/drm/drm.ko
With this patch:
373831 5689 4352 383872 5db80 drivers/gpu/drm/drm.ko
Moving the if outside (below):
377629 5689 4352 387670 5ea56 drivers/gpu/drm/drm.ko
Bye savings..
I don't think there are any places in which the debug output is performance sensitive,
so I'm ok with not inlining.
---
diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
index 79abf6d5b4db..928822403a59 100644
--- a/drivers/gpu/drm/drm_print.c
+++ b/drivers/gpu/drm/drm_print.c
@@ -89,14 +89,11 @@ void drm_dev_printk(const struct device *dev, const char *level,
}
EXPORT_SYMBOL(drm_dev_printk);
-void drm_dbg(unsigned int category, const char *format, ...)
+void __drm_dbg(const char *format, ...)
{
struct va_format vaf;
va_list args;
- if (!(drm_debug & category))
- return;
-
va_start(args, format);
vaf.fmt = format;
vaf.va = &args;
@@ -106,7 +103,7 @@ void drm_dbg(unsigned int category, const char *format, ...)
va_end(args);
}
-EXPORT_SYMBOL(drm_dbg);
+EXPORT_SYMBOL(__drm_dbg);
void drm_err(const char *format, ...)
{
diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
index 3a40c5a3a5fa..2a145b97bdfc 100644
--- a/include/drm/drm_print.h
+++ b/include/drm/drm_print.h
@@ -200,8 +200,17 @@ __printf(6, 7)
void drm_dev_printk(const struct device *dev, const char *level,
unsigned int category, const char *function_name,
const char *prefix, const char *format, ...);
-__printf(2, 3)
-void drm_dbg(unsigned int category, const char *format, ...);
+
+__printf(1, 2)
+void __drm_dbg(const char *format, ...);
+
+
+#define drm_dbg(category, format, ...) \
+ do { \
+ if (drm_debug & category) \
+ __drm_dbg(format, ## __VA_ARGS__); \
+ } while (0)
+
__printf(1, 2)
void drm_err(const char *format, ...);
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
2018-03-15 13:22 ` Maarten Lankhorst
@ 2018-03-15 14:48 ` Joe Perches
-1 siblings, 0 replies; 27+ messages in thread
From: Joe Perches @ 2018-03-15 14:48 UTC (permalink / raw)
To: Maarten Lankhorst, Gustavo Padovan, Sean Paul, David Airlie,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
Cc: dri-devel, linux-kernel, intel-gfx
On Thu, 2018-03-15 at 14:22 +0100, Maarten Lankhorst wrote:
> Op 13-03-18 om 23:02 schreef Joe Perches:
> > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > arguments that can be removed by creating separate functins.
> >
> > Create specific functions for these calls to reduce x86/64 defconfig
> > size by ~20k.
> >
> > Modify the existing macros to use the specific calls.
> >
> > new:
> > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > 1876562 44542 995 1922099 1d5433 (TOTALS)
> >
> > old:
> > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > 1897565 44542 995 1943102 1da63e (TOTALS)
[]
> I guess this adds up. Nice reduction. :)
Yup. 1% of all drm object code.
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
[]
> >
> > - drm_printk(level, category, "mismatch in %s %pV", name, &vaf);
> > + if (adjust)
> > + drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, &vaf);
> > + else
> > + drm_err("mismatch in %s %pV", name, &vaf);
>
> Could this use DRM_DEBUG_KMS/DRM_ERROR?
>
> Rest looks good, so I can fix up if you want.
If want you change something like that, it should be separate patch.
btw: There was separate patch that also reduced object size
of the drm_dev_printk calls several months ago. Never applied.
https://lkml.org/lkml/2017/9/25/247
cheers, Joe
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
@ 2018-03-15 14:48 ` Joe Perches
0 siblings, 0 replies; 27+ messages in thread
From: Joe Perches @ 2018-03-15 14:48 UTC (permalink / raw)
To: Maarten Lankhorst, Gustavo Padovan, Sean Paul, David Airlie,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi
Cc: intel-gfx, linux-kernel, dri-devel
On Thu, 2018-03-15 at 14:22 +0100, Maarten Lankhorst wrote:
> Op 13-03-18 om 23:02 schreef Joe Perches:
> > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > arguments that can be removed by creating separate functins.
> >
> > Create specific functions for these calls to reduce x86/64 defconfig
> > size by ~20k.
> >
> > Modify the existing macros to use the specific calls.
> >
> > new:
> > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > 1876562 44542 995 1922099 1d5433 (TOTALS)
> >
> > old:
> > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > 1897565 44542 995 1943102 1da63e (TOTALS)
[]
> I guess this adds up. Nice reduction. :)
Yup. 1% of all drm object code.
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
[]
> >
> > - drm_printk(level, category, "mismatch in %s %pV", name, &vaf);
> > + if (adjust)
> > + drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, &vaf);
> > + else
> > + drm_err("mismatch in %s %pV", name, &vaf);
>
> Could this use DRM_DEBUG_KMS/DRM_ERROR?
>
> Rest looks good, so I can fix up if you want.
If want you change something like that, it should be separate patch.
btw: There was separate patch that also reduced object size
of the drm_dev_printk calls several months ago. Never applied.
https://lkml.org/lkml/2017/9/25/247
cheers, Joe
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
2018-03-15 14:04 ` Maarten Lankhorst
@ 2018-03-15 15:05 ` Ville Syrjälä
-1 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2018-03-15 15:05 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: Joe Perches, Gustavo Padovan, Sean Paul, David Airlie,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, intel-gfx,
linux-kernel, dri-devel
On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote:
> Op 15-03-18 om 14:30 schreef Ville Syrjälä:
> > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> >> drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> >> arguments that can be removed by creating separate functins.
> >>
> >> Create specific functions for these calls to reduce x86/64 defconfig
> >> size by ~20k.
> >>
> >> Modify the existing macros to use the specific calls.
> >>
> >> new:
> >> $ size -t drivers/gpu/drm/built-in.a | tail -1
> >> 1876562 44542 995 1922099 1d5433 (TOTALS)
> >>
> >> old:
> >> $ size -t drivers/gpu/drm/built-in.a | tail -1
> >> 1897565 44542 995 1943102 1da63e (TOTALS)
> >>
> >> Miscellanea:
> >>
> >> o intel_display requires a change to use the specific calls.
> > How much would we lose if we move the (drm_debug&FOO) outside the
> > functions again? I'm somewhat concerned about all the function call
> > overhead when debugs aren't even enabled.
>
> Upstream:
> text data bss dec hex filename
> 377143 5689 4352 387184 5e870 drivers/gpu/drm/drm.ko
>
> With this patch:
> 373831 5689 4352 383872 5db80 drivers/gpu/drm/drm.ko
>
> Moving the if outside (below):
> 377629 5689 4352 387670 5ea56 drivers/gpu/drm/drm.ko
>
> Bye savings..
>
> I don't think there are any places in which the debug output is performance sensitive,
> so I'm ok with not inlining.
Not performance sensitive as such perhaps. But pointlessly wasting cpu
cycles for nop function calls isn't particularly great. Would be nice
to actually measure how much overhead there is on some weaker systems.
IIRC older Atoms were particularly bad at this stuff.
> ---
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 79abf6d5b4db..928822403a59 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -89,14 +89,11 @@ void drm_dev_printk(const struct device *dev, const char *level,
> }
> EXPORT_SYMBOL(drm_dev_printk);
>
> -void drm_dbg(unsigned int category, const char *format, ...)
> +void __drm_dbg(const char *format, ...)
> {
> struct va_format vaf;
> va_list args;
>
> - if (!(drm_debug & category))
> - return;
> -
> va_start(args, format);
> vaf.fmt = format;
> vaf.va = &args;
> @@ -106,7 +103,7 @@ void drm_dbg(unsigned int category, const char *format, ...)
>
> va_end(args);
> }
> -EXPORT_SYMBOL(drm_dbg);
> +EXPORT_SYMBOL(__drm_dbg);
>
> void drm_err(const char *format, ...)
> {
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 3a40c5a3a5fa..2a145b97bdfc 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -200,8 +200,17 @@ __printf(6, 7)
> void drm_dev_printk(const struct device *dev, const char *level,
> unsigned int category, const char *function_name,
> const char *prefix, const char *format, ...);
> -__printf(2, 3)
> -void drm_dbg(unsigned int category, const char *format, ...);
> +
> +__printf(1, 2)
> +void __drm_dbg(const char *format, ...);
> +
> +
> +#define drm_dbg(category, format, ...) \
> + do { \
> + if (drm_debug & category) \
> + __drm_dbg(format, ## __VA_ARGS__); \
> + } while (0)
> +
> __printf(1, 2)
> void drm_err(const char *format, ...);
>
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
@ 2018-03-15 15:05 ` Ville Syrjälä
0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2018-03-15 15:05 UTC (permalink / raw)
To: Maarten Lankhorst
Cc: David Airlie, intel-gfx, linux-kernel, dri-devel, Rodrigo Vivi,
Joe Perches
On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote:
> Op 15-03-18 om 14:30 schreef Ville Syrjälä:
> > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> >> drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> >> arguments that can be removed by creating separate functins.
> >>
> >> Create specific functions for these calls to reduce x86/64 defconfig
> >> size by ~20k.
> >>
> >> Modify the existing macros to use the specific calls.
> >>
> >> new:
> >> $ size -t drivers/gpu/drm/built-in.a | tail -1
> >> 1876562 44542 995 1922099 1d5433 (TOTALS)
> >>
> >> old:
> >> $ size -t drivers/gpu/drm/built-in.a | tail -1
> >> 1897565 44542 995 1943102 1da63e (TOTALS)
> >>
> >> Miscellanea:
> >>
> >> o intel_display requires a change to use the specific calls.
> > How much would we lose if we move the (drm_debug&FOO) outside the
> > functions again? I'm somewhat concerned about all the function call
> > overhead when debugs aren't even enabled.
>
> Upstream:
> text data bss dec hex filename
> 377143 5689 4352 387184 5e870 drivers/gpu/drm/drm.ko
>
> With this patch:
> 373831 5689 4352 383872 5db80 drivers/gpu/drm/drm.ko
>
> Moving the if outside (below):
> 377629 5689 4352 387670 5ea56 drivers/gpu/drm/drm.ko
>
> Bye savings..
>
> I don't think there are any places in which the debug output is performance sensitive,
> so I'm ok with not inlining.
Not performance sensitive as such perhaps. But pointlessly wasting cpu
cycles for nop function calls isn't particularly great. Would be nice
to actually measure how much overhead there is on some weaker systems.
IIRC older Atoms were particularly bad at this stuff.
> ---
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 79abf6d5b4db..928822403a59 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -89,14 +89,11 @@ void drm_dev_printk(const struct device *dev, const char *level,
> }
> EXPORT_SYMBOL(drm_dev_printk);
>
> -void drm_dbg(unsigned int category, const char *format, ...)
> +void __drm_dbg(const char *format, ...)
> {
> struct va_format vaf;
> va_list args;
>
> - if (!(drm_debug & category))
> - return;
> -
> va_start(args, format);
> vaf.fmt = format;
> vaf.va = &args;
> @@ -106,7 +103,7 @@ void drm_dbg(unsigned int category, const char *format, ...)
>
> va_end(args);
> }
> -EXPORT_SYMBOL(drm_dbg);
> +EXPORT_SYMBOL(__drm_dbg);
>
> void drm_err(const char *format, ...)
> {
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 3a40c5a3a5fa..2a145b97bdfc 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -200,8 +200,17 @@ __printf(6, 7)
> void drm_dev_printk(const struct device *dev, const char *level,
> unsigned int category, const char *function_name,
> const char *prefix, const char *format, ...);
> -__printf(2, 3)
> -void drm_dbg(unsigned int category, const char *format, ...);
> +
> +__printf(1, 2)
> +void __drm_dbg(const char *format, ...);
> +
> +
> +#define drm_dbg(category, format, ...) \
> + do { \
> + if (drm_debug & category) \
> + __drm_dbg(format, ## __VA_ARGS__); \
> + } while (0)
> +
> __printf(1, 2)
> void drm_err(const char *format, ...);
>
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
2018-03-15 15:05 ` Ville Syrjälä
(?)
@ 2018-03-15 15:17 ` Joe Perches
2018-03-15 15:37 ` Ville Syrjälä
-1 siblings, 1 reply; 27+ messages in thread
From: Joe Perches @ 2018-03-15 15:17 UTC (permalink / raw)
To: Ville Syrjälä, Maarten Lankhorst
Cc: Gustavo Padovan, Sean Paul, David Airlie, Jani Nikula,
Joonas Lahtinen, Rodrigo Vivi, intel-gfx, linux-kernel,
dri-devel
On Thu, 2018-03-15 at 17:05 +0200, Ville Syrjälä wrote:
> On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote:
> > Op 15-03-18 om 14:30 schreef Ville Syrjälä:
> > > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> > > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > > > arguments that can be removed by creating separate functins.
> > > >
> > > > Create specific functions for these calls to reduce x86/64 defconfig
> > > > size by ~20k.
> > > >
> > > > Modify the existing macros to use the specific calls.
> > > >
> > > > new:
> > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > 1876562 44542 995 1922099 1d5433 (TOTALS)
> > > >
> > > > old:
> > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > 1897565 44542 995 1943102 1da63e (TOTALS)
> > > >
> > > > Miscellanea:
> > > >
> > > > o intel_display requires a change to use the specific calls.
> > >
> > > How much would we lose if we move the (drm_debug&FOO) outside the
> > > functions again?
again?
> > > I'm somewhat concerned about all the function call
> > > overhead when debugs aren't even enabled.
Perhaps better to have compilation elimination
of the entire debug output instead.
I think you are discussing a different issue and
this discussion should not block this patch as
this patch has no impact other than code size
reduction.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
2018-03-15 15:17 ` Joe Perches
@ 2018-03-15 15:37 ` Ville Syrjälä
0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2018-03-15 15:37 UTC (permalink / raw)
To: Joe Perches
Cc: Maarten Lankhorst, Gustavo Padovan, Sean Paul, David Airlie,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, intel-gfx,
linux-kernel, dri-devel
On Thu, Mar 15, 2018 at 08:17:53AM -0700, Joe Perches wrote:
> On Thu, 2018-03-15 at 17:05 +0200, Ville Syrjälä wrote:
> > On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote:
> > > Op 15-03-18 om 14:30 schreef Ville Syrjälä:
> > > > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> > > > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > > > > arguments that can be removed by creating separate functins.
> > > > >
> > > > > Create specific functions for these calls to reduce x86/64 defconfig
> > > > > size by ~20k.
> > > > >
> > > > > Modify the existing macros to use the specific calls.
> > > > >
> > > > > new:
> > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > > 1876562 44542 995 1922099 1d5433 (TOTALS)
> > > > >
> > > > > old:
> > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > > 1897565 44542 995 1943102 1da63e (TOTALS)
> > > > >
> > > > > Miscellanea:
> > > > >
> > > > > o intel_display requires a change to use the specific calls.
> > > >
> > > > How much would we lose if we move the (drm_debug&FOO) outside the
> > > > functions again?
>
> again?
We used to do that. Someone changed it a while back, unintentially
I believe.
>
> > > > I'm somewhat concerned about all the function call
> > > > overhead when debugs aren't even enabled.
>
> Perhaps better to have compilation elimination
> of the entire debug output instead.
That would require every bug reporter to recompile the kernel first.
So this is not a solution we would ever seriously consider.
Not sure if it would be possible to use the alternatives thing to
eliminate the function calls unless the user boots wih drm.debug!=0?
>
> I think you are discussing a different issue and
> this discussion should not block this patch as
> this patch has no impact other than code size
> reduction.
But what is the goal of the code size reduction? I assume the main
goal is to make better use of the instruction cache to make the
code faster. If there's a tradeoff between smaller and slightly
faster vs. larger and a singificantly faster I tend to think we
should go for the latter option.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
@ 2018-03-15 15:37 ` Ville Syrjälä
0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2018-03-15 15:37 UTC (permalink / raw)
To: Joe Perches
Cc: David Airlie, intel-gfx, linux-kernel, dri-devel, Rodrigo Vivi
On Thu, Mar 15, 2018 at 08:17:53AM -0700, Joe Perches wrote:
> On Thu, 2018-03-15 at 17:05 +0200, Ville Syrjälä wrote:
> > On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote:
> > > Op 15-03-18 om 14:30 schreef Ville Syrjälä:
> > > > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> > > > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > > > > arguments that can be removed by creating separate functins.
> > > > >
> > > > > Create specific functions for these calls to reduce x86/64 defconfig
> > > > > size by ~20k.
> > > > >
> > > > > Modify the existing macros to use the specific calls.
> > > > >
> > > > > new:
> > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > > 1876562 44542 995 1922099 1d5433 (TOTALS)
> > > > >
> > > > > old:
> > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > > 1897565 44542 995 1943102 1da63e (TOTALS)
> > > > >
> > > > > Miscellanea:
> > > > >
> > > > > o intel_display requires a change to use the specific calls.
> > > >
> > > > How much would we lose if we move the (drm_debug&FOO) outside the
> > > > functions again?
>
> again?
We used to do that. Someone changed it a while back, unintentially
I believe.
>
> > > > I'm somewhat concerned about all the function call
> > > > overhead when debugs aren't even enabled.
>
> Perhaps better to have compilation elimination
> of the entire debug output instead.
That would require every bug reporter to recompile the kernel first.
So this is not a solution we would ever seriously consider.
Not sure if it would be possible to use the alternatives thing to
eliminate the function calls unless the user boots wih drm.debug!=0?
>
> I think you are discussing a different issue and
> this discussion should not block this patch as
> this patch has no impact other than code size
> reduction.
But what is the goal of the code size reduction? I assume the main
goal is to make better use of the instruction cache to make the
code faster. If there's a tradeoff between smaller and slightly
faster vs. larger and a singificantly faster I tend to think we
should go for the latter option.
--
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
2018-03-15 15:37 ` Ville Syrjälä
@ 2018-03-15 15:44 ` Joe Perches
-1 siblings, 0 replies; 27+ messages in thread
From: Joe Perches @ 2018-03-15 15:44 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Maarten Lankhorst, Gustavo Padovan, Sean Paul, David Airlie,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, intel-gfx,
linux-kernel, dri-devel
On Thu, 2018-03-15 at 17:37 +0200, Ville Syrjälä wrote:
> On Thu, Mar 15, 2018 at 08:17:53AM -0700, Joe Perches wrote:
> > On Thu, 2018-03-15 at 17:05 +0200, Ville Syrjälä wrote:
> > > On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote:
> > > > Op 15-03-18 om 14:30 schreef Ville Syrjälä:
> > > > > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> > > > > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > > > > > arguments that can be removed by creating separate functins.
> > > > > >
> > > > > > Create specific functions for these calls to reduce x86/64 defconfig
> > > > > > size by ~20k.
> > > > > >
> > > > > > Modify the existing macros to use the specific calls.
> > > > > >
> > > > > > new:
> > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > > > 1876562 44542 995 1922099 1d5433 (TOTALS)
> > > > > >
> > > > > > old:
> > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > > > 1897565 44542 995 1943102 1da63e (TOTALS)
> > > > > >
> > > > > > Miscellanea:
> > > > > >
> > > > > > o intel_display requires a change to use the specific calls.
> > > > >
> > > > > How much would we lose if we move the (drm_debug&FOO) outside the
> > > > > functions again?
> >
> > again?
>
> We used to do that. Someone changed it a while back, unintentially
> I believe.
>
> >
> > > > > I'm somewhat concerned about all the function call
> > > > > overhead when debugs aren't even enabled.
> >
> > Perhaps better to have compilation elimination
> > of the entire debug output instead.
>
> That would require every bug reporter to recompile the kernel first.
> So this is not a solution we would ever seriously consider.
>
> Not sure if it would be possible to use the alternatives thing to
> eliminate the function calls unless the user boots wih drm.debug!=0?
>
> >
> > I think you are discussing a different issue and
> > this discussion should not block this patch as
> > this patch has no impact other than code size
> > reduction.
>
> But what is the goal of the code size reduction?
Smaller code.
> I assume the main
> goal is to make better use of the instruction cache to make the
> code faster. If there's a tradeoff between smaller and slightly
> faster vs. larger and a singificantly faster I tend to think we
> should go for the latter option.
There's no trade-off in this patch for faster/larger.
This patch is simply smaller. Smaller is better.
Your faster/larger should be a different patch proposal.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
@ 2018-03-15 15:44 ` Joe Perches
0 siblings, 0 replies; 27+ messages in thread
From: Joe Perches @ 2018-03-15 15:44 UTC (permalink / raw)
To: Ville Syrjälä
Cc: David Airlie, intel-gfx, linux-kernel, dri-devel, Rodrigo Vivi
On Thu, 2018-03-15 at 17:37 +0200, Ville Syrjälä wrote:
> On Thu, Mar 15, 2018 at 08:17:53AM -0700, Joe Perches wrote:
> > On Thu, 2018-03-15 at 17:05 +0200, Ville Syrjälä wrote:
> > > On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote:
> > > > Op 15-03-18 om 14:30 schreef Ville Syrjälä:
> > > > > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> > > > > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > > > > > arguments that can be removed by creating separate functins.
> > > > > >
> > > > > > Create specific functions for these calls to reduce x86/64 defconfig
> > > > > > size by ~20k.
> > > > > >
> > > > > > Modify the existing macros to use the specific calls.
> > > > > >
> > > > > > new:
> > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > > > 1876562 44542 995 1922099 1d5433 (TOTALS)
> > > > > >
> > > > > > old:
> > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > > > 1897565 44542 995 1943102 1da63e (TOTALS)
> > > > > >
> > > > > > Miscellanea:
> > > > > >
> > > > > > o intel_display requires a change to use the specific calls.
> > > > >
> > > > > How much would we lose if we move the (drm_debug&FOO) outside the
> > > > > functions again?
> >
> > again?
>
> We used to do that. Someone changed it a while back, unintentially
> I believe.
>
> >
> > > > > I'm somewhat concerned about all the function call
> > > > > overhead when debugs aren't even enabled.
> >
> > Perhaps better to have compilation elimination
> > of the entire debug output instead.
>
> That would require every bug reporter to recompile the kernel first.
> So this is not a solution we would ever seriously consider.
>
> Not sure if it would be possible to use the alternatives thing to
> eliminate the function calls unless the user boots wih drm.debug!=0?
>
> >
> > I think you are discussing a different issue and
> > this discussion should not block this patch as
> > this patch has no impact other than code size
> > reduction.
>
> But what is the goal of the code size reduction?
Smaller code.
> I assume the main
> goal is to make better use of the instruction cache to make the
> code faster. If there's a tradeoff between smaller and slightly
> faster vs. larger and a singificantly faster I tend to think we
> should go for the latter option.
There's no trade-off in this patch for faster/larger.
This patch is simply smaller. Smaller is better.
Your faster/larger should be a different patch proposal.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
2018-03-15 15:44 ` Joe Perches
@ 2018-03-15 16:14 ` Ville Syrjälä
-1 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2018-03-15 16:14 UTC (permalink / raw)
To: Joe Perches
Cc: Maarten Lankhorst, Gustavo Padovan, Sean Paul, David Airlie,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, intel-gfx,
linux-kernel, dri-devel
On Thu, Mar 15, 2018 at 08:44:05AM -0700, Joe Perches wrote:
> On Thu, 2018-03-15 at 17:37 +0200, Ville Syrjälä wrote:
> > On Thu, Mar 15, 2018 at 08:17:53AM -0700, Joe Perches wrote:
> > > On Thu, 2018-03-15 at 17:05 +0200, Ville Syrjälä wrote:
> > > > On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote:
> > > > > Op 15-03-18 om 14:30 schreef Ville Syrjälä:
> > > > > > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> > > > > > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > > > > > > arguments that can be removed by creating separate functins.
> > > > > > >
> > > > > > > Create specific functions for these calls to reduce x86/64 defconfig
> > > > > > > size by ~20k.
> > > > > > >
> > > > > > > Modify the existing macros to use the specific calls.
> > > > > > >
> > > > > > > new:
> > > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > > > > 1876562 44542 995 1922099 1d5433 (TOTALS)
> > > > > > >
> > > > > > > old:
> > > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > > > > 1897565 44542 995 1943102 1da63e (TOTALS)
> > > > > > >
> > > > > > > Miscellanea:
> > > > > > >
> > > > > > > o intel_display requires a change to use the specific calls.
> > > > > >
> > > > > > How much would we lose if we move the (drm_debug&FOO) outside the
> > > > > > functions again?
> > >
> > > again?
> >
> > We used to do that. Someone changed it a while back, unintentially
> > I believe.
> >
> > >
> > > > > > I'm somewhat concerned about all the function call
> > > > > > overhead when debugs aren't even enabled.
> > >
> > > Perhaps better to have compilation elimination
> > > of the entire debug output instead.
> >
> > That would require every bug reporter to recompile the kernel first.
> > So this is not a solution we would ever seriously consider.
> >
> > Not sure if it would be possible to use the alternatives thing to
> > eliminate the function calls unless the user boots wih drm.debug!=0?
> >
> > >
> > > I think you are discussing a different issue and
> > > this discussion should not block this patch as
> > > this patch has no impact other than code size
> > > reduction.
> >
> > But what is the goal of the code size reduction?
>
> Smaller code.
>
> > I assume the main
> > goal is to make better use of the instruction cache to make the
> > code faster. If there's a tradeoff between smaller and slightly
> > faster vs. larger and a singificantly faster I tend to think we
> > should go for the latter option.
>
> There's no trade-off in this patch for faster/larger.
> This patch is simply smaller. Smaller is better.
This feels a bit like saying pink is better than red because it's
more pink.
That said, I'm not arguing against this patch as such. Making things
smaller "just because" usually doesn't cause problems. But I was
hoping that we might be after some more tangible gains here, and
thus pointed out that there may be a better way to achieve even
bigger gains.
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
@ 2018-03-15 16:14 ` Ville Syrjälä
0 siblings, 0 replies; 27+ messages in thread
From: Ville Syrjälä @ 2018-03-15 16:14 UTC (permalink / raw)
To: Joe Perches
Cc: David Airlie, intel-gfx, linux-kernel, dri-devel, Rodrigo Vivi
On Thu, Mar 15, 2018 at 08:44:05AM -0700, Joe Perches wrote:
> On Thu, 2018-03-15 at 17:37 +0200, Ville Syrjälä wrote:
> > On Thu, Mar 15, 2018 at 08:17:53AM -0700, Joe Perches wrote:
> > > On Thu, 2018-03-15 at 17:05 +0200, Ville Syrjälä wrote:
> > > > On Thu, Mar 15, 2018 at 03:04:52PM +0100, Maarten Lankhorst wrote:
> > > > > Op 15-03-18 om 14:30 schreef Ville Syrjälä:
> > > > > > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> > > > > > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > > > > > > arguments that can be removed by creating separate functins.
> > > > > > >
> > > > > > > Create specific functions for these calls to reduce x86/64 defconfig
> > > > > > > size by ~20k.
> > > > > > >
> > > > > > > Modify the existing macros to use the specific calls.
> > > > > > >
> > > > > > > new:
> > > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > > > > 1876562 44542 995 1922099 1d5433 (TOTALS)
> > > > > > >
> > > > > > > old:
> > > > > > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > > > > > 1897565 44542 995 1943102 1da63e (TOTALS)
> > > > > > >
> > > > > > > Miscellanea:
> > > > > > >
> > > > > > > o intel_display requires a change to use the specific calls.
> > > > > >
> > > > > > How much would we lose if we move the (drm_debug&FOO) outside the
> > > > > > functions again?
> > >
> > > again?
> >
> > We used to do that. Someone changed it a while back, unintentially
> > I believe.
> >
> > >
> > > > > > I'm somewhat concerned about all the function call
> > > > > > overhead when debugs aren't even enabled.
> > >
> > > Perhaps better to have compilation elimination
> > > of the entire debug output instead.
> >
> > That would require every bug reporter to recompile the kernel first.
> > So this is not a solution we would ever seriously consider.
> >
> > Not sure if it would be possible to use the alternatives thing to
> > eliminate the function calls unless the user boots wih drm.debug!=0?
> >
> > >
> > > I think you are discussing a different issue and
> > > this discussion should not block this patch as
> > > this patch has no impact other than code size
> > > reduction.
> >
> > But what is the goal of the code size reduction?
>
> Smaller code.
>
> > I assume the main
> > goal is to make better use of the instruction cache to make the
> > code faster. If there's a tradeoff between smaller and slightly
> > faster vs. larger and a singificantly faster I tend to think we
> > should go for the latter option.
>
> There's no trade-off in this patch for faster/larger.
> This patch is simply smaller. Smaller is better.
This feels a bit like saying pink is better than red because it's
more pink.
That said, I'm not arguing against this patch as such. Making things
smaller "just because" usually doesn't cause problems. But I was
hoping that we might be after some more tangible gains here, and
thus pointed out that there may be a better way to achieve even
bigger gains.
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
2018-03-15 16:14 ` Ville Syrjälä
@ 2018-03-15 16:29 ` Joe Perches
-1 siblings, 0 replies; 27+ messages in thread
From: Joe Perches @ 2018-03-15 16:29 UTC (permalink / raw)
To: Ville Syrjälä
Cc: Maarten Lankhorst, Gustavo Padovan, Sean Paul, David Airlie,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, intel-gfx,
linux-kernel, dri-devel
On Thu, 2018-03-15 at 18:14 +0200, Ville Syrjälä wrote:
> > There's no trade-off in this patch for faster/larger.
> > This patch is simply smaller. Smaller is better.
>
> This feels a bit like saying pink is better than red because it's
> more pink.
Silly. If you can't say smaller total object code that
performs the same task identically is better, I think
we can't discuss much of anything about code together.
Any printk related mechanism is not fast-path so any
icache dilution isn't an issue.
> That said, I'm not arguing against this patch as such. Making things
> smaller "just because" usually doesn't cause problems.
It seems more like you haven't read the patch.
> But I was
> hoping that we might be after some more tangible gains here, and
> thus pointed out that there may be a better way to achieve even
> bigger gains.
Sure, it's just any such a discussion should not affect
this patch being applied.
This patch reduces the argument count of the drm_printk
(now drm_dbg) call and so is faster to execute even if
the emit test is internal to the drm_dbg function.
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
@ 2018-03-15 16:29 ` Joe Perches
0 siblings, 0 replies; 27+ messages in thread
From: Joe Perches @ 2018-03-15 16:29 UTC (permalink / raw)
To: Ville Syrjälä
Cc: David Airlie, intel-gfx, linux-kernel, dri-devel, Rodrigo Vivi
On Thu, 2018-03-15 at 18:14 +0200, Ville Syrjälä wrote:
> > There's no trade-off in this patch for faster/larger.
> > This patch is simply smaller. Smaller is better.
>
> This feels a bit like saying pink is better than red because it's
> more pink.
Silly. If you can't say smaller total object code that
performs the same task identically is better, I think
we can't discuss much of anything about code together.
Any printk related mechanism is not fast-path so any
icache dilution isn't an issue.
> That said, I'm not arguing against this patch as such. Making things
> smaller "just because" usually doesn't cause problems.
It seems more like you haven't read the patch.
> But I was
> hoping that we might be after some more tangible gains here, and
> thus pointed out that there may be a better way to achieve even
> bigger gains.
Sure, it's just any such a discussion should not affect
this patch being applied.
This patch reduces the argument count of the drm_printk
(now drm_dbg) call and so is faster to execute even if
the emit test is internal to the drm_dbg function.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
2018-03-13 22:02 [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses Joe Perches
@ 2018-03-16 7:41 ` Daniel Vetter
2018-03-14 2:16 ` ✓ Fi.CI.IGT: " Patchwork
` (3 subsequent siblings)
4 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2018-03-16 7:41 UTC (permalink / raw)
To: Joe Perches
Cc: Gustavo Padovan, Maarten Lankhorst, Sean Paul, David Airlie,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, intel-gfx,
linux-kernel, dri-devel
On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> arguments that can be removed by creating separate functins.
>
> Create specific functions for these calls to reduce x86/64 defconfig
> size by ~20k.
>
> Modify the existing macros to use the specific calls.
>
> new:
> $ size -t drivers/gpu/drm/built-in.a | tail -1
> 1876562 44542 995 1922099 1d5433 (TOTALS)
>
> old:
> $ size -t drivers/gpu/drm/built-in.a | tail -1
> 1897565 44542 995 1943102 1da63e (TOTALS)
>
> Miscellanea:
>
> o intel_display requires a change to use the specific calls.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Impressed with the size of the bikeshed piled on top of this I decided to
cut this all short by merging it.
Thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/drm_print.c | 28 +++++++++++++++++++++-------
> drivers/gpu/drm/i915/intel_display.c | 15 ++++-----------
> include/drm/drm_print.h | 27 ++++++++++++++-------------
> 3 files changed, 39 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 781518fd88e3..79abf6d5b4db 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -89,23 +89,37 @@ void drm_dev_printk(const struct device *dev, const char *level,
> }
> EXPORT_SYMBOL(drm_dev_printk);
>
> -void drm_printk(const char *level, unsigned int category,
> - const char *format, ...)
> +void drm_dbg(unsigned int category, const char *format, ...)
> {
> struct va_format vaf;
> va_list args;
>
> - if (category != DRM_UT_NONE && !(drm_debug & category))
> + if (!(drm_debug & category))
> return;
>
> va_start(args, format);
> vaf.fmt = format;
> vaf.va = &args;
>
> - printk("%s" "[" DRM_NAME ":%ps]%s %pV",
> - level, __builtin_return_address(0),
> - strcmp(level, KERN_ERR) == 0 ? " *ERROR*" : "", &vaf);
> + printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
> + __builtin_return_address(0), &vaf);
> +
> + va_end(args);
> +}
> +EXPORT_SYMBOL(drm_dbg);
> +
> +void drm_err(const char *format, ...)
> +{
> + struct va_format vaf;
> + va_list args;
> +
> + va_start(args, format);
> + vaf.fmt = format;
> + vaf.va = &args;
> +
> + printk(KERN_ERR "[" DRM_NAME ":%ps] *ERROR* %pV",
> + __builtin_return_address(0), &vaf);
>
> va_end(args);
> }
> -EXPORT_SYMBOL(drm_printk);
> +EXPORT_SYMBOL(drm_err);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2933ad38094f..d8e522e3cd39 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11059,24 +11059,17 @@ intel_compare_link_m_n(const struct intel_link_m_n *m_n,
> static void __printf(3, 4)
> pipe_config_err(bool adjust, const char *name, const char *format, ...)
> {
> - char *level;
> - unsigned int category;
> struct va_format vaf;
> va_list args;
>
> - if (adjust) {
> - level = KERN_DEBUG;
> - category = DRM_UT_KMS;
> - } else {
> - level = KERN_ERR;
> - category = DRM_UT_NONE;
> - }
> -
> va_start(args, format);
> vaf.fmt = format;
> vaf.va = &args;
>
> - drm_printk(level, category, "mismatch in %s %pV", name, &vaf);
> + if (adjust)
> + drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, &vaf);
> + else
> + drm_err("mismatch in %s %pV", name, &vaf);
>
> va_end(args);
> }
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 2a4a42e59a47..3a40c5a3a5fa 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -200,9 +200,10 @@ __printf(6, 7)
> void drm_dev_printk(const struct device *dev, const char *level,
> unsigned int category, const char *function_name,
> const char *prefix, const char *format, ...);
> -__printf(3, 4)
> -void drm_printk(const char *level, unsigned int category,
> - const char *format, ...);
> +__printf(2, 3)
> +void drm_dbg(unsigned int category, const char *format, ...);
> +__printf(1, 2)
> +void drm_err(const char *format, ...);
>
> /* Macros to make printk easier */
>
> @@ -236,7 +237,7 @@ void drm_printk(const char *level, unsigned int category,
> drm_dev_printk(dev, KERN_ERR, DRM_UT_NONE, __func__, " *ERROR*",\
> fmt, ##__VA_ARGS__)
> #define DRM_ERROR(fmt, ...) \
> - drm_printk(KERN_ERR, DRM_UT_NONE, fmt, ##__VA_ARGS__)
> + drm_err(fmt, ##__VA_ARGS__)
>
> /**
> * Rate limited error output. Like DRM_ERROR() but won't flood the log.
> @@ -279,40 +280,40 @@ void drm_printk(const char *level, unsigned int category,
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_CORE, __func__, "", fmt, \
> ##args)
> #define DRM_DEBUG(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_CORE, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_DRIVER(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_DRIVER, __func__, "", \
> fmt, ##args)
> #define DRM_DEBUG_DRIVER(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_KMS(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_KMS, __func__, "", fmt, \
> ##args)
> -#define DRM_DEBUG_KMS(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_KMS, fmt, ##__VA_ARGS__)
> +#define DRM_DEBUG_KMS(fmt, ...) \
> + drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_PRIME(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_PRIME, __func__, "", \
> fmt, ##args)
> #define DRM_DEBUG_PRIME(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_PRIME, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_PRIME, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_ATOMIC(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_ATOMIC, __func__, "", \
> fmt, ##args)
> #define DRM_DEBUG_ATOMIC(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_VBL(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_VBL, __func__, "", fmt, \
> ##args)
> -#define DRM_DEBUG_VBL(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__)
> +#define DRM_DEBUG_VBL(fmt, ...) \
> + drm_dbg(DRM_UT_VBL, fmt, ##__VA_ARGS__)
>
> #define DRM_DEBUG_LEASE(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_LEASE, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_LEASE, fmt, ##__VA_ARGS__)
>
> #define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...) \
> ({ \
> --
> 2.15.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
@ 2018-03-16 7:41 ` Daniel Vetter
0 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2018-03-16 7:41 UTC (permalink / raw)
To: Joe Perches
Cc: David Airlie, intel-gfx, linux-kernel, dri-devel, Rodrigo Vivi
On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> arguments that can be removed by creating separate functins.
>
> Create specific functions for these calls to reduce x86/64 defconfig
> size by ~20k.
>
> Modify the existing macros to use the specific calls.
>
> new:
> $ size -t drivers/gpu/drm/built-in.a | tail -1
> 1876562 44542 995 1922099 1d5433 (TOTALS)
>
> old:
> $ size -t drivers/gpu/drm/built-in.a | tail -1
> 1897565 44542 995 1943102 1da63e (TOTALS)
>
> Miscellanea:
>
> o intel_display requires a change to use the specific calls.
>
> Signed-off-by: Joe Perches <joe@perches.com>
Impressed with the size of the bikeshed piled on top of this I decided to
cut this all short by merging it.
Thanks for the patch.
-Daniel
> ---
> drivers/gpu/drm/drm_print.c | 28 +++++++++++++++++++++-------
> drivers/gpu/drm/i915/intel_display.c | 15 ++++-----------
> include/drm/drm_print.h | 27 ++++++++++++++-------------
> 3 files changed, 39 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c
> index 781518fd88e3..79abf6d5b4db 100644
> --- a/drivers/gpu/drm/drm_print.c
> +++ b/drivers/gpu/drm/drm_print.c
> @@ -89,23 +89,37 @@ void drm_dev_printk(const struct device *dev, const char *level,
> }
> EXPORT_SYMBOL(drm_dev_printk);
>
> -void drm_printk(const char *level, unsigned int category,
> - const char *format, ...)
> +void drm_dbg(unsigned int category, const char *format, ...)
> {
> struct va_format vaf;
> va_list args;
>
> - if (category != DRM_UT_NONE && !(drm_debug & category))
> + if (!(drm_debug & category))
> return;
>
> va_start(args, format);
> vaf.fmt = format;
> vaf.va = &args;
>
> - printk("%s" "[" DRM_NAME ":%ps]%s %pV",
> - level, __builtin_return_address(0),
> - strcmp(level, KERN_ERR) == 0 ? " *ERROR*" : "", &vaf);
> + printk(KERN_DEBUG "[" DRM_NAME ":%ps] %pV",
> + __builtin_return_address(0), &vaf);
> +
> + va_end(args);
> +}
> +EXPORT_SYMBOL(drm_dbg);
> +
> +void drm_err(const char *format, ...)
> +{
> + struct va_format vaf;
> + va_list args;
> +
> + va_start(args, format);
> + vaf.fmt = format;
> + vaf.va = &args;
> +
> + printk(KERN_ERR "[" DRM_NAME ":%ps] *ERROR* %pV",
> + __builtin_return_address(0), &vaf);
>
> va_end(args);
> }
> -EXPORT_SYMBOL(drm_printk);
> +EXPORT_SYMBOL(drm_err);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 2933ad38094f..d8e522e3cd39 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11059,24 +11059,17 @@ intel_compare_link_m_n(const struct intel_link_m_n *m_n,
> static void __printf(3, 4)
> pipe_config_err(bool adjust, const char *name, const char *format, ...)
> {
> - char *level;
> - unsigned int category;
> struct va_format vaf;
> va_list args;
>
> - if (adjust) {
> - level = KERN_DEBUG;
> - category = DRM_UT_KMS;
> - } else {
> - level = KERN_ERR;
> - category = DRM_UT_NONE;
> - }
> -
> va_start(args, format);
> vaf.fmt = format;
> vaf.va = &args;
>
> - drm_printk(level, category, "mismatch in %s %pV", name, &vaf);
> + if (adjust)
> + drm_dbg(DRM_UT_KMS, "mismatch in %s %pV", name, &vaf);
> + else
> + drm_err("mismatch in %s %pV", name, &vaf);
>
> va_end(args);
> }
> diff --git a/include/drm/drm_print.h b/include/drm/drm_print.h
> index 2a4a42e59a47..3a40c5a3a5fa 100644
> --- a/include/drm/drm_print.h
> +++ b/include/drm/drm_print.h
> @@ -200,9 +200,10 @@ __printf(6, 7)
> void drm_dev_printk(const struct device *dev, const char *level,
> unsigned int category, const char *function_name,
> const char *prefix, const char *format, ...);
> -__printf(3, 4)
> -void drm_printk(const char *level, unsigned int category,
> - const char *format, ...);
> +__printf(2, 3)
> +void drm_dbg(unsigned int category, const char *format, ...);
> +__printf(1, 2)
> +void drm_err(const char *format, ...);
>
> /* Macros to make printk easier */
>
> @@ -236,7 +237,7 @@ void drm_printk(const char *level, unsigned int category,
> drm_dev_printk(dev, KERN_ERR, DRM_UT_NONE, __func__, " *ERROR*",\
> fmt, ##__VA_ARGS__)
> #define DRM_ERROR(fmt, ...) \
> - drm_printk(KERN_ERR, DRM_UT_NONE, fmt, ##__VA_ARGS__)
> + drm_err(fmt, ##__VA_ARGS__)
>
> /**
> * Rate limited error output. Like DRM_ERROR() but won't flood the log.
> @@ -279,40 +280,40 @@ void drm_printk(const char *level, unsigned int category,
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_CORE, __func__, "", fmt, \
> ##args)
> #define DRM_DEBUG(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_CORE, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_CORE, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_DRIVER(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_DRIVER, __func__, "", \
> fmt, ##args)
> #define DRM_DEBUG_DRIVER(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_DRIVER, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_KMS(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_KMS, __func__, "", fmt, \
> ##args)
> -#define DRM_DEBUG_KMS(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_KMS, fmt, ##__VA_ARGS__)
> +#define DRM_DEBUG_KMS(fmt, ...) \
> + drm_dbg(DRM_UT_KMS, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_PRIME(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_PRIME, __func__, "", \
> fmt, ##args)
> #define DRM_DEBUG_PRIME(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_PRIME, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_PRIME, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_ATOMIC(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_ATOMIC, __func__, "", \
> fmt, ##args)
> #define DRM_DEBUG_ATOMIC(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_ATOMIC, fmt, ##__VA_ARGS__)
>
> #define DRM_DEV_DEBUG_VBL(dev, fmt, args...) \
> drm_dev_printk(dev, KERN_DEBUG, DRM_UT_VBL, __func__, "", fmt, \
> ##args)
> -#define DRM_DEBUG_VBL(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_VBL, fmt, ##__VA_ARGS__)
> +#define DRM_DEBUG_VBL(fmt, ...) \
> + drm_dbg(DRM_UT_VBL, fmt, ##__VA_ARGS__)
>
> #define DRM_DEBUG_LEASE(fmt, ...) \
> - drm_printk(KERN_DEBUG, DRM_UT_LEASE, fmt, ##__VA_ARGS__)
> + drm_dbg(DRM_UT_LEASE, fmt, ##__VA_ARGS__)
>
> #define _DRM_DEV_DEFINE_DEBUG_RATELIMITED(dev, level, fmt, args...) \
> ({ \
> --
> 2.15.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
2018-03-16 7:41 ` Daniel Vetter
(?)
@ 2018-03-16 12:29 ` Joe Perches
2018-03-19 13:53 ` Daniel Vetter
-1 siblings, 1 reply; 27+ messages in thread
From: Joe Perches @ 2018-03-16 12:29 UTC (permalink / raw)
To: Daniel Vetter
Cc: Gustavo Padovan, Maarten Lankhorst, Sean Paul, David Airlie,
Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, intel-gfx,
linux-kernel, dri-devel
On Fri, 2018-03-16 at 08:41 +0100, Daniel Vetter wrote:
> On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > arguments that can be removed by creating separate functins.
> >
> > Create specific functions for these calls to reduce x86/64 defconfig
> > size by ~20k.
> >
> > Modify the existing macros to use the specific calls.
> >
> > new:
> > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > 1876562 44542 995 1922099 1d5433 (TOTALS)
> >
> > old:
> > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > 1897565 44542 995 1943102 1da63e (TOTALS)
> >
> > Miscellanea:
> >
> > o intel_display requires a change to use the specific calls.
> >
> > Signed-off-by: Joe Perches <joe@perches.com>
>
> Impressed with the size of the bikeshed piled on top of this I decided to
> cut this all short by merging it.
Thanks.
There was a similar patch for the DRM_DEV_ macros
awhile ago that also reduced object code.
https://lkml.org/lkml/2017/9/25/247
Never applied.
Want a remerge resend?
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
2018-03-16 12:29 ` Joe Perches
@ 2018-03-19 13:53 ` Daniel Vetter
0 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2018-03-19 13:53 UTC (permalink / raw)
To: Joe Perches
Cc: Daniel Vetter, Gustavo Padovan, Maarten Lankhorst, Sean Paul,
David Airlie, Jani Nikula, Joonas Lahtinen, Rodrigo Vivi,
intel-gfx, linux-kernel, dri-devel
On Fri, Mar 16, 2018 at 05:29:02AM -0700, Joe Perches wrote:
> On Fri, 2018-03-16 at 08:41 +0100, Daniel Vetter wrote:
> > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > > arguments that can be removed by creating separate functins.
> > >
> > > Create specific functions for these calls to reduce x86/64 defconfig
> > > size by ~20k.
> > >
> > > Modify the existing macros to use the specific calls.
> > >
> > > new:
> > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > 1876562 44542 995 1922099 1d5433 (TOTALS)
> > >
> > > old:
> > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > 1897565 44542 995 1943102 1da63e (TOTALS)
> > >
> > > Miscellanea:
> > >
> > > o intel_display requires a change to use the specific calls.
> > >
> > > Signed-off-by: Joe Perches <joe@perches.com>
> >
> > Impressed with the size of the bikeshed piled on top of this I decided to
> > cut this all short by merging it.
>
> Thanks.
>
> There was a similar patch for the DRM_DEV_ macros
> awhile ago that also reduced object code.
>
> https://lkml.org/lkml/2017/9/25/247
>
> Never applied.
>
> Want a remerge resend?
Yeah dropped out of my inbox, resending is easier. Please do so.
In case you wonder, I try to fairly intentionally drop stuff on the floor,
to force other people on dri-devel to not load everything onto me, making
me a bottleneck. But then occasionally a patch drops through all nets
because tracking mailing lists is impossible :-/
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 27+ messages in thread
* Re: [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses
@ 2018-03-19 13:53 ` Daniel Vetter
0 siblings, 0 replies; 27+ messages in thread
From: Daniel Vetter @ 2018-03-19 13:53 UTC (permalink / raw)
To: Joe Perches
Cc: David Airlie, intel-gfx, linux-kernel, dri-devel, Rodrigo Vivi
On Fri, Mar 16, 2018 at 05:29:02AM -0700, Joe Perches wrote:
> On Fri, 2018-03-16 at 08:41 +0100, Daniel Vetter wrote:
> > On Tue, Mar 13, 2018 at 03:02:15PM -0700, Joe Perches wrote:
> > > drm_printk is used for both DRM_ERROR and DRM_DEBUG with unnecessary
> > > arguments that can be removed by creating separate functins.
> > >
> > > Create specific functions for these calls to reduce x86/64 defconfig
> > > size by ~20k.
> > >
> > > Modify the existing macros to use the specific calls.
> > >
> > > new:
> > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > 1876562 44542 995 1922099 1d5433 (TOTALS)
> > >
> > > old:
> > > $ size -t drivers/gpu/drm/built-in.a | tail -1
> > > 1897565 44542 995 1943102 1da63e (TOTALS)
> > >
> > > Miscellanea:
> > >
> > > o intel_display requires a change to use the specific calls.
> > >
> > > Signed-off-by: Joe Perches <joe@perches.com>
> >
> > Impressed with the size of the bikeshed piled on top of this I decided to
> > cut this all short by merging it.
>
> Thanks.
>
> There was a similar patch for the DRM_DEV_ macros
> awhile ago that also reduced object code.
>
> https://lkml.org/lkml/2017/9/25/247
>
> Never applied.
>
> Want a remerge resend?
Yeah dropped out of my inbox, resending is easier. Please do so.
In case you wonder, I try to fairly intentionally drop stuff on the floor,
to force other people on dri-devel to not load everything onto me, making
me a bottleneck. But then occasionally a patch drops through all nets
because tracking mailing lists is impossible :-/
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 27+ messages in thread
end of thread, other threads:[~2018-03-19 13:54 UTC | newest]
Thread overview: 27+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-13 22:02 [PATCH] drm: Reduce object size of DRM_ERROR and DRM_DEBUG uses Joe Perches
2018-03-13 23:33 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-03-14 2:16 ` ✓ Fi.CI.IGT: " Patchwork
2018-03-15 13:22 ` [PATCH] " Maarten Lankhorst
2018-03-15 13:22 ` Maarten Lankhorst
2018-03-15 14:48 ` Joe Perches
2018-03-15 14:48 ` Joe Perches
2018-03-15 13:30 ` Ville Syrjälä
2018-03-15 13:30 ` Ville Syrjälä
2018-03-15 14:04 ` Maarten Lankhorst
2018-03-15 14:04 ` Maarten Lankhorst
2018-03-15 15:05 ` Ville Syrjälä
2018-03-15 15:05 ` Ville Syrjälä
2018-03-15 15:17 ` Joe Perches
2018-03-15 15:37 ` Ville Syrjälä
2018-03-15 15:37 ` Ville Syrjälä
2018-03-15 15:44 ` Joe Perches
2018-03-15 15:44 ` Joe Perches
2018-03-15 16:14 ` Ville Syrjälä
2018-03-15 16:14 ` Ville Syrjälä
2018-03-15 16:29 ` Joe Perches
2018-03-15 16:29 ` Joe Perches
2018-03-16 7:41 ` Daniel Vetter
2018-03-16 7:41 ` Daniel Vetter
2018-03-16 12:29 ` Joe Perches
2018-03-19 13:53 ` Daniel Vetter
2018-03-19 13:53 ` Daniel Vetter
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.