All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file
@ 2021-01-13  3:43 Jinzhou Su
  2021-01-13  3:43 ` [PATCH 2/2] drm/amdgpu: Add secure display TA interface Jinzhou Su
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Jinzhou Su @ 2021-01-13  3:43 UTC (permalink / raw)
  To: amd-gfx; +Cc: Jinzhou Su, ray.huang

Add file ta_secureDisplay_if.h for Secure Display TA

Signed-off-by: Jinzhou Su <Jinzhou.Su@amd.com>
Reviewed-by: Huang Rui <ray.huang@amd.com>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
---
 .../gpu/drm/amd/amdgpu/ta_secureDisplay_if.h  | 154 ++++++++++++++++++
 1 file changed, 154 insertions(+)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h

diff --git a/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h b/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
new file mode 100644
index 000000000000..5039375bb1d4
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
@@ -0,0 +1,154 @@
+/*
+ * Copyright 2019 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ */
+
+#ifndef _TA_SECUREDISPLAY_IF_H
+#define _TA_SECUREDISPLAY_IF_H
+
+/** Secure Display related enumerations */
+/**********************************************************/
+
+/** @enum ta_securedisplay_command
+ *    Secure Display Command ID
+ */
+enum ta_securedisplay_command {
+	/* Query whether TA is responding 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,
+	/* Maximum Command ID */
+	TA_SECUREDISPLAY_COMMAND__MAX_ID                = 0x7FFFFFFF,
+};
+
+/** @enum ta_securedisplay_status
+ *    Secure Display status returns in shared buffer status
+ */
+enum ta_securedisplay_status {
+	TA_SECUREDISPLAY_STATUS__SUCCESS                 = 0x00,         /* Success */
+	TA_SECUREDISPLAY_STATUS__GENERIC_FAILURE         = 0x01,         /* Generic Failure */
+	TA_SECUREDISPLAY_STATUS__INVALID_PARAMETER       = 0x02,         /* Invalid Parameter */
+	TA_SECUREDISPLAY_STATUS__NULL_POINTER            = 0x03,         /* Null Pointer*/
+	TA_SECUREDISPLAY_STATUS__I2C_WRITE_ERROR         = 0x04,         /* Fail to Write to I2C */
+	TA_SECUREDISPLAY_STATUS__READ_DIO_SCRATCH_ERROR  = 0x05, /*Fail Read DIO Scratch Register*/
+	TA_SECUREDISPLAY_STATUS__READ_CRC_ERROR          = 0x06,         /* Fail to Read CRC*/
+
+	TA_SECUREDISPLAY_STATUS__MAX                     = 0x7FFFFFFF,/* Maximum Value for status*/
+};
+
+/** @enum ta_securedisplay_max_phy
+ *    Physical ID number to use for reading corresponding DIO Scratch register for ROI
+ */
+enum  ta_securedisplay_max_phy {
+	TA_SECUREDISPLAY_PHY0                           = 0,
+	TA_SECUREDISPLAY_PHY1                           = 1,
+	TA_SECUREDISPLAY_PHY2                           = 2,
+	TA_SECUREDISPLAY_PHY3                           = 3,
+	TA_SECUREDISPLAY_MAX_PHY                        = 4,
+};
+
+/** @enum ta_securedisplay_ta_query_cmd_ret
+ *    A predefined specific reteurn value which is 0xAB only used to validate
+ *    communication to Secure Display TA is functional.
+ *    This value is used to validate whether TA is responding successfully
+ */
+enum ta_securedisplay_ta_query_cmd_ret {
+	/* This is a value to validate if TA is loaded successfully */
+	TA_SECUREDISPLAY_QUERY_CMD_RET                 = 0xAB,
+};
+
+/** @enum ta_securedisplay_buffer_size
+ *    I2C Buffer size which contains 8 bytes of ROI  (X start, X end, Y start, Y end)
+ *    and 6 bytes of CRC( R,G,B) and 1  byte for physical ID
+ */
+enum ta_securedisplay_buffer_size {
+	/* 15 bytes = 8 byte (ROI) + 6 byte(CRC) + 1 byte(phy_id) */
+	TA_SECUREDISPLAY_I2C_BUFFER_SIZE                = 15,
+};
+
+/** Input/output structures for Secure Display commands */
+/**********************************************************/
+/**
+ * Input structures
+ */
+
+/** @struct ta_securedisplay_send_roi_crc_input
+ *    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 */
+};
+
+/** @union ta_securedisplay_cmd_input
+ *    Input buffer
+ */
+union ta_securedisplay_cmd_input {
+	/* send ROI and CRC input buffer format */
+	struct ta_securedisplay_send_roi_crc_input        send_roi_crc;
+	uint32_t                                          reserved[4];
+};
+
+/**
+ * Output structures
+ */
+
+/** @struct ta_securedisplay_query_ta_output
+ *  Output buffer format for query TA whether TA is responding used only for validation purpose
+ */
+struct ta_securedisplay_query_ta_output {
+	/* return value from TA when it is queried for validation purpose only */
+	uint32_t  query_cmd_ret;
+};
+
+/** @struct ta_securedisplay_send_roi_crc_output
+ *  Output buffer format for send ROI CRC command which will pass I2c buffer created inside TA
+ *  and used to write to I2C used only for validation purpose
+ */
+struct ta_securedisplay_send_roi_crc_output {
+	uint8_t  i2c_buf[TA_SECUREDISPLAY_I2C_BUFFER_SIZE];  /* I2C buffer */
+	uint8_t  reserved;
+};
+
+/** @union ta_securedisplay_cmd_output
+ *    Output buffer
+ */
+union ta_securedisplay_cmd_output {
+	/* Query TA output buffer format used only for validation purpose*/
+	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;
+	uint32_t                                           reserved[4];
+};
+
+/** @struct securedisplay_cmd
+ *    Secure Display Command which is shared buffer memory
+ */
+struct securedisplay_cmd {
+	uint32_t                             cmd_id;                    /* +0  Bytes Command ID */
+	enum ta_securedisplay_status         status;     /* +4  Bytes Status of Secure Display TA */
+	uint32_t                             reserved[2];               /* +8  Bytes Reserved */
+	union ta_securedisplay_cmd_input     securedisplay_in_message;  /* +16 Bytes Input Buffer */
+	union ta_securedisplay_cmd_output    securedisplay_out_message;/* +32 Bytes Output Buffer */
+	/**@note Total 48 Bytes */
+};
+
+#endif   //_TA_SECUREDISPLAY_IF_H
+
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/2] drm/amdgpu: Add secure display TA interface
  2021-01-13  3:43 [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file Jinzhou Su
@ 2021-01-13  3:43 ` Jinzhou Su
  2021-01-13  5:45   ` Chen, Guchun
                     ` (2 more replies)
  2021-01-13  3:51 ` [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file Huang Rui
  2021-01-13 19:05 ` Paul Menzel
  2 siblings, 3 replies; 10+ messages in thread
From: Jinzhou Su @ 2021-01-13  3:43 UTC (permalink / raw)
  To: amd-gfx; +Cc: Jinzhou Su, ray.huang

Add interface to load, unload, invoke command for
secure display TA.

v2: Add debugfs interface for secure display TA

Signed-off-by: Jinzhou.Su <Jinzhou.Su@amd.com>
Reviewed-by: Huang Rui <ray.huang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/Makefile           |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c   |   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c       | 195 ++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h       |  17 ++
 .../gpu/drm/amd/amdgpu/amdgpu_securedisplay.c | 174 ++++++++++++++++
 .../gpu/drm/amd/amdgpu/amdgpu_securedisplay.h |  36 ++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h     |   4 +
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c        |  12 +-
 8 files changed, 440 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index c6262689e14e..e4bebbfa88af 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -56,7 +56,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
 	amdgpu_gmc.o amdgpu_mmhub.o amdgpu_xgmi.o amdgpu_csa.o amdgpu_ras.o amdgpu_vm_cpu.o \
 	amdgpu_vm_sdma.o amdgpu_discovery.o amdgpu_ras_eeprom.o amdgpu_nbio.o \
 	amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
-	amdgpu_fw_attestation.o
+	amdgpu_fw_attestation.o amdgpu_securedisplay.o
 
 amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 477bead4fab1..4c38c5771cbc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -35,6 +35,7 @@
 #include "amdgpu_dm_debugfs.h"
 #include "amdgpu_ras.h"
 #include "amdgpu_rap.h"
+#include "amdgpu_securedisplay.h"
 #include "amdgpu_fw_attestation.h"
 
 /**
@@ -1666,6 +1667,8 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev)
 
 	amdgpu_rap_debugfs_init(adev);
 
+	amdgpu_securedisplay_debugfs_init(adev);
+
 	amdgpu_fw_attestation_debugfs_init(adev);
 
 	return amdgpu_debugfs_add_files(adev, amdgpu_debugfs_list,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 523d22db094b..eb19ae734396 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -36,6 +36,7 @@
 #include "psp_v12_0.h"
 
 #include "amdgpu_ras.h"
+#include "amdgpu_securedisplay.h"
 
 static int psp_sysfs_init(struct amdgpu_device *adev);
 static void psp_sysfs_fini(struct amdgpu_device *adev);
@@ -1642,6 +1643,179 @@ int psp_rap_invoke(struct psp_context *psp, uint32_t ta_cmd_id)
 }
 // RAP end
 
+/* securedisplay start */
+static int psp_securedisplay_init_shared_buf(struct psp_context *psp)
+{
+	int ret;
+
+	/*
+	 * Allocate 16k memory aligned to 4k from Frame Buffer (local
+	 * physical) for sa ta <-> Driver
+	 */
+	ret = amdgpu_bo_create_kernel(psp->adev, PSP_SECUREDISPLAY_SHARED_MEM_SIZE,
+				      PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM,
+				      &psp->securedisplay_context.securedisplay_shared_bo,
+				      &psp->securedisplay_context.securedisplay_shared_mc_addr,
+				      &psp->securedisplay_context.securedisplay_shared_buf);
+
+	return ret;
+}
+
+static int psp_securedisplay_load(struct psp_context *psp)
+{
+	int ret;
+	struct psp_gfx_cmd_resp *cmd;
+
+	/*
+	 * TODO: bypass the loading in sriov for now
+	 */
+
+	cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
+	if (!cmd)
+		return -ENOMEM;
+
+	memset(psp->fw_pri_buf, 0, PSP_1_MEG);
+	memcpy(psp->fw_pri_buf, psp->ta_securedisplay_start_addr, psp->ta_securedisplay_ucode_size);
+
+	psp_prep_ta_load_cmd_buf(cmd,
+				 psp->fw_pri_mc_addr,
+				 psp->ta_securedisplay_ucode_size,
+				 psp->securedisplay_context.securedisplay_shared_mc_addr,
+				 PSP_SECUREDISPLAY_SHARED_MEM_SIZE);
+
+	ret = psp_cmd_submit_buf(psp, NULL, cmd, psp->fence_buf_mc_addr);
+
+	if (ret)
+		goto failed;
+
+	psp->securedisplay_context.securedisplay_initialized = true;
+	psp->securedisplay_context.session_id = cmd->resp.session_id;
+	mutex_init(&psp->securedisplay_context.mutex);
+
+failed:
+	kfree(cmd);
+	return ret;
+}
+
+static int psp_securedisplay_unload(struct psp_context *psp)
+{
+	int ret;
+	struct psp_gfx_cmd_resp *cmd;
+
+	cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
+	if (!cmd)
+		return -ENOMEM;
+
+	psp_prep_ta_unload_cmd_buf(cmd, psp->securedisplay_context.session_id);
+
+	ret = psp_cmd_submit_buf(psp, NULL, cmd, psp->fence_buf_mc_addr);
+
+	kfree(cmd);
+
+	return ret;
+}
+
+static int psp_securedisplay_initialize(struct psp_context *psp)
+{
+	int ret;
+	struct securedisplay_cmd *securedisplay_cmd;
+
+	/*
+	 * TODO: bypass the initialize in sriov for now
+	 */
+	if (amdgpu_sriov_vf(psp->adev))
+		return 0;
+
+	if (!psp->adev->psp.ta_securedisplay_ucode_size ||
+	    !psp->adev->psp.ta_securedisplay_start_addr) {
+		dev_info(psp->adev->dev, "SECUREDISPLAY: securedisplay ta ucode is not available\n");
+		return 0;
+	}
+
+	if (!psp->securedisplay_context.securedisplay_initialized) {
+		ret = psp_securedisplay_init_shared_buf(psp);
+		if (ret)
+			return ret;
+	}
+
+	ret = psp_securedisplay_load(psp);
+	if (ret)
+		return ret;
+
+	psp_prep_securedisplay_cmd_buf(psp, &securedisplay_cmd,
+			TA_SECUREDISPLAY_COMMAND__QUERY_TA);
+
+	ret = psp_securedisplay_invoke(psp, TA_SECUREDISPLAY_COMMAND__QUERY_TA);
+	if (ret) {
+		psp_securedisplay_unload(psp);
+
+		amdgpu_bo_free_kernel(&psp->securedisplay_context.securedisplay_shared_bo,
+			      &psp->securedisplay_context.securedisplay_shared_mc_addr,
+			      &psp->securedisplay_context.securedisplay_shared_buf);
+
+		psp->securedisplay_context.securedisplay_initialized = false;
+
+		dev_err(psp->adev->dev, "SECUREDISPLAY TA initialize fail.\n");
+		return -EINVAL;
+	}
+
+	if (securedisplay_cmd->status != TA_SECUREDISPLAY_STATUS__SUCCESS) {
+		psp_securedisplay_parse_resp_status(psp, securedisplay_cmd->status);
+		dev_err(psp->adev->dev, "SECUREDISPLAY: query securedisplay TA failed. ret 0x%x\n",
+			securedisplay_cmd->securedisplay_out_message.query_ta.query_cmd_ret);
+	}
+
+	return 0;
+}
+
+static int psp_securedisplay_terminate(struct psp_context *psp)
+{
+	int ret;
+
+	/*
+	 * TODO:bypass the terminate in sriov for now
+	 */
+	if (amdgpu_sriov_vf(psp->adev))
+		return 0;
+
+	if (!psp->securedisplay_context.securedisplay_initialized)
+		return 0;
+
+	ret = psp_securedisplay_unload(psp);
+	if (ret)
+		return ret;
+
+	psp->securedisplay_context.securedisplay_initialized = false;
+
+	/* free securedisplay shared memory */
+	amdgpu_bo_free_kernel(&psp->securedisplay_context.securedisplay_shared_bo,
+			      &psp->securedisplay_context.securedisplay_shared_mc_addr,
+			      &psp->securedisplay_context.securedisplay_shared_buf);
+
+	return ret;
+}
+
+int psp_securedisplay_invoke(struct psp_context *psp, uint32_t ta_cmd_id)
+{
+	int ret;
+
+	if (!psp->securedisplay_context.securedisplay_initialized)
+		return -EINVAL;
+
+	if (ta_cmd_id != TA_SECUREDISPLAY_COMMAND__QUERY_TA &&
+	    ta_cmd_id != TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC)
+		return -EINVAL;
+
+	mutex_lock(&psp->securedisplay_context.mutex);
+
+	ret = psp_ta_invoke(psp, ta_cmd_id, psp->securedisplay_context.session_id);
+
+	mutex_unlock(&psp->securedisplay_context.mutex);
+
+	return ret;
+}
+/* SECUREDISPLAY end */
+
 static int psp_hw_start(struct psp_context *psp)
 {
 	struct amdgpu_device *adev = psp->adev;
@@ -2116,6 +2290,11 @@ static int psp_load_fw(struct amdgpu_device *adev)
 		if (ret)
 			dev_err(psp->adev->dev,
 				"RAP: Failed to initialize RAP\n");
+
+		ret = psp_securedisplay_initialize(psp);
+		if (ret)
+			dev_err(psp->adev->dev,
+				"SECUREDISPLAY: Failed to initialize SECUREDISPLAY\n");
 	}
 
 	return 0;
@@ -2166,6 +2345,7 @@ static int psp_hw_fini(void *handle)
 
 	if (psp->adev->psp.ta_fw) {
 		psp_ras_terminate(psp);
+		psp_securedisplay_terminate(psp);
 		psp_rap_terminate(psp);
 		psp_dtm_terminate(psp);
 		psp_hdcp_terminate(psp);
@@ -2230,6 +2410,11 @@ static int psp_suspend(void *handle)
 			DRM_ERROR("Failed to terminate rap ta\n");
 			return ret;
 		}
+		ret = psp_securedisplay_terminate(psp);
+		if (ret) {
+			DRM_ERROR("Failed to terminate securedisplay ta\n");
+			return ret;
+		}
 	}
 
 	ret = psp_asd_unload(psp);
@@ -2313,6 +2498,11 @@ static int psp_resume(void *handle)
 		if (ret)
 			dev_err(psp->adev->dev,
 				"RAP: Failed to initialize RAP\n");
+
+		ret = psp_securedisplay_initialize(psp);
+		if (ret)
+			dev_err(psp->adev->dev,
+				"SECUREDISPLAY: Failed to initialize SECUREDISPLAY\n");
 	}
 
 	mutex_unlock(&adev->firmware.mutex);
