lkml.org 
[lkml]   [2022]   [Nov]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    Date
    SubjectRe: [RFC PATCH v2 1/3] drivers/accel: define kconfig and register a new major
    On Mon, Nov 7, 2022 at 2:56 PM Jason Gunthorpe <jgg@nvidia.com> wrote:
    >
    > On Thu, Nov 03, 2022 at 10:39:36PM +0200, Oded Gabbay wrote:
    > > On Thu, Nov 3, 2022 at 3:31 PM Oded Gabbay <ogabbay@kernel.org> wrote:
    > > >
    > > > On Thu, Nov 3, 2022 at 2:31 AM Greg Kroah-Hartman
    > > > <gregkh@linuxfoundation.org> wrote:
    > > > >
    > > > > On Wed, Nov 02, 2022 at 10:34:03PM +0200, Oded Gabbay wrote:
    > > > > > --- /dev/null
    > > > > > +++ b/drivers/accel/Kconfig
    > > > > > @@ -0,0 +1,24 @@
    > > > > > +# SPDX-License-Identifier: GPL-2.0-only
    > > > > > +#
    > > > > > +# Compute Acceleration device configuration
    > > > > > +#
    > > > > > +# This framework provides support for compute acceleration devices, such
    > > > > > +# as, but not limited to, Machine-Learning and Deep-Learning acceleration
    > > > > > +# devices
    > > > > > +#
    > > > > > +menuconfig ACCEL
    > > > > > + tristate "Compute Acceleration Framework"
    > > > > > + depends on DRM
    > > > > > + help
    > > > > > + Framework for device drivers of compute acceleration devices, such
    > > > > > + as, but not limited to, Machine-Learning and Deep-Learning
    > > > > > + acceleration devices.
    > > > > > + If you say Y here, you need to select the module that's right for
    > > > > > + your acceleration device from the list below.
    > > > > > + This framework is integrated with the DRM subsystem as compute
    > > > > > + accelerators and GPUs share a lot in common and can use almost the
    > > > > > + same infrastructure code.
    > > > > > + Having said that, acceleration devices will have a different
    > > > > > + major number than GPUs, and will be exposed to user-space using
    > > > > > + different device files, called accel/accel* (in /dev, sysfs
    > > > > > + and debugfs)
    > > > >
    > > > > Module name if "M" is chosen?
    > > > Will add
    > > So, unfortunately, the path of doing accel as a kernel module won't
    > > work cleanly (Thanks stanislaw for pointing this out to me).
    > > The reason is the circular dependency between drm and accel. drm calls
    > > accel exported symbols during init and when devices are registering
    > > (all the minor handling), and accel calls drm exported symbols because
    > > I don't want to duplicate the entire drm core code.
    >
    > I really don't think this is the right way to integrate with
    > DRM. Accel should be a layer over top of DRM, not have these wakky
    > co-dependencies.
    >
    > The fact you are running into stuff like this already smells really
    > bad.
    >
    > Jason
    I don't agree with your statement that it should be "a layer over top of DRM".
    Anything on top of DRM is a device driver.
    Accel is not a device driver, it is a new type of drm minor / drm driver.

    Please look at v3 of the patch-set. There I abandoned the idea of
    having accel as a separate module and instead it is part of drm.ko, as
    it should be because it is just a new drm minor.

    The only alternative imo to that is to abandon the idea of reusing
    drm, and just make an independant accel core code.

    Oded

    \
     
     \ /
      Last update: 2022-11-07 14:02    [W:2.813 / U:0.708 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site