All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v7 00/11] Introduce the implementation of GVT context
@ 2016-06-07 15:18 Zhi Wang
  2016-06-07 15:18 ` [PATCH v7 01/11] drm/i915: Factor out i915_pvinfo.h Zhi Wang
                   ` (11 more replies)
  0 siblings, 12 replies; 30+ messages in thread
From: Zhi Wang @ 2016-06-07 15:18 UTC (permalink / raw)
  To: chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

This patchset introduces the implementation of GVT context. GVT
context is a special GEM context used by GVT-g. GVT-g uses it as the shadow
context.It doesn't have a drm client nor a PPGTT. And it requires a larger
ring buffer with several special features need by GVT-g workload scheduler
like context status change notification, context single submission...

v7:

- Take Joonas comments.

v6:

- Take Chris comments.

v5:

- Drop PPGTT related patches.
- Let most functions take struct drm_i915_private *
- Fixed some misspelled words in Kconfig
- Only complied some feature when CONFIG_DRM_I915_GVT=y
- Drop the fecne related changes, will send it after this series.

v4:

- Based on the latest drm-intel-nightly branch.
- Drop PPGTT refactor patches. (GVT-g will use LRI to load PDPs)
- Drop i915_gem_context() refactor patches, reuse kernel context functions.
  (Dave Gordon)
- Drop context allocation params and refactor as the lrc deferred
  allocation function has been refactored in another styles.
- Re-wrtie GVT context creation function

Difference from community release
---------------------------------

This patchset is different from regular iGVT-g code release[4], which
is still based on old host-mediated architecture. Furthermore, this
patchset only supports BDW whereas code release supports HSW/BDW/SKL.
We will add SKL support later based on this RFC code and HSW support
will be dropped.

Internally we tested this RFC patchset with both linux and windows VM
and the architecture changes work fine.

Acknowledgment
---------------

iGVT-g implementation is several years effort and many people
contributed to the code. There names are not here yet. In later formal
patchset we will reflect individual's contribution.

Meanwhile, in the previous iGVT-g related discussion, Daniel, Chris
and Joonas ever gave very good inputs. We appreciate them and look
forward to more comments/suggestions from community.

We are trying to get more familiar with i915 but may still have gaps.
We are willing to adopt suggestions to keep improving. We hope to work
with community together to make iGVT-g a great component in i915 to
support graphics virtualization. Thanks!

Reference
---------

[1] https://01.org/igvt-g
[2] http://lists.freedesktop.org/archives/intel-gfx/2014-September/053098.html
[3] http://lists.freedesktop.org/archives/intel-gfx/2015-September/075397.html


Bing Niu (1):
  drm/i915: Introduce host graphics memory partition for GVT-g

Zhi Wang (10):
  drm/i915: Factor out i915_pvinfo.h
  drm/i915: Use offsetof() to calculate the offset of members in PVINFO
    page
  drm/i915: Fold vGPU active check into inner functions
  drm/i915: Add teardown path in intel_vgt_ballon()
  drm/i915: gvt: Introduce the basic architecture of GVT-g
  drm/i915: Make ring buffer size of a LRC context configurable
  drm/i915: Make addressing mode bits in context descriptor configurable
  drm/i915: Introduce execlist context status change notification
  drm/i915: Support LRC context single submission
  drm/i915: Introduce GVT context creation API

 drivers/gpu/drm/i915/Kconfig            |  22 +++++
 drivers/gpu/drm/i915/Makefile           |   5 +
 drivers/gpu/drm/i915/gvt/Makefile       |   5 +
 drivers/gpu/drm/i915/gvt/debug.h        |  34 +++++++
 drivers/gpu/drm/i915/gvt/gvt.c          | 158 ++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/gvt.h          |  72 +++++++++++++++
 drivers/gpu/drm/i915/gvt/hypercall.h    |  38 ++++++++
 drivers/gpu/drm/i915/gvt/mpt.h          |  49 ++++++++++
 drivers/gpu/drm/i915/i915_dma.c         |  16 +++-
 drivers/gpu/drm/i915/i915_drv.h         |  15 +++
 drivers/gpu/drm/i915/i915_gem_context.c |  69 ++++++++++++++
 drivers/gpu/drm/i915/i915_gem_gtt.c     |  11 +--
 drivers/gpu/drm/i915/i915_params.c      |   5 +
 drivers/gpu/drm/i915/i915_params.h      |   1 +
 drivers/gpu/drm/i915/i915_pvinfo.h      | 113 +++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_reg.h         |  12 +++
 drivers/gpu/drm/i915/i915_vgpu.c        |  39 ++++++--
 drivers/gpu/drm/i915/i915_vgpu.h        |  90 +-----------------
 drivers/gpu/drm/i915/intel_gvt.c        | 100 ++++++++++++++++++++
 drivers/gpu/drm/i915/intel_gvt.h        |  81 ++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c        |  56 ++++++++---
 drivers/gpu/drm/i915/intel_lrc.h        |   5 +
 22 files changed, 875 insertions(+), 121 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/Makefile
 create mode 100644 drivers/gpu/drm/i915/gvt/debug.h
 create mode 100644 drivers/gpu/drm/i915/gvt/gvt.c
 create mode 100644 drivers/gpu/drm/i915/gvt/gvt.h
 create mode 100644 drivers/gpu/drm/i915/gvt/hypercall.h
 create mode 100644 drivers/gpu/drm/i915/gvt/mpt.h
 create mode 100644 drivers/gpu/drm/i915/i915_pvinfo.h
 create mode 100644 drivers/gpu/drm/i915/intel_gvt.c
 create mode 100644 drivers/gpu/drm/i915/intel_gvt.h

-- 
1.9.1

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* [PATCH v7 01/11] drm/i915: Factor out i915_pvinfo.h
  2016-06-07 15:18 [PATCH v7 00/11] Introduce the implementation of GVT context Zhi Wang
@ 2016-06-07 15:18 ` Zhi Wang
  2016-06-08  7:55   ` Joonas Lahtinen
  2016-06-07 15:18 ` [PATCH v7 02/11] drm/i915: Use offsetof() to calculate the offset of members in PVINFO page Zhi Wang
                   ` (10 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Zhi Wang @ 2016-06-07 15:18 UTC (permalink / raw)
  To: chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

As the PVINFO page definition is used by both GVT-g guest (vGPU) and GVT-g
host (GVT-g kernel device model), factor it out for better code structure.

v7:
- Split the "offsetof" modification into a dedicated patch. (Joonas)

v3:
- Use offsetof to calculate the member offset of PVINFO structure (Joonas)

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_pvinfo.h | 113 +++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_vgpu.h   |  86 +---------------------------
 2 files changed, 114 insertions(+), 85 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/i915_pvinfo.h

diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h
new file mode 100644
index 0000000..68bdf60
--- /dev/null
+++ b/drivers/gpu/drm/i915/i915_pvinfo.h
@@ -0,0 +1,113 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef _I915_PVINFO_H_
+#define _I915_PVINFO_H_
+
+/* The MMIO offset of the shared info between guest and host emulator */
+#define VGT_PVINFO_PAGE	0x78000
+#define VGT_PVINFO_SIZE	0x1000
+
+/*
+ * The following structure pages are defined in GEN MMIO space
+ * for virtualization. (One page for now)
+ */
+#define VGT_MAGIC         0x4776544776544776ULL	/* 'vGTvGTvG' */
+#define VGT_VERSION_MAJOR 1
+#define VGT_VERSION_MINOR 0
+
+#define INTEL_VGT_IF_VERSION_ENCODE(major, minor) ((major) << 16 | (minor))
+#define INTEL_VGT_IF_VERSION \
+	INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR, VGT_VERSION_MINOR)
+
+/*
+ * notifications from guest to vgpu device model
+ */
+enum vgt_g2v_type {
+	VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE = 2,
+	VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY,
+	VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE,
+	VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
+	VGT_G2V_EXECLIST_CONTEXT_CREATE,
+	VGT_G2V_EXECLIST_CONTEXT_DESTROY,
+	VGT_G2V_MAX,
+};
+
+struct vgt_if {
+	uint64_t magic;		/* VGT_MAGIC */
+	uint16_t version_major;
+	uint16_t version_minor;
+	uint32_t vgt_id;	/* ID of vGT instance */
+	uint32_t rsv1[12];	/* pad to offset 0x40 */
+	/*
+	 *  Data structure to describe the balooning info of resources.
+	 *  Each VM can only have one portion of continuous area for now.
+	 *  (May support scattered resource in future)
+	 *  (starting from offset 0x40)
+	 */
+	struct {
+		/* Aperture register balooning */
+		struct {
+			uint32_t base;
+			uint32_t size;
+		} mappable_gmadr;	/* aperture */
+		/* GMADR register balooning */
+		struct {
+			uint32_t base;
+			uint32_t size;
+		} nonmappable_gmadr;	/* non aperture */
+		/* allowed fence registers */
+		uint32_t fence_num;
+		uint32_t rsv2[3];
+	} avail_rs;		/* available/assigned resource */
+	uint32_t rsv3[0x200 - 24];	/* pad to half page */
+	/*
+	 * The bottom half page is for response from Gfx driver to hypervisor.
+	 */
+	uint32_t rsv4;
+	uint32_t display_ready;	/* ready for display owner switch */
+
+	uint32_t rsv5[4];
+
+	uint32_t g2v_notify;
+	uint32_t rsv6[7];
+
+	struct {
+		uint32_t lo;
+		uint32_t hi;
+	} pdp[4];
+
+	uint32_t execlist_context_descriptor_lo;
+	uint32_t execlist_context_descriptor_hi;
+
+	uint32_t  rsv7[0x200 - 24];    /* pad to one page */
+} __packed;
+
+#define vgtif_reg(x) \
+	_MMIO((VGT_PVINFO_PAGE + (long)&((struct vgt_if *)NULL)->x))
+
+/* vGPU display status to be used by the host side */
+#define VGT_DRV_DISPLAY_NOT_READY 0
+#define VGT_DRV_DISPLAY_READY     1  /* ready for display switch */
+
+#endif /* _I915_PVINFO_H_ */
diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
index 21ffcfe..07e67d5 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.h
+++ b/drivers/gpu/drm/i915/i915_vgpu.h
@@ -24,91 +24,7 @@
 #ifndef _I915_VGPU_H_
 #define _I915_VGPU_H_
 
