Re: [PATCH v1] drm/xe/hwmon: expose package and vram temperature

[Raag Jadav <raag.jadav@intel.com>]

On Fri, Jan 24, 2025 at 11:46:05AM +0530, Riana Tauro wrote:
> Hi Raag
> 
> On 1/23/2025 8:21 AM, Raag Jadav wrote:
> > On Tue, Jan 21, 2025 at 01:56:05PM +0530, Riana Tauro wrote:
> > > Hi Raag
> > > 
> > > On 1/8/2025 2:54 PM, Raag Jadav wrote:
> > > > Add hwmon support for temp1_input and temp2_input attributes, which will
> > > > expose package and vram temperature in millidegree Celsius. With this in
> > > > place we can monitor temperature using lm-sensors tool.
> > > > 
> > > > Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> > > > ---
> > > >    .../ABI/testing/sysfs-driver-intel-xe-hwmon   | 16 +++++
> > > >    drivers/gpu/drm/xe/regs/xe_mchbar_regs.h      |  3 +
> > > >    drivers/gpu/drm/xe/regs/xe_pcode_regs.h       |  2 +
> > > >    drivers/gpu/drm/xe/xe_hwmon.c                 | 63 +++++++++++++++++++
> > > >    4 files changed, 84 insertions(+)
> > > > 
> > > > diff --git a/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon b/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon
> > > > index d792a56f59ac..998cfb0ee1a6 100644
> > > > --- a/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon
> > > > +++ b/Documentation/ABI/testing/sysfs-driver-intel-xe-hwmon
> > > > @@ -108,3 +108,19 @@ Contact:	intel-xe@lists.freedesktop.org
> > > >    Description:	RO. Package current voltage in millivolt.
> > > >    		Only supported for particular Intel Xe graphics platforms.
> > > > +
> > > > +What:		/sys/bus/pci/drivers/xe/.../hwmon/hwmon<i>/temp1_input
> > > > +Date:		April 2025
> > > > +KernelVersion:	6.15
> > > > +Contact:	intel-xe@lists.freedesktop.org
> > > > +Description:	RO. Package temperature in millidegree Celsius.
> > > > +
> > > > +		Only supported for particular Intel Xe graphics platforms.
> > > > +
> > > > +What:		/sys/bus/pci/drivers/xe/.../hwmon/hwmon<i>/temp2_input
> > > > +Date:		April 2025
> > > > +KernelVersion:	6.15
> > > > +Contact:	intel-xe@lists.freedesktop.org
> > > > +Description:	RO. VRAM temperature in millidegree Celsius.
> > > > +
> > > > +		Only supported for particular Intel Xe graphics platforms.
> > > > diff --git a/drivers/gpu/drm/xe/regs/xe_mchbar_regs.h b/drivers/gpu/drm/xe/regs/xe_mchbar_regs.h
> > > > index 519dd1067a19..f5e5234857c1 100644
> > > > --- a/drivers/gpu/drm/xe/regs/xe_mchbar_regs.h
> > > > +++ b/drivers/gpu/drm/xe/regs/xe_mchbar_regs.h
> > > > @@ -34,6 +34,9 @@
> > > >    #define PCU_CR_PACKAGE_ENERGY_STATUS		XE_REG(MCHBAR_MIRROR_BASE_SNB + 0x593c)
> > > > +#define PCU_CR_PACKAGE_TEMPERATURE		XE_REG(MCHBAR_MIRROR_BASE_SNB + 0x5978)
> > > > +#define   TEMP_MASK				REG_GENMASK(7, 0)
> > > > +
> > > >    #define PCU_CR_PACKAGE_RAPL_LIMIT		XE_REG(MCHBAR_MIRROR_BASE_SNB + 0x59a0)
> > > >    #define   PKG_PWR_LIM_1				REG_GENMASK(14, 0)
> > > >    #define   PKG_PWR_LIM_1_EN			REG_BIT(15)
> > > > diff --git a/drivers/gpu/drm/xe/regs/xe_pcode_regs.h b/drivers/gpu/drm/xe/regs/xe_pcode_regs.h
> > > > index 0b0b49d850ae..8846eb9ce2a4 100644
> > > > --- a/drivers/gpu/drm/xe/regs/xe_pcode_regs.h
> > > > +++ b/drivers/gpu/drm/xe/regs/xe_pcode_regs.h
> > > > @@ -21,6 +21,8 @@
> > > >    #define BMG_PACKAGE_POWER_SKU			XE_REG(0x138098)
> > > >    #define BMG_PACKAGE_POWER_SKU_UNIT		XE_REG(0x1380dc)
> > > >    #define BMG_PACKAGE_ENERGY_STATUS		XE_REG(0x138120)
> > > > +#define BMG_VRAM_TEMPERATURE			XE_REG(0x1382c0)
> > > > +#define BMG_PACKAGE_TEMPERATURE			XE_REG(0x138434)
> > > indentation.
> > 
> > It's a git quirk, you won't see it in file.
> > 
> > > Also you are using the same for DG2. Should have a common name
> > 
> > Just following the conventions.
> Did not find this convention in the file.
> BMG_VRAM_TEMPERATURE is used in both dg2 and bmg and has a bmg prefix.
> Doesn't seem right		

That's what we use for pcode registers.

Raag