Re: [PATCH v1] drm/xe/pf: Add runtime registers for GFX ver >= 35

["Piotr Piórkowski" <piotr.piorkowski@intel.com>]

Michal Wajdeczko <michal.wajdeczko@intel.com> wrote on pią [2025-lis-07 19:40:13 +0100]:
> 
> 
> On 11/6/2025 6:15 PM, Piórkowski, Piotr wrote:
> > From: Piotr Piórkowski <piotr.piorkowski@intel.com>
> > 
> > Add missing runtime registers for GFX ver >= 35.
> 
> I guess it's more about being a different set of registers than
> defined for the 30.x
> 
> maybe also worth to mention new SERVICE_COPY_ENABLE register
> and no need for HUC_KERNEL_LOAD_INFO (if that's true).
> 
> > 
> 
> and maybe we should add
> 
> Fixes: 5e0de2dfbc1b ("drm/xe/cri: Add CRI platform definition")
> 
> where we already claimed .has_sriov support ?
> 
> > Signed-off-by: Piotr Piórkowski <piotr.piorkowski@intel.com>
> > ---
> >  drivers/gpu/drm/xe/xe_gt_sriov_pf_service.c | 22 ++++++++++++++++++++-
> >  1 file changed, 21 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_service.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf_service.c
> > index 76dd9233ef9f..c3281ed76aa1 100644
> > --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_service.c
> > +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_service.c
> > @@ -99,11 +99,31 @@ static const struct xe_reg ver_3000_runtime_regs[] = {
> >  	HUC_KERNEL_LOAD_INFO,		/* _MMIO(0xc1dc) */
> >  };
> >  
> > +static const struct xe_reg ver_35_runtime_regs[] = {
> > +	RPM_CONFIG0,			/* _MMIO(0x0d00) */
> > +	XEHP_FUSE4,			/* _MMIO(0x9114) */
> > +	MIRROR_FUSE3,			/* _MMIO(0x9118) */
> > +	MIRROR_FUSE1,			/* _MMIO(0x911c) */
> 
> btw, do we still need this?
> it was added to support legacy platforms we used as SDV
> 
> probably we also need to check the 3000 table for this
> 

You're right. I confirmed that it's not necessary for gfx >= 35,
but from what I've seen in the code, it seems to me that it's not
used on all other platforms.
We can clean it up, but that's for another patch.

> > +	MIRROR_L3BANK_ENABLE,		/* _MMIO(0x9130) */
> > +	XELP_EU_ENABLE,			/* _MMIO(0x9134) */
> > +	XELP_GT_GEOMETRY_DSS_ENABLE,	/* _MMIO(0x913c) */
> > +	GT_VEBOX_VDBOX_DISABLE,		/* _MMIO(0x9140) */
> > +	XEHP_GT_COMPUTE_DSS_ENABLE,	/* _MMIO(0x9144) */
> > +	XEHPC_GT_COMPUTE_DSS_ENABLE_EXT,/* _MMIO(0x9148) */
> > +	XE2_GT_COMPUTE_DSS_2,		/* _MMIO(0x914c) */
> > +	XE2_GT_GEOMETRY_DSS_1,		/* _MMIO(0x9150) */
> > +	XE2_GT_GEOMETRY_DSS_2,		/* _MMIO(0x9154) */
> > +	SERVICE_COPY_ENABLE,		/* _MMIO(0x9170) */
> > +};
> > +
> >  static const struct xe_reg *pick_runtime_regs(struct xe_device *xe, unsigned int *count)
> >  {
> >  	const struct xe_reg *regs;
> >  
> > -	if (GRAPHICS_VERx100(xe) >= 3000) {
> > +	if (GRAPHICS_VER(xe) >= 35) {
> > +		*count = ARRAY_SIZE(ver_35_runtime_regs);
> > +		regs = ver_35_runtime_regs;
> > +	} else if (GRAPHICS_VERx100(xe) >= 3000) {
> >  		*count = ARRAY_SIZE(ver_3000_runtime_regs);
> >  		regs = ver_3000_runtime_regs;
> >  	} else if (GRAPHICS_VERx100(xe) >= 2000) {
> 
> otherwise LGTM, so with updated commit message and clarified
> the need for the FUSE1,
> 
> Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>


Thanks for review!
Piotr
> 

--