[PATCH 0/5] rust: drm: add panel bindings

[PATCH 0/5] rust: drm: add panel bindings

[Albert Esteve]

This series adds Rust abstractions for the DRM panel subsystem,
covering both the consumer and producer sides of the API.
This helps closing the gap so that future panel drivers
can be written directly in Rust.

Background
----------
The DRM panel C API has been progressively hardened over the past
year (see [1], [2], and [3]). All panel drivers were fully migrated
to kref-based allocation via devm_drm_panel_alloc(). Panel lookup
functions (i.e., of_drm_find_panel() and find_panel_by_fwnode())
were also updated to acquire a reference before returning.

Note this series depends on [3] being merged in the base
tree. Currently it has been only applied to drm-misc-next.
The other series mentioned above are already part of the
base for the current version of this series.

As a personal note, part of my motivation for creating this series
was learning more about Rust in the kernel. I would appreciate
feedback on Rust-specific idioms or kernel conventions used here
that I may have missed.

What this series does
---------------------
Patch 1 adds of::Node, a Rust wrapper for struct device_node
implementing AlwaysRefCounted. C helpers are added so that of_node_get()
and of_node_put() are reachable from Rust independently of CONFIG_OF_DYNAMIC.

Patch 2 adds drm::Connector, a minimal Rust wrapper for
struct drm_connector. Its sole purpose is to provide a typed reference
for the get_modes() callback argument; no ownership or refcounting is
exposed at this stage.

Patch 3 adds the consumer side of the panel abstraction:
- Panel wraps struct drm_panel and implements AlwaysRefCounted via
  drm_panel_get/put. Consumer lifecycle methods are exposed as safe wrappers.
- PanelOrientation wraps enum drm_panel_orientation and provides
  from_of_node() to read the "rotation" device tree property.
- Registration manages the panel's presence in the global registry
  via drm_panel_add/remove, decoupling registry lifetime from
  reference count lifetime.

Patch 4 adds the producer side:
- PanelFuncs is a #[vtable] trait that panel drivers implement to
  provide their callbacks. All but get_modes are optional.
- Panel::new wraps __devm_drm_panel_alloc, returning an ARef<Panel>.
  Registration with the global registry is kept separate via
  Registration::register, following the pattern of drm::Device::new.
- ConnectorType mirrors the DRM_MODE_CONNECTOR_* defines from
  include/uapi/drm/drm_mode.h, required by Panel::new to specify the
  panel's connector type.

Patch 5 adds gated tests, covering data type conversions and layout
invariants.

[1] https://lore.kernel.org/all/20250331-b4-panel-refcounting-v4-0-dad50c60c6c9@redhat.com/
[2] https://lore.kernel.org/all/20260508-drm_panel_init_rm-v2-0-0bd4ac429971@redhat.com/
[3] https://lore.kernel.org/all/20260717-drm_refcount_wiring-v3-0-023900c32e01@redhat.com/

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
Albert Esteve (5):
      rust: of: add Node type
      rust: drm: add connector abstraction
      rust: drm: add panel consumer abstractions
      rust: drm: add panel producer abstractions
      rust: drm: add KUnit tests for panel

 rust/bindings/bindings_helper.h |   2 +
 rust/helpers/drm.c              |  15 +
 rust/helpers/of.c               |  10 +
 rust/kernel/Kconfig.test        |  10 +
 rust/kernel/drm/connector.rs    |  33 ++
 rust/kernel/drm/mod.rs          |   2 +
 rust/kernel/drm/panel.rs        | 684 ++++++++++++++++++++++++++++++++++++++++
 rust/kernel/of.rs               |  43 +++
 8 files changed, 799 insertions(+)
---
base-commit: c21bb4193868a8de71fc4693fa741e195fdf5d86
change-id: 20260814-drm_panel_bindings-91a3aae5d09c

Best regards,
-- 
Albert Esteve <aesteve@redhat.com>

[PATCH 1/5] rust: of: add Node type

[Albert Esteve]

Add a Rust abstraction for `struct device_node`, the device
tree node type.

`Node` wraps `device_node` type pointer and implements
`AlwaysRefCounted`, allowing owned references to device
tree nodes to be held. To do so, add C helpers to make
them always accesible from Rust bindings, independently
from the configuration.

This abstraction is needed for subsequent patches, in particular
for creating drm_panel instances from a `const struct device_node`
pointer argument.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 rust/helpers/of.c | 10 ++++++++++
 rust/kernel/of.rs | 43 +++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+)

diff --git a/rust/helpers/of.c b/rust/helpers/of.c
index 8f62ca69e8ba5..dca306852975e 100644
--- a/rust/helpers/of.c
+++ b/rust/helpers/of.c
@@ -6,3 +6,13 @@ __rust_helper bool rust_helper_is_of_node(const struct fwnode_handle *fwnode)
 {
 	return is_of_node(fwnode);
 }
+
+__rust_helper struct device_node *rust_helper_of_node_get(struct device_node *node)
+{
+	return of_node_get(node);
+}
+
+__rust_helper void rust_helper_of_node_put(struct device_node *node)
+{
+	of_node_put(node);
+}
diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
index 58b20c367f993..e75ab81cfe1f7 100644
--- a/rust/kernel/of.rs
+++ b/rust/kernel/of.rs
@@ -6,7 +6,10 @@
     bindings,
     device_id::{RawDeviceId, RawDeviceIdIndex},
     prelude::*,
+    sync::aref::AlwaysRefCounted,
+    types::Opaque,
 };
+use core::ptr::NonNull;
 
 /// IdTable type for OF drivers.
 pub type IdTable<T> = &'static dyn kernel::device_id::IdTable<DeviceId, T>;
@@ -63,3 +66,43 @@ macro_rules! of_device_table {
         $crate::module_device_table!("of", $module_table_name, $table_name);
     };
 }
+
+/// A device tree node (`struct device_node`).
+///
+/// # Invariants
+///
+/// The inner pointer is always a valid, non-null pointer to a `struct device_node`
+/// with a positive reference count.
+#[repr(transparent)]
+pub struct Node(Opaque<bindings::device_node>);
+
+impl Node {
+    /// Creates a reference from a raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// `ptr` must be a valid, non-null `struct device_node` pointer that remains
+    /// valid for the lifetime `'a`.
+    pub unsafe fn from_raw<'a>(ptr: *const bindings::device_node) -> &'a Self {
+        // SAFETY: Caller guarantees `ptr` is valid and lives for `'a`.
+        unsafe { &*ptr.cast() }
+    }
+
+    /// Returns the raw pointer to the underlying `struct device_node`.
+    pub fn as_raw(&self) -> *const bindings::device_node {
+        self.0.get() as _
+    }
+}
+
+// SAFETY: By the type invariants, this type is always refcounted.
+unsafe impl AlwaysRefCounted for Node {
+    fn inc_ref(&self) {
+        // SAFETY: The type invariant guarantees the pointer is valid.
+        unsafe { bindings::of_node_get(self.as_raw().cast_mut()) };
+    }
+
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        // SAFETY: The safety requirements guarantee that the refcount is non-zero.
+        unsafe { bindings::of_node_put(obj.cast().as_ptr()) };
+    }
+}

-- 
2.55.0

Re: [PATCH 1/5] rust: of: add Node type

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Medium] The `# Safety` section of `Node::from_raw` fails to enforce a positive reference count, and the function is missing the mandatory `// INVARIANT:` comment.
- [Medium] The `Node` type lacks explicit `Send` and `Sync` trait implementations.
- [Low] Missing `#[inline]` annotations on small forwarding abstraction functions.
- [Low] Violation of the kernel vertical import style.
- [Low] `Node` is incorrectly documented as wrapping a pointer.
--

--- Patch [1]: [PATCH 1/5] rust: of: add Node type ---
commit 61d64a4023f5c5b35bf62557893fca7718f72090
Author: Albert Esteve <aesteve@redhat.com>

rust: of: add Node type

This commit introduces a Rust abstraction for the device tree node type. It
wraps the device_node struct and implements the AlwaysRefCounted trait,
allowing safe ownership and reference counting of device tree nodes in Rust.
It also adds C helpers to access the reference counting functions.

> Node wraps device_node type pointer and implements
> AlwaysRefCounted, allowing owned references to device
> tree nodes to be held.

[Severity: Low]
This isn't a bug, but could this description cause confusion? 

The commit message and the invariants section state that Node wraps a 
pointer, but the struct actually wraps Opaque<bindings::device_node>. This 
represents the struct itself rather than a pointer to it, which might confuse 
whether it is meant to be passed by value or strictly by reference.

> diff --git a/rust/kernel/of.rs b/rust/kernel/of.rs
> index 58b20c367f993..e75ab81cfe1f7 100644
> --- a/rust/kernel/of.rs
> +++ b/rust/kernel/of.rs
> @@ -6,7 +6,10 @@
>      bindings,
>      device_id::{RawDeviceId, RawDeviceIdIndex},
>      prelude::*,
> +    sync::aref::AlwaysRefCounted,
> +    types::Opaque,
>  };
> +use core::ptr::NonNull;

[Severity: Low]
This isn't a bug, but does this import ordering follow the kernel vertical 
import style? 

Standard library imports like core::ptr::NonNull are typically placed before 
the crate imports and separated by a blank line.

> +
> +/// A device tree node (struct device_node).
> +///
> +/// # Invariants
> +///
> +/// The inner pointer is always a valid, non-null pointer to a struct device_node
> +/// with a positive reference count.
> +#[repr(transparent)]
> +pub struct Node(Opaque<bindings::device_node>);

[Severity: Medium]
Are we missing explicit Send and Sync trait implementations for Node?

Since Node wraps Opaque<bindings::device_node>, it defaults to not 
implementing Send and Sync. Device Tree nodes use thread-safe atomic reference 
counting and are safe to share across threads. Without explicit Send and Sync 
implementations, ARef<Node> cannot be passed across thread boundaries or used 
in concurrent driver structures.

> +impl Node {
> +    /// Creates a reference from a raw pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// ptr must be a valid, non-null struct device_node pointer that remains
> +    /// valid for the lifetime 'a.
> +    pub unsafe fn from_raw<'a>(ptr: *const bindings::device_node) -> &'a Self {

[Severity: Medium]
Should the safety documentation for from_raw require that the pointer has a
positive reference count?

The type invariant for Node strictly requires a positive reference count, but 
the safety preconditions here omit this requirement. A caller could pass a 
valid but zero-refcount pointer, complying with the safety contract but 
violating the type invariant. If this is later converted to an owned ARef, 
inc_ref will call of_node_get on a node with a zero refcount.

[Severity: Low]
This isn't a bug, but are we missing the #[inline] annotation on this small 
forwarding abstraction function?

> +        // SAFETY: Caller guarantees ptr is valid and lives for 'a.
> +        unsafe { &*ptr.cast() }
> +    }

[Severity: Medium]
Are we missing the mandatory // INVARIANT: comment here when casting to the 
struct, as required by the Rust subsystem guidelines?

> +
> +    /// Returns the raw pointer to the underlying struct device_node.
> +    pub fn as_raw(&self) -> *const bindings::device_node {
> +        self.0.get() as _
> +    }
> +}

[Severity: Low]
This isn't a bug, but should this small getter function be annotated with 
#[inline]?

> +// SAFETY: By the type invariants, this type is always refcounted.
> +unsafe impl AlwaysRefCounted for Node {
> +    fn inc_ref(&self) {

[Severity: Low]
This isn't a bug, but should this forwarding abstraction method be annotated 
with #[inline]?

> +        // SAFETY: The type invariant guarantees the pointer is valid.
> +        unsafe { bindings::of_node_get(self.as_raw().cast_mut()) };
> +    }
> +
> +    unsafe fn dec_ref(obj: NonNull<Self>) {

[Severity: Low]
This isn't a bug, but should this forwarding abstraction method be annotated 
with #[inline]?

> +        // SAFETY: The safety requirements guarantee that the refcount is non-zero.
> +        unsafe { bindings::of_node_put(obj.cast().as_ptr()) };
> +    }
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-drm_panel_bindings-v1-0-1f974508a31c@redhat.com?part=1

[PATCH 2/5] rust: drm: add connector abstraction

[Albert Esteve]

Add DRM Connector abstraction wrapping `struct drm_connector`.
Used by the DRM panel abstractions added in the following patch.
Since it is merely used as a raw pointer and managed internally in
the panel's C functions, the Connector does not need to expose
anything else for now.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 rust/bindings/bindings_helper.h |  1 +
 rust/kernel/drm/connector.rs    | 33 +++++++++++++++++++++++++++++++++
 rust/kernel/drm/mod.rs          |  1 +
 3 files changed, 35 insertions(+)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 1124785e210b3..7f3030ac0feb6 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -30,6 +30,7 @@
 
 #include <linux/acpi.h>
 #include <linux/gpu_buddy.h>
+#include <drm/drm_connector.h>
 #include <drm/drm_device.h>
 #include <drm/drm_drv.h>
 #include <drm/drm_file.h>
diff --git a/rust/kernel/drm/connector.rs b/rust/kernel/drm/connector.rs
new file mode 100644
index 0000000000000..b3b36644375dd
--- /dev/null
+++ b/rust/kernel/drm/connector.rs
@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! DRM connector abstractions.
+//!
+//! C header: [`include/drm/drm_connector.h`](srctree/include/drm/drm_connector.h)
+
+use crate::{bindings, types::Opaque};
+
+/// A DRM connector (`struct drm_connector`).
+///
+/// # Invariants
+///
+/// The inner pointer is always a valid, non-null pointer to a `struct drm_connector`.
+#[repr(transparent)]
+pub struct Connector(Opaque<bindings::drm_connector>);
+
+impl Connector {
+    /// Creates a reference to a [`Connector`] from a raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// `ptr` must be a valid, non-null pointer to a `struct drm_connector` that
+    /// remains valid for at least the lifetime `'a`.
+    pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_connector) -> &'a Self {
+        // SAFETY: Caller guarantees `ptr` is valid and lives for `'a`.
+        unsafe { &*ptr.cast() }
+    }
+
+    /// Returns the raw pointer to the underlying `struct drm_connector`.
+    pub fn as_raw(&self) -> *mut bindings::drm_connector {
+        self.0.get()
+    }
+}
diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
index a66e7166f66b5..672ea8728e1c3 100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -2,6 +2,7 @@
 
 //! DRM subsystem abstractions.
 
+pub mod connector;
 pub mod device;
 pub mod driver;
 pub mod file;

-- 
2.55.0

Re: [PATCH 2/5] rust: drm: add connector abstraction

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Low] The `Connector` struct's `# Invariants` documentation incorrectly references an 'inner pointer', whereas the struct is a transparent wrapper around an `Opaque` type.
- [Low] The forwarding methods `from_raw` and `as_raw` in the `Connector` abstraction lack the required `#[inline]` attribute.
--

--- Patch [2]: [PATCH 2/5] rust: drm: add connector abstraction ---
commit 3b78dba370077b309a3e8fb4bdf3afb4e77c1754
Author: Albert Esteve <aesteve@redhat.com>

rust: drm: add connector abstraction

Add DRM Connector abstraction wrapping struct drm_connector.
Used by the DRM panel abstractions added in the following patch.

> diff --git a/rust/kernel/drm/connector.rs b/rust/kernel/drm/connector.rs
> new file mode 100644
> index 0000000000000..b3b36644375dd
> --- /dev/null
> +++ b/rust/kernel/drm/connector.rs
> @@ -0,0 +1,33 @@
[ ... ]
> +/// A DRM connector (`struct drm_connector`).
> +///
> +/// # Invariants
> +///
> +/// The inner pointer is always a valid, non-null pointer to a `struct drm_connector`.
> +#[repr(transparent)]
> +pub struct Connector(Opaque<bindings::drm_connector>);

[Severity: Low]
this isn't a bug, but the invariant documentation references an "inner pointer".
Since the struct directly wraps an Opaque type rather than holding a pointer
itself, should this be updated to reflect that?

> +
> +impl Connector {
> +    /// Creates a reference to a [`Connector`] from a raw pointer.
> +    ///
> +    /// # Safety
> +    ///
> +    /// `ptr` must be a valid, non-null pointer to a `struct drm_connector` that
> +    /// remains valid for at least the lifetime `'a`.
> +    pub unsafe fn from_raw<'a>(ptr: *mut bindings::drm_connector) -> &'a Self {

[Severity: Low]
this isn't a bug, but as a small forwarding function for an abstraction, should
this method be annotated with #[inline]?

> +        // SAFETY: Caller guarantees `ptr` is valid and lives for `'a`.
> +        unsafe { &*ptr.cast() }
> +    }
> +
> +    /// Returns the raw pointer to the underlying `struct drm_connector`.
> +    pub fn as_raw(&self) -> *mut bindings::drm_connector {

[Severity: Low]
this isn't a bug, but should this method also be annotated with #[inline]
since it merely forwards to the underlying binding call?

> +        self.0.get()
> +    }
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-drm_panel_bindings-v1-0-1f974508a31c@redhat.com?part=2

[PATCH 3/5] rust: drm: add panel consumer abstractions

[Albert Esteve]

Add Rust abstraction for the DRM panel subsystem, covering
the consumer side of the panel API.

`Panel` wraps a reference-counted `struct drm_panel` and
implements `AlwaysRefCounted`. It exposes consumer-side lifecycle
operations and provides `from_of_node()` associated method for
looking up a registered panel by device tree node.

`PanelOrientation` wraps `enum drm_panel_orientation` and
provides `from_of_node()` to read "rotation" property from
the device tree.

`Registration` manages the panel's presence in the global
registry, decoupling its lifetime from reference count lifetime.

Following patches will handle the producer side.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 rust/bindings/bindings_helper.h |   1 +
 rust/helpers/drm.c              |  15 +++
 rust/kernel/drm/mod.rs          |   1 +
 rust/kernel/drm/panel.rs        | 227 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 244 insertions(+)

diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
index 7f3030ac0feb6..0f23d323cbb24 100644
--- a/rust/bindings/bindings_helper.h
+++ b/rust/bindings/bindings_helper.h
@@ -33,6 +33,7 @@
 #include <drm/drm_connector.h>
 #include <drm/drm_device.h>
 #include <drm/drm_drv.h>
+#include <drm/drm_panel.h>
 #include <drm/drm_file.h>
 #include <drm/drm_gem.h>
 #include <drm/drm_gem_shmem_helper.h>
diff --git a/rust/helpers/drm.c b/rust/helpers/drm.c
index 65f3f22b0e1d2..fc5b9af6e4d23 100644
--- a/rust/helpers/drm.c
+++ b/rust/helpers/drm.c
@@ -2,6 +2,7 @@
 
 #include <drm/drm_gem.h>
 #include <drm/drm_gem_shmem_helper.h>
+#include <drm/drm_panel.h>
 #include <drm/drm_vma_manager.h>
 
 #ifdef CONFIG_DRM
@@ -75,4 +76,18 @@ rust_helper_drm_gem_shmem_object_mmap(struct drm_gem_object *obj, struct vm_area
 }
 
 #endif /* CONFIG_DRM_GEM_SHMEM_HELPER */
+
+__rust_helper struct drm_panel *
+rust_helper_of_drm_find_panel(const struct device_node *np)
+{
+	return of_drm_find_panel(np);
+}
+
+__rust_helper int
+rust_helper_of_drm_get_panel_orientation(const struct device_node *np,
+					 enum drm_panel_orientation *orientation)
+{
+	return of_drm_get_panel_orientation(np, orientation);
+}
+
 #endif /* CONFIG_DRM */
diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
index 672ea8728e1c3..d1de5d448a1a5 100644
--- a/rust/kernel/drm/mod.rs
+++ b/rust/kernel/drm/mod.rs
@@ -9,6 +9,7 @@
 pub mod gem;
 pub mod gpuvm;
 pub mod ioctl;
+pub mod panel;
 
 pub use self::device::Device;
 pub use self::device::DeviceContext;
diff --git a/rust/kernel/drm/panel.rs b/rust/kernel/drm/panel.rs
new file mode 100644
index 0000000000000..fd21cc2236685
--- /dev/null
+++ b/rust/kernel/drm/panel.rs
@@ -0,0 +1,227 @@
+// SPDX-License-Identifier: GPL-2.0
+
+//! DRM panel abstractions.
+//!
+//! C header: [`include/drm/drm_panel.h`](srctree/include/drm/drm_panel.h)
+
+use crate::drm::connector::Connector;
+use crate::{
+    bindings, error, of,
+    prelude::*,
+    sync::aref::{ARef, AlwaysRefCounted},
+    types::Opaque,
+};
+use core::ptr::NonNull;
+
+/// A DRM panel object.
+///
+/// Wraps `struct drm_panel`. Instances are reference-counted via [`drm_panel_get`] and
+/// [`drm_panel_put`]; use [`ARef<Panel>`] to hold an owned reference.
+///
+/// The DRM panel methods allow drivers to register panel objects with a
+/// central registry and provide functions to retrieve those panels in display
+/// drivers.
+///
+/// # Invariants
+///
+/// The inner pointer is always a valid, non-null pointer to a `struct drm_panel` with a
+/// positive reference count.
+///
+/// [`drm_panel_get`]: srctree/include/drm/drm_panel.h
+/// [`drm_panel_put`]: srctree/include/drm/drm_panel.h
+#[repr(transparent)]
+pub struct Panel(Opaque<bindings::drm_panel>);
+
+impl Panel {
+    /// Creates a reference from a raw pointer.
+    ///
+    /// # Safety
+    ///
+    /// `ptr` must be a valid, non-null `struct drm_panel` pointer that remains
+    /// valid for the lifetime `'a`.
+    pub unsafe fn from_raw<'a>(ptr: *const bindings::drm_panel) -> &'a Self {
+        // SAFETY: Caller guarantees `ptr` is valid and lives for `'a`.
+        unsafe { &*ptr.cast() }
+    }
+
+    /// Returns the raw pointer to the underlying `struct drm_panel`.
+    pub fn as_raw(&self) -> *mut bindings::drm_panel {
+        self.0.get()
+    }
+
+    /// Power on a panel.
+    ///
+    /// Calling this function will enable power and deassert any reset signals to
+    /// the panel. After this has completed it is possible to communicate with any
+    /// integrated circuitry via a command bus. This function cannot fail (as it is
+    /// called from the pre_enable call chain). There will always be a call to
+    /// [`Panel::disable`] afterwards.
+    pub fn prepare(&self) {
+        // SAFETY: The type invariant guarantees the pointer is valid.
+        unsafe { bindings::drm_panel_prepare(self.as_raw()) }
+    }
+
+    /// Power off a panel.
+    ///
+    /// Calling this function will completely power off a panel (assert the panel's
+    /// reset, turn off power supplies, ...). After this function has completed, it
+    /// is usually no longer possible to communicate with the panel until another
+    /// call to [`Panel::prepare`].
+    pub fn unprepare(&self) {
+        // SAFETY: The type invariant guarantees the pointer is valid.
+        unsafe { bindings::drm_panel_unprepare(self.as_raw()) }
+    }
+
+    /// Enable a panel.
+    ///
+    /// Calling this function will cause the panel display drivers to be turned on
+    /// and the backlight to be enabled. Content will be visible on screen after
+    /// this call completes. This function cannot fail (as it is called from the
+    /// enable call chain). There will always be a call to [`Panel::disable`]
+    /// afterwards.
+    pub fn enable(&self) {
+        // SAFETY: The type invariant guarantees the pointer is valid.
+        unsafe { bindings::drm_panel_enable(self.as_raw()) }
+    }
+
+    /// Disable a panel.
+    ///
+    /// This will typically turn off the panel's backlight or disable the display
+    /// drivers. For smart panels it should still be possible to communicate with
+    /// the integrated circuitry via any command bus after this call.
+    pub fn disable(&self) {
+        // SAFETY: The type invariant guarantees the pointer is valid.
+        unsafe { bindings::drm_panel_disable(self.as_raw()) }
+    }
+
+    /// Probe the available display modes of a panel.
+    ///
+    /// The modes probed from the panel are automatically added to the connector
+    /// that the panel is attached to.
+    ///
+    /// Return: The number of modes available from the panel on success, or 0 on
+    /// failure (no modes).
+    pub fn get_modes(&self, connector: &Connector) -> i32 {
+        // SAFETY: The type invariants guarantee the pointers are valid.
+        unsafe { bindings::drm_panel_get_modes(self.as_raw(), connector.as_raw()) } as i32
+    }
+
+    /// Use backlight device node for backlight.
+    ///
+    /// Use this function to enable backlight handling if your panel
+    /// uses device tree and has a backlight phandle.
+    ///
+    /// When the panel is enabled backlight will be enabled after a
+    /// successful call to [`Panel::enable`].
+    ///
+    /// When the panel is disabled backlight will be disabled before the
+    /// call to [`Panel::disable`].
+    ///
+    /// A typical implementation for a panel driver supporting device tree
+    /// will call this function at probe time. Backlight will then be handled
+    /// transparently without requiring any intervention from the driver.
+    #[cfg(CONFIG_BACKLIGHT_CLASS_DEVICE)]
+    pub fn of_backlight(&self) -> Result<()> {
+        // SAFETY: The type invariant guarantees the pointer is valid.
+        error::to_result(unsafe { bindings::drm_panel_of_backlight(self.as_raw()) })?;
+        Ok(())
+    }
+
+    /// Look up the panel associated with the given device tree node.
+    ///
+    /// Searches the set of registered panels for one that matches the given device
+    /// tree node. If a matching panel is found, return a pointer to it.
+    pub fn from_of_node(node: &of::Node) -> Result<ARef<Self>> {
+        // SAFETY: `node.as_raw()` is a valid device_node pointer.
+        let panel = error::from_err_ptr(unsafe { bindings::of_drm_find_panel(node.as_raw()) })?;
+
+        // SAFETY: `from_err_ptr` guarantees a non-null pointer on success.
+        // `of_drm_find_panel` returns a kref-incremented reference.
+        Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(panel).cast()) })
+    }
+}
+
+// SAFETY: By the type invariants, this type is always refcounted.
+unsafe impl AlwaysRefCounted for Panel {
+    fn inc_ref(&self) {
+        // SAFETY: The type invariant guarantees the pointer is valid.
+        unsafe { bindings::drm_panel_get(self.as_raw()) };
+    }
+
+    unsafe fn dec_ref(obj: NonNull<Self>) {
+        // SAFETY: The existence of `obj` guarantees the refcount is positive.
+        unsafe { bindings::drm_panel_put(obj.cast().as_ptr()) };
+    }
+}
+
+/// This enum is used to track the (LCD) panel orientation.
+///
+/// C header: [`include/drm/drm_connector.h`](srctree/include/drm/drm_connector.h)
+#[repr(i32)]
+pub enum PanelOrientation {
+    /// The drm driver has not provided any panel orientation information.
+    Unknown = -1,
+    /// The top side of the panel matches the top side of the device's casing.
+    Normal = 0,
+    /// The top side of the panel matches the bottom side of the device's casing.
+    BottomUp = 1,
+    /// The left side of the panel matches the top side of the device's casing.
+    LeftUp = 2,
+    /// The right side of the panel matches the top side of the device's casing.
+    RightUp = 3,
+}
+
+impl TryFrom<i32> for PanelOrientation {
+    type Error = Error;
+    fn try_from(v: i32) -> Result<Self> {
+        match v {
+            -1 => Ok(Self::Unknown),
+            0 => Ok(Self::Normal),
+            1 => Ok(Self::BottomUp),
+            2 => Ok(Self::LeftUp),
+            3 => Ok(Self::RightUp),
+            _ => Err(EINVAL),
+        }
+    }
+}
+
+impl PanelOrientation {
+    /// Look up the orientation of the panel through the "rotation" binding
+    /// from a device tree node
+    ///
+    /// Looks up the rotation of a panel in the device tree. The orientation of the
+    /// panel is expressed as a property name "rotation" in the device tree. The
+    /// rotation in the device tree is counter clockwise.
+    pub fn from_of_node(node: &of::Node) -> Result<Self> {
+        let mut orientation = 0i32;
+        // SAFETY: `node.as_raw()` is a valid device_node pointer.
+        error::to_result(unsafe {
+            bindings::of_drm_get_panel_orientation(node.as_raw(), &mut orientation)
+        })?;
+        Ok(PanelOrientation::try_from(orientation)?)
+    }
+}
+
+/// A registration of a panel to the global panel registry.
+pub struct Registration(ARef<Panel>);
+
+impl Registration {
+    /// Registers a panel with the global panel registry.
+    pub fn register(panel: ARef<Panel>) -> Self {
+        // SAFETY: The type invariant guarantees the pointer is valid.
+        unsafe { bindings::drm_panel_add(panel.as_raw()) };
+        Self(panel)
+    }
+
+    /// Returns a reference to the panel.
+    pub fn panel(&self) -> &Panel {
+        &self.0
+    }
+}
+
+impl Drop for Registration {
+    fn drop(&mut self) {
+        // SAFETY: The type invariant guarantees the pointer is valid.
+        unsafe { bindings::drm_panel_remove(self.0.as_raw()) };
+    }
+}

