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

[Alice Ryhl <aliceryhl@google.com>]

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