Re: [PATCH v6 2/7] drm/tyr: add a generic slot manager

[Daniel Almeida <daniel.almeida@collabora.com>]

> On 9 Jul 2026, at 18:36, Deborah Brouwer <deborah.brouwer@collabora.com> wrote:
> 
> From: Boris Brezillon <boris.brezillon@collabora.com>
> 
> Introduce a generic slot manager to dynamically allocate limited hardware
> slots to software "seats". It can be used for both address space (AS) and
> command stream group (CSG) slots.
> 
> The slot manager initially assigns seats to its free slots. It will
> continue to reuse the same slot for a seat, as long as another seat does
> not start to use the slot in the interim.
> 
> When contention arises because all of the slots are allocated, the slot
> manager will lazily evict and reuse slots that have become idle (if any).
> 
> The seat state is protected using the LockedBy pattern with the same lock
> that guards the SlotManager. This ensures the seat state stays consistent
> across slot operations.
> 
> Hardware specific behaviour is controlled through the SlotManager's
> specific manager type that implements the `SlotOperations` trait.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@collabora.com>
> Co-developed-by: Deborah Brouwer <deborah.brouwer@collabora.com>
> Signed-off-by: Deborah Brouwer <deborah.brouwer@collabora.com>
> ---
> drivers/gpu/drm/tyr/slot.rs | 385 ++++++++++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/tyr/tyr.rs  |   1 +
> 2 files changed, 386 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tyr/slot.rs b/drivers/gpu/drm/tyr/slot.rs
> new file mode 100644
> index 000000000000..3f58b23238ef
> --- /dev/null
> +++ b/drivers/gpu/drm/tyr/slot.rs
> @@ -0,0 +1,385 @@
> +// SPDX-License-Identifier: GPL-2.0 or MIT
> +
> +//! Slot management abstraction for limited hardware resources.
> +//!
> +//! This module provides a generic [`SlotManager`] that assigns limited hardware
> +//! slots to logical "seats". A seat represents an entity (such as a virtual memory
> +//! (VM) address space) that needs access to a hardware slot.
> +//!
> +//! The [`SlotManager`] tracks slot allocation using sequence numbers (seqno) to detect
> +//! when a seat's binding has been invalidated. When a seat requests activation,
> +//! the manager will either reuse the seat's existing slot (if still valid),
> +//! allocate a free slot (if any are available), or evict the oldest idle slot if any
> +//! slots are idle.
> +//!
> +//! Hardware-specific behavior is customized by implementing the [`SlotOperations`]
> +//! trait, which allows callbacks when slots are activated or evicted.
> +//!
> +//! This is currently used for managing address space slots in the GPU, and it will
> +//! also be used to manage Command Stream Group (CSG) interface slots in the future.
> +//!
> +//! [SlotOperations]: crate::slot::SlotOperations
> +//! [SlotManager]: crate::slot::SlotManager
> +#![allow(dead_code)]
> +
> +use core::{
> +    mem::take,
> +    ops::{
> +        Deref,
> +        DerefMut, //
> +    }, //
> +};
> +
> +use kernel::{
> +    prelude::*,
> +    sync::LockedBy, //
> +};
> +
> +/// Seat information.
> +///
> +/// This can't be accessed directly by the element embedding a `Seat`,
> +/// but is used by the generic slot manager logic to control residency
> +/// of a certain object on a hardware slot.
> +pub(crate) struct SeatInfo {
> +    /// Slot used by this seat.
> +    ///
> +    /// This index is only valid if the slot pointed to by this index
> +    /// has its `SlotInfo::seqno` match `SeatInfo::seqno`. Otherwise,
> +    /// it means the object has been evicted from the hardware slot,
> +    /// and a new slot needs to be acquired to make this object
> +    /// resident again.
> +    slot: u8,
> +
> +    /// Sequence number encoding the last time this seat was active.
> +    /// We also use it to check if a slot is still bound to a seat.
> +    seqno: u64,
> +}
> +
> +/// Seat state.
> +///
> +/// This is meant to be embedded in the object that wants to acquire
> +/// hardware slots. It also starts in the `Seat::NoSeat` state, and
> +/// the slot manager will change the object value when an active/evict
> +/// request is issued.
> +#[derive(Default)]
> +pub(crate) enum Seat {
> +    #[expect(clippy::enum_variant_names)]
> +    /// Resource is not resident.
> +    ///
> +    /// All objects start with a seat in the `Seat::NoSeat` state. The seat also
> +    /// gets back to that state if the user requests eviction. It
> +    /// can also end up in that state next time an operation is done
> +    /// on a `Seat::Idle` seat and the slot manager finds out this
> +    /// object has been evicted from the slot.
> +    #[default]
> +    NoSeat,
> +
> +    /// Resource is actively used and resident.
> +    ///
> +    /// When a seat is in the `Seat::Active` state, it can't be evicted, and the
> +    /// slot pointed to by `SeatInfo::slot` is guaranteed to be reserved
> +    /// for this object as long as the seat stays active.
> +    Active(SeatInfo),
> +
> +    /// Resource is idle and might or might not be resident.
> +    ///
> +    /// When a seat is in the`Seat::Idle` state, we can't know for sure if the
> +    /// object is resident or evicted until the next request we issue
> +    /// to the slot manager. This tells the slot manager it can
> +    /// reclaim the underlying slot if needed.
> +    /// In order for the hardware to use this object again, the seat
> +    /// needs to be turned into an `Seat::Active` state again
> +    /// with a `SlotManager::activate()` call.
> +    Idle(SeatInfo),
> +}
> +
> +impl Seat {
> +    /// Get the slot index this seat is pointing to.
> +    ///
> +    /// If the seat is not `Seat::Active` we can't trust the
> +    /// `SeatInfo`. In that case `None` is returned, otherwise
> +    /// `Some(SeatInfo::slot)` is returned.
> +    pub(super) fn slot(&self) -> Option<u8> {

Can we use on pub(crate) instead of pub(super)?

> +        match self {
> +            Self::Active(info) => Some(info.slot),
> +            _ => None,
> +        }
> +    }
> +}
> +
> +/// Information related to a slot.
> +struct SlotInfo<T> {
> +    /// Type specific data attached to a slot.
> +    slot_data: T,
> +
> +    /// Sequence number from when this slot was last activated.
> +    seqno: u64,
> +}
> +
> +/// Slot state.
> +#[derive(Default)]
> +enum SlotState<T> {
> +    /// Slot is free.
> +    #[default]
> +    Free,
> +
> +    /// Slot is active.
> +    Active(SlotInfo<T>),
> +
> +    /// Slot is idle.
> +    Idle(SlotInfo<T>),
> +}
> +
> +/// Trait describing the slot-related operations.
> +pub(crate) trait SlotOperations {
> +    /// Implementation-specific data associated with each slot.
> +    type SlotData;
> +
> +    /// Called when a slot is being activated for a seat.
> +    fn activate(&mut self, _slot_idx: usize, _slot_data: &Self::SlotData) -> Result {
> +        Ok(())
> +    }
> +
> +    /// Called when a slot is being evicted and freed.
> +    fn evict(&mut self, _slot_idx: usize, _slot_data: &Self::SlotData) -> Result {
> +        Ok(())
> +    }
> +}
> +
> +/// A generic slot manager that provides access to a limited number of hardware slots.
> +pub(crate) struct SlotManager<T: SlotOperations, const MAX_SLOTS: usize> {
> +    /// A specific implementation of the generic slot manager.
> +    manager: T,
> +
> +    /// Number of slots actually available.
> +    slot_count: usize,
> +
> +    /// Slot array used to track the state of each slot.
> +    slots: [SlotState<T::SlotData>; MAX_SLOTS],
> +
> +    /// Sequence number incremented each time a Seat is successfully activated
> +    use_seqno: u64,
> +}
> +
> +/// A `Seat` that can only be accessed when holding a `SlotManager` lock guard.
> +type LockedSeat<T, const MAX_SLOTS: usize> = LockedBy<Seat, SlotManager<T, MAX_SLOTS>>;
> +
> +impl<T: SlotOperations, const MAX_SLOTS: usize> SlotManager<T, MAX_SLOTS> {
> +    /// Creates a specific instance of a slot manager.
> +    pub(crate) fn new(manager: T, slot_count: usize) -> Result<Self> {
> +        if slot_count == 0 {
> +            pr_err!("Invalid slot count: 0");
> +            return Err(EINVAL);
> +        }
> +        if slot_count > MAX_SLOTS {
> +            pr_err!(
> +                "Slot count: {} cannot exceed MAX_SLOTS: {}",
> +                slot_count,
> +                MAX_SLOTS
> +            );
> +            return Err(EINVAL);
> +        }
> +        Ok(Self {
> +            manager,
> +            slot_count,
> +            slots: [const { SlotState::Free }; MAX_SLOTS],
> +            use_seqno: 1,
> +        })
> +    }
> +
> +    /// Records a slot as active for the given seat.
> +    fn record_active_slot(
> +        &mut self,
> +        slot_idx: usize,
> +        locked_seat: &LockedSeat<T, MAX_SLOTS>,
> +        slot_data: T::SlotData,
> +    ) -> Result {

This looks infallible?

> +        let cur_seqno = self.use_seqno;
> +
> +        *locked_seat.access_mut(self) = Seat::Active(SeatInfo {
> +            slot: slot_idx as u8,
> +            seqno: cur_seqno,
> +        });
> +
> +        self.slots[slot_idx] = SlotState::Active(SlotInfo {
> +            slot_data,
> +            seqno: cur_seqno,
> +        });
> +
> +        self.use_seqno += 1;
> +        Ok(())
> +    }
> +
> +    /// Activates a slot for the given seat.
> +    fn activate_slot(
> +        &mut self,
> +        slot_idx: usize,
> +        locked_seat: &LockedSeat<T, MAX_SLOTS>,
> +        slot_data: T::SlotData,
> +    ) -> Result {
> +        self.manager.activate(slot_idx, &slot_data)?;
> +        self.record_active_slot(slot_idx, locked_seat, slot_data)
> +    }
> +
> +    /// Finds a slot for the given seat. A free slot is preferred, but if none
> +    /// are available, the oldest idle slot is evicted and reused. Otherwise, if
> +    /// there are no free or idle slots, return [`EBUSY`].
> +    fn allocate_slot(
> +        &mut self,
> +        locked_seat: &LockedSeat<T, MAX_SLOTS>,
> +        slot_data: T::SlotData,
> +    ) -> Result {
> +        let slots = &self.slots[..self.slot_count];
> +
> +        let mut idle_slot_idx = None;
> +        let mut idle_slot_seqno: u64 = 0;
> +
> +        for (slot_idx, slot) in slots.iter().enumerate() {
> +            match slot {
> +                SlotState::Free => {
> +                    return self.activate_slot(slot_idx, locked_seat, slot_data);
> +                }
> +                SlotState::Idle(slot_info) => {
> +                    if idle_slot_idx.is_none() || slot_info.seqno < idle_slot_seqno {
> +                        idle_slot_idx = Some(slot_idx);
> +                        idle_slot_seqno = slot_info.seqno;
> +                    }
> +                }
> +                SlotState::Active(_) => (),
> +            }
> +        }
> +
> +        match idle_slot_idx {
> +            Some(slot_idx) => {
> +                // Lazily evict idle slot just before it is reused.
> +                if let SlotState::Idle(slot_info) = &self.slots[slot_idx] {
> +                    self.manager.evict(slot_idx, &slot_info.slot_data)?;
> +                }
> +                self.activate_slot(slot_idx, locked_seat, slot_data)
> +            }
> +            None => {
> +                pr_err!(
> +                    "Slot allocation failed: all {} slots in use\n",
> +                    self.slot_count
> +                );
> +                Err(EBUSY)
> +            }
> +        }
> +    }
> +
> +    /// Converts an active slot and its seat to idle state.
> +    fn idle_slot(&mut self, slot_idx: usize, locked_seat: &LockedSeat<T, MAX_SLOTS>) -> Result {
> +        let slot = take(&mut self.slots[slot_idx]);

I think it’d be clearer to have mem::take here instead of take.

> +
> +        // If the slot was active, make it idle.
> +        if let SlotState::Active(slot_info) = slot {
> +            self.slots[slot_idx] = SlotState::Idle(slot_info);

Is it me or we don’t place the slot back in the array if the slot is
already idle? Looking at the caller, it seems like this can’t happen today,
but still, perhaps we should use a match here as future-proofing

I think that would fit better, specially given the fact that the code below
does use a match.

> +        }
> +
> +        // If the seat was active, make it idle, or keep it idle if it was already idle.
> +        *locked_seat.access_mut(self) = match locked_seat.access(self) {
> +            Seat::Active(seat_info) | Seat::Idle(seat_info) => Seat::Idle(SeatInfo {
> +                slot: seat_info.slot,
> +                seqno: seat_info.seqno,
> +            }),
> +            Seat::NoSeat => Seat::NoSeat,
> +        };

> +        Ok(())
> +    }
> +
> +    /// Evicts an active or idle slot: calls the eviction callback and marks the slot as free
> +    /// and the seat as NoSeat.
> +    fn evict_slot(&mut self, slot_idx: usize, locked_seat: &LockedSeat<T, MAX_SLOTS>) -> Result {
> +        match &self.slots[slot_idx] {
> +            SlotState::Active(slot_info) | SlotState::Idle(slot_info) => {
> +                self.manager.evict(slot_idx, &slot_info.slot_data)?;
> +                take(&mut self.slots[slot_idx]);
> +            }
> +            _ => (),
> +        }
> +
> +        *locked_seat.access_mut(self) = Seat::NoSeat;
> +        Ok(())
> +    }
> +
> +    /// Checks that the seat state matches the slot's state.
> +    /// If they don't match, the seat is stale and is reset to `NoSeat`.
> +    fn check_seat(&mut self, locked_seat: &LockedSeat<T, MAX_SLOTS>) {
> +        let (slot_idx, seat_seqno, is_active) = match locked_seat.access(self) {
> +            Seat::Active(seat_info) => (seat_info.slot as usize, seat_info.seqno, true),
> +            Seat::Idle(seat_info) => (seat_info.slot as usize, seat_info.seqno, false),
> +            _ => return,
> +        };
> +
> +        let valid = if is_active {
> +            !kernel::warn_on!(!matches!(
> +                &self.slots[slot_idx],
> +                SlotState::Active(slot_info) if slot_info.seqno == seat_seqno
> +            ))
> +        } else {
> +            matches!(
> +                &self.slots[slot_idx],
> +                SlotState::Idle(slot_info) if slot_info.seqno == seat_seqno
> +            )
> +        };
> +
> +        if !valid {
> +            *locked_seat.access_mut(self) = Seat::NoSeat;
> +        }
> +    }
> +
> +    /// Activate a resource on any available/reclaimable slot.
> +    pub(crate) fn activate(
> +        &mut self,
> +        locked_seat: &LockedSeat<T, MAX_SLOTS>,
> +        slot_data: T::SlotData,
> +    ) -> Result {
> +        self.check_seat(locked_seat);
> +        match locked_seat.access(self) {
> +            // If a seat still has a valid slot, just reuse the slot and refresh the bookkeeping.
> +            Seat::Active(seat_info) | Seat::Idle(seat_info) => {
> +                self.record_active_slot(seat_info.slot as usize, locked_seat, slot_data)
> +            }
> +            _ => self.allocate_slot(locked_seat, slot_data),
> +        }
> +    }
> +
> +    /// Flag a resource as idle. This method will be used for user VM support.
> +    #[expect(dead_code)]
> +    pub(crate) fn idle(&mut self, locked_seat: &LockedSeat<T, MAX_SLOTS>) -> Result {
> +        self.check_seat(locked_seat);
> +        if let Seat::Active(seat_info) = locked_seat.access(self) {
> +            self.idle_slot(seat_info.slot as usize, locked_seat)?;
> +        }
> +        Ok(())
> +    }
> +
> +    /// Evict a resource from its slot.
> +    pub(crate) fn evict(&mut self, locked_seat: &LockedSeat<T, MAX_SLOTS>) -> Result {
> +        self.check_seat(locked_seat);
> +
> +        match locked_seat.access(self) {
> +            Seat::Active(seat_info) | Seat::Idle(seat_info) => {
> +                let slot_idx = seat_info.slot as usize;
> +                self.evict_slot(slot_idx, locked_seat)?;
> +            }
> +            _ => (),
> +        }
> +
> +        Ok(())
> +    }
> +}
> +
> +impl<T: SlotOperations, const MAX_SLOTS: usize> Deref for SlotManager<T, MAX_SLOTS> {
> +    type Target = T;
> +
> +    fn deref(&self) -> &Self::Target {
> +        &self.manager
> +    }
> +}
> +
> +impl<T: SlotOperations, const MAX_SLOTS: usize> DerefMut for SlotManager<T, MAX_SLOTS> {
> +    fn deref_mut(&mut self) -> &mut Self::Target {
> +        &mut self.manager
> +    }
> +}
> diff --git a/drivers/gpu/drm/tyr/tyr.rs b/drivers/gpu/drm/tyr/tyr.rs
> index 95cda7b0962f..7c9a8063b3b9 100644
> --- a/drivers/gpu/drm/tyr/tyr.rs
> +++ b/drivers/gpu/drm/tyr/tyr.rs
> @@ -12,6 +12,7 @@
> mod gem;
> mod gpu;
> mod regs;
> +mod slot;
> 
> kernel::module_platform_driver! {
>     type: TyrPlatformDriver,
> 
> -- 
> 2.54.0
>