All of lore.kernel.org
 help / color / mirror / Atom feed
* [Intel-gfx] [PATCH 0/2] drm/i915/guc: Refactor CT access to use iosys_map
@ 2022-03-08  9:38 Mullati Siva
  2022-03-08  9:38 ` [Intel-gfx] [PATCH 1/2] iosys-map: Add a helper for pointer difference Mullati Siva
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Mullati Siva @ 2022-03-08  9:38 UTC (permalink / raw)
  To: intel-gfx, siva.mullati; +Cc: lucas.demarchi

From: Siva Mullati <siva.mullati@intel.com>

This is continuation to the below patch series to use iosys map
APIs to use CT commands and descriptors. 
https://patchwork.freedesktop.org/series/99711/


Siva Mullati (2):
  iosys-map: Add a helper for pointer difference
  drm/i915/guc: Convert ct buffer to iosys_map

 drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 170 +++++++++++++---------
 drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h |   9 +-
 include/linux/iosys-map.h                 |  21 +++
 3 files changed, 131 insertions(+), 69 deletions(-)

-- 
2.33.0


^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Intel-gfx] [PATCH 1/2] iosys-map: Add a helper for pointer difference
  2022-03-08  9:38 [Intel-gfx] [PATCH 0/2] drm/i915/guc: Refactor CT access to use iosys_map Mullati Siva
@ 2022-03-08  9:38 ` Mullati Siva
  2022-03-09  1:13   ` Lucas De Marchi
  2022-03-08  9:38 ` [Intel-gfx] [PATCH 2/2] drm/i915/guc: Convert ct buffer to iosys_map Mullati Siva
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Mullati Siva @ 2022-03-08  9:38 UTC (permalink / raw)
  To: intel-gfx, siva.mullati; +Cc: lucas.demarchi

From: Siva Mullati <siva.mullati@intel.com>

iosys_map_ptrdiff to get the difference in address of
same memory type.

Signed-off-by: Siva Mullati <siva.mullati@intel.com>
---
 include/linux/iosys-map.h | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/include/linux/iosys-map.h b/include/linux/iosys-map.h
index e69a002d5aa4..8987f69ec1e9 100644
--- a/include/linux/iosys-map.h
+++ b/include/linux/iosys-map.h
@@ -8,6 +8,7 @@
 
 #include <linux/io.h>
 #include <linux/string.h>
+#include <linux/types.h>
 
 /**
  * DOC: overview
@@ -208,6 +209,26 @@ static inline bool iosys_map_is_equal(const struct iosys_map *lhs,
 		return lhs->vaddr == rhs->vaddr;
 }
 
+/**
+ * iosys_map_ptrdiff - Difference of two iosys mapping addresses of same memory type
+ * @lhs:       The iosys_map structure
+ * @rhs:       A iosys_map structure to compare with
+ *
+ * Two iosys mapping structures of same memory type with the differences
+ * in address within that memory.
+ *
+ * Returns:
+ * Address difference of two memory locations with same memory type.
+ */
+static inline ptrdiff_t iosys_map_ptrdiff(const struct iosys_map *lhs,
+					  const struct iosys_map *rhs)
+{
+	if (lhs->is_iomem)
+		return lhs->vaddr_iomem - rhs->vaddr_iomem;
+	else
+		return lhs->vaddr - rhs->vaddr;
+}
+
 /**
  * iosys_map_is_null - Tests for a iosys mapping to be NULL
  * @map:	The iosys_map structure
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] [PATCH 2/2] drm/i915/guc: Convert ct buffer to iosys_map
  2022-03-08  9:38 [Intel-gfx] [PATCH 0/2] drm/i915/guc: Refactor CT access to use iosys_map Mullati Siva
  2022-03-08  9:38 ` [Intel-gfx] [PATCH 1/2] iosys-map: Add a helper for pointer difference Mullati Siva
@ 2022-03-08  9:38 ` Mullati Siva
  2022-03-09  1:38   ` Lucas De Marchi
  2022-03-08 22:08 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Refactor CT access to use iosys_map Patchwork
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 12+ messages in thread
From: Mullati Siva @ 2022-03-08  9:38 UTC (permalink / raw)
  To: intel-gfx, siva.mullati; +Cc: lucas.demarchi

From: Siva Mullati <siva.mullati@intel.com>

Convert CT commands and descriptors to use iosys_map rather
than plain pointer and save it in the intel_guc_ct_buffer struct.
This will help with ct_write and ct_read for cmd send and receive
 after the initialization by abstracting the IO vs system memory.

Signed-off-by: Siva Mullati <siva.mullati@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 170 +++++++++++++---------
 drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h |   9 +-
 2 files changed, 110 insertions(+), 69 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 f01325cd1b62..457deca1c25a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
@@ -44,6 +44,11 @@ static inline struct drm_device *ct_to_drm(struct intel_guc_ct *ct)
 #define CT_PROBE_ERROR(_ct, _fmt, ...) \
 	i915_probe_error(ct_to_i915(ct), "CT: " _fmt, ##__VA_ARGS__)
 
+#define ct_desc_read(desc_map_, field_) \
+	iosys_map_rd_field(desc_map_, 0, struct guc_ct_buffer_desc, field_)
+#define ct_desc_write(desc_map_, field_, val_) \
+	iosys_map_wr_field(desc_map_, 0, struct guc_ct_buffer_desc, field_, val_)
+
 /**
  * DOC: CTB Blob
  *
@@ -113,9 +118,9 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
 	init_waitqueue_head(&ct->wq);
 }
 
-static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc)
+static void guc_ct_buffer_desc_init(struct iosys_map *desc)
 {
-	memset(desc, 0, sizeof(*desc));
+	iosys_map_memset(desc, 0, 0, sizeof(struct guc_ct_buffer_desc));
 }
 
 static void guc_ct_buffer_reset(struct intel_guc_ct_buffer *ctb)
@@ -128,17 +133,24 @@ static void guc_ct_buffer_reset(struct intel_guc_ct_buffer *ctb)
 	space = CIRC_SPACE(ctb->tail, ctb->head, ctb->size) - ctb->resv_space;
 	atomic_set(&ctb->space, space);
 
-	guc_ct_buffer_desc_init(ctb->desc);
+	guc_ct_buffer_desc_init(&ctb->desc_map);
 }
 
 static void guc_ct_buffer_init(struct intel_guc_ct_buffer *ctb,
-			       struct guc_ct_buffer_desc *desc,
-			       u32 *cmds, u32 size_in_bytes, u32 resv_space)
+			       void *desc, void *cmds, u32 size_in_bytes,
+			       u32 resv_space, bool lmem)
 {
 	GEM_BUG_ON(size_in_bytes % 4);
 
-	ctb->desc = desc;
-	ctb->cmds = cmds;
+	if (lmem) {
+		iosys_map_set_vaddr_iomem(&ctb->desc_map,
+					  (void __iomem *)desc);
+		iosys_map_set_vaddr_iomem(&ctb->cmds_map,
+					  (void __iomem *)cmds);
+	} else {
+		iosys_map_set_vaddr(&ctb->desc_map, desc);
+		iosys_map_set_vaddr(&ctb->cmds_map, cmds);
+	}
 	ctb->size = size_in_bytes / 4;
 	ctb->resv_space = resv_space / 4;
 
@@ -218,13 +230,12 @@ static int ct_register_buffer(struct intel_guc_ct *ct, bool send,
 int intel_guc_ct_init(struct intel_guc_ct *ct)
 {
 	struct intel_guc *guc = ct_to_guc(ct);
-	struct guc_ct_buffer_desc *desc;
 	u32 blob_size;
 	u32 cmds_size;
 	u32 resv_space;
-	void *blob;
-	u32 *cmds;
+	void *blob, *desc, *cmds;
 	int err;
+	bool lmem;
 
 	err = i915_inject_probe_error(guc_to_gt(guc)->i915, -ENXIO);
 	if (err)
@@ -242,6 +253,8 @@ int intel_guc_ct_init(struct intel_guc_ct *ct)
 
 	CT_DEBUG(ct, "base=%#x size=%u\n", intel_guc_ggtt_offset(guc, ct->vma), blob_size);
 
+	lmem = i915_gem_object_is_lmem(ct->vma->obj);
+
 	/* store pointers to desc and cmds for send ctb */
 	desc = blob;
 	cmds = blob + 2 * CTB_DESC_SIZE;
@@ -251,7 +264,8 @@ int intel_guc_ct_init(struct intel_guc_ct *ct)
 		 ptrdiff(desc, blob), ptrdiff(cmds, blob), cmds_size,
 		 resv_space);
 
-	guc_ct_buffer_init(&ct->ctbs.send, desc, cmds, cmds_size, resv_space);
+	guc_ct_buffer_init(&ct->ctbs.send,
+			   desc, cmds, cmds_size, resv_space, lmem);
 
 	/* store pointers to desc and cmds for recv ctb */
 	desc = blob + CTB_DESC_SIZE;
@@ -262,7 +276,8 @@ int intel_guc_ct_init(struct intel_guc_ct *ct)
 		 ptrdiff(desc, blob), ptrdiff(cmds, blob), cmds_size,
 		 resv_space);
 
-	guc_ct_buffer_init(&ct->ctbs.recv, desc, cmds, cmds_size, resv_space);
+	guc_ct_buffer_init(&ct->ctbs.recv,
+			   desc, cmds, cmds_size, resv_space, lmem);
 
 	return 0;
 }
@@ -279,6 +294,10 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct)
 
 	tasklet_kill(&ct->receive_tasklet);
 	i915_vma_unpin_and_release(&ct->vma, I915_VMA_RELEASE_MAP);
+	iosys_map_clear(&ct->ctbs.send.desc_map);
+	iosys_map_clear(&ct->ctbs.send.cmds_map);
+	iosys_map_clear(&ct->ctbs.recv.desc_map);
+	iosys_map_clear(&ct->ctbs.recv.cmds_map);
 	memset(ct, 0, sizeof(*ct));
 }
 
@@ -291,6 +310,7 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct)
 int intel_guc_ct_enable(struct intel_guc_ct *ct)
 {
 	struct intel_guc *guc = ct_to_guc(ct);
+	struct iosys_map blob_map;
 	u32 base, desc, cmds, size;
 	void *blob;
 	int err;
@@ -302,9 +322,14 @@ int intel_guc_ct_enable(struct intel_guc_ct *ct)
 	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(ct->vma->obj));
 	base = intel_guc_ggtt_offset(guc, ct->vma);
 
-	/* blob should start with send descriptor */
 	blob = __px_vaddr(ct->vma->obj);
-	GEM_BUG_ON(blob != ct->ctbs.send.desc);
+	if (i915_gem_object_is_lmem(ct->vma->obj))
+		iosys_map_set_vaddr_iomem(&blob_map, (void __iomem *)blob);
+	else
+		iosys_map_set_vaddr(&blob_map, blob);
+
+	/* blob should start with send descriptor */
+	GEM_BUG_ON(!iosys_map_is_equal(&blob_map, &ct->ctbs.send.desc_map));
 
 	/* (re)initialize descriptors */
 	guc_ct_buffer_reset(&ct->ctbs.send);
@@ -314,15 +339,15 @@ int intel_guc_ct_enable(struct intel_guc_ct *ct)
 	 * Register both CT buffers starting with RECV buffer.
 	 * Descriptors are in first half of the blob.
 	 */
-	desc = base + ptrdiff(ct->ctbs.recv.desc, blob);
-	cmds = base + ptrdiff(ct->ctbs.recv.cmds, blob);
+	desc = base + iosys_map_ptrdiff(&ct->ctbs.recv.desc_map, &blob_map);
+	cmds = base + iosys_map_ptrdiff(&ct->ctbs.recv.cmds_map, &blob_map);
 	size = ct->ctbs.recv.size * 4;
 	err = ct_register_buffer(ct, false, desc, cmds, size);
 	if (unlikely(err))
 		goto err_out;
 
-	desc = base + ptrdiff(ct->ctbs.send.desc, blob);
-	cmds = base + ptrdiff(ct->ctbs.send.cmds, blob);
+	desc = base + iosys_map_ptrdiff(&ct->ctbs.send.desc_map, &blob_map);
+	cmds = base + iosys_map_ptrdiff(&ct->ctbs.send.cmds_map, &blob_map);
 	size = ct->ctbs.send.size * 4;
 	err = ct_register_buffer(ct, true, desc, cmds, size);
 	if (unlikely(err))
