diff mbox series

[v2,03/22] drm/i915/guc: Simplify preparation of GuC parameter block

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

Commit Message

Michal Wajdeczko April 11, 2019, 8:44 a.m. UTC 
Definition of the parameters block passed to GuC is about to change.
Slightly refactor code now to make upcoming patch smaller.

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: John Spotswood <john.a.spotswood@intel.com>
Reviewed-by: John Spotswood <john.a.spotswood@intel.com>
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_guc.c | 38 +++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 15 deletions(-)

Comments

Daniele Ceraolo Spurio April 15, 2019, 6:27 p.m. UTC | #1 
On 4/11/19 1:44 AM, Michal Wajdeczko wrote:
> Definition of the parameters block passed to GuC is about to change.
> Slightly refactor code now to make upcoming patch smaller.

I don't think this simplifies the upcoming patch (6/22) in any way, 
since most changes in that one are concentrated in the individual 
*_flags functions, which you're not touching here. The only related 
changes are the ones that the other patch does in guc_prepare_params, 
but those looks like they would be the same without splitting the 
function out.
Personally I would just drop this an keep intel_guc_init_params() as is, 
since it is clearer to follow IMO.

Daniele

> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: John Spotswood <john.a.spotswood@intel.com>
> Reviewed-by: John Spotswood <john.a.spotswood@intel.com>
> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_guc.c | 38 +++++++++++++++++++-------------
>   1 file changed, 23 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
> index 3aabfa2d9198..c0e8b359b23a 100644
> --- a/drivers/gpu/drm/i915/intel_guc.c
> +++ b/drivers/gpu/drm/i915/intel_guc.c
> @@ -333,19 +333,8 @@ static u32 guc_ctl_log_params_flags(struct intel_guc *guc)
>   	return flags;
>   }
>   
> -/*
> - * Initialise the GuC parameter block before starting the firmware
> - * transfer. These parameters are read by the firmware on startup
> - * and cannot be changed thereafter.
> - */
> -void intel_guc_init_params(struct intel_guc *guc)
> +static void guc_prepare_params(struct intel_guc *guc, u32 *params)
>   {
> -	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> -	u32 params[GUC_CTL_MAX_DWORDS];
> -	int i;
> -
> -	memset(params, 0, sizeof(params));
> -
>   	/*
>   	 * GuC ARAT increment is 10 ns. GuC default scheduler quantum is one
>   	 * second. This ARAR is calculated by:
> @@ -360,9 +349,12 @@ void intel_guc_init_params(struct intel_guc *guc)
>   	params[GUC_CTL_LOG_PARAMS]  = guc_ctl_log_params_flags(guc);
>   	params[GUC_CTL_DEBUG] = guc_ctl_debug_flags(guc);
>   	params[GUC_CTL_CTXINFO] = guc_ctl_ctxinfo_flags(guc);
> +}
>   
> -	for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
> -		DRM_DEBUG_DRIVER("param[%2d] = %#x\n", i, params[i]);
> +static void guc_write_params(struct intel_guc *guc, const u32 *params)
> +{
> +	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> +	int i;
>   
>   	/*
>   	 * All SOFT_SCRATCH registers are in FORCEWAKE_BLITTER domain and
> @@ -373,12 +365,28 @@ void intel_guc_init_params(struct intel_guc *guc)
>   
>   	I915_WRITE(SOFT_SCRATCH(0), 0);
>   
> -	for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
> +	for (i = 0; i < GUC_CTL_MAX_DWORDS; i++) {
> +		DRM_DEBUG_DRIVER("param[%2d] = %#x\n", i, params[i]);
>   		I915_WRITE(SOFT_SCRATCH(1 + i), params[i]);
> +	}
>   
>   	intel_uncore_forcewake_put(&dev_priv->uncore, FORCEWAKE_BLITTER);
>   }
>   
> +/*
> + * Initialise the GuC parameter block before starting the firmware
> + * transfer. These parameters are read by the firmware on startup
> + * and cannot be changed thereafter.
> + */
> +void intel_guc_init_params(struct intel_guc *guc)
> +{
> +	u32 params[GUC_CTL_MAX_DWORDS];
> +
> +	memset(params, 0, sizeof(params));
> +	guc_prepare_params(guc, params);
> +	guc_write_params(guc, params);
> +}
> +
>   int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
>   		       u32 *response_buf, u32 response_buf_size)
>   {
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/intel_guc.c b/drivers/gpu/drm/i915/intel_guc.c
index 3aabfa2d9198..c0e8b359b23a 100644
--- a/drivers/gpu/drm/i915/intel_guc.c
+++ b/drivers/gpu/drm/i915/intel_guc.c
@@ -333,19 +333,8 @@  static u32 guc_ctl_log_params_flags(struct intel_guc *guc)
 	return flags;
 }
 
-/*
- * Initialise the GuC parameter block before starting the firmware
- * transfer. These parameters are read by the firmware on startup
- * and cannot be changed thereafter.
- */
-void intel_guc_init_params(struct intel_guc *guc)
+static void guc_prepare_params(struct intel_guc *guc, u32 *params)
 {
-	struct drm_i915_private *dev_priv = guc_to_i915(guc);
-	u32 params[GUC_CTL_MAX_DWORDS];
-	int i;
-
-	memset(params, 0, sizeof(params));
-
 	/*
 	 * GuC ARAT increment is 10 ns. GuC default scheduler quantum is one
 	 * second. This ARAR is calculated by:
@@ -360,9 +349,12 @@  void intel_guc_init_params(struct intel_guc *guc)
 	params[GUC_CTL_LOG_PARAMS]  = guc_ctl_log_params_flags(guc);
 	params[GUC_CTL_DEBUG] = guc_ctl_debug_flags(guc);
 	params[GUC_CTL_CTXINFO] = guc_ctl_ctxinfo_flags(guc);
+}
 
-	for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
-		DRM_DEBUG_DRIVER("param[%2d] = %#x\n", i, params[i]);
+static void guc_write_params(struct intel_guc *guc, const u32 *params)
+{
+	struct drm_i915_private *dev_priv = guc_to_i915(guc);
+	int i;
 
 	/*
 	 * All SOFT_SCRATCH registers are in FORCEWAKE_BLITTER domain and
@@ -373,12 +365,28 @@  void intel_guc_init_params(struct intel_guc *guc)
 
 	I915_WRITE(SOFT_SCRATCH(0), 0);
 
-	for (i = 0; i < GUC_CTL_MAX_DWORDS; i++)
+	for (i = 0; i < GUC_CTL_MAX_DWORDS; i++) {
+		DRM_DEBUG_DRIVER("param[%2d] = %#x\n", i, params[i]);
 		I915_WRITE(SOFT_SCRATCH(1 + i), params[i]);
+	}
 
 	intel_uncore_forcewake_put(&dev_priv->uncore, FORCEWAKE_BLITTER);
 }
 
+/*
+ * Initialise the GuC parameter block before starting the firmware
+ * transfer. These parameters are read by the firmware on startup
+ * and cannot be changed thereafter.
+ */
+void intel_guc_init_params(struct intel_guc *guc)
+{
+	u32 params[GUC_CTL_MAX_DWORDS];
+
+	memset(params, 0, sizeof(params));
+	guc_prepare_params(guc, params);
+	guc_write_params(guc, params);
+}
+
 int intel_guc_send_nop(struct intel_guc *guc, const u32 *action, u32 len,
 		       u32 *response_buf, u32 response_buf_size)
 {