[PATCH v2 2/2] drm/amdgpu/vcn: Remove redundant indirect SRAM HW model check

[Limonciello, Mario]

[Public]



> -----Original Message-----
> From: Guilherme G. Piccoli <gpiccoli at igalia.com>
> Sent: Tuesday, January 17, 2023 11:59
> To: amd-gfx at lists.freedesktop.org
> Cc: dri-devel at lists.freedesktop.org; Deucher, Alexander
> <Alexander.Deucher at amd.com>; Koenig, Christian
> <Christian.Koenig at amd.com>; Pan, Xinhui <Xinhui.Pan at amd.com>;
> kernel at gpiccoli.net; kernel-dev at igalia.com; Guilherme G. Piccoli
> <gpiccoli at igalia.com>; Zhu, James <James.Zhu at amd.com>; Lazar, Lijo
> <Lijo.Lazar at amd.com>; Liu, Leo <Leo.Liu at amd.com>; Limonciello, Mario
> <Mario.Limonciello at amd.com>; Jiang, Sonny <Sonny.Jiang at amd.com>
> Subject: [PATCH v2 2/2] drm/amdgpu/vcn: Remove redundant indirect SRAM
> HW model check
> 
> The HW model validation that guards the indirect SRAM checking in the
> VCN code path is redundant - there's no model that's not included in the
> switch, making it useless in practice [0].
> 
> So, let's remove this switch statement for good.
> 
> [0] lore.kernel.org/amd-
> gfx/MN0PR12MB61013D20B8A2263B22AE1BCFE2C19 at MN0PR12MB6101.na
> mprd12.prod.outlook.com
> 
> Cc: James Zhu <James.Zhu at amd.com>
> Cc: Lazar Lijo <Lijo.Lazar at amd.com>
> Cc: Leo Liu <leo.liu at amd.com>
> Cc: Mario Limonciello <mario.limonciello at amd.com>
> Cc: Sonny Jiang <sonny.jiang at amd.com>
> Signed-off-by: Guilherme G. Piccoli <gpiccoli at igalia.com>

Should have added this tag too:
Suggested-by: Alexander Deucher <Alexander.Deucher at amd.com>

Looks good to me, thanks!
Reviewed-by: Mario Limonciello <mario.limonciello at amd.com>

> ---
> 
> 
> V2:
> * Changed the approach after ML discussion- instead of cleaning up
> the switch statement, removed it entirely - special thanks to Alex
> and Mario for the feedback!
> 
> Notice that patch 3 was dropped from this series after reviews.
> 
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 81 +------------------------
>  1 file changed, 3 insertions(+), 78 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> index 1b1a3c9e1863..02d428ddf2f8 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c
> @@ -110,84 +110,9 @@ int amdgpu_vcn_sw_init(struct amdgpu_device
> *adev)
>  	for (i = 0; i < adev->vcn.num_vcn_inst; i++)
>  		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(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, 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->vcn.indirect_sram = true;
> -		break;
> -	default:
> -		return -EINVAL;
> -	}
> +	if ((adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) &&
> +	    (adev->pg_flags & AMD_PG_SUPPORT_VCN_DPG))
> +		adev->vcn.indirect_sram = true;
> 
>  	hdr = (const struct common_firmware_header *)adev->vcn.fw-
> >data;
>  	adev->vcn.fw_version = le32_to_cpu(hdr->ucode_version);
> --
> 2.39.0