[PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Ekansh Gupta]

Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
to be added for PD notifications and other missing features. Adding
and maintaining new files from within fastrpc directory would be easy.

Example of feature that is being planned to be introduced in a new C
file:
https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/

Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
---
 MAINTAINERS                          |  2 +-
 drivers/misc/Kconfig                 | 13 +------------
 drivers/misc/Makefile                |  2 +-
 drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
 drivers/misc/fastrpc/Makefile        |  2 ++
 drivers/misc/{ => fastrpc}/fastrpc.c |  0
 6 files changed, 21 insertions(+), 14 deletions(-)
 create mode 100644 drivers/misc/fastrpc/Kconfig
 create mode 100644 drivers/misc/fastrpc/Makefile
 rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index d6c90161c7bf..e9c79e9063f8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -18501,7 +18501,7 @@ M:	Amol Maheshwari <amahesh@qti.qualcomm.com>
 L:	linux-arm-msm@vger.kernel.org
 S:	Maintained
 F:	Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
-F:	drivers/misc/fastrpc.c
+F:	drivers/misc/fastrpc/
 F:	include/uapi/misc/fastrpc.h
 
 QUALCOMM HEXAGON ARCHITECTURE
diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
index faf983680040..630e8ccd8669 100644
--- a/drivers/misc/Kconfig
+++ b/drivers/misc/Kconfig
@@ -276,18 +276,6 @@ config QCOM_COINCELL
 	  to maintain PMIC register and RTC state in the absence of
 	  external power.
 
-config QCOM_FASTRPC
-	tristate "Qualcomm FastRPC"
-	depends on ARCH_QCOM || COMPILE_TEST
-	depends on RPMSG
-	select DMA_SHARED_BUFFER
-	select QCOM_SCM
-	help
-	  Provides a communication mechanism that allows for clients to
-	  make remote method invocations across processor boundary to
-	  applications DSP processor. Say M if you want to enable this
-	  module.
-
 config SGI_GRU
 	tristate "SGI GRU driver"
 	depends on X86_UV && SMP
@@ -602,4 +590,5 @@ source "drivers/misc/cardreader/Kconfig"
 source "drivers/misc/uacce/Kconfig"
 source "drivers/misc/pvpanic/Kconfig"
 source "drivers/misc/mchp_pci1xxxx/Kconfig"
+source "drivers/misc/fastrpc/Kconfig"
 endmenu
diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
index 153a3f4837e8..f83d73844ea5 100644
--- a/drivers/misc/Makefile
+++ b/drivers/misc/Makefile
@@ -16,7 +16,6 @@ obj-$(CONFIG_TIFM_CORE)       	+= tifm_core.o
 obj-$(CONFIG_TIFM_7XX1)       	+= tifm_7xx1.o
 obj-$(CONFIG_PHANTOM)		+= phantom.o
 obj-$(CONFIG_QCOM_COINCELL)	+= qcom-coincell.o
-obj-$(CONFIG_QCOM_FASTRPC)	+= fastrpc.o
 obj-$(CONFIG_SENSORS_BH1770)	+= bh1770glc.o
 obj-$(CONFIG_SENSORS_APDS990X)	+= apds990x.o
 obj-$(CONFIG_ENCLOSURE_SERVICES) += enclosure.o
@@ -69,3 +68,4 @@ obj-$(CONFIG_TMR_INJECT)	+= xilinx_tmr_inject.o
 obj-$(CONFIG_TPS6594_ESM)	+= tps6594-esm.o
 obj-$(CONFIG_TPS6594_PFSM)	+= tps6594-pfsm.o
 obj-$(CONFIG_NSM)		+= nsm.o
+obj-y				+= fastrpc/
diff --git a/drivers/misc/fastrpc/Kconfig b/drivers/misc/fastrpc/Kconfig
new file mode 100644
index 000000000000..3243dc56b2a0
--- /dev/null
+++ b/drivers/misc/fastrpc/Kconfig
@@ -0,0 +1,16 @@
+# SPDX-License-Identifier: GPL-2.0-only
+#
+# Qualcomm FastRPC devices
+#
+
+config QCOM_FASTRPC
+	tristate "Qualcomm FastRPC"
+	depends on ARCH_QCOM || COMPILE_TEST
+	depends on RPMSG
+	select DMA_SHARED_BUFFER
+	select QCOM_SCM
+	help
+	  Provides a communication mechanism that allows for clients to
+	  make remote method invocations across processor boundary to
+	  applications DSP processor. Say M if you want to enable this
+	  module.
\ No newline at end of file
diff --git a/drivers/misc/fastrpc/Makefile b/drivers/misc/fastrpc/Makefile
new file mode 100644
index 000000000000..77fd2b763b6b
--- /dev/null
+++ b/drivers/misc/fastrpc/Makefile
@@ -0,0 +1,2 @@
+# SPDX-License-Identifier: GPL-2.0
+obj-$(CONFIG_QCOM_FASTRPC)	+= fastrpc.o
diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc/fastrpc.c
similarity index 100%
rename from drivers/misc/fastrpc.c
rename to drivers/misc/fastrpc/fastrpc.c
-- 
2.43.0

Re: [PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Dmitry Baryshkov]

On Wed, Jun 12, 2024 at 12:17:28PM +0530, Ekansh Gupta wrote:
> Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
> to be added for PD notifications and other missing features. Adding
> and maintaining new files from within fastrpc directory would be easy.
> 
> Example of feature that is being planned to be introduced in a new C
> file:
> https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/
> 
> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> ---
>  MAINTAINERS                          |  2 +-
>  drivers/misc/Kconfig                 | 13 +------------
>  drivers/misc/Makefile                |  2 +-
>  drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
>  drivers/misc/fastrpc/Makefile        |  2 ++
>  drivers/misc/{ => fastrpc}/fastrpc.c |  0
>  6 files changed, 21 insertions(+), 14 deletions(-)
>  create mode 100644 drivers/misc/fastrpc/Kconfig
>  create mode 100644 drivers/misc/fastrpc/Makefile
>  rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)

Please consider whether it makes sense to move to drivers/accel instead
(and possibly writing a better Kconfig entry, specifying that the driver
is to be used to offload execution to the DSP).

