Re: [PATCH v3 6/7] venus: Make debug infrastructure more flexible

Joe Perches
2020-06-11T06:42:51Z

On Thu, 2020-06-11 at 08:26 +0200, Greg Kroah-Hartman wrote:
> On Wed, Jun 10, 2020 at 01:23:56PM -0700, Joe Perches wrote:
> > On Wed, 2020-06-10 at 12:49 -0700, Joe Perches wrote:
> > > On Wed, 2020-06-10 at 15:37 +0200, Greg Kroah-Hartman wrote:
> > > > Please work with the infrastructure we have, we have spent a lot of time
> > > > and effort to make it uniform to make it easier for users and
> > > > developers.
> > >
> > > Not quite.
> > >
> > > This lack of debug grouping by type has been a
> > > _long_ standing issue with drivers.
> > >
> > > > Don't regress and try to make driver-specific ways of doing
> > > > things, that way lies madness...
> > >
> > > It's not driver specific, it allows driver developers to
> > > better isolate various debug states instead of keeping
> > > lists of specific debug messages and enabling them
> > > individually.
> >
> > For instance, look at the homebrew content in
> > drivers/gpu/drm/drm_print.c that does _not_ use
> > dynamic_debug.
> >
> > 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)\n"
> > "\t\tBit 8 (0x100) will enable DP messages (displayport code)");
> > module_param_named(debug, __drm_debug, int, 0600);
> >
> > void drm_dev_dbg(const struct device *dev, enum drm_debug_category category,
> > const char *format, ...)
> > {
> > struct va_format vaf;
> > va_list args;
> >
> > if (!drm_debug_enabled(category))
> > return;
>
> Ok, and will this proposal be able to handle stuff like this?

Yes, that's the entire point.

If it doesn't have the capability to handle stuff like this,
then no, it wouldn't be a good or useful change.

That includes the ability to work without dynamic debug and
perhaps still use a MODULE_PARM_DESC.