@@ -371,31 +396,35 @@ static int ct_write(struct intel_guc_ct *ct,
 		    u32 fence, u32 flags)
 {
 	struct intel_guc_ct_buffer *ctb = &ct->ctbs.send;
-	struct guc_ct_buffer_desc *desc = ctb->desc;
+	struct iosys_map desc = ctb->desc_map;
+	struct iosys_map cmds = ctb->cmds_map;
 	u32 tail = ctb->tail;
 	u32 size = ctb->size;
 	u32 header;
 	u32 hxg;
 	u32 type;
-	u32 *cmds = ctb->cmds;
+	u32 status = ct_desc_read(&desc, status);
 	unsigned int i;
 
-	if (unlikely(desc->status))
+	if (unlikely(status))
 		goto corrupted;
 
 	GEM_BUG_ON(tail > size);
 
 #ifdef CONFIG_DRM_I915_DEBUG_GUC
-	if (unlikely(tail != READ_ONCE(desc->tail))) {
+	if (unlikely(tail != ct_desc_read(&desc, tail))) {
 		CT_ERROR(ct, "Tail was modified %u != %u\n",
-			 desc->tail, tail);
-		desc->status |= GUC_CTB_STATUS_MISMATCH;
+			 ct_desc_read(&desc, tail), tail);
+		status |= GUC_CTB_STATUS_MISMATCH;
+		ct_desc_write(&desc, status, status);
 		goto corrupted;
 	}
-	if (unlikely(READ_ONCE(desc->head) >= size)) {
+	if (unlikely(ct_desc_read(&desc, head) >= size)) {
 		CT_ERROR(ct, "Invalid head offset %u >= %u)\n",
-			 desc->head, size);
-		desc->status |= GUC_CTB_STATUS_OVERFLOW;
+			 ct_desc_read(&desc, head), size);
+		status = ct_desc_read(&desc, status) |
+			GUC_CTB_STATUS_OVERFLOW;
+		ct_desc_write(&desc, status, status);
 		goto corrupted;
 	}
 #endif
@@ -418,14 +447,14 @@ static int ct_write(struct intel_guc_ct *ct,
 	CT_DEBUG(ct, "writing (tail %u) %*ph %*ph %*ph\n",
 		 tail, 4, &header, 4, &hxg, 4 * (len - 1), &action[1]);
 
-	cmds[tail] = header;
+	iosys_map_wr(&cmds, (4 * tail), u32, header);
 	tail = (tail + 1) % size;
 
-	cmds[tail] = hxg;
+	iosys_map_wr(&cmds, (4 * tail), u32, hxg);
 	tail = (tail + 1) % size;
 
 	for (i = 1; i < len; i++) {
-		cmds[tail] = action[i];
+		iosys_map_wr(&cmds, (4 * tail), u32, action[i]);
 		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;
 
 		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;
 	u32 head;
 	u32 space;
+	u32 status = ct_desc_read(&desc, status);
 
 	if (atomic_read(&ctb->space) >= len_dw)
 		return true;
 
-	head = READ_ONCE(desc->head);
+	head = ct_desc_read(&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;
+		status |= GUC_CTB_STATUS_OVERFLOW;
+		ct_desc_write(&desc, status, status);
 		ctb->broken = true;
 		return false;
 	}
@@ -803,11 +836,12 @@ static void ct_free_msg(struct ct_incoming_msg *msg)
 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;
+	struct iosys_map desc = ctb->desc_map;
+	struct iosys_map cmds = ctb->cmds_map;
 	u32 head = ctb->head;
-	u32 tail = READ_ONCE(desc->tail);
+	u32 tail = ct_desc_read(&desc, tail);
 	u32 size = ctb->size;
-	u32 *cmds = ctb->cmds;
+	u32 status = ct_desc_read(&desc, status);
 	s32 available;
 	unsigned int len;
 	unsigned int i;
@@ -816,23 +850,26 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
 	if (unlikely(ctb->broken))
 		return -EPIPE;
 
-	if (unlikely(desc->status))
+	if (unlikely(status))
 		goto corrupted;
 
 	GEM_BUG_ON(head > size);
 
 #ifdef CONFIG_DRM_I915_DEBUG_GUC
-	if (unlikely(head != READ_ONCE(desc->head))) {
+	if (unlikely(head != ct_desc_read(&desc, head))) {
 		CT_ERROR(ct, "Head was modified %u != %u\n",
-			 desc->head, head);
-		desc->status |= GUC_CTB_STATUS_MISMATCH;
+			 ct_desc_read(&desc, head), head);
+		status |= GUC_CTB_STATUS_MISMATCH;
+		ct_desc_write(&desc, status, status);
 		goto corrupted;
 	}
 #endif
 	if (unlikely(tail >= size)) {
 		CT_ERROR(ct, "Invalid tail offset %u >= %u)\n",
 			 tail, size);
-		desc->status |= GUC_CTB_STATUS_OVERFLOW;
+		status = ct_desc_read(&desc, status) |
+			GUC_CTB_STATUS_OVERFLOW;
+		ct_desc_write(&desc, status, status);
 		goto corrupted;
 	}
 
@@ -849,7 +886,7 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
 	CT_DEBUG(ct, "available %d (%u:%u:%u)\n", available, head, tail, size);
 	GEM_BUG_ON(available < 0);
 
-	header = cmds[head];
+	header = iosys_map_rd(&cmds, (4 * head), u32);
 	head = (head + 1) % size;
 
 	/* message len with header */
@@ -857,11 +894,13 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
 	if (unlikely(len > (u32)available)) {
 		CT_ERROR(ct, "Incomplete message %*ph %*ph %*ph\n",
 			 4, &header,
+			 4 * (head + available - 1 > size ? size - head :
+			      available - 1), (cmds.vaddr + (4 * head)),
 			 4 * (head + available - 1 > size ?
-			      size - head : available - 1), &cmds[head],
-			 4 * (head + available - 1 > size ?
-			      available - 1 - size + head : 0), &cmds[0]);
-		desc->status |= GUC_CTB_STATUS_UNDERFLOW;
+			      available - 1 - size + head : 0), cmds.vaddr);
+		status = ct_desc_read(&desc, status) |
+			GUC_CTB_STATUS_UNDERFLOW;
+		ct_desc_write(&desc, status, status);
 		goto corrupted;
 	}
 
@@ -869,17 +908,17 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
 	if (!*msg) {
 		CT_ERROR(ct, "No memory for message %*ph %*ph %*ph\n",
 			 4, &header,
+			 4 * (head + available - 1 > size ? size - head :
+			      available - 1), (cmds.vaddr + (4 * head)),
 			 4 * (head + available - 1 > size ?
-			      size - head : available - 1), &cmds[head],
-			 4 * (head + available - 1 > size ?
-			      available - 1 - size + head : 0), &cmds[0]);
+			      available - 1 - size + head : 0), cmds.vaddr);
 		return available;
 	}
 
 	(*msg)->msg[0] = header;
 
 	for (i = 1; i < len; i++) {
-		(*msg)->msg[i] = cmds[head];
+		(*msg)->msg[i] = iosys_map_rd(&cmds, (4 * head), u32);
 		head = (head + 1) % size;
 	}
 	CT_DEBUG(ct, "received %*ph\n", 4 * len, (*msg)->msg);
@@ -888,13 +927,14 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
 	ctb->head = head;
 
 	/* now update descriptor */
-	WRITE_ONCE(desc->head, head);
+	ct_desc_write(&desc, head, head);
 
 	return available - len;
 
 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;
 }
@@ -1211,13 +1251,13 @@ void intel_guc_ct_print_info(struct intel_guc_ct *ct,
 	drm_printf(p, "H2G Space: %u\n",
 		   atomic_read(&ct->ctbs.send.space) * 4);
 	drm_printf(p, "Head: %u\n",
-		   ct->ctbs.send.desc->head);
+		   ct_desc_read(&ct->ctbs.send.desc_map, head));
 	drm_printf(p, "Tail: %u\n",
-		   ct->ctbs.send.desc->tail);
+		   ct_desc_read(&ct->ctbs.send.desc_map, tail));
 	drm_printf(p, "G2H Space: %u\n",
 		   atomic_read(&ct->ctbs.recv.space) * 4);
 	drm_printf(p, "Head: %u\n",
-		   ct->ctbs.recv.desc->head);
+		   ct_desc_read(&ct->ctbs.recv.desc_map, head));
 	drm_printf(p, "Tail: %u\n",
-		   ct->ctbs.recv.desc->tail);
+		   ct_desc_read(&ct->ctbs.recv.desc_map, tail));
 }
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 f709a19c7e21..867fe13fb47d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
@@ -7,6 +7,7 @@
 #define _INTEL_GUC_CT_H_
 
 #include <linux/interrupt.h>
+#include <linux/iosys-map.h>
 #include <linux/spinlock.h>
 #include <linux/workqueue.h>
 #include <linux/ktime.h>
@@ -32,8 +33,8 @@ struct drm_printer;
  * holds the commands.
  *
  * @lock: protects access to the commands buffer and buffer descriptor
- * @desc: pointer to the buffer descriptor
- * @cmds: pointer to the commands buffer
+ * @desc: iosys map to the buffer descriptor
+ * @cmds: iosys map to the commands buffer
  * @size: size of the commands buffer in dwords
  * @resv_space: reserved space in buffer in dwords
  * @head: local shadow copy of head in dwords
