Re: [PATCH 4/5] drm/xe/hdcp: Enable HDCP for XE

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 2/13/2024 9:33 PM, Kandpal, Suraj wrote:
interaction of HDCP as a client with the GSC CS interface.

--v2
-add kfree at appropriate place [Daniele] -remove useless define
[Daniele] -move host session logic to xe_gsc_submit.c [Daniele] -call
xe_gsc_check_and_update_pending directly in an if condition [Daniele]
-use xe_device instead of drm_i915_private [Daniele]

--v3
-use xe prefix for newly exposed function [Daniele] -remove client
specific defines from intel_gsc_mtl_header [Daniele] -add missing
kfree() [Daniele] -have NULL check for hdcp_message in finish function
[Daniele] -dont have too many variable declarations in the same line
[Daniele]

Signed-off-by: Suraj Kandpal <suraj.kandpal@xxxxxxxxx>
---
   drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 180
++++++++++++++++++++++-
   drivers/gpu/drm/xe/xe_gsc_submit.c       |  15 ++
   drivers/gpu/drm/xe/xe_gsc_submit.h       |   1 +
   3 files changed, 193 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
index 425db3532ce5..aa8c13916bb6 100644
--- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
+++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c
@@ -4,12 +4,27 @@
    */

   #include <drm/drm_print.h>
+#include <linux/delay.h>

+#include "abi/gsc_command_header_abi.h"
   #include "intel_hdcp_gsc.h"
   #include "xe_device_types.h"
   #include "xe_gt.h"
   #include "xe_gsc_proxy.h"
   #include "xe_pm.h"
+#include "xe_bo.h"
+#include "xe_map.h"
+#include "xe_gsc_submit.h"
+
+#define HECI_MEADDRESS_HDCP 18
+
+struct intel_hdcp_gsc_message {
+	struct xe_bo *hdcp_bo;
+	u64 hdcp_cmd_in;
+	u64 hdcp_cmd_out;
+};
+
+#define HDCP_GSC_HEADER_SIZE sizeof(struct intel_gsc_mtl_header)

   bool intel_hdcp_gsc_cs_required(struct xe_device *xe)
   {
@@ -40,19 +55,178 @@ bool intel_hdcp_gsc_check_status(struct xe_device
*xe)
   	return ret;
   }

