On 23.11.2021 00:03, Alan Previn wrote:
> Add device specific tables and register lists to cover different engines
> class types for GuC error state capture.
>
> Also, add runtime allocation and freeing of extended register lists
> for registers that need steering identifiers that depend on
> the detected HW config.
>
> Signed-off-by: Alan Previn <[email protected]>
> ---
> .../gpu/drm/i915/gt/uc/intel_guc_capture.c | 260 +++++++++++++-----
> .../gpu/drm/i915/gt/uc/intel_guc_capture.h | 2 +
> drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 2 +
> 3 files changed, 197 insertions(+), 67 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 c741c77b7fc8..eec1d193ac26 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.c
> @@ -9,120 +9,245 @@
> #include "i915_drv.h"
> #include "i915_memcpy.h"
> #include "gt/intel_gt.h"
> +#include "gt/intel_lrc_reg.h"
>
> #include "intel_guc_fwif.h"
> #include "intel_guc_capture.h"
>
> -/* Define all device tables of GuC error capture register lists */
> +/*
> + * Define all device tables of GuC error capture register lists
> + * NOTE: For engine-registers, GuC only needs the register offsets
> + * from the engine-mmio-base
> + */
> +#define COMMON_GEN12BASE_GLOBAL() \
> + {GEN12_FAULT_TLB_DATA0, 0, 0, "GEN12_FAULT_TLB_DATA0"}, \
> + {GEN12_FAULT_TLB_DATA1, 0, 0, "GEN12_FAULT_TLB_DATA1"}, \
> + {FORCEWAKE_MT, 0, 0, "FORCEWAKE_MT"}, \
> + {DERRMR, 0, 0, "DERRMR"}, \
> + {GEN12_AUX_ERR_DBG, 0, 0, "GEN12_AUX_ERR_DBG"}, \
> + {GEN12_GAM_DONE, 0, 0, "GEN12_GAM_DONE"}, \
> + {GEN11_GUC_SG_INTR_ENABLE, 0, 0, "GEN11_GUC_SG_INTR_ENABLE"}, \
> + {GEN11_CRYPTO_RSVD_INTR_ENABLE, 0, 0, "GEN11_CRYPTO_RSVD_INTR_ENABLE"},
> \
> + {GEN11_GUNIT_CSME_INTR_ENABLE, 0, 0, "GEN11_GUNIT_CSME_INTR_ENABLE"}, \
> + {GEN12_RING_FAULT_REG, 0, 0, "GEN12_RING_FAULT_REG"}
> +
> +#define COMMON_GEN12BASE_ENGINE_INSTANCE() \
> + {RING_PSMI_CTL(0), 0, 0, "RING_PSMI_CTL"}, \
> + {RING_ESR(0), 0, 0, "RING_ESR"}, \
> + {RING_ESR(0), 0, 0, "RING_ESR"}, \
> + {RING_DMA_FADD(0), 0, 0, "RING_DMA_FADD_LOW32"}, \
> + {RING_DMA_FADD_UDW(0), 0, 0, "RING_DMA_FADD_UP32"}, \
> + {RING_IPEIR(0), 0, 0, "RING_IPEIR"}, \
> + {RING_IPEHR(0), 0, 0, "RING_IPEHR"}, \
> + {RING_INSTPS(0), 0, 0, "RING_INSTPS"}, \
> + {RING_BBADDR(0), 0, 0, "RING_BBADDR_LOW32"}, \
> + {RING_BBADDR_UDW(0), 0, 0, "RING_BBADDR_UP32"}, \
> + {RING_BBSTATE(0), 0, 0, "RING_BBSTATE"}, \
> + {CCID(0), 0, 0, "CCID"}, \
> + {RING_ACTHD(0), 0, 0, "RING_ACTHD_LOW32"}, \
> + {RING_ACTHD_UDW(0), 0, 0, "RING_ACTHD_UP32"}, \
> + {RING_INSTPM(0), 0, 0, "RING_INSTPM"}, \
> + {RING_NOPID(0), 0, 0, "RING_NOPID"}, \
> + {RING_START(0), 0, 0, "RING_START"}, \
> + {RING_HEAD(0), 0, 0, "RING_HEAD"}, \
> + {RING_TAIL(0), 0, 0, "RING_TAIL"}, \
> + {RING_CTL(0), 0, 0, "RING_CTL"}, \
> + {RING_MI_MODE(0), 0, 0, "RING_MI_MODE"}, \
> + {RING_CONTEXT_CONTROL(0), 0, 0, "RING_CONTEXT_CONTROL"}, \
> + {RING_INSTDONE(0), 0, 0, "RING_INSTDONE"}, \
> + {RING_HWS_PGA(0), 0, 0, "RING_HWS_PGA"}, \
> + {RING_MODE_GEN7(0), 0, 0, "RING_MODE_GEN7"}, \
> + {GEN8_RING_PDP_LDW(0, 0), 0, 0, "GEN8_RING_PDP0_LDW"}, \
> + {GEN8_RING_PDP_UDW(0, 0), 0, 0, "GEN8_RING_PDP0_UDW"}, \
> + {GEN8_RING_PDP_LDW(0, 1), 0, 0, "GEN8_RING_PDP1_LDW"}, \
> + {GEN8_RING_PDP_UDW(0, 1), 0, 0, "GEN8_RING_PDP1_UDW"}, \
> + {GEN8_RING_PDP_LDW(0, 2), 0, 0, "GEN8_RING_PDP2_LDW"}, \
> + {GEN8_RING_PDP_UDW(0, 2), 0, 0, "GEN8_RING_PDP2_UDW"}, \
> + {GEN8_RING_PDP_LDW(0, 3), 0, 0, "GEN8_RING_PDP3_LDW"}, \
> + {GEN8_RING_PDP_UDW(0, 3), 0, 0, "GEN8_RING_PDP3_UDW"}
> +
> +#define COMMON_GEN12BASE_HAS_EU() \
> + {EIR, 0, 0, "EIR"}
> +
> +#define COMMON_GEN12BASE_RENDER() \
> + {GEN7_SC_INSTDONE, 0, 0, "GEN7_SC_INSTDONE"}, \
> + {GEN12_SC_INSTDONE_EXTRA, 0, 0, "GEN12_SC_INSTDONE_EXTRA"}, \
> + {GEN12_SC_INSTDONE_EXTRA2, 0, 0, "GEN12_SC_INSTDONE_EXTRA2"}
> +
> +#define COMMON_GEN12BASE_VEC() \
> + {GEN11_VCS_VECS_INTR_ENABLE, 0, 0, "GEN11_VCS_VECS_INTR_ENABLE"}, \
> + {GEN12_SFC_DONE(0), 0, 0, "GEN12_SFC_DONE0"}, \
> + {GEN12_SFC_DONE(1), 0, 0, "GEN12_SFC_DONE1"}, \
> + {GEN12_SFC_DONE(2), 0, 0, "GEN12_SFC_DONE2"}, \
> + {GEN12_SFC_DONE(3), 0, 0, "GEN12_SFC_DONE3"}
>
> /********************************* Gen12 LP
> *********************************/
> /************** GLOBAL *************/
> struct __guc_mmio_reg_descr gen12lp_global_regs[] = {
> - {SWF_ILK(0), 0, 0, "SWF_ILK0"},
we should avoid adding/removing code in same series
> - /* Add additional register list */
> + COMMON_GEN12BASE_GLOBAL(),
> + {GEN7_ROW_INSTDONE, 0, 0, "GEN7_ROW_INSTDONE"},
> };
>
> /********** RENDER/COMPUTE *********/
> /* Per-Class */
> struct __guc_mmio_reg_descr gen12lp_rc_class_regs[] = {
> - {SWF_ILK(0), 0, 0, "SWF_ILK0"},
> - /* Add additional register list */
> + COMMON_GEN12BASE_HAS_EU(),
> + COMMON_GEN12BASE_RENDER(),
> + {GEN11_RENDER_COPY_INTR_ENABLE, 0, 0, "GEN11_RENDER_COPY_INTR_ENABLE"},
> };
>
> /* Per-Engine-Instance */
> struct __guc_mmio_reg_descr gen12lp_rc_inst_regs[] = {
> - {SWF_ILK(0), 0, 0, "SWF_ILK0"},
> - /* Add additional register list */
> + COMMON_GEN12BASE_ENGINE_INSTANCE(),
> };
>
> /************* MEDIA-VD ************/
> /* Per-Class */
> struct __guc_mmio_reg_descr gen12lp_vd_class_regs[] = {
> - {SWF_ILK(0), 0, 0, "SWF_ILK0"},
> - /* Add additional register list */
> };
>
> /* Per-Engine-Instance */
> struct __guc_mmio_reg_descr gen12lp_vd_inst_regs[] = {
> - {SWF_ILK(0), 0, 0, "SWF_ILK0"},
> - /* Add additional register list */
> + COMMON_GEN12BASE_ENGINE_INSTANCE(),
> };
>
> /************* MEDIA-VEC ***********/
> /* Per-Class */
> struct __guc_mmio_reg_descr gen12lp_vec_class_regs[] = {
> - {SWF_ILK(0), 0, 0, "SWF_ILK0"},
> - /* Add additional register list */
> + COMMON_GEN12BASE_VEC(),
> };
>
> /* Per-Engine-Instance */
> struct __guc_mmio_reg_descr gen12lp_vec_inst_regs[] = {
> - {SWF_ILK(0), 0, 0, "SWF_ILK0"},
> - /* Add additional register list */
> + COMMON_GEN12BASE_ENGINE_INSTANCE(),
> +};
> +
> +/************* BLITTER ***********/
> +/* Per-Class */
> +struct __guc_mmio_reg_descr gen12lp_blt_class_regs[] = {
> +};
> +
> +/* Per-Engine-Instance */
> +struct __guc_mmio_reg_descr gen12lp_blt_inst_regs[] = {
> + COMMON_GEN12BASE_ENGINE_INSTANCE(),
> };
>
> +#define TO_GCAP_DEF(x) (GUC_CAPTURE_LIST_##x)
> +#define MAKE_GCAP_REGLIST_DESCR(regslist, regsowner, regstype, class) \
> + { \
> + .list = (regslist), \
> + .num_regs = (sizeof(regslist) / sizeof(struct
> __guc_mmio_reg_descr)), \
> + .owner = TO_GCAP_DEF(regsowner), \
> + .type = TO_GCAP_DEF(regstype), \
> + .engine = class, \
> + .num_ext = 0, \
> + .ext = NULL, \
> + }
> +
> +
> /********** List of lists **********/
> -struct __guc_mmio_reg_descr_group gen12lp_lists[] = {
> - {
> - .list = gen12lp_global_regs,
> - .num_regs = (sizeof(gen12lp_global_regs) / sizeof(struct
> __guc_mmio_reg_descr)),
> - .owner = GUC_CAPTURE_LIST_INDEX_PF,
> - .type = GUC_CAPTURE_LIST_TYPE_GLOBAL,
> - .engine = 0
> - },
> - {
> - .list = gen12lp_rc_class_regs,
> - .num_regs = (sizeof(gen12lp_rc_class_regs) / sizeof(struct
> __guc_mmio_reg_descr)),
> - .owner = GUC_CAPTURE_LIST_INDEX_PF,
> - .type = GUC_CAPTURE_LIST_TYPE_ENGINE_CLASS,
> - .engine = RENDER_CLASS
> - },
> - {
> - .list = gen12lp_rc_inst_regs,
> - .num_regs = (sizeof(gen12lp_rc_inst_regs) / sizeof(struct
> __guc_mmio_reg_descr)),
> - .owner = GUC_CAPTURE_LIST_INDEX_PF,
> - .type = GUC_CAPTURE_LIST_TYPE_ENGINE_INSTANCE,
> - .engine = RENDER_CLASS
> - },
> - {
> - .list = gen12lp_vd_class_regs,
> - .num_regs = (sizeof(gen12lp_vd_class_regs) / sizeof(struct
> __guc_mmio_reg_descr)),
> - .owner = GUC_CAPTURE_LIST_INDEX_PF,
> - .type = GUC_CAPTURE_LIST_TYPE_ENGINE_CLASS,
> - .engine = VIDEO_DECODE_CLASS
> - },
> - {
> - .list = gen12lp_vd_inst_regs,
> - .num_regs = (sizeof(gen12lp_vd_inst_regs) / sizeof(struct
> __guc_mmio_reg_descr)),
> - .owner = GUC_CAPTURE_LIST_INDEX_PF,
> - .type = GUC_CAPTURE_LIST_TYPE_ENGINE_INSTANCE,
> - .engine = VIDEO_DECODE_CLASS
> - },
> - {
> - .list = gen12lp_vec_class_regs,
> - .num_regs = (sizeof(gen12lp_vec_class_regs) / sizeof(struct
> __guc_mmio_reg_descr)),
> - .owner = GUC_CAPTURE_LIST_INDEX_PF,
> - .type = GUC_CAPTURE_LIST_TYPE_ENGINE_CLASS,
> - .engine = VIDEO_ENHANCEMENT_CLASS
> - },
> - {
> - .list = gen12lp_vec_inst_regs,
> - .num_regs = (sizeof(gen12lp_vec_inst_regs) / sizeof(struct
> __guc_mmio_reg_descr)),
> - .owner = GUC_CAPTURE_LIST_INDEX_PF,
> - .type = GUC_CAPTURE_LIST_TYPE_ENGINE_INSTANCE,
> - .engine = VIDEO_ENHANCEMENT_CLASS
> - },
> +struct __guc_mmio_reg_descr_group xe_lpd_lists[] = {
> + MAKE_GCAP_REGLIST_DESCR(gen12lp_global_regs, INDEX_PF, TYPE_GLOBAL, 0),
> + MAKE_GCAP_REGLIST_DESCR(gen12lp_rc_class_regs, INDEX_PF,
> TYPE_ENGINE_CLASS, GUC_RENDER_CLASS),
> + MAKE_GCAP_REGLIST_DESCR(gen12lp_rc_inst_regs, INDEX_PF,
> TYPE_ENGINE_INSTANCE, GUC_RENDER_CLASS),
> + MAKE_GCAP_REGLIST_DESCR(gen12lp_vd_class_regs, INDEX_PF,
> TYPE_ENGINE_CLASS, GUC_VIDEO_CLASS),
> + MAKE_GCAP_REGLIST_DESCR(gen12lp_vd_inst_regs, INDEX_PF,
> TYPE_ENGINE_INSTANCE, GUC_VIDEO_CLASS),
> + MAKE_GCAP_REGLIST_DESCR(gen12lp_vec_class_regs, INDEX_PF,
> TYPE_ENGINE_CLASS, GUC_VIDEOENHANCE_CLASS),
> + MAKE_GCAP_REGLIST_DESCR(gen12lp_vec_inst_regs, INDEX_PF,
> TYPE_ENGINE_INSTANCE, GUC_VIDEOENHANCE_CLASS),
> + MAKE_GCAP_REGLIST_DESCR(gen12lp_blt_class_regs, INDEX_PF,
> TYPE_ENGINE_CLASS, GUC_BLITTER_CLASS),
> + MAKE_GCAP_REGLIST_DESCR(gen12lp_blt_inst_regs, INDEX_PF,
> TYPE_ENGINE_INSTANCE, GUC_BLITTER_CLASS),
if you knew that you want to use macros, why not start with them in
previous patch ?
> {NULL, 0, 0, 0, 0}
> };
>
> -/************ FIXME: Populate tables for other devices in subsequent patch
> ************/
> +/************* Populate additional registers / device tables *************/
> +
> +static inline struct __guc_mmio_reg_descr **
> +guc_capture_get_ext_list_ptr(struct __guc_mmio_reg_descr_group * lists, u32
> owner, u32 type, u32 class)
> +{
> + while(lists->list){
please run checkpatch.pl
> + if (lists->owner == owner && lists->type == type &&
> lists->engine == class)
> + break;
> + ++lists;
> + }
> + if (!lists->list)
> + return NULL;
> +
> + return &(lists->ext);
> +}
> +
> +void guc_capture_clear_ext_regs(struct __guc_mmio_reg_descr_group * lists)
> +{
> + while(lists->list){
> + if (lists->ext) {
> + kfree(lists->ext);
> + lists->ext = NULL;
> + }
> + ++lists;
> + }
> + return;
> +}
> +
> +static void
> +xelpd_alloc_steered_ext_list(struct drm_i915_private *i915,
> + struct __guc_mmio_reg_descr_group * lists)
> +{
> + struct intel_gt *gt = &i915->gt;
> + struct sseu_dev_info *sseu;
> + int slice, subslice, i, num_tot_regs = 0;
> + struct __guc_mmio_reg_descr **ext;
> + static char * const strings[] = {
> + [0] = "GEN7_SAMPLER_INSTDONE",
> + [1] = "GEN7_ROW_INSTDONE",
> + };
> +
> + /* In XE_LP we only care about render-class steering registers during
> error-capture */
> + ext = guc_capture_get_ext_list_ptr(lists, GUC_CAPTURE_LIST_INDEX_PF,
> + GUC_CAPTURE_LIST_TYPE_ENGINE_CLASS,
> GUC_RENDER_CLASS);
> + if (!ext)
> + return;
> + if (*ext)
> + return; /* already populated */
> +
> + sseu = >->info.sseu;
> + for_each_instdone_slice_subslice(i915, sseu, slice, subslice) {
> + num_tot_regs += 2; /* two registers of interest for now */
> + }
> + if (!num_tot_regs)
> + return;
> +
> + *ext = kzalloc(2 * num_tot_regs * sizeof(struct __guc_mmio_reg_descr),
> GFP_KERNEL);
kcalloc ?
> + if (!*ext) {
> + drm_warn(&i915->drm, "GuC-capture: Fail to allocate for
> extended registers\n");
> + return;
> + }
> +
> + for_each_instdone_slice_subslice(i915, sseu, slice, subslice) {
> + for (i = 0; i < 2; i++) {
> + if (i == 0)
> + (*ext)->reg = GEN7_SAMPLER_INSTDONE;
> + else
> + (*ext)->reg = GEN7_ROW_INSTDONE;
> + (*ext)->flags = FIELD_PREP(GUC_REGSET_STEERING_GROUP,
> slice);
> + (*ext)->flags |=
> FIELD_PREP(GUC_REGSET_STEERING_INSTANCE, subslice);
> + (*ext)->regname = strings[i];
> + (*ext)++;
> + }
> + }
> +}
>
> static struct __guc_mmio_reg_descr_group *
> guc_capture_get_device_reglist(struct drm_i915_private *dev_priv)
> {
> if (IS_TIGERLAKE(dev_priv) || IS_ROCKETLAKE(dev_priv) ||
> IS_ALDERLAKE_S(dev_priv) || IS_ALDERLAKE_P(dev_priv)) {
> - return gen12lp_lists;
patch2: gen12lp_lists
patch3: xe_lpd_lists
please be consistent across series
> + /*
> + * 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.
> + */
> + xelpd_alloc_steered_ext_list(dev_priv, xe_lpd_lists);
> + return xe_lpd_lists;
> }
>
> return NULL;
> @@ -221,6 +346,7 @@ int intel_guc_capture_list_init(struct intel_guc *guc,
> u32 owner, u32 type, u32
>
> void intel_guc_capture_destroy(struct intel_guc *guc)
> {
> + guc_capture_clear_ext_regs(guc->capture.reglists);
> }
maybe whole function shall be introduced in this patch ?
-Michal
>
> int intel_guc_capture_init(struct intel_guc *guc)
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.h
> index 352940b8bc87..df420f0f49b3 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_capture.h
> @@ -25,6 +25,8 @@ struct __guc_mmio_reg_descr_group {
> u32 owner; /* see enum guc_capture_owner */
> u32 type; /* see enum guc_capture_type */
> u32 engine; /* as per MAX_ENGINE_CLASS */
> + int num_ext;
> + struct __guc_mmio_reg_descr * ext;
> };
>
> struct intel_guc_state_capture {
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> index 1a1d2271c7e9..c26cfefd916c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
> @@ -267,6 +267,8 @@ struct guc_mmio_reg {
> u32 value;
> u32 flags;
> #define GUC_REGSET_MASKED (1 << 0)
> +#define GUC_REGSET_STEERING_GROUP GENMASK(15, 12)
> +#define GUC_REGSET_STEERING_INSTANCE GENMASK(23, 20)
> u32 mask;
> } __packed;
>
>
