Xe_HP removed "slice" as a first-class unit in the hardware design. Instead we now have a single pool of subslices (which are now referred to as "DSS") that different hardware units have different ways of grouping ("compute slices," "geometry slices," etc.). For the purposes of topology representation, we treat Xe_HP-based platforms as having a single slice that contains all of the platform's DSS. There's no need to allocate storage space for (max legacy slices * max dss); let's update some of our macros to minimize the storage requirement for sseu topology. We'll also document some of the constants to make it a little bit more clear what they represent.
Signed-off-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/i915/gt/intel_engine_types.h | 2 +- drivers/gpu/drm/i915/gt/intel_sseu.h | 47 +++++++++++++++----- 2 files changed, 36 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_engine_types.h b/drivers/gpu/drm/i915/gt/intel_engine_types.h index 4fbf45a74ec0..f9e246004bc0 100644 --- a/drivers/gpu/drm/i915/gt/intel_engine_types.h +++ b/drivers/gpu/drm/i915/gt/intel_engine_types.h @@ -645,7 +645,7 @@ intel_engine_has_relative_mmio(const struct intel_engine_cs * const engine)
#define for_each_instdone_gslice_dss_xehp(dev_priv_, sseu_, iter_, gslice_, dss_) \ for ((iter_) = 0, (gslice_) = 0, (dss_) = 0; \ - (iter_) < GEN_MAX_SUBSLICES; \ + (iter_) < GEN_SS_MASK_SIZE; \ (iter_)++, (gslice_) = (iter_) / GEN_DSS_PER_GSLICE, \ (dss_) = (iter_) % GEN_DSS_PER_GSLICE) \ for_each_if(intel_sseu_has_subslice((sseu_), 0, (iter_))) diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h b/drivers/gpu/drm/i915/gt/intel_sseu.h index 8a79cd8eaab4..4f59eadbb61a 100644 --- a/drivers/gpu/drm/i915/gt/intel_sseu.h +++ b/drivers/gpu/drm/i915/gt/intel_sseu.h @@ -15,26 +15,49 @@ struct drm_i915_private; struct intel_gt; struct drm_printer;
-#define GEN_MAX_SLICES (3) /* SKL upper bound */ -#define GEN_MAX_SUBSLICES (32) /* XEHPSDV upper bound */ -#define GEN_SSEU_STRIDE(max_entries) DIV_ROUND_UP(max_entries, BITS_PER_BYTE) -#define GEN_MAX_SUBSLICE_STRIDE GEN_SSEU_STRIDE(GEN_MAX_SUBSLICES) -#define GEN_MAX_EUS (16) /* TGL upper bound */ -#define GEN_MAX_EU_STRIDE GEN_SSEU_STRIDE(GEN_MAX_EUS) +/* + * Maximum number of legacy slices. Legacy slices no longer exist starting on + * Xe_HP ("gslices," "cslices," etc. on Xe_HP and beyond are a different + * concept and are not expressed through fusing). + */ +#define GEN_MAX_LEGACY_SLICES 3 + +/* + * Maximum number of subslices that can exist within a legacy slice. This is + * only relevant to pre-Xe_HP platforms (Xe_HP and beyond use the GEN_MAX_DSS + * value below). + */ +#define GEN_MAX_LEGACY_SUBSLICES 6 + +/* Maximum number of DSS on newer platforms (Xe_HP and beyond). */ +#define GEN_MAX_DSS 32 + +/* Maximum number of EUs that can exist within a subslice or DSS. */ +#define GEN_MAX_EUS_PER_SS 16 + +#define MAX(a, b) ((a) > (b) ? (a) : (b)) + +/* The maximum number of bits needed to express each subslice/DSS independently */ +#define GEN_SS_MASK_SIZE MAX(GEN_MAX_DSS, \ + GEN_MAX_LEGACY_SLICES * GEN_MAX_LEGACY_SUBSLICES) + +#define GEN_SSEU_STRIDE(max_entries) DIV_ROUND_UP(max_entries, BITS_PER_BYTE) +#define GEN_MAX_SUBSLICE_STRIDE GEN_SSEU_STRIDE(GEN_SS_MASK_SIZE) +#define GEN_MAX_EU_STRIDE GEN_SSEU_STRIDE(GEN_MAX_EUS_PER_SS)
#define GEN_DSS_PER_GSLICE 4 #define GEN_DSS_PER_CSLICE 8 #define GEN_DSS_PER_MSLICE 8
-#define GEN_MAX_GSLICES (GEN_MAX_SUBSLICES / GEN_DSS_PER_GSLICE) -#define GEN_MAX_CSLICES (GEN_MAX_SUBSLICES / GEN_DSS_PER_CSLICE) +#define GEN_MAX_GSLICES (GEN_MAX_DSS / GEN_DSS_PER_GSLICE) +#define GEN_MAX_CSLICES (GEN_MAX_DSS / GEN_DSS_PER_CSLICE)
struct sseu_dev_info { u8 slice_mask; - u8 subslice_mask[GEN_MAX_SLICES * GEN_MAX_SUBSLICE_STRIDE]; - u8 geometry_subslice_mask[GEN_MAX_SLICES * GEN_MAX_SUBSLICE_STRIDE]; - u8 compute_subslice_mask[GEN_MAX_SLICES * GEN_MAX_SUBSLICE_STRIDE]; - u8 eu_mask[GEN_MAX_SLICES * GEN_MAX_SUBSLICES * GEN_MAX_EU_STRIDE]; + u8 subslice_mask[GEN_SS_MASK_SIZE]; + u8 geometry_subslice_mask[GEN_SS_MASK_SIZE]; + u8 compute_subslice_mask[GEN_SS_MASK_SIZE]; + u8 eu_mask[GEN_SS_MASK_SIZE * GEN_MAX_EU_STRIDE]; u16 eu_total; u8 eu_per_subslice; u8 min_eu_in_pool;
When running on Xe_HP or beyond, let's use an updated format for describing topology in our error state dumps and debugfs to give a more accurate view of the hardware:
- Just report DSS directly without the legacy "slice0" output that's no longer meaningful. - Indicate whether each DSS is accessible for geometry and/or compute. - Rename "rcs_topology" to "sseu_topology" since the information reported is common to both RCS and CCS engines now.
Signed-off-by: Matt Roper matthew.d.roper@intel.com --- drivers/gpu/drm/i915/gt/intel_sseu.c | 48 +++++++++++++++++--- drivers/gpu/drm/i915/gt/intel_sseu.h | 3 +- drivers/gpu/drm/i915/gt/intel_sseu_debugfs.c | 8 ++-- drivers/gpu/drm/i915/i915_gpu_error.c | 2 +- 4 files changed, 48 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.c b/drivers/gpu/drm/i915/gt/intel_sseu.c index 614915ffbd37..4d28458ab768 100644 --- a/drivers/gpu/drm/i915/gt/intel_sseu.c +++ b/drivers/gpu/drm/i915/gt/intel_sseu.c @@ -10,6 +10,8 @@ #include "intel_gt_regs.h" #include "intel_sseu.h"
+#include "linux/string_helpers.h" + void intel_sseu_set_info(struct sseu_dev_info *sseu, u8 max_slices, u8 max_subslices, u8 max_eus_per_subslice) { @@ -54,6 +56,11 @@ u32 intel_sseu_get_subslices(const struct sseu_dev_info *sseu, u8 slice) return _intel_sseu_get_subslices(sseu, sseu->subslice_mask, slice); }
+u32 intel_sseu_get_geometry_subslices(const struct sseu_dev_info *sseu) +{ + return _intel_sseu_get_subslices(sseu, sseu->geometry_subslice_mask, 0); +} + u32 intel_sseu_get_compute_subslices(const struct sseu_dev_info *sseu) { return _intel_sseu_get_subslices(sseu, sseu->compute_subslice_mask, 0); @@ -720,16 +727,11 @@ void intel_sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p) str_yes_no(sseu->has_eu_pg)); }
-void intel_sseu_print_topology(const struct sseu_dev_info *sseu, - struct drm_printer *p) +static void intel_sseu_print_legacy_topology(const struct sseu_dev_info *sseu, + struct drm_printer *p) { int s, ss;
- if (sseu->max_slices == 0) { - drm_printf(p, "Unavailable\n"); - return; - } - for (s = 0; s < sseu->max_slices; s++) { drm_printf(p, "slice%d: %u subslice(s) (0x%08x):\n", s, intel_sseu_subslices_per_slice(sseu, s), @@ -744,6 +746,38 @@ void intel_sseu_print_topology(const struct sseu_dev_info *sseu, } }
+static void intel_sseu_print_xehp_topology(const struct sseu_dev_info *sseu, + struct drm_printer *p) +{ + u32 g_dss_mask = intel_sseu_get_geometry_subslices(sseu); + u32 c_dss_mask = intel_sseu_get_compute_subslices(sseu); + int dss; + + for (dss = 0; dss < sseu->max_subslices; dss++) { + u16 enabled_eus = sseu_get_eus(sseu, 0, dss); + + drm_printf(p, "DSS%02d: G:%3s C:%3s, %2u EUs (0x%04hx)\n", dss, + str_yes_no(g_dss_mask & BIT(dss)), + str_yes_no(c_dss_mask & BIT(dss)), + hweight16(enabled_eus), enabled_eus); + } +} + + +void intel_sseu_print_topology(struct drm_i915_private *i915, + const struct sseu_dev_info *sseu, + struct drm_printer *p) +{ + if (sseu->max_slices == 0) { + drm_printf(p, "Unavailable\n"); + return; + } else if (GRAPHICS_VER_FULL(i915) >= IP_VER(12, 50)) { + intel_sseu_print_xehp_topology(sseu, p); + } else { + intel_sseu_print_legacy_topology(sseu, p); + } +} + u16 intel_slicemask_from_dssmask(u64 dss_mask, int dss_per_slice) { u16 slice_mask = 0; diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.h b/drivers/gpu/drm/i915/gt/intel_sseu.h index 4f59eadbb61a..fe22ea9bb213 100644 --- a/drivers/gpu/drm/i915/gt/intel_sseu.h +++ b/drivers/gpu/drm/i915/gt/intel_sseu.h @@ -139,7 +139,8 @@ u32 intel_sseu_make_rpcs(struct intel_gt *gt, const struct intel_sseu *req_sseu);
void intel_sseu_dump(const struct sseu_dev_info *sseu, struct drm_printer *p); -void intel_sseu_print_topology(const struct sseu_dev_info *sseu, +void intel_sseu_print_topology(struct drm_i915_private *i915, + const struct sseu_dev_info *sseu, struct drm_printer *p);
u16 intel_slicemask_from_dssmask(u64 dss_mask, int dss_per_slice); diff --git a/drivers/gpu/drm/i915/gt/intel_sseu_debugfs.c b/drivers/gpu/drm/i915/gt/intel_sseu_debugfs.c index a9d5bc49f361..6b944de48666 100644 --- a/drivers/gpu/drm/i915/gt/intel_sseu_debugfs.c +++ b/drivers/gpu/drm/i915/gt/intel_sseu_debugfs.c @@ -287,22 +287,22 @@ static int sseu_status_show(struct seq_file *m, void *unused) } DEFINE_INTEL_GT_DEBUGFS_ATTRIBUTE(sseu_status);
-static int rcs_topology_show(struct seq_file *m, void *unused) +static int sseu_topology_show(struct seq_file *m, void *unused) { struct intel_gt *gt = m->private; struct drm_printer p = drm_seq_file_printer(m);
- intel_sseu_print_topology(>->info.sseu, &p); + intel_sseu_print_topology(gt->i915, >->info.sseu, &p);
return 0; } -DEFINE_INTEL_GT_DEBUGFS_ATTRIBUTE(rcs_topology); +DEFINE_INTEL_GT_DEBUGFS_ATTRIBUTE(sseu_topology);
void intel_sseu_debugfs_register(struct intel_gt *gt, struct dentry *root) { static const struct intel_gt_debugfs_file files[] = { { "sseu_status", &sseu_status_fops, NULL }, - { "rcs_topology", &rcs_topology_fops, NULL }, + { "sseu_topology", &sseu_topology_fops, NULL }, };
intel_gt_debugfs_register_files(root, files, ARRAY_SIZE(files), gt); diff --git a/drivers/gpu/drm/i915/i915_gpu_error.c b/drivers/gpu/drm/i915/i915_gpu_error.c index 5e09a4e4b01a..44ff2b899893 100644 --- a/drivers/gpu/drm/i915/i915_gpu_error.c +++ b/drivers/gpu/drm/i915/i915_gpu_error.c @@ -712,7 +712,7 @@ static void err_print_gt_info(struct drm_i915_error_state_buf *m, struct drm_printer p = i915_error_printer(m);
intel_gt_info_print(>->info, &p); - intel_sseu_print_topology(>->info.sseu, &p); + intel_sseu_print_topology(gt->_gt->i915, >->info.sseu, &p); }
static void err_print_gt(struct drm_i915_error_state_buf *m,
Hi Matt,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip] [also build test WARNING on drm-exynos/exynos-drm-next drm/drm-next next-20220310] [cannot apply to drm-intel/for-linux-next tegra-drm/drm/tegra/for-next airlied/drm-next v5.17-rc7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Matt-Roper/drm-i915-sseu-Don-t-over... base: git://anongit.freedesktop.org/drm/drm-tip drm-tip config: i386-debian-10.3 (https://download.01.org/0day-ci/archive/20220311/202203112245.eDvNTHyE-lkp@i...) compiler: gcc-9 (Ubuntu 9.4.0-1ubuntu1~20.04) 9.4.0 reproduce (this is a W=1 build): # https://github.com/0day-ci/linux/commit/38985b2e6acdbe67dedb5de8a8aeef917b74... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Matt-Roper/drm-i915-sseu-Don-t-overallocate-subslice-storage/20220311-141705 git checkout 38985b2e6acdbe67dedb5de8a8aeef917b746453 # save the config file to linux build tree mkdir build_dir make W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All warnings (new ones prefixed by >>):
drivers/gpu/drm/i915/gt/intel_sseu.c:59:5: warning: no previous prototype for 'intel_sseu_get_geometry_subslices' [-Wmissing-prototypes]
59 | u32 intel_sseu_get_geometry_subslices(const struct sseu_dev_info *sseu) | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vim +/intel_sseu_get_geometry_subslices +59 drivers/gpu/drm/i915/gt/intel_sseu.c
58
59 u32 intel_sseu_get_geometry_subslices(const struct sseu_dev_info *sseu)
60 { 61 return _intel_sseu_get_subslices(sseu, sseu->geometry_subslice_mask, 0); 62 } 63
--- 0-DAY CI Kernel Test Service https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Hi Matt,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip] [also build test WARNING on drm-exynos/exynos-drm-next drm/drm-next next-20220310] [cannot apply to drm-intel/for-linux-next tegra-drm/drm/tegra/for-next v5.17-rc7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Matt-Roper/drm-i915-sseu-Don-t-over... base: git://anongit.freedesktop.org/drm/drm-tip drm-tip config: i386-randconfig-a013 (https://download.01.org/0day-ci/archive/20220311/202203112234.rtvKSbsq-lkp@i...) compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project 276ca87382b8f16a65bddac700202924228982f6) reproduce (this is a W=1 build): wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # https://github.com/0day-ci/linux/commit/38985b2e6acdbe67dedb5de8a8aeef917b74... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Matt-Roper/drm-i915-sseu-Don-t-overallocate-subslice-storage/20220311-141705 git checkout 38985b2e6acdbe67dedb5de8a8aeef917b746453 # save the config file to linux build tree mkdir build_dir COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/i915/ net/core/
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
All warnings (new ones prefixed by >>):
drivers/gpu/drm/i915/gt/intel_sseu.c:59:5: warning: no previous prototype for function 'intel_sseu_get_geometry_subslices' [-Wmissing-prototypes]
u32 intel_sseu_get_geometry_subslices(const struct sseu_dev_info *sseu) ^ drivers/gpu/drm/i915/gt/intel_sseu.c:59:1: note: declare 'static' if the function is not intended to be used outside of this translation unit u32 intel_sseu_get_geometry_subslices(const struct sseu_dev_info *sseu) ^ static 1 warning generated.
vim +/intel_sseu_get_geometry_subslices +59 drivers/gpu/drm/i915/gt/intel_sseu.c
58
59 u32 intel_sseu_get_geometry_subslices(const struct sseu_dev_info *sseu)
60 { 61 return _intel_sseu_get_subslices(sseu, sseu->geometry_subslice_mask, 0); 62 } 63
--- 0-DAY CI Kernel Test Service https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
drivers/gpu/drm/i915/gt/intel_sseu.c:59:5: warning: symbol 'intel_sseu_get_geometry_subslices' was not declared. Should it be static?
Reported-by: kernel test robot lkp@intel.com Signed-off-by: kernel test robot lkp@intel.com --- drivers/gpu/drm/i915/gt/intel_sseu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/gt/intel_sseu.c b/drivers/gpu/drm/i915/gt/intel_sseu.c index 4d28458ab768d..99e31dcf3db06 100644 --- a/drivers/gpu/drm/i915/gt/intel_sseu.c +++ b/drivers/gpu/drm/i915/gt/intel_sseu.c @@ -56,7 +56,7 @@ u32 intel_sseu_get_subslices(const struct sseu_dev_info *sseu, u8 slice) return _intel_sseu_get_subslices(sseu, sseu->subslice_mask, slice); }
-u32 intel_sseu_get_geometry_subslices(const struct sseu_dev_info *sseu) +static u32 intel_sseu_get_geometry_subslices(const struct sseu_dev_info *sseu) { return _intel_sseu_get_subslices(sseu, sseu->geometry_subslice_mask, 0); }
Hi Matt,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on drm-tip/drm-tip] [also build test WARNING on drm-exynos/exynos-drm-next drm/drm-next next-20220310] [cannot apply to drm-intel/for-linux-next tegra-drm/drm/tegra/for-next airlied/drm-next v5.17-rc7] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch]
url: https://github.com/0day-ci/linux/commits/Matt-Roper/drm-i915-sseu-Don-t-over... base: git://anongit.freedesktop.org/drm/drm-tip drm-tip config: x86_64-rhel-8.3-kselftests (https://download.01.org/0day-ci/archive/20220312/202203120322.OkxCdfs7-lkp@i...) compiler: gcc-9 (Ubuntu 9.4.0-1ubuntu1~20.04) 9.4.0 reproduce: # apt-get install sparse # sparse version: v0.6.4-dirty # https://github.com/0day-ci/linux/commit/38985b2e6acdbe67dedb5de8a8aeef917b74... git remote add linux-review https://github.com/0day-ci/linux git fetch --no-tags linux-review Matt-Roper/drm-i915-sseu-Don-t-overallocate-subslice-storage/20220311-141705 git checkout 38985b2e6acdbe67dedb5de8a8aeef917b746453 # save the config file to linux build tree mkdir build_dir make W=1 C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/gpu/drm/i915/
If you fix the issue, kindly add following tag as appropriate Reported-by: kernel test robot lkp@intel.com
sparse warnings: (new ones prefixed by >>)
drivers/gpu/drm/i915/gt/intel_sseu.c:59:5: sparse: sparse: symbol 'intel_sseu_get_geometry_subslices' was not declared. Should it be static?
Please review and possibly fold the followup patch.
--- 0-DAY CI Kernel Test Service https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
On Thu, Mar 10, 2022 at 10:15:43PM -0800, Matt Roper wrote:
this func with a single underscore is the one inconsistent with the rest of the file. Just rename it while touching this part of the code?
}
+u32 intel_sseu_get_geometry_subslices(const struct sseu_dev_info *sseu)
since it's only local to this compilation unit, make it static and remove the intel_ prefix?
removing the intel_ prefix would make it consistent with the rest of the file too
ditto
either make this an early return, or remove the return
other than coding style nits metioned above,
Reviewed-by: Lucas De Marchi lucas.demarchi@intel.com
Lucas De Marchi
On Thu, Mar 10, 2022 at 10:15:42PM -0800, Matt Roper wrote:
instead of calling the old legacy, maybe just add the XEHP_ prefix to the new ones?
what's worse, include kernel.h in another header file or redefine MAX everywhere? Re-defining it in headers we risk situations in which the include order may create warnings about defining it in multiple places.
Aside the minor things above, everything look correct.
Reviewed-by: Lucas De Marchi lucas.demarchi@intel.com
thanks Lucas De Marchi
On Fri, Mar 11, 2022 at 11:00:09AM -0800, Lucas De Marchi wrote:
Maybe a "HSW_" prefix on the old ones would be better? People still use the termm 'subslice' in casual discussion when talking about DSS, so I want to somehow distinguish that what we're talking about here is a different, older design than we have on modern platforms.
Matt
On Fri, Mar 11, 2022 at 12:38:17PM -0800, Matt Roper wrote:
Hmm, or maybe just "GEN_MAX_SUBSLICES_PER_LEGACY_SLICE" to tie it into the slice definition above?
Matt
On Fri, Mar 11, 2022 at 12:43:40PM -0800, Matt Roper wrote:
I'm not too fond of calling it "legacy" when everywhere else in the driver we just use the platform as prefix/suffix. Some may see legacy as < ver 12, others as 12.50, etc.
Well... but I will leave that up to you if you are convinced one is better than the other.
thanks Lucas De Marchi
On Fri, Mar 11, 2022 at 12:52:33PM -0800, Lucas De Marchi wrote:
Everything will become legacy at some point. This kind of naming scheme falls apart when the next shiny new thing comes around and we end up with multiple different leagacies. Are we going to have ANCIENT_LEGACY, RECENT_LEGACY, NOT_YET_LEGACY etc?
On Fri, Mar 11, 2022 at 11:01:01PM +0200, Ville Syrjälä wrote:
Well that's kind of the point --- there is no shiny new thing and never will be. "slice" is gone for good and the places we use the term are _not_ the same thing as similar-sounding terms like gslice, cslice, mslice, etc.
But I'll go ahead and switch it to "HSW_" and hope people figure out that it stops being a meaningful concept Xe_HP.
Matt
dri-devel@lists.freedesktop.org
-
kernel test robot -
Lucas De Marchi -
Matt Roper -
Ville Syrjälä