| Message ID | 20200901003300.11985-1-jonathan@marek.ca (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | misc: fastrpc: add ioctl for attaching to sensors pd | expand |
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 <jonathan@marek.ca> > --- > 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, &args[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
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)
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)
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 <jonathan@marek.ca> >> --- >> 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, &args[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 >
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
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)
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
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)
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
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, &args[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;
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 <jonathan@marek.ca> --- drivers/misc/fastrpc.c | 9 ++++++--- include/uapi/misc/fastrpc.h | 5 +++-- 2 files changed, 9 insertions(+), 5 deletions(-)