[PATCH 0/5] HDCP SRM interface

[PATCH 0/5] HDCP SRM interface

[Bhawanpreet Lakha]

These patches adds support for SRM loading. By providing an interface to the
usermode

SRM has to be persistent and since the kernel cannot directly write to system
storage we need to provide an interface so that the usermode can do it for us


Bhawanpreet Lakha (5):
  drm/amd/display: Pass amdgpu_device instead of psp_context
  drm/amd/display: update psp interface header
  drm/amd/display: Add sysfs interface for set/get srm
  drm/amd/display: Load srm before enabling HDCP
  drm/amd/display: call psp set/get interfaces

 .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   2 +-
 .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c    | 182 +++++++++++++++++-
 .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.h    |   9 +-
 .../drm/amd/display/modules/hdcp/hdcp_psp.h   |  26 ++-
 4 files changed, 212 insertions(+), 7 deletions(-)

-- 
2.17.1

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

[PATCH 1/5] drm/amd/display: Pass amdgpu_device instead of psp_context

[Bhawanpreet Lakha]

[Why]
We need this to create sysfs (followup patch)

[How]
Change the parameter

Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c      | 2 +-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c | 4 ++--
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h | 3 ++-
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 48ad49c9a33c..db2404a9e2f1 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -960,7 +960,7 @@ static int amdgpu_dm_init(struct amdgpu_device *adev)
 
 #ifdef CONFIG_DRM_AMD_DC_HDCP
 	if (adev->asic_type >= CHIP_RAVEN) {
-		adev->dm.hdcp_workqueue = hdcp_create_workqueue(&adev->psp, &init_params.cp_psp, adev->dm.dc);
+		adev->dm.hdcp_workqueue = hdcp_create_workqueue(adev, &init_params.cp_psp, adev->dm.dc);
 
 		if (!adev->dm.hdcp_workqueue)
 			DRM_ERROR("amdgpu: failed to initialize hdcp_workqueue.\n");
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
index 0acd3409dd6c..a269916f7dd6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
@@ -338,7 +338,7 @@ static void update_config(void *handle, struct cp_psp_stream_config *config)
 	hdcp_update_display(hdcp_work, link_index, aconnector, DRM_MODE_HDCP_CONTENT_TYPE0, false);
 }
 
