On Tue, 08 Feb 2022, Zhi Wang <zhi.wang.linux@xxxxxxxxx> wrote:
> From: Zhi Wang <zhi.wang.linux@xxxxxxxxx>
>
> 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@xxxxxx>
> Cc: Jason Gunthorpe <jgg@xxxxxxxxxx>
> Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
> Cc: Vivi Rodrigo <rodrigo.vivi@xxxxxxxxx>
> Cc: Zhenyu Wang <zhenyuw@xxxxxxxxxxxxxxx>
> Cc: Zhi Wang <zhi.a.wang@xxxxxxxxx>
> Tested-by: Terrence Xu <terrence.xu@xxxxxxxxx>
> Signed-off-by: Zhi Wang <zhi.wang.linux@xxxxxxxxx>
> ---
> 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.
--
Jani Nikula, Intel Open Source Graphics Center