-- 
2.55.0

Re: [PATCH 3/5] rust: drm: add panel consumer abstractions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [High] Refcount underflow and use-after-free vulnerability in `Panel::from_of_node`.
- [High] Soundness hole in `Registration::register` allowing memory corruption from safe Rust.
- [Low] Missing conditional compilation guard for `panel` module causing build failures.
- [Low] Improper `#[cfg]` guard on `of_backlight` breaks the build for module configurations.
- [Low] Missing `#[inline]` annotations on small forwarding functions.
- [Low] Imports violate the kernel vertical import style.
--

--- Patch [3]: [PATCH 3/5] rust: drm: add panel consumer abstractions ---
commit e330c1b98aba78a95f008b2fccb2105d337a8df8
Author: Albert Esteve <aesteve@redhat.com>
rust: drm: add panel consumer abstractions

This commit adds a Rust abstraction for the DRM panel subsystem, providing
wrappers for the consumer side of the panel API. It introduces the `Panel`,
`PanelOrientation`, and `Registration` types, handling refcounting and device
tree lookups.

> diff --git a/rust/kernel/drm/mod.rs b/rust/kernel/drm/mod.rs
> index 672ea8728e1c3..d1de5d448a1a5 100644
> --- a/rust/kernel/drm/mod.rs
> +++ b/rust/kernel/drm/mod.rs
> @@ -9,6 +9,7 @@
>  pub mod gem;
>  pub mod gpuvm;
>  pub mod ioctl;
> +pub mod panel;

[Severity: Low]
Does this module need a conditional compilation guard?

If `CONFIG_DRM=y` but `CONFIG_DRM_PANEL` is disabled, `panel.rs` will still
be compiled.

However, `panel.rs` relies on C functions like `drm_panel_prepare`
which become static inline dummies in C when the config is disabled,
meaning bindgen won't export them and the Rust build will fail.

[ ... ]

> diff --git a/rust/kernel/drm/panel.rs b/rust/kernel/drm/panel.rs
> new file mode 100644
> index 0000000000000..fd21cc2236685
> --- /dev/null
> +++ b/rust/kernel/drm/panel.rs
> @@ -0,0 +1,227 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +//! DRM panel abstractions.
> +//!
> +//! C header: [`include/drm/drm_panel.h`](srctree/include/drm/drm_panel.h)
> +
> +use crate::drm::connector::Connector;
> +use crate::{
> +    bindings, error, of,
> +    prelude::*,
> +    sync::aref::{ARef, AlwaysRefCounted},
> +    types::Opaque,
> +};

[Severity: Low]
Could these imports be reformatted to follow the kernel's vertical import
style guidelines?

[ ... ]

> +    /// Returns the raw pointer to the underlying `struct drm_panel`.
> +    pub fn as_raw(&self) -> *mut bindings::drm_panel {
> +        self.0.get()
> +    }

[Severity: Low]
Should small forwarding functions like this one and `prepare()` below
have `#[inline]` annotations, as recommended by the Rust subsystem
guidelines for abstractions?

[ ... ]

> +    /// A typical implementation for a panel driver supporting device tree
> +    /// will call this function at probe time. Backlight will then be handled
> +    /// transparently without requiring any intervention from the driver.
> +    #[cfg(CONFIG_BACKLIGHT_CLASS_DEVICE)]
> +    pub fn of_backlight(&self) -> Result<()> {
> +        // SAFETY: The type invariant guarantees the pointer is valid.
> +        error::to_result(unsafe { bindings::drm_panel_of_backlight(self.as_raw()) })?;
> +        Ok(())
> +    }

[Severity: Low]
Does this `#[cfg]` attribute handle the module build correctly?

If `CONFIG_BACKLIGHT_CLASS_DEVICE=m`, the `#[cfg(CONFIG_BACKLIGHT_CLASS_DEVICE)]`
attribute evaluates to false, which entirely strips the `of_backlight` method
from the Rust API.

This would cause build failures for any Rust driver trying to call it, even
though the C fallback `drm_panel_of_backlight` stub exists.

> +
> +    /// Look up the panel associated with the given device tree node.
> +    ///
> +    /// Searches the set of registered panels for one that matches the given device
> +    /// tree node. If a matching panel is found, return a pointer to it.
> +    pub fn from_of_node(node: &of::Node) -> Result<ARef<Self>> {
> +        // SAFETY: `node.as_raw()` is a valid device_node pointer.
> +        let panel = error::from_err_ptr(unsafe { bindings::of_drm_find_panel(node.as_raw()) })?;
> +
> +        // SAFETY: `from_err_ptr` guarantees a non-null pointer on success.
> +        // `of_drm_find_panel` returns a kref-incremented reference.
> +        Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(panel).cast()) })
> +    }

[Severity: High]
Does `of_drm_find_panel` actually increment the refcount as the comment states?

Looking at the C implementation, `of_drm_find_panel()` returns a raw pointer
but does not increment the panel's `kref` internally.

By wrapping this pointer directly in `ARef::from_raw()`, the Rust code assumes
ownership of a refcount increment that didn't happen. When the resulting
`ARef` is dropped, it will call `drm_panel_put()` and cause a refcount
underflow, leading to a use-after-free.

[ ... ]

