Re: [Intel-gfx] [PATCH v4 1/3] drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification
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> > --- > > 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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 1/3] drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification
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> > > --- > > > 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. -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: PGP signature ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 1/3] drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification
On Tue, 2017-09-19 at 18:03 +0800, Shuo Liu wrote: > On Tue 19.Sep'17 at 12:11:28 +0300, Joonas Lahtinen wrote: > > The title should begin with "drm/i915:" not "drm/i915/gvt:". > > My fault. Will correct it. > > > > On Tue, 2017-09-19 at 15:54 +0800, Shuo Liu wrote: > > > > Commit message is always needed. > > OK, will add it. > > > > > Signed-off-by: Shuo Liu> > > > > > > > > @@ -65,6 +65,7 @@ struct i915_params i915 __read_mostly = { > > > .inject_load_failure = 0, > > > .enable_dpcd_backlight = false, > > > .enable_gvt = false, > > > + .debug_gvt = 0, > > > > Let's try to keep the struct ordered. > > What's the order? I see it's different with struct definition. Yeah, it seems to be messed. Leave it as it is, I'll send a fix for all of them. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 1/3] drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification
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--- 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. 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? Thanks Shuo -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 1/3] drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification
On Tue 19.Sep'17 at 12:11:28 +0300, Joonas Lahtinen wrote: The title should begin with "drm/i915:" not "drm/i915/gvt:". My fault. Will correct it. On Tue, 2017-09-19 at 15:54 +0800, Shuo Liu wrote: Commit message is always needed. OK, will add it. Signed-off-by: Shuo Liu@@ -65,6 +65,7 @@ struct i915_params i915 __read_mostly = { .inject_load_failure = 0, .enable_dpcd_backlight = false, .enable_gvt = false, + .debug_gvt = 0, Let's try to keep the struct ordered. What's the order? I see it's different with struct definition. @@ -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" + "\t\tBit 0 (0x01) will enable CORE messages (GVT-g core message)\n" + "\t\tBit 1 (0x02) will enable IRQ messages (GVT-g interrupt message)\n" + "\t\tBit 2 (0x04) will enable MM messages (GVT-g memory management message)\n" + "\t\tBit 3 (0x08) will enable MMIO messages (GVT-g MMIO message)\n" + "\t\tBit 4 (0x10) will enable DPY messages (GVT-g display message)\n" + "\t\tBit 5 (0x20) will enable EL messages (GVT-g execlist message)\n" + "\t\tBit 6 (0x40) will enable SCHED messages (GVT-g schedule message)\n" + "\t\tBit 7 (0x80) will enable RENDER messages (GVT-g render message)\n" + "\t\tBit 8 (0x100) will enable CMD messages (GVT-g command message)"); s/message/code/ for the description in braces, like the drm counterpart. Or change them to be just "GVT-g core", "GVT-g scheduling", "GVT-g commands" etc. Will change to "code". One tab indent is enough like mostly used in the file. OK. Thanks. Shuo Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 1/3] drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification
Quoting Shuo Liu (2017-09-19 08:54:43) > Signed-off-by: Shuo Liu> --- > 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. 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. -Chris ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v4 1/3] drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification
The title should begin with "drm/i915:" not "drm/i915/gvt:". On Tue, 2017-09-19 at 15:54 +0800, Shuo Liu wrote: Commit message is always needed. > Signed-off-by: Shuo Liu> @@ -65,6 +65,7 @@ struct i915_params i915 __read_mostly = { > .inject_load_failure = 0, > .enable_dpcd_backlight = false, > .enable_gvt = false, > + .debug_gvt = 0, Let's try to keep the struct ordered. > @@ -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" > + "\t\tBit 0 (0x01) will enable CORE messages (GVT-g core > message)\n" > + "\t\tBit 1 (0x02) will enable IRQ messages (GVT-g interrupt > message)\n" > + "\t\tBit 2 (0x04) will enable MM messages (GVT-g memory > management message)\n" > + "\t\tBit 3 (0x08) will enable MMIO messages (GVT-g MMIO > message)\n" > + "\t\tBit 4 (0x10) will enable DPY messages (GVT-g display > message)\n" > + "\t\tBit 5 (0x20) will enable EL messages (GVT-g execlist > message)\n" > + "\t\tBit 6 (0x40) will enable SCHED messages (GVT-g schedule > message)\n" > + "\t\tBit 7 (0x80) will enable RENDER messages (GVT-g render > message)\n" > + "\t\tBit 8 (0x100) will enable CMD messages (GVT-g command > message)"); s/message/code/ for the description in braces, like the drm counterpart. Or change them to be just "GVT-g core", "GVT-g scheduling", "GVT-g commands" etc. One tab indent is enough like mostly used in the file. Regards, Joonas -- Joonas Lahtinen Open Source Technology Center Intel Corporation ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH v4 1/3] drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification
Signed-off-by: Shuo Liu--- 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" + "\t\tBit 0 (0x01) will enable CORE messages (GVT-g core message)\n" + "\t\tBit 1 (0x02) will enable IRQ messages (GVT-g interrupt message)\n" + "\t\tBit 2 (0x04) will enable MM messages (GVT-g memory management message)\n" + "\t\tBit 3 (0x08) will enable MMIO messages (GVT-g MMIO message)\n" + "\t\tBit 4 (0x10) will enable DPY messages (GVT-g display message)\n" + "\t\tBit 5 (0x20) will enable EL messages (GVT-g execlist message)\n" + "\t\tBit 6 (0x40) will enable SCHED messages (GVT-g schedule message)\n" + "\t\tBit 7 (0x80) will enable RENDER messages (GVT-g render message)\n" + "\t\tBit 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); \ -- 1.9.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx