[v7] drm/xe: Add vram frequency sysfs attributes
Sundaresan, Sujaritha
sujaritha.sundaresan at intel.com
Tue Jan 2 11:18:50 UTC 2024
On 1/2/2024 4:19 PM, Riana Tauro wrote:
>
> 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.
Sure, can't hurt to have extra cleanup.
>>>> + 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
Sure will fix above comments, before merge.
Thanks,
Suja
>>>> +
>>>> +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