@@ -2620,6 +2810,11 @@ static int parse_ta_bin_descriptor(struct psp_context *psp,
 		psp->ta_rap_ucode_size     = le32_to_cpu(desc->size_bytes);
 		psp->ta_rap_start_addr     = ucode_start_addr;
 		break;
+	case TA_FW_TYPE_PSP_SECUREDISPLAY:
+		psp->ta_securedisplay_ucode_version  = le32_to_cpu(desc->fw_version);
+		psp->ta_securedisplay_ucode_size     = le32_to_cpu(desc->size_bytes);
+		psp->ta_securedisplay_start_addr     = ucode_start_addr;
+		break;
 	default:
 		dev_warn(psp->adev->dev, "Unsupported TA type: %d\n", desc->fw_type);
 		break;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index da250bc1ac57..cb50ba445f8c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -30,6 +30,7 @@
 #include "ta_xgmi_if.h"
 #include "ta_ras_if.h"
 #include "ta_rap_if.h"
+#include "ta_secureDisplay_if.h"
 
 #define PSP_FENCE_BUFFER_SIZE	0x1000
 #define PSP_CMD_BUFFER_SIZE	0x1000
@@ -40,6 +41,7 @@
 #define PSP_HDCP_SHARED_MEM_SIZE	0x4000
 #define PSP_DTM_SHARED_MEM_SIZE	0x4000
 #define PSP_RAP_SHARED_MEM_SIZE	0x4000
+#define PSP_SECUREDISPLAY_SHARED_MEM_SIZE	0x4000
 #define PSP_SHARED_MEM_SIZE		0x4000
 #define PSP_FW_NAME_LEN		0x24
 
@@ -171,6 +173,15 @@ struct psp_rap_context {
 	struct mutex		mutex;
 };
 
+struct psp_securedisplay_context {
+	bool			securedisplay_initialized;
+	uint32_t		session_id;
+	struct amdgpu_bo	*securedisplay_shared_bo;
+	uint64_t		securedisplay_shared_mc_addr;
+	void			*securedisplay_shared_buf;
+	struct mutex		mutex;
+};
+
 #define MEM_TRAIN_SYSTEM_SIGNATURE		0x54534942
 #define GDDR6_MEM_TRAINING_DATA_SIZE_IN_BYTES	0x1000
 #define GDDR6_MEM_TRAINING_OFFSET		0x8000
@@ -298,12 +309,17 @@ struct psp_context
 	uint32_t			ta_rap_ucode_size;
 	uint8_t				*ta_rap_start_addr;
 
+	uint32_t			ta_securedisplay_ucode_version;
+	uint32_t			ta_securedisplay_ucode_size;
+	uint8_t				*ta_securedisplay_start_addr;
+
 	struct psp_asd_context		asd_context;
 	struct psp_xgmi_context		xgmi_context;
 	struct psp_ras_context		ras;
 	struct psp_hdcp_context 	hdcp_context;
 	struct psp_dtm_context		dtm_context;
 	struct psp_rap_context		rap_context;
+	struct psp_securedisplay_context	securedisplay_context;
 	struct mutex			mutex;
 	struct psp_memory_training_context mem_train_ctx;
 };
@@ -380,6 +396,7 @@ int psp_ras_trigger_error(struct psp_context *psp,
 int psp_hdcp_invoke(struct psp_context *psp, uint32_t ta_cmd_id);
 int psp_dtm_invoke(struct psp_context *psp, uint32_t ta_cmd_id);
 int psp_rap_invoke(struct psp_context *psp, uint32_t ta_cmd_id);
+int psp_securedisplay_invoke(struct psp_context *psp, uint32_t ta_cmd_id);
 
 int psp_rlc_autoload_start(struct psp_context *psp);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
new file mode 100644
index 000000000000..455978781380
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
@@ -0,0 +1,174 @@
+/*
+ * Copyright 2020 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ *
+ */
+#include <linux/debugfs.h>
+#include <linux/pm_runtime.h>
+
+#include "amdgpu.h"
+#include "amdgpu_securedisplay.h"
+
+/**
+ * DOC: AMDGPU SECUREDISPLAY debugfs test interface
+ *
+ * how to use?
+ * echo opcode <value> > <debugfs_dir>/dri/xxx/securedisplay_test
+ * eg. echo 1 > <debugfs_dir>/dri/xxx/securedisplay_test
+ * eg. echo 2 phy_id > <debugfs_dir>/dri/xxx/securedisplay_test
+ *
+ * opcode:
+ * 1:Query whether TA is responding used only for validation pupose
+ * 2: Send region of Interest and CRC value to I2C. (uint32)phy_id is
+ * send to determine which DIO scratch register should be used to get
+ * ROI and receive i2c_buf as the output.
+ *
+ * You can refer more detail from header file ta_securedisplay_if.h
+ *
+ */
+
+void psp_securedisplay_parse_resp_status(struct psp_context *psp,
+	enum ta_securedisplay_status status)
+{
+	switch (status) {
+	case TA_SECUREDISPLAY_STATUS__SUCCESS:
+		break;
+	case TA_SECUREDISPLAY_STATUS__GENERIC_FAILURE:
+		dev_err(psp->adev->dev, "Secure display: Generic Failure.");
+		break;
+	case TA_SECUREDISPLAY_STATUS__INVALID_PARAMETER:
+		dev_err(psp->adev->dev, "Secure display: Invalid Parameter.");
+		break;
+	case TA_SECUREDISPLAY_STATUS__NULL_POINTER:
+		dev_err(psp->adev->dev, "Secure display: Null Pointer.");
+		break;
+	case TA_SECUREDISPLAY_STATUS__I2C_WRITE_ERROR:
+		dev_err(psp->adev->dev, "Secure display: Failed to write to I2C.");
+		break;
+	case TA_SECUREDISPLAY_STATUS__READ_DIO_SCRATCH_ERROR:
+		dev_err(psp->adev->dev, "Secure display: Failed to Read DIO Scratch Register.");
+		break;
+	case TA_SECUREDISPLAY_STATUS__READ_CRC_ERROR:
+		dev_err(psp->adev->dev, "Secure display: Failed to Read CRC");
+		break;
+	default:
+		dev_err(psp->adev->dev, "Secure display: Failed to parse status: %d\n", status);
+	}
+}
+
+void psp_prep_securedisplay_cmd_buf(struct psp_context *psp, struct securedisplay_cmd **cmd,
+	enum ta_securedisplay_command command_id)
+{
+	*cmd = (struct securedisplay_cmd *)psp->securedisplay_context.securedisplay_shared_buf;
+	memset(*cmd, 0, sizeof(struct securedisplay_cmd));
+	(*cmd)->status = TA_SECUREDISPLAY_STATUS__GENERIC_FAILURE;
+	(*cmd)->cmd_id = command_id;
+}
+
+static ssize_t amdgpu_securedisplay_debugfs_write(struct file *f, const char __user *buf,
+		size_t size, loff_t *pos)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)file_inode(f)->i_private;
+	struct psp_context *psp = &adev->psp;
+	struct securedisplay_cmd *securedisplay_cmd;
+	struct drm_device *dev = adev_to_drm(adev);
+	uint32_t phy_id;
+	uint32_t op;
+	int i;
+	char str[64];
+	char i2c_output[256];
+	int ret;
+
+	if (*pos || size > sizeof(str) - 1)
+		return -EINVAL;
+
+	memset(str,  0, sizeof(str));
+	copy_from_user(str, buf, size);
+
+	ret = pm_runtime_get_sync(dev->dev);
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(dev->dev);
+		return ret;
+	}
+
+	if (size < 3)
+		sscanf(str, "%u ", &op);
+	else
+		sscanf(str, "%u %u", &op, &phy_id);
+
+	switch (op) {
+	case 1:
+		psp_prep_securedisplay_cmd_buf(psp, &securedisplay_cmd,
+			TA_SECUREDISPLAY_COMMAND__QUERY_TA);
+		ret = psp_securedisplay_invoke(psp, TA_SECUREDISPLAY_COMMAND__QUERY_TA);
+		if (!ret) {
+			if (securedisplay_cmd->status == TA_SECUREDISPLAY_STATUS__SUCCESS)
+				dev_info(adev->dev, "SECUREDISPLAY: query securedisplay TA ret is 0x%X\n",
+					securedisplay_cmd->securedisplay_out_message.query_ta.query_cmd_ret);
+			else
+				psp_securedisplay_parse_resp_status(psp, securedisplay_cmd->status);
+		}
+		break;
+	case 2:
+		psp_prep_securedisplay_cmd_buf(psp, &securedisplay_cmd,
+			TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC);
+		securedisplay_cmd->securedisplay_in_message.send_roi_crc.phy_id = phy_id;
+		ret = psp_securedisplay_invoke(psp, TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC);
+		if (!ret) {
+			if (securedisplay_cmd->status == TA_SECUREDISPLAY_STATUS__SUCCESS) {
+				memset(i2c_output,  0, sizeof(i2c_output));
+				for (i = 0; i < TA_SECUREDISPLAY_I2C_BUFFER_SIZE; i++)
+					sprintf(i2c_output, "%s 0x%X", i2c_output,
+						securedisplay_cmd->securedisplay_out_message.send_roi_crc.i2c_buf[i]);
+				dev_info(adev->dev, "SECUREDISPLAY: I2C buffer out put is :%s\n", i2c_output);
+			} else {
+				psp_securedisplay_parse_resp_status(psp, securedisplay_cmd->status);
+			}
+		}
+		break;
+	default:
+		dev_err(adev->dev, "Invalid input: %s\n", str);
+	}
+
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
+
+	return size;
+}
+
+static const struct file_operations amdgpu_securedisplay_debugfs_ops = {
+	.owner = THIS_MODULE,
+	.read = NULL,
+	.write = amdgpu_securedisplay_debugfs_write,
+	.llseek = default_llseek
+};
+
+void amdgpu_securedisplay_debugfs_init(struct amdgpu_device *adev)
+{
+#if defined(CONFIG_DEBUG_FS)
+
+	if (!adev->psp.securedisplay_context.securedisplay_initialized)
+		return;
+
+	debugfs_create_file("securedisplay_test", S_IWUSR, adev_to_drm(adev)->primary->debugfs_root,
+				adev, &amdgpu_securedisplay_debugfs_ops);
+#endif
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h
new file mode 100644
index 000000000000..983446c72520
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2020 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ *
+ */
+#ifndef _AMDGPU_SECUREDISPLAY_H
+#define _AMDGPU_SECUREDISPLAY_H
+
+#include "amdgpu.h"
+#include "ta_secureDisplay_if.h"
+
+void amdgpu_securedisplay_debugfs_init(struct amdgpu_device *adev);
+void psp_securedisplay_parse_resp_status(struct psp_context *psp,
+		enum ta_securedisplay_status status);
+void psp_prep_securedisplay_cmd_buf(struct psp_context *psp, struct securedisplay_cmd **cmd,
+		enum ta_securedisplay_command command_id);
+
+#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
index 0e43b46d3ab5..46449e70348b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
@@ -122,6 +122,9 @@ struct ta_firmware_header_v1_0 {
 	uint32_t ta_dtm_ucode_version;
 	uint32_t ta_dtm_offset_bytes;
 	uint32_t ta_dtm_size_bytes;
+	uint32_t ta_securedisplay_ucode_version;
+	uint32_t ta_securedisplay_offset_bytes;
+	uint32_t ta_securedisplay_size_bytes;
 };
 
 enum ta_fw_type {
@@ -132,6 +135,7 @@ enum ta_fw_type {
 	TA_FW_TYPE_PSP_HDCP,
 	TA_FW_TYPE_PSP_DTM,
 	TA_FW_TYPE_PSP_RAP,
+	TA_FW_TYPE_PSP_SECUREDISPLAY,
 };
 
 struct ta_fw_bin_desc {
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
index d7f92634eba2..4b1cc5e9ee92 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
@@ -92,8 +92,6 @@ static int psp_v10_0_init_microcode(struct psp_context *psp)
 			(uint8_t *)ta_hdr +
 			le32_to_cpu(ta_hdr->header.ucode_array_offset_bytes);
 
-		adev->psp.ta_fw_version = le32_to_cpu(ta_hdr->header.ucode_version);
-
 		adev->psp.ta_dtm_ucode_version =
 			le32_to_cpu(ta_hdr->ta_dtm_ucode_version);
 		adev->psp.ta_dtm_ucode_size =
@@ -101,6 +99,16 @@ static int psp_v10_0_init_microcode(struct psp_context *psp)
 		adev->psp.ta_dtm_start_addr =
 			(uint8_t *)adev->psp.ta_hdcp_start_addr +
 			le32_to_cpu(ta_hdr->ta_dtm_offset_bytes);
+
+		adev->psp.ta_securedisplay_ucode_version =
+			le32_to_cpu(ta_hdr->ta_securedisplay_ucode_version);
+		adev->psp.ta_securedisplay_ucode_size =
+			le32_to_cpu(ta_hdr->ta_securedisplay_size_bytes);
+		adev->psp.ta_securedisplay_start_addr =
+			(uint8_t *)adev->psp.ta_hdcp_start_addr +
+			le32_to_cpu(ta_hdr->ta_securedisplay_offset_bytes);
+
+		adev->psp.ta_fw_version = le32_to_cpu(ta_hdr->header.ucode_version);
 	}
 
 	return 0;
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file
  2021-01-13  3:43 [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file Jinzhou Su
  2021-01-13  3:43 ` [PATCH 2/2] drm/amdgpu: Add secure display TA interface Jinzhou Su
@ 2021-01-13  3:51 ` Huang Rui
  2021-01-13  5:33   ` Chen, Guchun
  2021-01-13 19:05 ` Paul Menzel
  2 siblings, 1 reply; 10+ messages in thread
From: Huang Rui @ 2021-01-13  3:51 UTC (permalink / raw)
  To: Su, Jinzhou (Joe); +Cc: amd-gfx

On Wed, Jan 13, 2021 at 11:43:53AM +0800, Su, Jinzhou (Joe) wrote:
> Add file ta_secureDisplay_if.h for Secure Display TA
> 
> Signed-off-by: Jinzhou Su <Jinzhou.Su@amd.com>
> Reviewed-by: Huang Rui <ray.huang@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  .../gpu/drm/amd/amdgpu/ta_secureDisplay_if.h  | 154 ++++++++++++++++++
>  1 file changed, 154 insertions(+)
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h b/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
> new file mode 100644
> index 000000000000..5039375bb1d4
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
> @@ -0,0 +1,154 @@
> +/*
> + * Copyright 2019 Advanced Micro Devices, Inc.

Please update the date as "2020". Others look good for me.

Series are Reviewed-by: Huang Rui <ray.huang@amd.com>

> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef _TA_SECUREDISPLAY_IF_H
> +#define _TA_SECUREDISPLAY_IF_H
> +
> +/** Secure Display related enumerations */
> +/**********************************************************/
> +
> +/** @enum ta_securedisplay_command
> + *    Secure Display Command ID
> + */
> +enum ta_securedisplay_command {
> +	/* Query whether TA is responding 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,
> +	/* Maximum Command ID */
> +	TA_SECUREDISPLAY_COMMAND__MAX_ID                = 0x7FFFFFFF,
> +};
> +
> +/** @enum ta_securedisplay_status
> + *    Secure Display status returns in shared buffer status
> + */
> +enum ta_securedisplay_status {
> +	TA_SECUREDISPLAY_STATUS__SUCCESS                 = 0x00,         /* Success */
> +	TA_SECUREDISPLAY_STATUS__GENERIC_FAILURE         = 0x01,         /* Generic Failure */
> +	TA_SECUREDISPLAY_STATUS__INVALID_PARAMETER       = 0x02,         /* Invalid Parameter */
> +	TA_SECUREDISPLAY_STATUS__NULL_POINTER            = 0x03,         /* Null Pointer*/
> +	TA_SECUREDISPLAY_STATUS__I2C_WRITE_ERROR         = 0x04,         /* Fail to Write to I2C */
> +	TA_SECUREDISPLAY_STATUS__READ_DIO_SCRATCH_ERROR  = 0x05, /*Fail Read DIO Scratch Register*/
> +	TA_SECUREDISPLAY_STATUS__READ_CRC_ERROR          = 0x06,         /* Fail to Read CRC*/
> +
> +	TA_SECUREDISPLAY_STATUS__MAX                     = 0x7FFFFFFF,/* Maximum Value for status*/
> +};
> +
> +/** @enum ta_securedisplay_max_phy
> + *    Physical ID number to use for reading corresponding DIO Scratch register for ROI
> + */
> +enum  ta_securedisplay_max_phy {
> +	TA_SECUREDISPLAY_PHY0                           = 0,
> +	TA_SECUREDISPLAY_PHY1                           = 1,
> +	TA_SECUREDISPLAY_PHY2                           = 2,
> +	TA_SECUREDISPLAY_PHY3                           = 3,
> +	TA_SECUREDISPLAY_MAX_PHY                        = 4,
> +};
> +
> +/** @enum ta_securedisplay_ta_query_cmd_ret
> + *    A predefined specific reteurn value which is 0xAB only used to validate
> + *    communication to Secure Display TA is functional.
> + *    This value is used to validate whether TA is responding successfully
> + */
> +enum ta_securedisplay_ta_query_cmd_ret {
> +	/* This is a value to validate if TA is loaded successfully */
> +	TA_SECUREDISPLAY_QUERY_CMD_RET                 = 0xAB,
> +};
> +
> +/** @enum ta_securedisplay_buffer_size
> + *    I2C Buffer size which contains 8 bytes of ROI  (X start, X end, Y start, Y end)
> + *    and 6 bytes of CRC( R,G,B) and 1  byte for physical ID
> + */
> +enum ta_securedisplay_buffer_size {
> +	/* 15 bytes = 8 byte (ROI) + 6 byte(CRC) + 1 byte(phy_id) */
> +	TA_SECUREDISPLAY_I2C_BUFFER_SIZE                = 15,
> +};
> +
> +/** Input/output structures for Secure Display commands */
> +/**********************************************************/
> +/**
> + * Input structures
> + */
> +
> +/** @struct ta_securedisplay_send_roi_crc_input
> + *    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 */
> +};
> +
> +/** @union ta_securedisplay_cmd_input
> + *    Input buffer
> + */
> +union ta_securedisplay_cmd_input {
> +	/* send ROI and CRC input buffer format */
> +	struct ta_securedisplay_send_roi_crc_input        send_roi_crc;
> +	uint32_t                                          reserved[4];
> +};
> +
> +/**
> + * Output structures
> + */
> +
> +/** @struct ta_securedisplay_query_ta_output
> + *  Output buffer format for query TA whether TA is responding used only for validation purpose
> + */
> +struct ta_securedisplay_query_ta_output {
> +	/* return value from TA when it is queried for validation purpose only */
> +	uint32_t  query_cmd_ret;
> +};
> +
> +/** @struct ta_securedisplay_send_roi_crc_output
> + *  Output buffer format for send ROI CRC command which will pass I2c buffer created inside TA
> + *  and used to write to I2C used only for validation purpose
> + */
> +struct ta_securedisplay_send_roi_crc_output {
> +	uint8_t  i2c_buf[TA_SECUREDISPLAY_I2C_BUFFER_SIZE];  /* I2C buffer */
> +	uint8_t  reserved;
> +};
> +
> +/** @union ta_securedisplay_cmd_output
> + *    Output buffer
> + */
> +union ta_securedisplay_cmd_output {
> +	/* Query TA output buffer format used only for validation purpose*/
> +	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;
> +	uint32_t                                           reserved[4];
> +};
> +
> +/** @struct securedisplay_cmd
> + *    Secure Display Command which is shared buffer memory
> + */
> +struct securedisplay_cmd {
> +	uint32_t                             cmd_id;                    /* +0  Bytes Command ID */
> +	enum ta_securedisplay_status         status;     /* +4  Bytes Status of Secure Display TA */
> +	uint32_t                             reserved[2];               /* +8  Bytes Reserved */
> +	union ta_securedisplay_cmd_input     securedisplay_in_message;  /* +16 Bytes Input Buffer */
> +	union ta_securedisplay_cmd_output    securedisplay_out_message;/* +32 Bytes Output Buffer */
> +	/**@note Total 48 Bytes */
> +};
> +
> +#endif   //_TA_SECUREDISPLAY_IF_H
> +
> -- 
> 2.17.1
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file
  2021-01-13  3:51 ` [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file Huang Rui
@ 2021-01-13  5:33   ` Chen, Guchun
  0 siblings, 0 replies; 10+ messages in thread
From: Chen, Guchun @ 2021-01-13  5:33 UTC (permalink / raw)
  To: Huang, Ray, Su, Jinzhou (Joe); +Cc: amd-gfx

[AMD Public Use]

Copyright year should be 2021.

Regards,
Guchun

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Huang Rui
Sent: Wednesday, January 13, 2021 11:52 AM
To: Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>
Cc: amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file

On Wed, Jan 13, 2021 at 11:43:53AM +0800, Su, Jinzhou (Joe) wrote:
> Add file ta_secureDisplay_if.h for Secure Display TA
> 
> Signed-off-by: Jinzhou Su <Jinzhou.Su@amd.com>
> Reviewed-by: Huang Rui <ray.huang@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>  .../gpu/drm/amd/amdgpu/ta_secureDisplay_if.h  | 154 
> ++++++++++++++++++
>  1 file changed, 154 insertions(+)
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h 
> b/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
> new file mode 100644
> index 000000000000..5039375bb1d4
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
> @@ -0,0 +1,154 @@
> +/*
> + * Copyright 2019 Advanced Micro Devices, Inc.

Please update the date as "2020". Others look good for me.

Series are Reviewed-by: Huang Rui <ray.huang@amd.com>

> + *
> + * Permission is hereby granted, free of charge, to any person 
> + obtaining a
> + * copy of this software and associated documentation files (the 
> + "Software"),
> + * to deal in the Software without restriction, including without 
> + limitation
> + * the rights to use, copy, modify, merge, publish, distribute, 
> + sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom 
> + the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be 
> + included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> + EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> + MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
> + SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, 
> + DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
> + OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE 
> + OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + */
> +
> +#ifndef _TA_SECUREDISPLAY_IF_H
> +#define _TA_SECUREDISPLAY_IF_H
> +
> +/** Secure Display related enumerations */ 
> +/**********************************************************/
> +
> +/** @enum ta_securedisplay_command
> + *    Secure Display Command ID
> + */
> +enum ta_securedisplay_command {
> +	/* Query whether TA is responding 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,
> +	/* Maximum Command ID */
> +	TA_SECUREDISPLAY_COMMAND__MAX_ID                = 0x7FFFFFFF,
> +};
> +
> +/** @enum ta_securedisplay_status
> + *    Secure Display status returns in shared buffer status
> + */
> +enum ta_securedisplay_status {
> +	TA_SECUREDISPLAY_STATUS__SUCCESS                 = 0x00,         /* Success */
> +	TA_SECUREDISPLAY_STATUS__GENERIC_FAILURE         = 0x01,         /* Generic Failure */
> +	TA_SECUREDISPLAY_STATUS__INVALID_PARAMETER       = 0x02,         /* Invalid Parameter */
> +	TA_SECUREDISPLAY_STATUS__NULL_POINTER            = 0x03,         /* Null Pointer*/
> +	TA_SECUREDISPLAY_STATUS__I2C_WRITE_ERROR         = 0x04,         /* Fail to Write to I2C */
> +	TA_SECUREDISPLAY_STATUS__READ_DIO_SCRATCH_ERROR  = 0x05, /*Fail Read DIO Scratch Register*/
> +	TA_SECUREDISPLAY_STATUS__READ_CRC_ERROR          = 0x06,         /* Fail to Read CRC*/
> +
> +	TA_SECUREDISPLAY_STATUS__MAX                     = 0x7FFFFFFF,/* Maximum Value for status*/
> +};
> +
> +/** @enum ta_securedisplay_max_phy
> + *    Physical ID number to use for reading corresponding DIO Scratch register for ROI
> + */
> +enum  ta_securedisplay_max_phy {
> +	TA_SECUREDISPLAY_PHY0                           = 0,
> +	TA_SECUREDISPLAY_PHY1                           = 1,
> +	TA_SECUREDISPLAY_PHY2                           = 2,
> +	TA_SECUREDISPLAY_PHY3                           = 3,
> +	TA_SECUREDISPLAY_MAX_PHY                        = 4,
> +};
> +
> +/** @enum ta_securedisplay_ta_query_cmd_ret
> + *    A predefined specific reteurn value which is 0xAB only used to validate
> + *    communication to Secure Display TA is functional.
> + *    This value is used to validate whether TA is responding successfully
> + */
> +enum ta_securedisplay_ta_query_cmd_ret {
> +	/* This is a value to validate if TA is loaded successfully */
> +	TA_SECUREDISPLAY_QUERY_CMD_RET                 = 0xAB,
> +};
> +
> +/** @enum ta_securedisplay_buffer_size
> + *    I2C Buffer size which contains 8 bytes of ROI  (X start, X end, Y start, Y end)
> + *    and 6 bytes of CRC( R,G,B) and 1  byte for physical ID
> + */
> +enum ta_securedisplay_buffer_size {
> +	/* 15 bytes = 8 byte (ROI) + 6 byte(CRC) + 1 byte(phy_id) */
> +	TA_SECUREDISPLAY_I2C_BUFFER_SIZE                = 15,
> +};
> +
> +/** Input/output structures for Secure Display commands */ 
> +/**********************************************************/
> +/**
> + * Input structures
> + */
> +
> +/** @struct ta_securedisplay_send_roi_crc_input
> + *    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 */ };
> +
> +/** @union ta_securedisplay_cmd_input
> + *    Input buffer
> + */
> +union ta_securedisplay_cmd_input {
> +	/* send ROI and CRC input buffer format */
> +	struct ta_securedisplay_send_roi_crc_input        send_roi_crc;
> +	uint32_t                                          reserved[4];
> +};
> +
> +/**
> + * Output structures
> + */
> +
> +/** @struct ta_securedisplay_query_ta_output
> + *  Output buffer format for query TA whether TA is responding used 
> +only for validation purpose  */ struct 
> +ta_securedisplay_query_ta_output {
> +	/* return value from TA when it is queried for validation purpose only */
> +	uint32_t  query_cmd_ret;
> +};
> +
> +/** @struct ta_securedisplay_send_roi_crc_output
> + *  Output buffer format for send ROI CRC command which will pass I2c 
> +buffer created inside TA
> + *  and used to write to I2C used only for validation purpose  */ 
> +struct ta_securedisplay_send_roi_crc_output {
> +	uint8_t  i2c_buf[TA_SECUREDISPLAY_I2C_BUFFER_SIZE];  /* I2C buffer */
> +	uint8_t  reserved;
> +};
> +
> +/** @union ta_securedisplay_cmd_output
> + *    Output buffer
> + */
> +union ta_securedisplay_cmd_output {
> +	/* Query TA output buffer format used only for validation purpose*/
> +	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;
> +	uint32_t                                           reserved[4];
> +};
> +
> +/** @struct securedisplay_cmd
> + *    Secure Display Command which is shared buffer memory
> + */
> +struct securedisplay_cmd {
> +	uint32_t                             cmd_id;                    /* +0  Bytes Command ID */
> +	enum ta_securedisplay_status         status;     /* +4  Bytes Status of Secure Display TA */
> +	uint32_t                             reserved[2];               /* +8  Bytes Reserved */
> +	union ta_securedisplay_cmd_input     securedisplay_in_message;  /* +16 Bytes Input Buffer */
> +	union ta_securedisplay_cmd_output    securedisplay_out_message;/* +32 Bytes Output Buffer */
> +	/**@note Total 48 Bytes */
> +};
> +
> +#endif   //_TA_SECUREDISPLAY_IF_H
> +
> --
> 2.17.1
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cguchun.chen%40amd.com%7C412e6e36b944466d66d308d8b776971b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637461067283628266%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=uEThCAg2t2tTIXv34TH4NKiE4D%2BXimtmcuMF8lb15HU%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 2/2] drm/amdgpu: Add secure display TA interface
  2021-01-13  3:43 ` [PATCH 2/2] drm/amdgpu: Add secure display TA interface Jinzhou Su
@ 2021-01-13  5:45   ` Chen, Guchun
  2021-01-13 13:53   ` Alex Deucher
  2021-01-13 15:16   ` Wang, Kevin(Yang)
  2 siblings, 0 replies; 10+ messages in thread
