[PATCH V2] drm/admgpu: fix vcn reset sysfs warning

Alex Deucher alexdeucher at gmail.com
Wed Nov 13 06:11:43 UTC 2024


On Wed, Nov 13, 2024 at 12:44 AM Jesse.zhang at amd.com
<jesse.zhang at amd.com> wrote:
>
> From: "Jesse.zhang at amd.com" <Jesse.zhang at amd.com>
>
> sysfs: cannot create duplicate filename '/devices/pci0000:00/0000:00:01.1/0000:01:00.0/0000:02:00.0/0000:03:00.0/vcn_reset_mask'
> [  562.443738] CPU: 13 PID: 4888 Comm: modprobe Tainted: G            E      6.10.0+ #51
> [  562.443740] Hardware name: AMD Splinter/Splinter-RPL, BIOS VS2683299N.FD 05/10/2023
> [  562.443741] Call Trace:
> [  562.443743]  <TASK>
> [  562.443746]  dump_stack_lvl+0x70/0x90
> [  562.443751]  dump_stack+0x14/0x20
> [  562.443753]  sysfs_warn_dup+0x60/0x80
> [  562.443757]  sysfs_add_file_mode_ns+0x126/0x130
> [  562.443760]  sysfs_create_file_ns+0x68/0xa0
> [  562.443762]  device_create_file+0x46/0x90
> [  562.443766]  amdgpu_vcn_sysfs_reset_mask_init+0x1c/0x30 [amdgpu]
> [  562.443991]  vcn_v4_0_3_sw_init+0x270/0x3e0 [amdgpu]
> [  562.444120]  amdgpu_device_init+0x1a0e/0x35a0 [amdgpu]
> [  562.444227]  ? srso_alias_return_thunk+0x5/0xfbef5
> [  562.444230]  ? pci_read_config_word+0x2d/0x50
> [  562.444235]  amdgpu_driver_load_kms+0x1e/0xc0 [amdgpu]
> [  562.444340]  amdgpu_pci_probe+0x1c3/0x660 [amdgpu]
> [  562.444451]  local_pci_probe+0x4c/0xb0
>
> For multiple vcn instances, to avoid creating reset sysfs multiple times,
> add the instance paramter in reset mask init.

parameter.

>
> Signed-off-by: Jesse Zhang <jesse.zhang at amd.com>

