thr3ads.net - Nouveau - [PATCH v3 03/15] nvkm: rename "argv" to what it represnts on the GSP message send path [Oct 2024]

If this information is useful, please help other people find it:
Share via:

Zhi Wang

2024-Oct-31 08:52 UTC

[PATCH v3 00/15] NVKM GSP RPC kernel docs, cleanups and fixes

Hi folks:

Here is the leftover of the previous spin of NVKM GSP RPC fixes, which
is handling the return of large GSP message. PATCH 1 and 2 in the previous
spin were merged [1], and this spin is based on top of PATCH 1 and PATCH 2
in the previous spin.

Besides the support of the large GSP message, kernel doc and many cleanups
are introduced according to the comments in the previous spin [2].

For testing, I tried the following without any problem:

- run Uniengine Heaven [3] on my RTX 4060 for 8 hours
- the GL CTS runner [4] (commandline: ./cts-runner --type-gl40) from
Khronos
- with the vGPU RFC patches [5], boot up a ubuntu VM with a vGPU on a L40
and run glmark2.

v3:

- Add a kernel doc and chart to introduce the GSP message and denote the
memory layout.
- Add a decoder section in the kernel doc to explain the terms in the code.
- Refine the many confusing variable names to align with the terms in the
kernel doc.
- Introduce the continaution records in the kernel doc.
- Re-factor the complicated function r535_gsp_msgq_wait().
- Other minor cleanups.

v2:

- Remove the Fixes: tags as the vanilla nouveau aren't going to hit these
bugs. (Danilo)
- Test the patchset on VK-GL-CTS. (Danilo)
- Remove the ambiguous empty line in PATCH 2. (Danilo)
- Rename the r535_gsp_msgq_wait to gsp_msgq_recv. (Danilo)
- Introduce a data structure to hold the params of gsp_msgq_recv(). (Danilo)
- Document the params and the states they are related to. (Danilo)

[1] https://lore.kernel.org/all/20241017071922.2518724-1-zhiw at nvidia.com/
[2] https://lore.kernel.org/all/ZxD1BBUUeLQdvXVX at pollux/
[3] https://benchmark.unigine.com/heaven
[4] https://github.com/KhronosGroup/VK-GL-CTS
[5] https://lore.kernel.org/kvm/20240922124951.1946072-1-zhiw at nvidia.com/T/#t

Zhi Wang (15):
  nvkm: add a kernel doc to introduce the GSP RPC
  nvkm: rename "repc" to "gsp_rpc_len" on the GSP message
recv path
  nvkm: rename "argv" to what it represnts on the GSP message send
path
  nvkm: remove unused param repc in *rm_alloc_push()
  nvkm: rename "argv" to what it represents in *rm_{alloc, ctrl}_*()
  nvkm: rename "argc" to what it represents in GSP RPC routines
  nvkm: fix the broken marco GSP_MSG_MAX_SIZE
  nvkm: remove the magic number in r535_gsp_rpc_push()
  nvkm: refine the variable names in r535_gsp_rpc_push()
  nvkm: refine the variable names in r535_gsp_msg_recv()
  nvkm: rename the variable "cmd" to "msg" in
r535_gsp_cmdq_{get,
    push}()
  nvkm: factor out r535_gsp_msgq_peek()
  nvkm: factor out r535_gsp_msgq_recv_one_elem()
  nvkm: support handling the return of large GSP message
  nvkm: consume the return of large GSP message

 .../gpu/drm/nouveau/include/nvkm/subdev/gsp.h |   8 +-
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 501 ++++++++++++------
 2 files changed, 354 insertions(+), 155 deletions(-)

-- 
2.34.1

Zhi Wang

2024-Oct-31 08:52 UTC

head link

[PATCH v3 01/15] nvkm: add a kernel doc to introduce the GSP RPC

In order to explain the name clean-ups in GSP RPC routines, a kernel
doc to explain the memory layout and terms is required.

Add a kernel doc to introduce the GSP RPC.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 46 +++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index a4b8381c4e3e..1a07c0191356 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -60,6 +60,52 @@
 #define GSP_MSG_MIN_SIZE GSP_PAGE_SIZE
 #define GSP_MSG_MAX_SIZE GSP_PAGE_MIN_SIZE * 16
 
+/**
+ * DOC: GSP message queue element
+ *
+ *
https://github.com/NVIDIA/open-gpu-kernel-modules/blob/main/src/nvidia/inc/kernel/gpu/gsp/message_queue_priv.h
+ *
+ * The GSP command queue and status queue are message queues for the
+ * communication between software and GSP. The software submits the GSP
+ * RPC via the GSP command queue, GSP writes the status of the submitted
+ * RPC in the status queue.
+ *
+ * A GSP message queue element consists of three parts:
+ *
+ * - message element header (struct r535_gsp_msg), which mostly maintains
+ * the metadata for queuing the element.
+ *
+ * - RPC message header (struct nvfw_gsp_rpc), which maintains the info
+ * of the RPC. E.g., the RPC function number.
+ *
+ * - The payload, where the RPC message stays. E.g. the params of a
+ * specific RPC function. Some RPC functions also have their headers
+ * in the payload. E.g. rm_alloc, rm_control.
+ *
+ * The memory layout of a GSP message element can be illustrated below:
+ *
+ * +------------------------+
+ * | Message Element Header |
+ * |    (r535_gsp_msg)      |
+ * |                        |
+ * | (r535_gsp_msg.data)    |
+ * |          |             |
+ * |----------V-------------|
+ * |    GSP RPC Header      |
+ * |    (nvfw_gsp_rpc)      |
+ * |                        |
+ * | (nvfw_gsp_rpc.data)    |
+ * |          |             |
+ * |----------V-------------|
+ * |                        |
+ * |       Payload          |
+ * |                        |
+ * |   header(optional)     |
+ * |        params          |
+ * +------------------------+
+ *
+ */
+
 struct r535_gsp_msg {
 	u8 auth_tag_buffer[16];
 	u8 aad_buffer[16];
-- 
2.34.1

Zhi Wang

2024-Oct-31 08:52 UTC

head link

[PATCH v3 02/15] nvkm: rename "repc" to "gsp_rpc_len" on the GSP message recv path

The name "repc" has different meanings in different contexts.

To improve the readability, it's better to refine it to a name that
reflects what it actually represents.

Rename "repc" to "gsp_rpc_len" in the GSP message recv path.
Add an
section in the doc to explain the terms.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 46 +++++++++++--------
 1 file changed, 27 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 1a07c0191356..f6ed51921e50 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -104,6 +104,9 @@
  * |        params          |
  * +------------------------+
  *
+ * decoders:
+ *
+ * - gsp_rpc_len: size of (GSP RPC header + payload)
  */
 
 struct r535_gsp_msg {
@@ -133,7 +136,8 @@ r535_rpc_status_to_errno(uint32_t rpc_status)
 }
 
 static void *
-r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 repc, u32 *prepc, int *ptime)
+r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 gsp_rpc_len, u32 *prepc,
+		   int *ptime)
 {
 	struct r535_gsp_msg *mqe;
 	u32 size, rptr = *gsp->msgq.rptr;
@@ -141,7 +145,8 @@ r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 repc, u32
*prepc, int *ptime)
 	u8 *msg;
 	u32 len;
 
-	size = DIV_ROUND_UP(GSP_MSG_HDR_SIZE + repc, GSP_PAGE_SIZE);
+	size = DIV_ROUND_UP(GSP_MSG_HDR_SIZE + gsp_rpc_len,
+			    GSP_PAGE_SIZE);
 	if (WARN_ON(!size || size >= gsp->msgq.cnt))
 		return ERR_PTR(-EINVAL);
 
@@ -167,21 +172,21 @@ r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 repc, u32
*prepc, int *ptime)
 		return mqe->data;
 	}
 
-	size = ALIGN(repc + GSP_MSG_HDR_SIZE, GSP_PAGE_SIZE);
+	size = ALIGN(gsp_rpc_len + GSP_MSG_HDR_SIZE, GSP_PAGE_SIZE);
 
-	msg = kvmalloc(repc, GFP_KERNEL);
+	msg = kvmalloc(gsp_rpc_len, GFP_KERNEL);
 	if (!msg)
 		return ERR_PTR(-ENOMEM);
 
 	len = ((gsp->msgq.cnt - rptr) * GSP_PAGE_SIZE) - sizeof(*mqe);
-	len = min_t(u32, repc, len);
+	len = min_t(u32, gsp_rpc_len, len);
 	memcpy(msg, mqe->data, len);
 
