public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915/gvt: clean up intel_gvt.h as interface for i915 core
@ 2016-10-20  7:29 Zhenyu Wang
  2016-10-20  7:29 ` [PATCH] Documentation/gpu: Add section for Intel GVT-g host support Zhenyu Wang
  2016-10-20  7:41 ` [PATCH] drm/i915/gvt: clean up intel_gvt.h as interface for i915 core Chris Wilson
  0 siblings, 2 replies; 3+ messages in thread
From: Zhenyu Wang @ 2016-10-20  7:29 UTC (permalink / raw)
  To: intel-gfx

i915 core should only call functions and structures exposed through
intel_gvt.h. Remove internal gvt.h and i915_pvinfo.h.

Change for internal intel_gvt structure as private handler which
not requires to expose gvt internal structure for i915 core.

Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 drivers/gpu/drm/i915/gvt/aperture_gm.c  |  1 +
 drivers/gpu/drm/i915/gvt/cfg_space.c    |  1 +
 drivers/gpu/drm/i915/gvt/cmd_parser.c   |  2 ++
 drivers/gpu/drm/i915/gvt/display.c      |  1 +
 drivers/gpu/drm/i915/gvt/edid.c         |  1 +
 drivers/gpu/drm/i915/gvt/execlist.c     |  1 +
 drivers/gpu/drm/i915/gvt/firmware.c     |  2 ++
 drivers/gpu/drm/i915/gvt/gtt.c          |  2 ++
 drivers/gpu/drm/i915/gvt/gvt.c          | 14 +++++++++++---
 drivers/gpu/drm/i915/gvt/gvt.h          |  2 ++
 drivers/gpu/drm/i915/gvt/handlers.c     |  2 ++
 drivers/gpu/drm/i915/gvt/interrupt.c    |  1 +
 drivers/gpu/drm/i915/gvt/mmio.c         |  1 +
 drivers/gpu/drm/i915/gvt/opregion.c     |  1 +
 drivers/gpu/drm/i915/gvt/render.c       |  1 +
 drivers/gpu/drm/i915/gvt/sched_policy.c |  1 +
 drivers/gpu/drm/i915/gvt/scheduler.c    |  5 +++--
 drivers/gpu/drm/i915/gvt/vgpu.c         |  2 ++
 drivers/gpu/drm/i915/i915_drv.h         |  4 ++--
 drivers/gpu/drm/i915/intel_gvt.c        |  1 +
 drivers/gpu/drm/i915/intel_gvt.h        |  3 ---
 21 files changed, 39 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/gvt/aperture_gm.c b/drivers/gpu/drm/i915/gvt/aperture_gm.c
index e0211f8..db503c1 100644
--- a/drivers/gpu/drm/i915/gvt/aperture_gm.c
+++ b/drivers/gpu/drm/i915/gvt/aperture_gm.c
@@ -35,6 +35,7 @@
  */
 
 #include "i915_drv.h"
+#include "gvt.h"
 
 #define MB_TO_BYTES(mb) ((mb) << 20ULL)
 #define BYTES_TO_MB(b) ((b) >> 20ULL)
diff --git a/drivers/gpu/drm/i915/gvt/cfg_space.c b/drivers/gpu/drm/i915/gvt/cfg_space.c
index 16360e4..4c68774 100644
--- a/drivers/gpu/drm/i915/gvt/cfg_space.c
+++ b/drivers/gpu/drm/i915/gvt/cfg_space.c
@@ -32,6 +32,7 @@
  */
 
 #include "i915_drv.h"
+#include "gvt.h"
 
 enum {
 	INTEL_GVT_PCI_BAR_GTTMMIO = 0,
diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
index 5808ee7..5b4658f 100644
--- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
+++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
@@ -36,6 +36,8 @@
 
 #include <linux/slab.h>
 #include "i915_drv.h"
+#include "gvt.h"
+#include "i915_pvinfo.h"
 #include "trace.h"
 
 #define INVALID_OP    (~0U)
diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
index 534000b..d8908d4 100644
--- a/drivers/gpu/drm/i915/gvt/display.c
+++ b/drivers/gpu/drm/i915/gvt/display.c
@@ -33,6 +33,7 @@
  */
 
 #include "i915_drv.h"
+#include "gvt.h"
 
 static int get_edp_pipe(struct intel_vgpu *vgpu)
 {
diff --git a/drivers/gpu/drm/i915/gvt/edid.c b/drivers/gpu/drm/i915/gvt/edid.c
index a07e427..7e1da1c 100644
--- a/drivers/gpu/drm/i915/gvt/edid.c
+++ b/drivers/gpu/drm/i915/gvt/edid.c
@@ -33,6 +33,7 @@
  */
 
 #include "i915_drv.h"
+#include "gvt.h"
 
 #define GMBUS1_TOTAL_BYTES_SHIFT 16
 #define GMBUS1_TOTAL_BYTES_MASK 0x1ff
diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c
index c50a3d1..b87b4f5 100644
--- a/drivers/gpu/drm/i915/gvt/execlist.c
+++ b/drivers/gpu/drm/i915/gvt/execlist.c
@@ -33,6 +33,7 @@
  */
 
 #include "i915_drv.h"
+#include "gvt.h"
 
 #define _EL_OFFSET_STATUS       0x234
 #define _EL_OFFSET_STATUS_BUF   0x370
diff --git a/drivers/gpu/drm/i915/gvt/firmware.c b/drivers/gpu/drm/i915/gvt/firmware.c
index 4578a4d..d068a52 100644
--- a/drivers/gpu/drm/i915/gvt/firmware.c
+++ b/drivers/gpu/drm/i915/gvt/firmware.c
@@ -32,6 +32,8 @@
 #include <linux/crc32.h>
 
 #include "i915_drv.h"
+#include "gvt.h"
+#include "i915_pvinfo.h"
 
 #define FIRMWARE_VERSION (0x0)
 
diff --git a/drivers/gpu/drm/i915/gvt/gtt.c b/drivers/gpu/drm/i915/gvt/gtt.c
index 29de179..0722d1e 100644
--- a/drivers/gpu/drm/i915/gvt/gtt.c
+++ b/drivers/gpu/drm/i915/gvt/gtt.c
@@ -34,6 +34,8 @@
  */
 
 #include "i915_drv.h"
+#include "gvt.h"
+#include "i915_pvinfo.h"
 #include "trace.h"
 
 static bool enable_out_of_sync = false;
diff --git a/drivers/gpu/drm/i915/gvt/gvt.c b/drivers/gpu/drm/i915/gvt/gvt.c
index e72e26c..be9ccc8 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.c
+++ b/drivers/gpu/drm/i915/gvt/gvt.c
@@ -35,6 +35,7 @@
 #include <linux/kthread.h>
 
 #include "i915_drv.h"
+#include "gvt.h"
 
 struct intel_gvt_host intel_gvt_host;
 
@@ -173,7 +174,7 @@ static int init_service_thread(struct intel_gvt *gvt)
  */
 void intel_gvt_clean_device(struct drm_i915_private *dev_priv)
 {
-	struct intel_gvt *gvt = &dev_priv->gvt;
+	struct intel_gvt *gvt = to_gvt(dev_priv);
 
 	if (WARN_ON(!gvt->initialized))
 		return;
@@ -204,7 +205,7 @@ void intel_gvt_clean_device(struct drm_i915_private *dev_priv)
  */
 int intel_gvt_init_device(struct drm_i915_private *dev_priv)
 {
-	struct intel_gvt *gvt = &dev_priv->gvt;
+	struct intel_gvt *gvt;
 	int ret;
 
 	/*
@@ -214,9 +215,15 @@ int intel_gvt_init_device(struct drm_i915_private *dev_priv)
 	if (WARN_ON(!intel_gvt_host.initialized))
 		return -EINVAL;
 
-	if (WARN_ON(gvt->initialized))
+	if (WARN_ON(dev_priv->gvt))
 		return -EEXIST;
 
+	dev_priv->gvt = kzalloc(sizeof(struct intel_gvt), GFP_KERNEL);
+	if (!dev_priv->gvt)
+		return -ENOMEM;
+
+	gvt = to_gvt(dev_priv);
+
 	gvt_dbg_core("init gvt device\n");
 
 	mutex_init(&gvt->lock);
@@ -280,5 +287,6 @@ out_free_firmware:
 	intel_gvt_free_firmware(gvt);
 out_clean_mmio_info:
 	intel_gvt_clean_mmio_info(gvt);
+	kfree(dev_priv->gvt);
 	return ret;
 }
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index 1564554..fd33e6b 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -213,6 +213,8 @@ struct intel_gvt {
 	unsigned long service_request;
 };
 
+#define to_gvt(dev_priv) (struct intel_gvt *)(dev_priv->gvt)
+
 enum {
 	INTEL_GVT_REQUEST_EMULATE_VBLANK = 0,
 };
diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
index e8ec403..b21115f 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -37,6 +37,8 @@
  */
 
 #include "i915_drv.h"
+#include "gvt.h"
+#include "i915_pvinfo.h"
 
 /* XXX FIXME i915 has changed PP_XXX definition */
 #define PCH_PP_STATUS  _MMIO(0xc7200)
diff --git a/drivers/gpu/drm/i915/gvt/interrupt.c b/drivers/gpu/drm/i915/gvt/interrupt.c
index 84d7174..e43ef72 100644
--- a/drivers/gpu/drm/i915/gvt/interrupt.c
+++ b/drivers/gpu/drm/i915/gvt/interrupt.c
@@ -30,6 +30,7 @@
  */
 
 #include "i915_drv.h"
+#include "gvt.h"
 
 /* common offset among interrupt control registers */
 #define regbase_to_isr(base)	(base)
diff --git a/drivers/gpu/drm/i915/gvt/mmio.c b/drivers/gpu/drm/i915/gvt/mmio.c
index ce3af95..585b01f 100644
--- a/drivers/gpu/drm/i915/gvt/mmio.c
+++ b/drivers/gpu/drm/i915/gvt/mmio.c
@@ -34,6 +34,7 @@
  */
 
 #include "i915_drv.h"
+#include "gvt.h"
 
 /**
  * intel_vgpu_gpa_to_mmio_offset - translate a GPA to MMIO offset
diff --git a/drivers/gpu/drm/i915/gvt/opregion.c b/drivers/gpu/drm/i915/gvt/opregion.c
index 46cc240..53ac81f 100644
--- a/drivers/gpu/drm/i915/gvt/opregion.c
+++ b/drivers/gpu/drm/i915/gvt/opregion.c
@@ -23,6 +23,7 @@
 
 #include <linux/acpi.h>
 #include "i915_drv.h"
+#include "gvt.h"
 
 static int init_vgpu_opregion(struct intel_vgpu *vgpu, u32 gpa)
 {
diff --git a/drivers/gpu/drm/i915/gvt/render.c b/drivers/gpu/drm/i915/gvt/render.c
index f54ab85..feebb65 100644
--- a/drivers/gpu/drm/i915/gvt/render.c
+++ b/drivers/gpu/drm/i915/gvt/render.c
@@ -34,6 +34,7 @@
  */
 
 #include "i915_drv.h"
+#include "gvt.h"
 
 struct render_mmio {
 	int ring_id;
diff --git a/drivers/gpu/drm/i915/gvt/sched_policy.c b/drivers/gpu/drm/i915/gvt/sched_policy.c
index c607354..278db0c 100644
--- a/drivers/gpu/drm/i915/gvt/sched_policy.c
+++ b/drivers/gpu/drm/i915/gvt/sched_policy.c
@@ -32,6 +32,7 @@
  */
 
 #include "i915_drv.h"
+#include "gvt.h"
 
 static bool vgpu_has_pending_workload(struct intel_vgpu *vgpu)
 {
diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
index b15cdf5..01d23ad 100644
--- a/drivers/gpu/drm/i915/gvt/scheduler.c
+++ b/drivers/gpu/drm/i915/gvt/scheduler.c
@@ -33,10 +33,11 @@
  *
  */
 
-#include "i915_drv.h"
-
 #include <linux/kthread.h>
 
+#include "i915_drv.h"
+#include "gvt.h"
+
 #define RING_CTX_OFF(x) \
 	offsetof(struct execlist_ring_context, x)
 
diff --git a/drivers/gpu/drm/i915/gvt/vgpu.c b/drivers/gpu/drm/i915/gvt/vgpu.c
index e5e0a72..9401436 100644
--- a/drivers/gpu/drm/i915/gvt/vgpu.c
+++ b/drivers/gpu/drm/i915/gvt/vgpu.c
@@ -32,6 +32,8 @@
  */
 
 #include "i915_drv.h"
+#include "gvt.h"
+#include "i915_pvinfo.h"
 
 static void clean_vgpu_mmio(struct intel_vgpu *vgpu)
 {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4d1133f..964d33d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1778,7 +1778,7 @@ struct drm_i915_private {
 
 	struct i915_virtual_gpu vgpu;
 
-	struct intel_gvt gvt;
+	void *gvt;
 
 	struct intel_guc guc;
 
@@ -2992,7 +2992,7 @@ int intel_wait_for_register_fw(struct drm_i915_private *dev_priv,
 
 static inline bool intel_gvt_active(struct drm_i915_private *dev_priv)
 {
-	return dev_priv->gvt.initialized;
+	return (dev_priv->gvt != NULL);
 }
 
 static inline bool intel_vgpu_active(struct drm_i915_private *dev_priv)
diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
index 8e8596d..bae1a7d 100644
--- a/drivers/gpu/drm/i915/intel_gvt.c
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -103,4 +103,5 @@ void intel_gvt_cleanup(struct drm_i915_private *dev_priv)
 		return;
 
 	intel_gvt_clean_device(dev_priv);
+	kfree(dev_priv->gvt);
 }
diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h
index 0f00105..0a9822d 100644
--- a/drivers/gpu/drm/i915/intel_gvt.h
+++ b/drivers/gpu/drm/i915/intel_gvt.h
@@ -24,9 +24,6 @@
 #ifndef _INTEL_GVT_H_
 #define _INTEL_GVT_H_
 
-#include "i915_pvinfo.h"
-#include "gvt/gvt.h"
-
 #ifdef CONFIG_DRM_I915_GVT
 int intel_gvt_init(struct drm_i915_private *dev_priv);
 void intel_gvt_cleanup(struct drm_i915_private *dev_priv);
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* [PATCH] Documentation/gpu: Add section for Intel GVT-g host support
  2016-10-20  7:29 [PATCH] drm/i915/gvt: clean up intel_gvt.h as interface for i915 core Zhenyu Wang
@ 2016-10-20  7:29 ` Zhenyu Wang
  2016-10-20  7:41 ` [PATCH] drm/i915/gvt: clean up intel_gvt.h as interface for i915 core Chris Wilson
  1 sibling, 0 replies; 3+ messages in thread
From: Zhenyu Wang @ 2016-10-20  7:29 UTC (permalink / raw)
  To: intel-gfx

Update with brief overview and reference for more detailed
arch design documents.

Add new section for Intel GVT-g host support.

Signed-off-by: Zhenyu Wang <zhenyuw@linux.intel.com>
---
 Documentation/gpu/i915.rst       | 9 +++++++++
 drivers/gpu/drm/i915/intel_gvt.c | 8 ++++++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/Documentation/gpu/i915.rst b/Documentation/gpu/i915.rst
index 87aaffc..95ce77f 100644
--- a/Documentation/gpu/i915.rst
+++ b/Documentation/gpu/i915.rst
@@ -49,6 +49,15 @@ Intel GVT-g Guest Support(vGPU)
 .. kernel-doc:: drivers/gpu/drm/i915/i915_vgpu.c
    :internal:
 
+Intel GVT-g Host Support(vGPU device model)
+-------------------------------------------
+
+.. kernel-doc:: drivers/gpu/drm/i915/intel_gvt.c
+   :doc: Intel GVT-g host support
+
+.. kernel-doc:: drivers/gpu/drm/i915/intel_gvt.c
+   :internal:
+
 Display Hardware Handling
 =========================
 
diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
index bae1a7d..88126b6 100644
--- a/drivers/gpu/drm/i915/intel_gvt.c
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -31,8 +31,12 @@
  * 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.
+ * seamlessly in a virtual machine.
+ *
+ * To virtualize GPU resources GVT-g driver depends on hypervisor technology
+ * e.g KVM/VFIO/mdev, Xen, etc. to provide resource access trapping capability
+ * and be virtualized within GVT-g device module. More architectural design
+ * doc is available on https://01.org/group/2230/documentation-list.
  */
 
 static bool is_supported_device(struct drm_i915_private *dev_priv)
-- 
2.9.3

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

^ permalink raw reply related	[flat|nested] 3+ messages in thread
* Re: [PATCH] drm/i915/gvt: clean up intel_gvt.h as interface for i915 core
  2016-10-20  7:29 [PATCH] drm/i915/gvt: clean up intel_gvt.h as interface for i915 core Zhenyu Wang
  2016-10-20  7:29 ` [PATCH] Documentation/gpu: Add section for Intel GVT-g host support Zhenyu Wang
@ 2016-10-20  7:41 ` Chris Wilson
  1 sibling, 0 replies; 3+ messages in thread
From: Chris Wilson @ 2016-10-20  7:41 UTC (permalink / raw)
  To: Zhenyu Wang; +Cc: intel-gfx

On Thu, Oct 20, 2016 at 03:29:13PM +0800, Zhenyu Wang wrote:
> @@ -214,9 +215,15 @@ int intel_gvt_init_device(struct drm_i915_private *dev_priv)
>  	if (WARN_ON(!intel_gvt_host.initialized))
>  		return -EINVAL;
>  
> -	if (WARN_ON(gvt->initialized))
> +	if (WARN_ON(dev_priv->gvt))
>  		return -EEXIST;
>  
> +	dev_priv->gvt = kzalloc(sizeof(struct intel_gvt), GFP_KERNEL);
> +	if (!dev_priv->gvt)
> +		return -ENOMEM;
> +
> +	gvt = to_gvt(dev_priv);
> +
>  	gvt_dbg_core("init gvt device\n");
>  
>  	mutex_init(&gvt->lock);
> @@ -280,5 +287,6 @@ out_free_firmware:
>  	intel_gvt_free_firmware(gvt);
>  out_clean_mmio_info:
>  	intel_gvt_clean_mmio_info(gvt);
> +	kfree(dev_priv->gvt);

And now dev_priv->gvt points where? What does
intel_gvt_active() think about that?

Allocate to the local gvt, and assign that to dev_priv->gvt at the end
once initialised and it has passed all the tests.

> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index 1564554..fd33e6b 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -213,6 +213,8 @@ struct intel_gvt {
>  	unsigned long service_request;
>  };
>  
> +#define to_gvt(dev_priv) (struct intel_gvt *)(dev_priv->gvt)

Why so few brackets? Like to play dangerously?

> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4d1133f..964d33d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1778,7 +1778,7 @@ struct drm_i915_private {
>  
>  	struct i915_virtual_gpu vgpu;
>  
> -	struct intel_gvt gvt;
> +	void *gvt;

Better to have a struct intel_gvt forward declaration than a void *.

>  	struct intel_guc guc;
>  
> @@ -2992,7 +2992,7 @@ int intel_wait_for_register_fw(struct drm_i915_private *dev_priv,
>  
>  static inline bool intel_gvt_active(struct drm_i915_private *dev_priv)
>  {
> -	return dev_priv->gvt.initialized;
> +	return (dev_priv->gvt != NULL);

(Why (so (many (brackets))))? Why redundant tests, why not Zoidberg?

>  }
>  
>  static inline bool intel_vgpu_active(struct drm_i915_private *dev_priv)
> diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
> index 8e8596d..bae1a7d 100644
> --- a/drivers/gpu/drm/i915/intel_gvt.c
> +++ b/drivers/gpu/drm/i915/intel_gvt.c
> @@ -103,4 +103,5 @@ void intel_gvt_cleanup(struct drm_i915_private *dev_priv)
>  		return;
>  
>  	intel_gvt_clean_device(dev_priv);
> +	kfree(dev_priv->gvt);

It wasn't allocated in intel_gvt.c, it shouldn't be freed here either.
-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] 3+ messages in thread
end of thread, other threads:[~2016-10-20  7:41 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-20  7:29 [PATCH] drm/i915/gvt: clean up intel_gvt.h as interface for i915 core Zhenyu Wang
2016-10-20  7:29 ` [PATCH] Documentation/gpu: Add section for Intel GVT-g host support Zhenyu Wang
2016-10-20  7:41 ` [PATCH] drm/i915/gvt: clean up intel_gvt.h as interface for i915 core Chris Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox