Re: [PATCH v4 1/3] drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification

Shuo Liu
2017-09-20T02:21:42Z

On Wed 20.Sep'17 at 4:45:31 +0800, Zhenyu Wang wrote:
>On 2017.09.19 18:17:04 +0800, Shuo Liu wrote:
>> On Tue 19.Sep'17 at 10:22:16 +0100, Chris Wilson wrote:
>> > Quoting Shuo Liu (2017-09-19 08:54:43)
>> > > Signed-off-by: Shuo Liu <shuo.a.liu@intel.com>
>> > > ---
>> > > drivers/gpu/drm/i915/i915_params.c | 13 +++++++++++++
>> > > drivers/gpu/drm/i915/i915_params.h | 1 +
>> > > 2 files changed, 14 insertions(+)
>> > >
>> > > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
>> > > index 8ab003d..ceeae1d 100644
>> > > --- a/drivers/gpu/drm/i915/i915_params.c
>> > > +++ b/drivers/gpu/drm/i915/i915_params.c
>> > > @@ -65,6 +65,7 @@ struct i915_params i915 __read_mostly = {
>> > > .inject_load_failure = 0,
>> > > .enable_dpcd_backlight = false,
>> > > .enable_gvt = false,
>> > > + .debug_gvt = 0,
>> > > };
>> > >
>> > > module_param_named(modeset, i915.modeset, int, 0400);
>> > > @@ -257,3 +258,15 @@ struct i915_params i915 __read_mostly = {
>> > > module_param_named(enable_gvt, i915.enable_gvt, bool, 0400);
>> > > MODULE_PARM_DESC(enable_gvt,
>> > > "Enable support for Intel GVT-g graphics virtualization host support(default:false)");
>> > > +
>> > > +module_param_named(debug_gvt, i915.debug_gvt, int, 0600);
>> > > +MODULE_PARM_DESC(debug_gvt, "Enable GVT-g debug output, where each bit enables a category.\n"
>> > > + " Bit 0 (0x01) will enable CORE messages (GVT-g core message)\n"
>> > > + " Bit 1 (0x02) will enable IRQ messages (GVT-g interrupt message)\n"
>> > > + " Bit 2 (0x04) will enable MM messages (GVT-g memory management message)\n"
>> > > + " Bit 3 (0x08) will enable MMIO messages (GVT-g MMIO message)\n"
>> > > + " Bit 4 (0x10) will enable DPY messages (GVT-g display message)\n"
>> > > + " Bit 5 (0x20) will enable EL messages (GVT-g execlist message)\n"
>> > > + " Bit 6 (0x40) will enable SCHED messages (GVT-g schedule message)\n"
>> > > + " Bit 7 (0x80) will enable RENDER messages (GVT-g render message)\n"
>> > > + " Bit 8 (0x100) will enable CMD messages (GVT-g command message)");
>> > > diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
>> > > index ac84470..cd29c78 100644
>> > > --- a/drivers/gpu/drm/i915/i915_params.h
>> > > +++ b/drivers/gpu/drm/i915/i915_params.h
>> > > @@ -54,6 +54,7 @@
>> > > func(int, edp_vswing); \
>> > > func(int, reset); \
>> > > func(unsigned int, inject_load_failure); \
>> > > + func(int, debug_gvt); \
>> > > /* leave bools at the end to not create holes */ \
>> > > func(bool, alpha_support); \
>> > > func(bool, enable_cmd_parser); \
>> >
>> > Note that you are not forced to use i915_params. If you have gvt only
>> > module options that you don't want exposed outside of gvt, just create
>> > them within gvt.
>> Have talked this with Zhenyu, his suggestion is put params together as
>> gvt is in i915 module.
>
>As gvt is in i915 module now, so I think better to put all the params
>in one place instead of any surprise.
>
>> >
>> > Having said that, I would strongly advise against having module options,
>> > and I would advise you to go the dyndebug route instead of copying a
>> > rectangular wheel.
>> Thanks Chris. dyndebug might be an option. The disadvantage of dyndebug
>> is complicated to use, expecially in bootup debugging (add a long
>> cmdline to enable interesting messages). Module option is
>> straightforward. Zhenyu, any comments on this?
>>
>
>yeah, I think moving to dyndebug is better than module option and boot debug
>shouldn't be that hard right? as we're mostly with file seperation for different
>function unit.
>
OK. I will try to move to dyndebug approach.

Thanks
Shuo