-	repc -= len;
+	gsp_rpc_len -= len;
 
-	if (repc) {
+	if (gsp_rpc_len) {
 		mqe = (void *)((u8 *)gsp->shm.msgq.ptr + 0x1000 + 0 * 0x1000);
-		memcpy(msg + len, mqe, repc);
+		memcpy(msg + len, mqe, gsp_rpc_len);
 	}
 
 	rptr = (rptr + DIV_ROUND_UP(size, GSP_PAGE_SIZE)) % gsp->msgq.cnt;
@@ -192,9 +197,9 @@ r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 repc, u32
*prepc, int *ptime)
 }
 
 static void *
-r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 repc, int *ptime)
+r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *ptime)
 {
-	return r535_gsp_msgq_wait(gsp, repc, NULL, ptime);
+	return r535_gsp_msgq_wait(gsp, gsp_rpc_len, NULL, ptime);
 }
 
 static int
@@ -317,7 +322,7 @@ r535_gsp_msg_dump(struct nvkm_gsp *gsp, struct nvfw_gsp_rpc
*msg, int lvl)
 }
 
 static struct nvfw_gsp_rpc *
-r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 repc)
+r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 gsp_rpc_len)
 {
 	struct nvkm_subdev *subdev = &gsp->subdev;
 	struct nvfw_gsp_rpc *msg;
@@ -342,10 +347,11 @@ r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 repc)
 	r535_gsp_msg_dump(gsp, msg, NV_DBG_TRACE);
 
 	if (fn && msg->function == fn) {
-		if (repc) {
-			if (msg->length < sizeof(*msg) + repc) {
+		if (gsp_rpc_len) {
+			if (msg->length < sizeof(*msg) + gsp_rpc_len) {
 				nvkm_error(subdev, "msg len %d < %zd\n",
-					   msg->length, sizeof(*msg) + repc);
+					   msg->length, sizeof(*msg) +
+					   gsp_rpc_len);
 				r535_gsp_msg_dump(gsp, msg, NV_DBG_ERROR);
 				r535_gsp_msg_done(gsp, msg);
 				return ERR_PTR(-EIO);
@@ -414,7 +420,8 @@ r535_gsp_rpc_poll(struct nvkm_gsp *gsp, u32 fn)
 }
 
 static void *
-r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *argv, bool wait, u32 repc)
+r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *argv, bool wait,
+		  u32 gsp_rpc_len)
 {
 	struct nvfw_gsp_rpc *rpc = container_of(argv, typeof(*rpc), data);
 	struct nvfw_gsp_rpc *msg;
@@ -434,7 +441,7 @@ r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *argv, bool
wait, u32 repc)
 		return ERR_PTR(ret);
 
 	if (wait) {
-		msg = r535_gsp_msg_recv(gsp, fn, repc);
+		msg = r535_gsp_msg_recv(gsp, fn, gsp_rpc_len);
 		if (!IS_ERR_OR_NULL(msg))
 			repv = msg->data;
 		else
@@ -770,7 +777,8 @@ r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 argc)
 }
 
 static void *
-r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool wait, u32 repc)
+r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool wait,
+		  u32 gsp_rpc_len)
 {
 	struct nvfw_gsp_rpc *rpc = container_of(argv, typeof(*rpc), data);
 	struct r535_gsp_msg *cmd = container_of((void *)rpc, typeof(*cmd), data);
@@ -817,7 +825,7 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool
wait, u32 repc)
 
 		/* Wait for reply. */
 		if (wait) {
-			rpc = r535_gsp_msg_recv(gsp, fn, repc);
+			rpc = r535_gsp_msg_recv(gsp, fn, gsp_rpc_len);
 			if (!IS_ERR_OR_NULL(rpc))
 				repv = rpc->data;
 			else
@@ -826,7 +834,7 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool
wait, u32 repc)
 			repv = NULL;
 		}
 	} else {
-		repv = r535_gsp_rpc_send(gsp, argv, wait, repc);
+		repv = r535_gsp_rpc_send(gsp, argv, wait, gsp_rpc_len);
 	}
 
 done:
-- 
2.34.1

Zhi Wang

2024-Oct-31 08:52 UTC

head link

[PATCH v3 03/15] nvkm: rename "argv" to what it represnts on the GSP message send path

The name "argv" has different meanings in different functions.

To improve the readability, it's better to refine it to a name that
reflects what it represents.

Rename "repc" to what it represents in the GSP message send path.
Wrap the long container_of() into to_gsp_hdr() to make checkpatch.pl
happy.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 27 ++++++++++---------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index f6ed51921e50..6a9315addacb 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -121,6 +121,9 @@ struct r535_gsp_msg {
 
 #define GSP_MSG_HDR_SIZE offsetof(struct r535_gsp_msg, data)
 
+#define to_gsp_hdr(p, header) \
+	container_of((void *)p, typeof(*header), data)
+
 static int
 r535_rpc_status_to_errno(uint32_t rpc_status)
 {
@@ -203,9 +206,9 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len,
int *ptime)
 }
 
 static int
-r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *argv)
+r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 {
-	struct r535_gsp_msg *cmd = container_of(argv, typeof(*cmd), data);
+	struct r535_gsp_msg *cmd = to_gsp_hdr(rpc, cmd);
 	struct r535_gsp_msg *cqe;
 	u32 argc = cmd->checksum;
 	u64 *ptr = (void *)cmd;
@@ -420,10 +423,10 @@ r535_gsp_rpc_poll(struct nvkm_gsp *gsp, u32 fn)
 }
 
 static void *
-r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *argv, bool wait,
+r535_gsp_rpc_send(struct nvkm_gsp *gsp, void *payload, bool wait,
 		  u32 gsp_rpc_len)
 {
-	struct nvfw_gsp_rpc *rpc = container_of(argv, typeof(*rpc), data);
+	struct nvfw_gsp_rpc *rpc = to_gsp_hdr(payload, rpc);
 	struct nvfw_gsp_rpc *msg;
 	u32 fn = rpc->function;
 	void *repv = NULL;
@@ -777,11 +780,11 @@ r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 argc)
 }
 
 static void *
-r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool wait,
+r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload, bool wait,
 		  u32 gsp_rpc_len)
 {
-	struct nvfw_gsp_rpc *rpc = container_of(argv, typeof(*rpc), data);
-	struct r535_gsp_msg *cmd = container_of((void *)rpc, typeof(*cmd), data);
+	struct nvfw_gsp_rpc *rpc = to_gsp_hdr(payload, rpc);
+	struct r535_gsp_msg *cmd = to_gsp_hdr(rpc, cmd);
 	const u32 max_msg_size = (16 * 0x1000) - sizeof(struct r535_gsp_msg);
 	const u32 max_rpc_size = max_msg_size - sizeof(*rpc);
 	u32 rpc_size = rpc->length - sizeof(*rpc);
@@ -795,11 +798,11 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool
wait,
 		rpc->length = sizeof(*rpc) + max_rpc_size;
 		cmd->checksum = rpc->length;
 
-		repv = r535_gsp_rpc_send(gsp, argv, false, 0);
+		repv = r535_gsp_rpc_send(gsp, payload, false, 0);
 		if (IS_ERR(repv))
 			goto done;
 
-		argv += max_rpc_size;
+		payload += max_rpc_size;
 		rpc_size -= max_rpc_size;
 
 		/* Remaining chunks sent as CONTINUATION_RECORD RPCs. */
@@ -813,13 +816,13 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool
wait,
 				goto done;
 			}
 
-			memcpy(next, argv, size);
+			memcpy(next, payload, size);
 
 			repv = r535_gsp_rpc_send(gsp, next, false, 0);
 			if (IS_ERR(repv))
 				goto done;
 
-			argv += size;
+			payload += size;
 			rpc_size -= size;
 		}
 
@@ -834,7 +837,7 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *argv, bool
wait,
 			repv = NULL;
 		}
 	} else {
-		repv = r535_gsp_rpc_send(gsp, argv, wait, gsp_rpc_len);
+		repv = r535_gsp_rpc_send(gsp, payload, wait, gsp_rpc_len);
 	}
 
 done:
-- 
2.34.1

Zhi Wang

2024-Oct-31 08:52 UTC

head link

[PATCH v3 04/15] nvkm: remove unused param repc in *rm_alloc_push()

The user of *rm_alloc_push() always pass 0 in repc.

