[Intel-xe] [RFC PATCH 1/6] drm/xe/hwmon: Add HWMON infrastructure
Nilawar, Badal
badal.nilawar at intel.com
Wed Jun 7 13:36:26 UTC 2023
On 07-06-2023 15:10, Riana Tauro wrote:
>
> Hi Badal
>
> On 6/6/2023 6:20 PM, Badal Nilawar wrote:
>> The xe HWMON module will be used to expose voltage, power and energy
>> values for dGfx. Here we set up xe hwmon infrastructure including xe
>> hwmon registration, basic data structures and functions.
>>
>> Signed-off-by: Badal Nilawar <badal.nilawar at intel.com>
>> ---
>> drivers/gpu/drm/xe/Makefile | 3 +
>> drivers/gpu/drm/xe/xe_device.c | 5 ++
>> drivers/gpu/drm/xe/xe_device_types.h | 2 +
>> drivers/gpu/drm/xe/xe_hwmon.c | 119 +++++++++++++++++++++++++++
>> drivers/gpu/drm/xe/xe_hwmon.h | 22 +++++
>> 5 files changed, 151 insertions(+)
>> create mode 100644 drivers/gpu/drm/xe/xe_hwmon.c
>> create mode 100644 drivers/gpu/drm/xe/xe_hwmon.h
>>
>> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
>> index f34d4bdd510b..7ac4469b325b 100644
>> --- a/drivers/gpu/drm/xe/Makefile
>> +++ b/drivers/gpu/drm/xe/Makefile
>> @@ -112,6 +112,9 @@ xe-y += xe_bb.o \
>> xe_wa.o \
>> xe_wopcm.o
>> +# graphics hardware monitoring (HWMON) support
>> +xe-$(CONFIG_HWMON) += xe_hwmon.o
>> +
>> # i915 Display compat #defines and #includes
>> subdir-ccflags-$(CONFIG_DRM_XE_DISPLAY) += \
>> -I$(srctree)/$(src)/display/ext \
>> diff --git a/drivers/gpu/drm/xe/xe_device.c
>> b/drivers/gpu/drm/xe/xe_device.c
>> index c7985af85a53..0fcd60037d66 100644
>> --- a/drivers/gpu/drm/xe/xe_device.c
>> +++ b/drivers/gpu/drm/xe/xe_device.c
>> @@ -34,6 +34,7 @@
>> #include "xe_vm.h"
>> #include "xe_vm_madvise.h"
>> #include "xe_wait_user_fence.h"
>> +#include "xe_hwmon.h"
>> static int xe_file_open(struct drm_device *dev, struct drm_file *file)
>> {
>> @@ -328,6 +329,8 @@ int xe_device_probe(struct xe_device *xe)
>> xe_debugfs_register(xe);
>> + xe_hwmon_register(xe);
> Can we call this if config hwmon is enabled instead and remove empty
> inline function?
During i915_hwmon implementation there was suggestion but not mandatory
to add dummy functions to avoid conditional code. So I decided to avoid
conditional code.
>> +
>> err = drmm_add_action_or_reset(&xe->drm, xe_device_sanitize, xe);
>> if (err)
>> return err;
>> @@ -354,6 +357,8 @@ static void xe_device_remove_display(struct
>> xe_device *xe)
>> void xe_device_remove(struct xe_device *xe)
>> {
>> + xe_hwmon_unregister(xe);
>> +
>> xe_device_remove_display(xe);
>> xe_display_unlink(xe);
>> diff --git a/drivers/gpu/drm/xe/xe_device_types.h
>> b/drivers/gpu/drm/xe/xe_device_types.h
>> index 17b6b1cc5adb..6f511d359de8 100644
>> --- a/drivers/gpu/drm/xe/xe_device_types.h
>> +++ b/drivers/gpu/drm/xe/xe_device_types.h
>> @@ -378,6 +378,8 @@ struct xe_device {
>> /** @d3cold_allowed: Indicates if d3cold is a valid device state */
>> bool d3cold_allowed;
>> + struct xe_hwmon *hwmon;
>> +
>> /* private: */
>> #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
>> diff --git a/drivers/gpu/drm/xe/xe_hwmon.c
>> b/drivers/gpu/drm/xe/xe_hwmon.c
>> new file mode 100644
>> index 000000000000..65e5488dec82
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_hwmon.c
>> @@ -0,0 +1,119 @@
>> +// SPDX-License-Identifier: MIT
>> +/*
>> + * Copyright © 2023 Intel Corporation
>> + */
>> +
>> +#include <linux/hwmon.h>
>> +#include <linux/hwmon-sysfs.h>
>> +#include <linux/types.h>
>> +
>> +#include "regs/xe_gt_regs.h"
> Above 3 are Unused include for this patch
>> +#include "xe_device.h"
>> +#include "xe_hwmon.h"
>> +#include "xe_mmio.h"
> Unused include
>> +#include "xe_gt.h"
> Unused include
Sure, I will remove unused includes.
>> +
>> +struct hwm_drvdata {
>> + struct xe_hwmon *hwmon;
>> + struct device *hwmon_dev;
>> + char name[12];
>> +};
>> +
>> +struct xe_hwmon {
>> + struct hwm_drvdata ddat;
>> + struct mutex hwmon_lock; /* counter overflow logic and rmw */
> Comment can be removed for this patch and added with the overflow logic
Will remove this comment.
>> +};
>> + > +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 xe_device *xe)
>> +{
>> +}
>> +
>> +void xe_hwmon_register(struct xe_device *xe)
>> +{
>> + struct device *dev = xe->drm.dev;
>> + struct xe_hwmon *hwmon;
>> + struct device *hwmon_dev;
>> + struct hwm_drvdata *ddat;
>> +
>> + /* hwmon is available only for dGfx */
>> + if (!IS_DGFX(xe))
>> + return;
>> +
>> + hwmon = devm_kzalloc(dev, sizeof(*hwmon), GFP_KERNEL);
>> + if (!hwmon)
>> + return;
>> +
>> + xe->hwmon = hwmon;
>> + mutex_init(&hwmon->hwmon_lock);
>> + ddat = &hwmon->ddat;
>> +
>> + ddat->hwmon = hwmon;
>> + snprintf(ddat->name, sizeof(ddat->name), "xe");
>> +
>> + hwm_get_preregistration_info(xe);
>> +
>> + drm_dbg(&xe->drm, "Register HWMON interface\n");
>> +
>> + /* hwmon_dev points to device hwmon<i> */
>> + hwmon_dev = devm_hwmon_device_register_with_info(dev, ddat->name,
>> + ddat,
>> + &hwm_chip_info,
>> + NULL);
>> + if (IS_ERR(hwmon_dev)) {
>> + xe->hwmon = NULL;
> dbg or error statement if registration fails?
Sure will add dbg/error statement.
> mutex_destroy needs to be added?
As xe->hwmon is allocated throuh devm_kzalloc think just xe->hwmon =
NULL is enough here.
Regards,
Badal
>
> Thanks
> Riana
>> + return;
>> + }
>> +
>> + ddat->hwmon_dev = hwmon_dev;
>> +}
>> +
>> +void xe_hwmon_unregister(struct xe_device *xe)
>> +{
>> + xe->hwmon = NULL;
>> +}
>> diff --git a/drivers/gpu/drm/xe/xe_hwmon.h
>> b/drivers/gpu/drm/xe/xe_hwmon.h
>> new file mode 100644
>> index 000000000000..a078eeb0a68b
>> --- /dev/null
>> +++ b/drivers/gpu/drm/xe/xe_hwmon.h
>> @@ -0,0 +1,22 @@
>> +/* SPDX-License-Identifier: MIT */
>> +
>> +/*
>> + * Copyright © 2023 Intel Corporation
>> + */
>> +
>> +#ifndef __XE_HWMON_H__
>> +#define __XE_HWMON_H__
>> +
>> +#include <linux/types.h>
>> +
>> +struct xe_device;
>> +
>> +#if IS_REACHABLE(CONFIG_HWMON)
>> +void xe_hwmon_register(struct xe_device *xe);
>> +void xe_hwmon_unregister(struct xe_device *xe);
>> +#else
>> +static inline void xe_hwmon_register(struct xe_device *xe) { };
>> +static inline void xe_hwmon_unregister(struct xe_device *xe) { };
>> +#endif
>> +
>> +#endif /* __XE_HWMON_H__ */
More information about the Intel-xe
mailing list