[Intel-gfx] [PATCH 2/2] drm/i915/xehp: Update topology dumps for Xe_HP

[Lucas De Marchi]

On Thu, Mar 10, 2022 at 10:15:43PM -0800, Matt Roper wrote:
>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 at 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);

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?

>+{
>+	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,

removing the intel_ prefix would make it consistent with the rest of the file too

>+					     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)

ditto

>+{
>+	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;

either make this an early return, or remove the return

other than coding style nits metioned above,


Reviewed-by: Lucas De Marchi <lucas.demarchi at intel.com>

Lucas De Marchi