-/* The MMIO offset of the shared info between guest and host emulator */
-#define VGT_PVINFO_PAGE	0x78000
-#define VGT_PVINFO_SIZE	0x1000
-
-/*
- * The following structure pages are defined in GEN MMIO space
- * for virtualization. (One page for now)
- */
-#define VGT_MAGIC         0x4776544776544776ULL	/* 'vGTvGTvG' */
-#define VGT_VERSION_MAJOR 1
-#define VGT_VERSION_MINOR 0
-
-#define INTEL_VGT_IF_VERSION_ENCODE(major, minor) ((major) << 16 | (minor))
-#define INTEL_VGT_IF_VERSION \
-	INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR, VGT_VERSION_MINOR)
-
-/*
- * notifications from guest to vgpu device model
- */
-enum vgt_g2v_type {
-	VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE = 2,
-	VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY,
-	VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE,
-	VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
-	VGT_G2V_EXECLIST_CONTEXT_CREATE,
-	VGT_G2V_EXECLIST_CONTEXT_DESTROY,
-	VGT_G2V_MAX,
-};
-
-struct vgt_if {
-	uint64_t magic;		/* VGT_MAGIC */
-	uint16_t version_major;
-	uint16_t version_minor;
-	uint32_t vgt_id;	/* ID of vGT instance */
-	uint32_t rsv1[12];	/* pad to offset 0x40 */
-	/*
-	 *  Data structure to describe the balooning info of resources.
-	 *  Each VM can only have one portion of continuous area for now.
-	 *  (May support scattered resource in future)
-	 *  (starting from offset 0x40)
-	 */
-	struct {
-		/* Aperture register balooning */
-		struct {
-			uint32_t base;
-			uint32_t size;
-		} mappable_gmadr;	/* aperture */
-		/* GMADR register balooning */
-		struct {
-			uint32_t base;
-			uint32_t size;
-		} nonmappable_gmadr;	/* non aperture */
-		/* allowed fence registers */
-		uint32_t fence_num;
-		uint32_t rsv2[3];
-	} avail_rs;		/* available/assigned resource */
-	uint32_t rsv3[0x200 - 24];	/* pad to half page */
-	/*
-	 * The bottom half page is for response from Gfx driver to hypervisor.
-	 */
-	uint32_t rsv4;
-	uint32_t display_ready;	/* ready for display owner switch */
-
-	uint32_t rsv5[4];
-
-	uint32_t g2v_notify;
-	uint32_t rsv6[7];
-
-	struct {
-		uint32_t lo;
-		uint32_t hi;
-	} pdp[4];
-
-	uint32_t execlist_context_descriptor_lo;
-	uint32_t execlist_context_descriptor_hi;
-
-	uint32_t  rsv7[0x200 - 24];    /* pad to one page */
-} __packed;
-
-#define vgtif_reg(x) \
-	_MMIO((VGT_PVINFO_PAGE + (long)&((struct vgt_if *)NULL)->x))
-
-/* vGPU display status to be used by the host side */
-#define VGT_DRV_DISPLAY_NOT_READY 0
-#define VGT_DRV_DISPLAY_READY     1  /* ready for display switch */
+#include "i915_pvinfo.h"
 
 extern void i915_check_vgpu(struct drm_i915_private *dev_priv);
 extern int intel_vgt_balloon(struct drm_device *dev);
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v7 02/11] drm/i915: Use offsetof() to calculate the offset of members in PVINFO page
  2016-06-07 15:18 [PATCH v7 00/11] Introduce the implementation of GVT context Zhi Wang
  2016-06-07 15:18 ` [PATCH v7 01/11] drm/i915: Factor out i915_pvinfo.h Zhi Wang
@ 2016-06-07 15:18 ` Zhi Wang
  2016-06-08  7:57   ` Joonas Lahtinen
  2016-06-07 15:18 ` [PATCH v7 03/11] drm/i915: Fold vGPU active check into inner functions Zhi Wang
                   ` (9 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Zhi Wang @ 2016-06-07 15:18 UTC (permalink / raw)
  To: chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

To get the offset of the members in PVINFO page, offsetof() looks much
better than the tricky approach in current code.

v7:

- Move "offsetof()" modification into a dedicated patch. (Joonas)

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_pvinfo.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h
index 68bdf60..7b3cec4 100644
--- a/drivers/gpu/drm/i915/i915_pvinfo.h
+++ b/drivers/gpu/drm/i915/i915_pvinfo.h
@@ -104,7 +104,7 @@ struct vgt_if {
 } __packed;
 
 #define vgtif_reg(x) \
-	_MMIO((VGT_PVINFO_PAGE + (long)&((struct vgt_if *)NULL)->x))
+	_MMIO((VGT_PVINFO_PAGE + offsetof(struct vgt_if, x)))
 
 /* vGPU display status to be used by the host side */
 #define VGT_DRV_DISPLAY_NOT_READY 0
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v7 03/11] drm/i915: Fold vGPU active check into inner functions
  2016-06-07 15:18 [PATCH v7 00/11] Introduce the implementation of GVT context Zhi Wang
  2016-06-07 15:18 ` [PATCH v7 01/11] drm/i915: Factor out i915_pvinfo.h Zhi Wang
  2016-06-07 15:18 ` [PATCH v7 02/11] drm/i915: Use offsetof() to calculate the offset of members in PVINFO page Zhi Wang
@ 2016-06-07 15:18 ` Zhi Wang
  2016-06-08  8:04   ` Joonas Lahtinen
  2016-06-07 15:18 ` [PATCH v7 04/11] drm/i915: Add teardown path in intel_vgt_ballon() Zhi Wang
                   ` (8 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Zhi Wang @ 2016-06-07 15:18 UTC (permalink / raw)
  To: chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

v5:
- Let functions take struct drm_i915_private *. (Tvrtko)

- Fold vGPU related active check into the inner functions. (Kevin)

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++-------
 drivers/gpu/drm/i915/i915_vgpu.c    | 13 +++++++++----
 drivers/gpu/drm/i915/i915_vgpu.h    |  4 ++--
 3 files changed, 15 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
index 4668477..6f203fa 100644
--- a/drivers/gpu/drm/i915/i915_gem_gtt.c
+++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
@@ -2732,11 +2732,9 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
 	i915_address_space_init(&ggtt->base, dev_priv);
 	ggtt->base.total += PAGE_SIZE;
 
-	if (intel_vgpu_active(dev_priv)) {
-		ret = intel_vgt_balloon(dev);
-		if (ret)
-			return ret;
-	}
+	ret = intel_vgt_balloon(dev_priv);
+	if (ret)
+		return ret;
 
 	if (!HAS_LLC(dev))
 		ggtt->base.mm.color_adjust = i915_gtt_color_adjust;
@@ -2836,8 +2834,7 @@ void i915_ggtt_cleanup_hw(struct drm_device *dev)
 	i915_gem_cleanup_stolen(dev);
 
 	if (drm_mm_initialized(&ggtt->base.mm)) {
-		if (intel_vgpu_active(dev_priv))
-			intel_vgt_deballoon();
+		intel_vgt_deballoon(dev_priv);
 
 		drm_mm_takedown(&ggtt->base.mm);
 		list_del(&ggtt->base.global_link);
diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index 0043262..c9800d4 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -101,10 +101,13 @@ static struct _balloon_info_ bl_info;
  * This function is called to deallocate the ballooned-out graphic memory, when
  * driver is unloaded or when ballooning fails.
  */
-void intel_vgt_deballoon(void)
+void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
 {
 	int i;
 
+	if (!intel_vgpu_active(dev_priv))
+		return;
+
 	DRM_DEBUG("VGT deballoon.\n");
 
 	for (i = 0; i < 4; i++) {
@@ -177,9 +180,8 @@ static int vgt_balloon_space(struct drm_mm *mm,
  * Returns:
  * zero on success, non-zero if configuration invalid or ballooning failed
  */
-int intel_vgt_balloon(struct drm_device *dev)
+int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 {
-	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct i915_ggtt *ggtt = &dev_priv->ggtt;
 	unsigned long ggtt_end = ggtt->base.start + ggtt->base.total;
 
@@ -187,6 +189,9 @@ int intel_vgt_balloon(struct drm_device *dev)
 	unsigned long unmappable_base, unmappable_size, unmappable_end;
 	int ret;
 
+	if (!intel_vgpu_active(dev_priv))
+		return 0;
+
 	mappable_base = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.base));
 	mappable_size = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.size));
 	unmappable_base = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.base));
@@ -258,6 +263,6 @@ int intel_vgt_balloon(struct drm_device *dev)
 
 err:
 	DRM_ERROR("VGT balloon fail\n");
-	intel_vgt_deballoon();
+	intel_vgt_deballoon(dev_priv);
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
index 07e67d5..f8917c6 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.h
+++ b/drivers/gpu/drm/i915/i915_vgpu.h
@@ -27,7 +27,7 @@
 #include "i915_pvinfo.h"
 
 extern void i915_check_vgpu(struct drm_i915_private *dev_priv);
-extern int intel_vgt_balloon(struct drm_device *dev);
-extern void intel_vgt_deballoon(void);
+extern int intel_vgt_balloon(struct drm_i915_private *dev_priv);
+extern void intel_vgt_deballoon(struct drm_i915_private *dev_priv);
 
 #endif /* _I915_VGPU_H_ */
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v7 04/11] drm/i915: Add teardown path in intel_vgt_ballon()
  2016-06-07 15:18 [PATCH v7 00/11] Introduce the implementation of GVT context Zhi Wang
                   ` (2 preceding siblings ...)
  2016-06-07 15:18 ` [PATCH v7 03/11] drm/i915: Fold vGPU active check into inner functions Zhi Wang
@ 2016-06-07 15:18 ` Zhi Wang
  2016-06-08  8:12   ` Joonas Lahtinen
  2016-06-07 15:18 ` [PATCH v7 05/11] drm/i915: gvt: Introduce the basic architecture of GVT-g Zhi Wang
                   ` (7 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Zhi Wang @ 2016-06-07 15:18 UTC (permalink / raw)
  To: chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

This function needs to be changed to have a proper goto teardown path.
Destructors/fini functions are only expected to be called after a
successful initialization, so calling it at random phase in init function
is bad. (Joonas)

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_vgpu.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index c9800d4..d41a29e 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -187,7 +187,7 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 
 	unsigned long mappable_base, mappable_size, mappable_end;
 	unsigned long unmappable_base, unmappable_size, unmappable_end;
-	int ret;
+	int ret, i;
 
 	if (!intel_vgpu_active(dev_priv))
 		return 0;
@@ -263,6 +263,9 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 
 err:
 	DRM_ERROR("VGT balloon fail\n");
-	intel_vgt_deballoon(dev_priv);
+	for (i = 0; i < ARRAY_SIZE(bl_info.space); i++) {
+		if (bl_info.space[i].allocated)
+			drm_mm_remove_node(&bl_info.space[i]);
+	}
 	return ret;
 }
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v7 05/11] drm/i915: gvt: Introduce the basic architecture of GVT-g
  2016-06-07 15:18 [PATCH v7 00/11] Introduce the implementation of GVT context Zhi Wang
                   ` (3 preceding siblings ...)
  2016-06-07 15:18 ` [PATCH v7 04/11] drm/i915: Add teardown path in intel_vgt_ballon() Zhi Wang
@ 2016-06-07 15:18 ` Zhi Wang
  2016-06-08  8:28   ` Joonas Lahtinen
  2016-06-07 15:18 ` [PATCH v7 06/11] drm/i915: Introduce host graphics memory partition for GVT-g Zhi Wang
                   ` (6 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Zhi Wang @ 2016-06-07 15:18 UTC (permalink / raw)
  To: chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

This patch introduces the very basic framework of GVT-g device model,
includes basic prototypes, definitions, initialization.

v7:
- Refine the URL link in Kconfig. (Joonas)
- Refine the introduction of GVT-g host support in Kconfig. (Joonas)
- Remove the macro GVT_ALIGN(), use round_down() instead. (Joonas)
- Make "struct intel_gvt" a data member in struct drm_i915_private.(Joonas)
	- Remove {alloc, free}_gvt_device()
	- Rename intel_gvt_{create, destroy}_gvt_device()
	- Expost intel_gvt_init_host()
- Remove the dummy "struct intel_gvt" declaration in intel_gvt.h (Joonas)

v6:
- Refine introduction in Kconfig. (Chris)
- The exposed API functions will take struct intel_gvt * instead of
void *. (Chris/Tvrtko)
- Remove most memebers of strct intel_gvt_device_info. Will add them
in the device model patches.(Chris)
- Remove gvt_info() and gvt_err() in debug.h. (Chris)
- Move GVT kernel parameter into i915_params. (Chris)
- Remove include/drm/i915_gvt.h, as GVT-g will be built within i915.
- Remove the redundant struct i915_gvt *, as the functions in i915
will directly take struct intel_gvt *.
- Add more comments for reviewer.

v5:
Take Tvrtko's comments:
- Fix the misspelled words in Kconfig
- Let functions take drm_i915_private * instead of struct drm_device *
- Remove redundant prints/local varible initialization

v3:
Take Joonas' comments:
- Change file name i915_gvt.* to intel_gvt.*
- Move GVT kernel parameter into intel_gvt.c
- Remove redundant debug macros
- Change error handling style
- Add introductions for some stub functions
- Introduce drm/i915_gvt.h.

Take Kevin's comments:
- Move GVT-g host/guest check into intel_vgt_balloon in i915_gem_gtt.c

v2:
- Introduce i915_gvt.c.
It's necessary to introduce the stubs between i915 driver and GVT-g host,
as GVT-g components is configurable in kernel config. When disabled, the
stubs here do nothing.

Take Joonas' comments:
- Replace boolean return value with int.
- Replace customized info/warn/debug macros with DRM macros.
- Document all non-static functions like i915.
- Remove empty and unused functions.
- Replace magic number with marcos.
- Set GVT-g in kernel config to "n" by default.

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/Kconfig         |  22 +++++
 drivers/gpu/drm/i915/Makefile        |   5 ++
 drivers/gpu/drm/i915/gvt/Makefile    |   5 ++
 drivers/gpu/drm/i915/gvt/debug.h     |  34 ++++++++
 drivers/gpu/drm/i915/gvt/gvt.c       | 158 +++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/gvt/gvt.h       |  72 ++++++++++++++++
 drivers/gpu/drm/i915/gvt/hypercall.h |  38 +++++++++
 drivers/gpu/drm/i915/gvt/mpt.h       |  49 +++++++++++
 drivers/gpu/drm/i915/i915_dma.c      |  16 +++-
 drivers/gpu/drm/i915/i915_drv.h      |  10 +++
 drivers/gpu/drm/i915/i915_params.c   |   5 ++
 drivers/gpu/drm/i915/i915_params.h   |   1 +
 drivers/gpu/drm/i915/intel_gvt.c     | 100 ++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_gvt.h     |  45 ++++++++++
 14 files changed, 556 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/gvt/Makefile
 create mode 100644 drivers/gpu/drm/i915/gvt/debug.h
 create mode 100644 drivers/gpu/drm/i915/gvt/gvt.c
 create mode 100644 drivers/gpu/drm/i915/gvt/gvt.h
 create mode 100644 drivers/gpu/drm/i915/gvt/hypercall.h
 create mode 100644 drivers/gpu/drm/i915/gvt/mpt.h
 create mode 100644 drivers/gpu/drm/i915/intel_gvt.c
 create mode 100644 drivers/gpu/drm/i915/intel_gvt.h

diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 29a32b1..7769e46 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -57,6 +57,28 @@ config DRM_I915_USERPTR
 
 	  If in doubt, say "Y".
 
+config DRM_I915_GVT
+        bool "Enable Intel GVT-g graphics virtualization host support"
+        depends on DRM_I915
+        default n
+        help
+	  Choose this option if you want to enable Intel GVT-g graphics
+	  virtualization technology host support with integrated graphics.
+	  With GVT-g, it's possible to have one integrated graphics
+	  device shared by multiple VMs under different hypervisors.
+
+	  Note that at least one hypervisor like Xen or KVM is required for
+	  this driver to work, and it only supports newer device from
+	  Broadwell+. For further information and setup guide, you can
+	  visit: http://01.org/igvt-g.
+
+	  Now it's just a stub to support the modifications of i915 for
+	  GVT device model. It requires at least one MPT modules for Xen/KVM
+	  and other components of GVT device model to work. Use it under
+	  you own risk.
+
+	  If in doubt, say "N".
+
 menu "drm/i915 Debugging"
 depends on DRM_I915
 depends on EXPERT
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index 7e29444..276abf1 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -104,6 +104,11 @@ i915-y += i915_vgpu.o
 # legacy horrors
 i915-y += i915_dma.o
 
+ifeq ($(CONFIG_DRM_I915_GVT),y)
+i915-y += intel_gvt.o
+include $(src)/gvt/Makefile
+endif
+
 obj-$(CONFIG_DRM_I915)  += i915.o
 
 CFLAGS_i915_trace_points.o := -I$(src)
diff --git a/drivers/gpu/drm/i915/gvt/Makefile b/drivers/gpu/drm/i915/gvt/Makefile
new file mode 100644
index 0000000..d0f21a6
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/Makefile
@@ -0,0 +1,5 @@
+GVT_DIR := gvt
+GVT_SOURCE := gvt.o
+
+ccflags-y                      += -I$(src) -I$(src)/$(GVT_DIR) -Wall
+i915-y			       += $(addprefix $(GVT_DIR)/, $(GVT_SOURCE))
diff --git a/drivers/gpu/drm/i915/gvt/debug.h b/drivers/gpu/drm/i915/gvt/debug.h
new file mode 100644
index 0000000..7ef412b
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/debug.h
@@ -0,0 +1,34 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef __GVT_DEBUG_H__
+#define __GVT_DEBUG_H__
+
+#define gvt_dbg_core(fmt, args...) \
+	DRM_DEBUG_DRIVER("gvt: core: "fmt, ##args)
+
+/*
+ * Other GVT debug stuff will be introduced in the GVT device model patches.
+ */
+
+#endif
diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
new file mode 100644
index 0000000..def1bc8
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -0,0 +1,158 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include <linux/types.h>
+#include <xen/xen.h>
+
+#include "i915_drv.h"
+
+struct intel_gvt_host intel_gvt_host;
+
+static const char * const supported_hypervisors[] = {
+	[INTEL_GVT_HYPERVISOR_XEN] = "XEN",
+	[INTEL_GVT_HYPERVISOR_KVM] = "KVM",
+};
+
+/**
+ * intel_gvt_init_host - Load MPT modules and detect if we're running in host
+ * @gvt: intel gvt device
+ *
+ * This function is called at the driver loading stage. If failed to find a
+ * loadable MPT module or detect currently we're running in a VM, then GVT-g
+ * will be disabled
+ *
+ * Returns:
+ * Zero on success, negative error code if failed.
+ *
+ */
+int intel_gvt_init_host(void)
+{
+	if (intel_gvt_host.initialized)
+		return 0;
+
+	/* Xen DOM U */
+	if (xen_domain() && !xen_initial_domain())
+		return -ENODEV;
+
+	/* Try to load MPT modules for hypervisors */
+	if (xen_initial_domain()) {
+		/* In Xen dom0 */
+		intel_gvt_host.mpt = try_then_request_module(
+				symbol_get(xengt_mpt), "xengt");
+		intel_gvt_host.hypervisor_type = INTEL_GVT_HYPERVISOR_XEN;
+	} else {
+		/* not in Xen. Try KVMGT */
+		intel_gvt_host.mpt = try_then_request_module(
+				symbol_get(kvmgt_mpt), "kvm");
+		intel_gvt_host.hypervisor_type = INTEL_GVT_HYPERVISOR_KVM;
+	}
+
+	/* Fail to load MPT modules - bail out */
+	if (!intel_gvt_host.mpt)
+		return -EINVAL;
+
+	/* Try to detect if we're running in host instead of VM. */
+	if (!intel_gvt_hypervisor_detect_host())
+		return -ENODEV;
+
+	gvt_dbg_core("Running with hypervisor %s in host mode\n",
+			supported_hypervisors[intel_gvt_host.hypervisor_type]);
+
+	idr_init(&intel_gvt_host.gvt_idr);
+	mutex_init(&intel_gvt_host.gvt_idr_lock);
+	intel_gvt_host.initialized = true;
+	return 0;
+}
+
+static void init_device_info(struct intel_gvt *gvt)
+{
+	if (IS_BROADWELL(gvt->dev_priv))
+		gvt->device_info.max_support_vgpus = 8;
+	/* This function will grow large in GVT device model patches. */
+}
+
+/**
+ * intel_gvt_clean_device - clean a GVT device
+ * @gvt: intel gvt device
+ *
+ * This function is called at the driver unloading stage, to free the
+ * resources owned by a GVT device.
+ *
+ */
+void intel_gvt_clean_device(struct drm_i915_private *dev_priv)
+{
+	struct intel_gvt *gvt = &dev_priv->gvt;
+
+	if (WARN_ON(!gvt->initialized))
+		return;
+
+	mutex_lock(&intel_gvt_host.gvt_idr_lock);
+	idr_remove(&intel_gvt_host.gvt_idr, gvt->id);
+	mutex_unlock(&intel_gvt_host.gvt_idr_lock);
+	/* Other de-initialization of GVT components will be introduced. */
+
+	gvt->initialized = false;
+}
+
+/**
+ * intel_gvt_init_device - initialize a GVT device
+ * @dev_priv: drm i915 private data
+ *
+ * This function is called at the initialization stage, to initialize
+ * necessary GVT components.
+ *
+ * Returns:
+ * Zero on success, negative error code if failed.
+ *
+ */
+int intel_gvt_init_device(struct drm_i915_private *dev_priv)
+{
+	struct intel_gvt *gvt = &dev_priv->gvt;
+	int ret;
+	/*
+	 * Cannot initialize GVT device without intel_gvt_host gets
+	 * initialized first.
+	 */
+	if (WARN_ON(!intel_gvt_host.initialized))
+		return -EINVAL;
+
+	if (WARN_ON(gvt->initialized))
+		return -EEXIST;
+
+	mutex_lock(&intel_gvt_host.gvt_idr_lock);
+	ret = idr_alloc(&intel_gvt_host.gvt_idr, gvt, 0, 0, GFP_KERNEL);
+	mutex_unlock(&intel_gvt_host.gvt_idr_lock);
+
+	if (ret < 0)
+		return ret;
+
+	gvt_dbg_core("init gvt device, id %d\n", gvt->id);
+
+	init_device_info(gvt);
+	/*
+	 * Other initialization of GVT components will be introduce here.
+	 */
+	gvt_dbg_core("gvt device creation is done, id %d\n", gvt->id);
+	gvt->initialized = true;
+	return 0;
+}
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
new file mode 100644
index 0000000..bf3f79d
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -0,0 +1,72 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef _GVT_H_
+#define _GVT_H_
+
+#include "debug.h"
+#include "hypercall.h"
+
+#define GVT_MAX_VGPU 8
+
+enum {
+	INTEL_GVT_HYPERVISOR_XEN = 0,
+	INTEL_GVT_HYPERVISOR_KVM,
+};
+
+struct intel_gvt_host {
+	bool initialized;
+	int hypervisor_type;
+	struct mutex gvt_idr_lock;
+	struct idr gvt_idr;
+	struct intel_gvt_mpt *mpt;
+};
+
+extern struct intel_gvt_host intel_gvt_host;
+
+/* Describe per-platform limitations. */
+struct intel_gvt_device_info {
+	u32 max_support_vgpus;
+	/* This data structure will grow bigger in GVT device model patches */
+};
+
+struct intel_vgpu {
+	struct intel_gvt *gvt;
+	int id;
+	unsigned long handle; /* vGPU handle used by hypervisor MPT modules */
+};
+
+struct intel_gvt {
+	struct mutex lock;
+	int id;
+	bool initialized;
+
+	struct drm_i915_private *dev_priv;
+	struct idr vgpu_idr;	/* vGPU IDR pool */
+
+	struct intel_gvt_device_info device_info;
+};
+
+#include "mpt.h"
+
+#endif
diff --git a/drivers/gpu/drm/i915/gvt/hypercall.h b/drivers/gpu/drm/i915/gvt/hypercall.h
new file mode 100644
index 0000000..254df8b
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/hypercall.h
@@ -0,0 +1,38 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef _GVT_HYPERCALL_H_
+#define _GVT_HYPERCALL_H_
+
+/*
+ * Specific GVT-g MPT modules function collections. Currently GVT-g supports
+ * both Xen and KVM by providing dedicated hypervisor-related MPT modules.
+ */
+struct intel_gvt_mpt {
+	int (*detect_host)(void);
+};
+
+extern struct intel_gvt_mpt xengt_mpt;
+extern struct intel_gvt_mpt kvmgt_mpt;
+
+#endif /* _GVT_HYPERCALL_H_ */
diff --git a/drivers/gpu/drm/i915/gvt/mpt.h b/drivers/gpu/drm/i915/gvt/mpt.h
new file mode 100644
index 0000000..783f4f8
--- /dev/null
+++ b/drivers/gpu/drm/i915/gvt/mpt.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef _GVT_MPT_H_
+#define _GVT_MPT_H_
+
+/**
+ * DOC: Hypervisor Service APIs for GVT-g Core Logic
+ *
+ * This is the glue layer between specific hypervisor MPT modules and GVT-g core
+ * logic. Each kind of hypervisor MPT module provides a collection of function
+ * callbacks via gvt_kernel_dm and will be attached to GVT host when driver
+ * loading. GVT-g core logic will call these APIs to request specific services
+ * from hypervisor.
+ */
+
+/**
+ * intel_gvt_hypervisor_detect_host - check if GVT-g is running within
+ * hypervisor host/privilged domain
+ *
+ * Returns:
+ * Zero on success, -ENODEV if current kernel is running inside a VM
+ */
+static inline int intel_gvt_hypervisor_detect_host(void)
+{
+	return intel_gvt_host.mpt->detect_host();
+}
+
+#endif /* _GVT_MPT_H_ */
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 07edaed..66b7229 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1244,18 +1244,22 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 		goto out_ggtt;
 	}
 
+	ret = intel_gvt_init(dev_priv);
+	if (ret)
+		goto out_ggtt;
+
 	/* WARNING: Apparently we must kick fbdev drivers before vgacon,
 	 * otherwise the vga fbdev driver falls over. */
 	ret = i915_kick_out_firmware_fb(dev_priv);
 	if (ret) {
 		DRM_ERROR("failed to remove conflicting framebuffer drivers\n");
-		goto out_ggtt;
+		goto out_gvt;
 	}
 
 	ret = i915_kick_out_vgacon(dev_priv);
 	if (ret) {
 		DRM_ERROR("failed to remove conflicting VGA console\n");
-		goto out_ggtt;
+		goto out_gvt;
 	}
 
 	pci_set_master(dev->pdev);
@@ -1266,7 +1270,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 		if (ret) {
 			DRM_ERROR("failed to set DMA mask\n");
 
-			goto out_ggtt;
+			goto out_gvt;
 		}
 	}
 
@@ -1296,7 +1300,7 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 				     aperture_size);
 	if (!ggtt->mappable) {
 		ret = -EIO;
-		goto out_ggtt;
+		goto out_gvt;
 	}
 
 	ggtt->mtrr = arch_phys_wc_add(ggtt->mappable_base,
@@ -1329,6 +1333,8 @@ static int i915_driver_init_hw(struct drm_i915_private *dev_priv)
 
 	return 0;
 
+out_gvt:
+	intel_gvt_cleanup(dev_priv);
 out_ggtt:
 	i915_ggtt_cleanup_hw(dev);
 
@@ -1487,6 +1493,8 @@ int i915_driver_unload(struct drm_device *dev)
 
 	intel_fbdev_fini(dev);
 
+	intel_gvt_cleanup(dev_priv);
+
 	ret = i915_gem_suspend(dev);
 	if (ret) {
 		DRM_ERROR("failed to idle hardware: %d\n", ret);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 0113207..33147b1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -61,6 +61,8 @@
 #include "i915_gem_gtt.h"
 #include "i915_gem_render_state.h"
 
+#include "intel_gvt.h"
+
 /* General customization:
  */
 
@@ -1739,6 +1741,8 @@ struct drm_i915_private {
 
 	struct i915_virtual_gpu vgpu;
 
+	struct intel_gvt gvt;
+
 	struct intel_guc guc;
 
 	struct intel_csr csr;
@@ -2940,6 +2944,12 @@ void intel_uncore_forcewake_put__locked(struct drm_i915_private *dev_priv,
 u64 intel_uncore_edram_size(struct drm_i915_private *dev_priv);
 
 void assert_forcewakes_inactive(struct drm_i915_private *dev_priv);
+
+static inline bool intel_gvt_active(struct drm_i915_private *dev_priv)
+{
+	return dev_priv->gvt.initialized;
+}
+
 static inline bool intel_vgpu_active(struct drm_i915_private *dev_priv)
 {
 	return dev_priv->vgpu.active;
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 5e18cf9..c9615ce 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -60,6 +60,7 @@ struct i915_params i915 __read_mostly = {
 	.enable_dp_mst = true,
 	.inject_load_failure = 0,
 	.enable_dpcd_backlight = false,
+	.enable_gvt = false,
 };
 
 module_param_named(modeset, i915.modeset, int, 0400);
@@ -222,3 +223,7 @@ MODULE_PARM_DESC(inject_load_failure,
 module_param_named(enable_dpcd_backlight, i915.enable_dpcd_backlight, bool, 0600);
 MODULE_PARM_DESC(enable_dpcd_backlight,
 	"Enable support for DPCD backlight control (default:false)");
+
+module_param_named(enable_gvt, i915.enable_gvt, bool, 0600);
+MODULE_PARM_DESC(enable_gvt,
+	"Enable support for Intel GVT-g graphics virtualization host support(default:false)");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index 1323261..0ad020b 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -63,6 +63,7 @@ struct i915_params {
 	bool nuclear_pageflip;
 	bool enable_dp_mst;
 	bool enable_dpcd_backlight;
+	bool enable_gvt;
 };
 
 extern struct i915_params i915 __read_mostly;
diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
new file mode 100644
index 0000000..9fa458c
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -0,0 +1,100 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include "i915_drv.h"
+#include "intel_gvt.h"
+
+/**
+ * DOC: Intel GVT-g host support
+ *
+ * Intel GVT-g is a graphics virtualization technology which shares the
+ * GPU among multiple virtual machines on a time-sharing basis. Each
+ * virtual machine is presented a virtual GPU (vGPU), which has equivalent
+ * features as the underlying physical GPU (pGPU), so i915 driver can run
+ * seamlessly in a virtual machine. This file provides the englightments
+ * of GVT and the necessary components used by GVT in i915 driver.
+ */
+
+static bool is_supported_device(struct drm_i915_private *dev_priv)
+{
+	if (IS_BROADWELL(dev_priv))
+		return true;
+	return false;
+}
+
+/**
+ * intel_gvt_init - initialize GVT components
+ * @dev_priv: drm i915 private data
+ *
+ * This function is called at the initialization stage to create a GVT device.
+ *
+ * Returns:
+ * Zero on success, negative error code if failed.
+ *
+ */
+int intel_gvt_init(struct drm_i915_private *dev_priv)
+{
+	int ret;
+
+	if (!i915.enable_gvt) {
+		DRM_DEBUG_DRIVER("GVT-g is disabled by kernel params\n");
+		return 0;
+	}
+
+	if (!is_supported_device(dev_priv)) {
+		DRM_DEBUG_DRIVER("Unsupported device. GVT-g is disabled\n");
+		return 0;
+	}
+
+	/*
+	 * We're not in host or fail to find a MPT module, disable GVT-g
+	 */
+	ret = intel_gvt_init_host();
+	if (ret) {
+		DRM_DEBUG_DRIVER("Not in host or MPT modules not found\n");
+		return 0;
+	}
+
+	ret = intel_gvt_init_device(dev_priv);
+	if (ret) {
+		DRM_DEBUG_DRIVER("Fail to init GVT device\n");
+		return 0;
+	}
+
+	return 0;
+}
+
+/**
+ * intel_gvt_cleanup - cleanup GVT components when i915 driver is unloading
+ * @dev_priv: drm i915 private *
+ *
+ * This function is called at the i915 driver unloading stage, to shutdown
+ * GVT components and release the related resources.
+ */
+void intel_gvt_cleanup(struct drm_i915_private *dev_priv)
+{
+	if (!intel_gvt_active(dev_priv))
+		return;
+
+	intel_gvt_clean_device(dev_priv);
+}
diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
new file mode 100644
index 0000000..91e129f
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_gvt.h
@@ -0,0 +1,45 @@
+/*
+ * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
+ * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#ifndef _INTEL_GVT_H_
+#define _INTEL_GVT_H_
+
+#include "gvt/gvt.h"
+
+#ifdef CONFIG_DRM_I915_GVT
+extern int intel_gvt_init(struct drm_i915_private *dev_priv);
+extern void intel_gvt_cleanup(struct drm_i915_private *dev_priv);
+extern int intel_gvt_init_device(struct drm_i915_private *dev_priv);
+extern void intel_gvt_clean_device(struct drm_i915_private *dev_priv);
+extern int intel_gvt_init_host(void);
+#else
+static inline int intel_gvt_init(struct drm_i915_private *dev_priv)
+{
+	return 0;
+}
+static inline void intel_gvt_cleanup(struct drm_i915_private *dev_priv)
+{
+}
+#endif
+
+#endif /* _INTEL_GVT_H_ */
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v7 06/11] drm/i915: Introduce host graphics memory partition for GVT-g
  2016-06-07 15:18 [PATCH v7 00/11] Introduce the implementation of GVT context Zhi Wang
                   ` (4 preceding siblings ...)
  2016-06-07 15:18 ` [PATCH v7 05/11] drm/i915: gvt: Introduce the basic architecture of GVT-g Zhi Wang
@ 2016-06-07 15:18 ` Zhi Wang
  2016-06-07 15:18 ` [PATCH v7 07/11] drm/i915: Make ring buffer size of a LRC context configurable Zhi Wang
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 30+ messages in thread
From: Zhi Wang @ 2016-06-07 15:18 UTC (permalink / raw)
  To: chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

From: Bing Niu <bing.niu@intel.com>

This patch introduces host graphics memory partition when GVT-g
is enabled.

Under GVT-g, i915 host driver only owned limited graphics resources,
others are managed by GVT-g resource allocator and kept for other vGPUs.

v7:

- Add comments about low/high GM size for host. (Joonas)

v6:

- Remove kernel parameters used to configure GGTT owned by host. (Chris)
- Other coding style comments from Chris.
- Add more comments for reviewer.

v3:

- Remove fence partition, will use i915 fence stealing in future.(Kevin)
- Santinize GVT host gm kernel parameters. (Joonas)

v2:
- Address all coding-style comments from Joonas previously.
- Fix errors and warnning reported by checkpatch.pl. (Joonas)
- Move the graphs into the header files. (Daniel)

Signed-off-by: Bing Niu <bing.niu@intel.com>
Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_vgpu.c | 23 +++++++++++++++++------
 drivers/gpu/drm/i915/intel_gvt.h | 36 ++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
index d41a29e..f343335 100644
--- a/drivers/gpu/drm/i915/i915_vgpu.c
+++ b/drivers/gpu/drm/i915/i915_vgpu.c
@@ -189,14 +189,25 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
 	unsigned long unmappable_base, unmappable_size, unmappable_end;
 	int ret, i;
 
-	if (!intel_vgpu_active(dev_priv))
+	if (intel_gvt_active(dev_priv)) {
+		/* Retrieve GGTT partition information from macros */
+		mappable_base = 0;
+		mappable_size = INTEL_GVT_HOST_LOW_GM_SIZE;
+		unmappable_base = dev_priv->ggtt.mappable_end;
+		unmappable_size = INTEL_GVT_HOST_HIGH_GM_SIZE;
+	} else if (intel_vgpu_active(dev_priv)) {
+		/* Retrieve GGTT partition information from PVINFO */
+		mappable_base = I915_READ(
+				vgtif_reg(avail_rs.mappable_gmadr.base));
+		mappable_size = I915_READ(
+				vgtif_reg(avail_rs.mappable_gmadr.size));
+		unmappable_base = I915_READ(
+				vgtif_reg(avail_rs.nonmappable_gmadr.base));
+		unmappable_size = I915_READ(
+				vgtif_reg(avail_rs.nonmappable_gmadr.size));
+	} else
 		return 0;
 
-	mappable_base = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.base));
-	mappable_size = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.size));
-	unmappable_base = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.base));
-	unmappable_size = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.size));
-
 	mappable_end = mappable_base + mappable_size;
 	unmappable_end = unmappable_base + unmappable_size;
 
diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
index 91e129f..d0d71d1 100644
--- a/drivers/gpu/drm/i915/intel_gvt.h
+++ b/drivers/gpu/drm/i915/intel_gvt.h
@@ -26,6 +26,42 @@
 
 #include "gvt/gvt.h"
 
+/*
+ * Under GVT-g, i915 host driver only owned limited graphics resources,
+ * others are managed by GVT-g resource allocator and kept for other vGPUs.
+ *
+ * For graphics memory space partition, a typical layout looks like:
+ *
+ * +-------+-----------------------+------+-----------------------+
+ * |* Host |   *GVT-g Resource     |* Host|   *GVT-g Resource     |
+ * | Owned |   Allocator Managed   | Owned|   Allocator Managed   |
+ * |       |                       |      |                       |
+ * +---------------+-------+----------------------+-------+-------+
+ * |       |       |       |       |      |       |       |       |
+ * | i915  | vm 1  | vm 2  | vm 3  | i915 | vm 1  | vm 2  | vm 3  |
+ * |       |       |       |       |      |       |       |       |
+ * +-------+-------+-------+--------------+-------+-------+-------+
+ * |           Aperture            |            Hidden            |
+ * +-------------------------------+------------------------------+
+ * |                       GGTT memory space                      |
+ * +--------------------------------------------------------------+
+ */
+
+/* GGTT memory space owned by host */
+/*
+ * This amount is heavily related to the max screen resolution / multiple
+ * display in *host*. If you are using a 4K monitor or multiple display
+ * monitor, probably you should enlarge the low gm size.
+ */
+#define INTEL_GVT_HOST_LOW_GM_SIZE (96 * 1024 * 1024)
+
+/*
+ * This amount is related to the GPU workload in host. If you wish to run
+ * heavy workload like 3D gaming, media transcoding *in host* and encounter
+ * performance drops, probably you should enlarge the high gm size.
+ */
+#define INTEL_GVT_HOST_HIGH_GM_SIZE (384 * 1024 * 1024)
+
 #ifdef CONFIG_DRM_I915_GVT
 extern int intel_gvt_init(struct drm_i915_private *dev_priv);
 extern void intel_gvt_cleanup(struct drm_i915_private *dev_priv);
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v7 07/11] drm/i915: Make ring buffer size of a LRC context configurable
  2016-06-07 15:18 [PATCH v7 00/11] Introduce the implementation of GVT context Zhi Wang
                   ` (5 preceding siblings ...)
  2016-06-07 15:18 ` [PATCH v7 06/11] drm/i915: Introduce host graphics memory partition for GVT-g Zhi Wang
@ 2016-06-07 15:18 ` Zhi Wang
  2016-06-08  7:08   ` Chris Wilson
  2016-06-07 15:18 ` [PATCH v7 08/11] drm/i915: Make addressing mode bits in context descriptor configurable Zhi Wang
                   ` (4 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Zhi Wang @ 2016-06-07 15:18 UTC (permalink / raw)
  To: chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

This patch introduces an option for configuring the ring buffer size
of a LRC context after the context creation.

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         | 1 +
 drivers/gpu/drm/i915/i915_gem_context.c | 1 +
 drivers/gpu/drm/i915/intel_lrc.c        | 3 ++-
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 33147b1..50b3ea7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -878,6 +878,7 @@ struct i915_gem_context {
 		int pin_count;
 		bool initialised;
 	} engine[I915_NUM_ENGINES];
+	u32 lrc_ring_buffer_size;
 
 	struct list_head link;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index a3b11aa..1663981 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -295,6 +295,7 @@ __create_hw_context(struct drm_device *dev,
 	ctx->remap_slice = ALL_L3_SLICES(dev_priv);
 
 	ctx->hang_stats.ban_period_seconds = DRM_I915_CTX_BAN_PERIOD;
+	ctx->lrc_ring_buffer_size = 4 * PAGE_SIZE;
 
 	return ctx;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 5c191a1..72a0cca 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -2484,7 +2484,8 @@ static int execlists_context_deferred_alloc(struct i915_gem_context *ctx,
 		return PTR_ERR(ctx_obj);
 	}
 
-	ringbuf = intel_engine_create_ringbuffer(engine, 4 * PAGE_SIZE);
+	ringbuf = intel_engine_create_ringbuffer(engine,
+			ctx->lrc_ring_buffer_size);
 	if (IS_ERR(ringbuf)) {
 		ret = PTR_ERR(ringbuf);
 		goto error_deref_obj;
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v7 08/11] drm/i915: Make addressing mode bits in context descriptor configurable
  2016-06-07 15:18 [PATCH v7 00/11] Introduce the implementation of GVT context Zhi Wang
                   ` (6 preceding siblings ...)
  2016-06-07 15:18 ` [PATCH v7 07/11] drm/i915: Make ring buffer size of a LRC context configurable Zhi Wang
@ 2016-06-07 15:18 ` Zhi Wang
  2016-06-08  7:12   ` Chris Wilson
  2016-06-08  8:38   ` Joonas Lahtinen
  2016-06-07 15:18 ` [PATCH v7 09/11] drm/i915: Introduce execlist context status change notification Zhi Wang
                   ` (3 subsequent siblings)
  11 siblings, 2 replies; 30+ messages in thread