> +/// A registration of a panel to the global panel registry.
> +pub struct Registration(ARef<Panel>);
> +
> +impl Registration {
> +    /// Registers a panel with the global panel registry.
> +    pub fn register(panel: ARef<Panel>) -> Self {
> +        // SAFETY: The type invariant guarantees the pointer is valid.
> +        unsafe { bindings::drm_panel_add(panel.as_raw()) };
> +        Self(panel)
> +    }

[Severity: High]
Is it safe to expose this as a safe function taking an `ARef<Panel>`?

If a safe caller passes an already registered panel (for instance, one
obtained via `from_of_node`, or by cloning the `ARef` and calling
`Registration::register()` twice), this will unconditionally call
`drm_panel_add()`.

In C, `drm_panel_add()` executes `list_add_tail(&panel->list, &panel_list)`.
If the panel is already on a list, this will corrupt the global linked list
pointers. This seems to allow memory corruption from entirely safe Rust code.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-drm_panel_bindings-v1-0-1f974508a31c@redhat.com?part=3

[PATCH 4/5] rust: drm: add panel producer abstractions

[Albert Esteve]

`PanelFuncs` is a `#[vtable]` trait that panel drivers implement to
provide their callbacks.

`PanelFuncsVTable` builds a `struct drm_panel_funcs` from `PanelFuncs`
through `extern "C"` trampolines, using the `HAS_*` flags generated by
`#[vtable]` to populate optional callbacks selectively.

`PanelContainer<T>` is a `#[repr(C)]` struct embedding `ManuallyDrop<T>`
at offset zero followed by the `drm_panel`. This layout lets
`__devm_drm_panel_alloc` allocate both in a single `kzalloc` call with
`panel->container` pointing to the base, so `__drm_panel_free` can call
`kfree(container)` to free the entire block. A separate
`devm_add_action_or_reset` runs `drop_in_place::<T>` before the memory
is reclaimed, ensuring T's destructor is called at the right time.

`Panel::new` wraps `__devm_drm_panel_alloc` and ties the above together,
returning an `ARef<Panel>`. Registration with the global registry is kept
separate via `Registration::register`, following the pattern established
by `drm::Device::new`.

`ConnectorType` mirrors the `DRM_MODE_CONNECTOR_*` defines from
`include/uapi/drm/drm_mode.h`, required by `Panel::new` to specify the
panel's connector type.

Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 rust/kernel/drm/panel.rs | 376 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 373 insertions(+), 3 deletions(-)

diff --git a/rust/kernel/drm/panel.rs b/rust/kernel/drm/panel.rs
index fd21cc2236685..8f87774e06ed7 100644
--- a/rust/kernel/drm/panel.rs
+++ b/rust/kernel/drm/panel.rs
@@ -6,11 +6,15 @@
 
 use crate::drm::connector::Connector;
 use crate::{
-    bindings, error, of,
+    bindings,
+    device::Device,
+    error, of,
     prelude::*,
     sync::aref::{ARef, AlwaysRefCounted},
     types::Opaque,
 };
+use core::marker::PhantomData;
+use core::mem::{ManuallyDrop, MaybeUninit};
 use core::ptr::NonNull;
 
 /// A DRM panel object.
@@ -103,7 +107,7 @@ pub fn disable(&self) {
     /// failure (no modes).
     pub fn get_modes(&self, connector: &Connector) -> i32 {
         // SAFETY: The type invariants guarantee the pointers are valid.
-        unsafe { bindings::drm_panel_get_modes(self.as_raw(), connector.as_raw()) } as i32
+        unsafe { bindings::drm_panel_get_modes(self.as_raw(), connector.as_raw()) }
     }
 
     /// Use backlight device node for backlight.
@@ -139,6 +143,66 @@ pub fn from_of_node(node: &of::Node) -> Result<ARef<Self>> {
         // `of_drm_find_panel` returns a kref-incremented reference.
         Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(panel).cast()) })
     }
+
+    /// Allocates and initialises a device-managed panel.
+    ///
+    /// `data` is embedded in the same allocation as the `drm_panel` and its
+    /// destructor is called automatically when `dev` is unbound.
+    ///
+    /// Use [`Registration::register`] to add the panel to the global registry
+    /// once it is ready to be used by display drivers.
+    pub fn new<T: PanelFuncs>(
+        dev: &Device,
+        data: T,
+        connector_type: ConnectorType,
+    ) -> Result<ARef<Self>> {
+        // SAFETY: `dev` is valid by its type invariants; `PanelFuncsVTable::build()`
+        // returns a valid, static `drm_panel_funcs` pointer.
+        let container = error::from_err_ptr(unsafe {
+            bindings::__devm_drm_panel_alloc(
+                dev.as_raw(),
+                core::mem::size_of::<PanelContainer<T>>(),
+                core::mem::offset_of!(PanelContainer<T>, panel),
+                PanelFuncsVTable::<T>::build(),
+                connector_type as i32,
+            )
+        })? as *mut PanelContainer<T>;
+
+        // SAFETY: `container` is a valid pointer to uninitialized memory.
+        unsafe {
+            core::ptr::write(
+                core::ptr::addr_of_mut!((*container).data),
+                ManuallyDrop::new(data),
+            )
+        };
+
+        // SAFETY:
+        // - `dev.as_raw()` is a pointer to a valid and bound device.
+        // - `container.cast()` is a valid pointer to the initialized `PanelContainer<T>`.
+        error::to_result(unsafe {
+            // `devm_add_action_or_reset` calls `drop_panel_data` on failure, so `data`
+            // is dropped even if this registration fails.
+            // Registering after `__devm_drm_panel_alloc` ensures devres LIFO order:
+            // `drop_panel_data` runs before `kfree(container)`.
+            bindings::devm_add_action_or_reset(
+                dev.as_raw(),
+                Some(drop_panel_data::<T>),
+                container.cast(),
+            )
+        })?;
+
+        // SAFETY: `__devm_drm_panel_alloc` was successful, hence `container` is
+        // valid and the `drm_panel` at this offset is initialised.
+        let raw = unsafe {
+            (container as *mut u8)
+                .add(core::mem::offset_of!(PanelContainer<T>, panel))
+                .cast::<bindings::drm_panel>()
+        };
+
+        // SAFETY: `__devm_drm_panel_alloc` was successful, hence `raw` is valid
+        // and the refcount is non-zero.
+        Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(raw).cast()) })
+    }
 }
 
 // SAFETY: By the type invariants, this type is always refcounted.
@@ -149,7 +213,7 @@ fn inc_ref(&self) {
     }
 
     unsafe fn dec_ref(obj: NonNull<Self>) {
-        // SAFETY: The existence of `obj` guarantees the refcount is positive.
+        // SAFETY: The safety requirements guarantee that the refcount is non-zero.
         unsafe { bindings::drm_panel_put(obj.cast().as_ptr()) };
     }
 }
@@ -202,6 +266,87 @@ pub fn from_of_node(node: &of::Node) -> Result<Self> {
     }
 }
 
+/// The type of a DRM connector.
+///
+/// Mirrors the `DRM_MODE_CONNECTOR_*` defines in
+/// [`include/uapi/drm/drm_mode.h`](srctree/include/uapi/drm/drm_mode.h).
+#[repr(u32)]
+pub enum ConnectorType {
+    /// Unknown connector type (`DRM_MODE_CONNECTOR_Unknown`).
+    Unknown = 0,
+    /// VGA connector (`DRM_MODE_CONNECTOR_VGA`).
+    VGA = 1,
+    /// DVI-I connector (`DRM_MODE_CONNECTOR_DVII`).
+    DVII = 2,
+    /// DVI-D connector (`DRM_MODE_CONNECTOR_DVID`).
+    DVID = 3,
+    /// DVI-A connector (`DRM_MODE_CONNECTOR_DVIA`).
+    DVIA = 4,
+    /// Composite connector (`DRM_MODE_CONNECTOR_Composite`).
+    Composite = 5,
+    /// S-Video connector (`DRM_MODE_CONNECTOR_SVIDEO`).
+    SVIDEO = 6,
+    /// LVDS connector (`DRM_MODE_CONNECTOR_LVDS`).
+    LVDS = 7,
+    /// Component connector (`DRM_MODE_CONNECTOR_Component`).
+    Component = 8,
+    /// 9-pin DIN connector (`DRM_MODE_CONNECTOR_9PinDIN`).
+    NinePinDin = 9,
+    /// DisplayPort connector (`DRM_MODE_CONNECTOR_DisplayPort`).
+    DisplayPort = 10,
+    /// HDMI type A connector (`DRM_MODE_CONNECTOR_HDMIA`).
+    HDMIA = 11,
+    /// HDMI type B connector (`DRM_MODE_CONNECTOR_HDMIB`).
+    HDMIB = 12,
+    /// TV connector (`DRM_MODE_CONNECTOR_TV`).
+    TV = 13,
+    /// Embedded DisplayPort connector (`DRM_MODE_CONNECTOR_eDP`).
+    #[allow(non_camel_case_types)]
+    eDP = 14,
+    /// Virtual connector (`DRM_MODE_CONNECTOR_VIRTUAL`).
+    Virtual = 15,
+    /// MIPI DSI connector (`DRM_MODE_CONNECTOR_DSI`).
+    DSI = 16,
+    /// DPI connector (`DRM_MODE_CONNECTOR_DPI`).
+    DPI = 17,
+    /// Writeback connector (`DRM_MODE_CONNECTOR_WRITEBACK`).
+    Writeback = 18,
+    /// SPI connector (`DRM_MODE_CONNECTOR_SPI`).
+    SPI = 19,
+    /// USB connector (`DRM_MODE_CONNECTOR_USB`).
+    USB = 20,
+}
+
+impl TryFrom<u32> for ConnectorType {
+    type Error = Error;
+    fn try_from(v: u32) -> Result<Self> {
+        match v {
+            0 => Ok(Self::Unknown),
+            1 => Ok(Self::VGA),
+            2 => Ok(Self::DVII),
+            3 => Ok(Self::DVID),
+            4 => Ok(Self::DVIA),
+            5 => Ok(Self::Composite),
+            6 => Ok(Self::SVIDEO),
+            7 => Ok(Self::LVDS),
+            8 => Ok(Self::Component),
+            9 => Ok(Self::NinePinDin),
+            10 => Ok(Self::DisplayPort),
+            11 => Ok(Self::HDMIA),
+            12 => Ok(Self::HDMIB),
+            13 => Ok(Self::TV),
+            14 => Ok(Self::eDP),
+            15 => Ok(Self::Virtual),
+            16 => Ok(Self::DSI),
+            17 => Ok(Self::DPI),
+            18 => Ok(Self::Writeback),
+            19 => Ok(Self::SPI),
+            20 => Ok(Self::USB),
+            _ => Err(EINVAL),
+        }
+    }
+}
+
 /// A registration of a panel to the global panel registry.
 pub struct Registration(ARef<Panel>);
 
@@ -225,3 +370,228 @@ fn drop(&mut self) {
         unsafe { bindings::drm_panel_remove(self.0.as_raw()) };
     }
 }
+
+/// Operations implemented by a DRM panel driver.
+///
+/// Implement this trait to provide a DRM panel driver and its callbacks. Use
+/// [`Panel::new`] to allocate the panel, passing the driver data as `T`.
+///
+/// C header: [`include/drm/drm_panel.h`](srctree/include/drm/drm_panel.h)
+#[vtable]
+pub trait PanelFuncs {
+    /// Turn on panel and perform set up.
+    ///
+    /// This function is optional.
+    fn prepare(&self, _panel: &Panel) -> Result<()> {
+        Ok(())
+    }
+
+    /// Turn off panel.
+    ///
+    /// This function is optional.
+    fn unprepare(&self, _panel: &Panel) -> Result<()> {
+        Ok(())
+    }
+
+    /// Enable panel (turn on back light, etc.).
+    ///
+    /// This function is optional.
+    fn enable(&self, _panel: &Panel) -> Result<()> {
+        Ok(())
+    }
+
+    /// Disable panel (turn off back light, etc.).
+    ///
+    /// This function is optional.
+    fn disable(&self, _panel: &Panel) -> Result<()> {
+        Ok(())
+    }
+
+    /// Add modes to the connector that the panel is attached to
+    /// and returns the number of modes added.
+    ///
+    /// This function is mandatory.
+    fn get_modes(&self, _panel: &Panel, _connector: &Connector) -> i32 {
+        build_error!("get_modes is mandatory")
+    }
+
+    /// Return the panel orientation set by device tree or EDID.
+    ///
+    /// This function is optional.
+    fn get_orientation(&self, _panel: &Panel) -> PanelOrientation {
+        PanelOrientation::Unknown
+    }
+}
+
+// Outer allocation layout used by `Panel::new`.
+//
+// `__devm_drm_panel_alloc` allocates a block of `size_of::<PanelContainer<T>>()`
+// bytes, places `drm_panel` at `offset_of!(PanelContainer<T>, panel)`, and
+// stores the block's base address in `panel->container`.
+//
+// Lifetime:
+//   1. A devres action registered right after allocation calls `drop_in_place`
+//      on the `data` field (T's destructor) when the device is unbound.
+//   2. `__drm_panel_free` calls `kfree(panel->container)` when the kref hits
+//      zero, freeing the entire block.
+//
+// `data` is `ManuallyDrop<T>` so that Rust does not implicitly drop it; the
+// devres action owns the destructor call.
+#[repr(C)]
+struct PanelContainer<T> {
+    data: ManuallyDrop<T>,
+    panel: MaybeUninit<bindings::drm_panel>,
+}
+
+// Devres action: run T's destructor before `kfree(container)`.
+//
+// # Safety
+//
+// `ptr` must be the base of a live `PanelContainer<T>` whose `data` field was
+// initialised by `Panel::new` and has not yet been dropped.
+unsafe extern "C" fn drop_panel_data<T>(ptr: *mut core::ffi::c_void) {
+    // SAFETY: Caller guarantees `ptr` is the base of a live `PanelContainer<T>`
+    // with an initialised `data` field. `data` is at offset 0, so `ptr as *mut T`
+    // is valid.
+    unsafe { core::ptr::drop_in_place(ptr as *mut T) };
+}
+
+/// A vtable for the DRM core to interact with a panel driver.
+///
+/// A `bindings::drm_panel_funcs` vtable is constructed from pointers to the
+/// `extern "C"` functions of this struct, exposed through
+/// `PanelFuncsVTable::VTABLE`.
+///
+/// For general documentation of these methods, see the kernel source
+/// documentation related to `struct drm_panel_funcs` in
+/// [`include/drm/drm_panel.h`].
+///
+/// [`include/drm/drm_panel.h`]: srctree/include/drm/drm_panel.h
+pub(crate) struct PanelFuncsVTable<T: PanelFuncs>(PhantomData<T>);
+
+impl<T: PanelFuncs> PanelFuncsVTable<T> {
+    // Recover &T from panel->container.
+    //
+    // # Safety
+    //
+    // `panel` must be a valid pointer to a live `drm_panel` allocated by
+    // `Panel::new`, whose `container` field points to the base of a live
+    // `PanelContainer<T>` with an initialised `data` field.
+    unsafe fn data_from_panel<'a>(panel: *mut bindings::drm_panel) -> &'a T {
+        // SAFETY: Caller guarantees `panel` is valid and `panel->container` points
+        // to the base of a live `PanelContainer<T>` with an initialised `data`
+        // field. `data` is at offset 0, so `container as *const T` is valid.
+        unsafe { &*((*panel).container as *const T) }
+    }
+
+    unsafe extern "C" fn prepare_callback(panel: *mut bindings::drm_panel) -> i32 {
+        // SAFETY: The C DRM core only invokes callbacks on a live, initialised
+        // panel allocated by `Panel::new` (see `data_from_panel`).
+        let data = unsafe { Self::data_from_panel(panel) };
+        // SAFETY: `panel` is a valid `drm_panel` pointer per the callback contract.
+        let panel_ref = unsafe { Panel::from_raw(panel) };
+        match T::prepare(data, panel_ref) {
+            Ok(()) => 0,
+            Err(e) => e.to_errno(),
+        }
+    }
+
+    unsafe extern "C" fn unprepare_callback(panel: *mut bindings::drm_panel) -> i32 {
+        // SAFETY: The C DRM core only invokes callbacks on a live, initialised
+        // panel allocated by `Panel::new` (see `data_from_panel`).
+        let data = unsafe { Self::data_from_panel(panel) };
+        // SAFETY: `panel` is a valid `drm_panel` pointer per the callback contract.
+        let panel_ref = unsafe { Panel::from_raw(panel) };
+        match T::unprepare(data, panel_ref) {
+            Ok(()) => 0,
+            Err(e) => e.to_errno(),
+        }
+    }
+
+    unsafe extern "C" fn enable_callback(panel: *mut bindings::drm_panel) -> i32 {
+        // SAFETY: The C DRM core only invokes callbacks on a live, initialised
+        // panel allocated by `Panel::new` (see `data_from_panel`).
+        let data = unsafe { Self::data_from_panel(panel) };
+        // SAFETY: `panel` is a valid `drm_panel` pointer per the callback contract.
+        let panel_ref = unsafe { Panel::from_raw(panel) };
+        match T::enable(data, panel_ref) {
+            Ok(()) => 0,
+            Err(e) => e.to_errno(),
+        }
+    }
+
+    unsafe extern "C" fn disable_callback(panel: *mut bindings::drm_panel) -> i32 {
+        // SAFETY: The C DRM core only invokes callbacks on a live, initialised
+        // panel allocated by `Panel::new` (see `data_from_panel`).
+        let data = unsafe { Self::data_from_panel(panel) };
+        // SAFETY: `panel` is a valid `drm_panel` pointer per the callback contract.
+        let panel_ref = unsafe { Panel::from_raw(panel) };
+        match T::disable(data, panel_ref) {
+            Ok(()) => 0,
+            Err(e) => e.to_errno(),
+        }
+    }
+
+    unsafe extern "C" fn get_modes_callback(
+        panel: *mut bindings::drm_panel,
+        connector: *mut bindings::drm_connector,
+    ) -> i32 {
+        // SAFETY: The C DRM core only invokes callbacks on a live, initialised
+        // panel allocated by `Panel::new` (see `data_from_panel`).
+        let data = unsafe { Self::data_from_panel(panel) };
+        // SAFETY: `panel` is a valid `drm_panel` pointer per the callback contract.
+        let panel_ref = unsafe { Panel::from_raw(panel) };
+        // SAFETY: `connector` is a valid, non-null `drm_connector` pointer
+        // supplied by the DRM core for the duration of the callback.
+        let connector_ref = unsafe { Connector::from_raw(connector) };
+        T::get_modes(data, panel_ref, connector_ref)
+    }
+
+    unsafe extern "C" fn get_orientation_callback(panel: *mut bindings::drm_panel) -> i32 {
+        // SAFETY: The C DRM core only invokes callbacks on a live, initialised
+        // panel allocated by `Panel::new` (see `data_from_panel`).
+        let data = unsafe { Self::data_from_panel(panel) };
+        // SAFETY: `panel` is a valid `drm_panel` pointer per the callback contract.
+        let panel_ref = unsafe { Panel::from_raw(panel) };
+        T::get_orientation(data, panel_ref) as i32
+    }
+
+    const VTABLE: bindings::drm_panel_funcs = bindings::drm_panel_funcs {
+        // Initialize optional callbacks based on the traits of `T`.
+        prepare: if T::HAS_PREPARE {
+            Some(Self::prepare_callback)
+        } else {
+            None
+        },
+        unprepare: if T::HAS_UNPREPARE {
+            Some(Self::unprepare_callback)
+        } else {
+            None
+        },
+        enable: if T::HAS_ENABLE {
+            Some(Self::enable_callback)
+        } else {
+            None
+        },
+        disable: if T::HAS_DISABLE {
+            Some(Self::disable_callback)
+        } else {
+            None
+        },
+        get_orientation: if T::HAS_GET_ORIENTATION {
+            Some(Self::get_orientation_callback)
+        } else {
+            None
+        },
+
+        // Initialize mandatory callbacks.
+        get_modes: Some(Self::get_modes_callback),
+
+        get_timings: None,
+        debugfs_init: None,
+    };
+
+    pub(crate) const fn build() -> &'static bindings::drm_panel_funcs {
+        &Self::VTABLE
+    }
+}

