public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH v4 0/2] drm/i915: split out intel_vtd.[ch] from i915_drv.h
@ 2022-02-08 16:54 Jani Nikula
  2022-02-08 16:54 ` [Intel-gfx] [PATCH v4 1/2] " Jani Nikula
                   ` (4 more replies)
  0 siblings, 5 replies; 9+ messages in thread
From: Jani Nikula @ 2022-02-08 16:54 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Another rebase & resend of [1].


[1] https://patchwork.freedesktop.org/series/98789/

Jani Nikula (2):
  drm/i915: split out intel_vtd.[ch] from i915_drv.h
  drm/i915/vtd: rename functions to have the usual prefix

 drivers/gpu/drm/i915/Makefile                |  1 +
 drivers/gpu/drm/i915/display/intel_bw.c      |  1 +
 drivers/gpu/drm/i915/display/intel_display.c |  5 +-
 drivers/gpu/drm/i915/display/intel_fb_pin.c  |  3 +-
 drivers/gpu/drm/i915/display/intel_fbc.c     |  1 +
 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c |  3 +-
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c   |  1 +
 drivers/gpu/drm/i915/gem/i915_gemfs.c        |  1 +
 drivers/gpu/drm/i915/gt/intel_ggtt.c         | 14 +++---
 drivers/gpu/drm/i915/gt/intel_gtt.c          |  3 +-
 drivers/gpu/drm/i915/i915_debugfs.c          |  3 +-
 drivers/gpu/drm/i915/i915_driver.c           |  9 +---
 drivers/gpu/drm/i915/i915_drv.h              | 37 --------------
 drivers/gpu/drm/i915/i915_gpu_error.c        |  1 +
 drivers/gpu/drm/i915/intel_device_info.c     |  3 +-
 drivers/gpu/drm/i915/intel_pch.c             |  3 +-
 drivers/gpu/drm/i915/intel_pm.c              |  1 +
 drivers/gpu/drm/i915/intel_vtd.c             | 14 ++++++
 drivers/gpu/drm/i915/intel_vtd.h             | 51 ++++++++++++++++++++
 19 files changed, 96 insertions(+), 59 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_vtd.c
 create mode 100644 drivers/gpu/drm/i915/intel_vtd.h

-- 
2.30.2


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

* [Intel-gfx] [PATCH v4 1/2] drm/i915: split out intel_vtd.[ch] from i915_drv.h
  2022-02-08 16:54 [Intel-gfx] [PATCH v4 0/2] drm/i915: split out intel_vtd.[ch] from i915_drv.h Jani Nikula
@ 2022-02-08 16:54 ` Jani Nikula
  2022-02-09  9:40   ` Tvrtko Ursulin
  2022-02-08 16:54 ` [Intel-gfx] [PATCH v4 2/2] drm/i915/vtd: rename functions to have the usual prefix Jani Nikula
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2022-02-08 16:54 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

Perhaps a bit contrived, but we need to reduce the size of i915_drv.h
from all the accumulated cruft.

v4: Rebase

v3: Rebase

v2: Turns out asm/hypervisor.h is not self-contained

Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/Makefile                |  1 +
 drivers/gpu/drm/i915/display/intel_bw.c      |  1 +
 drivers/gpu/drm/i915/display/intel_display.c |  5 +-
 drivers/gpu/drm/i915/display/intel_fb_pin.c  |  1 +
 drivers/gpu/drm/i915/display/intel_fbc.c     |  1 +
 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c |  1 +
 drivers/gpu/drm/i915/gem/i915_gem_stolen.c   |  1 +
 drivers/gpu/drm/i915/gem/i915_gemfs.c        |  1 +
 drivers/gpu/drm/i915/gt/intel_ggtt.c         |  8 +--
 drivers/gpu/drm/i915/gt/intel_gtt.c          |  1 +
 drivers/gpu/drm/i915/i915_debugfs.c          |  1 +
 drivers/gpu/drm/i915/i915_driver.c           |  7 +--
 drivers/gpu/drm/i915/i915_drv.h              | 37 --------------
 drivers/gpu/drm/i915/i915_gpu_error.c        |  1 +
 drivers/gpu/drm/i915/intel_device_info.c     |  3 +-
 drivers/gpu/drm/i915/intel_pch.c             |  1 +
 drivers/gpu/drm/i915/intel_pm.c              |  1 +
 drivers/gpu/drm/i915/intel_vtd.c             | 14 ++++++
 drivers/gpu/drm/i915/intel_vtd.h             | 51 ++++++++++++++++++++
 19 files changed, 87 insertions(+), 50 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/intel_vtd.c
 create mode 100644 drivers/gpu/drm/i915/intel_vtd.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index a26e6736bebb..d79c06b362f1 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -55,6 +55,7 @@ i915-y += i915_driver.o \
 	  intel_sbi.o \
 	  intel_step.o \
 	  intel_uncore.o \
+	  intel_vtd.o \
 	  intel_wakeref.o \
 	  vlv_sideband.o \
 	  vlv_suspend.o
diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 5dce3cf0ed12..edcf77d3694d 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -12,6 +12,7 @@
 #include "intel_display_types.h"
 #include "intel_pcode.h"
 #include "intel_pm.h"
+#include "intel_vtd.h"
 
 /* Parameters for Qclk Geyserville (QGV) */
 struct intel_qgv_point {
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 7f512f9e9e5c..2af5eca2fe12 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -75,6 +75,7 @@
 #include "g4x_dp.h"
 #include "g4x_hdmi.h"
 #include "i915_drv.h"
+#include "i9xx_plane.h"
 #include "icl_dsi.h"
 #include "intel_acpi.h"
 #include "intel_atomic.h"
@@ -109,12 +110,12 @@
 #include "intel_sprite.h"
 #include "intel_tc.h"
 #include "intel_vga.h"
-#include "i9xx_plane.h"
+#include "intel_vtd.h"
 #include "skl_scaler.h"
 #include "skl_universal_plane.h"
+#include "vlv_dsi.h"
 #include "vlv_dsi_pll.h"
 #include "vlv_sideband.h"
-#include "vlv_dsi.h"
 
 static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state);
 static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c b/drivers/gpu/drm/i915/display/intel_fb_pin.c
index c4b3d76341f3..05339b3bc39b 100644
--- a/drivers/gpu/drm/i915/display/intel_fb_pin.c
+++ b/drivers/gpu/drm/i915/display/intel_fb_pin.c
@@ -14,6 +14,7 @@
 #include "intel_dpt.h"
 #include "intel_fb.h"
 #include "intel_fb_pin.h"
+#include "intel_vtd.h"
 
 static struct i915_vma *
 intel_pin_fb_obj_dpt(struct drm_framebuffer *fb,
diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
index bcdffe62f3cb..4c5a5aed7ca7 100644
--- a/drivers/gpu/drm/i915/display/intel_fbc.c
+++ b/drivers/gpu/drm/i915/display/intel_fbc.c
@@ -48,6 +48,7 @@
 #include "intel_display_types.h"
 #include "intel_fbc.h"
 #include "intel_frontbuffer.h"
+#include "intel_vtd.h"
 
 #define for_each_fbc_id(__dev_priv, __fbc_id) \
 	for ((__fbc_id) = INTEL_FBC_A; (__fbc_id) < I915_MAX_FBCS; (__fbc_id)++) \
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index 6a6ff98a8746..0fb0921c81ab 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -16,6 +16,7 @@
 #include "gt/intel_gt_requests.h"
 
 #include "i915_trace.h"
+#include "intel_vtd.h"
 
 static bool swap_available(void)
 {
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
index 1de73a644965..361378846127 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
@@ -16,6 +16,7 @@
 #include "i915_gem_stolen.h"
 #include "i915_reg.h"
 #include "i915_vgpu.h"
+#include "intel_vtd.h"
 
 /*
  * The BIOS typically reserves some of the system's memory for the exclusive
diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/i915/gem/i915_gemfs.c
index 7271fbf813fa..7eb4b5ff9147 100644
--- a/drivers/gpu/drm/i915/gem/i915_gemfs.c
+++ b/drivers/gpu/drm/i915/gem/i915_gemfs.c
@@ -9,6 +9,7 @@
 
 #include "i915_drv.h"
 #include "i915_gemfs.h"
+#include "intel_vtd.h"
 
 int i915_gemfs_init(struct drm_i915_private *i915)
 {
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 8850d4e0f9cc..e49d8d17095a 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -14,14 +14,14 @@
 
 #include "gem/i915_gem_lmem.h"
 
-#include "intel_gt.h"
-#include "intel_gt_regs.h"
+#include "gen8_ppgtt.h"
 #include "i915_drv.h"
 #include "i915_scatterlist.h"
 #include "i915_vgpu.h"
-
+#include "intel_gt.h"
+#include "intel_gt_regs.h"
 #include "intel_gtt.h"
-#include "gen8_ppgtt.h"
+#include "intel_vtd.h"
 
 static void i915_ggtt_color_adjust(const struct drm_mm_node *node,
 				   unsigned long color,
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 0d6bbc8c57f2..524f3c3eb027 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -15,6 +15,7 @@
 #include "intel_gt.h"
 #include "intel_gt_regs.h"
 #include "intel_gtt.h"
+#include "intel_vtd.h"
 
 struct drm_i915_gem_object *alloc_pt_lmem(struct i915_address_space *vm, int sz)
 {
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index ca52ee0742ce..001958238cfe 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -50,6 +50,7 @@
 #include "i915_irq.h"
 #include "i915_scheduler.h"
 #include "intel_pm.h"
+#include "intel_vtd.h"
 
 static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
 {
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 3d41f532a5d6..32a3265a177a 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -92,6 +92,7 @@
 #include "intel_pcode.h"
 #include "intel_pm.h"
 #include "intel_region_ttm.h"
+#include "intel_vtd.h"
 #include "vlv_suspend.h"
 
 static const struct drm_driver i915_drm_driver;
@@ -746,12 +747,6 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 	i915_gem_driver_unregister(dev_priv);
 }
 
-void
-i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p)
-{
-	drm_printf(p, "iommu: %s\n", enableddisabled(intel_vtd_active(i915)));
-}
-
 static void i915_welcome_messages(struct drm_i915_private *dev_priv)
 {
 	if (drm_debug_enabled(DRM_UT_DRIVER)) {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8c1706fd81f9..b82528ebe70a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -33,14 +33,11 @@
 #include <uapi/drm/i915_drm.h>
 #include <uapi/drm/drm_fourcc.h>
 
-#include <asm/hypervisor.h>
-
 #include <linux/io-mapping.h>
 #include <linux/i2c.h>
 #include <linux/i2c-algo-bit.h>
 #include <linux/backlight.h>
 #include <linux/hash.h>
-#include <linux/intel-iommu.h>
 #include <linux/kref.h>
 #include <linux/mm_types.h>
 #include <linux/perf_event.h>
@@ -1562,43 +1559,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define HAS_GUC_DEPRIVILEGE(dev_priv) \
 	(INTEL_INFO(dev_priv)->has_guc_deprivilege)
 
-static inline bool run_as_guest(void)
-{
-	return !hypervisor_is_type(X86_HYPER_NATIVE);
-}
-
 #define HAS_D12_PLANE_MINIMIZATION(dev_priv) (IS_ROCKETLAKE(dev_priv) || \
 					      IS_ALDERLAKE_S(dev_priv))
 
-static inline bool intel_vtd_active(struct drm_i915_private *i915)
-{
-	if (device_iommu_mapped(i915->drm.dev))
-		return true;
-
-	/* Running as a guest, we assume the host is enforcing VT'd */
-	return run_as_guest();
-}
-
-void
-i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p);
-
-static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv)
-{
-	return DISPLAY_VER(dev_priv) >= 6 && intel_vtd_active(dev_priv);
-}
-
-static inline bool
-intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)
-{
-	return IS_BROXTON(i915) && intel_vtd_active(i915);
-}
-
-static inline bool
-intel_vm_no_concurrent_access_wa(struct drm_i915_private *i915)
-{
-	return IS_CHERRYVIEW(i915) || intel_ggtt_update_needs_vtd_wa(i915);
-}
-
 /* i915_gem.c */
 void i915_gem_init_early(struct drm_i915_private *dev_priv);
 void i915_gem_cleanup_early(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
index 127ff56c8ce6..ccb6d62a51df 100644
--- a/drivers/gpu/drm/i915/i915_gpu_error.c
+++ b/drivers/gpu/drm/i915/i915_gpu_error.c
@@ -50,6 +50,7 @@
 #include "i915_gpu_error.h"
 #include "i915_memcpy.h"
 #include "i915_scatterlist.h"
+#include "intel_vtd.h"
 
 #define ALLOW_FAIL (__GFP_KSWAPD_RECLAIM | __GFP_RETRY_MAYFAIL | __GFP_NOWARN)
 #define ATOMIC_MAYFAIL (GFP_ATOMIC | __GFP_NOWARN)
diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
index 94da5aa37391..13abae12cf10 100644
--- a/drivers/gpu/drm/i915/intel_device_info.c
+++ b/drivers/gpu/drm/i915/intel_device_info.c
@@ -27,8 +27,9 @@
 
 #include "display/intel_cdclk.h"
 #include "display/intel_de.h"
-#include "intel_device_info.h"
 #include "i915_drv.h"
+#include "intel_device_info.h"
+#include "intel_vtd.h"
 
 #define PLATFORM_NAME(x) [INTEL_##x] = #x
 static const char * const platform_names[] = {
diff --git a/drivers/gpu/drm/i915/intel_pch.c b/drivers/gpu/drm/i915/intel_pch.c
index da8f82c2342f..4d8a98dbf465 100644
--- a/drivers/gpu/drm/i915/intel_pch.c
+++ b/drivers/gpu/drm/i915/intel_pch.c
@@ -5,6 +5,7 @@
 
 #include "i915_drv.h"
 #include "intel_pch.h"
+#include "intel_vtd.h"
 
 /* Map PCH device id to PCH type, or PCH_NONE if unknown. */
 static enum intel_pch
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 23d4bb011fc8..23f0e2473e27 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -52,6 +52,7 @@
 #include "i915_irq.h"
 #include "intel_pcode.h"
 #include "intel_pm.h"
+#include "intel_vtd.h"
 #include "vlv_sideband.h"
 #include "../../../platform/x86/intel_ips.h"
 
diff --git a/drivers/gpu/drm/i915/intel_vtd.c b/drivers/gpu/drm/i915/intel_vtd.c
new file mode 100644
index 000000000000..e52432141081
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_vtd.c
@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#include <drm/drm_print.h>
+
+#include "intel_vtd.h"
+
+void
+i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p)
+{
+	drm_printf(p, "iommu: %s\n", enableddisabled(intel_vtd_active(i915)));
+}
diff --git a/drivers/gpu/drm/i915/intel_vtd.h b/drivers/gpu/drm/i915/intel_vtd.h
new file mode 100644
index 000000000000..421bf8ca0b36
--- /dev/null
+++ b/drivers/gpu/drm/i915/intel_vtd.h
@@ -0,0 +1,51 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2022 Intel Corporation
+ */
+
+#ifndef __INTEL_VTD_H__
+#define __INTEL_VTD_H__
+
+#include <linux/device.h>
+#include <linux/types.h>
+#include <asm/hypervisor.h>
+
+#include "i915_drv.h"
+
+struct drm_printer;
+
+static inline bool run_as_guest(void)
+{
+	return !hypervisor_is_type(X86_HYPER_NATIVE);
+}
+
+static inline bool intel_vtd_active(struct drm_i915_private *i915)
+{
+	if (device_iommu_mapped(i915->drm.dev))
+		return true;
+
+	/* Running as a guest, we assume the host is enforcing VT'd */
+	return run_as_guest();
+}
+
+static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *i915)
+{
+	return GRAPHICS_VER(i915) >= 6 && intel_vtd_active(i915);
+}
+
+static inline bool
+intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)
+{
+	return IS_BROXTON(i915) && intel_vtd_active(i915);
+}
+
+static inline bool
+intel_vm_no_concurrent_access_wa(struct drm_i915_private *i915)
+{
+	return IS_CHERRYVIEW(i915) || intel_ggtt_update_needs_vtd_wa(i915);
+}
+
+void
+i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p);
+
+#endif /* __INTEL_VTD_H__ */
-- 
2.30.2


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

* [Intel-gfx] [PATCH v4 2/2] drm/i915/vtd: rename functions to have the usual prefix
  2022-02-08 16:54 [Intel-gfx] [PATCH v4 0/2] drm/i915: split out intel_vtd.[ch] from i915_drv.h Jani Nikula
  2022-02-08 16:54 ` [Intel-gfx] [PATCH v4 1/2] " Jani Nikula