From: Zhi Wang @ 2016-06-07 15:18 UTC (permalink / raw)
  To: chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

Currently the addressing mode bit in context descriptor is statically
generated from the configuration of system-wide PPGTT usage model.

GVT-g will load the PPGTT shadow page table by itself and probably one
guest is using a different addressing mode with i915 host. The addressing
mode bits of a LRC context should be configurable under this case.

v7

- Move context addressing mode bit into i915_reg.h. (Joonas/Chris)
- Add prefix "INTEL_" for related definitions. (Joonas)

v6:

- Directly save the addressing mode bits inside i915_gem_context. (Chris)
- Move the LRC context addressing mode bits into intel_lrc.h. (Chris)

v5:

- Change USES_FULL_48BIT(dev) to USES_FULL_48BIT(dev_priv) (Tvrtko)

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  1 +
 drivers/gpu/drm/i915/i915_gem_context.c |  2 ++
 drivers/gpu/drm/i915/i915_reg.h         | 12 ++++++++++++
 drivers/gpu/drm/i915/intel_lrc.c        | 13 +------------
 4 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 50b3ea7..a3ef3eb 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -879,6 +879,7 @@ struct i915_gem_context {
 		bool initialised;
 	} engine[I915_NUM_ENGINES];
 	u32 lrc_ring_buffer_size;
