I don't think that anyone understands all of the internals of FW version handling 😉 Right now we're just returning the same values as what debugfs returns. The SMI tool will do the version interpretation (like turning 0x00282b00 into 40.43 for the SMU, for example). Or knowing that the CP firmware is in decimal, while the VCN is in hex. It's all confusing and inconsistent, but the sysfs files won't change from here on out, just how the SMI interprets those hex values. Thanks!
Kent
-----Original Message-----
From: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
Sent: Tuesday, May 14, 2019 6:33 AM
To: Russell, Kent <Kent.Russell@xxxxxxx>; Messinger, Ori <Ori.Messinger@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Subject: Re: [PATCH] drm/amdgpu: Report firmware versions with sysfs v2
[CAUTION: External Email]
I don't claim that I understand all of the internals of fw version handling, but this looks really nice to me from a kernel perspective.
Feel free to add my Reviewed-by: Christian König <christian.koenig@xxxxxxx>.
Regards,
Christian.
Am 14.05.19 um 12:19 schrieb Russell, Kent:
> Looks fine to me, but hoping to get an RB from Christian now that we're moving them into a subfolder.
>
> Kent
>
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of
> Messinger, Ori
> Sent: Thursday, May 9, 2019 4:35 PM
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Messinger, Ori <Ori.Messinger@xxxxxxx>
> Subject: [PATCH] drm/amdgpu: Report firmware versions with sysfs v2
>
> [CAUTION: External Email]
>
> Firmware versions can be found as separate sysfs files at:
> /sys/class/drm/cardX/device/fw_version (where X is the card number) The firmware versions are displayed in hexadecimal.
> v2: Moved sysfs files to subfolder
>
> Change-Id: I10cae4c0ca6f1b6a9ced07da143426e1d011e203
> Signed-off-by: Ori Messinger <ori.messinger@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 5 ++ drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c | 63 ++++++++++++++++++++++ drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h | 2 +
> 3 files changed, 70 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index ec864740efb5..8c56c49c8dfc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -2705,6 +2705,10 @@ int amdgpu_device_init(struct amdgpu_device *adev,
> if (r)
> DRM_ERROR("registering pm debugfs failed (%d).\n",
> r);
>
> + r = amdgpu_ucode_sysfs_init(adev);
> + if (r)
> + DRM_ERROR("Creating firmware sysfs failed (%d).\n",
> + r);
> +
> r = amdgpu_debugfs_gem_init(adev);
> if (r)
> DRM_ERROR("registering gem debugfs failed (%d).\n", r); @@ -2817,6 +2821,7 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
> amdgpu_device_doorbell_fini(adev);
> amdgpu_debugfs_regs_cleanup(adev);
> device_remove_file(adev->dev, &dev_attr_pcie_replay_count);
> + amdgpu_ucode_sysfs_fini(adev);
> }
>
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> index 7b33867036e7..33c1eb76c076 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.c
> @@ -313,6 +313,69 @@ amdgpu_ucode_get_load_type(struct amdgpu_device *adev, int load_type)
> return AMDGPU_FW_LOAD_DIRECT;
> }
>
> +#define FW_VERSION_ATTR(name, mode, field) \
> +static ssize_t show_##name(struct device *dev, \
> + struct device_attribute *attr, \
> + char *buf) \
> +{ \
> + struct drm_device *ddev = dev_get_drvdata(dev); \
> + struct amdgpu_device *adev = ddev->dev_private; \
> + \
> + return snprintf(buf, PAGE_SIZE, "0x%08x\n", adev->field); \
> +} \
> +static DEVICE_ATTR(name, mode, show_##name, NULL)
> +
> +FW_VERSION_ATTR(vce_fw_version, 0444, vce.fw_version);
> +FW_VERSION_ATTR(uvd_fw_version, 0444, uvd.fw_version);
> +FW_VERSION_ATTR(mc_fw_version, 0444, gmc.fw_version);
> +FW_VERSION_ATTR(me_fw_version, 0444, gfx.me_fw_version);
> +FW_VERSION_ATTR(pfp_fw_version, 0444, gfx.pfp_fw_version);
> +FW_VERSION_ATTR(ce_fw_version, 0444, gfx.ce_fw_version);
> +FW_VERSION_ATTR(rlc_fw_version, 0444, gfx.rlc_fw_version);
> +FW_VERSION_ATTR(rlc_srlc_fw_version, 0444, gfx.rlc_srlc_fw_version);
> +FW_VERSION_ATTR(rlc_srlg_fw_version, 0444, gfx.rlc_srlg_fw_version);
> +FW_VERSION_ATTR(rlc_srls_fw_version, 0444, gfx.rlc_srls_fw_version);
> +FW_VERSION_ATTR(mec_fw_version, 0444, gfx.mec_fw_version);
> +FW_VERSION_ATTR(mec2_fw_version, 0444, gfx.mec2_fw_version);
> +FW_VERSION_ATTR(sos_fw_version, 0444, psp.sos_fw_version);
> +FW_VERSION_ATTR(asd_fw_version, 0444, psp.asd_fw_version);
> +FW_VERSION_ATTR(ta_ras_fw_version, 0444, psp.ta_fw_version);
> +FW_VERSION_ATTR(ta_xgmi_fw_version, 0444, psp.ta_fw_version);
> +FW_VERSION_ATTR(smc_fw_version, 0444, pm.fw_version);
> +FW_VERSION_ATTR(sdma_fw_version, 0444, sdma.instance[0].fw_version);
> +FW_VERSION_ATTR(sdma2_fw_version, 0444, sdma.instance[1].fw_version);
> +FW_VERSION_ATTR(vcn_fw_version, 0444, vcn.fw_version);
> +FW_VERSION_ATTR(dmcu_fw_version, 0444, dm.dmcu_fw_version);
> +
> +static struct attribute *fw_attrs[] = {
> + &dev_attr_vce_fw_version.attr, &dev_attr_uvd_fw_version.attr,
> + &dev_attr_mc_fw_version.attr, &dev_attr_me_fw_version.attr,
> + &dev_attr_pfp_fw_version.attr, &dev_attr_ce_fw_version.attr,
> + &dev_attr_rlc_fw_version.attr, &dev_attr_rlc_srlc_fw_version.attr,
> + &dev_attr_rlc_srlg_fw_version.attr, &dev_attr_rlc_srls_fw_version.attr,
> + &dev_attr_mec_fw_version.attr, &dev_attr_mec2_fw_version.attr,
> + &dev_attr_sos_fw_version.attr, &dev_attr_asd_fw_version.attr,
> + &dev_attr_ta_ras_fw_version.attr, &dev_attr_ta_xgmi_fw_version.attr,
> + &dev_attr_smc_fw_version.attr, &dev_attr_sdma_fw_version.attr,
> + &dev_attr_sdma2_fw_version.attr, &dev_attr_vcn_fw_version.attr,
> + &dev_attr_dmcu_fw_version.attr, NULL };
> +
> +static const struct attribute_group fw_attr_group = {
> + .name = "fw_version",
> + .attrs = fw_attrs
> +};
> +
> +int amdgpu_ucode_sysfs_init(struct amdgpu_device *adev) {
> + return sysfs_create_group(&adev->dev->kobj, &fw_attr_group); }
> +
> +void amdgpu_ucode_sysfs_fini(struct amdgpu_device *adev) {
> + sysfs_remove_group(&adev->dev->kobj, &fw_attr_group); }
> +
> static int amdgpu_ucode_init_single_fw(struct amdgpu_device *adev,
> struct amdgpu_firmware_info *ucode,
> uint64_t mc_addr, void *kptr)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> index 7ac25a1c7853..ec4c2ea1f05a 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> @@ -291,7 +291,9 @@ bool amdgpu_ucode_hdr_version(union
> amdgpu_firmware_header *hdr,
>
> int amdgpu_ucode_init_bo(struct amdgpu_device *adev); int
> amdgpu_ucode_create_bo(struct amdgpu_device *adev);
> +int amdgpu_ucode_sysfs_init(struct amdgpu_device *adev);
> void amdgpu_ucode_free_bo(struct amdgpu_device *adev);
> +void amdgpu_ucode_sysfs_fini(struct amdgpu_device *adev);
>
> enum amdgpu_firmware_load_type
> amdgpu_ucode_get_load_type(struct amdgpu_device *adev, int
> load_type);
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
