Re: [PATCH v2 16/22] drm/xe/xe3p_xpc: Add MCR steering

[Gustavo Sousa <gustavo.sousa@intel.com>]

Quoting Lucas De Marchi (2025-10-16 16:48:01-03:00)
>On Thu, Oct 16, 2025 at 08:44:34AM -0300, Gustavo Sousa wrote:
>>Quoting Lucas De Marchi (2025-10-15 19:06:31-03:00)
>>>From: Matt Roper <matthew.d.roper@intel.com>
>>>
>>>Xe3p_XPC's steering has a few changes from Xe3.  Aside from
>>>minor changes to the XeCore (the new name for what used to be "DSS") and
>>>INSTANCE0 tables, different rules apply to different subranges of type
>>>"GAM."  Certain GAM subranges require steering to grp/instance (0,0)
>>>(and thus use the INSTANCE0 table), while others require special
>>>steering to (1,0) instead.  Similarly, there are multiple classes of
>>>"PSMI" steering, with some requiring steering to (0,0) while others
>>>require (19,0).
>>>
>>>FIXME: There's an "L3BANK" range listed in the bspec that needs
>>>clarification.
>>
>>I think we also have a NODE range that is also missing termination
>>details.  I think a TODO/FIXME in the code for those two types of range
>>would help reminind ourselves about the need to complete the tables.
>
>Like this?

Yep, but with a few tweaks, noted below.

>
>
>| diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.c b/drivers/gpu/drm/xe/xe_gt_mcr.c
>| index e5506ec28e147..f35d8a31c1079 100644
>| --- a/drivers/gpu/drm/xe/xe_gt_mcr.c
>| +++ b/drivers/gpu/drm/xe/xe_gt_mcr.c
>| @@ -515,6 +515,12 @@ void xe_gt_mcr_init_early(struct xe_gt *gt)
>|                 }
>|         } else {
>|                 if (GRAPHICS_VERx100(xe) == 3511) {
>| +                       /*
>| +                        * TODO: there are 2 ranges in bspec with missing
>| +                        * termination: [0x00B000, 0x00B0FF] (NODE) and

The range [0x00D880, 0x00D8FF] is also tagged as NODE.

>| +                        * [0x00B100, 0x00B3FF] (L3BANK). Updated them here once


s/Updated/Update/


--
Gustavo Sousa

>| +                        * bspec is updated.
>| +                        */
>|                         gt->steering[DSS].ranges = xe3p_xpc_xecore_steering_table;
>|                         gt->steering[GAM1].ranges = xe3p_xpc_gam_grp1_steering_table;
>|                         gt->steering[INSTANCE0].ranges = xe3p_xpc_instance0_steering_table;
>
>>
>>For the ranges added in this patch, I checked and they match the Bspec,
>>so:
>>
>>Reviewed-by: Gustavo Sousa <gustavo.sousa@intel.com>
>
>thanks
>Lucas De Marchi
>
>>
>>>
>>>Bspec: 74418
>>>Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
>>>Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>>>---
>>> drivers/gpu/drm/xe/xe_gt_mcr.c   | 56 ++++++++++++++++++++++++++++++++++++++--
>>> drivers/gpu/drm/xe/xe_gt_types.h | 15 +++++++++++
>>> 2 files changed, 69 insertions(+), 2 deletions(-)
>>>
>>>diff --git a/drivers/gpu/drm/xe/xe_gt_mcr.c b/drivers/gpu/drm/xe/xe_gt_mcr.c
>>>index e1a2b38fc2a86..e5506ec28e147 100644
>>>--- a/drivers/gpu/drm/xe/xe_gt_mcr.c
>>>+++ b/drivers/gpu/drm/xe/xe_gt_mcr.c
>>>@@ -169,6 +169,15 @@ static const struct xe_mmio_range xelpg_dss_steering_table[] = {
>>>         {},
>>> };
>>>
>>>+static const struct xe_mmio_range xe3p_xpc_xecore_steering_table[] = {
>>>+        { 0x008140, 0x00817F },         /* SLICE, XeCore, SLICE */
>>>+        { 0x009480, 0x00955F },         /* SLICE, XeCore */
>>>+        { 0x00D800, 0x00D87F },                /* SLICE */
>>>+        { 0x00DC00, 0x00E9FF },         /* SLICE, rsvd, XeCore, rsvd, XeCore, rsvd, XeCore */
>>>+        { 0x013000, 0x0135FF },         /* XeCore, SLICE */
>>>+        {},
>>>+};
>>>+
>>> static const struct xe_mmio_range xelpmp_oaddrm_steering_table[] = {
>>>         { 0x393200, 0x39323F },
>>>         { 0x393400, 0x3934FF },
>>>@@ -247,6 +256,30 @@ static const struct xe_mmio_range xe3lpm_instance0_steering_table[] = {
>>>         {},
>>> };
>>>
>>>+/*
>>>+ * Different "GAM" ranges have different rules; GAMWKRS, STLB, and GAMREQSTRM
>>>+ * range subtypes need to be steered to (1,0), while all other GAM subtypes
>>>+ * are steered to (0,0) and are included in the "INSTANCE0" table farther
>>>+ * down.
>>>+ */
>>>+static const struct xe_mmio_range xe3p_xpc_gam_grp1_steering_table[] = {
>>>+        { 0x004000, 0x004AFF },                /* GAMREQSTRM, rsvd, STLB, GAMWKRS, GAMREQSTRM */
>>>+        { 0x00F100, 0x00FFFF },                /* GAMWKRS */
>>>+        {},
>>>+};
>>>+
>>>+static const struct xe_mmio_range xe3p_xpc_psmi_grp19_steering_table[] = {
>>>+        { 0x00B500, 0x00B5FF },
>>>+        {},
>>>+};
>>>+
>>>+static const struct xe_mmio_range xe3p_xpc_instance0_steering_table[] = {
>>>+        { 0x00B600, 0x00B6FF },                /* PSMI0 */
>>>+        { 0x00C800, 0x00CFFF },                /* GAMCTRL */
>>>+        { 0x00F000, 0x00F0FF },                /* GAMCTRL */
>>>+        {},
>>>+};
>>>+
>>> static void init_steering_l3bank(struct xe_gt *gt)
>>> {
>>>         struct xe_mmio *mmio = &gt->mmio;
>>>@@ -419,6 +452,18 @@ static void init_steering_sqidi_psmi(struct xe_gt *gt)
>>>         gt->steering[SQIDI_PSMI].instance_target = select & 0x1;
>>> }
>>>
>>>+static void init_steering_psmi(struct xe_gt *gt)
>>>+{
>>>+        gt->steering[PSMI19].group_target = 19;
>>>+        gt->steering[PSMI19].instance_target = 0;
>>>+}
>>>+
>>>+static void init_steering_gam1(struct xe_gt *gt)
>>>+{
>>>+        gt->steering[GAM1].group_target = 1;
>>>+        gt->steering[GAM1].instance_target = 0;
>>>+}
>>>+
>>> static const struct {
>>>         const char *name;
>>>         void (*init)(struct xe_gt *gt);
>>>@@ -426,9 +471,11 @@ static const struct {
>>>         [L3BANK] =        { "L3BANK",        init_steering_l3bank },
>>>         [MSLICE] =        { "MSLICE",        init_steering_mslice },
>>>         [LNCF] =        { "LNCF",        NULL }, /* initialized by mslice init */
>>>-        [DSS] =                { "DSS",        init_steering_dss },
>>>+        [DSS] =                { "DSS / XeCore", init_steering_dss },
>>>         [OADDRM] =        { "OADDRM / GPMXMT", init_steering_oaddrm },
>>>         [SQIDI_PSMI] =  { "SQIDI_PSMI", init_steering_sqidi_psmi },
>>>+        [PSMI19] =        { "PSMI[19]",        init_steering_psmi },
>>>+        [GAM1] =        { "GAMWKRS / STLB / GAMREQSTRM", init_steering_gam1 },
>>>         [INSTANCE0] =        { "INSTANCE 0",        NULL },
>>>         [IMPLICIT_STEERING] = { "IMPLICIT", NULL },
>>> };
>>>@@ -467,7 +514,12 @@ void xe_gt_mcr_init_early(struct xe_gt *gt)
>>>                         gt->steering[OADDRM].ranges = xelpmp_oaddrm_steering_table;
>>>                 }
>>>         } else {
>>>-                if (GRAPHICS_VER(xe) >= 20) {
>>>+                if (GRAPHICS_VERx100(xe) == 3511) {
>>>+                        gt->steering[DSS].ranges = xe3p_xpc_xecore_steering_table;
>>>+                        gt->steering[GAM1].ranges = xe3p_xpc_gam_grp1_steering_table;
>>>+                        gt->steering[INSTANCE0].ranges = xe3p_xpc_instance0_steering_table;
>>>+                        gt->steering[PSMI19].ranges = xe3p_xpc_psmi_grp19_steering_table;
>>>+                } else if (GRAPHICS_VER(xe) >= 20) {
>>>                         gt->steering[DSS].ranges = xe2lpg_dss_steering_table;
>>>                         gt->steering[SQIDI_PSMI].ranges = xe2lpg_sqidi_psmi_steering_table;
>>>                         gt->steering[INSTANCE0].ranges = xe2lpg_instance0_steering_table;
>>>diff --git a/drivers/gpu/drm/xe/xe_gt_types.h b/drivers/gpu/drm/xe/xe_gt_types.h
>>>index 8b5f604d7883a..d93faa1eedef8 100644
>>>--- a/drivers/gpu/drm/xe/xe_gt_types.h
>>>+++ b/drivers/gpu/drm/xe/xe_gt_types.h
>>>@@ -72,6 +72,21 @@ enum xe_steering_type {
>>>         OADDRM,
>>>         SQIDI_PSMI,
>>>
>>>+        /*
>>>+         * The bspec lists multiple ranges as "PSMI," but the different
>>>+         * ranges with that label have different grpid steering values so we
>>>+         * treat them independently in code.  Note that the ranges with grpid=0
>>>+         * are included in the INSTANCE0 group above.
>>>+         */
>>>+        PSMI19,
>>>+
>>>+        /*
>>>+         * Although most GAM ranges must be steered to (0,0) and thus use the
>>>+         * INSTANCE0 type farther down, some platforms have special rules
>>>+         * for specific subtypes that require steering to (1,0) instead.
>>>+         */
>>>+        GAM1,
>>>+
>>>         /*
>>>          * On some platforms there are multiple types of MCR registers that
>>>          * will always return a non-terminated value at instance (0, 0).  We'll
>>>
>>>--
>>>2.51.0
>>>