On Thu, Oct 15, 2026 at 01:08:24PM +0530, Jayant Shekhar wrote:
DCP may be power-collapsed while the VirtIO transport remains available.
The host therefore needs the platform IPCC channel to wake the firmware and
to receive notification that it can service HFI queues.
Is it allowed per the VirtIO spec?
Add the IPCC mailbox client and power-notify interrupt handling. Wake DCP
before VirtIO queue activity, wait for the firmware acknowledgment, and
notify the response queue when the IPCC receive path completes.
Don't describe patch contents. Describe your design decisions. The
reasons for the flags, init/deinit functions, etc.
Signed-off-by: Jayant Shekhar <[email protected]>
Assisted-by: LLM
---
drivers/gpu/drm/msm/Makefile | 1 +
drivers/gpu/drm/msm/dcp/dcp_drv.h | 1 +
drivers/gpu/drm/msm/dcp/dcp_ipcc.c | 212 +++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/msm/dcp/dcp_ipcc.h | 56 ++++++++++
4 files changed, 270 insertions(+)
diff --git a/drivers/gpu/drm/msm/Makefile b/drivers/gpu/drm/msm/Makefile
index bdc34babfc21..4195bc0e1fc9 100644
--- a/drivers/gpu/drm/msm/Makefile
+++ b/drivers/gpu/drm/msm/Makefile
@@ -71,6 +71,7 @@ msm-display-$(CONFIG_DRM_MSM_MDP5) += \
msm-display-$(CONFIG_DRM_MSM_DCP) += \
dcp/dcp_hfi_core.o \
+ dcp/dcp_ipcc.o \
dcp/hfi/hfi_packer.o \
dcp/hfi/hfi_unpacker.o \
dcp/hfi/hfi_utils.o
diff --git a/drivers/gpu/drm/msm/dcp/dcp_drv.h
b/drivers/gpu/drm/msm/dcp/dcp_drv.h
index bf6c34af43a0..af2db3cf9066 100644
--- a/drivers/gpu/drm/msm/dcp/dcp_drv.h
+++ b/drivers/gpu/drm/msm/dcp/dcp_drv.h
@@ -13,6 +13,7 @@
#include <linux/virtio.h>
#include "dcp_hfi.h"
+#include "dcp_ipcc.h"
struct dcp_dpu_kms;
diff --git a/drivers/gpu/drm/msm/dcp/dcp_ipcc.c b/drivers/gpu/drm/msm/dcp/dcp_ipcc.c
new file mode 100644
index 000000000000..e60a3ecbe3d8
--- /dev/null
+++ b/drivers/gpu/drm/msm/dcp/dcp_ipcc.c
@@ -0,0 +1,212 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ *
+ * DCP IPCC power-notify mailbox client.
+ *
+ * The APSS-to-DCP channel must be signaled before a VirtIO queue kick.
+ */
+
+#include <linux/device.h>
+#include <linux/err.h>
+#include <linux/jiffies.h>
+#include <linux/interrupt.h>
+#include <linux/mailbox_client.h>
+#include <linux/mutex.h>
+#include <linux/of_irq.h>
+#include <linux/string.h>
+#include <linux/virtio.h>
+
+#include "dcp_ipcc.h"
+
+#define DCP_IPCC_POWER_NOTIFY_TIMEOUT_MS 10000
+
+static void dcp_ipcc_handle_rx(struct dcp_ipcc *ipcc)
+{
+ unsigned long flags;
+
+ spin_lock_irqsave(&ipcc->state_lock, flags);
+ if (ipcc->shutting_down) {
+ spin_unlock_irqrestore(&ipcc->state_lock, flags);
+ return;
+ }
+
+ if (atomic_xchg(&ipcc->power_notify_pending, 0))
+ complete(&ipcc->power_notify_done);
Can there be more than one power notification pending? Can't we just
call complete() here?
+
+ if (ipcc->resp_vq && virtqueue_kick_prepare(ipcc->resp_vq))
+ virtqueue_notify(ipcc->resp_vq);
+ spin_unlock_irqrestore(&ipcc->state_lock, flags);
+}
+
+static irqreturn_t dcp_ipcc_power_notify_irq(int irq, void *data)
+{
+ struct dcp_ipcc *ipcc = data;
+
+ dcp_ipcc_handle_rx(ipcc);
+
+ return IRQ_HANDLED;
+}
+
+static void dcp_ipcc_rx_callback(struct mbox_client *cl, void *data)
+{
+ struct dcp_ipcc *ipcc = container_of(cl, struct dcp_ipcc, client);
+
+ dcp_ipcc_handle_rx(ipcc);
+}
+
+int dcp_ipcc_init(struct dcp_ipcc *ipcc, struct device *dev)
+{
+ int irq, ret;
+
+ if (!ipcc || !dev)
+ return -EINVAL;
+
+ memset(ipcc, 0, sizeof(*ipcc));
+ init_completion(&ipcc->power_notify_done);
+ mutex_init(&ipcc->power_notify_lock);
+ spin_lock_init(&ipcc->state_lock);
+ atomic_set(&ipcc->power_notify_pending, 0);
+
+ ipcc->client.dev = dev;
+ ipcc->client.tx_block = false;
+ ipcc->client.tx_tout = 0;
+ ipcc->client.knows_txdone = true;
+ ipcc->client.rx_callback = dcp_ipcc_rx_callback;
+
+ ipcc->power_chan = mbox_request_channel_byname(&ipcc->client,
+ "dcp-power-tx");
+ if (IS_ERR(ipcc->power_chan)) {
+ ret = PTR_ERR(ipcc->power_chan);
+ ipcc->power_chan = NULL;
+ if (ret != -EPROBE_DEFER)
+ dev_err(dev, "IPCC: failed to get 'dcp-power-tx':
%d\n", ret);
+ return ret;
+ }
+
+ irq = of_irq_get_byname(dev->of_node, "dcp_power_notify_irq");
+ if (irq <= 0) {
+ ret = irq ? irq : -EINVAL;
+ ret = dev_err_probe(dev, ret,
+ "failed to get required power notify
IRQ\n");
+ goto err_free_channel;
+ }
+
+ ret = request_irq(irq, dcp_ipcc_power_notify_irq,
+ 0, "dcp-power-notify", ipcc);
+ if (ret) {
+ ret = dev_err_probe(dev, ret,
+ "failed to request power notify IRQ %d\n",
irq);
+ goto err_free_channel;
+ }
+ ipcc->power_notify_irq = irq;
+
+ ret = enable_irq_wake(irq);
This is typically called from the suspend callback. Why are you calling
it in init path?
+ if (ret) {
+ dev_err(dev, "failed to enable power notify IRQ wake: %d\n",
ret);
+ goto err_free_irq;
+ }
+ ipcc->power_notify_irq_wake = true;
+
+ return 0;
+
+err_free_irq:
+ free_irq(irq, ipcc);
+ ipcc->power_notify_irq = 0;
+err_free_channel:
+ mbox_free_channel(ipcc->power_chan);
+ ipcc->power_chan = NULL;
+ return ret;
+}
+
+void dcp_ipcc_set_resp_vq(struct dcp_ipcc *ipcc, struct virtqueue *resp_vq)
+{
+ unsigned long flags;
+
+ if (!ipcc)
+ return;
+
+ spin_lock_irqsave(&ipcc->state_lock, flags);
+ if (!ipcc->shutting_down)
+ ipcc->resp_vq = resp_vq;
+ spin_unlock_irqrestore(&ipcc->state_lock, flags);
+}
+
+void dcp_ipcc_deinit(struct dcp_ipcc *ipcc)
+{
+ struct mbox_chan *power_chan;
+ int irq;
+ bool irq_wake;
+ unsigned long flags;
+
+ if (!ipcc)
+ return;
Oh, you again ignored the feedback. Can ipcc be NULL here? What would
that mean?
+
+ mutex_lock(&ipcc->power_notify_lock);
+ spin_lock_irqsave(&ipcc->state_lock, flags);
+ ipcc->shutting_down = true;
+ ipcc->resp_vq = NULL;
+ irq = ipcc->power_notify_irq;
+ ipcc->power_notify_irq = 0;
+ irq_wake = ipcc->power_notify_irq_wake;
+ ipcc->power_notify_irq_wake = false;
+ power_chan = ipcc->power_chan;
+ ipcc->power_chan = NULL;
+ spin_unlock_irqrestore(&ipcc->state_lock, flags);
Ugh. No.
+
+ if (irq > 0) {
Can it be 0 here?
+ disable_irq(irq);
What for?
+ if (irq_wake)
+ disable_irq_wake(irq);
It is enabled at this point. Drop the useless flags.
+ free_irq(irq, ipcc);
We have devm_foo() functions to simplify the teardown. Can we use them
here?
+ }
+ if (power_chan)
+ mbox_free_channel(power_chan);
+ mutex_unlock(&ipcc->power_notify_lock);
+}
+
+int dcp_ipcc_send_power_notify(struct dcp_ipcc *ipcc)
+{
+ unsigned long flags, timeout;
+ int ret;
+
+ if (!ipcc)
+ return -ENODEV;
+
+ /* Wait for the doorbell acknowledgment before returning to VirtIO. */
+ mutex_lock(&ipcc->power_notify_lock);
+ spin_lock_irqsave(&ipcc->state_lock, flags);
+ if (ipcc->shutting_down || !ipcc->power_chan) {
+ spin_unlock_irqrestore(&ipcc->state_lock, flags);
+ ret = -ENODEV;
+ goto out_unlock;
+ }
+ spin_unlock_irqrestore(&ipcc->state_lock, flags);
+
+ reinit_completion(&ipcc->power_notify_done);
+ atomic_set(&ipcc->power_notify_pending, 1);
+
+ ret = mbox_send_message(ipcc->power_chan, NULL);
+ mbox_client_txdone(ipcc->power_chan, ret);
+ if (ret < 0) {
+ atomic_set(&ipcc->power_notify_pending, 0);
+ dev_err(ipcc->client.dev,
+ "IPCC TX: power notify failed: %d\n", ret);
+ goto out_unlock;
+ }
+
+ timeout = wait_for_completion_timeout(&ipcc->power_notify_done,
+
msecs_to_jiffies(DCP_IPCC_POWER_NOTIFY_TIMEOUT_MS));
+ atomic_set(&ipcc->power_notify_pending, 0);
+ if (!timeout) {
+ dev_err(ipcc->client.dev,
+ "IPCC power notify acknowledgment timed out\n");
+ ret = -ETIMEDOUT;
+ } else {
+ ret = 0;
+ }
+
+out_unlock:
+ mutex_unlock(&ipcc->power_notify_lock);
+ return ret;
+}
diff --git a/drivers/gpu/drm/msm/dcp/dcp_ipcc.h
b/drivers/gpu/drm/msm/dcp/dcp_ipcc.h
new file mode 100644
index 000000000000..a3430e5cbe72
--- /dev/null
+++ b/drivers/gpu/drm/msm/dcp/dcp_ipcc.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
+ *
+ * IPCC power-notify mailbox interface for DCP firmware.
+ *
+ * Channel 5 carries APSS-to-DCP wake notifications; responses arrive through
+ * the mailbox client's RX callback.
+ */
+
+#ifndef __DCP_IPCC_H__
+#define __DCP_IPCC_H__
+
+#include <linux/atomic.h>
+#include <linux/completion.h>
+#include <linux/mailbox_client.h>
+#include <linux/mutex.h>
+#include <linux/spinlock.h>
+
+struct device;
+struct virtqueue;
+
+/**
+ * struct dcp_ipcc - DCP IPCC power-notification state
+ * @client: Mailbox client used for APSS-to-DCP notifications.
+ * @power_chan: Mailbox channel carrying power notifications.
+ * @resp_vq: VirtIO response queue notified by the IPCC receive path.
+ * @power_notify_done: Completion for the firmware acknowledgment.
+ * @power_notify_lock: Serializes power notifications and teardown.
+ * @power_notify_pending: Indicates an outstanding acknowledgment.
+ * @state_lock: Protects shutdown state and response queue access.
+ * @shutting_down: Prevents callbacks from accessing teardown state.
What is going to access it during teardown? Can we shutdown in a logical
way, so that there are no accessors after starting the teardown?
