On Mon, Jan 16, 2023 at 4:20 PM Guilherme G. Piccoli
<gpiccoli@xxxxxxxxxx> wrote:
>
> Currently both conditionals checked to enable indirect SRAM are
> repeated for all HW/IP models. Let's just simplify it a bit so
> code is more compact and readable.
>
> While at it, add the legacy names as a comment per case block, to
> help whoever is reading the code and doesn't have much experience
> with the name/number mapping.
>
> Cc: James Zhu <James.Zhu@xxxxxxx>
> Cc: Lazar Lijo <Lijo.Lazar@xxxxxxx>
> Cc: Leo Liu <leo.liu@xxxxxxx>
> Cc: Mario Limonciello <mario.limonciello@xxxxxxx>
> Cc: Sonny Jiang <sonny.jiang@xxxxxxx>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli@xxxxxxxxxx>
> ---
>
>
> Hi folks, first of all thanks in advance for reviews/comments!
>
> This work is based on agd5f/amd-staging-drm-next branch - there is this
> patch from Mario that refactored the amdgpu_vcn.c, and since it dropped
> the legacy names from the switch cases, I've decided to also include them
> here as comments.
>
> I'm not sure if that's a good idea, feels good for somebody not so
> used to the code read the codenames instead of purely numbers, but
> if you wanna move away from the legacy names for good, lemme know
> and I can rework without these comments.
>
> Cheers,
>
>
> Guilherme
>
>
> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 84 +++++--------------------
> 1 file changed, 16 insertions(+), 68 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 1b1a3c9e1863..1f880e162d9d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -111,78 +111,26 @@ int amdgpu_vcn_sw_init(struct amdgpu_device *adev)
> atomic_set(&adev->vcn.inst[i].dpg_enc_submission_cnt, 0);
>
> switch (adev->ip_versions[UVD_HWIP][0]) {
> - case IP_VERSION(1, 0, 0):
> - case IP_VERSION(1, 0, 1):
> - case IP_VERSION(2, 5, 0):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(2, 2, 0):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(2, 6, 0):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(2, 0, 0):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(2, 0, 2):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(3, 0, 0):
> - case IP_VERSION(3, 0, 64):
> - case IP_VERSION(3, 0, 192):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(3, 0, 2):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(3, 0, 16):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(3, 0, 33):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(3, 1, 1):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> + case IP_VERSION(1, 0, 0): /* Raven (1/2) / Picasso */
> + case IP_VERSION(1, 0, 1): /* Raven (1/2) / Picasso */
> + case IP_VERSION(2, 0, 0): /* Navi10 */
> + case IP_VERSION(2, 0, 2): /* Navi12 / Navi14 */
> + case IP_VERSION(2, 2, 0): /* Renoir / Green Sardine */
> + case IP_VERSION(2, 5, 0): /* Arcturus */
> + case IP_VERSION(2, 6, 0): /* Aldebaran */
> + case IP_VERSION(3, 0, 0): /* Sienna Cichlid / Navy Flounder */
> + case IP_VERSION(3, 0, 2): /* Vangogh */
> + case IP_VERSION(3, 0, 64): /* Sienna Cichlid / Navy Flounder */
> + case IP_VERSION(3, 0, 16): /* Dimgray Cavefish */
> + case IP_VERSION(3, 0, 33): /* Beige Goby */
> + case IP_VERSION(3, 0, 192): /* Sienna Cichlid / Navy Flounder */
> + case IP_VERSION(3, 1, 1): /* Yellow Carp */
> case IP_VERSION(3, 1, 2):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> - case IP_VERSION(4, 0, 0):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> + case IP_VERSION(4, 0, 0): /* Vega10 */
This comment is incorrect. Vega10 does not have VCN (it has UVD and VCE).
Alex
Alex
> case IP_VERSION(4, 0, 2):
> - if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> - adev->vcn.indirect_sram = true;
> - break;
> case IP_VERSION(4, 0, 4):
> if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> - (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> + (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> adev->vcn.indirect_sram = true;
> break;
> default:
> --
> 2.39.0
>