From: Chen, Guchun @ 2021-01-13  5:45 UTC (permalink / raw)
  To: Su, Jinzhou (Joe), amd-gfx; +Cc: Su, Jinzhou (Joe), Huang, Ray

[AMD Public Use]

One comment from coding-style's perspective.

Regards,
Guchun

-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Jinzhou Su
Sent: Wednesday, January 13, 2021 11:44 AM
To: amd-gfx@lists.freedesktop.org
Cc: Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>; Huang, Ray <Ray.Huang@amd.com>
Subject: [PATCH 2/2] drm/amdgpu: Add secure display TA interface

Add interface to load, unload, invoke command for secure display TA.

v2: Add debugfs interface for secure display TA

Signed-off-by: Jinzhou.Su <Jinzhou.Su@amd.com>
Reviewed-by: Huang Rui <ray.huang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/Makefile           |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c   |   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c       | 195 ++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h       |  17 ++
 .../gpu/drm/amd/amdgpu/amdgpu_securedisplay.c | 174 ++++++++++++++++  .../gpu/drm/amd/amdgpu/amdgpu_securedisplay.h |  36 ++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h     |   4 +
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c        |  12 +-
 8 files changed, 440 insertions(+), 3 deletions(-)  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index c6262689e14e..e4bebbfa88af 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -56,7 +56,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
 	amdgpu_gmc.o amdgpu_mmhub.o amdgpu_xgmi.o amdgpu_csa.o amdgpu_ras.o amdgpu_vm_cpu.o \
 	amdgpu_vm_sdma.o amdgpu_discovery.o amdgpu_ras_eeprom.o amdgpu_nbio.o \
 	amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
-	amdgpu_fw_attestation.o
+	amdgpu_fw_attestation.o amdgpu_securedisplay.o
 
 amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 477bead4fab1..4c38c5771cbc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -35,6 +35,7 @@
 #include "amdgpu_dm_debugfs.h"
 #include "amdgpu_ras.h"
 #include "amdgpu_rap.h"
+#include "amdgpu_securedisplay.h"
 #include "amdgpu_fw_attestation.h"
 
 /**
@@ -1666,6 +1667,8 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev)
 
 	amdgpu_rap_debugfs_init(adev);
 
+	amdgpu_securedisplay_debugfs_init(adev);
+
 	amdgpu_fw_attestation_debugfs_init(adev);
 
 	return amdgpu_debugfs_add_files(adev, amdgpu_debugfs_list, diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 523d22db094b..eb19ae734396 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -36,6 +36,7 @@
 #include "psp_v12_0.h"
 
 #include "amdgpu_ras.h"
+#include "amdgpu_securedisplay.h"
 
 static int psp_sysfs_init(struct amdgpu_device *adev);  static void psp_sysfs_fini(struct amdgpu_device *adev); @@ -1642,6 +1643,179 @@ int psp_rap_invoke(struct psp_context *psp, uint32_t ta_cmd_id)  }  // RAP end
 
+/* securedisplay start */
+static int psp_securedisplay_init_shared_buf(struct psp_context *psp) {
+	int ret;
+
+	/*
+	 * Allocate 16k memory aligned to 4k from Frame Buffer (local
+	 * physical) for sa ta <-> Driver
+	 */
+	ret = amdgpu_bo_create_kernel(psp->adev, PSP_SECUREDISPLAY_SHARED_MEM_SIZE,
+				      PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM,
+				      &psp->securedisplay_context.securedisplay_shared_bo,
+				      &psp->securedisplay_context.securedisplay_shared_mc_addr,
+				      &psp->securedisplay_context.securedisplay_shared_buf);
+
+	return ret;
+}
+
+static int psp_securedisplay_load(struct psp_context *psp) {
+	int ret;
+	struct psp_gfx_cmd_resp *cmd;
+
+	/*
+	 * TODO: bypass the loading in sriov for now
+	 */
[Guchun] Above comment is not needed, as this is guaranteed by psp_securedisplay_initialize.


+	cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
+	if (!cmd)
+		return -ENOMEM;
+
+	memset(psp->fw_pri_buf, 0, PSP_1_MEG);
+	memcpy(psp->fw_pri_buf, psp->ta_securedisplay_start_addr, 
+psp->ta_securedisplay_ucode_size);
+
+	psp_prep_ta_load_cmd_buf(cmd,
+				 psp->fw_pri_mc_addr,
+				 psp->ta_securedisplay_ucode_size,
+				 psp->securedisplay_context.securedisplay_shared_mc_addr,
+				 PSP_SECUREDISPLAY_SHARED_MEM_SIZE);
+
+	ret = psp_cmd_submit_buf(psp, NULL, cmd, psp->fence_buf_mc_addr);
+
+	if (ret)
+		goto failed;
+
+	psp->securedisplay_context.securedisplay_initialized = true;
+	psp->securedisplay_context.session_id = cmd->resp.session_id;
+	mutex_init(&psp->securedisplay_context.mutex);
+
+failed:
+	kfree(cmd);
+	return ret;
+}
+
+static int psp_securedisplay_unload(struct psp_context *psp) {
+	int ret;
+	struct psp_gfx_cmd_resp *cmd;
+
+	cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
+	if (!cmd)
+		return -ENOMEM;
+
+	psp_prep_ta_unload_cmd_buf(cmd, 
+psp->securedisplay_context.session_id);
+
+	ret = psp_cmd_submit_buf(psp, NULL, cmd, psp->fence_buf_mc_addr);
+
+	kfree(cmd);
+
+	return ret;
+}
+
+static int psp_securedisplay_initialize(struct psp_context *psp) {
+	int ret;
+	struct securedisplay_cmd *securedisplay_cmd;
+
+	/*
+	 * TODO: bypass the initialize in sriov for now
+	 */
+	if (amdgpu_sriov_vf(psp->adev))
+		return 0;
+
+	if (!psp->adev->psp.ta_securedisplay_ucode_size ||
+	    !psp->adev->psp.ta_securedisplay_start_addr) {
+		dev_info(psp->adev->dev, "SECUREDISPLAY: securedisplay ta ucode is not available\n");
+		return 0;
+	}
+
+	if (!psp->securedisplay_context.securedisplay_initialized) {
+		ret = psp_securedisplay_init_shared_buf(psp);
+		if (ret)
+			return ret;
+	}
+
+	ret = psp_securedisplay_load(psp);
+	if (ret)
+		return ret;
+
+	psp_prep_securedisplay_cmd_buf(psp, &securedisplay_cmd,
+			TA_SECUREDISPLAY_COMMAND__QUERY_TA);
+
+	ret = psp_securedisplay_invoke(psp, TA_SECUREDISPLAY_COMMAND__QUERY_TA);
+	if (ret) {
+		psp_securedisplay_unload(psp);
+
+		amdgpu_bo_free_kernel(&psp->securedisplay_context.securedisplay_shared_bo,
+			      &psp->securedisplay_context.securedisplay_shared_mc_addr,
+			      &psp->securedisplay_context.securedisplay_shared_buf);
+
+		psp->securedisplay_context.securedisplay_initialized = false;
+
+		dev_err(psp->adev->dev, "SECUREDISPLAY TA initialize fail.\n");
+		return -EINVAL;
+	}
+
+	if (securedisplay_cmd->status != TA_SECUREDISPLAY_STATUS__SUCCESS) {
+		psp_securedisplay_parse_resp_status(psp, securedisplay_cmd->status);
+		dev_err(psp->adev->dev, "SECUREDISPLAY: query securedisplay TA failed. ret 0x%x\n",
+			securedisplay_cmd->securedisplay_out_message.query_ta.query_cmd_ret);
+	}
+
+	return 0;
+}
+
+static int psp_securedisplay_terminate(struct psp_context *psp) {
+	int ret;
+
+	/*
+	 * TODO:bypass the terminate in sriov for now
+	 */
+	if (amdgpu_sriov_vf(psp->adev))
+		return 0;
+
+	if (!psp->securedisplay_context.securedisplay_initialized)
+		return 0;
+
+	ret = psp_securedisplay_unload(psp);
+	if (ret)
+		return ret;
+
+	psp->securedisplay_context.securedisplay_initialized = false;
+
+	/* free securedisplay shared memory */
+	amdgpu_bo_free_kernel(&psp->securedisplay_context.securedisplay_shared_bo,
+			      &psp->securedisplay_context.securedisplay_shared_mc_addr,
+			      &psp->securedisplay_context.securedisplay_shared_buf);
+
+	return ret;
+}
+
+int psp_securedisplay_invoke(struct psp_context *psp, uint32_t 
+ta_cmd_id) {
+	int ret;
+
+	if (!psp->securedisplay_context.securedisplay_initialized)
+		return -EINVAL;
+
+	if (ta_cmd_id != TA_SECUREDISPLAY_COMMAND__QUERY_TA &&
+	    ta_cmd_id != TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC)
+		return -EINVAL;
+
+	mutex_lock(&psp->securedisplay_context.mutex);
+
+	ret = psp_ta_invoke(psp, ta_cmd_id, 
+psp->securedisplay_context.session_id);
+
+	mutex_unlock(&psp->securedisplay_context.mutex);
+
+	return ret;
+}
+/* SECUREDISPLAY end */
+
 static int psp_hw_start(struct psp_context *psp)  {
 	struct amdgpu_device *adev = psp->adev; @@ -2116,6 +2290,11 @@ static int psp_load_fw(struct amdgpu_device *adev)
 		if (ret)
 			dev_err(psp->adev->dev,
 				"RAP: Failed to initialize RAP\n");
+
+		ret = psp_securedisplay_initialize(psp);
+		if (ret)
+			dev_err(psp->adev->dev,
+				"SECUREDISPLAY: Failed to initialize SECUREDISPLAY\n");
 	}
 
 	return 0;
