On 16-08-2025 03:22, Rodrigo Vivi wrote: > On Wed, Jul 30, 2025 at 12:19:53PM +0530, Aravind Iddamsetty wrote: >> Register netlink capability with the DRM and register the driver >> callbacks to DRM RAS netlink commands. >> >> v2: >> Move the netlink registration parts to DRM susbsytem (Tomer Tayar) >> >> v3: compile only if CONFIG_NET is enabled >> >> Reviewed-by: Michael J. Ruhl <[email protected]> #v2 >> Signed-off-by: Aravind Iddamsetty <[email protected]> >> --- >> drivers/gpu/drm/xe/Makefile | 2 ++ >> drivers/gpu/drm/xe/xe_device.c | 6 ++++++ >> drivers/gpu/drm/xe/xe_device_types.h | 1 + >> drivers/gpu/drm/xe/xe_netlink.c | 26 ++++++++++++++++++++++++++ >> 4 files changed, 35 insertions(+) >> create mode 100644 drivers/gpu/drm/xe/xe_netlink.c >> >> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile >> index 80eecd35e807..e960c2dbe658 100644 >> --- a/drivers/gpu/drm/xe/Makefile >> +++ b/drivers/gpu/drm/xe/Makefile >> @@ -304,6 +304,8 @@ xe-$(CONFIG_DRM_XE_DISPLAY) += \ >> i915-display/skl_universal_plane.o \ >> i915-display/skl_watermark.o >> >> +xe-$(CONFIG_NET) += xe_netlink.o >> + >> ifeq ($(CONFIG_ACPI),y) >> xe-$(CONFIG_DRM_XE_DISPLAY) += \ >> i915-display/intel_acpi.o \ >> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c >> index 806dbdf8118c..ca7a17c16aa5 100644 >> --- a/drivers/gpu/drm/xe/xe_device.c >> +++ b/drivers/gpu/drm/xe/xe_device.c >> @@ -363,6 +363,8 @@ static const struct file_operations xe_driver_fops = { >> .fop_flags = FOP_UNSIGNED_OFFSET, >> }; >> >> +extern const struct driver_genl_ops xe_genl_ops[]; >> + >> static struct drm_driver driver = { >> /* Don't use MTRRs here; the Xserver or userspace app should >> * deal with them for Intel hardware. >> @@ -381,6 +383,10 @@ static struct drm_driver driver = { >> #ifdef CONFIG_PROC_FS >> .show_fdinfo = xe_drm_client_fdinfo, >> #endif >> +#ifdef CONFIG_NET >> + .genl_ops = xe_genl_ops, >> +#endif >> + > we should definitely have a drm function to register it instead of hard-coding > it here, regardless if we go with the group split or not. > It is not okay forcing this to every platform, even the ones without any RAS > available for instance. ok. >> .ioctls = xe_ioctls, >> .num_ioctls = ARRAY_SIZE(xe_ioctls), >> .fops = &xe_driver_fops, >> diff --git a/drivers/gpu/drm/xe/xe_device_types.h >> b/drivers/gpu/drm/xe/xe_device_types.h >> index 3a851c7a55dd..08d3e53e4b37 100644 >> --- a/drivers/gpu/drm/xe/xe_device_types.h >> +++ b/drivers/gpu/drm/xe/xe_device_types.h >> @@ -10,6 +10,7 @@ >> >> #include <drm/drm_device.h> >> #include <drm/drm_file.h> >> +#include <drm/drm_netlink.h> >> #include <drm/ttm/ttm_device.h> >> >> #include "xe_devcoredump_types.h" >> diff --git a/drivers/gpu/drm/xe/xe_netlink.c >> b/drivers/gpu/drm/xe/xe_netlink.c >> new file mode 100644 >> index 000000000000..9e588fb19631 >> --- /dev/null >> +++ b/drivers/gpu/drm/xe/xe_netlink.c >> @@ -0,0 +1,26 @@ >> +// SPDX-License-Identifier: MIT >> +/* >> + * Copyright © 2023 Intel Corporation >> + */ >> + >> +#include <net/genetlink.h> >> +#include <uapi/drm/drm_netlink.h> >> + >> +#include "xe_device.h" >> + >> +static int xe_genl_list_errors(struct drm_device *drm, struct sk_buff *msg, >> struct genl_info *info) >> +{ >> + return 0; >> +} >> + >> +static int xe_genl_read_error(struct drm_device *drm, struct sk_buff *msg, >> struct genl_info *info) >> +{ >> + return 0; >> +} >> + >> +/* driver callbacks to DRM netlink commands*/ >> +const struct driver_genl_ops xe_genl_ops[] = { >> + [DRM_RAS_CMD_QUERY] = { .doit = xe_genl_list_errors }, >> + [DRM_RAS_CMD_READ_ONE] = { .doit = xe_genl_read_error }, >> + [DRM_RAS_CMD_READ_ALL] = { .doit = xe_genl_list_errors, }, >> +}; > this is another space that is strange. you declare it here and drm > magically uses it. Another reason for more explicity registration. > and with the struct drm_ras where these commands are part of that. > as well as the group name, etc.
agree, this shall be part of explicit registration. Thanks, Aravind. > >> -- >> 2.25.1 >>
