Re: [RFC PATCH 0/3] new subsystem for compute accelerator devices

Re: [RFC PATCH 0/3] new subsystem for compute accelerator devices

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 3268 bytes --]

On Sun, Oct 23, 2022 at 12:46:19AM +0300, Oded Gabbay wrote:
> In the last couple of months we had a discussion [1] about creating a new
> subsystem for compute accelerator devices in the kernel.
> 
> After an analysis that was done by DRM maintainers and myself, and following
> a BOF session at the Linux Plumbers conference a few weeks ago [2], we
> decided to create a new subsystem that will use the DRM subsystem's code and
> functionality. i.e. the accel core code will be part of the DRM subsystem.
> 
> This will allow us to leverage the extensive DRM code-base and
> collaborate with DRM developers that have experience with this type of
> devices. In addition, new features that will be added for the accelerator
> drivers can be of use to GPU drivers as well (e.g. RAS).
> 
> As agreed in the BOF session, the accelerator devices will be exposed to
> user-space with a new, dedicated device char files and a dedicated major
> number (261), to clearly separate them from graphic cards and the graphic
> user-space s/w stack. Furthermore, the drivers will be located in a separate
> place in the kernel tree (drivers/accel/).
> 
> This series of patches is the first step in this direction as it adds the
> necessary infrastructure for accelerator devices to DRM. The new devices will
> be exposed with the following convention:
> 
> device char files - /dev/accel/accel*
> sysfs             - /sys/class/accel/accel*/
> debugfs           - /sys/kernel/debug/accel/accel*/
> 
> I tried to reuse the existing DRM code as much as possible, while keeping it
> readable and maintainable.
> 
> One thing that is missing from this series is defining a namespace for the
> new accel subsystem, while I'll add in the next iteration of this patch-set,
> after I will receive feedback from the community.
> 
> As for drivers, once this series will be accepted (after adding the namespace),
> I will start working on migrating the habanalabs driver to the new accel
> subsystem. I have talked about it with Dave and we agreed that it will be
> a good start to simply move the driver as-is with minimal changes, and then
> start working on the driver's individual features that will be either added
> to the accel core code (with or without changes), or will be removed and
> instead the driver will use existing DRM code.
> 
> In addition, I know of at least 3 or 4 drivers that were submitted for review
> and are good candidates to be included in this new subsystem, instead of being
> a drm render node driver or a misc driver.
> 
> [1] https://lkml.org/lkml/2022/7/31/83
> [2] https://airlied.blogspot.com/2022/09/accelerators-bof-outcomes-summary.html
> 

Since this is new subsystem, it should have its own git tree where you
collected accelerator-related patches. By convention, there should be
"next" branch targeting for next kernel release and "fixes" branch for
bugfixes pending for current release. Both branches should be included
into linux-next. The names don't necessarily be that, though.

Also, it had been great if you write short, descriptive documentation
about the subsystem (maintainers handbook).

Cc'ing linux-doc folks.

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC PATCH 0/3] new subsystem for compute accelerator devices

[Greg Kroah-Hartman]

On Sun, Oct 23, 2022 at 09:02:49PM +0700, Bagas Sanjaya wrote:
> On Sun, Oct 23, 2022 at 12:46:19AM +0300, Oded Gabbay wrote:
> > In the last couple of months we had a discussion [1] about creating a new
> > subsystem for compute accelerator devices in the kernel.
> > 
> > After an analysis that was done by DRM maintainers and myself, and following
> > a BOF session at the Linux Plumbers conference a few weeks ago [2], we
> > decided to create a new subsystem that will use the DRM subsystem's code and
> > functionality. i.e. the accel core code will be part of the DRM subsystem.
> > 
> > This will allow us to leverage the extensive DRM code-base and
> > collaborate with DRM developers that have experience with this type of
> > devices. In addition, new features that will be added for the accelerator
> > drivers can be of use to GPU drivers as well (e.g. RAS).
> > 
> > As agreed in the BOF session, the accelerator devices will be exposed to
> > user-space with a new, dedicated device char files and a dedicated major
> > number (261), to clearly separate them from graphic cards and the graphic
> > user-space s/w stack. Furthermore, the drivers will be located in a separate
> > place in the kernel tree (drivers/accel/).
> > 
> > This series of patches is the first step in this direction as it adds the
> > necessary infrastructure for accelerator devices to DRM. The new devices will
> > be exposed with the following convention:
> > 
> > device char files - /dev/accel/accel*
> > sysfs             - /sys/class/accel/accel*/
> > debugfs           - /sys/kernel/debug/accel/accel*/
> > 
> > I tried to reuse the existing DRM code as much as possible, while keeping it
> > readable and maintainable.
> > 
> > One thing that is missing from this series is defining a namespace for the
> > new accel subsystem, while I'll add in the next iteration of this patch-set,
> > after I will receive feedback from the community.
> > 
> > As for drivers, once this series will be accepted (after adding the namespace),
> > I will start working on migrating the habanalabs driver to the new accel
> > subsystem. I have talked about it with Dave and we agreed that it will be
> > a good start to simply move the driver as-is with minimal changes, and then
> > start working on the driver's individual features that will be either added
> > to the accel core code (with or without changes), or will be removed and
> > instead the driver will use existing DRM code.
> > 
> > In addition, I know of at least 3 or 4 drivers that were submitted for review
> > and are good candidates to be included in this new subsystem, instead of being
> > a drm render node driver or a misc driver.
> > 
> > [1] https://lkml.org/lkml/2022/7/31/83
> > [2] https://airlied.blogspot.com/2022/09/accelerators-bof-outcomes-summary.html
> > 
> 
> Since this is new subsystem, it should have its own git tree where you
> collected accelerator-related patches.

No, that is never a requirement, where did you get that idea?

> By convention, there should be
> "next" branch targeting for next kernel release and "fixes" branch for
> bugfixes pending for current release. Both branches should be included
> into linux-next. The names don't necessarily be that, though.

Again, no, that has never been a requirement.

> Also, it had been great if you write short, descriptive documentation
> about the subsystem (maintainers handbook).

Also no, this is fine, it's an RFC sent to all of the people involved in
the discussions about this new subsystem.  The changelog here is totally
sufficient.

Please do not confuse people and ask them to do things that are not
requirements, that's not helpful at all.

greg k-h