RE: [PATCH] drm/amdgpu: Bypass display ta if it is harvested

[Zhang, Hawking]

[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

-Original Message-
From: Alex Deucher 
Sent: Thursday, March 14, 2024 21:17
To: Zhang, Hawking 
Cc: amd-gfx@lists.freedesktop.org; Pillai, Aurabindo 
; Feng, Kenneth 
Subject: Re: [PATCH] drm/amdgpu: Bypass display ta if it is harvested

On Thu, Mar 14, 2024 at 6:47 AM Hawking Zhang 
mailto:hawking.zh...@amd.com>> wrote:
>
> Display TA doesn't need to be loaded/invoked if it is harvested.
>
> Signed-off-by: Hawking Zhang 
> mailto:hawking.zh...@amd.com>>
> ---
>  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,
> >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,
> >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
>

Re: [PATCH] drm/amdgpu: Bypass display ta if it is harvested

[Alex Deucher]

On Thu, Mar 14, 2024 at 6:47 AM Hawking Zhang  wrote:
>
> Display TA doesn't need to be loaded/invoked if it
> is harvested.
>
> Signed-off-by: Hawking Zhang 
> ---
>  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, >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.

> +
> 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?

> +
> return psp_ta_invoke(psp, ta_cmd_id, >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()?

> +
> 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
>

RE: [PATCH] drm/amdgpu: Bypass display ta if it is harvested

[Zhang, Hawking]

[AMD Official Use Only - General]

Never mind. There is helper function to check if display hardware is available. 
I will move to the helper in v2. Thanks @Wang, 
Yang(Kevin)<mailto:kevinyang.w...@amd.com> for his reminder.

Regards,
Hawking

From: amd-gfx  On Behalf Of Zhang, 
Hawking
Sent: Thursday, March 14, 2024 18:45
To: amd-gfx@lists.freedesktop.org; Pillai, Aurabindo 
; Feng, Kenneth ; Deucher, 
Alexander 
Subject: RE: [PATCH] drm/amdgpu: Bypass display ta if it is harvested


[AMD Official Use Only - General]

[AMD Official Use Only - General]

Copy @Deucher, Alexander<mailto:alexander.deuc...@amd.com> for the awareness.

Regards,
Hawking

-Original Message-
From: Zhang, Hawking mailto:hawking.zh...@amd.com>>
Sent: Thursday, March 14, 2024 18:36
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>; 
Pillai, Aurabindo mailto:aurabindo.pil...@amd.com>>; 
Feng, Kenneth mailto:kenneth.f...@amd.com>>
Cc: Zhang, Hawking mailto:hawking.zh...@amd.com>>
Subject: [PATCH] drm/amdgpu: Bypass display ta if it is harvested

Display TA doesn't need to be loaded/invoked if it is harvested.

Signed-off-by: Hawking Zhang 
mailto:hawking.zh...@amd.com>>
---
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, >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;
+
 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;
+
 return psp_ta_invoke(psp, ta_cmd_id, >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;
+
 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

RE: [PATCH] drm/amdgpu: Bypass display ta if it is harvested

[Zhang, Hawking]

[AMD Official Use Only - General]


Copy @Deucher, Alexander for the awareness.

Regards,
Hawking

-Original Message-
From: Zhang, Hawking 
Sent: Thursday, March 14, 2024 18:36
To: amd-gfx@lists.freedesktop.org; Pillai, Aurabindo 
; Feng, Kenneth 
Cc: Zhang, Hawking 
Subject: [PATCH] drm/amdgpu: Bypass display ta if it is harvested

Display TA doesn't need to be loaded/invoked if it is harvested.

Signed-off-by: Hawking Zhang 
mailto:hawking.zh...@amd.com>>
---
 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, >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;
+
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;
+
return psp_ta_invoke(psp, ta_cmd_id, >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;
+
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