What changed with V2?  This looks the same as the original patch.

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c |  8 ++++----
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c   | 10 ++++------
>  drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c |  4 ++--
>  drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c |  4 ++--
>  5 files changed, 14 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 25f490ad3a85..1d4eda649845 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -1295,11 +1295,11 @@ static ssize_t amdgpu_get_vcn_reset_mask(struct device *dev,
>  static DEVICE_ATTR(vcn_reset_mask, 0444,
>                    amdgpu_get_vcn_reset_mask, NULL);
>
> -int amdgpu_vcn_sysfs_reset_mask_init(struct amdgpu_device *adev)
> +int amdgpu_vcn_sysfs_reset_mask_init(struct amdgpu_device *adev, int inst)
>  {
>         int r = 0;
>
> -       if (adev->vcn.num_vcn_inst) {
> +       if (inst == 0) {
>                 r = device_create_file(adev->dev, &dev_attr_vcn_reset_mask);
>                 if (r)
>                         return r;
> @@ -1308,12 +1308,12 @@ int amdgpu_vcn_sysfs_reset_mask_init(struct amdgpu_device *adev)
>         return r;
>  }
>
> -void amdgpu_vcn_sysfs_reset_mask_fini(struct amdgpu_device *adev)
> +void amdgpu_vcn_sysfs_reset_mask_fini(struct amdgpu_device *adev, int inst)
>  {
>         int idx;
>
>         if (drm_dev_enter(adev_to_drm(adev), &idx)) {
> -               if (adev->vcn.num_vcn_inst)
> +               if (inst == 0)
>                         device_remove_file(adev->dev, &dev_attr_vcn_reset_mask);
>                 drm_dev_exit(idx);
>         }

I was thinking something more like:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index 25f490ad3a85..5e6cf8d08a6d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -1295,26 +1295,49 @@ static ssize_t
amdgpu_get_vcn_reset_mask(struct device *dev,
 static DEVICE_ATTR(vcn_reset_mask, 0444,
                   amdgpu_get_vcn_reset_mask, NULL);

-int amdgpu_vcn_sysfs_reset_mask_init(struct amdgpu_device *adev)
+int amdgpu_vcn_sysfs_reset_mask_init(struct amdgpu_device *adev, int inst)
 {
        int r = 0;

        if (adev->vcn.num_vcn_inst) {
-               r = device_create_file(adev->dev, &dev_attr_vcn_reset_mask);
-               if (r)
-                       return r;
+               switch (inst) {
+               case 0:
+                       r = device_create_file(adev->dev,
&dev_attr_vcn_reset_mask);
+                       if (r)
+                               return r;
+                       break;
+               case 1:
+                       r = device_create_file(adev->dev,
&dev_attr_vcn1_reset_mask);
+                       if (r)
+                               return r;
+                       break;
+               case 2:
+                       r = device_create_file(adev->dev,
&dev_attr_vcn2_reset_mask);
+                       if (r)
+                               return r;
+                       break;
+                       ...
+               }
        }

        return r;
 }

-void amdgpu_vcn_sysfs_reset_mask_fini(struct amdgpu_device *adev)
+void amdgpu_vcn_sysfs_reset_mask_fini(struct amdgpu_device *adev, int inst)
 {
        int idx;

        if (drm_dev_enter(adev_to_drm(adev), &idx)) {
-               if (adev->vcn.num_vcn_inst)
-                       device_remove_file(adev->dev, &dev_attr_vcn_reset_mask);
+               if (adev->vcn.num_vcn_inst) {
+                       switch (inst) {
+                       case 0:
+                               device_remove_file(adev->dev,
&dev_attr_vcn_reset_mask);
+                               break;
+                       case 1:
+                               device_remove_file(adev->dev,
&dev_attr_vcn1_reset_mask);
+                               break;
+                               ...
+                       }
                drm_dev_exit(idx);
        }
 }

Alex


> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> index 7ff4ae2a0432..9b10044c61a3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
> @@ -519,7 +519,7 @@ int amdgpu_vcn_ras_sw_init(struct amdgpu_device *adev);
>  int amdgpu_vcn_psp_update_sram(struct amdgpu_device *adev, int inst_idx,
>                                enum AMDGPU_UCODE_ID ucode_id);
>  int amdgpu_vcn_save_vcpu_bo(struct amdgpu_device *adev, int inst);
> -int amdgpu_vcn_sysfs_reset_mask_init(struct amdgpu_device *adev);
> -void amdgpu_vcn_sysfs_reset_mask_fini(struct amdgpu_device *adev);
> +int amdgpu_vcn_sysfs_reset_mask_init(struct amdgpu_device *adev, int inst);
> +void amdgpu_vcn_sysfs_reset_mask_fini(struct amdgpu_device *adev, int inst);
>
>  #endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> index 59f83409d323..109b27904984 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> @@ -250,11 +250,9 @@ static int vcn_v4_0_sw_init(struct amdgpu_ip_block *ip_block)
>                 ip_block->ip_dump = ptr;
>         }
>
> -       if (inst == 0) {
> -               r = amdgpu_vcn_sysfs_reset_mask_init(adev);
> -               if (r)
> -                       return r;
> -       }
> +       r = amdgpu_vcn_sysfs_reset_mask_init(adev, inst);
> +       if (r)
> +               return r;
>
>         return 0;
>  }
> @@ -292,7 +290,7 @@ static int vcn_v4_0_sw_fini(struct amdgpu_ip_block *ip_block)
>         if (r)
>                 return r;
>
> -       amdgpu_vcn_sysfs_reset_mask_fini(adev);
> +       amdgpu_vcn_sysfs_reset_mask_fini(adev, inst);
>         r = amdgpu_vcn_sw_fini(adev, inst);
>
>         kfree(ip_block->ip_dump);
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
> index e9b869f373c9..ef3dfd44a022 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_3.c
> @@ -217,7 +217,7 @@ static int vcn_v4_0_3_sw_init(struct amdgpu_ip_block *ip_block)
>                 ip_block->ip_dump = ptr;
>         }
>
> -       r = amdgpu_vcn_sysfs_reset_mask_init(adev);
> +       r = amdgpu_vcn_sysfs_reset_mask_init(adev, inst);
>         if (r)
>                 return r;
>
> @@ -254,7 +254,7 @@ static int vcn_v4_0_3_sw_fini(struct amdgpu_ip_block *ip_block)
>         if (r)
>                 return r;
>
> -       amdgpu_vcn_sysfs_reset_mask_fini(adev);
> +       amdgpu_vcn_sysfs_reset_mask_fini(adev, inst);
>         r = amdgpu_vcn_sw_fini(adev, inst);
>
>         kfree(ip_block->ip_dump);
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
> index 96ec01cffea3..8f9c19c68d88 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v5_0_0.c
> @@ -186,7 +186,7 @@ static int vcn_v5_0_0_sw_init(struct amdgpu_ip_block *ip_block)
>                 ip_block->ip_dump = ptr;
>         }
>
> -       r = amdgpu_vcn_sysfs_reset_mask_init(adev);
> +       r = amdgpu_vcn_sysfs_reset_mask_init(adev, inst);
>         if (r)
>                 return r;
>
> @@ -223,7 +223,7 @@ static int vcn_v5_0_0_sw_fini(struct amdgpu_ip_block *ip_block)
>         if (r)
>                 return r;
>
> -       amdgpu_vcn_sysfs_reset_mask_fini(adev);
> +       amdgpu_vcn_sysfs_reset_mask_fini(adev, inst);
>         r = amdgpu_vcn_sw_fini(adev, inst);
>
>         kfree(ip_block->ip_dump);
> --
> 2.25.1
>


More information about the amd-gfx mailing list