Remove unused param repc since no user actually uses it.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h | 8 ++++----
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 8 +++-----
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
index f52143df45c1..79b98f3d8656 100644
--- a/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
+++ b/drivers/gpu/drm/nouveau/include/nvkm/subdev/gsp.h
@@ -192,7 +192,7 @@ struct nvkm_gsp {
 		void (*rm_ctrl_done)(struct nvkm_gsp_object *, void *repv);
 
 		void *(*rm_alloc_get)(struct nvkm_gsp_object *, u32 oclass, u32 argc);
-		void *(*rm_alloc_push)(struct nvkm_gsp_object *, void *argv, u32 repc);
+		void *(*rm_alloc_push)(struct nvkm_gsp_object *, void *argv);
 		void (*rm_alloc_done)(struct nvkm_gsp_object *, void *repv);
 
 		int (*rm_free)(struct nvkm_gsp_object *);
@@ -331,9 +331,9 @@ nvkm_gsp_rm_alloc_get(struct nvkm_gsp_object *parent, u32
handle, u32 oclass, u3
 }
 
 static inline void *
-nvkm_gsp_rm_alloc_push(struct nvkm_gsp_object *object, void *argv, u32 repc)
+nvkm_gsp_rm_alloc_push(struct nvkm_gsp_object *object, void *argv)
 {
-	void *repv = object->client->gsp->rm->rm_alloc_push(object, argv,
repc);
+	void *repv = object->client->gsp->rm->rm_alloc_push(object, argv);
 
 	if (IS_ERR(repv))
 		object->client = NULL;
@@ -344,7 +344,7 @@ nvkm_gsp_rm_alloc_push(struct nvkm_gsp_object *object, void
*argv, u32 repc)
 static inline int
 nvkm_gsp_rm_alloc_wr(struct nvkm_gsp_object *object, void *argv)
 {
-	void *repv = nvkm_gsp_rm_alloc_push(object, argv, 0);
+	void *repv = nvkm_gsp_rm_alloc_push(object, argv);
 
 	if (IS_ERR(repv))
 		return PTR_ERR(repv);
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 6a9315addacb..881e6da9987a 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -647,13 +647,13 @@ r535_gsp_rpc_rm_alloc_done(struct nvkm_gsp_object *object,
void *repv)
 }
 
 static void *
-r535_gsp_rpc_rm_alloc_push(struct nvkm_gsp_object *object, void *argv, u32
repc)
+r535_gsp_rpc_rm_alloc_push(struct nvkm_gsp_object *object, void *argv)
 {
 	rpc_gsp_rm_alloc_v03_00 *rpc = container_of(argv, typeof(*rpc), params);
 	struct nvkm_gsp *gsp = object->client->gsp;
-	void *ret;
+	void *ret = NULL;
 
-	rpc = nvkm_gsp_rpc_push(gsp, rpc, true, sizeof(*rpc) + repc);
+	rpc = nvkm_gsp_rpc_push(gsp, rpc, true, sizeof(*rpc));
 	if (IS_ERR_OR_NULL(rpc))
 		return rpc;
 
@@ -661,8 +661,6 @@ r535_gsp_rpc_rm_alloc_push(struct nvkm_gsp_object *object,
void *argv, u32 repc)
 		ret = ERR_PTR(r535_rpc_status_to_errno(rpc->status));
 		if (PTR_ERR(ret) != -EAGAIN)
 			nvkm_error(&gsp->subdev, "RM_ALLOC: 0x%x\n",
rpc->status);
-	} else {
-		ret = repc ? rpc->params : NULL;
 	}
 
 	nvkm_gsp_rpc_done(gsp, rpc);
-- 
2.34.1

Zhi Wang

2024-Oct-31 08:52 UTC

head link

[PATCH v3 05/15] nvkm: rename "argv" to what it represents in rm_{alloc, ctrl}_()

The name "argv" has different meanings in different functions.

To improve the readability, it's better to refine it to a name that
reflects what it represents.

Rename "argv" to what it represents. Wrap the long container_of() into
to_payload_header() to denote a clear meaning and make checkpatch.pl
happy.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 25 +++++++++++--------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 881e6da9987a..d00c446e2bf9 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -124,6 +124,9 @@ struct r535_gsp_msg {
 #define to_gsp_hdr(p, header) \
 	container_of((void *)p, typeof(*header), data)
 
+#define to_payload_hdr(p, header) \
+	container_of((void *)p, typeof(*header), params)
+
 static int
 r535_rpc_status_to_errno(uint32_t rpc_status)
 {
@@ -639,17 +642,17 @@ r535_gsp_rpc_rm_free(struct nvkm_gsp_object *object)
 }
 
 static void
-r535_gsp_rpc_rm_alloc_done(struct nvkm_gsp_object *object, void *repv)
+r535_gsp_rpc_rm_alloc_done(struct nvkm_gsp_object *object, void *params)
 {
-	rpc_gsp_rm_alloc_v03_00 *rpc = container_of(repv, typeof(*rpc), params);
+	rpc_gsp_rm_alloc_v03_00 *rpc = to_payload_hdr(params, rpc);
 
 	nvkm_gsp_rpc_done(object->client->gsp, rpc);
 }
 
 static void *
-r535_gsp_rpc_rm_alloc_push(struct nvkm_gsp_object *object, void *argv)
+r535_gsp_rpc_rm_alloc_push(struct nvkm_gsp_object *object, void *params)
 {
-	rpc_gsp_rm_alloc_v03_00 *rpc = container_of(argv, typeof(*rpc), params);
+	rpc_gsp_rm_alloc_v03_00 *rpc = to_payload_hdr(params, rpc);
 	struct nvkm_gsp *gsp = object->client->gsp;
 	void *ret = NULL;
 
@@ -692,25 +695,25 @@ r535_gsp_rpc_rm_alloc_get(struct nvkm_gsp_object *object,
u32 oclass, u32 argc)
 }
 
 static void
-r535_gsp_rpc_rm_ctrl_done(struct nvkm_gsp_object *object, void *repv)
+r535_gsp_rpc_rm_ctrl_done(struct nvkm_gsp_object *object, void *params)
 {
-	rpc_gsp_rm_control_v03_00 *rpc = container_of(repv, typeof(*rpc), params);
+	rpc_gsp_rm_control_v03_00 *rpc = to_payload_hdr(params, rpc);
 
-	if (!repv)
+	if (!params)
 		return;
 	nvkm_gsp_rpc_done(object->client->gsp, rpc);
 }
 
 static int
-r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object, void **argv, u32
repc)
+r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object, void **params, u32
repc)
 {
-	rpc_gsp_rm_control_v03_00 *rpc = container_of((*argv), typeof(*rpc), params);
+	rpc_gsp_rm_control_v03_00 *rpc = to_payload_hdr((*params), rpc);
 	struct nvkm_gsp *gsp = object->client->gsp;
 	int ret = 0;
 
 	rpc = nvkm_gsp_rpc_push(gsp, rpc, true, repc);
 	if (IS_ERR_OR_NULL(rpc)) {
-		*argv = NULL;
+		*params = NULL;
 		return PTR_ERR(rpc);
 	}
 
@@ -722,7 +725,7 @@ r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object,
void **argv, u32 repc)
 	}
 
 	if (repc)
-		*argv = rpc->params;
+		*params = rpc->params;
 	else
 		nvkm_gsp_rpc_done(gsp, rpc);
 
-- 
2.34.1

Zhi Wang

2024-Oct-31 08:52 UTC

head link

[PATCH v3 06/15] nvkm: rename "argc" to what it represents in GSP RPC routines

The name "argc" has different meanings in different functions.

To improve the readability, it's better to refine it to a name that
reflects what it represents.

Rename "argc" to what it represents. Add terms in the decoder section
to
explain their meaning.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 58 +++++++++++--------
 1 file changed, 34 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index d00c446e2bf9..3bb6b161c9b7 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -107,6 +107,8 @@
  * decoders:
  *
  * - gsp_rpc_len: size of (GSP RPC header + payload)