@ 2022-02-08 16:54 ` Jani Nikula
  2022-02-08 18:06 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: split out intel_vtd.[ch] from i915_drv.h Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 9+ messages in thread
From: Jani Nikula @ 2022-02-08 16:54 UTC (permalink / raw)
  To: intel-gfx; +Cc: jani.nikula

The prefix should tell where the function is to be found and where it
belongs.

Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_fb_pin.c  |  2 +-
 drivers/gpu/drm/i915/gem/i915_gem_shrinker.c |  2 +-
 drivers/gpu/drm/i915/gt/intel_ggtt.c         |  6 +++---
 drivers/gpu/drm/i915/gt/intel_gtt.c          |  2 +-
 drivers/gpu/drm/i915/i915_debugfs.c          |  2 +-
 drivers/gpu/drm/i915/i915_driver.c           |  2 +-
 drivers/gpu/drm/i915/intel_pch.c             |  2 +-
 drivers/gpu/drm/i915/intel_vtd.c             |  2 +-
 drivers/gpu/drm/i915/intel_vtd.h             | 14 +++++++-------
 9 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c b/drivers/gpu/drm/i915/display/intel_fb_pin.c
index 05339b3bc39b..39cff135c2ae 100644
--- a/drivers/gpu/drm/i915/display/intel_fb_pin.c
+++ b/drivers/gpu/drm/i915/display/intel_fb_pin.c
@@ -108,7 +108,7 @@ intel_pin_and_fence_fb_obj(struct drm_framebuffer *fb,
 	 * we should always have valid PTE following the scanout preventing
 	 * the VT-d warning.
 	 */
-	if (intel_scanout_needs_vtd_wa(dev_priv) && alignment < 256 * 1024)
+	if (intel_vtd_scanout_needs_wa(dev_priv) && alignment < 256 * 1024)
 		alignment = 256 * 1024;
 
 	/*
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
index 0fb0921c81ab..3ae9d1ae7ba6 100644
--- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
+++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
@@ -123,7 +123,7 @@ i915_gem_shrink(struct i915_gem_ww_ctx *ww,
 	int err = 0;
 
 	/* CHV + VTD workaround use stop_machine(); need to trylock vm->mutex */
-	bool trylock_vm = !ww && intel_vm_no_concurrent_access_wa(i915);
+	bool trylock_vm = !ww && intel_vtd_vm_no_concurrent_access_wa(i915);
 
 	trace_i915_gem_shrink(i915, target, shrink);
 
diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index e49d8d17095a..f3ca1f93192a 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -974,7 +974,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 	ggtt->vm.cleanup = gen6_gmch_remove;
 	ggtt->vm.insert_page = gen8_ggtt_insert_page;
 	ggtt->vm.clear_range = nop_clear_range;
-	if (intel_scanout_needs_vtd_wa(i915))
+	if (intel_vtd_scanout_needs_wa(i915))
 		ggtt->vm.clear_range = gen8_ggtt_clear_range;
 
 	ggtt->vm.insert_entries = gen8_ggtt_insert_entries;
@@ -983,7 +983,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt)
 	 * Serialize GTT updates with aperture access on BXT if VT-d is on,
 	 * and always on CHV.
 	 */
-	if (intel_vm_no_concurrent_access_wa(i915)) {
+	if (intel_vtd_vm_no_concurrent_access_wa(i915)) {
 		ggtt->vm.insert_entries = bxt_vtd_ggtt_insert_entries__BKL;
 		ggtt->vm.insert_page    = bxt_vtd_ggtt_insert_page__BKL;
 		ggtt->vm.bind_async_flags =
@@ -1122,7 +1122,7 @@ static int gen6_gmch_probe(struct i915_ggtt *ggtt)
 	ggtt->vm.alloc_scratch_dma = alloc_pt_dma;
 
 	ggtt->vm.clear_range = nop_clear_range;
-	if (!HAS_FULL_PPGTT(i915) || intel_scanout_needs_vtd_wa(i915))
+	if (!HAS_FULL_PPGTT(i915) || intel_vtd_scanout_needs_wa(i915))
 		ggtt->vm.clear_range = gen6_ggtt_clear_range;
 	ggtt->vm.insert_page = gen6_ggtt_insert_page;
 	ggtt->vm.insert_entries = gen6_ggtt_insert_entries;
diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
index 524f3c3eb027..5331ad7decfe 100644
--- a/drivers/gpu/drm/i915/gt/intel_gtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
@@ -206,7 +206,7 @@ void i915_address_space_init(struct i915_address_space *vm, int subclass)
 	mutex_init(&vm->mutex);
 	lockdep_set_subclass(&vm->mutex, subclass);
 
-	if (!intel_vm_no_concurrent_access_wa(vm->i915)) {
+	if (!intel_vtd_vm_no_concurrent_access_wa(vm->i915)) {
 		i915_gem_shrinker_taints_mutex(vm->i915, &vm->mutex);
 	} else {
 		/*
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 001958238cfe..2aeee6130fe1 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -66,7 +66,7 @@ static int i915_capabilities(struct seq_file *m, void *data)
 
 	intel_device_info_print_static(INTEL_INFO(i915), &p);
 	intel_device_info_print_runtime(RUNTIME_INFO(i915), &p);
-	i915_print_iommu_status(i915, &p);
+	intel_vtd_print_iommu_status(i915, &p);
 	intel_gt_info_print(&to_gt(i915)->info, &p);
 	intel_driver_caps_print(&i915->caps, &p);
 
diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
index 32a3265a177a..ff1f74891b8c 100644
--- a/drivers/gpu/drm/i915/i915_driver.c
+++ b/drivers/gpu/drm/i915/i915_driver.c
@@ -762,7 +762,7 @@ static void i915_welcome_messages(struct drm_i915_private *dev_priv)
 
 		intel_device_info_print_static(INTEL_INFO(dev_priv), &p);
 		intel_device_info_print_runtime(RUNTIME_INFO(dev_priv), &p);
-		i915_print_iommu_status(dev_priv, &p);
+		intel_vtd_print_iommu_status(dev_priv, &p);
 		intel_gt_info_print(&to_gt(dev_priv)->info, &p);
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_pch.c b/drivers/gpu/drm/i915/intel_pch.c
index 4d8a98dbf465..6c53cef85115 100644
--- a/drivers/gpu/drm/i915/intel_pch.c
+++ b/drivers/gpu/drm/i915/intel_pch.c
@@ -256,7 +256,7 @@ void intel_detect_pch(struct drm_i915_private *dev_priv)
 		dev_priv->pch_type = PCH_NOP;
 		dev_priv->pch_id = 0;
 	} else if (!pch) {
-		if (run_as_guest() && HAS_DISPLAY(dev_priv)) {
+		if (intel_vtd_run_as_guest() && HAS_DISPLAY(dev_priv)) {
 			intel_virt_detect_pch(dev_priv, &id, &pch_type);
 			dev_priv->pch_type = pch_type;
 			dev_priv->pch_id = id;
diff --git a/drivers/gpu/drm/i915/intel_vtd.c b/drivers/gpu/drm/i915/intel_vtd.c
index e52432141081..8639e785254b 100644
--- a/drivers/gpu/drm/i915/intel_vtd.c
+++ b/drivers/gpu/drm/i915/intel_vtd.c
@@ -8,7 +8,7 @@
 #include "intel_vtd.h"
 
 void
-i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p)
+intel_vtd_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p)
 {
 	drm_printf(p, "iommu: %s\n", enableddisabled(intel_vtd_active(i915)));
 }
diff --git a/drivers/gpu/drm/i915/intel_vtd.h b/drivers/gpu/drm/i915/intel_vtd.h
index 421bf8ca0b36..d20ae8314f8c 100644
--- a/drivers/gpu/drm/i915/intel_vtd.h
+++ b/drivers/gpu/drm/i915/intel_vtd.h
@@ -14,7 +14,7 @@
 
 struct drm_printer;
 
-static inline bool run_as_guest(void)
+static inline bool intel_vtd_run_as_guest(void)
 {
 	return !hypervisor_is_type(X86_HYPER_NATIVE);
 }
@@ -25,27 +25,27 @@ static inline bool intel_vtd_active(struct drm_i915_private *i915)
 		return true;
 
 	/* Running as a guest, we assume the host is enforcing VT'd */
-	return run_as_guest();
+	return intel_vtd_run_as_guest();
 }
 
-static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *i915)
+static inline bool intel_vtd_scanout_needs_wa(struct drm_i915_private *i915)
 {
 	return GRAPHICS_VER(i915) >= 6 && intel_vtd_active(i915);
 }
 
 static inline bool
-intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)
+intel_vtd_ggtt_update_needs_wa(struct drm_i915_private *i915)
 {
 	return IS_BROXTON(i915) && intel_vtd_active(i915);
 }
 
 static inline bool
-intel_vm_no_concurrent_access_wa(struct drm_i915_private *i915)
+intel_vtd_vm_no_concurrent_access_wa(struct drm_i915_private *i915)
 {
-	return IS_CHERRYVIEW(i915) || intel_ggtt_update_needs_vtd_wa(i915);
+	return IS_CHERRYVIEW(i915) || intel_vtd_ggtt_update_needs_wa(i915);
 }
 
 void
-i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p);
+intel_vtd_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p);
 
 #endif /* __INTEL_VTD_H__ */
-- 
2.30.2


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

* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: split out intel_vtd.[ch] from i915_drv.h
  2022-02-08 16:54 [Intel-gfx] [PATCH v4 0/2] drm/i915: split out intel_vtd.[ch] from i915_drv.h Jani Nikula
  2022-02-08 16:54 ` [Intel-gfx] [PATCH v4 1/2] " Jani Nikula
  2022-02-08 16:54 ` [Intel-gfx] [PATCH v4 2/2] drm/i915/vtd: rename functions to have the usual prefix Jani Nikula
@ 2022-02-08 18:06 ` Patchwork
  2022-02-08 18:07 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
  2022-02-08 18:39 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2022-02-08 18:06 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: split out intel_vtd.[ch] from i915_drv.h
