[Intel-xe] [PATCH 1/2] drm/xe/ats-m: Expose uid for ATS-M via sysfs

[Michal Wajdeczko]

On 08.11.2023 14:54, Tejas Upadhyay wrote:
> This patch exports uid to the userspace via sysfs.
> For example, uid can be read by:
> cat /sys/class/drm/cardX/device/csc_unique_id
> 
> Signed-off-by: Tejas Upadhyay <tejas.upadhyay at intel.com>
> ---
>  drivers/gpu/drm/xe/regs/xe_regs.h    |  3 +++
>  drivers/gpu/drm/xe/xe_device.c       | 11 +++++++++++
>  drivers/gpu/drm/xe/xe_device_sysfs.c | 21 +++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_device_types.h |  4 ++++
>  drivers/gpu/drm/xe/xe_pci.c          |  3 +++
>  5 files changed, 42 insertions(+)
> 
> diff --git a/drivers/gpu/drm/xe/regs/xe_regs.h b/drivers/gpu/drm/xe/regs/xe_regs.h
> index a646d13af03a..1b8de9b8daf1 100644
> --- a/drivers/gpu/drm/xe/regs/xe_regs.h
> +++ b/drivers/gpu/drm/xe/regs/xe_regs.h
> @@ -70,6 +70,9 @@
>  #define XEHP_CLOCK_GATE_DIS			XE_REG(0x101014)
>  #define   SGSI_SIDECLK_DIS			REG_BIT(17)
>  
> +#define DEVUID_LWORD				XE_REG(0x102008)
> +#define DEVUID_HWORD				XE_REG(0x10200C)
> +
>  #define GGC					XE_REG(0x108040)
>  #define   GMS_MASK				REG_GENMASK(15, 8)
>  #define   GGMS_MASK				REG_GENMASK(7, 6)
> diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c
> index 515cdf599fab..266e0d5d7159 100644
> --- a/drivers/gpu/drm/xe/xe_device.c
> +++ b/drivers/gpu/drm/xe/xe_device.c
> @@ -344,6 +344,15 @@ static void xe_device_sanitize(struct drm_device *drm, void *arg)
>  		xe_gt_sanitize(gt);
>  }
>  
> +static void xe_read_dev_uid(struct xe_device *xe)
> +{
> +	if (xe->info.has_uid) {
> +		struct xe_gt *mmio = xe_root_mmio_gt(xe);
> +
> +		xe->info.uid  = xe_mmio_read64_2x32(mmio, DEVUID_LWORD);
> +	}
> +}
> +
>  int xe_device_probe(struct xe_device *xe)
>  {
>  	struct xe_tile *tile;
> @@ -368,6 +377,8 @@ int xe_device_probe(struct xe_device *xe)
>  	if (err)
>  		return err;
>  
> +	xe_read_dev_uid(xe);

as this function updates xe->info then maybe it should be a part of
xe_info_init() ?

> +
>  	err = drmm_add_action_or_reset(&xe->drm, xe_driver_flr_fini, xe);
>  	if (err)
>  		return err;
> diff --git a/drivers/gpu/drm/xe/xe_device_sysfs.c b/drivers/gpu/drm/xe/xe_device_sysfs.c
> index 99113a5a2b84..66f69eada76f 100644
> --- a/drivers/gpu/drm/xe/xe_device_sysfs.c
> +++ b/drivers/gpu/drm/xe/xe_device_sysfs.c
> @@ -65,11 +65,24 @@ vram_d3cold_threshold_store(struct device *dev, struct device_attribute *attr,
>  
>  static DEVICE_ATTR_RW(vram_d3cold_threshold);
>  
> +static ssize_t csc_unique_id_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);
> +
> +	return sysfs_emit(buf, "%llx\n", xe->info.uid);

%#llx

otherwise there will be no 0x prefix so could be treated as decimal

> +}
> +
> +static DEVICE_ATTR_RO(csc_unique_id);

what's "csc" ?

