[Public] >> This one is kind of urgent, hence the change is specifically done inside >> amdgpu_virt.
I prefer to use the origin patch (v1) to fix it if it is an urgent issue. And the new solution can be submitted later, and all hwmon/nodes will be managed uniformly, instead of using one method for each hwmon through patching. Best Regards, Kevin From: Lazar, Lijo <[email protected]> Sent: Tuesday, September 9, 2025 11:16 AM To: Wang, Yang(Kevin) <[email protected]>; Kamal, Asad <[email protected]>; [email protected] Cc: Zhang, Hawking <[email protected]>; Ma, Le <[email protected]>; Zhang, Morris <[email protected]>; Kamal, Asad <[email protected]>; Deucher, Alexander <[email protected]> Subject: Re: [PATCH 1/3] drm/amdgpu: Add generic capability class [Public] The intent is to keep hwmon/sysfs attributes based on new RW capabilities of attributes as in this series and replace existing logic. This one is kind of urgent, hence the change is specifically done inside amdgpu_virt. Thanks, Lijo ________________________________ From: Wang, Yang(Kevin) <[email protected]<mailto:[email protected]>> Sent: Tuesday, September 9, 2025 7:58:20 AM To: Kamal, Asad <[email protected]<mailto:[email protected]>>; [email protected]<mailto:[email protected]> <[email protected]<mailto:[email protected]>>; Lazar, Lijo <[email protected]<mailto:[email protected]>> Cc: Zhang, Hawking <[email protected]<mailto:[email protected]>>; Ma, Le <[email protected]<mailto:[email protected]>>; Zhang, Morris <[email protected]<mailto:[email protected]>>; Kamal, Asad <[email protected]<mailto:[email protected]>>; Deucher, Alexander <[email protected]<mailto:[email protected]>> Subject: RE: [PATCH 1/3] drm/amdgpu: Add generic capability class [AMD Official Use Only - AMD Internal Distribution Only] Yes, it seems there is a new hwmon/sysnode visible attribute check path, but there is already a check logic in amdgpu_pm.c now, such as checking the return value - EPONOTUPPORT. e.g: (amdgpu_dpm_get_fan_speed_pwm(adev, NULL) == -EOPNOTSUPP We won't discuss which approach is better here, but unifying the code logic and maintaining a certain style is undoubtedly a better choice. what do you think? Best Regards, Kevin -----Original Message----- From: amd-gfx <[email protected]<mailto:[email protected]>> On Behalf Of Asad Kamal Sent: Friday, September 5, 2025 12:43 PM To: [email protected]<mailto:[email protected]>; Lazar, Lijo <[email protected]<mailto:[email protected]>> Cc: Zhang, Hawking <[email protected]<mailto:[email protected]>>; Ma, Le <[email protected]<mailto:[email protected]>>; Zhang, Morris <[email protected]<mailto:[email protected]>>; Kamal, Asad <[email protected]<mailto:[email protected]>>; Deucher, Alexander <[email protected]<mailto:[email protected]>> Subject: [PATCH 1/3] drm/amdgpu: Add generic capability class From: Lijo Lazar <[email protected]<mailto:[email protected]>> Define a utility macro for defining capabilities and their attributes. Capability attributes are read-only, write-only, read-write. Signed-off-by: Lijo Lazar <[email protected]<mailto:[email protected]>> --- drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 + drivers/gpu/drm/amd/amdgpu/amdgpu_utils.h | 91 +++++++++++++++++++++++ 2 files changed, 92 insertions(+) create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_utils.h diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h index 17848ce65d1f..a06e69681ff6 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h @@ -63,6 +63,7 @@ #include "kgd_pp_interface.h" #include "amd_shared.h" +#include "amdgpu_utils.h" #include "amdgpu_mode.h" #include "amdgpu_ih.h" #include "amdgpu_irq.h" diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_utils.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_utils.h new file mode 100644 index 000000000000..1e40ca3b1584 --- /dev/null +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_utils.h @@ -0,0 +1,91 @@ +/* SPDX-License-Identifier: MIT */ +/* + * Copyright 2025 Advanced Micro Devices, Inc. + * + * Permission is hereby granted, free of charge, to any person +obtaining a + * copy of this software and associated documentation files (the +"Software"), + * to deal in the Software without restriction, including without +limitation + * the rights to use, copy, modify, merge, publish, distribute, +sublicense, + * and/or sell copies of the Software, and to permit persons to whom +the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be +included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, +EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF +MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT +SHALL + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, +DAMAGES OR + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +OTHERWISE, + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE +OR + * OTHER DEALINGS IN THE SOFTWARE. + * + */ + +#ifndef AMDGPU_UTILS_H_ +#define AMDGPU_UTILS_H_ + +/* ---------- Generic 2‑bit capability attribute encoding ---------- + * 00 INVALID, 01 RO, 10 WO, 11 RW + */ +enum amdgpu_cap_attr { + AMDGPU_CAP_ATTR_INVALID = 0, + AMDGPU_CAP_ATTR_RO = 1 << 0, + AMDGPU_CAP_ATTR_WO = 1 << 1, + AMDGPU_CAP_ATTR_RW = (AMDGPU_CAP_ATTR_RO | AMDGPU_CAP_ATTR_WO), +}; + +#define AMDGPU_CAP_ATTR_BITS 2 +#define AMDGPU_CAP_ATTR_MAX ((1U << AMDGPU_CAP_ATTR_BITS) - 1) + +/* Internal helper to build helpers for a given enum NAME */ +#define DECLARE_ATTR_CAP_CLASS_HELPERS(NAME) \ +enum { NAME##_BITMAP_BITS = NAME##_COUNT * AMDGPU_CAP_ATTR_BITS }; \ +struct NAME##_caps { \ + DECLARE_BITMAP(bmap, NAME##_BITMAP_BITS); \ +}; \ +static inline unsigned int NAME##_ATTR_START(enum NAME##_cap_id cap) \ +{ return (unsigned int)cap * AMDGPU_CAP_ATTR_BITS; } \ +static inline void NAME##_attr_init(struct NAME##_caps *c) \ +{ if (c) bitmap_zero(c->bmap, NAME##_BITMAP_BITS); } \ +static inline int NAME##_attr_set(struct NAME##_caps *c, \ + enum NAME##_cap_id cap, enum amdgpu_cap_attr attr) \ +{ \ + if (!c) \ + return -EINVAL; \ + if (cap >= NAME##_COUNT) \ + return -EINVAL; \ + if ((unsigned int)attr > AMDGPU_CAP_ATTR_MAX) \ + return -EINVAL; \ + bitmap_write(c->bmap, (unsigned long)attr, \ + NAME##_ATTR_START(cap), AMDGPU_CAP_ATTR_BITS); \ + return 0; \ +} \ +static inline int NAME##_attr_get(const struct NAME##_caps *c, \ + enum NAME##_cap_id cap, enum amdgpu_cap_attr *out) \ +{ \ + unsigned long v; \ + if (!c || !out) \ + return -EINVAL; \ + if (cap >= NAME##_COUNT) \ + return -EINVAL; \ + v = bitmap_read(c->bmap, NAME##_ATTR_START(cap), AMDGPU_CAP_ATTR_BITS); \ + *out = (enum amdgpu_cap_attr)v; \ + return 0; \ +} \ +static inline bool NAME##_cap_is_ro(const struct NAME##_caps *c, enum NAME##_cap_id id) \ +{ enum amdgpu_cap_attr a; return !NAME##_attr_get(c, id, &a) && a == AMDGPU_CAP_ATTR_RO; } \ +static inline bool NAME##_cap_is_wo(const struct NAME##_caps *c, enum NAME##_cap_id id) \ +{ enum amdgpu_cap_attr a; return !NAME##_attr_get(c, id, &a) && a == AMDGPU_CAP_ATTR_WO; } \ +static inline bool NAME##_cap_is_rw(const struct NAME##_caps *c, enum NAME##_cap_id id) \ +{ enum amdgpu_cap_attr a; return !NAME##_attr_get(c, id, &a) && a == +AMDGPU_CAP_ATTR_RW; } + +/* Element expander for enum creation */ #define _CAP_ENUM_ELEM(x) x, + +/* Public macro: declare enum + helpers from an X‑macro list */ +#define DECLARE_ATTR_CAP_CLASS(NAME, LIST_MACRO) \ + enum NAME##_cap_id { LIST_MACRO(_CAP_ENUM_ELEM) NAME##_COUNT }; \ + DECLARE_ATTR_CAP_CLASS_HELPERS(NAME) + +#endif /* AMDGPU_UTILS_H_ */ -- 2.46.0