> diff --git a/MAINTAINERS b/MAINTAINERS
> index d6c90161c7bf..e9c79e9063f8 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -18501,7 +18501,7 @@ M:	Amol Maheshwari <amahesh@qti.qualcomm.com>
>  L:	linux-arm-msm@vger.kernel.org
>  S:	Maintained
>  F:	Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
> -F:	drivers/misc/fastrpc.c
> +F:	drivers/misc/fastrpc/
>  F:	include/uapi/misc/fastrpc.h
>  
>  QUALCOMM HEXAGON ARCHITECTURE
> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
> index faf983680040..630e8ccd8669 100644
> --- a/drivers/misc/Kconfig
> +++ b/drivers/misc/Kconfig
> @@ -276,18 +276,6 @@ config QCOM_COINCELL
>  	  to maintain PMIC register and RTC state in the absence of
>  	  external power.
>  
> -config QCOM_FASTRPC
> -	tristate "Qualcomm FastRPC"
> -	depends on ARCH_QCOM || COMPILE_TEST
> -	depends on RPMSG
> -	select DMA_SHARED_BUFFER
> -	select QCOM_SCM
> -	help
> -	  Provides a communication mechanism that allows for clients to
> -	  make remote method invocations across processor boundary to
> -	  applications DSP processor. Say M if you want to enable this
> -	  module.
> -
>  config SGI_GRU
>  	tristate "SGI GRU driver"
>  	depends on X86_UV && SMP
> @@ -602,4 +590,5 @@ source "drivers/misc/cardreader/Kconfig"
>  source "drivers/misc/uacce/Kconfig"
>  source "drivers/misc/pvpanic/Kconfig"
>  source "drivers/misc/mchp_pci1xxxx/Kconfig"
> +source "drivers/misc/fastrpc/Kconfig"
>  endmenu
> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
> index 153a3f4837e8..f83d73844ea5 100644
> --- a/drivers/misc/Makefile
> +++ b/drivers/misc/Makefile
> @@ -16,7 +16,6 @@ obj-$(CONFIG_TIFM_CORE)       	+= tifm_core.o
>  obj-$(CONFIG_TIFM_7XX1)       	+= tifm_7xx1.o
>  obj-$(CONFIG_PHANTOM)		+= phantom.o
>  obj-$(CONFIG_QCOM_COINCELL)	+= qcom-coincell.o
> -obj-$(CONFIG_QCOM_FASTRPC)	+= fastrpc.o
>  obj-$(CONFIG_SENSORS_BH1770)	+= bh1770glc.o
>  obj-$(CONFIG_SENSORS_APDS990X)	+= apds990x.o
>  obj-$(CONFIG_ENCLOSURE_SERVICES) += enclosure.o
> @@ -69,3 +68,4 @@ obj-$(CONFIG_TMR_INJECT)	+= xilinx_tmr_inject.o
>  obj-$(CONFIG_TPS6594_ESM)	+= tps6594-esm.o
>  obj-$(CONFIG_TPS6594_PFSM)	+= tps6594-pfsm.o
>  obj-$(CONFIG_NSM)		+= nsm.o
> +obj-y				+= fastrpc/
> diff --git a/drivers/misc/fastrpc/Kconfig b/drivers/misc/fastrpc/Kconfig
> new file mode 100644
> index 000000000000..3243dc56b2a0
> --- /dev/null
> +++ b/drivers/misc/fastrpc/Kconfig
> @@ -0,0 +1,16 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +#
> +# Qualcomm FastRPC devices
> +#
> +
> +config QCOM_FASTRPC
> +	tristate "Qualcomm FastRPC"
> +	depends on ARCH_QCOM || COMPILE_TEST
> +	depends on RPMSG
> +	select DMA_SHARED_BUFFER
> +	select QCOM_SCM
> +	help
> +	  Provides a communication mechanism that allows for clients to
> +	  make remote method invocations across processor boundary to
> +	  applications DSP processor. Say M if you want to enable this
> +	  module.
> \ No newline at end of file
> diff --git a/drivers/misc/fastrpc/Makefile b/drivers/misc/fastrpc/Makefile
> new file mode 100644
> index 000000000000..77fd2b763b6b
> --- /dev/null
> +++ b/drivers/misc/fastrpc/Makefile
> @@ -0,0 +1,2 @@
> +# SPDX-License-Identifier: GPL-2.0
> +obj-$(CONFIG_QCOM_FASTRPC)	+= fastrpc.o
> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc/fastrpc.c
> similarity index 100%
> rename from drivers/misc/fastrpc.c
> rename to drivers/misc/fastrpc/fastrpc.c
> -- 
> 2.43.0
> 

-- 
With best wishes
Dmitry

Re: [PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Ekansh Gupta]

On 6/12/2024 11:58 PM, Dmitry Baryshkov wrote:
> On Wed, Jun 12, 2024 at 12:17:28PM +0530, Ekansh Gupta wrote:
>> Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
>> to be added for PD notifications and other missing features. Adding
>> and maintaining new files from within fastrpc directory would be easy.
>>
>> Example of feature that is being planned to be introduced in a new C
>> file:
>> https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/
>>
>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
>> ---
>>  MAINTAINERS                          |  2 +-
>>  drivers/misc/Kconfig                 | 13 +------------
>>  drivers/misc/Makefile                |  2 +-
>>  drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
>>  drivers/misc/fastrpc/Makefile        |  2 ++
>>  drivers/misc/{ => fastrpc}/fastrpc.c |  0
>>  6 files changed, 21 insertions(+), 14 deletions(-)
>>  create mode 100644 drivers/misc/fastrpc/Kconfig
>>  create mode 100644 drivers/misc/fastrpc/Makefile
>>  rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)
> Please consider whether it makes sense to move to drivers/accel instead
> (and possibly writing a better Kconfig entry, specifying that the driver
> is to be used to offload execution to the DSP).
Planning to keep the driver to misc/ only as part of this patch. Moving to accel/ might
introduce some conventions to be followed which might require significant changes
in driver.

I'll write more meaningful Kconfig entry in next spin.