+	u32 lrc_addressing_mode_bits;
 
 	struct list_head link;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index 1663981..d9d7779 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -296,6 +296,8 @@ __create_hw_context(struct drm_device *dev,
 
 	ctx->hang_stats.ban_period_seconds = DRM_I915_CTX_BAN_PERIOD;
 	ctx->lrc_ring_buffer_size = 4 * PAGE_SIZE;
+	ctx->lrc_addressing_mode_bits = GEN8_CTX_ADDRESSING_MODE(dev_priv) <<
+		GEN8_CTX_ADDRESSING_MODE_SHIFT;
 
 	return ctx;
 
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index bfebfc9..ab5970c 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -3022,6 +3022,18 @@ enum skl_disp_power_wells {
 /* Same as Haswell, but 72064 bytes now. */
 #define GEN8_CXT_TOTAL_SIZE		(18 * PAGE_SIZE)
 
+enum {
+	INTEL_ADVANCED_CONTEXT = 0,
+	INTEL_LEGACY_32B_CONTEXT,
+	INTEL_ADVANCED_AD_CONTEXT,
+	INTEL_LEGACY_64B_CONTEXT
+};
+
+#define GEN8_CTX_ADDRESSING_MODE_SHIFT 3
+#define GEN8_CTX_ADDRESSING_MODE(dev_priv) (USES_FULL_48BIT_PPGTT(dev_priv) ?\
+		INTEL_LEGACY_64B_CONTEXT : \
+		INTEL_LEGACY_32B_CONTEXT)
+
 #define CHV_CLK_CTL1			_MMIO(0x101100)
 #define VLV_CLK_CTL2			_MMIO(0x101104)
 #define   CLK_CTL2_CZCOUNT_30NS_SHIFT	28
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 72a0cca..ffb436c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -208,16 +208,6 @@
 } while (0)
 
 enum {
-	ADVANCED_CONTEXT = 0,
-	LEGACY_32B_CONTEXT,
-	ADVANCED_AD_CONTEXT,
-	LEGACY_64B_CONTEXT
-};
-#define GEN8_CTX_ADDRESSING_MODE_SHIFT 3
-#define GEN8_CTX_ADDRESSING_MODE(dev)  (USES_FULL_48BIT_PPGTT(dev) ?\
-		LEGACY_64B_CONTEXT :\
-		LEGACY_32B_CONTEXT)
-enum {
 	FAULT_AND_HANG = 0,
 	FAULT_AND_HALT, /* Debug only */
 	FAULT_AND_STREAM,
@@ -281,8 +271,6 @@ logical_ring_init_platform_invariants(struct intel_engine_cs *engine)
 					(engine->id == VCS || engine->id == VCS2);
 
 	engine->ctx_desc_template = GEN8_CTX_VALID;
-	engine->ctx_desc_template |= GEN8_CTX_ADDRESSING_MODE(dev_priv) <<
-				   GEN8_CTX_ADDRESSING_MODE_SHIFT;
 	if (IS_GEN8(dev_priv))
 		engine->ctx_desc_template |= GEN8_CTX_L3LLC_COHERENT;
 	engine->ctx_desc_template |= GEN8_CTX_PRIVILEGE;
@@ -326,6 +314,7 @@ intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
 	BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (1<<GEN8_CTX_ID_WIDTH));
 
 	desc = engine->ctx_desc_template;			/* bits  0-11 */
+	desc |= ctx->lrc_addressing_mode_bits;			/* bits  3-4  */
 	desc |= ce->lrc_vma->node.start + LRC_PPHWSP_PN * PAGE_SIZE;
 								/* bits 12-31 */
 	desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT;		/* bits 32-52 */
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v7 09/11] drm/i915: Introduce execlist context status change notification
  2016-06-07 15:18 [PATCH v7 00/11] Introduce the implementation of GVT context Zhi Wang
                   ` (7 preceding siblings ...)
  2016-06-07 15:18 ` [PATCH v7 08/11] drm/i915: Make addressing mode bits in context descriptor configurable Zhi Wang
@ 2016-06-07 15:18 ` Zhi Wang
  2016-06-07 22:01   ` Chris Wilson
  2016-06-07 15:18 ` [PATCH v7 10/11] drm/i915: Support LRC context single submission Zhi Wang
                   ` (2 subsequent siblings)
  11 siblings, 1 reply; 30+ messages in thread
From: Zhi Wang @ 2016-06-07 15:18 UTC (permalink / raw)
  To: chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

This patch introduces an approach to track the execlist context status
change.

GVT-g uses GVT context as the "shadow context". The content inside GVT
context will be copied back to guest after the context is idle. And GVT-g
has to know the status of the execlist context.

This function is configurable when creating a new GEM context. Currently,
Only GVT-g will create the "status-change-notification" enabled GEM
context.

v7:

- Remove per-engine ctx status notifiers. Use one status notifier for all
engines. (Joonas)
- Add prefix "INTEL_" for related definitions. (Joonas)
- Refine the comments in execlists_context_status_change(). (Joonas)

v6:

- When !CONFIG_DRM_I915_GVT, make GVT code as dead code then compiler
could automatically eliminate them for us. (Chris)
- Always initialize the notifier header, so it could be switched on/off
at runtime. (Chris)

v5:

- Only compile this feature when CONFIG_DRM_I915_GVT is enabled.(Tvrtko)

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  2 ++
 drivers/gpu/drm/i915/i915_gem_context.c |  1 +
 drivers/gpu/drm/i915/intel_lrc.c        | 25 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_lrc.h        |  5 +++++
 4 files changed, 33 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a3ef3eb..4ab4cf7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -880,6 +880,8 @@ struct i915_gem_context {
 	} engine[I915_NUM_ENGINES];
 	u32 lrc_ring_buffer_size;
 	u32 lrc_addressing_mode_bits;
+	struct atomic_notifier_head status_notifier;
+	bool enable_lrc_status_change_notification;
 
 	struct list_head link;
 
diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index d9d7779..b0e82a1 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -298,6 +298,7 @@ __create_hw_context(struct drm_device *dev,
 	ctx->lrc_ring_buffer_size = 4 * PAGE_SIZE;
 	ctx->lrc_addressing_mode_bits = GEN8_CTX_ADDRESSING_MODE(dev_priv) <<
 		GEN8_CTX_ADDRESSING_MODE_SHIFT;
+	ATOMIC_INIT_NOTIFIER_HEAD(&ctx->status_notifier);
 
 	return ctx;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index ffb436c..956585d 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -404,6 +404,23 @@ static void execlists_submit_requests(struct drm_i915_gem_request *rq0,
 	spin_unlock_irq(&dev_priv->uncore.lock);
 }
 
+static inline void execlists_context_status_change(
+		struct drm_i915_gem_request *rq,
+		unsigned long status)
+{
+	/*
+	 * Only used when GVT-g is enabled now. When GVT-g is disabled,
+	 * The compiler should eliminate this function as dead-code.
+	 */
+	if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
+		return;
+
+	if (!rq->ctx->enable_lrc_status_change_notification)
+		return;
+
+	atomic_notifier_call_chain(&rq->ctx->status_notifier, status, rq);
+}
+
 static void execlists_context_unqueue(struct intel_engine_cs *engine)
 {
 	struct drm_i915_gem_request *req0 = NULL, *req1 = NULL;
@@ -439,6 +456,12 @@ static void execlists_context_unqueue(struct intel_engine_cs *engine)
 	if (unlikely(!req0))
 		return;
 
+	execlists_context_status_change(req0, INTEL_CONTEXT_SCHEDULE_IN);
+
+	if (req1)
+		execlists_context_status_change(req1,
+				INTEL_CONTEXT_SCHEDULE_IN);
+
 	if (req0->elsp_submitted & engine->idle_lite_restore_wa) {
 		/*
 		 * WaIdleLiteRestore: make sure we never cause a lite restore
@@ -477,6 +500,8 @@ execlists_check_remove_request(struct intel_engine_cs *engine, u32 ctx_id)
 	if (--head_req->elsp_submitted > 0)
 		return 0;
 
+	execlists_context_status_change(head_req, INTEL_CONTEXT_SCHEDULE_OUT);
+
 	list_del(&head_req->execlist_link);
 	i915_gem_request_unreference(head_req);
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h
index a8db42a..2b8255c 100644
--- a/drivers/gpu/drm/i915/intel_lrc.h
+++ b/drivers/gpu/drm/i915/intel_lrc.h
@@ -57,6 +57,11 @@
 #define GEN8_CSB_READ_PTR(csb_status) \
 	(((csb_status) & GEN8_CSB_READ_PTR_MASK) >> 8)
 
+enum {
+	INTEL_CONTEXT_SCHEDULE_IN = 0,
+	INTEL_CONTEXT_SCHEDULE_OUT,
+};
+
 /* Logical Rings */
 int intel_logical_ring_alloc_request_extras(struct drm_i915_gem_request *request);
 int intel_logical_ring_reserve_space(struct drm_i915_gem_request *request);
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v7 10/11] drm/i915: Support LRC context single submission
  2016-06-07 15:18 [PATCH v7 00/11] Introduce the implementation of GVT context Zhi Wang
                   ` (8 preceding siblings ...)
  2016-06-07 15:18 ` [PATCH v7 09/11] drm/i915: Introduce execlist context status change notification Zhi Wang
@ 2016-06-07 15:18 ` Zhi Wang
  2016-06-08  7:04   ` Chris Wilson
  2016-06-07 15:18 ` [PATCH v7 11/11] drm/i915: Introduce GVT context creation API Zhi Wang
  2016-06-07 15:53 ` ✓ Ro.CI.BAT: success for Introduce the implementation of GVT context (rev5) Patchwork
  11 siblings, 1 reply; 30+ messages in thread
From: Zhi Wang @ 2016-06-07 15:18 UTC (permalink / raw)
  To: chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

This patch introduces the support of LRC context single submission.
As GVT context may come from different guests, which require different
configuration of render registers. It can't be combined into a dual ELSP
submission combo.

Only GVT-g will create this kinds of GEM context currently.

v7:

- Fix typos in commit message. (Joonas)

v6:
- Make GVT code as dead code when !CONFIG_DRM_I915_GVT. (Chris)

v5:

- Only compile this feature when CONFIG_DRM_I915_GVT=y. (Tvrtko)

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  |  1 +
 drivers/gpu/drm/i915/intel_lrc.c | 15 +++++++++++++++
 2 files changed, 16 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4ab4cf7..8f8a5bc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -882,6 +882,7 @@ struct i915_gem_context {
 	u32 lrc_addressing_mode_bits;
 	struct atomic_notifier_head status_notifier;
 	bool enable_lrc_status_change_notification;
+	bool enable_lrc_single_submission;
 
 	struct list_head link;
 
diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
index 956585d..e3cab06 100644
--- a/drivers/gpu/drm/i915/intel_lrc.c
+++ b/drivers/gpu/drm/i915/intel_lrc.c
@@ -447,6 +447,21 @@ static void execlists_context_unqueue(struct intel_engine_cs *engine)
 			i915_gem_request_unreference(req0);
 			req0 = cursor;
 		} else {
+			/* Compiler will do the dead-code elimination */
+			if (IS_ENABLED(CONFIG_DRM_I915_GVT)) {
+				/*
+				 * req0 (after merged) ctx requires single
+				 * submission, stop picking
+				 */
+				if (req0->ctx->enable_lrc_single_submission)
+					break;
+				/*
+				 * req0 ctx doesn't require single submission,
+				 * but next req ctx requires, stop picking
+				 */
+				if (cursor->ctx->enable_lrc_single_submission)
+					break;
+			}
 			req1 = cursor;
 			WARN_ON(req1->elsp_submitted);
 			break;
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* [PATCH v7 11/11] drm/i915: Introduce GVT context creation API
  2016-06-07 15:18 [PATCH v7 00/11] Introduce the implementation of GVT context Zhi Wang
                   ` (9 preceding siblings ...)
  2016-06-07 15:18 ` [PATCH v7 10/11] drm/i915: Support LRC context single submission Zhi Wang
@ 2016-06-07 15:18 ` Zhi Wang
  2016-06-08  6:59   ` Chris Wilson
  2016-06-07 15:53 ` ✓ Ro.CI.BAT: success for Introduce the implementation of GVT context (rev5) Patchwork
  11 siblings, 1 reply; 30+ messages in thread
From: Zhi Wang @ 2016-06-07 15:18 UTC (permalink / raw)
  To: chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

GVT workload scheduler needs special host LRC contexts, the so called
"shadow LRC context" to submit guest workload to host i915. During the
guest workload submission, workload scheduler fills the shadow LRC
context with the content of guest LRC context: engine context is copied
without changes, ring context is mostly owned by host i915.

v7:

- Move chart to a better place. (Joonas)

v6:

- Make GVT code as dead code when !CONFIG_DRM_I915_GVT. (Chris)

v5:
- Only compile this feature when CONFIG_DRM_I915_GVT is enabled. (Tvrtko)
- Rebase the code into new repo.
- Add a comment about the ring buffer size. (Joonas)

v2:

Mostly based on Daniel's idea. Call the refactored core logic of GEM
context creation service and LRC context creation service to create the GVT
context.

Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
---
 drivers/gpu/drm/i915/i915_gem_context.c | 65 +++++++++++++++++++++++++++++++++
 1 file changed, 65 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
index b0e82a1..2d8e22a 100644
--- a/drivers/gpu/drm/i915/i915_gem_context.c
+++ b/drivers/gpu/drm/i915/i915_gem_context.c
@@ -343,6 +343,71 @@ i915_gem_create_context(struct drm_device *dev,
 	return ctx;
 }
 
+/**
+ * i915_gem_create_gvt_context - create a GVT GEM context
+ * @dev: drm device *
+ *
+ * This function is used to create a GVT specific GEM context.
+ *
+ * Returns:
+ * pointer to i915_gem_context on success, error pointer if failed
+ *
+ */
+/* GVT context usage flow:
+ *
+ *          +-----------+                   +-----------+
+ *          |   vGPU    |                   |   vGPU    |
+ *          +-+-----^---+                   +-+-----^---+
+ *            |     |                         |     |
+ *            |     | GVT-g                   |     | GVT-g
+ * vELSP write|     | emulates     vELSP write|     | emulates
+ *            |     | Execlist/CSB            |     | Execlist/CSB
+ *            |     | Status                  |     | Status
+ *            |     |                         |     |
+ *     +------v-----+-------------------------v-----+---------+
+ *     |           GVT Virtual Execlist Submission            |
+ *     +------+-------------------------------+---------------+
+ *            |                               |
+ *            | Per-VM/Ring Workoad Q         | Per-VM/Ring Workload Q
+ *     +---------------------+--+      +------------------------+
+ *        +---v--------+    ^             +---v--------+
+ *        |GVT Workload|... |             |GVT Workload|...
+ *        +------------+    |             +------------+
+ *                          |
+ *                          | Pick Workload from Q
+ *     +--------------------+---------------------------------+
+ *     |                GVT Workload Scheduler                |
+ *     +--------------------+---------------------------------+
+ *                          |         * Shadow guest LRC context
+ *                   +------v------+  * Shadow guest ring buffer
+ *                   | GVT Context |  * Scan/Patch guest RB instructions
+ *                   +------+------+
+ *                          |
+ *                          v
+ *              Host i915 GEM Submission
+ */
+struct i915_gem_context *
+i915_gem_create_gvt_context(struct drm_device *dev)
+{
+	struct i915_gem_context *ctx;
+
+	if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
+		return ERR_PTR(-ENODEV);
+
+	mutex_lock(&dev->struct_mutex);
+
+	ctx = i915_gem_create_context(dev, NULL);
+	if (IS_ERR(ctx))
+		goto out;
+
+	ctx->enable_lrc_status_change_notification = true;
+	ctx->enable_lrc_single_submission = true;
+	ctx->lrc_ring_buffer_size = 512 * PAGE_SIZE; /* Max ring buffer size */
+out:
+	mutex_unlock(&dev->struct_mutex);
+	return ctx;
+}
+
 static void i915_gem_context_unpin(struct i915_gem_context *ctx,
 				   struct intel_engine_cs *engine)
 {
-- 
1.9.1

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

^ permalink raw reply related	[flat|nested] 30+ messages in thread

* ✓ Ro.CI.BAT: success for Introduce the implementation of GVT context (rev5)
  2016-06-07 15:18 [PATCH v7 00/11] Introduce the implementation of GVT context Zhi Wang
                   ` (10 preceding siblings ...)
  2016-06-07 15:18 ` [PATCH v7 11/11] drm/i915: Introduce GVT context creation API Zhi Wang
@ 2016-06-07 15:53 ` Patchwork
  11 siblings, 0 replies; 30+ messages in thread
From: Patchwork @ 2016-06-07 15:53 UTC (permalink / raw)
  To: Wang, Zhi A; +Cc: intel-gfx

== Series Details ==

Series: Introduce the implementation of GVT context (rev5)
URL   : https://patchwork.freedesktop.org/series/7208/
State : success

== Summary ==

Series 7208v5 Introduce the implementation of GVT context
http://patchwork.freedesktop.org/api/1.0/series/7208/revisions/5/mbox


fi-bdw-i7-5557u  total:102  pass:93   dwarn:0   dfail:0   fail:0   skip:8  
fi-skl-i5-6260u  total:209  pass:198  dwarn:0   dfail:0   fail:0   skip:11 
fi-skl-i7-6700k  total:209  pass:184  dwarn:0   dfail:0   fail:0   skip:25 
fi-snb-i7-2600   total:209  pass:170  dwarn:0   dfail:0   fail:0   skip:39 
ro-bdw-i5-5250u  total:183  pass:172  dwarn:0   dfail:0   fail:0   skip:10 
ro-bdw-i7-5600u  total:183  pass:154  dwarn:0   dfail:0   fail:0   skip:28 
ro-bsw-n3050     total:208  pass:167  dwarn:0   dfail:0   fail:2   skip:39 
ro-hsw-i3-4010u  total:208  pass:185  dwarn:0   dfail:0   fail:0   skip:23 
ro-hsw-i7-4770r  total:183  pass:161  dwarn:0   dfail:0   fail:0   skip:21 
ro-ilk1-i5-650   total:204  pass:146  dwarn:0   dfail:0   fail:1   skip:57 
ro-ivb-i7-3770   total:183  pass:154  dwarn:0   dfail:0   fail:0   skip:28 
ro-ivb2-i7-3770  total:183  pass:158  dwarn:0   dfail:0   fail:0   skip:24 
ro-snb-i7-2620M  total:183  pass:151  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-i7-4770k failed to connect after reboot
ro-bdw-i5-5250u failed to connect after reboot
ro-bdw-i7-5557U failed to connect after reboot

Results at /archive/results/CI_IGT_test/RO_Patchwork_1131/

b771b07 drm-intel-nightly: 2016y-06m-07d-14h-40m-53s UTC integration manifest
bb178f5 drm/i915: Introduce GVT context creation API
92209e6 drm/i915: Support LRC context single submission
2abf653 drm/i915: Introduce execlist context status change notification
9f1f9fc drm/i915: Make addressing mode bits in context descriptor configurable
784498f drm/i915: Make ring buffer size of a LRC context configurable
1e12ef6 drm/i915: Introduce host graphics memory partition for GVT-g
6d30d66 drm/i915: gvt: Introduce the basic architecture of GVT-g
29e835e drm/i915: Add teardown path in intel_vgt_ballon()
7eeffc5 drm/i915: Fold vGPU active check into inner functions
e170013 drm/i915: Use offsetof() to calculate the offset of members in PVINFO page
d44ca80 drm/i915: Factor out i915_pvinfo.h

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 09/11] drm/i915: Introduce execlist context status change notification
  2016-06-07 15:18 ` [PATCH v7 09/11] drm/i915: Introduce execlist context status change notification Zhi Wang
@ 2016-06-07 22:01   ` Chris Wilson
  2016-06-08  8:40     ` Joonas Lahtinen
  0 siblings, 1 reply; 30+ messages in thread
From: Chris Wilson @ 2016-06-07 22:01 UTC (permalink / raw)
  To: Zhi Wang; +Cc: intel-gfx, zhiyuan.lv

On Tue, Jun 07, 2016 at 11:18:45AM -0400, Zhi Wang wrote:
> This patch introduces an approach to track the execlist context status
> change.
> 
> GVT-g uses GVT context as the "shadow context". The content inside GVT
> context will be copied back to guest after the context is idle. And GVT-g
> has to know the status of the execlist context.
> 
> This function is configurable when creating a new GEM context. Currently,
> Only GVT-g will create the "status-change-notification" enabled GEM
> context.
> 
> v7:
> 
> - Remove per-engine ctx status notifiers. Use one status notifier for all
> engines. (Joonas)
> - Add prefix "INTEL_" for related definitions. (Joonas)
> - Refine the comments in execlists_context_status_change(). (Joonas)
> 
> v6:
> 
> - When !CONFIG_DRM_I915_GVT, make GVT code as dead code then compiler
> could automatically eliminate them for us. (Chris)
> - Always initialize the notifier header, so it could be switched on/off
> at runtime. (Chris)
> 
> v5:
> 
> - Only compile this feature when CONFIG_DRM_I915_GVT is enabled.(Tvrtko)
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
> +static inline void execlists_context_status_change(
> +		struct drm_i915_gem_request *rq,
> +		unsigned long status)
> +{
> +	/*
> +	 * Only used when GVT-g is enabled now. When GVT-g is disabled,
> +	 * The compiler should eliminate this function as dead-code.
> +	 */
> +	if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
> +		return;
> +
> +	if (!rq->ctx->enable_lrc_status_change_notification)
> +		return;

Was there any other justification for the boolean? (i.e. does it get
used elsewhere) I thought we mentioned this as probably premature
optimisation and should favour speeding up a no-op call_chain() if
required. So can we have callbacks in the notifier but need to disable
notification? If so, that is never explained.

> +
> +	atomic_notifier_call_chain(&rq->ctx->status_notifier, status, rq);
> +}

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 11/11] drm/i915: Introduce GVT context creation API
  2016-06-07 15:18 ` [PATCH v7 11/11] drm/i915: Introduce GVT context creation API Zhi Wang
@ 2016-06-08  6:59   ` Chris Wilson
  0 siblings, 0 replies; 30+ messages in thread
From: Chris Wilson @ 2016-06-08  6:59 UTC (permalink / raw)
  To: Zhi Wang; +Cc: intel-gfx, zhiyuan.lv

On Tue, Jun 07, 2016 at 11:18:47AM -0400, Zhi Wang wrote:
> GVT workload scheduler needs special host LRC contexts, the so called
> "shadow LRC context" to submit guest workload to host i915. During the
> guest workload submission, workload scheduler fills the shadow LRC
> context with the content of guest LRC context: engine context is copied
> without changes, ring context is mostly owned by host i915.
> 
> v7:
> 
> - Move chart to a better place. (Joonas)
> 
> v6:
> 
> - Make GVT code as dead code when !CONFIG_DRM_I915_GVT. (Chris)
> 
> v5:
> - Only compile this feature when CONFIG_DRM_I915_GVT is enabled. (Tvrtko)
> - Rebase the code into new repo.
> - Add a comment about the ring buffer size. (Joonas)
> 
> v2:
> 
> Mostly based on Daniel's idea. Call the refactored core logic of GEM
> context creation service and LRC context creation service to create the GVT
> context.
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c | 65 +++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index b0e82a1..2d8e22a 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -343,6 +343,71 @@ i915_gem_create_context(struct drm_device *dev,
>  	return ctx;
>  }
>  
> +/**
> + * i915_gem_create_gvt_context - create a GVT GEM context
> + * @dev: drm device *
> + *
> + * This function is used to create a GVT specific GEM context.
> + *
> + * Returns:
> + * pointer to i915_gem_context on success, error pointer if failed
> + *
> + */
> +/* GVT context usage flow:
> + *
> + *          +-----------+                   +-----------+
> + *          |   vGPU    |                   |   vGPU    |
> + *          +-+-----^---+                   +-+-----^---+
> + *            |     |                         |     |
> + *            |     | GVT-g                   |     | GVT-g
> + * vELSP write|     | emulates     vELSP write|     | emulates
> + *            |     | Execlist/CSB            |     | Execlist/CSB
> + *            |     | Status                  |     | Status
> + *            |     |                         |     |
> + *     +------v-----+-------------------------v-----+---------+
> + *     |           GVT Virtual Execlist Submission            |
> + *     +------+-------------------------------+---------------+
> + *            |                               |
> + *            | Per-VM/Ring Workoad Q         | Per-VM/Ring Workload Q
> + *     +---------------------+--+      +------------------------+
> + *        +---v--------+    ^             +---v--------+
> + *        |GVT Workload|... |             |GVT Workload|...
> + *        +------------+    |             +------------+
> + *                          |
> + *                          | Pick Workload from Q
> + *     +--------------------+---------------------------------+
> + *     |                GVT Workload Scheduler                |
> + *     +--------------------+---------------------------------+
> + *                          |         * Shadow guest LRC context
> + *                   +------v------+  * Shadow guest ring buffer
> + *                   | GVT Context |  * Scan/Patch guest RB instructions
> + *                   +------+------+
> + *                          |
> + *                          v
> + *              Host i915 GEM Submission
> + */

I presume you move this later.

> +struct i915_gem_context *
> +i915_gem_create_gvt_context(struct drm_device *dev)

i915_gem_context_create_gvt

No function declaration in the header, build incomplete. It is worth
making sure that every patch in the series builds and doesn't introduce
new (sparse) warnings.

> +{
> +	struct i915_gem_context *ctx;
> +
> +	if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
> +		return ERR_PTR(-ENODEV);
> +
> +	mutex_lock(&dev->struct_mutex);

User facing? If so, this is the wrong type of mutex_lock.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 10/11] drm/i915: Support LRC context single submission
  2016-06-07 15:18 ` [PATCH v7 10/11] drm/i915: Support LRC context single submission Zhi Wang
@ 2016-06-08  7:04   ` Chris Wilson
  2016-06-08  8:44     ` Joonas Lahtinen
  0 siblings, 1 reply; 30+ messages in thread
From: Chris Wilson @ 2016-06-08  7:04 UTC (permalink / raw)
  To: Zhi Wang; +Cc: intel-gfx, zhiyuan.lv

On Tue, Jun 07, 2016 at 11:18:46AM -0400, Zhi Wang wrote:
> This patch introduces the support of LRC context single submission.
> As GVT context may come from different guests, which require different
> configuration of render registers. It can't be combined into a dual ELSP
> submission combo.
> 
> Only GVT-g will create this kinds of GEM context currently.
> 
> v7:
> 
> - Fix typos in commit message. (Joonas)
> 
> v6:
> - Make GVT code as dead code when !CONFIG_DRM_I915_GVT. (Chris)
> 
> v5:
> 
> - Only compile this feature when CONFIG_DRM_I915_GVT=y. (Tvrtko)
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h  |  1 +
>  drivers/gpu/drm/i915/intel_lrc.c | 15 +++++++++++++++
>  2 files changed, 16 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4ab4cf7..8f8a5bc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -882,6 +882,7 @@ struct i915_gem_context {
>  	u32 lrc_addressing_mode_bits;
>  	struct atomic_notifier_head status_notifier;
>  	bool enable_lrc_status_change_notification;
> +	bool enable_lrc_single_submission;
>  
>  	struct list_head link;
>  
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 956585d..e3cab06 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -447,6 +447,21 @@ static void execlists_context_unqueue(struct intel_engine_cs *engine)
>  			i915_gem_request_unreference(req0);
>  			req0 = cursor;
>  		} else {
> +			/* Compiler will do the dead-code elimination */
> +			if (IS_ENABLED(CONFIG_DRM_I915_GVT)) {
> +				/*
> +				 * req0 (after merged) ctx requires single
> +				 * submission, stop picking
> +				 */
> +				if (req0->ctx->enable_lrc_single_submission)

enable_ is a bad verb here, force_ would be more apt.

s/lrc/execlists/

ctx->execlists_force_single_submission
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 07/11] drm/i915: Make ring buffer size of a LRC context configurable
  2016-06-07 15:18 ` [PATCH v7 07/11] drm/i915: Make ring buffer size of a LRC context configurable Zhi Wang
@ 2016-06-08  7:08   ` Chris Wilson
  2016-06-08  8:34     ` Joonas Lahtinen
  0 siblings, 1 reply; 30+ messages in thread
From: Chris Wilson @ 2016-06-08  7:08 UTC (permalink / raw)
  To: Zhi Wang; +Cc: intel-gfx, zhiyuan.lv

On Tue, Jun 07, 2016 at 11:18:43AM -0400, Zhi Wang wrote:
> This patch introduces an option for configuring the ring buffer size
> of a LRC context after the context creation.
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         | 1 +
>  drivers/gpu/drm/i915/i915_gem_context.c | 1 +
>  drivers/gpu/drm/i915/intel_lrc.c        | 3 ++-
>  3 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 33147b1..50b3ea7 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -878,6 +878,7 @@ struct i915_gem_context {
>  		int pin_count;
>  		bool initialised;
>  	} engine[I915_NUM_ENGINES];
> +	u32 lrc_ring_buffer_size;

u32 ring_size;

There is no reason why we need to limit it to lrc other than immediate
convenience.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 08/11] drm/i915: Make addressing mode bits in context descriptor configurable
  2016-06-07 15:18 ` [PATCH v7 08/11] drm/i915: Make addressing mode bits in context descriptor configurable Zhi Wang
@ 2016-06-08  7:12   ` Chris Wilson
  2016-06-08  8:17     ` Wang, Zhi A
  2016-06-08  8:38   ` Joonas Lahtinen
  1 sibling, 1 reply; 30+ messages in thread
From: Chris Wilson @ 2016-06-08  7:12 UTC (permalink / raw)
  To: Zhi Wang; +Cc: intel-gfx, zhiyuan.lv

On Tue, Jun 07, 2016 at 11:18:44AM -0400, Zhi Wang wrote:
> Currently the addressing mode bit in context descriptor is statically
> generated from the configuration of system-wide PPGTT usage model.
> 
> GVT-g will load the PPGTT shadow page table by itself and probably one
> guest is using a different addressing mode with i915 host. The addressing
> mode bits of a LRC context should be configurable under this case.
> 
> v7
> 
> - Move context addressing mode bit into i915_reg.h. (Joonas/Chris)
> - Add prefix "INTEL_" for related definitions. (Joonas)
> 
> v6:
> 
> - Directly save the addressing mode bits inside i915_gem_context. (Chris)
> - Move the LRC context addressing mode bits into intel_lrc.h. (Chris)
> 
> v5:
> 
> - Change USES_FULL_48BIT(dev) to USES_FULL_48BIT(dev_priv) (Tvrtko)
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  1 +
>  drivers/gpu/drm/i915/i915_gem_context.c |  2 ++
>  drivers/gpu/drm/i915/i915_reg.h         | 12 ++++++++++++
>  drivers/gpu/drm/i915/intel_lrc.c        | 13 +------------
>  4 files changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 50b3ea7..a3ef3eb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -879,6 +879,7 @@ struct i915_gem_context {
>  		bool initialised;
>  	} engine[I915_NUM_ENGINES];
>  	u32 lrc_ring_buffer_size;
> +	u32 lrc_addressing_mode_bits;

u32 addressing_mode.

Though this is better as ctx->desc_template;
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 01/11] drm/i915: Factor out i915_pvinfo.h
  2016-06-07 15:18 ` [PATCH v7 01/11] drm/i915: Factor out i915_pvinfo.h Zhi Wang
@ 2016-06-08  7:55   ` Joonas Lahtinen
  2016-06-08  8:20     ` Wang, Zhi A
  0 siblings, 1 reply; 30+ messages in thread
From: Joonas Lahtinen @ 2016-06-08  7:55 UTC (permalink / raw)
  To: Zhi Wang, chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

On ti, 2016-06-07 at 11:18 -0400, Zhi Wang wrote:
> As the PVINFO page definition is used by both GVT-g guest (vGPU) and GVT-g
> host (GVT-g kernel device model), factor it out for better code structure.
> 
> v7:
> - Split the "offsetof" modification into a dedicated patch. (Joonas)
> 
> v3:
> - Use offsetof to calculate the member offset of PVINFO structure (Joonas)
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>

Assuming this is just code motion patch per description;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_pvinfo.h | 113 +++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_vgpu.h   |  86 +---------------------------
>  2 files changed, 114 insertions(+), 85 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/i915_pvinfo.h
> 
> diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h
> new file mode 100644
> index 0000000..68bdf60
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> @@ -0,0 +1,113 @@
> +/*
> + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
> + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#ifndef _I915_PVINFO_H_
> +#define _I915_PVINFO_H_
> +
> +/* The MMIO offset of the shared info between guest and host emulator */
> +#define VGT_PVINFO_PAGE	0x78000
> +#define VGT_PVINFO_SIZE	0x1000
> +
> +/*
> + * The following structure pages are defined in GEN MMIO space
> + * for virtualization. (One page for now)
> + */
> +#define VGT_MAGIC         0x4776544776544776ULL	/* 'vGTvGTvG' */
> +#define VGT_VERSION_MAJOR 1
> +#define VGT_VERSION_MINOR 0
> +
> +#define INTEL_VGT_IF_VERSION_ENCODE(major, minor) ((major) << 16 | (minor))
> +#define INTEL_VGT_IF_VERSION \
> +	INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR, VGT_VERSION_MINOR)
> +
> +/*
> + * notifications from guest to vgpu device model
> + */
> +enum vgt_g2v_type {
> +	VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE = 2,
> +	VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY,
> +	VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE,
> +	VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
> +	VGT_G2V_EXECLIST_CONTEXT_CREATE,
> +	VGT_G2V_EXECLIST_CONTEXT_DESTROY,
> +	VGT_G2V_MAX,
> +};
> +
> +struct vgt_if {
> +	uint64_t magic;		/* VGT_MAGIC */
> +	uint16_t version_major;
> +	uint16_t version_minor;
> +	uint32_t vgt_id;	/* ID of vGT instance */
> +	uint32_t rsv1[12];	/* pad to offset 0x40 */
> +	/*
> +	 *  Data structure to describe the balooning info of resources.
> +	 *  Each VM can only have one portion of continuous area for now.
> +	 *  (May support scattered resource in future)
> +	 *  (starting from offset 0x40)
> +	 */
> +	struct {
> +		/* Aperture register balooning */
> +		struct {
> +			uint32_t base;
> +			uint32_t size;
> +		} mappable_gmadr;	/* aperture */
> +		/* GMADR register balooning */
> +		struct {
> +			uint32_t base;
> +			uint32_t size;
> +		} nonmappable_gmadr;	/* non aperture */
> +		/* allowed fence registers */
> +		uint32_t fence_num;
> +		uint32_t rsv2[3];
> +	} avail_rs;		/* available/assigned resource */
> +	uint32_t rsv3[0x200 - 24];	/* pad to half page */
> +	/*
> +	 * The bottom half page is for response from Gfx driver to hypervisor.
> +	 */
> +	uint32_t rsv4;
> +	uint32_t display_ready;	/* ready for display owner switch */
> +
> +	uint32_t rsv5[4];
> +
> +	uint32_t g2v_notify;
> +	uint32_t rsv6[7];
> +
> +	struct {
> +		uint32_t lo;
> +		uint32_t hi;
> +	} pdp[4];
> +
> +	uint32_t execlist_context_descriptor_lo;
> +	uint32_t execlist_context_descriptor_hi;
> +
> +	uint32_t  rsv7[0x200 - 24];    /* pad to one page */
> +} __packed;
> +
> +#define vgtif_reg(x) \
> +	_MMIO((VGT_PVINFO_PAGE + (long)&((struct vgt_if *)NULL)->x))
> +
> +/* vGPU display status to be used by the host side */
> +#define VGT_DRV_DISPLAY_NOT_READY 0
> +#define VGT_DRV_DISPLAY_READY     1  /* ready for display switch */
> +
> +#endif /* _I915_PVINFO_H_ */
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
> index 21ffcfe..07e67d5 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.h
> +++ b/drivers/gpu/drm/i915/i915_vgpu.h
> @@ -24,91 +24,7 @@
>  #ifndef _I915_VGPU_H_
>  #define _I915_VGPU_H_
>  
> -/* The MMIO offset of the shared info between guest and host emulator */
> -#define VGT_PVINFO_PAGE	0x78000
> -#define VGT_PVINFO_SIZE	0x1000
> -
> -/*
> - * The following structure pages are defined in GEN MMIO space
> - * for virtualization. (One page for now)
> - */
> -#define VGT_MAGIC         0x4776544776544776ULL	/* 'vGTvGTvG' */
> -#define VGT_VERSION_MAJOR 1
> -#define VGT_VERSION_MINOR 0
> -
> -#define INTEL_VGT_IF_VERSION_ENCODE(major, minor) ((major) << 16 | (minor))
> -#define INTEL_VGT_IF_VERSION \
> -	INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR, VGT_VERSION_MINOR)
> -
> -/*
> - * notifications from guest to vgpu device model
> - */
> -enum vgt_g2v_type {
> -	VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE = 2,
> -	VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY,
> -	VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE,
> -	VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
> -	VGT_G2V_EXECLIST_CONTEXT_CREATE,
> -	VGT_G2V_EXECLIST_CONTEXT_DESTROY,
> -	VGT_G2V_MAX,
> -};
> -
> -struct vgt_if {
> -	uint64_t magic;		/* VGT_MAGIC */
> -	uint16_t version_major;
> -	uint16_t version_minor;
> -	uint32_t vgt_id;	/* ID of vGT instance */
> -	uint32_t rsv1[12];	/* pad to offset 0x40 */
> -	/*
> -	 *  Data structure to describe the balooning info of resources.
> -	 *  Each VM can only have one portion of continuous area for now.
> -	 *  (May support scattered resource in future)
> -	 *  (starting from offset 0x40)
> -	 */
> -	struct {
> -		/* Aperture register balooning */
> -		struct {
> -			uint32_t base;
> -			uint32_t size;
> -		} mappable_gmadr;	/* aperture */
> -		/* GMADR register balooning */
> -		struct {
> -			uint32_t base;
> -			uint32_t size;
> -		} nonmappable_gmadr;	/* non aperture */
> -		/* allowed fence registers */
> -		uint32_t fence_num;
> -		uint32_t rsv2[3];
> -	} avail_rs;		/* available/assigned resource */
> -	uint32_t rsv3[0x200 - 24];	/* pad to half page */
> -	/*
> -	 * The bottom half page is for response from Gfx driver to hypervisor.
> -	 */
> -	uint32_t rsv4;
> -	uint32_t display_ready;	/* ready for display owner switch */
> -
> -	uint32_t rsv5[4];
> -
> -	uint32_t g2v_notify;
> -	uint32_t rsv6[7];
> -
> -	struct {
> -		uint32_t lo;
> -		uint32_t hi;
> -	} pdp[4];
> -
> -	uint32_t execlist_context_descriptor_lo;
> -	uint32_t execlist_context_descriptor_hi;
> -
> -	uint32_t  rsv7[0x200 - 24];    /* pad to one page */
> -} __packed;
> -
> -#define vgtif_reg(x) \
> -	_MMIO((VGT_PVINFO_PAGE + (long)&((struct vgt_if *)NULL)->x))
> -
> -/* vGPU display status to be used by the host side */
> -#define VGT_DRV_DISPLAY_NOT_READY 0
> -#define VGT_DRV_DISPLAY_READY     1  /* ready for display switch */
> +#include "i915_pvinfo.h"
>  
>  extern void i915_check_vgpu(struct drm_i915_private *dev_priv);
>  extern int intel_vgt_balloon(struct drm_device *dev);
-- 
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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 02/11] drm/i915: Use offsetof() to calculate the offset of members in PVINFO page
  2016-06-07 15:18 ` [PATCH v7 02/11] drm/i915: Use offsetof() to calculate the offset of members in PVINFO page Zhi Wang
@ 2016-06-08  7:57   ` Joonas Lahtinen
  0 siblings, 0 replies; 30+ messages in thread
