| Message ID | 20220825132118.784407-2-badal.nilawar@intel.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | drm/i915: Add HWMON support | expand |
On Thu, Aug 25, 2022 at 06:51:12PM +0530, Badal Nilawar wrote: > From: Dale B Stimson <dale.b.stimson@intel.com> > > The i915 HWMON module will be used to expose voltage, power and energy > values for dGfx. Here we set up i915 hwmon infrastructure including i915 > hwmon registration, basic data structures and functions. > > v2: > - Create HWMON infra patch (Ashutosh) > - Fixed review comments (Jani) > - Remove "select HWMON" from i915/Kconfig (Jani) > v3: Use hwm_ prefix for static functions (Ashutosh) > v4: s/#ifdef CONFIG_HWMON/#if IS_REACHABLE(CONFIG_HWMON)/ since the former > doesn't work if hwmon is compiled as a module (Guenter) > v5: Fixed review comments (Jani) > > Cc: Guenter Roeck <linux@roeck-us.net> > Signed-off-by: Dale B Stimson <dale.b.stimson@intel.com> > Signed-off-by: Ashutosh Dixit <ashutosh.dixit@intel.com> > Signed-off-by: Riana Tauro <riana.tauro@intel.com> > Signed-off-by: Badal Nilawar <badal.nilawar@intel.com> Acked-by: Guenter Roeck <linux@roeck-us.net> > --- > drivers/gpu/drm/i915/Makefile | 3 + > drivers/gpu/drm/i915/i915_driver.c | 5 ++ > drivers/gpu/drm/i915/i915_drv.h | 2 + > drivers/gpu/drm/i915/i915_hwmon.c | 136 +++++++++++++++++++++++++++++ > drivers/gpu/drm/i915/i915_hwmon.h | 20 +++++ > 5 files changed, 166 insertions(+) > create mode 100644 drivers/gpu/drm/i915/i915_hwmon.c > create mode 100644 drivers/gpu/drm/i915/i915_hwmon.h > > diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile > index 522ef9b4aff3..2b235f747490 100644 > --- a/drivers/gpu/drm/i915/Makefile > +++ b/drivers/gpu/drm/i915/Makefile > @@ -208,6 +208,9 @@ i915-y += gt/uc/intel_uc.o \ > # graphics system controller (GSC) support > i915-y += gt/intel_gsc.o > > +# graphics hardware monitoring (HWMON) support > +i915-$(CONFIG_HWMON) += i915_hwmon.o > + > # modesetting core code > i915-y += \ > display/hsw_ips.o \ > diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c > index 1332c70370a6..248deecd26a5 100644 > --- a/drivers/gpu/drm/i915/i915_driver.c > +++ b/drivers/gpu/drm/i915/i915_driver.c > @@ -80,6 +80,7 @@ > #include "i915_drm_client.h" > #include "i915_drv.h" > #include "i915_getparam.h" > +#include "i915_hwmon.h" > #include "i915_ioc32.h" > #include "i915_ioctl.h" > #include "i915_irq.h" > @@ -736,6 +737,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) > > intel_gt_driver_register(to_gt(dev_priv)); > > + i915_hwmon_register(dev_priv); > + > intel_display_driver_register(dev_priv); > > intel_power_domains_enable(dev_priv); > @@ -762,6 +765,8 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv) > > intel_display_driver_unregister(dev_priv); > > + i915_hwmon_unregister(dev_priv); > + > intel_gt_driver_unregister(to_gt(dev_priv)); > > i915_perf_unregister(dev_priv); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 69ce6db6a7c1..7b5b10df3404 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -705,6 +705,8 @@ struct drm_i915_private { > > struct i915_perf perf; > > + struct i915_hwmon *hwmon; > + > /* Abstract the submission mechanism (legacy ringbuffer or execlists) away */ > struct intel_gt gt0; > > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c > new file mode 100644 > index 000000000000..103dd543a214 > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_hwmon.c > @@ -0,0 +1,136 @@ > +// SPDX-License-Identifier: MIT > +/* > + * Copyright © 2022 Intel Corporation > + */ > + > +#include <linux/hwmon.h> > +#include <linux/hwmon-sysfs.h> > +#include <linux/types.h> > + > +#include "i915_drv.h" > +#include "i915_hwmon.h" > +#include "i915_reg.h" > +#include "intel_mchbar_regs.h" > + > +struct hwm_reg { > +}; > + > +struct hwm_drvdata { > + struct i915_hwmon *hwmon; > + struct intel_uncore *uncore; > + struct device *hwmon_dev; > + char name[12]; > +}; > + > +struct i915_hwmon { > + struct hwm_drvdata ddat; > + struct mutex hwmon_lock; /* counter overflow logic and rmw */ > + struct hwm_reg rg; > +}; > + > +static const struct hwmon_channel_info *hwm_info[] = { > + NULL > +}; > + > +static umode_t > +hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type, > + u32 attr, int channel) > +{ > + switch (type) { > + default: > + return 0; > + } > +} > + > +static int > +hwm_read(struct device *dev, enum hwmon_sensor_types type, u32 attr, > + int channel, long *val) > +{ > + switch (type) { > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static int > +hwm_write(struct device *dev, enum hwmon_sensor_types type, u32 attr, > + int channel, long val) > +{ > + switch (type) { > + default: > + return -EOPNOTSUPP; > + } > +} > + > +static const struct hwmon_ops hwm_ops = { > + .is_visible = hwm_is_visible, > + .read = hwm_read, > + .write = hwm_write, > +}; > + > +static const struct hwmon_chip_info hwm_chip_info = { > + .ops = &hwm_ops, > + .info = hwm_info, > +}; > + > +static void > +hwm_get_preregistration_info(struct drm_i915_private *i915) > +{ > +} > + > +void i915_hwmon_register(struct drm_i915_private *i915) > +{ > + struct device *dev = i915->drm.dev; > + struct i915_hwmon *hwmon; > + struct device *hwmon_dev; > + struct hwm_drvdata *ddat; > + > + /* hwmon is available only for dGfx */ > + if (!IS_DGFX(i915)) > + return; > + > + hwmon = kzalloc(sizeof(*hwmon), GFP_KERNEL); > + if (!hwmon) > + return; > + > + i915->hwmon = hwmon; > + mutex_init(&hwmon->hwmon_lock); > + ddat = &hwmon->ddat; > + > + ddat->hwmon = hwmon; > + ddat->uncore = &i915->uncore; > + snprintf(ddat->name, sizeof(ddat->name), "i915"); > + > + hwm_get_preregistration_info(i915); > + > + /* hwmon_dev points to device hwmon<i> */ > + hwmon_dev = hwmon_device_register_with_info(dev, ddat->name, > + ddat, > + &hwm_chip_info, > + NULL); > + if (IS_ERR(hwmon_dev)) { > + mutex_destroy(&hwmon->hwmon_lock); > + i915->hwmon = NULL; > + kfree(hwmon); > + return; > + } > + > + ddat->hwmon_dev = hwmon_dev; > +} > + > +void i915_hwmon_unregister(struct drm_i915_private *i915) > +{ > + struct i915_hwmon *hwmon; > + struct hwm_drvdata *ddat; > + > + hwmon = fetch_and_zero(&i915->hwmon); > + if (!hwmon) > + return; > + > + ddat = &hwmon->ddat; > + if (ddat->hwmon_dev) > + hwmon_device_unregister(ddat->hwmon_dev); > + > + mutex_destroy(&hwmon->hwmon_lock); > + kfree(hwmon); > +} > diff --git a/drivers/gpu/drm/i915/i915_hwmon.h b/drivers/gpu/drm/i915/i915_hwmon.h > new file mode 100644 > index 000000000000..7ca9cf2c34c9 > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_hwmon.h > @@ -0,0 +1,20 @@ > +/* SPDX-License-Identifier: MIT */ > + > +/* > + * Copyright © 2022 Intel Corporation > + */ > + > +#ifndef __I915_HWMON_H__ > +#define __I915_HWMON_H__ > + > +struct drm_i915_private; > + > +#if IS_REACHABLE(CONFIG_HWMON) > +void i915_hwmon_register(struct drm_i915_private *i915); > +void i915_hwmon_unregister(struct drm_i915_private *i915); > +#else > +static inline void i915_hwmon_register(struct drm_i915_private *i915) { }; > +static inline void i915_hwmon_unregister(struct drm_i915_private *i915) { }; > +#endif > + > +#endif /* __I915_HWMON_H__ */ > -- > 2.25.1 >
On Thu, 25 Aug 2022 06:21:12 -0700, Badal Nilawar wrote: > A couple of minor observations below but otherwise this patch is: Reviewed-by: Ashutosh Dixit <ashutosh.dixit@intel.com> > diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c > new file mode 100644 > index 000000000000..103dd543a214 > --- /dev/null > +++ b/drivers/gpu/drm/i915/i915_hwmon.c /snip/ > +struct hwm_reg { > +}; > + > +struct hwm_drvdata { > + struct i915_hwmon *hwmon; > + struct intel_uncore *uncore; Instead of 'struct intel_uncore' we could have a 'struct intel_gt' here since intel_gt is a higher level but I think uncore is fine and anyway has a backpointer to intel_gt should we need it. So no changes needed. > + struct device *hwmon_dev; > + char name[12]; > +}; > + > +struct i915_hwmon { > + struct hwm_drvdata ddat; > + struct mutex hwmon_lock; /* counter overflow logic and rmw */ > + struct hwm_reg rg; > +}; Somebody looking at just this patch might wonder why we have two data structs hwm_drvdata and i915_hwmon, rather than just one. The answer becomes clear in a later patch and that of course is that i915 exposes multiple hwmon devices. Anyway, just an observation, no changes required.
diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 522ef9b4aff3..2b235f747490 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -208,6 +208,9 @@ i915-y += gt/uc/intel_uc.o \ # graphics system controller (GSC) support i915-y += gt/intel_gsc.o +# graphics hardware monitoring (HWMON) support +i915-$(CONFIG_HWMON) += i915_hwmon.o + # modesetting core code i915-y += \ display/hsw_ips.o \ diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index 1332c70370a6..248deecd26a5 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -80,6 +80,7 @@ #include "i915_drm_client.h" #include "i915_drv.h" #include "i915_getparam.h" +#include "i915_hwmon.h" #include "i915_ioc32.h" #include "i915_ioctl.h" #include "i915_irq.h" @@ -736,6 +737,8 @@ static void i915_driver_register(struct drm_i915_private *dev_priv) intel_gt_driver_register(to_gt(dev_priv)); + i915_hwmon_register(dev_priv); + intel_display_driver_register(dev_priv); intel_power_domains_enable(dev_priv); @@ -762,6 +765,8 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv) intel_display_driver_unregister(dev_priv); + i915_hwmon_unregister(dev_priv); + intel_gt_driver_unregister(to_gt(dev_priv)); i915_perf_unregister(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 69ce6db6a7c1..7b5b10df3404 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -705,6 +705,8 @@ struct drm_i915_private { struct i915_perf perf; + struct i915_hwmon *hwmon; + /* Abstract the submission mechanism (legacy ringbuffer or execlists) away */ struct intel_gt gt0; diff --git a/drivers/gpu/drm/i915/i915_hwmon.c b/drivers/gpu/drm/i915/i915_hwmon.c new file mode 100644 index 000000000000..103dd543a214 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_hwmon.c @@ -0,0 +1,136 @@ +// SPDX-License-Identifier: MIT +/* + * Copyright © 2022 Intel Corporation + */ + +#include <linux/hwmon.h> +#include <linux/hwmon-sysfs.h> +#include <linux/types.h> + +#include "i915_drv.h" +#include "i915_hwmon.h" +#include "i915_reg.h" +#include "intel_mchbar_regs.h" + +struct hwm_reg { +}; + +struct hwm_drvdata { + struct i915_hwmon *hwmon; + struct intel_uncore *uncore; + struct device *hwmon_dev; + char name[12]; +}; + +struct i915_hwmon { + struct hwm_drvdata ddat; + struct mutex hwmon_lock; /* counter overflow logic and rmw */ + struct hwm_reg rg; +}; + +static const struct hwmon_channel_info *hwm_info[] = { + NULL +}; + +static umode_t +hwm_is_visible(const void *drvdata, enum hwmon_sensor_types type, + u32 attr, int channel) +{ + switch (type) { + default: + return 0; + } +} + +static int +hwm_read(struct device *dev, enum hwmon_sensor_types type, u32 attr, + int channel, long *val) +{ + switch (type) { + default: + return -EOPNOTSUPP; + } +} + +static int +hwm_write(struct device *dev, enum hwmon_sensor_types type, u32 attr, + int channel, long val) +{ + switch (type) { + default: + return -EOPNOTSUPP; + } +} + +static const struct hwmon_ops hwm_ops = { + .is_visible = hwm_is_visible, + .read = hwm_read, + .write = hwm_write, +}; + +static const struct hwmon_chip_info hwm_chip_info = { + .ops = &hwm_ops, + .info = hwm_info, +}; + +static void +hwm_get_preregistration_info(struct drm_i915_private *i915) +{ +} + +void i915_hwmon_register(struct drm_i915_private *i915) +{ + struct device *dev = i915->drm.dev; + struct i915_hwmon *hwmon; + struct device *hwmon_dev; + struct hwm_drvdata *ddat; + + /* hwmon is available only for dGfx */ + if (!IS_DGFX(i915)) + return; + + hwmon = kzalloc(sizeof(*hwmon), GFP_KERNEL); + if (!hwmon) + return; + + i915->hwmon = hwmon; + mutex_init(&hwmon->hwmon_lock); + ddat = &hwmon->ddat; + + ddat->hwmon = hwmon; + ddat->uncore = &i915->uncore; + snprintf(ddat->name, sizeof(ddat->name), "i915"); + + hwm_get_preregistration_info(i915); + + /* hwmon_dev points to device hwmon<i> */ + hwmon_dev = hwmon_device_register_with_info(dev, ddat->name, + ddat, + &hwm_chip_info, + NULL); + if (IS_ERR(hwmon_dev)) { + mutex_destroy(&hwmon->hwmon_lock); + i915->hwmon = NULL; + kfree(hwmon); + return; + } + + ddat->hwmon_dev = hwmon_dev; +} + +void i915_hwmon_unregister(struct drm_i915_private *i915) +{ + struct i915_hwmon *hwmon; + struct hwm_drvdata *ddat; + + hwmon = fetch_and_zero(&i915->hwmon); + if (!hwmon) + return; + + ddat = &hwmon->ddat; + if (ddat->hwmon_dev) + hwmon_device_unregister(ddat->hwmon_dev); + + mutex_destroy(&hwmon->hwmon_lock); + kfree(hwmon); +} diff --git a/drivers/gpu/drm/i915/i915_hwmon.h b/drivers/gpu/drm/i915/i915_hwmon.h new file mode 100644 index 000000000000..7ca9cf2c34c9 --- /dev/null +++ b/drivers/gpu/drm/i915/i915_hwmon.h @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: MIT */ + +/* + * Copyright © 2022 Intel Corporation + */ + +#ifndef __I915_HWMON_H__ +#define __I915_HWMON_H__ + +struct drm_i915_private; + +#if IS_REACHABLE(CONFIG_HWMON) +void i915_hwmon_register(struct drm_i915_private *i915); +void i915_hwmon_unregister(struct drm_i915_private *i915); +#else +static inline void i915_hwmon_register(struct drm_i915_private *i915) { }; +static inline void i915_hwmon_unregister(struct drm_i915_private *i915) { }; +#endif + +#endif /* __I915_HWMON_H__ */