<html> <head> <meta http-equiv="Content-Type" content="text/html; charset=utf-8"> <meta name="Generator" content="Microsoft Exchange Server">  <style></style> </head> <body> <font face="Arial" size="2"><span style="font-size:10pt;"> <div style="padding-right:5pt;padding-left:5pt;"><font color="blue">[AMD Official Use Only - General]<br> </font></div> <div style="margin-top:5pt;"><font face="Times New Roman" size="3"><span style="font-size:12pt;"><br> </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">Hi Alex,</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">Please check my comments inline. Please also check v2 where I switch to an existing helper to check is display hardware is available.</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">Regards,<br> Hawking</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">-----Original Message-----<br> From: Alex Deucher <alexdeucher@gmail.com> <br> Sent: Thursday, March 14, 2024 21:17<br> To: Zhang, Hawking <Hawking.Zhang@amd.com><br> Cc: amd-gfx@lists.freedesktop.org; Pillai, Aurabindo <Aurabindo.Pillai@amd.com>; Feng, Kenneth <Kenneth.Feng@amd.com><br> Subject: Re: [PATCH] drm/amdgpu: Bypass display ta if it is harvested</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">On Thu, Mar 14, 2024 at 6:47 AM Hawking Zhang <<a href="mailto:Hawking.Zhang@amd.com">Hawking.Zhang@amd.com</a>> wrote:</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">></span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> Display TA doesn't need to be loaded/invoked if it is harvested.</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">></span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> Signed-off-by: Hawking Zhang <<a href="mailto:Hawking.Zhang@amd.com">Hawking.Zhang@amd.com</a>></span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> ---</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 18 ++++++++++++++++++</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> 1 file changed, 18 insertions(+)</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">></span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> index 867397fe2e9d..bb4988c45ca9 100644</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> @@ -1830,6 +1830,10 @@ static int psp_hdcp_initialize(struct psp_context *psp)</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> if (amdgpu_sriov_vf(psp->adev))</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> return 0;</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">></span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> + /* bypass hdcp initialization if dmu is harvested */</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> + if (psp->adev->harvest_ip_mask & AMD_HARVEST_IP_DMU_MASK)</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> + return 0;</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> +</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> if (!psp->hdcp_context.context.bin_desc.size_bytes ||</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> !psp->hdcp_context.context.bin_desc.start_addr) {</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> dev_info(psp->adev->dev, "HDCP: optional hdcp ta ucode </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> is not available\n"); @@ -1862,6 +1866,9 @@ int psp_hdcp_invoke(struct psp_context *psp, uint32_t ta_cmd_id)</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> if (amdgpu_sriov_vf(psp->adev))</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> return 0;</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">></span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> + if (!psp->hdcp_context.context.initialized)</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> + return 0;</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> +</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> return psp_ta_invoke(psp, ta_cmd_id, </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> &psp->hdcp_context.context); }</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">></span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> @@ -1897,6 +1904,10 @@ static int psp_dtm_initialize(struct psp_context *psp)</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> if (amdgpu_sriov_vf(psp->adev))</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> return 0;</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">></span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> + /* bypass dtm initialization if dmu is harvested */</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> + if (psp->adev->harvest_ip_mask & AMD_HARVEST_IP_DMU_MASK)</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> + return 0;</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">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.</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">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.</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div> <div><font face="Calibri" size="2" color="#2F5496"><span style="font-size:11pt;">[Hawking]: Yes. it comes before psp_hw_init. so we know display hardware is not available before loading psp firmware</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> +</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> if (!psp->dtm_context.context.bin_desc.size_bytes ||</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> !psp->dtm_context.context.bin_desc.start_addr) {</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> dev_info(psp->adev->dev, "DTM: optional dtm ta ucode </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> is not available\n"); @@ -1929,6 +1940,9 @@ int psp_dtm_invoke(struct psp_context *psp, uint32_t ta_cmd_id)</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> if (amdgpu_sriov_vf(psp->adev))</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> return 0;</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">></span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> + if (!psp->dtm_context.context.initialized)</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> + return 0;</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">Doesn't the dtm_initialize function need a harvest check?</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div> <div><font face="Calibri" size="2" color="#2F5496"><span style="font-size:11pt;">[Hawking]: yes, the same check is applied for dtm</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">+ /* bypass dtm initialization if dmu is harvested */</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">+ if (psp->adev->harvest_ip_mask & AMD_HARVEST_IP_DMU_MASK)</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">+ return 0;</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">+</span></font></div> <div><font face="Calibri" size="2" color="#2F5496"><span style="font-size:11pt;"> </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> +</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> return psp_ta_invoke(psp, ta_cmd_id, </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> &psp->dtm_context.context); }</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">></span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> @@ -2063,6 +2077,10 @@ static int psp_securedisplay_initialize(struct psp_context *psp)</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> if (amdgpu_sriov_vf(psp->adev))</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> return 0;</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">></span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> + /* bypass securedisplay initialization if dmu is harvested */</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> + if (psp->adev->harvest_ip_mask & AMD_HARVEST_IP_DMU_MASK)</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> + return 0;</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">Don't we need to check if the context is initialized in psp_securedisplay_invoke()?</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div> <div><font face="Calibri" size="2" color="#2F5496"><span style="font-size:11pt;">[Hawking]: The check is already in psp_securedisplay_invoke.</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;"> </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> +</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> if (!psp->securedisplay_context.context.bin_desc.size_bytes ||</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> !psp->securedisplay_context.context.bin_desc.start_addr) {</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> dev_info(psp->adev->dev, "SECUREDISPLAY: securedisplay </span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> ta ucode is not available\n");</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> --</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">> 2.17.1</span></font></div> <div><font face="Calibri" size="2"><span style="font-size:11pt;">></span></font></div> </span></font> </body> </html>