[PATCH v6 0/2] drm/tyr: add Job IRQ handling

[PATCH v6 0/2] drm/tyr: add Job IRQ handling

[Laura Nao]

Changes in v6:
- Split changes into two separate commits: the first one creates a new 
  file with TyrIrq/TyrIrqTrait and their related impls, the second 
  commit imports and uses TyrIrq/TyrIrqTrait from crate::irq instead of
  crate::driver
- Removed irq() accessor, unmasking the interrupts in TyrIrq::request() 
  by accessing the private irq field directly instead
- Add JobIrqMaskGuard to mask the Job IRQ before free_irq() runs, fixing 
  incorrect drop order in the IRQ teardown path 
- Combined fw_ready/job_irq_wait into a separate JobIrqEvents type
- Store firmware-ready state only once and always wake waiters 
  unconditionally
- Clear the status once the handler has been successfully registered
- Use mask() in reenable_mask() to reduce code duplication
- Reworded commit messages to reflect the changes
- Link to v5: https://lore.kernel.org/r/20260728-tyr-irq-v2-v5-1-59fd9dea952a@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 series 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 series 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 series 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-v6

[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/

Signed-off-by: Laura Nao <laura.nao@collabora.com>
---
Laura Nao (2):
      drm/tyr: add TyrIrq threaded IRQ wrapper
      drm/tyr: add Job IRQ handling

 drivers/gpu/drm/tyr/fw.rs     |   1 +
 drivers/gpu/drm/tyr/fw/irq.rs | 174 ++++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/tyr/irq.rs    | 114 +++++++++++++++++++++++++++
 drivers/gpu/drm/tyr/tyr.rs    |   1 +
 4 files changed, 290 insertions(+)
---
base-commit: d7962d990307dc28dfded8cf6ba96d7498af7631
change-id: 20260728-tyr-irq-v2-0b3c5022be33

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

[PATCH v6 1/2] drm/tyr: add TyrIrq threaded IRQ wrapper

[Laura Nao]

Introduce a generic TyrIrq threaded IRQ wrapper along with the
TyrIrqTrait trait describing the operations required to handle a
Tyr interrupt source.

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>
---
 drivers/gpu/drm/tyr/irq.rs | 115 +++++++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/tyr/tyr.rs |   1 +
 2 files changed, 116 insertions(+)

diff --git a/drivers/gpu/drm/tyr/irq.rs b/drivers/gpu/drm/tyr/irq.rs
new file mode 100644
index 000000000000..e6a739532daa
--- /dev/null
+++ b/drivers/gpu/drm/tyr/irq.rs
@@ -0,0 +1,115 @@
+// SPDX-License-Identifier: GPL-2.0 or MIT
+
+//! Threaded IRQ wrapper shared by all Tyr interrupt sources.
+#![allow(dead_code)]
+use core::marker::PhantomPinned;
+
+use kernel::{
+    device::Bound,
+    irq::{
+        Flags,
+        IrqReturn,
+        ThreadedHandler,
+        ThreadedIrqReturn,
+        ThreadedRegistration, //
+    },
+    platform,
+    prelude::*, //
+};
+
+/// 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]
+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.
+        let registration =
+            unsafe { pdev.request_threaded_irq_by_name(Flags::SHARED, name, name, handler) };
+
+        // Clear the status and unmask once the handler has been
+        // successfully registered.
+        Ok(registration.pin_chain(|reg| {
+            let irq = &reg.handler().irq;
+            irq.clear_status(irq.mask());
+            irq.reenable_mask();
+            Ok(())
+        }))
+    }
+}
+
+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
+    }
+}
diff --git a/drivers/gpu/drm/tyr/tyr.rs b/drivers/gpu/drm/tyr/tyr.rs
index e7ec450bdc9c..31205e3bf0e7 100644
--- a/drivers/gpu/drm/tyr/tyr.rs
+++ b/drivers/gpu/drm/tyr/tyr.rs
@@ -12,6 +12,7 @@
 mod fw;
 mod gem;
 mod gpu;
+mod irq;
 mod mmu;
 mod regs;
 mod slot;

-- 
2.39.5

[PATCH v6 2/2] drm/tyr: add Job IRQ handling

[Laura Nao]

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. The Job IRQ handler masks the
interrupt in the primary IRQ handler, processes pending raw status in
the threaded handler, clears the handled bits, and reenables the mask
before returning.
Add JobIrqEvents to hold the wait queue and the ready flag used to
signal firmware readiness when the GLB bit is set, and JobIrqMaskGuard
to ensure the Job IRQ is masked before its registration is freed.

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>
---
 drivers/gpu/drm/tyr/fw.rs     |   1 +
 drivers/gpu/drm/tyr/fw/irq.rs | 174 ++++++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/tyr/irq.rs    |   1 -
 3 files changed, 175 insertions(+), 1 deletion(-)

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..7dd894de18cb
--- /dev/null
+++ b/drivers/gpu/drm/tyr/fw/irq.rs
@@ -0,0 +1,174 @@
+// 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 kernel::{
+    device::Bound, //
+    io::Io,
+    irq::ThreadedRegistration,
+    new_waitqueue,
+    platform,
+    prelude::*,
+    sync::{
+        atomic::{
+            ordering,
+            Atomic, //
+        },
+        Arc,
+        WaitQueue, //
+    },
+    time::{
+        msecs_to_jiffies,
+        Msecs, //
+    },
+};
+
+use crate::{
+    driver::IoMem,
+    irq::{
+        TyrIrq,
+        TyrIrqTrait, //
+    },
+    regs::job_control::{
+        JOB_IRQ_CLEAR,
+        JOB_IRQ_MASK,
+        JOB_IRQ_RAWSTAT,
+        JOB_IRQ_STATUS, //
+    }, //
+};
+
+/// 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 {
+    /// Creates a new, empty set of Job IRQ events.
+    pub(crate) fn new() -> Result<Arc<Self>> {
+        Arc::pin_init(
+            pin_init!(JobIrqEvents {
+                wait <- new_waitqueue!(),
+                ready: Atomic::new(false),
+            }),
+            GFP_KERNEL,
+        )
+    }
+
+    /// Clears the firmware-ready state.
+    pub(crate) fn clear_ready(&self) {
+        self.ready.store(false, ordering::Relaxed);
+    }
+
+    /// Waits until the firmware signals readiness via the GLB IRQ bit, or the timeout expires.
+    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(())
+    }
+
+    /// Updates state and wakes up waiters
+    fn signal(&self, status: u32) {
+        // TODO: handle other Job IRQ events (e.g. CSG attention bits) here once
+        // support for them is added.
+
+        // The GLB bit only signals firmware readiness once, at power up
+        if JOB_IRQ_RAWSTAT::from_raw(status).glb() && !self.ready.load(ordering::Relaxed) {
+            self.ready.store(true, ordering::Release);
+        }
+
+        self.wait.wake_up_all();
+    }
+}
+
+// The Job IRQ, signalling requests or notification from the MCU.
+pub(crate) struct JobIrq<'a> {
+    /// GPU MMIO register mapping.
+    iomem: Arc<IoMem<'a>>,
+    /// Firmware events signalled by this IRQ.
+    events: Arc<JobIrqEvents>,
+}
+
+/// Guard that masks the Job IRQ when dropped.
+///
+/// To mask the Job IRQ before it is freed, this guard must be stored in a field declared
+/// before the corresponding `ThreadedRegistration` in the struct that owns both. Since
+/// struct fields are dropped in declaration order, this guarantees the Job IRQ is masked
+/// first, and only then does `free_irq()` run and wait for any in-flight handler to
+/// complete.
+pub(crate) struct JobIrqMaskGuard<'a>(Arc<IoMem<'a>>);
+
+impl Drop for JobIrqMaskGuard<'_> {
+    fn drop(&mut self) {
+        self.0.write_reg(JOB_IRQ_MASK::zeroed());
+    }
+}
+
+/// Requests a threaded IRQ registration for the Job IRQ.
+///
+/// Returns a [`JobIrqMaskGuard`] alongside the registration. The guard must be stored in a
+/// field declared before the registration in the struct that owns both, so that the Job
+/// IRQ is masked before it is freed. See [`JobIrqMaskGuard`] for details.
+///
+/// # 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>>,
+    events: Arc<JobIrqEvents>,
+) -> Result<(
+    JobIrqMaskGuard<'a>,
+    impl PinInit<ThreadedRegistration<'a, TyrIrq<JobIrq<'a>>>, Error> + 'a,
+)> {
+    let mask_guard = JobIrqMaskGuard(iomem.clone());
+
+    let job_irq = JobIrq { iomem, events };
+    // 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) }?;
+
+    Ok((mask_guard, registration))
+}
+
+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::from_raw(self.mask()));
+    }
+
+    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) {
+        self.events.signal(status);
+    }
+}
diff --git a/drivers/gpu/drm/tyr/irq.rs b/drivers/gpu/drm/tyr/irq.rs
index e6a739532daa..1905fd251ead 100644
--- a/drivers/gpu/drm/tyr/irq.rs
+++ b/drivers/gpu/drm/tyr/irq.rs
@@ -1,7 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0 or MIT
 
 //! Threaded IRQ wrapper shared by all Tyr interrupt sources.
-#![allow(dead_code)]
 use core::marker::PhantomPinned;
 
 use kernel::{

-- 
2.39.5

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

[Alice Ryhl]

On Thu, Aug 27, 2026 at 10:01:11AM +0200, Laura Nao wrote:
> 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. The Job IRQ handler masks the
> interrupt in the primary IRQ handler, processes pending raw status in
> the threaded handler, clears the handled bits, and reenables the mask
> before returning.
> Add JobIrqEvents to hold the wait queue and the ready flag used to
> signal firmware readiness when the GLB bit is set, and JobIrqMaskGuard
> to ensure the Job IRQ is masked before its registration is freed.
> 
> 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>
> ---
>  drivers/gpu/drm/tyr/fw.rs     |   1 +
>  drivers/gpu/drm/tyr/fw/irq.rs | 174 ++++++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/tyr/irq.rs    |   1 -
>  3 files changed, 175 insertions(+), 1 deletion(-)
> 
> 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..7dd894de18cb
> --- /dev/null
> +++ b/drivers/gpu/drm/tyr/fw/irq.rs
> @@ -0,0 +1,174 @@
> +// 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 kernel::{
> +    device::Bound, //
> +    io::Io,
> +    irq::ThreadedRegistration,
> +    new_waitqueue,
> +    platform,
> +    prelude::*,
> +    sync::{
> +        atomic::{
> +            ordering,
> +            Atomic, //
> +        },
> +        Arc,
> +        WaitQueue, //
> +    },
> +    time::{
> +        msecs_to_jiffies,
> +        Msecs, //
> +    },
> +};
> +
> +use crate::{
> +    driver::IoMem,
> +    irq::{
> +        TyrIrq,
> +        TyrIrqTrait, //
> +    },
> +    regs::job_control::{
> +        JOB_IRQ_CLEAR,
> +        JOB_IRQ_MASK,
> +        JOB_IRQ_RAWSTAT,
> +        JOB_IRQ_STATUS, //
> +    }, //
> +};
> +
> +/// 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 {
> +    /// Creates a new, empty set of Job IRQ events.
> +    pub(crate) fn new() -> Result<Arc<Self>> {
> +        Arc::pin_init(
> +            pin_init!(JobIrqEvents {
> +                wait <- new_waitqueue!(),
> +                ready: Atomic::new(false),
> +            }),
> +            GFP_KERNEL,
> +        )
> +    }
> +
> +    /// Clears the firmware-ready state.
> +    pub(crate) fn clear_ready(&self) {
> +        self.ready.store(false, ordering::Relaxed);
> +    }
> +
> +    /// Waits until the firmware signals readiness via the GLB IRQ bit, or the timeout expires.
> +    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(())
> +    }
> +
> +    /// Updates state and wakes up waiters
> +    fn signal(&self, status: u32) {
> +        // TODO: handle other Job IRQ events (e.g. CSG attention bits) here once
> +        // support for them is added.
> +
> +        // The GLB bit only signals firmware readiness once, at power up
> +        if JOB_IRQ_RAWSTAT::from_raw(status).glb() && !self.ready.load(ordering::Relaxed) {
> +            self.ready.store(true, ordering::Release);
> +        }
> +
> +        self.wait.wake_up_all();
> +    }
> +}
> +
> +// The Job IRQ, signalling requests or notification from the MCU.
> +pub(crate) struct JobIrq<'a> {
> +    /// GPU MMIO register mapping.
> +    iomem: Arc<IoMem<'a>>,
> +    /// Firmware events signalled by this IRQ.
> +    events: Arc<JobIrqEvents>,
> +}
> +
> +/// Guard that masks the Job IRQ when dropped.
> +///
> +/// To mask the Job IRQ before it is freed, this guard must be stored in a field declared
> +/// before the corresponding `ThreadedRegistration` in the struct that owns both. Since
> +/// struct fields are dropped in declaration order, this guarantees the Job IRQ is masked
> +/// first, and only then does `free_irq()` run and wait for any in-flight handler to
> +/// complete.
> +pub(crate) struct JobIrqMaskGuard<'a>(Arc<IoMem<'a>>);
> +
> +impl Drop for JobIrqMaskGuard<'_> {
> +    fn drop(&mut self) {
> +        self.0.write_reg(JOB_IRQ_MASK::zeroed());
> +    }
> +}

I agree with sashiko's review here.

This needs to happen after the free_irq() call in the destructor of
TyrIrq. And most likely, we need an atomic along the lines of ACTIVE /
PROCESSING / SUSPENDING in panthor_irq.

Alice

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

[Laura Nao]

Hi Alice,

On 9/23/26 10:37, Alice Ryhl wrote:
> I agree with sashiko's review here.
>
> This needs to happen after the free_irq() call in the destructor of
> TyrIrq.

Thanks for the feedback.

In v5, I was masking the interrupts in TyrIrq's PinnedDrop impl which
resulted in clear_mask() being called after free_irq(). However, sashiko
warned the device could then assert irqs after free_irq() completes
but before clear_mask() runs, potentially leading to an IRQ storm that
could result in the shared line being permanently disabled. 

So calling clear_mask() before free_irq() should avoid this, but we
still have to deal with threaded handlers potentially re-enabling the
mask after clear_mask() has run (as per sashiko's review on this current
revision). Adding the atomic state should help with that though, as you
suggested:

> And most likely, we need an atomic along the lines of ACTIVE /
> PROCESSING / SUSPENDING in panthor_irq.
>

So I'm thinking something like this for the state:

#[derive(Clone, Copy, PartialEq, Eq)]
#[repr(i32)]
enum IrqState {
    Active = 0,
    Processing,
    Unregistering,
}

unsafe impl AtomicType for IrqState {
    type Repr = i32;
}

This could then be stored in TyrIrq:

#[pin_data]
pub(crate) struct TyrIrq<T: TyrIrqTrait> {
    irq: T,
    state: Arc<Atomic<IrqState>>,
    #[pin]
    _pin: PhantomPinned,
}

Then TyrIrq::handle() proceeds only when the state is active and
TyrIrq::handle_threaded() re-enables the mask only if the state is not
`Unregistering`.

Does this make sense to you?

As for masking before free_irq() runs, I'm thinking of possible
alternatives to JobIrqMaskGuard and its ordering convention (i.e. must
be stored in a field declared before the corresponding
`ThreadedRegistration` in the struct that owns both). Would it make
sense to define a TyrIrqRegistration struct that wraps
ThreadedRegistration instead? and then clear the mask in
TyrIrqRegistration's drop impl.

Something like:

#[pin_data(PinnedDrop)]
pub(crate) struct TyrIrqRegistration<'a, T: TyrIrqTrait> {
    #[pin]
    registration: ThreadedRegistration<'a, TyrIrq<T>>,
}

#[pinned_drop]
impl<T: TyrIrqTrait> PinnedDrop for TyrIrqRegistration<'_, T> {
    fn drop(self: Pin<&mut Self>) {
        let handler = self.registration.handler();
        handler.state.store(IrqState::Unregistering, Release);
        handler.irq.clear_mask();
    }
}

This should make sure teardown order is still respected without relying
on the user correctly putting the guard before `ThreadedRegistration`.

Any thoughts on this approach? In case it helps as a reference, I've
drafted both IrqState and TyrIrqRegistration in [1].

[1] https://gitlab.freedesktop.org/laura.nao/linux/-/commit/c78e296690e643b790fb3195660f1229e3cdb5ca

Best,

Laura

Re: [PATCH v6 0/2] drm/tyr: add Job IRQ handling

[Daniel Almeida]

Hi Laura,

It makes more sense to have Onur's reset v7 land first, so the Job IRQ series
needs a rebase on top of it:

https://lore.kernel.org/r/20260912-tyr-reset-impl-v7-0-077ce72084eb@onurozkan.dev

After his 3/3, the registers are only reachable through HwGate, whose
access() takes a mutex. I added try_access_atomic() for the hard handler
on top of v7 [0]. Can you rebase onto that and carry the patch at the
front of your series? Then:

a) hard handler: try_access_atomic(), return ThreadedIrqReturn::None on None
b) threaded handler: access()
c) drop the #[expect(dead_code)]

— Daniel

[0] https://gitlab.collabora.com/dwlsalmeida/for-upstream/-/tree/tyr-hwgate-try-atomic?ref_type=heads

Re: [PATCH v6 0/2] drm/tyr: add Job IRQ handling

[Laura Nao]

Hi Daniel,

On 9/25/26 21:29, Daniel Almeida wrote:
> Hi Laura,
>
> It makes more sense to have Onur's reset v7 land first, so the Job IRQ series
> needs a rebase on top of it:
>
> https://lore.kernel.org/r/20260912-tyr-reset-impl-v7-0-077ce72084eb@onurozkan.dev
>
> After his 3/3, the registers are only reachable through HwGate, whose
> access() takes a mutex. I added try_access_atomic() for the hard handler
> on top of v7 [0]. Can you rebase onto that and carry the patch at the
> front of your series? Then:
>
> a) hard handler: try_access_atomic(), return ThreadedIrqReturn::None on None
> b) threaded handler: access()
> c) drop the #[expect(dead_code)]
>
> — Daniel
>
> [0] https://gitlab.collabora.com/dwlsalmeida/for-upstream/-/tree/tyr-hwgate-try-atomic?ref_type=heads

Ack, thanks for the pointer. I'll make sure to address this for v7.

Best,

Laura