All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/4] misc: fastrpc: Add polling mode support
@ 2026-02-15 18:21 Ekansh Gupta
  2026-02-15 18:21 ` [PATCH v6 1/4] misc: fastrpc: Move fdlist to invoke context structure Ekansh Gupta
                   ` (3 more replies)
  0 siblings, 4 replies; 17+ messages in thread
From: Ekansh Gupta @ 2026-02-15 18:21 UTC (permalink / raw)
  To: srini, linux-arm-msm
  Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd,
	dmitry.baryshkov

This patch series adds polling mode feature that have been missing in
upstream FastRPC driver.

- Add changes to move fdlist to ctx structure to avoid code duplicacy.
- Update context mask to support polling mode.
- Add changes to support polling feature.

Userspace change: https://github.com/qualcomm/fastrpc/pull/258
Patch [v5]: https://lore.kernel.org/all/20251230062831.1195116-1-ekansh.gupta@oss.qualcomm.com/

Changes in v5:
  - Fixed poll memory calculation.
  - Added few formatting changes.

Changes in v5:
  - Add more details in commit text.

Changes in v4:
  - Replace hardcoded ctxid mask with GENMASK.
  - Fixed commit text.

Changes in v3:
  - Resolve compilation warning.

Changes in v2:
  - Added comments and fixed commit text.
  - Defined context id position as a macro.
  - Added new IOCTL to control polling mode as always enabling
    it might cause excess power consumption.
  - Cleaned up polling mode implementation.

Ekansh Gupta (4):
  misc: fastrpc: Move fdlist to invoke context structure
  misc: fastrpc: Replace hardcoded ctxid mask with GENMASK
  misc: fastrpc: Expand context ID mask for DSP polling mode support
  misc: fastrpc: Add polling mode support for fastRPC driver

 drivers/misc/fastrpc.c      | 166 +++++++++++++++++++++++++++++++-----
 include/uapi/misc/fastrpc.h |  10 +++
 2 files changed, 154 insertions(+), 22 deletions(-)

-- 
2.34.1


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

* [PATCH v6 1/4] misc: fastrpc: Move fdlist to invoke context structure
  2026-02-15 18:21 [PATCH v6 0/4] misc: fastrpc: Add polling mode support Ekansh Gupta
@ 2026-02-15 18:21 ` Ekansh Gupta
  2026-02-16  3:56   ` Bjorn Andersson
  2026-02-15 18:21 ` [PATCH v6 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK Ekansh Gupta
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 17+ messages in thread
From: Ekansh Gupta @ 2026-02-15 18:21 UTC (permalink / raw)
  To: srini, linux-arm-msm
  Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd,
	dmitry.baryshkov

The fdlist is currently part of the meta buffer, computed during
put_args. This leads to code duplication when preparing and reading
critical meta buffer contents used by the FastRPC driver.

Move fdlist to the invoke context structure to improve maintainability
and reduce redundancy. This centralizes its handling and simplifies
meta buffer preparation and reading logic.

Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 4f5a79c50f58..ce397c687161 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -233,6 +233,7 @@ struct fastrpc_invoke_ctx {
 	int pid;
 	int client_id;
 	u32 sc;
+	u64 *fdlist;
 	u32 *crc;
 	u64 ctxid;
 	u64 msg_sz;
@@ -1018,6 +1019,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
 	rpra = ctx->buf->virt;
 	list = fastrpc_invoke_buf_start(rpra, ctx->nscalars);
 	pages = fastrpc_phy_page_start(list, ctx->nscalars);
+	ctx->fdlist = (u64 *)(pages + ctx->nscalars);
 	args = (uintptr_t)ctx->buf->virt + metalen;
 	rlen = pkt_size - metalen;
 	ctx->rpra = rpra;
@@ -1120,18 +1122,10 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx,
 	union fastrpc_remote_arg *rpra = ctx->rpra;
 	struct fastrpc_user *fl = ctx->fl;
 	struct fastrpc_map *mmap = NULL;
-	struct fastrpc_invoke_buf *list;
-	struct fastrpc_phy_page *pages;
-	u64 *fdlist;
-	int i, inbufs, outbufs, handles;
+	int i, inbufs;
 	int ret = 0;
 
 	inbufs = REMOTE_SCALARS_INBUFS(ctx->sc);
-	outbufs = REMOTE_SCALARS_OUTBUFS(ctx->sc);
-	handles = REMOTE_SCALARS_INHANDLES(ctx->sc) + REMOTE_SCALARS_OUTHANDLES(ctx->sc);
-	list = fastrpc_invoke_buf_start(rpra, ctx->nscalars);
-	pages = fastrpc_phy_page_start(list, ctx->nscalars);
-	fdlist = (uint64_t *)(pages + inbufs + outbufs + handles);
 
 	for (i = inbufs; i < ctx->nbufs; ++i) {
 		if (!ctx->maps[i]) {
@@ -1153,9 +1147,9 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx,
 cleanup_fdlist:
 	/* Clean up fdlist which is updated by DSP */
 	for (i = 0; i < FASTRPC_MAX_FDLIST; i++) {
-		if (!fdlist[i])
+		if (!ctx->fdlist[i])
 			break;
-		if (!fastrpc_map_lookup(fl, (int)fdlist[i], &mmap))
+		if (!fastrpc_map_lookup(fl, (int)ctx->fdlist[i], &mmap))
 			fastrpc_map_put(mmap);
 	}
 
-- 
2.34.1


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

* [PATCH v6 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK
  2026-02-15 18:21 [PATCH v6 0/4] misc: fastrpc: Add polling mode support Ekansh Gupta
  2026-02-15 18:21 ` [PATCH v6 1/4] misc: fastrpc: Move fdlist to invoke context structure Ekansh Gupta
@ 2026-02-15 18:21 ` Ekansh Gupta
  2026-02-16  2:39   ` kernel test robot
  2026-02-16  3:13   ` kernel test robot
  2026-02-15 18:21 ` [PATCH v6 3/4] misc: fastrpc: Expand context ID mask for DSP polling mode support Ekansh Gupta
  2026-02-15 18:21 ` [PATCH v6 4/4] misc: fastrpc: Add polling mode support for fastRPC driver Ekansh Gupta
  3 siblings, 2 replies; 17+ messages in thread
From: Ekansh Gupta @ 2026-02-15 18:21 UTC (permalink / raw)
  To: srini, linux-arm-msm
  Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd,
	dmitry.baryshkov, Konrad Dybcio

Replace the hardcoded context ID mask (0xFF0) with GENMASK(11, 4) to
improve readability and follow kernel bitfield conventions. Use
FIELD_PREP and FIELD_GET instead of manual shifts for setting and
extracting ctxid values.

Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com>
Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index ce397c687161..0d8d89a2e220 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -37,7 +37,7 @@
 #define FASTRPC_CTX_MAX (256)
 #define FASTRPC_INIT_HANDLE	1
 #define FASTRPC_DSP_UTILITIES_HANDLE	2
-#define FASTRPC_CTXID_MASK (0xFF0)
+#define FASTRPC_CTXID_MASK GENMASK(11, 4)
 #define INIT_FILELEN_MAX (2 * 1024 * 1024)
 #define INIT_FILE_NAMELEN_MAX (128)
 #define FASTRPC_DEVICE_NAME	"fastrpc"
@@ -515,7 +515,7 @@ static void fastrpc_context_free(struct kref *ref)
 		fastrpc_buf_free(ctx->buf);
 
 	spin_lock_irqsave(&cctx->lock, flags);
-	idr_remove(&cctx->ctx_idr, ctx->ctxid >> 4);
+	idr_remove(&cctx->ctx_idr, FIELD_GET(FASTRPC_CTXID_MASK, ctx->ctxid));
 	spin_unlock_irqrestore(&cctx->lock, flags);
 
 	kfree(ctx->maps);
@@ -651,7 +651,7 @@ static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
 		spin_unlock_irqrestore(&cctx->lock, flags);
 		goto err_idr;
 	}
-	ctx->ctxid = ret << 4;
+	ctx->ctxid = FIELD_PREP(FASTRPC_CTXID_MASK, ret);
 	spin_unlock_irqrestore(&cctx->lock, flags);
 
 	kref_init(&ctx->refcount);
@@ -2506,7 +2506,7 @@ static int fastrpc_rpmsg_callback(struct rpmsg_device *rpdev, void *data,
 	if (len < sizeof(*rsp))
 		return -EINVAL;
 
-	ctxid = ((rsp->ctx & FASTRPC_CTXID_MASK) >> 4);
+	ctxid = FIELD_GET(FASTRPC_CTXID_MASK, rsp->ctx);
 
 	spin_lock_irqsave(&cctx->lock, flags);
 	ctx = idr_find(&cctx->ctx_idr, ctxid);
-- 
2.34.1


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

* [PATCH v6 3/4] misc: fastrpc: Expand context ID mask for DSP polling mode support
  2026-02-15 18:21 [PATCH v6 0/4] misc: fastrpc: Add polling mode support Ekansh Gupta
  2026-02-15 18:21 ` [PATCH v6 1/4] misc: fastrpc: Move fdlist to invoke context structure Ekansh Gupta
  2026-02-15 18:21 ` [PATCH v6 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK Ekansh Gupta
@ 2026-02-15 18:21 ` Ekansh Gupta
  2026-02-15 18:21 ` [PATCH v6 4/4] misc: fastrpc: Add polling mode support for fastRPC driver Ekansh Gupta
  3 siblings, 0 replies; 17+ messages in thread
From: Ekansh Gupta @ 2026-02-15 18:21 UTC (permalink / raw)
  To: srini, linux-arm-msm
  Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd,
	dmitry.baryshkov

Current FastRPC context uses a 12-bit mask:
  [ID(8 bits)][PD type(4 bits)] = GENMASK(11, 4)

This works for normal calls but fails for DSP polling mode.
Polling mode expects a 16-bit layout:
  [15:8] = context ID (8 bits)
  [7:5]  = reserved
  [4]    = async mode bit
  [3:0]  = PD type (4 bits)

If async bit (bit 4) is set, DSP disables polling. With current
mask, odd IDs can set this bit, causing DSP to skip poll updates.

Update FASTRPC_CTXID_MASK to GENMASK(15, 8) so IDs occupy upper
byte and lower byte is left for DSP flags and PD type.

Reserved bits remain unused. This change is compatible with
polling mode and does not break non-polling behavior.

Bit layout:
  [15:8] = CCCCCCCC (context ID)
  [7:5]  = xxx (reserved)
  [4]    = A (async mode)
  [3:0]  = PPPP (PD type)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 0d8d89a2e220..e935ae3776b4 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -37,7 +37,7 @@
 #define FASTRPC_CTX_MAX (256)
 #define FASTRPC_INIT_HANDLE	1
 #define FASTRPC_DSP_UTILITIES_HANDLE	2
-#define FASTRPC_CTXID_MASK GENMASK(11, 4)
+#define FASTRPC_CTXID_MASK GENMASK(15, 8)
 #define INIT_FILELEN_MAX (2 * 1024 * 1024)
 #define INIT_FILE_NAMELEN_MAX (128)
 #define FASTRPC_DEVICE_NAME	"fastrpc"
-- 
2.34.1


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

* [PATCH v6 4/4] misc: fastrpc: Add polling mode support for fastRPC driver
  2026-02-15 18:21 [PATCH v6 0/4] misc: fastrpc: Add polling mode support Ekansh Gupta
                   ` (2 preceding siblings ...)
  2026-02-15 18:21 ` [PATCH v6 3/4] misc: fastrpc: Expand context ID mask for DSP polling mode support Ekansh Gupta
@ 2026-02-15 18:21 ` Ekansh Gupta
  2026-02-16  3:21   ` Bjorn Andersson
  2026-02-19  7:36   ` Dmitry Baryshkov
  3 siblings, 2 replies; 17+ messages in thread
From: Ekansh Gupta @ 2026-02-15 18:21 UTC (permalink / raw)
  To: srini, linux-arm-msm
  Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd,
	dmitry.baryshkov

For any remote call to DSP, after sending an invocation message,
fastRPC driver waits for glink response and during this time the
CPU can go into low power modes. This adds latency to overall fastrpc
call as CPU wakeup and scheduling latencies are included. Add polling
mode support with which fastRPC driver will poll continuously on a
memory after sending a message to remote subsystem which will eliminate
CPU wakeup and scheduling latencies and reduce fastRPC overhead. Poll
mode can be enabled by user by using FASTRPC_IOCTL_SET_OPTION ioctl
request with FASTRPC_POLL_MODE request id.

Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
---
 drivers/misc/fastrpc.c      | 142 ++++++++++++++++++++++++++++++++++--
 include/uapi/misc/fastrpc.h |  10 +++
 2 files changed, 145 insertions(+), 7 deletions(-)

diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index e935ae3776b4..c1e67dbacf2c 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -23,6 +23,8 @@
 #include <uapi/misc/fastrpc.h>
 #include <linux/of_reserved_mem.h>
 #include <linux/bits.h>
+#include <linux/compiler.h>
+#include <linux/iopoll.h>
 
 #define ADSP_DOMAIN_ID (0)
 #define MDSP_DOMAIN_ID (1)
@@ -37,6 +39,7 @@
 #define FASTRPC_CTX_MAX (256)
 #define FASTRPC_INIT_HANDLE	1
 #define FASTRPC_DSP_UTILITIES_HANDLE	2
+#define FASTRPC_MAX_STATIC_HANDLE (20)
 #define FASTRPC_CTXID_MASK GENMASK(15, 8)
 #define INIT_FILELEN_MAX (2 * 1024 * 1024)
 #define INIT_FILE_NAMELEN_MAX (128)
@@ -105,6 +108,12 @@
 
 #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
 
+/* Poll response number from remote processor for call completion */
+#define FASTRPC_POLL_RESPONSE (0xdecaf)
+
+/* Polling mode timeout limit */
+#define FASTRPC_POLL_MAX_TIMEOUT_US (10000)
+
 struct fastrpc_phy_page {
 	dma_addr_t addr;	/* dma address */
 	u64 size;		/* size of contiguous region */
@@ -235,8 +244,14 @@ struct fastrpc_invoke_ctx {
 	u32 sc;
 	u64 *fdlist;
 	u32 *crc;
+	/* Poll memory that DSP updates */
+	u32 *poll;
 	u64 ctxid;
 	u64 msg_sz;
+	/* work done status flag */
+	bool is_work_done;
+	/* process updates poll memory instead of glink response */
+	bool is_polled;
 	struct kref refcount;
 	struct list_head node; /* list of ctxs */
 	struct completion work;
@@ -307,6 +322,8 @@ struct fastrpc_user {
 	int client_id;
 	int pd;
 	bool is_secure_dev;
+	/* Flags poll mode state */
+	bool poll_mode;
 	/* Lock for lists */
 	spinlock_t lock;
 	/* lock for allocations */
@@ -924,7 +941,8 @@ static int fastrpc_get_meta_size(struct fastrpc_invoke_ctx *ctx)
 		sizeof(struct fastrpc_invoke_buf) +
 		sizeof(struct fastrpc_phy_page)) * ctx->nscalars +
 		sizeof(u64) * FASTRPC_MAX_FDLIST +
-		sizeof(u32) * FASTRPC_MAX_CRCLIST;
+		sizeof(u32) * FASTRPC_MAX_CRCLIST +
+		sizeof(u32);
 
 	return size;
 }
@@ -1020,6 +1038,9 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
 	list = fastrpc_invoke_buf_start(rpra, ctx->nscalars);
 	pages = fastrpc_phy_page_start(list, ctx->nscalars);
 	ctx->fdlist = (u64 *)(pages + ctx->nscalars);
+	ctx->poll = (u32 *)((uintptr_t)ctx->fdlist + sizeof(u64) * FASTRPC_MAX_FDLIST +
+			     sizeof(u32) * FASTRPC_MAX_CRCLIST);
+
 	args = (uintptr_t)ctx->buf->virt + metalen;
 	rlen = pkt_size - metalen;
 	ctx->rpra = rpra;
@@ -1188,6 +1209,75 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
 
 }
 
+static inline u32 fastrpc_poll_op(void *p)
+{
+	struct fastrpc_invoke_ctx *ctx = p;
+
+	dma_rmb();
+	return READ_ONCE(*ctx->poll);
+}
+
+static int poll_for_remote_response(struct fastrpc_invoke_ctx *ctx)
+{
+	u32 val;
+	int ret;
+
+	/*
+	 * Poll until DSP writes FASTRPC_POLL_RESPONSE into *ctx->poll
+	 * or until another path marks the work done.
+	 */
+	ret = read_poll_timeout_atomic(fastrpc_poll_op, val,
+				       (val == FASTRPC_POLL_RESPONSE) ||
+				       ctx->is_work_done, 1,
+				       FASTRPC_POLL_MAX_TIMEOUT_US, false, ctx);
+
+	if (!ret && val == FASTRPC_POLL_RESPONSE) {
+		ctx->is_work_done = true;
+		ctx->retval = 0;
+	}
+
+	if (ret == -ETIMEDOUT)
+		ret = -EIO;
+
+	return ret;
+}
+
+static inline int fastrpc_wait_for_response(struct fastrpc_invoke_ctx *ctx,
+					    u32 kernel)
+{
+	int err = 0;
+
+	if (kernel) {
+		if (!wait_for_completion_timeout(&ctx->work, 10 * HZ))
+			err = -ETIMEDOUT;
+	} else {
+		err = wait_for_completion_interruptible(&ctx->work);
+	}
+
+	return err;
+}
+
+static int fastrpc_wait_for_completion(struct fastrpc_invoke_ctx *ctx,
+				       u32 kernel)
+{
+	int err;
+
+	do {
+		if (ctx->is_polled) {
+			err = poll_for_remote_response(ctx);
+			/* If polling timed out, move to normal response mode */
+			if (err)
+				ctx->is_polled = false;
+		} else {
+			err = fastrpc_wait_for_response(ctx, kernel);
+			if (err)
+				return err;
+		}
+	} while (!ctx->is_work_done);
+
+	return err;
+}
+
 static int fastrpc_internal_invoke(struct fastrpc_user *fl,  u32 kernel,
 				   u32 handle, u32 sc,
 				   struct fastrpc_invoke_args *args)
@@ -1223,16 +1313,26 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl,  u32 kernel,
 	if (err)
 		goto bail;
 
-	if (kernel) {
-		if (!wait_for_completion_timeout(&ctx->work, 10 * HZ))
-			err = -ETIMEDOUT;
-	} else {
-		err = wait_for_completion_interruptible(&ctx->work);
-	}
+	/*
+	 * Set message context as polled if the call is for a user PD
+	 * dynamic module and user has enabled poll mode.
+	 */
+	if (handle > FASTRPC_MAX_STATIC_HANDLE && fl->pd == USER_PD &&
+	    fl->poll_mode)
+		ctx->is_polled = true;
+
+	err = fastrpc_wait_for_completion(ctx, kernel);
 
 	if (err)
 		goto bail;
 
+	if (!ctx->is_work_done) {
+		err = -ETIMEDOUT;
+		dev_dbg(fl->sctx->dev, "Invalid workdone state for handle 0x%x, sc 0x%x\n",
+			handle, sc);
+		goto bail;
+	}
+
 	/* make sure that all memory writes by DSP are seen by CPU */
 	dma_rmb();
 	/* populate all the output buffers with results */
@@ -1812,6 +1912,30 @@ static int fastrpc_get_info_from_kernel(struct fastrpc_ioctl_capability *cap,
 	return 0;
 }
 
