[Intel-gfx] [PATCH v5 02/10] drm/i915/guc: Add XE_LP registers for GuC error state capture.

Teres Alexis, Alan Previn alan.previn.teres.alexis at intel.com
Wed Jan 26 21:46:33 UTC 2022


Thanks Jani for taking the time to review... 

1. apologies on the const issue, this is my bad, i think it was
one of the comments from earlier rev not sure how i missed it.
Will fix this on next rev.

2. I do have a question below on the const for one of specific types
of tables. Need your thoughts

...alan


On Wed, 2022-01-26 at 20:13 +0200, Jani Nikula wrote:
> On Wed, 26 Jan 2022, Alan Previn <alan.previn.teres.alexis at intel.com> wrote:
> > Add device specific tables and register lists to cover different engines
> > class types for GuC error state capture for XE_LP products.
> > 
...

> > +static struct __ext_steer_reg xelpd_extregs[] = {
> > +	{"GEN7_SAMPLER_INSTDONE", GEN7_SAMPLER_INSTDONE},
> > +	{"GEN7_ROW_INSTDONE", GEN7_ROW_INSTDONE}
> > +};
> 
> Either this needs to be const or, if it needs to be mutable, moved to
> device specific data.
> 
> Ditto for all such things all over the place.
> 
> BR,
> Jani.


I had a question though... the list of registers like the one above as well
as below shall be made const... however, the table-of-lists (see farther down), contains a pointer to "extended_regs"
that shall be allocated at startup - is it okay for that list to remain non-const
since the others with actual register offsets remain const?

Alan: will add const for this and above tables:
	static struct __guc_mmio_reg_descr xe_lpd_global_regs[] = {
		COMMON_BASE_GLOBAL(),
		COMMON_GEN9BASE_GLOBAL(),
		COMMON_GEN12BASE_GLOBAL(),
	};

Is this okay to not be const?:
	static struct __guc_mmio_reg_descr_group default_lists[] = {
		MAKE_REGLIST(default_global_regs, PF, GLOBAL, 0),
		MAKE_REGLIST(default_rc_class_regs, PF, ENGINE_CLASS, GUC_RENDER_CLASS),
		MAKE_REGLIST(xe_lpd_rc_inst_regs, PF, ENGINE_INSTANCE, GUC_RENDER_CLASS),
		MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_VIDEO_CLASS),
		MAKE_REGLIST(xe_lpd_vd_inst_regs, PF, ENGINE_INSTANCE, GUC_VIDEO_CLASS),
		MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_VIDEOENHANCE_CLASS),
		MAKE_REGLIST(xe_lpd_vec_inst_regs, PF, ENGINE_INSTANCE, GUC_VIDEOENHANCE_CLASS),
		MAKE_REGLIST(empty_regs_list, PF, ENGINE_CLASS, GUC_BLITTER_CLASS),
		MAKE_REGLIST(xe_lpd_blt_inst_regs, PF, ENGINE_INSTANCE, GUC_BLITTER_CLASS),
		{}
	};




More information about the Intel-gfx mailing list