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 <[email protected]>
---
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);
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
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 <[email protected]>
> ---
> 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 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 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 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 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 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 <[email protected]>
>> ---
>> 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: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 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 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