Re: [PATCH 3/4] drm/i915/guc: Update CTB helpers to use CT_ERROR

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

 





On 1/11/2020 3:11 PM, Michal Wajdeczko wrote:
Update GuC CTB action helpers to benefit from new CT_ERROR macro.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
---
  drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 57 ++++++++++++-----------
  1 file changed, 31 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
index eb123543392a..1da69425029b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -107,31 +107,40 @@ static int guc_action_register_ct_buffer(struct intel_guc *guc,
  		sizeof(struct guc_ct_buffer_desc),
  		type
  	};
-	int err;
/* Can't use generic send(), CT registration must go over MMIO */
-	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
-	if (err)
-		DRM_ERROR("CT: register %s buffer failed; err=%d\n",
-			  guc_ct_buffer_type_to_str(type), err);
+	return intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
+}
+
+static int ct_register_buffer(struct intel_guc_ct *ct, u32 desc_addr, u32 type)
+{
+	int err = guc_action_register_ct_buffer(ct_to_guc(ct), desc_addr, type);
+
+	if (unlikely(err))
+		CT_ERROR(ct, "Failed to register %s buffer (err=%d)\n",
+			 guc_ct_buffer_type_to_str(type), err);
  	return err;
  }

Now that we're passing ct to this I'm wondering if it makes sense to move some of the math from outside in here, e.g:

const u32 i915_ct_to_guc_ct_type [] = {
	[CTB_RECV] = INTEL_GUC_CT_BUFFER_TYPE_RECV;
	[CTB_SEND] = INTEL_GUC_CT_BUFFER_TYPE_SEND;
}

#define CT_DESC_OFFSET 0
#define CT_DESC_SIZE (PAGE_SIZE / 4)
#define CT_BUF_OFFSET (PAGE_SIZE / 2)
#define CT_BUF_SIZE (PAGE_SIZE / 4)
static int ct_register_buffer(struct intel_guc_ct *ct, u32 type)
{
	struct intel_guc *guc = ct_to_guc(ct);
	u32 base = intel_guc_ggtt_offset(guc, ct->vma);
	u32 desc_addr = base + CT_DESC_OFFSET + CT_DESC_SIZE * type;
	u32 buf_addr = base + CT_BUF_OFFSET + CT_BUF_SIZE * type;
	int err;

	ct_buffer_desc_init(ct->ctbs[type].desc, buf_addr, CT_BUF_SIZE);

	err = guc_action_register_ct_buffer(guc, desc_addr,
					    i915_ct_to_guc_ct_type[type]);
	if (unlikely(err))
		CT_ERROR(ct, "Failed to register %s buffer (err=%d)\n",
			 guc_ct_buffer_type_to_str(type), err);
 	return err;
}


And then just call:

     err = ct_register_buffer(ct, CTB_RECV);


Or maybe it's overkill?
With or without the above changes:

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>

Daniele

-static int guc_action_deregister_ct_buffer(struct intel_guc *guc,
-					   u32 type)
+static int guc_action_deregister_ct_buffer(struct intel_guc *guc, u32 type)
  {
  	u32 action[] = {
  		INTEL_GUC_ACTION_DEREGISTER_COMMAND_TRANSPORT_BUFFER,
  		CTB_OWNER_HOST,
  		type
  	};
-	int err;
/* Can't use generic send(), CT deregistration must go over MMIO */
-	err = intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
-	if (err)
-		DRM_ERROR("CT: deregister %s buffer failed; err=%d\n",
-			  guc_ct_buffer_type_to_str(type), err);
+	return intel_guc_send_mmio(guc, action, ARRAY_SIZE(action), NULL, 0);
+}
+
+static int ct_deregister_buffer(struct intel_guc_ct *ct, u32 type)
+{
+	int err = guc_action_deregister_ct_buffer(ct_to_guc(ct), type);
+
+	if (unlikely(err))
+		CT_ERROR(ct, "Failed to deregister %s buffer (err=%d)\n",
+			 guc_ct_buffer_type_to_str(type), err);
  	return err;
  }
@@ -235,18 +244,17 @@ int intel_guc_ct_enable(struct intel_guc_ct *ct)
  					PAGE_SIZE/4);
  	}
- /* register buffers, starting wirh RECV buffer
-	 * descriptors are in first half of the blob
+	/*
+	 * Register both CT buffers starting with RECV buffer.
+	 * Descriptors are in first half of the blob.
  	 */
-	err = guc_action_register_ct_buffer(guc,
-					    base + PAGE_SIZE/4 * CTB_RECV,
-					    INTEL_GUC_CT_BUFFER_TYPE_RECV);
+	err = ct_register_buffer(ct, base + PAGE_SIZE/4 * CTB_RECV,
+				 INTEL_GUC_CT_BUFFER_TYPE_RECV);
  	if (unlikely(err))
  		goto err_out;
- err = guc_action_register_ct_buffer(guc,
-					    base + PAGE_SIZE/4 * CTB_SEND,
-					    INTEL_GUC_CT_BUFFER_TYPE_SEND);
+	err = ct_register_buffer(ct, base + PAGE_SIZE/4 * CTB_SEND,
+				 INTEL_GUC_CT_BUFFER_TYPE_SEND);
  	if (unlikely(err))
  		goto err_deregister;
@@ -255,8 +263,7 @@ int intel_guc_ct_enable(struct intel_guc_ct *ct)
  	return 0;
err_deregister:
-	guc_action_deregister_ct_buffer(guc,
-					INTEL_GUC_CT_BUFFER_TYPE_RECV);
+	ct_deregister_buffer(ct, INTEL_GUC_CT_BUFFER_TYPE_RECV);
  err_out:
  	CT_ERROR(ct, "Failed to open open CT channel (err=%d)\n", err);
  	return err;
@@ -275,10 +282,8 @@ void intel_guc_ct_disable(struct intel_guc_ct *ct)
  	ct->enabled = false;
if (intel_guc_is_running(guc)) {
-		guc_action_deregister_ct_buffer(guc,
-						INTEL_GUC_CT_BUFFER_TYPE_SEND);
-		guc_action_deregister_ct_buffer(guc,
-						INTEL_GUC_CT_BUFFER_TYPE_RECV);
+		ct_deregister_buffer(ct, INTEL_GUC_CT_BUFFER_TYPE_SEND);
+		ct_deregister_buffer(ct, INTEL_GUC_CT_BUFFER_TYPE_RECV);
  	}
  }

_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




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

  Powered by Linux