+static int fastrpc_set_option(struct fastrpc_user *fl, char __user *argp)
+{
+	struct fastrpc_ioctl_set_option opt = {0};
+	int i;
+
+	if (copy_from_user(&opt, argp, sizeof(opt)))
+		return -EFAULT;
+
+	for (i = 0; i < ARRAY_SIZE(opt.reserved); i++) {
+		if (opt.reserved[i] != 0)
+			return -EINVAL;
+	}
+
+	if (opt.req != FASTRPC_POLL_MODE)
+		return -EINVAL;
+
+	if (opt.value)
+		fl->poll_mode = true;
+	else
+		fl->poll_mode = false;
+
+	return 0;
+}
+
 static int fastrpc_get_dsp_info(struct fastrpc_user *fl, char __user *argp)
 {
 	struct fastrpc_ioctl_capability cap = {0};
@@ -2167,6 +2291,9 @@ static long fastrpc_device_ioctl(struct file *file, unsigned int cmd,
 	case FASTRPC_IOCTL_MEM_UNMAP:
 		err = fastrpc_req_mem_unmap(fl, argp);
 		break;
+	case FASTRPC_IOCTL_SET_OPTION:
+		err = fastrpc_set_option(fl, argp);
+		break;
 	case FASTRPC_IOCTL_GET_DSP_INFO:
 		err = fastrpc_get_dsp_info(fl, argp);
 		break;
@@ -2518,6 +2645,7 @@ static int fastrpc_rpmsg_callback(struct rpmsg_device *rpdev, void *data,
 	}
 
 	ctx->retval = rsp->retval;
+	ctx->is_work_done = true;
 	complete(&ctx->work);
 
 	/*
diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
index c6e2925f47e6..c37e24a764ae 100644
--- a/include/uapi/misc/fastrpc.h
+++ b/include/uapi/misc/fastrpc.h
@@ -16,6 +16,7 @@
 #define FASTRPC_IOCTL_INIT_CREATE_STATIC _IOWR('R', 9, struct fastrpc_init_create_static)
 #define FASTRPC_IOCTL_MEM_MAP		_IOWR('R', 10, struct fastrpc_mem_map)
 #define FASTRPC_IOCTL_MEM_UNMAP		_IOWR('R', 11, struct fastrpc_mem_unmap)
+#define FASTRPC_IOCTL_SET_OPTION	_IOWR('R', 12, struct fastrpc_ioctl_set_option)
 #define FASTRPC_IOCTL_GET_DSP_INFO	_IOWR('R', 13, struct fastrpc_ioctl_capability)
 
 /**
@@ -67,6 +68,9 @@ enum fastrpc_proc_attr {
 /* Fastrpc attribute for memory protection of buffers */
 #define FASTRPC_ATTR_SECUREMAP	(1)
 
+/* Set option request ID to enable poll mode */
+#define FASTRPC_POLL_MODE	(1)
+
 struct fastrpc_invoke_args {
 	__u64 ptr;
 	__u64 length;
@@ -133,6 +137,12 @@ struct fastrpc_mem_unmap {
 	__s32 reserved[5];
 };
 
+struct fastrpc_ioctl_set_option {
+	__u32 req;	/* request id */
+	__u32 value;	/* value */
+	__s32 reserved[6];
+};
+
 struct fastrpc_ioctl_capability {
 	__u32 unused; /* deprecated, ignored by the kernel */
 	__u32 attribute_id;
-- 
2.34.1


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

* Re: [PATCH v6 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK
  2026-02-15 18:21 ` [PATCH v6 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK Ekansh Gupta
@ 2026-02-16  2:39   ` kernel test robot
  2026-02-16 11:08     ` Konrad Dybcio
  2026-02-16  3:13   ` kernel test robot
  1 sibling, 1 reply; 17+ messages in thread
From: kernel test robot @ 2026-02-16  2:39 UTC (permalink / raw)
  To: Ekansh Gupta, srini, linux-arm-msm
  Cc: llvm, oe-kbuild-all, gregkh, quic_bkumar, linux-kernel,
	quic_chennak, dri-devel, arnd, dmitry.baryshkov, Konrad Dybcio

Hi Ekansh,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.19 next-20260213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ekansh-Gupta/misc-fastrpc-Move-fdlist-to-invoke-context-structure/20260216-022305
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20260215182136.3995111-3-ekansh.gupta%40oss.qualcomm.com
patch subject: [PATCH v6 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK
config: hexagon-randconfig-001-20260216 (https://download.01.org/0day-ci/archive/20260216/202602161009.M2K52X34-lkp@intel.com/config)
compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project e86750b29fa0ff207cd43213d66dabe565417638)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260216/202602161009.M2K52X34-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602161009.M2K52X34-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/misc/fastrpc.c:518:29: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     518 |         idr_remove(&cctx->ctx_idr, FIELD_GET(FASTRPC_CTXID_MASK, ctx->ctxid));
         |                                    ^
>> drivers/misc/fastrpc.c:654:15: error: call to undeclared function 'FIELD_PREP'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
     654 |         ctx->ctxid = FIELD_PREP(FASTRPC_CTXID_MASK, ret);
         |                      ^
   drivers/misc/fastrpc.c:2509:10: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
    2509 |         ctxid = FIELD_GET(FASTRPC_CTXID_MASK, rsp->ctx);
         |                 ^
   3 errors generated.


vim +/FIELD_GET +518 drivers/misc/fastrpc.c

   500	
   501	static void fastrpc_context_free(struct kref *ref)
   502	{
   503		struct fastrpc_invoke_ctx *ctx;
   504		struct fastrpc_channel_ctx *cctx;
   505		unsigned long flags;
   506		int i;
   507	
   508		ctx = container_of(ref, struct fastrpc_invoke_ctx, refcount);
   509		cctx = ctx->cctx;
   510	
   511		for (i = 0; i < ctx->nbufs; i++)
   512			fastrpc_map_put(ctx->maps[i]);
   513	
   514		if (ctx->buf)
   515			fastrpc_buf_free(ctx->buf);
   516	
   517		spin_lock_irqsave(&cctx->lock, flags);
 > 518		idr_remove(&cctx->ctx_idr, FIELD_GET(FASTRPC_CTXID_MASK, ctx->ctxid));
   519		spin_unlock_irqrestore(&cctx->lock, flags);
   520	
   521		kfree(ctx->maps);
   522		kfree(ctx->olaps);
   523		kfree(ctx);
   524	
   525		fastrpc_channel_ctx_put(cctx);
   526	}
   527	
   528	static void fastrpc_context_get(struct fastrpc_invoke_ctx *ctx)
   529	{
   530		kref_get(&ctx->refcount);
   531	}
   532	
   533	static void fastrpc_context_put(struct fastrpc_invoke_ctx *ctx)
   534	{
   535		kref_put(&ctx->refcount, fastrpc_context_free);
   536	}
   537	
   538	static void fastrpc_context_put_wq(struct work_struct *work)
   539	{
   540		struct fastrpc_invoke_ctx *ctx =
   541				container_of(work, struct fastrpc_invoke_ctx, put_work);
   542	
   543		fastrpc_context_put(ctx);
   544	}
   545	
   546	#define CMP(aa, bb) ((aa) == (bb) ? 0 : (aa) < (bb) ? -1 : 1)
   547	static int olaps_cmp(const void *a, const void *b)
   548	{
   549		struct fastrpc_buf_overlap *pa = (struct fastrpc_buf_overlap *)a;
   550		struct fastrpc_buf_overlap *pb = (struct fastrpc_buf_overlap *)b;
   551		/* sort with lowest starting buffer first */
   552		int st = CMP(pa->start, pb->start);
   553		/* sort with highest ending buffer first */
   554		int ed = CMP(pb->end, pa->end);
   555	
   556		return st == 0 ? ed : st;
   557	}
   558	
   559	static void fastrpc_get_buff_overlaps(struct fastrpc_invoke_ctx *ctx)
   560	{
   561		u64 max_end = 0;
   562		int i;
   563	
   564		for (i = 0; i < ctx->nbufs; ++i) {
   565			ctx->olaps[i].start = ctx->args[i].ptr;
   566			ctx->olaps[i].end = ctx->olaps[i].start + ctx->args[i].length;
   567			ctx->olaps[i].raix = i;
   568		}
   569	
   570		sort(ctx->olaps, ctx->nbufs, sizeof(*ctx->olaps), olaps_cmp, NULL);
   571	
   572		for (i = 0; i < ctx->nbufs; ++i) {
   573			/* Falling inside previous range */
   574			if (ctx->olaps[i].start < max_end) {
   575				ctx->olaps[i].mstart = max_end;
   576				ctx->olaps[i].mend = ctx->olaps[i].end;
   577				ctx->olaps[i].offset = max_end - ctx->olaps[i].start;
   578	
   579				if (ctx->olaps[i].end > max_end) {
   580					max_end = ctx->olaps[i].end;
   581				} else {
   582					ctx->olaps[i].mend = 0;
   583					ctx->olaps[i].mstart = 0;
   584				}
   585	
   586			} else  {
   587				ctx->olaps[i].mend = ctx->olaps[i].end;
   588				ctx->olaps[i].mstart = ctx->olaps[i].start;
   589				ctx->olaps[i].offset = 0;
   590				max_end = ctx->olaps[i].end;
   591			}
   592		}
   593	}
   594	
   595	static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
   596				struct fastrpc_user *user, u32 kernel, u32 sc,
   597				struct fastrpc_invoke_args *args)
   598	{
   599		struct fastrpc_channel_ctx *cctx = user->cctx;
   600		struct fastrpc_invoke_ctx *ctx = NULL;
   601		unsigned long flags;
   602		int ret;
   603	
   604		ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
   605		if (!ctx)
   606			return ERR_PTR(-ENOMEM);
   607	
   608		INIT_LIST_HEAD(&ctx->node);
   609		ctx->fl = user;
   610		ctx->nscalars = REMOTE_SCALARS_LENGTH(sc);
   611		ctx->nbufs = REMOTE_SCALARS_INBUFS(sc) +
   612			     REMOTE_SCALARS_OUTBUFS(sc);
   613	
   614		if (ctx->nscalars) {
   615			ctx->maps = kcalloc(ctx->nscalars,
   616					    sizeof(*ctx->maps), GFP_KERNEL);
   617			if (!ctx->maps) {
   618				kfree(ctx);
   619				return ERR_PTR(-ENOMEM);
   620			}
   621			ctx->olaps = kcalloc(ctx->nscalars,
   622					    sizeof(*ctx->olaps), GFP_KERNEL);
   623			if (!ctx->olaps) {
   624				kfree(ctx->maps);
   625				kfree(ctx);
   626				return ERR_PTR(-ENOMEM);
   627			}
   628			ctx->args = args;
   629			fastrpc_get_buff_overlaps(ctx);
   630		}
   631	
   632		/* Released in fastrpc_context_put() */
   633		fastrpc_channel_ctx_get(cctx);
   634	
   635		ctx->sc = sc;
   636		ctx->retval = -1;
   637		ctx->pid = current->pid;
   638		ctx->client_id = user->client_id;
   639		ctx->cctx = cctx;
   640		init_completion(&ctx->work);
   641		INIT_WORK(&ctx->put_work, fastrpc_context_put_wq);
   642	
   643		spin_lock(&user->lock);
   644		list_add_tail(&ctx->node, &user->pending);
   645		spin_unlock(&user->lock);
   646	
   647		spin_lock_irqsave(&cctx->lock, flags);
   648		ret = idr_alloc_cyclic(&cctx->ctx_idr, ctx, 1,
   649				       FASTRPC_CTX_MAX, GFP_ATOMIC);
   650		if (ret < 0) {
   651			spin_unlock_irqrestore(&cctx->lock, flags);
   652			goto err_idr;
   653		}
 > 654		ctx->ctxid = FIELD_PREP(FASTRPC_CTXID_MASK, ret);
   655		spin_unlock_irqrestore(&cctx->lock, flags);
   656	
   657		kref_init(&ctx->refcount);
   658	
   659		return ctx;
   660	err_idr:
   661		spin_lock(&user->lock);
   662		list_del(&ctx->node);
   663		spin_unlock(&user->lock);
   664		fastrpc_channel_ctx_put(cctx);
   665		kfree(ctx->maps);
   666		kfree(ctx->olaps);
   667		kfree(ctx);
   668	
   669		return ERR_PTR(ret);
   670	}
   671	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH v6 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK
  2026-02-15 18:21 ` [PATCH v6 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK Ekansh Gupta
  2026-02-16  2:39   ` kernel test robot
@ 2026-02-16  3:13   ` kernel test robot
  1 sibling, 0 replies; 17+ messages in thread
From: kernel test robot @ 2026-02-16  3:13 UTC (permalink / raw)
  To: Ekansh Gupta, srini, linux-arm-msm
  Cc: oe-kbuild-all, gregkh, quic_bkumar, linux-kernel, quic_chennak,
	dri-devel, arnd, dmitry.baryshkov, Konrad Dybcio

Hi Ekansh,

kernel test robot noticed the following build errors:

[auto build test ERROR on char-misc/char-misc-testing]
[also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.19 next-20260213]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Ekansh-Gupta/misc-fastrpc-Move-fdlist-to-invoke-context-structure/20260216-022305
base:   char-misc/char-misc-testing
patch link:    https://lore.kernel.org/r/20260215182136.3995111-3-ekansh.gupta%40oss.qualcomm.com
patch subject: [PATCH v6 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK
config: sh-allyesconfig (https://download.01.org/0day-ci/archive/20260216/202602161130.5b7COBet-lkp@intel.com/config)
compiler: sh4-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260216/202602161130.5b7COBet-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202602161130.5b7COBet-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/misc/fastrpc.c: In function 'fastrpc_context_free':
>> drivers/misc/fastrpc.c:518:36: error: implicit declaration of function 'FIELD_GET' [-Wimplicit-function-declaration]
     518 |         idr_remove(&cctx->ctx_idr, FIELD_GET(FASTRPC_CTXID_MASK, ctx->ctxid));
         |                                    ^~~~~~~~~
   drivers/misc/fastrpc.c: In function 'fastrpc_context_alloc':
>> drivers/misc/fastrpc.c:654:22: error: implicit declaration of function 'FIELD_PREP' [-Wimplicit-function-declaration]
     654 |         ctx->ctxid = FIELD_PREP(FASTRPC_CTXID_MASK, ret);
         |                      ^~~~~~~~~~


vim +/FIELD_GET +518 drivers/misc/fastrpc.c

   500	
   501	static void fastrpc_context_free(struct kref *ref)
   502	{
   503		struct fastrpc_invoke_ctx *ctx;
   504		struct fastrpc_channel_ctx *cctx;
   505		unsigned long flags;
   506		int i;
   507	
   508		ctx = container_of(ref, struct fastrpc_invoke_ctx, refcount);
   509		cctx = ctx->cctx;
   510	
   511		for (i = 0; i < ctx->nbufs; i++)
   512			fastrpc_map_put(ctx->maps[i]);
   513	
   514		if (ctx->buf)
   515			fastrpc_buf_free(ctx->buf);
   516	
   517		spin_lock_irqsave(&cctx->lock, flags);
 > 518		idr_remove(&cctx->ctx_idr, FIELD_GET(FASTRPC_CTXID_MASK, ctx->ctxid));
   519		spin_unlock_irqrestore(&cctx->lock, flags);
   520	
   521		kfree(ctx->maps);
   522		kfree(ctx->olaps);
   523		kfree(ctx);
   524	
   525		fastrpc_channel_ctx_put(cctx);
   526	}
   527	
   528	static void fastrpc_context_get(struct fastrpc_invoke_ctx *ctx)
   529	{
   530		kref_get(&ctx->refcount);
   531	}
   532	
   533	static void fastrpc_context_put(struct fastrpc_invoke_ctx *ctx)
   534	{
   535		kref_put(&ctx->refcount, fastrpc_context_free);
   536	}
   537	
   538	static void fastrpc_context_put_wq(struct work_struct *work)
   539	{
   540		struct fastrpc_invoke_ctx *ctx =
   541				container_of(work, struct fastrpc_invoke_ctx, put_work);
   542	
   543		fastrpc_context_put(ctx);
   544	}
   545	
   546	#define CMP(aa, bb) ((aa) == (bb) ? 0 : (aa) < (bb) ? -1 : 1)
   547	static int olaps_cmp(const void *a, const void *b)
   548	{
   549		struct fastrpc_buf_overlap *pa = (struct fastrpc_buf_overlap *)a;
   550		struct fastrpc_buf_overlap *pb = (struct fastrpc_buf_overlap *)b;
   551		/* sort with lowest starting buffer first */
   552		int st = CMP(pa->start, pb->start);
   553		/* sort with highest ending buffer first */
   554		int ed = CMP(pb->end, pa->end);
   555	
   556		return st == 0 ? ed : st;
   557	}
   558	
   559	static void fastrpc_get_buff_overlaps(struct fastrpc_invoke_ctx *ctx)
   560	{
   561		u64 max_end = 0;
   562		int i;
   563	
   564		for (i = 0; i < ctx->nbufs; ++i) {
   565			ctx->olaps[i].start = ctx->args[i].ptr;
   566			ctx->olaps[i].end = ctx->olaps[i].start + ctx->args[i].length;
   567			ctx->olaps[i].raix = i;
   568		}
   569	
   570		sort(ctx->olaps, ctx->nbufs, sizeof(*ctx->olaps), olaps_cmp, NULL);
   571	
   572		for (i = 0; i < ctx->nbufs; ++i) {
   573			/* Falling inside previous range */
   574			if (ctx->olaps[i].start < max_end) {
   575				ctx->olaps[i].mstart = max_end;
   576				ctx->olaps[i].mend = ctx->olaps[i].end;
   577				ctx->olaps[i].offset = max_end - ctx->olaps[i].start;
   578	
   579				if (ctx->olaps[i].end > max_end) {
   580					max_end = ctx->olaps[i].end;
   581				} else {
   582					ctx->olaps[i].mend = 0;
   583					ctx->olaps[i].mstart = 0;
   584				}
   585	
   586			} else  {
   587				ctx->olaps[i].mend = ctx->olaps[i].end;
   588				ctx->olaps[i].mstart = ctx->olaps[i].start;
   589				ctx->olaps[i].offset = 0;
   590				max_end = ctx->olaps[i].end;
   591			}
   592		}
   593	}
   594	
   595	static struct fastrpc_invoke_ctx *fastrpc_context_alloc(
   596				struct fastrpc_user *user, u32 kernel, u32 sc,
   597				struct fastrpc_invoke_args *args)
   598	{
   599		struct fastrpc_channel_ctx *cctx = user->cctx;
   600		struct fastrpc_invoke_ctx *ctx = NULL;
   601		unsigned long flags;
   602		int ret;
   603	
   604		ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
   605		if (!ctx)
   606			return ERR_PTR(-ENOMEM);
   607	
   608		INIT_LIST_HEAD(&ctx->node);
   609		ctx->fl = user;
   610		ctx->nscalars = REMOTE_SCALARS_LENGTH(sc);
   611		ctx->nbufs = REMOTE_SCALARS_INBUFS(sc) +
   612			     REMOTE_SCALARS_OUTBUFS(sc);
   613	
   614		if (ctx->nscalars) {
   615			ctx->maps = kcalloc(ctx->nscalars,
   616					    sizeof(*ctx->maps), GFP_KERNEL);
   617			if (!ctx->maps) {
   618				kfree(ctx);
   619				return ERR_PTR(-ENOMEM);
   620			}
   621			ctx->olaps = kcalloc(ctx->nscalars,
   622					    sizeof(*ctx->olaps), GFP_KERNEL);
   623			if (!ctx->olaps) {
   624				kfree(ctx->maps);
   625				kfree(ctx);
   626				return ERR_PTR(-ENOMEM);
   627			}
   628			ctx->args = args;
   629			fastrpc_get_buff_overlaps(ctx);
   630		}
   631	
   632		/* Released in fastrpc_context_put() */
   633		fastrpc_channel_ctx_get(cctx);
   634	
   635		ctx->sc = sc;
   636		ctx->retval = -1;
   637		ctx->pid = current->pid;
   638		ctx->client_id = user->client_id;
   639		ctx->cctx = cctx;
   640		init_completion(&ctx->work);
   641		INIT_WORK(&ctx->put_work, fastrpc_context_put_wq);
   642	
   643		spin_lock(&user->lock);
   644		list_add_tail(&ctx->node, &user->pending);
   645		spin_unlock(&user->lock);
   646	
   647		spin_lock_irqsave(&cctx->lock, flags);
   648		ret = idr_alloc_cyclic(&cctx->ctx_idr, ctx, 1,
   649				       FASTRPC_CTX_MAX, GFP_ATOMIC);
   650		if (ret < 0) {
   651			spin_unlock_irqrestore(&cctx->lock, flags);
   652			goto err_idr;
   653		}
 > 654		ctx->ctxid = FIELD_PREP(FASTRPC_CTXID_MASK, ret);
   655		spin_unlock_irqrestore(&cctx->lock, flags);
   656	
   657		kref_init(&ctx->refcount);
   658	
   659		return ctx;
   660	err_idr:
   661		spin_lock(&user->lock);
   662		list_del(&ctx->node);
   663		spin_unlock(&user->lock);
   664		fastrpc_channel_ctx_put(cctx);
   665		kfree(ctx->maps);
   666		kfree(ctx->olaps);
   667		kfree(ctx);
   668	
   669		return ERR_PTR(ret);
   670	}
   671	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

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

* Re: [PATCH v6 4/4] misc: fastrpc: Add polling mode support for fastRPC driver
  2026-02-15 18:21 ` [PATCH v6 4/4] misc: fastrpc: Add polling mode support for fastRPC driver Ekansh Gupta
@ 2026-02-16  3:21   ` Bjorn Andersson
  2026-02-16  9:06     ` Ekansh Gupta
  2026-02-19  7:36   ` Dmitry Baryshkov
  1 sibling, 1 reply; 17+ messages in thread
From: Bjorn Andersson @ 2026-02-16  3:21 UTC (permalink / raw)
  To: Ekansh Gupta
  Cc: srini, linux-arm-msm, gregkh, quic_bkumar, linux-kernel,
	quic_chennak, dri-devel, arnd, dmitry.baryshkov

On Sun, Feb 15, 2026 at 11:51:35PM +0530, Ekansh Gupta wrote:
> For any remote call to DSP, after sending an invocation message,
> fastRPC driver waits for glink response and during this time the
> CPU can go into low power modes. This adds latency to overall fastrpc
> call as CPU wakeup and scheduling latencies are included. Add polling
> mode support with which fastRPC driver will poll continuously on a
> memory after sending a message to remote subsystem which will eliminate
> CPU wakeup and scheduling latencies and reduce fastRPC overhead. Poll
> mode can be enabled by user by using FASTRPC_IOCTL_SET_OPTION ioctl
> request with FASTRPC_POLL_MODE request id.
> 
> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
> ---
>  drivers/misc/fastrpc.c      | 142 ++++++++++++++++++++++++++++++++++--
>  include/uapi/misc/fastrpc.h |  10 +++
>  2 files changed, 145 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index e935ae3776b4..c1e67dbacf2c 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -23,6 +23,8 @@
>  #include <uapi/misc/fastrpc.h>
>  #include <linux/of_reserved_mem.h>
>  #include <linux/bits.h>
> +#include <linux/compiler.h>
> +#include <linux/iopoll.h>
>  
>  #define ADSP_DOMAIN_ID (0)
>  #define MDSP_DOMAIN_ID (1)
> @@ -37,6 +39,7 @@
>  #define FASTRPC_CTX_MAX (256)
>  #define FASTRPC_INIT_HANDLE	1
>  #define FASTRPC_DSP_UTILITIES_HANDLE	2
> +#define FASTRPC_MAX_STATIC_HANDLE (20)
>  #define FASTRPC_CTXID_MASK GENMASK(15, 8)
>  #define INIT_FILELEN_MAX (2 * 1024 * 1024)
>  #define INIT_FILE_NAMELEN_MAX (128)
> @@ -105,6 +108,12 @@
>  
>  #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>  
> +/* Poll response number from remote processor for call completion */
> +#define FASTRPC_POLL_RESPONSE (0xdecaf)
> +
> +/* Polling mode timeout limit */
> +#define FASTRPC_POLL_MAX_TIMEOUT_US (10000)
> +
>  struct fastrpc_phy_page {
>  	dma_addr_t addr;	/* dma address */
>  	u64 size;		/* size of contiguous region */
> @@ -235,8 +244,14 @@ struct fastrpc_invoke_ctx {
>  	u32 sc;
>  	u64 *fdlist;
>  	u32 *crc;
> +	/* Poll memory that DSP updates */
> +	u32 *poll;
>  	u64 ctxid;
>  	u64 msg_sz;
> +	/* work done status flag */
> +	bool is_work_done;
> +	/* process updates poll memory instead of glink response */
> +	bool is_polled;
>  	struct kref refcount;
>  	struct list_head node; /* list of ctxs */
>  	struct completion work;
> @@ -307,6 +322,8 @@ struct fastrpc_user {
>  	int client_id;
>  	int pd;
>  	bool is_secure_dev;
> +	/* Flags poll mode state */
> +	bool poll_mode;
>  	/* Lock for lists */
>  	spinlock_t lock;
>  	/* lock for allocations */
> @@ -924,7 +941,8 @@ static int fastrpc_get_meta_size(struct fastrpc_invoke_ctx *ctx)
>  		sizeof(struct fastrpc_invoke_buf) +
>  		sizeof(struct fastrpc_phy_page)) * ctx->nscalars +
>  		sizeof(u64) * FASTRPC_MAX_FDLIST +
> -		sizeof(u32) * FASTRPC_MAX_CRCLIST;
> +		sizeof(u32) * FASTRPC_MAX_CRCLIST +
> +		sizeof(u32);
>  
>  	return size;
>  }
> @@ -1020,6 +1038,9 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>  	list = fastrpc_invoke_buf_start(rpra, ctx->nscalars);
>  	pages = fastrpc_phy_page_start(list, ctx->nscalars);
>  	ctx->fdlist = (u64 *)(pages + ctx->nscalars);
> +	ctx->poll = (u32 *)((uintptr_t)ctx->fdlist + sizeof(u64) * FASTRPC_MAX_FDLIST +
> +			     sizeof(u32) * FASTRPC_MAX_CRCLIST);
> +
>  	args = (uintptr_t)ctx->buf->virt + metalen;
>  	rlen = pkt_size - metalen;
>  	ctx->rpra = rpra;
> @@ -1188,6 +1209,75 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
>  
>  }
>  
> +static inline u32 fastrpc_poll_op(void *p)
> +{
> +	struct fastrpc_invoke_ctx *ctx = p;
> +
> +	dma_rmb();
> +	return READ_ONCE(*ctx->poll);
> +}
> +
> +static int poll_for_remote_response(struct fastrpc_invoke_ctx *ctx)
> +{
> +	u32 val;
> +	int ret;
> +
> +	/*
> +	 * Poll until DSP writes FASTRPC_POLL_RESPONSE into *ctx->poll
> +	 * or until another path marks the work done.
> +	 */
> +	ret = read_poll_timeout_atomic(fastrpc_poll_op, val,
> +				       (val == FASTRPC_POLL_RESPONSE) ||
> +				       ctx->is_work_done, 1,

Weird line wrap of the conditional, please put the val == and the
ctx->is_work_done on the same line - it's just 90 characters.

> +				       FASTRPC_POLL_MAX_TIMEOUT_US, false, ctx);
> +
> +	if (!ret && val == FASTRPC_POLL_RESPONSE) {
> +		ctx->is_work_done = true;
> +		ctx->retval = 0;
> +	}
> +
> +	if (ret == -ETIMEDOUT)
> +		ret = -EIO;
> +
> +	return ret;
> +}
> +
> +static inline int fastrpc_wait_for_response(struct fastrpc_invoke_ctx *ctx,
> +					    u32 kernel)
> +{
> +	int err = 0;
> +
> +	if (kernel) {
> +		if (!wait_for_completion_timeout(&ctx->work, 10 * HZ))
> +			err = -ETIMEDOUT;
> +	} else {
> +		err = wait_for_completion_interruptible(&ctx->work);
> +	}
> +
> +	return err;
> +}
> +
> +static int fastrpc_wait_for_completion(struct fastrpc_invoke_ctx *ctx,
> +				       u32 kernel)
> +{
> +	int err;
> +
> +	do {
> +		if (ctx->is_polled) {
> +			err = poll_for_remote_response(ctx);
> +			/* If polling timed out, move to normal response mode */

I had already written to question the lack of fallback to non-polling
mode and how this would prohibit me from mixing expected long and short
calls...

Would certainly be nice to clarify this behavior in the commit
message...

> +			if (err)
> +				ctx->is_polled = false;
> +		} else {
> +			err = fastrpc_wait_for_response(ctx, kernel);
> +			if (err)
> +				return err;
> +		}
> +	} while (!ctx->is_work_done);
> +
> +	return err;

Isn't 0 the only value of err you can get here with?

> +}
> +
>  static int fastrpc_internal_invoke(struct fastrpc_user *fl,  u32 kernel,
>  				   u32 handle, u32 sc,
>  				   struct fastrpc_invoke_args *args)
> @@ -1223,16 +1313,26 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl,  u32 kernel,
>  	if (err)
>  		goto bail;
>  
> -	if (kernel) {
> -		if (!wait_for_completion_timeout(&ctx->work, 10 * HZ))
> -			err = -ETIMEDOUT;
> -	} else {
> -		err = wait_for_completion_interruptible(&ctx->work);
> -	}
> +	/*
> +	 * Set message context as polled if the call is for a user PD
> +	 * dynamic module and user has enabled poll mode.
> +	 */
> +	if (handle > FASTRPC_MAX_STATIC_HANDLE && fl->pd == USER_PD &&
> +	    fl->poll_mode)

The line is 85 characters if you don't break it. You're allowed to use
up to 100 characters if it makes the code easier to read - and it does.

> +		ctx->is_polled = true;
> +
> +	err = fastrpc_wait_for_completion(ctx, kernel);
>  

Ugly blank line between the assignment and error check...

>  	if (err)
>  		goto bail;
>  
> +	if (!ctx->is_work_done) {

"err" is the return value of the wait, and this checks the outcome of
the wait... Returning "success" and pass "failure" through a sideband
channel is confusing.

That said, as far as I can see, there are three ways
fastrpc_wait_for_completion() can exit:

1) err = 0 && ctx->is_work_done = true after polling
2) err = 0 && ctx->is_work_done = true after wait
3) err != 0 && ctx->is_work_done is undefined after wait

For #1 and #2 we won't hit either if statement here.
For #3 we already hit above condition and went to bail.

So do we ever enter here?

> +		err = -ETIMEDOUT;
> +		dev_dbg(fl->sctx->dev, "Invalid workdone state for handle 0x%x, sc 0x%x\n",
> +			handle, sc);

jfyi, you can use %#x to format 0x%x

> +		goto bail;
> +	}
> +
>  	/* make sure that all memory writes by DSP are seen by CPU */
>  	dma_rmb();
>  	/* populate all the output buffers with results */
> @@ -1812,6 +1912,30 @@ static int fastrpc_get_info_from_kernel(struct fastrpc_ioctl_capability *cap,
>  	return 0;
>  }
>  
> +static int fastrpc_set_option(struct fastrpc_user *fl, char __user *argp)
> +{
> +	struct fastrpc_ioctl_set_option opt = {0};
> +	int i;
> +
> +	if (copy_from_user(&opt, argp, sizeof(opt)))
> +		return -EFAULT;
> +
> +	for (i = 0; i < ARRAY_SIZE(opt.reserved); i++) {
> +		if (opt.reserved[i] != 0)
> +			return -EINVAL;
> +	}
> +
> +	if (opt.req != FASTRPC_POLL_MODE)
> +		return -EINVAL;
> +
> +	if (opt.value)

Would it make sense to allow the caller to affect the poll timeout using
the other 31 bits of this value?

Regards,
Bjorn

> +		fl->poll_mode = true;
> +	else
> +		fl->poll_mode = false;
> +
> +	return 0;
> +}
> +
>  static int fastrpc_get_dsp_info(struct fastrpc_user *fl, char __user *argp)
>  {
>  	struct fastrpc_ioctl_capability cap = {0};
> @@ -2167,6 +2291,9 @@ static long fastrpc_device_ioctl(struct file *file, unsigned int cmd,
>  	case FASTRPC_IOCTL_MEM_UNMAP:
>  		err = fastrpc_req_mem_unmap(fl, argp);
>  		break;
> +	case FASTRPC_IOCTL_SET_OPTION:
> +		err = fastrpc_set_option(fl, argp);
> +		break;
>  	case FASTRPC_IOCTL_GET_DSP_INFO:
>  		err = fastrpc_get_dsp_info(fl, argp);
>  		break;
> @@ -2518,6 +2645,7 @@ static int fastrpc_rpmsg_callback(struct rpmsg_device *rpdev, void *data,
>  	}
>  
>  	ctx->retval = rsp->retval;
> +	ctx->is_work_done = true;
>  	complete(&ctx->work);
>  
>  	/*
> diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
> index c6e2925f47e6..c37e24a764ae 100644
> --- a/include/uapi/misc/fastrpc.h
> +++ b/include/uapi/misc/fastrpc.h
> @@ -16,6 +16,7 @@
>  #define FASTRPC_IOCTL_INIT_CREATE_STATIC _IOWR('R', 9, struct fastrpc_init_create_static)
>  #define FASTRPC_IOCTL_MEM_MAP		_IOWR('R', 10, struct fastrpc_mem_map)
>  #define FASTRPC_IOCTL_MEM_UNMAP		_IOWR('R', 11, struct fastrpc_mem_unmap)
> +#define FASTRPC_IOCTL_SET_OPTION	_IOWR('R', 12, struct fastrpc_ioctl_set_option)
>  #define FASTRPC_IOCTL_GET_DSP_INFO	_IOWR('R', 13, struct fastrpc_ioctl_capability)
>  
>  /**
> @@ -67,6 +68,9 @@ enum fastrpc_proc_attr {
>  /* Fastrpc attribute for memory protection of buffers */
>  #define FASTRPC_ATTR_SECUREMAP	(1)
>  
> +/* Set option request ID to enable poll mode */
> +#define FASTRPC_POLL_MODE	(1)
> +
>  struct fastrpc_invoke_args {
>  	__u64 ptr;
>  	__u64 length;
> @@ -133,6 +137,12 @@ struct fastrpc_mem_unmap {
>  	__s32 reserved[5];
>  };
>  
> +struct fastrpc_ioctl_set_option {
> +	__u32 req;	/* request id */
> +	__u32 value;	/* value */
> +	__s32 reserved[6];
> +};
> +
>  struct fastrpc_ioctl_capability {
>  	__u32 unused; /* deprecated, ignored by the kernel */
>  	__u32 attribute_id;
> -- 
> 2.34.1
> 
> 

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

* Re: [PATCH v6 1/4] misc: fastrpc: Move fdlist to invoke context structure
  2026-02-15 18:21 ` [PATCH v6 1/4] misc: fastrpc: Move fdlist to invoke context structure Ekansh Gupta
@ 2026-02-16  3:56   ` Bjorn Andersson
  2026-02-16  9:19     ` Ekansh Gupta
  0 siblings, 1 reply; 17+ messages in thread
From: Bjorn Andersson @ 2026-02-16  3:56 UTC (permalink / raw)
  To: Ekansh Gupta
  Cc: srini, linux-arm-msm, gregkh, quic_bkumar, linux-kernel,
	quic_chennak, dri-devel, arnd, dmitry.baryshkov

On Sun, Feb 15, 2026 at 11:51:32PM +0530, Ekansh Gupta wrote:
> The fdlist is currently part of the meta buffer,

Do you mean "already part of"?

> computed during
> put_args.

The only "computation" I can see wrt fdlist in fastrpc_put_args() is
where we calculate it to be at pages + inbufs + outbufs + handles.

So, why do you say that the content of the meta buffer is calculated
there? Did you mean fastrpc_get_args()?

PS. Use full function names and suffix them with (), to be clear in your
description.

> This leads to code duplication when preparing and reading
> critical meta buffer contents used by the FastRPC driver.

"used by the whole entire FastRPC driver" is rather broad... 

As far as I can tell, the thing this patch is doing is caching the
"fdlist" address, to avoid having to derive "pages" (and thereby
indirectly "list"), "outbufs", "handles", and then sum these up.

> 
> Move fdlist to the invoke context structure to improve maintainability
> and reduce redundancy. This centralizes its handling and simplifies
> meta buffer preparation and reading logic.

I think the patch looks good, but your commit message is too high-level
sales pitch.

Describe the specific problem that you're solving (i.e. you're
recalculating the offset which was known at the time of
fastrpc_get_args()) and leave it at that.

> 
> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
> ---
>  drivers/misc/fastrpc.c | 16 +++++-----------
>  1 file changed, 5 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 4f5a79c50f58..ce397c687161 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -233,6 +233,7 @@ struct fastrpc_invoke_ctx {
>  	int pid;
>  	int client_id;
>  	u32 sc;
> +	u64 *fdlist;
>  	u32 *crc;
>  	u64 ctxid;
>  	u64 msg_sz;
> @@ -1018,6 +1019,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>  	rpra = ctx->buf->virt;
>  	list = fastrpc_invoke_buf_start(rpra, ctx->nscalars);
>  	pages = fastrpc_phy_page_start(list, ctx->nscalars);
> +	ctx->fdlist = (u64 *)(pages + ctx->nscalars);
>  	args = (uintptr_t)ctx->buf->virt + metalen;
>  	rlen = pkt_size - metalen;
>  	ctx->rpra = rpra;
> @@ -1120,18 +1122,10 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx,
>  	union fastrpc_remote_arg *rpra = ctx->rpra;
>  	struct fastrpc_user *fl = ctx->fl;
>  	struct fastrpc_map *mmap = NULL;
> -	struct fastrpc_invoke_buf *list;
> -	struct fastrpc_phy_page *pages;
> -	u64 *fdlist;
> -	int i, inbufs, outbufs, handles;
> +	int i, inbufs;
>  	int ret = 0;
>  
>  	inbufs = REMOTE_SCALARS_INBUFS(ctx->sc);
> -	outbufs = REMOTE_SCALARS_OUTBUFS(ctx->sc);
> -	handles = REMOTE_SCALARS_INHANDLES(ctx->sc) + REMOTE_SCALARS_OUTHANDLES(ctx->sc);
> -	list = fastrpc_invoke_buf_start(rpra, ctx->nscalars);
> -	pages = fastrpc_phy_page_start(list, ctx->nscalars);
> -	fdlist = (uint64_t *)(pages + inbufs + outbufs + handles);
>  
>  	for (i = inbufs; i < ctx->nbufs; ++i) {
>  		if (!ctx->maps[i]) {
> @@ -1153,9 +1147,9 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx,
>  cleanup_fdlist:
>  	/* Clean up fdlist which is updated by DSP */
>  	for (i = 0; i < FASTRPC_MAX_FDLIST; i++) {
> -		if (!fdlist[i])
> +		if (!ctx->fdlist[i])

It wouldn't hurt to keep the local fdlist variable, keeps the code less
noisy - and you don't need to change these two places.

>  			break;
> -		if (!fastrpc_map_lookup(fl, (int)fdlist[i], &mmap))
> +		if (!fastrpc_map_lookup(fl, (int)ctx->fdlist[i], &mmap))

Why are the fds stored as u64 in the metadata? Are the actual fd numbers
somehow consumed by the DSP side?

Regards,
Bjorn

>  			fastrpc_map_put(mmap);
>  	}
>  
> -- 
> 2.34.1
> 
> 

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

* Re: [PATCH v6 4/4] misc: fastrpc: Add polling mode support for fastRPC driver
  2026-02-16  3:21   ` Bjorn Andersson
@ 2026-02-16  9:06     ` Ekansh Gupta
  2026-02-18 14:36       ` Bjorn Andersson
  0 siblings, 1 reply; 17+ messages in thread
From: Ekansh Gupta @ 2026-02-16  9:06 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: srini, linux-arm-msm, gregkh, quic_bkumar, linux-kernel,
	quic_chennak, dri-devel, arnd, dmitry.baryshkov



On 2/16/2026 8:51 AM, Bjorn Andersson wrote:
> On Sun, Feb 15, 2026 at 11:51:35PM +0530, Ekansh Gupta wrote:
>> For any remote call to DSP, after sending an invocation message,
>> fastRPC driver waits for glink response and during this time the
>> CPU can go into low power modes. This adds latency to overall fastrpc
>> call as CPU wakeup and scheduling latencies are included. Add polling
>> mode support with which fastRPC driver will poll continuously on a
>> memory after sending a message to remote subsystem which will eliminate
>> CPU wakeup and scheduling latencies and reduce fastRPC overhead. Poll
>> mode can be enabled by user by using FASTRPC_IOCTL_SET_OPTION ioctl
>> request with FASTRPC_POLL_MODE request id.
>>
>> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
>> ---
>>  drivers/misc/fastrpc.c      | 142 ++++++++++++++++++++++++++++++++++--
>>  include/uapi/misc/fastrpc.h |  10 +++
>>  2 files changed, 145 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index e935ae3776b4..c1e67dbacf2c 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -23,6 +23,8 @@
>>  #include <uapi/misc/fastrpc.h>
>>  #include <linux/of_reserved_mem.h>
>>  #include <linux/bits.h>
>> +#include <linux/compiler.h>
>> +#include <linux/iopoll.h>
>>  
>>  #define ADSP_DOMAIN_ID (0)
>>  #define MDSP_DOMAIN_ID (1)
>> @@ -37,6 +39,7 @@
>>  #define FASTRPC_CTX_MAX (256)
>>  #define FASTRPC_INIT_HANDLE	1
>>  #define FASTRPC_DSP_UTILITIES_HANDLE	2
>> +#define FASTRPC_MAX_STATIC_HANDLE (20)
>>  #define FASTRPC_CTXID_MASK GENMASK(15, 8)
>>  #define INIT_FILELEN_MAX (2 * 1024 * 1024)
>>  #define INIT_FILE_NAMELEN_MAX (128)
>> @@ -105,6 +108,12 @@
>>  
>>  #define miscdev_to_fdevice(d) container_of(d, struct fastrpc_device, miscdev)
>>  
>> +/* Poll response number from remote processor for call completion */
>> +#define FASTRPC_POLL_RESPONSE (0xdecaf)
>> +
>> +/* Polling mode timeout limit */
>> +#define FASTRPC_POLL_MAX_TIMEOUT_US (10000)
>> +
>>  struct fastrpc_phy_page {
>>  	dma_addr_t addr;	/* dma address */
>>  	u64 size;		/* size of contiguous region */
>> @@ -235,8 +244,14 @@ struct fastrpc_invoke_ctx {
>>  	u32 sc;
>>  	u64 *fdlist;
>>  	u32 *crc;
>> +	/* Poll memory that DSP updates */
>> +	u32 *poll;
>>  	u64 ctxid;
>>  	u64 msg_sz;
>> +	/* work done status flag */
>> +	bool is_work_done;
>> +	/* process updates poll memory instead of glink response */
>> +	bool is_polled;
>>  	struct kref refcount;
>>  	struct list_head node; /* list of ctxs */
>>  	struct completion work;
>> @@ -307,6 +322,8 @@ struct fastrpc_user {
>>  	int client_id;
>>  	int pd;
>>  	bool is_secure_dev;
>> +	/* Flags poll mode state */
>> +	bool poll_mode;
>>  	/* Lock for lists */
>>  	spinlock_t lock;
>>  	/* lock for allocations */
>> @@ -924,7 +941,8 @@ static int fastrpc_get_meta_size(struct fastrpc_invoke_ctx *ctx)
>>  		sizeof(struct fastrpc_invoke_buf) +
>>  		sizeof(struct fastrpc_phy_page)) * ctx->nscalars +
>>  		sizeof(u64) * FASTRPC_MAX_FDLIST +
>> -		sizeof(u32) * FASTRPC_MAX_CRCLIST;
>> +		sizeof(u32) * FASTRPC_MAX_CRCLIST +
>> +		sizeof(u32);
>>  
>>  	return size;
>>  }
>> @@ -1020,6 +1038,9 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>>  	list = fastrpc_invoke_buf_start(rpra, ctx->nscalars);
>>  	pages = fastrpc_phy_page_start(list, ctx->nscalars);
>>  	ctx->fdlist = (u64 *)(pages + ctx->nscalars);
>> +	ctx->poll = (u32 *)((uintptr_t)ctx->fdlist + sizeof(u64) * FASTRPC_MAX_FDLIST +
>> +			     sizeof(u32) * FASTRPC_MAX_CRCLIST);
>> +
>>  	args = (uintptr_t)ctx->buf->virt + metalen;
>>  	rlen = pkt_size - metalen;
>>  	ctx->rpra = rpra;
>> @@ -1188,6 +1209,75 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
>>  
>>  }
>>  
>> +static inline u32 fastrpc_poll_op(void *p)
>> +{
>> +	struct fastrpc_invoke_ctx *ctx = p;
>> +
>> +	dma_rmb();
>> +	return READ_ONCE(*ctx->poll);
>> +}
>> +
>> +static int poll_for_remote_response(struct fastrpc_invoke_ctx *ctx)
>> +{
>> +	u32 val;
>> +	int ret;
>> +
>> +	/*
>> +	 * Poll until DSP writes FASTRPC_POLL_RESPONSE into *ctx->poll
>> +	 * or until another path marks the work done.
>> +	 */
>> +	ret = read_poll_timeout_atomic(fastrpc_poll_op, val,
>> +				       (val == FASTRPC_POLL_RESPONSE) ||
>> +				       ctx->is_work_done, 1,
> Weird line wrap of the conditional, please put the val == and the
> ctx->is_work_done on the same line - it's just 90 characters.
Ack.
>
>> +				       FASTRPC_POLL_MAX_TIMEOUT_US, false, ctx);
>> +
>> +	if (!ret && val == FASTRPC_POLL_RESPONSE) {
>> +		ctx->is_work_done = true;
>> +		ctx->retval = 0;
>> +	}
>> +
>> +	if (ret == -ETIMEDOUT)
>> +		ret = -EIO;
>> +
>> +	return ret;
>> +}
>> +
>> +static inline int fastrpc_wait_for_response(struct fastrpc_invoke_ctx *ctx,
>> +					    u32 kernel)
>> +{
>> +	int err = 0;
>> +
>> +	if (kernel) {
>> +		if (!wait_for_completion_timeout(&ctx->work, 10 * HZ))
>> +			err = -ETIMEDOUT;
>> +	} else {
>> +		err = wait_for_completion_interruptible(&ctx->work);
>> +	}
>> +
>> +	return err;
>> +}
>> +
>> +static int fastrpc_wait_for_completion(struct fastrpc_invoke_ctx *ctx,
>> +				       u32 kernel)
>> +{
>> +	int err;
>> +
>> +	do {
>> +		if (ctx->is_polled) {
>> +			err = poll_for_remote_response(ctx);
>> +			/* If polling timed out, move to normal response mode */
> I had already written to question the lack of fallback to non-polling
> mode and how this would prohibit me from mixing expected long and short
> calls...
>
> Would certainly be nice to clarify this behavior in the commit
> message...
I'll add more details for this.
>
>> +			if (err)
>> +				ctx->is_polled = false;
>> +		} else {
>> +			err = fastrpc_wait_for_response(ctx, kernel);
>> +			if (err)
>> +				return err;
>> +		}
>> +	} while (!ctx->is_work_done);
>> +
>> +	return err;
> Isn't 0 the only value of err you can get here with?
yes, it's always going to be return 0; I'll update this.
>
>> +}
>> +
>>  static int fastrpc_internal_invoke(struct fastrpc_user *fl,  u32 kernel,
>>  				   u32 handle, u32 sc,
>>  				   struct fastrpc_invoke_args *args)
>> @@ -1223,16 +1313,26 @@ static int fastrpc_internal_invoke(struct fastrpc_user *fl,  u32 kernel,
>>  	if (err)
>>  		goto bail;
>>  
>> -	if (kernel) {
>> -		if (!wait_for_completion_timeout(&ctx->work, 10 * HZ))
>> -			err = -ETIMEDOUT;
>> -	} else {
>> -		err = wait_for_completion_interruptible(&ctx->work);
>> -	}
>> +	/*
>> +	 * Set message context as polled if the call is for a user PD
>> +	 * dynamic module and user has enabled poll mode.
>> +	 */
>> +	if (handle > FASTRPC_MAX_STATIC_HANDLE && fl->pd == USER_PD &&
>> +	    fl->poll_mode)
> The line is 85 characters if you don't break it. You're allowed to use
> up to 100 characters if it makes the code easier to read - and it does.
Ack.
>
>> +		ctx->is_polled = true;
>> +
>> +	err = fastrpc_wait_for_completion(ctx, kernel);
>>  
> Ugly blank line between the assignment and error check...
I'll remove this.
>
>>  	if (err)
>>  		goto bail;
>>  
>> +	if (!ctx->is_work_done) {
> "err" is the return value of the wait, and this checks the outcome of
> the wait... Returning "success" and pass "failure" through a sideband
> channel is confusing.
>
> That said, as far as I can see, there are three ways
> fastrpc_wait_for_completion() can exit:
>
> 1) err = 0 && ctx->is_work_done = true after polling
> 2) err = 0 && ctx->is_work_done = true after wait
> 3) err != 0 && ctx->is_work_done is undefined after wait
>
> For #1 and #2 we won't hit either if statement here.
> For #3 we already hit above condition and went to bail.
>
> So do we ever enter here?
You're right, the check is not getting encountered due to the following reasons:
1) Poll success -> err = 0, is_work_done =true.
2) Wait success -> err = 0, is_work_done =true.
3) Poll failed -> fallback to wait.
4) Wait failed -> err check before this if condition.

I'll remove this check.
>
>> +		err = -ETIMEDOUT;
>> +		dev_dbg(fl->sctx->dev, "Invalid workdone state for handle 0x%x, sc 0x%x\n",
>> +			handle, sc);
> jfyi, you can use %#x to format 0x%x
Ack.
>
>> +		goto bail;
>> +	}
>> +
>>  	/* make sure that all memory writes by DSP are seen by CPU */
>>  	dma_rmb();
>>  	/* populate all the output buffers with results */
>> @@ -1812,6 +1912,30 @@ static int fastrpc_get_info_from_kernel(struct fastrpc_ioctl_capability *cap,
>>  	return 0;
>>  }
>>  
>> +static int fastrpc_set_option(struct fastrpc_user *fl, char __user *argp)
>> +{
>> +	struct fastrpc_ioctl_set_option opt = {0};
>> +	int i;
>> +
>> +	if (copy_from_user(&opt, argp, sizeof(opt)))
>> +		return -EFAULT;
>> +
>> +	for (i = 0; i < ARRAY_SIZE(opt.reserved); i++) {
>> +		if (opt.reserved[i] != 0)
>> +			return -EINVAL;
>> +	}
>> +
>> +	if (opt.req != FASTRPC_POLL_MODE)
>> +		return -EINVAL;
>> +
>> +	if (opt.value)
> Would it make sense to allow the caller to affect the poll timeout using
> the other 31 bits of this value?
I was planning to bring that control[1], but it's might be difficult for the caller

[1] https://lore.kernel.org/all/20250127044239.578540-5-quic_ekangupt@quicinc.com/

//Ekansh
>
> Regards,
> Bjorn
>
>> +		fl->poll_mode = true;
>> +	else
>> +		fl->poll_mode = false;
>> +
>> +	return 0;
>> +}
>> +
>>  static int fastrpc_get_dsp_info(struct fastrpc_user *fl, char __user *argp)
>>  {
>>  	struct fastrpc_ioctl_capability cap = {0};
>> @@ -2167,6 +2291,9 @@ static long fastrpc_device_ioctl(struct file *file, unsigned int cmd,
>>  	case FASTRPC_IOCTL_MEM_UNMAP:
>>  		err = fastrpc_req_mem_unmap(fl, argp);
>>  		break;
>> +	case FASTRPC_IOCTL_SET_OPTION:
>> +		err = fastrpc_set_option(fl, argp);
>> +		break;
>>  	case FASTRPC_IOCTL_GET_DSP_INFO:
>>  		err = fastrpc_get_dsp_info(fl, argp);
>>  		break;
>> @@ -2518,6 +2645,7 @@ static int fastrpc_rpmsg_callback(struct rpmsg_device *rpdev, void *data,
>>  	}
>>  
>>  	ctx->retval = rsp->retval;
>> +	ctx->is_work_done = true;
>>  	complete(&ctx->work);
>>  
>>  	/*
>> diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
>> index c6e2925f47e6..c37e24a764ae 100644
>> --- a/include/uapi/misc/fastrpc.h
>> +++ b/include/uapi/misc/fastrpc.h
>> @@ -16,6 +16,7 @@
>>  #define FASTRPC_IOCTL_INIT_CREATE_STATIC _IOWR('R', 9, struct fastrpc_init_create_static)
>>  #define FASTRPC_IOCTL_MEM_MAP		_IOWR('R', 10, struct fastrpc_mem_map)
>>  #define FASTRPC_IOCTL_MEM_UNMAP		_IOWR('R', 11, struct fastrpc_mem_unmap)
>> +#define FASTRPC_IOCTL_SET_OPTION	_IOWR('R', 12, struct fastrpc_ioctl_set_option)
>>  #define FASTRPC_IOCTL_GET_DSP_INFO	_IOWR('R', 13, struct fastrpc_ioctl_capability)
>>  
>>  /**
>> @@ -67,6 +68,9 @@ enum fastrpc_proc_attr {
>>  /* Fastrpc attribute for memory protection of buffers */
>>  #define FASTRPC_ATTR_SECUREMAP	(1)
>>  
>> +/* Set option request ID to enable poll mode */
>> +#define FASTRPC_POLL_MODE	(1)
>> +
>>  struct fastrpc_invoke_args {
>>  	__u64 ptr;
>>  	__u64 length;
>> @@ -133,6 +137,12 @@ struct fastrpc_mem_unmap {
>>  	__s32 reserved[5];
>>  };
>>  
>> +struct fastrpc_ioctl_set_option {
>> +	__u32 req;	/* request id */
>> +	__u32 value;	/* value */
>> +	__s32 reserved[6];
>> +};
>> +
>>  struct fastrpc_ioctl_capability {
>>  	__u32 unused; /* deprecated, ignored by the kernel */
>>  	__u32 attribute_id;
>> -- 
>> 2.34.1
>>
>>


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

* Re: [PATCH v6 1/4] misc: fastrpc: Move fdlist to invoke context structure
  2026-02-16  3:56   ` Bjorn Andersson
@ 2026-02-16  9:19     ` Ekansh Gupta
  2026-02-18 14:15       ` Bjorn Andersson
  0 siblings, 1 reply; 17+ messages in thread
From: Ekansh Gupta @ 2026-02-16  9:19 UTC (permalink / raw)
  To: Bjorn Andersson
  Cc: srini, linux-arm-msm, gregkh, quic_bkumar, linux-kernel,
	quic_chennak, dri-devel, arnd, dmitry.baryshkov



On 2/16/2026 9:26 AM, Bjorn Andersson wrote:
> On Sun, Feb 15, 2026 at 11:51:32PM +0530, Ekansh Gupta wrote:
>> The fdlist is currently part of the meta buffer,
> Do you mean "already part of"?
yes.
>
>> computed during
>> put_args.
> The only "computation" I can see wrt fdlist in fastrpc_put_args() is
> where we calculate it to be at pages + inbufs + outbufs + handles.
>
> So, why do you say that the content of the meta buffer is calculated
> there? Did you mean fastrpc_get_args()?
The fdlist is updated by DSP. By "computation", I meant getting the location of fdlist
in metadata buffer.

fastrpc_get_args allocates metadata buffer, fdlist is at some offset in this buffer. This
list is updated by the DSP and rechecked during fastrpc_put_args for any entries.
>
> PS. Use full function names and suffix them with (), to be clear in your
> description.
Ack.
>
>> This leads to code duplication when preparing and reading
>> critical meta buffer contents used by the FastRPC driver.
> "used by the whole entire FastRPC driver" is rather broad... 
>
> As far as I can tell, the thing this patch is doing is caching the
> "fdlist" address, to avoid having to derive "pages" (and thereby
> indirectly "list"), "outbufs", "handles", and then sum these up.
>
>> Move fdlist to the invoke context structure to improve maintainability
>> and reduce redundancy. This centralizes its handling and simplifies
>> meta buffer preparation and reading logic.
> I think the patch looks good, but your commit message is too high-level
> sales pitch.
>
> Describe the specific problem that you're solving (i.e. you're
> recalculating the offset which was known at the time of
> fastrpc_get_args()) and leave it at that.
I see the problem with the commit text. Let me come with a better description.
>
>> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
>> ---
>>  drivers/misc/fastrpc.c | 16 +++++-----------
>>  1 file changed, 5 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
>> index 4f5a79c50f58..ce397c687161 100644
>> --- a/drivers/misc/fastrpc.c
>> +++ b/drivers/misc/fastrpc.c
>> @@ -233,6 +233,7 @@ struct fastrpc_invoke_ctx {
>>  	int pid;
>>  	int client_id;
>>  	u32 sc;
>> +	u64 *fdlist;
>>  	u32 *crc;
>>  	u64 ctxid;
>>  	u64 msg_sz;
>> @@ -1018,6 +1019,7 @@ static int fastrpc_get_args(u32 kernel, struct fastrpc_invoke_ctx *ctx)
>>  	rpra = ctx->buf->virt;
>>  	list = fastrpc_invoke_buf_start(rpra, ctx->nscalars);
>>  	pages = fastrpc_phy_page_start(list, ctx->nscalars);
>> +	ctx->fdlist = (u64 *)(pages + ctx->nscalars);
>>  	args = (uintptr_t)ctx->buf->virt + metalen;
>>  	rlen = pkt_size - metalen;
>>  	ctx->rpra = rpra;
>> @@ -1120,18 +1122,10 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx,
>>  	union fastrpc_remote_arg *rpra = ctx->rpra;
>>  	struct fastrpc_user *fl = ctx->fl;
>>  	struct fastrpc_map *mmap = NULL;
>> -	struct fastrpc_invoke_buf *list;
>> -	struct fastrpc_phy_page *pages;
>> -	u64 *fdlist;
>> -	int i, inbufs, outbufs, handles;
>> +	int i, inbufs;
>>  	int ret = 0;
>>  
>>  	inbufs = REMOTE_SCALARS_INBUFS(ctx->sc);
>> -	outbufs = REMOTE_SCALARS_OUTBUFS(ctx->sc);
>> -	handles = REMOTE_SCALARS_INHANDLES(ctx->sc) + REMOTE_SCALARS_OUTHANDLES(ctx->sc);
>> -	list = fastrpc_invoke_buf_start(rpra, ctx->nscalars);
>> -	pages = fastrpc_phy_page_start(list, ctx->nscalars);
>> -	fdlist = (uint64_t *)(pages + inbufs + outbufs + handles);
>>  
>>  	for (i = inbufs; i < ctx->nbufs; ++i) {
>>  		if (!ctx->maps[i]) {
>> @@ -1153,9 +1147,9 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx,
>>  cleanup_fdlist:
>>  	/* Clean up fdlist which is updated by DSP */
>>  	for (i = 0; i < FASTRPC_MAX_FDLIST; i++) {
>> -		if (!fdlist[i])
>> +		if (!ctx->fdlist[i])
> It wouldn't hurt to keep the local fdlist variable, keeps the code less
> noisy - and you don't need to change these two places.
Ack.
>
>>  			break;
>> -		if (!fastrpc_map_lookup(fl, (int)fdlist[i], &mmap))
>> +		if (!fastrpc_map_lookup(fl, (int)ctx->fdlist[i], &mmap))
> Why are the fds stored as u64 in the metadata? Are the actual fd numbers
> somehow consumed by the DSP side?
DSP uses it for book-keeping mostly for maintaining DSP side mapping based on fd.

Thanks for spending time on reviewing this change. I'll fix your concerns in the next spin.

//Ekansh
>
> Regards,
> Bjorn
>
>>  			fastrpc_map_put(mmap);
>>  	}
>>  
>> -- 
>> 2.34.1
>>
>>


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

* Re: [PATCH v6 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK
  2026-02-16  2:39   ` kernel test robot
@ 2026-02-16 11:08     ` Konrad Dybcio
  0 siblings, 0 replies; 17+ messages in thread
From: Konrad Dybcio @ 2026-02-16 11:08 UTC (permalink / raw)
  To: kernel test robot, Ekansh Gupta, srini, linux-arm-msm
  Cc: llvm, oe-kbuild-all, gregkh, quic_bkumar, linux-kernel,
	quic_chennak, dri-devel, arnd, dmitry.baryshkov

On 2/16/26 3:39 AM, kernel test robot wrote:
> Hi Ekansh,
> 
> kernel test robot noticed the following build errors:
> 
> [auto build test ERROR on char-misc/char-misc-testing]
> [also build test ERROR on char-misc/char-misc-next char-misc/char-misc-linus linus/master v6.19 next-20260213]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
> 
> url:    https://github.com/intel-lab-lkp/linux/commits/Ekansh-Gupta/misc-fastrpc-Move-fdlist-to-invoke-context-structure/20260216-022305
> base:   char-misc/char-misc-testing
> patch link:    https://lore.kernel.org/r/20260215182136.3995111-3-ekansh.gupta%40oss.qualcomm.com
> patch subject: [PATCH v6 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK
> config: hexagon-randconfig-001-20260216 (https://download.01.org/0day-ci/archive/20260216/202602161009.M2K52X34-lkp@intel.com/config)
> compiler: clang version 23.0.0git (https://github.com/llvm/llvm-project e86750b29fa0ff207cd43213d66dabe565417638)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260216/202602161009.M2K52X34-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202602161009.M2K52X34-lkp@intel.com/
> 
> All errors (new ones prefixed by >>):
> 
>>> drivers/misc/fastrpc.c:518:29: error: call to undeclared function 'FIELD_GET'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration]
>      518 |         idr_remove(&cctx->ctx_idr, FIELD_GET(FASTRPC_CTXID_MASK, ctx->ctxid));


#include <linux/bitops.h>

Konrad

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

* Re: [PATCH v6 1/4] misc: fastrpc: Move fdlist to invoke context structure
  2026-02-16  9:19     ` Ekansh Gupta
@ 2026-02-18 14:15       ` Bjorn Andersson
  0 siblings, 0 replies; 17+ messages in thread
From: Bjorn Andersson @ 2026-02-18 14:15 UTC (permalink / raw)
  To: Ekansh Gupta
  Cc: srini, linux-arm-msm, gregkh, quic_bkumar, linux-kernel,
	quic_chennak, dri-devel, arnd, dmitry.baryshkov

On Mon, Feb 16, 2026 at 02:49:36PM +0530, Ekansh Gupta wrote:
> 
> 
> On 2/16/2026 9:26 AM, Bjorn Andersson wrote:
> > On Sun, Feb 15, 2026 at 11:51:32PM +0530, Ekansh Gupta wrote:
> >> The fdlist is currently part of the meta buffer,
> > Do you mean "already part of"?
> yes.
> >
> >> computed during
> >> put_args.
> > The only "computation" I can see wrt fdlist in fastrpc_put_args() is
> > where we calculate it to be at pages + inbufs + outbufs + handles.
> >
> > So, why do you say that the content of the meta buffer is calculated
> > there? Did you mean fastrpc_get_args()?
> The fdlist is updated by DSP. By "computation", I meant getting the location of fdlist
> in metadata buffer.
> 
> fastrpc_get_args allocates metadata buffer, fdlist is at some offset in this buffer. This
> list is updated by the DSP and rechecked during fastrpc_put_args for any entries.

Does that mean that fastrpc_get_args() and fastrpc_put_args() aren't
symmetrical?

What is the life cycle for the mappings then? Can a buffer be "passed"
to the DSP in one invoke only to be "returned" in a later invoke?

> >
> > PS. Use full function names and suffix them with (), to be clear in your
> > description.
> Ack.
> >
> >> This leads to code duplication when preparing and reading
> >> critical meta buffer contents used by the FastRPC driver.
> > "used by the whole entire FastRPC driver" is rather broad... 
> >
> > As far as I can tell, the thing this patch is doing is caching the
> > "fdlist" address, to avoid having to derive "pages" (and thereby
> > indirectly "list"), "outbufs", "handles", and then sum these up.
> >
> >> Move fdlist to the invoke context structure to improve maintainability
> >> and reduce redundancy. This centralizes its handling and simplifies
> >> meta buffer preparation and reading logic.
> > I think the patch looks good, but your commit message is too high-level
> > sales pitch.
> >
> > Describe the specific problem that you're solving (i.e. you're
> > recalculating the offset which was known at the time of
> > fastrpc_get_args()) and leave it at that.
> I see the problem with the commit text. Let me come with a better description.

Thank you.

> >
> >> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
> >> ---
[..]
> >> @@ -1153,9 +1147,9 @@ static int fastrpc_put_args(struct fastrpc_invoke_ctx *ctx,
> >>  cleanup_fdlist:
> >>  	/* Clean up fdlist which is updated by DSP */
> >>  	for (i = 0; i < FASTRPC_MAX_FDLIST; i++) {
> >> -		if (!fdlist[i])
> >> +		if (!ctx->fdlist[i])
> > It wouldn't hurt to keep the local fdlist variable, keeps the code less
> > noisy - and you don't need to change these two places.
> Ack.
> >
> >>  			break;
> >> -		if (!fastrpc_map_lookup(fl, (int)fdlist[i], &mmap))
> >> +		if (!fastrpc_map_lookup(fl, (int)ctx->fdlist[i], &mmap))
> > Why are the fds stored as u64 in the metadata? Are the actual fd numbers
> > somehow consumed by the DSP side?
> DSP uses it for book-keeping mostly for maintaining DSP side mapping based on fd.
> 

Okay, so then the size is part of the ABI. Thanks for clarifying.

Regards,
Bjorn

> Thanks for spending time on reviewing this change. I'll fix your concerns in the next spin.
> 
> //Ekansh
> >
> > Regards,
> > Bjorn
> >
> >>  			fastrpc_map_put(mmap);
> >>  	}
> >>  
> >> -- 
> >> 2.34.1
> >>
> >>
> 

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

* Re: [PATCH v6 4/4] misc: fastrpc: Add polling mode support for fastRPC driver
  2026-02-16  9:06     ` Ekansh Gupta
@ 2026-02-18 14:36       ` Bjorn Andersson
  2026-02-18 14:38         ` Konrad Dybcio
  0 siblings, 1 reply; 17+ messages in thread
From: Bjorn Andersson @ 2026-02-18 14:36 UTC (permalink / raw)
  To: Ekansh Gupta
  Cc: srini, linux-arm-msm, gregkh, quic_bkumar, linux-kernel,
	quic_chennak, dri-devel, arnd, dmitry.baryshkov

On Mon, Feb 16, 2026 at 02:36:40PM +0530, Ekansh Gupta wrote:
> On 2/16/2026 8:51 AM, Bjorn Andersson wrote:
> > On Sun, Feb 15, 2026 at 11:51:35PM +0530, Ekansh Gupta wrote:
> >> @@ -1812,6 +1912,30 @@ static int fastrpc_get_info_from_kernel(struct fastrpc_ioctl_capability *cap,
> >>  	return 0;
> >>  }
> >>  
> >> +static int fastrpc_set_option(struct fastrpc_user *fl, char __user *argp)
> >> +{
> >> +	struct fastrpc_ioctl_set_option opt = {0};
> >> +	int i;
> >> +
> >> +	if (copy_from_user(&opt, argp, sizeof(opt)))
> >> +		return -EFAULT;
> >> +
> >> +	for (i = 0; i < ARRAY_SIZE(opt.reserved); i++) {
> >> +		if (opt.reserved[i] != 0)
> >> +			return -EINVAL;
> >> +	}
> >> +
> >> +	if (opt.req != FASTRPC_POLL_MODE)
> >> +		return -EINVAL;
> >> +
> >> +	if (opt.value)
> > Would it make sense to allow the caller to affect the poll timeout using
> > the other 31 bits of this value?
> I was planning to bring that control[1], but it's might be difficult for the caller
> 

Skimming through the thread, it seems you're discussing how to determine
if the DSP supports polling or not; that sounds like a separate problem
in my view. Not sure if you settled that discussion, but couldn't that
be handled through FASTRPC_IOCTL_GET_DSP_INFO?

I assume though, this would be subject to firmware changes. How do you
determine downstream if polling should be used or not today?


For my specific question here, I'm merely wondering if the timeout value
should be a boolean or have a unit. We could punt on that question, to
not block this feature from making progress upstream, by defining that
only 0 and 1 are valid values today (all other result in -EINVAL).

This would leave the door open for having 0 == off, 1 == default, > 1
represent the actual timeout in microseconds in the future.

Treating any non-zero value as "the default timeout" means that you
would have to assume that there's userspace who might pass other values
and you can't add additional meaning to the field in the future.

Regards,
Bjorn

> [1] https://lore.kernel.org/all/20250127044239.578540-5-quic_ekangupt@quicinc.com/
> 
> //Ekansh

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

* Re: [PATCH v6 4/4] misc: fastrpc: Add polling mode support for fastRPC driver
  2026-02-18 14:36       ` Bjorn Andersson
@ 2026-02-18 14:38         ` Konrad Dybcio
  2026-02-19 13:39           ` Bjorn Andersson
  0 siblings, 1 reply; 17+ messages in thread
From: Konrad Dybcio @ 2026-02-18 14:38 UTC (permalink / raw)
  To: Bjorn Andersson, Ekansh Gupta
  Cc: srini, linux-arm-msm, gregkh, quic_bkumar, linux-kernel,
	quic_chennak, dri-devel, arnd, dmitry.baryshkov



On 18-Feb-26 15:36, Bjorn Andersson wrote:
> On Mon, Feb 16, 2026 at 02:36:40PM +0530, Ekansh Gupta wrote:
>> On 2/16/2026 8:51 AM, Bjorn Andersson wrote:
>>> On Sun, Feb 15, 2026 at 11:51:35PM +0530, Ekansh Gupta wrote:
>>>> @@ -1812,6 +1912,30 @@ static int fastrpc_get_info_from_kernel(struct fastrpc_ioctl_capability *cap,
>>>>  	return 0;
>>>>  }
>>>>  
>>>> +static int fastrpc_set_option(struct fastrpc_user *fl, char __user *argp)
>>>> +{
>>>> +	struct fastrpc_ioctl_set_option opt = {0};
>>>> +	int i;
>>>> +
>>>> +	if (copy_from_user(&opt, argp, sizeof(opt)))
>>>> +		return -EFAULT;
>>>> +
>>>> +	for (i = 0; i < ARRAY_SIZE(opt.reserved); i++) {
>>>> +		if (opt.reserved[i] != 0)
>>>> +			return -EINVAL;
>>>> +	}
>>>> +
>>>> +	if (opt.req != FASTRPC_POLL_MODE)
>>>> +		return -EINVAL;
>>>> +
>>>> +	if (opt.value)
>>> Would it make sense to allow the caller to affect the poll timeout using
>>> the other 31 bits of this value?
>> I was planning to bring that control[1], but it's might be difficult for the caller
>>
> 
> Skimming through the thread, it seems you're discussing how to determine
> if the DSP supports polling or not; that sounds like a separate problem
> in my view. Not sure if you settled that discussion, but couldn't that
> be handled through FASTRPC_IOCTL_GET_DSP_INFO?
> 
> I assume though, this would be subject to firmware changes. How do you
> determine downstream if polling should be used or not today?
> 
> 
> For my specific question here, I'm merely wondering if the timeout value
> should be a boolean or have a unit. We could punt on that question, to
> not block this feature from making progress upstream, by defining that
> only 0 and 1 are valid values today (all other result in -EINVAL).
> 
> This would leave the door open for having 0 == off, 1 == default, > 1

Giving '1' a special non-numerical meaning sounds odd.. maybe 0:default,
-1:off (or the opposite)?

Konrad

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

* Re: [PATCH v6 4/4] misc: fastrpc: Add polling mode support for fastRPC driver
  2026-02-15 18:21 ` [PATCH v6 4/4] misc: fastrpc: Add polling mode support for fastRPC driver Ekansh Gupta
  2026-02-16  3:21   ` Bjorn Andersson
@ 2026-02-19  7:36   ` Dmitry Baryshkov
  1 sibling, 0 replies; 17+ messages in thread
From: Dmitry Baryshkov @ 2026-02-19  7:36 UTC (permalink / raw)
  To: Ekansh Gupta
  Cc: srini, linux-arm-msm, gregkh, quic_bkumar, linux-kernel,
	quic_chennak, dri-devel, arnd

On Sun, Feb 15, 2026 at 11:51:35PM +0530, Ekansh Gupta wrote:
> For any remote call to DSP, after sending an invocation message,
> fastRPC driver waits for glink response and during this time the
> CPU can go into low power modes. This adds latency to overall fastrpc
> call as CPU wakeup and scheduling latencies are included. Add polling
> mode support with which fastRPC driver will poll continuously on a
> memory after sending a message to remote subsystem which will eliminate
> CPU wakeup and scheduling latencies and reduce fastRPC overhead. Poll
> mode can be enabled by user by using FASTRPC_IOCTL_SET_OPTION ioctl
> request with FASTRPC_POLL_MODE request id.
> 
> Signed-off-by: Ekansh Gupta <ekansh.gupta@oss.qualcomm.com>
> ---
>  drivers/misc/fastrpc.c      | 142 ++++++++++++++++++++++++++++++++++--
>  include/uapi/misc/fastrpc.h |  10 +++
>  2 files changed, 145 insertions(+), 7 deletions(-)
> 
> @@ -1812,6 +1912,30 @@ static int fastrpc_get_info_from_kernel(struct fastrpc_ioctl_capability *cap,
>  	return 0;
>  }
>  
> +static int fastrpc_set_option(struct fastrpc_user *fl, char __user *argp)
> +{
> +	struct fastrpc_ioctl_set_option opt = {0};
> +	int i;
> +
> +	if (copy_from_user(&opt, argp, sizeof(opt)))
> +		return -EFAULT;
> +
> +	for (i = 0; i < ARRAY_SIZE(opt.reserved); i++) {
> +		if (opt.reserved[i] != 0)
> +			return -EINVAL;
> +	}
> +
> +	if (opt.req != FASTRPC_POLL_MODE)
> +		return -EINVAL;
> +
> +	if (opt.value)
> +		fl->poll_mode = true;
> +	else
> +		fl->poll_mode = false;

I think I've raised this question beforehand. This implementation will
return success to the userspace even on the platforms where polling is
not supported. This is not correct.

> +
> +	return 0;
> +}
> +
>  static int fastrpc_get_dsp_info(struct fastrpc_user *fl, char __user *argp)
>  {
>  	struct fastrpc_ioctl_capability cap = {0};

-- 
With best wishes
Dmitry

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

* Re: [PATCH v6 4/4] misc: fastrpc: Add polling mode support for fastRPC driver
  2026-02-18 14:38         ` Konrad Dybcio
@ 2026-02-19 13:39           ` Bjorn Andersson
  0 siblings, 0 replies; 17+ messages in thread
From: Bjorn Andersson @ 2026-02-19 13:39 UTC (permalink / raw)
  To: Konrad Dybcio
  Cc: Ekansh Gupta, srini, linux-arm-msm, gregkh, quic_bkumar,
	linux-kernel, quic_chennak, dri-devel, arnd, dmitry.baryshkov

On Wed, Feb 18, 2026 at 03:38:28PM +0100, Konrad Dybcio wrote:
> 
> 
> On 18-Feb-26 15:36, Bjorn Andersson wrote:
> > On Mon, Feb 16, 2026 at 02:36:40PM +0530, Ekansh Gupta wrote:
> >> On 2/16/2026 8:51 AM, Bjorn Andersson wrote:
> >>> On Sun, Feb 15, 2026 at 11:51:35PM +0530, Ekansh Gupta wrote:
> >>>> @@ -1812,6 +1912,30 @@ static int fastrpc_get_info_from_kernel(struct fastrpc_ioctl_capability *cap,
> >>>>  	return 0;
> >>>>  }
> >>>>  
> >>>> +static int fastrpc_set_option(struct fastrpc_user *fl, char __user *argp)
> >>>> +{
> >>>> +	struct fastrpc_ioctl_set_option opt = {0};
> >>>> +	int i;
> >>>> +
> >>>> +	if (copy_from_user(&opt, argp, sizeof(opt)))
> >>>> +		return -EFAULT;
> >>>> +
> >>>> +	for (i = 0; i < ARRAY_SIZE(opt.reserved); i++) {
> >>>> +		if (opt.reserved[i] != 0)
> >>>> +			return -EINVAL;
> >>>> +	}
> >>>> +
> >>>> +	if (opt.req != FASTRPC_POLL_MODE)
> >>>> +		return -EINVAL;
> >>>> +
> >>>> +	if (opt.value)
> >>> Would it make sense to allow the caller to affect the poll timeout using
> >>> the other 31 bits of this value?
> >> I was planning to bring that control[1], but it's might be difficult for the caller
> >>
> > 
> > Skimming through the thread, it seems you're discussing how to determine
> > if the DSP supports polling or not; that sounds like a separate problem
> > in my view. Not sure if you settled that discussion, but couldn't that
> > be handled through FASTRPC_IOCTL_GET_DSP_INFO?
> > 
> > I assume though, this would be subject to firmware changes. How do you
> > determine downstream if polling should be used or not today?
> > 
> > 
> > For my specific question here, I'm merely wondering if the timeout value
> > should be a boolean or have a unit. We could punt on that question, to
> > not block this feature from making progress upstream, by defining that
> > only 0 and 1 are valid values today (all other result in -EINVAL).
> > 
> > This would leave the door open for having 0 == off, 1 == default, > 1
> 
> Giving '1' a special non-numerical meaning sounds odd.. maybe 0:default,
> -1:off (or the opposite)?
> 

I guess it comes down to the question of how big the likelihood that you
would want a different value than the default. We should provide sane
defaults and avoid sprinkling unergonomic knobs throughout the system,
but [0,1] and the rest of the bits reserved would leave the door open
for future use of the upper 31 bits.

I find 0 == "enabled" to be unintuitive...

And using -1 means that the bits aren't reserved for future use.

Regards,
Bjorn

> Konrad

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

end of thread, other threads:[~2026-02-19 13:39 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-15 18:21 [PATCH v6 0/4] misc: fastrpc: Add polling mode support Ekansh Gupta
2026-02-15 18:21 ` [PATCH v6 1/4] misc: fastrpc: Move fdlist to invoke context structure Ekansh Gupta
2026-02-16  3:56   ` Bjorn Andersson
2026-02-16  9:19     ` Ekansh Gupta
2026-02-18 14:15       ` Bjorn Andersson
2026-02-15 18:21 ` [PATCH v6 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK Ekansh Gupta
2026-02-16  2:39   ` kernel test robot
2026-02-16 11:08     ` Konrad Dybcio
2026-02-16  3:13   ` kernel test robot
2026-02-15 18:21 ` [PATCH v6 3/4] misc: fastrpc: Expand context ID mask for DSP polling mode support Ekansh Gupta
2026-02-15 18:21 ` [PATCH v6 4/4] misc: fastrpc: Add polling mode support for fastRPC driver Ekansh Gupta
2026-02-16  3:21   ` Bjorn Andersson
2026-02-16  9:06     ` Ekansh Gupta
2026-02-18 14:36       ` Bjorn Andersson
2026-02-18 14:38         ` Konrad Dybcio
2026-02-19 13:39           ` Bjorn Andersson
2026-02-19  7:36   ` Dmitry Baryshkov

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.