From: Joonas Lahtinen @ 2016-06-08  7:57 UTC (permalink / raw)
  To: Zhi Wang, chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

On ti, 2016-06-07 at 11:18 -0400, Zhi Wang wrote:
> To get the offset of the members in PVINFO page, offsetof() looks much
> better than the tricky approach in current code.
> 
> v7:
> 
> - Move "offsetof()" modification into a dedicated patch. (Joonas)
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>

It's good idea to add Cc: in patches resulting from discussions (or
Suggested-by:). Then it would be easier to spot for me if I was not
reviewing the series as whole.

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_pvinfo.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h b/drivers/gpu/drm/i915/i915_pvinfo.h
> index 68bdf60..7b3cec4 100644
> --- a/drivers/gpu/drm/i915/i915_pvinfo.h
> +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> @@ -104,7 +104,7 @@ struct vgt_if {
>  } __packed;
>  
>  #define vgtif_reg(x) \
> -	_MMIO((VGT_PVINFO_PAGE + (long)&((struct vgt_if *)NULL)->x))
> +	_MMIO((VGT_PVINFO_PAGE + offsetof(struct vgt_if, x)))
>  
>  /* vGPU display status to be used by the host side */
>  #define VGT_DRV_DISPLAY_NOT_READY 0
-- 
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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 03/11] drm/i915: Fold vGPU active check into inner functions
  2016-06-07 15:18 ` [PATCH v7 03/11] drm/i915: Fold vGPU active check into inner functions Zhi Wang
@ 2016-06-08  8:04   ` Joonas Lahtinen
  2016-06-08  8:06     ` Wang, Zhi A
  0 siblings, 1 reply; 30+ messages in thread
From: Joonas Lahtinen @ 2016-06-08  8:04 UTC (permalink / raw)
  To: Zhi Wang, chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

On ti, 2016-06-07 at 11:18 -0400, Zhi Wang wrote:
> v5:
> - Let functions take struct drm_i915_private *. (Tvrtko)
> 
> - Fold vGPU related active check into the inner functions. (Kevin)
> 

I already reviewed this patch, so you should add Reviewed-by: and Cc:
tags. Also good to add Cc: tag for Tvrtko and Kevin as they have given
comments.

Regards, Joonas

> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++-------
>  drivers/gpu/drm/i915/i915_vgpu.c    | 13 +++++++++----
>  drivers/gpu/drm/i915/i915_vgpu.h    |  4 ++--
>  3 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 4668477..6f203fa 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2732,11 +2732,9 @@ static int i915_gem_setup_global_gtt(struct drm_device *dev,
>  	i915_address_space_init(&ggtt->base, dev_priv);
>  	ggtt->base.total += PAGE_SIZE;
>  
> -	if (intel_vgpu_active(dev_priv)) {
> -		ret = intel_vgt_balloon(dev);
> -		if (ret)
> -			return ret;
> -	}
> +	ret = intel_vgt_balloon(dev_priv);
> +	if (ret)
> +		return ret;
>  
>  	if (!HAS_LLC(dev))
>  		ggtt->base.mm.color_adjust = i915_gtt_color_adjust;
> @@ -2836,8 +2834,7 @@ void i915_ggtt_cleanup_hw(struct drm_device *dev)
>  	i915_gem_cleanup_stolen(dev);
>  
>  	if (drm_mm_initialized(&ggtt->base.mm)) {
> -		if (intel_vgpu_active(dev_priv))
> -			intel_vgt_deballoon();
> +		intel_vgt_deballoon(dev_priv);
>  
>  		drm_mm_takedown(&ggtt->base.mm);
>  		list_del(&ggtt->base.global_link);
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
> index 0043262..c9800d4 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -101,10 +101,13 @@ static struct _balloon_info_ bl_info;
>   * This function is called to deallocate the ballooned-out graphic memory, when
>   * driver is unloaded or when ballooning fails.
>   */
> -void intel_vgt_deballoon(void)
> +void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
>  {
>  	int i;
>  
> +	if (!intel_vgpu_active(dev_priv))
> +		return;
> +
>  	DRM_DEBUG("VGT deballoon.\n");
>  
>  	for (i = 0; i < 4; i++) {
> @@ -177,9 +180,8 @@ static int vgt_balloon_space(struct drm_mm *mm,
>   * Returns:
>   * zero on success, non-zero if configuration invalid or ballooning failed
>   */
> -int intel_vgt_balloon(struct drm_device *dev)
> +int intel_vgt_balloon(struct drm_i915_private *dev_priv)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(dev);
>  	struct i915_ggtt *ggtt = &dev_priv->ggtt;
>  	unsigned long ggtt_end = ggtt->base.start + ggtt->base.total;
>  
> @@ -187,6 +189,9 @@ int intel_vgt_balloon(struct drm_device *dev)
>  	unsigned long unmappable_base, unmappable_size, unmappable_end;
>  	int ret;
>  
> +	if (!intel_vgpu_active(dev_priv))
> +		return 0;
> +
>  	mappable_base = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.base));
>  	mappable_size = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.size));
>  	unmappable_base = I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.base));
> @@ -258,6 +263,6 @@ int intel_vgt_balloon(struct drm_device *dev)
>  
>  err:
>  	DRM_ERROR("VGT balloon fail\n");
> -	intel_vgt_deballoon();
> +	intel_vgt_deballoon(dev_priv);
>  	return ret;
>  }
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h
> index 07e67d5..f8917c6 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.h
> +++ b/drivers/gpu/drm/i915/i915_vgpu.h
> @@ -27,7 +27,7 @@
>  #include "i915_pvinfo.h"
>  
>  extern void i915_check_vgpu(struct drm_i915_private *dev_priv);
> -extern int intel_vgt_balloon(struct drm_device *dev);
> -extern void intel_vgt_deballoon(void);
> +extern int intel_vgt_balloon(struct drm_i915_private *dev_priv);
> +extern void intel_vgt_deballoon(struct drm_i915_private *dev_priv);
>  
>  #endif /* _I915_VGPU_H_ */
-- 
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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 03/11] drm/i915: Fold vGPU active check into inner functions
  2016-06-08  8:04   ` Joonas Lahtinen
@ 2016-06-08  8:06     ` Wang, Zhi A
  0 siblings, 0 replies; 30+ messages in thread
From: Wang, Zhi A @ 2016-06-08  8:06 UTC (permalink / raw)
  To: Joonas Lahtinen, chris, Lv, Zhiyuan, Tian, Kevin, tvrtko.ursulin,
	intel-gfx

OK. Will do in the next version. :)

> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> Sent: Wednesday, June 08, 2016 11:05 AM
> To: Wang, Zhi A <zhi.a.wang@intel.com>; chris@chris-wilson.co.uk; Lv, Zhiyuan
> <zhiyuan.lv@intel.com>; Tian, Kevin <kevin.tian@intel.com>;
> tvrtko.ursulin@linux.intel.com; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v7 03/11] drm/i915: Fold vGPU active check into
> inner functions
> 
> On ti, 2016-06-07 at 11:18 -0400, Zhi Wang wrote:
> > v5:
> > - Let functions take struct drm_i915_private *. (Tvrtko)
> >
> > - Fold vGPU related active check into the inner functions. (Kevin)
> >
> 
> I already reviewed this patch, so you should add Reviewed-by: and Cc:
> tags. Also good to add Cc: tag for Tvrtko and Kevin as they have given
> comments.
> 
> Regards, Joonas
> 
> > Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_gem_gtt.c | 11 ++++-------
> >  drivers/gpu/drm/i915/i915_vgpu.c    | 13 +++++++++----
> >  drivers/gpu/drm/i915/i915_vgpu.h    |  4 ++--
> >  3 files changed, 15 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > index 4668477..6f203fa 100644
> > --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> > +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> > @@ -2732,11 +2732,9 @@ static int i915_gem_setup_global_gtt(struct
> drm_device *dev,
> >  	i915_address_space_init(&ggtt->base, dev_priv);
> >  	ggtt->base.total += PAGE_SIZE;
> >
> > -	if (intel_vgpu_active(dev_priv)) {
> > -		ret = intel_vgt_balloon(dev);
> > -		if (ret)
> > -			return ret;
> > -	}
> > +	ret = intel_vgt_balloon(dev_priv);
> > +	if (ret)
> > +		return ret;
> >
> >  	if (!HAS_LLC(dev))
> >  		ggtt->base.mm.color_adjust = i915_gtt_color_adjust; @@ -2836,8
> > +2834,7 @@ void i915_ggtt_cleanup_hw(struct drm_device *dev)
> >  	i915_gem_cleanup_stolen(dev);
> >
> >  	if (drm_mm_initialized(&ggtt->base.mm)) {
> > -		if (intel_vgpu_active(dev_priv))
> > -			intel_vgt_deballoon();
> > +		intel_vgt_deballoon(dev_priv);
> >
> >  		drm_mm_takedown(&ggtt->base.mm);
> >  		list_del(&ggtt->base.global_link);
> > diff --git a/drivers/gpu/drm/i915/i915_vgpu.c
> > b/drivers/gpu/drm/i915/i915_vgpu.c
> > index 0043262..c9800d4 100644
> > --- a/drivers/gpu/drm/i915/i915_vgpu.c
> > +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> > @@ -101,10 +101,13 @@ static struct _balloon_info_ bl_info;
> >   * This function is called to deallocate the ballooned-out graphic
> > memory, when
> >   * driver is unloaded or when ballooning fails.
> >   */
> > -void intel_vgt_deballoon(void)
> > +void intel_vgt_deballoon(struct drm_i915_private *dev_priv)
> >  {
> >  	int i;
> >
> > +	if (!intel_vgpu_active(dev_priv))
> > +		return;
> > +
> >  	DRM_DEBUG("VGT deballoon.\n");
> >
> >  	for (i = 0; i < 4; i++) {
> > @@ -177,9 +180,8 @@ static int vgt_balloon_space(struct drm_mm *mm,
> >   * Returns:
> >   * zero on success, non-zero if configuration invalid or ballooning
> > failed
> >   */
> > -int intel_vgt_balloon(struct drm_device *dev)
> > +int intel_vgt_balloon(struct drm_i915_private *dev_priv)
> >  {
> > -	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	struct i915_ggtt *ggtt = &dev_priv->ggtt;
> >  	unsigned long ggtt_end = ggtt->base.start + ggtt->base.total;
> >
> > @@ -187,6 +189,9 @@ int intel_vgt_balloon(struct drm_device *dev)
> >  	unsigned long unmappable_base, unmappable_size, unmappable_end;
> >  	int ret;
> >
> > +	if (!intel_vgpu_active(dev_priv))
> > +		return 0;
> > +
> >  	mappable_base = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.base));
> >  	mappable_size = I915_READ(vgtif_reg(avail_rs.mappable_gmadr.size));
> >  	unmappable_base =
> > I915_READ(vgtif_reg(avail_rs.nonmappable_gmadr.base));
> > @@ -258,6 +263,6 @@ int intel_vgt_balloon(struct drm_device *dev)
> >
> >  err:
> >  	DRM_ERROR("VGT balloon fail\n");
> > -	intel_vgt_deballoon();
> > +	intel_vgt_deballoon(dev_priv);
> >  	return ret;
> >  }
> > diff --git a/drivers/gpu/drm/i915/i915_vgpu.h
> > b/drivers/gpu/drm/i915/i915_vgpu.h
> > index 07e67d5..f8917c6 100644
> > --- a/drivers/gpu/drm/i915/i915_vgpu.h
> > +++ b/drivers/gpu/drm/i915/i915_vgpu.h
> > @@ -27,7 +27,7 @@
> >  #include "i915_pvinfo.h"
> >
> >  extern void i915_check_vgpu(struct drm_i915_private *dev_priv);
> > -extern int intel_vgt_balloon(struct drm_device *dev); -extern void
> > intel_vgt_deballoon(void);
> > +extern int intel_vgt_balloon(struct drm_i915_private *dev_priv);
> > +extern void intel_vgt_deballoon(struct drm_i915_private *dev_priv);
> >
> >  #endif /* _I915_VGPU_H_ */
> --
> 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 04/11] drm/i915: Add teardown path in intel_vgt_ballon()
  2016-06-07 15:18 ` [PATCH v7 04/11] drm/i915: Add teardown path in intel_vgt_ballon() Zhi Wang
@ 2016-06-08  8:12   ` Joonas Lahtinen
  0 siblings, 0 replies; 30+ messages in thread
From: Joonas Lahtinen @ 2016-06-08  8:12 UTC (permalink / raw)
  To: Zhi Wang, chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

Patch title s/ballon/balloon/.

On ti, 2016-06-07 at 11:18 -0400, Zhi Wang wrote:
> This function needs to be changed to have a proper goto teardown path.
> Destructors/fini functions are only expected to be called after a
> successful initialization, so calling it at random phase in init function
> is bad. (Joonas)
> 

Cc: and Suggested-by: help to track where the patch appeared from.

> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_vgpu.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c
> index c9800d4..d41a29e 100644
> --- a/drivers/gpu/drm/i915/i915_vgpu.c
> +++ b/drivers/gpu/drm/i915/i915_vgpu.c
> @@ -187,7 +187,7 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
>  
>  	unsigned long mappable_base, mappable_size, mappable_end;
>  	unsigned long unmappable_base, unmappable_size, unmappable_end;
> -	int ret;
> +	int ret, i;
>  
>  	if (!intel_vgpu_active(dev_priv))
>  		return 0;
> @@ -263,6 +263,9 @@ int intel_vgt_balloon(struct drm_i915_private *dev_priv)
>  
>  err:
>  	DRM_ERROR("VGT balloon fail\n");
> -	intel_vgt_deballoon(dev_priv);
> +	for (i = 0; i < ARRAY_SIZE(bl_info.space); i++) {
> +		if (bl_info.space[i].allocated)
> +			drm_mm_remove_node(&bl_info.space[i]);
> +	}

Maybe add a counterpart vgt_deballoon_space for vgt_balloon_space and
use it in intel_vgt_deballoon and here. That way the code will be more
readable.

Regards, Joonas

>  	return ret;
>  }
-- 
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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 08/11] drm/i915: Make addressing mode bits in context descriptor configurable
  2016-06-08  7:12   ` Chris Wilson
