[Intel-gfx] [PATCH 2/3] drm/i915/guc: Optimized CTB writes and reads

John.C.Harrison at Intel.com John.C.Harrison at Intel.com
Wed Nov 20 23:56:03 UTC 2019


From: Matthew Brost <matthew.brost at intel.com>

CTB writes are now in the path of command submission and should be
optimized for performance. Rather than reading CTB descriptor values
(e.g. head, tail, size) which could result in accesses across the PCIe
bus, store shadow local copies and only read/write the descriptor
values when absolutely necessary.

Cc: John Harrison <john.c.harrison at intel.com>
Signed-off-by: Matthew Brost <matthew.brost at intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 79 +++++++++++------------
 drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h |  8 +++
 2 files changed, 45 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
index e50d968b15d5..4d8a4c6afd71 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -68,23 +68,29 @@ static inline const char *guc_ct_buffer_type_to_str(u32 type)
 	}
 }
 
-static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc,
+static void guc_ct_buffer_desc_init(struct intel_guc_ct_buffer *ctb,
 				    u32 cmds_addr, u32 size, u32 owner)
 {
+	struct guc_ct_buffer_desc *desc = ctb->desc;
 	CT_DEBUG_DRIVER("CT: desc %p init addr=%#x size=%u owner=%u\n",
 			desc, cmds_addr, size, owner);
 	memset(desc, 0, sizeof(*desc));
 	desc->addr = cmds_addr;
-	desc->size = size;
+	ctb->size = desc->size = size;
 	desc->owner = owner;
+	ctb->tail = 0;
+	ctb->head = 0;
+	ctb->space = CIRC_SPACE(ctb->tail, ctb->head, ctb->size);
 }
 
-static void guc_ct_buffer_desc_reset(struct guc_ct_buffer_desc *desc)
+static void guc_ct_buffer_desc_reset(struct intel_guc_ct_buffer *ctb)
 {
+	struct guc_ct_buffer_desc *desc = ctb->desc;
 	CT_DEBUG_DRIVER("CT: desc %p reset head=%u tail=%u\n",
 			desc, desc->head, desc->tail);
-	desc->head = 0;
-	desc->tail = 0;
+	ctb->head = desc->head = 0;
+	ctb->tail = desc->tail = 0;
+	ctb->space = CIRC_SPACE(ctb->tail, ctb->head, ctb->size);
 	desc->is_in_error = 0;
 }
 