-- 
2.55.0

Re: [PATCH 4/5] rust: drm: add panel producer abstractions

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Critical] `Panel::new` steals the devres-owned reference count initialized by `__devm_drm_panel_alloc`, leading to a double-free on device unbind or drop.
- [Critical] The destructor of the driver data `T` is tied to device unbind via devres, rather than the panel's actual reference count, creating a split-lifetime Use-After-Free.
- [High] Missing `Send`, `Sync`, and `'static` bounds on the driver data type `T` in `Panel::new` allows data races and Use-After-Free via non-thread-safe or short-lived types.

Pre-existing issues:
- [Critical] `Panel::from_of_node` improperly converts the result of `of_drm_find_panel` into an `ARef<Panel>` without incrementing the refcount, causing a premature free.
--

--- Patch [4]: [PATCH 4/5] rust: drm: add panel producer abstractions ---
commit 0e243089bb12e5256a62894a39811280cadcb027
Author: Albert Esteve <aesteve@redhat.com>
Subject: rust: drm: add panel producer abstractions

This commit adds abstractions for DRM panel drivers, introducing the
PanelFuncs trait and PanelContainer struct. It provides devres-managed
allocation via __devm_drm_panel_alloc and ties the lifetime of the 
data to the device.

> diff --git a/rust/kernel/drm/panel.rs b/rust/kernel/drm/panel.rs
> index fd21cc2236685..8f87774e06ed7 100644
> --- a/rust/kernel/drm/panel.rs
> +++ b/rust/kernel/drm/panel.rs

[ ... ]

> @@ -139,6 +143,66 @@ pub fn from_of_node(node: &of::Node) -> Result<ARef<Self>> {
>          // `of_drm_find_panel` returns a kref-incremented reference.
>          Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(panel).cast()) })
>      }

[Severity: Critical]
This is a pre-existing issue, but does of_drm_find_panel actually increment
the reference count?

Looking at the C implementation, of_drm_find_panel iterates over the global
panel_list and returns the pointer without calling kref_get or
drm_panel_get. If we consume the pointer directly with ARef::from_raw(),
dropping the ARef will decrement the refcount. Could this drop the refcount
to zero and prematurely free the panel while the provider driver's devres
still owns it?

> +
> +    /// Allocates and initialises a device-managed panel.
> +    ///
> +    /// `data` is embedded in the same allocation as the `drm_panel` and its
> +    /// destructor is called automatically when `dev` is unbound.
> +    ///
> +    /// Use [`Registration::register`] to add the panel to the global registry
> +    /// once it is ready to be used by display drivers.
> +    pub fn new<T: PanelFuncs>(
> +        dev: &Device,
> +        data: T,
> +        connector_type: ConnectorType,
> +    ) -> Result<ARef<Self>> {

[Severity: High]
Does T need Send, Sync, and 'static bounds here?

Since Panel::new takes ownership of T and stores it in a heap allocation
accessed by the concurrent C DRM subsystem, safe Rust code could provide a
non-thread-safe type (like one containing Cell or Rc) or a short-lived
reference. Would this allow data races or use-after-free when the C DRM
core concurrently invokes vtable callbacks?

> +        // SAFETY: `dev` is valid by its type invariants; `PanelFuncsVTable::build()`
> +        // returns a valid, static `drm_panel_funcs` pointer.
> +        let container = error::from_err_ptr(unsafe {
> +            bindings::__devm_drm_panel_alloc(
> +                dev.as_raw(),
> +                core::mem::size_of::<PanelContainer<T>>(),
> +                core::mem::offset_of!(PanelContainer<T>, panel),
> +                PanelFuncsVTable::<T>::build(),
> +                connector_type as i32,
> +            )
> +        })? as *mut PanelContainer<T>;
> +
> +        // SAFETY: `container` is a valid pointer to uninitialized memory.
> +        unsafe {
> +            core::ptr::write(
> +                core::ptr::addr_of_mut!((*container).data),
> +                ManuallyDrop::new(data),
> +            )
> +        };
> +
> +        // SAFETY:
> +        // - `dev.as_raw()` is a pointer to a valid and bound device.
> +        // - `container.cast()` is a valid pointer to the initialized `PanelContainer<T>`.
> +        error::to_result(unsafe {
> +            // `devm_add_action_or_reset` calls `drop_panel_data` on failure, so `data`
> +            // is dropped even if this registration fails.
> +            // Registering after `__devm_drm_panel_alloc` ensures devres LIFO order:
> +            // `drop_panel_data` runs before `kfree(container)`.
> +            bindings::devm_add_action_or_reset(
> +                dev.as_raw(),
> +                Some(drop_panel_data::<T>),
> +                container.cast(),
> +            )

[Severity: Critical]
Is it safe to tie the destruction of T to device unbind rather than the
panel's reference count?

Since the panel is reference-counted and exposed as an ARef<Panel>, consumer
drivers could keep the panel alive after the provider device unbinds. When
unbind occurs, drop_panel_data will destroy T. If a consumer later calls
a panel method like get_modes, the C code will invoke the Rust vtable
callbacks which operate on the dropped T, potentially leading to a
use-after-free.

> +        })?;
> +
> +        // SAFETY: `__devm_drm_panel_alloc` was successful, hence `container` is
> +        // valid and the `drm_panel` at this offset is initialised.
> +        let raw = unsafe {
> +            (container as *mut u8)
> +                .add(core::mem::offset_of!(PanelContainer<T>, panel))
> +                .cast::<bindings::drm_panel>()
> +        };
> +
> +        // SAFETY: `__devm_drm_panel_alloc` was successful, hence `raw` is valid
> +        // and the refcount is non-zero.
> +        Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(raw).cast()) })

[Severity: Critical]
Does ARef::from_raw steal the devres-owned reference count here?

__devm_drm_panel_alloc initializes the panel's refcount to 1 and immediately
registers a devres action (drm_panel_put_void) that owns this reference.
Wrapping the raw pointer directly with ARef::from_raw without incrementing
the refcount means both ARef and devres think they own the same refcount.

When the ARef is dropped, won't it free the panel via __drm_panel_free,
causing a double-free later when device unbind triggers the devres action?

> +    }
>  }

[ ... ]

> +/// Operations implemented by a DRM panel driver.
> +///
> +/// Implement this trait to provide a DRM panel driver and its callbacks. Use
> +/// [`Panel::new`] to allocate the panel, passing the driver data as `T`.
> +///
> +/// C header: [`include/drm/drm_panel.h`](srctree/include/drm/drm_panel.h)
> +#[vtable]
> +pub trait PanelFuncs {

[Severity: High]
Should this trait require Send, Sync, and 'static supertraits?

Similar to the missing bounds in Panel::new(), without these supertraits,
types implementing PanelFuncs might contain thread-unsafe primitives
or short-lived references, which could cause data races or use-after-free
when accessed concurrently by the DRM subsystem.

> +    /// Turn on panel and perform set up.

[ ... ]

> +// Devres action: run T's destructor before `kfree(container)`.
> +//
> +// # Safety
> +//
> +// `ptr` must be the base of a live `PanelContainer<T>` whose `data` field was
> +// initialised by `Panel::new` and has not yet been dropped.
> +unsafe extern "C" fn drop_panel_data<T>(ptr: *mut core::ffi::c_void) {
> +    // SAFETY: Caller guarantees `ptr` is the base of a live `PanelContainer<T>`
> +    // with an initialised `data` field. `data` is at offset 0, so `ptr as *mut T`
> +    // is valid.
> +    unsafe { core::ptr::drop_in_place(ptr as *mut T) };

[Severity: Critical]
Does this execute the destructor while the container might still be alive?

If a consumer driver still holds an ARef<Panel>, the container's reference
count is non-zero. Since drop_panel_data is called unconditionally on
device unbind, won't drop_in_place destroy T while it could still be
accessed via the panel's vtable callbacks?

> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-drm_panel_bindings-v1-0-1f974508a31c@redhat.com?part=4

[PATCH 5/5] rust: drm: add KUnit tests for panel

[Albert Esteve]

Add a test module to verify basic data type conversions
and layout. The tests covers:

- PanelOrientation and ConnectorType: TryFrom conversion for valid
  values and also out-of-range invalid inputs.
- PanelContainer: layout invariants (offset, alignment, size) on which
  Panel::new relies on for memory access.

Tests are gated on CONFIG_RUST_DRM_PANEL_KUNIT_TEST.

Signed-off-by: Albert Esteve <aesteve@redhat.com>
---
 rust/kernel/Kconfig.test | 10 ++++++
 rust/kernel/drm/panel.rs | 87 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 97 insertions(+)

diff --git a/rust/kernel/Kconfig.test b/rust/kernel/Kconfig.test
index e6a5c7a795f0f..397b916dca0d4 100644
--- a/rust/kernel/Kconfig.test
+++ b/rust/kernel/Kconfig.test
@@ -83,4 +83,14 @@ config RUST_BITFIELD_KUNIT_TEST
 
 	  If unsure, say N.
 
+config RUST_DRM_PANEL_KUNIT_TEST
+	bool "KUnit tests for Rust panel API" if !KUNIT_ALL_TESTS
+	default KUNIT_ALL_TESTS
+	help
+	  This option enables KUnit tests for the Rust panel API.
+	  These are only for development and testing, not for regular
+	  kernel use cases.
+
+	  If unsure, say N.
+
 endif
diff --git a/rust/kernel/drm/panel.rs b/rust/kernel/drm/panel.rs
index 8f87774e06ed7..3da203c39edd6 100644
--- a/rust/kernel/drm/panel.rs
+++ b/rust/kernel/drm/panel.rs
@@ -595,3 +595,90 @@ pub(crate) const fn build() -> &'static bindings::drm_panel_funcs {
         &Self::VTABLE
     }
 }