@@ -43,8 +44,8 @@ struct drm_printer;
  */
 struct intel_guc_ct_buffer {
 	spinlock_t lock;
-	struct guc_ct_buffer_desc *desc;
-	u32 *cmds;
+	struct iosys_map desc_map;
+	struct iosys_map cmds_map;
 	u32 size;
 	u32 resv_space;
 	u32 tail;
-- 
2.33.0


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Refactor CT access to use iosys_map
  2022-03-08  9:38 [Intel-gfx] [PATCH 0/2] drm/i915/guc: Refactor CT access to use iosys_map Mullati Siva
  2022-03-08  9:38 ` [Intel-gfx] [PATCH 1/2] iosys-map: Add a helper for pointer difference Mullati Siva
  2022-03-08  9:38 ` [Intel-gfx] [PATCH 2/2] drm/i915/guc: Convert ct buffer to iosys_map Mullati Siva
@ 2022-03-08 22:08 ` Patchwork
  2022-03-08 22:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
  2022-03-09  5:47 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2022-03-08 22:08 UTC (permalink / raw)
  To: Mullati Siva; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc: Refactor CT access to use iosys_map
URL   : https://patchwork.freedesktop.org/series/101148/
State : warning

== Summary ==

$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/i915/guc: Refactor CT access to use iosys_map
  2022-03-08  9:38 [Intel-gfx] [PATCH 0/2] drm/i915/guc: Refactor CT access to use iosys_map Mullati Siva
                   ` (2 preceding siblings ...)
  2022-03-08 22:08 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Refactor CT access to use iosys_map Patchwork
@ 2022-03-08 22:40 ` Patchwork
  2022-03-09  5:47 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork
  4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2022-03-08 22:40 UTC (permalink / raw)
  To: Mullati Siva; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 8011 bytes --]

== Series Details ==

Series: drm/i915/guc: Refactor CT access to use iosys_map
URL   : https://patchwork.freedesktop.org/series/101148/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11337 -> Patchwork_22509
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/index.html

Participating hosts (46 -> 42)
------------------------------

  Additional (1): fi-icl-u2 
  Missing    (5): shard-tglu fi-bsw-cyan fi-ctg-p8600 shard-rkl shard-dg1 

Known issues
------------

  Here are the changes found in Patchwork_22509 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@fork-gfx0:
    - fi-icl-u2:          NOTRUN -> [SKIP][1] ([fdo#109315]) +17 similar issues
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-icl-u2/igt@amdgpu/amd_cs_nop@fork-gfx0.html

  * igt@gem_huc_copy@huc-copy:
    - fi-skl-6600u:       NOTRUN -> [SKIP][2] ([fdo#109271] / [i915#2190])
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-skl-6600u/igt@gem_huc_copy@huc-copy.html
    - fi-icl-u2:          NOTRUN -> [SKIP][3] ([i915#2190])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-icl-u2/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@parallel-random-engines:
    - fi-icl-u2:          NOTRUN -> [SKIP][4] ([i915#4613]) +3 similar issues
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-icl-u2/igt@gem_lmem_swapping@parallel-random-engines.html

  * igt@gem_lmem_swapping@verify-random:
    - fi-skl-6600u:       NOTRUN -> [SKIP][5] ([fdo#109271] / [i915#4613]) +3 similar issues
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-skl-6600u/igt@gem_lmem_swapping@verify-random.html

  * igt@i915_selftest@live@hangcheck:
    - bat-dg1-5:          [PASS][6] -> [DMESG-FAIL][7] ([i915#4494] / [i915#4957])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/bat-dg1-5/igt@i915_selftest@live@hangcheck.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/bat-dg1-5/igt@i915_selftest@live@hangcheck.html

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-icl-u2:          NOTRUN -> [SKIP][8] ([fdo#111827]) +8 similar issues
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-icl-u2/igt@kms_chamelium@hdmi-hpd-fast.html

  * igt@kms_chamelium@vga-edid-read:
    - fi-skl-6600u:       NOTRUN -> [SKIP][9] ([fdo#109271] / [fdo#111827]) +8 similar issues
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-skl-6600u/igt@kms_chamelium@vga-edid-read.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic:
    - fi-icl-u2:          NOTRUN -> [SKIP][10] ([fdo#109278]) +2 similar issues
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-icl-u2/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-atomic.html

  * igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy:
    - fi-skl-6600u:       NOTRUN -> [SKIP][11] ([fdo#109271]) +21 similar issues
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-skl-6600u/igt@kms_cursor_legacy@basic-busy-flip-before-cursor-legacy.html

  * igt@kms_flip@basic-plain-flip@c-dp2:
    - fi-cfl-8109u:       [PASS][12] -> [DMESG-WARN][13] ([i915#165])
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/fi-cfl-8109u/igt@kms_flip@basic-plain-flip@c-dp2.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-cfl-8109u/igt@kms_flip@basic-plain-flip@c-dp2.html

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-icl-u2:          NOTRUN -> [SKIP][14] ([fdo#109285])
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-icl-u2/igt@kms_force_connector_basic@force-load-detect.html

  * igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d:
    - fi-skl-6600u:       NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#533])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-skl-6600u/igt@kms_pipe_crc_basic@compare-crc-sanitycheck-pipe-d.html

  * igt@kms_pipe_crc_basic@read-crc-pipe-b:
    - fi-cfl-8109u:       [PASS][16] -> [DMESG-WARN][17] ([i915#165] / [i915#295]) +14 similar issues
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/fi-cfl-8109u/igt@kms_pipe_crc_basic@read-crc-pipe-b.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-cfl-8109u/igt@kms_pipe_crc_basic@read-crc-pipe-b.html

  * igt@prime_vgem@basic-userptr:
    - fi-icl-u2:          NOTRUN -> [SKIP][18] ([i915#3301])
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-icl-u2/igt@prime_vgem@basic-userptr.html

  * igt@runner@aborted:
    - fi-bdw-5557u:       NOTRUN -> [FAIL][19] ([i915#2426] / [i915#4312])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-bdw-5557u/igt@runner@aborted.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s3@smem:
    - fi-skl-6600u:       [INCOMPLETE][20] ([i915#4547]) -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/fi-skl-6600u/igt@gem_exec_suspend@basic-s3@smem.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/fi-skl-6600u/igt@gem_exec_suspend@basic-s3@smem.html

  * igt@i915_selftest@live@requests:
    - {bat-rpls-2}:       [INCOMPLETE][22] -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/bat-rpls-2/igt@i915_selftest@live@requests.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/bat-rpls-2/igt@i915_selftest@live@requests.html

  * igt@kms_flip@basic-flip-vs-modeset@a-edp1:
    - {bat-adlp-6}:       [DMESG-WARN][24] ([i915#3576]) -> [PASS][25]
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/bat-adlp-6/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/bat-adlp-6/igt@kms_flip@basic-flip-vs-modeset@a-edp1.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109285]: https://bugs.freedesktop.org/show_bug.cgi?id=109285
  [fdo#109315]: https://bugs.freedesktop.org/show_bug.cgi?id=109315
  [fdo#111827]: https://bugs.freedesktop.org/show_bug.cgi?id=111827
  [i915#165]: https://gitlab.freedesktop.org/drm/intel/issues/165
  [i915#2190]: https://gitlab.freedesktop.org/drm/intel/issues/2190
  [i915#2426]: https://gitlab.freedesktop.org/drm/intel/issues/2426
  [i915#295]: https://gitlab.freedesktop.org/drm/intel/issues/295
  [i915#3301]: https://gitlab.freedesktop.org/drm/intel/issues/3301
  [i915#3576]: https://gitlab.freedesktop.org/drm/intel/issues/3576
  [i915#4312]: https://gitlab.freedesktop.org/drm/intel/issues/4312
  [i915#4391]: https://gitlab.freedesktop.org/drm/intel/issues/4391
  [i915#4494]: https://gitlab.freedesktop.org/drm/intel/issues/4494
  [i915#4547]: https://gitlab.freedesktop.org/drm/intel/issues/4547
  [i915#4613]: https://gitlab.freedesktop.org/drm/intel/issues/4613
  [i915#4957]: https://gitlab.freedesktop.org/drm/intel/issues/4957
  [i915#533]: https://gitlab.freedesktop.org/drm/intel/issues/533


Build changes
-------------

  * Linux: CI_DRM_11337 -> Patchwork_22509

  CI-20190529: 20190529
  CI_DRM_11337: 69f3f2cf125ccd5ad74d79202ba11aae6e21fb0f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_6368: 60e5ffca027b38398c279fba0f5a1b7517aa6061 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_22509: 2f2634026857ac37089a64a127228d2b1af69515 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

2f2634026857 drm/i915/guc: Convert ct buffer to iosys_map
4e5fee6e29d1 iosys-map: Add a helper for pointer difference

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/index.html

[-- Attachment #2: Type: text/html, Size: 9563 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] iosys-map: Add a helper for pointer difference
  2022-03-08  9:38 ` [Intel-gfx] [PATCH 1/2] iosys-map: Add a helper for pointer difference Mullati Siva
@ 2022-03-09  1:13   ` Lucas De Marchi
  2022-03-14  5:40     ` Siva Mullati
  0 siblings, 1 reply; 12+ messages in thread
From: Lucas De Marchi @ 2022-03-09  1:13 UTC (permalink / raw)
  To: Mullati Siva; +Cc: intel-gfx

On Tue, Mar 08, 2022 at 03:08:04PM +0530, Mullati Siva wrote:
>From: Siva Mullati <siva.mullati@intel.com>
>
>iosys_map_ptrdiff to get the difference in address of
>same memory type.
>
>Signed-off-by: Siva Mullati <siva.mullati@intel.com>
>---
> include/linux/iosys-map.h | 21 +++++++++++++++++++++
> 1 file changed, 21 insertions(+)
>
>diff --git a/include/linux/iosys-map.h b/include/linux/iosys-map.h
>index e69a002d5aa4..8987f69ec1e9 100644
>--- a/include/linux/iosys-map.h
>+++ b/include/linux/iosys-map.h
>@@ -8,6 +8,7 @@
>
> #include <linux/io.h>
> #include <linux/string.h>
>+#include <linux/types.h>
>
> /**
>  * DOC: overview
>@@ -208,6 +209,26 @@ static inline bool iosys_map_is_equal(const struct iosys_map *lhs,
> 		return lhs->vaddr == rhs->vaddr;
> }
>
>+/**
>+ * iosys_map_ptrdiff - Difference of two iosys mapping addresses of same memory type
>+ * @lhs:       The iosys_map structure
>+ * @rhs:       A iosys_map structure to compare with
>+ *
>+ * Two iosys mapping structures of same memory type with the differences
>+ * in address within that memory.
>+ *
>+ * Returns:
>+ * Address difference of two memory locations with same memory type.
>+ */
>+static inline ptrdiff_t iosys_map_ptrdiff(const struct iosys_map *lhs,
>+					  const struct iosys_map *rhs)
>+{
>+	if (lhs->is_iomem)

the other interfaces always check both arguments to make sure they are
both iomem. So if we want this interface we will want to check like
that.

I'm not sure this is really needed, but will depend on the secon patch,
let's wait to settle this discussion there.


thanks
Lucas De Marchi

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 2/2] drm/i915/guc: Convert ct buffer to iosys_map
  2022-03-08  9:38 ` [Intel-gfx] [PATCH 2/2] drm/i915/guc: Convert ct buffer to iosys_map Mullati Siva
@ 2022-03-09  1:38   ` Lucas De Marchi
  2022-03-14  5:53     ` Siva Mullati
  2022-03-17  5:12     ` Lucas De Marchi
  0 siblings, 2 replies; 12+ messages in thread
From: Lucas De Marchi @ 2022-03-09  1:38 UTC (permalink / raw)
  To: Mullati Siva; +Cc: intel-gfx

On Tue, Mar 08, 2022 at 03:08:05PM +0530, Mullati Siva wrote:
>From: Siva Mullati <siva.mullati@intel.com>
>
>Convert CT commands and descriptors to use iosys_map rather
>than plain pointer and save it in the intel_guc_ct_buffer struct.
>This will help with ct_write and ct_read for cmd send and receive
> after the initialization by abstracting the IO vs system memory.
>
>Signed-off-by: Siva Mullati <siva.mullati@intel.com>
>---
> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 170 +++++++++++++---------
> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h |   9 +-
> 2 files changed, 110 insertions(+), 69 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 f01325cd1b62..457deca1c25a 100644
>--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>@@ -44,6 +44,11 @@ static inline struct drm_device *ct_to_drm(struct intel_guc_ct *ct)
> #define CT_PROBE_ERROR(_ct, _fmt, ...) \
> 	i915_probe_error(ct_to_i915(ct), "CT: " _fmt, ##__VA_ARGS__)
>
>+#define ct_desc_read(desc_map_, field_) \
>+	iosys_map_rd_field(desc_map_, 0, struct guc_ct_buffer_desc, field_)

one thing I found useful in intel_guc_ads, was to use the first argument
as the struct type instead of map. That's because then I enforce the
right type to be passed rather than a random iosys_map. See :

	#define ads_blob_read(guc_, field_)                                     \
		iosys_map_rd_field(&(guc_)->ads_map, 0, struct __guc_ads_blob, field_)

First arg is expected to be `struct intel_guc *`. Yes, I didn't do this
for info_map_{read,write}, because there were situation in which I had
to pass a info from outside (forcefully from system memory). If you
don't have such case,  I think it would be better to pass the typed
pointer.

>+#define ct_desc_write(desc_map_, field_, val_) \
>+	iosys_map_wr_field(desc_map_, 0, struct guc_ct_buffer_desc, field_, val_)
>+
> /**
>  * DOC: CTB Blob
>  *
>@@ -113,9 +118,9 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
> 	init_waitqueue_head(&ct->wq);
> }
>
>-static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc)
>+static void guc_ct_buffer_desc_init(struct iosys_map *desc)

if you can apply the comment above, then leave it as
struct intel_guc_ct_buffer.

> {
>-	memset(desc, 0, sizeof(*desc));
>+	iosys_map_memset(desc, 0, 0, sizeof(struct guc_ct_buffer_desc));
> }
>
> static void guc_ct_buffer_reset(struct intel_guc_ct_buffer *ctb)
>@@ -128,17 +133,24 @@ static void guc_ct_buffer_reset(struct intel_guc_ct_buffer *ctb)
> 	space = CIRC_SPACE(ctb->tail, ctb->head, ctb->size) - ctb->resv_space;
> 	atomic_set(&ctb->space, space);
>
>-	guc_ct_buffer_desc_init(ctb->desc);
>+	guc_ct_buffer_desc_init(&ctb->desc_map);
> }
>
> static void guc_ct_buffer_init(struct intel_guc_ct_buffer *ctb,
>-			       struct guc_ct_buffer_desc *desc,
>-			       u32 *cmds, u32 size_in_bytes, u32 resv_space)
>+			       void *desc, void *cmds, u32 size_in_bytes,
>+			       u32 resv_space, bool lmem)

bool arguments are really hard to read, because you always have to
lookup the function prototype to understand what that is about.

Here we could turn struct guc_ct_buffer_desc *desc into the map, and let
the caller prepare it for iomem or system memory.

> {
> 	GEM_BUG_ON(size_in_bytes % 4);
>
>-	ctb->desc = desc;
>-	ctb->cmds = cmds;
>+	if (lmem) {
>+		iosys_map_set_vaddr_iomem(&ctb->desc_map,
>+					  (void __iomem *)desc);
>+		iosys_map_set_vaddr_iomem(&ctb->cmds_map,
>+					  (void __iomem *)cmds);
>+	} else {
>+		iosys_map_set_vaddr(&ctb->desc_map, desc);
>+		iosys_map_set_vaddr(&ctb->cmds_map, cmds);
>+	}
> 	ctb->size = size_in_bytes / 4;
> 	ctb->resv_space = resv_space / 4;
>
>@@ -218,13 +230,12 @@ static int ct_register_buffer(struct intel_guc_ct *ct, bool send,
> int intel_guc_ct_init(struct intel_guc_ct *ct)
> {
> 	struct intel_guc *guc = ct_to_guc(ct);
>-	struct guc_ct_buffer_desc *desc;
> 	u32 blob_size;
> 	u32 cmds_size;
> 	u32 resv_space;
>-	void *blob;
>-	u32 *cmds;
>+	void *blob, *desc, *cmds;
> 	int err;
>+	bool lmem;
>
> 	err = i915_inject_probe_error(guc_to_gt(guc)->i915, -ENXIO);
> 	if (err)
>@@ -242,6 +253,8 @@ int intel_guc_ct_init(struct intel_guc_ct *ct)
>
> 	CT_DEBUG(ct, "base=%#x size=%u\n", intel_guc_ggtt_offset(guc, ct->vma), blob_size);
>
>+	lmem = i915_gem_object_is_lmem(ct->vma->obj);
>+
> 	/* store pointers to desc and cmds for send ctb */
> 	desc = blob;
> 	cmds = blob + 2 * CTB_DESC_SIZE;
>@@ -251,7 +264,8 @@ int intel_guc_ct_init(struct intel_guc_ct *ct)
> 		 ptrdiff(desc, blob), ptrdiff(cmds, blob), cmds_size,
> 		 resv_space);
>
>-	guc_ct_buffer_init(&ct->ctbs.send, desc, cmds, cmds_size, resv_space);
>+	guc_ct_buffer_init(&ct->ctbs.send,
>+			   desc, cmds, cmds_size, resv_space, lmem);
>
> 	/* store pointers to desc and cmds for recv ctb */
> 	desc = blob + CTB_DESC_SIZE;
>@@ -262,7 +276,8 @@ int intel_guc_ct_init(struct intel_guc_ct *ct)
> 		 ptrdiff(desc, blob), ptrdiff(cmds, blob), cmds_size,
> 		 resv_space);
>
>-	guc_ct_buffer_init(&ct->ctbs.recv, desc, cmds, cmds_size, resv_space);
>+	guc_ct_buffer_init(&ct->ctbs.recv,
>+			   desc, cmds, cmds_size, resv_space, lmem);
>
> 	return 0;
> }
>@@ -279,6 +294,10 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct)
>
> 	tasklet_kill(&ct->receive_tasklet);
> 	i915_vma_unpin_and_release(&ct->vma, I915_VMA_RELEASE_MAP);
>+	iosys_map_clear(&ct->ctbs.send.desc_map);
>+	iosys_map_clear(&ct->ctbs.send.cmds_map);
>+	iosys_map_clear(&ct->ctbs.recv.desc_map);
>+	iosys_map_clear(&ct->ctbs.recv.cmds_map);
> 	memset(ct, 0, sizeof(*ct));
> }
>
>@@ -291,6 +310,7 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct)
> int intel_guc_ct_enable(struct intel_guc_ct *ct)
> {
> 	struct intel_guc *guc = ct_to_guc(ct);
>+	struct iosys_map blob_map;
> 	u32 base, desc, cmds, size;
> 	void *blob;
> 	int err;
>@@ -302,9 +322,14 @@ int intel_guc_ct_enable(struct intel_guc_ct *ct)
> 	GEM_BUG_ON(!i915_gem_object_has_pinned_pages(ct->vma->obj));
> 	base = intel_guc_ggtt_offset(guc, ct->vma);
>
>-	/* blob should start with send descriptor */
> 	blob = __px_vaddr(ct->vma->obj);
>-	GEM_BUG_ON(blob != ct->ctbs.send.desc);
>+	if (i915_gem_object_is_lmem(ct->vma->obj))
>+		iosys_map_set_vaddr_iomem(&blob_map, (void __iomem *)blob);
>+	else
>+		iosys_map_set_vaddr(&blob_map, blob);

probably better to remove blob and pass __px_vaddr(ct->vma->obj)
directly as argument. This avoids later having users making (wrong)
use of the blob variable, that shouldn't be used anymore after this
point in the function.

other possibility would be to do a blob = NULL here, but I think the
other approach is cleaner.

>+
>+	/* blob should start with send descriptor */
>+	GEM_BUG_ON(!iosys_map_is_equal(&blob_map, &ct->ctbs.send.desc_map));
>
> 	/* (re)initialize descriptors */
> 	guc_ct_buffer_reset(&ct->ctbs.send);
>@@ -314,15 +339,15 @@ int intel_guc_ct_enable(struct intel_guc_ct *ct)
> 	 * Register both CT buffers starting with RECV buffer.
> 	 * Descriptors are in first half of the blob.
> 	 */
>-	desc = base + ptrdiff(ct->ctbs.recv.desc, blob);
>-	cmds = base + ptrdiff(ct->ctbs.recv.cmds, blob);
>+	desc = base + iosys_map_ptrdiff(&ct->ctbs.recv.desc_map, &blob_map);
>+	cmds = base + iosys_map_ptrdiff(&ct->ctbs.recv.cmds_map, &blob_map);
> 	size = ct->ctbs.recv.size * 4;
> 	err = ct_register_buffer(ct, false, desc, cmds, size);
> 	if (unlikely(err))
> 		goto err_out;
>
>-	desc = base + ptrdiff(ct->ctbs.send.desc, blob);
>-	cmds = base + ptrdiff(ct->ctbs.send.cmds, blob);
>+	desc = base + iosys_map_ptrdiff(&ct->ctbs.send.desc_map, &blob_map);
>+	cmds = base + iosys_map_ptrdiff(&ct->ctbs.send.cmds_map, &blob_map);
> 	size = ct->ctbs.send.size * 4;
> 	err = ct_register_buffer(ct, true, desc, cmds, size);
> 	if (unlikely(err))
>@@ -371,31 +396,35 @@ static int ct_write(struct intel_guc_ct *ct,
> 		    u32 fence, u32 flags)
> {
> 	struct intel_guc_ct_buffer *ctb = &ct->ctbs.send;
>-	struct guc_ct_buffer_desc *desc = ctb->desc;
>+	struct iosys_map desc = ctb->desc_map;
>+	struct iosys_map cmds = ctb->cmds_map;
> 	u32 tail = ctb->tail;
> 	u32 size = ctb->size;
> 	u32 header;
> 	u32 hxg;
> 	u32 type;
>-	u32 *cmds = ctb->cmds;
>+	u32 status = ct_desc_read(&desc, status);
> 	unsigned int i;
>
>-	if (unlikely(desc->status))
>+	if (unlikely(status))
> 		goto corrupted;
>
> 	GEM_BUG_ON(tail > size);
>
> #ifdef CONFIG_DRM_I915_DEBUG_GUC
>-	if (unlikely(tail != READ_ONCE(desc->tail))) {
>+	if (unlikely(tail != ct_desc_read(&desc, tail))) {
> 		CT_ERROR(ct, "Tail was modified %u != %u\n",
>-			 desc->tail, tail);
>-		desc->status |= GUC_CTB_STATUS_MISMATCH;
>+			 ct_desc_read(&desc, tail), tail);
>+		status |= GUC_CTB_STATUS_MISMATCH;
>+		ct_desc_write(&desc, status, status);
> 		goto corrupted;
> 	}
>-	if (unlikely(READ_ONCE(desc->head) >= size)) {
>+	if (unlikely(ct_desc_read(&desc, head) >= size)) {
> 		CT_ERROR(ct, "Invalid head offset %u >= %u)\n",
>-			 desc->head, size);
>-		desc->status |= GUC_CTB_STATUS_OVERFLOW;
>+			 ct_desc_read(&desc, head), size);
>+		status = ct_desc_read(&desc, status) |
>+			GUC_CTB_STATUS_OVERFLOW;
>+		ct_desc_write(&desc, status, status);
> 		goto corrupted;
> 	}
> #endif
>@@ -418,14 +447,14 @@ static int ct_write(struct intel_guc_ct *ct,
> 	CT_DEBUG(ct, "writing (tail %u) %*ph %*ph %*ph\n",
> 		 tail, 4, &header, 4, &hxg, 4 * (len - 1), &action[1]);
>
>-	cmds[tail] = header;
>+	iosys_map_wr(&cmds, (4 * tail), u32, header);

hiding sizeof(u32) here. Just make it &cmds[tail]?

> 	tail = (tail + 1) % size;
>
>-	cmds[tail] = hxg;
>+	iosys_map_wr(&cmds, (4 * tail), u32, hxg);

ditto

> 	tail = (tail + 1) % size;
>
> 	for (i = 1; i < len; i++) {
>-		cmds[tail] = action[i];
>+		iosys_map_wr(&cmds, (4 * tail), u32, action[i]);

ditto

and in some other places too. I will try to finish the review later.

Lucas De Marchi

> 		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;
>
> 		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;
> 	u32 head;
> 	u32 space;
>+	u32 status = ct_desc_read(&desc, status);
>
> 	if (atomic_read(&ctb->space) >= len_dw)
> 		return true;
>
>-	head = READ_ONCE(desc->head);
>+	head = ct_desc_read(&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;
>+		status |= GUC_CTB_STATUS_OVERFLOW;
>+		ct_desc_write(&desc, status, status);
> 		ctb->broken = true;
> 		return false;
> 	}
>@@ -803,11 +836,12 @@ static void ct_free_msg(struct ct_incoming_msg *msg)
> 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;
>+	struct iosys_map desc = ctb->desc_map;
>+	struct iosys_map cmds = ctb->cmds_map;
> 	u32 head = ctb->head;
>-	u32 tail = READ_ONCE(desc->tail);
>+	u32 tail = ct_desc_read(&desc, tail);
> 	u32 size = ctb->size;
>-	u32 *cmds = ctb->cmds;
>+	u32 status = ct_desc_read(&desc, status);
> 	s32 available;
> 	unsigned int len;
> 	unsigned int i;
>@@ -816,23 +850,26 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
> 	if (unlikely(ctb->broken))
> 		return -EPIPE;
>
>-	if (unlikely(desc->status))
>+	if (unlikely(status))
> 		goto corrupted;
>
> 	GEM_BUG_ON(head > size);
>
> #ifdef CONFIG_DRM_I915_DEBUG_GUC
>-	if (unlikely(head != READ_ONCE(desc->head))) {
>+	if (unlikely(head != ct_desc_read(&desc, head))) {
> 		CT_ERROR(ct, "Head was modified %u != %u\n",
>-			 desc->head, head);
>-		desc->status |= GUC_CTB_STATUS_MISMATCH;
>+			 ct_desc_read(&desc, head), head);
>+		status |= GUC_CTB_STATUS_MISMATCH;
>+		ct_desc_write(&desc, status, status);
> 		goto corrupted;
> 	}
> #endif
> 	if (unlikely(tail >= size)) {
> 		CT_ERROR(ct, "Invalid tail offset %u >= %u)\n",
> 			 tail, size);
>-		desc->status |= GUC_CTB_STATUS_OVERFLOW;
>+		status = ct_desc_read(&desc, status) |
>+			GUC_CTB_STATUS_OVERFLOW;
>+		ct_desc_write(&desc, status, status);
> 		goto corrupted;
> 	}
>
>@@ -849,7 +886,7 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
> 	CT_DEBUG(ct, "available %d (%u:%u:%u)\n", available, head, tail, size);
> 	GEM_BUG_ON(available < 0);
>
>-	header = cmds[head];
>+	header = iosys_map_rd(&cmds, (4 * head), u32);
> 	head = (head + 1) % size;
>
> 	/* message len with header */
>@@ -857,11 +894,13 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
> 	if (unlikely(len > (u32)available)) {
> 		CT_ERROR(ct, "Incomplete message %*ph %*ph %*ph\n",
> 			 4, &header,
>+			 4 * (head + available - 1 > size ? size - head :
>+			      available - 1), (cmds.vaddr + (4 * head)),
> 			 4 * (head + available - 1 > size ?
>-			      size - head : available - 1), &cmds[head],
>-			 4 * (head + available - 1 > size ?
>-			      available - 1 - size + head : 0), &cmds[0]);
>-		desc->status |= GUC_CTB_STATUS_UNDERFLOW;
>+			      available - 1 - size + head : 0), cmds.vaddr);
>+		status = ct_desc_read(&desc, status) |
>+			GUC_CTB_STATUS_UNDERFLOW;
>+		ct_desc_write(&desc, status, status);
> 		goto corrupted;
> 	}
>
>@@ -869,17 +908,17 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
> 	if (!*msg) {
> 		CT_ERROR(ct, "No memory for message %*ph %*ph %*ph\n",
> 			 4, &header,
>+			 4 * (head + available - 1 > size ? size - head :
>+			      available - 1), (cmds.vaddr + (4 * head)),
> 			 4 * (head + available - 1 > size ?
>-			      size - head : available - 1), &cmds[head],
>-			 4 * (head + available - 1 > size ?
>-			      available - 1 - size + head : 0), &cmds[0]);
>+			      available - 1 - size + head : 0), cmds.vaddr);
> 		return available;
> 	}
>
> 	(*msg)->msg[0] = header;
>
> 	for (i = 1; i < len; i++) {
>-		(*msg)->msg[i] = cmds[head];
>+		(*msg)->msg[i] = iosys_map_rd(&cmds, (4 * head), u32);
> 		head = (head + 1) % size;
> 	}
> 	CT_DEBUG(ct, "received %*ph\n", 4 * len, (*msg)->msg);
>@@ -888,13 +927,14 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
> 	ctb->head = head;
>
> 	/* now update descriptor */
>-	WRITE_ONCE(desc->head, head);
>+	ct_desc_write(&desc, head, head);
>
> 	return available - len;
>
> 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;
> }
>@@ -1211,13 +1251,13 @@ void intel_guc_ct_print_info(struct intel_guc_ct *ct,
> 	drm_printf(p, "H2G Space: %u\n",
> 		   atomic_read(&ct->ctbs.send.space) * 4);
> 	drm_printf(p, "Head: %u\n",
>-		   ct->ctbs.send.desc->head);
>+		   ct_desc_read(&ct->ctbs.send.desc_map, head));
> 	drm_printf(p, "Tail: %u\n",
>-		   ct->ctbs.send.desc->tail);
>+		   ct_desc_read(&ct->ctbs.send.desc_map, tail));
> 	drm_printf(p, "G2H Space: %u\n",
> 		   atomic_read(&ct->ctbs.recv.space) * 4);
> 	drm_printf(p, "Head: %u\n",
>-		   ct->ctbs.recv.desc->head);
>+		   ct_desc_read(&ct->ctbs.recv.desc_map, head));
> 	drm_printf(p, "Tail: %u\n",
>-		   ct->ctbs.recv.desc->tail);
>+		   ct_desc_read(&ct->ctbs.recv.desc_map, tail));
> }
>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 f709a19c7e21..867fe13fb47d 100644
>--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
>+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
>@@ -7,6 +7,7 @@
> #define _INTEL_GUC_CT_H_
>
> #include <linux/interrupt.h>
>+#include <linux/iosys-map.h>
> #include <linux/spinlock.h>
> #include <linux/workqueue.h>
> #include <linux/ktime.h>
>@@ -32,8 +33,8 @@ struct drm_printer;
>  * holds the commands.
>  *
>  * @lock: protects access to the commands buffer and buffer descriptor
>- * @desc: pointer to the buffer descriptor
>- * @cmds: pointer to the commands buffer
>+ * @desc: iosys map to the buffer descriptor
>+ * @cmds: iosys map to the commands buffer
>  * @size: size of the commands buffer in dwords
>  * @resv_space: reserved space in buffer in dwords
>  * @head: local shadow copy of head in dwords
>@@ -43,8 +44,8 @@ struct drm_printer;
>  */
> struct intel_guc_ct_buffer {
> 	spinlock_t lock;
>-	struct guc_ct_buffer_desc *desc;
>-	u32 *cmds;
>+	struct iosys_map desc_map;
>+	struct iosys_map cmds_map;
> 	u32 size;
> 	u32 resv_space;
> 	u32 tail;
>-- 
>2.33.0
>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/i915/guc: Refactor CT access to use iosys_map
  2022-03-08  9:38 [Intel-gfx] [PATCH 0/2] drm/i915/guc: Refactor CT access to use iosys_map Mullati Siva
                   ` (3 preceding siblings ...)
  2022-03-08 22:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
@ 2022-03-09  5:47 ` Patchwork
  4 siblings, 0 replies; 12+ messages in thread
From: Patchwork @ 2022-03-09  5:47 UTC (permalink / raw)
  To: Mullati Siva; +Cc: intel-gfx

[-- Attachment #1: Type: text/plain, Size: 30273 bytes --]

== Series Details ==

Series: drm/i915/guc: Refactor CT access to use iosys_map
URL   : https://patchwork.freedesktop.org/series/101148/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_11337_full -> Patchwork_22509_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (13 -> 13)
------------------------------

  No changes in participating hosts

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_22509_full:

### IGT changes ###

#### Suppressed ####

  The following results come from untrusted machines, tests, or statuses.
  They do not affect the overall result.

  * {igt@kms_hdr@bpc-switch-suspend@bpc-switch-suspend-edp-1-pipe-a}:
    - shard-skl:          [PASS][1] -> [FAIL][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-skl7/igt@kms_hdr@bpc-switch-suspend@bpc-switch-suspend-edp-1-pipe-a.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl1/igt@kms_hdr@bpc-switch-suspend@bpc-switch-suspend-edp-1-pipe-a.html

  
Known issues
------------

  Here are the changes found in Patchwork_22509_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_eio@in-flight-contexts-1us:
    - shard-iclb:         [PASS][3] -> [TIMEOUT][4] ([i915#3070])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-iclb4/igt@gem_eio@in-flight-contexts-1us.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-iclb2/igt@gem_eio@in-flight-contexts-1us.html

  * igt@gem_eio@unwedge-stress:
    - shard-tglb:         [PASS][5] -> [TIMEOUT][6] ([i915#3063] / [i915#3648])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-tglb6/igt@gem_eio@unwedge-stress.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-tglb6/igt@gem_eio@unwedge-stress.html

  * igt@gem_exec_capture@pi@vcs0:
    - shard-iclb:         [PASS][7] -> [INCOMPLETE][8] ([i915#3371])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-iclb3/igt@gem_exec_capture@pi@vcs0.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-iclb3/igt@gem_exec_capture@pi@vcs0.html

  * igt@gem_exec_fair@basic-none-rrul@rcs0:
    - shard-glk:          [PASS][9] -> [FAIL][10] ([i915#2842])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-glk5/igt@gem_exec_fair@basic-none-rrul@rcs0.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-glk8/igt@gem_exec_fair@basic-none-rrul@rcs0.html

  * igt@gem_exec_fair@basic-none@vecs0:
    - shard-kbl:          [PASS][11] -> [FAIL][12] ([i915#2842]) +1 similar issue
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-kbl6/igt@gem_exec_fair@basic-none@vecs0.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-kbl4/igt@gem_exec_fair@basic-none@vecs0.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-tglb:         [PASS][13] -> [FAIL][14] ([i915#2842]) +1 similar issue
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-tglb2/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-tglb6/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_huc_copy@huc-copy:
    - shard-skl:          NOTRUN -> [SKIP][15] ([fdo#109271] / [i915#2190])
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl3/igt@gem_huc_copy@huc-copy.html

  * igt@gem_lmem_swapping@verify-random:
    - shard-skl:          NOTRUN -> [SKIP][16] ([fdo#109271] / [i915#4613])
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl8/igt@gem_lmem_swapping@verify-random.html

  * igt@gem_workarounds@suspend-resume:
    - shard-kbl:          [PASS][17] -> [DMESG-WARN][18] ([i915#180])
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-kbl7/igt@gem_workarounds@suspend-resume.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-kbl6/igt@gem_workarounds@suspend-resume.html

  * igt@gen7_exec_parse@batch-without-end:
    - shard-iclb:         NOTRUN -> [SKIP][19] ([fdo#109289])
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-iclb5/igt@gen7_exec_parse@batch-without-end.html

  * igt@i915_pm_dc@dc3co-vpb-simulation:
    - shard-skl:          NOTRUN -> [SKIP][20] ([fdo#109271] / [i915#658])
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl3/igt@i915_pm_dc@dc3co-vpb-simulation.html

  * igt@i915_suspend@sysfs-reader:
    - shard-apl:          [PASS][21] -> [DMESG-WARN][22] ([i915#180]) +4 similar issues
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-apl6/igt@i915_suspend@sysfs-reader.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-apl3/igt@i915_suspend@sysfs-reader.html

  * igt@kms_big_fb@linear-32bpp-rotate-180:
    - shard-glk:          [PASS][23] -> [DMESG-WARN][24] ([i915#118])
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-glk1/igt@kms_big_fb@linear-32bpp-rotate-180.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-glk1/igt@kms_big_fb@linear-32bpp-rotate-180.html

  * igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip:
    - shard-skl:          NOTRUN -> [FAIL][25] ([i915#3743]) +1 similar issue
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl3/igt@kms_big_fb@x-tiled-max-hw-stride-64bpp-rotate-180-async-flip.html

  * igt@kms_ccs@pipe-b-crc-primary-basic-y_tiled_gen12_mc_ccs:
    - shard-skl:          NOTRUN -> [SKIP][26] ([fdo#109271] / [i915#3886]) +3 similar issues
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl3/igt@kms_ccs@pipe-b-crc-primary-basic-y_tiled_gen12_mc_ccs.html

  * igt@kms_color_chamelium@pipe-a-ctm-red-to-blue:
    - shard-iclb:         NOTRUN -> [SKIP][27] ([fdo#109284] / [fdo#111827]) +1 similar issue
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-iclb5/igt@kms_color_chamelium@pipe-a-ctm-red-to-blue.html

  * igt@kms_color_chamelium@pipe-c-ctm-0-75:
    - shard-apl:          NOTRUN -> [SKIP][28] ([fdo#109271] / [fdo#111827])
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-apl3/igt@kms_color_chamelium@pipe-c-ctm-0-75.html

  * igt@kms_color_chamelium@pipe-d-ctm-negative:
    - shard-iclb:         NOTRUN -> [SKIP][29] ([fdo#109278] / [fdo#109284] / [fdo#111827])
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-iclb5/igt@kms_color_chamelium@pipe-d-ctm-negative.html

  * igt@kms_color_chamelium@pipe-d-degamma:
    - shard-skl:          NOTRUN -> [SKIP][30] ([fdo#109271] / [fdo#111827]) +5 similar issues
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl3/igt@kms_color_chamelium@pipe-d-degamma.html

  * igt@kms_content_protection@dp-mst-type-0:
    - shard-tglb:         NOTRUN -> [SKIP][31] ([i915#3116] / [i915#3299])
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-tglb1/igt@kms_content_protection@dp-mst-type-0.html

  * igt@kms_content_protection@uevent:
    - shard-iclb:         NOTRUN -> [SKIP][32] ([fdo#109300] / [fdo#111066])
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-iclb5/igt@kms_content_protection@uevent.html

  * igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions:
    - shard-skl:          [PASS][33] -> [FAIL][34] ([i915#2346])
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-skl10/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl6/igt@kms_cursor_legacy@flip-vs-cursor-atomic-transitions.html

  * igt@kms_flip@2x-flip-vs-absolute-wf_vblank-interruptible:
    - shard-apl:          NOTRUN -> [SKIP][35] ([fdo#109271]) +19 similar issues
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-apl3/igt@kms_flip@2x-flip-vs-absolute-wf_vblank-interruptible.html

  * igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible@a-edp1:
    - shard-skl:          [PASS][36] -> [FAIL][37] ([i915#2122])
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-skl8/igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible@a-edp1.html
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl5/igt@kms_flip@flip-vs-absolute-wf_vblank-interruptible@a-edp1.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1:
    - shard-skl:          [PASS][38] -> [FAIL][39] ([i915#79])
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-skl2/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl10/igt@kms_flip@flip-vs-expired-vblank-interruptible@a-edp1.html

  * igt@kms_flip@flip-vs-expired-vblank@b-hdmi-a2:
    - shard-glk:          [PASS][40] -> [FAIL][41] ([i915#79])
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-glk2/igt@kms_flip@flip-vs-expired-vblank@b-hdmi-a2.html
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-glk5/igt@kms_flip@flip-vs-expired-vblank@b-hdmi-a2.html

  * igt@kms_flip@flip-vs-suspend@c-dp1:
    - shard-kbl:          [PASS][42] -> [INCOMPLETE][43] ([i915#636])
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-kbl1/igt@kms_flip@flip-vs-suspend@c-dp1.html
   [43]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-kbl3/igt@kms_flip@flip-vs-suspend@c-dp1.html
    - shard-apl:          NOTRUN -> [DMESG-WARN][44] ([i915#180])
   [44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-apl3/igt@kms_flip@flip-vs-suspend@c-dp1.html

  * igt@kms_flip@plain-flip-fb-recreate-interruptible@b-hdmi-a1:
    - shard-glk:          [PASS][45] -> [FAIL][46] ([i915#2122]) +1 similar issue
   [45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-glk6/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-hdmi-a1.html
   [46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-glk7/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-hdmi-a1.html

  * igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-pwrite:
    - shard-iclb:         NOTRUN -> [SKIP][47] ([fdo#109280])
   [47]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-iclb5/igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-cur-indfb-draw-pwrite.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d:
    - shard-apl:          NOTRUN -> [SKIP][48] ([fdo#109271] / [i915#533])
   [48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-apl3/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d.html

  * igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max:
    - shard-skl:          NOTRUN -> [FAIL][49] ([fdo#108145] / [i915#265]) +1 similar issue
   [49]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl8/igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max.html

  * igt@kms_plane_alpha_blend@pipe-a-coverage-7efc:
    - shard-skl:          [PASS][50] -> [FAIL][51] ([fdo#108145] / [i915#265])
   [50]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-skl7/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html
   [51]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl1/igt@kms_plane_alpha_blend@pipe-a-coverage-7efc.html

  * igt@kms_plane_alpha_blend@pipe-d-alpha-7efc:
    - shard-iclb:         NOTRUN -> [SKIP][52] ([fdo#109278])
   [52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-iclb5/igt@kms_plane_alpha_blend@pipe-d-alpha-7efc.html

  * igt@kms_psr2_su@frontbuffer-xrgb8888:
    - shard-iclb:         [PASS][53] -> [SKIP][54] ([fdo#109642] / [fdo#111068] / [i915#658])
   [53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-iclb2/igt@kms_psr2_su@frontbuffer-xrgb8888.html
   [54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-iclb6/igt@kms_psr2_su@frontbuffer-xrgb8888.html

  * igt@kms_psr@psr2_dpms:
    - shard-iclb:         [PASS][55] -> [SKIP][56] ([fdo#109441])
   [55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-iclb2/igt@kms_psr@psr2_dpms.html
   [56]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-iclb5/igt@kms_psr@psr2_dpms.html

  * igt@kms_psr@suspend:
    - shard-skl:          [PASS][57] -> [INCOMPLETE][58] ([i915#4939])
   [57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-skl10/igt@kms_psr@suspend.html
   [58]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl1/igt@kms_psr@suspend.html

  * igt@kms_scaling_modes@scaling-mode-none@edp-1-pipe-a:
    - shard-skl:          NOTRUN -> [SKIP][59] ([fdo#109271]) +88 similar issues
   [59]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl3/igt@kms_scaling_modes@scaling-mode-none@edp-1-pipe-a.html

  * igt@sysfs_clients@sema-10:
    - shard-skl:          NOTRUN -> [SKIP][60] ([fdo#109271] / [i915#2994]) +1 similar issue
   [60]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl3/igt@sysfs_clients@sema-10.html

  * igt@sysfs_clients@split-25:
    - shard-iclb:         NOTRUN -> [SKIP][61] ([i915#2994])
   [61]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-iclb5/igt@sysfs_clients@split-25.html

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@preservation-s3@vecs0:
    - {shard-dg1}:        [FAIL][62] ([fdo#103375]) -> [PASS][63] +1 similar issue
   [62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-dg1-13/igt@gem_ctx_isolation@preservation-s3@vecs0.html
   [63]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-dg1-17/igt@gem_ctx_isolation@preservation-s3@vecs0.html

  * igt@gem_ctx_persistence@smoketest:
    - {shard-rkl}:        [FAIL][64] ([i915#5099]) -> [PASS][65]
   [64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@gem_ctx_persistence@smoketest.html
   [65]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@gem_ctx_persistence@smoketest.html

  * igt@gem_ctx_shared@q-smoketest-all:
    - shard-glk:          [DMESG-WARN][66] ([i915#118]) -> [PASS][67]
   [66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-glk4/igt@gem_ctx_shared@q-smoketest-all.html
   [67]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-glk9/igt@gem_ctx_shared@q-smoketest-all.html

  * igt@gem_eio@in-flight-suspend:
    - {shard-rkl}:        [FAIL][68] ([fdo#103375]) -> [PASS][69]
   [68]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-5/igt@gem_eio@in-flight-suspend.html
   [69]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-1/igt@gem_eio@in-flight-suspend.html

  * igt@gem_exec_endless@dispatch@vcs1:
    - shard-tglb:         [INCOMPLETE][70] ([i915#3778]) -> [PASS][71]
   [70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-tglb7/igt@gem_exec_endless@dispatch@vcs1.html
   [71]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-tglb3/igt@gem_exec_endless@dispatch@vcs1.html
    - {shard-tglu}:       [INCOMPLETE][72] ([i915#3778]) -> [PASS][73]
   [72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-tglu-2/igt@gem_exec_endless@dispatch@vcs1.html
   [73]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-tglu-8/igt@gem_exec_endless@dispatch@vcs1.html

  * igt@gem_exec_fair@basic-pace-share@rcs0:
    - shard-glk:          [FAIL][74] ([i915#2842]) -> [PASS][75]
   [74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-glk8/igt@gem_exec_fair@basic-pace-share@rcs0.html
   [75]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-glk2/igt@gem_exec_fair@basic-pace-share@rcs0.html

  * igt@gem_exec_fair@basic-pace@bcs0:
    - shard-iclb:         [FAIL][76] ([i915#2842]) -> [PASS][77]
   [76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-iclb4/igt@gem_exec_fair@basic-pace@bcs0.html
   [77]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-iclb5/igt@gem_exec_fair@basic-pace@bcs0.html

  * igt@gem_exec_fair@basic-pace@rcs0:
    - shard-kbl:          [FAIL][78] ([i915#2842]) -> [PASS][79]
   [78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-kbl1/igt@gem_exec_fair@basic-pace@rcs0.html
   [79]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-kbl1/igt@gem_exec_fair@basic-pace@rcs0.html

  * igt@gem_exec_fence@syncobj-wait:
    - shard-skl:          [DMESG-WARN][80] ([i915#1982]) -> [PASS][81]
   [80]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-skl5/igt@gem_exec_fence@syncobj-wait.html
   [81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl5/igt@gem_exec_fence@syncobj-wait.html

  * igt@gem_exec_schedule@u-submit-golden-slice@rcs0:
    - shard-skl:          [INCOMPLETE][82] ([i915#3797]) -> [PASS][83]
   [82]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-skl2/igt@gem_exec_schedule@u-submit-golden-slice@rcs0.html
   [83]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl10/igt@gem_exec_schedule@u-submit-golden-slice@rcs0.html

  * igt@gem_exec_schedule@u-submit-golden-slice@vcs1:
    - {shard-dg1}:        [INCOMPLETE][84] -> [PASS][85]
   [84]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-dg1-15/igt@gem_exec_schedule@u-submit-golden-slice@vcs1.html
   [85]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-dg1-13/igt@gem_exec_schedule@u-submit-golden-slice@vcs1.html

  * igt@i915_pm_dc@dc9-dpms:
    - {shard-rkl}:        [SKIP][86] ([i915#4281]) -> [PASS][87]
   [86]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-5/igt@i915_pm_dc@dc9-dpms.html
   [87]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-2/igt@i915_pm_dc@dc9-dpms.html

  * igt@i915_pm_rpm@dpms-mode-unset-lpsp:
    - {shard-dg1}:        [SKIP][88] ([i915#1397]) -> [PASS][89]
   [88]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-dg1-18/igt@i915_pm_rpm@dpms-mode-unset-lpsp.html
   [89]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-dg1-16/igt@i915_pm_rpm@dpms-mode-unset-lpsp.html

  * igt@i915_pm_rpm@gem-execbuf:
    - {shard-rkl}:        [SKIP][90] ([fdo#109308]) -> [PASS][91]
   [90]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@i915_pm_rpm@gem-execbuf.html
   [91]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@i915_pm_rpm@gem-execbuf.html

  * igt@i915_selftest@live@gt_pm:
    - {shard-tglu}:       [DMESG-FAIL][92] ([i915#3987]) -> [PASS][93]
   [92]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-tglu-6/igt@i915_selftest@live@gt_pm.html
   [93]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-tglu-8/igt@i915_selftest@live@gt_pm.html

  * igt@kms_atomic@atomic_plane_damage:
    - {shard-rkl}:        [SKIP][94] ([i915#4098]) -> [PASS][95] +3 similar issues
   [94]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_atomic@atomic_plane_damage.html
   [95]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_atomic@atomic_plane_damage.html

  * igt@kms_big_fb@y-tiled-32bpp-rotate-270:
    - {shard-rkl}:        [SKIP][96] ([i915#1845]) -> [PASS][97] +10 similar issues
   [96]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_big_fb@y-tiled-32bpp-rotate-270.html
   [97]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_big_fb@y-tiled-32bpp-rotate-270.html

  * igt@kms_ccs@pipe-a-bad-rotation-90-y_tiled_gen12_rc_ccs:
    - {shard-rkl}:        [SKIP][98] ([i915#1845] / [i915#4098]) -> [PASS][99] +3 similar issues
   [98]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_ccs@pipe-a-bad-rotation-90-y_tiled_gen12_rc_ccs.html
   [99]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_ccs@pipe-a-bad-rotation-90-y_tiled_gen12_rc_ccs.html

  * igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_rc_ccs:
    - {shard-rkl}:        ([SKIP][100], [SKIP][101]) ([i915#1845] / [i915#4098]) -> [PASS][102]
   [100]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_rc_ccs.html
   [101]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-4/igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_rc_ccs.html
   [102]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_ccs@pipe-a-crc-primary-rotation-180-y_tiled_gen12_rc_ccs.html

  * igt@kms_color@pipe-a-degamma:
    - {shard-rkl}:        [SKIP][103] ([i915#1149] / [i915#1849] / [i915#4070]) -> [PASS][104]
   [103]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_color@pipe-a-degamma.html
   [104]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_color@pipe-a-degamma.html

  * igt@kms_color@pipe-a-gamma:
    - {shard-rkl}:        ([SKIP][105], [SKIP][106]) ([i915#1149] / [i915#1849] / [i915#4070] / [i915#4098]) -> [PASS][107]
   [105]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-4/igt@kms_color@pipe-a-gamma.html
   [106]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_color@pipe-a-gamma.html
   [107]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_color@pipe-a-gamma.html

  * igt@kms_cursor_crc@pipe-a-cursor-256x85-sliding:
    - {shard-rkl}:        [SKIP][108] ([fdo#112022]) -> [PASS][109]
   [108]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-4/igt@kms_cursor_crc@pipe-a-cursor-256x85-sliding.html
   [109]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_cursor_crc@pipe-a-cursor-256x85-sliding.html

  * igt@kms_cursor_crc@pipe-b-cursor-256x256-sliding:
    - {shard-rkl}:        [SKIP][110] ([fdo#112022] / [i915#4070]) -> [PASS][111] +4 similar issues
   [110]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_cursor_crc@pipe-b-cursor-256x256-sliding.html
   [111]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_cursor_crc@pipe-b-cursor-256x256-sliding.html

  * igt@kms_cursor_legacy@flip-vs-cursor-crc-atomic:
    - {shard-rkl}:        [SKIP][112] ([fdo#111825] / [i915#4070]) -> [PASS][113] +2 similar issues
   [112]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_cursor_legacy@flip-vs-cursor-crc-atomic.html
   [113]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_cursor_legacy@flip-vs-cursor-crc-atomic.html

  * igt@kms_cursor_legacy@pipe-c-single-bo:
    - {shard-rkl}:        [SKIP][114] ([i915#4070]) -> [PASS][115]
   [114]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-2/igt@kms_cursor_legacy@pipe-c-single-bo.html
   [115]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-5/igt@kms_cursor_legacy@pipe-c-single-bo.html

  * igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-ytiled:
    - {shard-rkl}:        ([SKIP][116], [SKIP][117]) ([fdo#111314] / [i915#4098]) -> [PASS][118] +1 similar issue
   [116]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-4/igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-ytiled.html
   [117]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-ytiled.html
   [118]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_draw_crc@draw-method-rgb565-mmap-gtt-ytiled.html

  * igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-xtiled:
    - {shard-rkl}:        [SKIP][119] ([fdo#111314]) -> [PASS][120] +4 similar issues
   [119]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-xtiled.html
   [120]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_draw_crc@draw-method-xrgb2101010-mmap-gtt-xtiled.html

  * igt@kms_fbcon_fbt@psr:
    - {shard-rkl}:        [SKIP][121] ([fdo#110189] / [i915#3955]) -> [PASS][122]
   [121]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_fbcon_fbt@psr.html
   [122]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_fbcon_fbt@psr.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1:
    - shard-skl:          [FAIL][123] ([i915#79]) -> [PASS][124]
   [123]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-skl2/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1.html
   [124]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl10/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-edp1.html

  * igt@kms_flip@flip-vs-expired-vblank-interruptible@c-hdmi-a2:
    - shard-glk:          [FAIL][125] ([i915#79]) -> [PASS][126]
   [125]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-glk6/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-hdmi-a2.html
   [126]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-glk7/igt@kms_flip@flip-vs-expired-vblank-interruptible@c-hdmi-a2.html

  * igt@kms_flip@flip-vs-suspend-interruptible@c-dp1:
    - shard-apl:          [DMESG-WARN][127] ([i915#180]) -> [PASS][128] +2 similar issues
   [127]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-apl4/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html
   [128]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-apl1/igt@kms_flip@flip-vs-suspend-interruptible@c-dp1.html

  * igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1:
    - shard-skl:          [FAIL][129] ([i915#2122]) -> [PASS][130] +2 similar issues
   [129]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-skl2/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1.html
   [130]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-skl10/igt@kms_flip@plain-flip-fb-recreate-interruptible@b-edp1.html

  * igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw:
    - {shard-rkl}:        [SKIP][131] ([i915#1849]) -> [PASS][132] +19 similar issues
   [131]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html
   [132]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_frontbuffer_tracking@fbc-1p-pri-indfb-multidraw.html

  * igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-gtt:
    - {shard-rkl}:        ([SKIP][133], [SKIP][134]) ([i915#1849] / [i915#4098]) -> [PASS][135] +1 similar issue
   [133]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-4/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-gtt.html
   [134]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-gtt.html
   [135]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_frontbuffer_tracking@fbcpsr-1p-offscren-pri-shrfb-draw-mmap-gtt.html

  * igt@kms_invalid_mode@zero-vdisplay:
    - {shard-rkl}:        [SKIP][136] ([i915#4278]) -> [PASS][137]
   [136]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_invalid_mode@zero-vdisplay.html
   [137]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_invalid_mode@zero-vdisplay.html

  * igt@kms_plane_alpha_blend@pipe-a-alpha-basic:
    - {shard-rkl}:        ([SKIP][138], [SKIP][139]) ([i915#1849] / [i915#4070] / [i915#4098]) -> [PASS][140]
   [138]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html
   [139]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-4/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html
   [140]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_plane_alpha_blend@pipe-a-alpha-basic.html

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - {shard-rkl}:        [SKIP][141] ([i915#3558] / [i915#4070]) -> [PASS][142]
   [141]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-1/igt@kms_plane_multiple@atomic-pipe-a-tiling-y.html
   [142]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_plane_multiple@atomic-pipe-a-tiling-y.html

  * {igt@kms_plane_scaling@downscale-with-pixel-format-factor-0-5@pipe-c-edp-1-downscale-with-pixel-format}:
    - shard-iclb:         [SKIP][143] ([i915#5176]) -> [PASS][144] +2 similar issues
   [143]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-iclb2/igt@kms_plane_scaling@downscale-with-pixel-format-factor-0-5@pipe-c-edp-1-downscale-with-pixel-format.html
   [144]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-iclb5/igt@kms_plane_scaling@downscale-with-pixel-format-factor-0-5@pipe-c-edp-1-downscale-with-pixel-format.html

  * igt@kms_psr@primary_render:
    - {shard-rkl}:        [SKIP][145] ([i915#1072]) -> [PASS][146] +1 similar issue
   [145]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-rkl-4/igt@kms_psr@primary_render.html
   [146]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-rkl-6/igt@kms_psr@primary_render.html

  * igt@kms_psr@psr2_cursor_render:
    - shard-iclb:         [SKIP][147] ([fdo#109441]) -> [PASS][148] +2 similar issues
   [147]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-iclb5/igt@kms_psr@psr2_cursor_render.html
   [148]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-iclb2/igt@kms_psr@psr2_cursor_render.html

  * igt@kms_universal_plane@cursor-fb-leak-pipe-a:
    - shard-iclb:         [FAIL][149] -> [PASS][150]
   [149]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_11337/shard-iclb3/igt@kms_universal_plane@cursor-fb-leak-pipe-a.html
   [150]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/shard-iclb3/igt@kms_universal_plane@cursor-fb-leak-pipe-a.html

  * igt@perf@polling-parameterized:
    - shard-skl:          [FAIL][151] ([i915#1542]) -> [PASS][152]
   [151]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_22509/index.html

[-- Attachment #2: Type: text/html, Size: 33075 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 1/2] iosys-map: Add a helper for pointer difference
  2022-03-09  1:13   ` Lucas De Marchi
@ 2022-03-14  5:40     ` Siva Mullati
  0 siblings, 0 replies; 12+ messages in thread
From: Siva Mullati @ 2022-03-14  5:40 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx


On 09/03/22 06:43, Lucas De Marchi wrote:
> On Tue, Mar 08, 2022 at 03:08:04PM +0530, Mullati Siva wrote:
>> From: Siva Mullati <siva.mullati@intel.com>
>>
>> iosys_map_ptrdiff to get the difference in address of
>> same memory type.
>>
>> Signed-off-by: Siva Mullati <siva.mullati@intel.com>
>> ---
>> include/linux/iosys-map.h | 21 +++++++++++++++++++++
>> 1 file changed, 21 insertions(+)
>>
>> diff --git a/include/linux/iosys-map.h b/include/linux/iosys-map.h
>> index e69a002d5aa4..8987f69ec1e9 100644
>> --- a/include/linux/iosys-map.h
>> +++ b/include/linux/iosys-map.h
>> @@ -8,6 +8,7 @@
>>
>> #include <linux/io.h>
>> #include <linux/string.h>
>> +#include <linux/types.h>
>>
>> /**
>>  * DOC: overview
>> @@ -208,6 +209,26 @@ static inline bool iosys_map_is_equal(const struct iosys_map *lhs,
>>         return lhs->vaddr == rhs->vaddr;
>> }
>>
>> +/**
>> + * iosys_map_ptrdiff - Difference of two iosys mapping addresses of same memory type
>> + * @lhs:       The iosys_map structure
>> + * @rhs:       A iosys_map structure to compare with
>> + *
>> + * Two iosys mapping structures of same memory type with the differences
>> + * in address within that memory.
>> + *
>> + * Returns:
>> + * Address difference of two memory locations with same memory type.
>> + */
>> +static inline ptrdiff_t iosys_map_ptrdiff(const struct iosys_map *lhs,
>> +                      const struct iosys_map *rhs)
>> +{
>> +    if (lhs->is_iomem)
>
> the other interfaces always check both arguments to make sure they are
> both iomem. So if we want this interface we will want to check like
> that.
>
Agreed, will use the other interface.
> I'm not sure this is really needed, but will depend on the secon patch,
> let's wait to settle this discussion there.
>
Since you have already covered the other patch, intel_guc_ct_enable() where this API has been used for calculating offset for cmds and desc. Do you this as good approach or is okay to directly access the ctb.send.cmds_map.vaddr?

>
> thanks
> Lucas De Marchi

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 2/2] drm/i915/guc: Convert ct buffer to iosys_map
  2022-03-09  1:38   ` Lucas De Marchi
@ 2022-03-14  5:53     ` Siva Mullati
  2022-03-17  5:06       ` Lucas De Marchi
  2022-03-17  5:12     ` Lucas De Marchi
  1 sibling, 1 reply; 12+ messages in thread
From: Siva Mullati @ 2022-03-14  5:53 UTC (permalink / raw)
  To: Lucas De Marchi; +Cc: intel-gfx


On 09/03/22 07:08, Lucas De Marchi wrote:
> On Tue, Mar 08, 2022 at 03:08:05PM +0530, Mullati Siva wrote:
>> From: Siva Mullati <siva.mullati@intel.com>
>>
>> Convert CT commands and descriptors to use iosys_map rather
>> than plain pointer and save it in the intel_guc_ct_buffer struct.
>> This will help with ct_write and ct_read for cmd send and receive
>> after the initialization by abstracting the IO vs system memory.
>>
>> Signed-off-by: Siva Mullati <siva.mullati@intel.com>
>> ---
>> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 170 +++++++++++++---------
>> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h |   9 +-
>> 2 files changed, 110 insertions(+), 69 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 f01325cd1b62..457deca1c25a 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>> @@ -44,6 +44,11 @@ static inline struct drm_device *ct_to_drm(struct intel_guc_ct *ct)
>> #define CT_PROBE_ERROR(_ct, _fmt, ...) \
>>     i915_probe_error(ct_to_i915(ct), "CT: " _fmt, ##__VA_ARGS__)
>>
>> +#define ct_desc_read(desc_map_, field_) \
>> +    iosys_map_rd_field(desc_map_, 0, struct guc_ct_buffer_desc, field_)
>
> one thing I found useful in intel_guc_ads, was to use the first argument
> as the struct type instead of map. That's because then I enforce the
> right type to be passed rather than a random iosys_map. See :
>
>     #define ads_blob_read(guc_, field_)                                     \
>         iosys_map_rd_field(&(guc_)->ads_map, 0, struct __guc_ads_blob, field_)
>
> First arg is expected to be `struct intel_guc *`. Yes, I didn't do this
> for info_map_{read,write}, because there were situation in which I had
> to pass a info from outside (forcefully from system memory). If you
> don't have such case,  I think it would be better to pass the typed
> pointer.
>
understood, will change it as a "typed pointer".
>> +#define ct_desc_write(desc_map_, field_, val_) \
>> +    iosys_map_wr_field(desc_map_, 0, struct guc_ct_buffer_desc, field_, val_)
>> +
>> /**
>>  * DOC: CTB Blob
>>  *
>> @@ -113,9 +118,9 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
>>     init_waitqueue_head(&ct->wq);
>> }
>>
>> -static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc)
>> +static void guc_ct_buffer_desc_init(struct iosys_map *desc)
>
> if you can apply the comment above, then leave it as
> struct intel_guc_ct_buffer.
>
yes
>> {
>> -    memset(desc, 0, sizeof(*desc));
>> +    iosys_map_memset(desc, 0, 0, sizeof(struct guc_ct_buffer_desc));
>> }
>>
>> static void guc_ct_buffer_reset(struct intel_guc_ct_buffer *ctb)
>> @@ -128,17 +133,24 @@ static void guc_ct_buffer_reset(struct intel_guc_ct_buffer *ctb)
>>     space = CIRC_SPACE(ctb->tail, ctb->head, ctb->size) - ctb->resv_space;
>>     atomic_set(&ctb->space, space);
>>
>> -    guc_ct_buffer_desc_init(ctb->desc);
>> +    guc_ct_buffer_desc_init(&ctb->desc_map);
>> }
>>
>> static void guc_ct_buffer_init(struct intel_guc_ct_buffer *ctb,
>> -                   struct guc_ct_buffer_desc *desc,
>> -                   u32 *cmds, u32 size_in_bytes, u32 resv_space)
>> +                   void *desc, void *cmds, u32 size_in_bytes,
>> +                   u32 resv_space, bool lmem)
>
> bool arguments are really hard to read, because you always have to
> lookup the function prototype to understand what that is about.
>
Tried to avoid bool but could not find a better alternative code path.
Please suggest, if you have something.

> Here we could turn struct guc_ct_buffer_desc *desc into the map, and let
> the caller prepare it for iomem or system memory.
>
Agreed
>> {
>>     GEM_BUG_ON(size_in_bytes % 4);
>>
>> -    ctb->desc = desc;
>> -    ctb->cmds = cmds;
>> +    if (lmem) {
>> +        iosys_map_set_vaddr_iomem(&ctb->desc_map,
>> +                      (void __iomem *)desc);
>> +        iosys_map_set_vaddr_iomem(&ctb->cmds_map,
>> +                      (void __iomem *)cmds);
>> +    } else {
>> +        iosys_map_set_vaddr(&ctb->desc_map, desc);
>> +        iosys_map_set_vaddr(&ctb->cmds_map, cmds);
>> +    }
>>     ctb->size = size_in_bytes / 4;
>>     ctb->resv_space = resv_space / 4;
>>
>> @@ -218,13 +230,12 @@ static int ct_register_buffer(struct intel_guc_ct *ct, bool send,
>> int intel_guc_ct_init(struct intel_guc_ct *ct)
>> {
>>     struct intel_guc *guc = ct_to_guc(ct);
>> -    struct guc_ct_buffer_desc *desc;
>>     u32 blob_size;
>>     u32 cmds_size;
>>     u32 resv_space;
>> -    void *blob;
>> -    u32 *cmds;
>> +    void *blob, *desc, *cmds;
>>     int err;
>> +    bool lmem;
>>
>>     err = i915_inject_probe_error(guc_to_gt(guc)->i915, -ENXIO);
>>     if (err)
>> @@ -242,6 +253,8 @@ int intel_guc_ct_init(struct intel_guc_ct *ct)
>>
>>     CT_DEBUG(ct, "base=%#x size=%u\n", intel_guc_ggtt_offset(guc, ct->vma), blob_size);
>>
>> +    lmem = i915_gem_object_is_lmem(ct->vma->obj);
>> +
>>     /* store pointers to desc and cmds for send ctb */
>>     desc = blob;
>>     cmds = blob + 2 * CTB_DESC_SIZE;
>> @@ -251,7 +264,8 @@ int intel_guc_ct_init(struct intel_guc_ct *ct)
>>          ptrdiff(desc, blob), ptrdiff(cmds, blob), cmds_size,
>>          resv_space);
>>
>> -    guc_ct_buffer_init(&ct->ctbs.send, desc, cmds, cmds_size, resv_space);
>> +    guc_ct_buffer_init(&ct->ctbs.send,
>> +               desc, cmds, cmds_size, resv_space, lmem);
>>
>>     /* store pointers to desc and cmds for recv ctb */
>>     desc = blob + CTB_DESC_SIZE;
>> @@ -262,7 +276,8 @@ int intel_guc_ct_init(struct intel_guc_ct *ct)
>>          ptrdiff(desc, blob), ptrdiff(cmds, blob), cmds_size,
>>          resv_space);
>>
>> -    guc_ct_buffer_init(&ct->ctbs.recv, desc, cmds, cmds_size, resv_space);
>> +    guc_ct_buffer_init(&ct->ctbs.recv,
>> +               desc, cmds, cmds_size, resv_space, lmem);
>>
>>     return 0;
>> }
>> @@ -279,6 +294,10 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct)
>>
>>     tasklet_kill(&ct->receive_tasklet);
>>     i915_vma_unpin_and_release(&ct->vma, I915_VMA_RELEASE_MAP);
>> +    iosys_map_clear(&ct->ctbs.send.desc_map);
>> +    iosys_map_clear(&ct->ctbs.send.cmds_map);
>> +    iosys_map_clear(&ct->ctbs.recv.desc_map);
>> +    iosys_map_clear(&ct->ctbs.recv.cmds_map);
>>     memset(ct, 0, sizeof(*ct));
>> }
>>
>> @@ -291,6 +310,7 @@ void intel_guc_ct_fini(struct intel_guc_ct *ct)
>> int intel_guc_ct_enable(struct intel_guc_ct *ct)
>> {
>>     struct intel_guc *guc = ct_to_guc(ct);
>> +    struct iosys_map blob_map;
>>     u32 base, desc, cmds, size;
>>     void *blob;
>>     int err;
>> @@ -302,9 +322,14 @@ int intel_guc_ct_enable(struct intel_guc_ct *ct)
>>     GEM_BUG_ON(!i915_gem_object_has_pinned_pages(ct->vma->obj));
>>     base = intel_guc_ggtt_offset(guc, ct->vma);
>>
>> -    /* blob should start with send descriptor */
>>     blob = __px_vaddr(ct->vma->obj);
>> -    GEM_BUG_ON(blob != ct->ctbs.send.desc);
>> +    if (i915_gem_object_is_lmem(ct->vma->obj))
>> +        iosys_map_set_vaddr_iomem(&blob_map, (void __iomem *)blob);
>> +    else
>> +        iosys_map_set_vaddr(&blob_map, blob);
>
> probably better to remove blob and pass __px_vaddr(ct->vma->obj)
> directly as argument. This avoids later having users making (wrong)
> use of the blob variable, that shouldn't be used anymore after this
> point in the function.
>
> other possibility would be to do a blob = NULL here, but I think the
> other approach is cleaner.
>
Will proceed with first approach
>> +
>> +    /* blob should start with send descriptor */
>> +    GEM_BUG_ON(!iosys_map_is_equal(&blob_map, &ct->ctbs.send.desc_map));
>>
>>     /* (re)initialize descriptors */
>>     guc_ct_buffer_reset(&ct->ctbs.send);
>> @@ -314,15 +339,15 @@ int intel_guc_ct_enable(struct intel_guc_ct *ct)
>>      * Register both CT buffers starting with RECV buffer.
>>      * Descriptors are in first half of the blob.
>>      */
>> -    desc = base + ptrdiff(ct->ctbs.recv.desc, blob);
>> -    cmds = base + ptrdiff(ct->ctbs.recv.cmds, blob);
>> +    desc = base + iosys_map_ptrdiff(&ct->ctbs.recv.desc_map, &blob_map);
>> +    cmds = base + iosys_map_ptrdiff(&ct->ctbs.recv.cmds_map, &blob_map);
>>     size = ct->ctbs.recv.size * 4;
>>     err = ct_register_buffer(ct, false, desc, cmds, size);
>>     if (unlikely(err))
>>         goto err_out;
>>
>> -    desc = base + ptrdiff(ct->ctbs.send.desc, blob);
>> -    cmds = base + ptrdiff(ct->ctbs.send.cmds, blob);
>> +    desc = base + iosys_map_ptrdiff(&ct->ctbs.send.desc_map, &blob_map);
>> +    cmds = base + iosys_map_ptrdiff(&ct->ctbs.send.cmds_map, &blob_map);
>>     size = ct->ctbs.send.size * 4;
>>     err = ct_register_buffer(ct, true, desc, cmds, size);
>>     if (unlikely(err))
>> @@ -371,31 +396,35 @@ static int ct_write(struct intel_guc_ct *ct,
>>             u32 fence, u32 flags)
>> {
>>     struct intel_guc_ct_buffer *ctb = &ct->ctbs.send;
>> -    struct guc_ct_buffer_desc *desc = ctb->desc;
>> +    struct iosys_map desc = ctb->desc_map;
>> +    struct iosys_map cmds = ctb->cmds_map;
>>     u32 tail = ctb->tail;
>>     u32 size = ctb->size;
>>     u32 header;
>>     u32 hxg;
>>     u32 type;
>> -    u32 *cmds = ctb->cmds;
>> +    u32 status = ct_desc_read(&desc, status);
>>     unsigned int i;
>>
>> -    if (unlikely(desc->status))
>> +    if (unlikely(status))
>>         goto corrupted;
>>
>>     GEM_BUG_ON(tail > size);
>>
>> #ifdef CONFIG_DRM_I915_DEBUG_GUC
>> -    if (unlikely(tail != READ_ONCE(desc->tail))) {
>> +    if (unlikely(tail != ct_desc_read(&desc, tail))) {
>>         CT_ERROR(ct, "Tail was modified %u != %u\n",
>> -             desc->tail, tail);
>> -        desc->status |= GUC_CTB_STATUS_MISMATCH;
>> +             ct_desc_read(&desc, tail), tail);
>> +        status |= GUC_CTB_STATUS_MISMATCH;
>> +        ct_desc_write(&desc, status, status);
>>         goto corrupted;
>>     }
>> -    if (unlikely(READ_ONCE(desc->head) >= size)) {
>> +    if (unlikely(ct_desc_read(&desc, head) >= size)) {
>>         CT_ERROR(ct, "Invalid head offset %u >= %u)\n",
>> -             desc->head, size);
>> -        desc->status |= GUC_CTB_STATUS_OVERFLOW;
>> +             ct_desc_read(&desc, head), size);
>> +        status = ct_desc_read(&desc, status) |
>> +            GUC_CTB_STATUS_OVERFLOW;
>> +        ct_desc_write(&desc, status, status);
>>         goto corrupted;
>>     }
>> #endif
>> @@ -418,14 +447,14 @@ static int ct_write(struct intel_guc_ct *ct,
>>     CT_DEBUG(ct, "writing (tail %u) %*ph %*ph %*ph\n",
>>          tail, 4, &header, 4, &hxg, 4 * (len - 1), &action[1]);
>>
>> -    cmds[tail] = header;
>> +    iosys_map_wr(&cmds, (4 * tail), u32, header);
>
> hiding sizeof(u32) here. Just make it &cmds[tail]?
>
yes, instead of hardcode will use sizeof(u32). Here cmds is of type
struct iosys_map.
>>     tail = (tail + 1) % size;
>>
>> -    cmds[tail] = hxg;
>> +    iosys_map_wr(&cmds, (4 * tail), u32, hxg);
>
> ditto
>
>>     tail = (tail + 1) % size;
>>
>>     for (i = 1; i < len; i++) {
>> -        cmds[tail] = action[i];
>> +        iosys_map_wr(&cmds, (4 * tail), u32, action[i]);
>
> ditto
>
> and in some other places too. I will try to finish the review later.
>
Sure, will wait for you complete review before I start with rev2.

Thank you for the feedback.

> Lucas De Marchi
>
>>         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;
>>
>>         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;
>>     u32 head;
>>     u32 space;
>> +    u32 status = ct_desc_read(&desc, status);
>>
>>     if (atomic_read(&ctb->space) >= len_dw)
>>         return true;
>>
>> -    head = READ_ONCE(desc->head);
>> +    head = ct_desc_read(&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;
>> +        status |= GUC_CTB_STATUS_OVERFLOW;
>> +        ct_desc_write(&desc, status, status);
>>         ctb->broken = true;
>>         return false;
>>     }
>> @@ -803,11 +836,12 @@ static void ct_free_msg(struct ct_incoming_msg *msg)
>> 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;
>> +    struct iosys_map desc = ctb->desc_map;
>> +    struct iosys_map cmds = ctb->cmds_map;
>>     u32 head = ctb->head;
>> -    u32 tail = READ_ONCE(desc->tail);
>> +    u32 tail = ct_desc_read(&desc, tail);
>>     u32 size = ctb->size;
>> -    u32 *cmds = ctb->cmds;
>> +    u32 status = ct_desc_read(&desc, status);
>>     s32 available;
>>     unsigned int len;
>>     unsigned int i;
>> @@ -816,23 +850,26 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
>>     if (unlikely(ctb->broken))
>>         return -EPIPE;
>>
>> -    if (unlikely(desc->status))
>> +    if (unlikely(status))
>>         goto corrupted;
>>
>>     GEM_BUG_ON(head > size);
>>
>> #ifdef CONFIG_DRM_I915_DEBUG_GUC
>> -    if (unlikely(head != READ_ONCE(desc->head))) {
>> +    if (unlikely(head != ct_desc_read(&desc, head))) {
>>         CT_ERROR(ct, "Head was modified %u != %u\n",
>> -             desc->head, head);
>> -        desc->status |= GUC_CTB_STATUS_MISMATCH;
>> +             ct_desc_read(&desc, head), head);
>> +        status |= GUC_CTB_STATUS_MISMATCH;
>> +        ct_desc_write(&desc, status, status);
>>         goto corrupted;
>>     }
>> #endif
>>     if (unlikely(tail >= size)) {
>>         CT_ERROR(ct, "Invalid tail offset %u >= %u)\n",
>>              tail, size);
>> -        desc->status |= GUC_CTB_STATUS_OVERFLOW;
>> +        status = ct_desc_read(&desc, status) |
>> +            GUC_CTB_STATUS_OVERFLOW;
>> +        ct_desc_write(&desc, status, status);
>>         goto corrupted;
>>     }
>>
>> @@ -849,7 +886,7 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
>>     CT_DEBUG(ct, "available %d (%u:%u:%u)\n", available, head, tail, size);
>>     GEM_BUG_ON(available < 0);
>>
>> -    header = cmds[head];
>> +    header = iosys_map_rd(&cmds, (4 * head), u32);
>>     head = (head + 1) % size;
>>
>>     /* message len with header */
>> @@ -857,11 +894,13 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
>>     if (unlikely(len > (u32)available)) {
>>         CT_ERROR(ct, "Incomplete message %*ph %*ph %*ph\n",
>>              4, &header,
>> +             4 * (head + available - 1 > size ? size - head :
>> +                  available - 1), (cmds.vaddr + (4 * head)),
>>              4 * (head + available - 1 > size ?
>> -                  size - head : available - 1), &cmds[head],
>> -             4 * (head + available - 1 > size ?
>> -                  available - 1 - size + head : 0), &cmds[0]);
>> -        desc->status |= GUC_CTB_STATUS_UNDERFLOW;
>> +                  available - 1 - size + head : 0), cmds.vaddr);
>> +        status = ct_desc_read(&desc, status) |
>> +            GUC_CTB_STATUS_UNDERFLOW;
>> +        ct_desc_write(&desc, status, status);
>>         goto corrupted;
>>     }
>>
>> @@ -869,17 +908,17 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
>>     if (!*msg) {
>>         CT_ERROR(ct, "No memory for message %*ph %*ph %*ph\n",
>>              4, &header,
>> +             4 * (head + available - 1 > size ? size - head :
>> +                  available - 1), (cmds.vaddr + (4 * head)),
>>              4 * (head + available - 1 > size ?
>> -                  size - head : available - 1), &cmds[head],
>> -             4 * (head + available - 1 > size ?
>> -                  available - 1 - size + head : 0), &cmds[0]);
>> +                  available - 1 - size + head : 0), cmds.vaddr);
>>         return available;
>>     }
>>
>>     (*msg)->msg[0] = header;
>>
>>     for (i = 1; i < len; i++) {
>> -        (*msg)->msg[i] = cmds[head];
>> +        (*msg)->msg[i] = iosys_map_rd(&cmds, (4 * head), u32);
>>         head = (head + 1) % size;
>>     }
>>     CT_DEBUG(ct, "received %*ph\n", 4 * len, (*msg)->msg);
>> @@ -888,13 +927,14 @@ static int ct_read(struct intel_guc_ct *ct, struct ct_incoming_msg **msg)
>>     ctb->head = head;
>>
>>     /* now update descriptor */
>> -    WRITE_ONCE(desc->head, head);
>> +    ct_desc_write(&desc, head, head);
>>
>>     return available - len;
>>
>> 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;
>> }
>> @@ -1211,13 +1251,13 @@ void intel_guc_ct_print_info(struct intel_guc_ct *ct,
>>     drm_printf(p, "H2G Space: %u\n",
>>            atomic_read(&ct->ctbs.send.space) * 4);
>>     drm_printf(p, "Head: %u\n",
>> -           ct->ctbs.send.desc->head);
>> +           ct_desc_read(&ct->ctbs.send.desc_map, head));
>>     drm_printf(p, "Tail: %u\n",
>> -           ct->ctbs.send.desc->tail);
>> +           ct_desc_read(&ct->ctbs.send.desc_map, tail));
>>     drm_printf(p, "G2H Space: %u\n",
>>            atomic_read(&ct->ctbs.recv.space) * 4);
>>     drm_printf(p, "Head: %u\n",
>> -           ct->ctbs.recv.desc->head);
>> +           ct_desc_read(&ct->ctbs.recv.desc_map, head));
>>     drm_printf(p, "Tail: %u\n",
>> -           ct->ctbs.recv.desc->tail);
>> +           ct_desc_read(&ct->ctbs.recv.desc_map, tail));
>> }
>> 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 f709a19c7e21..867fe13fb47d 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h
>> @@ -7,6 +7,7 @@
>> #define _INTEL_GUC_CT_H_
>>
>> #include <linux/interrupt.h>
>> +#include <linux/iosys-map.h>
>> #include <linux/spinlock.h>
>> #include <linux/workqueue.h>
>> #include <linux/ktime.h>
>> @@ -32,8 +33,8 @@ struct drm_printer;
>>  * holds the commands.
>>  *
>>  * @lock: protects access to the commands buffer and buffer descriptor
>> - * @desc: pointer to the buffer descriptor
>> - * @cmds: pointer to the commands buffer
>> + * @desc: iosys map to the buffer descriptor
>> + * @cmds: iosys map to the commands buffer
>>  * @size: size of the commands buffer in dwords
>>  * @resv_space: reserved space in buffer in dwords
>>  * @head: local shadow copy of head in dwords
>> @@ -43,8 +44,8 @@ struct drm_printer;
>>  */
>> struct intel_guc_ct_buffer {
>>     spinlock_t lock;
>> -    struct guc_ct_buffer_desc *desc;
>> -    u32 *cmds;
>> +    struct iosys_map desc_map;
>> +    struct iosys_map cmds_map;
>>     u32 size;
>>     u32 resv_space;
>>     u32 tail;
>> -- 
>> 2.33.0
>>

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 2/2] drm/i915/guc: Convert ct buffer to iosys_map
  2022-03-14  5:53     ` Siva Mullati
@ 2022-03-17  5:06       ` Lucas De Marchi
  0 siblings, 0 replies; 12+ messages in thread
From: Lucas De Marchi @ 2022-03-17  5:06 UTC (permalink / raw)
  To: Siva Mullati; +Cc: intel-gfx

On Mon, Mar 14, 2022 at 11:23:07AM +0530, Siva Mullati wrote:
>
>On 09/03/22 07:08, Lucas De Marchi wrote:
>> On Tue, Mar 08, 2022 at 03:08:05PM +0530, Mullati Siva wrote:
>>> From: Siva Mullati <siva.mullati@intel.com>
>>>
>>> Convert CT commands and descriptors to use iosys_map rather
>>> than plain pointer and save it in the intel_guc_ct_buffer struct.
>>> This will help with ct_write and ct_read for cmd send and receive
>>> after the initialization by abstracting the IO vs system memory.
>>>
>>> Signed-off-by: Siva Mullati <siva.mullati@intel.com>
>>> ---
>>> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 170 +++++++++++++---------
>>> drivers/gpu/drm/i915/gt/uc/intel_guc_ct.h |   9 +-
>>> 2 files changed, 110 insertions(+), 69 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 f01325cd1b62..457deca1c25a 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c
>>> @@ -44,6 +44,11 @@ static inline struct drm_device *ct_to_drm(struct intel_guc_ct *ct)
>>> #define CT_PROBE_ERROR(_ct, _fmt, ...) \
>>>     i915_probe_error(ct_to_i915(ct), "CT: " _fmt, ##__VA_ARGS__)
>>>
>>> +#define ct_desc_read(desc_map_, field_) \
>>> +    iosys_map_rd_field(desc_map_, 0, struct guc_ct_buffer_desc, field_)
>>
>> one thing I found useful in intel_guc_ads, was to use the first argument
>> as the struct type instead of map. That's because then I enforce the
>> right type to be passed rather than a random iosys_map. See :
>>
>>     #define ads_blob_read(guc_, field_)                                     \
>>         iosys_map_rd_field(&(guc_)->ads_map, 0, struct __guc_ads_blob, field_)
>>
>> First arg is expected to be `struct intel_guc *`. Yes, I didn't do this
>> for info_map_{read,write}, because there were situation in which I had
>> to pass a info from outside (forcefully from system memory). If you
>> don't have such case,  I think it would be better to pass the typed
>> pointer.
>>
>understood, will change it as a "typed pointer".
>>> +#define ct_desc_write(desc_map_, field_, val_) \
>>> +    iosys_map_wr_field(desc_map_, 0, struct guc_ct_buffer_desc, field_, val_)
>>> +
>>> /**
>>>  * DOC: CTB Blob
>>>  *
>>> @@ -113,9 +118,9 @@ void intel_guc_ct_init_early(struct intel_guc_ct *ct)
>>>     init_waitqueue_head(&ct->wq);
>>> }
>>>
>>> -static void guc_ct_buffer_desc_init(struct guc_ct_buffer_desc *desc)
>>> +static void guc_ct_buffer_desc_init(struct iosys_map *desc)
>>
>> if you can apply the comment above, then leave it as
>> struct intel_guc_ct_buffer.
>>
>yes
>>> {
>>> -    memset(desc, 0, sizeof(*desc));
>>> +    iosys_map_memset(desc, 0, 0, sizeof(struct guc_ct_buffer_desc));
>>> }
>>>
>>> static void guc_ct_buffer_reset(struct intel_guc_ct_buffer *ctb)
>>> @@ -128,17 +133,24 @@ static void guc_ct_buffer_reset(struct intel_guc_ct_buffer *ctb)
>>>     space = CIRC_SPACE(ctb->tail, ctb->head, ctb->size) - ctb->resv_space;
>>>     atomic_set(&ctb->space, space);
>>>
>>> -    guc_ct_buffer_desc_init(ctb->desc);
>>> +    guc_ct_buffer_desc_init(&ctb->desc_map);
>>> }
>>>
>>> static void guc_ct_buffer_init(struct intel_guc_ct_buffer *ctb,
>>> -                   struct guc_ct_buffer_desc *desc,
>>> -                   u32 *cmds, u32 size_in_bytes, u32 resv_space)
>>> +                   void *desc, void *cmds, u32 size_in_bytes,
>>> +                   u32 resv_space, bool lmem)
>>
>> bool arguments are really hard to read, because you always have to
>> lookup the function prototype to understand what that is about.
>>
>Tried to avoid bool but could not find a better alternative code path.
>Please suggest, if you have something.

humn... suggestion as as below:

>
>> Here we could turn struct guc_ct_buffer_desc *desc into the map, and let
>> the caller prepare it for iomem or system memory.

In other words, maybe instead of receiving guc_ct_buffer_desc as
parameter, receiving a struct iosys_map *desc_map. That map can be
created by the caller and then you do:

	ctb->desc_map = *desc_map;


Lucas De Marchi

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [Intel-gfx] [PATCH 2/2] drm/i915/guc: Convert ct buffer to iosys_map
  2022-03-09  1:38   ` Lucas De Marchi
  2022-03-14  5:53     ` Siva Mullati
@ 2022-03-17  5:12     ` Lucas De Marchi
  1 sibling, 0 replies; 12+ messages in thread
From: Lucas De Marchi @ 2022-03-17  5:12 UTC (permalink / raw)
  To: Mullati Siva; +Cc: intel-gfx

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

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2022-03-17  5:12 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-08  9:38 [Intel-gfx] [PATCH 0/2] drm/i915/guc: Refactor CT access to use iosys_map Mullati Siva
2022-03-08  9:38 ` [Intel-gfx] [PATCH 1/2] iosys-map: Add a helper for pointer difference Mullati Siva
2022-03-09  1:13   ` Lucas De Marchi
2022-03-14  5:40     ` Siva Mullati
2022-03-08  9:38 ` [Intel-gfx] [PATCH 2/2] drm/i915/guc: Convert ct buffer to iosys_map Mullati Siva
2022-03-09  1:38   ` Lucas De Marchi
2022-03-14  5:53     ` Siva Mullati
2022-03-17  5:06       ` Lucas De Marchi
2022-03-17  5:12     ` Lucas De Marchi
2022-03-08 22:08 ` [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/i915/guc: Refactor CT access to use iosys_map Patchwork
2022-03-08 22:40 ` [Intel-gfx] ✓ Fi.CI.BAT: success " Patchwork
2022-03-09  5:47 ` [Intel-gfx] ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.