Skip to content

Commit

Permalink
drm_print: optimize drm_debug_enabled for jump-label
Browse files Browse the repository at this point in the history
When CONFIG_DRM_USE_DYNAMIC_DEBUG=y, the drm.debug API (a macro stack,
calling _+drm_*dbg() eventually) invokes a dyndbg Factory macro to
create a descriptor for each callsite, thus making them individually
>control-able.

In this case, the calls to _drm_*dbg are unreachable unless the
callsite is enabled.  So those calls can short-circuit their early
do-nothing returns.  Provide and use __drm_debug_enabled(), to do this
when config'd, or the _raw flags-check otherwize.

And since dyndbg is in use, lets also instrument the remaining users
of drm_debug_enabled, by wrapping the _raw in a macro with a:

  pr_debug("todo: is this frequent enough to optimize ?\n");

For CONFIG_DRM_USE_DYNAMIC_DEBUG=n, do no site instrumenting at all,
since JUMP_LABEL might be off, and we don't want to make work.

With drm, amdgpu, i915, nouveau loaded, heres remaining uses of
drm_debug_enabled(), which costs ~1.5kb data to control the
pr_debug("todo:..")s.

Some of those uses might be ok to use __drm_debug_enabled() by
inspection, others might warrant conversion to use dyndbg Factory
macros, and that would want callrate data to estimate the savings
possible.  TBH, any remaining savings are probably small; drm.debug
covers the vast bulk of the uses.  Maybe "vblank" is the exception.

:#> grep todo /proc/dynamic_debug/control | wc
     21     168    2357
:#> grep todo /proc/dynamic_debug/control
drivers/gpu/drm/drm_edid_load.c:178 [drm]edid_load =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/drm_vblank.c:410 [drm]drm_crtc_accurate_vblank_count =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/drm_vblank.c:787 [drm]drm_crtc_vblank_helper_get_vblank_timestamp_internal =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/drm_vblank.c:1491 [drm]drm_vblank_restore =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/drm_vblank.c:1433 [drm]drm_vblank_enable =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/drm_plane.c:2168 [drm]drm_mode_setplane =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/display/drm_dp_mst_topology.c:1359 [drm_display_helper]drm_dp_mst_wait_tx_reply =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/display/drm_dp_mst_topology.c:2864 [drm_display_helper]process_single_tx_qlock =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/display/drm_dp_mst_topology.c:2909 [drm_display_helper]drm_dp_queue_down_tx =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/display/drm_dp_mst_topology.c:1686 [drm_display_helper]drm_dp_mst_update_slots =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/display/intel_dp.c:1111 [i915]intel_dp_print_rates =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/display/intel_backlight.c:5434 [i915]cnp_enable_backlight =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/display/intel_backlight.c:5459 [i915]intel_backlight_device_register =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/display/intel_opregion.c:43 [i915]intel_opregion_notify_encoder =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/display/intel_opregion.c:53 [i915]asle_set_backlight =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/display/intel_bios.c:1088 [i915]intel_bios_is_dsi_present =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/display/intel_display_debugfs.c:6153 [i915]i915_drrs_ctl_set =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/intel_pcode.c:26 [i915]snb_pcode_read =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/i915/i915_getparam.c:785 [i915]i915_getparam_ioctl =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/amd/amdgpu/vcn_v2_5.c:282 [amdgpu]vcn_v2_5_process_interrupt =_ "todo: maybe avoid via dyndbg\n"
drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c:433 [amdgpu]vcn_v2_0_process_interrupt =_ "todo: maybe avoid via dyndbg\n"
:#>

Signed-off-by: Jim Cromie <jim.cromie@gmail.com>
Link: https://lore.kernel.org/r/20220912052852.1123868-8-jim.cromie@gmail.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Jim Cromie authored and Greg Kroah-Hartman committed Sep 24, 2022
1 parent 95a77b6 commit 6ce6fae
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 3 deletions.
4 changes: 2 additions & 2 deletions drivers/gpu/drm/drm_print.c
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ void __drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
struct va_format vaf;
va_list args;

if (!drm_debug_enabled(category))
if (!__drm_debug_enabled(category))
return;

va_start(args, format);
Expand All @@ -308,7 +308,7 @@ void ___drm_dbg(enum drm_debug_category category, const char *format, ...)
struct va_format vaf;
va_list args;

if (!drm_debug_enabled(category))
if (!__drm_debug_enabled(category))
return;

va_start(args, format);
Expand Down
21 changes: 20 additions & 1 deletion include/drm/drm_print.h
Original file line number Diff line number Diff line change
Expand Up @@ -321,11 +321,30 @@ enum drm_debug_category {
DRM_UT_DRMRES
};

static inline bool drm_debug_enabled(enum drm_debug_category category)
static inline bool drm_debug_enabled_raw(enum drm_debug_category category)
{
return unlikely(__drm_debug & BIT(category));
}

#define drm_debug_enabled_instrumented(category) \
({ \
pr_debug("todo: is this frequent enough to optimize ?\n"); \
drm_debug_enabled_raw(category); \
})

#if defined(CONFIG_DRM_USE_DYNAMIC_DEBUG)
/*
* the drm.debug API uses dyndbg, so each drm_*dbg macro/callsite gets
* a descriptor, and only enabled callsites are reachable. They use
* the private macro to avoid re-testing the enable-bit.
*/
#define __drm_debug_enabled(category) true
#define drm_debug_enabled(category) drm_debug_enabled_instrumented(category)
#else
#define __drm_debug_enabled(category) drm_debug_enabled_raw(category)
#define drm_debug_enabled(category) drm_debug_enabled_raw(category)
#endif

/*
* struct device based logging
*
Expand Down

0 comments on commit 6ce6fae

Please sign in to comment.