On 2018-12-06 3:51 a.m., Joe Perches wrote: > On Thu, 2018-12-06 at 10:40 +0800, Zhang, Jerry(Junwei) wrote: >> On 12/6/18 12:56 AM, Michel Dänzer wrote: >>> From: Michel Dänzer <michel.daenzer@xxxxxxx> >>> >>> The following cases are possible for pr_debug(): >>> >>> 1. CONFIG_DYNAMIC_DEBUG disabled >>> a) DEBUG not defined: pr_debug() translates to no_printk(...), i.e. >>> it never generates any output. >>> b) DEBUG defined: pr_debug() translates to printk(KERN_DEBUG ...), >>> i.e. it generates output which doesn't appear in dmesg by default, >>> can be enabled dynamically. >>> >>> 2. CONFIG_DYNAMIC_DEBUG enabled: pr_debug() translates to >>> dynamic_pr_debug() >>> a) DEBUG not defined: dynamic_pr_debug() generates no output by >>> default, can be enabled dynamically. >>> b) DEBUG defined: dynamic_pr_debug() generates output by default, >>> can be disabled dynamically. >>> >>> The intention for drm_debug_printer() is to generate output which >>> doesn't appear in dmesg by default, but can be enabled dynamically, i.e. >>> cases 1b) and 2a). However, defining DEBUG unconditionally gave us 2b) >>> instead of 2a) with CONFIG_DYNAMIC_DEBUG enabled. >>> >>> Fixes: 79a5ad2fdb3c ("drm: Enable pr_debug() for drm_printer") > > I very much doubt this is a fix. > > Did you read the commit log for this commit? > > It says "make sure it will always produce output" I thought the commit log covered this, suggestions for improvement welcome. Chris' change addressed case 1a), but also took us from 2a) to 2b). But we want 2a). I suspect Chris missed that pr_debug()'s output is visible by default if CONFIG_DYNAMIC_DEBUG and DEBUG are both defined. > And why didn't you cc Chris Wilson, the author of that patch? I used the get_maintainer.pl script. Thanks for adding Chris. P.S. FYI, your e-mail had a very aggressive tone to me, not sure what for. >>> Signed-off-by: Michel Dänzer <michel.daenzer@xxxxxxx> >> Reviewed-by: Junwei Zhang <Jerry.Zhang@xxxxxxx> >> >>> --- >>> drivers/gpu/drm/drm_print.c | 6 ++++-- >>> 1 file changed, 4 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/drm_print.c b/drivers/gpu/drm/drm_print.c >>> index 0e7fc3e7dfb4..ee56e4a1b343 100644 >>> --- a/drivers/gpu/drm/drm_print.c >>> +++ b/drivers/gpu/drm/drm_print.c >>> @@ -23,11 +23,13 @@ >>> * Rob Clark <robdclark@xxxxxxxxx> >>> */ >>> >>> -#define DEBUG /* for pr_debug() */ >>> - >>> #include <stdarg.h> >>> #include <linux/seq_file.h> >>> #include <drm/drmP.h> >>> + >>> +#ifndef CONFIG_DYNAMIC_DEBUG >>> +#define DEBUG /* for pr_debug() */ >>> +#endif >>> #include <drm/drm_print.h> >>> >>> void __drm_puts_coredump(struct drm_printer *p, const char *str) > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel > -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel