[PATCH v5] drm/tyr: add Job IRQ handling

[PATCH v5] drm/tyr: add Job IRQ handling

[Laura Nao]

Add a threaded IRQ wrapper for Tyr interrupt sources and use it to
handle the firmware Job IRQ.

The Job IRQ reports requests from the CSF firmware, including global
interface requests and CSG attention bits. Only the GLB bit is currently
handled, as it will be used to check firmware readiness. CSG bits
handling will be added at a later stage. Add a Job IRQ handler that
masks the interrupt in the primary IRQ handler, processes pending raw
status in the threaded handler, clears the handled bit, and reenables
the mask before returning.
Add a wait queue and a bool flag so the handler can signal firmware
readiness when the GLB bit is set.

Co-developed-by: Daniel Almeida <daniel.almeida@collabora.com>
Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
Co-developed-by: Deborah Brouwer <deborah.brouwer@collabora.com>
Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
Signed-off-by: Laura Nao <laura.nao@collabora.com>
---
Changes in v5:
- Implement PinnedDrop on TyrIrq to clear the interrupt mask on teardown 
- Link to v4: https://lore.kernel.org/r/20260805-tyr-irq-v2-v4-1-81a4701611c0@collabora.com

Changes in v4:
- Replaced c_str! with C-String in irq.rs
- Moved clear_status() before handle() in handle_threaded(), to avoid 
  losing events if hw re-asserts the bit while handle() is running.
- Only unmask interrupt sources in job_irq_init() when the handler is 
  correctly registered.
- Link to v3: https://lore.kernel.org/r/20260728-tyr-irq-v2-v3-1-9c9bc5c029c5@collabora.com

Changes in v3:
- Propagated request_threaded_irq_by_name safety requirement up the
  call chain, made TyrIrq::request() and job_irq_init() unsafe
- Dropped CSG bits masking, added TODO comment to note this will be
  handled at a later stage. Reworded commit message to reflect the
  change.
- Added documentation for TyrIrqTrait, TyrIrq, JobIrq and job_irq_init()
- Link to v2: https://lore.kernel.org/r/20260729-tyr-irq-v2-v2-1-c2750fc11d59@collabora.com

Changes in v2:
- Dropped Wait custom type in favor of WaitQueue
- Renamed JobIrq lifetime to generic 'a

This patch follows up to [1], which adds support for firmware loading
and MCU booting to the Tyr driver. The changes included here were
originally introduced in its v4[2], then dropped to reduce the scope of
the series, and have been adjusted to work with the HRT (Higher-Ranked
Lifetime Types) driver architecture recently introduced.

The patch adds a threaded IRQ wrapper for the firmware Job interrupts,
used to signal events from the global CSF (GLB) and Command Stream Group
(CSG) interfaces.

These changes will be later used to wait for global CSF interface
readiness after firmware boot, as part of the CSF firmware interfaces
support that will be submitted as a separate series.

This patch is based on drm-rust-next and depends on:
- [PATCH v2] rust: irq: make Registration compatible with lifetime-bound drivers[3]
- [PATCH 0/5] rust: sync: add WaitQueue infrastructure[4]

Note: [4] doesn't apply cleanly on drm-rust-next at the moment, due to
missing changes in rust/kernel/sync/lock/spinlock.rs[5]. I've applied
all dependencies and fixed conflicts for the purpose of testing this
patch on top of drm-rust-next, a branch with these changes is available
here: https://gitlab.freedesktop.org/laura.nao/linux/-/commits/b4/tyr-irq-v5

[1] https://lore.kernel.org/all/20260728-fw-boot-b4-v10-0-9187aefa3f2f@collabora.com/ 
[2] https://lore.kernel.org/rust-for-linux/20260424-b4-fw-boot-v4-v4-15-a5d91050789d@collabora.com/
[3] https://lore.kernel.org/rust-for-linux/20260719153631.559341-1-dakr@kernel.org/
[4] https://lore.kernel.org/rust-for-linux/20260726223613.1242940-1-dakr@kernel.org/
[5] https://lore.kernel.org/all/20260302232154.861916-1-lyude@redhat.com/
---
 drivers/gpu/drm/tyr/driver.rs | 108 ++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/tyr/fw.rs     |   1 +
 drivers/gpu/drm/tyr/fw/irq.rs | 112 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 221 insertions(+)

diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
index d78ad9d292ff..495cc3baa98c 100644
--- a/drivers/gpu/drm/tyr/driver.rs
+++ b/drivers/gpu/drm/tyr/driver.rs
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0 or MIT
 
