[Intel-gfx] [PATCH 0/2] Use different intel_hdcp_gsc_message instances

[Intel-gfx] [PATCH 0/2] Use different intel_hdcp_gsc_message instances

[Suraj Kandpal]

Use different intel_hdcp_gsc_message instances to send
and receive the messages from gsc since there are chances
using the same instance can cause corruption of data.

Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>

Suraj Kandpal (2):
  drm/i915/hdcp: Create hdcp_gsc_message in and out
  drm/i915/hdcp: Fill in hdcp_gsc_out message

 .../gpu/drm/i915/display/intel_display_core.h |  3 +-
 drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 98 ++++++++++++-------
 2 files changed, 64 insertions(+), 37 deletions(-)

-- 
2.25.1

[Intel-gfx] [PATCH 1/2] drm/i915/hdcp: Create hdcp_gsc_message in and out

[Suraj Kandpal]

Add hdcp_gsc_message_in and hdcp_gsc_message_out to help
differenctiate the reply given by gsc to avoid any kind of
message corruption due message structure reuse.
hdcp_gsc_message_out will be filled in upcoming patches

Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 .../gpu/drm/i915/display/intel_display_core.h |  3 +-
 drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 41 +++++++++++++------
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
index e36f88a39b86..ead16d341f5c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_core.h
+++ b/drivers/gpu/drm/i915/display/intel_display_core.h
@@ -403,7 +403,8 @@ struct intel_display {
 		 * reused when sending message to gsc cs.
 		 * this is only populated post Meteorlake
 		 */
-		struct intel_hdcp_gsc_message *hdcp_message;
+		struct intel_hdcp_gsc_message *hdcp_message_in;
+		struct intel_hdcp_gsc_message *hdcp_message_out;
 		/* Mutex to protect the above hdcp component related values. */
 		struct mutex comp_mutex;
 	} hdcp;
diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
index 7e52aea6aa17..be505b2d679e 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
@@ -665,34 +665,51 @@ static int intel_hdcp_gsc_initialize_message(struct drm_i915_private *i915,
 
 static int intel_hdcp_gsc_hdcp2_init(struct drm_i915_private *i915)
 {
-	struct intel_hdcp_gsc_message *hdcp_message;
+	struct intel_hdcp_gsc_message *hdcp_message_in, *hdcp_message_out;
 	int ret;
 
-	hdcp_message = kzalloc(sizeof(*hdcp_message), GFP_KERNEL);
+	hdcp_message_in = kzalloc(sizeof(*hdcp_message_in), GFP_KERNEL);
 
-	if (!hdcp_message)
+	if (!hdcp_message_in)
 		return -ENOMEM;
 
+	hdcp_message_out = kzalloc(sizeof(*hdcp_message_out), GFP_KERNEL);
+
+	if (!hdcp_message_out)
+		return -ENOMEM;
 	/*
 	 * NOTE: No need to lock the comp mutex here as it is already
 	 * going to be taken before this function called
 	 */
-	i915->display.hdcp.hdcp_message = hdcp_message;
-	ret = intel_hdcp_gsc_initialize_message(i915, hdcp_message);
+	i915->display.hdcp.hdcp_message_in = hdcp_message_in;
+	ret = intel_hdcp_gsc_initialize_message(i915, hdcp_message_in);
+
+	if (ret) {
+		drm_err(&i915->drm, "Could not initialize hdcp_message_in\n");
+		goto out;
+	}
+
+	i915->display.hdcp.hdcp_message_out = hdcp_message_out;
+	ret = intel_hdcp_gsc_initialize_message(i915, hdcp_message_out);
 
 	if (ret)
-		drm_err(&i915->drm, "Could not initialize hdcp_message\n");
+		drm_err(&i915->drm, "Could not initialize hdcp_message_out\n");
 
+out:
 	return ret;
 }
 
 static void intel_hdcp_gsc_free_message(struct drm_i915_private *i915)
 {
-	struct intel_hdcp_gsc_message *hdcp_message =
-					i915->display.hdcp.hdcp_message;
-
-	i915_vma_unpin_and_release(&hdcp_message->vma, I915_VMA_RELEASE_MAP);
-	kfree(hdcp_message);
+	struct intel_hdcp_gsc_message *hdcp_message_in =
+					i915->display.hdcp.hdcp_message_in;
+	struct intel_hdcp_gsc_message *hdcp_message_out =
+					i915->display.hdcp.hdcp_message_out;
+
+	i915_vma_unpin_and_release(&hdcp_message_in->vma, I915_VMA_RELEASE_MAP);
+	i915_vma_unpin_and_release(&hdcp_message_out->vma, I915_VMA_RELEASE_MAP);
+	kfree(hdcp_message_in);
+	kfree(hdcp_message_out);
 }
 
 int intel_hdcp_gsc_init(struct drm_i915_private *i915)
@@ -782,7 +799,7 @@ ssize_t intel_hdcp_gsc_msg_send(struct drm_i915_private *i915, u8 *msg_in,
 	if (msg_in_len > max_msg_size || msg_out_len > max_msg_size)
 		return -ENOSPC;
 
-	hdcp_message = i915->display.hdcp.hdcp_message;
+	hdcp_message = i915->display.hdcp.hdcp_message_in;
 	header = hdcp_message->hdcp_cmd;
 	addr = i915_ggtt_offset(hdcp_message->vma);
 
-- 
2.25.1

[Intel-gfx] [PATCH 2/2] drm/i915/hdcp: Fill in hdcp_gsc_out message

[Suraj Kandpal]

Fill out hdcp_gsc_message_in and hdcp_gsc_message_out structure
which also includes differentiating header of both messages
using header_in and header_out.

Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 59 +++++++++++--------
 1 file changed, 34 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
index be505b2d679e..ab47724f7f05 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
@@ -738,38 +738,42 @@ void intel_hdcp_gsc_fini(struct drm_i915_private *i915)
 }
 
 static int intel_gsc_send_sync(struct drm_i915_private *i915,
-			       struct intel_gsc_mtl_header *header, u64 addr,
+			       struct intel_gsc_mtl_header *header_in,
+			       struct intel_gsc_mtl_header *header_out,
+			       u64 addr_in, u64 addr_out,
 			       size_t msg_out_len)
 {
 	struct intel_gt *gt = i915->media_gt;
 	int ret;
 
-	header->flags = 0;
-	ret = intel_gsc_uc_heci_cmd_submit_packet(&gt->uc.gsc, addr,
-						  header->message_size,
-						  addr,
-						  msg_out_len + sizeof(*header));
+	ret = intel_gsc_uc_heci_cmd_submit_packet(&gt->uc.gsc, addr_in,
+						  header_in->message_size,
+						  addr_out,
+						  msg_out_len + sizeof(*header_in));
 	if (ret) {
 		drm_err(&i915->drm, "failed to send gsc HDCP msg (%d)\n", ret);
 		return ret;
 	}
 
 	/*
-	 * Checking validity marker for memory sanity
+	 * Check validity marker and status to see if some error is
+	 * blocking SW to FW communication
 	 */
-	if (header->validity_marker != GSC_HECI_VALIDITY_MARKER) {
+	if (header_out->validity_marker != GSC_HECI_VALIDITY_MARKER) {
 		drm_err(&i915->drm, "invalid validity marker\n");
 		return -EINVAL;
 	}
 
-	if (header->status != 0) {
+	if (header_out->status != 0) {
 		drm_err(&i915->drm, "header status indicates error %d\n",
-			header->status);
+			header_out->status);
 		return -EINVAL;
 	}
 
-	if (header->flags & GSC_OUTFLAG_MSG_PENDING)
+	if (header_out->flags & GSC_OUTFLAG_MSG_PENDING) {
+		header_in->gsc_message_handle = header_out->gsc_message_handle;
 		return -EAGAIN;
+	}
 
 	return 0;
 }
@@ -786,10 +790,10 @@ ssize_t intel_hdcp_gsc_msg_send(struct drm_i915_private *i915, u8 *msg_in,
 				size_t msg_out_len)
 {
 	struct intel_gt *gt = i915->media_gt;
-	struct intel_gsc_mtl_header *header;
-	const size_t max_msg_size = PAGE_SIZE - sizeof(*header);
-	struct intel_hdcp_gsc_message *hdcp_message;
-	u64 addr, host_session_id;
+	struct intel_gsc_mtl_header *header_in, *header_out;
+	const size_t max_msg_size = PAGE_SIZE - sizeof(*header_in);
+	struct intel_hdcp_gsc_message *hdcp_message_in, *hdcp_message_out;
+	u64 addr_in, addr_out, host_session_id;
 	u32 reply_size, msg_size;
 	int ret, tries = 0;
 
@@ -799,16 +803,20 @@ ssize_t intel_hdcp_gsc_msg_send(struct drm_i915_private *i915, u8 *msg_in,
 	if (msg_in_len > max_msg_size || msg_out_len > max_msg_size)
 		return -ENOSPC;
 
-	hdcp_message = i915->display.hdcp.hdcp_message_in;
-	header = hdcp_message->hdcp_cmd;
-	addr = i915_ggtt_offset(hdcp_message->vma);
+	hdcp_message_in = i915->display.hdcp.hdcp_message_in;
+	hdcp_message_out = i915->display.hdcp.hdcp_message_out;
+	header_in = hdcp_message_in->hdcp_cmd;
+	header_out = hdcp_message_out->hdcp_cmd;
+	addr_in = i915_ggtt_offset(hdcp_message_in->vma);
+	addr_out = i915_ggtt_offset(hdcp_message_out->vma);
 
-	msg_size = msg_in_len + sizeof(*header);
-	memset(header, 0, msg_size);
+	msg_size = msg_in_len + sizeof(*header_in);
+	memset(header_in, 0, msg_size);
+	memset(header_out, 0, msg_size);
 	get_random_bytes(&host_session_id, sizeof(u64));
-	intel_gsc_uc_heci_cmd_emit_mtl_header(header, HECI_MEADDRESS_HDCP,
+	intel_gsc_uc_heci_cmd_emit_mtl_header(header_in, HECI_MEADDRESS_HDCP,
 					      msg_size, host_session_id);
-	memcpy(hdcp_message->hdcp_cmd + sizeof(*header), msg_in, msg_in_len);
+	memcpy(hdcp_message_in->hdcp_cmd + sizeof(*header_in), msg_in, msg_in_len);
 
 	/*
 	 * Keep sending request in case the pending bit is set no need to add
@@ -817,7 +825,8 @@ ssize_t intel_hdcp_gsc_msg_send(struct drm_i915_private *i915, u8 *msg_in,
 	 * 20 times each message 50 ms apart
 	 */
 	do {
-		ret = intel_gsc_send_sync(i915, header, addr, msg_out_len);
+		ret = intel_gsc_send_sync(i915, header_in, header_out,  addr_in,
+					  addr_out, msg_out_len);
 
 		/* Only try again if gsc says so */
 		if (ret != -EAGAIN)
@@ -831,7 +840,7 @@ ssize_t intel_hdcp_gsc_msg_send(struct drm_i915_private *i915, u8 *msg_in,
 		goto err;
 
 	/* we use the same mem for the reply, so header is in the same loc */
-	reply_size = header->message_size - sizeof(*header);
+	reply_size = header_out->message_size - sizeof(*header_out);
 	if (reply_size > msg_out_len) {
 		drm_warn(&i915->drm, "caller with insufficient HDCP reply size %u (%d)\n",
 			 reply_size, (u32)msg_out_len);
@@ -841,7 +850,7 @@ ssize_t intel_hdcp_gsc_msg_send(struct drm_i915_private *i915, u8 *msg_in,
 			    reply_size, (u32)msg_out_len);
 	}
 
-	memcpy(msg_out, hdcp_message->hdcp_cmd + sizeof(*header), msg_out_len);
+	memcpy(msg_out, hdcp_message_out->hdcp_cmd + sizeof(*header_out), msg_out_len);
 
 err:
 	return ret;
-- 
2.25.1

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Use different intel_hdcp_gsc_message instances

[Patchwork]

== Series Details ==

Series: Use different intel_hdcp_gsc_message instances
URL   : https://patchwork.freedesktop.org/series/118009/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

[Intel-gfx] ✗ Fi.CI.BAT: failure for Use different intel_hdcp_gsc_message instances

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 9807 bytes --]

== Series Details ==

Series: Use different intel_hdcp_gsc_message instances
URL   : https://patchwork.freedesktop.org/series/118009/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13165 -> Patchwork_118009v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_118009v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_118009v1, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/index.html

Participating hosts (37 -> 38)
------------------------------

  Additional (2): bat-rpls-2 bat-mtlp-8 
  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_118009v1:

### IGT changes ###

#### Possible regressions ####

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-a-edp-1:
    - bat-adlp-6:         NOTRUN -> [ABORT][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-adlp-6/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-a-edp-1.html

  
#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * igt@i915_selftest@live@migrate:
    - {bat-mtlp-8}:       NOTRUN -> [DMESG-FAIL][2]
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-mtlp-8/igt@i915_selftest@live@migrate.html

  
Known issues
------------

  Here are the changes found in Patchwork_118009v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@debugfs_test@basic-hwmon:
    - bat-rpls-2:         NOTRUN -> [SKIP][3] ([i915#7456])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@debugfs_test@basic-hwmon.html

  * igt@fbdev@read:
    - bat-rpls-2:         NOTRUN -> [SKIP][4] ([i915#2582]) +4 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@fbdev@read.html

  * igt@gem_lmem_swapping@verify-random:
    - bat-rpls-2:         NOTRUN -> [SKIP][5] ([i915#4613]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@gem_lmem_swapping@verify-random.html

  * igt@gem_tiled_pread_basic:
    - bat-rpls-2:         NOTRUN -> [SKIP][6] ([i915#3282])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@gem_tiled_pread_basic.html

  * igt@i915_pm_backlight@basic-brightness:
    - bat-rpls-2:         NOTRUN -> [SKIP][7] ([i915#7561])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@i915_pm_backlight@basic-brightness.html

  * igt@i915_pm_rps@basic-api:
    - bat-rpls-2:         NOTRUN -> [SKIP][8] ([i915#6621])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@i915_pm_rps@basic-api.html

  * igt@i915_selftest@live@gt_pm:
    - bat-rpls-2:         NOTRUN -> [DMESG-FAIL][9] ([i915#4258] / [i915#7913])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@i915_selftest@live@gt_pm.html

  * igt@i915_selftest@live@slpc:
    - bat-rpls-2:         NOTRUN -> [DMESG-WARN][10] ([i915#6367])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@i915_selftest@live@slpc.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - bat-rpls-2:         NOTRUN -> [ABORT][11] ([i915#6687])
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@i915_suspend@basic-s2idle-without-i915.html

  * igt@kms_busy@basic:
    - bat-rpls-2:         NOTRUN -> [SKIP][12] ([i915#1845]) +14 similar issues
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@kms_busy@basic.html

  * igt@kms_chamelium_edid@hdmi-edid-read:
    - bat-rpls-2:         NOTRUN -> [SKIP][13] ([i915#7828]) +7 similar issues
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@kms_chamelium_edid@hdmi-edid-read.html

  * igt@kms_flip@basic-flip-vs-dpms:
    - bat-rpls-2:         NOTRUN -> [SKIP][14] ([i915#3637]) +3 similar issues
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@kms_flip@basic-flip-vs-dpms.html

  * igt@kms_force_connector_basic@force-load-detect:
    - bat-rpls-2:         NOTRUN -> [SKIP][15] ([fdo#109285])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_frontbuffer_tracking@basic:
    - bat-rpls-2:         NOTRUN -> [SKIP][16] ([i915#1849])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
    - bat-dg2-11:         NOTRUN -> [SKIP][17] ([i915#1845] / [i915#5354]) +2 similar issues
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html

  * igt@kms_psr@sprite_plane_onoff:
    - bat-rpls-2:         NOTRUN -> [SKIP][18] ([i915#1072]) +3 similar issues
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@kms_psr@sprite_plane_onoff.html

  * igt@kms_setmode@basic-clone-single-crtc:
    - bat-rpls-2:         NOTRUN -> [SKIP][19] ([i915#3555] / [i915#4579])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@kms_setmode@basic-clone-single-crtc.html

  * igt@prime_vgem@basic-fence-flip:
    - bat-rpls-2:         NOTRUN -> [SKIP][20] ([fdo#109295] / [i915#1845] / [i915#3708])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@prime_vgem@basic-fence-flip.html

  * igt@prime_vgem@basic-fence-read:
    - bat-rpls-2:         NOTRUN -> [SKIP][21] ([fdo#109295] / [i915#3708]) +2 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-2/igt@prime_vgem@basic-fence-read.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@gt_engines:
    - bat-atsm-1:         [FAIL][22] ([i915#6268]) -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13165/bat-atsm-1/igt@i915_selftest@live@gt_engines.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-atsm-1/igt@i915_selftest@live@gt_engines.html

  * igt@i915_selftest@live@slpc:
    - bat-rpls-1:         [DMESG-WARN][24] ([i915#6367]) -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13165/bat-rpls-1/igt@i915_selftest@live@slpc.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/bat-rpls-1/igt@i915_selftest@live@slpc.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109295]: https://bugs.freedesktop.org/show_bug.cgi?id=109295
  [i915#1072]: https://gitlab.freedesktop.org/drm/intel/issues/1072
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#1849]: https://gitlab.freedesktop.org/drm/intel/issues/1849
  [i915#2582]: https://gitlab.freedesktop.org/drm/intel/issues/2582
  [i915#3282]: https://gitlab.freedesktop.org/drm/intel/issues/3282
  [i915#3555]: https://gitlab.freedesktop.org/drm/intel/issues/3555
  [i915#3595]: https://gitlab.freedesktop.org/drm/intel/issues/3595
  [i915#3637]: https://gitlab.freedesktop.org/drm/intel/issues/3637
  [i915#3708]: https://gitlab.freedesktop.org/drm/intel/issues/3708
  [i915#4077]: https://gitlab.freedesktop.org/drm/intel/issues/4077
  [i915#4079]: https://gitlab.freedesktop.org/drm/intel/issues/4079
  [i915#4083]: https://gitlab.freedesktop.org/drm/intel/issues/4083
  [i915#4212]: https://gitlab.freedesktop.org/drm/intel/issues/4212
  [i915#4213]: https://gitlab.freedesktop.org/drm/intel/issues/4213
  [i915#4258]: https://gitlab.freedesktop.org/drm/intel/issues/4258
  [i915#4579]: https://gitlab.freedesktop.org/drm/intel/issues/4579
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#5190]: https://gitlab.freedesktop.org/drm/intel/issues/5190
  [i915#5274]: https://gitlab.freedesktop.org/drm/intel/issues/5274
  [i915#5354]: https://gitlab.freedesktop.org/drm/intel/issues/5354
  [i915#6268]: https://gitlab.freedesktop.org/drm/intel/issues/6268
  [i915#6367]: https://gitlab.freedesktop.org/drm/intel/issues/6367
  [i915#6621]: https://gitlab.freedesktop.org/drm/intel/issues/6621
  [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645
  [i915#6687]: https://gitlab.freedesktop.org/drm/intel/issues/6687
  [i915#7456]: https://gitlab.freedesktop.org/drm/intel/issues/7456
  [i915#7561]: https://gitlab.freedesktop.org/drm/intel/issues/7561
  [i915#7828]: https://gitlab.freedesktop.org/drm/intel/issues/7828
  [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913


Build changes
-------------

  * Linux: CI_DRM_13165 -> Patchwork_118009v1

  CI-20190529: 20190529
  CI_DRM_13165: b5b59a92a8b9df8696f50cae4cea1635e5f8dc16 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7296: f58eaf30c30c1cc9f00c8b5c596ee5c94d054198 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_118009v1: b5b59a92a8b9df8696f50cae4cea1635e5f8dc16 @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

aeb6202ccc76 drm/i915/hdcp: Fill in hdcp_gsc_out message
193869e9315b drm/i915/hdcp: Create hdcp_gsc_message in and out

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_118009v1/index.html

[-- Attachment #2: Type: text/html, Size: 10753 bytes --]

Re: [Intel-gfx] [PATCH 1/2] drm/i915/hdcp: Create hdcp_gsc_message in and out

[Ceraolo Spurio, Daniele]

On 5/19/2023 2:49 AM, Suraj Kandpal wrote:
> Add hdcp_gsc_message_in and hdcp_gsc_message_out to help
> differenctiate the reply given by gsc to avoid any kind of
> message corruption due message structure reuse.
> hdcp_gsc_message_out will be filled in upcoming patches

Generic question on the approach: for both PXP and GSC proxy, we 
allocate a single multi-page object and use the first half for input and 
the second half output. This makes things simpler because we don't need 
to allocate, map and then cleanup 2 separate objects. Any reason not to 
follow a similar approach here?

Daniele

>
> Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> ---
>   .../gpu/drm/i915/display/intel_display_core.h |  3 +-
>   drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 41 +++++++++++++------
>   2 files changed, 31 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h b/drivers/gpu/drm/i915/display/intel_display_core.h
> index e36f88a39b86..ead16d341f5c 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> @@ -403,7 +403,8 @@ struct intel_display {
>   		 * reused when sending message to gsc cs.
>   		 * this is only populated post Meteorlake
>   		 */
> -		struct intel_hdcp_gsc_message *hdcp_message;
> +		struct intel_hdcp_gsc_message *hdcp_message_in;
> +		struct intel_hdcp_gsc_message *hdcp_message_out;
>   		/* Mutex to protect the above hdcp component related values. */
>   		struct mutex comp_mutex;
>   	} hdcp;
> diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> index 7e52aea6aa17..be505b2d679e 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> @@ -665,34 +665,51 @@ static int intel_hdcp_gsc_initialize_message(struct drm_i915_private *i915,
>   
>   static int intel_hdcp_gsc_hdcp2_init(struct drm_i915_private *i915)
>   {
> -	struct intel_hdcp_gsc_message *hdcp_message;
> +	struct intel_hdcp_gsc_message *hdcp_message_in, *hdcp_message_out;
>   	int ret;
>   
> -	hdcp_message = kzalloc(sizeof(*hdcp_message), GFP_KERNEL);
> +	hdcp_message_in = kzalloc(sizeof(*hdcp_message_in), GFP_KERNEL);
>   
> -	if (!hdcp_message)
> +	if (!hdcp_message_in)
>   		return -ENOMEM;
>   
> +	hdcp_message_out = kzalloc(sizeof(*hdcp_message_out), GFP_KERNEL);
> +
> +	if (!hdcp_message_out)
> +		return -ENOMEM;
>   	/*
>   	 * NOTE: No need to lock the comp mutex here as it is already
>   	 * going to be taken before this function called
>   	 */
> -	i915->display.hdcp.hdcp_message = hdcp_message;
> -	ret = intel_hdcp_gsc_initialize_message(i915, hdcp_message);
> +	i915->display.hdcp.hdcp_message_in = hdcp_message_in;
> +	ret = intel_hdcp_gsc_initialize_message(i915, hdcp_message_in);
> +
> +	if (ret) {
> +		drm_err(&i915->drm, "Could not initialize hdcp_message_in\n");
> +		goto out;
> +	}
> +
> +	i915->display.hdcp.hdcp_message_out = hdcp_message_out;
> +	ret = intel_hdcp_gsc_initialize_message(i915, hdcp_message_out);
>   
>   	if (ret)
> -		drm_err(&i915->drm, "Could not initialize hdcp_message\n");
> +		drm_err(&i915->drm, "Could not initialize hdcp_message_out\n");
>   
> +out:
>   	return ret;
>   }
>   
>   static void intel_hdcp_gsc_free_message(struct drm_i915_private *i915)
>   {
> -	struct intel_hdcp_gsc_message *hdcp_message =
> -					i915->display.hdcp.hdcp_message;
> -
> -	i915_vma_unpin_and_release(&hdcp_message->vma, I915_VMA_RELEASE_MAP);
> -	kfree(hdcp_message);
> +	struct intel_hdcp_gsc_message *hdcp_message_in =
> +					i915->display.hdcp.hdcp_message_in;
> +	struct intel_hdcp_gsc_message *hdcp_message_out =
> +					i915->display.hdcp.hdcp_message_out;
> +
> +	i915_vma_unpin_and_release(&hdcp_message_in->vma, I915_VMA_RELEASE_MAP);
> +	i915_vma_unpin_and_release(&hdcp_message_out->vma, I915_VMA_RELEASE_MAP);
> +	kfree(hdcp_message_in);
> +	kfree(hdcp_message_out);
>   }
>   
>   int intel_hdcp_gsc_init(struct drm_i915_private *i915)
> @@ -782,7 +799,7 @@ ssize_t intel_hdcp_gsc_msg_send(struct drm_i915_private *i915, u8 *msg_in,
>   	if (msg_in_len > max_msg_size || msg_out_len > max_msg_size)
>   		return -ENOSPC;
>   
> -	hdcp_message = i915->display.hdcp.hdcp_message;
> +	hdcp_message = i915->display.hdcp.hdcp_message_in;
>   	header = hdcp_message->hdcp_cmd;
>   	addr = i915_ggtt_offset(hdcp_message->vma);
>   

Re: [Intel-gfx] [PATCH 1/2] drm/i915/hdcp: Create hdcp_gsc_message in and out

[Kandpal, Suraj]

> On 5/19/2023 2:49 AM, Suraj Kandpal wrote:
> > Add hdcp_gsc_message_in and hdcp_gsc_message_out to help
> > differenctiate the reply given by gsc to avoid any kind of message
> > corruption due message structure reuse.
> > hdcp_gsc_message_out will be filled in upcoming patches
> 
> Generic question on the approach: for both PXP and GSC proxy, we allocate a
> single multi-page object and use the first half for input and the second half
> output. This makes things simpler because we don't need to allocate, map
> and then cleanup 2 separate objects. Any reason not to follow a similar
> approach here?
> 

Tbh I wasn't aware PXP and GSC Proxy were taking this approach maybe I too can modify
the code to use this approach, can you point me towards any reference function which I can have a look at.

Regards,
Suraj Kandpal

> Daniele
> 
> >
> > Cc: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> > Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
> > Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> > Signed-off-by: Suraj Kandpal <suraj.kandpal@intel.com>
> > ---
> >   .../gpu/drm/i915/display/intel_display_core.h |  3 +-
> >   drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 41 +++++++++++++------
> >   2 files changed, 31 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_core.h
> > b/drivers/gpu/drm/i915/display/intel_display_core.h
> > index e36f88a39b86..ead16d341f5c 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_core.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_core.h
> > @@ -403,7 +403,8 @@ struct intel_display {
> >   		 * reused when sending message to gsc cs.
> >   		 * this is only populated post Meteorlake
> >   		 */
> > -		struct intel_hdcp_gsc_message *hdcp_message;
> > +		struct intel_hdcp_gsc_message *hdcp_message_in;
> > +		struct intel_hdcp_gsc_message *hdcp_message_out;
> >   		/* Mutex to protect the above hdcp component related
> values. */
> >   		struct mutex comp_mutex;
> >   	} hdcp;
> > diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> > b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> > index 7e52aea6aa17..be505b2d679e 100644
> > --- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> > +++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
> > @@ -665,34 +665,51 @@ static int
> > intel_hdcp_gsc_initialize_message(struct drm_i915_private *i915,
> >
> >   static int intel_hdcp_gsc_hdcp2_init(struct drm_i915_private *i915)
> >   {
> > -	struct intel_hdcp_gsc_message *hdcp_message;
> > +	struct intel_hdcp_gsc_message *hdcp_message_in,
> *hdcp_message_out;
> >   	int ret;
> >
> > -	hdcp_message = kzalloc(sizeof(*hdcp_message), GFP_KERNEL);
> > +	hdcp_message_in = kzalloc(sizeof(*hdcp_message_in), GFP_KERNEL);
> >
> > -	if (!hdcp_message)
> > +	if (!hdcp_message_in)
> >   		return -ENOMEM;
> >
> > +	hdcp_message_out = kzalloc(sizeof(*hdcp_message_out),
> GFP_KERNEL);
> > +
> > +	if (!hdcp_message_out)
> > +		return -ENOMEM;
> >   	/*
> >   	 * NOTE: No need to lock the comp mutex here as it is already
> >   	 * going to be taken before this function called
> >   	 */
> > -	i915->display.hdcp.hdcp_message = hdcp_message;
> > -	ret = intel_hdcp_gsc_initialize_message(i915, hdcp_message);
> > +	i915->display.hdcp.hdcp_message_in = hdcp_message_in;
> > +	ret = intel_hdcp_gsc_initialize_message(i915, hdcp_message_in);
> > +
> > +	if (ret) {
> > +		drm_err(&i915->drm, "Could not initialize
> hdcp_message_in\n");
> > +		goto out;
> > +	}
> > +
> > +	i915->display.hdcp.hdcp_message_out = hdcp_message_out;
> > +	ret = intel_hdcp_gsc_initialize_message(i915, hdcp_message_out);
> >
> >   	if (ret)
> > -		drm_err(&i915->drm, "Could not initialize
> hdcp_message\n");
> > +		drm_err(&i915->drm, "Could not initialize
> hdcp_message_out\n");
> >
> > +out:
> >   	return ret;
> >   }
> >
> >   static void intel_hdcp_gsc_free_message(struct drm_i915_private *i915)
> >   {
> > -	struct intel_hdcp_gsc_message *hdcp_message =
> > -					i915->display.hdcp.hdcp_message;
> > -
> > -	i915_vma_unpin_and_release(&hdcp_message->vma,
> I915_VMA_RELEASE_MAP);
> > -	kfree(hdcp_message);
> > +	struct intel_hdcp_gsc_message *hdcp_message_in =
> > +					i915-
> >display.hdcp.hdcp_message_in;
> > +	struct intel_hdcp_gsc_message *hdcp_message_out =
> > +					i915-
> >display.hdcp.hdcp_message_out;
> > +
> > +	i915_vma_unpin_and_release(&hdcp_message_in->vma,
> I915_VMA_RELEASE_MAP);
> > +	i915_vma_unpin_and_release(&hdcp_message_out->vma,
> I915_VMA_RELEASE_MAP);
> > +	kfree(hdcp_message_in);
> > +	kfree(hdcp_message_out);
> >   }
> >
> >   int intel_hdcp_gsc_init(struct drm_i915_private *i915) @@ -782,7
> > +799,7 @@ ssize_t intel_hdcp_gsc_msg_send(struct drm_i915_private
> *i915, u8 *msg_in,
> >   	if (msg_in_len > max_msg_size || msg_out_len > max_msg_size)
> >   		return -ENOSPC;
> >
> > -	hdcp_message = i915->display.hdcp.hdcp_message;
> > +	hdcp_message = i915->display.hdcp.hdcp_message_in;
> >   	header = hdcp_message->hdcp_cmd;
> >   	addr = i915_ggtt_offset(hdcp_message->vma);
> >