Re: [PATCH v4 3/4] drm/tyr: add GPU reset infrastructure

"Danilo Krummrich" <[email protected]>
Newsgroups org.kernel.vger.rust-for-linux,org.freedesktop.lists.dri-devel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On Sat Aug 15, 2026 at 12:23 PM CEST, Onur Özkan wrote:
> +#[pin_data]
> +struct Controller<'bound> {

Please only use the lifetime name 'bound for bus device private data (which
defines the 'bound lifetime). Everything within is shorter lived and should
carry a different name (see also [1]).

I'd suggest 'ctrl or just 'a, once you have self-referencial fields 'ctrl (or
similar) is preferred.

[1] https://lore.kernel.org/all/[email protected]/

> +    fn reset_work(self: &Arc<Self>) {
> +        if !self.try_transition(ResetState::Pending, ResetState::InProgress) {
> +            return;
> +        }
> +
> +        dev_info!(self.pdev, "Starting GPU reset.\n");

Please use dev_dbg!().

> +        // Wait for current hardware accesses to finish before resetting.
> +        let reset_guard = self.hw.close();
> +        let reset_result = run_reset(self.pdev.as_ref(), &self.iomem);
> +        drop(reset_guard);
> +
> +        if let Err(e) = reset_result {
> +            dev_err!(self.pdev, "GPU reset failed: {:?}\n", e);
> +
> +            // TODO: Unplug the GPU.
> +            // There is no API for unplugging the GPU and this is unreachable
> +            // for now since there are no hardware users for reset API.
> +        } else {
> +            dev_info!(self.pdev, "GPU reset completed.\n");

Same here.

> +        }
> +
> +        let _ = self.try_transition(ResetState::InProgress, ResetState::Idle);
> +    }
> +}
> +
> +/// User-facing handle for scheduling resets.
> +///
> +/// Dropping the handle drains any queued or in-flight reset work before the
> +/// [`ScopedQueue`] and the clock and regulator resources are released.
> +pub(crate) struct ResetHandle<'bound> {

Same as above 'reset or just 'a.

> +    controller: Arc<Controller<'bound>>,

This can just be ScopedWork<Controller<'a>>; no extra reference count and
allocation needed, as the constructor gives you an impl PinInit.

It also gets you rid of the open-coded cancel_sync() call in the destructor of
ResetHandle.

Also note that [2] already has Send + Sync for OwnedQueue when OwnedQueue is
introduced, so you can drop patch 1 of this series.

Thanks,
Danilo

[2] https://lore.kernel.org/all/[email protected]/
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.