@ 2016-06-08  8:17     ` Wang, Zhi A
  0 siblings, 0 replies; 30+ messages in thread
From: Wang, Zhi A @ 2016-06-08  8:17 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx, Lv, Zhiyuan

Good idea. :)

> -----Original Message-----
> From: Chris Wilson [mailto:chris@chris-wilson.co.uk]
> Sent: Wednesday, June 08, 2016 10:13 AM
> To: Wang, Zhi A <zhi.a.wang@intel.com>
> Cc: Lv, Zhiyuan <zhiyuan.lv@intel.com>; Tian, Kevin <kevin.tian@intel.com>;
> tvrtko.ursulin@linux.intel.com; intel-gfx@lists.freedesktop.org
> Subject: Re: [PATCH v7 08/11] drm/i915: Make addressing mode bits in context
> descriptor configurable
> 
> On Tue, Jun 07, 2016 at 11:18:44AM -0400, Zhi Wang wrote:
> > Currently the addressing mode bit in context descriptor is statically
> > generated from the configuration of system-wide PPGTT usage model.
> >
> > GVT-g will load the PPGTT shadow page table by itself and probably one
> > guest is using a different addressing mode with i915 host. The
> > addressing mode bits of a LRC context should be configurable under this case.
> >
> > v7
> >
> > - Move context addressing mode bit into i915_reg.h. (Joonas/Chris)
> > - Add prefix "INTEL_" for related definitions. (Joonas)
> >
> > v6:
> >
> > - Directly save the addressing mode bits inside i915_gem_context.
> > (Chris)
> > - Move the LRC context addressing mode bits into intel_lrc.h. (Chris)
> >
> > v5:
> >
> > - Change USES_FULL_48BIT(dev) to USES_FULL_48BIT(dev_priv) (Tvrtko)
> >
> > Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h         |  1 +
> >  drivers/gpu/drm/i915/i915_gem_context.c |  2 ++
> >  drivers/gpu/drm/i915/i915_reg.h         | 12 ++++++++++++
> >  drivers/gpu/drm/i915/intel_lrc.c        | 13 +------------
> >  4 files changed, 16 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> > b/drivers/gpu/drm/i915/i915_drv.h index 50b3ea7..a3ef3eb 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -879,6 +879,7 @@ struct i915_gem_context {
> >  		bool initialised;
> >  	} engine[I915_NUM_ENGINES];
> >  	u32 lrc_ring_buffer_size;
> > +	u32 lrc_addressing_mode_bits;
> 
> u32 addressing_mode.
> 
> Though this is better as ctx->desc_template; -Chris
> 
> --
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 01/11] drm/i915: Factor out i915_pvinfo.h
  2016-06-08  7:55   ` Joonas Lahtinen
@ 2016-06-08  8:20     ` Wang, Zhi A
  0 siblings, 0 replies; 30+ messages in thread
From: Wang, Zhi A @ 2016-06-08  8:20 UTC (permalink / raw)
  To: Joonas Lahtinen, chris, Lv, Zhiyuan, Tian, Kevin, tvrtko.ursulin,
	intel-gfx

Sure. Moving code in one patch, "offsetof()" guy will be in another patch

> -----Original Message-----
> From: Joonas Lahtinen [mailto:joonas.lahtinen@linux.intel.com]
> Sent: Wednesday, June 08, 2016 10:55 AM
> To: Wang, Zhi A <zhi.a.wang@intel.com>; chris@chris-wilson.co.uk; Lv, Zhiyuan
> <zhiyuan.lv@intel.com>; Tian, Kevin <kevin.tian@intel.com>;
> tvrtko.ursulin@linux.intel.com; intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v7 01/11] drm/i915: Factor out i915_pvinfo.h
> 
> On ti, 2016-06-07 at 11:18 -0400, Zhi Wang wrote:
> > As the PVINFO page definition is used by both GVT-g guest (vGPU) and
> > GVT-g host (GVT-g kernel device model), factor it out for better code
> structure.
> >
> > v7:
> > - Split the "offsetof" modification into a dedicated patch. (Joonas)
> >
> > v3:
> > - Use offsetof to calculate the member offset of PVINFO structure
> > (Joonas)
> >
> > Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> 
> Assuming this is just code motion patch per description;
> 
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> 
> > ---
> >  drivers/gpu/drm/i915/i915_pvinfo.h | 113
> > +++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/i915_vgpu.h   |  86 +---------------------------
> >  2 files changed, 114 insertions(+), 85 deletions(-)
> >  create mode 100644 drivers/gpu/drm/i915/i915_pvinfo.h
> >
> > diff --git a/drivers/gpu/drm/i915/i915_pvinfo.h
> > b/drivers/gpu/drm/i915/i915_pvinfo.h
> > new file mode 100644
> > index 0000000..68bdf60
> > --- /dev/null
> > +++ b/drivers/gpu/drm/i915/i915_pvinfo.h
> > @@ -0,0 +1,113 @@
> > +/*
> > + * Copyright(c) 2011-2016 Intel Corporation. All rights reserved.
> > + *
> > + * Permission is hereby granted, free of charge, to any person
> > +obtaining a
> > + * copy of this software and associated documentation files (the
> > +"Software"),
> > + * to deal in the Software without restriction, including without
> > +limitation
> > + * the rights to use, copy, modify, merge, publish, distribute,
> > +sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom
> > +the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including
> > +the next
> > + * paragraph) shall be included in all copies or substantial portions
> > +of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY
> KIND,
> > +EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> > +MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO
> EVENT
> > +SHALL
> > + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM,
> DAMAGES
> > +OR OTHER
> > + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
> OTHERWISE,
> > +ARISING FROM,
> > + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> OTHER
> > +DEALINGS IN THE
> > + * SOFTWARE.
> > + */
> > +
> > +#ifndef _I915_PVINFO_H_
> > +#define _I915_PVINFO_H_
> > +
> > +/* The MMIO offset of the shared info between guest and host emulator */
> > +#define VGT_PVINFO_PAGE	0x78000
> > +#define VGT_PVINFO_SIZE	0x1000
> > +
> > +/*
> > + * The following structure pages are defined in GEN MMIO space
> > + * for virtualization. (One page for now)  */
> > +#define VGT_MAGIC         0x4776544776544776ULL	/* 'vGTvGTvG'
> */
> > +#define VGT_VERSION_MAJOR 1
> > +#define VGT_VERSION_MINOR 0
> > +
> > +#define INTEL_VGT_IF_VERSION_ENCODE(major, minor) ((major) << 16 |
> > +(minor)) #define INTEL_VGT_IF_VERSION \
> > +	INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR,
> VGT_VERSION_MINOR)
> > +
> > +/*
> > + * notifications from guest to vgpu device model  */ enum
> > +vgt_g2v_type {
> > +	VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE = 2,
> > +	VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY,
> > +	VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE,
> > +	VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
> > +	VGT_G2V_EXECLIST_CONTEXT_CREATE,
> > +	VGT_G2V_EXECLIST_CONTEXT_DESTROY,
> > +	VGT_G2V_MAX,
> > +};
> > +
> > +struct vgt_if {
> > +	uint64_t magic;		/* VGT_MAGIC */
> > +	uint16_t version_major;
> > +	uint16_t version_minor;
> > +	uint32_t vgt_id;	/* ID of vGT instance */
> > +	uint32_t rsv1[12];	/* pad to offset 0x40 */
> > +	/*
> > +	 *  Data structure to describe the balooning info of resources.
> > +	 *  Each VM can only have one portion of continuous area for now.
> > +	 *  (May support scattered resource in future)
> > +	 *  (starting from offset 0x40)
> > +	 */
> > +	struct {
> > +		/* Aperture register balooning */
> > +		struct {
> > +			uint32_t base;
> > +			uint32_t size;
> > +		} mappable_gmadr;	/* aperture */
> > +		/* GMADR register balooning */
> > +		struct {
> > +			uint32_t base;
> > +			uint32_t size;
> > +		} nonmappable_gmadr;	/* non aperture */
> > +		/* allowed fence registers */
> > +		uint32_t fence_num;
> > +		uint32_t rsv2[3];
> > +	} avail_rs;		/* available/assigned resource */
> > +	uint32_t rsv3[0x200 - 24];	/* pad to half page */
> > +	/*
> > +	 * The bottom half page is for response from Gfx driver to hypervisor.
> > +	 */
> > +	uint32_t rsv4;
> > +	uint32_t display_ready;	/* ready for display owner switch */
> > +
> > +	uint32_t rsv5[4];
> > +
> > +	uint32_t g2v_notify;
> > +	uint32_t rsv6[7];
> > +
> > +	struct {
> > +		uint32_t lo;
> > +		uint32_t hi;
> > +	} pdp[4];
> > +
> > +	uint32_t execlist_context_descriptor_lo;
> > +	uint32_t execlist_context_descriptor_hi;
> > +
> > +	uint32_t  rsv7[0x200 - 24];    /* pad to one page */ } __packed;
> > +
> > +#define vgtif_reg(x) \
> > +	_MMIO((VGT_PVINFO_PAGE + (long)&((struct vgt_if *)NULL)->x))
> > +
> > +/* vGPU display status to be used by the host side */ #define
> > +VGT_DRV_DISPLAY_NOT_READY 0 #define
> VGT_DRV_DISPLAY_READY     1  /*
> > +ready for display switch */
> > +
> > +#endif /* _I915_PVINFO_H_ */
> > diff --git a/drivers/gpu/drm/i915/i915_vgpu.h
> > b/drivers/gpu/drm/i915/i915_vgpu.h
> > index 21ffcfe..07e67d5 100644
> > --- a/drivers/gpu/drm/i915/i915_vgpu.h
> > +++ b/drivers/gpu/drm/i915/i915_vgpu.h
> > @@ -24,91 +24,7 @@
> >  #ifndef _I915_VGPU_H_
> >  #define _I915_VGPU_H_
> >
> > -/* The MMIO offset of the shared info between guest and host emulator */
> > -#define VGT_PVINFO_PAGE	0x78000
> > -#define VGT_PVINFO_SIZE	0x1000
> > -
> > -/*
> > - * The following structure pages are defined in GEN MMIO space
> > - * for virtualization. (One page for now)
> > - */
> > -#define VGT_MAGIC         0x4776544776544776ULL	/* 'vGTvGTvG'
> */
> > -#define VGT_VERSION_MAJOR 1
> > -#define VGT_VERSION_MINOR 0
> > -
> > -#define INTEL_VGT_IF_VERSION_ENCODE(major, minor) ((major) << 16 |
> > (minor)) -#define INTEL_VGT_IF_VERSION \
> > -	INTEL_VGT_IF_VERSION_ENCODE(VGT_VERSION_MAJOR,
> VGT_VERSION_MINOR)
> > -
> > -/*
> > - * notifications from guest to vgpu device model
> > - */
> > -enum vgt_g2v_type {
> > -	VGT_G2V_PPGTT_L3_PAGE_TABLE_CREATE = 2,
> > -	VGT_G2V_PPGTT_L3_PAGE_TABLE_DESTROY,
> > -	VGT_G2V_PPGTT_L4_PAGE_TABLE_CREATE,
> > -	VGT_G2V_PPGTT_L4_PAGE_TABLE_DESTROY,
> > -	VGT_G2V_EXECLIST_CONTEXT_CREATE,
> > -	VGT_G2V_EXECLIST_CONTEXT_DESTROY,
> > -	VGT_G2V_MAX,
> > -};
> > -
> > -struct vgt_if {
> > -	uint64_t magic;		/* VGT_MAGIC */
> > -	uint16_t version_major;
> > -	uint16_t version_minor;
> > -	uint32_t vgt_id;	/* ID of vGT instance */
> > -	uint32_t rsv1[12];	/* pad to offset 0x40 */
> > -	/*
> > -	 *  Data structure to describe the balooning info of resources.
> > -	 *  Each VM can only have one portion of continuous area for now.
> > -	 *  (May support scattered resource in future)
> > -	 *  (starting from offset 0x40)
> > -	 */
> > -	struct {
> > -		/* Aperture register balooning */
> > -		struct {
> > -			uint32_t base;
> > -			uint32_t size;
> > -		} mappable_gmadr;	/* aperture */
> > -		/* GMADR register balooning */
> > -		struct {
> > -			uint32_t base;
> > -			uint32_t size;
> > -		} nonmappable_gmadr;	/* non aperture */
> > -		/* allowed fence registers */
> > -		uint32_t fence_num;
> > -		uint32_t rsv2[3];
> > -	} avail_rs;		/* available/assigned resource */
> > -	uint32_t rsv3[0x200 - 24];	/* pad to half page */
> > -	/*
> > -	 * The bottom half page is for response from Gfx driver to hypervisor.
> > -	 */
> > -	uint32_t rsv4;
> > -	uint32_t display_ready;	/* ready for display owner switch */
> > -
> > -	uint32_t rsv5[4];
> > -
> > -	uint32_t g2v_notify;
> > -	uint32_t rsv6[7];
> > -
> > -	struct {
> > -		uint32_t lo;
> > -		uint32_t hi;
> > -	} pdp[4];
> > -
> > -	uint32_t execlist_context_descriptor_lo;
> > -	uint32_t execlist_context_descriptor_hi;
> > -
> > -	uint32_t  rsv7[0x200 - 24];    /* pad to one page */
> > -} __packed;
> > -
> > -#define vgtif_reg(x) \
> > -	_MMIO((VGT_PVINFO_PAGE + (long)&((struct vgt_if *)NULL)->x))
> > -
> > -/* vGPU display status to be used by the host side */ -#define
> > VGT_DRV_DISPLAY_NOT_READY 0 -#define
> VGT_DRV_DISPLAY_READY     1  /*
> > ready for display switch */
> > +#include "i915_pvinfo.h"
> >
> >  extern void i915_check_vgpu(struct drm_i915_private *dev_priv);
> >  extern int intel_vgt_balloon(struct drm_device *dev);
> --
> 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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 05/11] drm/i915: gvt: Introduce the basic architecture of GVT-g
  2016-06-07 15:18 ` [PATCH v7 05/11] drm/i915: gvt: Introduce the basic architecture of GVT-g Zhi Wang
@ 2016-06-08  8:28   ` Joonas Lahtinen
  0 siblings, 0 replies; 30+ messages in thread
From: Joonas Lahtinen @ 2016-06-08  8:28 UTC (permalink / raw)
  To: Zhi Wang, chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

On ti, 2016-06-07 at 11:18 -0400, Zhi Wang wrote:
> This patch introduces the very basic framework of GVT-g device model,
> includes basic prototypes, definitions, initialization.
> 
> v7:
> - Refine the URL link in Kconfig. (Joonas)
> - Refine the introduction of GVT-g host support in Kconfig. (Joonas)
> - Remove the macro GVT_ALIGN(), use round_down() instead. (Joonas)
> - Make "struct intel_gvt" a data member in struct drm_i915_private.(Joonas)
> 	- Remove {alloc, free}_gvt_device()
> 	- Rename intel_gvt_{create, destroy}_gvt_device()
> 	- Expost intel_gvt_init_host()
> - Remove the dummy "struct intel_gvt" declaration in intel_gvt.h (Joonas)
> 
> v6:
> - Refine introduction in Kconfig. (Chris)
> - The exposed API functions will take struct intel_gvt * instead of
> void *. (Chris/Tvrtko)
> - Remove most memebers of strct intel_gvt_device_info. Will add them
> in the device model patches.(Chris)
> - Remove gvt_info() and gvt_err() in debug.h. (Chris)
> - Move GVT kernel parameter into i915_params. (Chris)
> - Remove include/drm/i915_gvt.h, as GVT-g will be built within i915.
> - Remove the redundant struct i915_gvt *, as the functions in i915
> will directly take struct intel_gvt *.
> - Add more comments for reviewer.
> 
> v5:
> Take Tvrtko's comments:
> - Fix the misspelled words in Kconfig
> - Let functions take drm_i915_private * instead of struct drm_device *
> - Remove redundant prints/local varible initialization
> 
> v3:
> Take Joonas' comments:
> - Change file name i915_gvt.* to intel_gvt.*
> - Move GVT kernel parameter into intel_gvt.c
> - Remove redundant debug macros
> - Change error handling style
> - Add introductions for some stub functions
> - Introduce drm/i915_gvt.h.
> 
> Take Kevin's comments:
> - Move GVT-g host/guest check into intel_vgt_balloon in i915_gem_gtt.c
> 
> v2:
> - Introduce i915_gvt.c.
> It's necessary to introduce the stubs between i915 driver and GVT-g host,
> as GVT-g components is configurable in kernel config. When disabled, the
> stubs here do nothing.
> 
> Take Joonas' comments:
> - Replace boolean return value with int.
> - Replace customized info/warn/debug macros with DRM macros.
> - Document all non-static functions like i915.
> - Remove empty and unused functions.
> - Replace magic number with marcos.
> - Set GVT-g in kernel config to "n" by default.
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>

Again, add Cc: everyone who has given comments.

> ---
>  drivers/gpu/drm/i915/Kconfig         |  22 +++++
>  drivers/gpu/drm/i915/Makefile        |   5 ++
>  drivers/gpu/drm/i915/gvt/Makefile    |   5 ++
>  drivers/gpu/drm/i915/gvt/debug.h     |  34 ++++++++
>  drivers/gpu/drm/i915/gvt/gvt.c       | 158 +++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/gvt/gvt.h       |  72 ++++++++++++++++
>  drivers/gpu/drm/i915/gvt/hypercall.h |  38 +++++++++
>  drivers/gpu/drm/i915/gvt/mpt.h       |  49 +++++++++++
>  drivers/gpu/drm/i915/i915_dma.c      |  16 +++-
>  drivers/gpu/drm/i915/i915_drv.h      |  10 +++
>  drivers/gpu/drm/i915/i915_params.c   |   5 ++
>  drivers/gpu/drm/i915/i915_params.h   |   1 +
>  drivers/gpu/drm/i915/intel_gvt.c     | 100 ++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_gvt.h     |  45 ++++++++++
>  14 files changed, 556 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/gpu/drm/i915/gvt/Makefile
>  create mode 100644 drivers/gpu/drm/i915/gvt/debug.h
>  create mode 100644 drivers/gpu/drm/i915/gvt/gvt.c
>  create mode 100644 drivers/gpu/drm/i915/gvt/gvt.h
>  create mode 100644 drivers/gpu/drm/i915/gvt/hypercall.h
>  create mode 100644 drivers/gpu/drm/i915/gvt/mpt.h
>  create mode 100644 drivers/gpu/drm/i915/intel_gvt.c
>  create mode 100644 drivers/gpu/drm/i915/intel_gvt.h
> 
> +/**
> + * intel_gvt_clean_device - clean a GVT device
> + * @gvt: intel gvt device
> + *
> + * This function is called at the driver unloading stage, to free the
> + * resources owned by a GVT device.
> + *
> + */
> +void intel_gvt_clean_device(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_gvt *gvt = &dev_priv->gvt;
> +
> +	if (WARN_ON(!gvt->initialized))
> +		return;
> +
> +	mutex_lock(&intel_gvt_host.gvt_idr_lock);

Why is this lock necessary? I assume intel_gvt_init will be called on
driver load (same for fini part), and there should be no races.

With that explained or removed;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> +	idr_remove(&intel_gvt_host.gvt_idr, gvt->id);
> +	mutex_unlock(&intel_gvt_host.gvt_idr_lock);
> +	/* Other de-initialization of GVT components will be introduced. */
> +
> +	gvt->initialized = false;
> +}
> +
-- 
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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 07/11] drm/i915: Make ring buffer size of a LRC context configurable
  2016-06-08  7:08   ` Chris Wilson
