diff mbox series

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

Message ID 20200111231114.59208-4-michal.wajdeczko@intel.com (mailing list archive)
State New, archived
Headers show
Series Misc GuC CT improvements | expand

Commit Message

Michal Wajdeczko Jan. 11, 2020, 11:11 p.m. UTC 
Update GuC CTB action helpers to benefit from new CT_ERROR macro.

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

Comments

Daniele Ceraolo Spurio Jan. 13, 2020, 10:25 p.m. UTC | #1 
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@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> ---
>   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@intel.com>

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);
>   	}
>   }
>
diff mbox series

Patch

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;
 }
 
-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);
 	}
 }