This is the fourth (and hopefully last) version of the patch-set to add the
new subsystem for compute accelerators. I removed the RFC headline as
I believe it is now ready for merging.
Compare to v3, this patch-set contains one additional patch that adds
documentation regarding the accel subsystem. I hope it's good enough for
this stage. In addition, there were few very minor fixes according to
comments received on v3.
The patches are in the following repo:
https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4
As in v3, The HEAD of that branch is a commit adding a dummy driver that
registers an accel device using the new framework. This can be served
as a simple reference.
v1 cover letter:
https://lkml.org/lkml/2022/10/22/544
v2 cover letter:
https://lore.kernel.org/lkml/[email protected]/T/
v3 cover letter:
https://lore.kernel.org/lkml/[email protected]/T/
Thanks,
Oded.
Oded Gabbay (4):
drivers/accel: define kconfig and register a new major
accel: add dedicated minor for accelerator devices
drm: initialize accel framework
doc: add documentation for accel subsystem
Documentation/accel/index.rst | 17 ++
Documentation/accel/introduction.rst | 109 +++++++++
Documentation/admin-guide/devices.txt | 5 +
Documentation/subsystem-apis.rst | 1 +
MAINTAINERS | 9 +
drivers/Kconfig | 2 +
drivers/accel/Kconfig | 24 ++
drivers/accel/drm_accel.c | 323 ++++++++++++++++++++++++++
drivers/gpu/drm/Makefile | 1 +
drivers/gpu/drm/drm_drv.c | 102 +++++---
drivers/gpu/drm/drm_file.c | 2 +-
drivers/gpu/drm/drm_sysfs.c | 24 +-
include/drm/drm_accel.h | 97 ++++++++
include/drm/drm_device.h | 3 +
include/drm/drm_drv.h | 8 +
include/drm/drm_file.h | 21 +-
16 files changed, 711 insertions(+), 37 deletions(-)
create mode 100644 Documentation/accel/index.rst
create mode 100644 Documentation/accel/introduction.rst
create mode 100644 drivers/accel/Kconfig
create mode 100644 drivers/accel/drm_accel.c
create mode 100644 include/drm/drm_accel.h
--
2.25.1
Add an introduction section for the accel subsystem. Most of the
relevant data is in the DRM documentation, so the introduction only
presents the why of the new subsystem, how are the compute accelerators
exposed to user-space and what changes need to be done in a standard
DRM driver to register it to the new accel subsystem.
Signed-off-by: Oded Gabbay <[email protected]>
---
Documentation/accel/index.rst | 17 +++++
Documentation/accel/introduction.rst | 109 +++++++++++++++++++++++++++
Documentation/subsystem-apis.rst | 1 +
MAINTAINERS | 1 +
4 files changed, 128 insertions(+)
create mode 100644 Documentation/accel/index.rst
create mode 100644 Documentation/accel/introduction.rst
diff --git a/Documentation/accel/index.rst b/Documentation/accel/index.rst
new file mode 100644
index 000000000000..2b43c9a7f67b
--- /dev/null
+++ b/Documentation/accel/index.rst
@@ -0,0 +1,17 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+====================
+Compute Accelerators
+====================
+
+.. toctree::
+ :maxdepth: 1
+
+ introduction
+
+.. only:: subproject and html
+
+ Indices
+ =======
+
+ * :ref:`genindex`
diff --git a/Documentation/accel/introduction.rst b/Documentation/accel/introduction.rst
new file mode 100644
index 000000000000..5a3963eae973
--- /dev/null
+++ b/Documentation/accel/introduction.rst
@@ -0,0 +1,109 @@
+.. SPDX-License-Identifier: GPL-2.0
+
+============
+Introduction
+============
+
+The Linux compute accelerators subsystem is designed to expose compute
+accelerators in a common way to user-space and provide a common set of
+functionality.
+
+These devices can be either stand-alone ASICs or IP blocks inside an SoC/GPU.
+Although these devices are typically designed to accelerate Machine-Learning
+and/or Deep-Learning computations, the accel layer is not limited to handling
+these types of accelerators.
+
+typically, a compute accelerator will belong to one of the following
+categories:
+
+- Edge AI - doing inference at an edge device. It can be an embedded ASIC/FPGA,
+ or an IP inside a SoC (e.g. laptop web camera). These devices
+ are typically configured using registers and can work with or without DMA.
+
+- Inference data-center - single/multi user devices in a large server. This
+ type of device can be stand-alone or an IP inside a SoC or a GPU. It will
+ have on-board DRAM (to hold the DL topology), DMA engines and
+ command submission queues (either kernel or user-space queues).
+ It might also have an MMU to manage multiple users and might also enable
+ virtualization (SR-IOV) to support multiple VMs on the same device. In
+ addition, these devices will usually have some tools, such as profiler and
+ debugger.
+
+- Training data-center - Similar to Inference data-center cards, but typically
+ have more computational power and memory b/w (e.g. HBM) and will likely have
+ a method of scaling-up/out, i.e. connecting to other training cards inside
+ the server or in other servers, respectively.
+
+All these devices typically have different runtime user-space software stacks,
+that are tailored-made to their h/w. In addition, they will also probably
+include a compiler to generate programs to their custom-made computational
+engines. Typically, the common layer in user-space will be the DL frameworks,
+such as PyTorch and TensorFlow.
+
+Sharing code with DRM
+=====================
+
+Because this type of devices can be an IP inside GPUs or have similar
+characteristics as those of GPUs, the accel subsystem will use the
+DRM subsystem's code and functionality. i.e. the accel core code will
+be part of the DRM subsystem and an accel device will be a new type of DRM
+device.
+
+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.
+
+Differentiation from GPUs
+=========================
+
+Because we want to prevent the extensive user-space graphic software stack
+from trying to use an accelerator as a GPU, the compute accelerators will be
+differentiated from GPUs by using a new major number and new device char files.
+
+Furthermore, the drivers will be located in a separate place in the kernel
+tree - drivers/accel/.
+
+The accelerator devices will be exposed to the user space with the dedicated
+261 major number and will have the following convention:
+
+- device char files - /dev/accel/accel*
+- sysfs - /sys/class/accel/accel*/
+- debugfs - /sys/kernel/debug/accel/accel*/
+
+Getting Started
+===============
+
+First, read the DRM documentation. Not only it will explain how to write a new
+DRM driver but it will also contain all the information on how to contribute,
+the Code Of Conduct and what is the coding style/documentation. All of that
+is the same for the accel subsystem.
+
+Second, make sure the kernel is configured with CONFIG_DRM_ACCEL.
+
+To expose your device as an accelerator, two changes are needed to
+be done in your driver (as opposed to a standard DRM driver):
+
+- Add the DRIVER_COMPUTE_ACCEL feature flag in your drm_driver's
+ driver_features field. It is important to note that this driver feature is
+ mutually exclusive with DRIVER_RENDER and DRIVER_MODESET. Devices that want
+ to expose both graphics and compute device char files should be handled by
+ two drivers that are connected using the auxiliary bus framework.
+
+- Change the open callback in your driver fops structure to accel_open().
+ Alternatively, your driver can use DEFINE_DRM_ACCEL_FOPS macro to easily
+ set the correct function operations pointers structure.
+
+External References
+===================
+
+email threads
+-------------
+
+* `Initial discussion on the New subsystem for acceleration devices <https://lkml.org/lkml/2022/7/31/83>`_ - Oded Gabbay (2022)
+* `patch-set to add the new subsystem <https://lkml.org/lkml/2022/10/22/544>`_ - Oded Gabbay (2022)
+
+Conference talks
+----------------
+
+* `LPC 2022 Accelerators BOF outcomes summary <https://airlied.blogspot.com/2022/09/accelerators-bof-outcomes-summary.html>`_ - Dave Airlie (2022)
diff --git a/Documentation/subsystem-apis.rst b/Documentation/subsystem-apis.rst
index af65004a80aa..b51f38527e14 100644
--- a/Documentation/subsystem-apis.rst
+++ b/Documentation/subsystem-apis.rst
@@ -43,6 +43,7 @@ needed).
input/index
hwmon/index
gpu/index
+ accel/index
security/index
sound/index
crypto/index
diff --git a/MAINTAINERS b/MAINTAINERS
index 4d752aac3ec0..6ba7bb35208a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6837,6 +6837,7 @@ L: [email protected]
S: Maintained
C: irc://irc.oftc.net/dri-devel
T: git https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git
+F: Documentation/accel/
F: drivers/accel/
DRM DRIVERS FOR ALLWINNER A10
--
2.25.1
Now that we have the accel framework code ready, let's call the
accel functions from all the appropriate places. These places are the
drm module init/exit functions, and all the drm_minor handling
functions.
Signed-off-by: Oded Gabbay <[email protected]>
---
drivers/gpu/drm/drm_drv.c | 102 ++++++++++++++++++++++++++----------
drivers/gpu/drm/drm_sysfs.c | 24 ++++++---
2 files changed, 91 insertions(+), 35 deletions(-)
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 8214a0b1ab7f..1aec019f6389 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -35,6 +35,7 @@
#include <linux/slab.h>
#include <linux/srcu.h>
+#include <drm/drm_accel.h>
#include <drm/drm_cache.h>
#include <drm/drm_client.h>
#include <drm/drm_color_mgmt.h>
@@ -90,6 +91,8 @@ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev,
return &dev->primary;
case DRM_MINOR_RENDER:
return &dev->render;
+ case DRM_MINOR_ACCEL:
+ return &dev->accel;
default:
BUG();
}
@@ -104,9 +107,13 @@ static void drm_minor_alloc_release(struct drm_device *dev, void *data)
put_device(minor->kdev);
- spin_lock_irqsave(&drm_minor_lock, flags);
- idr_remove(&drm_minors_idr, minor->index);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ if (minor->type == DRM_MINOR_ACCEL) {
+ accel_minor_remove(minor->index);
+ } else {
+ spin_lock_irqsave(&drm_minor_lock, flags);
+ idr_remove(&drm_minors_idr, minor->index);
+ spin_unlock_irqrestore(&drm_minor_lock, flags);
+ }
}
static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
@@ -123,13 +130,17 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
minor->dev = dev;
idr_preload(GFP_KERNEL);
- spin_lock_irqsave(&drm_minor_lock, flags);
- r = idr_alloc(&drm_minors_idr,
- NULL,
- 64 * type,
- 64 * (type + 1),
- GFP_NOWAIT);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ if (type == DRM_MINOR_ACCEL) {
+ r = accel_minor_alloc();
+ } else {
+ spin_lock_irqsave(&drm_minor_lock, flags);
+ r = idr_alloc(&drm_minors_idr,
+ NULL,
+ 64 * type,
+ 64 * (type + 1),
+ GFP_NOWAIT);
+ spin_unlock_irqrestore(&drm_minor_lock, flags);
+ }
idr_preload_end();
if (r < 0)
@@ -161,10 +172,14 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
if (!minor)
return 0;
- ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root);
- if (ret) {
- DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
- goto err_debugfs;
+ if (minor->type == DRM_MINOR_ACCEL) {
+ accel_debugfs_init(minor, minor->index);
+ } else {
+ ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root);
+ if (ret) {
+ DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
+ goto err_debugfs;
+ }
}
ret = device_add(minor->kdev);
@@ -172,9 +187,13 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
goto err_debugfs;
/* replace NULL with @minor so lookups will succeed from now on */
- spin_lock_irqsave(&drm_minor_lock, flags);
- idr_replace(&drm_minors_idr, minor, minor->index);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ if (minor->type == DRM_MINOR_ACCEL) {
+ accel_minor_replace(minor, minor->index);
+ } else {
+ spin_lock_irqsave(&drm_minor_lock, flags);
+ idr_replace(&drm_minors_idr, minor, minor->index);
+ spin_unlock_irqrestore(&drm_minor_lock, flags);
+ }
DRM_DEBUG("new minor registered %d\n", minor->index);
return 0;
@@ -194,9 +213,13 @@ static void drm_minor_unregister(struct drm_device *dev, unsigned int type)
return;
/* replace @minor with NULL so lookups will fail from now on */
- spin_lock_irqsave(&drm_minor_lock, flags);
- idr_replace(&drm_minors_idr, NULL, minor->index);
- spin_unlock_irqrestore(&drm_minor_lock, flags);
+ if (minor->type == DRM_MINOR_ACCEL) {
+ accel_minor_replace(NULL, minor->index);
+ } else {
+ spin_lock_irqsave(&drm_minor_lock, flags);
+ idr_replace(&drm_minors_idr, NULL, minor->index);
+ spin_unlock_irqrestore(&drm_minor_lock, flags);
+ }
device_del(minor->kdev);
dev_set_drvdata(minor->kdev, NULL); /* safety belt */
@@ -603,6 +626,14 @@ static int drm_dev_init(struct drm_device *dev,
/* no per-device feature limits by default */
dev->driver_features = ~0u;
+ if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL) &&
+ (drm_core_check_feature(dev, DRIVER_RENDER) ||
+ drm_core_check_feature(dev, DRIVER_MODESET))) {
+
+ DRM_ERROR("DRM driver can't be both a compute acceleration and graphics driver\n");
+ return -EINVAL;
+ }
+
drm_legacy_init_members(dev);
INIT_LIST_HEAD(&dev->filelist);
INIT_LIST_HEAD(&dev->filelist_internal);
@@ -628,15 +659,21 @@ static int drm_dev_init(struct drm_device *dev,
dev->anon_inode = inode;
- if (drm_core_check_feature(dev, DRIVER_RENDER)) {
- ret = drm_minor_alloc(dev, DRM_MINOR_RENDER);
+ if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL)) {
+ ret = drm_minor_alloc(dev, DRM_MINOR_ACCEL);
if (ret)
goto err;
- }
+ } else {
+ if (drm_core_check_feature(dev, DRIVER_RENDER)) {
+ ret = drm_minor_alloc(dev, DRM_MINOR_RENDER);
+ if (ret)
+ goto err;
+ }
- ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
- if (ret)
- goto err;
+ ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
+ if (ret)
+ goto err;
+ }
ret = drm_legacy_create_map_hash(dev);
if (ret)
@@ -883,6 +920,10 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
if (ret)
goto err_minors;
+ ret = drm_minor_register(dev, DRM_MINOR_ACCEL);
+ if (ret)
+ goto err_minors;
+
ret = create_compat_control_link(dev);
if (ret)
goto err_minors;
@@ -902,12 +943,13 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
driver->name, driver->major, driver->minor,
driver->patchlevel, driver->date,
dev->dev ? dev_name(dev->dev) : "virtual device",
- dev->primary->index);
+ dev->primary ? dev->primary->index : dev->accel->index);
goto out_unlock;
err_minors:
remove_compat_control_link(dev);
+ drm_minor_unregister(dev, DRM_MINOR_ACCEL);
drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
drm_minor_unregister(dev, DRM_MINOR_RENDER);
out_unlock:
@@ -950,6 +992,7 @@ void drm_dev_unregister(struct drm_device *dev)
drm_legacy_rmmaps(dev);
remove_compat_control_link(dev);
+ drm_minor_unregister(dev, DRM_MINOR_ACCEL);
drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
drm_minor_unregister(dev, DRM_MINOR_RENDER);
}
@@ -1034,6 +1077,7 @@ static const struct file_operations drm_stub_fops = {
static void drm_core_exit(void)
{
drm_privacy_screen_lookup_exit();
+ accel_core_exit();
unregister_chrdev(DRM_MAJOR, "drm");
debugfs_remove(drm_debugfs_root);
drm_sysfs_destroy();
@@ -1061,6 +1105,10 @@ static int __init drm_core_init(void)
if (ret < 0)
goto error;
+ ret = accel_core_init();
+ if (ret < 0)
+ goto error;
+
drm_privacy_screen_lookup_init();
drm_core_init_complete = true;
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 430e00b16eec..b8da978d85bb 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -19,6 +19,7 @@
#include <linux/kdev_t.h>
#include <linux/slab.h>
+#include <drm/drm_accel.h>
#include <drm/drm_connector.h>
#include <drm/drm_device.h>
#include <drm/drm_file.h>
@@ -471,19 +472,26 @@ struct device *drm_sysfs_minor_alloc(struct drm_minor *minor)
struct device *kdev;
int r;
- if (minor->type == DRM_MINOR_RENDER)
- minor_str = "renderD%d";
- else
- minor_str = "card%d";
-
kdev = kzalloc(sizeof(*kdev), GFP_KERNEL);
if (!kdev)
return ERR_PTR(-ENOMEM);
device_initialize(kdev);
- kdev->devt = MKDEV(DRM_MAJOR, minor->index);
- kdev->class = drm_class;
- kdev->type = &drm_sysfs_device_minor;
+
+ if (minor->type == DRM_MINOR_ACCEL) {
+ minor_str = "accel%d";
+ accel_set_device_instance_params(kdev, minor->index);
+ } else {
+ if (minor->type == DRM_MINOR_RENDER)
+ minor_str = "renderD%d";
+ else
+ minor_str = "card%d";
+
+ kdev->devt = MKDEV(DRM_MAJOR, minor->index);
+ kdev->class = drm_class;
+ kdev->type = &drm_sysfs_device_minor;
+ }
+
kdev->parent = minor->dev->dev;
kdev->release = drm_sysfs_release;
dev_set_drvdata(kdev, minor);
--
2.25.1
Add a new Kconfig for the accel subsystem. The Kconfig currently
contains only the basic CONFIG_DRM_ACCEL option that will be used to
decide whether to compile the accel registration code. Therefore, the
kconfig option is defined as bool.
The accel code will be compiled as part of drm.ko and will be called
directly from the DRM core code. The reason we compile it as part of
drm.ko and not as a separate module is because of cyclic dependency
between drm.ko and the separate module (if it would have existed).
This is due to the fact that DRM core code calls accel functions and
vice-versa.
The accelerator devices will be exposed to the user space with a new,
dedicated major number - 261.
The accel init function registers the new major number as a char device
and create corresponding sysfs and debugfs root entries, similar to
what is done in DRM init function.
I added a new header called drm_accel.h to include/drm/, that will hold
the prototypes of the drm_accel.c functions. In case CONFIG_DRM_ACCEL
is set to 'N', that header will contain empty inline implementations of
those functions, to allow DRM core code to compile successfully
without dependency on CONFIG_DRM_ACCEL.
I Updated the MAINTAINERS file accordingly with the newly added folder
and I have taken the liberty to appropriate the dri-devel mailing list
and the dri-devel IRC channel for the accel subsystem.
Signed-off-by: Oded Gabbay <[email protected]>
---
Changes in v4:
- Fix comment style
- Remove useless goto
- Fix indentation of major define
- Move comment from next patch to this patch
Documentation/admin-guide/devices.txt | 5 ++
MAINTAINERS | 8 +++
drivers/Kconfig | 2 +
drivers/accel/Kconfig | 24 ++++++++
drivers/accel/drm_accel.c | 83 +++++++++++++++++++++++++++
drivers/gpu/drm/Makefile | 1 +
include/drm/drm_accel.h | 32 +++++++++++
7 files changed, 155 insertions(+)
create mode 100644 drivers/accel/Kconfig
create mode 100644 drivers/accel/drm_accel.c
create mode 100644 include/drm/drm_accel.h
diff --git a/Documentation/admin-guide/devices.txt b/Documentation/admin-guide/devices.txt
index 9764d6edb189..06c525e01ea5 100644
--- a/Documentation/admin-guide/devices.txt
+++ b/Documentation/admin-guide/devices.txt
@@ -3080,6 +3080,11 @@
...
255 = /dev/osd255 256th OSD Device
+ 261 char Compute Acceleration Devices
+ 0 = /dev/accel/accel0 First acceleration device
+ 1 = /dev/accel/accel1 Second acceleration device
+ ...
+
384-511 char RESERVED FOR DYNAMIC ASSIGNMENT
Character devices that request a dynamic allocation of major
number will take numbers starting from 511 and downward,
diff --git a/MAINTAINERS b/MAINTAINERS
index 178a7a383b5f..4d752aac3ec0 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6831,6 +6831,14 @@ F: include/drm/drm*
F: include/linux/vga*
F: include/uapi/drm/drm*
+DRM COMPUTE ACCELERATORS DRIVERS AND FRAMEWORK
+M: Oded Gabbay <[email protected]>
+L: [email protected]
+S: Maintained
+C: irc://irc.oftc.net/dri-devel
+T: git https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git
+F: drivers/accel/
+
DRM DRIVERS FOR ALLWINNER A10
M: Maxime Ripard <[email protected]>
M: Chen-Yu Tsai <[email protected]>
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 19ee995bd0ae..968bd0a6fd78 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -99,6 +99,8 @@ source "drivers/media/Kconfig"
source "drivers/video/Kconfig"
+source "drivers/accel/Kconfig"
+
source "sound/Kconfig"
source "drivers/hid/Kconfig"
diff --git a/drivers/accel/Kconfig b/drivers/accel/Kconfig
new file mode 100644
index 000000000000..c9ce849b2984
--- /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 DRM_ACCEL
+ bool "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).
diff --git a/drivers/accel/drm_accel.c b/drivers/accel/drm_accel.c
new file mode 100644
index 000000000000..fac6ad6ac28e
--- /dev/null
+++ b/drivers/accel/drm_accel.c
@@ -0,0 +1,83 @@
+// SPDX-License-Identifier: GPL-2.0
+
+/*
+ * Copyright 2022 HabanaLabs, Ltd.
+ * All Rights Reserved.
+ *
+ */
+
+#include <linux/debugfs.h>
+#include <linux/device.h>
+
+#include <drm/drm_accel.h>
+#include <drm/drm_ioctl.h>
+#include <drm/drm_print.h>
+
+static struct dentry *accel_debugfs_root;
+static struct class *accel_class;
+
+static char *accel_devnode(struct device *dev, umode_t *mode)
+{
+ return kasprintf(GFP_KERNEL, "accel/%s", dev_name(dev));
+}
+
+static int accel_sysfs_init(void)
+{
+ accel_class = class_create(THIS_MODULE, "accel");
+ if (IS_ERR(accel_class))
+ return PTR_ERR(accel_class);
+
+ accel_class->devnode = accel_devnode;
+
+ return 0;
+}
+
+static void accel_sysfs_destroy(void)
+{
+ if (IS_ERR_OR_NULL(accel_class))
+ return;
+ class_destroy(accel_class);
+ accel_class = NULL;
+}
+
+static int accel_stub_open(struct inode *inode, struct file *filp)
+{
+ return -EOPNOTSUPP;
+}
+
+static const struct file_operations accel_stub_fops = {
+ .owner = THIS_MODULE,
+ .open = accel_stub_open,
+ .llseek = noop_llseek,
+};
+
+void accel_core_exit(void)
+{
+ unregister_chrdev(ACCEL_MAJOR, "accel");
+ debugfs_remove(accel_debugfs_root);
+ accel_sysfs_destroy();
+}
+
+int __init accel_core_init(void)
+{
+ int ret;
+
+ ret = accel_sysfs_init();
+ if (ret < 0) {
+ DRM_ERROR("Cannot create ACCEL class: %d\n", ret);
+ goto error;
+ }
+
+ accel_debugfs_root = debugfs_create_dir("accel", NULL);
+
+ ret = register_chrdev(ACCEL_MAJOR, "accel", &accel_stub_fops);
+ if (ret < 0)
+ DRM_ERROR("Cannot register ACCEL major: %d\n", ret);
+
+error:
+ /*
+ * Any cleanup due to errors will be done in drm_core_exit() that
+ * will call accel_core_exit()
+ */
+ return ret;
+}
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index c44a54cadb61..2773e1ad1b6b 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -70,6 +70,7 @@ drm-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
drm-$(CONFIG_DRM_PRIVACY_SCREEN) += \
drm_privacy_screen.o \
drm_privacy_screen_x86.o
+drm-$(CONFIG_DRM_ACCEL) += ../../accel/drm_accel.o
obj-$(CONFIG_DRM) += drm.o
obj-$(CONFIG_DRM_NOMODESET) += drm_nomodeset.o
diff --git a/include/drm/drm_accel.h b/include/drm/drm_accel.h
new file mode 100644
index 000000000000..e1758f484278
--- /dev/null
+++ b/include/drm/drm_accel.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * Copyright 2022 HabanaLabs, Ltd.
+ * All Rights Reserved.
+ *
+ */
+
+#ifndef DRM_ACCEL_H_
+#define DRM_ACCEL_H_
+
+#define ACCEL_MAJOR 261
+
+#if IS_ENABLED(CONFIG_DRM_ACCEL)
+
+void accel_core_exit(void);
+int accel_core_init(void);
+
+#else
+
+static inline void accel_core_exit(void)
+{
+}
+
+static inline int __init accel_core_init(void)
+{
+ /* Return 0 to allow drm_core_init to complete successfully */
+ return 0;
+}
+
+#endif /* IS_ENABLED(CONFIG_DRM_ACCEL) */
+
+#endif /* DRM_ACCEL_H_ */
--
2.25.1
On Sat, Nov 19, 2022 at 10:44:31PM +0200, Oded Gabbay wrote:
> This is the fourth (and hopefully last) version of the patch-set to add the
> new subsystem for compute accelerators. I removed the RFC headline as
> I believe it is now ready for merging.
>
> Compare to v3, this patch-set contains one additional patch that adds
> documentation regarding the accel subsystem. I hope it's good enough for
> this stage. In addition, there were few very minor fixes according to
> comments received on v3.
>
> The patches are in the following repo:
> https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4
>
> As in v3, The HEAD of that branch is a commit adding a dummy driver that
> registers an accel device using the new framework. This can be served
> as a simple reference.
Looks good, thanks for doing this:
Reviewed-by: Greg Kroah-Hartman <[email protected]>
On 11/19/2022 1:44 PM, Oded Gabbay wrote:
> Add an introduction section for the accel subsystem. Most of the
> relevant data is in the DRM documentation, so the introduction only
> presents the why of the new subsystem, how are the compute accelerators
> exposed to user-space and what changes need to be done in a standard
> DRM driver to register it to the new accel subsystem.
>
> Signed-off-by: Oded Gabbay <[email protected]>
> ---
> Documentation/accel/index.rst | 17 +++++
> Documentation/accel/introduction.rst | 109 +++++++++++++++++++++++++++
> Documentation/subsystem-apis.rst | 1 +
> MAINTAINERS | 1 +
> 4 files changed, 128 insertions(+)
> create mode 100644 Documentation/accel/index.rst
> create mode 100644 Documentation/accel/introduction.rst
>
> diff --git a/Documentation/accel/index.rst b/Documentation/accel/index.rst
> new file mode 100644
> index 000000000000..2b43c9a7f67b
> --- /dev/null
> +++ b/Documentation/accel/index.rst
> @@ -0,0 +1,17 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +====================
> +Compute Accelerators
> +====================
> +
> +.. toctree::
> + :maxdepth: 1
> +
> + introduction
> +
> +.. only:: subproject and html
> +
> + Indices
> + =======
> +
> + * :ref:`genindex`
> diff --git a/Documentation/accel/introduction.rst b/Documentation/accel/introduction.rst
> new file mode 100644
> index 000000000000..5a3963eae973
> --- /dev/null
> +++ b/Documentation/accel/introduction.rst
> @@ -0,0 +1,109 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +============
> +Introduction
> +============
> +
> +The Linux compute accelerators subsystem is designed to expose compute
> +accelerators in a common way to user-space and provide a common set of
> +functionality.
> +
> +These devices can be either stand-alone ASICs or IP blocks inside an SoC/GPU.
> +Although these devices are typically designed to accelerate Machine-Learning
> +and/or Deep-Learning computations, the accel layer is not limited to handling
You use "DL" later on as a short form for Deep-Learning. It would be
good to introduce that here.
> +these types of accelerators.
> +
> +typically, a compute accelerator will belong to one of the following
Typically
> +categories:
> +
> +- Edge AI - doing inference at an edge device. It can be an embedded ASIC/FPGA,
> + or an IP inside a SoC (e.g. laptop web camera). These devices
> + are typically configured using registers and can work with or without DMA.
> +
> +- Inference data-center - single/multi user devices in a large server. This
> + type of device can be stand-alone or an IP inside a SoC or a GPU. It will
> + have on-board DRAM (to hold the DL topology), DMA engines and
> + command submission queues (either kernel or user-space queues).
> + It might also have an MMU to manage multiple users and might also enable
> + virtualization (SR-IOV) to support multiple VMs on the same device. In
> + addition, these devices will usually have some tools, such as profiler and
> + debugger.
> +
> +- Training data-center - Similar to Inference data-center cards, but typically
> + have more computational power and memory b/w (e.g. HBM) and will likely have
> + a method of scaling-up/out, i.e. connecting to other training cards inside
> + the server or in other servers, respectively.
> +
> +All these devices typically have different runtime user-space software stacks,
> +that are tailored-made to their h/w. In addition, they will also probably
> +include a compiler to generate programs to their custom-made computational
> +engines. Typically, the common layer in user-space will be the DL frameworks,
> +such as PyTorch and TensorFlow.
> +
> +Sharing code with DRM
> +=====================
> +
> +Because this type of devices can be an IP inside GPUs or have similar
> +characteristics as those of GPUs, the accel subsystem will use the
> +DRM subsystem's code and functionality. i.e. the accel core code will
> +be part of the DRM subsystem and an accel device will be a new type of DRM
> +device.
> +
> +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.
> +
> +Differentiation from GPUs
> +=========================
> +
> +Because we want to prevent the extensive user-space graphic software stack
> +from trying to use an accelerator as a GPU, the compute accelerators will be
> +differentiated from GPUs by using a new major number and new device char files.
> +
> +Furthermore, the drivers will be located in a separate place in the kernel
> +tree - drivers/accel/.
> +
> +The accelerator devices will be exposed to the user space with the dedicated
> +261 major number and will have the following convention:
> +
> +- device char files - /dev/accel/accel*
> +- sysfs - /sys/class/accel/accel*/
> +- debugfs - /sys/kernel/debug/accel/accel*/
> +
> +Getting Started
> +===============
> +
> +First, read the DRM documentation. Not only it will explain how to write a new
How about a link to the DRM documentation?
> +DRM driver but it will also contain all the information on how to contribute,
> +the Code Of Conduct and what is the coding style/documentation. All of that
> +is the same for the accel subsystem.
> +
> +Second, make sure the kernel is configured with CONFIG_DRM_ACCEL.
> +
> +To expose your device as an accelerator, two changes are needed to
> +be done in your driver (as opposed to a standard DRM driver):
> +
> +- Add the DRIVER_COMPUTE_ACCEL feature flag in your drm_driver's
> + driver_features field. It is important to note that this driver feature is
> + mutually exclusive with DRIVER_RENDER and DRIVER_MODESET. Devices that want
I don't remember seeing code that validates a driver with
DRIVER_COMPUTE_ACCEL does not also have DRIVER_MODESET. What am I missing?
> + to expose both graphics and compute device char files should be handled by
> + two drivers that are connected using the auxiliary bus framework.
> +
> +- Change the open callback in your driver fops structure to accel_open().
> + Alternatively, your driver can use DEFINE_DRM_ACCEL_FOPS macro to easily
> + set the correct function operations pointers structure.
> +
> +External References
> +===================
> +
> +email threads
> +-------------
> +
> +* `Initial discussion on the New subsystem for acceleration devices <https://lkml.org/lkml/2022/7/31/83>`_ - Oded Gabbay (2022)
> +* `patch-set to add the new subsystem <https://lkml.org/lkml/2022/10/22/544>`_ - Oded Gabbay (2022)
> +
> +Conference talks
> +----------------
> +
> +* `LPC 2022 Accelerators BOF outcomes summary <https://airlied.blogspot.com/2022/09/accelerators-bof-outcomes-summary.html>`_ - Dave Airlie (2022)
> diff --git a/Documentation/subsystem-apis.rst b/Documentation/subsystem-apis.rst
> index af65004a80aa..b51f38527e14 100644
> --- a/Documentation/subsystem-apis.rst
> +++ b/Documentation/subsystem-apis.rst
> @@ -43,6 +43,7 @@ needed).
> input/index
> hwmon/index
> gpu/index
> + accel/index
> security/index
> sound/index
> crypto/index
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 4d752aac3ec0..6ba7bb35208a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -6837,6 +6837,7 @@ L: [email protected]
> S: Maintained
> C: irc://irc.oftc.net/dri-devel
> T: git https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git
> +F: Documentation/accel/
> F: drivers/accel/
>
> DRM DRIVERS FOR ALLWINNER A10
On 11/19/2022 1:44 PM, Oded Gabbay wrote:
> This is the fourth (and hopefully last) version of the patch-set to add the
> new subsystem for compute accelerators. I removed the RFC headline as
> I believe it is now ready for merging.
>
> Compare to v3, this patch-set contains one additional patch that adds
> documentation regarding the accel subsystem. I hope it's good enough for
> this stage. In addition, there were few very minor fixes according to
> comments received on v3.
>
> The patches are in the following repo:
> https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4
>
> As in v3, The HEAD of that branch is a commit adding a dummy driver that
> registers an accel device using the new framework. This can be served
> as a simple reference.
>
> v1 cover letter:
> https://lkml.org/lkml/2022/10/22/544
>
> v2 cover letter:
> https://lore.kernel.org/lkml/[email protected]/T/
>
> v3 cover letter:
> https://lore.kernel.org/lkml/[email protected]/T/
>
> Thanks,
> Oded.
Reviewed-by: Jeffrey Hugo <[email protected]>
I have some nits. Nothing that I think should be a blocker for this series.
On Sun, 20 Nov 2022 at 06:44, Oded Gabbay <[email protected]> wrote:
>
> This is the fourth (and hopefully last) version of the patch-set to add the
> new subsystem for compute accelerators. I removed the RFC headline as
> I believe it is now ready for merging.
>
> Compare to v3, this patch-set contains one additional patch that adds
> documentation regarding the accel subsystem. I hope it's good enough for
> this stage. In addition, there were few very minor fixes according to
> comments received on v3.
>
> The patches are in the following repo:
> https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4
>
> As in v3, The HEAD of that branch is a commit adding a dummy driver that
> registers an accel device using the new framework. This can be served
> as a simple reference.
>
FIx the nits Jeffery raised and the one I brought up and I think we
should be good for this to be in a PR.
Reviewed-by: Dave Airlie <[email protected]>
Acked-by: Thomas Zimmermann <[email protected]>
Am 19.11.22 um 21:44 schrieb Oded Gabbay:
> This is the fourth (and hopefully last) version of the patch-set to add the
> new subsystem for compute accelerators. I removed the RFC headline as
> I believe it is now ready for merging.
>
> Compare to v3, this patch-set contains one additional patch that adds
> documentation regarding the accel subsystem. I hope it's good enough for
> this stage. In addition, there were few very minor fixes according to
> comments received on v3.
>
> The patches are in the following repo:
> https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4
>
> As in v3, The HEAD of that branch is a commit adding a dummy driver that
> registers an accel device using the new framework. This can be served
> as a simple reference.
>
> v1 cover letter:
> https://lkml.org/lkml/2022/10/22/544
>
> v2 cover letter:
> https://lore.kernel.org/lkml/[email protected]/T/
>
> v3 cover letter:
> https://lore.kernel.org/lkml/[email protected]/T/
>
> Thanks,
> Oded.
>
> Oded Gabbay (4):
> drivers/accel: define kconfig and register a new major
> accel: add dedicated minor for accelerator devices
> drm: initialize accel framework
> doc: add documentation for accel subsystem
>
> Documentation/accel/index.rst | 17 ++
> Documentation/accel/introduction.rst | 109 +++++++++
> Documentation/admin-guide/devices.txt | 5 +
> Documentation/subsystem-apis.rst | 1 +
> MAINTAINERS | 9 +
> drivers/Kconfig | 2 +
> drivers/accel/Kconfig | 24 ++
> drivers/accel/drm_accel.c | 323 ++++++++++++++++++++++++++
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/drm_drv.c | 102 +++++---
> drivers/gpu/drm/drm_file.c | 2 +-
> drivers/gpu/drm/drm_sysfs.c | 24 +-
> include/drm/drm_accel.h | 97 ++++++++
> include/drm/drm_device.h | 3 +
> include/drm/drm_drv.h | 8 +
> include/drm/drm_file.h | 21 +-
> 16 files changed, 711 insertions(+), 37 deletions(-)
> create mode 100644 Documentation/accel/index.rst
> create mode 100644 Documentation/accel/introduction.rst
> create mode 100644 drivers/accel/Kconfig
> create mode 100644 drivers/accel/drm_accel.c
> create mode 100644 include/drm/drm_accel.h
>
> --
> 2.25.1
>
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev
On 11/21/2022 8:18 AM, Oded Gabbay wrote:
> On Mon, Nov 21, 2022 at 12:02 AM Jeffrey Hugo <[email protected]> wrote:
>>
>> On 11/19/2022 1:44 PM, Oded Gabbay wrote:
>>> Add an introduction section for the accel subsystem. Most of the
>>> relevant data is in the DRM documentation, so the introduction only
>>> presents the why of the new subsystem, how are the compute accelerators
>>> exposed to user-space and what changes need to be done in a standard
>>> DRM driver to register it to the new accel subsystem.
>>>
>>> Signed-off-by: Oded Gabbay <[email protected]>
>>> ---
>>> Documentation/accel/index.rst | 17 +++++
>>> Documentation/accel/introduction.rst | 109 +++++++++++++++++++++++++++
>>> Documentation/subsystem-apis.rst | 1 +
>>> MAINTAINERS | 1 +
>>> 4 files changed, 128 insertions(+)
>>> create mode 100644 Documentation/accel/index.rst
>>> create mode 100644 Documentation/accel/introduction.rst
>>>
>>> diff --git a/Documentation/accel/index.rst b/Documentation/accel/index.rst
>>> new file mode 100644
>>> index 000000000000..2b43c9a7f67b
>>> --- /dev/null
>>> +++ b/Documentation/accel/index.rst
>>> @@ -0,0 +1,17 @@
>>> +.. SPDX-License-Identifier: GPL-2.0
>>> +
>>> +====================
>>> +Compute Accelerators
>>> +====================
>>> +
>>> +.. toctree::
>>> + :maxdepth: 1
>>> +
>>> + introduction
>>> +
>>> +.. only:: subproject and html
>>> +
>>> + Indices
>>> + =======
>>> +
>>> + * :ref:`genindex`
>>> diff --git a/Documentation/accel/introduction.rst b/Documentation/accel/introduction.rst
>>> new file mode 100644
>>> index 000000000000..5a3963eae973
>>> --- /dev/null
>>> +++ b/Documentation/accel/introduction.rst
>>> @@ -0,0 +1,109 @@
>>> +.. SPDX-License-Identifier: GPL-2.0
>>> +
>>> +============
>>> +Introduction
>>> +============
>>> +
>>> +The Linux compute accelerators subsystem is designed to expose compute
>>> +accelerators in a common way to user-space and provide a common set of
>>> +functionality.
>>> +
>>> +These devices can be either stand-alone ASICs or IP blocks inside an SoC/GPU.
>>> +Although these devices are typically designed to accelerate Machine-Learning
>>> +and/or Deep-Learning computations, the accel layer is not limited to handling
>>
>> You use "DL" later on as a short form for Deep-Learning. It would be
>> good to introduce that here.
>>
>>> +these types of accelerators.
>>> +
>>> +typically, a compute accelerator will belong to one of the following
>>
>> Typically
>>
>>> +categories:
>>> +
>>> +- Edge AI - doing inference at an edge device. It can be an embedded ASIC/FPGA,
>>> + or an IP inside a SoC (e.g. laptop web camera). These devices
>>> + are typically configured using registers and can work with or without DMA.
>>> +
>>> +- Inference data-center - single/multi user devices in a large server. This
>>> + type of device can be stand-alone or an IP inside a SoC or a GPU. It will
>>> + have on-board DRAM (to hold the DL topology), DMA engines and
>>> + command submission queues (either kernel or user-space queues).
>>> + It might also have an MMU to manage multiple users and might also enable
>>> + virtualization (SR-IOV) to support multiple VMs on the same device. In
>>> + addition, these devices will usually have some tools, such as profiler and
>>> + debugger.
>>> +
>>> +- Training data-center - Similar to Inference data-center cards, but typically
>>> + have more computational power and memory b/w (e.g. HBM) and will likely have
>>> + a method of scaling-up/out, i.e. connecting to other training cards inside
>>> + the server or in other servers, respectively.
>>> +
>>> +All these devices typically have different runtime user-space software stacks,
>>> +that are tailored-made to their h/w. In addition, they will also probably
>>> +include a compiler to generate programs to their custom-made computational
>>> +engines. Typically, the common layer in user-space will be the DL frameworks,
>>> +such as PyTorch and TensorFlow.
>>> +
>>> +Sharing code with DRM
>>> +=====================
>>> +
>>> +Because this type of devices can be an IP inside GPUs or have similar
>>> +characteristics as those of GPUs, the accel subsystem will use the
>>> +DRM subsystem's code and functionality. i.e. the accel core code will
>>> +be part of the DRM subsystem and an accel device will be a new type of DRM
>>> +device.
>>> +
>>> +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.
>>> +
>>> +Differentiation from GPUs
>>> +=========================
>>> +
>>> +Because we want to prevent the extensive user-space graphic software stack
>>> +from trying to use an accelerator as a GPU, the compute accelerators will be
>>> +differentiated from GPUs by using a new major number and new device char files.
>>> +
>>> +Furthermore, the drivers will be located in a separate place in the kernel
>>> +tree - drivers/accel/.
>>> +
>>> +The accelerator devices will be exposed to the user space with the dedicated
>>> +261 major number and will have the following convention:
>>> +
>>> +- device char files - /dev/accel/accel*
>>> +- sysfs - /sys/class/accel/accel*/
>>> +- debugfs - /sys/kernel/debug/accel/accel*/
>>> +
>>> +Getting Started
>>> +===============
>>> +
>>> +First, read the DRM documentation. Not only it will explain how to write a new
>>
>> How about a link to the DRM documentation?
>>
>>> +DRM driver but it will also contain all the information on how to contribute,
>>> +the Code Of Conduct and what is the coding style/documentation. All of that
>>> +is the same for the accel subsystem.
>>> +
>>> +Second, make sure the kernel is configured with CONFIG_DRM_ACCEL.
>>> +
>>> +To expose your device as an accelerator, two changes are needed to
>>> +be done in your driver (as opposed to a standard DRM driver):
>>> +
>>> +- Add the DRIVER_COMPUTE_ACCEL feature flag in your drm_driver's
>>> + driver_features field. It is important to note that this driver feature is
>>> + mutually exclusive with DRIVER_RENDER and DRIVER_MODESET. Devices that want
>>
>> I don't remember seeing code that validates a driver with
>> DRIVER_COMPUTE_ACCEL does not also have DRIVER_MODESET. What am I missing?
>
> Look at drm_dev_init() (patch 3/4):
>
> if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL) &&
> (drm_core_check_feature(dev, DRIVER_RENDER) ||
> drm_core_check_feature(dev, DRIVER_MODESET))) {
> DRM_ERROR("DRM driver can't be both a compute acceleration
> and graphics driver\n");
> return -EINVAL;
> }
Ah. I saw "RENDER", but "MODESET" didn't register in my brain. Thanks
for pointing it out to me. All good here.
On Mon, Nov 21, 2022 at 8:26 AM Dave Airlie <[email protected]> wrote:
>
> On Sun, 20 Nov 2022 at 06:44, Oded Gabbay <[email protected]> wrote:
> >
> > This is the fourth (and hopefully last) version of the patch-set to add the
> > new subsystem for compute accelerators. I removed the RFC headline as
> > I believe it is now ready for merging.
> >
> > Compare to v3, this patch-set contains one additional patch that adds
> > documentation regarding the accel subsystem. I hope it's good enough for
> > this stage. In addition, there were few very minor fixes according to
> > comments received on v3.
> >
> > The patches are in the following repo:
> > https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4
> >
> > As in v3, The HEAD of that branch is a commit adding a dummy driver that
> > registers an accel device using the new framework. This can be served
> > as a simple reference.
> >
>
> FIx the nits Jeffery raised and the one I brought up and I think we
> should be good for this to be in a PR.
>
> Reviewed-by: Dave Airlie <[email protected]>
Sure, np.
Oded
On Mon, Nov 21, 2022 at 12:02 AM Jeffrey Hugo <[email protected]> wrote:
>
> On 11/19/2022 1:44 PM, Oded Gabbay wrote:
> > Add an introduction section for the accel subsystem. Most of the
> > relevant data is in the DRM documentation, so the introduction only
> > presents the why of the new subsystem, how are the compute accelerators
> > exposed to user-space and what changes need to be done in a standard
> > DRM driver to register it to the new accel subsystem.
> >
> > Signed-off-by: Oded Gabbay <[email protected]>
> > ---
> > Documentation/accel/index.rst | 17 +++++
> > Documentation/accel/introduction.rst | 109 +++++++++++++++++++++++++++
> > Documentation/subsystem-apis.rst | 1 +
> > MAINTAINERS | 1 +
> > 4 files changed, 128 insertions(+)
> > create mode 100644 Documentation/accel/index.rst
> > create mode 100644 Documentation/accel/introduction.rst
> >
> > diff --git a/Documentation/accel/index.rst b/Documentation/accel/index.rst
> > new file mode 100644
> > index 000000000000..2b43c9a7f67b
> > --- /dev/null
> > +++ b/Documentation/accel/index.rst
> > @@ -0,0 +1,17 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +====================
> > +Compute Accelerators
> > +====================
> > +
> > +.. toctree::
> > + :maxdepth: 1
> > +
> > + introduction
> > +
> > +.. only:: subproject and html
> > +
> > + Indices
> > + =======
> > +
> > + * :ref:`genindex`
> > diff --git a/Documentation/accel/introduction.rst b/Documentation/accel/introduction.rst
> > new file mode 100644
> > index 000000000000..5a3963eae973
> > --- /dev/null
> > +++ b/Documentation/accel/introduction.rst
> > @@ -0,0 +1,109 @@
> > +.. SPDX-License-Identifier: GPL-2.0
> > +
> > +============
> > +Introduction
> > +============
> > +
> > +The Linux compute accelerators subsystem is designed to expose compute
> > +accelerators in a common way to user-space and provide a common set of
> > +functionality.
> > +
> > +These devices can be either stand-alone ASICs or IP blocks inside an SoC/GPU.
> > +Although these devices are typically designed to accelerate Machine-Learning
> > +and/or Deep-Learning computations, the accel layer is not limited to handling
>
> You use "DL" later on as a short form for Deep-Learning. It would be
> good to introduce that here.
>
> > +these types of accelerators.
> > +
> > +typically, a compute accelerator will belong to one of the following
>
> Typically
>
> > +categories:
> > +
> > +- Edge AI - doing inference at an edge device. It can be an embedded ASIC/FPGA,
> > + or an IP inside a SoC (e.g. laptop web camera). These devices
> > + are typically configured using registers and can work with or without DMA.
> > +
> > +- Inference data-center - single/multi user devices in a large server. This
> > + type of device can be stand-alone or an IP inside a SoC or a GPU. It will
> > + have on-board DRAM (to hold the DL topology), DMA engines and
> > + command submission queues (either kernel or user-space queues).
> > + It might also have an MMU to manage multiple users and might also enable
> > + virtualization (SR-IOV) to support multiple VMs on the same device. In
> > + addition, these devices will usually have some tools, such as profiler and
> > + debugger.
> > +
> > +- Training data-center - Similar to Inference data-center cards, but typically
> > + have more computational power and memory b/w (e.g. HBM) and will likely have
> > + a method of scaling-up/out, i.e. connecting to other training cards inside
> > + the server or in other servers, respectively.
> > +
> > +All these devices typically have different runtime user-space software stacks,
> > +that are tailored-made to their h/w. In addition, they will also probably
> > +include a compiler to generate programs to their custom-made computational
> > +engines. Typically, the common layer in user-space will be the DL frameworks,
> > +such as PyTorch and TensorFlow.
> > +
> > +Sharing code with DRM
> > +=====================
> > +
> > +Because this type of devices can be an IP inside GPUs or have similar
> > +characteristics as those of GPUs, the accel subsystem will use the
> > +DRM subsystem's code and functionality. i.e. the accel core code will
> > +be part of the DRM subsystem and an accel device will be a new type of DRM
> > +device.
> > +
> > +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.
> > +
> > +Differentiation from GPUs
> > +=========================
> > +
> > +Because we want to prevent the extensive user-space graphic software stack
> > +from trying to use an accelerator as a GPU, the compute accelerators will be
> > +differentiated from GPUs by using a new major number and new device char files.
> > +
> > +Furthermore, the drivers will be located in a separate place in the kernel
> > +tree - drivers/accel/.
> > +
> > +The accelerator devices will be exposed to the user space with the dedicated
> > +261 major number and will have the following convention:
> > +
> > +- device char files - /dev/accel/accel*
> > +- sysfs - /sys/class/accel/accel*/
> > +- debugfs - /sys/kernel/debug/accel/accel*/
> > +
> > +Getting Started
> > +===============
> > +
> > +First, read the DRM documentation. Not only it will explain how to write a new
>
> How about a link to the DRM documentation?
>
> > +DRM driver but it will also contain all the information on how to contribute,
> > +the Code Of Conduct and what is the coding style/documentation. All of that
> > +is the same for the accel subsystem.
> > +
> > +Second, make sure the kernel is configured with CONFIG_DRM_ACCEL.
> > +
> > +To expose your device as an accelerator, two changes are needed to
> > +be done in your driver (as opposed to a standard DRM driver):
> > +
> > +- Add the DRIVER_COMPUTE_ACCEL feature flag in your drm_driver's
> > + driver_features field. It is important to note that this driver feature is
> > + mutually exclusive with DRIVER_RENDER and DRIVER_MODESET. Devices that want
>
> I don't remember seeing code that validates a driver with
> DRIVER_COMPUTE_ACCEL does not also have DRIVER_MODESET. What am I missing?
Look at drm_dev_init() (patch 3/4):
if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL) &&
(drm_core_check_feature(dev, DRIVER_RENDER) ||
drm_core_check_feature(dev, DRIVER_MODESET))) {
DRM_ERROR("DRM driver can't be both a compute acceleration
and graphics driver\n");
return -EINVAL;
}
Thanks for your other comments, I'll fix them before sending the PR.
Oded
>
> > + to expose both graphics and compute device char files should be handled by
> > + two drivers that are connected using the auxiliary bus framework.
> > +
> > +- Change the open callback in your driver fops structure to accel_open().
> > + Alternatively, your driver can use DEFINE_DRM_ACCEL_FOPS macro to easily
> > + set the correct function operations pointers structure.
> > +
> > +External References
> > +===================
> > +
> > +email threads
> > +-------------
> > +
> > +* `Initial discussion on the New subsystem for acceleration devices <https://lkml.org/lkml/2022/7/31/83>`_ - Oded Gabbay (2022)
> > +* `patch-set to add the new subsystem <https://lkml.org/lkml/2022/10/22/544>`_ - Oded Gabbay (2022)
> > +
> > +Conference talks
> > +----------------
> > +
> > +* `LPC 2022 Accelerators BOF outcomes summary <https://airlied.blogspot.com/2022/09/accelerators-bof-outcomes-summary.html>`_ - Dave Airlie (2022)
> > diff --git a/Documentation/subsystem-apis.rst b/Documentation/subsystem-apis.rst
> > index af65004a80aa..b51f38527e14 100644
> > --- a/Documentation/subsystem-apis.rst
> > +++ b/Documentation/subsystem-apis.rst
> > @@ -43,6 +43,7 @@ needed).
> > input/index
> > hwmon/index
> > gpu/index
> > + accel/index
> > security/index
> > sound/index
> > crypto/index
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 4d752aac3ec0..6ba7bb35208a 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -6837,6 +6837,7 @@ L: [email protected]
> > S: Maintained
> > C: irc://irc.oftc.net/dri-devel
> > T: git https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git
> > +F: Documentation/accel/
> > F: drivers/accel/
> >
> > DRM DRIVERS FOR ALLWINNER A10
>
On Sat, Nov 19, 2022 at 3:44 PM Oded Gabbay <[email protected]> wrote:
>
> This is the fourth (and hopefully last) version of the patch-set to add the
> new subsystem for compute accelerators. I removed the RFC headline as
> I believe it is now ready for merging.
>
> Compare to v3, this patch-set contains one additional patch that adds
> documentation regarding the accel subsystem. I hope it's good enough for
> this stage. In addition, there were few very minor fixes according to
> comments received on v3.
>
> The patches are in the following repo:
> https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4
>
> As in v3, The HEAD of that branch is a commit adding a dummy driver that
> registers an accel device using the new framework. This can be served
> as a simple reference.
>
> v1 cover letter:
> https://lkml.org/lkml/2022/10/22/544
>
> v2 cover letter:
> https://lore.kernel.org/lkml/[email protected]/T/
>
> v3 cover letter:
> https://lore.kernel.org/lkml/[email protected]/T/
>
With the understanding that individual drivers can choose to use
either classic drm or accel, whichever makes the most sense to them,
this series is:
Acked-by: Alex Deucher <[email protected]>
> Thanks,
> Oded.
>
> Oded Gabbay (4):
> drivers/accel: define kconfig and register a new major
> accel: add dedicated minor for accelerator devices
> drm: initialize accel framework
> doc: add documentation for accel subsystem
>
> Documentation/accel/index.rst | 17 ++
> Documentation/accel/introduction.rst | 109 +++++++++
> Documentation/admin-guide/devices.txt | 5 +
> Documentation/subsystem-apis.rst | 1 +
> MAINTAINERS | 9 +
> drivers/Kconfig | 2 +
> drivers/accel/Kconfig | 24 ++
> drivers/accel/drm_accel.c | 323 ++++++++++++++++++++++++++
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/drm_drv.c | 102 +++++---
> drivers/gpu/drm/drm_file.c | 2 +-
> drivers/gpu/drm/drm_sysfs.c | 24 +-
> include/drm/drm_accel.h | 97 ++++++++
> include/drm/drm_device.h | 3 +
> include/drm/drm_drv.h | 8 +
> include/drm/drm_file.h | 21 +-
> 16 files changed, 711 insertions(+), 37 deletions(-)
> create mode 100644 Documentation/accel/index.rst
> create mode 100644 Documentation/accel/introduction.rst
> create mode 100644 drivers/accel/Kconfig
> create mode 100644 drivers/accel/drm_accel.c
> create mode 100644 include/drm/drm_accel.h
>
> --
> 2.25.1
>
On Mon, Nov 21, 2022 at 10:57 AM Alex Deucher <[email protected]> wrote:
>
> On Sat, Nov 19, 2022 at 3:44 PM Oded Gabbay <[email protected]> wrote:
> >
> > This is the fourth (and hopefully last) version of the patch-set to add the
> > new subsystem for compute accelerators. I removed the RFC headline as
> > I believe it is now ready for merging.
> >
> > Compare to v3, this patch-set contains one additional patch that adds
> > documentation regarding the accel subsystem. I hope it's good enough for
> > this stage. In addition, there were few very minor fixes according to
> > comments received on v3.
> >
> > The patches are in the following repo:
> > https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4
> >
> > As in v3, The HEAD of that branch is a commit adding a dummy driver that
> > registers an accel device using the new framework. This can be served
> > as a simple reference.
> >
> > v1 cover letter:
> > https://lkml.org/lkml/2022/10/22/544
> >
> > v2 cover letter:
> > https://lore.kernel.org/lkml/[email protected]/T/
> >
> > v3 cover letter:
> > https://lore.kernel.org/lkml/[email protected]/T/
> >
>
> With the understanding that individual drivers can choose to use
> either classic drm or accel, whichever makes the most sense to them,
> this series is:
> Acked-by: Alex Deucher <[email protected]>
and not typo my email:
Acked-by: Alex Deucher <[email protected]>
>
> > Thanks,
> > Oded.
> >
> > Oded Gabbay (4):
> > drivers/accel: define kconfig and register a new major
> > accel: add dedicated minor for accelerator devices
> > drm: initialize accel framework
> > doc: add documentation for accel subsystem
> >
> > Documentation/accel/index.rst | 17 ++
> > Documentation/accel/introduction.rst | 109 +++++++++
> > Documentation/admin-guide/devices.txt | 5 +
> > Documentation/subsystem-apis.rst | 1 +
> > MAINTAINERS | 9 +
> > drivers/Kconfig | 2 +
> > drivers/accel/Kconfig | 24 ++
> > drivers/accel/drm_accel.c | 323 ++++++++++++++++++++++++++
> > drivers/gpu/drm/Makefile | 1 +
> > drivers/gpu/drm/drm_drv.c | 102 +++++---
> > drivers/gpu/drm/drm_file.c | 2 +-
> > drivers/gpu/drm/drm_sysfs.c | 24 +-
> > include/drm/drm_accel.h | 97 ++++++++
> > include/drm/drm_device.h | 3 +
> > include/drm/drm_drv.h | 8 +
> > include/drm/drm_file.h | 21 +-
> > 16 files changed, 711 insertions(+), 37 deletions(-)
> > create mode 100644 Documentation/accel/index.rst
> > create mode 100644 Documentation/accel/introduction.rst
> > create mode 100644 drivers/accel/Kconfig
> > create mode 100644 drivers/accel/drm_accel.c
> > create mode 100644 include/drm/drm_accel.h
> >
> > --
> > 2.25.1
> >
On 11/19/22 12:44, Oded Gabbay wrote:
> This is the fourth (and hopefully last) version of the patch-set to add the
> new subsystem for compute accelerators. I removed the RFC headline as
> I believe it is now ready for merging.
>
> Compare to v3, this patch-set contains one additional patch that adds
> documentation regarding the accel subsystem. I hope it's good enough for
> this stage. In addition, there were few very minor fixes according to
> comments received on v3.
>
> The patches are in the following repo:
> https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4
>
> As in v3, The HEAD of that branch is a commit adding a dummy driver that
> registers an accel device using the new framework. This can be served
> as a simple reference.
>
> v1 cover letter:
> https://lkml.org/lkml/2022/10/22/544
>
> v2 cover letter:
> https://lore.kernel.org/lkml/[email protected]/T/
>
> v3 cover letter:
> https://lore.kernel.org/lkml/[email protected]/T/
Thanks for defining the new accel subsystem. We are currently working on
DRM based drivers for unannounced acceleration devices. I am fine with
these changes with the assumption that the choice of using classic DRM
or accel is left up to the individual driver.
-Sonal
>
> Thanks,
> Oded.
>
> Oded Gabbay (4):
> drivers/accel: define kconfig and register a new major
> accel: add dedicated minor for accelerator devices
> drm: initialize accel framework
> doc: add documentation for accel subsystem
>
> Documentation/accel/index.rst | 17 ++
> Documentation/accel/introduction.rst | 109 +++++++++
> Documentation/admin-guide/devices.txt | 5 +
> Documentation/subsystem-apis.rst | 1 +
> MAINTAINERS | 9 +
> drivers/Kconfig | 2 +
> drivers/accel/Kconfig | 24 ++
> drivers/accel/drm_accel.c | 323 ++++++++++++++++++++++++++
> drivers/gpu/drm/Makefile | 1 +
> drivers/gpu/drm/drm_drv.c | 102 +++++---
> drivers/gpu/drm/drm_file.c | 2 +-
> drivers/gpu/drm/drm_sysfs.c | 24 +-
> include/drm/drm_accel.h | 97 ++++++++
> include/drm/drm_device.h | 3 +
> include/drm/drm_drv.h | 8 +
> include/drm/drm_file.h | 21 +-
> 16 files changed, 711 insertions(+), 37 deletions(-)
> create mode 100644 Documentation/accel/index.rst
> create mode 100644 Documentation/accel/introduction.rst
> create mode 100644 drivers/accel/Kconfig
> create mode 100644 drivers/accel/drm_accel.c
> create mode 100644 include/drm/drm_accel.h
>
> --
> 2.25.1
>
On Tue, 22 Nov 2022 at 09:06, Sonal Santan <[email protected]> wrote:
>
> On 11/19/22 12:44, Oded Gabbay wrote:
> > This is the fourth (and hopefully last) version of the patch-set to add the
> > new subsystem for compute accelerators. I removed the RFC headline as
> > I believe it is now ready for merging.
> >
> > Compare to v3, this patch-set contains one additional patch that adds
> > documentation regarding the accel subsystem. I hope it's good enough for
> > this stage. In addition, there were few very minor fixes according to
> > comments received on v3.
> >
> > The patches are in the following repo:
> > https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4
> >
> > As in v3, The HEAD of that branch is a commit adding a dummy driver that
> > registers an accel device using the new framework. This can be served
> > as a simple reference.
> >
> > v1 cover letter:
> > https://lkml.org/lkml/2022/10/22/544
> >
> > v2 cover letter:
> > https://lore.kernel.org/lkml/[email protected]/T/
> >
> > v3 cover letter:
> > https://lore.kernel.org/lkml/[email protected]/T/
>
> Thanks for defining the new accel subsystem. We are currently working on
> DRM based drivers for unannounced acceleration devices. I am fine with
> these changes with the assumption that the choice of using classic DRM
> or accel is left up to the individual driver.
I don't think that decision should be up to any individual driver
author. It will have to be consensus with me/Daniel/Oded and the
driver authors.
Dave.
Hi,
On 11/19/2022 9:44 PM, Oded Gabbay wrote:
> This is the fourth (and hopefully last) version of the patch-set to add the
> new subsystem for compute accelerators. I removed the RFC headline as
> I believe it is now ready for merging.
Looks good and works without issues.
I will rebase next version of Intel VPU patchest on top of this.
Acked-by: Jacek Lawrynowicz <[email protected]>
Tested-by: Jacek Lawrynowicz <[email protected]>
Regards,
Jacek
On Tue, Nov 22, 2022 at 12:56 PM Melissa Wen <[email protected]> wrote:
>
> On 11/19, Oded Gabbay wrote:
> > Now that we have the accel framework code ready, let's call the
> > accel functions from all the appropriate places. These places are the
> > drm module init/exit functions, and all the drm_minor handling
> > functions.
>
> Hi Oded,
>
> The proposal overall LGTM, but I didn't manage to compile the kernel
> with your patch series when DRM is enabled but accel support doesn't.
> Multiple missing link errors...
>
> Am I missing something? Can you take a look at it from this patch 3/4?
>
> Thanks,
>
> Melissa
Looking at it now, thanks for letting me know.
Oded
> >
> > Signed-off-by: Oded Gabbay <[email protected]>
> > ---
> > drivers/gpu/drm/drm_drv.c | 102 ++++++++++++++++++++++++++----------
> > drivers/gpu/drm/drm_sysfs.c | 24 ++++++---
> > 2 files changed, 91 insertions(+), 35 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 8214a0b1ab7f..1aec019f6389 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -35,6 +35,7 @@
> > #include <linux/slab.h>
> > #include <linux/srcu.h>
> >
> > +#include <drm/drm_accel.h>
> > #include <drm/drm_cache.h>
> > #include <drm/drm_client.h>
> > #include <drm/drm_color_mgmt.h>
> > @@ -90,6 +91,8 @@ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev,
> > return &dev->primary;
> > case DRM_MINOR_RENDER:
> > return &dev->render;
> > + case DRM_MINOR_ACCEL:
> > + return &dev->accel;
> > default:
> > BUG();
> > }
> > @@ -104,9 +107,13 @@ static void drm_minor_alloc_release(struct drm_device *dev, void *data)
> >
> > put_device(minor->kdev);
> >
> > - spin_lock_irqsave(&drm_minor_lock, flags);
> > - idr_remove(&drm_minors_idr, minor->index);
> > - spin_unlock_irqrestore(&drm_minor_lock, flags);
> > + if (minor->type == DRM_MINOR_ACCEL) {
> > + accel_minor_remove(minor->index);
> > + } else {
> > + spin_lock_irqsave(&drm_minor_lock, flags);
> > + idr_remove(&drm_minors_idr, minor->index);
> > + spin_unlock_irqrestore(&drm_minor_lock, flags);
> > + }
> > }
> >
> > static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
> > @@ -123,13 +130,17 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
> > minor->dev = dev;
> >
> > idr_preload(GFP_KERNEL);
> > - spin_lock_irqsave(&drm_minor_lock, flags);
> > - r = idr_alloc(&drm_minors_idr,
> > - NULL,
> > - 64 * type,
> > - 64 * (type + 1),
> > - GFP_NOWAIT);
> > - spin_unlock_irqrestore(&drm_minor_lock, flags);
> > + if (type == DRM_MINOR_ACCEL) {
> > + r = accel_minor_alloc();
> > + } else {
> > + spin_lock_irqsave(&drm_minor_lock, flags);
> > + r = idr_alloc(&drm_minors_idr,
> > + NULL,
> > + 64 * type,
> > + 64 * (type + 1),
> > + GFP_NOWAIT);
> > + spin_unlock_irqrestore(&drm_minor_lock, flags);
> > + }
> > idr_preload_end();
> >
> > if (r < 0)
> > @@ -161,10 +172,14 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
> > if (!minor)
> > return 0;
> >
> > - ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root);
> > - if (ret) {
> > - DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
> > - goto err_debugfs;
> > + if (minor->type == DRM_MINOR_ACCEL) {
> > + accel_debugfs_init(minor, minor->index);
> > + } else {
> > + ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root);
> > + if (ret) {
> > + DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
> > + goto err_debugfs;
> > + }
> > }
> >
> > ret = device_add(minor->kdev);
> > @@ -172,9 +187,13 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
> > goto err_debugfs;
> >
> > /* replace NULL with @minor so lookups will succeed from now on */
> > - spin_lock_irqsave(&drm_minor_lock, flags);
> > - idr_replace(&drm_minors_idr, minor, minor->index);
> > - spin_unlock_irqrestore(&drm_minor_lock, flags);
> > + if (minor->type == DRM_MINOR_ACCEL) {
> > + accel_minor_replace(minor, minor->index);
> > + } else {
> > + spin_lock_irqsave(&drm_minor_lock, flags);
> > + idr_replace(&drm_minors_idr, minor, minor->index);
> > + spin_unlock_irqrestore(&drm_minor_lock, flags);
> > + }
> >
> > DRM_DEBUG("new minor registered %d\n", minor->index);
> > return 0;
> > @@ -194,9 +213,13 @@ static void drm_minor_unregister(struct drm_device *dev, unsigned int type)
> > return;
> >
> > /* replace @minor with NULL so lookups will fail from now on */
> > - spin_lock_irqsave(&drm_minor_lock, flags);
> > - idr_replace(&drm_minors_idr, NULL, minor->index);
> > - spin_unlock_irqrestore(&drm_minor_lock, flags);
> > + if (minor->type == DRM_MINOR_ACCEL) {
> > + accel_minor_replace(NULL, minor->index);
> > + } else {
> > + spin_lock_irqsave(&drm_minor_lock, flags);
> > + idr_replace(&drm_minors_idr, NULL, minor->index);
> > + spin_unlock_irqrestore(&drm_minor_lock, flags);
> > + }
> >
> > device_del(minor->kdev);
> > dev_set_drvdata(minor->kdev, NULL); /* safety belt */
> > @@ -603,6 +626,14 @@ static int drm_dev_init(struct drm_device *dev,
> > /* no per-device feature limits by default */
> > dev->driver_features = ~0u;
> >
> > + if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL) &&
> > + (drm_core_check_feature(dev, DRIVER_RENDER) ||
> > + drm_core_check_feature(dev, DRIVER_MODESET))) {
> > +
> > + DRM_ERROR("DRM driver can't be both a compute acceleration and graphics driver\n");
> > + return -EINVAL;
> > + }
> > +
> > drm_legacy_init_members(dev);
> > INIT_LIST_HEAD(&dev->filelist);
> > INIT_LIST_HEAD(&dev->filelist_internal);
> > @@ -628,15 +659,21 @@ static int drm_dev_init(struct drm_device *dev,
> >
> > dev->anon_inode = inode;
> >
> > - if (drm_core_check_feature(dev, DRIVER_RENDER)) {
> > - ret = drm_minor_alloc(dev, DRM_MINOR_RENDER);
> > + if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL)) {
> > + ret = drm_minor_alloc(dev, DRM_MINOR_ACCEL);
> > if (ret)
> > goto err;
> > - }
> > + } else {
> > + if (drm_core_check_feature(dev, DRIVER_RENDER)) {
> > + ret = drm_minor_alloc(dev, DRM_MINOR_RENDER);
> > + if (ret)
> > + goto err;
> > + }
> >
> > - ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
> > - if (ret)
> > - goto err;
> > + ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
> > + if (ret)
> > + goto err;
> > + }
> >
> > ret = drm_legacy_create_map_hash(dev);
> > if (ret)
> > @@ -883,6 +920,10 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> > if (ret)
> > goto err_minors;
> >
> > + ret = drm_minor_register(dev, DRM_MINOR_ACCEL);
> > + if (ret)
> > + goto err_minors;
> > +
> > ret = create_compat_control_link(dev);
> > if (ret)
> > goto err_minors;
> > @@ -902,12 +943,13 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> > driver->name, driver->major, driver->minor,
> > driver->patchlevel, driver->date,
> > dev->dev ? dev_name(dev->dev) : "virtual device",
> > - dev->primary->index);
> > + dev->primary ? dev->primary->index : dev->accel->index);
> >
> > goto out_unlock;
> >
> > err_minors:
> > remove_compat_control_link(dev);
> > + drm_minor_unregister(dev, DRM_MINOR_ACCEL);
> > drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
> > drm_minor_unregister(dev, DRM_MINOR_RENDER);
> > out_unlock:
> > @@ -950,6 +992,7 @@ void drm_dev_unregister(struct drm_device *dev)
> > drm_legacy_rmmaps(dev);
> >
> > remove_compat_control_link(dev);
> > + drm_minor_unregister(dev, DRM_MINOR_ACCEL);
> > drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
> > drm_minor_unregister(dev, DRM_MINOR_RENDER);
> > }
> > @@ -1034,6 +1077,7 @@ static const struct file_operations drm_stub_fops = {
> > static void drm_core_exit(void)
> > {
> > drm_privacy_screen_lookup_exit();
> > + accel_core_exit();
> > unregister_chrdev(DRM_MAJOR, "drm");
> > debugfs_remove(drm_debugfs_root);
> > drm_sysfs_destroy();
> > @@ -1061,6 +1105,10 @@ static int __init drm_core_init(void)
> > if (ret < 0)
> > goto error;
> >
> > + ret = accel_core_init();
> > + if (ret < 0)
> > + goto error;
> > +
> > drm_privacy_screen_lookup_init();
> >
> > drm_core_init_complete = true;
> > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> > index 430e00b16eec..b8da978d85bb 100644
> > --- a/drivers/gpu/drm/drm_sysfs.c
> > +++ b/drivers/gpu/drm/drm_sysfs.c
> > @@ -19,6 +19,7 @@
> > #include <linux/kdev_t.h>
> > #include <linux/slab.h>
> >
> > +#include <drm/drm_accel.h>
> > #include <drm/drm_connector.h>
> > #include <drm/drm_device.h>
> > #include <drm/drm_file.h>
> > @@ -471,19 +472,26 @@ struct device *drm_sysfs_minor_alloc(struct drm_minor *minor)
> > struct device *kdev;
> > int r;
> >
> > - if (minor->type == DRM_MINOR_RENDER)
> > - minor_str = "renderD%d";
> > - else
> > - minor_str = "card%d";
> > -
> > kdev = kzalloc(sizeof(*kdev), GFP_KERNEL);
> > if (!kdev)
> > return ERR_PTR(-ENOMEM);
> >
> > device_initialize(kdev);
> > - kdev->devt = MKDEV(DRM_MAJOR, minor->index);
> > - kdev->class = drm_class;
> > - kdev->type = &drm_sysfs_device_minor;
> > +
> > + if (minor->type == DRM_MINOR_ACCEL) {
> > + minor_str = "accel%d";
> > + accel_set_device_instance_params(kdev, minor->index);
> > + } else {
> > + if (minor->type == DRM_MINOR_RENDER)
> > + minor_str = "renderD%d";
> > + else
> > + minor_str = "card%d";
> > +
> > + kdev->devt = MKDEV(DRM_MAJOR, minor->index);
> > + kdev->class = drm_class;
> > + kdev->type = &drm_sysfs_device_minor;
> > + }
> > +
> > kdev->parent = minor->dev->dev;
> > kdev->release = drm_sysfs_release;
> > dev_set_drvdata(kdev, minor);
> > --
> > 2.25.1
> >
On 11/19, Oded Gabbay wrote:
> Now that we have the accel framework code ready, let's call the
> accel functions from all the appropriate places. These places are the
> drm module init/exit functions, and all the drm_minor handling
> functions.
Hi Oded,
The proposal overall LGTM, but I didn't manage to compile the kernel
with your patch series when DRM is enabled but accel support doesn't.
Multiple missing link errors...
Am I missing something? Can you take a look at it from this patch 3/4?
Thanks,
Melissa
>
> Signed-off-by: Oded Gabbay <[email protected]>
> ---
> drivers/gpu/drm/drm_drv.c | 102 ++++++++++++++++++++++++++----------
> drivers/gpu/drm/drm_sysfs.c | 24 ++++++---
> 2 files changed, 91 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 8214a0b1ab7f..1aec019f6389 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -35,6 +35,7 @@
> #include <linux/slab.h>
> #include <linux/srcu.h>
>
> +#include <drm/drm_accel.h>
> #include <drm/drm_cache.h>
> #include <drm/drm_client.h>
> #include <drm/drm_color_mgmt.h>
> @@ -90,6 +91,8 @@ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev,
> return &dev->primary;
> case DRM_MINOR_RENDER:
> return &dev->render;
> + case DRM_MINOR_ACCEL:
> + return &dev->accel;
> default:
> BUG();
> }
> @@ -104,9 +107,13 @@ static void drm_minor_alloc_release(struct drm_device *dev, void *data)
>
> put_device(minor->kdev);
>
> - spin_lock_irqsave(&drm_minor_lock, flags);
> - idr_remove(&drm_minors_idr, minor->index);
> - spin_unlock_irqrestore(&drm_minor_lock, flags);
> + if (minor->type == DRM_MINOR_ACCEL) {
> + accel_minor_remove(minor->index);
> + } else {
> + spin_lock_irqsave(&drm_minor_lock, flags);
> + idr_remove(&drm_minors_idr, minor->index);
> + spin_unlock_irqrestore(&drm_minor_lock, flags);
> + }
> }
>
> static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
> @@ -123,13 +130,17 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
> minor->dev = dev;
>
> idr_preload(GFP_KERNEL);
> - spin_lock_irqsave(&drm_minor_lock, flags);
> - r = idr_alloc(&drm_minors_idr,
> - NULL,
> - 64 * type,
> - 64 * (type + 1),
> - GFP_NOWAIT);
> - spin_unlock_irqrestore(&drm_minor_lock, flags);
> + if (type == DRM_MINOR_ACCEL) {
> + r = accel_minor_alloc();
> + } else {
> + spin_lock_irqsave(&drm_minor_lock, flags);
> + r = idr_alloc(&drm_minors_idr,
> + NULL,
> + 64 * type,
> + 64 * (type + 1),
> + GFP_NOWAIT);
> + spin_unlock_irqrestore(&drm_minor_lock, flags);
> + }
> idr_preload_end();
>
> if (r < 0)
> @@ -161,10 +172,14 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
> if (!minor)
> return 0;
>
> - ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root);
> - if (ret) {
> - DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
> - goto err_debugfs;
> + if (minor->type == DRM_MINOR_ACCEL) {
> + accel_debugfs_init(minor, minor->index);
> + } else {
> + ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root);
> + if (ret) {
> + DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
> + goto err_debugfs;
> + }
> }
>
> ret = device_add(minor->kdev);
> @@ -172,9 +187,13 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
> goto err_debugfs;
>
> /* replace NULL with @minor so lookups will succeed from now on */
> - spin_lock_irqsave(&drm_minor_lock, flags);
> - idr_replace(&drm_minors_idr, minor, minor->index);
> - spin_unlock_irqrestore(&drm_minor_lock, flags);
> + if (minor->type == DRM_MINOR_ACCEL) {
> + accel_minor_replace(minor, minor->index);
> + } else {
> + spin_lock_irqsave(&drm_minor_lock, flags);
> + idr_replace(&drm_minors_idr, minor, minor->index);
> + spin_unlock_irqrestore(&drm_minor_lock, flags);
> + }
>
> DRM_DEBUG("new minor registered %d\n", minor->index);
> return 0;
> @@ -194,9 +213,13 @@ static void drm_minor_unregister(struct drm_device *dev, unsigned int type)
> return;
>
> /* replace @minor with NULL so lookups will fail from now on */
> - spin_lock_irqsave(&drm_minor_lock, flags);
> - idr_replace(&drm_minors_idr, NULL, minor->index);
> - spin_unlock_irqrestore(&drm_minor_lock, flags);
> + if (minor->type == DRM_MINOR_ACCEL) {
> + accel_minor_replace(NULL, minor->index);
> + } else {
> + spin_lock_irqsave(&drm_minor_lock, flags);
> + idr_replace(&drm_minors_idr, NULL, minor->index);
> + spin_unlock_irqrestore(&drm_minor_lock, flags);
> + }
>
> device_del(minor->kdev);
> dev_set_drvdata(minor->kdev, NULL); /* safety belt */
> @@ -603,6 +626,14 @@ static int drm_dev_init(struct drm_device *dev,
> /* no per-device feature limits by default */
> dev->driver_features = ~0u;
>
> + if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL) &&
> + (drm_core_check_feature(dev, DRIVER_RENDER) ||
> + drm_core_check_feature(dev, DRIVER_MODESET))) {
> +
> + DRM_ERROR("DRM driver can't be both a compute acceleration and graphics driver\n");
> + return -EINVAL;
> + }
> +
> drm_legacy_init_members(dev);
> INIT_LIST_HEAD(&dev->filelist);
> INIT_LIST_HEAD(&dev->filelist_internal);
> @@ -628,15 +659,21 @@ static int drm_dev_init(struct drm_device *dev,
>
> dev->anon_inode = inode;
>
> - if (drm_core_check_feature(dev, DRIVER_RENDER)) {
> - ret = drm_minor_alloc(dev, DRM_MINOR_RENDER);
> + if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL)) {
> + ret = drm_minor_alloc(dev, DRM_MINOR_ACCEL);
> if (ret)
> goto err;
> - }
> + } else {
> + if (drm_core_check_feature(dev, DRIVER_RENDER)) {
> + ret = drm_minor_alloc(dev, DRM_MINOR_RENDER);
> + if (ret)
> + goto err;
> + }
>
> - ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
> - if (ret)
> - goto err;
> + ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
> + if (ret)
> + goto err;
> + }
>
> ret = drm_legacy_create_map_hash(dev);
> if (ret)
> @@ -883,6 +920,10 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> if (ret)
> goto err_minors;
>
> + ret = drm_minor_register(dev, DRM_MINOR_ACCEL);
> + if (ret)
> + goto err_minors;
> +
> ret = create_compat_control_link(dev);
> if (ret)
> goto err_minors;
> @@ -902,12 +943,13 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> driver->name, driver->major, driver->minor,
> driver->patchlevel, driver->date,
> dev->dev ? dev_name(dev->dev) : "virtual device",
> - dev->primary->index);
> + dev->primary ? dev->primary->index : dev->accel->index);
>
> goto out_unlock;
>
> err_minors:
> remove_compat_control_link(dev);
> + drm_minor_unregister(dev, DRM_MINOR_ACCEL);
> drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
> drm_minor_unregister(dev, DRM_MINOR_RENDER);
> out_unlock:
> @@ -950,6 +992,7 @@ void drm_dev_unregister(struct drm_device *dev)
> drm_legacy_rmmaps(dev);
>
> remove_compat_control_link(dev);
> + drm_minor_unregister(dev, DRM_MINOR_ACCEL);
> drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
> drm_minor_unregister(dev, DRM_MINOR_RENDER);
> }
> @@ -1034,6 +1077,7 @@ static const struct file_operations drm_stub_fops = {
> static void drm_core_exit(void)
> {
> drm_privacy_screen_lookup_exit();
> + accel_core_exit();
> unregister_chrdev(DRM_MAJOR, "drm");
> debugfs_remove(drm_debugfs_root);
> drm_sysfs_destroy();
> @@ -1061,6 +1105,10 @@ static int __init drm_core_init(void)
> if (ret < 0)
> goto error;
>
> + ret = accel_core_init();
> + if (ret < 0)
> + goto error;
> +
> drm_privacy_screen_lookup_init();
>
> drm_core_init_complete = true;
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index 430e00b16eec..b8da978d85bb 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -19,6 +19,7 @@
> #include <linux/kdev_t.h>
> #include <linux/slab.h>
>
> +#include <drm/drm_accel.h>
> #include <drm/drm_connector.h>
> #include <drm/drm_device.h>
> #include <drm/drm_file.h>
> @@ -471,19 +472,26 @@ struct device *drm_sysfs_minor_alloc(struct drm_minor *minor)
> struct device *kdev;
> int r;
>
> - if (minor->type == DRM_MINOR_RENDER)
> - minor_str = "renderD%d";
> - else
> - minor_str = "card%d";
> -
> kdev = kzalloc(sizeof(*kdev), GFP_KERNEL);
> if (!kdev)
> return ERR_PTR(-ENOMEM);
>
> device_initialize(kdev);
> - kdev->devt = MKDEV(DRM_MAJOR, minor->index);
> - kdev->class = drm_class;
> - kdev->type = &drm_sysfs_device_minor;
> +
> + if (minor->type == DRM_MINOR_ACCEL) {
> + minor_str = "accel%d";
> + accel_set_device_instance_params(kdev, minor->index);
> + } else {
> + if (minor->type == DRM_MINOR_RENDER)
> + minor_str = "renderD%d";
> + else
> + minor_str = "card%d";
> +
> + kdev->devt = MKDEV(DRM_MAJOR, minor->index);
> + kdev->class = drm_class;
> + kdev->type = &drm_sysfs_device_minor;
> + }
> +
> kdev->parent = minor->dev->dev;
> kdev->release = drm_sysfs_release;
> dev_set_drvdata(kdev, minor);
> --
> 2.25.1
>
On Tue, Nov 22, 2022 at 12:59 PM Oded Gabbay <[email protected]> wrote:
>
> On Tue, Nov 22, 2022 at 12:56 PM Melissa Wen <[email protected]> wrote:
> >
> > On 11/19, Oded Gabbay wrote:
> > > Now that we have the accel framework code ready, let's call the
> > > accel functions from all the appropriate places. These places are the
> > > drm module init/exit functions, and all the drm_minor handling
> > > functions.
> >
> > Hi Oded,
> >
> > The proposal overall LGTM, but I didn't manage to compile the kernel
> > with your patch series when DRM is enabled but accel support doesn't.
> > Multiple missing link errors...
> >
> > Am I missing something? Can you take a look at it from this patch 3/4?
> >
> > Thanks,
> >
> > Melissa
> Looking at it now, thanks for letting me know.
> Oded
Found the issue, missing } at end of accel_debugfs_init() in
drm_accel.h. Only compiles when accel support isn't compiled in.
I'll fix it before sending the PR to Dave.
Much appreciated :)
Oded
>
> > >
> > > Signed-off-by: Oded Gabbay <[email protected]>
> > > ---
> > > drivers/gpu/drm/drm_drv.c | 102 ++++++++++++++++++++++++++----------
> > > drivers/gpu/drm/drm_sysfs.c | 24 ++++++---
> > > 2 files changed, 91 insertions(+), 35 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > > index 8214a0b1ab7f..1aec019f6389 100644
> > > --- a/drivers/gpu/drm/drm_drv.c
> > > +++ b/drivers/gpu/drm/drm_drv.c
> > > @@ -35,6 +35,7 @@
> > > #include <linux/slab.h>
> > > #include <linux/srcu.h>
> > >
> > > +#include <drm/drm_accel.h>
> > > #include <drm/drm_cache.h>
> > > #include <drm/drm_client.h>
> > > #include <drm/drm_color_mgmt.h>
> > > @@ -90,6 +91,8 @@ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev,
> > > return &dev->primary;
> > > case DRM_MINOR_RENDER:
> > > return &dev->render;
> > > + case DRM_MINOR_ACCEL:
> > > + return &dev->accel;
> > > default:
> > > BUG();
> > > }
> > > @@ -104,9 +107,13 @@ static void drm_minor_alloc_release(struct drm_device *dev, void *data)
> > >
> > > put_device(minor->kdev);
> > >
> > > - spin_lock_irqsave(&drm_minor_lock, flags);
> > > - idr_remove(&drm_minors_idr, minor->index);
> > > - spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > + if (minor->type == DRM_MINOR_ACCEL) {
> > > + accel_minor_remove(minor->index);
> > > + } else {
> > > + spin_lock_irqsave(&drm_minor_lock, flags);
> > > + idr_remove(&drm_minors_idr, minor->index);
> > > + spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > + }
> > > }
> > >
> > > static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
> > > @@ -123,13 +130,17 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
> > > minor->dev = dev;
> > >
> > > idr_preload(GFP_KERNEL);
> > > - spin_lock_irqsave(&drm_minor_lock, flags);
> > > - r = idr_alloc(&drm_minors_idr,
> > > - NULL,
> > > - 64 * type,
> > > - 64 * (type + 1),
> > > - GFP_NOWAIT);
> > > - spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > + if (type == DRM_MINOR_ACCEL) {
> > > + r = accel_minor_alloc();
> > > + } else {
> > > + spin_lock_irqsave(&drm_minor_lock, flags);
> > > + r = idr_alloc(&drm_minors_idr,
> > > + NULL,
> > > + 64 * type,
> > > + 64 * (type + 1),
> > > + GFP_NOWAIT);
> > > + spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > + }
> > > idr_preload_end();
> > >
> > > if (r < 0)
> > > @@ -161,10 +172,14 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
> > > if (!minor)
> > > return 0;
> > >
> > > - ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root);
> > > - if (ret) {
> > > - DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
> > > - goto err_debugfs;
> > > + if (minor->type == DRM_MINOR_ACCEL) {
> > > + accel_debugfs_init(minor, minor->index);
> > > + } else {
> > > + ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root);
> > > + if (ret) {
> > > + DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
> > > + goto err_debugfs;
> > > + }
> > > }
> > >
> > > ret = device_add(minor->kdev);
> > > @@ -172,9 +187,13 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
> > > goto err_debugfs;
> > >
> > > /* replace NULL with @minor so lookups will succeed from now on */
> > > - spin_lock_irqsave(&drm_minor_lock, flags);
> > > - idr_replace(&drm_minors_idr, minor, minor->index);
> > > - spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > + if (minor->type == DRM_MINOR_ACCEL) {
> > > + accel_minor_replace(minor, minor->index);
> > > + } else {
> > > + spin_lock_irqsave(&drm_minor_lock, flags);
> > > + idr_replace(&drm_minors_idr, minor, minor->index);
> > > + spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > + }
> > >
> > > DRM_DEBUG("new minor registered %d\n", minor->index);
> > > return 0;
> > > @@ -194,9 +213,13 @@ static void drm_minor_unregister(struct drm_device *dev, unsigned int type)
> > > return;
> > >
> > > /* replace @minor with NULL so lookups will fail from now on */
> > > - spin_lock_irqsave(&drm_minor_lock, flags);
> > > - idr_replace(&drm_minors_idr, NULL, minor->index);
> > > - spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > + if (minor->type == DRM_MINOR_ACCEL) {
> > > + accel_minor_replace(NULL, minor->index);
> > > + } else {
> > > + spin_lock_irqsave(&drm_minor_lock, flags);
> > > + idr_replace(&drm_minors_idr, NULL, minor->index);
> > > + spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > + }
> > >
> > > device_del(minor->kdev);
> > > dev_set_drvdata(minor->kdev, NULL); /* safety belt */
> > > @@ -603,6 +626,14 @@ static int drm_dev_init(struct drm_device *dev,
> > > /* no per-device feature limits by default */
> > > dev->driver_features = ~0u;
> > >
> > > + if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL) &&
> > > + (drm_core_check_feature(dev, DRIVER_RENDER) ||
> > > + drm_core_check_feature(dev, DRIVER_MODESET))) {
> > > +
> > > + DRM_ERROR("DRM driver can't be both a compute acceleration and graphics driver\n");
> > > + return -EINVAL;
> > > + }
> > > +
> > > drm_legacy_init_members(dev);
> > > INIT_LIST_HEAD(&dev->filelist);
> > > INIT_LIST_HEAD(&dev->filelist_internal);
> > > @@ -628,15 +659,21 @@ static int drm_dev_init(struct drm_device *dev,
> > >
> > > dev->anon_inode = inode;
> > >
> > > - if (drm_core_check_feature(dev, DRIVER_RENDER)) {
> > > - ret = drm_minor_alloc(dev, DRM_MINOR_RENDER);
> > > + if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL)) {
> > > + ret = drm_minor_alloc(dev, DRM_MINOR_ACCEL);
> > > if (ret)
> > > goto err;
> > > - }
> > > + } else {
> > > + if (drm_core_check_feature(dev, DRIVER_RENDER)) {
> > > + ret = drm_minor_alloc(dev, DRM_MINOR_RENDER);
> > > + if (ret)
> > > + goto err;
> > > + }
> > >
> > > - ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
> > > - if (ret)
> > > - goto err;
> > > + ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
> > > + if (ret)
> > > + goto err;
> > > + }
> > >
> > > ret = drm_legacy_create_map_hash(dev);
> > > if (ret)
> > > @@ -883,6 +920,10 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> > > if (ret)
> > > goto err_minors;
> > >
> > > + ret = drm_minor_register(dev, DRM_MINOR_ACCEL);
> > > + if (ret)
> > > + goto err_minors;
> > > +
> > > ret = create_compat_control_link(dev);
> > > if (ret)
> > > goto err_minors;
> > > @@ -902,12 +943,13 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> > > driver->name, driver->major, driver->minor,
> > > driver->patchlevel, driver->date,
> > > dev->dev ? dev_name(dev->dev) : "virtual device",
> > > - dev->primary->index);
> > > + dev->primary ? dev->primary->index : dev->accel->index);
> > >
> > > goto out_unlock;
> > >
> > > err_minors:
> > > remove_compat_control_link(dev);
> > > + drm_minor_unregister(dev, DRM_MINOR_ACCEL);
> > > drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
> > > drm_minor_unregister(dev, DRM_MINOR_RENDER);
> > > out_unlock:
> > > @@ -950,6 +992,7 @@ void drm_dev_unregister(struct drm_device *dev)
> > > drm_legacy_rmmaps(dev);
> > >
> > > remove_compat_control_link(dev);
> > > + drm_minor_unregister(dev, DRM_MINOR_ACCEL);
> > > drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
> > > drm_minor_unregister(dev, DRM_MINOR_RENDER);
> > > }
> > > @@ -1034,6 +1077,7 @@ static const struct file_operations drm_stub_fops = {
> > > static void drm_core_exit(void)
> > > {
> > > drm_privacy_screen_lookup_exit();
> > > + accel_core_exit();
> > > unregister_chrdev(DRM_MAJOR, "drm");
> > > debugfs_remove(drm_debugfs_root);
> > > drm_sysfs_destroy();
> > > @@ -1061,6 +1105,10 @@ static int __init drm_core_init(void)
> > > if (ret < 0)
> > > goto error;
> > >
> > > + ret = accel_core_init();
> > > + if (ret < 0)
> > > + goto error;
> > > +
> > > drm_privacy_screen_lookup_init();
> > >
> > > drm_core_init_complete = true;
> > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> > > index 430e00b16eec..b8da978d85bb 100644
> > > --- a/drivers/gpu/drm/drm_sysfs.c
> > > +++ b/drivers/gpu/drm/drm_sysfs.c
> > > @@ -19,6 +19,7 @@
> > > #include <linux/kdev_t.h>
> > > #include <linux/slab.h>
> > >
> > > +#include <drm/drm_accel.h>
> > > #include <drm/drm_connector.h>
> > > #include <drm/drm_device.h>
> > > #include <drm/drm_file.h>
> > > @@ -471,19 +472,26 @@ struct device *drm_sysfs_minor_alloc(struct drm_minor *minor)
> > > struct device *kdev;
> > > int r;
> > >
> > > - if (minor->type == DRM_MINOR_RENDER)
> > > - minor_str = "renderD%d";
> > > - else
> > > - minor_str = "card%d";
> > > -
> > > kdev = kzalloc(sizeof(*kdev), GFP_KERNEL);
> > > if (!kdev)
> > > return ERR_PTR(-ENOMEM);
> > >
> > > device_initialize(kdev);
> > > - kdev->devt = MKDEV(DRM_MAJOR, minor->index);
> > > - kdev->class = drm_class;
> > > - kdev->type = &drm_sysfs_device_minor;
> > > +
> > > + if (minor->type == DRM_MINOR_ACCEL) {
> > > + minor_str = "accel%d";
> > > + accel_set_device_instance_params(kdev, minor->index);
> > > + } else {
> > > + if (minor->type == DRM_MINOR_RENDER)
> > > + minor_str = "renderD%d";
> > > + else
> > > + minor_str = "card%d";
> > > +
> > > + kdev->devt = MKDEV(DRM_MAJOR, minor->index);
> > > + kdev->class = drm_class;
> > > + kdev->type = &drm_sysfs_device_minor;
> > > + }
> > > +
> > > kdev->parent = minor->dev->dev;
> > > kdev->release = drm_sysfs_release;
> > > dev_set_drvdata(kdev, minor);
> > > --
> > > 2.25.1
> > >
11/22, Oded Gabbay wrote:
> On Tue, Nov 22, 2022 at 12:59 PM Oded Gabbay <[email protected]> wrote:
> >
> > On Tue, Nov 22, 2022 at 12:56 PM Melissa Wen <[email protected]> wrote:
> > >
> > > On 11/19, Oded Gabbay wrote:
> > > > Now that we have the accel framework code ready, let's call the
> > > > accel functions from all the appropriate places. These places are the
> > > > drm module init/exit functions, and all the drm_minor handling
> > > > functions.
> > >
> > > Hi Oded,
> > >
> > > The proposal overall LGTM, but I didn't manage to compile the kernel
> > > with your patch series when DRM is enabled but accel support doesn't.
> > > Multiple missing link errors...
> > >
> > > Am I missing something? Can you take a look at it from this patch 3/4?
> > >
> > > Thanks,
> > >
> > > Melissa
> > Looking at it now, thanks for letting me know.
> > Oded
> Found the issue, missing } at end of accel_debugfs_init() in
> drm_accel.h. Only compiles when accel support isn't compiled in.
> I'll fix it before sending the PR to Dave.
> Much appreciated :)
> Oded
Oh, great! Just checked here and everything is fine now.
With that, you can add my r-b for the entire series too
Reviewed-by: Melissa Wen <[email protected]>
Thanks for working on it,
Melissa
>
> >
> > > >
> > > > Signed-off-by: Oded Gabbay <[email protected]>
> > > > ---
> > > > drivers/gpu/drm/drm_drv.c | 102 ++++++++++++++++++++++++++----------
> > > > drivers/gpu/drm/drm_sysfs.c | 24 ++++++---
> > > > 2 files changed, 91 insertions(+), 35 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > > > index 8214a0b1ab7f..1aec019f6389 100644
> > > > --- a/drivers/gpu/drm/drm_drv.c
> > > > +++ b/drivers/gpu/drm/drm_drv.c
> > > > @@ -35,6 +35,7 @@
> > > > #include <linux/slab.h>
> > > > #include <linux/srcu.h>
> > > >
> > > > +#include <drm/drm_accel.h>
> > > > #include <drm/drm_cache.h>
> > > > #include <drm/drm_client.h>
> > > > #include <drm/drm_color_mgmt.h>
> > > > @@ -90,6 +91,8 @@ static struct drm_minor **drm_minor_get_slot(struct drm_device *dev,
> > > > return &dev->primary;
> > > > case DRM_MINOR_RENDER:
> > > > return &dev->render;
> > > > + case DRM_MINOR_ACCEL:
> > > > + return &dev->accel;
> > > > default:
> > > > BUG();
> > > > }
> > > > @@ -104,9 +107,13 @@ static void drm_minor_alloc_release(struct drm_device *dev, void *data)
> > > >
> > > > put_device(minor->kdev);
> > > >
> > > > - spin_lock_irqsave(&drm_minor_lock, flags);
> > > > - idr_remove(&drm_minors_idr, minor->index);
> > > > - spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > > + if (minor->type == DRM_MINOR_ACCEL) {
> > > > + accel_minor_remove(minor->index);
> > > > + } else {
> > > > + spin_lock_irqsave(&drm_minor_lock, flags);
> > > > + idr_remove(&drm_minors_idr, minor->index);
> > > > + spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > > + }
> > > > }
> > > >
> > > > static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
> > > > @@ -123,13 +130,17 @@ static int drm_minor_alloc(struct drm_device *dev, unsigned int type)
> > > > minor->dev = dev;
> > > >
> > > > idr_preload(GFP_KERNEL);
> > > > - spin_lock_irqsave(&drm_minor_lock, flags);
> > > > - r = idr_alloc(&drm_minors_idr,
> > > > - NULL,
> > > > - 64 * type,
> > > > - 64 * (type + 1),
> > > > - GFP_NOWAIT);
> > > > - spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > > + if (type == DRM_MINOR_ACCEL) {
> > > > + r = accel_minor_alloc();
> > > > + } else {
> > > > + spin_lock_irqsave(&drm_minor_lock, flags);
> > > > + r = idr_alloc(&drm_minors_idr,
> > > > + NULL,
> > > > + 64 * type,
> > > > + 64 * (type + 1),
> > > > + GFP_NOWAIT);
> > > > + spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > > + }
> > > > idr_preload_end();
> > > >
> > > > if (r < 0)
> > > > @@ -161,10 +172,14 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
> > > > if (!minor)
> > > > return 0;
> > > >
> > > > - ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root);
> > > > - if (ret) {
> > > > - DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
> > > > - goto err_debugfs;
> > > > + if (minor->type == DRM_MINOR_ACCEL) {
> > > > + accel_debugfs_init(minor, minor->index);
> > > > + } else {
> > > > + ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root);
> > > > + if (ret) {
> > > > + DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
> > > > + goto err_debugfs;
> > > > + }
> > > > }
> > > >
> > > > ret = device_add(minor->kdev);
> > > > @@ -172,9 +187,13 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
> > > > goto err_debugfs;
> > > >
> > > > /* replace NULL with @minor so lookups will succeed from now on */
> > > > - spin_lock_irqsave(&drm_minor_lock, flags);
> > > > - idr_replace(&drm_minors_idr, minor, minor->index);
> > > > - spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > > + if (minor->type == DRM_MINOR_ACCEL) {
> > > > + accel_minor_replace(minor, minor->index);
> > > > + } else {
> > > > + spin_lock_irqsave(&drm_minor_lock, flags);
> > > > + idr_replace(&drm_minors_idr, minor, minor->index);
> > > > + spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > > + }
> > > >
> > > > DRM_DEBUG("new minor registered %d\n", minor->index);
> > > > return 0;
> > > > @@ -194,9 +213,13 @@ static void drm_minor_unregister(struct drm_device *dev, unsigned int type)
> > > > return;
> > > >
> > > > /* replace @minor with NULL so lookups will fail from now on */
> > > > - spin_lock_irqsave(&drm_minor_lock, flags);
> > > > - idr_replace(&drm_minors_idr, NULL, minor->index);
> > > > - spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > > + if (minor->type == DRM_MINOR_ACCEL) {
> > > > + accel_minor_replace(NULL, minor->index);
> > > > + } else {
> > > > + spin_lock_irqsave(&drm_minor_lock, flags);
> > > > + idr_replace(&drm_minors_idr, NULL, minor->index);
> > > > + spin_unlock_irqrestore(&drm_minor_lock, flags);
> > > > + }
> > > >
> > > > device_del(minor->kdev);
> > > > dev_set_drvdata(minor->kdev, NULL); /* safety belt */
> > > > @@ -603,6 +626,14 @@ static int drm_dev_init(struct drm_device *dev,
> > > > /* no per-device feature limits by default */
> > > > dev->driver_features = ~0u;
> > > >
> > > > + if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL) &&
> > > > + (drm_core_check_feature(dev, DRIVER_RENDER) ||
> > > > + drm_core_check_feature(dev, DRIVER_MODESET))) {
> > > > +
> > > > + DRM_ERROR("DRM driver can't be both a compute acceleration and graphics driver\n");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > drm_legacy_init_members(dev);
> > > > INIT_LIST_HEAD(&dev->filelist);
> > > > INIT_LIST_HEAD(&dev->filelist_internal);
> > > > @@ -628,15 +659,21 @@ static int drm_dev_init(struct drm_device *dev,
> > > >
> > > > dev->anon_inode = inode;
> > > >
> > > > - if (drm_core_check_feature(dev, DRIVER_RENDER)) {
> > > > - ret = drm_minor_alloc(dev, DRM_MINOR_RENDER);
> > > > + if (drm_core_check_feature(dev, DRIVER_COMPUTE_ACCEL)) {
> > > > + ret = drm_minor_alloc(dev, DRM_MINOR_ACCEL);
> > > > if (ret)
> > > > goto err;
> > > > - }
> > > > + } else {
> > > > + if (drm_core_check_feature(dev, DRIVER_RENDER)) {
> > > > + ret = drm_minor_alloc(dev, DRM_MINOR_RENDER);
> > > > + if (ret)
> > > > + goto err;
> > > > + }
> > > >
> > > > - ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
> > > > - if (ret)
> > > > - goto err;
> > > > + ret = drm_minor_alloc(dev, DRM_MINOR_PRIMARY);
> > > > + if (ret)
> > > > + goto err;
> > > > + }
> > > >
> > > > ret = drm_legacy_create_map_hash(dev);
> > > > if (ret)
> > > > @@ -883,6 +920,10 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> > > > if (ret)
> > > > goto err_minors;
> > > >
> > > > + ret = drm_minor_register(dev, DRM_MINOR_ACCEL);
> > > > + if (ret)
> > > > + goto err_minors;
> > > > +
> > > > ret = create_compat_control_link(dev);
> > > > if (ret)
> > > > goto err_minors;
> > > > @@ -902,12 +943,13 @@ int drm_dev_register(struct drm_device *dev, unsigned long flags)
> > > > driver->name, driver->major, driver->minor,
> > > > driver->patchlevel, driver->date,
> > > > dev->dev ? dev_name(dev->dev) : "virtual device",
> > > > - dev->primary->index);
> > > > + dev->primary ? dev->primary->index : dev->accel->index);
> > > >
> > > > goto out_unlock;
> > > >
> > > > err_minors:
> > > > remove_compat_control_link(dev);
> > > > + drm_minor_unregister(dev, DRM_MINOR_ACCEL);
> > > > drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
> > > > drm_minor_unregister(dev, DRM_MINOR_RENDER);
> > > > out_unlock:
> > > > @@ -950,6 +992,7 @@ void drm_dev_unregister(struct drm_device *dev)
> > > > drm_legacy_rmmaps(dev);
> > > >
> > > > remove_compat_control_link(dev);
> > > > + drm_minor_unregister(dev, DRM_MINOR_ACCEL);
> > > > drm_minor_unregister(dev, DRM_MINOR_PRIMARY);
> > > > drm_minor_unregister(dev, DRM_MINOR_RENDER);
> > > > }
> > > > @@ -1034,6 +1077,7 @@ static const struct file_operations drm_stub_fops = {
> > > > static void drm_core_exit(void)
> > > > {
> > > > drm_privacy_screen_lookup_exit();
> > > > + accel_core_exit();
> > > > unregister_chrdev(DRM_MAJOR, "drm");
> > > > debugfs_remove(drm_debugfs_root);
> > > > drm_sysfs_destroy();
> > > > @@ -1061,6 +1105,10 @@ static int __init drm_core_init(void)
> > > > if (ret < 0)
> > > > goto error;
> > > >
> > > > + ret = accel_core_init();
> > > > + if (ret < 0)
> > > > + goto error;
> > > > +
> > > > drm_privacy_screen_lookup_init();
> > > >
> > > > drm_core_init_complete = true;
> > > > diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> > > > index 430e00b16eec..b8da978d85bb 100644
> > > > --- a/drivers/gpu/drm/drm_sysfs.c
> > > > +++ b/drivers/gpu/drm/drm_sysfs.c
> > > > @@ -19,6 +19,7 @@
> > > > #include <linux/kdev_t.h>
> > > > #include <linux/slab.h>
> > > >
> > > > +#include <drm/drm_accel.h>
> > > > #include <drm/drm_connector.h>
> > > > #include <drm/drm_device.h>
> > > > #include <drm/drm_file.h>
> > > > @@ -471,19 +472,26 @@ struct device *drm_sysfs_minor_alloc(struct drm_minor *minor)
> > > > struct device *kdev;
> > > > int r;
> > > >
> > > > - if (minor->type == DRM_MINOR_RENDER)
> > > > - minor_str = "renderD%d";
> > > > - else
> > > > - minor_str = "card%d";
> > > > -
> > > > kdev = kzalloc(sizeof(*kdev), GFP_KERNEL);
> > > > if (!kdev)
> > > > return ERR_PTR(-ENOMEM);
> > > >
> > > > device_initialize(kdev);
> > > > - kdev->devt = MKDEV(DRM_MAJOR, minor->index);
> > > > - kdev->class = drm_class;
> > > > - kdev->type = &drm_sysfs_device_minor;
> > > > +
> > > > + if (minor->type == DRM_MINOR_ACCEL) {
> > > > + minor_str = "accel%d";
> > > > + accel_set_device_instance_params(kdev, minor->index);
> > > > + } else {
> > > > + if (minor->type == DRM_MINOR_RENDER)
> > > > + minor_str = "renderD%d";
> > > > + else
> > > > + minor_str = "card%d";
> > > > +
> > > > + kdev->devt = MKDEV(DRM_MAJOR, minor->index);
> > > > + kdev->class = drm_class;
> > > > + kdev->type = &drm_sysfs_device_minor;
> > > > + }
> > > > +
> > > > kdev->parent = minor->dev->dev;
> > > > kdev->release = drm_sysfs_release;
> > > > dev_set_drvdata(kdev, minor);
> > > > --
> > > > 2.25.1
> > > >
On Tue, Nov 22, 2022 at 03:46:25PM +1000, Dave Airlie wrote:
> On Tue, 22 Nov 2022 at 09:06, Sonal Santan <[email protected]> wrote:
> >
> > On 11/19/22 12:44, Oded Gabbay wrote:
> > > This is the fourth (and hopefully last) version of the patch-set to add the
> > > new subsystem for compute accelerators. I removed the RFC headline as
> > > I believe it is now ready for merging.
> > >
> > > Compare to v3, this patch-set contains one additional patch that adds
> > > documentation regarding the accel subsystem. I hope it's good enough for
> > > this stage. In addition, there were few very minor fixes according to
> > > comments received on v3.
> > >
> > > The patches are in the following repo:
> > > https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4
> > >
> > > As in v3, The HEAD of that branch is a commit adding a dummy driver that
> > > registers an accel device using the new framework. This can be served
> > > as a simple reference.
> > >
> > > v1 cover letter:
> > > https://lkml.org/lkml/2022/10/22/544
> > >
> > > v2 cover letter:
> > > https://lore.kernel.org/lkml/[email protected]/T/
> > >
> > > v3 cover letter:
> > > https://lore.kernel.org/lkml/[email protected]/T/
> >
> > Thanks for defining the new accel subsystem. We are currently working on
> > DRM based drivers for unannounced acceleration devices. I am fine with
> > these changes with the assumption that the choice of using classic DRM
> > or accel is left up to the individual driver.
>
> I don't think that decision should be up to any individual driver
> author. It will have to be consensus with me/Daniel/Oded and the
> driver authors.
Plus the entire point of this is that it's _still_ a drm based driver. So
aside from changing a flag in the kernel driver and adjusting userspace to
find the right chardev, there should be zero changes need for an existing
drm based driver stack that gets ported to drivers/accel.
And of course if we realize there's issues as we add drivers, we can fix
things up. This is just to kick things off, not something that's going to
be cast in stone for all eternity.
Sonal, with that clarification/explanation, is this entire thing
reasonable in principal and you can drop an Ack onto the series?
Thanks, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
On 11/20/2022 3:04 PM, Jeffrey Hugo wrote:
> On 11/19/2022 1:44 PM, Oded Gabbay wrote:
>> This is the fourth (and hopefully last) version of the patch-set to
>> add the
>> new subsystem for compute accelerators. I removed the RFC headline as
>> I believe it is now ready for merging.
>>
>> Compare to v3, this patch-set contains one additional patch that adds
>> documentation regarding the accel subsystem. I hope it's good enough for
>> this stage. In addition, there were few very minor fixes according to
>> comments received on v3.
>>
>> The patches are in the following repo:
>> https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4
>>
>>
>> As in v3, The HEAD of that branch is a commit adding a dummy driver that
>> registers an accel device using the new framework. This can be served
>> as a simple reference.
>>
>> v1 cover letter:
>> https://lkml.org/lkml/2022/10/22/544
>>
>> v2 cover letter:
>> https://lore.kernel.org/lkml/[email protected]/T/
>>
>>
>> v3 cover letter:
>> https://lore.kernel.org/lkml/[email protected]/T/
>>
>>
>> Thanks,
>> Oded.
>
> Reviewed-by: Jeffrey Hugo <[email protected]>
>
> I have some nits. Nothing that I think should be a blocker for this
> series.
I don't recall if I previously mentioned this. I'm planning on updating
the QAIC series to be an accel driver. Therefore there should be
at-least 1 user (probably several) for this subsystem in short order.
Hi,
On Sat, Nov 19, 2022 at 10:44:31PM +0200, Oded Gabbay wrote:
> This is the fourth (and hopefully last) version of the patch-set to add the
> new subsystem for compute accelerators. I removed the RFC headline as
> I believe it is now ready for merging.
>
> Compare to v3, this patch-set contains one additional patch that adds
> documentation regarding the accel subsystem. I hope it's good enough for
> this stage. In addition, there were few very minor fixes according to
> comments received on v3.
>
> The patches are in the following repo:
> https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4
Acked-by: Maxime Ripard <[email protected]>
Maxime
On 11/22/22 06:54, Daniel Vetter wrote:
> On Tue, Nov 22, 2022 at 03:46:25PM +1000, Dave Airlie wrote:
>> On Tue, 22 Nov 2022 at 09:06, Sonal Santan <[email protected]> wrote:
>>>
>>> On 11/19/22 12:44, Oded Gabbay wrote:
>>>> This is the fourth (and hopefully last) version of the patch-set to add the
>>>> new subsystem for compute accelerators. I removed the RFC headline as
>>>> I believe it is now ready for merging.
>>>>
>>>> Compare to v3, this patch-set contains one additional patch that adds
>>>> documentation regarding the accel subsystem. I hope it's good enough for
>>>> this stage. In addition, there were few very minor fixes according to
>>>> comments received on v3.
>>>>
>>>> The patches are in the following repo:
>>>> https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4
>>>>
>>>> As in v3, The HEAD of that branch is a commit adding a dummy driver that
>>>> registers an accel device using the new framework. This can be served
>>>> as a simple reference.
>>>>
>>>> v1 cover letter:
>>>> https://lkml.org/lkml/2022/10/22/544
>>>>
>>>> v2 cover letter:
>>>> https://lore.kernel.org/lkml/[email protected]/T/
>>>>
>>>> v3 cover letter:
>>>> https://lore.kernel.org/lkml/[email protected]/T/
>>>
>>> Thanks for defining the new accel subsystem. We are currently working on
>>> DRM based drivers for unannounced acceleration devices. I am fine with
>>> these changes with the assumption that the choice of using classic DRM
>>> or accel is left up to the individual driver.
>>
>> I don't think that decision should be up to any individual driver
>> author. It will have to be consensus with me/Daniel/Oded and the
>> driver authors.
>
> Plus the entire point of this is that it's _still_ a drm based driver. So
> aside from changing a flag in the kernel driver and adjusting userspace to
> find the right chardev, there should be zero changes need for an existing
> drm based driver stack that gets ported to drivers/accel.
>
> And of course if we realize there's issues as we add drivers, we can fix
> things up. This is just to kick things off, not something that's going to
> be cast in stone for all eternity.
>
> Sonal, with that clarification/explanation, is this entire thing
> reasonable in principal and you can drop an Ack onto the series?
>
> Thanks, Daniel
Sounds good. The accel patch series is:
Acked-by: Sonal Santan <[email protected]>
-Sonal
Hi Oded,
On Sat, 19 Nov 2022 at 20:44, Oded Gabbay <[email protected]> wrote:
> This is the fourth (and hopefully last) version of the patch-set to add the
> new subsystem for compute accelerators. I removed the RFC headline as
> I believe it is now ready for merging.
>
> Compare to v3, this patch-set contains one additional patch that adds
> documentation regarding the accel subsystem. I hope it's good enough for
> this stage. In addition, there were few very minor fixes according to
> comments received on v3.
>
> The patches are in the following repo:
> https://git.kernel.org/pub/scm/linux/kernel/git/ogabbay/accel.git/log/?h=accel_v4
Series is:
Acked-by: Daniel Stone <[email protected]>
Cheers,
Daniel