+use core::marker::PhantomPinned;
+
 use kernel::{
     clk::{
         Clk,
@@ -21,6 +23,13 @@
         poll,
         Io, //
     },
+    irq::{
+        Flags,
+        IrqReturn,
+        ThreadedHandler,
+        ThreadedIrqReturn,
+        ThreadedRegistration, //
+    },
     new_mutex,
     of,
     platform,
@@ -236,3 +245,102 @@ struct Regulators {
     _mali: Regulator<regulator::Enabled>,
     _sram: Regulator<regulator::Enabled>,
 }
+
+/// Trait describing the operations required to service a Tyr interrupt source.
+pub(crate) trait TyrIrqTrait: Sync {
+    /// Returns the masked interrupt status.
+    fn read_status(&self) -> u32;
+
+    /// Clears all bits in the mask register, disabling the interrupt sources.
+    fn clear_mask(&self);
+
+    /// Sets the mask register to re-enable the interrupt sources.
+    fn reenable_mask(&self);
+
+    /// Returns the raw interrupt status, regardless of the current mask.
+    fn read_raw_status(&self) -> u32;
+
+    /// Clears the given bits in the interrupt status register.
+    fn clear_status(&self, status: u32);
+
+    /// Returns the bitmask of interrupt sources handled by this instance.
+    fn mask(&self) -> u32;
+
+    /// Handles the given pending, unmasked interrupt bits.
+    fn handle(&self, status: u32);
+}
+
+/// A threaded IRQ wrapper shared by all Tyr interrupt sources.
+#[pin_data(PinnedDrop)]
+pub(crate) struct TyrIrq<T: TyrIrqTrait> {
+    /// The interrupt source.
+    irq: T,
+    #[pin]
+    _pin: PhantomPinned,
+}
+
+impl<T: TyrIrqTrait> TyrIrq<T> {
+    /// Requests a threaded IRQ registration for `irq`, using `name` to look up the interrupt.
+    ///
+    /// # Safety
+    ///
+    /// Callers must not `mem::forget()` the resulting registration or otherwise prevent its
+    /// [`Drop`] implementation from running.
+    pub(crate) unsafe fn request<'a>(
+        pdev: &'a platform::Device<Bound>,
+        name: &'static CStr,
+        irq: T,
+    ) -> Result<impl PinInit<ThreadedRegistration<'a, Self>, Error> + 'a>
+    where
+        T: 'a,
+    {
+        let handler = try_pin_init!(Self {
+            irq,
+            _pin: PhantomPinned,
+        });
+
+        // SAFETY: The caller guarantees the resulting registration will not be leaked.
+        Ok(unsafe { pdev.request_threaded_irq_by_name(Flags::SHARED, name, name, handler) })
+    }
+
+    /// Returns a reference to the wrapped interrupt source.
+    pub(crate) fn irq(&self) -> &T {
+        &self.irq
+    }
+}
+
+impl<T: TyrIrqTrait> ThreadedHandler for TyrIrq<T> {
+    fn handle(&self) -> ThreadedIrqReturn {
+        let masked_status = self.irq.read_status();
+
+        if masked_status == 0 {
+            return ThreadedIrqReturn::None;
+        }
+        self.irq.clear_mask();
+        ThreadedIrqReturn::WakeThread
+    }
+
+    fn handle_threaded(&self) -> IrqReturn {
+        let mut ret = IrqReturn::None;
+
+        loop {
+            let raw_status = self.irq.read_raw_status() & self.irq.mask();
+            if raw_status == 0 {
+                break;
+            }
+            self.irq.clear_status(raw_status);
+            self.irq.handle(raw_status);
+            ret = IrqReturn::Handled;
+        }
+
+        self.irq.reenable_mask();
+        ret
+    }
+}
+
+#[pinned_drop]
+impl<T: TyrIrqTrait> PinnedDrop for TyrIrq<T> {
+    fn drop(self: Pin<&mut Self>) {
+        self.irq.clear_mask();
+    }
+}
diff --git a/drivers/gpu/drm/tyr/fw.rs b/drivers/gpu/drm/tyr/fw.rs
index 47d25c901bd0..8162b9806c97 100644
--- a/drivers/gpu/drm/tyr/fw.rs
+++ b/drivers/gpu/drm/tyr/fw.rs
@@ -69,6 +69,7 @@
     vm::Vm, //
 };
 
+pub(crate) mod irq;
 mod parser;
 
 pub(super) const CSF_MCU_SHARED_REGION_START: u32 = 0x04000000;
diff --git a/drivers/gpu/drm/tyr/fw/irq.rs b/drivers/gpu/drm/tyr/fw/irq.rs
new file mode 100644
index 000000000000..fcdc0097f2dd
--- /dev/null
+++ b/drivers/gpu/drm/tyr/fw/irq.rs
@@ -0,0 +1,112 @@
+// SPDX-License-Identifier: GPL-2.0 or MIT
+
+//! IRQ handling for the Job IRQ.
+//!
+//! The Job IRQ signals events from the MCU, including global interface acknowledgements.
+#![allow(dead_code)]
+
+use core::sync::atomic::{
+    AtomicBool,
+    Ordering, //
+};
+
+use kernel::{
+    device::Bound, //
+    io::Io,
+    irq::ThreadedRegistration,
+    platform,
+    prelude::*,
+    sync::{
+        Arc,
+        WaitQueue, //
+    },
+};
+
+use crate::{
+    driver::{
+        IoMem,
+        TyrIrq,
+        TyrIrqTrait, //
+    },
+    regs::job_control::{
+        JOB_IRQ_CLEAR,
+        JOB_IRQ_MASK,
+        JOB_IRQ_RAWSTAT,
+        JOB_IRQ_STATUS, //
+    }, //
+};
+
+// The Job IRQ, signalling requests or notification from the MCU.
+pub(crate) struct JobIrq<'a> {
+    /// GPU MMIO register mapping.
+    iomem: Arc<IoMem<'a>>,
+    /// Flag to signal firmware readiness.
+    fw_ready: Arc<AtomicBool>,
+    /// Wait queue used to signal firmware events to waiters.
+    job_irq_wait: Arc<WaitQueue>,
+}
+
+/// Requests a threaded IRQ registration for the Job IRQ.
+///
+/// # Safety
+///
+/// Callers must not `mem::forget()` the resulting registration or otherwise prevent its
+/// [`Drop`] implementation from running.
+pub(crate) unsafe fn job_irq_init<'a>(
+    pdev: &'a platform::Device<Bound>,
+    iomem: Arc<IoMem<'a>>,
+    fw_ready: Arc<AtomicBool>,
+    job_irq_wait: Arc<WaitQueue>,
+) -> Result<impl PinInit<ThreadedRegistration<'a, TyrIrq<JobIrq<'a>>>, Error> + 'a> {
+    let job_irq = JobIrq {
+        iomem,
+        fw_ready,
+        job_irq_wait,
+    };
+    // Ensure interrupt sources are disabled until the handler is registered.
+    job_irq.clear_mask();
+
+    // SAFETY: The caller guarantees the resulting registration will not be leaked.
+    let registration = unsafe { TyrIrq::request(pdev, c"job", job_irq) };
+
+    // Unmask once the handler has been successfully registered.
+    Ok(registration?.pin_chain(|reg| {
+        reg.handler().irq().reenable_mask();
+        Ok(())
+    }))
+}
+
+impl TyrIrqTrait for JobIrq<'_> {
+    fn read_status(&self) -> u32 {
+        self.iomem.read(JOB_IRQ_STATUS).into_raw()
+    }
+
+    fn clear_mask(&self) {
+        self.iomem.write_reg(JOB_IRQ_MASK::zeroed());
+    }
+
+    fn reenable_mask(&self) {
+        self.iomem.write_reg(JOB_IRQ_MASK::zeroed().with_glb(true));
+    }
+
+    fn read_raw_status(&self) -> u32 {
+        self.iomem.read(JOB_IRQ_RAWSTAT).into_raw()
+    }
+
+    fn clear_status(&self, status: u32) {
+        self.iomem.write_reg(JOB_IRQ_CLEAR::from_raw(status));
+    }
+
+    fn mask(&self) -> u32 {
+        JOB_IRQ_MASK::zeroed().with_glb(true).into_raw()
+    }
+
+    fn handle(&self, status: u32) {
+        // TODO: handle other Job IRQ events (e.g. CSG attention bits) here once
+        // support for them is added.
+        if JOB_IRQ_RAWSTAT::from_raw(status).glb() {
+            self.fw_ready.store(true, Ordering::Release);
+            self.job_irq_wait.wake_up_all();
+        }
+    }
+}

---
base-commit: 0710742e26a06c2c3167e77a8ea82b59632d9f7a
change-id: 20260728-tyr-irq-v2-0b3c5022be33

