[Intel-xe] [PATCH 04/14] drm/xe: Extract non mapped regions out of GuC CTB into its own struct.

Matthew Brost matthew.brost at intel.com
Tue May 2 05:12:43 UTC 2023


On Wed, Apr 26, 2023 at 04:57:03PM -0400, Rodrigo Vivi wrote:
> No functional change here. The goal is to have a clear split between
> the mapped portions of the CTB and the static information, so we can
> easily capture snapshots that will be used for later read out with
> the devcoredump infrastructure.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>

Good clean up. Let's adopt this style everywhere in Xe going forward.

Reviewed-by: Matthew Brost <matthew.brost at intel.com>

> ---
>  drivers/gpu/drm/xe/xe_guc_ct.c       | 155 ++++++++++++++-------------
>  drivers/gpu/drm/xe/xe_guc_ct_types.h |  20 ++--
>  2 files changed, 95 insertions(+), 80 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> index 9055ff133a7c..e16e5fe37ed4 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> @@ -172,13 +172,14 @@ int xe_guc_ct_init(struct xe_guc_ct *ct)
>  static void guc_ct_ctb_h2g_init(struct xe_device *xe, struct guc_ctb *h2g,
>  				struct iosys_map *map)
>  {
> -	h2g->size = CTB_H2G_BUFFER_SIZE / sizeof(u32);
> -	h2g->resv_space = 0;
> -	h2g->tail = 0;
> -	h2g->head = 0;
> -	h2g->space = CIRC_SPACE(h2g->tail, h2g->head, h2g->size) -
> -		h2g->resv_space;
> -	h2g->broken = false;
> +	h2g->info.size = CTB_H2G_BUFFER_SIZE / sizeof(u32);
> +	h2g->info.resv_space = 0;
> +	h2g->info.tail = 0;
> +	h2g->info.head = 0;
> +	h2g->info.space = CIRC_SPACE(h2g->info.tail, h2g->info.head,
> +				     h2g->info.size) -
> +			  h2g->info.resv_space;
> +	h2g->info.broken = false;
>  
>  	h2g->desc = *map;
>  	xe_map_memset(xe, &h2g->desc, 0, 0, sizeof(struct guc_ct_buffer_desc));
> @@ -189,13 +190,14 @@ static void guc_ct_ctb_h2g_init(struct xe_device *xe, struct guc_ctb *h2g,
>  static void guc_ct_ctb_g2h_init(struct xe_device *xe, struct guc_ctb *g2h,
>  				struct iosys_map *map)
>  {
> -	g2h->size = CTB_G2H_BUFFER_SIZE / sizeof(u32);
> -	g2h->resv_space = G2H_ROOM_BUFFER_SIZE / sizeof(u32);
> -	g2h->head = 0;
> -	g2h->tail = 0;
> -	g2h->space = CIRC_SPACE(g2h->tail, g2h->head, g2h->size) -
> -		g2h->resv_space;
> -	g2h->broken = false;
> +	g2h->info.size = CTB_G2H_BUFFER_SIZE / sizeof(u32);
> +	g2h->info.resv_space = G2H_ROOM_BUFFER_SIZE / sizeof(u32);
> +	g2h->info.head = 0;
> +	g2h->info.tail = 0;
> +	g2h->info.space = CIRC_SPACE(g2h->info.tail, g2h->info.head,
> +				     g2h->info.size) -
> +			  g2h->info.resv_space;
> +	g2h->info.broken = false;
>  
>  	g2h->desc = IOSYS_MAP_INIT_OFFSET(map, CTB_DESC_SIZE);
>  	xe_map_memset(xe, &g2h->desc, 0, 0, sizeof(struct guc_ct_buffer_desc));
> @@ -212,7 +214,7 @@ static int guc_ct_ctb_h2g_register(struct xe_guc_ct *ct)
>  
>  	desc_addr = xe_bo_ggtt_addr(ct->bo);
>  	ctb_addr = xe_bo_ggtt_addr(ct->bo) + CTB_DESC_SIZE * 2;
> -	size = ct->ctbs.h2g.size * sizeof(u32);
> +	size = ct->ctbs.h2g.info.size * sizeof(u32);
>  
>  	err = xe_guc_self_cfg64(guc,
>  				GUC_KLV_SELF_CFG_H2G_CTB_DESCRIPTOR_ADDR_KEY,
> @@ -240,7 +242,7 @@ static int guc_ct_ctb_g2h_register(struct xe_guc_ct *ct)
>  	desc_addr = xe_bo_ggtt_addr(ct->bo) + CTB_DESC_SIZE;
>  	ctb_addr = xe_bo_ggtt_addr(ct->bo) + CTB_DESC_SIZE * 2 +
>  		CTB_H2G_BUFFER_SIZE;
> -	size = ct->ctbs.g2h.size * sizeof(u32);
> +	size = ct->ctbs.g2h.info.size * sizeof(u32);
>  
>  	err = xe_guc_self_cfg64(guc,
>  				GUC_KLV_SELF_CFG_G2H_CTB_DESCRIPTOR_ADDR_KEY,
> @@ -329,11 +331,12 @@ static bool h2g_has_room(struct xe_guc_ct *ct, u32 cmd_len)
>  
>  	lockdep_assert_held(&ct->lock);
>  
> -	if (cmd_len > h2g->space) {
> -		h2g->head = desc_read(ct_to_xe(ct), h2g, head);
> -		h2g->space = CIRC_SPACE(h2g->tail, h2g->head, h2g->size) -
> -			h2g->resv_space;
> -		if (cmd_len > h2g->space)
> +	if (cmd_len > h2g->info.space) {
> +		h2g->info.head = desc_read(ct_to_xe(ct), h2g, head);
> +		h2g->info.space = CIRC_SPACE(h2g->info.tail, h2g->info.head,
> +					     h2g->info.size) -
> +				  h2g->info.resv_space;
> +		if (cmd_len > h2g->info.space)
>  			return false;
>  	}
>  
> @@ -344,7 +347,7 @@ static bool g2h_has_room(struct xe_guc_ct *ct, u32 g2h_len)
>  {
>  	lockdep_assert_held(&ct->lock);
>  
> -	return ct->ctbs.g2h.space > g2h_len;
> +	return ct->ctbs.g2h.info.space > g2h_len;
>  }
>  
>  static int has_room(struct xe_guc_ct *ct, u32 cmd_len, u32 g2h_len)
> @@ -360,16 +363,16 @@ static int has_room(struct xe_guc_ct *ct, u32 cmd_len, u32 g2h_len)
>  static void h2g_reserve_space(struct xe_guc_ct *ct, u32 cmd_len)
>  {
>  	lockdep_assert_held(&ct->lock);
> -	ct->ctbs.h2g.space -= cmd_len;
> +	ct->ctbs.h2g.info.space -= cmd_len;
>  }
>  
>  static void g2h_reserve_space(struct xe_guc_ct *ct, u32 g2h_len, u32 num_g2h)
>  {
> -	XE_BUG_ON(g2h_len > ct->ctbs.g2h.space);
> +	XE_BUG_ON(g2h_len > ct->ctbs.g2h.info.space);
>  
>  	if (g2h_len) {
>  		spin_lock_irq(&ct->fast_lock);
> -		ct->ctbs.g2h.space -= g2h_len;
> +		ct->ctbs.g2h.info.space -= g2h_len;
>  		ct->g2h_outstanding += num_g2h;
>  		spin_unlock_irq(&ct->fast_lock);
>  	}
> @@ -378,10 +381,10 @@ static void g2h_reserve_space(struct xe_guc_ct *ct, u32 g2h_len, u32 num_g2h)
>  static void __g2h_release_space(struct xe_guc_ct *ct, u32 g2h_len)
>  {
>  	lockdep_assert_held(&ct->fast_lock);
> -	XE_WARN_ON(ct->ctbs.g2h.space + g2h_len >
> -		   ct->ctbs.g2h.size - ct->ctbs.g2h.resv_space);
> +	XE_WARN_ON(ct->ctbs.g2h.info.space + g2h_len >
> +		   ct->ctbs.g2h.info.size - ct->ctbs.g2h.info.resv_space);
>  
> -	ct->ctbs.g2h.space += g2h_len;
> +	ct->ctbs.g2h.info.space += g2h_len;
>  	--ct->g2h_outstanding;
>  }
>  
> @@ -400,20 +403,21 @@ static int h2g_write(struct xe_guc_ct *ct, const u32 *action, u32 len,
>  	u32 cmd[GUC_CTB_MSG_MAX_LEN / sizeof(u32)];
>  	u32 cmd_len = len + GUC_CTB_HDR_LEN;
>  	u32 cmd_idx = 0, i;
> -	u32 tail = h2g->tail;
> +	u32 tail = h2g->info.tail;
>  	struct iosys_map map = IOSYS_MAP_INIT_OFFSET(&h2g->cmds,
>  							 tail * sizeof(u32));
>  
>  	lockdep_assert_held(&ct->lock);
>  	XE_BUG_ON(len * sizeof(u32) > GUC_CTB_MSG_MAX_LEN);
> -	XE_BUG_ON(tail > h2g->size);
> +	XE_BUG_ON(tail > h2g->info.size);
>  
>  	/* Command will wrap, zero fill (NOPs), return and check credits again */
> -	if (tail + cmd_len > h2g->size) {
> -		xe_map_memset(xe, &map, 0, 0, (h2g->size - tail) * sizeof(u32));
> -		h2g_reserve_space(ct, (h2g->size - tail));
> -		h2g->tail = 0;
> -		desc_write(xe, h2g, tail, h2g->tail);
> +	if (tail + cmd_len > h2g->info.size) {
> +		xe_map_memset(xe, &map, 0, 0,
> +			      (h2g->info.size - tail) * sizeof(u32));
> +		h2g_reserve_space(ct, (h2g->info.size - tail));
> +		h2g->info.tail = 0;
> +		desc_write(xe, h2g, tail, h2g->info.tail);
>  
>  		return -EAGAIN;
>  	}
> @@ -445,11 +449,11 @@ static int h2g_write(struct xe_guc_ct *ct, const u32 *action, u32 len,
>  	xe_device_wmb(ct_to_xe(ct));
>  
>  	/* Update local copies */
> -	h2g->tail = (tail + cmd_len) % h2g->size;
> +	h2g->info.tail = (tail + cmd_len) % h2g->info.size;
>  	h2g_reserve_space(ct, cmd_len);
>  
>  	/* Update descriptor */
> -	desc_write(xe, h2g, tail, h2g->tail);
> +	desc_write(xe, h2g, tail, h2g->info.tail);
>  
>  	return 0;
>  }
> @@ -466,7 +470,7 @@ static int __guc_ct_send_locked(struct xe_guc_ct *ct, const u32 *action,
>  	XE_BUG_ON(!g2h_len && num_g2h);
>  	lockdep_assert_held(&ct->lock);
>  
> -	if (unlikely(ct->ctbs.h2g.broken)) {
> +	if (unlikely(ct->ctbs.h2g.info.broken)) {
>  		ret = -EPIPE;
>  		goto out;
>  	}
> @@ -554,8 +558,9 @@ static int guc_ct_send_locked(struct xe_guc_ct *ct, const u32 *action, u32 len,
>  		if (sleep_period_ms == 1024)
>  			goto broken;
>  
> -		trace_xe_guc_ct_h2g_flow_control(h2g->head, h2g->tail,
> -						 h2g->size, h2g->space,
> +		trace_xe_guc_ct_h2g_flow_control(h2g->info.head, h2g->info.tail,
> +						 h2g->info.size,
> +						 h2g->info.space,
>  						 len + GUC_CTB_HDR_LEN);
>  		msleep(sleep_period_ms);
>  		sleep_period_ms <<= 1;
> @@ -565,15 +570,16 @@ static int guc_ct_send_locked(struct xe_guc_ct *ct, const u32 *action, u32 len,
>  		struct xe_device *xe = ct_to_xe(ct);
>  		struct guc_ctb *g2h = &ct->ctbs.g2h;
>  
> -		trace_xe_guc_ct_g2h_flow_control(g2h->head,
> +		trace_xe_guc_ct_g2h_flow_control(g2h->info.head,
>  						 desc_read(xe, g2h, tail),
> -						 g2h->size, g2h->space,
> +						 g2h->info.size,
> +						 g2h->info.space,
>  						 g2h_fence ?
>  						 GUC_CTB_HXG_MSG_MAX_LEN :
>  						 g2h_len);
>  
>  #define g2h_avail(ct)	\
> -	(desc_read(ct_to_xe(ct), (&ct->ctbs.g2h), tail) != ct->ctbs.g2h.head)
> +	(desc_read(ct_to_xe(ct), (&ct->ctbs.g2h), tail) != ct->ctbs.g2h.info.head)
>  		if (!wait_event_timeout(ct->wq, !ct->g2h_outstanding ||
>  					g2h_avail(ct), HZ))
>  			goto broken;
> @@ -590,7 +596,7 @@ static int guc_ct_send_locked(struct xe_guc_ct *ct, const u32 *action, u32 len,
>  broken:
>  	drm_err(drm, "No forward process on H2G, reset required");
>  	xe_guc_ct_print(ct, &p);
> -	ct->ctbs.h2g.broken = true;
> +	ct->ctbs.h2g.info.broken = true;
>  
>  	return -EDEADLK;
>  }
> @@ -656,7 +662,7 @@ static bool retry_failure(struct xe_guc_ct *ct, int ret)
>  		return false;
>  
>  #define ct_alive(ct)	\
> -	(ct->enabled && !ct->ctbs.h2g.broken && !ct->ctbs.g2h.broken)
> +	(ct->enabled && !ct->ctbs.h2g.info.broken && !ct->ctbs.g2h.info.broken)
>  	if (!wait_event_interruptible_timeout(ct->wq, ct_alive(ct),  HZ * 5))
>  		return false;
>  #undef ct_alive
> @@ -821,7 +827,7 @@ static int parse_g2h_msg(struct xe_guc_ct *ct, u32 *msg, u32 len)
>  		drm_err(&xe->drm,
>  			"G2H channel broken on read, origin=%d, reset required\n",
>  			origin);
> -		ct->ctbs.g2h.broken = true;
> +		ct->ctbs.g2h.info.broken = true;
>  
>  		return -EPROTO;
>  	}
> @@ -840,7 +846,7 @@ static int parse_g2h_msg(struct xe_guc_ct *ct, u32 *msg, u32 len)
>  		drm_err(&xe->drm,
>  			"G2H channel broken on read, type=%d, reset required\n",
>  			type);
> -		ct->ctbs.g2h.broken = true;
> +		ct->ctbs.g2h.info.broken = true;
>  
>  		ret = -EOPNOTSUPP;
>  	}
> @@ -919,36 +925,37 @@ static int g2h_read(struct xe_guc_ct *ct, u32 *msg, bool fast_path)
>  	if (!ct->enabled)
>  		return -ENODEV;
>  
> -	if (g2h->broken)
> +	if (g2h->info.broken)
>  		return -EPIPE;
>  
>  	/* Calculate DW available to read */
>  	tail = desc_read(xe, g2h, tail);
> -	avail = tail - g2h->head;
> +	avail = tail - g2h->info.head;
>  	if (unlikely(avail == 0))
>  		return 0;
>  
>  	if (avail < 0)
> -		avail += g2h->size;
> +		avail += g2h->info.size;
>  
>  	/* Read header */
> -	xe_map_memcpy_from(xe, msg, &g2h->cmds, sizeof(u32) * g2h->head, sizeof(u32));
> +	xe_map_memcpy_from(xe, msg, &g2h->cmds, sizeof(u32) * g2h->info.head,
> +			   sizeof(u32));
>  	len = FIELD_GET(GUC_CTB_MSG_0_NUM_DWORDS, msg[0]) + GUC_CTB_MSG_MIN_LEN;
>  	if (len > avail) {
>  		drm_err(&xe->drm,
>  			"G2H channel broken on read, avail=%d, len=%d, reset required\n",
>  			avail, len);
> -		g2h->broken = true;
> +		g2h->info.broken = true;
>  
>  		return -EPROTO;
>  	}
>  
> -	head = (g2h->head + 1) % g2h->size;
> +	head = (g2h->info.head + 1) % g2h->info.size;
>  	avail = len - 1;
>  
>  	/* Read G2H message */
> -	if (avail + head > g2h->size) {
> -		u32 avail_til_wrap = g2h->size - head;
> +	if (avail + head > g2h->info.size) {
> +		u32 avail_til_wrap = g2h->info.size - head;
>  
>  		xe_map_memcpy_from(xe, msg + 1,
>  				   &g2h->cmds, sizeof(u32) * head,
> @@ -983,8 +990,8 @@ static int g2h_read(struct xe_guc_ct *ct, u32 *msg, bool fast_path)
>  	}
>  
>  	/* Update local / descriptor header */
> -	g2h->head = (head + avail) % g2h->size;
> -	desc_write(xe, g2h, head, g2h->head);
> +	g2h->info.head = (head + avail) % g2h->info.size;
> +	desc_write(xe, g2h, head, g2h->info.head);
>  
>  	return len;
>  }
> @@ -1093,12 +1100,12 @@ static void guc_ct_ctb_print(struct xe_device *xe, struct guc_ctb *ctb,
>  {
>  	u32 head, tail;
>  
> -	drm_printf(p, "\tsize: %d\n", ctb->size);
> -	drm_printf(p, "\tresv_space: %d\n", ctb->resv_space);
> -	drm_printf(p, "\thead: %d\n", ctb->head);
> -	drm_printf(p, "\ttail: %d\n", ctb->tail);
> -	drm_printf(p, "\tspace: %d\n", ctb->space);
> -	drm_printf(p, "\tbroken: %d\n", ctb->broken);
> +	drm_printf(p, "\tsize: %d\n", ctb->info.size);
> +	drm_printf(p, "\tresv_space: %d\n", ctb->info.resv_space);
> +	drm_printf(p, "\thead: %d\n", ctb->info.head);
> +	drm_printf(p, "\ttail: %d\n", ctb->info.tail);
> +	drm_printf(p, "\tspace: %d\n", ctb->info.space);
> +	drm_printf(p, "\tbroken: %d\n", ctb->info.broken);
>  
>  	head = desc_read(xe, ctb, head);
>  	tail = desc_read(xe, ctb, tail);
> @@ -1114,7 +1121,7 @@ static void guc_ct_ctb_print(struct xe_device *xe, struct guc_ctb *ctb,
>  			drm_printf(p, "\tcmd[%d]: 0x%08x\n", head,
>  				   xe_map_rd(xe, &map, 0, u32));
>  			++head;
> -			if (head == ctb->size) {
> +			if (head == ctb->info.size) {
>  				head = 0;
>  				map = ctb->cmds;
>  			} else {
> @@ -1168,12 +1175,12 @@ void xe_guc_ct_selftest(struct xe_guc_ct *ct, struct drm_printer *p)
>  	if (!ret) {
>  		xe_guc_ct_irq_handler(ct);
>  		msleep(200);
> -		if (g2h->space !=
> -		    CIRC_SPACE(0, 0, g2h->size) - g2h->resv_space) {
> +		if (g2h->info.space !=
> +		    CIRC_SPACE(0, 0, g2h->info.size) - g2h->info.resv_space) {
>  			drm_printf(p, "Mismatch on space %d, %d\n",
> -				   g2h->space,
> -				   CIRC_SPACE(0, 0, g2h->size) -
> -				   g2h->resv_space);
> +				   g2h->info.space,
> +				   CIRC_SPACE(0, 0, g2h->info.size) -
> +				   g2h->info.resv_space);
>  			ret = -EIO;
>  		}
>  		if (ct->g2h_outstanding) {
> @@ -1185,12 +1192,12 @@ void xe_guc_ct_selftest(struct xe_guc_ct *ct, struct drm_printer *p)
>  
>  	/* Check failure path for blocking CTs too */
>  	xe_guc_ct_send_block(ct, bad_action, ARRAY_SIZE(bad_action));
> -	if (g2h->space !=
> -	    CIRC_SPACE(0, 0, g2h->size) - g2h->resv_space) {
> +	if (g2h->info.space !=
> +	    CIRC_SPACE(0, 0, g2h->info.size) - g2h->info.resv_space) {
>  		drm_printf(p, "Mismatch on space %d, %d\n",
> -			   g2h->space,
> -			   CIRC_SPACE(0, 0, g2h->size) -
> -			   g2h->resv_space);
> +			   g2h->info.space,
> +			   CIRC_SPACE(0, 0, g2h->info.size) -
> +			   g2h->info.resv_space);
>  		ret = -EIO;
>  	}
>  	if (ct->g2h_outstanding) {
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct_types.h b/drivers/gpu/drm/xe/xe_guc_ct_types.h
> index fd27dacf00c5..64e3dd14d4b2 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct_types.h
> +++ b/drivers/gpu/drm/xe/xe_guc_ct_types.h
> @@ -19,13 +19,9 @@
>  struct xe_bo;
>  
>  /**
> - * struct guc_ctb - GuC command transport buffer (CTB)
> + * struct guc_ctb_info - GuC command transport buffer (CTB) info
>   */
> -struct guc_ctb {
> -	/** @desc: dma buffer map for CTB descriptor */
> -	struct iosys_map desc;
> -	/** @cmds: dma buffer map for CTB commands */
> -	struct iosys_map cmds;
> +struct guc_ctb_info {
>  	/** @size: size of CTB commands (DW) */
>  	u32 size;
>  	/** @resv_space: reserved space of CTB commands (DW) */
> @@ -40,6 +36,18 @@ struct guc_ctb {
>  	bool broken;
>  };
>  
> +/**
> + * struct guc_ctb - GuC command transport buffer (CTB)
> + */
> +struct guc_ctb {
> +	/** @desc: dma buffer map for CTB descriptor */
> +	struct iosys_map desc;
> +	/** @cmds: dma buffer map for CTB commands */
> +	struct iosys_map cmds;
> +	/** @info: CTB info */
> +	struct guc_ctb_info info;
> +};
> +
>  /**
>   * struct xe_guc_ct - GuC command transport (CT) layer
>   *
> -- 
> 2.39.2
> 


More information about the Intel-xe mailing list