From: Rodrigo Vivi <rodrigo.vivi@intel.com>
To: Raag Jadav <raag.jadav@intel.com>
Cc: <lucas.demarchi@intel.com>, <intel-xe@lists.freedesktop.org>,
<anshuman.gupta@intel.com>, <badal.nilawar@intel.com>,
<riana.tauro@intel.com>
Subject: Re: [PATCH v1] drm/xe: Introduce xe_pm_sysfs.c
Date: Mon, 5 May 2025 15:09:42 -0400 [thread overview]
Message-ID: <aBkM9q-BksNG7D5i@intel.com> (raw)
In-Reply-To: <20250505053716.3360242-1-raag.jadav@intel.com>
On Mon, May 05, 2025 at 11:07:16AM +0530, Raag Jadav wrote:
> Introduce xe_pm_sysfs.c for Power Management knobs and move existing
> attributes to it.
>
> Signed-off-by: Raag Jadav <raag.jadav@intel.com>
> ---
>
> This depends on BMG PCIe link downgrade series[1].
> [1] https://lore.kernel.org/r/20250502183739.3308069-1-raag.jadav@intel.com
>
> drivers/gpu/drm/xe/Makefile | 1 +
> drivers/gpu/drm/xe/xe_device_sysfs.c | 64 +---------------------
> drivers/gpu/drm/xe/xe_device_sysfs.h | 1 -
> drivers/gpu/drm/xe/xe_pm.c | 2 +-
> drivers/gpu/drm/xe/xe_pm_sysfs.c | 81 ++++++++++++++++++++++++++++
> drivers/gpu/drm/xe/xe_pm_sysfs.h | 13 +++++
> 6 files changed, 97 insertions(+), 65 deletions(-)
> create mode 100644 drivers/gpu/drm/xe/xe_pm_sysfs.c
> create mode 100644 drivers/gpu/drm/xe/xe_pm_sysfs.h
>
> diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
> index c5d6681645ed..683b5a7073ba 100644
> --- a/drivers/gpu/drm/xe/Makefile
> +++ b/drivers/gpu/drm/xe/Makefile
> @@ -86,6 +86,7 @@ xe-y += xe_bb.o \
> xe_pci.o \
> xe_pcode.o \
> xe_pm.o \
> + xe_pm_sysfs.o \
> xe_preempt_fence.o \
> xe_pt.o \
> xe_pt_walk.o \
> diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/xe/xe_device_sysfs.c
> index 7172972b5b60..cdea5e5ebf3d 100644
> --- a/drivers/gpu/drm/xe/xe_device_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
> @@ -3,12 +3,11 @@
> * Copyright © 2023 Intel Corporation
> */
>
> +#include <linux/device.h>
> #include <linux/kobject.h>
> #include <linux/pci.h>
> #include <linux/sysfs.h>
>
> -#include <drm/drm_managed.h>
> -
> #include "xe_device.h"
> #include "xe_device_sysfs.h"
> #include "xe_mmio.h"
> @@ -22,69 +21,8 @@
> * each graphics device. Considering this, we need to add sysfs attributes at device
> * level granularity.
> * These sysfs attributes will be available under pci device kobj directory.
> - *
> - * vram_d3cold_threshold - Report/change vram used threshold(in MB) below
> - * which vram save/restore is permissible during runtime D3cold entry/exit.
> */
>
> -static ssize_t
> -vram_d3cold_threshold_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct pci_dev *pdev = to_pci_dev(dev);
> - struct xe_device *xe = pdev_to_xe_device(pdev);
> - int ret;
> -
> - xe_pm_runtime_get(xe);
> - ret = sysfs_emit(buf, "%d\n", xe->d3cold.vram_threshold);
> - xe_pm_runtime_put(xe);
> -
> - return ret;
> -}
> -
> -static ssize_t
> -vram_d3cold_threshold_store(struct device *dev, struct device_attribute *attr,
> - const char *buff, size_t count)
> -{
> - struct pci_dev *pdev = to_pci_dev(dev);
> - struct xe_device *xe = pdev_to_xe_device(pdev);
> - u32 vram_d3cold_threshold;
> - int ret;
> -
> - ret = kstrtou32(buff, 0, &vram_d3cold_threshold);
> - if (ret)
> - return ret;
> -
> - drm_dbg(&xe->drm, "vram_d3cold_threshold: %u\n", vram_d3cold_threshold);
> -
> - xe_pm_runtime_get(xe);
> - ret = xe_pm_set_vram_threshold(xe, vram_d3cold_threshold);
> - xe_pm_runtime_put(xe);
> -
> - return ret ?: count;
> -}
> -
> -static DEVICE_ATTR_RW(vram_d3cold_threshold);
> -
> -static void xe_pm_sysfs_fini(void *arg)
> -{
> - struct xe_device *xe = arg;
> -
> - sysfs_remove_file(&xe->drm.dev->kobj, &dev_attr_vram_d3cold_threshold.attr);
> -}
> -
> -int xe_pm_sysfs_init(struct xe_device *xe)
> -{
> - struct device *dev = xe->drm.dev;
> - int ret;
> -
> - ret = sysfs_create_file(&dev->kobj, &dev_attr_vram_d3cold_threshold.attr);
> - if (ret)
> - return ret;
> -
> - return devm_add_action_or_reset(dev, xe_pm_sysfs_fini, xe);
> -}
> -
> /**
> * DOC: PCIe Gen5 Limitations
> *
> diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.h b/drivers/gpu/drm/xe/xe_device_sysfs.h
> index b557db0a6023..f9e83d8bd2c7 100644
> --- a/drivers/gpu/drm/xe/xe_device_sysfs.h
> +++ b/drivers/gpu/drm/xe/xe_device_sysfs.h
> @@ -8,7 +8,6 @@
>
> struct xe_device;
>
> -int xe_pm_sysfs_init(struct xe_device *xe);
> int xe_device_sysfs_init(struct xe_device *xe);
yeap, clearly 'xe_pm' here is bad...
But I don't believe that making a new component make things better.
Because pm_sysfs can be many things and in many layer, device, tile or gt.
So, perhaps we add a 'xe_device_pm_sysfs, or perhaps we just kill this
pm_sysfs_init functions and move that to inisde the device_sysfs_init
and add the logic of creating it or not inside it.
>
> #endif
> diff --git a/drivers/gpu/drm/xe/xe_pm.c b/drivers/gpu/drm/xe/xe_pm.c
> index 183813e43b29..365ce76b14ec 100644
> --- a/drivers/gpu/drm/xe/xe_pm.c
> +++ b/drivers/gpu/drm/xe/xe_pm.c
> @@ -16,12 +16,12 @@
> #include "xe_bo.h"
> #include "xe_bo_evict.h"
> #include "xe_device.h"
> -#include "xe_device_sysfs.h"
> #include "xe_ggtt.h"
> #include "xe_gt.h"
> #include "xe_guc.h"
> #include "xe_irq.h"
> #include "xe_pcode.h"
> +#include "xe_pm_sysfs.h"
> #include "xe_pxp.h"
> #include "xe_trace.h"
> #include "xe_wa.h"
> diff --git a/drivers/gpu/drm/xe/xe_pm_sysfs.c b/drivers/gpu/drm/xe/xe_pm_sysfs.c
> new file mode 100644
> index 000000000000..64ddfbf79424
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_pm_sysfs.c
> @@ -0,0 +1,81 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +#include <linux/device.h>
> +#include <linux/kobject.h>
> +#include <linux/pci.h>
> +#include <linux/sysfs.h>
> +
> +#include "xe_device.h"
> +#include "xe_mmio.h"
> +#include "xe_pm.h"
> +#include "xe_pm_sysfs.h"
> +
> +/**
> + * DOC: Xe PM sysfs
> + * Xe driver requires exposing certain tunable knobs controlled by user space
> + * for Power Management. Considering this, we need to add sysfs attributes for
> + * it. These sysfs attributes will be available under pci device kobj directory.
> + *
> + * vram_d3cold_threshold - Report/change vram used threshold(in MB) below
> + * which vram save/restore is permissible during runtime D3cold entry/exit.
> + */
> +
> +static ssize_t
> +vram_d3cold_threshold_show(struct device *dev, struct device_attribute *attr, char *buf)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct xe_device *xe = pdev_to_xe_device(pdev);
> + int ret;
> +
> + xe_pm_runtime_get(xe);
> + ret = sysfs_emit(buf, "%d\n", xe->d3cold.vram_threshold);
> + xe_pm_runtime_put(xe);
> +
> + return ret;
> +}
> +
> +static ssize_t
> +vram_d3cold_threshold_store(struct device *dev, struct device_attribute *attr,
> + const char *buff, size_t count)
> +{
> + struct pci_dev *pdev = to_pci_dev(dev);
> + struct xe_device *xe = pdev_to_xe_device(pdev);
> + u32 vram_d3cold_threshold;
> + int ret;
> +
> + ret = kstrtou32(buff, 0, &vram_d3cold_threshold);
> + if (ret)
> + return ret;
> +
> + drm_dbg(&xe->drm, "vram_d3cold_threshold: %u\n", vram_d3cold_threshold);
> +
> + xe_pm_runtime_get(xe);
> + ret = xe_pm_set_vram_threshold(xe, vram_d3cold_threshold);
> + xe_pm_runtime_put(xe);
> +
> + return ret ?: count;
> +}
> +
> +static DEVICE_ATTR_RW(vram_d3cold_threshold);
> +
> +static void xe_pm_sysfs_fini(void *arg)
> +{
> + struct xe_device *xe = arg;
> +
> + sysfs_remove_file(&xe->drm.dev->kobj, &dev_attr_vram_d3cold_threshold.attr);
> +}
> +
> +int xe_pm_sysfs_init(struct xe_device *xe)
> +{
> + struct device *dev = xe->drm.dev;
> + int ret;
> +
> + ret = sysfs_create_file(&dev->kobj, &dev_attr_vram_d3cold_threshold.attr);
> + if (ret)
> + return ret;
> +
> + return devm_add_action_or_reset(dev, xe_pm_sysfs_fini, xe);
> +}
> diff --git a/drivers/gpu/drm/xe/xe_pm_sysfs.h b/drivers/gpu/drm/xe/xe_pm_sysfs.h
> new file mode 100644
> index 000000000000..9213c178c515
> --- /dev/null
> +++ b/drivers/gpu/drm/xe/xe_pm_sysfs.h
> @@ -0,0 +1,13 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2025 Intel Corporation
> + */
> +
> +#ifndef _XE_PM_SYSFS_H_
> +#define _XE_PM_SYSFS_H_
> +
> +struct xe_device;
> +
> +int xe_pm_sysfs_init(struct xe_device *xe);
> +
> +#endif
> --
> 2.34.1
>
prev parent reply other threads:[~2025-05-05 19:10 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-05 5:37 [PATCH v1] drm/xe: Introduce xe_pm_sysfs.c Raag Jadav
2025-05-05 6:02 ` ✗ CI.Patch_applied: failure for " Patchwork
2025-05-05 19:09 ` Rodrigo Vivi [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aBkM9q-BksNG7D5i@intel.com \
--to=rodrigo.vivi@intel.com \
--cc=anshuman.gupta@intel.com \
--cc=badal.nilawar@intel.com \
--cc=intel-xe@lists.freedesktop.org \
--cc=lucas.demarchi@intel.com \
--cc=raag.jadav@intel.com \
--cc=riana.tauro@intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox