Thanks for the feedback! Another idea I had that kind of align to what you are suggesting is adding a wrapper for wbinvd_on_all_cpus within drm_cache.h, then having it be included in the three files that calls this function. Thoughts on that? ________________________________ From: Jani Nikula jani.nikula@linux.intel.com Sent: Friday, February 11, 2022 5:33:37 AM To: Cheng, Michael michael.cheng@intel.com; intel-gfx@lists.freedesktop.org intel-gfx@lists.freedesktop.org Cc: tvrtko.ursulin@linux.intel.com tvrtko.ursulin@linux.intel.com; Cheng, Michael michael.cheng@intel.com; Vivekanandan, Balasubramani balasubramani.vivekanandan@intel.com; Boyer, Wayne wayne.boyer@intel.com; Bowman, Casey G casey.g.bowman@intel.com; De Marchi, Lucas lucas.demarchi@intel.com; dri-devel@lists.freedesktop.org dri-devel@lists.freedesktop.org Subject: Re: [PATCH v1 1/1] drm/i915/gt: Move wbvind_on_all_cpus #define
On Thu, 10 Feb 2022, Michael Cheng michael.cheng@intel.com wrote:
Move wbvind_on_all_cpus to intel_gt.h. This will allow other wbind_on_all_cpus calls to benefit from the #define logic, and prevent compiler errors when building for non-x86 architectures.
Signed-off-by: Michael Cheng michael.cheng@intel.com
drivers/gpu/drm/i915/gem/i915_gem_pm.c | 7 ------- drivers/gpu/drm/i915/gt/intel_gt.h | 7 +++++++ 2 files changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c index 6da68b38f00f..ff7340ae5ac8 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c @@ -12,13 +12,6 @@
#include "i915_drv.h"
-#if defined(CONFIG_X86) -#include <asm/smp.h> -#else -#define wbinvd_on_all_cpus() \
pr_warn(DRIVER_NAME ": Missing cache flush in %s\n", __func__)-#endif
void i915_gem_suspend(struct drm_i915_private *i915) { GEM_TRACE("%s\n", dev_name(i915->drm.dev)); diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h index 2dad46c3eff2..149e8c13e402 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.h +++ b/drivers/gpu/drm/i915/gt/intel_gt.h @@ -10,6 +10,13 @@ #include "intel_gt_types.h" #include "intel_reset.h"
+#if defined(CONFIG_X86) +#include <asm/smp.h> +#else +#define wbinvd_on_all_cpus() \
pr_warn(DRIVER_NAME ": Missing cache flush in %s\n", __func__)+#endif
Don't include headers from headers if it can be avoided.
gt/intel_gt.h is included from 79 files. We don't want all of them to include <asm/smp.h> when only 3 files actually need wbinvd_on_all_cpus().
Also, gt/intel_gt.h has absolutely nothing to do with wbinvd_on_all_cpus() or asm/smp.h. Please don't use topical headers as dumping grounds for random things.
Maybe a better idea is to add a local wrapper for wbinvd_on_all_cpus() that does the right thing. Or add the above in a dedicated header.
BR, Jani.
struct drm_i915_private; struct drm_printer;
-- Jani Nikula, Intel Open Source Graphics Center