@ 2016-06-08  8:34     ` Joonas Lahtinen
  0 siblings, 0 replies; 30+ messages in thread
From: Joonas Lahtinen @ 2016-06-08  8:34 UTC (permalink / raw)
  To: Chris Wilson, Zhi Wang; +Cc: intel-gfx, zhiyuan.lv

On ke, 2016-06-08 at 08:08 +0100, Chris Wilson wrote:
> On Tue, Jun 07, 2016 at 11:18:43AM -0400, Zhi Wang wrote:
> > 
> > This patch introduces an option for configuring the ring buffer size
> > of a LRC context after the context creation.
> > 
> > Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h         | 1 +
> >  drivers/gpu/drm/i915/i915_gem_context.c | 1 +
> >  drivers/gpu/drm/i915/intel_lrc.c        | 3 ++-
> >  3 files changed, 4 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 33147b1..50b3ea7 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -878,6 +878,7 @@ struct i915_gem_context {
> >  		int pin_count;
> >  		bool initialised;
> >  	} engine[I915_NUM_ENGINES];
> > +	u32 lrc_ring_buffer_size;
> u32 ring_size;
> 

My Reviewed-by: can be kept with that change.

Regards, Joonas

> There is no reason why we need to limit it to lrc other than immediate
> convenience.
> -Chris
> 
-- 
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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 08/11] drm/i915: Make addressing mode bits in context descriptor configurable
  2016-06-07 15:18 ` [PATCH v7 08/11] drm/i915: Make addressing mode bits in context descriptor configurable Zhi Wang
  2016-06-08  7:12   ` Chris Wilson
@ 2016-06-08  8:38   ` Joonas Lahtinen
  1 sibling, 0 replies; 30+ messages in thread
From: Joonas Lahtinen @ 2016-06-08  8:38 UTC (permalink / raw)
  To: Zhi Wang, chris, zhiyuan.lv, kevin.tian, tvrtko.ursulin, intel-gfx

On ti, 2016-06-07 at 11:18 -0400, Zhi Wang wrote:
> Currently the addressing mode bit in context descriptor is statically
> generated from the configuration of system-wide PPGTT usage model.
> 
> GVT-g will load the PPGTT shadow page table by itself and probably one
> guest is using a different addressing mode with i915 host. The addressing
> mode bits of a LRC context should be configurable under this case.
> 
> v7
> 
> - Move context addressing mode bit into i915_reg.h. (Joonas/Chris)
> - Add prefix "INTEL_" for related definitions. (Joonas)
> 
> v6:
> 
> - Directly save the addressing mode bits inside i915_gem_context. (Chris)
> - Move the LRC context addressing mode bits into intel_lrc.h. (Chris)
> 
> v5:
> 
> - Change USES_FULL_48BIT(dev) to USES_FULL_48BIT(dev_priv) (Tvrtko)
> 
> Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>

With Chris's suggestion of renaming to addressing_mode,

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  1 +
>  drivers/gpu/drm/i915/i915_gem_context.c |  2 ++
>  drivers/gpu/drm/i915/i915_reg.h         | 12 ++++++++++++
>  drivers/gpu/drm/i915/intel_lrc.c        | 13 +------------
>  4 files changed, 16 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 50b3ea7..a3ef3eb 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -879,6 +879,7 @@ struct i915_gem_context {
>  		bool initialised;
>  	} engine[I915_NUM_ENGINES];
>  	u32 lrc_ring_buffer_size;
> +	u32 lrc_addressing_mode_bits;
>  
>  	struct list_head link;
>  
> diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c
> index 1663981..d9d7779 100644
> --- a/drivers/gpu/drm/i915/i915_gem_context.c
> +++ b/drivers/gpu/drm/i915/i915_gem_context.c
> @@ -296,6 +296,8 @@ __create_hw_context(struct drm_device *dev,
>  
>  	ctx->hang_stats.ban_period_seconds = DRM_I915_CTX_BAN_PERIOD;
>  	ctx->lrc_ring_buffer_size = 4 * PAGE_SIZE;
> +	ctx->lrc_addressing_mode_bits = GEN8_CTX_ADDRESSING_MODE(dev_priv) <<
> +		GEN8_CTX_ADDRESSING_MODE_SHIFT;
>  
>  	return ctx;
>  
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index bfebfc9..ab5970c 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -3022,6 +3022,18 @@ enum skl_disp_power_wells {
>  /* Same as Haswell, but 72064 bytes now. */
>  #define GEN8_CXT_TOTAL_SIZE		(18 * PAGE_SIZE)
>  
> +enum {
> +	INTEL_ADVANCED_CONTEXT = 0,
> +	INTEL_LEGACY_32B_CONTEXT,
> +	INTEL_ADVANCED_AD_CONTEXT,
> +	INTEL_LEGACY_64B_CONTEXT
> +};
> +
> +#define GEN8_CTX_ADDRESSING_MODE_SHIFT 3
> +#define GEN8_CTX_ADDRESSING_MODE(dev_priv) (USES_FULL_48BIT_PPGTT(dev_priv) ?\
> +		INTEL_LEGACY_64B_CONTEXT : \
> +		INTEL_LEGACY_32B_CONTEXT)
> +
>  #define CHV_CLK_CTL1			_MMIO(0x101100)
>  #define VLV_CLK_CTL2			_MMIO(0x101104)
>  #define   CLK_CTL2_CZCOUNT_30NS_SHIFT	28
> diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> index 72a0cca..ffb436c 100644
> --- a/drivers/gpu/drm/i915/intel_lrc.c
> +++ b/drivers/gpu/drm/i915/intel_lrc.c
> @@ -208,16 +208,6 @@
>  } while (0)
>  
>  enum {
> -	ADVANCED_CONTEXT = 0,
> -	LEGACY_32B_CONTEXT,
> -	ADVANCED_AD_CONTEXT,
> -	LEGACY_64B_CONTEXT
> -};
> -#define GEN8_CTX_ADDRESSING_MODE_SHIFT 3
> -#define GEN8_CTX_ADDRESSING_MODE(dev)  (USES_FULL_48BIT_PPGTT(dev) ?\
> -		LEGACY_64B_CONTEXT :\
> -		LEGACY_32B_CONTEXT)
> -enum {
>  	FAULT_AND_HANG = 0,
>  	FAULT_AND_HALT, /* Debug only */
>  	FAULT_AND_STREAM,
> @@ -281,8 +271,6 @@ logical_ring_init_platform_invariants(struct intel_engine_cs *engine)
>  					(engine->id == VCS || engine->id == VCS2);
>  
>  	engine->ctx_desc_template = GEN8_CTX_VALID;
> -	engine->ctx_desc_template |= GEN8_CTX_ADDRESSING_MODE(dev_priv) <<
> -				   GEN8_CTX_ADDRESSING_MODE_SHIFT;
>  	if (IS_GEN8(dev_priv))
>  		engine->ctx_desc_template |= GEN8_CTX_L3LLC_COHERENT;
>  	engine->ctx_desc_template |= GEN8_CTX_PRIVILEGE;
> @@ -326,6 +314,7 @@ intel_lr_context_descriptor_update(struct i915_gem_context *ctx,
>  	BUILD_BUG_ON(MAX_CONTEXT_HW_ID > (1<
>  
>  	desc = engine->ctx_desc_template;			/* bits  0-11 */
> +	desc |= ctx->lrc_addressing_mode_bits;			/* bits  3-4  */
>  	desc |= ce->lrc_vma->node.start + LRC_PPHWSP_PN * PAGE_SIZE;
>  								/* bits 12-31 */
>  	desc |= (u64)ctx->hw_id << GEN8_CTX_ID_SHIFT;		/* bits 32-52 */
-- 
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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 09/11] drm/i915: Introduce execlist context status change notification
  2016-06-07 22:01   ` Chris Wilson
@ 2016-06-08  8:40     ` Joonas Lahtinen
  0 siblings, 0 replies; 30+ messages in thread
From: Joonas Lahtinen @ 2016-06-08  8:40 UTC (permalink / raw)
  To: Chris Wilson, Zhi Wang; +Cc: intel-gfx, zhiyuan.lv

On ti, 2016-06-07 at 23:01 +0100, Chris Wilson wrote:
> On Tue, Jun 07, 2016 at 11:18:45AM -0400, Zhi Wang wrote:
> > 
> > This patch introduces an approach to track the execlist context status
> > change.
> > 
> > GVT-g uses GVT context as the "shadow context". The content inside GVT
> > context will be copied back to guest after the context is idle. And GVT-g
> > has to know the status of the execlist context.
> > 
> > This function is configurable when creating a new GEM context. Currently,
> > Only GVT-g will create the "status-change-notification" enabled GEM
> > context.
> > 
> > v7:
> > 
> > - Remove per-engine ctx status notifiers. Use one status notifier for all
> > engines. (Joonas)
> > - Add prefix "INTEL_" for related definitions. (Joonas)
> > - Refine the comments in execlists_context_status_change(). (Joonas)
> > 
> > v6:
> > 
> > - When !CONFIG_DRM_I915_GVT, make GVT code as dead code then compiler
> > could automatically eliminate them for us. (Chris)
> > - Always initialize the notifier header, so it could be switched on/off
> > at runtime. (Chris)
> > 
> > v5:
> > 
> > - Only compile this feature when CONFIG_DRM_I915_GVT is enabled.(Tvrtko)
> > 
> > Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> > ---
> > +static inline void execlists_context_status_change(
> > +		struct drm_i915_gem_request *rq,
> > +		unsigned long status)
> > +{
> > +	/*
> > +	 * Only used when GVT-g is enabled now. When GVT-g is disabled,
> > +	 * The compiler should eliminate this function as dead-code.
> > +	 */
> > +	if (!IS_ENABLED(CONFIG_DRM_I915_GVT))
> > +		return;
> > +
> > +	if (!rq->ctx->enable_lrc_status_change_notification)
> > +		return;
> Was there any other justification for the boolean? (i.e. does it get
> used elsewhere) I thought we mentioned this as probably premature
> optimisation and should favour speeding up a no-op call_chain() if
> required. So can we have callbacks in the notifier but need to disable
> notification? If so, that is never explained.
> 

By my original comments, after removing this boolean, you can add my;

Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>

> > 
> > +
> > +	atomic_notifier_call_chain(&rq->ctx->status_notifier, status, rq);
> > +}
-- 
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

^ permalink raw reply	[flat|nested] 30+ messages in thread

* Re: [PATCH v7 10/11] drm/i915: Support LRC context single submission
  2016-06-08  7:04   ` Chris Wilson
@ 2016-06-08  8:44     ` Joonas Lahtinen
  0 siblings, 0 replies; 30+ messages in thread
From: Joonas Lahtinen @ 2016-06-08  8:44 UTC (permalink / raw)
  To: Chris Wilson, Zhi Wang; +Cc: intel-gfx, zhiyuan.lv

On ke, 2016-06-08 at 08:04 +0100, Chris Wilson wrote:
> On Tue, Jun 07, 2016 at 11:18:46AM -0400, Zhi Wang wrote:
> > 
> > This patch introduces the support of LRC context single submission.
> > As GVT context may come from different guests, which require different
> > configuration of render registers. It can't be combined into a dual ELSP
> > submission combo.
> > 
> > Only GVT-g will create this kinds of GEM context currently.
> > 
> > v7:
> > 
> > - Fix typos in commit message. (Joonas)
> > 
> > v6:
> > - Make GVT code as dead code when !CONFIG_DRM_I915_GVT. (Chris)
> > 
> > v5:
> > 
> > - Only compile this feature when CONFIG_DRM_I915_GVT=y. (Tvrtko)
> > 
> > Signed-off-by: Zhi Wang <zhi.a.wang@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h  |  1 +
> >  drivers/gpu/drm/i915/intel_lrc.c | 15 +++++++++++++++
> >  2 files changed, 16 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 4ab4cf7..8f8a5bc 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -882,6 +882,7 @@ struct i915_gem_context {
> >  	u32 lrc_addressing_mode_bits;
> >  	struct atomic_notifier_head status_notifier;
> >  	bool enable_lrc_status_change_notification;
> > +	bool enable_lrc_single_submission;
> >  
> >  	struct list_head link;
> >  
> > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c
> > index 956585d..e3cab06 100644
> > --- a/drivers/gpu/drm/i915/intel_lrc.c
> > +++ b/drivers/gpu/drm/i915/intel_lrc.c
> > @@ -447,6 +447,21 @@ static void execlists_context_unqueue(struct intel_engine_cs *engine)
> >  			i915_gem_request_unreference(req0);
> >  			req0 = cursor;
> >  		} else {
> > +			/* Compiler will do the dead-code elimination */
> > +			if (IS_ENABLED(CONFIG_DRM_I915_GVT)) {
> > +				/*
> > +				 * req0 (after merged) ctx requires single
> > +				 * submission, stop picking
> > +				 */
> > +				if (req0->ctx->enable_lrc_single_submission)
> enable_ is a bad verb here, force_ would be more apt.
> 
> s/lrc/execlists/
> 
> ctx->execlists_force_single_submission
> -Chris

Also CCing Kevin here, who commented on the previous patch. I still
think it would be good to do the req0->{vgpu,vmid} != cursor-
>{vgpu,vmid} test. That way, we get better results if there's one
dominating VM submitting workloads, right?

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

^ permalink raw reply	[flat|nested] 30+ messages in thread

end of thread, other threads:[~2016-06-08  8:44 UTC | newest]

Thread overview: 30+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-06-07 15:18 [PATCH v7 00/11] Introduce the implementation of GVT context Zhi Wang
2016-06-07 15:18 ` [PATCH v7 01/11] drm/i915: Factor out i915_pvinfo.h Zhi Wang
2016-06-08  7:55   ` Joonas Lahtinen
2016-06-08  8:20     ` Wang, Zhi A
2016-06-07 15:18 ` [PATCH v7 02/11] drm/i915: Use offsetof() to calculate the offset of members in PVINFO page Zhi Wang
2016-06-08  7:57   ` Joonas Lahtinen
2016-06-07 15:18 ` [PATCH v7 03/11] drm/i915: Fold vGPU active check into inner functions Zhi Wang
2016-06-08  8:04   ` Joonas Lahtinen
2016-06-08  8:06     ` Wang, Zhi A
2016-06-07 15:18 ` [PATCH v7 04/11] drm/i915: Add teardown path in intel_vgt_ballon() Zhi Wang
2016-06-08  8:12   ` Joonas Lahtinen
2016-06-07 15:18 ` [PATCH v7 05/11] drm/i915: gvt: Introduce the basic architecture of GVT-g Zhi Wang
2016-06-08  8:28   ` Joonas Lahtinen
2016-06-07 15:18 ` [PATCH v7 06/11] drm/i915: Introduce host graphics memory partition for GVT-g Zhi Wang
2016-06-07 15:18 ` [PATCH v7 07/11] drm/i915: Make ring buffer size of a LRC context configurable Zhi Wang
2016-06-08  7:08   ` Chris Wilson
2016-06-08  8:34     ` Joonas Lahtinen
2016-06-07 15:18 ` [PATCH v7 08/11] drm/i915: Make addressing mode bits in context descriptor configurable Zhi Wang
2016-06-08  7:12   ` Chris Wilson
2016-06-08  8:17     ` Wang, Zhi A
2016-06-08  8:38   ` Joonas Lahtinen
2016-06-07 15:18 ` [PATCH v7 09/11] drm/i915: Introduce execlist context status change notification Zhi Wang
2016-06-07 22:01   ` Chris Wilson
2016-06-08  8:40     ` Joonas Lahtinen
2016-06-07 15:18 ` [PATCH v7 10/11] drm/i915: Support LRC context single submission Zhi Wang
2016-06-08  7:04   ` Chris Wilson
2016-06-08  8:44     ` Joonas Lahtinen
2016-06-07 15:18 ` [PATCH v7 11/11] drm/i915: Introduce GVT context creation API Zhi Wang
2016-06-08  6:59   ` Chris Wilson
2016-06-07 15:53 ` ✓ Ro.CI.BAT: success for Introduce the implementation of GVT context (rev5) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.