Thanks
--Ekansh
>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index d6c90161c7bf..e9c79e9063f8 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -18501,7 +18501,7 @@ M:	Amol Maheshwari <amahesh@qti.qualcomm.com>
>>  L:	linux-arm-msm@vger.kernel.org
>>  S:	Maintained
>>  F:	Documentation/devicetree/bindings/misc/qcom,fastrpc.yaml
>> -F:	drivers/misc/fastrpc.c
>> +F:	drivers/misc/fastrpc/
>>  F:	include/uapi/misc/fastrpc.h
>>  
>>  QUALCOMM HEXAGON ARCHITECTURE
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index faf983680040..630e8ccd8669 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -276,18 +276,6 @@ config QCOM_COINCELL
>>  	  to maintain PMIC register and RTC state in the absence of
>>  	  external power.
>>  
>> -config QCOM_FASTRPC
>> -	tristate "Qualcomm FastRPC"
>> -	depends on ARCH_QCOM || COMPILE_TEST
>> -	depends on RPMSG
>> -	select DMA_SHARED_BUFFER
>> -	select QCOM_SCM
>> -	help
>> -	  Provides a communication mechanism that allows for clients to
>> -	  make remote method invocations across processor boundary to
>> -	  applications DSP processor. Say M if you want to enable this
>> -	  module.
>> -
>>  config SGI_GRU
>>  	tristate "SGI GRU driver"
>>  	depends on X86_UV && SMP
>> @@ -602,4 +590,5 @@ source "drivers/misc/cardreader/Kconfig"
>>  source "drivers/misc/uacce/Kconfig"
>>  source "drivers/misc/pvpanic/Kconfig"
>>  source "drivers/misc/mchp_pci1xxxx/Kconfig"
>> +source "drivers/misc/fastrpc/Kconfig"
>>  endmenu
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 153a3f4837e8..f83d73844ea5 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -16,7 +16,6 @@ obj-$(CONFIG_TIFM_CORE)       	+= tifm_core.o
>>  obj-$(CONFIG_TIFM_7XX1)       	+= tifm_7xx1.o
>>  obj-$(CONFIG_PHANTOM)		+= phantom.o
>>  obj-$(CONFIG_QCOM_COINCELL)	+= qcom-coincell.o
>> -obj-$(CONFIG_QCOM_FASTRPC)	+= fastrpc.o
>>  obj-$(CONFIG_SENSORS_BH1770)	+= bh1770glc.o
>>  obj-$(CONFIG_SENSORS_APDS990X)	+= apds990x.o
>>  obj-$(CONFIG_ENCLOSURE_SERVICES) += enclosure.o
>> @@ -69,3 +68,4 @@ obj-$(CONFIG_TMR_INJECT)	+= xilinx_tmr_inject.o
>>  obj-$(CONFIG_TPS6594_ESM)	+= tps6594-esm.o
>>  obj-$(CONFIG_TPS6594_PFSM)	+= tps6594-pfsm.o
>>  obj-$(CONFIG_NSM)		+= nsm.o
>> +obj-y				+= fastrpc/
>> diff --git a/drivers/misc/fastrpc/Kconfig b/drivers/misc/fastrpc/Kconfig
>> new file mode 100644
>> index 000000000000..3243dc56b2a0
>> --- /dev/null
>> +++ b/drivers/misc/fastrpc/Kconfig
>> @@ -0,0 +1,16 @@
>> +# SPDX-License-Identifier: GPL-2.0-only
>> +#
>> +# Qualcomm FastRPC devices
>> +#
>> +
>> +config QCOM_FASTRPC
>> +	tristate "Qualcomm FastRPC"
>> +	depends on ARCH_QCOM || COMPILE_TEST
>> +	depends on RPMSG
>> +	select DMA_SHARED_BUFFER
>> +	select QCOM_SCM
>> +	help
>> +	  Provides a communication mechanism that allows for clients to
>> +	  make remote method invocations across processor boundary to
>> +	  applications DSP processor. Say M if you want to enable this
>> +	  module.
>> \ No newline at end of file
>> diff --git a/drivers/misc/fastrpc/Makefile b/drivers/misc/fastrpc/Makefile
>> new file mode 100644
>> index 000000000000..77fd2b763b6b
>> --- /dev/null
>> +++ b/drivers/misc/fastrpc/Makefile
>> @@ -0,0 +1,2 @@
>> +# SPDX-License-Identifier: GPL-2.0
>> +obj-$(CONFIG_QCOM_FASTRPC)	+= fastrpc.o
>> diff --git a/drivers/misc/fastrpc.c b/drivers/misc/fastrpc/fastrpc.c
>> similarity index 100%
>> rename from drivers/misc/fastrpc.c
>> rename to drivers/misc/fastrpc/fastrpc.c
>> -- 
>> 2.43.0
>>

Re: [PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Greg KH]

On Wed, Jun 19, 2024 at 12:15:03PM +0530, Ekansh Gupta wrote:
> 
> 
> On 6/12/2024 11:58 PM, Dmitry Baryshkov wrote:
> > On Wed, Jun 12, 2024 at 12:17:28PM +0530, Ekansh Gupta wrote:
> >> Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
> >> to be added for PD notifications and other missing features. Adding
> >> and maintaining new files from within fastrpc directory would be easy.
> >>
> >> Example of feature that is being planned to be introduced in a new C
> >> file:
> >> https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/
> >>
> >> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> >> ---
> >>  MAINTAINERS                          |  2 +-
> >>  drivers/misc/Kconfig                 | 13 +------------
> >>  drivers/misc/Makefile                |  2 +-
> >>  drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
> >>  drivers/misc/fastrpc/Makefile        |  2 ++
> >>  drivers/misc/{ => fastrpc}/fastrpc.c |  0
> >>  6 files changed, 21 insertions(+), 14 deletions(-)
> >>  create mode 100644 drivers/misc/fastrpc/Kconfig
> >>  create mode 100644 drivers/misc/fastrpc/Makefile
> >>  rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)
> > Please consider whether it makes sense to move to drivers/accel instead
> > (and possibly writing a better Kconfig entry, specifying that the driver
> > is to be used to offload execution to the DSP).
> Planning to keep the driver to misc/ only as part of this patch. Moving to accel/ might
> introduce some conventions to be followed which might require significant changes
> in driver.

Which is a good thing, please don't avoid this :)

thanks,

greg k-h

Re: [PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Dmitry Baryshkov]

On Wed, 19 Jun 2024 at 09:45, Ekansh Gupta <quic_ekangupt@quicinc.com> wrote:
>
>
>
> On 6/12/2024 11:58 PM, Dmitry Baryshkov wrote:
> > On Wed, Jun 12, 2024 at 12:17:28PM +0530, Ekansh Gupta wrote:
> >> Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
> >> to be added for PD notifications and other missing features. Adding
> >> and maintaining new files from within fastrpc directory would be easy.
> >>
> >> Example of feature that is being planned to be introduced in a new C
> >> file:
> >> https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/
> >>
> >> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> >> ---
> >>  MAINTAINERS                          |  2 +-
> >>  drivers/misc/Kconfig                 | 13 +------------
> >>  drivers/misc/Makefile                |  2 +-
> >>  drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
> >>  drivers/misc/fastrpc/Makefile        |  2 ++
> >>  drivers/misc/{ => fastrpc}/fastrpc.c |  0
> >>  6 files changed, 21 insertions(+), 14 deletions(-)
> >>  create mode 100644 drivers/misc/fastrpc/Kconfig
> >>  create mode 100644 drivers/misc/fastrpc/Makefile
> >>  rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)
> > Please consider whether it makes sense to move to drivers/accel instead
> > (and possibly writing a better Kconfig entry, specifying that the driver
> > is to be used to offload execution to the DSP).
> Planning to keep the driver to misc/ only as part of this patch. Moving to accel/ might
> introduce some conventions to be followed which might require significant changes
> in driver.

To me this sounds like "we are trying to avoid following the
conventions by hiding in the shadows".

>
> I'll write more meaningful Kconfig entry in next spin.
>

-- 
With best wishes
Dmitry

Re: [PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Ekansh Gupta]

