[v7] drm/xe: Add vram frequency sysfs attributes
Riana Tauro
riana.tauro at intel.com
Tue Jan 2 10:49:24 UTC 2024
Hi Suja
On 1/2/2024 3:48 PM, Sundaresan, Sujaritha wrote:
>
> On 1/2/2024 3:39 PM, Gupta, Anshuman wrote:
>>
>>> -----Original Message-----
>>> From: Sundaresan, Sujaritha <sujaritha.sundaresan at intel.com>
>>> Sent: Tuesday, January 2, 2024 10:58 AM
>>> To: intel-xe at lists.freedesktop.org
>>> Cc: Gupta, Anshuman <anshuman.gupta at intel.com>; Sundaresan, Sujaritha
>>> <sujaritha.sundaresan at intel.com>
>>> Subject: [v7] drm/xe: Add vram frequency sysfs attributes
>>>
>>> Add vram frequency sysfs attributes under the below hierarchy;
>>>
>>> /device/tile#/memory/freq0
>>> |-max_freq
>>> |-min_freq
>>>
>>> v2: Drop "vram" from attribute names (Rodrigo)
>>>
>>> v3: Add documentation for new sysfs (Riana)
>>> Drop prefix from XEHP_PCODE_FREQUENCY_CONFIG (Riana)
>>>
>>> v4: Create sysfs under tile#/freq0 after removal of
>>> physical_memsize attrbute
>>>
>>> v5: Revert back to creating sysfs under tile#/memory/freq0
>>> Remove definition of GT_FREQUENCY_MULTIPLIER (Rodrigo)
>>>
>>> v6: Rename attributes to max/min_freq (Anshuman)
>>> Fix review comments (Rodrigo)
>>>
>>> v7: Make docuemntation more verbose
typo
>>> Move sysfs to separate file (Anshuman)
Separate file will not be needed if drmm_add_action_or_reset is added
similar to the file xe_hw_engine_class_sysfs.c
>>>
>>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan at intel.com>
>>> ---
>>> drivers/gpu/drm/xe/Makefile | 1 +
>>> drivers/gpu/drm/xe/xe_pcode_api.h | 7 ++
>>> drivers/gpu/drm/xe/xe_tile_sysfs.c | 3 +
>>> drivers/gpu/drm/xe/xe_vram_freq.c | 127
>>> +++++++++++++++++++++++++++++ drivers/gpu/drm/xe/xe_vram_freq.h
>>> | 16 ++++
>>> 5 files changed, 154 insertions(+)
>>> create mode 100644 drivers/gpu/drm/xe/xe_vram_freq.c create mode
>>> 100644 drivers/gpu/drm/xe/xe_vram_freq.h
>>>
>>> diff --git a/drivers/gpu/drm/xe/Makefile
>>> b/drivers/gpu/drm/xe/Makefile index
>>> df8601d6a59f..17884e422cec 100644
>>> --- a/drivers/gpu/drm/xe/Makefile
>>> +++ b/drivers/gpu/drm/xe/Makefile
>>> @@ -139,6 +139,7 @@ xe-y += xe_bb.o \
>>> xe_uc_debugfs.o \
>>> xe_uc_fw.o \
>>> xe_vm.o \
>>> + xe_vram_freq.o \
>>> xe_wait_user_fence.o \
>>> xe_wa.o \
>>> xe_wopcm.o
>>> diff --git a/drivers/gpu/drm/xe/xe_pcode_api.h
>>> b/drivers/gpu/drm/xe/xe_pcode_api.h
>>> index 5935cfe30204..f153ce96f69a 100644
>>> --- a/drivers/gpu/drm/xe/xe_pcode_api.h
>>> +++ b/drivers/gpu/drm/xe/xe_pcode_api.h
>>> @@ -42,6 +42,13 @@
>>> #define POWER_SETUP_I1_SHIFT 6 /* 10.6 fixed
>>> point format */
>>> #define POWER_SETUP_I1_DATA_MASK
>>> REG_GENMASK(15, 0)
>>>
>>> +#define PCODE_FREQUENCY_CONFIG 0x6e
>>> +/* Frequency Config Sub Commands (param1) */
>>> +#define PCODE_MBOX_FC_SC_READ_FUSED_P0 0x0
>>> +#define PCODE_MBOX_FC_SC_READ_FUSED_PN 0x1
>>> +/* Domain IDs (param2) */
>>> +#define PCODE_MBOX_DOMAIN_HBM 0x2
>>> +
>>> struct pcode_err_decode {
>>> int errno;
>>> const char *str;
>>> diff --git a/drivers/gpu/drm/xe/xe_tile_sysfs.c
>>> b/drivers/gpu/drm/xe/xe_tile_sysfs.c
>>> index 0f8d3e7fce46..ed60d12f5cf0 100644
>>> --- a/drivers/gpu/drm/xe/xe_tile_sysfs.c
>>> +++ b/drivers/gpu/drm/xe/xe_tile_sysfs.c
>>> @@ -9,6 +9,7 @@
>>>
>>> #include "xe_tile.h"
>>> #include "xe_tile_sysfs.h"
>>> +#include "xe_vram_freq.h"
>>>
>>> static void xe_tile_sysfs_kobj_release(struct kobject *kobj) { @@
>>> -50,6 +51,8
>>> @@ void xe_tile_sysfs_init(struct xe_tile *tile)
>>>
>>> tile->sysfs = &kt->base;
>>>
>>> + xe_vram_freq_init(tile);
>>> +
>>> err = drmm_add_action_or_reset(&xe->drm, tile_sysfs_fini, tile);
>>> if (err)
>>> drm_warn(&xe->drm, "%s: drmm_add_action_or_reset failed,
>>> err: %d\n", diff --git a/drivers/gpu/drm/xe/xe_vram_freq.c
>>> b/drivers/gpu/drm/xe/xe_vram_freq.c
>>> new file mode 100644
>>> index 000000000000..166d41a6b222
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/xe/xe_vram_freq.c
>>> @@ -0,0 +1,127 @@
>>> +// SPDX-License-Identifier: MIT
>>> +/*
>>> + * Copyright © 2023 Intel Corporation >>> + */
>>> +#include <linux/sysfs.h>
>>> +#include <drm/drm_managed.h>
>>> +
>>> +#include "xe_gt_types.h"
>>> +#include "xe_pcode.h"
>>> +#include "xe_pcode_api.h"
>>> +#include "xe_tile.h"
>>> +#include "xe_tile_sysfs.h"
unnecessary header
>>> +#include "xe_vram_freq.h"
>>> +
>>> +/**
>>> + * DOC: Xe VRAM freq
>>> + *
>>> + * Provides sysfs entries for vram frequency in tile
>>> + *
>>> + * device/tile#/memory/freq0/max_freq - This is maximum frequency. This
>>> value is read-only as it
>>> + * is the fixed fuse point P0. It is not the
>>> system
>>> + * configuration.
>>> + * device/tile#/memory/freq0/min_freq - This is minimum frequency. This
>>> value is read-only as it
>>> + * is the fixed fuse point PN. It is not the
>>> system
>>> + * configuration.
>>> + */
>>> +
>>> +static struct xe_tile *dev_to_tile(struct device *dev) {
>>> + return kobj_to_tile(dev->kobj.parent); }
>>> +
>>> +static ssize_t max_freq_show(struct device *dev, struct
>>> device_attribute
>>> *attr,
>>> + char *buf)
>>> +{
>>> + struct xe_tile *tile = dev_to_tile(dev);
>>> + struct xe_gt *gt = tile->primary_gt;
>>> + u32 val, mbox;
>>> + int err;
>>> +
>>> + mbox = REG_FIELD_PREP(PCODE_MB_COMMAND,
>>> PCODE_FREQUENCY_CONFIG)
>>> + | REG_FIELD_PREP(PCODE_MB_PARAM1,
>>> PCODE_MBOX_FC_SC_READ_FUSED_P0)
>>> + | REG_FIELD_PREP(PCODE_MB_PARAM2,
>>> PCODE_MBOX_DOMAIN_HBM);
>>> +
>>> + err = xe_pcode_read(gt, mbox, &val, NULL);
>>> + if (err)
>>> + return err;
>>> +
>>> + /* data_out - Fused P0 for domain ID in units of 50 MHz */
>>> + val *= 50;
>>> +
>>> + return sysfs_emit(buf, "%u\n", val);
>>> +}
>>> +static DEVICE_ATTR_RO(max_freq);
>>> +
>>> +static ssize_t min_freq_show(struct device *dev, struct
>>> device_attribute *attr,
>>> + char *buf)
>>> +{
>>> + struct xe_tile *tile = dev_to_tile(dev);
>>> + struct xe_gt *gt = tile->primary_gt;
>>> + u32 val, mbox;
>>> + int err;
>>> +
>>> + mbox = REG_FIELD_PREP(PCODE_MB_COMMAND,
>>> PCODE_FREQUENCY_CONFIG)
>>> + | REG_FIELD_PREP(PCODE_MB_PARAM1,
>>> PCODE_MBOX_FC_SC_READ_FUSED_PN)
>>> + | REG_FIELD_PREP(PCODE_MB_PARAM2,
>>> PCODE_MBOX_DOMAIN_HBM);
>>> +
>>> + err = xe_pcode_read(gt, mbox, &val, NULL);
>>> + if (err)
>>> + return err;
>>> +
>>> + /* data_out - Fused Pn for domain ID in units of 50 MHz */
>>> + val *= 50;
>>> +
>>> + return sysfs_emit(buf, "%u\n", val);
>>> +}
>>> +static DEVICE_ATTR_RO(min_freq);
>>> +
>>> +static struct attribute *freq_attrs[] = {
>>> + &dev_attr_max_freq.attr,
>>> + &dev_attr_min_freq.attr,
>>> + NULL
>>> +};
>>> +
>>> +static const struct attribute_group freq_group_attrs = {
>>> + .name = "freq0",
>>> + .attrs = freq_attrs,
>>> +};
>>> +
>>> +static void vram_freq_fini(struct drm_device *drm, void *arg) {
>>> + struct kobject *kobj = arg;
>>> +
>>> + sysfs_remove_group(kobj, &freq_group_attrs);
>>> + kobject_put(kobj);
>>> +}
>>> +
>>> +void xe_vram_freq_init(struct xe_tile *tile) {
>> Please provide a kernel function doc for this exported function.
>>> + struct xe_device *xe = tile_to_xe(tile);
>>> + struct kobject *kobj;
>>> + int err;
>>> +
>>> + if (xe->info.platform == XE_PVC) {
>> Drop these platform checks(including below ones) , instead use a early
>> return at starts of this function.
>> If (xe->info.platform != XE_PVC)
>> Return.
>> With all of above comment.
>> Reviewed-by: Anshuman Gupta <anshuman.gupta at intel.com>
>
> Will make the changes. Thanks for the r-b.
>
> Regards,
>
> Suja
>
>>> + kobj = kobject_create_and_add("memory", tile->sysfs);
>>> + if (!kobj)
>>> + drm_warn(&xe->drm, "failed to add memory
>>> directory, err: %d\n", -ENOMEM);
>>> + }
>>> +
>>> + if (kobj && xe->info.platform == XE_PVC) {
>>> + err = sysfs_create_group(kobj, &freq_group_attrs);
>>> + if (err) {
Need kobject_put here?
If there is failure in creating freq attrs, memory dir will be empty.
Should be removed.
>>> + drm_warn(&xe->drm, "failed to register vram freq
>>> sysfs, err: %d\n", err);
>>> + return;
>>> + }
>>> + }
>>> +
>>> + if (xe->info.platform == XE_PVC) {
>>> + err = drmm_add_action_or_reset(&xe->drm, vram_freq_fini,
>>> kobj);
>>> + if (err)
>>> + drm_warn(&xe->drm, "%s:
>>> drmm_add_action_or_reset failed, err: %d\n",
>>> + __func__, err);
>>> + }
>>> +
>>> +}
>>> +
>>> +
>>> diff --git a/drivers/gpu/drm/xe/xe_vram_freq.h
>>> b/drivers/gpu/drm/xe/xe_vram_freq.h
>>> new file mode 100644
>>> index 000000000000..94b04178a798
>>> --- /dev/null
>>> +++ b/drivers/gpu/drm/xe/xe_vram_freq.h
>>> @@ -0,0 +1,16 @@
>>> +/* SPDX-License-Identifier: MIT */
>>> +/*
>>> + * Copyright © 2023 Intel Corporation
2024
>>> + */
>>> +
>>> +#ifndef _XE_VRAM_FREQ_H_
>>> +#define _XE_VRAM_FREQ_H_
>>> +
>>> +#include <drm/drm_managed.h>
>>> +
>>> +#include "xe_device.h"
>>> +#include "xe_tile_sysfs.h"
unnecessary headers. Use forward declaration of tile
Thanks
Riana
>>> +
>>> +void xe_vram_freq_init(struct xe_tile *tile);
>>> +
>>> +#endif /* _XE_VRAM_FREQ_H_ */
>>> --
>>> 2.25.1
More information about the Intel-xe
mailing list