Re: [PATCH v3 11/13] drm/vino: add the USB driver frontend

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Calling sleeping functions in Plane and CRTC atomic callbacks
- [Low] Non-compliant import formatting.
--

commit 8a75fe9a885fe8920f6bb62eb588d5aa17fbe92e
Author: Mike Lothian <mike@fireburn.co.uk>

drm/vino: add the USB driver frontend
    
Vino drives DisplayLink DL3 docks as ordinary DRM devices, replacing the
out-of-tree EVDI module and the closed-source DisplayLinkManager daemon.
It adds the probe lifecycle, places the device by family, and handles
presence and EDID workers off the probe path. Sinks that drop and return
quickly are absorbed to prevent dock-wide re-activations.

> diff --git a/drivers/gpu/drm/vino/vino.rs b/drivers/gpu/drm/vino/vino.rs
> new file mode 100644
> index 0000000000000..0813faa43c031
> --- /dev/null
> +++ b/drivers/gpu/drm/vino/vino.rs
> @@ -0,0 +1,1747 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// SPDX-FileCopyrightText: Copyright (C) 2026 Mike Lothian
> +
> +//! DRM/KMS driver for DisplayLink DL3 docks.
> +//!
> +//! Vino drives the Dell Universal Dock D6000 using a clean-room implementation of its USB control,
> +//! HDCP authentication and compressed video protocols. Each device owns its control session and
> +//! exposes two atomic KMS pipelines backed by shmem GEM objects.
> +
> +use kernel::{
> +    alloc::flags::GFP_KERNEL,
> +    alloc::Flags,
> +    device::{self, Core},

[Severity: Low]
Does this import block violate the vertical import style requirement?

The kernel Rust coding guidelines strictly require a vertical import style
instead of nested brace groups.

[ ... ]


Also in drivers/gpu/drm/vino/drm_sink/mode_objects.rs:

[Severity: High]
Does this call read_cursor_bgra() in VinoPlane::atomic_update() from atomic
context?

VinoPlane::atomic_update():
    if let Ok(bgra) = read_cursor_bgra(fb, usize::from(w), usize::from(h)) {

read_cursor_bgra() allocates memory with GFP_KERNEL and invokes vmap(), both
of which can sleep. Plane and CRTC atomic callbacks run in atomic context
where sleeping is forbidden, which can cause kernel warnings and deadlocks.


[Severity: High]
Does queue_scanout() sleep here while in atomic context?

VinoPlane::atomic_update():
    data.queue_scanout(
        dev,
        fb,
        PendingScanout { ... },
    );

It acquires the data.shadow Mutex and calls SourceBindingCache::get(), which
allocates with GFP_KERNEL and calls vmap().


[Severity: High]
Does this acquire a sleeping Mutex inside VinoCrtc::atomic_enable()?

VinoCrtc::atomic_enable():
    let mut pinned = crtc.vblank_pinned.lock();

CRTC atomic callbacks run in atomic context where sleeping is forbidden.


[Severity: High]
Do these acquire multiple sleeping Mutexes inside VinoCrtc::atomic_disable()?

VinoCrtc::atomic_disable():
    drop(crtc.vblank_pinned.lock().take());
    ...
    data.pending_scanout.lock()[connector as usize] = None;
    data.settle_repaint.lock()[connector as usize] = None;
    data.shadow[connector as usize].lock().discard();

CRTC atomic callbacks run in atomic context, so acquiring sleeping locks here
is forbidden and can cause kernel warnings or deadlocks.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260826163913.7052-1-mike@fireburn.co.uk?part=11