+ * - params_size: size of params in the payload
+ * - payload_size: size of (header if exists + params) in the payload
  */
 
 struct r535_gsp_msg {
@@ -213,21 +215,21 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 {
 	struct r535_gsp_msg *cmd = to_gsp_hdr(rpc, cmd);
 	struct r535_gsp_msg *cqe;
-	u32 argc = cmd->checksum;
+	u32 gsp_rpc_len = cmd->checksum;
 	u64 *ptr = (void *)cmd;
 	u64 *end;
 	u64 csum = 0;
 	int free, time = 1000000;
-	u32 wptr, size, step;
+	u32 wptr, size, step, len;
 	u32 off = 0;
 
-	argc = ALIGN(GSP_MSG_HDR_SIZE + argc, GSP_PAGE_SIZE);
+	len = ALIGN(GSP_MSG_HDR_SIZE + gsp_rpc_len, GSP_PAGE_SIZE);
 
-	end = (u64 *)((char *)ptr + argc);
+	end = (u64 *)((char *)ptr + len);
 	cmd->pad = 0;
 	cmd->checksum = 0;
 	cmd->sequence = gsp->cmdq.seq++;
-	cmd->elem_count = DIV_ROUND_UP(argc, 0x1000);
+	cmd->elem_count = DIV_ROUND_UP(len, 0x1000);
 
 	while (ptr < end)
 		csum ^= *ptr++;
@@ -255,7 +257,7 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 
 		cqe = (void *)((u8 *)gsp->shm.cmdq.ptr + 0x1000 + wptr * 0x1000);
 		step = min_t(u32, free, (gsp->cmdq.cnt - wptr));
-		size = min_t(u32, argc, step * GSP_PAGE_SIZE);
+		size = min_t(u32, len, step * GSP_PAGE_SIZE);
 
 		memcpy(cqe, (u8 *)cmd + off, size);
 
@@ -264,8 +266,8 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 			wptr = 0;
 
 		off  += size;
-		argc -= size;
-	} while(argc);
+		len -= size;
+	} while (len);
 
 	nvkm_trace(&gsp->subdev, "cmdq: wptr %d\n", wptr);
 	wmb();
@@ -279,17 +281,17 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 }
 
 static void *
-r535_gsp_cmdq_get(struct nvkm_gsp *gsp, u32 argc)
+r535_gsp_cmdq_get(struct nvkm_gsp *gsp, u32 gsp_rpc_len)
 {
 	struct r535_gsp_msg *cmd;
-	u32 size = GSP_MSG_HDR_SIZE + argc;
+	u32 size = GSP_MSG_HDR_SIZE + gsp_rpc_len;
 
 	size = ALIGN(size, GSP_MSG_MIN_SIZE);
 	cmd = kvzalloc(size, GFP_KERNEL);
 	if (!cmd)
 		return ERR_PTR(-ENOMEM);
 
-	cmd->checksum = argc;
+	cmd->checksum = gsp_rpc_len;
 	return cmd->data;
 }
 
@@ -672,16 +674,22 @@ r535_gsp_rpc_rm_alloc_push(struct nvkm_gsp_object *object,
void *params)
 }
 
 static void *
-r535_gsp_rpc_rm_alloc_get(struct nvkm_gsp_object *object, u32 oclass, u32 argc)
+r535_gsp_rpc_rm_alloc_get(struct nvkm_gsp_object *object, u32 oclass,
+			  u32 params_size)
 {
 	struct nvkm_gsp_client *client = object->client;
 	struct nvkm_gsp *gsp = client->gsp;
 	rpc_gsp_rm_alloc_v03_00 *rpc;
 
-	nvkm_debug(&gsp->subdev, "cli:0x%08x obj:0x%08x new obj:0x%08x
cls:0x%08x argc:%d\n",
-		   client->object.handle, object->parent->handle, object->handle,
oclass, argc);
+	nvkm_debug(&gsp->subdev, "cli:0x%08x obj:0x%08x new
obj:0x%08x\n",
+		   client->object.handle, object->parent->handle,
+		   object->handle);
 
-	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_GSP_RM_ALLOC, sizeof(*rpc) +
argc);
+	nvkm_debug(&gsp->subdev, "cls:0x%08x params_size:%d\n",
oclass,
+		   params_size);
+
+	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_GSP_RM_ALLOC,
+			       sizeof(*rpc) + params_size);
 	if (IS_ERR(rpc))
 		return rpc;
 
@@ -690,7 +698,7 @@ r535_gsp_rpc_rm_alloc_get(struct nvkm_gsp_object *object,
u32 oclass, u32 argc)
 	rpc->hObject = object->handle;
 	rpc->hClass = oclass;
 	rpc->status = 0;
-	rpc->paramsSize = argc;
+	rpc->paramsSize = params_size;
 	return rpc->params;
 }
 
@@ -733,16 +741,17 @@ r535_gsp_rpc_rm_ctrl_push(struct nvkm_gsp_object *object,
void **params, u32 rep
 }
 
 static void *
-r535_gsp_rpc_rm_ctrl_get(struct nvkm_gsp_object *object, u32 cmd, u32 argc)
+r535_gsp_rpc_rm_ctrl_get(struct nvkm_gsp_object *object, u32 cmd, u32
params_size)
 {
 	struct nvkm_gsp_client *client = object->client;
 	struct nvkm_gsp *gsp = client->gsp;
 	rpc_gsp_rm_control_v03_00 *rpc;
 
-	nvkm_debug(&gsp->subdev, "cli:0x%08x obj:0x%08x ctrl cmd:0x%08x
argc:%d\n",
-		   client->object.handle, object->handle, cmd, argc);
+	nvkm_debug(&gsp->subdev, "cli:0x%08x obj:0x%08x ctrl cmd:0x%08x
params_size:%d\n",
+		   client->object.handle, object->handle, cmd, params_size);
 
-	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_GSP_RM_CONTROL, sizeof(*rpc)
+ argc);
+	rpc = nvkm_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_GSP_RM_CONTROL,
+			       sizeof(*rpc) + params_size);
 	if (IS_ERR(rpc))
 		return rpc;
 
@@ -750,7 +759,7 @@ r535_gsp_rpc_rm_ctrl_get(struct nvkm_gsp_object *object, u32
cmd, u32 argc)
 	rpc->hObject    = object->handle;
 	rpc->cmd	= cmd;
 	rpc->status     = 0;
-	rpc->paramsSize = argc;
+	rpc->paramsSize = params_size;
 	return rpc->params;
 }
 
@@ -763,11 +772,12 @@ r535_gsp_rpc_done(struct nvkm_gsp *gsp, void *repv)
 }
 
 static void *
-r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 argc)
+r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 payload_size)
 {
 	struct nvfw_gsp_rpc *rpc;
 
-	rpc = r535_gsp_cmdq_get(gsp, ALIGN(sizeof(*rpc) + argc, sizeof(u64)));
+	rpc = r535_gsp_cmdq_get(gsp, ALIGN(sizeof(*rpc) + payload_size,
+				sizeof(u64)));
 	if (IS_ERR(rpc))
 		return ERR_CAST(rpc);
 
@@ -776,7 +786,7 @@ r535_gsp_rpc_get(struct nvkm_gsp *gsp, u32 fn, u32 argc)
 	rpc->function = fn;
 	rpc->rpc_result = 0xffffffff;
 	rpc->rpc_result_private = 0xffffffff;
-	rpc->length = sizeof(*rpc) + argc;
+	rpc->length = sizeof(*rpc) + payload_size;
 	return rpc->data;
 }
 
-- 
2.34.1

Zhi Wang

2024-Oct-31 08:52 UTC

head link

[PATCH v3 07/15] nvkm: fix the broken marco GSP_MSG_MAX_SIZE

The macro GSP_MSG_MAX_SIZE refers to another macro that doesn't exist.
It represents the max GSP message element size.

Fix the broken marco so it can be used to replace some magic numbers in
the code.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 3bb6b161c9b7..8d7b884f5adb 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -58,7 +58,7 @@
 #include <linux/parser.h>
 
 #define GSP_MSG_MIN_SIZE GSP_PAGE_SIZE
-#define GSP_MSG_MAX_SIZE GSP_PAGE_MIN_SIZE * 16
+#define GSP_MSG_MAX_SIZE (GSP_MSG_MIN_SIZE * 16)
 
 /**
  * DOC: GSP message queue element
-- 
2.34.1

Zhi Wang

2024-Oct-31 08:52 UTC

head link

[PATCH v3 08/15] nvkm: remove the magic number in r535_gsp_rpc_push()

There has been a GSP_MSG_MAX_SIZE which represents the max size of a GSP
message element header. Use it instead of a magic number.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 8d7b884f5adb..5bc56a9ba3f8 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -796,7 +796,7 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload, bool
wait,
 {
 	struct nvfw_gsp_rpc *rpc = to_gsp_hdr(payload, rpc);
 	struct r535_gsp_msg *cmd = to_gsp_hdr(rpc, cmd);
-	const u32 max_msg_size = (16 * 0x1000) - sizeof(struct r535_gsp_msg);
+	const u32 max_msg_size = GSP_MSG_MAX_SIZE - sizeof(*cmd);
 	const u32 max_rpc_size = max_msg_size - sizeof(*rpc);
 	u32 rpc_size = rpc->length - sizeof(*rpc);
 	void *repv;
-- 
2.34.1

Zhi Wang

2024-Oct-31 08:52 UTC

head link

[PATCH v3 09/15] nvkm: refine the variable names in r535_gsp_rpc_push()

The variable names in r535_gsp_rpc_push() are quite confusing and some
of them are not representing what they really are.

Update the names and explanations in the decoder section of the
kernel doc. Refine the names to align with the terms in the kernel doc.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 27 ++++++++++---------
 1 file changed, 15 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 5bc56a9ba3f8..2ac0868eb30c 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -106,6 +106,9 @@
  *
  * decoders:
  *
+ * - gsp_msg(msg): GSP message element (element header + GSP RPC header +
+ *   payload)
+ * - gsp_rpc(rpc): GSP RPC (RPC header + payload)
  * - gsp_rpc_len: size of (GSP RPC header + payload)
  * - params_size: size of params in the payload
  * - payload_size: size of (header if exists + params) in the payload
@@ -795,30 +798,30 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload,
bool wait,
 		  u32 gsp_rpc_len)
 {
 	struct nvfw_gsp_rpc *rpc = to_gsp_hdr(payload, rpc);
-	struct r535_gsp_msg *cmd = to_gsp_hdr(rpc, cmd);
-	const u32 max_msg_size = GSP_MSG_MAX_SIZE - sizeof(*cmd);
-	const u32 max_rpc_size = max_msg_size - sizeof(*rpc);
-	u32 rpc_size = rpc->length - sizeof(*rpc);
+	struct r535_gsp_msg *msg = to_gsp_hdr(rpc, msg);
+	const u32 max_rpc_size = GSP_MSG_MAX_SIZE - sizeof(*msg);
+	const u32 max_payload_size = max_rpc_size - sizeof(*rpc);
+	u32 payload_size = rpc->length - sizeof(*rpc);
 	void *repv;
 
 	mutex_lock(&gsp->cmdq.mutex);
-	if (rpc_size > max_rpc_size) {
+	if (payload_size > max_payload_size) {
 		const u32 fn = rpc->function;
 
 		/* Adjust length, and send initial RPC. */
-		rpc->length = sizeof(*rpc) + max_rpc_size;
-		cmd->checksum = rpc->length;
+		rpc->length = sizeof(*rpc) + max_payload_size;
+		msg->checksum = rpc->length;
 
 		repv = r535_gsp_rpc_send(gsp, payload, false, 0);
 		if (IS_ERR(repv))
 			goto done;
 
-		payload += max_rpc_size;
-		rpc_size -= max_rpc_size;
+		payload += max_payload_size;
+		payload_size -= max_payload_size;
 
 		/* Remaining chunks sent as CONTINUATION_RECORD RPCs. */
-		while (rpc_size) {
-			u32 size = min(rpc_size, max_rpc_size);
+		while (payload_size) {
+			u32 size = min(payload_size, max_payload_size);
 			void *next;
 
 			next = r535_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_CONTINUATION_RECORD,
size);
@@ -834,7 +837,7 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload, bool
wait,
 				goto done;
 
 			payload += size;
-			rpc_size -= size;
+			payload_size -= size;
 		}
 
 		/* Wait for reply. */
-- 
2.34.1

Zhi Wang

2024-Oct-31 08:52 UTC

head link

[PATCH v3 10/15] nvkm: refine the variable names in r535_gsp_msg_recv()

The variable "msg" in r535_gsp_msg_recv() actually means the GSP RPC.