On 6/19/2024 12:21 PM, Dmitry Baryshkov wrote:
> On Wed, 19 Jun 2024 at 09:45, Ekansh Gupta <quic_ekangupt@quicinc.com> wrote:
>>
>>
>> On 6/12/2024 11:58 PM, Dmitry Baryshkov wrote:
>>> On Wed, Jun 12, 2024 at 12:17:28PM +0530, Ekansh Gupta wrote:
>>>> Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
>>>> to be added for PD notifications and other missing features. Adding
>>>> and maintaining new files from within fastrpc directory would be easy.
>>>>
>>>> Example of feature that is being planned to be introduced in a new C
>>>> file:
>>>> https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/
>>>>
>>>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
>>>> ---
>>>>  MAINTAINERS                          |  2 +-
>>>>  drivers/misc/Kconfig                 | 13 +------------
>>>>  drivers/misc/Makefile                |  2 +-
>>>>  drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
>>>>  drivers/misc/fastrpc/Makefile        |  2 ++
>>>>  drivers/misc/{ => fastrpc}/fastrpc.c |  0
>>>>  6 files changed, 21 insertions(+), 14 deletions(-)
>>>>  create mode 100644 drivers/misc/fastrpc/Kconfig
>>>>  create mode 100644 drivers/misc/fastrpc/Makefile
>>>>  rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)
>>> Please consider whether it makes sense to move to drivers/accel instead
>>> (and possibly writing a better Kconfig entry, specifying that the driver
>>> is to be used to offload execution to the DSP).
>> Planning to keep the driver to misc/ only as part of this patch. Moving to accel/ might
>> introduce some conventions to be followed which might require significant changes
>> in driver.
> To me this sounds like "we are trying to avoid following the
> conventions by hiding in the shadows".
Not trying to avoid, just trying to look into this separately as the need to take ABI also in account which
includes current device nodes and the uapi header which is present in uapi/misc/fastrpc.h whereas I see all
accel driver uapi headers are part of uapi/drm/.

Will be taking inputs from fastrpc maintainers also.
>> I'll write more meaningful Kconfig entry in next spin.
>>

Re: [PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Dmitry Baryshkov]

On Wed, 19 Jun 2024 at 10:01, Ekansh Gupta <quic_ekangupt@quicinc.com> wrote:
>
>
>
> On 6/19/2024 12:21 PM, Dmitry Baryshkov wrote:
> > On Wed, 19 Jun 2024 at 09:45, Ekansh Gupta <quic_ekangupt@quicinc.com> wrote:
> >>
> >>
> >> On 6/12/2024 11:58 PM, Dmitry Baryshkov wrote:
> >>> On Wed, Jun 12, 2024 at 12:17:28PM +0530, Ekansh Gupta wrote:
> >>>> Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
> >>>> to be added for PD notifications and other missing features. Adding
> >>>> and maintaining new files from within fastrpc directory would be easy.
> >>>>
> >>>> Example of feature that is being planned to be introduced in a new C
> >>>> file:
> >>>> https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/
> >>>>
> >>>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> >>>> ---
> >>>>  MAINTAINERS                          |  2 +-
> >>>>  drivers/misc/Kconfig                 | 13 +------------
> >>>>  drivers/misc/Makefile                |  2 +-
> >>>>  drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
> >>>>  drivers/misc/fastrpc/Makefile        |  2 ++
> >>>>  drivers/misc/{ => fastrpc}/fastrpc.c |  0
> >>>>  6 files changed, 21 insertions(+), 14 deletions(-)
> >>>>  create mode 100644 drivers/misc/fastrpc/Kconfig
> >>>>  create mode 100644 drivers/misc/fastrpc/Makefile
> >>>>  rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)
> >>> Please consider whether it makes sense to move to drivers/accel instead
> >>> (and possibly writing a better Kconfig entry, specifying that the driver
> >>> is to be used to offload execution to the DSP).
> >> Planning to keep the driver to misc/ only as part of this patch. Moving to accel/ might
> >> introduce some conventions to be followed which might require significant changes
> >> in driver.
> > To me this sounds like "we are trying to avoid following the
> > conventions by hiding in the shadows".
> Not trying to avoid, just trying to look into this separately as the need to take ABI also in account which
> includes current device nodes and the uapi header which is present in uapi/misc/fastrpc.h whereas I see all
> accel driver uapi headers are part of uapi/drm/.

I'd say this depends on the accel/ maintainer's opinion.

>
> Will be taking inputs from fastrpc maintainers also.
> >> I'll write more meaningful Kconfig entry in next spin.
> >>
>


-- 
With best wishes
Dmitry

Re: [PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Bjorn Andersson]

On Wed, Jun 12, 2024 at 09:28:39PM GMT, Dmitry Baryshkov wrote:
> On Wed, Jun 12, 2024 at 12:17:28PM +0530, Ekansh Gupta wrote:
> > Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
> > to be added for PD notifications and other missing features. Adding
> > and maintaining new files from within fastrpc directory would be easy.
> > 
> > Example of feature that is being planned to be introduced in a new C
> > file:
> > https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/
> > 
> > Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> > ---
> >  MAINTAINERS                          |  2 +-
> >  drivers/misc/Kconfig                 | 13 +------------
> >  drivers/misc/Makefile                |  2 +-
> >  drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
> >  drivers/misc/fastrpc/Makefile        |  2 ++
> >  drivers/misc/{ => fastrpc}/fastrpc.c |  0
> >  6 files changed, 21 insertions(+), 14 deletions(-)
> >  create mode 100644 drivers/misc/fastrpc/Kconfig
> >  create mode 100644 drivers/misc/fastrpc/Makefile
> >  rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)
> 
> Please consider whether it makes sense to move to drivers/accel instead
> (and possibly writing a better Kconfig entry, specifying that the driver
> is to be used to offload execution to the DSP).
> 

Wouldn't this come with the expectation of following the ABIs of
drivers/accel and thereby breaking userspace?

Regards,
Bjorn

Re: [PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Dmitry Baryshkov]

On Fri, 21 Jun 2024 at 09:19, Bjorn Andersson <andersson@kernel.org> wrote:
>
> On Wed, Jun 12, 2024 at 09:28:39PM GMT, Dmitry Baryshkov wrote:
> > On Wed, Jun 12, 2024 at 12:17:28PM +0530, Ekansh Gupta wrote:
> > > Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
> > > to be added for PD notifications and other missing features. Adding
> > > and maintaining new files from within fastrpc directory would be easy.
> > >
> > > Example of feature that is being planned to be introduced in a new C
> > > file:
> > > https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/
> > >
> > > Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> > > ---
> > >  MAINTAINERS                          |  2 +-
> > >  drivers/misc/Kconfig                 | 13 +------------
> > >  drivers/misc/Makefile                |  2 +-
> > >  drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
> > >  drivers/misc/fastrpc/Makefile        |  2 ++
> > >  drivers/misc/{ => fastrpc}/fastrpc.c |  0
> > >  6 files changed, 21 insertions(+), 14 deletions(-)
> > >  create mode 100644 drivers/misc/fastrpc/Kconfig
> > >  create mode 100644 drivers/misc/fastrpc/Makefile
> > >  rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)
> >
> > Please consider whether it makes sense to move to drivers/accel instead
> > (and possibly writing a better Kconfig entry, specifying that the driver
> > is to be used to offload execution to the DSP).
> >
>
> Wouldn't this come with the expectation of following the ABIs of
> drivers/accel and thereby breaking userspace?

As I wrote earlier, that depends on the accel/ maintainers decision,
whether it's acceptable to have non-DRM_ACCEL code underneath.
But at least I'd try doing that on the grounds of keeping the code at
the proper place in the drivers/ tree, raising awareness of the
FastRPC, etc.
For example current fastrpc driver bypasses dri-devel reviews, while
if I remember correctly, at some point it was suggested that all
dma-buf-handling drivers should also notify the dri-devel ML.

Also having the driver under drivers/accels makes it possible and
logical to  implement DRM_ACCEL uAPI at some point. In the ideal world
we should be able to declare existing FastRPC uAPI as legacy /
deprecated / backwards compatibility only and migrate to the
recommended uAPI approach, which is DRM_ACCEL.

-- 
With best wishes
Dmitry

Re: [PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Jeffrey Hugo]

On 6/21/2024 5:19 AM, Dmitry Baryshkov wrote:
> On Fri, 21 Jun 2024 at 09:19, Bjorn Andersson <andersson@kernel.org> wrote:
>>
>> On Wed, Jun 12, 2024 at 09:28:39PM GMT, Dmitry Baryshkov wrote:
>>> On Wed, Jun 12, 2024 at 12:17:28PM +0530, Ekansh Gupta wrote:
>>>> Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
>>>> to be added for PD notifications and other missing features. Adding
>>>> and maintaining new files from within fastrpc directory would be easy.
>>>>
>>>> Example of feature that is being planned to be introduced in a new C
>>>> file:
>>>> https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/
>>>>
>>>> Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
>>>> ---
>>>>   MAINTAINERS                          |  2 +-
>>>>   drivers/misc/Kconfig                 | 13 +------------
>>>>   drivers/misc/Makefile                |  2 +-
>>>>   drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
>>>>   drivers/misc/fastrpc/Makefile        |  2 ++
>>>>   drivers/misc/{ => fastrpc}/fastrpc.c |  0
>>>>   6 files changed, 21 insertions(+), 14 deletions(-)
>>>>   create mode 100644 drivers/misc/fastrpc/Kconfig
>>>>   create mode 100644 drivers/misc/fastrpc/Makefile
>>>>   rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)
>>>
>>> Please consider whether it makes sense to move to drivers/accel instead
>>> (and possibly writing a better Kconfig entry, specifying that the driver
>>> is to be used to offload execution to the DSP).
>>>
>>
>> Wouldn't this come with the expectation of following the ABIs of
>> drivers/accel and thereby breaking userspace?
> 
> As I wrote earlier, that depends on the accel/ maintainers decision,
> whether it's acceptable to have non-DRM_ACCEL code underneath.
> But at least I'd try doing that on the grounds of keeping the code at
> the proper place in the drivers/ tree, raising awareness of the
> FastRPC, etc.
> For example current fastrpc driver bypasses dri-devel reviews, while
> if I remember correctly, at some point it was suggested that all
> dma-buf-handling drivers should also notify the dri-devel ML.
> 
> Also having the driver under drivers/accels makes it possible and
> logical to  implement DRM_ACCEL uAPI at some point. In the ideal world
> we should be able to declare existing FastRPC uAPI as legacy /
> deprecated / backwards compatibility only and migrate to the
> recommended uAPI approach, which is DRM_ACCEL.
> 

I suspect Vetter/Airlie need to be involved in this.

Its my understanding that accelerator drivers are able to reside in misc 
as long as there is no use of dma-buf.  Use of dma-buf means they need 
to be in drm/accel.

There is precedent for moving a driver from misc to accel (HabanaLabs).

Right now, I'm not aware that fastRPC meets the requirements for 
drm/accel.  There is an open source userspace driver, but I'm not aware 
of an open source compiler.  From what I know of the architecture, it 
should be possible to utilize upstream LLVM to produce one.

-Jeff

Re: [PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Daniel Vetter]

On Fri, Jun 21, 2024 at 09:40:09AM -0600, Jeffrey Hugo wrote:
> On 6/21/2024 5:19 AM, Dmitry Baryshkov wrote:
> > On Fri, 21 Jun 2024 at 09:19, Bjorn Andersson <andersson@kernel.org> wrote:
> > > 
> > > On Wed, Jun 12, 2024 at 09:28:39PM GMT, Dmitry Baryshkov wrote:
> > > > On Wed, Jun 12, 2024 at 12:17:28PM +0530, Ekansh Gupta wrote:
> > > > > Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
> > > > > to be added for PD notifications and other missing features. Adding
> > > > > and maintaining new files from within fastrpc directory would be easy.
> > > > > 
> > > > > Example of feature that is being planned to be introduced in a new C
> > > > > file:
> > > > > https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/
> > > > > 
> > > > > Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> > > > > ---
> > > > >   MAINTAINERS                          |  2 +-
> > > > >   drivers/misc/Kconfig                 | 13 +------------
> > > > >   drivers/misc/Makefile                |  2 +-
> > > > >   drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
> > > > >   drivers/misc/fastrpc/Makefile        |  2 ++
> > > > >   drivers/misc/{ => fastrpc}/fastrpc.c |  0
> > > > >   6 files changed, 21 insertions(+), 14 deletions(-)
> > > > >   create mode 100644 drivers/misc/fastrpc/Kconfig
> > > > >   create mode 100644 drivers/misc/fastrpc/Makefile
> > > > >   rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)
> > > > 
> > > > Please consider whether it makes sense to move to drivers/accel instead
> > > > (and possibly writing a better Kconfig entry, specifying that the driver
> > > > is to be used to offload execution to the DSP).
> > > > 
> > > 
> > > Wouldn't this come with the expectation of following the ABIs of
> > > drivers/accel and thereby breaking userspace?
> > 
> > As I wrote earlier, that depends on the accel/ maintainers decision,
> > whether it's acceptable to have non-DRM_ACCEL code underneath.
> > But at least I'd try doing that on the grounds of keeping the code at
> > the proper place in the drivers/ tree, raising awareness of the
> > FastRPC, etc.
> > For example current fastrpc driver bypasses dri-devel reviews, while
> > if I remember correctly, at some point it was suggested that all
> > dma-buf-handling drivers should also notify the dri-devel ML.
> > 
> > Also having the driver under drivers/accels makes it possible and
> > logical to  implement DRM_ACCEL uAPI at some point. In the ideal world
> > we should be able to declare existing FastRPC uAPI as legacy /
> > deprecated / backwards compatibility only and migrate to the
> > recommended uAPI approach, which is DRM_ACCEL.
> > 
> 
> I suspect Vetter/Airlie need to be involved in this.
> 
> Its my understanding that accelerator drivers are able to reside in misc as
> long as there is no use of dma-buf.  Use of dma-buf means they need to be in
> drm/accel.
> 
> There is precedent for moving a driver from misc to accel (HabanaLabs).
> 
> Right now, I'm not aware that fastRPC meets the requirements for drm/accel.
> There is an open source userspace driver, but I'm not aware of an open
> source compiler.  From what I know of the architecture, it should be
> possible to utilize upstream LLVM to produce one.

Yeah so fastrpc is one of the reasons why I've added a dma_buf regex match
to MAINTAINERS, and given this move has shown up here on dri-devel that
seems to work.

But also, it slipped through, can't break uapi, so I just pretend it's not
really there :-)

That aside, going forward it might make sense to look into drivers/accel,
and also going forward new dma_buf uapi will be reviewed to fairly
stringent standards. We're not going to impose the dri-devel userspace
rules on everyone, each subsystem tends to know what's best in their
ecosystem. But if something just ends up in misc so it can avoid the drm
or accel rules (and I think media is also pretty much on the same page
nowadays), then expect some serious heat ...

Cheers, Sima
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Dmitry Baryshkov]

On Fri, 21 Jun 2024 at 18:52, Daniel Vetter <daniel@ffwll.ch> wrote:
>
> On Fri, Jun 21, 2024 at 09:40:09AM -0600, Jeffrey Hugo wrote:
> > On 6/21/2024 5:19 AM, Dmitry Baryshkov wrote:
> > > On Fri, 21 Jun 2024 at 09:19, Bjorn Andersson <andersson@kernel.org> wrote:
> > > >
> > > > On Wed, Jun 12, 2024 at 09:28:39PM GMT, Dmitry Baryshkov wrote:
> > > > > On Wed, Jun 12, 2024 at 12:17:28PM +0530, Ekansh Gupta wrote:
> > > > > > Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
> > > > > > to be added for PD notifications and other missing features. Adding
> > > > > > and maintaining new files from within fastrpc directory would be easy.
> > > > > >
> > > > > > Example of feature that is being planned to be introduced in a new C
> > > > > > file:
> > > > > > https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/
> > > > > >
> > > > > > Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> > > > > > ---
> > > > > >   MAINTAINERS                          |  2 +-
> > > > > >   drivers/misc/Kconfig                 | 13 +------------
> > > > > >   drivers/misc/Makefile                |  2 +-
> > > > > >   drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
> > > > > >   drivers/misc/fastrpc/Makefile        |  2 ++
> > > > > >   drivers/misc/{ => fastrpc}/fastrpc.c |  0
> > > > > >   6 files changed, 21 insertions(+), 14 deletions(-)
> > > > > >   create mode 100644 drivers/misc/fastrpc/Kconfig
> > > > > >   create mode 100644 drivers/misc/fastrpc/Makefile
> > > > > >   rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)
> > > > >
> > > > > Please consider whether it makes sense to move to drivers/accel instead
> > > > > (and possibly writing a better Kconfig entry, specifying that the driver
> > > > > is to be used to offload execution to the DSP).
> > > > >
> > > >
> > > > Wouldn't this come with the expectation of following the ABIs of
> > > > drivers/accel and thereby breaking userspace?
> > >
> > > As I wrote earlier, that depends on the accel/ maintainers decision,
> > > whether it's acceptable to have non-DRM_ACCEL code underneath.
> > > But at least I'd try doing that on the grounds of keeping the code at
> > > the proper place in the drivers/ tree, raising awareness of the
> > > FastRPC, etc.
> > > For example current fastrpc driver bypasses dri-devel reviews, while
> > > if I remember correctly, at some point it was suggested that all
> > > dma-buf-handling drivers should also notify the dri-devel ML.
> > >
> > > Also having the driver under drivers/accels makes it possible and
> > > logical to  implement DRM_ACCEL uAPI at some point. In the ideal world
> > > we should be able to declare existing FastRPC uAPI as legacy /
> > > deprecated / backwards compatibility only and migrate to the
> > > recommended uAPI approach, which is DRM_ACCEL.
> > >
> >
> > I suspect Vetter/Airlie need to be involved in this.
> >
> > Its my understanding that accelerator drivers are able to reside in misc as
> > long as there is no use of dma-buf.  Use of dma-buf means they need to be in
> > drm/accel.
> >
> > There is precedent for moving a driver from misc to accel (HabanaLabs).
> >
> > Right now, I'm not aware that fastRPC meets the requirements for drm/accel.
> > There is an open source userspace driver, but I'm not aware of an open
> > source compiler.  From what I know of the architecture, it should be
> > possible to utilize upstream LLVM to produce one.
>
> Yeah so fastrpc is one of the reasons why I've added a dma_buf regex match
> to MAINTAINERS, and given this move has shown up here on dri-devel that
> seems to work.
>
> But also, it slipped through, can't break uapi, so I just pretend it's not
> really there :-)
>
> That aside, going forward it might make sense to look into drivers/accel,
> and also going forward new dma_buf uapi will be reviewed to fairly
> stringent standards. We're not going to impose the dri-devel userspace
> rules on everyone, each subsystem tends to know what's best in their
> ecosystem. But if something just ends up in misc so it can avoid the drm
> or accel rules (and I think media is also pretty much on the same page
> nowadays), then expect some serious heat ...

After discussing this on #dri-devel, I'm going to retract my
suggestion of moving the driver to drivers/accel/, unless there is an
actual interest in moving to drm_accel.h style of uAPI.

It should still be noted that there is a strong recommendation to
start from scratch and to use DRM / accel uAPI, either using the
existing driver for the legacy platforms or dropping it completely.
When the fastrpc driver was started by Qualcomm engineers, there was
no standard method of implementing the accel drivers. Since 1st of
November 2022 we have drm_accel.h.

-- 
With best wishes
Dmitry

Re: [PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Daniel Vetter]

On Fri, Jun 21, 2024 at 10:48:27PM +0300, Dmitry Baryshkov wrote:
> On Fri, 21 Jun 2024 at 18:52, Daniel Vetter <daniel@ffwll.ch> wrote:
> >
> > On Fri, Jun 21, 2024 at 09:40:09AM -0600, Jeffrey Hugo wrote:
> > > On 6/21/2024 5:19 AM, Dmitry Baryshkov wrote:
> > > > On Fri, 21 Jun 2024 at 09:19, Bjorn Andersson <andersson@kernel.org> wrote:
> > > > >
> > > > > On Wed, Jun 12, 2024 at 09:28:39PM GMT, Dmitry Baryshkov wrote:
> > > > > > On Wed, Jun 12, 2024 at 12:17:28PM +0530, Ekansh Gupta wrote:
> > > > > > > Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
> > > > > > > to be added for PD notifications and other missing features. Adding
> > > > > > > and maintaining new files from within fastrpc directory would be easy.
> > > > > > >
> > > > > > > Example of feature that is being planned to be introduced in a new C
> > > > > > > file:
> > > > > > > https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/
> > > > > > >
> > > > > > > Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> > > > > > > ---
> > > > > > >   MAINTAINERS                          |  2 +-
> > > > > > >   drivers/misc/Kconfig                 | 13 +------------
> > > > > > >   drivers/misc/Makefile                |  2 +-
> > > > > > >   drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
> > > > > > >   drivers/misc/fastrpc/Makefile        |  2 ++
> > > > > > >   drivers/misc/{ => fastrpc}/fastrpc.c |  0
> > > > > > >   6 files changed, 21 insertions(+), 14 deletions(-)
> > > > > > >   create mode 100644 drivers/misc/fastrpc/Kconfig
> > > > > > >   create mode 100644 drivers/misc/fastrpc/Makefile
> > > > > > >   rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)
> > > > > >
> > > > > > Please consider whether it makes sense to move to drivers/accel instead
> > > > > > (and possibly writing a better Kconfig entry, specifying that the driver
> > > > > > is to be used to offload execution to the DSP).
> > > > > >
> > > > >
> > > > > Wouldn't this come with the expectation of following the ABIs of
> > > > > drivers/accel and thereby breaking userspace?
> > > >
> > > > As I wrote earlier, that depends on the accel/ maintainers decision,
> > > > whether it's acceptable to have non-DRM_ACCEL code underneath.
> > > > But at least I'd try doing that on the grounds of keeping the code at
> > > > the proper place in the drivers/ tree, raising awareness of the
> > > > FastRPC, etc.
> > > > For example current fastrpc driver bypasses dri-devel reviews, while
> > > > if I remember correctly, at some point it was suggested that all
> > > > dma-buf-handling drivers should also notify the dri-devel ML.
> > > >
> > > > Also having the driver under drivers/accels makes it possible and
> > > > logical to  implement DRM_ACCEL uAPI at some point. In the ideal world
> > > > we should be able to declare existing FastRPC uAPI as legacy /
> > > > deprecated / backwards compatibility only and migrate to the
> > > > recommended uAPI approach, which is DRM_ACCEL.
> > > >
> > >
> > > I suspect Vetter/Airlie need to be involved in this.
> > >
> > > Its my understanding that accelerator drivers are able to reside in misc as
> > > long as there is no use of dma-buf.  Use of dma-buf means they need to be in
> > > drm/accel.
> > >
> > > There is precedent for moving a driver from misc to accel (HabanaLabs).
> > >
> > > Right now, I'm not aware that fastRPC meets the requirements for drm/accel.
> > > There is an open source userspace driver, but I'm not aware of an open
> > > source compiler.  From what I know of the architecture, it should be
> > > possible to utilize upstream LLVM to produce one.
> >
> > Yeah so fastrpc is one of the reasons why I've added a dma_buf regex match
> > to MAINTAINERS, and given this move has shown up here on dri-devel that
> > seems to work.
> >
> > But also, it slipped through, can't break uapi, so I just pretend it's not
> > really there :-)
> >
> > That aside, going forward it might make sense to look into drivers/accel,
> > and also going forward new dma_buf uapi will be reviewed to fairly
> > stringent standards. We're not going to impose the dri-devel userspace
> > rules on everyone, each subsystem tends to know what's best in their
> > ecosystem. But if something just ends up in misc so it can avoid the drm
> > or accel rules (and I think media is also pretty much on the same page
> > nowadays), then expect some serious heat ...
> 
> After discussing this on #dri-devel, I'm going to retract my
> suggestion of moving the driver to drivers/accel/, unless there is an
> actual interest in moving to drm_accel.h style of uAPI.
> 
> It should still be noted that there is a strong recommendation to
> start from scratch and to use DRM / accel uAPI, either using the
> existing driver for the legacy platforms or dropping it completely.
> When the fastrpc driver was started by Qualcomm engineers, there was
> no standard method of implementing the accel drivers. Since 1st of
> November 2022 we have drm_accel.h.

Yeah, if fastrpc just keeps growing the story will completely different.

Cheers, Sima
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Bjorn Andersson]

On Fri, Jun 21, 2024 at 05:52:32PM GMT, Daniel Vetter wrote:
> On Fri, Jun 21, 2024 at 09:40:09AM -0600, Jeffrey Hugo wrote:
> > On 6/21/2024 5:19 AM, Dmitry Baryshkov wrote:
> > > On Fri, 21 Jun 2024 at 09:19, Bjorn Andersson <andersson@kernel.org> wrote:
> > > > 
> > > > On Wed, Jun 12, 2024 at 09:28:39PM GMT, Dmitry Baryshkov wrote:
> > > > > On Wed, Jun 12, 2024 at 12:17:28PM +0530, Ekansh Gupta wrote:
> > > > > > Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
> > > > > > to be added for PD notifications and other missing features. Adding
> > > > > > and maintaining new files from within fastrpc directory would be easy.
> > > > > > 
> > > > > > Example of feature that is being planned to be introduced in a new C
> > > > > > file:
> > > > > > https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/
> > > > > > 
> > > > > > Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> > > > > > ---
> > > > > >   MAINTAINERS                          |  2 +-
> > > > > >   drivers/misc/Kconfig                 | 13 +------------
> > > > > >   drivers/misc/Makefile                |  2 +-
> > > > > >   drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
> > > > > >   drivers/misc/fastrpc/Makefile        |  2 ++
> > > > > >   drivers/misc/{ => fastrpc}/fastrpc.c |  0
> > > > > >   6 files changed, 21 insertions(+), 14 deletions(-)
> > > > > >   create mode 100644 drivers/misc/fastrpc/Kconfig
> > > > > >   create mode 100644 drivers/misc/fastrpc/Makefile
> > > > > >   rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)
> > > > > 
> > > > > Please consider whether it makes sense to move to drivers/accel instead
> > > > > (and possibly writing a better Kconfig entry, specifying that the driver
> > > > > is to be used to offload execution to the DSP).
> > > > > 
> > > > 
> > > > Wouldn't this come with the expectation of following the ABIs of
> > > > drivers/accel and thereby breaking userspace?
> > > 
> > > As I wrote earlier, that depends on the accel/ maintainers decision,
> > > whether it's acceptable to have non-DRM_ACCEL code underneath.
> > > But at least I'd try doing that on the grounds of keeping the code at
> > > the proper place in the drivers/ tree, raising awareness of the
> > > FastRPC, etc.
> > > For example current fastrpc driver bypasses dri-devel reviews, while
> > > if I remember correctly, at some point it was suggested that all
> > > dma-buf-handling drivers should also notify the dri-devel ML.

If the agreement is that dma-buf-handling drivers must get reviews from
dri-devel, then let's document that in MAINTAINERS and agree with the
maintainer.

There's no need to move the driver for that.

> > > 
> > > Also having the driver under drivers/accels makes it possible and
> > > logical to  implement DRM_ACCEL uAPI at some point. In the ideal world
> > > we should be able to declare existing FastRPC uAPI as legacy /
> > > deprecated / backwards compatibility only and migrate to the
> > > recommended uAPI approach, which is DRM_ACCEL.
> > > 
> > 
> > I suspect Vetter/Airlie need to be involved in this.
> > 
> > Its my understanding that accelerator drivers are able to reside in misc as
> > long as there is no use of dma-buf.  Use of dma-buf means they need to be in
> > drm/accel.
> > 
> > There is precedent for moving a driver from misc to accel (HabanaLabs).
> > 
> > Right now, I'm not aware that fastRPC meets the requirements for drm/accel.
> > There is an open source userspace driver, but I'm not aware of an open
> > source compiler.  From what I know of the architecture, it should be
> > possible to utilize upstream LLVM to produce one.
> 
> Yeah so fastrpc is one of the reasons why I've added a dma_buf regex match
> to MAINTAINERS, and given this move has shown up here on dri-devel that
> seems to work.
> 

Sounds good.

> But also, it slipped through, can't break uapi, so I just pretend it's not
> really there :-)
> 

There's a small, but growing userbase of the current upstream fastrpc
uAPI. If there are benefits and a migration path, I think it's
reasonable to explore that - at this point, but probably not much later.

> That aside, going forward it might make sense to look into drivers/accel,
> and also going forward new dma_buf uapi will be reviewed to fairly
> stringent standards. We're not going to impose the dri-devel userspace
> rules on everyone, each subsystem tends to know what's best in their
> ecosystem.

