Re: [PATCH RFC 07/18] accel/qda: Add DRM accel device registration for QDA driver
From: Dmitry Baryshkov
Date: Mon Feb 23 2026 - 17:17:46 EST
On Tue, Feb 24, 2026 at 12:39:01AM +0530, Ekansh Gupta wrote:
> Add DRM accel integration for the QDA DSP accelerator driver. A new
> qda_drm_priv structure is introduced to hold per-device DRM state,
> including a pointer to the memory manager and the parent qda_dev
> instance. The driver now allocates a drm_device, initializes
> driver-private state, and registers the device via the DRM accel
> infrastructure.
>
> qda_register_device() performs allocation and registration of the DRM
> device, while qda_unregister_device() handles device teardown and
> releases references using drm_dev_unregister() and drm_dev_put().
> Initialization and teardown paths are updated so DRM resources are
> allocated after IOMMU/memory-manager setup and cleaned during RPMsg
> remove.
>
> This patch lays the foundation for adding GEM buffer support and IOCTL
> handling in later patches as part of the compute accelerator interface.
>
> Signed-off-by: Ekansh Gupta <ekansh.gupta@xxxxxxxxxxxxxxxx>
> ---
> drivers/accel/qda/qda_drv.c | 103 ++++++++++++++++++++++++++++++++++++++++++
> drivers/accel/qda/qda_drv.h | 33 +++++++++++++-
> drivers/accel/qda/qda_rpmsg.c | 8 ++++
> 3 files changed, 142 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/accel/qda/qda_drv.c b/drivers/accel/qda/qda_drv.c
> index 69132737f964..a9113ec78fa2 100644
> --- a/drivers/accel/qda/qda_drv.c
> +++ b/drivers/accel/qda/qda_drv.c
> @@ -4,9 +4,31 @@
> #include <linux/kernel.h>
> #include <linux/atomic.h>
> #include <linux/slab.h>
> +#include <drm/drm_accel.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_file.h>
> +#include <drm/drm_gem.h>
> +#include <drm/drm_ioctl.h>
> #include "qda_drv.h"
> #include "qda_rpmsg.h"
>
> +DEFINE_DRM_ACCEL_FOPS(qda_accel_fops);
> +
> +static struct drm_driver qda_drm_driver = {
> + .driver_features = DRIVER_COMPUTE_ACCEL,
> + .fops = &qda_accel_fops,
Strange indentation in the middle. Please drop it.
> + .name = DRIVER_NAME,
> + .desc = "Qualcomm DSP Accelerator Driver",
> +};
> +
> +static void cleanup_drm_private(struct qda_dev *qdev)
> +{
> + if (qdev->drm_priv) {
> + qda_dbg(qdev, "Cleaning up DRM private data\n");
> + kfree(qdev->drm_priv);
> + }
> +}
> +
> static void cleanup_iommu_manager(struct qda_dev *qdev)
> {
> if (qdev->iommu_mgr) {
> @@ -24,6 +46,7 @@ static void cleanup_device_resources(struct qda_dev *qdev)
>
> void qda_deinit_device(struct qda_dev *qdev)
> {
> + cleanup_drm_private(qdev);
> cleanup_iommu_manager(qdev);
> cleanup_device_resources(qdev);
> }
> @@ -59,6 +82,18 @@ static int init_memory_manager(struct qda_dev *qdev)
> return 0;
> }
>
> +static int init_drm_private(struct qda_dev *qdev)
> +{
> + qda_dbg(qdev, "Initializing DRM private data\n");
> +
> + qdev->drm_priv = kzalloc_obj(*qdev->drm_priv, GFP_KERNEL);
> + if (!qdev->drm_priv)
> + return -ENOMEM;
> +
> + qda_dbg(qdev, "DRM private data initialized successfully\n");
> + return 0;
> +}
> +
> int qda_init_device(struct qda_dev *qdev)
> {
> int ret;
> @@ -71,14 +106,82 @@ int qda_init_device(struct qda_dev *qdev)
> goto err_cleanup_resources;
> }
>
> + ret = init_drm_private(qdev);
> + if (ret) {
> + qda_err(qdev, "DRM private data initialization failed: %d\n", ret);
> + goto err_cleanup_iommu;
> + }
> +
> qda_dbg(qdev, "QDA device initialized successfully\n");
> return 0;
>
> +err_cleanup_iommu:
> + cleanup_iommu_manager(qdev);
> err_cleanup_resources:
> cleanup_device_resources(qdev);
> return ret;
> }
>
> +static int setup_and_register_drm_device(struct qda_dev *qdev)
> +{
> + struct drm_device *ddev;
> + int ret;
> +
> + qda_dbg(qdev, "Setting up and registering DRM device\n");
> +
> + ddev = drm_dev_alloc(&qda_drm_driver, qdev->dev);
devm_drm_dev_alloc() please. Move this patch to the front of the series,
making everything else depend on the allocated data structure.
> + if (IS_ERR(ddev)) {
> + ret = PTR_ERR(ddev);
> + qda_err(qdev, "Failed to allocate DRM device: %d\n", ret);
> + return ret;
> + }
> +
> + qdev->drm_priv->drm_dev = ddev;
> + qdev->drm_priv->iommu_mgr = qdev->iommu_mgr;
> + qdev->drm_priv->qdev = qdev;
> +
> + ddev->dev_private = qdev->drm_priv;
> + qdev->drm_dev = ddev;
> +
> + ret = drm_dev_register(ddev, 0);
> + if (ret) {
> + qda_err(qdev, "Failed to register DRM device: %d\n", ret);
> + drm_dev_put(ddev);
> + return ret;
> + }
> +
> + qda_dbg(qdev, "DRM device registered successfully\n");
> + return 0;
> +}
> +
> +int qda_register_device(struct qda_dev *qdev)
> +{
> + int ret;
> +
> + ret = setup_and_register_drm_device(qdev);
> + if (ret) {
> + qda_err(qdev, "DRM device setup failed: %d\n", ret);
> + return ret;
> + }
> +
> + qda_dbg(qdev, "QDA device registered successfully\n");
> + return 0;
> +}
> +
> +void qda_unregister_device(struct qda_dev *qdev)
> +{
> + qda_info(qdev, "Unregistering QDA device\n");
> +
> + if (qdev->drm_dev) {
> + qda_dbg(qdev, "Unregistering DRM device\n");
> + drm_dev_unregister(qdev->drm_dev);
> + drm_dev_put(qdev->drm_dev);
> + qdev->drm_dev = NULL;
> + }
> +
> + qda_dbg(qdev, "QDA device unregistered successfully\n");
> +}
> +
> static int __init qda_core_init(void)
> {
> int ret;
> diff --git a/drivers/accel/qda/qda_drv.h b/drivers/accel/qda/qda_drv.h
> index 2cb97e4eafbf..2b80401a3741 100644
> --- a/drivers/accel/qda/qda_drv.h
> +++ b/drivers/accel/qda/qda_drv.h
> @@ -11,13 +11,35 @@
> #include <linux/mutex.h>
> #include <linux/rpmsg.h>
> #include <linux/xarray.h>
> +#include <drm/drm_drv.h>
> +#include <drm/drm_file.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_accel.h>
> #include "qda_memory_manager.h"
>
> /* Driver identification */
> #define DRIVER_NAME "qda"
>
> +/**
> + * struct qda_drm_priv - DRM device private data for QDA device
> + *
> + * This structure serves as the DRM device private data (stored in dev_private),
> + * bridging the DRM device context with the QDA device and providing access to
> + * shared resources like the memory manager during buffer operations.
> + */
> +struct qda_drm_priv {
Shared between what and what? Why do you need a separate structure
instead of using qda_dev?
> + /* DRM device structure */
> + struct drm_device *drm_dev;
> + /* Global memory/IOMMU manager */
> + struct qda_memory_manager *iommu_mgr;
> + /* Back-pointer to qda_dev */
> + struct qda_dev *qdev;
> +};
> +
> /* struct qda_dev - Main device structure for QDA driver */
> struct qda_dev {
> + /* DRM device for accelerator interface */
> + struct drm_device *drm_dev;
Drop the pointer here.
> /* RPMsg device for communication with remote processor */
> struct rpmsg_device *rpdev;
> /* Underlying device structure */
> @@ -26,6 +48,8 @@ struct qda_dev {
> struct mutex lock;
> /* IOMMU/memory manager */
> struct qda_memory_manager *iommu_mgr;
> + /* DRM device private data */
> + struct qda_drm_priv *drm_priv;
> /* Flag indicating device removal in progress */
> atomic_t removing;
> /* Name of the DSP (e.g., "cdsp", "adsp") */
> @@ -39,8 +63,8 @@ struct qda_dev {
> * @qdev: QDA device structure
> *
> * Returns the most appropriate device structure for logging messages.
> - * Prefers qdev->dev, or returns NULL if the device is being removed
> - * or invalid.
> + * Prefers qdev->dev, falls back to qdev->drm_dev->dev, or returns NULL
> + * if the device is being removed or invalid.
> */
> static inline struct device *qda_get_log_device(struct qda_dev *qdev)
> {
> @@ -50,6 +74,9 @@ static inline struct device *qda_get_log_device(struct qda_dev *qdev)
> if (qdev->dev)
> return qdev->dev;
>
> + if (qdev->drm_dev)
> + return qdev->drm_dev->dev;
> +
> return NULL;
> }
>
> @@ -93,5 +120,7 @@ static inline struct device *qda_get_log_device(struct qda_dev *qdev)
> */
> int qda_init_device(struct qda_dev *qdev);
> void qda_deinit_device(struct qda_dev *qdev);
> +int qda_register_device(struct qda_dev *qdev);
> +void qda_unregister_device(struct qda_dev *qdev);
>
> #endif /* __QDA_DRV_H__ */
> diff --git a/drivers/accel/qda/qda_rpmsg.c b/drivers/accel/qda/qda_rpmsg.c
> index 5a57384de6a2..b2b44b4d3ca8 100644
> --- a/drivers/accel/qda/qda_rpmsg.c
> +++ b/drivers/accel/qda/qda_rpmsg.c
> @@ -80,6 +80,7 @@ static void qda_rpmsg_remove(struct rpmsg_device *rpdev)
> qdev->rpdev = NULL;
> mutex_unlock(&qdev->lock);
>
> + qda_unregister_device(qdev);
> qda_unpopulate_child_devices(qdev);
> qda_deinit_device(qdev);
>
> @@ -123,6 +124,13 @@ static int qda_rpmsg_probe(struct rpmsg_device *rpdev)
> return ret;
> }
>
> + ret = qda_register_device(qdev);
> + if (ret) {
> + qda_deinit_device(qdev);
> + qda_unpopulate_child_devices(qdev);
> + return ret;
> + }
> +
> qda_info(qdev, "QDA RPMsg probe completed successfully for %s\n", qdev->dsp_name);
> return 0;
> }
>
> --
> 2.34.1
>
--
With best wishes
Dmitry