Refine the names to align with the terms in the kernel doc.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 47 ++++++++++---------
 1 file changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 2ac0868eb30c..c4164d79240c 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -336,59 +336,60 @@ static struct nvfw_gsp_rpc *
 r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32 gsp_rpc_len)
 {
 	struct nvkm_subdev *subdev = &gsp->subdev;
-	struct nvfw_gsp_rpc *msg;
+	struct nvfw_gsp_rpc *rpc;
 	int time = 4000000, i;
 	u32 size;
 
 retry:
-	msg = r535_gsp_msgq_wait(gsp, sizeof(*msg), &size, &time);
-	if (IS_ERR_OR_NULL(msg))
-		return msg;
+	rpc = r535_gsp_msgq_wait(gsp, sizeof(*rpc), &size, &time);
+	if (IS_ERR_OR_NULL(rpc))
+		return rpc;
 
-	msg = r535_gsp_msgq_recv(gsp, msg->length, &time);
-	if (IS_ERR_OR_NULL(msg))
-		return msg;
+	rpc = r535_gsp_msgq_recv(gsp, rpc->length, &time);
+	if (IS_ERR_OR_NULL(rpc))
+		return rpc;
 
-	if (msg->rpc_result) {
-		r535_gsp_msg_dump(gsp, msg, NV_DBG_ERROR);
-		r535_gsp_msg_done(gsp, msg);
+	if (rpc->rpc_result) {
+		r535_gsp_msg_dump(gsp, rpc, NV_DBG_ERROR);
+		r535_gsp_msg_done(gsp, rpc);
 		return ERR_PTR(-EINVAL);
 	}
 
-	r535_gsp_msg_dump(gsp, msg, NV_DBG_TRACE);
+	r535_gsp_msg_dump(gsp, rpc, NV_DBG_TRACE);
 
-	if (fn && msg->function == fn) {
+	if (fn && rpc->function == fn) {
 		if (gsp_rpc_len) {
-			if (msg->length < sizeof(*msg) + gsp_rpc_len) {
-				nvkm_error(subdev, "msg len %d < %zd\n",
-					   msg->length, sizeof(*msg) +
+			if (rpc->length < sizeof(*rpc) + gsp_rpc_len) {
+				nvkm_error(subdev, "rpc len %d < %zd\n",
+					   rpc->length, sizeof(*rpc) +
 					   gsp_rpc_len);
-				r535_gsp_msg_dump(gsp, msg, NV_DBG_ERROR);
-				r535_gsp_msg_done(gsp, msg);
+				r535_gsp_msg_dump(gsp, rpc, NV_DBG_ERROR);
+				r535_gsp_msg_done(gsp, rpc);
 				return ERR_PTR(-EIO);
 			}
 
-			return msg;
+			return rpc;
 		}
 
-		r535_gsp_msg_done(gsp, msg);
+		r535_gsp_msg_done(gsp, rpc);
 		return NULL;
 	}
 
 	for (i = 0; i < gsp->msgq.ntfy_nr; i++) {
 		struct nvkm_gsp_msgq_ntfy *ntfy = &gsp->msgq.ntfy[i];
 
-		if (ntfy->fn == msg->function) {
+		if (ntfy->fn == rpc->function) {
 			if (ntfy->func)
-				ntfy->func(ntfy->priv, ntfy->fn, msg->data, msg->length -
sizeof(*msg));
+				ntfy->func(ntfy->priv, ntfy->fn, rpc->data,
+					   rpc->length - sizeof(*rpc));
 			break;
 		}
 	}
 
 	if (i == gsp->msgq.ntfy_nr)
-		r535_gsp_msg_dump(gsp, msg, NV_DBG_WARN);
+		r535_gsp_msg_dump(gsp, rpc, NV_DBG_WARN);
 
-	r535_gsp_msg_done(gsp, msg);
+	r535_gsp_msg_done(gsp, rpc);
 	if (fn)
 		goto retry;
 
-- 
2.34.1

Zhi Wang

2024-Oct-31 08:52 UTC

head link

[PATCH v3 11/15] nvkm: rename the variable "cmd" to "msg" in r535_gsp_cmdq_{get, push}()

Refine the name to align with the terms in the kernel doc.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 32 +++++++++----------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index c4164d79240c..8b507858a63d 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -216,10 +216,10 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len,
int *ptime)
 static int
 r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 {
-	struct r535_gsp_msg *cmd = to_gsp_hdr(rpc, cmd);
+	struct r535_gsp_msg *msg = to_gsp_hdr(rpc, msg);
 	struct r535_gsp_msg *cqe;
-	u32 gsp_rpc_len = cmd->checksum;
-	u64 *ptr = (void *)cmd;
+	u32 gsp_rpc_len = msg->checksum;
+	u64 *ptr = (void *)msg;
 	u64 *end;
 	u64 csum = 0;
 	int free, time = 1000000;
@@ -229,15 +229,15 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 	len = ALIGN(GSP_MSG_HDR_SIZE + gsp_rpc_len, GSP_PAGE_SIZE);
 
 	end = (u64 *)((char *)ptr + len);
-	cmd->pad = 0;
-	cmd->checksum = 0;
-	cmd->sequence = gsp->cmdq.seq++;
-	cmd->elem_count = DIV_ROUND_UP(len, 0x1000);
+	msg->pad = 0;
+	msg->checksum = 0;
+	msg->sequence = gsp->cmdq.seq++;
+	msg->elem_count = DIV_ROUND_UP(len, 0x1000);
 
 	while (ptr < end)
 		csum ^= *ptr++;
 
-	cmd->checksum = upper_32_bits(csum) ^ lower_32_bits(csum);
+	msg->checksum = upper_32_bits(csum) ^ lower_32_bits(csum);
 
 	wptr = *gsp->cmdq.wptr;
 
@@ -254,7 +254,7 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 		} while(--time);
 
 		if (WARN_ON(!time)) {
-			kvfree(cmd);
+			kvfree(msg);
 			return -ETIMEDOUT;
 		}
 
@@ -262,7 +262,7 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 		step = min_t(u32, free, (gsp->cmdq.cnt - wptr));
 		size = min_t(u32, len, step * GSP_PAGE_SIZE);
 
-		memcpy(cqe, (u8 *)cmd + off, size);
+		memcpy(cqe, (u8 *)msg + off, size);
 
 		wptr += DIV_ROUND_UP(size, 0x1000);
 		if (wptr == gsp->cmdq.cnt)
@@ -279,23 +279,23 @@ r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 
 	nvkm_falcon_wr32(&gsp->falcon, 0xc00, 0x00000000);
 
-	kvfree(cmd);
+	kvfree(msg);
 	return 0;
 }
 
 static void *
 r535_gsp_cmdq_get(struct nvkm_gsp *gsp, u32 gsp_rpc_len)
 {
-	struct r535_gsp_msg *cmd;
+	struct r535_gsp_msg *msg;
 	u32 size = GSP_MSG_HDR_SIZE + gsp_rpc_len;
 
 	size = ALIGN(size, GSP_MSG_MIN_SIZE);
-	cmd = kvzalloc(size, GFP_KERNEL);
-	if (!cmd)
+	msg = kvzalloc(size, GFP_KERNEL);
+	if (!msg)
 		return ERR_PTR(-ENOMEM);
 
-	cmd->checksum = gsp_rpc_len;
-	return cmd->data;
+	msg->checksum = gsp_rpc_len;
+	return msg->data;
 }
 
 struct nvfw_gsp_rpc {
-- 
2.34.1

Zhi Wang

2024-Oct-31 08:52 UTC

head link

[PATCH v3 12/15] nvkm: factor out r535_gsp_msgq_peek()

To receive a GSP message queue element from the GSP status queue, the
driver needs to make sure there are available elements in the queue.

The previous r535_gsp_msgq_wait() consists of three functions, which is
a little too complicated for a single function:
- wait for an available element.
- peek the message element header in the queue.
- recevice the element from the queue.

Factor out r535_gsp_msgq_peek() and divide the functions in
r535_gsp_msgq_wait() into three functions.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 68 ++++++++++++-------
 1 file changed, 45 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 8b507858a63d..7818c0be41f2 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -146,20 +146,16 @@ r535_rpc_status_to_errno(uint32_t rpc_status)
 	}
 }
 
-static void *
-r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 gsp_rpc_len, u32 *prepc,
-		   int *ptime)
+static int
+r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *ptime)
 {
-	struct r535_gsp_msg *mqe;
 	u32 size, rptr = *gsp->msgq.rptr;
 	int used;
-	u8 *msg;
-	u32 len;
 
 	size = DIV_ROUND_UP(GSP_MSG_HDR_SIZE + gsp_rpc_len,
 			    GSP_PAGE_SIZE);
 	if (WARN_ON(!size || size >= gsp->msgq.cnt))
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
 
 	do {
 		u32 wptr = *gsp->msgq.wptr;
@@ -174,15 +170,48 @@ r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 gsp_rpc_len,
u32 *prepc,
 	} while (--(*ptime));
 
 	if (WARN_ON(!*ptime))
-		return ERR_PTR(-ETIMEDOUT);
+		return -ETIMEDOUT;
 
-	mqe = (void *)((u8 *)gsp->shm.msgq.ptr + 0x1000 + rptr * 0x1000);
+	return used;
+}
 
-	if (prepc) {
-		*prepc = (used * GSP_PAGE_SIZE) - sizeof(*mqe);
-		return mqe->data;
-	}
+static struct r535_gsp_msg *
+r535_gsp_msgq_get_entry(struct nvkm_gsp *gsp)
+{
+	u32 rptr = *gsp->msgq.rptr;
 
+	return (void *)((u8 *)gsp->shm.msgq.ptr + 0x1000 + rptr * 0x1000);
+}
+
+static void *
+r535_gsp_msgq_peek(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
+{
+	struct r535_gsp_msg *mqe;
+	int ret;
+
+	ret = r535_gsp_msgq_wait(gsp, gsp_rpc_len, retries);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	mqe = r535_gsp_msgq_get_entry(gsp);
+
+	return mqe->data;
+}
+
+static void *
+r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
+{
+	u32 rptr = *gsp->msgq.rptr;
+	struct r535_gsp_msg *mqe;
+	u32 size, len;
+	u8 *msg;
+	int ret;
+
+	ret = r535_gsp_msgq_wait(gsp, gsp_rpc_len, retries);
+	if (ret < 0)
+		return ERR_PTR(ret);
+
+	mqe = r535_gsp_msgq_get_entry(gsp);
 	size = ALIGN(gsp_rpc_len + GSP_MSG_HDR_SIZE, GSP_PAGE_SIZE);
 
 	msg = kvmalloc(gsp_rpc_len, GFP_KERNEL);
@@ -207,12 +236,6 @@ r535_gsp_msgq_wait(struct nvkm_gsp *gsp, u32 gsp_rpc_len,
u32 *prepc,
 	return msg;
 }
 
-static void *
-r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *ptime)
-{
-	return r535_gsp_msgq_wait(gsp, gsp_rpc_len, NULL, ptime);
-}
-
 static int
 r535_gsp_cmdq_push(struct nvkm_gsp *gsp, void *rpc)
 {
@@ -337,15 +360,14 @@ r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32
gsp_rpc_len)
 {
 	struct nvkm_subdev *subdev = &gsp->subdev;
 	struct nvfw_gsp_rpc *rpc;
-	int time = 4000000, i;
-	u32 size;
+	int retries = 4000000, i;
 
 retry:
-	rpc = r535_gsp_msgq_wait(gsp, sizeof(*rpc), &size, &time);
+	rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), &retries);
 	if (IS_ERR_OR_NULL(rpc))
 		return rpc;
 
-	rpc = r535_gsp_msgq_recv(gsp, rpc->length, &time);
+	rpc = r535_gsp_msgq_recv(gsp, rpc->length, &retries);
 	if (IS_ERR_OR_NULL(rpc))
 		return rpc;
 
-- 
2.34.1

Zhi Wang

2024-Oct-31 08:52 UTC

head link

[PATCH v3 13/15] nvkm: factor out r535_gsp_msgq_recv_one_elem()

Prepare for supporting receive the large GSP RPC message.

Factor out r535_gsp_msgq_recv_one_elem(). Fold its params into a data
structure of params. Move the allocation of the GSP RPC message to its
caller. Refine the variable names in the re-factor.

No functional change is intended.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 59 ++++++++++++++-----
 1 file changed, 44 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 7818c0be41f2..08a74d8bd06f 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -109,6 +109,7 @@
  * - gsp_msg(msg): GSP message element (element header + GSP RPC header +
  *   payload)
  * - gsp_rpc(rpc): GSP RPC (RPC header + payload)
+ * - gsp_rpc_buf: buffer for (GSP RPC header + payload)
  * - gsp_rpc_len: size of (GSP RPC header + payload)
  * - params_size: size of params in the payload
  * - payload_size: size of (header if exists + params) in the payload
@@ -198,42 +199,70 @@ r535_gsp_msgq_peek(struct nvkm_gsp *gsp, u32 gsp_rpc_len,
int *retries)
 	return mqe->data;
 }
 