@@ -2166,6 +2345,7 @@ static int psp_hw_fini(void *handle)
 
 	if (psp->adev->psp.ta_fw) {
 		psp_ras_terminate(psp);
+		psp_securedisplay_terminate(psp);
 		psp_rap_terminate(psp);
 		psp_dtm_terminate(psp);
 		psp_hdcp_terminate(psp);
@@ -2230,6 +2410,11 @@ static int psp_suspend(void *handle)
 			DRM_ERROR("Failed to terminate rap ta\n");
 			return ret;
 		}
+		ret = psp_securedisplay_terminate(psp);
+		if (ret) {
+			DRM_ERROR("Failed to terminate securedisplay ta\n");
+			return ret;
+		}
 	}
 
 	ret = psp_asd_unload(psp);
@@ -2313,6 +2498,11 @@ static int psp_resume(void *handle)
 		if (ret)
 			dev_err(psp->adev->dev,
 				"RAP: Failed to initialize RAP\n");
+
+		ret = psp_securedisplay_initialize(psp);
+		if (ret)
+			dev_err(psp->adev->dev,
+				"SECUREDISPLAY: Failed to initialize SECUREDISPLAY\n");
 	}
 
 	mutex_unlock(&adev->firmware.mutex);
@@ -2620,6 +2810,11 @@ static int parse_ta_bin_descriptor(struct psp_context *psp,
 		psp->ta_rap_ucode_size     = le32_to_cpu(desc->size_bytes);
 		psp->ta_rap_start_addr     = ucode_start_addr;
 		break;
+	case TA_FW_TYPE_PSP_SECUREDISPLAY:
+		psp->ta_securedisplay_ucode_version  = le32_to_cpu(desc->fw_version);
+		psp->ta_securedisplay_ucode_size     = le32_to_cpu(desc->size_bytes);
+		psp->ta_securedisplay_start_addr     = ucode_start_addr;
+		break;
 	default:
 		dev_warn(psp->adev->dev, "Unsupported TA type: %d\n", desc->fw_type);
 		break;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index da250bc1ac57..cb50ba445f8c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -30,6 +30,7 @@
 #include "ta_xgmi_if.h"
 #include "ta_ras_if.h"
 #include "ta_rap_if.h"
+#include "ta_secureDisplay_if.h"
 
 #define PSP_FENCE_BUFFER_SIZE	0x1000
 #define PSP_CMD_BUFFER_SIZE	0x1000
@@ -40,6 +41,7 @@
 #define PSP_HDCP_SHARED_MEM_SIZE	0x4000
 #define PSP_DTM_SHARED_MEM_SIZE	0x4000
 #define PSP_RAP_SHARED_MEM_SIZE	0x4000
+#define PSP_SECUREDISPLAY_SHARED_MEM_SIZE	0x4000
 #define PSP_SHARED_MEM_SIZE		0x4000
 #define PSP_FW_NAME_LEN		0x24
 
@@ -171,6 +173,15 @@ struct psp_rap_context {
 	struct mutex		mutex;
 };
 
+struct psp_securedisplay_context {
+	bool			securedisplay_initialized;
+	uint32_t		session_id;
+	struct amdgpu_bo	*securedisplay_shared_bo;
+	uint64_t		securedisplay_shared_mc_addr;
+	void			*securedisplay_shared_buf;
+	struct mutex		mutex;
+};
+
 #define MEM_TRAIN_SYSTEM_SIGNATURE		0x54534942
 #define GDDR6_MEM_TRAINING_DATA_SIZE_IN_BYTES	0x1000
 #define GDDR6_MEM_TRAINING_OFFSET		0x8000
@@ -298,12 +309,17 @@ struct psp_context
 	uint32_t			ta_rap_ucode_size;
 	uint8_t				*ta_rap_start_addr;
 
+	uint32_t			ta_securedisplay_ucode_version;
+	uint32_t			ta_securedisplay_ucode_size;
+	uint8_t				*ta_securedisplay_start_addr;
+
 	struct psp_asd_context		asd_context;
 	struct psp_xgmi_context		xgmi_context;
 	struct psp_ras_context		ras;
 	struct psp_hdcp_context 	hdcp_context;
 	struct psp_dtm_context		dtm_context;
 	struct psp_rap_context		rap_context;
+	struct psp_securedisplay_context	securedisplay_context;
 	struct mutex			mutex;
 	struct psp_memory_training_context mem_train_ctx;  }; @@ -380,6 +396,7 @@ int psp_ras_trigger_error(struct psp_context *psp,  int psp_hdcp_invoke(struct psp_context *psp, uint32_t ta_cmd_id);  int psp_dtm_invoke(struct psp_context *psp, uint32_t ta_cmd_id);  int psp_rap_invoke(struct psp_context *psp, uint32_t ta_cmd_id);
+int psp_securedisplay_invoke(struct psp_context *psp, uint32_t 
+ta_cmd_id);
 
 int psp_rlc_autoload_start(struct psp_context *psp);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
new file mode 100644
index 000000000000..455978781380
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
@@ -0,0 +1,174 @@
+/*
+ * Copyright 2020 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person 
+obtaining a
+ * copy of this software and associated documentation files (the 
+"Software"),
+ * to deal in the Software without restriction, including without 
+limitation
+ * the rights to use, copy, modify, merge, publish, distribute, 
+sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom 
+the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be 
+included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
+EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
+MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
+SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, 
+DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
+OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE 
+OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ *
+ */
+#include <linux/debugfs.h>
+#include <linux/pm_runtime.h>
+
+#include "amdgpu.h"
+#include "amdgpu_securedisplay.h"
+
+/**
+ * DOC: AMDGPU SECUREDISPLAY debugfs test interface
+ *
+ * how to use?
+ * echo opcode <value> > <debugfs_dir>/dri/xxx/securedisplay_test
+ * eg. echo 1 > <debugfs_dir>/dri/xxx/securedisplay_test
+ * eg. echo 2 phy_id > <debugfs_dir>/dri/xxx/securedisplay_test
+ *
+ * opcode:
+ * 1:Query whether TA is responding used only for validation pupose
+ * 2: Send region of Interest and CRC value to I2C. (uint32)phy_id is
+ * send to determine which DIO scratch register should be used to get
+ * ROI and receive i2c_buf as the output.
+ *
+ * You can refer more detail from header file ta_securedisplay_if.h
+ *
+ */
+
+void psp_securedisplay_parse_resp_status(struct psp_context *psp,
+	enum ta_securedisplay_status status)
+{
+	switch (status) {
+	case TA_SECUREDISPLAY_STATUS__SUCCESS:
+		break;
+	case TA_SECUREDISPLAY_STATUS__GENERIC_FAILURE:
+		dev_err(psp->adev->dev, "Secure display: Generic Failure.");
+		break;
+	case TA_SECUREDISPLAY_STATUS__INVALID_PARAMETER:
+		dev_err(psp->adev->dev, "Secure display: Invalid Parameter.");
+		break;
+	case TA_SECUREDISPLAY_STATUS__NULL_POINTER:
+		dev_err(psp->adev->dev, "Secure display: Null Pointer.");
+		break;
+	case TA_SECUREDISPLAY_STATUS__I2C_WRITE_ERROR:
+		dev_err(psp->adev->dev, "Secure display: Failed to write to I2C.");
+		break;
+	case TA_SECUREDISPLAY_STATUS__READ_DIO_SCRATCH_ERROR:
+		dev_err(psp->adev->dev, "Secure display: Failed to Read DIO Scratch Register.");
+		break;
+	case TA_SECUREDISPLAY_STATUS__READ_CRC_ERROR:
+		dev_err(psp->adev->dev, "Secure display: Failed to Read CRC");
+		break;
+	default:
+		dev_err(psp->adev->dev, "Secure display: Failed to parse status: %d\n", status);
+	}
+}
+
+void psp_prep_securedisplay_cmd_buf(struct psp_context *psp, struct securedisplay_cmd **cmd,
+	enum ta_securedisplay_command command_id) {
+	*cmd = (struct securedisplay_cmd *)psp->securedisplay_context.securedisplay_shared_buf;
+	memset(*cmd, 0, sizeof(struct securedisplay_cmd));
+	(*cmd)->status = TA_SECUREDISPLAY_STATUS__GENERIC_FAILURE;
+	(*cmd)->cmd_id = command_id;
+}
+
+static ssize_t amdgpu_securedisplay_debugfs_write(struct file *f, const char __user *buf,
+		size_t size, loff_t *pos)
+{
+	struct amdgpu_device *adev = (struct amdgpu_device *)file_inode(f)->i_private;
+	struct psp_context *psp = &adev->psp;
+	struct securedisplay_cmd *securedisplay_cmd;
+	struct drm_device *dev = adev_to_drm(adev);
+	uint32_t phy_id;
+	uint32_t op;
+	int i;
+	char str[64];
+	char i2c_output[256];
+	int ret;
+
+	if (*pos || size > sizeof(str) - 1)
+		return -EINVAL;
+
+	memset(str,  0, sizeof(str));
+	copy_from_user(str, buf, size);
+
+	ret = pm_runtime_get_sync(dev->dev);
+	if (ret < 0) {
+		pm_runtime_put_autosuspend(dev->dev);
+		return ret;
+	}
+
+	if (size < 3)
+		sscanf(str, "%u ", &op);
+	else
+		sscanf(str, "%u %u", &op, &phy_id);
+
+	switch (op) {
+	case 1:
+		psp_prep_securedisplay_cmd_buf(psp, &securedisplay_cmd,
+			TA_SECUREDISPLAY_COMMAND__QUERY_TA);
+		ret = psp_securedisplay_invoke(psp, TA_SECUREDISPLAY_COMMAND__QUERY_TA);
+		if (!ret) {
+			if (securedisplay_cmd->status == TA_SECUREDISPLAY_STATUS__SUCCESS)
+				dev_info(adev->dev, "SECUREDISPLAY: query securedisplay TA ret is 0x%X\n",
+					securedisplay_cmd->securedisplay_out_message.query_ta.query_cmd_ret);
+			else
+				psp_securedisplay_parse_resp_status(psp, securedisplay_cmd->status);
+		}
+		break;
+	case 2:
+		psp_prep_securedisplay_cmd_buf(psp, &securedisplay_cmd,
+			TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC);
+		securedisplay_cmd->securedisplay_in_message.send_roi_crc.phy_id = phy_id;
+		ret = psp_securedisplay_invoke(psp, TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC);
+		if (!ret) {
+			if (securedisplay_cmd->status == TA_SECUREDISPLAY_STATUS__SUCCESS) {
+				memset(i2c_output,  0, sizeof(i2c_output));
+				for (i = 0; i < TA_SECUREDISPLAY_I2C_BUFFER_SIZE; i++)
+					sprintf(i2c_output, "%s 0x%X", i2c_output,
+						securedisplay_cmd->securedisplay_out_message.send_roi_crc.i2c_buf[i]);
+				dev_info(adev->dev, "SECUREDISPLAY: I2C buffer out put is :%s\n", i2c_output);
+			} else {
+				psp_securedisplay_parse_resp_status(psp, securedisplay_cmd->status);
+			}
+		}
+		break;
+	default:
+		dev_err(adev->dev, "Invalid input: %s\n", str);
+	}
+
+	pm_runtime_mark_last_busy(dev->dev);
+	pm_runtime_put_autosuspend(dev->dev);
+
+	return size;
+}
+
+static const struct file_operations amdgpu_securedisplay_debugfs_ops = {
+	.owner = THIS_MODULE,
+	.read = NULL,
+	.write = amdgpu_securedisplay_debugfs_write,
+	.llseek = default_llseek
+};
+
+void amdgpu_securedisplay_debugfs_init(struct amdgpu_device *adev) { 
+#if defined(CONFIG_DEBUG_FS)
+
+	if (!adev->psp.securedisplay_context.securedisplay_initialized)
+		return;
+
+	debugfs_create_file("securedisplay_test", S_IWUSR, adev_to_drm(adev)->primary->debugfs_root,
+				adev, &amdgpu_securedisplay_debugfs_ops);
+#endif
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h
new file mode 100644
index 000000000000..983446c72520
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2020 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person 
+obtaining a
+ * copy of this software and associated documentation files (the 
+"Software"),
+ * to deal in the Software without restriction, including without 
+limitation
+ * the rights to use, copy, modify, merge, publish, distribute, 
+sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom 
+the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be 
+included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
+EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
+MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
+SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, 
+DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
+OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE 
+OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ *
+ */
+#ifndef _AMDGPU_SECUREDISPLAY_H
+#define _AMDGPU_SECUREDISPLAY_H
+
+#include "amdgpu.h"
+#include "ta_secureDisplay_if.h"
+
+void amdgpu_securedisplay_debugfs_init(struct amdgpu_device *adev); 
+void psp_securedisplay_parse_resp_status(struct psp_context *psp,
+		enum ta_securedisplay_status status); void 
+psp_prep_securedisplay_cmd_buf(struct psp_context *psp, struct securedisplay_cmd **cmd,
+		enum ta_securedisplay_command command_id);
+
+#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
index 0e43b46d3ab5..46449e70348b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
@@ -122,6 +122,9 @@ struct ta_firmware_header_v1_0 {
 	uint32_t ta_dtm_ucode_version;
 	uint32_t ta_dtm_offset_bytes;
 	uint32_t ta_dtm_size_bytes;
+	uint32_t ta_securedisplay_ucode_version;
+	uint32_t ta_securedisplay_offset_bytes;
+	uint32_t ta_securedisplay_size_bytes;
 };
 
 enum ta_fw_type {
@@ -132,6 +135,7 @@ enum ta_fw_type {
 	TA_FW_TYPE_PSP_HDCP,
 	TA_FW_TYPE_PSP_DTM,
 	TA_FW_TYPE_PSP_RAP,
+	TA_FW_TYPE_PSP_SECUREDISPLAY,
 };
 
 struct ta_fw_bin_desc {
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
index d7f92634eba2..4b1cc5e9ee92 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
@@ -92,8 +92,6 @@ static int psp_v10_0_init_microcode(struct psp_context *psp)
 			(uint8_t *)ta_hdr +
 			le32_to_cpu(ta_hdr->header.ucode_array_offset_bytes);
 
-		adev->psp.ta_fw_version = le32_to_cpu(ta_hdr->header.ucode_version);
-
 		adev->psp.ta_dtm_ucode_version =
 			le32_to_cpu(ta_hdr->ta_dtm_ucode_version);
 		adev->psp.ta_dtm_ucode_size =
@@ -101,6 +99,16 @@ static int psp_v10_0_init_microcode(struct psp_context *psp)
 		adev->psp.ta_dtm_start_addr =
 			(uint8_t *)adev->psp.ta_hdcp_start_addr +
 			le32_to_cpu(ta_hdr->ta_dtm_offset_bytes);
+
+		adev->psp.ta_securedisplay_ucode_version =
+			le32_to_cpu(ta_hdr->ta_securedisplay_ucode_version);
+		adev->psp.ta_securedisplay_ucode_size =
+			le32_to_cpu(ta_hdr->ta_securedisplay_size_bytes);
+		adev->psp.ta_securedisplay_start_addr =
+			(uint8_t *)adev->psp.ta_hdcp_start_addr +
+			le32_to_cpu(ta_hdr->ta_securedisplay_offset_bytes);
+
+		adev->psp.ta_fw_version = le32_to_cpu(ta_hdr->header.ucode_version);
 	}
 
 	return 0;
--
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7Cguchun.chen%40amd.com%7C105d330e2a9f417d324408d8b775887e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637461062775011351%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=2LyTLuKVDdzQ0Dk3EOsEpgfmcl4vbr1CmxXsoe7O%2F%2Bo%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: Add secure display TA interface
  2021-01-13  3:43 ` [PATCH 2/2] drm/amdgpu: Add secure display TA interface Jinzhou Su
  2021-01-13  5:45   ` Chen, Guchun
@ 2021-01-13 13:53   ` Alex Deucher
  2021-01-13 15:16   ` Wang, Kevin(Yang)
  2 siblings, 0 replies; 10+ messages in thread
From: Alex Deucher @ 2021-01-13 13:53 UTC (permalink / raw)
  To: Jinzhou Su; +Cc: Huang Rui, amd-gfx list

On Tue, Jan 12, 2021 at 10:44 PM Jinzhou Su <Jinzhou.Su@amd.com> wrote:
>
> Add interface to load, unload, invoke command for
> secure display TA.
>
> v2: Add debugfs interface for secure display TA
>
> Signed-off-by: Jinzhou.Su <Jinzhou.Su@amd.com>
> Reviewed-by: Huang Rui <ray.huang@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/Makefile           |   2 +-
>  drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c   |   3 +
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c       | 195 ++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h       |  17 ++
>  .../gpu/drm/amd/amdgpu/amdgpu_securedisplay.c | 174 ++++++++++++++++
>  .../gpu/drm/amd/amdgpu/amdgpu_securedisplay.h |  36 ++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h     |   4 +
>  drivers/gpu/drm/amd/amdgpu/psp_v10_0.c        |  12 +-
>  8 files changed, 440 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
>  create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
> index c6262689e14e..e4bebbfa88af 100644
> --- a/drivers/gpu/drm/amd/amdgpu/Makefile
> +++ b/drivers/gpu/drm/amd/amdgpu/Makefile
> @@ -56,7 +56,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
>         amdgpu_gmc.o amdgpu_mmhub.o amdgpu_xgmi.o amdgpu_csa.o amdgpu_ras.o amdgpu_vm_cpu.o \
>         amdgpu_vm_sdma.o amdgpu_discovery.o amdgpu_ras_eeprom.o amdgpu_nbio.o \
>         amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
> -       amdgpu_fw_attestation.o
> +       amdgpu_fw_attestation.o amdgpu_securedisplay.o
>
>  amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index 477bead4fab1..4c38c5771cbc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -35,6 +35,7 @@
>  #include "amdgpu_dm_debugfs.h"
>  #include "amdgpu_ras.h"
>  #include "amdgpu_rap.h"
> +#include "amdgpu_securedisplay.h"
>  #include "amdgpu_fw_attestation.h"
>
>  /**
> @@ -1666,6 +1667,8 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev)
>
>         amdgpu_rap_debugfs_init(adev);
>
> +       amdgpu_securedisplay_debugfs_init(adev);
> +
>         amdgpu_fw_attestation_debugfs_init(adev);
>
>         return amdgpu_debugfs_add_files(adev, amdgpu_debugfs_list,
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index 523d22db094b..eb19ae734396 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -36,6 +36,7 @@
>  #include "psp_v12_0.h"
>
>  #include "amdgpu_ras.h"
> +#include "amdgpu_securedisplay.h"
>
>  static int psp_sysfs_init(struct amdgpu_device *adev);
>  static void psp_sysfs_fini(struct amdgpu_device *adev);
> @@ -1642,6 +1643,179 @@ int psp_rap_invoke(struct psp_context *psp, uint32_t ta_cmd_id)
>  }
>  // RAP end
>
> +/* securedisplay start */
> +static int psp_securedisplay_init_shared_buf(struct psp_context *psp)
> +{
> +       int ret;
> +
> +       /*
> +        * Allocate 16k memory aligned to 4k from Frame Buffer (local
> +        * physical) for sa ta <-> Driver
> +        */
> +       ret = amdgpu_bo_create_kernel(psp->adev, PSP_SECUREDISPLAY_SHARED_MEM_SIZE,
> +                                     PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM,
> +                                     &psp->securedisplay_context.securedisplay_shared_bo,
> +                                     &psp->securedisplay_context.securedisplay_shared_mc_addr,
> +                                     &psp->securedisplay_context.securedisplay_shared_buf);
> +
> +       return ret;
> +}
> +
> +static int psp_securedisplay_load(struct psp_context *psp)
> +{
> +       int ret;
> +       struct psp_gfx_cmd_resp *cmd;
> +
> +       /*
> +        * TODO: bypass the loading in sriov for now
> +        */
> +
> +       cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
> +       if (!cmd)
> +               return -ENOMEM;
> +
> +       memset(psp->fw_pri_buf, 0, PSP_1_MEG);
> +       memcpy(psp->fw_pri_buf, psp->ta_securedisplay_start_addr, psp->ta_securedisplay_ucode_size);
> +
> +       psp_prep_ta_load_cmd_buf(cmd,
> +                                psp->fw_pri_mc_addr,
> +                                psp->ta_securedisplay_ucode_size,
> +                                psp->securedisplay_context.securedisplay_shared_mc_addr,
> +                                PSP_SECUREDISPLAY_SHARED_MEM_SIZE);
> +
> +       ret = psp_cmd_submit_buf(psp, NULL, cmd, psp->fence_buf_mc_addr);
> +
> +       if (ret)
> +               goto failed;
> +
> +       psp->securedisplay_context.securedisplay_initialized = true;
> +       psp->securedisplay_context.session_id = cmd->resp.session_id;
> +       mutex_init(&psp->securedisplay_context.mutex);
> +
> +failed:
> +       kfree(cmd);
> +       return ret;
> +}
> +
> +static int psp_securedisplay_unload(struct psp_context *psp)
> +{
> +       int ret;
> +       struct psp_gfx_cmd_resp *cmd;
> +
> +       cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
> +       if (!cmd)
> +               return -ENOMEM;
> +
> +       psp_prep_ta_unload_cmd_buf(cmd, psp->securedisplay_context.session_id);
> +
> +       ret = psp_cmd_submit_buf(psp, NULL, cmd, psp->fence_buf_mc_addr);
> +
> +       kfree(cmd);
> +
> +       return ret;
> +}
> +
> +static int psp_securedisplay_initialize(struct psp_context *psp)
> +{
> +       int ret;
> +       struct securedisplay_cmd *securedisplay_cmd;
> +
> +       /*
> +        * TODO: bypass the initialize in sriov for now
> +        */
> +       if (amdgpu_sriov_vf(psp->adev))
> +               return 0;
> +
> +       if (!psp->adev->psp.ta_securedisplay_ucode_size ||
> +           !psp->adev->psp.ta_securedisplay_start_addr) {
> +               dev_info(psp->adev->dev, "SECUREDISPLAY: securedisplay ta ucode is not available\n");
> +               return 0;
> +       }
> +
> +       if (!psp->securedisplay_context.securedisplay_initialized) {
> +               ret = psp_securedisplay_init_shared_buf(psp);
> +               if (ret)
> +                       return ret;
> +       }
> +
> +       ret = psp_securedisplay_load(psp);
> +       if (ret)
> +               return ret;
> +
> +       psp_prep_securedisplay_cmd_buf(psp, &securedisplay_cmd,
> +                       TA_SECUREDISPLAY_COMMAND__QUERY_TA);
> +
> +       ret = psp_securedisplay_invoke(psp, TA_SECUREDISPLAY_COMMAND__QUERY_TA);
> +       if (ret) {
> +               psp_securedisplay_unload(psp);
> +
> +               amdgpu_bo_free_kernel(&psp->securedisplay_context.securedisplay_shared_bo,
> +                             &psp->securedisplay_context.securedisplay_shared_mc_addr,
> +                             &psp->securedisplay_context.securedisplay_shared_buf);
> +
> +               psp->securedisplay_context.securedisplay_initialized = false;
> +
> +               dev_err(psp->adev->dev, "SECUREDISPLAY TA initialize fail.\n");
> +               return -EINVAL;
> +       }
> +
> +       if (securedisplay_cmd->status != TA_SECUREDISPLAY_STATUS__SUCCESS) {
> +               psp_securedisplay_parse_resp_status(psp, securedisplay_cmd->status);
> +               dev_err(psp->adev->dev, "SECUREDISPLAY: query securedisplay TA failed. ret 0x%x\n",
> +                       securedisplay_cmd->securedisplay_out_message.query_ta.query_cmd_ret);
> +       }
> +
> +       return 0;
> +}
> +
> +static int psp_securedisplay_terminate(struct psp_context *psp)
> +{
> +       int ret;
> +
> +       /*
> +        * TODO:bypass the terminate in sriov for now
> +        */
> +       if (amdgpu_sriov_vf(psp->adev))
> +               return 0;
> +
> +       if (!psp->securedisplay_context.securedisplay_initialized)
> +               return 0;
> +
> +       ret = psp_securedisplay_unload(psp);
> +       if (ret)
> +               return ret;
> +
> +       psp->securedisplay_context.securedisplay_initialized = false;
> +
> +       /* free securedisplay shared memory */
> +       amdgpu_bo_free_kernel(&psp->securedisplay_context.securedisplay_shared_bo,
> +                             &psp->securedisplay_context.securedisplay_shared_mc_addr,
> +                             &psp->securedisplay_context.securedisplay_shared_buf);
> +
> +       return ret;
> +}
> +
> +int psp_securedisplay_invoke(struct psp_context *psp, uint32_t ta_cmd_id)
> +{
> +       int ret;
> +
> +       if (!psp->securedisplay_context.securedisplay_initialized)
> +               return -EINVAL;
> +
> +       if (ta_cmd_id != TA_SECUREDISPLAY_COMMAND__QUERY_TA &&
> +           ta_cmd_id != TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC)
> +               return -EINVAL;
> +
> +       mutex_lock(&psp->securedisplay_context.mutex);
> +
> +       ret = psp_ta_invoke(psp, ta_cmd_id, psp->securedisplay_context.session_id);
> +
> +       mutex_unlock(&psp->securedisplay_context.mutex);
> +
> +       return ret;
> +}
> +/* SECUREDISPLAY end */
> +
>  static int psp_hw_start(struct psp_context *psp)
>  {
>         struct amdgpu_device *adev = psp->adev;
> @@ -2116,6 +2290,11 @@ static int psp_load_fw(struct amdgpu_device *adev)
>                 if (ret)
>                         dev_err(psp->adev->dev,
>                                 "RAP: Failed to initialize RAP\n");
> +
> +               ret = psp_securedisplay_initialize(psp);
> +               if (ret)
> +                       dev_err(psp->adev->dev,
> +                               "SECUREDISPLAY: Failed to initialize SECUREDISPLAY\n");
>         }
>
>         return 0;
> @@ -2166,6 +2345,7 @@ static int psp_hw_fini(void *handle)
>
>         if (psp->adev->psp.ta_fw) {
>                 psp_ras_terminate(psp);
> +               psp_securedisplay_terminate(psp);
>                 psp_rap_terminate(psp);
>                 psp_dtm_terminate(psp);
>                 psp_hdcp_terminate(psp);
> @@ -2230,6 +2410,11 @@ static int psp_suspend(void *handle)
>                         DRM_ERROR("Failed to terminate rap ta\n");
>                         return ret;
>                 }
> +               ret = psp_securedisplay_terminate(psp);
> +               if (ret) {
> +                       DRM_ERROR("Failed to terminate securedisplay ta\n");
> +                       return ret;
> +               }
>         }
>
>         ret = psp_asd_unload(psp);
> @@ -2313,6 +2498,11 @@ static int psp_resume(void *handle)
>                 if (ret)
>                         dev_err(psp->adev->dev,
>                                 "RAP: Failed to initialize RAP\n");
> +
> +               ret = psp_securedisplay_initialize(psp);
> +               if (ret)
> +                       dev_err(psp->adev->dev,
> +                               "SECUREDISPLAY: Failed to initialize SECUREDISPLAY\n");
>         }
>
>         mutex_unlock(&adev->firmware.mutex);
> @@ -2620,6 +2810,11 @@ static int parse_ta_bin_descriptor(struct psp_context *psp,
>                 psp->ta_rap_ucode_size     = le32_to_cpu(desc->size_bytes);
>                 psp->ta_rap_start_addr     = ucode_start_addr;
>                 break;
> +       case TA_FW_TYPE_PSP_SECUREDISPLAY:
> +               psp->ta_securedisplay_ucode_version  = le32_to_cpu(desc->fw_version);
> +               psp->ta_securedisplay_ucode_size     = le32_to_cpu(desc->size_bytes);
> +               psp->ta_securedisplay_start_addr     = ucode_start_addr;
> +               break;
>         default:
>                 dev_warn(psp->adev->dev, "Unsupported TA type: %d\n", desc->fw_type);
>                 break;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> index da250bc1ac57..cb50ba445f8c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> @@ -30,6 +30,7 @@
>  #include "ta_xgmi_if.h"
>  #include "ta_ras_if.h"
>  #include "ta_rap_if.h"
> +#include "ta_secureDisplay_if.h"
>
>  #define PSP_FENCE_BUFFER_SIZE  0x1000
>  #define PSP_CMD_BUFFER_SIZE    0x1000
> @@ -40,6 +41,7 @@
>  #define PSP_HDCP_SHARED_MEM_SIZE       0x4000
>  #define PSP_DTM_SHARED_MEM_SIZE        0x4000
>  #define PSP_RAP_SHARED_MEM_SIZE        0x4000
> +#define PSP_SECUREDISPLAY_SHARED_MEM_SIZE      0x4000
>  #define PSP_SHARED_MEM_SIZE            0x4000
>  #define PSP_FW_NAME_LEN                0x24
>
> @@ -171,6 +173,15 @@ struct psp_rap_context {
>         struct mutex            mutex;
>  };
>
> +struct psp_securedisplay_context {
> +       bool                    securedisplay_initialized;
> +       uint32_t                session_id;
> +       struct amdgpu_bo        *securedisplay_shared_bo;
> +       uint64_t                securedisplay_shared_mc_addr;
> +       void                    *securedisplay_shared_buf;
> +       struct mutex            mutex;
> +};
> +
>  #define MEM_TRAIN_SYSTEM_SIGNATURE             0x54534942
>  #define GDDR6_MEM_TRAINING_DATA_SIZE_IN_BYTES  0x1000
>  #define GDDR6_MEM_TRAINING_OFFSET              0x8000
> @@ -298,12 +309,17 @@ struct psp_context
>         uint32_t                        ta_rap_ucode_size;
>         uint8_t                         *ta_rap_start_addr;
>
> +       uint32_t                        ta_securedisplay_ucode_version;
> +       uint32_t                        ta_securedisplay_ucode_size;
> +       uint8_t                         *ta_securedisplay_start_addr;
> +
>         struct psp_asd_context          asd_context;
>         struct psp_xgmi_context         xgmi_context;
>         struct psp_ras_context          ras;
>         struct psp_hdcp_context         hdcp_context;
>         struct psp_dtm_context          dtm_context;
>         struct psp_rap_context          rap_context;
> +       struct psp_securedisplay_context        securedisplay_context;
>         struct mutex                    mutex;
>         struct psp_memory_training_context mem_train_ctx;
>  };
> @@ -380,6 +396,7 @@ int psp_ras_trigger_error(struct psp_context *psp,
>  int psp_hdcp_invoke(struct psp_context *psp, uint32_t ta_cmd_id);
>  int psp_dtm_invoke(struct psp_context *psp, uint32_t ta_cmd_id);
>  int psp_rap_invoke(struct psp_context *psp, uint32_t ta_cmd_id);
> +int psp_securedisplay_invoke(struct psp_context *psp, uint32_t ta_cmd_id);
>
>  int psp_rlc_autoload_start(struct psp_context *psp);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
> new file mode 100644
> index 000000000000..455978781380
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
> @@ -0,0 +1,174 @@
> +/*
> + * Copyright 2020 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + *
> + */
> +#include <linux/debugfs.h>
> +#include <linux/pm_runtime.h>
> +
> +#include "amdgpu.h"
> +#include "amdgpu_securedisplay.h"
> +
> +/**
> + * DOC: AMDGPU SECUREDISPLAY debugfs test interface
> + *
> + * how to use?
> + * echo opcode <value> > <debugfs_dir>/dri/xxx/securedisplay_test
> + * eg. echo 1 > <debugfs_dir>/dri/xxx/securedisplay_test
> + * eg. echo 2 phy_id > <debugfs_dir>/dri/xxx/securedisplay_test
> + *
> + * opcode:
> + * 1:Query whether TA is responding used only for validation pupose
> + * 2: Send region of Interest and CRC value to I2C. (uint32)phy_id is
> + * send to determine which DIO scratch register should be used to get
> + * ROI and receive i2c_buf as the output.
> + *
> + * You can refer more detail from header file ta_securedisplay_if.h
> + *
> + */
> +
> +void psp_securedisplay_parse_resp_status(struct psp_context *psp,
> +       enum ta_securedisplay_status status)
> +{
> +       switch (status) {
> +       case TA_SECUREDISPLAY_STATUS__SUCCESS:
> +               break;
> +       case TA_SECUREDISPLAY_STATUS__GENERIC_FAILURE:
> +               dev_err(psp->adev->dev, "Secure display: Generic Failure.");
> +               break;
> +       case TA_SECUREDISPLAY_STATUS__INVALID_PARAMETER:
> +               dev_err(psp->adev->dev, "Secure display: Invalid Parameter.");
> +               break;
> +       case TA_SECUREDISPLAY_STATUS__NULL_POINTER:
> +               dev_err(psp->adev->dev, "Secure display: Null Pointer.");
> +               break;
> +       case TA_SECUREDISPLAY_STATUS__I2C_WRITE_ERROR:
> +               dev_err(psp->adev->dev, "Secure display: Failed to write to I2C.");
> +               break;
> +       case TA_SECUREDISPLAY_STATUS__READ_DIO_SCRATCH_ERROR:
> +               dev_err(psp->adev->dev, "Secure display: Failed to Read DIO Scratch Register.");
> +               break;
> +       case TA_SECUREDISPLAY_STATUS__READ_CRC_ERROR:
> +               dev_err(psp->adev->dev, "Secure display: Failed to Read CRC");
> +               break;
> +       default:
> +               dev_err(psp->adev->dev, "Secure display: Failed to parse status: %d\n", status);
> +       }
> +}
> +
> +void psp_prep_securedisplay_cmd_buf(struct psp_context *psp, struct securedisplay_cmd **cmd,
> +       enum ta_securedisplay_command command_id)
> +{
> +       *cmd = (struct securedisplay_cmd *)psp->securedisplay_context.securedisplay_shared_buf;
> +       memset(*cmd, 0, sizeof(struct securedisplay_cmd));
> +       (*cmd)->status = TA_SECUREDISPLAY_STATUS__GENERIC_FAILURE;
> +       (*cmd)->cmd_id = command_id;
> +}
> +
> +static ssize_t amdgpu_securedisplay_debugfs_write(struct file *f, const char __user *buf,
> +               size_t size, loff_t *pos)
> +{
> +       struct amdgpu_device *adev = (struct amdgpu_device *)file_inode(f)->i_private;
> +       struct psp_context *psp = &adev->psp;
> +       struct securedisplay_cmd *securedisplay_cmd;
> +       struct drm_device *dev = adev_to_drm(adev);
> +       uint32_t phy_id;
> +       uint32_t op;
> +       int i;
> +       char str[64];
> +       char i2c_output[256];
> +       int ret;
> +
> +       if (*pos || size > sizeof(str) - 1)
> +               return -EINVAL;
> +
> +       memset(str,  0, sizeof(str));
> +       copy_from_user(str, buf, size);

You need to check the return value from copy_from_user(), otherwise,
you'll get a warning.

Alex


> +
> +       ret = pm_runtime_get_sync(dev->dev);
> +       if (ret < 0) {
> +               pm_runtime_put_autosuspend(dev->dev);
> +               return ret;
> +       }
> +
> +       if (size < 3)
> +               sscanf(str, "%u ", &op);
> +       else
> +               sscanf(str, "%u %u", &op, &phy_id);
> +
> +       switch (op) {
> +       case 1:
> +               psp_prep_securedisplay_cmd_buf(psp, &securedisplay_cmd,
> +                       TA_SECUREDISPLAY_COMMAND__QUERY_TA);
> +               ret = psp_securedisplay_invoke(psp, TA_SECUREDISPLAY_COMMAND__QUERY_TA);
> +               if (!ret) {
> +                       if (securedisplay_cmd->status == TA_SECUREDISPLAY_STATUS__SUCCESS)
> +                               dev_info(adev->dev, "SECUREDISPLAY: query securedisplay TA ret is 0x%X\n",
> +                                       securedisplay_cmd->securedisplay_out_message.query_ta.query_cmd_ret);
> +                       else
> +                               psp_securedisplay_parse_resp_status(psp, securedisplay_cmd->status);
> +               }
> +               break;
> +       case 2:
> +               psp_prep_securedisplay_cmd_buf(psp, &securedisplay_cmd,
> +                       TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC);
> +               securedisplay_cmd->securedisplay_in_message.send_roi_crc.phy_id = phy_id;
> +               ret = psp_securedisplay_invoke(psp, TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC);
> +               if (!ret) {
> +                       if (securedisplay_cmd->status == TA_SECUREDISPLAY_STATUS__SUCCESS) {
> +                               memset(i2c_output,  0, sizeof(i2c_output));
> +                               for (i = 0; i < TA_SECUREDISPLAY_I2C_BUFFER_SIZE; i++)
> +                                       sprintf(i2c_output, "%s 0x%X", i2c_output,
> +                                               securedisplay_cmd->securedisplay_out_message.send_roi_crc.i2c_buf[i]);
> +                               dev_info(adev->dev, "SECUREDISPLAY: I2C buffer out put is :%s\n", i2c_output);
> +                       } else {
> +                               psp_securedisplay_parse_resp_status(psp, securedisplay_cmd->status);
> +                       }
> +               }
> +               break;
> +       default:
> +               dev_err(adev->dev, "Invalid input: %s\n", str);
> +       }
> +
> +       pm_runtime_mark_last_busy(dev->dev);
> +       pm_runtime_put_autosuspend(dev->dev);
> +
> +       return size;
> +}
> +
> +static const struct file_operations amdgpu_securedisplay_debugfs_ops = {
> +       .owner = THIS_MODULE,
> +       .read = NULL,
> +       .write = amdgpu_securedisplay_debugfs_write,
> +       .llseek = default_llseek
> +};
> +
> +void amdgpu_securedisplay_debugfs_init(struct amdgpu_device *adev)
> +{
> +#if defined(CONFIG_DEBUG_FS)
> +
> +       if (!adev->psp.securedisplay_context.securedisplay_initialized)
> +               return;
> +
> +       debugfs_create_file("securedisplay_test", S_IWUSR, adev_to_drm(adev)->primary->debugfs_root,
> +                               adev, &amdgpu_securedisplay_debugfs_ops);
> +#endif
> +}
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h
> new file mode 100644
> index 000000000000..983446c72520
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h
> @@ -0,0 +1,36 @@
> +/*
> + * Copyright 2020 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + *
> + */
> +#ifndef _AMDGPU_SECUREDISPLAY_H
> +#define _AMDGPU_SECUREDISPLAY_H
> +
> +#include "amdgpu.h"
> +#include "ta_secureDisplay_if.h"
> +
> +void amdgpu_securedisplay_debugfs_init(struct amdgpu_device *adev);
> +void psp_securedisplay_parse_resp_status(struct psp_context *psp,
> +               enum ta_securedisplay_status status);
> +void psp_prep_securedisplay_cmd_buf(struct psp_context *psp, struct securedisplay_cmd **cmd,
> +               enum ta_securedisplay_command command_id);
> +
> +#endif
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> index 0e43b46d3ab5..46449e70348b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
> @@ -122,6 +122,9 @@ struct ta_firmware_header_v1_0 {
>         uint32_t ta_dtm_ucode_version;
>         uint32_t ta_dtm_offset_bytes;
>         uint32_t ta_dtm_size_bytes;
> +       uint32_t ta_securedisplay_ucode_version;
> +       uint32_t ta_securedisplay_offset_bytes;
> +       uint32_t ta_securedisplay_size_bytes;
>  };
>
>  enum ta_fw_type {
> @@ -132,6 +135,7 @@ enum ta_fw_type {
>         TA_FW_TYPE_PSP_HDCP,
>         TA_FW_TYPE_PSP_DTM,
>         TA_FW_TYPE_PSP_RAP,
> +       TA_FW_TYPE_PSP_SECUREDISPLAY,
>  };
>
>  struct ta_fw_bin_desc {
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> index d7f92634eba2..4b1cc5e9ee92 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
> @@ -92,8 +92,6 @@ static int psp_v10_0_init_microcode(struct psp_context *psp)
>                         (uint8_t *)ta_hdr +
>                         le32_to_cpu(ta_hdr->header.ucode_array_offset_bytes);
>
> -               adev->psp.ta_fw_version = le32_to_cpu(ta_hdr->header.ucode_version);
> -
>                 adev->psp.ta_dtm_ucode_version =
>                         le32_to_cpu(ta_hdr->ta_dtm_ucode_version);
>                 adev->psp.ta_dtm_ucode_size =
> @@ -101,6 +99,16 @@ static int psp_v10_0_init_microcode(struct psp_context *psp)
>                 adev->psp.ta_dtm_start_addr =
>                         (uint8_t *)adev->psp.ta_hdcp_start_addr +
>                         le32_to_cpu(ta_hdr->ta_dtm_offset_bytes);
> +
> +               adev->psp.ta_securedisplay_ucode_version =
> +                       le32_to_cpu(ta_hdr->ta_securedisplay_ucode_version);
> +               adev->psp.ta_securedisplay_ucode_size =
> +                       le32_to_cpu(ta_hdr->ta_securedisplay_size_bytes);
> +               adev->psp.ta_securedisplay_start_addr =
> +                       (uint8_t *)adev->psp.ta_hdcp_start_addr +
> +                       le32_to_cpu(ta_hdr->ta_securedisplay_offset_bytes);
> +
> +               adev->psp.ta_fw_version = le32_to_cpu(ta_hdr->header.ucode_version);
>         }
>
>         return 0;
> --
> 2.17.1
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 2/2] drm/amdgpu: Add secure display TA interface
  2021-01-13  3:43 ` [PATCH 2/2] drm/amdgpu: Add secure display TA interface Jinzhou Su
  2021-01-13  5:45   ` Chen, Guchun
  2021-01-13 13:53   ` Alex Deucher
@ 2021-01-13 15:16   ` Wang, Kevin(Yang)
  2 siblings, 0 replies; 10+ messages in thread
From: Wang, Kevin(Yang) @ 2021-01-13 15:16 UTC (permalink / raw)
  To: Su, Jinzhou (Joe), amd-gfx; +Cc: Huang, Ray


[-- Attachment #1.1: Type: text/plain, Size: 27331 bytes --]

[AMD Official Use Only - Internal Distribution Only]



________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Jinzhou Su <Jinzhou.Su@amd.com>
Sent: Wednesday, January 13, 2021 11:43 AM
To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>; Huang, Ray <Ray.Huang@amd.com>
Subject: [PATCH 2/2] drm/amdgpu: Add secure display TA interface

Add interface to load, unload, invoke command for
secure display TA.

v2: Add debugfs interface for secure display TA

Signed-off-by: Jinzhou.Su <Jinzhou.Su@amd.com>
Reviewed-by: Huang Rui <ray.huang@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/Makefile           |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c   |   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c       | 195 ++++++++++++++++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h       |  17 ++
 .../gpu/drm/amd/amdgpu/amdgpu_securedisplay.c | 174 ++++++++++++++++
 .../gpu/drm/amd/amdgpu/amdgpu_securedisplay.h |  36 ++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h     |   4 +
 drivers/gpu/drm/amd/amdgpu/psp_v10_0.c        |  12 +-
 8 files changed, 440 insertions(+), 3 deletions(-)
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
 create mode 100644 drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h

diff --git a/drivers/gpu/drm/amd/amdgpu/Makefile b/drivers/gpu/drm/amd/amdgpu/Makefile
index c6262689e14e..e4bebbfa88af 100644
--- a/drivers/gpu/drm/amd/amdgpu/Makefile
+++ b/drivers/gpu/drm/amd/amdgpu/Makefile
@@ -56,7 +56,7 @@ amdgpu-y += amdgpu_device.o amdgpu_kms.o \
         amdgpu_gmc.o amdgpu_mmhub.o amdgpu_xgmi.o amdgpu_csa.o amdgpu_ras.o amdgpu_vm_cpu.o \
         amdgpu_vm_sdma.o amdgpu_discovery.o amdgpu_ras_eeprom.o amdgpu_nbio.o \
         amdgpu_umc.o smu_v11_0_i2c.o amdgpu_fru_eeprom.o amdgpu_rap.o \
-       amdgpu_fw_attestation.o
+       amdgpu_fw_attestation.o amdgpu_securedisplay.o

 amdgpu-$(CONFIG_PERF_EVENTS) += amdgpu_pmu.o

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 477bead4fab1..4c38c5771cbc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -35,6 +35,7 @@
 #include "amdgpu_dm_debugfs.h"
 #include "amdgpu_ras.h"
 #include "amdgpu_rap.h"
+#include "amdgpu_securedisplay.h"
 #include "amdgpu_fw_attestation.h"

 /**
@@ -1666,6 +1667,8 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev)

         amdgpu_rap_debugfs_init(adev);

+       amdgpu_securedisplay_debugfs_init(adev);
+
         amdgpu_fw_attestation_debugfs_init(adev);

         return amdgpu_debugfs_add_files(adev, amdgpu_debugfs_list,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index 523d22db094b..eb19ae734396 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -36,6 +36,7 @@
 #include "psp_v12_0.h"

 #include "amdgpu_ras.h"
+#include "amdgpu_securedisplay.h"

 static int psp_sysfs_init(struct amdgpu_device *adev);
 static void psp_sysfs_fini(struct amdgpu_device *adev);
@@ -1642,6 +1643,179 @@ int psp_rap_invoke(struct psp_context *psp, uint32_t ta_cmd_id)
 }
 // RAP end

+/* securedisplay start */
+static int psp_securedisplay_init_shared_buf(struct psp_context *psp)
+{
+       int ret;
+
+       /*
+        * Allocate 16k memory aligned to 4k from Frame Buffer (local
+        * physical) for sa ta <-> Driver
+        */
+       ret = amdgpu_bo_create_kernel(psp->adev, PSP_SECUREDISPLAY_SHARED_MEM_SIZE,
+                                     PAGE_SIZE, AMDGPU_GEM_DOMAIN_VRAM,
+                                     &psp->securedisplay_context.securedisplay_shared_bo,
+                                     &psp->securedisplay_context.securedisplay_shared_mc_addr,
+                                     &psp->securedisplay_context.securedisplay_shared_buf);
+
+       return ret;
+}
+
+static int psp_securedisplay_load(struct psp_context *psp)
+{
+       int ret;
+       struct psp_gfx_cmd_resp *cmd;
+
+       /*
+        * TODO: bypass the loading in sriov for now
+        */
+
+       cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
+       if (!cmd)
+               return -ENOMEM;
+
+       memset(psp->fw_pri_buf, 0, PSP_1_MEG);
+       memcpy(psp->fw_pri_buf, psp->ta_securedisplay_start_addr, psp->ta_securedisplay_ucode_size);
+
+       psp_prep_ta_load_cmd_buf(cmd,
+                                psp->fw_pri_mc_addr,
+                                psp->ta_securedisplay_ucode_size,
+                                psp->securedisplay_context.securedisplay_shared_mc_addr,
+                                PSP_SECUREDISPLAY_SHARED_MEM_SIZE);
+
+       ret = psp_cmd_submit_buf(psp, NULL, cmd, psp->fence_buf_mc_addr);
+
+       if (ret)
+               goto failed;
+
+       psp->securedisplay_context.securedisplay_initialized = true;
+       psp->securedisplay_context.session_id = cmd->resp.session_id;
+       mutex_init(&psp->securedisplay_context.mutex);
+
+failed:
+       kfree(cmd);
+       return ret;
+}
+
+static int psp_securedisplay_unload(struct psp_context *psp)
+{
+       int ret;
+       struct psp_gfx_cmd_resp *cmd;
+
+       cmd = kzalloc(sizeof(struct psp_gfx_cmd_resp), GFP_KERNEL);
+       if (!cmd)
+               return -ENOMEM;
+
+       psp_prep_ta_unload_cmd_buf(cmd, psp->securedisplay_context.session_id);
+
+       ret = psp_cmd_submit_buf(psp, NULL, cmd, psp->fence_buf_mc_addr);
+
+       kfree(cmd);
+
+       return ret;
+}
+
+static int psp_securedisplay_initialize(struct psp_context *psp)
+{
+       int ret;
+       struct securedisplay_cmd *securedisplay_cmd;
+
+       /*
+        * TODO: bypass the initialize in sriov for now
+        */
+       if (amdgpu_sriov_vf(psp->adev))
+               return 0;
+
+       if (!psp->adev->psp.ta_securedisplay_ucode_size ||
+           !psp->adev->psp.ta_securedisplay_start_addr) {
+               dev_info(psp->adev->dev, "SECUREDISPLAY: securedisplay ta ucode is not available\n");
+               return 0;
+       }
+
+       if (!psp->securedisplay_context.securedisplay_initialized) {
+               ret = psp_securedisplay_init_shared_buf(psp);
+               if (ret)
+                       return ret;
+       }
+
+       ret = psp_securedisplay_load(psp);
+       if (ret)
+               return ret;
+
+       psp_prep_securedisplay_cmd_buf(psp, &securedisplay_cmd,
+                       TA_SECUREDISPLAY_COMMAND__QUERY_TA);
+
+       ret = psp_securedisplay_invoke(psp, TA_SECUREDISPLAY_COMMAND__QUERY_TA);
+       if (ret) {
+               psp_securedisplay_unload(psp);
+
+               amdgpu_bo_free_kernel(&psp->securedisplay_context.securedisplay_shared_bo,
+                             &psp->securedisplay_context.securedisplay_shared_mc_addr,
+                             &psp->securedisplay_context.securedisplay_shared_buf);
+
+               psp->securedisplay_context.securedisplay_initialized = false;
+
+               dev_err(psp->adev->dev, "SECUREDISPLAY TA initialize fail.\n");
+               return -EINVAL;
+       }
+
+       if (securedisplay_cmd->status != TA_SECUREDISPLAY_STATUS__SUCCESS) {
+               psp_securedisplay_parse_resp_status(psp, securedisplay_cmd->status);
+               dev_err(psp->adev->dev, "SECUREDISPLAY: query securedisplay TA failed. ret 0x%x\n",
+                       securedisplay_cmd->securedisplay_out_message.query_ta.query_cmd_ret);
+       }
+
+       return 0;
+}
+
+static int psp_securedisplay_terminate(struct psp_context *psp)
+{
+       int ret;
+
+       /*
+        * TODO:bypass the terminate in sriov for now
+        */
+       if (amdgpu_sriov_vf(psp->adev))
+               return 0;
+
+       if (!psp->securedisplay_context.securedisplay_initialized)
+               return 0;
+
+       ret = psp_securedisplay_unload(psp);
+       if (ret)
+               return ret;
+
+       psp->securedisplay_context.securedisplay_initialized = false;
+
+       /* free securedisplay shared memory */
+       amdgpu_bo_free_kernel(&psp->securedisplay_context.securedisplay_shared_bo,
+                             &psp->securedisplay_context.securedisplay_shared_mc_addr,
+                             &psp->securedisplay_context.securedisplay_shared_buf);
+
+       return ret;
+}
+
+int psp_securedisplay_invoke(struct psp_context *psp, uint32_t ta_cmd_id)
+{
+       int ret;
+
+       if (!psp->securedisplay_context.securedisplay_initialized)
+               return -EINVAL;
+
+       if (ta_cmd_id != TA_SECUREDISPLAY_COMMAND__QUERY_TA &&
+           ta_cmd_id != TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC)
+               return -EINVAL;
+
+       mutex_lock(&psp->securedisplay_context.mutex);
+
+       ret = psp_ta_invoke(psp, ta_cmd_id, psp->securedisplay_context.session_id);
+
+       mutex_unlock(&psp->securedisplay_context.mutex);
+
+       return ret;
+}
+/* SECUREDISPLAY end */
+
 static int psp_hw_start(struct psp_context *psp)
 {
         struct amdgpu_device *adev = psp->adev;
@@ -2116,6 +2290,11 @@ static int psp_load_fw(struct amdgpu_device *adev)
                 if (ret)
                         dev_err(psp->adev->dev,
                                 "RAP: Failed to initialize RAP\n");
+
+               ret = psp_securedisplay_initialize(psp);
+               if (ret)
+                       dev_err(psp->adev->dev,
+                               "SECUREDISPLAY: Failed to initialize SECUREDISPLAY\n");
         }

         return 0;
@@ -2166,6 +2345,7 @@ static int psp_hw_fini(void *handle)

         if (psp->adev->psp.ta_fw) {
                 psp_ras_terminate(psp);
+               psp_securedisplay_terminate(psp);
                 psp_rap_terminate(psp);
                 psp_dtm_terminate(psp);
                 psp_hdcp_terminate(psp);
@@ -2230,6 +2410,11 @@ static int psp_suspend(void *handle)
                         DRM_ERROR("Failed to terminate rap ta\n");
                         return ret;
                 }
+               ret = psp_securedisplay_terminate(psp);
+               if (ret) {
+                       DRM_ERROR("Failed to terminate securedisplay ta\n");
+                       return ret;
+               }
         }

         ret = psp_asd_unload(psp);
@@ -2313,6 +2498,11 @@ static int psp_resume(void *handle)
                 if (ret)
                         dev_err(psp->adev->dev,
                                 "RAP: Failed to initialize RAP\n");
+
+               ret = psp_securedisplay_initialize(psp);
+               if (ret)
+                       dev_err(psp->adev->dev,
+                               "SECUREDISPLAY: Failed to initialize SECUREDISPLAY\n");
         }

         mutex_unlock(&adev->firmware.mutex);
@@ -2620,6 +2810,11 @@ static int parse_ta_bin_descriptor(struct psp_context *psp,
                 psp->ta_rap_ucode_size     = le32_to_cpu(desc->size_bytes);
                 psp->ta_rap_start_addr     = ucode_start_addr;
                 break;
+       case TA_FW_TYPE_PSP_SECUREDISPLAY:
+               psp->ta_securedisplay_ucode_version  = le32_to_cpu(desc->fw_version);
+               psp->ta_securedisplay_ucode_size     = le32_to_cpu(desc->size_bytes);
+               psp->ta_securedisplay_start_addr     = ucode_start_addr;
+               break;
         default:
                 dev_warn(psp->adev->dev, "Unsupported TA type: %d\n", desc->fw_type);
                 break;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index da250bc1ac57..cb50ba445f8c 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -30,6 +30,7 @@
 #include "ta_xgmi_if.h"
 #include "ta_ras_if.h"
 #include "ta_rap_if.h"
+#include "ta_secureDisplay_if.h"

 #define PSP_FENCE_BUFFER_SIZE   0x1000
 #define PSP_CMD_BUFFER_SIZE     0x1000
@@ -40,6 +41,7 @@
 #define PSP_HDCP_SHARED_MEM_SIZE        0x4000
 #define PSP_DTM_SHARED_MEM_SIZE 0x4000
 #define PSP_RAP_SHARED_MEM_SIZE 0x4000
+#define PSP_SECUREDISPLAY_SHARED_MEM_SIZE      0x4000
 #define PSP_SHARED_MEM_SIZE             0x4000
 #define PSP_FW_NAME_LEN         0x24

@@ -171,6 +173,15 @@ struct psp_rap_context {
         struct mutex            mutex;
 };

+struct psp_securedisplay_context {
+       bool                    securedisplay_initialized;
+       uint32_t                session_id;
+       struct amdgpu_bo        *securedisplay_shared_bo;
+       uint64_t                securedisplay_shared_mc_addr;
+       void                    *securedisplay_shared_buf;
+       struct mutex            mutex;
+};
+
 #define MEM_TRAIN_SYSTEM_SIGNATURE              0x54534942
 #define GDDR6_MEM_TRAINING_DATA_SIZE_IN_BYTES   0x1000
 #define GDDR6_MEM_TRAINING_OFFSET               0x8000
@@ -298,12 +309,17 @@ struct psp_context
         uint32_t                        ta_rap_ucode_size;
         uint8_t                         *ta_rap_start_addr;

+       uint32_t                        ta_securedisplay_ucode_version;
+       uint32_t                        ta_securedisplay_ucode_size;
+       uint8_t                         *ta_securedisplay_start_addr;
+
         struct psp_asd_context          asd_context;
         struct psp_xgmi_context         xgmi_context;
         struct psp_ras_context          ras;
         struct psp_hdcp_context  hdcp_context;
         struct psp_dtm_context          dtm_context;
         struct psp_rap_context          rap_context;
+       struct psp_securedisplay_context        securedisplay_context;
         struct mutex                    mutex;
         struct psp_memory_training_context mem_train_ctx;
 };
@@ -380,6 +396,7 @@ int psp_ras_trigger_error(struct psp_context *psp,
 int psp_hdcp_invoke(struct psp_context *psp, uint32_t ta_cmd_id);
 int psp_dtm_invoke(struct psp_context *psp, uint32_t ta_cmd_id);
 int psp_rap_invoke(struct psp_context *psp, uint32_t ta_cmd_id);
+int psp_securedisplay_invoke(struct psp_context *psp, uint32_t ta_cmd_id);

 int psp_rlc_autoload_start(struct psp_context *psp);

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
new file mode 100644
index 000000000000..455978781380
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.c
@@ -0,0 +1,174 @@
+/*
+ * Copyright 2020 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ *
+ */
+#include <linux/debugfs.h>
+#include <linux/pm_runtime.h>
+
+#include "amdgpu.h"
+#include "amdgpu_securedisplay.h"
+
+/**
+ * DOC: AMDGPU SECUREDISPLAY debugfs test interface
+ *
+ * how to use?
+ * echo opcode <value> > <debugfs_dir>/dri/xxx/securedisplay_test
+ * eg. echo 1 > <debugfs_dir>/dri/xxx/securedisplay_test
+ * eg. echo 2 phy_id > <debugfs_dir>/dri/xxx/securedisplay_test
+ *
+ * opcode:
+ * 1:Query whether TA is responding used only for validation pupose
+ * 2: Send region of Interest and CRC value to I2C. (uint32)phy_id is
+ * send to determine which DIO scratch register should be used to get
+ * ROI and receive i2c_buf as the output.
+ *
+ * You can refer more detail from header file ta_securedisplay_if.h
+ *
+ */
+
+void psp_securedisplay_parse_resp_status(struct psp_context *psp,
+       enum ta_securedisplay_status status)
+{
+       switch (status) {
+       case TA_SECUREDISPLAY_STATUS__SUCCESS:
+               break;
+       case TA_SECUREDISPLAY_STATUS__GENERIC_FAILURE:
+               dev_err(psp->adev->dev, "Secure display: Generic Failure.");
+               break;
+       case TA_SECUREDISPLAY_STATUS__INVALID_PARAMETER:
+               dev_err(psp->adev->dev, "Secure display: Invalid Parameter.");
+               break;
+       case TA_SECUREDISPLAY_STATUS__NULL_POINTER:
+               dev_err(psp->adev->dev, "Secure display: Null Pointer.");
+               break;
+       case TA_SECUREDISPLAY_STATUS__I2C_WRITE_ERROR:
+               dev_err(psp->adev->dev, "Secure display: Failed to write to I2C.");
+               break;
+       case TA_SECUREDISPLAY_STATUS__READ_DIO_SCRATCH_ERROR:
+               dev_err(psp->adev->dev, "Secure display: Failed to Read DIO Scratch Register.");
+               break;
+       case TA_SECUREDISPLAY_STATUS__READ_CRC_ERROR:
+               dev_err(psp->adev->dev, "Secure display: Failed to Read CRC");
+               break;
+       default:
+               dev_err(psp->adev->dev, "Secure display: Failed to parse status: %d\n", status);
+       }
+}
+
+void psp_prep_securedisplay_cmd_buf(struct psp_context *psp, struct securedisplay_cmd **cmd,
+       enum ta_securedisplay_command command_id)
+{
+       *cmd = (struct securedisplay_cmd *)psp->securedisplay_context.securedisplay_shared_buf;
+       memset(*cmd, 0, sizeof(struct securedisplay_cmd));
+       (*cmd)->status = TA_SECUREDISPLAY_STATUS__GENERIC_FAILURE;
+       (*cmd)->cmd_id = command_id;
+}
+
+static ssize_t amdgpu_securedisplay_debugfs_write(struct file *f, const char __user *buf,
+               size_t size, loff_t *pos)
+{
+       struct amdgpu_device *adev = (struct amdgpu_device *)file_inode(f)->i_private;
+       struct psp_context *psp = &adev->psp;
+       struct securedisplay_cmd *securedisplay_cmd;
+       struct drm_device *dev = adev_to_drm(adev);
+       uint32_t phy_id;
+       uint32_t op;
+       int i;
+       char str[64];
+       char i2c_output[256];
+       int ret;

[Kevin]:
the driver should avoid allocate big block size memory in kernel stack. (x86_64:: kernel stack size is fix16K).
In other words, you don't know how many kernel stacks will be used in upstream.
Generally speaking,  there is no problem, but it is really unreasonable.

Best Regards,
Kevin
+
+       if (*pos || size > sizeof(str) - 1)
+               return -EINVAL;
+
+       memset(str,  0, sizeof(str));
+       copy_from_user(str, buf, size);
+
+       ret = pm_runtime_get_sync(dev->dev);
+       if (ret < 0) {
+               pm_runtime_put_autosuspend(dev->dev);
+               return ret;
+       }
+
+       if (size < 3)
+               sscanf(str, "%u ", &op);
+       else
+               sscanf(str, "%u %u", &op, &phy_id);
+
+       switch (op) {
+       case 1:
+               psp_prep_securedisplay_cmd_buf(psp, &securedisplay_cmd,
+                       TA_SECUREDISPLAY_COMMAND__QUERY_TA);
+               ret = psp_securedisplay_invoke(psp, TA_SECUREDISPLAY_COMMAND__QUERY_TA);
+               if (!ret) {
+                       if (securedisplay_cmd->status == TA_SECUREDISPLAY_STATUS__SUCCESS)
+                               dev_info(adev->dev, "SECUREDISPLAY: query securedisplay TA ret is 0x%X\n",
+                                       securedisplay_cmd->securedisplay_out_message.query_ta.query_cmd_ret);
+                       else
+                               psp_securedisplay_parse_resp_status(psp, securedisplay_cmd->status);
+               }
+               break;
+       case 2:
+               psp_prep_securedisplay_cmd_buf(psp, &securedisplay_cmd,
+                       TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC);
+               securedisplay_cmd->securedisplay_in_message.send_roi_crc.phy_id = phy_id;
+               ret = psp_securedisplay_invoke(psp, TA_SECUREDISPLAY_COMMAND__SEND_ROI_CRC);
+               if (!ret) {
+                       if (securedisplay_cmd->status == TA_SECUREDISPLAY_STATUS__SUCCESS) {
+                               memset(i2c_output,  0, sizeof(i2c_output));
+                               for (i = 0; i < TA_SECUREDISPLAY_I2C_BUFFER_SIZE; i++)
+                                       sprintf(i2c_output, "%s 0x%X", i2c_output,
+                                               securedisplay_cmd->securedisplay_out_message.send_roi_crc.i2c_buf[i]);
+                               dev_info(adev->dev, "SECUREDISPLAY: I2C buffer out put is :%s\n", i2c_output);
+                       } else {
+                               psp_securedisplay_parse_resp_status(psp, securedisplay_cmd->status);
+                       }
+               }
+               break;
+       default:
+               dev_err(adev->dev, "Invalid input: %s\n", str);
+       }
+
+       pm_runtime_mark_last_busy(dev->dev);
+       pm_runtime_put_autosuspend(dev->dev);
+
+       return size;
+}
+
+static const struct file_operations amdgpu_securedisplay_debugfs_ops = {
+       .owner = THIS_MODULE,
+       .read = NULL,
+       .write = amdgpu_securedisplay_debugfs_write,
+       .llseek = default_llseek
+};
+
+void amdgpu_securedisplay_debugfs_init(struct amdgpu_device *adev)
+{
+#if defined(CONFIG_DEBUG_FS)
+
+       if (!adev->psp.securedisplay_context.securedisplay_initialized)
+               return;
+
+       debugfs_create_file("securedisplay_test", S_IWUSR, adev_to_drm(adev)->primary->debugfs_root,
+                               adev, &amdgpu_securedisplay_debugfs_ops);
+#endif
+}
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h
new file mode 100644
index 000000000000..983446c72520
--- /dev/null
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_securedisplay.h
@@ -0,0 +1,36 @@
+/*
+ * Copyright 2020 Advanced Micro Devices, Inc.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ *
+ */
+#ifndef _AMDGPU_SECUREDISPLAY_H
+#define _AMDGPU_SECUREDISPLAY_H
+
+#include "amdgpu.h"
+#include "ta_secureDisplay_if.h"
+
+void amdgpu_securedisplay_debugfs_init(struct amdgpu_device *adev);
+void psp_securedisplay_parse_resp_status(struct psp_context *psp,
+               enum ta_securedisplay_status status);
+void psp_prep_securedisplay_cmd_buf(struct psp_context *psp, struct securedisplay_cmd **cmd,
+               enum ta_securedisplay_command command_id);
+
+#endif
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
index 0e43b46d3ab5..46449e70348b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ucode.h
@@ -122,6 +122,9 @@ struct ta_firmware_header_v1_0 {
         uint32_t ta_dtm_ucode_version;
         uint32_t ta_dtm_offset_bytes;
         uint32_t ta_dtm_size_bytes;
+       uint32_t ta_securedisplay_ucode_version;
+       uint32_t ta_securedisplay_offset_bytes;
+       uint32_t ta_securedisplay_size_bytes;
 };

 enum ta_fw_type {
@@ -132,6 +135,7 @@ enum ta_fw_type {
         TA_FW_TYPE_PSP_HDCP,
         TA_FW_TYPE_PSP_DTM,
         TA_FW_TYPE_PSP_RAP,
+       TA_FW_TYPE_PSP_SECUREDISPLAY,
 };

 struct ta_fw_bin_desc {
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
index d7f92634eba2..4b1cc5e9ee92 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v10_0.c
@@ -92,8 +92,6 @@ static int psp_v10_0_init_microcode(struct psp_context *psp)
                         (uint8_t *)ta_hdr +
                         le32_to_cpu(ta_hdr->header.ucode_array_offset_bytes);

-               adev->psp.ta_fw_version = le32_to_cpu(ta_hdr->header.ucode_version);
-
                 adev->psp.ta_dtm_ucode_version =
                         le32_to_cpu(ta_hdr->ta_dtm_ucode_version);
                 adev->psp.ta_dtm_ucode_size =
@@ -101,6 +99,16 @@ static int psp_v10_0_init_microcode(struct psp_context *psp)
                 adev->psp.ta_dtm_start_addr =
                         (uint8_t *)adev->psp.ta_hdcp_start_addr +
                         le32_to_cpu(ta_hdr->ta_dtm_offset_bytes);
+
+               adev->psp.ta_securedisplay_ucode_version =
+                       le32_to_cpu(ta_hdr->ta_securedisplay_ucode_version);
+               adev->psp.ta_securedisplay_ucode_size =
+                       le32_to_cpu(ta_hdr->ta_securedisplay_size_bytes);
+               adev->psp.ta_securedisplay_start_addr =
+                       (uint8_t *)adev->psp.ta_hdcp_start_addr +
+                       le32_to_cpu(ta_hdr->ta_securedisplay_offset_bytes);
+
+               adev->psp.ta_fw_version = le32_to_cpu(ta_hdr->header.ucode_version);
         }

         return 0;
--
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=04%7C01%7CKevin1.Wang%40amd.com%7C105d330e2a9f417d324408d8b775887e%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637461062772245289%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=j3wrRxVVnVEu9UWN%2F1UiLZUHyO6jNy8tBEOlkq0DOGk%3D&amp;reserved=0

[-- Attachment #1.2: Type: text/html, Size: 54123 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file
  2021-01-13  3:43 [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file Jinzhou Su
  2021-01-13  3:43 ` [PATCH 2/2] drm/amdgpu: Add secure display TA interface Jinzhou Su
  2021-01-13  3:51 ` [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file Huang Rui
@ 2021-01-13 19:05 ` Paul Menzel
  2021-01-14  7:29   ` Su, Jinzhou (Joe)
  2 siblings, 1 reply; 10+ messages in thread
From: Paul Menzel @ 2021-01-13 19:05 UTC (permalink / raw)
  To: Jinzhou Su; +Cc: Ray Huang, amd-gfx

Dear Jinzhou,


Am 13.01.21 um 04:43 schrieb Jinzhou Su:
> Add file ta_secureDisplay_if.h for Secure Display TA

What is *Secure Display TA*? Please give some background, and even 
examples how it can be used.

How is the header file generated?

What do the comments mean, when they refer to “for validation only” or 
similar.

> Signed-off-by: Jinzhou Su <Jinzhou.Su@amd.com>
> Reviewed-by: Huang Rui <ray.huang@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>   .../gpu/drm/amd/amdgpu/ta_secureDisplay_if.h  | 154 ++++++++++++++++++
>   1 file changed, 154 insertions(+)
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h b/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
> new file mode 100644
> index 000000000000..5039375bb1d4
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h

Why is the header added in a separate commit, and not both commits squashed?

> @@ -0,0 +1,154 @@
> +/*
> + * Copyright 2019 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + */

Why not use SPDX headers?

> +
> +#ifndef _TA_SECUREDISPLAY_IF_H
> +#define _TA_SECUREDISPLAY_IF_H
> +
> +/** Secure Display related enumerations */
> +/**********************************************************/
> +
> +/** @enum ta_securedisplay_command
> + *    Secure Display Command ID
> + */
> +enum ta_securedisplay_command {
> +	/* Query whether TA is responding 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,
> +	/* Maximum Command ID */
> +	TA_SECUREDISPLAY_COMMAND__MAX_ID                = 0x7FFFFFFF,
> +};
> +
> +/** @enum ta_securedisplay_status
> + *    Secure Display status returns in shared buffer status
> + */
> +enum ta_securedisplay_status {
> +	TA_SECUREDISPLAY_STATUS__SUCCESS                 = 0x00,         /* Success */
> +	TA_SECUREDISPLAY_STATUS__GENERIC_FAILURE         = 0x01,         /* Generic Failure */
> +	TA_SECUREDISPLAY_STATUS__INVALID_PARAMETER       = 0x02,         /* Invalid Parameter */
> +	TA_SECUREDISPLAY_STATUS__NULL_POINTER            = 0x03,         /* Null Pointer*/
> +	TA_SECUREDISPLAY_STATUS__I2C_WRITE_ERROR         = 0x04,         /* Fail to Write to I2C */
> +	TA_SECUREDISPLAY_STATUS__READ_DIO_SCRATCH_ERROR  = 0x05, /*Fail Read DIO Scratch Register*/
> +	TA_SECUREDISPLAY_STATUS__READ_CRC_ERROR          = 0x06,         /* Fail to Read CRC*/
> +
> +	TA_SECUREDISPLAY_STATUS__MAX                     = 0x7FFFFFFF,/* Maximum Value for status*/
> +};
> +
> +/** @enum ta_securedisplay_max_phy
> + *    Physical ID number to use for reading corresponding DIO Scratch register for ROI
> + */
> +enum  ta_securedisplay_max_phy {
> +	TA_SECUREDISPLAY_PHY0                           = 0,
> +	TA_SECUREDISPLAY_PHY1                           = 1,
> +	TA_SECUREDISPLAY_PHY2                           = 2,
> +	TA_SECUREDISPLAY_PHY3                           = 3,
> +	TA_SECUREDISPLAY_MAX_PHY                        = 4,
> +};
> +
> +/** @enum ta_securedisplay_ta_query_cmd_ret
> + *    A predefined specific reteurn value which is 0xAB only used to validate

return

(A spell checker should have found this.)

> + *    communication to Secure Display TA is functional.
> + *    This value is used to validate whether TA is responding successfully
> + */
> +enum ta_securedisplay_ta_query_cmd_ret {
> +	/* This is a value to validate if TA is loaded successfully */

*the* value?

> +	TA_SECUREDISPLAY_QUERY_CMD_RET                 = 0xAB,
> +};
> +
> +/** @enum ta_securedisplay_buffer_size
> + *    I2C Buffer size which contains 8 bytes of ROI  (X start, X end, Y start, Y end)
> + *    and 6 bytes of CRC( R,G,B) and 1  byte for physical ID

Please fix the whitespace: one space, instead of two, and *CRC (R,G,B)*.

> + */
> +enum ta_securedisplay_buffer_size {
> +	/* 15 bytes = 8 byte (ROI) + 6 byte(CRC) + 1 byte(phy_id) */

Please add exactly one space before the (, where missing.

> +	TA_SECUREDISPLAY_I2C_BUFFER_SIZE                = 15,
> +};
> +
> +/** Input/output structures for Secure Display commands */
> +/**********************************************************/
> +/**
> + * Input structures
> + */
> +
> +/** @struct ta_securedisplay_send_roi_crc_input
> + *    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 */
> +};
> +
> +/** @union ta_securedisplay_cmd_input
> + *    Input buffer
> + */
> +union ta_securedisplay_cmd_input {
> +	/* send ROI and CRC input buffer format */
> +	struct ta_securedisplay_send_roi_crc_input        send_roi_crc;
> +	uint32_t                                          reserved[4];
> +};
> +
> +/**
> + * Output structures
> + */
> +
> +/** @struct ta_securedisplay_query_ta_output
> + *  Output buffer format for query TA whether TA is responding used only for validation purpose

Add period/dot at the end of sentence, and put the last sentence on a 
new line?

> + */
> +struct ta_securedisplay_query_ta_output {
> +	/* return value from TA when it is queried for validation purpose only */
> +	uint32_t  query_cmd_ret;
> +};
> +
> +/** @struct ta_securedisplay_send_roi_crc_output
> + *  Output buffer format for send ROI CRC command which will pass I2c buffer created inside TA
> + *  and used to write to I2C used only for validation purpose
> + */
> +struct ta_securedisplay_send_roi_crc_output {
> +	uint8_t  i2c_buf[TA_SECUREDISPLAY_I2C_BUFFER_SIZE];  /* I2C buffer */
> +	uint8_t  reserved;
> +};
> +
> +/** @union ta_securedisplay_cmd_output
> + *    Output buffer
> + */
> +union ta_securedisplay_cmd_output {
> +	/* Query TA output buffer format used only for validation purpose*/

Please add one space before `*/`.

> +	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;
> +	uint32_t                                           reserved[4];
> +};
> +
> +/** @struct securedisplay_cmd
> + *    Secure Display Command which is shared buffer memory
> + */
> +struct securedisplay_cmd {
> +	uint32_t                             cmd_id;                    /* +0  Bytes Command ID */
> +	enum ta_securedisplay_status         status;     /* +4  Bytes Status of Secure Display TA */
> +	uint32_t                             reserved[2];               /* +8  Bytes Reserved */
> +	union ta_securedisplay_cmd_input     securedisplay_in_message;  /* +16 Bytes Input Buffer */
> +	union ta_securedisplay_cmd_output    securedisplay_out_message;/* +32 Bytes Output 

Please align the comments, or just use one space before them.

Buffer */
> +	/**@note Total 48 Bytes */

The + are confusing. If I add the four numbers, I get 56 bytes.

I’d spell byets lower case.

> +};
> +
> +#endif   //_TA_SECUREDISPLAY_IF_H
> +
> 

Please remove blank lines at the end of the file.


Kind regards,

Paul
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file
  2021-01-13 19:05 ` Paul Menzel
@ 2021-01-14  7:29   ` Su, Jinzhou (Joe)
  2021-01-14 13:00     ` Paul Menzel
  0 siblings, 1 reply; 10+ messages in thread
From: Su, Jinzhou (Joe) @ 2021-01-14  7:29 UTC (permalink / raw)
  To: Paul Menzel; +Cc: Huang, Ray, amd-gfx list

[AMD Official Use Only - Internal Distribution Only]

Dear Paul,

Thanks so much for your review. Answer your Question inline. Please check.

Regards,
Joe

-----Original Message-----
From: Paul Menzel <pmenzel@molgen.mpg.de> 
Sent: Thursday, January 14, 2021 3:06 AM
To: Su, Jinzhou (Joe) <Jinzhou.Su@amd.com>
Cc: Huang, Ray <Ray.Huang@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file

Dear Jinzhou,


Am 13.01.21 um 04:43 schrieb Jinzhou Su:
> Add file ta_secureDisplay_if.h for Secure Display TA

What is *Secure Display TA*? Please give some background, and even examples how it can be used.

How is the header file generated?

Joe: Actually I got this from Display Team. Driver team's responsibility is to load the TA and add interface. 😊

What do the comments mean, when they refer to “for validation only” or similar.

> Signed-off-by: Jinzhou Su <Jinzhou.Su@amd.com>
> Reviewed-by: Huang Rui <ray.huang@amd.com>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> ---
>   .../gpu/drm/amd/amdgpu/ta_secureDisplay_if.h  | 154 ++++++++++++++++++
>   1 file changed, 154 insertions(+)
>   create mode 100644 drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h 
> b/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
> new file mode 100644
> index 000000000000..5039375bb1d4
> --- /dev/null
> +++ b/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h

Why is the header added in a separate commit, and not both commits squashed?

Joe: Header file need to do IP review.

> @@ -0,0 +1,154 @@
> +/*
> + * Copyright 2019 Advanced Micro Devices, Inc.
> + *
> + * Permission is hereby granted, free of charge, to any person 
> +obtaining a
> + * copy of this software and associated documentation files (the 
> +"Software"),
> + * to deal in the Software without restriction, including without 
> +limitation
> + * the rights to use, copy, modify, merge, publish, distribute, 
> +sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom 
> +the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice shall be 
> +included in
> + * all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, 
> +EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF 
> +MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT 
> +SHALL
> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, 
> +DAMAGES OR
> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR 
> +OTHERWISE,
> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE 
> +OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + *
> + */

Why not use SPDX headers?

Joe: sorry, I don't know.
> +
> +#ifndef _TA_SECUREDISPLAY_IF_H
> +#define _TA_SECUREDISPLAY_IF_H
> +
> +/** Secure Display related enumerations */ 
> +/**********************************************************/
> +
> +/** @enum ta_securedisplay_command
> + *    Secure Display Command ID
> + */
> +enum ta_securedisplay_command {
> +	/* Query whether TA is responding 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,
> +	/* Maximum Command ID */
> +	TA_SECUREDISPLAY_COMMAND__MAX_ID                = 0x7FFFFFFF,
> +};
> +
> +/** @enum ta_securedisplay_status
> + *    Secure Display status returns in shared buffer status
> + */
> +enum ta_securedisplay_status {
> +	TA_SECUREDISPLAY_STATUS__SUCCESS                 = 0x00,         /* Success */
> +	TA_SECUREDISPLAY_STATUS__GENERIC_FAILURE         = 0x01,         /* Generic Failure */
> +	TA_SECUREDISPLAY_STATUS__INVALID_PARAMETER       = 0x02,         /* Invalid Parameter */
> +	TA_SECUREDISPLAY_STATUS__NULL_POINTER            = 0x03,         /* Null Pointer*/
> +	TA_SECUREDISPLAY_STATUS__I2C_WRITE_ERROR         = 0x04,         /* Fail to Write to I2C */
> +	TA_SECUREDISPLAY_STATUS__READ_DIO_SCRATCH_ERROR  = 0x05, /*Fail Read DIO Scratch Register*/
> +	TA_SECUREDISPLAY_STATUS__READ_CRC_ERROR          = 0x06,         /* Fail to Read CRC*/
> +
> +	TA_SECUREDISPLAY_STATUS__MAX                     = 0x7FFFFFFF,/* Maximum Value for status*/
> +};
> +
> +/** @enum ta_securedisplay_max_phy
> + *    Physical ID number to use for reading corresponding DIO Scratch register for ROI
> + */
> +enum  ta_securedisplay_max_phy {
> +	TA_SECUREDISPLAY_PHY0                           = 0,
> +	TA_SECUREDISPLAY_PHY1                           = 1,
> +	TA_SECUREDISPLAY_PHY2                           = 2,
> +	TA_SECUREDISPLAY_PHY3                           = 3,
> +	TA_SECUREDISPLAY_MAX_PHY                        = 4,
> +};
> +
> +/** @enum ta_securedisplay_ta_query_cmd_ret
> + *    A predefined specific reteurn value which is 0xAB only used to validate

return

(A spell checker should have found this.)

Joe: Sure.

> + *    communication to Secure Display TA is functional.
> + *    This value is used to validate whether TA is responding successfully
> + */
> +enum ta_securedisplay_ta_query_cmd_ret {
> +	/* This is a value to validate if TA is loaded successfully */

*the* value?

> +	TA_SECUREDISPLAY_QUERY_CMD_RET                 = 0xAB,
> +};
> +
> +/** @enum ta_securedisplay_buffer_size
> + *    I2C Buffer size which contains 8 bytes of ROI  (X start, X end, Y start, Y end)
> + *    and 6 bytes of CRC( R,G,B) and 1  byte for physical ID

Please fix the whitespace: one space, instead of two, and *CRC (R,G,B)*.

Joe: OK.

> + */
> +enum ta_securedisplay_buffer_size {
> +	/* 15 bytes = 8 byte (ROI) + 6 byte(CRC) + 1 byte(phy_id) */

Please add exactly one space before the (, where missing.

> +	TA_SECUREDISPLAY_I2C_BUFFER_SIZE                = 15,
> +};
> +
> +/** Input/output structures for Secure Display commands */ 
> +/**********************************************************/
> +/**
> + * Input structures
> + */
> +
> +/** @struct ta_securedisplay_send_roi_crc_input
> + *    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 */ };
> +
> +/** @union ta_securedisplay_cmd_input
> + *    Input buffer
> + */
> +union ta_securedisplay_cmd_input {
> +	/* send ROI and CRC input buffer format */
> +	struct ta_securedisplay_send_roi_crc_input        send_roi_crc;
> +	uint32_t                                          reserved[4];
> +};
> +
> +/**
> + * Output structures
> + */
> +
> +/** @struct ta_securedisplay_query_ta_output
> + *  Output buffer format for query TA whether TA is responding used 
> +only for validation purpose

Add period/dot at the end of sentence, and put the last sentence on a new line?

Joe: OK.

> + */
> +struct ta_securedisplay_query_ta_output {
> +	/* return value from TA when it is queried for validation purpose only */
> +	uint32_t  query_cmd_ret;
> +};
> +
> +/** @struct ta_securedisplay_send_roi_crc_output
> + *  Output buffer format for send ROI CRC command which will pass I2c 
> +buffer created inside TA
> + *  and used to write to I2C used only for validation purpose  */ 
> +struct ta_securedisplay_send_roi_crc_output {
> +	uint8_t  i2c_buf[TA_SECUREDISPLAY_I2C_BUFFER_SIZE];  /* I2C buffer */
> +	uint8_t  reserved;
> +};
> +
> +/** @union ta_securedisplay_cmd_output
> + *    Output buffer
> + */
> +union ta_securedisplay_cmd_output {
> +	/* Query TA output buffer format used only for validation purpose*/

Please add one space before `*/`.

Joe: Sure.

> +	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;
> +	uint32_t                                           reserved[4];
> +};
> +
> +/** @struct securedisplay_cmd
> + *    Secure Display Command which is shared buffer memory
> + */
> +struct securedisplay_cmd {
> +	uint32_t                             cmd_id;                    /* +0  Bytes Command ID */
> +	enum ta_securedisplay_status         status;     /* +4  Bytes Status of Secure Display TA */
> +	uint32_t                             reserved[2];               /* +8  Bytes Reserved */
> +	union ta_securedisplay_cmd_input     securedisplay_in_message;  /* +16 Bytes Input Buffer */
> +	union ta_securedisplay_cmd_output    securedisplay_out_message;/* +32 Bytes Output 

Please align the comments, or just use one space before them.

Joe: OK

Buffer */
> +	/**@note Total 48 Bytes */

The + are confusing. If I add the four numbers, I get 56 bytes.

I’d spell byets lower case.

> +};
> +
> +#endif   //_TA_SECUREDISPLAY_IF_H
> +
> 

Please remove blank lines at the end of the file.

Joe: OK

Kind regards,

Paul
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file
  2021-01-14  7:29   ` Su, Jinzhou (Joe)
@ 2021-01-14 13:00     ` Paul Menzel
  0 siblings, 0 replies; 10+ messages in thread
From: Paul Menzel @ 2021-01-14 13:00 UTC (permalink / raw)
  To: Jinzhou Su; +Cc: Ray Huang, amd-gfx


Dear Jinzhou,


Am 14.01.21 um 08:29 schrieb Su, Jinzhou (Joe):

> Thanks so much for your review. Answer your Question inline. Please check.

An email client, following standards when responding and citing messages 
correctly, would be nice.

> -----Original Message-----
> From: Paul Menzel <pmenzel@molgen.mpg.de>
> Sent: Thursday, January 14, 2021 3:06 AM

> Am 13.01.21 um 04:43 schrieb Jinzhou Su:
>> Add file ta_secureDisplay_if.h for Secure Display TA
> 
> What is *Secure Display TA*? Please give some background, and even examples how it can be used.
> 
> How is the header file generated?
> 
> Joe: Actually I got this from Display Team. Driver team's
> responsibility is to load the TA and add interface. 😊

Then please mention that in the commit message.

> What do the comments mean, when they refer to “for validation only” or similar.
> 
>> Signed-off-by: Jinzhou Su <Jinzhou.Su@amd.com>
>> Reviewed-by: Huang Rui <ray.huang@amd.com>
>> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>> ---
>>    .../gpu/drm/amd/amdgpu/ta_secureDisplay_if.h  | 154 ++++++++++++++++++
>>    1 file changed, 154 insertions(+)
>>    create mode 100644 drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
>> b/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
>> new file mode 100644
>> index 000000000000..5039375bb1d4
>> --- /dev/null
>> +++ b/drivers/gpu/drm/amd/amdgpu/ta_secureDisplay_if.h
> 
> Why is the header added in a separate commit, and not both commits squashed?
> 
> Joe: Header file need to do IP review.

Sorry, I do not understand the answer. But leave it a separate commit to 
adhere to your internal processes – it’s not that important.

>> @@ -0,0 +1,154 @@
>> +/*
>> + * Copyright 2019 Advanced Micro Devices, Inc.
>> + *
>> + * Permission is hereby granted, free of charge, to any person
>> +obtaining a
>> + * copy of this software and associated documentation files (the
>> +"Software"),
>> + * to deal in the Software without restriction, including without
>> +limitation
>> + * the rights to use, copy, modify, merge, publish, distribute,
>> +sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom
>> +the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice shall be
>> +included in
>> + * all copies or substantial portions of the Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
>> +EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
>> +MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT
>> +SHALL
>> + * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM,
>> +DAMAGES OR
>> + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR
>> +OTHERWISE,
>> + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE
>> +OR
>> + * OTHER DEALINGS IN THE SOFTWARE.
>> + *
>> + */
> 
> Why not use SPDX headers?
> 
> Joe: sorry, I don't know.

Please find out? ;-)

[…]


Kind regards,

Paul
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-01-14 13:00 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-13  3:43 [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file Jinzhou Su
2021-01-13  3:43 ` [PATCH 2/2] drm/amdgpu: Add secure display TA interface Jinzhou Su
2021-01-13  5:45   ` Chen, Guchun
2021-01-13 13:53   ` Alex Deucher
2021-01-13 15:16   ` Wang, Kevin(Yang)
2021-01-13  3:51 ` [PATCH 1/2] drm/amdgpu: Add Secure Display TA header file Huang Rui
2021-01-13  5:33   ` Chen, Guchun
2021-01-13 19:05 ` Paul Menzel
2021-01-14  7:29   ` Su, Jinzhou (Joe)
2021-01-14 13:00     ` Paul Menzel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.