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

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

[Shuo Liu]

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"
+		"\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

[PATCH v4 2/3] drm/i915/gvt: Change log function of kvmgt to common ones

[Shuo Liu]

This can remove the dependence of i915 module if we change gvt_dbg_* to
function in i915 module.

Signed-off-by: Shuo Liu <shuo.a.liu@intel.com>
---
 drivers/gpu/drm/i915/gvt/kvmgt.c | 55 ++++++++++++++++++++++++++--------------
 1 file changed, 36 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/kvmgt.c b/drivers/gpu/drm/i915/gvt/kvmgt.c
index 83e88c7..0dae361 100644
--- a/drivers/gpu/drm/i915/gvt/kvmgt.c
+++ b/drivers/gpu/drm/i915/gvt/kvmgt.c
@@ -89,6 +89,20 @@ static inline bool handle_valid(unsigned long handle)
 static void intel_vgpu_release_work(struct work_struct *work);
 static bool kvmgt_guest_exit(struct kvmgt_guest_info *info);
 
+static void gvt_kvmgt_dbg(const char *format, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, format);
+	vaf.fmt = format;
+	vaf.va = &args;
+
+	pr_debug("[gvt-mpt:%ps] %pV", __builtin_return_address(0), &vaf);
+
+	va_end(args);
+}
+
 static int gvt_dma_map_iova(struct intel_vgpu *vgpu, kvm_pfn_t pfn,
 		unsigned long *iova)
 {
@@ -443,7 +457,7 @@ static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
 
 	type = intel_gvt_find_vgpu_type(gvt, kobject_name(kobj));
 	if (!type) {
-		gvt_vgpu_err("failed to find type %s to create\n",
+		pr_err("failed to find type %s to create\n",
 						kobject_name(kobj));
 		ret = -EINVAL;
 		goto out;
@@ -452,7 +466,7 @@ static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
 	vgpu = intel_gvt_ops->vgpu_create(gvt, type);
 	if (IS_ERR_OR_NULL(vgpu)) {
 		ret = vgpu == NULL ? -EFAULT : PTR_ERR(vgpu);
-		gvt_vgpu_err("failed to create intel vgpu: %d\n", ret);
+		pr_err("failed to create intel vgpu: %d\n", ret);
 		goto out;
 	}
 
@@ -461,8 +475,7 @@ static int intel_vgpu_create(struct kobject *kobj, struct mdev_device *mdev)
 	vgpu->vdev.mdev = mdev;
 	mdev_set_drvdata(mdev, vgpu);
 
-	gvt_dbg_core("intel_vgpu_create succeeded for mdev: %s\n",
-		     dev_name(mdev_dev(mdev)));
+	gvt_kvmgt_dbg("succeeded for mdev: %s\n", dev_name(mdev_dev(mdev)));
 	ret = 0;
 
 out:
@@ -532,8 +545,8 @@ static int intel_vgpu_open(struct mdev_device *mdev)
 	ret = vfio_register_notifier(mdev_dev(mdev), VFIO_IOMMU_NOTIFY, &events,
 				&vgpu->vdev.iommu_notifier);
 	if (ret != 0) {
-		gvt_vgpu_err("vfio_register_notifier for iommu failed: %d\n",
-			ret);
+		pr_err("vgpu[%d]: vfio_register_notifier for iommu failed:%d\n",
+			vgpu->id, ret);
 		goto out;
 	}
 
@@ -541,8 +554,8 @@ static int intel_vgpu_open(struct mdev_device *mdev)
 	ret = vfio_register_notifier(mdev_dev(mdev), VFIO_GROUP_NOTIFY, &events,
 				&vgpu->vdev.group_notifier);
 	if (ret != 0) {
-		gvt_vgpu_err("vfio_register_notifier for group failed: %d\n",
-			ret);
+		pr_err("vgpu[%d]: vfio_register_notifier for group failed:%d\n",
+			vgpu->id, ret);
 		goto undo_iommu;
 	}
 
@@ -647,7 +660,7 @@ static ssize_t intel_vgpu_rw(struct mdev_device *mdev, char *buf,
 
 
 	if (index >= VFIO_PCI_NUM_REGIONS) {
-		gvt_vgpu_err("invalid index: %u\n", index);
+		pr_err("vgpu[%d]: invalid index: %u\n", vgpu->id, index);
 		return -EINVAL;
 	}
 
@@ -681,7 +694,7 @@ static ssize_t intel_vgpu_rw(struct mdev_device *mdev, char *buf,
 	case VFIO_PCI_VGA_REGION_INDEX:
 	case VFIO_PCI_ROM_REGION_INDEX:
 	default:
-		gvt_vgpu_err("unsupported region: %u\n", index);
+		pr_err("vgpu[%d]: unsupported region: %u\n", vgpu->id, index);
 	}
 
 	return ret == 0 ? count : ret;
@@ -873,7 +886,8 @@ static int intel_vgpu_set_msi_trigger(struct intel_vgpu *vgpu,
 
 		trigger = eventfd_ctx_fdget(fd);
 		if (IS_ERR(trigger)) {
-			gvt_vgpu_err("eventfd_ctx_fdget failed\n");
+			pr_err("vgpu[%d]: eventfd_ctx_fdget failed\n",
+					vgpu->id);
 			return PTR_ERR(trigger);
 		}
 		vgpu->vdev.msi_trigger = trigger;
@@ -929,7 +943,7 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
 	struct intel_vgpu *vgpu = mdev_get_drvdata(mdev);
 	unsigned long minsz;
 
-	gvt_dbg_core("vgpu%d ioctl, cmd: %d\n", vgpu->id, cmd);
+	gvt_kvmgt_dbg("vgpu%d ioctl, cmd: %d\n", vgpu->id, cmd);
 
 	if (cmd == VFIO_DEVICE_GET_INFO) {
 		struct vfio_device_info info;
@@ -1016,12 +1030,12 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
 			info.size = 0;
 
 			info.flags = 0;
-			gvt_dbg_core("get region info bar:%d\n", info.index);
+			gvt_kvmgt_dbg("get region info bar:%d\n", info.index);
 			break;
 
 		case VFIO_PCI_ROM_REGION_INDEX:
 		case VFIO_PCI_VGA_REGION_INDEX:
-			gvt_dbg_core("get region info index:%d\n", info.index);
+			gvt_kvmgt_dbg("get region info index:%d\n", info.index);
 			break;
 		default:
 			{
@@ -1132,7 +1146,8 @@ static long intel_vgpu_ioctl(struct mdev_device *mdev, unsigned int cmd,
 			ret = vfio_set_irqs_validate_and_prepare(&hdr, max,
 						VFIO_PCI_NUM_IRQS, &data_size);
 			if (ret) {
-				gvt_vgpu_err("intel:vfio_set_irqs_validate_and_prepare failed\n");
+				pr_err("vgpu[%d]:vfio_set_irqs_validate_and_prepare failed\n",
+						vgpu->id);
 				return -EINVAL;
 			}
 			if (data_size) {
@@ -1371,7 +1386,8 @@ static int kvmgt_guest_init(struct mdev_device *mdev)
 
 	kvm = vgpu->vdev.kvm;
 	if (!kvm || kvm->mm != current->mm) {
-		gvt_vgpu_err("KVM is required to use Intel vGPU\n");
+		pr_err("vgpu[%d]: KVM is required to use Intel vGPU\n",
+				vgpu->id);
 		return -ESRCH;
 	}
 
@@ -1457,14 +1473,15 @@ static unsigned long kvmgt_gfn_to_pfn(unsigned long handle, unsigned long gfn)
 	dev = mdev_dev(info->vgpu->vdev.mdev);
 	rc = vfio_pin_pages(dev, &gfn, 1, IOMMU_READ | IOMMU_WRITE, &pfn);
 	if (rc != 1) {
-		gvt_vgpu_err("vfio_pin_pages failed for gfn 0x%lx: %d\n",
-			gfn, rc);
+		pr_err("vgpu[%d]: vfio_pin_pages failed for gfn 0x%lx: %d\n",
+			vgpu->id, gfn, rc);
 		return INTEL_GVT_INVALID_ADDR;
 	}
 	/* transfer to host iova for GFX to use DMA */
 	rc = gvt_dma_map_iova(info->vgpu, pfn, &iova);
 	if (rc) {
-		gvt_vgpu_err("gvt_dma_map_iova failed for gfn: 0x%lx\n", gfn);
+		pr_err("vgpu[%d]: gvt_dma_map_iova failed for gfn: 0x%lx\n",
+				vgpu->id, gfn);
 		vfio_unpin_pages(dev, &gfn, 1);
 		return INTEL_GVT_INVALID_ADDR;
 	}
-- 
1.9.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH v4 3/3] drm/i915/gvt: Use debug_gvt to classify GVT-g log messages

[Shuo Liu]

Add a silimar log mechanism as like drm. Classify GVT-g log messages
as different categories by differnt log functions.

Signed-off-by: Shuo Liu <shuo.a.liu@intel.com>
---
 drivers/gpu/drm/i915/Kconfig      |  8 +++++++
 drivers/gpu/drm/i915/gvt/Makefile |  1 +
 drivers/gpu/drm/i915/gvt/debug.c  | 24 +++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/debug.h  | 44 +++++++++++++++++++++++++++++----------
 4 files changed, 66 insertions(+), 11 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/debug.c

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index e9e64e8..bb6ee6e 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -125,6 +125,14 @@ config DRM_I915_GVT_KVMGT
 	  Choose this option if you want to enable KVMGT support for
 	  Intel GVT-g.
 
+config DRM_I915_GVT_DEBUG
+	bool "Enable debug support for Intel GVT-g"
+	depends on DRM_I915_GVT
+	default n
+	help
+	  Choose this option if you want to enable debug support for
+	  Intel GVT-g.
+
 menu "drm/i915 Debugging"
 depends on DRM_I915
 depends on EXPERT
diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
index f5486cb9..b1826c2 100644
--- a/drivers/gpu/drm/i915/gvt/Makefile
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -5,4 +5,5 @@ GVT_SOURCE := gvt.o aperture_gm.o handlers.o vgpu.o trace_points.o firmware.o \
 
 ccflags-y				+= -I$(src) -I$(src)/$(GVT_DIR)
 i915-y					+= $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
+i915-$(CONFIG_DRM_I915_GVT_DEBUG)	+= $(GVT_DIR)/debug.o
 obj-$(CONFIG_DRM_I915_GVT_KVMGT)	+= $(GVT_DIR)/kvmgt.o
diff --git a/drivers/gpu/drm/i915/gvt/debug.c b/drivers/gpu/drm/i915/gvt/debug.c
new file mode 100644
index 0000000..75b4d0c
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/debug.c
@@ -0,0 +1,24 @@
+#include <linux/kernel.h>
+#include <linux/string.h>
+#include "i915_drv.h"
+#include "debug.h"
+
+void gvt_printk(const char *level, unsigned int category,
+		const char *format, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	if (category != GVT_MSG_NONE && !(i915.debug_gvt & category))
+		return;
+
+	va_start(args, format);
+	vaf.fmt = format;
+	vaf.va = &args;
+
+	printk("%s[gvt:%ps]%s %pV",
+		level, __builtin_return_address(0),
+		strcmp(level, KERN_ERR) == 0 ? " *ERROR*" : "", &vaf);
+
+	va_end(args);
+}
diff --git a/drivers/gpu/drm/i915/gvt/debug.h b/drivers/gpu/drm/i915/gvt/debug.h
index b0cff4d..7a54f82 100644
--- a/drivers/gpu/drm/i915/gvt/debug.h
+++ b/drivers/gpu/drm/i915/gvt/debug.h
@@ -24,42 +24,64 @@
 #ifndef __GVT_DEBUG_H__
 #define __GVT_DEBUG_H__
 
+#define GVT_MSG_NONE		0x00
+#define GVT_MSG_CORE		0x01
+#define GVT_MSG_IRQ		0x02
+#define GVT_MSG_MM		0x04
+#define GVT_MSG_MMIO		0x08
+#define GVT_MSG_DPY		0x10
+#define GVT_MSG_EL		0x20
+#define GVT_MSG_SCHED		0x40
+#define GVT_MSG_RENDER		0x80
+#define GVT_MSG_CMD		0x100
+
+#if IS_ENABLED(CONFIG_DRM_I915_GVT_DEBUG)
+__printf(3, 4)
+void gvt_printk(const char *level, unsigned int category,
+		const char *format, ...);
+#else
+static inline __printf(3, 4)
+void gvt_printk(const char *level, unsigned int category,
+		const char *format, ...) {}
+#endif
+
 #define gvt_err(fmt, args...) \
 	DRM_ERROR("gvt: "fmt, ##args)
 
 #define gvt_vgpu_err(fmt, args...)					\
 do {									\
 	if (IS_ERR_OR_NULL(vgpu))					\
-		DRM_DEBUG_DRIVER("gvt: "fmt, ##args);			\
+		gvt_printk(KERN_WARNING, GVT_MSG_CORE, fmt, ##args);	\
 	else								\
-		DRM_DEBUG_DRIVER("gvt: vgpu %d: "fmt, vgpu->id, ##args);\
+		gvt_printk(KERN_WARNING, GVT_MSG_CORE,			\
+				"vgpu %d: "fmt, vgpu->id, ##args);	\
 } while (0)
 
 #define gvt_dbg_core(fmt, args...) \
-	DRM_DEBUG_DRIVER("gvt: core: "fmt, ##args)
+	gvt_printk(KERN_DEBUG, GVT_MSG_CORE, "core: "fmt, ##args)
 
 #define gvt_dbg_irq(fmt, args...) \
-	DRM_DEBUG_DRIVER("gvt: irq: "fmt, ##args)
+	gvt_printk(KERN_DEBUG, GVT_MSG_IRQ, "irq: "fmt, ##args)
 
 #define gvt_dbg_mm(fmt, args...) \
-	DRM_DEBUG_DRIVER("gvt: mm: "fmt, ##args)
+	gvt_printk(KERN_DEBUG, GVT_MSG_MM, "mm: "fmt, ##args)
 
 #define gvt_dbg_mmio(fmt, args...) \
-	DRM_DEBUG_DRIVER("gvt: mmio: "fmt, ##args)
+	gvt_printk(KERN_DEBUG, GVT_MSG_MMIO, "mmio: "fmt, ##args)
 
 #define gvt_dbg_dpy(fmt, args...) \
-	DRM_DEBUG_DRIVER("gvt: dpy: "fmt, ##args)
+	gvt_printk(KERN_DEBUG, GVT_MSG_DPY, "dpy: "fmt, ##args)
 
 #define gvt_dbg_el(fmt, args...) \
-	DRM_DEBUG_DRIVER("gvt: el: "fmt, ##args)
+	gvt_printk(KERN_DEBUG, GVT_MSG_EL, "el: "fmt, ##args)
 
 #define gvt_dbg_sched(fmt, args...) \
-	DRM_DEBUG_DRIVER("gvt: sched: "fmt, ##args)
+	gvt_printk(KERN_DEBUG, GVT_MSG_SCHED, "sched: "fmt, ##args)
 
 #define gvt_dbg_render(fmt, args...) \
-	DRM_DEBUG_DRIVER("gvt: render: "fmt, ##args)
+	gvt_printk(KERN_DEBUG, GVT_MSG_RENDER, "render: "fmt, ##args)
 
 #define gvt_dbg_cmd(fmt, args...) \
-	DRM_DEBUG_DRIVER("gvt: cmd: "fmt, ##args)
+	gvt_printk(KERN_DEBUG, GVT_MSG_CMD, "cmd: "fmt, ##args)
 
 #endif
-- 
1.9.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Joonas Lahtinen]

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 <shuo.a.liu@intel.com>

<SNIP>

> @@ -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

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

[Chris Wilson]

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.

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: [PATCH v4 1/3] drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification

[Shuo Liu]

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 <shuo.a.liu@intel.com>
>
><SNIP>
>
>> @@ -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: [PATCH v4 1/3] drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification

[Shuo Liu]

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.
>
>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: [PATCH v4 1/3] drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification

[Joonas Lahtinen]

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 <shuo.a.liu@intel.com>
> > 
> > <SNIP>
> > 
> > > @@ -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

✗ Fi.CI.BAT: failure for series starting with [v4,1/3] drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification

[Patchwork]

== Series Details ==

Series: series starting with [v4,1/3] drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification
URL   : https://patchwork.freedesktop.org/series/30574/
State : failure

== Summary ==

Series 30574v1 series starting with [v4,1/3] drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification
https://patchwork.freedesktop.org/api/1.0/series/30574/revisions/1/mbox/

Test chamelium:
        Subgroup dp-crc-fast:
                pass       -> FAIL       (fi-kbl-7500u) fdo#102514
Test kms_addfb_basic:
        Subgroup small-bo:
                pass       -> DMESG-WARN (fi-kbl-7500u)
        Subgroup unused-pitches:
                pass       -> DMESG-WARN (fi-kbl-7500u)
Test kms_cursor_legacy:
        Subgroup basic-busy-flip-before-cursor-atomic:
                pass       -> FAIL       (fi-snb-2600) fdo#100215
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                pass       -> INCOMPLETE (fi-kbl-7500u)

fdo#102514 https://bugs.freedesktop.org/show_bug.cgi?id=102514
fdo#100215 https://bugs.freedesktop.org/show_bug.cgi?id=100215

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:436s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:476s
fi-blb-e6850     total:289  pass:224  dwarn:1   dfail:0   fail:0   skip:64  time:421s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:520s
fi-bwr-2160      total:289  pass:184  dwarn:0   dfail:0   fail:0   skip:105 time:279s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:500s
fi-byt-j1900     total:289  pass:254  dwarn:1   dfail:0   fail:0   skip:34  time:491s
fi-byt-n2820     total:289  pass:250  dwarn:1   dfail:0   fail:0   skip:38  time:489s
fi-cfl-s         total:289  pass:222  dwarn:35  dfail:0   fail:0   skip:32  time:539s
fi-elk-e7500     total:289  pass:230  dwarn:0   dfail:0   fail:0   skip:59  time:419s
fi-glk-1         total:289  pass:259  dwarn:1   dfail:0   fail:0   skip:29  time:563s
fi-hsw-4770r     total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:404s
fi-ilk-650       total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:428s
fi-ivb-3520m     total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:487s
fi-ivb-3770      total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:463s
fi-kbl-7500u     total:245  pass:220  dwarn:3   dfail:0   fail:1   skip:20 
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:580s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:585s
fi-pnv-d510      total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:545s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:449s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:748s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:493s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:469s
fi-snb-2520m     total:289  pass:251  dwarn:0   dfail:0   fail:0   skip:38  time:566s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:1   skip:39  time:420s
fi-hsw-4770 failed to connect after reboot

ab2e3a0685b0a5cfba35ddda1f9179c2e9bff3e0 drm-tip: 2017y-09m-19d-16h-17m-40s UTC integration manifest
c3c73d8ad385 drm/i915/gvt: Use debug_gvt to classify GVT-g log messages
56ead48f3560 drm/i915/gvt: Change log function of kvmgt to common ones
af7592e743ac drm/i915/gvt: Add gvt_debug in i915_params for GVT-g log classification

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_5746/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Zhenyu Wang]

[-- Attachment #1.1: Type: text/plain, Size: 3945 bytes --]

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.

-- 
Open Source Technology Center, Intel ltd.

$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

[Shuo Liu]

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx