[AMD Official Use Only - General]
Hi Alex,
Please check my comments inline. Please also check v2 where I switch to an existing helper to check is display hardware is available.
Regards,
Hawking
Hawking
-----Original Message-----
From: Alex Deucher <alexdeucher@xxxxxxxxx>
Sent: Thursday, March 14, 2024 21:17
To: Zhang, Hawking <Hawking.Zhang@xxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Pillai, Aurabindo <Aurabindo.Pillai@xxxxxxx>; Feng, Kenneth <Kenneth.Feng@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: Bypass display ta if it is harvested
From: Alex Deucher <alexdeucher@xxxxxxxxx>
Sent: Thursday, March 14, 2024 21:17
To: Zhang, Hawking <Hawking.Zhang@xxxxxxx>
Cc: amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Pillai, Aurabindo <Aurabindo.Pillai@xxxxxxx>; Feng, Kenneth <Kenneth.Feng@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: Bypass display ta if it is harvested
On Thu, Mar 14, 2024 at 6:47 AM Hawking Zhang <Hawking.Zhang@xxxxxxx> wrote:
>
> Display TA doesn't need to be loaded/invoked if it is harvested.
>
> Signed-off-by: Hawking Zhang <Hawking.Zhang@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 867397fe2e9d..bb4988c45ca9 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -1830,6 +1830,10 @@ static int psp_hdcp_initialize(struct psp_context *psp)
> if (amdgpu_sriov_vf(psp->adev))
> return 0;
>
> + /* bypass hdcp initialization if dmu is harvested */
> + if (psp->adev->harvest_ip_mask & AMD_HARVEST_IP_DMU_MASK)
> + return 0;
> +
> if (!psp->hdcp_context.context.bin_desc.size_bytes ||
> !psp->hdcp_context.context.bin_desc.start_addr) {
> dev_info(psp->adev->dev, "HDCP: optional hdcp ta ucode
> is not available\n"); @@ -1862,6 +1866,9 @@ int psp_hdcp_invoke(struct psp_context *psp, uint32_t ta_cmd_id)
> if (amdgpu_sriov_vf(psp->adev))
> return 0;
>
> + if (!psp->hdcp_context.context.initialized)
> + return 0;
> +
> return psp_ta_invoke(psp, ta_cmd_id,
> &psp->hdcp_context.context); }
>
> @@ -1897,6 +1904,10 @@ static int psp_dtm_initialize(struct psp_context *psp)
> if (amdgpu_sriov_vf(psp->adev))
> return 0;
>
> + /* bypass dtm initialization if dmu is harvested */
> + if (psp->adev->harvest_ip_mask & AMD_HARVEST_IP_DMU_MASK)
> + return 0;
I think there may be some SKUs where the display blocks are not harvested, but they are not used so the atombios tables are empty.
This gets fixed up in dm_early_init() so the harvest flag should be set by the end of early_init. That should come before this code gets called so I think we should be fine.
[Hawking]: Yes. it comes before psp_hw_init. so we know display hardware is not available before loading psp firmware
> +
> if (!psp->dtm_context.context.bin_desc.size_bytes ||
> !psp->dtm_context.context.bin_desc.start_addr) {
> dev_info(psp->adev->dev, "DTM: optional dtm ta ucode
> is not available\n"); @@ -1929,6 +1940,9 @@ int psp_dtm_invoke(struct psp_context *psp, uint32_t ta_cmd_id)
> if (amdgpu_sriov_vf(psp->adev))
> return 0;
>
> + if (!psp->dtm_context.context.initialized)
> + return 0;
Doesn't the dtm_initialize function need a harvest check?
[Hawking]: yes, the same check is applied for dtm
+ /* bypass dtm initialization if dmu is harvested */
+ if (psp->adev->harvest_ip_mask & AMD_HARVEST_IP_DMU_MASK)
+ return 0;
+
> +
> return psp_ta_invoke(psp, ta_cmd_id,
> &psp->dtm_context.context); }
>
> @@ -2063,6 +2077,10 @@ static int psp_securedisplay_initialize(struct psp_context *psp)
> if (amdgpu_sriov_vf(psp->adev))
> return 0;
>
> + /* bypass securedisplay initialization if dmu is harvested */
> + if (psp->adev->harvest_ip_mask & AMD_HARVEST_IP_DMU_MASK)
> + return 0;
Don't we need to check if the context is initialized in psp_securedisplay_invoke()?
[Hawking]: The check is already in psp_securedisplay_invoke.
> +
> if (!psp->securedisplay_context.context.bin_desc.size_bytes ||
> !psp->securedisplay_context.context.bin_desc.start_addr) {
> dev_info(psp->adev->dev, "SECUREDISPLAY: securedisplay
> ta ucode is not available\n");
> --
> 2.17.1
>