+struct r535_gsp_msg_info {
+	int *retries;
+	u32 gsp_rpc_len;
+	void *gsp_rpc_buf;
+};
+
 static void *
-r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
+r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
+			    struct r535_gsp_msg_info *info)
 {
+	u8 *buf = info->gsp_rpc_buf;
 	u32 rptr = *gsp->msgq.rptr;
 	struct r535_gsp_msg *mqe;
-	u32 size, len;
-	u8 *msg;
+	u32 size, expected, len;
 	int ret;
 
-	ret = r535_gsp_msgq_wait(gsp, gsp_rpc_len, retries);
+	expected = info->gsp_rpc_len;
+
+	ret = r535_gsp_msgq_wait(gsp, expected, info->retries);
 	if (ret < 0)
 		return ERR_PTR(ret);
 
 	mqe = r535_gsp_msgq_get_entry(gsp);
-	size = ALIGN(gsp_rpc_len + GSP_MSG_HDR_SIZE, GSP_PAGE_SIZE);
-
-	msg = kvmalloc(gsp_rpc_len, GFP_KERNEL);
-	if (!msg)
-		return ERR_PTR(-ENOMEM);
+	size = ALIGN(expected + GSP_MSG_HDR_SIZE, GSP_PAGE_SIZE);
 
 	len = ((gsp->msgq.cnt - rptr) * GSP_PAGE_SIZE) - sizeof(*mqe);
-	len = min_t(u32, gsp_rpc_len, len);
-	memcpy(msg, mqe->data, len);
+	len = min_t(u32, expected, len);
+	memcpy(buf, mqe->data, len);
 
-	gsp_rpc_len -= len;
+	expected -= len;
 
-	if (gsp_rpc_len) {
+	if (expected) {
 		mqe = (void *)((u8 *)gsp->shm.msgq.ptr + 0x1000 + 0 * 0x1000);
-		memcpy(msg + len, mqe, gsp_rpc_len);
+		memcpy(buf + len, mqe, expected);
 	}
 
 	rptr = (rptr + DIV_ROUND_UP(size, GSP_PAGE_SIZE)) % gsp->msgq.cnt;
 
 	mb();
 	(*gsp->msgq.rptr) = rptr;
-	return msg;
+	return buf;
+}
+
+static void *
+r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
+{
+	struct r535_gsp_msg_info info = {0};
+	void *buf;
+
+	buf = kvmalloc(gsp_rpc_len, GFP_KERNEL);
+	if (!buf)
+		return ERR_PTR(-ENOMEM);
+
+	info.gsp_rpc_buf = buf;
+	info.retries = retries;
+	info.gsp_rpc_len = gsp_rpc_len;
+
+	buf = r535_gsp_msgq_recv_one_elem(gsp, &info);
+	if (IS_ERR(buf)) {
+		kvfree(info.gsp_rpc_buf);
+		info.gsp_rpc_buf = NULL;
+	}
+
+	return buf;
 }
 
 static int
-- 
2.34.1

Zhi Wang

2024-Oct-31 08:52 UTC

head link

[PATCH v3 14/15] nvkm: support handling the return of large GSP message

The max GSP message element size is 16 pages (including the headers). To
send a message larger than 16 pages, nvkm should split it into multiple
and send them accordingly. The first element has the expected function
number, while the rest are sent with function number as
NV_VGPU_MSG_FUNCTION_CONTINUATION_RECORD. GSP consumes the elements from
the cmdq and always writes the result back to the msgq. The result is also
formed as split elements.

However, nvkm is able to split the large GSP message and send them, but
totally not aware of handling the return of the large GSP message, which
are the split elements in the msgq. Thus, it keeps dumping the unknown RPC
messages from msgq, which is actually CONTINUATION_RECORD message,
discard them unexpectly. Thus, the caller will not be able to consume
the result from GSP.

Introduce the handling of the return of large GSP message on the msgq path.
Slightly re-factor the low-level part of msg receiving routines. Merge the
split elements back into a large element before handling it to the upper
level. Thus, the upper-level of GSP RPC APIs don't need to be heavily
changed.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 111 +++++++++++++++---
 1 file changed, 92 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 08a74d8bd06f..41da8c72d618 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -104,6 +104,17 @@
  * |        params          |
  * +------------------------+
  *
+ * The max size of a message queue element is 16 pages (including the
+ * headers). When a GSP message to be sent is larger than 16 pages, the
+ * message should be split into multiple elements and sent accordingly.
+ *
+ * In the bunch of the split elements, the first element has the expected
+ * function number, while the rest of the elements are sent with the
+ * function number NV_VGPU_MSG_FUNCTION_CONTINUATION_RECORD.
+ *
+ * GSP consumes the elements from the cmdq and always writes the result
+ * back to the msgq. The result is also formed as split elements.
+ *
  * decoders:
  *
  * - gsp_msg(msg): GSP message element (element header + GSP RPC header +
@@ -125,6 +136,21 @@ struct r535_gsp_msg {
 	u8  data[];
 };
 
+struct nvfw_gsp_rpc {
+	u32 header_version;
+	u32 signature;
+	u32 length;
+	u32 function;
+	u32 rpc_result;
+	u32 rpc_result_private;
+	u32 sequence;
+	union {
+		u32 spare;
+		u32 cpuRmGfid;
+	};
+	u8  data[];
+};
+
 #define GSP_MSG_HDR_SIZE offsetof(struct r535_gsp_msg, data)
 
 #define to_gsp_hdr(p, header) \
@@ -203,8 +229,12 @@ struct r535_gsp_msg_info {
 	int *retries;
 	u32 gsp_rpc_len;
 	void *gsp_rpc_buf;
+	bool continuation;
 };
 
+static void
+r535_gsp_msg_dump(struct nvkm_gsp *gsp, struct nvfw_gsp_rpc *msg, int lvl);
+
 static void *
 r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
 			    struct r535_gsp_msg_info *info)
@@ -222,11 +252,28 @@ r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
 		return ERR_PTR(ret);
 
 	mqe = r535_gsp_msgq_get_entry(gsp);
+
+	if (info->continuation) {
+		struct nvfw_gsp_rpc *rpc = (struct nvfw_gsp_rpc *)mqe->data;
+
+		if (rpc->function != NV_VGPU_MSG_FUNCTION_CONTINUATION_RECORD) {
+			nvkm_error(&gsp->subdev,
+				   "Not a continuation of a large RPC\n");
+			r535_gsp_msg_dump(gsp, rpc, NV_DBG_ERROR);
+			return ERR_PTR(-EIO);
+		}
+	}
+
 	size = ALIGN(expected + GSP_MSG_HDR_SIZE, GSP_PAGE_SIZE);
 
 	len = ((gsp->msgq.cnt - rptr) * GSP_PAGE_SIZE) - sizeof(*mqe);
 	len = min_t(u32, expected, len);
-	memcpy(buf, mqe->data, len);
+
+	if (info->continuation)
+		memcpy(buf, mqe->data + sizeof(struct nvfw_gsp_rpc),
+		       len - sizeof(struct nvfw_gsp_rpc));
+	else
+		memcpy(buf, mqe->data, len);
 
 	expected -= len;
 
@@ -245,16 +292,26 @@ r535_gsp_msgq_recv_one_elem(struct nvkm_gsp *gsp,
 static void *
 r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len, int *retries)
 {
+	struct r535_gsp_msg *mqe;
+	const u32 max_rpc_size = GSP_MSG_MAX_SIZE - sizeof(*mqe);
+	struct nvfw_gsp_rpc *rpc;
 	struct r535_gsp_msg_info info = {0};
+	u32 expected = gsp_rpc_len;
 	void *buf;
 
-	buf = kvmalloc(gsp_rpc_len, GFP_KERNEL);
+	mqe = r535_gsp_msgq_get_entry(gsp);
+	rpc = (struct nvfw_gsp_rpc *)mqe->data;
+
+	if (WARN_ON(rpc->length > max_rpc_size))
+		return NULL;
+
+	buf = kvmalloc(max_t(u32, rpc->length, expected), GFP_KERNEL);
 	if (!buf)
 		return ERR_PTR(-ENOMEM);
 
 	info.gsp_rpc_buf = buf;
 	info.retries = retries;
-	info.gsp_rpc_len = gsp_rpc_len;
+	info.gsp_rpc_len = rpc->length;
 
 	buf = r535_gsp_msgq_recv_one_elem(gsp, &info);
 	if (IS_ERR(buf)) {
@@ -262,6 +319,37 @@ r535_gsp_msgq_recv(struct nvkm_gsp *gsp, u32 gsp_rpc_len,
int *retries)
 		info.gsp_rpc_buf = NULL;
 	}
 
+	if (expected <= max_rpc_size)
+		return buf;
+
+	info.gsp_rpc_buf += info.gsp_rpc_len;
+	expected -= info.gsp_rpc_len;
+
+	while (expected) {
+		u32 size;
+
+		rpc = r535_gsp_msgq_peek(gsp, sizeof(*rpc), info.retries);
+		if (IS_ERR_OR_NULL(rpc)) {
+			kfree(buf);
+			return rpc;
+		}
+
+		info.gsp_rpc_len = rpc->length;
+		info.continuation = true;
+
+		rpc = r535_gsp_msgq_recv_one_elem(gsp, &info);
+		if (IS_ERR_OR_NULL(rpc)) {
+			kfree(buf);
+			return rpc;
+		}
+
+		size = info.gsp_rpc_len - sizeof(*rpc);
+		expected -= size;
+		info.gsp_rpc_buf += size;
+	}
+
+	rpc = buf;
+	rpc->length = gsp_rpc_len;
 	return buf;
 }
 
@@ -350,21 +438,6 @@ r535_gsp_cmdq_get(struct nvkm_gsp *gsp, u32 gsp_rpc_len)
 	return msg->data;
 }
 
