On Tue, 08 Feb 2022, Zhi Wang zhi.wang.linux@gmail.com wrote:
From: Zhi Wang zhi.wang.linux@gmail.com
To support the new mdev interfaces and the re-factor patches from Christoph, which moves the GVT-g code into a dedicated module, the GVT-g initialization path has to be separated into two phases:
a) Early initialization.
The early initialization of GVT requires to be done when loading i915. Mostly it's because the initial clean HW state needs to be saved before i915 touches the HW.
b) Late initalization.
This phases of initalization will setup the rest components of GVT-g, which can be done later when the dedicated module is being loaded.
What's the baseline for this series?
v6:
- Move the mmio_table.c into i915. (Christoph)
- Keep init_device_info and related structures in GVT-g. (Christoph)
- Refine the callbacks of the iterator. (Christoph)
- Move the flags of MMIO register defination to GVT-g. (Chrsitoph)
- Move the mmio block handling to GVT-g.
v5:
- Re-design the mmio table framework. (Christoph)
v4:
- Fix the errors of patch checking scripts.
v3:
- Fix the errors when CONFIG_DRM_I915_WERROR is turned on. (Jani)
v2:
- Implement a mmio table instead of generating it by marco in i915. (Jani)
Cc: Christoph Hellwig hch@lst.de Cc: Jason Gunthorpe jgg@nvidia.com Cc: Jani Nikula jani.nikula@linux.intel.com Cc: Joonas Lahtinen joonas.lahtinen@linux.intel.com Cc: Vivi Rodrigo rodrigo.vivi@intel.com Cc: Zhenyu Wang zhenyuw@linux.intel.com Cc: Zhi Wang zhi.a.wang@intel.com Tested-by: Terrence Xu terrence.xu@intel.com Signed-off-by: Zhi Wang zhi.wang.linux@gmail.com
drivers/gpu/drm/i915/Makefile | 2 +- drivers/gpu/drm/i915/gvt/cmd_parser.c | 2 +- drivers/gpu/drm/i915/gvt/gvt.h | 3 +- drivers/gpu/drm/i915/gvt/handlers.c | 1062 ++------------- drivers/gpu/drm/i915/gvt/mmio.h | 17 - drivers/gpu/drm/i915/gvt/reg.h | 9 +- drivers/gpu/drm/i915/intel_gvt.c | 20 +- drivers/gpu/drm/i915/intel_gvt.h | 37 + drivers/gpu/drm/i915/intel_gvt_mmio_table.c | 1308 +++++++++++++++++++ 9 files changed, 1501 insertions(+), 959 deletions(-) create mode 100644 drivers/gpu/drm/i915/intel_gvt_mmio_table.c
diff --git a/drivers/gpu/drm/i915/intel_gvt.h b/drivers/gpu/drm/i915/intel_gvt.h index d7d3fb6186fd..6d3031f3ac25 100644 --- a/drivers/gpu/drm/i915/intel_gvt.h +++ b/drivers/gpu/drm/i915/intel_gvt.h @@ -26,7 +26,32 @@
struct drm_i915_private;
+#include <linux/kernel.h>
Please use minimal includes. Looks like linux/types.h is enough. Please also put the includes before the forward declarations.
#ifdef CONFIG_DRM_I915_GVT
+#define D_BDW (1 << 0) +#define D_SKL (1 << 1) +#define D_KBL (1 << 2) +#define D_BXT (1 << 3) +#define D_CFL (1 << 4)
+#define D_GEN9PLUS (D_SKL | D_KBL | D_BXT | D_CFL) +#define D_GEN8PLUS (D_BDW | D_SKL | D_KBL | D_BXT | D_CFL)
+#define D_SKL_PLUS (D_SKL | D_KBL | D_BXT | D_CFL) +#define D_BDW_PLUS (D_BDW | D_SKL | D_KBL | D_BXT | D_CFL)
+#define D_PRE_SKL (D_BDW) +#define D_ALL (D_BDW | D_SKL | D_KBL | D_BXT | D_CFL)
If these really need to be in a header in i915/, I think they need to be longer with some namespacing or something. I do wish these could be hidden though.
+struct intel_gvt_mmio_table_iter {
- struct drm_i915_private *i915;
- void *data;
- int (*handle_mmio_cb)(struct intel_gvt_mmio_table_iter *iter,
u32 offset, u32 device, u32 size);+};
We're heavily transitioning towards having a corresponding .h for each .c instead of catch all headers. It's still a work in progress, but I'd prefer having the declarations for stuff in intel_gvt_mmio_table.c placed in intel_gvt_mmio_table.h, and named accordingly. Like I suggested in my previous mails.
int intel_gvt_init(struct drm_i915_private *dev_priv); void intel_gvt_driver_remove(struct drm_i915_private *dev_priv); int intel_gvt_init_device(struct drm_i915_private *dev_priv); @@ -34,6 +59,8 @@ void intel_gvt_clean_device(struct drm_i915_private *dev_priv); int intel_gvt_init_host(void); void intel_gvt_sanitize_options(struct drm_i915_private *dev_priv); void intel_gvt_resume(struct drm_i915_private *dev_priv); +unsigned long intel_gvt_get_device_type(struct drm_i915_private *i915); +int intel_gvt_iterate_mmio_table(struct intel_gvt_mmio_table_iter *iter); #else static inline int intel_gvt_init(struct drm_i915_private *dev_priv) { @@ -51,6 +78,16 @@ static inline void intel_gvt_sanitize_options(struct drm_i915_private *dev_priv) static inline void intel_gvt_resume(struct drm_i915_private *dev_priv) { }
+unsigned long intel_gvt_get_device_type(struct drm_i915_private *i915) +{
- return 0;
+}
+int intel_gvt_iterate_mmio_table(struct intel_gvt_mmio_table_iter *iter) +{
- return 0;
+}
Stubs need to be static inlines.
#endif
#endif /* _INTEL_GVT_H_ */ diff --git a/drivers/gpu/drm/i915/intel_gvt_mmio_table.c b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c new file mode 100644 index 000000000000..b9de72e939d3 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_gvt_mmio_table.c @@ -0,0 +1,1308 @@ +/*
- Copyright(c) 2021 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.
Please use SPDX headers for new files.
BR, Jani.