On Sat, Feb 26, 2022 at 01:55:32AM -0800, Alan Previn wrote:
Add additional DG2 registers for GuC error state capture.
Signed-off-by: Alan Previn <alan.previn.teres.alexis@xxxxxxxxx>
---
.../gpu/drm/i915/gt/uc/intel_guc_capture.c | 80 ++++++++++++++++++-
1 file changed, 78 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
index 6370943ea300..c8441ca1566b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
@@ -285,20 +285,96 @@ guc_capture_alloc_steered_lists_xe_lpd(struct intel_guc *guc,
guc->capture.priv->extlists = extlists;
}
+static const struct __ext_steer_reg xehpg_extregs[] = {
+ {"XEHPG_INSTDONE_GEOM_SVG", XEHPG_INSTDONE_GEOM_SVG}
+};
+
+static bool __has_xehpg_extregs(u32 ipver)
What I meant was to make has_xehpg_extregs part of platform feature, but
this is fine too.
+{
+ return (ipver >= IP_VER(12, 55));
+}
+
+static void
+guc_capture_alloc_steered_lists_xe_hpg(struct intel_guc *guc,
+ const struct __guc_mmio_reg_descr_group *lists,
+ u32 ipver)
+{
+ struct intel_gt *gt = guc_to_gt(guc);
+ struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
+ struct sseu_dev_info *sseu;
+ int slice, subslice, i, iter, num_steer_regs, num_tot_regs = 0;
+ const struct __guc_mmio_reg_descr_group *list;
+ struct __guc_mmio_reg_descr_group *extlists;
+ struct __guc_mmio_reg_descr *extarray;
+
+ /* In XE_LP / HPG we only have render-class steering registers during error-capture */
+ list = guc_capture_get_one_list(lists, GUC_CAPTURE_LIST_INDEX_PF,
+ GUC_CAPTURE_LIST_TYPE_ENGINE_CLASS, GUC_RENDER_CLASS);
+ /* skip if extlists was previously allocated */
+ if (!list || guc->capture.priv->extlists)
+ return;
+
+ num_steer_regs = ARRAY_SIZE(xe_extregs);
+ if (__has_xehpg_extregs(ipver))
+ num_steer_regs += ARRAY_SIZE(xehpg_extregs);
+
+ sseu = >->info.sseu;
+ for_each_instdone_gslice_dss_xehp(i915, sseu, iter, slice, subslice) {
+ num_tot_regs += num_steer_regs;
+ }
+
+ if (!num_tot_regs)
+ return;
+
+ /* allocate an extra for an end marker */
+ extlists = kcalloc(2, sizeof(struct __guc_mmio_reg_descr_group), GFP_KERNEL);
+ if (!extlists)
+ return;
+
+ if (__alloc_ext_regs(&extlists[0], list, num_tot_regs)) {
+ kfree(extlists);
+ return;
+ }
+
+ extarray = extlists[0].extlist;
+ for_each_instdone_gslice_dss_xehp(i915, sseu, iter, slice, subslice) {
+ for (i = 0; i < ARRAY_SIZE(xe_extregs); ++i) {
+ __fill_ext_reg(extarray, &xe_extregs[i], slice, subslice);
+ ++extarray;
+ }
+ if (__has_xehpg_extregs(ipver)) {
+ for (i = 0; i < ARRAY_SIZE(xehpg_extregs); ++i) {
+ __fill_ext_reg(extarray, &xehpg_extregs[i], slice, subslice);
+ ++extarray;
+ }
+ }
+ }
+
+ drm_dbg(&i915->drm, "GuC-capture found %d-ext-regs.\n", num_tot_regs);
+ guc->capture.priv->extlists = extlists;
+}
+
static const struct __guc_mmio_reg_descr_group *
guc_capture_get_device_reglist(struct intel_guc *guc)
{
struct drm_i915_private *i915 = guc_to_gt(guc)->i915;
if (IS_TIGERLAKE(i915) || IS_ROCKETLAKE(i915) ||
- IS_ALDERLAKE_S(i915) || IS_ALDERLAKE_P(i915)) {
+ IS_ALDERLAKE_S(i915) || IS_ALDERLAKE_P(i915) ||
+ IS_DG2(i915) || IS_XEHPSDV(i915)) {
/*
* For certain engine classes, there are slice and subslice
* level registers requiring steering. We allocate and populate
* these at init time based on hw config add it as an extension
* list at the end of the pre-populated render list.
*/
- guc_capture_alloc_steered_lists_xe_lpd(guc, xe_lpd_lists);
+ if (IS_DG2(i915))
+ guc_capture_alloc_steered_lists_xe_hpg(guc, xe_lpd_lists, IP_VER(12, 55));
+ else if (IS_XEHPSDV(i915))
+ guc_capture_alloc_steered_lists_xe_hpg(guc, xe_lpd_lists, IP_VER(12, 50));
+ else
+ guc_capture_alloc_steered_lists_xe_lpd(guc, xe_lpd_lists);
+
return xe_lpd_lists;
}
If you know that this applies to gen12 platforms, then you could do away with
the ORed platform checks. I think it is cumbersome to maintain an OR of
platforms that need this. The other pattern I see in the driver is like this:
i.e, If you return xe_lpd_lists back from
guc_capture_alloc_steered_lists_xe_hpg, then this functions simplifies to
struct __guc_mmio_reg_descr_group *ret;
if (IS_DG2(i915))
ret = guc_capture_alloc_steered_lists_xe_hpg(guc, xe_lpd_lists, IP_VER(12, 55));
else if (IS_XEHPSDV(i915))
ret = guc_capture_alloc_steered_lists_xe_hpg(guc, xe_lpd_lists, IP_VER(12, 50));
else if (GRAPHICS_VER(i915) > 11) /* OR maybe == 12 */
ret = guc_capture_alloc_steered_lists_xe_lpd(guc, xe_lpd_lists);
else
ret = NULL;
if !(ret)
drm_...
return ret;
Irrespective of that:
Reviewed-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@xxxxxxxxx>
Umesh
--
2.25.1
