[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <6833E564-ED5B-42F4-A0D9-932521AF18DE@collabora.com>
Date: Tue, 20 Jan 2026 11:32:45 -0300
From: Daniel Almeida <daniel.almeida@...labora.com>
To: Danilo Krummrich <dakr@...nel.org>
Cc: Miguel Ojeda <ojeda@...nel.org>,
Boqun Feng <boqun.feng@...il.com>,
Gary Guo <gary@...yguo.net>,
Björn Roy Baron <bjorn3_gh@...tonmail.com>,
Benno Lossin <lossin@...nel.org>,
Andreas Hindborg <a.hindborg@...nel.org>,
Alice Ryhl <aliceryhl@...gle.com>,
Trevor Gross <tmgross@...ch.edu>,
David Airlie <airlied@...il.com>,
Simona Vetter <simona@...ll.ch>,
rust-for-linux@...r.kernel.org,
linux-kernel@...r.kernel.org,
dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH 4/4] rust: drm: dispatch delayed work items to the private
data
Hi Danilo,
> On 20 Jan 2026, at 08:14, Danilo Krummrich <dakr@...nel.org> wrote:
>
> On Fri Jan 16, 2026 at 1:36 AM CET, Daniel Almeida wrote:
>> +// SAFETY: Our `HasWork<T, ID>` implementation returns a `work_struct` that is
>> +// stored in the `work` field of a `delayed_work` with the same access rules as
>> +// the `work_struct` owing to the bound on `T::Data: HasDelayedWork<Device<T>,
>> +// ID>`, which requires that `T::Data::raw_get_work` return a `work_struct` that
>> +// is inside a `delayed_work`.
>> +unsafe impl<T, const ID: u64> HasDelayedWork<Device<T>, ID> for Device<T>
>> +where
>> + T: drm::Driver,
>> + T::Data: HasDelayedWork<Device<T>, ID>,
>> +{
>> +}
>
> What do you intend to do within work that is queued for a DRM device? I assume
> you have to access bus device resources?
>
> Also, it would be nice to see how this is used in a driver. Can you please add a
> patch for Tyr to the series?
Yes, we need resources from the bus device, as need to access the underlying
mmio range for register reads and writes. We also need to allocate GEM objects
in that path, which requires the drm class device.
I've just pushed a demo with some WIP at [0] for illustration purposes. Apart
from the other cruft in that code, the main point is:
- impl WorkItem<2> for TyrData {
- type Pointer = Arc<Self>;
+ impl WorkItem<2> for crate::driver::TyrInlineData {
+ type Pointer = ARef<TyrDevice>;
1) The data is now defined inline with the drm::Device. This is used by the
work_container_of implementation in commit #2 in this series.
2) This inline data now has the work and an Arc<TyrData> to keep the shared
ownership semantics of TyrData, i.e.:
/// Inline data stored directly in the DRM device.
/// Contains work items that need access to the device itself.
#[pin_data]
pub(crate) struct TyrInlineData {
/// The work to process firmware events (tiler OOM, etc.)
#[pin]
pub(crate) fw_events_work: Work<TyrDevice, 2>,
/// The scheduler tick work
#[pin]
pub(crate) tick_work: Work<TyrDevice, 1>,
/// The actual driver data
pub(crate) data: Arc<TyrData>,
}
3) When processing a TILER_OOM event, we now have access to ARef<TyrDevice>
which lets us allocate more memory to the tiler inside the handler routine.
This applies for regular work items, but there is a use-case for delayed work
items as well. As we spoke during Kangrejos, Tyr and Panthor have this internal
job scheduler that bridges the gap between the outstanding jobs and the limited
amount of ring buffers exposed by the firmware. This runs periodically, which
is a major use case for delayed work items in Tyr.
This is not implemented in the prototype at the moment, because it's not needed
for a demo. We simply reject jobs with -EBUSY if all the firmware slots are
full. Sadly, there is not enough upstream at the moment to provide a patch that is
actually useful for Tyr itself in this series, but I am sure we will eventually
need it. This statement applies to both regular and delayed work items.
I assume you're worried about the device being unbound in the meantime? These types
of discussions are precisely why I am submitting this a bit in advance :)
By the way, I’d say in any case we should merge patch #1 once there’s no more comments,
as implementing ARef support seems to be orthogonal to the issue I raised above, unless
you’re worried about something else?
[0]: https://gitlab.collabora.com/dwlsalmeida/for-upstream/-/commit/4ac5ec7d8b13d4d4fc5280b414b02ba5ab8e8f23
Powered by blists - more mailing lists