Re: [Intel-gfx] [PATCH v2 4/7] drm/i915/guc/ct: Group request-related variables in a sub-structure

["Michal Wajdeczko" <michal.wajdeczko@intel.com>]

On Tue, 17 Dec 2019 02:23:13 +0100, Daniele Ceraolo Spurio  
<daniele.ceraolospurio@intel.com> wrote:

> For better isolation of the request tracking from the rest of the
> CT-related data.
>
> v2: split to separate patch, move next_fence to substructure (Michal)
>
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: John Harrison <John.C.Harrison@Intel.com>
> Cc: Matthew Brost <matthew.brost@intel.com>
> ---

Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>

with some nits below (we may fix them later)

/snip/

> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h  
> b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
> index 6e3d789b9f01..29a026dc3a13 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
> @@ -48,12 +48,15 @@ struct intel_guc_ct {
>  	/* buffers for sending(0) and receiving(1) commands */
>  	struct intel_guc_ct_buffer ctbs[2];
> -	u32 next_fence; /* fence to be used with next send command */
> +	struct {
> +		u32 next_fence; /* fence to be used with next request to send */

nit: strictly speaking this is "last" fence
      we just use it to generate next one

> -	spinlock_t lock; /* protects pending requests list */
> -	struct list_head pending_requests; /* requests waiting for response */
> -	struct list_head incoming_requests; /* incoming requests */
> -	struct work_struct worker; /* handler for incoming requests */
> +		spinlock_t lock; /* protects pending requests list */

nit: do we want to use this lock to protect "next/last" fence ?
      if yes, then maybe lock shall be first ?

> +		struct list_head pending; /* requests waiting for response */
> +
> +		struct list_head incoming; /* incoming requests */
> +		struct work_struct worker; /* handler for incoming requests */
> +	} requests;
>  };
> void intel_guc_ct_init_early(struct intel_guc_ct *ct);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx