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@xxxxxxxxxx> 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@xxxxxxxxxxx/ > > > > > > > > > > > > Signed-off-by: Ekansh Gupta <quic_ekangupt@xxxxxxxxxxx> > > > > > > --- > > > > > > 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