Best regards,
-- 
Laura Nao <laura.nao@collabora.com>

Re: [PATCH v5] drm/tyr: add Job IRQ handling

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Teardown race condition: device interrupts are disabled after the IRQ handler is unregistered, which can cause an interrupt storm on a shared IRQ line.
- [High] The threaded IRQ handler will enter an infinite loop if the device becomes inaccessible, causing a CPU stall.
- [Medium] Missing dummy read after clearing the interrupt mask causes spurious hardirq invocations.
--

--- Patch [1]: [PATCH v5] drm/tyr: add Job IRQ handling ---
commit 3c87c90f73f8609cae24e8591456f2e88af085b5
Author: Laura Nao <laura.nao@collabora.com>

drm/tyr: add Job IRQ handling

Add a threaded IRQ wrapper for Tyr interrupt sources and use it to
handle the firmware Job IRQ.

Link: https://lore.kernel.org/all/20260728-fw-boot-b4-v10-0-9187aefa3f2f@collabora.com/
Link: https://lore.kernel.org/rust-for-linux/20260424-b4-fw-boot-v4-v4-15-a5d91050789d@collabora.com/
Link: https://lore.kernel.org/rust-for-linux/20260719153631.559341-1-dakr@kernel.org/
Link: https://lore.kernel.org/rust-for-linux/20260726223613.1242940-1-dakr@kernel.org/
Link: https://lore.kernel.org/all/20260302232154.861916-1-lyude@redhat.com/

> diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> index d78ad9d292ff7..495cc3baa98c3 100644
> --- a/drivers/gpu/drm/tyr/driver.rs
> +++ b/drivers/gpu/drm/tyr/driver.rs

[ ... ]

> @@ -236,3 +245,102 @@ struct Regulators {
> +impl<T: TyrIrqTrait> ThreadedHandler for TyrIrq<T> {
> +    fn handle(&self) -> ThreadedIrqReturn {
> +        let masked_status = self.irq.read_status();
> +
> +        if masked_status == 0 {
> +            return ThreadedIrqReturn::None;
> +        }
> +        self.irq.clear_mask();
> +        ThreadedIrqReturn::WakeThread
> +    }

[Severity: Medium]
Is a dummy read required after clearing the mask to flush the posted write?

Looking at JobIrq::clear_mask() in fw/irq.rs, it performs a raw MMIO write
without a subsequent dummy read:

    fn clear_mask(&self) {
        self.iomem.write_reg(JOB_IRQ_MASK::zeroed());
    }

If the write hasn't reached the device when handle() returns WakeThread,
could the interrupt controller immediately re-invoke the hardirq handler,
causing spurious invocations?

> +
> +    fn handle_threaded(&self) -> IrqReturn {
> +        let mut ret = IrqReturn::None;
> +
> +        loop {
> +            let raw_status = self.irq.read_raw_status() & self.irq.mask();
> +            if raw_status == 0 {
> +                break;
> +            }

[Severity: High]
Does this loop need a safeguard against MMIO reads returning 0xFFFFFFFF?

If the device becomes inaccessible (e.g. from surprise removal, loss of
power, or a bus error) while handle_threaded() is running, 
read_raw_status() might return all 1s.

Since that value bitwise-ANDed with mask() is non-zero, could this cause 
the kernel thread to spin indefinitely and result in CPU stalls?

> +            self.irq.clear_status(raw_status);
> +            self.irq.handle(raw_status);
> +            ret = IrqReturn::Handled;
> +        }
> +
> +        self.irq.reenable_mask();
> +        ret
> +    }
> +}
> +
> +#[pinned_drop]
> +impl<T: TyrIrqTrait> PinnedDrop for TyrIrq<T> {
> +    fn drop(self: Pin<&mut Self>) {
> +        self.irq.clear_mask();
> +    }
> +}

[Severity: High]
Will clear_mask() execute too late during teardown, leading to an interrupt
storm?

Since TyrIrq is embedded inside the ThreadedRegistration returned by
request_threaded_irq_by_name(), the Rust drop order will drop the
ThreadedRegistration first (which calls free_irq()), followed by TyrIrq
(which calls clear_mask() here).

If the device asserts an interrupt after free_irq() completes but before
clear_mask() runs, could the unhandled interrupt on a shared line result
in an IRQ storm that permanently disables the IRQ line?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-tyr-irq-v2-v5-1-59fd9dea952a@collabora.com?part=1

Re: [PATCH v5] drm/tyr: add Job IRQ handling

[Daniel Almeida]

Hi Laura, thanks for working on this! :)

> On 10 Aug 2026, at 05:11, Laura Nao <laura.nao@collabora.com> wrote:
> 
> Add a threaded IRQ wrapper for Tyr interrupt sources and use it to
> handle the firmware Job IRQ.
> 
> The Job IRQ reports requests from the CSF firmware, including global
> interface requests and CSG attention bits. Only the GLB bit is currently
> handled, as it will be used to check firmware readiness. CSG bits
> handling will be added at a later stage. Add a Job IRQ handler that
> masks the interrupt in the primary IRQ handler, processes pending raw
> status in the threaded handler, clears the handled bit, and reenables
> the mask before returning.
> Add a wait queue and a bool flag so the handler can signal firmware
> readiness when the GLB bit is set.
> 
> Co-developed-by: Daniel Almeida <daniel.almeida@collabora.com>
> Signed-off-by: Daniel Almeida <daniel.almeida@collabora.com>
> Co-developed-by: Deborah Brouwer <deborah.brouwer@collabora.com>
> Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
> Signed-off-by: Laura Nao <laura.nao@collabora.com>
> ---
> Changes in v5:
> - Implement PinnedDrop on TyrIrq to clear the interrupt mask on teardown 
> - Link to v4: https://lore.kernel.org/r/20260805-tyr-irq-v2-v4-1-81a4701611c0@collabora.com
> 
> Changes in v4:
> - Replaced c_str! with C-String in irq.rs
> - Moved clear_status() before handle() in handle_threaded(), to avoid 
>  losing events if hw re-asserts the bit while handle() is running.
> - Only unmask interrupt sources in job_irq_init() when the handler is 
>  correctly registered.
> - Link to v3: https://lore.kernel.org/r/20260728-tyr-irq-v2-v3-1-9c9bc5c029c5@collabora.com
> 
> Changes in v3:
> - Propagated request_threaded_irq_by_name safety requirement up the
>  call chain, made TyrIrq::request() and job_irq_init() unsafe
> - Dropped CSG bits masking, added TODO comment to note this will be
>  handled at a later stage. Reworded commit message to reflect the
>  change.
> - Added documentation for TyrIrqTrait, TyrIrq, JobIrq and job_irq_init()
> - Link to v2: https://lore.kernel.org/r/20260729-tyr-irq-v2-v2-1-c2750fc11d59@collabora.com
> 
> Changes in v2:
> - Dropped Wait custom type in favor of WaitQueue
> - Renamed JobIrq lifetime to generic 'a

Why?

> 
> This patch follows up to [1], which adds support for firmware loading
> and MCU booting to the Tyr driver. The changes included here were
> originally introduced in its v4[2], then dropped to reduce the scope of
> the series, and have been adjusted to work with the HRT (Higher-Ranked
> Lifetime Types) driver architecture recently introduced.
> 
> The patch adds a threaded IRQ wrapper for the firmware Job interrupts,
> used to signal events from the global CSF (GLB) and Command Stream Group
> (CSG) interfaces.
> 
> These changes will be later used to wait for global CSF interface
> readiness after firmware boot, as part of the CSF firmware interfaces
> support that will be submitted as a separate series.
> 
> This patch is based on drm-rust-next and depends on:
> - [PATCH v2] rust: irq: make Registration compatible with lifetime-bound drivers[3]
> - [PATCH 0/5] rust: sync: add WaitQueue infrastructure[4]
> 
> Note: [4] doesn't apply cleanly on drm-rust-next at the moment, due to
> missing changes in rust/kernel/sync/lock/spinlock.rs[5]. I've applied
> all dependencies and fixed conflicts for the purpose of testing this
> patch on top of drm-rust-next, a branch with these changes is available
> here: https://gitlab.freedesktop.org/laura.nao/linux/-/commits/b4/tyr-irq-v5
> 
> [1] https://lore.kernel.org/all/20260728-fw-boot-b4-v10-0-9187aefa3f2f@collabora.com/ 
> [2] https://lore.kernel.org/rust-for-linux/20260424-b4-fw-boot-v4-v4-15-a5d91050789d@collabora.com/
> [3] https://lore.kernel.org/rust-for-linux/20260719153631.559341-1-dakr@kernel.org/
> [4] https://lore.kernel.org/rust-for-linux/20260726223613.1242940-1-dakr@kernel.org/
> [5] https://lore.kernel.org/all/20260302232154.861916-1-lyude@redhat.com/
> ---
> drivers/gpu/drm/tyr/driver.rs | 108 ++++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/tyr/fw.rs     |   1 +
> drivers/gpu/drm/tyr/fw/irq.rs | 112 ++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 221 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tyr/driver.rs b/drivers/gpu/drm/tyr/driver.rs
> index d78ad9d292ff..495cc3baa98c 100644
> --- a/drivers/gpu/drm/tyr/driver.rs
> +++ b/drivers/gpu/drm/tyr/driver.rs
> @@ -1,5 +1,7 @@
> // SPDX-License-Identifier: GPL-2.0 or MIT
> 
> +use core::marker::PhantomPinned;
> +
> use kernel::{
>     clk::{
>         Clk,
> @@ -21,6 +23,13 @@
>         poll,
>         Io, //
>     },
> +    irq::{
> +        Flags,
> +        IrqReturn,
> +        ThreadedHandler,
> +        ThreadedIrqReturn,
> +        ThreadedRegistration, //
> +    },
>     new_mutex,
>     of,
>     platform,
> @@ -236,3 +245,102 @@ struct Regulators {
>     _mali: Regulator<regulator::Enabled>,
>     _sram: Regulator<regulator::Enabled>,
> }
> +
> +/// Trait describing the operations required to service a Tyr interrupt source.
> +pub(crate) trait TyrIrqTrait: Sync {

Can we move this and friends to its own irq.rs file? 

driver.rs is more of platform/device/initialization glue. I think it would be
more tidy if we split this. Additionally, I think TyrIrq, TyrIrqTrait and etc
can be their own patch, separate from the patch that adds support to FW IRQs
specifically. You can use #[expect(dead_code)] in patch 1.

> +    /// Returns the masked interrupt status.
> +    fn read_status(&self) -> u32;
> +
> +    /// Clears all bits in the mask register, disabling the interrupt sources.
> +    fn clear_mask(&self);
> +
> +    /// Sets the mask register to re-enable the interrupt sources.
> +    fn reenable_mask(&self);
> +
> +    /// Returns the raw interrupt status, regardless of the current mask.
> +    fn read_raw_status(&self) -> u32;
> +
> +    /// Clears the given bits in the interrupt status register.
> +    fn clear_status(&self, status: u32);
> +
> +    /// Returns the bitmask of interrupt sources handled by this instance.
> +    fn mask(&self) -> u32;
> +
> +    /// Handles the given pending, unmasked interrupt bits.
> +    fn handle(&self, status: u32);
> +}
> +
> +/// A threaded IRQ wrapper shared by all Tyr interrupt sources.
> +#[pin_data(PinnedDrop)]
> +pub(crate) struct TyrIrq<T: TyrIrqTrait> {
> +    /// The interrupt source.
> +    irq: T,
> +    #[pin]
> +    _pin: PhantomPinned,
> +}
> +
> +impl<T: TyrIrqTrait> TyrIrq<T> {
> +    /// Requests a threaded IRQ registration for `irq`, using `name` to look up the interrupt.
> +    ///
> +    /// # Safety
> +    ///
> +    /// Callers must not `mem::forget()` the resulting registration or otherwise prevent its
> +    /// [`Drop`] implementation from running.
> +    pub(crate) unsafe fn request<'a>(
> +        pdev: &'a platform::Device<Bound>,
> +        name: &'static CStr,
> +        irq: T,
> +    ) -> Result<impl PinInit<ThreadedRegistration<'a, Self>, Error> + 'a>
> +    where
> +        T: 'a,
> +    {

Instead of 'a, I think we should converge on the 'bound nomenclature. Is there
a reason why we are using 'a here instead? Same for JobIrq<‘a> and job_irq_init.


> +        let handler = try_pin_init!(Self {
> +            irq,
> +            _pin: PhantomPinned,
> +        });
> +
> +        // SAFETY: The caller guarantees the resulting registration will not be leaked.
> +        Ok(unsafe { pdev.request_threaded_irq_by_name(Flags::SHARED, name, name, handler) })
> +    }
> +
> +    /// Returns a reference to the wrapped interrupt source.
> +    pub(crate) fn irq(&self) -> &T {
> +        &self.irq
> +    }

The only caller seems to be pin_chain in job_irq_init()? I think we can remove
this accessor and just access the field directly in request().

> +}
> +
> +impl<T: TyrIrqTrait> ThreadedHandler for TyrIrq<T> {
> +    fn handle(&self) -> ThreadedIrqReturn {
> +        let masked_status = self.irq.read_status();
> +
> +        if masked_status == 0 {
> +            return ThreadedIrqReturn::None;
> +        }
> +        self.irq.clear_mask();
> +        ThreadedIrqReturn::WakeThread
> +    }
> +
> +    fn handle_threaded(&self) -> IrqReturn {
> +        let mut ret = IrqReturn::None;
> +
> +        loop {
> +            let raw_status = self.irq.read_raw_status() & self.irq.mask();
> +            if raw_status == 0 {
> +                break;
> +            }
> +            self.irq.clear_status(raw_status);
> +            self.irq.handle(raw_status);
> +            ret = IrqReturn::Handled;
> +        }
> +
> +        self.irq.reenable_mask();
> +        ret
> +    }
> +}
> +
> +#[pinned_drop]
> +impl<T: TyrIrqTrait> PinnedDrop for TyrIrq<T> {
> +    fn drop(self: Pin<&mut Self>) {
> +        self.irq.clear_mask();
> +    }

In Rust, the drop order is first a struct then its fields, and handler is a
field of ThreadedRegistration. By the time this drop runs, free_irq() has
already run. Ideally we'd write this in the opposite order.

Perhaps what would work is moving this drop into a separate type:

_job_irq_mask: JobIrqMaskGuard<'bound>,
job_irq: Pin<KBox<ThreadedRegistration<...>>>

Because within a struct, fields drop in their declaration order, so first all
firmware interrupts are masked, then free_irq() waits for the in-flight
interrupts before deregistering.

> +}
> diff --git a/drivers/gpu/drm/tyr/fw.rs b/drivers/gpu/drm/tyr/fw.rs
> index 47d25c901bd0..8162b9806c97 100644
> --- a/drivers/gpu/drm/tyr/fw.rs
> +++ b/drivers/gpu/drm/tyr/fw.rs
> @@ -69,6 +69,7 @@
>     vm::Vm, //
> };
> 
> +pub(crate) mod irq;
> mod parser;
> 
> pub(super) const CSF_MCU_SHARED_REGION_START: u32 = 0x04000000;
> diff --git a/drivers/gpu/drm/tyr/fw/irq.rs b/drivers/gpu/drm/tyr/fw/irq.rs
> new file mode 100644
> index 000000000000..fcdc0097f2dd
> --- /dev/null
> +++ b/drivers/gpu/drm/tyr/fw/irq.rs
> @@ -0,0 +1,112 @@
> +// SPDX-License-Identifier: GPL-2.0 or MIT
> +
> +//! IRQ handling for the Job IRQ.
> +//!
> +//! The Job IRQ signals events from the MCU, including global interface acknowledgements.
> +#![allow(dead_code)]
> +
> +use core::sync::atomic::{
> +    AtomicBool,

kernel::sync::atomic::Atomic<bool>

> +    Ordering, //
> +};
> +
> +use kernel::{
> +    device::Bound, //
> +    io::Io,
> +    irq::ThreadedRegistration,
> +    platform,
> +    prelude::*,
> +    sync::{
> +        Arc,
> +        WaitQueue, //
> +    },
> +};
> +
> +use crate::{
> +    driver::{
> +        IoMem,
> +        TyrIrq,
> +        TyrIrqTrait, //
> +    },
> +    regs::job_control::{
> +        JOB_IRQ_CLEAR,
> +        JOB_IRQ_MASK,
> +        JOB_IRQ_RAWSTAT,
> +        JOB_IRQ_STATUS, //
> +    }, //
> +};
> +
> +// The Job IRQ, signalling requests or notification from the MCU.
> +pub(crate) struct JobIrq<'a> {
> +    /// GPU MMIO register mapping.
> +    iomem: Arc<IoMem<'a>>,
> +    /// Flag to signal firmware readiness.
> +    fw_ready: Arc<AtomicBool>,

