[Intel-gfx] [PATCH 2/2] drm/i915/guc: Convert ct buffer to iosys_map

[Lucas De Marchi]

On Tue, Mar 08, 2022 at 05:38:45PM -0800, Lucas De Marchi wrote:
>and in some other places too. I will try to finish the review later.
>

getting back to the rest now.

>
>>		tail = (tail + 1) % size;
>>	}
>>	GEM_BUG_ON(tail > size);
>>@@ -442,13 +471,14 @@ static int ct_write(struct intel_guc_ct *ct,
>>	atomic_sub(len + GUC_CTB_HDR_LEN, &ctb->space);
>>
>>	/* now update descriptor */
>>-	WRITE_ONCE(desc->tail, tail);
>>+	ct_desc_write(&desc, tail, tail);
>>
>>	return 0;
>>
>>corrupted:
>>	CT_ERROR(ct, "Corrupted descriptor head=%u tail=%u status=%#x\n",
>>-		 desc->head, desc->tail, desc->status);
>>+		 ct_desc_read(&desc, head), ct_desc_read(&desc, tail),
>>+		 ct_desc_read(&desc, status));
>>	ctb->broken = true;
>>	return -EPIPE;
>>}
>>@@ -499,20 +529,21 @@ static inline bool ct_deadlocked(struct intel_guc_ct *ct)
>>	bool ret = ktime_ms_delta(ktime_get(), ct->stall_time) > timeout;
>>
>>	if (unlikely(ret)) {
>>-		struct guc_ct_buffer_desc *send = ct->ctbs.send.desc;
>>-		struct guc_ct_buffer_desc *recv = ct->ctbs.send.desc;
>>+		struct iosys_map send = ct->ctbs.send.desc_map;
>>+		struct iosys_map recv = ct->ctbs.recv.desc_map;

you should only do a copy of the struct in places that you need to
(temporarily) increement the mapping or to overly another struct from an
offset of the original

Here you just use it as an alias for read operations, so a pointer would
do fine.

>>
>>		CT_ERROR(ct, "Communication stalled for %lld ms, desc status=%#x,%#x\n",
>>			 ktime_ms_delta(ktime_get(), ct->stall_time),
>>-			 send->status, recv->status);
>>+			 ct_desc_read(&send, status),
>>+			 ct_desc_read(&recv, status));
>>		CT_ERROR(ct, "H2G Space: %u (Bytes)\n",
>>			 atomic_read(&ct->ctbs.send.space) * 4);
>>-		CT_ERROR(ct, "Head: %u (Dwords)\n", ct->ctbs.send.desc->head);
>>-		CT_ERROR(ct, "Tail: %u (Dwords)\n", ct->ctbs.send.desc->tail);
>>+		CT_ERROR(ct, "Head: %u (Dwords)\n", ct_desc_read(&send, head));
>>+		CT_ERROR(ct, "Tail: %u (Dwords)\n", ct_desc_read(&send, tail));
>>		CT_ERROR(ct, "G2H Space: %u (Bytes)\n",
>>			 atomic_read(&ct->ctbs.recv.space) * 4);
>>-		CT_ERROR(ct, "Head: %u\n (Dwords)", ct->ctbs.recv.desc->head);
>>-		CT_ERROR(ct, "Tail: %u\n (Dwords)", ct->ctbs.recv.desc->tail);
>>+		CT_ERROR(ct, "Head: %u\n (Dwords)", ct_desc_read(&recv, head));
>>+		CT_ERROR(ct, "Tail: %u\n (Dwords)", ct_desc_read(&recv, tail));
>>
>>		ct->ctbs.send.broken = true;
>>	}
>>@@ -549,18 +580,20 @@ static inline void g2h_release_space(struct intel_guc_ct *ct, u32 g2h_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;
>>+	struct iosys_map desc = ctb->desc_map;

same thing here, and in other places in this patch. The rest of the
patch seems to have the same pattern as things suggested above.

Lucas De Marchi