drm/i915/guc: Optimize CTB writes and reads

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) which could result in accesses across the PCIe bus,
store shadow local copies and only read/write the descriptor values when
absolutely necessary. Also store the current space in the each channel
locally.

v2:
 (Michal)
  - Add additional sanity checks for head / tail pointers
  - Use GUC_CTB_HDR_LEN rather than magic 1
v3:
 (Michal / John H)
  - Drop redundant check of head value
v4:
 (John H)
  - Drop redundant checks of tail / head values
v5:
 (Michal)
  - Address more nits
v6:
 (Michal)
  - Add GEM_BUG_ON sanity check on ctb->space

Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Signed-off-by: Matthew Brost <matthew.brost@intel.com>
Reviewed-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Signed-off-by: John Harrison <John.C.Harrison@Intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20210708162055.129996-7-matthew.brost@intel.com

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 db3e85b..ad33708 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -130,6 +130,10 @@ static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc)
 static void guc_ct_buffer_reset(struct intel_guc_ct_buffer *ctb)
 {
        ctb->broken = false;
+       ctb->tail = 0;
+       ctb->head = 0;
+       ctb->space = CIRC_SPACE(ctb->tail, ctb->head, ctb->size);
+
        guc_ct_buffer_desc_init(ctb->desc);
 }
 
@@ -383,10 +387,8 @@ static int ct_write(struct intel_guc_ct *ct,
 {
        struct intel_guc_ct_buffer *ctb = &ct->ctbs.send;
        struct guc_ct_buffer_desc *desc = ctb->desc;
-       u32 head = desc->head;
-       u32 tail = desc->tail;
+       u32 tail = ctb->tail;
        u32 size = ctb->size;
-       u32 used;
        u32 header;
        u32 hxg;
        u32 type;
@@ -396,25 +398,22 @@ static int ct_write(struct intel_guc_ct *ct,
        if (unlikely(desc->status))
                goto corrupted;
 
-       if (unlikely((tail | head) >= size)) {
-               CT_ERROR(ct, "Invalid offsets head=%u tail=%u (size=%u)\n",
-                        head, tail, size);
+       GEM_BUG_ON(tail > size);
+
+#ifdef CONFIG_DRM_I915_DEBUG_GUC
+       if (unlikely(tail != READ_ONCE(desc->tail))) {
+               CT_ERROR(ct, "Tail was modified %u != %u\n",
+                        desc->tail, tail);
+               desc->status |= GUC_CTB_STATUS_MISMATCH;
+               goto corrupted;
+       }
+       if (unlikely(READ_ONCE(desc->head) >= size)) {
+               CT_ERROR(ct, "Invalid head offset %u >= %u)\n",
+                        desc->head, size);
                desc->status |= GUC_CTB_STATUS_OVERFLOW;
                goto corrupted;
        }
-
-       /*
-        * 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 header */
-       if (unlikely(used + len + GUC_CTB_HDR_LEN >= size))
-               return -ENOSPC;
+#endif
 
        /*
         * dw0: CT header (including fence)
@@ -452,6 +451,11 @@ static int ct_write(struct intel_guc_ct *ct,
         */
        write_barrier(ct);
 
+       /* update local copies */
+       ctb->tail = tail;
+       GEM_BUG_ON(ctb->space < len + GUC_CTB_HDR_LEN);
+       ctb->space -= len + GUC_CTB_HDR_LEN;
+
        /* now update descriptor */
        WRITE_ONCE(desc->tail, tail);
 
@@ -469,7 +473,7 @@ corrupted:
  * @req:       pointer to pending request
  * @status:    placeholder for status
  *
- * For each sent request, Guc shall send bac CT response message.
+ * For each sent request, GuC shall send back CT response message.
  * Our message handler will update status of tracked request once
  * response message with given fence is received. Wait here and
  * check for valid response status value.
@@ -525,24 +529,36 @@ static inline bool ct_deadlocked(struct intel_guc_ct *ct)
        return ret;
 }
 
-static inline bool h2g_has_room(struct intel_guc_ct_buffer *ctb, u32 len_dw)
+static inline bool h2g_has_room(struct intel_guc_ct *ct, u32 len_dw)
 {
+       struct intel_guc_ct_buffer *ctb = &ct->ctbs.send;
        struct guc_ct_buffer_desc *desc = ctb->desc;
-       u32 head = READ_ONCE(desc->head);
+       u32 head;
        u32 space;
 
-       space = CIRC_SPACE(desc->tail, head, ctb->size);
+       if (ctb->space >= len_dw)
+               return true;
+
+       head = READ_ONCE(desc->head);
+       if (unlikely(head > ctb->size)) {
+               CT_ERROR(ct, "Invalid head offset %u >= %u)\n",
+                        head, ctb->size);
+               desc->status |= GUC_CTB_STATUS_OVERFLOW;
+               ctb->broken = true;
+               return false;
+       }
+
+       space = CIRC_SPACE(ctb->tail, head, ctb->size);
+       ctb->space = space;
 
        return space >= len_dw;
 }
 
 static int has_room_nb(struct intel_guc_ct *ct, u32 len_dw)
 {
-       struct intel_guc_ct_buffer *ctb = &ct->ctbs.send;
-
        lockdep_assert_held(&ct->ctbs.send.lock);
 
-       if (unlikely(!h2g_has_room(ctb, len_dw))) {
+       if (unlikely(!h2g_has_room(ct, len_dw))) {
                if (ct->stall_time == KTIME_MAX)
                        ct->stall_time = ktime_get();
 
@@ -612,7 +628,7 @@ static int ct_send(struct intel_guc_ct *ct,
         */
 retry:
        spin_lock_irqsave(&ctb->lock, flags);
-       if (unlikely(!h2g_has_room(ctb, len + GUC_CTB_HDR_LEN))) {
+       if (unlikely(!h2g_has_room(ct, len + GUC_CTB_HDR_LEN))) {
                if (ct->stall_time == KTIME_MAX)
                        ct->stall_time = ktime_get();
                spin_unlock_irqrestore(&ctb->lock, flags);
@@ -732,8 +748,8 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
 {
        struct intel_guc_ct_buffer *ctb = &ct->ctbs.recv;
        struct guc_ct_buffer_desc *desc = ctb->desc;
-       u32 head = desc->head;
-       u32 tail = desc->tail;
+       u32 head = ctb->head;
+       u32 tail = READ_ONCE(desc->tail);
        u32 size = ctb->size;
        u32 *cmds = ctb->cmds;
        s32 available;
@@ -747,9 +763,19 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
        if (unlikely(desc->status))
                goto corrupted;
 
-       if (unlikely((tail | head) >= size)) {
-               CT_ERROR(ct, "Invalid offsets head=%u tail=%u (size=%u)\n",
-                        head, tail, size);
+       GEM_BUG_ON(head > size);
+
+#ifdef CONFIG_DRM_I915_DEBUG_GUC
+       if (unlikely(head != READ_ONCE(desc->head))) {
+               CT_ERROR(ct, "Head was modified %u != %u\n",
+                        desc->head, head);
+               desc->status |= GUC_CTB_STATUS_MISMATCH;
+               goto corrupted;
+       }
+#endif
+       if (unlikely(tail >= size)) {
+               CT_ERROR(ct, "Invalid tail offset %u >= %u)\n",
+                        tail, size);
                desc->status |= GUC_CTB_STATUS_OVERFLOW;
                goto corrupted;
        }
@@ -802,6 +828,9 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
        }
        CT_DEBUG(ct, "received %*ph\n", 4 * len, (*msg)->msg);
 
+       /* update local copies */
+       ctb->head = head;
+
        /* now update descriptor */
        WRITE_ONCE(desc->head, head);
 

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 bee0379..edd1bba 100644 (file)
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
@@ -33,6 +33,9 @@ struct intel_guc;
  * @desc: pointer to the buffer descriptor
  * @cmds: pointer to the commands buffer
  * @size: size of the commands buffer in dwords
+ * @head: local shadow copy of head in dwords
+ * @tail: local shadow copy of tail in dwords
+ * @space: local shadow copy of space in dwords
  * @broken: flag to indicate if descriptor data is broken
  */
 struct intel_guc_ct_buffer {
@@ -40,6 +43,9 @@ struct intel_guc_ct_buffer {
        struct guc_ct_buffer_desc *desc;
        u32 *cmds;
        u32 size;
+       u32 tail;
+       u32 head;
+       u32 space;
        bool broken;
 };