[PATCH] drm/xe/hwmon: Refactor xe hwmon

[Matt Roper]

On Fri, Jan 26, 2024 at 05:11:37AM -0800, Gupta, Anshuman wrote:
> 
> 
> > -----Original Message-----
> > From: Poosa, Karthik <karthik.poosa at intel.com>
> > Sent: Friday, January 26, 2024 1:18 PM
> > To: intel-xe at lists.freedesktop.org
> > Cc: Gupta, Anshuman <anshuman.gupta at intel.com>; Nilawar, Badal
> > <badal.nilawar at intel.com>; Roper, Matthew D
> > <matthew.d.roper at intel.com>; Poosa, Karthik <karthik.poosa at intel.com>
> > Subject: [PATCH] drm/xe/hwmon: Refactor xe hwmon
> > 
> > Check latest platform first in xe_hwmon_get_reg.
> > Move PVC registers to xe_mchbar_regs.h.
> > 
> > Fixes: fb1b70607f73 ("drm/xe/hwmon: Expose power attributes")
> > Suggested-by: Matt Roper <matthew.d.roper at intel.com>
> > Signed-off-by: Karthik Poosa <karthik.poosa at intel.com>
> > ---
> >  drivers/gpu/drm/xe/regs/xe_gt_regs.h     |  6 ------
> >  drivers/gpu/drm/xe/regs/xe_mchbar_regs.h |  6 ++++++
> >  drivers/gpu/drm/xe/xe_hwmon.c            | 24 ++++++++++++------------
> >  3 files changed, 18 insertions(+), 18 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > index cd27480f6486..15ac2d284d48 100644
> > --- a/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > +++ b/drivers/gpu/drm/xe/regs/xe_gt_regs.h
> > @@ -490,10 +490,4 @@
> >  #define   GT_CS_MASTER_ERROR_INTERRUPT		REG_BIT(3)
> >  #define   GT_RENDER_USER_INTERRUPT		REG_BIT(0)
> > 
> > -#define PVC_GT0_PACKAGE_ENERGY_STATUS		XE_REG(0x281004)
> > -#define PVC_GT0_PACKAGE_RAPL_LIMIT		XE_REG(0x281008)
> > -#define PVC_GT0_PACKAGE_POWER_SKU_UNIT
> > 	XE_REG(0x281068)
> > -#define PVC_GT0_PLATFORM_ENERGY_STATUS
> > 	XE_REG(0x28106c)
> > -#define PVC_GT0_PACKAGE_POWER_SKU		XE_REG(0x281080)
> > -
> >  #endif
> > diff --git a/drivers/gpu/drm/xe/regs/xe_mchbar_regs.h
> > b/drivers/gpu/drm/xe/regs/xe_mchbar_regs.h
> > index 519dd1067a19..97652b4297ff 100644
> > --- a/drivers/gpu/drm/xe/regs/xe_mchbar_regs.h
> > +++ b/drivers/gpu/drm/xe/regs/xe_mchbar_regs.h
> > @@ -41,4 +41,10 @@
> >  #define   PKG_PWR_LIM_1_TIME_X			REG_GENMASK(23,
> > 22)
> >  #define   PKG_PWR_LIM_1_TIME_Y			REG_GENMASK(21,
> > 17)
> > 
> > +#define PVC_GT0_PACKAGE_ENERGY_STATUS           XE_REG(0x281004)
> > +#define PVC_GT0_PACKAGE_RAPL_LIMIT              XE_REG(0x281008)
> > +#define PVC_GT0_PACKAGE_POWER_SKU_UNIT          XE_REG(0x281068)
> > +#define PVC_GT0_PLATFORM_ENERGY_STATUS          XE_REG(0x28106c)
> > +#define PVC_GT0_PACKAGE_POWER_SKU               XE_REG(0x281080)
> Above are pcode specific register, IMO it is better to create a 
> regs/xe_pcode.h header.
> @Roper, Matthew D what is your opinion here ?

Agreed; modern platforms don't mirror the MCH BAR into our gttmmadr
register space anymore, so this header isn't the right place to put it
either.  A new pcode-specific register header sounds good to me.


Matt

> Thanks,
> Anshuman Gupta.
> > +
> >  #endif /* _XE_MCHBAR_REGS_H_ */
> > diff --git a/drivers/gpu/drm/xe/xe_hwmon.c
> > b/drivers/gpu/drm/xe/xe_hwmon.c index 89c6f7f84b5a..c19a0daf7fd7
> > 100644
> > --- a/drivers/gpu/drm/xe/xe_hwmon.c
> > +++ b/drivers/gpu/drm/xe/xe_hwmon.c
> > @@ -77,32 +77,32 @@ static u32 xe_hwmon_get_reg(struct xe_hwmon
> > *hwmon, enum xe_hwmon_reg hwmon_reg)
> > 
> >  	switch (hwmon_reg) {
> >  	case REG_PKG_RAPL_LIMIT:
> > -		if (xe->info.platform == XE_DG2)
> > -			reg = PCU_CR_PACKAGE_RAPL_LIMIT;
> > -		else if (xe->info.platform == XE_PVC)
> > +		if (xe->info.platform == XE_PVC)
> >  			reg = PVC_GT0_PACKAGE_RAPL_LIMIT;
> > +		else if (xe->info.platform == XE_DG2)
> > +			reg = PCU_CR_PACKAGE_RAPL_LIMIT;
> >  		break;
> >  	case REG_PKG_POWER_SKU:
> > -		if (xe->info.platform == XE_DG2)
> > -			reg = PCU_CR_PACKAGE_POWER_SKU;
> > -		else if (xe->info.platform == XE_PVC)
> > +		if (xe->info.platform == XE_PVC)
> >  			reg = PVC_GT0_PACKAGE_POWER_SKU;
> > +		else if (xe->info.platform == XE_DG2)
> > +			reg = PCU_CR_PACKAGE_POWER_SKU;
> >  		break;
> >  	case REG_PKG_POWER_SKU_UNIT:
> > -		if (xe->info.platform == XE_DG2)
> > -			reg = PCU_CR_PACKAGE_POWER_SKU_UNIT;
> > -		else if (xe->info.platform == XE_PVC)
> > +		if (xe->info.platform == XE_PVC)
> >  			reg = PVC_GT0_PACKAGE_POWER_SKU_UNIT;
> > +		else if (xe->info.platform == XE_DG2)
> > +			reg = PCU_CR_PACKAGE_POWER_SKU_UNIT;
> >  		break;
> >  	case REG_GT_PERF_STATUS:
> >  		if (xe->info.platform == XE_DG2)
> >  			reg = GT_PERF_STATUS;
> >  		break;
> >  	case REG_PKG_ENERGY_STATUS:
> > -		if (xe->info.platform == XE_DG2)
> > -			reg = PCU_CR_PACKAGE_ENERGY_STATUS;
> > -		else if (xe->info.platform == XE_PVC)
> > +		if (xe->info.platform == XE_PVC)
> >  			reg = PVC_GT0_PLATFORM_ENERGY_STATUS;
> > +		else if (xe->info.platform == XE_DG2)
> > +			reg = PCU_CR_PACKAGE_ENERGY_STATUS;
> >  		break;
> >  	default:
> >  		drm_warn(&xe->drm, "Unknown xe hwmon reg id: %d\n",
> > hwmon_reg);
> > --
> > 2.25.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation