On 10/22/2022 3:46 PM, Oded Gabbay wrote:
The accelerator devices are exposed to user-space using a dedicated
major. In addition, they are represented in /dev with new, dedicated
device char names: /dev/accel/accel*. This is done to make sure any
user-space software that tries to open a graphic card won't open
the accelerator device by mistake.
The above implies that the minor numbering should be separated from
the rest of the drm devices. However, to avoid code duplication, we
want the drm_minor structure to be able to represent the accelerator
device.
To achieve this, we add a new drm_minor* to drm_device that represents
the accelerator device. This pointer is initialized for drivers that
declare they handle compute accelerator, using a new driver feature
flag called DRIVER_COMPUTE_ACCEL. It is important to note that this
driver feature is mutually exclusive with DRIVER_RENDER. 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.
In addition, we define a different idr to handle the accelerators
minors. This is done to make the minor's index be identical to the
device index in /dev/. In most places, this is hidden inside the drm
core functions except when calling drm_minor_acquire(), where I had to
add an extra parameter to specify the idr to use (because the
accelerators minors index and the drm primary minor index both begin
at 0).
Signed-off-by: Oded Gabbay <ogabbay@xxxxxxxxxx>
---
drivers/gpu/drm/drm_drv.c | 171 +++++++++++++++++++++++++--------
drivers/gpu/drm/drm_file.c | 69 +++++++++----
drivers/gpu/drm/drm_internal.h | 2 +-
drivers/gpu/drm/drm_sysfs.c | 29 ++++--
include/drm/drm_device.h | 3 +
include/drm/drm_drv.h | 8 ++
include/drm/drm_file.h | 21 +++-
7 files changed, 235 insertions(+), 68 deletions(-)
Can we please add something to Documentation? I know this leverages DRM
a lot, but I believe that a new subsystem should not be introduced
without documentation. A lot of the info in the commit message is very
good, but should not be buried in the git log.
Besides, imagine this has been in mainline for N years, and someone
completely new to the kernel wants to write an accel driver. They
should be able to get started with something from Documentation that
at-least gives that person some insight into what to grep the code for.
diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index b58ffb1433d6..c13701a8d4be 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -56,6 +56,9 @@ MODULE_LICENSE("GPL and additional rights");
static DEFINE_SPINLOCK(drm_minor_lock);
static struct idr drm_minors_idr;
+static DEFINE_SPINLOCK(accel_minor_lock);
+static struct idr accel_minors_idr;
IDR is deprecated. XArray is the preferred mechanism.
Yes, there already is IDR here, but I believe we should not be adding
new uses. Maybe at some point, the current IDR will be converted. Also
with XArray, I think you don't need the spinlock since XArray has
internal locking already.
