[PATCH] drm/amdgpu: Fix uvd firmware version information for vega20

[Alex Deucher]

On Thu, Jun 14, 2018 at 12:36 PM, James Zhu <jamesz at amd.com> wrote:
> +Peter
>
>
>
> On 2018-06-14 11:51 AM, Leo Liu wrote:
>>
>>
>>
>> On 06/14/2018 11:29 AM, Alex Deucher wrote:
>>>
>>> The uvd version information was not set correctly for vega20.
>>> Rearrange the logic to set it correctly and fix the warnings
>>> as a result.
>>>
>>> Fixes: 7b3c773f405 (drm/amdgpu/vg20:support new UVD FW version naming
>>> convention)
>>> Signed-off-by: Alex Deucher <alexander.deucher at amd.com>
>>> ---
>>>
>>> I'm not sure if this is exactly how we want to expose the new firmware
>>> versioning
>>> to userspace, but it's better than random stack data.
>>
>> The encode team like to have the origin number which they put to FW, back
>> to user space for chip beyond RV and Vega20.
>>
>>>
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c | 43
>>> +++++++++++++++++++--------------
>>>   1 file changed, 25 insertions(+), 18 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>>> index 630e273c16ce..dde0c4383014 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_uvd.c
>>> @@ -127,7 +127,7 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev)
>>>       unsigned long bo_size;
>>>       const char *fw_name;
>>>       const struct common_firmware_header *hdr;
>>> -    unsigned version_major, version_minor, family_id;
>>> +    unsigned family_id;
>>>       int i, j, r;
>>>         INIT_DELAYED_WORK(&adev->uvd.inst->idle_work,
>>> amdgpu_uvd_idle_work_handler);
>>> @@ -210,10 +210,31 @@ int amdgpu_uvd_sw_init(struct amdgpu_device *adev)
>>>       family_id = le32_to_cpu(hdr->ucode_version) & 0xff;
>>>         if (adev->asic_type < CHIP_VEGA20) {
>>> +        unsigned version_major, version_minor;
>>> +
>>>           version_major = (le32_to_cpu(hdr->ucode_version) >> 24) & 0xff;
>>>           version_minor = (le32_to_cpu(hdr->ucode_version) >> 8) & 0xff;
>>>           DRM_INFO("Found UVD firmware Version: %hu.%hu Family ID:
>>> %hu\n",
>>>               version_major, version_minor, family_id);
>>> +
>>> +        /*
>>> +         * Limit the number of UVD handles depending on microcode major
>>> +         * and minor versions. The firmware version which has 40 UVD
>>> +         * instances support is 1.80. So all subsequent versions should
>>> +         * also have the same support.
>>> +         */
>>> +        if ((version_major > 0x01) ||
>>> +            ((version_major == 0x01) && (version_minor >= 0x50)))
>>> +            adev->uvd.max_handles = AMDGPU_MAX_UVD_HANDLES;
>>> +
>>> +        adev->uvd.fw_version = ((version_major << 24) | (version_minor
>>> << 16) |
>>> +                    (family_id << 8));
>>> +
>>> +        if ((adev->asic_type == CHIP_POLARIS10 ||
>>> +             adev->asic_type == CHIP_POLARIS11) &&
>>> +            (adev->uvd.fw_version < FW_1_66_16))
>>> +            DRM_ERROR("POLARIS10/11 UVD firmware version %hu.%hu is too
>>> old.\n",
>>> +                  version_major, version_minor);
>>>       } else {
>>>           unsigned int enc_major, enc_minor, dec_minor;
>>>   @@ -222,26 +243,12 @@ int amdgpu_uvd_sw_init(struct amdgpu_device
>>> *adev)
>>>           enc_major = (le32_to_cpu(hdr->ucode_version) >> 30) & 0x3;
>>>           DRM_INFO("Found UVD firmware ENC: %hu.%hu DEC: .%hu Family ID:
>>> %hu\n",
>>>               enc_major, enc_minor, dec_minor, family_id);
>>> -    }
>>>   -    /*
>>> -     * Limit the number of UVD handles depending on microcode major
>>> -     * and minor versions. The firmware version which has 40 UVD
>>> -     * instances support is 1.80. So all subsequent versions should
>>> -     * also have the same support.
>>> -     */
>>> -    if (adev->asic_type >= CHIP_VEGA20 || (version_major > 0x01) ||
>>> -        ((version_major == 0x01) && (version_minor >= 0x50)))
>>>           adev->uvd.max_handles = AMDGPU_MAX_UVD_HANDLES;
>>>   -    adev->uvd.fw_version = ((version_major << 24) | (version_minor <<
>>> 16) |
>>> -                (family_id << 8));
>>> -
>>> -    if ((adev->asic_type == CHIP_POLARIS10 ||
>>> -         adev->asic_type == CHIP_POLARIS11) &&
>>> -        (adev->uvd.fw_version < FW_1_66_16))
>>> -        DRM_ERROR("POLARIS10/11 UVD firmware version %hu.%hu is too
>>> old.\n",
>>> -              version_major, version_minor);
>>> +        adev->uvd.fw_version = ((enc_major << 24) | (enc_minor << 16) |
>>> +                    (dec_minor << 8) | (family_id << 0));
>>
>> It should be:
>>
>> adev->uvd.fw_version = hdr->ucode_version;
>
> Hi Peter, Does user space need the whole ucode_version or just fields with
> enc/dec/familyID? James

We also export this info via debugfs for debugging to query what
firmware versions are loaded on each engine on the GPU.

Alex

>>
>>
>> With that fixed the patch is:
>>
>> Reviewed-by: Leo Liu <leo.liu at amd.com>
>>
>>
>> Regards,
>> Leo
>>
>>
>>> +    }
>>>         bo_size = AMDGPU_UVD_STACK_SIZE + AMDGPU_UVD_HEAP_SIZE
>>>             +  AMDGPU_UVD_SESSION_SIZE * adev->uvd.max_handles;
>>
>>
>> _______________________________________________
>> amd-gfx mailing list
>> amd-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>
>