>-----Original Message-----
>From: Christian König <ckoenig.leichtzumerken at gmail.com>
>Sent: Thursday, August 16, 2018 9:17 PM
>To: Deng, Emily <Emily.Deng at amd.com>; amd-gfx at lists.freedesktop.org
>Subject: Re: [PATCH 1/2] drm/amdgpu: Remove the sriov checking and add
>firmware checking
>
>Am 16.08.2018 um 15:05 schrieb Emily Deng:
>> Unify bare metal and sriov, and add firmware checking for reg write
>> and reg wait unify command.
>>
>> Signed-off-by: Emily Deng <Emily.Deng at amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h | 2 ++
>> drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 47
>++++++++++++++++++++++++++++++++-
>> 2 files changed, 48 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>> index 53e9e2a..f172e92 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gfx.h
>> @@ -274,6 +274,8 @@ struct amdgpu_gfx {
>> uint32_t rlc_srls_feature_version;
>> uint32_t mec_feature_version;
>> uint32_t mec2_feature_version;
>> + bool mec_fw_write_wait;
>> + bool me_fw_write_wait;
>> struct amdgpu_ring gfx_ring[AMDGPU_MAX_GFX_RINGS];
>> unsigned num_gfx_rings;
>> struct amdgpu_ring
> compute_ring[AMDGPU_MAX_COMPUTE_RINGS];
>> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> index 76d979e..76e8973 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
>> @@ -482,6 +482,47 @@ static void gfx_v9_0_init_rlc_ext_microcode(struct
>amdgpu_device *adev)
>> le32_to_cpu(rlc_hdr-
>>reg_list_format_direct_reg_list_length);
>> }
>>
>> +static void gfx_v9_0_check_fw_write_wait(struct amdgpu_device *adev)
>> +{
>> + adev->gfx.me_fw_write_wait = false;
>> + adev->gfx.mec_fw_write_wait = false;
>> +
>> + switch (adev->asic_type) {
>> + case CHIP_VEGA10:
>> + if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev-
>>gfx.me_feature_version >= 42) &&
>> + (adev->gfx.pfp_fw_version >= 0x000000b1) && (adev-
>>gfx.pfp_feature_version >= 42))
>> + adev->gfx.me_fw_write_wait = true;
>
>Well that looks like the correct solution to me. Only a nit pick left to fix, the
>coding style looks a bit off.
>
>For example the "if" should be indented so that "(adev.." in both lines is on the
>same column:
>
>if ((adev.... ) &&
> Â Â Â (adev....
>
>And the "adev->gfx.me_fw_write_wait = true;" is to far to the right.
>
Thanks, will modify as your suggestion.
>With that fixed the patch is Acked-by: Christian König
><christian.koenig at amd.com>.
>
>Regards,
>Christian.
>
>> +
>> + if ((adev->gfx.mec_fw_version >= 0x00000193) && (adev-
>>gfx.mec_feature_version >= 42))
>> + adev->gfx.mec_fw_write_wait = true;
>> + break;
>> + case CHIP_VEGA12:
>> + if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev-
>>gfx.me_feature_version >= 44) &&
>> + (adev->gfx.pfp_fw_version >= 0x000000b2) && (adev-
>>gfx.pfp_feature_version >= 44))
>> + adev->gfx.me_fw_write_wait = true;
>> +
>> + if ((adev->gfx.mec_fw_version >= 0x00000196) && (adev-
>>gfx.mec_feature_version >= 44))
>> + adev->gfx.mec_fw_write_wait = true;
>> + break;
>> + case CHIP_VEGA20:
>> + if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev-
>>gfx.me_feature_version >= 44) &&
>> + (adev->gfx.pfp_fw_version >= 0x000000b2) && (adev-
>>gfx.pfp_feature_version >= 44))
>> + adev->gfx.me_fw_write_wait = true;
>> +
>> + if ((adev->gfx.mec_fw_version >= 0x00000197) && (adev-
>>gfx.mec_feature_version >= 44))
>> + adev->gfx.mec_fw_write_wait = true;
>> + break;
>> + case CHIP_RAVEN:
>> + if ((adev->gfx.me_fw_version >= 0x0000009c) && (adev-
>>gfx.me_feature_version >= 42) &&
>> + (adev->gfx.pfp_fw_version >= 0x000000b1) && (adev-
>>gfx.pfp_feature_version >= 42))
>> + adev->gfx.me_fw_write_wait = true;
>> +
>> + if ((adev->gfx.mec_fw_version >= 0x00000192) && (adev-
>>gfx.mec_feature_version >= 42))
>> + adev->gfx.mec_fw_write_wait = true;
>> + break;
>> + }
>> +}
>> +
>> static int gfx_v9_0_init_microcode(struct amdgpu_device *adev)
>> {
>> const char *chip_name;
>> @@ -716,6 +757,7 @@ static int gfx_v9_0_init_microcode(struct
>amdgpu_device *adev)
>> }
>>
>> out:
>> + gfx_v9_0_check_fw_write_wait(adev);
>> if (err) {
>> dev_err(adev->dev,
>> "gfx9: Failed to load firmware \"%s\"\n", @@ -4348,8
>+4390,11 @@
>> static void gfx_v9_0_ring_emit_reg_write_reg_wait(struct amdgpu_ring *ring,
>> uint32_t ref, uint32_t mask)
>> {
>> int usepfp = (ring->funcs->type == AMDGPU_RING_TYPE_GFX);
>> + struct amdgpu_device *adev = ring->adev;
>> + bool fw_version_ok = (ring->funcs->type == AMDGPU_RING_TYPE_GFX) ?
>> + adev->gfx.me_fw_write_wait : adev->gfx.mec_fw_write_wait;
>>
>> - if (amdgpu_sriov_vf(ring->adev))
>> + if (fw_version_ok)
>> gfx_v9_0_wait_reg_mem(ring, usepfp, 0, 1, reg0, reg1,
>> ref, mask, 0x20);
>> else