@@ -220,7 +226,7 @@ static int ctch_enable(struct intel_guc *guc,
 	 */
 	for (i = 0; i < ARRAY_SIZE(ctch->ctbs); i++) {
 		GEM_BUG_ON((i != CTB_SEND) && (i != CTB_RECV));
-		guc_ct_buffer_desc_init(ctch->ctbs[i].desc,
+		guc_ct_buffer_desc_init(&ctch->ctbs[i],
 					base + PAGE_SIZE/4 * i + PAGE_SIZE/2,
 					PAGE_SIZE/4,
 					ctch->owner);
@@ -301,32 +307,16 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 		     bool want_response)
 {
 	struct guc_ct_buffer_desc *desc = ctb->desc;
-	u32 head = desc->head / 4;	/* in dwords */
-	u32 tail = desc->tail / 4;	/* in dwords */
-	u32 size = desc->size / 4;	/* in dwords */
-	u32 used;			/* in dwords */
+	u32 tail = ctb->tail / 4;	/* in dwords */
+	u32 size = ctb->size / 4;	/* in dwords */
 	u32 header;
 	u32 *cmds = ctb->cmds;
 	unsigned int i;
 
-	GEM_BUG_ON(desc->size % 4);
-	GEM_BUG_ON(desc->head % 4);
-	GEM_BUG_ON(desc->tail % 4);
+	GEM_BUG_ON(ctb->size % 4);
+	GEM_BUG_ON(ctb->tail % 4);
 	GEM_BUG_ON(tail >= size);
 
-	/*
-	 * tail == head condition indicates empty. GuC FW does not support
-	 * using up the entire buffer to get tail == head meaning full.
-	 */
-	if (tail < head)
-		used = (size - head) + tail;
-	else
-		used = tail - head;
-
-	/* make sure there is a space including extra dw for the fence */
-	if (unlikely(used + len + 1 >= size))
-		return -ENOSPC;
-
 	/*
 	 * Write the message. The format is the following:
 	 * DW0: header (including action code)
@@ -354,15 +344,16 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
 	}
 
 	/* now update desc tail (back in bytes) */
-	desc->tail = tail * 4;
-	GEM_BUG_ON(desc->tail > desc->size);
+	ctb->tail = desc->tail = tail * 4;
+	ctb->space -= (len + 1) * 4;
+	GEM_BUG_ON(ctb->tail > ctb->size);
 
 	return 0;
 }
 
 /**
  * wait_for_ctb_desc_update - Wait for the CT buffer descriptor update.
- * @desc:	buffer descriptor
+ * @ctb:	ctb buffer
  * @fence:	response fence
  * @status:	placeholder for status
  *
@@ -376,11 +367,12 @@ static int ctb_write(struct intel_guc_ct_buffer *ctb,
  * *	-ETIMEDOUT no response within hardcoded timeout
  * *	-EPROTO no response, CT buffer is in error
  */
-static int wait_for_ctb_desc_update(struct guc_ct_buffer_desc *desc,
+static int wait_for_ctb_desc_update(struct intel_guc_ct_buffer *ctb,
 				    u32 fence,
 				    u32 *status)
 {
 	int err;
+	struct guc_ct_buffer_desc *desc = ctb->desc;
 
 	/*
 	 * Fast commands should complete in less than 10us, so sample quickly
@@ -401,7 +393,7 @@ static int wait_for_ctb_desc_update(struct guc_ct_buffer_desc *desc,
 			/* Something went wrong with the messaging, try to reset
 			 * the buffer and hope for the best
 			 */
-			guc_ct_buffer_desc_reset(desc);
+			guc_ct_buffer_desc_reset(ctb);
 			err = -EPROTO;
 		}
 	}
@@ -446,12 +438,17 @@ static int wait_for_ct_request_update(struct ct_request *req, u32 *status)
 	return err;
 }
 
-static inline bool ctb_has_room(struct guc_ct_buffer_desc *desc, u32 len)
+static inline bool ctb_has_room(struct intel_guc_ct_buffer *ctb, u32 len)
 {
-	u32 head = READ_ONCE(desc->head);
+	u32 head;
 	u32 space;
 
-	space = CIRC_SPACE(desc->tail, head, desc->size);
+	if (ctb->space >= len)
+		return true;
+
+	head = READ_ONCE(ctb->desc->head);
+	space = CIRC_SPACE(ctb->tail, head, ctb->size);
+	ctb->space = space;
 
 	return space >= len;
 }
@@ -462,7 +459,6 @@ int intel_guc_send_nb(struct intel_guc_ct *ct,
 {
 	struct intel_guc_ct_channel *ctch = &ct->host_channel;
 	struct intel_guc_ct_buffer *ctb = &ctch->ctbs[CTB_SEND];
-	struct guc_ct_buffer_desc *desc = ctb->desc;
 	int err;
 
 	GEM_BUG_ON(!ctch->enabled);
@@ -470,7 +466,7 @@ int intel_guc_send_nb(struct intel_guc_ct *ct,
 	GEM_BUG_ON(len & ~GUC_CT_MSG_LEN_MASK);
 	lockdep_assert_held(&ct->send_lock);
 
-	if (unlikely(!ctb_has_room(desc, (len + 1) * 4))) {
+	if (unlikely(!ctb_has_room(ctb, (len + 1) * 4))) {
 		ct->retry++;
 		if (ct->retry >= MAX_RETRY)
 			return -EDEADLK;
@@ -496,7 +492,6 @@ static int ctch_send(struct intel_guc_ct *ct,
 		     u32 *status)
 {
 	struct intel_guc_ct_buffer *ctb = &ctch->ctbs[CTB_SEND];
-	struct guc_ct_buffer_desc *desc = ctb->desc;
 	struct ct_request request;
 	unsigned long flags;
 	u32 fence;
@@ -514,7 +509,7 @@ static int ctch_send(struct intel_guc_ct *ct,
 	 */
 retry:
 	spin_lock_irqsave(&ct->send_lock, flags);
-	if (unlikely(!ctb_has_room(desc, (len + 1) * 4))) {
+	if (unlikely(!ctb_has_room(ctb, (len + 1) * 4))) {
 		spin_unlock_irqrestore(&ct->send_lock, flags);
 		ct->retry++;
 		if (ct->retry >= MAX_RETRY)
@@ -544,7 +539,7 @@ static int ctch_send(struct intel_guc_ct *ct,
 	if (response_buf)
 		err = wait_for_ct_request_update(&request, status);
 	else
-		err = wait_for_ctb_desc_update(desc, fence, status);
+		err = wait_for_ctb_desc_update(ctb, fence, status);
 	if (unlikely(err))
 		goto unlink;
 
@@ -618,9 +613,9 @@ static inline bool ct_header_is_response(u32 header)
 static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
 {
 	struct guc_ct_buffer_desc *desc = ctb->desc;
-	u32 head = desc->head / 4;	/* in dwords */
+	u32 head = ctb->head / 4;	/* in dwords */
 	u32 tail = desc->tail / 4;	/* in dwords */
-	u32 size = desc->size / 4;	/* in dwords */
+	u32 size = ctb->size / 4;	/* in dwords */
 	u32 *cmds = ctb->cmds;
 	s32 available;			/* in dwords */
 	unsigned int len;
@@ -664,7 +659,7 @@ static int ctb_read(struct intel_guc_ct_buffer *ctb, u32 *data)
 	}
 	CT_DEBUG_DRIVER("CT: received %*ph\n", 4 * len, data);
 
-	desc->head = head * 4;
+	ctb->head = desc->head = head * 4;
 	return 0;
 }
 
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 bc670a796bd8..1bff4f0b91f7 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
@@ -29,10 +29,18 @@ struct intel_guc;
  *
  * @desc: pointer to the buffer descriptor
  * @cmds: pointer to the commands buffer
+ * @size: local shadow copy of size
+ * @head: local shadow copy of head
+ * @tail: local shadow copy of tail
+ * @space: local shadow copy of space
  */
 struct intel_guc_ct_buffer {
 	struct guc_ct_buffer_desc *desc;
 	u32 *cmds;
+	u32 size;
+	u32 tail;
+	u32 head;
+	u32 space;
 };
 
 /** Represents pair of command transport buffers.
-- 
2.21.0.5.gaeb582a983



More information about the Intel-gfx mailing list