+
+#[cfg(CONFIG_RUST_DRM_PANEL_KUNIT_TEST)]
+#[macros::kunit_tests(rust_kernel_drm_panel)]
+mod tests {
+    use super::*;
+    use core::mem::{align_of, offset_of, size_of, ManuallyDrop, MaybeUninit};
+
+    struct TestData {
+        _ptr: *const u8,
+        _byte: u8,
+    }
+
+    #[test]
+    fn panel_orientation() {
+        // Test valid values.
+        assert!(matches!(
+            PanelOrientation::try_from(-1),
+            Ok(PanelOrientation::Unknown)
+        ));
+        assert!(matches!(
+            PanelOrientation::try_from(0),
+            Ok(PanelOrientation::Normal)
+        ));
+        assert!(matches!(
+            PanelOrientation::try_from(1),
+            Ok(PanelOrientation::BottomUp)
+        ));
+        assert!(matches!(
+            PanelOrientation::try_from(2),
+            Ok(PanelOrientation::LeftUp)
+        ));
+        assert!(matches!(
+            PanelOrientation::try_from(3),
+            Ok(PanelOrientation::RightUp)
+        ));
+
+        // Test invalid values.
+        assert!(PanelOrientation::try_from(4).is_err());
+        assert!(PanelOrientation::try_from(-2).is_err());
+    }
+
+    #[test]
+    fn connector_type() {
+        // Test valid values.
+        assert!(matches!(
+            ConnectorType::try_from(0),
+            Ok(ConnectorType::Unknown)
+        ));
+        assert!(matches!(
+            ConnectorType::try_from(10),
+            Ok(ConnectorType::DisplayPort)
+        ));
+        assert!(matches!(
+            ConnectorType::try_from(14),
+            Ok(ConnectorType::eDP)
+        ));
+        assert!(matches!(
+            ConnectorType::try_from(16),
+            Ok(ConnectorType::DSI)
+        ));
+        assert!(matches!(
+            ConnectorType::try_from(20),
+            Ok(ConnectorType::USB)
+        ));
+
+        // Test invalid values.
+        assert!(ConnectorType::try_from(21).is_err());
+    }
+
+    #[test]
+    fn panel_container_layout() {
+        assert_eq!(offset_of!(PanelContainer<TestData>, data), 0);
+
+        assert_eq!(size_of::<ManuallyDrop<TestData>>(), size_of::<TestData>());
+
+        let panel_offset = offset_of!(PanelContainer<TestData>, panel);
+        assert_eq!(
+            panel_offset % align_of::<MaybeUninit<bindings::drm_panel>>(),
+            0
+        );
+
+        assert!(
+            size_of::<PanelContainer<TestData>>()
+                >= panel_offset + size_of::<bindings::drm_panel>()
+        );
+    }
+}

-- 
2.55.0

Re: [PATCH 5/5] rust: drm: add KUnit tests for panel

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [Low] Missing Kconfig dependency on DRM for RUST_DRM_PANEL_KUNIT_TEST
- [Low] Horizontal import formatting violates subsystem guidelines.

Pre-existing issues:
- [Critical] `Panel::from_of_node` creates an `ARef<Panel>` without incrementing the reference count, leading to premature free and use-after-free.
- [Critical] `Panel::new` improperly steals the devres-managed reference count, leading to use-after-free and double-free.
--

--- Patch [5]: [PATCH 5/5] rust: drm: add KUnit tests for panel ---
commit 075cc8b6578147fc552359e30a2b237e1bdb801a
Author: Albert Esteve <aesteve@redhat.com>

rust: drm: add KUnit tests for panel

Add a test module to verify basic data type conversions and layout for
the Rust DRM panel API.

> diff --git a/rust/kernel/Kconfig.test b/rust/kernel/Kconfig.test
> index e6a5c7a795f0f..397b916dca0d4 100644
> --- a/rust/kernel/Kconfig.test
> +++ b/rust/kernel/Kconfig.test
> @@ -83,4 +83,14 @@ config RUST_BITFIELD_KUNIT_TEST
>  
>  	  If unsure, say N.
>  
> +config RUST_DRM_PANEL_KUNIT_TEST
> +	bool "KUnit tests for Rust panel API" if !KUNIT_ALL_TESTS

[Severity: Low]
Will this Kconfig option be available even when CONFIG_DRM is disabled?

Since the tests reside in rust/kernel/drm/panel.rs which is compiled
conditionally under CONFIG_DRM, could this result in a dead configuration
option where Kconfig allows enabling the test without DRM, causing the
code to be silently skipped during compilation?

> +	default KUNIT_ALL_TESTS
> +	help
> +	  This option enables KUnit tests for the Rust panel API.
> +	  These are only for development and testing, not for regular
> +	  kernel use cases.
> +
> +	  If unsure, say N.
> +
>  endif
> diff --git a/rust/kernel/drm/panel.rs b/rust/kernel/drm/panel.rs
> index 8f87774e06ed7..3da203c39edd6 100644
> --- a/rust/kernel/drm/panel.rs
> +++ b/rust/kernel/drm/panel.rs
> @@ -595,3 +595,90 @@ pub(crate) const fn build() -> &'static bindings::drm_panel_funcs {
>          &Self::VTABLE
>      }
>  }
> +
> +#[cfg(CONFIG_RUST_DRM_PANEL_KUNIT_TEST)]
> +#[macros::kunit_tests(rust_kernel_drm_panel)]
> +mod tests {
> +    use super::*;
> +    use core::mem::{align_of, offset_of, size_of, ManuallyDrop, MaybeUninit};

[Severity: Low]
Does this horizontal import list violate the Rust subsystem guidelines?

The guidelines mandate following the kernel vertical import style when
commits touch imports, so shouldn't each import be placed on its own line?

[ ... ]

While reviewing this file, I noticed a couple of pre-existing issues in
rust/kernel/drm/panel.rs:

rust/kernel/drm/panel.rs:from_of_node() {
    ...
    // SAFETY: `from_err_ptr` guarantees a non-null pointer on success.
    // `of_drm_find_panel` returns a kref-incremented reference.
    Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(panel).cast()) })
}

[Severity: Critical]
This is a pre-existing issue, but I noticed that of_drm_find_panel() returns
a pointer without calling drm_panel_get() to increment its reference count.

Since ARef::from_raw() takes ownership of a reference count increment that
doesn't actually exist, will dropping the returned ARef<Panel> decrement the
panel's kref via drm_panel_put(), effectively stealing a reference and
causing the panel to be prematurely freed?


rust/kernel/drm/panel.rs:new() {
    ...
    let raw = unsafe {
        (container as *mut u8)
            .add(core::mem::offset_of!(PanelContainer<T>, panel))
            .cast::<bindings::drm_panel>()
    };

    // SAFETY: `__devm_drm_panel_alloc` was successful, hence `raw` is valid
    // and the refcount is non-zero.
    Ok(unsafe { ARef::from_raw(NonNull::new_unchecked(raw).cast()) })
}

[Severity: Critical]
This isn't a bug introduced by this patch, but does Panel::new steal the
devres-managed reference count here?

__devm_drm_panel_alloc() allocates the panel, initializes the kref to 1,
and registers a devres action that owns this initial kref. By wrapping the
returned pointer in ARef::from_raw(), we assume ownership of an existing
reference count without incrementing it.

If the ARef is dropped, the kref reaches 0 and the panel is freed. Later,
when the device unbinds, won't devres execute its action on the freed
memory, causing a use-after-free or double-free?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817-drm_panel_bindings-v1-0-1f974508a31c@redhat.com?part=5