Re: [Intel-gfx] [PATCH 2/2] drm/i915/hdcp: Modify intel_gsc_send_sync function

[Ceraolo Spurio, Daniele]

On 5/29/2023 4:49 AM, Suraj Kandpal wrote:
Modify intel_gsc_send_sync() to take into account header_out
and addr_out so as to use them to verify the message send status.

Cc: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com>
Cc: Alan Previn <alan.previn.teres.ale...@intel.com>
Cc: Ankit Nautiyal <ankit.k.nauti...@intel.com>
Signed-off-by: Suraj Kandpal <suraj.kand...@intel.com>

I think this patch should be squashed with the previous one, because if 
I understand correctly the code won't work with just the first one, 
which could be a problem with bisection. The changes themselves look ok 
to me though, so for the unified patch (with the small fix I pointed out 
in the other reply):

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospu...@intel.com>

Daniele

---
  drivers/gpu/drm/i915/display/intel_hdcp_gsc.c | 29 +++++++++++--------
  1 file changed, 17 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c 
b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
index 72d1e261d0a9..5f29c3c559fa 100644
--- a/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
+++ b/drivers/gpu/drm/i915/display/intel_hdcp_gsc.c
@@ -726,38 +726,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_out));
        if (ret) {
                drm_err(&i915->drm, "failed to send gsc HDCP msg (%d)\n", ret);
                return ret;
        }
  
  	/*
-        * Checking validity marker for memory sanity
+        * Checking validity marker and header status to see if some error has
+        * blocked us from sending message to gsc cs
         */
-       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;
  }
@@ -809,7 +813,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_in, addr_in, 
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)