Re: [PATCH 43/44] drm/amdgpu/vcn: optimize firmware storage

[Boyuan Zhang]

On 2025-01-31 11:57, Alex Deucher wrote:

If each instance uses the same fw image, only store one
copy in the driver.

Signed-off-by: Alex Deucher<alexander.deuc...@amd.com>

Acked-by: Boyuan Zhang <boyuan.zh...@amd.com> <mailto:boyuan.zh...@amd.com>

---
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 30 +++++++++++++++++--------
  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h |  4 +++-
  drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c |  3 +++
  3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
index e4ef0fb970b29..edbcb11c382a2 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
@@ -102,18 +102,25 @@ int amdgpu_vcn_early_init(struct amdgpu_device *adev, int 
i)
        adev->vcn.inst[i].inst = i;
        amdgpu_ucode_ip_version_decode(adev, UVD_HWIP, ucode_prefix, 
sizeof(ucode_prefix));

- if (i == 1 && amdgpu_ip_version(adev, UVD_HWIP, 0) == IP_VERSION(4, 0, 6))

+       if (i != 0 && adev->vcn.per_inst_fw) {
                r = amdgpu_ucode_request(adev, &adev->vcn.inst[i].fw,
                                         AMDGPU_UCODE_REQUIRED,
                                         "amdgpu/%s_%d.bin", ucode_prefix, i);
-       else
-               r = amdgpu_ucode_request(adev, &adev->vcn.inst[i].fw,
-                                        AMDGPU_UCODE_REQUIRED,
-                                        "amdgpu/%s.bin", ucode_prefix);
-       if (r) {
-               amdgpu_ucode_release(&adev->vcn.inst[i].fw);
-               return r;
+               if (r)
+                       amdgpu_ucode_release(&adev->vcn.inst[i].fw);
+       } else {
+               if (!adev->vcn.inst[0].fw) {
+                       r = amdgpu_ucode_request(adev, &adev->vcn.inst[0].fw,
+                                                AMDGPU_UCODE_REQUIRED,
+                                                "amdgpu/%s.bin", ucode_prefix);
+                       if (r)
+                               amdgpu_ucode_release(&adev->vcn.inst[0].fw);
+               } else {
+                       r = 0;
+               }
+               adev->vcn.inst[i].fw = adev->vcn.inst[0].fw;
        }
+
        return r;
  }

@@ -270,7 +277,12 @@ int amdgpu_vcn_sw_fini(struct amdgpu_device *adev, int i)

        for (j = 0; j < adev->vcn.inst[i].num_enc_rings; ++j)
                amdgpu_ring_fini(&adev->vcn.inst[i].ring_enc[j]);

- amdgpu_ucode_release(&adev->vcn.inst[i].fw);

+       if (adev->vcn.per_inst_fw) {
+               amdgpu_ucode_release(&adev->vcn.inst[i].fw);
+       } else {
+               amdgpu_ucode_release(&adev->vcn.inst[0].fw);
+               adev->vcn.inst[i].fw = NULL;
+       }
        mutex_destroy(&adev->vcn.inst[i].vcn_pg_lock);
        mutex_destroy(&adev->vcn.inst[i].vcn1_jpeg1_workaround);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h

index a023f46ec904a..26c9c2d90f455 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.h
@@ -339,7 +339,6 @@ struct amdgpu_vcn_ras {
  };

struct amdgpu_vcn {

-       unsigned                fw_version;
        uint8_t num_vcn_inst;
        struct amdgpu_vcn_inst   inst[AMDGPU_MAX_VCN_INSTANCES];

@@ -356,6 +355,9 @@ struct amdgpu_vcn { uint32_t supported_reset;

        uint32_t                caps;
+
+       bool                    per_inst_fw;
+       unsigned                fw_version;
  };

struct amdgpu_fw_shared_rb_ptrs_struct {

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c 
b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
index 0d6a97c4bf8f6..347a2c40c32d0 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0_5.c
@@ -114,6 +114,9 @@ static int vcn_v4_0_5_early_init(struct amdgpu_ip_block 
*ip_block)
        struct amdgpu_device *adev = ip_block->adev;
        int i, r;

+ if (amdgpu_ip_version(adev, UVD_HWIP, 0) == IP_VERSION(4, 0, 6))

+               adev->vcn.per_inst_fw = true;
+
        for (i = 0; i < adev->vcn.num_vcn_inst; ++i)
                /* re-use enc ring as unified ring */
                adev->vcn.inst[i].num_enc_rings = 1;