Re: [PATCH v10 4/4] misc: fastrpc: Add polling mode support for fastRPC driver

Re: [PATCH v10 4/4] misc: fastrpc: Add polling mode support for fastRPC driver

[Srinivas Kandagatla]

On 4/24/26 9:59 AM, 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      | 155 ++++++++++++++++++++++++++++++++++--
>  include/uapi/misc/fastrpc.h |  25 ++++++
>  2 files changed, 173 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index c4a3547a5c7f..b24845c7f016 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_addr;
>  	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;
> @@ -263,6 +283,7 @@ struct fastrpc_soc_data {
>  	u32 sid_pos;
>  	u32 dma_addr_bits_cdsp;
>  	u32 dma_addr_bits_default;
> +	bool poll_mode_supported;
>  };
>  
>  struct fastrpc_channel_ctx {
> @@ -285,6 +306,7 @@ struct fastrpc_channel_ctx {
>  	struct list_head invoke_interrupted_mmaps;
>  	bool secure;
>  	bool unsigned_support;
> +	bool poll_mode_supported;
>  	u64 dma_mask;
>  	const struct fastrpc_soc_data *soc_data;
>  };
> @@ -308,6 +330,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 +947,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 +1044,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_addr = (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 +1216,61 @@ static int fastrpc_invoke_send(struct fastrpc_session_ctx *sctx,
>  
>  }
>  
> +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_addr
> +	 * or until another path marks the work done.
> +	 */
> +	ret = readl_poll_timeout_atomic(ctx->poll_addr, val,
ctx->poll_addr is not an io address, how are you making sure that
compiler does not optimize this?

> +					(val == FASTRPC_POLL_RESPONSE) || ctx->is_work_done, 1,
> +					FASTRPC_POLL_MAX_TIMEOUT_US);
> +
> +	if (!ret && val == FASTRPC_POLL_RESPONSE) {
> +		ctx->is_work_done = true;

THis will race with dsp callback. Can we


> +		ctx->retval = 0;
Why are we forcing the retrun value here, does this mean DSP POLL
RESPONSE is success?

> +	}
> +
> +	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;
> +
> +	if (ctx->is_polled) {
> +		err = poll_for_remote_response(ctx);
> +		if (!err)
> +			return 0;
> +		/* If polling timed out or failed, move to normal response mode */
> +		ctx->is_polled = false;
> +	}
> +
> +	return fastrpc_wait_for_response(ctx, kernel);
> +}
> +
>  static int fastrpc_internal_invoke(struct fastrpc_user *fl,  u32 kernel,
>  				   u32 handle, u32 sc,
>  				   struct fastrpc_invoke_args *args)
> @@ -1223,13 +1306,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 +1897,33 @@ 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) {
the possible values are either 0 or 1  here, so this does not match what
is in UAPI . Please define these values rather than wild card matching.


> +		if (!fl->cctx->poll_mode_supported)
> +			return -EOPNOTSUPP;
This should be the first check in this function.

> +		fl->poll_mode = true;
> +	} else {
> +		fl->poll_mode = false;
eventhough poll_mode_supported is not supported you are giving user
positive feedback of setting it to off, is this what you wanted here?

> +	}
> +
> +	return 0;
> +}
> +
>  static int fastrpc_get_dsp_info(struct fastrpc_user *fl, char __user *argp)
>  {
>  	struct fastrpc_ioctl_capability cap = {0};
> @@ -2168,6 +2279,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;
> @@ -2323,6 +2437,7 @@ static const struct fastrpc_soc_data kaanapali_soc_data = {
>  	.sid_pos = 56,
>  	.dma_addr_bits_cdsp = 34,
>  	.dma_addr_bits_default = 32,
> +	.poll_mode_supported = true,
>  };
>  
>  static const struct fastrpc_soc_data default_soc_data = {
> @@ -2331,6 +2446,29 @@ static const struct fastrpc_soc_data default_soc_data = {
>  	.dma_addr_bits_default = 32,
>  };
>  
> +/*
> + * Exception list for older platforms that use default_soc_data but whose
> + * DSP firmware supports FastRPC polling mode.
> + *
> + * NOTE: This list is intentionally closed.
> + * Do NOT add new platforms here. New SoCs must advertise polling mode
> + * support via their soc_data.
> + */
> +
> +static const struct of_device_id fastrpc_poll_supported_machines[] __maybe_unused = {
> +	{ .compatible = "qcom,milos" },
> +	{ .compatible = "qcom,qcs8300" },
> +	{ .compatible = "qcom,sa8775p" },
> +	{ .compatible = "qcom,sar2130p" },
> +	{ .compatible = "qcom,sm8450" },
> +	{ .compatible = "qcom,sm8550" },
> +	{ .compatible = "qcom,sm8650" },
> +	{ .compatible = "qcom,sm8750" },
> +	{ .compatible = "qcom,x1e80100" },
> +	{ .compatible = "qcom,x1p42100" },
> +	{},
> +};
> +
>  static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  {
>  	struct device *rdev = &rpdev->dev;
> @@ -2397,6 +2535,8 @@ static int fastrpc_rpmsg_probe(struct rpmsg_device *rpdev)
>  	secure_dsp = !(of_property_read_bool(rdev->of_node, "qcom,non-secure-domain"));
>  	data->secure = secure_dsp;
>  	data->soc_data = soc_data;
> +	data->poll_mode_supported = soc_data->poll_mode_supported ||
> +		of_machine_get_match(fastrpc_poll_supported_machines);
>  
>  	switch (domain_id) {
>  	case ADSP_DOMAIN_ID:
> @@ -2521,6 +2661,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;