[PATCH] drm/amdgpu: Add secure display v2 command

[Yu, Lang]

[AMD Official Use Only - AMD Internal Distribution Only]

>-----Original Message-----
>From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of Jinzhou Su
>Sent: Friday, November 29, 2024 10:30 AM
>To: amd-gfx at lists.freedesktop.org
>Cc: Zhang, Hawking <Hawking.Zhang at amd.com>; Lin, Wayne
><Wayne.Lin at amd.com>; Strauss, Andrew <Andrew.Strauss at amd.com>; Yu, Lang
><Lang.Yu at amd.com>; Su, Joe <Jinzhou.Su at amd.com>; Lin, Wayne
><Wayne.Lin at amd.com>
>Subject: [PATCH] drm/amdgpu: Add secure display v2 command
>
>Add secure display v2 command to support multiple ROI instances per display.
>
>v2: fix typo and coding style issue
>
>Signed-off-by: Wayne Lin <Wayne.Lin at amd.com>
>Signed-off-by: Jinzhou Su <jinzhou.su at amd.com>
>---
> drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c       |  3 ++-
> .../gpu/drm/amd/amdgpu/ta_secureDisplay_if.h  | 25 +++++++++++++++++--
> 2 files changed, 25 insertions(+), 3 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>index f015961f257a..12832fd834fb 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>@@ -2264,7 +2264,8 @@ int psp_securedisplay_invoke(struct psp_context *psp,
>uint32_t ta_cmd_id)
>               return -EINVAL;
>
>       if (ta_cmd_id != TA_SECUREDISPLAY_COMMAND__QUERY_TA &&
>-          ta_cmd_id != TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC)
>+          ta_cmd_id != TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC &&
>+          ta_cmd_id != TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC_V2)
>               return -EINVAL;
>
>       ret = psp_ta_invoke(psp, ta_cmd_id, &psp->securedisplay_context.context);
>diff --git a/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
>b/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
>index 00d8bdb8254f..926da6d46322 100644
>--- a/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
>+++ b/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
>@@ -31,10 +31,12 @@
>  *    Secure Display Command ID
>  */
> enum ta_securedisplay_command {
>-      /* Query whether TA is responding used only for validation purpose */
>+      /* Query whether TA is responding. It is used only for validation
>+purpose */
>       TA_SECUREDISPLAY_COMMAND__QUERY_TA              = 1,
>       /* Send region of Interest and CRC value to I2C */
>       TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC          = 2,
>+      /* V2 to send multiple regions of Interest and CRC value to I2C */
>+      TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC_V2       = 3,
>       /* Maximum Command ID */
>       TA_SECUREDISPLAY_COMMAND__MAX_ID                = 0x7FFFFFFF,
> };
>@@ -83,6 +85,8 @@ enum ta_securedisplay_ta_query_cmd_ret {  enum
>ta_securedisplay_buffer_size {
>       /* 15 bytes = 8 byte (ROI) + 6 byte(CRC) + 1 byte(phy_id) */
>       TA_SECUREDISPLAY_I2C_BUFFER_SIZE                = 15,
>+      /* 16 bytes = 8 byte (ROI) + 6 byte(CRC) + 1 byte(phy_id) + 1 byte(roi_idx) */
>+      TA_SECUREDISPLAY_V2_I2C_BUFFER_SIZE             = 16,
> };
>
> /** Input/output structures for Secure Display commands */ @@ -95,7 +99,15 @@
>enum ta_securedisplay_buffer_size {
>  *    Physical ID to determine which DIO scratch register should be used to get ROI
>  */
> struct ta_securedisplay_send_roi_crc_input {
>-      uint32_t  phy_id;  /* Physical ID */
>+      /* Physical ID */
>+      uint32_t  phy_id;
>+};
>+
>+struct ta_securedisplay_send_roi_crc_v2_input {
>+      /* Physical ID */
>+      uint32_t phy_id;
>+      /* Region of interest index */
>+      uint8_t  roi_idx;
> };
>
> /** @union ta_securedisplay_cmd_input
>@@ -104,6 +116,9 @@ struct ta_securedisplay_send_roi_crc_input {  union
>ta_securedisplay_cmd_input {
>       /* send ROI and CRC input buffer format */
>       struct ta_securedisplay_send_roi_crc_input        send_roi_crc;
>+      /* send ROI and CRC input buffer format, v2 adds a ROI index */
>+      struct ta_securedisplay_send_roi_crc_v2_input     send_roi_crc_v2;
>+      /* Reserved space for future expansion */

The comment "/* Reserved space for future expansion */" is misleading here.

Get this fixed, the patch is

Reviewed-by: Lang Yu <lang.yu at amd.com>


>       uint32_t                                          reserved[4];
> };
>
>@@ -128,6 +143,10 @@ struct ta_securedisplay_send_roi_crc_output {
>       uint8_t  reserved;
> };
>
>+struct ta_securedisplay_send_roi_crc_v2_output {
>+      uint8_t  i2c_buf[TA_SECUREDISPLAY_V2_I2C_BUFFER_SIZE];  /* I2C buffer
>+*/ };
>+
> /** @union ta_securedisplay_cmd_output
>  *    Output buffer
>  */
>@@ -136,6 +155,8 @@ union ta_securedisplay_cmd_output {
>       struct ta_securedisplay_query_ta_output            query_ta;
>       /* Send ROI CRC output buffer format used only for validation purpose */
>       struct ta_securedisplay_send_roi_crc_output        send_roi_crc;
>+      /* Send ROI CRC output buffer format used only for validation purpose */
>+      struct ta_securedisplay_send_roi_crc_v2_output     send_roi_crc_v2;
>       uint32_t                                           reserved[4];
> };
>
>--
>2.43.0