Re: [Intel-xe] [v2 2/2] drm/xe: Add vram frequency sysfs attributes

["Sundaresan, Sujaritha" <sujaritha.sundaresan@intel.com>]

On 12/6/2023 5:40 PM, Sundaresan, Sujaritha wrote:
>
> On 12/6/2023 5:25 PM, Gupta, Anshuman wrote:
>>
>>> -----Original Message-----
>>> From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of 
>>> Sujaritha
>>> Sundaresan
>>> Sent: Wednesday, December 6, 2023 10:18 AM
>>> To: intel-xe@lists.freedesktop.org
>>> Cc: Sundaresan, Sujaritha <sujaritha.sundaresan@intel.com>; Vivi, 
>>> Rodrigo
>>> <rodrigo.vivi@intel.com>
>>> Subject: [Intel-xe] [v2 2/2] drm/xe: Add vram frequency sysfs 
>>> attributes
>>>
>>> Add vram frequency sysfs attributes under the below hierarchy;
>>>
>>> /device/tile<n>/memory/freq
>>>             |-rp0_freq
>>>             |-rpn_freq
>>>
>>> v2: Drop "vram" from attribute names (Rodrigo)
>>>
>>> Signed-off-by: Sujaritha Sundaresan <sujaritha.sundaresan@intel.com>
>>> ---
>>>   drivers/gpu/drm/xe/xe_pcode_api.h  |  8 ++++
>>> drivers/gpu/drm/xe/xe_tile_sysfs.c | 72 ++++++++++++++++++++++++++++++
>>>   2 files changed, 80 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/xe/xe_pcode_api.h
>>> b/drivers/gpu/drm/xe/xe_pcode_api.h
>>> index 5935cfe30204..edde5335bdb1 100644
>>> --- a/drivers/gpu/drm/xe/xe_pcode_api.h
>>> +++ b/drivers/gpu/drm/xe/xe_pcode_api.h
>>> @@ -42,6 +42,14 @@
>>>   #define        POWER_SETUP_I1_SHIFT        6    /* 10.6 fixed
>>> point format */
>>>   #define        POWER_SETUP_I1_DATA_MASK
>>>     REG_GENMASK(15, 0)
>>>
>>> +#define   XEHP_PCODE_FREQUENCY_CONFIG 0x6e    /* xehp, pvc */
>>> +/* XEHP_PCODE_FREQUENCY_CONFIG sub-commands (param1) */
>>> +#define     PCODE_MBOX_FC_SC_READ_FUSED_P0             0x0
>>> +#define     PCODE_MBOX_FC_SC_READ_FUSED_PN             0x1
>>> +/* PCODE_MBOX_DOMAIN_* - mailbox domain IDs */
>>> +/* XEHP_PCODE_FREQUENCY_CONFIG 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 e8ce4d9270e6..38d334833594 100644
>>> --- a/drivers/gpu/drm/xe/xe_tile_sysfs.c
>>> +++ b/drivers/gpu/drm/xe/xe_tile_sysfs.c
>>> @@ -7,9 +7,14 @@
>>>   #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"
>>>
>>> +#define GT_FREQUENCY_MULTIPLIER    50
>>> +
>>>   static void xe_tile_sysfs_kobj_release(struct kobject *kobj)  {
>>>       kfree(kobj);
>>> @@ -35,6 +40,65 @@ static DEVICE_ATTR_RO(physical_vram_size_bytes);
>>>   static const struct attribute *physical_memsize_attr =
>>>       &dev_attr_physical_vram_size_bytes.attr;
>>>
>>> +static ssize_t rp0_freq_show(struct device *kdev, struct 
>>> device_attribute *attr,
>>> +                  char *buf)
>>> +{
>>> +    struct kobject *kobj = &kdev->kobj;
>>> +    struct xe_tile *tile = kobj_to_tile(kobj->parent);
>>> +    struct xe_gt *gt = tile->primary_gt;
>>> +    u32 val, mbox;
>>> +    int err;
>>> +
>>> +    mbox = REG_FIELD_PREP(PCODE_MB_COMMAND,
>>> XEHP_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 *= GT_FREQUENCY_MULTIPLIER;
>>> +
>>> +    return sysfs_emit(buf, "%u\n", val);
>>> +}
>>> +static DEVICE_ATTR_RO(rp0_freq);
>>> +
>>> +static ssize_t rpn_freq_show(struct device *kdev, struct 
>>> device_attribute *attr,
>>> +                  char *buf)
>>> +{
>>> +    struct kobject *kobj = &kdev->kobj;
>>> +    struct xe_tile *tile = kobj_to_tile(kobj->parent);
>>> +    struct xe_gt *gt = tile->primary_gt;
>>> +    u32 val, mbox;
>>> +    int err;
>>> +
>>> +    mbox = REG_FIELD_PREP(PCODE_MB_COMMAND,
>>> XEHP_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 *= GT_FREQUENCY_MULTIPLIER;
>>> +
>>> +    return sysfs_emit(buf, "%u\n", val);
>>> +}
>>> +static DEVICE_ATTR_RO(rpn_freq);
>>> +
>>> +static struct attribute *freq_attrs[] = {
>>> +    &dev_attr_rp0_freq.attr,
>>> +    &dev_attr_rpn_freq.attr,
>>> +    NULL
>>> +};
>>> +
>>> +static const struct attribute_group freq_group_attrs = {
>>> +    .name = "freq",
>>> +    .attrs = freq_attrs,
>>> +};
>>> +
>>>   static void tile_sysfs_fini(struct drm_device *drm, void *arg)  {
>>>       struct xe_tile *tile = arg;
>>> @@ -78,6 +142,14 @@ void xe_tile_sysfs_init(struct xe_tile *tile)
>>>           drm_warn(&xe->drm,
>>>                "Sysfs creation to read addr_range per tile failed\n");
>>>
>>> +    if (xe->info.platform == XE_PVC) {
>>> +        err = sysfs_create_group(kobj, &freq_group_attrs);
>>> +        if (err) {
>>> +            drm_warn(&xe->drm, "failed to register vram freq sysfs,
>>> err: %d\n", err);
>>> +            return;
>>> +        }
>>> +    }
>>> +
>> We are missing to remove these sysfs files.
>> Thanks,
>> Anshuman Gupta.
>
> Hi Anshuman,
>
> Sure I can add this. Assumed wouldn't be needed when we are 
> un-registering the tile directory.
>
> Also noticed some alignment issues in the show function. Will fix.
>
> Thanks,
>
> Suja

Actually with adding the kobject_put to the fini function, I don't think 
we need the sysfs_remove

cleanup added as well. So i'll skip adding this.

Thanks,

Suja

>
>>>       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",
>>> -- 
>>> 2.25.1