<!DOCTYPE html><html><head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> </head> <body> <div class="moz-cite-prefix">On 12/14/2023 7:10 PM, Rodrigo Vivi wrote: </div> <blockquote type="cite" cite="mid:ZXsF5POCTNwVoO7K@intel.com"> <pre class="moz-quote-pre" wrap="">On Thu, Dec 14, 2023 at 04:41:59PM +0530, Sujaritha Sundaresan wrote: </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">Add vram frequency sysfs attributes under the below hierarchy; /device/tile#/freq0 |-rp0_freq |-rpn_freq </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> hmm... you have a good point of not leaving an empty memory dir on platforms where we don't have the freq interfaces. But then we should only create the mem dir when we are going to create the freq. In the way this patch is proposing it looks like the frequency is about the tile. like a tile basedie frequency or something like that. So, we do need the memory or vram dir to make it really clear that the freq is about the memory. </pre> </blockquote> <blockquote type="cite" cite="mid:ZXsF5POCTNwVoO7K@intel.com"> <blockquote type="cite"> <pre class="moz-quote-pre" wrap=""> 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 Signed-off-by: Sujaritha Sundaresan <a class="moz-txt-link-rfc2396E" href="mailto:sujaritha.sundaresan@intel.com"><sujaritha.sundaresan@intel.com></a> --- drivers/gpu/drm/xe/xe_pcode_api.h | 8 +++ drivers/gpu/drm/xe/xe_tile_sysfs.c | 80 ++++++++++++++++++++++++++++++ 2 files changed, 88 insertions(+) diff --git a/drivers/gpu/drm/xe/xe_pcode_api.h b/drivers/gpu/drm/xe/xe_pcode_api.h index 5935cfe30204..26ceb8c0c010 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 PCODE_FREQUENCY_CONFIG 0x6e +/* 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 */ +/* 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 0f8d3e7fce46..45518442ed15 100644 --- a/drivers/gpu/drm/xe/xe_tile_sysfs.c +++ b/drivers/gpu/drm/xe/xe_tile_sysfs.c @@ -7,9 +7,23 @@ #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 </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> hmmm... it is kind of strange to define a frequency multiplier in the tile_sysfs component. This is probably an indication that we need a specific component for this memory freq? perhaps xe_vram_freq?</pre> </blockquote> I would prefer not to over complicate for the sake of two attributes that appear only on PVC. I can revert the path to /tile#/memory/freq0 and have all of it appear only for PVC. Thoughts ? <blockquote type="cite" cite="mid:ZXsF5POCTNwVoO7K@intel.com"> <pre class="moz-quote-pre" wrap=""> </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap="">+ +/** + * DOC: Xe Tile sysfs + * + * Provides sysfs entries for memory related frequency in tile + * + * device/tile#/freq0/rp0_freq - The Render Performance 0 level, which is the maximum one. + * device/tile#/freq0/rpn_freq - The Render Performance N level, which is the minimal one. + */ + static void xe_tile_sysfs_kobj_release(struct kobject *kobj) { kfree(kobj); @@ -20,10 +34,68 @@ static const struct kobj_type xe_tile_sysfs_kobj_type = { .sysfs_ops = &kobj_sysfs_ops, }; +static ssize_t rp0_freq_show(struct device *kdev, struct device_attribute *attr, + char *buf) +{ + struct xe_tile *tile = kobj_to_tile(&kdev->kobj); + 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 *= 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 xe_tile *tile = kobj_to_tile(&kdev->kobj); + 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 *= 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 = "freq0", + .attrs = freq_attrs, +}; + static void tile_sysfs_fini(struct drm_device *drm, void *arg) { struct xe_tile *tile = arg; + sysfs_remove_group(tile->sysfs, &freq_group_attrs); </pre> </blockquote> <pre class="moz-quote-pre" wrap=""> if you didn't create the group you shouldn't remove it, so same conditions for create should apply here. </pre> <blockquote type="cite"> <pre class="moz-quote-pre" wrap=""> kobject_put(tile->sysfs); } @@ -50,6 +122,14 @@ void xe_tile_sysfs_init(struct xe_tile *tile) tile->sysfs = &kt->base; + if (xe->info.platform == XE_PVC) { + err = sysfs_create_group(tile->sysfs, &freq_group_attrs); + if (err) { + drm_warn(&xe->drm, "failed to register vram freq sysfs, err: %d\n", err); + return; + } + } + 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 </pre> </blockquote> </blockquote> </body> </html>