* [PATCH v8 0/4] misc: fastrpc: Add polling mode support
@ 2026-04-15 11:25 Ekansh Gupta
2026-04-15 11:25 ` [PATCH v8 1/4] misc: fastrpc: Move fdlist to invoke context structure Ekansh Gupta
` (3 more replies)
0 siblings, 4 replies; 11+ messages in thread
From: Ekansh Gupta @ 2026-04-15 11:25 UTC (permalink / raw)
To: srini, linux-arm-msm
Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd,
dmitry.baryshkov, konrad.dybcio, andersson
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 [v7]: https://lore.kernel.org/all/20260402054923.3061925-1-ekansh.gupta@oss.qualcomm.com/
Changes in v8:
- Added more comments.
Changes in v7:
- Fixed warnings.
- Fixed commit text.
- Addressed clean-up comments.
Changes in v6:
- 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 | 159 +++++++++++++++++++++++++++++++-----
include/uapi/misc/fastrpc.h | 25 ++++++
2 files changed, 164 insertions(+), 20 deletions(-)
--
2.34.1
^ permalink raw reply [flat|nested] 11+ messages in thread* [PATCH v8 1/4] misc: fastrpc: Move fdlist to invoke context structure 2026-04-15 11:25 [PATCH v8 0/4] misc: fastrpc: Add polling mode support Ekansh Gupta @ 2026-04-15 11:25 ` Ekansh Gupta 2026-04-15 11:25 ` [PATCH v8 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK Ekansh Gupta ` (2 subsequent siblings) 3 siblings, 0 replies; 11+ messages in thread From: Ekansh Gupta @ 2026-04-15 11:25 UTC (permalink / raw) To: srini, linux-arm-msm Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd, dmitry.baryshkov, konrad.dybcio, andersson The fdlist is currently part of the meta buffer which is set during fastrpc_get_args(), this fdlist is getting recalculated during fastrpc_put_args(). 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 | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index 1080f9acf70a..a9f507a88c67 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; @@ -1016,6 +1017,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; @@ -1118,18 +1120,11 @@ 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; + u64 *fdlist = ctx->fdlist; + 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]) { -- 2.34.1 ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v8 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK 2026-04-15 11:25 [PATCH v8 0/4] misc: fastrpc: Add polling mode support Ekansh Gupta 2026-04-15 11:25 ` [PATCH v8 1/4] misc: fastrpc: Move fdlist to invoke context structure Ekansh Gupta @ 2026-04-15 11:25 ` Ekansh Gupta 2026-04-15 11:25 ` [PATCH v8 3/4] misc: fastrpc: Expand context ID mask for DSP polling mode support Ekansh Gupta 2026-04-15 11:25 ` [PATCH v8 4/4] misc: fastrpc: Add polling mode support for fastRPC driver Ekansh Gupta 3 siblings, 0 replies; 11+ messages in thread From: Ekansh Gupta @ 2026-04-15 11:25 UTC (permalink / raw) To: srini, linux-arm-msm Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd, dmitry.baryshkov, konrad.dybcio, andersson 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 | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index a9f507a88c67..3f5d5d73be5a 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -23,6 +23,7 @@ #include <uapi/misc/fastrpc.h> #include <linux/of_reserved_mem.h> #include <linux/bits.h> +#include <linux/bitops.h> #define ADSP_DOMAIN_ID (0) #define MDSP_DOMAIN_ID (1) @@ -37,7 +38,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 +516,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); @@ -649,7 +650,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); @@ -2508,7 +2509,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 [flat|nested] 11+ messages in thread
* [PATCH v8 3/4] misc: fastrpc: Expand context ID mask for DSP polling mode support 2026-04-15 11:25 [PATCH v8 0/4] misc: fastrpc: Add polling mode support Ekansh Gupta 2026-04-15 11:25 ` [PATCH v8 1/4] misc: fastrpc: Move fdlist to invoke context structure Ekansh Gupta 2026-04-15 11:25 ` [PATCH v8 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK Ekansh Gupta @ 2026-04-15 11:25 ` Ekansh Gupta 2026-04-15 11:25 ` [PATCH v8 4/4] misc: fastrpc: Add polling mode support for fastRPC driver Ekansh Gupta 3 siblings, 0 replies; 11+ messages in thread From: Ekansh Gupta @ 2026-04-15 11:25 UTC (permalink / raw) To: srini, linux-arm-msm Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd, dmitry.baryshkov, konrad.dybcio, andersson 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 3f5d5d73be5a..c4a3547a5c7f 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -38,7 +38,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 [flat|nested] 11+ messages in thread
* [PATCH v8 4/4] misc: fastrpc: Add polling mode support for fastRPC driver 2026-04-15 11:25 [PATCH v8 0/4] misc: fastrpc: Add polling mode support Ekansh Gupta ` (2 preceding siblings ...) 2026-04-15 11:25 ` [PATCH v8 3/4] misc: fastrpc: Expand context ID mask for DSP polling mode support Ekansh Gupta @ 2026-04-15 11:25 ` Ekansh Gupta 2026-04-16 8:17 ` Luben Tuikov 2026-04-16 10:55 ` Dmitry Baryshkov 3 siblings, 2 replies; 11+ messages in thread From: Ekansh Gupta @ 2026-04-15 11:25 UTC (permalink / raw) To: srini, linux-arm-msm Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd, dmitry.baryshkov, konrad.dybcio, andersson 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. In case poll timeout happens, the call will fallback to normal RPC mode. 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 | 137 ++++++++++++++++++++++++++++++++++-- include/uapi/misc/fastrpc.h | 25 +++++++ 2 files changed, 155 insertions(+), 7 deletions(-) diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c index c4a3547a5c7f..5311a4ba4bb7 100644 --- a/drivers/misc/fastrpc.c +++ b/drivers/misc/fastrpc.c @@ -24,6 +24,8 @@ #include <linux/of_reserved_mem.h> #include <linux/bits.h> #include <linux/bitops.h> +#include <linux/compiler.h> +#include <linux/iopoll.h> #define ADSP_DOMAIN_ID (0) #define MDSP_DOMAIN_ID (1) @@ -38,6 +40,12 @@ #define FASTRPC_CTX_MAX (256) #define FASTRPC_INIT_HANDLE 1 #define FASTRPC_DSP_UTILITIES_HANDLE 2 +/* + * Maximum handle value for static handles. + * Static handles are pre-defined, fixed numeric values statically assigned + * in the IDL file or FastRPC framework. + */ +#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) @@ -106,6 +114,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 */ @@ -236,8 +250,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; @@ -308,6 +328,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 */ @@ -923,7 +945,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; } @@ -1019,6 +1042,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 +1214,74 @@ 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 0; +} + static int fastrpc_internal_invoke(struct fastrpc_user *fl, u32 kernel, u32 handle, u32 sc, struct fastrpc_invoke_args *args) @@ -1223,13 +1317,14 @@ 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; @@ -1813,6 +1908,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.request_id != 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}; @@ -2168,6 +2287,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; @@ -2521,6 +2643,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..63346e27d5e9 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,24 @@ enum fastrpc_proc_attr { /* Fastrpc attribute for memory protection of buffers */ #define FASTRPC_ATTR_SECUREMAP (1) +/** + * FASTRPC_POLL_MODE - Enable/disable poll mode for FastRPC invocations + * + * Poll mode is an optimization that allows the CPU to poll shared memory + * for completion instead of waiting for an interrupt-based response. + * This reduces latency for fast-completing operations. + * + * Restrictions: + * - Only supported for USER_PD (User Protection Domain) + * - Only applies to dynamic modules (handle > 20) + * - Static modules always use interrupt-based completion + * + * Values: + * - 0: Disable poll mode (use interrupt-based completion) + * - 1: Enable poll mode (poll shared memory for completion) + */ +#define FASTRPC_POLL_MODE (1) + struct fastrpc_invoke_args { __u64 ptr; __u64 length; @@ -133,6 +152,12 @@ struct fastrpc_mem_unmap { __s32 reserved[5]; }; +struct fastrpc_ioctl_set_option { + __u32 request_id; /* Request type (e.g., FASTRPC_POLL_MODE) */ + __u32 value; /* Request-specific 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] 11+ messages in thread
* Re: [PATCH v8 4/4] misc: fastrpc: Add polling mode support for fastRPC driver 2026-04-15 11:25 ` [PATCH v8 4/4] misc: fastrpc: Add polling mode support for fastRPC driver Ekansh Gupta @ 2026-04-16 8:17 ` Luben Tuikov 2026-04-16 13:58 ` Ekansh Gupta 2026-04-16 10:55 ` Dmitry Baryshkov 1 sibling, 1 reply; 11+ messages in thread From: Luben Tuikov @ 2026-04-16 8:17 UTC (permalink / raw) To: Ekansh Gupta, srini, linux-arm-msm Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd, dmitry.baryshkov, konrad.dybcio, andersson [-- Attachment #1.1.1: Type: text/plain, Size: 7161 bytes --] Hi Ekansh, Good work. A couple of notes below: On 2026-04-15 07:25, 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. In case > poll timeout happens, the call will fallback to normal RPC mode. 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 | 137 ++++++++++++++++++++++++++++++++++-- > include/uapi/misc/fastrpc.h | 25 +++++++ > 2 files changed, 155 insertions(+), 7 deletions(-) > > diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c > index c4a3547a5c7f..5311a4ba4bb7 100644 > --- a/drivers/misc/fastrpc.c > +++ b/drivers/misc/fastrpc.c > @@ -24,6 +24,8 @@ > #include <linux/of_reserved_mem.h> > #include <linux/bits.h> > #include <linux/bitops.h> > +#include <linux/compiler.h> > +#include <linux/iopoll.h> > > #define ADSP_DOMAIN_ID (0) > #define MDSP_DOMAIN_ID (1) > @@ -38,6 +40,12 @@ > #define FASTRPC_CTX_MAX (256) > #define FASTRPC_INIT_HANDLE 1 > #define FASTRPC_DSP_UTILITIES_HANDLE 2 > +/* > + * Maximum handle value for static handles. > + * Static handles are pre-defined, fixed numeric values statically assigned > + * in the IDL file or FastRPC framework. > + */ > +#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) > @@ -106,6 +114,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 */ > @@ -236,8 +250,14 @@ struct fastrpc_invoke_ctx { > u32 sc; > u64 *fdlist; > u32 *crc; > + /* Poll memory that DSP updates */ > + u32 *poll; Perhaps "poll_addr"? "poll" seems just too generic. > 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; > @@ -308,6 +328,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 */ > @@ -923,7 +945,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; > } > @@ -1019,6 +1042,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 +1214,74 @@ 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); I think you're better off using readl() here, but see my comment below, which obviates this function. > +} > + > +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); Is there any reason you're not using readl_poll_timeout_atomic() as documented in linux/iopoll.h? Does readl() not satisfy the read operation in fastrpc_poll_op()? How can ctx->is_work_done be updated here? Perhaps you just want to use "val == FASTRPC_POLL_RESPONSE" as a condition here... > + > + 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) What is "kernel" and why is it a u32 when it is used as a "bool"? Perhaps a better name can be had? > +{ > + 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); Perhaps you want to also check "err" here to make the exit condition more explicit. (The invariant in do-while loops is generally directly determined by something within the loop and generally not implicit.) Is it possible that in poll_for_remote_response() you get 0 as a poll result and val is not equal to FASTRCPC_POLL_RESPONSE? In such a case, this may hang. (Is a hang desired here?) Is it possible that if polling is enabled, then you want to poll only once, and if unsuccessful, or successful but "!work_done", then transition to fastrpc_wait_for_response() and return, without looping? (since polling is looping after all...) > + > + return 0; "err" is always initialized so you can return "err" here if you exit with "err" as part of the exit condition. (And if you add "!err &&" in the loop invariant, then you don't need (if (err) return err;) after "fastrpc_wait_for_response()"). -- Regards, Luben [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 677 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 236 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 4/4] misc: fastrpc: Add polling mode support for fastRPC driver 2026-04-16 8:17 ` Luben Tuikov @ 2026-04-16 13:58 ` Ekansh Gupta 2026-04-17 3:54 ` Luben Tuikov 0 siblings, 1 reply; 11+ messages in thread From: Ekansh Gupta @ 2026-04-16 13:58 UTC (permalink / raw) To: Luben Tuikov, srini, linux-arm-msm Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd, dmitry.baryshkov, konrad.dybcio, andersson On 16-04-2026 13:47, Luben Tuikov wrote: > Hi Ekansh, > > Good work. A couple of notes below: > > On 2026-04-15 07:25, 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. In case >> poll timeout happens, the call will fallback to normal RPC mode. 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 | 137 ++++++++++++++++++++++++++++++++++-- >> include/uapi/misc/fastrpc.h | 25 +++++++ >> 2 files changed, 155 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c >> index c4a3547a5c7f..5311a4ba4bb7 100644 >> --- a/drivers/misc/fastrpc.c >> +++ b/drivers/misc/fastrpc.c >> @@ -24,6 +24,8 @@ >> #include <linux/of_reserved_mem.h> >> #include <linux/bits.h> >> #include <linux/bitops.h> >> +#include <linux/compiler.h> >> +#include <linux/iopoll.h> >> >> #define ADSP_DOMAIN_ID (0) >> #define MDSP_DOMAIN_ID (1) >> @@ -38,6 +40,12 @@ >> #define FASTRPC_CTX_MAX (256) >> #define FASTRPC_INIT_HANDLE 1 >> #define FASTRPC_DSP_UTILITIES_HANDLE 2 >> +/* >> + * Maximum handle value for static handles. >> + * Static handles are pre-defined, fixed numeric values statically assigned >> + * in the IDL file or FastRPC framework. >> + */ >> +#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) >> @@ -106,6 +114,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 */ >> @@ -236,8 +250,14 @@ struct fastrpc_invoke_ctx { >> u32 sc; >> u64 *fdlist; >> u32 *crc; >> + /* Poll memory that DSP updates */ >> + u32 *poll; > > Perhaps "poll_addr"? "poll" seems just too generic. ack > >> 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; >> @@ -308,6 +328,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 */ >> @@ -923,7 +945,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; >> } >> @@ -1019,6 +1042,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 +1214,74 @@ 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); > > I think you're better off using readl() here, but see my comment below, which obviates this function. > >> +} >> + >> +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); > > Is there any reason you're not using readl_poll_timeout_atomic() as documented in linux/iopoll.h? > Does readl() not satisfy the read operation in fastrpc_poll_op()? I didn't check this, I'll try this out. > > How can ctx->is_work_done be updated here? Perhaps you just want to use "val == FASTRPC_POLL_RESPONSE" as a condition here... That is to handle the possibility that normal response comes while the polling is ongoing. In that case, the call will get completed by fastrpc_rpmsg_callback. > >> + >> + 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) > > What is "kernel" and why is it a u32 when it is used as a "bool"? Perhaps a better name can be had? This reflects kernel message. As of now, just propagated the same that is used across the driver, maybe can address this as a separate patch. > >> +{ >> + 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); > > Perhaps you want to also check "err" here to make the exit condition more explicit. (The invariant in do-while loops is generally directly determined by something within the loop and generally not implicit.) The reason to not keep "err" check is because the call should fallback to normal response(fastrpc_wait_for_response()) in case poll_for_remote_response() fails. > > Is it possible that in poll_for_remote_response() you get 0 as a poll result and val is not equal to FASTRCPC_POLL_RESPONSE? In such a case, this may hang. (Is a hang desired here?) That's actually a good point, let me try making it more robust, this condition might get encountered in case normal response is sent instead of poll memory update. > > Is it possible that if polling is enabled, then you want to poll only once, and if unsuccessful, or successful but "!work_done", then transition to fastrpc_wait_for_response() and return, without looping? (since polling is looping after all...) This is correct, the intention is the poll until it returns, continue if successful and fallback to normal response if unsuccessful. > >> + >> + return 0; > > "err" is always initialized so you can return "err" here if you exit with "err" as part of the exit condition. (And if you add "!err &&" in the loop invariant, then you don't need (if (err) return err;) after "fastrpc_wait_for_response()"). I'll be keeping this unchanged if I don't end up adding "!err &&" check here, as err is always going to be zero here as suggested in earlier version[1]. Thanks, Luben, for taking the time to review this change and for providing insightful comments. [1] https://lore.kernel.org/all/wipphezpxtuuxtwhpwamsmvhwgwuesexmy5ev5pcqb65vov5kz@vuzzyyqnu7ci/ > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 4/4] misc: fastrpc: Add polling mode support for fastRPC driver 2026-04-16 13:58 ` Ekansh Gupta @ 2026-04-17 3:54 ` Luben Tuikov 0 siblings, 0 replies; 11+ messages in thread From: Luben Tuikov @ 2026-04-17 3:54 UTC (permalink / raw) To: Ekansh Gupta, srini, linux-arm-msm Cc: gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd, dmitry.baryshkov, konrad.dybcio, andersson [-- Attachment #1.1.1: Type: text/plain, Size: 3463 bytes --] On 2026-04-16 09:58, Ekansh Gupta wrote: > On 16-04-2026 13:47, Luben Tuikov wrote: >> Hi Ekansh, >> >> Good work. A couple of notes below: --cut--->>> +static inline int fastrpc_wait_for_response(struct fastrpc_invoke_ctx *ctx, >>> + u32 kernel) >> >> What is "kernel" and why is it a u32 when it is used as a "bool"? Perhaps a better name can be had? > This reflects kernel message. As of now, just propagated the same that > is used across the driver, maybe can address this as a separate patch. I can see that its origin is internal to the driver, as a boolean. Perhaps "kernel_message" or "kmessage" or something descriptive like that. I think it's more important that it is a "message", rather than "kernel message". Yes, a separate patch indeed makes sense for this. --cut--->>> +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); >> >> Perhaps you want to also check "err" here to make the exit condition more explicit. (The invariant in do-while loops is generally directly determined by something within the loop and generally not implicit.) > The reason to not keep "err" check is because the call should fallback > to normal response(fastrpc_wait_for_response()) in case > poll_for_remote_response() fails. >> >> Is it possible that in poll_for_remote_response() you get 0 as a poll result and val is not equal to FASTRCPC_POLL_RESPONSE? In such a case, this may hang. (Is a hang desired here?) > That's actually a good point, let me try making it more robust, this > condition might get encountered in case normal response is sent instead > of poll memory update. Right. We want to avoid this dependency. If the device hangs for whatever reason (defective device, cosmic ray, etc.) this should not result in a process or a kernel execution context hanging. >> Is it possible that if polling is enabled, then you want to poll only once, and if unsuccessful, or successful but "!work_done", then transition to fastrpc_wait_for_response() and return, without looping? (since polling is looping after all...) > This is correct, the intention is the poll until it returns, continue if > successful and fallback to normal response if unsuccessful. Right. So this was obvious by reading the contents of the do-while loop. If you prefer, you can remove the do-while loop, or at least take out the poll_for_remote_response() out, and only leave the fastrpc_wait_for_response() inside the loop, and decide how many time intervals you want to wait. If that is once, then you don't need the do-while loop. (Effectively, you've waited once in the poll and 2nd time in the fast_wait_for_response().) We just want to avoid hangs. > Thanks, Luben, for taking the time to review this change and for > providing insightful comments. Yes, no problem. Good work and thank you for your work and contribution! > [1] > https://lore.kernel.org/all/wipphezpxtuuxtwhpwamsmvhwgwuesexmy5ev5pcqb65vov5kz@vuzzyyqnu7ci/ Ah, thank you for this reference. -- Regards, Luben [-- Attachment #1.1.2: OpenPGP public key --] [-- Type: application/pgp-keys, Size: 677 bytes --] [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 236 bytes --] ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 4/4] misc: fastrpc: Add polling mode support for fastRPC driver 2026-04-15 11:25 ` [PATCH v8 4/4] misc: fastrpc: Add polling mode support for fastRPC driver Ekansh Gupta 2026-04-16 8:17 ` Luben Tuikov @ 2026-04-16 10:55 ` Dmitry Baryshkov 2026-04-16 14:02 ` Ekansh Gupta 1 sibling, 1 reply; 11+ messages in thread From: Dmitry Baryshkov @ 2026-04-16 10:55 UTC (permalink / raw) To: Ekansh Gupta Cc: srini, linux-arm-msm, gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd, konrad.dybcio, andersson On Wed, Apr 15, 2026 at 04:55:30PM +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. In case > poll timeout happens, the call will fallback to normal RPC mode. 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 | 137 ++++++++++++++++++++++++++++++++++-- > include/uapi/misc/fastrpc.h | 25 +++++++ > 2 files changed, 155 insertions(+), 7 deletions(-) > > @@ -1813,6 +1908,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.request_id != FASTRPC_POLL_MODE) > + return -EINVAL; > + > + if (opt.value) > + fl->poll_mode = true; This will enable poll mode on the platforms where it is not supported, later silinently changing back to the normal mode. Please don't surprise users and make this call fail if polling mode is not supported. > + else > + fl->poll_mode = false; > + > + return 0; > +} > + -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 4/4] misc: fastrpc: Add polling mode support for fastRPC driver 2026-04-16 10:55 ` Dmitry Baryshkov @ 2026-04-16 14:02 ` Ekansh Gupta 2026-04-17 13:18 ` Dmitry Baryshkov 0 siblings, 1 reply; 11+ messages in thread From: Ekansh Gupta @ 2026-04-16 14:02 UTC (permalink / raw) To: Dmitry Baryshkov Cc: srini, linux-arm-msm, gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd, konrad.dybcio, andersson On 16-04-2026 16:25, Dmitry Baryshkov wrote: > On Wed, Apr 15, 2026 at 04:55:30PM +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. In case >> poll timeout happens, the call will fallback to normal RPC mode. 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 | 137 ++++++++++++++++++++++++++++++++++-- >> include/uapi/misc/fastrpc.h | 25 +++++++ >> 2 files changed, 155 insertions(+), 7 deletions(-) >> >> @@ -1813,6 +1908,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.request_id != FASTRPC_POLL_MODE) >> + return -EINVAL; >> + >> + if (opt.value) >> + fl->poll_mode = true; > > This will enable poll mode on the platforms where it is not supported, > later silinently changing back to the normal mode. Please don't surprise > users and make this call fail if polling mode is not supported. Ack. Working on adding compatible based checks for supporting platforms by reading the root compatible string, something like pdmapper[1]. [1] https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/soc/qcom/qcom_pd_mapper.c#n643 Thanks, Ekansh > >> + else >> + fl->poll_mode = false; >> + >> + return 0; >> +} >> + > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH v8 4/4] misc: fastrpc: Add polling mode support for fastRPC driver 2026-04-16 14:02 ` Ekansh Gupta @ 2026-04-17 13:18 ` Dmitry Baryshkov 0 siblings, 0 replies; 11+ messages in thread From: Dmitry Baryshkov @ 2026-04-17 13:18 UTC (permalink / raw) To: Ekansh Gupta Cc: srini, linux-arm-msm, gregkh, quic_bkumar, linux-kernel, quic_chennak, dri-devel, arnd, konrad.dybcio, andersson On Thu, Apr 16, 2026 at 07:32:43PM +0530, Ekansh Gupta wrote: > On 16-04-2026 16:25, Dmitry Baryshkov wrote: > > On Wed, Apr 15, 2026 at 04:55:30PM +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. In case > >> poll timeout happens, the call will fallback to normal RPC mode. 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 | 137 ++++++++++++++++++++++++++++++++++-- > >> include/uapi/misc/fastrpc.h | 25 +++++++ > >> 2 files changed, 155 insertions(+), 7 deletions(-) > >> > >> @@ -1813,6 +1908,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.request_id != FASTRPC_POLL_MODE) > >> + return -EINVAL; > >> + > >> + if (opt.value) > >> + fl->poll_mode = true; > > > > This will enable poll mode on the platforms where it is not supported, > > later silinently changing back to the normal mode. Please don't surprise > > users and make this call fail if polling mode is not supported. > Ack. Working on adding compatible based checks for supporting platforms > by reading the root compatible string, something like pdmapper[1]. SGTM > > [1] > https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/drivers/soc/qcom/qcom_pd_mapper.c#n643 > > Thanks, > Ekansh > > > > >> + else > >> + fl->poll_mode = false; > >> + > >> + return 0; > >> +} > >> + > > > -- With best wishes Dmitry ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-04-17 13:18 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-04-15 11:25 [PATCH v8 0/4] misc: fastrpc: Add polling mode support Ekansh Gupta 2026-04-15 11:25 ` [PATCH v8 1/4] misc: fastrpc: Move fdlist to invoke context structure Ekansh Gupta 2026-04-15 11:25 ` [PATCH v8 2/4] misc: fastrpc: Replace hardcoded ctxid mask with GENMASK Ekansh Gupta 2026-04-15 11:25 ` [PATCH v8 3/4] misc: fastrpc: Expand context ID mask for DSP polling mode support Ekansh Gupta 2026-04-15 11:25 ` [PATCH v8 4/4] misc: fastrpc: Add polling mode support for fastRPC driver Ekansh Gupta 2026-04-16 8:17 ` Luben Tuikov 2026-04-16 13:58 ` Ekansh Gupta 2026-04-17 3:54 ` Luben Tuikov 2026-04-16 10:55 ` Dmitry Baryshkov 2026-04-16 14:02 ` Ekansh Gupta 2026-04-17 13:18 ` Dmitry Baryshkov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®