Re: [PATCH 1/3] i915/gvt: seperate tracked MMIO table from handlers.c
From: Christoph Hellwig
Date: Tue Nov 09 2021 - 04:44:23 EST
On Tue, Nov 09, 2021 at 09:00:39AM +0200, Jani Nikula wrote:
> On Mon, 08 Nov 2021, 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
> > MMIO snapshot still needs to be saved in i915 so that the inital clean HW
> > state can be used for the further vGPU. Seperate the tracked MMIO table
> > from GVT-g, so that GVT-g and i915 can both use it.
>
> Do you really have to both put code in a header and then include that in
> multiple places?
>
> I think you may need to rethink the whole approach, maybe make them
> actual tables instead of code.
I played around with this a bit and I can't think of anyting better,
especially given that a function (i915_mmio_reg_offset) is used to
get the offset. So except for the cosmetic cleanup below I think this
is the best we can do for now:
diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
index 4e2fd564abea1..c1f5f3b8abb2c 100644
--- a/drivers/gpu/drm/i915/gvt/gvt.h
+++ b/drivers/gpu/drm/i915/gvt/gvt.h
@@ -295,8 +295,8 @@ struct intel_vgpu_type {
};
struct intel_gvt_hw_state {
- void *cfg_space;
- void *mmio;
+ u32 *cfg_space;
+ u32 *mmio;
};
struct intel_gvt {
diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
index 6a08d362bf664..41d1bb80aba40 100644
--- a/drivers/gpu/drm/i915/gvt/handlers.c
+++ b/drivers/gpu/drm/i915/gvt/handlers.c
@@ -2124,6 +2124,17 @@ static int csfe_chicken1_mmio_write(struct intel_vgpu *vgpu,
return 0;
}
+/*
+ * Generate the MMIO handler hash table.
+ */
+#define MMIO_F(reg, s, f, am, rm, d, r, w) \
+do { \
+ int ret = new_mmio_info(gvt, i915_mmio_reg_offset(reg), \
+ f, s, am, rm, d, r, w); \
+ if (ret) \
+ return ret; \
+} while (0)
+
#include "mmio_table.h"
static struct gvt_mmio_block *find_mmio_block(struct intel_gvt *gvt,
diff --git a/drivers/gpu/drm/i915/gvt/mmio_table.h b/drivers/gpu/drm/i915/gvt/mmio_table.h
index 39a4cb59695ae..2a17f7162224d 100644
--- a/drivers/gpu/drm/i915/gvt/mmio_table.h
+++ b/drivers/gpu/drm/i915/gvt/mmio_table.h
@@ -25,20 +25,7 @@
#ifndef _GVT_MMIO_TABLE_H_
#define _GVT_MMIO_TABLE_H_
-#ifdef GENERATE_MMIO_TABLE_IN_I915
-#define MMIO_F(reg, s, f, am, rm, d, r, w) do { \
- ret = new_mmio_info(gvt, i915_mmio_reg_offset(reg)); \
- if (ret) \
- return ret; \
-} while (0)
-#else
-#define MMIO_F(reg, s, f, am, rm, d, r, w) do { \
- ret = new_mmio_info(gvt, i915_mmio_reg_offset(reg), \
- f, s, am, rm, d, r, w); \
- if (ret) \
- return ret; \
-} while (0)
-#endif
+#include "gvt/reg.h"
#define MMIO_D(reg, d) \
MMIO_F(reg, 4, 0, 0, 0, d, NULL, NULL)
@@ -86,8 +73,6 @@ static int intel_gvt_init_generic_mmio_info(struct intel_gvt *gvt)
{
struct drm_i915_private *dev_priv = gvt->gt->i915;
- int ret;
-
MMIO_RING_DFH(RING_IMR, D_ALL, 0, NULL,
intel_vgpu_reg_imr_handler);
@@ -905,7 +890,6 @@ static int intel_gvt_init_generic_mmio_info(struct intel_gvt *gvt)
static int intel_gvt_init_bdw_mmio_info(struct intel_gvt *gvt)
{
struct drm_i915_private *dev_priv = gvt->gt->i915;
- int ret;
MMIO_DH(GEN8_GT_IMR(0), D_BDW_PLUS, NULL, intel_vgpu_reg_imr_handler);
MMIO_DH(GEN8_GT_IER(0), D_BDW_PLUS, NULL, intel_vgpu_reg_ier_handler);
@@ -1095,7 +1079,6 @@ static int intel_gvt_init_bdw_mmio_info(struct intel_gvt *gvt)
static int intel_gvt_init_skl_mmio_info(struct intel_gvt *gvt)
{
struct drm_i915_private *dev_priv = gvt->gt->i915;
- int ret;
MMIO_DH(FORCEWAKE_RENDER_GEN9, D_SKL_PLUS, NULL, mul_force_wake_write);
MMIO_DH(FORCEWAKE_ACK_RENDER_GEN9, D_SKL_PLUS, NULL, NULL);
@@ -1346,7 +1329,6 @@ static int intel_gvt_init_skl_mmio_info(struct intel_gvt *gvt)
static int intel_gvt_init_bxt_mmio_info(struct intel_gvt *gvt)
{
struct drm_i915_private *dev_priv = gvt->gt->i915;
- int ret;
MMIO_F(_MMIO(0x80000), 0x3000, 0, 0, 0, D_BXT, NULL, NULL);
diff --git a/drivers/gpu/drm/i915/intel_gvt.c b/drivers/gpu/drm/i915/intel_gvt.c
index 4fd51974bd359..fa9d79815af26 100644
--- a/drivers/gpu/drm/i915/intel_gvt.c
+++ b/drivers/gpu/drm/i915/intel_gvt.c
@@ -86,19 +86,17 @@ void intel_gvt_sanitize_options(struct drm_i915_private *dev_priv)
dev_priv->params.enable_gvt = 0;
}
-#define GENERATE_MMIO_TABLE_IN_I915
-static int new_mmio_info(struct intel_gvt *gvt, u32 offset)
-{
- void *mmio = gvt->hw_state.mmio;
-
- *(u32 *)(mmio + offset) = intel_uncore_read_notrace(gvt->gt->uncore,
- _MMIO(offset));
- return 0;
-}
-
-#include "gvt/reg.h"
+/*
+ * Generates the MMIO golden state table.
+ */
+#define MMIO_F(reg, s, f, am, rm, d, r, w) \
+do { \
+ u32 offset = i915_mmio_reg_offset(reg); \
+ \
+ (gvt)->hw_state.mmio[offset] = \
+ intel_uncore_read_notrace((gvt)->gt->uncore, _MMIO(offset)); \
+} while (0)
#include "gvt/mmio_table.h"
-#undef GENERATE_MMIO_TABLE_IN_I915
static void init_device_info(struct intel_gvt *gvt)
{