diff mbox series

[RFC,3/7] drm/i915/guc: Populate XE_LP register lists for GuC error state capture.

Message ID 20211122230402.2023576-4-alan.previn.teres.alexis@intel.com (mailing list archive)
State New, archived
Headers show
Series Add GuC Error Capture Support | expand

Commit Message

Teres Alexis, Alan Previn Nov. 22, 2021, 11:03 p.m. UTC
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 <alan.previn.teres.alexis@intel.com>
---
 .../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(-)

Comments

Michal Wajdeczko Nov. 23, 2021, 9:55 p.m. UTC | #1
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 <alan.previn.teres.alexis@intel.com>
> ---
>  .../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 = &gt->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;
>  
>
Teres Alexis, Alan Previn Nov. 24, 2021, 5:16 p.m. UTC | #2
Thanks Michal for reviewing the code. I will get all of these fixed.

I still would like continue to have a first patch with a skeleton table of registers
as the patch that focuses on the infrastructure and another patch just for the registers.

That sad, to align with your review comments, i shall ensure the first patch starts
with one valid register that doesnt get removed and also move the ext-list
functions and macros into that first patch. But keep full register table population
in the 2nd patch..

...alan



On Tue, 2021-11-23 at 22:55 +0100, Michal Wajdeczko wrote:
>  
> >  /********************************* 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
> 
> > +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
> 
> > +
> > +	sseu = &gt->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 ?
> 
> >  
> >  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
> 
> >
diff mbox series

Patch

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"},
-	/* 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),
 	{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){
+		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 = &gt->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);
+	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;
+		/*
+		* 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);
 }
 
 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;