> But if something just ends up in misc so it can avoid the drm
> or accel rules (and I think media is also pretty much on the same page
> nowadays), then expect some serious heat ...
> 

Certainly sounds reasonable to avoid adding new accel-drivers in
drivers/misc.

Regards,
Bjorn

> Cheers, Sima
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch

Re: [PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Dmitry Baryshkov]

On Sun, Jun 23, 2024 at 03:19:17PM GMT, Bjorn Andersson wrote:
> On Fri, Jun 21, 2024 at 05:52:32PM GMT, Daniel Vetter wrote:
> > On Fri, Jun 21, 2024 at 09:40:09AM -0600, Jeffrey Hugo wrote:
> > > On 6/21/2024 5:19 AM, Dmitry Baryshkov wrote:
> > > > On Fri, 21 Jun 2024 at 09:19, Bjorn Andersson <andersson@kernel.org> wrote:
> > > > > 
> > > > > On Wed, Jun 12, 2024 at 09:28:39PM GMT, Dmitry Baryshkov wrote:
> > > > > > On Wed, Jun 12, 2024 at 12:17:28PM +0530, Ekansh Gupta wrote:
> > > > > > > Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
> > > > > > > to be added for PD notifications and other missing features. Adding
> > > > > > > and maintaining new files from within fastrpc directory would be easy.
> > > > > > > 
> > > > > > > Example of feature that is being planned to be introduced in a new C
> > > > > > > file:
> > > > > > > https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/
> > > > > > > 
> > > > > > > Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> > > > > > > ---
> > > > > > >   MAINTAINERS                          |  2 +-
> > > > > > >   drivers/misc/Kconfig                 | 13 +------------
> > > > > > >   drivers/misc/Makefile                |  2 +-
> > > > > > >   drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
> > > > > > >   drivers/misc/fastrpc/Makefile        |  2 ++
> > > > > > >   drivers/misc/{ => fastrpc}/fastrpc.c |  0
> > > > > > >   6 files changed, 21 insertions(+), 14 deletions(-)
> > > > > > >   create mode 100644 drivers/misc/fastrpc/Kconfig
> > > > > > >   create mode 100644 drivers/misc/fastrpc/Makefile
> > > > > > >   rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)
> > > > > > 
> > > > > > Please consider whether it makes sense to move to drivers/accel instead
> > > > > > (and possibly writing a better Kconfig entry, specifying that the driver
> > > > > > is to be used to offload execution to the DSP).
> > > > > > 
> > > > > 
> > > > > Wouldn't this come with the expectation of following the ABIs of
> > > > > drivers/accel and thereby breaking userspace?
> > > > 
> > > > As I wrote earlier, that depends on the accel/ maintainers decision,
> > > > whether it's acceptable to have non-DRM_ACCEL code underneath.
> > > > But at least I'd try doing that on the grounds of keeping the code at
> > > > the proper place in the drivers/ tree, raising awareness of the
> > > > FastRPC, etc.
> > > > For example current fastrpc driver bypasses dri-devel reviews, while
> > > > if I remember correctly, at some point it was suggested that all
> > > > dma-buf-handling drivers should also notify the dri-devel ML.
> 
> If the agreement is that dma-buf-handling drivers must get reviews from
> dri-devel, then let's document that in MAINTAINERS and agree with the
> maintainer.

Good idea.

> 
> There's no need to move the driver for that.

-- 
With best wishes
Dmitry

Re: [PATCH v1] misc: fastrpc: Move fastrpc driver to misc/fastrpc/

[Daniel Vetter]

On Sun, Jun 23, 2024 at 03:19:17PM -0500, Bjorn Andersson wrote:
> On Fri, Jun 21, 2024 at 05:52:32PM GMT, Daniel Vetter wrote:
> > On Fri, Jun 21, 2024 at 09:40:09AM -0600, Jeffrey Hugo wrote:
> > > On 6/21/2024 5:19 AM, Dmitry Baryshkov wrote:
> > > > On Fri, 21 Jun 2024 at 09:19, Bjorn Andersson <andersson@kernel.org> wrote:
> > > > > 
> > > > > On Wed, Jun 12, 2024 at 09:28:39PM GMT, Dmitry Baryshkov wrote:
> > > > > > On Wed, Jun 12, 2024 at 12:17:28PM +0530, Ekansh Gupta wrote:
> > > > > > > Move fastrpc.c from misc/ to misc/fastrpc/. New C files are planned
> > > > > > > to be added for PD notifications and other missing features. Adding
> > > > > > > and maintaining new files from within fastrpc directory would be easy.
> > > > > > > 
> > > > > > > Example of feature that is being planned to be introduced in a new C
> > > > > > > file:
> > > > > > > https://lore.kernel.org/all/20240606165939.12950-6-quic_ekangupt@quicinc.com/
> > > > > > > 
> > > > > > > Signed-off-by: Ekansh Gupta <quic_ekangupt@quicinc.com>
> > > > > > > ---
> > > > > > >   MAINTAINERS                          |  2 +-
> > > > > > >   drivers/misc/Kconfig                 | 13 +------------
> > > > > > >   drivers/misc/Makefile                |  2 +-
> > > > > > >   drivers/misc/fastrpc/Kconfig         | 16 ++++++++++++++++
> > > > > > >   drivers/misc/fastrpc/Makefile        |  2 ++
> > > > > > >   drivers/misc/{ => fastrpc}/fastrpc.c |  0
> > > > > > >   6 files changed, 21 insertions(+), 14 deletions(-)
> > > > > > >   create mode 100644 drivers/misc/fastrpc/Kconfig
> > > > > > >   create mode 100644 drivers/misc/fastrpc/Makefile
> > > > > > >   rename drivers/misc/{ => fastrpc}/fastrpc.c (100%)
> > > > > > 
> > > > > > Please consider whether it makes sense to move to drivers/accel instead
> > > > > > (and possibly writing a better Kconfig entry, specifying that the driver
> > > > > > is to be used to offload execution to the DSP).
> > > > > > 
> > > > > 
> > > > > Wouldn't this come with the expectation of following the ABIs of
> > > > > drivers/accel and thereby breaking userspace?
> > > > 
> > > > As I wrote earlier, that depends on the accel/ maintainers decision,
> > > > whether it's acceptable to have non-DRM_ACCEL code underneath.
> > > > But at least I'd try doing that on the grounds of keeping the code at
> > > > the proper place in the drivers/ tree, raising awareness of the
> > > > FastRPC, etc.
> > > > For example current fastrpc driver bypasses dri-devel reviews, while
> > > > if I remember correctly, at some point it was suggested that all
> > > > dma-buf-handling drivers should also notify the dri-devel ML.
> 
> If the agreement is that dma-buf-handling drivers must get reviews from
> dri-devel, then let's document that in MAINTAINERS and agree with the
> maintainer.

About 5 years ahead of you here with 78baee8d3b97 ("MAINTAINERS: Match on
dma_buf|fence|resv anywhere").

Cheers, Sima
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch