As patch 8/11 explains, I noticed that we where evaluating the arguments to drm_ut_debug_printk() even when drm.debug was 0, doing some work for no good reason. By pulling the test on drm_debug before calling drm_ut_debug_printk(), we skip those instructions that only need to be executed when logging is on.
The remaining patches are bonus clean-ups, with the main goal of removing DRM_LOG* macros that are not really used throughout the code base. After that, it's possible to simplify a bit drm_ut_debug_printk(). All pretty straightforward cleanups, but still worthwhile IMHO.
For driver-specific patches (why some of you are Cced in that series), I'd love if you could take the time to throw a Acked-by/Reviewed-by tag. Also, do you have any objection if the driver specific patch go through the DRM tree? should people judge that series worthwhile, of course.
That comment wasn't super-readable, so I tried to improve it:
- Put the comment before the values it's documenting - Add a mention to PRIME - Reword things a bit to be a lighter read - Add a note about the option to set the debug value at run-time
Signed-off-by: Damien Lespiau damien.lespiau@intel.com --- include/drm/drmP.h | 57 ++++++++++++++++++++++++++---------------------------- 1 file changed, 27 insertions(+), 30 deletions(-)
diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 3857450..97900b7 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -88,41 +88,38 @@ struct videomode; #include <drm/drm_hashtab.h> #include <drm/drm_mm.h>
-#define DRM_UT_CORE 0x01 -#define DRM_UT_DRIVER 0x02 -#define DRM_UT_KMS 0x04 -#define DRM_UT_PRIME 0x08 /* - * Three debug levels are defined. - * drm_core, drm_driver, drm_kms - * drm_core level can be used in the generic drm code. For example: - * drm_ioctl, drm_mm, drm_memory - * The macro definition of DRM_DEBUG is used. - * DRM_DEBUG(fmt, args...) - * The debug info by using the DRM_DEBUG can be obtained by adding - * the boot option of "drm.debug=1". + * 4 debug categories are defined: + * + * CORE: Used in the generic drm code: drm_ioctl.c, drm_mm.c, drm_memory.c, ... + * This is the category used by the DRM_DEBUG() macro. + * + * DRIVER: Used in the vendor specific part of the driver: i915, radeon, ... + * This is the category used by the DRM_DEBUG_DRIVER() macro. * - * drm_driver level can be used in the specific drm driver. It is used - * to add the debug info related with the drm driver. For example: - * i915_drv, i915_dma, i915_gem, radeon_drv, - * The macro definition of DRM_DEBUG_DRIVER can be used. - * DRM_DEBUG_DRIVER(fmt, args...) - * The debug info by using the DRM_DEBUG_DRIVER can be obtained by - * adding the boot option of "drm.debug=0x02" + * KMS: used in the modesetting code. + * This is the category used by the DRM_DEBUG_KMS() macro. * - * drm_kms level can be used in the KMS code related with specific drm driver. - * It is used to add the debug info related with KMS mode. For example: - * the connector/crtc , - * The macro definition of DRM_DEBUG_KMS can be used. - * DRM_DEBUG_KMS(fmt, args...) - * The debug info by using the DRM_DEBUG_KMS can be obtained by - * adding the boot option of "drm.debug=0x04" + * PRIME: used in the prime code. + * This is the category used by the DRM_DEBUG_PRIME() macro. * - * If we add the boot option of "drm.debug=0x06", we can get the debug info by - * using the DRM_DEBUG_KMS and DRM_DEBUG_DRIVER. - * If we add the boot option of "drm.debug=0x05", we can get the debug info by - * using the DRM_DEBUG_KMS and DRM_DEBUG. + * Enabling verbose debug messages is done through the drm.debug parameter, + * each category being enabled by a bit. + * + * drm.debug=0x1 will enable CORE messages + * drm.debug=0x2 will enable DRIVER messages + * drm.debug=0x3 will enable CORE and DRIVER messages + * ... + * drm.debug=0xf will enable all messages + * + * An interesting feature is that it's possible to enable verbose logging at + * run-time by echoing the debug value in its sysfs node: + * # echo 0xf > /sys/module/drm/parameters/debug */ +#define DRM_UT_CORE 0x01 +#define DRM_UT_DRIVER 0x02 +#define DRM_UT_KMS 0x04 +#define DRM_UT_PRIME 0x08
extern __printf(4, 5) void drm_ut_debug_printk(unsigned int request_level,
This macro was trying to use the non existing DRM_UT_MODE debug category and looks like it should be covered by DRM_LOG_KMS().
Signed-off-by: Damien Lespiau damien.lespiau@intel.com --- include/drm/drmP.h | 6 ------ 1 file changed, 6 deletions(-)
diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 97900b7..1455e58 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -238,11 +238,6 @@ int drm_err(const char *func, const char *format, ...); drm_ut_debug_printk(DRM_UT_KMS, NULL, \ NULL, fmt, ##args); \ } while (0) -#define DRM_LOG_MODE(fmt, args...) \ - do { \ - drm_ut_debug_printk(DRM_UT_MODE, NULL, \ - NULL, fmt, ##args); \ - } while (0) #define DRM_LOG_DRIVER(fmt, args...) \ do { \ drm_ut_debug_printk(DRM_UT_DRIVER, NULL, \ @@ -255,7 +250,6 @@ int drm_err(const char *func, const char *format, ...); #define DRM_DEBUG(fmt, arg...) do { } while (0) #define DRM_LOG(fmt, arg...) do { } while (0) #define DRM_LOG_KMS(fmt, args...) do { } while (0) -#define DRM_LOG_MODE(fmt, arg...) do { } while (0) #define DRM_LOG_DRIVER(fmt, arg...) do { } while (0)
#endif
There are only a few users of the DRM_LOG_KMS() macro. We can simplify the DRM code a bit by replacing them by DRM_DEBUG_KMS().
Cc: Inki Dae inki.dae@samsung.com Signed-off-by: Damien Lespiau damien.lespiau@intel.com --- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_plane.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c index e7c2f2d..5fa342e 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_fbdev.c +++ b/drivers/gpu/drm/exynos/exynos_drm_fbdev.c @@ -90,7 +90,7 @@ static int exynos_drm_fbdev_update(struct drm_fb_helper *helper, /* RGB formats use only one buffer */ buffer = exynos_drm_fb_buffer(fb, 0); if (!buffer) { - DRM_LOG_KMS("buffer is null.\n"); + DRM_DEBUG_KMS("buffer is null.\n"); return -EFAULT; }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c index fcb0652..2c765d5 100644 --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c @@ -87,7 +87,7 @@ int exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc, struct exynos_drm_gem_buf *buffer = exynos_drm_fb_buffer(fb, i);
if (!buffer) { - DRM_LOG_KMS("buffer is null\n"); + DRM_DEBUG_KMS("buffer is null\n"); return -EFAULT; }
There are only a few users of the DRM_LOG_KMS() macro. We can simplify the DRM code a bit by replacing them by DRM_DEBUG_KMS().
Cc: Patrik Jakobsson patrik.r.jakobsson@gmail.com Signed-off-by: Damien Lespiau damien.lespiau@intel.com --- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c index 07d3a9e..681efec 100644 --- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c +++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c @@ -406,18 +406,18 @@ static void psb_intel_sdvo_debug_write(struct psb_intel_sdvo *psb_intel_sdvo, u8 DRM_DEBUG_KMS("%s: W: %02X ", SDVO_NAME(psb_intel_sdvo), cmd); for (i = 0; i < args_len; i++) - DRM_LOG_KMS("%02X ", ((u8 *)args)[i]); + DRM_DEBUG_KMS("%02X ", ((u8 *)args)[i]); for (; i < 8; i++) - DRM_LOG_KMS(" "); + DRM_DEBUG_KMS(" "); for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) { if (cmd == sdvo_cmd_names[i].cmd) { - DRM_LOG_KMS("(%s)", sdvo_cmd_names[i].name); + DRM_DEBUG_KMS("(%s)", sdvo_cmd_names[i].name); break; } } if (i == ARRAY_SIZE(sdvo_cmd_names)) - DRM_LOG_KMS("(%02X)", cmd); - DRM_LOG_KMS("\n"); + DRM_DEBUG_KMS("(%02X)", cmd); + DRM_DEBUG_KMS("\n"); }
static const char *cmd_status_names[] = { @@ -512,9 +512,9 @@ static bool psb_intel_sdvo_read_response(struct psb_intel_sdvo *psb_intel_sdvo, }
if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP) - DRM_LOG_KMS("(%s)", cmd_status_names[status]); + DRM_DEBUG_KMS("(%s)", cmd_status_names[status]); else - DRM_LOG_KMS("(??? %d)", status); + DRM_DEBUG_KMS("(??? %d)", status);
if (status != SDVO_CMD_STATUS_SUCCESS) goto log_fail; @@ -525,13 +525,13 @@ static bool psb_intel_sdvo_read_response(struct psb_intel_sdvo *psb_intel_sdvo, SDVO_I2C_RETURN_0 + i, &((u8 *)response)[i])) goto log_fail; - DRM_LOG_KMS(" %02X", ((u8 *)response)[i]); + DRM_DEBUG_KMS(" %02X", ((u8 *)response)[i]); } - DRM_LOG_KMS("\n"); + DRM_DEBUG_KMS("\n"); return true;
log_fail: - DRM_LOG_KMS("... failed\n"); + DRM_DEBUG_KMS("... failed\n"); return false; }
On Mon, Mar 24, 2014 at 03:53:11PM +0000, Damien Lespiau wrote:
There are only a few users of the DRM_LOG_KMS() macro. We can simplify the DRM code a bit by replacing them by DRM_DEBUG_KMS().
Cc: Patrik Jakobsson patrik.r.jakobsson@gmail.com Signed-off-by: Damien Lespiau damien.lespiau@intel.com
Note that the point here of using LOG_KMS is to esssentially have continuations. Which means your patch here will make the output extremely noisy and hard to read.
But the current code is already racy which annoyed me sufficently a while ago in i915's copy to fix it all up properly in
commit 84fcb46977e57bafba40bde32067bacc1e510f9c Author: Daniel Vetter daniel.vetter@ffwll.ch Date: Wed Nov 27 16:03:01 2013 +0100
drm/i915/sdvo: Fix up debug output to not split lines
It leads to a big mess when stuff interleaves. Especially with the new patch I've submitted for the drm core to no longer artificially split up debug messages.
v2: The size parameter to snprintf includes the terminating 0, but the return value does not. Adjust the logic accordingly. Spotted by Mika.
Cc: Mika Kuoppala mika.kuoppala@linux.intel.com Reviewed-by: Mika Kuoppala mika.kuoppala@intel.com Signed-off-by: Daniel Vetter daniel.vetter@ffwll.ch
Just my 2c.
Cheers, Daniel
drivers/gpu/drm/gma500/psb_intel_sdvo.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c b/drivers/gpu/drm/gma500/psb_intel_sdvo.c index 07d3a9e..681efec 100644 --- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c +++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c @@ -406,18 +406,18 @@ static void psb_intel_sdvo_debug_write(struct psb_intel_sdvo *psb_intel_sdvo, u8 DRM_DEBUG_KMS("%s: W: %02X ", SDVO_NAME(psb_intel_sdvo), cmd); for (i = 0; i < args_len; i++)
DRM_LOG_KMS("%02X ", ((u8 *)args)[i]);
DRM_DEBUG_KMS("%02X ", ((u8 *)args)[i]);
DRM_LOG_KMS(" ");
DRM_DEBUG_KMS(" ");
DRM_LOG_KMS("(%s)", sdvo_cmd_names[i].name);
DRM_DEBUG_KMS("(%s)", sdvo_cmd_names[i].name); break;
DRM_LOG_KMS("(%02X)", cmd);- DRM_LOG_KMS("\n");
DRM_DEBUG_KMS("(%02X)", cmd);- DRM_DEBUG_KMS("\n");
}
static const char *cmd_status_names[] = { @@ -512,9 +512,9 @@ static bool psb_intel_sdvo_read_response(struct psb_intel_sdvo *psb_intel_sdvo, }
if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
DRM_LOG_KMS("(%s)", cmd_status_names[status]);
DRM_DEBUG_KMS("(%s)", cmd_status_names[status]);
DRM_LOG_KMS("(??? %d)", status);
DRM_DEBUG_KMS("(??? %d)", status);if (status != SDVO_CMD_STATUS_SUCCESS) goto log_fail;
@@ -525,13 +525,13 @@ static bool psb_intel_sdvo_read_response(struct psb_intel_sdvo *psb_intel_sdvo, SDVO_I2C_RETURN_0 + i, &((u8 *)response)[i])) goto log_fail;
DRM_LOG_KMS(" %02X", ((u8 *)response)[i]);
DRM_DEBUG_KMS(" %02X", ((u8 *)response)[i]);
- DRM_LOG_KMS("\n");
- DRM_DEBUG_KMS("\n"); return true;
log_fail:
- DRM_LOG_KMS("... failed\n");
- DRM_DEBUG_KMS("... failed\n"); return false;
}
-- 1.8.3.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Mar 24, 2014 at 9:39 PM, Daniel Vetter daniel@ffwll.ch wrote:
On Mon, Mar 24, 2014 at 03:53:11PM +0000, Damien Lespiau wrote:
There are only a few users of the DRM_LOG_KMS() macro. We can simplify the DRM code a bit by replacing them by DRM_DEBUG_KMS().
Cc: Patrik Jakobsson patrik.r.jakobsson@gmail.com Signed-off-by: Damien Lespiau damien.lespiau@intel.com
Note that the point here of using LOG_KMS is to esssentially have continuations. Which means your patch here will make the output extremely noisy and hard to read.
The noise is already there (tons of empty lines) so this patch will not make it any worse. It needs to be fixed like in i915. I'll put it on my todo-list.
Acked-by: Patrik Jakobsson patrik.r.jakobsson@gmail.com
But the current code is already racy which annoyed me sufficently a while ago in i915's copy to fix it all up properly in
commit 84fcb46977e57bafba40bde32067bacc1e510f9c Author: Daniel Vetter daniel.vetter@ffwll.ch Date: Wed Nov 27 16:03:01 2013 +0100
drm/i915/sdvo: Fix up debug output to not split lines It leads to a big mess when stuff interleaves. Especially with the new patch I've submitted for the drm core to no longer artificially split up debug messages. v2: The size parameter to snprintf includes the terminating 0, but the return value does not. Adjust the logic accordingly. Spotted by Mika. Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>Just my 2c.
Cheers, Daniel
drivers/gpu/drm/gma500/psb_intel_sdvo.c | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/gma500/psb_intel_sdvo.c
b/drivers/gpu/drm/gma500/psb_intel_sdvo.c
index 07d3a9e..681efec 100644 --- a/drivers/gpu/drm/gma500/psb_intel_sdvo.c +++ b/drivers/gpu/drm/gma500/psb_intel_sdvo.c @@ -406,18 +406,18 @@ static void psb_intel_sdvo_debug_write(struct
psb_intel_sdvo *psb_intel_sdvo, u8
DRM_DEBUG_KMS("%s: W: %02X ", SDVO_NAME(psb_intel_sdvo), cmd); for (i = 0; i < args_len; i++)
DRM_LOG_KMS("%02X ", ((u8 *)args)[i]);
DRM_DEBUG_KMS("%02X ", ((u8 *)args)[i]); for (; i < 8; i++)
DRM_LOG_KMS(" ");
DRM_DEBUG_KMS(" "); for (i = 0; i < ARRAY_SIZE(sdvo_cmd_names); i++) { if (cmd == sdvo_cmd_names[i].cmd) {
DRM_LOG_KMS("(%s)", sdvo_cmd_names[i].name);
DRM_DEBUG_KMS("(%s)", sdvo_cmd_names[i].name); break; } } if (i == ARRAY_SIZE(sdvo_cmd_names))
DRM_LOG_KMS("(%02X)", cmd);DRM_LOG_KMS("\n");
DRM_DEBUG_KMS("(%02X)", cmd);DRM_DEBUG_KMS("\n");}
static const char *cmd_status_names[] = { @@ -512,9 +512,9 @@ static bool psb_intel_sdvo_read_response(struct
psb_intel_sdvo *psb_intel_sdvo,
} if (status <= SDVO_CMD_STATUS_SCALING_NOT_SUPP)
DRM_LOG_KMS("(%s)", cmd_status_names[status]);
DRM_DEBUG_KMS("(%s)", cmd_status_names[status]); else
DRM_LOG_KMS("(??? %d)", status);
DRM_DEBUG_KMS("(??? %d)", status); if (status != SDVO_CMD_STATUS_SUCCESS) goto log_fail;@@ -525,13 +525,13 @@ static bool psb_intel_sdvo_read_response(struct
psb_intel_sdvo *psb_intel_sdvo,
SDVO_I2C_RETURN_0 + i, &((u8 *)response)[i])) goto log_fail;
DRM_LOG_KMS(" %02X", ((u8 *)response)[i]);
DRM_DEBUG_KMS(" %02X", ((u8 *)response)[i]); }
DRM_LOG_KMS("\n");
DRM_DEBUG_KMS("\n"); return true;log_fail:
DRM_LOG_KMS("... failed\n");
DRM_DEBUG_KMS("... failed\n"); return false;}
-- 1.8.3.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
There are only a few users of the DRM_LOG_KMS() macro. We can simplify the DRM code a bit by replacing them by DRM_DEBUG_KMS().
Signed-off-by: Damien Lespiau damien.lespiau@intel.com --- drivers/gpu/drm/i915/dvo_ch7xxx.c | 4 ++-- drivers/gpu/drm/i915/dvo_ivch.c | 30 +++++++++++++++--------------- drivers/gpu/drm/i915/dvo_ns2501.c | 10 +++++----- drivers/gpu/drm/i915/dvo_sil164.c | 10 +++++----- drivers/gpu/drm/i915/dvo_tfp410.c | 24 ++++++++++++------------ 5 files changed, 39 insertions(+), 39 deletions(-)
diff --git a/drivers/gpu/drm/i915/dvo_ch7xxx.c b/drivers/gpu/drm/i915/dvo_ch7xxx.c index af42e94..a0f5bdd 100644 --- a/drivers/gpu/drm/i915/dvo_ch7xxx.c +++ b/drivers/gpu/drm/i915/dvo_ch7xxx.c @@ -340,9 +340,9 @@ static void ch7xxx_dump_regs(struct intel_dvo_device *dvo) for (i = 0; i < CH7xxx_NUM_REGS; i++) { uint8_t val; if ((i % 8) == 0) - DRM_LOG_KMS("\n %02X: ", i); + DRM_DEBUG_KMS("\n %02X: ", i); ch7xxx_readb(dvo, i, &val); - DRM_LOG_KMS("%02X ", val); + DRM_DEBUG_KMS("%02X ", val); } }
diff --git a/drivers/gpu/drm/i915/dvo_ivch.c b/drivers/gpu/drm/i915/dvo_ivch.c index baaf65b..0f1865d 100644 --- a/drivers/gpu/drm/i915/dvo_ivch.c +++ b/drivers/gpu/drm/i915/dvo_ivch.c @@ -377,41 +377,41 @@ static void ivch_dump_regs(struct intel_dvo_device *dvo) uint16_t val;
ivch_read(dvo, VR00, &val); - DRM_LOG_KMS("VR00: 0x%04x\n", val); + DRM_DEBUG_KMS("VR00: 0x%04x\n", val); ivch_read(dvo, VR01, &val); - DRM_LOG_KMS("VR01: 0x%04x\n", val); + DRM_DEBUG_KMS("VR01: 0x%04x\n", val); ivch_read(dvo, VR30, &val); - DRM_LOG_KMS("VR30: 0x%04x\n", val); + DRM_DEBUG_KMS("VR30: 0x%04x\n", val); ivch_read(dvo, VR40, &val); - DRM_LOG_KMS("VR40: 0x%04x\n", val); + DRM_DEBUG_KMS("VR40: 0x%04x\n", val);
/* GPIO registers */ ivch_read(dvo, VR80, &val); - DRM_LOG_KMS("VR80: 0x%04x\n", val); + DRM_DEBUG_KMS("VR80: 0x%04x\n", val); ivch_read(dvo, VR81, &val); - DRM_LOG_KMS("VR81: 0x%04x\n", val); + DRM_DEBUG_KMS("VR81: 0x%04x\n", val); ivch_read(dvo, VR82, &val); - DRM_LOG_KMS("VR82: 0x%04x\n", val); + DRM_DEBUG_KMS("VR82: 0x%04x\n", val); ivch_read(dvo, VR83, &val); - DRM_LOG_KMS("VR83: 0x%04x\n", val); + DRM_DEBUG_KMS("VR83: 0x%04x\n", val); ivch_read(dvo, VR84, &val); - DRM_LOG_KMS("VR84: 0x%04x\n", val); + DRM_DEBUG_KMS("VR84: 0x%04x\n", val); ivch_read(dvo, VR85, &val); - DRM_LOG_KMS("VR85: 0x%04x\n", val); + DRM_DEBUG_KMS("VR85: 0x%04x\n", val); ivch_read(dvo, VR86, &val); - DRM_LOG_KMS("VR86: 0x%04x\n", val); + DRM_DEBUG_KMS("VR86: 0x%04x\n", val); ivch_read(dvo, VR87, &val); - DRM_LOG_KMS("VR87: 0x%04x\n", val); + DRM_DEBUG_KMS("VR87: 0x%04x\n", val); ivch_read(dvo, VR88, &val); - DRM_LOG_KMS("VR88: 0x%04x\n", val); + DRM_DEBUG_KMS("VR88: 0x%04x\n", val);
/* Scratch register 0 - AIM Panel type */ ivch_read(dvo, VR8E, &val); - DRM_LOG_KMS("VR8E: 0x%04x\n", val); + DRM_DEBUG_KMS("VR8E: 0x%04x\n", val);
/* Scratch register 1 - Status register */ ivch_read(dvo, VR8F, &val); - DRM_LOG_KMS("VR8F: 0x%04x\n", val); + DRM_DEBUG_KMS("VR8F: 0x%04x\n", val); }
static void ivch_destroy(struct intel_dvo_device *dvo) diff --git a/drivers/gpu/drm/i915/dvo_ns2501.c b/drivers/gpu/drm/i915/dvo_ns2501.c index 954acb2..8155ded 100644 --- a/drivers/gpu/drm/i915/dvo_ns2501.c +++ b/drivers/gpu/drm/i915/dvo_ns2501.c @@ -490,15 +490,15 @@ static void ns2501_dump_regs(struct intel_dvo_device *dvo) uint8_t val;
ns2501_readb(dvo, NS2501_FREQ_LO, &val); - DRM_LOG_KMS("NS2501_FREQ_LO: 0x%02x\n", val); + DRM_DEBUG_KMS("NS2501_FREQ_LO: 0x%02x\n", val); ns2501_readb(dvo, NS2501_FREQ_HI, &val); - DRM_LOG_KMS("NS2501_FREQ_HI: 0x%02x\n", val); + DRM_DEBUG_KMS("NS2501_FREQ_HI: 0x%02x\n", val); ns2501_readb(dvo, NS2501_REG8, &val); - DRM_LOG_KMS("NS2501_REG8: 0x%02x\n", val); + DRM_DEBUG_KMS("NS2501_REG8: 0x%02x\n", val); ns2501_readb(dvo, NS2501_REG9, &val); - DRM_LOG_KMS("NS2501_REG9: 0x%02x\n", val); + DRM_DEBUG_KMS("NS2501_REG9: 0x%02x\n", val); ns2501_readb(dvo, NS2501_REGC, &val); - DRM_LOG_KMS("NS2501_REGC: 0x%02x\n", val); + DRM_DEBUG_KMS("NS2501_REGC: 0x%02x\n", val); }
static void ns2501_destroy(struct intel_dvo_device *dvo) diff --git a/drivers/gpu/drm/i915/dvo_sil164.c b/drivers/gpu/drm/i915/dvo_sil164.c index 4debd32..7b3e9e9 100644 --- a/drivers/gpu/drm/i915/dvo_sil164.c +++ b/drivers/gpu/drm/i915/dvo_sil164.c @@ -246,15 +246,15 @@ static void sil164_dump_regs(struct intel_dvo_device *dvo) uint8_t val;
sil164_readb(dvo, SIL164_FREQ_LO, &val); - DRM_LOG_KMS("SIL164_FREQ_LO: 0x%02x\n", val); + DRM_DEBUG_KMS("SIL164_FREQ_LO: 0x%02x\n", val); sil164_readb(dvo, SIL164_FREQ_HI, &val); - DRM_LOG_KMS("SIL164_FREQ_HI: 0x%02x\n", val); + DRM_DEBUG_KMS("SIL164_FREQ_HI: 0x%02x\n", val); sil164_readb(dvo, SIL164_REG8, &val); - DRM_LOG_KMS("SIL164_REG8: 0x%02x\n", val); + DRM_DEBUG_KMS("SIL164_REG8: 0x%02x\n", val); sil164_readb(dvo, SIL164_REG9, &val); - DRM_LOG_KMS("SIL164_REG9: 0x%02x\n", val); + DRM_DEBUG_KMS("SIL164_REG9: 0x%02x\n", val); sil164_readb(dvo, SIL164_REGC, &val); - DRM_LOG_KMS("SIL164_REGC: 0x%02x\n", val); + DRM_DEBUG_KMS("SIL164_REGC: 0x%02x\n", val); }
static void sil164_destroy(struct intel_dvo_device *dvo) diff --git a/drivers/gpu/drm/i915/dvo_tfp410.c b/drivers/gpu/drm/i915/dvo_tfp410.c index e17f1b0..12ea4b1 100644 --- a/drivers/gpu/drm/i915/dvo_tfp410.c +++ b/drivers/gpu/drm/i915/dvo_tfp410.c @@ -267,33 +267,33 @@ static void tfp410_dump_regs(struct intel_dvo_device *dvo) uint8_t val, val2;
tfp410_readb(dvo, TFP410_REV, &val); - DRM_LOG_KMS("TFP410_REV: 0x%02X\n", val); + DRM_DEBUG_KMS("TFP410_REV: 0x%02X\n", val); tfp410_readb(dvo, TFP410_CTL_1, &val); - DRM_LOG_KMS("TFP410_CTL1: 0x%02X\n", val); + DRM_DEBUG_KMS("TFP410_CTL1: 0x%02X\n", val); tfp410_readb(dvo, TFP410_CTL_2, &val); - DRM_LOG_KMS("TFP410_CTL2: 0x%02X\n", val); + DRM_DEBUG_KMS("TFP410_CTL2: 0x%02X\n", val); tfp410_readb(dvo, TFP410_CTL_3, &val); - DRM_LOG_KMS("TFP410_CTL3: 0x%02X\n", val); + DRM_DEBUG_KMS("TFP410_CTL3: 0x%02X\n", val); tfp410_readb(dvo, TFP410_USERCFG, &val); - DRM_LOG_KMS("TFP410_USERCFG: 0x%02X\n", val); + DRM_DEBUG_KMS("TFP410_USERCFG: 0x%02X\n", val); tfp410_readb(dvo, TFP410_DE_DLY, &val); - DRM_LOG_KMS("TFP410_DE_DLY: 0x%02X\n", val); + DRM_DEBUG_KMS("TFP410_DE_DLY: 0x%02X\n", val); tfp410_readb(dvo, TFP410_DE_CTL, &val); - DRM_LOG_KMS("TFP410_DE_CTL: 0x%02X\n", val); + DRM_DEBUG_KMS("TFP410_DE_CTL: 0x%02X\n", val); tfp410_readb(dvo, TFP410_DE_TOP, &val); - DRM_LOG_KMS("TFP410_DE_TOP: 0x%02X\n", val); + DRM_DEBUG_KMS("TFP410_DE_TOP: 0x%02X\n", val); tfp410_readb(dvo, TFP410_DE_CNT_LO, &val); tfp410_readb(dvo, TFP410_DE_CNT_HI, &val2); - DRM_LOG_KMS("TFP410_DE_CNT: 0x%02X%02X\n", val2, val); + DRM_DEBUG_KMS("TFP410_DE_CNT: 0x%02X%02X\n", val2, val); tfp410_readb(dvo, TFP410_DE_LIN_LO, &val); tfp410_readb(dvo, TFP410_DE_LIN_HI, &val2); - DRM_LOG_KMS("TFP410_DE_LIN: 0x%02X%02X\n", val2, val); + DRM_DEBUG_KMS("TFP410_DE_LIN: 0x%02X%02X\n", val2, val); tfp410_readb(dvo, TFP410_H_RES_LO, &val); tfp410_readb(dvo, TFP410_H_RES_HI, &val2); - DRM_LOG_KMS("TFP410_H_RES: 0x%02X%02X\n", val2, val); + DRM_DEBUG_KMS("TFP410_H_RES: 0x%02X%02X\n", val2, val); tfp410_readb(dvo, TFP410_V_RES_LO, &val); tfp410_readb(dvo, TFP410_V_RES_HI, &val2); - DRM_LOG_KMS("TFP410_V_RES: 0x%02X%02X\n", val2, val); + DRM_DEBUG_KMS("TFP410_V_RES: 0x%02X%02X\n", val2, val); }
static void tfp410_destroy(struct intel_dvo_device *dvo)
There are only a few users of the DRM_LOG_KMS() macro. We can simplify the DRM code a bit by replacing them by DRM_DEBUG_KMS().
Cc: Philipp Zabel p.zabel@pengutronix.de Cc: Lucas Stach l.stach@pengutronix.de Signed-off-by: Damien Lespiau damien.lespiau@intel.com --- drivers/staging/imx-drm/ipuv3-plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/imx-drm/ipuv3-plane.c b/drivers/staging/imx-drm/ipuv3-plane.c index 34b642a..5128dc39 100644 --- a/drivers/staging/imx-drm/ipuv3-plane.c +++ b/drivers/staging/imx-drm/ipuv3-plane.c @@ -68,7 +68,7 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
cma_obj = drm_fb_cma_get_gem_obj(fb, 0); if (!cma_obj) { - DRM_LOG_KMS("entry is null.\n"); + DRM_DEBUG_KMS("entry is null.\n"); return -EFAULT; }
Am Montag, den 24.03.2014, 15:53 +0000 schrieb Damien Lespiau:
There are only a few users of the DRM_LOG_KMS() macro. We can simplify the DRM code a bit by replacing them by DRM_DEBUG_KMS().
Cc: Philipp Zabel p.zabel@pengutronix.de Cc: Lucas Stach l.stach@pengutronix.de Signed-off-by: Damien Lespiau damien.lespiau@intel.com
Acked-by: Philipp Zabel p.zabel@pengutronix.de
regards Philipp
drivers/staging/imx-drm/ipuv3-plane.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/staging/imx-drm/ipuv3-plane.c b/drivers/staging/imx-drm/ipuv3-plane.c index 34b642a..5128dc39 100644 --- a/drivers/staging/imx-drm/ipuv3-plane.c +++ b/drivers/staging/imx-drm/ipuv3-plane.c @@ -68,7 +68,7 @@ int ipu_plane_set_base(struct ipu_plane *ipu_plane, struct drm_framebuffer *fb,
cma_obj = drm_fb_cma_get_gem_obj(fb, 0); if (!cma_obj) {
DRM_LOG_KMS("entry is null.\n");
DRM_DEBUG_KMS("entry is null.\n");
Signed-off-by: Damien Lespiau damien.lespiau@intel.com --- include/drm/drmP.h | 19 ------------------- 1 file changed, 19 deletions(-)
diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 1455e58..3055b36 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -228,30 +228,11 @@ int drm_err(const char *func, const char *format, ...); drm_ut_debug_printk(DRM_UT_PRIME, DRM_NAME, \ __func__, fmt, ##args); \ } while (0) -#define DRM_LOG(fmt, args...) \ - do { \ - drm_ut_debug_printk(DRM_UT_CORE, NULL, \ - NULL, fmt, ##args); \ - } while (0) -#define DRM_LOG_KMS(fmt, args...) \ - do { \ - drm_ut_debug_printk(DRM_UT_KMS, NULL, \ - NULL, fmt, ##args); \ - } while (0) -#define DRM_LOG_DRIVER(fmt, args...) \ - do { \ - drm_ut_debug_printk(DRM_UT_DRIVER, NULL, \ - NULL, fmt, ##args); \ - } while (0) #else #define DRM_DEBUG_DRIVER(fmt, args...) do { } while (0) #define DRM_DEBUG_KMS(fmt, args...) do { } while (0) #define DRM_DEBUG_PRIME(fmt, args...) do { } while (0) #define DRM_DEBUG(fmt, arg...) do { } while (0) -#define DRM_LOG(fmt, arg...) do { } while (0) -#define DRM_LOG_KMS(fmt, args...) do { } while (0) -#define DRM_LOG_DRIVER(fmt, arg...) do { } while (0) - #endif
/*@}*/
In the logging code, we are currently checking is we need to output in drm_ut_debug_printk(). This is too late. The problem is that when we write something like:
DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n", connector->base.id, drm_get_connector_name(connector), connector->encoder->base.id, drm_get_encoder_name(connector->encoder));
We start by evaluating the arguments (so call drm_get_connector_name() and drm_get_connector_name()) before ending up in drm_ut_debug_printk() which will then does nothing.
This means we execute a lot of instructions (drm_get_connector_name(), in turn, calls snprintf() for example) to happily discard them in the normal case, drm.debug=0.
So, let's put the test on drm_debug earlier, in the macros themselves. Sprinkle an unlikely() as well for good measure.
Signed-off-by: Damien Lespiau damien.lespiau@intel.com --- drivers/gpu/drm/drm_stub.c | 26 ++++++++++++-------------- include/drm/drmP.h | 27 +++++++++++++++------------ 2 files changed, 27 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index dc2c609..81ed832 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -97,26 +97,24 @@ int drm_err(const char *func, const char *format, ...) } EXPORT_SYMBOL(drm_err);
-void drm_ut_debug_printk(unsigned int request_level, - const char *prefix, +void drm_ut_debug_printk(const char *prefix, const char *function_name, const char *format, ...) { struct va_format vaf; va_list args;
- if (drm_debug & request_level) { - va_start(args, format); - vaf.fmt = format; - vaf.va = &args; - - if (function_name) - printk(KERN_DEBUG "[%s:%s], %pV", prefix, - function_name, &vaf); - else - printk(KERN_DEBUG "%pV", &vaf); - va_end(args); - } + va_start(args, format); + vaf.fmt = format; + vaf.va = &args; + + if (function_name) + printk(KERN_DEBUG "[%s:%s], %pV", prefix, + function_name, &vaf); + else + printk(KERN_DEBUG "%pV", &vaf); + + va_end(args); } EXPORT_SYMBOL(drm_ut_debug_printk);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 3055b36..87b558a 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -121,9 +121,8 @@ struct videomode; #define DRM_UT_KMS 0x04 #define DRM_UT_PRIME 0x08
-extern __printf(4, 5) -void drm_ut_debug_printk(unsigned int request_level, - const char *prefix, +extern __printf(3, 4) +void drm_ut_debug_printk(const char *prefix, const char *function_name, const char *format, ...); extern __printf(2, 3) @@ -209,24 +208,28 @@ int drm_err(const char *func, const char *format, ...); #if DRM_DEBUG_CODE #define DRM_DEBUG(fmt, args...) \ do { \ - drm_ut_debug_printk(DRM_UT_CORE, DRM_NAME, \ - __func__, fmt, ##args); \ + if (unlikely(drm_debug & DRM_UT_CORE)) \ + drm_ut_debug_printk(DRM_NAME, __func__, \ + fmt, ##args); \ } while (0)
#define DRM_DEBUG_DRIVER(fmt, args...) \ do { \ - drm_ut_debug_printk(DRM_UT_DRIVER, DRM_NAME, \ - __func__, fmt, ##args); \ + if (unlikely(drm_debug & DRM_UT_DRIVER)) \ + drm_ut_debug_printk(DRM_NAME, __func__, \ + fmt, ##args); \ } while (0) -#define DRM_DEBUG_KMS(fmt, args...) \ +#define DRM_DEBUG_KMS(fmt, args...) \ do { \ - drm_ut_debug_printk(DRM_UT_KMS, DRM_NAME, \ - __func__, fmt, ##args); \ + if (unlikely(drm_debug & DRM_UT_KMS)) \ + drm_ut_debug_printk(DRM_NAME, __func__, \ + fmt, ##args); \ } while (0) #define DRM_DEBUG_PRIME(fmt, args...) \ do { \ - drm_ut_debug_printk(DRM_UT_PRIME, DRM_NAME, \ - __func__, fmt, ##args); \ + if (unlikely(drm_debug & DRM_UT_PRIME)) \ + drm_ut_debug_printk(DRM_NAME, __func__, \ + fmt, ##args); \ } while (0) #else #define DRM_DEBUG_DRIVER(fmt, args...) do { } while (0)
On Mon, 24 Mar 2014, Damien Lespiau damien.lespiau@intel.com wrote:
In the logging code, we are currently checking is we need to output in
s/is/if/
drm_ut_debug_printk(). This is too late. The problem is that when we write something like:
DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n", connector->base.id, drm_get_connector_name(connector), connector->encoder->base.id, drm_get_encoder_name(connector->encoder));We start by evaluating the arguments (so call drm_get_connector_name() and drm_get_connector_name()) before ending up in drm_ut_debug_printk() which will then does nothing.
s/does/do/
This means we execute a lot of instructions (drm_get_connector_name(), in turn, calls snprintf() for example) to happily discard them in the normal case, drm.debug=0.
So, let's put the test on drm_debug earlier, in the macros themselves. Sprinkle an unlikely() as well for good measure.
Bikeshed, is it likely that the unlikely matters all that much? https://lwn.net/Articles/70473/
I won't insist on removing them, it's more the casual nature of the "sprinkling unlikely for good measure" that bugs me.
BR, Jani.
Signed-off-by: Damien Lespiau damien.lespiau@intel.com
drivers/gpu/drm/drm_stub.c | 26 ++++++++++++-------------- include/drm/drmP.h | 27 +++++++++++++++------------ 2 files changed, 27 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index dc2c609..81ed832 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -97,26 +97,24 @@ int drm_err(const char *func, const char *format, ...) } EXPORT_SYMBOL(drm_err);
-void drm_ut_debug_printk(unsigned int request_level,
const char *prefix,+void drm_ut_debug_printk(const char *prefix, const char *function_name, const char *format, ...) { struct va_format vaf; va_list args;
- if (drm_debug & request_level) {
va_start(args, format);vaf.fmt = format;vaf.va = &args;if (function_name)printk(KERN_DEBUG "[%s:%s], %pV", prefix,function_name, &vaf);elseprintk(KERN_DEBUG "%pV", &vaf);va_end(args);- }
- va_start(args, format);
- vaf.fmt = format;
- vaf.va = &args;
- if (function_name)
printk(KERN_DEBUG "[%s:%s], %pV", prefix,function_name, &vaf);- else
printk(KERN_DEBUG "%pV", &vaf);- va_end(args);
} EXPORT_SYMBOL(drm_ut_debug_printk);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 3055b36..87b558a 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -121,9 +121,8 @@ struct videomode; #define DRM_UT_KMS 0x04 #define DRM_UT_PRIME 0x08
-extern __printf(4, 5) -void drm_ut_debug_printk(unsigned int request_level,
const char *prefix,+extern __printf(3, 4) +void drm_ut_debug_printk(const char *prefix, const char *function_name, const char *format, ...); extern __printf(2, 3) @@ -209,24 +208,28 @@ int drm_err(const char *func, const char *format, ...); #if DRM_DEBUG_CODE #define DRM_DEBUG(fmt, args...) \ do { \
drm_ut_debug_printk(DRM_UT_CORE, DRM_NAME, \__func__, fmt, ##args); \
if (unlikely(drm_debug & DRM_UT_CORE)) \drm_ut_debug_printk(DRM_NAME, __func__, \fmt, ##args); \#define DRM_DEBUG_DRIVER(fmt, args...) \ do { \
drm_ut_debug_printk(DRM_UT_DRIVER, DRM_NAME, \__func__, fmt, ##args); \
if (unlikely(drm_debug & DRM_UT_DRIVER)) \drm_ut_debug_printk(DRM_NAME, __func__, \fmt, ##args); \-#define DRM_DEBUG_KMS(fmt, args...) \ +#define DRM_DEBUG_KMS(fmt, args...) \ do { \
drm_ut_debug_printk(DRM_UT_KMS, DRM_NAME, \__func__, fmt, ##args); \
if (unlikely(drm_debug & DRM_UT_KMS)) \drm_ut_debug_printk(DRM_NAME, __func__, \fmt, ##args); \#define DRM_DEBUG_PRIME(fmt, args...) \ do { \
drm_ut_debug_printk(DRM_UT_PRIME, DRM_NAME, \__func__, fmt, ##args); \
if (unlikely(drm_debug & DRM_UT_PRIME)) \drm_ut_debug_printk(DRM_NAME, __func__, \fmt, ##args); \#else
#define DRM_DEBUG_DRIVER(fmt, args...) do { } while (0)
1.8.3.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Mar 25, 2014 at 11:56:24AM +0200, Jani Nikula wrote:
On Mon, 24 Mar 2014, Damien Lespiau damien.lespiau@intel.com wrote:
In the logging code, we are currently checking is we need to output in
s/is/if/
drm_ut_debug_printk(). This is too late. The problem is that when we write something like:
DRM_DEBUG_DRIVER("ELD on [CONNECTOR:%d:%s], [ENCODER:%d:%s]\n", connector->base.id, drm_get_connector_name(connector), connector->encoder->base.id, drm_get_encoder_name(connector->encoder));We start by evaluating the arguments (so call drm_get_connector_name() and drm_get_connector_name()) before ending up in drm_ut_debug_printk() which will then does nothing.
s/does/do/
This means we execute a lot of instructions (drm_get_connector_name(), in turn, calls snprintf() for example) to happily discard them in the normal case, drm.debug=0.
So, let's put the test on drm_debug earlier, in the macros themselves. Sprinkle an unlikely() as well for good measure.
Bikeshed, is it likely that the unlikely matters all that much? https://lwn.net/Articles/70473/
I won't insist on removing them, it's more the casual nature of the "sprinkling unlikely for good measure" that bugs me.
I've considered this and since most users don't set debug every we should be easily able to hit the 1:1M ratio seemingly required to make debug work.
And once you enable debugging the actual printk overhead will wash out anything you pay in branch mispredicts anyway. -Daniel
BR, Jani.
Signed-off-by: Damien Lespiau damien.lespiau@intel.com
drivers/gpu/drm/drm_stub.c | 26 ++++++++++++-------------- include/drm/drmP.h | 27 +++++++++++++++------------ 2 files changed, 27 insertions(+), 26 deletions(-)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index dc2c609..81ed832 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -97,26 +97,24 @@ int drm_err(const char *func, const char *format, ...) } EXPORT_SYMBOL(drm_err);
-void drm_ut_debug_printk(unsigned int request_level,
const char *prefix,+void drm_ut_debug_printk(const char *prefix, const char *function_name, const char *format, ...) { struct va_format vaf; va_list args;
- if (drm_debug & request_level) {
va_start(args, format);vaf.fmt = format;vaf.va = &args;if (function_name)printk(KERN_DEBUG "[%s:%s], %pV", prefix,function_name, &vaf);elseprintk(KERN_DEBUG "%pV", &vaf);va_end(args);- }
- va_start(args, format);
- vaf.fmt = format;
- vaf.va = &args;
- if (function_name)
printk(KERN_DEBUG "[%s:%s], %pV", prefix,function_name, &vaf);- else
printk(KERN_DEBUG "%pV", &vaf);- va_end(args);
} EXPORT_SYMBOL(drm_ut_debug_printk);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 3055b36..87b558a 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -121,9 +121,8 @@ struct videomode; #define DRM_UT_KMS 0x04 #define DRM_UT_PRIME 0x08
-extern __printf(4, 5) -void drm_ut_debug_printk(unsigned int request_level,
const char *prefix,+extern __printf(3, 4) +void drm_ut_debug_printk(const char *prefix, const char *function_name, const char *format, ...); extern __printf(2, 3) @@ -209,24 +208,28 @@ int drm_err(const char *func, const char *format, ...); #if DRM_DEBUG_CODE #define DRM_DEBUG(fmt, args...) \ do { \
drm_ut_debug_printk(DRM_UT_CORE, DRM_NAME, \__func__, fmt, ##args); \
if (unlikely(drm_debug & DRM_UT_CORE)) \drm_ut_debug_printk(DRM_NAME, __func__, \fmt, ##args); \#define DRM_DEBUG_DRIVER(fmt, args...) \ do { \
drm_ut_debug_printk(DRM_UT_DRIVER, DRM_NAME, \__func__, fmt, ##args); \
if (unlikely(drm_debug & DRM_UT_DRIVER)) \drm_ut_debug_printk(DRM_NAME, __func__, \fmt, ##args); \-#define DRM_DEBUG_KMS(fmt, args...) \ +#define DRM_DEBUG_KMS(fmt, args...) \ do { \
drm_ut_debug_printk(DRM_UT_KMS, DRM_NAME, \__func__, fmt, ##args); \
if (unlikely(drm_debug & DRM_UT_KMS)) \drm_ut_debug_printk(DRM_NAME, __func__, \fmt, ##args); \#define DRM_DEBUG_PRIME(fmt, args...) \ do { \
drm_ut_debug_printk(DRM_UT_PRIME, DRM_NAME, \__func__, fmt, ##args); \
if (unlikely(drm_debug & DRM_UT_PRIME)) \drm_ut_debug_printk(DRM_NAME, __func__, \fmt, ##args); \#else
#define DRM_DEBUG_DRIVER(fmt, args...) do { } while (0)
1.8.3.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
-- Jani Nikula, Intel Open Source Technology Center _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
The DRM_LOG* macros where the only sites where drm_ut_debug_printk was called with NULL arguments for prefix and function_name. Now that they are gone, we can remove that case.
Signed-off-by: Damien Lespiau damien.lespiau@intel.com --- drivers/gpu/drm/drm_stub.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index 81ed832..0b1a912 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -108,11 +108,7 @@ void drm_ut_debug_printk(const char *prefix, vaf.fmt = format; vaf.va = &args;
- if (function_name) - printk(KERN_DEBUG "[%s:%s], %pV", prefix, - function_name, &vaf); - else - printk(KERN_DEBUG "%pV", &vaf); + printk(KERN_DEBUG "[%s:%s], %pV", prefix, function_name, &vaf);
va_end(args); }
This is always DRM_NAME, so we can just make it part of the format string instead of asking prink to do it for us.
Signed-off-by: Damien Lespiau damien.lespiau@intel.com --- drivers/gpu/drm/drm_stub.c | 6 ++---- include/drm/drmP.h | 17 ++++++----------- 2 files changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index 0b1a912..9b14873 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -97,9 +97,7 @@ int drm_err(const char *func, const char *format, ...) } EXPORT_SYMBOL(drm_err);
-void drm_ut_debug_printk(const char *prefix, - const char *function_name, - const char *format, ...) +void drm_ut_debug_printk(const char *function_name, const char *format, ...) { struct va_format vaf; va_list args; @@ -108,7 +106,7 @@ void drm_ut_debug_printk(const char *prefix, vaf.fmt = format; vaf.va = &args;
- printk(KERN_DEBUG "[%s:%s], %pV", prefix, function_name, &vaf); + printk(KERN_DEBUG "[" DRM_NAME ":%s], %pV", function_name, &vaf);
va_end(args); } diff --git a/include/drm/drmP.h b/include/drm/drmP.h index 87b558a..2f49510 100644 --- a/include/drm/drmP.h +++ b/include/drm/drmP.h @@ -121,9 +121,8 @@ struct videomode; #define DRM_UT_KMS 0x04 #define DRM_UT_PRIME 0x08
-extern __printf(3, 4) -void drm_ut_debug_printk(const char *prefix, - const char *function_name, +extern __printf(2, 3) +void drm_ut_debug_printk(const char *function_name, const char *format, ...); extern __printf(2, 3) int drm_err(const char *func, const char *format, ...); @@ -209,27 +208,23 @@ int drm_err(const char *func, const char *format, ...); #define DRM_DEBUG(fmt, args...) \ do { \ if (unlikely(drm_debug & DRM_UT_CORE)) \ - drm_ut_debug_printk(DRM_NAME, __func__, \ - fmt, ##args); \ + drm_ut_debug_printk(__func__, fmt, ##args); \ } while (0)
#define DRM_DEBUG_DRIVER(fmt, args...) \ do { \ if (unlikely(drm_debug & DRM_UT_DRIVER)) \ - drm_ut_debug_printk(DRM_NAME, __func__, \ - fmt, ##args); \ + drm_ut_debug_printk(__func__, fmt, ##args); \ } while (0) #define DRM_DEBUG_KMS(fmt, args...) \ do { \ if (unlikely(drm_debug & DRM_UT_KMS)) \ - drm_ut_debug_printk(DRM_NAME, __func__, \ - fmt, ##args); \ + drm_ut_debug_printk(__func__, fmt, ##args); \ } while (0) #define DRM_DEBUG_PRIME(fmt, args...) \ do { \ if (unlikely(drm_debug & DRM_UT_PRIME)) \ - drm_ut_debug_printk(DRM_NAME, __func__, \ - fmt, ##args); \ + drm_ut_debug_printk(__func__, fmt, ##args); \ } while (0) #else #define DRM_DEBUG_DRIVER(fmt, args...) do { } while (0)
Right now a debug message looks like:
[drm:drm_ioctl], pid=860, dev=0xe200, auth=1, DRM_IOCTL_MODE_GETCRTC
That first comma looks weird as we already have ']' as a separator. Remove it.
If anyone sees this commit message and also thinks that auth=1 isn't the most useful info to have here, let's just say I'd happily review a patch removing it. If I don't get annoyed enough to submit a patch, that is.
Signed-off-by: Damien Lespiau damien.lespiau@intel.com --- drivers/gpu/drm/drm_stub.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/drm_stub.c b/drivers/gpu/drm/drm_stub.c index 9b14873..ed8a995 100644 --- a/drivers/gpu/drm/drm_stub.c +++ b/drivers/gpu/drm/drm_stub.c @@ -106,7 +106,7 @@ void drm_ut_debug_printk(const char *function_name, const char *format, ...) vaf.fmt = format; vaf.va = &args;
- printk(KERN_DEBUG "[" DRM_NAME ":%s], %pV", function_name, &vaf); + printk(KERN_DEBUG "[" DRM_NAME ":%s] %pV", function_name, &vaf);
va_end(args); }
On Mon, Mar 24, 2014 at 03:53:07PM +0000, Damien Lespiau wrote:
As patch 8/11 explains, I noticed that we where evaluating the arguments to drm_ut_debug_printk() even when drm.debug was 0, doing some work for no good reason. By pulling the test on drm_debug before calling drm_ut_debug_printk(), we skip those instructions that only need to be executed when logging is on.
The remaining patches are bonus clean-ups, with the main goal of removing DRM_LOG* macros that are not really used throughout the code base. After that, it's possible to simplify a bit drm_ut_debug_printk(). All pretty straightforward cleanups, but still worthwhile IMHO.
For driver-specific patches (why some of you are Cced in that series), I'd love if you could take the time to throw a Acked-by/Reviewed-by tag. Also, do you have any objection if the driver specific patch go through the DRM tree? should people judge that series worthwhile, of course.
Except for the gma500 patch this series is
Reviewed-by: Daniel Vetter daniel.vetter@ffwll.ch
For the gma500 I've replied with my opinion but will defer to Patrik. -Daniel
-- Damien
Damien Lespiau (11): drm: Refresh the explanation of debug categories drm: Remove the unused (and unusable) DRM_LOG_MODE() drm/exynos: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS() drm/gma500: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS() drm/i915: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS() staging: imx-drm: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS() drm: Remove the now unused DRM_LOG* macros drm: Pull the test on drm_debug in the logging macros drm: drm_ut_debug_printk() isn't called with NULL anywmore drm: Remove the prefix argument of drm_ut_debug_printk() drm: Remove the ',' after the function name in debug logs
drivers/gpu/drm/drm_stub.c | 24 +++---- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_plane.c | 2 +- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 20 +++--- drivers/gpu/drm/i915/dvo_ch7xxx.c | 4 +- drivers/gpu/drm/i915/dvo_ivch.c | 30 ++++----- drivers/gpu/drm/i915/dvo_ns2501.c | 10 +-- drivers/gpu/drm/i915/dvo_sil164.c | 10 +-- drivers/gpu/drm/i915/dvo_tfp410.c | 24 +++---- drivers/staging/imx-drm/ipuv3-plane.c | 2 +- include/drm/drmP.h | 106 +++++++++++------------------- 11 files changed, 98 insertions(+), 136 deletions(-)
-- 1.8.3.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Mon, Mar 24, 2014 at 4:53 PM, Damien Lespiau damien.lespiau@intel.comwrote:
As patch 8/11 explains, I noticed that we where evaluating the arguments to drm_ut_debug_printk() even when drm.debug was 0, doing some work for no good reason. By pulling the test on drm_debug before calling drm_ut_debug_printk(), we skip those instructions that only need to be executed when logging is on.
The remaining patches are bonus clean-ups, with the main goal of removing DRM_LOG* macros that are not really used throughout the code base. After that, it's possible to simplify a bit drm_ut_debug_printk(). All pretty straightforward cleanups, but still worthwhile IMHO.
For driver-specific patches (why some of you are Cced in that series), I'd love if you could take the time to throw a Acked-by/Reviewed-by tag. Also, do you have any objection if the driver specific patch go through the DRM tree? should people judge that series worthwhile, of course.
I'm ok with this going through the DRM tree.
Reviewed-by: Patrik Jakobsson patrik.r.jakobsson@gmail.com
-- Damien
Damien Lespiau (11): drm: Refresh the explanation of debug categories drm: Remove the unused (and unusable) DRM_LOG_MODE() drm/exynos: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS() drm/gma500: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS() drm/i915: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS() staging: imx-drm: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS() drm: Remove the now unused DRM_LOG* macros drm: Pull the test on drm_debug in the logging macros drm: drm_ut_debug_printk() isn't called with NULL anywmore drm: Remove the prefix argument of drm_ut_debug_printk() drm: Remove the ',' after the function name in debug logs
drivers/gpu/drm/drm_stub.c | 24 +++---- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_plane.c | 2 +- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 20 +++--- drivers/gpu/drm/i915/dvo_ch7xxx.c | 4 +- drivers/gpu/drm/i915/dvo_ivch.c | 30 ++++----- drivers/gpu/drm/i915/dvo_ns2501.c | 10 +-- drivers/gpu/drm/i915/dvo_sil164.c | 10 +-- drivers/gpu/drm/i915/dvo_tfp410.c | 24 +++---- drivers/staging/imx-drm/ipuv3-plane.c | 2 +- include/drm/drmP.h | 106 +++++++++++------------------- 11 files changed, 98 insertions(+), 136 deletions(-)
-- 1.8.3.1
2014-03-25 0:53 GMT+09:00 Damien Lespiau damien.lespiau@intel.com:
As patch 8/11 explains, I noticed that we where evaluating the arguments to drm_ut_debug_printk() even when drm.debug was 0, doing some work for no good reason. By pulling the test on drm_debug before calling drm_ut_debug_printk(), we skip those instructions that only need to be executed when logging is on.
The remaining patches are bonus clean-ups, with the main goal of removing DRM_LOG* macros that are not really used throughout the code base. After that, it's possible to simplify a bit drm_ut_debug_printk(). All pretty straightforward cleanups, but still worthwhile IMHO.
For driver-specific patches (why some of you are Cced in that series), I'd love if you could take the time to throw a Acked-by/Reviewed-by tag. Also, do you have any objection if the driver specific patch go through the DRM tree? should people judge that series worthwhile, of course.
For exynos parts,
Reviewed-by: Inki Dae inki.dae@samsung.com
Thanks, Inki Dae
-- Damien
Damien Lespiau (11): drm: Refresh the explanation of debug categories drm: Remove the unused (and unusable) DRM_LOG_MODE() drm/exynos: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS() drm/gma500: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS() drm/i915: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS() staging: imx-drm: Replace DRM_LOG_KMS() by DRM_DEBUG_KMS() drm: Remove the now unused DRM_LOG* macros drm: Pull the test on drm_debug in the logging macros drm: drm_ut_debug_printk() isn't called with NULL anywmore drm: Remove the prefix argument of drm_ut_debug_printk() drm: Remove the ',' after the function name in debug logs
drivers/gpu/drm/drm_stub.c | 24 +++---- drivers/gpu/drm/exynos/exynos_drm_fbdev.c | 2 +- drivers/gpu/drm/exynos/exynos_drm_plane.c | 2 +- drivers/gpu/drm/gma500/psb_intel_sdvo.c | 20 +++--- drivers/gpu/drm/i915/dvo_ch7xxx.c | 4 +- drivers/gpu/drm/i915/dvo_ivch.c | 30 ++++----- drivers/gpu/drm/i915/dvo_ns2501.c | 10 +-- drivers/gpu/drm/i915/dvo_sil164.c | 10 +-- drivers/gpu/drm/i915/dvo_tfp410.c | 24 +++---- drivers/staging/imx-drm/ipuv3-plane.c | 2 +- include/drm/drmP.h | 106 +++++++++++------------------- 11 files changed, 98 insertions(+), 136 deletions(-)
-- 1.8.3.1
dri-devel mailing list dri-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel
On Tue, Mar 25, 2014 at 4:35 PM, Inki Dae inki.dae@samsung.com wrote:
2014-03-25 0:53 GMT+09:00 Damien Lespiau damien.lespiau@intel.com:
As patch 8/11 explains, I noticed that we where evaluating the arguments to drm_ut_debug_printk() even when drm.debug was 0, doing some work for no good reason. By pulling the test on drm_debug before calling drm_ut_debug_printk(), we skip those instructions that only need to be executed when logging is on.
The remaining patches are bonus clean-ups, with the main goal of removing DRM_LOG* macros that are not really used throughout the code base. After that, it's possible to simplify a bit drm_ut_debug_printk(). All pretty straightforward cleanups, but still worthwhile IMHO.
For driver-specific patches (why some of you are Cced in that series), I'd love if you could take the time to throw a Acked-by/Reviewed-by tag. Also, do you have any objection if the driver specific patch go through the DRM tree? should people judge that series worthwhile, of course.
All pulled into -next, thanks,
Dave.
dri-devel@lists.freedesktop.org
-
Damien Lespiau -
Daniel Vetter -
Dave Airlie -
Inki Dae -
Jani Nikula -
Patrik Jakobsson -
Philipp Zabel