kernel::sync::atomic::Atomic<bool>

> +    /// Wait queue used to signal firmware events to waiters.
> +    job_irq_wait: Arc<WaitQueue>,
> +}


I think I'd rather have a single type and a single Arc here, e.g.:

    /// The firmware events published by the Job IRQ handler, and the queue used
    /// to announce them.
    #[pin_data]
    pub(crate) struct JobIrqEvents {
        #[pin]
        wait: WaitQueue,
        /// Set once the firmware has signalled that the global interface is ready.
        ready: Atomic<bool>
    }

    impl JobIrqEvents {
        pub(crate) fn new() -> Result<Arc<Self>> {
            Arc::pin_init(
                pin_init!(JobIrqEvents {
                    wait <- new_waitqueue!(),
                    ready: AtomicBool::new(false),
                }),
                GFP_KERNEL,
            )
        }

        pub(crate) fn clear_ready(&self) {
            self.ready.store(false, Ordering::Relaxed);
        }

        pub(crate) fn wait_ready(&self, timeout_ms: Msecs) -> Result {
            self.wait.wait_event_timeout(
                || self.ready.load(Ordering::Acquire),
                msecs_to_jiffies(timeout_ms),
            )?;

            Ok(())
        }

        fn signal(&self, status: u32) { ... }
    }

> +
> +/// Requests a threaded IRQ registration for the Job IRQ.
> +///
> +/// # Safety
> +///
> +/// Callers must not `mem::forget()` the resulting registration or otherwise prevent its
> +/// [`Drop`] implementation from running.
> +pub(crate) unsafe fn job_irq_init<'a>(
> +    pdev: &'a platform::Device<Bound>,
> +    iomem: Arc<IoMem<'a>>,
> +    fw_ready: Arc<AtomicBool>,
> +    job_irq_wait: Arc<WaitQueue>,
> +) -> Result<impl PinInit<ThreadedRegistration<'a, TyrIrq<JobIrq<'a>>>, Error> + 'a> {
> +    let job_irq = JobIrq {
> +        iomem,
> +        fw_ready,
> +        job_irq_wait,
> +    };
> +    // Ensure interrupt sources are disabled until the handler is registered.
> +    job_irq.clear_mask();

IIRC panthor also clears the status here?

> +
> +    // SAFETY: The caller guarantees the resulting registration will not be leaked.
> +    let registration = unsafe { TyrIrq::request(pdev, c"job", job_irq) };
> +
> +    // Unmask once the handler has been successfully registered.
> +    Ok(registration?.pin_chain(|reg| {
> +        reg.handler().irq().reenable_mask();
> +        Ok(())
> +    }))
> +}
> +
> +impl TyrIrqTrait for JobIrq<'_> {
> +    fn read_status(&self) -> u32 {
> +        self.iomem.read(JOB_IRQ_STATUS).into_raw()
> +    }
> +
> +    fn clear_mask(&self) {
> +        self.iomem.write_reg(JOB_IRQ_MASK::zeroed());
> +    }
> +
> +    fn reenable_mask(&self) {
> +        self.iomem.write_reg(JOB_IRQ_MASK::zeroed().with_glb(true));
> +    }

Can this use mask() internally, please? :)

> +
> +    fn read_raw_status(&self) -> u32 {
> +        self.iomem.read(JOB_IRQ_RAWSTAT).into_raw()
> +    }
> +
> +    fn clear_status(&self, status: u32) {
> +        self.iomem.write_reg(JOB_IRQ_CLEAR::from_raw(status));
> +    }
> +
> +    fn mask(&self) -> u32 {
> +        JOB_IRQ_MASK::zeroed().with_glb(true).into_raw()
> +    }
> +
> +    fn handle(&self, status: u32) {
> +        // TODO: handle other Job IRQ events (e.g. CSG attention bits) here once
> +        // support for them is added.
> +        if JOB_IRQ_RAWSTAT::from_raw(status).glb() {
> +            self.fw_ready.store(true, Ordering::Release);

We only need to store this once. In fact, the “firmware is now ready”
meaning only applies during power up from the hardware’s perspective.


> +            self.job_irq_wait.wake_up_all();

We need to wake up waiters unconditionally. Not a problem now,
but this will be a problem in the future.

> +        }
> +    }
> +}
> 
> ---
> base-commit: 0710742e26a06c2c3167e77a8ea82b59632d9f7a
> change-id: 20260728-tyr-irq-v2-0b3c5022be33
> 
> Best regards,
> -- 
> Laura Nao <laura.nao@collabora.com>
> 
> 

Currently, the firmware booting code will poll for “fw_ready”. Can you
replace that with the JobIrqEvents logic you are introducing above?

— Daniel

Re: [PATCH v5] drm/tyr: add Job IRQ handling

[Danilo Krummrich]

On Thu Aug 13, 2026 at 3:22 AM CEST, Daniel Almeida wrote:
>> - Renamed JobIrq lifetime to generic 'a
>
> Why?

Please see [1].

[1] https://lore.kernel.org/all/DKAINVQDNE79.3JRKZDQJCSX9@kernel.org/