Re: [PATCH] misc: fastrpc: add ioctl for attaching to sensors pd
On 07/09/2020 15:02, Jonathan Marek wrote: Srini do you have any suggestions for how to name these values? These are domain id corresponding to each core. you can use SDSP_DOMAIN_ID in here! these are already defined in the file as: #define ADSP_DOMAIN_ID (0) #define MDSP_DOMAIN_ID (1) #define SDSP_DOMAIN_ID (2) #define CDSP_DOMAIN_ID (3) I don't think this is right: FASTRPC_IOCTL_INIT_ATTACH uses pd = 0 FASTRPC_IOCTL_INIT_CREATE uses pd = 1 And these two ioctl are used with all DSP cores. So it wouldn't make sense for the pd value to correspond to the domain id. You are right, values are pretty much similar to domain ids but not exactly the same as Protection Domain(PD) ids. I spoke to qcom guys about this, and this is what I have. 0 is Audio Process PD 1 is Dynamic User PD, cases like SNPE or CV 2 is Sensor Process PD. Hope this helps! --srini
Re: [PATCH] misc: fastrpc: add ioctl for attaching to sensors pd
On 01/09/2020 01:32, Jonathan Marek wrote:
-#define FASTRPC_IOCTL_MMAP _IOWR('R', 6, struct fastrpc_req_mmap)
-#define FASTRPC_IOCTL_MUNMAP_IOWR('R', 7, struct
fastrpc_req_munmap)
+#define FASTRPC_IOCTL_MMAP _IOWR('R', 6, struct fastrpc_req_mmap)
+#define FASTRPC_IOCTL_MUNMAP _IOWR('R', 7, struct fastrpc_req_munmap)
Looks like changes that do not belong to this patch!
I wanted to try this patch on SM8250.
How do you test attaching fastrpc to sensor core?, I mean which
userspace lib/tool do you use?
--srini
+#define FASTRPC_IOCTL_INIT_ATTACH_SNS _IO('R', 8)
Re: [PATCH] misc: fastrpc: add ioctl for attaching to sensors pd
On 9/7/20 8:33 AM, Greg Kroah-Hartman wrote:
On Mon, Aug 31, 2020 at 08:32:59PM -0400, Jonathan Marek wrote:
Initializing sensors requires attaching to pd 2. Add an ioctl for that.
This corresponds to FASTRPC_INIT_ATTACH_SENSORS in the downstream driver.
Signed-off-by: Jonathan Marek
---
drivers/misc/fastrpc.c | 9 ++---
include/uapi/misc/fastrpc.h | 5 +++--
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 7939c55daceb..ea5e9ca0d705 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1276,7 +1276,7 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user *fl,
char __user *argp)
return 0;
}
-static int fastrpc_init_attach(struct fastrpc_user *fl)
+static int fastrpc_init_attach(struct fastrpc_user *fl, int pd)
{
struct fastrpc_invoke_args args[1];
int tgid = fl->tgid;
@@ -1287,7 +1287,7 @@ static int fastrpc_init_attach(struct fastrpc_user *fl)
args[0].fd = -1;
args[0].reserved = 0;
sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_ATTACH, 1, 0);
- fl->pd = 0;
+ fl->pd = pd;
return fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE,
sc, [0]);
@@ -1477,7 +1477,10 @@ static long fastrpc_device_ioctl(struct file *file,
unsigned int cmd,
err = fastrpc_invoke(fl, argp);
break;
case FASTRPC_IOCTL_INIT_ATTACH:
- err = fastrpc_init_attach(fl);
+ err = fastrpc_init_attach(fl, 0);
+ break;
+ case FASTRPC_IOCTL_INIT_ATTACH_SNS:
+ err = fastrpc_init_attach(fl, 2);
Shouldn't you have #defines for those magic numbers somewhere? What
does 0 and 2 mean?
This is based off a downstream driver which also uses magic numbers,
although I can make an educated guess about the meaning.
Srini do you have any suggestions for how to name these values?
thanks,
greg k-h
Re: [PATCH] misc: fastrpc: add ioctl for attaching to sensors pd
On 07/09/2020 14:47, Jonathan Marek wrote:
On 9/7/20 8:36 AM, Srinivas Kandagatla wrote:
On 01/09/2020 01:32, Jonathan Marek wrote:
-#define FASTRPC_IOCTL_MMAP _IOWR('R', 6, struct
fastrpc_req_mmap)
-#define FASTRPC_IOCTL_MUNMAP _IOWR('R', 7, struct
fastrpc_req_munmap)
+#define FASTRPC_IOCTL_MMAP _IOWR('R', 6, struct
fastrpc_req_mmap)
+#define FASTRPC_IOCTL_MUNMAP _IOWR('R', 7, struct
fastrpc_req_munmap)
Looks like changes that do not belong to this patch!
I wanted to try this patch on SM8250.
How do you test attaching fastrpc to sensor core?, I mean which
userspace lib/tool do you use?
--srini
I pushed my sdsprpcd implementation to github, which is responsible for
initializing the sensors, and uses this ioctl:
https://github.com/flto/fastrpc
Thanks!, I can take a look and see if I can try it out with linaro
fastrpc library!
Note: it uses my own WIP fastrpc "library" instead of the one from
linaro, I also have other related code, like a sensor client, and
cDSP/aDSP compute examples, but need to confirm that I can share them
Also, the corresponding dts patch I sent has a problem, the label =
"dsps"; should be label = "sdsp"; (copied the "dsps" from downstream,
but upstream expects "sdsp"), will send a v2 later today.
Also the dts patch will fail to apply as it is, as it seems me that you
have based the patch after adding audio dts patch!
--srini
+#define FASTRPC_IOCTL_INIT_ATTACH_SNS _IO('R', 8)
Re: [PATCH] misc: fastrpc: add ioctl for attaching to sensors pd
On 07/09/2020 14:51, Jonathan Marek wrote: @@ -1477,7 +1477,10 @@ static long fastrpc_device_ioctl(struct file *file, unsigned int cmd, err = fastrpc_invoke(fl, argp); break; case FASTRPC_IOCTL_INIT_ATTACH: - err = fastrpc_init_attach(fl); + err = fastrpc_init_attach(fl, 0); + break; + case FASTRPC_IOCTL_INIT_ATTACH_SNS: + err = fastrpc_init_attach(fl, 2); Shouldn't you have #defines for those magic numbers somewhere? What does 0 and 2 mean? This is based off a downstream driver which also uses magic numbers, although I can make an educated guess about the meaning. Srini do you have any suggestions for how to name these values? These are domain id corresponding to each core. you can use SDSP_DOMAIN_ID in here! these are already defined in the file as: #define ADSP_DOMAIN_ID (0) #define MDSP_DOMAIN_ID (1) #define SDSP_DOMAIN_ID (2) #define CDSP_DOMAIN_ID (3) --srini
Re: [PATCH] misc: fastrpc: add ioctl for attaching to sensors pd
On 9/7/20 9:58 AM, Srinivas Kandagatla wrote: On 07/09/2020 14:51, Jonathan Marek wrote: @@ -1477,7 +1477,10 @@ static long fastrpc_device_ioctl(struct file *file, unsigned int cmd, err = fastrpc_invoke(fl, argp); break; case FASTRPC_IOCTL_INIT_ATTACH: - err = fastrpc_init_attach(fl); + err = fastrpc_init_attach(fl, 0); + break; + case FASTRPC_IOCTL_INIT_ATTACH_SNS: + err = fastrpc_init_attach(fl, 2); Shouldn't you have #defines for those magic numbers somewhere? What does 0 and 2 mean? This is based off a downstream driver which also uses magic numbers, although I can make an educated guess about the meaning. Srini do you have any suggestions for how to name these values? These are domain id corresponding to each core. you can use SDSP_DOMAIN_ID in here! these are already defined in the file as: #define ADSP_DOMAIN_ID (0) #define MDSP_DOMAIN_ID (1) #define SDSP_DOMAIN_ID (2) #define CDSP_DOMAIN_ID (3) I don't think this is right: FASTRPC_IOCTL_INIT_ATTACH uses pd = 0 FASTRPC_IOCTL_INIT_CREATE uses pd = 1 And these two ioctl are used with all DSP cores. So it wouldn't make sense for the pd value to correspond to the domain id. --srini
Re: [PATCH] misc: fastrpc: add ioctl for attaching to sensors pd
On 9/7/20 10:01 AM, Srinivas Kandagatla wrote:
On 07/09/2020 14:47, Jonathan Marek wrote:
On 9/7/20 8:36 AM, Srinivas Kandagatla wrote:
On 01/09/2020 01:32, Jonathan Marek wrote:
-#define FASTRPC_IOCTL_MMAP _IOWR('R', 6, struct
fastrpc_req_mmap)
-#define FASTRPC_IOCTL_MUNMAP _IOWR('R', 7, struct
fastrpc_req_munmap)
+#define FASTRPC_IOCTL_MMAP _IOWR('R', 6, struct
fastrpc_req_mmap)
+#define FASTRPC_IOCTL_MUNMAP _IOWR('R', 7, struct
fastrpc_req_munmap)
Looks like changes that do not belong to this patch!
I wanted to try this patch on SM8250.
How do you test attaching fastrpc to sensor core?, I mean which
userspace lib/tool do you use?
--srini
I pushed my sdsprpcd implementation to github, which is responsible
for initializing the sensors, and uses this ioctl:
https://github.com/flto/fastrpc
Thanks!, I can take a look and see if I can try it out with linaro
fastrpc library!
You don't need linaro fastrpc library to try it, everything you need is
in that repo.
Note: it uses my own WIP fastrpc "library" instead of the one from
linaro, I also have other related code, like a sensor client, and
cDSP/aDSP compute examples, but need to confirm that I can share them
Also, the corresponding dts patch I sent has a problem, the label =
"dsps"; should be label = "sdsp"; (copied the "dsps" from downstream,
but upstream expects "sdsp"), will send a v2 later today.
Also the dts patch will fail to apply as it is, as it seems me that you
have based the patch after adding audio dts patch!
Thanks for pointing it out, will make sure the v2 applies cleanly
without audio dts patches applied.
--srini
+#define FASTRPC_IOCTL_INIT_ATTACH_SNS _IO('R', 8)
Re: [PATCH] misc: fastrpc: add ioctl for attaching to sensors pd
On 9/7/20 8:36 AM, Srinivas Kandagatla wrote:
On 01/09/2020 01:32, Jonathan Marek wrote:
-#define FASTRPC_IOCTL_MMAP _IOWR('R', 6, struct
fastrpc_req_mmap)
-#define FASTRPC_IOCTL_MUNMAP _IOWR('R', 7, struct
fastrpc_req_munmap)
+#define FASTRPC_IOCTL_MMAP _IOWR('R', 6, struct fastrpc_req_mmap)
+#define FASTRPC_IOCTL_MUNMAP _IOWR('R', 7, struct
fastrpc_req_munmap)
Looks like changes that do not belong to this patch!
I wanted to try this patch on SM8250.
How do you test attaching fastrpc to sensor core?, I mean which
userspace lib/tool do you use?
--srini
I pushed my sdsprpcd implementation to github, which is responsible for
initializing the sensors, and uses this ioctl:
https://github.com/flto/fastrpc
Note: it uses my own WIP fastrpc "library" instead of the one from
linaro, I also have other related code, like a sensor client, and
cDSP/aDSP compute examples, but need to confirm that I can share them
Also, the corresponding dts patch I sent has a problem, the label =
"dsps"; should be label = "sdsp"; (copied the "dsps" from downstream,
but upstream expects "sdsp"), will send a v2 later today.
+#define FASTRPC_IOCTL_INIT_ATTACH_SNS _IO('R', 8)
Re: [PATCH] misc: fastrpc: add ioctl for attaching to sensors pd
On Mon, Aug 31, 2020 at 08:32:59PM -0400, Jonathan Marek wrote:
> Initializing sensors requires attaching to pd 2. Add an ioctl for that.
>
> This corresponds to FASTRPC_INIT_ATTACH_SENSORS in the downstream driver.
>
> Signed-off-by: Jonathan Marek
> ---
> drivers/misc/fastrpc.c | 9 ++---
> include/uapi/misc/fastrpc.h | 5 +++--
> 2 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
> index 7939c55daceb..ea5e9ca0d705 100644
> --- a/drivers/misc/fastrpc.c
> +++ b/drivers/misc/fastrpc.c
> @@ -1276,7 +1276,7 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user
> *fl, char __user *argp)
> return 0;
> }
>
> -static int fastrpc_init_attach(struct fastrpc_user *fl)
> +static int fastrpc_init_attach(struct fastrpc_user *fl, int pd)
> {
> struct fastrpc_invoke_args args[1];
> int tgid = fl->tgid;
> @@ -1287,7 +1287,7 @@ static int fastrpc_init_attach(struct fastrpc_user *fl)
> args[0].fd = -1;
> args[0].reserved = 0;
> sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_ATTACH, 1, 0);
> - fl->pd = 0;
> + fl->pd = pd;
>
> return fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE,
> sc, [0]);
> @@ -1477,7 +1477,10 @@ static long fastrpc_device_ioctl(struct file *file,
> unsigned int cmd,
> err = fastrpc_invoke(fl, argp);
> break;
> case FASTRPC_IOCTL_INIT_ATTACH:
> - err = fastrpc_init_attach(fl);
> + err = fastrpc_init_attach(fl, 0);
> + break;
> + case FASTRPC_IOCTL_INIT_ATTACH_SNS:
> + err = fastrpc_init_attach(fl, 2);
Shouldn't you have #defines for those magic numbers somewhere? What
does 0 and 2 mean?
thanks,
greg k-h
[PATCH] misc: fastrpc: add ioctl for attaching to sensors pd
Initializing sensors requires attaching to pd 2. Add an ioctl for that.
This corresponds to FASTRPC_INIT_ATTACH_SENSORS in the downstream driver.
Signed-off-by: Jonathan Marek
---
drivers/misc/fastrpc.c | 9 ++---
include/uapi/misc/fastrpc.h | 5 +++--
2 files changed, 9 insertions(+), 5 deletions(-)
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc.c
index 7939c55daceb..ea5e9ca0d705 100644
--- a/drivers/misc/fastrpc.c
+++ b/drivers/misc/fastrpc.c
@@ -1276,7 +1276,7 @@ static int fastrpc_dmabuf_alloc(struct fastrpc_user *fl,
char __user *argp)
return 0;
}
-static int fastrpc_init_attach(struct fastrpc_user *fl)
+static int fastrpc_init_attach(struct fastrpc_user *fl, int pd)
{
struct fastrpc_invoke_args args[1];
int tgid = fl->tgid;
@@ -1287,7 +1287,7 @@ static int fastrpc_init_attach(struct fastrpc_user *fl)
args[0].fd = -1;
args[0].reserved = 0;
sc = FASTRPC_SCALARS(FASTRPC_RMID_INIT_ATTACH, 1, 0);
- fl->pd = 0;
+ fl->pd = pd;
return fastrpc_internal_invoke(fl, true, FASTRPC_INIT_HANDLE,
sc, [0]);
@@ -1477,7 +1477,10 @@ static long fastrpc_device_ioctl(struct file *file,
unsigned int cmd,
err = fastrpc_invoke(fl, argp);
break;
case FASTRPC_IOCTL_INIT_ATTACH:
- err = fastrpc_init_attach(fl);
+ err = fastrpc_init_attach(fl, 0);
+ break;
+ case FASTRPC_IOCTL_INIT_ATTACH_SNS:
+ err = fastrpc_init_attach(fl, 2);
break;
case FASTRPC_IOCTL_INIT_CREATE:
err = fastrpc_init_create_process(fl, argp);
diff --git a/include/uapi/misc/fastrpc.h b/include/uapi/misc/fastrpc.h
index 07de2b7aac85..0a89f95463f6 100644
--- a/include/uapi/misc/fastrpc.h
+++ b/include/uapi/misc/fastrpc.h
@@ -10,8 +10,9 @@
#define FASTRPC_IOCTL_INVOKE _IOWR('R', 3, struct fastrpc_invoke)
#define FASTRPC_IOCTL_INIT_ATTACH _IO('R', 4)
#define FASTRPC_IOCTL_INIT_CREATE _IOWR('R', 5, struct
fastrpc_init_create)
-#define FASTRPC_IOCTL_MMAP _IOWR('R', 6, struct fastrpc_req_mmap)
-#define FASTRPC_IOCTL_MUNMAP_IOWR('R', 7, struct
fastrpc_req_munmap)
+#define FASTRPC_IOCTL_MMAP _IOWR('R', 6, struct fastrpc_req_mmap)
+#define FASTRPC_IOCTL_MUNMAP _IOWR('R', 7, struct fastrpc_req_munmap)
+#define FASTRPC_IOCTL_INIT_ATTACH_SNS _IO('R', 8)
struct fastrpc_invoke_args {
__u64 ptr;
--
2.26.1
