[Sound-open-firmware] Introduce a module parameter to configure SOF driver debug output?

[Lin, Mengdong]

> -----Original Message-----
> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart at linux.intel.com]
> Sent: Wednesday, May 2, 2018 9:20 PM
> 
> >>> The kernel debug message from SOF driver can help us to debug issues
> >>> and
> >> CI to provide hints for failed test cases. But it will be too much to
> >> output all of debug message by default.
> >>>
> >>> So maybe we can try to manage SOF debug messages as drm does?
> >>>
> >>> DRM is kernel gfx subsystem and Intel i915 driver is based on it.
> >>> drm
> >> defines a module parameter 'debug' for user to specify a debug mask
> >> when booting the kernel. The debug mask is a bit mask of different
> >> message categories. By setting a suitable mask, user or CI can make
> >> DRM driver output message of specified categories. Usually we use
> >> drm.debug=0x0e to get most important debug messages.
> >>>
> >>> Maybe SOF driver can use this method to configure its messages? If
> >>> this is
> >> doable, we can do some RFC patch.
> >>>
> >>> Here is how drm defines its debug mode in drivers/gpu/drm/drm_drv.c:
> >>>
> >>> MODULE_PARM_DESC(debug, "Enable debug output, where each bit
> >> enables a debug category.\n"
> >>> "\t\tBit 0 (0x01) will enable CORE messages (drm core code)\n"
> >>> "\t\tBit 1 (0x02) will enable DRIVER messages (drm controller code)\n"
> >>> "\t\tBit 2 (0x04) will enable KMS messages (modesetting code)\n"
> >>> "\t\tBit 3 (0x08) will enable PRIME messages (prime code)\n"
> >>> "\t\tBit 4 (0x10) will enable ATOMIC messages (atomic code)\n"
> >>> "\t\tBit 5 (0x20) will enable VBL messages (vblank code)\n"
> >>> "\t\tBit 7 (0x80) will enable LEASE messages (leasing code)");
> >>
> >> not a bad idea for developers. when folks debug topology or IPC
> >> stuff, they don't necessarily care about PCM or page tables, so it's a good
> way to filter.
> >> we could have one bit per module (core, pcm, ipc, topology, acpi, pci, etc).
> >>
> >> I don't see how this improves CI work though?
> >
> > CI can save the debug message for developers to debug failed test cases.
> And 0day can also detect some pattern in the debug message to decide if
> there is an audio test case failure.
> 
> ok, but you will have to find a debug level that is both verbose enough to
> point to the problem without overwhelming the reader...
> >
> > However, 0day does not allow us to apply patch enable DEBUG by default
> in Makefile. That's why I want to introduce a kernel module parameter to
> control this at runtime.
> >
> > I'll check how ALSA uses snd_debug() or snd_printk() as Liam suggested.
> Maybe we can build sof_debug on top of them.
> 
> if we want this to be used in other OSes we might as well create a log
> mechanism which can be easily ported without dependencies on GPL or alsa.

Thanks for the tips. I overlooked this.

ALSA and ASoC use dev_warn() and dev_err() to output warning and errors. ALSA also defines snd_printk() based on printk(). But ALSA and ASoC core don't use snd_printk() themselves, they just output warning and errors. 

DRM implements its drm_dev_printk() based on dev_printk().

So for SOF log mechanism, it seems we don't need to use snd_printk(). We may introduce an adaption layer. On Linux, it can be based on dev_printk(const char *level, const struct device *dev, const char *fmt, ...). For other non-GPL OSes, we can change the implementation. 

Thanks
Mengdong

> 
> >
> > Thanks
> > Mengdong
> >
> >>
> >>
> >>>
> >>> module_param_named(debug, drm_debug, int, 0600);
> >>>
> >>> void drm_dev_printk(const struct device *dev, const char *level,
> >>> 		    unsigned int category, const char *function_name,
> >>> 		    const char *prefix, const char *format, ...) {
> >>> 	struct va_format vaf;
> >>> 	va_list args;
> >>>
> >>> 	if (category != DRM_UT_NONE && !(drm_debug & category))
> >>> 		return;
> >>>
> >>> 	va_start(args, format);
> >>> 	vaf.fmt = format;
> >>> 	vaf.va = &args;
> >>>
> >>> 	if (dev)
> >>> 		dev_printk(level, dev, DRM_PRINTK_FMT, function_name,
> >> prefix,
> >>> 			   &vaf);
> >>> 	...
> >>> }
> >>>
> >>> Then base on this, it defines macros for outputting of different
> categories:
> >>>
> >>> #define DRM_DEV_DEBUG(dev, fmt, args...)
> 	\
> >>> 	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__)
> >>>
> >>>
> >>> #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__)
> >>>
> >>>
> >>> #define DRM_DEV_DEBUG_KMS(dev, fmt, args...)
> >> 	\
> >>> 	drm_dev_printk(dev, KERN_DEBUG, DRM_UT_KMS, __func__, "", fmt,
> >> 	\
> >>> 		       ##args)
> >>>
> >>>
> >>> Thanks
> >>> Mengdong
> >>>
> >>>
> >>> _______________________________________________
> >>> Sound-open-firmware mailing list
> >>> Sound-open-firmware at alsa-project.org
> >>> http://mailman.alsa-project.org/mailman/listinfo/sound-open-firmware
> >>>
> >