URL   : https://patchwork.freedesktop.org/series/99852/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
019a148267a5 drm/i915: split out intel_vtd.[ch] from i915_drv.h
-:321: WARNING:FILE_PATH_CHANGES: added, moved or deleted file(s), does MAINTAINERS need updating?
#321: 
new file mode 100644

total: 0 errors, 1 warnings, 0 checks, 274 lines checked
f6d09243630e drm/i915/vtd: rename functions to have the usual prefix



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

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915: split out intel_vtd.[ch] from i915_drv.h
  2022-02-08 16:54 [Intel-gfx] [PATCH v4 0/2] drm/i915: split out intel_vtd.[ch] from i915_drv.h Jani Nikula
                   ` (2 preceding siblings ...)
  2022-02-08 18:06 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: split out intel_vtd.[ch] from i915_drv.h Patchwork
@ 2022-02-08 18:07 ` Patchwork
  2022-02-08 18:39 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2022-02-08 18:07 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: split out intel_vtd.[ch] from i915_drv.h
URL   : https://patchwork.freedesktop.org/series/99852/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.



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

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915: split out intel_vtd.[ch] from i915_drv.h
  2022-02-08 16:54 [Intel-gfx] [PATCH v4 0/2] drm/i915: split out intel_vtd.[ch] from i915_drv.h Jani Nikula
                   ` (3 preceding siblings ...)
  2022-02-08 18:07 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
@ 2022-02-08 18:39 ` Patchwork
  4 siblings, 0 replies; 9+ messages in thread
From: Patchwork @ 2022-02-08 18:39 UTC (permalink / raw)
  To: Jani Nikula; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 4239 bytes --]

== Series Details ==

Series: drm/i915: split out intel_vtd.[ch] from i915_drv.h
URL   : https://patchwork.freedesktop.org/series/99852/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_11203 -> Patchwork_22209
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_22209 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_22209, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22209/index.html

