[Intel-gfx] [PATCH] drm/i915/guc: Unify parameters of public CT functions

[Sagar Arun Kamble]

On 3/19/2018 8:58 PM, Michal Wajdeczko wrote:
> There is no need to mix parameter types in public CT functions
> as we can always accept intel_guc_ct.
>
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/intel_guc_ct.c | 34 ++++++++++++++++++++++++----------
>   drivers/gpu/drm/i915/intel_guc_ct.h |  6 ++----
>   drivers/gpu/drm/i915/intel_uc.c     |  4 ++--
>   3 files changed, 28 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.c b/drivers/gpu/drm/i915/intel_guc_ct.c
> index 0a0d3d5..7dd7de0 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.c
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.c
> @@ -28,12 +28,21 @@
>   
>   enum { CTB_OWNER_HOST = 0 };
>   
> +/**
> + * intel_guc_ct_init_early - Initialize CT state without requiring device access
> + * @ct: pointer to CT struct
> + */
>   void intel_guc_ct_init_early(struct intel_guc_ct *ct)
>   {
>   	/* we're using static channel owners */
>   	ct->host_channel.owner = CTB_OWNER_HOST;
>   }
>   
> +static inline struct intel_guc *ct_to_guc(struct intel_guc_ct *ct)
> +{
> +	return container_of(ct, struct intel_guc, ct);
> +}
> +
>   static inline const char *guc_ct_buffer_type_to_str(u32 type)
>   {
>   	switch (type) {
> @@ -416,16 +425,19 @@ static int intel_guc_send_ct(struct intel_guc *guc, const u32 *action, u32 len)
>   }
>   
>   /**
> - * Enable buffer based command transport
> + * intel_guc_ct_enable - Enable buffer based command transport.
> + * @ct: pointer to CT struct
> + *
>    * Shall only be called for platforms with HAS_GUC_CT.
> - * @guc:	the guc
> - * return:	0 on success
> - *		non-zero on failure
> + *
> + * Returns:
> + * 0 on success, a negative errno code on failure.
Should be
      * Return: 0 on sucess ...
>    */
> -int intel_guc_enable_ct(struct intel_guc *guc)
> +int intel_guc_ct_enable(struct intel_guc_ct *ct)
>   {
> +	struct intel_guc *guc = ct_to_guc(ct);
>   	struct drm_i915_private *dev_priv = guc_to_i915(guc);
change to *i915 as part of this patch itself? :) similar for disable.
Otherwise LGTM
Reviewed-by: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> -	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
> +	struct intel_guc_ct_channel *ctch = &ct->host_channel;
>   	int err;
>   
>   	GEM_BUG_ON(!HAS_GUC_CT(dev_priv));
> @@ -441,14 +453,16 @@ int intel_guc_enable_ct(struct intel_guc *guc)
>   }
>   
>   /**
> - * Disable buffer based command transport.
> + * intel_guc_ct_disable - Disable buffer based command transport.
> + * @ct: pointer to CT struct
> + *
>    * Shall only be called for platforms with HAS_GUC_CT.
> - * @guc: the guc
>    */
> -void intel_guc_disable_ct(struct intel_guc *guc)
> +void intel_guc_ct_disable(struct intel_guc_ct *ct)
>   {
> +	struct intel_guc *guc = ct_to_guc(ct);
>   	struct drm_i915_private *dev_priv = guc_to_i915(guc);
> -	struct intel_guc_ct_channel *ctch = &guc->ct.host_channel;
> +	struct intel_guc_ct_channel *ctch = &ct->host_channel;
>   
>   	GEM_BUG_ON(!HAS_GUC_CT(dev_priv));
>   
> diff --git a/drivers/gpu/drm/i915/intel_guc_ct.h b/drivers/gpu/drm/i915/intel_guc_ct.h
> index 6d97f36..595c8ad 100644
> --- a/drivers/gpu/drm/i915/intel_guc_ct.h
> +++ b/drivers/gpu/drm/i915/intel_guc_ct.h
> @@ -78,9 +78,7 @@ struct intel_guc_ct {
>   };
>   
>   void intel_guc_ct_init_early(struct intel_guc_ct *ct);
> -
> -/* XXX: move to intel_uc.h ? don't fit there either */
> -int intel_guc_enable_ct(struct intel_guc *guc);
> -void intel_guc_disable_ct(struct intel_guc *guc);
> +int intel_guc_ct_enable(struct intel_guc_ct *ct);
> +void intel_guc_ct_disable(struct intel_guc_ct *ct);
>   
>   #endif /* _INTEL_GUC_CT_H_ */
> diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
> index 34e847d..a45c2ce 100644
> --- a/drivers/gpu/drm/i915/intel_uc.c
> +++ b/drivers/gpu/drm/i915/intel_uc.c
> @@ -232,7 +232,7 @@ static int guc_enable_communication(struct intel_guc *guc)
>   	gen9_enable_guc_interrupts(dev_priv);
>   
>   	if (HAS_GUC_CT(dev_priv))
> -		return intel_guc_enable_ct(guc);
> +		return intel_guc_ct_enable(&guc->ct);
>   
>   	guc->send = intel_guc_send_mmio;
>   	return 0;
> @@ -243,7 +243,7 @@ static void guc_disable_communication(struct intel_guc *guc)
>   	struct drm_i915_private *dev_priv = guc_to_i915(guc);
>   
>   	if (HAS_GUC_CT(dev_priv))
> -		intel_guc_disable_ct(guc);
> +		intel_guc_ct_disable(&guc->ct);
>   
>   	gen9_disable_guc_interrupts(dev_priv);
>   

-- 
Thanks,
Sagar