in patch you use different names:
 - csc_unique_id
 - UID
 - devuid


> +
>  static void xe_device_sysfs_fini(struct drm_device *drm, void *arg)
>  {
>  	struct xe_device *xe = arg;
>  
>  	sysfs_remove_file(&xe->drm.dev->kobj, &dev_attr_vram_d3cold_threshold.attr);
> +	sysfs_remove_file(&xe->drm.dev->kobj, &dev_attr_csc_unique_id.attr);
>  }
>  
>  void xe_device_sysfs_init(struct xe_device *xe)
> @@ -83,6 +96,14 @@ void xe_device_sysfs_init(struct xe_device *xe)
>  		return;
>  	}
>  
> +	if (xe->info.has_uid) {
> +		ret = sysfs_create_file(&dev->kobj, &dev_attr_csc_unique_id.attr);
> +		if (ret) {
> +			drm_warn(&xe->drm, "UID sysfs setup failed\n");

if you really want to print warn then maybe also include %pe to help
diagnose why it failed

> +			return;
> +		}
> +	}
> +
>  	ret = drmm_add_action_or_reset(&xe->drm, xe_device_sysfs_fini, xe);
>  	if (ret)
>  		drm_warn(&xe->drm, "Failed to add sysfs fini drm action\n");
> diff --git a/drivers/gpu/drm/xe/xe_device_types.h b/drivers/gpu/drm/xe/xe_device_types.h
> index 4119ef03fb7e..1cc8488f8c86 100644
> --- a/drivers/gpu/drm/xe/xe_device_types.h
> +++ b/drivers/gpu/drm/xe/xe_device_types.h
> @@ -238,6 +238,8 @@ struct xe_device {
>  		u8 vm_max_level;
>  		/** @va_bits: Maximum bits of a virtual address */
>  		u8 va_bits;
> +		/** @uid: device uid, used for generating uuid */
> +		u64 uid;
>  
>  		/** @is_dgfx: is discrete device */
>  		u8 is_dgfx:1;
> @@ -261,6 +263,8 @@ struct xe_device {
>  		u8 supports_mmio_ext:1;
>  		/** @has_heci_gscfi: device has heci gscfi */
>  		u8 has_heci_gscfi:1;
> +		/** @has_uid: device has uid, used for generating uuid */
> +		u8 has_uid:1;

IMO introduction of has_uid capability flag should be done in separate
patch than sysfs and uid changes

>  
>  #if IS_ENABLED(CONFIG_DRM_XE_DISPLAY)
>  		struct {
> diff --git a/drivers/gpu/drm/xe/xe_pci.c b/drivers/gpu/drm/xe/xe_pci.c
> index 2fae45b9d88e..4d91ad049e59 100644
> --- a/drivers/gpu/drm/xe/xe_pci.c
> +++ b/drivers/gpu/drm/xe/xe_pci.c
> @@ -60,6 +60,7 @@ struct xe_device_desc {
>  	u8 has_heci_gscfi:1;
>  
>  	u8 has_llc:1;
> +	u8 has_uid;
>  	u8 bypass_mtcfg:1;
>  	u8 supports_mmio_ext:1;
>  };
> @@ -298,6 +299,7 @@ static const struct xe_device_desc ats_m_desc = {
>  
>  	DG2_FEATURES,
>  	.has_display = false,
> +	.has_uid = true,
>  };
>  
>  static const struct xe_device_desc dg2_desc = {
> @@ -577,6 +579,7 @@ static int xe_info_init(struct xe_device *xe,
>  	xe->info.graphics_name = graphics_desc->name;
>  	xe->info.media_name = media_desc ? media_desc->name : "none";
>  	xe->info.has_llc = desc->has_llc;
> +	xe->info.has_uid = desc->has_uid;
>  	xe->info.bypass_mtcfg = desc->bypass_mtcfg;
>  	xe->info.supports_mmio_ext = desc->supports_mmio_ext;
>  	xe->info.tile_mmio_ext_size = graphics_desc->tile_mmio_ext_size;