Participating hosts (45 -> 43)
------------------------------

  Missing    (2): shard-tglu fi-pnv-d510 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_22209:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@gt_lrc:
    - fi-rkl-guc:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11203/fi-rkl-guc/igt@i915_selftest@live@gt_lrc.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22209/fi-rkl-guc/igt@i915_selftest@live@gt_lrc.html

  * igt@i915_selftest@live@hangcheck:
    - fi-bdw-5557u:       NOTRUN -> [INCOMPLETE][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22209/fi-bdw-5557u/igt@i915_selftest@live@hangcheck.html

  
Known issues
------------

  Here are the changes found in Patchwork_22209 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@kms_chamelium@vga-edid-read:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][4] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22209/fi-bdw-5557u/igt@kms_chamelium@vga-edid-read.html

  * igt@kms_psr@cursor_plane_move:
    - fi-bdw-5557u:       NOTRUN -> [SKIP][5] ([fdo#109271]) +13 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22209/fi-bdw-5557u/igt@kms_psr@cursor_plane_move.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_heartbeat:
    - fi-skl-guc:         [DMESG-FAIL][6] ([i915#2291] / [i915#541]) -> [PASS][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11203/fi-skl-guc/igt@i915_selftest@live@gt_heartbeat.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22209/fi-skl-guc/igt@i915_selftest@live@gt_heartbeat.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-cml-u2:          [DMESG-WARN][8] ([i915#4269]) -> [PASS][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11203/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22209/fi-cml-u2/igt@kms_frontbuffer_tracking@basic.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#2291]: https://gitlab.freedesktop.org/drm/intel/issues/2291
  [i915#4269]: https://gitlab.freedesktop.org/drm/intel/issues/4269
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4897]: https://gitlab.freedesktop.org/drm/intel/issues/4897
  [i915#4898]: https://gitlab.freedesktop.org/drm/intel/issues/4898
  [i915#541]: https://gitlab.freedesktop.org/drm/intel/issues/541


Build changes
-------------

  * Linux: CI_DRM_11203 -> Patchwork_22209

  CI-20190529: 20190529
  CI_DRM_11203: e0f14f95759ad65e896868e1f9efd3247d93d28e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6341: a96674e747ea2f2431bbf8813156adc44ec3162a @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_22209: f6d09243630e3679c1fb7f6780748517fc0cbb19 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

f6d09243630e drm/i915/vtd: rename functions to have the usual prefix
019a148267a5 drm/i915: split out intel_vtd.[ch] from i915_drv.h

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22209/index.html

[-- Attachment #2: Type: text/html, Size: 4885 bytes --]

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

* Re: [Intel-gfx] [PATCH v4 1/2] drm/i915: split out intel_vtd.[ch] from i915_drv.h
  2022-02-08 16:54 ` [Intel-gfx] [PATCH v4 1/2] " Jani Nikula
@ 2022-02-09  9:40   ` Tvrtko Ursulin
  2022-02-09  9:51     ` Jani Nikula
  0 siblings, 1 reply; 9+ messages in thread
From: Tvrtko Ursulin @ 2022-02-09  9:40 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx


On 08/02/2022 16:54, Jani Nikula wrote:
> Perhaps a bit contrived, but we need to reduce the size of i915_drv.h
> from all the accumulated cruft.
> 
> v4: Rebase
> 
> v3: Rebase
> 
> v2: Turns out asm/hypervisor.h is not self-contained
> 
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>   drivers/gpu/drm/i915/Makefile                |  1 +
>   drivers/gpu/drm/i915/display/intel_bw.c      |  1 +
>   drivers/gpu/drm/i915/display/intel_display.c |  5 +-
>   drivers/gpu/drm/i915/display/intel_fb_pin.c  |  1 +
>   drivers/gpu/drm/i915/display/intel_fbc.c     |  1 +
>   drivers/gpu/drm/i915/gem/i915_gem_shrinker.c |  1 +
>   drivers/gpu/drm/i915/gem/i915_gem_stolen.c   |  1 +
>   drivers/gpu/drm/i915/gem/i915_gemfs.c        |  1 +
>   drivers/gpu/drm/i915/gt/intel_ggtt.c         |  8 +--
>   drivers/gpu/drm/i915/gt/intel_gtt.c          |  1 +
>   drivers/gpu/drm/i915/i915_debugfs.c          |  1 +
>   drivers/gpu/drm/i915/i915_driver.c           |  7 +--
>   drivers/gpu/drm/i915/i915_drv.h              | 37 --------------
>   drivers/gpu/drm/i915/i915_gpu_error.c        |  1 +
>   drivers/gpu/drm/i915/intel_device_info.c     |  3 +-
>   drivers/gpu/drm/i915/intel_pch.c             |  1 +
>   drivers/gpu/drm/i915/intel_pm.c              |  1 +
>   drivers/gpu/drm/i915/intel_vtd.c             | 14 ++++++
>   drivers/gpu/drm/i915/intel_vtd.h             | 51 ++++++++++++++++++++
>   19 files changed, 87 insertions(+), 50 deletions(-)
>   create mode 100644 drivers/gpu/drm/i915/intel_vtd.c
>   create mode 100644 drivers/gpu/drm/i915/intel_vtd.h
> 
> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
> index a26e6736bebb..d79c06b362f1 100644
> --- a/drivers/gpu/drm/i915/Makefile
> +++ b/drivers/gpu/drm/i915/Makefile
> @@ -55,6 +55,7 @@ i915-y += i915_driver.o \
>   	  intel_sbi.o \
>   	  intel_step.o \
>   	  intel_uncore.o \
> +	  intel_vtd.o \
>   	  intel_wakeref.o \
>   	  vlv_sideband.o \
>   	  vlv_suspend.o
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index 5dce3cf0ed12..edcf77d3694d 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -12,6 +12,7 @@
>   #include "intel_display_types.h"
>   #include "intel_pcode.h"
>   #include "intel_pm.h"
> +#include "intel_vtd.h"
>   
>   /* Parameters for Qclk Geyserville (QGV) */
>   struct intel_qgv_point {
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 7f512f9e9e5c..2af5eca2fe12 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -75,6 +75,7 @@
>   #include "g4x_dp.h"
>   #include "g4x_hdmi.h"
>   #include "i915_drv.h"
> +#include "i9xx_plane.h"
>   #include "icl_dsi.h"
>   #include "intel_acpi.h"
>   #include "intel_atomic.h"
> @@ -109,12 +110,12 @@
>   #include "intel_sprite.h"
>   #include "intel_tc.h"
>   #include "intel_vga.h"
> -#include "i9xx_plane.h"
> +#include "intel_vtd.h"
>   #include "skl_scaler.h"
>   #include "skl_universal_plane.h"
> +#include "vlv_dsi.h"
>   #include "vlv_dsi_pll.h"
>   #include "vlv_sideband.h"
> -#include "vlv_dsi.h"
>   
>   static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state);
>   static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c b/drivers/gpu/drm/i915/display/intel_fb_pin.c
> index c4b3d76341f3..05339b3bc39b 100644
> --- a/drivers/gpu/drm/i915/display/intel_fb_pin.c
> +++ b/drivers/gpu/drm/i915/display/intel_fb_pin.c
> @@ -14,6 +14,7 @@
>   #include "intel_dpt.h"
>   #include "intel_fb.h"
>   #include "intel_fb_pin.h"
> +#include "intel_vtd.h"
>   
>   static struct i915_vma *
>   intel_pin_fb_obj_dpt(struct drm_framebuffer *fb,
> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
> index bcdffe62f3cb..4c5a5aed7ca7 100644
> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
> @@ -48,6 +48,7 @@
>   #include "intel_display_types.h"
>   #include "intel_fbc.h"
>   #include "intel_frontbuffer.h"
> +#include "intel_vtd.h"
>   
>   #define for_each_fbc_id(__dev_priv, __fbc_id) \
>   	for ((__fbc_id) = INTEL_FBC_A; (__fbc_id) < I915_MAX_FBCS; (__fbc_id)++) \
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> index 6a6ff98a8746..0fb0921c81ab 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
> @@ -16,6 +16,7 @@
>   #include "gt/intel_gt_requests.h"
>   
>   #include "i915_trace.h"
> +#include "intel_vtd.h"
>   
>   static bool swap_available(void)
>   {
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> index 1de73a644965..361378846127 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
> @@ -16,6 +16,7 @@
>   #include "i915_gem_stolen.h"
>   #include "i915_reg.h"
>   #include "i915_vgpu.h"
> +#include "intel_vtd.h"
>   
>   /*
>    * The BIOS typically reserves some of the system's memory for the exclusive
> diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/i915/gem/i915_gemfs.c
> index 7271fbf813fa..7eb4b5ff9147 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gemfs.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gemfs.c
> @@ -9,6 +9,7 @@
>   
>   #include "i915_drv.h"
>   #include "i915_gemfs.h"
> +#include "intel_vtd.h"
>   
>   int i915_gemfs_init(struct drm_i915_private *i915)
>   {
> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> index 8850d4e0f9cc..e49d8d17095a 100644
> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
> @@ -14,14 +14,14 @@
>   
>   #include "gem/i915_gem_lmem.h"
>   
> -#include "intel_gt.h"
> -#include "intel_gt_regs.h"
> +#include "gen8_ppgtt.h"
>   #include "i915_drv.h"
>   #include "i915_scatterlist.h"
>   #include "i915_vgpu.h"
> -
> +#include "intel_gt.h"
> +#include "intel_gt_regs.h"
>   #include "intel_gtt.h"
> -#include "gen8_ppgtt.h"
> +#include "intel_vtd.h"
>   
>   static void i915_ggtt_color_adjust(const struct drm_mm_node *node,
>   				   unsigned long color,
> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
> index 0d6bbc8c57f2..524f3c3eb027 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gtt.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
> @@ -15,6 +15,7 @@
>   #include "intel_gt.h"
>   #include "intel_gt_regs.h"
>   #include "intel_gtt.h"
> +#include "intel_vtd.h"
>   
>   struct drm_i915_gem_object *alloc_pt_lmem(struct i915_address_space *vm, int sz)
>   {
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index ca52ee0742ce..001958238cfe 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -50,6 +50,7 @@
>   #include "i915_irq.h"
>   #include "i915_scheduler.h"
>   #include "intel_pm.h"
> +#include "intel_vtd.h"
>   
>   static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
>   {
> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
> index 3d41f532a5d6..32a3265a177a 100644
> --- a/drivers/gpu/drm/i915/i915_driver.c
> +++ b/drivers/gpu/drm/i915/i915_driver.c
> @@ -92,6 +92,7 @@
>   #include "intel_pcode.h"
>   #include "intel_pm.h"
>   #include "intel_region_ttm.h"
> +#include "intel_vtd.h"
>   #include "vlv_suspend.h"
>   
>   static const struct drm_driver i915_drm_driver;
> @@ -746,12 +747,6 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>   	i915_gem_driver_unregister(dev_priv);
>   }
>   
> -void
> -i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p)
> -{
> -	drm_printf(p, "iommu: %s\n", enableddisabled(intel_vtd_active(i915)));
> -}
> -
>   static void i915_welcome_messages(struct drm_i915_private *dev_priv)
>   {
>   	if (drm_debug_enabled(DRM_UT_DRIVER)) {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8c1706fd81f9..b82528ebe70a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -33,14 +33,11 @@
>   #include <uapi/drm/i915_drm.h>
>   #include <uapi/drm/drm_fourcc.h>
>   
> -#include <asm/hypervisor.h>
> -
>   #include <linux/io-mapping.h>
>   #include <linux/i2c.h>
>   #include <linux/i2c-algo-bit.h>
>   #include <linux/backlight.h>
>   #include <linux/hash.h>
> -#include <linux/intel-iommu.h>
>   #include <linux/kref.h>
>   #include <linux/mm_types.h>
>   #include <linux/perf_event.h>
> @@ -1562,43 +1559,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>   #define HAS_GUC_DEPRIVILEGE(dev_priv) \
>   	(INTEL_INFO(dev_priv)->has_guc_deprivilege)
>   
> -static inline bool run_as_guest(void)
> -{
> -	return !hypervisor_is_type(X86_HYPER_NATIVE);
> -}
> -
>   #define HAS_D12_PLANE_MINIMIZATION(dev_priv) (IS_ROCKETLAKE(dev_priv) || \
>   					      IS_ALDERLAKE_S(dev_priv))
>   
> -static inline bool intel_vtd_active(struct drm_i915_private *i915)
> -{
> -	if (device_iommu_mapped(i915->drm.dev))
> -		return true;
> -
> -	/* Running as a guest, we assume the host is enforcing VT'd */
> -	return run_as_guest();
> -}
> -
> -void
> -i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p);
> -
> -static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv)
> -{
> -	return DISPLAY_VER(dev_priv) >= 6 && intel_vtd_active(dev_priv);
> -}
> -
> -static inline bool
> -intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)
> -{
> -	return IS_BROXTON(i915) && intel_vtd_active(i915);
> -}
> -
> -static inline bool
> -intel_vm_no_concurrent_access_wa(struct drm_i915_private *i915)
> -{
> -	return IS_CHERRYVIEW(i915) || intel_ggtt_update_needs_vtd_wa(i915);
> -}
> -
>   /* i915_gem.c */
>   void i915_gem_init_early(struct drm_i915_private *dev_priv);
>   void i915_gem_cleanup_early(struct drm_i915_private *dev_priv);
> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
> index 127ff56c8ce6..ccb6d62a51df 100644
> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
> @@ -50,6 +50,7 @@
>   #include "i915_gpu_error.h"
>   #include "i915_memcpy.h"
>   #include "i915_scatterlist.h"
> +#include "intel_vtd.h"
>   
>   #define ALLOW_FAIL (__GFP_KSWAPD_RECLAIM | __GFP_RETRY_MAYFAIL | __GFP_NOWARN)
>   #define ATOMIC_MAYFAIL (GFP_ATOMIC | __GFP_NOWARN)
> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
> index 94da5aa37391..13abae12cf10 100644
> --- a/drivers/gpu/drm/i915/intel_device_info.c
> +++ b/drivers/gpu/drm/i915/intel_device_info.c
> @@ -27,8 +27,9 @@
>   
>   #include "display/intel_cdclk.h"
>   #include "display/intel_de.h"
> -#include "intel_device_info.h"
>   #include "i915_drv.h"
> +#include "intel_device_info.h"
> +#include "intel_vtd.h"
>   
>   #define PLATFORM_NAME(x) [INTEL_##x] = #x
>   static const char * const platform_names[] = {
> diff --git a/drivers/gpu/drm/i915/intel_pch.c b/drivers/gpu/drm/i915/intel_pch.c
> index da8f82c2342f..4d8a98dbf465 100644
> --- a/drivers/gpu/drm/i915/intel_pch.c
> +++ b/drivers/gpu/drm/i915/intel_pch.c
> @@ -5,6 +5,7 @@
>   
>   #include "i915_drv.h"
>   #include "intel_pch.h"
> +#include "intel_vtd.h"
>   
>   /* Map PCH device id to PCH type, or PCH_NONE if unknown. */
>   static enum intel_pch
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 23d4bb011fc8..23f0e2473e27 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -52,6 +52,7 @@
>   #include "i915_irq.h"
>   #include "intel_pcode.h"
>   #include "intel_pm.h"
> +#include "intel_vtd.h"
>   #include "vlv_sideband.h"
>   #include "../../../platform/x86/intel_ips.h"
>   
> diff --git a/drivers/gpu/drm/i915/intel_vtd.c b/drivers/gpu/drm/i915/intel_vtd.c
> new file mode 100644
> index 000000000000..e52432141081
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_vtd.c
> @@ -0,0 +1,14 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#include <drm/drm_print.h>
> +
> +#include "intel_vtd.h"
> +
> +void
> +i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p)
> +{
> +	drm_printf(p, "iommu: %s\n", enableddisabled(intel_vtd_active(i915)));
> +}
> diff --git a/drivers/gpu/drm/i915/intel_vtd.h b/drivers/gpu/drm/i915/intel_vtd.h
> new file mode 100644
> index 000000000000..421bf8ca0b36
> --- /dev/null
> +++ b/drivers/gpu/drm/i915/intel_vtd.h
> @@ -0,0 +1,51 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2022 Intel Corporation
> + */
> +
> +#ifndef __INTEL_VTD_H__
> +#define __INTEL_VTD_H__
> +
> +#include <linux/device.h>
> +#include <linux/types.h>
> +#include <asm/hypervisor.h>
> +
> +#include "i915_drv.h"
> +
> +struct drm_printer;
> +
> +static inline bool run_as_guest(void)
> +{
> +	return !hypervisor_is_type(X86_HYPER_NATIVE);
> +}
> +
> +static inline bool intel_vtd_active(struct drm_i915_private *i915)
> +{
> +	if (device_iommu_mapped(i915->drm.dev))
> +		return true;
> +
> +	/* Running as a guest, we assume the host is enforcing VT'd */
> +	return run_as_guest();
> +}

How about make this one take struct device and then intel_vtd.h does not 
have to include i915_drv.h back in? Consider also..

> +
> +static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *i915)
> +{
> +	return GRAPHICS_VER(i915) >= 6 && intel_vtd_active(i915);
> +}
> +
> +static inline bool
> +intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)
> +{
> +	return IS_BROXTON(i915) && intel_vtd_active(i915);
> +}
> +
> +static inline bool
> +intel_vm_no_concurrent_access_wa(struct drm_i915_private *i915)
> +{
> +	return IS_CHERRYVIEW(i915) || intel_ggtt_update_needs_vtd_wa(i915);
> +}
> +
> +void
> +i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p);

.. that I think all the rest feel a bit out of place in intel_vtd.[hc] 
to me. They all take i915 as argument and answer the question on whether 
i915 needs something, depending on the state of the iommu. Especially 
when you add the intel_vtd_ prefix to them in the next patch it gets 
extra weird for me.

In my mind, in the ideal world, they would live somewhere else and 
include intel_vtd.h so they can legimately answer the question at hand. 
First somewhere in display, 2nd and 3rd probably in intel_gtt.h, last in 
i915_driver. How does that sound to you?

Regards,

Tvrtko

> +
> +#endif /* __INTEL_VTD_H__ */

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

* Re: [Intel-gfx] [PATCH v4 1/2] drm/i915: split out intel_vtd.[ch] from i915_drv.h
  2022-02-09  9:40   ` Tvrtko Ursulin
@ 2022-02-09  9:51     ` Jani Nikula
  2022-02-09 10:01       ` Tvrtko Ursulin
  0 siblings, 1 reply; 9+ messages in thread
From: Jani Nikula @ 2022-02-09  9:51 UTC (permalink / raw)
  To: Tvrtko Ursulin, intel-gfx

On Wed, 09 Feb 2022, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
> On 08/02/2022 16:54, Jani Nikula wrote:
>> Perhaps a bit contrived, but we need to reduce the size of i915_drv.h
>> from all the accumulated cruft.
>> 
>> v4: Rebase
>> 
>> v3: Rebase
>> 
>> v2: Turns out asm/hypervisor.h is not self-contained
>> 
>> Cc: Daniel Vetter <daniel@ffwll.ch>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>   drivers/gpu/drm/i915/Makefile                |  1 +
>>   drivers/gpu/drm/i915/display/intel_bw.c      |  1 +
>>   drivers/gpu/drm/i915/display/intel_display.c |  5 +-
>>   drivers/gpu/drm/i915/display/intel_fb_pin.c  |  1 +
>>   drivers/gpu/drm/i915/display/intel_fbc.c     |  1 +
>>   drivers/gpu/drm/i915/gem/i915_gem_shrinker.c |  1 +
>>   drivers/gpu/drm/i915/gem/i915_gem_stolen.c   |  1 +
>>   drivers/gpu/drm/i915/gem/i915_gemfs.c        |  1 +
>>   drivers/gpu/drm/i915/gt/intel_ggtt.c         |  8 +--
>>   drivers/gpu/drm/i915/gt/intel_gtt.c          |  1 +
>>   drivers/gpu/drm/i915/i915_debugfs.c          |  1 +
>>   drivers/gpu/drm/i915/i915_driver.c           |  7 +--
>>   drivers/gpu/drm/i915/i915_drv.h              | 37 --------------
>>   drivers/gpu/drm/i915/i915_gpu_error.c        |  1 +
>>   drivers/gpu/drm/i915/intel_device_info.c     |  3 +-
>>   drivers/gpu/drm/i915/intel_pch.c             |  1 +
>>   drivers/gpu/drm/i915/intel_pm.c              |  1 +
>>   drivers/gpu/drm/i915/intel_vtd.c             | 14 ++++++
>>   drivers/gpu/drm/i915/intel_vtd.h             | 51 ++++++++++++++++++++
>>   19 files changed, 87 insertions(+), 50 deletions(-)
>>   create mode 100644 drivers/gpu/drm/i915/intel_vtd.c
>>   create mode 100644 drivers/gpu/drm/i915/intel_vtd.h
>> 
>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>> index a26e6736bebb..d79c06b362f1 100644
>> --- a/drivers/gpu/drm/i915/Makefile
>> +++ b/drivers/gpu/drm/i915/Makefile
>> @@ -55,6 +55,7 @@ i915-y += i915_driver.o \
>>   	  intel_sbi.o \
>>   	  intel_step.o \
>>   	  intel_uncore.o \
>> +	  intel_vtd.o \
>>   	  intel_wakeref.o \
>>   	  vlv_sideband.o \
>>   	  vlv_suspend.o
>> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
>> index 5dce3cf0ed12..edcf77d3694d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bw.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
>> @@ -12,6 +12,7 @@
>>   #include "intel_display_types.h"
>>   #include "intel_pcode.h"
>>   #include "intel_pm.h"
>> +#include "intel_vtd.h"
>>   
>>   /* Parameters for Qclk Geyserville (QGV) */
>>   struct intel_qgv_point {
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 7f512f9e9e5c..2af5eca2fe12 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -75,6 +75,7 @@
>>   #include "g4x_dp.h"
>>   #include "g4x_hdmi.h"
>>   #include "i915_drv.h"
>> +#include "i9xx_plane.h"
>>   #include "icl_dsi.h"
>>   #include "intel_acpi.h"
>>   #include "intel_atomic.h"
>> @@ -109,12 +110,12 @@
>>   #include "intel_sprite.h"
>>   #include "intel_tc.h"
>>   #include "intel_vga.h"
>> -#include "i9xx_plane.h"
>> +#include "intel_vtd.h"
>>   #include "skl_scaler.h"
>>   #include "skl_universal_plane.h"
>> +#include "vlv_dsi.h"
>>   #include "vlv_dsi_pll.h"
>>   #include "vlv_sideband.h"
>> -#include "vlv_dsi.h"
>>   
>>   static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state);
>>   static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state);
>> diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c b/drivers/gpu/drm/i915/display/intel_fb_pin.c
>> index c4b3d76341f3..05339b3bc39b 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fb_pin.c
>> +++ b/drivers/gpu/drm/i915/display/intel_fb_pin.c
>> @@ -14,6 +14,7 @@
>>   #include "intel_dpt.h"
>>   #include "intel_fb.h"
>>   #include "intel_fb_pin.h"
>> +#include "intel_vtd.h"
>>   
>>   static struct i915_vma *
>>   intel_pin_fb_obj_dpt(struct drm_framebuffer *fb,
>> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
>> index bcdffe62f3cb..4c5a5aed7ca7 100644
>> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
>> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
>> @@ -48,6 +48,7 @@
>>   #include "intel_display_types.h"
>>   #include "intel_fbc.h"
>>   #include "intel_frontbuffer.h"
>> +#include "intel_vtd.h"
>>   
>>   #define for_each_fbc_id(__dev_priv, __fbc_id) \
>>   	for ((__fbc_id) = INTEL_FBC_A; (__fbc_id) < I915_MAX_FBCS; (__fbc_id)++) \
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
>> index 6a6ff98a8746..0fb0921c81ab 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
>> @@ -16,6 +16,7 @@
>>   #include "gt/intel_gt_requests.h"
>>   
>>   #include "i915_trace.h"
>> +#include "intel_vtd.h"
>>   
>>   static bool swap_available(void)
>>   {
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>> index 1de73a644965..361378846127 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>> @@ -16,6 +16,7 @@
>>   #include "i915_gem_stolen.h"
>>   #include "i915_reg.h"
>>   #include "i915_vgpu.h"
>> +#include "intel_vtd.h"
>>   
>>   /*
>>    * The BIOS typically reserves some of the system's memory for the exclusive
>> diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/i915/gem/i915_gemfs.c
>> index 7271fbf813fa..7eb4b5ff9147 100644
>> --- a/drivers/gpu/drm/i915/gem/i915_gemfs.c
>> +++ b/drivers/gpu/drm/i915/gem/i915_gemfs.c
>> @@ -9,6 +9,7 @@
>>   
>>   #include "i915_drv.h"
>>   #include "i915_gemfs.h"
>> +#include "intel_vtd.h"
>>   
>>   int i915_gemfs_init(struct drm_i915_private *i915)
>>   {
>> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
>> index 8850d4e0f9cc..e49d8d17095a 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
>> @@ -14,14 +14,14 @@
>>   
>>   #include "gem/i915_gem_lmem.h"
>>   
>> -#include "intel_gt.h"
>> -#include "intel_gt_regs.h"
>> +#include "gen8_ppgtt.h"
>>   #include "i915_drv.h"
>>   #include "i915_scatterlist.h"
>>   #include "i915_vgpu.h"
>> -
>> +#include "intel_gt.h"
>> +#include "intel_gt_regs.h"
>>   #include "intel_gtt.h"
>> -#include "gen8_ppgtt.h"
>> +#include "intel_vtd.h"
>>   
>>   static void i915_ggtt_color_adjust(const struct drm_mm_node *node,
>>   				   unsigned long color,
>> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
>> index 0d6bbc8c57f2..524f3c3eb027 100644
>> --- a/drivers/gpu/drm/i915/gt/intel_gtt.c
>> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
>> @@ -15,6 +15,7 @@
>>   #include "intel_gt.h"
>>   #include "intel_gt_regs.h"
>>   #include "intel_gtt.h"
>> +#include "intel_vtd.h"
>>   
>>   struct drm_i915_gem_object *alloc_pt_lmem(struct i915_address_space *vm, int sz)
>>   {
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index ca52ee0742ce..001958238cfe 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -50,6 +50,7 @@
>>   #include "i915_irq.h"
>>   #include "i915_scheduler.h"
>>   #include "intel_pm.h"
>> +#include "intel_vtd.h"
>>   
>>   static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
>>   {
>> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>> index 3d41f532a5d6..32a3265a177a 100644
>> --- a/drivers/gpu/drm/i915/i915_driver.c
>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>> @@ -92,6 +92,7 @@
>>   #include "intel_pcode.h"
>>   #include "intel_pm.h"
>>   #include "intel_region_ttm.h"
>> +#include "intel_vtd.h"
>>   #include "vlv_suspend.h"
>>   
>>   static const struct drm_driver i915_drm_driver;
>> @@ -746,12 +747,6 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>>   	i915_gem_driver_unregister(dev_priv);
>>   }
>>   
>> -void
>> -i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p)
>> -{
>> -	drm_printf(p, "iommu: %s\n", enableddisabled(intel_vtd_active(i915)));
>> -}
>> -
>>   static void i915_welcome_messages(struct drm_i915_private *dev_priv)
>>   {
>>   	if (drm_debug_enabled(DRM_UT_DRIVER)) {
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 8c1706fd81f9..b82528ebe70a 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -33,14 +33,11 @@
>>   #include <uapi/drm/i915_drm.h>
>>   #include <uapi/drm/drm_fourcc.h>
>>   
>> -#include <asm/hypervisor.h>
>> -
>>   #include <linux/io-mapping.h>
>>   #include <linux/i2c.h>
>>   #include <linux/i2c-algo-bit.h>
>>   #include <linux/backlight.h>
>>   #include <linux/hash.h>
>> -#include <linux/intel-iommu.h>
>>   #include <linux/kref.h>
>>   #include <linux/mm_types.h>
>>   #include <linux/perf_event.h>
>> @@ -1562,43 +1559,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>>   #define HAS_GUC_DEPRIVILEGE(dev_priv) \
>>   	(INTEL_INFO(dev_priv)->has_guc_deprivilege)
>>   
>> -static inline bool run_as_guest(void)
>> -{
>> -	return !hypervisor_is_type(X86_HYPER_NATIVE);
>> -}
>> -
>>   #define HAS_D12_PLANE_MINIMIZATION(dev_priv) (IS_ROCKETLAKE(dev_priv) || \
>>   					      IS_ALDERLAKE_S(dev_priv))
>>   
>> -static inline bool intel_vtd_active(struct drm_i915_private *i915)
>> -{
>> -	if (device_iommu_mapped(i915->drm.dev))
>> -		return true;
>> -
>> -	/* Running as a guest, we assume the host is enforcing VT'd */
>> -	return run_as_guest();
>> -}
>> -
>> -void
>> -i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p);
>> -
>> -static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv)
>> -{
>> -	return DISPLAY_VER(dev_priv) >= 6 && intel_vtd_active(dev_priv);
>> -}
>> -
>> -static inline bool
>> -intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)
>> -{
>> -	return IS_BROXTON(i915) && intel_vtd_active(i915);
>> -}
>> -
>> -static inline bool
>> -intel_vm_no_concurrent_access_wa(struct drm_i915_private *i915)
>> -{
>> -	return IS_CHERRYVIEW(i915) || intel_ggtt_update_needs_vtd_wa(i915);
>> -}
>> -
>>   /* i915_gem.c */
>>   void i915_gem_init_early(struct drm_i915_private *dev_priv);
>>   void i915_gem_cleanup_early(struct drm_i915_private *dev_priv);
>> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
>> index 127ff56c8ce6..ccb6d62a51df 100644
>> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
>> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
>> @@ -50,6 +50,7 @@
>>   #include "i915_gpu_error.h"
>>   #include "i915_memcpy.h"
>>   #include "i915_scatterlist.h"
>> +#include "intel_vtd.h"
>>   
>>   #define ALLOW_FAIL (__GFP_KSWAPD_RECLAIM | __GFP_RETRY_MAYFAIL | __GFP_NOWARN)
>>   #define ATOMIC_MAYFAIL (GFP_ATOMIC | __GFP_NOWARN)
>> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
>> index 94da5aa37391..13abae12cf10 100644
>> --- a/drivers/gpu/drm/i915/intel_device_info.c
>> +++ b/drivers/gpu/drm/i915/intel_device_info.c
>> @@ -27,8 +27,9 @@
>>   
>>   #include "display/intel_cdclk.h"
>>   #include "display/intel_de.h"
>> -#include "intel_device_info.h"
>>   #include "i915_drv.h"
>> +#include "intel_device_info.h"
>> +#include "intel_vtd.h"
>>   
>>   #define PLATFORM_NAME(x) [INTEL_##x] = #x
>>   static const char * const platform_names[] = {
>> diff --git a/drivers/gpu/drm/i915/intel_pch.c b/drivers/gpu/drm/i915/intel_pch.c
>> index da8f82c2342f..4d8a98dbf465 100644
>> --- a/drivers/gpu/drm/i915/intel_pch.c
>> +++ b/drivers/gpu/drm/i915/intel_pch.c
>> @@ -5,6 +5,7 @@
>>   
>>   #include "i915_drv.h"
>>   #include "intel_pch.h"
>> +#include "intel_vtd.h"
>>   
>>   /* Map PCH device id to PCH type, or PCH_NONE if unknown. */
>>   static enum intel_pch
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 23d4bb011fc8..23f0e2473e27 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -52,6 +52,7 @@
>>   #include "i915_irq.h"
>>   #include "intel_pcode.h"
>>   #include "intel_pm.h"
>> +#include "intel_vtd.h"
>>   #include "vlv_sideband.h"
>>   #include "../../../platform/x86/intel_ips.h"
>>   
>> diff --git a/drivers/gpu/drm/i915/intel_vtd.c b/drivers/gpu/drm/i915/intel_vtd.c
>> new file mode 100644
>> index 000000000000..e52432141081
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_vtd.c
>> @@ -0,0 +1,14 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2022 Intel Corporation
>> + */
>> +
>> +#include <drm/drm_print.h>
>> +
>> +#include "intel_vtd.h"
>> +
>> +void
>> +i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p)
>> +{
>> +	drm_printf(p, "iommu: %s\n", enableddisabled(intel_vtd_active(i915)));
>> +}
>> diff --git a/drivers/gpu/drm/i915/intel_vtd.h b/drivers/gpu/drm/i915/intel_vtd.h
>> new file mode 100644
>> index 000000000000..421bf8ca0b36
>> --- /dev/null
>> +++ b/drivers/gpu/drm/i915/intel_vtd.h
>> @@ -0,0 +1,51 @@
>> +/* SPDX-License-Identifier: MIT */
>> +/*
>> + * Copyright © 2022 Intel Corporation
>> + */
>> +
>> +#ifndef __INTEL_VTD_H__
>> +#define __INTEL_VTD_H__
>> +
>> +#include <linux/device.h>
>> +#include <linux/types.h>
>> +#include <asm/hypervisor.h>
>> +
>> +#include "i915_drv.h"
>> +
>> +struct drm_printer;
>> +
>> +static inline bool run_as_guest(void)
>> +{
>> +	return !hypervisor_is_type(X86_HYPER_NATIVE);
>> +}
>> +
>> +static inline bool intel_vtd_active(struct drm_i915_private *i915)
>> +{
>> +	if (device_iommu_mapped(i915->drm.dev))
>> +		return true;
>> +
>> +	/* Running as a guest, we assume the host is enforcing VT'd */
>> +	return run_as_guest();
>> +}
>
> How about make this one take struct device and then intel_vtd.h does not 
> have to include i915_drv.h back in? Consider also..
>
>> +
>> +static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *i915)
>> +{
>> +	return GRAPHICS_VER(i915) >= 6 && intel_vtd_active(i915);
>> +}
>> +
>> +static inline bool
>> +intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)
>> +{
>> +	return IS_BROXTON(i915) && intel_vtd_active(i915);
>> +}
>> +
>> +static inline bool
>> +intel_vm_no_concurrent_access_wa(struct drm_i915_private *i915)
>> +{
>> +	return IS_CHERRYVIEW(i915) || intel_ggtt_update_needs_vtd_wa(i915);
>> +}
>> +
>> +void
>> +i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p);
>
> .. that I think all the rest feel a bit out of place in intel_vtd.[hc] 
> to me. They all take i915 as argument and answer the question on whether 
> i915 needs something, depending on the state of the iommu. Especially 
> when you add the intel_vtd_ prefix to them in the next patch it gets 
> extra weird for me.
>
> In my mind, in the ideal world, they would live somewhere else and 
> include intel_vtd.h so they can legimately answer the question at hand. 
> First somewhere in display, 2nd and 3rd probably in intel_gtt.h, last in 
> i915_driver. How does that sound to you?

I'm fine with that. Like I start the commit message, "Perhaps a bit
contrived". All I want is to axe cruft out of i915_drv.h.

So we'd still have intel_vtd_run_as_guest() and intel_vtd_active() in
intel_vtd.h?

BR,
Jani.



>
> Regards,
>
> Tvrtko
>
>> +
>> +#endif /* __INTEL_VTD_H__ */

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [Intel-gfx] [PATCH v4 1/2] drm/i915: split out intel_vtd.[ch] from i915_drv.h
  2022-02-09  9:51     ` Jani Nikula
@ 2022-02-09 10:01       ` Tvrtko Ursulin
  0 siblings, 0 replies; 9+ messages in thread
From: Tvrtko Ursulin @ 2022-02-09 10:01 UTC (permalink / raw)
  To: Jani Nikula, intel-gfx


On 09/02/2022 09:51, Jani Nikula wrote:
> On Wed, 09 Feb 2022, Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> wrote:
>> On 08/02/2022 16:54, Jani Nikula wrote:
>>> Perhaps a bit contrived, but we need to reduce the size of i915_drv.h
>>> from all the accumulated cruft.
>>>
>>> v4: Rebase
>>>
>>> v3: Rebase
>>>
>>> v2: Turns out asm/hypervisor.h is not self-contained
>>>
>>> Cc: Daniel Vetter <daniel@ffwll.ch>
>>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
>>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/Makefile                |  1 +
>>>    drivers/gpu/drm/i915/display/intel_bw.c      |  1 +
>>>    drivers/gpu/drm/i915/display/intel_display.c |  5 +-
>>>    drivers/gpu/drm/i915/display/intel_fb_pin.c  |  1 +
>>>    drivers/gpu/drm/i915/display/intel_fbc.c     |  1 +
>>>    drivers/gpu/drm/i915/gem/i915_gem_shrinker.c |  1 +
>>>    drivers/gpu/drm/i915/gem/i915_gem_stolen.c   |  1 +
>>>    drivers/gpu/drm/i915/gem/i915_gemfs.c        |  1 +
>>>    drivers/gpu/drm/i915/gt/intel_ggtt.c         |  8 +--
>>>    drivers/gpu/drm/i915/gt/intel_gtt.c          |  1 +
>>>    drivers/gpu/drm/i915/i915_debugfs.c          |  1 +
>>>    drivers/gpu/drm/i915/i915_driver.c           |  7 +--
>>>    drivers/gpu/drm/i915/i915_drv.h              | 37 --------------
>>>    drivers/gpu/drm/i915/i915_gpu_error.c        |  1 +
>>>    drivers/gpu/drm/i915/intel_device_info.c     |  3 +-
>>>    drivers/gpu/drm/i915/intel_pch.c             |  1 +
>>>    drivers/gpu/drm/i915/intel_pm.c              |  1 +
>>>    drivers/gpu/drm/i915/intel_vtd.c             | 14 ++++++
>>>    drivers/gpu/drm/i915/intel_vtd.h             | 51 ++++++++++++++++++++
>>>    19 files changed, 87 insertions(+), 50 deletions(-)
>>>    create mode 100644 drivers/gpu/drm/i915/intel_vtd.c
>>>    create mode 100644 drivers/gpu/drm/i915/intel_vtd.h
>>>
>>> diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
>>> index a26e6736bebb..d79c06b362f1 100644
>>> --- a/drivers/gpu/drm/i915/Makefile
>>> +++ b/drivers/gpu/drm/i915/Makefile
>>> @@ -55,6 +55,7 @@ i915-y += i915_driver.o \
>>>    	  intel_sbi.o \
>>>    	  intel_step.o \
>>>    	  intel_uncore.o \
>>> +	  intel_vtd.o \
>>>    	  intel_wakeref.o \
>>>    	  vlv_sideband.o \
>>>    	  vlv_suspend.o
>>> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
>>> index 5dce3cf0ed12..edcf77d3694d 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_bw.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
>>> @@ -12,6 +12,7 @@
>>>    #include "intel_display_types.h"
>>>    #include "intel_pcode.h"
>>>    #include "intel_pm.h"
>>> +#include "intel_vtd.h"
>>>    
>>>    /* Parameters for Qclk Geyserville (QGV) */
>>>    struct intel_qgv_point {
>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>>> index 7f512f9e9e5c..2af5eca2fe12 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>> @@ -75,6 +75,7 @@
>>>    #include "g4x_dp.h"
>>>    #include "g4x_hdmi.h"
>>>    #include "i915_drv.h"
>>> +#include "i9xx_plane.h"
>>>    #include "icl_dsi.h"
>>>    #include "intel_acpi.h"
>>>    #include "intel_atomic.h"
>>> @@ -109,12 +110,12 @@
>>>    #include "intel_sprite.h"
>>>    #include "intel_tc.h"
>>>    #include "intel_vga.h"
>>> -#include "i9xx_plane.h"
>>> +#include "intel_vtd.h"
>>>    #include "skl_scaler.h"
>>>    #include "skl_universal_plane.h"
>>> +#include "vlv_dsi.h"
>>>    #include "vlv_dsi_pll.h"
>>>    #include "vlv_sideband.h"
>>> -#include "vlv_dsi.h"
>>>    
>>>    static void intel_set_transcoder_timings(const struct intel_crtc_state *crtc_state);
>>>    static void intel_set_pipe_src_size(const struct intel_crtc_state *crtc_state);
>>> diff --git a/drivers/gpu/drm/i915/display/intel_fb_pin.c b/drivers/gpu/drm/i915/display/intel_fb_pin.c
>>> index c4b3d76341f3..05339b3bc39b 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_fb_pin.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_fb_pin.c
>>> @@ -14,6 +14,7 @@
>>>    #include "intel_dpt.h"
>>>    #include "intel_fb.h"
>>>    #include "intel_fb_pin.h"
>>> +#include "intel_vtd.h"
>>>    
>>>    static struct i915_vma *
>>>    intel_pin_fb_obj_dpt(struct drm_framebuffer *fb,
>>> diff --git a/drivers/gpu/drm/i915/display/intel_fbc.c b/drivers/gpu/drm/i915/display/intel_fbc.c
>>> index bcdffe62f3cb..4c5a5aed7ca7 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_fbc.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_fbc.c
>>> @@ -48,6 +48,7 @@
>>>    #include "intel_display_types.h"
>>>    #include "intel_fbc.h"
>>>    #include "intel_frontbuffer.h"
>>> +#include "intel_vtd.h"
>>>    
>>>    #define for_each_fbc_id(__dev_priv, __fbc_id) \
>>>    	for ((__fbc_id) = INTEL_FBC_A; (__fbc_id) < I915_MAX_FBCS; (__fbc_id)++) \
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
>>> index 6a6ff98a8746..0fb0921c81ab 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_shrinker.c
>>> @@ -16,6 +16,7 @@
>>>    #include "gt/intel_gt_requests.h"
>>>    
>>>    #include "i915_trace.h"
>>> +#include "intel_vtd.h"
>>>    
>>>    static bool swap_available(void)
>>>    {
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>>> index 1de73a644965..361378846127 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gem_stolen.c
>>> @@ -16,6 +16,7 @@
>>>    #include "i915_gem_stolen.h"
>>>    #include "i915_reg.h"
>>>    #include "i915_vgpu.h"
>>> +#include "intel_vtd.h"
>>>    
>>>    /*
>>>     * The BIOS typically reserves some of the system's memory for the exclusive
>>> diff --git a/drivers/gpu/drm/i915/gem/i915_gemfs.c b/drivers/gpu/drm/i915/gem/i915_gemfs.c
>>> index 7271fbf813fa..7eb4b5ff9147 100644
>>> --- a/drivers/gpu/drm/i915/gem/i915_gemfs.c
>>> +++ b/drivers/gpu/drm/i915/gem/i915_gemfs.c
>>> @@ -9,6 +9,7 @@
>>>    
>>>    #include "i915_drv.h"
>>>    #include "i915_gemfs.h"
>>> +#include "intel_vtd.h"
>>>    
>>>    int i915_gemfs_init(struct drm_i915_private *i915)
>>>    {
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c
>>> index 8850d4e0f9cc..e49d8d17095a 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
>>> @@ -14,14 +14,14 @@
>>>    
>>>    #include "gem/i915_gem_lmem.h"
>>>    
>>> -#include "intel_gt.h"
>>> -#include "intel_gt_regs.h"
>>> +#include "gen8_ppgtt.h"
>>>    #include "i915_drv.h"
>>>    #include "i915_scatterlist.h"
>>>    #include "i915_vgpu.h"
>>> -
>>> +#include "intel_gt.h"
>>> +#include "intel_gt_regs.h"
>>>    #include "intel_gtt.h"
>>> -#include "gen8_ppgtt.h"
>>> +#include "intel_vtd.h"
>>>    
>>>    static void i915_ggtt_color_adjust(const struct drm_mm_node *node,
>>>    				   unsigned long color,
>>> diff --git a/drivers/gpu/drm/i915/gt/intel_gtt.c b/drivers/gpu/drm/i915/gt/intel_gtt.c
>>> index 0d6bbc8c57f2..524f3c3eb027 100644
>>> --- a/drivers/gpu/drm/i915/gt/intel_gtt.c
>>> +++ b/drivers/gpu/drm/i915/gt/intel_gtt.c
>>> @@ -15,6 +15,7 @@
>>>    #include "intel_gt.h"
>>>    #include "intel_gt_regs.h"
>>>    #include "intel_gtt.h"
>>> +#include "intel_vtd.h"
>>>    
>>>    struct drm_i915_gem_object *alloc_pt_lmem(struct i915_address_space *vm, int sz)
>>>    {
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index ca52ee0742ce..001958238cfe 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -50,6 +50,7 @@
>>>    #include "i915_irq.h"
>>>    #include "i915_scheduler.h"
>>>    #include "intel_pm.h"
>>> +#include "intel_vtd.h"
>>>    
>>>    static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
>>>    {
>>> diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c
>>> index 3d41f532a5d6..32a3265a177a 100644
>>> --- a/drivers/gpu/drm/i915/i915_driver.c
>>> +++ b/drivers/gpu/drm/i915/i915_driver.c
>>> @@ -92,6 +92,7 @@
>>>    #include "intel_pcode.h"
>>>    #include "intel_pm.h"
>>>    #include "intel_region_ttm.h"
>>> +#include "intel_vtd.h"
>>>    #include "vlv_suspend.h"
>>>    
>>>    static const struct drm_driver i915_drm_driver;
>>> @@ -746,12 +747,6 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
>>>    	i915_gem_driver_unregister(dev_priv);
>>>    }
>>>    
>>> -void
>>> -i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p)
>>> -{
>>> -	drm_printf(p, "iommu: %s\n", enableddisabled(intel_vtd_active(i915)));
>>> -}
>>> -
>>>    static void i915_welcome_messages(struct drm_i915_private *dev_priv)
>>>    {
>>>    	if (drm_debug_enabled(DRM_UT_DRIVER)) {
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> index 8c1706fd81f9..b82528ebe70a 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -33,14 +33,11 @@
>>>    #include <uapi/drm/i915_drm.h>
>>>    #include <uapi/drm/drm_fourcc.h>
>>>    
>>> -#include <asm/hypervisor.h>
>>> -
>>>    #include <linux/io-mapping.h>
>>>    #include <linux/i2c.h>
>>>    #include <linux/i2c-algo-bit.h>
>>>    #include <linux/backlight.h>
>>>    #include <linux/hash.h>
>>> -#include <linux/intel-iommu.h>
>>>    #include <linux/kref.h>
>>>    #include <linux/mm_types.h>
>>>    #include <linux/perf_event.h>
>>> @@ -1562,43 +1559,9 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>>>    #define HAS_GUC_DEPRIVILEGE(dev_priv) \
>>>    	(INTEL_INFO(dev_priv)->has_guc_deprivilege)
>>>    
>>> -static inline bool run_as_guest(void)
>>> -{
>>> -	return !hypervisor_is_type(X86_HYPER_NATIVE);
>>> -}
>>> -
>>>    #define HAS_D12_PLANE_MINIMIZATION(dev_priv) (IS_ROCKETLAKE(dev_priv) || \
>>>    					      IS_ALDERLAKE_S(dev_priv))
>>>    
>>> -static inline bool intel_vtd_active(struct drm_i915_private *i915)
>>> -{
>>> -	if (device_iommu_mapped(i915->drm.dev))
>>> -		return true;
>>> -
>>> -	/* Running as a guest, we assume the host is enforcing VT'd */
>>> -	return run_as_guest();
>>> -}
>>> -
>>> -void
>>> -i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p);
>>> -
>>> -static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *dev_priv)
>>> -{
>>> -	return DISPLAY_VER(dev_priv) >= 6 && intel_vtd_active(dev_priv);
>>> -}
>>> -
>>> -static inline bool
>>> -intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)
>>> -{
>>> -	return IS_BROXTON(i915) && intel_vtd_active(i915);
>>> -}
>>> -
>>> -static inline bool
>>> -intel_vm_no_concurrent_access_wa(struct drm_i915_private *i915)
>>> -{
>>> -	return IS_CHERRYVIEW(i915) || intel_ggtt_update_needs_vtd_wa(i915);
>>> -}
>>> -
>>>    /* i915_gem.c */
>>>    void i915_gem_init_early(struct drm_i915_private *dev_priv);
>>>    void i915_gem_cleanup_early(struct drm_i915_private *dev_priv);
>>> diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c
>>> index 127ff56c8ce6..ccb6d62a51df 100644
>>> --- a/drivers/gpu/drm/i915/i915_gpu_error.c
>>> +++ b/drivers/gpu/drm/i915/i915_gpu_error.c
>>> @@ -50,6 +50,7 @@
>>>    #include "i915_gpu_error.h"
>>>    #include "i915_memcpy.h"
>>>    #include "i915_scatterlist.h"
>>> +#include "intel_vtd.h"
>>>    
>>>    #define ALLOW_FAIL (__GFP_KSWAPD_RECLAIM | __GFP_RETRY_MAYFAIL | __GFP_NOWARN)
>>>    #define ATOMIC_MAYFAIL (GFP_ATOMIC | __GFP_NOWARN)
>>> diff --git a/drivers/gpu/drm/i915/intel_device_info.c b/drivers/gpu/drm/i915/intel_device_info.c
>>> index 94da5aa37391..13abae12cf10 100644
>>> --- a/drivers/gpu/drm/i915/intel_device_info.c
>>> +++ b/drivers/gpu/drm/i915/intel_device_info.c
>>> @@ -27,8 +27,9 @@
>>>    
>>>    #include "display/intel_cdclk.h"
>>>    #include "display/intel_de.h"
>>> -#include "intel_device_info.h"
>>>    #include "i915_drv.h"
>>> +#include "intel_device_info.h"
>>> +#include "intel_vtd.h"
>>>    
>>>    #define PLATFORM_NAME(x) [INTEL_##x] = #x
>>>    static const char * const platform_names[] = {
>>> diff --git a/drivers/gpu/drm/i915/intel_pch.c b/drivers/gpu/drm/i915/intel_pch.c
>>> index da8f82c2342f..4d8a98dbf465 100644
>>> --- a/drivers/gpu/drm/i915/intel_pch.c
>>> +++ b/drivers/gpu/drm/i915/intel_pch.c
>>> @@ -5,6 +5,7 @@
>>>    
>>>    #include "i915_drv.h"
>>>    #include "intel_pch.h"
>>> +#include "intel_vtd.h"
>>>    
>>>    /* Map PCH device id to PCH type, or PCH_NONE if unknown. */
>>>    static enum intel_pch
>>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>>> index 23d4bb011fc8..23f0e2473e27 100644
>>> --- a/drivers/gpu/drm/i915/intel_pm.c
>>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>>> @@ -52,6 +52,7 @@
>>>    #include "i915_irq.h"
>>>    #include "intel_pcode.h"
>>>    #include "intel_pm.h"
>>> +#include "intel_vtd.h"
>>>    #include "vlv_sideband.h"
>>>    #include "../../../platform/x86/intel_ips.h"
>>>    
>>> diff --git a/drivers/gpu/drm/i915/intel_vtd.c b/drivers/gpu/drm/i915/intel_vtd.c
>>> new file mode 100644
>>> index 000000000000..e52432141081
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/i915/intel_vtd.c
>>> @@ -0,0 +1,14 @@
>>> +// SPDX-License-Identifier: MIT
>>> +/*
>>> + * Copyright © 2022 Intel Corporation
>>> + */
>>> +
>>> +#include <drm/drm_print.h>
>>> +
>>> +#include "intel_vtd.h"
>>> +
>>> +void
>>> +i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p)
>>> +{
>>> +	drm_printf(p, "iommu: %s\n", enableddisabled(intel_vtd_active(i915)));
>>> +}
>>> diff --git a/drivers/gpu/drm/i915/intel_vtd.h b/drivers/gpu/drm/i915/intel_vtd.h
>>> new file mode 100644
>>> index 000000000000..421bf8ca0b36
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/i915/intel_vtd.h
>>> @@ -0,0 +1,51 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +/*
>>> + * Copyright © 2022 Intel Corporation
>>> + */
>>> +
>>> +#ifndef __INTEL_VTD_H__
>>> +#define __INTEL_VTD_H__
>>> +
>>> +#include <linux/device.h>
>>> +#include <linux/types.h>
>>> +#include <asm/hypervisor.h>
>>> +
>>> +#include "i915_drv.h"
>>> +
>>> +struct drm_printer;
>>> +
>>> +static inline bool run_as_guest(void)
>>> +{
>>> +	return !hypervisor_is_type(X86_HYPER_NATIVE);
>>> +}
>>> +
>>> +static inline bool intel_vtd_active(struct drm_i915_private *i915)
>>> +{
>>> +	if (device_iommu_mapped(i915->drm.dev))
>>> +		return true;
>>> +
>>> +	/* Running as a guest, we assume the host is enforcing VT'd */
>>> +	return run_as_guest();
>>> +}
>>
>> How about make this one take struct device and then intel_vtd.h does not
>> have to include i915_drv.h back in? Consider also..
>>
>>> +
>>> +static inline bool intel_scanout_needs_vtd_wa(struct drm_i915_private *i915)
>>> +{
>>> +	return GRAPHICS_VER(i915) >= 6 && intel_vtd_active(i915);
>>> +}
>>> +
>>> +static inline bool
>>> +intel_ggtt_update_needs_vtd_wa(struct drm_i915_private *i915)
>>> +{
>>> +	return IS_BROXTON(i915) && intel_vtd_active(i915);
>>> +}
>>> +
>>> +static inline bool
>>> +intel_vm_no_concurrent_access_wa(struct drm_i915_private *i915)
>>> +{
>>> +	return IS_CHERRYVIEW(i915) || intel_ggtt_update_needs_vtd_wa(i915);
>>> +}
>>> +
>>> +void
>>> +i915_print_iommu_status(struct drm_i915_private *i915, struct drm_printer *p);
>>
>> .. that I think all the rest feel a bit out of place in intel_vtd.[hc]
>> to me. They all take i915 as argument and answer the question on whether
>> i915 needs something, depending on the state of the iommu. Especially
>> when you add the intel_vtd_ prefix to them in the next patch it gets
>> extra weird for me.
>>
>> In my mind, in the ideal world, they would live somewhere else and
>> include intel_vtd.h so they can legimately answer the question at hand.
>> First somewhere in display, 2nd and 3rd probably in intel_gtt.h, last in
>> i915_driver. How does that sound to you?
> 
> I'm fine with that. Like I start the commit message, "Perhaps a bit
> contrived". All I want is to axe cruft out of i915_drv.h.
> 
> So we'd still have intel_vtd_run_as_guest() and intel_vtd_active() in
> intel_vtd.h?

Yeah, hm well the remaining uglyness being intel_vtd_run_as_guest.. 
which has nothing to do with intel_vtd.. :(

The only outside caller being intel_pch.c, but I don't know what to 
suggest for that one. And I wonder how this will align with the Arm 
efforts. So I guess I can live with it for now.

Regards,

Tvrtko

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

end of thread, other threads:[~2022-02-09 10:01 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-02-08 16:54 [Intel-gfx] [PATCH v4 0/2] drm/i915: split out intel_vtd.[ch] from i915_drv.h Jani Nikula
2022-02-08 16:54 ` [Intel-gfx] [PATCH v4 1/2] " Jani Nikula
2022-02-09  9:40   ` Tvrtko Ursulin
2022-02-09  9:51     ` Jani Nikula
2022-02-09 10:01       ` Tvrtko Ursulin
2022-02-08 16:54 ` [Intel-gfx] [PATCH v4 2/2] drm/i915/vtd: rename functions to have the usual prefix Jani Nikula
2022-02-08 18:06 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/i915: split out intel_vtd.[ch] from i915_drv.h Patchwork
2022-02-08 18:07 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork
2022-02-08 18:39 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork

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