+/*This function helps allocate memory for the command that we will
+send to gsc cs */ static int intel_hdcp_gsc_initialize_message(struct
xe_device *xe,
+					     struct intel_hdcp_gsc_message
*hdcp_message) {
+	struct xe_bo *bo = NULL;
+	u64 cmd_in, cmd_out;
+	int ret = 0;
+
+	/* allocate object of two page for HDCP command memory and store
it */
+	xe_device_mem_access_get(xe);
+	bo = xe_bo_create_pin_map(xe, xe_device_get_root_tile(xe), NULL,
PAGE_SIZE * 2,
+				  ttm_bo_type_kernel,
+				  XE_BO_CREATE_SYSTEM_BIT |
+				  XE_BO_CREATE_GGTT_BIT);
+
+	if (IS_ERR(bo)) {
+		drm_err(&xe->drm, "Failed to allocate bo for HDCP streaming
command!\n");
+		ret = PTR_ERR(bo);
+		goto out;
+	}
+
+	cmd_in = xe_bo_ggtt_addr(bo);
+	cmd_out = cmd_in + PAGE_SIZE;
+	xe_map_memset(xe, &bo->vmap, 0, 0, bo->size);
+
+	hdcp_message->hdcp_bo = bo;
+	hdcp_message->hdcp_cmd_in = cmd_in;
+	hdcp_message->hdcp_cmd_out = cmd_out;
+out:
+	xe_device_mem_access_put(xe);
+	return ret;
+}
+
+static int intel_hdcp_gsc_hdcp2_init(struct xe_device *xe) {
+	struct intel_hdcp_gsc_message *hdcp_message;
+	int ret;
+
+	hdcp_message = kzalloc(sizeof(*hdcp_message), GFP_KERNEL);
+
+	if (!hdcp_message)
+		return -ENOMEM;
+
+	/*
+	 * NOTE: No need to lock the comp mutex here as it is already
+	 * going to be taken before this function called
+	 */
+	ret = intel_hdcp_gsc_initialize_message(xe, hdcp_message);
+	xe->display.hdcp.hdcp_message = hdcp_message;
+
+	if (ret) {
+		drm_err(&xe->drm, "Could not initialize hdcp_message\n");
+		kfree(hdcp_message);
Here you're leaving xe->display.hdcp.hdcp_message pointing to a memory
location that we no longer own. is that safe for the _fini function?

Would it be better to not have a kfree here if initialization fails it gets taken care of
In finish function anyways that would be safer I presume.

We normally try to clean up immediately when things go wrong, also because if this failure causes the driver load to abort the _fini function might not get called (but not sure if this is the case here).
In this case this can be easily fixed by simply changing it to:

ret = intel_hdcp_gsc_initialize_message(xe, hdcp_message);
if (ret) {
	drm_err(&xe->drm, "Could not initialize hdcp_message\n");
	kfree(hdcp_message);
	return ret;
}

xe->display.hdcp.hdcp_message = hdcp_message;
return 0;


Which guarantees that xe->display.hdcp.hdcp_message is only set when the allocation exists with minimal changes to the code.

Daniele


LGTM apart from this (assuming it is going to be squashed with the next
patch for merge).
Yes this will be squashed with the next patch when I send the new revision

Regards,
Suraj Kandpal
Daniele

+	}
+
+	return ret;
+}
+
   int intel_hdcp_gsc_init(struct xe_device *xe)
   {
-	drm_dbg_kms(&xe->drm, "HDCP support not yet implemented\n");
-	return -ENODEV;
+	struct i915_hdcp_arbiter *data;
+	int ret;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	mutex_lock(&xe->display.hdcp.hdcp_mutex);
+	xe->display.hdcp.arbiter = data;
+	xe->display.hdcp.arbiter->hdcp_dev = xe->drm.dev;
+	xe->display.hdcp.arbiter->ops = &gsc_hdcp_ops;
+	ret = intel_hdcp_gsc_hdcp2_init(xe);
+	if (ret)
+		kfree(data);
+
+	mutex_unlock(&xe->display.hdcp.hdcp_mutex);
+
+	return ret;
   }

   void intel_hdcp_gsc_fini(struct xe_device *xe)
   {
+	struct intel_hdcp_gsc_message *hdcp_message =
+					xe->display.hdcp.hdcp_message;
+
+	if (!hdcp_message)
+		return;
+
+	xe_bo_unpin_map_no_vm(hdcp_message->hdcp_bo);
+	kfree(hdcp_message);
+}
+
+static int xe_gsc_send_sync(struct xe_device *xe,
+			    struct intel_hdcp_gsc_message *hdcp_message,
+			    u32 msg_size_in, u32 msg_size_out,
+			    u32 addr_out_off)
+{
+	struct xe_gt *gt = hdcp_message->hdcp_bo->tile->media_gt;
+	struct iosys_map *map = &hdcp_message->hdcp_bo->vmap;
+	struct xe_gsc *gsc = &gt->uc.gsc;
+	int ret;
+
+	ret = xe_gsc_pkt_submit_kernel(gsc, hdcp_message->hdcp_cmd_in,
msg_size_in,
+				       hdcp_message->hdcp_cmd_out,
msg_size_out);
+	if (ret) {
+		drm_err(&xe->drm, "failed to send gsc HDCP msg (%d)\n",
ret);
+		return ret;
+	}
+
+	if (xe_gsc_check_and_update_pending(xe, map, 0, map,
addr_out_off))
+		return -EAGAIN;
+
+	ret = xe_gsc_read_out_header(xe, map, addr_out_off,
+				     sizeof(struct hdcp_cmd_header), NULL);
+
+	return ret;
   }

   ssize_t intel_hdcp_gsc_msg_send(struct xe_device *xe, u8 *msg_in,
   				size_t msg_in_len, u8 *msg_out,
   				size_t msg_out_len)
   {
-	return -ENODEV;
+	const size_t max_msg_size = PAGE_SIZE - HDCP_GSC_HEADER_SIZE;
+	struct intel_hdcp_gsc_message *hdcp_message;
+	u64 host_session_id;
+	u32 msg_size_in, msg_size_out;
+	u32 addr_out_off, addr_in_wr_off = 0;
+	int ret, tries = 0;
+
+	if (msg_in_len > max_msg_size || msg_out_len > max_msg_size) {
+		ret = -ENOSPC;
+		goto out;
+	}
+
+	msg_size_in = msg_in_len + HDCP_GSC_HEADER_SIZE;
+	msg_size_out = msg_out_len + HDCP_GSC_HEADER_SIZE;
+	hdcp_message = xe->display.hdcp.hdcp_message;
+	addr_out_off = PAGE_SIZE;
+
+	host_session_id = xe_gsc_create_host_session_id();
+	xe_device_mem_access_get(xe);
+	addr_in_wr_off = xe_gsc_emit_header(xe, &hdcp_message-
hdcp_bo->vmap,
+					    addr_in_wr_off,
HECI_MEADDRESS_HDCP,
+					    host_session_id, msg_in_len);
+	xe_map_memcpy_to(xe, &hdcp_message->hdcp_bo->vmap,
addr_in_wr_off,
+			 msg_in, msg_in_len);
+	/*
+	 * Keep sending request in case the pending bit is set no need to add
+	 * message handle as we are using same address hence loc. of header
is
+	 * same and it will contain the message handle. we will send the
message
+	 * 20 times each message 50 ms apart
+	 */
+	do {
+		ret = xe_gsc_send_sync(xe, hdcp_message, msg_size_in,
msg_size_out,
+				       addr_out_off);
+
+		/* Only try again if gsc says so */
+		if (ret != -EAGAIN)
+			break;
+
+		msleep(50);
+
+	} while (++tries < 20);
+
+	if (ret)
+		goto out;
+
+	xe_map_memcpy_from(xe, msg_out, &hdcp_message->hdcp_bo-
vmap,
+			   addr_out_off + HDCP_GSC_HEADER_SIZE,
+			   msg_out_len);
+
+out:
+	xe_device_mem_access_put(xe);
+	return ret;
   }
diff --git a/drivers/gpu/drm/xe/xe_gsc_submit.c
b/drivers/gpu/drm/xe/xe_gsc_submit.c
index 348994b271be..9a18f5667db3 100644
--- a/drivers/gpu/drm/xe/xe_gsc_submit.c
+++ b/drivers/gpu/drm/xe/xe_gsc_submit.c
@@ -40,6 +40,21 @@ gsc_to_gt(struct xe_gsc *gsc)
   	return container_of(gsc, struct xe_gt, uc.gsc);
   }

+/**
+ * xe_gsc_get_host_session_id - Creates a random 64 bit host_session
+id with
+ * bits 56-63 masked.
+ *
+ * Returns: random host_session_id which can be used to send messages
+to gsc cs  */
+u64 xe_gsc_create_host_session_id(void)
+{
+	u64 host_session_id;
+
+	get_random_bytes(&host_session_id, sizeof(u64));
+	host_session_id &= ~HOST_SESSION_CLIENT_MASK;
+	return host_session_id;
+}
+
   /**
    * xe_gsc_emit_header - write the MTL GSC header in memory
    * @xe: the Xe device
diff --git a/drivers/gpu/drm/xe/xe_gsc_submit.h
b/drivers/gpu/drm/xe/xe_gsc_submit.h
index 1939855031a6..1416b5745a4c 100644
--- a/drivers/gpu/drm/xe/xe_gsc_submit.h
+++ b/drivers/gpu/drm/xe/xe_gsc_submit.h
@@ -28,4 +28,5 @@ int xe_gsc_read_out_header(struct xe_device *xe,
   int xe_gsc_pkt_submit_kernel(struct xe_gsc *gsc, u64 addr_in, u32 size_in,
   			     u64 addr_out, u32 size_out);

+u64 xe_gsc_create_host_session_id(void);
   #endif




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux