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@xxxxxxxxx>
Good clean up. Let's adopt this style everywhere in Xe going forward.
Reviewed-by: Matthew Brost <matthew.brost@xxxxxxxxx>
> ---
> 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
>
