<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
</head>
<body>
<div>
<div>
<div style="color: rgb(0, 0, 0); background-color: rgb(255, 255, 255);" dir="ltr">
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?</div>
</div>
<div id="ms-outlook-mobile-signature">
<div></div>
</div>
</div>
<hr style="display:inline-block;width:98%" tabindex="-1">
<div id="divRplyFwdMsg" dir="ltr"><font face="Calibri, sans-serif" style="font-size:11pt" color="#000000"><b>From:</b> Jani Nikula <jani.nikula@linux.intel.com><br>
<b>Sent:</b> Friday, February 11, 2022 5:33:37 AM<br>
<b>To:</b> Cheng, Michael <michael.cheng@intel.com>; intel-gfx@lists.freedesktop.org <intel-gfx@lists.freedesktop.org><br>
<b>Cc:</b> 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><br>
<b>Subject:</b> Re: [PATCH v1 1/1] drm/i915/gt: Move wbvind_on_all_cpus #define</font>
<div> </div>
</div>
<div class="BodyFragment"><font size="2"><span style="font-size:11pt;">
<div class="PlainText">On Thu, 10 Feb 2022, Michael Cheng <michael.cheng@intel.com> wrote:<br>
> Move wbvind_on_all_cpus to intel_gt.h. This will allow other wbind_on_all_cpus<br>
> calls to benefit from the #define logic, and prevent compiler errors<br>
> when building for non-x86 architectures.<br>
><br>
> Signed-off-by: Michael Cheng <michael.cheng@intel.com><br>
> ---<br>
> drivers/gpu/drm/i915/gem/i915_gem_pm.c | 7 -------<br>
> drivers/gpu/drm/i915/gt/intel_gt.h | 7 +++++++<br>
> 2 files changed, 7 insertions(+), 7 deletions(-)<br>
><br>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_pm.c b/drivers/gpu/drm/i915/gem/i915_gem_pm.c<br>
> index 6da68b38f00f..ff7340ae5ac8 100644<br>
> --- a/drivers/gpu/drm/i915/gem/i915_gem_pm.c<br>
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_pm.c<br>
> @@ -12,13 +12,6 @@<br>
> <br>
> #include "i915_drv.h"<br>
> <br>
> -#if defined(CONFIG_X86)<br>
> -#include <asm/smp.h><br>
> -#else<br>
> -#define wbinvd_on_all_cpus() \<br>
> - pr_warn(DRIVER_NAME ": Missing cache flush in %s\n", __func__)<br>
> -#endif<br>
> -<br>
> void i915_gem_suspend(struct drm_i915_private *i915)<br>
> {<br>
> GEM_TRACE("%s\n", dev_name(i915->drm.dev));<br>
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h<br>
> index 2dad46c3eff2..149e8c13e402 100644<br>
> --- a/drivers/gpu/drm/i915/gt/intel_gt.h<br>
> +++ b/drivers/gpu/drm/i915/gt/intel_gt.h<br>
> @@ -10,6 +10,13 @@<br>
> #include "intel_gt_types.h"<br>
> #include "intel_reset.h"<br>
> <br>
> +#if defined(CONFIG_X86)<br>
> +#include <asm/smp.h><br>
> +#else<br>
> +#define wbinvd_on_all_cpus() \<br>
> + pr_warn(DRIVER_NAME ": Missing cache flush in %s\n", __func__)<br>
> +#endif<br>
<br>
Don't include headers from headers if it can be avoided.<br>
<br>
gt/intel_gt.h is included from 79 files. We don't want all of them to<br>
include <asm/smp.h> when only 3 files actually need<br>
wbinvd_on_all_cpus().<br>
<br>
Also, gt/intel_gt.h has absolutely nothing to do with<br>
wbinvd_on_all_cpus() or asm/smp.h. Please don't use topical headers as<br>
dumping grounds for random things.<br>
<br>
Maybe a better idea is to add a local wrapper for wbinvd_on_all_cpus()<br>
that does the right thing. Or add the above in a dedicated header.<br>
<br>
<br>
BR,<br>
Jani.<br>
<br>
<br>
> +<br>
> struct drm_i915_private;<br>
> struct drm_printer;<br>
<br>
-- <br>
Jani Nikula, Intel Open Source Graphics Center<br>
</div>
</span></font></div>
</body>
</html>