-struct hdcp_workqueue *hdcp_create_workqueue(void *psp_context, struct cp_psp *cp_psp, struct dc *dc)
+struct hdcp_workqueue *hdcp_create_workqueue(struct amdgpu_device *adev, struct cp_psp *cp_psp, struct dc *dc)
 {
 
 	int max_caps = dc->caps.max_links;
@@ -360,7 +360,7 @@ struct hdcp_workqueue *hdcp_create_workqueue(void *psp_context, struct cp_psp *c
 		INIT_DELAYED_WORK(&hdcp_work[i].watchdog_timer_dwork, event_watchdog_timer);
 		INIT_DELAYED_WORK(&hdcp_work[i].property_validate_dwork, event_property_validate);
 
-		hdcp_work[i].hdcp.config.psp.handle =  psp_context;
+		hdcp_work[i].hdcp.config.psp.handle = &adev->psp;
 		hdcp_work[i].hdcp.config.ddc.handle = dc_get_link_at_index(dc, i);
 		hdcp_work[i].hdcp.config.ddc.funcs.write_i2c = lp_write_i2c;
 		hdcp_work[i].hdcp.config.ddc.funcs.read_i2c = lp_read_i2c;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
index 6abde86bce4a..331b50825510 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
@@ -30,6 +30,7 @@
 #include "hdcp.h"
 #include "dc.h"
 #include "dm_cp_psp.h"
+#include "amdgpu.h"
 
 struct mod_hdcp;
 struct mod_hdcp_link;
@@ -64,6 +65,6 @@ void hdcp_reset_display(struct hdcp_workqueue *work, unsigned int link_index);
 void hdcp_handle_cpirq(struct hdcp_workqueue *work, unsigned int link_index);
 void hdcp_destroy(struct hdcp_workqueue *work);
 
-struct hdcp_workqueue *hdcp_create_workqueue(void *psp_context, struct cp_psp *cp_psp, struct dc *dc);
+struct hdcp_workqueue *hdcp_create_workqueue(struct amdgpu_device *adev, struct cp_psp *cp_psp, struct dc *dc);
 
 #endif /* AMDGPU_DM_AMDGPU_DM_HDCP_H_ */
-- 
2.17.1

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

[PATCH 2/5] drm/amd/display: update psp interface header

[Bhawanpreet Lakha]

[Why]
We need to support SRM

[How]
Add the interface to the header file

Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
---
 .../drm/amd/display/modules/hdcp/hdcp_psp.h   | 26 ++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.h b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.h
index 82a5e997d573..d5cb3f46606f 100644
--- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.h
+++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.h
@@ -117,6 +117,8 @@ struct ta_dtm_shared_memory {
 int psp_cmd_submit_buf(struct psp_context *psp, struct amdgpu_firmware_info *ucode, struct psp_gfx_cmd_resp *cmd,
 		uint64_t fence_mc_addr);
 
+enum { PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE = 5120 };
+
 enum ta_hdcp_command {
 	TA_HDCP_COMMAND__INITIALIZE,
 	TA_HDCP_COMMAND__HDCP1_CREATE_SESSION,
@@ -134,7 +136,10 @@ enum ta_hdcp_command {
 	TA_HDCP_COMMAND__UNUSED_3,
 	TA_HDCP_COMMAND__HDCP2_CREATE_SESSION_V2,
 	TA_HDCP_COMMAND__HDCP2_PREPARE_PROCESS_AUTHENTICATION_MSG_V2,
-	TA_HDCP_COMMAND__HDCP2_ENABLE_DP_STREAM_ENCRYPTION
+	TA_HDCP_COMMAND__HDCP2_ENABLE_DP_STREAM_ENCRYPTION,
+	TA_HDCP_COMMAND__HDCP_DESTROY_ALL_SESSIONS,
+	TA_HDCP_COMMAND__HDCP_SET_SRM,
+	TA_HDCP_COMMAND__HDCP_GET_SRM
 };
 
 enum ta_hdcp2_msg_id {
@@ -415,6 +420,22 @@ struct ta_hdcp_cmd_hdcp2_enable_dp_stream_encryption_input {
 	uint32_t display_handle;
 };
 
+struct ta_hdcp_cmd_set_srm_input {
+	uint32_t srm_buf_size;
+	uint8_t srm_buf[PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE];
+};
+
+struct ta_hdcp_cmd_set_srm_output {
+	uint8_t valid_signature;
+	uint32_t srm_version;
+};
+
+struct ta_hdcp_cmd_get_srm_output {
+	uint32_t srm_version;
+	uint32_t srm_buf_size;
+	uint8_t srm_buf[PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE];
+};
+
 /**********************************************************/
 /* Common input structure for HDCP callbacks */
 union ta_hdcp_cmd_input {
@@ -432,6 +453,7 @@ union ta_hdcp_cmd_input {
 	struct ta_hdcp_cmd_hdcp2_process_prepare_authentication_message_input_v2
 		hdcp2_prepare_process_authentication_message_v2;
 	struct ta_hdcp_cmd_hdcp2_enable_dp_stream_encryption_input hdcp2_enable_dp_stream_encryption;
+	struct ta_hdcp_cmd_set_srm_input hdcp_set_srm;
 };
 
 /* Common output structure for HDCP callbacks */
@@ -444,6 +466,8 @@ union ta_hdcp_cmd_output {
 	struct ta_hdcp_cmd_hdcp2_create_session_output_v2 hdcp2_create_session_v2;
 	struct ta_hdcp_cmd_hdcp2_process_prepare_authentication_message_output_v2
 		hdcp2_prepare_process_authentication_message_v2;
+	struct ta_hdcp_cmd_set_srm_output hdcp_set_srm;
+	struct ta_hdcp_cmd_get_srm_output hdcp_get_srm;
 };
 /**********************************************************/
 
-- 
2.17.1

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

Re: [PATCH 2/5] drm/amd/display: update psp interface header

[Harry Wentland]

On 2020-01-16 3:29 p.m., Bhawanpreet Lakha wrote:
> [Why]
> We need to support SRM

Why do we need to support SRM?

Please refer to section 5 "Renewability" of the HDCP 2.x spec and its
requirement to store the SRM in non-volatile memory.

Why does this involve PSP?

Describe that for AMD's content protection solution PSP owns the SRM
check but has no ability to store it in SRM memory directly, hence the
need for the kernel driver to facilitate it.

Why a sysfs?

Describe that the kernel driver cannot (or should not) write to the file
system. Outline how we expect usermode scripts to facilitate this and
use the sysfs to retrieve an SRM from PSP to store it to the filesystem
and passes it to PSP from the filesystem. We'll want to highlight at
which points (boot, suspend, etc.) we expect this to happen.

I recommend documenting this whole process here in the patch description
to provide a reference for posterity as it's not necessarily clear from
the patches why and what we're doing.

Harry

> 
> [How]
> Add the interface to the header file
> 
> Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
> Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> ---
>  .../drm/amd/display/modules/hdcp/hdcp_psp.h   | 26 ++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.h b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.h
> index 82a5e997d573..d5cb3f46606f 100644
> --- a/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.h
> +++ b/drivers/gpu/drm/amd/display/modules/hdcp/hdcp_psp.h
> @@ -117,6 +117,8 @@ struct ta_dtm_shared_memory {
>  int psp_cmd_submit_buf(struct psp_context *psp, struct amdgpu_firmware_info *ucode, struct psp_gfx_cmd_resp *cmd,
>  		uint64_t fence_mc_addr);
>  
> +enum { PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE = 5120 };
> +
>  enum ta_hdcp_command {
>  	TA_HDCP_COMMAND__INITIALIZE,
>  	TA_HDCP_COMMAND__HDCP1_CREATE_SESSION,
> @@ -134,7 +136,10 @@ enum ta_hdcp_command {
>  	TA_HDCP_COMMAND__UNUSED_3,
>  	TA_HDCP_COMMAND__HDCP2_CREATE_SESSION_V2,
>  	TA_HDCP_COMMAND__HDCP2_PREPARE_PROCESS_AUTHENTICATION_MSG_V2,
> -	TA_HDCP_COMMAND__HDCP2_ENABLE_DP_STREAM_ENCRYPTION
> +	TA_HDCP_COMMAND__HDCP2_ENABLE_DP_STREAM_ENCRYPTION,
> +	TA_HDCP_COMMAND__HDCP_DESTROY_ALL_SESSIONS,
> +	TA_HDCP_COMMAND__HDCP_SET_SRM,
> +	TA_HDCP_COMMAND__HDCP_GET_SRM
>  };
>  
>  enum ta_hdcp2_msg_id {
> @@ -415,6 +420,22 @@ struct ta_hdcp_cmd_hdcp2_enable_dp_stream_encryption_input {
>  	uint32_t display_handle;
>  };
>  
> +struct ta_hdcp_cmd_set_srm_input {
> +	uint32_t srm_buf_size;
> +	uint8_t srm_buf[PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE];
> +};
> +
> +struct ta_hdcp_cmd_set_srm_output {
> +	uint8_t valid_signature;
> +	uint32_t srm_version;
> +};
> +
> +struct ta_hdcp_cmd_get_srm_output {
> +	uint32_t srm_version;
> +	uint32_t srm_buf_size;
> +	uint8_t srm_buf[PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE];
> +};
> +
>  /**********************************************************/
>  /* Common input structure for HDCP callbacks */
>  union ta_hdcp_cmd_input {
> @@ -432,6 +453,7 @@ union ta_hdcp_cmd_input {
>  	struct ta_hdcp_cmd_hdcp2_process_prepare_authentication_message_input_v2
>  		hdcp2_prepare_process_authentication_message_v2;
>  	struct ta_hdcp_cmd_hdcp2_enable_dp_stream_encryption_input hdcp2_enable_dp_stream_encryption;
> +	struct ta_hdcp_cmd_set_srm_input hdcp_set_srm;
>  };
>  
>  /* Common output structure for HDCP callbacks */
> @@ -444,6 +466,8 @@ union ta_hdcp_cmd_output {
>  	struct ta_hdcp_cmd_hdcp2_create_session_output_v2 hdcp2_create_session_v2;
>  	struct ta_hdcp_cmd_hdcp2_process_prepare_authentication_message_output_v2
>  		hdcp2_prepare_process_authentication_message_v2;
> +	struct ta_hdcp_cmd_set_srm_output hdcp_set_srm;
> +	struct ta_hdcp_cmd_get_srm_output hdcp_get_srm;
>  };
>  /**********************************************************/
>  
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 3/5] drm/amd/display: Add sysfs interface for set/get srm

[Bhawanpreet Lakha]

[Why]
We need to set/get SRM and linux kernel is not suppose to write to the
storage, so we need to provide a interface.

[How]
Provide interface so usermode can set/get srm

Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c    | 124 +++++++++++++++++-
 .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.h    |   6 +
 2 files changed, 128 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
index a269916f7dd6..a191c84ad8eb 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
@@ -28,6 +28,8 @@
 #include "amdgpu_dm.h"
 #include "dm_helpers.h"
 #include <drm/drm_hdcp.h>
+#include "hdcp_psp.h"
+
 
 static bool
 lp_write_i2c(void *handle, uint32_t address, const uint8_t *data, uint32_t size)
@@ -67,6 +69,16 @@ lp_read_dpcd(void *handle, uint32_t address, uint8_t *data, uint32_t size)
 	return dm_helpers_dp_read_dpcd(link->ctx, link, address, data, size);
 }
 
+static uint8_t *psp_get_srm(struct psp_context *psp, uint32_t *srm_version, uint32_t *srm_size)
+{
+	return NULL;
+}
+
+static int psp_set_srm(struct psp_context *psp, uint8_t *srm, uint32_t srm_size, uint32_t *srm_version)
+{
+	return 0;
+}
+
 static void process_output(struct hdcp_workqueue *hdcp_work)
 {
 	struct mod_hdcp_output output = hdcp_work->output;
@@ -88,6 +100,18 @@ static void process_output(struct hdcp_workqueue *hdcp_work)
 	schedule_delayed_work(&hdcp_work->property_validate_dwork, msecs_to_jiffies(0));
 }
 
+static void link_lock(struct hdcp_workqueue *work, bool lock)
+{
+
+	int i = 0;
+
+	for (i = 0; i < work->max_link; i++) {
+		if (lock)
+			mutex_lock(&work[i].mutex);
+		else
+			mutex_unlock(&work[i].mutex);
+	}
+}
 void hdcp_update_display(struct hdcp_workqueue *hdcp_work,
 			 unsigned int link_index,
 			 struct amdgpu_dm_connector *aconnector,
@@ -302,7 +326,8 @@ void hdcp_destroy(struct hdcp_workqueue *hdcp_work)
 	}
 
 	kfree(hdcp_work);
-
+	kfree(hdcp_work->srm);
+	kfree(hdcp_work->srm_temp);
 }
 
 static void update_config(void *handle, struct cp_psp_stream_config *config)
@@ -338,6 +363,84 @@ static void update_config(void *handle, struct cp_psp_stream_config *config)
 	hdcp_update_display(hdcp_work, link_index, aconnector, DRM_MODE_HDCP_CONTENT_TYPE0, false);
 }
 
+
+/*
+ * This can be called twice, because SRM_SIZE > PAGE_SIZE.
+ *
+ * We set the SRM on each call, if SRM_SIZE > PAGE_SIZE, PSP will fail on the
+ * first call but pass on the second call.
+ *
+ * Because of this we are not throwing any errors as it will stop the next call.
+ * So it is a good idea to call the "read" sysfs to verify that the SRM was set
+ *
+ */
+static ssize_t srm_data_write(struct file *filp, struct kobject *kobj, struct bin_attribute *bin_attr, char *buffer,
+			      loff_t pos, size_t count)
+{
+	struct hdcp_workqueue *work;
+	uint32_t srm_version = 0;
+
+	work = container_of(bin_attr, struct hdcp_workqueue, attr);
+	link_lock(work, true);
+
+	memcpy(work->srm_temp + pos, buffer, count);
+
+	if (!psp_set_srm(work->hdcp.config.psp.handle, work->srm_temp, pos + count, &srm_version)) {
+		DRM_DEBUG_DRIVER("HDCP SRM SET version 0x%X", srm_version);
+		memcpy(work->srm, work->srm_temp, pos + count);
+		work->srm_size = pos + count;
+		work->srm_version = srm_version;
+	}
+
+
+	link_lock(work, false);
+
+	return count;
+}
+
+static ssize_t srm_data_read(struct file *filp, struct kobject *kobj, struct bin_attribute *bin_attr, char *buffer,
+			     loff_t pos, size_t count)
+{
+	struct hdcp_workqueue *work;
+	uint8_t *srm = NULL;
+	uint32_t srm_version;
+	uint32_t srm_size;
+	size_t ret = count;
+
+	work = container_of(bin_attr, struct hdcp_workqueue, attr);
+
+	link_lock(work, true);
+
+	srm = psp_get_srm(work->hdcp.config.psp.handle, &srm_version, &srm_size);
+
+	if (!srm)
+		return -EINVAL;
+
+	if (pos >= srm_size)
+		ret = 0;
+
+	if (srm_size - pos < count) {
+		memcpy(buffer, srm + pos, srm_size - pos);
+		ret = srm_size - pos;
+		goto ret;
+	}
+
+	memcpy(buffer, srm + pos, count);
+
+ret:
+	link_lock(work, false);
+	return ret;
+}
+
+
+static const struct bin_attribute data_attr = {
+	.attr = {.name = "hdcp_srm", .mode = 0664},
+	.size = PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE, /* Limit SRM size */
+	.write = srm_data_write,
+	.read = srm_data_read,
+};
+
+
 struct hdcp_workqueue *hdcp_create_workqueue(struct amdgpu_device *adev, struct cp_psp *cp_psp, struct dc *dc)
 {
 
@@ -348,10 +451,19 @@ struct hdcp_workqueue *hdcp_create_workqueue(struct amdgpu_device *adev, struct
 	if (hdcp_work == NULL)
 		goto fail_alloc_context;
 
+	hdcp_work->srm = kcalloc(PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE, sizeof(*hdcp_work->srm), GFP_KERNEL);
+
+	if (hdcp_work->srm == NULL)
+		goto fail_alloc_context;
+
+	hdcp_work->srm_temp = kcalloc(PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE, sizeof(*hdcp_work->srm_temp), GFP_KERNEL);
+
+	if (hdcp_work->srm_temp == NULL)
+		goto fail_alloc_context;
+
 	hdcp_work->max_link = max_caps;
 
 	for (i = 0; i < max_caps; i++) {
-
 		mutex_init(&hdcp_work[i].mutex);
 
 		INIT_WORK(&hdcp_work[i].cpirq_work, event_cpirq);
@@ -371,10 +483,18 @@ struct hdcp_workqueue *hdcp_create_workqueue(struct amdgpu_device *adev, struct
 	cp_psp->funcs.update_stream_config = update_config;
 	cp_psp->handle = hdcp_work;
 
+	/* File created at /sys/class/drm/card0/device/hdcp_srm*/
+	hdcp_work[0].attr = data_attr;
+
+	if (sysfs_create_bin_file(&adev->dev->kobj, &hdcp_work[0].attr))
+		DRM_WARN("Failed to create device file hdcp_srm");
+
 	return hdcp_work;
 
 fail_alloc_context:
 	kfree(hdcp_work);
+	kfree(hdcp_work->srm);
+	kfree(hdcp_work->srm_temp);
 
 	return NULL;
 
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
index 331b50825510..5159b3a5e5b0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
@@ -53,6 +53,12 @@ struct hdcp_workqueue {
 
 	enum mod_hdcp_encryption_status encryption_status;
 	uint8_t max_link;
+
+	uint8_t *srm;
+	uint8_t *srm_temp;
+	uint32_t srm_version;
+	uint32_t srm_size;
+	struct bin_attribute attr;
 };
 
 void hdcp_update_display(struct hdcp_workqueue *hdcp_work,
-- 
2.17.1

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

Re: [PATCH 3/5] drm/amd/display: Add sysfs interface for set/get srm

[Alex Deucher]

On Thu, Jan 16, 2020 at 3:30 PM Bhawanpreet Lakha
<Bhawanpreet.Lakha@amd.com> wrote:
>
> [Why]
> We need to set/get SRM and linux kernel is not suppose to write to the
> storage, so we need to provide a interface.
>
> [How]
> Provide interface so usermode can set/get srm
>
> Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
> Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c    | 124 +++++++++++++++++-
>  .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.h    |   6 +
>  2 files changed, 128 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> index a269916f7dd6..a191c84ad8eb 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> @@ -28,6 +28,8 @@
>  #include "amdgpu_dm.h"
>  #include "dm_helpers.h"
>  #include <drm/drm_hdcp.h>
> +#include "hdcp_psp.h"
> +
>
>  static bool
>  lp_write_i2c(void *handle, uint32_t address, const uint8_t *data, uint32_t size)
> @@ -67,6 +69,16 @@ lp_read_dpcd(void *handle, uint32_t address, uint8_t *data, uint32_t size)
>         return dm_helpers_dp_read_dpcd(link->ctx, link, address, data, size);
>  }
>
> +static uint8_t *psp_get_srm(struct psp_context *psp, uint32_t *srm_version, uint32_t *srm_size)
> +{
> +       return NULL;
> +}
> +
> +static int psp_set_srm(struct psp_context *psp, uint8_t *srm, uint32_t srm_size, uint32_t *srm_version)
> +{
> +       return 0;
> +}
> +
>  static void process_output(struct hdcp_workqueue *hdcp_work)
>  {
>         struct mod_hdcp_output output = hdcp_work->output;
> @@ -88,6 +100,18 @@ static void process_output(struct hdcp_workqueue *hdcp_work)
>         schedule_delayed_work(&hdcp_work->property_validate_dwork, msecs_to_jiffies(0));
>  }
>
> +static void link_lock(struct hdcp_workqueue *work, bool lock)
> +{
> +
> +       int i = 0;
> +
> +       for (i = 0; i < work->max_link; i++) {
> +               if (lock)
> +                       mutex_lock(&work[i].mutex);
> +               else
> +                       mutex_unlock(&work[i].mutex);
> +       }
> +}
>  void hdcp_update_display(struct hdcp_workqueue *hdcp_work,
>                          unsigned int link_index,
>                          struct amdgpu_dm_connector *aconnector,
> @@ -302,7 +326,8 @@ void hdcp_destroy(struct hdcp_workqueue *hdcp_work)
>         }
>
>         kfree(hdcp_work);
> -
> +       kfree(hdcp_work->srm);
> +       kfree(hdcp_work->srm_temp);
>  }
>
>  static void update_config(void *handle, struct cp_psp_stream_config *config)
> @@ -338,6 +363,84 @@ static void update_config(void *handle, struct cp_psp_stream_config *config)
>         hdcp_update_display(hdcp_work, link_index, aconnector, DRM_MODE_HDCP_CONTENT_TYPE0, false);
>  }
>
> +
> +/*
> + * This can be called twice, because SRM_SIZE > PAGE_SIZE.
> + *
> + * We set the SRM on each call, if SRM_SIZE > PAGE_SIZE, PSP will fail on the
> + * first call but pass on the second call.
> + *
> + * Because of this we are not throwing any errors as it will stop the next call.
> + * So it is a good idea to call the "read" sysfs to verify that the SRM was set
> + *
> + */

Rather than using a file to get the data directly in chunks, how about
adding a sysfs file where you can specify the path to the srm file.
The driver can then use the path provided to call request firmware and
just get the entire binary in one shot.

Alex


> +static ssize_t srm_data_write(struct file *filp, struct kobject *kobj, struct bin_attribute *bin_attr, char *buffer,
> +                             loff_t pos, size_t count)
> +{
> +       struct hdcp_workqueue *work;
> +       uint32_t srm_version = 0;
> +
> +       work = container_of(bin_attr, struct hdcp_workqueue, attr);
> +       link_lock(work, true);
> +
> +       memcpy(work->srm_temp + pos, buffer, count);
> +
> +       if (!psp_set_srm(work->hdcp.config.psp.handle, work->srm_temp, pos + count, &srm_version)) {
> +               DRM_DEBUG_DRIVER("HDCP SRM SET version 0x%X", srm_version);
> +               memcpy(work->srm, work->srm_temp, pos + count);
> +               work->srm_size = pos + count;
> +               work->srm_version = srm_version;
> +       }
> +
> +
> +       link_lock(work, false);
> +
> +       return count;
> +}
> +
> +static ssize_t srm_data_read(struct file *filp, struct kobject *kobj, struct bin_attribute *bin_attr, char *buffer,
> +                            loff_t pos, size_t count)
> +{
> +       struct hdcp_workqueue *work;
> +       uint8_t *srm = NULL;
> +       uint32_t srm_version;
> +       uint32_t srm_size;
> +       size_t ret = count;
> +
> +       work = container_of(bin_attr, struct hdcp_workqueue, attr);
> +
> +       link_lock(work, true);
> +
> +       srm = psp_get_srm(work->hdcp.config.psp.handle, &srm_version, &srm_size);
> +
> +       if (!srm)
> +               return -EINVAL;
> +
> +       if (pos >= srm_size)
> +               ret = 0;
> +
> +       if (srm_size - pos < count) {
> +               memcpy(buffer, srm + pos, srm_size - pos);
> +               ret = srm_size - pos;
> +               goto ret;
> +       }
> +
> +       memcpy(buffer, srm + pos, count);
> +
> +ret:
> +       link_lock(work, false);
> +       return ret;
> +}
> +
> +
> +static const struct bin_attribute data_attr = {
> +       .attr = {.name = "hdcp_srm", .mode = 0664},
> +       .size = PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE, /* Limit SRM size */
> +       .write = srm_data_write,
> +       .read = srm_data_read,
> +};
> +
> +
>  struct hdcp_workqueue *hdcp_create_workqueue(struct amdgpu_device *adev, struct cp_psp *cp_psp, struct dc *dc)
>  {
>
> @@ -348,10 +451,19 @@ struct hdcp_workqueue *hdcp_create_workqueue(struct amdgpu_device *adev, struct
>         if (hdcp_work == NULL)
>                 goto fail_alloc_context;
>
> +       hdcp_work->srm = kcalloc(PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE, sizeof(*hdcp_work->srm), GFP_KERNEL);
> +
> +       if (hdcp_work->srm == NULL)
> +               goto fail_alloc_context;
> +
> +       hdcp_work->srm_temp = kcalloc(PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE, sizeof(*hdcp_work->srm_temp), GFP_KERNEL);
> +
> +       if (hdcp_work->srm_temp == NULL)
> +               goto fail_alloc_context;
> +
>         hdcp_work->max_link = max_caps;
>
>         for (i = 0; i < max_caps; i++) {
> -
>                 mutex_init(&hdcp_work[i].mutex);
>
>                 INIT_WORK(&hdcp_work[i].cpirq_work, event_cpirq);
> @@ -371,10 +483,18 @@ struct hdcp_workqueue *hdcp_create_workqueue(struct amdgpu_device *adev, struct
>         cp_psp->funcs.update_stream_config = update_config;
>         cp_psp->handle = hdcp_work;
>
> +       /* File created at /sys/class/drm/card0/device/hdcp_srm*/
> +       hdcp_work[0].attr = data_attr;
> +
> +       if (sysfs_create_bin_file(&adev->dev->kobj, &hdcp_work[0].attr))
> +               DRM_WARN("Failed to create device file hdcp_srm");
> +
>         return hdcp_work;
>
>  fail_alloc_context:
>         kfree(hdcp_work);
> +       kfree(hdcp_work->srm);
> +       kfree(hdcp_work->srm_temp);
>
>         return NULL;
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
> index 331b50825510..5159b3a5e5b0 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
> @@ -53,6 +53,12 @@ struct hdcp_workqueue {
>
>         enum mod_hdcp_encryption_status encryption_status;
>         uint8_t max_link;
> +
> +       uint8_t *srm;
> +       uint8_t *srm_temp;
> +       uint32_t srm_version;
> +       uint32_t srm_size;
> +       struct bin_attribute attr;
>  };
>
>  void hdcp_update_display(struct hdcp_workqueue *hdcp_work,
> --
> 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

Re: [PATCH 3/5] drm/amd/display: Add sysfs interface for set/get srm

[Bhawanpreet Lakha]

On 2020-01-17 2:23 p.m., Alex Deucher wrote:
> On Thu, Jan 16, 2020 at 3:30 PM Bhawanpreet Lakha
> <Bhawanpreet.Lakha@amd.com> wrote:
>> [Why]
>> We need to set/get SRM and linux kernel is not suppose to write to the
>> storage, so we need to provide a interface.
>>
>> [How]
>> Provide interface so usermode can set/get srm
>>
>> Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
>> Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
>> ---
>>   .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c    | 124 +++++++++++++++++-
>>   .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.h    |   6 +
>>   2 files changed, 128 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
>> index a269916f7dd6..a191c84ad8eb 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
>> @@ -28,6 +28,8 @@
>>   #include "amdgpu_dm.h"
>>   #include "dm_helpers.h"
>>   #include <drm/drm_hdcp.h>
>> +#include "hdcp_psp.h"
>> +
>>
>>   static bool
>>   lp_write_i2c(void *handle, uint32_t address, const uint8_t *data, uint32_t size)
>> @@ -67,6 +69,16 @@ lp_read_dpcd(void *handle, uint32_t address, uint8_t *data, uint32_t size)
>>          return dm_helpers_dp_read_dpcd(link->ctx, link, address, data, size);
>>   }
>>
>> +static uint8_t *psp_get_srm(struct psp_context *psp, uint32_t *srm_version, uint32_t *srm_size)
>> +{
>> +       return NULL;
>> +}
>> +
>> +static int psp_set_srm(struct psp_context *psp, uint8_t *srm, uint32_t srm_size, uint32_t *srm_version)
>> +{
>> +       return 0;
>> +}
>> +
>>   static void process_output(struct hdcp_workqueue *hdcp_work)
>>   {
>>          struct mod_hdcp_output output = hdcp_work->output;
>> @@ -88,6 +100,18 @@ static void process_output(struct hdcp_workqueue *hdcp_work)
>>          schedule_delayed_work(&hdcp_work->property_validate_dwork, msecs_to_jiffies(0));
>>   }
>>
>> +static void link_lock(struct hdcp_workqueue *work, bool lock)
>> +{
>> +
>> +       int i = 0;
>> +
>> +       for (i = 0; i < work->max_link; i++) {
>> +               if (lock)
>> +                       mutex_lock(&work[i].mutex);
>> +               else
>> +                       mutex_unlock(&work[i].mutex);
>> +       }
>> +}
>>   void hdcp_update_display(struct hdcp_workqueue *hdcp_work,
>>                           unsigned int link_index,
>>                           struct amdgpu_dm_connector *aconnector,
>> @@ -302,7 +326,8 @@ void hdcp_destroy(struct hdcp_workqueue *hdcp_work)
>>          }
>>
>>          kfree(hdcp_work);
>> -
>> +       kfree(hdcp_work->srm);
>> +       kfree(hdcp_work->srm_temp);
>>   }
>>
>>   static void update_config(void *handle, struct cp_psp_stream_config *config)
>> @@ -338,6 +363,84 @@ static void update_config(void *handle, struct cp_psp_stream_config *config)
>>          hdcp_update_display(hdcp_work, link_index, aconnector, DRM_MODE_HDCP_CONTENT_TYPE0, false);
>>   }
>>
>> +
>> +/*
>> + * This can be called twice, because SRM_SIZE > PAGE_SIZE.
>> + *
>> + * We set the SRM on each call, if SRM_SIZE > PAGE_SIZE, PSP will fail on the
>> + * first call but pass on the second call.
>> + *
>> + * Because of this we are not throwing any errors as it will stop the next call.
>> + * So it is a good idea to call the "read" sysfs to verify that the SRM was set
>> + *
>> + */
> Rather than using a file to get the data directly in chunks, how about
> adding a sysfs file where you can specify the path to the srm file.
> The driver can then use the path provided to call request firmware and
> just get the entire binary in one shot.
>
> Alex
>
>
I thought about using request_firmware but, since we also need to save 
the data aswell

and there is no "save_firmware" interface, we would have difference 
interfaces for reading and writing.

So to keep it consistent I used sysfs for both. Now we just cat and echo 
the raw data. vs echo file patch and cat raw data.


Bhawan

>> +static ssize_t srm_data_write(struct file *filp, struct kobject *kobj, struct bin_attribute *bin_attr, char *buffer,
>> +                             loff_t pos, size_t count)
>> +{
>> +       struct hdcp_workqueue *work;
>> +       uint32_t srm_version = 0;
>> +
>> +       work = container_of(bin_attr, struct hdcp_workqueue, attr);
>> +       link_lock(work, true);
>> +
>> +       memcpy(work->srm_temp + pos, buffer, count);
>> +
>> +       if (!psp_set_srm(work->hdcp.config.psp.handle, work->srm_temp, pos + count, &srm_version)) {
>> +               DRM_DEBUG_DRIVER("HDCP SRM SET version 0x%X", srm_version);
>> +               memcpy(work->srm, work->srm_temp, pos + count);
>> +               work->srm_size = pos + count;
>> +               work->srm_version = srm_version;
>> +       }
>> +
>> +
>> +       link_lock(work, false);
>> +
>> +       return count;
>> +}
>> +
>> +static ssize_t srm_data_read(struct file *filp, struct kobject *kobj, struct bin_attribute *bin_attr, char *buffer,
>> +                            loff_t pos, size_t count)
>> +{
>> +       struct hdcp_workqueue *work;
>> +       uint8_t *srm = NULL;
>> +       uint32_t srm_version;
>> +       uint32_t srm_size;
>> +       size_t ret = count;
>> +
>> +       work = container_of(bin_attr, struct hdcp_workqueue, attr);
>> +
>> +       link_lock(work, true);
>> +
>> +       srm = psp_get_srm(work->hdcp.config.psp.handle, &srm_version, &srm_size);
>> +
>> +       if (!srm)
>> +               return -EINVAL;
>> +
>> +       if (pos >= srm_size)
>> +               ret = 0;
>> +
>> +       if (srm_size - pos < count) {
>> +               memcpy(buffer, srm + pos, srm_size - pos);
>> +               ret = srm_size - pos;
>> +               goto ret;
>> +       }
>> +
>> +       memcpy(buffer, srm + pos, count);
>> +
>> +ret:
>> +       link_lock(work, false);
>> +       return ret;
>> +}
>> +
>> +
>> +static const struct bin_attribute data_attr = {
>> +       .attr = {.name = "hdcp_srm", .mode = 0664},
>> +       .size = PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE, /* Limit SRM size */
>> +       .write = srm_data_write,
>> +       .read = srm_data_read,
>> +};
>> +
>> +
>>   struct hdcp_workqueue *hdcp_create_workqueue(struct amdgpu_device *adev, struct cp_psp *cp_psp, struct dc *dc)
>>   {
>>
>> @@ -348,10 +451,19 @@ struct hdcp_workqueue *hdcp_create_workqueue(struct amdgpu_device *adev, struct
>>          if (hdcp_work == NULL)
>>                  goto fail_alloc_context;
>>
>> +       hdcp_work->srm = kcalloc(PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE, sizeof(*hdcp_work->srm), GFP_KERNEL);
>> +
>> +       if (hdcp_work->srm == NULL)
>> +               goto fail_alloc_context;
>> +
>> +       hdcp_work->srm_temp = kcalloc(PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE, sizeof(*hdcp_work->srm_temp), GFP_KERNEL);
>> +
>> +       if (hdcp_work->srm_temp == NULL)
>> +               goto fail_alloc_context;
>> +
>>          hdcp_work->max_link = max_caps;
>>
>>          for (i = 0; i < max_caps; i++) {
>> -
>>                  mutex_init(&hdcp_work[i].mutex);
>>
>>                  INIT_WORK(&hdcp_work[i].cpirq_work, event_cpirq);
>> @@ -371,10 +483,18 @@ struct hdcp_workqueue *hdcp_create_workqueue(struct amdgpu_device *adev, struct
>>          cp_psp->funcs.update_stream_config = update_config;
>>          cp_psp->handle = hdcp_work;
>>
>> +       /* File created at /sys/class/drm/card0/device/hdcp_srm*/
>> +       hdcp_work[0].attr = data_attr;
>> +
>> +       if (sysfs_create_bin_file(&adev->dev->kobj, &hdcp_work[0].attr))
>> +               DRM_WARN("Failed to create device file hdcp_srm");
>> +
>>          return hdcp_work;
>>
>>   fail_alloc_context:
>>          kfree(hdcp_work);
>> +       kfree(hdcp_work->srm);
>> +       kfree(hdcp_work->srm_temp);
>>
>>          return NULL;
>>
>> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
>> index 331b50825510..5159b3a5e5b0 100644
>> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
>> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
>> @@ -53,6 +53,12 @@ struct hdcp_workqueue {
>>
>>          enum mod_hdcp_encryption_status encryption_status;
>>          uint8_t max_link;
>> +
>> +       uint8_t *srm;
>> +       uint8_t *srm_temp;
>> +       uint32_t srm_version;
>> +       uint32_t srm_size;
>> +       struct bin_attribute attr;
>>   };
>>
>>   void hdcp_update_display(struct hdcp_workqueue *hdcp_work,
>> --
>> 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=02%7C01%7CBhawanpreet.Lakha%40amd.com%7C31850cb2894b4459b6cf08d79b82b6af%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637148858026280913&amp;sdata=%2BncbIbzgws%2BKPbTAqzARNGAZWM66UoxzjDeJm6vvXlg%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 3/5] drm/amd/display: Add sysfs interface for set/get srm

[Harry Wentland]

On 2020-01-16 3:29 p.m., Bhawanpreet Lakha wrote:
> [Why]
> We need to set/get SRM and linux kernel is not suppose to write to the

"We need to set/get SRM from/to PSP FW..."

> storage, so we need to provide a interface.
> 

an interface.

Highlight that we expect these to be exercised by usermode scripts that
run at boot, shutdown, suspend, and resume.

Harry

> [How]
> Provide interface so usermode can set/get srm
> 
> Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
> Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c    | 124 +++++++++++++++++-
>  .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.h    |   6 +
>  2 files changed, 128 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> index a269916f7dd6..a191c84ad8eb 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> @@ -28,6 +28,8 @@
>  #include "amdgpu_dm.h"
>  #include "dm_helpers.h"
>  #include <drm/drm_hdcp.h>
> +#include "hdcp_psp.h"
> +
>  
>  static bool
>  lp_write_i2c(void *handle, uint32_t address, const uint8_t *data, uint32_t size)
> @@ -67,6 +69,16 @@ lp_read_dpcd(void *handle, uint32_t address, uint8_t *data, uint32_t size)
>  	return dm_helpers_dp_read_dpcd(link->ctx, link, address, data, size);
>  }
>  
> +static uint8_t *psp_get_srm(struct psp_context *psp, uint32_t *srm_version, uint32_t *srm_size)
> +{
> +	return NULL;
> +}
> +
> +static int psp_set_srm(struct psp_context *psp, uint8_t *srm, uint32_t srm_size, uint32_t *srm_version)
> +{
> +	return 0;
> +}
> +
>  static void process_output(struct hdcp_workqueue *hdcp_work)
>  {
>  	struct mod_hdcp_output output = hdcp_work->output;
> @@ -88,6 +100,18 @@ static void process_output(struct hdcp_workqueue *hdcp_work)
>  	schedule_delayed_work(&hdcp_work->property_validate_dwork, msecs_to_jiffies(0));
>  }
>  
> +static void link_lock(struct hdcp_workqueue *work, bool lock)
> +{
> +
> +	int i = 0;
> +
> +	for (i = 0; i < work->max_link; i++) {
> +		if (lock)
> +			mutex_lock(&work[i].mutex);
> +		else
> +			mutex_unlock(&work[i].mutex);
> +	}
> +}
>  void hdcp_update_display(struct hdcp_workqueue *hdcp_work,
>  			 unsigned int link_index,
>  			 struct amdgpu_dm_connector *aconnector,
> @@ -302,7 +326,8 @@ void hdcp_destroy(struct hdcp_workqueue *hdcp_work)
>  	}
>  
>  	kfree(hdcp_work);
> -
> +	kfree(hdcp_work->srm);
> +	kfree(hdcp_work->srm_temp);
>  }
>  
>  static void update_config(void *handle, struct cp_psp_stream_config *config)
> @@ -338,6 +363,84 @@ static void update_config(void *handle, struct cp_psp_stream_config *config)
>  	hdcp_update_display(hdcp_work, link_index, aconnector, DRM_MODE_HDCP_CONTENT_TYPE0, false);
>  }
>  
> +
> +/*
> + * This can be called twice, because SRM_SIZE > PAGE_SIZE.
> + *
> + * We set the SRM on each call, if SRM_SIZE > PAGE_SIZE, PSP will fail on the
> + * first call but pass on the second call.
> + *
> + * Because of this we are not throwing any errors as it will stop the next call.
> + * So it is a good idea to call the "read" sysfs to verify that the SRM was set
> + *
> + */
> +static ssize_t srm_data_write(struct file *filp, struct kobject *kobj, struct bin_attribute *bin_attr, char *buffer,
> +			      loff_t pos, size_t count)
> +{
> +	struct hdcp_workqueue *work;
> +	uint32_t srm_version = 0;
> +
> +	work = container_of(bin_attr, struct hdcp_workqueue, attr);
> +	link_lock(work, true);
> +
> +	memcpy(work->srm_temp + pos, buffer, count);
> +
> +	if (!psp_set_srm(work->hdcp.config.psp.handle, work->srm_temp, pos + count, &srm_version)) {
> +		DRM_DEBUG_DRIVER("HDCP SRM SET version 0x%X", srm_version);
> +		memcpy(work->srm, work->srm_temp, pos + count);
> +		work->srm_size = pos + count;
> +		work->srm_version = srm_version;
> +	}
> +
> +
> +	link_lock(work, false);
> +
> +	return count;
> +}
> +
> +static ssize_t srm_data_read(struct file *filp, struct kobject *kobj, struct bin_attribute *bin_attr, char *buffer,
> +			     loff_t pos, size_t count)
> +{
> +	struct hdcp_workqueue *work;
> +	uint8_t *srm = NULL;
> +	uint32_t srm_version;
> +	uint32_t srm_size;
> +	size_t ret = count;
> +
> +	work = container_of(bin_attr, struct hdcp_workqueue, attr);
> +
> +	link_lock(work, true);
> +
> +	srm = psp_get_srm(work->hdcp.config.psp.handle, &srm_version, &srm_size);
> +
> +	if (!srm)
> +		return -EINVAL;
> +
> +	if (pos >= srm_size)
> +		ret = 0;
> +
> +	if (srm_size - pos < count) {
> +		memcpy(buffer, srm + pos, srm_size - pos);
> +		ret = srm_size - pos;
> +		goto ret;
> +	}
> +
> +	memcpy(buffer, srm + pos, count);
> +
> +ret:
> +	link_lock(work, false);
> +	return ret;
> +}
> +
> +
> +static const struct bin_attribute data_attr = {
> +	.attr = {.name = "hdcp_srm", .mode = 0664},
> +	.size = PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE, /* Limit SRM size */
> +	.write = srm_data_write,
> +	.read = srm_data_read,
> +};
> +
> +
>  struct hdcp_workqueue *hdcp_create_workqueue(struct amdgpu_device *adev, struct cp_psp *cp_psp, struct dc *dc)
>  {
>  
> @@ -348,10 +451,19 @@ struct hdcp_workqueue *hdcp_create_workqueue(struct amdgpu_device *adev, struct
>  	if (hdcp_work == NULL)
>  		goto fail_alloc_context;
>  
> +	hdcp_work->srm = kcalloc(PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE, sizeof(*hdcp_work->srm), GFP_KERNEL);
> +
> +	if (hdcp_work->srm == NULL)
> +		goto fail_alloc_context;
> +
> +	hdcp_work->srm_temp = kcalloc(PSP_HDCP_SRM_FIRST_GEN_MAX_SIZE, sizeof(*hdcp_work->srm_temp), GFP_KERNEL);
> +
> +	if (hdcp_work->srm_temp == NULL)
> +		goto fail_alloc_context;
> +
>  	hdcp_work->max_link = max_caps;
>  
>  	for (i = 0; i < max_caps; i++) {
> -
>  		mutex_init(&hdcp_work[i].mutex);
>  
>  		INIT_WORK(&hdcp_work[i].cpirq_work, event_cpirq);
> @@ -371,10 +483,18 @@ struct hdcp_workqueue *hdcp_create_workqueue(struct amdgpu_device *adev, struct
>  	cp_psp->funcs.update_stream_config = update_config;
>  	cp_psp->handle = hdcp_work;
>  
> +	/* File created at /sys/class/drm/card0/device/hdcp_srm*/
> +	hdcp_work[0].attr = data_attr;
> +
> +	if (sysfs_create_bin_file(&adev->dev->kobj, &hdcp_work[0].attr))
> +		DRM_WARN("Failed to create device file hdcp_srm");
> +
>  	return hdcp_work;
>  
>  fail_alloc_context:
>  	kfree(hdcp_work);
> +	kfree(hdcp_work->srm);
> +	kfree(hdcp_work->srm_temp);
>  
>  	return NULL;
>  
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
> index 331b50825510..5159b3a5e5b0 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.h
> @@ -53,6 +53,12 @@ struct hdcp_workqueue {
>  
>  	enum mod_hdcp_encryption_status encryption_status;
>  	uint8_t max_link;
> +
> +	uint8_t *srm;
> +	uint8_t *srm_temp;
> +	uint32_t srm_version;
> +	uint32_t srm_size;
> +	struct bin_attribute attr;
>  };
>  
>  void hdcp_update_display(struct hdcp_workqueue *hdcp_work,
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

[PATCH 4/5] drm/amd/display: Load srm before enabling HDCP

[Bhawanpreet Lakha]

[Why]
we need to load SRM before we start HDCP. For S3 case the sysfs call will be
after we already enabled HDCP

[How]
Set srm before starting HDCP

Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
index a191c84ad8eb..129696e997b6 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
@@ -136,6 +136,13 @@ void hdcp_update_display(struct hdcp_workqueue *hdcp_work,
 		hdcp_w->link.adjust.hdcp2.force_type = MOD_HDCP_FORCE_TYPE_0;
 
 		if (enable_encryption) {
+			/* Explicitly set the saved SRM as sysfs call will be after we already enabled hdcp
+			 * (s3 resume case)
+			 */
+			if (hdcp_work->srm_size > 0)
+				psp_set_srm(hdcp_work->hdcp.config.psp.handle, hdcp_work->srm, hdcp_work->srm_size,
+					    &hdcp_work->srm_version);
+
 			display->adjust.disable = 0;
 			if (content_type == DRM_MODE_HDCP_CONTENT_TYPE0)
 				hdcp_w->link.adjust.hdcp2.force_type = MOD_HDCP_FORCE_TYPE_0;
-- 
2.17.1

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

[PATCH 5/5] drm/amd/display: call psp set/get interfaces

[Bhawanpreet Lakha]

Call the cmd ids for set/get srm according to the sysfs call

Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
---
 .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c    | 49 ++++++++++++++++++-
 1 file changed, 48 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
index 129696e997b6..913d0e1e0828 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
@@ -71,11 +71,58 @@ lp_read_dpcd(void *handle, uint32_t address, uint8_t *data, uint32_t size)
 
 static uint8_t *psp_get_srm(struct psp_context *psp, uint32_t *srm_version, uint32_t *srm_size)
 {
-	return NULL;
+
+	struct ta_hdcp_shared_memory *hdcp_cmd;
+
+	if (!psp->hdcp_context.hdcp_initialized) {
+		DRM_WARN("Failed to get hdcp srm. HDCP TA is not initialized.");
+		return NULL;
+	}
+
+	hdcp_cmd = (struct ta_hdcp_shared_memory *)psp->hdcp_context.hdcp_shared_buf;
+	memset(hdcp_cmd, 0, sizeof(struct ta_hdcp_shared_memory));
+
+	hdcp_cmd->cmd_id = TA_HDCP_COMMAND__HDCP_GET_SRM;
+	psp_hdcp_invoke(psp, hdcp_cmd->cmd_id);
+
+	if (hdcp_cmd->hdcp_status != TA_HDCP_STATUS__SUCCESS)
+		return NULL;
+
+	*srm_version = hdcp_cmd->out_msg.hdcp_get_srm.srm_version;
+	*srm_size = hdcp_cmd->out_msg.hdcp_get_srm.srm_buf_size;
+
+
+	return hdcp_cmd->out_msg.hdcp_get_srm.srm_buf;
 }
 
 static int psp_set_srm(struct psp_context *psp, uint8_t *srm, uint32_t srm_size, uint32_t *srm_version)
 {
+
+	struct ta_hdcp_shared_memory *hdcp_cmd;
+
+	if (!psp->hdcp_context.hdcp_initialized) {
+		DRM_WARN("Failed to get hdcp srm. HDCP TA is not initialized.");
+		return -EINVAL;
+	}
+
+	hdcp_cmd = (struct ta_hdcp_shared_memory *)psp->hdcp_context.hdcp_shared_buf;
+	memset(hdcp_cmd, 0, sizeof(struct ta_hdcp_shared_memory));
+
+	memcpy(hdcp_cmd->in_msg.hdcp_set_srm.srm_buf, srm, srm_size);
+	hdcp_cmd->in_msg.hdcp_set_srm.srm_buf_size = srm_size;
+	hdcp_cmd->cmd_id = TA_HDCP_COMMAND__HDCP_SET_SRM;
+
+	psp_hdcp_invoke(psp, hdcp_cmd->cmd_id);
+
+	/*
+	 * If the SRM version being loaded is less than or equal to the
+	 * currently loaded SRM, psp will return 0xFFFF
+	 */
+	if (hdcp_cmd->hdcp_status != TA_HDCP_STATUS__SUCCESS || hdcp_cmd->out_msg.hdcp_set_srm.valid_signature != 1 ||
+	    hdcp_cmd->out_msg.hdcp_set_srm.srm_version == 0xFFFF)
+		return -EINVAL;
+
+	*srm_version = hdcp_cmd->out_msg.hdcp_set_srm.srm_version;
 	return 0;
 }
 
-- 
2.17.1

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

Re: [PATCH 5/5] drm/amd/display: call psp set/get interfaces

[Harry Wentland]

On 2020-01-16 3:29 p.m., Bhawanpreet Lakha wrote:
> Call the cmd ids for set/get srm according to the sysfs call
> 
> Signed-off-by: Bhawanpreet Lakha <Bhawanpreet.Lakha@amd.com>
> Reviewed-by: Rodrigo Siqueira <Rodrigo.Siqueira@amd.com>
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c    | 49 ++++++++++++++++++-
>  1 file changed, 48 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> index 129696e997b6..913d0e1e0828 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_hdcp.c
> @@ -71,11 +71,58 @@ lp_read_dpcd(void *handle, uint32_t address, uint8_t *data, uint32_t size)
>  
>  static uint8_t *psp_get_srm(struct psp_context *psp, uint32_t *srm_version, uint32_t *srm_size)
>  {
> -	return NULL;
> +
> +	struct ta_hdcp_shared_memory *hdcp_cmd;
> +
> +	if (!psp->hdcp_context.hdcp_initialized) {
> +		DRM_WARN("Failed to get hdcp srm. HDCP TA is not initialized.");
> +		return NULL;
> +	}
> +
> +	hdcp_cmd = (struct ta_hdcp_shared_memory *)psp->hdcp_context.hdcp_shared_buf;
> +	memset(hdcp_cmd, 0, sizeof(struct ta_hdcp_shared_memory));
> +
> +	hdcp_cmd->cmd_id = TA_HDCP_COMMAND__HDCP_GET_SRM;
> +	psp_hdcp_invoke(psp, hdcp_cmd->cmd_id);
> +
> +	if (hdcp_cmd->hdcp_status != TA_HDCP_STATUS__SUCCESS)
> +		return NULL;
> +
> +	*srm_version = hdcp_cmd->out_msg.hdcp_get_srm.srm_version;
> +	*srm_size = hdcp_cmd->out_msg.hdcp_get_srm.srm_buf_size;
> +
> +
> +	return hdcp_cmd->out_msg.hdcp_get_srm.srm_buf;
>  }
>  
>  static int psp_set_srm(struct psp_context *psp, uint8_t *srm, uint32_t srm_size, uint32_t *srm_version)
>  {
> +
> +	struct ta_hdcp_shared_memory *hdcp_cmd;
> +
> +	if (!psp->hdcp_context.hdcp_initialized) {
> +		DRM_WARN("Failed to get hdcp srm. HDCP TA is not initialized.");
> +		return -EINVAL;
> +	}
> +
> +	hdcp_cmd = (struct ta_hdcp_shared_memory *)psp->hdcp_context.hdcp_shared_buf;
> +	memset(hdcp_cmd, 0, sizeof(struct ta_hdcp_shared_memory));
> +
> +	memcpy(hdcp_cmd->in_msg.hdcp_set_srm.srm_buf, srm, srm_size);
> +	hdcp_cmd->in_msg.hdcp_set_srm.srm_buf_size = srm_size;
> +	hdcp_cmd->cmd_id = TA_HDCP_COMMAND__HDCP_SET_SRM;
> +
> +	psp_hdcp_invoke(psp, hdcp_cmd->cmd_id);
> +
> +	/*
> +	 * If the SRM version being loaded is less than or equal to the
> +	 * currently loaded SRM, psp will return 0xFFFF
> +	 */
> +	if (hdcp_cmd->hdcp_status != TA_HDCP_STATUS__SUCCESS || hdcp_cmd->out_msg.hdcp_set_srm.valid_signature != 1 ||
> +	    hdcp_cmd->out_msg.hdcp_set_srm.srm_version == 0xFFFF)

Can we define the 0xFFFF value in this file instead of using and
documenting a magic value?

Harry

> +		return -EINVAL;
> +
> +	*srm_version = hdcp_cmd->out_msg.hdcp_set_srm.srm_version;
>  	return 0;
>  }
>  
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 0/5] HDCP SRM interface

[Harry Wentland]

On 2020-01-16 3:29 p.m., Bhawanpreet Lakha wrote:
> These patches adds support for SRM loading. By providing an interface to the
> usermode
> 
> SRM has to be persistent and since the kernel cannot directly write to system
> storage we need to provide an interface so that the usermode can do it for us
> 

We'll want to elaborate a bit more on why and how this is done. As
mentioned on my patch 2 comments I recommend to do this there as the
cover letter is lost after merge.

Harry

> 
> Bhawanpreet Lakha (5):
>   drm/amd/display: Pass amdgpu_device instead of psp_context
>   drm/amd/display: update psp interface header
>   drm/amd/display: Add sysfs interface for set/get srm
>   drm/amd/display: Load srm before enabling HDCP
>   drm/amd/display: call psp set/get interfaces
> 
>  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   2 +-
>  .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c    | 182 +++++++++++++++++-
>  .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.h    |   9 +-
>  .../drm/amd/display/modules/hdcp/hdcp_psp.h   |  26 ++-
>  4 files changed, 212 insertions(+), 7 deletions(-)
> 
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 0/5] HDCP SRM interface

[Alex Deucher]

On Wed, Jan 22, 2020 at 11:12 AM Harry Wentland <hwentlan@amd.com> wrote:
>
> On 2020-01-16 3:29 p.m., Bhawanpreet Lakha wrote:
> > These patches adds support for SRM loading. By providing an interface to the
> > usermode
> >
> > SRM has to be persistent and since the kernel cannot directly write to system
> > storage we need to provide an interface so that the usermode can do it for us
> >
>
> We'll want to elaborate a bit more on why and how this is done. As
> mentioned on my patch 2 comments I recommend to do this there as the
> cover letter is lost after merge.
>

You might also want to cc dri-devel if you resend to get more reviews.
I'm also not crazy about having to update the file in chunks, but I
don't have any better ideas off hand.  Maybe an ioctl would be
cleaner?

Alex

> Harry
>
> >
> > Bhawanpreet Lakha (5):
> >   drm/amd/display: Pass amdgpu_device instead of psp_context
> >   drm/amd/display: update psp interface header
> >   drm/amd/display: Add sysfs interface for set/get srm
> >   drm/amd/display: Load srm before enabling HDCP
> >   drm/amd/display: call psp set/get interfaces
> >
> >  .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   2 +-
> >  .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c    | 182 +++++++++++++++++-
> >  .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.h    |   9 +-
> >  .../drm/amd/display/modules/hdcp/hdcp_psp.h   |  26 ++-
> >  4 files changed, 212 insertions(+), 7 deletions(-)
> >
> _______________________________________________
> 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

Re: [PATCH 0/5] HDCP SRM interface

[Bhawanpreet Lakha]

On 2020-01-22 11:23 a.m., Alex Deucher wrote:
> On Wed, Jan 22, 2020 at 11:12 AM Harry Wentland <hwentlan@amd.com> wrote:
>> On 2020-01-16 3:29 p.m., Bhawanpreet Lakha wrote:
>>> These patches adds support for SRM loading. By providing an interface to the
>>> usermode
>>>
>>> SRM has to be persistent and since the kernel cannot directly write to system
>>> storage we need to provide an interface so that the usermode can do it for us
>>>
>> We'll want to elaborate a bit more on why and how this is done. As
>> mentioned on my patch 2 comments I recommend to do this there as the
>> cover letter is lost after merge.
>>
> You might also want to cc dri-devel if you resend to get more reviews.
> I'm also not crazy about having to update the file in chunks, but I
> don't have any better ideas off hand.  Maybe an ioctl would be
> cleaner?
>
> Alex

The kernel can only send PAGE_SIZE (4KB) at once, so if the file is 
bigger than PAGE_SIZE (max SRM is 5KB) it will send it again until its 
finished (unless we increase the page size).

 From the user space its just a single command to read/write

     save to storage : cat /sys/class/drm/card0/device/hdcp_srm > file

     load from storage : cat file> /sys/class/drm/card0/device/hdcp_srm

I will send it to dri-devel after fixing what Harry suggested.

Thanks

Bhawan

>> Harry
>>
>>> Bhawanpreet Lakha (5):
>>>    drm/amd/display: Pass amdgpu_device instead of psp_context
>>>    drm/amd/display: update psp interface header
>>>    drm/amd/display: Add sysfs interface for set/get srm
>>>    drm/amd/display: Load srm before enabling HDCP
>>>    drm/amd/display: call psp set/get interfaces
>>>
>>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   2 +-
>>>   .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c    | 182 +++++++++++++++++-
>>>   .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.h    |   9 +-
>>>   .../drm/amd/display/modules/hdcp/hdcp_psp.h   |  26 ++-
>>>   4 files changed, 212 insertions(+), 7 deletions(-)
>>>
>> _______________________________________________
>> 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=02%7C01%7CBhawanpreet.Lakha%40amd.com%7Ce190d770392e468225c308d79f57655d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637153070030895879&amp;sdata=5mlINUlUGz4bkIcK1hUjves9N3BviOdtKuLFPftn1SY%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

Re: [PATCH 0/5] HDCP SRM interface

[Harry Wentland]

On 2020-01-22 11:44 a.m., Bhawanpreet Lakha wrote:
> 
> On 2020-01-22 11:23 a.m., Alex Deucher wrote:
>> On Wed, Jan 22, 2020 at 11:12 AM Harry Wentland <hwentlan@amd.com> wrote:
>>> On 2020-01-16 3:29 p.m., Bhawanpreet Lakha wrote:
>>>> These patches adds support for SRM loading. By providing an
>>>> interface to the
>>>> usermode
>>>>
>>>> SRM has to be persistent and since the kernel cannot directly write
>>>> to system
>>>> storage we need to provide an interface so that the usermode can do
>>>> it for us
>>>>
>>> We'll want to elaborate a bit more on why and how this is done. As
>>> mentioned on my patch 2 comments I recommend to do this there as the
>>> cover letter is lost after merge.
>>>
>> You might also want to cc dri-devel if you resend to get more reviews.
>> I'm also not crazy about having to update the file in chunks, but I
>> don't have any better ideas off hand.  Maybe an ioctl would be
>> cleaner?
>>
>> Alex
> 
> The kernel can only send PAGE_SIZE (4KB) at once, so if the file is
> bigger than PAGE_SIZE (max SRM is 5KB) it will send it again until its
> finished (unless we increase the page size).
> 
> From the user space its just a single command to read/write
> 
>     save to storage : cat /sys/class/drm/card0/device/hdcp_srm > file
> 
>     load from storage : cat file> /sys/class/drm/card0/device/hdcp_srm
> 

Please also add this info in the patch description or cover letter as
well, including how you iterate for a large SRM. A simple copy-paste
from the shell script should suffice. It's a bit hard to see how this
interface is being used from userspace, especially around the get/set in
chunks.

Harry

> I will send it to dri-devel after fixing what Harry suggested.
> 
> Thanks
> 
> Bhawan
> 
>>> Harry
>>>
>>>> Bhawanpreet Lakha (5):
>>>>    drm/amd/display: Pass amdgpu_device instead of psp_context
>>>>    drm/amd/display: update psp interface header
>>>>    drm/amd/display: Add sysfs interface for set/get srm
>>>>    drm/amd/display: Load srm before enabling HDCP
>>>>    drm/amd/display: call psp set/get interfaces
>>>>
>>>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   2 +-
>>>>   .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c    | 182
>>>> +++++++++++++++++-
>>>>   .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.h    |   9 +-
>>>>   .../drm/amd/display/modules/hdcp/hdcp_psp.h   |  26 ++-
>>>>   4 files changed, 212 insertions(+), 7 deletions(-)
>>>>
>>> _______________________________________________
>>> 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

Re: [PATCH 0/5] HDCP SRM interface

[Deucher, Alexander]

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

[AMD Public Use]

If you resend the patch set, it might be good to include the script in the series for reference as well.

Alex

________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Harry Wentland <hwentlan@amd.com>
Sent: Wednesday, January 22, 2020 2:27 PM
To: Lakha, Bhawanpreet <Bhawanpreet.Lakha@amd.com>; Alex Deucher <alexdeucher@gmail.com>
Cc: Wentland, Harry <Harry.Wentland@amd.com>; amd-gfx list <amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 0/5] HDCP SRM interface



On 2020-01-22 11:44 a.m., Bhawanpreet Lakha wrote:
>
> On 2020-01-22 11:23 a.m., Alex Deucher wrote:
>> On Wed, Jan 22, 2020 at 11:12 AM Harry Wentland <hwentlan@amd.com> wrote:
>>> On 2020-01-16 3:29 p.m., Bhawanpreet Lakha wrote:
>>>> These patches adds support for SRM loading. By providing an
>>>> interface to the
>>>> usermode
>>>>
>>>> SRM has to be persistent and since the kernel cannot directly write
>>>> to system
>>>> storage we need to provide an interface so that the usermode can do
>>>> it for us
>>>>
>>> We'll want to elaborate a bit more on why and how this is done. As
>>> mentioned on my patch 2 comments I recommend to do this there as the
>>> cover letter is lost after merge.
>>>
>> You might also want to cc dri-devel if you resend to get more reviews.
>> I'm also not crazy about having to update the file in chunks, but I
>> don't have any better ideas off hand.  Maybe an ioctl would be
>> cleaner?
>>
>> Alex
>
> The kernel can only send PAGE_SIZE (4KB) at once, so if the file is
> bigger than PAGE_SIZE (max SRM is 5KB) it will send it again until its
> finished (unless we increase the page size).
>
> From the user space its just a single command to read/write
>
>     save to storage : cat /sys/class/drm/card0/device/hdcp_srm > file
>
>     load from storage : cat file> /sys/class/drm/card0/device/hdcp_srm
>

Please also add this info in the patch description or cover letter as
well, including how you iterate for a large SRM. A simple copy-paste
from the shell script should suffice. It's a bit hard to see how this
interface is being used from userspace, especially around the get/set in
chunks.

Harry

> I will send it to dri-devel after fixing what Harry suggested.
>
> Thanks
>
> Bhawan
>
>>> Harry
>>>
>>>> Bhawanpreet Lakha (5):
>>>>    drm/amd/display: Pass amdgpu_device instead of psp_context
>>>>    drm/amd/display: update psp interface header
>>>>    drm/amd/display: Add sysfs interface for set/get srm
>>>>    drm/amd/display: Load srm before enabling HDCP
>>>>    drm/amd/display: call psp set/get interfaces
>>>>
>>>>   .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c |   2 +-
>>>>   .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.c    | 182
>>>> +++++++++++++++++-
>>>>   .../amd/display/amdgpu_dm/amdgpu_dm_hdcp.h    |   9 +-
>>>>   .../drm/amd/display/modules/hdcp/hdcp_psp.h   |  26 ++-
>>>>   4 files changed, 212 insertions(+), 7 deletions(-)
>>>>
>>> _______________________________________________
>>> 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=02%7C01%7Calexander.deucher%40amd.com%7C6045839ca9f14797ff3a08d79f713094%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637153180855191595&amp;sdata=evmiajEJOkOb2L2GgUxg6JsWadR99PgoTE%2F6cQoBKfs%3D&amp;reserved=0
>>>
_______________________________________________
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=02%7C01%7Calexander.deucher%40amd.com%7C6045839ca9f14797ff3a08d79f713094%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637153180855191595&amp;sdata=evmiajEJOkOb2L2GgUxg6JsWadR99PgoTE%2F6cQoBKfs%3D&amp;reserved=0

[-- Attachment #1.2: Type: text/html, Size: 6953 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