-struct nvfw_gsp_rpc {
-	u32 header_version;
-	u32 signature;
-	u32 length;
-	u32 function;
-	u32 rpc_result;
-	u32 rpc_result_private;
-	u32 sequence;
-	union {
-		u32 spare;
-		u32 cpuRmGfid;
-	};
-	u8  data[];
-};
-
 static void
 r535_gsp_msg_done(struct nvkm_gsp *gsp, struct nvfw_gsp_rpc *msg)
 {
@@ -396,7 +469,7 @@ r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32
gsp_rpc_len)
 	if (IS_ERR_OR_NULL(rpc))
 		return rpc;
 
-	rpc = r535_gsp_msgq_recv(gsp, rpc->length, &retries);
+	rpc = r535_gsp_msgq_recv(gsp, gsp_rpc_len, &retries);
 	if (IS_ERR_OR_NULL(rpc))
 		return rpc;
 
-- 
2.34.1

Zhi Wang

2024-Oct-31 08:52 UTC

head link

[PATCH v3 15/15] nvkm: consume the return of large GSP message

As the GSP message recv path is able to handle the return of large GSP
message, consume the return of large GSP message in the sending path.

Signed-off-by: Zhi Wang <zhiw at nvidia.com>
---
 .../gpu/drm/nouveau/nvkm/subdev/gsp/r535.c    | 35 ++++++++++---------
 1 file changed, 19 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
index 41da8c72d618..5a9c7ffa635c 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/gsp/r535.c
@@ -483,10 +483,9 @@ r535_gsp_msg_recv(struct nvkm_gsp *gsp, int fn, u32
gsp_rpc_len)
 
 	if (fn && rpc->function == fn) {
 		if (gsp_rpc_len) {
-			if (rpc->length < sizeof(*rpc) + gsp_rpc_len) {
-				nvkm_error(subdev, "rpc len %d < %zd\n",
-					   rpc->length, sizeof(*rpc) +
-					   gsp_rpc_len);
+			if (rpc->length < gsp_rpc_len) {
+				nvkm_error(subdev, "rpc len %d < %d\n",
+					   rpc->length, gsp_rpc_len);
 				r535_gsp_msg_dump(gsp, rpc, NV_DBG_ERROR);
 				r535_gsp_msg_done(gsp, rpc);
 				return ERR_PTR(-EIO);
@@ -932,6 +931,7 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload, bool
wait,
 	mutex_lock(&gsp->cmdq.mutex);
 	if (payload_size > max_payload_size) {
 		const u32 fn = rpc->function;
+		u32 remain_payload_size = payload_size;
 
 		/* Adjust length, and send initial RPC. */
 		rpc->length = sizeof(*rpc) + max_payload_size;
@@ -942,11 +942,12 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload,
bool wait,
 			goto done;
 
 		payload += max_payload_size;
-		payload_size -= max_payload_size;
+		remain_payload_size -= max_payload_size;
 
 		/* Remaining chunks sent as CONTINUATION_RECORD RPCs. */
-		while (payload_size) {
-			u32 size = min(payload_size, max_payload_size);
+		while (remain_payload_size) {
+			u32 size = min(remain_payload_size,
+				       max_payload_size);
 			void *next;
 
 			next = r535_gsp_rpc_get(gsp, NV_VGPU_MSG_FUNCTION_CONTINUATION_RECORD,
size);
@@ -962,19 +963,21 @@ r535_gsp_rpc_push(struct nvkm_gsp *gsp, void *payload,
bool wait,
 				goto done;
 
 			payload += size;
-			payload_size -= size;
+			remain_payload_size -= size;
 		}
 
 		/* Wait for reply. */
-		if (wait) {
-			rpc = r535_gsp_msg_recv(gsp, fn, gsp_rpc_len);
-			if (!IS_ERR_OR_NULL(rpc))
+		rpc = r535_gsp_msg_recv(gsp, fn, payload_size +
+					sizeof(*rpc));
+		if (!IS_ERR_OR_NULL(rpc)) {
+			if (wait)
 				repv = rpc->data;
-			else
-				repv = rpc;
-		} else {
-			repv = NULL;
-		}
+			else {
+				nvkm_gsp_rpc_done(gsp, rpc);
+				repv = NULL;
+			}
+		} else
+			repv = wait ? rpc : NULL;
 	} else {
 		repv = r535_gsp_rpc_send(gsp, payload, wait, gsp_rpc_len);
 	}
-- 
2.34.1

Danilo Krummrich

2024-Oct-31 09:05 UTC

head link

[PATCH v3 00/15] NVKM GSP RPC kernel docs, cleanups and fixes

Hi Zhi,

On 10/31/24 9:52 AM, Zhi Wang wrote:> Hi folks:
> 
> Here is the leftover of the previous spin of NVKM GSP RPC fixes, which
> is handling the return of large GSP message. PATCH 1 and 2 in the previous
> spin were merged [1], and this spin is based on top of PATCH 1 and PATCH 2
> in the previous spin.
> 
> Besides the support of the large GSP message, kernel doc and many cleanups
> are introduced according to the comments in the previous spin [2].
Thanks for this work! I'll go through the patches in the next days.

- Danilo

Nouveau - Oct 2024 - [PATCH v3 03/15] nvkm: rename "argv" to what it represnts on the GSP message send path

[PATCH v3 00/15] NVKM GSP RPC kernel docs, cleanups and fixes

[PATCH v3 01/15] nvkm: add a kernel doc to introduce the GSP RPC

[PATCH v3 02/15] nvkm: rename "repc" to "gsp_rpc_len" on the GSP message recv path

[PATCH v3 03/15] nvkm: rename "argv" to what it represnts on the GSP message send path

[PATCH v3 04/15] nvkm: remove unused param repc in *rm_alloc_push()

[PATCH v3 05/15] nvkm: rename "argv" to what it represents in *rm_{alloc, ctrl}_*()

[PATCH v3 06/15] nvkm: rename "argc" to what it represents in GSP RPC routines

[PATCH v3 07/15] nvkm: fix the broken marco GSP_MSG_MAX_SIZE

[PATCH v3 08/15] nvkm: remove the magic number in r535_gsp_rpc_push()

[PATCH v3 09/15] nvkm: refine the variable names in r535_gsp_rpc_push()

[PATCH v3 10/15] nvkm: refine the variable names in r535_gsp_msg_recv()

[PATCH v3 11/15] nvkm: rename the variable "cmd" to "msg" in r535_gsp_cmdq_{get, push}()

[PATCH v3 12/15] nvkm: factor out r535_gsp_msgq_peek()

[PATCH v3 13/15] nvkm: factor out r535_gsp_msgq_recv_one_elem()

[PATCH v3 14/15] nvkm: support handling the return of large GSP message

[PATCH v3 15/15] nvkm: consume the return of large GSP message

[PATCH v3 00/15] NVKM GSP RPC kernel docs, cleanups and fixes

[PATCH v3 05/15] nvkm: rename "argv" to what it represents in rm_{alloc, ctrl}_()